Skip to content

feat(adopt): adopt tmux sessions a human started, in all three locations - #510

Open
dignfei wants to merge 3 commits into
Ark0N:masterfrom
dignfei:feat/adopt-foreign-tmux
Open

dignfei wants to merge 3 commits into
Ark0N:masterfrom
dignfei:feat/adopt-foreign-tmux

Conversation

@dignfei

@dignfei dignfei commented Sep 30, 2026

Copy link
Copy Markdown

What

A tmux session someone opened by hand (tmux new -s work, then claude, or just a shell) is invisible to Codeman: it lives on the default socket, not the instance-scoped one Codeman owns. This adds an "outside sessions" block to the home screen listing those, and one click wraps one in a Codeman tab. Local, in-container and over-ssh all work.

Shape: a wrapper session, not a direct attach

Adoption creates an ordinary codeman-<8hex> session on our own socket whose pane runs the attach. That makes "detach, never kill the foreign session" structural rather than a matter of discipline: killSession can only ever reach our own wrapper, so no shape of caller bug reaches the foreign server. Same reasoning as the detach-not-kill early return for non-owned remote sessions.

View reclamation differs by location: local relies on client death and ssh on SIGHUP, but a docker exec does not die with its client, so the in-container view has to be reclaimed explicitly on kill or every adoption leaks one.

Measured facts, recorded in the code comments

  • A grouped session (new-session -t) buys status off and an independent current window. It does not buy an independent size. A window is one object with one size and a group shares it: on tmux 3.3a a bare attach and a grouped attach both shrink a 200x49 target to 80x23. Only window-size largest holds it, and that is a shared window option which survives our detach, so setting it would permanently rewrite the owner's configuration. Left unset.
  • Session names come from other people, while the local launch chain ends in bash -c + JSON.stringify, which escapes neither $ nor backticks: the outer shell performs command substitution before the inner single quotes are considered (verified, both substitutions inside the inner quotes ran). So session names and socket paths go through a character allowlist and are dropped at discovery, which means a non-conforming candidate never gets an id and the adopt endpoint fails closed when it re-resolves.
  • pane_current_command is node for both claude and codex, so the mode can only come from a bounded process-tree walk of pane_pid's argv; anything unrecognized is treated as a shell.

Refusals

An adopted session says no to every assumption that Codeman owns the pane: respawn, Ralph, the orchestrator and hook waits all refuse. The hook check runs before the mode check, because adoption asks who launched the process and no mode can vouch for a workspace Codeman never touched. paneExit is forced to unknown, since the pane runs tmux attach rather than the agent and its exiting with 0 only means the connection ended. Reboot restore refuses too: all Codeman ever had was an attach command, and replaying it would build a connection to a foreign session a reboot has almost certainly taken with it, while presenting whatever it lands on as a session we restored.

Adoption is admin-only under multi-user mode: adopting someone's shell is equivalent to arbitrary host execution. Discovery is read-only, never creates a session, and degrades to an empty list whenever a probe cannot reach its target.

Testing

test/foreign-tmux.test.ts covers the pure core (probe script, output parsing, mode classification, the name/socket allowlists, the three attach-command builders). test/reboot-restore.test.ts and test/docker-adopted-container.test.ts cover the refusals.

npm test green on this branch (the 2 pre-existing docker-entrypoint failures on this machine reproduce on unmodified master), plus typecheck, lint, format:check and check:frontend-syntax.

Note

This is a large change. Happy to split it, turn it into a Discussion first, or trim scope if you would rather see it land differently.

A tmux session someone opened by hand (`tmux new -s work`, then `claude`, or
just a shell) is invisible to Codeman: it lives on the default socket, not the
instance-scoped one Codeman owns. The home screen now lists those, and one
click wraps one in a Codeman tab. Local, in-container and over-ssh all work.

The shape is a WRAPPER session, not a direct attach: adoption creates an
ordinary `codeman-<8hex>` session on our own socket whose pane runs the
attach. That makes "detach, never kill the foreign session" structural rather
than a matter of discipline — `killSession` can only ever reach our own
wrapper, so no shape of caller bug reaches the foreign server. View reclamation
differs by location: local relies on client death and ssh on SIGHUP, but a
`docker exec` does NOT die with its client, so the in-container view has to be
reclaimed explicitly on kill or every adoption leaks one.

Measured facts, recorded in the code comments:

- A grouped session (`new-session -t`) buys `status off` and an independent
  current window. It does NOT buy an independent size. A window is one object
  with one size and a group shares it: on tmux 3.3a a bare attach and a grouped
  attach both shrink a 200x49 target to 80x23. Only `window-size largest` holds
  it, and that is a shared window option which survives our detach, so setting
  it would permanently rewrite the owner's configuration. Left unset.
