mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
Jinwoo Hong
211821dc17
* fix(mobile): refuse a page target the shell will not take instead of opening it here
`useRouteHandoff`'s web sibling answered two things — handed off, or push it
locally — and fell through to the local router for three different reasons. Only
one of them is a page route. An href the protocol's own pattern drops and a shell
that answered no are the page reaching past what this shell can serve, and the
bundle carries every route under `app/h`, so the fallback does not paint
Unmatched: it mounts `session/[worktreeId]` on React Native Web inside the shell.
The outcome is now tri-state. A target outside `pageRoutes` is never pushed
locally; the page stays where it is and names the reason once per client, which
is the bound the other page-side reporters take.
Proved in the render check against the real bundle: with the double granting no
`navigate`, "Back to hosts" left the host route for `/` and painted Unmatched
before this, and now stays put, posts nothing and reports no page fault.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): encode the host id the worktree row's navigation actions build
Both targets this sheet offers interpolated `hostId` raw — the C1.2 class the
C1.8 stack fixed at the route files and `web.tsx`, at the last two sites that
still had it. `useLocalSearchParams` answers the decoded value, so a deep-linked
id carrying `?`, `#` or whitespace stops being one segment.
It matters more from C5.1 on. Inside the page these targets go through
`useRouteHandoff`, which matches the pathname against the shell's `pageRoutes`
before deciding anything, and the id is the segment the pattern is reading.
The worktree id was already encoded at both sites; this makes the host id match,
and the new test pins all four targets rather than only the one that moved.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile): render agent session history from the desktop's bundle (OTA phase C, C5.1)
The second page route. `agent-history/[worktreeId].tsx` already shipped in the
bundle with its own chunk, so listing it adds nothing to the download and moves
no route count: the switch is `index.tsx`'s, and the shell still decides, because
a bundle naming a grant this app lacks renders the native panel instead.
Its `.web.tsx` sibling is required for `index.web.tsx`'s reason — the native file
reaches OrcaMobileWebShellView, whose requireNativeViewManager runs at import and
takes the whole bundle down in a browser, since the manifest imports every route.
First route with two dynamic segments, so both are encoded. Grants are `navigate`
and `storage`: a resumed session opens the native session screen, the worktree
list now reaches this screen without leaving the page, and `app/h/_layout.tsx`
reads the app's own sidebar width above every page route.
The panel's router becomes `useRouteHandoff`, which is the seam that tells those
two apart: agent history is a page route and is pushed here, the session screen is
not and goes to the shell.
The three writes a resume makes needed no page-side handling and have none. What
they needed was a test that the descriptor's handling survives the extra hop, so
each is run through the bridge and against the same fake directly and the two
verdicts compared: a refused create raises the host's message, and a lost reply or
a shell disposed mid-flight stays delivery-unknown rather than becoming a failure
a user would retry blindly. No golden covers those three.
The flag census grows its first entry since C1.3, which is what it is for.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): pin the agent-history Back button to the shell handoff
C5.1 wired this button by swapping the panel's `useRouter` for `useRouteHandoff`;
nothing else was needed, because `RouteHandoff` is the router's own shape and the
seam's web sibling decides `back`. So this commit is the test that would have
caught the wiring being absent, not a fix.
Red before the merge, green after, on the same four cases: on `e897e8123a`, where
`back` was still expo-router's own spread member, 3 failed and 1 passed — the one
that passed is the local-pop case, which is the branch C2.2 did not change. After
the merge brought in C2.2's `back`, all 4 pass. The pre-merge run named the notify
by its literal `'navigate-back'` because the contract constant did not exist yet;
it is the same string `BRIDGE_NAVIGATE_BACK_NOTIFY` holds, so the two runs asked
the same question.
Both module substitutions are the builder's own rather than conveniences: the web
bundle resolves `route-handoff` and `client-context` to their `.web` siblings, so
mocking each to its sibling gives this screen the module graph it has inside the
page. The frames are read off the port pair's lane rather than off a spy, and one
case asserts a frame crossed at all before either absence is read as an answer.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): compare the resume's second write, not only its first
Round 1, finding 1. All three `resumeAiVaultSessionInTerminal` cases settled the
create (`ai-vault-resume-launch.ts:158`), so `terminal.send` had never crossed the
bridge and the half of the resume that types the command into the pane was
uncompared. Three cases now drive both writes: a refused send raises the host's
message, an accepted send reporting `accepted: false` in-band says "Terminal input
is locked", and a send the host takes resolves — the last one being the presence
precondition, since a run that failed at the create would give the same shape of
verdict as one that failed at the send.
Reading `requests[1]` straight after settling the create finds nothing on the
bridged leg: the second write is made only once the first settles, so it is two
more lane round trips away. `nthRequest` waits instead, and says how many it saw
when it gives up, so this cannot pass by proving the opposite of what it says.
The locked reply is `{ send: { accepted: false } }`, not `{ accepted: false }`:
the reader is `reply.send?.accepted !== false` (`review-terminal-reply-schema.ts:65`),
and the flat shape resolves rather than throwing. Written the flat way first, both
legs agreed on "(resolved)", which is the comparison doing its job.
Also finding 1's second half: the file docstring claimed every case runs twice and
differences the verdicts, which was false for the dispose case — a fake RPC client
has no door to shut, so there is no native run to compare against. The docstring
now says so and the case carries the same note.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): pin what encoding cannot save about a dot-segment id
Round 1, finding 2. The route's docstring listed `/ ? #` and whitespace and the
encoding test covered five ids of that kind, which together implied encoding makes
any id safe. It does not: `encodeURIComponent('..')` is `'..'`, so the pathname
reaches `BRIDGE_ROUTE_PATHNAME_PATTERN` intact, fails the lookahead that stops a
climb out of `/h/` (`bridge-caps.ts:68`, read through `bridge-envelope.ts:117`),
and the shell answers with `reportShellFailure` — a failure screen where the route
would otherwise have rendered the native panel it already has.
Pinned, not fixed, and the docstring now says which. `app/h/[hostId]/index.tsx`
builds its pathname identically and has the same hole, so this series fixing one
of two call sites would leave the shape behind and stop describing it. The new
case asserts both halves — the segment survives encoding unchanged, and the
pattern refuses the pathname — so a later change that starts encoding dots fails
here and has to say which screen it wants instead.
Characterisation, so it was green on the first run rather than red: the claim is
about behaviour that already ships, and the value is that the refusal is on record.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): mount the agent-history route in a real browser
Round 1, finding 3. Nothing rendered this route's real module graph anywhere. The
unit tests mock react-native, safe-area, svg, lucide and the icon assets away —
they have to, since react-native is Flow source vitest cannot parse — so a
component in this closure with no web build would have reached a device before it
reached a test. The render check is the only place the graph meets React Native
Web, and this route was not in it.
Two cases. The first mounts the route from the shell double and reads the screen:
"Agent Session History" and the worktree label the params half carried, no fault,
no console error, no CSP refusal, and the URL the page wrote for itself. That also
proves `init.route.params` end to end on a route that has a dynamic segment too,
which §1 of the design claimed and nothing checked.
The second pins the chunk. C5 is the first series whose success path pulls a
second chunk after the first paint, which on iOS goes through WKURLSchemeHandler
under `script-src 'self'`. The chunk is named from the builder's own route map
rather than guessed from the bytes, and asserted absent from what the first route
loaded, so this says the route came over the wire now.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): say the real lifetime of the route-handoff refusal set
Round 1, finding 4. The comment claimed one line per reason "for the life of one
client", borrowing `createPageDiagnosticReporter`'s bound. The set is built inside
the `useMemo` keyed on `[client, router]`, so it is per hook instance: in practice
the memo is not recomputed, because `useRouter()` is expo-router's module
singleton and the page holds one client, but every screen calling the hook gets
its own set and a reason can be reported once per screen rather than once per
document.
Says that now, and why it is not tightened: a per-module set would outlive the
page's client, which is the lifetime the rest of these reporters are scoped to,
and there is no document-wide reporter to join without reaching into a contract
file the C2 lane owns.
Records the other half of the finding too, which came back confirmed rather than
changed: `console.warn` is right here. It is the vocabulary `page-bootstrap.ts:35`
already writes in, and a `fault` notify would be wrong twice — the shell drops the
generation on a page fault, and a navigation the page declined is not a failure.
Comment only; no behaviour change, 25 navigation tests unchanged and green.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): wrap every router member that takes a target, not three of five
Round 2, finding 1. `...router` hands through everything this file does not name,
and two of the members it did not name take an href: `navigate` and `prefetch`.
`navigate` to a route outside `pageRoutes` went straight to expo-router and pushed
it into this document — the hole the tri-state exists to close, reopened under a
name nobody had looked at. No call site uses it today, which is why it shipped.
`navigate` is now wrapped exactly as `push` is: which of push-or-collapse it does
is a decision about this document's stack, and a target outside this document has
no such stack.
`prefetch` is decided the other way, explicitly. It is the one target-taker that
must never reach the shell: a prefetch is a background load, `navigate` is the
only thing the shell can be told, so handing one over would open a screen nobody
asked for. A route this document serves is prefetched here, which is what the
per-route chunk split makes worth doing; every other one is dropped without a
line, because a warm-up that did not happen is not a failure to report.
The docstring's "four members that can leave this document" is now five wrapped
members and a rule for which is which.
A list would rot, so the pin is derived: `HrefTakingRouterMember` reads the
parameter tuple of every member of `RouteHandoff` and `WRAPPED_HREF_MEMBERS` is
asserted equal to it in both directions. It reads the tuple rather than testing
assignability because `() => void` is assignable to `(href: RouterHref) => void`,
which would make `back`, `dismissAll` and `reload` target-takers and prove
nothing. Checked both ways: dropping `prefetch` from the list fails the compile
with "Type 'HrefTakingRouterMember' does not satisfy the constraint", and the
union resolves to exactly the five, with `back` and `setParams` outside it.
The pin is in the product module because `mobile/tsconfig.json` excludes tests.
The runtime test asserts each wrapped member is not the router's own function and
that `setParams` still is, so a hook that wrapped everything fails too.
Red first: 4 of the new cases fail against the previous file, 33 pass now.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): say per hook instance in the title too, not per client
Round 2, finding 2. The source comment was corrected in round 1 and this test's
title was not, so the two disagreed about the bound the refusal set actually has:
the set lives in the `useMemo`, so it is per hook instance, and a title claiming
per client is the stronger promise the code does not make.
Title only; the case and its assertions are unchanged.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): say what the agent-history render case does not cover
Round 2, finding 3. The docstring claimed the case is where the panel's closure
meets React Native Web, which overstates it. The shell double answers no RPC, so
the session scan fails and the panel paints its "Unable to Load" state: the
session list, its rows, the resume button and the scope tabs never render, and a
render-time gap inside any of them would pass this check.
Now says both halves — import-time evaluation of every module in the closure and
the panel's own chrome are covered, the list subtree is not — and names what
covering the rest would take: a double that answers `aiVault.listSessions`, which
is a different instrument and would put domain behaviour in this file.
Text only. This case moves to its own file on the extracted harness after the
merge with #21592; the corrected text travels with it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): import the handoff module once in its own test
My round-2 fold added `WRAPPED_HREF_MEMBERS` as a second import of
`./route-handoff.web`, which `import(no-duplicates)` fails in the focused-plugins
pass of the changed-code gate. Joined to the existing import below the mocks,
which is where an import of the module under test has to sit in this file.
Found by running the changed-code gate rather than by review: mobile tsc, whole
tree oxlint and the suite were all green with it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): give agent-history its own render file on the extracted harness
The render check gained browser cases from three domain series at once, each
under the `.mjs` cap of 600 counted lines alone and no two of them together: at
39b15e395d the file was 781 raw lines and clean, main's was 786 and clean, and
their merge was 857 raw and 623 counted, which is the CI red on #21596. C1.10
extracted the harness so a domain gets a file instead. This is C5's, and the
render check is back to 639 raw lines and clean.
Five cases. The two that moved — the route mounts and paints, and its chunk is
fetched on navigation — plus three new ones.
Back, twice. With `navigate` granted the page's Back control posts exactly one
`navigate-back` notify and the page does not move; with the grant withheld the
same tap reaches the same handler and posts nothing. The pair is the point: the
document holds the single history entry the entry wrote with `replaceState`, so a
Back this page served itself would also have gone nowhere and looked identical.
This is the first proof of that handoff in a browser rather than against a mocked
router.
And a row. The harness's new `replies` lets the double answer named methods, so
the panel now renders a real session instead of its "Unable to Load" state, which
is the render-time gap the round-2 docstring conceded. Assertions are on the row's
own text and message count, plus the absence of both silent states — the scan
failing, and a session out of scope.
Replies lifted from the corpus, and one of them needed two scenarios. The session
and worktree lists are `aivault-history-screen-listed`'s. Its `status.get` is a
capability list alone, and the first run painted "Update Orca on your computer":
`HostProtocolGate` above every host route reads the same method for fields that
scenario never scripts. The status reply merges those from
`transport-host-status-gates-ready`, and the comment says why two.
`wt-history` is load-bearing, not incidental. The panel opens on the `workspace`
scope and filters by paths from the worktree list, so on any other worktree these
same replies paint "No agent sessions" — green, and proving nothing.
Registered in the `mobile_web_app` job beside the drawer check, which is the job
that makes a missing mobile install fail rather than skip.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): encode the host id at every href the host page builds
Pullfrog on #21596. My earlier commit fixed the two hrefs in the row's navigation
sheet and stopped there; five more sites in the same page interpolate the decoded
id raw — Accounts and Tasks in both header layouts
(`host-screen-header.tsx:188,204,307,318`) and the session target
`openWorktreeSession` builds (`use-host-worktree-actions.ts:192`).
Same C1.2 class. The persisted host store admits any non-empty id and both
`useLocalSearchParams` and the store answer it decoded, so one carrying `/`, `?`,
`#` or whitespace stops being the single segment `matchesRoutePattern` reads.
Inside the page that decides where a tap goes, because the handoff matches the
pathname against the shell's `pageRoutes` before choosing this document or the
native stack.
A census rather than five more assertions: the failure is a habit, not a bug —
each of these was written by copying the one beside it, and the seventh will be
too. It counts `/h/${...}` interpolations across the host page's four source
files and requires `encodeURIComponent` at each, with a presence check so it
cannot pass on an empty list.
Two sites are exempt and stay raw: `use-host-worktree-actions.ts:171` and
`app/h/_layout.tsx:100` compare against a pathname the router answers rather than
building a link, so encoding them would change what a comparison matches instead
of what a tap opens. The exemption is subtracted by count rather than matched
away, so a file that lost its comparison and gained a raw target does not come
out even.
ONE BEHAVIOURAL EDGE, named rather than fixed. `navigateFromHostList` short
-circuits when `pathname` equals the target minus its query. That comparison now
has an encoded target on one side and whatever `usePathname()` answers on the
other, so for a host id that needs encoding the short-circuit stops firing and a
tap on the screen you are already on re-navigates instead of doing nothing. It is
a redundant navigation, not a wrong one, and the guard at :171 is unaffected
because it compares against the same raw form it always did. Left alone because
fixing it means deciding what `usePathname()` returns for an encoded segment,
which is a question worth its own change.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): guard the optional host id the session target encodes
The commit before this one did not typecheck: `useHostWorktreeActions` takes
`hostId` as `string | undefined`, and `encodeURIComponent` does not. I committed
on a green test run without waiting for `tsc`, which is my error and the reason
this is a second commit rather than an amend — the lane forbids rewriting a
commit that exists.
`?? ''` rather than a cast or a non-null assertion. An absent id then builds
`/h//session/...`, an empty segment the shell's own route rule refuses, instead
of the string "undefined", which that rule would accept as a host genuinely named
undefined. Every other member of this hook already guards the same field.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): keep the agent-history route native when the bridge would refuse its id
CodeRabbit on #21596. The persisted host store admits any non-empty id, so `.` or
`..` reaches this route, survives `encodeURIComponent` unchanged, and fails the
bridge's own segment rule. The route handed it over anyway: `bridge-host.ts`
parses the route against `BridgeInitRouteSchema`, drops it to null when it fails,
and the page answers an `init` naming no screen with "Update Orca to open this
workspace". A failure screen, in place of the native panel sitting right behind
this switch.
The route asks the schema first now and stays native when the answer is no, which
is where every route starts. Mirrors C3.1's call for the files routes
(`69e618e19a`), including its reason for using the schema rather than a copy of
its bounds: two spellings of one rule drift, and the half that matters is the
half the page reads.
The pin moves with it. It characterised the refusal before — asserting the
pathname was built and that the pattern rejected it — and now asserts the native
render, for a dot host id and for a dot worktree id, which is the other segment
and was never covered.
`app/h/[hostId]/index.tsx` has the same hole and is not fixed here, as asked: it
builds its pathname the same way and hands it over unchecked. When C3.1 is also
on main the two guards and `mobile-file-shell-route.ts` belong in one module
beside the schema, rather than a third spelling of a one-line call.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile): forward navigation options through the wrapped router members
CodeRabbit (major) on #21596. expo-router's `push`, `replace`, `navigate` and
`dismissTo` are `(href, options?)`, and the wrappers took the href alone. A local
push asking for `{ withAnchor: false }` reached the router without it, so inside
the page the router did something other than what the caller wrote — silently,
because dropping an optional argument is not a type error.
Each wrapper forwards both on its local branch now. Nothing in this tree passes
options today, which is why it went unnoticed and exactly why it needed pinning:
the first caller to pass one would have had it dropped without a word.
Options do not cross to the shell, and the docstring says so rather than leaving
it to be discovered. The `navigate` notify carries an href and nothing else, so a
target handed over is opened by the native stack on that stack's own terms. That
is the right shape — the options describe a push inside a document the shell's
target is not in — but it is a loss, and a loss worth naming.
Four existing assertions moved from `toHaveBeenCalledWith(href)` to
`(href, undefined)`. That is what the router now receives when a caller passes
none, and expo-router reads an undefined second argument as absent; the comment
above them says so, so the next reader does not take it for a bug.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): count the wrapped members the way the returned object does
Pullfrog on #21596. The header still described the set as it stood before
`860577cc30`: "the four members that can leave this document", "the three that
carry a target", "the other three". There are six wrapped members now and five
carry a target, so every count in the paragraph was one or two short and a reader
checking the object against the prose would have found neither explained.
Now says six wrapped, five target-takers named and pinned by
`WRAPPED_HREF_MEMBERS`, four decided by the shell's route list, `prefetch` the
fifth and decided differently for a reason the member's own comment gives, and
`back` the sixth carrying no target at all.
Comment only; 36 navigation tests unchanged and green.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb