mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 00:02:35 +00:00
err-probe-ssh-includes
706
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo Hong
|
4b876758d3 |
refactor(mobile): checked reply readers for the session domain (step 7) (#21089)
* test(mobile): record main's session reply behaviour at every unrecorded read site Step 7 for the session domain changes how 51 RPC readers read a *malformed* reply. Eleven of the session read sites had no recording family, so main's answer to a malformed reply at those sites was undocumented and the reader change would have had nothing to move. This commit is the before picture, taken from main's own tree with no product edit in it. Ten new families, twelve scenarios, twenty-five goldens: - `session.review-file-diff` / `session.review-branch-diff` — `git.diff` and `git.branchDiff` read through the review projection, which the Changes screen's verbatim readers do not cover. - `session.review-git-mutations` — the single-file `git.stage` / `git.discard` and the bulk stage sweep's second `git.stage`. - `session.review-send-sheet` — `session.tabs.list` read for the agent terminals the send sheet lists, the third reader on that method. Needs an `open-send-sheet` action on the review-action adapter, which re-digests that family's eight goldens on `adapterSha256` and nothing else. - `session.browser-tab-create` — `browser.tabCreate`. - `agentSession.structured-create` — `agentSession.create`, whose family base only ever covered the support probe. - `session.tab-rename` / `session.tab-close-session` — `terminal.rename` and `session.tabs.close`. - `settings.new-tab-local-agents` — `preflight.detectAgents`, the arm the new-tab loader takes for a workspace with no connection. `baseline` is repinned to main's tip because two commits (#20659, #21004) touched a fenced path after the pilot's pin, so `--record` refuses on main's own tree until it moves. The repin is what rewrites `baseline` on all 705 existing goldens; nothing else about them moves. Decoded against origin/main through the value pool: 705 header-only (`baseline` on every one, `adapterSha256` on the eight review-action goldens), 0 body-moved, 25 added, 0 deleted. Not covered, with the reason: the chunked clipboard upload's `appendImageUploadChunk`, `commitImageUpload` and `abortImageUpload` cannot be matrixed, because `replyMatrixSites` takes every completion in the base scenario and the chain's later params carry the `uploadId` the start reply named. Driving `clipboard.startImageUpload#1` therefore makes main send an append whose params no scripted step matches, and the recorder raises `Request params mismatch: clipboard.appendImageUploadChunk#1` instead of recording. The two families were written, probed and removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): checked reply readers for the session domain (step 7) Fifty-one unchecked reply readers across nine files become checked zod readers, so a malformed host reply surfaces as one readable error at the operation boundary instead of a downstream `TypeError`, a rendered `undefined`, or a screen left ready over garbage. Deliberately a behaviour change on malformed replies only. Eight schema modules, one per reply family, each recording the consumer line behind every requirement and the host handler that publishes it: - `clipboard-image-reply-schema.ts` — the upload slot's `uploadId`, the commit and single-frame path strings, and the two legs whose body nothing reads. - `github-pr-mutation-reply-schema.ts` — the `{ ok, error }` status envelope as two variants, and the bare-boolean confirmation. - `github-pr-entity-reply-schema.ts` / `github-pr-read-reply-schema.ts` — the seven PR sidebar reads. Every identity requirement the hand parsers had is kept, so a payload that degraded to null still degrades to null; what changes is a payload that is not the declared container at all. - `diff-review-reply-schema.ts` — the normalized branch compare, the review notes on the worktree record, the three file-diff arms, and the file-level git mutations. - `review-terminal-reply-schema.ts`, `session-launch-reply-schema.ts`, `session-read-reply-schema.ts`, `session-write-reply-schema.ts` — the review send sheet, the launch paths, the session screen's reads and its writes. Requirements are exactly the members a consumer reads unguarded, everything else is a salvaged optional with main's own default applied in the transform, and no schema is `.strict()`: a member a newer host adds passes through untouched. Enum arm sets that a reader compares against pass through or degrade to the arm the reader handles most conservatively; the two closed sets — the committed change status and the diff kind — are closed because main *dropped* an arm it did not know rather than passing it through, and degrading them would draw a row or render a diff main never did. No member is coerced on the way back to the host. `github-pr-parsers.ts`, `github-pr-comment-parsers.ts` and `github-pr-value-readers.ts` are gone; their suite is now the parity record for the schemas that replaced them, with the four cases that refuse rather than degrade marked as such. Twelve call-site casts are deleted, and three dead "response was invalid" branches with them: the reader refuses those replies now, so the error names its method. The nine session files come off `unchecked-rpc-reader-inventory.ts` entirely rather than being lowered. `git show --stat` on this commit touches nothing under `mobile/rpc-foundation`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): unit-pin every session reply schema's decision Three kinds of case, one per kind of decision the schemas encode: a member a consumer reads unguarded is required and its absence refuses, an arm set a reader compares against degrades to the arm that reader handles most conservatively, and a reply whose arms need different members is declared as variants and each arm is read. The last suite is the wire-compatibility claim: a member no reader knows passes straight through, on the markdown document, the upload slot and the terminal inventory alike, so a newer host is never refused for a field mobile does not read. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh the corpus for the session domain's checked readers Repins `baseline` to the last commit touching a fenced path and re-records all 730 goldens, which is the disclosed behaviour change taken as an observation. Decoded through the value pool against the pre-refactor tree on this branch: 688 header-only with `baseline` the only key that moved, 42 body-moved, 0 added, 0 deleted. The 42 are seven named scenarios and thirty-five matrix goldens, and every moved checkpoint's own reply is malformed or refused. Three `normal` partitions appear in the list and none of them reads a well-formed reply differently: the review file-diff family's base scenario drives three legs and its third is scripted `{ kind: 'unknown' }`, so that leg's checkpoint moves in every variant, the varied leg included. The same append-only-history effect puts `pr-read-upstream-error`'s `no-pr` checkpoint in the list for the malformed PR recorded before it. What the corpus now records, in one sentence: a property read on null, a V8 destructuring message shown to the user, and four hand-written "response was invalid" strings are replaced by one message that names the method, and four screens that published a malformed payload as ready state now show an error instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): split the expanded check run out of the PR read schemas `github-pr-read-reply-schema.ts` was 328 code lines against the 300-line cap. The expanded check run and the annotations, jobs and steps listed under it are one reply with no reader in common with the other six, so they move to `github-pr-check-reply-schema.ts` whole. A move, not an edit: no schema changes and no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch's last fenced-path commit The schema-module split touched `mobile/src`, so `--record` refuses on the pin the previous refresh left behind. Repins to that commit and re-records. Decoded against the previous corpus: 730 header-only with `baseline` the only key that moved, 0 body-moved, 0 added, 0 deleted — the split is a move, and the corpus says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the worktree display-name cast's type import The live-title read is typed by its schema now, so the cast it annotated is gone and the import it needed with it. oxlint flags the leftover. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch tip The unused-import removal touched a fenced path, so the pin moves with it. Decoded against the previous corpus: 730 header-only on `baseline` alone, 0 body-moved, 0 added, 0 deleted. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): contain a refused prChecks reply to the checks section The checks read was the one phase-1 dependency that could take the whole PR sidebar down. `loadPrSidebarData` routed `!checksOutcome.ok` through `failureState`, so a host whose `github.prChecks` shape drifted cost the user the title, body, comments, reviewers and merge controls — everything they opened the sidebar for — over a section that renders a row of icons. Main never noticed because its unchecked reader answered `[]` for the same reply; this branch's reader refuses it, which is correct, and which is what makes the containment necessary. Contained the way phase 2 already is: a failed read keeps `kind: 'ready'`, empties `checks`, and carries the message in a new `checksError` so the checks section can say what happened. The sidebar can no longer reach `error` or `blocked` on the checks read alone. Also pins the enum departure this PR makes deliberately. The degrading arm sets go through `salvagedOptional(name, z.enum(...))` rather than `openEnum` because `openEnum` refuses a non-string where main mapped it to the conservative arm; nothing held that, and all 2477 tests stayed green against the swap. Six cases now hold both halves: a non-string degrades on the three open sets, and an unknown arm drops the row on the closed ones. Four deletions the reviewer found: a reaction-token alias with no importers, the `errorType`/`fetchedAt` the branch-lookup reader fabricated to satisfy a type whose only consumer reads neither, two bare schema aliases, and a quick-commands pass-through with two callers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the containment commit `--record` refuses unless the product tree equals `baseline`, so the fix above moves the pin. The corpus re-recorded in place against it: 730 goldens, every one header-only on `baseline`, no observation moved. No observation moved because no family reaches the code the fix changed. The `github.pr-read` family calls the seven wrapper reads directly and records their `{ ok, error }` outcomes; `loadPrSidebarData` sits a layer above that and no scenario mounts it. The prChecks outcome is identical before and after — what changed is what the sidebar does with it — so the unit suite is the only oracle for the containment. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the PR sidebar's checks containment The containment landed with no golden: no scenario mounted `loadPrSidebarData`, so the row in the delta table rested on unit tests alone. `PrSidebarLoadDeps` is five client-taking functions, so a new adapter drives phase 1 directly and records the `PrSidebarState` it resolves to — no React host, and no edit to an existing adapter, so no recorded golden moves. Two scenarios: a normal load, and one whose checks leg answers a shape the reader refuses. The matrix over the base then drives all eleven partitions at `github.prChecks#1`, and every one of them records `ready` with a `checksError` where main took the whole sidebar to `error`. `pr-sidebar-checks-failure-state` is the mutant that routes the refusal back through `failureState`; it moves both `pr-sidebar-checks-refused` and the prChecks matrix golden. Also pins two closed-and-required enum decisions that were free to become defaults — an unknown check-summary state drops the summary block, an unknown reaction content drops the reaction — deletes four exported type aliases and five enum constants with no reader outside their own file, makes `PRChecksSection`'s `checksError` required so a second caller cannot silently lose the message, and stops the header reading "No checks" when the checks were unreadable rather than absent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the pr-sidebar family commit Six new goldens — two pilots and the four matrix sites the base scenario scripts — and `baseline` on the 730 that already existed. No body moved and no `adapterSha256`: the family is a new adapter module, so nothing recorded through another one re-digests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the corpus against the merged main Repins `baseline` to the merge commit and re-records all 736 goldens in place. Against `origin/main` the 705 shared goldens move only on `baseline` (672 of them header-only), leaving the same 33 body moves and the same partitions the branch carried before the merge, plus its 31 added goldens. Every body also takes main's recorder shape from #21088: `sent` becomes `ordinal` over one interleaved write counter, subscriptions record a cleanup checkpoint, and a salvaging read now reports a `reply-salvage` effect naming what it dropped. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep an explicit null on the two tri-state PR flags `autoMergeAllowed` and `mergeQueueRequired` carry three answers, not two: `null` is GitHub saying auto-merge is not allowed, `undefined` is the host not carrying the member at all. The readers coalesced the null away, so a well-formed reply read differently from the parsers they replaced, which preserved it explicitly. Both shared types already declare `boolean | null`. No consumer separates the two today — `pull-request-auto-merge-availability` compares with `=== true` and `!== false` — so this is parity, not a visible fix, which is exactly why it needed a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tri-state flag commit All 736 goldens move on `baseline` alone: no scenario scripts an explicit null on either flag, so preserving it changes no recorded screen. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): check the two session-write readers #21083 brought Step 7 empties the session block of the unchecked-reader inventory, and #21083 landed two readers into it after that: the New Tab create's member read of `tab`, and the display-mode toggle's payload. Converting them is what keeps the claim true — a session line reappearing would mean the domain is not migrated. `created-terminal-tab` requires `tab.id` and `tab.type === 'terminal'`, because the strip keys the new tab on the id and spreads the rest into a union whose arm `type` picks. `terminal`, `title` and `terminalTheme` stay optional behind main's own guards, and unknown members pass through. `terminal-display-mode-set` reads nothing, so it takes the same `z.unknown()` the other five unread writes take. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record over #21083's corpus All 736 goldens this branch already had move on `baseline` alone, and #21083's 22 arrive beside them. One of the 22 moves against main's own recording: `matrix-session.create-terminal-session.tabs.createterminal-1`, where the New Tab create's five malformed partitions read `Cannot read properties of undefined (reading 'tab')` and now read the method's own message. Two of them also stop unsubscribing the terminal the user was watching before the property read threw, so a create that never happened no longer costs the live pane its subscription. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what carries a refused create reply to the catch Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
 Brennan Benson
