Skip to content

feat(sdk): wire workspace:/tools.fs: scope-compiler into preflight (#308) - #359

Merged
kjgbot merged 2 commits into
mainfrom
feat/spec-H-path-scoped-auth
Sep 12, 2026
Merged

kjgbot merged 2 commits into
mainfrom
feat/spec-H-path-scoped-auth

Conversation

@kjgbot

@kjgbot kjgbot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Closes #308. Builds on #329's scope-compiler + mount-registry foundation, which was shipped unwired.

What lands

  • FlowSpec.workspace?: string | string[] and FlowSpec.tools?.fs?: string | string[] — flow-header scope grants ("mount/path: readonly|readwrite|append")
  • validate.ts accepts the two new fields (closed vocabulary, strict scope-grant shape check)
  • compile.ts preserves them in the CompiledFlowSpec, and toKernelSpec drops them (kernel dialect stays unchanged — grants are preflight-only)
  • preflight.ts calls compileScopes with readMountRegistry(projectSearchStart); three closed refusal kinds surface at gate 1:
    • scope_syntax_invalid — grant doesn't match mount/path: mode
    • mount_unknown — mount name not present in the manifest
    • scope_ungrantable — mount exists but cannot grant that path or mode
  • PREFLIGHT_FAILURE_KINDS extends with the three kinds; walker parity test adds scenarios so covenant 2's closed-vocabulary check still holds
  • tests/scope-preflight.test.ts — 6 acceptance cases (accept, syntax refuse, mount_unknown, scope_ungrantable path, scope_ungrantable mode, tools.fs parity, absence)

Deferred (follow-ups)

  • Step-level AgentOptions.workspace widening (surface-side) — step-level grants sit alongside flow-header grants once we settle whether they compose or override
  • Worker-session scope threading into the AgentWorker start payload — token minting still runs the same code, this just refuses ungrantable ones earlier
  • Slice-C's tools.mcp remains owned by that slice; this PR only touches tools.fs

Test plan

  • linux-x64-artifact — 27 preflight scenarios (up from 24) all pass locally
  • packed-consumer — dedicated scope-preflight.test.ts (6 cases) added to tsconfig.tests.json

Note

Medium Risk
Changes authorization preflight for filesystem scope grants and reads mount manifests from disk; incorrect behavior could block valid flows or miss bad grants before run.

Overview
Adds flow-header path-scoped grants via workspace and tools.fs ("mount/path: readonly|readwrite|append"), validated in validate.ts, preserved through compileSpec, and not lowered into the kernel dialect (preflight-only).

Preflight now runs compileScopes against relayfile.mounts.json from projectSearchStart, surfacing three refusal kinds: scope_syntax_invalid, mount_unknown, and scope_ungrantable. Unreadable manifests fail closed (unknown mounts).

Tests extend the closed refusal-kind walker and add scope-preflight.test.ts (accept, syntax, mount, ungrantable path/mode, tools.fs parity, absence).

Reviewed by Cursor Bugbot for commit 5c99a5c. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3758bc31-b095-4990-8819-7e2da6c3a940


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/sdk/src/step-fields.ts
Comment thread packages/sdk/src/validate.ts
@kjgbot

kjgbot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

maintainability lens — FAIL

Maintainability Review — PR #359

Blocker

scopeDiagnostics swallows every manifest error into a generic mount_unknown refusal (packages/sdk/src/preflight.ts:294-297). The bare catch { mounts = {} } collapses three distinct failure modes into one indistinguishable outcome:

  1. Manifest legitimately absent (also returns {} from readMountRegistry).
  2. JSON syntax error in relayfile.mounts.json (JSON.parse throws SyntaxError).
  3. Schema violation — readMountRegistry throws "Invalid relayfile mount manifest ..." or "Invalid relayfile mount "<name>" in ..." (mount-registry.ts:28-36), losing the path and the specific validation that failed.

The comment "fail closed — unreadable manifest treats every mount as unknown" describes the policy, but the author staring at mount_unknown: mount "acme" is not present… will never suspect that their manifest is malformed. Bind the error and either emit it as a dedicated refusal kind (mount_manifest_invalid, message including the caught error), or at minimum only swallow ENOENT here and let schema errors surface — readMountRegistry already handles ENOENT via directory-walking, so a thrown error here is always a real problem worth surfacing.

Concerns

  • No test covers the fail-closed catch path. tests/scope-preflight.test.ts never writes a malformed manifest, so the comment's claim is unenforced — regressions here will pass CI. Add one case with { "version": 2 } or "not json" at relayfile.mounts.json.
  • tools.mcp accepted as a reserved key but does nothing (validate.ts:180). No schema, no consumer, no test — it will rot. Either wire it or drop it; reserved slots with no owner become mystery keys during future edits.
  • workspace/tools never round-trip to the kernel. toKernelSpec (compile.ts:307-329) does not spread these fields, so authoring grants exist only for preflight. That may be intentional ("Compiled by preflight" in spec.ts:354), but the JSDoc should say "preflight-only, stripped at kernel boundary" — otherwise a maintainer will "fix" the missing propagation and quietly reshape the boundary contract.
  • ScopeInput.steps is dead code in this PR. scope-compiler.ts:100-103 iterates flow.steps[].workspace, but no StepSpec field, validator, or test exposes step-level grants. Either add the surface or defer the code path until needed.
  • Silent-skip on missing projectSearchStart (preflight.ts:293). If the caller omits it, grants parse cleanly and no diagnostic warns the author that mount enforcement was skipped — a fragile assumption baked into every call site.

Notes

  • Validator.validateScopeGrants (validate.ts:226-234) returns on first bad grant; author sees one error per validation pass. compileScopes collects all — consider parity so batched authoring fixes aren't drip-fed.
  • scopeDiagnostics builds ScopeInput by copying workspace / tools.fs verbatim; step grants aren't threaded (matches the dead-code point above).

REVIEW_FAILED

@kjgbot

kjgbot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

history lens — PASS

Blockers: None under the three HISTORY criteria.

Notes: The change follows commit c5055b7b (#329), which explicitly deferred preflight integration and worker-session scope threading. Preserving declarations in packages/sdk/src/compile.ts:169–170 and invoking the existing compiler in packages/sdk/src/preflight.ts:216,288–311 advances that documented sequence. I found no reintroduction of a deliberately removed implementation.

The additions remain SDK-side and introduce no kernel vocabulary, provider adapters, replay mechanism, or gate-authority changes. They do not introduce a contradiction with RFC-0001 settled decisions #1, #5, or #13. The PR body explicitly defers worker-session enforcement and step-level composition; those omissions are not HISTORY blockers.

Commit a8fa834f describes wiring the existing compiler and extending refusal parity. The diff supports those statements: packages/sdk/tests/preflight.test.ts:413–416,469–491 adds scenarios for the three refusal kinds. The commit makes no test-pass or mutation-verification claim. I did not execute tests or validate the PR body’s local-pass assertion.

Concerns: In packages/sdk/src/preflight.ts:296–302, omitting projectSearchStart leaves mounts undefined. The existing compiler then skips mount availability checks, so the comment at lines 281–286 needs qualification: missing discovery input differs from a searched-for but absent manifest. Add coverage for both cases alongside packages/sdk/tests/scope-preflight.test.ts:35–53. This merits follow-up, but I found no previously fixed scope-preflight behavior that this branch reverses.

The older gate reference in ops/NEXT.md remains a brief-maintenance concern and does not affect this verdict.

REVIEW_PASSED

@kjgbot

kjgbot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

structure lens — MISSING

@kjgbot

kjgbot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

🎯 review-swarm: FAILED (M:fail H:pass S:missing)

Lens transcripts posted as sibling comments above.

kjgbot added 2 commits September 12, 2026 12:18
)

Consumes the scope-compiler and mount-registry from #329. Flow-header
declarations compile against the nearest relayfile.mounts.json; preflight
refuses with scope_syntax_invalid / mount_unknown / scope_ungrantable before
any token is minted. Walker parity extended for the three new kinds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
The scope-compiler wiring in this PR adds workspace and tools to FLOW_FIELDS;
update the pinned expectation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82

Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
@kjgbot
kjgbot force-pushed the feat/spec-H-path-scoped-auth branch from 4a2178f to 5c99a5c Compare September 12, 2026 10:18

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5c99a5c. Configure here.

let mounts: MountRegistry | undefined;
if (options.projectSearchStart !== undefined) {
try { mounts = readMountRegistry(options.projectSearchStart); }
catch { mounts = {}; /* fail closed — unreadable manifest treats every mount as unknown */ }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manifest errors reported as unknown mounts

Medium Severity

scopeDiagnostics catches every readMountRegistry failure and replaces it with an empty registry, so corrupt JSON, an invalid manifest, or a permission error all surface as mount_unknown. The registry already throws a path-specific reason, but that message is discarded and the grant is described as missing from the manifest.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5c99a5c. Configure here.

@kjgbot
kjgbot merged commit ee9f753 into main Sep 12, 2026
8 of 10 checks passed
@kjgbot
kjgbot deleted the feat/spec-H-path-scoped-auth branch September 12, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flows: path-scoped auth from workspace:/tools: — SURFACE §2 rule 3

1 participant