feat(mcp): add a zero-execution action flight plan

[Mihir-Rabari]

MCP Flight Plan

Agents can invoke a large MCP surface, but the operator previously had to infer the current outcome from separate kill-switch, policy, and activity views. This adds a zero-execution flight plan to the clickable MCP status surface.

Open MCP Activity Log → Flight Plan, select a registered tool, and BOSS shows its current four-stage route:

• tool switch and access availability
• effective policy
• human approval requirement
• whether the handler would run, wait, or be withheld

The dialog also shows the tool's risk classification, mutating/read-only classification, provider, description, and expected input schema. It receives registry and policy snapshots but has no invocation, approval, or ledger-writing path.

Validation

• :composeApp:desktopTest --tests "ai.rever.boss.mcp.McpFlightPlanTest"
• :composeApp:ktlintCheck :composeApp:detekt

[kshivang]

Thanks Mihir-Rabari — the isolated feature commit c93ffcdc3 is a legitimate, focused addition and cherry-picks cleanly onto current dev, preserving your implementation and test credit. I reviewed the full submitted diff, current MCP admission/policy code, related open/closed PRs/issues, Claude feedback availability, and CI.

I’m converting this PR to draft because the submitted head is not safe to merge as-is:

1. The branch is stale/stacked. Against current dev, the tip-to-tip diff is 137 files / 11k additions and includes unrelated historical changes plus two Supabase migrations (20260916130000_set_definer_search_paths.sql and 20260918000000_revoke_client_ciphertext_writes.sql). Please rebase onto current dev and retain only the flight-plan change (the clean cherry-pick of c93ffcdc3 is the expected shape). No migrations were applied and main was not touched.

2. The forecast misclassifies recoverable policy-write faults. McpFlightPlanLauncher passes policyFault != null, but McpPolicyEngine.policyFor fails closed only for PersistedPolicyUnreadable. PolicyPersistFailed and ProviderPolicyPersistFailed leave invocation policy resolution unchanged (the call may still be ALLOW/ASK), so the dialog can show WITHHELD when the live route would not be withheld. Pass only the unreadable fault (or remove the separate boolean and derive it from the already-resolved policy), and add a regression test for a write-failure fault versus an unreadable-policy fault.

3. The new status-item gate still checks tools.isNotEmpty(), not allTools.isNotEmpty(). If every registered tool is disabled or permission-denied, the MCP status item disappears, so the operator cannot open the flight plan that is meant to explain those blocked routes. Gate the item on registered tools (while preserving the existing empty-state behavior) and add a focused test for an all-blocked registry.

Please rerun the focused McpFlightPlanTest plus the relevant Compose/static checks on the rebased exact head; the currently reported CI is green, but it validates the stale submitted head rather than the clean current-dev delta. No Claude review is attached to #1380, and the related MCP PRs I found are adjacent—not duplicates: #624/#434 history/observations, #1365 session guard, and #1462/#1463 approval/activity-log UI.

[Mihir-Rabari]

Addressed the review on the current dev base. The branch now contains the isolated flight-plan cherry-pick plus a focused fix commit (20a3778), with no unrelated migrations. The forecast marks only PersistedPolicyUnreadable as blocking; regression tests cover both write-failure fault types and the unreadable fault. The status item now gates on all registered tools, with a test for an all-blocked registry and the empty state. Local Gradle tests could not start because this checkout lacks cached JxBrowser, SQLite JDBC, and Compose icon artifacts; CI is running against the pushed head.

[Mihir-Rabari]

Final update: CI on the rebased head 37d3821 passed the macOS, Ubuntu, and Windows build-test jobs, code quality, compatibility, database, contracts, version, and shell checks. The branch is ready for another review.

[Mihir-Rabari]

Rebuilt on current dev (bc9aa3b): the branch now carries only the isolated flight-plan series, 10 files (+672/-2) covering the McpFlightPlan model, dialog, launcher, view, the activity-log hook, and focused tests. The unrelated 139-file aggregate diff is gone. Fresh CI is starting on ff1b71f; I will post the full matrix when it completes.

[Mihir-Rabari]

Fresh CI on ff1b71f: build-test (macOS/Ubuntu/Windows), code quality, compatibility, database, contracts, version, and shell checks all passed. Ready for re-review.

