diff --git a/CHANGELOG.md b/CHANGELOG.md index 9848740c7..07772ec99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. - `agent-relay workspace rebind ` pins a project's next broker start to a named workspace without changing the machine-global active workspace. +- `agent-relay workspace active` reports whether Relaycast, Relayfile, and RelayAuth resolve one data-plane workspace id: a `dataPlane` block in `--json`, the previously omitted Relaycast id in human output, and `--require-unified` to exit non-zero on a divergence. ### Changed diff --git a/packages/cli/src/cli/commands/workspace.test.ts b/packages/cli/src/cli/commands/workspace.test.ts index ddad0b885..9311566e9 100644 --- a/packages/cli/src/cli/commands/workspace.test.ts +++ b/packages/cli/src/cli/commands/workspace.test.ts @@ -1,12 +1,20 @@ import { Command } from 'commander'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -vi.mock('@agent-relay/cloud', () => ({ - readWorkspaceStore: vi.fn(() => ({ workspaces: {} })), - resolveActiveWorkspace: vi.fn(), - setWorkspaceKey: vi.fn(), - switchWorkspace: vi.fn(), -})); +vi.mock('@agent-relay/cloud', async (importOriginal) => { + const actual = await importOriginal(); + return { + readWorkspaceStore: vi.fn(() => ({ workspaces: {} })), + resolveActiveWorkspace: vi.fn(), + setWorkspaceKey: vi.fn(), + switchWorkspace: vi.fn(), + // The real convergence helpers, not stand-ins: these tests assert on what + // the command reports about the AR-448 data-plane invariant, so a copy here + // would let the real check drift past them. + describeDataPlaneConvergence: actual.describeDataPlaneConvergence, + formatDataPlaneDivergence: actual.formatDataPlaneDivergence, + }; +}); vi.mock('../lib/workspace-session.js', async (importOriginal) => ({ // Returns a result object describing what the write changed beyond the key. @@ -106,6 +114,13 @@ describe('registerWorkspaceCommands', () => { slug: 'ops', urls: {}, apiUrl: 'https://cloud.test', + // This fixture's planes genuinely disagree (rc_ops vs rw_ops), so the + // evidence block reports the split rather than a shared identity. + dataPlane: { + unified: false, + planes: { relaycast: 'rc_ops', relayfile: 'rw_ops', relayauth: 'rw_ops' }, + divergent: ['relayfile', 'relayauth'], + }, }); }); @@ -151,6 +166,100 @@ describe('registerWorkspaceCommands', () => { expect(printed.relaycastApiKey).toBe('rk_live_…ey01'); }); + it('workspace active --json proves one data-plane workspace ID when the planes agree', async () => { + const { program, deps } = createHarness(); + vi.mocked(resolveActiveWorkspace).mockResolvedValueOnce({ + name: 'Ops', + key: 'rk_live_ops', + // The cloud ID is a UUID in a different id space and must not count + // against convergence. + cloudWorkspaceId: '50587328-441d-4acb-b8f3-dbe1b3c5de99', + relaycastWorkspaceId: 'rw_7ccfea89', + relayfileWorkspaceId: 'rw_7ccfea89', + relayauthWorkspaceId: 'rw_7ccfea89', + urls: {}, + apiUrl: 'https://cloud.test', + }); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'active', '--json']); + + const printed = JSON.parse(String(vi.mocked(deps.log).mock.calls[0][0])); + expect(printed.dataPlane).toEqual({ + unified: true, + workspaceId: 'rw_7ccfea89', + planes: { relaycast: 'rw_7ccfea89', relayfile: 'rw_7ccfea89', relayauth: 'rw_7ccfea89' }, + divergent: [], + }); + expect(deps.error).not.toHaveBeenCalled(); + expect(deps.exit).not.toHaveBeenCalled(); + }); + + it('workspace active reports the Relaycast ID and the unified data plane in human output', async () => { + const { program, deps } = createHarness(); + vi.mocked(resolveActiveWorkspace).mockResolvedValueOnce({ + name: 'Ops', + key: 'rk_live_ops', + cloudWorkspaceId: '50587328-441d-4acb-b8f3-dbe1b3c5de99', + relaycastWorkspaceId: 'rw_7ccfea89', + relayfileWorkspaceId: 'rw_7ccfea89', + relayauthWorkspaceId: 'rw_7ccfea89', + urls: {}, + apiUrl: 'https://cloud.test', + }); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'active']); + + const output = vi.mocked(deps.log).mock.calls.flat().map(String).join('\n'); + // The Relaycast ID is the durable delivery identity and was previously the + // one plane the human output omitted. + expect(output).toContain('Relaycast workspace ID: rw_7ccfea89'); + expect(output).toContain('Data-plane workspace ID: rw_7ccfea89 (unified)'); + // Human output is not a place for credentials. + expect(output).not.toContain('rk_live_ops'); + }); + + it('workspace active warns on a divergence but still exits 0 without --require-unified', async () => { + const { program, deps } = createHarness(); + vi.mocked(resolveActiveWorkspace).mockResolvedValueOnce({ + name: 'Ops', + key: 'rk_live_ops', + cloudWorkspaceId: 'cloud-uuid', + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_a', + urls: {}, + apiUrl: 'https://cloud.test', + }); + + await program.parseAsync(['node', 'agent-relay', 'workspace', 'active']); + + expect(vi.mocked(deps.error).mock.calls.flat().map(String).join('\n')).toContain('relayfile=rw_b'); + // Existing scripted callers keep their exit code; only the explicit gate + // below changes it. + expect(deps.exit).not.toHaveBeenCalled(); + }); + + it('workspace active --require-unified exits non-zero on a divergence', async () => { + const { program, deps } = createHarness(); + vi.mocked(resolveActiveWorkspace).mockResolvedValueOnce({ + name: 'Ops', + key: 'rk_live_ops', + cloudWorkspaceId: 'cloud-uuid', + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_c', + urls: {}, + apiUrl: 'https://cloud.test', + }); + + // The harness `exit` throws, which is how a real exit aborts the action. + await expect( + program.parseAsync(['node', 'agent-relay', 'workspace', 'active', '--require-unified']) + ).rejects.toThrow('exit:1'); + + expect(deps.exit).toHaveBeenCalledWith(1); + }); + it('workspace create starts and persists a new workspace session', async () => { const { program, deps } = createHarness(); vi.mocked(deps.createWorkspace).mockResolvedValueOnce({ diff --git a/packages/cli/src/cli/commands/workspace.ts b/packages/cli/src/cli/commands/workspace.ts index 659e61d02..c5446317e 100644 --- a/packages/cli/src/cli/commands/workspace.ts +++ b/packages/cli/src/cli/commands/workspace.ts @@ -1,6 +1,10 @@ import type { Command } from 'commander'; import { InvalidArgumentError } from 'commander'; -import { resolveActiveWorkspace } from '@agent-relay/cloud'; +import { + describeDataPlaneConvergence, + formatDataPlaneDivergence, + resolveActiveWorkspace, +} from '@agent-relay/cloud'; import { maskSecret } from '../lib/redact.js'; import { printJson, runSdk, withSdkDefaults, type SdkCommandDeps } from '../lib/sdk-command.js'; @@ -50,6 +54,7 @@ export function registerWorkspaceCommands( .option('--api-url ', 'Cloud API base URL') .option('--json', 'Output the active workspace as JSON (keys masked unless --reveal-secrets)') .option('--reveal-secrets', 'Include raw workspace keys in --json output') + .option('--require-unified', 'Exit non-zero when the data planes disagree on the workspace identity') .option( '--refresh-timeout ', 'Timeout for refreshing the cloud session', @@ -60,6 +65,7 @@ export function registerWorkspaceCommands( apiUrl?: string; json?: boolean; revealSecrets?: boolean; + requireUnified?: boolean; refreshTimeout?: number; }) => { await runSdk(deps, async () => { @@ -68,11 +74,14 @@ export function registerWorkspaceCommands( interactive: false, refreshTimeoutMs: options.refreshTimeout, }); + // Emitted on every call so the output is self-sufficient evidence of + // the AR-448 invariant rather than something a caller has to + // recompute from the three plane IDs. + const dataPlane = describeDataPlaneConvergence(workspace); if (options.json) { - printJson( - deps, - options.revealSecrets + printJson(deps, { + ...(options.revealSecrets ? workspace : { ...workspace, @@ -80,15 +89,31 @@ export function registerWorkspaceCommands( ...(workspace.relaycastApiKey ? { relaycastApiKey: maskSecret(workspace.relaycastApiKey) } : {}), - } + }), + dataPlane, + }); + } else { + deps.log(`Workspace: ${workspace.name ?? workspace.cloudWorkspaceId}`); + deps.log(`Cloud workspace ID: ${workspace.cloudWorkspaceId}`); + deps.log(`Relaycast workspace ID: ${workspace.relaycastWorkspaceId}`); + deps.log(`Relayfile workspace ID: ${workspace.relayfileWorkspaceId}`); + deps.log(`Relayauth workspace ID: ${workspace.relayauthWorkspaceId}`); + deps.log( + dataPlane.unified + ? `Data-plane workspace ID: ${dataPlane.workspaceId} (unified)` + : `Data-plane workspace ID: divergent (${dataPlane.divergent.join(', ')})` ); - return; } - deps.log(`Workspace: ${workspace.name ?? workspace.cloudWorkspaceId}`); - deps.log(`Cloud workspace ID: ${workspace.cloudWorkspaceId}`); - deps.log(`Relayfile workspace ID: ${workspace.relayfileWorkspaceId}`); - deps.log(`Relayauth workspace ID: ${workspace.relayauthWorkspaceId}`); + if (!dataPlane.unified) { + // A divergence is reported on stderr either way; only + // --require-unified turns it into a gate, so existing scripted + // callers keep their exit code. + deps.error(formatDataPlaneDivergence(dataPlane)); + if (options.requireUnified) { + deps.exit(1); + } + } }); } ); diff --git a/packages/cli/src/cli/lib/workspace-identity-restart.test.ts b/packages/cli/src/cli/lib/workspace-identity-restart.test.ts new file mode 100644 index 000000000..88d94d8cc --- /dev/null +++ b/packages/cli/src/cli/lib/workspace-identity-restart.test.ts @@ -0,0 +1,412 @@ +/** + * AR-448 regression: a node's workspace — and therefore its resident agent's + * address and mailbox — must survive a full stop/start. + * + * The failure this guards against is quiet. A node started with no repository + * pin used to fall through to the broker, which mints a brand-new + * messaging-only workspace. Everything still "works": the broker comes up, the + * agent registers, `node status` looks healthy — but the agent is a stranger in + * a different workspace with a new address, so DMs sent to its previous address + * go nowhere. + * + * `broker-lifecycle.test.ts` covers the precedence ladder one start at a time. + * This file covers what only shows up ACROSS starts: that a second start lands + * on the same workspace as the first, that the resident keeps its address, and + * that a checkout which never pinned drifts. + * + * ## What is (and is not) exercised + * + * These are unit tests over the CLI's own TypeScript — `runUpCommand` and the + * shared `resolveWorkspaceSelection` ladder. `createRelay` is a stand-in, so no + * broker binary, released or otherwise, participates. What the stand-in models + * is the two pieces of real behavior that decide whether identity is durable: + * + * - the broker joins `RELAY_WORKSPACE_KEY` when one is set and otherwise + * mints a fresh workspace (`startup_single_session_set_from_sources`, + * crates/broker/src/relaycast/auth.rs); + * - registration is a fail-closed admission gate (`admit_agent_registration`, + * same file). Re-registering a name in a workspace it already belongs to + * returns the EXISTING agent — same id, same address, same inbox — only + * when the caller proves it is the same work unit. The broker's own startup + * registration proves it with `stable_node_identity_key`, derived from the + * persisted state directory, which hashes identically across a kill and + * restart of the same node and differently for any other node. + * + * That second point post-dates the original AR-448 work (commit 5c2ad8ee3), + * and it narrows what durability means: a restart of the SAME node reclaims its + * name, while a DIFFERENT node claiming that name in the same workspace is + * rejected rather than handed the incumbent's credentials. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../telemetry/index.js', () => ({ track: vi.fn() })); +vi.mock('./reflex-capture.js', () => ({ + startReflexCapture: vi.fn(() => ({ stop: vi.fn(async () => undefined) })), +})); +vi.mock('@agent-relay/fleet', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + startServeNode: vi.fn(() => ({ stop: vi.fn(async () => undefined), done: Promise.resolve() })), + }; +}); +vi.mock('@agent-relay/harness-driver', () => ({ + HarnessDriverClient: class { + async getSession() { + return { + node_id: 'node_a', + node_name: 'the-node', + broker_version: 'test', + protocol_version: 2, + }; + } + async getStatus() { + return {}; + } + disconnect() {} + }, +})); + +import fsReal from 'node:fs'; +import os from 'node:os'; +import pathReal from 'node:path'; + +import { runUpCommand } from './broker-lifecycle.js'; +import { readProjectWorkspaceSession } from './project-workspace-key.js'; +import type { CoreDependencies } from '../commands/core.js'; + +/** The resident agent whose address has to survive the restart. */ +const RESIDENT_AGENT = 'khaliq-chief'; +/** A workspace already selected machine-wide by `agent-relay workspace switch`. */ +const CANONICAL_KEY = 'rk_live_canonical0001'; + +const tmpRoots: string[] = []; + +function mkTmp(prefix: string): string { + const dir = fsReal.mkdtempSync(pathReal.join(os.tmpdir(), prefix)); + tmpRoots.push(dir); + return dir; +} + +/** A registration that the admission gate refused. */ +class AdmissionRejected extends Error { + readonly code = 'agent_identity_mismatch'; +} + +/** + * A stand-in for Relaycast shared by every checkout in a test, the way the real + * cloud is shared by every node on a machine. + * + * Workspace keys it mints are globally unique, and each maps to one stable + * workspace id. Registration mirrors `admit_agent_registration`: a free name is + * granted, a name already held is reclaimed only on a matching identity proof, + * and a mismatched or absent proof is rejected outright. + */ +const relaycast = (() => { + const workspaceIds = new Map(); + const agents = new Map(); + let mintedWorkspaces = 0; + let mintedAgents = 0; + + return { + mintWorkspaceKey(): string { + const key = `rk_live_minted${(mintedWorkspaces += 1)}`; + workspaceIds.set(key, `rw_minted${mintedWorkspaces}`); + return key; + }, + workspaceIdFor(workspaceKey: string): string { + const existing = workspaceIds.get(workspaceKey); + if (existing) return existing; + // A key the broker joined rather than minted (canonical, pinned, or + // explicit) still addresses exactly one workspace. + const id = `rw_joined${workspaceIds.size + 1}`; + workspaceIds.set(workspaceKey, id); + return id; + }, + /** @throws AdmissionRejected when the name is held by another work unit. */ + register(workspaceKey: string, agentName: string, identity: string): string { + const slot = `${workspaceKey}::${agentName}`; + const existing = agents.get(slot); + if (existing) { + if (existing.identity !== identity) { + throw new AdmissionRejected( + `agent name '${agentName}' is already registered and this registration did not prove ownership` + ); + } + return existing.address; + } + const address = `agent_${(mintedAgents += 1)}@${workspaceKey}`; + agents.set(slot, { address, identity }); + return address; + }, + reset(): void { + workspaceIds.clear(); + agents.clear(); + mintedWorkspaces = 0; + mintedAgents = 0; + }, + }; +})(); + +afterEach(() => { + relaycast.reset(); + for (const dir of tmpRoots.splice(0)) { + fsReal.rmSync(dir, { recursive: true, force: true }); + } +}); + +interface StartResult { + workspaceKey: string; + workspaceId: string; + /** The resident's delivery address, or `undefined` when admission rejected it. */ + residentAddress?: string; + rejected?: string; + log: string[]; +} + +/** + * One machine across restarts: a stable project checkout and a stable + * `AGENT_RELAY_HOME`. Each `start()` builds a fresh env and a fresh dependency + * set over that same persistent on-disk state, which is exactly what a + * stop/start looks like from the CLI's point of view — stopping the node is + * process exit, and nothing in the CLI's own state is carried over. + */ +function createMachine(options: { canonicalWorkspaceKey?: string } = {}) { + const projectRoot = mkTmp('ar448-project-'); + const relayHome = mkTmp('ar448-home-'); + const dataDir = pathReal.join(projectRoot, '.agentworkforce', 'relay'); + fsReal.mkdirSync(dataDir, { recursive: true }); + + if (options.canonicalWorkspaceKey) { + fsReal.writeFileSync( + pathReal.join(relayHome, 'workspaces.json'), + JSON.stringify({ + active: 'default', + workspaces: { default: { key: options.canonicalWorkspaceKey } }, + }) + ); + } + + const connection = JSON.stringify({ + url: 'http://127.0.0.1:4999', + port: 4999, + api_key: 'test', + pid: 999999, + }); + + // The broker derives its own identity proof from its persisted state + // directory, so it is stable across this machine's restarts and unique to it. + const nodeIdentity = `node-${dataDir}`; + + async function start(options: { workspaceKey?: string } = {}): Promise { + const log: string[] = []; + // A fresh process: only what was persisted to disk crosses the restart. + const env: NodeJS.ProcessEnv = { + AGENT_RELAY_HOME: relayHome, + // A real resident node is Cloud-enrolled. The node token selects node + // identity, never a workspace, so it must not suppress the ladder. + RELAY_NODE_TOKEN: 'nt_live_test', + RELAY_BASE_URL: 'https://engine.test', + }; + + const deps = { + getProjectPaths: () => ({ projectRoot, dataDir, teamDir: projectRoot }), + loadTeamsConfig: () => null, + // Mirrors the broker: join the env-selected workspace, else mint one. + createRelay: vi.fn(async () => { + const workspaceKey = env.RELAY_WORKSPACE_KEY?.trim() || relaycast.mintWorkspaceKey(); + return { + spawn: vi.fn(async () => undefined), + getStatus: vi.fn(async () => ({})), + shutdown: vi.fn(async () => undefined), + workspaceKey, + workspaceId: relaycast.workspaceIdFor(workspaceKey), + }; + }), + spawnProcess: vi.fn(), + execCommand: vi.fn(async () => ({ stdout: '', stderr: '' })), + killProcess: vi.fn(() => { + throw new Error('not running'); + }), + fs: { + existsSync: fsReal.existsSync, + // The connection file is written by the broker; the stand-in writes + // nothing, so serve it from memory for any connection.json read. + 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, + rmSync: fsReal.rmSync, + accessSync: fsReal.accessSync, + }, + generateAgentName: () => RESIDENT_AGENT, + checkForUpdates: vi.fn(async () => ({ updateAvailable: false })), + getVersion: () => 'test', + env, + argv: ['node', 'agent-relay', 'node', 'up'], + execPath: process.execPath, + cliScript: 'cli.js', + pid: process.pid, + isPortInUse: vi.fn(async () => false), + now: () => 0, + sleep: async () => undefined, + onSignal: vi.fn(), + holdOpen: async () => undefined, + log: (...args: unknown[]) => log.push(args.map(String).join(' ')), + warn: vi.fn(), + error: vi.fn(), + exit: vi.fn(), + } as unknown as CoreDependencies; + + await runUpCommand(options.workspaceKey ? { workspaceKey: options.workspaceKey } : {}, deps); + + const relay = await vi.mocked(deps.createRelay).mock.results[0]!.value; + const workspaceKey = relay.workspaceKey as string; + const workspaceId = relay.workspaceId as string; + + // The resident registers into whatever workspace the broker joined. + try { + return { + workspaceKey, + workspaceId, + residentAddress: relaycast.register(workspaceKey, RESIDENT_AGENT, nodeIdentity), + log, + }; + } catch (err) { + if (!(err instanceof AdmissionRejected)) throw err; + return { workspaceKey, workspaceId, rejected: err.message, log }; + } + } + + return { start, dataDir, projectRoot }; +} + +describe('workspace identity across a node stop/start', () => { + it('keeps one workspace and one resident address across a restart', async () => { + const machine = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await machine.start(); + const second = await machine.start(); + + expect(first.workspaceKey).toBe(CANONICAL_KEY); + expect(second.workspaceKey).toBe(CANONICAL_KEY); + expect(second.workspaceId).toBe(first.workspaceId); + // The invariant AR-448 exists for: the address someone recorded before the + // restart still reaches the resident after it. + expect(second.residentAddress).toBe(first.residentAddress); + expect(second.rejected).toBeUndefined(); + }); + + it('resumes from the repository pin on the second start, not the machine-global store', async () => { + const machine = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await machine.start(); + const second = await machine.start(); + + // The first start joined the canonical workspace and pinned it; the second + // never has to consult the store at all. Pinning is what makes the identity + // survive someone later running `workspace switch` machine-wide. + expect(first.log.join('\n')).toContain('Workspace source: machine-global active workspace'); + expect(readProjectWorkspaceSession(machine.dataDir)?.workspaceKey).toBe(CANONICAL_KEY); + expect(second.log.join('\n')).toContain('Workspace source: repository pin'); + }); + + it('re-pins after an explicit --workspace-key so the next start resumes the new workspace', async () => { + const machine = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const before = await machine.start(); + const moved = await machine.start({ workspaceKey: 'rk_live_explicit0001' }); + const after = await machine.start(); + + // Documented migration path: an explicit key outranks both stores AND + // rewrites the pin, so the move is durable rather than one-shot. + expect(before.workspaceKey).toBe(CANONICAL_KEY); + expect(moved.workspaceKey).toBe('rk_live_explicit0001'); + expect(after.workspaceKey).toBe('rk_live_explicit0001'); + expect(after.log.join('\n')).toContain('Workspace source: repository pin'); + // Moving workspaces is exactly the operation that changes an address. + expect(after.residentAddress).not.toBe(before.residentAddress); + }); + + it('joins the canonical workspace rather than minting on a first start with no pin', async () => { + const machine = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await machine.start(); + + expect(first.workspaceKey).toBe(CANONICAL_KEY); + const output = first.log.join('\n'); + expect(output).toContain(`Workspace: joined ${first.workspaceId}`); + expect(output).not.toContain('created new workspace'); + }); + + it('drifts onto a new workspace per checkout when no canonical workspace is set', async () => { + // The pre-AR-448 behavior, kept as the negative control: with nothing to + // anchor identity to, the repository pin is the ONLY thing holding a node + // together, and a checkout that never pinned starts life somewhere else. + const machine = createMachine(); + + const first = await machine.start(); + const second = await machine.start(); + + expect(first.log.join('\n')).toContain('Workspace: created new workspace'); + // The pin written by the first start does hold THIS checkout steady... + expect(second.workspaceKey).toBe(first.workspaceKey); + expect(second.residentAddress).toBe(first.residentAddress); + + // ...but a second checkout, with no canonical workspace to join, is a + // different node in a different workspace entirely. + const elsewhere = await createMachine().start(); + expect(elsewhere.workspaceKey).not.toBe(first.workspaceKey); + expect(elsewhere.residentAddress).not.toBe(first.residentAddress); + }); + + it('lands a second checkout in the canonical workspace without anyone copying a key', async () => { + const original = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + const clone = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await original.start(); + const second = await clone.start(); + + // Workspace membership converges from the machine-global store alone. + expect(second.workspaceKey).toBe(first.workspaceKey); + expect(second.workspaceId).toBe(first.workspaceId); + }); + + it('refuses to hand the resident address to a different node claiming its name', async () => { + // Narrower than AR-448 originally assumed. Two checkouts converge on one + // WORKSPACE, but they are not one work unit: registration is a fail-closed + // admission gate, and only a proof derived from the same state directory + // reclaims a held name (crates/broker/src/relaycast/auth.rs, + // `admit_agent_registration` / `stable_node_identity_key`). Handing the + // incumbent's credentials to whoever asks second is the duplicate-agent + // failure that gate exists to stop. + const original = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + const clone = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await original.start(); + const second = await clone.start(); + + expect(second.workspaceKey).toBe(first.workspaceKey); + expect(second.residentAddress).toBeUndefined(); + expect(second.rejected).toContain('did not prove ownership'); + + // The incumbent is untouched: its own restart still reclaims its address. + const restarted = await original.start(); + expect(restarted.residentAddress).toBe(first.residentAddress); + }); + + it('never prints workspace key material on any start of the sequence', async () => { + const machine = createMachine({ canonicalWorkspaceKey: CANONICAL_KEY }); + + const first = await machine.start(); + const second = await machine.start(); + + const output = [...first.log, ...second.log].join('\n'); + expect(output).toContain('Workspace Key:'); + expect(output).not.toContain(CANONICAL_KEY); + }); +}); diff --git a/packages/cloud/src/index.ts b/packages/cloud/src/index.ts index 36d75bf6b..1c3d178cd 100644 --- a/packages/cloud/src/index.ts +++ b/packages/cloud/src/index.ts @@ -148,6 +148,13 @@ export { type WorkspaceSelection, } from './project-workspace-key.js'; +export { + describeDataPlaneConvergence, + formatDataPlaneDivergence, + type DataPlaneConvergence, + type DataPlaneWorkspaceIds, +} from './workspace-convergence.js'; + export { deployProactiveAgent, listProactiveAgents, diff --git a/packages/cloud/src/workspace-convergence.test.ts b/packages/cloud/src/workspace-convergence.test.ts new file mode 100644 index 000000000..2f1aff552 --- /dev/null +++ b/packages/cloud/src/workspace-convergence.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from 'vitest'; + +import { describeDataPlaneConvergence, formatDataPlaneDivergence } from './workspace-convergence.js'; + +describe('describeDataPlaneConvergence', () => { + it('reports one shared data-plane ID when all three planes agree', () => { + expect( + describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_7ccfea89', + relayfileWorkspaceId: 'rw_7ccfea89', + relayauthWorkspaceId: 'rw_7ccfea89', + }) + ).toEqual({ + unified: true, + workspaceId: 'rw_7ccfea89', + planes: { relaycast: 'rw_7ccfea89', relayfile: 'rw_7ccfea89', relayauth: 'rw_7ccfea89' }, + divergent: [], + }); + }); + + it('names the plane that disagrees and withholds a shared ID', () => { + const convergence = describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_a', + }); + + expect(convergence.unified).toBe(false); + // No single id may be offered as "the" workspace when the planes disagree: + // publishing one would paper over the split it exists to surface. + expect(convergence.workspaceId).toBeUndefined(); + expect(convergence.divergent).toEqual(['relayfile']); + }); + + it('names every plane that disagrees when all three differ', () => { + const convergence = describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_c', + }); + + expect(convergence.unified).toBe(false); + expect(convergence.divergent).toEqual(['relayfile', 'relayauth']); + }); + + it('keeps every plane ID on a divergence so the report is actionable', () => { + const convergence = describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_a', + }); + + expect(convergence.planes).toEqual({ + relaycast: 'rw_a', + relayfile: 'rw_b', + relayauth: 'rw_a', + }); + }); + + it('treats the cloud control-plane ID as irrelevant to data-plane convergence', () => { + // The cloud workspace is a UUID in a different id space. It is not a + // parameter here at all — including it would make every healthy workspace + // look divergent. + expect( + describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_same', + relayfileWorkspaceId: 'rw_same', + relayauthWorkspaceId: 'rw_same', + }).unified + ).toBe(true); + }); +}); + +describe('formatDataPlaneDivergence', () => { + it('describes a divergence with every plane ID', () => { + const message = formatDataPlaneDivergence( + describeDataPlaneConvergence({ + relaycastWorkspaceId: 'rw_a', + relayfileWorkspaceId: 'rw_b', + relayauthWorkspaceId: 'rw_c', + }) + ); + + expect(message).toContain('relaycast=rw_a'); + expect(message).toContain('relayfile=rw_b'); + expect(message).toContain('relayauth=rw_c'); + // The consequence, not just the symptom — this line is what tells an + // operator why a split matters. + expect(message).toContain('may be issued a new address'); + }); +}); diff --git a/packages/cloud/src/workspace-convergence.ts b/packages/cloud/src/workspace-convergence.ts new file mode 100644 index 000000000..134f4a7fa --- /dev/null +++ b/packages/cloud/src/workspace-convergence.ts @@ -0,0 +1,92 @@ +/** + * The data-plane half of the workspace-identity invariant (AR-448). + * + * `resolveWorkspaceSelection` decides WHICH workspace this checkout addresses. + * That is necessary but not sufficient: a workspace is only durable if + * Relaycast, Relayfile, and RelayAuth all resolve it to the SAME data-plane + * workspace id. When they diverge, each plane keeps its own view of who is a + * member, so a restart can re-register an agent against a different plane and + * hand it a new address — the exact failure this check exists to make visible + * before it happens rather than after agents land in the wrong place. + * + * The Cloud workspace id is deliberately excluded: it is the control-plane + * record that *points at* the data plane, and it legitimately uses a different + * id space (a UUID rather than an `rw_` identity). Including it would make + * every healthy workspace look divergent. + */ + +import type { ActiveWorkspaceDescriptor } from './types.js'; + +/** The three data planes that must agree on one workspace identity. */ +export interface DataPlaneWorkspaceIds { + relaycast: string; + relayfile: string; + relayauth: string; +} + +export interface DataPlaneConvergence { + /** True when all three planes report one identical workspace id. */ + unified: boolean; + /** The single shared data-plane id. Present only when `unified`. */ + workspaceId?: string; + /** Per-plane ids, always present so a divergence report names the culprits. */ + planes: DataPlaneWorkspaceIds; + /** Plane names that disagree with the reference id. Empty when unified. */ + divergent: Array; +} + +/** The plane whose id the others are compared against. */ +const REFERENCE_PLANE: keyof DataPlaneWorkspaceIds = 'relaycast'; + +/** + * Describe whether a resolved workspace satisfies the data-plane identity + * invariant. + * + * Pure and side-effect free: callers decide whether a divergence is a warning + * or a hard failure, because the answer differs by caller. A human running + * `workspace active` wants to see it; a setup doctor or supervisor wants to + * stop on it. + * + * @param workspace - A resolved workspace's three data-plane ids. + * @returns Whether the planes agree, and which disagree when they do not. + */ +export function describeDataPlaneConvergence( + workspace: Pick< + ActiveWorkspaceDescriptor, + 'relaycastWorkspaceId' | 'relayfileWorkspaceId' | 'relayauthWorkspaceId' + > +): DataPlaneConvergence { + const planes: DataPlaneWorkspaceIds = { + relaycast: workspace.relaycastWorkspaceId, + relayfile: workspace.relayfileWorkspaceId, + relayauth: workspace.relayauthWorkspaceId, + }; + + const reference = planes[REFERENCE_PLANE]; + const divergent = (Object.keys(planes) as Array).filter( + (plane) => planes[plane] !== reference + ); + + return divergent.length === 0 + ? { unified: true, workspaceId: reference, planes, divergent: [] } + : { unified: false, planes, divergent }; +} + +/** + * Human-readable one-liner naming which planes disagree. + * + * Contains no secrets — workspace ids are identifiers, not credentials — so it + * is safe to print or log verbatim. + * + * @param convergence - A result from {@link describeDataPlaneConvergence}. + */ +export function formatDataPlaneDivergence(convergence: DataPlaneConvergence): string { + const detail = (Object.entries(convergence.planes) as Array<[string, string]>) + .map(([plane, id]) => `${plane}=${id}`) + .join(', '); + return ( + 'Workspace identity is not durable: Relaycast, Relayfile, and RelayAuth resolve ' + + `to different data-plane workspaces (${detail}). Agents re-registering after a ` + + 'node restart may be issued a new address.' + ); +} diff --git a/specs/workspace-identity.md b/specs/workspace-identity.md new file mode 100644 index 000000000..b71b9d626 --- /dev/null +++ b/specs/workspace-identity.md @@ -0,0 +1,258 @@ +# Workspace Identity — Durable Across Node Restarts + +**Tracking**: AR-448 +**Status**: the behavior described here is implemented on `main`. See +[§7 Coverage](#7-coverage) for exactly which parts are proven by tests and +which are not. + +--- + +## 1. The invariant + +> A local Relay node, and every resident agent on it, keeps the same workspace +> identity and the same delivery address across a full stop/start. + +Three things have to hold for that to be true, and AR-448 originally accounted +for only the first two. + +**One workspace per node, chosen — not minted.** A node start must join a +workspace that already existed, unless the operator explicitly asked for a new +one. This is [§2](#2-how-a-node-picks-its-workspace). + +**One data-plane ID per workspace.** Relaycast, Relayfile, and RelayAuth must +all resolve the canonical workspace to the _same_ `rw_…` identity. The Cloud +workspace ID is deliberately excluded: it is the control-plane record that +points at the data plane, and it lives in a different ID space (a UUID). This is +[§4](#4-proving-the-data-plane-invariant). + +**One work unit per agent name.** Re-registering a name must return the existing +agent — same ID, same address, same inbox — only when the caller can prove it is +the same work unit that holds that name. This is [§3](#3-who-may-reclaim-a-name), +and it is the half AR-448 did not originally account for. + +The first two make the mailbox reachable. The third decides who is allowed to +pick it up. + +**The durable lesson from AR-448 is that its own premise was too strong.** AR-448 +was scoped on the assumption that workspace convergence is _sufficient_ for +address identity. It is **necessary and not sufficient**. Agent-identity +admission is a separate second half, and it landed a week later as its own fix +(`5c2ad8ee3`) rather than falling out of the workspace work. Anyone reasoning +about "will this agent keep its address" needs to check both halves; checking +only the workspace half is how a node that resolves the correct workspace can +still fail to come back as itself. + +## 2. How a node picks its workspace + +`agent-relay up` / `node up` resolves the workspace through one shared +precedence ladder — `resolveWorkspaceSelection` in +`packages/cloud/src/project-workspace-key.ts`. Every caller (SDK clients, the +CLI, the broker start path) walks the same ladder, 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`, written by the previous + successful start. +4. **`store`** — the machine-global active entry in + `~/.agentworkforce/relay/workspaces.json`, set by + `agent-relay workspace join|switch|create`. +5. **Nothing resolves** — the broker mints a workspace so it can come up at all. + +Two rules make the ladder durable rather than merely ordered: + +- **The repository pin always outranks the machine-global entry.** A global + selection must never silently re-home a checkout that already pinned a + workspace. +- **A Fleet enrollment (`RELAY_NODE_TOKEN`) selects the node's _identity_, never + its workspace**, so it does not appear on the ladder at all. Letting it + short-circuit the walk is what re-homed an enrolled node out of its + repository's workspace and into a freshly minted one. + +Step 4 is what makes a first start durable. Without it, a start with no +repository pin fell straight through to step 5: the broker minted a brand-new +workspace, the resident agent registered into it as a stranger, and every +message addressed to its previous address went nowhere. Nothing errored — the +node came up and the agent looked healthy. + +After a successful start the resolved key is pinned to the project, so the next +start takes step 3 and does not consult the store at all. + +Startup prints which step won — the flag name, the variable, or the file path, +never key material — and says explicitly whether it joined a workspace or +created one: + +``` +Workspace source: machine-global active workspace (~/.agentworkforce/relay/workspaces.json (active: "default")) +Workspace Key: rk_live_…de99 +Workspace: joined rw_7ccfea89 +``` + +## 3. Who may reclaim a name + +Landing in the right workspace gets a registration to the right door. It does +not decide who is let through it. + +Registration is a fail-closed admission gate: `admit_agent_registration` in +`crates/broker/src/relaycast/auth.rs`. A name collision is **rejected** by +default. Reclaim — returning the existing agent's ID and address with a freshly +rotated token — is permitted only when the request proves it is the same work +unit: a caller-supplied identity key must match the one stamped on the existing +agent's metadata at its creation, compared as a SHA-256 hash rather than a raw +value, because metadata is readable by anyone holding the same workspace key. + +The broker's own startup registration proves it with `stable_node_identity_key`, +derived from the broker's persisted state directory. The same project/state +directory hashes identically across a kill and restart; a different checkout +hashes to something else. + +**This narrows what AR-448 originally claimed.** The original work assumed that +converging two checkouts on one workspace also converged their resident +addresses — that a second checkout would simply pick up the same agent. Under +the gate it does not, and should not: two checkouts are two work units, and +handing the second one the incumbent's credentials is the duplicate-agent +failure the gate exists to stop. So: + +| Situation | Outcome | +| --------------------------------------------- | --------------------------------- | +| Same node restarts, same state dir, same name | Reclaims its address and inbox | +| Different checkout, same workspace, same name | Rejected — no credential hand-off | +| Any node, name not currently held | Registers fresh | + +An operator who genuinely needs to move a resident to a new checkout sets +`RELAY_AGENT_IDENTITY_KEY` to the original work unit's identity, which is the +documented, deliberate path rather than an accident of ordering. + +## 4. Proving the data-plane invariant + +`agent-relay workspace active` emits a `dataPlane` block on every call, so its +output is self-sufficient evidence rather than something a caller has to +recompute from the three plane IDs: + +``` +$ agent-relay workspace active --json +{ + "name": "default", + "key": "rk_live_…de99", + "cloudWorkspaceId": "50587328-441d-4acb-b8f3-dbe1b3c5de99", + "relaycastWorkspaceId": "rw_7ccfea89", + "relayfileWorkspaceId": "rw_7ccfea89", + "relayauthWorkspaceId": "rw_7ccfea89", + "dataPlane": { + "unified": true, + "workspaceId": "rw_7ccfea89", + "planes": { + "relaycast": "rw_7ccfea89", + "relayfile": "rw_7ccfea89", + "relayauth": "rw_7ccfea89" + }, + "divergent": [] + } +} +``` + +On a divergence, `unified` is `false`, `workspaceId` is **absent** — offering a +single ID would paper over the split the block exists to surface — and +`divergent` names the planes that disagree. + +By default a divergence is reported on stderr and the command still exits 0, so +existing scripted callers keep their exit code. Pass `--require-unified` to turn +it into a hard gate; that is the form supervisors and setup doctors should use: + +``` +$ agent-relay workspace active --json --require-unified +``` + +The human output reports the same thing, including the Relaycast ID it +previously omitted: + +``` +$ agent-relay workspace active +Workspace: default +Cloud workspace ID: 50587328-441d-4acb-b8f3-dbe1b3c5de99 +Relaycast workspace ID: rw_7ccfea89 +Relayfile workspace ID: rw_7ccfea89 +Relayauth workspace ID: rw_7ccfea89 +Data-plane workspace ID: rw_7ccfea89 (unified) +``` + +## 5. Secrets + +Status output and startup logs never print a raw workspace key, agent token, +node token, or a credential-bearing observer URL. + +- Keys printed on purpose go through `maskSecret` — prefix plus last four + characters. +- Error and log _text_ goes through `redactCredentialValues`, which catches + credentials embedded in URL paths and query strings, where key-name redaction + cannot help. +- Structured dumps go through `redactSecrets`, which replaces the value of any + credential-named key. +- The ladder logs only which step won and its origin — a flag name, a variable + name, or a file path. It never names the key. + +Workspace IDs are identifiers, not credentials, and are printed in full. + +## 6. Migration for existing local nodes + +No action is required, and nothing is rewritten on upgrade. + +- **A node with a repository pin** keeps using that pinned workspace. The store + step sits below the pin in precedence and never overrides it. +- **A node with no repository pin** now joins the machine-global active + workspace on its next start instead of minting. If that node had been drifting + onto a new workspace each restart, this is the fix — but its resident agents + move to the canonical workspace, and any address someone recorded from a + previous throwaway workspace stops resolving. That address was already invalid + after the next restart. +- **A machine with no active workspace set** behaves exactly as before: the + broker mints one. Set one with `agent-relay workspace join ` (or + `switch`) to opt into durable identity. +- **A node pinned to a workspace you no longer want** re-pins on the next start + after an explicit `--workspace-key`, since step 1 wins and the resolved key is + written back to the pin. + +To move an existing node onto the canonical workspace deliberately: + +``` +$ agent-relay workspace switch default +$ rm .agentworkforce/relay/workspace-key.json # drop the stale repository pin +$ agent-relay node down && agent-relay node up +$ agent-relay workspace active --require-unified +``` + +Note that moving a checkout onto a workspace where its resident's name is +already held by a **different** work unit is a rejection, not a merge — see +[§3](#3-who-may-reclaim-a-name). + +## 7. Coverage + +These are unit tests over the CLI's own TypeScript and the broker's Rust. They +prove the CLI **selects** the right workspace across starts and that the broker +**decides** reclaim correctly. They do not prove that identity survives a real +stop/start of a live node — that is a live proof, and it is not automated here. + +| Guarantee | Test | +| ----------------------------------------------------------------- | ------------------------------------------------------------- | +| Workspace and resident address are preserved across a restart | `packages/cli/src/cli/lib/workspace-identity-restart.test.ts` | +| A first start with no pin joins the canonical workspace | same | +| The resolved workspace is pinned, so start 2 resumes from the pin | same | +| An explicit `--workspace-key` re-pins durably | same | +| A second checkout joins the same canonical workspace | same | +| A second checkout may **not** take the resident's name | same | +| No canonical workspace ⇒ per-checkout drift (negative control) | same | +| Single-start ladder precedence, each step | `packages/cli/src/cli/lib/broker-lifecycle.test.ts` | +| Startup prints the winning source and leaks no credential | same | +| The shared ladder itself | `packages/cloud/src/project-workspace-key.test.ts` | +| Data-plane convergence detection | `packages/cloud/src/workspace-convergence.test.ts` | +| `workspace active` emits convergence evidence | `packages/cli/src/cli/commands/workspace.test.ts` | +| `--require-unified` exits non-zero on divergence | same | +| A restart reclaims its own registration; another node cannot | `crates/broker/src/relaycast/auth.rs` (`#[cfg(test)]`) | + +**Not covered by any test:** the live proof that a resident agent keeps its +address and mailbox across a real `node down` / `node up`. It requires an +operator at the keyboard, because stopping the broker stops the resident agent +performing the check. It also depends on commit `5c2ad8ee3` (§3), which is on +`main` but not in any released broker — a live attempt on an older installed +broker fails for that reason rather than because the invariant is broken.