mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 00:03:15 +01:00
refactor(ship): Step 7 and 8 audit prompts move to prompt-only sections the parent never reads (GSTA-167)
The coverage and plan-completion audit prompts were text fences inside the sections the /ship parent reads at Steps 7 and 8, so every parent call carried them although only gstack-review-packet build reads them. They now live in ship/sections/audit-test-coverage.md and audit-plan-completion.md (runner_only, out of the section index); auditPrompt reads them there. test-coverage.md 37498 -> 15645 bytes, plan-completion.md 26538 -> 17537. Every assertion on the prompts now reads the new files; the parent-side gates are unchanged. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed
1 parent
f570a6707f
commit
91b89e3ace
16 files changed
+506
-450
No files matched your search
@@ -298,11 +298,11 @@ export function draftFor(d: Dispatch, ctx: BuildContext): PacketDraft {
|
||||
diff: reviewerDiff(ctx.root, ctx.diffBase, d.exclude, t.summary, insideOut(ctx), d.role === 'specialist' && (d.checklists ? d.checklists.includes('testing') : d.name === 'testing') ? underTests : undefined), diffLabel: t.diffLabel, diffCmd: diffCmd(ctx.diffBase, d.exclude, t.summary) };
|
||||
}
|
||||
|
||||
/** An audit's prompt is the generated section's text fence: the packet and the SKILL.md prose are one text. */
|
||||
/** An audit's prompt is the text fence of its generated prompt-only section (ship/sections/audit-<name>.md), which the /ship parent never reads. */
|
||||
export function auditPrompt(skillRoot: string, audit: AuditName, base: string, baseRef = `origin/${base}`): string {
|
||||
const section = fs.readFileSync(path.join(skillRoot, 'ship', 'sections', `${audit}.md`), 'utf8');
|
||||
const section = fs.readFileSync(path.join(skillRoot, 'ship', 'sections', `audit-${audit}.md`), 'utf8');
|
||||
const m = /^(`{4,})text\n([\s\S]*?)\n\1$/m.exec(section);
|
||||
if (!m) throw new Error(`ship/sections/${audit}.md has no prompt fence`);
|
||||
if (!m) throw new Error(`ship/sections/audit-${audit}.md has no prompt fence`);
|
||||
return m[2]!.replaceAll('origin/<base>', baseRef).replaceAll('<base>', base);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
<!-- AUTO-GENERATED from audit-plan-completion.md.tmpl — do not edit directly -->
|
||||
<!-- Regenerate: bun run gen:skill-docs -->
|
||||
## Plan completion audit prompt
|
||||
|
||||
Read only by `gstack-review-packet build --audit plan-completion`, which inlines the fence
|
||||
below into the audit packet. The /ship parent never reads this file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow plan completion audit. The base branch is `<base>`. Use `git diff origin/<base>` and inspect untracked files from `git status` to see the full proposed change. Do not commit or push. Report only: classify every item, but do not execute Gate Logic, ask the user, or advance the workflow. The parent applies those gates to your report.
|
||||
|
||||
### Plan input
|
||||
|
||||
Audit only the plan the parent supplied (path or full text). Do not search for another plan. If the supplied file is unreadable (permissions, encoding), return an audit error: do not report no plan or successful zero counts.
|
||||
|
||||
### Actionable Item Extraction
|
||||
|
||||
**Separate deliverables from execution-only verification.** Audit implementation and test-creation requirements below.
|
||||
For a local execution-only check, retain its command, expected outcome and source verbatim in the summary
|
||||
for Step 8.1/9, outside implementation counts. It remains required and pending actual execution,
|
||||
never DONE from static inspection and not EXTERNAL-STATE merely because it has not run.
|
||||
Keep genuine external-state and human-only checks in this audit with their existing gates.
|
||||
A mixed item retains its implementation obligation here and its execution check in Step 8.1/9;
|
||||
zero implementation counts do not waive those checks.
|
||||
|
||||
Extract deliverables and test-creation work, not the local checks routed above. Look for:
|
||||
|
||||
- **Checkbox items:** `- [ ] ...` or `- [x] ...`
|
||||
- **Numbered steps** under implementation headings: "1. Create ...", "2. Add ...", "3. Modify ..."
|
||||
- **Imperative statements:** "Add X to Y", "Create a Z service", "Modify the W controller"
|
||||
- **File-level specifications:** "New file: path/to/file.ts", "Modify path/to/existing.rb"
|
||||
- **Test requirements:** "Add test for Y" or another required test deliverable; route execution-only local verification as above.
|
||||
- **Data model changes:** "Add column X to table Y", "Create migration for Z"
|
||||
|
||||
**Ignore:**
|
||||
- Context/Background sections (`## Context`, `## Background`, `## Problem`)
|
||||
- Questions and open items (marked with ?, "TBD", "TODO: decide")
|
||||
- Review report sections (`## GSTACK REVIEW REPORT`)
|
||||
- Explicitly deferred items ("Future:", "Out of scope:", "NOT in scope:", "P2:", "P3:", "P4:")
|
||||
- CEO Review Decisions sections (these record choices, not work items)
|
||||
|
||||
**Cap:** Extract at most 50 items. If the plan has more, note: "Showing top 50 of N plan items — full list in plan file."
|
||||
|
||||
**No items found:** If no audited deliverables remain, report zero implementation counts and retain pending execution-only checks verbatim in summary for Step 8.1/9. This skips only the implementation audit, never required verification.
|
||||
|
||||
For each item, note:
|
||||
- The item text (verbatim or concise summary)
|
||||
- Its category: CODE | TEST | MIGRATION | CONFIG | DOCS
|
||||
|
||||
### Verification Mode
|
||||
|
||||
Classify how each item can be verified. The diff cannot prove work in another repo or external system.
|
||||
|
||||
- **DIFF-VERIFIABLE** — A code change in this repo would manifest in `git diff origin/<base>`. Examples: "add UserService" (file appears), "validate input X" (validation logic appears), "create users table" (migration file appears).
|
||||
- **CROSS-REPO** — Item names a file or change in a sibling repo (e.g., `domain-hq/docs/dashboard.md`, `~/Development/<other-repo>/...`). The current diff CANNOT prove this.
|
||||
- **EXTERNAL-STATE** — Item names state in an external system: Supabase config/RLS, Cloudflare DNS, Vercel env vars, OAuth provider allowlists, third-party SaaS, DNS records. The current diff CANNOT prove this.
|
||||
- **CONTENT-SHAPE** — Item requires a file to follow a specific convention. If the file is in this repo: diff-verifiable. If in another repo or system: see CROSS-REPO / EXTERNAL-STATE.
|
||||
|
||||
**Verification dispatch:**
|
||||
|
||||
- **DIFF-VERIFIABLE** → cross-reference against diff (next section).
|
||||
- **CROSS-REPO** → if the sibling repo is reachable on disk (try `~/Development/<repo>/`, `~/code/<repo>/`, the parent of the current repo), run `[ -f <path> ]` to check file existence. File exists → DONE (cite path). File missing → NOT DONE (cite path). Path unreachable → UNVERIFIABLE (cite what needs manual check).
|
||||
- **EXTERNAL-STATE** → UNVERIFIABLE. Cite the system and the specific check the user must perform.
|
||||
- **CONTENT-SHAPE in another repo** → if the file exists, run any project-detected validator (see "Validator detection" below) before falling back to UNVERIFIABLE. With a validator: pass → DONE; fail → NOT DONE (cite validator output). No validator available: classify UNVERIFIABLE and cite both the file path and the convention to confirm.
|
||||
|
||||
**Path concreteness rule.** If a plan item names a *concrete filesystem path* (absolute, `~/...`, or `<sibling-repo>/<file>`), it MUST be classified DONE or NOT DONE based on `[ -f <path> ]`. UNVERIFIABLE is only valid when the path is genuinely abstract ("Cloudflare DNS", "Supabase allowlist") or the sibling root is unreachable on this machine. "I don't want to check" is not unreachable.
|
||||
|
||||
**Validator detection.** Before falling back to UNVERIFIABLE on a CONTENT-SHAPE item, scan the target repo's `package.json` for any script matching `validate-*`, `lint-wiki`, `check-docs`, or similar. If found, invoke it with the relevant path argument (e.g., `npm run validate-wiki -- <path>`). For multi-target validators (e.g., `validate-wiki --all`), run once and reconcile per-item from the output. A passing validator promotes the item from UNVERIFIABLE to DONE; a failing one demotes to NOT DONE.
|
||||
|
||||
**Honesty rule.** Do NOT classify an item as DONE just because related code shipped. Code that *handles* a deliverable is not the deliverable. Shipping a markdown-extraction library is not the same as shipping the markdown file. When in doubt between DONE and UNVERIFIABLE, prefer UNVERIFIABLE — better to surface a confirmation prompt than silently miss a deliverable.
|
||||
|
||||
### Cross-Reference Against Diff
|
||||
|
||||
Run `git diff origin/<base>` and `git log origin/<base>..HEAD --oneline` to understand what was implemented.
|
||||
|
||||
For each extracted plan item, run the verification dispatch from the previous section, then classify:
|
||||
|
||||
- **DONE** — Clear evidence the item shipped. Cite the specific file(s) changed in the diff for DIFF-VERIFIABLE items, or the verified path that exists for CROSS-REPO items with a reachable sibling repo.
|
||||
- **PARTIAL** — Some work toward this item exists but is incomplete (e.g., model created but controller missing, function exists but edge cases not handled).
|
||||
- **NOT DONE** — Verification ran and produced negative evidence (file missing, code absent in diff, sibling-repo file confirmed absent).
|
||||
- **CHANGED** — The item was implemented using a different approach than the plan described, but the same goal is achieved. Note the difference.
|
||||
- **UNVERIFIABLE** — The diff and any reachable sibling-repo checks cannot prove or disprove this. Always applies to EXTERNAL-STATE items and to CROSS-REPO items where the sibling repo isn't reachable. Cite the specific manual verification the user must perform (e.g., "check Cloudflare DNS shows DNS-only mode for dashboard.example.com", "confirm /docs/dashboard.md exists in domain-hq repo").
|
||||
|
||||
**Be conservative with DONE** — require clear evidence. A file being touched is not enough; the specific functionality described must be present.
|
||||
**Be generous with CHANGED** — if the goal is met by different means, that counts as addressed.
|
||||
**Be honest with UNVERIFIABLE** — better to surface 5 items the user must manually confirm than silently classify them DONE.
|
||||
|
||||
### Output Format
|
||||
|
||||
```
|
||||
PLAN COMPLETION AUDIT
|
||||
════════════════════
|
||||
Plan: {plan file path}
|
||||
|
||||
## Implementation Items
|
||||
[DONE] Create UserService — src/services/user_service.rb (+142 lines)
|
||||
[PARTIAL] Add validation — model validates but missing controller checks
|
||||
[NOT DONE] Add caching layer — no cache-related changes in diff
|
||||
[CHANGED] "Redis queue" → implemented with Sidekiq instead
|
||||
|
||||
## Test Items
|
||||
[DONE] Unit tests for UserService — test/services/user_service_test.rb
|
||||
[NOT DONE] E2E test for signup flow
|
||||
|
||||
## Migration Items
|
||||
[DONE] Create users table — db/migrate/20240315_create_users.rb
|
||||
|
||||
## Cross-Repo / External Items
|
||||
[DONE] sibling-repo has /docs/dashboard.md — verified at ~/Development/sibling-repo/docs/dashboard.md
|
||||
[UNVERIFIABLE] Cloudflare DNS-only on api.example.com — external system, manual check required
|
||||
[UNVERIFIABLE] Supabase auth allowlist contains user email — external system, confirm in Supabase dashboard
|
||||
|
||||
────────────────────
|
||||
COMPLETION: 4/10 DONE, 1 PARTIAL, 2 NOT DONE, 1 CHANGED, 2 UNVERIFIABLE
|
||||
────────────────────
|
||||
```
|
||||
|
||||
After your analysis, output a single JSON object with exactly these seven fields on the LAST LINE of your response (no other text after it):
|
||||
{"total_items":N,"done":N,"changed":N,"partial":N,"not_done":N,"unverifiable":N,"summary":"<markdown checklist for PR body>"}
|
||||
Counts map one-to-one to the classifications above and sum to total_items. No plan or no actionable items means all counts are zero with the skip reason in summary. Do not classify work as deferred; only the parent can record a user-approved deferral.
|
||||
````
|
||||
@@ -0,0 +1,14 @@
|
||||
## Plan completion audit prompt
|
||||
|
||||
Read only by `gstack-review-packet build --audit plan-completion`, which inlines the fence
|
||||
below into the audit packet. The /ship parent never reads this file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow plan completion audit. The base branch is `<base>`. Use `git diff origin/<base>` and inspect untracked files from `git status` to see the full proposed change. Do not commit or push. Report only: classify every item, but do not execute Gate Logic, ask the user, or advance the workflow. The parent applies those gates to your report.
|
||||
|
||||
{{PLAN_COMPLETION_AUDIT_SHIP}}
|
||||
|
||||
After your analysis, output a single JSON object with exactly these seven fields on the LAST LINE of your response (no other text after it):
|
||||
{"total_items":N,"done":N,"changed":N,"partial":N,"not_done":N,"unverifiable":N,"summary":"<markdown checklist for PR body>"}
|
||||
Counts map one-to-one to the classifications above and sum to total_items. No plan or no actionable items means all counts are zero with the skip reason in summary. Do not classify work as deferred; only the parent can record a user-approved deferral.
|
||||
````
|
||||
@@ -0,0 +1,303 @@
|
||||
<!-- AUTO-GENERATED from audit-test-coverage.md.tmpl — do not edit directly -->
|
||||
<!-- Regenerate: bun run gen:skill-docs -->
|
||||
## Test coverage audit prompt
|
||||
|
||||
Read only by `gstack-review-packet build --audit test-coverage`, which inlines the fence
|
||||
below into the audit packet. The /ship parent never reads this file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow test coverage audit. Run `git diff origin/<base>` to include uncommitted tracked changes; also read relevant non-ignored untracked source/tests. Do not commit or push. Perform only this audit; return unresolved user decisions to the parent instead of asking or advancing to another workflow step.
|
||||
|
||||
Generation: <allowed|audit-only>; passes used: <N> of 2. Audit-only overrides every generation instruction below.
|
||||
|
||||
Coverage goal: every changed behavior is protected by a test that would catch a real regression. Test count is not a goal. Evaluate what was ACTUALLY coded (from the diff), not what was planned.
|
||||
|
||||
### Test Framework Detection
|
||||
|
||||
Before analyzing coverage, detect the project's test framework:
|
||||
|
||||
1. **Read CLAUDE.md** — look for a `## Testing` section with test command and framework name. If found, use that as the authoritative source.
|
||||
2. **If CLAUDE.md has no testing section, auto-detect:**
|
||||
|
||||
```bash
|
||||
setopt +o nomatch 2>/dev/null || true # zsh compat
|
||||
# Runtime markers: evidence, not commands to run
|
||||
[ -f manage.py ] && echo "RUNTIME:python FRAMEWORK:django"
|
||||
{ [ -f pyproject.toml ] || [ -f pytest.ini ] || [ -f tox.ini ] || [ -f setup.cfg ] || [ -f requirements.txt ]; } && echo "RUNTIME:python"
|
||||
{ [ -f Gemfile ] || [ -f Rakefile ] || [ -f .rspec ]; } && echo "RUNTIME:ruby"
|
||||
[ -f package.json ] && echo "RUNTIME:node"
|
||||
[ -f go.mod ] && echo "RUNTIME:go"
|
||||
[ -f Cargo.toml ] && echo "RUNTIME:rust"
|
||||
[ -f pubspec.yaml ] && echo "RUNTIME:dart"
|
||||
[ -f pom.xml ] && echo "RUNTIME:jvm BUILD:maven"
|
||||
{ [ -f build.gradle ] || [ -f build.gradle.kts ]; } && echo "RUNTIME:jvm BUILD:gradle"
|
||||
# Test infra: configs, scripts, test files
|
||||
ls jest.config.* vitest.config.* playwright.config.* cypress.config.* .rspec pytest.ini tox.ini phpunit.xml dart_test.yaml 2>/dev/null
|
||||
[ -f package.json ] && grep -q '"test"[[:space:]]*:' package.json && echo "SCRIPT:package.json test"
|
||||
[ -f Makefile ] && grep -qE '^(test|check):' Makefile && echo "TARGET:make test"
|
||||
git ls-files | grep -cE '(^|/)(tests?|spec|__tests__)/|(^|/)tests?\.py$|(^|/)test_[^/]+\.py$|_test\.(go|py|rb|ts|js|exs|dart)$|\.(test|spec)\.[jt]sx?$|_spec\.rb$|Test\.(java|kt)$' | sed 's/^/TESTFILES:/'
|
||||
```
|
||||
|
||||
3. **If no framework detected:** use the bootstrap decision already made in Step 4; report diagram-only coverage if setup was declined. Do not restart bootstrap from this audit.
|
||||
|
||||
**0. Before/after test count:**
|
||||
|
||||
```bash
|
||||
# Count test files before any generation
|
||||
git ls-files 2>/dev/null | grep -E '(\.test\.|\.spec\.|_test\.|_spec\.)' | wc -l
|
||||
```
|
||||
|
||||
Store this number for the PR body.
|
||||
|
||||
**1. Trace every codepath changed** using `git diff origin/<base>`:
|
||||
|
||||
Read every changed file. For each one, trace how data flows through the code — don't just list functions, actually follow the execution:
|
||||
|
||||
1. **Read the diff.** For each changed file, read the full file (not just the diff hunk) to understand context.
|
||||
Definition: a **targeted audit** reviews named concrete source/test files or a
|
||||
branch diff. A **prototype** is existing runnable code referenced by the plan,
|
||||
not a proposed future component.
|
||||
|
||||
When grounded in concrete source and test files, read them in a dedicated tool
|
||||
call before drawing the diagram. Finish this source read before tracing data
|
||||
flow in audit item 2 below; map user flows afterward. Do not mix diff, grep,
|
||||
package/config, git, or commentary into that read; use separate calls for
|
||||
context. Base the diagram on that read.
|
||||
2. **Trace data flow.** Starting from each entry point (route handler, exported function, event listener, component render), follow the data through every branch:
|
||||
- Where does input come from? (request params, props, database, API call)
|
||||
- What transforms it? (validation, mapping, computation)
|
||||
- Where does it go? (database write, API response, rendered output, side effect)
|
||||
- What can go wrong at each step? (null/undefined, invalid input, network failure, empty collection)
|
||||
3. **Diagram the execution.** For each changed file, draw an ASCII diagram showing:
|
||||
- Every function/method that was added or modified
|
||||
- Every conditional branch (if/else, switch, ternary, guard clause, early return)
|
||||
- Every error path (try/catch, rescue, error boundary, fallback)
|
||||
- Every call to another function (trace into it — does IT have untested branches?)
|
||||
- Every edge: what happens with null input? Empty array? Invalid type?
|
||||
|
||||
This is the critical step — you're building a map of every line of code that can execute differently based on input. Every branch in this diagram needs coverage that would catch a real regression; the test value bar below decides whether that is a new test, an extension of an existing one, or already covered.
|
||||
|
||||
**2. Map user flows, interactions, and error states:**
|
||||
|
||||
Code coverage isn't enough — you need to cover how real users interact with the changed code. For each changed feature, think through:
|
||||
|
||||
- **User flows:** What sequence of actions does a user take that touches this code? Map the full journey (e.g., "user clicks 'Pay' → form validates → API call → success/failure screen"). Each step in the journey needs coverage.
|
||||
- **Interaction edge cases:** What happens when the user does something unexpected?
|
||||
- Double-click/rapid resubmit
|
||||
- Navigate away mid-operation (back button, close tab, click another link)
|
||||
- Submit with stale data (page sat open for 30 minutes, session expired)
|
||||
- Slow connection (API takes 10 seconds — what does the user see?)
|
||||
- Concurrent actions (two tabs, same form)
|
||||
- **Error states the user can see:** For every error the code handles, what does the user actually experience?
|
||||
- Is there a clear error message or a silent failure?
|
||||
- Can the user recover (retry, go back, fix input) or are they stuck?
|
||||
- What happens with no network? With a 500 from the API? With invalid data from the server?
|
||||
- **Empty/zero/boundary states:** What does the UI show with zero results? With 10,000 results? With a single character input? With maximum-length input?
|
||||
|
||||
Add these to your diagram alongside the code branches. A user flow with no test is just as much a gap as an untested if/else.
|
||||
|
||||
**3. Check each branch against existing tests:**
|
||||
|
||||
Go through your diagram branch by branch — both code paths AND user flows. For each one, search for a test that exercises it:
|
||||
- Function `processPayment()` → look for `billing.test.ts`, `billing.spec.ts`, `test/billing_test.rb`
|
||||
- An if/else → look for tests covering BOTH the true AND false path
|
||||
- An error handler → look for a test that triggers that specific error condition
|
||||
- A call to `helperFn()` that has its own branches → those branches need tests too
|
||||
- A user flow → look for an integration or E2E test that walks through the journey
|
||||
- An interaction edge case → look for a test that simulates the unexpected action
|
||||
|
||||
Quality scoring rubric:
|
||||
- ★★★ Tests behavior with edge cases AND error paths
|
||||
- ★★ Tests correct behavior, happy path only
|
||||
- ★ Smoke test / existence check / trivial assertion (e.g., "it renders", "it doesn't throw"); weak, never counts as coverage
|
||||
|
||||
**Test value bar.** Propose or write a test only with all four answers; otherwise extend an existing test or drop it:
|
||||
|
||||
1. What observable behavior, invariant or independent contract does it protect?
|
||||
2. What credible regression makes it fail?
|
||||
3. Why does existing coverage not already catch that? Prefer adding a row to an existing table-driven test or shared fixture over a near-duplicate.
|
||||
4. Does it need a production seam (export, flag, wrapper, injection hook) that no production caller needs? If yes, test at the real boundary instead.
|
||||
|
||||
A test that breaks under a behavior-preserving refactor asserts implementation: rewrite it at the owning boundary, unless exact output is the declared contract (goldens, prompt bytes, wire formats).
|
||||
|
||||
Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none; no_claim=<...>` (seam: `none` or its name; `no_claim`: one sentence on what green does not prove and which fixtures or mocks stand in for what, e.g. `no_claim=provider acceptance (mocked provider; unit only)` or `no_claim=none beyond protects`; it never waives a required live check); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; written JSON keeps full values). Write it as a header comment in each generated test, next to the attribution (wrap, do not truncate); with no known comment syntax, put it in the PR body's Test value details. A missing upstream card never blocks: derive it; ignore unknown fields.
|
||||
|
||||
Example: Value: protects=refundPayment rejects an empty reason; fails_when=the reason guard is removed or inverted; why_new=billing.test.ts covers processPayment only; seam=none; no_claim=provider acceptance (mocked provider; unit only)
|
||||
Rejected (covered_elsewhere): "checkout renders"; checkout.e2e.ts:15 covers it, so extend that test.
|
||||
|
||||
Weak tests (★ smoke/existence/trivial, gate-failing or unrated) never count as coverage. X = paths with a ★★/★★★ test / total paths (value-weighted; the gate uses X); Y = paths with any test / total paths. Total paths = the diff's codepath trace, max 30; zero skips the gate. A path with only weak tests is uncovered in X, covered in Y, and goes to `weak_gaps` (reason `star_one|gate_failed|unrated`), not `gaps`. Rate stars only for tests reachable from changed paths.
|
||||
|
||||
Retention bar: keep a test that independently enforces a public API, protocol, config, migration, storage, security, platform, default, prompt-byte, generated-output (golden), package, release or architecture contract; static or slow is no reason to delete.
|
||||
|
||||
Regression proof: a regression test must fail at HEAD before any repair, in its own assertion (a pass at HEAD drops the regression label; an import, fixture or env failure is a test defect: correct once or drop). It must pass at base as the control (an assertion failure there marks it invalid; any other failure is "base control unavailable: collection error") and pass after the repair. Record: `Regression proof — fails at HEAD: yes · passes at base: yes | unavailable (<reason>) | manual · passes after fix: yes | pending`.
|
||||
|
||||
### E2E Test Decision Matrix
|
||||
|
||||
When checking each branch, also determine whether a unit test or E2E/integration test is the right tool:
|
||||
|
||||
**RECOMMEND E2E (mark as [→E2E] in the diagram):**
|
||||
- Common user flow spanning 3+ components/services (e.g., signup → verify email → first login)
|
||||
- Integration point where mocking hides real failures (e.g., API → queue → worker → DB)
|
||||
- Auth/payment/data-destruction flows — too important to trust unit tests alone
|
||||
|
||||
**RECOMMEND EVAL (mark as [→EVAL] in the diagram):**
|
||||
- Critical LLM call that needs a quality eval (e.g., prompt change → test output still meets quality bar)
|
||||
- Changes to prompt templates, system instructions, or tool definitions
|
||||
|
||||
**STICK WITH UNIT TESTS:**
|
||||
- Pure function with clear inputs/outputs
|
||||
- Internal helper with no side effects
|
||||
- Edge case of a single function (null input, empty array)
|
||||
- Obscure/rare flow that isn't customer-facing
|
||||
|
||||
### REGRESSION RULE (mandatory)
|
||||
|
||||
**IRON RULE:** When the coverage audit identifies a REGRESSION — code that previously worked but the diff broke — a regression test is written immediately. No AskUserQuestion. No skipping. Regressions are the highest-priority test because they prove something broke.
|
||||
|
||||
A regression is when:
|
||||
- The diff modifies existing behavior (not new code)
|
||||
- The existing test suite (if any) doesn't cover the changed path
|
||||
- The change introduces a new failure mode for existing callers
|
||||
|
||||
When uncertain whether a change is a regression, err on the side of writing the test.
|
||||
|
||||
**Red-first proof.** Apply the value bar's Regression proof to every regression test: the diff at HEAD is the pre-fix code, so run the new test at HEAD before any repair. Then, unless the parent says `Base control: off`, run this base control once per regression test in diff order, within a 3-minute total per /ship run (`Base control budget:` seconds per test, default 90). Past the total, record "base control unavailable: budget" and report "N of M regression tests got base control". The block is one shell invocation; it installs nothing and runs no build or postinstall.
|
||||
|
||||
```bash
|
||||
# Set: BASE = the base branch this /ship run resolved; TEST = the test file; FIXTURES = new
|
||||
# test-only fixtures it imports (repo-relative, may be empty); RUN = the detected
|
||||
# runner for one file (e.g. "bun test $TEST"); BUDGET = seconds for this run (default 90).
|
||||
ROOT=$(git rev-parse --show-toplevel)
|
||||
CTL_TMP=$(mktemp -d "${TMPDIR:-/tmp}/gstack-base-control.XXXXXX")
|
||||
cleanup() { git -C "$ROOT" worktree remove --force "$CTL_TMP/wt" >/dev/null 2>&1; rm -rf "$CTL_TMP"; [ -e "$CTL_TMP" ] && echo "BASE_CONTROL_LEFTOVER: $CTL_TMP (run: git worktree prune)"; }
|
||||
trap cleanup EXIT INT TERM
|
||||
(
|
||||
[ -f "$ROOT/package.json" ] || { echo "BASE_CONTROL: unavailable (ecosystem)"; exit 0; }
|
||||
git -C "$ROOT" remote get-url origin >/dev/null 2>&1 || { echo "BASE_CONTROL: unavailable (no base remote)"; exit 0; }
|
||||
timeout 30 git -C "$ROOT" fetch --quiet origin "$BASE" || { echo "BASE_CONTROL: unavailable (base not fetched)"; exit 0; }
|
||||
git -C "$ROOT" worktree add --quiet --detach "$CTL_TMP/wt" "origin/$BASE" >/dev/null 2>&1 || { echo "BASE_CONTROL: unavailable (worktree add failed)"; exit 0; }
|
||||
for f in $(echo "$TEST $FIXTURES"); do mkdir -p "$CTL_TMP/wt/$(dirname "$f")" && cp "$ROOT/$f" "$CTL_TMP/wt/$f"; done
|
||||
[ -d "$ROOT/node_modules" ] && ln -s "$ROOT/node_modules" "$CTL_TMP/wt/node_modules"
|
||||
cd "$CTL_TMP/wt" && timeout "${BUDGET:-90}" sh -c "$RUN" > "$CTL_TMP/out" 2>&1; rc=$?
|
||||
tail -n 40 "$CTL_TMP/out"
|
||||
if [ "$rc" -eq 0 ]; then echo "BASE_CONTROL: passes at base"
|
||||
elif [ "$rc" -eq 124 ]; then echo "BASE_CONTROL: unavailable (budget)"
|
||||
else echo "BASE_CONTROL: fails at base (exit $rc)"; fi
|
||||
)
|
||||
```
|
||||
|
||||
Classify "fails at base" from the output: a failure in the test's own assertion marks it invalid (correct once or drop it); an import, collection, missing generated artifact or dependency failure is "base control unavailable: collection error" and the test stays. Print each unavailable result as: base control unavailable: <reason>. The fails-at-HEAD result still stands. To check by hand: `git worktree add --detach <tmp> <base>`; copy the test and its new fixtures to the same paths; run the detected test command in <tmp>; `git worktree remove --force <tmp>`. Then report `passes at base: manual`. (see ~/.claude/skills/gstack/docs/test-value-bar.md#base-control-unavailable)
|
||||
|
||||
Return `"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}` counts in the JSON and each test's record line in the diagram.
|
||||
|
||||
**4. Output ASCII coverage diagram:**
|
||||
|
||||
For targeted audits, start Test review output with the coverage diagram. In full
|
||||
plan reviews, put it inside the normal Test review section. Required outputs
|
||||
keep the final terminal report order.
|
||||
|
||||
Include BOTH code paths and user flows in the same diagram. Mark E2E-worthy and eval-worthy paths:
|
||||
|
||||
```
|
||||
CODE PATHS USER FLOWS
|
||||
[+] src/services/billing.ts [+] Payment checkout
|
||||
├── processPayment() ├── [★★★ TESTED] Complete purchase — checkout.e2e.ts:15
|
||||
│ ├── [★★★ TESTED] happy + declined + timeout ├── [GAP] [→E2E] Double-click submit
|
||||
│ ├── [GAP] Network timeout └── [GAP] Navigate away mid-payment
|
||||
│ └── [GAP] Invalid currency
|
||||
└── refundPayment() [+] Error states
|
||||
├── [★★ TESTED] Full refund — :89 ├── [★★ TESTED] Card declined message
|
||||
└── [★ TESTED] Partial (non-throw only) — :101 └── [GAP] Network timeout UX
|
||||
|
||||
LLM integration: [GAP] [→EVAL] Prompt template change — needs eval test
|
||||
|
||||
COVERAGE: 5/13 paths tested (38%) | Code paths: 3/5 (60%) | User flows: 2/8 (25%)
|
||||
QUALITY: ★★★:2 ★★:2 ★:1 | GAPS: 8 (2 E2E, 1 eval)
|
||||
```
|
||||
|
||||
Legend: ★★★ behavior + edge + error | ★★ happy path | ★ smoke check
|
||||
[→E2E] = needs integration test | [→EVAL] = needs LLM eval
|
||||
|
||||
Avoid bare `[ ]` or `[x]` in diagrams unless the block includes
|
||||
`Legend: [x] tested | [ ] no test`. Prefer `[GAP]`, `[★★ TESTED]`,
|
||||
`[→E2E]`, `[→EVAL]`; keep user-flow markers off code-path rows.
|
||||
|
||||
**Fast path:** All paths covered → "Step 7: All new code paths have test coverage ✓" Continue.
|
||||
|
||||
**5. Generate tests for uncovered paths:**
|
||||
|
||||
If test framework detected (or bootstrapped in Step 4):
|
||||
- Apply the test value bar before writing each test. Extend an existing test (a new table row, fixture case or assertion) before creating a file. Record every proposal you decline in `tests_rejected` with a `reason_code` from `duplicate_protects, needs_seam, incomplete_card, no_credible_regression, covered_elsewhere, implementation_coupled`.
|
||||
- Never add a production seam for a test; a seam that is not `none` names its non-test callers: `seam=<name> (non-test callers: N, via <search command>)`.
|
||||
- Write the value card as a header comment in each generated or extended test.
|
||||
- Prioritize error handlers and edge cases first (happy paths are more likely already tested)
|
||||
- Read 2-3 existing test files to match conventions exactly
|
||||
- Generate the smallest native test for each path: unit tests for logic, mocking only services unrelated to the behavior under test (integration points where mocking hides real failures go to [→E2E]).
|
||||
- For paths marked [→E2E]: generate integration/E2E tests using the project's E2E framework (Playwright, Cypress, Capybara, etc.)
|
||||
- For paths marked [→EVAL]: generate eval tests using the project's eval framework, or flag for manual eval if none exists
|
||||
- Write tests that exercise the specific uncovered path with real assertions
|
||||
- Run each test. Passes → keep the change and report its path; the parent commits in Step 15.
|
||||
- Fails → diagnose whether the test/fixture is invalid or a declared product contract is broken. Correct a demonstrated test defect once; preserve a valid red regression and route the reproduced product failure through the parent's fix/approval flow. Never delete or weaken it to manufacture green; retain unresolved coverage in the diagram.
|
||||
|
||||
Caps: 30 code paths max; 5 tests per generation pass (code + user flow combined; the parent's `Generation cap:` overrides 5); an extension uses one slot and a rejection uses none; 2-min per-test exploration cap. List each remaining gap below the diagram (inside `diagram`) as a proposed test with its value card.
|
||||
|
||||
Do not rate stars for tests you wrote in this pass: count them as unrated (weak, reason `unrated`). The parent's read-only rating dispatch rates them. Counts are disjoint, precedence extended > added > rejected: one gap lands in at most one of `tests_extended`, `tests_added`, `tests_rejected`.
|
||||
|
||||
If no test framework AND user declined bootstrap → diagram only, no generation. Note: "Test generation skipped — no test framework configured."
|
||||
|
||||
**Diff is test-only changes:** Return a skipped audit with null coverage, zero gaps, and "No new application code paths to audit."
|
||||
|
||||
**6. After-count and coverage summary:**
|
||||
|
||||
```bash
|
||||
# Count test files after generation
|
||||
git ls-files 2>/dev/null | grep -E '(\.test\.|\.spec\.|_test\.|_spec\.)' | wc -l
|
||||
```
|
||||
|
||||
For PR body: `Tests: {before} → {after} (+{delta} new)`
|
||||
Coverage line: `Test Coverage Audit: N new code paths. M covered (Y% any test, X% value-weighted). K tests generated, awaiting parent commit.`
|
||||
|
||||
### Test Plan Artifact
|
||||
|
||||
After producing the coverage diagram, write a test plan artifact so `/qa` and `/qa-only` can consume it:
|
||||
|
||||
```bash
|
||||
GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}"
|
||||
SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) && mkdir -p "$GSTACK_STATE_ROOT/projects/$SLUG" && echo "PROJECT_DIR: $GSTACK_STATE_ROOT/projects/$SLUG"
|
||||
USER=$(whoami)
|
||||
DATETIME=$(date +%Y%m%d-%H%M%S)
|
||||
```
|
||||
|
||||
Write to `<PROJECT_DIR>/{user}-{branch}-ship-test-plan-{datetime}.md` (`PROJECT_DIR` printed above):
|
||||
|
||||
```markdown
|
||||
# Test Plan
|
||||
Generated by /ship on {date}
|
||||
Branch: {branch}
|
||||
Repo: {owner/repo}
|
||||
|
||||
## Affected Pages/Routes
|
||||
- {URL path} — {what to test and why}
|
||||
|
||||
## Key Interactions to Verify
|
||||
- {interaction description} on {page}
|
||||
|
||||
## Edge Cases
|
||||
- {edge case} on {page}
|
||||
|
||||
## Critical Paths
|
||||
- {end-to-end flow that must work}
|
||||
```
|
||||
|
||||
Rules for the implementing agent:
|
||||
1. Acceptance criteria are the `protects / fails_when / no_claim` profile of each test: the observable it protects, the wrong result it rejects, and what green does not prove. Never "make the tests pass".
|
||||
2. Real code and real tests land in the same change; a placeholder, a stub assertion or a test deferred to a follow-up is not done.
|
||||
3. Any change to a test, validator, CI step, timeout, tolerance, threshold, snapshot or suppression pragma is reported on its own line in your summary with its justification, before any other result.
|
||||
4. A reported command names the command and its observed result (exit status, counts, the failing line); a command you did not run is not reported, and stderr is never silenced in a command you cite.
|
||||
5. Your report is a claim until the parent re-executes it: say what you verified, what you reused, and what you could not check.
|
||||
Report any such change in the prose before the JSON line; the JSON fields below are unchanged.
|
||||
|
||||
After your analysis, output a single JSON object on the LAST LINE of your response (no other text after it):
|
||||
{"coverage_pct":N,"gaps":N,"diagram":"<full markdown coverage diagram for PR body>","tests_added":["path",...],"coverage_pct_value":N,"weak_gaps":[{"path":"...","existing_test":"...","reason":"star_one|gate_failed|unrated"}],"tests_extended":["path",...],"tests_rejected":[{"path_or_gap":"...","reason_code":"...","reason":"..."}],"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}}
|
||||
`coverage_pct` is Y (paths with any test), `coverage_pct_value` is X (paths with a ★★/★★★ test), `gaps` counts only paths with no test. Use null for an undetermined or skipped coverage percentage, not zero. Include every remaining gap in the diagram so the parent can target a second pass.
|
||||
````
|
||||
@@ -0,0 +1,20 @@
|
||||
## Test coverage audit prompt
|
||||
|
||||
Read only by `gstack-review-packet build --audit test-coverage`, which inlines the fence
|
||||
below into the audit packet. The /ship parent never reads this file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow test coverage audit. Run `git diff origin/<base>` to include uncommitted tracked changes; also read relevant non-ignored untracked source/tests. Do not commit or push. Perform only this audit; return unresolved user decisions to the parent instead of asking or advancing to another workflow step.
|
||||
|
||||
Generation: <allowed|audit-only>; passes used: <N> of 2. Audit-only overrides every generation instruction below.
|
||||
|
||||
{{TEST_COVERAGE_AUDIT_SHIP}}
|
||||
|
||||
Rules for the implementing agent:
|
||||
{{IMPLEMENTER_DISPATCH_RULES}}
|
||||
Report any such change in the prose before the JSON line; the JSON fields below are unchanged.
|
||||
|
||||
After your analysis, output a single JSON object on the LAST LINE of your response (no other text after it):
|
||||
{"coverage_pct":N,"gaps":N,"diagram":"<full markdown coverage diagram for PR body>","tests_added":["path",...],"coverage_pct_value":N,"weak_gaps":[{"path":"...","existing_test":"...","reason":"star_one|gate_failed|unrated"}],"tests_extended":["path",...],"tests_rejected":[{"path_or_gap":"...","reason_code":"...","reason":"..."}],"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}}
|
||||
`coverage_pct` is Y (paths with any test), `coverage_pct_value` is X (paths with a ★★/★★★ test), `gaps` counts only paths with no test. Use null for an undetermined or skipped coverage percentage, not zero. Include every remaining gap in the diagram so the parent can target a second pass.
|
||||
````
|
||||
@@ -40,6 +40,20 @@
|
||||
"title": "Plan completion + verification audit (subagent)",
|
||||
"trigger": "auditing plan completion, verification, and scope drift (Step 8)"
|
||||
},
|
||||
{
|
||||
"id": "audit-test-coverage",
|
||||
"file": "audit-test-coverage.md",
|
||||
"title": "Step 7 audit prompt (packet input)",
|
||||
"trigger": "never read by the agent: gstack-review-packet build --audit test-coverage inlines it into the Step 7 audit packet",
|
||||
"runner_only": true
|
||||
},
|
||||
{
|
||||
"id": "audit-plan-completion",
|
||||
"file": "audit-plan-completion.md",
|
||||
"title": "Step 8 audit prompt (packet input)",
|
||||
"trigger": "never read by the agent: gstack-review-packet build --audit plan-completion inlines it into the Step 8 audit packet",
|
||||
"runner_only": true
|
||||
},
|
||||
{
|
||||
"id": "review-army",
|
||||
"file": "review-army.md",
|
||||
@@ -86,7 +100,7 @@
|
||||
"id": "capy-pr",
|
||||
"file": "capy-pr.md",
|
||||
"title": "Capy PR body: What changed (before/after images), Why, Approvals",
|
||||
"trigger": "Step 19 printed CAPY_PR: on (a Capy session, GSTACK_CAPY=1) — read before composing the PR body"
|
||||
"trigger": "Step 19 printed CAPY_PR: on (a Capy session, GSTACK_CAPY=1) \u2014 read before composing the PR body"
|
||||
},
|
||||
{
|
||||
"id": "handoff",
|
||||
|
||||
@@ -56,122 +56,12 @@ or complete text, including user-approved scope changes. When discovery printed
|
||||
not-run line, skip dispatch. The child does not inherit the parent's conversation.
|
||||
**By packet:** `~/.claude/skills/gstack/bin/gstack-review-packet build --audit plan-completion
|
||||
--base <base> --plan-file <bound plan> --out-dir .gstack/tmp/review-packets-audit` writes the
|
||||
prompt below, the whole plan and the diff into one packet; brief the child only with Step 7's
|
||||
audit prompt, the whole plan and the diff into one packet; brief the child only with Step 7's
|
||||
packet brief and read the LAST line of its `out=` file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow plan completion audit. The base branch is `<base>`. Use `git diff origin/<base>` and inspect untracked files from `git status` to see the full proposed change. Do not commit or push. Report only: classify every item, but do not execute Gate Logic, ask the user, or advance the workflow. The parent applies those gates to your report.
|
||||
|
||||
### Plan input
|
||||
|
||||
Audit only the plan the parent supplied (path or full text). Do not search for another plan. If the supplied file is unreadable (permissions, encoding), return an audit error: do not report no plan or successful zero counts.
|
||||
|
||||
### Actionable Item Extraction
|
||||
|
||||
**Separate deliverables from execution-only verification.** Audit implementation and test-creation requirements below.
|
||||
For a local execution-only check, retain its command, expected outcome and source verbatim in the summary
|
||||
for Step 8.1/9, outside implementation counts. It remains required and pending actual execution,
|
||||
never DONE from static inspection and not EXTERNAL-STATE merely because it has not run.
|
||||
Keep genuine external-state and human-only checks in this audit with their existing gates.
|
||||
A mixed item retains its implementation obligation here and its execution check in Step 8.1/9;
|
||||
zero implementation counts do not waive those checks.
|
||||
|
||||
Extract deliverables and test-creation work, not the local checks routed above. Look for:
|
||||
|
||||
- **Checkbox items:** `- [ ] ...` or `- [x] ...`
|
||||
- **Numbered steps** under implementation headings: "1. Create ...", "2. Add ...", "3. Modify ..."
|
||||
- **Imperative statements:** "Add X to Y", "Create a Z service", "Modify the W controller"
|
||||
- **File-level specifications:** "New file: path/to/file.ts", "Modify path/to/existing.rb"
|
||||
- **Test requirements:** "Add test for Y" or another required test deliverable; route execution-only local verification as above.
|
||||
- **Data model changes:** "Add column X to table Y", "Create migration for Z"
|
||||
|
||||
**Ignore:**
|
||||
- Context/Background sections (`## Context`, `## Background`, `## Problem`)
|
||||
- Questions and open items (marked with ?, "TBD", "TODO: decide")
|
||||
- Review report sections (`## GSTACK REVIEW REPORT`)
|
||||
- Explicitly deferred items ("Future:", "Out of scope:", "NOT in scope:", "P2:", "P3:", "P4:")
|
||||
- CEO Review Decisions sections (these record choices, not work items)
|
||||
|
||||
**Cap:** Extract at most 50 items. If the plan has more, note: "Showing top 50 of N plan items — full list in plan file."
|
||||
|
||||
**No items found:** If no audited deliverables remain, report zero implementation counts and retain pending execution-only checks verbatim in summary for Step 8.1/9. This skips only the implementation audit, never required verification.
|
||||
|
||||
For each item, note:
|
||||
- The item text (verbatim or concise summary)
|
||||
- Its category: CODE | TEST | MIGRATION | CONFIG | DOCS
|
||||
|
||||
### Verification Mode
|
||||
|
||||
Classify how each item can be verified. The diff cannot prove work in another repo or external system.
|
||||
|
||||
- **DIFF-VERIFIABLE** — A code change in this repo would manifest in `git diff origin/<base>`. Examples: "add UserService" (file appears), "validate input X" (validation logic appears), "create users table" (migration file appears).
|
||||
- **CROSS-REPO** — Item names a file or change in a sibling repo (e.g., `domain-hq/docs/dashboard.md`, `~/Development/<other-repo>/...`). The current diff CANNOT prove this.
|
||||
- **EXTERNAL-STATE** — Item names state in an external system: Supabase config/RLS, Cloudflare DNS, Vercel env vars, OAuth provider allowlists, third-party SaaS, DNS records. The current diff CANNOT prove this.
|
||||
- **CONTENT-SHAPE** — Item requires a file to follow a specific convention. If the file is in this repo: diff-verifiable. If in another repo or system: see CROSS-REPO / EXTERNAL-STATE.
|
||||
|
||||
**Verification dispatch:**
|
||||
|
||||
- **DIFF-VERIFIABLE** → cross-reference against diff (next section).
|
||||
- **CROSS-REPO** → if the sibling repo is reachable on disk (try `~/Development/<repo>/`, `~/code/<repo>/`, the parent of the current repo), run `[ -f <path> ]` to check file existence. File exists → DONE (cite path). File missing → NOT DONE (cite path). Path unreachable → UNVERIFIABLE (cite what needs manual check).
|
||||
- **EXTERNAL-STATE** → UNVERIFIABLE. Cite the system and the specific check the user must perform.
|
||||
- **CONTENT-SHAPE in another repo** → if the file exists, run any project-detected validator (see "Validator detection" below) before falling back to UNVERIFIABLE. With a validator: pass → DONE; fail → NOT DONE (cite validator output). No validator available: classify UNVERIFIABLE and cite both the file path and the convention to confirm.
|
||||
|
||||
**Path concreteness rule.** If a plan item names a *concrete filesystem path* (absolute, `~/...`, or `<sibling-repo>/<file>`), it MUST be classified DONE or NOT DONE based on `[ -f <path> ]`. UNVERIFIABLE is only valid when the path is genuinely abstract ("Cloudflare DNS", "Supabase allowlist") or the sibling root is unreachable on this machine. "I don't want to check" is not unreachable.
|
||||
|
||||
**Validator detection.** Before falling back to UNVERIFIABLE on a CONTENT-SHAPE item, scan the target repo's `package.json` for any script matching `validate-*`, `lint-wiki`, `check-docs`, or similar. If found, invoke it with the relevant path argument (e.g., `npm run validate-wiki -- <path>`). For multi-target validators (e.g., `validate-wiki --all`), run once and reconcile per-item from the output. A passing validator promotes the item from UNVERIFIABLE to DONE; a failing one demotes to NOT DONE.
|
||||
|
||||
**Honesty rule.** Do NOT classify an item as DONE just because related code shipped. Code that *handles* a deliverable is not the deliverable. Shipping a markdown-extraction library is not the same as shipping the markdown file. When in doubt between DONE and UNVERIFIABLE, prefer UNVERIFIABLE — better to surface a confirmation prompt than silently miss a deliverable.
|
||||
|
||||
### Cross-Reference Against Diff
|
||||
|
||||
Run `git diff origin/<base>` and `git log origin/<base>..HEAD --oneline` to understand what was implemented.
|
||||
|
||||
For each extracted plan item, run the verification dispatch from the previous section, then classify:
|
||||
|
||||
- **DONE** — Clear evidence the item shipped. Cite the specific file(s) changed in the diff for DIFF-VERIFIABLE items, or the verified path that exists for CROSS-REPO items with a reachable sibling repo.
|
||||
- **PARTIAL** — Some work toward this item exists but is incomplete (e.g., model created but controller missing, function exists but edge cases not handled).
|
||||
- **NOT DONE** — Verification ran and produced negative evidence (file missing, code absent in diff, sibling-repo file confirmed absent).
|
||||
- **CHANGED** — The item was implemented using a different approach than the plan described, but the same goal is achieved. Note the difference.
|
||||
- **UNVERIFIABLE** — The diff and any reachable sibling-repo checks cannot prove or disprove this. Always applies to EXTERNAL-STATE items and to CROSS-REPO items where the sibling repo isn't reachable. Cite the specific manual verification the user must perform (e.g., "check Cloudflare DNS shows DNS-only mode for dashboard.example.com", "confirm /docs/dashboard.md exists in domain-hq repo").
|
||||
|
||||
**Be conservative with DONE** — require clear evidence. A file being touched is not enough; the specific functionality described must be present.
|
||||
**Be generous with CHANGED** — if the goal is met by different means, that counts as addressed.
|
||||
**Be honest with UNVERIFIABLE** — better to surface 5 items the user must manually confirm than silently classify them DONE.
|
||||
|
||||
### Output Format
|
||||
|
||||
```
|
||||
PLAN COMPLETION AUDIT
|
||||
════════════════════
|
||||
Plan: {plan file path}
|
||||
|
||||
## Implementation Items
|
||||
[DONE] Create UserService — src/services/user_service.rb (+142 lines)
|
||||
[PARTIAL] Add validation — model validates but missing controller checks
|
||||
[NOT DONE] Add caching layer — no cache-related changes in diff
|
||||
[CHANGED] "Redis queue" → implemented with Sidekiq instead
|
||||
|
||||
## Test Items
|
||||
[DONE] Unit tests for UserService — test/services/user_service_test.rb
|
||||
[NOT DONE] E2E test for signup flow
|
||||
|
||||
## Migration Items
|
||||
[DONE] Create users table — db/migrate/20240315_create_users.rb
|
||||
|
||||
## Cross-Repo / External Items
|
||||
[DONE] sibling-repo has /docs/dashboard.md — verified at ~/Development/sibling-repo/docs/dashboard.md
|
||||
[UNVERIFIABLE] Cloudflare DNS-only on api.example.com — external system, manual check required
|
||||
[UNVERIFIABLE] Supabase auth allowlist contains user email — external system, confirm in Supabase dashboard
|
||||
|
||||
────────────────────
|
||||
COMPLETION: 4/10 DONE, 1 PARTIAL, 2 NOT DONE, 1 CHANGED, 2 UNVERIFIABLE
|
||||
────────────────────
|
||||
```
|
||||
|
||||
After your analysis, output a single JSON object with exactly these seven fields on the LAST LINE of your response (no other text after it):
|
||||
The audit's prompt is `ship/sections/audit-plan-completion.md`; build inlines it into the packet, so
|
||||
never read it yourself. Its `out=` file ends with one JSON line of exactly these seven fields:
|
||||
{"total_items":N,"done":N,"changed":N,"partial":N,"not_done":N,"unverifiable":N,"summary":"<markdown checklist for PR body>"}
|
||||
Counts map one-to-one to the classifications above and sum to total_items. No plan or no actionable items means all counts are zero with the skip reason in summary. Do not classify work as deferred; only the parent can record a user-approved deferral.
|
||||
````
|
||||
|
||||
**Parent processing:**
|
||||
|
||||
|
||||
@@ -20,18 +20,12 @@ or complete text, including user-approved scope changes. When discovery printed
|
||||
not-run line, skip dispatch. The child does not inherit the parent's conversation.
|
||||
**By packet:** `~/.claude/skills/gstack/bin/gstack-review-packet build --audit plan-completion
|
||||
--base <base> --plan-file <bound plan> --out-dir .gstack/tmp/review-packets-audit` writes the
|
||||
prompt below, the whole plan and the diff into one packet; brief the child only with Step 7's
|
||||
audit prompt, the whole plan and the diff into one packet; brief the child only with Step 7's
|
||||
packet brief and read the LAST line of its `out=` file.
|
||||
|
||||
````text
|
||||
You are running a ship-workflow plan completion audit. The base branch is `<base>`. Use `git diff origin/<base>` and inspect untracked files from `git status` to see the full proposed change. Do not commit or push. Report only: classify every item, but do not execute Gate Logic, ask the user, or advance the workflow. The parent applies those gates to your report.
|
||||
|
||||
{{PLAN_COMPLETION_AUDIT_SHIP}}
|
||||
|
||||
After your analysis, output a single JSON object with exactly these seven fields on the LAST LINE of your response (no other text after it):
|
||||
The audit's prompt is `ship/sections/audit-plan-completion.md`; build inlines it into the packet, so
|
||||
never read it yourself. Its `out=` file ends with one JSON line of exactly these seven fields:
|
||||
{"total_items":N,"done":N,"changed":N,"partial":N,"not_done":N,"unverifiable":N,"summary":"<markdown checklist for PR body>"}
|
||||
Counts map one-to-one to the classifications above and sum to total_items. No plan or no actionable items means all counts are zero with the skip reason in summary. Do not classify work as deferred; only the parent can record a user-approved deferral.
|
||||
````
|
||||
|
||||
**Parent processing:**
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@ Dispatch the audit through Agent with `subagent_type: "general-purpose"` and
|
||||
**By packet:** write the subagent-prompt inputs below to `.gstack/tmp/audit-context.txt`,
|
||||
run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverage --base <base>
|
||||
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
|
||||
--passes-used <N>` (the packet holds the prompt below and
|
||||
--passes-used <N>` (the packet holds the audit prompt and
|
||||
the diff; an audit is one packet, never sharded) and brief the child only with the packet brief and its `read=` ranges:
|
||||
"Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names."
|
||||
Wait for its `out=` file's LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
@@ -44,302 +44,9 @@ git worktree prune
|
||||
find "${TMPDIR:-/tmp}" -maxdepth 1 -name 'gstack-base-control.*' -mmin +10 2>/dev/null | while IFS= read -r d; do git worktree remove --force "$d/wt" >/dev/null 2>&1; rm -rf "$d"; done
|
||||
```
|
||||
|
||||
````text
|
||||
You are running a ship-workflow test coverage audit. Run `git diff origin/<base>` to include uncommitted tracked changes; also read relevant non-ignored untracked source/tests. Do not commit or push. Perform only this audit; return unresolved user decisions to the parent instead of asking or advancing to another workflow step.
|
||||
|
||||
Generation: <allowed|audit-only>; passes used: <N> of 2. Audit-only overrides every generation instruction below.
|
||||
|
||||
Coverage goal: every changed behavior is protected by a test that would catch a real regression. Test count is not a goal. Evaluate what was ACTUALLY coded (from the diff), not what was planned.
|
||||
|
||||
### Test Framework Detection
|
||||
|
||||
Before analyzing coverage, detect the project's test framework:
|
||||
|
||||
1. **Read CLAUDE.md** — look for a `## Testing` section with test command and framework name. If found, use that as the authoritative source.
|
||||
2. **If CLAUDE.md has no testing section, auto-detect:**
|
||||
|
||||
```bash
|
||||
setopt +o nomatch 2>/dev/null || true # zsh compat
|
||||
# Runtime markers: evidence, not commands to run
|
||||
[ -f manage.py ] && echo "RUNTIME:python FRAMEWORK:django"
|
||||
{ [ -f pyproject.toml ] || [ -f pytest.ini ] || [ -f tox.ini ] || [ -f setup.cfg ] || [ -f requirements.txt ]; } && echo "RUNTIME:python"
|
||||
{ [ -f Gemfile ] || [ -f Rakefile ] || [ -f .rspec ]; } && echo "RUNTIME:ruby"
|
||||
[ -f package.json ] && echo "RUNTIME:node"
|
||||
[ -f go.mod ] && echo "RUNTIME:go"
|
||||
[ -f Cargo.toml ] && echo "RUNTIME:rust"
|
||||
[ -f pubspec.yaml ] && echo "RUNTIME:dart"
|
||||
[ -f pom.xml ] && echo "RUNTIME:jvm BUILD:maven"
|
||||
{ [ -f build.gradle ] || [ -f build.gradle.kts ]; } && echo "RUNTIME:jvm BUILD:gradle"
|
||||
# Test infra: configs, scripts, test files
|
||||
ls jest.config.* vitest.config.* playwright.config.* cypress.config.* .rspec pytest.ini tox.ini phpunit.xml dart_test.yaml 2>/dev/null
|
||||
[ -f package.json ] && grep -q '"test"[[:space:]]*:' package.json && echo "SCRIPT:package.json test"
|
||||
[ -f Makefile ] && grep -qE '^(test|check):' Makefile && echo "TARGET:make test"
|
||||
git ls-files | grep -cE '(^|/)(tests?|spec|__tests__)/|(^|/)tests?\.py$|(^|/)test_[^/]+\.py$|_test\.(go|py|rb|ts|js|exs|dart)$|\.(test|spec)\.[jt]sx?$|_spec\.rb$|Test\.(java|kt)$' | sed 's/^/TESTFILES:/'
|
||||
```
|
||||
|
||||
3. **If no framework detected:** use the bootstrap decision already made in Step 4; report diagram-only coverage if setup was declined. Do not restart bootstrap from this audit.
|
||||
|
||||
**0. Before/after test count:**
|
||||
|
||||
```bash
|
||||
# Count test files before any generation
|
||||
git ls-files 2>/dev/null | grep -E '(\.test\.|\.spec\.|_test\.|_spec\.)' | wc -l
|
||||
```
|
||||
|
||||
Store this number for the PR body.
|
||||
|
||||
**1. Trace every codepath changed** using `git diff origin/<base>`:
|
||||
|
||||
Read every changed file. For each one, trace how data flows through the code — don't just list functions, actually follow the execution:
|
||||
|
||||
1. **Read the diff.** For each changed file, read the full file (not just the diff hunk) to understand context.
|
||||
Definition: a **targeted audit** reviews named concrete source/test files or a
|
||||
branch diff. A **prototype** is existing runnable code referenced by the plan,
|
||||
not a proposed future component.
|
||||
|
||||
When grounded in concrete source and test files, read them in a dedicated tool
|
||||
call before drawing the diagram. Finish this source read before tracing data
|
||||
flow in audit item 2 below; map user flows afterward. Do not mix diff, grep,
|
||||
package/config, git, or commentary into that read; use separate calls for
|
||||
context. Base the diagram on that read.
|
||||
2. **Trace data flow.** Starting from each entry point (route handler, exported function, event listener, component render), follow the data through every branch:
|
||||
- Where does input come from? (request params, props, database, API call)
|
||||
- What transforms it? (validation, mapping, computation)
|
||||
- Where does it go? (database write, API response, rendered output, side effect)
|
||||
- What can go wrong at each step? (null/undefined, invalid input, network failure, empty collection)
|
||||
3. **Diagram the execution.** For each changed file, draw an ASCII diagram showing:
|
||||
- Every function/method that was added or modified
|
||||
- Every conditional branch (if/else, switch, ternary, guard clause, early return)
|
||||
- Every error path (try/catch, rescue, error boundary, fallback)
|
||||
- Every call to another function (trace into it — does IT have untested branches?)
|
||||
- Every edge: what happens with null input? Empty array? Invalid type?
|
||||
|
||||
This is the critical step — you're building a map of every line of code that can execute differently based on input. Every branch in this diagram needs coverage that would catch a real regression; the test value bar below decides whether that is a new test, an extension of an existing one, or already covered.
|
||||
|
||||
**2. Map user flows, interactions, and error states:**
|
||||
|
||||
Code coverage isn't enough — you need to cover how real users interact with the changed code. For each changed feature, think through:
|
||||
|
||||
- **User flows:** What sequence of actions does a user take that touches this code? Map the full journey (e.g., "user clicks 'Pay' → form validates → API call → success/failure screen"). Each step in the journey needs coverage.
|
||||
- **Interaction edge cases:** What happens when the user does something unexpected?
|
||||
- Double-click/rapid resubmit
|
||||
- Navigate away mid-operation (back button, close tab, click another link)
|
||||
- Submit with stale data (page sat open for 30 minutes, session expired)
|
||||
- Slow connection (API takes 10 seconds — what does the user see?)
|
||||
- Concurrent actions (two tabs, same form)
|
||||
- **Error states the user can see:** For every error the code handles, what does the user actually experience?
|
||||
- Is there a clear error message or a silent failure?
|
||||
- Can the user recover (retry, go back, fix input) or are they stuck?
|
||||
- What happens with no network? With a 500 from the API? With invalid data from the server?
|
||||
- **Empty/zero/boundary states:** What does the UI show with zero results? With 10,000 results? With a single character input? With maximum-length input?
|
||||
|
||||
Add these to your diagram alongside the code branches. A user flow with no test is just as much a gap as an untested if/else.
|
||||
|
||||
**3. Check each branch against existing tests:**
|
||||
|
||||
Go through your diagram branch by branch — both code paths AND user flows. For each one, search for a test that exercises it:
|
||||
- Function `processPayment()` → look for `billing.test.ts`, `billing.spec.ts`, `test/billing_test.rb`
|
||||
- An if/else → look for tests covering BOTH the true AND false path
|
||||
- An error handler → look for a test that triggers that specific error condition
|
||||
- A call to `helperFn()` that has its own branches → those branches need tests too
|
||||
- A user flow → look for an integration or E2E test that walks through the journey
|
||||
- An interaction edge case → look for a test that simulates the unexpected action
|
||||
|
||||
Quality scoring rubric:
|
||||
- ★★★ Tests behavior with edge cases AND error paths
|
||||
- ★★ Tests correct behavior, happy path only
|
||||
- ★ Smoke test / existence check / trivial assertion (e.g., "it renders", "it doesn't throw"); weak, never counts as coverage
|
||||
|
||||
**Test value bar.** Propose or write a test only with all four answers; otherwise extend an existing test or drop it:
|
||||
|
||||
1. What observable behavior, invariant or independent contract does it protect?
|
||||
2. What credible regression makes it fail?
|
||||
3. Why does existing coverage not already catch that? Prefer adding a row to an existing table-driven test or shared fixture over a near-duplicate.
|
||||
4. Does it need a production seam (export, flag, wrapper, injection hook) that no production caller needs? If yes, test at the real boundary instead.
|
||||
|
||||
A test that breaks under a behavior-preserving refactor asserts implementation: rewrite it at the owning boundary, unless exact output is the declared contract (goldens, prompt bytes, wire formats).
|
||||
|
||||
Value card: `Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none; no_claim=<...>` (seam: `none` or its name; `no_claim`: one sentence on what green does not prove and which fixtures or mocks stand in for what, e.g. `no_claim=provider acceptance (mocked provider; unit only)` or `no_claim=none beyond protects`; it never waives a required live check); each field at most 160 UTF-8 bytes here (clamp to 157 plus `...`; written JSON keeps full values). Write it as a header comment in each generated test, next to the attribution (wrap, do not truncate); with no known comment syntax, put it in the PR body's Test value details. A missing upstream card never blocks: derive it; ignore unknown fields.
|
||||
|
||||
Example: Value: protects=refundPayment rejects an empty reason; fails_when=the reason guard is removed or inverted; why_new=billing.test.ts covers processPayment only; seam=none; no_claim=provider acceptance (mocked provider; unit only)
|
||||
Rejected (covered_elsewhere): "checkout renders"; checkout.e2e.ts:15 covers it, so extend that test.
|
||||
|
||||
Weak tests (★ smoke/existence/trivial, gate-failing or unrated) never count as coverage. X = paths with a ★★/★★★ test / total paths (value-weighted; the gate uses X); Y = paths with any test / total paths. Total paths = the diff's codepath trace, max 30; zero skips the gate. A path with only weak tests is uncovered in X, covered in Y, and goes to `weak_gaps` (reason `star_one|gate_failed|unrated`), not `gaps`. Rate stars only for tests reachable from changed paths.
|
||||
|
||||
Retention bar: keep a test that independently enforces a public API, protocol, config, migration, storage, security, platform, default, prompt-byte, generated-output (golden), package, release or architecture contract; static or slow is no reason to delete.
|
||||
|
||||
Regression proof: a regression test must fail at HEAD before any repair, in its own assertion (a pass at HEAD drops the regression label; an import, fixture or env failure is a test defect: correct once or drop). It must pass at base as the control (an assertion failure there marks it invalid; any other failure is "base control unavailable: collection error") and pass after the repair. Record: `Regression proof — fails at HEAD: yes · passes at base: yes | unavailable (<reason>) | manual · passes after fix: yes | pending`.
|
||||
|
||||
### E2E Test Decision Matrix
|
||||
|
||||
When checking each branch, also determine whether a unit test or E2E/integration test is the right tool:
|
||||
|
||||
**RECOMMEND E2E (mark as [→E2E] in the diagram):**
|
||||
- Common user flow spanning 3+ components/services (e.g., signup → verify email → first login)
|
||||
- Integration point where mocking hides real failures (e.g., API → queue → worker → DB)
|
||||
- Auth/payment/data-destruction flows — too important to trust unit tests alone
|
||||
|
||||
**RECOMMEND EVAL (mark as [→EVAL] in the diagram):**
|
||||
- Critical LLM call that needs a quality eval (e.g., prompt change → test output still meets quality bar)
|
||||
- Changes to prompt templates, system instructions, or tool definitions
|
||||
|
||||
**STICK WITH UNIT TESTS:**
|
||||
- Pure function with clear inputs/outputs
|
||||
- Internal helper with no side effects
|
||||
- Edge case of a single function (null input, empty array)
|
||||
- Obscure/rare flow that isn't customer-facing
|
||||
|
||||
### REGRESSION RULE (mandatory)
|
||||
|
||||
**IRON RULE:** When the coverage audit identifies a REGRESSION — code that previously worked but the diff broke — a regression test is written immediately. No AskUserQuestion. No skipping. Regressions are the highest-priority test because they prove something broke.
|
||||
|
||||
A regression is when:
|
||||
- The diff modifies existing behavior (not new code)
|
||||
- The existing test suite (if any) doesn't cover the changed path
|
||||
- The change introduces a new failure mode for existing callers
|
||||
|
||||
When uncertain whether a change is a regression, err on the side of writing the test.
|
||||
|
||||
**Red-first proof.** Apply the value bar's Regression proof to every regression test: the diff at HEAD is the pre-fix code, so run the new test at HEAD before any repair. Then, unless the parent says `Base control: off`, run this base control once per regression test in diff order, within a 3-minute total per /ship run (`Base control budget:` seconds per test, default 90). Past the total, record "base control unavailable: budget" and report "N of M regression tests got base control". The block is one shell invocation; it installs nothing and runs no build or postinstall.
|
||||
|
||||
```bash
|
||||
# Set: BASE = the base branch this /ship run resolved; TEST = the test file; FIXTURES = new
|
||||
# test-only fixtures it imports (repo-relative, may be empty); RUN = the detected
|
||||
# runner for one file (e.g. "bun test $TEST"); BUDGET = seconds for this run (default 90).
|
||||
ROOT=$(git rev-parse --show-toplevel)
|
||||
CTL_TMP=$(mktemp -d "${TMPDIR:-/tmp}/gstack-base-control.XXXXXX")
|
||||
cleanup() { git -C "$ROOT" worktree remove --force "$CTL_TMP/wt" >/dev/null 2>&1; rm -rf "$CTL_TMP"; [ -e "$CTL_TMP" ] && echo "BASE_CONTROL_LEFTOVER: $CTL_TMP (run: git worktree prune)"; }
|
||||
trap cleanup EXIT INT TERM
|
||||
(
|
||||
[ -f "$ROOT/package.json" ] || { echo "BASE_CONTROL: unavailable (ecosystem)"; exit 0; }
|
||||
git -C "$ROOT" remote get-url origin >/dev/null 2>&1 || { echo "BASE_CONTROL: unavailable (no base remote)"; exit 0; }
|
||||
timeout 30 git -C "$ROOT" fetch --quiet origin "$BASE" || { echo "BASE_CONTROL: unavailable (base not fetched)"; exit 0; }
|
||||
git -C "$ROOT" worktree add --quiet --detach "$CTL_TMP/wt" "origin/$BASE" >/dev/null 2>&1 || { echo "BASE_CONTROL: unavailable (worktree add failed)"; exit 0; }
|
||||
for f in $(echo "$TEST $FIXTURES"); do mkdir -p "$CTL_TMP/wt/$(dirname "$f")" && cp "$ROOT/$f" "$CTL_TMP/wt/$f"; done
|
||||
[ -d "$ROOT/node_modules" ] && ln -s "$ROOT/node_modules" "$CTL_TMP/wt/node_modules"
|
||||
cd "$CTL_TMP/wt" && timeout "${BUDGET:-90}" sh -c "$RUN" > "$CTL_TMP/out" 2>&1; rc=$?
|
||||
tail -n 40 "$CTL_TMP/out"
|
||||
if [ "$rc" -eq 0 ]; then echo "BASE_CONTROL: passes at base"
|
||||
elif [ "$rc" -eq 124 ]; then echo "BASE_CONTROL: unavailable (budget)"
|
||||
else echo "BASE_CONTROL: fails at base (exit $rc)"; fi
|
||||
)
|
||||
```
|
||||
|
||||
Classify "fails at base" from the output: a failure in the test's own assertion marks it invalid (correct once or drop it); an import, collection, missing generated artifact or dependency failure is "base control unavailable: collection error" and the test stays. Print each unavailable result as: base control unavailable: <reason>. The fails-at-HEAD result still stands. To check by hand: `git worktree add --detach <tmp> <base>`; copy the test and its new fixtures to the same paths; run the detected test command in <tmp>; `git worktree remove --force <tmp>`. Then report `passes at base: manual`. (see ~/.claude/skills/gstack/docs/test-value-bar.md#base-control-unavailable)
|
||||
|
||||
Return `"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}` counts in the JSON and each test's record line in the diagram.
|
||||
|
||||
**4. Output ASCII coverage diagram:**
|
||||
|
||||
For targeted audits, start Test review output with the coverage diagram. In full
|
||||
plan reviews, put it inside the normal Test review section. Required outputs
|
||||
keep the final terminal report order.
|
||||
|
||||
Include BOTH code paths and user flows in the same diagram. Mark E2E-worthy and eval-worthy paths:
|
||||
|
||||
```
|
||||
CODE PATHS USER FLOWS
|
||||
[+] src/services/billing.ts [+] Payment checkout
|
||||
├── processPayment() ├── [★★★ TESTED] Complete purchase — checkout.e2e.ts:15
|
||||
│ ├── [★★★ TESTED] happy + declined + timeout ├── [GAP] [→E2E] Double-click submit
|
||||
│ ├── [GAP] Network timeout └── [GAP] Navigate away mid-payment
|
||||
│ └── [GAP] Invalid currency
|
||||
└── refundPayment() [+] Error states
|
||||
├── [★★ TESTED] Full refund — :89 ├── [★★ TESTED] Card declined message
|
||||
└── [★ TESTED] Partial (non-throw only) — :101 └── [GAP] Network timeout UX
|
||||
|
||||
LLM integration: [GAP] [→EVAL] Prompt template change — needs eval test
|
||||
|
||||
COVERAGE: 5/13 paths tested (38%) | Code paths: 3/5 (60%) | User flows: 2/8 (25%)
|
||||
QUALITY: ★★★:2 ★★:2 ★:1 | GAPS: 8 (2 E2E, 1 eval)
|
||||
```
|
||||
|
||||
Legend: ★★★ behavior + edge + error | ★★ happy path | ★ smoke check
|
||||
[→E2E] = needs integration test | [→EVAL] = needs LLM eval
|
||||
|
||||
Avoid bare `[ ]` or `[x]` in diagrams unless the block includes
|
||||
`Legend: [x] tested | [ ] no test`. Prefer `[GAP]`, `[★★ TESTED]`,
|
||||
`[→E2E]`, `[→EVAL]`; keep user-flow markers off code-path rows.
|
||||
|
||||
**Fast path:** All paths covered → "Step 7: All new code paths have test coverage ✓" Continue.
|
||||
|
||||
**5. Generate tests for uncovered paths:**
|
||||
|
||||
If test framework detected (or bootstrapped in Step 4):
|
||||
- Apply the test value bar before writing each test. Extend an existing test (a new table row, fixture case or assertion) before creating a file. Record every proposal you decline in `tests_rejected` with a `reason_code` from `duplicate_protects, needs_seam, incomplete_card, no_credible_regression, covered_elsewhere, implementation_coupled`.
|
||||
- Never add a production seam for a test; a seam that is not `none` names its non-test callers: `seam=<name> (non-test callers: N, via <search command>)`.
|
||||
- Write the value card as a header comment in each generated or extended test.
|
||||
- Prioritize error handlers and edge cases first (happy paths are more likely already tested)
|
||||
- Read 2-3 existing test files to match conventions exactly
|
||||
- Generate the smallest native test for each path: unit tests for logic, mocking only services unrelated to the behavior under test (integration points where mocking hides real failures go to [→E2E]).
|
||||
- For paths marked [→E2E]: generate integration/E2E tests using the project's E2E framework (Playwright, Cypress, Capybara, etc.)
|
||||
- For paths marked [→EVAL]: generate eval tests using the project's eval framework, or flag for manual eval if none exists
|
||||
- Write tests that exercise the specific uncovered path with real assertions
|
||||
- Run each test. Passes → keep the change and report its path; the parent commits in Step 15.
|
||||
- Fails → diagnose whether the test/fixture is invalid or a declared product contract is broken. Correct a demonstrated test defect once; preserve a valid red regression and route the reproduced product failure through the parent's fix/approval flow. Never delete or weaken it to manufacture green; retain unresolved coverage in the diagram.
|
||||
|
||||
Caps: 30 code paths max; 5 tests per generation pass (code + user flow combined; the parent's `Generation cap:` overrides 5); an extension uses one slot and a rejection uses none; 2-min per-test exploration cap. List each remaining gap below the diagram (inside `diagram`) as a proposed test with its value card.
|
||||
|
||||
Do not rate stars for tests you wrote in this pass: count them as unrated (weak, reason `unrated`). The parent's read-only rating dispatch rates them. Counts are disjoint, precedence extended > added > rejected: one gap lands in at most one of `tests_extended`, `tests_added`, `tests_rejected`.
|
||||
|
||||
If no test framework AND user declined bootstrap → diagram only, no generation. Note: "Test generation skipped — no test framework configured."
|
||||
|
||||
**Diff is test-only changes:** Return a skipped audit with null coverage, zero gaps, and "No new application code paths to audit."
|
||||
|
||||
**6. After-count and coverage summary:**
|
||||
|
||||
```bash
|
||||
# Count test files after generation
|
||||
git ls-files 2>/dev/null | grep -E '(\.test\.|\.spec\.|_test\.|_spec\.)' | wc -l
|
||||
```
|
||||
|
||||
For PR body: `Tests: {before} → {after} (+{delta} new)`
|
||||
Coverage line: `Test Coverage Audit: N new code paths. M covered (Y% any test, X% value-weighted). K tests generated, awaiting parent commit.`
|
||||
|
||||
### Test Plan Artifact
|
||||
|
||||
After producing the coverage diagram, write a test plan artifact so `/qa` and `/qa-only` can consume it:
|
||||
|
||||
```bash
|
||||
GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); : "${GSTACK_STATE_ROOT:?gstack-paths failed; reinstall with ./setup or /gstack-upgrade}"
|
||||
SLUG=$(~/.claude/skills/gstack/bin/gstack-slug --get SLUG 2>/dev/null) && mkdir -p "$GSTACK_STATE_ROOT/projects/$SLUG" && echo "PROJECT_DIR: $GSTACK_STATE_ROOT/projects/$SLUG"
|
||||
USER=$(whoami)
|
||||
DATETIME=$(date +%Y%m%d-%H%M%S)
|
||||
```
|
||||
|
||||
Write to `<PROJECT_DIR>/{user}-{branch}-ship-test-plan-{datetime}.md` (`PROJECT_DIR` printed above):
|
||||
|
||||
```markdown
|
||||
# Test Plan
|
||||
Generated by /ship on {date}
|
||||
Branch: {branch}
|
||||
Repo: {owner/repo}
|
||||
|
||||
## Affected Pages/Routes
|
||||
- {URL path} — {what to test and why}
|
||||
|
||||
## Key Interactions to Verify
|
||||
- {interaction description} on {page}
|
||||
|
||||
## Edge Cases
|
||||
- {edge case} on {page}
|
||||
|
||||
## Critical Paths
|
||||
- {end-to-end flow that must work}
|
||||
```
|
||||
|
||||
Rules for the implementing agent:
|
||||
1. Acceptance criteria are the `protects / fails_when / no_claim` profile of each test: the observable it protects, the wrong result it rejects, and what green does not prove. Never "make the tests pass".
|
||||
2. Real code and real tests land in the same change; a placeholder, a stub assertion or a test deferred to a follow-up is not done.
|
||||
3. Any change to a test, validator, CI step, timeout, tolerance, threshold, snapshot or suppression pragma is reported on its own line in your summary with its justification, before any other result.
|
||||
4. A reported command names the command and its observed result (exit status, counts, the failing line); a command you did not run is not reported, and stderr is never silenced in a command you cite.
|
||||
5. Your report is a claim until the parent re-executes it: say what you verified, what you reused, and what you could not check.
|
||||
Report any such change in the prose before the JSON line; the JSON fields below are unchanged.
|
||||
|
||||
After your analysis, output a single JSON object on the LAST LINE of your response (no other text after it):
|
||||
The audit's prompt is `ship/sections/audit-test-coverage.md`; build inlines it into the packet, so
|
||||
never read it yourself. Its `out=` file ends with one JSON line:
|
||||
{"coverage_pct":N,"gaps":N,"diagram":"<full markdown coverage diagram for PR body>","tests_added":["path",...],"coverage_pct_value":N,"weak_gaps":[{"path":"...","existing_test":"...","reason":"star_one|gate_failed|unrated"}],"tests_extended":["path",...],"tests_rejected":[{"path_or_gap":"...","reason_code":"...","reason":"..."}],"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}}
|
||||
`coverage_pct` is Y (paths with any test), `coverage_pct_value` is X (paths with a ★★/★★★ test), `gaps` counts only paths with no test. Use null for an undetermined or skipped coverage percentage, not zero. Include every remaining gap in the diagram so the parent can target a second pass.
|
||||
````
|
||||
|
||||
**Parent processing:**
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ Dispatch the audit through Agent with `subagent_type: "general-purpose"` and
|
||||
**By packet:** write the subagent-prompt inputs below to `.gstack/tmp/audit-context.txt`,
|
||||
run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverage --base <base>
|
||||
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
|
||||
--passes-used <N>` (the packet holds the prompt below and
|
||||
--passes-used <N>` (the packet holds the audit prompt and
|
||||
the diff; an audit is one packet, never sharded) and brief the child only with the packet brief and its `read=` ranges:
|
||||
"Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names."
|
||||
Wait for its `out=` file's LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
@@ -42,21 +42,9 @@ git worktree prune
|
||||
find "${TMPDIR:-/tmp}" -maxdepth 1 -name 'gstack-base-control.*' -mmin +10 2>/dev/null | while IFS= read -r d; do git worktree remove --force "$d/wt" >/dev/null 2>&1; rm -rf "$d"; done
|
||||
```
|
||||
|
||||
````text
|
||||
You are running a ship-workflow test coverage audit. Run `git diff origin/<base>` to include uncommitted tracked changes; also read relevant non-ignored untracked source/tests. Do not commit or push. Perform only this audit; return unresolved user decisions to the parent instead of asking or advancing to another workflow step.
|
||||
|
||||
Generation: <allowed|audit-only>; passes used: <N> of 2. Audit-only overrides every generation instruction below.
|
||||
|
||||
{{TEST_COVERAGE_AUDIT_SHIP}}
|
||||
|
||||
Rules for the implementing agent:
|
||||
{{IMPLEMENTER_DISPATCH_RULES}}
|
||||
Report any such change in the prose before the JSON line; the JSON fields below are unchanged.
|
||||
|
||||
After your analysis, output a single JSON object on the LAST LINE of your response (no other text after it):
|
||||
The audit's prompt is `ship/sections/audit-test-coverage.md`; build inlines it into the packet, so
|
||||
never read it yourself. Its `out=` file ends with one JSON line:
|
||||
{"coverage_pct":N,"gaps":N,"diagram":"<full markdown coverage diagram for PR body>","tests_added":["path",...],"coverage_pct_value":N,"weak_gaps":[{"path":"...","existing_test":"...","reason":"star_one|gate_failed|unrated"}],"tests_extended":["path",...],"tests_rejected":[{"path_or_gap":"...","reason_code":"...","reason":"..."}],"regression_proof":{"red_at_head":N,"base_green":N,"base_unavailable":N}}
|
||||
`coverage_pct` is Y (paths with any test), `coverage_pct_value` is X (paths with a ★★/★★★ test), `gaps` counts only paths with no test. Use null for an undetermined or skipped coverage percentage, not zero. Include every remaining gap in the diagram so the parent can target a second pass.
|
||||
````
|
||||
|
||||
**Parent processing:**
|
||||
|
||||
|
||||
@@ -68,12 +68,12 @@ describe('/ship Step 7 child prompt carries the rules before an unchanged JSON p
|
||||
const FIELDS = ['coverage_pct', 'gaps', 'diagram', 'tests_added', 'coverage_pct_value', 'weak_gaps', 'tests_extended', 'tests_rejected', 'regression_proof'];
|
||||
|
||||
test('template order: audit, rules, JSON protocol', () => {
|
||||
const child = between(read('ship/sections/test-coverage.md.tmpl'), '````text', '````\n\n**Parent processing:**');
|
||||
const child = between(read('ship/sections/audit-test-coverage.md.tmpl'), '````text', '````');
|
||||
expectOrdered(child, ['{{TEST_COVERAGE_AUDIT_SHIP}}', `${IMPLEMENTER_DISPATCH_RULES_HEADING}:`, '{{IMPLEMENTER_DISPATCH_RULES}}', 'output a single JSON object on the LAST LINE', JSON_LINE_PREFIX], 'ship child prompt');
|
||||
});
|
||||
|
||||
test('rendered prompt carries the list and the LAST-line field set is unchanged', () => {
|
||||
const child = between(read('ship/sections/test-coverage.md'), '````text', '````\n\n**Parent processing:**');
|
||||
const child = between(read('ship/sections/audit-test-coverage.md'), '````text', '````');
|
||||
expect(child).toContain(IMPLEMENTER_DISPATCH_RULES);
|
||||
expect(child.indexOf(IMPLEMENTER_DISPATCH_RULES)).toBeLessThan(child.indexOf(JSON_LINE_PREFIX));
|
||||
const line = child.split('\n').find(l => l.startsWith(JSON_LINE_PREFIX))!;
|
||||
|
||||
@@ -60,7 +60,7 @@ test('/ship runs the gate scan before the checklist read, again after the covera
|
||||
const parent = between(coverage, '**Parent processing:**', '**Audit failure:**');
|
||||
expectOrdered(parent, ['6. Print a one-line summary', '7. **Rescan gate edits after the child returns.**', '{{GATE_SCAN_BLOCK}}', 'Gate Integrity'], 'Step 7 parent rescan');
|
||||
expectMentions(compact(parent), [['never', 'paths', 'child', 'reported'], ['child', 'green', 'claim']], 'Step 7 parent rescan');
|
||||
const child = between(coverage, '````text', '````');
|
||||
const child = between(read('ship/sections/audit-test-coverage.md.tmpl'), '````text', '````');
|
||||
expectAbsent(child, ['GATE_SCAN_BLOCK', 'gate_id'], 'Step 7 child prompt');
|
||||
expect(child).toContain('{"coverage_pct":N,"gaps":N,"diagram":');
|
||||
});
|
||||
|
||||
@@ -119,7 +119,7 @@ describe('ship/SKILL.md — Plan Completion gate invariants', () => {
|
||||
});
|
||||
|
||||
test('successful plan audits use the declared seven-field contract without an error field', () => {
|
||||
const audit = fs.readFileSync(path.join(SHIP_DIR, 'sections/plan-completion.md'), 'utf8');
|
||||
const audit = ['plan-completion.md', 'audit-plan-completion.md'].map(f => fs.readFileSync(path.join(SHIP_DIR, 'sections', f), 'utf8')).join('\n');
|
||||
const line = audit.split('\n').find(line => line.startsWith('{"total_items":N,'));
|
||||
expect(line).toBeDefined();
|
||||
const contract = JSON.parse(line!.replace(/:N([,}])/g, ':0$1'));
|
||||
@@ -330,8 +330,12 @@ describe('F1: plan binding, docs/designs candidates, exact not-run line', () =>
|
||||
|
||||
test('ship binds the plan in the parent before dispatch; the child never searches', () => {
|
||||
const md = fs.readFileSync(sections.ship, 'utf8');
|
||||
expect(md.indexOf('### Plan File Discovery')).toBeLessThan(md.indexOf('````text'));
|
||||
const child = md.slice(md.indexOf('````text'), md.lastIndexOf('````'));
|
||||
const prompt = fs.readFileSync(path.join(ROOT, 'ship', 'sections', 'audit-plan-completion.md'), 'utf8');
|
||||
expect(md).toContain('### Plan File Discovery');
|
||||
expect(md).not.toContain('````text');
|
||||
expect(md).toContain('ship/sections/audit-plan-completion.md');
|
||||
const child = prompt.slice(prompt.indexOf('````text'), prompt.lastIndexOf('````'));
|
||||
expect(child.length).toBeGreaterThan(0);
|
||||
expect(child).not.toContain('### Plan File Discovery');
|
||||
expectMentions(child, [['do not', 'another', 'search']], 'child');
|
||||
expect(md).toMatch(/not-run line, skip dispatch/);
|
||||
@@ -362,7 +366,8 @@ describe('ACCEPTANCE_EDITED: parent-computed acceptance-text drift', () => {
|
||||
const parent = between(rendered, '**Parent processing:**', '**Audit-failure fallback:**');
|
||||
expectOrdered(parent, ['1. Check the task', `\`${ACCEPTANCE_EDITED}\` beside the child's classes`, 'Apply Gate Logic below', '### Acceptance edits (parent-computed)'], 'parent processing');
|
||||
expect(rendered.indexOf('### Acceptance edits (parent-computed)')).toBeLessThan(rendered.indexOf('### Gate Logic'));
|
||||
expectAbsent(between(rendered, '````text', '````\n\n**Parent processing:**'), [ACCEPTANCE_EDITED], 'child prompt');
|
||||
const prompt = fs.readFileSync(path.join(SHIP_DIR, 'sections', 'audit-plan-completion.md'), 'utf8');
|
||||
expectAbsent(between(prompt, '````text', /````\s*$/), [ACCEPTANCE_EDITED], 'child prompt');
|
||||
const contract = rendered.split('\n').find(line => line.startsWith('{"total_items":N,'))!;
|
||||
expect(Object.keys(JSON.parse(contract.replace(/:N([,}])/g, ':0$1')))).toHaveLength(7);
|
||||
});
|
||||
|
||||
@@ -14,6 +14,7 @@ const readShip = () => readWorkflowExcerpt('ship/SKILL.md', '# Ship:', '## Impor
|
||||
const readTemplate = (name: string) => readFileSync(new URL(`../${name}`, import.meta.url), 'utf8');
|
||||
const reviewTemplate = readTemplate('ship/sections/review-army.md.tmpl');
|
||||
const coverageTemplate = readTemplate('ship/sections/test-coverage.md.tmpl');
|
||||
const coveragePrompt = readTemplate('ship/sections/audit-test-coverage.md.tmpl');
|
||||
const entryTemplate = readTemplate('ship/SKILL.md.tmpl');
|
||||
const controlTemplate = between(entryTemplate, '### Ship control flow', '{{SECTION_INDEX:ship}}');
|
||||
const entry = compact(entryTemplate);
|
||||
@@ -68,9 +69,9 @@ test('named QA risks need explicit acceptance through AskUserQuestion', () => {
|
||||
});
|
||||
|
||||
test('coverage generation has one bounded allowance and an audit-only child prompt', () => {
|
||||
const allowance = compact(between(coverageTemplate, /^/, '````text'));
|
||||
expectTokens(allowance, ['2 generation passes', '30-path/5-tests-per-pass/2-minute'], 'coverage allowance');
|
||||
const prompt = coverageTemplate.split('````text\n')[1]?.split('\n````')[0] ?? '';
|
||||
expect(coverageTemplate).not.toContain('````text');
|
||||
expectTokens(compact(coverageTemplate), ['2 generation passes', '30-path/5-tests-per-pass/2-minute', 'ship/sections/audit-test-coverage.md'], 'coverage allowance');
|
||||
const prompt = coveragePrompt.split('````text\n')[1]?.split('\n````')[0] ?? '';
|
||||
expectTokens(prompt, ['Generation: <allowed|audit-only>; passes used: <N> of 2.', '"coverage_pct":N,"gaps":N', '"tests_added":["path",...]'], 'coverage child prompt');
|
||||
expectOrdered(prompt, [/audit-only/, '{{TEST_COVERAGE_AUDIT_SHIP}}'], 'coverage child prompt');
|
||||
expectMentions(prompt, [['not', 'commit'], ['not', 'push']], 'coverage child prompt');
|
||||
|
||||
@@ -27,7 +27,7 @@ const bar = (mode: TestValueBarMode) => generateTestValueBar(ctx, [mode]);
|
||||
|
||||
const CARD_FORMAT = 'Value: protects=<...>; fails_when=<...>; why_new=<...>; seam=none; no_claim=<...>';
|
||||
const NO_CLAIM_EXAMPLE_TOKENS = NO_CLAIM_EXAMPLES.map(example => `no_claim=${example}`);
|
||||
const shipSection = read('ship/sections/test-coverage.md');
|
||||
const shipSection = read('ship/sections/test-coverage.md') + '\n' + read('ship/sections/audit-test-coverage.md');
|
||||
const shipGate = generateTestCoverageAuditInner(ctx, 'ship', 'gate');
|
||||
|
||||
const CONTRACT: Record<TestValueBarMode, { render: string; required: string[]; absent?: string[] }> = {
|
||||
|
||||
@@ -141,7 +141,7 @@ describe('selectTests', () => {
|
||||
const consumers = [
|
||||
['plan-eng-review/sections/review-sections.md', 'TEST_COVERAGE_AUDIT_PLAN'],
|
||||
['ship/sections/tests.md', 'TEST_BOOTSTRAP'],
|
||||
['ship/sections/test-coverage.md', 'TEST_COVERAGE_AUDIT_SHIP'],
|
||||
['ship/sections/audit-test-coverage.md', 'TEST_COVERAGE_AUDIT_SHIP'],
|
||||
['design-review/sections/test-bootstrap.md', 'TEST_BOOTSTRAP'],
|
||||
];
|
||||
for (const [output, token] of consumers) {
|
||||
@@ -215,7 +215,7 @@ describe('selectTests', () => {
|
||||
['ship/sections/tests.md', 'ship/SKILL.md workflow',
|
||||
'ship/SKILL.md', '# Ship:', '## Important Rules', '## Test Framework Bootstrap'],
|
||||
['ship/sections/test-coverage.md', 'ship/SKILL.md workflow',
|
||||
'ship/SKILL.md', '# Ship:', '## Important Rules', '### REGRESSION RULE (mandatory)'],
|
||||
'ship/SKILL.md', '# Ship:', '## Important Rules', '### Shared subagent dispatch'],
|
||||
['plan-design-review/sections/review-sections.md', 'plan-design-review/SKILL.md passes',
|
||||
'plan-design-review/SKILL.md', '## Review Sections', ASK_QUESTIONS_HEADING,
|
||||
'## Review Sections (7 passes, after scope is agreed)'],
|
||||
|
||||
Reference in new issue
Block a user