[fluck-boss]

@claude please review this exact head (ff1b71f) for merge into dev. Check that the change is correct and complete for its stated goal, that tests cover the risk being fixed (a test that passes without the fix is a finding), and flag any security issues (secrets, injection, auth or approval bypass, path handling). Keep the review focused on this diff.

[claude]

Claude finished @fluck-boss's task in 6m 5s —— View job

---

Review — PR #1380 @ ff1b71f2

• Gather the diff against the stated dev base (bc9aa3b) — 10 files, +672/-2, matches the claimed shape
• Verify the policy-fault classification fix
• Verify the status-item gate now uses registered tools
• Check tests actually fail without the fixes
• Security pass (approval bypass, injection, secrets, path handling, RBAC disclosure)

The branch is clean — git diff --stat bc9aa3b HEAD is exactly the 10 flight-plan files, no migrations, no stacked history. Both blocking items from the earlier review are genuinely fixed in the product code. My findings are about the tests not reaching the sites that were wrong, plus one correctness issue nobody has flagged yet.

---

✅ Both prior blockers are fixed in the code

Policy fault. mcpPolicyFaultBlocksInvocation (McpFlightPlan.kt:25) matches McpPolicyEngine.policyFor exactly — that resolver fails closed on one fault only:

// McpPolicyEngine.kt:210
if (_fault.value is McpPolicyFault.PersistedPolicyUnreadable) return McpPolicyAction.DENY

Status item. BossBottomBar.kt:469-472 now reads allTools, so an all-blocked registry keeps the entry reachable.

Delegation is right. The launcher calls policyEngine.policyFor(name, providerId, readOnly) — byte-identical to the live invoke path at McpToolRegistryImpl.kt:756, so session trust, provider DENY precedence and the #804 readOnly rule are inherited rather than re-derived. declaredReadOnly goes through the same single McpMutatingToolCatalog.isMutating call the gate uses. That is the correct shape for this feature and the main thing I'd have rejected it for.

---

🔴 F1 — McpActivityStatusItemTest passes without the fix (blocking, per your criterion)

McpActivityStatusVisibility.kt is a || b || c || d, and the test feeds it literals:

assertTrue(mcpActivityStatusShouldRender(false, true, false, false))

Revert BossBottomBar.kt:469 to McpToolRegistryImpl.tools and this test stays green. The bug was never in the boolean — it was in which flow is bound to the hasRegisteredTools parameter, and the test cannot observe that binding. As written it's a tautology test; the parameter name is the only thing carrying the intent.

The repo has the machinery for a real one — McpActivityLogLayoutTest, SpaceSaveAffordanceLayoutTest, BossTopRightBarTest all mount composables. Make McpActivityStatusItem internal (or have it take the two flows), register a tool into an McpToolRegistryCore, disable it so allTools is non-empty while tools is empty, and assert the item renders.

Fix this →

🟠 F2 — the policy-fault test also does not reach the site that was wrong

val blocks = mcpPolicyFaultBlocksInvocation(fault)   // test calls the predicate itself
val plan = mcpFlightPlan(input(policy = ALLOW, policyFaulted = blocks))

Revert McpFlightPlanLauncher.kt:43 to policyFault != null and this stays green too. This one is weaker than F1 — it does pin the predicate, so mutating McpFlightPlan.kt:25 to fault != null fails it, and that's the substance of the rule. But the launcher wiring is where the defect lived and it is still untested. Extracting a tiny non-composable flightPlansFor(allTools, tools, disabled, fault, policyOf) from the launcher would make both this and the schema/permission wiring testable in one move.

🟠 F3 — isPermitted is derived from the exposed set, so the tested input combination is unreachable in production

McpFlightPlanLauncher.kt:18,36:

val availableIds = availableTools.map { "${it.providerId}/${it.definition.name}" }.toSet()  // = tools
…
isPermitted = id in availableIds

tools is already allTools − disabled − permission-denied (McpToolRegistryImpl.kt:725). So a disabled but permitted tool arrives as (isEnabled = false, isPermitted = false). The test fixture's (enabled = false, isPermitted = true) is a state the launcher can never emit — so the production path for a disabled tool is the one combination with no coverage.