|
aad41b1a40 |
fix(native-chat): render approvals from the harness presentation, not serialized tool input (#21087)
* fix(native-chat): render approvals from the harness presentation, not serialized tool input The approval card built its title from the tool name and rendered JSON.stringify(input) into an element with no height bound. Any large payload - a file write's contents, a proposed plan - pushed the action buttons past the viewport with no way to scroll to them, leaving the prompt unanswerable without zooming the pane out. Thread the agent SDK's own presentation fields through the prompt registry into the journal item: title, displayName, description, decisionReason, blockedPath and matchedAskRule. The SDK documents its title as the prompt text to use instead of reconstructing one, and warns that the decision reason may carry terminal escapes, so those are stripped before rendering. The card now also shows why a request was raised rather than only what it was. Bound the detail in a scrollable region that is reachable by keyboard, and cap it main-side with the existing shared tool-detail limit rather than the far looser journal payload bound. Focus moves to the card when a prompt appears and Escape resolves it, which previously did nothing because the composer owning that handler is unmounted while a prompt is pending. Mobile rendered the same unbounded detail and is fixed alongside. * fix(native-chat): keep approval actions reachable |
||
|
Jinwoo Hong
|
ccb4d2044b |
refactor(mobile): send the last session-route raw-port calls as operations (step 6, migration 2) (#21083)
* test(mobile): record the session startup, create and display-mode families
Three mount adapters and ten scenarios for the last raw-port sends in the
session route, recorded at the pinned main baseline before any product edit.
The three hooks were listed as blocked on a WebView-ref substitute. They are
not: none imports the terminal WebView, and all three send with no ref. The
display-mode toggle reads a `{cols, rows}` cell and a device-token cell; the
create path calls scope callbacks; the startup effect drives scope callbacks
only. Each stub is an effect sink, shapes no param and swallows no throw.
One scenario reaches both `worktree.activate` sites the way the product does:
the auto-create clears `created` off the route, the effect re-runs on the same
mount and takes the other branch, so the reply matrix drives both.
The create adapter mounts in its factory rather than as a scripted step. React
draws one `Math.random()` lazily the first time `enqueueTask` runs, and the
runner flushes through `await act` after every step, so a scripted mount would
make `clientMutationId` the second draw of the seeded sequence on the first
recording in a process and the first on every later one. The two determinism
runs caught it.
Recorded through the pinned-baseline worktree recipe, because main has moved
past `a28085adbf` in `src/shared` and this branch does not repin. 705 existing
goldens byte-identical, 15 added, 0 moved, 0 deleted.
Mutation census against the raw-port code, applied and reverted by hand, all
twelve killed: wrong method at each of the four sites; acceptance verdict
swapped at each of the four verdict-reading sites; dropped `unsubscribeTerminal`
on replace; the two activation branches swapped; a delayed `fetchTerminals` pass
dropped; the viewport pair not forwarded on `terminal.setDisplayMode`.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): send the last session-route raw-port calls as operations
Four references, three files, no behaviour change. Proven by replay: the
fifteen goldens recorded at the pin before this commit pass unchanged, so no
re-record.
- `use-mobile-session-startup.ts` both `worktree.activate` sends reuse
host-screen's `worktreeActivate`. Its skip verdict was never read before;
the startup effect is its first reader, and it reads exactly what main read
off the envelope — whether an accepted reply says the host is headless.
- `use-mobile-session-terminal-create-actions.ts` `session.tabs.createTerminal`
gets `sessionTabCreateTerminal`, a single-reader operation beside the other
session-screen writes. `require-result-or-throw-message` replaces the
`if (response.ok)` branch because the throw lands in the catch that already
reported the host's message, character for character, including the empty
message falling back to the screen's own copy. The reader stays the unguarded
`.tab` read, because that policy rethrows a reader's exception rather than
converting it, which is what keeps a null or absent result failing where it
failed before.
- `use-mobile-session-terminal-stream-display.ts` `terminal.setDisplayMode` gets
`terminalDisplayModeSet`, a skip whose verdict the caller does not read, the
way `terminalBufferClear` already works: the server does the resize and
reports it on the terminal's existing subscription, so main looked at nothing
in the envelope and only a transport rejection was ever a failure.
The prompt `terminal.send` in the create path stays on the raw port. It is the
only `terminal.send` caller that falls back to its own copy when the host
refuses with an empty message, so no existing operation carries its acceptance
and a new one is a fourth method outside this migration's scope. It is recorded
either way.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): lower the raw-port inventory and refresh the session route pins
Pending raw-port inventory: two entries deleted and one lowered, 12 files / 21
references to 10 / 17. The startup and display-mode entries reach zero; the
create entry keeps the prompt `terminal.send` and states its own reason.
Three stale comments corrected. The startup, create and display-mode entries
claimed a WebView-ref or subscription wall that measurement did not find: none
of the three hooks imports the terminal WebView, the display-mode write is not
gated on an open subscription, and the create path's `subscribeToTerminal` is a
scope callback rather than a `client.subscribe`. The accounts screen's entry
said the runner is request-only, which stopped being true when `ScenarioStep`
gained `frame`; what actually blocks it is that no scenario has been written for
`accounts.subscribe`, so its entry now says that instead.
Unchecked-reader inventory: `mobile-session-write-operations.ts` 8 to 10 for the
two readers the migration added, named in the header the way #20954's three are.
Route parity: four pins refreshed with their reasons — the callback bodies for
the display-mode toggle, the effects for the startup activation pair, the nested
function bodies for the create, and the runtime strings, whose count falls 535 to
531 as four more method literals move to their operations' definitions. The
startup source pins now name `worktreeActivate` and still hold what they held:
the plain activation is fired rather than awaited, and it goes out before the tab
load.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): say which part of the display-mode operation no golden holds
Post-refactor census survivor, measured rather than assumed: swapping
`terminalDisplayModeSet`'s acceptance for `require-result-or-throw-message`
moves none of the fifteen goldens. The call site reads no verdict and its own
`catch` swallows a throw either way, so no policy is observable there. The
method, the params and the viewport pair are what the goldens hold at that site.
The six other operation-level mutations all kill.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): record the empty cells the session guards are written for
Three session sends are gated on a cell every existing scenario filled: the
display-mode toggle carries `viewport` only once a surface has measured one and
`client` only once the phone holds a device token, and the startup sequence
swallows a refused tab load before loading terminals behind it. Every recording
declared those cells full, so the arm each guard exists for was never on the
wire and dropping the guard moved no golden.
The two device cells become scenario arguments rather than adapter constants, so
a scenario can declare them empty; the tab load may now be declared to reject,
which is the only way a refused scope callback is reachable at all. Declared, not
shaped: the stubs build no param and swallow no throw.
Three scenarios take the empty arm. The token and viewport ones send `auto`,
which is the direction both members ride, and the startup one records that the
terminal loads and the activation timer still run behind a refused tab load.
Recorded at the pinned baseline through the detached-pin worktree recipe, since
this branch may not repin. 705 goldens identical, 0 body moved, 3 added, 0
deleted; the 15 header-only moves are `adapterSha256` on the three edited
families and `scenarioSha256` on the four scenarios that now declare their token.
The create adapter's determinism comment now names the draw it works around:
React's lazy `("require" + Math.random())` in `enqueueTask`, the scheduler line
that seeds the sequence, and the mismatch a misplaced mount reports. #21088
retires the workaround.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): witness the three session guards the recordings had not pinned
Each mutation is the guard deleted: the display-mode send carries `client` with
an empty id, carries `viewport` before anything measured one, and the startup
sequence lets a refused tab load reject it so the terminal loads and activation
timer behind it never run. All three survived the whole suite before the
scenarios above; the witness asserts each is killed by its scenario and that
every other scenario of the same family still cannot see it.
A mutation that changes a param the scenario completes aborts at the transport's
params assertion instead of producing a divergent recording. That is the
scenario detecting it, so the witness reads that one message as a kill, narrowed
to it and taken only after the anchor is proved applied.
The README gains the class as its fifth bounding fact: a value an adapter holds
as a constant is a cell no scenario can empty, so the arm that reads it empty is
unreachable until the constant becomes an argument. Corpus counts refreshed to
what the suite measures.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): drop the terminal-create result type nothing reads
`TerminalCreateResult` wrapped the created tab for the old `sendRequest` reply
shape. The migrated call site reads the tab off the operation and names the tab
type directly, leaving the wrapper with zero readers repo-wide. Using it at the
cast site would have kept the cast and only renamed it, so it goes.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): let the create scenarios declare what the create puts on the wire
The terminal-create adapter decided four of the members its own goldens hold:
the worktree, the tab a new one is inserted after, and every launch option but
the prompt and its two toasts. A value an adapter supplies itself is a cell no
scenario can empty, so `afterTabId`'s omission arm — the arm a fresh session and
a last-tab close both take — was unreachable, and the quick-command members were
recorded only as absent. All of it now comes from the scenario, and the mount
moves to the first action so the arguments are in place before the hook reads
them. It stays out of a scripted mount step for the determinism reason above it.
Four scenarios follow the new arguments: a create with no active tab, a shell
quick command, an agent quick command, and a second tap while the host is still
answering the first. The refused scenario stops declaring an `errorToast` the
adapter dropped: forwarding the toast independently of the prompt is what the
product does, so that golden now records the failure toast it always showed.
Recorded at the branch's pin, so 705 goldens stay byte-identical to the merge
base; five headers move on adapter and scenario digests and one body moves, the
refused create's new toast effect.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): witness the three create guards the recordings had not pinned
Each of the three new create scenarios closes a mutation that survived all 853
tests before it: putting the active tab on the wire as `null` instead of
omitting it, swapping the `command` and `agentPrompt` members the host reads,
and dropping the in-flight guard so a second tap opens a terminal nobody asked
for. The witness asserts the hole and the closure together, as the others do.
The params-mismatch abort the witness reads as a kill now rests on an assertion
rather than on an argument: no scenario in the manifest completes a request
after its last checkpoint, so a send whose params stopped matching always
suppressed an observation a golden holds.
Known-open holes loses its prose count and becomes a list that names the site,
the mutant and why no scenario can see it. Two entries join it: the display-mode
acceptance, which no call site reads, and the startup timer's attached-terminal
guard, which needs an adapter that can attach a terminal mid-scenario.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): interpret the activation reply where it is reported
`reportActivationOutcome` took a verdict, which left the timer site hand-building
`{ accepted: false }` for the case where there is no reply to interpret at all.
Taking `RpcResponse | null` and interpreting inside puts the operation's own
policy at both sites and spells the absent reply as absence. Nothing is lost:
`worktreeActivate` reads an unchecked payload and admits every success, so its
`interpret` cannot throw on a reply either site can receive.
No golden moves; the effect digest is repinned.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): give the create family its mount step back
The create adapter mounted inside its first action so the create would run
ahead of the flush that made React pay its one lazy `Math.random()` draw.
#21088 pays that draw in the scheduler before it installs the seed, so the
position of the mount no longer decides which seeded value `clientMutationId`
reads, and the family goes back to the shape every other one uses: a declared
`mount` step carrying the cells the hook reads as it renders — the worktree,
the active tab, the device token — and a `create` step carrying the launch
options it passes.
The display-mode family keeps mounting from its `mount` action, which is that
same declared shape and never was the workaround.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the corpus at main's pin
|
||
|
Brennan Benson
|
4b87bc718e |
refactor(agent-launch): redefine the agent.launch contract (#20999)
* refactor(agent-launch): redefine the agent.launch contract
`agent.launch` has no clients yet, so the contract is redefined in place
rather than versioned.
- params require `operation.id`, pinned to the shipped operation-id mint so
the host can read the embedded timestamp back. No caller-supplied
fingerprint: the host derives its own.
- the result carries `disposition` ('created' | 'replayed', the same
vocabulary `RuntimeCreateAgentSessionResult` already uses) and a single
top-level `warning` instead of one on the terminal arm only.
- the prompt receipt becomes an outcome enum, so a receipt can under-claim
instead of reporting a bare `delivered: false`.
- the dead `customization` field is deleted, and the mode-reason union and
receipt are declared once in shared with main re-exporting.
- `clientMutationId` joins the reserved create fields, with a test pinning
the list to the create schema in both directions.
Contract only; no behaviour change and no ledger wiring.
* docs(agent-launch): stop calling the stripped set "agent fields"
`clientMutationId` joined AGENT_LAUNCH_RESERVED_CREATE_FIELDS, so three
comments describing the stripped set as agent fields now teach the wrong
model — including a SAFETY rationale, where a reader is trusting it most.
The rationale's claim is unchanged and still sound: deleting keys from a
parsed object leaves the rest the parsed shape.
* refactor(agent-launch): make the attempt id the launch's only idempotency key
Review follow-ups on the contract redefinition.
`operation: { id }` becomes a flat `clientOperationId`, spelled the way
`terminal.createAgentSession` and the structured mutation envelope already
spell the same concept, and admitted by the shipped
`parseAgentSessionOperationTimestamp` rather than a second copy of its
pattern — so `agent-session-host-authority` keeps the regex private.
The handler now dedupes on that id instead of the create payload's
`clientMutationId`. That field is optional, so keying on it left any launch
that omitted one with no idempotency at all, while the required attempt id
did nothing. Reserving `clientMutationId` is still right, but for the reason
the comments now give: `createManagedWorktree` never reads it, so a copy left
in the forwarded payload is inert while still reading as a guarantee. The
previous rationale — that it was a second live dedupe key — was not true.
`messageId` moves onto the prompt receipt's `journaled` arm so a producer
cannot report the text as committed without saying where, and `rpcCallerKey`
picks up the `terminal.create` call site it was lifted from instead of
shipping with no callers.
* docs(agent-launch): record why disposition is two-valued only for now
The ledger admits attempts whose outcome was never recorded, and neither
`created` nor `replayed` can say "I cannot tell you" — a caller handed
`created` for an unresolved attempt starts a second agent. Noted at the type
rather than in review, so whoever wires the ledger reads it where they edit.
* fix(agent-launch): keep contract within implemented guarantees
|
||
|
Brennan Benson
|
533b0bd02e |
fix(native-chat): count a turn from the send that opened it (#21086)
* fix(native-chat): count a turn from the send that opened it The live turn indicator switched on at the submission but anchored its clock at the provider turn-open, so it jumped back by exactly the dispatch latency the moment the turn opened. Measured on a real Claude session: the counter climbed to "Working for 25s", reset to "Working for 0s", then settled "Worked for 26s" — three readings of one turn, from two different instants. The host now resolves the send that opened a turn and publishes it as an additive optional `requestedAt` on the turn lifecycle row. `startedAt` keeps its exact meaning, the provider turn-open, and is never rewritten, so clients that cannot be upgraded see no change to any value they already read. Both providers write it; it is omitted when no send can be named (provider-resumed turns, replayed history). Readers take one origin, `requestedAt ?? startedAt`, for both the live counter and the settled host interval, so the two cannot disagree. The provider's own reported duration keeps outranking the host interval, unchanged. The host-to-local clock conversion is now latched once per turn rather than re-derived per render. `receivedAt - hostNow` carries that sample's one-way delivery latency as well as skew, and the reducer replaces the sample on every frame, so re-deriving imported fresh jitter and could move the anchor later — the same class of backwards jump this change removes. With the conversion fixed, an origin that improves moves the anchor earlier by exactly that much, so displayed elapsed only grows. No monotonicity guard is added; the ordering is structural. Desktop and mobile drove byte-identical copies of the timing hook, so both are collapsed onto one React-free helper in shared. Regression tests drive the origin resolution rather than an already-resolved anchor, assert in milliseconds because second-flooring hides the sub-second case, and include a deliberate host/client skew so a raw timestamp assignment cannot pass on a machine where the two clocks agree. * fix(native-chat): correlate Codex turn origins by echo * fix(native-chat): preserve causal turn timing ownership * fix(native-chat): keep settled turn timing continuous |
||
|
Jinwoo Hong
|
1c4f271478 |
test(mobile): repin the recording baseline to main after #21088 (#21105)
#21088 landed product changes on two fenced paths — the mobile hosted-review create params and the shared hosted-review contract — without moving the manifest baseline, so the corpus stayed pinned to |
||
|
Jinwoo Hong
|
9add08bb59 |
test(mobile): recorder follow-ups — write ordinal, teardown streams, context anchor, salvage observation, provider pass-through, React draw (#21088)
* refactor(mobile-recorder): one shared write ordinal for requests, payloads and effects
`sent` stamped each payload and effect with the number of requests sent at
write time, which orders those two lists against sends but never against each
other. A family that sends no requests therefore had every stamp at `0`:
moving `host-worktree-refresh.ts`'s two initial snapshot reads from after
`client.subscribe` to before it moved none of the 705 goldens.
One monotonic counter per recording now stamps requests, payloads and effects
alike at the moment each is written, so the three append-only lists are ordered
against each other. The same reorder now fails five goldens. A request is
stamped at the logical `sendRequest` call rather than when its physical payload
is published, so a send that waited for connected carries two distinct stamps.
Full re-record from the pinned baseline: 699 bodies moved, 6 header-only,
0 added, 0 deleted; the only moved JSON paths are `sent` leaving and `ordinal`
arriving on `sender`, `payloads` and `effects`. Decoding with those two fields
stripped leaves all 705 header-only.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile-recorder): observe streams still registered at teardown
Closing a stream only writes to the wire when its method has an unsubscribe
builder. `notifications.subscribe` has none, so a cleanup that forgets its
local `unsubscribeStream()` leaks a live registry record and nothing on the
wire changes. Until now that class was covered by one hand-written scenario
per method, which stops the stream and cuts over so the leak reappears as a
second subscribe payload.
Teardown now asks each session's `RpcClientStreamRegistry` what it still holds,
after the product's cleanup and before the transport disposes it, and records a
non-empty answer as a `streams-registered-at-teardown` effect carrying each
stream's method, subscribe payload and cancelled flag. The set is read off the
registry's own map: a mirror kept by the recorder would reproduce the product's
bookkeeping rather than observe it. Deleting `unsubscribeStream()` from
`mobile-notifications.ts` fails 7 goldens now, against 1 before.
Re-record: 4 bodies moved, 701 header-only, 0 added, 0 deleted. All four are
the two `runtime.clientEvents.subscribe` matrices, on partitions whose subscribe
reply is not a well-formed `ready`: with no subscription id to unsubscribe with,
the registry deliberately holds the cancelled record, which is why the
observation carries `cancelled`.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile-recorder): one host-client context exposure, anchored on the product source
Five adapter modules each carried `exports.recorderHostClientContext = Ctx;`
inside a source string appended to `client-context.tsx`. `Ctx` is a
module-private local, so the reference lives in a string no type checker
follows: renaming it typechecks clean and fails a recording with a
`ReferenceError` a hundred seconds in, five times over.
`hostClientContextExposure` and `loadHostClientContext` are the one copy, and
`adapter-seam.test.ts` asserts the declaration the exposure names still exists
exactly once in `client-context.tsx` and refuses a sixth inline copy. A rename
remains invisible to `tsc` — nothing but editing the fenced product module
makes a private local checkable — so the anchor is what turns it into one
failure that says what moved.
Also splits the subscription tests out of `recording-runner.test.ts`, which
items 1 and 2 had pushed past `max-lines`.
Re-record: 705 header-only, 0 bodies moved, 0 added, 0 deleted; `recorderSha256`
on all 705 and `adapterSha256` on the 23 goldens mounted through the five
modules.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile-recorder): record what a checked read salvaged
`collectSalvageDrops` builds a report on every decoded reply — which array
elements a `salvagingArray` threw away, which members a `salvagedOptional`
read as absent — and `classifyRpcReply` puts it on the outcome, where nothing
reads it. Which rows a reply lost was therefore visible nowhere, including in
a golden.
The recorder wraps `classifyRpcReply` on the mounted module, the one seam every
checked read passes through and the only one that knows the operation the drop
happened under, and records a non-empty report as a `reply-salvage` effect. No
product code changes; the report was already being built and discarded.
No golden carries one. All 19,384 checked reads in the corpus decode their reply
whole, because the reply matrix varies the envelope a host sends rather than the
shape of a row inside a result. The observation pins that absence, and moves the
first time a narrowed element or member schema drops a recorded row — including
where nothing downstream reads it. `salvage-observation.test.ts` is what keeps
the observation honest, driving a malformed row and a malformed optional through
the real `git.status` reply schema.
Re-record: 705 header-only on `recorderSha256`, 0 bodies moved, 0 added,
0 deleted.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(source-control): let hostedReview.create carry a provider token this build does not list
`HostedReviewCreate.provider` was a closed `z.enum`, so a client repeating back
a provider a newer host named in its own eligibility reply had its create
rejected at params validation. Mobile worked around it with a SAFETY-annotated
assertion: narrowing to `'unsupported'` before sending would have made the host
refuse its own provider, so the token was cast through instead.
The schema member is now `z.string()`, and both create handlers narrow through
`supportsHostedReviewCreation` before calling the runtime, so an arm this build
does not know answers `unsupported_provider` with readable copy rather than a
params error the client cannot act on. `createHostedReview`'s own refusal is
the single source of that copy. The mobile assertion is deleted.
Product change on a fenced path, so the goldens are not re-recorded: the whole
recording suite replays green against the corpus committed in the previous
commit, 825 passed, zero golden movement.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(source-control): annotate the runtime stub cast in the provider refusal test
The changed-code quality gate counts a new `as unknown as OrcaRuntimeService`
as a finding. A narrower stand-in does not exist: the interface has 1047
members and `Pick` of the three this test uses is not assignable.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-recorder): pay React's lazy Math.random draw before the seeded run
React resolves `enqueueTask` by reading `module['require' + Math.random()]` and memoizes the
result, so a process draws exactly one `Math.random()` the first time it awaits `act`. The runner
drains through `act` after every step, so that draw landed inside whichever recording ran first and
ate the seeded sequence's first value: a family recording a `Math.random()`-derived param recorded
one value when it ran alone and a different one when it ran after any other family, and an adapter
could only dodge it by drawing in its factory ahead of the first drain.
The scheduler now pays that draw once per process, before it installs the seeded generator, so the
seeded sequence starts at the same value for every recording. Priming is awaited, which makes
`start` async.
Goldens re-recorded: 705 header-only, `recorderSha256` alone. No golden carried a first-in-process
value, so nothing moved in a body.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-recorder): drain before reading the streams left at teardown
The teardown observation read the registry after `dispose()` returned but before the scheduler
drained, so a cleanup that closes its stream on a due 0ms timer had not run yet and was recorded as
an uncancelled registration — the one shape this observation reserves for a cleanup that never ran.
A deferred close and a stream nobody ever closed were byte-identical.
The drain now runs before the read, with the transport still disposed after it. A second drain stays
after disposal: tearing the registries down rejects what the product still awaited, and an unhandled
rejection is an effect the cleanup checkpoint has to see.
Also: the registry size comparison in `registeredStreams()` could never fire, because `size()`
returns `this.streams.size` on the same object; `RECORDER_HOST_CLIENT_CONTEXT` is used only in its
own module and no longer exported; and `streamPayloads` now says what it holds, which is every frame
the registry publishes rather than only subscribes.
Goldens are stale in this commit and are re-recorded in the next one.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the corpus after the baseline repin and the teardown drain
Recorded from a detached worktree pinned at
|
||
  Gon SongandNeil
