Skip to content

feat(skills): ask where a decision already lives, in the plan, the review and the risk verdict - #154

Merged
mgratzer merged 5 commits into
mainfrom
feat/design-guardrails
Nov 10, 2026
Merged

mgratzer merged 5 commits into
mainfrom
feat/design-guardrails

Conversation

@mgratzer

Copy link
Copy Markdown
Owner

A copied module passes every forge step today, because each step judges one diff and the other copy is outside it. This makes the plan, the review and the risk verdict ask what already exists, who reads what is new, and what a replacement leaves behind.

Notes

  • Deliberate cuts, to pay for the additions: the maintainability deep pass and the optional second reviewer for broad diffs (the default pass now asks their questions), the rubric's What to Flag list, the module-shape checklist, and pattern-audit.md (one consumer, now inline in forge-implement, whose steps 5 and 6 are one step).
  • The Design check sends more PRs to review. Sandbox PR refactor(skills): adopt AGENTS.md-first context model #11 flips from low risk to needs review for one exported helper only its test imports. docs/testing.md says so, and no sandbox PR isolates the Exercise check any more.
  • Size: 64 lines added, 98 removed in skills and docs, one shared module less. Words in skills/ go up by about 1 %.
  • Not done: the periodic architecture-review skill (a new skill, so it needs its own yes).
  • Judgment calls (run unattended from another project's session): which amendments from a second codebase review to take. Taken: methods and options in the review questions, options only a test sets as test seams, Leaves behind, text made false by the change, tests that cannot fail, removal conditions. Left out: deleting the "Alternative implementations" bullet (it now carries the exception instead).

Change outline

One rule restated per place:

Rule Plan and Issue Review Risk verdict
A decision has one home deep-modules.md design block: Already lives review-dimensions.md item 3, question 1; a copy is P1 risk-rubric.md Design: copies
Nothing unread Interface, Hides questions 2 and 3 judgment test
No test seam phase-execution.md 2. Test; forge-implement step 6 question 4 Design: test seams, read from the PR's Notes
A replacement leaves nothing design block: Leaves behind; forge-implement step 5 acceptance criterion in forge-create-issue Design: leftover

Exercised

Snapshots: forge-new at 8fc4c8f (the first commit of this branch), forge-old at 89ad722. The later commits (Leaves behind, the merged step, the removed checklist) were not exercised.

Try it: snapshot the branch as docs/testing.md says and run /forge-new:forge-land 49 read-only in the sandbox; expect a Design: line.

Plan

Plan as first made (no approver: the owner approved the proposal's items B1 to B5, B7 and B8 in another session, not this edit list)
  • Design: no new module or variant. Extends deep-modules.md, review-dimensions.md, risk-rubric.md, phase-execution.md.
  • Phase 1: design block in deep-modules.md, used by forge-implement step 2, forge-create-issue and pre-flight.
  • Phase 2: four design questions in the default review pass; copies are P1; the reviewer searches outside the diff.
  • Phase 3: coverage through the interface; test seams listed in the PR's Notes.
  • Phase 4: a Design check in the risk rubric, replacing the where-to-look sentence about restated rules.
  • Phase 5: an Enforcement row in forge-setup-project's audit; a removal-condition rule for one-off code.
  • Cuts: maintainability deep pass, optional second reviewer, What to Flag, pattern-audit.md.
  • Not changing: forge-guard, forge-land's steps, the lens, the PR-state script.

Deviations (as of 41a66db):

  • Leaves behind, the test-must-fail rule, the merged step 5 and the removed module-shape checklist were added after the plan: input from a second codebase review, sent by another session at the owner's request.

…view and the risk verdict

A Plan and an Issue state a design block (Interface, Hides, Already lives).
The default review answers four design questions and searches outside the
diff. Land gains a Design check for copies and test seams. Coverage is reached
through the interface, and a forced test seam is listed in the PR.

To pay for it: the maintainability deep pass and the optional second reviewer
for broad diffs go (the default pass now asks their questions), the rubric's
What to Flag list goes (the checklist says what to look for), and the
single-consumer pattern-audit module moves into forge-implement.
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