feat: the daemon builds the review agent's command — no MCP unless allowlisted, the skill shipped in the prompt, JSON output - #96
Merged
Conversation
`prepare` was raw argv, so it could not carry the flags the review's safety rests on, and the agent ran with the reviewer's MCP servers, memory and skills. The `agent` block replaces it — model, effort, mcpAllow, extraArgs, maxBudgetUsd — and `buildAgentArgv` builds the rest: `--setting-sources ""` so no user settings load at all, a PreToolUse gate (`inbox mcp-gate`) when MCP tools are allowlisted, a deny list for the gh writes and the package managers CI has already run, `--output-format json` for the run's cost, and the shipped diffity-review skill in the system prompt. A loaded config that still has `prepare` is refused by name. Part of #93 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w
… out of the deny list Review findings on #96: - The answer pass ran with no skills installed and nothing in their place, while the live prompt still told it to follow diffity-live. `skillBody` now takes the skill name, and the answer argv ships the live body. - `extraArgs` sat after the variadic `--disallowedTools`, so a non-flag extra arg was read as another denied tool; the deny list goes last. - A budget hit with no `agent.maxBudgetUsd` read "$null". - The page let the browser submit a budget of 0, which the daemon refuses. - The README claimed no Claude settings load even with `mcpAllow` set. - `inbox mcp-gate` had no test of the command itself. Also, from the new tests: the log's write stream had no error handler, so a log that could not be opened would end the daemon on an unhandled event, and `runAgent` resolved before the log had flushed. Part of #93 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w
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.
preparewas the raw argv, so it could not carry the flags the review's safety rests on: the agent ran with the reviewer's MCP servers (in #14188 it called gcloud with the reviewer's credentials inside a checkout the author controls), their memory, theirCLAUDE.mdand their skills, and it re-ranpnpm typecheck/pnpm testin nearly every prepare although CI had already done it.The
agentblock replaces it —model,effort,mcpAllow,extraArgs,maxBudgetUsd— and the daemon builds the rest inbuildAgentArgv:--setting-sources "", so no user settings load at all: no MCP servers, no memory, noCLAUDE.md, no installed skills.mcpAllow(exact tool names) switches to--setting-sources userplus aPreToolUsehook onmcp__.*running the new hiddendiffity inbox mcp-gate, which exits 2 for anything not on the list. The prompt then says which tools it may read the ticket or document with.gh pr review|comment|merge,gh api, andpnpm|npm|npx|yarn|bun|make.diffity-reviewskill body in--append-system-prompt, so the review no longer depends on the reviewer's installed skills; the prompt points at the system prompt instead of the skill.--output-format json, parsed byparseAgentOutputinto the textverdictOfreads plus the run's cost, duration, turns, tokens and models. Stats ride on everyPrepareResultbut are not persisted — that is (2).subtype: error_max_budget_usdfails the attempt with the budget as the reason.A loaded config that still has
prepareis refused by name, with the migration in the message. The page's Settings panel gains model, effort, MCP allowlist and budget; the README's inbox section covers the block, what the built command closes off, and the migration.Verified:
npm testat the root is green (1069 tests, 135 files);inbox mcp-gateexercised end to end against the built dist (allowed tool 0, denied tool 2 with the message, built-in tool 0, garbage stdin 2); every flag the builder emits is present in the installedclaude --help.One thing to call out: the live-answer pass now also runs with
--setting-sources ""and no system prompt, so the answering agent can no longer load thediffity-liveskill the live prompt still names. The prompt spells out the reply command, so answers should still land, but if that skill is doing real work there we should ship its body too.Part of #93
🤖 Generated with Claude Code
https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w