It produces the right label today only because availabilityCheckpoint tests !isEnabled first (McpFlightPlan.kt:85). Swap those two branches and every switched-off tool starts telling the operator "Current permissions do not expose this tool" — a governance UI confidently naming the wrong gate. That is an order-load-bearing check with no mutation test, which is precisely the trap AGENTS.md documents for pluginSectionAbsence ("All three orderings are mutation-verified: reversing each one fails a named test").

McpToolRegistryImpl.permittedTools() (:148) exists for exactly this reading — its KDoc: "it keeps the ones a user has switched off … but drops the ones they have no permission for." Using it makes the two flags orthogonal and the ordering no longer load-bearing.

Fix this →

🟡 F4 — an open Flight Plan goes stale after a policy change

The launcher collects allTools, tools, disabledToolNames and policyFault as Compose state, but resolves policy with a plain call:

policy = McpToolRegistryImpl.policyEngine.policyFor(...)   // reads _config.value, no subscription

McpPolicyEngine.config and .sessionTrustedTools are both public StateFlows (:124, :127) and neither is collected. So an "Always Allow" from an approval dialog, a section-policy write, or a session-trust revoke leaves the dialog showing the pre-change route until some other collected flow happens to emit. For a surface whose stated job is "its current four-stage route", that's worth closing — collecting the two flows (even just as recomposition keys) is a two-line change.

🟡 F5 — the fail-closed rule is now duplicated over a sealed interface

mcpPolicyFaultBlocksInvocation is a second copy of policyFor:210. McpPolicyFault is a sealed interface, so an exhaustive when would make a fourth fault type a compile error here instead of silently defaulting to "does not block" — the convention this repo already states for McpApprovalDisposition ("an exhaustive when … so a disposition the enum grows later is a compile error here rather than silently counted as a tool fault").

Worth saying in the KDoc that the drift is currently benign: because policyFor returns DENY for any fail-closed fault, a missed fault type degrades the checkpoint wording ("Current policy is DENY" instead of "Policy storage is unhealthy") and never the outcome. That's a good property of the design and it should be written down rather than rediscovered.

🟡 F6 — the KDoc that likely caused the original bug is still wrong

// McpToolRegistryImpl.kt:144
/** Non-null when the policy engine degraded to fail-closed state. */
val policyFault: StateFlow<McpPolicyFault?> get() = core.policyEngine.fault

That is false for PolicyPersistFailed and ProviderPolicyPersistFailed, and reads as a direct invitation to write policyFault != null — which is what the first revision did. One sentence here prevents the next occurrence.

---

Security pass

No approval-bypass, injection, secret or path-handling issues in this diff.

• No execution path. McpFlightPlanDialog takes plans and onDismiss and nothing else — no handler, approval-bus, trust-grant or ledger callback exists to be reached. The "zero-execution" claim holds structurally, not just by convention.
• No second authorization path. Policy is delegated to policyEngine.policyFor, not reimplemented. The one duplicated rule (F5) is display-only and cannot widen access.
• Nothing rendered is interpreted. description and inputSchema go to plain Text; no shell, SQL or path is constructed.
• onOpenFlightPlan: (() -> Unit)? = null is inserted before onDismiss, so the existing trailing-lambda call sites in McpActivityLogLayoutTest still bind correctly — consistent with green CI.

🔵 One posture note. Gating on allTools means an operator with zero exposed tools now gets the MCP status item and can enumerate every registered tool's name, description and input schema. This is within the documented posture — McpToolRegistryImpl.kt:55-62 sanctions unfiltered allTools in management UI as "metadata disclosure only, local to the signed-in user's own desktop app — it grants no invocation" — and it's the behaviour you explicitly asked for. But two things shift: the precedent surface (the Toolbox MCP tab) lives in the plugin-manager plugin, not this repo (setToolEnabled has no in-host caller), so it is installable and RBAC-hideable, whereas the bottom bar is always-present host UI; and inputSchema goes beyond the "name/description" the posture paragraph enumerates. Not a blocker — worth one line added to that KDoc so the posture doc stays accurate.

Style / conventions

Clean: no em-dashes in the diff, all new files newline-terminated, BossLogger not bypassed (nothing logs), the non-lazy verticalScroll + Close-outside-the-scroll-body layout matches its sibling McpActivityLogDialog exactly.

