Accept a workspace passed by name off the live list (fixes #317) - #318
Merged
Conversation
…#317) `validateWorkspaceId` tested the name and the id against different lists. The name was only ever matched against `knownWorkspaces` — the boot-time SchemaData snapshot, which this file's own comment notes is empty whenever SQLite was not query-ready at agent registration — while the id was matched against the live `listWorkspaces()`. The error message was already built from the live list (#311), so on exactly the vaults #311 targeted the guard could only ever suggest a name it had never checked for acceptance, producing a self-refuting rejection: Invalid workspace "Desenvolvedor". Closest match: "Desenvolvedor". … That contradicted the other half of the mandatory pair: getTools grounds the caller with the live names and says to pass one verbatim, and useTools then answered "do not infer a workspace name from the user's wording". An agent following the grounding perfectly was told it invented the name. Every by-name envelope was unusable on such a vault until callers switched to UUIDs — the opposite of the grounding #311 introduced. Match id OR name against the live list in one pass, so acceptance and suggestion read the same source. The snapshot fast path stays (it avoids the async lookup where the snapshot is populated) but is now documented as accept-only: a miss falls through to the live list, so a stale or empty snapshot can never reject a real workspace. Reported with a diagnosis by @gcp007-ops; verified independently against the code rather than applied as given. Tests: tests/unit/EnvelopeWorkspaceValidation.test.ts, 7 cases, RED verified first — the name cases failed while the id case and the legitimate rejection passed, pinning the defect to the by-name path. The last case is a self-contradiction lock: for every live workspace, a rejection may never name that same value as its closest match, so any future divergence between the accept list and the suggest list fails there in either direction. That lock plus the rejection case also caught a fail-open regression mid-fix, when a dropped `listWorkspaces()` line sent a ReferenceError into the catch and made the guard accept everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016dqXjmfKkH27qMhVAtS6JH
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #317.
The bug
validateWorkspaceIdtested the name and the id against different lists:This file's own comment, three lines below, notes that the snapshot "is taken at boot and is empty whenever SQLite was not ready then" — which is exactly why #311 moved the error message to the live list. The acceptance check never moved with it.
So on the vaults #311 was written for, the guard could only ever suggest a name it had never checked for acceptance, and the rejection refuted itself:
That also put the two halves of the mandatory pair in contradiction:
getToolsgrounds the caller with the live names and instructs it to pass one verbatim, thenuseToolsanswers "do not infer a workspace name from the user's wording." An agent following the grounding perfectly was told it invented the name, and every by-name envelope stayed unusable until callers were rewritten to pass UUIDs — the opposite of what #311 introduced.Fix
Match id or name against the live list in one pass, so acceptance and suggestion read the same source.
The snapshot fast path stays — it avoids the async lookup on vaults where the snapshot is populated — but is now documented as accept-only: a miss falls through to the live list rather than rejecting, so a stale or empty snapshot can never reject a real workspace. That asymmetry is what keeps it from becoming a second source of truth again, and there's a comment saying so.
Tests
tests/unit/EnvelopeWorkspaceValidation.test.ts, 7 cases, RED verified before the fix: the name cases failed while the id case and the legitimate rejection passed, which pins the defect to the by-name path rather than to the guard as a whole. Removing the name clause afterwards puts the same 4 back to red, confirming it is load-bearing.Two cases beyond the reported repro:
That lock plus the rejection case earned their keep during this change: a dropped
listWorkspaces()line mid-fix sent aReferenceErrorinto the enclosingcatchand made the guard fail open, accepting everything. Three of the acceptance tests went green for the wrong reason; the rejection case caught it. Worth keeping in mind for this guard generally — a fail-open regression is indistinguishable from a fix if only the happy path is asserted.Verification
tsc --noEmitclean,eslintclean, full suite 4257 passed. The single failure is the pre-existingLocalCliInstaller"namesake command earlier on PATH" case, unrelated and untouched.Credit
Reported with an accurate diagnosis and a proposed patch by @gcp007-ops. The fix here was verified independently against the code rather than applied as given, and lands as a single combined id-or-name lookup instead of a second
find.Generated by Claude Code