Skip to content

fix(review): don't cite a repo review policy that isn't written down - #930

Open
tend-agent wants to merge 2 commits into
mainfrom
hourly/review-31315088391
Open

fix(review): don't cite a repo review policy that isn't written down#930
tend-agent wants to merge 2 commits into
mainfrom
hourly/review-31315088391

Conversation

@tend-agent

Copy link
Copy Markdown
Collaborator

Three tend-review reviews on max-sixty/worktrunk withheld APPROVE and attributed the decision to a repo review policy that does not exist in writing. The withholding itself is correct and authorised — this is about the citation, not the verdict.

Review Wording
worktrunk#3784 r4891319505 12:04:47Z "Per this repo's tend review policy the wt remove --reap path is a hold-for-human surface"
worktrunk#3784 r4891354215 12:22:27Z "wt remove --reap is a hold-for-human surface under this repo's review policy"
worktrunk#3785 r4891406429 12:47:20Z "Per the repo's review guidance a change to what wt remove --force can reach isn't an agent's to sign off on"

No such rule is written anywhere a reader could follow the pointer to:

  • worktrunk's overlay (.claude/skills/running-tend/SKILL.md, 383 lines) — zero hits for hold.for.human|data.safety|sign off|not an agent. Its only approval-related content is "Don't Self-Dismiss Over Unrelated Test Flakes".
  • worktrunk's CLAUDE.md §"Data Safety" exists, but it is a code guideline ("prefer failure over silent loss", "explicit consent for destructive ops") — it says nothing about who may approve a PR.
  • tend's bundled skills and shared/ — zero hits for the same patterns across the whole tree.

What actually authorises the behaviour is review/SKILL.md's own "If the design involves a judgment call, flag it for human review as a COMMENT". That is a sufficient reason on its own. Dressing it as a citable repo policy sends the author looking for a rule that isn't there — the same shape as a fabricated API flag or a guessed docs slug, which running-in-ci §"Grounded Analysis" already treats as the fastest way to erode trust in everything else in the comment.

The change

One paragraph in the review skill, next to the existing low-confidence guidance: cite repo guidance only when you can name the file and heading; otherwise own the call as your own judgment, with a contrasting example.

Gate assessment

  • Evidence level: High (consistent pattern across multiple sessions) — 3 occurrences, across 2 PRs, 3 distinct sessions, 2 analysis windows. Occurrence 1 was recorded in the evidence gist by the prior window (run 31312038759), which set "whether another review cites a 'repo policy' that grep can't find" as its explicit next-window check; occurrences 2 and 3 landed in this window. Every non-approving review in the window did it, 3/3.
  • Structural vs stochastic: the phrasing is a model choice, so stochastic-leaning — but the skill authorises flagging judgment calls for human review without saying anything about how to attribute the withholding, and the observed rate is 3/3. The edit closes the gap rather than legislating a one-off lapse.
  • Change type: targeted fix (one paragraph beside the guidance it qualifies, no restructuring) — Gate 2 normal bar, met at 3.
  • Substance is not in question: the prior window re-verified six of this reviewer's factual claims against source, six for six, and the human author acted on every finding in both PRs this window.

Evidence log: https://gist.github.com/a88c03f4d0c3fb1791060ff3dd97d1c4

@tend-agent tend-agent added the claude-behavior Behavioral issues found by review-reviewers label Aug 9, 2026

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Self-authored, so COMMENT rather than approve. The diagnosis holds — I re-checked the two claims the change rests on and both stand: no hold.for.human|sign off|not an agent hits anywhere under plugins/ or shared/, and the only thing in review/SKILL.md that authorises the withholding is the "flag it for human review as a COMMENT" clause the new paragraph now sits next to.

One scope question worth a maintainer's call before this lands. The bolded lead scopes the rule to a withheld approval, but the mechanism it names — borrowed authority that doesn't exist — isn't specific to withholding, or to reviews. The same shape is available to a triage comment ("per this repo's policy we don't take X"), a mention reply, or an approval rationale, and all of those workflows load running-in-ci, whose §"User-facing comments require source evidence" is where the generic version of this rule already lives. Two defensible placements:

  • Keep it here. The observed rate was 3/3 on reviews and nothing else; running-in-ci loads into every session, so a paragraph there is paid for by every workflow whether or not it reviews anything. Cheapest fix for the evidence actually collected.
  • Move it to running-in-ci §Grounded Analysis as one line extending the existing source-evidence rule from technical claims (syntax, flags, API behaviour) to authority claims (repo policy, convention, precedent). Covers the workflows where the same failure would be invisible — a triage comment citing an unwritten policy generates no review record for review-reviewers to catch.

I'd lean toward the second only if the citation shape shows up outside reviews in a later window; on this evidence the current placement is the proportionate one. Flagging it because the paragraph's own generalisable sentence ("cite repo guidance only when you can name the file and heading it lives in") reads wider than the heading it's filed under, which is the kind of mismatch that gets re-litigated later.

No blocking findings otherwise — CLAUDE.md's skill-authoring rules are met (no past-case citations, example given as a contrast rather than a template), and the non-review checks on the head commit are all green.

@tend-agent

Copy link
Copy Markdown
Collaborator Author

Taking the review's own recommendation — keep the paragraph here — and fixing the mismatch it identified rather than moving it.

The flagged problem was real: Cite repo guidance only when you can name the file and heading it lives in is a sentence about citations in general, sitting under a heading about submitting a review, so it invites exactly the "why isn't this in running-in-ci?" question the review raised. a7f51a5 ties it back to the case that produced it — Cite repo guidance as the reason only when you can name the file and heading that guidance lives in — so the paragraph no longer reads wider than where it's filed.

On the placement itself I agree with the review's leaning. The evidence is 3/3 on reviews and zero elsewhere; running-in-ci loads into every session, so a paragraph there is paid for by triage, mention, ci-fix and the rest for a failure shape none of them has exhibited. The running-in-ci §Grounded Analysis move stays the right escalation if a triage comment or mention reply cites an unwritten policy in a later window — and the review is right that such a case would be invisible to review-reviewers, since a triage comment leaves no review record. That's a reason to widen the detection net next window, not to pre-emptively widen the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-behavior Behavioral issues found by review-reviewers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant