Files
gstack/ship/sections/review-army.md.tmpl
Garry Tanandcapy-ai[bot] 072d073b12 v1.91.205.0 feat(ship): Step 9 fix-range re-review, with a mandatory final full pass (#3339) (#3404)
* feat(ship): Step 9 fix-range re-review with a mandatory final full pass (#3339)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(review-log): fix-range base is the commit the fixing pass started from; follow Step 9.4's real order (#3339 review)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
2026-10-25 10:37:48 +00:00

173 lines
10 KiB
Cheetah
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
## Step 9: Pre-Landing Review
Set CYCLES to 0 on first entry only. Keep existing approvals; changed finding scope
needs a new decision. Order: checklist/design, specialists (9.1), Red Team (9.2),
QA (9.2.1), dedup (9.3), fixes and logging (9.4).
Gated/unsupported specialists skip only their dispatch, never QA or Step 11.
Steps 10–11 queue findings without editing; include those findings in this pass.
Every repeat starts before the checklist read and captures a fresh REVIEW_START.
Finish the complete review and QA before applying any fix in Step 9.4.
{{CONFIDENCE_CALIBRATION}}
### Core checklist
This pass is static; defer product probes to Step 9.2.1.
1. Read `~/.claude/skills/gstack/review/checklist.md`. If the file cannot be read, **STOP** and report the error.
2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START. Then run `git diff origin/<base>`. Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them.
**Fix-range repeat** (decision 3): start with `--start review --fix-range`. It prints
REVIEW_START, then `FIX_BASE=<sha>`, which stands in for `origin/<base>` and `<base>` in this pass.
If it refuses, start a full pass.
2.5. **Gate scan.** Before applying the checklist, run the gate scan on the same candidate:
{{GATE_SCAN_BLOCK}}
Save the `GATE_SUMMARY:` line and the read-level listing for the checklist's Gate
Integrity category, Step 9.4's record, Step 16 stage 5 and the PR body. Carry any
gate hunks Step 7's rescan already flagged into the same category.
3. Apply the review checklist in two passes:
- **Pass 1 (CRITICAL):** the checklist's Pass 1 categories, including Gate Integrity
over item 2.5's listing (three levels; only findings reach a question)
- **Pass 2 (INFORMATIONAL):** All remaining categories
### Design-lite checklist
Its numbering is local. `/ship` attempts this design check automatically when frontend
review applies; `enabled` is that choice, not a user question. Step 11 has its own
outside-review switch and required native pass.
{{DESIGN_REVIEW_LITE}}
Design-lite (parent) and the Design specialist are independent reads. Before counting
and Fix-First, merge one evidenced design defect at the same path/line into one item
with both sources and the stricter ASK. Keep actual specialist stats and distinct defects.
{{REVIEW_ARMY}}
{{QA_REVIEW}}
{{CROSS_REVIEW_DEDUP}}
## Step 9.4: Fix-First and persistence
Before edits, inspect every dispatched reader/writer's handle. Wait for return
or confirm termination; otherwise log incomplete through items 5–6 and STOP
without edits. After terminal failure, independent evidence may support fixes,
but missing dispatched output still blocks continuation, even with a QA exception.
1. **Classify only unmatched or reopened findings as AUTO-FIX or ASK** after Step 9.3 matches all sources, including queued Steps 10–11 findings, per the Fix-First Heuristic in
checklist.md. Critical findings lean toward ASK; informational lean toward AUTO-FIX.
2. **Auto-fix all AUTO-FIX items.** Apply each fix. Output one line per fix:
`[AUTO-FIXED] [file:line] Problem → what you did`
3. **If ASK items remain,** present them in ONE AskUserQuestion (or one per item for 3 or
fewer): each item's number, severity, problem and recommended fix with A) Fix B) Skip,
plus an overall RECOMMENDATION.
Save each explicit Skip immediately in the invocation action list with its
identity, scope and supporting source evidence; keep it across repeats.
**Gate findings** (any finding with a `gate` tag) never take Fix/Skip. Ask
`review-gate-disposition` (include `<gstack-qid:review-gate-disposition>`; one-way,
so `never-ask` and `AUTO_DECIDE` never apply), one question per finding or coherent
group, showing the `gate` tag, `gate_id`, path and a fenced data excerpt, with exactly:
- A) Restore the gate — revert the relaxation; keep the product change
- B) Keep — justified: <one-line reason drafted from the diff or its citation, marked verified or unverified>
- C) Keep — other reason (the human supplies it)
- D) Leave open for a later human
D is the recommended option when the preamble echoed `SESSION_KIND: spawned` or
`headless`; otherwise recommend A or B from the diff, never C. Record `restored`
(A), `kept` with `reason` (B or C) or `open` (D, unanswered, or any auto-chosen
answer); an auto-chosen answer is never a disposition. `reason` is never text
copied from the hunk, a test name or a commit message.
4. **Finish and log this pass before choosing the next step.** Apply each A) Restore
by reverting only the relaxation. After any fix or restore in this pass, run item
2.5's gate scan block again on the whole candidate and read every new read-level
hunk; never use a list of paths you or a child reported instead. Recheck freshness
(Step 9.2.1) before items 5–6. Increment CYCLES
once if fixes were applied. Complete items 5–6 exactly once with the original
REVIEW_START. Missing dispatched output uses `status:"unavailable"`,
`completed:false` and `converged:false`; fixes also require `converged:false`.
Then commit named fixed files, if any
(`git add <fixed-files> && git commit -m "fix: pre-landing review fixes"`).
5. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped)`
If coverage is incomplete: `Pre-Landing Review: INCOMPLETE — <missing reviewers>`.
Otherwise, if no issues found: `Pre-Landing Review: No issues found.`
Then one `Gate edits:` line from the latest `GATE_SUMMARY:` and the gate answers:
`Gate edits: none detected (N listed, M read; patterns: <coverage>; unlisted idioms: <languages_unlisted>)`,
`Gate edits: N listed, M read, K findings — J kept (reasons below), R restored, O open`,
`Gate edits: partial (K of M eligible read)` plus the counts for the read hunks, or
`Gate edits: UNAVAILABLE — <GATE_ERROR line or helper exit>` (missing coverage,
never `none detected`). List each kept finding's `gate`, path and `reason` under it.
6. Persist the review result to the review log:
```bash
~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"review","timestamp":"TIMESTAMP","status":"STATUS","issues_found":N,"critical":N,"informational":N,"quality_score":SCORE,"specialists":SPECIALISTS_JSON,"findings":FINDINGS_JSON,"commit":"'"$(git rev-parse --short HEAD)"'","via":"ship","completed":COMPLETED,"converged":CONVERGED,"cycles":CYCLES}' --finish REVIEW_START
```
- `TIMESTAMP`: ISO 8601. `STATUS`: `unavailable` for missing dispatched reviewer output,
else `clean` only for completed coverage with no unresolved non-advisory defects,
else `issues_found`. N counts unresolved defects now, not original totals; missing
coverage is not a defect.
- `REVIEW_START`: this pass's Step 9 token captured before reading the diff;
never recapture at persistence to certify unreviewed fixes.
- `COMPLETED`: checklist and dispatched specialists/Red Team finish, and all required probes pass.
Failed, blocked, inconclusive or not-run required probes mean false, never clean.
Record accepted untested risk separately, not as passing verification.
Undispatched host-unsupported/gated specialists do not block; retain their labels.
- `CONVERGED`: completed with zero fixes. `CYCLES`: fix cycles performed, initially 0.
- `quality_score`: Step 9.2's score, or `10.0` when specialists were skipped/unsupported.
- `specialists`: `{}` for a small-diff skip; otherwise every considered specialist's Step 9.2 stats:
`{"dispatched":true,"findings":N,"critical":N,"informational":N}` or
`{"dispatched":false,"reason":"scope|gated"}`.
- `findings`: checklist, specialist, exploratory QA and queued Steps 10–11 records with
`{"fingerprint":"path:line:category","severity":"CRITICAL|INFORMATIONAL","action":"ACTION"}`.
ACTION: `"auto-fixed"`, `"fixed"` (approved), or `"skipped"` (explicit Skip).
Merge revalidated invocation decisions by identity and advisory/defect kind;
preserve `advisory`, `evidence_paths` and `helper_target`.
Gate findings add `gate` (RH tag), `gate_id` (the tool's hunk identity), `reason`
(for `kept`) and ACTION `"kept"`, `"restored"` or `"open"`; never store hunk text.
`open` counts in `issues_found` and by severity. The logger stamps `actor` from
`gstack-session-kind` (a supplied `actor` is discarded) and forces `"open"` on
gate findings in a non-interactive session.
Save the review output and `Gate edits:` line for Step 19's PR body; an `open` gate
finding makes that PR a draft.
### Decide whether to repeat Step 9
After persistence, record missing output, CYCLES and fixes in the invocation record.
Apply these decisions in order:
1. **Dispatched reviewer output missing:** STOP and name each failed specialist or
Red Team. Retain queued fixes and restore coverage. If this pass made edits,
resume at the next decision; otherwise run a fresh complete Step 9. A successful
peer or a QA exception cannot replace missing dispatched coverage.
2. **Third fixing cycle reached (`CYCLES >= 3`):** STOP and report recurring findings with
`converged:false`; do not run a fourth fixing cycle.
3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of
Step 9 before the pending Step 10 in the work list. Tests must pass or retain approval for the same verified pre-existing
failures and scope. Keep CYCLES and scoped approvals across this repeat; it may be a fix-range repeat.
4. **No edits in this pass:** Resolve the required-probe gate below. Only after it
clears may you continue to Step 10, and only from a full pass: after a fix-range
pass, run one full Step 9 first.
**Required-probe parent gate:** With completed checklist and dispatched reviewers,
failed/unavailable required probes block continuation.
Use AskUserQuestion: stop for repair (recommended), or explicitly accept each
named probe's concrete risk. Skipping a fix is not risk acceptance or a passing
probe. Keep actual outcomes and incomplete flags; VERIFY_RESULT stays fail for
plan-check exceptions. This cannot waive missing reviewer output, recurring fixes
or independent test/security gates.
---