diff --git a/.agentworkforce/trajectories/completed/2026-07/traj_rifnk10uqgl4.md b/.agentworkforce/trajectories/completed/2026-07/traj_rifnk10uqgl4.md new file mode 100644 index 000000000..7d2644ef0 --- /dev/null +++ b/.agentworkforce/trajectories/completed/2026-07/traj_rifnk10uqgl4.md @@ -0,0 +1,39 @@ +# Fix Relay v11.2 CLI demo visuals and Relay-driven coordination + +**Status:** Completed +**Confidence:** 90% +**Date:** 2026-07-25 + +## Summary + +Fixed `attach --mode drive|passthrough` terminal corruption by reserving a +non-wrapping local status row, clipping its label, preserving the child +application's ANSI boundary, scroll-region, origin-mode, and alternate-screen +state, and reconciling terminal resizes before and during setup. Predictive echo +now updates boundary state only after its actual terminal writes. + +Published Relay-first coordination rules through MCP initialize instructions so +interactive Codex sessions contact existing named Relay participants instead of +substituting provider-native subagents. Added a credential-gated real Codex E2E +that requires a relevant message to the named participant and rejects the +native-subagent wait flow. + +## Decisions + +- Reserve a dedicated terminal row and spare autowrap column for Relay's attach + status, with dynamic handling for degenerate and resized terminals. +- Track terminal controls with a streaming ANSI state machine so Relay repaints + restore the child's exact DECSTBM/DECOM/buffer state without interpreting + control-looking bytes inside OSC/DCS payloads. +- Deliver named-participant routing guidance as MCP server instructions because + interactive CLI sessions may start without a task-prefix prompt. + +## Validation + +- Full CLI suite: 816 passed, 11 skipped. +- Focused attach/MCP suite: 211 passed. +- CLI build, broker integration TypeScript build, lint (zero errors), and + `git diff --check` passed. +- Independent Codex fresh-context review approved the final state. +- Claude review and the credentialed live Codex E2E were unavailable because + Claude was not authenticated and external credential egress was not approved. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0daac1d24..683303b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Identity forwarding to the Relaycast gateway is no longer gated on the local process carrying a PostHog key. An npm-installed CLI bakes no key, so it previously forwarded no identity at all and every hosted event fell back to being keyed on the workspace. Forwarding now follows the telemetry preference alone. - `node agent list` no longer reports an exited agent as `working` indefinitely; the broker now reaps a worker whose harness has gone away or never started. - `node agent attach --mode drive|passthrough` truncates its status line to the terminal width, so a narrow pane no longer stacks status lines over the agent's output. +- PTY `node agent attach --mode drive|passthrough` sessions now reserve and safely clip their status row, preventing full-screen agent CLIs from tearing, scrolling, or duplicating Relay's controls. +- Detaching from `node agent attach --mode drive|passthrough` restores the row and column the status line reserved, so a later `--mode view` session no longer inherits a PTY one row and column short. `POST /api/resize/{name}` applies dimensions sent alongside `release: true`. +- Agent Relay MCP instructions now route work with existing named participants through Relay instead of provider-native subagents. - `node agent attach --mode view` now exits on the first Ctrl-C instead of waiting for a WebSocket close handshake. - The broker now sends its anonymous telemetry id (`X-Agent-Relay-Distinct-Id`) and origin actor with its Relaycast requests, so hosted usage can be attributed to an install instead of only to a workspace. The id header is omitted when telemetry is opted out; requests and origin actor are unaffected. - The broker now reads its telemetry preference and machine-id files from `AGENT_RELAY_DATA_DIR` when set, matching the CLI. It previously only read `~/.agentworkforce/relay/telemetry.json`, so an opt-out written by `agent-relay telemetry disable` under a configured data directory was ignored. diff --git a/crates/broker/src/listen_api.rs b/crates/broker/src/listen_api.rs index 2a33a6aea..f218ca654 100644 --- a/crates/broker/src/listen_api.rs +++ b/crates/broker/src/listen_api.rs @@ -2095,9 +2095,11 @@ async fn send_pty_input_ws_error( #[derive(Deserialize)] struct ResizePtyBody { - /// Target dimensions. Defaulted so a pure ownership release (`release: - /// true`) doesn't have to carry dummy dimensions — the handler skips the - /// resize entirely on release. + /// Target dimensions. Defaulted to zero so a pure ownership release + /// (`release: true`) doesn't have to carry dummy dimensions — the handler + /// skips the resize when a release carries no real size. A release that + /// *does* carry dimensions applies them before dropping ownership, so an + /// attach client can hand back a reserved status row in one request. #[serde(default)] rows: u16, #[serde(default)] @@ -4802,6 +4804,54 @@ mod auth_tests { replier.await.expect("replier should complete"); } + #[tokio::test] + async fn resize_pty_route_release_forwards_restore_dimensions() { + // An attach that reserved a status row hands it back on the release + // itself, so the route must forward rows/cols alongside `release: true` + // rather than dropping them as it would for a pure release. + let (router, mut rx) = test_router(Some("secret")); + let replier = tokio::spawn(async move { + match rx.recv().await { + Some(ListenApiRequest::ResizePty { + rows, + cols, + session_id, + release, + reply, + .. + }) => { + assert_eq!(rows, 30); + assert_eq!(cols, 100); + assert_eq!(session_id.as_deref(), Some("sess-1")); + assert!(release); + let _ = reply.send(Ok(json!({ "released": true, "resized": true }))); + } + other => panic!("unexpected request: {:?}", other.map(|_| "other")), + } + }); + + let response = router + .oneshot( + Request::builder() + .uri("/api/resize/worker-a") + .method("POST") + .header("x-api-key", "secret") + .header("content-type", "application/json") + .body(Body::from( + json!({ "rows": 30, "cols": 100, "session_id": "sess-1", "release": true }) + .to_string(), + )) + .expect("request should build"), + ) + .await + .expect("request should succeed"); + + assert_eq!(response.status(), StatusCode::OK); + let body = response_json(response).await; + assert_eq!(body["resized"], json!(true)); + replier.await.expect("replier should complete"); + } + #[tokio::test] async fn resize_pty_route_normalises_blank_session_id() { // A whitespace-only session id must arrive as `None`, never as a shared diff --git a/crates/broker/src/runtime/api.rs b/crates/broker/src/runtime/api.rs index 4b1179e6e..d4376f5e7 100644 --- a/crates/broker/src/runtime/api.rs +++ b/crates/broker/src/runtime/api.rs @@ -1392,26 +1392,56 @@ impl BrokerRuntime { // Explicit ownership release on detach (see `resize_owners` // doc on `BrokerRuntime`). A release carries the owning - // `session_id`; we drop ownership only if it matches, then - // return without touching the PTY size. A release without a - // session id, or from a non-owner, is a no-op. The response - // reports the *actual* outcome so the client can tell a real - // release from a no-op. + // `session_id`; we drop ownership only if it matches. A release + // without a session id, or from a non-owner, is a no-op. The + // response reports the *actual* outcome so the client can tell + // a real release from a no-op. + // + // A release MAY carry dimensions, which are applied to the + // worker before ownership is dropped. Attach clients that + // reserve a status row need to hand that row back on detach, + // and doing it here keeps the restore atomic with the release: + // a separate resize call would need to land strictly before the + // release (a later one re-claims the lease — the detach race in + // #1247) and would add a second round-trip to teardown. Zero + // dimensions mean "no restore", so a pure release still needs + // no placeholder size. if release { - let released = match session_id.as_deref() { - Some(sid) - if resize_owners - .get(&name) - .is_some_and(|owner| owner.session_id == sid) => - { - resize_owners.remove(&name); - true - } - _ => false, + let owns = match session_id.as_deref() { + Some(sid) => resize_owners + .get(&name) + .is_some_and(|owner| owner.session_id == sid), + None => false, }; + // Only the owner may resize, and only a real size restores. + // A worker that has already exited just releases: teardown + // is best-effort and must not fail on a gone worker. + let resized = owns + && rows > 0 + && cols > 0 + && matches!( + workers + .workers + .get(&name) + .map(|handle| handle.spec.runtime.clone()), + Some(AgentRuntime::Pty) + ) + && workers + .send_to_worker( + &name, + "resize_pty", + Some(RequestId::new(format!("api_{}", Uuid::new_v4().simple()))), + json!({ "rows": rows, "cols": cols }), + ) + .await + .is_ok(); + if owns { + resize_owners.remove(&name); + } let _ = reply.send(Ok(json!({ "name": name, - "released": released, + "released": owns, + "resized": resized, }))); } else if rows == 0 || cols == 0 { let _ = diff --git a/packages/cli/src/cli/agent-relay-mcp.protocol.test.ts b/packages/cli/src/cli/agent-relay-mcp.protocol.test.ts new file mode 100644 index 000000000..8231309bc --- /dev/null +++ b/packages/cli/src/cli/agent-relay-mcp.protocol.test.ts @@ -0,0 +1,26 @@ +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { describe, expect, it } from 'vitest'; + +import { createAgentRelayMcpServer } from './agent-relay-mcp.js'; + +describe('Agent Relay MCP initialization', () => { + it('delivers Relay-first coordination instructions through the MCP protocol', async () => { + const server = createAgentRelayMcpServer({}); + const client = new Client({ name: 'relay-protocol-test', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + + expect(client.getInstructions()).toContain( + 'Existing Relay participants are not local or built-in subagents' + ); + expect(client.getInstructions()).toContain('"send_dm"'); + } finally { + await client.close(); + await server.close(); + } + }); +}); diff --git a/packages/cli/src/cli/agent-relay-mcp.startup.test.ts b/packages/cli/src/cli/agent-relay-mcp.startup.test.ts index 67b50d388..b9ca08a9c 100644 --- a/packages/cli/src/cli/agent-relay-mcp.startup.test.ts +++ b/packages/cli/src/cli/agent-relay-mcp.startup.test.ts @@ -62,6 +62,7 @@ async function loadAgentRelayMcpModule(options: LoadOptions = {}) { class FakeTransport {} class FakeMcpServer { + readonly options: unknown; readonly tools = new Map Promise }>(); readonly prompts = new Map Promise }>(); readonly resources = new Map< @@ -80,7 +81,8 @@ async function loadAgentRelayMcpModule(options: LoadOptions = {}) { }; listToolsHandler?: (req: unknown, extra: unknown) => Promise<{ tools?: Array> }>; - constructor(_info: unknown, _capabilities: unknown) { + constructor(_info: unknown, capabilities: unknown) { + this.options = capabilities; this.server = { _requestHandlers: new Map([ [ @@ -425,6 +427,11 @@ describe('createAgentRelayMcpServer', () => { expect(server.server._requestHandlers.has('resources/subscribe')).toBe(true); expect(server.server._requestHandlers.has('resources/unsubscribe')).toBe(true); expect(server.prompts.get('system')).toBeDefined(); + expect(server.options).toMatchObject({ + instructions: expect.stringContaining( + 'Existing Relay participants are not local or built-in subagents' + ), + }); await expect(server.tools.get('register_agent')?.handler({ name: 'WorkerA' })).rejects.toThrow( 'Workspace key not configured. Call "create_workspace" first, or "set_workspace_key" if someone shared a workspace key.' diff --git a/packages/cli/src/cli/agent-relay-mcp.ts b/packages/cli/src/cli/agent-relay-mcp.ts index 8007c828e..0125de853 100644 --- a/packages/cli/src/cli/agent-relay-mcp.ts +++ b/packages/cli/src/cli/agent-relay-mcp.ts @@ -61,7 +61,12 @@ function withExitAfterTaskInstruction(task: string): string { return `${task}\n\n${EXIT_AFTER_TASK_INSTRUCTION}`; } -const DEFAULT_SYSTEM_PROMPT = `You are an AI agent in a collaborative workspace powered by Agent Relay. You can communicate with other agents using these MCP tools: +export const AGENT_RELAY_MCP_INSTRUCTIONS = `You are an AI agent in a collaborative workspace powered by Agent Relay. You can communicate with other agents using these MCP tools: + +## Coordination rule +- When the user asks you to work with, contact, coordinate with, or wait for named participants that already exist in this Agent Relay workspace, use Agent Relay tools such as "list_agents", "send_dm", "post_message", and "check_inbox". +- Existing Relay participants are not local or built-in subagents. Do not replace them with your CLI's native subagent, team, task, or collaboration feature. +- Do not claim to have contacted or waited for a Relay participant unless the corresponding Relay tool call succeeded. ## Getting Started 1. The current project workspace is resumed automatically when one was selected before @@ -89,6 +94,8 @@ const DEFAULT_SYSTEM_PROMPT = `You are an AI agent in a collaborative workspace - React with emoji to acknowledge messages - Keep messages concise and actionable`; +const DEFAULT_SYSTEM_PROMPT = AGENT_RELAY_MCP_INSTRUCTIONS; + type AgentResultCallbackConfig = { url: string; token: string; @@ -738,6 +745,7 @@ export function createAgentRelayMcpServer(options: AgentRelayMcpServerOptions): tools: {}, prompts: {}, }, + instructions: AGENT_RELAY_MCP_INSTRUCTIONS, } ); diff --git a/packages/cli/src/cli/lib/attach-drive.test.ts b/packages/cli/src/cli/lib/attach-drive.test.ts index fa3408fe7..e299b6857 100644 --- a/packages/cli/src/cli/lib/attach-drive.test.ts +++ b/packages/cli/src/cli/lib/attach-drive.test.ts @@ -1,7 +1,9 @@ import { Buffer } from 'node:buffer'; +import { Terminal } from '@xterm/headless'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { LOCAL_TERMINAL_RESET_SEQUENCE } from './attach.js'; import { KeybindParser, classifyWsEvent, @@ -569,7 +571,7 @@ describe('KeybindParser', () => { /** Strip the save/position/clear/reverse-video wrapper down to the visible label. */ function stripStatusLineAnsi(rendered: string): string { // eslint-disable-next-line no-control-regex -- matching the raw ESC bytes this module emits - return rendered.replace(/\x1b(?:[78]|\[[0-9;]*[A-Za-z])/g, ''); + return rendered.replace(/\x1b(?:[78]|\[[?0-9;]*[A-Za-z])/g, ''); } describe('renderStatusLine', () => { @@ -621,7 +623,7 @@ describe('renderStatusLine', () => { cols, }); const text = stripStatusLineAnsi(out); - expect(text.length).toBeLessThanOrEqual(cols); + expect(text.length).toBeLessThan(cols); // Middle-truncated: the verb + agent name and the key hints both survive. expect(text).toContain('[drive Gamemaster'); expect(text).toContain('Ctrl+C detach]'); @@ -1091,21 +1093,33 @@ describe('runDriveSession', () => { await sessionPromise; }); - it('writes worker_stream chunks to stdout and repaints the status line', async () => { + it('writes worker_stream chunks and safely restores the reserved status row', async () => { const { deps, sockets, writes, stdin } = createHarness(); const sessionPromise = runDriveSession('Alice', {}, deps); const socket = await openSocket(sockets); socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'live output' })); expect(writes.includes('live output')).toBe(true); - // Some paint should follow the worker chunk. - const liveIdx = writes.indexOf('live output'); - const repaintAfter = writes.slice(liveIdx + 1).some((w) => w.includes('drive Alice')); - expect(repaintAfter).toBe(true); + const paintsAfter = writes.filter((w) => w.includes('drive Alice')).length; + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'more output' })); + expect(writes.filter((w) => w.includes('drive Alice')).length).toBeGreaterThan(paintsAfter); stdin.type(Buffer.from([0x03])); await sessionPromise; }); + it('repaints after worker output erases the full display', async () => { + const { deps, sockets, writes, stdin } = createHarness(); + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + const paintsBefore = writes.filter((w) => w.includes('drive Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '\x1b[2J' })); + + expect(writes.filter((w) => w.includes('drive Alice')).length).toBeGreaterThan(paintsBefore); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + it('forwards stdin keystrokes through the SDK PTY input stream', async () => { const { deps, sockets, stdin, fetchLog, inputStreams } = createHarness(); const sessionPromise = runDriveSession('Alice', {}, deps); @@ -1260,7 +1274,7 @@ describe('runDriveSession', () => { // ---- resize forwarding (table-stakes for a take-over UX) ---- - it('forwards the local terminal size to the broker on attach', async () => { + it('reserves the final local row when sizing the agent PTY on attach', async () => { const { deps, sockets, signals, fetchLog } = createHarness({ terminalSize: { rows: 60, cols: 200 }, }); @@ -1270,7 +1284,7 @@ describe('runDriveSession', () => { const resizeCalls = fetchLog.filter((call) => call.method === 'POST' && call.url.includes('/resize/')); expect(resizeCalls).toHaveLength(1); const body = resizeCalls[0].body as { rows: number; cols: number; session_id?: string }; - expect({ rows: body.rows, cols: body.cols }).toEqual({ rows: 60, cols: 200 }); + expect({ rows: body.rows, cols: body.cols }).toEqual({ rows: 59, cols: 199 }); // The on-attach sync carries a session id for the single-resizer policy. expect(body.session_id).toEqual(expect.any(String)); @@ -1297,9 +1311,9 @@ describe('runDriveSession', () => { // First the on-attach sync, then each user-driven resize. Every resize // carries the same per-attach session id (single-resizer policy, #1247). expect(resizeBodies.map(({ rows, cols }) => ({ rows, cols }))).toEqual([ - { rows: 30, cols: 100 }, - { rows: 50, cols: 150 }, - { rows: 24, cols: 80 }, + { rows: 29, cols: 99 }, + { rows: 49, cols: 149 }, + { rows: 23, cols: 79 }, ]); const sessionIds = new Set(resizeBodies.map((b) => b.session_id)); expect(sessionIds.size).toBe(1); @@ -1344,7 +1358,37 @@ describe('runDriveSession', () => { ); expect(releaseCall).toBeDefined(); expect((releaseCall?.body as { session_id?: string }).session_id).toBe(sessionId); - // A pure release carries no placeholder dimensions (#1247). + // The release also gives back the row and column reserved for the status + // line, so the worker isn't left at 29x99 for the next `view` session. + // Carrying it on the release keeps the restore atomic with dropping + // ownership — a separate resize could land after it and re-claim (#1247). + const releaseBody = releaseCall?.body as { rows?: number; cols?: number }; + expect(releaseBody.rows).toBe(30); + expect(releaseBody.cols).toBe(100); + // And it is the only release: the restore did not add a second round-trip. + const releaseCalls = fetchLog.filter( + (call) => call.url.includes('/resize/') && (call.body as { release?: boolean }).release === true + ); + expect(releaseCalls).toHaveLength(1); + }); + + it('releases without dimensions when there is no local TTY to restore', async () => { + const { deps, sockets, signals, fetchLog } = createHarness({ terminalSize: null }); + const sessionPromise = runDriveSession('Alice', {}, deps); + await openSocket(sockets); + + await signals.get('SIGINT')?.(); + await sessionPromise; + + const releaseCall = fetchLog.find( + (call) => + call.method === 'POST' && + call.url.includes('/resize/') && + (call.body as { release?: boolean }).release === true + ); + expect(releaseCall).toBeDefined(); + // Nothing was reserved, so there is no size to restore and no placeholder + // dimensions to invent (#1247). const releaseBody = releaseCall?.body as { rows?: number; cols?: number }; expect(releaseBody.rows).toBeUndefined(); expect(releaseBody.cols).toBeUndefined(); @@ -1376,13 +1420,90 @@ describe('runDriveSession', () => { expect(sessionIds.size).toBe(1); // Re-asserts re-send the unchanged current size. for (const call of activeResizes) { - expect(call.body as { rows: number; cols: number }).toMatchObject({ rows: 30, cols: 100 }); + expect(call.body as { rows: number; cols: number }).toMatchObject({ rows: 29, cols: 99 }); } await signals.get('SIGINT')?.(); await sessionPromise; }); + it('refreshes terminal size when it changes before the event socket opens', async () => { + const { deps, sockets, terminal, signals, fetchLog, writes } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + for (let i = 0; i < 10 && sockets.length === 0; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } + terminal.setSize({ rows: 42, cols: 120 }); + await openSocket(sockets); + + const firstResize = fetchLog.find((call) => call.url.includes('/api/resize/')); + expect(firstResize?.body).toMatchObject({ rows: 41, cols: 119 }); + expect([...writes].reverse().find((write) => write.includes('[drive Alice'))).toContain('\x1b[42;1H'); + + await signals.get('SIGINT')?.(); + await sessionPromise; + }); + + it('forwards a local resize that occurs while the initial resize is still pending', async () => { + let resolveInitialResize: ((response: Response) => void) | undefined; + let resizeCalls = 0; + const { deps, sockets, terminal, signals, fetchLog, writes } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + routes: { + 'POST /resize': async () => { + resizeCalls += 1; + if (resizeCalls === 1) { + return new Promise((resolve) => { + resolveInitialResize = resolve; + }); + } + return new Response(JSON.stringify({ applied: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }, + }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + for (let i = 0; i < 10 && sockets.length === 0; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } + sockets[0]?.emit('open'); + await new Promise((resolve) => setImmediate(resolve)); + + terminal.setSize({ rows: 42, cols: 120 }); + await new Promise((resolve) => setImmediate(resolve)); + expect( + fetchLog.some( + (call) => + call.url.includes('/api/resize/') && + (call.body as { rows?: number; cols?: number }).rows === 41 && + (call.body as { rows?: number; cols?: number }).cols === 119 + ) + ).toBe(true); + + // Without this the test can pass on the SIGWINCH resize alone: if setup + // never reached the first `POST /resize`, the optional resolve below is a + // no-op and the stale path is never exercised (silent false green). + expect(resolveInitialResize).toBeTypeOf('function'); + resolveInitialResize?.( + new Response(JSON.stringify({ applied: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + for (let i = 0; i < 10; i++) await new Promise((resolve) => setImmediate(resolve)); + const activeResizes = fetchLog.filter( + (call) => call.url.includes('/api/resize/') && !(call.body as { release?: boolean }).release + ); + expect(activeResizes.at(-1)?.body).toMatchObject({ rows: 41, cols: 119 }); + expect([...writes].reverse().find((write) => write.includes('[drive Alice'))).toContain('\x1b[42;1H'); + await signals.get('SIGINT')?.(); + await sessionPromise; + }); + it('logs a rejected periodic resize ownership re-assert', async () => { let resizeCount = 0; const { deps, sockets, signals, logs } = createHarness({ @@ -1444,6 +1565,28 @@ describe('runDriveSession', () => { await sessionPromise; }); + it('falls back to repainting after ordinary output when PTY row reservation is rejected', async () => { + const { deps, sockets, writes, stdin } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + routes: { + 'POST /resize': async () => + new Response(JSON.stringify({ applied: false }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + const paintsBefore = writes.filter((write) => write.includes('[drive Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'ordinary' })); + + expect(writes.filter((write) => write.includes('[drive Alice')).length).toBeGreaterThan(paintsBefore); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + // ---- signal safety during setup (item 1) ---- it('restores the delivery mode and exits if interrupted before the session loop starts', async () => { @@ -1548,9 +1691,11 @@ describe('runDriveSession', () => { const socket = await openSocket(sockets); const paintsBefore = writes.filter((w) => w.includes('drive Alice')).length; - // Chunk ends mid-CSI (ESC [) — repainting the status here would splice - // reverse-video controls into the agent's half-sent sequence. + // Chunk ends mid-CSI (ESC [), then a pending-count change requests a + // repaint. Painting now would splice reverse-video controls into the + // agent's half-sent sequence. socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'data\x1b[' })); + socket.emit('message', jsonMessage({ kind: 'delivery_queued', name: 'Alice', event_id: 'e1' })); expect(writes.filter((w) => w.includes('drive Alice')).length).toBe(paintsBefore); // Completing the CSI lands at a boundary → the deferred repaint fires. socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '2J' })); @@ -1560,6 +1705,219 @@ describe('runDriveSession', () => { await sessionPromise; }); + it('tracks the bytes predictive echo actually writes before releasing a held repaint', async () => { + const { deps, sockets, writes, stdin } = createHarness(); + let predictiveWrite: ((chunk: string) => void) | undefined; + const pendingOutputs: Array<() => void> = []; + deps.createPredictiveEcho = vi.fn((opts) => { + predictiveWrite = opts.write; + return { + seed: async () => undefined, + onUserInput: () => undefined, + onServerOutput: (chunk: string) => + new Promise((resolve) => { + opts.write(chunk); + pendingOutputs.push(resolve); + }), + rollback: () => undefined, + onResize: () => undefined, + reset: () => undefined, + }; + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + const paintsBefore = writes.filter((write) => write.includes('[drive Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'data\x1b[' })); + socket.emit('message', jsonMessage({ kind: 'delivery_queued', name: 'Alice', event_id: 'e1' })); + expect(writes.filter((write) => write.includes('[drive Alice'))).toHaveLength(paintsBefore); + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '2J' })); + const completionIndex = writes.lastIndexOf('2J'); + const repaintIndex = writes.findIndex( + (write, index) => index > completionIndex && write.includes('[drive Alice') + ); + expect(predictiveWrite).toBeTypeOf('function'); + expect(completionIndex).toBeGreaterThanOrEqual(0); + expect(repaintIndex).toBeGreaterThan(completionIndex); + + for (const resolve of pendingOutputs) resolve(); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + + it('repaints the clipped reserved status row after worker stream chunks', async () => { + const { deps, sockets, writes, stdin } = createHarness(); + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + const paintsBefore = writes.filter((w) => w.includes('drive Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'frame one' })); + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'frame two' })); + + expect(writes.filter((w) => w.includes('drive Alice')).length).toBeGreaterThan(paintsBefore); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + + it.each([80, 66, 40])( + 'keeps a full-screen TUI and the Relay status bar on separate rows at %i columns', + async (cols) => { + const terminal = new Terminal({ cols, rows: 10, allowProposedApi: true }); + const { deps, sockets, stdin, writes } = createHarness({ + terminalSize: { rows: 10, cols }, + }); + const writeTerminal = (chunk: string): void => { + writes.push(chunk); + terminal.write(chunk); + }; + deps.writeChunk = writeTerminal; + deps.captureAndRenderSnapshot = vi.fn(async (_connection, _name, snapshotDeps) => { + snapshotDeps.writeChunk('\x1b[2J\x1b[HAgent frame\x1b[9;1HAgent bottom'); + return { status: 'ok', rows: 9, cols, offset: 32 }; + }); + + const flushTerminal = () => + new Promise((resolve) => { + terminal.write('', resolve); + }); + const line = (row: number) => + terminal.buffer.active.getLine(terminal.buffer.active.viewportY + row - 1)?.translateToString(true) ?? + ''; + + try { + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + await flushTerminal(); + + expect(line(9)).toContain('Agent bottom'); + expect(line(10)).toContain('[drive Alice'); + + socket.emit( + 'message', + jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '\x1b[3;8r\x1b[?6h' }) + ); + const marginAwareStatus = [...writes].reverse().find((write) => write.includes('[drive Alice')); + expect(marginAwareStatus).toContain('\x1b[3;8r'); + expect(marginAwareStatus).toContain('\x1b[?6h'); + socket.emit( + 'message', + jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '\x1b[?6l\x1b[r' }) + ); + + // Plain newlines and autowrap scroll only the child's DECSTBM region; + // neither is cursor-addressed, so this catches the physical-row leak + // that a PTY resize alone cannot prevent. + socket.emit( + 'message', + jsonMessage({ + kind: 'worker_stream', + name: 'Alice', + chunk: `\x1b[9;1H${'wrapped '.repeat(20)}\r\nline two\r\nline three`, + }) + ); + await flushTerminal(); + expect(line(10)).toContain('[drive Alice'); + + // A normal cursor-addressed update stays inside the 9-row child grid and + // does not duplicate or displace Relay's reserved row. + socket.emit( + 'message', + jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '\x1b[5;1HTurn update' }) + ); + await flushTerminal(); + expect(Array.from({ length: 10 }, (_, index) => line(index + 1)).join('\n')).toContain('Turn update'); + expect(line(10)).toContain('[drive Alice'); + + // A full-screen erase clears the local buffer too; the invalidation + // scanner restores exactly one status bar after the new frame lands. + socket.emit( + 'message', + jsonMessage({ + kind: 'worker_stream', + name: 'Alice', + chunk: '\x1b[2J\x1b[HNext frame\x1b[9;1HNext bottom', + }) + ); + await flushTerminal(); + const finalScreen = Array.from({ length: 10 }, (_, index) => line(index + 1)); + expect(finalScreen.join('\n')).toContain('Next frame'); + expect(line(9)).toContain('Next bottom'); + expect(line(10)).toContain('[drive Alice'); + const statusRows = finalScreen.filter((value) => value.includes('[drive Alice')); + expect(statusRows).toHaveLength(1); + + stdin.type(Buffer.from([0x03])); + await sessionPromise; + } finally { + terminal.dispose(); + } + } + ); + + it.each([1, 2])('does not reserve or paint status in a %i-row terminal', async (rows) => { + const { deps, sockets, writes, stdin, fetchLog } = createHarness({ + terminalSize: { rows, cols: 80 }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + await openSocket(sockets); + + expect(writes.some((write) => write.includes('[drive Alice'))).toBe(false); + const resize = fetchLog.find((call) => call.method === 'POST' && call.url.includes('/api/resize/')); + expect(resize?.body).toMatchObject({ rows, cols: 80 }); + + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + + it('disables status painting when a large terminal shrinks to one row', async () => { + const { deps, sockets, writes, stdin, terminal, fetchLog } = createHarness({ + terminalSize: { rows: 10, cols: 80 }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + const socket = await openSocket(sockets); + terminal.setSize({ rows: 1, cols: 80 }); + const paintsAfterShrink = writes.filter((write) => write.includes('[drive Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'only row' })); + + expect(writes.filter((write) => write.includes('[drive Alice'))).toHaveLength(paintsAfterShrink); + expect( + fetchLog.some( + (call) => + call.url.includes('/api/resize/') && + (call.body as { rows?: number; cols?: number }).rows === 1 && + (call.body as { rows?: number; cols?: number }).cols === 80 + ) + ).toBe(true); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + expect(writes).toContain(LOCAL_TERMINAL_RESET_SEQUENCE); + }); + + it('activates row reservation when a one-row terminal grows', async () => { + const { deps, sockets, writes, stdin, terminal, fetchLog } = createHarness({ + terminalSize: { rows: 1, cols: 80 }, + }); + const sessionPromise = runDriveSession('Alice', {}, deps); + await openSocket(sockets); + expect(writes.some((write) => write.includes('[drive Alice'))).toBe(false); + + terminal.setSize({ rows: 10, cols: 80 }); + + expect(writes.some((write) => write.includes('[drive Alice'))).toBe(true); + expect( + fetchLog.some( + (call) => + call.url.includes('/api/resize/') && + (call.body as { rows?: number; cols?: number }).rows === 9 && + (call.body as { rows?: number; cols?: number }).cols === 79 + ) + ).toBe(true); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + // ---- non-TTY skips the status line (item 5) ---- it('skips status-line painting when stdout is not a TTY', async () => { diff --git a/packages/cli/src/cli/lib/attach-drive.ts b/packages/cli/src/cli/lib/attach-drive.ts index eee4860bf..4274764b1 100644 --- a/packages/cli/src/cli/lib/attach-drive.ts +++ b/packages/cli/src/cli/lib/attach-drive.ts @@ -52,18 +52,22 @@ import WebSocket from 'ws'; import { captureAndRenderSnapshot, + canReserveStatusLine, clampStatusLineText, createBackpressureAwareWriter, DETACH_CLEANUP_DEADLINE_MS, pickInitialTerminalCols, pickInitialTerminalRows, prepareAttachTarget, + reserveStatusLineRow, + renderChildScrollRegion, resetLocalTerminalOnDetach, restoreInboundDeliveryModeOnDetach, StatusLineController, StreamSyncBuffer, switchInboundDeliveryModeOrAbort, syncInitialPtySize, + TerminalScrollRegionTracker, type AttachSnapshotConnection, type AttachSnapshotDeps, } from '../lib/attach.js'; @@ -392,10 +396,10 @@ export async function resizeWorker( cols: number, fetchFn: typeof globalThis.fetch, options?: { sessionId?: string } -): Promise<{ ok: boolean; message?: string }> { +): Promise<{ ok: boolean; message?: string; applied?: boolean }> { try { - await createBrokerClient(connection, fetchFn).resizePty(name, rows, cols, options); - return { ok: true }; + const result = await createBrokerClient(connection, fetchFn).resizePty(name, rows, cols, options); + return { ok: true, applied: result.applied !== false }; } catch (err: unknown) { const failure = mapBrokerSdkFailure(err); return { ok: false, message: failure.message }; @@ -406,17 +410,26 @@ export async function resizeWorker( * Release this session's PTY resize ownership on detach (single-resizer * policy, #1247), so the next client that attaches can resize the shared PTY. * Best-effort: the broker also supersedes a crashed owner after an idle window. + * + * `restoreSize` gives back the row and column a writable attach reserved for + * Relay's status line (see `reserveStatusLineRow`). Without it the worker stays + * at `rows - 1`/`cols - 1` after the status line disappears, and since a + * read-only `view` session never resizes the PTY, the agent's TUI would remain + * one row and column short until the next writable attach. The broker applies + * the size before dropping ownership, so this stays one round-trip and cannot + * lose the ordering race a separate resize call would introduce. */ export async function releaseResizeOwnership( connection: BrokerConnection, name: string, sessionId: string, - fetchFn: typeof globalThis.fetch + fetchFn: typeof globalThis.fetch, + restoreSize?: { rows: number; cols: number } | null ): Promise { try { - // A pure release carries no dimensions — the broker skips the resize and - // only drops ownership, so there are no placeholder sizes to invent. - await createBrokerClient(connection, fetchFn).resizePty(name, undefined, undefined, { + // Omitting the dimensions is a pure release — the broker skips the resize, + // so a session with no local TTY invents no placeholder size. + await createBrokerClient(connection, fetchFn).resizePty(name, restoreSize?.rows, restoreSize?.cols, { sessionId, release: true, }); @@ -547,8 +560,13 @@ export function renderStatusLine(opts: { rows?: number; /** Terminal columns — the label is truncated to fit. Defaults to 80. */ cols?: number; + scrollTop?: number; + scrollBottom?: number; + originMode?: boolean; }): string { const row = Math.max(opts.rows ?? 24, 1); + const scrollTop = Math.max(1, opts.scrollTop ?? 1); + const scrollBottom = Math.max(scrollTop + 1, opts.scrollBottom ?? row - 1); // The Ctrl+] hint names the action the NEXT press performs: in manual_flush // it delivers (drains the parked queue and goes live); in auto_inject it // re-holds. Without the hint, a parked message is invisible beyond the @@ -556,11 +574,16 @@ export function renderStatusLine(opts: { const toggleHint = opts.mode === 'manual_flush' ? 'Ctrl+] deliver' : 'Ctrl+] hold'; const text = clampStatusLineText( `[drive ${opts.name} | delivery=${opts.mode} | pending=${opts.pending} | ${toggleHint} | Ctrl+C detach]`, - opts.cols + opts.cols, + true ); // ESC 7 = save cursor; ESC[;1H = move to bottom row; ESC[2K = clear line; // ESC[7m = reverse video; ESC[0m = reset; ESC 8 = restore cursor. - return `\x1b7\x1b[${row};1H\x1b[2K\x1b[7m${text}\x1b[0m\x1b8`; + // Temporarily restore the full physical scroll region so CUP can reach the + // reserved row even if autowrap left the cursor below the child margin. + // Reinstall the child margin before restoring its cursor. + const restoreOrigin = opts.originMode ? '\x1b[?6h' : ''; + return `\x1b7\x1b[?6l\x1b[r\x1b[${row};1H\x1b[2K\x1b[7m${text}\x1b[0m\x1b[${scrollTop};${scrollBottom}r${restoreOrigin}\x1b8`; } /** ----- Main session runner ----- */ @@ -655,9 +678,10 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): const inputDecoder = new StringDecoder('utf8'); let inputStream: CliPtyInputStream | null = null; const cleanupSignals: Array<() => void> = []; + const isTtyOutput = state.initialLocalSize !== null; // Skip the status line entirely when stdout is not a TTY (e.g. piped to // `tee`) — a fabricated row-24 repaint would corrupt the captured log. - const statusLineEnabled = state.initialLocalSize !== null; + let statusLineEnabled = canReserveStatusLine(state.initialLocalSize); // Subscribe-first: buffer live `worker_stream` chunks until the snapshot // is painted and reconciled against its per-worker offset. const sync = new StreamSyncBuffer(); @@ -665,11 +689,17 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): // Adaptive predictive echo masks round-trip latency on remote brokers. // Seeded with the snapshot (once painted) so its confirmed model matches // the screen. + let initialAgentSize = reserveStatusLineRow(state.initialLocalSize); + const scrollRegion = new TerminalScrollRegionTracker(initialAgentSize?.rows ?? 1); + let observeRenderedOutput = (_chunk: string): void => {}; const predictiveEcho = deps.createPredictiveEcho?.({ - cols: state.initialLocalSize?.cols ?? 0, - rows: state.initialLocalSize?.rows ?? 0, - write: deps.writeChunk, + cols: initialAgentSize?.cols ?? 0, + rows: initialAgentSize?.rows ?? 0, + write: (chunk) => { + deps.writeChunk(chunk); + observeRenderedOutput(chunk); + }, getInputSrtt: () => inputStream?.srttMs ?? null, }) ?? null; @@ -686,30 +716,85 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): snapshotBytes += chunk; }; + const correctSetupResize = async (baseline: { rows: number; cols: number } | null): Promise => { + const latest = reserveStatusLineRow(deps.terminal.getSize()); + if (!latest || (latest.rows === baseline?.rows && latest.cols === baseline?.cols)) return; + const correction = resizeWorker(connection, name, latest.rows, latest.cols, deps.fetch, { + sessionId: resizeSessionId, + }); + trackResize(correction); + const result = await correction; + if (!result.ok) { + deps.log(`[drive] setup resize correction failed: ${result.message ?? 'unknown error'}`); + } + }; + + const beginSubscribedLayout = (): void => { + // Install this before the first resize/snapshot await so a local resize + // during setup cannot be lost. + unsubscribeResize ??= deps.terminal.onResize(resizeHandler); + const currentSize = deps.terminal.getSize(); + terminalRows = pickInitialTerminalRows(currentSize, undefined); + terminalCols = currentSize?.cols; + statusLineEnabled = canReserveStatusLine(currentSize); + initialAgentSize = reserveStatusLineRow(currentSize); + if (initialAgentSize) { + scrollRegion.setRows(initialAgentSize.rows); + predictiveEcho?.onResize(initialAgentSize.cols, initialAgentSize.rows); + } + if (statusLineEnabled && initialAgentSize) { + deps.writeChunk(renderChildScrollRegion(initialAgentSize.rows)); + } + }; + // Boundary-held + coalesced status painter. Holds repaints while the agent // is mid escape-sequence (no splicing into a half-sent CSI), rate-limits // per-chunk repaints, and skips painting entirely on a non-TTY stdout. const statusController = new StatusLineController({ - render: () => - renderStatusLine({ name, mode: currentMode, pending, rows: terminalRows, cols: terminalCols }), + render: () => { + const region = scrollRegion.region; + return renderStatusLine({ + name, + mode: currentMode, + pending, + rows: terminalRows, + cols: terminalCols, + scrollTop: region.top, + scrollBottom: region.bottom, + originMode: scrollRegion.isOriginMode, + }); + }, write: deps.writeChunk, - enabled: statusLineEnabled, + enabled: () => statusLineEnabled, coalesceMs: deps.statusRepaintCoalesceMs ?? 40, }); const paintStatus = (): void => { statusController.request(); }; + observeRenderedOutput = (chunk): void => { + scrollRegion.push(chunk); + statusController.observeOutput(chunk); + }; // Route server output through the predictive-echo engine (which owns - // cursor save/restore) or straight to stdout, then repaint the status. - // Feed every chunk to the status controller for boundary tracking so the - // repaint holds off until the stream is back at a sequence boundary. + // cursor save/restore) or straight to stdout. Feed every chunk to the + // status controller for boundary tracking. Repaint after each completed + // chunk because terminal autowrap can briefly cross a DECSTBM bottom + // margin. The clipped label cannot autowrap itself, and the child PTY's + // reserved row prevents cursor-addressed TUI frames from fighting it. const applyServerOutput = (chunk: string): void => { - statusController.observeOutput(chunk); if (predictiveEcho) { - void predictiveEcho.onServerOutput(chunk).then(paintStatus, paintStatus); + void predictiveEcho.onServerOutput(chunk).then( + () => { + paintStatus(); + }, + () => { + paintStatus(); + } + ); } else { deps.writeChunk(chunk); + observeRenderedOutput(chunk); paintStatus(); } }; @@ -724,13 +809,19 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): if (!size) return; terminalRows = size.rows; terminalCols = size.cols; - predictiveEcho?.onResize(size.cols, size.rows); + statusLineEnabled = canReserveStatusLine(size); + const agentSize = reserveStatusLineRow(size); + if (!agentSize) return; + scrollRegion.setRows(agentSize.rows); + predictiveEcho?.onResize(agentSize.cols, agentSize.rows); trackResize( - resizeWorker(connection, name, size.rows, size.cols, deps.fetch, { + resizeWorker(connection, name, agentSize.rows, agentSize.cols, deps.fetch, { sessionId: resizeSessionId, }).then((res) => { if (!res.ok) { deps.log(`[drive] resize forward failed: ${res.message ?? 'unknown error'}`); + } else if (res.applied === false) { + deps.log('[drive] broker did not apply the reserved PTY size; using status repaint fallback'); } }) ); @@ -868,7 +959,7 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): // Heal the local terminal: the snapshot + live stream may have left it // in app-cursor / mouse / bracketed-paste / alt-screen mode. Gate on a // TTY stdout (same signal that gates the status line). - resetLocalTerminalOnDetach(deps.writeChunk, statusLineEnabled); + resetLocalTerminalOnDetach(deps.writeChunk, isTtyOutput); } catch { // best effort } @@ -934,7 +1025,15 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): if (outstandingResizes.size > 0) { await Promise.allSettled([...outstandingResizes]); } - await releaseResizeOwnership(connection, name, resizeSessionId, deps.fetch); + // Hand the reserved status row/column back in the same request, so + // the agent's TUI is not left one row and column short. + await releaseResizeOwnership( + connection, + name, + resizeSessionId, + deps.fetch, + deps.terminal.getSize() + ); } catch { // Best-effort — the broker's idle-takeover net still frees ownership. } @@ -1031,7 +1130,6 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): // Subscribe to local-terminal resize events at the same point // we take over stdin so the lifecycles match — both go away in // `teardownStdin` on any exit path. - unsubscribeResize = deps.terminal.onResize(resizeHandler); // Start the periodic ownership re-assert so an idle-but-live session // keeps the single-resizer lease past the broker's stale window. The // broker no-ops a same-size re-assert (no SIGWINCH/repaint). @@ -1039,9 +1137,10 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): if (reassertMs > 0) { reassertTimer = setInterval(() => { const size = deps.terminal.getSize() ?? state.initialLocalSize; - if (!size) return; + const agentSize = reserveStatusLineRow(size); + if (!agentSize) return; trackResize( - resizeWorker(connection, name, size.rows, size.cols, deps.fetch, { + resizeWorker(connection, name, agentSize.rows, agentSize.cols, deps.fetch, { sessionId: resizeSessionId, }).then((res) => { if (!res.ok) { @@ -1061,13 +1160,23 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): } }; - // Runs once the event WS is subscribed. Take over stdin before replaying - // the snapshot: a source TUI can enable mouse/focus/alternate-scroll - // reporting in that replay, and cooked-mode stdin would echo those reports - // as visible escape text until the later input-stream setup completed. + // Runs once the event WS is subscribed: first reserve the local bottom row + // and take over stdin before replaying the snapshot. A source TUI can + // enable mouse/focus/alternate-scroll reporting in that replay, and + // cooked-mode stdin would echo those reports as visible escape text. + // The WS is already buffering, so the resize repaint is reconciled against + // the snapshot without a dead zone. const onSubscribed = async (): Promise => { + beginSubscribedLayout(); await openInputStreamAndSetRawMode(); if (settled) return; + const initialResize = syncInitialPtySize(connection, name, initialAgentSize, 'drive', deps, { + sessionId: resizeSessionId, + }); + trackResize(initialResize); + await initialResize; + if (settled) return; + await correctSetupResize(initialAgentSize); const snapshot = await deps.captureAndRenderSnapshot( { url: connection.url, apiKey: connection.apiKey }, name, @@ -1103,9 +1212,11 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): } if (settled) return; } - terminalRows = pickInitialTerminalRows(state.initialLocalSize, snapshot.rows); - terminalCols = pickInitialTerminalCols(state.initialLocalSize, snapshot.cols); + const currentLocalSize = deps.terminal.getSize(); + terminalRows = pickInitialTerminalRows(currentLocalSize, snapshot.rows); + terminalCols = pickInitialTerminalCols(currentLocalSize, snapshot.cols); // Track the snapshot bytes for boundary state before the first repaint. + scrollRegion.push(snapshotBytes); statusController.observeOutput(snapshotBytes); paintStatus(); // Reconcile buffered chunks. On `ok`, drop what the snapshot already @@ -1114,12 +1225,6 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies): // transient snapshot failure nothing was painted, so apply everything. const pendingChunks = snapshot.status === 'ok' ? sync.reconcile(snapshot.offset) : sync.flushAll(); for (const chunk of pendingChunks) applyServerOutput(chunk); - const initialResize = syncInitialPtySize(connection, name, state.initialLocalSize, 'drive', deps, { - sessionId: resizeSessionId, - }); - trackResize(initialResize); - await initialResize; - if (settled) return; // Input forwarding starts only after predictive echo has been seeded by // the snapshot. Raw mode was already enabled above, so terminal reports // could not echo during the setup window. diff --git a/packages/cli/src/cli/lib/attach-passthrough.test.ts b/packages/cli/src/cli/lib/attach-passthrough.test.ts index 4772a2a69..f0ee36178 100644 --- a/packages/cli/src/cli/lib/attach-passthrough.test.ts +++ b/packages/cli/src/cli/lib/attach-passthrough.test.ts @@ -2,6 +2,7 @@ import { Buffer } from 'node:buffer'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { LOCAL_TERMINAL_RESET_SEQUENCE } from './attach.js'; import type { CliPtyInputStream } from './attach-drive.js'; import { PassthroughKeybindParser, @@ -501,9 +502,9 @@ describe('renderStatusLine', () => { it('truncates the label to the terminal width so it can never wrap', () => { const cols = 40; // eslint-disable-next-line no-control-regex -- matching the raw ESC bytes this module emits - const strip = (s: string) => s.replace(/\x1b(?:[78]|\[[0-9;]*[A-Za-z])/g, ''); + const strip = (s: string) => s.replace(/\x1b(?:[78]|\[[?0-9;]*[A-Za-z])/g, ''); const text = strip(renderStatusLine({ name: 'Gamemaster', mode: 'auto_inject', rows: 24, cols })); - expect(text.length).toBeLessThanOrEqual(cols); + expect(text.length).toBeLessThan(cols); expect(text).toContain('[passthrough'); expect(text).toContain('detach]'); }); @@ -675,20 +676,33 @@ describe('runPassthroughSession', () => { await sessionPromise; }); - it('writes worker_stream chunks to stdout and repaints the status line', async () => { + it('writes worker_stream chunks and safely restores the reserved status row', async () => { const { deps, sockets, writes, stdin } = createHarness(); const sessionPromise = runPassthroughSession('Alice', {}, deps); const socket = await openSocket(sockets); socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'live output' })); expect(writes.includes('live output')).toBe(true); - const liveIdx = writes.indexOf('live output'); - const repaintAfter = writes.slice(liveIdx + 1).some((w) => w.includes('passthrough Alice')); - expect(repaintAfter).toBe(true); + const paintsAfter = writes.filter((w) => w.includes('passthrough Alice')).length; + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'more output' })); + expect(writes.filter((w) => w.includes('passthrough Alice')).length).toBeGreaterThan(paintsAfter); stdin.type(Buffer.from([0x03])); await sessionPromise; }); + it('repaints after worker output erases the full display', async () => { + const { deps, sockets, writes, stdin } = createHarness(); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + const socket = await openSocket(sockets); + const paintsBefore = writes.filter((w) => w.includes('passthrough Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '\x1b[2J' })); + + expect(writes.filter((w) => w.includes('passthrough Alice')).length).toBeGreaterThan(paintsBefore); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + it('forwards stdin keystrokes through the SDK PTY input stream', async () => { const { deps, sockets, stdin, fetchLog, inputStreams } = createHarness(); const sessionPromise = runPassthroughSession('Alice', {}, deps); @@ -811,7 +825,7 @@ describe('runPassthroughSession', () => { } }); - it('forwards the local terminal size to the broker on attach', async () => { + it('reserves the final local row when sizing the agent PTY on attach', async () => { const { deps, sockets, signals, fetchLog } = createHarness({ terminalSize: { rows: 60, cols: 200 }, }); @@ -821,7 +835,7 @@ describe('runPassthroughSession', () => { const resizeCalls = fetchLog.filter((c) => c.method === 'POST' && c.url.includes('/resize/')); expect(resizeCalls).toHaveLength(1); const body = resizeCalls[0].body as { rows: number; cols: number; session_id?: string }; - expect({ rows: body.rows, cols: body.cols }).toEqual({ rows: 60, cols: 200 }); + expect({ rows: body.rows, cols: body.cols }).toEqual({ rows: 59, cols: 199 }); // The on-attach sync carries a session id for the single-resizer policy. expect(body.session_id).toEqual(expect.any(String)); @@ -829,6 +843,79 @@ describe('runPassthroughSession', () => { await sessionPromise; }); + it('refreshes terminal size when it changes before the event socket opens', async () => { + const { deps, sockets, terminal, signals, fetchLog, writes } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + }); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + for (let i = 0; i < 10 && sockets.length === 0; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } + terminal.setSize({ rows: 42, cols: 120 }); + await openSocket(sockets); + + const firstResize = fetchLog.find((call) => call.url.includes('/api/resize/')); + expect(firstResize?.body).toMatchObject({ rows: 41, cols: 119 }); + expect([...writes].reverse().find((write) => write.includes('[passthrough Alice'))).toContain( + '\x1b[42;1H' + ); + + await signals.get('SIGINT')?.(); + await sessionPromise; + }); + + it('reapplies the latest size after a stale initial resize completes', async () => { + let resolveInitialResize: ((response: Response) => void) | undefined; + let resizeCalls = 0; + const { deps, sockets, terminal, signals, fetchLog, writes } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + routes: { + 'POST /resize': async () => { + resizeCalls += 1; + if (resizeCalls === 1) { + return new Promise((resolve) => { + resolveInitialResize = resolve; + }); + } + return new Response(JSON.stringify({ applied: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }, + }, + }); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + for (let i = 0; i < 10 && sockets.length === 0; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } + sockets[0]?.emit('open'); + await new Promise((resolve) => setImmediate(resolve)); + terminal.setSize({ rows: 42, cols: 120 }); + await new Promise((resolve) => setImmediate(resolve)); + // Without this the test can pass on the SIGWINCH resize alone: if setup + // never reached the first `POST /resize`, the optional resolve below is a + // no-op and the stale path is never exercised (silent false green). + expect(resolveInitialResize).toBeTypeOf('function'); + resolveInitialResize?.( + new Response(JSON.stringify({ applied: true }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + for (let i = 0; i < 10; i++) await new Promise((resolve) => setImmediate(resolve)); + + const activeResizes = fetchLog.filter( + (call) => call.url.includes('/api/resize/') && !(call.body as { release?: boolean }).release + ); + expect(activeResizes.at(-1)?.body).toMatchObject({ rows: 41, cols: 119 }); + expect([...writes].reverse().find((write) => write.includes('[passthrough Alice'))).toContain( + '\x1b[42;1H' + ); + + await signals.get('SIGINT')?.(); + await sessionPromise; + }); + it('waits for the initial resize before releasing ownership on detach', async () => { let finishInitialResize: ((response: Response) => void) | undefined; const { deps, sockets, signals, fetchLog } = createHarness({ @@ -875,6 +962,32 @@ describe('runPassthroughSession', () => { expect(resizeBodies.map((body) => body.release === true)).toEqual([false, true]); }); + it('restores the reserved row and column on the release itself', async () => { + const { deps, sockets, signals, fetchLog } = createHarness({ + terminalSize: { rows: 30, cols: 100 }, + }); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + await openSocket(sockets); + + // The attach sizes the worker one row and column short to reserve the + // status line. + const attachResize = fetchLog.find((call) => call.method === 'POST' && call.url.includes('/resize/')); + expect(attachResize?.body).toMatchObject({ rows: 29, cols: 99 }); + + await signals.get('SIGINT')?.(); + await sessionPromise; + + // Detach hands the reserved row and column back on the release request, so + // a later read-only `view` session doesn't inherit a short PTY. Doing it on + // the release keeps it atomic: a separate resize could land afterwards and + // re-claim ownership (#1247). + const releaseCalls = fetchLog.filter( + (call) => call.url.includes('/resize/') && (call.body as { release?: boolean }).release === true + ); + expect(releaseCalls).toHaveLength(1); + expect(releaseCalls[0]?.body).toMatchObject({ rows: 30, cols: 100 }); + }); + // ---- predictive-echo wiring ---- it('seeds the engine with the snapshot and routes input + output through it', async () => { @@ -954,13 +1067,18 @@ describe('runPassthroughSession', () => { // ---- status line boundary-hold (item 4) ---- it('holds the status repaint while a worker chunk ends mid escape sequence', async () => { - const { deps, sockets, writes, stdin } = createHarness(); + const { deps, sockets, writes, stdin, terminal } = createHarness(); const sessionPromise = runPassthroughSession('Alice', {}, deps); const socket = await openSocket(sockets); const paintsBefore = writes.filter((w) => w.includes('passthrough Alice')).length; socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'data\x1b[' })); + const writesBeforeResize = writes.length; + // A local resize requests a status repaint, but it must wait until the + // worker completes its split CSI sequence. + terminal.setSize({ rows: 31, cols: 100 }); expect(writes.filter((w) => w.includes('passthrough Alice')).length).toBe(paintsBefore); + expect(writes).toHaveLength(writesBeforeResize); socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: '2J' })); expect(writes.filter((w) => w.includes('passthrough Alice')).length).toBeGreaterThan(paintsBefore); @@ -968,6 +1086,54 @@ describe('runPassthroughSession', () => { await sessionPromise; }); + it.each([1, 2])('disables status painting when a large terminal shrinks to %i rows', async (rows) => { + const { deps, sockets, writes, stdin, terminal, fetchLog } = createHarness({ + terminalSize: { rows: 10, cols: 80 }, + }); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + const socket = await openSocket(sockets); + terminal.setSize({ rows, cols: 80 }); + const paintsAfterShrink = writes.filter((write) => write.includes('[passthrough Alice')).length; + + socket.emit('message', jsonMessage({ kind: 'worker_stream', name: 'Alice', chunk: 'only row' })); + + expect(writes.filter((write) => write.includes('[passthrough Alice'))).toHaveLength(paintsAfterShrink); + expect( + fetchLog.some( + (call) => + call.url.includes('/api/resize/') && + (call.body as { rows?: number; cols?: number }).rows === rows && + (call.body as { rows?: number; cols?: number }).cols === 80 + ) + ).toBe(true); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + expect(writes).toContain(LOCAL_TERMINAL_RESET_SEQUENCE); + }); + + it.each([1, 2])('activates row reservation when a %i-row terminal grows', async (rows) => { + const { deps, sockets, writes, stdin, terminal, fetchLog } = createHarness({ + terminalSize: { rows, cols: 80 }, + }); + const sessionPromise = runPassthroughSession('Alice', {}, deps); + await openSocket(sockets); + expect(writes.some((write) => write.includes('[passthrough Alice'))).toBe(false); + + terminal.setSize({ rows: 10, cols: 80 }); + + expect(writes.some((write) => write.includes('[passthrough Alice'))).toBe(true); + expect( + fetchLog.some( + (call) => + call.url.includes('/api/resize/') && + (call.body as { rows?: number; cols?: number }).rows === 9 && + (call.body as { rows?: number; cols?: number }).cols === 79 + ) + ).toBe(true); + stdin.type(Buffer.from([0x03])); + await sessionPromise; + }); + // ---- non-TTY skips the status line (item 5) ---- it('skips status-line painting when stdout is not a TTY', async () => { diff --git a/packages/cli/src/cli/lib/attach-passthrough.ts b/packages/cli/src/cli/lib/attach-passthrough.ts index dfc1219ea..39baeccf0 100644 --- a/packages/cli/src/cli/lib/attach-passthrough.ts +++ b/packages/cli/src/cli/lib/attach-passthrough.ts @@ -31,18 +31,22 @@ import WebSocket from 'ws'; import { captureAndRenderSnapshot, + canReserveStatusLine, clampStatusLineText, createBackpressureAwareWriter, DETACH_CLEANUP_DEADLINE_MS, pickInitialTerminalCols, pickInitialTerminalRows, prepareAttachTarget, + reserveStatusLineRow, + renderChildScrollRegion, resetLocalTerminalOnDetach, restoreInboundDeliveryModeOnDetach, StatusLineController, StreamSyncBuffer, switchInboundDeliveryModeOrAbort, syncInitialPtySize, + TerminalScrollRegionTracker, type AttachSnapshotConnection, type AttachSnapshotDeps, } from '../lib/attach.js'; @@ -274,13 +278,20 @@ export function renderStatusLine(opts: { rows?: number; /** Terminal columns — the label is truncated to fit. Defaults to 80. */ cols?: number; + scrollTop?: number; + scrollBottom?: number; + originMode?: boolean; }): string { const row = Math.max(opts.rows ?? 24, 1); + const scrollTop = Math.max(1, opts.scrollTop ?? 1); + const scrollBottom = Math.max(scrollTop + 1, opts.scrollBottom ?? row - 1); const text = clampStatusLineText( `[passthrough ${opts.name} | delivery=${opts.mode} | Ctrl+C detach]`, - opts.cols + opts.cols, + true ); - return `\x1b7\x1b[${row};1H\x1b[2K\x1b[7m${text}\x1b[0m\x1b8`; + const restoreOrigin = opts.originMode ? '\x1b[?6h' : ''; + return `\x1b7\x1b[?6l\x1b[r\x1b[${row};1H\x1b[2K\x1b[7m${text}\x1b[0m\x1b[${scrollTop};${scrollBottom}r${restoreOrigin}\x1b8`; } /** ----- Main session runner ----- */ @@ -384,8 +395,9 @@ export async function runPassthroughSession( const inputDecoder = new StringDecoder('utf8'); let inputStream: CliPtyInputStream | null = null; const cleanupSignals: Array<() => void> = []; + const isTtyOutput = initialLocalSize !== null; // Skip the status line entirely when stdout is not a TTY (piped output). - const statusLineEnabled = initialLocalSize !== null; + let statusLineEnabled = canReserveStatusLine(initialLocalSize); // Subscribe-first: buffer live `worker_stream` chunks until the snapshot // is painted and reconciled against its per-worker offset (no lost/dup // output around attach time). See StreamSyncBuffer. @@ -396,11 +408,17 @@ export async function runPassthroughSession( // Adaptive predictive echo masks round-trip latency on remote brokers. // Seeded with the snapshot (after it is painted) so its confirmed model // matches the screen. + let initialAgentSize = reserveStatusLineRow(initialLocalSize); + const scrollRegion = new TerminalScrollRegionTracker(initialAgentSize?.rows ?? 1); + let observeRenderedOutput = (_chunk: string): void => {}; const predictiveEcho = deps.createPredictiveEcho?.({ - cols: initialLocalSize?.cols ?? 0, - rows: initialLocalSize?.rows ?? 0, - write: deps.writeChunk, + cols: initialAgentSize?.cols ?? 0, + rows: initialAgentSize?.rows ?? 0, + write: (chunk) => { + deps.writeChunk(chunk); + observeRenderedOutput(chunk); + }, getInputSrtt: () => inputStream?.srttMs ?? null, }) ?? null; @@ -417,25 +435,78 @@ export async function runPassthroughSession( snapshotBytes += chunk; }; + const correctSetupResize = async (baseline: { rows: number; cols: number } | null): Promise => { + const latest = reserveStatusLineRow(deps.terminal.getSize()); + if (!latest || (latest.rows === baseline?.rows && latest.cols === baseline?.cols)) return; + const correction = resizeWorker(connection, name, latest.rows, latest.cols, deps.fetch, { + sessionId: resizeSessionId, + }); + trackResize(correction); + const result = await correction; + if (!result.ok) { + deps.log(`[passthrough] setup resize correction failed: ${result.message ?? 'unknown error'}`); + } + }; + + const beginSubscribedLayout = (): void => { + unsubscribeResize ??= deps.terminal.onResize(resizeHandler); + const currentSize = deps.terminal.getSize(); + terminalRows = pickInitialTerminalRows(currentSize, undefined); + terminalCols = currentSize?.cols; + statusLineEnabled = canReserveStatusLine(currentSize); + initialAgentSize = reserveStatusLineRow(currentSize); + if (initialAgentSize) { + scrollRegion.setRows(initialAgentSize.rows); + predictiveEcho?.onResize(initialAgentSize.cols, initialAgentSize.rows); + } + if (statusLineEnabled && initialAgentSize) { + deps.writeChunk(renderChildScrollRegion(initialAgentSize.rows)); + } + }; + // Boundary-held + coalesced status painter (skips non-TTY stdout). const statusController = new StatusLineController({ - render: () => renderStatusLine({ name, mode: 'auto_inject', rows: terminalRows, cols: terminalCols }), + render: () => { + const region = scrollRegion.region; + return renderStatusLine({ + name, + mode: 'auto_inject', + rows: terminalRows, + cols: terminalCols, + scrollTop: region.top, + scrollBottom: region.bottom, + originMode: scrollRegion.isOriginMode, + }); + }, write: deps.writeChunk, - enabled: statusLineEnabled, + enabled: () => statusLineEnabled, coalesceMs: deps.statusRepaintCoalesceMs ?? 40, }); const paintStatus = (): void => { statusController.request(); }; + observeRenderedOutput = (chunk): void => { + scrollRegion.push(chunk); + statusController.observeOutput(chunk); + }; // Route server output through the predictive-echo engine (which owns - // cursor save/restore) or straight to stdout, then repaint the status. + // cursor save/restore) or straight to stdout. Repaint at safe ANSI + // boundaries after each chunk: cursor-addressed output remains confined + // to the smaller PTY, while this restores the row after terminal autowrap. const applyServerOutput = (chunk: string): void => { - statusController.observeOutput(chunk); if (predictiveEcho) { - void predictiveEcho.onServerOutput(chunk).then(paintStatus, paintStatus); + void predictiveEcho.onServerOutput(chunk).then( + () => { + paintStatus(); + }, + () => { + paintStatus(); + } + ); } else { deps.writeChunk(chunk); + observeRenderedOutput(chunk); paintStatus(); } }; @@ -445,13 +516,21 @@ export async function runPassthroughSession( if (!size) return; terminalRows = size.rows; terminalCols = size.cols; - predictiveEcho?.onResize(size.cols, size.rows); + statusLineEnabled = canReserveStatusLine(size); + const agentSize = reserveStatusLineRow(size); + if (!agentSize) return; + scrollRegion.setRows(agentSize.rows); + predictiveEcho?.onResize(agentSize.cols, agentSize.rows); trackResize( - resizeWorker(connection, name, size.rows, size.cols, deps.fetch, { + resizeWorker(connection, name, agentSize.rows, agentSize.cols, deps.fetch, { sessionId: resizeSessionId, }).then((res) => { if (!res.ok) { deps.log(`[passthrough] resize forward failed: ${res.message ?? 'unknown error'}`); + } else if (res.applied === false) { + deps.log( + '[passthrough] broker did not apply the reserved PTY size; using status repaint fallback' + ); } }) ); @@ -520,7 +599,7 @@ export async function runPassthroughSession( // Heal the local terminal on detach: the snapshot + live stream may // have left it in app-cursor / mouse / bracketed-paste / alt-screen // mode. Gate on TTY stdout (same signal as the status line). - resetLocalTerminalOnDetach(deps.writeChunk, statusLineEnabled); + resetLocalTerminalOnDetach(deps.writeChunk, isTtyOutput); } catch { // best effort } @@ -585,7 +664,15 @@ export async function runPassthroughSession( if (outstandingResizes.size > 0) { await Promise.allSettled([...outstandingResizes]); } - await releaseResizeOwnership(connection, name, resizeSessionId, deps.fetch); + // Hand the reserved status row/column back in the same request, so + // the agent's TUI is not left one row and column short. + await releaseResizeOwnership( + connection, + name, + resizeSessionId, + deps.fetch, + deps.terminal.getSize() + ); } catch { // Best-effort — the broker's idle-takeover net still frees ownership. } @@ -663,7 +750,6 @@ export async function runPassthroughSession( try { if (settled) return; stdinReady = true; - unsubscribeResize = deps.terminal.onResize(resizeHandler); // Periodic ownership re-assert (see `ownershipReassertMs`): keeps the // single-resizer lease alive on an idle-but-live session; the broker // no-ops a same-size re-assert (no SIGWINCH/repaint). @@ -671,9 +757,10 @@ export async function runPassthroughSession( if (reassertMs > 0) { reassertTimer = setInterval(() => { const size = deps.terminal.getSize() ?? initialLocalSize; - if (!size) return; + const agentSize = reserveStatusLineRow(size); + if (!agentSize) return; trackResize( - resizeWorker(connection, name, size.rows, size.cols, deps.fetch, { + resizeWorker(connection, name, agentSize.rows, agentSize.cols, deps.fetch, { sessionId: resizeSessionId, }).then((res) => { if (!res.ok) { @@ -694,13 +781,22 @@ export async function runPassthroughSession( } }; - // Runs once the event WS is subscribed. Take over stdin before replaying - // the snapshot: a source TUI can enable mouse/focus/alternate-scroll - // reporting in that replay, and cooked-mode stdin would echo those reports - // as visible escape text until the later input-stream setup completed. + // Runs once the event WS is subscribed: first reserve the local bottom row + // and take over stdin before replaying the snapshot. A source TUI can + // enable mouse/focus/alternate-scroll reporting in that replay, and + // cooked-mode stdin would echo those reports as visible escape text. The + // subscribed WS buffers the resize repaint for reconciliation. const onSubscribed = async (): Promise => { + beginSubscribedLayout(); await openInputStreamAndSetRawMode(); if (settled) return; + const initialResize = syncInitialPtySize(connection, name, initialAgentSize, 'passthrough', deps, { + sessionId: resizeSessionId, + }); + trackResize(initialResize); + await initialResize; + if (settled) return; + await correctSetupResize(initialAgentSize); const snapshot = await deps.captureAndRenderSnapshot( { url: connection.url, apiKey: connection.apiKey }, name, @@ -736,9 +832,11 @@ export async function runPassthroughSession( } if (settled) return; } - terminalRows = pickInitialTerminalRows(initialLocalSize, snapshot.rows); - terminalCols = pickInitialTerminalCols(initialLocalSize, snapshot.cols); + const currentLocalSize = deps.terminal.getSize(); + terminalRows = pickInitialTerminalRows(currentLocalSize, snapshot.rows); + terminalCols = pickInitialTerminalCols(currentLocalSize, snapshot.cols); // Track the snapshot bytes for boundary state before the first repaint. + scrollRegion.push(snapshotBytes); statusController.observeOutput(snapshotBytes); paintStatus(); // Reconcile buffered chunks. On `ok`, drop what the snapshot already @@ -747,12 +845,6 @@ export async function runPassthroughSession( // transient snapshot failure nothing was painted, so apply everything. const pending = snapshot.status === 'ok' ? sync.reconcile(snapshot.offset) : sync.flushAll(); for (const chunk of pending) applyServerOutput(chunk); - const initialResize = syncInitialPtySize(connection, name, initialLocalSize, 'passthrough', deps, { - sessionId: resizeSessionId, - }); - trackResize(initialResize); - await initialResize; - if (settled) return; // Input forwarding starts only after predictive echo has been seeded by // the snapshot. Raw mode was already enabled above, so terminal reports // could not echo during the setup window. diff --git a/packages/cli/src/cli/lib/attach.test.ts b/packages/cli/src/cli/lib/attach.test.ts index cf7e82ff2..d6f8253a0 100644 --- a/packages/cli/src/cli/lib/attach.test.ts +++ b/packages/cli/src/cli/lib/attach.test.ts @@ -7,11 +7,15 @@ import { createBackpressureAwareWriter, DEFAULT_STATUS_LINE_COLS, pickInitialTerminalCols, + fitStatusLineText, LOCAL_TERMINAL_RESET_SEQUENCE, + reserveStatusLineRow, resetLocalTerminalOnDetach, restoreInboundDeliveryModeOnDetach, StatusLineController, + StatusLineInvalidationScanner, StreamSyncBuffer, + TerminalScrollRegionTracker, type AttachSnapshotConnection, type AttachSnapshotDeps, type BackpressureWritable, @@ -23,6 +27,46 @@ afterEach(() => { vi.restoreAllMocks(); }); +describe('reserveStatusLineRow', () => { + it('gives a TUI one fewer row and column', () => { + expect(reserveStatusLineRow({ rows: 40, cols: 120 })).toEqual({ rows: 39, cols: 119 }); + }); + + it('keeps degenerate terminals usable and leaves non-TTY output alone', () => { + expect(reserveStatusLineRow({ rows: 1, cols: 80 })).toEqual({ rows: 1, cols: 80 }); + expect(reserveStatusLineRow({ rows: 2, cols: 80 })).toEqual({ rows: 2, cols: 80 }); + expect(reserveStatusLineRow(null)).toBeNull(); + }); +}); + +describe('fitStatusLineText', () => { + it('leaves the autowrap column unused and marks truncation', () => { + expect(fitStatusLineText('1234567890', 8)).toBe('123456~'); + }); + + it('conservatively accounts for non-ASCII names', () => { + expect(fitStatusLineText('A🤖BC', 5)).toBe('A🤖~'); + expect(fitStatusLineText('short', undefined)).toBe('short'); + }); +}); + +describe('StatusLineInvalidationScanner', () => { + it('ignores ordinary cursor-addressed TUI output', () => { + const scanner = new StatusLineInvalidationScanner(); + expect(scanner.push('\x1b[4;10Hhello\x1b[32mworld')).toBe(false); + }); + + it('detects display erases, resets, and alternate-screen switches across chunks', () => { + const scanner = new StatusLineInvalidationScanner(); + expect(scanner.push('\x1b[')).toBe(false); + expect(scanner.push('2J')).toBe(true); + expect(scanner.push('ordinary follow-up')).toBe(false); + expect(scanner.push('\x1b[?25;10')).toBe(false); + expect(scanner.push('49h')).toBe(true); + expect(scanner.push('\x1bc')).toBe(true); + }); +}); + function makeDeps(overrides: Partial = {}): { deps: AttachSnapshotDeps; writes: string[]; @@ -303,6 +347,71 @@ describe('StreamSyncBuffer', () => { }); }); +describe('TerminalScrollRegionTracker', () => { + it('tracks narrow DECSTBM margins split across chunks and resets them', () => { + const tracker = new TerminalScrollRegionTracker(9); + tracker.push('frame\x1b[3;'); + expect(tracker.region).toEqual({ top: 1, bottom: 9 }); + tracker.setRows(12); + tracker.push('8r'); + expect(tracker.region).toEqual({ top: 3, bottom: 8 }); + tracker.push('\x1b[r'); + expect(tracker.region).toEqual({ top: 1, bottom: 12 }); + }); + + it('resets margins for a resized child and alternate-screen switch', () => { + const tracker = new TerminalScrollRegionTracker(9); + tracker.push('\x1b[2;7r'); + tracker.setRows(14); + expect(tracker.region).toEqual({ top: 1, bottom: 14 }); + tracker.push('\x1b[2;8r\x1b[?25;10'); + tracker.push('49h'); + expect(tracker.region).toEqual({ top: 1, bottom: 14 }); + tracker.push('\x1b[3;7r\x1b[?1049l'); + expect(tracker.region).toEqual({ top: 2, bottom: 8 }); + tracker.push('\x1b[?1049h'); + expect(tracker.region).toEqual({ top: 1, bottom: 14 }); + tracker.push('\x1b[?6h'); + expect(tracker.isOriginMode).toBe(true); + tracker.push('\x1bc'); + expect(tracker.isOriginMode).toBe(false); + tracker.push('\x1b[2;8r\x1b[?1049h'); + expect(tracker.region).toEqual({ top: 1, bottom: 14 }); + }); + + it('ignores CSI-looking bytes inside DCS payloads', () => { + const tracker = new TerminalScrollRegionTracker(12); + tracker.push('\x1bPtmux;\x1b[3;8r'); + tracker.push('\x1b\\'); + expect(tracker.region).toEqual({ top: 1, bottom: 12 }); + tracker.push('\x1b[4;9r'); + expect(tracker.region).toEqual({ top: 4, bottom: 9 }); + tracker.setRows(9); + tracker.push('\x1b[10;20r'); + expect(tracker.region).toEqual({ top: 1, bottom: 9 }); + tracker.push('\x1b[3;7r\x1b[0;0r'); + expect(tracker.region).toEqual({ top: 1, bottom: 9 }); + tracker.push('\x1bPpayload\x9c\x1b[3;8r'); + expect(tracker.region).toEqual({ top: 3, bottom: 8 }); + tracker.push('\x1b[4;\x18' + '9r'); + expect(tracker.region).toEqual({ top: 3, bottom: 8 }); + }); + + it('resets alternate margins on every supported alternate-screen re-entry', () => { + for (const mode of ['47', '1047', '1049']) { + const tracker = new TerminalScrollRegionTracker(10); + tracker.push(`\x1b[?${mode}h\x1b[3;7r\x1b[?${mode}l\x1b[?${mode}h`); + expect(tracker.region).toEqual({ top: 1, bottom: 10 }); + } + }); + + it('preserves margins for redundant alternate-screen set commands', () => { + const tracker = new TerminalScrollRegionTracker(10); + tracker.push('\x1b[?1049h\x1b[4;7r\x1b[?1049h'); + expect(tracker.region).toEqual({ top: 4, bottom: 7 }); + }); +}); + describe('AnsiBoundaryScanner', () => { it('reports a boundary for plain text', () => { const s = new AnsiBoundaryScanner(); @@ -344,6 +453,18 @@ describe('AnsiBoundaryScanner', () => { expect(s.atBoundary).toBe(true); }); + it('handles split C1 CSI and C1 string controls', () => { + const s = new AnsiBoundaryScanner(); + s.push('\x9b3;'); + expect(s.atBoundary).toBe(false); + s.push('8r'); + expect(s.atBoundary).toBe(true); + s.push('\x90payload'); + expect(s.atBoundary).toBe(false); + s.push('\x9c'); + expect(s.atBoundary).toBe(true); + }); + it('is unaffected by multi-byte UTF-8 payload in the ground state', () => { const s = new AnsiBoundaryScanner(); s.push('café 你好 🎉'); @@ -429,24 +550,33 @@ describe('StatusLineController', () => { expect(writes).toEqual(['S']); }); - it('force-paints after boundaryHoldMs if output stays mid-sequence', () => { - const writes: string[] = []; + it.each([ + ['CSI', '\x1b[', '2J'], + ['OSC', '\x1b]0;title', '\x07'], + ['DCS', '\x1bP$qm', '\x1b\\'], + ])('never splices a status repaint into a delayed split %s sequence', (_name, start, end) => { + const output: string[] = []; const t = fakeTimers(); const c = new StatusLineController({ - render: () => 'S', - write: (s) => writes.push(s), + render: () => 'STATUS', + write: (s) => output.push(s), enabled: true, coalesceMs: 0, - boundaryHoldMs: 100, now: t.now, setTimer: t.setTimer, clearTimer: t.clearTimer, }); - c.observeOutput('\x1b['); + output.push(start); + c.observeOutput(start); c.request(); - expect(writes).toEqual([]); - t.advance(100); - expect(writes).toEqual(['S']); // bounded fallback + t.advance(10_000); + expect(output).toEqual([start]); + expect(t.pending()).toBe(0); + + output.push(end); + c.observeOutput(end); + expect(output).toEqual([start, end, 'STATUS']); + expect(output.slice(0, 2).join('')).toBe(start + end); }); it('coalesces rapid repaints into one per window (latest state wins)', () => { @@ -474,7 +604,7 @@ describe('StatusLineController', () => { expect(writes).toEqual(['S1', 'S3']); }); - it('re-arms a plain coalescing timer when a boundary-hold timer is pending and output returns to a boundary', () => { + it('starts the remaining coalescing delay only after output returns to a boundary', () => { const writes: string[] = []; const t = fakeTimers(); const c = new StatusLineController({ @@ -482,7 +612,6 @@ describe('StatusLineController', () => { write: (s) => writes.push(s), enabled: true, coalesceMs: 50, - boundaryHoldMs: 100, now: t.now, setTimer: t.setTimer, clearTimer: t.clearTimer, @@ -491,24 +620,42 @@ describe('StatusLineController', () => { // inside the coalescing window. c.request(); expect(writes).toEqual(['S']); - // Output ends mid-CSI, then a repaint is requested → a force (boundary-hold) - // timer is armed for boundaryHoldMs (deadline now+100). + // Output ends mid-CSI, then a repaint is requested. No timer may force a + // write into the incomplete sequence. c.observeOutput('\x1b['); c.request(); - expect(t.pending()).toBe(1); - // A little later, output returns to a boundary. The stale force timer must - // be cleared and a plain coalescing timer armed for the *remainder* of the - // 50ms window (≈40ms from here), not left to fire at the 100ms deadline. + expect(t.pending()).toBe(0); + // A little later, output returns to a boundary. A coalescing timer is + // armed for the remainder of the 50ms window (≈40ms from here). t.advance(10); c.observeOutput('0m'); // completes the CSI → boundary expect(writes).toEqual(['S']); // still deferred within the coalescing window - // The coalescing remainder elapses at +40ms; a boundary-hold timer would - // not have fired until +90ms, so this paint proves the re-arm. + expect(t.pending()).toBe(1); t.advance(40); expect(writes).toEqual(['S', 'S']); expect(t.pending()).toBe(0); }); + it('does not let a held repaint fire after dynamic status disablement', () => { + const writes: string[] = []; + const t = fakeTimers(); + let enabled = true; + const c = new StatusLineController({ + render: () => 'S', + write: (s) => writes.push(s), + enabled: () => enabled, + coalesceMs: 0, + now: t.now, + setTimer: t.setTimer, + clearTimer: t.clearTimer, + }); + c.observeOutput('\x1b['); + c.request(); + enabled = false; + c.observeOutput('0m'); + expect(writes).toEqual([]); + }); + it('stops painting after dispose', () => { const writes: string[] = []; const t = fakeTimers(); @@ -884,6 +1031,10 @@ describe('clampStatusLineText', () => { } }); + it('can reserve the final terminal column to avoid wrap-pending state', () => { + expect(clampStatusLineText('1234567890', 10, true)).toBe('1234…7890'); + }); + it('degrades to a bare head rather than wrapping on a pathological width', () => { expect(clampStatusLineText(label, 1)).toBe('['); expect(clampStatusLineText(label, 0).length).toBeLessThanOrEqual(DEFAULT_STATUS_LINE_COLS); diff --git a/packages/cli/src/cli/lib/attach.ts b/packages/cli/src/cli/lib/attach.ts index 708573539..28e1a967a 100644 --- a/packages/cli/src/cli/lib/attach.ts +++ b/packages/cli/src/cli/lib/attach.ts @@ -392,17 +392,24 @@ export const DEFAULT_STATUS_LINE_COLS = 80; * six rows of agent output. * * Truncation keeps the paint inside one row, so the wrap — and the scroll - * cascade it triggers — can never happen. The tail is the part that carries - * the key hints, so an over-long label is trimmed from the *middle*: the verb - * and agent name stay readable and the `Ctrl+…` hints survive. + * cascade it triggers — can never happen. Callers painting a terminal row can + * reserve the final column as well, avoiding the emulator's wrap-pending state. + * The tail is the part that carries the key hints, so an over-long label is + * trimmed from the *middle*: the verb and agent name stay readable and the + * `Ctrl+…` hints survive. * * Measured in display columns, not UTF-16 code units: an agent name carrying * CJK or emoji is double-width, so a code-unit count would pass a label that * still wraps — re-arming the exact cascade this prevents. Slicing is done on * whole code points so a surrogate pair is never split in half. */ -export function clampStatusLineText(text: string, cols: number | undefined): string { - const width = typeof cols === 'number' && cols > 0 ? Math.floor(cols) : DEFAULT_STATUS_LINE_COLS; +export function clampStatusLineText( + text: string, + cols: number | undefined, + reserveFinalColumn = false +): string { + const terminalWidth = typeof cols === 'number' && cols > 0 ? Math.floor(cols) : DEFAULT_STATUS_LINE_COLS; + const width = reserveFinalColumn ? Math.max(terminalWidth - 1, 0) : terminalWidth; if (displayWidth(text) <= width) return text; // Too narrow to say anything useful — a bare head is still better than a // wrap, and `…` alone would be meaningless. @@ -475,6 +482,86 @@ function takeColumns(text: string, budget: number, from: 'start' | 'end' = 'star return { text: (from === 'end' ? taken.reverse() : taken).join('') }; } +/** + * Reserve the local terminal's final row for an attach status line. + * + * Full-screen CLIs treat every PTY row as application-owned. If the remote PTY + * is resized to the local terminal's full height and Relay then paints a status + * line over its final row, both renderers continually overwrite each other. + * The result is the duplicated status text and torn TUI frames seen in drive + * and passthrough sessions. Give the child one fewer row and column: the row + * prevents cursor-addressed overlap, while the spare column prevents terminal + * autowrap from stepping past the child scroll margin into Relay's row. + */ +export function reserveStatusLineRow( + localSize: { rows: number; cols: number } | null +): { rows: number; cols: number } | null { + if (!localSize) return null; + if (localSize.rows < 3 || localSize.cols < 2) return localSize; + return { + rows: localSize.rows - 1, + cols: localSize.cols - 1, + }; +} + +/** Whether a terminal is large enough to dedicate a non-wrapping status row. */ +export function canReserveStatusLine( + localSize: { rows: number; cols: number } | null +): localSize is { rows: number; cols: number } { + return localSize !== null && localSize.rows >= 3 && localSize.cols >= 2; +} + +/** + * Constrain local scrolling to the child PTY's rows while preserving the + * current cursor. DECSTBM otherwise defaults to the physical terminal height, + * which includes Relay's status row. + */ +export function renderChildScrollRegion(rows: number): string { + // CSI s/u save only the cursor. DEC ESC 7/8 also preserve terminal state in + // common emulators and would therefore undo the newly installed margins. + return `\x1b[s\x1b[1;${Math.max(1, Math.floor(rows))}r\x1b[u`; +} + +/** + * Keep a status label strictly inside one terminal row. + * + * Writing the final column with autowrap enabled advances to the next row and, + * on the terminal's bottom row, scrolls the entire TUI. Leave one cell unused + * and conservatively count non-ASCII code points as two cells so even unusual + * agent names cannot trigger a wrap. + */ +function conservativeTerminalCellWidth(character: string): number { + const codePoint = character.codePointAt(0) ?? 0; + return codePoint >= 0x20 && codePoint <= 0x7e ? 1 : 2; +} + +export function fitStatusLineText(text: string, columns: number | undefined): string { + if (columns === undefined || !Number.isFinite(columns)) return text; + const available = Math.max(0, Math.floor(columns) - 1); + if (available === 0) return ''; + + let width = 0; + let result = ''; + let truncated = false; + for (const character of text) { + const characterWidth = conservativeTerminalCellWidth(character); + if (width + characterWidth > available) { + truncated = true; + break; + } + result += character; + width += characterWidth; + } + if (!truncated) return result; + while (width + 1 > available && result.length > 0) { + const characters = Array.from(result); + const removed = characters.pop() ?? ''; + width -= conservativeTerminalCellWidth(removed); + result = characters.join(''); + } + return `${result}~`; +} + /** * Sync the agent's PTY to the driver's local terminal size. tmux / * screen / ssh all do this — without it a TUI in the agent renders into @@ -489,15 +576,26 @@ export async function syncInitialPtySize( verb: string, deps: { fetch: typeof globalThis.fetch; log: (...args: unknown[]) => void }, options?: { sessionId?: string } -): Promise { - if (!localSize) return; +): Promise { + if (!localSize) return false; try { - await createBrokerClient(connection, deps.fetch).resizePty(name, localSize.rows, localSize.cols, options); + const result = await createBrokerClient(connection, deps.fetch).resizePty( + name, + localSize.rows, + localSize.cols, + options + ); + if (result.applied === false) { + deps.log(`[${verb}] broker did not apply the reserved PTY size; using status repaint fallback`); + return false; + } + return true; } catch (err: unknown) { const failure = mapBrokerSdkFailure(err); deps.log( `[${verb}] could not sync agent PTY size to local terminal (${failure.message ?? 'unknown'}); continuing` ); + return false; } } @@ -652,6 +750,9 @@ export class AnsiBoundaryScanner { switch (this.state) { case 'ground': if (c === 0x1b) this.state = 'esc'; + else if (c === 0x9b) this.state = 'csi'; + else if (c === 0x9d) this.state = 'osc'; + else if (c === 0x90 || c === 0x98 || c === 0x9e || c === 0x9f) this.state = 'str'; return; case 'esc': if (c === 0x5b) @@ -663,28 +764,269 @@ export class AnsiBoundaryScanner { else this.state = 'ground'; // 2-byte escape (ESC 7, ESC 8, ESC c, …) return; case 'csi': - if (c === 0x1b) + if (c === 0x18 || c === 0x1a) + this.state = 'ground'; // CAN / SUB cancel the sequence + else if (c === 0x1b) this.state = 'esc'; // stray ESC restarts else if (c >= 0x40 && c <= 0x7e) this.state = 'ground'; // final byte ends the CSI return; case 'osc': - if (c === 0x07) + if (c === 0x07 || c === 0x9c || c === 0x18 || c === 0x1a) this.state = 'ground'; // BEL terminator else if (c === 0x1b) this.state = 'osc_esc'; // possible ST (ESC \) return; case 'osc_esc': - this.state = c === 0x5c ? 'ground' : 'osc'; + this.state = + c === 0x5c || c === 0x9c || c === 0x18 || c === 0x1a ? 'ground' : c === 0x1b ? 'osc_esc' : 'osc'; return; case 'str': - if (c === 0x1b) this.state = 'str_esc'; + if (c === 0x9c || c === 0x18 || c === 0x1a) this.state = 'ground'; + else if (c === 0x1b) this.state = 'str_esc'; return; case 'str_esc': - this.state = c === 0x5c ? 'ground' : 'str'; + this.state = + c === 0x5c || c === 0x9c || c === 0x18 || c === 0x1a ? 'ground' : c === 0x1b ? 'str_esc' : 'str'; return; } } } +/** + * Detect terminal controls that can erase or replace Relay's reserved status + * row even though the child PTY itself is one row shorter. + * + * Cursor-addressed TUI drawing stays inside the child grid, but erase-display, + * terminal reset, and alternate-screen switches act on the local terminal as a + * whole. Attach clients reinstall their child scroll region after those + * operations; the status controller separately repaints at a safe ANSI + * boundary. A short carry handles controls split across WS frames. + */ +export class StatusLineInvalidationScanner { + private carry = ''; + + push(chunk: string): boolean { + const input = this.carry + chunk; + const esc = String.fromCharCode(0x1b); + const privateModes = new RegExp(`${esc}\\[\\?([0-9;]+)[hl]`, 'g'); + let invalidatesDisplay = false; + let privateMatch: RegExpExecArray | null; + while ((privateMatch = privateModes.exec(input)) !== null) { + if (privateMatch[1].split(';').some((mode) => ['47', '1047', '1049'].includes(mode))) { + invalidatesDisplay = true; + } + } + // Retain only an incomplete candidate control. Keeping arbitrary completed + // output would rediscover the same erase on later chunks and recreate the + // repaint storm this scanner exists to prevent. + const lastEscape = input.lastIndexOf(esc); + const suffix = lastEscape >= 0 ? input.slice(lastEscape) : ''; + const incompleteCsi = suffix.startsWith(`${esc}[`) && /^[?0-9;]*$/.test(suffix.slice(2)); + this.carry = suffix === esc || incompleteCsi ? suffix : ''; + + return ( + // ED 0/default clears from the cursor through the reserved final row; + // ED 2 clears the full display. + input.includes(`${esc}[J`) || + input.includes(`${esc}[0J`) || + input.includes(`${esc}[2J`) || + // Alternate-screen changes replace the entire visible buffer. + invalidatesDisplay || + // RIS resets the terminal and clears the active display. + input.includes(`${esc}c`) + ); + } +} + +/** Track the child application's active DECSTBM margins across streamed ANSI. */ +export class TerminalScrollRegionTracker { + private parserState: 'ground' | 'escape' | 'csi' | 'string' | 'string_escape' = 'ground'; + private csiParams = ''; + private csiSupported = true; + private stringAllowsBel = false; + private rows: number; + private top = 1; + private bottom: number; + private originMode = false; + private alternateActive = false; + private primaryRegion: { top: number; bottom: number }; + private alternateRegion: { top: number; bottom: number }; + + constructor(rows: number) { + this.rows = Math.max(1, Math.floor(rows)); + this.bottom = this.rows; + this.primaryRegion = { top: 1, bottom: this.rows }; + this.alternateRegion = { top: 1, bottom: this.rows }; + } + + setRows(rows: number): void { + this.rows = Math.max(1, Math.floor(rows)); + this.top = 1; + this.bottom = this.rows; + this.primaryRegion = { top: 1, bottom: this.rows }; + this.alternateRegion = { top: 1, bottom: this.rows }; + } + + push(chunk: string): void { + for (const character of chunk) this.consumeCharacter(character); + } + + get region(): { top: number; bottom: number } { + return { top: this.top, bottom: this.bottom }; + } + + get isOriginMode(): boolean { + return this.originMode; + } + + private consumeCharacter(character: string): void { + switch (this.parserState) { + case 'ground': + this.consumeGround(character); + break; + case 'escape': + this.consumeEscape(character); + break; + case 'csi': + this.consumeCsi(character); + break; + case 'string': + this.consumeString(character); + break; + case 'string_escape': + this.consumeStringEscape(character); + break; + } + } + + private consumeString(character: string): void { + if (character === '\x1b') { + this.parserState = 'string_escape'; + return; + } + if ( + character === '\x9c' || + character === '\x18' || + character === '\x1a' || + (this.stringAllowsBel && character === '\x07') + ) { + this.parserState = 'ground'; + } + } + + private consumeStringEscape(character: string): void { + if (character === '\\' || character === '\x9c' || character === '\x18' || character === '\x1a') { + this.parserState = 'ground'; + } else { + this.parserState = character === '\x1b' ? 'string_escape' : 'string'; + } + } + + private consumeGround(character: string): void { + if (character === '\x1b') this.parserState = 'escape'; + else if (character === '\x9b') this.beginCsi(); + else if (['\x90', '\x98', '\x9d', '\x9e', '\x9f'].includes(character)) { + this.beginString(character === '\x9d'); + } + } + + private consumeEscape(character: string): void { + if (character === '[') this.beginCsi(); + else if (['P', 'X', ']', '^', '_'].includes(character)) this.beginString(character === ']'); + else { + if (character === 'c') this.resetTrackedState(); + this.parserState = character === '\x1b' ? 'escape' : 'ground'; + } + } + + private beginCsi(): void { + this.parserState = 'csi'; + this.csiParams = ''; + this.csiSupported = true; + } + + private beginString(allowsBel: boolean): void { + this.parserState = 'string'; + this.stringAllowsBel = allowsBel; + } + + private consumeCsi(character: string): void { + if (character === '\x18' || character === '\x1a') { + this.parserState = 'ground'; + return; + } + if (character === '\x1b') { + this.parserState = 'escape'; + return; + } + const code = character.codePointAt(0) ?? 0; + if (code >= 0x30 && code <= 0x3f) { + this.csiParams += character; + return; + } + if (code >= 0x20 && code <= 0x2f) { + this.csiSupported = false; + return; + } + if (code >= 0x40 && code <= 0x7e) { + if (this.csiSupported) this.applyCsi(this.csiParams, character); + this.parserState = 'ground'; + } + } + + private applyCsi(params: string, final: string): void { + if ((final === 'h' || final === 'l') && params.startsWith('?')) { + this.applyPrivateModes(params.slice(1).split(';'), final === 'h'); + return; + } + if (final !== 'r' || params.startsWith('?')) return; + const [topRaw = '', bottomRaw = ''] = params.split(';'); + const parsedTop = topRaw === '' ? 0 : Number.parseInt(topRaw, 10); + const parsedBottom = bottomRaw === '' ? 0 : Number.parseInt(bottomRaw, 10); + const top = parsedTop === 0 ? 1 : parsedTop; + const bottom = parsedBottom === 0 ? this.rows : parsedBottom; + if (Number.isFinite(top) && Number.isFinite(bottom) && top >= 1 && bottom > top) { + const clampedTop = Math.min(top, this.rows); + const clampedBottom = Math.min(bottom, this.rows); + if (clampedBottom > clampedTop) { + this.top = clampedTop; + this.bottom = clampedBottom; + } + } + } + + private applyPrivateModes(modes: string[], enabled: boolean): void { + if (modes.includes('6')) this.originMode = enabled; + if (modes.some((mode) => ['47', '1047', '1049'].includes(mode))) { + this.switchScreenBuffer(enabled, enabled); + } + } + + private resetTrackedState(): void { + this.top = 1; + this.bottom = this.rows; + this.originMode = false; + this.alternateActive = false; + this.primaryRegion = { top: 1, bottom: this.rows }; + this.alternateRegion = { top: 1, bottom: this.rows }; + } + + private switchScreenBuffer(enterAlternate: boolean, resetAlternate: boolean): void { + if (enterAlternate === this.alternateActive) return; + if (resetAlternate) { + this.alternateRegion = { top: 1, bottom: this.rows }; + } + if (this.alternateActive) { + this.alternateRegion = { top: this.top, bottom: this.bottom }; + } else { + this.primaryRegion = { top: this.top, bottom: this.bottom }; + } + this.alternateActive = enterAlternate; + const region = enterAlternate ? this.alternateRegion : this.primaryRegion; + this.top = region.top; + this.bottom = region.bottom; + } +} + /** Options for {@link StatusLineController}. Timers are injectable for tests. */ export interface StatusLineControllerOptions { /** Produce the current status-line ANSI string. */ @@ -692,18 +1034,13 @@ export interface StatusLineControllerOptions { /** Write to stdout. */ write: (chunk: string) => void; /** When false (stdout is not a TTY) the status line is never painted. */ - enabled: boolean; + enabled: boolean | (() => boolean); /** * Minimum ms between repaints. `0` paints on every request (no coalescing); * a positive value coalesces bursts of per-chunk repaints into at most one * paint per window, shrinking the splice/DECSC-clobber window. */ coalesceMs: number; - /** - * Max ms to hold a repaint while output keeps ending mid escape-sequence - * before painting anyway (bounded residual splice risk). Default 100. - */ - boundaryHoldMs?: number; setTimer?: (fn: () => void, ms: number) => ReturnType; clearTimer?: (t: ReturnType) => void; now?: () => number; @@ -716,8 +1053,8 @@ export interface StatusLineControllerOptions { * - **Non-TTY skip** — when `enabled` is false, nothing is ever written. * - **Boundary-hold** — a repaint is deferred while the observed output ends * mid ANSI escape sequence, so the status line never splices into a - * half-transmitted CSI. It paints as soon as the next chunk lands at a - * boundary (or after `boundaryHoldMs` as a bounded fallback). + * half-transmitted CSI. It paints only after a later chunk returns the + * stream to a real boundary. * - **Coalescing** — repaints are rate-limited to at most one per * `coalesceMs`, shrinking the window in which our save/restore-cursor wrap * can clobber the agent's own pending DECSC. @@ -731,24 +1068,32 @@ export interface StatusLineControllerOptions { export class StatusLineController { private readonly scanner = new AnsiBoundaryScanner(); private timer: ReturnType | null = null; - private timerForce = false; private wantPaint = false; private lastPaintAt = Number.NEGATIVE_INFINITY; private disposed = false; constructor(private readonly opts: StatusLineControllerOptions) {} + private isEnabled(): boolean { + return typeof this.opts.enabled === 'function' ? this.opts.enabled() : this.opts.enabled; + } + /** Record server output for boundary tracking; flush a held repaint if the * stream is now back at a sequence boundary. */ observeOutput(chunk: string): void { if (this.disposed) return; this.scanner.push(chunk); + if (!this.isEnabled()) { + this.wantPaint = false; + this.clearTimer(); + return; + } if (this.wantPaint) this.flush(); } /** Request a repaint (coalesced + boundary-held). */ request(): void { - if (this.disposed || !this.opts.enabled) return; + if (this.disposed || !this.isEnabled()) return; this.wantPaint = true; this.flush(); } @@ -765,37 +1110,31 @@ export class StatusLineController { } private flush(): void { - if (this.disposed || !this.wantPaint) return; + if (this.disposed || !this.wantPaint || !this.isEnabled()) return; if (!this.scanner.atBoundary) { - this.arm(this.opts.boundaryHoldMs ?? 100, true); return; } - // Back at a boundary: any pending *force* (boundary-hold) timer carries the - // wrong deadline now — it was armed to paint mid-sequence after - // `boundaryHoldMs`, which would either paint later than the coalescing - // window needs or bypass the window entirely. Clear it so the normal - // coalescing logic below re-arms a plain timer for the correct remainder. - // A non-force (coalescing) timer already has the right deadline, so leave - // it in place. - if (this.timer && this.timerForce) this.clearTimer(); const elapsed = this.now() - this.lastPaintAt; if (elapsed >= this.opts.coalesceMs) { this.paintNow(); } else { - this.arm(this.opts.coalesceMs - elapsed, false); + this.arm(this.opts.coalesceMs - elapsed); } } private paintNow(): void { this.clearTimer(); + if (!this.isEnabled()) { + this.wantPaint = false; + return; + } this.wantPaint = false; this.lastPaintAt = this.now(); this.opts.write(this.opts.render()); } - private arm(ms: number, force: boolean): void { + private arm(ms: number): void { if (this.timer) return; - this.timerForce = force; const set = this.opts.setTimer ?? ((fn, delay) => { @@ -805,9 +1144,11 @@ export class StatusLineController { }); this.timer = set(() => { this.timer = null; - if (this.disposed || !this.wantPaint) return; - if (this.timerForce) this.paintNow(); - else this.flush(); + if (this.disposed || !this.wantPaint || !this.isEnabled()) { + this.wantPaint = false; + return; + } + this.flush(); }, ms); } diff --git a/packages/harness-driver/src/client.ts b/packages/harness-driver/src/client.ts index debf87686..0044666da 100644 --- a/packages/harness-driver/src/client.ts +++ b/packages/harness-driver/src/client.ts @@ -711,14 +711,29 @@ export class HarnessDriverClient { * * `rows`/`cols` are optional so a pure ownership release (`release: true`) * can omit them entirely rather than sending placeholder dimensions — the - * broker defaults them and skips the resize on a release. + * broker defaults them to zero and skips the resize on such a release. + * + * A release MAY still carry dimensions, in which case the broker applies + * them and *then* drops ownership. Attach clients that reserved a status row + * use this to hand the row back atomically: a separate resize would need to + * land strictly before the release or it re-claims the lease (#1247). + * `resized` reports whether the broker *dispatched* the restore resize to the + * worker — unlike `write_pty` it parks no pending request, so this is not a + * worker-side acknowledgement. */ async resizePty( name: string, rows?: number, cols?: number, options?: { sessionId?: string; release?: boolean } - ): Promise<{ name: string; rows?: number; cols?: number; applied?: boolean; released?: boolean }> { + ): Promise<{ + name: string; + rows?: number; + cols?: number; + applied?: boolean; + released?: boolean; + resized?: boolean; + }> { return this.transport.request(`/api/resize/${encodeURIComponent(name)}`, { method: 'POST', body: JSON.stringify({ diff --git a/tests/integration/broker/mcp-injection.test.ts b/tests/integration/broker/mcp-injection.test.ts index 8d8f408ff..573e48fdb 100644 --- a/tests/integration/broker/mcp-injection.test.ts +++ b/tests/integration/broker/mcp-injection.test.ts @@ -254,6 +254,73 @@ test( } ); +test( + 'mcp-injection: codex — named Relay participants are not replaced by built-in subagents', + { timeout: 180_000 }, + async (t) => { + if (skipIfMissing(t)) return; + if (skipIfNotRealCli(t)) return; + if (skipIfCliMissing(t, 'codex')) return; + + const harness = new BrokerHarness(); + const suffix = uniqueSuffix(); + const playerName = `PlayerA-${suffix}`; + const gameMasterName = `Gamemaster-${suffix}`; + + // Startup is inside the try so a failing `start()` still runs the cleanup + // below — a partially-created broker must not survive the test. + try { + await harness.start(); + // A lightweight, already-running Relay participant. The assertion is on + // the Gamemaster's outbound Relay message, so PlayerA need not answer. + await harness.spawnAgent(playerName, 'cat', ['general']); + await harness.spawnAgent(gameMasterName, 'codex', ['general'], { + task: + `Work with the existing participant ${playerName} to choose a tic-tac-toe move. ` + + `Ask ${playerName} which move they choose, then wait for their response. ` + + `Do not choose or play the move on their behalf.`, + }); + + // Wait for the routing evidence itself rather than a fixed delay: a slow + // Codex run used to fail before its message arrived, and a fast one still + // paid the full sleep. A timeout here is not the assertion — fall through + // so the descriptive checks below report what actually happened. + const isMessageToPlayer = (event: BrokerEvent): boolean => { + if (event.kind !== 'relay_inbound') return false; + const message = event as Extract; + return ( + message.from === gameMasterName && + message.target === playerName && + /move|choose|tic-tac-toe/i.test(message.body) + ); + }; + await harness.waitForEvent('relay_inbound', 90_000, isMessageToPlayer).promise.catch(() => undefined); + + const events = harness.getEvents(); + const agentMessages = getRelayInboundFromAgent(events, gameMasterName); + const messagesToPlayer = agentMessages.filter((event) => { + const message = event as Extract; + return message.target === playerName && /move|choose|tic-tac-toe/i.test(message.body); + }); + assert.ok( + messagesToPlayer.length >= 1, + `Codex Gamemaster should contact the existing Relay participant instead of starting ` + + `a built-in subagent; got ${messagesToPlayer.length} relevant messages to ${playerName}.` + ); + const output = collectStreamOutput(events, gameMasterName); + assert.doesNotMatch( + output, + /No agents completed yet|Finished waiting|Waiting for agents/i, + 'Codex Gamemaster should not enter its provider-native subagent wait flow' + ); + } finally { + await harness.releaseAgent(gameMasterName).catch(() => undefined); + await harness.releaseAgent(playerName).catch(() => undefined); + await harness.stop(); + } + } +); + // ══════════════════════════════════════════════════════════════════════════════ // OPENCODE MCP INJECTION // Verifies opencode.json is written and --agent agent-relay flag is passed.