Repository navigation
fix(repos): a project on an unreachable host can still leave Orca - #3
Conversation
Removing a project dispatches to the host that owns it. When that host never answered, the RPC rejection aborted the whole removal before the local purge and the user got a bare "Failed to remove project" — the row could not be removed at all while the host stayed down. Split that outcome out. A host that answers and refuses is still a failure. A host that never answers is `owner-unverifiable`: its catalog is untouched, so the row is kept and the caller is told, rather than the store spending a dead-end toast on it. The confirm dialog then stays open, names the host, and re-offers the removal as `mode: 'forget-local'` — which clears this client's records and dispatches nothing to the owner, terminal teardown included. Loss of contact is never evidence of absence, so the copy says plainly that nothing on the host is touched and the project returns if that host reconnects (docs/reference/ssh-execution-boundary.md). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review complete. 🟠 1 high 📍 Outside the diff (1)
🧹 Nitpicks (1) — 🟢 1 low
Adds a Reviewed commit: a47417c |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a47417c439
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * protocol mismatch, an error from its own catalog — is positive evidence and stays a failure. | ||
| */ | ||
| export function isOwnerContactFailure(error: unknown): boolean { | ||
| return isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error)) |
There was a problem hiding this comment.
When repo.rm reaches the host but its response is lost or arrives after the 15-second timeout, this classifier treats runtime_timeout/timeout as owner-unverifiable, while the new dialog and outcome documentation assert that the catalog is untouched, the project remains registered, and it will return after reconnecting. A timeout proves only that no response arrived—the operation may already have succeeded—so either use outcome-unknown wording or verify the host's durable catalog before making those claims.
AGENTS.md reference: AGENTS.md:L89-L91
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8e7e406 — you're right, and this was the most important finding of the batch.
The outcome name was honest but the copy attached to it was not. owner-unverifiable really does mean no answer arrived, which includes a repo.rm that landed and whose reply was lost past the 15s timeout. The dialog then went on to assert the opposite:
…so my-project is still registered there… nothing on alexdevbox2 is touched, and the project returns if that host reconnects.
Now:
Orca could not reach alexdevbox2, so whether my-project was removed there is unknown. Removing it now only clears this computer's records — if it is still registered on alexdevbox2, it returns when that host reconnects.
Nothing is asserted about the host; the return is conditional. The RemoveProjectOutcome doc and the store comments got the same treatment — they said "its catalog is untouched and the project is still registered there", they now say what the host did is unknown, and that neither reading may be asserted.
One thing worth stating rather than leaving implied, so I added it to the doc: retrying once the host answers is safe, because a removal that did land comes back as repo_not_found, which removeProject already tolerates and reconciles (stablyai#11994). That is what makes "reconnect and try again" real advice rather than a shrug, and it is why verifying the durable catalog up front is not needed — the retry does it.
- forget-local no longer reaches the ephemeral-VM teardown. That branch destroys the project's VM, which contradicts "clears this client's records only", so the mode is refused for a runtime-owned SSH target rather than leaking a live VM or quietly breaking the promise. Unreachable in practice — those repos dispatch locally and never answer owner-unverifiable — so this pins the invariant instead of relying on it. - Stop asserting what a lost answer cannot establish. A timeout proves only that no reply arrived; the host may already have removed the project. The outcome doc and the dialog now say the result is unknown and make the return conditional, and note that retrying is safe because a removal that did land comes back as the tolerated repo_not_found. - Guard a late removal against the dialog that replaced it. Cancel stays live during the host call and closeModal is global, so a stale completion could dismiss the next dialog. The reset effect bumps a token on teardown and the completion fences on it. - The settings toast names the host instead of printing a raw runtime: id, via the existing selectExecutionHostDisplayLabel — which also honours a user's per-host rename. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review complete. No blocking issues — approved ✅; 2 nitpicks below. 🧹 Nitpicks (2) — 🟢 2 low
Reworks project/repo removal into an explicit outcome model (removed / failed / owner-unverifiable) across the repo-removal store, SSH host workspace cleanup, and the remove-folder dialog, with tests. Reviewed commit: 8e7e406 |
|
All four findings addressed in 8e7e406. The two inline Codex threads have replies; the two from @tenki-reviewer are below. 🟠
|
|
Review complete. 🟡 4 medium 💬 Inline comments (3)
📍 Outside the diff (1)
Reviewed commit: 8e7e406 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expand to see previous review
This PR threads a structured RemoveProjectOutcome through remote project removal so SSH/ephemeral-owner removals distinguish confirmed removal from owner-unverifiable, updates the removal dialog and settings list copy accordingly, and adds tests — but one error path is misclassified and two callers don't fully honor the new contract.
A newer Tenki review found issues on this pull request.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e7e406115
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : idExistsOnOtherHost || forgetLocalOnly | ||
| ? window.api.repos.removeForHost({ repoId: projectId, hostId: ownerHostId }) |
There was a problem hiding this comment.
When a paired web client loses its runtime connection, the first removal returns owner-unverifiable, but the offered “Remove from Orca” retry reaches this removeForHost branch. The web implementation explicitly throws for every such call in src/renderer/src/web/preload-api/web-repositories-api.ts:32-35, so removeProject returns failed, the dialog closes, and the project remains. Handle the browser's client-only state locally or suppress the unsupported affordance there.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 98031b3 — took the "suppress the unsupported affordance" option.
You're right that the retry lands on removeForHost, which web-repositories-api.ts throws for unconditionally. The reason it throws is not an oversight, though, and that decided the fix: a paired web client has one runtime and keeps no records of its own. There is nothing client-only to clear there, so even a working implementation could only appear to succeed until the next repo.list put the row back.
So on a paired web client the dialog withholds the "Remove from Orca" button entirely and gets its own sentence, ending with "This client keeps no records of its own to clear, so reconnect {{host}} and try again" instead of offering an action guaranteed to fail. The desktop path is unchanged. I also left a note at the removeForHost throw pointing back at the dialog, so the next reader finds the affordance decision from the throw rather than rediscovering it.
| </Button> | ||
| <Button variant="destructive" onClick={handleConfirm}> | ||
| {translate('auto.components.sidebar.RemoveFolderDialog.4dc5b5065b', 'Remove')} | ||
| <Button variant="destructive" disabled={isRemoving} onClick={() => void handleConfirm()}> |
There was a problem hiding this comment.
When an owning runtime is unreachable, the awaited call can remain pending for the full 15-second timeout, but the dialog only disables the unchanged “Remove” button and shows no spinner or pending label. This makes the newly persistent dialog appear frozen during the exact failure path being added; the mandated docs/STYLEGUIDE.md:268-284 requires visible feedback for remote actions lasting more than a few seconds, ideally delayed briefly so fast removals do not flicker.
AGENTS.md reference: AGENTS.md:L1-L3
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 98031b3, following the section you cited.
The button already bound disabled immediately; what was missing was the visible state. It now shows a spinner on a 200ms timer, which is the guide's own prescription for an action that is fast locally and slow remotely — local removals return in milliseconds and show nothing, an unreachable runtime gets feedback well before its 15s timeout.
One deliberate deviation, flagged rather than hidden. The same section's corollary says to pre-reserve the space a control will later occupy, and this implementation grows the button by an icon when the spinner appears. I left it that way because HostRemoveDialog — the adjacent destructive host/project removal dialog in the same sidebar — uses exactly this pattern at line 291, and §2 of the styleguide says a component and its sibling should read as one design, with divergence needing a reason. The reason here would be "the sibling is wrong", which the guide says to fix in both; doing that inside a PR about project removal is scope I'd rather not take unannounced. Happy to do it as a follow-up across both dialogs if you'd prefer that over consistency.
Six review findings on the unreachable-owner removal path. A manual disconnect is the client refusing to dispatch, so the host never answered — it now reads as owner-unverifiable instead of a flat failure. The classification lives here rather than in RECOVERABLE_CODES, which means "retry or reconnect" and drives loops that would spin against an environment the user deliberately switched off. A lost answer proves nothing either way, so the copy stops promising the catalog is untouched and says the request may never have arrived or may have been carried out with the reply lost. The host name in that sentence now resolves the way removeProject does, so an ssh row or a modal opened without a hostId no longer renders a blank, and a missing environment record falls back to "that host" instead of a routing slug. A forget-local host clear forwards its mode for repo rows, so it can no longer dispatch repo.rm or tear down an ephemeral VM during an operation defined as touching nothing outside this client. A paired web client has no records of its own to clear, so it gets the explanation without a button whose only outcome was a throw. The removal spinner appears after 200ms, so a slow remote call reads as pending while a local one does not flicker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Review complete. No blocking issues — approved ✅; 1 nitpick below. 🧹 Nitpicks (1) — 🟢 1 low
This PR adds an Reviewed commit: 98031b3 |
|
Round 3 pushed as 98031b3 — all five open findings addressed, each with a reply on its thread. Summary of what changed:
Attribution: the implementation in 98031b3 is by a second agent session working on this account. I reviewed it line by line before landing it, and it arrived as a fast-forward on my HEAD, byte-identical to the patch I had already reviewed. Verification — and one gap now closed. That session ran
Tests were run per-path rather than as a full-suite run, deliberately: some fixtures in this repo create signed commits, and a full run repeatedly prompts for the maintainer's hardware key. Anything broader is left to CI. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98031b307e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| disabled={isRemoving} | ||
| onClick={() => void handleConfirm()} | ||
| > | ||
| {showRemovingSpinner ? <Loader2 className="size-3.5 animate-spin" /> : null} |
There was a problem hiding this comment.
When a remote removal exceeds 200 ms, this condition inserts an icon into an auto-sized button, increasing its width and shifting the dialog footer while the request is pending. The required STYLEGUIDE.md loading guidance explicitly says to pre-reserve space for delayed icons with a fixed width; reserve the spinner slot or fix the button width so the delayed feedback does not reflow the controls.
AGENTS.md reference: AGENTS.md:L1-L3
Useful? React with 👍 / 👎.
… manifest (OTA phase C follow-up) (stablyai#22376) * perf(mobile): keep four bundle chunk reads in flight across the whole manifest The fetch ran one worker per asset and paged inside an asset sequentially, so the largest script's 71 chunks were 71 serial round trips while the other readers idled. One window of four chunk reads now covers every (asset, offset) on the host's chunk grid, largest asset first. A read_limited refusal narrows the window and retries the read; eof is still read from the reply. Synthetic manifest (one 71-chunk asset, five small): 72 round trips -> 19. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the bundle-fetch family under pipelined reads Baseline moves to a1ee317, the pipelined fetch. 781 goldens change only their `baseline` header line. Six bodies move: mobile-web-bundle-fetch-paged, mobile-web-bundle-build-changed, and the four matrix-mobileweb.bundle-fetch-* goldens. The two bundle-fetch scenarios now bind requests in pipelined order, largest asset first, with every chunk sent before any reply: index.html@0 (#1), index.html@16 (#2), assets/app.js@0 (#3). - fetch-paged: the request set is identical, only reordered. The chunk sender names/ordinals and the scenarioSha256 moved; the replies and the fetched bytes did not. - build-changed: the same reorder, plus one request that is new because pipelining puts it in flight before the refusal lands (index.html@16). The refusal and the checkpoint are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): the bundle chunk comment no longer says a reply picks the next offset Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the bundle comment fix Baseline moves from a1ee317 to e94bde3, the comment-only commit on a fenced path. All 787 goldens and the scenarios file change only their `baseline` line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): pool four workers over planned bundle chunks, report progress per chunk Design-review fix round, sketch C: four workers take reads from one planned chunk queue, largest asset first. They replace the central pump and the read_limited narrowing. A host frees its read slot before it replies, so a lone fetch capped at four cannot trip the limit. A refusal now fails the fetch, as it did on base, and stops the other reads. - Each asset's buffer is allocated when the plan is built. That removes the nullable buffer and its guard. The per-asset byte count is gone, and the hash is the oracle (S1, S2). - The caller's signal is checked before each read and once after the pool drains, so an abort during the final window rejects with fetch-stopped (S3). The stopped check now covers only the caller's abort. The internal stop only makes late replies skip checks, hashing and progress (S4). - Progress is reported per accepted chunk. completedAssets still counts on completion (S6). - Renames: MAX_CONCURRENT_CHUNK_READS, and `reply` for the RPC reply (N1). - The slot check states exact-slot acceptance once, then classifies the refusal (N3). - Stale test titles and comments are renamed (N4). The synthetic manifest still takes 19 round trips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say why bundle reads settle at on-settle under pipelined chunks Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the bundle-fetch family under per-chunk progress Baseline moves to 34fda6f. 782 goldens and the scenarios file change only their `baseline` line. Five bodies move: mobile-web-bundle-fetch-paged and the four matrix-mobileweb.bundle-fetch-* goldens. The only change is bundle-progress effects. One report now lands after the first accepted chunk of index.html (0 assets, 16 bytes), and the later progress ordinals shift by one. Requests, replies and fetched bytes are identical. mobile-web-bundle-build-changed keeps its body, because its one accepted chunk is the whole of assets/app.js. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drain the fake host after the fetch settles so the sibling-stop bounds can fail The wave host stopped releasing replies once the fetch settled. Reads that should have been stopped were never answered, so the read_limited bound (7) and the chunk-failure bound (5) held even with no sibling stop at all. It now drains until nothing waits. With the worker's stopped.abort() removed, both bounds fail at 76 requests. assertChunkDescribesAsset's parameter is renamed to `reply`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the sibling-stop test fix Baseline moves from 34fda6f to 97b13ec. All 787 goldens and the scenarios file change only their `baseline` line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
…otiated mobileWeb.bundle.range (OTA phase C follow-up) (stablyai#22381) * feat(mobile-web): serve gzipped 384 KiB bundle ranges behind a capability Adds mobileWeb.bundle.range with its own strict params and result, so shipped chunk readers see no reply change. The host gzips each range at level 6 and sends identity when gzip does not shrink it, sharing the chunk method's read-slot budget and per-asset verification. status.get advertises mobileWeb.bundle.range.v1 beside mobileWeb.bundle.v1, and the method is allowlisted for paired phones. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): read the bundle range capability and range replies Adds the range reply reader and operation, and picks range or chunk from the status.get capabilities the connection already proved, so an older desktop keeps being paged in chunks with no probe round trip. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * perf(mobile): keep four bundle chunk reads in flight across the whole manifest The fetch ran one worker per asset and paged inside an asset sequentially, so the largest script's 71 chunks were 71 serial round trips while the other readers idled. One window of four chunk reads now covers every (asset, offset) on the host's chunk grid, largest asset first. A read_limited refusal narrows the window and retries the read; eof is still read from the reply. Synthetic manifest (one 71-chunk asset, five small): 72 round trips -> 19. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): add fflate 0.8.2 for gzip bundle ranges Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): decode gzip bundle ranges into a bounded buffer Inflates each range into a buffer one byte past its window, so a gzip bomb costs at most that allocation and an overlong body is visible. A corrupt, truncated or unknown-encoding body refuses as range-undecodable; a body of the wrong decoded length refuses as range-length-mismatch. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): pass the bundle read method from the session to the fetch The download reads the capabilities of the gates the reducer decided under and hands the fetch range or chunk. The fetch does not act on it yet; the range read lands on the pipelined window. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the bundle-fetch family under pipelined reads Baseline moves to a1ee317, the pipelined fetch. 781 goldens change only their `baseline` header line. Six bodies move: mobile-web-bundle-fetch-paged, mobile-web-bundle-build-changed, and the four matrix-mobileweb.bundle-fetch-* goldens. The two bundle-fetch scenarios now bind requests in pipelined order, largest asset first, with every chunk sent before any reply: index.html@0 (#1), index.html@16 (#2), assets/app.js@0 (#3). - fetch-paged: the request set is identical, only reordered. The chunk sender names/ordinals and the scenarioSha256 moved; the replies and the fetched bytes did not. - build-changed: the same reorder, plus one request that is new because pipelining puts it in flight before the refusal lands (index.html@16). The refusal and the checkpoint are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): the bundle chunk comment no longer says a reply picks the next offset Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): read gzipped bundle ranges on the pipelined window A host that advertised mobileWeb.bundle.range.v1 is paged in 384 KiB ranges on the range grid, through the same four-read window and queue as chunks; any other host keeps the chunk grid. Each range is decoded to its exact window length before the fill checks and the asset hash. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the bundle comment fix Baseline moves from a1ee317 to e94bde3, the comment-only commit on a fenced path. All 787 goldens and the scenarios file change only their `baseline` line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the corpus at the range-read pin Repins baseline to the range-read commit and re-records every golden. Only the baseline and lockfileSha256 headers move: the lockfile gained fflate, and the bundle-fetch adapter pages the chunk path, whose requests and replies are unchanged, so no golden body moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state the on-settle reason that holds for pipelined bundle reads Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the on-settle comment fix Baseline moves from 252c592 to fe41226, the comment-only commit on a fenced path. Re-recorded: all 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): keep the lockfile's patch block in main's form Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the lockfile patch-block restore Baseline moves from fe41226 to 84d6fca, which restores main's patchedDependencies form in mobile/pnpm-lock.yaml. Re-recorded: all 787 goldens change only baseline and lockfileSha256, and the scenarios file only baseline. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): pool four workers over planned bundle chunks, report progress per chunk Design-review fix round, sketch C: four workers take reads from one planned chunk queue, largest asset first. They replace the central pump and the read_limited narrowing. A host frees its read slot before it replies, so a lone fetch capped at four cannot trip the limit. A refusal now fails the fetch, as it did on base, and stops the other reads. - Each asset's buffer is allocated when the plan is built. That removes the nullable buffer and its guard. The per-asset byte count is gone, and the hash is the oracle (S1, S2). - The caller's signal is checked before each read and once after the pool drains, so an abort during the final window rejects with fetch-stopped (S3). The stopped check now covers only the caller's abort. The internal stop only makes late replies skip checks, hashing and progress (S4). - Progress is reported per accepted chunk. completedAssets still counts on completion (S6). - Renames: MAX_CONCURRENT_CHUNK_READS, and `reply` for the RPC reply (N1). - The slot check states exact-slot acceptance once, then classifies the refusal (N3). - Stale test titles and comments are renamed (N4). The synthetic manifest still takes 19 round trips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say why bundle reads settle at on-settle under pipelined chunks Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web): announce bundle ranges on the manifest reply The manifest reply now names the range grid in an optional rangeBytes, beside chunkBytes, and the status capability is gone. The range method takes exactly the chunk params on that grid instead of a caller length. Both methods share one verified read that returns the six-field header, and the range handler checks the connection again before deflating. Range schemas move into the bundle RPC contract; SHA256_PATTERN is shared from the manifest contract. Shared refusals are tested once over both methods. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the range capability read and its session threading The phone will read rangeBytes off the loose manifest reply instead, so the read-method module goes and the session effects and hook return to the pipeline branch's version. Range imports move to the bundle RPC contract, the reply reader reuses the shared SHA256_PATTERN, and a new test pins that node's level-6 gzip from the host encoder inflates with fflate to the same bytes. The fetch and window-read modules still import the deleted names until part 2. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the bundle-fetch family under per-chunk progress Baseline moves to 34fda6f. 782 goldens and the scenarios file change only their `baseline` line. Five bodies move: mobile-web-bundle-fetch-paged and the four matrix-mobileweb.bundle-fetch-* goldens. The only change is bundle-progress effects. One report now lands after the first accepted chunk of index.html (0 assets, 16 bytes), and the later progress ordinals shift by one. Requests, replies and fetched bytes are identical. mobile-web-bundle-build-changed keeps its body, because its one accepted chunk is the whole of assets/app.js. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): page bundle ranges through one window reader chosen by the manifest The fetch keeps the pipeline's four-worker pool and builds one window reader from the manifest reply: ranges on rangeBytes when the host names it, chunks on chunkBytes otherwise. The reader returns the six-field header and a lazy bytes() so the stop and misroute checks run before any decode. A range that inflates to the wrong length now falls to the slot checks, with the one-byte-over buffer as the memory bound, so range-length-mismatch is gone. A rangeBytes this build cannot page reads as absent. Fetch names say window, not chunk. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drain the fake host after the fetch settles so the sibling-stop bounds can fail The wave host stopped releasing replies once the fetch settled. Reads that should have been stopped were never answered, so the read_limited bound (7) and the chunk-failure bound (5) held even with no sibling stop at all. It now drains until nothing waits. With the worker's stopped.abort() removed, both bounds fail at 76 requests. assertChunkDescribesAsset's parameter is renamed to `reply`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the window-reader commit Baseline moves to a25a355. Re-recorded: every golden and the scenarios file move only baseline, and the five bundle-fetch goldens also move lockfileSha256 to this branch's lockfile. Every golden body is byte-identical to the pipeline branch's. The recording adapter's scripted host names no rangeBytes, so the bundle family still records the chunk path. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus after the sibling-stop test fix Baseline moves from 34fda6f to 97b13ec. All 787 goldens and the scenarios file change only their `baseline` line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the second pipeline merge Baseline moves from a25a355 to 4d2cab3, the merge of the pipeline's sibling-stop test fix. Re-recorded: every golden and the scenarios file move only baseline. Against the pipeline branch, only baseline and lockfileSha256 differ; every golden body is identical. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the bomb inflation without depending on call order With the fetch's sibling stop removed, a read left over from the previous test inflated into the bomb test's record first, and indexOf(601) picked it. The test now asserts some inflation stopped at 601 and none exceeded its buffer, whatever else ran. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the bomb-test fix Baseline moves from 4d2cab3 to 73fde15, the test-only commit on a fenced path. All 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web): tighten the bundle window contract and pin the range sibling stop The range bomb test reads only its own host's inflations, keyed by the gzip bodies that host sent, and plans twenty reads so a missing sibling stop is visible: with stopped.abort() removed it sends all twenty. The range params are an alias of the chunk params, and the chunk data bound is the exact base64 length of a full chunk. The phone's chunk and range replies share one header shape. The window reader closes over the client and bytes() takes the slot length the fetch computes. The host's positional read is readMobileWebBundleAssetWindow. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the window-contract commit Baseline moves from 73fde15 to 7346e00. All 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the main merge Baseline moves from 7346e00 to 9c0fe1a, the merge of main at 98a6a53. Recorded with --record: all 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web): drop a test cast and shape-named field maps The bomb test's inflation log is typed by its hoisted factory's return instead of an assertion, and the zod field maps shared by the bundle window schemas are windowParamsFields and windowHeaderFields. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the lint fix Baseline moves from 9c0fe1a to f4f0915. Recorded with --record: all 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): page the range fetch fixture on a small advertised grid The fake host names a 4 KiB range grid and a 1 KiB chunk grid on its manifest reply, which the phone pages as it would the real ones, so the fixtures shrink to a few KiB with the same shapes and each asset is hashed once. The file runs in about 360 ms instead of 3.5 s, which a loaded CI runner pushed past the 5 s test timeout. The desktop range suite still pins that the real host names MOBILE_WEB_BUNDLE_RANGE_BYTES. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the recording corpus at the range fixture fix Baseline moves from f4f0915 to d4d6aad. Recorded with --record: all 787 goldens and the scenarios file change only their baseline line. No golden body moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
Navigation note for anyone arriving here later: this PR shows as merged into The change is not lost. It lives in two places:
It has not reached |
ELI5
If the machine a project lives on is switched off, you could not remove that project from Orca. You got "Failed to remove project" and the row stayed, forever. Now Orca tells you the machine did not answer and offers to drop it from this computer's list — while being honest that the machine itself is untouched.
What Changed
Before. Right-click a project owned by an Orca server host that is offline → Remove Project → Remove. After ~15s: an error toast reading
Failed to remove project, and the row is still in the sidebar. Retrying never works. The only escape is deleting the entire environment, which takes every other project on it with it.After. The dialog stays open instead of closing on a removal that did not happen, and rewrites itself:
with the button relabelled Remove from Orca. Clicking it clears the row.
Mechanism.
removeProjectdispatches the removal to the host that owns the project, then purges local state once it resolves. Any rejection other thanrepo_not_foundwas rethrown, aborting before the purge — so an unreachable host produced no state change and one generic toast.That outcome is now split in two, by whether the host actually answered:
failed, still toasts, row kept. Unchanged.owner-unverifiableoutcome. What the host did is unknown: it may never have seen the request, or it may have removed the project and lost the reply past the timeout. Neither may be asserted, so the row is kept and the store returns instead of spending a dead-end toast. Retrying once the host answers is safe — a removal that did land comes back asrepo_not_found, whichremoveProjectalready reconciles.Classification reuses the existing
isRecoverableRemoteRuntimeConnectionErrorcorpus rather than a new code list.removeProjectnow returnsRemoveProjectOutcomeinstead ofPromise<void>, and takesmode: 'forget-local', which skips every dispatch to the owner — therepo.rmcall and theterminal.stopfan-out — and does the host-scoped local purge only. It is refused outright for an ephemeral-VM-backed project, whose removal exists to destroy that VM: forgetting it locally would either leak the VM or break the mode's own promise.Only
runtime:*-owned projects could ever hit this: local and SSH owners route through the localrepos:removeIPC, which needs no remote answer.The Settings → Projects pane has no per-host forget affordance, so it voices the unanswered host in its own toast rather than looking successful.
Why
The alternative was to purge locally whenever the host call fails. That is wrong twice over: it claims a removal that did not happen, and it would silently drop a project on a transient blip.
docs/reference/ssh-execution-boundary.mdis explicit — loss of contact is never evidence of absence, and "when the host cannot acknowledge the request… may clear only client state". So the client-only forget is offered, never taken automatically, and its copy states both limits: nothing on the host changes, and the project comes back if that host reconnects.Deciding the route up front from cached host status was also considered and rejected:
runtimeStatusByEnvironmentIdis empty before the first probe, so a healthy host would be forgotten locally instead of properly removed. Failing the real call is the only evidence that does not go stale.Linked Issue
N/A — issues are disabled on this repository. Reported directly: removing a project on a paired Orca server that had been offline for days failed with
Failed to remove projecton every attempt, while a project on a (also disconnected) SSH host removed fine.Visual Proof
Not attached. Reproducing the before/after needs a paired Orca server host that is offline, which I could not stand up in a hidden-renderer harness. The reviewer-visible states are pinned by tests instead:
RemoveFolderDialog.test.tsxasserts the dialog stays open, renders the host name and the "returns if that host reconnects" sentence, and then sendsmode: 'forget-local'. Happy to add a recording if someone has a spare host to take down.Testing
Reviewer steps:
Automated:
repos-remove-missing-remote-project.test.ts— the refusal path still fails and toasts; a host that never answers reportsowner-unverifiable, keeps the row and does not toast;forget-localclears the row and makes no call to the host at all.RemoveFolderDialog.test.tsx— happy path closes; the unverifiable path stays open, shows the host-named copy, and re-runs asforget-local.settings-project-list.test.ts— the pane names the unanswered host (by display name, never the rawruntime:id) instead of looking successful.repos-ephemeral-vm-cleanup-retention.test.ts—forget-localon a runtime-owned SSH target is refused with the reason surfaced, and never reachesephemeralVm.cleanup.RemoveFolderDialog.test.tsx— a removal completing after its dialog was dismissed does not close the dialog that replaced it (verified failing without the guard).Ran locally on macOS:
pnpm tc(all projects),pnpm test src/renderer(3925 files / 33189 tests passed),pnpm run check:code-quality:changed.AI Disclosure
Claude Opus 5 (1M context), via Claude Code.
Review
Agent skill upstream boundary
Notes
ssh-host-remove-workspaces.tsnow reads the returned outcome but keeps its existing store-state verification, since a returned status is not proof of the durable change.forget-localis the absence of a call.removeProject's return type widened fromPromise<void>; all six call sites updated.removeSettingsProjectFromAllHoststakes a requiredresolveHostLabel; its one caller passesselectExecutionHostDisplayLabel.forget-localstrictly removes calls.Checklist
pnpm typecheck,pnpm test(renderer), and the changed-code quality gate pass locally;pnpm buildleft to CI🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.