Skip to content

feat(reviewing-prs): fix-forward finding routing — native severity as the blocker bit - #12

Merged
SSFSKIM merged 1 commit into
mainfrom
tech-debt-fix-forward-routing
Jul 12, 2026
Merged

feat(reviewing-prs): fix-forward finding routing — native severity as the blocker bit#12
SSFSKIM merged 1 commit into
mainfrom
tech-debt-fix-forward-routing

Conversation

@SSFSKIM

@SSFSKIM SSFSKIM commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Problem

The review loop's routing default spends momentum on non-blockers. Recorded instance: the SD-5 dogfood run (docs/doperpowers/2026-07-10-codex-workers-shakedown.md) routed 7/7 findings FIX NOW → 7 commits → 3 engine rounds on a single PR. The old TOO SMALL bin required arguing a triple condition (valid AND non-blocking AND momentum cost) to defer anything, so a diligent worker defaults to fixing everything.

Change

Adopts agent-harness's burden-of-proof direction (core-beliefs #8: minimal blocking gates, fix-forward), using the engine's native severity as the blocker bit — no re-tagging instructions, per the minimal-prompting rule:

  • ROUTE: critical/high (P1 class) → FIX NOW. Everything below → new LOG bin (renamed from TOO SMALL): structured comment on the standing tech-debt issue by default. Promoting a non-blocker to an in-loop fix is the argued exception — the exact reverse of before.
  • RE-REVIEW: new statelessness clause — re-review rounds WILL re-flag logged findings; match by file+substance, don't fix/re-log/re-count, exit = no NEW blocker (not a clean report).
  • SELF-MERGE: wording aligned ("only non-blocker findings, each explicitly routed").
  • SKILL.md tech-debt sink wording; spec Revision Note 2026-07-13 (living tail; body stays historical draft).

Validation Evidence

  • tests/reviewing-prs/test-review-dispatch.sh — all tests passed
  • tests/reviewing-prs/test-review-engine.sh — all green
  • tests/reviewing-prs/test-land-dispatch.sh — all tests passed
  • Grep: no remaining TOO SMALL refs outside historical plan/spec bodies

… the blocker bit

ROUTE flips the burden of proof (agent-harness direction): the engine's
critical/high (P1 class) is the blocker bit → FIX NOW; every finding
below it defaults to the new LOG bin (was TOO SMALL's argued deferral)
and lands on the standing tech-debt issue. Promoting a non-blocker to an
in-loop fix is now the exception that needs a stated reason.

RE-REVIEW gains a statelessness clause: re-review rounds re-flag logged
findings; match by file+substance, never fix/re-log/re-count them, and
exit on 'no NEW blocker', not a clean report.

SELF-MERGE wording aligned; SKILL.md tech-debt sink updated; spec gets a
2026-07-13 Revision Note.

Motivation: SD-5 dogfood routed 7/7 findings FIX NOW across 3 engine
rounds — the old default spends momentum on non-blockers.
@SSFSKIM
SSFSKIM merged commit 402de28 into main Jul 12, 2026
@SSFSKIM
SSFSKIM deleted the tech-debt-fix-forward-routing branch July 12, 2026 18:49
SSFSKIM added a commit that referenced this pull request Jul 15, 2026
… tail

Adversarial review of the entrypoint restructure found two defects the
file-based protocol introduced and one doc drift:

- The bootstrap told the worker to 'Use doperpowers:reviewing-prs' without
  pinning where the protocol resolves from. In the detached review worktree
  the workspace .agents/skills is PR-controlled, so a PR could ship a
  same-named skill replacing the merge rubric. The bootstrap now binds
  SKILL_FILE (dispatcher-owned absolute path), instructs the worker to open
  it unconditionally, and forbids workspace resolution.
- SKILL.md ended with unrendered '---- PR brief ----' template sections that
  nothing renders (dispatch renders the bootstrap, not SKILL.md). Removed;
  the protocol now points at the dispatch prompt for briefs and manifests.
- operation-manual.md still described the pre-#12 self-merge clause ('only
  low findings'); it now matches the protocol's non-blocker wording.

Regression asserts pin the SKILL_FILE binding, the spoofing rejection, the
tail's absence, and the manual wording.
SSFSKIM added a commit that referenced this pull request Jul 15, 2026
* refactor(reviewing-prs): make skill the worker protocol

* fix(reviewing-prs): pin dispatcher-owned protocol, drop dead template tail

Adversarial review of the entrypoint restructure found two defects the
file-based protocol introduced and one doc drift:

- The bootstrap told the worker to 'Use doperpowers:reviewing-prs' without
  pinning where the protocol resolves from. In the detached review worktree
  the workspace .agents/skills is PR-controlled, so a PR could ship a
  same-named skill replacing the merge rubric. The bootstrap now binds
  SKILL_FILE (dispatcher-owned absolute path), instructs the worker to open
  it unconditionally, and forbids workspace resolution.
- SKILL.md ended with unrendered '---- PR brief ----' template sections that
  nothing renders (dispatch renders the bootstrap, not SKILL.md). Removed;
  the protocol now points at the dispatch prompt for briefs and manifests.
- operation-manual.md still described the pre-#12 self-merge clause ('only
  low findings'); it now matches the protocol's non-blocker wording.

Regression asserts pin the SKILL_FILE binding, the spoofing rejection, the
tail's absence, and the manual wording.
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.

1 participant