Skip to content

refactor(server): one J5 tool pre-approval list for Codex and Claude - #301

Merged
bryantderosier merged 8 commits into
j5/mainfrom
j5/captains-audit
Sep 29, 2026
Merged

bryantderosier merged 8 commits into
j5/mainfrom
j5/captains-audit

Conversation

@bryantderosier

@bryantderosier bryantderosier commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Jackson's ruling on #233 (2026-09-26): every shipped adapter can be a Captain, and J5 pre-approves its own tools on Codex and Claude only, where the roster card is the only human step before a Crew launches. Other harnesses may show their own MCP prompt once before the roster card. Before this PR, Codex and Claude each carried their own copy of the J5 tool list; this PR gives them one shared list.

What I changed

  • apps/server/src/j5/a2a/mcp/j5ToolPreapproval.ts (new): J5_PREAPPROVED_TOOLS (the full J5 set, used where the thread never asks for approval), J5_COORDINATION_TOOLS (used in interactive modes), j5ApprovalPolicyIsNever, j5PreapprovedTools(policy), and j5T3McpToolName.
  • codexToolApproval.ts and claudeAllowedTools.ts now derive their lists from the shared module instead of keeping their own.
  • j5ToolPreapproval.testkit.ts (new): J5_APPROVAL_POLICY_MATRIX (every runtime mode and every explicit approval policy) and J5_NEVER_PREAPPROVED_TOOLS, which each harness's test walks.
  • docs/j5/product/features/crews.md (Definition, AC5, History) and docs/user/personas.md: every provider can captain; on Codex and Claude the roster card is the only human step, other harnesses may prompt once first, and a read-only persona on them can't propose.

