Repository navigation
refactor(mobile): send the source-control domain through typed RpcOperations - #20544
Merged
Merged
Conversation
…ting 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
…rations
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
recorderSha256 only, all 118 files; every recorded observation is byte-identical. Re-recorded from c57de48 in a separate worktree so the goldens stay attributable to pre-migration product source — recording from this branch would have made the parity claim circular. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…ail closed
The matrix picked its driven request from a hardcoded prefix list and `continue`d
past any family the list did not name. That was 10 of 23 families — every one the
source-control migration added — with no red test to say so, which is why that
migration's mutation evidence came down to single hand-written scenarios.
`replyMatrixSites` now takes every completion step in a family's base scenario:
61 sites instead of 13, one golden per site, no judgement about which request is
the "real" one and nothing to edit when a domain is added. A family that scripts
no reply throws, a repeated request name throws, and a census test asserts every
family in the manifest has a matrix. A variant's downstream replies are marked
`optional` and answered only if the request is outstanding, so a diverged reply
that ends the chain records the truth instead of failing on an unsent request.
The `normal` partition replays the first fulfilled reply the family records for
that request, rather than a payload the test file invented per family. Absent and
null do not count — each is already a partition — so four sites with no other
recorded success are inventoried in REPLY_MATRIX_NORMAL_RESULT_INVENTORY with a
reason each, and an entry whose family later records a success fails.
Two partitions added: a refusal and a transport rejection with no message. That
is the axis that separates a refusal falling back to the screen's copy from a
transport drop surfacing its empty message verbatim; without it the two paths
produce the same text and collapsing them is invisible. Every source-control
family carried a hand-written `*-empty-message` scenario for exactly that.
13 hand-written scenarios the matrix now covers are deleted: 8 `*-empty-message`
cases plus sc-history-rejected, sc-commit-message-null-result, sc-eligibility-
refused, sc-create-stops-on-push-refusal and sc-base-ref-rejected. Kept, with
reasons, are the ones the matrix cannot reach: a different action or action args
(sc-review-commit-*, sc-prefill-*, sc-create-{refused,rejected}-empty-message,
sc-prerequisite-{publish,force-with-lease,skipped}), a payload shape rather than
an envelope shape (sc-review-status-entries-not-array, sc-create-existing-review),
and multi-request combinations (sc-base-ref-{unavailable,repo-fallback},
sc-reveal-timeout).
Goldens: 118 -> 153. All 92 survivors changed by their `recorderSha256` line only;
no recorded observation moved. Re-recorded from the pinned baseline in a separate
tree so the goldens stay attributable to pre-migration product source.
`matrix-hostedreview.create-intent-git.commit-1` fails on this branch, and it is
a true positive: `hostReplyErrorTextOrFallback` stringifies a non-string in-band
host error where main returned `result?.error || fallback` and passed the object
through. Left failing — the fix is a product change, documented in the README.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
The expanded reply matrix caught a real divergence the nine original partitions
missed. Main returned `result?.error || 'Commit failed'`, passing a truthy
non-string straight through under a `string` annotation; the migrated helper
stringified it to "[object Object]".
Stringifying is arguably better — downstream does `result.error.replace(...)`,
which throws on an object and merely looks ugly on a string. But this migration's
contract is that no behaviour changes, and shipping an unannounced improvement
inside a refactor is exactly what the parity evidence exists to prevent. Restores
the pass-through; the latent throw is its own ticket.
No host sends this today (`git.commit` is typed `{success, error?: string}`), but
nothing validates it and mixed client/host versions are normal.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Golden conflicts resolved with --ours: this branch's goldens carry the reply-matrix content from 1c06ea3, and main's side of every conflict only bumped recorderSha256. Main's own recorder change (inverting two guards in family-recordings.test.ts and pilot-recordings.test.ts) moves recorderSha256 again, so the goldens are stale at this commit and the next one re-records them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Main inverted two guards in family-recordings.test.ts and pilot-recordings.test.ts. No behaviour change, but both files are inside recorderSha256, so all 153 goldens failed the header check after the merge. Re-recorded from 16d1ab8 in a separate worktree carrying main's product source and this branch's merged recorder, so the goldens still capture main's behaviour rather than the migration's. `baseline` moves from 7ce8e18 to 16d1ab8 because main touched src/shared/skills*.ts, which the record guard compares; that change moved no recorded observation. Every field except `baseline` and `recorderSha256` is byte-identical across all 153 files. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
Too many files changed for review (204 files, 100 file limit). Bypass the limit by tagging |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (156)
🚧 Files skipped from review as they are similar to previous changes (90)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change migrates mobile source-control flows from raw RPC calls to typed deferred operations. It adds shared readers, mutation operations, sender contracts, refusal handling, and source-control adapters. It expands RPC recording matrices to cover message-less failures, optional downstream calls, and normal-result inventories. It adds source-control and hosted-review scenarios and golden recordings, and refreshes existing golden metadata hashes. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No actionable current-head risk remains from the reviewed changes. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the migration, reply-matrix changes, parity results, follow-up work, and test gates. However, it omits required template sections, including the Linked Issue, Visual Proof or an explicit N/A explanation, completed Testing checklist, AI Disclosure, Review, Agent skill boundary, Notes, and Checklist. Resolution Add a linked issue number, complete the required template sections, provide Visual Proof or write N/A with a reason, document testing platforms and commands, add the AI Disclosure if applicable, complete the Review and Agent skill boundary sections, address the Notes items, and mark the Checklist items. Full details: Docstring CoverageExplanation Docstring coverage is 34.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 73 functions across 36 files. (145 skipped: 145 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ℹ️ No critical issues — two minor test-hardening suggestions inline.
Reviewed changes
This run reviewed the full source-control migration and the reply-matrix harness rewrite.
- Source-control migration to typed operations. 13/14 files now send through
bindDeferredRpcOperation(mobile-git-read-operations.ts,mobile-git-mutation-operations.ts,mobile-hosted-review-operations.ts,mobile-repo-base-ref-operations.ts,mobile-worktree-metadata-operations.ts,mobile-source-file-open-operations.ts), with the raw refusal read in one place (readMobileGitRefusal/isMobileGitUnavailableReply) and two new transport helpers,rpc-reader-payload.tsandrpc-refusal-message.ts. - Two readers for
git.status.gitStatusHostPayloadReadpublishes the host payload verbatim;gitStatusProjectionReadkeeps the normalized projection hosted-review preparation has always read. Called out as deliberate because the projection is not a superset. - Reply-matrix harness drives every scripted reply.
replyMatrixSitesreplaces the hardcoded prefix list;optional+ScriptedRpcTransport.outstandinganswer downstream replies only if the operation actually sent them;REPLY_MATRIX_NORMAL_RESULT_INVENTORYcovers sites with no recorded success; newouter-refused-no-message/transport-rejection-no-messagepartitions. - Goldens and inventory. 118 goldens over 78 scenarios, re-recorded from the pinned
baselinewith this branch's recorder;unvalidated-rpc-request-port-inventory.tsdrops the 13 migrated files.
I checked parity call-site by call-site against the removed lines: no behavioral divergence for refusal (with/without message), transport rejection (with/without message), malformed/absent/null result, or in-band {ok:false}/{success:false} envelopes. refusedRpcMessageOrFallback matches main's error?.message || fallback because requireRpcResultOrThrowMessage throws new Error(response.error.message), and hostReplyErrorTextOrFallback deliberately preserves main's truthy-non-string pass-through.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Making the reply matrix fail closed took the goldens from 118 files / 1.53 MB to 153 / 5.35 MB, because a per-site golden replays the chain across 11 reply partitions and every checkpoint's sender, payloads, settlements and effects re-state the whole history that came before them. Format version 2 pooled those fields whole, so the shared prefix was stored once per checkpoint, and once per partition again. Version 3 pools each entry of a list or map field instead. `golden-value-pool.ts` declares the container per field rather than sniffing it from the value, so a projection that changes one fails loudly instead of silently switching encodings. 153 files / 5.35 MB becomes 153 / 2.78 MB; the family that drove this, hostedReview.create-intent, 2.0 MB over 12 sites becomes 792 KB. This is a re-encoding, not a re-observation. Every one of the 153 goldens resolves to the recording its version 2 file resolved to, checked field by field, and every header field except recorderSha256 and goldenFormatVersion is byte-identical. The three mutations this branch's coverage rests on fail exactly as before: the gitStatusProjectionRead acceptance policy 16 (13 matrix, 3 hand-written), interpret inside the request chain 5 (all matrix), and the rewrapped transport rejection 4 (all matrix). It also makes diffs smaller, which is the opposite of what version 2's note predicted when it rejected this. Adding a timeoutMs to the first git.status of the create-intent chain touches the same 16 goldens either way, but version 2 moves 17,100 lines / 1.03 MB and version 3 moves 3,764 / 0.20 MB, because a changed entry no longer rewrites every field value containing it. `readGolden` now also refuses a pool entry that does not hash to its own key, and one no checkpoint reads. Content addressing is what makes an entry shared between checkpoints safe to share; an unreferenced entry would be content in the file that nothing compares. Recorded from 16d1ab8 with this branch's recorder laid over it, per the README's flow. The record fence is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: b473521e-275f-4df7-822e-a0450f5c35bc
📒 Files selected for processing (157)
mobile/rpc-foundation/goldens/b1.jsonmobile/rpc-foundation/goldens/b2.jsonmobile/rpc-foundation/goldens/b3.jsonmobile/rpc-foundation/goldens/interruptions-inventory-lifecycle.jsonmobile/rpc-foundation/goldens/interruptions-settings-bot-overrides-fulfilled.jsonmobile/rpc-foundation/goldens/inventory-lifecycle.jsonmobile/rpc-foundation/goldens/inventory-repeat-query.jsonmobile/rpc-foundation/goldens/lifecycle-b3.jsonmobile/rpc-foundation/goldens/lifecycle-inventory-lifecycle.jsonmobile/rpc-foundation/goldens/lifecycle-settings-bot-overrides-fulfilled.jsonmobile/rpc-foundation/goldens/lifecycle-settings-task-hydration-fulfilled.jsonmobile/rpc-foundation/goldens/lifecycle-settings-workspace-context-fulfilled.jsonmobile/rpc-foundation/goldens/matrix-git.base-ref-chain-repo.baserefdefault-1.jsonmobile/rpc-foundation/goldens/matrix-git.base-ref-chain-repo.list-1.jsonmobile/rpc-foundation/goldens/matrix-git.base-ref-chain-worktree.show-1.jsonmobile/rpc-foundation/goldens/matrix-git.commit-message-ai-git.generatecommitmessage-1.jsonmobile/rpc-foundation/goldens/matrix-git.history-read-git.history-1.jsonmobile/rpc-foundation/goldens/matrix-git.remote-prerequisite-git.push-1.jsonmobile/rpc-foundation/goldens/matrix-git.review-preparation-git.status-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-chain-git.push-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-chain-hostedreview.create-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-chain-worktree.set-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.bulkstage-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.commit-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.generatecommitmessage-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.push-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.status-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.status-2.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.status-3.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.status-4.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-hostedreview.create-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-hostedreview.getcreationeligibility-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-hostedreview.getcreationeligibility-2.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-worktree.set-1.jsonmobile/rpc-foundation/goldens/matrix-hostedreview.eligibility-hostedreview.getcreationeligibility-1.jsonmobile/rpc-foundation/goldens/matrix-legacy-inventory-files.searchpaths-1.jsonmobile/rpc-foundation/goldens/matrix-legacy-inventory-files.searchpaths-2.jsonmobile/rpc-foundation/goldens/matrix-legacy-inventory-fresh-inventory.jsonmobile/rpc-foundation/goldens/matrix-legacy-inventory-old-inventory.jsonmobile/rpc-foundation/goldens/matrix-linear-detail-barrier-linear.getissue-1.jsonmobile/rpc-foundation/goldens/matrix-linear-detail-barrier-linear.issuecomments-1.jsonmobile/rpc-foundation/goldens/matrix-project-explicit-false-github.project.updateissuebyslug-1.jsonmobile/rpc-foundation/goldens/matrix-session.tab-reveal-session.tabs.activate-1.jsonmobile/rpc-foundation/goldens/matrix-session.tab-reveal-session.tabs.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings-agent-read-preflight.detectremoteagents-1.jsonmobile/rpc-foundation/goldens/matrix-settings-agent-read-repo.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings-agent-read-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings-best-effort-settings.update-1.jsonmobile/rpc-foundation/goldens/matrix-settings.bot-overrides-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.home-providers-linear.status-1.jsonmobile/rpc-foundation/goldens/matrix-settings.home-providers-preflight.check-1.jsonmobile/rpc-foundation/goldens/matrix-settings.home-providers-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.repo-metadata-host.platform-1.jsonmobile/rpc-foundation/goldens/matrix-settings.repo-metadata-repo.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings.repo-metadata-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.repo-metadata-ssh.listtargetsummaries-1.jsonmobile/rpc-foundation/goldens/matrix-settings.resume-metadata-folderworkspace.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings.resume-metadata-projectgroup.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings.resume-metadata-repo.list-1.jsonmobile/rpc-foundation/goldens/matrix-settings.resume-metadata-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.resume-metadata-worktree.ps-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-hydration-linear.status-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-hydration-preflight.check-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-hydration-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-hydration-status.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-hydration-ui.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.task-workspace-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.workspace-context-linear.status-1.jsonmobile/rpc-foundation/goldens/matrix-settings.workspace-context-preflight.check-1.jsonmobile/rpc-foundation/goldens/matrix-settings.workspace-context-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.workspace-context-ui.get-1.jsonmobile/rpc-foundation/goldens/matrix-settings.workspace-submit-settings.get-1.jsonmobile/rpc-foundation/goldens/matrix-worktree.review-link-worktree.set-1.jsonmobile/rpc-foundation/goldens/probe-new-tab-both-refused.jsonmobile/rpc-foundation/goldens/probe-new-tab-null-sibling-refused.jsonmobile/rpc-foundation/goldens/probe-new-tab-refused-sibling-rejects.jsonmobile/rpc-foundation/goldens/probe-new-tab-rejects-sibling-refused.jsonmobile/rpc-foundation/goldens/sc-base-ref-default.jsonmobile/rpc-foundation/goldens/sc-base-ref-repo-fallback.jsonmobile/rpc-foundation/goldens/sc-base-ref-unavailable.jsonmobile/rpc-foundation/goldens/sc-base-ref-worktree-hit.jsonmobile/rpc-foundation/goldens/sc-commit-message-cancel-rejected.jsonmobile/rpc-foundation/goldens/sc-commit-message-canceled.jsonmobile/rpc-foundation/goldens/sc-commit-message-generated.jsonmobile/rpc-foundation/goldens/sc-create-existing-review.jsonmobile/rpc-foundation/goldens/sc-create-intent-stage-commit-push-create.jsonmobile/rpc-foundation/goldens/sc-create-link-failure-is-non-fatal.jsonmobile/rpc-foundation/goldens/sc-create-pushes-then-creates.jsonmobile/rpc-foundation/goldens/sc-create-refused-empty-message.jsonmobile/rpc-foundation/goldens/sc-create-rejected-empty-message.jsonmobile/rpc-foundation/goldens/sc-eligibility-fetched.jsonmobile/rpc-foundation/goldens/sc-history-loaded.jsonmobile/rpc-foundation/goldens/sc-pr-link-hosted-review.jsonmobile/rpc-foundation/goldens/sc-pr-link-read.jsonmobile/rpc-foundation/goldens/sc-pr-link-set.jsonmobile/rpc-foundation/goldens/sc-prefill-unavailable-on-refusal.jsonmobile/rpc-foundation/goldens/sc-prefill-unavailable-on-rejection.jsonmobile/rpc-foundation/goldens/sc-prerequisite-force-with-lease.jsonmobile/rpc-foundation/goldens/sc-prerequisite-publish.jsonmobile/rpc-foundation/goldens/sc-prerequisite-push.jsonmobile/rpc-foundation/goldens/sc-prerequisite-skipped.jsonmobile/rpc-foundation/goldens/sc-reveal-first-poll.jsonmobile/rpc-foundation/goldens/sc-reveal-timeout.jsonmobile/rpc-foundation/goldens/sc-review-commit-inner-failure.jsonmobile/rpc-foundation/goldens/sc-review-commit-refused-empty-message.jsonmobile/rpc-foundation/goldens/sc-review-commit-rejected.jsonmobile/rpc-foundation/goldens/sc-review-commit.jsonmobile/rpc-foundation/goldens/sc-review-status-entries-not-array.jsonmobile/rpc-foundation/goldens/sc-review-status-normalized.jsonmobile/rpc-foundation/goldens/schedules-b3.jsonmobile/rpc-foundation/goldens/schedules-settings-home-providers-fulfilled.jsonmobile/rpc-foundation/goldens/schedules-settings-new-tab-ssh.jsonmobile/rpc-foundation/goldens/schedules-settings-repo-metadata-fulfilled.jsonmobile/rpc-foundation/goldens/schedules-settings-resume-metadata-fulfilled.jsonmobile/rpc-foundation/goldens/schedules-settings-task-hydration-fulfilled.jsonmobile/rpc-foundation/goldens/schedules-settings-workspace-context-fulfilled.jsonmobile/rpc-foundation/goldens/settings-bot-overrides-fulfilled.jsonmobile/rpc-foundation/goldens/settings-bot-overrides-refresh-refused.jsonmobile/rpc-foundation/goldens/settings-bot-overrides-refused.jsonmobile/rpc-foundation/goldens/settings-bot-overrides-transport-error.jsonmobile/rpc-foundation/goldens/settings-home-coalesced.jsonmobile/rpc-foundation/goldens/settings-home-providers-fulfilled.jsonmobile/rpc-foundation/goldens/settings-home-providers-refuse-after-data.jsonmobile/rpc-foundation/goldens/settings-home-providers-refused.jsonmobile/rpc-foundation/goldens/settings-home-providers-transport-error.jsonmobile/rpc-foundation/goldens/settings-new-tab-refused.jsonmobile/rpc-foundation/goldens/settings-new-tab-ssh.jsonmobile/rpc-foundation/goldens/settings-new-tab-transport-error.jsonmobile/rpc-foundation/goldens/settings-repo-cache-expiry.jsonmobile/rpc-foundation/goldens/settings-repo-metadata-fulfilled.jsonmobile/rpc-foundation/goldens/settings-repo-metadata-refuse-after-data.jsonmobile/rpc-foundation/goldens/settings-repo-metadata-refused.jsonmobile/rpc-foundation/goldens/settings-repo-metadata-single-host.jsonmobile/rpc-foundation/goldens/settings-repo-metadata-transport-error.jsonmobile/rpc-foundation/goldens/settings-resume-metadata-fulfilled.jsonmobile/rpc-foundation/goldens/settings-resume-metadata-refuse-after-data.jsonmobile/rpc-foundation/goldens/settings-resume-metadata-refused.jsonmobile/rpc-foundation/goldens/settings-resume-metadata-transport-error.jsonmobile/rpc-foundation/goldens/settings-task-hydration-fulfilled.jsonmobile/rpc-foundation/goldens/settings-task-hydration-refuse-after-data.jsonmobile/rpc-foundation/goldens/settings-task-hydration-refused.jsonmobile/rpc-foundation/goldens/settings-task-hydration-transport-error.jsonmobile/rpc-foundation/goldens/settings-task-workspace-fulfilled.jsonmobile/rpc-foundation/goldens/settings-task-workspace-refused.jsonmobile/rpc-foundation/goldens/settings-task-workspace-transport-error.jsonmobile/rpc-foundation/goldens/settings-task-write.jsonmobile/rpc-foundation/goldens/settings-workspace-context-fulfilled.jsonmobile/rpc-foundation/goldens/settings-workspace-context-refuse-after-data.jsonmobile/rpc-foundation/goldens/settings-workspace-context-refused.jsonmobile/rpc-foundation/goldens/settings-workspace-context-transport-error.jsonmobile/rpc-foundation/goldens/settings-workspace-submit-fulfilled.jsonmobile/rpc-foundation/goldens/settings-workspace-submit-refused.jsonmobile/rpc-foundation/goldens/settings-workspace-submit-transport-error.jsonmobile/src/test-support/rpc-recording/README.mdmobile/src/test-support/rpc-recording/golden-recording.tsmobile/src/test-support/rpc-recording/golden-value-pool.tsmobile/src/test-support/rpc-recording/recording-runner.test.ts
💤 Files with no reviewable changes (27)
- mobile/rpc-foundation/goldens/matrix-settings.task-hydration-settings.get-1.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.bulkstage-1.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.status-4.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.generatecommitmessage-1.json
- mobile/rpc-foundation/goldens/matrix-settings.task-hydration-linear.status-1.json
- mobile/rpc-foundation/goldens/matrix-settings.repo-metadata-settings.get-1.json
- mobile/rpc-foundation/goldens/sc-create-intent-stage-commit-push-create.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-worktree.set-1.json
- mobile/rpc-foundation/goldens/matrix-legacy-inventory-fresh-inventory.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-git.push-1.json
- mobile/rpc-foundation/goldens/matrix-git.base-ref-chain-repo.list-1.json
- mobile/rpc-foundation/goldens/matrix-legacy-inventory-files.searchpaths-2.json
- mobile/rpc-foundation/goldens/matrix-settings.task-hydration-preflight.check-1.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-hostedreview.getcreationeligibility-2.json
- mobile/rpc-foundation/goldens/matrix-hostedreview.create-intent-hostedreview.create-1.json
- mobile/rpc-foundation/goldens/matrix-settings.task-hydration-ui.get-1.json
- mobile/rpc-foundation/goldens/matrix-settings.repo-metadata-ssh.listtargetsummaries-1.json
- mobile/rpc-foundation/goldens/matrix-settings.resume-metadata-projectgroup.list-1.json
- mobile/rpc-foundation/goldens/matrix-settings.task-hydration-status.get-1.json
- mobile/rpc-foundation/goldens/matrix-settings.resume-metadata-folderworkspace.list-1.json
- mobile/rpc-foundation/goldens/matrix-settings.resume-metadata-settings.get-1.json
- mobile/rpc-foundation/goldens/matrix-legacy-inventory-old-inventory.json
- mobile/rpc-foundation/goldens/matrix-settings.resume-metadata-repo.list-1.json
- mobile/rpc-foundation/goldens/matrix-legacy-inventory-files.searchpaths-1.json
- mobile/rpc-foundation/goldens/matrix-settings.repo-metadata-host.platform-1.json
- mobile/rpc-foundation/goldens/matrix-settings.resume-metadata-worktree.ps-1.json
- mobile/rpc-foundation/goldens/matrix-git.base-ref-chain-worktree.show-1.json
🚧 Files skipped from review as they are similar to previous changes (2)
- mobile/rpc-foundation/goldens/sc-prerequisite-force-with-lease.json
- mobile/src/test-support/rpc-recording/README.md
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
Three review findings, all in the recorder, none in product code. The family census pushed every family unconditionally, so it could never differ from the manifest keys; it now records a family only when a site generated a test, which is independent of replyMatrixSites throwing on an empty list. REPLY_MATRIX_NORMAL_RESULT_INVENTORY was only consulted for a live site, so a stale entry retired silently; a new assertion fails on any entry that names no live (family, request). Both verified by mutation: an empty site list and a renamed inventory request each fail the suite. The value pool resolves hashes with Object.hasOwn so a malformed golden cannot read an inherited key. Re-recorded from 16d1ab8 with this recorder laid over main's product source, per the README. All 153 goldens move on recorderSha256 only. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the two commits landed since the prior pullfrog review (4a3e796d93): the golden-format v3 re-encoding and the test-hardening follow-up that closes both prior inline findings.
- Golden format v3 — per-entry interning.
golden-value-pool.tsnow declares each observation field's container (FIELD_SHAPES:sender/payloads/effectslist,settlementsmap,statewhole) and interns each entry of a list/map field rather than the whole field, so an append-only history is stored once per file instead of once per checkpoint and once per reply partition. A field recorded with the wrong container fails loudly. - Pool integrity on resolve.
resolveRecordingverifies every pool entry hashes to its own key and rejects entries no checkpoint reads;readGoldenhard-fails on anygoldenFormatVersion !== 3, so an old file re-records rather than comparing. - Census and inventory can now fail. The
matrixed/liveSitessets infamily-recordings.test.tsare filled only when a site generates a test, and a new assertion requires everyREPLY_MATRIX_NORMAL_RESULT_INVENTORYentry to name a live(family, request)site — closing the stale-entry gap flagged previously. - Own-property pool lookup.
hash in valuesbecameObject.hasOwn(values, hash). - Goldens re-recorded for the new recorder digest and format version; only
recorderSha256andgoldenFormatVersionmoved, no observation changed.
Both prior inline threads were verified addressed and resolved. I read the v3 intern/resolve code and its new runner tests directly: empty lists contribute no pool entries, state stays whole, the orphan check only fires on dead file content, and the four inventoried sites all map to live matrix sites.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
This was referenced Sep 14, 2026
zpyoung
added a commit
to zpyoung/orca
that referenced
this pull request
Sep 16, 2026
* fix(native-chat): wait for the runtime capability probe before resolving the creation launch route (#19819)
* fix(native-chat): wait for the runtime capability probe before resolving the launch route
A worktree created before the renderer's hydration-gated capability refresh
runs read the local capability set as null, which
resolveStructuredNativeChatSupport treats as a blocker, silently degrading
structured native chat to the legacy terminal-backed route. Creation submits
now await ensureLocalRuntimeCapabilities(), which probes the local runtime
when no answer has landed yet, so the route resolves on an actual answer.
Fixes #19154
* fix(native-chat): await the capability probe in the work-item direct launch route too
prepareDirectWorkItemAgentLaunch is the fourth creation-flow route owner and
already async; a pre-hydration submit-after-ready launch (fix-checks) read the
unprobed cache as unsupported and silently degraded to legacy. Draft-delivery
launches were unaffected (draft-prompt blocks structured before the capability
check). Same shape as the three creation-submit sites.
* fix(native-chat): keep the capability probe starting synchronously
The broken-bridge hardening wrapped the probe in Promise.resolve().then(...),
which deferred window.api.runtime.getStatus() by a microtask. The session-tabs
restore deliberately overlaps its inventory RPC with this refresh and relies on
the probe already being in flight when refresh returns, so the deferral broke it.
The bridge call is synchronous again; a synchronous throw becomes a rejection
instead, which is what the wrapper was actually for.
* fix(native-chat): hydrate local runtime capabilities at renderer boot
The capability cache's only writer was `useLocalStructuredSessionTabsSync`,
gated on workspaceSessionReady + terminalStartupRestorationReady + the
experimental flag. Every `resolveAgentLaunchRoute` reader treats an
unanswered cache as "unsupported", so the answer arriving seconds late is
what produces the bare-terminal create in #19154 — awaiting the probe at a
route decision guards four call sites but leaves the window open for the
three readers that are synchronous and cannot await.
Start the probe from the renderer boot chain, ungated, so the answer is
cached before any launch route is resolved. The per-call-site awaits stay
as the backstop for the residual window and for re-probing after a failed
probe.
Also: hoist the full-creation probe above its cancel gate so the gate stays
adjacent to createWorktree; pin the retry-after-failure, concurrent-ensure
and missing-bridge contracts; drop a stale microtask tick and correct two
comments that no longer described the code.
* test(native-chat): pin the cancel gate around the capability probe
The probe added an await to two composer creation paths. Full creation had
no gate between the route decision and createWorktree, so the earlier
revision opened a window where a dismissed composer still created a
worktree; the hoist that closed it was unpinned. Quick creation already
gated immediately before runBackgroundWorktreeCreation, so its inline
await is safe — pin that too, since nothing asserted it.
Both tests fail against origin/main (no probe) and the full-creation one
fails against the pre-hoist revision.
* fix(native-chat): close the folder-create cancel window the probe opened
The probe added the first `await` inside `submitFolderWorkspaceCreate`. On
`main` that function ran straight through to `createFolderWorkspace` with no
suspension of its own, so its caller's `isSubmissionCancelled()` gate and the
create call sat in the same turn. With the probe inline, a composer dismissed
while the probe is in flight still creates the folder workspace and launches
an agent — the same defect the full-creation hoist fixed on the git path.
Resolve capabilities in `folder-submit-orchestration` above its existing gate
and hand them down, so the create path's prefix is synchronous again. The
parameter stays optional: a caller without a cancel gate keeps the probe.
Both new tests fail against `origin/main` and against this branch's previous
head; the cancel-window one still fails with its probe-pending assertion
removed, so it pins the create, not just the probe.
* refactor(native-chat): require pre-resolved capabilities on the folder create path
The cancel-window fix in f492064432 left its invariant -- a caller that gates
on cancellation must resolve capabilities above its gate -- enforced only by a
comment, because `hostCapabilities` stayed optional with an inline probe as the
fallback. A future caller that owns a cancel gate and forgets the parameter
would silently reopen the window twice fixed already, and nothing would catch
it: the caller census test pins `resolveAgentLaunchRoute` callers, not this
function's, and `exactOptionalPropertyTypes` is off so even an explicit
`undefined` is legal.
Make it required and drop the now-unreachable inline probe. The sole
production caller already passes it, so runtime behaviour is unchanged: the
old ternary never evaluated its `await` when a value was supplied.
`null` keeps its meaning -- probed, genuinely unknown -- and still degrades to
the legacy route; only absence becomes impossible. The launch-route test that
covered the removed probe is replaced by one pinning that `null` contract with
the cache and the bridge both holding the structured capability, so only the
handed-in value can produce the legacy outcome. The cases in the sibling suite
are not about the route, so they go through one typed wrapper that supplies the
unknown answer rather than repeating it 21 times.
---------
Co-authored-by: Merge Sim <sim@local>
* fix(worktrees): retire the chat tab of a chat with no child when its workspace goes (#19970)
* fix(worktrees): retire the chat tab of a chat with no child when its workspace goes
Deleting a workspace left a chat tab behind for every structured session that
had no attached provider child at the time, and that tab came back at the next
launch pointing at a workspace that no longer exists.
A provider child is scoped to a VISIBLE pane, not to a tab: the hold that keeps
one is `enabled: isVisible && isWorktreeActive`, and dropping the last hold
evicts the child after the release grace. So the sweep's liveness predicate
selected only "the chat that is the visible pane in the active workspace, or was
moments ago" — which means deleting a workspace from the sidebar while a
different one is active left every chat in the target invisible to the sweep,
and the `live.length === 0` early return did nothing at all.
The durable reference is `visibleSessionIds` in the agent-session record store.
Both purges a removal already performs miss it: the renderer drops
`unifiedTabsByWorktree` and the main process drops the workspace metadata, and
neither touches that index. Startup replays it, restores the session from it and
republishes the tab. Worktree ids are path-derived, so a later workspace created
at the same path inherits the old chat.
Splits the two concerns the sweep conflated in one list. Liveness still decides
what to CLOSE and what to refuse over, unchanged. Membership — the same fenced
record filter minus the liveness clause — decides what to RETIRE, and covers
exactly the complement of the close list so each session's tab is handled once.
Retirement runs from `killAllProcessesForWorktree`, past every point that can
refuse, not from the structured sweep itself: that sweep is joined BEFORE the
unstopped-PTY verdict so a structured refusal can outrank a terminal one, and a
tab retired there would still be ahead of a gate that can refuse the whole
removal — leaving the workspace in place with its chats gone.
* fix(worktrees): retire tabs across all teardown outcomes
* test(worktrees): type teardown fixtures
---------
Co-authored-by: Merge Sim <sim@local>
* feat(ai-vault-search): public session search contract and transports (#20277)
* feat(ai-vault-search): define public contract and service seam
* feat(ai-vault-search): add IPC runtime relay and web transports
* fix(ai-vault-search): register search IPC at the core handler site
ai-vault.ts was two lines over the 300-line max-lines limit; the search
handlers belong with the other register*Handlers calls anyway.
* fix(ai-vault-search): withhold degraded-root paths from relay status
Status carried local filesystem paths over the relay while hits redact
theirs. redactStatusForTransport applies the same policy at the same
boundary: relay callers keep each root's reason and the array length as
the count, so the type only makes root optional.
* fix(ai-vault-search): close diagnostic path leak and remove test casts
* feat(ai-vault-search): carry an execution host id and per-host outcomes on hits
* feat(ai-vault-search): route desktop search by execution host scope, including runtimes
* feat(preload): accept an execution host scope on session search
* feat(web): answer only for the paired runtime on session search
* docs(ai-vault-search): describe execution-host routing and the all-hosts merge
* test(ai-vault-search): cover every host scope, the all-hosts merge and wire compat
* fix(ai-vault-search): resume every host mid-page so a merged page never drops a hit
* fix(ai-vault-search): decode the merged cursor with a schema instead of casts
CI's type-aware audit refuses type assertions; a zod record validates the
per-host entries and yields the typed map without one.
* refactor(ai-vault-search): defer cross-host merged search
* 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
* 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>
* 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
* fix(runtime): reject malformed file Base64 padding (#20283)
Require padded file-write payloads to end on a Base64 quartet boundary. Cover both RPC methods and padded final upload chunks, and document client compatibility evidence.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(tasks): preserve repository results under GitHub search quota (#20460)
* fix(tasks): preserve repository results under GitHub search quota
* fix(github): preserve search budget on count fallback
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
* fix(ai-vault): stream oversized remote session transcripts (#20455)
* fix(ai-vault): stream oversized remote session transcripts
* fix(build): bundle streamed JSON parser in desktop main
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
* fix(grok): stop replayed Claude/Cursor hooks reporting Grok panes as Claude (#20507)
* fix(grok): stop replayed Claude/Cursor hooks reporting Grok panes as Claude
Grok's hook discovery reads ~/.claude/settings.json (and the Cursor equivalent)
for vendor compatibility, and that is on by default. So inside every Grok pane
Orca's managed Claude hook fires in addition to Orca's managed Grok hook, and
both POST the same Grok envelope. The Claude-routed copy lands last and wins, so
the pane's agent type is resolved from the POST route as "claude" and no
Grok-specific normalization runs for it.
Guard the managed Claude and Cursor scripts on GROK_HOOK_EVENT, which Grok's hook
runner stamps into every hook subprocess it spawns — including replayed vendor
configs — after any user-supplied environment, so a hook cannot spoof it. This
mirrors the existing DEVIN_PROJECT_DIR guard in the same script, which solves the
identical problem for another agent that imports Claude hooks.
Placement is load-bearing: the guard sits after the stdin capture, so Grok's
writer never blocks, and before both the spool write and the HTTP POST, so a
replayed event cannot leave a spool entry that replays later. The Windows
variants jump to the stdin-drain label rather than exiting, because abandoning
stdin there hangs the writer.
The guard is scoped to agent === 'claude'; OpenClaude reuses ClaudeHookService
with its own settings file, which Grok does not replay, so it is unaffected.
Verified live against Grok 1.0.25 in a dev instance: the pane's reported agent
type goes from "claude" to "grok" on every turn-end, including the hidden
follow-up turns Grok runs when background work finishes.
The guard pushed hook-service.ts past the 300-line cap, so the script builder
moves to a sibling hook-script.ts. That mirrors the existing split under
src/main/cursor/, where the service owns install/status and the script module
owns script text.
* fix(agent-hooks): preserve Windows background worker stdin contract
* fix(browser): press keys through a US-layout CDP key table instead of a subprocess per keystroke (#15310)
Typing in the remote browser pane spawned an agent-browser process per
keystroke -- ~160ms each, so a 17-character password took seconds -- and
some keys arrived half-formed: F-keys, Insert and ContextMenu dispatched
windowsVirtualKeyCode 0, Shift+1 typed '1' instead of '!', and non-ASCII
printables reported success while typing nothing at all.
keypress now resolves the key name through a US-layout table and
dispatches the Input.dispatchKeyEvent pair over the electron debugger,
the same transport mouseClick already uses. Two fallbacks keep the old
behavior reachable:
- a single printable BMP character outside the table dispatches in
process as an IME-style event (keyCode 229 with the character as text,
the shape composed input already has when it reaches pages)
- anything else -- media keys, surrogate pairs, unrecognized names --
goes to the helper exactly as before, and only that path pays for
creating the helper session
Virtual key codes come from the table, never from the character's own
char code: charCodeAt puts '&' on 38 (VK_UP) and '.' on 46 (VK_DELETE),
which Blink runs as caret commands that swallow the character.
Dispatch failures normalize the way evaluate's already do -- a gone page
becomes browser_tab_not_found, anything else browser_error -- because
attach and sendCommand reject with plain Errors that the RPC layer would
report as runtime_error, and the pane only reclaims a dead page when it
sees a browser_* code.
Result shape is unchanged and no wire, schema or RPC surface moves, so
mixed-version client/server pairs see no difference. Pages can observe
the fidelity fixes: Shift+a now types 'A', Shift+1 now types '!',
Alt+<char> no longer carries text, and editing keys arrive as rawKeyDown.
Each matches what a real US keyboard produces.
Verified against the shipped agent-browser 0.27 binary on the same
browser: every difference is a fix, nothing regressed. macOS editing
shortcuts (Cmd+A) do not fire through either path -- Blink runs those off
the native responder chain and neither sends CDP `commands` -- so that
gap is unchanged, not introduced.
Co-authored-by: Neil <neil@stably.ai>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(rpc): validate provider-specific fields in TaskProviderIdentity (#20284)
* fix(rpc): validate task provider identity fields
Validate provider-specific field types while preserving nullable scopes and unknown identity fields. Record the producer census and pin validation with regression tests.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(rpc): reject a blank GitHub owner or repo
normalizeTaskProviderIdentity treats a blank owner or repo as no identity at
all, but the schema accepted '' and whitespace-only, so the two disagreed about
the same payload. Refined rather than trimmed: trimming would rewrite the
parsed value and change what the handler receives.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(rpc): re-measure the identity evidence counts
The blank-field commit added seven tests, so the recorded 74/16/58 described the
commit before it. Re-ran both: 81 tests, and the discriminant-only mutation now
gives 17 failures / 64 passes.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(rpc): correct the remaining stale gate count
The blank-field commit moved the full-RPC total too; 2,463 was the count before
it. Re-ran: 278 files, 2,470 passed, one skipped.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* perf(terminal): batch file-link checks on their owning host (#20463)
* perf(terminal): batch file-link existence checks on their owning host
* test(relay): allow additive filesystem capabilities
* fix(web): keep terminal file links working under batched existence checks
createShellApi omitted pathsExist, so withFallback answered the new batch
call with a truthy proxy resolving to undefined and the whole hover batch
rejected — dropping every link on lines with an out-of-worktree path.
* test(web): assert the shim without type assertions
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
Co-authored-by: Neil <neil@stably.ai>
* feat(native-chat): support file drag and drop (#20494)
* feat(native-chat): support workspace file drops
* fix(native-chat): report OS file drops that attach nothing
#15782 is a silent failure on the Finder route, and that route still
swallowed every way it could fail:
- the preload handler returned with no feedback when the OS handed us
file items `webUtils.getPathForFile` could read no path from (promised
or virtual files). It now sends the existing `rejected` payload with a
new `unresolved-paths` reason, which the global drop toast names.
- the composer's external-attach path dropped the batch with no notice
when every path failed authorization, when an upload came back empty,
and (new in this branch) when the owner changed mid-flight. Each exit
now sets a notice; only a disabled composer stays quiet, because it has
no notice surface.
Also stops `resolveNativeChatAttachmentOwnerForWorktree` throwing out of a
drop/IME handler when an SSH connection's generation is gone mid-attach —
that is an unknown owner, which the resolver already models as
`not-ready`.
* refactor(native-chat): one owner-identity check for composer attachments
The branch had two near-identical "is this still the same owner" helpers,
one per attach route, and they disagreed: the workspace-drop copy ignored
the SSH connection generation, so a reconnect between the drop and the IME
flush read as the same owner and the path landed on a new connection.
Collapses both onto one predicate in the pure ownership module (the
store/toast-free seam both routes already depend on), which compares the
full SSH expectation and never treats `not-ready` as a match.
* perf(file-explorer): resolve drag ownership at dragstart, not per render
The virtualized row list resolved the selection's source execution host on
every render — the virtualizer re-renders on every scroll frame, so a large
multi-selection paid a full projection scan plus a route allocation per
selected path per frame, and per visible row on top of that. Only
`onDragStart` ever read the result.
Rows now receive a resolver they call with the paths they are about to
drag. The three copies of the "stamp only if both halves resolve" guard
(explorer row, both combined-diff row shapes) collapse into one helper next
to the writer.
* fix(native-chat): refuse a guarded composer drop visibly
The drop handlers claimed the drag (preventDefault + stopPropagation) before
checking `disabled`, so a guarded composer told the browser it accepted the
drop, left the copy cursor up, and then did nothing — the same silent swallow
this branch exists to remove.
Dragover now answers `none` when the composer is guarded, so the cursor refuses
and no drop event follows. It still claims the event either way: the composer
sits inside the terminal surface, which accepts the same drag and would paste
the paths into the shell instead.
Drops `stopImmediatePropagation`. The capture-phase `stopPropagation` already
keeps the event off the editor below, so the stronger form only risked
suppressing unrelated listeners on the React root.
The fake DataTransfer in the test now starts at a dropEffect we never write, so
asserting `none` or `copy` proves the handler set it.
* fix(native-chat): decide attachment ownership per path, not per batch
A queued batch can mix sources — a workspace drop the target host owns and a
client-local paste it cannot read — because IME composition holds both until it
settles. Collapsing the batch to one verdict refused the whole thing on a remote
target, including the drop the user was entitled to make.
The verdict now follows the path it belongs to: owned paths attach, client-local
ones are refused, and the refusal is reported rather than dropped. A stale owner
still refuses everything, since that means the target moved under all of them.
Also guards the empty-batch case, which previously read as "every path owned".
* refactor(combined-diff): resolve drag ownership from the live workspace
The combined diff captured an execution host into the open-file record at tab
open and drilled it through three components to reach the row. That host was
never persisted, so after a restart every drag from a restored diff was refused
until the tab was reopened, and the capture failure was swallowed into an
undefined source with no trace.
Rows now resolve the owner the same way the source-control rows already do, from
the workspace the diff belongs to at the moment of the drag. That deletes the
prop drilling, the store capture and its bare catch, and leaves one way to
answer "who owns these paths" for every live listing.
The file explorer keeps its per-node owner: its tree is a cache that can still be
showing a previous host's listing, which is exactly what that field records.
* revert(file-explorer): drop the workspace-id tree reset
Resetting and reloading the tree when the workspace id changes at an unchanged
path is not needed for the drag source to be correct. The tree already records
the workspace whose root listing it committed, so a cache left over from a
previous workspace stamps that workspace and the composer refuses the drop —
the intended answer, reached without touching the reset rule.
That rule clears selection, the name filter and undo history, which is more
file-explorer behaviour change than this feature asked for.
* test(native-chat): stop the external-attach mock hiding new notices
The hook's test replaced the whole attachment-owner module with a hand-written
stub, so the two notices added alongside the owner-change guards resolved to
undefined. Calling them threw inside the async attach loop — an unhandled
rejection, which leaves every test in the file reported as passing while the run
as a whole fails. CI caught it; a local run reporting only pass/fail counts does
not.
The mock now spreads the real module, so a notice added later cannot go missing
from it, and both owner-change tests assert the string a user would read instead
of only asserting that nothing attached.
* test(native-chat): guard the last-path owner change on a one-file drop
The owner flipping while the final path is authorizing has no next loop
iteration to catch it, so the post-loop check is all that stands between a
single-file drop and a path attached to a host that no longer owns it — and a
one-file drop is the ordinary shape. No test covered that exit.
Removing the post-loop check now turns this red; before it, only the
multi-path exit was guarded.
* fix(native-chat): keep a mixed attachment batch in attach order
applyResolvedPaths partitioned a queued batch into a target-owned half and
a client-local half and concatenated them. An IME-delayed batch that mixed
a workspace drop with a paste made earlier in the same composition was
therefore inserted owned-first, so the dropped reference jumped ahead of
the pasted one in the draft.
Filter against the two verdicts in place instead. Membership is unchanged,
the order the user attached in survives, and the two intermediate arrays go
away.
* fix(file-explorer): name the owner of a dragged path whose row is hidden
A multi-selection outlives the rows that showed it. Nothing prunes
selectedPaths when a directory collapses, when the name filter narrows, or
when dotfiles are hidden, and the drag still carries every selected path.
Drag-source resolution read those owners from the row projection, which is
built from visible rows only, so one hidden path collapsed the whole drag to
an unstamped one and the composer refused it as coming from another
workspace.
The owner was never unknowable — the dir cache the projection is built from
still records which host listed that path. Fall back to it when the path has
no visible row. A path in neither (a name-filter synthetic node for a
directory that was never listed) still fails closed.
* fix(native-chat): ask which workspace the composer serves now
The IME-flush ownership check compared the workspace id captured when the
drop happened against the same captured value, so for a structured pane the
comparison could only ever hold. The live protection came from the host and
owner checks beside it; this one asked nothing.
Read the id through a ref so the check means what it reads as. A pane whose
structured target moves between the drop and the composition settling now
refuses the queued path instead of attaching it.
* fix(native-chat): ask which workspace an external attach lands on
The post-await ownership gate resolved the owner through the render closure, so
it re-asked the workspace the attach started in and compared the answer with
itself. A tab moved to another workspace mid-authorization passed the gate, and
the paths landed in a composer that no longer served that workspace.
Read the pane through a ref and compare the workspace identity as well as the
owner: two workspaces can both report a local owner, so the owner alone cannot
tell them apart.
* test(native-chat): read the real notice on a workspace drop
The drop tests hand-built their attachment-upload mock and hand-copied the
not-ready wording into it, so the assertion tracked the copy rather than the
string a user reads: rewording the real notice left all 15 tests green.
Spread the real module and override only the owner resolver, matching the two
sibling test files in this directory. Rewording the notice now fails the test.
* docs(native-chat): restore the hook's doc comment to the hook
The workspace comparison landed between the doc block and the function it
describes, leaving the comment attached to a type alias.
* test(native-chat): cover the upload window for a moved pane
The workspace-currency gate guards two windows and only the authorize loop was
covered. The upload window is the longer one: the paths go to the worktree the
attach captured, so a pane that moved workspaces meanwhile must not receive
remote paths living under the workspace it left.
* test(native-chat): pin the two untested attachment refusals
Refusing an already-blocked target at the drop rather than queueing it had no
test: queued paths that can never attach still spend the pending budget, and the
next legitimate drop is then turned away for being one too many.
Also pins the immediate already-false ownership verdict. Today's only caller
settles ownership synchronously so it cannot arrive false, but the hook exports
this entry point and the fallback is not a refusal — a false verdict is not
"owned", so a remote target blames client-local attachments for an ownership
failure. Verified: removing the branch reports the wrong notice.
* docs(native-chat): say which rule the ownership refusal follows
The per-path comment sat directly above the batch-wide ownership refusal while
describing the blocked-target logic below it, so the refusal read as a
contradiction of the line under it rather than as the file's stated rule.
Name the rule at the refusal: a failed ownership verdict refuses the whole
completion, the same way the pending-limit rejection does.
---------
Co-authored-by: Merge Sim <sim@local>
* fix: show unexpected signout notice only once across versions (#20526)
* 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 bcba08b3e4 tree is bare `mobile/src` and `src` with no package.json
and no node_modules, so no bundler-resolved import can reach it and the seed
rejection gate cannot run without it. Direct import also swaps a 38-module lazy
graph for a 338-module eager one behind 20 native mocks, because
`mobile-tasks-dependencies.ts` re-exports from `react-native` and four other
native packages and `export *` enumerates.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): treat the recorded lockfile hash as provenance, not an oracle
Every golden pinned `lockfileSha256`, so any dependency bump on main failed all
61 comparisons on the merge commit while the traces were identical. A dependency
that changes behaviour changes the trace itself; one that does not must not fail
a candidate. `platform` already had this exemption — `lockfileSha256` joins it.
Recording still refuses to run unless the lockfile matches the pinned baseline,
so goldens are still produced under frozen conditions.
Verified against main's lockfile: 85 passed, previously 61 failed. The declared
mutation set still reports every mutant killed.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(rpc): cover repeat queries and settings refresh boundaries
Add three scenarios, preserve existing traces, remove unreachable archived checks, and document observed mutation kills and remaining adapter limitations.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(rpc): keep the known-open holes, drop the review transcript
The audit file was mostly a point-in-time record of mutation runs that had already
happened, in an artifacts directory, where it would go stale on the next scenario
change. The durable part is which holes are still open and why they cannot be
reached, which belongs beside the runner it describes.
Markdown is outside recorderSha256, so no re-record; 88 passed | 3 skipped.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* chore(mobile): stage the nine RPC probe scenarios and goldens
These existed only on one machine's /tmp. Landing them verbatim first so a
reboot cannot lose them; a follow-up commit moves them into the suite.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): fold the nine probe scenarios into the recording oracle
The probes were env-var invocations over loose /tmp manifests. They now live in
pilot-scenarios.json and rpc-foundation/goldens, so `pnpm --dir mobile test` runs
them with no flag to remember.
Re-records every golden against main (22f56f7c2a). Two causes:
- #20280 gave LogicalClientCutoverError the delivery-unknown mark and its cause,
so nine cutover/interruption goldens now record `isRpcDeliveryUnknown: true`
plus a `Connection closed` cause. The other 55 are byte-identical after 260
commits of main.
- #20499 replaced the five anchored raw-envelope reads with typed operations, so
those mutation anchors matched zero sites. Each is re-anchored at the same
defect's new home; bot-overrides moves to the shared reader that now owns it.
probe-hole-witness.test.ts pins hole and closure together: a probe must kill its
mutation and every pre-probe scenario of the same operation must still survive
it, so a redundant probe fails instead of accumulating.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): list the recording harness in the raw-port inventory
main's #20026 boundary test fails on any non-test file that reaches the raw
request port and is not inventoried. The oracle's scripted transport is exactly
that — it drives the real tracker and logical client — so it belongs in OWNERS
beside the supervisor fakes, not in the step-4 pending backlog.
Also states what the oracle covers, the two holes it was blind to until the
probes, and the step-4 runbook.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): pin the goldens to the tree that recorded them
The inventory entry is a fenced product-tree edit, so --record refused against
main's sha. Baseline now names the branch commit the goldens were recorded from;
the next re-record after this lands bumps it to the merge commit.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): carry a SAFETY rationale on every recorder cast
main added a changed-code casting gate after this branch was cut, so 45 `as`
sites in the recorder read as new findings. Each now states why the assertion
holds; they cluster into five reasons — recorded observations are RecordedValue
by construction, parsed manifests and goldens are validated on the next lines,
interned pools resolve their own hashes, a VM-evaluated module has no static
type, and the mount adapters supply only the members each hook reads.
Re-records the goldens: the comments move recorderSha256.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(rpc): state the measured blindness, not the assumed one
Applying each mutation to real product source shows the two holes are not equal.
The reorder is invisible to 83 of 84 tests and only a probe sees it. The refusal
blanking is also caught by the family reply matrix, because a refusal from cold
publishes null over a non-null initial value — an observational gap, not a
detection gap. Says so rather than letting the stronger claim carry both.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): close three ways the oracle could pass without checking
All four review findings were real; three let the oracle report green while
verifying less than it claimed.
- The baseline guard used `git diff --quiet`, which ignores untracked files, so
an untracked module under mobile/src or src/shared could change resolution
while a golden still recorded a pinned baseline header. Adds a
`git ls-files --others` check over the same paths, recorder still exempt.
- The determinism loop read `Number(env ?? 2)` unvalidated, so
RPC_FOUNDATION_DETERMINISM_RUNS=0 skipped the body and 57 tests passed having
recorded and compared nothing. Now requires an integer >= 2.
- Cleanup-time observations were dropped: every checkpoint clones the effects
array, so anything appended during dispose or the final flush never reached a
golden. Warns and documents the six scenarios that hit it today; recording
them changes every golden and is its own change.
- Two SAFETY rationales described each other's assertion. Swapped.
Goldens re-recorded for the recorder-digest change: 73 files, one header line
each, no recorded observation moved.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): record teardown observations as a cleanup checkpoint
Each checkpoint clones the effects array, so a rejection or state write produced
by dispose, the transport teardown or the final flush landed after the recording
was built and never reached a golden. An unmount leak is exactly what this
oracle exists to catch, so teardown now runs on the recorded path and anything
it observes becomes a checkpoint with id `cleanup`. State is captured before
dispose, since the operation is gone afterwards.
Six scenarios were dropping observations, across five goldens: projectRowDetailError,
projectMutating, hostLabelById, hostPlatform, workspaceAgent, workspaceAgentOverridden,
creatingKey, selectedAgent, agentOverridden and error. Those five gain a cleanup
checkpoint; the other 68 goldens change by their header line only, so no existing
observation moved.
Also fixes the README's own formatting, which failed `format:check`.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* Update README downloads badge
* refactor(attention): move the agent attention boundary off terminal panes (#20525)
* refactor(attention): move the agent attention boundary off terminal panes
The completion-attention pipeline asked PTY questions inline, so a non-terminal
agent surface was structurally excluded from unread, delivery and acknowledgement.
Extract a provider-neutral policy under `src/renderer/src/attention/` that reaches
every surface fact through an adapter, and move the PTY-shaped predicates
(`hasLivePtyForNotification`, `isCurrentLivePaneKey`, `isCurrentKnownPaneKey`,
`isVisibleForegroundPaneKey`, plus the leaf/foreground resolution the auto-ack scan
did inline) into `terminal-attention-surface.ts`, now their only call site.
Unread markers carry the reason that wrote them (`agent-completion`,
`terminal-bell`, `manual-mark-unread`, `legacy`); an unclassified boolean still on
live state reads as `legacy` rather than being guessed or migrated.
Main's `notifications:dispatch` closure becomes a delivery service with injected
collaborators; the IPC handler is a thin adapter. Behaviour is unchanged: unread is
still written before main's desktop gate, the tray dot before the cooldown/focus
gates, and mobile fan-out before the desktop early returns.
* fix(attention): recognize classified tab unread markers
* fix(grok): defer managed hook pane guard expansion to shell (#20534)
* 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
* perf(skills): bound WSL installed skill discovery (#12314)
* perf(skills): bound WSL installed skill discovery
* fix(skills): preserve bounded discovery correctness
* fix(skills): bound WSL metadata prefilter reads
* fix(skills): isolate absent discovery cwd cache keys
* test(skills): adapt WSL discovery mocks to runner
* fix(skills): preserve filtered discovery fallbacks
* fix(skills): share filtered scans and preserve WSL inventory
* fix(skills): share WSL scans without losing skill aliases
* fix: preserve skill metadata and retire filtered peer caches
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
* fix(git): skip upstream remote probes when the remote is absent (#18455)
* fix(git): skip upstream remote probes when the remote is absent
Issue and PR resolvers listed remotes by probing `git remote get-url
upstream` on every poll, including origin-only clones where that remote
cannot exist. List remotes once, cache against git config, and skip the
probe unless `upstream` is present.
* fix(git): avoid stale remote probe cache entries
* fix(github): observe origin repository probe failures
* fix(github): observe verified origin probe failures
* fix(github): skip missing upstream probe for PR lists
* test(github): scope the #9171 lazy-resolution guard to default-branch commands
The guard asserted that no git command runs for an open PR, using "no git at
all" as a proxy for "no default-branch resolution". Remote-name listing is a
separate concern, so allow it and keep every other command forbidden; the
symbolic-ref/rev-parse resolution this issue is about stays unreachable.
---------
Co-authored-by: Neil <neil@stably.ai>
* refactor(orchestration): derive delivery eligibility from messages (#19837)
* fix(orchestration): retire read deliveries and clarify mailbox recovery
* fix(orchestration): simplify delivery recovery and update nudge contracts
* test: align orchestration check help expectation
* refactor(orchestration): derive delivery eligibility from messages
* fix(orchestration): validate live consumers and simplify batch revocation
* refactor(orchestration): keep deliveries.status and derive eligibility without a column drop
The outstanding_deliveries view now reads status = 'outstanding' plus unread
membership, so v41 only drops uniqueness from idx_deliveries_one_outstanding
and adds the view and trigger. Older binaries can still open the database.
Removes the column-drop migration, the v40 test fixture and hasColumn guards,
the fenced skew probe, and the unrelated nudge-text change.
* docs(orchestration): drop delivery storage reference
The compatibility caveat it existed to explain no longer applies; the view
and index comments carry the remaining rationale.
* docs: revert unrelated formatter churn
* test(orchestration): verify historical database downgrade round trip
* fix(ssh): bound relay incumbent lsof probe (#18304)
* fix ssh relay incumbent probe timeout
* test ssh relay unconfirmed probe termination
* fix(ssh): preserve connect evidence while bounding lsof
* fix: preserve uncertainty when relay holder enumeration fails
* fix(ssh): supervise lsof helpers and preserve partial holder evidence
* test(ssh): prevent GC racing unconfirmed probe cleanup
* refactor(process): keep POSIX lsof supervision in process owner
* fix(ssh): confirm census cleanup and handle probe startup signals
* fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts
lsof warns to stderr about mounts it cannot stat, and any stderr byte forced
the holder enumeration to 'unavailable' — making the 'exited' verdict, and so
husk reaping, unreachable on those hosts. Pass -w to suppress the warnings.
* Revert "fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts"
This reverts commit 10d7b0db375.
Passing -w is correct in isolation, but it activates a previously dormant
path: in the SSH docker e2e lsof warns about the container filesystem, so
every probe used to degrade to 'unavailable'. Suppressing the warning lets
holder enumeration succeed and the takeover/reaping path run for the first
time there, and 'e2e / ssh docker watcher isolation' then hung to the job
timeout (52m, vs 28m passing on the parent commit).
The underlying gap is real and still open: on hosts where lsof always warns,
the 'exited' verdict and husk reaping stay unreachable. Fixing it needs the
reaping path understood in that environment, not just the flag.
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Neil <neil@stably.ai>
* perf(remote): avoid repeated capability probes during file imports (#14555)
* perf: avoid repeated remote import capability probes
* test: cover cold remote import compatibility probe
* fix(remote): fence imports across runtime reconnects
* fix(remote): bind import proof to connection
* fix(remote): fence import routing by runtime identity
* test(remote): remove unsafe import fixture assertions
- type remote RPC mocks at declaration so call arguments stay checked
- narrow upload params before reusing generated temp paths
---------
Co-authored-by: Neil <neil@stably.ai>
* fix(grok): announce a completion once, when Grok is actually finished (#20523)
* fix(grok): announce a completion once, when Grok is actually finished
Orca pinged on every Grok turn-end. Grok runs turns the user never asked for:
when a background task finishes it wakes itself, does a little work, and ends
another turn. One request produced several pings.
Grok already reports, on every turn-end, whether it still has work outstanding.
Read that instead of trying to classify which turns are "real":
backgroundTasks absent -> silent, this is the session-end tail
StopFailure / StopCancelled -> announce, a failure is never hidden
stopHookActive -> silent, a Stop hook is keeping it working
a shell task or subagent running -> silent, the work is not done
otherwise -> announce
Nothing here knows what an auto-wake turn is. A turn that ends with work
outstanding stays quiet; the later turn where that work is finally done is the
one that announces. That is also why this survives the case where Grok completes
a user's goal inside one of those turns — prefix-based suppression would have
silenced it.
Monitors and scheduled entries are deliberately not counted as outstanding work.
They can run indefinitely, so counting them would suppress a user's completion
permanently, and a lost ping is worse than an extra one.
Also registers StopCancelled, which Grok fires instead of Stop on a user
interrupt, a declined permission, --max-turns, or a no-progress bail-out. Orca
never subscribed to it, so those turns were reported as successes.
Also removes a stale notification matcher that searched for prose the shipping
binary never sends; the typed notification kind is matched instead, and neither
idle_prompt nor task_complete is treated as a completion.
Needs-input behaviour (permission prompts and ask_user_question waits) is
unchanged and stays ungated by background work.
* fix(grok): never hide a failed or cancelled turn behind the background-work gate
The announce predicate checked field-absence before terminal outcome. Grok's
StopFailure and StopCancelled payloads carry no background inventory at all, so
the absent-field branch — added so the session-end tail stays silent — fired
first and silenced every failure and every cancellation.
That inverted the rule it was meant to serve. Before this series a cancelled turn
at least surfaced as a (wrong) success; gated this way it surfaced as nothing.
Terminal outcome is now checked first, so a failure or cancellation announces
regardless of what other fields the payload happens to carry.
The existing tests passed straight through the bug because they built failure
payloads with a backgroundTasks field Grok never sends for those events. They now
model the real payload shapes, verified against the provider's payload
definitions and the captured envelopes.
* fix(grok): settle completion from provider lifecycle state
* fix(grok): fence stale turn ends without prompt ids
* 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.
* fix(claude): install SessionEnd for capable versions (#20530)
* 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 …
8 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 4's first domain. Migrates source-control onto the typed
RpcOperationboundary, and — because the pilot exposed it — fixes the reply matrix that was silently skipping most families.44 of 45 references migrated, 13 of 14 files. Backlog 145 files / 386 refs → 132 / 342.
Measurement (the pilot's actual purpose)
git.status)Unlike step 3's nine reads of one method, this domain includes mutations —
git.commit,git.push,worktree.set— so delivery ambiguity and error ordering carry real weight.One reader per method: the rule held
Two methods looked like they'd break it and didn't.
worktree.showhas two consumers reading different fields, but they agree on every input, so one reader is behaviour-identical — exactly the case step 3 solved with two.git.pushserves three param shapes across two call sites with three fallback strings: still one reader, because the fallback is a call-site concern.The single exception is genuine and the rule is what surfaced it.
git.statushas one consumer publishing the host payload verbatim and another reading a normalized projection that returnsnullon a non-array and drops entries missing a path. The projection is not a superset, so a shared reader would reject input the Changes screen renders today. Goldensc-review-status-entries-not-arraypins the difference.The reply matrix was skipping 10 of 23 families
The matrix drives every failure shape once per family — that is the mechanism that removes reliance on a migrator imagining edge cases. It selected which request to drive with a hardcoded prefix list (
settings., plus three named scenarios) andcontinued past everything else. Every family this domain added was skipped.That is why the migration's mutation testing initially came down to single hand-written scenarios. Measured: destroying the delivery-unknown mark on
git.pushwas caught by exactly one golden out of 163, and only because someone predicted it.Now
replyMatrixSites()returns every completion step — no selection, nothing to edit per domain. It throws on a family that scripts no reply and on an ambiguous repeated request name, and a census test asserts every family is matrixed, so a reintroducedcontinuefails the build rather than shrinking coverage silently. 13 sites → 61. Four families sit on an inventoried exception list with written reasons; an entry whose family later records a success fails the suite, so the list only shrinks.gitStatusProjectionReadpolicy swapThat last row is the honest one. The matrix removes dependence on foresight for the partitions it enumerates; someone still has to enumerate them, and the original nine were incomplete.
A real divergence, caught and reverted
The expanded matrix found that the migration had changed behaviour. Main returned
result?.error || 'Commit failed', passing a truthy non-string straight through under astringannotation; the migrated helper stringified it to"[object Object]".Stringifying is arguably better — downstream does
result.error.replace(...), which throws on an object and merely looks ugly on a string. Reverted anyway: this migration's contract is that nothing changes, and an unannounced improvement inside a refactor is precisely what parity evidence exists to prevent. The latent throw is its own ticket. No host sends this today (git.commitis typed{success, error?: string}), but nothing validates it and mixed versions are normal.Goldens
Suite 46 → 91 scenarios, 73 → 118 goldens; recording tests 198 pass. Re-recorded after merging main using the pre-migration flow — main's cosmetic recorder refactor moved
recorderSha256on all 153. OnlyrecorderSha256andbaselinechanged; no recorded observation moved on any file.Follow-up found, not fixed here
family-recordings.test.ts's schedules loop has the samecontinueshape over 7 hardcoded scenario ids. Measured: forcing the skip drops 7 registered tests and orphans 7 goldens, and no test enumerates the goldens directory, so an unreferenced golden is invisible. It is milder — both trigger inputs are insiderecorderSha256, so the loss cannot happen during a comparison run, only at the next deliberate re-record. Worth its own fail-closed change.Gates
mobile suite 574 files / 4,930 passed, 6 skipped · recording suite 198/201 · mobile typecheck ·
pnpm tc·check:code-quality:changed0 findings across 34 files ·check:react-doctor:changedclean ·oxfmt --checkclean across 1,889 files.