Skip to content

refactor(harness): extract retained workspace scope owners (SAP-3091) - #909

Open
ynadge wants to merge 1 commit into
yashnadge/sap-3090-delete-browser-graph-topologyfrom
yashnadge/sap-3091-extract-workspace-scope-owners
Open

refactor(harness): extract retained workspace scope owners (SAP-3091)#909
ynadge wants to merge 1 commit into
yashnadge/sap-3090-delete-browser-graph-topologyfrom
yashnadge/sap-3091-extract-workspace-scope-owners

Conversation

@ynadge

@ynadge ynadge commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix
  • Documentation
  • Feature
  • Tests
  • Dependency update
  • Maintenance or refactor

Problem and motivation

Workspace identity, allowed-root resolution and per-agent invocation types were still owned by mixed legacy graph modules. Deleting those modules directly would remove contracts used by current Studio discovery and Canvas.

Summary and scope

Extract workspace scope types and local identity helpers into shared/workspace-scope.ts, the catalog into core/workspace-scope-catalog.ts, and containment helpers into core/workspace-path.ts. Move AgentInvocationMode to Canvas ownership. Repoint live imports, browser aliases and the existing public type export; temporary legacy re-exports keep this boundary buildable.

Related work

Related issue or discussion: SAP-3091. Part of the authorized cleanup stack above #892 and #893, targeting main. Human review follows the complete stack. Implementation and PR creation do not authorize merging or releasing it.

Validation

Verification on the cumulative implementation at cab477b5 (each earlier boundary also passed its affected build/typecheck and focused checks):

pnpm build — passed
pnpm typecheck — passed
pnpm lint — passed (existing warnings)
pnpm test — stops at the unchanged agent-core unreadable-directory test on this VM
pnpm --filter @sapiom/harness exec vitest run --maxWorkers=2 — 3,840 passed, 2 skipped
pnpm --filter @sapiom/harness test:perf — 10 passed
pnpm -r --filter @sapiom/mcp --filter @sapiom/cli --filter @sapiom/harness-desktop test — passed (179 + 73 + 205 cases; 3 MCP skips)
pnpm terminology:check — passed
pnpm provider-copy:check — passed
pnpm examples:check:test — 178 passed
pnpm pr-labeler:check — passed
pnpm pr-ci-security:check — passed

The root test failure reproduces on unchanged prior code; it is not treated as a green root run. The final retirement record includes artifact hashes, request-observation scope and all local limitations.

pnpm --filter @sapiom/harness test:ui --config /home/vercel-sandbox/sapiom/.context/sap-3089/playwright.config.mjs --workers=3 — 625/627 passed; two Chrome target crashes
pnpm --filter @sapiom/harness test:ui --config /home/vercel-sandbox/sapiom/.context/sap-3089/playwright.config.mjs --workers=2 template-harness — full affected file passed 23/23 unchanged
pnpm --filter @sapiom/harness test:canvas --config /home/vercel-sandbox/sapiom/.context/sap-3090-3091/playwright-canvas.config.mjs --workers=2 — 15 passed
node packages/harness-desktop/scripts/pack.mjs --linux — passed with copied dependencies and output redirected to task storage
bash packages/harness-desktop/scripts/smoke.sh — fresh linux-unpacked artifact passed 16 checks, 1 Windows-only skip
AppImage --appimage-extract-and-run --smoke --no-sandbox under Xvfb and the same isolated smoke setup — passed 16 checks, 1 skip on isolated rerun

Browser checks used installed Google Chrome, isolated TMPDIR and E2E_PORT=5497; the override changes only the executable, temporary output/report paths and Vite invocation. The package's 195 Harness runtime/assets match the clean build byte-for-byte. Its saved-map check records legacy read/refresh/navigation requests 0/0/0, direct removed routes 404/404/404, and unchanged map/history.

The first AppImage run failed session-create with a preexisting PROJECT_SESSION_SCOPE_UNAVAILABLE retention race; the identical artifact passed an isolated rerun. The initial failure and unchanged source ancestry are preserved in the record, with a separate follow-up test/fix outline. This cleanup does not fix that race. Signed macOS installer/upgrade evidence remains the SAP-3086 release gate.