|
85d1ffc072 |
fix: accept enterprise managed GitHub owner logins (#20450)
Unify owner validation across project pickers and repository overrides. Preserve EMU usernames in API and auth-status branch-prefix resolution, with regression coverage. Co-authored-by: Neil <neil@stably.ai> |
||
|
Jinwoo Hong
|
66a894d913 |
test(mobile): repin the recording baseline to main after #19850 (#21092)
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
 Brennan BensonandMerge Sim
|
97aa5ff19b |
fix(mobile): open native chat when a new worktree launches a default agent (#19850)
* refactor(agent-launch): make the launch-mode decision surface-neutral
`decideWorkerStartMode` was the only shared answer to "structured chat session
or terminal agent?", but it lived in an orchestration-named module and spoke
orchestration's vocabulary, so the other launch surfaces could not call it.
Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave
`orchestration-worker-start-mode` as the adapter that supplies the noun.
A worker is not a special kind of launch; it is the same launch with a dispatch
attached. Naming the receipt's subject is the only thing orchestration actually
contributed, so that is the only thing the adapter keeps: "worker" in both
sentences, plus the `--terminal` wording, which reads as nonsense anywhere a
`--terminal` flag does not exist. Both are pinned, because they are asserted.
No behavior change. The receipts are byte-identical for every reachable case,
proven by running the new pin against both implementations.
Also pins the wording, which nothing was holding. The existing suites assert
`toContain` fragments ('terminal agent', 'cannot create') and the CLI suite
asserts a receipt handed to it by a mock rather than one this code produced;
all six files stayed green against a deliberately corrupted vocabulary. A
dispatch receipt is the only place a structured-to-terminal downgrade explains
itself, so the whole sentence is the contract, not a fragment of it.
* feat(agent-launch): add the launch intent and the one executor that runs it
The sequencing around the launch decision was duplicated per surface, and the
duplicate is where the bug lives. A new worktree was created agent-first, so
its startup terminal WAS the agent and the structured branch below it could
never be reached — every new-worktree launch was a PTY regardless of the user's
default. Orchestration fixed that for itself in #19431; mobile and the CLI
still have it.
`executeAgentLaunch` inverts the order once, for everyone. When the preference
is structured the worktree is created with NO startup agent, the executing host
is then asked whether it can host a session for the workspace that now exists,
and only then is a surface created. The host verdict cannot be hoisted above
creation: `agentSession.createSupport` only answers for a workspace it can
resolve, which is why the decision stays in two halves.
Agent-first creation is deliberately preserved for PTY launches — it is what
sequences the agent's startup command behind the setup runner, so wait-for-setup
comes for free there.
What actually differs per surface is only how a surface is built (an
orchestration worker's session takes a dispatch hold and a mailbox a plain
launch must not take), so that is injected as a factory rather than branched on.
The intent also strips the reserved agent fields from a migrated create payload:
a caller moving off `worktree.create` passes its existing params, and a stale
`startupAgent` in there would re-create the very path this replaces.
Tests assert order and arguments, not just the resulting mode. Reintroducing
agent-first creation reddens 4 of 11.
* feat(agent-launch): expose the launch executor as the agent.launch RPC
Adds `agent.launch` — one host-side method that decides structured-vs-terminal and
creates the surface — wired to the real runtime factories: `createManagedWorktree`
for the workspace, forking on `startupAgent` exactly as the orchestration worker
path does; `createStructuredAgentSessionForWorktree` for a chat session; and
`createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the
routing gap was reported on.
`worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent"
verbatim, because it answers with `agentTerminalHandle` only on that path: a host
that quietly routed it to a structured session would hand every older client a
response with no handle and no error. All new behaviour sits behind
`agent.launch.v1`, which the host now advertises and a remote client must negotiate,
so a client that does not gets today's behaviour unchanged.
* feat(mobile): route workspace creates through agent.launch
Picking an agent on the mobile create sheet always produced a terminal, even
when the user's default was native chat, because all three create paths put
`startupAgent` on `worktree.create`. That means "create the worktree
agent-first", so its startup terminal IS the agent and the structured branch
below it is unreachable — while the same phone's in-workspace "+" button opened
a chat.
The blank, branch and new-branch creates now send the same payload through
`agent.launch` and let the host settle the surface. `worktree.create` is
untouched, and a host that does not advertise `agent.launch.v1` (read from the
existing `status.get` probe) keeps today's path exactly.
Work-item creates stay on `worktree.create`: they pre-fill the issue/PR URL as
an unsent `startupDraft`, which a structured session cannot hold yet, so routing
them would submit the URL as a first turn.
* fix(agent-launch): drop the deleted draft-prompt blocker from the reason map
main removed the draft-prompt blocker in #19681 (a structured session now holds
an unsent draft), so the exhaustive Record no longer typechecks.
* chore(agent-launch): carry a SAFETY rationale on the agent placement cast
The type-assertion gate landed after this branch's base, so the new file's
copy of the worker-start cast is now a changed-code finding.
* chore(agent-launch): carry agent.launch through main's RPC typing and casting gates
The typed-method contract, the generated params catalog and the
`assertionStyle: never` casting scan all landed after this branch's base.
- AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its
method name to `string` and broke assignability; every sibling infers instead.
- `agent.launch` binds a schema under src/main, so it joins the catalog's
RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin.
- The now-typed methods make most test casts unnecessary; the few that remain
carry the line-specific SAFETY rationale the casting gate requires.
* test(mobile): supply the agent-launch fixture the create-submit recording needs
The golden RPC recordings landed upstream while this branch was out, so they
first met agent.launch here. Three things had to happen, and only one of them is
a fixture bump.
1. workspace-settings-mounts.ts mounts useNewWorkspaceCreateSubmit against a
fixture model that throws on any member it was not given. This PR added a
required getAgentLaunchSupport, so the submit aborted with "Missing model
fixture" before it ever issued the create, and three cleanup checkpoints
vanished. That read like a product regression and was not one. Supplying the
member restores the recording byte-for-byte; it is pinned false for the same
reason the cutover probe is, so the baseline stays on worktree.create.
2. Editing that adapter moves adapterSha256 for the twelve settings goldens it
mounts. Their recordings are unchanged - header only, by design: the digest
is per-golden so editing a module fails exactly the goldens that mounted it.
3. Five goldens changed behaviourally, and both changes are this PR's:
the capability probe now reports agentLaunch, and a create whose reply
carries no worktree returns "Failed to create workspace" instead of throwing
a TypeError off an unguarded result.worktree read. The launch route needs
that guard, since a receipt can arrive without a worktreeId.
* refactor(mobile): decode the launch receipt instead of asserting its shape
The changed-code quality gate refuses type assertions, and the eight it flagged
were worth removing rather than suppressing.
The production one was the point. readAgentLaunchCreateOutcome asserted the RPC
payload into Partial<AgentLaunchResult> and then runtime-checked it anyway, so
the assertion bought nothing and claimed a contract the host had not proven. It
now narrows with `in` and validates each hop, which is the same nullability
question readCreateResult already answers on the sibling path - a launch receipt
can legitimately arrive without a worktreeId. AgentLaunchCreateOutcome ties
worktreeId to the shared contract so a change there fails this reader's
typecheck rather than passing a differently-typed field through.
The test fakes claimed a whole RpcClient via `as unknown as RpcClient` while
implementing one member. They now build a typed literal, matching the pattern in
use-mobile-structured-agent-options.test.ts. The read sites cast params and then
read one field; they now assert the payload with toMatchObject, which removes
the cast and pins more of the shape than the cast did.
Also pins the warning passthrough, which nothing covered: a terminal launch that
seats the workspace but cannot start the pty reports why, and the absent, blank,
non-string and structured-surface cases report nothing. Writing that test caught
a real drop I had introduced in the reader.
* ci(mobile): re-run Mobile Checks when a shared capability changes
Mobile Checks is path-filtered to mobile/**, but mobile imports the negotiated
capability names straight from src/shared/protocol-version.ts and records the
whole capability read verbatim in its goldens. So a capability added desktop-side
rewrites a mobile fixture while never triggering the suite that would catch it.
That is what happened here: #19849 introduced agent.launch.v1 and Mobile Checks
never ran on it. Verified at the run level rather than by check name - the
window-free check-runs API on
|
||
|
Jinwoo Hong
|
383c543e0f |
test(mobile): repin the recording baseline to main after #20950 (#21065)
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
a28085adbf |
refactor(mobile): checked reply readers for the source-control domain (step 7 pilot) (#20950)
* test(mobile): ratchet the 201 unchecked RPC reply readers Step 4 moved every call-site cast into an RpcOperation's `read`, but 201 of those readers still answer `compatible: true` for any payload: `rpcUncheckedPayloadReader` (163), `rpcReadUnchecked` (26 outside its own module) and `rpcUncheckedMemberReader` (12), across 42 files. The cast moved; it did not become true. Held as data with an AST boundary test, shaped on the raw-request-port ratchet: a file that is not listed fails, a listed file that no longer has one fails, and a count that rises fails. Only a call counts, so an import is not a reader and prose never is. No behaviour change: this commit adds a list and a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): validate the source-control domain's RPC replies at arrival Replaces all 17 unchecked readers in mobile/src/source-control/ with `rpcResultVariant(variant, schema)`, so a malformed reply is an `RpcIncompatibleReplyError` naming the operation instead of a TypeError three frames downstream. The inventory drops 201 -> 184 and the five source-control operations files leave it entirely. This is a behaviour change, scoped to malformed replies. Six reply-matrix goldens move; every named-scenario golden and every `normal` partition is byte-identical, which is the parity claim. Schemas live one module per reply domain, beside the operations that read them: git-status, git-compare, git-history, hosted-review and worktree-metadata. A member is required only where a consumer reads it unguarded, and each schema records the consumer line that justifies it. Nothing is `.strict()`; every reply a consumer publishes verbatim keeps `z.looseObject` so an undeclared host member still passes through. Six replies have no reader anywhere in mobile and get `z.unknown()`, which is the honest schema for them, not a holdout. Three readers stay total by construction, because their contract is that an unreadable reply is a value rather than an error: the `git.status` projection (a null status three screens route on), the `session.tabs.list` reveal (a null list means poll again) and the generated commit message (a screen's copy, never a decode error in a text field). They gain the salvage report, not a verdict. Consumers take the schema's output type, so `MobileGitStatusResult` and the branch-compare aliases now name what mobile reads rather than the desktop aggregate, and seven call-site casts are gone. Three requirements came from the goldens, not from the host types: `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the six source-control reply-matrix goldens step 7 moves Six goldens, all on malformed partitions. Every named-scenario golden and every `normal` partition is unchanged, which is the parity claim for this step. git.history-read / git.history#1 result-absent, result-null, inner-ok-missing, inner-false-string-error, inner-false-object-error: the load rejected with a TypeError reading 'items' or 'map' off undefined/null; it now rejects with `incompatible_reply: git.history-page (git.history)`. hostedReview.eligibility + create-intent / hostedReview.getCreationEligibility result-absent, result-null, inner-ok-*: the fetch fulfilled with the error envelope itself, re-typed as an eligibility and published into the compose prefill; it now rejects, and both callers already route that to the same "eligibility unavailable" state a null answer produced. hostedReview.create-chain + create-intent / hostedReview.create result-absent, result-null, inner-ok-missing, inner-false-object-error: the create form showed the raw TypeError text "Cannot read properties of undefined (reading 'ok')"; it now shows the incompatible-reply message. Every header digest is unchanged -- baseline, recorder, adapter, scenario and lockfile all match -- so the diff is the behaviour and nothing else. Recorded from this branch into a scratch directory and copied in, because there is no scoped honest alternative: scripts/rpc-recording.mts refuses to run unless the product tree equals the pinned baseline, and the README's remedy for an intended behaviour change is to repin, which rewrites the `baseline` header of all 667 goldens. So these six now carry a pin whose tree no longer produces them. That is a real gap in the oracle's design for behaviour changes, not a detail of this step, and it needs a decision before this lands. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the four reply-schema properties the goldens found Each of these cost a reply-matrix golden while writing the source-control schemas, and none of them follows from reading the consumers or the host types: a newer host's undeclared members must still decode, `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. The `.strict()` case is the one worth stating twice: at the top level it rejects the reply, and on the entry it drops the row, which shows a dirty worktree an empty Changes list. The fifth test pins the salvage report that makes such a drop visible instead of silent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give an unreadable reply a message a user can read `RpcIncompatibleReplyError` put `incompatible_reply: <op> (<method>)` in `message`, and `message` is what the screens hand to a toast. Step 7 is the first change that can reach this error at all, so the token would have shipped to users as its own error copy. Fixed at the boundary rather than per site: `message` is now plain copy, and the machine token moved to `code` (`incompatible_reply`) and `name` (`RpcIncompatibleReplyError`), both readable by callers. The cross-bundle fallback in `isRpcIncompatibleReplyError` matched on the old message prefix, so it now matches on `name`, which a foreign copy of the module still carries. No existing test pinned the old text. Two new ones pin the copy, the token and the foreign-copy match. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to this branch and re-record Commit |
||
|
Jinwoo Hong
|
f78483ec29 |
refactor(mobile): send the subscription-gated holdouts through typed RpcOperations (step 6, migration 1) (#20954)
* test(mobile): record the three step-6 families at the pin, and record a stream listener that dies Step 6 migrates the requests step 4 left behind because they share an effect with a `client.subscribe`. This records them first, from the pinned baseline, so the refactor that follows has a parity oracle. Three new families, one adapter module each: - `session.native-chat-page` — the older-history page. The read is a callback, but only the mount effect's `nativeChat.subscribe` arms what it pages against, so the frames are the setup: the snapshot's `beforeOffset` decides whether the request carries a cursor or asks for a growing tail. A cutover and a second snapshot pin the reconnect replay merging into paged-in history instead of collapsing the window. - `notifications.desktop-stream` — the desktop notification socket: the subscribe, the catch-up read its `ready` arms, the tray dismissals its events drive, and the server unsubscribe the disposer sends. Split in two so the base scenario's matrix sites all have partition-stable params: a variant that answers the second `ready` differently leaves the unsubscribe carrying the first subscription id, which the base's scripted params could not assert. - `session.terminal-gesture-input` — the debounced gesture flush and the menu's clear-buffer. Neither rides a subscription; a mount holding no terminal ref reaches both. The engine change is what makes the first two recordable at all. `ScriptedRpcTransport.frame` now returns what the product listener threw instead of throwing it on, and the runner records it as a `stream-listener-crash` effect. Only the two `runtime.clientEvents` listeners check that a frame payload is an object before reading its `type`; every other subscribing family took the matrix's `result-absent` and `result-null` partitions as an uncaught TypeError, which failed the suite rather than recording what a malformed frame does to a subscription. That is the same rule the crash boundary already holds for a screen and the unhandled-rejection window for a detached effect. The scenario's own faults stay loud: a missing subscribe payload, a params mismatch and a closed stream are all raised outside the caught region. `recorderSha256` therefore moves, so all 679 pre-existing goldens are re-recorded from the pin with this branch's recorder laid over it. Every one of them moves exactly one line and that line is `recorderSha256`: no `adapterSha256`, no `scenarioSha256` and no observation moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the subscription-gated holdouts through typed RpcOperations (step 6) Five references over four files leave the raw request port. Each was held out of step 4 because a request-only recorder could not mount it; the recordings landed in the previous commit and no golden moves here. - `use-live-worktree-name.ts` — `worktree.show` inside the focus effect that opens `runtime.clientEvents`. It reuses `sessionWorktreeRecordRead`, which is the diff-comment loader's reader renamed: both consumers read the `worktree` member whole and narrow their own field off it, so a second family would have been a second name for the same wire. The resolution still comes off the raw reply, because `selector_not_found` is what proves the worktree is gone and no acceptance policy carries a refusal code; the skip that follows is the same verdict main's `!response.ok` reached, since a refusal is the only reply this policy declines. - `use-mobile-native-chat-session.ts` — `nativeChat.readSession` in the paging callback. The payload stays whole because the reply is a union: an older runtime answers `{ error }` in place of a window, and the caller discriminates before reading a message list. - `mobile-notifications.ts` — `notifications.unsubscribe` in the `ready` branch of the subscription callback, in its own module rather than beside the push-route sends: one is the route this device holds with a gateway, the other the socket the paired connection holds. - `use-mobile-session-terminal-input.ts` — the gesture flush reuses `terminalInputSend`, which already carried the four other terminal-input call sites and the same accepted-verdict, and the menu's clear gets `terminalBufferClear` beside it. The clear is a skip because main never read the envelope: it toasted success on any fulfilled reply, so only a transport rejection reached the failure toast. That is preserved, not repaired. `mobile-session-route-parity.test.ts` refreshes three pins with their reason: the callback bodies and the twelve nested-function bodies moved where those send expressions were rewritten, and the runtime-string count drops by two because `terminal.send` and `terminal.clearBuffer` are now fixed at their operation's definition instead of spelled at the call site. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): hold the subscription coverage as a checked inventory instead of a README paragraph Every product `client.subscribe` is now an entry in `mobile/src/transport/rpc-subscription-inventory.ts`, classified as recorded (naming its family), an unwritten scenario, or walled with the wall named. `rpc-subscription-boundary.test.ts` fails on a new site with no entry, an entry whose file no longer subscribes, an entry naming a method the file does not open, and a `recorded` entry whose family the scenario manifest does not have. Both the unlisted-site and unresolved-family gates were checked by removing an entry and by misspelling a family; each fails on its own assertion. The paragraph this replaces said nine sites when there were ten. It counted over `mobile/src`, and the host screen's `accounts.subscribe` lives under `app/` — so the scan here covers both roots, the way the raw-port ratchet next door does. Ten sites today: four recorded, two unwritten scenarios, four walled (two on the webview ref, one on the multi-host client context, one on two unsubstituted view members). Unlike the raw-port inventory this list does not count down to zero. A typed operation fixes one method, one acceptance and one reader for one reply; a stream has many, and replacing a subscribe is not what this is asking for. The question it holds is the other one — which stream a golden actually has, and for the rest, what exactly stops it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the notification stream close, which writes nothing to the wire Deleting `unsubscribeStream()` from the notification cleanup — the local close, not the `notifications.unsubscribe` RPC beside it — survived all 810 tests. Neither unsubscribe builder in the stream registry knows `notifications.subscribe`, so closing that stream sends no frame; the mutant leaks a live subscription record instead, and the leak only surfaces when the logical client replays it onto the next session. `notifications-desktop-stream-closed` stops the stream and then cuts over, where the leak becomes a second `notifications.subscribe` payload. Recorded at the pin. No existing golden moves: the new scenario is appended, so it is not the family's matrix base, and every notification matrix site already had a fulfilled reply to replay. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): name the accounts screen's real wall, which is ScrollView and Alert The entry blamed `expo-router.useFocusEffect`, which is substituted, and the inventory's own `use-live-worktree-name` is recorded while importing it. Probed by mounting the screen through the trap: the first refusal is `Unsubstituted native member: react-native.ScrollView`, and `Alert` refuses too. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the terminal-send response reader that lost its last caller `isTerminalSendRpcAccepted` read the verdict off a whole envelope, which is what the raw call site did. Both callers now send through an operation and read the admitted payload, so the response form had only its own test left. The three cases move onto `isTerminalSendResultAccepted`, with the refusal envelope's missing result standing in for the failed response. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): attribute a frame crash to the listener that threw, not to the registry The try wrapped `stream.deliver`, so anything the registry raised on its way to the listener was recorded as a `stream-listener-crash` effect and blamed on the product. A reply like `{ok:false}` with no error object throws reaching for `error.message` before any listener runs, and that is a scenario that stopped matching, not an observation. Only the product's own `onData` is wrapped now. The throw is stashed and rethrown unchanged, so the registry still sees it the way a device's message handler does and what it skips after a dead listener stays recorded rather than invented; `frame` reports it only when the error it caught is the one the listener raised. `FrameListenerCrash` is local to the file again. Engine change, so every golden re-records: 694 files, every changed line the `recorderSha256` header, no body movement. Against main the set is 679 modified header-only and the same 15 added. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read the frame listener stash through a method, not a narrowed field `this.listenerCrash = null` before the try narrows the property to `null` for the rest of `frame`, so the catch compared against `never` and mobile's own `tsc --noEmit` failed. A private taker returns the declared type. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): abort a registry throw that stashed nothing, and fold the last native-chat read module in The frame catch compared `crashed?.error !== error`, which is false when nothing was stashed and the registry threw `undefined`, so that abort was swallowed and `frame` reported a clean delivery. It now asks whether a listener crashed at all. Also: `nativeChatSessionPageRead` moves beside the three other `nativeChat.*` reads and its one-export module goes; the session read header names the whole `worktree.show` record rather than review notes; the guarded-listener count is three, not two; the README names the ten subscribing sites blur is unrecorded across; and the gesture flush reads the send verdict as `=== true` like the other four sites. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): drop an oxlint disable the rule never needed `no-throw-literal` is not enabled here, so the directive read as unused and failed the changed-code quality gate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
c33a446190 |
feat(mobile): clarify the notification opt-in screen (#20930)
* feat(mobile): clarify the notification opt-in screen Replace the generic enable-notifications prompt with copy and a looping banner preview that show background alerts when an agent finishes or is waiting, even if the app is closed. * fix(mobile): share reduced-motion hook and wait before animating Extract the duplicated onboarding reduced-motion probe and hold the banner loop until the OS preference is known, so Reduce Motion users do not see the first cycle. |
||
|
Jinwoo Hong
|
d4c19d5db4 |
test(mobile): let the RPC recorder open a subscription and script its frames (step 6 capability) (#20920)
* test(mobile): let the RPC recorder open a subscription and script its frames The request-only runner threw on `client.subscribe`, which is why seven raw-port holdouts read "the recording runner refuses to open one". It no longer does. `ScriptedRpcTransport` drops the real `RpcClientStreamRegistry` into each physical session, the way it already reuses `RpcClientRequestTracker` for requests, so subscribe params, frame routing and the unsubscribe wire all come from product code. Per session, not shared: a frame is routed by the session that published its subscribe, and after a cutover the retiring registry is what holds a cancelled subscribe long enough to unsubscribe it once its id arrives. A subscribe writes to `payloads` through the same hook a request does, named by per-method occurrence, and frame ids come from the transport's existing counter because the real `DirectRpcClient` shares one counter across requests and streams. New scenario step kind `frame`: it names a subscribe payload, asserts its params the way `complete` does, and hands a whole host response to the real `handleResponse`, so `ready`, a data event, the host's end-of-stream pair and a refusal are one step kind rather than four. Every `payloads` entry now carries `sent`, the request count at write time, the same stamp `effects` already use. Without it, swapping `client.subscribe` and the first `sendRequest` in a product source moves zero bytes: a subscribe publishes synchronously while a request waits for connected, so the payload order is identical either way and only `sent` moves. The reply matrix now drives frames as sites, named by payload and occurrence because one subscribe carries many frames. Nine of the eleven partitions apply; the two transport rejections are what a request promise fails with and a subscription holds none. Success shapes keep the scripted frame's `streaming` flag, which is what routes a response to the open stream. `useFocusEffect` is substituted as `useEffect`, so a route's focus cleanup is recorded at unmount and a blur-triggered unsubscribe stays unrecorded; the README says so rather than a driven focus substitute no recording reads. Four tests, each killing a named mutation: routing a frame through the current session instead of the publisher, delivering a frame to the request tracker, dropping the `sent` stamp, and reading only `'complete' in step`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the two runtime client-event stream consumers Two families, both driven through the new frame step, as the capability proof for the subscription recorder. `session.live-worktree-name` mounts `use-live-worktree-name.ts` end to end: subscribe, `worktree.show`, a `ready` frame, the fulfilled name, a `worktreesChanged` frame, the follow-up `worktree.show`, then unmount and the `runtime.clientEvents.unsubscribe` its focus cleanup sends. `worktree.host-refresh` mounts `startHostWorktreeRefresh`, whose whole output is when it calls the two fetches it is handed. It sends no request of its own, so it is also the family that would have thrown `No scripted reply to drive a matrix over` before a frame was a matrix site. The 3 s foreground poll is driven by an `advance` step, which puts `WORKTREE_REFRESH_MS` under recorded time. Both adapters live in one new module, registered like every other domain, so the two families' goldens are pinned to a file that holds only them. No product source changes and no call site migrated: the seven raw-port holdouts and the `client.subscribe` zero-reference assertion belong to the migration PRs. `accounts.subscribe` in `use-mobile-home-host-connections.ts` is left out. Its snapshot decoder is re-exported through a React Native screen module the loader cannot reach, which is the same wall the accounts read has always been behind, so it needs a substitute beyond what these two read. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record every golden for the subscription recorder Engine files changed, so `recorderSha256` moves and every header re-digests, and `payloads` entries carry a new `sent` key. Nothing recorded moved. Recorded from a detached worktree at the pinned baseline with this branch's recorder laid over it, per the README's awkward case; `baseline` is unchanged. Decoding both sides through the value pool and ignoring `recorderSha256` and the new `sent` key: 641 compared, 6 header-only (the six goldens with no payload at all), 635 sent-only, 0 other, 9 added, 0 deleted. The 9 added are the two new families: a pilot golden each, four reply-matrix sites for the live title (two requests and two frames) and three for the host refresher (three frames, and no request of its own). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): take the broad object parameter out of the frame partitions `audit:anti-slop`'s no-object-parameters rule fires on a parameter typed `object`, which the frame-partition helper took to spread a success envelope. One function narrowing `unknown` to a spreadable envelope replaces the two that split the check, and the streaming flag is now read as `=== true` rather than by key presence, matching `isStreamingOpenerReply`. An engine edit moves `recorderSha256`, so every golden re-digests again. Decoded through the value pool, all 650 differ on that header alone and on nothing else. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): refresh the recorder's own scenario and golden counts The paragraph still claimed 78 scenarios and 153 goldens over 210 tests, which went stale across the domain additions since. It is 330 scenarios, 650 goldens and 757 tests as of this branch. The figures quoted further down are measurements of the change each one describes, so they stay as written; a line now says so. Prose is excluded from `recorderSha256`, so this moves no golden. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): take the inert optional off a frame, and pin the replay re-read Review of #20920 found four things the first pass got wrong. The `optional` flag on a frame step never gated anything: the registry routes every streaming response to the id that opened the stream, retired or not, so `frame()` only ever throws for a non-streaming reply. Dropping the parameter, the step field and the downstream marking moves the scenario digest of two matrix goldens and no recorded byte. The session comment claimed a mechanism that is not there. The re-send after a cutover comes from the logical client's own subscription replay, not from the registry being per-session; a shared registry is byte-identical. What being per-session buys is a frame routed through the session that published its subscribe, which is what `DirectRpcClient` does too. The host-refresh scenario now cuts over and answers a second `ready`, so the reconnect replay branch is recorded: deleting its re-read moves this family. Before, that branch was source no golden reached. README over-claimed the subscribe port as covered. Nine product call sites subscribe, two are recorded, and the other seven are now named with what stops each. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record for the frame flag removal and the replay cutover 644 goldens move on `recorderSha256` alone, from the engine edit. Two more also move `scenarioSha256`: the live-worktree-name matrix variants that used to carry `optional: true` on a downstream frame. Four bodies move, all in `host-worktree-refresh` — the pilot and its three matrix goldens now record the cutover, the re-subscribe payload, the retiring unsubscribe and the extra worktree/repo read the replay branch does. One golden is added, for the matrix site the second subscribe payload opens. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): count the golden the second subscribe payload adds Prose only; moves no golden. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the live-worktree-name replay re-read too The same cutover treatment as host-refresh: the scenario now migrates the logical client, answers a second `ready` on the re-sent subscribe, and answers the title read the replay branch makes. Before this, deleting that re-read from `use-live-worktree-name.ts` moved no golden. No engine file changes, so `recorderSha256` holds and 646 goldens are byte-identical. Five bodies move with their scenario digest, all in this family, and two matrix goldens are added for the sites the second subscribe payload and the third title read open. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what a request count cannot order, and name the accounts wall `sent` counts requests, so it orders payloads and effects against sends and not against each other. A family that sends none has no ordering at all: `host-worktree-refresh` keeps `sent` at 0 through every checkpoint, and moving its two initial reads across the subscribe moves no golden. The fix is one write ordinal shared by all three lists, which forces a full refresh. The `accounts.subscribe` wall was misdiagnosed. The loader reaches `decodeAccountsSnapshot` and it throws its own domain error; what the runner cannot supply is the multi-host client context `useAllHostClients` reads. Also honest about the record recipe: where a branch must not repin `baseline`, the detached-pin worktree is the only one that runs, merged main or not. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): file only a subscribe as an open stream, and drop three unused seams The registry sends its unsubscribes through the same `sendEncrypted` hook as its subscribes, and the hook filed every payload under `openStreams`. A frame aimed at an unsubscribe name therefore routed at that wire id, matched no stream, recorded nothing and reported success — where the README promises `Missing subscription payload`. A latch around the session's `subscribe` wrapper files only what a subscribe published. Its test fails without the latch. Three seams no caller varies, the same shape as the `optional` flag: `frameReplyPartitions` took a `scripted` reply to copy `streaming` from, but every frame site scripts a streaming reply, so the flag is stamped and a non-streaming unary closer as a base frame is called unsupported; the divergence map's three-deep ternary is early returns, since `index > divergence` already implies `index !== divergence`; and `MatrixSite` is no longer exported. Body-inert: re-recording into a scratch dir at this tree moves all 653 goldens on `recorderSha256` and nothing else, decoded through the value pool. The goldens are left stale for the merge re-record. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record every golden after the main merge One record at the pin, with this branch's recorder, scenarios and driver script overlaid on a fresh detached worktree. Decoded through the value pool against `origin/main`: 667 shared goldens, 6 header-only on `recorderSha256`, 661 also gaining the `sent` stamp this branch puts on every payload entry, nothing else moved, and 12 added — the two client-event families and their matrices. No `adapterSha256` moved, so main's adapter work was already recorded against its own goldens. Those 12 are byte-identical to their pre-merge bodies, `recorderSha256` aside, so the merge changed nothing this branch recorded. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
b8d4cde09f |
refactor(mobile): send six screen-mounted call sites through typed RpcOperations (step 4, wave 3) (#20919)
* test(mobile): record six screen-mounted call sites before migrating them Five new mount adapters and six scenarios, recorded against the pinned baseline's product code so the goldens are main's behaviour, not the refactor's. Each site is a screen the recorder could not previously mount: - `home.host-accounts` mounts `fetchMobileHomeAccounts`, whose decoder is re-exported through `AccountUsage.tsx`. That module loads under the mount loader, so the inventory's "no recording can load it" was already stale. - `notifications.display-test-screen` mounts the settings push probe and presses its button by reading the handler back off the rendered inert `Pressable`. - `aiVault.history-screen` mounts the history panel, which is where the last `worktree.ps` lives. Split in two: the base stops once the worktree list has seeded the scopes, because a reply partition there changes the scopePaths the downstream `aiVault.listSessions` carries, and a matrix variant cannot assert params it moved. The full chain is a second scenario, driven as a pilot only. - `tasks.route-repo-list` mounts the tasks screen-root hook and calls its own `ensureLoaded`, which is the only thing that fires `repo.list`. - `linear.select-workspace-picker` calls the render helper the tasks surface calls and invokes the `onSelect` on the element it returns. The picker draws inside `BottomDrawer`, whose reanimated timing driver and gesture builder the recorder would have to impersonate for a row to exist; the closure is the same either way, and the workspace a selection carries comes from the scenario. Five substitute members are added, each with the recording that reads it: `react-native-safe-area-context.useSafeAreaInsets` and `expo-router.useLocalSearchParams` for `tasks.route-repo-list`, and `react-native.TextInput`, `.SectionList` and `.RefreshControl` for `aiVault.history-screen` once its list renders. `useLocalSearchParams` answers one pinned route for the same reason the window size is pinned: a screen's own address is not a device reading, and the one screen that reads it sends `repo.list`, which takes no params. Touching the substitute table moves `recorderSha256`, so all 641 existing goldens are re-recorded. Recorded from a detached worktree at the pinned baseline with this branch's recorder laid over it: every pre-existing golden is header-only, verified by resolving both sides through the value pool — 641 header-only, 0 body, 0 deleted, one distinct `recorderSha256`, `baseline` and `lockfileSha256` across all of them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): type the linear workspace picker's model fixture `mobile/tsconfig.json` covers the recorder, and the fixture's setters were written with the argument the product happens to pass rather than the `SetStateAction` the model declares. Typing them moves `adapterSha256` on the two goldens recorded through this module, so they are re-recorded here rather than in the refactor commit, which must move none. Re-recorded at the pinned baseline: `linear-select-workspace` and its reply matrix, header-only, bodies unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send six screen-mounted call sites through typed RpcOperations Nine references off the raw request port, across six files. Every one is proven against the goldens recorded in the previous commit from the pinned baseline's product code: this commit moves no file under mobile/rpc-foundation/goldens. Reused rather than redefined: - `worktree.ps` in the history panel sends through `worktreeCatalogRead`. Same question, same acceptance — a refused list leaves the screen on what it holds. - `repo.list` in the tasks screen-root hook sends through `newTabRepoListRead`. Its policy raises the host's message and its reader takes `repos` off the payload while preserving the property-read exception a null result used to throw at the cast, which is what this call site did by hand. Its name still says new-tab; a third consumer does not make renaming it this bucket's business. Four operations are new, each because no existing reader on the method takes this consumer's input: - `files.read-directory-or-skip` and `files.legacy-explorer-list-or-skip` for the explorer. Both skip, because neither refusal is the operation's to decide: the readDir refusal code selects the legacy fallback and the list refusal supplies the message. The existing `files.list-or-skip` reads the `files` member alone, and the explorer also needs `truncated` for the "Showing first 5000" note. - `accounts.home-snapshot-or-skip` for the Home card, decoded by `decodeAccountsSnapshot` at the call site as before. - `notifications.test-push-or-skip` for the settings probe, whose `forbidden` and `method_not_found` refusals mean "try the next desktop". - `linear.select-workspace-or-skip` for the filter sheet. Two behaviours are preserved rather than repaired, both recorded: - The workspace switch never read its reply. `.then(() => loadLinearContext())` runs on a refusal exactly as on a success, so only a transport rejection reaches the error copy. Interpreting the operation here would surface a refused switch for the first time; that is a product change with its own re-record. - `app/terminal-settings.tsx` still reads `ms` off the reply envelope instead of off its result, so the value is always undefined. It did not migrate, and the inventory now carries the defect as its own note. Four mutants are added, one per new family that admits a state-only one: the Home snapshot, the push test result and the tasks repo list each decoded one level above the envelope, and the workspace switch with its context reload dropped. `aiVault.history-screen` gets none and says why in the suite: everything `worktree.ps` publishes also moves the `scopePaths` the next scripted completion asserts, so a mutant aborts the sequence instead of diverging from it. Its evidence is the reply matrix at that request. The tasks source-parity ratchet moves with the family it guards: hook, statement, declaration, render and style counts are unchanged, and the semantic source is a pure deletion of four lines — two `rpc:` call signatures and the two method literals they carried. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): matrix the six new screen families' replies One golden per scripted reply, eleven partitions each, recorded at the pinned baseline alongside the pilots. Seven sites: `accounts.list`, `notifications.testPush`, `repo.list`, `linear.selectWorkspace`, and all three of the history screen's — `worktree.ps` and the two `status.get` reads its scan chains off the worktree list. The history matrix is also that family's defect evidence in place of a mutant: every partition at `worktree.ps` changes the `scopePaths` the downstream `aiVault.listSessions` carries, and the sender args are recorded with it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct three operation and mutant comments Comment-only, no product behaviour and no golden movement. - `worktreeCatalogRead` says two readers; there are three. Names the third (the agent-history panel's `scopePaths` seed) and drops the stale count from the module header, which described call sites rather than the two operations. - `newTabRepoListRead`'s census counted the two operations over `repo.list`, not its own two callers, and claimed both read a workspace's connection id. The tasks route keeps the whole list for its repo pickers. The split from `nativeChatRepoListRead` stays where it belongs: acceptance. - The `aiVault.history-screen` mutant note pointed at the reply matrix as the accepted-vs-refused oracle. Decoding `matrix-aivault.history-screen-worktree.ps-1.json` through the value pool shows `normal`'s projected state is identical to all seven non-crashing partitions (spinner, two labels, zero rows). The real oracles are the next request's `scopePaths` (`["/repo/feature"]` vs `[]`) and the crash channel the three `inner-*` partitions land in. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): give the second files.list reader its real reason Comment-only, no product behaviour and no golden movement. `legacyFileListRead` claimed "the member reader rejects this consumer's input". Nothing rejects: `rpcUncheckedMemberReader` returns the member, and reusing it here would simply drop `truncated`. The reason the explorer declares its own operation is the other direction. Widening `files.list-or-skip` to a payload reader would split the `workspace-files` variant it shares with `nativeChatFileSearchRead` over `files.searchPaths`, whose only caller feeds both through one `extractPaths` in `use-mobile-native-chat-file-search.ts`, so the member read would move into that hook rather than disappear. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): indent the six scenario entries spliced during the merge The conflict on `pilot-scenarios.json` was resolved by id rather than by hunk, splicing this branch's six entries into main's text at the array's close. The splice started at the entry's `{` instead of at its line, so those six lines lost their indentation. oxfmt's only change is those six lines; the parsed document is identical, and the recording suite still matches all 667 goldens, so no scenario digest depends on the raw text. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the merged goldens once at the pin One record for the whole merged tree, at the unchanged baseline |
||
|
Jinwoo Hong
|
615b1370fb |
refactor(mobile): own the request/cache lifecycle in GenerationScopedRequestOwner, piloted on the legacy file inventory (step 5) (#20914)
* feat(mobile): own the request/cache lifecycle in GenerationScopedRequestOwner (step 5) Hooks guard stale replies with hand-rolled generation counters, `isCurrent` callbacks and latest-wins refs, so the guard is a callback a caller may forget. The owner keeps the cache, the in-flight identity and the generation token private. `read` and `load` are handed the scope and build the key themselves, so a scope the owner has not seen retires everything it held before it answers, and two workspaces cannot share a key. Publication goes only through `commit(lease, value)`: the lease brand is module-private, so no caller can mint one, and a lease whose generation moved is refused. `reset` bumps even when the scope came back to where it started, as in A to B to A. Three epochs may sit in a scope and they are not the same thing: the logical authority epoch, the physical authenticated session and the negotiated capability epoch. Which of them retires a given owner's data is that owner's decision, expressed by what its callers put in the scope. `lifecycle-owner.test.ts` carries one named schedule each for key-reset-cleanup, blur, cutover, reconnect-mid-request and stale-inflight-cleanup, each written as an explicit resolution order. It also fences loader bodies: a `load` callback that writes state it did not declare is rejected by the same kind of source scan that fences raw casts. Compile-time assertions live in a non-test file because mobile's tsconfig excludes tests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): put the legacy file inventory on the lifecycle owner (step 5) The native-chat file search kept three hand-rolled guards for one request: a generation counter bumped by an effect, a committed-paths ref, and an in-flight ref whose `finally` cleared itself conditionally. The stale-reply check lived in the reply handler, where a caller could forget it. The owner replaces all three. `read` and `load` are handed the scope, so the guard runs before either can answer, and the reply is published only through `commit(lease, value)`. What retires the inventory is named at the call site: this host, this workspace, this logical authority epoch. A reconnect to the same host leaves the files on disk alone, so the physical authenticated-session epoch is deliberately not in the scope. `RpcClient` gains one optional read-only signal, `getGeneration`, so a holder of a bare client can scope cached work to the logical authority epoch that `StableLogicalRpcClient.migrateTo` advances. Nothing else about either client widens. No golden moves: all nine legacy-inventory recordings reproduce byte for byte, including the A-to-B-to-A and cutover schedules. The `race` mutant is re-anchored on the owner's generation compare, which is now the only place that compare exists, and it still dies against b1. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): cut the lifecycle owner down to what callers use (step 5 review) Review round 1 on #20914 found three pieces of surface with no product reader and one vacuous assertion. `dispose()` is gone with the `disposed` field, the three guards that read it and the `'disposed'` verdict arm. A React effect cleanup cannot use it: the pilot's cleanup runs on every dep change and the owner outlives it in a ref, so a workspace select would dispose it permanently. Swapping `reset()` for `dispose()` there fails 7 tests across 3 files. `capacity` and its eviction loop are gone too. No caller varied it, so the loop never ran in production, its `if (oldest.done) break` was unreachable, and it evicted in insertion order while its name said capacity. `RequestCommitVerdict` and `RequestParameters` lose their `export` (no importer), as does the `generation` getter and the expect-error assertion that pinned it (test-only reader; `reset` advancing is proven by the verdict a lease from the previous generation gets). `LoadedRequest` keeps its export: it names the value of the public `load` promise, which a helper over that result has to write down. `key-reset-cleanup` now leaves a second request pending across the `reset()` and asserts the post-reset load starts its own, which is the half `inFlight.clear()` actually owns. Proof: deleting that line from `retire()` failed this schedule and `stale-inflight-cleanup`; before the change it failed only the latter. `read`'s doc now says it retires an unseen scope before answering and must not be called from render. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): read getGeneration off RpcClient and scope one attempt once (step 5 review) Two call-site findings from review round 1 on #20914. `mobile-session-tabs-stream-health.ts` hand-rolled `RpcClient & { getGeneration?: () => number }` and cast through it with no SAFETY rationale. `RpcClient` declares the member now, so both go and the read is `this.options.client.getGeneration?.() ?? 0`. The file-search pilot built its scope from a function it called twice in one attempt, so a `migrateTo` landing between the cache read and the load would have put one attempt in two scopes. It is a `const` computed once per attempt. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): type the scope, drop the in-flight wrapper (step 5 review) Review round 2 on #20914, owner side. `RequestScope`'s element type now excludes symbol and bigint, so both are compile errors with an assertion each in the fence. The runtime symbol throw is gone with the untested branch it guarded, and the bigint case it never covered (it reached `JSON.stringify` and threw V8's serialize message from two frames down) cannot be written. `InFlightRequest<Value>` existed only so its own `then` callbacks could name the entry they belonged to, which forced a throwaway `Promise.resolve(null)` that the next statement overwrote. The map holds the request promise itself and `settle` compares promise identity. `scopeMember` is inlined into `scopeKey`'s map callback: with symbol gone the member type is the scope's element type, which spells `object`, and anti-slop bans that in a parameter position. Inferred in a callback it is the same type with no annotation to ban. Header: `committed` says the generation still holds, not that the value already in the caller's hand is fresh. The pilot displays `loaded.value` directly and is fenced by the sequence counter it had on main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): gate the epoch in the pilot scope and the in-flight slot identity (step 5 review) Review round 2 on #20914 found two invariants no test held. The pilot's scope: replacing `client.getGeneration?.() ?? 0` with `0` left all 711 tests green. The new schedule pairs a control with the claim. A second query under the same epoch is answered from the inventory already held, and a query after the epoch advances issues a second `files.list` and displays what the new authority's host returned. Same client object, same workspace, so the epoch is the only thing that can retire it. Proof: with the literal `0`, `files.list` count is 1 where 2 is asserted. `settle`'s identity guard: making the delete unconditional left all ten schedules green. `stale-settlement-cleanup` puts a request in flight, resets, starts a live request on the same key, then settles the retired one last, whose cleanup names the slot the live request now holds. A third load must join rather than start. Proof: unconditional delete gives `started` 3 against 2. The fake clients go through one `fakeClient` helper, which is what lets the new case name the two members the hook reaches without a fifth `as unknown as RpcClient` (four deleted, one fenced assertion left with its rationale). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
ea7902cbee |
refactor(mobile): send the device-state holdouts through typed RpcOperations (step 4, wave 3) (#20915)
* test(mobile): record the terminal input surface before migrating it (step 4) Three families the recorder could not reach before, recorded against the pinned baseline's product code so the migration that follows has a parity oracle. The device state these hooks read is real, not declared. The pasteboard is the engine's existing per-recording fixture, so a paste reads the bytes a recorded copy put there one action earlier; the buffered draft store is the product's own useBufferedTerminalDrafts mounted in the same tree. No engine file is touched, so no existing golden moves and no header re-digests: 13 new goldens, 641 unchanged. Only the clipboard's text path is driven. The image path decodes a raster through expo-image-manipulator and stages it on expo-file-system, and recording it would mean inventing image and file-system behaviour. Both paths reach the same send. Two family mutants, one per family whose state() can observe a reply: keeping a refused send's draft cleared, and resolving the first repo's connection instead of the workspace's own. The paste family gets none — the hook returns void and calls onSuccess for an accepted and a refused send alike, so its only reply-dependent behaviour is the takeover report, which lives in the sender list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the device-state holdouts through typed RpcOperations (step 4) Ten references over six files, the last of the raw-port sites whose blocker was that a recording could not reach them. Zero goldens move: every one of the six was recorded first, and the suite replays them against the rewritten code. Two operations are new and four sites reuse one that already fixes their method: - accounts.consumeCodexResetCredit, throw-message, payload unread — the call site's decodeResetResult is one scope-and-snapshot check and splitting it across a reader would put one refusal rule in two places. - notifications.getMissedSince, skip — a background pass with no screen to raise a host message on. The member read stays where the optional chaining was. - repo.list: the accessory's connection lookup joins the new-tab reader, which already threw the host's message; the new-workspace dialog joins the skip reader, which already left the list it had. Same reader, same policies, no new acceptance rule and no third operation on that method. - terminal.send: the composed send, the live keystroke send and the clipboard paste all join terminal.input-send, which the accessory raw send already used and which reads acceptance the same way isTerminalSendRpcAccepted did. The typed contract is stricter than the client's own scope type on the redeem: the catalog pairs each runtime with the distro it may name, while the shared CodexResetCreditExpectedScope does not. The invariant is real and held by the attempt journal's schema, so the narrowing is asserted at the send with that named; the bytes are unchanged. Widening the catalog would be a wire change. Two source-shape ratchets pinned the old call text and move with it. The route parity suite's runtime strings drop from 540 to 537: the three method literals that became operation definitions, and nothing else. Every hook, callback identity, effect, JSX and style pin is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct the terminalInputSend and PTY-mode holdout comments `terminalInputSend`'s doc still claimed two call sites. It now has five non-test consumers, all on the same acceptance: the query-reply responder, the live accessory raw send, the session screen's composed draft send and live keystroke send, and the clipboard paste. That comment is where the next person narrowing `object-result-or-null` learns whose lost-ack meaning they are changing, so it names all five and their files. The session inventory block closed with "opens or rides a subscription, or takes its method as a parameter", which no longer covers every holdout below it: `use-mobile-session-terminal-input.ts` is held out for a webview handle. Its own reason also said PTY mode was unavailable in the runner, which this branch's terminal-input adapter contradicts by fixturing the mode map a paste reads. The sentence is narrowed and the holdout restated: PTY mode is recordable, the live webview handle is what is left. Comments only. No product behaviour, no golden, no parity hash moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pair the draft-restore mutant with the refused send `terminal-send-refusal-restores-draft` documents the harm of a refused send that leaves the composed draft cleared, but it was driven by the accepted scenario, where the kill comes from the inverse (a draft restored after a send that landed). The refused scenario shows the documented harm directly: without the restore the input stays empty after the runtime says no. Still one mutant per family, and it kills there — verified by running the suite, `terminal-input-send-refused: kills terminal-send-refusal-restores- draft`. No golden, no product change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct the gesture-input holdout and drop a dead repo type Three round-2 corrections, comments and one dead type; no behaviour. The gesture-input holdout claimed a recorder gap that does not exist. The flush path reads refs only — client, connection state, PTY modes, the gesture buckets, active handle and tab type — and the clear-buffer reference optional-chains the webview ref, so a mount with a null terminal ref puts both sends on the wire. The reason now says what is true: those 2 references are migratable as they stand and were out of this PR's bucket. The session summary sentence no longer offers a webview reason. `RuntimeRepoSummary` in mobile-session-route-types.ts lost its last consumer when the accessory hook moved to `MobileRuntimeRepoSummary`; `git grep RuntimeRepoSummary` now finds only the `Mobile`-prefixed type. Deleted. Both refreshed route-parity hashes still credited the `interpretOrThrowRefusalMessage` refresh for their current value. They now state the invariant they pin and this PR's reason for the move: the sends and repo reads inside those bodies name their `RpcOperation` instead of the raw `sendRequest` port. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
0869d997c8 |
refactor(mobile): send the rest of the session domain through typed RpcOperations (step 4) (#20891)
* test(mobile): record the session domain's remaining call sites before migrating them Freezes main's behaviour for what is left of `mobile/src/session/`: the AI Vault resume pair, the clipboard image upload and its two attachment surfaces, the native-chat terminal writes, session tab activation and reconciliation, the terminal-path tap, the structured agent launch, and the session screen's own reads, tab creates/closes, review actions, notes, markdown save and quick commands. 87 scenarios over 22 new families, recorded from the pinned baseline `c6a72169843ececf3a21da370ac50c5c5a4e6462` through a detached worktree, before any product edit. Ten new modules under `adapters/`, so each domain's goldens are pinned by `adapterSha256` and no existing family re-digests. `native-mounting-substitutes.ts` gains `expo-haptics` (inert: every caller is already fire-and-forget), `expo-clipboard` (a per-recording pasteboard cell, because these screens read back what they wrote) and `BackHandler`/`Keyboard`. That is an engine file, so all 509 pre-existing goldens move on `recorderSha256` and on nothing else. Two families cannot be matrixed at their first request: the clipboard upload chain puts the start reply's `uploadId` into the params of every later call, so a partition that answers the start differently changes a downstream assertion rather than a recorded observation. Their base scenarios stop at that first reply instead, and the fallback arm carries the upload family's second site. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): type the new session mount fixtures against the product model The ten adapter modules added with the session recordings typed several fixtures structurally — loose maps, a local `Terminal`/`Tab` shape, `unknown` for the review screen state — which `pnpm --dir mobile typecheck` rejects: `tsconfig` covers `src/test-support`, so an adapter is checked like product code even though no test file is. Each one now names the product type it stands in for (`MarkdownDocState`, `TerminalRecord`, `MobileSessionTab`, `ReviewScreenState`, `MobileDiffReviewQueueItem`, `DiffComment`) and supplies its members through `mountFixture`. Only one of those changed a recording: a `DiffComment` requires `side`, so the review actions now put it on the wire and the two `review-mark-reviewed-*` goldens carry it. That is the fixture becoming a real subset of the type it claimed, not a behaviour change — no product source moves in this commit, and the parity claim the next one makes is against these bytes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): format the session mount adapter and the recording manifest `pnpm exec oxfmt --check .` from `mobile/` flagged both after the previous commit's type fixes. The adapter is pinned by `adapterSha256`, so the eight goldens recorded through it are re-recorded from the same baseline; the manifest is not pinned by its bytes — `scenarioSha256` canonicalises the parsed scenarios — so no golden moves for it. Header-only either way: no observation changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the rest of the session domain through typed RpcOperations Thirty files and 58 references leave the raw request port. Every send now names a declared operation with a fixed method, one acceptance policy and one reader, and no call site casts a reply payload: the AI Vault resume pair, the clipboard image upload with both its attachment surfaces, the native-chat terminal writes, session tab activation and reconciliation, the terminal-path tap, the structured agent launch, and the session screen's reads, tab creates/closes, review actions, notes, markdown save, quick commands and file search. Five new operation modules, plus two readers added to existing ones. Three methods get a second reader, each argued where it is declared: `files.resolveTerminalPath` (the tap branches on five members the grant refresh hands back whole), `files.open` (the tap's miss is silent, the Changes screen raises the host's message) and `session.tabs.list` (the send sheet keeps only terminal tabs, which the reveal poller and the reconciliation controller both drop). `git.stage` carries two acceptances for the same reason `repo.list` does: a tapped file raises its refusal, a bulk sweep counts it. No new acceptance policy. The AI Vault resume launch and the review send sheet now share `mobile-review-terminal-operations.ts` with the PR triage launch instead of re-deriving the same create/send pair, and the file-search hook's `extractPaths` moves into the reader it belongs to. One latent behaviour is preserved rather than fixed and wants a ticket: `mobile-session-tab-activation.ts` decides whether to replay an activation with `error instanceof LogicalClientCutoverError`, not with the message-matching `isLogicalClientCutoverError` that exists because "instanceof can miss across bundle copies". Under a second copy of the module the retry silently does not happen, which the recorder reproduces. No golden scripts a cutover for this family, so none records the wrong behaviour. The offender floor in the port ratchet comes down from 50 to 20: it is an anti-vacuity guard on a list this migration is driving to zero, and 39 files still reach the port. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep main's malformed-reply message and re-anchor two mutants Three findings the reply matrices surfaced that the pilot scenarios could not. The clipboard upload's `uploadId` read goes back to the raw result. A success carrying no result throws a destructuring TypeError there, and V8 puts the destructured expression's source text in the message — which the composer shows. Reading the slot off the interpreted payload rewrote that sentence for every user who hits a malformed reply, on three families' `result-absent` and `result-null` partitions. The cast is the one main made, kept for the message alone. `race` and `new-tab-refusal-order` both anchored in text the migration rewrote, so each matched zero sites. Re-anchored at their new homes; the defect each injects is unchanged, and `probe-hole-witness.test.ts` still shows the probe killing the reorder while every pre-probe scenario survives it. `native-chat-send-delivery-unknown` is added as this domain's own mutant: dropping the delivery-unknown arm of a chat send makes an ack lost after the frame was written read as a definite rejection, which invites the user to send the same message twice. A second candidate — swapping `terminal.list` from skip to throw — survives every golden and is not registered: the inventory hook wraps its whole read in `catch { return false }`, so a refusal and a throw leave the same strip. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): matrix every reply the new session families script Thirty-four reply-matrix goldens, one per scripted reply across the twenty-two families the session migration added, each running the eleven partitions in `reply-matrix.ts`. Recorded from the same pinned baseline as the pilot goldens through the detached worktree, so they freeze main's answer to a result-less success, a null result, an inner refusal envelope, a message-less outer refusal and a transport drop — not the migrated code's. They are laid down after the refactor because they are what found its three remaining divergences, each fixed in the previous commit rather than recorded around: the clipboard upload's destructuring message, and two mutation anchors the rewrite left matching zero sites. Two families matrix only their first request. The clipboard upload chain puts the start reply's `uploadId` into every later call's params, so a partition that answers the start differently moves a downstream assertion instead of a recorded observation; their base scenarios stop there, and the upload family's fallback arm carries its second site. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): keep the new session readers inside the operation cast fence `rpc-operation-cast-fence.test.ts` refuses a type assertion anywhere in the region reachable from `rpc-operation-contract.ts`, and four of the new operation modules asserted their own result type. Each of those reads goes back to `rpcUncheckedPayloadReader`/`rpcUncheckedMemberReader`, and the shape the call site expects is named at the call site, which is where every migrated domain already puts it. `extractPaths` and `readQuickCommands` return to their hooks for the same reason — both were only movable by carrying a cast with them. No golden moves: the readers hand back the same values, and all 705 recordings still compare clean. The two frozen source-parity suites over the session route family are refreshed for the migrated text: three hashes, one runtime-string count, and the inventory hook's acceptance check, which now reads `!isCurrent() || !response.accepted`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): drop three assertions the changed-code casting gate rejects `pnpm run check:code-quality:changed` reports `consistent-type-assertions` separately from oxlint's own pass, and three sites had no rationale: the resume preparation's payload read, which gets the standard SAFETY line, and two adapter refs whose `null as string | null` is just an annotation written the wrong way round. The adapter is pinned by `adapterSha256`, so its thirteen goldens are re-recorded from the baseline — header-only, no observation moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): answer round 1 on the session migration Reuses `RpcOperationSender` for the file-tap client instead of respelling it as `Parameters<typeof fileTapPathResolve.request>[0]` at two sites; the type is the same by construction, and the handlers file no longer imports an operation only to name its first parameter. Names why `worktree.set-review-notes` stays separate from source-control's identical `worktree.set-review-link`, and why the new-tab loader's two preflight reads do not share the task drawer's readers on the same methods. Corrects the `callAgentSession` holdout count: five call sites across two hooks plus one inside the module's own mutation wrapper, not seven callers across five files. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state what actually gates the native chat paging read The `nativeChat.readSession` send sits in the paging callback, not in the mount effect. What blocks recording it is that the mount effect's `nativeChat.subscribe` is what arms the offset and generation the callback pages against, and the request-only runner refuses to open a subscription. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): answer round 2 on the session migration The tapped-path resolve is now a re-export of the preview screen's `terminalArtifactPathResolve` rather than a second definition: same method, same skip, same whole-payload read, so the only thing the old comment could claim was a difference that was not there. `fileOwnershipRuntimeStatusRead` already re-exports for this reason. `worktree.show` has four readers, not three. The one the notes read is closest to is `fileOwnershipWorktreeRead`, which reads the same member whole, and acceptance is all that separates them: a file mutation throws rather than write to the wrong host, a session screen without its notes shows none and keeps working. `interpretOrThrowRefusalMessage` is generic, so a caller keeps the interpretation's own type, and eight longhand copies of its try/catch across five files now call it. Three more copies wait on a frozen source hash; see the report. Three operations sit in the module matching their direction: the quick-command save with the writes, the review-notes read and the markdown read with the reads. The quick-command reader is shared across that line, which is what keeps the save from adopting `[]` on a payload the parser rejects. The native-chat readability probe imports `MobileRuntimeRepoSummary` instead of redeclaring it, and a stray mutant comment that described the race entry is gone. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): finish the refusal-interpretation helper across the session hooks Twelve try/catch blocks across the session hooks now call the generic interpretOrThrowRefusalMessage instead of rethrowing refusedRpcMessageOrFallback by hand. Each one throws the same message on the same inputs, and the request stays outside the catch, so a transport rejection keeps its delivery-unknown identity. Two frozen parity hashes move for that reason alone: - HEAD_CALLBACK_BODY_SHA256, for the one converted block that sits in a useCallback (use-mobile-session-diff-comments.ts) - HEAD_NESTED_FUNCTION_SHA256, for the three that sit in plain nested functions (use-mobile-session-content-create-actions.ts) Every other parity hash and every count is unchanged: hooks 269, callbacks 77, effects 24, nested functions 12, plus the callback identity, effect, main-hook, hook-binding, content-hook, native registration/removal and timer hashes. Copies that stay longhand, by design: - the action-level try blocks that wrap the request as well as the interpret and surface the failure to the UI, in the diff-review comment, git and send hooks - the two catches that call setActionError or setError and return instead of throwing, in use-mobile-diff-review-interactions.ts and use-quick-commands.ts - the call sites in files this branch does not touch (pr-ai-triage-launch.ts, mobile-diff-review-loaders.ts, github-pr-rpc.ts, github-pr-mutation-outcome.ts) No golden moves: this touches no golden, adapter or recorder engine file. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the goldens at the new baseline pin One re-record of all 641 goldens against pin |
||
|
Jinwoo Hong
|
2fccacadbe |
test(mobile): mount screens, declared device stores and declared OS state in the RPC recorder (#20884)
* test(mobile): mount screens, declared device stores and declared OS state in the RPC recorder Three recorder capabilities, each with a test, plus four holdout sites recorded against main's product code to prove them. No product source changes. - JSX compiles through the automatic runtime, which is what product sources use; the classic `React.createElement` emit threw `React is not defined` on the first render of every screen. - An unlisted package answers `__esModule` as undefined, so a default import loads and the refusal defers to the first real member read instead of killing the module at load. - The substitutes table gains the inert view packages a screen needs, split into `screen-native-substitutes.ts` behind the same rule: only what a screen reads is listed. - A scenario may declare `deviceStore` and `deviceState.notificationTray`. Reads resolve the declaration or null and never a write; writes are recorded as effects. Undeclared is unchanged. - `screenMount` mounts a component with a crash boundary, so a reply partition that takes a screen down is a recording rather than a suite failure. Every pre-existing golden re-records byte-identical except `recorderSha256`, recorded from a detached worktree at the pinned baseline. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): declare the crash boundary's state type instead of asserting it The changed-code casting gate counts `null as string | null` as a type assertion. Re-records every golden from the pinned baseline, because the edit moves `recorderSha256`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refuse an unlisted native package by the member a mount reads Both emitted interop helpers short-circuit on `__esModule`, so answering `true` hands the refusing trap back unwrapped to `__importDefault` and `__importStar`. All three import forms now load the importer and throw the named refusal at the first member read, instead of a namespace import silently yielding `undefined` and failing later at the call. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): run a task deferred past interactions instead of dropping it An inert `runAfterInteractions` swallows whatever send the screen deferred, and the recording then claims the screen sends nothing. It runs the task on a microtask and returns the RN-shaped handle, so a cancel before the task runs still prevents it. `Alert.alert` stays inert. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record every golden for the two recorder engine changes Only `recorderSha256` moves, from c58067de to a59b30e7. Recorded from a detached worktree at the pinned baseline |
||
|
Jinwoo Hong
|
005616171d |
test(mobile): record the real worktree catalog snapshot result in the RPC recorder (#20873)
* test(rpc-recording): record the real worktree catalog snapshot result `worktree.catalog-snapshot`'s action returned `WorktreeCatalogSnapshotClient.fetch`'s raw result. That value nests the live `RpcClient` under `pending.client`, so `captureValue` threw `Unsupported observation: function`: both goldens baked an `unhandled-rejection` effect and left the action's settlement `pending`, proving nothing about what the fetch returns. Project the result through `projectObservable` on the settlement path, the same way `state()` already shows it and the same way #20667 fixed `transport.pairing-race`. Rejections still propagate unchanged, so the two transport-rejection matrix partitions keep their recorded errors. The matrix golden now discriminates all eleven reply partitions: a full admission, six invalid ones, three `request_failed` codes and two rejections. `recorderSha256` does not move; `adapterSha256` moves on the six goldens mounted through this adapter module, four of which have no other change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): fail the suite when a golden records a recorder failure A projection refusal settles as data, not as a failed run: `captureValue` throws inside the action, the recorder captures it as an `unhandled-rejection` effect, and `--record` writes a green golden whose action never settled. The class has landed twice — `transport.pairing-race` in #20667 and `worktree.catalog-snapshot` in the previous commit — and reverting either one plus a re-record would go green with the broken golden back. Read every golden and fail on an `unhandled-rejection` effect or on either of `captureValue`'s refusal texts in any observation field. A positive control in the same case asserts both detectors fire, so the absence claim is load-bearing rather than vacuous. Not a recording driver, so `recorderSha256` excludes it and no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): ban only the recorder's own refusals, not product bugs The gate also failed on any effect named `unhandled-rejection`, which bans a real observation: detached-rejection capture exists to pin a main bug in a golden, and `unhandled-recording.test.ts` pins the capture precisely because no golden carries one today. Banning the name would make recording a genuine product failure a test failure. Drop that detector. `refusalText` alone catches all seven checkpoints of the worktree-catalog regression, because the refusal is the message of the captured error rather than the effect's name. The positive control now seeds that shape — the refusal inside a recorded error under `effects` — so the surviving detector is still proven to fire. Vacuity guards on golden and checkpoint counts are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): re-record the worktree catalog goldens over main's engine digest #20874 re-digested `recorderSha256` on all 509 goldens, so the six goldens mounted through the worktree catalog adapter had to be recorded again from the pin rather than merged. Bodies and `adapterSha256` are unchanged from the pre-merge recording. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
783d8feabb |
fix(lint): merge duplicate type imports in the mobile RPC recorder adapters (#20895)
* fix(lint): merge duplicate type imports in the mobile RPC recorder adapters The native code-quality audit rejects a module imported twice in one file, so main's static-analysis job is red for every open PR. * test(mobile): re-record RPC goldens against the merged adapters The duplicate-import fix changed two mount adapters, so the nine goldens that pin them by adapterSha256 needed re-recording. The recorder fence requires the pinned baseline to match the product tree, so the baseline moves to current main, which rewrites that header in all 509 goldens. Every recording body is identical, which also shows the commits between the two baselines changed no observed behavior. |
||
|
Brennan Benson
|
e7206f62a8 |
fix(mobile): retire a structured operation id the host has refused (#20868)
`agentSession.cancel` kept its client operation id whenever the outcome came back unknown. One of those unknowns is not transport doubt: when the host answers `agent_session_operation_unknown` it has decided about that id and will not run it again, because cancel's mutation plan recovers no unknown ledger row. Every later Stop on that turn re-sent the same refused id, so Stop stayed unusable until the row expired. The RPC layer collapsed both cases into a bare `unknown`, discarding the difference between "the effect is in doubt" and "the host answered about this id". It now reports the second case, and cancel spends the id there while still replaying under genuine transport doubt. `agentSession.conversationCommand` deliberately keeps its id: its plan sets `recoverUnknownFromDurableState`, so a reused id can still replay or rerun. |
||
|
Brennan Benson
|
6c03bb6e82 |
fix(lint): replace Reflect.get with typed property access in mounting substitutes (#20874)
* fix(lint): replace Reflect.get with typed property access in native mounting substitutes main's tip fails `pnpm run audit:anti-slop` (the `static analysis` CI gate) on `no-reflect-get` in mobile/src/test-support/rpc-recording/native-mounting-substitutes.ts, blocking every open PR. The Proxy get trap's key is `string | symbol`; branch on that to keep typed bracket access for strings and a symbol-indexed cast for symbols, preserving the existing throw-on-unsubstituted-member behavior exactly. * test(rpc-recording): re-record goldens for the recorderSha256 shift native-mounting-substitutes.ts changed bytes, so recorderSha256 (which pins every non-adapter file under this directory into every golden's header) moved. Re-recorded all 509 goldens; only recorderSha256 differs in any of them, confirming the checkpoint content is unchanged. |
||
|
Jinwoo Hong
|
d130347993 |
refactor(mobile): send the dictation, terminal, notification and browser domains through typed RpcOperations (#20702)
* refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from |
||
|
Jinwoo Hong
|
2dfdbc8657 |
refactor(mobile): send the task provider, detail and board domains through typed RpcOperations (#20685)
* test(mobile): record main's task provider item, detail and board RPC behaviour 35 scenarios over 22 of the 25 files left in src/tasks/, recorded from main so the step-4 migration of the provider half has a frozen answer to compare against. Every one of the 70 references this branch will migrate reaches a recorded wire here, which is the check the workspace-creation half added after it lost three sites to fixtures that short-circuited before the call. Scenario params are observed, not written: a generator drove each adapter with nothing answered, read the projected sender calls back, and emitted the completion steps from them, so no `params` in the manifest is a guess about what the screen sends. Five adapter modules, split the way the screens are: one item's reads, the list and composer, the item mutations, the board's reads and the board's row mutations. `mountModelHook` holds the mount/dispatch/project boilerplate these twenty-two hooks share, so each adapter is only its fixture, its actions and its projection. Two fixture modules hold the task items and the project rows, shared so the same pull request looks the same to the comment hook, the merge hook and the checks hook — which is what makes their recordings comparable. `baseline` moves from |
||
|
Jinwoo Hong
|
98784820d8 |
refactor(mobile): send the transport pairing and status domain through typed RpcOperations (#20667)
* test(mobile): record the transport pairing and status domain against main Adds seven recording families for `mobile/src/transport/`, recorded from main's unmigrated product code before any refactor: the protocol-gate hook, the retrying capability probe, the pairing candidate race, credential rotation, direct-to-relay upgrade, startup pairing recovery and first pairing. The relay modules build their `defaultDependencies` at module scope, so merely referencing `Platform.OS` or a storage-backed loader threw before an adapter could override it. `native-mounting-substitutes.ts` separates reference from use: react, zod and @noble/hashes are the real libraries, expo-crypto routes through the Web Crypto the scheduler already pins, and the two secret stores throw when called. `baseline` repins to |
||
|
Jinwoo Hong
|
44268d9616 |
refactor(mobile): send the github.* PR surface and the diff-review loaders through typed RpcOperations (#20668)
* test(mobile): record main's github.* PR and diff-review loaders before migrating them
Scenarios and goldens for the step-4 `src/session/` first half, recorded
against main's unmigrated product code so the migration that follows has a
frozen parity oracle instead of an assertion.
- 20 scenarios over seven new families: the seven `github.*` PR reads, the
twelve PR mutations split by their three reply contracts (`{ok}` envelope,
bare boolean, slug-addressed comment edit), the triage createTerminal+send
launch, the PR branch-context chain and the review screen's three loaders.
- Two new sender-style mount adapters. Both mount exported async functions
taking a client, so no React host is needed and the recorded state is each
wrapper's own outcome.
- 50 new goldens: 20 pilot, 30 reply-matrix sites. `recorderSha256` moved on
all 153 existing goldens because the adapters are in the whole-recorder
digest; no other line in any of them changed.
Text diffs are deliberately unscripted: highlighting one reaches `lowlight`,
which the module loader refuses as an unspecified native dependency.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): send the github.* PR surface and the review loaders through RpcOperation
The step-4 first half for `src/session/`: eight files, 38 references to the raw
request port, all replaced with declared operations. No behaviour change — the
50 goldens recorded in the previous commit do not move, which is the claim.
- 21 operations over 21 methods. The seven PR reads keep their defensive
parsers as readers; the ten status-envelope mutations share one reader
because the `{ok, error}` convention is one host convention, not ten; the two
bare-boolean mutations read the payload unchecked because `=== true` is the
caller's confirmation rule.
- Four second readers, each justified in place: git.status and git.branchCompare
for the PR branch context (a refusal costs a fallback, not the screen),
git.branchCompare and git.branchDiff for review (the projection is not a
superset of the verbatim payload), and worktree.show for the review notes the
summary reader drops.
- Every failure text is preserved, including the two main kept apart: a refusal
with no message falls back to the screen's copy, a transport drop with no
message surfaces its empty message verbatim. `sendRaw`'s callers replaced
theirs a second time, so those fall back on both paths.
- No retry, and no operation reads a dropped reply as a failed mutation: the
rejection reaches each wrapper's catch as the original object.
- `github-pr-mutations.ts` split along the action/comment seam it already had
in its consumers, so no file needs a max-lines bump.
Inventory: src/session/ 47 files / 114 references -> 39 / 76.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): record the review snapshot answering its notes leg first
The barrier mutation census found one survivor: moving
`reviewWorktreeMetadataRead.interpret` inside the `Promise.all` in
`loadMobileDiffReviewSnapshot` changed nothing any golden observed. The base
scenario answers the branch-base legs before the notes leg, so by the time the
notes reply lands the compare leg has already sent `git.branchCompare` and the
two orders record the same sender list.
This scenario answers the notes leg first, while the compare leg is still
resolving its base ref, and checkpoints before the rest. At that checkpoint the
barrier is the whole difference: the correct order has nothing settled, the
early interpretation has already rejected the action. The mutation now fails it.
Recorded from a detached checkout of the previous commit, which carries main's
unmigrated product code with this branch's recorder over it, so the parity claim
stays non-circular. One new golden; no existing golden moved, because the family
base is unchanged and `scenarioSha256` is per golden.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): bind one git.status projection reader, not a copy per domain
The branch-context read declared its own `statusProjectionReader` with the same
parser, the same 'normalized-status' variant and the same empty salvage as
source-control's `gitStatusProjectionReader`, while its doc block claimed "one
reader serves both". Export the source-control reader and bind it here so the
claim is true; the doc now names the reader and keeps the part that is actually
different, which is what a refusal means on each policy.
No wire change and no golden moves: the reader is the same function value the
copy computed.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): undo the github-pr-mutations split, which max-lines no longer forces
The split was made when the migrated file measured 319 lines. It does not any
more: `sendRaw`, `sendGithubPrMutation` and `extractMutationError` moved to
github-pr-mutation-outcome.ts and the prRepo/headSha allow-lists to
github-pr-repo-slug.ts, so the merged file is 293 lines against the 300 limit
and oxlint is clean.
Nothing imported github-pr-comment-mutations directly — every consumer went
through the re-export hub in github-pr-mutations — so the seam bought a reader
one extra file to open and nothing else. Merge it back and drop the hub.
Product-only: same wrappers, same params, same settle shapes, no golden moves.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one settleable-operation type for the PR reads and mutations
`GitHubPrMutationOperation` and the private `GitHubPrReadOperation` declared the
same two members for the same reason: a settle shape needs a bound operation's
method and its interpret, nothing else. Keep one, `GitHubPrSettleableOperation`,
and import it into the read settle. `extractMutationError` goes back to private,
as it was on main; it never had an importer outside its own file.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): drop the key-order claim from the PR param builder
The oracle does not observe param key order: `captureValue` in recording-values.ts
sorts keys, and no golden carries a raw frame string, so "the sender recordings
pin the bytes" was not a fact the evidence supports. The assertion stays for the
reason already in the doc — the builder is method-generic and returns a record.
`GitHubPrParamOptions` goes back to private; nothing outside the module names it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): read the bare-boolean mutations with the shared unchecked reader
`mutationConfirmationReader` spelled out what `rpcUncheckedPayloadReader` already
returns, under the same 'pr-mutation-confirmation' variant that eleven other
operations in this tree get from the helper. Same function value, same variant,
so no golden moves. The comment explaining why the payload is left unread stays.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one RpcOperationSender for both domains, not one alias each
`MobileSessionRpcSender` and `MobileSourceControlRpcSender` were the same type
with the same doc, each derived from whichever operation its domain happened to
own. Replace both with `RpcOperationSender` in transport, derived from
`settingsRead` there, and name it for what it is: what a bound operation needs
to send with.
Still derived rather than restated, so no module names the raw request port to
accept a client; the port inventory and its ratchet are untouched.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): point the moved PR and diff-review adapters at the seam and register them
The merge commit carried the two adapter files into adapters/ with their old
specifiers and left the register untouched, so this completes the move: the
relative imports climb one more level, and both modules are registered in
adapters/mounted-operation-modules.ts as identifiers imported from their own
source, which is what adapter-seam.test.ts checks.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the session goldens against #20662's adapter seam
The merge brought #20568's per-golden scenario digest and #20662's per-golden
adapter digest, so the 51 goldens this PR owns move on four header fields and
nothing else: baseline, goldenFormatVersion, recorderSha256, and the newly
added adapterSha256. No recorded byte outside those headers changed.
baseline stays at main's own pin
|
||
|
Jinwoo Hong
|
36ef93a64f |
refactor(mobile): migrate the small domains onto RpcOperation (step 4) (#20705)
* refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from |
||
 Neil
|
231e805b1e |
fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785)
Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear
every violation under src, config, tests and mobile.
What the rule bans
------------------
The case-insensitive substring "shape" in any JS/TS identifier: variables,
functions, parameters, types, type parameters, class members, private names,
object-literal keys and JSX identifiers. The one exemption is a statically
accessed member read owned by another value (`zodObject.shape` is fine), so
third-party APIs stay readable without a suppression.
"Shape" names a value's structure rather than its domain role. `UserShape`,
`validateArgShape` and `errorShape` all tell you the symbol is "an object
with some fields" -- which is already what a type says -- while saying
nothing about what the value is for or who owns it. The rule forces the
name to carry the domain instead.
Violations fixed
----------------
689 violations across 109 files at baseline (verified by re-running the
audit against the pre-change tree with the rule set to "error").
Fix pattern
-----------
Rename for the domain role, not the structure:
-type FieldShape = 'list' | 'map' | 'whole'
-const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape>
+type FieldEncoding = 'list' | 'map' | 'whole'
+const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding>
-function assertGitPushTargetShape(target: unknown): void
+function assertValidGitPushTarget(target: unknown): void
-function describeReadDirPathShape(p: string): ReadDirPathKind
+function classifyReadDirPath(p: string): ReadDirPathKind
Predicates became statements about the value (`isDeltaShapedProviderFrameKind`
-> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` ->
`discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` ->
`isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the
remaining name was already unambiguous (`GhGraphqlErrorShape` ->
`GhGraphqlError`).
No wire-visible name was renamed: no IPC or RPC channel, stream opcode,
request/response param, persisted field, or i18n key. The `--shape=symlink|copy`
CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged --
only the local variable holding it was renamed.
Exemptions
----------
They are file-scoped entries in config/oxlint-anti-slop.json, not inline
`oxlint-disable` comments. An inline directive naming an anti-slop rule reads
back as an UNUSED directive under the root lint scan, which does not load this
plugin -- the changed-code quality gate counts that warning, so the comment form
cannot be used for a rule that lives only in this config.
* src/renderer/src/components/browser-pane/annotate/**:
in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow,
rect, ellipse, highlight. That is a genuine domain noun, and it pervades
every symbol in the module.
* repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx:
lucide exports the icon component as `Shapes`. The name is theirs, and the
matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the
desktop picker -- renaming it would orphan saved repo icons.
* src/shared/onboarding-state-types.ts, src/shared/constants.ts:
`shapedSidebar` is a persisted onboarding-checklist field and a telemetry
enum member; renaming it would orphan saved state.
* src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape`
property is what selects the ZodObject branch of the conditional type.
No exemption was added merely to avoid a rename. Eight symbols initially
suppressed as "a cross-module refactor outside this change" were proven to have
zero non-TypeScript references repo-wide and renamed instead.
Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string,
z.ZodType>>` is its definition, so repo-update-params.ts and
ui-update-value-tolerance-params.ts spell it out instead. Likewise
telemetry-event-classification.ts now reads `.shape` through an `in` narrowing,
which also retires two pre-existing type assertions; three more assertions the
rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite
row read) became annotations and an explicit row mapping.
Verified
--------
* Audit reports zero violations; confirmed the rule genuinely fires by
planting a probe violation.
* node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0.
* Vitest over src/shared, src/main/github/project-view, the annotate module,
the repo-icon components and the Chromium SameSite electron spec: all green.
* All 66 removed "shape" identifiers grepped repo-wide across every file type;
none survive.
* node config/scripts/generate-rpc-params-catalog.mjs --check exits 0.
* node --check on every changed .mjs; oxfmt clean on all changed files.
* `pnpm run check:code-quality:changed` reports 0 findings.
Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve
`expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated
specs. All are rename- or comment-only hunks, read in full.
|
||
|
Neil
|
bfdec26352 |
fix(lint): enable anti-slop/no-object-parameters (#20781)
The rule rejects the broad `object` type on any function input (declarations, expressions, arrows, methods, call/construct signatures, function types), plus local aliases and unions that resolve to `object`. `object` accepts every non-primitive while exposing no properties, so it documents nothing and pushes callers into assertions at the boundary. Fixes all 185 violations across src, config, tests and mobile, and flips the rule from "off" to "error" in config/oxlint-anti-slop.json. Approach: replace each `object` input with the type its owner already has. Most sites took an existing domain type or a type-only import (36 added); 40 new aliases name shapes that had none. Where a value is genuinely only compared by reference, it gets a named identity token instead of a shape -- `Record<string, never>`, the built-in `WeakKey`, or a `unique symbol` brand, matching the branding already used in src/shared. Same treatment for WeakMap and Map key parameters. Two `as unknown as` casts became unnecessary once the parameter carried a real type and were removed; no new casts were added. Suppressions added: none. No `oxlint-disable` for this rule anywhere, and no max-lines disable or per-file bump. Three files sat exactly at their max-lines cap, so the added type imports were made line-neutral rather than suppressed: - src/main/ipc/browser.ts exports the existing guest-registration args type (renamed BrowserGuestArgs) so browser.test.ts reuses it on one line. - pane-scroll.ts takes TerminalScrollIntentTarget through the existing pane-manager-types import via a type-only re-export. - direct-rpc-client.ts drops the identity parameter entirely: the session check moved into the sendProbe callback that owns the token. Verified: anti-slop config reports zero violations over src config tests mobile; run-typecheck-projects-in-parallel exits 0; 144 affected test files pass (1749 tests); oxlint and oxfmt clean on all changed files. Mobile has no runnable test/typecheck target in this worktree (expo is not installed), so its 6 files were typechecked against a standalone config and diffed against the base branch -- error sets are byte-identical, including test files. |
||
|
Neil
|
f107499e44 |
fix(lint): enable anti-slop/no-reflect-get (#20786)
`anti-slop/no-reflect-get` rejects every call to `Reflect.get`. The
reflective read bypasses ordinary property access and throws away the
type evidence the compiler would otherwise give you: the result is
`any`/`unknown` with no narrowing, so a typo in the key or a shape drift
in the source object is invisible until runtime. The rule's remedy is to
parse dynamic input into a named domain type (or narrow it with `in`)
and then read the field normally.
Baseline: 86 violations across 67 files. Now zero unsuppressed
violations under
`npx oxlint --config config/oxlint-anti-slop.json --ignore-pattern 'config/oxlint-plugins/anti-slop/**' src config tests mobile`.
Fix pattern
-----------
44 of the 86 were rewritten. The dominant shape was an `unknown` value
read through `Reflect.get` right after a `typeof === 'object'` guard;
those became `in`-narrowed property access, which TypeScript checks:
- Reflect.get(value, 'agents')
+ 'agents' in value ? value.agents : null
Two further shapes:
- `Reflect.get(Object(x), 'k')` on a possibly-primitive envelope became a
small named reader that boxes once and indexes a
`Record<string, unknown>` (`settingsField` in
mobile/src/transport/settings-read-operations.ts).
- Tests reaching into private state moved to TypeScript's checked
bracket-index escape hatch (`runtime['layoutQueues']`), or to a
documented read-only accessor on the owning class
(`SearchSubprocessLineAccumulator.retainedCapacityBytes()`,
`CodexSubagentExecutions.retentionSizes()`).
No type assertion was added anywhere: the diff contains zero net-new
`as` casts, `as any`, `as unknown as`, `@ts-ignore`, or
`@ts-expect-error`, so nothing was laundered into the sibling
assertion rules.
Suppressions
------------
42x `// oxlint-disable-next-line anti-slop/no-reflect-get` across 38
files. Every one is the default-forward branch of a `Proxy` `get` trap:
get(target, property, receiver) {
...
return Reflect.get(target, property, receiver)
}
`Reflect.get(target, property, receiver)` is the only construct that
forwards with correct `receiver` semantics; `target[property]` invokes
an accessor with the wrong `this` and silently breaks getters that read
sibling state. There is no typed alternative, so these are suppressed
rather than rewritten.
3x `// oxlint-disable-next-line typescript-eslint/consistent-type-definitions
-- declaration merging requires interface` in
tests/e2e/github-url-smart-input-transition.spec.ts,
tests/e2e/linear-url-workspace-entry.spec.ts, and
tests/e2e/worktree-active-delete-scroll-position.spec.ts. Replacing
`Reflect.get(window, 'x')` with typed `window.x` requires a
`declare global { interface Window }` block, and `interface` is
mandatory for declaration merging. Matches the existing convention at
tests/e2e/helpers/runtime-types.ts:63.
1x `// eslint-disable-next-line no-var -- main-process gate handle for
this spec` in tests/e2e/project-group-creation-visibility.spec.ts, for
the same reason a `var` global is needed to type the handle. Matches
tests/e2e/agent-session-log-tail-stability.spec.ts:24.
Also updates two source-text anchors in mobile's rpc-recording mutation
harness (mobile/src/test-support/rpc-recording/operation-mutations.ts
and recording-runner.test.ts), which pin the exact text of the rewritten
line in settings-read-operations.ts and would otherwise fail with
"Mutant anchor matched 0 sites, expected 1".
|
||
|
Neil
|
2b34255d96 |
fix(ci): stop defining pilot mutant tests inside a conditional (#20755)
`vitest/no-conditional-tests` fires on the `if (mutation) { it(...) }` inside
the pilot loop, and `audit:code-quality:native` runs oxlint with
`--deny-warnings`, so main's "Enforce focused code-quality plugins" step exits
1 and blocks every open PR.
Pair each pilot with its pinned mutant and reference state before the loops, so
every iteration defines exactly one test unconditionally. Same 14 tests, same
names: 11 mutant-kill tests and the 3 reference tests that `skipIf` still gates
on RPC_FOUNDATION_REFERENCE_ROOT.
|
||
|
Jinwoo Hong
|
6a11a0b8e6 |
test(mobile): pin each RPC golden to the recorder inputs that can reach it, not the whole directory (#20662)
* refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from |
||
|
Brennan BensonandMerge Sim
|
f55b7ba680 |
fix(native-chat): cancel pending prompts precisely (#20601)
* fix(native-chat): hide activity while awaiting input * fix(native-chat): keep approval turns cancellable * test(native-chat): satisfy split PR quality gate * fix(native-chat): catalog approval cancellation label * fix(native-chat): include approval cancellation runtime label * fix(codex): settle prompts when cancelled turns complete * fix(codex): settle prompt registry fallbacks * test(native-chat): cover pending interaction fallbacks * test(native-chat): split prompt state coverage * test(native-chat): keep prompt state isolated * fix(native-chat): bound prompt turn backfill * refactor(codex): centralize prompt registry bounds * fix(native-chat): cancel pending prompts precisely * fix(native-chat): consolidate capability imports * fix(native-chat): harden precise prompt cancellation * fix claude cancellation teardown races * retry claude prompt lifecycle admission * bound claude prompt cancellation retry work * fix(codex): bound prompt turn identity on registration * fix(native-chat): route rejected late dispatch settlements * fix(codex): retain exact cancellable prompt turn ids --------- Co-authored-by: Merge Sim <sim@local> |
||
 Neilandplotarmordev
|
59d29af402 |
test: add a verified OMP native-chat mock scenario (#20655)
Co-authored-by: plotarmordev <plotarmordev@users.noreply.github.com> |
||
|
Brennan Benson
|
1d1bca2a7b | Bump mobile app.json to 0.0.50 (#20661) | ||
|
Brennan BensonandMerge Sim
|
c6a7216984 |
fix(native-chat): hide activity while awaiting input (#20496)
* fix(native-chat): hide activity while awaiting input * fix(native-chat): keep approval turns cancellable * test(native-chat): satisfy split PR quality gate * fix(native-chat): catalog approval cancellation label * fix(native-chat): include approval cancellation runtime label * fix(codex): settle prompts when cancelled turns complete * fix(codex): settle prompt registry fallbacks * test(native-chat): cover pending interaction fallbacks * test(native-chat): split prompt state coverage * test(native-chat): keep prompt state isolated * fix(native-chat): bound prompt turn backfill * refactor(codex): centralize prompt registry bounds --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
fc525c355d |
refactor(mobile): send the task workspace-creation domain through typed RpcOperations (#20568)
* test(mobile): record main's task workspace-creation RPC behaviour before migrating it 28 scenarios over nine task senders, recorded from main so the step-4 migration of the workspace-creation half of src/tasks/ has a frozen answer to compare against. Four senders mount as plain exported functions; three are model-chained hooks mounted the way the settings adapters mount theirs. The 153 existing goldens change header-only (`baseline`, `recorderSha256`): any new scenario re-digests the recorder, and the pinned baseline had drifted from main because the source-control migration landed. Content is byte-identical on all 153 — verified field-by-field against HEAD. `operation-module-loader.ts` now shares src/transport/rpc-delivery-ambiguity.ts with mounted modules instead of evaluating a second copy. The mark is a WeakSet keyed on the rejection object, so the copy the loader built had an empty registry and every delivery-unknown rejection read as a definite failure inside the operation under test — worktree.create's whole replay path was unreachable. With one registry, `tw-create-retry-ambiguous-after-drop` records the create still pending at the reconnect wait and abandoning at exactly 20000 ms, while the unstamped-create scenario records the same rejection surfacing at 0 ms. No existing golden moves: no other mounted module consumes the mark. `task-preferences-optimistic` is re-anchored above the send rather than across it, so migrating this file does not have to move the anchor. It still kills, and for the same reason: the preset the screen shows no longer follows the tap. Scenarios deliberately pin the empty-message refusals (`*-refused-empty-message`, `*-empty-message`), because a refusal with no message falls back to the screen's copy while a transport error with no message does not, and the two paths are easy to collapse when a call site moves behind an acceptance policy. Goldens: 153 -> 201, 2.9M -> 3.7M. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): format the recording manifest and re-digest the goldens `oxfmt --check` from mobile/ collapses a one-element `sites` array in each new scenario. The JSON value is unchanged — verified by comparing both files parsed and key-sorted — but the manifest is inside `recorderSha256`, so all 201 goldens carry a new digest. Every other field, header and observation alike, is byte-identical. Re-recorded in a separate worktree at the previous commit so the goldens stay attributable to main's product source rather than to the migration that follows. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): separate the goldens from the migration, and re-digest The previous commit accidentally carried the product migration alongside the manifest format, which both broke the commit that is supposed to prove parity and left the suite red: the digest was recorded without a comment move that a lint fix had made inside the adapter, so all 201 goldens failed their `recorderSha256` header. This backs the product half straight out again — the next commit re-applies it byte-for-byte — and re-records from the pinned baseline in a separate worktree carrying this branch's recorder, per the procedure in the recording README. Every field except `recorderSha256` is byte-identical to the previous commit's goldens on all 201 files, so no observation moved in either direction. The suite is green here with main's product source, which is what makes the next commit's "no golden changed" claim mean something. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the task workspace-creation domain through typed RpcOperations 12 of src/tasks/'s 37 raw-port files now send through a declared operation instead of the raw request port: 36 references to 0, leaving 25 files and 73 references for the provider item/detail/mutation half. No golden moved — `git show --stat` on this commit touches nothing under mobile/rpc-foundation/, which is the parity claim. Twenty-two operations over twenty methods, in four modules named for what they send: workspace create (create, PR/MR base resolution, create-time capabilities), workspace source (SSH connect/state, agent detection, repo hooks, sparse presets, ref search), task runtime (status, ui.get/ui.set, preflight, Linear status, settings.update) and the Smart picker's provider reads. Two methods carry two policies each, and both pairs are named. `status.get`: the Tasks screen cannot hydrate without it and surfaces the host's message, while create-time capability probing degrades to "no capabilities" and creates anyway — so one throws on refusal and one skips. `ui.set`: two sites await it, one is fire-and-forget and never interprets the reply at all. Both pairs share one reader, so no method has two readers. No new acceptance policy. worktree.create keeps its delivery-unknown contract. `request` returns the transport promise itself, so the retry loop catches the object the transport marked; two new tests assert `toBe(marked)` in one direction and that an unmarked rejection stays unmarked in the other, because a mark added on the way out would replay a create the host never received. `tw-create-retry-ambiguous-after-drop` records the create still pending at the reconnect wait and abandoning at exactly 20000 ms. Three sites still read the raw refusal envelope before interpreting, because the code or the message decides the route and no acceptance policy carries either through: the create retry needs the message for `isRetryableWorktreeCreateConflict`, and the paste lookup needs `method_not_found` to retire the slug probe host-wide. Both are documented at the site. The hydration barrier keeps raw requests inside its `Promise.all`. main's group rejects as soon as one leg rejects; `startRpcOperation` + `interpretAtRpcBarrier` would wait for the slowest peer and let a later policy surface a different error. Interpretation stays after the `stale` guard, where it was. `WorkspaceCreateParams` is now `RpcSendParams<'worktree.create'>` rather than `Record<string, unknown>`, which types the builder and the operation together; every field the three builders already sent typechecks against the host schema unchanged. `RpcSendArguments` now also makes params optional for a method whose params type has no required field, because `preflight.check` is such a method and main sent it none — requiring `{}` would have put a new object on the wire. The Mobile Tasks source-parity hashes move for the same reason bound settings requests moved them: the method string and the envelope read leave the screen. The signature diff is evidence rather than a re-pin — `semantics` is a pure deletion of 22 `rpc:` call signatures and 22 method literals with nothing added, statement/declaration/render/style counts are unchanged, and render tokens, styles and declarations are byte-identical. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): split the task workspace adapters at the sender/hook seam The single adapter file reached 344 lines against mobile's 300-line limit. CI lints every file, so this is red there even though the changed-code gate does not report it. Split along the seam the recording README already draws: exported async senders that take a client and need no React host, and the drawer's three model-chained hooks. No adapter body changed. Both files are inside `recorderSha256`, so all 201 goldens carry a new digest. Every other field is byte-identical, verified file by file. Re-recorded from the pinned baseline in a separate worktree carrying this branch's recorder, so the goldens stay attributable to main's product source. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): give the one-key unchecked reader a name Four readers were the same three lines: read one property off the reply, wrap it unchecked. `rpcUncheckedMemberReader` is the one-key sibling of the existing `rpcUncheckedPayloadReader`, so the annotation and the closure go away at each site. The pilot's `commitCompareEntriesReader` is converted too, so the helper has no longhand twin left to copy from. No behaviour change: the helper composes the same `rpcReadUnchecked` over `rpcPayloadMember`, including the property-read exception on a null result. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the local arm of workspace agent detection `preflight.detectAgents` was the one migrated operation with no recorded coverage: the ssh adapter hardcoded `connectionId: 'ssh-1'`, so the detection effect's ternary only ever took the remote arm and the local call site could be repointed at another method without a golden noticing. The adapter now takes the connectionId as a parameter and registers twice; `tasks.workspace-ssh-local` mounts the same hook with no connection, which is the only difference the effect branches on. Recorded at the pinned baseline with this branch's recorder laid over it, so the new golden is main's behaviour and the migrated code has to reproduce it — it does. Goldens: two added (`tw-workspace-ssh-local-agents` and its reply matrix). The other 201 changed on `recorderSha256` only, because the adapter edit moves the recorder digest every golden pins. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drive workspace create and the Linear list to a recorded wire Two operations passed a policy swap unnoticed, both because no golden reached their acceptance branch. `worktree.create`: the create hook's fixture resolved setup to a prompt, so all three settings.task-workspace scenarios stopped before the request and the only consumer that hands a refusal to interpret was never recorded. The adapter now takes the setup resolution as a parameter and registers a second family that resolves it, so createWorkspace runs to the wire. Two scenarios: a Linear item that creates directly, and a GitHub pull request that resolves its base first, which also puts this hook's built params — start point, generated display name, agent launch fields — in a golden for the first time. The existing prompt family is untouched, so its recordings still pin that branch. `linear.listIssues`: it appeared only in a non-base scenario, and the matrix reads the family base, so the family had no partition for it. The base now lists assigned issues after searching. Goldens: five added. Five moved beyond the digest, all derived from the smart-search base that gained the list leg. The other 198 changed on `recorderSha256` only. Recorded at the pinned baseline with this branch's recorder laid over it, so every new golden is main's behaviour. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drop the unreachable unmount branches from the task adapters Nothing dispatches `unmount` to these three adapters: the only producer is `lifecycleSchedules`, driven from a hardcoded five-id list that names no task-workspace family, and it pushes a `remount` right after, which these adapters would throw on. The branch read as lifecycle coverage that was never wired up. `dispose: hook.unmount` already tears the mount down. Goldens re-recorded at the pinned baseline because the recorder digest moved; `recorderSha256` is the only line that changed in all 208. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the goldens at main's post-squash baseline Recorded from a detached checkout of |
||
|
Jinwoo Hong
|
50e752fc66 |
test(mobile): pin each RPC golden to its own scenario input, not the whole manifest (#20562)
* refactor(mobile): pin each golden to its own scenario input, not the whole manifest `recorderSha256` covered the recorder directory plus `pilot-scenarios.json`, so every golden's header was a function of every other family's scenarios. Adding a family for one domain re-digested all 153 goldens and put a conflict on that line in every domain branch in flight, which serialized the step-4 fan-out. Split the two things it conflated. `recorderSha256` now covers the recorder directory only, with unchanged semantics: a recorder edit still forces a full, deliberate re-record. A new `scenarioSha256` pins the scenario input that golden was recorded from — every scenario `runRecording` consumed for it, in order — canonicalised through `captureValue` so an explicit-undefined param stays distinct from an absent one. `goldenRecording` takes that list instead of just its first member. The variants are hashed rather than the base they expand from because they are what was recorded: a matrix site, its replayed normal result and its partition replies are all visible in them without the derivation having to be restated. `derived-goldens.ts` is that derivation, extracted from `family-recordings.test.ts` so the digest and the recording agree by construction — a property test that restated how a matrix or schedule expands could agree with itself and with nothing else. It reproduces exactly the 153 golden ids on disk, and the census the suite already ran (every family matrixed, no stale normal-result inventory entry) now reads off its output. `golden-header-digest.test.ts` pins the four properties: - a new family in the manifest moves zero existing goldens' headers, and derives two of its own - editing one field of `b1` moves exactly `b1` and its family's four matrix goldens — not the two other `legacy-inventory` scenarios, and not the goldens that expand from `inventory-lifecycle` - editing a recorder file still moves every golden's `recorderSha256`, and no `scenarioSha256` - `recorderSha256` is unchanged by the manifest's contents, and no longer reads the file at all `GOLDEN_FORMAT_VERSION` goes to 4: a version-3 header has no `scenarioSha256`, and `compareGolden` walks the expected header's keys, so a reader that accepted one would compare that golden's own scenarios as though they were unpinned. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the 153 RPC goldens for the split digest Recorder edit, so every golden needs rewriting. Recorded from the pinned baseline `16d1ab81d3` with this branch's recorder overlaid, per the README's procedure: main has moved past the baseline, so recording in place would have failed the product-source fence. Three header fields moved and nothing else did: - `recorderSha256` 6a12160a87… -> 2fda557f58…, one value across all 153 files - `scenarioSha256` added, 153 distinct values - `goldenFormatVersion` 3 -> 4 No observation, checkpoint, value-pool entry, `baseline`, `lockfileSha256` or `platform` changed: git diff -U0 -- mobile/rpc-foundation | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' \ | grep -vcE 'recorderSha256|scenarioSha256|goldenFormatVersion' 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): certify the pilot goldens from the derivation that digests them `pilot-recordings.test.ts` restated `[scenario]` instead of consuming `pilotGoldens`, so the claim that a golden's `scenarioSha256` is a function of the same derivation that records the file held only for the 75 family goldens: dropping a scenario from `pilotGoldens` left the whole suite green and put that golden outside the header oracle. The pilot suite now iterates `pilotGoldens`, and a census fails if the derivation and the goldens directory disagree in either direction — which also closes the pre-existing orphan-golden gap. Also from review: pin the cross-sibling replay that hashing the generated variants buys (a matrix golden's `normal` partition replays a sibling's recorded reply, so editing that sibling must move it); state the real reason for the format bump, which is the diagnosis a version check gives rather than a rejection the byte compare already made; drop the fourth property test, which re-proved what tests 1 and 2 and `recording-runner`'s digest test already fail on; and drop a guard in `scenarioSha256` that its only caller reaches after an identical one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the 153 RPC goldens for the review edits Recorder files changed, so `recorderSha256` moved. Recorded from the pinned baseline with this branch's recorder laid over it, per the README's migration-branch procedure. That one header field is the only line that moved in all 153 files: `scenarioSha256` and `goldenFormatVersion` are unchanged, and no observation moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct the recording suite's test count Round-2 review: the README said 200 tests; the suite is 209 after the five added here. Markdown is outside recorderSha256, so no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the 153 goldens on the merged baseline Four header fields moved and nothing else. Proven against origin/main: every changed line in all 153 files is one of these, and the file set is unchanged. - `recorderSha256` 70aa6f59e0 -> 58a461dbc9: this branch's recorder, and it now digests only the recorder directory, not the scenario manifest. - `scenarioSha256` added, 153 distinct values over 153 goldens. - `goldenFormatVersion` 3 -> 4 for that added field. - `baseline` |
||
|
Jinwoo Hong
|
e53f1557e1 |
fix(mobile): two known main bugs the RPC migration preserved (#20563)
* fix(mobile): two known main bugs the RPC migration preserved
A malformed host `error` and a null settings result both reach a property read that throws.
Both are deliberate behaviour changes; the goldens move in the follow-up commit.
`hostReplyErrorTextOrFallback` passed a truthy non-string through under a `string` annotation.
Its one caller is the in-band `git.commit` failure, and every consumer of that text is display or
prompt copy: `use-mobile-create-pr-runner` and `PrSidebarCreateEmptyState` record it as a commit
failure, `use-mobile-commit-failure-recovery` hands it to `summarizeCommitFailure`, which starts
with `raw.slice(...).replace(...)`. So no consumer needs the value, and the decision is the
fallback rather than `String(value)` — the relay handler declares
`commit(): Promise<{ success: boolean; error?: string }>`, so a non-string is a malformed reply,
and `generatedCommitMessageReader` in the same domain already reads a non-string host error as
absent. The parameter stays `unknown`, which it honestly is, and the `SAFETY` cast is gone.
`useNewWorkspaceRuntimeContext` read settings through `settingsRead`, whose reader preserves
main's `boxed!.settings` throw, so a `null` or absent result threw a TypeError out of the effect —
losing the trusted-hooks publish and the available-provider computation that follow it, not just
the settings. It now uses `optionalSettingsRead`, the operation that already reads a null or
absent result as absent settings, so the reply degrades exactly the way a reply with no `settings`
member does. Reply-side only: same method, same params, same barrier, no wire change.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the goldens the two bug fixes move
Baseline bumped to
|
||
|
Neil
|
55b3392018 |
fix(terminal): drop the agent gutter from copied selections (#19770) (#20545)
* fix(terminal): drop the agent gutter from copied selections (#19770) xterm selections are screen cells, not logical text. Agent CLIs paint their messages behind a fixed left gutter, so every copied line carried that gutter into the clipboard and pasted replies came out indented. Terminal clipboard writes now drop the run of spaces that *every* selected line shares, so relative indentation (nested bullets, fenced code, YAML) survives and only the gutter is lost. A selection that starts mid-line, or that includes any column-0 line, has a shared run of zero and is copied verbatim. Applied at every terminal clipboard seam: the Cmd/Ctrl+C shortcut, the pane context menu's Copy, right-click-to-copy, the app menu's Copy, copy-on-select, the X11 primary selection, the dashboard popout's preview terminal, and mobile's selection Copy button. New "Trim Gutter on Copy" terminal setting (default on) restores the old verbatim-cell behaviour. * fix(terminal): honour the gutter-trim setting on mobile copy Mobile stripped the gutter unconditionally, so turning "Trim Gutter on Copy" off left one surface still rewriting the clipboard. Mobile now mirrors the desktop preference through the existing settings.get RPC — a host predating the setting sends no key, which reads as on, matching the desktop default. Also folds the single-use gutter helpers into their callers so the shared module exposes one function. * refactor(terminal): parse each selection line once in the gutter rule Also locks the Windows subtlety with a test: a blank CRLF row is '\r', which reads as a zero-indent content row and would cancel the gutter unless the CR is split off first. * fix(terminal): publish the gutter-trim setting to paired clients settings.get is an explicit allowlist projection, not the whole settings object, so terminalCopyTrimsGutter never reached mobile: the client read the key as absent, which means "older host", which means on. Mobile therefore always trimmed and the desktop opt-out was inert. Adds the field to the projection and a test that fails if it is ever dropped again — absence is indistinguishable on the client from an old host, so a silent regression here has no other signal. * chore: drop unrelated formatter drift from this branch A repo-wide `pnpm format` swept a quote-style change in pnpm-workspace.yaml and a blank line in source-tree-walk.test.ts into this branch; neither is related to the gutter fix. * fix(terminal): trim the gutter on native copy events too xterm binds its own DOM `copy` listener that writes raw screen cells (CoreBrowserTerminal `_initGlobal`). Orca's own chords never reach it — they preventDefault in keydown — but Ctrl+Insert is a Chromium copy accelerator on Windows/Linux and is not in `terminal.copySelection`'s bindings, so it still copied the gutter. Orca binds Shift+Insert for paste on those platforms, which makes the asymmetry worse. A capture-phase listener on the xterm element now writes the trimmed text, closing the class rather than the one chord: any native copy event — assistive tech, execCommand — lands on the same path. Installed for both terminal panes and the dashboard popout's preview terminal. |
||
|
Jinwoo Hong
|
7d98c8e2f3 |
refactor(mobile): send the source-control domain through typed RpcOperations (#20544)
* test(mobile): record main's source-control RPC behaviour before migrating it 45 scenarios over 11 source-control senders, recorded from main so the step-4 migration has a frozen answer to compare against. Adapters mount the real exported senders as plain functions, so no React host or device is needed. The 73 existing goldens change header-only (`baseline`, `recorderSha256`): any new scenario re-digests the recorder, and the pinned baseline had drifted from main in `src/shared` so recording required bumping it. Content is byte-identical on all 73 — verified field-by-field against HEAD. Scenarios deliberately pin the empty-message cases (`sc-*-refused-empty-message`, `sc-*-rejected-empty-message`), because a refusal with no message falls back to the screen's copy while a transport error with no message does not, and the two paths are easy to collapse when a call site moves behind an acceptance policy. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the source-control domain through typed RpcOperations 13 of the domain's 14 files now send through a declared operation instead of the raw request port: 44 references to 0. The holdout is use-mobile-git-requests.ts, whose single reference is a `(method: string, params)` dispatcher that five other hooks feed `{ method, params }` action steps at runtime; typing it is a step model change, not a call-site move, so its line stays at 1. Fifteen operations over fourteen methods. Two of them read git.status, and that is deliberate: the Changes screen publishes the host payload verbatim while hosted-review preparation reads the normalized projection, which returns null when `entries` is not an array and drops entries missing a path. Sharing the projecting reader would change what the Changes list renders, so both are named. Four loads still read the refusal envelope before interpreting, through readMobileGitRefusal: two degrade to a capability-missing screen, one retries a selector that is not visible yet, and one falls back from files.openDiff to files.open. `isMobileGitUnavailable` consults the code *and* the message and no acceptance policy carries either through, so the alternative was parsing a code out of a message. No new acceptance policy was added. Every migrated site keeps two error paths where it had two: a refusal with no message falls back to the screen's copy, a transport rejection surfaces its own message verbatim and keeps its delivery-unknown mark. Collapsing them into one catch is what would have turned an unknown mutation into a failed one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-digest the goldens after a lint fix in the new adapter recorderSha256 only, all 118 files; every recorded observation is byte-identical. Re-recorded from |
||
|
Brennan Benson
|
c287a5d9b7 |
feat(native-chat): add provider-aware Fast mode (#20506)
* feat(native-chat): add provider-aware fast mode * chore: drop unrelated formatter churn from the merge pnpm format reflowed pnpm-workspace.yaml quoting and a source-scan test that this PR does not otherwise touch. * fix(native-chat): review fixes for provider-aware fast mode Review pass over the Fast mode work. Claude reads its model catalog once per option write. The admit check, the effort guard and the Fast guard each took their own `list_models`, so a model write with Fast on paid two round trips for one list and let two guards answer from two different catalogs. The guards are now pure over a single read. Claude no longer refuses a Fast enable when the catalog identified nothing at all. An empty list is not evidence against a model -- the same rule the model admit-check already applies -- so a CLI that cannot answer would otherwise have Fast refused on every model. A catalog that did list the model and stayed silent about Fast is still not positive evidence and keeps refusing. Codex refuses a direct `serviceTier` write instead of accepting one the next turn discards. The turn derives the tier from `fastMode`; the key still restores so a session persisted before Fast existed migrates. Both option surfaces return a cached snapshot again. `SessionOptionsSurface` is read through `useSyncExternalStore`, whose contract is a stable snapshot, and rebuilding it per call breaks that for any consumer wired that way. Also records two decisions that were emergent rather than stated: routing Standard when Fast is on but no tier is named yet, and what a readback disagreement does and does not prove. Quality gate: merges the duplicate imports static analysis flagged, adds SAFETY rationales for two pre-existing casts the changed-code gate now sees, and drops a new assertion in favour of a checked narrowing. * fix(native-chat): read Claude Fast state from the session frame A fresh Claude session reports `fastModeState` while the settings readback still has no `fastMode` boolean, so the two are not redundant -- the frame answers at a moment the boolean has none. The picker fell back to "value unknown" and asked the user to disambiguate what the provider had already reported, and the state it reported had no reader at all. Falls back to the frame only when neither a pick nor the settings readback answers. `cooldown` throttles routing rather than clearing the pick, so it reads as on; reading it as off would flip a control nobody touched. Display only. The launch seed is untouched: an unset Fast preference still seeds nothing, which its own guard continues to pin. * perf(native-chat): skip the model catalog read when turning Fast off Turning Fast off needs no support evidence, so the read only cost a round trip — and restore replays a stored `false` on every acquire. Also narrows the alias-matcher comment: the effort and admit guards match on alias and resolved id only, so calling it the sole matcher overstated it. * fix(native-chat): clear a Claude Fast block once the child stops reporting it The child omits fast_mode_disabled_reason entirely when nothing blocks Fast and never sends a null, so requiring the key back latched the first reason for the session's life: switching to a model that disallows Fast and back retired the control for good, leaving a session running Fast with no way to turn it off. A frame that reports state without a reason is the all-clear. * test(native-chat): cover the mobile structured option hook useMobileStructuredAgentOptions gained generation fencing, a pending-write guard and a post-write options refresh with no test file. Pins the concurrency contract and the fast mode round trip: - a superseded options read is dropped instead of overwriting newer state - an overlapping write is refused and the pending guard is released after - an accepted same-fence write reads options back and applies the result, and a different-fence write does not - a boolean fastMode pick reaches the wire encoded and is remembered decoded - no Fast row when session support, catalog support or the model capability is missing Each behaviour was ablated against the production logic to confirm it fails without it. No production code changed. * feat(native-chat): render a boolean session option as one toggle On and Off were two radio rows under a header repeating the option name, so a binary choice cost three lines and two clicks to read. It is now a single switch row that owns its label, on desktop and mobile. An unknown value keeps its caption: a switch cannot say "unset". * fix(native-chat): resolve a boolean option's display value at the producer A boolean session option reached the UI in three states while its control had only two, so the renderer apologised for the gap with a "Current value unknown" caption beside a switch that had already collapsed to off. For `thinking`, whose catalog default is on, that caption sat next to a switch asserting the opposite of what every composed dispatch assumes. One expression fed both the displayed value and the option's provenance. Split them: the boolean descriptor now always carries a value, resolved to the same `values[id] ?? defaultValue` that buildNativeChatSessionOptionCommand already composes, while `valueSource` is untouched and still records whether anything confirmed it. `kind.currentValue` is required on the boolean arm so the third state cannot come back. The launch path is unaffected: resolveAgentSessionOptionLaunch and buildNativeChatSessionOptionCommand build the composed `--model` argument from the caller's picks and the catalog, never from a descriptor. Both surfaces mark an unconfirmed value instead of captioning it, and the two reasons stay distinct — `default` says the catalog value is what a launch will send, `unreported` says nothing has told us anything. Only `unreported` is reachable in the structured lane, where the agent may be routing a tier we have never been told about, so the two never share a label. * fix(native-chat): let assistive tech read the option value marker The marker was aria-hidden next to an explicit aria-label, so the label already won the accessible name and hiding it only cost screen reader users the default-vs-unreported distinction that sighted users get. It is now referenced by aria-describedby, which keeps the name Fast mode. Mobile's summary row said "Not set" for a boolean while the sheet behind it showed the switch on, so the two screens disagreed. A boolean always has a value; the summary states it and the sheet's marker qualifies it. * chore(i18n): drop the On/Off option strings the switch row retired Replacing the On/Off radio pair removed the only call sites for these two keys. i18next cannot rebuild a key with no call-site default, so leaving them in the catalogs forced them into the boot bundle as dead weight. Removing them shrinks it by two entries instead. |
||
|
Jinwoo Hong
|
c21c083224 |
fix(auth): report callback failures instead of cancellation (#20535)
* fix(auth): distinguish failed sign-ins from user cancellation * test(mobile): fix conditional registration lint and refresh recorder fingerprints |
||
|
Jinwoo Hong
|
7ce8e18d07 |
test(mobile): consolidate the RPC migration's verification infrastructure (#20521)
* test(mobile): record main RPC hooks and regression schedules Add scripted sender recordings, guarded main goldens, reply matrices, lifecycle schedules, settings caller fixtures, and targeted B-seed mutants. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): flush recording user actions through React act Keep lifecycle updates in separate act boundaries while wrapping direct stateful user actions. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): compile recorded modules with the Node VM API Use the same trusted-source execution boundary as existing mobile VM test harnesses. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pool golden values and hoist pre-divergence checkpoints Golden format version 2 stores each distinct observation field value once in a `values` map keyed by a 12-hex sha256 of its sorted-key JSON, and a checkpoint references five hashes. Output stays pretty-printed; the reader rejects any other format version, resolves hashes back to values, and reports the scenario, checkpoint, field and JSON path on a mismatch. Generated variants now declare where their distinguishing input lands, so checkpoints observed before that point are recorded once in a `.prelude` scenario instead of once per reply partition. Reply matrices, interruption schedules and lifecycle schedules share the primitive, which asserts each variant's pre-divergence prefix matches the base. Equal-but-differently-reached checkpoints are untouched. 17.71 MB / 3,599 checkpoints / 58.4% intra-file duplicates becomes 4.21 MB / 1,961 checkpoints / 23.6%, with every file's set of distinct observations unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): make the recordings sense deadlines, the recorder, and every family The goldens carried no temporal information, so a request deadline could be cut to a third and all 61 files stayed byte-identical. Every threshold is now straddled by two advances with a checkpoint between them: the 30 s request deadline in both schedule drivers, the 120 ms search debounce in b1, and the 60 s repo-metadata cache TTL. Shortening any of them moves an observation. The record fence pinned product sources but excluded the whole recorder, so --record could rewrite every golden from a modified runner and report the baseline intact. Goldens now pin recorderSha256 over every non-markdown file in the runner plus pilot-scenarios.json, and the fence exemption shrinks to the one directory that digest covers. Mutation evidence covered 3 of 13 mounted operations. There is now one anchored mutant per adapter family, covering 11 operations and 51 of the 61 goldens; the two omitted are the pure async loaders whose entire output is their settlement. Anchors are asserted to match exactly one site, which caught the acceptance mutant silently half-applying against three identical guards. The archived-tree assertion pins each seed's visible state instead of merely differing from main, and error observations carry code and cause when present. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record settlement times instead of straddling deadlines The previous commit made the reviewer's divide-by-three deadline mutant fail by placing checkpoints on each side of the 30 s deadline. That is a patch: a timing change that does not cross a hand-placed boundary stays invisible. Those scenario edits are reverted, and pilot-scenarios.json and schedule-driver.ts are byte-identical to what they were before them. The real defect was that the projection had no temporal dimension, so every settlement now carries startedAt and settledAt in virtual milliseconds on the pinned fake clock. Any transition the product schedules for itself is recorded at the time it actually fires, so a deadline or debounce change of any size, in either direction, moves a recorded number. A checkpoint's own clock is not recorded. It is always the sum of the scripted advances, so it is a function of the scenario rather than of the code under test; run-recording.ts asserts that equality at every checkpoint instead, which costs no bytes and fails loudly if it ever drifts. projectionVersion is 2 and all 61 goldens are re-recorded. With the added timestamps stripped, the distinct-observation set is identical to the previous recording, so the change is purely additive. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): probe the repo-metadata cache inside its TTL window Recorded settlement times cover thresholds the product schedules for itself, but not one it only consults when something else makes it act. The repo-metadata TTL is the single such case: with probes only at 0 s and 60 s, a 20 s TTL and a 60 s TTL are both expired at 60 s and record identically, so a 3x cache-lifetime regression was invisible. settings-repo-cache-expiry now probes the cache at 59 s as well. This is coverage, not a substitute for recorded time: it bounds how small a TTL reduction is visible rather than making the reduction itself observable, and the README says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the reply shapes a host can send, not a cross product The reply matrix froze ~26 malformed envelopes crossed against every consumed field and three boundary kinds, which is 163,925 lines of JSON pinning accidents on inputs no desktop produces. `successResponse` always sets `result`, so a JSON wire has no explicit-undefined slot, and no mounted handler returns a number, a string, an array, a bare `{}` or a boolean: `settings.get` returns `{settings: ...}`, and the seed methods return an object or nothing. Each family now runs nine witnessed partitions once, with no field cross: a normal result, an absent result, `null`, an inner `{ok: false}` envelope with a string or an object error, an inner envelope missing `ok`, an outer refusal, `method_not_found`, and a transport rejection. `null` stays because `linear.getIssue` returns it for a missing issue and b2 is a shipped null-result bug; it is also what carries the one named delta these goldens record. `run-step1-exit.ts` had zero callers and shelled out to the same two Vitest files as `rpc-recording.mts`, so it and its README paragraph go, along with `MUTATION_NAMES`, which only it read. In the module loader, the `rpc-delivery-ambiguity` escape is measured dead: over every scenario, mutant and reference run it was taken once, by the test that existed to take it. Golden comparison already fails loudly if a mounted module ever imports the marker, so both go. The history-panel exposure moves into a declarative table beside the mutation anchors, leaving the loader with one source-text mechanism and no per-file branch. The VM stays. Mount adapters load product sources from an arbitrary `root`, and the archived |
||
|
Jinwoo Hong
|
85d7cf3cc1 |
fix(mobile): preserve delivery ambiguity across transport cutover (#20280)
* fix(mobile): preserve delivery ambiguity across transport cutover Let physical close settle requests and retain its error as the cutover cause, copying only an existing delivery-unknown mark. Pin sent and unsent caller outcomes and both cutover predicate carriers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): pin the RpcClient.close() settlement contract close() was declared `() => void` with no stated obligation. That was harmless while migrateTo rejected pendings itself; now that it does not, close() is the retiring generation's only settlement path, so a type-compatible implementation that leaves a request pending strands its caller for good. States the obligation on the declaration and pins it for both trackers the real implementations reject through. Dropping the delivery-unknown flag, dropping the relay mark, or leaving pendings in the map each fail a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the cutover cause without a type assertion main's new casting gate rejects `(error as Error).cause`; narrow instead so the assertion still distinguishes a missing cause from an unmarked one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Brennan BensonandMerge Sim
|
2fc84cb492 |
fix(mobile): give native chat one tail-follow owner so streaming stops jumping (#20493)
* fix(mobile): stabilize native chat tail following * refactor(mobile): give native chat one tail-follow owner Extract the streaming scroll contract into use-mobile-native-chat-tail-follow, so intent and geometry have a single writer instead of a state/ref pair hand-synced at five call sites. No behaviour change: the existing guards pass untouched. * fix(mobile): fence native chat tail follow through momentum * fix(mobile): repin chat at measured tail --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
b0070e3720 |
refactor(mobile): migrate settings reads to RpcOperation (#20499)
* refactor(mobile): migrate settings reads to RpcOperation Replay the settings slice on the landed RPC foundation after rebasing onto main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh task parity snapshots after main rebase Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): correct rebased declaration parity hash Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): account for main task declaration Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): preserve raw RPC rejection timing Return the transport promise directly and interpret replies separately so sibling Promise.all rejection order cannot change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh parity hashes after timing fix Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): use operation interpreter after raw request * test(mobile): refresh settings migration parity hashes Refresh hook and statement parity hashes for the two task declarations whose settings reads now use RpcOperation request and interpretation. Changed declarations: - useMobileTasksRuntimeHydration: settings.get replaced by settingsRead request/interpret. - useMobileTasksWorkspaceCreateActions: settings.get response handling replaced by settingsRead request/interpret. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Brennan BensonandMerge Sim
|
8999a00281 |
refactor(native-chat): give each structured dispatch state exactly one meaning (#20133)
* refactor(native-chat): give each structured dispatch state exactly one meaning
`unknown` meant five different things. Only one of them was genuine
ambiguity.
A transport write that the provider's input pump never took is provably
undelivered -- which is what `rejected` already means. It was recorded as
`unknown` anyway, and a one-entry allowlist then existed solely to teach
Retry that this particular `unknown` was safe to re-deliver.
Collapsing that case into `rejected` deletes the allowlist and turns a
predicate into an invariant: Retry never re-delivers an `unknown`, with no
exception to reason about. The four states now each assert one thing --
`pending` written and awaiting, `accepted` the provider has it, `rejected`
provably did not happen, `unknown` genuinely cannot tell.
A fail-closed guard is the right default here because the asymmetry is
severe: refusing a legitimate retry costs the user a retype, while allowing
an illegitimate one sends the model a second copy of their message.
Also fixed, found while auditing every reader of `rejected`:
- The renderer printed `submission.reason` verbatim, so a broken pipe put
the internal token `provider_write_failed: broken pipe` on screen in
destructive red. The journal reason is unchanged -- it is the durable
evidence and the transport-versus-content discriminator -- but the screen
now gets copy that names the cause and says the message is safe to
resend. Content rejections still show the provider's own words.
- The fallback copy "Message was not accepted" read as a content refusal.
A null reason now yields "Message was not sent.", which asserts only what
every rejection shares.
- A refused worker-start preamble threw a plain Error out of the dispatch
path. It now throws `OrchestrationError('dispatch_preamble_undelivered')`
so a coordinator can tell "we could not send it" from "we sent it and
something else broke" without parsing prose. Retain/discard behaviour is
unchanged; only the verdict's legibility improves.
Two behaviours improve as a consequence rather than by design: a provably
undelivered message no longer blocks conversation commands, and no longer
leaves the session reading as "working" in chat and in every session list.
Not addressed here, and named rather than implied: a message left `unknown`
by a dead child or a host restart still has no recourse but retyping. The
restart reconciler that would decide those on evidence is written and has
never had a production caller. Parking the refused entry instead would
reintroduce the head-of-queue wedge removed in #19863, so it is not an
option.
Note for whoever edits `journal-reducer.ts` next: it sits at 297 of its 300
counted lines. The next statement added there needs a split, not a shave.
* fix(native-chat): close two gaps review found in the rejection taxonomy
Both are narrow and both were real.
A journal written before a refused write became `rejected` still holds that
submission as `unknown` with the transport marker. The predicate this change
replaced excluded exactly that shape from provider-echo matching; the
state-only check that replaced it does not, so on replay such a row could
claim the echo of a later, genuinely delivered send of the same text and
attach the delivery to the wrong message. Fail-closed still prevented any
re-delivery, so nothing duplicated — but the wrong submission was credited.
Replay now excludes the legacy shape too.
And the content-versus-transport split had a third case neither side covers:
a local capacity refusal is neither the provider explaining itself nor a
frame that failed to leave. It fell through to the verbatim branch, so
`claude structured dispatch queue is full` reached the screen — the same
class of leak this change set out to fix, one reason short of being caught.
Internal reasons now get copy; only a provider's own words are shown as
written.
Each is pinned by a test that fails with its guard reverted and passes with
it restored.
* fix(native-chat): preserve dispatch refusal across clients
* fix(native-chat): rotate immediately rejected retries
* docs(native-chat): correct rejection taxonomy reference
* docs(native-chat): align mobile retry comment
* docs(native-chat): clarify unknown replay semantics
* fix(native-chat): keep a mobile send's operation id when delivery is unknown
Mobile released the retained operation id whenever a send came back
`unknown`, so the user's next send of the same text went out under a fresh
id. A fresh id has no ledger row, so the host treats it as a first delivery
and dispatches it -- even though `unknown` is the one answer that says the
provider may already have the message. That is the duplicate this branch
exists to remove, reintroduced on the client that has no outbox.
Which case that was matters. Mobile only ever sees `unknown` from ack-loss
(`isRpcDeliveryUnknown`: "the host may have processed it and only the ack
was lost"), because the mapper reported every `ok` result as `accepted`
without reading `dispatchState`. So the rotation fired exclusively where
delivery was ambiguous and never where it was provably refused, which is
the inverse of the rule this branch establishes.
Retaining the id is what makes a retry safe, and it costs no liveness:
`performSend` answers a second request under a recorded id from the journal
and never puts it back on the wire, so a reused id delivers when nothing
landed and replays when something did. Rotating can only ever add a second
copy. The retention stays bounded by the host's admission window, which
`retainStructuredSessionOperationId` already enforces.
`retryUnknown` goes with it: the host ignores it for delivery, and all it
does is skip the cached answer to re-read the same row.
Keeping the id exposes what the rotation was hiding, so fix that too: a
replayed `unknown` comes back `ok`, and mobile called it `accepted` and
cleared the composer as if the message had landed. `dispatchState` now
decides, in one pure function:
accepted/pending sent, and the id is spent
rejected provably did not happen and terminal in the reducer, so
reusing the id could only replay that rejection: spent,
and the next attempt is a first delivery under a new id
unknown keeps its id
Reading `dispatchState` at all is a pre-existing defect, fixed here because
the false "sent" cannot be removed without it, and scoped to the send path.
`mutate`'s rotation for prompt/option/cancel plans is untouched. The
rejection copy is the desktop's notice, so an internal reason
(`provider_write_failed: ...`) still never reaches a person.
Tests: the hook test that was flipped to assert a rotated id now pins the
opposite -- one id across an ack-loss and two `unknown` replays, each
reported `unknown` rather than `accepted`. The send fixture grew the durable
submission row a real host returns; without it every send test asserted
against a shape that cannot express the bug.
* fix(native-chat): enforce fail-closed structured send replay
* fix(native-chat): align retry and mobile RPC contracts
* fix(native-chat): keep transient admissions retryable
* test(tab-bar): expand nested create menu in harness
---------
Co-authored-by: Merge Sim <sim@local>
|