fix(backend): resolve every scoped resource through one visibility authority - #424
Merged
Merged
Conversation
…thority Follow-up to #423. The sub-agent fix left three paths deciding for themselves which Agents a Workspace may use, and they disagreed. `listScopedByIds` is now the single authority for "which of these references may this Workspace use?" — an ids-filtered `listScoped`, used by the Chat-turn sub-agent lookup, the save-time sub-agent check, and Kanban assignee validation. The Chat-turn lookup no longer hand-rolls the two scope queries, and the save-time check no longer lists every Agent in the Workspace to test two ids. Kanban assignee validation accepted only workspace-scoped Agents while the assignee picker offers every Agent visible in the Workspace, so assigning an attached Shared Agent — one that can run here — failed with "Invalid agent assignee". The Agent Discovery tool set resolved at the wrong scope in both directions: listAgents, getAgent and the MCP list were workspace-only, so an Operator working through an Agent could not see a Shared resource attached to their Workspace, while listModelProviders offered every Shared Provider in the Organization including unattached ones, which produce an Agent that cannot run. All four now resolve at the Workspace's scope and tag each row with it. The read side is also hardened against a row carrying both scope columns — the XOR is a write-time schema refinement, not a database constraint. `resolveScoped` now classifies by the scope a row actually carries instead of by elimination, the org-scoped queries require a null workspace, and `findNonSharedReferences` counts a reference as Shared only if it is org-scoped and at no Workspace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review follow-ups, all one theme: the Chat turn's four remaining resource lookups each carried their own copy of the visibility rule, and two create routes let the request body decide a row's scope. The Chat-turn Agent, Provider, Skill and MCP lookups now go through the Scoped-resource read module like the sub-Agent one, deleting three near-identical copies of the rule and the local attachment helper. Each copy treated "has an organizationId, has no workspaceId" as the definition of Shared, so a row carrying both columns resolved in a Workspace that neither owned nor attached it. Such a row was reachable: the workspace Provider and MCP create routes spread the request body into the insert, and their create schemas accept both scope columns while dropping the XOR refinement. A caller could name another Workspace, or set organizationId and mint a Shared resource from the Workspace surface — something only an Org Admin may do. Both routes now take the scope from the route, as the Agent and Skill routes already did. Also from review: the Promotion guard reuses the exported Shared-row predicate rather than keeping a private copy, `listScoped` gains the optional id filter instead of delegating to a private helper, and three tests that passed against the old code now assert the Attachment join and the id filter that make the new behaviour different. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Follow-up to #423, which fixed the sub-agent lookup but left the same rule reimplemented in several other places — and they disagreed with each other.
One authority for "may this Workspace use this?"
listScopedByIdsis the ids-filtered form oflistScoped: the Workspace's own rows, plus the Organization-scoped (Shared) rows attached to it (ADR-0007). The Chat turn's Agent, Provider, Skill, MCP and sub-Agent lookups, the save-time sub-agent check, and Kanban assignee validation all resolve through it now. Three near-identical copies of the rule and one local attachment helper are gone.Bugs this closes
A workspace-surface create could choose its own scope. The Provider and MCP create routes spread the request body straight into the insert, and their create schemas accept
organizationId/workspaceIdwhile dropping the XOR refinement thatproviderSchema/mcpSchemacarry. A caller with workspace access could name a different Workspace, or setorganizationIdand mint a Shared Provider or MCP from the Workspace surface — something only an Org Admin may do (ADR-0006, ADR-0007). Both routes now take the scope from the route, as the Agent and Skill routes already did.A Kanban card could not be assigned to a Shared Agent. The assignee picker offers every Agent visible in the Workspace, which includes attached Shared Agents, but validation accepted only workspace-scoped ones — so assigning an Agent that can run here failed with
Invalid agent assignee.The Agent Discovery tool set resolved at the wrong scope in both directions.
listAgents,getAgentand the MCP list were workspace-only, so an Operator working through an Agent could not see a Shared resource attached to their Workspace — whilelistModelProvidersoffered every Shared Provider in the Organization, attached or not, which yields an Agent that cannot run. All four now resolve at the Workspace's scope and tag each row with it.Hardening
The scope columns are mutually exclusive by a Zod refinement on write, not by a database constraint.
resolveScopednow classifies a row by the scope it actually carries rather than by elimination, the org-scoped queries require a null workspace, and the Promotion guard counts a reference as Shared only if it is org-scoped and at no Workspace — so a row holding both columns can no longer reach a Workspace that neither owns nor attached it. With the create routes fixed, that is belt and braces rather than the only line of defence.Notes
typecheckandlintclean.🤖 Generated with Claude Code