---

Verdict

The feature is well-shaped and the two blockers are really fixed. F1 is the one I'd hold on, since it is the exact failure mode you asked me to flag — the test for the all-blocked registry cannot fail for the bug it was added to catch. F3 is a correctness issue that hasn't been raised before and is cheap to close. F2/F4/F5/F6 are follow-ups I'd accept in this PR or the next.

I did not run Gradle — this checkout has no cached JxBrowser/Compose artifacts, matching what the author reported. The verification above is by reading the code against the live admission path; CI on this head is reported green.
· branch feat/mcp-flight-plan

[fluck-boss]

@claude please review this exact head (3883b4f) for merge into dev. Check that the change is correct and complete for its stated goal, that tests cover the risk being fixed (a test that passes without the fix is a finding), that the earlier review findings on this PR are addressed, and flag any security issues (secrets, injection, auth or approval bypass, path handling). Keep the review focused on this diff.

[claude]

Claude finished @fluck-boss's task in 4m 11s —— View job

---

Review — PR #1380 @ 3883b4fa

• Gather the diff against dev (12 files, +969/-2 — clean series on 88c0e9d, no migrations)
• Re-check the three items from the CHANGES_REQUESTED review
• Re-check F1–F6 from the prior pass
• Correctness of the forecast against the live admission path
• Security pass (approval bypass, injection, secrets, path handling, disclosure)

Item 1 (the one that mattered) is genuinely fixed, and fixed well. Item 3's staleness half is fixed. Item 3's test half is not — the mutation still survives. And re-reading the forecast against McpPolicyEngine and applyExposed turned up two cases where the plan tells the operator the wrong thing about a live route, which is the one kind of bug this surface cannot have.

---

✅ What's fixed

Item 1 / F3 — isPermitted now comes from the permission set. McpToolRegistryCore._permittedToolNames (McpToolRegistryImpl.kt:562-565) applies permitted() without the disabled filter, keyed "providerId/toolName", recomputed inside applyExposed so it moves with every access change, registration and toggle. The two gates are orthogonal now, so the branch order in availabilityCheckpoint stops being load-bearing — which is the right shape of fix, not a reordering.