Per-harness audit, from the code (a Captain's propose_crew). Per Jackson's ruling on #233, only Codex and Claude get J5 pre-approval; the other rows stay as they are and are documented as prompting once:

Harness Interactive modes Never (Full access, or a persona policy of never) Status
Codex no prompt no prompt already appended
Claude no prompt no prompt (read-only runs dontAsk with the allowlist) already appended
OpenCode native prompt refused under a read-only persona won't do (#265, #302 closed)
Pi Pi confirm Pi confirm under a persona policy (Pi only sees the runtime mode) won't do (#266, #303 closed)
ACP registry native card refused under a read-only persona won't do (#267, #304 closed)
Grok, Antigravity native card refused under a read-only persona won't do (#268, #269 closed)
Cursor refused no prompt only in Full access won't do (#270 closed)

Why this shape

One list, decided per policy, is the smallest thing that keeps Captains and seats behaving the same on every harness. Each adapter only translates that list into its own approval model. The list stays a leaf module with no toolkit import, so adapters can import it without a cycle.

Invariants

Surfaces

Surface Decision
Entry points (chat, Settings, command palette, keybinding) Unaffected.
Clients (web, desktop, mobile) Unaffected; server and docs only.
Providers Codex and Claude refactored with no behavior change. Per Jackson's ruling on #233, no other adapter gets a pre-approval append (#302–#304 closed); other harnesses may prompt once before the roster card, as the docs now say.
Contracts (packages/contracts) Unchanged.
Reverse states n/a.
Connection modes (local, remote, tunnel) Unaffected.
Upstream files / FORK.md No new upstream touch; the FORK.md wording updated for the shared list.
Docs crews.md AC5 and History, docs/user/personas.md Crews section.

Out of scope

Upgrade and data

None.

Verification

  • vp test run apps/server/src/j5/a2a/mcp: passes, including the new j5ToolPreapproval.test.ts, which checks that the matrix covers every RuntimeMode and ProviderApprovalPolicy literal and that the list matches J5Toolkit.
  • Server typecheck clean.

Review focus

  • codexToolApproval.test.ts now asserts through the shared matrix instead of hand-written lists. Check it still pins "per tool, never a server-wide default".

Closes #233

Claude Opus 5.5 via Claude Code

🤖 Generated with Claude Code

bryantderosier and others added 2 commits September 24, 2026 20:02
Codex and Claude each kept their own copy of the J5 tools they pre-approve.
Move the list, the interactive-mode coordination subset, and the
approval-policy-never check into j5ToolPreapproval.ts so every adapter append
spreads the same set. Codex and Claude produce the same config as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: Jacksondr5/j5code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7b648fcc-6e3a-4b0e-bdb4-0b34dd6e5df0


Comment @coderabbitai help to get the list of available commands.

@bryantderosier
bryantderosier added this pull request to stack #305 September 25, 2026 00:11
@bryantderosier bryantderosier self-assigned this Sep 25, 2026
@github-actions github-actions Bot added size:L 100-499 effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Sep 25, 2026
Conflicts in FORK.md (the Crews paragraph and the upstream-hook table) and crews.md History: took the sync's text and reapplied this branch's shared j5ToolPreapproval module wording.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread docs/j5/product/features/crews.md Outdated
Comment thread apps/server/src/j5/a2a/mcp/j5ToolPreapproval.testkit.ts Outdated
@Jacksondr5

Copy link
Copy Markdown
Owner

Decision (Jackson, 2026-09-26): keep #301; the rest of the Captains stack is closed. See the ruling on #233.

#301 is J5-owned and adds no upstream surface, so it stays. Before it merges:

  • AC5 and the "Any harness can captain" bullet in crews.md: narrow them to the ruling. On Codex and Claude, the roster card is the only human step. Other harnesses may ask once with their own MCP prompt before it, and a read-only persona there can't propose. Also update the History line.
  • docs/user/personas.md: list Codex and Claude as the harnesses with no extra step. All others may prompt once, and read-only personas on them can't propose. Remove the "later PRs move entries off this list" framing.
  • FORK.md: cite Jackson's ruling on State the Captain decision per provider adapter #233, not "Bryant's ruling".
  • The panel's docs-accuracy comment: scope "never pre-approved in any mode" to J5's own appends, and name Claude's upstream mcp__t3-code__* wildcard as the exception. No change to upstream behavior.

bryantderosier and others added 2 commits September 28, 2026 08:51
…uman step

Per Jackson's ruling on #233, J5 pre-approves its tools on Codex and Claude
only. crews.md AC5, the "Any harness can captain" bullet, its History line,
and the Crews section of the personas guide now say that other harnesses may
ask once with their own MCP prompt before the roster card, and a read-only
persona there can't propose. The never-pre-approved testkit list is scoped to
J5's own appends and names Claude's upstream mcp__t3-code__* wildcard as the
exception.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bryantderosier

Copy link
Copy Markdown
Collaborator Author

@Jacksondr5 Your pre-merge list for #301 is done:

I also merged the latest j5/main. CI is running.

Comment thread FORK.md Outdated
bryantderosier and others added 2 commits September 28, 2026 13:20
FORK.md, j5ToolPreapproval.ts, and the testkit still described every harness pre-approving the set, with later adapter appends to come. Under Jackson's ruling on #233 only Codex and Claude do, and #302–#304 are closed.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bryantderosier bryantderosier changed the title refactor(server): one J5 tool pre-approval list, and every adapter can be a Captain refactor(server): one J5 tool pre-approval list for Codex and Claude Sep 28, 2026
Conflict in FORK.md: kept main's rewritten proposal-flow paragraph (migration 21, resolve-once approvals) with this branch's shared J5_PREAPPROVED_TOOLS wording.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bryantderosier
bryantderosier merged commit f34870c into j5/main Sep 29, 2026
29 checks passed
@bryantderosier
bryantderosier deleted the j5/captains-audit branch September 29, 2026 14:30
bryantderosier added a commit that referenced this pull request Sep 29, 2026
Conflicts: AC3 keeps #347's approval flag and adds this branch's preview-binding sentence; the personas guide keeps #301's and #347's text and adds the preview and ACP sentences, now in #344's access-mode names (Supervised, Auto-accept edits); both 2026-09-24 History lines stay.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bryantderosier added a commit that referenced this pull request Sep 29, 2026
Conflicts: agent-tools.md takes main's quoted propose_crew and request_crew_member descriptions and tables, which match the shipped strings after #347; crews.md takes main's Definition (Full access default, seats ask their Captain) with this branch's no-approver roster wording, and History keeps one date-ordered list with #301's and #307's 2026-09-24 lines.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

State the Captain decision per provider adapter

2 participants