diff --git a/.agentworkforce/trajectories/active/traj_io278d9ai5tw/trajectory.json b/.agentworkforce/trajectories/active/traj_io278d9ai5tw/trajectory.json index 31e1590f6..710db56fd 100644 --- a/.agentworkforce/trajectories/active/traj_io278d9ai5tw/trajectory.json +++ b/.agentworkforce/trajectories/active/traj_io278d9ai5tw/trajectory.json @@ -31,6 +31,18 @@ "reasoning": "handle_api_request is a method on BrokerRuntime that takes &mut self. Calling it from handle_terminal_control_event (also &mut self) is valid sequential Rust. The oneshot channel is used only for data handoff — tx.send() happens synchronously inside handle_api_request, rx.await() resolves immediately after. This reuses the exact HTTP code path including side effects: interactive-hold frame, sdk_out event emission, queue flush on transition." }, "significance": "high" + }, + { + "ts": 1786688147387, + "type": "decision", + "content": "Address every live PR #1502 review thread in one follow-up: raw option presence, path-specific guidance, regression coverage, and changelog: Address every live PR #1502 review thread in one follow-up: raw option presence, path-specific guidance, regression coverage, and changelog", + "raw": { + "question": "Address every live PR #1502 review thread in one follow-up: raw option presence, path-specific guidance, regression coverage, and changelog", + "chosen": "Address every live PR #1502 review thread in one follow-up: raw option presence, path-specific guidance, regression coverage, and changelog", + "alternatives": [], + "reasoning": "All five findings are valid or required by AGENTS.md; resolving the complete set avoids leaving the PR knowingly incomplete" + }, + "significance": "high" } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index ea0e529c1..75ca9b1bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,12 @@ All notable changes to Agent Relay will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Minor] + +### Added + +- `agent-relay node agent attach --node` now accepts `--workspace-key`, so commands copied from the Cloud dashboard resolve the intended workspace regardless of the working directory. +- Passing `--workspace-key` to the local or `--ssh-host` attach path is rejected because those paths authenticate with the broker instead. ## [11.6.1] - 2026-08-13 diff --git a/packages/cli/src/cli/commands/local-agent.test.ts b/packages/cli/src/cli/commands/local-agent.test.ts index 388ad3d6d..3b91c7937 100644 --- a/packages/cli/src/cli/commands/local-agent.test.ts +++ b/packages/cli/src/cli/commands/local-agent.test.ts @@ -137,6 +137,110 @@ describe('local agent subtree', () => { expect(exit).toHaveBeenCalledWith(1); }); + it('attach --node forwards --workspace-key so a copy-pasted command is cwd-independent', async () => { + const { program, attachNode } = harness(); + await program.parseAsync( + ['local', 'agent', 'attach', 'lead', '--node', 'sf-mini', '--workspace-key', 'rk_live_explicit'], + { from: 'user' } + ); + expect(attachNode).toHaveBeenCalledWith( + 'lead', + 'view', + 'sf-mini', + expect.objectContaining({ workspaceKey: 'rk_live_explicit' }) + ); + }); + + it('attach --node without --workspace-key leaves the precedence ladder to resolve it', async () => { + const { program, attachNode } = harness(); + await program.parseAsync(['local', 'agent', 'attach', 'lead', '--node', 'sf-mini'], { from: 'user' }); + expect(attachNode).toHaveBeenCalledWith( + 'lead', + 'view', + 'sf-mini', + expect.objectContaining({ workspaceKey: undefined }) + ); + }); + + it('attach --node treats a blank --workspace-key as unset rather than a literal credential', async () => { + const { program, attachNode } = harness(); + await program.parseAsync( + ['local', 'agent', 'attach', 'lead', '--node', 'sf-mini', '--workspace-key', ' '], + { from: 'user' } + ); + expect(attachNode).toHaveBeenCalledWith( + 'lead', + 'view', + 'sf-mini', + expect.objectContaining({ workspaceKey: undefined }) + ); + }); + + it('attach rejects --workspace-key without --node instead of silently ignoring it', async () => { + const { program, attach, attachNode, error, exit } = harness(); + await program.parseAsync(['local', 'agent', 'attach', 'lead', '--workspace-key', 'rk_live_explicit'], { + from: 'user', + }); + expect(attach).not.toHaveBeenCalled(); + expect(attachNode).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('--workspace-key requires --node')); + expect(exit).toHaveBeenCalledWith(1); + }); + + // `--workspace-key "$KEY"` with an unset variable reaches the parser as a + // blank string. Normalizing it away before the path check would hand the + // caller a silent local attach against whatever broker happened to be there. + it('attach rejects a blank --workspace-key without --node rather than falling through to the local broker', async () => { + const { program, attach, attachNode, error, exit } = harness(); + await program.parseAsync(['local', 'agent', 'attach', 'lead', '--workspace-key', ' '], { + from: 'user', + }); + expect(attach).not.toHaveBeenCalled(); + expect(attachNode).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('--workspace-key requires --node')); + expect(exit).toHaveBeenCalledWith(1); + }); + + it('attach rejects a blank --workspace-key on the --ssh-host path rather than silently attaching', async () => { + const { program, attachRemote, attachNode, error, exit } = harness(); + await program.parseAsync( + ['local', 'agent', 'attach', 'lead', '--ssh-host', 'barry', '--workspace-key', ''], + { from: 'user' } + ); + expect(attachRemote).not.toHaveBeenCalled(); + expect(attachNode).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith(expect.stringContaining('--workspace-key requires --node')); + expect(exit).toHaveBeenCalledWith(1); + }); + + // The two rejected paths do not take the same credentials: --ssh-host itself + // rejects --broker-url / --api-key, so naming them there sends the caller + // into a second, contradictory error. + it('attach points a rejected local-broker caller at its supported connection options', async () => { + const { program, error } = harness(); + await program.parseAsync(['local', 'agent', 'attach', 'lead', '--workspace-key', 'rk_live_explicit'], { + from: 'user', + }); + const message = error.mock.calls.at(0)?.[0] as string; + expect(message).toContain('--broker-url'); + expect(message).toContain('--api-key'); + expect(message).toContain('--state-dir'); + // Guidance for a path the caller is not on is guidance they cannot follow. + expect(message).not.toContain('--ssh-host'); + }); + + it('attach points a rejected --ssh-host caller at --state-dir, not the flags that path forbids', async () => { + const { program, error } = harness(); + await program.parseAsync( + ['local', 'agent', 'attach', 'lead', '--ssh-host', 'barry', '--workspace-key', 'rk_live_explicit'], + { from: 'user' } + ); + const message = error.mock.calls.at(0)?.[0] as string; + expect(message).toContain('--state-dir'); + expect(message).not.toContain('--broker-url'); + expect(message).not.toContain('--api-key'); + }); + it('attach --node prefixes a terminal setup error once', async () => { const attachNode = vi.fn(async () => { throw new Error('terminal unavailable'); diff --git a/packages/cli/src/cli/commands/local-agent.ts b/packages/cli/src/cli/commands/local-agent.ts index 2bdd16296..4885f80b9 100644 --- a/packages/cli/src/cli/commands/local-agent.ts +++ b/packages/cli/src/cli/commands/local-agent.ts @@ -78,6 +78,16 @@ export function runAttach(name: string, mode: AttachMode, options: NativeAttachO }); } +/** + * Options for the `--node` fleet path. Deliberately not `NativeAttachOptions`: + * a fleet attach authenticates with the Relaycast workspace key and has no + * local broker, so `--broker-url` / `--api-key` / `--state-dir` are rejected + * rather than accepted and ignored. + */ +export type FleetNodeAttachCliOptions = Pick & { + workspaceKey?: string; +}; + /** * Run an existing attach mode against the short-lived loopback adapter for a * remote fleet node. Deliberately bypasses native-harness detection: the proxy @@ -87,9 +97,14 @@ export async function attachFleetNode( name: string, mode: AttachMode, node: string, - options: NativeAttachOptions + options: FleetNodeAttachCliOptions ): Promise { - const proxy = await startFleetNodeAttachProxy({ agent: name, node, mode }); + const proxy = await startFleetNodeAttachProxy({ + agent: name, + node, + mode, + ...(options.workspaceKey === undefined ? {} : { workspaceKey: options.workspaceKey }), + }); const jsonWriter = options.json ? createBackpressureAwareWriter(process.stdout) : undefined; try { const connectionOptions = { brokerUrl: proxy.brokerUrl, apiKey: proxy.apiKey }; @@ -160,7 +175,12 @@ export interface LocalAgentDependencies { node: string, options: RemoteNodeAttachOptions ) => Promise; - attachNode: (name: string, mode: AttachMode, node: string, options: NativeAttachOptions) => Promise; + attachNode: ( + name: string, + mode: AttachMode, + node: string, + options: FleetNodeAttachCliOptions + ) => Promise; cwd: () => string; readConnectionFile: (stateDir: string) => unknown; getDefaultStateDir: () => string; @@ -573,6 +593,10 @@ export function registerLocalAgentCommands( '--state-dir ', 'Directory containing connection.json (with --ssh-host: path on target; auto-discovered when omitted)' ) + .option( + '--workspace-key ', + 'Relaycast workspace key for --node (overrides RELAY_WORKSPACE_KEY, the project pin, and the machine-global active workspace)' + ) .option('--json', 'Emit normalized agent events as NDJSON') .option('--reasoning', 'Include agent reasoning events') .option('--diagnostics', 'Include native harness diagnostics') @@ -585,6 +609,28 @@ export function registerLocalAgentCommands( } const sshHost = options.sshHost as string | undefined; const node = options.node as string | undefined; + // An all-whitespace flag must fall through to the normal precedence + // ladder rather than being sent as a literal credential. + const rawWorkspaceKey = options.workspaceKey as string | undefined; + const workspaceKey = rawWorkspaceKey?.trim() ? rawWorkspaceKey.trim() : undefined; + // Only the fleet path authenticates with a workspace key. The local and + // SSH paths speak the broker contract, so accepting the flag there would + // silently ignore it and send the caller to the wrong workspace. Gate on + // the raw option rather than the normalized one: `--workspace-key "$KEY"` + // with an unset variable is still a caller asking for a workspace, and + // must be told so instead of being routed to a broker. + if (rawWorkspaceKey !== undefined && node === undefined) { + // The two rejected paths take different credentials, so name the ones + // the caller's path actually accepts — --ssh-host rejects + // --broker-url / --api-key and reads connection.json on the target. + deps.error( + sshHost !== undefined + ? 'Error: --workspace-key requires --node. The --ssh-host attach path reads the target broker connection.json — locate it with --state-dir instead.' + : 'Error: --workspace-key requires --node. The local attach path uses --broker-url / --api-key or reads connection.json from --state-dir instead.' + ); + deps.exit(1); + return; + } if (node !== undefined && sshHost !== undefined) { deps.error( 'Error: --node cannot be combined with --ssh-host. Use --ssh-host only as the explicit SSH fallback.' @@ -606,6 +652,7 @@ export function registerLocalAgentCommands( } try { const code = await deps.attachNode(name, mode, node, { + workspaceKey, json: options.json as boolean | undefined, reasoning: options.reasoning as boolean | undefined, diagnostics: options.diagnostics as boolean | undefined, diff --git a/packages/cli/src/cli/lib/attach-fleet-node.test.ts b/packages/cli/src/cli/lib/attach-fleet-node.test.ts index cf43e9dce..7b5779f29 100644 --- a/packages/cli/src/cli/lib/attach-fleet-node.test.ts +++ b/packages/cli/src/cli/lib/attach-fleet-node.test.ts @@ -262,3 +262,74 @@ describe('startFleetNodeAttachProxy delivery-mode PUT lifecycle', () => { expect(elapsedMs).toBeLessThan(5_000); }, 10_000); }); + +describe('startFleetNodeAttachProxy workspace-key precedence', () => { + const cleanup: Array<() => Promise> = []; + + afterEach(async () => { + while (cleanup.length > 0) { + const fn = cleanup.pop()!; + await fn().catch(() => undefined); + } + }); + + /** Ticket fetch that records the Authorization header it was called with. */ + function capturingTicketFetch(remoteUrl: string): { + fetch: typeof globalThis.fetch; + authorization: () => string | undefined; + } { + let seen: string | undefined; + const fetchFn = (async (_url: string, init?: RequestInit) => { + const headers = (init?.headers ?? {}) as Record; + seen = headers.Authorization; + return { + ok: true, + status: 200, + json: async () => ({ + ok: true, + data: { + session_id: SESSION_ID, + terminal_url: `${remoteUrl}?ticket=abc`, + resume_token: RESUME_TOKEN, + }, + }), + } as unknown as Response; + }) as unknown as typeof globalThis.fetch; + return { fetch: fetchFn, authorization: () => seen }; + } + + it('presents an explicit workspace key ahead of the ambient environment', async () => { + const remote = await startFakeRemote(); + cleanup.push(remote.close); + const ticket = capturingTicketFetch(remote.url); + const proxy = await startFleetNodeAttachProxy({ + agent: 'agent-e', + node: 'node-e', + mode: 'view', + baseUrl: 'https://fake.example', + workspaceKey: 'rk_live_explicit', + env: { RELAY_WORKSPACE_KEY: 'rk_live_ambient' }, + fetch: ticket.fetch, + }); + cleanup.push(proxy.close); + + expect(ticket.authorization()).toBe('Bearer rk_live_explicit'); + }); + + it('falls back to the environment when no explicit key is supplied', async () => { + const remote = await startFakeRemote(); + cleanup.push(remote.close); + const ticket = capturingTicketFetch(remote.url); + const proxy = await startFleetNodeAttachProxy({ + agent: 'agent-f', + node: 'node-f', + mode: 'view', + baseUrl: 'https://fake.example', + env: { RELAY_WORKSPACE_KEY: 'rk_live_ambient' }, + fetch: ticket.fetch, + }); + cleanup.push(proxy.close); + + expect(ticket.authorization()).toBe('Bearer rk_live_ambient'); + }); +});