Commit Graph
189 Commits
Author SHA1 Message Date
shadowdaoandClaude Opus 5.5 a80c146b8a feat(viewer): second Vite entry, viewer capability, CodeMirror deps
Task 6 of the terminal file viewer plan: app/viewer.html plus
src/viewer/{main,ViewerApp}.tsx (placeholder ViewerApp for Task 11 to
replace), registers viewer.html as a second Rollup input in
vite.config.ts, adds the file-viewer capability restricted to
file-viewer-* windows (allow-listen/unlisten for the goto event,
allow-destroy for the close-button/prevent_close interaction,
allow-internal-toggle-devtools to match default.json's dev
convenience), installs the CodeMirror packages Task 10 builds
languages.ts on top of (P1), and pins the viewer entry/capability
with a Rust fallback-trap test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:08:01 -07:00
shadowdaoandClaude Opus 5.5 247ec764ea feat(viewer): reload/conflict reducer and editability rules
- viewerState.ts: pure reducer for the clean/dirty, same/changed/gone,
  container-down and overwrite-on-save states (spec §5), plus the
  pollEffect/canSave helpers EditorPane will drive off.
- editability.ts: classifies a fetched file as text/image/binary and
  decides whether it is editable, deferring to Rust's readonly_reason
  when it refuses.