Tests and documentation

Moved retained catalog, local-key and source-containment cases to their new owners and added path-boundary coverage. All 154 focused cases across 11 files passed at this boundary, including project preferences/catalog, per-agent Canvas, discovery freshness, rescan and wiring. Public @sapiom/agent PackageInventory source and export are unchanged.

Compatibility and release impact

  • Breaking or externally visible changes: Retained public WorkspaceScopeSummary and PackageInventory shapes are unchanged. This is an internal ownership move with no runtime migration.
  • Changeset: N/A — internal module/type moves preserve the existing public API and ship with the surrounding cleanup changesets.

Security

  • I have not included secrets, credentials, private data, or unsanitized logs.
  • This pull request does not publicly disclose a suspected vulnerability. I will follow the Security Policy for private reporting.

AI assistance

  • I did not use AI assistance for this change.
  • I used AI assistance and have described it below.

Codex implemented the cleanup and test/doc migrations, traced retained callers, and checked the resulting diffs. Automated checks above and independent read-only review provide implementation evidence; maintainer review is still required.

Checklist

  • I read CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.
  • This pull request addresses one focused problem and contains no unrelated cleanup.
  • I added or updated tests, or explained above why tests are not applicable.
  • I ran the relevant build, typecheck, lint, and test commands, or explained any N/A checks above.
  • I updated documentation for user-facing changes, or marked it N/A above.
  • I added a Changeset for a published-package change, or explained why it is not applicable.
  • I can explain and maintain every submitted change, including any AI-assisted work.

@ynadge
ynadge added this pull request to stack #894 September 9, 2026 23:15
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: fa75f7c3-d35f-4aeb-b829-5a7b98847b7c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c4b719 and 67337e6.

📒 Files selected for processing (31)
  • docs/plans/agent-studio-plan-first-agent-map/authority-retirement.md
  • packages/harness/src/core/canvas-interconnections.ts
  • packages/harness/src/core/studio-project-catalog.ts
  • packages/harness/src/core/studio-workspace-preferences.ts
  • packages/harness/src/core/system-graph-inventory.test.ts
  • packages/harness/src/core/system-graph-inventory.ts
  • packages/harness/src/core/system-graph.test.ts
  • packages/harness/src/core/system-graph.ts
  • packages/harness/src/core/workspace-path.test.ts
  • packages/harness/src/core/workspace-path.ts
  • packages/harness/src/core/workspace-scope-catalog.test.ts
  • packages/harness/src/core/workspace-scope-catalog.ts
  • packages/harness/src/index.ts
  • packages/harness/src/server/agent-map.ts
  • packages/harness/src/server/index.ts
  • packages/harness/src/server/rest.ts
  • packages/harness/src/shared/system-graph.ts
  • packages/harness/src/shared/types.ts
  • packages/harness/src/shared/workspace-scope.test.ts
  • packages/harness/src/shared/workspace-scope.ts
  • packages/harness/vitest.config.ts
  • packages/harness/web/src/App.tsx
  • packages/harness/web/src/components/ProjectTreeRows.tsx
  • packages/harness/web/src/components/WorkflowsRail.tsx
  • packages/harness/web/src/lib/agent-map.ts
  • packages/harness/web/src/lib/api.ts
  • packages/harness/web/src/lib/canvas-altitude.test.ts
  • packages/harness/web/src/lib/canvas-altitude.ts
  • packages/harness/web/src/lib/navigation-history.ts
  • packages/harness/web/tsconfig.json
  • packages/harness/web/vite.config.ts

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.

@ynadge
ynadge marked this pull request as ready for review September 10, 2026 03:33

@gwitwer gwitwer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. Compared the moved bodies against the originals: identical logic, identifier renames only. WorkspaceScopeSummary export repointed with an unchanged shape; the @shared alias is updated consistently across tsconfig, vite and vitest; the removed test cases are exactly the ones re-homed into the three new files.

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.

2 participants