Teach dormouse-bot to judge necessity before correctness - #921
Merged
Merged
Conversation
On the spec-audit stack #890–#907 the review bot approved every layer, including PowerShell DACL helpers duplicating Rust, a ~1,000-line transfer/close state machine, Noise guards against an unreachable race, and the gutting of SECURITY.md. Each finding it raised asked for more completeness (bound the retry, handle the elevated owner, extend the gap to one more file), so its feedback grew the machinery a maintainer pass then removed. Add a running-tend review section: name a reachable trigger for every new mechanism, propose the smaller design instead of patching hardening, weigh CI-invisible platform code, flag bundling, diff removed spec facts and reader-facing cuts, refuse parked "Known gap" findings, and flag injected-global tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
1cf0154
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://35994ecd.mouseterm.pages.dev |
| Branch Preview URL: | https://tend-review-proportionality.mouseterm.pages.dev |
dormouse-bot
reviewed
Oct 2, 2026
dormouse-bot
left a comment
Collaborator
There was a problem hiding this comment.
The platform bullet's example list overstates what CI skips, and the bot will repeat it in reviews as fact. standalone-platform-check in .github/workflows/ci.yml runs on windows-latest and macos-latest: cargo test over the crate's #[cfg(windows)] Rust, dor/test/launcher.test.mjs for dor.cmd, and browser-launch-env.test.ts. A review following this line would tell an author their Windows-only Rust is untested when a PR check runs it. The suggestion below keeps the rule but asks the reviewer to check the job first.
|
|
||
| - **Must name the trigger for every new guard, retry, fallback, or state.** Name the actor (user action, peer, Relay, local same-user process) and the shipping configuration that reaches it, and cite the caller. If nothing reachable triggers it, that is the finding: drop it, or file an issue. Do not review the robustness of a mechanism that should not exist. | ||
| - **Never answer a gap in a hardening mechanism by asking for more of it.** When a fix needs a fix (an unbounded retry, a missed owner case, a stuck state), first ask whether a smaller design avoids the problem, and propose that instead. For example, write the journal before ownership moves and refuse the move on failure, rather than adding phases, retries, and parking. | ||
| - **Must weigh platform code that no CI job runs.** Windows-only branches, PowerShell, and named pipes run only on the author's machine. Say so, and require a proportionally stronger trigger. |
Collaborator
There was a problem hiding this comment.
Suggested change
| - **Must weigh platform code that no CI job runs.** Windows-only branches, PowerShell, and named pipes run only on the author's machine. Say so, and require a proportionally stronger trigger. | |
| - **Must weigh platform code that no CI job runs.** PowerShell, named pipes, and any Windows-only path outside what `standalone-platform-check` in `.github/workflows/ci.yml` exercises run only on the author's machine. Check that job, say so, and require a proportionally stronger trigger. |
Drop the incident-specific checks (bundling, spec-cut diffs, reader-facing surfaces, Known gap parking, injected-global tests): each gave the bot more to flag, which is the verbosity this PR targets, and their examples named one stack. Fold CI-invisible platform code into the trigger rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nedtwigg
requested a deployment
to
hosted-preview
October 2, 2026 18:20 — with
GitHub Actions
Waiting
This branch is waiting to be deployed
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
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.
On the spec-audit stack #890–#907 the review bot approved every layer, including PowerShell DACL helpers that duplicated existing Rust, a ~1,000-line transfer/close state machine, and Noise guards against a race no caller can produce. Every finding it raised asked for more completeness (bound the retry, handle the elevated owner, extend the gap to one more file), so its feedback grew the machinery that a maintainer pass later removed, about 2,000 lines in total.
Tend's upstream review skill already asks whether a change carries its weight, but nothing in this repo says what that means here. This adds a "Reviewing PRs" section to
.claude/skills/running-tend/SKILL.md, which every tend workflow loads:COMMENTrather thanAPPROVE.The section stays to these general rules on purpose: incident-specific checks give the bot more to flag, which is the verbosity this targets.
Tend restores its configuration from the base branch, so this takes effect once merged.
🤖 Generated with Claude Code