fix: restore Dispatch access for organization members - #4597
Conversation
….md, packages/core, packages/dispatch (10 files)
Visual recap — generation failedThe visual recap could not be generated for this pull request. This is informational only and does not block the PR. Diagnostic: No plan URL: Repair changed too much of targeted file plan.mdx; expected a localized parser fix. Agent output: Repaired recap-source.json: normalized the malformed double-escaped newlines and quotes in all code-bearing MDX attributes. JSON and attribute-expression validation pass. --- ⠀ 🟢 Recap source repaired and ready for the hosted parser retry. stderr: Reading additional input from stdin... |
✅ Deploy Preview for nutritrack-daily-calories canceled.
|
|
Addressed the authorization concern in ba63a9d. Authenticated organization members can still open Dispatch and use its member/read surfaces. Organization owners/admins and explicitly assigned Dispatch admins now gate approval-policy changes and workspace-resource administration actions (including imports and starter restoration), with regression coverage for denied members. Existing vault, MCP-access, app-creation-settings, and shared-agent store guards remain in place. |
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🟡
Review Details
Code Review Summary
This revision fixes the previously reported cross-organization scoping, missing-schema handling, registry validation, and organization-wide dream-settings authorization issues. The new ctxScope predicate correctly separates personal rows from organization rows, the registry now reuses the Dispatch access check, and the standalone fallback is restricted to a detected standalone runtime. The focused authorization and HTTP checks show the intended member/admin behavior and cross-org isolation.
New Findings
- 🟡 MEDIUM — The client app switcher still synthesizes a Dispatch link whenever the registry omits Dispatch, so a federated-revoked member can continue seeing and navigating to a launcher link that the registry intentionally removed.
- 🟡 MEDIUM — Organization-scoped Dispatch admin actions now fail for supported CLI callers because federation validation requires an HTTP request origin that the CLI action runner does not provide.
- 🟡 MEDIUM — The newly added standalone-schema regression test relies on late environment mutation after app configuration is cached and fails in the focused test suite.
The browser planner again could not obtain screenshots because Chrome automation tools are unavailable. Authenticated HTTP fallback checks passed the member/admin gates, revocation behavior, dream ownership, and cross-org isolation.
🧪 Browser testing: Attempted after review; visual execution was blocked by unavailable Chrome automation tools, with authenticated HTTP fallback checks completed.
| const dispatchApp = | ||
| !canAccessDispatch || currentAppId === "dispatch" | ||
| currentAppId === "dispatch" | ||
| ? null | ||
| : (visibleDispatchApp ?? | ||
| ({ |
There was a problem hiding this comment.
🟡 Do not synthesize Dispatch after registry access is revoked
listWorkspaceApps now filters Dispatch out when federated membership is revoked, but this fallback inserts a ready Dispatch link whenever no visible Dispatch entry exists. A revoked member therefore still sees /dispatch/overview and the “more in Dispatch” link despite the registry denying access. Only synthesize the fallback when standalone mode or a positively available/loading registry state establishes that Dispatch should be shown.
Additional Info
Confirmed by a code-review agent; this is distinct from the fixed server-side registry validation because it occurs in the client switcher fallback.
There was a problem hiding this comment.
Fixed in 6a25fe9. The org menu now synthesizes Dispatch only for standalone mode or while the workspace registry is loading. Once an org-scoped registry response omits Dispatch, revoked members do not receive a client-side fallback. Added revoked-registry coverage.
| let membership; | ||
| try { | ||
| membership = await validateFederatedOrganizationMembershipForCurrentRequest( |
There was a problem hiding this comment.
🟡 Support CLI callers in organization-scoped admin authorization
authorizeDispatchAdmin invokes validateFederatedOrganizationMembershipForCurrentRequest for every organization. The supported CLI action runner supplies userEmail and orgId but no request origin, so the validator throws before local organizations can be recognized as non-federated. Newly gated organization-scoped Dispatch admin actions therefore fail from the CLI even for valid local-org callers; use a context-aware local-org path while preserving fail-closed behavior for linked federated organizations.
Additional Info
Confirmed independently by two code-review agents from the supported CLI runner context and the validator's request-origin requirement.
There was a problem hiding this comment.
Fixed in 6a25fe9. Federation validation now permits a missing request event for local organizations, so CLI action contexts work. Linked organizations still require a request origin when authority validation is needed and therefore fail closed. Added no-origin local-org coverage.
| it("keeps standalone Dispatch available when its org schema is absent", async () => { | ||
| vi.stubEnv("AGENT_NATIVE_APP_ID", "dispatch"); |
There was a problem hiding this comment.
🟡 Fix the standalone-schema regression test setup
The new test stubs AGENT_NATIVE_APP_ID after app configuration has already been initialized, but isStandaloneDispatchRuntime() reads the cached getAppConfig().app value. The focused workspace-app-access.spec.ts run therefore fails this assertion (expected true, received false) instead of validating the standalone fallback. Configure/reset the app config before the test or mock the runtime detector so the added regression suite passes reliably.
Additional Info
A code-review agent reproduced the focused test failure; other focused suites passed.
There was a problem hiding this comment.
Fixed in 6a25fe9. The standalone Dispatch regression tests now reset cached app configuration after stubbing runtime environment variables, so standalone and hosted missing-schema behavior is deterministic.
Summary
Validation
Fixes the mistaken admin-only interpretation from #4557.