fix(workspace): make resource authority explicit - #6287
Conversation
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen Thank you for the substantial workspace-authority hardening—the persisted scope, fresh verification, and single-writer direction are thoughtful. I found three reproducible main-path gaps: the BYOK tool loop bypasses the same write boundary, several newly guarded UI/CLI consumers cannot carry the required authority, and the creator-only access change reopens the existing extracted-project move-to-Personal regression. pnpm guard, pnpm typecheck, and 177 focused daemon/web assertions passed, but the existing move regression spec now deterministically returns 403. I’m requesting changes for these focused issues; the overall direction looks strong once the remaining paths share the same authority contract.
…am-resource-explicit-workspace-scope # Conflicts: # apps/web/src/collab/useProjectCollab.ts
…am-resource-explicit-workspace-scope
|
@mrcfps Final head is |
mrcfps
left a comment
There was a problem hiding this comment.
@lefarcen Thank you for the substantial follow-up work here. I verified that the previous BYOK media authorization, orphan-recovery, export/media polling, and pinned run-scope issues are addressed on this head. One clear CLI-parity blocker remains: several project commands still cannot carry the explicit Workspace/member identity to routes this PR now correctly fail-closes, so the Team-project creator cannot use those primary CLI paths. Please finish the same centralized transport migration for the remaining commands and cover them in the existing creator/non-creator/unbound matrix.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
mrcfps
left a comment
There was a problem hiding this comment.
Re-review complete on the final head. The explicit Workspace transport blocker is addressed across conversation/chat, plugin, and Figma commands with the 96/96 creator/non-creator/unbound matrix green. The first-share conversation recovery follow-up has red/green coverage for lifecycle generations, A→B→A authority reuse, concurrent completion signals, and non-404 error truth. Root typecheck, guard, daemon full tests, Web 575-file full tests/build, and real two-account headed-Chrome validation are green. No unresolved review threads remain.
Why
Real two-account validation exposed several cross-Workspace races in the Team
workspace feature. Resource reads and mutations mixed persisted project
bindings with daemon-global current/default Workspace state, so switching tabs,
restarting a shared project, or opening it from another account could select the
wrong authority, writer, or wallet.
The visible failures included funded Team projects being checked against a
Personal balance, the project creator reopening as a read-only viewer,
deep-linked conversations disappearing during hydration, presence writes
feeding a heartbeat storm, and Workspace owners being able to mutate or unshare
another member's project. Repeated file-list/preview navigation also discarded
warm preview state and repeatedly fetched the same raw HTML.
What users will see
stay bound to the exact Workspace/member identity that initiated them.
Other active Team members, including Workspace owners/admins, can view and
comment but cannot edit files, start runs, rename/delete, or move that project
to Personal.
the artifact. The panel is not locked: the viewer can expand it, and
collaboration refreshes do not collapse it again during that visit.
settles instead of loading forever, and presence refreshes stay on the hot
path without an event/write heartbeat loop.
exact HTML source instead of flashing a loading state or refetching it.
existing behavior.
Surface area
hydration, preview caching, and presence behavior in
apps/webodcommand paths and explicitVela/AMR runtime scope propagation
run, and SSE behavior
skills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolpackage.jsonthe persisted project/Workspace identity; read-only shared projects initially
collapse chat
Screenshots
Headed-Chrome captures were taken for both roles:
controls
view-and-comment-only state, default-collapsed but manually expandable chat,
no file creation/upload controls, and disabled run/chat mutation path
The captures contain real test-account display names, so they are retained as
private acceptance evidence rather than published on this public PR.
Bug fix verification
Representative red specifications:
apps/daemon/tests/collab/project-request-authority.test.tsapps/daemon/tests/routes/workspace-projects.test.tsapps/daemon/tests/run-create-workspace-gate.test.tsapps/web/tests/use-collab.test.tsxapps/web/tests/use-project-collab.test.tsxapps/web/tests/components/ProjectView.tabs-navigation.test.tsxapps/web/tests/components/ProjectView.sharedTitleRefresh.test.tsxapps/web/tests/components/FileViewer.test.tsxThe new specs were confirmed red against the PR's pre-fix base and green after
the fixes. This PR targets
feat/workspace-team, so that branch is theapplicable pre-fix baseline.
Real two-account checks also reproduced the previous authorization split: a
non-project-owner Workspace owner could write a file and unshare the project
before authority was unified.
Validation
Exact final integration head:
c0becbf5ebaf6e5798fa9f1b877943535a7152eapnpm guardpnpm typecheckpnpm --filter @open-design/daemon typecheckpnpm --filter @open-design/daemon buildpnpm --filter @open-design/daemon testpnpm --filter @open-design/web typecheckpnpm --filter @open-design/web buildpnpm --filter @open-design/web test98 ProjectView tests, including lifecycle generation fences, A to B to A
authority races, concurrent materialization signals, and 404 to 403/500
error truth.
the creator share request was still in flight. The page recovered without a
manual reload, kept the exact project title/file/conversation, and enforced
view-and-comment-only controls with no sticky conversations 404.
across multiple heartbeat intervals, and could be collapsed/reopened again
downstream Workspace requests
returned 403
Workspace balance did not
collaborator reported
canEdit: falseloggedIn:falseAMR balance dialog
Adjacent issues
The signed-out run exposed the existing empty-final-output quota
misclassification. It is already addressed separately by #6261 and is not
duplicated here.