- Session names come from other people, while the local launch chain ends in
  `bash -c` + `JSON.stringify`, which escapes neither `$` nor backticks — the
  outer shell performs command substitution before the inner single quotes are
  considered (verified: both substitutions inside the inner quotes ran). So
  session names and socket paths go through a character allowlist and are
  dropped at DISCOVERY, which means a non-conforming candidate never gets an
  id and the adopt endpoint fails closed when it re-resolves.
- `pane_current_command` is `node` for both claude and codex, so the mode can
  only come from a bounded process-tree walk of `pane_pid`'s argv; anything
  unrecognized is treated as a shell.

An adopted session has to say no to every assumption that Codeman owns the
pane: respawn, Ralph, the orchestrator and hook waits all refuse. The hook
check runs BEFORE the mode check, because adoption asks who launched the
process and no mode can vouch for a workspace Codeman never touched. `paneExit`
is forced to unknown, since the pane runs `tmux attach` rather than the agent
and its exiting with 0 only means the connection ended. Reboot restore refuses
too: all Codeman ever had was an attach command, and replaying it would build a
connection to a foreign session a reboot has almost certainly taken with it,
while presenting whatever it lands on as a session we restored.

Adoption is admin-only under multi-user mode: adopting someone's shell is
equivalent to arbitrary host execution. Discovery is read-only, never creates a
session, and degrades to an empty list whenever a probe cannot reach its target.
@Ark0N

Ark0N commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Thank you for this, @dignfei. The PR lets Codeman list tmux sessions someone started by hand (on this machine, inside a docker case, or on a saved ssh host) and open one as a tab through a wrapper session, so closing the tab can never reach the foreign server. The wrapper shape, the allowlist applied at discovery, and the opaque-id adopt request are all well reasoned, and the measurements in the comments made this much easier to review. Typecheck, lint, format and the full npm test gate all pass here.

A few places still let Codeman act on a session, server or repo it does not own. These need to change before merge:

  1. The attach command creates a new session when its target is gone (src/foreign-tmux.ts:505). tmux new-session -t <name> does not fail on a missing target: on tmux 3.4 it joined work-prod when work was gone, created a fresh shell when nothing matched, and started a tmux server on a socket that had none. The command re-runs whenever the wrapper pane is respawned, and boot recovery does that for every dead pane (src/web/server.ts:3696 to src/session.ts:2031 to src/tmux-manager.ts:2481), so after the owner exits their session the next Codeman restart puts a new shell on their default socket, in the container, or on the ssh host. Please guard it with an exact-match check that cannot start a server, for example tmux -S <sock> has-session -t '=<target>' 2>/dev/null || { echo '<target> is gone'; exit 1; } before the new-session. The = prefix does not work on new-session -t itself (it creates a group literally named =work), so it has to be a separate has-session. Please use =<target> in the attach-session -r fallback too.

  2. The read-only fallback is silent (src/foreign-tmux.ts:509). SessionAdopt.readOnly is never set or read, and the || fires on any new-session failure, not only an old tmux. A second run with the same view name (the leftover in-container view you describe for docker) turned into a read-only client showing the owner's status bar, and typed input is dropped while the API reports success. Please either drop the fallback and fail with a message, or remove our own stale view first, and if the fallback stays, set and show readOnly.

  3. The boot hook sweep writes into the foreign workspace (src/web/server.ts:3798). ensureHooksForRecoveredWorkspaces() skips only non-claude and session.remote; an adopted session keeps its connection in adopt.remote / adopt.docker, so after a restart Codeman writes .claude/settings.local.json into the human's repo (or into a local directory that shares a remote path). Please skip session.isAdopted there.

  4. The workspace-trust auto-accept runs in adopted claude panes (src/session.ts:2896). For 90 seconds after startInteractive() (on open and on every boot recovery), Codeman would answer the trust prompt in the owner's claude. Please skip that scan when the session is adopted.

  5. Tests for the new behaviour. test/foreign-tmux.test.ts covers the pure core well; please add route tests with app.inject() (see test/routes/_route-test-utils.ts) for GET /api/mux/foreign and POST /api/sessions/adopt (multi-user admin gate, 404 re-resolve, one wrapper per target), the respawn and Ralph refusals, hooksAvailableForMode(mode, { adopted: true }), and the adopt round-trip through recovery.

