fix(cli): resolve the broker workspace through one precedence ladder #1412
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,8 @@ export interface CoreRelay { | |
| shutdown: () => Promise<unknown>; | ||
| /** Agent Relay workspace key, available after the hello handshake. */ | ||
| workspaceKey?: string; | ||
| /** Relay workspace id the broker joined, available after the hello handshake. */ | ||
| workspaceId?: string; | ||
| /** PID of the underlying broker process, when available. */ | ||
| brokerPid?: number; | ||
| /** Actual HTTP API port bound by the broker, including OS-assigned ports. */ | ||
|
|
@@ -187,6 +189,9 @@ async function createDefaultRelay( | |
| get workspaceKey() { | ||
| return client.workspaceKey; | ||
| }, | ||
| get workspaceId() { | ||
| return client.workspaceId; | ||
| }, | ||
| get brokerPid() { | ||
| return client.brokerPid; | ||
| }, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,7 +47,14 @@ function createNodeHarness(opts?: { | |
| const error = vi.fn(); | ||
| const warn = vi.fn(); | ||
|
|
||
| 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']); | ||
|
|
@@ -236,7 +243,7 @@ describe('registerNodeCommands', () => { | |
| expect(env.RELAY_NODE_TOKEN).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('never adopts an enrollment for a project that pinned its own workspace', async () => { | ||
| const resolveEnrollment = vi.fn( | ||
| () => enrollmentRecord | ||
| ) as unknown as NodeCommandDependencies['resolveEnrollment']; | ||
|
|
@@ -251,13 +258,64 @@ 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_NODE_TOKEN).toBeUndefined(); | ||
| expect(brokerMocks.runUpCommand).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('refuses to start when the enrollment and the repository pin disagree (#1406)', async () => { | ||
| const resolveEnrollment = vi.fn( | ||
| () => enrollmentRecord | ||
| ) as unknown as NodeCommandDependencies['resolveEnrollment']; | ||
| const { program, env, error, exit } = createNodeHarness({ | ||
| env: { AGENT_RELAY_HOME: '/tmp/relay-home-fixture' }, | ||
| resolveEnrollment, | ||
| // A previous start recorded rw_stale; the enrollment points at rw_123. | ||
| resolveProjectWorkspaceSession: vi.fn(() => ({ | ||
| workspaceKey: 'rk_project_session', | ||
| enrolledNodeId: 'node_abc', | ||
| workspaceId: 'rw_stale', | ||
| })), | ||
| }); | ||
|
|
||
| await expect(program.parseAsync(['node', 'up'], { from: 'user' })).rejects.toBeInstanceOf(ExitSignal); | ||
|
|
||
| expect(exit).toHaveBeenCalledWith(1); | ||
| const message = error.mock.calls.flat().join('\n'); | ||
| expect(message).toContain('select different workspaces'); | ||
| expect(message).toContain('rw_stale'); | ||
| expect(message).toContain('rw_123'); | ||
| expect(message).toContain('workspace-key.json'); | ||
| // Diagnostics name sources, never credentials. | ||
| expect(message).not.toContain('rk_project_session'); | ||
| expect(message).not.toContain('nt_secret'); | ||
| expect(env.RELAY_NODE_TOKEN).toBeUndefined(); | ||
| expect(brokerMocks.runUpCommand).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('starts normally when the enrollment matches the pinned workspace', async () => { | ||
| const resolveEnrollment = vi.fn( | ||
| () => enrollmentRecord | ||
| ) as unknown as NodeCommandDependencies['resolveEnrollment']; | ||
| const { program, env } = createNodeHarness({ | ||
| env: {}, | ||
| resolveEnrollment, | ||
| resolveProjectWorkspaceSession: vi.fn(() => ({ | ||
| workspaceKey: 'rk_project_session', | ||
| enrolledNodeId: 'node_abc', | ||
| workspaceId: 'rw_123', | ||
| })), | ||
| }); | ||
|
|
||
| await program.parseAsync(['node', 'up'], { from: 'user' }); | ||
|
|
||
| expect(env.RELAY_NODE_TOKEN).toBe('nt_secret'); | ||
| expect(brokerMocks.runUpCommand).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('preserves an enrolled identity across a consecutive project-session restart', async () => { | ||
| const firstResolveEnrollment = vi.fn( | ||
| () => enrollmentRecord | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type { Command } from 'commander'; | ||
| import { fleetNodeEnrollmentStorePath, resolveActiveFleetNodeEnrollment } from '@agent-relay/cloud'; | ||
|
|
||
| import { | ||
| addUpCommandOptions, | ||
|
|
@@ -9,7 +9,11 @@ import { | |
| type UpCommandOptions, | ||
| } from './core.js'; | ||
| import { runUpCommand } from '../lib/broker-lifecycle.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'; | ||
|
|
@@ -81,10 +85,41 @@ function prepareExplicitWorkspaceForNodeUp( | |
| return Boolean(options.workspaceKey?.trim() || envWorkspaceKey); | ||
| } | ||
|
|
||
| /** | ||
| * 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<ReturnType<typeof resolveActiveFleetNodeEnrollment>>, | ||
| session: ProjectWorkspaceSession | undefined, | ||
| deps: NodeCommandDependencies | ||
| ): boolean { | ||
| const pinnedWorkspaceId = session?.workspaceId?.trim(); | ||
| const enrolledWorkspaceId = record.relayWorkspaceId?.trim(); | ||
| if (!pinnedWorkspaceId || !enrolledWorkspaceId || pinnedWorkspaceId === enrolledWorkspaceId) { | ||
| return false; | ||
| } | ||
|
|
||
| const pinPath = projectWorkspaceKeyPath(deps.core.getProjectPaths().dataDir); | ||
| deps.error( | ||
| 'Refusing to start: this repository and the stored Fleet enrollment select different workspaces.' | ||
| ); | ||
| deps.error(` repository pin ${pinPath} -> workspace ${pinnedWorkspaceId}`); | ||
| deps.error( | ||
| ` fleet enrollment ${fleetNodeEnrollmentStorePath(deps.core.env)} -> workspace ${enrolledWorkspaceId} (node ${record.nodeId})` | ||
| ); | ||
| deps.error( | ||
| 'Pass --workspace-key to choose explicitly, re-enroll this node in the pinned workspace, ' + | ||
| 'or delete the repository pin to adopt the enrollment.' | ||
| ); | ||
| return true; | ||
| } | ||
|
|
||
| /** Apply stored enrollment credentials and return the enrolled node name, when present. */ | ||
|
|
@@ -130,7 +165,14 @@ function resolveEnrollmentForProject( | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * 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<typeof resolveActiveFleetNodeEnrollment> | undefined, | ||
| projectSession: ProjectWorkspaceSession | undefined, | ||
|
|
@@ -139,16 +181,12 @@ function applyResolvedNodeSession( | |
| if (record) { | ||
| return applyEnrollment(record, deps); | ||
| } | ||
| 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.` | ||
| ); | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
|
|
@@ -184,6 +222,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); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the
local upalias.This section names
agent-relay upandagent-relay node up, but notagent-relay local up. Add the alias so users can find the same workspace-resolution rules for every supported startup command.🤖 Prompt for AI Agents