The remainder of the package-wide sweep's genuine survivors.
view-model.ts `truncate(firstLine(output), …)` -> `truncate(output, …)`
The tool preview keeps its newlines, so a one-line row renders as
several. Every `ls`, every stack trace, every file read breaks the
transcript's layout.
decode.ts drop `|| type === ""`
Survived my OWN earlier test, which asserted only that the frame was
refused. It still is -- but as `unknown_event` with an empty detail
instead of `missing_type` with the key list. The dropped-events row then
blames the engine for sending something unrecognised when what arrived
had no type at all, and a reason that names the wrong failure sends
whoever reads it to the wrong place.
bundle.ts `isMarked` `&&` -> `||`
"half and "half both report as marked-missing, so a translator's literal
bracket reads as a missing key and the report names strings that are
present.
segment.ts `end === 0` -> `end === 1`
A chunk whose only character is a terminator reports incomplete, so
`completedLength` returns 0 and the screen-reader announcement stalls.
CJK writes short sentences; a lone "。" is a whole one.
format.ts `seconds < 45` -> `< 44`, and session.ts `<= 60` -> `< 60`
Threshold and boundary, each the entire contract of its function, and
neither had a value placed on it. format-intl.ts has the same 45s
boundary and is pinned separately -- the two must not drift.
AND THE FIFTH FAKE
`testing/src/fake-http.ts` had no test file. Two of its behaviours are
load-bearing for every engine test written against it:
The route table picks the LONGEST matching suffix, so `/chats` does not shadow
`/chat`. Inverting it silently mis-routes a test to the wrong handler and the
test still passes -- against the wrong thing. I have already been bitten by
this file's suffix matching once this session, when a handler registered for
`/approve` never matched `/approve/{id}` and a test went green without
exercising what it was written for.
And `streamOf` chunks its body deliberately, "because a frame split across
chunks is the normal case on a real network". Emitting an extra zero-length
chunk changes what the SSE reader is exercised with -- and that reader's
pending-CR logic is precisely sensitive to where boundaries fall. A fake that
stops splitting properly can mask the CRLF defect this package shipped once
already.
That is the fifth fake here to get its own tests after being caught
misrepresenting what it stands in for, after setTimer's delay, every's period,
advance's amount, and the fake DOM that appended instead of ordering.
1121 tests on node 22.23 and 24.12. Eight mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`engines/` measured 46.7% genuine mutation survival -- more than double the
package average and 4.5x core -- with six of seven survivors on transport
failure or early exit. The last three.
praisonai-ts/engine.ts `if (signal.aborted) controller.abort()` deleted
The `else` branch attaches an abort LISTENER, which never fires for a
signal that went down before it was attached. So a run handed an
already-aborted signal streams to completion and PERSISTS an answer
nobody is waiting for. Reachable through dispose-then-new-run, and
through Stop followed immediately by Send.
praisonai-ts/classify.ts `status >= 500` -> `> 500`
A plain 500 -- the most common provider failure there is -- classifies
as `internal`, whose recovery is `none`. The user is told something went
wrong and offered no way to try again. 502 and 503 are unaffected, so
the boundary lands exactly on the case that matters most.
remote-http/readiness.ts `probe.status !== 200` -> `> 200`
A sub-200 status (100, 101, 199) falls through and is classified from its
BODY, so a websocket upgrade or a "continue" reports ready. The engine is
then offered as usable and every request to it fails.
Each is paired: a live signal must still stream, a 4xx must NOT be transport
(offering Retry for a revoked key can never succeed), and a 200 must still be
classified from its body.
A NEW BREAK MODE FOR THE ENGINE CONTRACT
`start_only` -- a run that opens and then simply stops, with no terminal event
at all. It is now asserted to redden "a run emits start first and exactly one
terminal event last" by name.
Writing it also settled a survivor I had listed as a gap: weakening that case's
`events.length >= 2` to `>= 1` does NOT let a start-only engine through. Nor
does weakening the terminal COUNT as well. The case catches it either way, so
the length assertion is redundant rather than unguarded -- worth knowing, and
worth not chasing.
1107 tests on node 22.23 and 24.12. Three mutations confirmed to fail; the
fourth proved redundant rather than surviving.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The remaining tier-1 survivors from the package-wide sweep.
main.ts `? actionStop : actionSend` transposed
The button READS "Send" while the turn is streaming, and tapping it stops
the run. The line below sets `dataset.action` and is untouched, so label
and behaviour disagree -- and nothing in 1091 tests read the label.
transcript.ts `blocks.slice(0, -1)` -> `slice(0, -2)`
Appending to the trailing text block dropped the block BEFORE it too. Any
answer that keeps talking after a tool call loses the tool row and its
output: the user sees two paragraphs and no evidence a tool ever ran.
a11y/names.ts `row.title === ""` -> `!== ""`
Both directions at once: a real title announced as "Untitled", and an
untitled row announced as an EMPTY accessible name. A nameless row reads
as just "button" and cannot be told apart from the one above it, which
the comment two lines above says is the whole point of the function.
tauri/shell.ts `sameInsets` without `right`
A landscape notch appearing on the right is deduped away as "no change",
so content sits under it after a rotation.
tauri/shell.ts `coerceInsets` requiring ALL four edges
A native side that sends only what moved has every event discarded, and
the insets stay at whatever the first CSS read gave them -- the composer
under the home indicator for the life of the app. Its pair: an EMPTY
payload must mean "re-read the CSS", not "every inset is zero".
WHERE EACH TEST HAD TO GO, AND WHY THAT MATTERS
Two of these could not live in the shared shell contract, and finding out why
was the useful part.
`emitInsets` takes a full `SafeAreaInsets`, so the harness cannot express a
PARTIAL native payload -- which is exactly the input `coerceInsets`'s guard is
about. And the inset dedupe is a Tauri-only optimisation: the fake and web
shells republish an identical payload, so asserting otherwise in the shared
contract failed two of three implementations. I wrote it there first and had to
move it.
A contract can only assert what every implementation owes. Behaviour that is
one adapter's alone needs its own test, and mistaking the two is how a case
either fails for the wrong shells or never reaches the branch at all.
1100 tests on node 22.23 and 24.12. Seven mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A package-wide sweep of 154 mutations on current main put genuine survival at
23.8%, against 26.3% originally -- an improvement inside the sampling error,
so flat package-wide. The layer split is the actionable result:
engines 46.7% <-- weakest, 4.5x core
adapters 29.2%
ui 20.5%
app 15.4%
core 10.3% <-- strongest
Six of the seven engine survivors were on TRANSPORT-FAILURE or EARLY-EXIT
paths. The conformance harness covers the happy path thoroughly; nothing
covered these, and a phone spends a disproportionate amount of its life there.
postOk: `if (response.status !== 200) return false` deleted
Every non-200 -- 202, 401, 500, 502 -- reports SUCCESS. The UI announces
a stop that never happened and marks an approval sent that the engine
never received. Both callers were written to trust this boolean.
the reader's `cancel()` in the finally, deleted
On New chat or Stop the consumer breaks out of the loop; without the
cancel the engine keeps generating into a socket nobody drains, and keeps
billing. Proved with a stream that records its own cancellation.
`{ ...parsed.value, type: frame.event }` transposed
A frame whose data object carries its own `type` is then decoded as THAT
type. A delta arriving as an ill-formed `error` is dropped silently, so
text vanishes from the answer with no diagnostic at all.
AND THE CHECKER STOPPED CHECKING ITSELF
`tools/check-boundaries.mjs` walked `.ts` and `.mjs`. Dropping the `.mjs` half
survived: it silently stops walking every .mjs in the package -- and `tools/`
is entirely .mjs -- while still printing "136 files checked, no violations".
That is verbatim the failure boundaries.json's own comment warns about: "a rule
that stops covering new code while still reporting a clean pass is worse than
no rule." The walk is now `sourceFilesUnder(base, roots)` in depgraph.mjs so a
test can call it, following the same move already made for `ungovernedRootsIn`.
ONE TEST OF MINE PASSED FOR THE WRONG REASON
The refusal test registered `http.on("/approve", ...)`, and the fake matches by
URL SUFFIX -- the engine posts to `/approve/{approvalId}`, so nothing matched
and every request 404'd. A 404 is not 200, so the test went green while never
exercising the status check it was written for. Its paired "a 200 with an ok
body IS accepted" case failed, which is the only reason I noticed.
1091 tests on node 22.23 and 24.12. Four mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The padded-URL case only asserted resolve/reject, so the fake and web shells
forwarded the raw padded string to the OS and still passed — the exact
validate-one-form/forward-another seam the Tauri-specific test was written for.
Surface the forwarded URLs through ShellHarness.forwarded() and assert the
trimmed value, then trim in the web and fake shells to match the port contract.
Co-authored-by: Mervin Praison <[email protected]>
The production-side findings from the same re-measure, verified live against
merged main before any were touched.
main.ts `setChat(mintChatId())` -> `setChat("unassigned")`
"unassigned" is what controller.ts uses to mean "nobody has said which
conversation this is". Shipping it to an engine keying history by chat_id
merges conversations. The existing test only asserted two ids DIFFER --
and the launch id is real now, so a placeholder on turn two passed it.
main.ts delete `input.value = ""`
The sent text stays in the composer, so the next Send tap re-submits the
SAME question. The user double-sends and pays twice, with nothing on
screen to say so.
platform.ts `view.localStorage` -> `view.sessionStorage`
Every chat dies with the tab -- on a phone, every time the OS reclaims
the webview. The file's own comment is about localStorage EVICTION being
the risk; nothing asserted which store it reached for.
web/http.ts `status: response.status` -> `status: 200`
Every 401, 403, 429 and 502 reported as success. The engine reads the
status to choose `auth` vs `transport`, so the whole recovery
distinction -- go to settings, or retry -- was fed a constant. The tests
that DO cover that distinction drive the fake http, so the one place the
real status is read had nothing on it.
tauri/shell.ts `url.trim()` -> `url`
The interesting one. It survived EVERY contract case, including the new
padded-URL case I wrote for it, because a contract can only observe
REJECTION and the allowlist tolerates padding either way. What changes is
what reaches the OS.
A URL validated in one form and forwarded in another is the shape of a
scheme-confusion bypass, and nothing in the suite read what the bridge was
handed. The probe bridge now records it, and the assertion is on the forwarded
string rather than on whether the promise settled. Mutating the forward to pass
the raw `url` instead of `target` also fails now -- so the test observes the
seam, not just the trim.
That is the general lesson of this one: `doesNotReject` proves a call did not
throw. It does not prove the call did the right thing, and for a security
boundary that gap is the whole question.
1085 tests on node 22.23 and 24.12. Six mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A re-measure of the regions a previous sweep named. The production rates DID
fall -- main.ts 74.4% -> 28.6%, adapters 44.3% -> 24.1%, i18n 43.8% -> 34.8%,
boot.ts 30.8% -> 22.2%. Those fixes were structural.
The survival moved into the machinery I added to guard them.
THE SHELL CONTRACT WAS COMPLETELY UNDEFENDED
`contract-fixture.ts` registered secrets, storage and time. Not shell -- the
largest of the four, 33 cases, including seven `openExternal` refusals. No
break mode, no floor. Measured: every assertion in it deletable, the whole
security section removable, the entire contract replaceable with
`assert.ok(shell)`, green each time. 57 of 61 assertions across all four
contracts were free; exactly four were load-bearing.
A `javascript:` URL reaching `openExternal` inside a webview is script
execution in the app's own origin with the user's session, from a URL that
routinely arrives from a model, a tool result, or pasted text. The RULE is in
core/src/ports/shell.ts and both adapters call it. What was undefended is the
contract proving they do -- and the next shell, React Native, would be written
against a contract that cannot fail.
Registered now, with three break modes: a shell that opens anything, a back
stack in registration order, and a listener count that cannot see a leak.
Built by wrapping a real web shell rather than hand-rolling a ShellPort, so
the fixture cannot drift from the interface it exercises.
TWO HOLES THE FLOOR HAD
`# SKIP` is reported as `ok` in TAP, so changing `test(` to `test.skip(` on a
security case kept the run green -- a case could be disabled rather than
deleted and the count never moved. The floor now excludes skips and todos.
And the meta-test filtered for `not ok` lines and matched separately, so
neutering the filter let a PASSING line satisfy the assertion: it went green
precisely when the contract stopped catching anything. The prefix is part of
the pattern now.
A CONTRACT CASE TOO WEAK TO CATCH ITS OWN MUTATION
"the most recently registered back handler gets first refusal" registers two
DIFFERENT closures, so `indexOf` and `lastIndexOf` behave identically and the
mutation between them survived in BOTH shells. React StrictMode double-invokes
effects, so the same reference registered twice is ordinary. The new case
registers A, B, A and drops the second A; removing the first instead inverts
the stack and the back press goes to the wrong screen.
WHERE THIS STOPS
Lowering a floor, or rewriting the meta-test's assertion back to its weaker
form, is not caught by anything -- and cannot be without another guard above
it. The regress has to end somewhere. It ends at code review, and this commit
moves the line up: the contracts, the cases, the break table and the skip
counting are all defended mechanically now. Only edits to the guard itself are
not, and those read as what they are in a diff.
1077 tests on node 22.23 and 24.12. boundaries: 136 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The `every no-Intl fallback differs from its formatted twin` control only
compared number/count/date. Elapsed and relative-time render byte-identical in
English through both Intl and the fallback ("1h 02m", "10 minutes ago"), so an
"en" twin can't distinguish the fallback branch from a successful memo(). Added
de-DE twins (comma decimal, native relative phrasing) that genuinely differ from
the ASCII/English en_US fallback, so the control now guards all seven paths.
Co-authored-by: Mervin Praison <[email protected]>
The last of the regions a mutation sweep named. `format-intl.ts` measured 67%
survival for one reason: every `fmt === null` branch is unreachable on a host
with full ICU, which is every host a test runs on. `memo(...)` always returns a
formatter, so seven fallbacks never executed.
They are not exotic old-WebView paths. Every Intl constructor throws
RangeError on an underscore tag, and locale.ts says in its own header that a
stored preference reaches us as "en_US" as often as "en-US". These are one bad
settings value away, in production, today.
Six mutations that survived, and what each shipped:
String(value) -> "" a number renders as nothing
String(whole) -> whole + 1 every count off by one
padStart(2, "0") dropped "1h 2m" instead of "1h 02m", which scans
and sorts wrongly in a list of durations
toFixed(1) dropped sub-10s durations lose their decimal, and
that decimal is the whole reason the branch
exists -- "3s" hides 2.5 from 3.4
slice(0, 10) -> slice(0, 7) every date in the chat list loses its day
and reads as a month
formatRelativeFromStrings "10 minutes ago" becomes "10"
replaced by the raw value
No export was needed, unlike `segment.ts` and `locale.ts` -- these functions
are already public and the fallback is reachable through the ordinary API. The
gap was never structural; nothing had passed a locale that fails.
Each assertion is paired with a check that the FORMATTED output differs from
the fallback, so the block cannot start passing for the wrong reason if
`memo()` ever succeeds for "en_US" or the formatted path degrades.
One of my expectations was wrong rather than the code: I asserted 5.25 seconds
renders "5.2s"; `toFixed(1)` rounds it to "5.3s".
1071 tests on node 22.23 and 24.12. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`app/src/main.ts` measured 74% mutation survival -- the highest of any file.
The tests it had each pinned one previously-reported defect and nothing around
it, which is what happens when tests are written from a bug list rather than
from what the code does.
Eight mutations that survived, and what each shipped:
phase === "streaming" ? "stop" : "send" -> "send"
THE STOP BUTTON NEVER APPEARS. A run cannot be cancelled from the UI at
all; it keeps generating and keeps billing.
keyboardHeightPx -> 0
The composer never lifts above the keyboard. You type behind it.
insets.top -> insets.bottom
The top safe-area is read from the bottom, so content sits under the
notch. Both custom properties were written and neither was checked.
render = diff.next removed
The render state never advances, so every publish re-inserts every row.
A three-delta answer paints as three separate paragraphs of the same
growing text.
transcript.textContent = "" removed
New chat leaves the previous conversation on screen.
input.value.trim() -> input.value
Whitespace-only sends start real turns against the engine.
root.textContent = "" removed (fatal screen)
The crash screen appends UNDER the corpse of the dead UI rather than
replacing it, so the app looks alive and is not.
AND ONE FOUND WHILE WRITING THEM
New chat cleared the transcript and not the live regions. `announcer =
initialAnnouncer` resets the state that decides what to SAY next; it does not
empty the regions themselves. So the previous conversation's answer stayed in
the accessibility tree of what the user believes is an empty chat -- not
announced again, but still there for anyone navigating the page.
I found it because my own test asserted on the whole document text and failed
while the transcript was correctly empty. The test now asserts on the
transcript AND on the regions, which is what it should have done from the
start.
1063 tests on node 22.23 and 24.12. Eight mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The corpus test's `assert.ok(compared > 20)` hard-failed on a small-ICU
build where `Intl.Locale.textInfo` is absent for most tags -- the exact
host the test's own header promised to skip. Replaced the assertion with
`t.skip()` below the threshold, so a full-ICU host still enforces parity
across ~50 tags while a degraded host reports nothing rather than failing
for the wrong reason.
Co-authored-by: Mervin Praison <[email protected]>
Two findings from a targeted re-measure, both about code that only runs where
no test can see it.
THIRTEEN LANGUAGES RENDERED LEFT TO RIGHT
`direction()` prefers `Intl.Locale.textInfo` and falls back to a table. The
fallback is consulted only when textInfo is ABSENT -- which is never, on any
host a test runs on. So the table could disagree with Intl indefinitely and
nothing would notice.
Asking both and comparing across a corpus: thirteen disagreed, every one of
them RTL by Intl and LTR by the table.
aeb acm ajp apc ary arz five spoken Arabic varieties -- which is what a
phone in those regions actually reports, rather
than plain "ar"
bal glk haz lrc mzn skr Baluchi, Gilaki, Hazaragi, Northern Luri,
Mazanderani, Saraiki
rhg Rohingya, whose Hanifi script is RTL
On an older Android or iOS WebView without textInfo, every one of those got
the entire UI laid out the wrong way round. A prior sweep reported eight; a
wider corpus found five more, which is the argument for the test rather than
the fix: it now asserts the table agrees with Intl across ~50 tags, so the
NEXT drift fails rather than the next audit finding it. Tags this Node cannot
answer for are skipped, so a small-ICU build reports nothing rather than
failing for the wrong reason.
THE FAKE DOM HAD TWELVE UNGUARDED BEHAVIOURS
Every composition-root test rests on `testing/src/fake-dom.ts`, and a sweep
broke it in twelve places with the whole suite staying green: `insertBefore`
appending unconditionally, `insertBefore` not detaching (so a move becomes a
duplicate), `remove()` not removing, buttons no longer satisfying `instanceof
HTMLButtonElement`, `preventDefault()` doing nothing, `style.setProperty`
being a no-op.
Each of those makes a `mount()` test pass for the wrong reason. The
`instanceof` one is the sharpest: `main.ts` reads `disabled` only off
something that passes that check, so a fixture answering false makes every
disabled control look enabled -- and the guard that stops a double-send is
never exercised while its test still passes.
This package has now found FOUR fakes lying about the port they stood in for
-- `setTimer`'s delay, `every`'s period, `advance`'s amount, and a fake DOM
that appended instead of ordering. A fixture with twelve unguarded behaviours
is the fifth waiting to happen, so it gets its own tests rather than being
trusted because the tests using it pass.
Thirteen self-tests. Seven representative mutations confirmed to fail.
`FakeDom` also exposes `make(tag)` now, so a test builds an element directly
rather than reaching through `ownerDocument` -- which is typed `unknown`,
because the app only ever hands it back to the fake.
1057 tests on node 22.23 and 24.12. boundaries: 136 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A targeted re-measure asked whether the contracts I added last round actually
defend anything. They do not, and neither did the two that came before them.
I deleted `assert.ok(fired >= 2)` from the time contract -- the one assertion
that catches `setInterval` becoming `setTimeout`, which stops every polling
loop in the app after a single tick -- and the suite reported 1035 pass, 0
fail. Fourteen assertions across all four contracts could be deleted for free.
Removing an entire SECTION of the time contract simply reported one test fewer
and zero failures.
The cause is the meta-tests. `contracts.test.ts` has a set named "the contract
catches an adapter that ...", and each one builds a broken adapter and then
RE-IMPLEMENTS the contract's assertion inline. That proves an assertion of that
shape would catch the defect. It proves nothing about whether the contract
still contains it. They are decorative.
This is not a regression in the new contracts. The old storage and shell
contracts have the identical hole, and the whole pattern in that file is the
same shape as the defect this package keeps finding: a mechanism that exists,
is documented, is tested, and is connected to nothing.
The fix already existed one directory over. `engines/src/contract-fixture.ts`
spawns the REAL contract against an engine broken in one named way and asserts
the matching case goes red BY NAME -- written for exactly this reason, with a
comment saying so. The adapter contracts never got it.
`adapters/src/conformance/contract-fixture.ts` is that, for adapters. Six break
modes, each asserted to redden its own named case, plus a `none` control so a
fixture that failed for an unrelated reason -- a syntax error, a bad import --
could not masquerade as proof. It builds its broken adapters INLINE rather than
importing the fakes, because `adapters` may not import `testing` and the layer
rule is right to say so. The boundary checker caught my first attempt.
The break modes are chosen from real defects: two keychain accounts sharing one
credential; an empty string read back as absent; a missing key reading
`undefined`; namespaces colliding; `every()` firing once; a clear that does not
clear.
And a count floor, because the break modes prove a case still WORKS while a
case with no break mode could simply be deleted. It is counted from a real run
of the fixture rather than from source text -- a regex over `test(` would be
satisfied by a case that asserts nothing.
Verified: gutting a case, deleting a case, and deleting the load-bearing
assertion of a case all now fail. Deleting ONE of two redundant assertions
still passes, correctly -- the case retains its power through the other.
1043 tests on node 22.23 and 24.12. boundaries: 135 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Round five, driving realistic SEQUENCES through the real mount() rather than
one action at a time. Two critical defects, one root cause.
`turn` was a controller-lifetime variable. `runTurn` reset `approvals` -- added
last round for exactly this reason -- and never reset `turn`. The only thing
that reset it was a `start` event.
So a turn producing NO start met a state left `ended` by its predecessor:
`apply()` dropped the error as `wrong_msg_id` or `after_terminal`, and
`finish()` returned the PREVIOUS turn untouched because its outcome was
already set. And a turn produces no start for every pre-first-token failure --
401, 403, 500, offline, a refused connection, a wrong baseUrl.
Measured, turn 1 answers and turn 2 gets a 401:
turn 2 text : "first answer" <- the PREVIOUS answer
turn 2 outcome: {"type":"end", ...} <- the PREVIOUS outcome
turn 2 dropped: 1
The user types a second question, taps Send, the composer clears, and NOTHING
HAPPENS. The old answer stays on screen. No error, no retry, no auth prompt,
not even a hint that a run was attempted. Silent, repeatable, permanent. The
default baseUrl is 127.0.0.1:8765 -- the phone itself -- so "engine
unreachable" is the common case here, not an exotic one.
Two amplifiers: the dropped error landed in `carry`, so the next SUCCESSFUL
turn inherited a "1 event could not be read" row blaming itself for its
predecessor; and `send()` publishes before `runTurn` starts, so a stale turn
was painted into what the user believed was a new conversation.
THE SECOND ONE IS THE SAME BUG WITH A WORSE PAYLOAD
New chat clears the screen. It could not clear the controller's `turn`, so the
wipe held only until the next Send -- at which point the PREVIOUS chat's
pending approval reappeared, with live buttons. `setChat` had already cleared
the approvals table, so the row found no entry, rendered `actionable: true`,
and Allow posted the decision to the engine.
The scenario is a user abandoning a conversation BECAUSE it asked to run
`rm -rf /`, starting a fresh chat, typing something unrelated, and being asked
again underneath it.
ALSO FIXED
A turn the user STOPPED rendered as a red "Connection lost" with a Retry
button, and announced "Connection lost" to a screen reader. The catch mapped
every thrown iterator to `transport` without asking whether the abort was
ours: Stop cancels the engine, aborts the reader, and a real fetch then errors
the body stream. The `cancelled` outcome and its "Stopped" notice were
reachable only if the engine's goodbye frame won a race against our own abort,
which it usually loses. Both directions are pinned -- a stream that dies on its
own is still a transport error, because hiding a real network failure behind a
calm "Stopped" removes the retry the user needs.
Every launch's FIRST conversation went to the engine as the literal
"unassigned". `chatId` defaults to it and `setChat` was only ever called by the
New chat handler. Against an engine keying server-side history by chat_id,
every user's first chat on every device was one shared thread. The existing
test asserted only that two chat ids DIFFER, and "unassigned" !== "chat-2"
passes that.
The suite passed 1029/1029 before any of this changed, which is the measure of
how little of it was observed: no test in the package ran a FAILING turn after
a SUCCEEDING one.
1035 tests on node 22.23 and 24.12. Five mutations confirmed to fail.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The last of the sweep's three structural recommendations. Its own words: "items
2, 4 and 41 all hide behind the same blind spot -- the tests read the event
stream and never read back the file."
The blind spot was one line. `engine.test.ts`'s fake persistence is
async record(req) { recorded.push(req); return indices; }
It records the request and THROWS THE ANSWER AWAY, so nothing in that file
could see what actually reaches the repository.
answer = event.text -> +=
survived. `finish` carries the full text and the engine trusts it over the
streamed accumulation, because upstream may normalise and a non-streaming
provider finishes with content having emitted no deltas at all. Appending
instead writes the answer TWICE to disk -- while the screen, built from the
deltas, looks perfectly correct. The user sees a normal answer, and finds it
doubled when they reopen the chat.
Four tests now assert the persisted side, covering both directions: a
normalising finish must win over what was streamed, and a stream with no finish
must still persist what it streamed. Deleting the finish handling entirely
fails four of them.
Same blind spot, second instance:
updated: at -> updated: base.updated
survived, because no test read a chat back across TWO turns. The chat keeps the
timestamp of its first message forever, so a list ordered by `updated` sinks
the conversation you are actively using to the bottom while one you have not
touched in a month sits at the top.
1029 tests. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The worst-scoring module in a 226-mutation sweep, and the last of that sweep's
three structural recommendations. `adapters/src/web/time.ts` had no test file
and no contract; every mutation aimed at it survived. What each one shipped:
setInterval -> setTimeout EVERY polling loop in the app fires once and
stops. Readiness polling, the coalescer's flush
tick, elapsed-time updates -- all of it, and the
app reads as merely slow rather than broken.
clearTimer does nothing A cancelled timer still fires, so the publish
gate reopens after it was deliberately shut.
performance.now -> Date.now Elapsed measurements jump backwards when the
clock is NTP-corrected mid-turn, which is
exactly why the port separates the two.
unsubscribe does nothing The interval leaks for the app's lifetime.
Written as a contract because the fake and the real adapter must agree, and
because this port has form: `every`'s period and `setTimer`'s delay were BOTH
found discarded in the fake, in two separate rounds. A test that only ever
drives the fake cannot see it a third time.
Two things the contract had to be honest about rather than paper over:
The fake deliberately uses ONE clock for nowMs and epochMs -- its own header
says so, "safe in a fake and wrong in an adapter". Asserting they differ would
be asserting the fake is broken, so that case is guarded and runs only against
a real-time adapter, where it is the case that matters.
The web adapter targets a browser, and `requestFrame` throws ReferenceError in
Node. The contract supplies requestAnimationFrame rather than skipping the
case. Not having that shim is part of why this adapter had never been
contract-tested at all.
All four mutations now fail by ASSERTION, not merely by timing out -- checked,
because the unsubscribe one also hangs the run (the leaked interval keeps the
event loop alive, which is the resource leak it ships), and a hang is not the
same as a test that says what is wrong.
1024 tests on node 22.23 and 24.12. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The sweep's structural finding, and the reason its survivor count did not fall:
almost every genuine survivor lives on a FALLBACK path -- the branch that runs
when the platform is missing something. The suite runs on a Node with full ICU
and no DOM, so the primary path is exercised hard and the degraded path is
never entered at all.
That is one gap, not many independent ones.
`direction()` is `directionFromIntl(tag) ?? directionFromTables(tag)`. On the
test host the first half always answers, so the second half -- the branch that
runs on an older Android WebView without `Intl.Locale.textInfo` -- had FIVE
indistinguishable mutations in it. What each shipped:
RTL_LANGUAGES never matches Arabic, Hebrew, Farsi and Urdu render LTR
script not title-cased az-Arab and ku-Arab render LTR
length test 4 -> 3 ar-Latn and ks-Deva render RTL
The whole UI mirrored the wrong way, or not at all, on exactly the devices most
likely to reach this code.
`sentences()` uses `Intl.Segmenter` whenever it exists, and it always does
here, so `fallbackSentences` never ran. Three mutations survived in it -- one
per failure the file's own header names: splitting on a decimal point,
splitting mid-CJK, and failing to split after a quoted full stop. A fourth
dropped the unterminated tail, which on a host without Segmenter means a blind
user never hears the in-progress end of an answer, and breaks the documented
seam guarantee that slice(0,n) + slice(n) recombine.
Both are now exported and called directly, which is this repo's stated rule --
extract the expression so the test calls it rather than asserting it appears in
the source. Stubbing globals would have worked too; exporting is honest about
what is being tested and does not leave a fixture that can itself drift.
1009 tests on node 22.23 and 24.12. Four representative mutations confirmed to
fail. Product source changed only by two `export` keywords and their comments.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A 226-mutation sweep against an isolated worktree. 59 genuine survivors, 10
equivalent mutants each proved by differential test rather than asserted. These
are the seven with the sharpest user consequence; the sweep's own structural
recommendation is the next commit.
TWO ACCOUNTS IN ONE SLOT SHARED ONE CREDENTIAL
`adapters/src/web/secrets.ts` had no test file. Collapsing its key from
`${slot}:${account}` to `${slot}` survived everything: adding a second
profile's API key silently overwrites the first, reading either returns
whichever was written last, `has()` answers true for an account never set, and
deleting one deletes both.
Written as a CONTRACT rather than a test of one adapter, matching shell and
storage. The fake and the web adapter must agree on key semantics or a test
that passes against the fake ships a device-only defect. That is not a guess:
in the same sweep the two ports WITH a contract scored best and the two
without scored worst. The mutation now fails three times, once per adapter.
A DISABLED CONTROL STILL FIRED AN ACTION
`if (el.disabled === true) return null` -> `continue`. The walk skips the
disabled element and keeps climbing, so a tap on a greyed-out approval button
reaches the enclosing row and fires THAT row's intent. It is the only thing
between a double tap and a second send, and between a tap on a decision already
in flight and a second decision.
ONE CORRUPT CHAT FILE HID EVERY CHAT AFTER IT
`if (!result.ok) continue` -> `break`. Half the library vanishes with no error
anywhere, and `listUnreadable` still reports only the one bad file -- so the
app is confidently wrong about how much is missing.
A CHAT WITH NO id LOADED SUCCESSFULLY
Dropping `typeof chat.id !== "string"` gives `ok: true` with `id: undefined`: a
row addressed at nothing, absent from `listUnreadable`, and saved back to the
storage key "chats/undefined". The header's rule -- "a chat that fails to parse
is reported, not skipped" -- broken in the one direction nothing checked.
A REFUSED DECISION COULD BE MARKED SENT
`acknowledge` accepting a `failed` entry flips `isActionable` to false and
drops `outstanding` to zero: the buttons die and the run stays blocked on a
prompt the engine never received, until its 300-second timeout. Verbatim the
desktop failure approvals.ts says it exists to prevent.
AN UNMOUNTED SCREEN WAS NEVER FORGOTTEN
Dropping `nodes.delete(id)` makes `live()` lie. screens.ts asks `live.has(next)`
to decide whether to mount, gets a stale yes, and never rebuilds -- a blank
screen with no way back. Its sibling, dropping the `nodes.has` guard, appends a
second copy and orphans the first.
Also covered: a chat id that vanishes between `listIds` and `read` is absent,
not corrupt -- reporting it unreadable shows a permanent corruption warning for
a file that was simply deleted.
999 tests on node 22.23 and 24.12. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Mutating the refused-stop fix back was `fail=0`: I had verified it with a
throwaway script and never added a test. Worth recording, because the fix
itself was a regression of my own from two commits earlier -- so the defect had
now escaped twice, once into the code and once past my own verification.
The first pair I wrote was also wrong. `SCRIPTS.happy` finishes on its own, so
whether a run was still live when Stop was pressed depended on microtask
ordering: the refusal test passed because `stop()` returned false for the
BORING reason -- nothing was running -- rather than because the engine refused.
Its sibling then failed and exposed it.
Both now use an engine whose turn stays open until the test releases it, so a
run is genuinely live when Stop is pressed. Three mutations die:
refused stop aborts anyway fail=1
refused stop keeps `cancelling` fail=1
bootOrFail rethrows fail=1
971 tests on node 22.23 and 24.12.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A round-four audit drove the real `mount()` through a fake DOM for the first
time. Every defect it found is the same shape: a pure function that is correct,
and a composition root that calls it wrongly or not at all. main.test.ts was
30 lines and said so out loud -- "`mount` needs a whole fake SSE transport to
drive end to end, so the parts of it that can be wrong are extracted and called
directly instead". Every one of these lived in the part that was not extracted.
1. EVERY PRE-FIRST-TOKEN FAILURE SAID "the engine produced no output"
`apply()` drops any event arriving before `start`, and both the controller's
synthesised transport error and the engine's synthesised auth error ARE the
first event. So the real reason was thrown away, `finish()` substituted
kind: "empty", and 401, 403, 500, 502, offline, DNS and TLS all rendered
identically -- plus a dropped row accusing the engine of sending an event
before the turn began.
The default baseUrl is 127.0.0.1:8765, which on a phone is the phone itself, so
this was the FIRST thing every new user saw. It also made the auth ->
"go to settings" recovery unreachable: the one distinction between "retry this"
and "fix your credentials" never survived to the view model.
An error is a terminal event; if nothing has started, it IS the turn's outcome.
2. A SCREEN-READER USER NEVER HEARD THE ANSWER
`for (const item of announcements) region.textContent = item.text` overwrites
the earlier items in the same task. A short answer completing inside one
interval produced [polite "<the answer>", polite "Response complete"], so the
user heard only "Response complete." That is verbatim what announce.ts rule 4
exists to prevent -- "the user would never hear the end of the response" --
reintroduced at the single point where the pure function meets the DOM.
3. NEW CHAT DID NOT STOP THE RUN
It cleared the screen and left the turn streaming, so the next token
reconciled against an empty render and re-inserted the previous conversation's
rows into what the user believed was a fresh chat, where it finished.
4. EVERY REQUEST FROM EVERY CHAT CARRIED chat_id "unassigned"
`setChat` was never called anywhere in the app. controller.ts says in as many
words that "an engine cannot tell two conversations apart if every turn claims
the same id"; against an engine keying server-side history by chat_id, every
conversation the user ever had was one thread.
5. A STORAGE FAILURE AT BOOT LEFT A RENDERED, DEAD APP
`createApp` returns a typed BootResult for anticipated failures and THROWS for
a StoragePort failure -- reachable via SecurityError with site data blocked or
QuotaExceededError. The chrome is appended before that await, so the rejection
skipped both the fatal screen and the listener registrations: top bar,
composer, green Send button, and nothing ever happened again. No spinner, no
error, no crash screen.
6. THE CRASH HANDLER WAS INSTALLED AFTER THE THING THAT THROWS
`detectPlatform()` ran first, directly contradicting the comment above the
crash handler. It reads window.localStorage, which raises SecurityError with
site data blocked, and a throw there left the root completely empty.
AND ONE REGRESSION OF MY OWN
`stop()`'s idempotence guard, added two commits ago, aborted the reader even
when the engine ANSWERED FALSE. So a refused stop set `aborted`, and the next
tap returned true without asking the engine again. The user is told the stop
was refused, taps the only affordance offered, and is told nothing at all --
which stopNotice defines as success -- while the run keeps generating. The
defect that notice exists for, inverted, one layer down. A refusal now leaves
the run retryable and the reader attached.
THE HARNESS
testing/src/fake-dom.ts models the DOM behaviours the app actually depends on:
ordering under insertBefore (which DETACHES first, the detail that hid two
ordering defects here), textContent replacing children, delegated event
dispatch, and the three constructors main.ts walks with `instanceof` -- without
which the click listener throws `Element is not defined` and every intent is
lost. That is why nobody had driven the composition root before: the first
attempt fails before reaching an assertion.
Eight tests now drive mount() end to end. Each fix was confirmed by mutating it
back and watching a named test fail.
969 tests. boundaries: 132 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`publish-gate.ts` is a verbatim port of the desktop's stream-pacing with two
constants tightened deliberately for mobile. It IS called -- controller.ts has
`if (frames.length > 0 && gate(streamed)) publish()` -- but in the shipped
pipeline that condition is almost never true, so neither constant affects
anything.
The cause is the coalescer's flush tick, added three PRs ago to fix answers
arriving in one lump. The tick drains the coalescer every maxDelayMs, so by the
time a delta arrives `push()` usually returns [] and the gate is never
consulted. The tick's own publish is deliberately ungated, which is right --
that frame exists precisely because nothing has painted recently.
Measured by driving the real controller with a virtual clock and counting
whether the gate was consulted even once. The count is exact rather than
inferred: the gate calls requestFrame on its FIRST invocation, so zero frame
requests proves zero calls.
20 tok/s 2004 publishes for 2000 tokens gate consulted: no
60 tok/s 2004 no
150 tok/s 670 no
400 tok/s 289 no
1600 tok/s 80 no
3200 tok/s 42 YES
8000 tok/s 42 YES
The threshold is where 256 bytes land inside one 16ms window. Real model
streaming is 20-150 tok/s, so it is unreachable in practice.
Not currently a defect: paints stay bounded by time, which is the property that
matters, and per-publish view work is under 9% of a frame in the worst measured
case. Recorded because two things follow. There is no backpressure path at all
-- the gate is the mechanism that notices the renderer cannot keep up, and it
never runs. And a module with its own tuned constants that never executes reads
as load-bearing to whoever next touches pacing.
The fix is to wire the tick's publish through the gate or delete the module.
Deciding that is a design call, not a bug fix, so it is written down rather
than done quietly. What is not acceptable is leaving it LOOKING wired, which is
the defect class this package keeps producing.
Docs only.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The documented tail of round three's 49 survivors. I confirmed all seven were
still live by mutating them and, through a mistake in my restore paths, ended
up applying every one simultaneously -- 957 tests still passed with all seven
defects in the tree at the same time. That is a better demonstration than the
one I intended, so it is worth recording rather than tidying away.
count: turn.dropped.length -> count: 1
The dropped row always said "1". `reasons: []` was caught; the count was
not. The reason the row carries a number at all is to distinguish one
malformed frame from a stream that is mostly unparseable.
copy: turn.text !== "" -> copy: true
A Copy button on an empty answer copies an empty string and reports
success -- an action that says it did something and did not.
signatureOf drops ${row.recovery}
The comment two lines above it says exactly why it is there: "the same
message can offer a different action once the engine reports a different
kind". Nothing checked. The row would keep a Retry button after the
engine reclassified the failure as one retrying cannot fix.
signatureOf drops ${row.reasons.join(",")}
Two refusals for different reasons look identical to a count, so the row
keeps naming the wrong failure.
pending.length >= maxBytes -> >
Off by one on the constant the coalescer's whole contract is written
around. No test ever landed exactly on the cap.
formatElapsed `seconds < 10` -> `<= 10`, `minutes < 60` -> `<= 60`
The entire point of a threshold is which side of it a value falls on,
and no test placed a value exactly on one. 60 seconds rendered "60s"
instead of "1m 00s"; 3600 rendered "60m 00s" instead of "1h 00m".
Each is now pinned in both directions -- an unchanged row still produces no
ops, so a signature that changed every time cannot pass by repainting always.
964 tests on node 22.23 and 24.12. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The reconciler fix landed, but the harness the PR body describes as proving
it -- a fake DOM with real insertBefore reference-node semantics, and a
reconcile->applyOps ordering sweep -- was never committed. dom.test.ts's fake
still appended unconditionally (a fake that cannot represent order cannot fail
an ordering test), and reconcile.test.ts asserts op shapes without applying
them.
Give the fake real insertBefore/remove semantics and add the exhaustive
prev/target sweep in app/ (the only layer allowed to import both halves),
applying the real ops through the real applyOps and asserting DOM order equals
the target. Verified both ways: reverting the reconciler to the surviving-index
model, or the fake back to append-only, makes the sweep fail. Also asserts the
minimal-op guarantees at the DOM (append is one insert; a settled list is zero
ops).
957 tests pass; boundaries clean; no new typecheck errors.
Co-authored-by: Mervin Praison <[email protected]>
`reconcile` tracked `surviving` -- the previous ids minus removals -- and
spliced each move into it at the TARGET index. Those are two different
coordinate systems: the target index counts rows inserted on this pass, and
`surviving` contains none of them. Inserted rows were never added to it either,
so the model drifted from the DOM as soon as one insert preceded a reorder, and
the emitted ops then described a list nobody had.
Measured by applying the REAL ops through the REAL applyOps to a DOM:
exhaustive, 5 ids, every prev/target pair 120 of 960 rendered wrong
fuzz, 8 ids, 200k pairs 1546 wrong
smallest case [a,b,c] -> [d,e,b,c,a] rendered [d,e,b,a,c]
It now models the list it is actually producing: `current` starts as the
survivors, every insert and move is applied to it, and a row already in place
emits nothing. Both sweeps are 0 wrong. The minimal-op guarantees are intact --
appending to 200 rows is still one insert, an unchanged list still zero ops --
and those are now asserted here too, so a correct-but-rebuilding reconciler
cannot pass by brute force.
WHY NOTHING CAUGHT IT
Two reasons, and both are worse than the bug.
`reconcile.test.ts` asserts the SHAPE of the ops and never applies them. It can
say "a move was emitted for c at index 0"; it cannot say whether running the
ops produces the intended order. The pair is tested here in app/, the only
layer allowed to import both halves.
And the fake DOM's `insertBefore` ignored its reference node and appended
unconditionally. A fake that cannot represent ORDER cannot fail an ordering
test. It now has real semantics, including detaching the node first -- and
mutating it back to appending makes the new sweep fail, so the fake is itself
under test.
That is the fourth fake in this package found lying about the port it stands
in for, after `setTimer`'s delay, `every`'s period and `advance`'s amount.
WHAT I DID NOT KEEP
I first "fixed" this in applyOps by detaching the node before placing it. With
the reconciler corrected, that mutation is `fail=0` -- no test can tell the
difference, because reading `children[op.index]` before `insertBefore` detaches
already names a real sibling. So it went back out. Unverified code that claims
to fix something is the thing this session keeps finding; adding more of it
while removing a bug is not a trade worth making. dom.ts keeps only a comment
saying why the existing line is correct.
Reachability today is low: transcript rows are append-only, so the wrong-order
path needs an insert before a reorder, which the current view model does not
produce. This is a latent defect in a core renderer, fixed before the screens
that would reach it are built.
952 tests on node 22.23 and 24.12.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A mutation sweep over the whole package found this file the weakest in it:
16 tests, and 15 of 16 mutations survived. The existing tests cover the SHAPE
of every event thoroughly -- and never once pass `versions` other than 1,
`active` other than 0, an absent `assistant_index`, or an empty-string id.
Those are the fields that decide which message the UI selects and which turn
Fork and Delete act on.
Nine mutations now fail. Each was applied and measured first:
`: userIndex + 1` -> `: userIndex` the assistant index points at the
USER'S OWN message, so anything
selecting by it renders the prompt
back as the answer
versions clamp removed `versions: 0` reaches the UI -- a
message existing in zero versions
active floor / ceiling / off-by-one events.ts says an out-of-range index
"selects nothing and the message
renders blank"; this clamp is the
only thing between a buggy engine and
an empty answer bubble
`active ?? 0` -> `?? 1` silently selects the SECOND version
of every message the engine gave no
cursor for. Invisible with one
version, which is why it survived
finite check removed NaN / Infinity accepted for versions,
active, chars and seconds
`!(key in o)` -> `=== undefined` collapses absent vs present-undefined,
the exact distinction this file's own
comment says it exists to prevent
array accepted as tool args every consumer gets an object keyed
"0","1",... -- and args are shown to
the user as the command being approved
`|| raw === "always"` "always allow" decodes to null, so the
button exists, is pressed, and nothing
happens
Two of my first attempts asserted my guess rather than the contract and failed:
present-but-undefined resolves to `null` (not on disk), not to a derived index;
and `approval_response` is not an event -- the choice decoder is a standalone
export. Both tests now assert what the code actually guarantees, which is the
better behaviour in each case.
`versions ?? 1` -> `?? 0` is genuinely EQUIVALENT: the Math.max(1, ...) below
it maps both to the same output. Recorded in the test file so nobody hunts it
as a live survivor.
952 tests. Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two defects the final review found in PR #4561's own changes:
- sse.ts recomputed the swallow-CR flag from every chunk, including an
EMPTY one. `"".endsWith("\r")` read false, cleared the flag, and a `\n`
in a later chunk survived as a false frame boundary -- splitting one
CRLF frame into two malformed ones. A chunk with no decoded characters
now leaves the pending-CR state untouched (and clears it only when the
swallow consumed the LF). Regression test added.
- controller.stop()'s `aborted` guard is sequential: two OVERLAPPING
stops (dispose + the Stop button) both pass it before either
engine.cancel resolves, each issues its own cancel, and an honest
engine refuses the duplicate -- reporting a real stop as rejected. The
in-flight cancel is now memoised per run so overlapping stops share one
result. Regression test forces the overlap.
Both new tests were mutation-verified (each fails against the un-fixed
line). npm run check: typecheck clean, boundaries 131 files no
violations, 939 tests pass on node 22.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
Round three of multi-agent validation, run against an isolated worktree so
nothing could interfere. It found what the previous two did not, including two
defects that lose or corrupt user-visible work.
1. AN SSE STREAM WITH CRLF LINE ENDINGS SILENTLY LOST CONTENT
`sse.ts` normalised line endings PER CHUNK. A chunk ending in `\r` became
`\n`, and the next chunk's leading `\n` then completed a `\n\n` that was never
a frame boundary -- one frame became two malformed ones.
Measured end to end through the real engine over a CRLF stream:
chunk size 1024 -> "The answer is 42." correct
chunk size 7 -> "The answer " half the answer gone
chunk size 1-3 -> "" + error "the engine produced no output"
Seven bytes is the repo's own test chunk default. CRLF is not exotic: the spec
permits it and any proxy may rewrite to it. A transport bug was rendering as
the model having said nothing.
The first fix held a trailing `\r` back until the next chunk could say whether
it was half of a `\r\n`. That regressed the OTHER legal ending -- a lone-CR
stream ends on `\r` with no chunk after it, so its last frame never completed,
taking CR-only streams from working to silent. Caught by testing all three
terminators rather than only the broken one. The `\r` is now emitted
immediately and a matching `\n` swallowed if one follows.
Verified exhaustively: every one-cut AND two-cut split of a two-frame stream,
for CRLF, LF and CR -- 0 wrong of 5253, 4656 and 4656 -- plus byte-by-byte
delivery for each. The file had a CRLF test and chunk-split tests; it never
had both at once, and the defect lived at exactly one offset out of 103.
2. STOP COULD CRASH THE APP OR CANCEL THE WRONG RUN
`stop()` captured `runId` but `live.abort.abort()` re-read the closure variable
AFTER awaiting the engine -- and cancel is a real network call, so the turn can
settle inside that window:
queue empty `live` is null, TypeError. Both callers reach stop() through
a floating `void`, so the crash handler replaced the whole
app with the crash screen. Tapping Stop at the wrong instant
blanked the page.
queue non-empty `live` is the QUEUED FOLLOW-UP. The engine was asked to
cancel the finished run while the abort killed the new one,
which rendered "the engine produced no output" with Retry --
and stop() returned true, so the UI announced success.
Cancelling "whatever is live" is what runId exists to stop.
The run is captured, not just its id. stop() is also idempotent now: a second
tap re-issued cancel for an already-cancelled run, every honest engine answers
false for that, and stopNotice then announced "the engine did not accept the
stop" for a stop that DID happen -- inverting the defect that notice was added
for two commits ago. The background -> dispose path calls stop() twice by
design.
My first test for this did not reproduce the race at all: the turn had already
settled before stop() ran, so `live?.abort.abort()` survived it. The test now
forces the ordering -- cancel() releases the first turn and waits until the
follow-up has genuinely started -- and the mutation dies.
ALSO CLOSED, each mutation-verified:
- `drainDrops` accumulated into `next`; restarting from `state` each iteration
kept only the LAST refusal of a batch. Every existing test drained exactly
one at a time. The channel exists to make "40% of this stream is
unparseable" visible; keeping one of every N divides that by N.
- `decide()`'s catch could `acknowledge` instead of `reject` and survive: an
approval whose engine call THREW rendered as sent and accepted. The
`ok === false` path was tested; the throw path was not.
- `set()` did not mark a key chosen -- only `load()` did -- so a setting the
user changes IN THE APP was ignored by `chosenStringOr` and the caller's
default won. That is precisely the defect `isSet` was added to prevent,
reachable through the commoner of its two paths.
- An empty-string `engineId`, trivially persisted by a settings field, outrank-
ed the composition root's default and boot died with `unknown_engine ''`.
- HTTP 403 was classified `transport`, not `auth`, so the UI offered Retry
forever instead of sending the user to credentials.
- `createCoalescer(maxBytes, maxDelayMs)` takes two numbers, so transposing
them typechecked and survived: frames would flush at 16 BYTES with a 256ms
delay bound. Same-typed adjacent arguments, the shape that has already
produced one defect here.
- `FakeTime.advance(ms)` could ignore `ms` entirely and leave all 915 tests
green, making every pacing constant reached through it untested. Its tick
period was not enforced either, contradicting the fake's own comment. This
is the THIRD time a fake in this package has been found discarding an
argument its port declares, so the fake now has its own tests rather than
being trusted because the tests using it pass.
937 tests on node 22.23 and 24.12. boundaries: 131 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The triage bot fixed the same leak independently and pushed to this branch
while I was fixing it. Both stop the leak; they disagree about one case, and
the disagreement is real.
bot: dropped: state.phase === "ended" ? [] : state.dropped
mine: dropped: state.carry (drops recorded while nothing was streaming)
The bot's discards the previous turn's list wholesale once it has ended, so a
frame the DECODER refused in the gap between two turns goes with it. Such a
refusal often has no msgId -- that is frequently why it was refused -- so it
cannot be attributed backwards to the turn that just finished. It belongs to
the turn about to open, and losing it quietly is the exact failure this whole
channel exists to undo.
Its test found the case mine had wrong, though: a LATE FRAME FROM THE OLD RUN
(`after_terminal`) is evidence against the old run, and carrying it forward
blames the next answer for its predecessor's mess. `carry` now takes
everything recorded while not streaming EXCEPT `after_terminal`, which is the
one out-of-band drop that is genuinely about the turn just finished.
My own between-turns test did not distinguish the two implementations: it
started from `initialTurn`, so the phase was `idle` rather than `ended`, and
the bot's version passed it. Mutating this file to the phase-only form was
`fail=0` until a test was added that ends a turn FIRST and then refuses a
frame. Another instance of a test observing an adjacent property rather than
the one that matters -- caught only by running the alternative implementation
against the suite.
Both tests are kept, and both implementations now fail:
carry ignores the after_terminal split fail=1
the phase-only fix instead fail=1
carry everything, even mid-turn fail=4
915 tests on node 22.23 and 24.12.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A second validation pass over the refusal channel this PR adds. The channel
works -- three previously-invisible refusals now reach the transcript -- but
making them reachable exposed two defects that were harmless while nothing
could produce one.
1. EVERY DROP LEAKED ONTO EVERY SUBSEQUENT TURN, PERMANENTLY
`apply`'s `start` case carried `state.dropped`, which at that moment is the
PREVIOUS TURN'S ENTIRE LIST. Its comment justifies carrying pre-start drops --
correct intent, wrong list. Nothing resets the controller's `turn`, so one
refusal on turn 1 made every later turn report itself damaged for the lifetime
of the app. Measured: turn 1 has one genuine refusal; turn 2 is a perfectly
clean stream and still renders it. `new-chat` does not help -- it resets the
renderer, not the controller.
That also made the count meaningless for the thing it exists to express, "this
stream is 40% unparseable", and removed the per-turn bound on an already
quadratic append (`[...state.dropped, x]`): a bad connection refusing 500
frames/s for a minute is ~2s of main-thread work on a phone, and every later
turn then pays for the inherited list on every publish.
`TurnState.carry` now holds only drops recorded while nothing was streaming,
and `start` takes exactly those. Four tests, both directions: a clean turn
after a damaged one shows nothing; a drop BETWEEN turns still reaches the next
one; it is carried once rather than forever; and a drop during a live turn
never enters the carry.
2. FIVE DISTINCT WIRE FAILURES ALL REPORTED THE SAME WRONG REASON
`safeParse` returned `{}` for anything it could not read, and the caller
spreads it and adds `type` -- so an HTML error page from a proxy, a body
truncated by a cut connection, a JSON array, a bare string and `null` all
arrived at the decoder as an object with only a type, and all came back as
`missing_msg_id`. The payload was discarded too, so `detail` could not recover
it. `decodeEvent` gets every one of these right when handed the raw string.
Survivable while rejections were discarded. They are user-visible prose now:
a 502 page from a proxy told the user "an event arrived with no message it
belongs to", pointing whoever they reported it to at an engine msg_id bug that
does not exist. Two shipped strings -- "not valid JSON" and "a value where an
event was expected" -- were unreachable for the same reason.
`parseFrame` returns a discriminated result, so the reason survives and the
detail carries enough of the frame to identify it. One test covers all five.
ALSO, from the same pass:
- `engineId.choices` offered `praisonai-ts`, which `enginesFor` only builds
when `createInProcess` is supplied and main.ts never supplies. Selecting it
persisted an id `selectEngine` then rejects, so the NEXT launch died at
renderFatal with no way back except editing storage by hand. The list is now
what the shipping composition can build, and its test compares against
`enginesFor` rather than against the same two constants the def names --
which is why it could not fail before.
- `adapters/src/web/secrets.ts` claimed "the composition root refuses to hand a
hardware-backed secret to an engine when the transport is not native, and
this flag is half of that check". There is no such check: the flag is read
only by the settings view's warning row, and boot.ts has never consulted it.
Nor is one needed yet -- no secret reaches an engine at all, because
registry.ts never passes the `token` the engine accepts. The comment now says
that, and says where the refusal belongs when the key is wired. Describing a
guard that does not exist is worse than having none: it stops the next reader
looking for one.
- Four settings are declared and consumed by nothing (`model`, `temperature`,
`showReasoning`, `showDiagnostics`). Expected -- their screens do not exist --
and now recorded in gaps.md so nobody concludes they work. `showDiagnostics`
is called out in place because unlike the others it contradicts behaviour
that already exists: the dropped row is rendered unconditionally.
912 tests on node 22.23 and 24.12. Every fix mutation-verified in both
directions.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
apply(start) carried `dropped` across every new start, including one that
followed a FINISHED turn -- so a refusal on turn 1 painted turn 2's clean
answer as damaged. A turn that already ended owns its drops; only a
before_start drop (recorded while idle) belongs to the turn now opening.
Clear `dropped` on a start that replaces an ended turn. Pinned at both hops:
reverting fails one reducer test and one controller test.
Co-authored-by: Mervin Praison <[email protected]>
gaps.md recorded it as knowingly open because closing it needed a channel from
the engine to the transcript that did not exist. It exists now, and the entry
says so rather than leaving a reader to discover the file is stale.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The last defect from the validation pass, and the one left open when #4558
merged because closing it needed a channel that did not exist.
remote-http is the ONLY production caller of decodeEvent, and it did:
const outcome = decodeEvent(...);
if (isDecoded(outcome)) yield outcome.event;
Every rejection went on the floor. So a `tool_result` frame missing its `ok`
-- and `ok` is the only signal of tool success, never inferred from output --
made its tool disappear entirely and the turn rendered as a clean answer with
a shorter reply. Meanwhile `Dropped.reason` already admitted every
IgnoredReason, the view model already rendered a dropped row, there was
already a `showDiagnostics` setting labelled "Show dropped events", and
ui/src/i18n/strings.ts already had a sentence for all seven reasons. None of
it was reachable, because nothing upstream ever produced one.
THE CHANNEL. The engine is built at composition and the controller per app, so
neither can call the other. `core/src/run/drop-sink.ts` sits between them:
the engine notes a refusal, the controller drains onto the transcript. It is
drained per event and again in `finally`, so a refusal arriving beside the
last frame -- or while the stream was dying -- is still shown. Deliberately
not an EventTarget: draining on the controller's own schedule means a refusal
paints with the frame it arrived beside rather than forcing its own publish.
`noteDropped` is the public form of the reducer's private `drop`, because
`apply` can only record events it was handed and a frame that failed to decode
never became one.
Found while writing this: my first drain sat at the end of the `catch` block,
so it only ran when the turn had already failed. The paired test for a refusal
on the last frame caught it. It is in `finally` now, before the turn settles.
WIRED, and proved wired. Eight mutations, one per hop, each now failing:
engine discards refusals again fail=2
engine reports a refusal for every frame fail=2
controller drains but discards fail=3
controller skips the final drain fail=1
sink drains without clearing fail=1
boot does not give the sink to the ctrl fail=1
boot hands engines a dead callback fail=1
registry declines to forward onIgnored fail=1
The last two mattered most. Removing `dropSink` from createRunController, and
dropping the registry's forward, BOTH left the suite green on the first pass:
every hop worked in isolation and the ends were not joined. That is the exact
mechanism-connected-to-nothing this commit exists to undo, reproduced by me
while undoing it. Both are now pinned by composition tests -- one through
createApp, one through the real enginesFor rather than a test double.
Every case is paired, so wiring that invented a dropped row would fail too.
907 tests, 0 failed. boundaries: 130 files, no violations.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
`check (node 24)` failed while `check (node 22.18)` passed on the identical
commit. The prover greps the spawned fixture's output for `not ok`, which
assumes TAP. Node's default reporter differs by version: 22 emits TAP when
stdout is a pipe, 24 emits the spec reporter, so the same failing case prints
not ok 3 - FIXTURE no_start: a run emits start first and ... (node 22)
✖ FIXTURE no_start: a run emits start first and ... (node 24)
The child's exit status was correct on both; only the line the test read for
the case NAME was missing, so the prover reported that the contract failed for
the wrong reason.
The reporter is now forced with `--test-reporter=tap`, so the format is a
decision this test makes rather than a default it inherits.
Reproduced and verified against a real node 24.12.0 rather than assumed: the
default reporter there prints `✖`, forcing tap restores `not ok`, and the full
suite is 895/895 on both 22.23 and 24.12.
This is the same mistake as the ones this branch has been fixing, made by me
while fixing them: a test that observes an incidental property rather than the
thing it means to check. CI caught it because it runs two node versions, which
is the only reason it was visible at all.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file exists to record claims that outran the code. It had two of its own.
1. Gap 3 ended "Removing the join removes the opportunity." The join was never
removed -- only the TYPE duplication was. view-model.ts still calls
findApproval(table, pending.approvalId), and replacing that with
`table.entries[0]` passed the entire suite, because every test had exactly
one approval in flight. With two outstanding, the second row rendered the
first's decision: an `rm -rf /` prompt shown as already-allowed against a
decision nobody made. The join is correct and is now pinned in both
directions; the opportunity still exists, and taking it now fails.
2. The closing line said what remained was "the Tauri Rust crate". The crate
exists -- src-tauri/src/{lib,commands,main}.rs, the shell module and two
test files -- with a CI job compiling it on ubuntu-22.04 and macos-15. The
same line silently dropped route->view dispatch, which the body of the file
lists as open and which is still open: main.ts imports neither screens.ts
nor mount.ts and never subscribes to the router, so tapping "Settings"
pushes onto a stack nothing reads.
"Every gap in this file is now closed" is narrowed to "every gap ORIGINALLY
RECORDED in this file", because a later validation pass found fourteen
mutation survivors and three product defects that were never gaps in the sense
this file records. A list of closed gaps is not evidence that nothing is wrong,
and the old phrasing invited exactly that reading.
The remaining-work list is now specific and each item was checked against the
tree before being written -- including that the in-process engine's stated
blocker (bare `crypto` and `events` on the Agent import graph) is already fixed
upstream, so the rationale for omitting that engine no longer holds.
One defect is recorded as knowingly open: remote-http discards decode
rejections, so a malformed frame makes a tool vanish and the turn renders as a
clean answer.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The tool's own header says "a check that silently stops covering anything is
worse than no check". It then did exactly that.
`drift` keeps only output lines mentioning `parity.ts`. When the compiler
could not start at all, the caught error's message is `spawn npx ENOENT` --
which mentions no such thing. Zero drift lines, so:
upstream-parity: the real Agent still satisfies PraisonAgent
exit 0
over nothing whatsoever. Reproduced by running it with npx off PATH.
A tsc that genuinely ran and failed prints diagnostics; a spawn failure, a
missing binary or a killed process prints none. The run is now required to
have either exited 0 or produced `error TS`-shaped output, and anything else
exits 1 saying the check did not run rather than reporting a pass.
Two tests, driven as subprocesses because this is a property of how the tool
is invoked rather than of anything it exports: one empties PATH and requires a
failure whose reason says so and which does NOT also print the success line;
its pair runs with the real PATH and requires the success, so a checker that
failed unconditionally cannot satisfy the first while breaking every build.
Same class as the ungoverned-roots hole fixed earlier on this branch, and the
third instance this session of a checker reporting green over work it did not
do.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
conformance.ts says the suite can fail. conformance.test.ts said each broken
engine "is asserted to fail a NAMED case". Two validators found independently
that neither was true, and I confirmed it: gutting a contract case so it
asserts nothing left all 890 tests green.
`describeEngineContract` was only ever called with CONFORMING engines -- the
scripted fake, praisonai-ts, remote-http. The five can-fail tests build a
broken engine and then hand-run assertions about the REDUCER instead. That is
a real check of the reducer and no check at all of the contract, so the
contract could silently stop checking anything while reporting a clean pass.
This is the exact failure mode tools/check-boundaries.mjs guards against one
directory over, in the file that claims to be the seam's guarantee.
Worse, one of those fixtures cannot fail the suite even in principle: the case
it purports to break is manufactured by `finish()` in the reducer, so it
passes regardless of the engine.
contract-fixture.ts registers the REAL contract against an engine broken in
one named way, selected by argv, and conformance.test.ts spawns it once per
mode -- asserting the process fails AND that the failing line names the case
that mode breaks, not merely that something failed.
no_start -> a run emits start first and exactly one terminal...
tool_failure_as_ok -> a failed tool is reported failed and never inferred...
empty_is_fine -> an empty stream is an error, not an empty answer
decide_always_true -> deciding an unknown approval returns false rather...
A `none` mode is spawned too and must PASS. Without it a runner that reported
failure for everything -- a bad path, a syntax error, a process that never
started -- would satisfy all four assertions while proving nothing.
Verified by gutting each of the four cases in turn: every one is now caught,
where every one previously passed.
NODE_TEST_CONTEXT is stripped from the child. Left in place, the spawned
fixture believes it is a worker of the parent run and reports through it
rather than printing its own verdict, so the output the test reads carries no
result -- which is how the first version of this passed while measuring
nothing, and is the same class of bug it exists to catch.
HONEST LIMIT: four of the contract's twenty-one cases have a prover. The
remaining seventeen can still be gutted silently. Adding a mode per case is
mechanical; the mechanism is here and the four highest-value rules are the
ones covered -- start-first, ok-is-the-only-signal, empty-is-an-error, and no
false success on an unknown approval.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two product defects. Not test gaps -- the app did these wrong on a device, and
no test could reach either.
1. THE ENGINE WAS BUILT WITH A STUB THAT ANSWERED undefined
main.ts built the engine list with `facadeStub()`, whose `get` returned
undefined for everything, and the engine closed over it at boot. The stub's
own comment said it was "replaced by the real one immediately after". It was
not: nothing rebuilt the engine.
So settings were loaded from disk at boot purely to be discarded. A user's
engine address fell back to the hardcoded http://127.0.0.1:8765, and
`engineId` -- a declared setting, with a `choices` list a picker renders --
was ignored outright, because main.ts passed the literal "remote-http".
createApp now builds the real facade before selecting the engine, which it
could always have done: settings load first precisely so the engine choice and
its credentials come from them. The factory takes `(persistence, settings)`,
facadeStub is deleted, and the persisted engineId is preferred over the
caller's default.
An existing test caught the first attempt, and was right to. Reading the value
with `get()` alone let a def's DEFAULT outrank the caller's explicit argument
-- `get` returns the default for a key nobody ever touched, which is
indistinguishable from a deliberate choice. That is the same class of bug in
the opposite direction: a setting nobody set silently overriding the thing
that asked. The store now tracks which keys came from storage or an accepted
`set`, and `chosenStringOr` consults `isSet` before preferring one.
2. STOP CONFIRMED A CANCELLATION THE ENGINE REFUSED
The controller returns the engine's own answer, and both the scripted engine
and the conformance contract go out of their way to keep that boolean honest
-- "a Stop button that confirms a cancellation which never happened is worse
than one that reports it could not."
The one caller discarded it. The button label flips off `turn.phase`, which
settles either way, so a refused Stop looked exactly like an accepted one
while the run kept generating and kept billing. It is now announced in the
assertive live region. The decision is extracted as `stopNotice` so a test
calls it -- driving `mount` end to end needs a whole fake SSE transport, and
this is the part that can be wrong.
Every mutation pinned in both directions.
STILL OPEN, deliberately not fixed here: the remote-http engine discards
decode rejections (`if (isDecoded(outcome)) yield`), so a malformed frame
makes a tool vanish and the turn renders as a clean answer. The reducer's
Dropped type, the view model's dropped row and seven user-facing strings are
all unreachable because of it. Fixing it needs a channel from the engine to
the transcript that does not exist yet; half-wiring it would be the same
mechanism-connected-to-nothing this commit is undoing.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Three more survivors, found by a validation pass and confirmed by mutation
here before anything was written.
view-model.ts approvalRow -- `findApproval(table, pending.approvalId)`
replaced by `table.entries[0]`. Its own comment says "Looked up by approvalId
in both -- never zipped by index", and the whole approvalId design exists to
stop position-pairing. It survived the entire suite because every existing
test has exactly ONE approval in flight, where the first entry and the right
entry are the same object.
With two outstanding, the second row renders the first row's decision: an
`rm -rf /` prompt shows as already-allowed, with dead buttons, against a
decision the user never made. The defect the design prevents, reintroduced at
the last hop before the DOM. Both directions are pinned -- an approval with no
entry of its own must render pending rather than borrow a stranger's.
dom.ts -- `el.dataset["status"] = row.status` hardcoded to "ok", and
`durationKnown ? durationLabel : UNKNOWN` collapsed to the label. Both
survived. The ok-is-the-only-signal chain is airtight through decode, the
reducer and the view model -- every mutation aimed at those dies -- and then
the attribute the CSS keys off was written by nothing that checked it. A
failed tool rendered with the success styling. `seconds: null` means the
engine never observed the call begin, which is not zero; rendering the label
regardless states a duration that was never measured, and that invariant was
written in a comment two lines above code nothing tested.
Each pinned in both directions, so a test that catches one mutation is not
satisfied by code broken the opposite way.
Tests only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The earlier fix here moved the length check BEFORE segmentation, which made
short input 42x faster. It does nothing for the case that actually hurts: at
the real call site `max` is PREVIEW_CHARS = 120, so any long output always
overflows the check and always paid full segmentation anyway.
`buildTranscript` re-derives every tool preview on EVERY publish, roughly
thirty times a second during a streaming answer. One tool returning 40kB of
single-line JSON -- an API response, fetched HTML, a minified file, base64 --
therefore cost ~10ms per publish on a laptop, and a phone is 3-5x slower, so
that one tool blows the frame budget on its own. `firstLine()` rescues the
multi-line case; single-line output gets no such reprieve.
Only `max + 1` clusters are ever needed: `max` to keep, and one more to learn
whether anything was cut. `Intl.Segmenter.segment()` returns a LAZILY iterable
Segments -- `Array.from` is what forces the whole string.
10kB one line 9.7ms -> 0.04ms 61x
40kB one line 9.7ms -> 0.05ms 197x
200kB one line 47.9ms -> 0.12ms 407x
Byte-identical to the old implementation over 50,010 differential cases: 21
hand-picked (ZWJ family emoji, skin-tone modifiers, regional-indicator flags,
Devanagari conjuncts, max of 0/1/NaN/Infinity) plus 50,000 randomised.
The regression test is timed rather than counted, because the segmenter is not
injectable. The threshold has ~170x headroom over the fixed implementation and
still fails the revert by more than 2x. Both reverts were confirmed to fail it.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A second audit pass, this time asking what the FAKES excuse rather than what
the tests assert. Five more mutations survived the suite; all five now fail.
The root cause for three of them: TimePort declares `setTimer(cb, ms)` and
`every(ms, cb)`, and both fakes accepted the callback and threw the delay
away. A fake that takes fewer arguments than its port silently excuses every
caller from getting them right.
publish-gate setTimer(reopen, UNPAINTED_REOPEN_MS) -> setTimer(reopen, 0)
The gate reopens on the next macrotask: publish per token,
which is the entire thing the module exists to prevent. It
survived even the file's own positive control, because that
control never involves a timer.
controller every(maxDelayMs, ...) -> every(60_000, ...)
The coalescer's flush tick runs once a minute. This restores
verbatim the defect fixed three commits ago -- a short answer
arriving in one lump with zero intermediate paints -- and the
test written for that fix did not notice, because the fake
fired every callback on every tick regardless of period.
`FakeTime.tick()` now fires only the intervals whose period has actually
elapsed on the fake clock, so the period is load-bearing in every existing
test rather than in one bespoke assertion. `FakeScheduler` records the delay
it was armed with, and the gate's is asserted against the constant AND
against zero -- pinning it to the constant alone still passes if the constant
itself is set to 0, which is the same shipped behaviour.
coalescer `if (pending === "") openedAt = nowMs` -> unconditional
Every arriving token restarts the delay budget, so on a device
-- where tokens arrive continuously -- `waited` never reaches
maxDelayMs and text paints only at the byte cap. The existing
trickle test pushes ONE token, which makes the two versions
identical. It needs more than one drop to be a trickle.
boot.ts delete the lifecycle subscription, or background -> active
`assert.equal(stopped || true, true)` is true for every value
of `stopped` -- and `stopped` was in fact false: the test never
started a run, so `controller.stop()` returned early and
`cancel` was never reached. The comment beside it shows the
assertion was neutralised rather than the setup fixed.
The turn is now genuinely in flight before backgrounding, and
a paired test proves only `background` stops it -- otherwise
every resume notification would kill the turn on screen.
Tests and fakes only; no product source changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
boundaries.json has said this since the day it was written:
"governedRoots exists so a NEW top-level directory is reported as ungoverned
rather than silently exempt. A rule that stops covering new code while still
reporting a clean pass is worse than no rule."
Nothing implemented it. governedRoots was only a list of directories to WALK,
so a directory absent from that list was never visited. Adding features/ and
importing both '@tauri-apps/api' and 'praisonai' from it -- the two seams the
whole layer arrangement exists to defend -- still printed "126 files checked,
no violations" and exited 0.
That is the failure the fixtures in depgraph.test.mjs exist to prevent, one
level up: not a rule that gets an answer wrong, a rule that reports success
over code it never looked at.
The scan is now ungovernedRootsIn(root, config) in depgraph.mjs, following the
note at the top of check-boundaries.mjs -- the rule's logic stays a library
tests can call. It takes root as an argument so the tests point it at fixture
trees rather than mutating the real one. check-boundaries.mjs runs it before
the walk and exits 1 with the two ways to resolve it.
ungovernedRoots is the other half: tools, src-tauri and docs are now
deliberately declared exempt. Listing a directory is a decision someone made;
not listing one is caught.
Seven tests, including a shallow-scan case (features/a/b/c/x.ts), a no-source
case so docs/ cannot cry wolf, and one asserting the real boundaries.json still
accounts for every directory on disk -- which is the test that will fail on
somebody's future branch. Verified against three broken implementations:
always-[], report-everything, and shallow-only.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two more mutation survivors, both silent.
controller.ts: `ok ? acknowledge : reject` collapsing to always-acknowledge.
The engine reports that it did not accept a decision -- it restarted, or the
approval expired -- and the row still renders resolved. The user sees their
Deny confirmed against a command the engine never denied, which is the one
failure the approval table exists to make visible. Both directions are now
pinned: an always-reject controller trips three tests.
settings/store.ts: the secret guard inside persist(). Deleting it survived
because nothing can reach it -- `set` refuses a secret-flagged key and `load`
drops one, so through the public API no secret ever enters the map. That is
precisely why it needed a test: an unreachable guard is one nobody notices the
deletion of, and it is the last line between an API key and a plaintext
settings file the day a new write path forgets the rule.
The guard is now `plainOnly(values, byKey)`, following the rule this file
already states -- extract the expression so a test can call it rather than
asserting it appears in the source. Behaviour is unchanged; persist() calls
the extracted function. A falsy-value test pins the other half, since a
truthiness filter here would reset every disabled toggle on the next load.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
FakeTime.advance mutated its own offset with no monotonic guard, while the
createFakeClock it composes throws on a negative advance. A mistaken negative
advance silently rewound nowMs -- a state a monotonic TimePort never produces --
letting a pacing test pass or fail for the wrong reason. Guard it the same way.
Co-authored-by: Mervin Praison <[email protected]>
A mutation run over the package found five files that no test executes at
all -- they are wiring, so nothing imports them, and the suite was green
with them arbitrarily broken. Four survivors, each silent on a device:
app/src/platform.ts -- always returning the web shell. An on-device build
falls back to the browser shell: no safe-area insets, no OS back gesture,
no lifecycle. It looks like it works.
adapters/src/web/http.ts -- deleting `signal: request.signal`. Stop reaches
the engine and never reaches fetch, so the request keeps streaming and
keeps billing after the user cancelled.
adapters/src/web/http.ts -- flipping sendsFromNative to true. That flag is
the claim that an API key does not enter the webview heap; nothing checked
the claim against the adapter making it.
core/src/chat/session.ts -- transposing the two same-typed arguments of
persistenceFor's record(). Every stored conversation gets the user's
message and the model's reply the wrong way round, and a transposed
transcript still renders and still round-trips.
Each test was confirmed to fail against its mutation before being kept.
Tests only; no source file changed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The coalescer has two bounds: flush at maxBytes, or flush after
maxDelayMs. Only the first was ever connected. `coalescer.tick()` was
never called, and `TimePort.every` -- whose own doc comment calls it
"the coalescer's flush tick" -- had zero call sites outside tests.
So the time bound never fired. Measured before this: a 130-character
answer produced ZERO intermediate paints and arrived in one lump when
the run ended, making time-to-first-visible-token the whole answer's
latency. A long answer advanced in 256-character jumps roughly every
420ms. The publish gate's MAX_HELD_CHARS = 96, tightened deliberately
for mobile with a comment explaining why 256 was too coarse, never bound
either -- the coalescer upstream was already withholding more.
In an app whose stated first priority is streaming performance, the
streaming was not connected.
The tick is stopped in `finally`, before anything else: one firing after
the turn settled would paint into a finished transcript and keep a timer
alive for the app's lifetime.
Its frame is deliberately NOT gated. The gate skips intermediate paints
when the renderer cannot keep up; this frame exists precisely because
nothing has painted for maxDelayMs, so gating it reintroduces the stall
it was added to prevent.
WHY NO TEST NOTICED: testing/src/fake-time.ts implemented `every` as
`() => () => {}`. A fake that cannot observe the thing it stands in for
makes every test using it pass for the wrong reason. It drives real
interval callbacks now, and can advance the clock.
And my first attempt at the test was itself vacuous -- it asserted "more
than one paint carried text", which the `end` flush satisfies with the
tick removed. The assertion is on PARTIAL text now: without a tick the
text goes straight from "" to the whole answer. Verified by removing the
tick again: it fails.
838 tests, boundaries clean.
Two findings from an adversarial audit. The first is the most serious
thing found in this package.
app/src/dom.ts HAD NO TEST AT ALL. Changing `el.textContent = row.text`
to `el.innerHTML = row.text` left all 830 tests passing. Model output,
tool results and summarised web pages all flow through that line, so the
file's own header -- "makes that structurally impossible rather than
depending on an escaping function nobody re-reads" -- was guaranteed by
nothing.
The only thing in the suite that mentioned applyOps asserted the BUNDLE
CONTAINED THE SYMBOL. Rename it and that breaks; gut its body and it
passes. That is the shape of a test that looks like coverage and is not.
Six tests now, and they kill what survived: the innerHTML swap fails 2,
and `b.disabled = false` -- the guard that stops a double tap sending
two decisions for one approval -- fails 1. Both verified by re-applying
the mutation.
TRUNCATE SEGMENTED BEFORE CHECKING. `graphemes(text)` ran over the whole
string, then the length check discovered it had not been needed. A code
unit count is an upper bound on graphemes, so the check can come first.
Measured: a 20-character preview went from 8.83us to 0.21us, 42x. It
matters because buildTranscript re-derives every tool preview on EVERY
publish -- with eight 40kB tool results that was 44ms per publish and
~361MB of transient allocation across one turn, to regenerate previews
that had not changed since the tool returned.
839 tests, boundaries clean.
test_the_recorded_pid_owns_the_trainer_signalable_by_group failed in CI with
AssertionError: recorded pid 2568 does not own the trainer
(trainer's parent is )
assert '' == '2568'
and it blocks unrelated pull requests when it does.
The stand-in trainer records its parent with
ps -o ppid= -p $$ | tr -d ' ' > trainer_ppid
and the wait loop broke as soon as all three files EXISTED. Shell redirection
creates its target before the command on the left produces a byte, so
trainer_ppid exists and is empty for as long as `ps` takes to run. The loop
saw it, stopped waiting, and compared '' against the recorded pid.
It is not a slow machine and not a short timeout -- the window is the runtime
of `ps`, so a longer wait does not help. Waiting for CONTENT does.
Measured on the same shell shape, 300 trials each:
existence-only (before) read an empty value 296/300
content (after) read an empty value 0/300
The real loop polls at 100ms, which usually outlasts `ps` -- which is why this
passes most of the time and fails under CI load rather than every run.
The emptiness check is also asserted separately from the ownership comparison,
so a genuine timeout reads as "the trainer never recorded its parent" instead
of masquerading as the ownership bug this test exists to catch.
Separately, and NOT fixed here: this test is unsafe to run on a host without
`setsid` (macOS). The launch script cannot make the wrapper a group leader,
so the wrapper stays in pytest's process group and the test's
`os.killpg(os.getpgid(recorded), SIGTERM)` terminates the test runner itself
-- pytest exits -15 before reporting. On Linux CI `setsid` exists and the
group is isolated, so this does not affect the pipeline. Flagged for whoever
owns this suite; fixing it is a design choice about how the test spawns.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The rust shell job ran cargo test/clippy directly, but tauri::generate_context!
expands frontendDist (../dist) at compile time and dist/ is gitignored -- absent
on a fresh checkout. Both matrix jobs failed to compile before any test ran.
Add setup-node + npm ci + npm run build to create ../dist first.
Co-authored-by: Mervin Praison <[email protected]>
Found by an adversarial validation pass. Two problems, and the second is
why the first survived.
THE BUILD IS BROKEN. capabilities/default.json names a permission
`allow-back-gesture-result` that does not exist, so tauri-build fails
before anything compiles:
Permission allow-back-gesture-result not found
An app command registered through invoke_handler is always reachable and
has no ACL entry to grant -- only plugin commands do. Removed, with the
reason recorded in the file's own description so it is not re-added.
NOTHING WOULD HAVE CAUGHT IT. mobile.yml's path filter already triggers
on src-tauri changes and it has no cargo job at all -- so a crate with
14 tests had none of them run, and a crate that does not compile merged
green. That is precisely the failure this workflow's header says it was
created to prevent, one language over.
The new `shell` job runs cargo test and clippy -D warnings on
ubuntu-22.04 AND macos-15. Both, because the crate is cfg-heavy: the
back-gesture fallback is #[cfg(target_os = "android")] / ios /
not(mobile), and a cfg mistake compiles perfectly on whichever host you
happened to try.
Clippy then caught something real in my own test: asserting on a const
inside a test is compile-time-known, so it proved nothing at runtime.
The floor is a const assertion in the source now -- lowering
ANSWER_TIMEOUT_MS stops the crate compiling rather than failing a test.
Verified by lowering it to 50: E0080, build refused.
14 Rust tests, cargo check clean, clippy clean with -D warnings.
- add src-tauri/capabilities/default.json so the main webview can
subscribe to the four shell events (core:event) and invoke
back_gesture_result; without a capability Tauri v2 silently rejects
these, disabling the entire shell contract (Greptile P1).
- shell-seam.test.mjs: use fileURLToPath(import.meta.url) instead of
URL.pathname so the test resolves on Windows checkouts, matching the
sibling check-boundaries.mjs (Greptile P2).
Co-authored-by: Mervin Praison <[email protected]>
The Rust side of the two seams. Everything the app does lives in the
webview; this crate exists to tell it four things a browser cannot know
and to act on the one answer it sends back.
THE EVENT NAMES ARE A CONTRACT, and a rename is silent: the webview
simply stops receiving that event and lays out as though the phone had
no notch, no keyboard and no lifecycle. Nothing throws. So there are two
guards -- a Rust test pinning each constant to a literal, and a Node
test that READS both sources and compares them, catching a rename in
either language. Verified by renaming one on the Rust side: both fail.
BACK-GESTURE ARBITRATION is the piece with real failure modes, and it is
pure with the platform injected, so all of them are tested on a laptop:
- The answer may never come. bridge.invoke swallows every rejection on
the TypeScript side, so silence is indistinguishable from success --
a bundle that failed to load would leave a back button that does
nothing FOREVER, which is worse than one that exits. Hence a
watchdog.
- There is no correlation id. The webview sends { handled } and
nothing else, so two presses close together produce two answers Rust
cannot tell apart, and the second could pop an activity the first
decided to keep. Dropping while pending is the only correct option
on this side of the frozen contract.
- A late answer after a timeout must not fall back twice. That is the
bug this design is most likely to ship, and it has its own test.
LIFECYCLE maps Suspended to "background" rather than "inactive", and the
comment says why rather than leaving it to be rediscovered: boot.ts only
flushes on background, and on iOS the app can be killed while suspended
with no further callback. Mapping to inactive would lose the transcript
on every backgrounding. The cost -- a control-centre pull-down stopping
the run loop -- is the cheaper mistake.
panic = "abort" is deliberately NOT set, unlike the desktop crate:
mobile_entry_point wraps the app in catch_unwind, and abort turns a
readable message into a bare SIGABRT on a device with no console.
Icons are generated placeholders in the product's own accent colour,
written by hand as RGBA because Tauri rejects RGB.
14 Rust tests, 830 TypeScript tests, boundaries clean.
- view-model: an approval kept from an ended turn (resolved) now renders
non-actionable even though its decision-table entry stays pending, closing
the P1 where a finished run could still submit/retry a decision it can no
longer deliver.
- strings: route droppedEvents through a module-level enDroppedReason instead
of the sibling en.droppedReason, since bundle.ts lifts each string function
off the table and there is no reliable `this` (Greptile P2).
- test: cover an ended-turn approval rendering as history, not a live prompt.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
None of them blocked shipping, which is exactly why they sat. Two were
one bad day away from mattering.
GAP 2 -- settle() deleted the approvals, so a finished transcript could
never say "you were asked to allow rm -rf / and the turn ended first",
which is the thing a reader most wants to see afterwards. They are kept
now and marked resolved: NOT ACTIONABLE rather than NOT RECORDED. The
pair test is the one that matters -- marking everything resolved would
satisfy the audit-trail case while making every live prompt
unanswerable.
`resolved` is optional rather than required, and the reason is in the
type: absent IS the answer for every approval that has not been settled,
which is all of them until a turn ends. Writing `resolved: false` at a
dozen construction sites would be noise carrying no information, and
noise is where a real value gets mistyped unnoticed.
GAP 3 -- ApprovalEntry is now literally PendingApproval & { state }. The
duplication forced the view model to join the two by approvalId at
render time, and that join is where index-pairing gets reintroduced --
pairing an approval by position is the defect the whole approvalId
design exists to prevent. Removing the join removes the opportunity.
GAP 7 -- a dropped event said `wrong_msg_id` to a user, which tells them
nothing. It reads as a sentence now AND keeps the tag: translating the
tag away would make the one searchable string in the message
unsearchable for whoever they report it to. An unknown tag passes
through rather than becoming "unknown reason", because a newer engine
can invent one and the tag is then the only information present.
Verified by reverting each: emptying the approvals again fails 2 tests,
returning bare tags fails 3.
An existing test also caught me adding a string key with no sample call,
which is the guard working exactly as intended.
826 tests, boundaries clean. Every gap in docs/gaps.md is now closed.
The wrapper now backgrounds the trainer and records the trainer's own pid
($!) instead of the wrapper shell's, so stop() sends SIGTERM to the process
actually doing the work rather than a sh -c wrapper that never forwards it
(which orphaned the run). The wrapper program is quoted once with shlex.quote
for the outer shell, so a stray apostrophe or metacharacter in the workdir or
dataset name can no longer close the sh -c string and rewrite the command.
Updated the regression test to assert the recorded pid IS the trainer and that
a TERM to it actually stops the trainer, and to kill all descendants on exit.
Addresses Greptile P1 (stop non-forwarding), P1 (nested quoting), P2 (test leak).
Co-authored-by: Mervin Praison <[email protected]>
_launch_script recorded $! from the outer shell -- the transient subshell
that backgrounded the whole `&&` list -- not the process that owns the
trainer. status() then read `unknown` for a live run and stop() signalled
the wrong pid. The wrapper now records its own pid from inside (echo $$),
and a brace group with </dev/null releases ssh's stdin/stdout so the launch
returns immediately.
Co-authored-by: MervinPraison <[email protected]>
Addresses Greptile P1: after SIGTERM to the group, stop() checked only the
recorded wrapper pid. A wrapper can exit while a child (dataloader worker,
torchrun rank) survives SIGTERM, so stop() reported "stopped" while paid GPU
work continued. Re-probe now scans the whole process group; "stopped" means
no member of the group remains. Adds a test where the wrapper exits and a
SIGTERM-trapping child survives — stop() must raise, not lie.
Co-authored-by: Mervin Praison <[email protected]>
Pinch-zoom shrinks visualViewport.height below innerHeight exactly as a
software keyboard does, so readKeyboardHeight reported a phantom keyboard
the moment a page was zoomed -- most visibly at construction, where a page
opened already zoomed seeded a positive height and pushed the composer up
on the first frame. A software keyboard never changes scale, so scale > 1
is zoom, not a keyboard; guard on it and report 0.
The live publishKeyboard path now reads through the same readKeyboardHeight
function, so the clamp and the zoom guard live in one place and cannot drift
between the first frame and every frame after it.
Positive control: fake-window gains setZoom(scale); reverting the guard fails
the two new tests. Documents the remote-http persistence scope in gaps.md so a
reviewer's finding is not later read as an oversight.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
The view models existed and nothing mounted them: the app rendered one
hard-coded transcript and the router drove nothing.
The decision layer is pure, and it is pure for a specific reason -- the
cases that matter are about what should be KEPT, and a test that can
only inspect a DOM cannot tell "rebuilt identically" from "left alone".
The chat screen is retained; the others are not. Rebuilding the
transcript on every return from settings throws away every row node and
the scroll position with it, so a reader who scrolls up, checks a
setting and comes back is dumped at the bottom of the conversation. That
is the most annoying bug a chat app can have and it is invisible to a
test that only asserts the right rows are present afterwards.
Retention is a cost, not a default: a retained screen holds its DOM for
the app's lifetime. Worth it for a live transcript with scroll state,
not for a settings form that should re-read its values on entry.
Mount happens BEFORE hide, so no navigation shows an empty frame.
A mutation run caught a vacuous test of my own here, and it is worth
recording. The fake DOM logged append and remove but not visibility --
so reordering hide before mount changed nothing it could see, and the
test named "mounting happens BEFORE hiding" asserted nothing at all. The
fake records show/hide now and the mutation fails it.
Other mutations caught: destroying a retained screen instead of hiding
it fails 2, unmounting a retained screen fails 1, and treating
same-route as always-noop fails 1.
816 tests, boundaries clean across 123 files.
A workdir like `-rf` passed the character-class check but would be parsed
as an option by the remote mkdir/cd. Add `--` to end option parsing so it
is always treated as a path; tilde expansion is unaffected since the shell
expands `~` before either command sees the argument. Adds a regression test.
Co-authored-by: Mervin Praison <[email protected]>
A reviewer amended gaps 4 and 5 back to open after I marked them closed,
and was right on both. The same mistake twice: a mechanism existing is
not a mechanism working.
GAP 4 -- "mechanism landed, not wired". createSession was called.
RunPersistence.record was called. Nothing connected them, and the two
signatures did not even line up -- record(prompt, answer) against
record(request, answer) -- which is why it went unnoticed. So record()
never ran in a real turn and no conversation was ever saved, while the
gap read as closed from either end.
persistenceFor() is now the named adapter where the two vocabularies
meet. Naming it rather than inlining a lambda at the call site is
deliberate: this is the one place they touch, and a lambda buried in
composition is a seam nobody finds later.
AppDeps.engines is now a FACTORY taking the persistence, so there is no
way to obtain the engine list without being handed the thing engines
write through. A pre-built array was the original bug; the type refuses
it now. That change surfaced every unwired call site immediately, which
is the argument for making it a type rather than a convention.
The session is also built BEFORE engine selection, because the engine
writes through it.
GAP 5 -- "property added, snapshot not seeded". keyboardHeightPx was
declared = 0 and only ever updated by an event, so a component mounting
while the keyboard was already up laid out at 0 for one frame and then
jumped: precisely the bug the synchronous property was added to prevent.
Seeded from visualViewport at construction now.
Both verified by positive control -- reverting either fails a named
test. 798 tests, boundaries clean.
RemoteRunner.stop() killed only the recorded pid -- the wrapper shell -- so
the trainer's children (dataloader workers, torchrun ranks) survived on the
GPU, reparented to init. Resolve the process group on the host, signal -$pgid,
then re-probe: return True only when the process is actually gone, raise when
it survives SIGTERM, and refuse to signal the ssh session's own group. Launch
under setsid where available (portable fallback for macOS).
Co-authored-by: MervinPraison <[email protected]>
resolve_workdir quoted the workdir before the remote shell saw it, so ~
never expanded and a directory literally named '~' was created. Validate
the workdir (same rule as settings.validate) and pass it unquoted so the
remote shell expands it; use mkdir -m 700 so a shared box's umask does
not decide who can read the shipped config and dataset.
Co-authored-by: MervinPraison <[email protected]>
Gap 4 (session/live-turn identity) and Gap 5 (keyboard snapshot) were marked
CLOSED but are only half-closed in code:
- Gap 4: core/src/chat/session.ts implements RunPersistence, but no engine is
wired to it in production — createPraisonTsEngine gets a RunPersistence only
in tests, boot.ts never passes session to an engine, and remote-http has no
persistence. Turns are not recorded end-to-end.
- Gap 5: ShellPort.keyboardHeightPx exists but both adapters init it to 0 and
only correct on a later event; unlike insets it is not seeded synchronously,
so a warm resume still paints at 0 then jumps.
Gap 6 is genuinely closed (defs/subscribe/setSecret/clearSecret on the facade)
and stays closed. Reclassifies 4 & 5 as still-open and updates the audit
preamble to match its own rule: a gap is closed only when wired end-to-end.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
Found while auditing what is outstanding. Gaps 4, 5 and 6 were fixed and
left recorded as open:
4 no user-message side closed by core/src/chat/session.ts
5 keyboard not synchronous closed by ShellPort.keyboardHeightPx
6 settings cannot enumerate closed by defs/subscribe/setSecret
That is not a filing problem. It is the same failure this package is
built against, one level up: a document asserting a state nobody
re-checked. And it has a measurable cost -- a stale P0 list for
praisonai-ts sent two agents to verify nine "blockers" that were every
one of them already fixed. Useful work, since it turned up three real
bugs nobody had listed, but not the work it was supposed to be.
So the file now carries the rule as well as the state: close a gap here
in the same commit that closes it in code, and verify before believing
what it says. Every claim in it is checkable in under a minute.
Genuinely still open, and recorded as such: gaps 2, 3 and 7, none of
which blocks shipping. Plus route-to-view dispatch and the Tauri crate,
which are unbuilt work rather than defects and are named separately so
they are not mistaken for gaps.
794 tests, boundaries clean.
Blanket digit deletion in normalise_error collapsed distinct failures
(HTTP 404 vs 500, exit code 12 vs 34) onto one signature, suppressing the
promised re-alert on a changed error (Greptile P1). Now strip only volatile
numbers — hex/uuid blobs, long digit runs (timestamps/pids/request ids) and
numbers fused into identifier words — while preserving short standalone codes
so genuinely different failures re-alert and changing request ids still group.
Co-authored-by: Mervin Praison <[email protected]>
Address reviewer findings on the shared durable idempotency store:
- DeliveryRouter: a send suppressed for a dead target, or cancelled while
waiting on the rate limiter or during the adapter send, now releases the
durable reservation claimed up-front by _is_duplicate(). Previously the key
stayed 'inflight' for up to the 15-min lease, wrongly suppressing legitimate
retries. CancelledError is a BaseException so it is handled explicitly.
- SchedulerDelivery: the durable store path now honours $PRAISONAI_HOME
(falling back to ~/.praisonai) so two isolated deployments under one OS
account keep separate dedup ledgers instead of cross-suppressing.
- Tests: add dead-target and cancellation release regressions.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
Promote the existing durable SqliteIdempotencyStore to an injectable dedup
substrate the DeliveryRouter uses on every path, so restart-safe
effectively-once delivery is no longer gateway-only. The lightweight
SchedulerDelivery now injects the same store the gateway uses, and a failed
send releases the durable reservation so it stays retryable.
Co-authored-by: MervinPraison <[email protected]>
Add a pure, dependency-free incident brain beside RunRecord: error_signature()
groups failures by a stable sha256 of the normalised error, and IncidentTracker
folds the run stream into de-duplicated operator alerts — alert once per
incident, re-alert only on a changed signature, resolve on recovery, and an
optional N-consecutive-failure threshold. Core-only decision logic so every
engine (gateway, wrapper, standalone) shares one policy; delivery reuses the
existing DeliveryTarget plumbing.
Co-authored-by: MervinPraison <[email protected]>
Address Greptile P1 findings: by_model, total_interactions and
recent_interactions in the token reports were still copied from the
process-global TokenCollector summary, leaking other concurrent
PraisonAIAgents instances' usage. Re-derive by_model and
total_interactions from this instance's own interaction records and
filter recent_interactions to owned agent names.
Co-authored-by: Mervin Praison <[email protected]>
Route the backward-compatible thinking_budget setter through the unified
reasoning_effort path so the CLI's post-construction override actually
reaches the provider request (previously dormant: it only set
_thinking_budget, never emitted the native parameter). Also sync a
post-construction change into an already-built LLM instance.
Wire session persistence + resume for reasoning effort in the CLI:
persist the graded effort alongside the model in apply_cli_session_continuity,
add find_session_reasoning_effort, and restore it on --continue/--session
next to the existing model restore, mirroring the model precedent (#3685).
Add unit coverage for the thinking_budget setter reaching the request and
for the wrapper effort-restore helper.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
Co-authored-by: Mervin Praison <[email protected]>
Store-backed jobs added via `praisonai schedule add` previously never fired
unless the gateway/BotOS/UI host was running, because only those hosts wired
the store poller. `praisonai schedule start` runs the unrelated agents.yaml
engine, so a plain CLI user's jobs silently never executed.
- Core: add `ScheduleLoop.run_forever(poll_seconds, owner_id)` — a foreground,
blocking counterpart to `start()` that reuses the same lease-safe `fire_due`
tick body (at-most-once `claim_due_jobs`, no double-fire alongside a gateway).
- Wrapper: add `praisonai schedule run` CLI command that reuses the existing
`schedules_runner` bridge executor (delivery fallback included) and blocks
until Ctrl-C.
No new engine, params, or exports beyond one core convenience method and one
CLI command.
Co-authored-by: MervinPraison <[email protected]>
- agent.py: last_stop_reason returns a specific agent-owned block/refusal/
truncation before backend fallbacks so a stale backend "max_steps" no longer
masks the OpenAI-native classification.
- chat_mixin.py: reset _last_stop_reason at the start of the async native turn
(mirrors the sync _chat_completion reset) to prevent cross-run leakage.
- execution_mixin.py: narrow the stop-reason lookup to AttributeError so a
lookup failure is not silently converted into a successful "completed".
- tests: add regressions for agent-owned-vs-backend precedence.
Co-authored-by: Mervin Praison <[email protected]>
Addresses reviewer feedback (Greptile) on the SqliteEventLog sink:
- Cursor no longer rewinds after a full prune: per-session seq is allocated
from a persistent event_seq high-water-mark table instead of MAX(seq) over
live rows, so deleting every row for a session never resets its counter.
- Seq allocation is atomic across connections/processes sharing one DB file:
read+bump+insert run inside a BEGIN IMMEDIATE transaction, eliminating the
read-then-insert race that previously dropped colliding events.
- Caller-provided event.metadata now persists (metadata_json column) and is
restored on query, with the durable seq/session_id overlaid on top.
- Additive migration for pre-existing databases: adds metadata_json and seeds
event_seq from MAX(seq) so the next append continues above existing rows
instead of colliding on the primary key.
Adds regression tests: post-full-prune monotonicity, metadata round-trip,
cross-connection no-collision, and legacy-schema migration.
Co-authored-by: Mervin Praison <[email protected]>
Add an opt-in SQLite sink for the existing EventBus so lifecycle events
survive a restart and can be read back per session with a cursor.
- New core module bus/event_log.py: EventLogProtocol + SqliteEventLog
(stdlib sqlite3, WAL, best-effort append, monotonic per-session seq,
cursor query(session_id, after_seq), age/count prune).
- EventBus.attach_sink()/detach_sink()/has_sinks: sinks receive every
published event (including the no-subscriber fast path) and their
failures are swallowed so a full/slow log never breaks a turn.
- Export EventLogProtocol/SqliteEventLog from bus/__init__ (lazy).
- Backward compatible: a bare Agent() attaches no sink, preserving the
zero-overhead in-memory path; the in-memory ring buffer is unchanged.
Co-authored-by: MervinPraison <[email protected]>
When check_same_thread=True, a worker-created connection cannot be closed
from close()'s caller thread — sqlite3 raises ProgrammingError. The prior
code swallowed it then cleared _conns, silently dropping (leaking) those
connections untracked. Now such connections are retained in _conns so they
stay reachable for a later close() from their owning thread; cleanly-closed
connections are still removed. Default check_same_thread=False path unchanged.
Co-authored-by: Mervin Praison <[email protected]>
- async_agent_scheduler: retry backoff now waits on _stop_event via
_sleep_or_stop(), so stop()/shutdown is no longer held for the full cap
- _base_scheduler: daemon state written atomically (tempfile + os.replace)
and JSON/OS errors logged at warning instead of being silently swallowed
- sqlite conversation store: track every thread-local connection and drain
them all in close() so a thread pool no longer leaks N-1 connections
Co-authored-by: MervinPraison <[email protected]>
Address two Greptile P1s on PR #4533:
1. Async tool exceptions now count toward the circuit breaker. The
post-invoke record block was skipped when the tool *raised* (control
jumped to the except handler), so raised failures never opened the
breaker. Record a breaker failure in the exception handler too, matching
the sync path's breaker.call.
2. The in-context is_already_approved/mark_approved cache is now keyed by
the per-instance scope_id (falling back to agent_name), so two same-named
agents sharing an execution context can't pre-authorize each other via the
name-keyed fast-path checked before the session lookup. Threaded scope_id
through approve_sync/approve_async, the two _check_tool_approval mark sites,
and the module-level wrappers.
Adds regression tests: async breaker opens after 5 raised failures and is
per-instance; context cache is isolated by scope_id with name-keyed fallback.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
Extract countSubstantiveMergeGateRuns helper and add direct regression
tests so a reload of workflow_run probes into the pending cap is caught
by the self-test (addresses Greptile coverage gap on PR #4534).
Co-authored-by: Mervin Praison <[email protected]>
Auto-dispatch probe runs were counted toward the max-pending cap, blocking
repository_dispatch for merge-ready PRs even when no assess job was queued.
Co-authored-by: Cursor <[email protected]>
Three sibling code paths silently diverged from the "safe by default,
multi-agent safe, async-safe" guarantee. Each mirrors an existing correct
pattern that the sibling path never received:
1. Async tool execution now engages the per-agent circuit breaker. Added
shared helpers (_circuit_breaker_precheck / _circuit_breaker_record) in
tool_execution.py using the same instance-scoped breaker key as the sync
path, and wired them into _execute_tool_async_impl so a persistently
failing tool short-circuits with the circuit_open signal the async retry
loop already checks for, instead of being hammered indefinitely.
2. ApprovalCallback.__call__ now routes async approve_fn through the shared
run_coroutine_from_any_context bridge instead of hand-rolling a new event
loop with no running-loop guard, replacing an uncaught loop-crash inside a
running loop (e.g. FastAPI async route) with the codebase's standard clean
error.
3. Session-scoped ("this session") tool approvals are now keyed by the
per-Agent-instance _approval_scope_id via a new scope_id parameter on
approve_sync/approve_async (threaded into _is_session_scoped /
_persist_scoped_decision), so same-named agents no longer inherit each
other's human-granted shell-execution approvals. agent_name is retained
for name-keyed lookups (per-agent backend, risk level, ask rules, durable
"always" persistence). Same fix applied to the doom-loop approval path.
Co-authored-by: MervinPraison <[email protected]>
FINAL architecture reviewer prompt now mandates the Phase 1 table with an
explicit SDK value row and TS AGENTS.md routing. Merge-gate helpers treat
src/praisonai-ts/ as SDK scope for test enforcement and CI check patterns.
Co-authored-by: Cursor <[email protected]>
The user data directory was only created inside provision_engine, which
runs solely after the user clicks "Get started" on the first-run setup
screen. A clean machine that merely opened the app -- or one whose data
folder was deleted to repair a corrupt state -- therefore left no Roaming
directory at all, indistinguishable in a clean-start QA check from a shell
that never started. Create it eagerly in setup() (the primary-only path;
a secondary never reaches it) so the directory exists the moment the app
is running, whether or not provisioning has begun. Best effort, so a shell
never fails to start because it could not create its data directory.
Co-authored-by: Dhivya-Bharathy <[email protected]>
Strengthen the fetch_url redirect regression test per reviewer
feedback (Greptile). The test previously only checked the secret
body was absent from the result -- an implementation that contacts
the unapproved loopback target and discards its response would still
pass. Record requests to server B and assert it receives none, since
the request itself is a side effect (actions, logging, rate limits).
Mutation-verified: reverting the one-line server.py fix makes the
strengthened test fail on both the leaked body and the recorded hit.
Co-authored-by: Mervin Praison <[email protected]>
The approval card authorises one URL, but urllib.request.urlopen follows
3xx by default, so a 302 could send the fetch to any host or port --
including this engine on loopback -- and return its body to the model
under an approval that never named it. Fetch through an opener that
refuses redirects and reports the unapproved target instead.
Part of the #4504 desktop audit tracking issue (finding #1 of 16, high).
Co-authored-by: MervinPraison <[email protected]>
The bulk "Delete all conversations" path returned early on any failure
without checking whether the active conversation was among the deletes
that succeeded, leaving its transcript on screen while gone on disk.
Track the active id and blank the transcript when it was deleted, even
if a different conversation's delete failed. Adds a regression test.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
The engine answered {"ok": true} whether or not the chat was removed,
and both UI delete paths blanked the transcript regardless. A read-only
or synced data dir left the conversation closed on screen and back in
the sidebar on the next refresh.
- engine/server.py: do_DELETE returns 400 on an invalid id and 500 on
an OSError instead of swallowing both.
- ui/index.html: the sidebar delete and runAction('clear') now check the
response before blanking the transcript, matching the per-message
delete already in place.
- tests: engine DeleteChats route tests and two buttons.test.mjs cases.
Co-authored-by: MervinPraison <[email protected]>
After rebasing onto main, saveCfg reconciles the server's echoed settings back
into CFG. The stub returned the original settings for both GET and POST, so the
persisted step was overwritten and rapid text-size steps collapsed. Echo the
written patch like the real engine does.
Co-authored-by: Mervin Praison <[email protected]>
Moving the CFG mutation to after the await made rapid text-size steps read
stale state, collapsing bursts and letting overlapping writes land out of
order. Queue writes FIFO and step from the pending target so bursts
accumulate (fixes Greptile P1).
Co-authored-by: Mervin Praison <[email protected]>
saveCfg mutated CFG/cfg and applied prefs before POSTing /settings, with no
r.ok check and no catch. A rejected write (e.g. across the engine restart the
base_url/api_key rows warn about) left the theme button dead, toggles diverged
from stored state, and setTextSize toasted success while every write failed.
saveCfg now persists first, mutates only on r.ok, toasts on failure, and
returns a boolean. apply() and setTextSize gate their visible effects on it.
Co-authored-by: MervinPraison <[email protected]>
Route the persistence doctor's URL->backend detection through the tested
PraisonAIDB._detect_backend instead of hand-rolled substring sniffing.
Doctor-specific lenient fallbacks (sqlite/qdrant/memory and the chroma
special-case) are retained via a CLI-local wrapper.
Co-authored-by: MervinPraison <[email protected]>
The trainer is spawned with start_new_session=True, so on macOS/Linux a
signal aimed at the engine's process group never reaches it. The engine's
exit handler was `lambda *_: sys.exit(0)`, which let the fine-tune reparent
to init and keep the GPU with nothing left able to find or stop it.
The exit handler now calls Trainer.stop() first, which terminates the
trainer's own process group before the engine exits. Well within the 2s
Engine::shutdown allows before SIGKILL.
Co-authored-by: MervinPraison <[email protected]>
The toast intentionally paints below the settings scrim (see layout.test
'the settings scrim covers the toast'), so blank-name and backend-error
feedback shown while the MCP overlay is open was dimmed and unreadable --
the same invisible-feedback bug this PR set out to fix. Route both MCP
error paths through askConfirm, whose .confirm-back (z-index 90) paints
above the overlay (z-index 80).
Co-authored-by: Mervin Praison <[email protected]>
The PR fixed list_chats() but /projects and /search call load_chat() on the
same files, where a valid-but-non-object JSON (the app's own array export)
returned the raw list and crashed on .get(...) with AttributeError -- dropping
the connection on exactly the file the PR set out to handle.
Co-authored-by: Mervin Praison <[email protected]>
close() flushes, so a full disk or revoked directory can raise from the
reader's finally block just as a per-line write can. If it escapes, it
skips proc.wait()/run.finish() and wedges the run in "running" forever --
the same permanent wedge this reader exists to prevent, moved to the last
line. Guard the close and add a regression test that reproduces the wedge
on the unfixed code.
Co-authored-by: Mervin Praison <[email protected]>
std::env::vars() panics on any non-Unicode env key/value, and the release
profile sets panic = "abort", so a single non-Unicode inherited variable would
abort the desktop app before the engine started. Switch to vars_os() and drop
undecodable entries; spawn_env only handles String, so nothing usable is lost.
Co-authored-by: Mervin Praison <[email protected]>
The settings client optimistically set launch_at_login=true and ignored
the POST response, so when the OS declined to register a login item the
toggle stayed on for the session even though the server persisted false.
saveCfg now reads the response and reconciles any keys the server wrote
back, and the launch_at_login handler surfaces the explanation.
Co-authored-by: Mervin Praison <[email protected]>
Two fixes to the restorable-conversation feature:
- Preserve the tool `name` through getHistory/setHistory. toAISDKPrompt sets
the tool-result part's toolName from msg.name; without it a restored tool
history sent to a non-OpenAI (AI SDK) backend emits an empty toolName and is
rejected. Added optional name to AgentMessage, copied it in copyMessage, and
carried it in the start() replay.
- Clear responseCache in setHistory so a repeat prompt is re-evaluated against
the restored history instead of returning the pre-restore prompt-keyed answer.
Added regression tests for both; full suite: 1141 passed, 0 failed.
Co-authored-by: Mervin Praison <[email protected]>
Two valid restart-recovery bugs from PR review:
- _persist now writes a temp file, fsyncs and os.replaces it, so an engine
killed mid write can never leave partial JSON that _reload silently skips
(which would lose the live run from history, stop() and the single-GPU guard).
- start() persists the run (pending) before the supervisor thread exists, and
the spawn-failure branch persists its FAILED state, so a child spawned before
the first post-spawn _persist is never left with no run.json.
Adds two tests. PID reuse is left as the documented minimal tradeoff.
Co-authored-by: Mervin Praison <[email protected]>
Return the client captured in a local before the awaited Logger.debug so a
concurrent credential change or resetOpenAIClient() during that yield cannot
hand a caller the wrong client or null. Adds two race regression tests.
Co-authored-by: Mervin Praison <[email protected]>
The tool_call/tool_result mapping landed but the seam documentation still
described the pre-tool three-variant union, contradicting the new support.
Aligns engine.ts, agent-api.ts and conformance.test.ts headers and gaps.md
so the capability status reads consistently: tools supported, approvals the
one remaining gap. Docs/comments only; no behaviour change.
Co-authored-by: Mervin Praison <[email protected]>
WKWebView under Tauri has no JS dialog panel, so prompt() returns null
and alert() shows nothing. This made "Move to project" inert and hid MCP
add errors. Add an askText() helper mirroring askConfirm(), and route
the three sites through askText/toast. Update the button test stubs to
model the real platform (prompt->null, alert throws) and add a
contextmenu test asserting a POST /project request is issued.
Co-authored-by: MervinPraison <[email protected]>
venv_resolve::spawn_env strips PYTHONPATH/PYTHONHOME/PYTHONSTARTUP and sets
VIRTUAL_ENV/PATH, but had no production caller -- supervisor::start inherited
the shell's environment and only added PYTHONUTF8/PYTHONIOENCODING. A user with
PYTHONHOME or PYTHONPATH exported got the engine pointed at a foreign stdlib or
site-packages, and the module written to prevent exactly that never ran.
start now takes the resolved env and applies it with env_clear + envs; main.rs
builds it from the already-resolved VenvLayout. Adds a regression test proving a
poisoned PYTHONHOME/PYTHONPATH never reaches the spawned child.
Co-authored-by: MervinPraison <[email protected]>
Wrap the settings runAction body in runActionInner and catch failures with
a toast, so Export/Check for updates/MCP no longer resolve into silence when
the engine is unreachable. Export additionally throws on non-ok responses so a
500 cannot slip past, and the clipboard is only written on success -- a failed
export can no longer masquerade as a backup via a stale paste.
Co-authored-by: MervinPraison <[email protected]>
The desktop settings registry described MCP servers as launched over stdio and offered an enable switch, but engine/server.py load_mcp() has no client consumer: _get_agent builds Agent(tools=_builtin_tools()) and never spawns a server. The text claimed a live capability that does not exist.
Apply the honest-text fix to all three synced registry copies and the Manage panel, and add a registry-drift guard asserting the hint no longer claims a launch the engine never performs.
Co-authored-by: MervinPraison <[email protected]>
On Windows venv_python_rel() returned "Scripts/python.exe" (forward slash),
which Path::join appends verbatim, so the expected interpreter string never
matched the backslashed sys.executable the engine records in its lockfile.
adopt::decide byte-compared them, always returned DifferentInterpreter, and
every relaunch taskkill'd a healthy engine (and its trainer via /T).
- platform.rs: use "Scripts\\python.exe" so the produced path is canonical.
- adopt.rs: compare interpreter and venv_root through same_path(), which folds
'/' to '\\' and lower-cases on Windows (case-insensitive FS) and compares
as-is elsewhere.
- tests: adopt across mixed separator/case spellings; same_path semantics.
Co-authored-by: MervinPraison <[email protected]>
esc() covered only & < > , so a model-authored URL containing a double
quote closed the double-quoted href built in inline() and injected its
own attributes -- id=f made getElementById('f') return the injected <a>
instead of the composer <form>, breaking Enter-to-send. Adding the two
quote characters to the escape class is the whole fix, since inline()
escapes before the link rule runs.
Co-authored-by: MervinPraison <[email protected]>
getHistory() now returns the exported AgentMessage type carrying tool_calls and
tool_call_id, so persisting it is lossless. Adds a validating setHistory() that
rejects unknown roles, orphaned tool_call_ids and non-arrays, strips a leading
system message, and copies on write. The non-stream/no-tools start() path now
sends the full restored history via generateChat instead of dropping it.
Co-authored-by: MervinPraison <[email protected]>
A file that is valid JSON but not an object (the app's own Export emits a
list) reached c.get(...) in list_chats() and raised AttributeError, which
escaped the (OSError, ValueError) handler and closed the connection with no
status -- so /chats, /projects and /search appeared as an engine outage.
Raise ValueError for a non-dict so it surfaces as a corrupt row instead.
Co-authored-by: MervinPraison <[email protected]>
praisonai-ts gained tool_call and tool_result, so the engine's
capabilities.tools goes true and three conformance scenarios move from
"cannot be produced" to produced and passing.
The flag was false for a real reason rather than caution: upstream
EXECUTED tools and never announced them, so a UI rendering rows from a
true flag would have rendered nothing and looked broken. The flag and
the behaviour agree again, which is the only thing the flag ever
claimed.
This also fixes a break the widening would otherwise have caused. The
mapping loop ended in an `else` that treated any unrecognised variant as
a terminal error -- so the first tool_call to arrive would have ended
the turn as a failure. Widening a union is source-breaking for exactly
this shape, and this is the consumer I predicted it would break.
Two things passed through rather than re-derived:
- `ok` comes straight from upstream. Inferring it from a non-empty
output is the defect the protocol comment was written against, and
the tool_failed scenario deliberately carries a non-empty output on
a failed call so a suite that inferred would fail it.
- `seconds: null`, because upstream reports no duration. Null means
unknown, which the view renders differently from zero.
Skipped conformance scenarios: 5 -> 2. The two that remain are the
approval prompts, which ApprovalManager gates upstream but cannot
surface on the event channel.
794 tests, boundaries clean, bundle unchanged at 42.4kB.
The training subsystem kept the live run and history in memory only, so any
engine restart lost the run: /train/runs returned empty, /train/stop had
nothing to stop, and start() launched a second trainer beside a live one --
the OOM that "one GPU runs one job" exists to refuse.
Each run now writes a run.json (state + pid) at spawn, on the RUNNING/STOPPING
transition, and on finish. Trainer.__init__ reloads from disk: a run whose pid
is still alive is adopted (current is set, so start still refuses and stop can
reach it via its pid's process group); an interrupted run is marked failed. The
engine stays stdlib-only.
Adds SurvivingARestart tests: a live run reappears, a second fine-tune is
refused, stop kills the adopted pid, an interrupted run reports failed, and a
finished run is not re-adopted.
Co-authored-by: MervinPraison <[email protected]>
A single OSError on the log write abandoned the reader while the child
was still writing to a PIPE nobody drained. The child blocked on the full
pipe, proc.wait() blocked on the child, run.finish() never ran, and the
run read "running" forever -- refusing every subsequent run for the life
of the engine.
The reader now drains the pipe whatever happens to the log: a log-open or
log-write failure drops the log and keeps consuming stdout, so the run
still ends. Each line is also flushed, so train.log is the live record it
is documented to be rather than 0 bytes until exit.
Co-authored-by: MervinPraison <[email protected]>
The /settings handler wrote the requested launch_at_login value first and
only attached set_launch_at_login's result to the response -- which nothing
reads. Off macOS (and in a macOS checkout) registration always returns
{"enabled": false}, so the toggle rendered on, persisted, and survived
restarts while no login item existed. Now the platform action runs first and
the persisted value is coerced to what actually happened.
Co-authored-by: MervinPraison <[email protected]>
The desktop engine's fetch_url gated on the approved URL, then let urllib
follow any 3xx to any host -- including the engine on loopback -- under
that same approval, so an author-controlled redirect could pull another
chat's transcript, /settings or /logs into the model's context.
Fetch now uses a no-redirect opener that raises on any 3xx, so the honest
error reaches the model and the user can approve the new URL via a second
card. Adds a test standing up a 302 -> secret-body pair and asserting the
secret is not returned.
Co-authored-by: MervinPraison <[email protected]>
When one agent declares tool_timeout and another omits it,
_resolve_uniform_tool_timeout classified the single value as uniform and
wrapped the shared tool dict, silently forcing the undeclared agent's
valid long-running calls to fail with ToolTimeoutError. Now a value is
uniform only when every agent/role declares it with the same value;
otherwise the per-agent resolver is the sole guard, leaving undeclared
agents unwrapped.
Co-authored-by: Mervin Praison <[email protected]>
Key the env-only getOpenAIClient() cache on API key + base URL instead of
mere existence, so a rotated/updated key or a changed base URL is picked up
without restarting the process. Add resetOpenAIClient() for tests and
settings-driven rebuilds. The identity string is never logged.
Co-authored-by: MervinPraison <[email protected]>
The remote dispatch fallback only recognised a dataset given as a bare
string, but the trainer's canonical shape is a list of mappings and it
loads any name/data_files that exists on disk. A config using
`dataset: [{name: ./local.json}]` therefore had its file left behind, and
the remote run failed on a path that existed only on the launching machine.
_local_dataset_in now inspects both forms, mirroring how the trainer
resolves a local file, and returns the first local dataset to ship.
Co-authored-by: Mervin Praison <[email protected]>
The triage bot fixed a real bug of mine: the dispatch compared
`state == "failed"` while status() returns `"failed (exit N)"`, so a remote run
that failed was reported as a success.
My test did not catch it because my fake returned exactly `"failed"` -- and
`"done"` for success, which the runner never returns either. The fake agreed
with the bug, so the test passed and the code was wrong. That is the same
defect as the code it was meant to check, one level up.
The fake now speaks the runner's actual vocabulary -- completed, running,
unknown, `failed (exit N)` -- and a test reads runner.py from disk and asserts
those spellings still exist, so the fake cannot quietly drift from the thing it
stands in for. Deleting `failed (exit` from the runner now fails the dispatch
tests.
Two test-harness repairs alongside it. `monkeypatch.setattr(train_cmd,
"import_code_module", ...)` never intercepted anything: train_llm imports that
name inside the function body, so the patch has to be on
praisonai_train._code_bridge, where the lookup happens. And the vocabulary test
read RemoteRunner through the module -- which the autouse fixture has already
replaced with the fake, so it would have asserted that the fake agrees with
itself.
Three mutations, three caught, including restoring the equality comparison.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
- Detect failed status by prefix: runner.status() returns 'failed (exit N)',
so the equality check reported failed remote runs as success.
- Ship a dataset named only in --config when it is a local file, so the
remote process is not handed a path that exists only locally.
- Resolve and validate the remote block in --dry-run so the preview matches
the real dispatch and bad settings are caught before renting a GPU.
- Reject falsey non-mapping remote values ([], '', false) instead of quietly
treating them as train-locally.
- Assert typer.Exit in the no-remote dispatch test rather than swallowing
every exception; skip the POSIX-only keyring tests on Windows.
Co-authored-by: Mervin Praison <[email protected]>
Track DeliveryOutcome.DELIVERED via a dedicated _delivered_count instead of
inferring delivered_deliveries from success - undelivered. NOT_CONFIGURED and
SUPPRESSED runs are recorded successes but not deliveries, so the derived metric
over-reported them. Applied to sync + async schedulers and _build_stats /
get_stats_sync, with sync + async tests asserting zero delivered for
NOT_CONFIGURED and SUPPRESSED runs.
Co-authored-by: Mervin Praison <[email protected]>
Standalone AgentScheduler recorded a run as success even when delivery of
its result to the chat channel failed: _deliver_result discarded the
delivery bool, on_success fired, and a failed send was only a swallowed
logger.error.
Now _deliver_result returns a typed DeliveryOutcome
(NOT_CONFIGURED/SUPPRESSED/DELIVERED/UNDELIVERED). Both the sync and async
schedulers observe it: an undelivered result is counted, surfaced via the
existing core HookEvent.MESSAGE_UNDELIVERED hook, and routed to on_failure
instead of on_success. get_stats() now exposes delivered/undelivered
counts. Intentional silence stays a recorded non-outcome.
Reuses the existing MESSAGE_UNDELIVERED protocol seam - no new user-facing
knobs or infrastructure.
Co-authored-by: Mervin Praison <[email protected]>
Serialize workflow_run auto-dispatch, skip dispatch when a gate is already active or the queue is saturated, and reuse the guard in pipeline label sync.
Co-authored-by: Cursor <[email protected]>
The AI-SDK tool loop called generateText unconditionally, so on any
non-OpenAI provider `stream: true` silently had no effect once tools
were configured -- and the round's assistant text was DROPPED.
A model that says "Let me check." before calling a tool had that
sentence recorded into `messages` for its own context and never
delivered to the caller. The user watched a tool run with no explanation
of why. Losing no streaming is a downgrade; losing a sentence the model
actually produced is a wrong transcript.
StreamChunk already carries text AND toolCalls, and StreamTextOptions
extends GenerateTextOptions, so one shape covers both and the loop now
streams when streaming is on.
This makes non-OpenAI consistent with OpenAI rather than inventing
behaviour: the OpenAI path has always streamed pre-tool text through
streamChatWithTools. Every real provider implements streamText --
anthropic, google and the ai-sdk backend all do -- so no provider loses
a transport it had.
tools-cross-provider.test.ts needed updating, and the reason matters.
Its guarantee is in its name -- "tools survive on every non-OpenAI
provider" -- but it asserted on generateText specifically, and `stream`
defaults to true. It now asserts on whichever transport carried the
call, which keeps the guarantee without pinning the mechanism. Its two
multi-round cases are pinned to stream:false, because they are about the
LOOP across rounds and that keeps a real generateText case in the suite.
Their mock also had `streamText: jest.fn()`, which returns undefined and
throws on `for await` before reaching any assertion.
1120 tests pass, tsc clean.
The routing fix now sends a bare claude-*/gemini- name to the AI-SDK
backend, but getBackend() never forwarded the constructor's apiKey and the
backend's key gate read only the provider env var. A caller who passed
apiKey (no env var) authenticated on the OpenAI path but was rejected on
the path their model actually takes.
- Agent stores apiKey/baseURL and forwards them via resolveBackend config
- resolveBackend maps the flat apiKey/baseUrl onto providers[providerId]
- validateProviderApiKey accepts an explicit key, gated after reading it
- 2 tests pin the forwarding and the key-gate behaviour
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
Adds a protocol-only SkillAutomation descriptor (schedule/deliver/prompt)
parsed from SKILL.md metadata.praisonai.automation, so an installed skill
can declare when to run and where to deliver. Preserves nested metadata
mappings during parsing and exports SkillAutomation. Materialisation via
the wrapper scheduler's consent-first SuggestionEngine/DeliveryRouter is
left to consumers.
Co-authored-by: Mervin Praison <[email protected]>
- chat_mixin: run input guardrail BEFORE begin_durable_run in _start_stream so a
guardrail-blocked stream never opens/finalizes a durable run as "succeeded"
(parity with chat()/achat()); remove the now-redundant check from
_start_stream_impl.
- tool_execution: a tool that *raised* (flattened error dict tagged
_praison_retryable) again surfaces as ToolExecutionError so the run fails,
while a plain tool-*returned* {"error": ...} still falls through to the LLM.
Restores test_tool_failure_not_relabelled without losing the PR's self-correct
path for shell_tools/tavily_tools-style returned errors.
- agents: scope every token-report field to the team's agents — rebuild
total_interactions and by_model from the per-interaction log, and add
_scoped_recent_interactions so get_detailed_token_report no longer leaks
another concurrent team's activity.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Mervin Praison <[email protected]>
Found by an agent auditing a stale P0 list. It is the most serious thing
in this session, and it is a hole in the gate I added two PRs ago.
`src/mobile.ts` passed the gate. `dist/esm/mobile.js` -- what
package.json actually resolves `praisonai/mobile` to -- failed to bundle
for a browser with TEN unresolved builtins:
Could not resolve "module" @ dist/esm/llm/backend-resolver.js:1
Could not resolve "url" @ dist/esm/llm/backend-resolver.js:2
Could not resolve "path" @ dist/esm/llm/backend-resolver.js:3
... and 7 more
ROOT CAUSE. scripts/esm-shim.js prepended a CJS banner -- static imports
of module, url and path -- to any emitted file whose source uses a bare
`require()`. Three first-party files on the mobile graph do, all of them
LAZY provider loads. The banner turned three lazy requires into three
STATIC Node imports: precisely the failure the gate exists to prevent.
WHY THE GATE MISSED IT. It checked TypeScript sources only, and the
banner does not exist until build:esm has run. So it passed, confidently
and repeatedly, on an input that is not what ships. A gate that reports
OK about the wrong artifact is worse than no gate, because it is
believed.
Two fixes, because either alone leaves something wrong:
- The banner is browser-safe now. A guarded top-level await keeps Node
behaviour identical and lets a bundler defer the import instead of
failing on it; in a browser the catch installs a `require` that
throws only if a lazy provider path is actually taken, which on a
phone it is not.
- The gate checks dist/esm too, and CI builds before gating. Only when
dist/ exists, so the gate still runs on a fresh clone.
One test needed updating and it is worth saying why rather than burying
it: esm-shim-banner.test.ts pinned BANNER_MARKER to
`__praisonCreateRequire(import.meta.url)` -- an internal identifier, not
a behaviour -- so renaming it failed four tests that were not about
naming. The marker is now `const require =`: a banner that stopped
defining require would still fail them.
Verified in Node after the change: dist/esm/mobile.js loads and exports
Agent/getEnv/randomUUID, backend-resolver parses claude-3-5-sonnet-latest
to anthropic, and the lazy require path still resolves --
createProvider('openai') returns an OpenAIProvider.
1136 tests pass. All four entries -- both sources, both built -- report
loadable in a webview.
- Add "local" to the three gateway tokenless gates (config validation,
start_channels, _start_single_channel) so a token-free terminal channel
starts instead of being skipped and marked degraded (Greptile P1 #1).
- Run the blocking stdin readline on a dedicated daemon thread so a pending
TTY read at shutdown can never join-block interpreter exit; cancellation
resolves the awaiting coroutine promptly (Greptile P1 #2).
- Add regression tests: cancelled-read clean exit, daemon reader guard.
Co-authored-by: Mervin Praison <[email protected]>
Found while verifying a stale P0 list -- not on it, and worse than
anything that was.
The Agent constructor carried its own copy of the model-parsing rule,
and it defaulted EVERY slash-less name to OpenAI:
const providerId = this.llm.includes('/') ? ... : 'openai';
So `new Agent({ llm: 'claude-3-5-sonnet-latest', tools: [...] })` made
one OpenAI call and zero Anthropic ones, sending OpenAI-format tools to
the OpenAI endpoint. The tools were not dropped -- the entire request
went to the wrong vendor, which surfaces as model-not-found, or as a
bill nobody can explain.
backend-resolver.parseModelString had always inferred anthropic from a
`claude-` prefix and google from `gemini-`. There were simply two rules
that disagreed and the constructor used the wrong one. Now there is one.
A custom baseURL is a deliberate exception: pointing at an
OpenAI-compatible proxy that serves claude-* is a real deployment, and
prefix inference would route it away from the endpoint the caller
explicitly asked for. An EXPLICIT anthropic/... prefix still wins over a
baseURL, because that is unambiguous.
Verified as a positive control: restoring the old parser fails the two
routing cases and nothing else.
1144 tests pass, tsc clean.
Drop unused mer.vin blog draft files from the monorepo root; they are not referenced by CI, docs, or package code.
Co-authored-by: Cursor <[email protected]>
The LeafStoreDeletion suite fakes `security`/`secret-tool` with #!/bin/sh
scripts, which Windows cannot execute, so set() returned False and three
asserts failed on the windows-latest engine leg. Those stores are the macOS
keychain and Linux libsecret and do not exist on Windows; skip the class
there using the file's existing os.name=='nt' pattern.
Co-authored-by: Mervin Praison <[email protected]>
_build_tools_dict set either _tool_timeout_wrap (uniform) or
_agent_tool_wrap_resolver (heterogeneous) but never cleared the
mutually-exclusive key. Reusing the same generator with a different
timeout layout leaked a prior run's closure into the adapters, applying
an earlier budget to current tools. Now both keys are written every call
(inactive one set to None). Adds a reuse regression test.
Co-authored-by: Mervin Praison <[email protected]>
Address the two P1 gaps raised in review:
- Cached LLM stayed stale: the reasoning_effort setter now updates the
already-materialized LLM instance (which _build_completion_params reads),
not just _llm_init_params, so post-construction changes take effect.
- Session effort was serialized but never written/restored in production:
apply_cli_session_continuity now persists reasoning_effort alongside the
model, and `run` restores it on --continue/--session (mirroring #3685).
Also: thinking_budget setter now routes through the effort setter, and the
effort value is normalised to a canonical graded level so the persisted/
queried value is always provider-portable.
Co-authored-by: Mervin Praison <[email protected]>
The desktop restores the last-used view from localStorage synchronously at
startup. When that is Train, body.training is set and the stylesheet hides
async; when it resolves to setup-needed/failed, firstRun()/failWithSetup()
render the setup wizard or failure banner into #thread -- which is now hidden
behind the Train view. The title bar shows "setup needed" with nothing on
screen to act on: the reported Windows dead end.
Force the Chat view (showEngineGate) when the engine is not ready so the
deliberate Train choice returns once the engine is up. Engine and Rust
supervisor untouched.
Co-authored-by: Mervin Praison <[email protected]>
The async reasoning_steps post-tool branch captured the completion but
never called _record_finish_reason, so a content-filter/refusal/length
cutoff returned after an async reasoning-enabled tool call was silently
collapsed into "completed" instead of surfacing the explicit provider
terminal reason. Mirrors the sync path which already records it.
Co-authored-by: Mervin Praison <[email protected]>
Wrap the two new guardrail run_in_executor offloads with
copy_context_to_callable so a custom guardrail sees the same
contextual state (trace emission, session context) as the sync path,
matching every other run_in_executor call site in the SDK.
Co-authored-by: Mervin Praison <[email protected]>
- Run the blocking stdin readline() in a dedicated single-thread executor and
tear it down with wait=False/cancel_futures on stop(), so cancelling start()
during a pending read never blocks asyncio.run's event-loop teardown.
- Propagate stdout write failures from send_message() instead of swallowing
them, so the shared DeliveryRouter records a real failure (returns False)
rather than a phantom success.
- Correct the startup log: Ctrl-D exits; /stop cancels the current run.
- Add regression tests: write-failure propagation, router failure on broken
stdout, and prompt shutdown while a stdin read is in flight.
Addresses CodeRabbit/Greptile review findings on PR #4463 (fixes#4455).
Co-authored-by: Mervin Praison <[email protected]>
Use the GitHub App token for post-issue CodeRabbit/Qodo kicks, treat kick failures as non-fatal, and recognise triage-bot kick comments for idempotency.
Co-authored-by: Cursor <[email protected]>
- AutoGen: local code execution is now opt-in via config.autogen.code_execution
(default off; Docker on when enabled), warns on use_docker=False, and
human_input_mode defaults to TERMINATE instead of forced NEVER.
- PraisonAIAdapter: injected ACP/LSP centric tools carry the same per-run
tool_timeout guard via a wrap threaded through cli_config.
- agents_generator: heterogeneous per-agent tool_timeout budgets are honoured
per-agent (no silent tightest-wins downgrade); uniform values still wrap once.
- _config_builder / crewai / autogen adapters: thread the per-agent wrap
resolver so every framework honours per-agent budgets (tools stay shared).
Co-authored-by: Mervin Praison <[email protected]>
A standalone scheduled run no longer reports success when channel delivery
of its result fails. `_deliver_result` now returns a typed `DeliveryOutcome`
(NOT_CONFIGURED / SUPPRESSED / DELIVERED / UNDELIVERED) instead of discarding
the delivery bool; the sync and async schedulers fold that outcome into their
accounting so an undelivered result fires `on_failure` (not `on_success`),
bumps an undelivered counter surfaced in `get_stats()`, and best-effort emits
the existing `HookEvent.MESSAGE_UNDELIVERED` seam. Intentional silence stays a
recorded non-outcome.
Co-authored-by: Mervin Praison <[email protected]>
Bridge installed skills to the scheduler by parsing an optional
metadata.praisonai.automation block (schedule/deliver/prompt) into a new
protocol-only SkillAutomation dataclass exposed via SkillProperties.automation.
Preserve nested metadata mappings during parsing so the namespaced block
survives; older parsers ignore it and skills without it get automation=None.
Materialisation stays in the wrapper's existing SuggestionEngine + DeliveryRouter.
Co-authored-by: Mervin Praison <[email protected]>
Malformed JSON or non-object arguments throw before the normal tool_call
emission, so the catch previously emitted an orphan tool_result with no
matching tool_call to pair by callId. Track emission and announce the call
in the catch when the parse/validation path rejected it early.
Co-authored-by: Mervin Praison <[email protected]>
Make the graded reasoning-effort knob live and provider-portable:
- core: add thinking/effort.py resolving off|minimal|low|medium|high to each
provider's native param (OpenAI/xAI reasoning_effort, Anthropic/Gemini
extended-thinking budget; no-op for non-reasoning models).
- llm/llm.py: emit the translated param in _build_completion_params from a new
reasoning_effort setting (with thinking_budget as a backward-compatible alias).
- agent/agent.py: accept reasoning_effort= and revive the previously-dead
thinking_budget alias, threading both into the LLM request pipeline.
- session/store.py: persist reasoning_effort on the session record and restore it
on resume via get_session_reasoning_effort, mirroring model-persist (#3685).
Co-authored-by: Mervin Praison <[email protected]>
Core now inspects the provider finish_reason/refusal signal and records a
distinct, additive terminal reason (content_filtered | refused |
length_truncated) instead of collapsing provider-side blocks into a silent
empty "completed" or a generic "failed".
- run_outcome.py: extend TerminalReason + precedence additively; add a shared
classify_finish_reason() helper and PROVIDER_BLOCK_REASONS.
- llm.py: add _record_finish_reason() and call it at the non-streaming response
capture points on both the sync and async LiteLLM paths (only overrides a
"completed" reason so max_steps stays sticky).
- chat_mixin.py: record the classification on the OpenAI-native path where the
empty-content finish_reason/refusal is already detected; reset the agent-level
reason per turn so a prior block never leaks.
- agent.py: last_stop_reason surfaces the agent-recorded provider block reason.
- execution_mixin.py: _outcome_for_result() threads the provider block reason
into RunOutcome for return_outcome=True callers.
- run.py (wrapper): map the new reasons to exit code 2 + a human-readable
message + a machine-readable reason in --output json, winning over a generic
empty-result failure.
Backward-compatible: unknown/absent finish reasons behave exactly as today.
Co-authored-by: Mervin Praison <[email protected]>
Every launch recorded itself as primary at main() start while the guard
callback -- which runs in the existing primary -- recorded that primary's
pid as secondary, reversing both role/pid pairs (greptile p1). Record
secondary at main() start (honest before the guard decides, own pid) and
upgrade to primary from setup(), which only the first instance reaches.
Co-authored-by: Mervin Praison <[email protected]>
Two async-safety parity gaps where an async code path silently failed to
match its sync sibling's behaviour:
1. Async guardrail validation ran a blocking, synchronous LLM call from
inside async methods, stalling the whole event loop (and every task
under asyncio.gather). _aapply_guardrail_with_retry and arun_task now
offload the sync guardrail chain via loop.run_in_executor, mirroring
async_memory_mixin's _run_memory_in_thread pattern.
2. The async tool-execution path had no circuit breaker (only loop-guard
parity was added by #4005), so a repeatedly-failing tool could be
hammered every turn via achat()/async workflows and the dead
circuit_open retry-skip never fired. _execute_tool_async_impl now
routes the invocation through the same per-agent/per-tool
CircuitBreaker.acall used by the sync path, counting error-dict results
as failures and surfacing circuit_open=True when the breaker opens.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
Deployment infra now lives under src/praisonai-deploy/infra/ and src/praisonai-bot/infra/; the monorepo-root deploy/README.md stub is unused.
Co-authored-by: Cursor <[email protected]>
Add a built-in `local` channel that reads stdin / writes stdout, registered
exactly like the nine remote platforms. It is token-free and owner-trusted,
and joins the same BotSessionManager, identity resolver and DeliveryRouter as
every remote channel — so `BotOS(agent=agent, platforms=["local"])` lets you
converse with a gateway agent from your shell with zero setup, and
`deliver="local:local"` routes through the shared router for cross-channel
continuity.
Co-authored-by: Mervin Praison <[email protected]>
Move the tool_call emission to immediately after argument parsing and
before the function-existence and approval gates, so denied and
unregistered calls no longer produce an orphan tool_result with no
matching invocation to pair by callId. Also reject non-object JSON
arguments (null, array, scalar) so AgentEvent.args honours its declared
Record<string, unknown> contract. Adds a denied-call ordering regression.
Co-authored-by: Mervin Praison <[email protected]>
- _start_stream_impl now runs input-guardrail validation like chat()/achat(),
so start(stream=True)/iter_stream() no longer skip it
- a tool's own {"error": ...} return with no transient/denial marker flows back
as a normal tool result instead of raising a fatal ToolExecutionError, letting
the LLM self-correct
- PraisonAIAgents token summaries filter to the instance's own agent names,
preventing concurrent instances from leaking each other's usage/cost
Co-authored-by: Mervin Praison <[email protected]>
The Windows first-run report was "double-click, nothing happens, no
window, no folder, no logs", read as a missing-AppData crash. But the
shell's window is eager and visible and AppData is only touched after
the webview exists, so an absent data dir cannot cause a pre-webview
exit. The exit 0 is the single-instance guard telling a secondary
launch to hand off to the primary (which is hidden to the tray on
close) and quit -- indistinguishable from the outside from a shell
that died.
Write one line to %TEMP%\PraisonAI-startup.log before the guard runs
and before the window is built, tagging each launch primary/secondary
with its pid. That log exists whether or not %APPDATA%\PraisonAI does,
which is the "no logs to inspect" gap the report named, and it tells a
handed-off secondary apart from a real crash. Pure path/format helpers
live in the core crate with tests; the write is best effort so a shell
never fails to start over a log it could not write.
Co-authored-by: Mervin Praison <[email protected]>
Move the skill to local ~/.cursor/skills so TS routing guidance stays personal rather than tracked in the monorepo.
Co-authored-by: Cursor <[email protected]>
Fine-tuning needs CUDA. A Mac does not have it, and neither does most of the
hardware this desktop app runs on -- so "Start" in the Training tab offered
every one of those users a button that cannot finish. Remote training was not a
missing convenience; it is the only way the feature works for them.
The SSH runner already existed (`praisonai_train.remote`) and was reachable
only from `praisonai-train remote start`. The gap was that nothing else could
ask for it: there was no YAML key, no flag on the command people actually run,
and nothing in the desktop form.
## One fact, three ways of saying it
remote: --remote-host gpubox [Run on: A remote server]
host: gpubox --remote-python /opt/py [SSH host: gpubox]
python: /opt/py --remote-workdir ~/runs [Remote python: /opt/py]
workdir: ~/runs
All three are the same instruction and are declared once, in
`praisonai_train/remote/settings.py`. The flags are named from that table, the
YAML is validated against it, and a test asserts every key has a flag and the
CLI declares exactly those and no others -- because three surfaces over one set
of facts is precisely the arrangement that drifts apart.
Precedence is the one this CLI already uses everywhere: the file is the
baseline, a flag wins, and an absent flag does not erase a file value.
`llm` dispatches on the resolved config, before the heavy import. That ordering
matters: the reason to train elsewhere is that this machine has no torch and no
GPU, so requiring them locally would make the remote path unreachable from
exactly the machines that need it. The desktop needed no new execution path at
all -- it writes the config, and the config decides.
Four things the design had to get right, each with a test:
* The remote block is stripped from the shipped config. Left in, the far side
reads its own config, finds a host, and dispatches again.
* A remote run does not write ./config.yaml into the directory the user
happened to be standing in. That is the local dispatcher's contract, not a
side effect training elsewhere should have.
* SIGINT and SIGTERM stop the run on the other machine. Without that the signal
ends the tail and leaves a rented GPU training a job reported as cancelled --
which is exactly how the desktop Stop button would lie.
* Credentials are refused, and refused *as credentials*: they are also unknown
keys, and telling someone they misspelled `password` invites them to spell it
correctly into a file that gets copied to another machine and printed by
--dry-run.
Host, workdir and python are validated as plain names -- they reach a remote
shell, and `remote.host: "gpu; rm -rf /"` is not a typo.
## Also fixed
`train_llm`'s `dataset` parameter was never unwrapped, and `_v()` recognised
only `OptionInfo` while `dataset` is a `typer.Argument` and produces
`ArgumentInfo`. Called as a plain function -- which its own docstring says is
supported -- it arrived as an ArgumentInfo, which is truthy, and was written
into the resolved config. The YAML dump then raised RepresenterError.
## Tests
30 in praisonai-train (parity, precedence, refusals, dispatch), 24 in the
desktop training suite. Nothing contacts a remote machine: the runner is
replaced, and what is under test is the decision and what gets handed over.
Thirteen mutations, thirteen caught. Four survived the first pass and each was
a test that did not reach its case -- a credential test passing because the
unknown-key check fired first, a dispatch test that never went through the
command, and two default-value tests where the field's own markup already
supplied the default. One survivor was a genuinely equivalent mutation: `get()`
already trims, so the extra `.trim()` calls were removed rather than pinned.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Task 7. AgentEvent carried three variants -- text, finish, error -- so a
consumer of streamEvents could see prose and nothing else. praisonai-ts
executed tools perfectly well and never said so, which meant any UI
built on it had to infer tool activity from the model's own description
of it. Inference is exactly how a tool call that silently failed still
looks like a normal answer.
Two new variants, and `ok` is the point of both. A tool that FAILED with
a message is byte-identical to one that succeeded with a message, so
success can never be read off a non-empty output. All three paths emit
it: success, throw, and an approval-gate denial -- a denied tool did not
run, and rendering it as a success tells the user something happened
that did not.
callId pairs a result to its call. Matching by position holds only while
exactly one call is ever in flight and silently attributes the wrong
output the moment that stops being true.
Structured events share the SAME queue as the text, which is what keeps
a tool call in its true position. On a separate channel it could arrive
before the sentence that introduced it, and a reader cannot tell a
reordered transcript from a model that said things in that order.
ADDITIVE BY CONSTRUCTION. `start()` gained a fifth optional parameter,
so every existing caller passes four and behaves exactly as before.
`stream()` filters on type === 'text' and is unaffected -- asserted, not
assumed. A run with no tools emits no tool events, which is the pair
that stops a spurious emit from satisfying every other case.
Verified by mutation: reporting a failed tool as ok fails 1 case,
never emitting tool_call fails 3.
One test bug worth noting rather than hiding: two cases first failed
because I passed inline arrow functions as tools. An anonymous arrow
registers under the name "", so the mocked call never matched and was
rejected before any event -- which is correct behaviour, and a small
piece of evidence that these events fire only for tools that really run.
1110 tests pass, tsc clean, build emits 290 files, both entries still
report webview-loadable.
Task 5. `praisonai` resolves to src/index.ts, which re-exports the CLI,
the MCP server, the tool registry and the knowledge store -- modules
that read the filesystem, spawn processes and open sockets. A phone
loads none of them, and a bundler cannot tell: it follows every
re-export, pulls the whole graph in, and the build dies on a Node
builtin nothing was ever going to call.
src/mobile.ts is an allowlist rather than a convenience. Everything it
names is checked by scripts/webview-gate.mjs, which now gates both
entries and fails the build if either gains an import-time Node
dependency. The rule for adding to it is written in the file: if it
cannot run in a browser it does not belong there, even if it would be
useful -- a consumer reaching for something absent gets a resolution
error at build time, which beats a blank screen on a device at import
time.
praisonai/mobile, minified: 77.8kB (target was <400kB)
praisonai, root entry: 2.97MB
The export is placed BEFORE './*' in the map so it wins the match.
TASK 2 IS A CORRECTION RATHER THAN A CHANGE. The list called node-fetch
a phantom dependency to delete. It is not a dependency at all -- it sits
in `overrides`, pinning a TRANSITIVE dep's version, most likely for a
security or compatibility fix. Removing it would change resolution for
whatever pulls node-fetch in, which is the opposite of harmless. Left
alone, and recorded here so it is not "fixed" later by someone reading
the same list. dotenv is genuinely gone: absent from every dependency
section and referenced nowhere in src.
Nothing was removed from package.json; the diff is purely additive.
1103 tests pass, tsc clean, and the build emits dist/mobile.js,
dist/esm/mobile.js and dist/mobile.d.ts.
Task 1 of the mobile-consumable list. PR #4438 replaced the Node
`crypto` import in the two files on the Agent graph and left the
identical import in 32 others -- every one of which re-breaks a webview
bundle the moment anything reachable from it is touched. All 32 were
byte-identical: `import { randomUUID } from 'crypto'`.
They now import src/utils/uuid, and the two local copies #4438 left
behind are gone with them, so there is one implementation rather than
three drifting ones.
The tests force the fallback, which is otherwise unreachable:
globalThis.crypto.randomUUID exists in every environment the suite runs
in, so that path would stay green however broken it was -- and it is the
path that exists for older webviews, where nobody is watching a console.
Verified by mutation, four for four:
version nibble ORed without masking -> 3 fail (0xff reads as version f)
variant nibble ORed without masking -> 1 fail
hex byte not zero-padded -> 4 fail
never using the platform generator -> 1 fail
Worth recording: the first mutation run reported the two mask mutations
as SURVIVING. They had not -- jest was serving a cached transform. With
--no-cache both fail. A mutation run against a cached transform is a
mutation run that proves nothing, and it reports the reassuring answer.
Existing behaviour unchanged: 1096 tests pass, tsc clean, cjs+esm build
produces 289 files, and the webview gate still reports the Agent entry
loadable.
An audit went looking for the pattern this codebase keeps producing -- a check
that something was attempted rather than that it worked -- and found two of my
own recent fixes were exactly that.
## The "no output" fix only worked when the stream was completely empty
_drain_tools() empties the queue and returns how many events it emitted. It is
called in two places: inside the streaming loop, and again on the failure path.
The in-loop call discarded its return value, so by the time the tally ran the
queue was already empty and tools_shown was zero. The no_answer branch never
fired, and the user got "the engine produced no output" printed directly
beneath the tool cards it had just been shown.
One frame of anything is enough to take that path -- a reasoning frame, an
empty text chunk. The only case that worked was a generator that yielded
literally nothing, and that is the only case the tests covered:
chunks=() -> no_answer (tested)
chunks=[reasoning frame] -> empty (not tested)
chunks=[""] -> empty (not tested)
The fix is one `tools_shown +=`. The tests now drive the loop body.
## Deleting a key still reported success
FallbackSecretStore was hardened so a delete has to succeed in every store the
secret could be read from. That layer is right, and it was only ever tested
against a fake -- so nobody looked underneath, where both real stores returned
True whatever happened.
Both had the same shape: the write path passes check=True, the delete path did
not. A locked keychain leaves the item intact and `security` exits 51; a Linux
session with no D-Bus cannot reach the collection at all. In both cases the app
said the key was removed, unset it for the running process so it looked gone,
and read it back on the next launch.
Exit 44 from `security` means "no such item", which is a delete that has
already happened, so it stays a success. `secret-tool clear` exits 0 whether or
not it matched, so any non-zero status is a real failure.
The tests put a fake `security` / `secret-tool` on PATH and exercise the real
KeychainSecretStore and SecretToolSecretStore. No keychain is touched.
Six mutations, six caught. Both of these fixes were mutation-tested when I
first wrote them -- against mutations of the layer I had just edited, which is
how a fix passes its own tests while the thing underneath it stays broken.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
PR #4438 made praisonai-ts loadable in a webview by removing the static
`crypto` and `events` imports from the Agent graph. Nothing stops them
coming back.
That gap is structural, not an oversight. `npm test` runs in Node, where
every one of these imports works perfectly -- so no unit test can catch
the regression. A single `import { x } from "fs"` added to any file the
Agent happens to reach would silently un-ship the mobile app, and the
first signal would be a blank screen on a device with no console.
The gate bundles src/agent/simple.ts for a browser target and fails on
any Node builtin reachable at IMPORT time.
STATIC vs DYNAMIC is distinguished deliberately. A static import is
evaluated at module load and kills the bundle outright; a dynamic
`await import()` inside a function only fails if that function is called
-- readline is reached only from createCLIApprovalPrompt, which is
CLI-only by name and contract. Failing on it would force a shim for a
code path a phone cannot take, which is how a gate gets worked around
rather than fixed. Lazy builtins are still printed on every run.
Verified as a positive control: re-adding `import { randomUUID } from
'crypto'` to simple.ts fails the gate with the builtin named and #4437
cited. Without that check this would be a gate whose state on real code
nobody knows.
NOTHING RUNTIME CHANGED. The diff is one new script, one new workflow,
and two lines of package.json. src/ is untouched, and the existing suite
is unaffected: 1096 tests pass, tsc clean, and the cjs+esm build still
produces its 288 files.
The workflow is path-filtered to praisonai-ts so it cannot affect any
other job.
Two bugs the user hit, with one thing in common: the app knew the answer and
did not use it.
## "No output" on a turn whose tools all ran
The desktop engine does not run the praisonaiagents in this repo. It runs the
one installed in the app's venv -- published 1.7.1 -- and in that version a
tool-using turn ends the stream without ever asking the model for its answer:
_start_stream_impl in 1.7.1: 1 x chat.completions.create
the same function in HEAD: 2 x chat.completions.create
So the generator yields nothing, `streamed` stays False (engine/server.py:1797,
set only in the text-delta branch at :1829), and the engine reports "the engine
produced no output" for a turn whose tools every one succeeded.
The library fix already existed here -- commit 06dfc8e59, 25 Aug -- and could
never reach anyone: pyproject.toml still declared 1.7.1, the same version
already on PyPI, and the desktop installs an unpinned `praisonaiagents` exactly
once, on first run. So the version is now 1.7.2 and the desktop asks for
`praisonaiagents>=1.7.2`. **This needs publishing to PyPI to reach users.**
Why it looked intermittent: whether the model answers in text or calls a tool
is a per-turn decision, and only the first shape streams. Why "Try again"
worked: the failed turn still left the tool result in the cached agent's
history, so the retry answered from it in plain text -- the failure fetched the
thing that made the next attempt succeed.
Three defensive changes so this cannot be silent again:
* The engine now drains tool activity on the failure path too. A turn that ran
tools and then died showed neither the answer nor the tools -- the work
happened and left no trace on screen.
* A turn that ran tools but produced no answer now says so, instead of
reporting the same "no output" as a turn where nothing happened at all.
* The library raises instead of ending a tool turn in silence. Returning
quietly there is indistinguishable from a model with nothing to say, which is
precisely how this reached users.
And the dev escape hatch that was supposed to prevent exactly this class of
confusion did not work: engine/server.py computed the checkout path with a
fixed parents[2], which from the bundled copy under src-tauri/target/<profile>/
resolves to target/praisonai-agents -- a directory that does not exist. Its own
comment says it exists so that "a fix appear[ing] to have no effect" cannot
happen. It searches upward now, and honours PRAISONAI_AGENTS_SOURCE.
## A reopened chat had no history
`grep -c chat_history engine/server.py` returned 0. The conversation lived in
two places and only one of them was durable: the transcript on disk, which the
sidebar renders, and the agent's chat_history, which is what the model is shown.
Nothing ever copied the first into the second.
The agent cache is a plain dict in the engine process, so it is empty after any
restart; save_settings clears it outright; and the tools toggle keys a
*different* agent for the same session. After any of those, reopening a chat
showed the user their whole conversation while the model got a blank slate --
and said so: "I don't have access to your previous questions."
The transcript is now replayed into an agent that has none. Only when it has
none: an agent mid-session already holds those turns, including tool messages
the transcript never stored, and replaying over the top would duplicate them.
Blank turns are skipped -- an empty assistant message is what a failed turn
leaves behind, and feeding it back teaches the model that silence is an answer.
## Tests
engine/test_chat_stream.py is new: the chat path had no test at all, which is
why both of these shipped. Twelve tests, agent stubbed, no provider and no key
required. Eight mutations, eight caught -- including removing the replay,
reversing its order, replaying over an agent that already has history, and
restoring the original "no output" branch.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
96 tests for the a11y and i18n layers, plus the fixes for what building
them exposed in the entry point I had just written.
THE LIVE REGION WAS THE SERIOUS ONE. I had put role="log" and
aria-live="polite" on the whole transcript element -- the same element
reconcile/applyOps mutates on EVERY publish. So a screen reader restarts
on each token batch and no sentence is ever finished, which is worse
than announcing nothing: the app becomes unusable rather than merely
opaque. Clearing it for a new chat announced the wipe, too.
Announcements now go through ui/src/a11y/announce.ts, into two small
regions that change only when there is something worth saying. That
policy coalesces to completed sentences, rate-limits at 1200ms, keeps a
character cursor so nothing repeats, flushes the tail when the turn
ends, never reads reasoning or tool arguments, and lets an approval
bypass the limit -- an approval blocks the run, so waiting politely for
the queue to drain waits for something that will not happen until the
user answers.
The second bug: the message textarea was labelled with the SEND button's
name, so the composer announced as "Send, edit text" -- telling a blind
user what the control beside them does and nothing about the one they
are in. It has its own key now, and the comment says why the two are
distinct.
.sr-only uses the clip-rect idiom rather than display:none, because
display:none and visibility:hidden remove an element from the
accessibility tree entirely -- a live region styled either way is a live
region nothing ever reads.
Also consolidates the duplicate string table I introduced in main.ts
onto ui/src/i18n/strings.ts, and routes dom.ts through it -- including
its hand-rolled `event`/`events` plural, which is wrong in most
languages: Polish has three plural categories, Welsh six, Arabic a zero.
One process note worth recording: my `git add -A` on a shared tree swept
four of the a11y agent's in-progress files into the previous commit,
whose message does not describe them. Content was unaffected.
794 tests, 119 files, bundle 42.4kB of 400kB.
The audit's headline was "every layer is a library; nothing is an
application." This is the file that was missing.
tools/bundle.mjs has defaulted its entry to app/src/main.ts since it was
written, and that file did not exist -- so `npm run build` failed on a
missing file and the webview gate had NEVER run against the real import
graph. Every check was in a known state about fixtures and an unknown
state about the code that would actually ship.
It runs now, and it passes:
bundle: 35.1kB of a 400kB budget, 0 external, 0 problems
tools/app-bundle.test.mjs keeps it that way, and asserts more than "it
built": one real symbol per layer must be present in the unminified
output, because a bundle that tree-shook the app away would pass every
other check -- and no testing/ fake may appear, because a scripted
engine reaching a shipped bundle means the app answers with a fixture on
a real device.
Also landing:
app/index.html carries viewport-fit=cover, which is not optional --
without it env(safe-area-inset-*) is 0px in WKWebView and every inset
collapses to flush, silently, because 0 is the documented safe collapse.
app/app.css uses NO viewport units at all. 100vh includes an address bar
that does not exist here and is wrong through every keyboard transition;
the layout comes from measured pixel geometry instead. The composer's
keyboard offset is padding rather than a transform, so the transcript's
scroll height shrinks with it and the last message stays reachable. The
textarea is 16px because iOS zooms the page when focusing anything
smaller.
app/src/platform.ts is the only file naming both shells, and the probe
is synchronous: an async "is there a native shell" answer would make the
first frame guess, paint wrong, and jump.
app/src/crash.ts is the last line of defence -- there was no onerror and
no unhandledrejection handler anywhere, so a throw in a click handler
left a UI that just stopped responding. It reports rather than swallows,
and draws once however many faults follow, because a crash cascades and
repainting each time pins the CPU.
Strings are injected rather than inlined, so gaining a language does not
touch this file.
698 tests, boundaries clean across 108 files.
The settings screen and the text input, both as pure logic. 45 tests,
each load-bearing rule mutation-checked rather than trusted green.
Two design points worth keeping:
Secret rows are structurally value-less -- SecretRow has no value field
plus `readonly value?: never`, so writing one is a compile error, and a
@ts-expect-error test fails the typecheck if that protection is ever
removed. A runtime `"value" in row` check only proves one build behaved.
Presence has THREE states. hasSecret is async and buildSettings is not,
so an unresolved check renders as UNKNOWN rather than "Not set" --
telling someone their key is missing while it sits in the keychain makes
them paste it again.
Closes the gap that build raised: SettingDef could say "this is a
secret" without saying WHICH keychain slot, so a screen could render
presence and still not know where setSecret should write. The mapping
had to be carried in by whoever built the view, which put the one
identifier that addresses a stored credential outside the registry that
declares it.
secretRefOf refuses both mismatches instead of guessing: a secret def
with no ref has nowhere to write, and a non-secret def with a ref would
route an ordinary setting into the keychain where nothing reads it back.
Guessing a slot from the key name would store a real credential at an
address that is never read, and report success.
684 tests, boundaries clean across 98 files.
A live bug, found by the Tauri planning agent reading both adapters
side by side. The Tauri shell read --safe-area-inset-*; the web shell,
which I wrote, read --sai-*. On a real page one of them was always going
to find "", parse it to 0, and lay content flush against the notch.
It fails silently in the worst way: 0 is the documented safe collapse
for a missing inset, so there is no error, no warning, and nothing in
the console -- just content under the camera.
The conformance suite could not have caught it, and that is the more
interesting part. Both shells pass every shell-contract case because
each is driven through its own harness, so NEITHER ever reads real CSS.
A contract that exercises two implementations through fakes cannot see
them disagree about something only the real environment supplies.
So the names now live once, in the port, beside the type they describe.
There is also a test that gives each shell a stylesheet defining only
the shared names -- which is what a real page has -- and asserts both
read the same insets from it. Verified by mutation: changing one entry
fails two cases across two different shells.
Kept the env() spelling rather than the shorter one. A stylesheet line
that reads `--safe-area-inset-top: env(safe-area-inset-top, 0px)` is
self-evidently correct at a glance, which is worth more than four
characters.
dom.ts rendered approval buttons carrying data-approval-id and
data-choice, and nothing in the package listened for them. The prompt
was drawn, it blocked the run, and it could not be answered -- the run
then hangs until the engine's timeout with a correct-looking screen,
which is the worst class of bug here.
The mapping is a pure function over an attribute bag rather than logic
inside a listener, because the cases that matter cannot be tested well
any other way: a tap that lands on the label INSIDE a button (which is
what every real tap looks like -- the finger hits the text, not the
control), a tap on a control that went disabled mid-decision, an id that
went missing from a row.
It walks the ancestor chain outward, which is what every real toolkit
does and what an event.target check gets wrong on exactly the taps users
make most.
Three refusals are deliberate and each is tested with its pair:
- a disabled control does nothing, and a disabled ANCESTOR blocks a
tap on its child -- otherwise the walk finds something actionable
further out, which is worse than doing nothing.
- an unrecognised choice is refused rather than coerced. The run stays
blocked either way, but a refusal is visible.
- an unknown action stops the walk instead of resolving to the
enclosing row's action, which is how a tap on an unfamiliar control
would delete a chat.
Verified by mutation, three for three.
634 tests.
The audit's sharpest structural point: every port, conformance suite and
view model existed, and nothing declared what the app actually OFFERS.
SettingDef[] was an injected parameter only a test ever supplied, and no
code path constructed a real engine, so no live answer was possible.
Both registries live in app/ because they are the only place that names
a concrete engine or a concrete setting -- which is exactly what keeps
every layer below unable to tell one engine from another.
An engine whose prerequisites are absent is OMITTED rather than offered
and then failed: a picker listing a choice that cannot work is a support
ticket, and selectEngine already reports "no engine X; available: ..."
which is the honest message. So praisonai-ts appears only when a factory
is injected -- praisonai cannot be a dependency until it is bundleable
for a webview (#4437), and the remote engine needs nothing injected and
works today.
The tests assert against the REAL registry rather than a fixture, which
is the only way the "apiKey is secret" case means anything: a def that
forgot secret:true would put a live key in a plain settings file, and a
fixture cannot catch that.
Also: the base URL loses a trailing slash. `${baseUrl}/v1/run` against
"http://host/" produces a double slash, which some servers 404 and
others silently redirect -- losing the POST body.
618 tests.
The controller hardcoded chatId: "current", so every turn from every
conversation carried the same literal and an engine had no way to tell
them apart. Switching chats mid-stream is the case that matters: the
answer still in flight belongs to the chat it started in.
markIdle was flagged by the audit as violating its own documented
contract -- "a turn finished AND was persisted" -- because the
controller calls it unconditionally. Traced it rather than changing it,
and the code is right while the doc was imprecise:
- persistence happens BEFORE `end` is emitted, so a successful turn is
already on disk by the time the finally block runs.
- a cancelled or errored turn is deliberately not persisted, and
idling anyway is required, not a bug: the alternative is a queue
that stays busy forever after one failure and deadlocks every prompt
behind it.
- nothing is at risk, because indices come from stored messages only,
so the next turn numbers from what is really there --
session.test.ts asserts exactly that.
Documented the reasoning where the next reader will hit it.
An audit found the keychain port completely inert: SettingsFacade had
no way to enumerate keys, no change notification, and -- the part that
mattered -- no way to WRITE a secret at all. set() refuses any
secret-flagged def and setSecret was not on the facade, so there was no
reachable code path in the package that stored a key. A fresh install
had no way to become usable.
The facade now carries defs(), subscribe(), setSecret() and
clearSecret(). It still has no getter, and that asymmetry is the design:
a view can configure a key and prove one is present without ever being
able to fault the value into a render tree, a log, or a screenshot.
SettingDef gained label, help, section and choices, so a screen renders
a control from data instead of hard-coding the key list that already
exists as data -- the two drift the first time a setting is added.
subscribe fires only after an ACCEPTED write. Notifying on a refused one
makes a screen redraw a value the user did not manage to change, which
reads as the setting having been accepted. Secret writes notify too, or
a row keeps saying "not set" after a key is stored.
619 tests.
ChatRepository was fully tested and called by NOTHING. Every turn
streamed, rendered, and evaporated. The hole was structural rather than
an oversight: TurnState is assistant-only, StoredChat has both roles,
and no layer owned the join -- docs/gaps.md #4.
createSession owns it, and it is what engines call to record a turn.
That placement is the point: end.userIndex is authoritative and cannot
be computed by a client, because a cancelled or errored turn stays on
screen and is never written, so screen position and disk position
diverge immediately. The indices now come from whatever actually did
the write, and null means the write failed -- which is what withholds
Fork and Delete rather than offering them against a message that is not
there.
The user's message is written when the turn SUCCEEDS, not when send was
pressed, so an abandoned turn cannot leave a dangling prompt with no
reply beneath it in a stored transcript.
A failed write also does not advance the in-memory chat, or the next
turn's indices are computed against messages that were never stored and
every index after it is wrong.
Verified by mutation, three for three: an off-by-one index fails 2
cases, keeping a failed write fails 10, dropping the save fails 4.
596 tests.
Two pieces the package needed before it could have a UI at all.
ui/src/render/reconcile.ts is the diff, and it is pure. A streaming
answer publishes many times a second; rebuilding the transcript each
time throws away every node, loses selection, and forces layout on the
whole list to repaint one character. The tests assert OPERATIONS rather
than outcomes, because a suite that only checks "the right rows are
present" passes just as well against the rebuild-everything renderer
this exists to prevent -- and one case measures it: 200 publishes of a
growing paragraph cost exactly 200 ops.
Verified by mutation. Always-update fails 4 cases; comparing raw
previous indices instead of surviving order fails 3 -- that second one
is the bug where inserting a row reports a move for everything below it.
The exhaustiveness check in signatureOf immediately caught two row kinds
I had missed (error, dropped), which is the whole reason it is written
as a never-assignment rather than a default case.
adapters/src/web/shell.ts makes "runs end-to-end in a browser, no
Tauri" true. It is honest about the platform: no haptics, share falls
back to the clipboard, and the keyboard comes from visualViewport --
the only browser API that reports a software keyboard, clamped at 0
because overscroll can make it briefly negative.
It reports all THREE lifecycle phases, from three different signals.
visibilitychange alone cannot express "inactive", and collapsing that
into "background" would flush and stop the render loop every time the
user pulled down a notification. The contract caught that; I had
collapsed it at first.
Back is wired to popstate, which is a browser's Back, with the same
last-registered-first stack every other shell uses.
All three shells -- fake, Tauri, web -- now pass one contract.
580 tests, boundaries clean across 85 files.
docs/gaps.md #1. TurnState had text as one accumulated string and tools
as a parallel array, so a view could render all the prose and then all
the tools -- and nothing else. That is not how any chat UI shows a
multi-step turn, and it is not something a renderer can work around.
TurnState now carries blocks: the turn in the order it happened. A delta
extends the open text block or starts a new one; a tool_call appends a
tool block; a tool_result for a call never seen gets a position too, or
its row exists in tools and appears nowhere on screen -- the same silent
loss the reducer creates that row to avoid.
The invariant that stops two representations of one thing drifting is
asserted rather than hoped for: every text block concatenated reproduces
text exactly. Both directions verified by mutation -- every delta
opening its own block fails one case (and would be thousands of nodes
per answer), a tool_call adding no block fails three.
buildTranscript walks blocks now, and only the LAST text block streams:
an earlier one was closed by the tool call after it, and a caret on it
would claim two places are being written at once. Row ids are distinct
per block, because two rows sharing an id is how a keyed renderer paints
the second paragraph into the first one's node.
534 tests.
The upstream-parity job failed twice. First on npm ci -- praisonai-ts
gitignores its lockfile. Then on npm install, which fails there with an
ERESOLVE peer conflict (bedrock-agentcore -> @strands-agents/sdk). The
package's dependencies simply do not install cleanly.
That turned out to be the wrong problem to solve. Every member the check
looks at -- streamEvents, lastStopReason and their types -- is declared
locally in simple.ts, so unresolved third-party imports degrade only
unrelated members. The check needs typescript, which THIS package has,
and nothing else.
Asserted rather than assumed, across all four combinations:
deps=present api=correct -> passes
deps=present api=drifted -> FAILS
deps=absent api=correct -> passes
deps=absent api=drifted -> FAILS <- the one that matters
Without that last row this would have been a check that always passes
once the install is gone, which is worse than deleting it.
tsc now runs from praisonai-mobile and only errors AT the generated
parity file count as drift. An assignability failure always reports at
the assignment site, so nothing real is lost -- while TS2307 and TS7006
noise from praisonai-ts's own sources would otherwise fail every run.
The job is also now cheap: one cached npm ci instead of installing a
large dependency tree that could not install anyway.
The upstream-parity job failed on its first run in 10s. praisonai-ts
deliberately gitignores package-lock.json (src/praisonai-ts/.gitignore:6),
so 'npm ci' fails outright -- it exists precisely to install FROM a
lockfile. 'npm install' is what the repo already uses for that package
elsewhere (claude.yml:345).
Also hardens the check itself. tsc has to resolve praisonai-ts's own
imports to typecheck its Agent, so with no node_modules it cannot run at
all. It now SKIPS loudly and names the command to fix it, rather than
failing -- reporting a missing install as upstream drift would send
someone looking in agent-api.ts for a problem that is not there.
Both paths verified by moving node_modules aside: absent -> skip with
the fix printed, exit 0; present -> the real check runs.
The two check jobs passed on their first run, on both node 22.18 and 24
-- so the floor declared in package.json is real rather than a guess.
No workflow referenced praisonai-mobile, so its 523 tests would never
have run on a PR. That is not hypothetical here: praisonai-desktop's
suite existed and ran nowhere for months for exactly this reason, and
desktop.yml opens by saying so.
Four gates, catching different things:
typecheck erasableSyntaxOnly. node:test STRIPS types rather than
compiling, so an enum or a constructor parameter property
is a runtime parse error at import -- this turns that into
a typecheck failure at the file that caused it.
boundaries the two seams. Only engines/src/praisonai-ts may import
'praisonai', only adapters/src/tauri may import
@tauri-apps. A rule enforced only by review stops being
enforced.
test the suite.
test:bundle the gate's OWN fixtures, which prove each check can fail.
A build check that has only ever seen clean input is a
check nobody knows the state of.
The bundle gate is the one a normal test run cannot do: node:test runs
in Node, where a 'crypto' import and a top-level process.env both work
and both kill a webview bundle at import time.
Node matrix is 22.18 (the floor declared in package.json -- tested
because a declared floor nobody runs is a guess; 22.18.0 is where type
stripping became unflagged) and 24 (the forward check).
upstream-parity is its own job so drift from the real Agent is visible
in the checks list rather than buried in another job's log.
All five steps verified locally against the real package.
The fix is right, but it swaps two battle-tested Node implementations
for hand-written ones and adds no tests. Both replacements sit on paths
CI cannot reach on its own:
randomUUID's fallback is dead code in every environment these tests run
in, because globalThis.crypto.randomUUID always exists there -- so it
would stay green no matter how broken it was. It is also the path that
matters, since it exists for older webviews, where nobody is watching a
console. The version and variant nibbles are set by hand, and an
all-0xff and an all-0x00 source are the inputs that expose a bad mask.
SimpleEventEmitter's risky parts are not the obvious methods. They are
once-unwrapping inside off -- without it a caller's unsubscribe silently
never matches, and the listener keeps firing afterwards -- and mutation
during emit, where splicing a live array shifts every later index down
and skips the next listener while still reporting success.
Verified non-vacuous by mutation, four for four:
emit iterates the live array (no slice copy) -> 3 fail
off compares by identity only -> 1 fail
version nibble ORed without masking -> 2 fail
hex byte not zero-padded -> 3 fail
21 tests. Full suite 1082 passing, tsc clean.
.gitignore has `src/*/build/`, which is right for build OUTPUT and
wrong for build TOOLING. So the previous commit's message described a
bundle.mjs that existed only on my disk. Moved to tools/, next to the
boundary checker, which is tracked and is where it belonged anyway.
Also refines the gate itself, after verifying PR #4438 against it
exposed a limitation: it treated every Node builtin alike, so a lazy
import no mobile path can reach failed the build exactly like an
import-time one. That is the difference between a blank screen and a
feature that is merely unavailable -- readline is reached only from
createCLIApprovalPrompt, which is CLI-only by name and contract.
Failing on it would force a shim for a code path a phone cannot take,
which is how a gate gets worked around rather than fixed. Lazy builtins
are still printed every build. Static wins when both kinds appear.
Against the real praisonai-ts:
main builtins=[crypto, events, readline] problems=1
PR #4438 builtins=[readline] problems=0
523 tests, boundaries clean.
Step 19. createApp() takes its adapters INJECTED rather than building
them, so the whole boot sequence runs under node:test against fakes. A
composition root that constructs its own dependencies is the one part of
an app that can never be tested, and it is also where ordering bugs live
-- every case in boot.test.ts is an ordering or a teardown question.
Ordering that is asserted rather than assumed: settings load before the
engine is built (the engine choice and its credentials come from there);
dispose unsubscribes from the shell BEFORE disposing the engine, or a
lifecycle event arriving mid-teardown calls stop() on a disposed engine.
app/src/engines.ts is the only file that knows more than one engine
exists. It returns a result rather than throwing -- an unusable engine
should land the user on a settings screen naming which and why, not on a
crash handler -- and it disposes an engine it rejects, since one left
holding a socket only ever shows up as a second, unrelated failure.
Two of my own tests had the wrong premise and the suite corrected me:
checkProtocol deliberately ACCEPTS a newer engine, because decode.ts
degrades an unrecognised event rather than dropping it, so a v3 client
against a v4 engine loses an affordance and not the conversation. The
real failures are too_old and unreadable, and there is now a pair
asserting the newer case is accepted.
Also lifts fakeTime out of controller.test.ts into testing/src/fake-time
-- it was private there, so any other test needing a TimePort either
rewrote it or reached for real timers, which is how a pacing suite goes
flaky on a loaded CI box.
519 tests, boundaries clean across 80 files.
Step 21. Adds adapters/src/tauri (bridge + shell) and a shell contract
run against BOTH the fake and the Tauri adapter so the two cannot drift.
Three things were living in one implementation and are now part of the
contract, because an adapter author reading only the port would ship
without them:
- openExternal's scheme allowlist. In a webview a javascript: URL is
script execution in the app's OWN origin with the user's session,
and the URL routinely comes from a model, a tool result or a pasted
message. The rule was in the Tauri shell only, so the fake accepted
anything and the contract could not assert it. Now in core, obeyed
by both, with an allowlist rather than a blocklist -- the dangerous
set is open-ended and platform specific.
- back handlers are a STACK, last registered first. This was a comment
inside the fake; a second author using a Set has no defined order and
ships a modal that never consumes the gesture.
- insets and keyboard height must be updated BEFORE subscribers run,
since every re-render re-reads the snapshot inside the callback.
keyboardHeightPx is new, and both agents building against this port
asked for it independently: a component mounting while the keyboard is
already up laid out at 0 until the next transition -- one wrong frame,
then a jump. Exactly the reasoning already written for insets.
Verified by mutation rather than assumed. A permissive allowlist fails
9 cases; notifying before snapshotting fails 3. One of my own tests was
vacuous and the mutation run caught it: an uppercase JAVASCRIPT: URL is
refused whether or not the comparison is case-insensitive, so the case
that matters is an uppercase HTTPS: still being OPENED.
504 tests, boundaries clean.
Building the ui layer against core and protocol surfaced nine design
gaps. Two were cheap and are fixed here; the other seven are written
down in docs/gaps.md rather than fixed, because each needs a decision.
ttft sat next to a field called seconds with no unit in its name, so a
renderer had to guess between seconds and milliseconds -- 1000x in a
number the user reads. The TS field is ttftSeconds now; the wire key
stays ttft, translated in decode.ts, which is the one place that
translation is supposed to happen. The all-11-events round-trip test
covers it.
versions and active had no documented meaning, so the UI could not
render them and dropped them. Both documented, including why decode.ts
clamps active into [0, versions-1] instead of trusting it: an
out-of-range index selects nothing and the message renders blank.
484 tests.
Step 18. Framework-free on purpose: every decision is a named exported
function returning a description of what to render, so the React Native
port reimplements the renderer and reuses all of this.
The load-bearing one is approval routing. decisionIdOf() is the only
sanctioned way to get the id that goes back on the wire, and the
two-approval test delivers the prompts in the OPPOSITE order to their
tool rows, so any index-zip crosses rm with curl. Verified by mutation:
returning row.callId instead of row.approvalId fails two tests.
Tool success comes only from tool_result.ok, never inferred from a
non-empty output; a call with no result renders unresolved, never
successful; seconds: null renders as unknown rather than zero.
Fork/Delete are withheld when end.userIndex is null -- and offered at
index 0, which is the falsy-check trap that would hide them on the
first message of every chat.
At the root route the back handler returns false so Android can exit; a
handler that always returns true traps the user inside the app.
79 tests, every 'must not' paired with the case that makes an
always-refuse implementation fail.
Both were written and never read, silenced with void. streamed was
genuinely dead and is gone. disposed now fails a run fast: a disposed
engine that still accepted runs would start a provider request cancel()
can no longer reach, since dispose() has already cleared the live map it
looks in -- so Stop would report false forever.
engine.ts claimed 'a signature change upstream is a typecheck error here
rather than a runtime surprise on a device'. Nothing actually checked
that, so it was a comment. This makes it true.
The check is one assignment -- the real Agent to the structural
PraisonAgent -- and tsc names the diverged member when it fails.
Verified as a positive control: injecting a member the real Agent lacks
produces 'Property renamedUpstream is missing in type Agent'.
Kept out of npm test deliberately: it needs the sibling praisonai-ts
checkout, present in the monorepo and absent in a standalone clone. It
SKIPS loudly there rather than passing quietly, because a check that
silently stops covering anything is worse than no check.
Step 20. build/bundle.mjs fails the build on three things node:test
cannot catch, because node:test runs in Node where all three work:
- a Node builtin surviving into the bundle. Read off the esbuild
metafile rather than grepped from the output, because a grep cannot
tell an import from a string containing the same word -- there is a
test for exactly that innocent case.
- a TOP-LEVEL process.env read. Scoped to depth zero on purpose: a
guarded read inside a function is how the codebase is expected to
read env, and a check that failed on it would be unusable. Both
directions are tested.
- silent size growth, against a deliberately tight 400kB budget.
Pointed at the real praisonai-ts Agent it reports crypto, events and
readline -- two of them static and unconditional, so the failure is at
import time with no error boundary and a blank screen. Filed upstream as
issue #4437. Top-level process reads are now 0 there, which is #4425.
Also adds testing/src/fake-shell.ts: back-gesture handlers are an ARRAY
rather than a Set, because they are a stack and the most recently
registered gets first refusal -- a Set has no defined order, so a modal
would not reliably consume the gesture ahead of the route beneath it.
326 tests.
Steps 15-16. Implements AgentEnginePort on top of praisonai-ts and runs
it through the same conformance suite as remote-http and the scripted
fake: 21/21, with 5 scenarios declared unsupported and printed.
The coupling is declared structurally in agent-api.ts rather than
imported from 'praisonai'. Three consequences: the tests need no
provider, no network and no key; the file IS the list of what swapping
frameworks would cost (one class, three methods, a three-variant union);
and a signature change upstream fails the typecheck here rather than a
device at runtime.
Capabilities are stated against what the engine can REPORT, not what it
does internally -- so tools is false even though praisonai-ts executes
tools, because it never announces them and a UI rendering rows from a
true flag would render nothing. Recorded in docs/gaps.md.
end.userIndex comes from an injected RunPersistence and is null when the
write failed, never a fabricated zero. Deriving it is what shifted every
subsequent Fork onto the wrong message on desktop.
The classifier test caught a real bug: /\bauthentication\b/ does not
match Anthropic's 'authentication_error' because _ is a word character,
so a bad key was classified internal and sent the user to 'try again'
instead of to settings.
311 tests, boundaries clean.
Store the original listener on the once wrapper and match it in off()
so passing the original callback to off() removes the subscription,
mirroring Node EventEmitter semantics.
Co-authored-by: Mervin Praison <[email protected]>
Replace the static `crypto` import in agent/simple.ts and the static
`events` (EventEmitter) import in ai/tool-approval.ts with browser-safe
internal implementations, so `Agent` can be bundled for browser/webview
targets without pulling Node builtins at import time.
- simple.ts: local `randomUUID()` backed by WebCrypto with a Node<19 fallback
- tool-approval.ts: `SimpleEventEmitter` (~20 lines) backing ApprovalManager,
plus WebCrypto-safe `randomUUID()` in place of bare `crypto.randomUUID()`
Public API is unchanged; `readline` stays lazy (CLI-only).
Co-authored-by: MervinPraison <[email protected]>
The interpolation depth-scanner counted braces inside line/block comments
and regex character classes as structural, prematurely closing the ${...}
interpolation and hiding a genuine require()/__dirname that followed it,
so the emitted ESM omitted its createRequire banner (Greptile P1).
The scanner now skips line comments, block comments, and regex literals
(disambiguated from division via the previous significant char), so their
braces no longer affect depth. Adds regression tests for regex-with-brace,
comment-with-brace, and division-not-regex cases.
Co-authored-by: Mervin Praison <[email protected]>
needsCjsBanner() tested /\brequire\b/ against raw file text, matching the
word require (and __dirname/__filename) inside string literals and comments.
24 of 288 emitted ESM files got a createRequire banner they did not need,
each gaining hard imports of module, url and path — e.g. openai.js bannered
solely by the error string "Zod schemas require zod-to-json-schema".
- Add stripStringsAndComments() to blank strings/templates/comments (newlines
preserved) before detection.
- Match real require(...) call expressions and identifier-position
__dirname/__filename (not obj.require).
- Run main() only via require.main === module and export helpers so tests
assert on the actual emit transform.
- Add esm-shim-banner.test.ts (10 cases, both directions).
Bannered ESM files: 24 -> 9 (remaining all have genuine require/__filename).
Co-authored-by: Mervin Praison <[email protected]>
`start()` with no `onToken` defaulted to `process.stdout.write`, which
threw on the FIRST streamed token in a webview -- after the constructor
had already succeeded, so the constructor guard alone did not cover it.
Extracted as `writeTokenToStdout` rather than left inline, so the test
calls the real function instead of re-implementing the guard beside it.
Both directions are asserted: absent stdout must not throw, and a
present stdout must still be written to, so "never write" cannot pass.
Credit to #4435, which spotted this call site.
A Tauri/React-Native webview has no `process`. Fifteen unguarded
`process.env` dereferences — five of them in the `Agent` constructor
itself — threw `ReferenceError: process is not defined` on the first
`new Agent(...)`, so the bundle died at import time on a device while
every Node test passed.
`getEnv()` already existed in llm/openaiClientOptions and already does
the `typeof process !== 'undefined'` guard; these call sites simply were
not using it. Routed all fifteen through it across six files.
The new test deletes `globalThis.process` rather than mocking `getEnv`,
so a newly added raw deref fails it too. Verified as a positive control:
against the pre-fix constructor all four cases fail with the
ReferenceError at simple.ts:244.
Closes#4425
needsCjsBanner() tested /\brequire\b/ against raw file text, matching the word
inside string literals and comments. 24/288 emitted ESM files gained a
createRequire banner plus hard imports of module/url/path they never needed;
the clearest case, llm/providers/openai.js, was banner-ed solely because of the
error string "Zod schemas require zod-to-json-schema" despite having no
require() call. This made the built ESM strictly worse to bundle than source.
Strip strings/comments before detection and match real CJS usage: require(...)
call expressions and __dirname/__filename/module in identifier positions.
Bannered files drop from 24 to 9 (all with genuine __filename/require).
Adds esm-shim-banner.test.ts asserting on the emitted transform output in both
directions: a string/comment/template "require" must NOT get the banner, while a
real require()/__dirname/module.exports must.
Co-authored-by: MervinPraison <[email protected]>
Breaking out of a stream's for-await loop detached the token sink but
never aborted the request, so the provider kept generating and billing.
- Add signal?: AbortSignal to AgentStreamOptions and thread it into start()
- streamEvents owns an AbortController aborted in its finally, so break
(the iterator's return()) stops the upstream request by default
- Set lastStopReason = 'cancelled' on abort, distinct from 'error'
- Pass abortSignal into the post-tool-loop structured-output generateChat
Co-authored-by: MervinPraison <[email protected]>
v4.7.3 shipped with no macOS downloads. Both Mac legs failed, and the gate
added one commit earlier is why -- it looked for the .app, and by the time it
runs there is no .app:
Bundling PraisonAI.app (.../bundle/macos/PraisonAI.app)
Signing with identity "-"
Signing .../PraisonAI.app: replacing existing signature
Bundling PraisonAI_0.1.0_aarch64.dmg
Cleaning .../bundle/macos/PraisonAI.app <- deleted here
So the same run proves two things: the signing fix works exactly as intended,
and the check written to guard it could never have passed.
It now mounts the DMG and verifies the bundle inside. That is also the better
check -- the DMG is the artifact people download, and the one that was broken.
Both directions verified locally against real disk images rather than argued:
the published v4.7.2 DMG fails the gate ("code has no resources but signature
indicates they must be present", Sealed Resources=none), and a properly ad-hoc
signed DMG passes it ("valid on disk", "satisfies its Designated Requirement",
Sealed Resources version=2 rules=13 files=7). A gate that only ever rejects is
not a gate; it needed the positive control as much as the negative one.
While testing, my own harness reported the broken DMG as passing. It piped
codesign to head and read ${PIPESTATUS[0]}, which zsh spells differently, so
the exit status was silently discarded. The workflow does not pipe that command
and was never affected -- but the near miss is worth recording, because a test
harness that swallows the status under test is the same defect as the one being
fixed, one level up.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
- Stop the MCPToolRunner (best-effort join) before raising on init
timeout/error so a blocked handshake does not leak a daemon thread and
its stdio child process across construction retries.
- Only exempt the empty-tools fail-closed check when a filter actually
removed tools: if the server itself advertised zero tools, raise even
when allowed_tools/disabled_tools is set (a filter cannot be blamed for
an already-empty server).
- Add regression tests for filter-plus-empty-server and filter-removes-all.
Co-authored-by: Mervin Praison <[email protected]>
Reserved job verbs (submit/status/result/cancel/list/stream) now win over
os.path.exists() so a stray file named after a verb can't steal the jobs
API. Explicit paths like ./submit still route to the YAML runner. Adds a
regression test for the collision case.
Co-authored-by: Mervin Praison <[email protected]>
- Re-raise KeyboardInterrupt/SystemExit/GeneratorExit in the Chroma client
init and directory-walk BaseException handlers so Ctrl+C / interpreter
shutdown is no longer masked as a backend RuntimeError.
- Point Knowledge.config's default vector_store path at
.praisonai/knowledge/chroma so the primary Agent flow actually uses the
isolated per-project store (matches _default_chroma_path). User-supplied
path still overrides.
- Add regression test for KeyboardInterrupt propagation.
Addresses Greptile & CodeRabbit review feedback on #4376.
Co-authored-by: Mervin Praison <[email protected]>
- Agent.processToolCalls now receives the resolved AbortSignal and refuses
to invoke (side-effecting) tools once aborted; cancellation propagates out
of the loop instead of being buried as a tool error result.
- ai-sdk backend: caller cancellation is terminal (non-retryable CANCELLED)
instead of being misclassified as a retryable TIMEOUT and re-attempted.
- mergeSignals now returns cleanup that detaches fallback abort listeners in
every request finally block, preventing MaxListenersExceededWarning on
Node < 20 with a shared agent-level signal.
- OpenAIService.streamText forwards an optional AbortSignal to the client.
- Tests: Agent tool-loop cancellation (aborted vs. running).
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
The legacy dispatcher routed every `args.command == 'run'` to the async-jobs
API, whose argparse only accepts {submit,status,result,cancel,list,stream}. So
`praisonai run agents.yaml` (and the unified dispatcher's rewrite of
`praisonai agents.yaml` -> `run agents.yaml`) hit the jobs parser and failed
with `invalid choice` (exit 2) instead of running the team.
Detect a YAML/file first token and hand it to the modern Typer agent runner
(commands/run.py `run_main`), which already resolves YAML files via
`_is_yaml_file`. Genuine job verbs (`run submit`, etc.) still reach the jobs
API unchanged.
Co-authored-by: Mervin Praison <[email protected]>
Adds a cooperative cancellation path so a UI Stop button can actually stop
a run and stop token billing. AbortSignal is threaded end-to-end:
- signal?: AbortSignal on GenerateTextOptions/StreamTextOptions/GenerateObjectOptions
- OpenAI provider forwards it via create(..., { signal })
- Anthropic and Google providers forward it via fetch(..., { signal })
- AI-SDK backend MERGES the caller signal with its timeout controller
(was overwriting it) so aborting either stops the request
- Tools registry honours the previously-dead ToolExecutionContext.signal
- OpenAIService + Agent.chat()/start() + SimpleAgentConfig.signal, with an
explicit per-call signal taking precedence over the agent-level default
(mirrors Python cancel_token > interrupt_controller)
Behavioural tests call the code and fail before the fix.
Co-authored-by: Mervin Praison <[email protected]>
A stdio MCP server that times out on init, errors on init, or lists zero
tools previously only printed a warning and returned an empty _tools list.
Agent(tools=mcp) then ran as a tool-less chat and the model hallucinated
answers (n_tools=0). Now MCP raises TimeoutError/RuntimeError instead of
handing back a usable-but-empty object.
Co-authored-by: Mervin Praison <[email protected]>
Default ChromaKnowledgeAdapter persist path was the cwd-relative string
"knowledge_db", which two projects/harnesses can share and which lets
Chroma's rust bindings panic (pyo3 PanicException, a BaseException) and
kill the process on Windows. Now defaults to an absolute per-project
.praisonai/knowledge/chroma, wraps PersistentClient in try/except
BaseException -> RuntimeError, and adds a BaseException guard to the
directory-walk indexer so a native panic no longer unwinds the process.
Co-authored-by: Mervin Praison <[email protected]>
Document TypeScript SDK ownership in the monorepo, add PR review checklists,
and point issue triage at src/praisonai-ts/AGENTS.md so agents review TS changes correctly.
Co-authored-by: Cursor <[email protected]>
CONTRIBUTOR bug reports with # [BUG] in the body but plain titles (e.g. #4374)
were skipped because autoClaude required an actionable title prefix. Now any
bug label or [BUG] marker triggers Claude; enhancement still needs title prefix.
Co-authored-by: Cursor <[email protected]>
Two correctness fixes flagged by Greptile (P1) and CodeRabbit (Major) in the
unified streaming+tools loop:
1. Assistant/tool message pairing: when a response exceeded
maxToolCallsPerTurn, the assistant message stored every declared tool_call
but only the capped subset got tool result messages, causing the next
OpenAI request to 400 on unresolved tool_call_ids. Now the assistant
message lists only the executed subset.
2. response_format + tools mutual exclusivity: tool rounds no longer send the
json_schema response_format (OpenAI treats them as exclusive). The schema is
applied on a final tools-omitted request once the model stops calling tools.
Adds a behavioral test asserting the assistant tool_calls stay 1:1 with tool
results when the per-turn cap truncates.
Co-authored-by: Mervin Praison <[email protected]>
Streaming and tools are no longer mutually exclusive. A streaming agent
can now call tools: text deltas, tool calls and tool results interleave
on one request per round-trip via a new OpenAIService.streamChatWithTools.
- Remove the `this.stream && !this.tools` gates; the tool loop streams
when `stream` is set and blocks otherwise.
- Default maxIterations 5 -> 20 (Python ExecutionConfig.max_iter parity).
- Add maxToolCallsPerTurn (default 10, Python parity).
- Add Agent.lastStopReason ('completed' | 'max_steps' | 'cancelled' |
'error') so a truncated run is distinguishable from a completed one.
- Tests call the code path: a streaming run with tools yields text AND
executes a tool, plus exhaustion sets lastStopReason='max_steps'.
Co-authored-by: Mervin Praison <[email protected]>
Resolved rebase onto main: kept main's new AI SDK backend branches
(tools + outputSchema via generateObject) and applied this PR's
stream->streamEnabled rename + emitToken sink. Added the required `name`
field to the approval-denial tool result so processToolCalls's return
type is satisfied (tsc clean; agent + llm suites green).
Co-authored-by: Mervin Praison <[email protected]>
The version the macOS signing fix ships in. v4.7.2's two DMGs cannot be opened
on any Mac -- the bundle was never signed, so the only signature on it was the
linker's, which declares a resource seal an unsigned bundle does not have, and
macOS reports that as "damaged" rather than as untrusted.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Agent.stream() ignored the finish event, so agents running with
stream:false, tools, or an outputSchema (which emit only a finish event,
never token deltas) yielded no output. Now stream() surfaces finish.text
when no deltas were emitted. Adds a regression test and completes JSDoc
@param/@returns for the streaming APIs.
Co-authored-by: Mervin Praison <[email protected]>
Add Agent.stream() yielding AsyncIterable<string> (mirrors Python's
iter_stream, which yields plain str) plus Agent.streamEvents() for
structured AgentEvent deltas/finish/error. Remove both
process.stdout.write calls in start(); tokens now route through an
internal onToken sink that defaults to stdout only for CLI use, so
non-terminal hosts (browser, webview, React Native, server) can render
answers as they arrive. Cancellation is a single path: breaking the loop
detaches the sink.
Co-authored-by: Mervin Praison <[email protected]>
Regenerate the lockfile importer to match package.json so
`pnpm install --frozen-lockfile` (CI) no longer fails with an
outdated-lockfile error. Removes the dotenv/node-fetch direct-dependency
entries and the now-unused dotenv package/snapshot; node-fetch remains as
a transitive dependency. Addresses Greptile P1 review feedback.
Co-authored-by: Mervin Praison <[email protected]>
Importing the package no longer reads process.env or runs dotenv.config()
at module-initialisation time, so it does not throw in runtimes where
`process` is undefined (webviews/bundles).
- logger.ts: lazy, `typeof process`-guarded `level` getter
- openai.ts: drop module-scope `dotenv.config()` and the dotenv import
- package.json: remove unused `dotenv` and `node-fetch` dependencies
- add behavioral tests that call the code (guarded level read; import
without throwing / no dotenv)
Co-authored-by: Mervin Praison <[email protected]>
Address reviewer findings on the dual ESM+CJS build:
- package.json: the restrictive exports map (only "." + "./package.json")
broke documented deep imports (praisonai/ai, praisonai/tools,
praisonai/integrations/*, praisonai/db/*) with ERR_PACKAGE_PATH_NOT_EXPORTED.
Add "./*" file subpath, "./*/index" and explicit "./ai"/"./tools" dir
entries mapping to both esm and cjs outputs.
- esm-shim.js: tsc leaves explicit relative require('./x') calls in the ESM
output; under dist/esm (type:module) those throw ERR_REQUIRE_ESM on Node <22
(engines: node >=18). Repoint every internal relative require at its mirrored
CommonJS twin under dist/ (resolved from the twin dir so package.json and
directory-index targets also resolve), so createRequire loads real CJS/JSON.
- dual-build.test.ts: always rebuild (npm run build) so tests never validate
stale artifacts; add behavioural tests asserting no relative require resolves
inside dist/esm and that documented deep-import subpaths load under native ESM.
npm test: 1008 passed, 87 skipped, 0 failed. verify:dist: cjs + esm OK.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Mervin Praison <[email protected]>
- Add getEnv() helper reading process.env only when defined, and route all
OpenAI client apiKey lookups through it (openai.ts, providers/openai.ts) so
browser/webview/React-Native hosts no longer throw ReferenceError.
- Guard top-level dotenv.config() behind a process check.
- Honour an explicit dangerouslyAllowBrowser:false even when the base options
already carried the flag (strip it).
- Add JSDoc (@param/@returns/@example) to exported helpers.
- Add regression tests for getEnv and the explicit-false override.
Co-authored-by: Mervin Praison <[email protected]>
The `approval: true` shorthand built a bare ApprovalManager with no handler,
so requestApproval() fell through to a respond()-awaiting Promise that nobody
resolves — blocking every tool call for the full 5-minute timeout before
denying, compounding across calls and exhausting maxIterations.
Wire the existing createCLIApprovalPrompt() as the default handler so the
shorthand prompts interactively (safe by default, hard to misuse). No new
deps or exports. Adds tests asserting a handler is wired and that denial
resolves without hitting the timeout.
Co-authored-by: Mervin Praison <[email protected]>
- processToolCalls now sets `name` on tool-role results so the AI SDK
adapter emits a non-empty toolName; non-OpenAI providers previously
rejected the follow-up request.
- When both tools and outputSchema are configured, the resolved final
answer is produced via generateObject instead of dropping the schema,
matching the OpenAI native path.
- Extend cross-provider tests: assert tool name propagation and the
tools+outputSchema combination.
Co-authored-by: Mervin Praison <[email protected]>
Two regression gaps flagged in review, both places where the release-signing
tests would keep passing after a future change that reintroduces the exact
failure they exist to catch:
- The signingIdentity assertion accepted any truthy value; only "-" is the
ad-hoc identity that makes the shipped "damaged" bundle's signature valid.
Any other value is a Developer ID with no keychain to satisfy it (codesign
fails) or a typo. Pinned to "-".
- The verify-before-upload test checked command presence and ordering but not
that the step is scoped to macOS. Flipping its `if` to skip macOS -- or
dropping it so it runs on Linux where codesign is absent and the step is
silently skipped -- passed every assertion. Now pins `if: runner.os == 'macOS'`
on the codesign step.
Mutation-checked: a bogus identity and a Linux-scoped gate each fail exactly
one test; 15/15 green otherwise.
Co-authored-by: Mervin Praison <[email protected]>
The published package was CommonJS-only with no exports map or module field,
so tsc downleveled every `await import()` into a `require()` that browser/RN
bundlers cannot resolve statically. Add a parallel ESM build plus an exports
map so dynamic import() survives as a lazy chunk.
- package.json: add `module` + `exports` (types/import/require/default);
split build into build:cjs + build:esm; add verify:dist smoke check.
- tsconfig.esm.json: emit esnext modules + declarations to dist/esm.
- scripts/esm-shim.js: post-process dist/esm — rewrite extensionless relative
specifiers to .js / /index.js, prepend a createRequire(import.meta.url)
banner to files using CJS globals, write dist/esm/package.json {type:module}.
Source files and the CJS build are untouched (backward-compatible require()).
- scripts/verify-dist.mjs: assert both entries load Agent (require + import).
- tests/unit/packaging/dual-build.test.ts: behavioural tests — assert emitted
ESM preserves import() (not require()), specifiers are extensioned, and the
ESM entry loads in a native Node child process and constructs an Agent.
Deferred (matches the issue; avoids green-lying): sideEffects:false,
mobile/node subpath allowlists, and a browser-platform CI gate all require the
separately-filed root-barrel split first.
Co-authored-by: Mervin Praison <[email protected]>
The openai SDK throws in any browser-like runtime (webview, Electron
renderer, React Native, browser) unless dangerouslyAllowBrowser is
passed. praisonai-ts passed it at 0 of 7 construction sites.
Add a shared buildOpenAIClientOptions helper that auto-detects
browser-like runtimes (leaving Node behaviour unchanged) and supports
an injectable fetch so hosts can route egress through native code,
keeping the API key out of the JS heap. Wire all 7 sites through it.
Extend ProviderConfig with fetch/dangerouslyAllowBrowser and add
per-agent apiKey/baseURL to SimpleAgentConfig to mirror Python's
per-agent api_key/base_url. Includes a behavioral test that constructs
the real SDK.
Co-authored-by: Mervin Praison <[email protected]>
Adds an optional `approval?: boolean | ApprovalManager` to SimpleAgentConfig
and gates every tool call in Agent.processToolCalls before invocation. Denials
are fed back to the model as the tool result (course correction) instead of
aborting the run. Reuses the existing ai/tool-approval.ts machinery; binds the
decision to the tool_call id (correlation, never positional).
Co-authored-by: MervinPraison <[email protected]>
Non-OpenAI providers silently dropped tools/outputSchema and answered
without them. The AI SDK backend already supports generateText({ tools }),
streamText and generateObject; simple.ts just never passed them through.
Format tools once in a provider-agnostic shape and run the tool-call loop
through the backend, matching what litellm gives the Python SDK. Adds a
cross-provider regression test asserting tools survive on every provider.
Co-authored-by: MervinPraison <[email protected]>
The parity tracker decides a capability exists by matching an exported
symbol name, so an exported-but-unreachable symbol (e.g. ApprovalCallback)
was reported as "DONE" while unusable. Rather than build a heavy
reachability/conformance subsystem, make the reports honest about what
they measure:
- Add a "What this measures / does NOT" note to generated PARITY.md
(TypeScript and Rust): counts are exported names, include barrel
re-exports, and are not a behaviour or reachability check.
- Relabel the markdown status from "DONE"/"TODO" to "exported"/"missing"
so a symbol-presence check is not mistaken for "works".
- Document the same limitation in the TypeScript extractor docstring.
- Add a regression test asserting the disclaimer + relabel are present.
Co-authored-by: MervinPraison <[email protected]>
v4.7.2's DMG fails to open on every Mac with *"PraisonAI is damaged and can't
be opened"*. This is not the ordinary unsigned-app warning, and the
instructions shipped next to it could not have worked.
Nothing signs the bundle. The only signature present is the one rustc's linker
puts on the bare executable:
CodeDirectory flags=0x20002(adhoc,linker-signed)
Sealed Resources=none
Its CodeDirectory declares a resource seal, and an unsigned bundle has none, so
verification fails outright:
code has no resources but signature indicates they must be present
macOS reports an *invalid* signature as "damaged" and an *untrusted* one as
"unidentified developer". Only the second can be bypassed with right-click →
Open, which is what INSTALL.md told people to do.
`signingIdentity: "-"` makes Tauri codesign the bundle with an ad-hoc identity.
Verified against the shipped artifact: signing it turns
code has no resources but signature indicates they must be present
into
valid on disk
satisfies its Designated Requirement
An ad-hoc identity is still not a Developer ID and Gatekeeper still refuses the
app on a double-click -- but it refuses it as untrusted rather than broken,
which is a door with a handle on it.
The release build now runs `codesign --verify --deep --strict` on the .app
before the artifact is attached, so this cannot ship again silently. Tests pin
the identity and the gate, including that the gate runs before the upload
rather than after.
INSTALL.md now distinguishes the two messages instead of conflating them, and
says plainly that v4.7.2 is affected.
Two notes on the tests. One assertion matched the phrase "codesign --verify"
inside the comment that explains the gate, so deleting the command changed
nothing -- comments are stripped now. And a second check for unsealed resources
was dropped rather than pinned: `--verify` already fails on the exact bundle
that shipped, so it was only a second way of saying the same thing, which is
why removing it broke no test.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Make PraisonAIDB the single owner of the agent lifecycle store write hooks
and have PersistenceOrchestrator delegate its store dispatch to it, removing
the duplicated sync/async store plumbing.
- db/adapter.py: add PraisonAIDB._from_stores() to build an adapter over
already-constructed stores (no URL parsing); fix the stale docstring that
falsely claimed it bridged to the orchestrator.
- persistence/orchestrator.py: lazily build an internal PraisonAIDB and
delegate on_message/aon_message + aon_agent_end store dispatch to it, while
keeping the orchestrator-only surface (ConversationMessage return contract,
resume-aware on_agent_start, LRU session cache). Docstring updated.
Public exports, all backends, and the MemoryConfig(db=...) live path unchanged.
Co-authored-by: Mervin Praison <[email protected]>
The desktop engine's "stream protocol v2" comment listed 9 events but the
engine emits 11: approval_request (the human-in-the-loop gate) and
tool_drafting were live on the wire but absent from the spec. The start and
end payloads had also drifted.
- Rewrite the comment to list all 11 events with their real payloads.
- Add StreamProtocolVocabulary tests that parse the comment and every
emit()/_emit_now() call site and assert the sets match both ways, so the
vocabulary can no longer drift without failing CI.
Co-authored-by: Mervin Praison <[email protected]>
Extract the duplicated ChromaDB metadata sanitisation into a single
module-level helper `sanitize_chroma_metadata` in memory/adapters/factories.py.
Both `Memory._sanitize_metadata` and `ChromaMemoryAdapter._sanitize_metadata`
now delegate to it. Public method names/signatures unchanged, behaviour
byte-for-byte preserved. The distinct core.py JSON-based variant is untouched.
Co-authored-by: Mervin Praison <[email protected]>
Canonical TS path stays src/praisonai-ts/; mirror sync direction is
monorepo to praisonai-js, not the reverse.
Co-authored-by: Cursor <[email protected]>
Finding 3 offloads task/knowledge memory writes via asyncio.to_thread, so
parallel task callbacks can now hit the shared in-memory adapter on different
worker threads concurrently. The non-atomic _next_id increment + append +
eviction sequence could produce duplicate memory IDs or lost entries. Guard all
adapter mutations/reads with a re-entrant lock (negligible cost single-threaded).
Addresses Greptile P1 review on PR #4403.
Co-authored-by: Mervin Praison <[email protected]>
Remove praisonai-js from merge-gate FINAL prompt, Claude STEP 3-ALT, and
tsMirrorPathReasons block. Hub issues and fixes stay in the same repo.
Co-authored-by: Cursor <[email protected]>
The engine (windows-latest) CI job failed in test_training.py -- a file
outside this PR's stream-protocol scope -- because _alive() and one inline
assertion used os.kill(pid, 0). That POSIX liveness idiom is broken on
Windows: signal 0 maps to TerminateProcess (it kills rather than queries),
and a dead pid raises OSError(WinError 87) instead of ProcessLookupError.
Route the Windows check through OpenProcess + GetExitCodeProcess, which
only observes, and replace the os.kill-raises assertion with the same
portable _alive helper. POSIX behaviour is unchanged; no production code
touched.
Co-authored-by: Mervin Praison <[email protected]>
Reject --image up front on execution paths that cannot carry a per-invocation
attachment (--agent, --profile/--profile-deep, YAML file, --output actions)
instead of silently discarding it, and reject single --image values containing
a comma (which ImageHandler treats as the multi-image separator) so a valid
path/URL is never fragmented. Adds 5 CLI guard tests.
Co-authored-by: Mervin Praison <[email protected]>
- edit_file: honour replace_all (default False = first occurrence only) so
the previewed diff matches what edit_file actually applies
- acp_edit_file: use its real whole-file contract (filepath + new_content)
instead of requiring old_string/new_string, which never matched and fell
back to the truncated argument summary
- add regression tests for first-only vs replace_all and ACP whole-file diff
Co-authored-by: Mervin Praison <[email protected]>
Address two P1 reviewer findings on PR #4400:
- agent_invoke._swap_llm: also clear _panel_descriptor (not just
_llm_explicit) so Agent._apply_default_llm no longer early-returns for
panel-configured agents. Previously an accepted agent_config.llm override
silently kept running on the panel model. Mirrored in the manual fallback.
- call._validate_public_base: replace prefix-only check with urlparse-based
validation. Reject malformed values with no host and cleartext ws:// to
non-local hosts (which would expose live audio + the one-shot session token
over an unencrypted transport). Localhost ws:// still permitted for dev.
Co-authored-by: Mervin Praison <[email protected]>
_config_write_target() used _find_config_file() which only sees cwd +
global, so with a nearer ancestor config the CLI wrote a lower-precedence
global file that the ancestor overrode on reload. Use the highest-precedence
discovered file instead, and seed set_plugin_enabled() from the target file
only (not the merged view) to avoid persisting inherited global keys.
Co-authored-by: Mervin Praison <[email protected]>
Strengthen the stream-protocol drift guard to also read the event names
returned by _classify_stream_item, the source of the variable forwarded
through emit(event, frame). A new classifier return value now fails the
guard instead of drifting onto the wire unseen (Greptile finding).
Skip test_a_delete_the_fallback_could_not_perform_is_not_reported_as_success
on Windows: its precondition (an unwritable store via chmod(0o500)) is
unreachable there because Windows ignores POSIX mode bits for directory
writability, matching the existing os.name == "nt" skips in this file.
Co-authored-by: Mervin Praison <[email protected]>
- Finding 1: key skill auto-approval by a unique per-instance
_approval_scope_id instead of the display name (which defaults to
"Agent" for every unnamed agent), preventing one agent's skill grant
from leaking to unrelated agents in the same process.
- Finding 2: add a substring fallback in _detect_provider so routed
models (bedrock/anthropic.*, vertex_ai/claude-*, openrouter/anthropic/*,
vertex_ai/gemini-*) get the correct adapter and its streaming guard
instead of falling through to the OpenAI adapter and crashing async
streaming.
- Finding 3: offload the synchronous, network-bound knowledge.search and
store_in_memory calls via asyncio.to_thread so they no longer block the
shared event loop for concurrently running agents/tasks.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Mervin Praison <[email protected]>
Attach a unified diff to the ApprovalRequest context for edit_file,
write_file and apply_patch so the console approval prompt renders the
actual +/- change instead of a truncated argument dump. Non-edit tools
keep the existing argument summary. The diff also travels on the request
context so non-console backends can render or attach it.
Co-authored-by: Mervin Praison <[email protected]>
- call.py §3: build Twilio media-stream URL from PRAISONAI_CALL_PUBLIC_BASE
instead of the client-controlled request.url.hostname (Host header), closing
the SSRF/call-audio-exfiltration path when CALL_SERVER_TOKEN leaks.
- call.py §2: move module-global `tools` + `_pending_stream_sessions` into a
per-app, thread-locked CallAppState on app.state so co-hosted/multi-tenant
build_call_app() instances never share tool schemas or cross-consume tokens.
- agent_invoke.py §1: route agent_config {"llm": ...} overrides through the
SDK's Agent._apply_default_llm invalidator (with a manual-fallback) so the
cached _llm_instance/_unified_dispatcher/_llm_init_params are dropped and the
request actually runs on the requested model.
Co-authored-by: Mervin Praison <[email protected]>
Add a repeatable --image option to `praisonai run` and plumb it to the
existing vision path instead of the hardcoded args.image = None. The core
Agent and the wrapper's handle_direct_prompt/ImageHandler already support
image input (local paths, http(s):// URLs, MIME detection, base64, multi-
image and clear validation errors); this closes the missing CLI surface.
Image runs bypass the warm-runtime fast path and the bare-Agent "actions"
path (neither can carry an attachment) so the vision-capable path handles
them. Text-only runs are unchanged.
Co-authored-by: Mervin Praison <[email protected]>
The praisonai-ts tool-call loop had a hardcoded 5-iteration cap and, on
exhaustion, logged a warning and returned finalResponse — which is ''
when tool calls were still pending. That empty string is
indistinguishable from the model saying nothing.
- Add optional maxIterations config field (default 5).
- Throw on genuine exhaustion (loop still needed another round-trip)
so callers can observe it instead of receiving ''.
Unifying streaming with the tool loop (interleaving text deltas and
tool calls on one stream) depends on the streaming Agent API in #4385
and is intentionally deferred to that work.
Co-authored-by: MervinPraison <[email protected]>
Config resolution previously returned the first config file found, so any
project-local .praisonai/config.* silently shadowed the user-global config,
discarding every global default. Introduce _discover_config_files() (global
first, ancestor project dirs, nearest project last) and a recursive
_deep_merge() (dicts recurse, lists/scalars replace); _load_config() folds
them lowest->highest so a project config selectively overrides globals while
unset keys fall through. A single-file fast path preserves prior behaviour.
Co-authored-by: Mervin Praison <[email protected]>
Make PraisonAIDB the single owner of the agent-lifecycle store-write
hooks and have PersistenceOrchestrator delegate its message-write and
session-end store dispatch to an internal adapter built via the new
PraisonAIDB._from_stores(), removing the duplicated sync/async store
plumbing while preserving the orchestrator's own return contract and
LRU session cache. Fix the drifted cross-referencing docstrings in both
modules. Public exports (PersistenceOrchestrator, wrap_agent_with_
persistence, PersistentAgent, create_persistent_session) and the
MemoryConfig(db=...) live path are unchanged.
Co-authored-by: MervinPraison <[email protected]>
Extract the byte-identical ChromaDB metadata sanitisation logic into a
shared module-level helper `sanitize_chroma_metadata` in
memory/adapters/factories.py. Both Memory._sanitize_metadata and
ChromaMemoryAdapter._sanitize_metadata now delegate to it, preserving
their public method names and behaviour. The distinct core.py JSON-based
variant is left untouched.
Co-authored-by: MervinPraison <[email protected]>
The engine emitted eleven stream events but the comment block listed nine,
omitting approval_request (the human-in-the-loop tool gate) and tool_drafting.
Documented payloads had also drifted from what start/end actually carry.
- Update the comment to list all eleven events with their real payloads.
- Add StreamProtocolVocabulary tests that parse the comment and every emit()
call site, failing on drift in either direction -- so the spec is now a
checked artifact rather than a comment nothing verifies.
Co-authored-by: MervinPraison <[email protected]>
2026-08-26 18:07:11 +00:00
425 changed files with 50366 additions and 2281 deletions
<p>praisonai-mcp is Tier 2e — the heavy MCP server host: full <code>MCPServer</code>, tool/resource/prompt registries, recipe bridge, HTTP-stream auth, and MCP client config management. Depends only on <code>praisonaiagents</code>. v0.0.2. Protocol: MCP 2025-11-25.</p>
<figureclass="wp-block-table"><table><thead><tr><th>Registry</th><th>Commands</th><th>Gate</th></tr></thead><tbody><tr><td>MCP-resident Typer</td><td>list, add, remove, sync, test, serve, doctor</td><td><code>mcp_package_available()</code></td></tr><tr><td>MCPServerCLI host</td><td>serve, list-tools, list-resources, config-generate, tasks, auth</td><td>Direct via <code>praisonai-mcp <cmd></code></td></tr><tr><td>Code-resident light</td><td><code>serve mcp</code></td><td>Always available with code + agents</td></tr><tr><td>Wrapper-resident</td><td>dashboard, schedule, langfuse</td><td><code>wrapper_package_available()</code></td></tr></tbody></table></figure>
<!-- /wp:table -->
<!-- wp:paragraph -->
<p>Standalone <code>praisonai-code</code> hides <code>mcp</code> from <code>--help</code> when <code>praisonai-mcp</code> is not installed. Host commands like <code>praisonai-mcp serve</code> skip the <code>mcp</code> prefix; <code>praisonai mcp serve</code> goes through the Typer group.</p>
<p>Heavy MCP hosting needs only <code>praisonaiagents</code> + <code>praisonai-mcp</code>. Full capability registry (workflows, deploy, extended tools) activates when the wrapper is co-installed via <code>_wrapper_bridge</code>.</p>
<!-- /wp:paragraph -->
<!-- wp:heading -->
<h2class="wp-block-heading">Bridges (no PyPI cycles)</h2>
<p>Wrapper pins <code>praisonai-mcp>=X</code> after mcp is on PyPI. CI: <code>.github/workflows/pypi-release.yml</code>. Import gate: <code>scripts/check_c12_mcp_imports.sh</code>.</p>
<p>praisonai is Tier 3 — the integration wrapper that wires <code>praisonaiagents</code> + five Tier-2 packages (<code>praisonai-code</code>, <code>praisonai-bot</code>, <code>praisonai-train</code>, <code>praisonai-browser</code>, <code>praisonai-mcp</code>) into one install, with dashboard, framework adapters, jobs API, serve orchestration, and backward-compat shims. v4.6.150.</p>
<p>Each Tier-2 package is installable standalone with its own console script. The wrapper pins all of them and preserves every old import path via shims. Use the wrapper when you need several surfaces together — HTTP serve, dashboard, framework YAML, jobs/recipe — without managing tier pins yourself.</p>
<p>Protocols live in <code>praisonaiagents/frameworks/</code>; adapter implementations in wrapper <code>framework_adapters/</code>. Entry-point group: <code>praisonai.framework_adapters</code>.</p>
<figureclass="wp-block-table"><table><thead><tr><th>Package</th><th>Status</th><th>Choose when…</th><th>Standalone install (planned)</th></tr></thead><tbody><tr><td><code>praisonai-sandbox</code></td><td><strong>C13 preferred</strong> (execution goal)</td><td>Isolated agent code execution — Docker/E2B/Modal/Daytona/Sandlock</td><td><code>pip install praisonai-sandbox[docker]</code></td></tr><tr><td><code>praisonai-deploy</code></td><td><strong>C13 alt</strong> (deploy goal)</td><td>Ship <code>agents.yaml</code> to Docker/AWS/GCP/Azure</td><td><code>pip install praisonai-deploy</code></td></tr><tr><td><code>praisonai-replay</code></td><td>Conditional C14+</td><td>Deterministic replay from run ledger / traces</td><td>After product sign-off</td></tr><tr><td><code>praisonai-serve</code></td><td>Deferred C15</td><td>HTTP API hosting without full umbrella (~14k LOC)</td><td>With recipe/jobs decision</td></tr><tr><td><code>praisonai-recipe</code></td><td>Deferred</td><td>NL → agent YAML without jobs UI</td><td>Tied to jobs + serve</td></tr><tr><td><code>praisonai-jobs</code></td><td>Deferred</td><td>Async long-running agent jobs API</td><td>Extract with serve or stay wrapper</td></tr></tbody></table></figure>
<!-- /wp:table -->
<!-- wp:paragraph -->
<p>C13 decision rule: <strong>sandbox</strong> = where code runs; <strong>deploy</strong> = where the agent service lives. Ship one first after product sign-off.</p>
<figureclass="wp-block-table"><table><thead><tr><th>Criterion</th><th>Pass (browser/MCP)</th><th>Fail (example)</th></tr></thead><tbody><tr><td>Named user goal without full <code>praisonai</code></td><td><code>pip install praisonai-browser</code> → automate Chrome</td><td><code>pip install praisonai-persist</code> → ???</td></tr><tr><td>Coherent product boundary</td><td>One Typer group / one console script</td><td>Whole <code>cli/features/</code> orchestration layer</td></tr><tr><td>PyPI deps: <code>praisonaiagents</code> only</td><td>Tier-2 + lazy <code>_wrapper_bridge</code></td><td>Hard <code>import praisonai</code> at module level</td></tr><tr><td>Worth a separate install</td><td>User who never wants dashboard/serve/bots</td><td>Internal docs tooling</td></tr><tr><td>Extraction shape</td><td>C10 single PR (~3–11k LOC)</td><td>Mega-epic without product sign-off</td></tr></tbody></table></figure>
<!-- /wp:table -->
<!-- wp:heading -->
<h2class="wp-block-heading">When to use which tier</h2>
<!-- /wp:heading -->
<!-- wp:table -->
<figureclass="wp-block-table"><table><thead><tr><th>Scenario</th><th>Install</th></tr></thead><tbody><tr><td>Pure Python agents, minimal deps</td><td><code>pip install praisonaiagents</code></td></tr><tr><td>Terminal coding agent, CI, headless</td><td><code>pip install praisonai-code</code></td></tr><tr><td>Telegram/Discord gateway only</td><td><code>pip install "praisonai-bot[gateway,bot]"</code></td></tr><tr><td>Agent training (no GPU)</td><td><code>pip install praisonai-train</code></td></tr><tr><td>LLM fine-tuning (GPU)</td><td><code>pip install "praisonai-train[llm]"</code></td></tr><tr><td>MCP server for Cursor/Claude</td><td><code>pip install "praisonai-mcp[all]"</code></td></tr><tr><td>Browser automation (extension/CDP)</td><td><code>pip install "praisonai-browser[all]"</code></td></tr><tr><td>CrewAI/AutoGen YAML</td><td><code>pip install "praisonai[crewai]"</code></td></tr><tr><td>Full ops + channels + dashboard</td><td><code>pip install "praisonai[claw]"</code></td></tr><tr><td>Everything in one install</td><td><code>pip install praisonai</code></td></tr><tr><td>Library author / embedding</td><td><code>praisonaiagents</code> only</td></tr></tbody></table></figure>
Explains Option B hub intake for PraisonAI TypeScript/JavaScript (npm praisonai):
accept TS issues on MervinPraison/PraisonAI, implement fixes in MervinPraison/praisonai-js,
sync to src/praisonai-ts/. Use when routing issues, implementing TS fixes, sync workflows,
merge-gate TS paths, or when the user mentions praisonai-js, praisonai-ts, npm SDK, or TS hub intake.
---
# praisonai-js hub intake (Option B)
## Model in one sentence
**Users file TypeScript issues on [PraisonAI](https://github.com/MervinPraison/PraisonAI); fixes land in [praisonai-js](https://github.com/MervinPraison/praisonai-js); the monorepo `src/praisonai-ts/` is a sync mirror only.**
Do **not** ask users to re-file on praisonai-js unless they opened a duplicate on the wrong repo.
- [ ] TypeScript / JavaScript (`npm install praisonai`) — fixes land in [praisonai-js](https://github.com/MervinPraison/praisonai-js); you can report here or there
If the issue is TypeScript/JavaScript SDK, also execute_command 'cat src/praisonai-ts/AGENTS.md'.
Fourth, decide routing before planning:
- TypeScript/JavaScript SDK → MervinPraison/praisonai-js: canonical npm repo. Do NOT edit src/praisonai-ts/ in this monorepo. Cross-link upstream issue in external PR body: Fixes MervinPraison/PraisonAI#N.
- TypeScript/JavaScript SDK → src/praisonai-ts/ in this monorepo (read src/praisonai-ts/AGENTS.md).
@@ -308,7 +319,7 @@ function finalClaudeCompletedOnSha(comments, headPushedAt) {
}
constFINAL_CLAUDE_REVIEW_BODY=
'@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify src/praisonai-rust or src/praisonai-ts in this repo — TypeScript SDK work belongs in MervinPraison/praisonai-js. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.\n\n**Phase 1: Review per AGENTS.md**\n1. Protocol-driven: check heavy implementations vs core SDK\n2. Backward compatible: ensure zero feature regressions\n3. Performance: no hot-path regressions\n4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep\n5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.\n6. Repo routing: TypeScript/JavaScript SDK → praisonai-js (not src/praisonai-ts/ here); agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (`praisonai.sandbox` entry point) — request changes if wrongly added to praisonaiagents/\n\n**Phase 2: FIX Valid Issues**\n7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix\n8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback\n9. Push all code fixes directly to THIS branch (do NOT create a new PR)\n10. Comment a summary of exact files modified and what you skipped\n\n**Phase 3: Final Verdict**\n11. If all issues are resolved, approve the PR / close the Issue\n12. If blocking issues remain, request changes / leave clear action items';
'@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.\n\n**MANDATORY READ (before reviewing):**\n- Always read src/praisonai-agents/AGENTS.md\n- If this PR touches src/praisonai-ts/, also read src/praisonai-ts/AGENTS.md §2.1.2 (TS triage + PR review checklist)\n\n**Phase 1: Review per AGENTS.md**\n1. Protocol-driven: check heavy implementations vs core SDK\n2. Backward compatible: ensure zero feature regressions\n3. Performance: no hot-path regressions\n4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep\n5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.\n6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (`praisonai.sandbox` entry point) — request changes if wrongly added to praisonaiagents/\n\n**MANDATORY COMMENT FORMAT — include this Phase 1 table in your review comment:**\n#### Phase 1 — AGENTS.md review\n| Check | Result |\n|---|---|\n| Protocol-driven / no heavy impl in core | ✅ or ❌ + one-line rationale |\n| Backward compatible | ✅ or ❌ + one-line rationale |\n| Performance (hot path) | ✅ or ❌ + one-line rationale |\n| **SDK value** | ✅ or ❌ + one-line rationale (explicitly judge whether the change strengthens the SDK) |\n| No Agent param bloat | ✅ or ❌ + one-line rationale |\n| Repo routing | ✅ or ❌ + one-line rationale |\n\nFor TypeScript PRs (src/praisonai-ts/), also add:\n| TS types / parity / tests | ✅ or ❌ + one-line rationale (npm run build && npm test) |\n\n**Phase 2: FIX Valid Issues**\n7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix\n8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback\n9. Push all code fixes directly to THIS branch (do NOT create a new PR)\n10. Comment a summary of exact files modified and what you skipped\n\n**Phase 3: Final Verdict**\n11. If all issues are resolved, approve the PR / close the Issue\n12. If blocking issues remain, request changes / leave clear action items';
body: '🤖 Auto-triaging with Claude. Analyzing issue and implementing a fix...\n\nClaude will:\n1. Route correctly: TypeScript/JavaScript SDK → praisonai-js; agent tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins\n2. Search codebase for root cause\n3. Implement a minimal fix following AGENTS.md\n4. Create a PR with the changes (external repo PRs link back to this issue)'
body: '🤖 Auto-triaging with Claude. Analyzing issue and implementing a fix...\n\nClaude will:\n1. Route correctly: TypeScript → src/praisonai-ts/; agent tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins\n2. Search codebase for root cause\n3. Implement a minimal fix following AGENTS.md\n4. Create a PR with the changes'
if (await mergeGate.shouldSkipMergeGateDispatch(github, owner, repo, prNumber, core)) {
return;
}
await github.rest.repos.createDispatchEvent({
owner,
repo,
@@ -288,8 +293,8 @@ jobs:
Review checklist:
1. Read AGENTS.md and the PR diff on the current branch.
2. Read all PR comments and reviews (FINAL architecture reviewer / Lead Engineer must have run).
3. Confirm SDK value gate: change strengthens praisonaiagents (no scope creep). The aim is a LIGHTWEIGHT AND POWERFUL package — BLOCK changes that add params/modules/exports duplicating existing Agent capabilities (instructions/backstory/tools/hooks/memory), or add knobs/fields with no live consumer, i.e. scope creep for the sake of it.
4. Repo routing: BLOCK if TypeScript/JavaScript SDK changes in src/praisonai-ts/ (→ praisonai-js + sync workflow); agent-callable tools (→ PraisonAI-Tools); lifecycle plugins (→ PraisonAI-Plugins); or optional sandbox backends (→ PraisonAI-Plugins via `praisonai.sandbox`) were added to praisonaiagents/ instead of the external repo.
3. Confirm SDK value gate: change strengthens the relevant SDK (Python: praisonaiagents/praisonai; TypeScript: src/praisonai-ts/) — no scope creep. The aim is a LIGHTWEIGHT AND POWERFUL package — BLOCK changes that add params/modules/exports duplicating existing Agent capabilities (instructions/backstory/tools/hooks/memory), or add knobs/fields with no live consumer, i.e. scope creep for the sake of it. FINAL architecture review must include the Phase 1 table with an explicit **SDK value** row.
4. Repo routing: BLOCK if agent-callable tools (→ PraisonAI-Tools), lifecycle plugins (→ PraisonAI-Plugins), or optional sandbox backends (→ PraisonAI-Plugins via `praisonai.sandbox`) were added to praisonaiagents/ instead of the external repo.
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. If agent.py adds >100 lines or new Agent __init__ params, respond MERGE_GATE_VERDICT: BLOCK (manual review required).
7. BLOCK if labels security/breaking-change/needs-manual-review/release, sensitive paths (.github/workflows, auth, pyproject.toml), secrets in diff, PR >800 lines or >30 files, or SDK changes without tests. Label `merge-gate-ci-only` exempts CI-only changes under `.github/workflows/`, `.github/actions/`, or merge-gate scripts.
'**Routing:** Agent-callable tools → [PraisonAI-Tools](https://github.com/MervinPraison/PraisonAI-Tools); lifecycle plugins (tracing/hooks/guardrails) → [PraisonAI-Plugins](https://github.com/MervinPraison/PraisonAI-Plugins). TypeScript/JavaScript SDK fixes land in `src/praisonai-ts/` in this repo.'
].join('\n')
});
}
// --- Auto-trigger Claude for owner issues and actionable external bug/enhancement reports ---
// --- Auto-trigger Claude for bug/enhancement reports ---
Read AGENTS.md to understand the architecture rules.
SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or src/praisonai-ts in this repo — TypeScript/JavaScript SDK work belongs in MervinPraison/praisonai-js (STEP 3-ALT).
SCOPE: Python SDK (praisonaiagents, praisonai) and TypeScript SDK (src/praisonai-ts/) live in this monorepo. Do NOT modify src/praisonai-rust. For TS issues read src/praisonai-ts/AGENTS.md.
AIM: This package must stay LIGHTWEIGHT AND POWERFUL. Do a CRITICAL REVIEW at each stage (issue triage, fix planning, PR review/implementation) and reject scope creep for the sake of adding features. Prefer existing Agent capabilities (instructions/backstory/tools/hooks/memory) over new params/modules/exports. Even when fixing a raised issue, keep the fix minimal and lightweight — do NOT expand the surface area or add knobs with no live consumer.
4. Documentation (PraisonAIDocs): Documentation pages, guides, API references.
5. Plugins → MervinPraison/PraisonAI-Plugins: framework lifecycle extensions (tracing, logging, metrics, hooks, guardrails). Not agent tools — use STEP 3-ALT.
6. Sandbox backends (optional/niche, e.g. Capsule) → PraisonAI-Plugins via `praisonai.sandbox` entry point; core keeps protocol/config/manager only.
7. TypeScript/JavaScript SDK → MervinPraison/praisonai-js: canonical npm SDK repo. Do NOT implement in src/praisonai-ts/ here — clone praisonai-js and use STEP 3-ALT. After merge there, run **Sync to PraisonAI Monorepo** workflow to update src/praisonai-ts/.
For items routed to an EXTERNAL repository (Tools, Docs, Plugins, praisonai-js), follow STEP 3-ALT below.
7. TypeScript/JavaScript SDK → src/praisonai-ts/ in this monorepo (not praisonaiagents/).
For items routed to an EXTERNAL repository (Tools, Docs, Plugins), follow STEP 3-ALT below.
- Does it duplicate existing functionality? Check if Agent already supports this via existing params (reflection, planning, tools, hooks, memory). If it does, reject the change as scope creep and prefer the existing capability — the aim is lightweight and powerful, not more knobs.
- Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
- Does it inherit from Agent properly? New agent types MUST inherit Agent, not wrap it with composition.
@@ -331,7 +320,7 @@ jobs:
${{ steps.check_fork.outputs.is_pr == 'true' && (steps.check_fork.outputs.is_fork != 'true' || steps.check_fork.outputs.can_push_fork == 'true') && steps.check_fork.outputs.merge_state != 'DIRTY' && format('STEP 3 — IMPLEMENT ON EXISTING PR (branch: {0}, PR #{1}{2}):\n- DO NOT create a new branch or PR\n- git checkout {0}\n- Implement minimal, focused fixes from reviewer feedback\n- Follow protocol-driven design: protocols in core SDK, heavy implementations in wrapper\n- Keep changes small and backward-compatible\n- Commit: git add -A && git commit -m "fix: <description>"\n- Push: git push origin {0}\n- Comment a summary of exact files modified and what you skipped\n', steps.check_fork.outputs.pr_branch, steps.check_fork.outputs.pr_number, steps.check_fork.outputs.can_push_fork == 'true' && format(', fork: {0}', steps.check_fork.outputs.fork_repo) || '') || '' }}
${{ steps.check_fork.outputs.is_pr != 'true' && 'STEP 3 — IMPLEMENT (for changes in THIS repo — PraisonAI):\n- Create a fix branch: git checkout -b claude/issue-$ISSUE_NUMBER-$(date +%Y%m%d)\n- Implement a minimal, focused fix\n- Follow protocol-driven design: protocols in core SDK, heavy implementations in wrapper\n- Keep changes small and backward-compatible\n- Commit: git add -A && git commit -m "fix: <description> (fixes #$ISSUE_NUMBER)"\n- Push: git push origin claude/issue-$ISSUE_NUMBER-$(date +%Y%m%d)\nCRITICAL: After pushing, you MUST immediately run: gh pr create --title "fix: <title>" --body "Fixes #$ISSUE_NUMBER" --head claude/issue-$ISSUE_NUMBER-$(date +%Y%m%d)\nDo NOT stop after pushing. Do NOT say "manually create a PR". Always create the PR automatically.\n' || '' }}
When work must happen in a different repository, follow these steps EXACTLY. Do NOT attempt to use `cd`, as directory state is not preserved. Use `git -C` for all commands.
h) Comment on THIS PraisonAI issue with the external PR URL. For praisonai-js: note that after merge, a maintainer runs **Sync to PraisonAI Monorepo** on praisonai-js (Actions → sync workflow) to update src/praisonai-ts/.
h) Comment on THIS PraisonAI issue with the external PR URL.
STEP 4 — TEST:
- For SDK changes: cd src/praisonai-agents && PYTHONPATH=. python -m pytest tests/ -x -q --timeout=30
- For praisonai-js: cd /tmp/praisonai-js && npm install && npm run build && npm test
- For Python SDK changes: cd src/praisonai-agents && PYTHONPATH=. python -m pytest tests/ -x -q --timeout=30
- For TypeScript SDK changes: cd src/praisonai-ts && npm install && npm run build && npm test
- For docs changes: verify files exist and are valid markdown/MDX
@@ -7,9 +7,22 @@ You are working on the PraisonAI project.
- Be concise and helpful in responses
- Test implementation thoroughly
- Ensure backward compatibility with existing APIs
- Follow protocol-driven design across the nine packages: core protocols in `praisonaiagents/`, agentic terminal CLI in `praisonai-code/`, bots/gateway in `praisonai-bot/`, LLM fine-tuning + agent training in `praisonai-train/`, browser automation in `praisonai-browser/`, MCP server host in `praisonai-mcp/`, sandbox backends in `praisonai-sandbox/`, deployment in `praisonai-deploy/`, integrations/serve/dashboard in the `praisonai/` wrapper
- Follow protocol-driven design across the nine Python PyPI packages plus the TypeScript SDK: core protocols in `praisonaiagents/`, agentic terminal CLI in `praisonai-code/`, bots/gateway in `praisonai-bot/`, LLM fine-tuning + agent training in `praisonai-train/`, browser automation in `praisonai-browser/`, MCP server host in `praisonai-mcp/`, sandbox backends in `praisonai-sandbox/`, deployment in `praisonai-deploy/`, integrations/serve/dashboard in the `praisonai/` wrapper, TypeScript/JavaScript SDK in `src/praisonai-ts/` (`npm: praisonai`)
- **TypeScript review routing:** For TS/JS issues and PRs, read [`src/praisonai-ts/AGENTS.md`](src/praisonai-ts/AGENTS.md) (not just this file). Canonical source is `src/praisonai-ts/` in this monorepo; [MervinPraison/praisonai-js](https://github.com/MervinPraison/praisonai-js) is the npm mirror only (sync: monorepo → praisonai-js via workflow **Sync to praisonai-js**). Do not implement TS fixes in praisonai-js for PraisonAI issues.
- Preserve old `praisonai.*` import paths via shims when moving code between packages (see §2.3 in `src/praisonai-agents/AGENTS.md`; shim helpers in `src/praisonai/praisonai/cli/_shim.py`)
- Package boundaries and dependency rules: `ARCHITECTURE.md` §2 (Tier 2 packages must never PyPI-depend on the wrapper; cross-tier access goes through lazy `_*_bridge` modules)
- When reviewing a PR or an issue, evaluate whether the change addresses a framework concern or a user goal, and design its surface (params, naming, defaults) accordingly
- The aim of this package is to stay **lightweight and powerful**. Do a critical review at each stage — when triaging an issue, when planning a fix, and when reviewing/implementing a PR. Reject scope creep for the sake of adding features: if a capability already exists (e.g. via existing Agent params like `instructions`/`backstory`/`tools`/`hooks`/`memory`), prefer it over a new API surface. A change must genuinely strengthen the SDK (simpler, more robust, more user-friendly) — do not add knobs, params, modules, or exports that have no live consumer or that merely duplicate existing behaviour.
- The aim of this package is to stay **lightweight and powerful**. Do a critical review at each stage — when triaging an issue, when planning a fix, and when reviewing/implementing a PR. Reject scope creep for the sake of adding features: if a capability already exists (e.g. via existing Agent params like `instructions`/`backstory`/`tools`/`hooks`/`memory`), prefer it over a new API surface. A change must genuinely strengthen the SDK (simpler, more robust, more user-friendly) — do not add knobs, params, modules, or exports that have no live consumer or that merely duplicate existing behaviour.
## Issue and PR routing
| Area | Where to implement | AGENTS.md to read | Tests |
| npm mirror (read-only for fixes) | [praisonai-js](https://github.com/MervinPraison/praisonai-js) | mirror of `src/praisonai-ts/AGENTS.md` | CI on praisonai-js |
You do **not** need to re-file TypeScript issues on praisonai-js — report on PraisonAI and automation routes fixes to the canonical TS repo.
## Guidelines
- Follow existing code style
- Add tests for new features
- Update documentation as needed
- TypeScript changes: implement in [praisonai-js](https://github.com/MervinPraison/praisonai-js), then run **Sync to PraisonAI Monorepo** workflow to update `src/praisonai-ts/`
- TypeScript changes: implement in `src/praisonai-ts/` and run `npm test` there
- After merge, sync the npm mirror: `gh workflow run "Sync to praisonai-js" --repo MervinPraison/PraisonAI`
Report npm/TypeScript vulnerabilities on **PraisonAI** (hub intake) or [praisonai-js](https://github.com/MervinPraison/praisonai-js). Fixes land in praisonai-js first.
Report issues against the **specific package** where the vulnerability exists. A platform-only issue does not necessarily affect the core SDK, and vice versa.
2. Implement fixes in **`src/praisonai-ts/`** in this monorepo (not in `praisonaiagents/`).
3. Verify with `cd src/praisonai-ts && npm install && npm run build && npm test`.
4. After merge, maintainers sync the npm mirror: `gh workflow run "Sync to praisonai-js" --repo MervinPraison/PraisonAI`.
[MervinPraison/praisonai-js](https://github.com/MervinPraison/praisonai-js) is the public npm checkout; direction is **monorepo → praisonai-js** only. Claude merge gate and `@claude` review TS changes under `src/praisonai-ts/` using the TS AGENTS.md criteria (lightweight, type-safe, Python parity where applicable).
worktree:bool=typer.Option(False,"--worktree",help="Run on an isolated git worktree/branch (branch-per-task); no-op when not a git repo"),
keep:bool=typer.Option(False,"--keep",help="With --worktree, keep the worktree/branch after the run for review instead of tearing it down"),
append_system_prompt:Optional[str]=typer.Option(None,"--append-system-prompt",help="Append text (or @file) to the system prompt for this invocation only. Env fallback: PRAISONAI_APPEND_SYSTEM_PROMPT"),
# Multimodal attachment for one-shot runs
image:Optional[List[str]]=typer.Option(None,"--image",help="Attach an image (local path or http(s):// URL) to a one-shot run so the agent can see it. Repeatable."),
):
"""
Runagentsfromafileorprompt.
@@ -1143,6 +1207,8 @@ def run_main(
praisonairun"Add tests"--sessionabc123
praisonairunagents.yaml--interactive
praisonairun"What is 2+2?"--profile
praisonairun"Describe this screenshot"--imagebug.png
Loaded 100 of 425 files, more files were not shown because too many files have changed in this diff.
Show more
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
