diff --git a/src/cli/fleet.test.ts b/src/cli/fleet.test.ts index 2a276d95..23dff196 100644 --- a/src/cli/fleet.test.ts +++ b/src/cli/fleet.test.ts @@ -1346,6 +1346,43 @@ describe('fleet CLI runtime', () => { } }) + // The plumbing that was missing: without it a cloud deployment can configure + // `relay.agentName` and still register as the default, colliding with itself. + it.each([ + ['forwards a configured relay agent name to fleet construction', { relay: { agentName: 'factory-cloud' } }, 'factory-cloud'], + ['leaves the relay agent name unset when the config omits it', {}, undefined], + ])('%s', async (_label, overrides, expected) => { + const root = await mkdtemp(join(tmpdir(), 'fleet-cli-relay-identity-')) + try { + const configPath = await writeConfig(root, overrides) + const createFleetCalls: Array<{ relayAgentName?: string }> = [] + const output = buffer() + + const code = await runFleetCli([ + '--backend', + 'relay', + 'run-once', + '--dry-run', + '--config', + configPath, + ], { + createFleet: (opts) => { + createFleetCalls.push(opts as { relayAgentName?: string }) + return new FakeFleetClient() + }, + cloudMountFromConfig: async () => new FakeMountClient({ [issuePath]: issueFile }), + stdout: output, + stderr: buffer(), + }) + + expect(code).toBe(0) + expect(createFleetCalls).toHaveLength(1) + expect(createFleetCalls[0]?.relayAgentName).toBe(expected) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + it('keeps explicit fixtureFiles configs on Fake fleet and mount for harness runs', async () => { const root = await mkdtemp(join(tmpdir(), 'fleet-cli-fixture-opt-in-')) try { diff --git a/src/cli/fleet.ts b/src/cli/fleet.ts index 3d476688..d0a26a60 100644 --- a/src/cli/fleet.ts +++ b/src/cli/fleet.ts @@ -1790,7 +1790,13 @@ async function buildFleet( // real broker bootstrap. if (deps.createFleet) { return deps.createFleet( - { backend: globals.backend, cwd, connectionPath, previewConfig: loaded?.config.preview }, + { + backend: globals.backend, + cwd, + connectionPath, + previewConfig: loaded?.config.preview, + relayAgentName: loaded?.config.relay.agentName, + }, { ownedBrokerAgentExitTimeoutMs: globals.agentExitTimeoutMs }, ) } @@ -1827,7 +1833,16 @@ async function buildFleet( ) } - return createFleet({ backend: globals.backend, cwd, connectionPath, previewConfig: loaded?.config.preview }, { env: deps.env }) + return createFleet( + { + backend: globals.backend, + cwd, + connectionPath, + previewConfig: loaded?.config.preview, + relayAgentName: loaded?.config.relay.agentName, + }, + { env: deps.env }, + ) } function streamLogger(stream: Pick): Logger { diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 96c204d5..2196fd50 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -5,6 +5,126 @@ import { join } from 'node:path' import { FactoryConfigSchema, NodeConfigSchema, loadFactoryConfig } from './schema' import { routedPrRepos } from '../github/routed-pr-babysitter' +describe('relay.agentName', () => { + it('keeps an explicit relay agent name', () => { + const parsed = FactoryConfigSchema.parse({ + repos: { byLabel: { pear: 'AgentWorkforce/pear' } }, + relay: { agentName: 'factory-cloud' }, + }) + + expect(parsed.relay.agentName).toBe('factory-cloud') + }) + + it('trims surrounding whitespace from an otherwise valid name', () => { + const parsed = FactoryConfigSchema.parse({ + repos: { byLabel: { pear: 'AgentWorkforce/pear' } }, + relay: { agentName: ' factory-cloud ' }, + }) + + expect(parsed.relay.agentName).toBe('factory-cloud') + }) + + // An identity that silently falls back to the default is how a workspace + // registration collision hides: the operator believes they pinned a name. + it.each([ + ['empty', ''], + ['whitespace-only', ' '], + ['a tab', '\t'], + ])('rejects %s relay.agentName at config load instead of defaulting it', (_label, agentName) => { + const result = FactoryConfigSchema.safeParse({ + repos: { byLabel: { pear: 'AgentWorkforce/pear' } }, + relay: { agentName }, + }) + + expect(result.success).toBe(false) + // Asserted by path and code, not by message text: the point is that the + // trimmed value failed the length check under `relay.agentName`, rather + // than some unrelated issue happening to mention the field. + const issue = result.success ? undefined : result.error.issues.find( + (candidate) => candidate.path.join('.') === 'relay.agentName', + ) + expect(issue?.code).toBe('too_small') + }) + + // Split configs are how a cloud deployment and another deployment share one + // workspace. If the identity could only live on the shared half, every + // deployment would register the same name -- the collision this prevents. + it('takes the relay agent name from the node-local half', () => { + const loaded = loadFactoryConfig({ + workspaceConfig: { repos: { default: 'AgentWorkforce/factory' } }, + nodeConfig: { relay: { agentName: 'factory-cloud' } }, + }) + + expect(loaded.factoryConfig.relay.agentName).toBe('factory-cloud') + // The node-local half must carry it back too, or a per-host identity is + // reflected as workspace-shared configuration. + expect(loaded.nodeConfig.relay.agentName).toBe('factory-cloud') + }) + + it('lets the node-local relay agent name override the workspace default', () => { + const loaded = loadFactoryConfig({ + workspaceConfig: { + repos: { default: 'AgentWorkforce/factory' }, + relay: { agentName: 'factory-shared' }, + }, + nodeConfig: { relay: { agentName: 'factory-cloud' } }, + }) + + expect(loaded.factoryConfig.relay.agentName).toBe('factory-cloud') + }) + + it('still applies a workspace-half relay agent name when the node half sets none', () => { + const loaded = loadFactoryConfig({ + workspaceConfig: { + repos: { default: 'AgentWorkforce/factory' }, + relay: { agentName: 'factory-shared' }, + }, + nodeConfig: {}, + }) + + expect(loaded.factoryConfig.relay.agentName).toBe('factory-shared') + }) + + // Both halves are validated before the merge, so a node override cannot + // launder an invalid shared value into a clean load on the hosts that + // override it. + it.each([ + ['workspaceConfig', { relay: { agentName: ' ' } }, { relay: { agentName: 'factory-cloud' } }], + ['nodeConfig', { relay: { agentName: 'factory-shared' } }, { relay: { agentName: ' ' } }], + ])('rejects an invalid relay agent name in the %s half even when the other half is valid', (field, workspaceRelay, nodeRelay) => { + expect(() => loadFactoryConfig({ + workspaceConfig: { repos: { default: 'AgentWorkforce/factory' }, ...workspaceRelay }, + nodeConfig: nodeRelay, + })).toThrow(new RegExp(`${field} has an invalid relay config`)) + }) + + // Pins existing `normalizeLoadedConfig` semantics rather than asserting an + // intent: both views are projected from the *merged* config, so the + // workspace-shaped view reports the effective identity, exactly as it already + // does for preview, cloneRoot, and clonePaths. Nothing serializes + // `workspaceConfig` back to a shared file today; this test fails loudly if + // that projection ever changes underneath a caller who starts to. + it('reports the effective identity on the workspace-shaped view after a node override', () => { + const loaded = loadFactoryConfig({ + workspaceConfig: { + repos: { default: 'AgentWorkforce/factory' }, + relay: { agentName: 'factory-shared' }, + }, + nodeConfig: { relay: { agentName: 'factory-cloud' } }, + }) + + expect(loaded.workspaceConfig.relay.agentName).toBe('factory-cloud') + expect(loaded.nodeConfig.relay.agentName).toBe('factory-cloud') + }) + + it('rejects an unknown key under relay rather than ignoring a typo', () => { + expect(() => FactoryConfigSchema.parse({ + repos: { byLabel: { pear: 'AgentWorkforce/pear' } }, + relay: { agentname: 'factory-cloud' }, + })).toThrow(/unrecognized key/i) + }) +}) + describe('FactoryConfigSchema', () => { it('parses a valid config and applies defaults', () => { const parsed = FactoryConfigSchema.parse({ @@ -44,6 +164,9 @@ describe('FactoryConfigSchema', () => { resetTimeoutMs: 60_000, requireDedicatedBroker: false, }) + // Absent, not defaulted in the schema: RelayFleetClient owns the + // `factory` fallback, so there is exactly one place the default lives. + expect(parsed.relay).toEqual({}) expect(parsed.models).toEqual({ babysitter: 'sonnet' }) // Agent CLI per role defaults to today's behavior: codex implements, claude // reviews/babysits — so existing configs are unaffected unless set. diff --git a/src/config/schema.ts b/src/config/schema.ts index db61fc1d..ba72b5a7 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -136,6 +136,33 @@ const fleetHealthSchema = z.object({ requireDedicatedBroker: z.boolean().default(false), }).default({}) +/** + * Relay-workspace identity settings for the `relay` fleet backend. + * + * `agentName` is the workspace agent Factory registers *as itself* — not an + * agent it spawns. Two Factory deployments sharing one workspace must not share + * it: the second registration collides with the first + * (`Agent "" already exists in this workspace`), the fleet control plane + * never initialises, and nothing is ever dispatched. Hosts with ephemeral disk + * cannot prove they own a previously-registered name after a restart, so a + * deployment that needs its own identity pins one here. + * + * Left unset, `RelayFleetClient` keeps its own `DEFAULT_AGENT_NAME`, so every + * existing deployment registers exactly the name it registers today. + * + * Declared on both config halves, like `preview`: the workspace half carries a + * shared default, and the node half overrides it per host. An identity that + * only ever lived on the workspace half would be handed to every deployment in + * the workspace -- the collision this exists to prevent. + */ +const relaySchema = z.object({ + // Deliberately not `.default(...)`: an empty or whitespace-only value is a + // misconfiguration, and coercing it to the default identity is precisely how + // an unconfigurable identity stayed invisible. `.trim().min(1)` rejects it at + // config load instead. + agentName: z.string().trim().min(1).optional(), +}).strict().default({}) + const loopSchema = z.object({ maxIterations: z.number().int().min(1).max(5).default(3), maxConsecutiveFailures: z.number().int().min(1).max(5).default(3), @@ -381,6 +408,7 @@ const WorkspaceConfigObjectSchema = z.object({ liveSubscription: liveSubscriptionSchema, dispatch: dispatchSchema, fleetHealth: fleetHealthSchema, + relay: relaySchema, loop: loopSchema, triage: triageSchema, repos: workspaceReposSchema, @@ -438,6 +466,9 @@ const WorkspaceConfigObjectSchema = z.object({ const NodeConfigObjectSchema = z.object({ workspaceId: z.string().optional(), + // Node-local, so two deployments sharing a workspace can register distinct + // relay identities. Overrides the workspace half when both set it. + relay: relaySchema, capabilities: z.array(z.string()).default([]), cloneRoot: z.string().optional(), clonePaths: z.record(z.string(), z.string()).default({}), @@ -681,6 +712,7 @@ function normalizeLoadedConfig(input: unknown): LoadedFactoryConfig { factoryLoopHeartbeatPath: factoryConfig.loop.heartbeatPath, factoryLoopRegistryPath: factoryConfig.loop.registryPath, preview: factoryConfig.preview, + relay: factoryConfig.relay, }) return { workspaceConfig, nodeConfig, factoryConfig } @@ -701,6 +733,16 @@ function combineSplitConfigInput(workspaceInput: unknown, nodeInput: unknown): R const nodePreview = asOptionalConfigRecord(node.preview) const hasPreview = workspace.preview !== undefined || node.preview !== undefined + // Each half is validated *before* the merge. A node half that overrides the + // identity otherwise discards an invalid workspace-half value without it ever + // reaching `relaySchema`, so a broken shared config would load clean on every + // host that happens to override it -- the silent acceptance this key exists + // to prevent. Mirrors validateClonePathSyntax, which validates both halves + // before node-local values take precedence. + const workspaceRelay = validateRelayHalf(workspace.relay, 'workspaceConfig') + const nodeRelay = validateRelayHalf(node.relay, 'nodeConfig') + const hasRelay = workspace.relay !== undefined || node.relay !== undefined + return { ...workspace, ...node, @@ -714,6 +756,9 @@ function combineSplitConfigInput(workspaceInput: unknown, nodeInput: unknown): R }, }, } : {}), + // Merged rather than replaced, so a node half that pins only `agentName` + // does not drop whatever else the workspace half configured. + ...(hasRelay ? { relay: { ...workspaceRelay, ...nodeRelay } } : {}), repos: { ...workspaceRepos, cloneRoot: node.cloneRoot ?? workspaceRepos.cloneRoot, @@ -722,6 +767,18 @@ function combineSplitConfigInput(workspaceInput: unknown, nodeInput: unknown): R } } +function validateRelayHalf(value: unknown, field: string): Record { + const record = asOptionalConfigRecord(value) + const parsed = relaySchema.safeParse(record) + if (!parsed.success) { + const detail = parsed.error.issues + .map((issue) => `${['relay', ...issue.path].join('.')}: ${issue.message}`) + .join('; ') + throw new Error(`${field} has an invalid relay config -- ${detail}`) + } + return record +} + function normalizePreviewConfig>(preview: T): T { if (!preview) return preview return { diff --git a/src/fleet/create-fleet.test.ts b/src/fleet/create-fleet.test.ts index e2cf8d95..7af7427b 100644 --- a/src/fleet/create-fleet.test.ts +++ b/src/fleet/create-fleet.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { createFleet } from './create-fleet' +import { FactoryConfigSchema } from '../config/schema' import { InternalFleetClient } from './internal-fleet-client' import { RelayFleetClient } from './relay-fleet-client' import type { HarnessDriverClientLike } from './internal-fleet-client' @@ -21,6 +22,11 @@ const fakeHarness: HarnessDriverClientLike = { async sendInput() {}, } +const baseConfig = { + workspaceId: 'ws_create_fleet_test', + repos: { byLabel: { pear: 'AgentWorkforce/pear' } }, +} + describe('createFleet', () => { afterEach(() => { vi.unstubAllEnvs() @@ -97,4 +103,32 @@ describe('createFleet', () => { await expect(fleet.roster()).rejects.toThrow(/requires a workspace key \(rk_live_…\) or agent token \(at_live_…\)/) }) + + // The identity the factory registers as, threaded config -> createFleet -> + // RelayFleetClient. Both directions are asserted from a really-parsed config + // so the schema, the option, and the client default stay in one story. + describe('relay workspace identity', () => { + const relayFleetFor = (raw: Record): RelayFleetClient => { + const config = FactoryConfigSchema.parse({ ...baseConfig, ...raw }) + return createFleet( + { backend: 'relay', relayAgentName: config.relay.agentName }, + { env: {} }, + ) as RelayFleetClient + } + + it('registers under the agent name the config supplies', () => { + expect(relayFleetFor({ relay: { agentName: 'factory-cloud' } }).agentName).toBe('factory-cloud') + }) + + // Guards the upgrade path, not the feature: a deployment that never sets + // `relay.agentName` must keep registering the exact name it registers + // today. A silent identity change here strands a live deployment the same + // way an unconfigurable identity did. + it('keeps the built-in `factory` identity when the config omits an agent name', () => { + const config = FactoryConfigSchema.parse(baseConfig) + + expect(config.relay.agentName).toBeUndefined() + expect(relayFleetFor({}).agentName).toBe('factory') + }) + }) }) diff --git a/src/fleet/create-fleet.ts b/src/fleet/create-fleet.ts index bbc4517d..9f55e238 100644 --- a/src/fleet/create-fleet.ts +++ b/src/fleet/create-fleet.ts @@ -11,6 +11,12 @@ export interface CreateFleetOptions { cwd?: string connectionPath?: string previewConfig?: PreviewConfig + /** + * Workspace identity the relay backend registers as (config `relay.agentName`). + * Relay-only; the internal backend has no workspace registration. Undefined + * leaves `RelayFleetClient`'s own default in place. + */ + relayAgentName?: string } export interface CreateFleetDeps { @@ -53,6 +59,7 @@ export function createFleet(options: CreateFleetOptions = {}, deps: CreateFleetD if (backend === 'relay') { return new RelayFleetClient({ workspaceKey: deps.workspaceKey, + agentName: options.relayAgentName, env: deps.env, log: deps.logger ? (message) => deps.logger?.info?.(message) : undefined, }) diff --git a/src/fleet/relay-fleet-client.ts b/src/fleet/relay-fleet-client.ts index 90693f3f..e6e763ba 100644 --- a/src/fleet/relay-fleet-client.ts +++ b/src/fleet/relay-fleet-client.ts @@ -162,6 +162,17 @@ export class RelayFleetClient implements FleetClient { #reconciling: Promise | undefined #pendingReleaseRetry: Promise | undefined + /** + * Workspace identity this client registers as, after defaulting. + * + * Exposed read-only so a caller (and its tests) can confirm which identity a + * given configuration actually resolved to. Registration reads `#agentName` + * directly; this changes nothing about it. + */ + get agentName(): string { + return this.#agentName + } + constructor(options: RelayFleetClientOptions = {}) { this.#options = options this.#agentName = options.agentName ?? DEFAULT_AGENT_NAME