Repository navigation
refactor(mobile): checked reply readers for the source-control domain (step 7 pilot) - #20950
Conversation
Step 4 moved every call-site cast into an RpcOperation's `read`, but 201 of those readers still answer `compatible: true` for any payload: `rpcUncheckedPayloadReader` (163), `rpcReadUnchecked` (26 outside its own module) and `rpcUncheckedMemberReader` (12), across 42 files. The cast moved; it did not become true. Held as data with an AST boundary test, shaped on the raw-request-port ratchet: a file that is not listed fails, a listed file that no longer has one fails, and a count that rises fails. Only a call counts, so an import is not a reader and prose never is. No behaviour change: this commit adds a list and a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…ival Replaces all 17 unchecked readers in mobile/src/source-control/ with `rpcResultVariant(variant, schema)`, so a malformed reply is an `RpcIncompatibleReplyError` naming the operation instead of a TypeError three frames downstream. The inventory drops 201 -> 184 and the five source-control operations files leave it entirely. This is a behaviour change, scoped to malformed replies. Six reply-matrix goldens move; every named-scenario golden and every `normal` partition is byte-identical, which is the parity claim. Schemas live one module per reply domain, beside the operations that read them: git-status, git-compare, git-history, hosted-review and worktree-metadata. A member is required only where a consumer reads it unguarded, and each schema records the consumer line that justifies it. Nothing is `.strict()`; every reply a consumer publishes verbatim keeps `z.looseObject` so an undeclared host member still passes through. Six replies have no reader anywhere in mobile and get `z.unknown()`, which is the honest schema for them, not a holdout. Three readers stay total by construction, because their contract is that an unreadable reply is a value rather than an error: the `git.status` projection (a null status three screens route on), the `session.tabs.list` reveal (a null list means poll again) and the generated commit message (a screen's copy, never a decode error in a text field). They gain the salvage report, not a verdict. Consumers take the schema's output type, so `MobileGitStatusResult` and the branch-compare aliases now name what mobile reads rather than the desktop aggregate, and seven call-site casts are gone. Three requirements came from the goldens, not from the host types: `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…tep 7 moves
Six goldens, all on malformed partitions. Every named-scenario golden and every
`normal` partition is unchanged, which is the parity claim for this step.
git.history-read / git.history#1
result-absent, result-null, inner-ok-missing, inner-false-string-error,
inner-false-object-error: the load rejected with a TypeError reading 'items'
or 'map' off undefined/null; it now rejects with
`incompatible_reply: git.history-page (git.history)`.
hostedReview.eligibility + create-intent / hostedReview.getCreationEligibility
result-absent, result-null, inner-ok-*: the fetch fulfilled with the error
envelope itself, re-typed as an eligibility and published into the compose
prefill; it now rejects, and both callers already route that to the same
"eligibility unavailable" state a null answer produced.
hostedReview.create-chain + create-intent / hostedReview.create
result-absent, result-null, inner-ok-missing, inner-false-object-error: the
create form showed the raw TypeError text "Cannot read properties of
undefined (reading 'ok')"; it now shows the incompatible-reply message.
Every header digest is unchanged -- baseline, recorder, adapter, scenario and
lockfile all match -- so the diff is the behaviour and nothing else.
Recorded from this branch into a scratch directory and copied in, because there
is no scoped honest alternative: scripts/rpc-recording.mts refuses to run unless
the product tree equals the pinned baseline, and the README's remedy for an
intended behaviour change is to repin, which rewrites the `baseline` header of
all 667 goldens. So these six now carry a pin whose tree no longer produces
them. That is a real gap in the oracle's design for behaviour changes, not a
detail of this step, and it needs a decision before this lands.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Each of these cost a reply-matrix golden while writing the source-control schemas, and none of them follows from reading the consumers or the host types: a newer host's undeclared members must still decode, `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. The `.strict()` case is the one worth stating twice: at the top level it rejects the reply, and on the entry it drops the row, which shows a dirty worktree an empty Changes list. The fifth test pins the salvage report that makes such a drop visible instead of silent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
`RpcIncompatibleReplyError` put `incompatible_reply: <op> (<method>)` in `message`, and `message` is what the screens hand to a toast. Step 7 is the first change that can reach this error at all, so the token would have shipped to users as its own error copy. Fixed at the boundary rather than per site: `message` is now plain copy, and the machine token moved to `code` (`incompatible_reply`) and `name` (`RpcIncompatibleReplyError`), both readable by callers. The cross-bundle fallback in `isRpcIncompatibleReplyError` matched on the old message prefix, so it now matches on `name`, which a foreign copy of the module still carries. No existing test pinned the old text. Two new ones pin the copy, the token and the foreign-copy match. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Commit adeb5f9 recorded the six moved goldens into a scratch directory and copied them back, which left them pinned to `e7206f62`, a tree that no longer produces them. That is the one claim the `baseline` header exists to make, so this replaces it with the README's remedy done in full. `baseline` is now f741b2e, the last commit on this branch that touches a fenced path, so the recording fence passes in place and every golden is pinned to the tree that produced it. All 667 were re-recorded through `scripts/rpc-recording.mts --record`; none were hand-edited. Decoding every value pool against the branch point b8d4cde sorts the corpus into 661 header-only moves where `baseline` is the only key that moved, 6 whose body moved as well, 0 added and 0 deleted. The 6 are the disclosed step-7 delta, unchanged at 69 moved observation fields across malformed reply partitions, plus the readable incompatible-reply copy from f741b2e. No `normal` partition and no named-scenario golden moved. `scenarioSha256` hashes the derived scenarios, not the manifest, so the repin moves no other header key; the README section this adds records that, the scratch-copy failure mode, and the follow-up repin main needs after a squash merge. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…by cast The two new tests in f741b2e read the error through `as` casts, which the changed-code casting gate rejects. An `instanceof` guard narrows the same value and checks the class at the same time. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…ecord 71d8c6a touched a fenced path (`mobile/src`), so the pin from 5f3f184 no longer named the tree that produces these goldens. The fence compares the whole of `mobile/src`, and a test file is inside it, so the pin follows the last commit that touches a fenced path rather than the commit whose behaviour moved. Re-recorded all 667 in place through `scripts/rpc-recording.mts --record`. Decoding every value pool against the branch point b8d4cde still gives 661 header-only moves with `baseline` the only moved key, 6 body moves, 0 added and 0 deleted; the six and their 69 moved observation fields are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
git.status (host payload), git.branchCompare, git.commitCompare and git.branchDiff were migrated to checked readers with no recording observing them, so a required member a host omits would have surfaced only in production. Three families mount the owners rather than the senders, because each reply is only visible in what the owner then publishes: the Changes screen's loader hook (git.status, and the base-ref chain and git.branchCompare it triggers), the history list screen (git.history and the per-commit git.commitCompare), and the committed-diff opener hook (git.branchDiff). Ten goldens: three pilot recordings and seven reply matrices. Two adapter capabilities this needed. An inert FlatList never calls `renderItem`, so the history adapter renders one row through the screen's own callback, both to reach the handler that expands a commit and to read the file list back; without that the commit-compare reply changes nothing observable. And `lowlight` joins `react` and `zod` as a real library rather than a refusing proxy, because the branch diff highlights on its success arm before the preview reaches state, so the shipped text arm was otherwise unrecordable. No golden recorded its absence, so only `recorderSha256` moves. Recording the same scenarios against 4b0009d, the pre-refactor tree, is the before column. Decoding every value pool across the two gives 11 body moves and 666 header-only, 0 added, 0 deleted: the 6 already disclosed, plus the 5 new matrices at 63 moved observation fields. What moved is the point. A malformed git.status used to leave Changes `ready` over the malformed payload and go on to fetch a branch compare; it now says the host sent a reply it could not read. An absent git.branchDiff result used to put "Cannot read properties of undefined (reading 'kind')" on the screen. An unreadable git.commitCompare used to spin the expanded commit forever; it now says "No file changes". Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Scenario manifest merged by id (main's two stream scenarios plus this branch's three source-control reads). Goldens take main's side except the six this branch re-recorded; the next commit repins and re-records all. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…-record The merge is the last commit touching a fenced path, so it is the only tree the recorder's fence can match. Every golden moves `baseline` and picks up main's `recorderSha256` from #20920; the six the checked readers changed are the only bodies that move against main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…d command `format:check` from `mobile/` caught the wrapped inline command the recipe had been carrying since it landed; pointing at the command above removes the duplicate and the wrap together. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
ℹ️ Minor suggestions only.
Reviewed changes
- 17 unchecked readers converted to checked zod readers —
git.status(both readers),git.history,git.branchCompare,git.commitCompare,git.branchDiff,hostedReview.getCreationEligibility,hostedReview.create,worktree.show,session.tabs.list,git.commit,git.publish,git.bulkStage,git.generateCommitMessage,git.cancelGenerateCommitMessage,files.openDiff,files.open,worktree.setnow flow throughrpcResultVariantagainst five new schema modules, and 7 call-site casts are gone. - Schemas sized to the reader, not the host type — required only where a consumer reads unguarded,
salvagedOptional/salvagingArrayfor malformed optionals and rows,z.unknown()for payloads with no reader. - Readable incompatible-reply copy —
RpcIncompatibleReplyErrornow carries user-facingmessagepluscode/name, and the cross-bundle fallback matches onname. - Ratchet —
unchecked-rpc-reader-inventory.ts(184 readers across 42 files) plus an AST boundary test; source-control comes off the list. - Recording corpus —
baselinerepinned to the merge commit, all 689 goldens re-recorded (673 header-only because thelowlightsubstitute changesrecorderSha256), 6 malformed matrices body-moved, 10 added, and a new mount adapter givesgit.commitComparean oracle.
I verified the golden delta from the diff directly: 673 header-only, exactly the 6 disclosed body-moved matrices, 10 added, 0 deleted — matching the PR body. A specialist pass over all five schemas found no well-formed reply that a real host produces being rejected.
ℹ️ git.status is now normalized in two places
readMobileGitStatusResult (in mobile-diff-review-rpc.ts, still used by mobile-open-pr-prefill.ts / readFreshGitStatus) and the new gitStatusProjectionSchema are independent implementations of the same host-payload normalization. They agree today — that is what the reviewer checked — but the Open PR readiness path and the Changes/review screens now read the same reply through different code.
Technical details
# Two implementations of the same `git.status` projection
## Affected sites
- `mobile/src/session/mobile-diff-review-rpc.ts:120-134` — `readMobileGitStatusResult`, still the
reader behind `mobile/src/source-control/mobile-open-pr-prefill.ts:12`.
- `mobile/src/source-control/git-status-reply-schema.ts:134-161` — the new
`gitStatusProjectionSchema`, behind `gitStatusProjectionRead` / `branchContextStatusRead`.
## Open questions for the human
- Can `readMobileGitStatusResult` be replaced by the new projection (parse through the schema and
return `value`), so the Open PR readiness path and the review screen cannot disagree? If not, a
cross-reference comment on both would at least flag that they must move together.DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| */ | ||
| export const gitStatusHostPayloadSchema = z.looseObject({ | ||
| entries: salvagingArray(gitStatusEntrySchema), | ||
| conflictOperation: salvagedOptional('conflictOperation', z.enum(GIT_CONFLICT_OPERATION)), |
There was a problem hiding this comment.
conflictOperation is read only as an opaque token (MobileSourceControlPanel.tsx:274 passes it to a card typed string | null, and mobile-source-control-conflict-abort.ts:9 compares it to 'merge'/'rebase'). Constraining it to the four known tokens means a host that adds a new operation yields undefined and hides the conflict banner, where the previous unchecked reader rendered the token. Widening to z.string(), as done for reviewLookupOutcome, would preserve the old behaviour; the projection's own coercion to 'unknown' is unaffected.
…m degrades A closed `z.enum` in a reply schema is a version claim, and it refused replies every declared reader could have rendered: a `git.branchCompare` summary status of 'shallow-base' failed the whole Changes compare, a 'codeberg' provider failed the whole eligibility, and a 'typechange' entry status dropped the row. Main passed all three through. `openEnum` in zod-salvage declares the arm set open: an unrecognised arm reads as a member the consumers already handle, while absence and a non-string stay fatal. Not `.catch()`, which would swallow those two as well. `area` stays closed and says why: every arm grants stage, unstage or commit, so there is no member to degrade to that would not offer an action against a row this build cannot place. Main rendered such a row in no section either. Also drops two claims the code does not back. Nothing reads the salvage report, so the two comments promising a dropped entry "arrives as salvage.droppedPaths" are gone, and `hostKind` on the non-text diff arm had no reader. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Rule 4 in the wire-compatibility page, beside the three rules it belongs with: an enum arm set is a wire surface, unknown arms degrade rather than reject, and leaving one closed is a decision to state where the schema is declared. The recorder recipe's step 4 said `baseline` would be the only moved header key, which is only true of a branch that never touched the recorder. It now names the three digests a branch's own edits move, so a reader recognises a clean result. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
The corpus records in ~110s warm and 160s under load, against a 120s budget, so a full re-record was killed roughly half the time. A killed run wrote a partial reporter banner and exited 1, which reads as a failing scenario rather than as a run that never finished — it cost two investigations here. The budget is now ten minutes, and a killed run says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…d re-record `baseline` is the only header key that moves and no golden body moves: no matrix partition scripts an unknown enum arm, so the corpus cannot see this change. The eight schema unit tests are its only oracle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
Important
The round-1 open-enum fix rewrites an unrecognised newer-host provider token to 'unsupported'. The action-sheet Create PR path forwards that value to the host, which refuses it — so a provider a newer host added can no longer create, where main passed the token through and the host accepted its own provider. Details inline.
Reviewed changes
This run re-reviewed the four commits since the prior pullfrog review at e50a5bba9f — the round-1 open-enum response, its docs, the recorder timeout, and the baseline repin.
- Opened the closed source-control enums — new
openEnuminsrc/shared/zod-salvage.ts; the compare/entry/file statuses and the three conflict sets now degrade to a conservative arm instead of refusing the reply or dropping the row carrying it. - Kept
git.status'sareaclosed with the reason stated in the schema (every arm grants an affordance, so an unknown area must drop the row rather than offer an action). - Trimmed the schema surface — leaf schemas de-exported;
MobileGitCommitCompareReply,MobileWorktreeSummaryReply, and the diff reply'shostKinddropped as unread. - Recorded the rule and the recipe — Rule 4 in
docs/reference/remote-wire-compatibility.md; README step 4 names the header keys a branch's own edits move;rpc-recording.mtsbudgets ten minutes and reports a timeout instead of exiting 1. - Repinned and re-recorded —
baseline2648e87dba→c4b430b35e; every moved golden is header-only (baselinealone), matching the disclosed delta. - Added 8 open-enum tests pinning each fallback, including that a non-string stays fatal.
A specialist pass over the five openEnum call sites confirmed the helper's parse verdicts (enum members pass, unknown strings degrade, non-strings stay fatal) and found four of the five fallbacks behavior-identical to main for their readers; the fifth is the inline finding.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
…not ready Both fallback prefills set `canCreate: false`, which is a determination nobody made. It short-circuits getMobilePrCreateBlockMessage before reviewLookupOutcome is read, so a malformed, refused or rejected eligibility told the user "This branch is not ready for a pull request yet." instead of asking them to retry. Dropping it leaves `canCreate` undefined, which is what "unproven" means here. Only a host that determined `canCreate: false` still gets the blocked copy. `area` now degrades to absent rather than staying closed. Dropping the row also dropped it from the unresolved-conflict gate, which grants create on a conflicted worktree; absent withholds stage, unstage and commit while keeping the row, since every area reader is an equality check. Its four consumers narrow explicitly: the diff-review queue filters unplaceable rows, the opener withholds the route, and the commit-failure prompt pins 'staged' where its own filter already did. `git.branchCompare` entries are nullish, matching the `?? []` its consumers use. Deletions: `MobileGitStatusProjection` and `uncheckedReaderCount` lose `export`, the boundary test drops its dead inventory self-file (the AST counter finds zero calls there, only prose), and `isRpcIncompatibleReplyError` is gone — it had no caller in mobile, desktop or e2e. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…record The round-2 eligibility fix is a behaviour change, so the corpus has to be re-recorded at a pin that includes it. Four goldens move body: the two create-intent eligibility matrices on every non-normal partition, and the two prefill scenarios that lose the fallback's `canCreate: false`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
ℹ️ No critical issues — one minor coverage suggestion inline.
Reviewed changes
This run re-reviewed the three commits since the prior pullfrog review at 0f68434e22 — the round-2 eligibility-copy fix, its test formatting, and the repin/re-record.
- Stopped an unresolvable eligibility claiming the branch is not ready — the unavailable fallback prefill no longer carries
canCreate: false, so a malformed, refused or transport-dropped lookup returns the "could not confirm… Try again" copy instead of main's "This branch is not ready…"; a host that actually determinedcanCreate: falsestill blocks with itsblockedReasoncopy. New tests cover all four cases. - Opened
git.status'sareato absent — an unrecognised staging-area token keeps the row (so the unresolved-conflict gate still sees it) but lands in no section and withholds stage/unstage/commit; the diff-review queue and the file opener guardarea === undefined. - Accepted an explicit
nullentries list ongit.branchCompare(.optional()→.nullish()), matching thetimestamp: nullprecedent. - Removed the unused
isRpcIncompatibleReplyErrorcross-bundle helper and its test; de-exported the boundary test's counter. - Pinned the staged-entry shape to
MobileCommitFailureStagedEntry(area: 'staged') so the filter, not the field, proves the literal. - Repinned and re-recorded —
baselinec4b430b35e→4af6f568a3; the four body-moved goldens are the twogetCreationEligibilitymatrices and the twosc-prefill-unavailable-*scenarios, allcanCreateremovals, with the rest header-only. - Corrected Rule 4 in the wire-compatibility doc to state that a fallback may be absent, with
areaas the worked example.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
#20954 landed the step-6 recorder engine change, so the corpus has to be re-recorded on top of it. Conflicts: the scenario manifest, merged by scenario id (no shared id changed on both sides), and 679 goldens whose headers both sides repinned — main's side everywhere except the eight this branch moves body. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…ecord The merge is now the last commit touching a fenced path, so the corpus has to carry its sha. No body moves against the pre-merge corpus: main's engine change shifts `recorderSha256` on every golden and nothing else, and main's fifteen step-6 goldens re-record byte-identical. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
The ratchet is a ceiling against this branch adding readers, not a claim about what main may land. #20954 brought `notification-stream-closed`, `native-chat-session-page` and `terminal-buffer-cleared`, so the merge has to raise those lines and say where they came from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…d re-record The ratchet inventory is a fenced path, so admitting #20954's three readers moved the fence head again. Baseline only; no body moves against the merge re-record. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…lback `provider` is not a member mobile only reads. The eligibility reply names it and the create call returns it, so `openEnum(..., 'unsupported')` did not soften a reading — it rewrote the bytes, and a host that had just named `codeberg` refused its own provider as unsupported. The action-sheet Create path has no provider gate, so nothing caught it. Passes the token through as a string from the reply to the create params. The allow-list that decides whether mobile may create stays supportsHostedReviewCreation(), which already answers no for a token this build does not know; its parameter widens to `string`, since answering for an unknown token is the whole job. The worktree-link switch gains a default, which also fixes an older hole: an unrecognised provider used to fall out of the switch as `undefined` params. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
Repins to the provider fix and records `sc-create-intent-unlisted-provider`, whose eligibility reply names `codeberg` and whose recorded `hostedReview.create` params carry it back unchanged. Restoring the old enum fallback fails that golden on `Request params mismatch: hostedReview.create#1` and nothing else. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the provider pass-through fix restores main's behaviour, and every changed line is either that fix or its pin.
Reviewed changes
This run re-reviewed the two commits since the prior pullfrog review at 8e80453055 — the provider pass-through fix and its recording-corpus pin.
- Passed the host's own provider token through instead of a fallback —
hostedReviewEligibilitySchema.provideris nowz.string(), the prefill/create-inputproviderwiden tostring,supportsHostedReviewCreationtakes a plain string, andbuildMobileHostedReviewCreateParamssends the token verbatim. This removes the round-1 fallback that rewrote a newer host's provider to'unsupported'on the create path. - Closed an older switch hole —
buildWorktreeSetHostedReviewLinkParamsreplacedcase 'unsupported':withdefault:, so an unrecognised provider returns the bareworktree/baseRefparams instead of falling out asundefined. - Pinned the pass-through end to end — the new
sc-create-intent-unlisted-providergolden records an eligibility reply namingcodebergand assertshostedReview.createechoes it back unchanged; a unit test round-trips the same token throughresolveMobilePrPrefill→createMobilePr. - Added the missing queue-filter test —
buildMobileDiffReviewQueueis now driven with anarea: undefinedentry and asserted to exclude it, pinningisPlaceableStatusEntry. - Recorded the send-side rule — Rule 4 in
docs/reference/remote-wire-compatibility.mdnow states that a reply-schema fallback must never shape a param. - Repinned and re-recorded —
baseline91acdd4267→9e01c50e6d; every other golden is header-only (baseline+recorderSha256).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
… (step 7 pilot) (stablyai#20950) * test(mobile): ratchet the 201 unchecked RPC reply readers Step 4 moved every call-site cast into an RpcOperation's `read`, but 201 of those readers still answer `compatible: true` for any payload: `rpcUncheckedPayloadReader` (163), `rpcReadUnchecked` (26 outside its own module) and `rpcUncheckedMemberReader` (12), across 42 files. The cast moved; it did not become true. Held as data with an AST boundary test, shaped on the raw-request-port ratchet: a file that is not listed fails, a listed file that no longer has one fails, and a count that rises fails. Only a call counts, so an import is not a reader and prose never is. No behaviour change: this commit adds a list and a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): validate the source-control domain's RPC replies at arrival Replaces all 17 unchecked readers in mobile/src/source-control/ with `rpcResultVariant(variant, schema)`, so a malformed reply is an `RpcIncompatibleReplyError` naming the operation instead of a TypeError three frames downstream. The inventory drops 201 -> 184 and the five source-control operations files leave it entirely. This is a behaviour change, scoped to malformed replies. Six reply-matrix goldens move; every named-scenario golden and every `normal` partition is byte-identical, which is the parity claim. Schemas live one module per reply domain, beside the operations that read them: git-status, git-compare, git-history, hosted-review and worktree-metadata. A member is required only where a consumer reads it unguarded, and each schema records the consumer line that justifies it. Nothing is `.strict()`; every reply a consumer publishes verbatim keeps `z.looseObject` so an undeclared host member still passes through. Six replies have no reader anywhere in mobile and get `z.unknown()`, which is the honest schema for them, not a holdout. Three readers stay total by construction, because their contract is that an unreadable reply is a value rather than an error: the `git.status` projection (a null status three screens route on), the `session.tabs.list` reveal (a null list means poll again) and the generated commit message (a screen's copy, never a decode error in a text field). They gain the salvage report, not a verdict. Consumers take the schema's output type, so `MobileGitStatusResult` and the branch-compare aliases now name what mobile reads rather than the desktop aggregate, and seven call-site casts are gone. Three requirements came from the goldens, not from the host types: `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the six source-control reply-matrix goldens step 7 moves Six goldens, all on malformed partitions. Every named-scenario golden and every `normal` partition is unchanged, which is the parity claim for this step. git.history-read / git.history#1 result-absent, result-null, inner-ok-missing, inner-false-string-error, inner-false-object-error: the load rejected with a TypeError reading 'items' or 'map' off undefined/null; it now rejects with `incompatible_reply: git.history-page (git.history)`. hostedReview.eligibility + create-intent / hostedReview.getCreationEligibility result-absent, result-null, inner-ok-*: the fetch fulfilled with the error envelope itself, re-typed as an eligibility and published into the compose prefill; it now rejects, and both callers already route that to the same "eligibility unavailable" state a null answer produced. hostedReview.create-chain + create-intent / hostedReview.create result-absent, result-null, inner-ok-missing, inner-false-object-error: the create form showed the raw TypeError text "Cannot read properties of undefined (reading 'ok')"; it now shows the incompatible-reply message. Every header digest is unchanged -- baseline, recorder, adapter, scenario and lockfile all match -- so the diff is the behaviour and nothing else. Recorded from this branch into a scratch directory and copied in, because there is no scoped honest alternative: scripts/rpc-recording.mts refuses to run unless the product tree equals the pinned baseline, and the README's remedy for an intended behaviour change is to repin, which rewrites the `baseline` header of all 667 goldens. So these six now carry a pin whose tree no longer produces them. That is a real gap in the oracle's design for behaviour changes, not a detail of this step, and it needs a decision before this lands. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the four reply-schema properties the goldens found Each of these cost a reply-matrix golden while writing the source-control schemas, and none of them follows from reading the consumers or the host types: a newer host's undeclared members must still decode, `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. The `.strict()` case is the one worth stating twice: at the top level it rejects the reply, and on the entry it drops the row, which shows a dirty worktree an empty Changes list. The fifth test pins the salvage report that makes such a drop visible instead of silent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give an unreadable reply a message a user can read `RpcIncompatibleReplyError` put `incompatible_reply: <op> (<method>)` in `message`, and `message` is what the screens hand to a toast. Step 7 is the first change that can reach this error at all, so the token would have shipped to users as its own error copy. Fixed at the boundary rather than per site: `message` is now plain copy, and the machine token moved to `code` (`incompatible_reply`) and `name` (`RpcIncompatibleReplyError`), both readable by callers. The cross-bundle fallback in `isRpcIncompatibleReplyError` matched on the old message prefix, so it now matches on `name`, which a foreign copy of the module still carries. No existing test pinned the old text. Two new ones pin the copy, the token and the foreign-copy match. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to this branch and re-record Commit adeb5f9 recorded the six moved goldens into a scratch directory and copied them back, which left them pinned to `e7206f62`, a tree that no longer produces them. That is the one claim the `baseline` header exists to make, so this replaces it with the README's remedy done in full. `baseline` is now f741b2e, the last commit on this branch that touches a fenced path, so the recording fence passes in place and every golden is pinned to the tree that produced it. All 667 were re-recorded through `scripts/rpc-recording.mts --record`; none were hand-edited. Decoding every value pool against the branch point b8d4cde sorts the corpus into 661 header-only moves where `baseline` is the only key that moved, 6 whose body moved as well, 0 added and 0 deleted. The 6 are the disclosed step-7 delta, unchanged at 69 moved observation fields across malformed reply partitions, plus the readable incompatible-reply copy from f741b2e. No `normal` partition and no named-scenario golden moved. `scenarioSha256` hashes the derived scenarios, not the manifest, so the repin moves no other header key; the README section this adds records that, the scratch-copy failure mode, and the follow-up repin main needs after a squash merge. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): narrow the incompatible-reply error by instanceof, not by cast The two new tests in f741b2e read the error through `as` casts, which the changed-code casting gate rejects. An `instanceof` guard narrows the same value and checks the class at the same time. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the branch tip and re-record 71d8c6a touched a fenced path (`mobile/src`), so the pin from 5f3f184 no longer named the tree that produces these goldens. The fence compares the whole of `mobile/src`, and a test file is inside it, so the pin follows the last commit that touches a fenced path rather than the commit whose behaviour moved. Re-recorded all 667 in place through `scripts/rpc-recording.mts --record`. Decoding every value pool against the branch point b8d4cde still gives 661 header-only moves with `baseline` the only moved key, 6 body moves, 0 added and 0 deleted; the six and their 69 moved observation fields are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the four source-control reads that had no oracle git.status (host payload), git.branchCompare, git.commitCompare and git.branchDiff were migrated to checked readers with no recording observing them, so a required member a host omits would have surfaced only in production. Three families mount the owners rather than the senders, because each reply is only visible in what the owner then publishes: the Changes screen's loader hook (git.status, and the base-ref chain and git.branchCompare it triggers), the history list screen (git.history and the per-commit git.commitCompare), and the committed-diff opener hook (git.branchDiff). Ten goldens: three pilot recordings and seven reply matrices. Two adapter capabilities this needed. An inert FlatList never calls `renderItem`, so the history adapter renders one row through the screen's own callback, both to reach the handler that expands a commit and to read the file list back; without that the commit-compare reply changes nothing observable. And `lowlight` joins `react` and `zod` as a real library rather than a refusing proxy, because the branch diff highlights on its success arm before the preview reaches state, so the shipped text arm was otherwise unrecordable. No golden recorded its absence, so only `recorderSha256` moves. Recording the same scenarios against 4b0009d, the pre-refactor tree, is the before column. Decoding every value pool across the two gives 11 body moves and 666 header-only, 0 added, 0 deleted: the 6 already disclosed, plus the 5 new matrices at 63 moved observation fields. What moved is the point. A malformed git.status used to leave Changes `ready` over the malformed payload and go on to fetch a branch compare; it now says the host sent a reply it could not read. An absent git.branchDiff result used to put "Cannot read properties of undefined (reading 'kind')" on the screen. An unreadable git.commitCompare used to spin the expanded commit forever; it now says "No file changes". Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the merge commit and re-record The merge is the last commit touching a fenced path, so it is the only tree the recorder's fence can match. Every golden moves `baseline` and picks up main's `recorderSha256` from stablyai#20920; the six the checked readers changed are the only bodies that move against main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): note the merge-commit pin and unwrap the recipe's record command `format:check` from `mobile/` caught the wrapped inline command the recipe had been carrying since it landed; pointing at the command above removes the duplicate and the wrap together. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): open the source-control reply enums so a newer host's arm degrades A closed `z.enum` in a reply schema is a version claim, and it refused replies every declared reader could have rendered: a `git.branchCompare` summary status of 'shallow-base' failed the whole Changes compare, a 'codeberg' provider failed the whole eligibility, and a 'typechange' entry status dropped the row. Main passed all three through. `openEnum` in zod-salvage declares the arm set open: an unrecognised arm reads as a member the consumers already handle, while absence and a non-string stay fatal. Not `.catch()`, which would swallow those two as well. `area` stays closed and says why: every arm grants stage, unstage or commit, so there is no member to degrade to that would not offer an action against a row this build cannot place. Main rendered such a row in no section either. Also drops two claims the code does not back. Nothing reads the salvage report, so the two comments promising a dropped entry "arrives as salvage.droppedPaths" are gone, and `hostKind` on the non-text diff arm had no reader. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs: write down the open-enum rule and the header keys a branch moves Rule 4 in the wire-compatibility page, beside the three rules it belongs with: an enum arm set is a wire surface, unknown arms degrade rather than reject, and leaving one closed is a decision to state where the schema is declared. The recorder recipe's step 4 said `baseline` would be the only moved header key, which is only true of a branch that never touched the recorder. It now names the three digests a branch's own edits move, so a reader recognises a clean result. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop the recorder's own timeout killing a full re-record The corpus records in ~110s warm and 160s under load, against a 120s budget, so a full re-record was killed roughly half the time. A killed run wrote a partial reporter banner and exited 1, which reads as a failing scenario rather than as a run that never finished — it cost two investigations here. The budget is now ten minutes, and a killed run says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the open-enum commit and re-record `baseline` is the only header key that moves and no golden body moves: no matrix partition scripts an unknown enum arm, so the corpus cannot see this change. The eight schema unit tests are its only oracle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop an unresolvable eligibility claiming the branch is not ready Both fallback prefills set `canCreate: false`, which is a determination nobody made. It short-circuits getMobilePrCreateBlockMessage before reviewLookupOutcome is read, so a malformed, refused or rejected eligibility told the user "This branch is not ready for a pull request yet." instead of asking them to retry. Dropping it leaves `canCreate` undefined, which is what "unproven" means here. Only a host that determined `canCreate: false` still gets the blocked copy. `area` now degrades to absent rather than staying closed. Dropping the row also dropped it from the unresolved-conflict gate, which grants create on a conflicted worktree; absent withholds stage, unstage and commit while keeping the row, since every area reader is an equality check. Its four consumers narrow explicitly: the diff-review queue filters unplaceable rows, the opener withholds the route, and the commit-failure prompt pins 'staged' where its own filter already did. `git.branchCompare` entries are nullish, matching the `?? []` its consumers use. Deletions: `MobileGitStatusProjection` and `uncheckedReaderCount` lose `export`, the boundary test drops its dead inventory self-file (the AST counter finds zero calls there, only prose), and `isRpcIncompatibleReplyError` is gone — it had no caller in mobile, desktop or e2e. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): formatting and a thrown rejection in the round-2 tests Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the round-2 tip and re-record The round-2 eligibility fix is a behaviour change, so the corpus has to be re-recorded at a pin that includes it. Four goldens move body: the two create-intent eligibility matrices on every non-normal partition, and the two prefill scenarios that lose the fallback's `canCreate: false`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the main merge and re-record The merge is now the last commit touching a fenced path, so the corpus has to carry its sha. No body moves against the pre-merge corpus: main's engine change shifts `recorderSha256` on every golden and nothing else, and main's fifteen step-6 goldens re-record byte-identical. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): admit the three unchecked readers stablyai#20954 landed The ratchet is a ceiling against this branch adding readers, not a claim about what main may land. stablyai#20954 brought `notification-stream-closed`, `native-chat-session-page` and `terminal-buffer-cleared`, so the merge has to raise those lines and say where they came from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to the inventory commit and re-record The ratchet inventory is a fenced path, so admitting stablyai#20954's three readers moved the fence head again. Baseline only; no body moves against the merge re-record. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): send the host's own provider token back instead of a fallback `provider` is not a member mobile only reads. The eligibility reply names it and the create call returns it, so `openEnum(..., 'unsupported')` did not soften a reading — it rewrote the bytes, and a host that had just named `codeberg` refused its own provider as unsupported. The action-sheet Create path has no provider gate, so nothing caught it. Passes the token through as a string from the reply to the create params. The allow-list that decides whether mobile may create stays supportsHostedReviewCreation(), which already answers no for a token this build does not know; its parameter widens to `string`, since answering for an unknown token is the whole job. The worktree-link switch gains a default, which also fixes an older hole: an unrecognised provider used to fall out of the switch as `undefined` params. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the provider pass-through in the corpus Repins to the provider fix and records `sc-create-intent-unlisted-provider`, whose eligibility reply names `codeberg` and whose recorded `hostedReview.create` params carry it back unchanged. Restoring the old enum fallback fails that golden on `Request params mismatch: hostedReview.create#1` and nothing else. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

Step 7 pilot, source-control domain: 17 unchecked reply readers become checked zod readers, so a malformed host reply surfaces as one readable error at the operation boundary instead of a downstream
TypeErroror a screen left ready over garbage. Deliberately a behaviour change on malformed replies only; every well-formed recording is byte-identical. Needs explicit authorization to land (not under the refactor-series standing authorization).Diff composition (head
dbef9a766c, vs the merge-base withorigin/main,f78483ec29)baselinerepinned to the last fenced-path commit +recorderSha256, which this branch moves because it adds an adapter module); 8 body-moved: 6 matrices + 2 named prefill scenarios (the delta); 11 added (three new source-control families + the provider pass-through pin)RpcIncompatibleReplyError.message, unchecked-reader ratchet inventory,openEnuminsrc/shared/zod-salvage.tsapplied at 10 sites over 7 enum sets, andhostedReviewproviderpassed through as a string because it is sent backgit.commitCompareis observable),lowlightas a real library in the substitutes table, README "Recording a behaviour change", a ten-minute recorder budget in place of the 120 s onedocs/reference/remote-wire-compatibility.mdThe delta, complete (13 goldens; every moved partition is a malformed or refused reply, 0
normalpartitions)Before column recorded at the pre-refactor tree on this branch (
4b0009d414), after column at the tip. Six pre-existing matrices moved body; five of the new families' matrices diverge between before and after (the other two new matrices,worktree.showandrepo.list, were already checked and did not move). Two named prefill recordings moved as well, from the round-2 fix; they are the last row.git.history-read/git.history#1TypeError: Cannot read properties of null (reading 'items')The host sent a reply this app could not read (git.history)hostedReview.eligibility+create-intent×2 /getCreationEligibilityhostedReview.create-chain+create-intent/hostedReview.createCannot read properties of undefined (reading 'ok')git.changes-load/git.status#1(new)readyover the malformed payload ({"error":"refused"}/null) and then fetched a branch compare against itgit.changes-load/git.branchCompare#1(new)git.history-commit-files/git.history#1,git.commitCompare#1(new)git.branch-diff-preview/git.branchDiff#1(new)Cannot read properties of undefined (reading 'kind')shown as error copy; inner-* claimed "Binary branch diff preview unavailable on mobile"hostedReview.create-intent×2 /getCreationEligibility, plussc-prefill-unavailable-on-refusaland-on-rejection(round 2)This branch is not ready for a pull request yet.; prefill carriedcanCreate: falseOrca could not confirm whether this branch already has a pull request. Try again in a moment.; the prefill no longer carriescanCreate. The fallback prefill's false flag short-circuited the unavailable check, so main showed the wrong copy on a refused or rejected lookup and the right one on a malformed reply only by accident.Contract
unchecked-rpc-reader-inventory.ts+ boundary test, 201 unchecked readers across 42 files on main (the count only goes down; bumping an entry by hand failslisted 4, found 5). Commit 2 the readers (source-control off the list, 201 → 184; after merging refactor(mobile): send the subscription-gated holdouts through typed RpcOperations (step 6, migration 1) #20954 the tree holds 187 across 38 files against main's 204 across 43, the three new lines named in the inventory);git show --staton it touches nothing undermobile/rpc-foundation. Then the matrix re-record, schema unit tests, readable error, item-4 recordings, merge of main, repin + full re-record.openEnum(values, fallback): an arm this build does not know degrades to the arm the reader handles most conservatively — compare status →error, entry/file status →modified, the three conflict sets → absent; absence and a non-string stay fatal; never.catch()). A fallback may shape a reading and nothing else: a member the client sends back to the host is passed through asz.string(), never coerced, because a fallback there rewrites the bytes instead of softening a read. Both are Rule 4 indocs/reference/remote-wire-compatibility.md.git.statusareais open too, asopenEnum(GIT_STAGING_AREA, undefined): every arm grants an affordance (stage, unstage, commit) and every reader is an equality check against a known one, so an absent area withholds all three and lands the row in no section. Keeping the row matters — dropping it would also drop it from the unresolved-conflict gate, which grants create on a conflicted worktree. Three requirements came from goldens rather than types:git.historysendstimestamp: null;getCreationEligibilitysendsreviewLookupOutcome: 'none'(absent fromHostedReviewLookupOutcome); zod 4 treats a barez.unknown()member as required.--recordrefuses any product tree that differs from the pin. Recipe (now in the README): repin to the last commit touching a fenced path (now9e01c50e6d, the provider pass-through), re-record everything, prove by value-pool decode that only the disclosed set moved. After squash-merge this pin is unreachable until a follow-up main repin (precedent fix(mobile): two known main bugs the RPC migration preserved #20563 → fix(lint): merge duplicate type imports in the mobile RPC recorder adapters #20895).RpcIncompatibleReplyError.messageis human-readable; the machine token stays ascode: incompatible_replyandname. Recorded settlements show both. The cross-bundle fallback inisRpcIncompatibleReplyErrormoved from a message-prefix match to a name match.Evidence
baseline+recorderSha256only), 8 body (the six pre-existing matrices above plus the two named prefill scenarios), 10 added, 0 deleted, 0 other; onebaseline/recorderSha256across all 689; the four recorder refusal texts absent.items→.optional()failsinner-ok-missing(expected "Error", actual "TypeError"); (b)gitHistoryReadback torpcUncheckedPayloadReaderfailsresult-absentand trips the ratchet; (c).strict()rejects a newer host's reply at the top level and, on the entry schema, shows a dirty worktree an empty Changes list.shallow-base,codeberg,typechange) that main passed through → opened (above), 8 unit tests, one lead-run mutant (closing the compare status again) failsdegrades a compare summary status rather than refusing the whole compare; two salvage-report claims that nothing reads deleted; README step 4 now names the three digests a branch's own edits move; 2 dead type exports and 6 over-exported schemas removed; unreadhostKinddropped. Round-1 commits alone move 689 goldens header-only withbaselinethe only key (no matrix partition scripts an unknown enum arm, so the corpus cannot see the fix; the unit tests are its oracle).canCreate: falsewas a determination nobody had made — dropped from both branches, which is the delta row above;areaopened to absent rather than staying closed, with the readers narrowed so a placeless row cannot reach a queue or a stage call; compareentriesaccepts an explicitnull;MobileGitStatusProjection,uncheckedReaderCountand the deadunchecked-rpc-reader-inventory.tsself-entry un-exported or removed;isRpcIncompatibleReplyErrordeleted (no caller insrc/or e2e). Declined, with reasons: wideningblockedReasonfromstringto a closed union is copy-module churn for a field only rendered, and the recorder's ten-minute budget stays a budget rather than a streamed progress report, because the recorder is outside the golden fence and a timeout now names itself.recorderSha256on every golden and nothing else — decoded against the pre-merge corpus, 689 header-only onbaseline+recorderSha256, 0 body, 0 deleted, and main's fifteen new step-6 goldens re-record byte-identical in body. The merge also brought three unchecked readers the ratchet had never seen, so the inventory raises those lines and names the PR; the ceiling binds this branch, not main.providerfallback was a send-side coercion. The eligibility reply names the provider and the create call returns it, and the action-sheet Create path has no provider gate, soopenEnum(..., 'unsupported')did not soften a reading — it putunsupportedon the wire and a host that had just named its own provider refused it as unsupported. That is a behaviour change on a well-formed reply, the one class this PR promised not to touch. Fixed by passing the token through asz.string()from the reply to the create params;supportsHostedReviewCreation()stays the allow-list and its parameter widens tostring, since answering for a token this build does not list is the whole job. Pinned bysc-create-intent-unlisted-provider, whose eligibility reply namescodebergand whose recordedhostedReview.createparams carry it back unchanged; restoring the old fallback fails that golden alone, onRequest params mismatch: hostedReview.create#1. The worktree-link switch gained adefault, which also closes an older hole: an unrecognised provider used to fall out of it asundefinedparams. Rule 4 now carries the send-side sentence.--recordexit 1" was the recording script's own 120 s timeout killing a ~160 s re-record under load.mobile/scripts/rpc-recording.mts(outside the golden fence) now budgets ten minutes and reports a timeout instead of a silent exit 1.Gates
ORCA_BACKGROUND_LAUNCH=1: rootpnpm tc0; mobilepnpm run typecheckfrommobile/0; full mobile suite 598 files / 5650 passed / 6 skipped; desktopsrc/shared737 files / 8239 passed;check:code-quality:changed0 across 39 files (all six sub-gates);format:check0 over 2590 files; oxlint 0 on the 39 changed files; readme-local-links and max-lines-ratchet 0.