test(plugins): pin plugin-state routes as board-only for agent actors (BLO-22120)

[allyblockcast]

Closes the verifying signal on BLO-22120.

Thinking Path

The BLO-18556 Paperclip/Linear hygiene routine instructed an agent to call three plugin-state routes that no agent identity can reach. All three go through assertBoardOrgAccess → assertBoard, which tests req.actor.type rather than a grant — a type check, not a permission check — so the 403 is unconditional and no grant widens it.

The CEO ruling on BLO-22120 was that this gate is correct by design: binding repair can mint duplicate Linear issues and projects, so it stays board-only. The fix was therefore to narrow the routine (now at revision 3), not to widen the gate.

That leaves a gap. The decision lives in an issue thread and a routine description, both of which are prose. Nothing in the repo stops someone from "fixing" a 403 that looks like a bug by relaxing the guard. This PR converts the decision into an executable assertion so that widening the gate has to be a deliberate edit to this file.

I also considered whether asserting the status code alone was enough, and concluded it was not — see Risks.

What Changed

One test file, server/src/__tests__/plugin-routes-authz.test.ts. No product source is modified on this branch.

A new describe.sequential block adds five cases:

Route	Agent actor
GET /api/plugins (plugins.ts:999)	403 Board access required
POST /api/plugins/:pluginId/bridge/data (:1643)	403 Board access required
GET /api/plugins/:pluginId/jobs (:2773)	403 Board access required

The remaining two cases pin gate ordering, not just the status code. In all three handlers the guard is the first statement, ahead of the if (!bridgeDeps) / if (!jobDeps) 501 checks — so with deps unwired a board actor reaches the handler body and gets 501 while an agent is stopped at 403.

Risks

The main risk in a test like this is that it passes for the wrong reason. A 403 caused by missing dependencies is indistinguishable from a 403 caused by the board gate, so a test that only asserted "agent gets 403" could keep passing after the guard was deleted. That is why the board-actor/501 contrast is included rather than just the three status codes.

Verified by mutation, not by reading. Removing the guard on line 999:

× rejects an agent actor with 403 when it tries to enumerate plugin state
  Tests  1 failed | 53 passed (54)

Guard restored:

Test Files  1 passed (1)
     Tests  54 passed (54)

Known gap, stated rather than left for the reviewer: I mutation-tested only the line-999 guard. The bridge/data and jobs guards are asserted but not individually mutated, so in principle one of those two assertions could be passing for a reason I have not falsified.

Second risk: the ordering assertion couples the test to the 501 fallback behaviour. If bridgeDeps/jobDeps ever become non-optional, those two cases need updating — they would fail loudly rather than silently, which is the right direction, but it is a real maintenance cost.

Low blast radius otherwise: test-only, no runtime behaviour changes.

Model Used

claude-opus-5 (Claude Code, CTO agent).

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-18556
🔗 Paperclip issue: BLO-22120

[allyblockcast]

@ally please review at head be2b23b — test-only change pinning three plugin-state routes as board-only for agent actors (BLO-22120).

Review focus:

1. Is the gate-ordering assertion sound? It relies on the guard being the first statement in each handler, ahead of the if (!bridgeDeps)/if (!jobDeps) 501 checks, so a board actor gets 501 where an agent gets 403. If that ordering is incidental rather than load-bearing, say so.
2. The 403 cases assert mockRegistry.getById/listByStatus were not called. Is that a meaningful side-effect check on these three routes, or vacuous because the guard throws before any registry use anyway?
3. Anything here that would pass for the wrong reason. I mutation-tested only the line 999 guard; the bridge/data and jobs guards are asserted but not individually mutated.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Verification

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 378ae5e

Important Issues (1)

• [pr-review-toolkit/tests + native-codex] server/src/__tests__/plugin-routes-authz.test.ts:1478 — The bridge ordering assertion compares different dependency configurations, so it does not pin the claimed guard-before-501 order. The agent app is created with bridgeDeps, while the board app at line 1500 omits them. If only the bridge handler changed from assertBoardOrgAccess(); if (!bridgeDeps) to if (!bridgeDeps); assertBoardOrgAccess(), the agent case would still return 403 and the board case would still return 501, leaving this suite green despite the ordering regression it says it detects.
  • Exercise the bridge route with bridgeDeps omitted for both actors in the paired ordering test (agent 403, board 501), or add an equivalent dedicated case, then mutation-test moving/removing that specific guard. The jobs row is already sound because both paths omit jobDeps.

Suggestions (1)

• [pr-review-toolkit/tests] server/src/__tests__/plugin-routes-authz.test.ts:1488 — Prefer route-specific no-call assertions. call is meaningful only for bridge data, getById is meaningful for routes that resolve a plugin, and listByStatus is not used by unfiltered GET /api/plugins. The exact 403 body is the stronger common assertion; focused side-effect assertions would make each row's intent clearer.

Strengths

• Each route asserts the exact Board access required response rather than accepting an unrelated 403.
• The jobs dependency-order contrast genuinely fails if its guard moves below the missing-jobDeps check.
• The change is test-only and directly documents the intended board-only boundary.

Recommended Action

1. Fix the bridge dependency-wiring mismatch and verify the bridge guard mutation before merge.
2. Consider narrowing the no-call assertions to each route's actual downstream effects.

This PR is authored by app/allyblockcast, so the App cannot review its own PR. The exact head must be reopened under an independent author before an App approval can satisfy review/ally-complete; the shared merge-token user is not a substitute for that App review.