Commit Graph
3 Commits
Author SHA1 Message Date
Jinwoo Hong 7d98c8e2f3 refactor(mobile): send the source-control domain through typed RpcOperations (#20544)
* test(mobile): record main's source-control RPC behaviour before migrating it

45 scenarios over 11 source-control senders, recorded from main so the step-4
migration has a frozen answer to compare against. Adapters mount the real
exported senders as plain functions, so no React host or device is needed.

The 73 existing goldens change header-only (`baseline`, `recorderSha256`): any
new scenario re-digests the recorder, and the pinned baseline had drifted from
main in `src/shared` so recording required bumping it. Content is byte-identical
on all 73 — verified field-by-field against HEAD.

Scenarios deliberately pin the empty-message cases (`sc-*-refused-empty-message`,
`sc-*-rejected-empty-message`), because a refusal with no message falls back to
the screen's copy while a transport error with no message does not, and the two
paths are easy to collapse when a call site moves behind an acceptance policy.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* refactor(mobile): send the source-control domain through typed RpcOperations

13 of the domain's 14 files now send through a declared operation instead of the
raw request port: 44 references to 0. The holdout is use-mobile-git-requests.ts,
whose single reference is a `(method: string, params)` dispatcher that five other
hooks feed `{ method, params }` action steps at runtime; typing it is a step model
change, not a call-site move, so its line stays at 1.

Fifteen operations over fourteen methods. Two of them read git.status, and that is
deliberate: the Changes screen publishes the host payload verbatim while
hosted-review preparation reads the normalized projection, which returns null when
`entries` is not an array and drops entries missing a path. Sharing the projecting
reader would change what the Changes list renders, so both are named.

Four loads still read the refusal envelope before interpreting, through
readMobileGitRefusal: two degrade to a capability-missing screen, one retries a
selector that is not visible yet, and one falls back from files.openDiff to
files.open. `isMobileGitUnavailable` consults the code *and* the message and no
acceptance policy carries either through, so the alternative was parsing a code out
of a message. No new acceptance policy was added.

Every migrated site keeps two error paths where it had two: a refusal with no
message falls back to the screen's copy, a transport rejection surfaces its own
message verbatim and keeps its delivery-unknown mark. Collapsing them into one catch
is what would have turned an unknown mutation into a failed one.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): re-digest the goldens after a lint fix in the new adapter

recorderSha256 only, all 118 files; every recorded observation is byte-identical.
Re-recorded from c57de48fd0 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

* test(mobile): drive the reply matrix over every scripted reply, and fail 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

* fix(mobile): keep main's in-band commit error pass-through

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

* test(mobile): re-digest the goldens for the merged recorder

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 16d1ab81d3 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 7ce8e18d07 to 16d1ab81d3 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

* test(mobile): intern each observation entry instead of the whole field

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 16d1ab81d3 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

* test(mobile): make the matrix census and inventory checks able to fail

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 16d1ab81d3 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
2026-09-14 01:02:28 -04:00
Jinwoo Hong c21c083224 fix(auth): report callback failures instead of cancellation (#20535)
* fix(auth): distinguish failed sign-ins from user cancellation

* test(mobile): fix conditional registration lint and refresh recorder fingerprints
2026-09-13 22:15:15 -04:00
Jinwoo Hong 7ce8e18d07 test(mobile): consolidate the RPC migration's verification infrastructure (#20521)
* test(mobile): record main RPC hooks and regression schedules

Add scripted sender recordings, guarded main goldens, reply matrices, lifecycle schedules, settings caller fixtures, and targeted B-seed mutants.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): flush recording user actions through React act

Keep lifecycle updates in separate act boundaries while wrapping direct stateful user actions.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): compile recorded modules with the Node VM API

Use the same trusted-source execution boundary as existing mobile VM test harnesses.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): pool golden values and hoist pre-divergence checkpoints

Golden format version 2 stores each distinct observation field value once in
a `values` map keyed by a 12-hex sha256 of its sorted-key JSON, and a
checkpoint references five hashes. Output stays pretty-printed; the reader
rejects any other format version, resolves hashes back to values, and reports
the scenario, checkpoint, field and JSON path on a mismatch.

Generated variants now declare where their distinguishing input lands, so
checkpoints observed before that point are recorded once in a `.prelude`
scenario instead of once per reply partition. Reply matrices, interruption
schedules and lifecycle schedules share the primitive, which asserts each
variant's pre-divergence prefix matches the base. Equal-but-differently-reached
checkpoints are untouched.

