mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
Jinwoo Hong
0817476b2c
* fix(mobile): encode the host id the native list hands the shell `web.tsx` encodes the host id into the pathname it opens the shell on; the worktree-list route beside it still interpolated it raw. `useLocalSearchParams` answers the decoded value, so a host id carrying `?`, `#` or whitespace builds a pathname that is no longer one segment. That shape is not refused where it is built. `matchesRoutePattern` splits on `/` alone, so `/h/a?b` reads as the single segment `/h/[hostId]` names and the session starts; the bridge's pathname rule is what refuses it, one `init` later, and the shell turns that refusal into `document-load-failed`. The route ends on a failure screen instead of the native list it already has and was about to render anyway. The fix sits at the interpolation rather than at the pattern or the bridge, because the other two are right: the pathname rule is what a path may be, and the page decodes the segment back when it matches `[hostId]`, so the screen it opens is the same one. A deep link is the way such an id arrives, which is what the sibling route's own test already establishes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): end a route segment at the query, not only at a slash The dot-segment lookahead both route patterns are built from treated `/` and end-of-string as the only things that close a segment. An href may carry a query, so the last segment can also be closed by `?`, and there the lookahead never fired: `/h/..?x`, `/h/%2e%2e?x` and `/h/.?x` all passed `BRIDGE_ROUTE_HREF_PATTERN` while their slash-terminated spellings were refused. The sink is `router.push`, and a URL parser resolves `/h/..?x` to `/?x` exactly as it resolves `/h/../x` to `/x`. That is the climb out of the `/h/` prefix the rule exists to stop, reached through the one punctuation the rule did not treat as a boundary. Fixed in `BRIDGE_ROUTE_SEGMENT_SOURCE`, which is the single place the segment rule is written and the reason the two patterns cannot drift apart. The pathname pattern is unaffected: a `?` fails its character class wherever it appears, so widening the boundary cannot let anything new through there. The existing segment-rule block gains the query-terminated spellings beside the ones it already pins. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say which notifies actually reach the mount-order throw The header said a call before `init` is a mount-order bug and throws, and named `notifyPageFault` as the one exception. Two more never reach that throw: a grant is read off the session, so before `init` there is no grant either, and the `&&` in `navigate` and `storage` short-circuits before `post` can require one. The code is right and the comment was not, so the comment is what changed. False is already these two members' refusal answer — it is what they give a shell that withheld the grant — and both callers handle it. `useRouteHandoff` calls `notifyNavigate` uncaught inside `push` and falls back to routing inside the page, so making this path throw would turn an early tap into an unhandled error in a handler nobody wrapped, which is the same reason the close path answers inertly rather than throwing. Pinned rather than left to the prose: the two gated notifies answer false and post nothing before `init`, the two ungated ones still throw, and the gated ones post once the shell has granted them. Not a red-first test — there is no defect here to reproduce — but the contract now has a test holding it in place. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): re-arm the shell session the route rebuilt, not only the host Two effects share one session. The first rebuilds it from `hostId` and `routePathname`; the second is the only thing that ever tells the reducer what the gates say, and it listed the host alone. A fresh session starts in `checking` and moves on nothing but `gates-changed`, so a route that changed under an unchanged host and unchanged gates threw the old session away and left the new one with no effect to run and no verdict to wait for. `routePathname` joins the gates effect's dependency list, beside the `hostId` that is already there for the same reason: both are what rebuild the session above, so both have to re-arm it. Fixing it in the dependency list rather than by having the reducer restart on a repeat verdict keeps the reducer's rule intact — a repeat verdict genuinely is nothing new — and keeps the coupling stated where the coupling lives. No caller can reach this today: both routes derive the pathname from the host id, so the one cannot change without the other. The test drives the hook directly and holds the invariant the wiring is supposed to have, since the thing protecting it was a property of the call sites and not of this hook. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): persist the last-visited worktree through the mirrored writer `writeLastVisitedWorktree` noted the write on the mirror and then dropped the store's promise with `void`. The mirror reports the key as written the moment it is noted, so a store that refuses the write leaves a value the page is handed on every `init` and that nothing ever persisted, and the rejection escapes as an unhandled one because no caller above it holds a catch. `writeMirroredStorage` in the same module is already exactly this: note first, persist second, and swallow the rejection deliberately, because a pin that failed to persist is not a reason to take the workspace off screen. This writer had grown its own copy of that pair without the last part. Reusing it rather than adding a local `.catch` is what stops the two copies drifting again, and it is the boundary that owns the relationship between the mirror and the store. The test drives a store that refuses the write and listens for an unhandled rejection, which is the failure the `void` produced and the only way to observe it from inside a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): pin the re-arm oracle, and move the query rationale to the rule Two review nits from round 1, neither changing behaviour. The re-arm test asserted the session was no longer `checking`, which a failure state satisfies just as well as a recovery does — the test would have passed on the opposite of what it is for. It now pins `native-route`, which is the state a re-armed session actually settles on here: `/h/host-1/tasks` is not the route the bundle lists, so the reducer answers with the native screen. The sentence about `?` closing a segment sat in the doc block for the `init` pathname bounds, which opens by saying that pathname carries no query. Read top to bottom the block contradicted itself. The rationale belongs beside `BRIDGE_ROUTE_SEGMENT_SOURCE`, where the shared rule is written and where the reason is legible: an href carries a query even though a pathname does not, both are held to the one segment rule, and widening its boundary cannot loosen the pathname pattern because a `?` fails that character class anywhere. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state what the native router really does with a dot segment Round 2 review, comment text only. The rationale beside `BRIDGE_ROUTE_SEGMENT_SOURCE` claimed `/h/..?x` resolves to `/?x`, borrowing the climb `history.replaceState` performs on the `init` pathname. That is the wrong sink. An href's sink is the native router, and expo-router's `resolveHrefStringWithSegments` normalises only an href beginning with `.`; a rooted one is passed through, its query stripped, and the forked `getStateFromPath` then matches segments literally against the route patterns. A dynamic segment compiles to `([^/]+\/)`, which takes `..` as happily as any other value. So the harm is not a climb and it is not Unmatched either: `..` is read as the `[hostId]` a screen is opened for, and the shell opens a host screen for an id no host has. A different wrong screen from the slash-terminated spellings, and the same reason one rule covers both patterns. The boundary and the test that pins it are unchanged; only the sentences describing them are. The notify header opened by saying three of the four share one guard, one paragraph above the one explaining that only two ever reach its throw. It now says both in the same breath. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb