mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 00:44:07 +01:00
* 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>
173 lines
10 KiB
Cheetah
173 lines
10 KiB
Cheetah
## 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.
|
||
|
||
---
|