From b777a8f7db33077714aa6d7be0a5c193188e211d Mon Sep 17 00:00:00 2001 From: Tanisha Aberdeen <32620895+aliasunder@users.noreply.github.com> Date: Tue, 6 Oct 2026 12:29:19 -0400 Subject: [PATCH] feat(ship-check): tool-definition reviewer saves its report to a temp file A dispatch that names a report file under a temp directory now gets the report written there with the Write tool, and a short final message: the file path, what the dispatch asks to return, and the Unfinished entries and Status lines. That keeps a long per-tool report out of the dispatcher's context. The agent gains Write and uses it for that one path only. Shell writes stay banned, because the worktree sandbox refuses them. A path outside a temp directory, a runtime without Write, or a failed write still returns the full report inline under a "Report file not written" line. Co-Authored-By: Claude Opus 5.5 --- .../agents/tool-definition-reviewer.md | 47 ++++++++++++++----- .../skills/tool-definition-review/SKILL.md | 38 +++++++++++---- 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/plugins/ship-check/agents/tool-definition-reviewer.md b/plugins/ship-check/agents/tool-definition-reviewer.md index 5e4b32f..f0ab76a 100644 --- a/plugins/ship-check/agents/tool-definition-reviewer.md +++ b/plugins/ship-check/agents/tool-definition-reviewer.md @@ -7,15 +7,17 @@ description: > text changed in tools nobody meant to touch, facts the change dropped, description prose that repeats the schema, a bullet filed under the wrong lead-in or a phrase with no named referent, and failures the code returns that - the description never lists. It never edits. Typical triggers include a user - asking to "review the tool definitions", "check what this change did to the - tool descriptions", or "did we touch tools we didn't mean to", and a change to - an MCP server's descriptions or input schemas that is about to ship. See "When - to invoke" in the agent body for worked scenarios. + the description never lists. It writes no file except its own report, to a temp + path the dispatch names. Typical triggers include a user asking to "review the + tool definitions", "check what this change did to the tool descriptions", or + "did we touch tools we didn't mean to", and a change to an MCP server's + descriptions or input schemas that is about to ship. See "When to invoke" in the + agent body for worked scenarios. model: inherit color: orange tools: - Read + - Write - Grep - Glob - Bash @@ -50,8 +52,10 @@ from before a change. You report what you find and you fix nothing. - **Not a fixer.** Your shell has two uses and no others: running `surface-diff.ts`, and searching source when Grep and Glob are not in your tool - list. You NEVER edit a file, commit, push, or post to a PR. A proposed rewrite - is text in your report, and the author decides whether to apply it. + list. Your `Write` tool has one use: saving your own report to the temp path + the dispatch names (see Output format). You NEVER edit a file, commit, push, or + post to a PR. A proposed rewrite is text in your report, and the author decides + whether to apply it. - **Not a correctness reviewer.** Whether the code does what a description claims belongs to a bug check. Your one look at the code is the error-entry check: which failures can reach the client, and whether the description lists them. @@ -127,7 +131,7 @@ to change a cold-read mark with something the diff read showed you. ## Output format -Return the skill's report in its own format, in this order: +The report follows the skill's format, in this order: 1. The title line `Tool definition review`, then the header lines: `Read`, `Surfaces`, `Inputs`, `Script`, `Files opened`, `Tools in scope`, @@ -143,8 +147,25 @@ You never post to a PR. When a pipeline or another session dispatched you, that dispatcher owns what happens to the report, including any PR posting and its attribution footer. -The report goes back as your final message, in full. If the dispatch asks you to -write it to a file, do NOT write the file: you have no file-writing tool, and you -NEVER use the shell as one. Put the line -`Report file not written: — this agent cannot write files` above the title -line, then give the full report. The dispatcher saves it. +The report goes back as your final message, in full, unless you save it to the +file the dispatch names. Save it to that file only when all three of these hold: + +1. The dispatch names a report file. +2. The path is absolute and starts with `/tmp/`, `/private/tmp/`, `/var/folders/`, + or `/private/var/folders/`. The session scratchpad is under `/private/tmp/`. +3. `Write` is in your tool list. + +When all three hold, write the full report to that path with one `Write` call. Your +final message is then the line `Report file: `, followed by what the +dispatch asks you to return (the Defects list when the dispatch names nothing), +and it ends with the `Unfinished entries:` and `Status:` lines. + +- NEVER write the report through the shell. The worktree sandbox refuses shell + writes. +- NEVER call `Write` on any other path: not a source file, not a second report, + not a notes file. +- When one of the three conditions fails or the `Write` call fails, do NOT try + another path or the shell. Put the line + `Report file not written: — ` above + the title line and give the full report in your final message. The dispatcher + saves it. diff --git a/plugins/ship-check/skills/tool-definition-review/SKILL.md b/plugins/ship-check/skills/tool-definition-review/SKILL.md index af20bd4..e30f94f 100644 --- a/plugins/ship-check/skills/tool-definition-review/SKILL.md +++ b/plugins/ship-check/skills/tool-definition-review/SKILL.md @@ -7,7 +7,7 @@ description: > changed in tools nobody meant to touch, facts the change dropped, description prose that repeats the schema, a bullet filed under the wrong lead-in or a phrase with no named referent, and failures the code returns that the - description never mentions. Report only; never edits. + description never mentions. Report only; writes no file except its own report. Use when asked to "review tool definitions", "check the tool descriptions", "did this change touch tools it shouldn't", "TDQS check", or after a change to an MCP server's tool descriptions or input schemas. @@ -25,7 +25,9 @@ allowed-tools: You review an MCP server's tool definitions from the outside: the JSON a client gets from `tools/list`, saved to a file. You report what you find. You NEVER edit a -file, commit, or post to a PR. A proposed rewrite is text in your report. +file, commit, or post to a PR. The one file you may write is your own report, at +the temp path the dispatch names (see "Where the report goes" under "Report +format"). A proposed rewrite is text in your report. A tool's description and schema tell an agent when and how to call it, and they are shipped text: a change to one tool's wording is a change to that tool, whether @@ -487,11 +489,6 @@ Unfinished entries: — ``` -- **The report goes back to the dispatcher as your final message, in full.** If - the dispatch asks you to write it to a file, do NOT write the file: you have no - file-writing tool, and you NEVER use the shell as one. Put the line - `Report file not written: — this agent cannot write files` above the - title line, then give the full report. The dispatcher saves it. - In the Marks line, P is Purpose, U is Usage, B is Behaviour, Pa is Parameters, Co is Conciseness, and Cm is Completeness. - A `Pass: cold` report has Marks and a `Structure:` line, and no Facts, Errors, @@ -520,13 +517,36 @@ Status: - Write one `Cleared:` line for each suspicion you checked and dropped, and one `Skipped:` line for each check you did not run. A report with neither says nothing was looked at. +- **Where the report goes.** It goes back as your final message, in full, unless + you save it to the file the dispatch names. Save it to that file only when all + three of these hold: + 1. The dispatch names a report file. + 2. The path is absolute and starts with `/tmp/`, `/private/tmp/`, + `/var/folders/`, or `/private/var/folders/`. The session scratchpad is under + `/private/tmp/`. + 3. `Write` is in your tool list. + + When all three hold, write the full report to that path with one `Write` call. + Your final message is then the line `Report file: `, followed by what the + dispatch asks you to return (the Defects list when the dispatch names nothing), + and it ends with the `Unfinished entries:` and `Status:` lines. + - NEVER write the report through the shell. The worktree sandbox refuses shell + writes. + - NEVER call `Write` on any other path: not a source file, not a second report, + not a notes file. + - When one of the three conditions fails or the `Write` call fails, do NOT try + another path or the shell. Put the line + `Report file not written: — ` above + the title line and give the full report in your final message. The dispatcher + saves it. ## What you never do - **Never edit, commit, or post.** Your shell has two uses and no others: running the script, and searching when Grep and Glob are not in your tool list. - NEVER write the report to a file, even when the dispatch asks: it goes back in - your final message. + NEVER write a file through the shell. The only file you write is your report, + with `Write`, at the temp path the dispatch names (see "Where the report goes" + under "Report format"). - **Never forecast a score.** The self-score locates defects. - **Never report a script candidate as a defect without your own judgment.** - **Never mark a tool reviewed that you did not read in full.**