Item 2 — the test drives the real derivation. McpFlightPlanLauncherTest.plansFor calls mcpFlightPlanViews (the composable's own call) over a real McpToolRegistryCore. Reverting permittedToolNames to a tools-derived id set fails a disabled tool the operator may run is blamed on the kill-switch, not on permissions at line 71. That test would not have passed before the fix. Confirmed.

Item 3b / F4 — the open plan no longer goes stale. policyEngine.config and .sessionTrustedTools are both collectAsState()'d, so a rule edit or trust revoke recomposes and mcpFlightPlanViews re-resolves every tool.

Delegation is still right. policyFor(name, providerId, readOnly) is byte-identical to McpToolRegistryImpl.kt:756; declaredReadOnly goes through the same single McpMutatingToolCatalog.isMutating call the gate uses. No second authorization path.

---

🔴 N1 — YOLO mode is not modelled, so the plan claims approval is required when it is not

McpPolicyEngine.policyFor deliberately does not consider YOLO — the bypass is applied later, at the approval step, recorded as YOLO_ALLOWED with policyApplied = ASK. The flight plan resolves policy and stops there, so with YOLO on an ASK tool renders:

Human approval — An operator must approve before the handler can start.
Handler — Held until the operator approves the request.
Human approval required

The live route is: runs immediately, no prompt. AGENTS.md calls YOLO "a live global bypass" that must keep a visible indicator; a forecast surface that reports a human gate while that bypass is live is the strongest version of the failure this feature exists to prevent — an operator checking the route before letting an agent loose gets told there is a human in the loop when there isn't.

Cheap to close: yoloMode is a public StateFlow<Boolean> (McpPolicyEngine.kt:305). Collect it in the launcher, put it on McpFlightPlanInput, and let ASK + YOLO produce READY_TO_RUN with a checkpoint naming the bypass. It is also the one case where the forecast should say why it is clear, not just that it is.

Fix this →

🟠 N2 — an unreadable kill-switch file is reported as "This tool is switched off"

applyExposed folds every registered tool name into _disabled while persistedSetUnreadable is up (McpToolRegistryImpl.kt:788-790) — a deliberate fail-closed, documented at :478. The launcher reads disabledToolNames, so in that state every tool reports:

Tool switch — This tool is switched off.

The outcome (WITHHELD) is right; the explanation is wrong, and it is wrong in the direction that sends the operator to the Toolbox toggle, which cannot fix it. This is the exact shape of bug the PR already handles correctly for policyFault — killSwitchFault is right there on the registry (McpToolRegistryImpl.kt:154) and is not consulted. A surface whose whole job is naming the gate should name this one.

Fix this →

🟠 F1 (still open) — the status-item test still passes if the binding regresses

The helper now takes the list rather than a boolean, and the KDoc states the contract — a real improvement. But the mutation is unchanged in outcome:

// BossBottomBar.kt:543
val allTools by McpToolRegistryImpl.allTools.collectAsState()   // revert to .tools

Both are StateFlow<List<RegisteredMcpTool>>, so it compiles, and every test calls mcpActivityStatusShowsFor(core.allTools.value, …) directly — the composable's binding is never executed. Green either way. By the criterion you set, this is still a finding.

McpActivityStatusItem is private; making it internal (or having it take the flow) and mounting it the way McpActivityLogLayoutTest does would close it in a few lines.

🟠 F2 (still open) — the launcher's fault mapping is untested end-to-end

McpFlightPlanTest computes mcpPolicyFaultBlocksInvocation(fault) itself and feeds the boolean; McpFlightPlanLauncherTest passes core.policyEngine.fault.value, which is null in all three tests. Revert McpFlightPlanLauncher.kt:83 to policyFault != null and nothing fails.

The predicate is pinned (mutating McpFlightPlan.kt:25 to fault != null fails policy write faults do not withhold a tool that remains allowed), which is the substance — so this is weaker than F1. McpToolRegistryCore takes an injectable policyEngine, so a core built over a corrupt policy file would drive plansFor with a real PersistedPolicyUnreadable and close it.

🟡 N3 — a policy change resets the operator's selection

var selectedId by remember(plans) { … } (McpFlightPlanDialog.kt:67). Everything in McpFlightPlanView is a data class of primitives/enums, so an equal recomputation is harmless — but a genuine policy change, trust revoke, registration or toggle produces a different plans and snaps the selection back to the first tool. Directly caused by the (correct) F4 fix. Keying on plans.map { it.id } keeps the selection across a policy edit, which is exactly when the operator is watching the route change.

🟡 F5 / F6 / posture note — unchanged from the last pass

• mcpPolicyFaultBlocksInvocation is still an is check, not an exhaustive when over the sealed McpPolicyFault. Worth saying in its KDoc that the drift is benign — policyFor returns DENY for any fail-closed fault, so a missed type degrades wording, never outcome.
• McpToolRegistryImpl.kt:156 still reads "Non-null when the policy engine degraded to fail-closed state", which is false for PolicyPersistFailed / ProviderPolicyPersistFailed and is the sentence that produced the original bug.
• The posture paragraph at :56-63 still enumerates "name/description"; this diff now renders inputSchema from always-present host UI. One line keeps the posture doc accurate.

🔵 Nits

• _permittedToolNames re-applies permitted() next to the existing permittedTools() (:812) rather than mapping over it — two copies of one filter, which this repo's own conventions treat as the thing to avoid.
• McpFlightPlanLauncher's KDoc credits key(...) with subscribing the composable; collectAsState does that, and with no remember inside the block the key is a no-op. Harmless, but the comment explains the mechanism wrongly.
• The launcher tests assert checkpoint states, never labels — the label is the user-visible payload and the thing item 1 was about.

---

Security pass

No approval-bypass, injection, secret or path-handling issue in this diff.

• Zero-execution claim holds structurally. McpFlightPlanDialog(plans, onDismiss) — no handler, approval-bus, trust-grant or ledger callback exists to reach. McpFlightPlanView carries strings only, not the RegisteredMcpTool, so the handler lambda is not even in the dialog's object graph.
• No second authorization path. Policy is delegated. The one duplicated rule is display-only and can only ever be more restrictive in wording, never wider in access.
• Nothing rendered is interpreted — description and inputSchema go to plain Text; no shell, SQL or path constructed.
• Additive parameter placement is safe: onOpenFlightPlan sits before the trailing onDismiss, so existing call sites bind unchanged.
• Disclosure widens as intended but beyond the written posture. Gating on allTools means an operator with zero exposed tools can now enumerate every registered tool's name, description and input schema from always-present host UI. That is the behaviour you asked for and within the sanctioned "metadata disclosure only, grants no invocation" stance — but the precedent surface is the plugin-manager's MCP tab, which is installable and RBAC-hideable, and the KDoc enumerates name/description only. Documentation gap, not a vulnerability.

Style

Clean. README addition uses the spaced hyphen; em-dashes appear only in KDoc/comments, which the guard excludes. Files newline-terminated, @Suppress("LongMethod") justified in place, layout matches its sibling McpActivityLogDialog.

---

Verdict

Item 1 is properly fixed and its test is real. I'd hold on N1 — a governance forecast that reports a human approval gate while YOLO mode is bypassing it is worse than no forecast, and it's a two-field change. N2 is the same defect class the PR already solved once for policy faults. F1 remains the test-that-can't-fail you asked me to flag. F2, N3, F5, F6 are fine as follow-ups.

I did not run Gradle — this checkout has no cached JxBrowser/Compose artifacts, matching what the author reported. Everything above is read against the live admission path in McpPolicyEngine.policyFor and McpToolRegistryCore.applyExposed; CI on this head is reported green.
· branch feat/mcp-flight-plan

[Mihir-Rabari]

Review asks, addressed at 3883b4f:

1. Permission from the permission set, not the exposed set — the registry core now publishes permittedToolNames (recomputed in applyExposed from the same mcpToolPermitted rule, minus the user-disabled filter; keyed providerId/toolName), the facade re-exposes it, and the launcher's mcpFlightPlanInput derives isPermitted from it. A user-disabled tool now reports the tool-switch checkpoint, not a permission failure.

2. Disabled-path test through the launcher's real derivation — new McpFlightPlanLauncherTest drives mcpFlightPlanViews/mcpFlightPlanInput (extracted from the composable, which now calls the same function) against a real McpToolRegistryCore. The disabled-but-permitted case asserts the registry's exposed set is empty while the plan still reports isPermitted = true and BLOCKS on the availability checkpoint; a second case drives the RBAC-denied path (requiresAdmin cleared by updateAccess).

3. Stale plan / status-item binding — the launcher collects the policy engine's config/sessionTrustedTools and derives the plan inside key(...), so an open plan recomputes when a rule or session trust changes (re-derivation after a policy change reflects the new rule covers a DENY edit). The status item's visibility moved to mcpActivityStatusShowsFor(allTools, ...) over the registry snapshot itself, so the binding cannot silently regress to the exposed set; its tests now drive a real core, including the every-tool-blocked case where the exposed set is empty but the item must still render.

Hosted checks green on 3883b4f. Marking ready for review.

[fluck-boss]

Thanks @Mihir-Rabari - item 1 is fixed and its test is real. Before merge (from Claude's review of 3883b4f):

1. YOLO mode isn't modelled. policyFor deliberately ignores YOLO (it's applied at the approval step), so with YOLO on, an ASK tool's plan says "Human approval required" when the call will actually run with no prompt. A forecast that shows a human gate while the bypass is live is the failure this feature exists to prevent. yoloMode is a public StateFlow (McpPolicyEngine.kt:305): put it on McpFlightPlanInput, and have ASK + YOLO give READY_TO_RUN with a checkpoint that names the bypass.
2. An unreadable kill-switch file shows as "This tool is switched off". The outcome (WITHHELD) is right, but the explanation sends the operator to a Toolbox toggle that can't fix it. Consult killSwitchFault (McpToolRegistryImpl.kt:154) the same way you already handle policyFault.
3. F1, still open: the status-item test still passes if BossBottomBar.kt:543 goes back to .tools. Please make it discriminate.

F2, N3, F5 and F6 are fine as follow-ups.

Changes requested + draft until then. Push, get the ubuntu and windows legs green, mark it ready for review, and it goes straight back into the merge queue.