diff --git a/docs/api-reference.md b/docs/api-reference.md index c733198e..00e64663 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1188,9 +1188,11 @@ session: respawn, Ralph (its config and the `fix-plan/write`, `fix-plan/read` an `ralph-prompt/write` file routes), auto-clear, auto-compact, auto-resume, custom model endpoints, prompt uploads and the image watcher. `stop` waits are a 400, and so are `blocked` waits unless Codeman reads that CLI's dialogs off its screen (codex), the git -routes answer `unsupported` with `reason: "adopted"`, and `last-response` answers empty, -since Codeman knows no transcript for it. Its `paneExit` stays absent, because the pane -holds a `tmux attach` and not the agent. Reboot restore does not rebuild one. +routes answer `unsupported` with `reason: "adopted"`, and +`last-response` answers empty, since Codeman knows no transcript for it. Its `paneExit` +stays absent, because the pane holds a `tmux attach` and not the agent. Reboot restore +does not rebuild one. A wrapper that cannot start answers `OPERATION_FAILED` and leaves +the tab to be tried again: Codeman never starts a local process in its place. ## Approvals Inbox diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 75db68ce..90238859 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -109,7 +109,7 @@ Input arriving during a wake is buffered: a chunk over 4 KB is dropped whole, ne **No workspace.** ⚠️ An adopted session keeps its connection on `adopt.remote`/`adopt.docker`, so `session.remote` and `session.docker` are undefined, and its `workingDir` is the neutral `/tmp`. Code that branches on those two reads an in-container or over-ssh adoption as a local session working in `/tmp`. So anything that treats `workingDir` as the session's workspace must also test `session.isAdopted` (`adopt` on a pure view): uploads, the upload GC, git status, git fetch and its scheduler, the image watcher, Read My Mind's git signals, the Discord folder, `last-response`, guarded input and Ralph's `@fix_plan.md` and `@ralph_prompt.md` routes all do. The file surfaces (Files panel, File Viewer and its edit mode) still treat `/tmp` as the workspace, which is harmless: it is world-writable anyway, and adoption is admin-only in multi-user mode. A wrapper reports `statusLineExporter: 'remote'`, which is also what keeps keep-warm off it. -**Lifecycle.** `paneExitApplies` is false for a wrapper, so an attach that ended is never `paneExit` and the sweep never closes the tab. Boot recovery re-attaches a dead wrapper, which prints that the session is gone and exits. Reboot restore refuses one, since all Codeman ever had was an attach command. +**Lifecycle.** `paneExitApplies` is false for a wrapper, so an attach that ended is never `paneExit` and the sweep never closes the tab. Boot recovery re-attaches a dead wrapper, which prints that the session is gone and exits. Reboot restore refuses one, since all Codeman ever had was an attach command. ⚠️ A wrapper that cannot start fails the start, and selecting the tab tries it again (`_failAdoptedStart()`). It never takes the direct-PTY fallback, which would run a new local `claude --dangerously-skip-permissions` or a server-account shell in `/tmp` under the foreign session's name. ### Docker cases diff --git a/src/session.ts b/src/session.ts index 03b03f7c..5bc1176c 100644 --- a/src/session.ts +++ b/src/session.ts @@ -3205,6 +3205,26 @@ export class Session extends EventEmitter { this.emit('output', data); } + /** + * End a start that cannot reach an adopted session's wrapper. + * + * ⚠️ Never the direct-PTY fallback. What that starts is a NEW process on THIS host + * (`claude --dangerously-skip-permissions`, or a server-account shell) in the neutral + * `/tmp`, under a tab still carrying the foreign session's name, so someone who opened an + * ssh or container session would end up typing into this machine. `_useMux` stays on, so + * the next start, which is what selecting the tab sends, tries the wrapper again. + * + * The `error` event is what the tab shows: the browser's re-attach does not read the + * route's answer. + */ + private _failAdoptedStart(err: unknown): never { + const message = `Could not attach to ${this._adopt?.targetSession}: ${getErrorMessage(err)}`; + console.error(`[Session] ${message}`); + this._status = 'stopped'; + this.emit('error', message); + throw new Error(message); + } + async startInteractive(): Promise { if (this.ptyProcess) { throw new Error('Session already has a running process'); @@ -3441,6 +3461,7 @@ export class Session extends EventEmitter { this._armPaneSettle(true); } } catch (err) { + if (this._adopt) this._failAdoptedStart(err); console.error('[Session] Failed to create mux session, falling back to direct PTY:', err); this._useMux = false; this._muxSession = null; @@ -3460,6 +3481,7 @@ export class Session extends EventEmitter { if (getCli(this.mode)?.capabilities.requiresMux) { throw new Error(`${getModeLabel(this.mode)} sessions require tmux. Direct PTY fallback is not supported.`); } + if (this._adopt) this._failAdoptedStart(new Error('tmux is not available')); try { // Pass --session-id to use the SAME ID as the Codeman session // This ensures subagents can be directly matched to the correct tab. @@ -4463,6 +4485,7 @@ export class Session extends EventEmitter { }, 100); } } catch (err) { + if (this._adopt) this._failAdoptedStart(err); console.error('[Session] Failed to create mux session, falling back to direct PTY:', err); this._useMux = false; this._muxSession = null; @@ -4471,6 +4494,7 @@ export class Session extends EventEmitter { // Fallback to direct PTY if mux is not used if (!this.ptyProcess) { + if (this._adopt) this._failAdoptedStart(new Error('tmux is not available')); try { this.ptyProcess = spawnPtyWithHelperRepair(() => pty.spawn(shell, [], { diff --git a/test/foreign-adoption-guards.test.ts b/test/foreign-adoption-guards.test.ts index ece62c1a..bed71d24 100644 --- a/test/foreign-adoption-guards.test.ts +++ b/test/foreign-adoption-guards.test.ts @@ -1,7 +1,7 @@ /** * @fileoverview The adoption guards that are not reachable over HTTP: the - * boot-time workspace-hook sweep, the workspace-trust auto-accept and the - * new-shell clear. + * boot-time workspace-hook sweep, the workspace-trust auto-accept, the new-shell + * clear and the direct-PTY fallback. * * Each acts on a session's WORKSPACE or its PANE, each runs unattended, and each * previously keyed off `session.remote`, the mode or nothing at all, none of which @@ -24,6 +24,20 @@ vi.mock('../src/hooks-config.js', async (importOriginal) => { return { ...actual, applyWorkspaceHooks: (...args: unknown[]) => applyWorkspaceHooks(...(args as [])) }; }); +// The direct-PTY fallback must never run the real `claude` or a real login shell from a +// test, and the spawn is the one thing that proves whether it happened at all. +const ptySpawn = vi.fn(); +vi.mock('node-pty', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + spawn: (file: string, args: string[] | string, options: import('node-pty').IPtyForkOptions) => { + ptySpawn(file, args); + return real.spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], options); + }, + }; +}); + const { WebServer } = await import('../src/web/server.js'); const { Session } = await import('../src/session.js'); @@ -163,3 +177,78 @@ describe('the new-shell clear', () => { expect(await bytesWrittenAfterStart(adopt)).toEqual([]); }); }); + +describe('the direct-PTY fallback', () => { + const adopt = { location: 'remote', socketPath: '/tmp/tmux-0/default', targetSession: 'work', viewSession: '' }; + + /** A wrapper that cannot start: `createSession` threw (a loaded tmux server timing out, say). */ + const session = (mode: 'claude' | 'shell', opts: { adopt?: typeof adopt; useMux?: boolean } = {}) => { + const s = new Session({ + name: 'work', + workingDir: '/tmp', + mode, + useMux: opts.useMux ?? true, + mux: {}, + ...(opts.adopt ? { adopt: opts.adopt } : {}), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + const errors: string[] = []; + s.on('error', (message: string) => errors.push(message)); + const priv = s as unknown as Record; + priv._setupOrAttachMuxSession = async () => { + throw new Error('tmux new-session timed out'); + }; + return { s, priv, errors }; + }; + + beforeEach(() => ptySpawn.mockClear()); + + it('still catches a session Codeman started, whose fallback is a real PTY', async () => { + // The control: without it, "nothing spawned" below is equally explained by a mock + // that never sees a spawn. A shell, because a claude fallback also probes the real binary. + const { s } = session('shell'); + await s.startShell(); + expect(ptySpawn).toHaveBeenCalledTimes(1); + await s.stop(true).catch(() => {}); + }); + + it.each([ + ['claude', 'startInteractive'], + ['shell', 'startShell'], + ] as const)('never starts a local %s for an adopted session (%s)', async (mode, start) => { + // ⚠️ The tab still says it is someone else's session. A fallback here runs + // `claude --dangerously-skip-permissions` or a server-account shell in `/tmp` on THIS + // host, so whoever opened an ssh or container session ends up typing into this one. + const { s, errors } = session(mode, { adopt }); + await expect(s[start]()).rejects.toThrow(/tmux new-session timed out/); + expect(ptySpawn).not.toHaveBeenCalled(); + expect(s.pid).toBeNull(); + expect(s.status).toBe('stopped'); + // The browser's re-attach does not read the POST's answer, so this is what the tab shows. + expect(errors.join()).toMatch(/work/); + }); + + it('leaves the wrapper to be tried again', async () => { + const { s, priv } = session('shell', { adopt }); + await expect(s.startShell()).rejects.toThrow(); + const disposable = () => ({ dispose() {} }); + priv._setupOrAttachMuxSession = async () => { + priv.ptyProcess = { write: vi.fn(), onData: disposable, onExit: disposable, kill() {}, pid: 1 }; + return { isRestored: true, respawnedDeadPane: false }; + }; + await s.startShell(); + expect(s.pid).toBe(1); + expect(ptySpawn).not.toHaveBeenCalled(); + priv.ptyProcess = null; + await s.stop(true).catch(() => {}); + }); + + it.each([ + ['claude', 'startInteractive'], + ['shell', 'startShell'], + ] as const)('never starts a local %s for an adopted session with no mux at all (%s)', async (mode, start) => { + const { s } = session(mode, { adopt, useMux: false }); + await expect(s[start]()).rejects.toThrow(/tmux/); + expect(ptySpawn).not.toHaveBeenCalled(); + }); +});