From 9e5590a81cb73080da1a5a8786e8df7eabda5707 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 24 Sep 2026 10:52:37 +0000 Subject: [PATCH] fix(pty): don't leak an outer session id into session-less PTYs --- docs/USAGE.md | 2 +- src/pty/createPty.ts | 21 ++++++++++---- test/unit/pty/createPty.test.ts | 50 +++++++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 49fda7e..58e7a7b 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -268,7 +268,7 @@ The shell also receives two session variables, so scripts inside a session can d | `AGENT_TTY_ACTIVE` | `true`. | | `AGENT_TTY_SESSION_ID` | The session's ID (a ULID, as printed by `create --json`). | -Both replace inherited values, so a session created from inside another session reports its own ID, not the outer one. Like the `PROMPT_EOL_MARK` default, they are set at spawn time and are not stored in the session's recorded `env`. +Both replace inherited values, so a session created from inside another session reports its own ID, not the outer one. A PTY that agent-tty spawns without a session (the `doctor` spawn probe) gets neither variable: inherited values are removed, so it never reports an outer session's ID. Like the `PROMPT_EOL_MARK` default, they are set at spawn time and are not stored in the session's recorded `env`. Any `--env` value always wins, so you can opt back into the shell's native behavior or override a session variable per session: diff --git a/src/pty/createPty.ts b/src/pty/createPty.ts index 2f98e37..4f76f2e 100644 --- a/src/pty/createPty.ts +++ b/src/pty/createPty.ts @@ -87,9 +87,11 @@ const PROMPT_EOL_MARK_ENV_KEY = 'PROMPT_EOL_MARK'; * * Precedence, lowest to highest: the inherited process environment (minus * host-only internals), then the `PROMPT_EOL_MARK=''` default, then the - * generated session metadata (`AGENT_TTY_ACTIVE=true` and, when `sessionId` is - * given, `AGENT_TTY_SESSION_ID`), then the caller-supplied `env` (so a `--env` - * value always wins — even an explicit empty one), then `TERM`. The defaults sit + * generated session metadata, then the caller-supplied `env` (so a `--env` + * value always wins — even an explicit empty one), then `TERM`. The session + * metadata is `AGENT_TTY_ACTIVE=true` plus `AGENT_TTY_SESSION_ID` when + * `sessionId` is given; without a `sessionId` both are removed instead, so a + * session-less PTY never claims an inherited outer session. The defaults sit * after the inherited environment so they also override inherited values: a * nested session reports its own id rather than the outer session's, and * captures stay deterministic regardless of the launching shell. The @@ -117,8 +119,17 @@ export function resolvePtyEnv( resolved[PROMPT_EOL_MARK_ENV_KEY] = ''; } - resolved.AGENT_TTY_ACTIVE = 'true'; - if (sessionId) { + invariant( + sessionId === undefined || sessionId.length > 0, + 'resolvePtyEnv: sessionId must be non-empty when provided', + ); + if (sessionId === undefined) { + // A PTY without a session (the doctor spawn probe) must not claim one, so + // drop any session variables inherited from an outer session. + delete resolved.AGENT_TTY_ACTIVE; + delete resolved.AGENT_TTY_SESSION_ID; + } else { + resolved.AGENT_TTY_ACTIVE = 'true'; resolved.AGENT_TTY_SESSION_ID = sessionId; } diff --git a/test/unit/pty/createPty.test.ts b/test/unit/pty/createPty.test.ts index 3d477a5..09568a4 100644 --- a/test/unit/pty/createPty.test.ts +++ b/test/unit/pty/createPty.test.ts @@ -65,24 +65,58 @@ describe('resolvePtyEnv', () => { expect(Object.prototype.hasOwnProperty.call(resolved, 'EMPTY')).toBe(false); }); - it('unconditionally sets AGENT_TTY_ACTIVE to true', () => { - const resolved = resolvePtyEnv({}, 'xterm-256color', {}); - expect(resolved.AGENT_TTY_ACTIVE).toBe('true'); - }); - - it('sets AGENT_TTY_SESSION_ID when sessionId is provided', () => { + it('sets AGENT_TTY_ACTIVE and AGENT_TTY_SESSION_ID when sessionId is provided', () => { const resolved = resolvePtyEnv( {}, 'xterm-256color', {}, 'test-session-123', ); + expect(resolved.AGENT_TTY_ACTIVE).toBe('true'); expect(resolved.AGENT_TTY_SESSION_ID).toBe('test-session-123'); }); - it('does not set AGENT_TTY_SESSION_ID when sessionId is not provided', () => { + it('sets no session variables when sessionId is not provided', () => { const resolved = resolvePtyEnv({}, 'xterm-256color', {}); - expect(resolved.AGENT_TTY_SESSION_ID).toBeUndefined(); + expect( + Object.prototype.hasOwnProperty.call(resolved, 'AGENT_TTY_ACTIVE'), + ).toBe(false); + expect( + Object.prototype.hasOwnProperty.call(resolved, 'AGENT_TTY_SESSION_ID'), + ).toBe(false); + }); + + it('strips inherited outer session variables when sessionId is not provided', () => { + const resolved = resolvePtyEnv({}, 'xterm-256color', { + AGENT_TTY_ACTIVE: 'true', + AGENT_TTY_SESSION_ID: 'outer-session', + OTHER: 'kept', + }); + + expect( + Object.prototype.hasOwnProperty.call(resolved, 'AGENT_TTY_ACTIVE'), + ).toBe(false); + expect( + Object.prototype.hasOwnProperty.call(resolved, 'AGENT_TTY_SESSION_ID'), + ).toBe(false); + expect(resolved.OTHER).toBe('kept'); + }); + + it('lets caller-supplied env set session variables when sessionId is not provided', () => { + const resolved = resolvePtyEnv( + { AGENT_TTY_ACTIVE: 'custom', AGENT_TTY_SESSION_ID: 'custom-id' }, + 'xterm-256color', + { AGENT_TTY_ACTIVE: 'true', AGENT_TTY_SESSION_ID: 'outer-session' }, + ); + + expect(resolved.AGENT_TTY_ACTIVE).toBe('custom'); + expect(resolved.AGENT_TTY_SESSION_ID).toBe('custom-id'); + }); + + it('rejects an empty sessionId', () => { + expect(() => resolvePtyEnv({}, 'xterm-256color', {}, '')).toThrow( + /sessionId must be non-empty/, + ); }); it('replaces an inherited outer session id with the session id', () => {