Skip to content

refactor(skills): move PR state into a script and give each rule one home - #150

Merged
mgratzer merged 2 commits into
mainfrom
claude/repo-complexity-assessment-1ebbc1
Nov 10, 2026
Merged

mgratzer merged 2 commits into
mainfrom
claude/repo-complexity-assessment-1ebbc1

Conversation

@mgratzer

Copy link
Copy Markdown
Owner

Skill prose more than doubled in six weeks (about 10k to 24k words), mostly in the unattended path, where a state machine was written as prose with pasted shell and the same rules were restated across skills. This restructures that path and trims the rest to 21.8k words.

Notes

  • New skills/_shared/scripts/pr-state.sh reads mergeability, CI, reviews and open feedback (bash 3.2 and gh only). The skills are no longer Markdown only; AGENTS.md and docs/architecture.md say so.
  • Behavior removed on purpose: the substitute review, failed-review retry and "partial" review kind (a failed bot review now ends the guard at needs-human), handling of events after a terminal state (one line saying to re-run /forge-guard), the Plan's Revisions block, and REVIEW.md reading in land.
  • Small shifts from the script: wait caps are total across re-runs, and failed-review detection applies to any bot in its REVIEWERS table.
  • shellcheck was not installed locally, so CI is its first run on the new script.
  • Not covered by a run: a failed bot review, a conflict, a feedback round. Those rest on the script's 25 fixture cases.

Exercised

Sandbox mgratzer/forge-e2e-sandbox, snapshots of this branch (new) and its base 1675f56 (old), same prompts.

  • new, /forge-ship --guard add GET /notes/:id/sentence-count ... → PR 52, review-ready, land "needs review" (package.json touched), deps-audit sorted pre-existing, publish-smoke flaky and green on rerun, 0 of 3 rounds, 3 review findings fixed. Read state through pr-state.sh (ci-wait in the background, failed-checks, mergeable, feedback). $4.45
  • old, same task → PR 53, review-ready, land "needs review" (same hit), same CI sorting, 0 of 3 rounds, 5 review findings fixed. $4.51
  • new, land judge read-only on PR 11 with a local body copy → low risk, all checks clear. $0.89
  • old, same → low risk, identical check lines. $0.88
  • Failure case: none run. The base run also ran the sandbox's /sandbox-risk pass, which the new run skipped as not triggered; one pair can't tell variance from regression.
  • Not exercised: failed bot review, conflict, feedback round, wake-up after a terminal state: no sandbox setup for them in this run
  • Try it: claude --plugin-dir ., then /forge-guard <pr> in a sandbox checkout; the CI wait should appear as bash .../_shared/scripts/pr-state.sh ci-wait

…home

Skill prose grew from about 10k to 24k words in six weeks. This trims it
to 21.8k and restructures the unattended path.

- Add skills/_shared/scripts/pr-state.sh (mergeable, ci-wait,
  failed-checks, review-wait, reviews, feedback) with fixture tests run
  by scripts/check-skills.sh. pr-readiness.md now describes outputs and
  the judgment left to the skill.
- Keep vendor specifics out of the core: bot reviewer failure patterns
  live in the script's REVIEWERS table, Claude Code notes in
  skills/_shared/runtimes/claude-code.md.
- Give each cross-skill rule one home: unattended triage in
  review-rubric.md, the outcome label and late wake-ups in
  status-reporting.md, review-ready in guard, merge-ready in land.
- Rewrite forge-guard and forge-land as steps and tables. Drop
  Guidelines sections that restated steps, and barrel-imports.md.

Behavior removed on purpose:
- No substitute review, retry or partial kind: a failed bot review ends
  the guard at needs-human.
- A wake-up after a terminal state changes nothing and says to re-run
  forge-guard.
- No Revisions block: the PR carries the Plan as last approved.
- Land's judge no longer reads REVIEW.md files.
@mgratzer
mgratzer merged commit 89ad722 into main Nov 10, 2026
2 checks passed
@mgratzer
mgratzer deleted the claude/repo-complexity-assessment-1ebbc1 branch November 10, 2026 21:41
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