Repository navigation
Opening a file brings its window over; signing out asks first - #92
Merged
Merged
Conversation
Reported as "when i look at changes of a file, the changes are not scrollable", and it was exactly that: nothing in the chain could scroll. Measured at a 898px pane with a 300-line file rewritten -- `.diff` drawn 10,563px tall, `.files__file` and `.files` at `overflow: visible`, and `.tile__pane--files` at `hidden`. So 600 changed lines were rendered, 22 were readable, and the rest was clipped away with no way to reach it. Both Changes and Commits, at every width. The box that used to scroll a patch is `.git__diff`, from when the git panel was a panel of its own. When the two panels became one the diff moved into `.files__file` -- the slot the editor and a rendered page also use -- and that rule was left behind in the stylesheet, rendered by nothing. It looked like the diff was fine because its neighbours are: CodeMirror brings `.cm-scroller` and a rendered page is `.md`, which is `overflow-y: auto`. The patch was the one thing in that column with no scroller of its own. So the rule is renamed `.files__diff` and rendered around the patch, keeping the pair `flex: 1; min-height: 0` -- the same pair the editor's host and the image box need, and for the same reason: without it a flex item is floored at its content's height, so the pane grows instead of the child scrolling. The scroller has to be the wrapper rather than `.diff` itself, which carries `min-width: max-content` so long lines scroll rather than wrap: a box as wide as its content cannot clip it, and so cannot scroll it either. Measured after, against a scratch instance: vertically 898 over 10,569, a real wheel walking 0 -> 600 -> 3,600 and back to 0; horizontally 678 over 3,334 with a 425-character line, scrolled to 400; the end of the patch reachable in Changes and in Commits (scrollTop == scrollHeight - clientHeight at 4,427); and `.grid` unmoved at 358 through all of it, which is the rule that a downward wheel never moves the row. At 390px, 722 over 5,325 with the row still where it was. A rendered `.md` was checked in the same slot and was already a scroller, so this is the one place it was missing. No test: this is layout, and jsdom has none. A test asserting the wrapper's class would pass with the stylesheet deleted, which is the kind this repository does not keep. The measurements are in the CSS comment and in `web/CLAUDE.md` beside the sibling trap they belong with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reported as "if a file is too large to be displayed, i can not use cmd right
to go to the worktree on the right", and it is a wall rather than a dead key:
Cmd+Right *into* that window worked, and then did nothing however often it
was pressed, while Cmd+Left still walked away.
`stepRow` starts from where the keyboard is, read off the DOM, and that is
deliberate -- React's record is a press behind when you walk quickly, and the
row scrolls smoothly, so `active` and `wholeOnScreen` were both measured
wrong once and the DOM is what fixed them. The cost, never noticed until
now: if a step lands somewhere that takes no keyboard, the keyboard stays in
the pane you left, and the next press computes the identical step. Nothing
moves, for ever.
`FilesPane` said as much in a comment -- that a target which refuses leaves
focus where it was, and the stepper listening on the document made that
survivable. That was the assumption, and it was wrong.
Three panes had to answer, each with the most useful thing it has:
- A refused file asked the *editor* for the keyboard, because arrival is
decided from the path before the read comes back -- which is what stops the
search box taking it and the editor snatching it back mid-word -- and
"refused" is only knowable afterwards. No editor ever mounts, so the
request sat outstanding. The panel now answers it once the refusal is in:
the file's row in the tree, or the content box where no tree is drawn. That
one may be decided from the fetch, unlike arrival, because a refusal means
nothing is coming that could race it.
- A view-only file with no tree beside it -- a phone -- landed on nothing at
all, written down as deliberate. It was half a decision: the argument is
that a picture must not take the keys off the tree, and there is no tree.
`.files__file` is `tabIndex={-1}` and takes it, as `.md` already did.
- The project pane on a phone refuses the caret, and should -- a caret there
is the keyboard over half a pane you asked only to look at. But refusing a
caret is not refusing the keyboard: `caret` says which, and the pane focuses
its own box, which summons nothing.
None draws a ring: the pane already underlines where you are.
Measured against a scratch instance. Before, at 1800px, a window showing a
3320 KB file: Alt+Right in, then 1073 -> 1073 -> 1073 with the keyboard in
that worktree's Claude throughout. At 390px, a picture: the row went 1170 ->
1560 with the keyboard left behind in the window it had quit, then nothing.
After: the whole row walks end to end at both widths -- at 390px through
project one, the panel showing a picture (the content box), fourth,
two-terms, project two, alpha and the machine, 0 -> 2340 in seven presses;
at 1800px the same, with the panel landing on the file's row and the project
panes landing in their branch box, caret intact. A text file still hands the
editor the keyboard on arrival (`DIV.cm-content`), a click still leaves the
keyboard on the row it clicked, and stepping on still leaves.
No test: this is focus and layout in a browser, which jsdom does not have,
and the rule it turns on -- that the walk reads the DOM -- is the part a unit
test would have to fake. The measurements are in the code beside each of the
three, and in `web/CLAUDE.md` in place of the claim they disprove.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The files panel drew a .png and said "This is not a text file." to everything else, including three kinds of file the browser renders perfectly well. `shared/src/media.ts` had already written down why, in one sentence: video and audio "would need `Range` to be honest -- a 200 with the whole file plays but cannot seek -- and that is a request handler of its own rather than a row in this table". This is that handler, and then the table grows. `server/src/range.ts` is it, pure and testable with no route: what a Range header asks for of a file of N bytes. Every arm fails *towards* sending the whole file, which is always legal, and the sole 416 is a range beginning past the end -- a 206 of nothing wedges a player on a seek past the end for ever. Two edges carry the weight. `bytes=-500` is a suffix and not a negative start, and it is the *first* request a non-faststart MP4 provokes, since the moov atom is at the end: get it wrong and there is no picture at all rather than a slow one. And Node's `end` for createReadStream is inclusive like the header's, so the pair passes straight through and the only `+1` in the change is content-length. A zero-byte file is sent whole rather than 416, against the letter of the RFC: an empty file is a legitimate file and a 416 reads to the browser as a broken source. The response moved out of the route into `server/src/raw.ts`, which is most of what /raw is -- the headers there are the whole posture of serving arbitrary bytes out of somebody's working tree -- and which a test can drive with a temp file and no workspace. It also opens the file once and takes the length from that handle: `streamable()` stats and the route then opens, so a file rewritten in between made us promise a content-length the bytes do not keep. Invisible at the size of an icon, a real window at the size of a video. A PDF is an iframe, because the page's own policy says `object-src 'none'`. Two headers forbade that: the global `x-frame-options: DENY`, which refuses *same-origin* framing too, and the route's own sandbox. So `headers.ts` now leaves a route's own X-Frame-Options alone exactly as it already left a route's own CSP alone, and /raw sends SAMEORIGIN for that one type. **The sandbox is not widened**, and that was measured rather than assumed: the obvious guess is that Chrome's viewer needs `allow-scripts`, but the viewer is a chrome-extension frame *inside* the sandboxed document rather than script belonging to it, and a plain sandbox renders the page in full -- toolbar, thumbnail and text -- checked side by side on a stand-in server serving one PDF two ways. The whole exception is `frame-ancestors 'self'`. The peer proxy streams /raw now instead of buffering it under a 32MB cap, which is the half of this that was already broken and nothing to do with video: a file over that on a linked machine could not even be *downloaded*, answering "that server sent too much" for exactly the files worth fetching. Range goes upstream and the peer's 206 comes back; the timeout covers the head and not the body, since a 400MB file legitimately outlives any read budget and nothing accumulates; and the upstream fetch is aborted when the browser goes away, which is not an edge case -- every seek cancels the request in flight. The peer's own security headers travel with the bytes, fixing a quieter bug: a proxied raw reply set no policy, so `headers.ts` gave it the *page* CSP, and remote file bytes were served looser than local ones for as long as linking has existed. In the panel, MediaView switches on `mediaKindOf` -- derived from the type so there is no second table -- and three of its rules are bugs if left out. A playing file is not restarted when the agent touches it: the rev changes on every rewrite and swapping src jumps to zero, so a player keeps its URL until it is paused at the start or has ended, and the caption says the file has changed meanwhile. The path is held with the URL, or clicking another video while one plays leaves the first on screen wearing the second's name -- measured, and fixed. One player at a time across the row, because two windows both playing is two soundtracks. And errors are read off the element, `MEDIA_ERR_SRC_NOT_SUPPORTED` apart from a decode failure, which is what lets .mov into the table at all: QuickTime holding H.264 plays and holding ProRes says so plainly, where a broken <img> gives the reader a glyph and nothing to catch. Measured against a scratch instance with real ffmpeg fixtures. A 126.5MB video: opens, seeks to 10s/4s/11.5s/1s instantly, 77 partial-content answers in the log, and the server's RSS flat at 161MB after 1.1GB streamed. A non-faststart MP4 plays, which is the suffix path. webm, mov, mp3 play; the PDF renders with Chrome's viewer inside the frame; captions read "320 × 240 · 0:05 · 22 KB" and "0:02 · 16 KB". Rewriting a file mid-playback held it at 16.9s on the old rev and adopted the new one on pause-and-rewind. Two mounted players: playing the second paused the first. Downloads byte-identical through the glyph. Cmd+arrow still walks into and out of a window showing a video. At 390px the video is 320x240 in a 387px pane, its controls hit-testable, no sideways scroll. And on a linked machine, which is the case this was asked about: the same 126.5MB file is byte-identical through the gateway where it used to be a 502, a range comes back 206 with the right slice, the peer's PDF keeps the peer's own policy, and the download carries its full length. PROTOCOL_VERSION is deliberately not bumped. A mismatch makes a linked machine unusable until it is updated, and the skew here is graceful: an older peer answers `binary` with no media, which is today's "not a text file", and an older peer ignoring Range returns the whole body, so a remote video plays and merely cannot seek. Breaking every peer for that is the wrong trade. Every new test was watched failing with its line reverted: the suffix clamp, the content-length +1, a kind the table knows and mediaKindOf does not, the X-Frame-Options exception, and the peer forgetting to forward content-range. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Master's own README, `cloud/provision.sh` and `cli/test/cloud.test.js` are taken whole. This branch had no business changing any of the three: an earlier commit here picked them up from a stale worktree after a reset, quietly reverting the "provision straight from the internet" work in e2c7275. Resolved by taking master's side outright, which is the only correct answer -- checked afterwards that the branch deletes nothing of master's at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The message was *"https://ide.vonrechenberg.ch:81 speaks protocol 1, this one speaks 2"*, which is true and useless: nobody chose those numbers, and what the reader has to know is that a linked machine is a checkout somebody must go and update. So the sentence ends with the command -- `Run pnpm pull in the Switchboard directory on that machine.` It names **which** machine, and that is not padding. A peer is usually the one behind, being the machine you deploy to less often, but the reverse happens the moment you link a machine you updated first -- and a fixed sentence would then send you to the newer one to make it newer still. A version that is not a number at all is read as "older", which is the likelier accident and the harmless guess, since `pnpm pull` on a current checkout does nothing. One helper for both places that threw it -- the per-reply header check and the identity check at link time -- which were two copies of the same sentence and would have drifted the first time either was touched. The test pins the whole sentence rather than a fragment, because the wording is the point of this change, and it covers both directions. Watched failing with the instruction removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was no way to get a file *into* a worktree from the browser. You could read one, edit one, and since last week download one; putting one in meant a terminal and a path, which is the one thing the files panel could not do at all. `POST /api/worktrees/:id/upload` takes the bytes as the body and the destination in the query, and that shape is the point: a `Blob` body is streamed by the browser and the body is streamed to disk here, so a 200MB recording is never assembled in the tab or in this process. `multipart/ form-data` is the other way to do it and would want a parser dependency to take apart an envelope nothing else here has a use for. The content type parser is what keeps the body a stream -- Fastify buffers what it can parse and refuses what it cannot, so the raw request is handed through untouched. Three decisions in `uploadFile` are the whole of it: **Containment is on the directory, and the name does the rest.** `containedPath` requires a path to exist and an upload's target does not yet, so the directory is checked and the name is then held to one ordinary segment. Without that second half, `../../x` is joined onto a directory that has just passed the check -- which is a test, and it was watched failing. **A temp file beside the target, renamed on.** The opposite of what `writeTextFile` does one door up, and its comment says why the reasoning does not carry: a save is to a file under git still held in an editor's buffer, where a rename changes the inode and drops the mode. This is a file that does not exist yet, arriving over a network that can stop halfway, and a half-written one under its final name is the thing to avoid. Measured: a body that dies mid-stream leaves neither name behind. **It refuses to overwrite.** A name already taken is far more often a mistake than an intention, and a silent overwrite is neither recoverable nor noticed. And there is **no size cap** -- `maxFileBytes` is a cap on text going through JSON, any number here would be the wrong one for somebody dropping a video, and the caller already has a shell on this machine through every terminal in the row. A linked machine takes a drop too (`uploadRaw`), streamed on rather than buffered, which needed one non-obvious thing: `fetch` silently refuses a streamed request body without `duplex: 'half'`. The peer's refusals come back whole, so a duplicate name on another machine reads exactly as it does here. In the panel every row is a drop target and a file's is its directory, so the mark lands on the folder that would receive it: hovering a file lights the folder above it, and hovering anything at the top level outlines the tree, which is the worktree root. Marking every row that shared the destination was the first cut and lit a folder's whole contents at once -- which reads as a warning about the files already there rather than as a destination. The tree is re-read at once rather than at the next poll, and the folder is expanded, since dropping into a folder you cannot see inside is a file you have to go looking for. **A drop anywhere else now does nothing, and that is the sharpest part of this.** The browser's own answer to a dropped file is to navigate to it: the IDE replaced by somebody's screen recording, every terminal on screen gone. The sessions survive, being tmux, but the page has to be loaded again. `App` takes `dragover` and `drop` on the window and declines them -- and taking `dragover` is what makes a drop *possible*, which reads backwards until you know the default: refusing the drag leaves the drop to the browser. Measured against a scratch instance. Two files dropped on `assets` landed there byte-identical and the folder expanded to show them; the same name again was refused with "there is already a file called shot.png here" and the file already there was untouched; a drop on a file row landed in that file's directory; 5MB and 120MB files arrived whole with the server's RSS at 158MB and no temp file left behind; "Copying…" is on screen while a large one is in flight; a stray drop on the top bar was prevented and the URL did not change; and on a linked machine a 3MB drop landed byte-identical through the gateway, with the peer's own 409 and a 400 for an escaping name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # server/CLAUDE.md # server/src/remote/peer.ts # server/src/remote/proxy.ts # web/CLAUDE.md
The sign-out button sits right beside the machine's terminal button in the top bar, so one slip signed the browser out and dropped the socket. A window.confirm now stands between the click and the logout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # server/src/files.ts # server/src/remote/peer.ts # server/test/peer-client.test.ts # web/src/views/FilesPane.tsx
Two things stopped the row from moving, and either one was enough. The tree kept the opened row in view with scrollIntoView, which scrolls every scroller above it -- the row of windows included. The click had already started a smooth scroll to bring the window over (revealTile, on focus), and any scroll on that element cancels one in flight, even a scroll to where it already is. Measured: with a file open and the window 385px past the right edge, opening a second file asked for 2779 -> 3176 and the row stayed at 2779. The first file opened always worked because the window grows then, and the growth reveal runs after the tree's effect. The tree now scrolls itself and nothing else. revealTile also skipped the scroll whenever the offset it wanted equalled sentTo, the target of the last requested glide. That guard exists for a request still waiting for the row to be wide enough after a reload, but sentTo outlives the glide -- only a finger, a wheel or a reveal clears it -- so after a reload any later reveal that wanted the reload's own offset did nothing. Measured at 1600px straight after a reload: the growth reveal wanted 3176, the reload had been sent to 3176, and the row stayed at 2779 for every file opened until something else cleared it. The guard now holds only while that wait is actually running. Measured on a scratch instance at 1600, 2000 and 2400px, each from a fresh load, with the window 100, 300 and 500px past the right edge, opening a first file and a second: 13/13 whole on screen afterwards, against 8/13 before. A reload back to the machine's window still lands it whole (3/3), and a file picked 70 rows down in Changes is still scrolled into view in the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
scrollIntoView, which also scrolls the row of windows and cancels the smooth scroll the click had just started. Measured: 2779 → 3176 asked for, row stayed at 2779. The tree now scrolls only itself.revealTileskipped any scroll aimed atsentTo, which outlives the reload's glide, so after a reload a reveal wanting that offset did nothing. The guard now holds only while the reload's wait is running.window.confirm): the button sits beside the machine's terminal button.Gates run locally: typecheck, 819 tests, build.
🤖 Generated with Claude Code