fix(cli): stop silently ignoring fleet enrollments behind a project pin - #1439
Conversation
A project workspace pin without an `enrolledNodeId` made `relay node up` skip the fleet enrollment store entirely — no warning, no credentials, no heartbeat. The Cloud dashboard showed the node, `fleet nodes` showed a different roster, and nothing said the two were different workspaces (#1432). One machine was enrolled but invisible for five days. Three fixes, smallest first: 1. `node.ts` warns before returning when a pin shadows stored enrollments, naming how many are being ignored and how to recover. The sibling branch already warned; this one did not. 2. `cloud enroll` records the enrolled node on the project pin so `node up` in that repo serves it. A pin naming a different node is reported and left untouched — the one-time token is already redeemed by then, so repointing it would trade one invisible mismatch for another. Pin failures never fail a completed enrollment. 3. `workspace switch|join` preserves the pin's `enrolledNodeId` instead of dropping it, which is what manufactured the broken state in fix 1. Refs #1432 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe CLI reconciles enrolled Fleet nodes with project workspace pins. ChangesEnrollment pin consistency
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CloudEnroll
participant FleetEnrollmentStore
participant ProjectWorkspacePin
Operator->>CloudEnroll: enroll node
CloudEnroll->>FleetEnrollmentStore: persist enrolled node
CloudEnroll->>ProjectWorkspacePin: reconcile project pin
ProjectWorkspacePin-->>CloudEnroll: link, unchanged, conflict, or error
CloudEnroll-->>Operator: success output and applicable warning
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe20d6ea9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const enrolledNodeId = readProjectWorkspaceSession(projectDataDir)?.enrolledNodeId; | ||
| writeProjectWorkspaceKey(projectDataDir, workspaceKey, { | ||
| ...(enrolledNodeId ? { enrolledNodeId } : {}), |
There was a problem hiding this comment.
When a project enrolled as node A in workspace A runs workspace switch or workspace join for workspace B, this preserves A's enrolledNodeId beside B's key. On the next node up, resolveEnrollmentForProject in commands/node.ts resolves solely by that node ID, and applyResolvedNodeSession applies the enrollment without applying the newly pinned key, so the broker reconnects to A while SDK commands use B. This recreates the split-workspace behavior the change is meant to prevent; the association must be cleared unless the selected workspace is verified to match the enrollment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and fixed in d40a3f013. You and the other reviewer landed this independently and you were right — preserving the id unconditionally recreated the split the PR exists to remove, because applyResolvedNodeSession returns applyEnrollment(record) without ever calling resumeProjectWorkspace, so the pinned key is never applied to the broker.
Fixed by comparing what can be compared locally. The enrollment store holds workspace ids and the pin holds a key, so "verify the enrollment belongs to the new workspace" is not answerable offline — but key-against-key is:
- same workspace key re-selected → keep
enrolledNodeId(the original bug: dropping it manufactured the pinnode upnow warns about) - different workspace key → clear it, and
workspace switch|joinprints which node association was dropped and how to re-enroll, rather than dropping it silently
persistWorkspaceSession now returns { clearedEnrolledNodeId? } so the command layer can report it. Regression test: clears the enrolled Fleet node id when moving to a different workspace — fails against the unconditional-preserve version, passes now.
| return { status: 'conflict', nodeId, pinnedNodeId: session.enrolledNodeId, pinPath }; | ||
| } | ||
|
|
||
| writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId }); |
There was a problem hiding this comment.
If cloud enroll --workspace B (or a token minted for B) is run from a project whose unassociated pin contains workspace A's key, this writes B's node ID alongside the unchanged A key without checking their workspaces. node up then resolves B's credentials by node ID and ignores the pinned key, while other project commands continue resolving A from the same file; the command nevertheless reports that the pin was successfully linked. Leave the pin unlinked with a warning unless the enrollment workspace can be verified against it, or update both sides together.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly accepted, and the overclaim is fixed in d40a3f013.
The diagnosis is right: nothing here verifies that the enrollment's workspace matches the pinned key, and it cannot — the enrollment store records workspace ids while the pin records a key, with no local mapping between them. That unreconciled gap is called out as the root cause in #1432 and is what #1440/#1442 depend on closing.
Where I did not follow the recommendation: "leave the pin unlinked with a warning unless verifiable" would mean never linking, since it is never locally verifiable — and that removes the fix for the case actually reported. In #1432 the repo was pinned to an auto-provisioned throwaway workspace (204337648549896192) and the user enrolled into rw_7ccfea89 specifically so node up would serve the enrolled node. Refusing to link leaves that user exactly where they started. (The throwaway pin itself is the separate bug in #1440.)
What is fixed is the part I agree was wrong — the command claiming a link it had not verified. The message now names the workspace that will actually be served and states plainly what was not checked:
Linked this project's workspace pin (…) to node …, so 'relay node up' here serves this enrollment in workspace rw_123. The pinned workspace key was not verified against that workspace — if they differ, agent commands in this project keep using the pinned one.
Test cloud enroll links the enrolled node to this project workspace pin now asserts both the served workspace id and the caveat, and fails against the previous message.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/cli/lib/enrollment-pin.ts">
<violation number="1" location="packages/cli/src/cli/lib/enrollment-pin.ts:66">
P2: Concurrent enrollments can still silently repoint a project pin: the conflict check and this write are not atomic. Protect the read/check/write with a file lock or add an atomic compare-and-set so a second enrollment reports a conflict instead of overwriting the first link.</violation>
<violation number="2" location="packages/cli/src/cli/lib/enrollment-pin.ts:66">
P2: linkEnrolledNodeToProjectPin links the freshly enrolled nodeId to the existing pin (`writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId })`) without verifying that the node's enrollment workspace actually matches `session.workspaceKey`. If the project pin currently holds a different workspace's key (e.g. workspace A) and `cloud enroll` redeems a token for workspace B, this silently links B's node id onto A's key. Subsequent `node up` runs then resolve B's node credentials by id while other project commands keep resolving workspace A from the same pin file — yet the command reports the link as successful (`status: 'linked'`). Consider validating the enrollment's workspace against the pinned workspace key before linking, or reporting a mismatch instead of linking.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return { status: 'conflict', nodeId, pinnedNodeId: session.enrolledNodeId, pinPath }; | ||
| } | ||
|
|
||
| writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId }); |
There was a problem hiding this comment.
P2: Concurrent enrollments can still silently repoint a project pin: the conflict check and this write are not atomic. Protect the read/check/write with a file lock or add an atomic compare-and-set so a second enrollment reports a conflict instead of overwriting the first link.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/cli/lib/enrollment-pin.ts, line 66:
<comment>Concurrent enrollments can still silently repoint a project pin: the conflict check and this write are not atomic. Protect the read/check/write with a file lock or add an atomic compare-and-set so a second enrollment reports a conflict instead of overwriting the first link.</comment>
<file context>
@@ -0,0 +1,68 @@
+ return { status: 'conflict', nodeId, pinnedNodeId: session.enrolledNodeId, pinPath };
+ }
+
+ writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId });
+ return { status: 'linked', nodeId, pinPath };
+}
</file context>
There was a problem hiding this comment.
Declined, with reasoning.
The read/check/write is genuinely not atomic, so the description is accurate. What I dispute is the consequence:
writeProjectWorkspaceKeyis already write-to-temp-then-rename, with a per-write nonce andwxexclusive creation. The pin file itself cannot be torn or partially written by a concurrent writer.- The only thing the race can lose is a conflict report. Interleaved as read(A)/read(A)/write(B)/write(C), both runs see an unlinked pin, both link, last writer wins — the file is valid and names one real node. No corruption, no partial state.
- Reaching that interleaving requires two
cloud enrollruns in the same project within the same few milliseconds, each holding its own one-time enrollment token, since a token is consumed on redemption. This is an interactive, operator-initiated command, not something a daemon or a loop drives.
A lockfile for that is disproportionate, and it adds a real failure mode this command must not have: enrollment runs after the one-time token is already burned, so a stale lock left by a killed process would block the pin update on every subsequent enroll, in a code path whose whole design constraint is that nothing after redemption may fail. The current code already treats every pin failure as report-and-continue for that reason.
If concurrent enrollment ever becomes a real workflow, the right fix is a compare-and-set inside writeProjectWorkspaceKey — one atomic primitive shared by every writer of that file, including workspace switch and the broker's own pin write at broker-lifecycle.ts:1583 — rather than a lock around one caller. Noted on #1440, which already has to revisit that write path.
| return { status: 'conflict', nodeId, pinnedNodeId: session.enrolledNodeId, pinPath }; | ||
| } | ||
|
|
||
| writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId }); |
There was a problem hiding this comment.
P2: linkEnrolledNodeToProjectPin links the freshly enrolled nodeId to the existing pin (writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId })) without verifying that the node's enrollment workspace actually matches session.workspaceKey. If the project pin currently holds a different workspace's key (e.g. workspace A) and cloud enroll redeems a token for workspace B, this silently links B's node id onto A's key. Subsequent node up runs then resolve B's node credentials by id while other project commands keep resolving workspace A from the same pin file — yet the command reports the link as successful (status: 'linked'). Consider validating the enrollment's workspace against the pinned workspace key before linking, or reporting a mismatch instead of linking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/cli/lib/enrollment-pin.ts, line 66:
<comment>linkEnrolledNodeToProjectPin links the freshly enrolled nodeId to the existing pin (`writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId })`) without verifying that the node's enrollment workspace actually matches `session.workspaceKey`. If the project pin currently holds a different workspace's key (e.g. workspace A) and `cloud enroll` redeems a token for workspace B, this silently links B's node id onto A's key. Subsequent `node up` runs then resolve B's node credentials by id while other project commands keep resolving workspace A from the same pin file — yet the command reports the link as successful (`status: 'linked'`). Consider validating the enrollment's workspace against the pinned workspace key before linking, or reporting a mismatch instead of linking.</comment>
<file context>
@@ -0,0 +1,68 @@
+ return { status: 'conflict', nodeId, pinnedNodeId: session.enrolledNodeId, pinPath };
+ }
+
+ writeProjectWorkspaceKey(dataDir, session.workspaceKey, { enrolledNodeId: nodeId });
+ return { status: 'linked', nodeId, pinPath };
+}
</file context>
There was a problem hiding this comment.
Partly accepted, and the overclaim is fixed in d40a3f013.
The diagnosis is right: nothing here verifies that the enrollment's workspace matches the pinned key, and it cannot — the enrollment store records workspace ids while the pin records a key, with no local mapping between them. That unreconciled gap is called out as the root cause in #1432 and is what #1440/#1442 depend on closing.
Where I did not follow the recommendation: "leave the pin unlinked with a warning unless verifiable" would mean never linking, since it is never locally verifiable — and that removes the fix for the case actually reported. In #1432 the repo was pinned to an auto-provisioned throwaway workspace (204337648549896192) and the user enrolled into rw_7ccfea89 specifically so node up would serve the enrolled node. Refusing to link leaves that user exactly where they started. (The throwaway pin itself is the separate bug in #1440.)
What is fixed is the part I agree was wrong — the command claiming a link it had not verified. The message now names the workspace that will actually be served and states plainly what was not checked:
Linked this project's workspace pin (…) to node …, so 'relay node up' here serves this enrollment in workspace rw_123. The pinned workspace key was not verified against that workspace — if they differ, agent commands in this project keep using the pinned one.
Test cloud enroll links the enrolled node to this project workspace pin now asserts both the served workspace id and the caveat, and fails against the previous message.
Codex and cubic both landed the same finding on the first cut, and they were right: preserving `enrolledNodeId` unconditionally in `persistWorkspaceSession` recreated the split it was meant to remove. `node up` resolves an enrollment by node id alone and applies its credentials *without* applying the pinned key, so a project that switched from workspace A to B would run the broker as A's node while every other command read B. The enrollment store holds workspace ids and the pin holds a key, so the two cannot be reconciled locally. What can be compared is key against key: - Re-selecting the same workspace keeps the enrolled node (the original fix — dropping it manufactured the pin `node up` warns about). - Moving to a different workspace clears it, and `workspace switch|join` now says so instead of dropping it silently. - `cloud enroll` still links the pin, but no longer implies the link was verified: it names the workspace that will actually be served and states that the pinned key was not checked against it. Declined cubic's P2 on read/check/write atomicity in `linkEnrolledNodeToProjectPin`: `writeProjectWorkspaceKey` is already write-then-rename, two concurrent `cloud enroll` runs in one project would each need their own one-time token, and the race can only lose a conflict *report*, never corrupt the pin. A lockfile is disproportionate here. Refs #1432 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes CodeRabbit's docstring-coverage pre-merge warning on the one undocumented function in a file this branch already touches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/cli/commands/workspace.ts`:
- Around line 20-32: Update the workspace create flow around
persistWorkspaceSession to retain its PersistWorkspaceSessionResult and pass it
through the command’s structured JSON output. When clearedEnrolledNodeId is
present, include that node ID and a relay cloud enroll recovery instruction in
JSON-safe result fields, while preserving valid JSON for all workspace create
responses.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1023c1e6-20cb-4833-9fa2-5f14588b2d3e
📒 Files selected for processing (6)
packages/cli/src/cli/commands/cloud.test.tspackages/cli/src/cli/commands/cloud.tspackages/cli/src/cli/commands/workspace.test.tspackages/cli/src/cli/commands/workspace.tspackages/cli/src/cli/lib/workspace-session.test.tspackages/cli/src/cli/lib/workspace-session.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/cli/commands/cloud.ts
- packages/cli/src/cli/commands/cloud.test.ts
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/cli/lib/workspace-session.ts">
<violation number="1" location="packages/cli/src/cli/lib/workspace-session.ts:80">
P2: Changing to a different project key clears `enrolledNodeId`, but MCP `set_workspace_key`/`create_workspace` and CLI `workspace create` discard the returned `clearedEnrolledNodeId`; these flows report success while the old fleet enrollment is shadowed until a later `node up` warning. Consuming this result in every pin-changing caller, with a structured warning where JSON/MCP output is required, would keep the clearing behavior visible.
(Based on your team's feedback about Reconcile Node IDs by Workspace Key (2026-08-06).)</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| switchWorkspace(name, options.env); | ||
| } | ||
|
|
||
| return existing?.enrolledNodeId && !enrolledNodeId |
There was a problem hiding this comment.
P2: Changing to a different project key clears enrolledNodeId, but MCP set_workspace_key/create_workspace and CLI workspace create discard the returned clearedEnrolledNodeId; these flows report success while the old fleet enrollment is shadowed until a later node up warning. Consuming this result in every pin-changing caller, with a structured warning where JSON/MCP output is required, would keep the clearing behavior visible.
(Based on your team's feedback about Reconcile Node IDs by Workspace Key (2026-08-06).)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/cli/lib/workspace-session.ts, line 80:
<comment>Changing to a different project key clears `enrolledNodeId`, but MCP `set_workspace_key`/`create_workspace` and CLI `workspace create` discard the returned `clearedEnrolledNodeId`; these flows report success while the old fleet enrollment is shadowed until a later `node up` warning. Consuming this result in every pin-changing caller, with a structured warning where JSON/MCP output is required, would keep the clearing behavior visible.
(Based on your team's feedback about Reconcile Node IDs by Workspace Key (2026-08-06).) </comment>
<file context>
@@ -56,4 +76,8 @@ export function persistWorkspaceSession(options: PersistWorkspaceSessionOptions)
switchWorkspace(name, options.env);
}
+
+ return existing?.enrolledNodeId && !enrolledNodeId
+ ? { clearedEnrolledNodeId: existing.enrolledNodeId }
+ : {};
</file context>
There was a problem hiding this comment.
Valid, fixed in 9ab642809. CodeRabbit landed the workspace create half of this independently; your version named the MCP callers too, and all three are now covered.
workspace create→clearedEnrolledNodeId+warninginside its JSON payload, so the report cannot break a parsing caller.- MCP
create_workspaceandset_workspace_key→ returned through thewarningfield both tools already carried for persistence failures, which is the structured path you asked for.
One shared describeClearedEnrollment supplies the wording to all of them, so the CLI and the MCP tools cannot describe the same event differently. Both test files bind the real implementation via importOriginal instead of a stand-in.
New regression tests fail against the discard-the-result version: workspace create reports a dropped enrolled node inside its JSON output and, on the MCP side, reports an enrolled fleet node dropped by joining another workspace.
CodeRabbit and cubic caught the same gap in the previous commit: only `workspace switch|join` consumed the new `clearedEnrolledNodeId`. The other three writers of the pin discarded it and reported success while the project's fleet enrollment was dropped, leaving the next `node up` warning as the first mention — the same silence this branch exists to remove. - `workspace create` carries it in its JSON output (`clearedEnrolledNodeId` plus a `warning`), so the report cannot break a parsing caller. - MCP `create_workspace` and `set_workspace_key` return it through the `warning` field they already had for persistence failures. - `describeClearedEnrollment` is now the single wording shared by all of them, and both test suites bind the real implementation via `importOriginal` rather than a stand-in, so output cannot drift past the assertions. `workspace create` is the sharpest case: a freshly minted key never matches an existing pin, so it always clears. Refs #1432 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The previous commit made `create_workspace` and `set_workspace_key` return a cleared-enrollment message through their existing `warning` field, but left the tool descriptions saying `warning` appears only when persistence failed. An MCP consumer reading that would take a successful create that dropped an enrolled node for a failed save — and a fresh key never matches an existing pin, so that case fires on every create over one. Both descriptions now name both cases and say the text distinguishes them. Refs #1432 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/cli/agent-relay-mcp.ts`:
- Around line 504-506: Track persistence failure independently in the
set_workspace_key flow around persistWorkspaceSession and
describeClearedEnrollment: use persistedMessage after a successful write,
appending the cleared-enrollment warning, while retaining the failure message
only when persistence fails. Update
packages/cli/src/cli/agent-relay-mcp.startup.test.ts lines 582-596 to assert the
cleared-enrollment response states that the key persisted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7df30b64-f237-43d5-a84c-1743b5e68f17
📒 Files selected for processing (5)
packages/cli/src/cli/agent-relay-mcp.startup.test.tspackages/cli/src/cli/agent-relay-mcp.tspackages/cli/src/cli/commands/workspace.test.tspackages/cli/src/cli/commands/workspace.tspackages/cli/src/cli/lib/workspace-session.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/cli/commands/workspace.test.ts
| // Joining a different workspace drops this project's enrolled fleet | ||
| // node; surface that here instead of at the next `node up`. | ||
| persistenceWarning = describeClearedEnrollment(persistWorkspaceSession({ workspaceKey: key })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep cleared enrollment separate from persistence failure.
A cleared enrollment means persistWorkspaceSession succeeded. The current persistenceWarning branch selects activeMessage, so set_workspace_key does not state that the key persisted despite the contract documented at Line 468.
packages/cli/src/cli/agent-relay-mcp.ts#L504-L506: Track persistence failure separately. UsepersistedMessageplus the cleared-enrollment warning after a successful write.packages/cli/src/cli/agent-relay-mcp.startup.test.ts#L582-L596: Assert that the cleared-enrollment response also says the key persisted.
Proposed fix
+ let persistenceFailed = false;
let persistenceWarning: string | undefined;
try {
persistenceWarning = describeClearedEnrollment(persistWorkspaceSession({ workspaceKey: key }));
} catch (error) {
+ persistenceFailed = true;
// existing error message assignment
}
- const message = persistenceWarning ? `${activeMessage} ${persistenceWarning}` : persistedMessage;
+ const message = persistenceWarning
+ ? `${persistenceFailed ? activeMessage : persistedMessage} ${persistenceWarning}`
+ : persistedMessage;Based on PR objectives: MCP tools must report cleared enrollment while preserving their output contract.
📍 Affects 2 files
packages/cli/src/cli/agent-relay-mcp.ts#L504-L506(this comment)packages/cli/src/cli/agent-relay-mcp.startup.test.ts#L582-L596
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/cli/agent-relay-mcp.ts` around lines 504 - 506, Track
persistence failure independently in the set_workspace_key flow around
persistWorkspaceSession and describeClearedEnrollment: use persistedMessage
after a successful write, appending the cleared-enrollment warning, while
retaining the failure message only when persistence fails. Update
packages/cli/src/cli/agent-relay-mcp.startup.test.ts lines 582-596 to assert the
cleared-enrollment response states that the key persisted.
1 participant
Fixes the top three items in #1432. The remaining four are deferred to follow-up issues (listed below) rather than piled into one PR.
The bug
A project pin at
<repo>/.agentworkforce/relay/workspace-key.jsonwithout anenrolledNodeIdmaderelay node upskip the fleet enrollment store entirely —packages/cli/src/cli/commands/node.ts:125was a bareif (session) return undefined;with no output, while the sibling branch at:147-149warned. No credentials, no heartbeat, no message. The Cloud dashboard showed the node,fleet nodesrun from that repo showed a different roster, and nothing said the two were different workspaces. One machine was enrolled but invisible for five days.What changed
1.
node upwarns before returning (commands/node.ts) — highest value, lowest risk. The warning names how many stored enrollments are being ignored and how to recover. It is gated on the store actually holding enrollments, so a plain pinned project with no fleet enrollment stays quiet; a store that cannot be read still warns (without a count) and still starts, because nothing on this path needs it.2.
cloud enrollrecords the node on the pin (commands/cloud.ts+ newlib/enrollment-pin.ts) — previously it wrote onlyfleet-enrollments.json, so "enrolled into A while pinned to B" was silently reachable. A pin that already names a different node is reported and left untouched: the one-time enrollment token is already redeemed by the time this runs, so silently repointing a pin would trade one invisible mismatch for another. Every pin failure is reported and swallowed — a redeemed enrollment is never failed by a pin problem, and--jsonstdout stays parseable.3.
workspace switch|joinpreservesenrolledNodeId(lib/workspace-session.ts) — it calledwriteProjectWorkspaceKeywith no options, manufacturing exactly the pin state fix 1 warns about.Deferred, with reasons
Scoped out to keep this reviewable; all four are real and none are regressions introduced here:
broker-lifecycle.ts:1327-1330/:1583-1586) — touches broker startup and pin-overwrite ordering, not CLI argument plumbing. Wants its own change with its own broker-level test.fleet statusmatches roster by name, notnodeId) — separate command, separate matching bug.--workspace-key+ enrollment footgun) — a second warning site; deliberately not bundled so item 1's warning can be evaluated on its own.node_name_conflictrename loop needs--broker-name) — Rust broker behavior (node_control), not CLI.Test bar
Every regression test here was run against the unfixed source and fails there. Mutations applied one at a time, each reverting exactly one fix:
node.tsreverted toorigin/mainworkspace-session.tsreverted toorigin/maincloud.tsreverted toorigin/mainenrollment-pin.tsbody reduced to pre-fix no-opThe silent case is asserted specifically:
warns that a project pin without an enrolled node id is shadowing stored enrollmentsasserts the warning text is emitted (toHaveBeenCalledTimes(1)plus content), not merely that behavior changed. Against the unfixed blob it fails withexpected "vi.fn()" to be called 1 times, but got 0 times.The tests that pass in both directions are guards, not regression tests —
stays quiet when a project pin shadows nothing,does not warn ... when the pin names an enrolled node,does not invent an enrolled node id,leaves an unpinned project alone.Gates
npm run typecheck— exit 0npx vitest run— 1754 passed, 16 skipped, 5 failednpm run lint— 0 errors (76 pre-existing warnings; none in the files touched here)npm run format:check— cleanThe 5 failures are pre-existing on
origin/mainand unrelated (telemetry machine-id / auth headers). Verified by stashing this branch and re-running the same three files on a clean tree: identical5 failed | 71 passed. Files:telemetry/client.test.ts(2),agent-relay-mcp.startup.test.ts(2),packages/cloud/src/auth.test.ts(1).Not covered
No live re-enrollment against Cloud was performed — that needs a one-time enrollment token and would burn it. The behavior is covered at the unit level on both the pin lib (real temp-dir filesystem) and the command wiring.
Refs #1432
🤖 Generated with Claude Code