Skip to content

Teach dormouse-bot to judge necessity before correctness - #921

Merged
nedtwigg merged 2 commits into
mainfrom
tend-review-proportionality
Oct 2, 2026
Merged

nedtwigg merged 2 commits into
mainfrom
tend-review-proportionality

Conversation

@nedtwigg

@nedtwigg nedtwigg commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

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:

  • Name a reachable trigger (actor, shipping configuration, caller) for every new guard, retry, fallback or state; code no CI job runs needs a stronger one.
  • Propose a smaller design instead of patching hardening.
  • When these checks are the only findings, submit COMMENT rather than APPROVE.

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

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>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1cf0154
Status: ✅  Deploy successful!
Preview URL: https://35994ecd.mouseterm.pages.dev
Branch Preview URL: https://tend-review-proportionality.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .claude/skills/running-tend/SKILL.md Outdated

- **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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
nedtwigg merged commit aa1e913 into main Oct 2, 2026
11 checks passed

This branch is waiting to be deployed

1 waiting deployment
hosted-preview — 1cf0154c Waiting Oct 2, 2026 by nedtwigg via cleanup #779
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants