Repository navigation
feat(machine): Code tab — branch-scoped file browser + read-only Monaco - #2004
Conversation
Fills in the placeholder CodeTab from the 4-tab Machine shell (#1999), composing the pieces earlier phases landed: - inner page-scoped sidebar (plain border-border chrome, mirroring TerminalTab's sidebar/main-pane split): a BARE MachineTree as the branch picker, then MachineFileTree over the picked branch's checkout - main pane: CodeFilePane reads the selected file via the files route (mode=read) and shows it in a read-only Monaco, language detected from the filename Nothing fetches until it has to: no file content is read until a file is actually clicked, and no checkout is touched until a branch is picked. Not-yet-cloned branches are an explicit empty state, not a raw fetch error. `machine_branches` tracks no clone state (its row carries only a `sandboxId`), so the checkout's existence is answered by the live filesystem: the files route already returns a typed `reason` — `not_found` (no row, or the repo dir isn't there) or `vanished` (row exists, Sprite is gone). BranchFiles probes the checkout root once per branch and turns both into distinct empty states with a re-check action, mounting the file tree only once the probe says ready. Tests: 13 colocated (CodeTab composition + probe states; CodeFilePane fetch/loading/error/truncated/refetch). apps/web typecheck + lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A real checkout is full of binaries (images, fonts, archives), and the files route decodes whatever it reads as UTF-8 — so clicking `logo.png` in the tree would have streamed up to 2 MiB off the Sprite only to render replacement characters in Monaco. CodeFilePane now recognises binaries from the filename (`isBinaryFile`, already exported alongside `detectLanguageFromFilename`) and shows an explicit no-preview state WITHOUT fetching: there is nothing useful to display, so the read is pure waste. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
…s, leaked route phrasing
Self-review with an adversarial reviewer on the diff turned up five real
defects, one of them a blocker I had introduced.
BLOCKER — CodeFilePane.test.tsx had two literal control bytes (NUL, 0x01)
where escapes were intended, so git classified the file as BINARY
("Bin 5605 -> 7093 bytes"). The test I had just added was undiffable and
unreviewable on GitHub. Bytes stripped; all five files verified clean.
The payload was dead anyway — that test asserts the read never happens.
Wasted root listings on every branch change. BranchFiles held its state on
the component instance while CodeTab re-rendered it with the new branch, so
for one frame it still said `ready` and mounted MachineFileTree against the
NEW branch — which self-keys, mounts, and fires a whole root listing that
is then thrown away when the probe effect resets to `loading`. A branch
switch cost 3 listings, not 2. Both children are now keyed by identity
(branch, and branch+path), making a switch a remount, not a re-render.
Regression-tested: probes are now exactly ['main', 'dev'].
Leaked internal phrasing. If a branch's Sprite was reclaimed while a file
was open, the main pane rendered the route's own words — "Branch machine
vanished" — in red, beside a Retry that could never succeed, while the
sidebar correctly said "This branch checkout is gone." Both surfaces now
share one vocabulary (checkout-states.ts): a checkout that isn't there is a
state of the world, not an error string. A read's 404 is deliberately NOT
folded in — the route overloads `not_found` for both "no checkout" and "no
such file" — so it reads as "This file is no longer in the checkout."
Binary detection was filename-only, and BINARY_EXTENSIONS was written for
GitHub import: no .pyc, .node, .class, .o, no extensionless a.out, and
'libfoo.so.1'.split('.').pop() is '1'. Added git's own heuristic as a
second guard — a NUL byte in the head of the content means binary — so an
unlisted binary is caught by content instead of mojibaked into Monaco.
Selection is now one atomic state object (branch + path move together)
instead of two setStates racing a stale closure.
Tests: 55 terminal-area tests pass (20 in this tab, +5). typecheck + lint green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
|
Coordination note re: the inert Machine/Project labels (sibling PR #2003). I flagged in the description that passing Checking the in-flight siblings, #2003 (Diff tab) hit the identical problem and has already solved it properly, adding an optional That's the right fix and I don't want to duplicate or race it. So:
Everything else in the tab is self-contained: the only shared code it consumes is read-only ( |
The "checkout absent" and "checkout error" branches of BranchFiles rendered byte-identical chrome and a byte-identical ghost retry button, differing only in their words. Extracted SidebarNotice — they differ in what they say, not in what they are — and gave the error row a testid to match. No behavior change: 55 terminal-area tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
…t for "gone" A second adversarial review pass found that two of the fixes I shipped last round were guarded by tests that could not fail. Both are now mutation-proven. TESTS THAT DIDN'T TEST ANYTHING - The "no wasted listing" test passed with CodeTab's key DELETED. The stubbed MachineFileTree issued no listing, so the test only counted BranchFiles' probes — while the wasted listing came from the real tree. The stub now models the one behaviour that matters (it self-keys, so a new branch = a new root listing). Mutation-checked: delete the key and it fails with listed=['main','dev','dev'] — the thrown-away listing, made visible. - Same trap in the pane: a stale-read test that passed with all four `cancelled` guards removed (RTL flushed the effect before the assertion). It now flushes inside act() and fails without them. - And the one-frame "previous file under the new file's name" render is invisible in the final DOM by construction — React flushes the effect before paint — so it is now caught by recording what Monaco was actually handed: the pair (python, contents-of-a.ts) must never occur. ONE CONTRACT FOR "GONE" The internal phrasing I claimed to have killed was still reachable one click later: with a checkout open, reclaim the Sprite and expand a folder, and MachineFileTree rendered the route's `error` verbatim — "Branch machine vanished". Fixed at the source instead of at each client: the route's `error` is now a sentence for a person, and `reason` stays the machine-readable fact that clients switch on. Same for a failed exec with empty stderr, which used to print the bare token "exec_failed" at the user. The route also overloaded `not_found` for two different facts — "this branch has no checkout" and "that one file is gone" — so removing a branch-terminal under an open file told the reader the FILE had vanished. They are now distinct reasons (`file_not_found`), and the pane says which actually happened. ALSO - The U+FFFD binary heuristic had no absolute floor, so "Café Résumé" saved as Latin-1 (3 bad bytes in 11 chars = 27%) was declared binary and hidden. Now needs a real count, not just a ratio. - Dropped the per-file key on CodeFilePane: it tore down and recreated Monaco on every single file click. The path now travels in the state instead, which is what the render guard checks. - The pane has a reload: the working tree is live, an agent can rewrite the open file, and re-clicking the same tree row is a no-op. apps/web: typecheck green, lint 0 errors, 77 tests pass (68 in this feature). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
|
Adjacent instances of the same bug, deliberately left out of this PR. While fixing the files route so it never renders internal phrasing at a user (
Neither is reachable from the Code tab, and both serve the Diff tab (#2003, in flight). I've left them alone rather than pull another surface into this diff — but they will print "Branch machine vanished" at a user the moment a Sprite is reclaimed with the Diff tab open, exactly as the files route did. The convention this PR establishes, if someone wants to apply it there (one line each): // error: a sentence fit to show a person. reason: the fact clients switch on.
{ error: 'This branch checkout is unavailable', reason: resolved.reason }Worth noting |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Round 3 found that my "never render internal phrasing at a user" fix had
landed on two of the route's three failure arms. The LIST arm still did
`error: result.detail` — the raw stderr of our own `ls`.
Expand a folder an agent terminal deleted a second earlier and the file tree
printed, verbatim:
ls: cannot access '/workspace/repo/src/generated': No such file or directory
Internal absolute path and all. And the same arm still conflated "this branch
has no checkout" with "that one folder is gone" under a single `not_found` —
the exact conflation I split the READ arm to prevent, left standing next door.
So the contract now says what it means, for all three arms:
not_found — this branch has no checkout (the ROOT listing is missing)
dir_not_found — the checkout is fine; that directory is gone
file_not_found — the checkout is fine; that file is gone
vanished — the Sprite was reclaimed
exec_failed — the exec died; its stderr is in `detail`, NOT in `error`
`error` is a sentence for a person. `detail` is for logs and developers. Our
stderr names paths inside the Sprite, so it belongs in the second, never the
first. Both new guards are mutation-proven: put stderr back in `error`, or
collapse `dir_not_found` into `not_found`, and a test goes red.
Also from round 3:
- Removed a dead `key` on CodeFilePane. It could never fire — a branch switch
clears `path` in the same update, so the pane unmounts regardless — and its
comment described that unreachable case as if it were load-bearing. (The key
on BranchFiles IS load-bearing; its mutant still dies.)
- Corrected a comment that justified the probe's duplicate root listing by
claiming MachineFileTree is also mounted by TerminalTab and the Diff tab. It
isn't: grep says this tab is its only mount. The comment now states the real
cost, and names the refactor that would remove it — deliberately not done in
the same PR that changes this route's contract.
- Folded MachineFileTree's private error-body reader onto the shared helper.
apps/web: typecheck green, lint 0 errors, 121 tests pass, production build OK.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ
Phase 3 of the Machine page rebuild (Terminal — GA Release epic). Spec: task page
ytxrq4mz1y7mrkjm0ls8w67k.Fills in the
CodeTabplaceholder left by the 4-tab Machine shell (#1999). Export name and{ machineId }signature unchanged —TerminalViewstill mounts it as<CodeTab machineId={pageId} />, andComingSoonTabstays for the Diff/Settings placeholders.What it does
Sidebar (plain
border-borderchrome — deliberately not liquid-glass; mirrorsTerminalTab's split, row states andw-64width): a bareMachineTreeas the branch picker, thenMachineFileTreeover the chosen branch's checkout.Main pane:
CodeFilePanereads the selected file (mode=read) into a read-onlyMonacoEditor, language fromdetectLanguageFromFilename.truncated(the route's 2 MiB cap) shows as a banner so a partial view is never mistaken for the whole file. The working tree is live — an agent terminal can rewrite the open file, and re-clicking the same tree row is a no-op — so the header carries a reload.Lazy: nothing is probed until a branch is picked; nothing is read until a file is clicked. Selection (branch + path) is one atomic value, so a branch switch drops the open file in the same update.
"It isn't there" is a state, not an error message
A branch's checkout exists only once cloned, and
machine_branchestracks no clone state (its row carries only asandboxId) — so the live filesystem answers, via the route's typedreason.Getting that honest end-to-end meant fixing the contract of
api/machines/files, because the problems were only reachable from a client and could only be fixed at the source:errorwas an internal token.`Branch machine ${reason}`— andMachineFileTreerenderserrorstraight into a row. Reclaim a Sprite, expand a folder, and the user read "Branch machine vanished". On the list arm it was worse:error: result.detailpiped our ownlsstderr through, so deleting a folder printedls: cannot access '/workspace/repo/src/generated': No such file or directory— internal absolute path and all.reasonmeant three different facts.not_foundcovered "this branch has no checkout", "that directory is gone", and "that file is gone" — so removing a branch-terminal under an open file told the reader the file had vanished.The contract now says what it means:
not_foundvanisheddir_not_foundfile_not_foundexec_faileddetailerroris a sentence for a person.detailis for logs and developers — our stderr names paths inside the Sprite, so it belongs there and never inerror. Both surfaces of the tab share one vocabulary (checkout-states.ts), and a real failure (403,exec_failed) is never dressed up as an empty state.Binary files
The route decodes whatever it reads as UTF-8, so a binary lands in Monaco as mojibake. Two guards: by extension before fetching (avoids streaming 2 MiB for something unviewable), and by content after — because
BINARY_EXTENSIONSwas written for GitHub import and misses.node,.pyc,.class,a.out,libfoo.so.1. The content check is git's own heuristic (NUL in the head) plus a U+FFFD flood — with an absolute floor, so a short Latin-1 file likeCafé Résumé(3 bad bytes in 11 chars = 27%) isn't wrongly hidden.On the tests
Three adversarial review passes ran against this diff, and two of them caught tests of mine that could not fail. Every correctness guard here is now mutation-proven — I delete the guard and confirm a test goes red:
listed: ['main','dev','dev'];cancelledguards removed (RTL flushed the effect before the assertion). It now flushes insideact();(python, contents-of-a.ts)must never occur;error, or collapsedir_not_foundintonot_found, and a test goes red.A dead
keyonCodeFilePanewas removed (a branch switch clearspathin the same update, so the pane unmounts regardless — the key could never fire).121 tests across tab, pane and route.
bun run typecheckgreen ·bun run lint0 errors · production build green.Notes for review
MachineTree'sTreeRowroutes a label click to select instead of expand, so Machine/Project labels are inert here (chevrons still expand). Sibling feat(machine): Diff tab — bare MachineTree + 3-way scope toggle + per-file Monaco diffs #2003 already addsisNodeSelectablefor exactly this; I did not touchMachineTree— zero file overlap with feat(machine): Settings tab — name/description, access toggles, delete #2002/feat(machine): Diff tab — bare MachineTree + 3-way scope toggle + per-file Monaco diffs #2003, so all three merge in any order. Whichever of feat(machine): Diff tab — bare MachineTree + 3-way scope toggle + per-file Monaco diffs #2003/feat(machine): Code tab — branch-scoped file browser + read-only Monaco #2004 lands second wires it up (one line, in a comment below).MachineFileTreeis mounted only here, so it could instead report root-level absence back via anonRootUnavailable(reason)prop and the probe would disappear. That's the better end state and a small refactor — deliberately not done in the same PR that changes this route's contract.diff/route.tsandgit-blob/route.tsstill carry the identicalBranch machine ${reason}leak. Not reachable from this tab (they serve the Diff tab), so they're flagged in a comment rather than pulled into this diff.🤖 Generated with Claude Code
https://claude.ai/code/session_01K3tLCaRft4iC2DH3hmk7PJ