diff --git a/CHANGELOG.md b/CHANGELOG.md index a9ef68e9e..efcc5c636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ 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. +- `agent-relay workspace rebind ` pins a project's next broker start to a named workspace 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. +- `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 @@ -17,6 +24,10 @@ 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. +- `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..bf8710898 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; @@ -351,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, @@ -912,6 +959,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(), @@ -946,15 +994,52 @@ 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 + .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 { + // `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); + } tracing::info!( target = "broker::spawn", @@ -2015,6 +2100,165 @@ 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(); + + let error = confirm_worker_process_alive( + "failed-worker", + &mut child, + Some(Path::new("/tmp/failed-worker.log")), + Duration::from_millis(500), + ) + .await + .unwrap_err(); + + let message = error.to_string(); + assert!(message.contains("process exited during startup")); + 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(); + + 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(); + } + + #[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/README.md b/packages/cli/README.md index adfd75b97..eceb7a325 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -47,6 +47,79 @@ 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. +### 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: + +| # | 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 | Created workspace | created only when nothing above resolves | + +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. + +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 — 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 +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 The `fleet` command group lists and controls agents across all live nodes in 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..49e25fb70 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 } : {}), }); } @@ -114,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); }), @@ -156,6 +173,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); @@ -494,11 +512,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); @@ -557,7 +579,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', @@ -566,6 +596,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'); @@ -652,7 +683,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', @@ -882,6 +913,73 @@ 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.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) => { @@ -1173,7 +1271,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 +1291,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) }); @@ -1538,6 +1675,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: ' ', @@ -1585,7 +1759,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'; @@ -1603,9 +1777,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), @@ -1615,15 +1801,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 5b5008160..fb63f5a03 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'; @@ -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. */ @@ -69,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; @@ -187,6 +190,9 @@ async function createDefaultRelay( get workspaceKey() { return client.workspaceKey; }, + get workspaceId() { + return client.workspaceId; + }, get brokerPid() { return client.brokerPid; }, @@ -203,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), @@ -273,6 +280,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; @@ -293,6 +302,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/node.test.ts b/packages/cli/src/cli/commands/node.test.ts index 7cda1db5d..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), @@ -48,7 +49,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 +251,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 +266,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 +351,57 @@ 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'); + 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'); + 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 @@ -376,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 febc49231..0fa000832 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,42 @@ 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( + '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; } /** Apply stored enrollment credentials and return the enrolled node name, when present. */ @@ -174,7 +211,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 +227,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 +268,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/commands/workspace.test.ts b/packages/cli/src/cli/commands/workspace.test.ts index e57c5e302..ddad0b885 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,65 @@ 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 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', + 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']); + + 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 +348,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..659e61d02 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'; @@ -102,9 +103,14 @@ export function registerWorkspaceCommands( 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 +137,7 @@ export function registerWorkspaceCommands( const store = readWorkspaceStore(); printJson(deps, { active: store.active, + previous: store.previous, workspaces: Object.keys(store.workspaces), }); }); @@ -199,4 +206,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 299d796cd..6dbab834b 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'); @@ -221,6 +229,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 +257,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); @@ -272,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, @@ -297,7 +308,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 +503,148 @@ 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(); + 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'); + expect(readBindingSource(dataDir)).toBe('project'); + }); + + 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'); + expect(readBindingSource(deps.getProjectPaths().dataDir)).toBe('store'); + }); + + it('announces a mint when no source resolves (#1378)', async () => { + const { deps, dataDir, 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'); + 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 }); + 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'); + 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 '; + + 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 () => { + 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..e015f5d4b 100644 --- a/packages/cli/src/cli/lib/broker-lifecycle.ts +++ b/packages/cli/src/cli/lib/broker-lifecycle.ts @@ -1,8 +1,10 @@ +import { randomUUID } from 'node:crypto'; import fs from 'node:fs'; 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'; @@ -24,12 +26,19 @@ 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 { promoteWorkspaceKeyEnvAlias } from './workspace-env.js'; +import { + readProjectWorkspaceSession, + resolveWorkspaceSelection, + writeProjectWorkspaceKey, + type ProjectWorkspaceSession, + type WorkspaceSelection, +} from './project-workspace-key.js'; type UpOptions = { spawn?: boolean; background?: boolean; + /** Internal marker set only on the detached child re-exec. */ + backgroundChild?: boolean; verbose?: boolean; workspaceKey?: string; stateDir?: string; @@ -65,6 +74,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; @@ -73,11 +84,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' | 'multi-workspace'; + 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 = { @@ -291,7 +306,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(' — ')); } /** @@ -713,6 +728,93 @@ 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 === 'multi-workspace' + ? 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)'; + 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, + deps: CoreDependencies +): void { + 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( + tmpPath, + `${JSON.stringify({ ...connection, workspace_source: source }, null, 2)}\n`, + 'utf-8' + ); + deps.fs.renameSync(tmpPath, connectionPath); +} + +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, + dataDir: string, + isDetachedChild: boolean, + deps: CoreDependencies +): void { + if (!isDetachedChild) return; + try { + // 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. + } +} + function readBrokerPid(dataDir: string, _deps: CoreDependencies): number | null { const conn = readBrokerConnectionFromFs(_deps.fs, dataDir); return conn?.pid ?? null; @@ -959,6 +1061,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 { @@ -1001,6 +1104,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; } @@ -1109,13 +1215,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); @@ -1287,57 +1397,73 @@ function planCapacitySource( return plan.mode === 'in-process' ? plan.definition : descriptorCapacitySource(plan.descriptor); } -interface PinnedProjectWorkspaceSession { - workspaceKey: string; - enrolledNodeId?: 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; - }>; - const workspaceKey = - typeof parsed.workspaceKey === 'string' ? parsed.workspaceKey.trim() || undefined : undefined; - if (!workspaceKey) { - return undefined; - } - const enrolledNodeId = - typeof parsed.enrolledNodeId === 'string' ? parsed.enrolledNodeId.trim() || undefined : undefined; - return { - workspaceKey, - ...(enrolledNodeId ? { enrolledNodeId } : {}), - }; - } catch { +/** + * 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 +): ProjectWorkspaceSession | 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; } -} -/** Resume the pinned project session unless explicit credentials override it. */ -function resumePinnedProjectWorkspace( - options: UpOptions, - deps: CoreDependencies, - projectDataDir: string -): PinnedProjectWorkspaceSession | undefined { - const explicitEnvWorkspaceKey = promoteWorkspaceKeyEnvAlias(deps.env); - if (options.workspaceKey?.trim() || explicitEnvWorkspaceKey || deps.env.RELAY_NODE_TOKEN?.trim()) { + deps.log(`Workspace source: ${describeWorkspaceSource(selection.source)} (${selection.origin})`); + // 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; + if (selection.source === 'project' || selection.source === 'store') { + deps.env.RELAY_API_KEY = selection.key; + } + if (selection.source !== 'project') { 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; - } + const pinned = readProjectWorkspaceSession(projectDataDir, deps.fs); + if (pinned?.enrolledNodeId) { + deps.env.AGENT_RELAY_ENROLLED_NODE_ID = pinned.enrolledNodeId; } - return session; + 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'; + } +} + +/** + * 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, + overrideSource?: WorkspaceBindingSource +): WorkspaceBindingSource { + const inheritedSource = workspaceBindingSource(deps.env[WORKSPACE_BINDING_SOURCE_ENV]); + const source: WorkspaceBindingSource = + overrideSource ?? + (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 { @@ -1350,7 +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 resumedProjectSession = resumePinnedProjectWorkspace(options, 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); @@ -1386,6 +1534,8 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): return; } + const startErrorPath = backgroundStartErrorPath(paths.dataDir); + safeUnlink(startErrorPath, deps); const args = childUpArgsForDetachedStart(options, deps); const invocation = detachedCliInvocation(deps, args); let child: SpawnedProcess; @@ -1411,23 +1561,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') { @@ -1495,6 +1658,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; } @@ -1569,10 +1733,31 @@ 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)'); 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'; + // 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}`); + deps.log( + 'Pin a workspace for this repository with `agent-relay up --workspace-key `, ' + + 'or select one machine-wide with `agent-relay workspace switch `.' + ); + } deps.log('Broker started.'); // Record the workspace this broker joined (explicitly passed or auto-minted) @@ -1583,6 +1768,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 @@ -1667,10 +1856,12 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies): stage, error_class: classifyBrokerStartError(err), }); + const detailedMessage = describeErrorWithCause(err); + recordBackgroundStartError(detailedMessage, paths.dataDir, options.backgroundChild === true, 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); } @@ -1829,6 +2020,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 0cac556c4..5d97af820 100644 --- a/packages/cli/src/cli/lib/project-workspace-key.ts +++ b/packages/cli/src/cli/lib/project-workspace-key.ts @@ -4,6 +4,10 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, + 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..312632579 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 and clears the 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 b11339ea7..36d75bf6b 100644 --- a/packages/cloud/src/index.ts +++ b/packages/cloud/src/index.ts @@ -136,12 +136,16 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, resolveWorkspaceKey, resolveWorkspaceKeyWithSource, + resolveWorkspaceSelection, writeProjectWorkspaceKey, type ProjectWorkspaceSession, type ResolveWorkspaceKeyOptions, + type WorkspaceKeyFileSystem, 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..29a5c1b4f 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,74 @@ 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', + }); + }); + + 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 5fbb77de1..cf39845ec 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'; @@ -23,6 +32,27 @@ 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; +} + +/** + * 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`. */ @@ -31,21 +61,29 @@ 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; const enrolledNodeId = trimOrUndefined(parsed.enrolledNodeId); + const workspaceId = trimOrUndefined(parsed.workspaceId); return { workspaceKey, ...(enrolledNodeId ? { enrolledNodeId } : {}), + ...(workspaceId ? { workspaceId } : {}), }; } catch { return undefined; @@ -59,11 +97,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 +111,7 @@ export function writeProjectWorkspaceKey( { workspaceKey: key, ...(enrolledNodeId ? { enrolledNodeId } : {}), + ...(workspaceId ? { workspaceId } : {}), } satisfies ProjectWorkspaceSession, null, 2 @@ -102,29 +142,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, options.fileSystem ?? fs) : 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..f10229583 100644 --- a/packages/cloud/src/workspace-key.ts +++ b/packages/cloud/src/workspace-key.ts @@ -2,10 +2,14 @@ export { projectWorkspaceKeyPath, readProjectWorkspaceKey, readProjectWorkspaceSession, + resolveActiveWorkspaceSelection, resolveWorkspaceKey, resolveWorkspaceKeyWithSource, + resolveWorkspaceSelection, 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; 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; } 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); diff --git a/tests/integration/broker/cli-spawn.test.ts b/tests/integration/broker/cli-spawn.test.ts index 74c984c17..f1977a146 100644 --- a/tests/integration/broker/cli-spawn.test.ts +++ b/tests/integration/broker/cli-spawn.test.ts @@ -480,6 +480,42 @@ 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 { + // 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|failed writing frame to worker/, + '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) => {