Smaller items, fine in the same round:

  • src/web/routes/session-routes.ts:1286: the foreign pane cwd becomes the session workingDir. A cwd with characters outside SAFE_PATH_PATTERN (for example ~/c++ or Program Files (x86)) makes createSession throw, so the tab never starts, and the Files panel reads the local path of a docker or ssh cwd. Launching the wrapper with a neutral local workingDir and keeping the foreign cwd in adopt.paneCurrentPath for display fixes this and item 3 together.
  • Auto-clear, auto-compact and auto-resume (session-routes.ts:3302, 3323, 3345) and the Ralph auto-enable in POST /interactive (session-routes.ts:1713) still drive an adopted pane. Please add the same isAdopted gate.
  • src/web/public/foreign-sessions.js:76: with the scan toggle on, every 8 second poll re-runs one ssh per host and one docker exec per container, uncached, with a 12 second timeout, so slow hosts stack connections. Please make the wide scan one-shot on toggle, or cache and single-flight it like the local scan.
  • src/web/public/foreign-sessions.js:141: _apiJson drops the error body, so the "could not reach it" message from the server always shows as "That session is gone".
  • src/foreign-tmux.ts:249: the CLI signature table hardcodes ids and misses omp; the registry already has discovery.binaries for every CLI, and CLAUDE.md asks that no code outside stock.ts key behaviour on a CLI id.
  • src/foreign-tmux.ts:303: pass onTruncated to collectDescendants.

I will handle the docs (docs/api-reference.md, the CLAUDE.md load order and an architecture note for the adoption overlay) at merge time, so no need to write those. Once the five items above are in, I will re-review.

d fei added 2 commits September 30, 2026 23:29
Review items 1 and 2 on Ark0N#510.

`tmux new-session -t <name>` does not fail on a missing target. Measured on
3.3a: with only `work-prod` present it joined that session's group, with
nothing matching it opened a fresh shell, and on an empty socket it started a
tmux server that had none. The command re-runs whenever the wrapper pane is
respawned, and boot recovery does that for every dead pane, so after the owner
exits their session the next Codeman restart would put a new shell on their
default socket, in their container, or on their ssh host.

The invocation now begins with an exact-match `has-session -t '=<target>'` and
exits with a message when it misses. Measured: `has-session` does not start a
server on an empty socket, and the `=` prefix is what stops it matching
`work-prod` by prefix. `=` cannot go on `new-session -t` itself — tmux reads it
as part of the name and creates a group literally called `=work` — so it has to
be its own command.

The read-only `attach-session -r` fallback is gone rather than fixed. It hung
off `||`, so it fired on ANY `new-session` failure and not just an old tmux: a
leftover view from a previous adoption of the same Codeman session makes
`new-session` report `duplicate session` (measured), and the user then got a
read-only client showing the owner's status bar, with typed input dropped,
while the API reported success. Our own stale view is now removed first with a
`kill-session -t '=<our view>'` — the view name is derived from the Codeman
session id so it is provably ours, and measured, killing by name touches only
that session and leaves the owner's alone. Nothing to remove is the normal
case, so that step must not abort the chain.

`SessionAdopt.readOnly` goes with it. It was never set and never read, while
its comment claimed the UI surfaced it. Adoption now either attaches a writable
view or fails with a message; there is no third state.
… start

Review items 3, 4 and the smaller ones on Ark0N#510. An adopted session wraps a
process someone else launched, and its mode is whatever the probe saw — very
often `claude`. Every guard below is therefore its own `isAdopted` check rather
than a side effect of the external-CLI rule: the two ask different questions,
and merging them means the day the first loosens, Codeman starts acting on a
live conversation that is not ours.

- The boot workspace-hook sweep skipped only non-claude and `session.remote`.
  An adopted session keeps its connection on `adopt.remote` / `adopt.docker`,
  so a tab wrapping someone's in-container or over-ssh claude looked local and
  Codeman wrote `.claude/settings.local.json` into their repo on every restart.
- The workspace-trust auto-accept read the pane and pressed keys for 90s after
  `startInteractive()`. In an adopted pane that is someone else's dialog, and
  boot recovery re-opens the window on every restart.
- Auto-clear, auto-compact, auto-resume and the Ralph auto-enable in
  `POST /interactive` all drove the pane. Each now refuses with its own gate.
- The wrapper's `workingDir` was the foreign pane's cwd. That cwd can carry
  characters outside SAFE_PATH_PATTERN (`~/c++`, `Program Files (x86)`), which
  makes `createSession` throw so the tab never starts, and for a container or
  ssh candidate it is not a path on this host at all. It is now a neutral local
  `/tmp`, the same cwd `resolveMuxAttachCwd` already gives the pane; the foreign
  cwd stays on `adopt.paneCurrentPath` for display.
- The CLI signature table hardcoded ids, which CLAUDE.md forbids outside
  `stock.ts`, and had already gone stale: `omp` was missing, so a hand-started
  omp pane read as `shell`. It is now derived from each entry's
  `discovery.binaries` at call time, regex-escaped, longest name first. Stock
  entries only, since `SessionMode` is the closed union of stock ids.
- The descendant walk now passes `onTruncated`. A capped walk returns "nothing
  matched", which is byte-identical to the answer for a real shell.