Per preflight P1, languages.ts/.test.ts move to Task 10 (needs the
CodeMirror packages Task 6 installs; out of scope for this task's
worktree). Per P3, the "reloaded" action now carries `truncated` and
`polledHash` so a poll-driven reload of a truncated (prefix-hash-only)
file adopts the polled full-file hash instead of re-triggering a
reload on every subsequent poll -- with a reducer test covering it.
Per P13, tightened the poll_failed/canSave test to start from a dirty
doc so it actually exercises containerDown rather than passing only
because the doc was clean.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:07:28 -07:00
shadowdaoandClaude Opus 5.5 3177af202d feat(viewer): pure file-path matcher for terminal text
Adds findFilePathLinks, a pure matcher that finds file paths (with
optional :line, :line:col, :start-end and #L42/#L40-L50 suffixes) in
a line of terminal text, skipping anything inside a URL and stripping
markdown/quote wrapping from the matched span.

Per pre-flight ruling P7, a slash-having token still requires the
basename to carry a real extension (any extension) or be a known
extensionless basename -- a bare and/or or src/components is no
longer treated as a path.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:06:52 -07:00
shadowdaoandClaude Opus 5.5 4ada54ffc4 feat(viewer): join wrapped xterm rows for link matching
Adds joinWrappedRows/offsetToCell for reassembling a wrapped xterm
line into its full text plus offset->cell mapping, so link matching
can find paths that xterm has soft-wrapped across rows.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:05:06 -07:00
shadowdaoandClaude Opus 5.5 3c1d120305 feat(viewer): IPC types, wrappers and Rust module skeleton for the file viewer
Task 0 of the terminal file viewer plan: the shared interfaces that seven
later tasks build against in parallel. Adds ViewerLocation/ViewerTargetState/
ViewerState/ViewerFile/ViewerPoll to types.ts and their invoke() wrappers to
tauri-commands.ts, creates the file_viewer Rust module (mod.rs with
MAX_VIEWER_WINDOWS/VIEWER_LABEL_PREFIX/is_viewer_label, plus placeholder
registry/resolve/poll/write/window submodules), wires it into lib.rs, and
loosens visibility on the file_commands.rs helpers the viewer commands will
reuse (MAX_READ_BYTES, validate_container_path, validate_container_write_path,
FetchedFile, fetch_container_file, require_running, clip_container_text).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 20:56:33 -07:00
shadowdaoandClaude Opus 5 83c9c24951 test: give two synthesised clicks the detail a real click carries
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 9s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m43s
Build App (Preview) / build-linux (pull_request) Successful in 7m58s
Build App (Preview) / build-windows (pull_request) Successful in 4m54s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The previous commit tightened the gate's click-count check from `> 1` to
`!== 1`, which two tests in the wiring block did not survive: they built
`new MouseEvent("click", { button: 0 })` directly rather than through the
`click()` helper, so `detail` defaulted to 0 and the gate refused them.

The gate is right and the tests were wrong -- a mouseup derived from a real
click always carries `detail >= 1`, and 0 is exactly the synthetic-event
shape the tightening was for. Both now pass `detail: 1`.

I pushed the previous commit without noticing this, having read a truncated
test summary that hid the failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 20:08:36 -07:00
shadowdaoandClaude Opus 5 c6f9c1d43f fix: tighten the click-count check and stop three comments overstating
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m44s
Build App (Preview) / build-linux (pull_request) Successful in 6m6s
Build App (Preview) / build-windows (pull_request) Successful in 5m0s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
Third-round review polish; no behaviour change beyond the first item.

`detail > 1` was justified in a comment by noting a synthesised event
carries `detail` 0 -- which is an argument for letting untrusted synthetic
events through the click-count half of the gate. A mouseup derived from a
real click always carries `detail >= 1`, so the check is now `!== 1`.
Nothing in the container can dispatch a DOM event, so this is hardening
rather than a hole; the comment now says that instead of the reverse.

Three comments claimed more than they hold. The selection check's
paragraph read as though it caught every copy gesture: it sees a drag only
once the drag has spanned a cell, so a press and release inside one
character cell -- or a drag walked back to its start -- still opens the
link. That is the gap the rejected mousedown/mouseup distance check would
have closed, and it is now recorded beside the reason for rejecting it.

`?1002l` was described as taking effect synchronously with the write; it
takes effect when xterm parses it, on its queued write task. And
`modifierPromised` was described as written on every hover, when `hover()`
clears and returns early with no host element -- which leaves it false, the
stricter direction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 20:07:59 -07:00
shadowdaoandClaude Opus 5 593b8168eb fix: a selection is not a request to leave the app
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 7s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 2m42s
Build App (Preview) / build-linux (pull_request) Successful in 6m39s
Build App (Preview) / build-windows (pull_request) Successful in 5m0s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
Re-review found the gate did not cover the gesture users actually make.
xterm's `Linkifier._handleMouseUp` has no click-count check, no distance
threshold and no timestamp, so it activates on the mouseup that *ends a
selection* as readily as on a click. Double-clicking a word or dragging
across a few characters inside an OSC 8 link therefore opened the browser.

Worse with a program holding the mouse: the only way to select text there
is Shift/Option+drag, which is byte-identical to the gesture the gate
accepted as a deliberate request to open. A container wrapping each output
row in a link would have harvested every legitimate copy.

`term.hasSelection()` is the load-bearing check: a drag is one press and
one release, so its click count is 1 and `detail` cannot see it. `detail >
1` is belt-and-braces for the case where the selection came out empty, and
for not depending on the selection model being written before the
Linkifier's listener runs -- it is, but the check costs nothing. Drag
distance was rejected rather than forgotten: xterm hands `activate` only
the mouseup, so measuring it means binding our own listener and keeping a
second source of truth about one gesture.

The hover card's promise is now sticky. The hint was computed once at hover
while the gate re-read the mode at mouseup, so a card reading "Shift+click
to open" could be on screen while a bare click opened the link. The gate
now requires the modifier if either the card asked for it or the live mode
does.

The same gate is applied to the WebLinksAddon branch, which had none. That
also closes a real bypass: `OscLinkProvider` drops non-http(s) OSC 8
targets before `linkHandler` sees them, so a `javascript:` target with an
`https://evil.tld` label fell through to WebLinks and opened ungated.

What is not closed, and is now recorded rather than papered over: the mouse
mode is a permission the container grants itself. It can drop tracking
before the pointer arrives and hold it off through the click. The selection
and click-count checks hold either way, so the mass-harvest variant is
gone, but the real fix needs a signal the container cannot write and this
pane does not have one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 20:00:42 -07:00
shadowdaoandClaude Opus 5 ac50c38891 fix: gate OSC 8 link activation instead of merely hinting at it
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 5s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m42s
Build App (Preview) / build-windows (pull_request) Successful in 5m3s
Build App (Preview) / build-linux (pull_request) Successful in 7m13s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Review of this branch found its central premise was false. The claim was
that xterm cancels a mousedown before the link layer while a program holds
the mouse, so only a Shift+click could reach a link. None of that holds:
`cancel()` is `if (this.options.cancelEvents || force)` and `cancelEvents`
defaults to false and is never set here, so it does nothing; the mouse
reporting listeners bind to `.xterm` while the Linkifier is constructed on
`screenElement`, a descendant, so the link layer sees the event first
regardless; and `_handleMouseUp` checks neither the modifier nor the
button before calling `activate`.

So a plain click opened the link, and so did a right-click. That is not a
missing convenience. OSC 8 lets the container wrap any clickable TUI widget
-- a menu row, a "1. Yes", a file chip -- in a link to anywhere, and
because the mouse report still reaches the program afterwards the widget
responds too and nothing looks wrong. The hover card was the only
mitigation, and it assumes a user deliberately reaching for a link.

`opensOnClick` is now a real gate: primary button only, and while a program
tracks the mouse the force-selection modifier is required -- the gesture
the user already has for "this click is for the terminal, not the program".
With nothing tracking, a bare click opens, which is what WebLinksAddon
already does for plain-text URLs in the same buffer. The mode is read per
click through a getter rather than captured, and `syncMouseCapture` and the
gate share one expression, because a gate that disagreed with the badge
would be the hole again.

The gate and the hint also share one modifier predicate, and the hint is
conditional on tracking, so it can never name a key that does nothing.

Three more from the same review. The origin span had `flexShrink: 0`, which
beats `overflowWrap` under flexbox, so an attacker-controlled 600-character
origin ran off the pane and hid the registrable domain -- the same spoof as
an ellipsis, without one; it now wraps and the remainder is what gives way.
The card had no `pointerEvents: none`, and `xterm-hover` is inert at this
placement, so a card under the pointer took `mouseleave` from screenElement
and made bottom-row links flicker and refuse to activate at all. And the
design doc comment had come adrift from its function.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 19:45:11 -07:00
shadowdaoandClaude Opus 5 f311ca1990 feat: make links in Claude's output clickable
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m47s
Build App (Preview) / build-linux (pull_request) Successful in 8m8s
Build App (Preview) / build-windows (pull_request) Failing after 13m30s
Build App (Preview) / prune-previews (pull_request) Skipped
Claude Code prints links as OSC 8 hyperlinks whose visible text is
hard-wrapped into terminal-width pieces -- urlDetector's header records a
346-character sign-in URL arriving as five emissions, each carrying the
whole URL in its parameter and about 80 characters on screen. WebLinksAddon
regex-matches the painted characters row by row, so against Claude it
matches a fragment or nothing, which is why the URL toast exists.

xterm 5.5 hands over the exact parameter through `linkHandler`, so the
slicing stops mattering. WebLinksAddon stays for plain-text URLs in
ordinary shell output; the two cover different cases and neither replaces
the other. Both now share one failure reporter and one validator.

No new key handling was needed. xterm's mousedown handler is
`if (areMouseEventsActive && !shouldForceSelection(e)) return cancel(e)`,
so holding the force-selection modifier lets the event reach the link
layer while Claude still holds the mouse -- Shift+click, or Option+click on
macOS, which this terminal already enables for text selection.

The hover card is the security half rather than decoration. OSC 8
decouples the label from the target completely: a container can print
`https://claude.ai` and link it anywhere, which is strictly worse than the
userinfo spoofing already guarded against and which invalidated the
justification for opening a click without confirmation ("a deliberate act
on visible text"). Hovering now shows the real origin, in full and never
truncated, because truncating it is the spoof. A target that fails
validation says so and deliberately echoes nothing of itself.

The hint names the modifier for the platform, from xterm's own `isMac`
list, so it cannot tell a Mac user to press a key that does nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 19:23:18 -07:00
shadowdaoandClaude Opus 5 60188610ee fix: do not let an in-flight open blank a newer prompt, or promise a bridge that is off
Two findings from review of this branch.

Awaiting the open instead of dismissing up front bought a window: on Linux
it is at least OPENER_GRACE, doubled when xdg-open fails and gio is tried.
If the container relays a second URL inside that window, the first open's
resolution blanked the second prompt -- losing a link that exists only in
the container's transcript, which is the failure "dismiss on success only"
was made to prevent. The slot already carried a `seq` for exactly this
reason; dismissal is now conditional on it.

`urlPromptRef` is written eagerly by the two functions that change the slot
rather than synced by an effect. That is load-bearing: an effect-synced
mirror lags state by a commit, and a promise microtask can resolve between
`setUrlPrompt` and React flushing passive effects -- so it answers "did a
newer prompt land?" wrong in precisely the window the guard exists for.
Dropping the functional updater also fixes `promptSeqRef.current += 1`
being mutated inside a state updater React is free to invoke twice.

The guard is a sibling function rather than an optional argument on
`dismissUrlPrompt`, because that function is passed by reference as
UrlToast's `onDismiss` and React would hand it a MouseEvent as its first
argument -- the seq check would fail and the close button would silently
stop working, with the types still assignable.

Separately, the sign-in hint was binary on which button leads, but "host
leads" covers both a live bridge and a fallback where nothing is set up to
catch the callback at all. In the second case the toast promised the bridge
would carry it and the login hung to its timeout. The target is now
three-state, the hint tells the truth in the fallback case and names the
control that fixes it, and the hook starts at `host-fallback` rather than
assuming a bridge it has not confirmed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 11:12:22 -07:00
shadowdaoandClaude Opus 5 5a09254538 fix: route every host-browser open through open_url_external
The Rust command existed but nothing called it. All four frontend call
sites still used `openUrl` from `@tauri-apps/plugin-opener`, so the
environment fix was inert and the three dialogs carried the same Linux bug
as the terminal: DockerInstallDialog's docs link, ClaudeAuthModal's sign-in
link and UpdateDialog's release link would all have reported success while
launching nothing.

`openUrlExternal` in tauri-commands.ts is now the single sink. There is no
platform branch: Linux gets the sanitized spawn, macOS and Windows reach
the same plugin as before but from Rust, and every platform picks up the
Rust-side re-validation, which matters because these URLs originate in an
untrusted container.

Comments in urlRelay.ts and urlDetector.ts that named `openUrl` as the sink
they guard are updated to match, and the two test files that mocked
`@tauri-apps/plugin-opener` now mock the command instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:09:57 -07:00
shadowdaoandClaude Opus 5 bf8094dbc4 fix: route sign-in links by what can actually catch the callback
`isAnthropicSignInUrl` made the container the default action for every
Anthropic sign-in link, justified by "the host has nothing to catch it
with". That was wrong in both directions. The host does have something --
the auth bridge -- and the container side is not a general browser at all
but Playwright's dashboard, whose packages and chromium are deliberately
not baked into the image. So the default pointed at the one path that is
uninstalled on a fresh project, on every platform, while the path that
works sat behind a switch.

The decision now lives in `useSignInOpenTarget`: a live auth bridge picks
the host, otherwise a container that can actually launch a browser picks
the container, otherwise the host. It resolves at mount rather than when a
URL arrives, so the buttons do not swap under a moving mouse, and it
re-decides on `auth-bridge-changed` so flipping the switch during a
hanging login takes effect. A bridge with port conflicts reads as not
live; an empty `active_ports` does not, since there is nothing to bridge
until the CLI binds its listener and that races the URL.

Both buttons still render either way -- this changes which one leads.
`sanitizeRelayUrl` is byte-for-byte unchanged, so the embedded copy in
web_terminal/terminal.html needs no matching edit.

The host "Open" path also failed silently: `dismissUrlPrompt()` ran before
`openUrl`, so the toast vanished and a rejected promise reached only the
devtools console. Dismissal now happens on success only, leaving "In
container" one click away after a failure, and the error surfaces through
the same toast the container path already used. On Linux this catch will
not fire for the common case -- `xdg-open` routinely exits 0 having done
nothing -- so it complements the AppImage environment fix rather than
replacing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:07:50 -07:00
jknapp b59c6148ff Merge pull request 'Read a stopped container instead of claiming there is nothing to read' (#55) from fix/staleness-probe-stopped-container into main
Build App / compute-version (push) Successful in 4s
Secret Scan / scan (push) Successful in 4s
Build App / build-macos (push) Successful in 3m29s
Build App / build-windows (push) Successful in 5m1s
Build App / build-linux (push) Successful in 5m10s
Build App / create-tag (push) Successful in 7s
Build App / sync-to-github (push) Successful in 1m36s
2026-09-11 03:54:40 +00:00
shadowdaoandClaude Opus 5 95a78fe9a3 Take the review: cache the stopped probe, and never let it cost an answer
Secret Scan / scan (push) Successful in 6s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m58s
Build App (Preview) / build-linux (pull_request) Successful in 4m43s
Build App (Preview) / build-windows (pull_request) Successful in 5m9s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
Six findings, all real. The one that mattered: `getContainerStaleness` is
called from a `useEffect` that fires whenever the container settles, so
merely opening a stopped project's Overview now committed its whole writable
layer — 44 s on a real project, against ~3 s for the snapshot probe it
replaced. Shipping that would have traded one bad banner for a bad page.

A stopped container's writable layer cannot change, so the probe is exactly
cacheable: `STOPPED_MANIFEST_CACHE` keys on the container's `FinishedAt`,
which moves on every stop. Cold 2967 ms, warm 1 ms, measured. A live test
asserts the restart case as well as the hit, because a cache that failed to
invalidate would plan a migration against a filesystem the project no longer
has — verified by breaking the token and watching that assertion fail.

Skipping the probe for projects that are not stale looked like the cheaper
fix and is unsafe: the deltas would be empty while `probeSettled` stayed
true, and the migrate action in the project menu is not gated on the banner,
so the pre-flight would report nothing to copy while the backend was told to
copy nothing. That is the hazard `canMigrate`'s comment already warns about.
Not done, and written down so it is not tried again.

Also from the review:

- A failed commit no longer costs an answer the snapshot could have given.
  Before this feature a stopped project read its snapshot directly, so
  surfacing this error would have made the banner worse than it was — and
  the failure modes are where the fallback earns its keep: a full disk (the
  commit allocates the whole layer, the snapshot probe allocates nothing)
  and a 409 from a concurrent claim.
- The probe no longer commits while the project is claimed. The collision is
  not symmetric: the probe losing is a retryable `probe_error`, but
  `start_project_container` removes the old container with a hard `?`, so a
  remove that raced a commit would fail the user's Start with an opaque
  error. `stopped_probe_policy` reads `project_lock::held` and probes the
  snapshot instead, or defers with a message that says so.
- The cleanup-failure warning claimed the next probe of the same container
  would reclaim the leftover. Unique names made that false the moment they
  landed; it is `reap_probe_images` that collects it.
- The TS binding still called the command read-only, which is how the
  auto-refresh got added in the first place.
- CLAUDE.md still documented the stable `triple-c-probe-{cid}:latest` name
  this PR removed as unsafe.

548 unit tests, 752 frontend tests, 4 live-Docker tests. Clippy unchanged at
44 warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RSaoDLovVV2wmH4H8VVxz
2026-09-10 20:48:10 -07:00
shadowdaoandClaude Opus 5 c0e4c87cec Give the mouse back, retire the follow controls, update Claude per session
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m44s
Build App (Preview) / build-linux (pull_request) Successful in 5m8s
Build App (Preview) / build-windows (pull_request) Successful in 6m28s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
Build Container / build-container (pull_request) Failing after 14m49s
Three things the terminal was getting wrong.

**A program that grabs the mouse and dies used to freeze the tab.** A TUI sets
DECSET ?1000/?1002/?1003; if it exits without resetting them, xterm keeps
routing clicks, drags and — under ?1003 — every pointer *move* to the PTY.
Text selection dies and escape bytes flood the prompt. The only exit was
closing the tab. `TerminalView` now reconciles a flag against
`term.modes.mouseTrackingMode` in the `term.write()` callback — the mode only
changes because the container printed a sequence, so one check per write
catches every transition with no polling — and `Ctrl+Shift+X` or a status-bar
button writes the resets back through `term.write`, never `sendInput`: the
reset belongs to xterm's parser, and a still-live TUI told about it would just
re-grab on its next repaint.

The control is in the status bar deliberately. Mouse tracking is the *normal*
state of htop, vim, lazygit and Claude Code, so a badge over the terminal
would be on screen for the whole life of those programs and would swallow
clicks aimed at their own top-right corner. `macOptionClickForcesSelection` is
also on now: xterm's force-select is Shift everywhere except macOS, where it
is Option and is gated behind that option, which defaults to false — so until
now Mac users had no way to select text while a program held the mouse.

**"Following" and "Jump to Current" are gone.** Claude Code draws on the
alternate screen, which has no scrollback, so `viewportY` always equalled
`baseY` and neither control could do anything. They did still work in bash
tabs; xterm's native follow covers that, and the per-write `scrollToBottom()`
went with them because it fought exactly that. What remains, on activate and
after a refit, now samples `viewportY >= baseY` *before* the fit, so opening
the Notes dock no longer yanks a reader to the tail.

**`claude update` runs before every Claude session, not just at container
start.** Containers here stop/start and often just keep running, so a
long-lived one never re-checked. Both copies take the same flock: the
entrypoint prints "container ready" only after its own update finishes, so
opening a tab immediately would otherwise run two updaters against the same
~/.claude/bin, with `|| echo` hiding a half-written install one line before
`exec claude` ran it.

This turns the non-Bedrock path from a bare argv into a `bash -c` wrapper, so
flags and session names are shell-interpolated now and must go through
`shell_quote_arg`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0145mQi9NZiCDrznBUEEDE4n
2026-09-08 11:02:33 -07:00
shadowdaoandClaude Opus 5 c16f0d5b70 Put the cursor in the terminal after sending a note
Secret Scan / scan (push) Successful in 6s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m44s
Build App (Preview) / build-windows (pull_request) Successful in 5m3s
Build App (Preview) / build-linux (pull_request) Successful in 5m21s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Sending already switched to the target terminal's tab, which looks like it
should be enough: `TerminalView` focuses xterm whenever a terminal becomes
active. But that effect keys off `active`, so it only fires on a *change* —
and the dock's ordinary case is sending to the terminal already on screen.
`setActiveTabKey` writes the key that is already set, nothing changes, no
effect re-runs, and focus stays on the Send button. The note is sitting in the
prompt and the user still has to click the terminal before pressing Enter.

So the send now asks for focus explicitly, through a one-shot request in the
store that `TerminalView` consumes and clears — the shape `pendingHomeTab`
already uses. Clearing is not tidiness: hold the id and the second send to the
same terminal writes a value that is already there, which is precisely the
no-op this exists to fix.

Focus is requested only on success. A failed send toasts and leaves the user
where they are, because there is nothing in the prompt to press Enter on.

The three `TerminalView` tests give focus away after mounting before making
any assertion, so what they observe is the request landing and never the focus
that `active` already grants on mount — which would pass with the feature
absent.

752 tests pass, 62 files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
2026-09-02 13:28:31 -07:00
shadowdaoandClaude Opus 5 3239057f8f Give the dock its own compact notes layout
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m44s
Build App (Preview) / build-windows (pull_request) Successful in 4m55s
Build App (Preview) / build-linux (pull_request) Successful in 5m29s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The dock was showing `NotesPanel`, which is a master/detail layout: a column
of titles beside an editor. The previous commit made that survive dock width;
it did not make it right. At 352px the layout still spends roughly 356px of
height on chrome — dock header, panel header, title strip, a button row that
wraps, and a paragraph of help — before the body gets a pixel.

So the dock now shows one note. The title field names what is open and the
chevron beside it switches; New and Delete move into the overflow menu; the
help text goes. Chrome drops to about 112px and the body takes the rest.

The two surfaces are now different components, which contradicts a docstring
I wrote — "shared so the two cannot drift into different behaviour". That
claim was about behaviour, and behaviour was never in the layout: it is in
`useNotes` for the cache and its write ordering, and now in `useNoteDraft`,
extracted here so when a keystroke becomes a save is defined in exactly one
place. Only the layout diverges. `NotesPanel.shared.test.tsx` gets stronger
for it — it now mounts the dock panel and the tab panel together, which is
what the app actually does, instead of the same component twice.

`NoteSwitcher` is not `OverflowMenu` despite the shape being close: that keys
items by label, and notes are addressed by id, so two untitled notes — the
ordinary case — would collapse into one row. It is also not a `combobox`; an
input plus a listbox button is two honest controls, where the role would owe
active-descendant tracking and filtering that nothing here needs.

`SendToAgentButton` picks up `useUnavailable` from #49, which is what its
`disabled` plus explanatory `title` was already asking for. Four tests moved
from `toBeDisabled()` to the new contract, and one of them — "does nothing for
an empty note" — turned out never to have asserted that it does nothing. It
does now, for click and for Enter, which is the guard the swap needs.

It also gains `dropUp`, and that is load-bearing rather than cosmetic: the
dock clips its own overflow, so a session menu opening downward from a button
on the bottom edge is drawn outside the panel and never seen.

744 tests pass, 62 files. As before, jsdom has no layout engine: that the dock
now reads as compact is not something the suite can tell you.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
2026-09-02 12:10:00 -07:00
shadowdao 23364f412e Merge remote-tracking branch 'origin/main' into feat/project-notes 2026-09-02 12:04:17 -07:00
shadowdaoandClaude Opus 5 0f3fff92f4 Lay the notes panel out by its own width, not the window's
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Secret Scan / scan (pull_request) Successful in 5s
Build App (Preview) / create-release (pull_request) Successful in 3s
Build App (Preview) / build-macos (pull_request) Successful in 2m45s
Build App (Preview) / build-windows (pull_request) Successful in 4m58s
Build App (Preview) / build-linux (pull_request) Successful in 5m13s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The panel splits master/detail unconditionally: a 192px title column beside
the editor. That fits the Project Home tab and does not fit the dock. At the
dock's 352px default the editor gets 157px, and its action row wants ~200px,
so the Delete button lands outside the dock's `overflow-hidden` with no
scrollbar to reach it, and the textarea collapses to a two-word column.

The two surfaces differ in width while sharing a viewport, so this is a
container query rather than a `md:` breakpoint — a viewport query reads the
window and hands both surfaces the same answer, which is wrong for one of
them. Tailwind v4 has these in core; verified as real
`@container (min-width: 32rem)` rules in the built CSS, since a variant that
silently compiles to nothing looks identical in review.

The threshold is arithmetic: side by side needs the 192px list, an editor
wide enough for its own buttons (~280px), and the divider. `@lg` (512px) is
the first stop clearing ~473px. Below it the titles become a capped strip
above the editor, so the note being written keeps the height.

The action row now wraps, which is the part that holds at *any* width rather
than on one side of a threshold: the buttons are a group that does not shrink,
the title field shrinks to 96px, and past that the title takes one row and the
buttons the next. Nothing can be pushed out of the panel.

Not covered by the suite — jsdom has no layout engine, so 711 tests pass
before and after. This needs eyes on the dock at its minimum, default and
maximum widths.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
2026-09-02 11:34:02 -07:00
shadowdaoandClaude Opus 5 1eb91a35eb Give the terminal and Add Project buttons a reason a screen reader can hear
Secret Scan / scan (push) Successful in 10s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 10s
Build App (Preview) / create-release (pull_request) Successful in 3s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m56s
Build App (Preview) / build-linux (pull_request) Successful in 6m47s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Both were the defect the new hook exists for. The sidebar's Claude terminal
button is disabled whenever the container is not running and never said so —
its `title` names the action, so the precondition appeared nowhere in the
accessibility tree at all. Add Project's submit button is disabled while an
add is in flight, and its only signal is the label swapping to "Adding…" on
an element a screen reader can no longer reach.

The submit button needs a second guard the hook cannot supply: Enter inside
a text field submits a form without touching the submit button, so
`handleSubmit` now returns early while loading. Without it, swapping
`disabled` for `aria-disabled` would have turned an accessibility fix into a
double-submit bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
2026-09-02 09:08:57 -07:00
shadowdaoandClaude Opus 5 aa0a574091 Announce unavailable controls instead of hiding them behind disabled
Native `disabled` removes an element from the tab order and from the
accessibility tree, so any explanation of why a control cannot be used is
delivered only to a sighted user with a mouse. `useUnavailable` is the way
out: `aria-disabled` keeps the control focusable and announced,
`aria-describedby` carries the reason, and — because `aria-disabled` is
advisory and blocks nothing — the hook hands back the click and Enter/Space
guards along with the attributes, so a call site cannot take the
announcement without the guard.

`Button` gets it as an opt-in `unavailable` / `unavailableReason` pair.
Opt-in matters: 37 files render this button and none of them change. The
`aria-disabled:` class mirrors exist because Tailwind's `disabled:` variant
only matches the native attribute, which this pattern deliberately omits.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
2026-09-02 09:08:50 -07:00
shadowdaoandClaude Opus 5 2708772bf9 Order the notes cache by sequence, not by who resolves last
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 2m48s
Build App (Preview) / build-linux (pull_request) Successful in 6m2s
Build App (Preview) / build-windows (pull_request) Successful in 6m4s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
One gesture puts two requests in flight. With the tab already loaded, clicking
the dock toggle while the textarea has focus fires `blur` -> `saveNote` and the
dock's mount -> `list_notes` in the same tick. The save finishes and its re-read
writes the post-save list; the mount's read -- issued earlier, still out -- then
lands its pre-save snapshot on top, and both panels show stale text until
something else refreshes.

`mutationChains` could not have caught this: it orders a project's writes
against each other and the mount load is a read outside it. Putting the read on
the chain would work, but it buys correctness with latency the user feels -- a
panel mount waiting behind `save_note`'s double-fsync write -- and leaves a
"mutation chain" holding reads.

The two requests are not competing for a resource; the loser's result is simply
older. So every write into `notesByProject[p]` now claims a per-project sequence
when the request behind it is issued, and `commitNotes` drops one whose sequence
predates what is already cached. Reads take their sequence at issue time, since
being ordered by resolution is the bug. Local patches -- the filter behind a
confirmed delete, the prepend behind a failed re-read -- take a fresh one at
commit time, because they are authoritative then rather than derived from an
earlier read, and anything still in flight behind them is genuinely stale. A
failed read commits under its *own* sequence, not a fresh one, so its empty list
cannot beat a later read that has the real answer.

`isCurrent()` stays, and is not folded in. It guards `setSaveState`, not the
cache: it asks whether this *panel* is still showing the project a save was made
for, which a per-project counter cannot answer -- two panels on one project share
every sequence value. Ordering and panel identity are two questions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HjL1E2JFNctUqCYotUwqqb
2026-09-01 14:23:55 -07:00
shadowdaoandClaude Opus 5 436b6dd470 Send a lone CR through the newline transform, and say what pinned is
`toClaudePayload` matched `/\r?\n/`, so a bare CR that is not part of a
CRLF went through verbatim — and a bare CR *submits* in a Claude prompt
and *runs* the line in a shell, which is the terminator the function's
own contract says it never appends. A `<textarea>` cannot produce one,
but `load_in` returns whatever a hand-edited or externally written notes
file holds, so the guarantee has to cover that rather than only what the
editor can type.

`Note.pinned` is persisted and sorted on, but nothing in the app sets
it: there is no pin control and no indicator. The spec stated the
ordering rule as though pinning existed and §8 did not list it, so the
spec is amended to say `pinned` is reserved and inert in v1, and pinning
is added to the out-of-scope list. No UI is added — a user-facing
affordance does not belong in a fix wave.

Also renamed NotesPanel.test.tsx's "deletes the selected note and falls
back to another": `useNotes` is mocked in that file and the mocked list
never changes, so the fallback was never exercised. A name that claims
coverage which is absent is worse than an absent test, because it makes
the gap invisible. The real assertion now lives against the real hook in
NotesPanel.shared.test.tsx.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HjL1E2JFNctUqCYotUwqqb
2026-09-01 13:47:01 -07:00
shadowdaoandClaude Opus 5 5c47656444 Cache notes in one place, and serialise a project's writes
Implements the design spec's §2 — "notes cached in zustand keyed by
project id" — which the plan substituted with a hook-local `useState`.

Sharing the `NotesPanel` *component* between the Project Home sub-tab and
the dock did not share the *cache*. Both resolve to the same project, so
two panels mount two `useNotes(P)`, each with its own list. Edit a note
in the dock and blur; the tab's copy is still pre-edit, and the tab's
next blur commits `{...staleRecord, title, body}` — the dock's edit gone
from disk with no error and no indicator. That is the feature's own
primary workflow: take notes in the dock while the agent runs, which is
the reason the dock exists, then go back to the tab.

`notesByProject` plus a per-project in-flight flag now hold the list.
Both surfaces render from one array; two panels mounting for one project
make one read; and because the write is keyed by project, a response
that lands after the user has moved on updates the project it belongs to
rather than whichever is on screen. This is also the boundary §8 says a
detached notes window needs.

Three more bugs in the same code, fixed with it:

- Delete-after-edit could resurrect the note. Clicking Delete with the
  textarea focused fires blur first, so `save_note` and `delete_note` go
  out back to back; Rust's `write_lock` stops them interleaving but does
  not order them, and a delete that wins the lock is undone by the
  upsert behind it. A project's mutations now go through one promise
  chain, module-scoped for the reason `useTerminal`'s input queue is.
- An unsaved draft vanished when any other note was saved, because the
  re-read replaced the list with the backend's. "New note" now persists,
  so the backend owns the row from the start — chosen over merging local
  drafts because a local-only row in a *shared* cache would exist in the
  panel that made it and nowhere else.
- The save outcome was reported for the wrong project after a switch:
  the guard covered only the list replacement, so the new project's
  SaveIndicator flashed "Saved ✓" for the old project's write. The
  indicator now resets on a project change and reports only its own.

`NotesPanel` also re-seeds its draft when the *stored* text of the note
it has selected changes, so an edit made in the other surface reaches
the editor and not only the list. It never overwrites something
half-typed; that still blurs into a last-writer-wins save, as any
blur-commit editor does.

NotesPanel.shared.test.tsx is the configuration none of the existing
tests had: two panels, one project, the real hook. Four of its six
assertions fail against the previous implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HjL1E2JFNctUqCYotUwqqb
2026-09-01 13:47:01 -07:00
shadowdao 037ed78570 Test the dock's load-path clamp and keyboard resize direction
- notesDockWidth store initialization now clamps/defaults a bad
  localStorage value on load, not just on write (verified this fails
  without the clamp).
- The keyboard resize test asserts the exact widened/narrowed value
  instead of just that the setter was called, so a swapped or
  inverted arrow-key branch would be caught.
2026-09-01 13:22:35 -07:00
shadowdao 31e8f9df5f Add the notes dock 2026-09-01 13:15:13 -07:00
shadowdao 3704064006 Add the Notes tab 2026-09-01 13:08:17 -07:00
shadowdao f79a44e0a8 Add the send-to-agent button 2026-09-01 13:02:22 -07:00
shadowdao 5a8e24ccbe Extract the Claude newline sequence and the session display name 2026-09-01 12:57:29 -07:00
shadowdao a1f4eee9a3 Fix critical cross-project data corruption bug in useNotes hook
When a save is in flight for project A and the user switches to project B before it resolves, the stale closure still has projectId=A. When A's save resolves, the post-save re-read of listNotes(projectId) runs with the stale closed-over projectId, and setNotes(reloaded) overwrites B's displayed notes with A's list—the same cross-project contamination class as Finding 2 but reintroduced through the fix itself.

Fix: Add a currentProjectId ref updated on every render, and guard both saveNote and deleteNote callbacks with a check before replacing/filtering the whole list. If the project changed while the async operation was in flight, bail out of the state update but still report success (the operation itself succeeded on the backend; only the stale list update is skipped).

Added test: a save in flight for one project, a switch to another, then the first save resolving—asserts the second project's notes are still displayed.
2026-09-01 12:50:23 -07:00
shadowdao b6ba6deb09 Fix critical data corruption and stale-data bugs in useNotes hook
- Finding 1 (saveNote): After a successful save, re-read the canonical list from the backend instead of patching in place. A successful save stamps a new updated_at, and the backend sorts by updated_at descending, so the record's position has changed and positional patching would disagree with what a reload would show. If the re-read fails, keep the save reported as successful and leave the existing list alone.

- Finding 2 (stale notes): Clear notes on projectId change (not only when empty) and on load failure. Previously, switching from project A to project B would leave A's notes on screen until B's fetch resolved, and if a user edited one, A's note would be written into B's notes file—cross-project data corruption. If a load fails, A's notes stay visible under B indefinitely.

- Added four new tests covering these scenarios: projectId change clears old notes, failed load leaves no stale notes, saving a new note ends with the backend's list, and saves re-read the list rather than patching.
2026-09-01 12:45:35 -07:00
shadowdao cd3160b1cd Add the notes hook and its IPC wrappers 2026-09-01 12:38:33 -07:00
shadowdaoandClaude Opus 5 3a49a67c1f Fix terminal input reordering and Linux terminal rendering
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-linux (pull_request) Successful in 5m25s
Build App (Preview) / build-windows (pull_request) Successful in 5m32s
Build App (Preview) / prune-previews (pull_request) Successful in 8s
Two separate defects behind the same report: typing in a container terminal
is sluggish on Linux, and a backspace can land *after* the characters typed
behind it.

The web terminal was the control that separated them. It shares the Docker
exec, the PTY, `exec_manager`, the input channel and its serial writer task,
and xterm.js itself — and it does not exhibit either symptom. Only three
things differ, and each accounts for part of the report.

**Input ordering.** Every keystroke was its own `invoke("terminal_input")`.
That command is `async`, so Tauri spawns each one as an independent task, and
those tasks then race for the session mutex in `ExecSessionManager::send_input`
— nothing preserved the order the bytes were typed in. The serial writer
downstream cannot help, because the order is already lost before anything
reaches the channel. The web terminal gets ordering for free by awaiting
`send_input` inline in a single WebSocket reader loop.

`useTerminal` now holds a per-session queue: one write in flight at a time,
the next only after the previous resolves. Anything typed meanwhile coalesces
into the next chunk, which also collapses a burst of typing into a couple of
IPC round trips rather than one per key. The queue is module scope, not hook
scope, because `useTerminal()` is called from several components — a per-hook
queue would leave speech-to-text, image paste and typing racing each other.
Each caller's promise still settles only when its own bytes have gone, so
`await sendInput(...)` keeps its meaning.

**The DMA-BUF escape hatch did not exist.** `apply_webkit_wayland_workaround`
left any pre-set value alone, including `0`, on a stated assumption that
WebKitGTK reads the variable as a boolean. It reads presence, so
`WEBKIT_DISABLE_DMABUF_RENDERER=0` disabled DMA-BUF exactly like `=1`, and no
value a user could set got the accelerated path back. `0`/`false`/`no`/empty
now remove the variable, which is the only thing WebKitGTK reads as enabled.
The default is unchanged: unset still means disabled on Linux.

**WebGL does not degrade to canvas here.** The comment on that workaround
assumed `@xterm/addon-webgl` would fall back to the canvas renderer once
DMA-BUF was off. Its constructor throws only when WebGL is *absent*, and with
DMA-BUF disabled WebGL is still present — served by software rasterisation.
So the addon loads and every frame is rendered on the CPU, slower than the
canvas renderer it was assumed to fall back to. `AppSettings::terminal_gpu_
rendering` decides whether it loads at all: `None` is auto (on for macOS and
Windows, off on Linux), `Some(_)` forces it either way from Settings →
Terminal. `Option<bool>` rather than `bool` so the zero value means "we
choose" instead of pinning every existing settings file to one answer.

Verified: 643 frontend tests and 530 Rust tests pass, clippy clean, secret
scan clean. The Linux rendering half needs confirming on a real desktop —
neither symptom reproduces in a headless container.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ApLYH6ybHwQFkMCtKuHrrV
2026-08-28 12:51:18 -07:00
shadow-test 97e58db3c1 Close gateway-secret desync, TOCTOU, and undisclosed custom-image gaps
Secret Scan / scan (push) Successful in 6s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 5s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m53s
Build App (Preview) / build-linux (pull_request) Successful in 7m5s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Round 4 review findings:

- Disclose and warn on a custom Docker image the import would set (HIGH):
  it's the image every project container is created from, so an
  undisclosed change here was a sharper version of the redirected-base-URL
  problem round 3 already flagged for the model backends.
- Recreate a running gateway container when an import restores a new
  secret with the shape unchanged (MEDIUM): reconcile_gateway's shape
  comparison can't see a secret-only change, so the container would
  otherwise keep serving old key material indefinitely.
- Report keychain write failures back to the caller instead of only
  logging them (MEDIUM): apply_settings_import now returns
  SettingsImportOutcome with secret_restore_warnings so a partial restore
  can't read as unqualified success.
- Pin a hash of the previewed file's ciphertext and refuse to apply if it
  changed on disk (MEDIUM): closes a TOCTOU between preview and apply.
- Sanitize and cap every free-form string a preview surfaces, and move the
  warning boxes above the replace list in the UI (MEDIUM): an unbounded
  base URL or image name could otherwise push the security warnings below
  the scroll fold.
- Validate the Docker socket path on import the same as the SSH key and CA
  cert paths (LOW): it was the one mounted host path validate_settings_update
  didn't cover.
- Fix ExportedSecrets::is_empty() to treat whitespace-only as blank, like
  every other secret-presence check in this feature (LOW).
- Authenticate the file header as AEAD associated data (LOW, defense in
  depth) and correct two doc comments that overstated the password not
  being cached.
2026-08-27 14:24:06 -07:00
shadow-test a606e3ab20 Validate settings imports before writing secrets; disclose base URLs
Secret Scan / scan (push) Successful in 14s
Build App (Preview) / compute-version (pull_request) Successful in 7s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m43s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 7m29s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A rejected import (bad env var name, disallowed host path) used to leave
keychain secrets already overwritten while the settings themselves stayed
unchanged. apply_settings_import now runs update_settings's validation
(extracted into validate_settings_update) before any secret write.

Also from this review round: sharpened two format-version tests that
previously passed against the pre-fix code too, added a direct test for
split_settings_and_secrets, warned on a dormant web terminal token even
when the terminal import leaves it off, matched the password-length check
to the frontend's unit of measure, zeroized the export plaintext buffer,
and surfaced non-blank Ollama/llama.cpp/OpenAI-compatible/gateway base
URLs in the import preview so a traffic redirect isn't silent.
2026-08-27 13:13:48 -07:00
shadow-testandClaude Sonnet 5 925e51e435 Fix a real credential-leak vector a review found, plus four smaller issues
Secret Scan / scan (push) Successful in 12s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m51s
Build App (Preview) / build-linux (pull_request) Successful in 5m12s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The headline finding: WebTerminalSettings::access_token is a live bearer
credential for a server that binds every interface, stored as a plain
field on AppSettings — which this feature was exporting and importing
wholesale as if it were as inert as a port number. A crafted export file
could set web_terminal.enabled and access_token together, and importing
it (with no more warning than any other setting change) would silently
stand up a LAN-listening terminal server with an attacker-known token on
the victim's next launch.

Fixed by carving the token out into ExportedSecrets, same as the other
three global secrets, with the same "only overwrite what the import
actually has" treatment — except that has to be done by hand here, since
this one lives inside the AppSettings blob that gets replaced wholesale
rather than in the keychain. Added SettingsImportPreview::
enables_web_terminal so "this turns on a listening service" gets its own
visible warning in the confirmation modal rather than hiding inside a
generic "settings replaced" bullet list.

Also fixed:

- read_and_decrypt checked format_version only after attempting to parse
  the full payload, so a future version bump that isn't
  deserialize-compatible would fail on the shape mismatch before the
  version check ever ran — and serde's type-mismatch errors quote the
  offending value inline, which is a real leak path since the plaintext
  here can hold a live credential. Now probes just the version field
  first, and neither error path interpolates the underlying serde message
  into what the user sees.
- apply_settings_import cleared the pending-import path before it could
  fail, so a rejected import (an invalid host path, anything
  update_settings validates) dead-ended the modal with no way back except
  cancelling and reopening the file picker. The path is now only cleared
  on success.
- Secrets are restored before the settings replace runs, not after —
  replacing settings is what triggers reconcile_gateway, and restoring
  secrets afterward left a real window where a gateway recreation
  happened against the destination's stale keys.
- The 8-character password minimum was frontend-only; export_settings now
  enforces it too, since that's the actual boundary a weak password has
  to cross. The derived key and decrypted plaintext are wrapped in
  zeroize::Zeroizing (already in the tree via aes-gcm).

Added test coverage the review named as missing: format-version
ordering, the generic-error-message guarantee, non_blank's blank-vs-
absent handling, and the new web-terminal preview/warning behavior on
both sides of the IPC boundary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 12:16:43 -07:00
shadow-testandClaude Sonnet 5 722d9aeff1 Add password-encrypted settings export/import
Secret Scan / scan (push) Successful in 8s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 9s
Build App (Preview) / create-release (pull_request) Successful in 5s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 6m29s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Closes #35. Exports the host environment — global AppSettings (already
the non-secret shape persisted to settings.json) plus the global secrets
that live in the OS keychain instead (the shared Claude Code OAuth login,
the model gateway's provider API key and master key) — to one
password-encrypted file, and restores it on another machine.
Per-project settings, per-project secrets, and Docker volumes are
deliberately out of scope; this is not a project backup.

Designed with the user in issue #35's comments: global settings only, no
docker volumes, the password is the lock/key, and the export is portable
as one file.

Crypto (storage/settings_crypto.rs): Argon2id derives a 256-bit key from
the password (memory-hard, meaningfully resistant to GPU/ASIC
brute-forcing in a way PBKDF2 at any reasonable iteration count is not),
AES-256-GCM does the actual encryption. A wrong password fails GCM's
authentication tag rather than producing silent garbage. Salt and nonce
are random per export and stored in the clear in the file header — their
job is uniqueness, not secrecy.

The save/open dialogs are opened from Rust, matching the boundary
file_commands.rs's pick_save_path/pick_files_to_upload already establish:
a frontend-driven dialog handing Rust a host path is the exact shape of
bug that produced this app's past criticals. preview_settings_import
resolves the chosen import path itself and remembers it
(AppState::pending_settings_import) so apply_settings_import re-reads the
same file without a path crossing back over IPC. The password is
re-entered rather than cached between preview and apply, so nothing here
holds decrypted plaintext in memory for longer than one command's
execution; the preview returned to the frontend carries counts and
presence flags only, never a secret value.

Import replaces settings wholesale (an import is "restore this
environment"), but only writes secrets actually present in the file — an
absent secret means "the source machine never had this configured," not
"delete this on import."

Added storage::secure::store_gateway_master_key and get_gateway_master_key
(read-only, unlike get_or_create_gateway_master_key which mints one as a
side effect) since neither existed and import needs to restore an exact
captured value rather than mint a new random one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 11:57:16 -07:00
shadow-testandClaude Sonnet 5 61bdbc4a5b Close the crash-window gap and exec-session leak a third review found
Secret Scan / scan (push) Successful in 16s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 3s
Build App (Preview) / build-macos (pull_request) Successful in 2m37s
Build App (Preview) / build-windows (pull_request) Successful in 4m52s
Build App (Preview) / build-linux (pull_request) Successful in 6m17s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
A third Opus review pass confirmed round 2's fixes hold up, then found:

- The pending-cleanup record `remove_project` writes is fully durable
  (fsync'd); the projects_store.remove() that follows it is a plain
  fs::write with no fsync. A crash or power loss in that window — or that
  store write failing outright, beyond what the previous round's in-process
  rollback catches — leaves a record on disk naming a project
  projects.json still lists as present. The very next startup retry would
  then delete that project's container, snapshot image, and both volumes
  (including the one holding the OAuth credential and every session
  transcript) out from under a project the user still sees in the sidebar.
  retry_pending_cleanup_logged now takes the ProjectsStore and refuses to
  touch — clearing instead — any record whose project id still exists.
  Also stopped swallowing the round-2 rollback's own failure.
- Resolving the container through find_existing_container instead of
  project.container_id (round 2's stale-id fix) changed what drove
  close_sessions_for_container in remove_project and rebuild_project_
  container: sessions are now leaked when Docker is unreachable (nothing
  resolves, so nothing closes, and the project record is gone a moment
  later) and in the stale-id race itself (sessions were opened against the
  container that actually exists, not the id find_existing_container
  bypasses). Both functions now close sessions for the stored id
  unconditionally, and again for the resolved id if it differs.
- A pronoun-agreement bug in the no-retry removal toast ("remove them
  manually" for a single leftover) that was fixed one line above for verb
  agreement but not for the pronoun.

Also closed the test gaps the review named: the pending-cleanup
corrupt-record aside-move had no test, the Reset toast's leftover copy
was inline and untested (extracted to lib/resetOutcome.ts, mirroring
components/projects/home/removalReport.ts, with unit tests), and nothing
asserted rebuild()'s success path maps outcome.project into the list
rather than the whole outcome.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:47:45 -07:00
shadow-testandClaude Sonnet 5 439ef16f07 Fix two new bugs a second review found: stale container id, orphaned record
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m40s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 6m28s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A second Opus review of commit 2 found it had introduced real problems of
its own rather than just polish gaps:

- remove_project's "None or stale" container-id fallback only handled
  None. A stale id (the documented start-failure race in
  start_project_container_locked, where the old container is removed and
  the new one's id isn't persisted until after start_container succeeds)
  still 404'd on removal — now treated as success by commit 1's own fix —
  while the real container survived to block every volume removal with a
  409 forever, with nothing in the pending-cleanup record ever naming it.
  Both remove_project and rebuild_project_container now resolve the
  container via find_existing_container() unconditionally, matching every
  other container-destroying path in the codebase, and remove_project
  fails closed (records a leftover rather than silently skipping) if
  Docker itself can't be reached to check.
- remove_project could leave a pending-cleanup record for a project still
  live in projects.json: if the store's own save failed after the record
  was written, startup housekeeping would delete that project's container
  and volumes out from under it on the next launch. The record is now
  rolled back when the store write fails.
- rebuild_project_container (Reset) only surfaced a leftover volume, not a
  leftover snapshot image — the more serious failure, since the next
  container is built from that image whenever it exists, silently
  reviving the exact system layer Reset was asked to discard.
  ProjectResetOutcome now carries leftover_image too, and the toast's
  "run docker volume rm" advice is corrected: the new container has
  already remounted the volume by the time the toast renders, so that
  command would just hit the same conflict Reset did.

Also from the same pass: reworded a couple of log/toast lines that still
asserted resources were "still present" when the daemon-unreachable case
covered by the same code path can't actually confirm that; fixed a
singular/verb mismatch in the leftover toast text; moved an unparseable
pending-cleanup record aside instead of re-warning about it forever; and
added a debug log when a record's recorded_at can't be parsed, so aging
never silently no-ops.

Pulled describeLeftovers/leftoverVerb out of ProjectHome.tsx into their
own module with unit tests, and added tests for the recorded_at staleness
check — the previous commit's equivalent logic had none.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:09:46 -07:00
shadow-testandClaude Sonnet 5 d8bb5ab262 Address review findings: durability, stale container ids, honest toasts
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 6m18s
Build App (Preview) / build-linux (pull_request) Successful in 7m48s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
An Opus review of the previous commit found several real gaps:

- pending_cleanup::save used plain write-temp-then-rename, unlike
  migration_store's fsync'd write it claimed to mirror — a crash in that
  window left a truncated record that list() would skip forever, silently
  reproducing the exact bug this module exists to fix. Now matches
  migration_store's File::create/write_all/sync_all/rename/sync_dir shape,
  and the tests exercise the real save/list/clear functions against a temp
  dir instead of re-implementing their bodies inline.
- remove_project and rebuild_project_container only ever looked at
  project.container_id, unlike every other container-destroying path in the
  codebase, which falls back to find_existing_container for exactly this
  race (a crash between creating a container and persisting its id). A miss
  here left a container that then blocked every subsequent volume removal
  with a 409, forever. Both now resolve the same way the rest of the
  codebase does, and record the container by its deterministic name rather
  than its id so a retry still has something that resolves.
- remove_project's toast promised an automatic retry unconditionally, even
  when writing the pending-cleanup record itself failed (the one case
  where nothing will actually retry). ProjectRemovalReport now carries
  retry_scheduled, and the UI is honest about which case it's in.
- remove_volumes_by_name now retries once after a short delay on a 409,
  since Docker releasing a volume's mount reference right after its
  container is removed is not always instantaneous, and this is exactly
  the sequence remove_project runs.
- rebuild_project_container (Reset) returns ProjectResetOutcome so the UI
  can warn when Reset could not fully clear a project's volumes, instead
  of only logging it — the new container silently reuses old data
  otherwise, which is what Reset promises not to do.
- retry_pending_cleanup_logged escalates a record's log level after it has
  failed for a week, since recorded_at was otherwise write-only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:36:27 -07:00
shadow-testandClaude Sonnet 5 4827170715 Report and retry Docker resources remove_project could not delete
Secret Scan / scan (push) Successful in 10s
Build App (Preview) / compute-version (pull_request) Successful in 7s
Secret Scan / scan (pull_request) Successful in 8s
Build App (Preview) / create-release (pull_request) Successful in 5s
Build App (Preview) / build-linux (pull_request) Successful in 6m5s
Build App (Preview) / build-macos (pull_request) Successful in 2m45s
Build App (Preview) / build-windows (pull_request) Successful in 5m42s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
remove_project_volumes always returned Ok(()) regardless of what actually
happened, making the `if let Err(e)` guarding it at every call site dead
code. remove_project then dropped the project record unconditionally, so a
volume, image or container that failed to delete became permanently
unreachable — confirmed against a real orphaned volume pair found in the
wild (fixes #31).

remove_project_volumes/remove_snapshot_image/remove_container now report
what they could not remove (treating "already gone" as success rather than
a leftover), remove_project surfaces this to the user via a toast, and
before dropping the project record it writes a pending-cleanup record that
startup housekeeping retries automatically on the next launch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:18:41 -07:00
shadow-testandClaude Opus 5 eead748222 Close what two reviews found in the Files tab transfers
Build App (Preview) / compute-version (pull_request) Successful in 5s
Build Container / build-container (pull_request) Successful in 37s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m40s
Build App (Preview) / build-windows (pull_request) Successful in 4m56s
Build App (Preview) / build-linux (pull_request) Successful in 5m11s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Two independent reviews of 2c9482a, one for correctness and one against the
threat model. Between them they found two ways to lose a file, one way for a
container to choose a Windows save destination, and a rule that made every
dotfile unsavable. Every finding below was demonstrated against a real
container before being fixed, and the fixes are demonstrated the same way.

## The download was accepting truncated reads and refusing whole ones

The `[ -f ]` bracket around the read was wrong in both directions.

It missed the case that loses data. Truncation *in place* — `> file`, log
rotation, `tar -x`, most build tools — leaves a regular file behind, so `dd`
stopped at the new EOF and exited 0. Measured: a 600 MB source truncated
mid-read delivered 34 MB, which was then renamed over the user's own earlier
copy and toasted as `Saved (34.1 MB)`. Truncate-to-zero did the same and needs
no adversary at all.

And it failed *good* downloads. `dd` already holds the fd, and neither `rm` nor
`mv` can touch an open one — the bytes are complete. But `rm` makes `[ -f ]`
false, so a finished 600 MB transfer of a file a bundler happened to unlink was
deleted, reporting "nothing was saved". The comment claiming the bracket caught
a "mix of two files" was simply wrong; an open fd cannot be a mix.

So the script now measures the file before reading it and puts the answer on
stderr, and Rust checks that at least that many bytes arrived. One rule,
subsuming everything the bracket was for. Verified against a real container:
truncation mid-read and the FIFO race are refused; deletion, rename-over, a
growing file, an empty file and an untouched 600 MB read all pass.

## On Windows the container, not the user, was naming the save destination

`suggested_save_name` split on `/` only, and it feeds the save dialog's
pre-filled name. Backslash is a legal Linux filename character and
`validate_container_path` has no reason to object — `..\..\Users\…` is one
POSIX segment. The Windows common file dialog parses its name box as a path on
Save, so a container-created file called
`..\..\..\Users\vic\AppData\Roaming\Microsoft\Word\STARTUP\x.dotm` put its own
bytes in an auto-loading Office directory on one un-read click. `resolve_host_path`
did not stop it: Word's `STARTUP` and Excel's `XLSTART` are not in the autorun
denylist, which this file's own docs already concede is "losing by
construction" and was never meant to be the boundary here.

The name is now sanitized of every separator, the drive colon and the rest of
what NTFS refuses, so it cannot be a path on any platform this ships to.

## No dotfile could be saved, and the app pre-filled the name that guaranteed it

The write policy judged the leaf for hiddenness, so `/workspace/.env` was
refused *after* the modal and the overwrite prompt — quoting the name the app
itself had suggested. `.gitignore`, `.dockerignore`, `.eslintrc.json`, `.nvmrc`:
all unsavable, while uploading them worked, so a dotfile could go in and never
come out.

`HostPathUse` gains a third mode. `WriteChosenName` drops the leaf check and
keeps every directory rule, and only `download_container_file` uses it — the
dialog is a real boundary for that caller and only that caller.
`download_container_backup` still takes its path over IPC as a string and keeps
the strict rule.

## Smaller, all found by the reviews

  * A download had no ceiling. `dd` resolves through the container's `PATH`,
    which its agent owns with passwordless sudo; a replacement writing forever
    was measured at ~6 GB/s, so one click on a file listed as 2 KB filled the
    host disk with no progress shown and no cancel. Bounded now by what the
    file measured, with slack that is absolute for small files and
    proportional for large ones, so an honest growing log is unaffected.
  * "Framed rather than verbatim" did not stop container text reaching the
    toast headline: `readableRefusal` matches with `includes`, and it has to,
    because the app's own refusals carry those markers mid-sentence. Anchoring
    would break them. The fix is at the injection point — container text is
    clipped to one 200-character line with control characters stripped — plus a
    `max-h-40` on the toast message, which the `detail` block always had and
    this half did not. 8 KB of prose in a `z-[60]` card pushed its own dismiss
    button off-screen.
  * `savingPath` was a scalar while the design deliberately allows concurrent
    saves. Starting a second freed the first's row mid-transfer, and whichever
    finished first cleared both; dismissing the second dialog was enough. It is
    a `Set` now.
  * `setUploading(false)` fired when the command settled, not when the refresh
    finished, so a second click landed mid-relisting.
  * `upload_files_to_container` checked the container directory before the
    picker and then used the unresolved path. A modal has no time limit. It
    re-checks after, which is what `docker::exec`'s doc comment already claimed.
  * `wait_for_exec_exit` flattened a missing exit code to `Some(0)`, which made
    the new `!= Some(0)` check unreachable by construction.
  * `normalize_host_path` did not collapse repeated separators, so the lexical
    system-root rule was silently absent for `C:\\Windows\…`. Not exploitable —
    the resolved pass catches it — but a documented layer that does nothing is
    a trap for the next caller.
  * README still carried the "no host path crosses IPC in either direction"
    claim the previous commit narrowed everywhere else, and HOW-TO-USE
    described the hidden rule without saying it applies to folders only.

480 Rust tests, 602 frontend, no new clippy warnings. Eleven mutations against
the new tests, all killed — two of the first round survived and were rewritten:
one because `split_whitespace` already handled the case I thought I was
testing, one because I had deleted a comment rather than the behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 10:42:26 -07:00
shadow-testandClaude Opus 5 2c9482a67d Give the Files tab back its uploads and downloads
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build Container / build-container (pull_request) Successful in 1m35s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 5m51s
Build App (Preview) / build-linux (pull_request) Successful in 6m50s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
`upload_file_to_container` and `download_container_file` existed on main before
any of this work started. "Ship the Files tab container-side only" removed them
and called it narrowing scope; from a user's side it was a regression they
upgraded into. This restores the feature.

The reason for the removal was real — four consecutive audits found their
criticals in host paths crossing IPC — so the feature comes back only in the
shape that removes the class rather than patching it a fifth time. The dialogs
are opened by **Rust** (`pick_save_path`, `pick_files_to_upload`), not by the
webview. A frontend `open()`/`save()` handing the backend a path string is
exactly what failed, and the backend cannot tell such a string from one a
compromised webview invented. Now the webview can ask for a picker and that is
the whole of its influence: it cannot name a host path as an input. That is the
shape the previous round's own notes named as the honest one if this ever
returned.

None of the machinery the audits condemned returns. No `link(2)` destination
reservation, no placeholder rollback, no collision marker: the OS save dialog
already asks about overwriting and Docker's extractor overwrites on upload the
way `cp` does, so there was nothing left for it to do. Download reuses the
sequence `download_container_backup` has been using unchanged — resolve, stream
into a partial file beside the destination, rename last — so a failed transfer
never touches the file that was already there. Upload reuses the terminal
drop's hardened uploader, with the container's uid/gid resolved once per
selection rather than once per file.

Against a container that is actively hostile rather than merely surprising:

  * the read is `dd iflag=nonblock`, not `cat`. `[ -f ]` and the `open` after it
    are two syscalls and the container owns the filesystem in between; a loop
    swapping the file for a FIFO wins that race, and `cat` then blocks forever
    with no writer and no timeout anywhere on the path — the `invoke` never
    settles and a partial is left in the user's directory for good. Verified in
    a real container that `cat` hangs, that `iflag=nonblock` returns, and that
    it is byte-identical on a regular file.
  * the read is bracketed by a second `[ -f ]`, because non-blocking turns that
    hang into an empty file that would otherwise be renamed over the
    destination and reported as a successful save.
  * an *undeterminable* exit code is a failure. Backup catches this class with
    its `total == 0` check, which download cannot have because an empty file is
    a legitimate save; without a replacement, a project restarted mid-download
    renames a truncated partial over the user's file and reports the byte count
    as if it were whole.
  * container stderr is capped. Every other reader of container output in the
    tree is capped for this reason; the two streaming commands were the
    exception, and stdout was bounded by disk while stderr was bounded by
    nothing.
  * the script's refusals are framed rather than used verbatim, so a directory
    named to look like one of our own sentences cannot become the toast
    headline through `readableRefusal`.
  * the partial name is capped at NAME_MAX. A bundler's 230-character content
    hash is a name that fits its directory and produces a partial name that
    does not.

Also: a non-UTF-8 dialog path is refused by name rather than silently mangled
into a different path by U+FFFD substitution; both actions carry in-flight
state, so a second click cannot open a second dialog and a slow save is not
indistinguishable from a dead button; and the upload's completion message names
the directory, since the picker is modal and the user can browse elsewhere
while it is open.

Not restored: drag-and-drop, in either direction. `drag:allow-start-drag` stays
ungranted and `hold/disk-and-dragout` still holds that work.

Two bugs the new tests caught while being written: a double-click on "Save to
host…" opened the file viewer on top of the save dialog, and an N-file upload
made N redundant execs to re-ask `id -u`.

Docs that asserted this feature did not and must not exist are corrected —
CLAUDE.md, README, HOW-TO-USE, TECHNICAL and the capability threat model. The
"no host path crosses IPC" claim is deliberately narrowed to the inbound
direction: paths do still travel outward inside error text, canonical ones
included, and the reviewed record should not overstate.

600 frontend tests, 473 Rust, no new clippy warnings. Every new test was
mutation-checked; four that survived their first mutation were rewritten,
including two whose mutations turned out to be unfaithful and one that was
blind to a dismissal leaving a row stuck on "Saving…".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 10:00:21 -07:00
shadow-testandClaude Opus 5 88ffb4744a Cancel the keydown on Shift+Enter, or xterm submits anyway
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build Container / build-container (pull_request) Successful in 36s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m56s
Build App (Preview) / build-linux (pull_request) Successful in 5m8s
Build App (Preview) / build-windows (pull_request) Successful in 5m38s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The handler returned `false` from xterm's custom key handler and a
comment claimed that was enough to stop the bare CR. It is not.
`_keyDown` returns the instant the handler says `false` — before it sets
`_keyDownHandled` and before it cancels the event — and `_keyPress` then
checks that same flag, finds it false, and emits a bare CR for Enter's
charCode 13.

So the headline feature of this branch did the wrong thing in a real
browser: Shift+Enter inserted the newline and then submitted the
half-written prompt, now with a stray blank line in it. Arguably worse
than before the fix. Reproduced in Chromium and confirmed against the
bundled xterm 5.5.0 source.

`preventDefault()` is what stops the browser firing keypress at all.
Applied at both call sites — the desktop terminal and the web terminal's
copy.

The test could not have caught this. jsdom never synthesizes the
follow-up keypress, so `expect(sent()).not.toContain("\r")` was asserting
a property the environment cannot falsify — a test named for a behaviour
it could not exercise. It now asserts `defaultPrevented`, which is the
mechanism that actually suppresses the keypress and which jsdom can
observe. Mutation-checked: removing the `preventDefault()` fails it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 20:47:01 -07:00
shadow-testandClaude Opus 5 016de8f641 Close the blockers from the fifth audit
Build App (Preview) / compute-version (pull_request) Successful in 3s
Build Container / build-container (pull_request) Successful in 10m5s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 4m31s
Build App (Preview) / build-linux (pull_request) Successful in 5m21s
Build App (Preview) / build-windows (pull_request) Successful in 19m1s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Docs and disclosure. HOW-TO-USE.md's settings table still described the
pre-fix behaviour — and help_commands.rs fetches that file from GitHub
main at runtime, ahead of the embedded copy, so it would have reached
every user's Help dialog the moment this merged. The Config tab named
three settings that need a base-image update; there are four, and the
omitted one (Session recap) is the one that fails *without* the "won't
switch off" symptom the warning teaches. Both now also state the cost
nobody had written down: changing any of these recreates the container,
which commits a layer.

Two stale comments that told a reviewer the code was safe when it was
not. compute_claude_code_settings_fingerprint still claimed the
historical fingerprint is preserved so an upgrade cannot churn every
container — carried over from before the widening, false since the
format string changed. And capabilities/default.json, which is the
reviewed threat model of record, described a "Save to host…" action this
branch deletes.

Security and correctness. update_settings validated env vars and nothing
else, so the *global* default_ssh_key_path — the fallback for every
project without an override — took `/` and read-only bind-mounted the
host, which entrypoint.sh then copies into the home volume. classify_
mount_source ran canonicalize on the raw string, which resolves a
relative path against Triple-C's own cwd, so `.` and `..` were accepted
or refused depending on where the app was launched; the daemon then
refuses the mount and the project can never start. Its test passed only
because its examples did not exist under app/src-tauri.

bind_mount_exclusions still derived a path from every row while
project_path_mounts had learned to skip unmountable ones, so a legacy
row made /workspace/<name> ordinary container content that a migration
would then exclude from staging and destroy. The skip is also logged now
rather than silently dropping a folder.

The terminal's file-in path checked is_dir() but not file type, so a
dropped FIFO blocked forever with no timeout — and it is the only route
in now. The web terminal labelled sessions from a global set at request
time, so two quick opens swapped them; harmless until Shift+Enter became
type-dependent, at which point a mislabelled Claude session submitted a
half-written prompt. Opened now carries the type.

Every ~/.claude.json write goes through one atomic helper. The
awsAuthRefresh branches still truncated in place — the same corruption
the Shift+Enter block was fixed for twenty lines later, and its own
comment said so. Demonstrated: a failed write now leaves the original
byte-identical.

And the registration test I added yesterday could pass while the
property was false: an audit got five real unregistered commands past its
exact-string attribute match, and "exactly once" was in its name but not
its body. Mutation-checked against all six shapes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 18:45:11 -07:00
shadow-testandClaude Opus 5 4d1a5a2417 Remove list_sibling_containers, which nothing called
It listed every container on the daemon — including the user's unrelated
postgres, mysql and other work — and handed the summaries to the webview.
It had a registration, a command, a docker-layer helper, a typed frontend
wrapper and a `SiblingContainer` type, and zero call sites.

An audit named it as step one of an escalation chain: enumerate the
daemon's containers, then point `update_project`'s unvalidated
`container_id` at one and read its files through the file-command
surface. The second half of that chain is closed now, but a command that
exposes the user's unrelated containers and serves no feature is surface
with no upside.

Found by the registration test added in the previous commit, which is the
answer to "why test something the compiler already checks": the compiler
is perfectly happy with a command nobody calls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:15:38 -07:00
shadow-test e9902f0564 Merge branch 'r4/narrow' into ship/core 2026-08-23 17:08:04 -07:00
shadow-test 7488fc5b70 Merge branch 'r4/host' into ship/core 2026-08-23 17:08:04 -07:00
shadow-testandClaude Opus 5 06ccb4d818 Ship the Files tab container-side only
Four successive audits found the same thing: host filesystem paths crossing
IPC is where the criticals in this work live. The most recent one found the
`link(2)` upload reservation returning success against a *directory* (linking
into it, leaving permanent stray files, and via a symlink-to-directory writing
outside the validated write root), failing every upload permanently on any
filesystem without hard links, and the post-resolution credential check
weakened from a general rule to an eleven-name denylist.

Rather than fix that a fifth time, the Files tab ships as what it is good at:
a browser, viewer and renamer that never touches the host.

Removed: `upload_file_to_container`, `download_container_file`, and everything
that existed only for them — the whole reservation (`UPLOAD_RESERVATION_SCRIPT`,
`reserve_upload_destination`, the placeholder rollback, `exec_oneshot_as_within`
which had no other caller), `stream_container_file_to_host`, `ChannelReader`,
`save_to_host`, the download ceiling, and the collision marker with its
frontend contract. On the frontend: the upload button, the pane's
`onDragDropEvent` handler, both "Save to host…" affordances, `uploadPaths` /
`downloadFile` / the overwrite prompt, and `OverwriteConfirmModal`.
`lib/uploadErrors.ts` is now `lib/refusalText.ts` and keeps only the half that
turns any backend refusal into the sentence a person reads.

Kept, and not weakened: `upload_host_file_to_terminal` and
`download_container_backup`. They predate this work, their hardening is a real
improvement over main, and they are now the whole answer to "how do I get a
file in or out" — drop it on the Terminal, or Back up container. The drop gate
(`lib/dropTarget.ts`, `PaneVisibility`) is untouched.

`resolve_host_path` gets the general hidden-component rule back. Round 3
replaced it with `HOST_CREDENTIAL_DIRS`, which is allow-by-omission for the
rest of `$HOME`: `~/.local/bin` (write there and you own the user's next shell
command), `~/.password-store`, browser profiles and `~/.pki/nssdb` were all
reachable through a planted symlink with a visible name — verified against a
real home directory, and all five refused now. It over-catches `.pnpm` and
`~/.cache`; for two occasional callers that is the cheaper mistake, and the
refusal says which folder it resolved through.

Two defects fixed while in here:

  * A symlinked directory listed as empty. `find` defaults to `-P`, which does
    not follow a symlink even as the starting point, so `-mindepth 1` discarded
    the only match and a real directory rendered as "Empty directory" — a
    first-order defect now that browsing *is* the feature. `-H` follows the
    starting point and nothing else, so a loop is `ELOOP` rather than a walk
    that does not end; verified against a live container for a symlinked
    directory, a broken link and a loop. `find`'s errno for the loop case is
    now a sentence.
  * `finish_download`'s replace path fired on *any* rename failure with a
    destination present — a vanished partial, a permission error, a directory
    at the destination — and deleted the user's file to complete a move that
    could not complete. It is now fenced to Windows (where a rename onto an
    existing path genuinely fails) and to a partial that still exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:05:56 -07:00