17.71 MB / 3,599 checkpoints / 58.4% intra-file duplicates becomes
4.21 MB / 1,961 checkpoints / 23.6%, with every file's set of distinct
observations unchanged.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): make the recordings sense deadlines, the recorder, and every family

The goldens carried no temporal information, so a request deadline could be cut
to a third and all 61 files stayed byte-identical. Every threshold is now
straddled by two advances with a checkpoint between them: the 30 s request
deadline in both schedule drivers, the 120 ms search debounce in b1, and the 60 s
repo-metadata cache TTL. Shortening any of them moves an observation.

The record fence pinned product sources but excluded the whole recorder, so
--record could rewrite every golden from a modified runner and report the
baseline intact. Goldens now pin recorderSha256 over every non-markdown file in
the runner plus pilot-scenarios.json, and the fence exemption shrinks to the one
directory that digest covers.

Mutation evidence covered 3 of 13 mounted operations. There is now one anchored
mutant per adapter family, covering 11 operations and 51 of the 61 goldens; the
two omitted are the pure async loaders whose entire output is their settlement.
Anchors are asserted to match exactly one site, which caught the acceptance
mutant silently half-applying against three identical guards.

The archived-tree assertion pins each seed's visible state instead of merely
differing from main, and error observations carry code and cause when present.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): record settlement times instead of straddling deadlines

The previous commit made the reviewer's divide-by-three deadline mutant fail by
placing checkpoints on each side of the 30 s deadline. That is a patch: a timing
change that does not cross a hand-placed boundary stays invisible. Those
scenario edits are reverted, and pilot-scenarios.json and schedule-driver.ts are
byte-identical to what they were before them.

The real defect was that the projection had no temporal dimension, so every
settlement now carries startedAt and settledAt in virtual milliseconds on the
pinned fake clock. Any transition the product schedules for itself is recorded
at the time it actually fires, so a deadline or debounce change of any size, in
either direction, moves a recorded number.

A checkpoint's own clock is not recorded. It is always the sum of the scripted
advances, so it is a function of the scenario rather than of the code under
test; run-recording.ts asserts that equality at every checkpoint instead, which
costs no bytes and fails loudly if it ever drifts.

projectionVersion is 2 and all 61 goldens are re-recorded. With the added
timestamps stripped, the distinct-observation set is identical to the previous
recording, so the change is purely additive.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): probe the repo-metadata cache inside its TTL window

Recorded settlement times cover thresholds the product schedules for itself, but
not one it only consults when something else makes it act. The repo-metadata TTL
is the single such case: with probes only at 0 s and 60 s, a 20 s TTL and a 60 s
TTL are both expired at 60 s and record identically, so a 3x cache-lifetime
regression was invisible.

settings-repo-cache-expiry now probes the cache at 59 s as well. This is
coverage, not a substitute for recorded time: it bounds how small a TTL
reduction is visible rather than making the reduction itself observable, and the
README says so.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): record the reply shapes a host can send, not a cross product

The reply matrix froze ~26 malformed envelopes crossed against every consumed
field and three boundary kinds, which is 163,925 lines of JSON pinning accidents
on inputs no desktop produces. `successResponse` always sets `result`, so a JSON
wire has no explicit-undefined slot, and no mounted handler returns a number, a
string, an array, a bare `{}` or a boolean: `settings.get` returns
`{settings: ...}`, and the seed methods return an object or nothing.

Each family now runs nine witnessed partitions once, with no field cross: a
normal result, an absent result, `null`, an inner `{ok: false}` envelope with a
string or an object error, an inner envelope missing `ok`, an outer refusal,
`method_not_found`, and a transport rejection. `null` stays because
`linear.getIssue` returns it for a missing issue and b2 is a shipped null-result
bug; it is also what carries the one named delta these goldens record.

`run-step1-exit.ts` had zero callers and shelled out to the same two Vitest
files as `rpc-recording.mts`, so it and its README paragraph go, along with
`MUTATION_NAMES`, which only it read.

In the module loader, the `rpc-delivery-ambiguity` escape is measured dead: over
every scenario, mutant and reference run it was taken once, by the test that
existed to take it. Golden comparison already fails loudly if a mounted module
ever imports the marker, so both go. The history-panel exposure moves into a
declarative table beside the mutation anchors, leaving the loader with one
source-text mechanism and no per-file branch.

The VM stays. Mount adapters load product sources from an arbitrary `root`, and
the archived 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
2026-09-13 20:56:21 -04:00