- The home screen's wide scan is one-shot. Riding `docker exec` and a full ssh
  handshake per target, each with a ~12s timeout, on the 8s poll stacked
  connections on a slow host. Wide rows are kept apart and merged, because a
  local-only payload cannot contain them.
- Adoption failures report the server's own message. `_apiJson` folds a non-2xx
  to null, which collapsed "could not reach it just now" into "that session is
  gone" and reported a live remote session as deleted on every link flicker.

Tests: route tests for the refusals, the admin gate, the 404 re-resolve, the
one-wrapper-per-target reuse and the adopt round-trip; unit tests for the two
non-HTTP guards, the registry-derived signatures, truncation reporting and the
frontend's scan and error paths. Each guard was removed in turn to confirm its
own tests go red.
@Ark0N

Ark0N commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks again, @dignfei. This PR lets Codeman list tmux sessions someone started by hand (on this machine, in a docker case container, or on a saved ssh host) and open one as a tab through a wrapper session that only ever detaches. The second round answers every item from my first review. I ran the new attach command against real tmux 3.4 servers, and the has-session guard, the stale-view cleanup, status off on the view only and the view collection on close all behave exactly as your comments say. CI and the full test gate are green.

Two bugs need fixing before merge:

  1. Same-named sessions in different locations collide (src/web/routes/session-routes.ts:1238, src/web/routes/mux-routes.ts:85 and :91). Both the one-wrapper-per-target check and adoptedBy key on socket path plus session name only. Containers commonly run tmux as root (/tmp/tmux-0/default) with default session names 0, 1, so once container A's 0 is open, adopting container B's 0 returns A's tab with alreadyAdopted: true, and B's row reads "Go to tab" and opens A. The same happens between this machine and an ssh host with the same uid. Please add one key helper used at both sites, built from location, the host identity (docker.hostId plus containerName, remote.hostId, empty for local), the socket and the session name, and a route test with two candidates that share socket and name.

  2. Docker discovery never reaches hosts with a context or daemon host (src/foreign-tmux-discovery.ts:222). buildDockerBaseArgs() single-quotes the --context and -H values because it is meant for shell strings, and execFile passes those quotes through literally: docker --context "'default'" fails with "context not found". Every container behind a remote daemon or named context then shows only the "not running, no tmux, or engine unreachable" note. src/docker-hosts.ts:985 has dockerEngineArgv() for exactly this case; please export it (or an equivalent) and use it here, with a test that the argv carries no quote characters. The attach and view-kill paths build shell strings, so they are right to keep buildDockerBaseArgs.

Smaller items, fine in the same round:

  • src/web/routes/session-routes.ts:1236: the wrapper check reads ctx.mux.getSessions(), but the tmux record only exists after POST /interactive starts the pane, so two adopts before the first tab starts create two wrappers. Please check ctx.sessions (each Session has adopt) with the key from item 1.
  • src/web/routes/session-routes.ts:1379: POST /api/sessions/:id/custom-model refuses remote and docker sessions because the env lands on the local wrapper pane. An adopted session has the same shape, so please refuse it there too (|| session.isAdopted).
  • src/tmux-manager.ts:2777: please run the in-container view kill with async exec(..., () => {}) like the docker branch at :2908 (the execSync can block the event loop for 5 s), add the clearPaneExit and clearRemoteReconnectState calls the non-owned remote branch makes, and target the view with = in buildForeignDockerViewKillCommand (src/foreign-tmux.ts:658).
  • src/web/public/foreign-sessions.js:156: a wide scan stores its notes in _foreignNotes, so the next local poll wipes "remote X: unreachable" while the rows stay. Please keep the non-local notes in _foreignWideNotes. Related: discoverRemoteForeign does not pass notes to candidatesFrom (src/foreign-tmux-discovery.ts:272), so skipped-name notes never appear for ssh hosts.
  • Tests for the recovery path: adopt: muxSession.adopt ?? savedState?.adopt (src/web/server.ts:3541), the respawnPane fallback to session.adopt (src/tmux-manager.ts:2481) and the adopted killSession branch are not exercised yet. Exporting buildAdoptSessionCommand and testing it covers the most important one.
  • Nits: the comment at src/foreign-tmux.ts:552 still mentions the removed read-only fallback; the route JSDoc at src/web/routes/session-routes.ts:1169 now sits above ADOPTED_WRAPPER_WORKING_DIR instead of the route; FOREIGN_MODE_LABEL (src/web/public/foreign-sessions.js:51) has no omp entry; the comment at foreign-sessions.js:179 says the lock is per button, but it is global.

As before, I will write the docs at merge time (docs/api-reference.md, the CLAUDE.md load order and an architecture note). Once items 1 and 2 are in, this is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants