From 4acdd97d4e5bd80a3136c72974bdbf5f2bdd85ed Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 03:52:38 +0000 Subject: [PATCH 1/7] fix(cli): resolve the broker workspace through one precedence ladder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent-relay up` / `node up` hand-rolled a narrower workspace chain than the SDK: flag -> env -> repository pin -> give up, with a `RELAY_NODE_TOKEN` short-circuit at the top. Two failures fell out of it. A Cloud enrollment set `RELAY_NODE_TOKEN` and no workspace key, so the pin was skipped and the broker, left with no key candidates, minted a fresh workspace — re-homing an enrolled node out of its repository's workspace. And a fresh directory never consulted the machine-global store, so a first start minted a new workspace even when the machine already had an active one selected. Both now resolve through one ladder, documented in `resolveWorkspaceSelection` and in the CLI README: 1. --workspace-key / --wk 2. RELAY_WORKSPACE_KEY > AGENT_RELAY_WORKSPACE_KEY > RELAY_API_KEY 3. /.agentworkforce/relay/workspace-key.json 4. the active entry in ~/.agentworkforce/relay/workspaces.json 5. create a workspace — only when nothing above resolves The repository pin always outranks the machine-global entry, and a node token selects node identity only, so it no longer suppresses steps 1-4. Startup prints the winning source (flag name, variable, or path — never key material) and says explicitly when it created a workspace rather than joined one. Each start now records the resolved workspace id on the pin, so a later start can detect a stored enrollment pointing at a different workspace and stop with both source paths named instead of silently choosing one. Fixes #1406 Fixes #1378 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P5FD3oEnWbMbAhjsdtmmiP --- CHANGELOG.md | 3 + packages/cli/README.md | 36 +++++ packages/cli/src/cli/commands/core.ts | 5 + packages/cli/src/cli/commands/node.test.ts | 66 +++++++- packages/cli/src/cli/commands/node.ts | 65 ++++++-- .../cli/src/cli/lib/broker-lifecycle.test.ts | 104 ++++++++++++- packages/cli/src/cli/lib/broker-lifecycle.ts | 141 +++++++++++++++--- .../cli/src/cli/lib/project-workspace-key.ts | 3 + packages/cloud/src/index.ts | 3 + .../cloud/src/project-workspace-key.test.ts | 54 +++++++ packages/cloud/src/project-workspace-key.ts | 111 +++++++++++--- packages/cloud/src/workspace-key.ts | 3 + packages/harness-driver/src/client.ts | 3 + 13 files changed, 546 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9ef68e9e..aeef27b06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. +- `agent-relay up` / `node up` resolve the workspace through one documented precedence ladder: `--workspace-key` → `RELAY_WORKSPACE_KEY`/`AGENT_RELAY_WORKSPACE_KEY`/`RELAY_API_KEY` → the repository pin in `.agentworkforce/relay/workspace-key.json` → the machine-global active workspace in `~/.agentworkforce/relay/workspaces.json` → creating one. Startup prints the winning source (flag, variable, or file path — never key material). +- A Cloud enrollment no longer re-homes an enrolled node out of its repository's workspace. `RELAY_NODE_TOKEN` selects the node's identity, not its workspace, and no longer suppresses the repository pin; when a stored enrollment addresses a different workspace than the pin, `node up` stops and names both sources instead of silently choosing one. +- A first `up` in a fresh directory joins the machine's active workspace instead of silently creating a new one, and a start that does create a workspace says so instead of printing the same output as a join. ## [11.4.0] - 2026-08-02 diff --git a/packages/cli/README.md b/packages/cli/README.md index adfd75b97..f1e15864d 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -47,6 +47,42 @@ agent-relay node agent release For AI SDK native harnesses, attach renders structured activity, text, tools, approvals, files, usage, and lifecycle events. Add `--json` for NDJSON, `--reasoning` for reasoning events, or `--diagnostics` for sidecar diagnostics. Native harness `drive` is line-oriented and acknowledged; native harness `passthrough` is unsupported because no terminal stream exists. PTY attach behavior is unchanged. +### Which workspace a broker joins + +`agent-relay up` and `agent-relay node up` resolve the workspace through one +precedence ladder. The first source that resolves wins: + +| # | Source | Where it comes from | +| --- | ------------------------------- | ----------------------------------------------------------------------------- | +| 1 | Command-line flag | `--workspace-key` / `--wk` | +| 2 | Environment | `RELAY_WORKSPACE_KEY`, then `AGENT_RELAY_WORKSPACE_KEY`, then `RELAY_API_KEY` | +| 3 | Repository pin | `/.agentworkforce/relay/workspace-key.json` | +| 4 | Machine-global active workspace | the `active` entry in `~/.agentworkforce/relay/workspaces.json` | +| 5 | New workspace | created only when nothing above resolves | + +Two rules follow from the order: + +- **The repository pin always beats the machine-global active workspace.** + Switching your active workspace (`agent-relay workspace use `) never + re-homes a checkout that already pinned one. +- **A new workspace is a last resort, not a default.** A fresh directory joins + the machine's active workspace when one is selected. When nothing resolves and + a workspace is created, startup says so explicitly. + +Startup prints the winning source (a flag name, an environment variable, or a +file path — never key material): + +``` +Workspace source: repository pin (/repo/.agentworkforce/relay/workspace-key.json) +Workspace: joined rw_7ccfea89 +``` + +A Cloud enrollment (`RELAY_NODE_TOKEN`, or a record in the Fleet enrollment +store) selects the node's _identity_, not its workspace, so it never appears on +this ladder. If a stored enrollment addresses a different workspace than the +repository pin, `node up` refuses to start and names both source files rather +than silently choosing one. + ## Remote fleet agents The `fleet` command group lists and controls agents across all live nodes in diff --git a/packages/cli/src/cli/commands/core.ts b/packages/cli/src/cli/commands/core.ts index 5b5008160..aa5b5256e 100644 --- a/packages/cli/src/cli/commands/core.ts +++ b/packages/cli/src/cli/commands/core.ts @@ -59,6 +59,8 @@ export interface CoreRelay { shutdown: () => Promise; /** Agent Relay workspace key, available after the hello handshake. */ workspaceKey?: string; + /** Relay workspace id the broker joined, available after the hello handshake. */ + workspaceId?: string; /** PID of the underlying broker process, when available. */ brokerPid?: number; /** Actual HTTP API port bound by the broker, including OS-assigned ports. */ @@ -187,6 +189,9 @@ async function createDefaultRelay( get workspaceKey() { return client.workspaceKey; }, + get workspaceId() { + return client.workspaceId; + }, get brokerPid() { return client.brokerPid; }, diff --git a/packages/cli/src/cli/commands/node.test.ts b/packages/cli/src/cli/commands/node.test.ts index 7cda1db5d..6d7779d1f 100644 --- a/packages/cli/src/cli/commands/node.test.ts +++ b/packages/cli/src/cli/commands/node.test.ts @@ -48,7 +48,14 @@ function createNodeHarness(opts?: { const error = vi.fn(); const warn = vi.fn(); - const core = { env, exit, log, error, warn } as unknown as CoreDependencies; + const core = { + env, + exit, + log, + error, + warn, + getProjectPaths: () => ({ projectRoot: '/repo', dataDir: '/repo/.agentworkforce/relay' }), + } as unknown as CoreDependencies; const resolveEnrollment = opts?.resolveEnrollment ?? (vi.fn(() => undefined) as unknown as NodeCommandDependencies['resolveEnrollment']); @@ -243,7 +250,7 @@ describe('registerNodeCommands', () => { expect(env.RELAY_NODE_TOKEN).toBeUndefined(); }); - it('resumes a project-pinned workspace instead of replacing it with an enrollment', async () => { + it('never adopts an enrollment for a project that pinned its own workspace', async () => { const resolveEnrollment = vi.fn( () => enrollmentRecord ) as unknown as NodeCommandDependencies['resolveEnrollment']; @@ -258,9 +265,10 @@ describe('registerNodeCommands', () => { await program.parseAsync(['node', 'up'], { from: 'user' }); + // A pin without an enrolled node id never reaches for the machine-global + // enrollment store, and no node token is applied — so `runUpCommand`'s + // precedence ladder resolves the repository pin unopposed. expect(resolveEnrollment).not.toHaveBeenCalled(); - expect(env.RELAY_WORKSPACE_KEY).toBe('rk_project_session'); - expect(env.RELAY_API_KEY).toBe('rk_project_session'); expect(env.RELAY_NODE_TOKEN).toBeUndefined(); expect(brokerMocks.runUpCommand).toHaveBeenCalledTimes(1); }); @@ -342,6 +350,56 @@ describe('registerNodeCommands', () => { expect(warn).not.toHaveBeenCalled(); }); + it('refuses to start when the enrollment and the repository pin disagree (#1406)', async () => { + const resolveEnrollment = vi.fn( + () => enrollmentRecord + ) as unknown as NodeCommandDependencies['resolveEnrollment']; + const { program, env, error, exit } = createNodeHarness({ + env: { AGENT_RELAY_HOME: '/tmp/relay-home-fixture' }, + resolveEnrollment, + // A previous start recorded rw_stale; the enrollment points at rw_123. + resolveProjectWorkspaceSession: vi.fn(() => ({ + workspaceKey: 'rk_project_session', + enrolledNodeId: 'node_abc', + workspaceId: 'rw_stale', + })), + }); + + await expect(program.parseAsync(['node', 'up'], { from: 'user' })).rejects.toBeInstanceOf(ExitSignal); + + expect(exit).toHaveBeenCalledWith(1); + const message = error.mock.calls.flat().join('\n'); + expect(message).toContain('select different workspaces'); + expect(message).toContain('rw_stale'); + expect(message).toContain('rw_123'); + expect(message).toContain('workspace-key.json'); + // Diagnostics name sources, never credentials. + expect(message).not.toContain('rk_project_session'); + expect(message).not.toContain('nt_secret'); + expect(env.RELAY_NODE_TOKEN).toBeUndefined(); + expect(brokerMocks.runUpCommand).not.toHaveBeenCalled(); + }); + + it('starts normally when the enrollment matches the pinned workspace', async () => { + const resolveEnrollment = vi.fn( + () => enrollmentRecord + ) as unknown as NodeCommandDependencies['resolveEnrollment']; + const { program, env } = createNodeHarness({ + env: {}, + resolveEnrollment, + resolveProjectWorkspaceSession: vi.fn(() => ({ + workspaceKey: 'rk_project_session', + enrolledNodeId: 'node_abc', + workspaceId: 'rw_123', + })), + }); + + await program.parseAsync(['node', 'up'], { from: 'user' }); + + expect(env.RELAY_NODE_TOKEN).toBe('nt_secret'); + expect(brokerMocks.runUpCommand).toHaveBeenCalledTimes(1); + }); + it('preserves an enrolled identity across a consecutive project-session restart', async () => { const firstResolveEnrollment = vi.fn( () => enrollmentRecord diff --git a/packages/cli/src/cli/commands/node.ts b/packages/cli/src/cli/commands/node.ts index febc49231..94102133d 100644 --- a/packages/cli/src/cli/commands/node.ts +++ b/packages/cli/src/cli/commands/node.ts @@ -1,5 +1,6 @@ import type { Command } from 'commander'; import { + fleetNodeEnrollmentStorePath, readFleetNodeEnrollmentStore, resolveActiveFleetNodeEnrollment, type FleetNodeEnrollmentRecord, @@ -13,7 +14,11 @@ import { type UpCommandOptions, } from './core.js'; import { runUpCommand } from '../lib/broker-lifecycle.js'; -import { readProjectWorkspaceSession, type ProjectWorkspaceSession } from '../lib/project-workspace-key.js'; +import { + projectWorkspaceKeyPath, + readProjectWorkspaceSession, + type ProjectWorkspaceSession, +} from '../lib/project-workspace-key.js'; import { promoteWorkspaceKeyEnvAlias } from '../lib/workspace-env.js'; import { registerLocalAgentCommands } from './local-agent.js'; import { registerLocalWorkflowCommands } from './local-workflow.js'; @@ -94,10 +99,41 @@ function prepareExplicitWorkspaceForNodeUp( return Boolean(options.workspaceKey?.trim() || envWorkspaceKey); } -/** Apply a project-pinned workspace without changing the persisted enrolled-node association. */ -function resumeProjectWorkspace(session: ProjectWorkspaceSession, deps: NodeCommandDependencies): void { - deps.core.env.RELAY_WORKSPACE_KEY = session.workspaceKey; - deps.core.env.RELAY_API_KEY = session.workspaceKey; +/** + * Refuse to start when the stored enrollment addresses a different workspace + * than the repository pin. + * + * The enrollment store is machine-global; the pin is per-repository. When they + * disagree, silently preferring either one re-homes the node — so name both + * sources and stop. Only possible once a previous start recorded the pin's + * workspace id; before that the two are simply passed through together (the + * pin wins for workspace selection, the enrollment for node identity) and a + * mismatched node token fails loudly at registration instead. + */ +function reportWorkspaceSourceConflict( + record: NonNullable>, + session: ProjectWorkspaceSession | undefined, + deps: NodeCommandDependencies +): boolean { + const pinnedWorkspaceId = session?.workspaceId?.trim(); + const enrolledWorkspaceId = record.relayWorkspaceId?.trim(); + if (!pinnedWorkspaceId || !enrolledWorkspaceId || pinnedWorkspaceId === enrolledWorkspaceId) { + return false; + } + + const pinPath = projectWorkspaceKeyPath(deps.core.getProjectPaths().dataDir); + deps.error( + 'Refusing to start: this repository and the stored Fleet enrollment select different workspaces.' + ); + deps.error(` repository pin ${pinPath} -> workspace ${pinnedWorkspaceId}`); + deps.error( + ` fleet enrollment ${fleetNodeEnrollmentStorePath(deps.core.env)} -> workspace ${enrolledWorkspaceId} (node ${record.nodeId})` + ); + deps.error( + 'Pass --workspace-key to choose explicitly, re-enroll this node in the pinned workspace, ' + + 'or delete the repository pin to adopt the enrollment.' + ); + return true; } /** Apply stored enrollment credentials and return the enrolled node name, when present. */ @@ -174,7 +210,14 @@ function resolveEnrollmentForProject( }); } -/** Apply an enrollment or safely resume a project workspace when its enrollment is unavailable. */ +/** + * Apply the node identity for this start. + * + * Workspace selection is NOT decided here — `runUpCommand` walks the shared + * precedence ladder (flag → env → repository pin → machine-global active) after + * this returns. This function only settles which node identity the broker runs + * as, so an enrollment can no longer suppress the repository's workspace. + */ function applyResolvedNodeSession( record: ReturnType | undefined, projectSession: ProjectWorkspaceSession | undefined, @@ -183,16 +226,12 @@ function applyResolvedNodeSession( if (record) { return applyEnrollment(record, deps); } - if (!projectSession) { - return undefined; - } - if (projectSession.enrolledNodeId) { + if (projectSession?.enrolledNodeId) { deps.core.env.AGENT_RELAY_ENROLLED_NODE_ID = projectSession.enrolledNodeId; deps.warn( `Persisted enrollment for node "${projectSession.enrolledNodeId}" was not found; resuming the pinned workspace without that node identity.` ); } - resumeProjectWorkspace(projectSession, deps); return undefined; } @@ -228,6 +267,10 @@ async function runNodeUp(options: UpCommandOptions, deps: NodeCommandDependencie deps.exit(1); return; } + if (record && reportWorkspaceSourceConflict(record, projectSession, deps)) { + deps.exit(1); + return; + } // Serve under the enrolled name (mirrors the old `fleet serve // --enrollment-token` behavior where --name beat the enrollment name). enrolledNodeName = applyResolvedNodeSession(record, projectSession, deps); diff --git a/packages/cli/src/cli/lib/broker-lifecycle.test.ts b/packages/cli/src/cli/lib/broker-lifecycle.test.ts index 299d796cd..fceb00920 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.test.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.test.ts @@ -221,6 +221,7 @@ import fsReal from 'node:fs'; import os from 'node:os'; import pathReal from 'node:path'; import { startServeNode } from '@agent-relay/fleet'; +import { setWorkspaceKey } from '@agent-relay/cloud'; import { runUpCommand } from './broker-lifecycle.js'; import { startReflexCapture } from './reflex-capture.js'; class ExitSignal extends Error { @@ -248,6 +249,7 @@ function createUpHarness() { getStatus: vi.fn(async () => ({})), shutdown: vi.fn(async () => undefined), workspaceKey: 'rk_test', + workspaceId: 'rw_test', })); const exit = vi.fn((code: number) => { throw new ExitSignal(code); @@ -297,7 +299,19 @@ function createUpHarness() { exit, } as unknown as CoreDependencies; - return { deps, projectRoot, createRelay, log, warn, error, exit }; + // Every start now consults the machine-global workspace store, so point it at + // a scratch home instead of the developer's real one. + const home = fsReal.mkdtempSync(pathReal.join(os.tmpdir(), 'broker-lifecycle-home-')); + upTmpRoots.push(home); + (deps.env as NodeJS.ProcessEnv).AGENT_RELAY_HOME = home; + + return { deps, projectRoot, dataDir, home, createRelay, log, warn, error, exit }; +} + +/** Pin a workspace to the harness project, as a previous `up` would have. */ +function writeRepositoryPin(dataDir: string, session: Record): void { + fsReal.mkdirSync(dataDir, { recursive: true }); + fsReal.writeFileSync(pathReal.join(dataDir, 'workspace-key.json'), JSON.stringify(session, null, 2)); } afterEach(() => { @@ -480,6 +494,94 @@ describe('runUpCommand node-config gating', () => { }); }); +describe('runUpCommand workspace precedence', () => { + const readPin = (dataDir: string): Record => + JSON.parse(fsReal.readFileSync(pathReal.join(dataDir, 'workspace-key.json'), 'utf-8')); + + it('prefers the repository pin over the machine-global active workspace (#1406)', async () => { + const { deps, dataDir, home, log } = createUpHarness(); + setWorkspaceKey('stale-global', 'rk_stale_global', { AGENT_RELAY_HOME: home }); + writeRepositoryPin(dataDir, { workspaceKey: 'rk_repository', workspaceId: 'rw_repository' }); + + await runUpCommand({}, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_repository'); + expect(deps.env.RELAY_API_KEY).toBe('rk_repository'); + expect(log.mock.calls.flat().join('\n')).toContain('Workspace source: repository pin'); + }); + + it('applies the repository pin even when an enrollment node token is present (#1406)', async () => { + const { deps, dataDir } = createUpHarness(); + // The harness env already carries RELAY_NODE_TOKEN, which is exactly the + // condition that used to skip the pin and let the broker mint instead. + expect(deps.env.RELAY_NODE_TOKEN).toBeTruthy(); + writeRepositoryPin(dataDir, { workspaceKey: 'rk_repository', enrolledNodeId: 'node_a' }); + + await runUpCommand({}, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_repository'); + expect(deps.env.AGENT_RELAY_ENROLLED_NODE_ID).toBe('node_a'); + }); + + it('joins the machine-global active workspace in a fresh directory instead of minting (#1378)', async () => { + const { deps, home, log } = createUpHarness(); + setWorkspaceKey('account', 'rk_account_active', { AGENT_RELAY_HOME: home }); + + await runUpCommand({}, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_account_active'); + const output = log.mock.calls.flat().join('\n'); + expect(output).toContain('Workspace source: machine-global active workspace'); + expect(output).toContain('active: "account"'); + expect(output).not.toContain('created new workspace'); + }); + + it('announces a mint when no source resolves (#1378)', async () => { + const { deps, log } = createUpHarness(); + + await runUpCommand({}, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBeUndefined(); + const output = log.mock.calls.flat().join('\n'); + expect(output).toContain('Workspace: none selected'); + expect(output).toContain('Workspace: created new workspace rw_test'); + }); + + it('keeps an explicit --workspace-key ahead of both stores', async () => { + const { deps, dataDir, home, log } = createUpHarness(); + setWorkspaceKey('global', 'rk_global', { AGENT_RELAY_HOME: home }); + writeRepositoryPin(dataDir, { workspaceKey: 'rk_repository' }); + + await runUpCommand({ workspaceKey: 'rk_flag' }, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_flag'); + expect(log.mock.calls.flat().join('\n')).toContain('Workspace source: command-line flag'); + }); + + it('records the resolved workspace id on the pin for later conflict detection', async () => { + const { deps, dataDir } = createUpHarness(); + + await runUpCommand({}, deps); + + expect(readPin(dataDir)).toMatchObject({ workspaceKey: 'rk_test', workspaceId: 'rw_test' }); + }); + + it('never prints workspace key material while reporting the winning source', async () => { + const { deps, dataDir, home, log, warn, error } = createUpHarness(); + setWorkspaceKey('global', 'rk_global', { AGENT_RELAY_HOME: home }); + writeRepositoryPin(dataDir, { workspaceKey: 'rk_repository' }); + + await runUpCommand({}, deps); + + const output = [log, warn, error] + .flatMap((fn) => vi.mocked(fn).mock.calls.flat()) + .map((arg) => String(arg)) + .join('\n'); + expect(output).not.toContain('rk_repository'); + expect(output).not.toContain('rk_global'); + }); +}); + describe('resolveNodeIdentityFromSession', () => { const noSleep = vi.fn(async () => {}); diff --git a/packages/cli/src/cli/lib/broker-lifecycle.ts b/packages/cli/src/cli/lib/broker-lifecycle.ts index 520adb93b..3be312a83 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.ts @@ -24,7 +24,12 @@ import { import { describeError } from './describe-error.js'; import { maskSecret } from './redact.js'; import { startReflexCapture, type RunningReflexCapture } from './reflex-capture.js'; -import { projectWorkspaceKeyPath, writeProjectWorkspaceKey } from './project-workspace-key.js'; +import { + projectWorkspaceKeyPath, + resolveActiveWorkspaceSelection, + writeProjectWorkspaceKey, + type WorkspaceSelection, +} from './project-workspace-key.js'; import { promoteWorkspaceKeyEnvAlias } from './workspace-env.js'; type UpOptions = { @@ -1290,6 +1295,7 @@ function planCapacitySource( interface PinnedProjectWorkspaceSession { workspaceKey: string; enrolledNodeId?: string; + workspaceId?: string; } /** Read the minimal project session needed during broker startup. */ @@ -1301,43 +1307,125 @@ function readPinnedProjectWorkspaceSession( const parsed = JSON.parse(deps.fs.readFileSync(projectWorkspaceKeyPath(dataDir), 'utf8')) as Partial<{ workspaceKey: string; enrolledNodeId: string; + workspaceId: string; }>; - const workspaceKey = - typeof parsed.workspaceKey === 'string' ? parsed.workspaceKey.trim() || undefined : undefined; + const workspaceKey = trimmedOrUndefined(parsed.workspaceKey); if (!workspaceKey) { return undefined; } - const enrolledNodeId = - typeof parsed.enrolledNodeId === 'string' ? parsed.enrolledNodeId.trim() || undefined : undefined; + const enrolledNodeId = trimmedOrUndefined(parsed.enrolledNodeId); + const workspaceId = trimmedOrUndefined(parsed.workspaceId); return { workspaceKey, ...(enrolledNodeId ? { enrolledNodeId } : {}), + ...(workspaceId ? { workspaceId } : {}), }; } catch { return undefined; } } -/** Resume the pinned project session unless explicit credentials override it. */ -function resumePinnedProjectWorkspace( +/** Narrow an unknown JSON field to a non-blank string. */ +function trimmedOrUndefined(value: unknown): string | undefined { + return typeof value === 'string' ? value.trim() || undefined : undefined; +} + +/** + * Resolve the workspace this broker start joins, walking the shared precedence + * ladder: `--workspace-key` → env → the repository pin → the machine-global + * active workspace. Nothing resolving means the broker will mint a workspace. + * + * The repository pin is read through {@link CoreDependencies.fs} (tests stub it) + * while the machine-global store is read by the shared cloud resolver, so both + * halves of the ladder stay in one place. + * + * A Fleet enrollment (`RELAY_NODE_TOKEN`) selects the node's identity, not its + * workspace, and no longer short-circuits this walk — letting it do so is what + * re-homed an enrolled node out of its repository's workspace and into a + * freshly minted one. + */ +function resolveWorkspaceForBrokerStart( options: UpOptions, deps: CoreDependencies, projectDataDir: string -): PinnedProjectWorkspaceSession | undefined { +): WorkspaceSelection | undefined { + const flag = options.workspaceKey?.trim(); + if (flag) { + return { key: flag, source: 'flag', origin: '--workspace-key' }; + } + const explicitEnvWorkspaceKey = promoteWorkspaceKeyEnvAlias(deps.env); - if (options.workspaceKey?.trim() || explicitEnvWorkspaceKey || deps.env.RELAY_NODE_TOKEN?.trim()) { + if (explicitEnvWorkspaceKey) { + return { key: explicitEnvWorkspaceKey, source: 'env', origin: '$RELAY_WORKSPACE_KEY' }; + } + + const pinned = readPinnedProjectWorkspaceSession(projectDataDir, deps); + if (pinned) { + return { + key: pinned.workspaceKey, + source: 'project', + origin: projectWorkspaceKeyPath(projectDataDir), + ...(pinned.workspaceId ? { workspaceId: pinned.workspaceId } : {}), + }; + } + + // Everything below the repository pin: the machine-global active workspace. + // Without this step a fresh checkout mints its own workspace even though the + // machine already has an active one selected. + return resolveActiveWorkspaceSelection(deps.env); +} + +/** + * Apply the resolved workspace to the environment the broker (and any detached + * child) inherits, and report which source won. Returns the pinned project + * session when the repository pin supplied the selection. + */ +function applyWorkspaceSelection( + selection: WorkspaceSelection | undefined, + deps: CoreDependencies, + projectDataDir: string +): PinnedProjectWorkspaceSession | undefined { + if (!selection) { + deps.log( + 'Workspace: none selected (no --workspace-key, no RELAY_WORKSPACE_KEY, no repository pin, ' + + 'no active workspace in the machine-global store). A new workspace will be created.' + ); return undefined; } - const session = readPinnedProjectWorkspaceSession(projectDataDir, deps); - if (session) { - deps.env.RELAY_WORKSPACE_KEY = session.workspaceKey; - deps.env.RELAY_API_KEY = session.workspaceKey; - if (session.enrolledNodeId) { - deps.env.AGENT_RELAY_ENROLLED_NODE_ID = session.enrolledNodeId; - } + deps.log(`Workspace source: ${describeWorkspaceSource(selection.source)} (${selection.origin})`); + if (selection.source === 'flag' || selection.source === 'env') { + // Both already live in the environment the broker inherits: the flag is + // exported by runUpCommand before the --background fork, and an env alias + // was promoted to RELAY_WORKSPACE_KEY during resolution. Writing + // RELAY_API_KEY here would clobber a value the caller set deliberately. + return undefined; + } + deps.env.RELAY_WORKSPACE_KEY = selection.key; + deps.env.RELAY_API_KEY = selection.key; + if (selection.source !== 'project') { + return undefined; + } + + const pinned = readPinnedProjectWorkspaceSession(projectDataDir, deps); + if (pinned?.enrolledNodeId) { + deps.env.AGENT_RELAY_ENROLLED_NODE_ID = pinned.enrolledNodeId; + } + return pinned; +} + +/** Human-readable name for a precedence-ladder step, for the startup line. */ +function describeWorkspaceSource(source: WorkspaceSelection['source']): string { + switch (source) { + case 'flag': + return 'command-line flag'; + case 'env': + return 'environment'; + case 'project': + return 'repository pin'; + case 'store': + return 'machine-global active workspace'; } - return session; } export async function runUpCommand(options: UpOptions, deps: CoreDependencies): Promise { @@ -1350,7 +1438,8 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): // --state-dir), so the key must be persisted here even when broker state is // redirected elsewhere. const projectWorkspaceKeyDataDir = paths.dataDir; - const resumedProjectSession = resumePinnedProjectWorkspace(options, deps, projectWorkspaceKeyDataDir); + const workspaceSelection = resolveWorkspaceForBrokerStart(options, deps, projectWorkspaceKeyDataDir); + const resumedProjectSession = applyWorkspaceSelection(workspaceSelection, deps, projectWorkspaceKeyDataDir); // --state-dir overrides where the broker writes state / connection files if (options.stateDir) { const resolved = path.resolve(options.stateDir); @@ -1573,6 +1662,18 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): deps.log(`Project: ${paths.projectRoot}`); deps.log('Mode: broker (stdio)'); deps.log(`Workspace Key: ${relay.workspaceKey ? maskSecret(relay.workspaceKey) : 'unknown'}`); + // Minting must be observable: without this line "created a workspace" and + // "joined the pinned workspace" print identically. + const joinedWorkspaceId = relay.workspaceId ?? 'unknown'; + if (workspaceSelection) { + deps.log(`Workspace: joined ${joinedWorkspaceId}`); + } else { + deps.log(`Workspace: created new workspace ${joinedWorkspaceId}`); + deps.log( + 'Pin a workspace for this repository with `agent-relay up --workspace-key `, ' + + 'or select one machine-wide with `agent-relay workspace use `.' + ); + } deps.log('Broker started.'); // Record the workspace this broker joined (explicitly passed or auto-minted) @@ -1583,6 +1684,10 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): try { writeProjectWorkspaceKey(projectWorkspaceKeyDataDir, relay.workspaceKey ?? undefined, { enrolledNodeId: deps.env.AGENT_RELAY_ENROLLED_NODE_ID ?? resumedProjectSession?.enrolledNodeId, + // Recording the resolved workspace id lets the NEXT start detect a + // conflicting source (a stored enrollment in another workspace) before + // the broker comes up, instead of after agents land in the wrong place. + workspaceId: relay.workspaceId ?? resumedProjectSession?.workspaceId, }); } catch { // best-effort: a broker that came up should stay up even if the key file diff --git a/packages/cli/src/cli/lib/project-workspace-key.ts b/packages/cli/src/cli/lib/project-workspace-key.ts index 0cac556c4..294542ab7 100644 --- a/packages/cli/src/cli/lib/project-workspace-key.ts +++ b/packages/cli/src/cli/lib/project-workspace-key.ts @@ -4,6 +4,9 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, + resolveWorkspaceSelection, writeProjectWorkspaceKey, type ProjectWorkspaceSession, + type WorkspaceSelection, } from '@agent-relay/cloud/workspace-key'; diff --git a/packages/cloud/src/index.ts b/packages/cloud/src/index.ts index b11339ea7..c748465a3 100644 --- a/packages/cloud/src/index.ts +++ b/packages/cloud/src/index.ts @@ -136,12 +136,15 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, resolveWorkspaceKey, resolveWorkspaceKeyWithSource, + resolveWorkspaceSelection, writeProjectWorkspaceKey, type ProjectWorkspaceSession, type ResolveWorkspaceKeyOptions, type WorkspaceKeySource, + type WorkspaceSelection, } from './project-workspace-key.js'; export { diff --git a/packages/cloud/src/project-workspace-key.test.ts b/packages/cloud/src/project-workspace-key.test.ts index 20e39662f..d5b6386b1 100644 --- a/packages/cloud/src/project-workspace-key.test.ts +++ b/packages/cloud/src/project-workspace-key.test.ts @@ -9,6 +9,7 @@ import { readProjectWorkspaceKey, readProjectWorkspaceSession, resolveWorkspaceKeyWithSource, + resolveWorkspaceSelection, writeProjectWorkspaceKey, } from './project-workspace-key.js'; import { setWorkspaceKey } from './workspace-store.js'; @@ -92,3 +93,56 @@ describe('project workspace key resolution', () => { ).toBeUndefined(); }); }); + +describe('workspace precedence ladder diagnostics', () => { + it('round-trips the resolved workspace id on the project pin', () => { + writeProjectWorkspaceKey(dataDir, 'rk_project', { workspaceId: ' rw_pinned ' }); + expect(readProjectWorkspaceSession(dataDir)).toEqual({ + workspaceKey: 'rk_project', + workspaceId: 'rw_pinned', + }); + expect( + resolveWorkspaceSelection({ projectDataDir: dataDir, env: { AGENT_RELAY_HOME: home } })?.workspaceId + ).toBe('rw_pinned'); + }); + + it('names each source without leaking key material', () => { + const env = { AGENT_RELAY_HOME: home, AGENT_RELAY_WORKSPACE_KEY: 'rk_env' }; + setWorkspaceKey('global', 'rk_global', env); + writeProjectWorkspaceKey(dataDir, 'rk_project'); + + const flag = resolveWorkspaceSelection({ workspaceKey: 'rk_flag', projectDataDir: dataDir, env }); + expect(flag).toMatchObject({ key: 'rk_flag', source: 'flag', origin: '--workspace-key' }); + + const fromEnv = resolveWorkspaceSelection({ projectDataDir: dataDir, env }); + expect(fromEnv).toMatchObject({ source: 'env', origin: '$AGENT_RELAY_WORKSPACE_KEY' }); + + const project = resolveWorkspaceSelection({ + projectDataDir: dataDir, + env: { AGENT_RELAY_HOME: home }, + }); + expect(project).toMatchObject({ source: 'project', origin: projectWorkspaceKeyPath(dataDir) }); + + fs.rmSync(projectWorkspaceKeyPath(dataDir)); + const store = resolveWorkspaceSelection({ projectDataDir: dataDir, env: { AGENT_RELAY_HOME: home } }); + expect(store).toMatchObject({ key: 'rk_global', source: 'store' }); + expect(store?.origin).toContain('workspaces.json'); + expect(store?.origin).toContain('active: "global"'); + + for (const selection of [flag, fromEnv, project, store]) { + expect(selection?.origin).not.toContain(selection?.key ?? ''); + } + }); + + it('keeps the repository pin ahead of the machine-global active entry (#1406)', () => { + const env = { AGENT_RELAY_HOME: home }; + setWorkspaceKey('stale-global', 'rk_stale_global', env); + writeProjectWorkspaceKey(dataDir, 'rk_repository', { workspaceId: 'rw_repository' }); + + expect(resolveWorkspaceSelection({ projectDataDir: dataDir, env })).toMatchObject({ + key: 'rk_repository', + source: 'project', + workspaceId: 'rw_repository', + }); + }); +}); diff --git a/packages/cloud/src/project-workspace-key.ts b/packages/cloud/src/project-workspace-key.ts index 5fbb77de1..2fbe224fa 100644 --- a/packages/cloud/src/project-workspace-key.ts +++ b/packages/cloud/src/project-workspace-key.ts @@ -4,14 +4,23 @@ import path from 'node:path'; import { getProjectPaths } from '@agent-relay/config'; -import { resolveActiveWorkspaceKey } from './workspace-store.js'; +import { readWorkspaceStore, workspaceStorePath } from './workspace-store.js'; const PROJECT_WORKSPACE_KEY_FILENAME = 'workspace-key.json'; +/** Workspace-key environment aliases, highest precedence first. */ +const WORKSPACE_KEY_ENV_VARS = ['RELAY_WORKSPACE_KEY', 'AGENT_RELAY_WORKSPACE_KEY', 'RELAY_API_KEY'] as const; + export interface ProjectWorkspaceSession { workspaceKey: string; /** Enrolled Fleet node associated with this project session, when one started the broker. */ enrolledNodeId?: string; + /** + * Relay workspace id the pinned key resolved to on a previous start. Recorded + * so a later start can detect — before the broker comes up — that another + * source (a stored Fleet enrollment, say) points at a different workspace. + */ + workspaceId?: string; } export type WorkspaceKeySource = 'flag' | 'env' | 'project' | 'store'; @@ -25,6 +34,21 @@ export interface ResolveWorkspaceKeyOptions { projectDataDir?: string; } +/** + * A resolved workspace selection plus where it came from. + * + * `origin` is safe to print: it names a flag, an environment variable, or a + * file path — never key material. + */ +export interface WorkspaceSelection { + key: string; + source: WorkspaceKeySource; + /** Human-readable origin for diagnostics. Never contains key material. */ + origin: string; + /** Workspace id this selection is known to address, when previously recorded. */ + workspaceId?: string; +} + /** Absolute path to the workspace key recorded by `agent-relay node up`. */ export function projectWorkspaceKeyPath(dataDir: string): string { return path.join(dataDir, PROJECT_WORKSPACE_KEY_FILENAME); @@ -43,9 +67,11 @@ export function readProjectWorkspaceSession(dataDir: string): ProjectWorkspaceSe const workspaceKey = trimOrUndefined(parsed.workspaceKey); if (!workspaceKey) return undefined; const enrolledNodeId = trimOrUndefined(parsed.enrolledNodeId); + const workspaceId = trimOrUndefined(parsed.workspaceId); return { workspaceKey, ...(enrolledNodeId ? { enrolledNodeId } : {}), + ...(workspaceId ? { workspaceId } : {}), }; } catch { return undefined; @@ -59,11 +85,12 @@ export function readProjectWorkspaceSession(dataDir: string): ProjectWorkspaceSe export function writeProjectWorkspaceKey( dataDir: string, workspaceKey: string | undefined, - options: { enrolledNodeId?: string } = {} + options: { enrolledNodeId?: string; workspaceId?: string } = {} ): void { const key = trimOrUndefined(workspaceKey); if (!key) return; const enrolledNodeId = trimOrUndefined(options.enrolledNodeId); + const workspaceId = trimOrUndefined(options.workspaceId); fs.mkdirSync(dataDir, { recursive: true, mode: 0o700 }); const file = projectWorkspaceKeyPath(dataDir); // Worker threads share a PID, so include a per-write nonce as well as the PID. @@ -72,6 +99,7 @@ export function writeProjectWorkspaceKey( { workspaceKey: key, ...(enrolledNodeId ? { enrolledNodeId } : {}), + ...(workspaceId ? { workspaceId } : {}), } satisfies ProjectWorkspaceSession, null, 2 @@ -102,29 +130,78 @@ export function writeProjectWorkspaceKey( } /** - * Resolve the Relay workspace used by SDK clients. The project-local key comes - * before the machine-global active workspace so a process addresses the same - * workspace as the broker and fleet node running in that checkout. + * Resolve which Relay workspace this process addresses. + * + * This is THE workspace precedence ladder — every caller (SDK clients, the CLI, + * `agent-relay up` / `node up`) resolves through it so a repository cannot end + * up in one workspace and its tooling in another: + * + * 1. `flag` — an explicit `--workspace-key` / `--wk`. + * 2. `env` — `RELAY_WORKSPACE_KEY` > `AGENT_RELAY_WORKSPACE_KEY` > `RELAY_API_KEY`. + * 3. `project` — the repository pin, `/.agentworkforce/relay/workspace-key.json`. + * 4. `store` — the machine-global active entry in `~/.agentworkforce/relay/workspaces.json`. + * 5. nothing resolves — the caller decides (the broker mints a new workspace). + * + * The repository pin always outranks the machine-global active entry: a global + * selection must never silently re-home a checkout that pinned a workspace. A + * Fleet enrollment / node token selects the node's *identity*, never its + * workspace, so it does not appear on this ladder at all. */ -export function resolveWorkspaceKeyWithSource( +export function resolveWorkspaceSelection( options: ResolveWorkspaceKeyOptions = {} -): { key: string; source: WorkspaceKeySource } | undefined { +): WorkspaceSelection | undefined { const env = options.env ?? process.env; const flag = trimOrUndefined(options.workspaceKey); - if (flag) return { key: flag, source: 'flag' }; + if (flag) return { key: flag, source: 'flag', origin: '--workspace-key' }; - const envKey = - trimOrUndefined(env.RELAY_WORKSPACE_KEY) ?? - trimOrUndefined(env.AGENT_RELAY_WORKSPACE_KEY) ?? - trimOrUndefined(env.RELAY_API_KEY); - if (envKey) return { key: envKey, source: 'env' }; + for (const name of WORKSPACE_KEY_ENV_VARS) { + const envKey = trimOrUndefined(env[name]); + if (envKey) return { key: envKey, source: 'env', origin: `$${name}` }; + } const dataDir = options.projectDataDir ?? projectDataDir(options.projectRoot); - const project = dataDir ? readProjectWorkspaceKey(dataDir) : undefined; - if (project) return { key: project, source: 'project' }; + const project = dataDir ? readProjectWorkspaceSession(dataDir) : undefined; + if (project) { + return { + key: project.workspaceKey, + source: 'project', + origin: projectWorkspaceKeyPath(dataDir as string), + ...(project.workspaceId ? { workspaceId: project.workspaceId } : {}), + }; + } + + return resolveActiveWorkspaceSelection(env); +} + +/** + * Step 4 of {@link resolveWorkspaceSelection} on its own: the machine-global + * active workspace. + * + * Exposed separately for callers that inject their own file system for the + * higher (repository-pin) steps and must not re-read the pin through `node:fs`. + * It is never correct to consult this ahead of steps 1–3. + */ +export function resolveActiveWorkspaceSelection( + env: NodeJS.ProcessEnv = process.env +): WorkspaceSelection | undefined { + const store = readWorkspaceStore(env); + const activeName = trimOrUndefined(store.active); + const storeKey = activeName ? trimOrUndefined(store.workspaces[activeName]?.key) : undefined; + return storeKey + ? { + key: storeKey, + source: 'store', + origin: `${workspaceStorePath(env)} (active: "${activeName}")`, + } + : undefined; +} - const store = trimOrUndefined(resolveActiveWorkspaceKey(env)); - return store ? { key: store, source: 'store' } : undefined; +/** Resolve the selected workspace key and its source. See {@link resolveWorkspaceSelection}. */ +export function resolveWorkspaceKeyWithSource( + options: ResolveWorkspaceKeyOptions = {} +): { key: string; source: WorkspaceKeySource } | undefined { + const selection = resolveWorkspaceSelection(options); + return selection ? { key: selection.key, source: selection.source } : undefined; } /** Resolve only the selected workspace key while preserving the shared precedence rules. */ diff --git a/packages/cloud/src/workspace-key.ts b/packages/cloud/src/workspace-key.ts index a2a153cdf..d4b7e8138 100644 --- a/packages/cloud/src/workspace-key.ts +++ b/packages/cloud/src/workspace-key.ts @@ -2,10 +2,13 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, resolveWorkspaceKey, resolveWorkspaceKeyWithSource, + resolveWorkspaceSelection, writeProjectWorkspaceKey, type ProjectWorkspaceSession, type ResolveWorkspaceKeyOptions, type WorkspaceKeySource, + type WorkspaceSelection, } from './project-workspace-key.js'; diff --git a/packages/harness-driver/src/client.ts b/packages/harness-driver/src/client.ts index 7b3bb420c..71a867c96 100644 --- a/packages/harness-driver/src/client.ts +++ b/packages/harness-driver/src/client.ts @@ -225,6 +225,8 @@ export class HarnessDriverClient { private brokerExitListeners = new Set(); workspaceKey?: string; + /** Relay workspace id the broker joined, as reported on `/api/session`. */ + workspaceId?: string; /** Resolved broker URL — captured so call-site lifecycle contexts can surface it. */ readonly baseUrl: string; /** Shared multi-listener registry. Created bare when no `eventBus` is passed in. */ @@ -502,6 +504,7 @@ export class HarnessDriverClient { async getSession(): Promise { const session = await this.transport.request('/api/session'); this.workspaceKey = session.workspace_key; + this.workspaceId = session.default_workspace_id; return session; } From 23cf3925e538a8ba11ef2d3c45ddacb1513c6401 Mon Sep 17 00:00:00 2001 From: Proactive Runtime Bot Date: Mon, 3 Aug 2026 22:50:43 +0200 Subject: [PATCH 2/7] fix(cli): make workspace activation reversible --- CHANGELOG.md | 11 +- packages/cli/README.md | 58 ++-- .../src/cli/agent-relay-mcp.startup.test.ts | 5 + packages/cli/src/cli/bootstrap.test.ts | 2 + packages/cli/src/cli/commands/core.test.ts | 97 ++++++- packages/cli/src/cli/commands/node.test.ts | 5 + packages/cli/src/cli/commands/node.ts | 5 +- .../cli/src/cli/commands/workspace.test.ts | 124 ++++++++- packages/cli/src/cli/commands/workspace.ts | 54 ++++ .../cli/src/cli/lib/broker-lifecycle.test.ts | 27 +- packages/cli/src/cli/lib/broker-lifecycle.ts | 259 ++++++++++-------- .../cli/src/cli/lib/project-workspace-key.ts | 1 + .../cli/src/cli/lib/workspace-session.test.ts | 44 ++- packages/cli/src/cli/lib/workspace-session.ts | 23 ++ packages/cli/src/cli/telemetry/client.test.ts | 11 + packages/cli/src/cli/telemetry/client.ts | 12 +- packages/cloud/src/auth.test.ts | 1 + packages/cloud/src/index.ts | 1 + .../cloud/src/project-workspace-key.test.ts | 18 ++ packages/cloud/src/project-workspace-key.ts | 22 +- packages/cloud/src/workspace-key.ts | 1 + packages/cloud/src/workspace-store.test.ts | 14 + packages/cloud/src/workspace-store.ts | 12 +- 23 files changed, 660 insertions(+), 147 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeef27b06..98011c03d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `agent-relay cloud login --device` logs in a machine with no browser through the OAuth device flow: the CLI prints a code you approve from any other device. Login and re-authentication fall back to it automatically over SSH or on a Unix host with no display server, and each machine gets its own cloud session instead of a copied `cloud-auth.json`. Requires cloud with the device authorization endpoints. +- `agent-relay workspace restore` returns to the recorded previous workspace, while `workspace rebind ` explicitly pins a project's next broker start without changing the machine-global active workspace. + +### Changed + +- `workspace create` warns on stderr when it changes the active workspace and records the prior name; named switches now record the same restore point, and first-run telemetry notices no longer contaminate JSON stdout. ### Fixed @@ -17,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `agent-relay node up` warns instead of silently ignoring stored Cloud fleet enrollments when the project workspace pin has no enrolled node id. That combination started the broker in the pinned workspace while the node never heartbeat, leaving the Cloud dashboard and `agent-relay fleet nodes` showing different rosters with no error from either. - `agent-relay cloud enroll` records the enrolled node on the project workspace pin, so `node up` in that repo serves the node it just enrolled. A pin that already names a different node is reported and left untouched rather than repointed. - `agent-relay workspace switch|join` keeps the project's enrolled fleet node id instead of dropping it, which previously produced the pin state that made the next `node up` ignore the enrollment store. +- `agent-relay up` / `node up` use one precedence ladder: `--workspace-key` → workspace environment variables → repository pin → machine-global active workspace → creating one. A fresh project joins the active workspace instead of silently creating another, startup announces the winning source, and `node status` reports the same five-source provenance. +- Cloud enrollment selects node identity without overriding workspace resolution. A conflict with the repository pin stops startup, names both non-secret sources, and points to `workspace rebind ` as the recovery path. +- Detached `node up --background` surfaces early child failures and stops polling when the child exits without trying to kill an already dead process. ## [11.4.1] - 2026-08-03 @@ -27,9 +35,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. -- `agent-relay up` / `node up` resolve the workspace through one documented precedence ladder: `--workspace-key` → `RELAY_WORKSPACE_KEY`/`AGENT_RELAY_WORKSPACE_KEY`/`RELAY_API_KEY` → the repository pin in `.agentworkforce/relay/workspace-key.json` → the machine-global active workspace in `~/.agentworkforce/relay/workspaces.json` → creating one. Startup prints the winning source (flag, variable, or file path — never key material). -- A Cloud enrollment no longer re-homes an enrolled node out of its repository's workspace. `RELAY_NODE_TOKEN` selects the node's identity, not its workspace, and no longer suppresses the repository pin; when a stored enrollment addresses a different workspace than the pin, `node up` stops and names both sources instead of silently choosing one. -- A first `up` in a fresh directory joins the machine's active workspace instead of silently creating a new one, and a start that does create a workspace says so instead of printing the same output as a join. ## [11.4.0] - 2026-08-02 diff --git a/packages/cli/README.md b/packages/cli/README.md index f1e15864d..1db68a8e7 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -47,7 +47,7 @@ agent-relay node agent release For AI SDK native harnesses, attach renders structured activity, text, tools, approvals, files, usage, and lifecycle events. Add `--json` for NDJSON, `--reasoning` for reasoning events, or `--diagnostics` for sidecar diagnostics. Native harness `drive` is line-oriented and acknowledged; native harness `passthrough` is unsupported because no terminal stream exists. PTY attach behavior is unchanged. -### Which workspace a broker joins +### Workspace binding and recovery `agent-relay up` and `agent-relay node up` resolve the workspace through one precedence ladder. The first source that resolves wins: @@ -58,30 +58,56 @@ precedence ladder. The first source that resolves wins: | 2 | Environment | `RELAY_WORKSPACE_KEY`, then `AGENT_RELAY_WORKSPACE_KEY`, then `RELAY_API_KEY` | | 3 | Repository pin | `/.agentworkforce/relay/workspace-key.json` | | 4 | Machine-global active workspace | the `active` entry in `~/.agentworkforce/relay/workspaces.json` | -| 5 | New workspace | created only when nothing above resolves | +| 5 | Created workspace | created only when nothing above resolves | -Two rules follow from the order: +The repository pin always beats the machine-global active workspace, so +`agent-relay workspace switch ` never silently re-homes a checkout that +already pinned one. A new workspace is a last resort: a fresh directory joins +the machine-global active workspace when one exists, and startup explicitly +announces creation when none of the first four sources resolves. -- **The repository pin always beats the machine-global active workspace.** - Switching your active workspace (`agent-relay workspace use `) never - re-homes a checkout that already pinned one. -- **A new workspace is a last resort, not a default.** A fresh directory joins - the machine's active workspace when one is selected. When nothing resolves and - a workspace is created, startup says so explicitly. +Startup and `node status` report the winning source without printing key +material. Status uses the same five labels: command-line flag, environment, +repository pin, machine-global active workspace, or created. -Startup prints the winning source (a flag name, an environment variable, or a -file path — never key material): - -``` +```text Workspace source: repository pin (/repo/.agentworkforce/relay/workspace-key.json) Workspace: joined rw_7ccfea89 ``` A Cloud enrollment (`RELAY_NODE_TOKEN`, or a record in the Fleet enrollment store) selects the node's _identity_, not its workspace, so it never appears on -this ladder. If a stored enrollment addresses a different workspace than the -repository pin, `node up` refuses to start and names both source files rather -than silently choosing one. +the ladder. If a stored enrollment addresses a different workspace than the +repository pin, `node up` refuses to start and names both source files and +workspace IDs, never their keys. + +`workspace create`, `join`, and `switch` select a named workspace globally and +pin it to the current project. A changed selection records the old name, so an +accidental create can be undone: + +```bash +agent-relay workspace restore +``` + +To change only the workspace this project's broker will use on its next start, +without changing the machine-global active workspace, use: + +```bash +agent-relay workspace rebind default +agent-relay node down +agent-relay node up +``` + +`rebind` is also the supported recovery command for the conflict above: it +writes the repository pin (which outranks the machine-global active workspace) +and clears the project's stale enrolled-node association so the next start does +not fight the conflict guard. It does not stop a running broker; restart the +broker when you are ready to apply the new pin. + +For detached startup failures, `node up --background` reports the child error +when available and otherwise tells you to retry without `--background`; a child +that already exited is no longer misreported as an unkillable half-started +broker. ## Remote fleet agents 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 94f664185..eedf4393d 100644 --- a/packages/cli/src/cli/agent-relay-mcp.startup.test.ts +++ b/packages/cli/src/cli/agent-relay-mcp.startup.test.ts @@ -298,6 +298,11 @@ beforeEach(() => { vi.stubEnv('RELAYCAST_HARNESS', ''); vi.stubEnv('X_RELAYCAST_HARNESS', ''); vi.stubEnv('AGENT_RELAY_DISTINCT_ID', ''); + vi.stubEnv('AGENT_RELAY_MACHINE_ID', ''); + vi.stubEnv('AGENT_RELAY_USER_ID', ''); + vi.stubEnv('AGENT_RELAY_ORG_ID', ''); + vi.stubEnv('AGENT_RELAY_ORG_SLUG', ''); + vi.stubEnv('AGENT_RELAY_USER_EMAIL', ''); }); afterEach(() => { diff --git a/packages/cli/src/cli/bootstrap.test.ts b/packages/cli/src/cli/bootstrap.test.ts index cb4da75b7..f686a5b10 100644 --- a/packages/cli/src/cli/bootstrap.test.ts +++ b/packages/cli/src/cli/bootstrap.test.ts @@ -87,6 +87,8 @@ const expectedLeafCommands = [ 'workspace join', 'workspace key', 'workspace switch', + 'workspace restore', + 'workspace rebind', // workspace agents 'agent register', 'agent list', diff --git a/packages/cli/src/cli/commands/core.test.ts b/packages/cli/src/cli/commands/core.test.ts index e7bb634bb..cac21e274 100644 --- a/packages/cli/src/cli/commands/core.test.ts +++ b/packages/cli/src/cli/commands/core.test.ts @@ -2,7 +2,7 @@ import { Command } from 'commander'; import nodeFs from 'node:fs'; import os from 'node:os'; import nodePath from 'node:path'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { readProjectWorkspaceKey, readProjectWorkspaceSession } from '../lib/project-workspace-key.js'; @@ -34,6 +34,8 @@ const telemetryMocks = vi.hoisted(() => ({ track: vi.fn(), })); +const isolatedWorkspaceHome = nodeFs.mkdtempSync(nodePath.join(os.tmpdir(), 'relay-core-test-home-')); + vi.mock('../telemetry/index.js', () => ({ track: telemetryMocks.track, })); @@ -60,6 +62,10 @@ beforeEach(() => { telemetryMocks.track.mockClear(); }); +afterAll(() => { + nodeFs.rmSync(isolatedWorkspaceHome, { recursive: true, force: true }); +}); + import { registerCoreCommands, registerCoreMaintenance, @@ -76,12 +82,18 @@ class ExitSignal extends Error { } } -function connectionFile(pid: number, url = 'http://127.0.0.1:3889', apiKey = 'br_secret'): string { +function connectionFile( + pid: number, + url = 'http://127.0.0.1:3889', + apiKey = 'br_secret', + workspaceSource?: string +): string { return JSON.stringify({ url, port: Number(new URL(url).port || '0'), api_key: apiKey, pid, + ...(workspaceSource ? { workspace_source: workspaceSource } : {}), }); } @@ -156,6 +168,7 @@ function createHarness(options?: { const spawnedProcess = options?.spawnedProcess ?? createSpawnedProcessMock(); const env = options?.env ?? {}; env.AGENT_RELAY_DISABLE_IMPLICIT_FLEET_NODE ??= '1'; + env.AGENT_RELAY_HOME ??= isolatedWorkspaceHome; const exit = vi.fn((code: number) => { throw new ExitSignal(code); @@ -566,6 +579,7 @@ describe('registerCoreCommands', () => { ); expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_live_customflag77'); expect(deps.env.RELAY_API_KEY).toBe('rk_live_customflag77'); + expect(deps.env.AGENT_RELAY_WORKSPACE_SOURCE).toBe('flag'); expect(deps.env.AGENT_RELAY_STATE_DIR).toBe(stateDir); expect(deps.log).toHaveBeenCalledWith('Broker started.'); expect(deps.log).toHaveBeenCalledWith('Broker PID: 5151'); @@ -882,6 +896,44 @@ describe('registerCoreCommands', () => { expect(deps.log).not.toHaveBeenCalledWith('Broker started.'); }); + it('up --background reports an early detached-child failure without trying to kill a dead PID', async () => { + const spawnedProcess = createSpawnedProcessMock(); + let now = 0; + let childRunning = true; + const fs = createFsMock(); + const sleepImpl = vi.fn(async (ms: number) => { + now += ms; + childRunning = false; + fs.writeFileSync( + '/tmp/project/.agentworkforce/relay/background-start-error.log', + 'explicit workspace key was rejected' + ); + }); + const killImpl = vi.fn((pid: number, signal?: NodeJS.Signals | number) => { + if (pid === 9001 && signal === 0 && childRunning) return; + throw new Error('not running'); + }); + const { program, deps } = createHarness({ + fs, + spawnedProcess, + killImpl, + nowImpl: vi.fn(() => now), + sleepImpl, + }); + + const exitCode = await runCommand(program, ['up', '--background', '--workspace-key', 'rk_live_other']); + + expect(exitCode).toBe(1); + expect(deps.error).toHaveBeenCalledWith( + 'Broker background child exited before becoming ready (pid: 9001).' + ); + expect(deps.error).toHaveBeenCalledWith('Detached broker error: explicit workspace key was rejected'); + expect(killImpl).not.toHaveBeenCalledWith(9001, 'SIGTERM'); + expect(deps.error).not.toHaveBeenCalledWith( + expect.stringContaining('Failed to stop half-started broker process') + ); + }); + it('down --force only kills actual orphaned broker executables for the project', async () => { const runningPids = new Set([222, 444, 666]); const execCommand = vi.fn(async (command: string) => { @@ -1173,7 +1225,9 @@ describe('registerCoreCommands', () => { it('status checks broker status and prints metrics', async () => { const connectionPath = '/tmp/project/.agentworkforce/relay/connection.json'; - const fs = createFsMock({ [connectionPath]: connectionFile(4242) }); + const fs = createFsMock({ + [connectionPath]: connectionFile(4242, 'http://127.0.0.1:3889', 'br_secret', 'project'), + }); sdkStatusClient.getStatus.mockResolvedValueOnce({ agent_count: 4, pending_delivery_count: 2 }); sdkStatusClient.getSession.mockResolvedValueOnce({ workspace_key: 'rk_live_teststatus123', @@ -1191,11 +1245,48 @@ describe('registerCoreCommands', () => { expect(deps.log).toHaveBeenCalledWith('Pending deliveries: 2'); expect(deps.log).toHaveBeenCalledWith('Node: sf-mini (node_enrolled)'); expect(deps.log).toHaveBeenCalledWith('Workspace Key: rk_live_…s123'); + expect(deps.log).toHaveBeenCalledWith( + 'Workspace source: repository pin (.agentworkforce/relay/workspace-key.json)' + ); const logCalls = (deps.log as unknown as { mock: { calls: unknown[][] } }).mock.calls; expect(logCalls.some((call) => String(call[0]).startsWith('Observer:'))).toBe(false); expect(sdkStatusClient.disconnect).toHaveBeenCalled(); }); + it.each([ + { + source: 'flag', + label: 'command-line flag (--workspace-key / --wk)', + }, + { + source: 'env', + label: 'environment (RELAY_WORKSPACE_KEY > AGENT_RELAY_WORKSPACE_KEY > RELAY_API_KEY)', + }, + { + source: 'project', + label: 'repository pin (.agentworkforce/relay/workspace-key.json)', + }, + { + source: 'store', + label: 'machine-global active workspace (~/.agentworkforce/relay/workspaces.json)', + }, + { + source: 'created', + label: 'created (no configured workspace resolved)', + }, + ])('status reports the $source workspace source', async ({ source, label }) => { + const connectionPath = '/tmp/project/.agentworkforce/relay/connection.json'; + const fs = createFsMock({ + [connectionPath]: connectionFile(4242, 'http://127.0.0.1:3889', 'br_secret', source), + }); + const { program, deps } = createHarness({ fs }); + + const exitCode = await runCommand(program, ['status']); + + expect(exitCode).toBeUndefined(); + expect(deps.log).toHaveBeenCalledWith(`Workspace source: ${label}`); + }); + it('status omits workspace key and observer when broker has no workspace_key', async () => { const connectionPath = '/tmp/project/.agentworkforce/relay/connection.json'; const fs = createFsMock({ [connectionPath]: connectionFile(4242) }); diff --git a/packages/cli/src/cli/commands/node.test.ts b/packages/cli/src/cli/commands/node.test.ts index 6d7779d1f..2d9b2fb34 100644 --- a/packages/cli/src/cli/commands/node.test.ts +++ b/packages/cli/src/cli/commands/node.test.ts @@ -9,6 +9,7 @@ const brokerMocks = vi.hoisted(() => ({ })); vi.mock('../lib/broker-lifecycle.js', () => ({ + WORKSPACE_BINDING_SOURCE_ENV: 'AGENT_RELAY_WORKSPACE_SOURCE', runUpCommand: (...args: unknown[]) => brokerMocks.runUpCommand(...args), runDownCommand: (...args: unknown[]) => brokerMocks.runDownCommand(...args), runStatusCommand: (...args: unknown[]) => brokerMocks.runStatusCommand(...args), @@ -373,6 +374,7 @@ describe('registerNodeCommands', () => { expect(message).toContain('rw_stale'); expect(message).toContain('rw_123'); expect(message).toContain('workspace-key.json'); + expect(message).toContain('agent-relay workspace rebind '); // Diagnostics name sources, never credentials. expect(message).not.toContain('rk_project_session'); expect(message).not.toContain('nt_secret'); @@ -434,7 +436,10 @@ describe('registerNodeCommands', () => { RELAY_NODE_ID: 'node_abc', RELAY_NODE_TOKEN: 'nt_secret', }); + // Node startup resolves identity only. The shared runUpCommand resolver + // applies the repository workspace, so there is no second ladder here. expect(restart.env.RELAY_WORKSPACE_KEY).toBeUndefined(); + expect(restart.env.RELAY_API_KEY).toBeUndefined(); expect(brokerMocks.runUpCommand).toHaveBeenLastCalledWith( expect.objectContaining({ background: true, diff --git a/packages/cli/src/cli/commands/node.ts b/packages/cli/src/cli/commands/node.ts index 94102133d..0fa000832 100644 --- a/packages/cli/src/cli/commands/node.ts +++ b/packages/cli/src/cli/commands/node.ts @@ -130,8 +130,9 @@ function reportWorkspaceSourceConflict( ` fleet enrollment ${fleetNodeEnrollmentStorePath(deps.core.env)} -> workspace ${enrolledWorkspaceId} (node ${record.nodeId})` ); deps.error( - 'Pass --workspace-key to choose explicitly, re-enroll this node in the pinned workspace, ' + - 'or delete the repository pin to adopt the enrollment.' + 'Run `agent-relay workspace rebind ` to repin this project and clear the stale ' + + 'enrolled-node association; alternatively pass --workspace-key or re-enroll this node in ' + + 'the pinned workspace.' ); return true; } diff --git a/packages/cli/src/cli/commands/workspace.test.ts b/packages/cli/src/cli/commands/workspace.test.ts index e57c5e302..e254c9dbe 100644 --- a/packages/cli/src/cli/commands/workspace.test.ts +++ b/packages/cli/src/cli/commands/workspace.test.ts @@ -11,6 +11,7 @@ vi.mock('@agent-relay/cloud', () => ({ vi.mock('../lib/workspace-session.js', async (importOriginal) => ({ // Returns a result object describing what the write changed beyond the key. persistWorkspaceSession: vi.fn(() => ({})), + pinProjectWorkspaceSession: vi.fn(), // The real formatter, not a copy: these tests assert on its wording, so a // stand-in here would let the command output drift past them. describeClearedEnrollment: (await importOriginal()) @@ -30,7 +31,11 @@ import { } from '@agent-relay/cloud'; import { registerWorkspaceCommands, type WorkspaceCommandDependencies } from './workspace.js'; -import { persistWorkspaceSession, validateWorkspaceSessionName } from '../lib/workspace-session.js'; +import { + persistWorkspaceSession, + pinProjectWorkspaceSession, + validateWorkspaceSessionName, +} from '../lib/workspace-session.js'; beforeEach(() => { vi.clearAllMocks(); @@ -165,6 +170,46 @@ describe('registerWorkspaceCommands', () => { }); }); + it('workspace create records and visibly warns about the previous active workspace on stderr', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'default', + workspaces: { default: { key: 'rk_live_default' } }, + }); + const { program, deps } = createHarness(); + vi.mocked(deps.createWorkspace).mockResolvedValueOnce({ + workspaceKey: 'rk_live_session_two', + } as never); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'create', 'session-two']); + + expect(persistWorkspaceSession).toHaveBeenCalledWith({ + name: 'session-two', + workspaceKey: 'rk_live_session_two', + }); + expect(deps.error).toHaveBeenNthCalledWith(1, '⚠ Active workspace changed: default → session-two'); + expect(deps.error).toHaveBeenNthCalledWith(2, ' Restore with: agent-relay workspace restore'); + expect(() => JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0]))).not.toThrow(); + }); + + it('workspace create --json keeps stdout parseable and routes the warning away from it', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'default', + workspaces: { default: { key: 'rk_live_default' } }, + }); + const { program, deps } = createHarness(); + vi.mocked(deps.createWorkspace).mockResolvedValueOnce({ + workspaceKey: 'rk_live_json_workspace', + } as never); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'create', 'json-workspace', '--json']); + + expect(vi.mocked(deps.log).mock.calls).toHaveLength(1); + expect(JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0]))).toMatchObject({ + name: 'json-workspace', + }); + expect(deps.error).toHaveBeenCalledWith('⚠ Active workspace changed: default → json-workspace'); + }); + it('workspace create rejects a blank name before provisioning a remote workspace', async () => { const { program, deps } = createHarness(); @@ -284,4 +329,81 @@ describe('registerWorkspaceCommands', () => { workspaceKey: 'rk_live_shared', }); }); + + it('workspace restore switches back to the recorded previous workspace', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'scratch', + previous: 'default', + workspaces: { + default: { key: 'rk_live_default' }, + scratch: { key: 'rk_live_scratch' }, + }, + }); + const { program, deps } = createHarness(); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'restore']); + + expect(persistWorkspaceSession).toHaveBeenCalledWith({ + name: 'default', + workspaceKey: 'rk_live_default', + }); + expect(deps.log).toHaveBeenCalledWith('Switched to workspace "default" (was scratch).'); + }); + + it('workspace restore reports when nothing was recorded', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ active: 'default', workspaces: {} }); + const { program, deps } = createHarness(); + + await expect(program.parseAsync(['node', 'agent-relay', 'workspace', 'restore'])).rejects.toThrow( + 'exit:1' + ); + + expect(deps.error).toHaveBeenCalledWith('No previous workspace is recorded.'); + }); + + it('workspace restore reports when the recorded workspace no longer exists', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'scratch', + previous: 'deleted', + workspaces: { scratch: { key: 'rk_live_scratch' } }, + }); + const { program, deps } = createHarness(); + + await expect(program.parseAsync(['node', 'agent-relay', 'workspace', 'restore'])).rejects.toThrow( + 'exit:1' + ); + + expect(deps.error).toHaveBeenCalledWith('The recorded previous workspace "deleted" no longer exists.'); + }); + + it('workspace restore reports when the recorded workspace is already active', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'default', + previous: 'default', + workspaces: { default: { key: 'rk_live_default' } }, + }); + const { program, deps } = createHarness(); + + await expect(program.parseAsync(['node', 'agent-relay', 'workspace', 'restore'])).rejects.toThrow( + 'exit:1' + ); + + expect(deps.error).toHaveBeenCalledWith('The recorded previous workspace "default" is already active.'); + }); + + it('workspace rebind pins the selected workspace to this project without changing global state', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'scratch', + workspaces: { default: { key: 'rk_live_default' } }, + }); + const { program, deps } = createHarness(); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'rebind', 'default']); + + expect(pinProjectWorkspaceSession).toHaveBeenCalledWith({ workspaceKey: 'rk_live_default' }); + expect(persistWorkspaceSession).not.toHaveBeenCalled(); + expect(deps.log).toHaveBeenCalledWith( + `Rebound this project's broker to workspace "default". Restart the broker to apply it.` + ); + }); }); diff --git a/packages/cli/src/cli/commands/workspace.ts b/packages/cli/src/cli/commands/workspace.ts index e727c00f2..51f34bb87 100644 --- a/packages/cli/src/cli/commands/workspace.ts +++ b/packages/cli/src/cli/commands/workspace.ts @@ -8,6 +8,7 @@ import { readWorkspaceStore, setWorkspaceKey } from '../lib/workspace-store.js'; import { describeClearedEnrollment, persistWorkspaceSession, + pinProjectWorkspaceSession, validateWorkspaceSessionName, type PersistWorkspaceSessionResult, } from '../lib/workspace-session.js'; @@ -97,14 +98,20 @@ export function registerWorkspaceCommands( .description('Create a new workspace and store its key') .argument('', 'Workspace name') .option('--base-url ', 'Override the API base URL') + .option('--json', 'Output the created workspace as JSON') .option('--reveal-secrets', 'Include the raw workspace key in the output') .action(async (name: string, o: Record) => { await runSdk(deps, async () => { const workspaceName = validateWorkspaceSessionName(name); const relay = await deps.createWorkspace(workspaceName, o.baseUrl as string | undefined); + const previousActive = readWorkspaceStore().active; const persisted = relay.workspaceKey ? persistWorkspaceSession({ name: workspaceName, workspaceKey: relay.workspaceKey }) : {}; + if (relay.workspaceKey && previousActive && previousActive !== workspaceName) { + deps.error(`⚠ Active workspace changed: ${previousActive} → ${workspaceName}`); + deps.error(' Restore with: agent-relay workspace restore'); + } // The key is persisted to the workspace store either way; the output // masks it unless the caller explicitly asks for the raw value. A // dropped enrollment rides in the JSON rather than a log line so the @@ -131,6 +138,7 @@ export function registerWorkspaceCommands( const store = readWorkspaceStore(); printJson(deps, { active: store.active, + previous: store.previous, workspaces: Object.keys(store.workspaces), }); }); @@ -199,4 +207,50 @@ export function registerWorkspaceCommands( reportClearedEnrollment(result, deps); }); }); + + group + .command('restore') + .description('Switch back to the previously active workspace') + .action(async () => { + await runSdk(deps, async () => { + const store = readWorkspaceStore(); + const previous = store.previous; + if (!previous) { + throw new Error('No previous workspace is recorded.'); + } + if (previous === store.active) { + throw new Error(`The recorded previous workspace "${previous}" is already active.`); + } + const workspace = Object.hasOwn(store.workspaces, previous) ? store.workspaces[previous] : undefined; + if (!workspace) { + throw new Error(`The recorded previous workspace "${previous}" no longer exists.`); + } + const current = store.active; + persistWorkspaceSession({ name: previous, workspaceKey: workspace.key }); + deps.log(`Switched to workspace "${previous}" (was ${current ?? 'none'}).`); + }); + }); + + group + .command('rebind') + .description("Pin this project's broker to a stored workspace") + .argument('', 'Stored workspace name') + .action(async (name: string) => { + await runSdk(deps, async () => { + const workspaceName = validateWorkspaceSessionName(name); + const store = readWorkspaceStore(); + const workspace = Object.hasOwn(store.workspaces, workspaceName) + ? store.workspaces[workspaceName] + : undefined; + if (!workspace) { + throw new Error( + `Unknown workspace "${workspaceName}". Add it with \`relay workspace set_key ${workspaceName} \`.` + ); + } + pinProjectWorkspaceSession({ workspaceKey: workspace.key }); + deps.log( + `Rebound this project's broker to workspace "${workspaceName}". Restart the broker to apply it.` + ); + }); + }); } diff --git a/packages/cli/src/cli/lib/broker-lifecycle.test.ts b/packages/cli/src/cli/lib/broker-lifecycle.test.ts index fceb00920..3c59541d0 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.test.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.test.ts @@ -63,6 +63,14 @@ describe('describeErrorWithCause', () => { expect(result).toContain('agentrelay.com'); }); + it('redacts credentials found only in a nested cause', () => { + const err = new Error('broker start failed', { + cause: new Error('workspace rk_live_0123456789abcdef was rejected'), + }); + + expect(describeErrorWithCause(err)).toBe('broker start failed — workspace rk_live_…cdef was rejected'); + }); + it('handles non-Error values without throwing', () => { expect(describeErrorWithCause('something went wrong')).toBe('something went wrong'); expect(describeErrorWithCause(undefined)).toBe('undefined'); @@ -497,6 +505,8 @@ describe('runUpCommand node-config gating', () => { describe('runUpCommand workspace precedence', () => { const readPin = (dataDir: string): Record => JSON.parse(fsReal.readFileSync(pathReal.join(dataDir, 'workspace-key.json'), 'utf-8')); + const readBindingSource = (dataDir: string): string => + JSON.parse(fsReal.readFileSync(pathReal.join(dataDir, 'connection.json'), 'utf-8')).workspace_source; it('prefers the repository pin over the machine-global active workspace (#1406)', async () => { const { deps, dataDir, home, log } = createUpHarness(); @@ -508,6 +518,7 @@ describe('runUpCommand workspace precedence', () => { expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_repository'); expect(deps.env.RELAY_API_KEY).toBe('rk_repository'); expect(log.mock.calls.flat().join('\n')).toContain('Workspace source: repository pin'); + expect(readBindingSource(dataDir)).toBe('project'); }); it('applies the repository pin even when an enrollment node token is present (#1406)', async () => { @@ -534,10 +545,11 @@ describe('runUpCommand workspace precedence', () => { expect(output).toContain('Workspace source: machine-global active workspace'); expect(output).toContain('active: "account"'); expect(output).not.toContain('created new workspace'); + expect(readBindingSource(deps.getProjectPaths().dataDir)).toBe('store'); }); it('announces a mint when no source resolves (#1378)', async () => { - const { deps, log } = createUpHarness(); + const { deps, dataDir, log } = createUpHarness(); await runUpCommand({}, deps); @@ -545,6 +557,7 @@ describe('runUpCommand workspace precedence', () => { const output = log.mock.calls.flat().join('\n'); expect(output).toContain('Workspace: none selected'); expect(output).toContain('Workspace: created new workspace rw_test'); + expect(readBindingSource(dataDir)).toBe('created'); }); it('keeps an explicit --workspace-key ahead of both stores', async () => { @@ -556,6 +569,18 @@ describe('runUpCommand workspace precedence', () => { expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_flag'); expect(log.mock.calls.flat().join('\n')).toContain('Workspace source: command-line flag'); + expect(readBindingSource(dataDir)).toBe('flag'); + }); + + it('records environment provenance after normalizing a workspace-key alias', async () => { + const { deps, dataDir, log } = createUpHarness(); + deps.env.AGENT_RELAY_WORKSPACE_KEY = ' rk_environment '; + + await runUpCommand({}, deps); + + expect(deps.env.RELAY_WORKSPACE_KEY).toBe('rk_environment'); + expect(log.mock.calls.flat().join('\n')).toContain('$AGENT_RELAY_WORKSPACE_KEY'); + expect(readBindingSource(dataDir)).toBe('env'); }); it('records the resolved workspace id on the pin for later conflict detection', async () => { diff --git a/packages/cli/src/cli/lib/broker-lifecycle.ts b/packages/cli/src/cli/lib/broker-lifecycle.ts index 3be312a83..5417c9d29 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.ts @@ -3,6 +3,7 @@ import path from 'node:path'; import { HarnessDriverClient } from '@agent-relay/harness-driver'; import { startServeNode, type FleetNodeDefinition, type RunningNode } from '@agent-relay/fleet'; import { createLogger } from '@agent-relay/utils'; +import { redactCredentialValues } from '@agent-relay/cloud/redact'; import type { CoreDependencies, CoreProjectPaths, CoreRelay, SpawnedProcess } from '../commands/core.js'; import { track } from '../telemetry/index.js'; @@ -25,12 +26,12 @@ import { describeError } from './describe-error.js'; import { maskSecret } from './redact.js'; import { startReflexCapture, type RunningReflexCapture } from './reflex-capture.js'; import { - projectWorkspaceKeyPath, - resolveActiveWorkspaceSelection, + readProjectWorkspaceSession, + resolveWorkspaceSelection, writeProjectWorkspaceKey, + type ProjectWorkspaceSession, type WorkspaceSelection, } from './project-workspace-key.js'; -import { promoteWorkspaceKeyEnvAlias } from './workspace-env.js'; type UpOptions = { spawn?: boolean; @@ -70,6 +71,8 @@ const DEFAULT_BROKER_BASE_PORT = 3888; /** The broker writes this file with URL, port, API key, and PID. */ const CONNECTION_FILENAME = 'connection.json'; +const BACKGROUND_START_ERROR_FILENAME = 'background-start-error.log'; +export const WORKSPACE_BINDING_SOURCE_ENV = 'AGENT_RELAY_WORKSPACE_SOURCE'; const STATUS_POLL_INTERVAL_MS = 500; const DETACHED_START_READY_TIMEOUT_MS = 10_000; const NODE_DELIVERY_READY_TIMEOUT_MS = 10_000; @@ -78,11 +81,15 @@ const NODE_DELIVERY_READY_TIMEOUT_MS = 10_000; // RELAY_NODE_TOKEN. const NODE_TOKEN_WAIT_MS = 15_000; +export type WorkspaceBindingSource = WorkspaceSelection['source'] | 'created'; + export interface BrokerConnection { url: string; port: number; api_key: string; pid: number; + /** Non-secret provenance recorded by the CLI after the broker handshake. */ + workspace_source?: WorkspaceBindingSource; } type BrokerStatusDetails = { @@ -296,7 +303,7 @@ export function describeErrorWithCause(err: unknown): string { const parts = [top]; if (detail && detail !== top) parts.push(detail); if (codes.length > 0) parts.push(`[${codes.join(', ')}]`); - return parts.join(' — '); + return redactCredentialValues(parts.join(' — ')); } /** @@ -718,6 +725,68 @@ function safeUnlink(filePath: string, deps: CoreDependencies): void { } } +function workspaceBindingSource(value: string | undefined): WorkspaceBindingSource | undefined { + return value === 'flag' || + value === 'env' || + value === 'project' || + value === 'store' || + value === 'created' + ? value + : undefined; +} + +function workspaceBindingSourceLabel(source: WorkspaceBindingSource): string { + switch (source) { + case 'flag': + return 'command-line flag (--workspace-key / --wk)'; + case 'env': + return 'environment (RELAY_WORKSPACE_KEY > AGENT_RELAY_WORKSPACE_KEY > RELAY_API_KEY)'; + case 'project': + return 'repository pin (.agentworkforce/relay/workspace-key.json)'; + case 'store': + return 'machine-global active workspace (~/.agentworkforce/relay/workspaces.json)'; + case 'created': + return 'created (no configured workspace resolved)'; + } +} + +function writeBrokerBindingSource( + dataDir: string, + source: WorkspaceBindingSource, + deps: CoreDependencies +): void { + const connectionPath = path.join(dataDir, CONNECTION_FILENAME); + const connection = readBrokerConnectionFromFs(deps.fs, dataDir); + if (!connection) return; + deps.fs.writeFileSync( + connectionPath, + `${JSON.stringify({ ...connection, workspace_source: source }, null, 2)}\n`, + 'utf-8' + ); +} + +function backgroundStartErrorPath(dataDir: string): string { + return path.join(dataDir, BACKGROUND_START_ERROR_FILENAME); +} + +function readBackgroundStartError(dataDir: string, deps: CoreDependencies): string | undefined { + try { + return deps.fs.readFileSync(backgroundStartErrorPath(dataDir), 'utf-8').trim() || undefined; + } catch { + return undefined; + } +} + +function recordBackgroundStartError(message: string, deps: CoreDependencies): void { + const file = deps.env.AGENT_RELAY_BACKGROUND_START_ERROR_FILE?.trim(); + if (!file) return; + try { + deps.fs.writeFileSync(file, `${message}\n`, 'utf-8'); + } catch { + // Diagnostics must never replace the original startup error. + } +} + function readBrokerPid(dataDir: string, _deps: CoreDependencies): number | null { const conn = readBrokerConnectionFromFs(_deps.fs, dataDir); return conn?.pid ?? null; @@ -964,6 +1033,7 @@ function cleanupBrokerFiles(paths: CoreProjectPaths, deps: CoreDependencies): vo safeUnlink(path.join(paths.dataDir, CONNECTION_FILENAME), deps); safeUnlink(relaySockPath, deps); safeUnlink(runtimePath, deps); + safeUnlink(backgroundStartErrorPath(paths.dataDir), deps); // Clean up lock files and legacy pid files try { @@ -1114,13 +1184,17 @@ async function waitForBrokerReadiness( deps: CoreDependencies, waitMs: number, requireApi: boolean, - verbose?: boolean + verbose?: boolean, + stopWhenPidExits?: number ): Promise { const deadline = deps.now() + waitMs; let latest = await checkBrokerReadiness(paths, deps, requireApi); vlog(deps, verbose, `Broker readiness: ${latest.state}`); while (latest.state !== 'running' && waitMs > 0 && deps.now() < deadline) { + if (stopWhenPidExits && !isProcessRunning(stopWhenPidExits, deps)) { + return latest; + } await deps.sleep(Math.min(STATUS_POLL_INTERVAL_MS, Math.max(0, deadline - deps.now()))); const previousState = latest.state; latest = await checkBrokerReadiness(paths, deps, requireApi); @@ -1292,89 +1366,6 @@ function planCapacitySource( return plan.mode === 'in-process' ? plan.definition : descriptorCapacitySource(plan.descriptor); } -interface PinnedProjectWorkspaceSession { - workspaceKey: string; - enrolledNodeId?: string; - workspaceId?: string; -} - -/** Read the minimal project session needed during broker startup. */ -function readPinnedProjectWorkspaceSession( - dataDir: string, - deps: CoreDependencies -): PinnedProjectWorkspaceSession | undefined { - try { - const parsed = JSON.parse(deps.fs.readFileSync(projectWorkspaceKeyPath(dataDir), 'utf8')) as Partial<{ - workspaceKey: string; - enrolledNodeId: string; - workspaceId: string; - }>; - const workspaceKey = trimmedOrUndefined(parsed.workspaceKey); - if (!workspaceKey) { - return undefined; - } - const enrolledNodeId = trimmedOrUndefined(parsed.enrolledNodeId); - const workspaceId = trimmedOrUndefined(parsed.workspaceId); - return { - workspaceKey, - ...(enrolledNodeId ? { enrolledNodeId } : {}), - ...(workspaceId ? { workspaceId } : {}), - }; - } catch { - return undefined; - } -} - -/** Narrow an unknown JSON field to a non-blank string. */ -function trimmedOrUndefined(value: unknown): string | undefined { - return typeof value === 'string' ? value.trim() || undefined : undefined; -} - -/** - * Resolve the workspace this broker start joins, walking the shared precedence - * ladder: `--workspace-key` → env → the repository pin → the machine-global - * active workspace. Nothing resolving means the broker will mint a workspace. - * - * The repository pin is read through {@link CoreDependencies.fs} (tests stub it) - * while the machine-global store is read by the shared cloud resolver, so both - * halves of the ladder stay in one place. - * - * A Fleet enrollment (`RELAY_NODE_TOKEN`) selects the node's identity, not its - * workspace, and no longer short-circuits this walk — letting it do so is what - * re-homed an enrolled node out of its repository's workspace and into a - * freshly minted one. - */ -function resolveWorkspaceForBrokerStart( - options: UpOptions, - deps: CoreDependencies, - projectDataDir: string -): WorkspaceSelection | undefined { - const flag = options.workspaceKey?.trim(); - if (flag) { - return { key: flag, source: 'flag', origin: '--workspace-key' }; - } - - const explicitEnvWorkspaceKey = promoteWorkspaceKeyEnvAlias(deps.env); - if (explicitEnvWorkspaceKey) { - return { key: explicitEnvWorkspaceKey, source: 'env', origin: '$RELAY_WORKSPACE_KEY' }; - } - - const pinned = readPinnedProjectWorkspaceSession(projectDataDir, deps); - if (pinned) { - return { - key: pinned.workspaceKey, - source: 'project', - origin: projectWorkspaceKeyPath(projectDataDir), - ...(pinned.workspaceId ? { workspaceId: pinned.workspaceId } : {}), - }; - } - - // Everything below the repository pin: the machine-global active workspace. - // Without this step a fresh checkout mints its own workspace even though the - // machine already has an active one selected. - return resolveActiveWorkspaceSelection(deps.env); -} - /** * Apply the resolved workspace to the environment the broker (and any detached * child) inherits, and report which source won. Returns the pinned project @@ -1384,7 +1375,7 @@ function applyWorkspaceSelection( selection: WorkspaceSelection | undefined, deps: CoreDependencies, projectDataDir: string -): PinnedProjectWorkspaceSession | undefined { +): ProjectWorkspaceSession | undefined { if (!selection) { deps.log( 'Workspace: none selected (no --workspace-key, no RELAY_WORKSPACE_KEY, no repository pin, ' + @@ -1394,20 +1385,18 @@ function applyWorkspaceSelection( } deps.log(`Workspace source: ${describeWorkspaceSource(selection.source)} (${selection.origin})`); - if (selection.source === 'flag' || selection.source === 'env') { - // Both already live in the environment the broker inherits: the flag is - // exported by runUpCommand before the --background fork, and an env alias - // was promoted to RELAY_WORKSPACE_KEY during resolution. Writing - // RELAY_API_KEY here would clobber a value the caller set deliberately. - return undefined; - } + // Normalize every winning source to the primary env var inherited by the + // broker and any detached child. Keep a caller-supplied RELAY_API_KEY intact + // when an explicit flag or environment variable won. deps.env.RELAY_WORKSPACE_KEY = selection.key; - deps.env.RELAY_API_KEY = selection.key; + if (selection.source === 'project' || selection.source === 'store') { + deps.env.RELAY_API_KEY = selection.key; + } if (selection.source !== 'project') { return undefined; } - const pinned = readPinnedProjectWorkspaceSession(projectDataDir, deps); + const pinned = readProjectWorkspaceSession(projectDataDir, deps.fs); if (pinned?.enrolledNodeId) { deps.env.AGENT_RELAY_ENROLLED_NODE_ID = pinned.enrolledNodeId; } @@ -1428,6 +1417,22 @@ function describeWorkspaceSource(source: WorkspaceSelection['source']): string { } } +/** + * Preserve the original source across `--background` re-exec. The detached + * child sees the normalized RELAY_WORKSPACE_KEY as an env selection, so this + * marker carries only provenance; it never participates in resolution. + */ +function recordWorkspaceBindingSource( + selection: WorkspaceSelection | undefined, + deps: CoreDependencies +): WorkspaceBindingSource { + const inheritedSource = workspaceBindingSource(deps.env[WORKSPACE_BINDING_SOURCE_ENV]); + const source: WorkspaceBindingSource = + selection?.source === 'env' && inheritedSource ? inheritedSource : (selection?.source ?? 'created'); + deps.env[WORKSPACE_BINDING_SOURCE_ENV] = source; + return source; +} + export async function runUpCommand(options: UpOptions, deps: CoreDependencies): Promise { ensureBundledAgentRelayMcpCommand(deps); @@ -1438,7 +1443,13 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): // --state-dir), so the key must be persisted here even when broker state is // redirected elsewhere. const projectWorkspaceKeyDataDir = paths.dataDir; - const workspaceSelection = resolveWorkspaceForBrokerStart(options, deps, projectWorkspaceKeyDataDir); + const workspaceSelection = resolveWorkspaceSelection({ + workspaceKey: options.workspaceKey, + env: deps.env, + projectDataDir: projectWorkspaceKeyDataDir, + fileSystem: deps.fs, + }); + const workspaceBindingSource = recordWorkspaceBindingSource(workspaceSelection, deps); const resumedProjectSession = applyWorkspaceSelection(workspaceSelection, deps, projectWorkspaceKeyDataDir); // --state-dir overrides where the broker writes state / connection files if (options.stateDir) { @@ -1475,6 +1486,9 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): return; } + const startErrorPath = backgroundStartErrorPath(paths.dataDir); + safeUnlink(startErrorPath, deps); + deps.env.AGENT_RELAY_BACKGROUND_START_ERROR_FILE = startErrorPath; const args = childUpArgsForDetachedStart(options, deps); const invocation = detachedCliInvocation(deps, args); let child: SpawnedProcess; @@ -1500,23 +1514,36 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): deps, DETACHED_START_READY_TIMEOUT_MS, true, - options.verbose + options.verbose, + child.pid ); if (readiness.state !== 'running') { const pid = readiness.state === 'starting' ? readiness.conn.pid : child.pid; - deps.error( - pid - ? `Broker background start did not become ready within ${DETACHED_START_READY_TIMEOUT_MS / 1000}s (pid: ${pid}).` - : `Broker background start did not become ready within ${DETACHED_START_READY_TIMEOUT_MS / 1000}s.` - ); + const childExited = + typeof child.pid === 'number' && child.pid > 0 && !isProcessRunning(child.pid, deps); + if (childExited) { + deps.error(`Broker background child exited before becoming ready (pid: ${child.pid}).`); + } else { + deps.error( + pid + ? `Broker background start did not become ready within ${DETACHED_START_READY_TIMEOUT_MS / 1000}s (pid: ${pid}).` + : `Broker background start did not become ready within ${DETACHED_START_READY_TIMEOUT_MS / 1000}s.` + ); + } if (readiness.state === 'starting') { deps.error('Broker process is running, but the API did not become ready.'); } + const detachedError = readBackgroundStartError(paths.dataDir, deps); + if (detachedError) { + deps.error(`Detached broker error: ${detachedError}`); + } else if (childExited) { + deps.error('Retry without --background to see the broker startup error.'); + } deps.error( 'Run `agent-relay status --wait-for=10` for details, or `agent-relay down --force` to clean up.' ); const cleanupPids = new Set(); - if (typeof child.pid === 'number' && child.pid > 0) { + if (typeof child.pid === 'number' && child.pid > 0 && isProcessRunning(child.pid, deps)) { cleanupPids.add(child.pid); } if (readiness.state === 'starting') { @@ -1584,6 +1611,7 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): deps.log('Broker started.'); deps.log(`Broker PID: ${readiness.conn.pid}`); deps.log('Stop with: agent-relay down'); + safeUnlink(startErrorPath, deps); deps.exit(0); return; } @@ -1658,6 +1686,13 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): ); relay = started.relay; + try { + writeBrokerBindingSource(paths.dataDir, workspaceBindingSource, deps); + } catch { + // Provenance is diagnostic metadata; a broker that came up stays up. + } + safeUnlink(backgroundStartErrorPath(paths.dataDir), deps); + deps.log(`Relay API: http://localhost:${started.apiPort}`); deps.log(`Project: ${paths.projectRoot}`); deps.log('Mode: broker (stdio)'); @@ -1671,7 +1706,7 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): deps.log(`Workspace: created new workspace ${joinedWorkspaceId}`); deps.log( 'Pin a workspace for this repository with `agent-relay up --workspace-key `, ' + - 'or select one machine-wide with `agent-relay workspace use `.' + 'or select one machine-wide with `agent-relay workspace switch `.' ); } deps.log('Broker started.'); @@ -1772,10 +1807,12 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): stage, error_class: classifyBrokerStartError(err), }); + const detailedMessage = describeErrorWithCause(err); + recordBackgroundStartError(detailedMessage, deps); if (isBrokerAlreadyRunningError(message)) { reportAlreadyRunningError(message, paths.dataDir, deps); } else { - deps.error(`Failed to start broker: ${describeErrorWithCause(err)}`); + deps.error(`Failed to start broker: ${detailedMessage}`); } deps.exit(1); } @@ -1934,6 +1971,12 @@ export async function runStatusCommand( deps.log('Mode: broker (stdio)'); deps.log(`PID: ${readiness.conn.pid}`); deps.log(`Project: ${paths.projectRoot}`); + const source = workspaceBindingSource(readiness.conn.workspace_source); + deps.log( + source + ? `Workspace source: ${workspaceBindingSourceLabel(source)}` + : 'Workspace source: unknown (startup provenance was not recorded)' + ); // Query the running broker for additional status info const statusDetails = diff --git a/packages/cli/src/cli/lib/project-workspace-key.ts b/packages/cli/src/cli/lib/project-workspace-key.ts index 294542ab7..5d97af820 100644 --- a/packages/cli/src/cli/lib/project-workspace-key.ts +++ b/packages/cli/src/cli/lib/project-workspace-key.ts @@ -8,5 +8,6 @@ export { resolveWorkspaceSelection, writeProjectWorkspaceKey, type ProjectWorkspaceSession, + type WorkspaceKeyFileSystem, type WorkspaceSelection, } from '@agent-relay/cloud/workspace-key'; diff --git a/packages/cli/src/cli/lib/workspace-session.test.ts b/packages/cli/src/cli/lib/workspace-session.test.ts index 26e2045c2..615e1fafe 100644 --- a/packages/cli/src/cli/lib/workspace-session.test.ts +++ b/packages/cli/src/cli/lib/workspace-session.test.ts @@ -5,13 +5,17 @@ import path from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { promoteWorkspaceKeyEnvAlias } from './workspace-env.js'; -import { persistWorkspaceSession, resolveWorkspaceSessionKey } from './workspace-session.js'; +import { + persistWorkspaceSession, + pinProjectWorkspaceSession, + resolveWorkspaceSessionKey, +} from './workspace-session.js'; import { readProjectWorkspaceKey, readProjectWorkspaceSession, writeProjectWorkspaceKey, } from './project-workspace-key.js'; -import { readWorkspaceStore, setWorkspaceKey } from './workspace-store.js'; +import { readWorkspaceStore, setWorkspaceKey, switchWorkspace } from './workspace-store.js'; const tempRoots: string[] = []; @@ -79,6 +83,22 @@ describe('workspace session persistence', () => { }); }); + it('records the previous global workspace when a named session changes it', () => { + const root = tempRoot(); + const projectDataDir = path.join(root, 'project', '.agentworkforce', 'relay'); + const env = isolatedEnv(root); + setWorkspaceKey('default', 'rk_live_default', env); + + persistWorkspaceSession({ + workspaceKey: 'rk_live_session_two', + name: 'session-two', + projectDataDir, + env, + }); + + expect(readWorkspaceStore(env)).toMatchObject({ active: 'session-two', previous: 'default' }); + }); + it('pins an explicitly supplied key without changing the named global workspace', () => { const root = tempRoot(); const projectDataDir = path.join(root, 'project', '.agentworkforce', 'relay'); @@ -178,4 +198,24 @@ describe('workspace session persistence', () => { expect(resolveWorkspaceSessionKey({ projectDataDir, env })).toBe('rk_live_project'); }); + + it('rebinds the project without changing the machine-global active workspace or old enrollment', () => { + const root = tempRoot(); + const projectDataDir = path.join(root, 'project', '.agentworkforce', 'relay'); + const env = isolatedEnv(root); + setWorkspaceKey('default', 'rk_live_default', env); + setWorkspaceKey('scratch', 'rk_live_scratch', env); + switchWorkspace('scratch', env); + writeProjectWorkspaceKey(projectDataDir, 'rk_live_old', { + enrolledNodeId: 'node_old', + workspaceId: 'rw_old', + }); + + pinProjectWorkspaceSession({ workspaceKey: 'rk_live_default', projectDataDir, env }); + + expect(readProjectWorkspaceKey(projectDataDir)).toBe('rk_live_default'); + expect(readWorkspaceStore(env).active).toBe('scratch'); + expect(resolveWorkspaceSessionKey({ projectDataDir, env })).toBe('rk_live_default'); + expect(readProjectWorkspaceSession(projectDataDir)).toEqual({ workspaceKey: 'rk_live_default' }); + }); }); diff --git a/packages/cli/src/cli/lib/workspace-session.ts b/packages/cli/src/cli/lib/workspace-session.ts index 80b9ffa5f..94ea94bb9 100644 --- a/packages/cli/src/cli/lib/workspace-session.ts +++ b/packages/cli/src/cli/lib/workspace-session.ts @@ -16,6 +16,10 @@ export interface PersistWorkspaceSessionOptions extends WorkspaceSessionOptions name?: string; } +export interface PinProjectWorkspaceSessionOptions extends WorkspaceSessionOptions { + workspaceKey: string; +} + /** Validate and normalize a workspace session name before local or remote writes. */ export function validateWorkspaceSessionName(name: string): string { return validateWorkspaceName(name); @@ -86,6 +90,11 @@ export function persistWorkspaceSession( // while every other command in this project reads the new one. That is the // split this whole change exists to remove, and the workspace ids the // enrollment store holds cannot be checked against the key the pin holds. + // + // This intentionally does NOT delegate to `pinProjectWorkspaceSession` + // below, which always drops the enrolled-node association — that is correct + // for an explicit `rebind` but would reintroduce the bug described above + // for an ordinary switch/join/create that happens to stay on the same key. const keepsWorkspace = existing?.workspaceKey === workspaceKey; const enrolledNodeId = keepsWorkspace ? existing?.enrolledNodeId : undefined; writeProjectWorkspaceKey(projectDataDir, workspaceKey, { @@ -101,3 +110,17 @@ export function persistWorkspaceSession( ? { clearedEnrolledNodeId: existing.enrolledNodeId } : {}; } + +/** + * Rebind only the current project to a workspace key. This intentionally drops + * any enrolled-node association: a later `node up` must honor the newly pinned + * messaging workspace instead of resuming credentials from the old binding. + */ +export function pinProjectWorkspaceSession(options: PinProjectWorkspaceSessionOptions): void { + const workspaceKey = options.workspaceKey.trim(); + if (!workspaceKey) { + throw new Error('Workspace key is required.'); + } + const projectDataDir = options.projectDataDir ?? getProjectPaths(options.projectRoot).dataDir; + writeProjectWorkspaceKey(projectDataDir, workspaceKey); +} diff --git a/packages/cli/src/cli/telemetry/client.test.ts b/packages/cli/src/cli/telemetry/client.test.ts index 494ad1313..c3a9ec7bb 100644 --- a/packages/cli/src/cli/telemetry/client.test.ts +++ b/packages/cli/src/cli/telemetry/client.test.ts @@ -56,12 +56,14 @@ describe('telemetry client events', () => { vi.stubEnv('AGENT_RELAY_ORG_ID', ''); vi.stubEnv('AGENT_RELAY_ORG_SLUG', ''); vi.stubEnv('AGENT_RELAY_USER_EMAIL', ''); + vi.stubEnv('AGENT_RELAY_MACHINE_ID', ''); posthogMocks.capture.mockClear(); posthogMocks.identify.mockClear(); posthogMocks.alias.mockClear(); posthogMocks.groupIdentify.mockClear(); posthogMocks.shutdown.mockClear(); vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'error').mockImplementation(() => undefined); }); afterEach(async () => { @@ -101,6 +103,15 @@ describe('telemetry client events', () => { expect(posthogMocks.capture).not.toHaveBeenCalledWith(expect.objectContaining({ event: 'cli_install' })); }); + it('writes the first-run notice to stderr so JSON stdout stays parseable', () => { + initTelemetry({ cliVersion: '1.2.3' }); + + expect(console.log).not.toHaveBeenCalled(); + expect(console.error).toHaveBeenCalledWith( + 'Agent Relay collects usage telemetry to improve the product.' + ); + }); + describe('anonymous (not logged in)', () => { it('keys events by the machine hash and marks them unauthenticated', () => { const machineDistinctId = getDistinctId(); diff --git a/packages/cli/src/cli/telemetry/client.ts b/packages/cli/src/cli/telemetry/client.ts index e37ffcff2..ddfafcdb7 100644 --- a/packages/cli/src/cli/telemetry/client.ts +++ b/packages/cli/src/cli/telemetry/client.ts @@ -233,11 +233,13 @@ function showFirstRunNotice(): void { return; } - console.log(''); - console.log('Agent Relay collects usage telemetry to improve the product.'); - console.log('Run `agent-relay telemetry disable` to opt out.'); - console.log('Learn more: https://agentrelay.com/telemetry'); - console.log(''); + // Notices are diagnostics, never command data. Keeping them on stderr means + // a first run cannot corrupt commands whose stdout is a JSON contract. + console.error(''); + console.error('Agent Relay collects usage telemetry to improve the product.'); + console.error('Run `agent-relay telemetry disable` to opt out.'); + console.error('Learn more: https://agentrelay.com/telemetry'); + console.error(''); markNotified(); } diff --git a/packages/cloud/src/auth.test.ts b/packages/cloud/src/auth.test.ts index 49773972f..e070775b2 100644 --- a/packages/cloud/src/auth.test.ts +++ b/packages/cloud/src/auth.test.ts @@ -749,6 +749,7 @@ describe('refreshStoredAuth', () => { describe('authorizedApiFetch telemetry headers', () => { const telemetryEnvKeys = [ 'AGENT_RELAY_DISTINCT_ID', + 'AGENT_RELAY_MACHINE_ID', 'AGENT_RELAY_USER_ID', 'AGENT_RELAY_ORG_ID', 'AGENT_RELAY_ORG_SLUG', diff --git a/packages/cloud/src/index.ts b/packages/cloud/src/index.ts index c748465a3..36d75bf6b 100644 --- a/packages/cloud/src/index.ts +++ b/packages/cloud/src/index.ts @@ -143,6 +143,7 @@ export { writeProjectWorkspaceKey, type ProjectWorkspaceSession, type ResolveWorkspaceKeyOptions, + type WorkspaceKeyFileSystem, type WorkspaceKeySource, type WorkspaceSelection, } from './project-workspace-key.js'; diff --git a/packages/cloud/src/project-workspace-key.test.ts b/packages/cloud/src/project-workspace-key.test.ts index d5b6386b1..29a5c1b4f 100644 --- a/packages/cloud/src/project-workspace-key.test.ts +++ b/packages/cloud/src/project-workspace-key.test.ts @@ -145,4 +145,22 @@ describe('workspace precedence ladder diagnostics', () => { workspaceId: 'rw_repository', }); }); + + it('keeps the shared ladder authoritative when a caller injects repository-pin I/O', () => { + const env = { AGENT_RELAY_HOME: home }; + setWorkspaceKey('global', 'rk_global', env); + const fileSystem = { + readFileSync: (filePath: string, encoding: BufferEncoding): string => { + expect(filePath).toBe(projectWorkspaceKeyPath(dataDir)); + expect(encoding).toBe('utf-8'); + return JSON.stringify({ workspaceKey: 'rk_injected', workspaceId: 'rw_injected' }); + }, + }; + + expect(resolveWorkspaceSelection({ projectDataDir: dataDir, env, fileSystem })).toMatchObject({ + key: 'rk_injected', + source: 'project', + workspaceId: 'rw_injected', + }); + }); }); diff --git a/packages/cloud/src/project-workspace-key.ts b/packages/cloud/src/project-workspace-key.ts index 2fbe224fa..cf39845ec 100644 --- a/packages/cloud/src/project-workspace-key.ts +++ b/packages/cloud/src/project-workspace-key.ts @@ -32,6 +32,12 @@ export interface ResolveWorkspaceKeyOptions { projectRoot?: string; /** Explicit project Relay data directory. Takes precedence over projectRoot. */ projectDataDir?: string; + /** Optional filesystem adapter for reading the repository pin. */ + fileSystem?: WorkspaceKeyFileSystem; +} + +export interface WorkspaceKeyFileSystem { + readFileSync(filePath: string, encoding: BufferEncoding): string; } /** @@ -55,14 +61,20 @@ export function projectWorkspaceKeyPath(dataDir: string): string { } /** Read a project broker's workspace key, falling through on absent or malformed state. */ -export function readProjectWorkspaceKey(dataDir: string): string | undefined { - return readProjectWorkspaceSession(dataDir)?.workspaceKey; +export function readProjectWorkspaceKey( + dataDir: string, + fileSystem: WorkspaceKeyFileSystem = fs +): string | undefined { + return readProjectWorkspaceSession(dataDir, fileSystem)?.workspaceKey; } /** Read the project workspace and its optional enrolled Fleet identity. */ -export function readProjectWorkspaceSession(dataDir: string): ProjectWorkspaceSession | undefined { +export function readProjectWorkspaceSession( + dataDir: string, + fileSystem: WorkspaceKeyFileSystem = fs +): ProjectWorkspaceSession | undefined { try { - const raw = fs.readFileSync(projectWorkspaceKeyPath(dataDir), 'utf-8'); + const raw = fileSystem.readFileSync(projectWorkspaceKeyPath(dataDir), 'utf-8'); const parsed = JSON.parse(raw) as Partial; const workspaceKey = trimOrUndefined(parsed.workspaceKey); if (!workspaceKey) return undefined; @@ -160,7 +172,7 @@ export function resolveWorkspaceSelection( } const dataDir = options.projectDataDir ?? projectDataDir(options.projectRoot); - const project = dataDir ? readProjectWorkspaceSession(dataDir) : undefined; + const project = dataDir ? readProjectWorkspaceSession(dataDir, options.fileSystem ?? fs) : undefined; if (project) { return { key: project.workspaceKey, diff --git a/packages/cloud/src/workspace-key.ts b/packages/cloud/src/workspace-key.ts index d4b7e8138..f10229583 100644 --- a/packages/cloud/src/workspace-key.ts +++ b/packages/cloud/src/workspace-key.ts @@ -9,6 +9,7 @@ export { writeProjectWorkspaceKey, type ProjectWorkspaceSession, type ResolveWorkspaceKeyOptions, + type WorkspaceKeyFileSystem, type WorkspaceKeySource, type WorkspaceSelection, } from './project-workspace-key.js'; diff --git a/packages/cloud/src/workspace-store.test.ts b/packages/cloud/src/workspace-store.test.ts index 03c0262c1..5e9c45dc0 100644 --- a/packages/cloud/src/workspace-store.test.ts +++ b/packages/cloud/src/workspace-store.test.ts @@ -36,6 +36,20 @@ describe('workspace store', () => { setActiveWorkspace('support'); expect(resolveActiveWorkspaceKey()).toBe('rk_support'); + expect(readWorkspaceStore().previous).toBe('ops'); + }); + + it('records only genuine active-workspace changes', () => { + setWorkspaceKey('ops', 'rk_ops'); + setActiveWorkspace('ops'); + expect(readWorkspaceStore().previous).toBeUndefined(); + + setWorkspaceKey('support', 'rk_support'); + setActiveWorkspace('support'); + expect(readWorkspaceStore()).toMatchObject({ active: 'support', previous: 'ops' }); + + setActiveWorkspace('support'); + expect(readWorkspaceStore()).toMatchObject({ active: 'support', previous: 'ops' }); }); it('throws when switching to an unknown workspace', () => { diff --git a/packages/cloud/src/workspace-store.ts b/packages/cloud/src/workspace-store.ts index e6977ff92..11ae26973 100644 --- a/packages/cloud/src/workspace-store.ts +++ b/packages/cloud/src/workspace-store.ts @@ -9,6 +9,8 @@ import path from 'node:path'; */ export interface WorkspaceStore { active?: string; + /** Workspace that was active before the most recent named selection. */ + previous?: string; workspaces: Record; } @@ -38,7 +40,12 @@ export function readWorkspaceStore(env: NodeJS.ProcessEnv = process.env): Worksp const file = workspaceStorePath(env); try { const parsed = JSON.parse(fs.readFileSync(file, 'utf-8')) as Partial; - return { active: parsed.active, workspaces: parsed.workspaces ?? {} }; + const previous = typeof parsed.previous === 'string' ? parsed.previous.trim() : ''; + return { + active: parsed.active, + ...(previous ? { previous } : {}), + workspaces: parsed.workspaces ?? {}, + }; } catch (err: unknown) { if (isNodeError(err) && err.code === 'ENOENT') { return { workspaces: {} }; @@ -75,6 +82,9 @@ export function setActiveWorkspace(name: string, env: NodeJS.ProcessEnv = proces `Unknown workspace "${workspaceName}". Add it with \`relay workspace set_key ${workspaceName} \`.` ); } + if (store.active && store.active !== workspaceName) { + store.previous = store.active; + } store.active = workspaceName; writeWorkspaceStore(store, env); return store; From 2a4f1b048c56261f772c5db7350aa3e12afee4d3 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Tue, 4 Aug 2026 12:34:05 +0200 Subject: [PATCH 3/7] fix(cli): address workspace restore review feedback --- CHANGELOG.md | 1 + packages/cli/src/cli/commands/core.test.ts | 132 +++++++++++++++--- packages/cli/src/cli/commands/core.ts | 5 +- .../cli/src/cli/commands/workspace.test.ts | 4 +- packages/cli/src/cli/commands/workspace.ts | 1 - packages/cli/src/cli/lib/broker-lifecycle.ts | 23 ++- .../cli/src/cli/lib/workspace-session.test.ts | 2 +- .../harness-driver/src/spawn-config.test.ts | 18 +++ packages/harness-driver/src/spawn-config.ts | 6 +- 9 files changed, 160 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98011c03d..122c34fd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - `workspace create` warns on stderr when it changes the active workspace and records the prior name; named switches now record the same restore point, and first-run telemetry notices no longer contaminate JSON stdout. +- `agent-relay node status` reports whether the broker workspace came from a command-line flag, environment variable, repository pin, machine-global active workspace, or first-run creation. ### Fixed diff --git a/packages/cli/src/cli/commands/core.test.ts b/packages/cli/src/cli/commands/core.test.ts index cac21e274..6bebd3d23 100644 --- a/packages/cli/src/cli/commands/core.test.ts +++ b/packages/cli/src/cli/commands/core.test.ts @@ -507,11 +507,15 @@ describe('registerCoreCommands', () => { const exitCode = await runCommand(program, ['up', '--background']); expect(exitCode).toBe(0); - expect(deps.spawnProcess).toHaveBeenCalledWith('/usr/bin/node', ['/tmp/agent-relay.js', 'up'], { - detached: true, - stdio: 'ignore', - env: deps.env, - }); + expect(deps.spawnProcess).toHaveBeenCalledWith( + '/usr/bin/node', + ['/tmp/agent-relay.js', 'up', '--background-child'], + { + detached: true, + stdio: 'ignore', + env: deps.env, + } + ); expect(spawnedProcess.unref).toHaveBeenCalled(); expect(sleepImpl).toHaveBeenCalledWith(500); expect(sdkStatusClient.getStatus).toHaveBeenCalledTimes(1); @@ -570,7 +574,15 @@ describe('registerCoreCommands', () => { // `ps` for the daemon's whole lifetime) must never carry it. expect(deps.spawnProcess).toHaveBeenCalledWith( '/usr/bin/node', - ['/tmp/agent-relay.js', 'up', '--state-dir', stateDir, '--broker-name', 'relayfile-dev'], + [ + '/tmp/agent-relay.js', + 'up', + '--state-dir', + stateDir, + '--broker-name', + 'relayfile-dev', + '--background-child', + ], { detached: true, stdio: 'ignore', @@ -666,7 +678,7 @@ describe('registerCoreCommands', () => { expect(exitCode).toBe(0); expect(deps.spawnProcess).toHaveBeenCalledWith( '/tmp/agent-relay-darwin-arm64', - ['node', 'up', '--config', 'agent-relay.mjs', '--broker-name', 'sf-mini'], + ['node', 'up', '--config', 'agent-relay.mjs', '--broker-name', 'sf-mini', '--background-child'], { detached: true, stdio: 'ignore', @@ -934,6 +946,35 @@ describe('registerCoreCommands', () => { ); }); + it.each(['../../../etc/relay-background-error', '/tmp/relay-background-error-escape'])( + 'detached-child failure ignores an untrusted background error path %s', + async (untrustedPath) => { + const fs = createFsMock(); + const relay = createRelayMock({ + getStatus: vi.fn(async () => { + throw new Error('detached child failed'); + }), + }); + const { program, dataDir } = createHarness({ + fs, + relay, + env: { + AGENT_RELAY_BACKGROUND_START_ERROR_FILE: untrustedPath, + }, + }); + + const exitCode = await runCommand(program, ['up', '--background-child']); + + expect(exitCode).toBe(1); + expect(fs.writeFileSync).toHaveBeenCalledWith( + `${dataDir}/background-start-error.log`, + 'detached child failed\n', + 'utf-8' + ); + expect(fs.writeFileSync).not.toHaveBeenCalledWith(untrustedPath, expect.anything(), expect.anything()); + } + ); + it('down --force only kills actual orphaned broker executables for the project', async () => { const runningPids = new Set([222, 444, 666]); const execCommand = vi.fn(async (command: string) => { @@ -1629,6 +1670,43 @@ describe('registerCoreCommands', () => { expect(env.RELAY_API_KEY).toBe('rk_live_pinned'); }); + it('up resumes the repository pin when an enrolled node token is present', async () => { + const env: NodeJS.ProcessEnv = { + RELAY_NODE_ID: 'node_enrolled', + RELAY_NODE_TOKEN: 'nt_enrolled', + }; + const projectSessionPath = '/tmp/project/.agentworkforce/relay/workspace-key.json'; + const fs = createFsMock({ + [projectSessionPath]: JSON.stringify({ + workspaceKey: 'rk_live_project_pin', + workspaceId: 'rw_project', + enrolledNodeId: 'node_enrolled', + }), + }); + const relay = createRelayMock({ + workspaceKey: 'rk_live_project_pin', + workspaceId: 'rw_project', + }); + const createRelay = vi.fn(async () => { + // This is the non-mocked handoff to broker creation: the project pin is + // already canonicalized even though the enrolled identity is present. + expect(env.RELAY_WORKSPACE_KEY).toBe('rk_live_project_pin'); + expect(env.RELAY_API_KEY).toBe('rk_live_project_pin'); + expect(env.RELAY_NODE_TOKEN).toBe('nt_enrolled'); + return relay; + }); + const { program, deps } = createHarness({ fs, env, relay, createRelay }); + + const exitCode = await runCommand(program, ['up']); + + expect(exitCode).toBeUndefined(); + expect(createRelay).toHaveBeenCalledTimes(1); + expect(deps.log).toHaveBeenCalledWith( + 'Workspace source: repository pin (/tmp/project/.agentworkforce/relay/workspace-key.json)' + ); + expect(deps.log).toHaveBeenCalledWith('Workspace: joined rw_project'); + }); + it('up treats a non-blank workspace env alias as explicit when the primary is blank', async () => { const env: NodeJS.ProcessEnv = { RELAY_WORKSPACE_KEY: ' ', @@ -1676,7 +1754,7 @@ describe('registerCoreCommands', () => { } }); - it('background up forwards a resumed enrolled-node association to the detached child', async () => { + it('background up forwards the repository pin with an enrolled identity to the detached child', async () => { const spawnedProcess = createSpawnedProcessMock(); let now = 0; const projectSessionPath = '/tmp/project/.agentworkforce/relay/workspace-key.json'; @@ -1694,9 +1772,21 @@ describe('registerCoreCommands', () => { if ((pid === 9001 || pid === 5151) && signal === 0) return; throw new Error('unexpected kill check'); }); + sdkStatusClient.getStatus.mockResolvedValue({ + node_connected: true, + node_delivery: { token_present: true, connected: true }, + }); + sdkStatusClient.getSession.mockResolvedValue({ + workspace_key: 'rk_live_pinned', + node_id: 'node_enrolled', + node_name: 'project', + }); const { program, deps } = createHarness({ fs, - env: {}, + env: { + RELAY_NODE_ID: 'node_enrolled', + RELAY_NODE_TOKEN: 'nt_enrolled', + }, spawnedProcess, killImpl, nowImpl: vi.fn(() => now), @@ -1706,15 +1796,21 @@ describe('registerCoreCommands', () => { const exitCode = await runCommand(program, ['up', '--background']); expect(exitCode).toBe(0); - expect(deps.spawnProcess).toHaveBeenCalledWith('/usr/bin/node', ['/tmp/agent-relay.js', 'up'], { - detached: true, - stdio: 'ignore', - env: expect.objectContaining({ - AGENT_RELAY_ENROLLED_NODE_ID: 'node_enrolled', - RELAY_API_KEY: 'rk_live_pinned', - RELAY_WORKSPACE_KEY: 'rk_live_pinned', - }), - }); + expect(deps.spawnProcess).toHaveBeenCalledWith( + '/usr/bin/node', + ['/tmp/agent-relay.js', 'up', '--background-child'], + { + detached: true, + stdio: 'ignore', + env: expect.objectContaining({ + AGENT_RELAY_ENROLLED_NODE_ID: 'node_enrolled', + RELAY_API_KEY: 'rk_live_pinned', + RELAY_NODE_ID: 'node_enrolled', + RELAY_NODE_TOKEN: 'nt_enrolled', + RELAY_WORKSPACE_KEY: 'rk_live_pinned', + }), + } + ); }); it('up configures a bundled Agent Relay MCP command when the wrapper script exists', async () => { diff --git a/packages/cli/src/cli/commands/core.ts b/packages/cli/src/cli/commands/core.ts index aa5b5256e..07113667d 100644 --- a/packages/cli/src/cli/commands/core.ts +++ b/packages/cli/src/cli/commands/core.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { exec, spawn as spawnProcess } from 'node:child_process'; import { promisify } from 'node:util'; -import { Command, InvalidArgumentError } from 'commander'; +import { Command, InvalidArgumentError, Option } from 'commander'; import { getProjectPaths, loadTeamsConfig } from '@agent-relay/config'; import { HarnessDriverClient, type BrokerInitArgs } from '@agent-relay/harness-driver'; @@ -278,6 +278,8 @@ export function withDefaults(overrides: Partial = {}): CoreDep export interface UpCommandOptions { spawn?: boolean; background?: boolean; + /** Internal marker set only on the detached child re-exec. */ + backgroundChild?: boolean; verbose?: boolean; workspaceKey?: string; stateDir?: string; @@ -298,6 +300,7 @@ export function addUpCommandOptions(command: Command): Command { .option('--spawn', 'Force spawn all agents from teams.json') .option('--no-spawn', 'Do not auto-spawn agents (just start broker)') .option('--background', 'Run broker in the background (detached)') + .addOption(new Option('--background-child').hideHelp()) .option('--verbose', 'Enable verbose logging') .option('--workspace-key ', 'Use a pre-established Relaycast workspace key') .option('--wk ', 'Alias for --workspace-key') diff --git a/packages/cli/src/cli/commands/workspace.test.ts b/packages/cli/src/cli/commands/workspace.test.ts index e254c9dbe..1ff175af9 100644 --- a/packages/cli/src/cli/commands/workspace.test.ts +++ b/packages/cli/src/cli/commands/workspace.test.ts @@ -191,7 +191,7 @@ describe('registerWorkspaceCommands', () => { expect(() => JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0]))).not.toThrow(); }); - it('workspace create --json keeps stdout parseable and routes the warning away from it', async () => { + it('workspace create keeps stdout parseable and routes the warning away from it', async () => { vi.mocked(readWorkspaceStore).mockReturnValueOnce({ active: 'default', workspaces: { default: { key: 'rk_live_default' } }, @@ -201,7 +201,7 @@ describe('registerWorkspaceCommands', () => { workspaceKey: 'rk_live_json_workspace', } as never); - await program.parseAsync(['node', 'agent-relay', 'workspace', 'create', 'json-workspace', '--json']); + await program.parseAsync(['node', 'agent-relay', 'workspace', 'create', 'json-workspace']); expect(vi.mocked(deps.log).mock.calls).toHaveLength(1); expect(JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0]))).toMatchObject({ diff --git a/packages/cli/src/cli/commands/workspace.ts b/packages/cli/src/cli/commands/workspace.ts index 51f34bb87..659e61d02 100644 --- a/packages/cli/src/cli/commands/workspace.ts +++ b/packages/cli/src/cli/commands/workspace.ts @@ -98,7 +98,6 @@ export function registerWorkspaceCommands( .description('Create a new workspace and store its key') .argument('', 'Workspace name') .option('--base-url ', 'Override the API base URL') - .option('--json', 'Output the created workspace as JSON') .option('--reveal-secrets', 'Include the raw workspace key in the output') .action(async (name: string, o: Record) => { await runSdk(deps, async () => { diff --git a/packages/cli/src/cli/lib/broker-lifecycle.ts b/packages/cli/src/cli/lib/broker-lifecycle.ts index 5417c9d29..d8cebbc6f 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.ts @@ -36,6 +36,8 @@ import { type UpOptions = { spawn?: boolean; background?: boolean; + /** Internal marker set only on the detached child re-exec. */ + backgroundChild?: boolean; verbose?: boolean; workspaceKey?: string; stateDir?: string; @@ -777,11 +779,18 @@ function readBackgroundStartError(dataDir: string, deps: CoreDependencies): stri } } -function recordBackgroundStartError(message: string, deps: CoreDependencies): void { - const file = deps.env.AGENT_RELAY_BACKGROUND_START_ERROR_FILE?.trim(); - if (!file) return; +function recordBackgroundStartError( + message: string, + dataDir: string, + isDetachedChild: boolean, + deps: CoreDependencies +): void { + if (!isDetachedChild) return; try { - deps.fs.writeFileSync(file, `${message}\n`, 'utf-8'); + // Never trust a project-loaded environment variable as a filesystem path. + // Detached startup owns one fixed diagnostic file inside its resolved + // broker state directory; foreground failures do not write it at all. + deps.fs.writeFileSync(backgroundStartErrorPath(dataDir), `${message}\n`, 'utf-8'); } catch { // Diagnostics must never replace the original startup error. } @@ -1076,6 +1085,9 @@ function childUpArgsForDetachedStart(options: UpOptions, deps: CoreDependencies) if (options.verbose === true && !args.includes('--verbose')) { args.push('--verbose'); } + if (!args.includes('--background-child')) { + args.push('--background-child'); + } return args; } @@ -1488,7 +1500,6 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): const startErrorPath = backgroundStartErrorPath(paths.dataDir); safeUnlink(startErrorPath, deps); - deps.env.AGENT_RELAY_BACKGROUND_START_ERROR_FILE = startErrorPath; const args = childUpArgsForDetachedStart(options, deps); const invocation = detachedCliInvocation(deps, args); let child: SpawnedProcess; @@ -1808,7 +1819,7 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): error_class: classifyBrokerStartError(err), }); const detailedMessage = describeErrorWithCause(err); - recordBackgroundStartError(detailedMessage, deps); + recordBackgroundStartError(detailedMessage, paths.dataDir, options.backgroundChild === true, deps); if (isBrokerAlreadyRunningError(message)) { reportAlreadyRunningError(message, paths.dataDir, deps); } else { diff --git a/packages/cli/src/cli/lib/workspace-session.test.ts b/packages/cli/src/cli/lib/workspace-session.test.ts index 615e1fafe..312632579 100644 --- a/packages/cli/src/cli/lib/workspace-session.test.ts +++ b/packages/cli/src/cli/lib/workspace-session.test.ts @@ -199,7 +199,7 @@ describe('workspace session persistence', () => { expect(resolveWorkspaceSessionKey({ projectDataDir, env })).toBe('rk_live_project'); }); - it('rebinds the project without changing the machine-global active workspace or old enrollment', () => { + it('rebinds the project without changing the machine-global active workspace and clears the old enrollment', () => { const root = tempRoot(); const projectDataDir = path.join(root, 'project', '.agentworkforce', 'relay'); const env = isolatedEnv(root); diff --git a/packages/harness-driver/src/spawn-config.test.ts b/packages/harness-driver/src/spawn-config.test.ts index 9fc18ce24..1b7afb7d6 100644 --- a/packages/harness-driver/src/spawn-config.test.ts +++ b/packages/harness-driver/src/spawn-config.test.ts @@ -82,4 +82,22 @@ describe('buildBrokerSpawnConfig', () => { '/tmp/relay-state', ]); }); + + it('prefers RELAY_WORKSPACE_KEY over AGENT_RELAY_WORKSPACE_KEY in the same env', () => { + const config = buildBrokerSpawnConfig( + { + cwd: '/tmp/my-project', + env: { + RELAY_WORKSPACE_KEY: 'rk_live_primary', + AGENT_RELAY_WORKSPACE_KEY: 'rk_live_alias', + }, + }, + 'br_test', + {} + ); + + expect(config.workspaceKey).toBe('rk_live_primary'); + expect(config.env.RELAY_WORKSPACE_KEY).toBe('rk_live_primary'); + expect(config.env.AGENT_RELAY_WORKSPACE_KEY).toBe('rk_live_primary'); + }); }); diff --git a/packages/harness-driver/src/spawn-config.ts b/packages/harness-driver/src/spawn-config.ts index ee0212ab3..f01047f34 100644 --- a/packages/harness-driver/src/spawn-config.ts +++ b/packages/harness-driver/src/spawn-config.ts @@ -94,10 +94,10 @@ export function buildBrokerSpawnConfig( (path.basename(cwd) || 'project'); const workspaceKey = nonEmptyString(options?.workspaceKey) ?? - nonEmptyString(options?.env?.AGENT_RELAY_WORKSPACE_KEY) ?? nonEmptyString(options?.env?.RELAY_WORKSPACE_KEY) ?? - nonEmptyString(parentEnv.AGENT_RELAY_WORKSPACE_KEY) ?? - nonEmptyString(parentEnv.RELAY_WORKSPACE_KEY); + nonEmptyString(options?.env?.AGENT_RELAY_WORKSPACE_KEY) ?? + nonEmptyString(parentEnv.RELAY_WORKSPACE_KEY) ?? + nonEmptyString(parentEnv.AGENT_RELAY_WORKSPACE_KEY); const channels = options?.channels ?? ['general']; const timeoutMs = options?.startupTimeoutMs ?? 45_000; const userArgs = buildBrokerInitArgs(options?.binaryArgs); From 270c887d0f9c791d84109521cd3bfb962769f5b2 Mon Sep 17 00:00:00 2001 From: Proactive Runtime Bot Date: Tue, 4 Aug 2026 22:58:11 +0200 Subject: [PATCH 4/7] fix(broker): verify worker process before spawn success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agent-relay node agent spawn could report success while the spawned worker process had already exited (e.g. a wrapper that fails to launch its harness) — Command::spawn only proves the wrapper was created, not that it survived. Add a brief stability window after spawn that non-blockingly checks the child is still alive before reporting success; if it already exited, remove the stale registry entry and return the real error (exit status + log path) instead of letting node agent list briefly advertise a dead process. Verified: - cargo test --package agent-relay-broker worker:: (62/62, 4 consecutive clean runs, plus 5 isolated runs of the specific new tests) - Real integration test: RELAY_INTEGRATION_REAL_CLI=1 node --test tests/integration/broker/dist/cli-spawn.test.js (missing-CLI rejection path, 21.5s, real broker) --- CHANGELOG.md | 1 + crates/broker/src/worker.rs | 86 ++++++++++++++++++++++ tests/integration/broker/cli-spawn.test.ts | 30 ++++++++ 3 files changed, 117 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 122c34fd2..e66fd1b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `agent-relay workspace switch|join` keeps the project's enrolled fleet node id instead of dropping it, which previously produced the pin state that made the next `node up` ignore the enrollment store. - `agent-relay up` / `node up` use one precedence ladder: `--workspace-key` → workspace environment variables → repository pin → machine-global active workspace → creating one. A fresh project joins the active workspace instead of silently creating another, startup announces the winning source, and `node status` reports the same five-source provenance. - Cloud enrollment selects node identity without overriding workspace resolution. A conflict with the repository pin stops startup, names both non-secret sources, and points to `workspace rebind ` as the recovery path. +- `agent-relay node agent spawn` now verifies that the worker process survives startup before reporting success, and reports its exit status and log path when launch fails. - Detached `node up --background` surfaces early child failures and stops polling when the child exits without trying to kill an already dead process. ## [11.4.1] - 2026-08-03 diff --git a/crates/broker/src/worker.rs b/crates/broker/src/worker.rs index 798f20c2b..71f201568 100644 --- a/crates/broker/src/worker.rs +++ b/crates/broker/src/worker.rs @@ -53,6 +53,12 @@ const APP_SERVER_RELEASE_GRACE: Duration = Duration::from_secs(35); /// healthy-but-slow agent is far worse than listing a dead one a little longer. const WORKER_READY_DEADLINE: Duration = Duration::from_secs(90); +/// Briefly hold the spawn acknowledgement so a wrapper that cannot launch its +/// harness has time to exit. `Command::spawn` only proves that the wrapper was +/// created; without this stability window the HTTP API can report success even +/// though the wrapper is already gone by the time the caller lists agents. +const WORKER_SPAWN_STABILITY_WINDOW: Duration = Duration::from_millis(250); + /// How long to wait for a SIGKILLed orphan wrapper to be reaped before giving /// up. Bounded so a wrapper stuck in uninterruptible sleep cannot stall the /// maintenance tick, which also drives delivery retries. @@ -119,6 +125,32 @@ pub(crate) fn orphaned_worker( None } +/// Confirm that a freshly-created worker process survives its initial handoff. +/// +/// This is intentionally narrower than `worker_ready`: PTY readiness may take +/// up to 25 seconds and is processed by the same runtime loop that services the +/// spawn request. The short stability probe catches launch failures without +/// deadlocking that loop or making every successful spawn wait for a TUI. +async fn confirm_worker_process_alive( + name: &str, + child: &mut Child, + log_path: Option<&Path>, + stability_window: Duration, +) -> Result<()> { + tokio::time::sleep(stability_window).await; + let Some(status) = child + .try_wait() + .with_context(|| format!("failed to verify agent '{name}' process after spawn"))? + else { + return Ok(()); + }; + + let log_hint = log_path + .map(|path| format!("; see worker log {}", path.display())) + .unwrap_or_default(); + anyhow::bail!("agent '{name}' process exited during startup ({status}){log_hint}") +} + // Working/idle activity inference from PTY output comes from the // harness-agnostic `relay-pty` crate. pub(crate) use relay_pty::detection; @@ -912,6 +944,7 @@ impl WorkerRegistry { let stdout = child.stdout.take().context("worker missing stdout pipe")?; let stderr = child.stderr.take().context("worker missing stderr pipe")?; let log_file = self.worker_log_path(&spec.name); + let startup_log_file = log_file.clone(); spawn_worker_reader( self.event_tx.clone(), @@ -956,6 +989,28 @@ impl WorkerRegistry { ) .await?; + let startup_confirmation = { + let handle = self + .workers + .get_mut(&spec.name) + .with_context(|| format!("unknown worker '{}' after spawn", spec.name))?; + confirm_worker_process_alive( + &spec.name, + &mut handle.child, + startup_log_file.as_deref(), + WORKER_SPAWN_STABILITY_WINDOW, + ) + .await + }; + if let Err(error) = startup_confirmation { + // `try_wait` reaped an exited wrapper. Remove the stale registry + // entry before returning the error so `node agent list` cannot + // briefly advertise a process the spawn call just rejected. + self.workers.remove(&spec.name); + self.initial_tasks.remove(&spec.name); + return Err(error); + } + tracing::info!( target = "broker::spawn", name = %spec.name, @@ -2015,6 +2070,37 @@ mod tests { assert!(reg.list(&HashMap::new()).is_empty()); } + #[tokio::test] + async fn spawn_confirmation_rejects_a_process_that_exits_immediately() { + let mut child = Command::new("sleep").arg("0").spawn().unwrap(); + + let error = confirm_worker_process_alive( + "failed-worker", + &mut child, + Some(Path::new("/tmp/failed-worker.log")), + Duration::from_millis(100), + ) + .await + .unwrap_err(); + + let message = error.to_string(); + assert!(message.contains("process exited during startup")); + assert!(message.contains("/tmp/failed-worker.log")); + } + + #[tokio::test] + async fn spawn_confirmation_accepts_a_process_that_stays_alive() { + let mut child = Command::new("sleep").arg("30").spawn().unwrap(); + + confirm_worker_process_alive("live-worker", &mut child, None, Duration::from_millis(100)) + .await + .unwrap(); + + terminate_child(&mut child, Duration::from_millis(200)) + .await + .unwrap(); + } + // The wrapper process can outlive the harness it hosts, so reaping on the // wrapper alone leaves a dead agent listed as `working` forever. mod orphaned_worker { diff --git a/tests/integration/broker/cli-spawn.test.ts b/tests/integration/broker/cli-spawn.test.ts index 74c984c17..f0b500960 100644 --- a/tests/integration/broker/cli-spawn.test.ts +++ b/tests/integration/broker/cli-spawn.test.ts @@ -480,6 +480,36 @@ test('cli-spawn: duplicate name — second spawn with same name fails', { timeou } }); +test( + 'cli-spawn: missing CLI fails before success and leaves no listed agent', + { timeout: 30_000 }, + async (t) => { + if (skipIfMissing(t)) return; + + const harness = new BrokerHarness(); + await harness.start(); + const suffix = uniqueSuffix(); + const agentName = `missing-cli-${suffix}`; + const missingCli = `agent-relay-missing-${suffix}`; + + try { + await assert.rejects( + () => harness.spawnAgent(agentName, missingCli, ['general']), + /process exited during startup/, + 'a wrapper that cannot launch its CLI must reject the spawn request' + ); + + const agents = await harness.listAgents(); + assert.ok( + !agents.some((agent) => agent.name === agentName), + 'a rejected startup must not leave a stale agent in the broker list' + ); + } finally { + await harness.stop(); + } + } +); + // ── Cat Process Tests (lightweight, no real CLI needed) ──────────────────── test('cli-spawn: cat — spawn lightweight process and deliver', { timeout: 30_000 }, async (t) => { From 9420f16ff423f71f5e94d8e571a70875cbc44a4b Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 6 Aug 2026 10:33:41 +0200 Subject: [PATCH 5/7] fix(cli,broker): address PR #1429 review round - Write connection.json atomically (tmp file + renameSync) so a concurrent writer's newer content is never clobbered by a stale read-modify-write. Extends CoreFileSystem with renameSync. - Attribute workspace provenance to RELAY_WORKSPACES_JSON when set, since the broker's startup_session_set_with_options() checks it before any single workspace key. node up / node status no longer report a source the broker didn't actually use. - Gate the two worker.rs process-alive tests behind #[cfg(unix)] (they shell out to `sleep`, unavailable on Windows) and widen the negative-case stability window to reduce CI flakiness. - Unregister a rejected spawn from the restart supervisor so it can never generate a pending restart for a worker that never launched. - Broaden the cli-spawn EPIPE-race assertion to accept either rejection message, since both are the correct contract. - Document that `node up` startup and `node status` print different workspace-source formats. Co-Authored-By: Claude Sonnet 5 --- crates/broker/src/worker.rs | 9 ++- packages/cli/README.md | 13 +++- packages/cli/src/cli/commands/core.test.ts | 5 ++ packages/cli/src/cli/commands/core.ts | 2 + .../cli/src/cli/lib/broker-lifecycle.test.ts | 38 +++++++++++ packages/cli/src/cli/lib/broker-lifecycle.ts | 66 +++++++++++++++---- tests/integration/broker/cli-spawn.test.ts | 8 ++- 7 files changed, 123 insertions(+), 18 deletions(-) diff --git a/crates/broker/src/worker.rs b/crates/broker/src/worker.rs index 71f201568..bfa0c114c 100644 --- a/crates/broker/src/worker.rs +++ b/crates/broker/src/worker.rs @@ -1005,9 +1005,12 @@ impl WorkerRegistry { if let Err(error) = startup_confirmation { // `try_wait` reaped an exited wrapper. Remove the stale registry // entry before returning the error so `node agent list` cannot - // briefly advertise a process the spawn call just rejected. + // briefly advertise a process the spawn call just rejected. Also + // unregister from the restart supervisor so a rejected spawn can + // never generate a pending restart for a worker that never launched. self.workers.remove(&spec.name); self.initial_tasks.remove(&spec.name); + self.supervisor.unregister(&spec.name); return Err(error); } @@ -2070,6 +2073,7 @@ mod tests { assert!(reg.list(&HashMap::new()).is_empty()); } + #[cfg(unix)] #[tokio::test] async fn spawn_confirmation_rejects_a_process_that_exits_immediately() { let mut child = Command::new("sleep").arg("0").spawn().unwrap(); @@ -2078,7 +2082,7 @@ mod tests { "failed-worker", &mut child, Some(Path::new("/tmp/failed-worker.log")), - Duration::from_millis(100), + Duration::from_millis(500), ) .await .unwrap_err(); @@ -2088,6 +2092,7 @@ mod tests { assert!(message.contains("/tmp/failed-worker.log")); } + #[cfg(unix)] #[tokio::test] async fn spawn_confirmation_accepts_a_process_that_stays_alive() { let mut child = Command::new("sleep").arg("30").spawn().unwrap(); diff --git a/packages/cli/README.md b/packages/cli/README.md index 1db68a8e7..eceb7a325 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -68,13 +68,24 @@ announces creation when none of the first four sources resolves. Startup and `node status` report the winning source without printing key material. Status uses the same five labels: command-line flag, environment, -repository pin, machine-global active workspace, or created. +repository pin, machine-global active workspace, or created — but the two +commands print different strings: startup shows the resolved origin +(an absolute path for a repository pin), `node status` shows a fixed, +relative-path label. + +Startup output: ```text Workspace source: repository pin (/repo/.agentworkforce/relay/workspace-key.json) Workspace: joined rw_7ccfea89 ``` +`node status` output: + +```text +Workspace source: repository pin (.agentworkforce/relay/workspace-key.json) +``` + A Cloud enrollment (`RELAY_NODE_TOKEN`, or a record in the Fleet enrollment store) selects the node's _identity_, not its workspace, so it never appears on the ladder. If a stored enrollment addresses a different workspace than the diff --git a/packages/cli/src/cli/commands/core.test.ts b/packages/cli/src/cli/commands/core.test.ts index 6bebd3d23..49e25fb70 100644 --- a/packages/cli/src/cli/commands/core.test.ts +++ b/packages/cli/src/cli/commands/core.test.ts @@ -126,6 +126,11 @@ function createFsMock(initialFiles: Record = {}): CoreFileSystem writeFileSync: vi.fn((filePath: string, data: string) => { files.set(filePath, String(data)); }), + renameSync: vi.fn((oldPath: string, newPath: string) => { + const data = files.get(oldPath); + files.delete(oldPath); + if (data !== undefined) files.set(newPath, data); + }), unlinkSync: vi.fn((filePath: string) => { files.delete(filePath); }), diff --git a/packages/cli/src/cli/commands/core.ts b/packages/cli/src/cli/commands/core.ts index 07113667d..fb63f5a03 100644 --- a/packages/cli/src/cli/commands/core.ts +++ b/packages/cli/src/cli/commands/core.ts @@ -71,6 +71,7 @@ export interface CoreFileSystem { existsSync: (path: string) => boolean; readFileSync: (path: string, encoding: BufferEncoding) => string; writeFileSync: (path: string, data: string, encoding?: BufferEncoding) => void; + renameSync: (oldPath: string, newPath: string) => void; unlinkSync: (path: string) => void; readdirSync: (path: string) => string[]; mkdirSync: (path: string, options?: { recursive?: boolean }) => void; @@ -208,6 +209,7 @@ export function withDefaults(overrides: Partial = {}): CoreDep existsSync: fs.existsSync, readFileSync: (filePath, encoding) => fs.readFileSync(filePath, encoding), writeFileSync: (filePath, data, encoding) => fs.writeFileSync(filePath, data, encoding), + renameSync: (oldPath, newPath) => fs.renameSync(oldPath, newPath), unlinkSync: fs.unlinkSync, readdirSync: (dirPath) => fs.readdirSync(dirPath), mkdirSync: (dirPath, options) => fs.mkdirSync(dirPath, options), diff --git a/packages/cli/src/cli/lib/broker-lifecycle.test.ts b/packages/cli/src/cli/lib/broker-lifecycle.test.ts index 3c59541d0..6dbab834b 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.test.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.test.ts @@ -282,6 +282,7 @@ function createUpHarness() { readFileSync: (file: string, encoding: BufferEncoding) => file.endsWith('connection.json') ? connection : fsReal.readFileSync(file, encoding), writeFileSync: fsReal.writeFileSync, + renameSync: fsReal.renameSync, unlinkSync: fsReal.unlinkSync, readdirSync: fsReal.readdirSync, mkdirSync: fsReal.mkdirSync, @@ -560,6 +561,27 @@ describe('runUpCommand workspace precedence', () => { expect(readBindingSource(dataDir)).toBe('created'); }); + it('records the workspace source atomically, never truncating connection.json in place (#1429)', async () => { + const { deps, dataDir } = createUpHarness(); + const connectionPath = pathReal.join(dataDir, 'connection.json'); + const writeFileSpy = vi.spyOn(deps.fs, 'writeFileSync'); + const renameSpy = vi.spyOn(deps.fs, 'renameSync'); + + await runUpCommand({}, deps); + + // A concurrent writer to connection.json (e.g. the broker updating its own + // port/pid) must never be clobbered by a direct, non-atomic overwrite here. + const directWrites = writeFileSpy.mock.calls.filter(([target]) => target === connectionPath); + expect(directWrites).toHaveLength(0); + + const renameToConnection = renameSpy.mock.calls.find(([, dest]) => dest === connectionPath); + expect(renameToConnection).toBeDefined(); + const [tmpPath] = renameToConnection ?? []; + expect(String(tmpPath)).not.toBe(connectionPath); + expect(writeFileSpy.mock.calls.some(([target]) => target === tmpPath)).toBe(true); + expect(readBindingSource(dataDir)).toBe('created'); + }); + it('keeps an explicit --workspace-key ahead of both stores', async () => { const { deps, dataDir, home, log } = createUpHarness(); setWorkspaceKey('global', 'rk_global', { AGENT_RELAY_HOME: home }); @@ -572,6 +594,22 @@ describe('runUpCommand workspace precedence', () => { expect(readBindingSource(dataDir)).toBe('flag'); }); + it('attributes provenance to the multi-workspace session, not a single key, when RELAY_WORKSPACES_JSON is set (#1429)', async () => { + const { deps, dataDir, home, log } = createUpHarness(); + // Every one of these would normally win the single-key ladder, but the + // broker's startup_session_set_with_options() checks RELAY_WORKSPACES_JSON + // before any of them, so none is what the broker actually joins. + setWorkspaceKey('global', 'rk_global', { AGENT_RELAY_HOME: home }); + writeRepositoryPin(dataDir, { workspaceKey: 'rk_repository' }); + deps.env.RELAY_WORKSPACES_JSON = '[{"workspace_id":"rw_a","api_key":"rk_a"}]'; + + await runUpCommand({ workspaceKey: 'rk_flag' }, deps); + + expect(log.mock.calls.flat().join('\n')).toContain('Workspace source: multi-workspace session'); + expect(log.mock.calls.flat().join('\n')).toContain('Workspace: joined'); + expect(readBindingSource(dataDir)).toBe('multi-workspace'); + }); + it('records environment provenance after normalizing a workspace-key alias', async () => { const { deps, dataDir, log } = createUpHarness(); deps.env.AGENT_RELAY_WORKSPACE_KEY = ' rk_environment '; diff --git a/packages/cli/src/cli/lib/broker-lifecycle.ts b/packages/cli/src/cli/lib/broker-lifecycle.ts index d8cebbc6f..e015f5d4b 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; import { HarnessDriverClient } from '@agent-relay/harness-driver'; @@ -83,7 +84,7 @@ const NODE_DELIVERY_READY_TIMEOUT_MS = 10_000; // RELAY_NODE_TOKEN. const NODE_TOKEN_WAIT_MS = 15_000; -export type WorkspaceBindingSource = WorkspaceSelection['source'] | 'created'; +export type WorkspaceBindingSource = WorkspaceSelection['source'] | 'created' | 'multi-workspace'; export interface BrokerConnection { url: string; @@ -732,7 +733,8 @@ function workspaceBindingSource(value: string | undefined): WorkspaceBindingSour value === 'env' || value === 'project' || value === 'store' || - value === 'created' + value === 'created' || + value === 'multi-workspace' ? value : undefined; } @@ -749,9 +751,20 @@ function workspaceBindingSourceLabel(source: WorkspaceBindingSource): string { return 'machine-global active workspace (~/.agentworkforce/relay/workspaces.json)'; case 'created': return 'created (no configured workspace resolved)'; + case 'multi-workspace': + return 'multi-workspace session ($RELAY_WORKSPACES_JSON)'; } } +/** True when `RELAY_WORKSPACES_JSON` carries at least one membership. The broker's + * `startup_session_set_with_options` checks this env var before any single + * workspace key (flag, env, repository pin, or machine-global store), so the + * CLI's precedence ladder must defer to it for provenance too — otherwise + * `node up` / `node status` can report a source the broker never used. */ +function usesMultiWorkspaceEnv(env: NodeJS.ProcessEnv): boolean { + return Boolean(env.RELAY_WORKSPACES_JSON?.trim()); +} + function writeBrokerBindingSource( dataDir: string, source: WorkspaceBindingSource, @@ -760,11 +773,17 @@ function writeBrokerBindingSource( const connectionPath = path.join(dataDir, CONNECTION_FILENAME); const connection = readBrokerConnectionFromFs(deps.fs, dataDir); if (!connection) return; + // Every CLI invocation resolves the broker through this file, so a + // concurrent writer must never observe a partial or clobbered write. + // Write to a private tmp file and rename it into place, which is atomic + // on the same filesystem. + const tmpPath = `${connectionPath}.tmp-${process.pid}-${randomUUID()}`; deps.fs.writeFileSync( - connectionPath, + tmpPath, `${JSON.stringify({ ...connection, workspace_source: source }, null, 2)}\n`, 'utf-8' ); + deps.fs.renameSync(tmpPath, connectionPath); } function backgroundStartErrorPath(dataDir: string): string { @@ -1436,11 +1455,13 @@ function describeWorkspaceSource(source: WorkspaceSelection['source']): string { */ function recordWorkspaceBindingSource( selection: WorkspaceSelection | undefined, - deps: CoreDependencies + deps: CoreDependencies, + overrideSource?: WorkspaceBindingSource ): WorkspaceBindingSource { const inheritedSource = workspaceBindingSource(deps.env[WORKSPACE_BINDING_SOURCE_ENV]); const source: WorkspaceBindingSource = - selection?.source === 'env' && inheritedSource ? inheritedSource : (selection?.source ?? 'created'); + overrideSource ?? + (selection?.source === 'env' && inheritedSource ? inheritedSource : (selection?.source ?? 'created')); deps.env[WORKSPACE_BINDING_SOURCE_ENV] = source; return source; } @@ -1455,14 +1476,29 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): // --state-dir), so the key must be persisted here even when broker state is // redirected elsewhere. const projectWorkspaceKeyDataDir = paths.dataDir; - const workspaceSelection = resolveWorkspaceSelection({ - workspaceKey: options.workspaceKey, - env: deps.env, - projectDataDir: projectWorkspaceKeyDataDir, - fileSystem: deps.fs, - }); - const workspaceBindingSource = recordWorkspaceBindingSource(workspaceSelection, deps); - const resumedProjectSession = applyWorkspaceSelection(workspaceSelection, deps, projectWorkspaceKeyDataDir); + // The broker's startup_session_set_with_options() checks RELAY_WORKSPACES_JSON + // before any single workspace key, so a flag/env/pin/store resolution here + // would report provenance the broker never actually used. + const joinsMultiWorkspaceSession = usesMultiWorkspaceEnv(deps.env); + const workspaceSelection = joinsMultiWorkspaceSession + ? undefined + : resolveWorkspaceSelection({ + workspaceKey: options.workspaceKey, + env: deps.env, + projectDataDir: projectWorkspaceKeyDataDir, + fileSystem: deps.fs, + }); + const workspaceBindingSource = recordWorkspaceBindingSource( + workspaceSelection, + deps, + joinsMultiWorkspaceSession ? 'multi-workspace' : undefined + ); + const resumedProjectSession = joinsMultiWorkspaceSession + ? undefined + : applyWorkspaceSelection(workspaceSelection, deps, projectWorkspaceKeyDataDir); + if (joinsMultiWorkspaceSession) { + deps.log(`Workspace source: ${workspaceBindingSourceLabel('multi-workspace')}`); + } // --state-dir overrides where the broker writes state / connection files if (options.stateDir) { const resolved = path.resolve(options.stateDir); @@ -1711,7 +1747,9 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): // Minting must be observable: without this line "created a workspace" and // "joined the pinned workspace" print identically. const joinedWorkspaceId = relay.workspaceId ?? 'unknown'; - if (workspaceSelection) { + // The multi-workspace session always joins a configured membership; it + // never mints a new workspace the way an unresolved single key does. + if (workspaceSelection || joinsMultiWorkspaceSession) { deps.log(`Workspace: joined ${joinedWorkspaceId}`); } else { deps.log(`Workspace: created new workspace ${joinedWorkspaceId}`); diff --git a/tests/integration/broker/cli-spawn.test.ts b/tests/integration/broker/cli-spawn.test.ts index f0b500960..f1977a146 100644 --- a/tests/integration/broker/cli-spawn.test.ts +++ b/tests/integration/broker/cli-spawn.test.ts @@ -493,9 +493,15 @@ test( const missingCli = `agent-relay-missing-${suffix}`; try { + // The wrapper exits before its CLI ever runs, so two rejection paths + // race: the stability-window check ("process exited during startup") + // and, if the wrapper's stdin closes before the broker writes the + // init_worker frame, an EPIPE from send_to_worker ("failed writing + // frame to worker"). Both are the correct rejection for this case, so + // accept either instead of pinning to whichever wins the race. await assert.rejects( () => harness.spawnAgent(agentName, missingCli, ['general']), - /process exited during startup/, + /process exited during startup|failed writing frame to worker/, 'a wrapper that cannot launch its CLI must reject the spawn request' ); From ae9a02a1e1c3285e6042dc5d91442f660a850976 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 6 Aug 2026 10:38:10 +0200 Subject: [PATCH 6/7] docs(changelog): split workspace restore/rebind into separate bullets Address CodeRabbit review feedback on the rebase merge: the two commands have distinct user-visible effects and belong in their own bullets. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e66fd1b5a..efcc5c636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `agent-relay cloud login --device` logs in a machine with no browser through the OAuth device flow: the CLI prints a code you approve from any other device. Login and re-authentication fall back to it automatically over SSH or on a Unix host with no display server, and each machine gets its own cloud session instead of a copied `cloud-auth.json`. Requires cloud with the device authorization endpoints. -- `agent-relay workspace restore` returns to the recorded previous workspace, while `workspace rebind ` explicitly pins a project's next broker start without changing the machine-global active workspace. +- `agent-relay workspace restore` returns to the recorded previous workspace. +- `agent-relay workspace rebind ` pins a project's next broker start to a named workspace without changing the machine-global active workspace. ### Changed From 97c65cdadf74fc5c3feb1463a1dc53bfa0fe01e6 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 6 Aug 2026 11:29:43 +0200 Subject: [PATCH 7/7] fix(broker): clean up rejected spawns fully, cover create's warning-suppression branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - confirm_worker_process_alive's rejection was treated as one case, but try_wait() can fail two different ways: a confirmed exit, or a genuine I/O error where the process may still be alive. Both dropped the WorkerHandle without killing the child — dropping a tokio::process::Child does not kill the OS process, so the I/O-error case could silently orphan a live, unsupervised worker. Extracted a shared cleanup_rejected_spawn that terminates+reaps the child before removing it from the registry and restart supervisor. - The same cleanup was also skipped entirely when send_to_worker("init_worker") itself failed (e.g. EPIPE if the wrapper's stdin closed before the broker's first write): that `?` returned before the cleanup block ever ran, leaving a stale entry `node agent list` could advertise. Wired that path through the same cleanup_rejected_spawn. - Added packages/cli/src/cli/commands/workspace.test.ts coverage for the untested previousActive === workspaceName suppression branch: re-creating the currently active workspace must persist without the "Active workspace changed" warning. Proven RED by relaxing the guard to just `previousActive` and confirming this test catches it (fails with 2 unexpected deps.error calls), then restored. Both worker.rs regression tests proven RED against the pre-fix code: - cleanup_rejected_spawn_terminates_a_still_alive_child_and_removes_it fails with "cleanup_rejected_spawn must terminate the child, not just drop the handle" when termination is removed from cleanup_rejected_spawn. - init_worker_send_failure_cleans_up_like_a_startup_rejection exercises a real EPIPE (writes to a ChildStdin after the child has already exited). cargo test -p agent-relay-broker --lib: 855 passed, 0 failed, 4 ignored. cargo fmt --check / cargo clippy -D warnings: clean (3 pre-existing errors in unrelated files, unchanged by this PR). npx vitest run packages/cli: 957 passed, 20 skipped. npx tsc --noEmit: clean. Co-Authored-By: Claude Sonnet 5 --- crates/broker/src/worker.rs | 187 ++++++++++++++++-- .../cli/src/cli/commands/workspace.test.ts | 19 ++ 2 files changed, 189 insertions(+), 17 deletions(-) diff --git a/crates/broker/src/worker.rs b/crates/broker/src/worker.rs index bfa0c114c..bf8710898 100644 --- a/crates/broker/src/worker.rs +++ b/crates/broker/src/worker.rs @@ -383,6 +383,21 @@ impl WorkerRegistry { self.workers.get(name).and_then(|h| h.harness_pid) } + /// Clean up a worker whose spawn was rejected after the handle was + /// already inserted into `self.workers` — whether `init_worker` failed to + /// send (e.g. the wrapper's stdin closed before the broker could write to + /// it, EPIPE) or the post-spawn stability check rejected it. Shared so + /// every rejection path leaves the registry, restart supervisor, and + /// child process in the same clean state. + async fn cleanup_rejected_spawn(&mut self, name: &WorkerName) { + if let Some(handle) = self.workers.get_mut(name) { + let _ = terminate_child(&mut handle.child, ORPHAN_REAP_TIMEOUT).await; + } + self.workers.remove(name); + self.initial_tasks.remove(name); + self.supervisor.unregister(name); + } + #[allow(clippy::too_many_arguments)] pub(crate) async fn spawn( &mut self, @@ -979,15 +994,26 @@ impl WorkerRegistry { }; self.workers.insert(spec.name.clone(), handle); - self.send_to_worker( - &spec.name, - "init_worker", - None, - json!({ - "agent": spec, - }), - ) - .await?; + if let Err(error) = self + .send_to_worker( + &spec.name, + "init_worker", + None, + json!({ + "agent": spec, + }), + ) + .await + { + // The wrapper can exit before the broker's first write reaches it + // (its stdin closes, and `send_to_worker` fails with EPIPE before + // the stability-window check below ever runs). Without this, that + // race left a stale entry in `self.workers` that `node agent + // list` could briefly advertise, exactly like a startup-check + // rejection — so it gets the identical cleanup. + self.cleanup_rejected_spawn(&spec.name).await; + return Err(error); + } let startup_confirmation = { let handle = self @@ -1003,14 +1029,15 @@ impl WorkerRegistry { .await }; if let Err(error) = startup_confirmation { - // `try_wait` reaped an exited wrapper. Remove the stale registry - // entry before returning the error so `node agent list` cannot - // briefly advertise a process the spawn call just rejected. Also - // unregister from the restart supervisor so a rejected spawn can - // never generate a pending restart for a worker that never launched. - self.workers.remove(&spec.name); - self.initial_tasks.remove(&spec.name); - self.supervisor.unregister(&spec.name); + // `confirm_worker_process_alive` rejects here for two different + // reasons: `try_wait` confirmed the wrapper exited, or `try_wait` + // itself returned an I/O error and we don't actually know the + // process is dead. Either way, terminate and reap it before + // dropping the handle — the confirmed-exit case is a no-op kill, + // but the I/O-error case would otherwise silently orphan a still + // -live, unsupervised process. The original verification error is + // preserved and returned either way. + self.cleanup_rejected_spawn(&spec.name).await; return Err(error); } @@ -2106,6 +2133,132 @@ mod tests { .unwrap(); } + #[cfg(unix)] + fn spec_for_test(name: &str) -> AgentSpec { + AgentSpec { + name: WorkerName::from(name), + runtime: AgentRuntime::Headless, + provider: None, + cli: None, + session_id: None, + harness_config: None, + model: None, + cwd: None, + team: None, + shadow_of: None, + shadow_mode: None, + args: Vec::new(), + channels: Vec::new(), + restart_policy: None, + } + } + + #[cfg(unix)] + fn is_process_alive(pid: u32) -> bool { + use nix::{sys::signal::kill, unistd::Pid}; + // `kill(pid, None)` is the POSIX liveness probe: it signals nothing, + // it only reports whether the pid still exists and is ours to signal. + kill(Pid::from_raw(pid as i32), None).is_ok() + } + + #[cfg(unix)] + #[tokio::test] + async fn cleanup_rejected_spawn_terminates_a_still_alive_child_and_removes_it() { + // Regression test: a rejected spawn used to remove the registry entry + // (and, before that fix, sometimes not even run cleanup — see the + // EPIPE-race test below) without ever touching the child process + // itself. Dropping a `tokio::process::Child` does not kill the OS + // process, so a spawn rejected while the wrapper was still alive + // orphaned it. `cleanup_rejected_spawn` must kill and reap it. + let mut reg = make_registry(vec![]); + let name = "cleanup-orphan-candidate"; + let mut child = Command::new("sleep") + .arg("30") + .stdin(Stdio::piped()) + .spawn() + .unwrap(); + let pid = child.id().expect("child has a pid"); + let stdin = child.stdin.take().expect("piped stdin"); + assert!( + is_process_alive(pid), + "precondition: child must start alive" + ); + + reg.workers.insert( + WorkerName::from(name), + WorkerHandle { + spec: spec_for_test(name), + parent: None, + workspace_id: None, + child, + stdin, + harness_pid: None, + spawned_at: Instant::now(), + ready_at: None, + last_activity_at: Instant::now(), + context_budget_pct: None, + state: AgentWorkState::Working, + exit_reason: None, + }, + ); + + reg.cleanup_rejected_spawn(&WorkerName::from(name)).await; + + assert!(!reg.workers.contains_key(&WorkerName::from(name))); + assert!( + !is_process_alive(pid), + "cleanup_rejected_spawn must terminate the child, not just drop the handle" + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn init_worker_send_failure_cleans_up_like_a_startup_rejection() { + // Regression test for the EPIPE race: if the wrapper exits before the + // broker's first write reaches it, `send_to_worker("init_worker")` + // fails before the stability-window check ever runs. Before this fix + // that early `?` skipped cleanup entirely, leaving a stale entry + // `node agent list` could advertise. Trigger a real write failure — + // once a child exits, its stdin's read end closes, so writing to our + // held `ChildStdin` fails — rather than asserting on message text. + let mut reg = make_registry(vec![]); + let name = "epipe-candidate"; + let mut child = Command::new("true").stdin(Stdio::piped()).spawn().unwrap(); + let stdin = child.stdin.take().expect("piped stdin"); + child.wait().await.expect("child exits immediately"); + + reg.workers.insert( + WorkerName::from(name), + WorkerHandle { + spec: spec_for_test(name), + parent: None, + workspace_id: None, + child, + stdin, + harness_pid: None, + spawned_at: Instant::now(), + ready_at: None, + last_activity_at: Instant::now(), + context_budget_pct: None, + state: AgentWorkState::Working, + exit_reason: None, + }, + ); + + let send_result = reg + .send_to_worker(name, "init_worker", None, json!({})) + .await; + assert!( + send_result.is_err(), + "writing to a worker whose process already exited must fail, proving the race is real" + ); + + // This mirrors exactly what `spawn()` now does on this error path. + reg.cleanup_rejected_spawn(&WorkerName::from(name)).await; + + assert!(!reg.workers.contains_key(&WorkerName::from(name))); + } + // The wrapper process can outlive the harness it hosts, so reaping on the // wrapper alone leaves a dead agent listed as `working` forever. mod orphaned_worker { diff --git a/packages/cli/src/cli/commands/workspace.test.ts b/packages/cli/src/cli/commands/workspace.test.ts index 1ff175af9..ddad0b885 100644 --- a/packages/cli/src/cli/commands/workspace.test.ts +++ b/packages/cli/src/cli/commands/workspace.test.ts @@ -191,6 +191,25 @@ describe('registerWorkspaceCommands', () => { expect(() => JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0]))).not.toThrow(); }); + it('workspace create suppresses the active-workspace warning when re-creating the already-active workspace', async () => { + vi.mocked(readWorkspaceStore).mockReturnValueOnce({ + active: 'session-two', + workspaces: { 'session-two': { key: 'rk_live_old' } }, + }); + const { program, deps } = createHarness(); + vi.mocked(deps.createWorkspace).mockResolvedValueOnce({ + workspaceKey: 'rk_live_session_two', + } as never); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'create', 'session-two']); + + expect(persistWorkspaceSession).toHaveBeenCalledWith({ + name: 'session-two', + workspaceKey: 'rk_live_session_two', + }); + expect(deps.error).not.toHaveBeenCalled(); + }); + it('workspace create keeps stdout parseable and routes the warning away from it', async () => { vi.mocked(readWorkspaceStore).mockReturnValueOnce({ active: 'default',