Adopt existing tmux sessions and containers, plus remote/terminal/input fixes - #375
Open
dignfei wants to merge 28 commits into
Open
Adopt existing tmux sessions and containers, plus remote/terminal/input fixes#375dignfei wants to merge 28 commits into
dignfei wants to merge 28 commits into
Conversation
Docker cases could only run in a container Codeman created itself. Attaching to one the user already built and runs means Codeman must leave that container's lifecycle completely alone, which the launch chain could not do: it was `image inspect` -> `inspect || create` -> `start` -> `exec`. Adds `DockerCase.owned`, mirroring the `owned:false` contract remote-SSH already uses for attached sessions. Absent (every existing case) means owned, so current behaviour is byte-identical. `false` means the container belongs to the user and Codeman may only exec into it. The launch chain for an attached container only looks, then execs: no image gate (the image is theirs), no create, and no `start` — starting a container we do not own is the very mutation attaching promises not to perform. A missing or stopped container fails closed with an actionable message instead. Credential seeding is skipped too: those copies read from create-time read-only mounts that do not exist here, and writing host credentials into someone's container is not ours to do, so its CLIs must already be authenticated inside it. Four fail-closed guards. buildDockerStopCommand and buildDockerRemoveCommand throw during pure string construction, so no caller bug can turn into a `docker stop`/`rm` on a container we do not own; removeDockerContainer refuses again at the lowest layer; drift reports "none" for an attached container, which carries no `codeman.confighash` label and would otherwise always look drifted and 409 the launch gate forever; and the orphan reaper skips attached containers through a check deliberately independent of the two conditions already covering them. `owned` is applied AFTER the config hash is computed. dockerConfigHash takes an explicit field list, so ownership can never shift an existing case's hash — if it did, every pre-existing case would trip the drift gate at once, and the remedy the UI offers is "recreate the container". Adds POST /api/cases/docker-adopt and a read-only POST /api/docker-cases/adopt-preflight. The preflight refuses at LINK time rather than at session launch, where the only ways out would be a dead pane or starting a container we do not own. Tests assert the negative guarantee directly — that create, start, stop, rm, restart and kill are absent from the generated commands while `docker exec -it` and `new-session -A` remain — since it cannot be observed by using the feature.
The Docker tab gains an "Attach to an existing container" toggle. Ticking it swaps the create-time fields (image, network, advanced) — which describe a `docker create` attaching never runs — for the container name, and routes the submit to the adopt endpoint. Reuses the existing linkDockerCase flow end to end: only the final call differs. The docker-host upsert still applies, since it is what resolves the engine/context/daemon for `docker exec`; its create-time fields are simply never read for an attached case.
Two defects that only a real container exposes. The probe chained `command -v X && echo X` with semicolons, and a script's exit status is its last command's. A container without the last probed CLI made the whole `sh -lc` exit 1, so a perfectly healthy container with tmux and claude was reported as "could not exec into the container". A missing CLI is data here, not failure, so the script now ends with `exit 0`. containerWorkdir defaulted to hostWorkspacePath. That default holds for an owned container only because the create-time bind mount puts the host directory at that exact path; attaching mounts nothing, so the two are independent facts. A host path absent inside the container makes `docker exec --workdir` fail with an OCI chdir error that surfaces in the pane as a bare "execvp failed". The preflight now proves the directory exists inside the container and refuses at link time.
Attaching lived only on the Docker tab, but the place users look for anything container-shaped is the "Run in an isolated Docker container" checkbox on Create New. A feature nobody can find is a feature nobody has. Adds a one-click link there that switches to the Docker tab, turns the toggle on and focuses the container field. Reuses switchCaseModalTab and the existing sync helper; no new CSS.
The new strings were English only. Adding entries surfaced a deeper problem: the
translator matches whole text nodes and skips `code`/`pre`, so an inline `<code>`
mid-sentence splits a hint into fragments that can never match an entry — which is
why the panel's existing "Build it once with <code>...</code>" hint was never
translated either.
Drops the inline markup from the new hints so each is a single text node, then
adds the zh-CN entries. The brand name goes through the existing {name}
placeholder.
Server-side error bodies are deliberately not added: the client receives them
already interpolated with a concrete container name, so a template key could
never match.Typing a container name from memory is error-prone. The field becomes a native datalist: pick from the engine's containers, type to filter, or type a name that is not listed (the engine may be remote, or the container may not exist yet). A datalist gives all three natively, so no dropdown state machine is introduced. Adds listDockerContainers and GET /api/docker-hosts/:hostId/containers, following the listRemoteCodemanSessions discovery precedent: read-only and never throwing, so an unreachable daemon returns an empty list and the field degrades to plain text instead of erroring. Stopped containers stay in the list, sorted after running ones and labelled. Attaching does require a running container, but hiding stopped ones turns "my container is not in the list" into a dead end, while showing `Exited (137) 8 days ago` says exactly what to fix.
The run-mode dropdown hides CLIs that are not installed on the HOST (Ark0N#201). That is right for local sessions and wrong for a container case, whose agents run inside the container: a host with no claude installed hides the mode while the container ships one, which is exactly what happened on a real deployment. The adoption preflight already probes what the container has, so that result is persisted on the case and surfaced through CaseInfo. Docker cases gate on it; every other case keeps the host probe unchanged. An absent list reads as "do not gate" rather than "nothing available": an owned container runs our base image, which ships every CLI, and treating unknown as empty would leave the menu with Shell alone.
The adoption preflight used the mode name as the binary name. claude, codex, opencode, gemini and pi happen to match, so it never showed — but antigravity ships as `agy` and deepseek as `dsh`, so a container that has either was reported as not having it, and the mode was silently dropped from the case. Adds a MODE_BINARIES map, single-sourced with defaultDockerCommandForMode, which launches those same binaries. Probing and result filtering share one `binaryFor` so the two cannot drift apart.
…ch time Storing the container's CLIs on the case at attach time left two gaps: a case linked before that field existed has none at all, and a container's CLIs can be installed or removed long after it was linked. A real deployment hit the first one — the host had only codex, the container only claude, and with no stored list the menu still gated on the host and hid the mode that actually worked. The probe now runs when a container case is selected, reusing the existing adopt-preflight endpoint, so there is no new backend surface. Results are cached per case for the page's lifetime, since the menu opens often and the probe is a `docker exec` round trip; a concurrent probe for the same case is deduplicated with an in-flight marker. A failed probe leaves the cache empty, which the caller reads as "unknown" and therefore does not gate. Hiding every mode because one probe failed is worse than offering one that turns out to be missing, which the launch path already refuses with a specific message. The repaint only happens while the menu is still open, so a late answer cannot make the list jump under a user who already closed it.
Attaching a container, picking claude and hitting Run gave one line — `execvp(3) failed.: No such file or directory` — and the run-mode menu offered every mode. Three separate defects, found on a real deployment. TmuxManager.createSession resolved the CLI directory without distinguishing a docker session, so a host with no claude threw, the catch fell back to a direct PTY, and that PTY exec'd the CLI on the HOST. The failure surfaced as a bare execvp error naming nothing. A docker session runs its CLI inside the container; the host does not need it. All eight modes now sit behind a cliRunsInContainer guard, and whether the container has the CLI is settled by the adoption preflight or the image gate before launch. The running check used a bare double quote and command substitution. The whole chain is embedded in an outer `bash -c "…"`, so the unescaped quote closed that string early and the remainder was re-tokenized. It is now a `grep -qx` pipeline using only the single-quote form every other line in the builder already uses. Claude Code refuses --dangerously-skip-permissions as root. Our base image runs a non-root user, so an owned container never hit this; an adopted container's user belongs to its owner and is frequently root, and keeping the flag killed the pane with a message visible only inside the container. The preflight now reports runsAsRoot and the launch chain drops the flag for it. The menu also showed every mode because the container CLI probe only started when the menu opened. It is warmed when the case is selected instead.
Link Existing's Browse did nothing: GET /api/filesystem/browse answered 403 "No filesystem browse roots are available". Two rules were fighting. /root is a default blocked tree in the attachment guard, and Codeman running as root — containers, plenty of servers — makes homedir() exactly /root, so the picker's own allowlisted Home root was blocked; the other candidates live under it or do not exist. The root list came out empty and there was nothing the user could open. The blocked trees exist to keep ~/.ssh and friends out of reach, not to seal off the user's own home. Only trees that would swallow a configured root whole are dropped now: /root goes when Home is it (or sits inside it), /etc holds no configured root and is untouched. Secrets stay protected — isSensitivePath independently matches .ssh/, .env and credentials* at any depth, and it is what the directory probe asks about.⚠️ Navigation must reuse the same narrowed list the roots were chosen with. Handing the raw trees downstream admits a root and then refuses every path inside it, which reads as a picker that opens and does nothing.
Both paths in the adoption form had to be typed. Each gets a Browse button using the same path-input-group markup Link Existing uses, so the two look and behave alike. What they can browse differs, and that is the point. The host workspace path reuses the existing host picker. The container workdir cannot: an adopted container has nothing mounted at a matching host path, so a host listing would be a different filesystem — and getting this field wrong is the source of the opaque OCI chdir error at launch, which makes it the field that most needs to be clickable. Adds a read-only POST /api/docker-cases/browse: one `ls` through docker exec, no writes, no lifecycle, path shell-escaped like every other value. `ls -Ap` marks directories with a trailing slash and keeps names with spaces intact. PathPicker takes an optional fetchListing source rather than being forked: the container variant only swaps where the rows come from, and reuses the rendering, navigation, Up and Choose/Select unchanged.
…able container
The run menu still offered every mode for an attached container. The browser's
actual request showed why:
POST /api/docker-cases/adopt-preflight -> 400
{"error":"Invalid input: expected object, received string"}
_api serializes `body` and sets Content-Type itself, and three call sites each
passed an already-stringified body, so it was encoded twice and the server saw a
JSON string where it expects an object. curl was fine throughout, so nothing in
the server logs pointed at it.
Also fixes the design defect underneath: a failed probe fell through to "do not
gate", which silently offered every mode. When the container has been recreated,
is stopped, or the engine is unreachable, the user sees claude, clicks it, and
it can only fail — with the reason visible nowhere. A failed probe now hides
every agent mode (Shell needs no CLI and stays) and shows the server's own
reason at the top of the menu.
Two static guards switched from a character window to brace matching. They
sliced between two call sites, and _loadRunModeHistory's call appears above its
definition, so the slice came out empty and the assertion verified nothing —
the same trap twice in one file.The home screen now lists tmux sessions Codeman did not start (a claude or codex running inside `tmux new -s work`, or just a shell); one click turns one into a tab you can keep working in. Adoption is a fourth LOCATION OVERLAY, structurally identical to remote/docker, and NOT a new SessionMode: the outer layer is still an ordinary codeman-<8hex> wrapper session on this instance's own socket, and only the pane inside it runs the attach. Session-name allowlisting, capture, input and the recovery chain are therefore untouched, and "detach, never kill the foreign session" becomes structural rather than a rule to remember — killSession can only ever reach our own wrapper. All three locations share one probe script, one parser and one classifier, and differ only in the shell around them (direct exec / docker exec / ssh). Pane mode is decided from the bounded process-tree argv of pane_pid, because claude and codex both report `node` as pane_current_command; anything unrecognised is treated as a shell. Things measured rather than assumed: - A grouped session buys only `status off` and an independent current window, not an independent size. Measured on tmux 3.3a: both a bare attach and a grouped one shrink the other client's 200x49 to 80x23. Only `window-size largest` preserves it, but that is a shared window option that survives our departure, so it is not set. - View reclamation: local relies on client death, ssh on SIGHUP, but a `docker exec` does not die with its client — the container-side view must be reclaimed explicitly on kill or every adoption leaks one. - The session name is chosen by someone else, while the local launch chain ends in `bash -c` plus JSON.stringify, which does not escape `$` or backticks, so the outer shell performs substitution before the inner single quotes close. Session names and socket paths therefore pass a character allowlist and are discarded during DISCOVERY, so a non-conforming candidate never gets an id. Capability degrades by "who started this process": an adopted session has no hooks, no envOverrides and no effort, and its working directory is merely the foreign pane's cwd at that moment (possibly not even on this host). Respawn, Ralph, the orchestrator, hook waits, and every watcher that tails the local filesystem by workingDir are refused or skipped, and the close dialog no longer offers a "kill the session" option it cannot honour.
…nt dirs Once a container is adopted, it could not be adopted a second time. But a container usually holds more than one project directory, and opening a case for another one had no path forward except starting a second container — precisely what adoption exists to avoid. The original reason was in a comment: two cases sharing an adopted container would make one case's teardown race the other's launch on the same tmux server. That reason does not hold. The in-container tmux session name is dockerTmuxSessionName(sessionId), i.e. codeman-dkr-<id8>, keyed by SESSION and not by case, and buildDockerKillCommand tears down exactly that name, so killing A never touches B — hosting multiple sessions is what a tmux server is for. The other three routes into an adopted container's lifecycle do not pass through here either, confirmed one by one: the stop and remove builders throw outright; recreate refuses `owned === false` before it even resolves the container name; and orphan reaping filters on `label=codeman.managed=1`, which a user-built container does not carry — a structural exclusion. That leaves exactly three cases worth refusing, none of them tmux-related, split into the pure, unit-tested classifyAdoptContainerConflict: - owned-case the container belongs to a Codeman-created case, whose lifecycle Codeman manages: one recreate or delete there would pull the container out from under the adopting case.⚠️ `owned` may be absent and absent means owned (cases predate the field), so the test is `!== false`, not truthiness. - other-owner already adopted by a different user. Adoption hands out a shell inside someone else's container. - duplicate same container, same directory. The second case would behave identically to the first, so name the existing one rather than silently minting a twin. A different in-container directory is the case this change exists to support and passes.
… a selection xterm 6 reads `selectionBackground`; the pre-6 name was `selection`, and it is now silently ignored (the vendored xterm.min.js contains zero occurrences of that key — no alias, no fallback). All seven skins still set the old name, so no skin's tuned selection colour has ever taken effect; every one of them fell back to xterm's own default of rgba(255,255,255,0.3). The consequence split by skin, which is why it survived so long. On dark skins white at 30% happens to be a reasonable selection colour and nothing looks wrong. On light skins white over white composites to within 3/255 of the background — invisible. The drag really did select, the screen just never changed, which reads as "text here cannot be selected". The skins' own colour choices were fine, they were simply never read, so this only renames the keys and adds `selectionInactiveBackground` (alpha 55%) so a selection stays visible after the terminal loses focus. Minimum channel delta after the change is 38, maximum 47. The guard test composites each skin's selection colour over that skin's own background and requires a visible delta of >= 12. Asserting "the key exists" would not catch this — a bad colour passes that just as happily — and the absence of this test is why the bug survived.
…ction In a native terminal running a TUI with mouse tracking on (claude, codex), Shift is the "let me select text" modifier: it bypasses the application's mouse reporting so the emulator selects locally. Users bring that habit here, where it did nothing — measured, `hasSelection` was already false during a Shift+drag and no clearSelection call ran at all, because there was never a selection to clear. The mismatch is that the two Shifts mean different things. xterm reads Shift as "force selection", but that path is only taken when the application really has mouse tracking on. The server strips the mouse DECSETs for claude/codex/gemini (isAltScreenStripMode), so xterm's mouseTrackingMode is permanently `none`, that branch is unreachable, and Shift instead lands in _onIncrementalClick — which EXTENDS an existing selection. Extension is a no-op while selectionStart is empty, so the drag had no anchor. So plant the anchor xterm is missing. The listener sits on the capture phase of the `.xterm` root, an ancestor of the `.xterm-screen` that SelectionService binds to, and therefore runs before xterm's own mousedown; xterm then extends from our anchor and the drag behaves like any other. Length is 0 so a Shift+click without a drag does not select a stray character. An existing selection is left alone — that is a genuine extend gesture, and xterm handles it correctly. Right-click copies the selection (the mintty/PuTTY convention), completing the gesture: until now there was nowhere for a finished selection to go. With no selection the native menu is not hijacked — taking it away while offering nothing in return is a pure loss.
…erlay `backdrop-filter` promotes an element to its own compositing layer. A position:fixed full-screen layer that is created and then hidden was measured to leave a stale hit-test region behind in Chrome: the page renders perfectly, but pointer events across the viewport go nowhere. The report came from a long-lived tab connected to a remote server, where a connection blip shows and then hides #offlineOverlay. The symptoms were a terminal that would not scroll and, at the same time, an unrelated click-to-expand that also stopped responding, while a freshly opened tab was fine; a read-only console command (getComputedStyle + elementFromPoint, both of which force a hit-test recomputation) then cured it. Two unrelated features dying together and one read-only command fixing both points at hit-testing itself rather than at either feature. So the `backdrop-filter` moves onto the actually-visible selector and the layer is never created while hidden. Only the two persistent overlays change: offline-overlay (toggled with [hidden]) and file-preview-overlay (toggled with .visible). path-picker and path-preview are created and removed by JS, leave nothing behind, and are untouched.⚠️ This is an evidence-based inference, not a fix verified by reproduction: reproducing it needs a long-lived page that has been through a connection blip, which I could not manufacture in a controlled environment. The guard test pins both halves — no such property while hidden, and a real blur while shown — so a later cleanup cannot quietly delete the effect.
claude advertises "Jump to bottom (ctrl+End)", so that chord has to actually reach it. But PASSTHROUGH_KEYS carried only the bare forms (End -> \x1b[F) and CTRL_KEYS held just six letters (c/d/l/z/a/e), which cannot express End. Ctrl+End therefore failed in both directions: - with an empty composer it went out as a bare \x1b[F, the modifier silently dropped, so the CLI received a plain End; - with text in the composer the forwarding branch requires empty, so nothing was forwarded and the browser default applied — the caret jumped to the end of the draft, which is the "the shortcut now edits my input box" the user saw. Encode them as CSI 1;<mod><final> instead, and forward Ctrl/Alt-modified navigation keys whether or not the composer is empty: they are commands for the CLI, and the composer has no editing semantics for them worth preserving (bare Home/End still use the old table and edit locally).⚠️ Bare Shift is deliberately excluded: Shift+arrow selects text in the composer, a real editing gesture that must stay local. Shift held together with Ctrl/Alt is still encoded into the modifier mask.
…assword optionally The remote path always passed `-o BatchMode=yes`, which disables every interactive prompt, so password-only hosts could never be used. BatchMode is not an oversight: Codeman launches ssh non-interactively from a service process with no terminal and nobody watching, and without it ssh hangs on a password prompt no one will ever answer — a dead pane, which is worse than an error. So the password goes through sshpass, handed to ssh in the SSHPASS environment variable: never in argv (same-host users can read /proc) and never in a temp file. The variable itself is injected into the pane with socket-scoped `tmux setenv`, the same rule every other secret here follows.⚠️ One thing measured, and a naive implementation will hit it: BatchMode=yes and sshpass are mutually exclusive. The former disables the password prompt, and answering that prompt is exactly how sshpass works, so using both yields `Permission denied (publickey,password)` — which reads like a wrong password rather than wrong arguments. With a password we therefore send BatchMode=no plus NumberOfPasswordPrompts=1, the latter so a wrong password fails immediately instead of hanging (also measured).⚠️ The preflight probe runs in the server process, not in the pane, so `tmux setenv` does not reach it and that path passes the variable through the child environment instead. A missing sshpass is reported as a named prerequisite during the probe as well; otherwise it surfaces as a pane dying with "sshpass: command not found", which reads like a broken host. Storage and exposure: - remote-hosts.json now holds a secret, so it is written 0600, and an existing file is explicitly tightened once (writeFile's mode only applies on create) - the API always redacts, returning only a `passwordSet` boolean - updates merge the stored password, because a redacted host posted back carries no `password` and a straight write would silently erase it; an explicit empty string still means "clear" - the schema deliberately does not apply NO_SHELL_META to `password`: a password legitimately contains `$` and backticks, and unlike the path fields it is never interpolated into a shell string
The backend could already authenticate with a password, but only the API could supply the field — the UI had no way in. It goes in "Advanced SSH" next to Identity File: the two are answers to the same question, and seeing them together is what makes the choice obvious. The hint says outright to prefer a key when one exists. Three details, each of which breaks something if skipped: - the field is type="password" and is never populated from the server. GET is redacted, so any code writing a value into this box can only be writing a placeholder — and the next save would store that placeholder as the real password. - the value is deliberately not trimmed: leading or trailing spaces may be part of the password. - it is added to the remoteFields clearing list. Without that, a typed password persists across forms and the next new host silently inherits it — credentials from two different machines bleeding together. Both submit paths are wired: the standalone "add remote host", and the one that creates a host as part of the remote-case flow. Wiring only one leaves the other silently key-only. Guard tests pin each of the above (including "must be both paths" and "must never repopulate").
…rmark
Browser input is delivered exactly once by (clientId, seq). The server records a
watermark per clientId and discards anything not above it as a duplicate — but
acknowledged it with an ACK indistinguishable from "applied". The client then
dropped the record from its queue, the UI looked perfectly normal, and the
terminal received nothing at all.
The counter is persisted to localStorage through a debounced write. Kill the page
between "sent" and "persisted" and the restored counter is below the server's
watermark, after which every keystroke lands under it, is discarded, and is
ACKed. Reloading does not help: the clientId is restored from localStorage
alongside that stale counter. Measured on a real session — typing into the same
session from a fresh browser (new clientId, no watermark on the server) worked
perfectly, which is what localised the fault to client state.
Three changes:
- on rejection the server replies {"t":"ia",seq,"dup":true,"last":<watermark>}.
It still ACKs, so the client can drop the record from its queue, but it now
says the input was not applied and supplies the number needed to climb out.
- on `dup` the client lifts its counter above the watermark and re-queues.
⚠️ Only records whose FIRST delivery is being retried are re-sent: a retry
judged duplicate means the mechanism is working (the original did arrive), and
re-sending would type the same text twice.
- the counter is now persisted synchronously. The queue payload can stay
debounced, but the counter is the thing that has to survive a crash, and
leaving it on the lossiest path cancels the only guarantee there is.
⚠️ Reading the watermark is defensive: the session arrives through a structured
port, and a port missing that method must not take the whole input path down —
a throw inside the handler means the ACK is never sent and the record is stuck in
the client queue forever, which is worse than the ambiguity being fixed. A mock
port's test timeout is what exposed this.The backend already lets one adopted container back several cases pointing at different in-container directories, but using it meant retyping the container name, host and workspace one by one — exactly the friction that leaves a capability unused. Picking an existing case from a dropdown now carries those three over, leaving only the two fields that must differ: the case name and the in-container directory. Clearing those two is the point of the feature, not a convenience: keeping the old name is refused by the server as "case already exists", and keeping the old directory is refused as "a twin case on the same container and directory". Both errors are clear, but a form pre-filled with values that are guaranteed to be rejected is a trap. Focus lands on the in-container directory — the thing the user came here to change.⚠️ Only adopted containers are listed (docker.owned === false). A Codeman-built container's lifecycle belongs to its one case — a second case would be torn out by that case's recreate or delete — so the server refuses it anyway, and listing it here would only manufacture a baffling error. `owned` may be absent and absent means owned, so the test is `!== false`, not truthiness. CaseInfo.docker gains containerWorkdir and owned for this: the former is the "which directory does this case use" half of the picker, without which the user cannot tell what to change it to; the latter backs the filter above.⚠️ Both places that build a docker CaseInfo (the list endpoint and the single-case query) must set them — filling in only one makes the picker work or not depending on which read path was taken, and a test pins "exactly two".
The previous version cleared the case name and the in-container directory on the grounds that they must differ. That left a form with three fields mysteriously filled and two empty, and turned the most common operation — changing /srv/app/api to /srv/app/web — into retyping a long path. Both are now pre-filled, with focus on the in-container directory and the caret at the end, since the tail is what changes. What stops an unmodified submit is no longer an empty field but a guard: the values applied are recorded, compared at submit time, and if nothing changed the reason is stated next to the field and focus moves to it, without sending a request that is certain to be refused. The server refuses these anyway (a duplicate case name, a twin case on the same container and directory) and its errors are clear; but making a round trip to be told "you forgot to edit the field you are looking at" is worse than saying so on the spot. The guard only applies when a source case was actually selected, so filling the adopt form from scratch is unaffected.⚠️ The status text is written into dockerLinkStatus. My first version referenced an id that does not exist (dockerAdoptStatus), which made the explanation vanish silently and left only a toast. The test now extracts that id from the code and looks it up in index.html, pinning that it must really exist.
… the same session handleInit() did not distinguish a first load from an SSE reconnect: it always cleared the terminal caches in _resetAllAppState() and re-ran selectSession() for the session that was already on screen. Every reconnect therefore refetched up to 1 MiB of buffer and reset+rewrote xterm. On a link that drops a connection about once a minute (measured at ~57s intervals against a healthy server) that reads as the page refreshing itself and throwing away your reading position. A reconnect that lands back on the still-open session now keeps the terminal caches and activeSessionId and resyncs through _onSessionNeedsRefresh(). That path still reloads the buffer, so output produced during the outage is not lost, but it preserves distance-from-bottom — the same rule Ark0N#259 established for a refresh the server triggered rather than the user. The WS is reconnected explicitly when it is not already on that session, since skipping selectSession() skips its _connectWs() call. First load (gen === 1) takes exactly the path it took before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rv24Pk4qzrsDYdVyDyJQmT
…anged renderForeignSessions() rebuilds the block with innerHTML, and the poll called it unconditionally every 8 seconds, so the block was destroyed and recreated once per cycle whether or not anything had changed — visible as a flicker, and it dropped any in-progress interaction with a row. Nothing in a row is time-varying, so an unchanged payload has nothing to repaint. Compare a signature of the payload and skip when it matches. The signature is cleared when the home screen is entered, so the first frame of every visit always paints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rv24Pk4qzrsDYdVyDyJQmT
Format check failed twice, on different files each time, because three prettier versions were in play: package.json says ^3.4.0, package-lock pins 3.8.3 (CI runs npm ci, so that is the one CI uses), and the local node_modules had 3.9.6. Files formatted with 3.9.6 were then "fixed" with 3.4.2, pushing session-routes and system-routes onto a third style — every version change moved the failure to a different set of files. Line-break placement in `await import` and a union type only; no logic changes.
Upstream added the `omp` mode after this branch established that a docker session runs its CLI INSIDE the container, so omp's host-CLI requirement kept the unguarded form the other eight modes had already dropped. A docker session on omp therefore threw on the host, the catch fell back to a direct PTY, and that PTY tried to exec omp on the HOST — surfacing as the bare `execvp(3) failed` this rule exists to prevent. Caught by the static guard in test/docker-adopted-container.test.ts, which requires zero unguarded `mode === '...' && !cliDir` branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rv24Pk4qzrsDYdVyDyJQmT
Ark0N
commented
Sep 5, 2026
Owner
Thanks — #357 is merged. I resolved its conflicts onto the newer abstractions rather than keeping the pre-#347 code: the root-claude arm became a registry field ( That means this branch needs a rebase onto master — the container half will mostly drop out, leaving the tmux adoption and the fixes. Happy to review those next; if you'd rather split the remaining fixes ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the ability to keep working in sessions and containers that already exist, plus a batch of fixes found while building it. 28 commits on top of
1e24817b; the suite is green here (333 files / 6508 tests).Adopt an already-running container (13 commits)
A case can point at a container you started yourself. Codeman
execs into it and never creates, starts, stops or removes it — a missing or stopped container is an error to report, not a state to fix. One adopted container can back several cases pointing at different in-container directories, since the in-container tmux session is named per SESSION, not per case. Run-mode availability is probed live inside the container rather than trusted from attach time, because a host with noclaudemay well be running a container that ships one.Adopt foreign tmux sessions (1 commit)
The home screen lists tmux sessions Codeman did not start — a
claudeinsidetmux new -s work, or just a shell — and one click turns one into a tab. This is a fourth location overlay alongside remote/docker, not a newSessionMode: the outer layer stays an ordinarycodeman-<8hex>wrapper on our own socket, so "detach, never kill the foreign session" is structural rather than a rule to remember. Capability degrades by who started the process: an adopted session has no hooks, noenvOverridesand no effort, so respawn, Ralph, the orchestrator and workingDir-based watchers all refuse or skip.Fixes
fix(input)— a client whoseseqcounter fell below the server watermark had every keystroke discarded and ACKed, so the UI looked normal while the terminal received nothing. The server now saysdupand returns the watermark; the client climbs past it and persists the counter synchronously.fix(terminal)— xterm 6 readsselectionBackground; all seven skins still set the pre-6selection, which is silently ignored. On light skins the fallback composited to within 3/255 of the background, so selections were invisible.feat(terminal)— Shift+drag now selects (the stripped mouse DECSETs make xterm's force-selection path unreachable, so the anchor is planted explicitly), and right-click copies.fix(sse)— an SSE reconnect landing on the session already on screen no longer refetches up to 1 MiB and rewrites xterm, which read as the page refreshing itself and losing your scroll position.fix(cjk),fix(ui),fix(files),feat(remote)— Ctrl/Alt-modified navigation keys reach the CLI; a hidden full-screen overlay no longer leaves a stale hit-test layer; the path picker gets a root when the server runs as root; SSH hosts can authenticate by password (BatchMode=yesandsshpassare mutually exclusive — measured).Each commit message carries the reasoning and the measurements behind it. Happy to split this into separate PRs if you would rather review it in pieces.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rv24Pk4qzrsDYdVyDyJQmT