diff --git a/AGENTS.md b/AGENTS.md index 4df7eec35..d7ea3c6bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,7 @@ Invoke them by name (e.g., `/office-hours`). | Skill | What it does | |-------|-------------| -| `/ship` | Run tests, review, push, open PR. Workspace-aware version queue. | +| `/ship` | Run tests, review, push, open PR, and return a full engineering handoff plus concise Why / What / How. UI changes require matched Before/After proof before push. Workspace-aware version queue. | | `/land-and-deploy` | Merge the PR, wait for CI and deploy, verify production health. | | `/canary` | Post-deploy monitoring loop using the browse daemon. | | `/landing-report` | Read-only dashboard for the workspace-aware ship queue. | diff --git a/CHANGELOG.md b/CHANGELOG.md index 351e7cabd..24f6b3737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Changelog +## [1.60.2.0] - 2026-07-19 + +## **Every `/ship` now ends with a complete engineering handoff and a plain-language answer.** +## **UI changes cannot be pushed without faithful before/after evidence.** + +`/ship` previously optimized the mechanics of landing code but left its final user-facing report underspecified. A successful run could end at a PR URL, omit important engineering context, or describe a UI change without proving what visually changed. This release makes the completion contract explicit across supported hosts. + +### What this means for builders + +Every ship attempt now reports the outcome, root cause, decisions, implementation, verification, operational risk, remaining work, and any decision required. The literal final section is an extremely concise **Why / What / How** explanation. User-visible UI changes additionally require matched Before/After screenshots from faithful base and current states before the push; missing evidence blocks the ship instead of becoming a waivable footnote. + +### Itemized changes + +#### Changed + +- `ship/SKILL.md.tmpl`: adds the mandatory full engineering handoff, makes `### Put simply` the final response section, and moves UI screenshot evidence into the pre-push verification gate. +- Generated Claude, Codex, and Factory ship skills and golden fixtures carry the same completion contract. +- The UI evidence contract pins provenance, route/state/data/viewport/theme/zoom parity, safe test data, inline artifact paths, and a hard stop before push or PR creation when a faithful comparison is impossible. + +#### For contributors + +- `test/ship-completion-report.test.ts` regression-locks every required heading, ordering, UI gate, screenshot contract, and final plain-language section across the template and three host variants. +- `test/skill-llm-eval.test.ts` adds both an instruction-quality judge and a three-scenario behavior eval covering non-UI success, complete UI evidence, and blocked incomplete UI evidence. +- Ship coverage/touchfile routing now selects the deterministic and LLM completion tests. Focused verification: 505 passed, 27 evals skipped without API credentials, 0 failed; a manual authenticated Sonnet behavior run passed all three scenarios. + ## [1.60.1.0] - 2026-07-09 ## **The /autoplan dual-voice eval is back on the board, catching real regressions.** diff --git a/README.md b/README.md index af534d2c5..21b3fd08a 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,10 @@ You: /qa https://staging.myapp.com [opens real browser, clicks through flows, finds and fixes a bug] You: /ship +Claude: [full engineering handoff: outcome, root cause, decisions, + implementation, verification, risk, remaining work, next decision] Tests: 42 → 51 (+9 new). PR: github.com/you/app/pull/42 + Put simply: Why / What / How ``` You said "daily briefing app." The agent said "you're building a chief of staff AI" — because it listened to your pain, not your feature request. Eight commands, end to end. That is not a copilot. That is a team. @@ -194,7 +197,7 @@ Each skill feeds into the next. `/office-hours` writes a design doc that `/plan- | `/qa-only` | **QA Reporter** | Same methodology as /qa but report only. Pure bug report without code changes. | | `/pair-agent` | **Multi-Agent Coordinator** | Share your browser with any AI agent. One command, one paste, connected. Works with OpenClaw, Hermes, Codex, Cursor, or anything that can curl. Each agent gets its own tab. Auto-launches headed mode so you watch everything. Auto-starts ngrok tunnel for remote agents. Scoped tokens, tab isolation, rate limiting, activity attribution. | | `/cso` | **Chief Security Officer** | OWASP Top 10 + STRIDE threat model. Zero-noise: 17 false positive exclusions, 8/10+ confidence gate, independent finding verification. Each finding includes a concrete exploit scenario. | -| `/ship` | **Release Engineer** | Sync main, run tests, audit coverage, push, open PR. Bootstraps test frameworks if you don't have one. | +| `/ship` | **Release Engineer** | Sync main, run tests, audit coverage, push, open PR, then return a full engineering handoff plus concise Why / What / How. UI changes require matched Before/After proof before push. | | `/land-and-deploy` | **Release Engineer** | Merge the PR, wait for CI and deploy, verify production health. One command from "approved" to "verified in production." | | `/canary` | **SRE** | Post-deploy monitoring loop. Watches for console errors, performance regressions, and page failures. | | `/benchmark` | **Performance Engineer** | Baseline page load times, Core Web Vitals, and resource sizes. Compare before/after on every PR. | diff --git a/VERSION b/VERSION index c4190e004..762f17554 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.60.1.0 +1.60.2.0 diff --git a/docs/skills.md b/docs/skills.md index 8e8cb7adc..b188fb399 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -19,7 +19,7 @@ Detailed guides for every gstack skill — philosophy, workflow, and examples. | [`/qa-only`](#qa) | **QA Reporter** | Same methodology as /qa but report only. Use when you want a pure bug report without code changes. | | [`/scrape`](#scrape) | **Browser Data Extractor** | Pull data from a web page. First call prototypes via `$B`; subsequent calls on a matching intent run a codified browser-skill in ~200ms. | | [`/skillify`](#skillify) | **Skill Codifier** | Walks back through your conversation, finds the last `/scrape` prototype, synthesizes script + test + fixture, runs the test, asks before committing. | -| [`/ship`](#ship) | **Release Engineer** | Sync main, run tests, audit coverage, push, open PR. Bootstraps test frameworks if you don't have one. One command. | +| [`/ship`](#ship) | **Release Engineer** | Sync main, run tests, audit coverage, push, open PR, then return a full engineering handoff plus concise Why / What / How. UI changes require matched Before/After proof before push. | | [`/land-and-deploy`](#land-and-deploy) | **Release Engineer** | Merge the PR, wait for CI and deploy, verify production health. One command from "approved" to "verified in production." | | [`/canary`](#canary) | **SRE** | Post-deploy monitoring loop. Watches for console errors, performance regressions, and page failures using the browse daemon. | | [`/benchmark`](#benchmark) | **Performance Engineer** | Baseline page load times, Core Web Vitals, and resource sizes. Compare before/after on every PR. Track trends over time. | @@ -657,6 +657,12 @@ Every `/ship` run builds a code path map from your diff, searches for correspond `/ship` checks the [Review Readiness Dashboard](#review-readiness-dashboard) before creating the PR. If the Eng Review is missing, it asks — but won't block you. Decisions are saved per-branch so you're never re-asked. +### Engineering handoff and UI proof + +Every `/ship` attempt ends with the same complete report, whether it succeeds or stops: outcome, root cause, decisions, implementation, verification, operational risk, remaining work, and any decision you need to make. The last section compresses the result into an extremely concise **Why / What / How**. + +For a user-visible UI change, `/ship` captures matched **Before** and **After** screenshots from faithful base and current states before push. Route, app state, data, viewport, theme, and zoom must match. If the comparison cannot be produced safely and faithfully, `/ship` stops before pushing or creating the PR instead of waiving the evidence. + A lot of branches die when the interesting work is done and only the boring release work is left. Humans procrastinate that part. AI should not. --- diff --git a/package.json b/package.json index 846438a60..78b98a86f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.60.1.0", + "version": "1.60.2.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/ship/SKILL.md b/ship/SKILL.md index eadffaa8f..aaa7396b0 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -843,7 +843,7 @@ branch name wherever the instructions say "the base branch" or ``. # Ship: Fully Automated Ship Workflow -You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and output the PR URL at the end. +You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and include the PR URL in the final engineering handoff. **Only stop for:** - On the base branch (abort) @@ -1246,7 +1246,11 @@ Before pushing, re-verify if code changed during Steps 4-6: 2. **Build verification:** If the project has a build step, run it. Paste output. -3. **Rationalization prevention:** +3. **UI evidence gate:** For a user-visible UI diff, produce the matched + Before/After evidence from Step 22 before push. If unavailable, STOP; do not + push or create/update the PR/MR. + +4. **Rationalization prevention:** - "Should work now" → RUN IT. - "I'm confident" → Confidence is not evidence. - "I already tested earlier" → Code changed since then. Test again. @@ -1415,3 +1419,64 @@ through `gstack-version-bump`; never hand-roll the VERSION/package.json write. - **Never push without fresh verification evidence.** If code changed after Step 5 tests, re-run before pushing. - **Step 7 generates coverage tests.** They must pass before committing. Never commit failing tests. - **The goal is: user says `/ship`, next thing they see is the review + PR URL + auto-synced docs.** + +--- + +## Step 22: Full engineering handoff + +End every ship attempt with this report, even when stopped. Use each heading; +write `Not applicable` briefly when a category does not apply. + +```markdown +## Engineering summary + +### Outcome +What shipped (version, branch, commit, PR/MR URL), or what blocked it. + +### Problem and root cause +What was wrong or missing, why, and the supporting evidence. + +### Investigation and decisions +Key findings, alternatives, decisions, and why this approach won. + +### Implementation +Changed behavior, components, data paths, APIs, migrations, infrastructure, and docs. + +### Verification +Fresh test/build/typecheck/lint/behavior/review/CI/deploy evidence; mark pass/fail/skipped. + +### Risks and operational impact +Tradeoffs, compatibility, rollout/migration, observability, rollback. + +### Remaining work +Unfinished work. Write `None` when the completion contract is fully satisfied. + +### Decision required +The user's next decision. Write `None` when no decision is required. +``` + +### UI before/after evidence + +For user-visible UI changes, include matched **Before** and **After** screenshots: + +1. Use a pre-change capture only if it faithfully represents the base; otherwise run the merge-base/base revision in an isolated worktree or use its known deployment. +2. Capture the current branch after implementation and verification. +3. Match route, application state, data, viewport, theme, and zoom. Use safe test data; never expose credentials, customer data, or private information. +4. Show `![Before](path)` and `![After](path)` inline; include both artifact paths. +5. Do not commit screenshot artifacts unless the repository explicitly requires it. + +If a faithful comparison is impossible, use the same report with Outcome blocked, +mark UI evidence incomplete, and STOP before Step 17. Do not push, create/update +the PR/MR, claim UI completion, or offer a waiver. + +### Put simply + +Final response section; nothing follows. Be intuitive, extremely concise, precise: + +```markdown +### Put simply + +- **Why:** The problem this solves. +- **What:** What is now different for the user or system. +- **How:** The essential mechanism, without unnecessary implementation detail. +``` diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 068ac4fe5..15460e69a 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -34,7 +34,7 @@ triggers: # Ship: Fully Automated Ship Workflow -You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and output the PR URL at the end. +You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and include the PR URL in the final engineering handoff. **Only stop for:** - On the base branch (abort) @@ -367,7 +367,11 @@ Before pushing, re-verify if code changed during Steps 4-6: 2. **Build verification:** If the project has a build step, run it. Paste output. -3. **Rationalization prevention:** +3. **UI evidence gate:** For a user-visible UI diff, produce the matched + Before/After evidence from Step 22 before push. If unavailable, STOP; do not + push or create/update the PR/MR. + +4. **Rationalization prevention:** - "Should work now" → RUN IT. - "I'm confident" → Confidence is not evidence. - "I already tested earlier" → Code changed since then. Test again. @@ -535,3 +539,64 @@ through `gstack-version-bump`; never hand-roll the VERSION/package.json write. - **Never push without fresh verification evidence.** If code changed after Step 5 tests, re-run before pushing. - **Step 7 generates coverage tests.** They must pass before committing. Never commit failing tests. - **The goal is: user says `/ship`, next thing they see is the review + PR URL + auto-synced docs.** + +--- + +## Step 22: Full engineering handoff + +End every ship attempt with this report, even when stopped. Use each heading; +write `Not applicable` briefly when a category does not apply. + +```markdown +## Engineering summary + +### Outcome +What shipped (version, branch, commit, PR/MR URL), or what blocked it. + +### Problem and root cause +What was wrong or missing, why, and the supporting evidence. + +### Investigation and decisions +Key findings, alternatives, decisions, and why this approach won. + +### Implementation +Changed behavior, components, data paths, APIs, migrations, infrastructure, and docs. + +### Verification +Fresh test/build/typecheck/lint/behavior/review/CI/deploy evidence; mark pass/fail/skipped. + +### Risks and operational impact +Tradeoffs, compatibility, rollout/migration, observability, rollback. + +### Remaining work +Unfinished work. Write `None` when the completion contract is fully satisfied. + +### Decision required +The user's next decision. Write `None` when no decision is required. +``` + +### UI before/after evidence + +For user-visible UI changes, include matched **Before** and **After** screenshots: + +1. Use a pre-change capture only if it faithfully represents the base; otherwise run the merge-base/base revision in an isolated worktree or use its known deployment. +2. Capture the current branch after implementation and verification. +3. Match route, application state, data, viewport, theme, and zoom. Use safe test data; never expose credentials, customer data, or private information. +4. Show `![Before](path)` and `![After](path)` inline; include both artifact paths. +5. Do not commit screenshot artifacts unless the repository explicitly requires it. + +If a faithful comparison is impossible, use the same report with Outcome blocked, +mark UI evidence incomplete, and STOP before Step 17. Do not push, create/update +the PR/MR, claim UI completion, or offer a waiver. + +### Put simply + +Final response section; nothing follows. Be intuitive, extremely concise, precise: + +```markdown +### Put simply + +- **Why:** The problem this solves. +- **What:** What is now different for the user or system. +- **How:** The essential mechanism, without unnecessary implementation detail. +``` diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index eadffaa8f..aaa7396b0 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -843,7 +843,7 @@ branch name wherever the instructions say "the base branch" or ``. # Ship: Fully Automated Ship Workflow -You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and output the PR URL at the end. +You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and include the PR URL in the final engineering handoff. **Only stop for:** - On the base branch (abort) @@ -1246,7 +1246,11 @@ Before pushing, re-verify if code changed during Steps 4-6: 2. **Build verification:** If the project has a build step, run it. Paste output. -3. **Rationalization prevention:** +3. **UI evidence gate:** For a user-visible UI diff, produce the matched + Before/After evidence from Step 22 before push. If unavailable, STOP; do not + push or create/update the PR/MR. + +4. **Rationalization prevention:** - "Should work now" → RUN IT. - "I'm confident" → Confidence is not evidence. - "I already tested earlier" → Code changed since then. Test again. @@ -1415,3 +1419,64 @@ through `gstack-version-bump`; never hand-roll the VERSION/package.json write. - **Never push without fresh verification evidence.** If code changed after Step 5 tests, re-run before pushing. - **Step 7 generates coverage tests.** They must pass before committing. Never commit failing tests. - **The goal is: user says `/ship`, next thing they see is the review + PR URL + auto-synced docs.** + +--- + +## Step 22: Full engineering handoff + +End every ship attempt with this report, even when stopped. Use each heading; +write `Not applicable` briefly when a category does not apply. + +```markdown +## Engineering summary + +### Outcome +What shipped (version, branch, commit, PR/MR URL), or what blocked it. + +### Problem and root cause +What was wrong or missing, why, and the supporting evidence. + +### Investigation and decisions +Key findings, alternatives, decisions, and why this approach won. + +### Implementation +Changed behavior, components, data paths, APIs, migrations, infrastructure, and docs. + +### Verification +Fresh test/build/typecheck/lint/behavior/review/CI/deploy evidence; mark pass/fail/skipped. + +### Risks and operational impact +Tradeoffs, compatibility, rollout/migration, observability, rollback. + +### Remaining work +Unfinished work. Write `None` when the completion contract is fully satisfied. + +### Decision required +The user's next decision. Write `None` when no decision is required. +``` + +### UI before/after evidence + +For user-visible UI changes, include matched **Before** and **After** screenshots: + +1. Use a pre-change capture only if it faithfully represents the base; otherwise run the merge-base/base revision in an isolated worktree or use its known deployment. +2. Capture the current branch after implementation and verification. +3. Match route, application state, data, viewport, theme, and zoom. Use safe test data; never expose credentials, customer data, or private information. +4. Show `![Before](path)` and `![After](path)` inline; include both artifact paths. +5. Do not commit screenshot artifacts unless the repository explicitly requires it. + +If a faithful comparison is impossible, use the same report with Outcome blocked, +mark UI evidence incomplete, and STOP before Step 17. Do not push, create/update +the PR/MR, claim UI completion, or offer a waiver. + +### Put simply + +Final response section; nothing follows. Be intuitive, extremely concise, precise: + +```markdown +### Put simply + +- **Why:** The problem this solves. +- **What:** What is now different for the user or system. +- **How:** The essential mechanism, without unnecessary implementation detail. +``` diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index d99630c4b..a9494af1e 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -829,7 +829,7 @@ branch name wherever the instructions say "the base branch" or ``. # Ship: Fully Automated Ship Workflow -You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and output the PR URL at the end. +You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and include the PR URL in the final engineering handoff. **Only stop for:** - On the base branch (abort) @@ -2429,7 +2429,11 @@ Before pushing, re-verify if code changed during Steps 4-6: 2. **Build verification:** If the project has a build step, run it. Paste output. -3. **Rationalization prevention:** +3. **UI evidence gate:** For a user-visible UI diff, produce the matched + Before/After evidence from Step 22 before push. If unavailable, STOP; do not + push or create/update the PR/MR. + +4. **Rationalization prevention:** - "Should work now" → RUN IT. - "I'm confident" → Confidence is not evidence. - "I already tested earlier" → Code changed since then. Test again. @@ -2801,3 +2805,64 @@ through `gstack-version-bump`; never hand-roll the VERSION/package.json write. - **Never push without fresh verification evidence.** If code changed after Step 5 tests, re-run before pushing. - **Step 7 generates coverage tests.** They must pass before committing. Never commit failing tests. - **The goal is: user says `/ship`, next thing they see is the review + PR URL + auto-synced docs.** + +--- + +## Step 22: Full engineering handoff + +End every ship attempt with this report, even when stopped. Use each heading; +write `Not applicable` briefly when a category does not apply. + +```markdown +## Engineering summary + +### Outcome +What shipped (version, branch, commit, PR/MR URL), or what blocked it. + +### Problem and root cause +What was wrong or missing, why, and the supporting evidence. + +### Investigation and decisions +Key findings, alternatives, decisions, and why this approach won. + +### Implementation +Changed behavior, components, data paths, APIs, migrations, infrastructure, and docs. + +### Verification +Fresh test/build/typecheck/lint/behavior/review/CI/deploy evidence; mark pass/fail/skipped. + +### Risks and operational impact +Tradeoffs, compatibility, rollout/migration, observability, rollback. + +### Remaining work +Unfinished work. Write `None` when the completion contract is fully satisfied. + +### Decision required +The user's next decision. Write `None` when no decision is required. +``` + +### UI before/after evidence + +For user-visible UI changes, include matched **Before** and **After** screenshots: + +1. Use a pre-change capture only if it faithfully represents the base; otherwise run the merge-base/base revision in an isolated worktree or use its known deployment. +2. Capture the current branch after implementation and verification. +3. Match route, application state, data, viewport, theme, and zoom. Use safe test data; never expose credentials, customer data, or private information. +4. Show `![Before](path)` and `![After](path)` inline; include both artifact paths. +5. Do not commit screenshot artifacts unless the repository explicitly requires it. + +If a faithful comparison is impossible, use the same report with Outcome blocked, +mark UI evidence incomplete, and STOP before Step 17. Do not push, create/update +the PR/MR, claim UI completion, or offer a waiver. + +### Put simply + +Final response section; nothing follows. Be intuitive, extremely concise, precise: + +```markdown +### Put simply + +- **Why:** The problem this solves. +- **What:** What is now different for the user or system. +- **How:** The essential mechanism, without unnecessary implementation detail. +``` diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index a2acad24f..ce9b755f8 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -831,7 +831,7 @@ branch name wherever the instructions say "the base branch" or ``. # Ship: Fully Automated Ship Workflow -You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and output the PR URL at the end. +You are running the `/ship` workflow. This is a **non-interactive, fully automated** workflow. Do NOT ask for confirmation at any step. The user said `/ship` which means DO IT. Run straight through and include the PR URL in the final engineering handoff. **Only stop for:** - On the base branch (abort) @@ -2835,7 +2835,11 @@ Before pushing, re-verify if code changed during Steps 4-6: 2. **Build verification:** If the project has a build step, run it. Paste output. -3. **Rationalization prevention:** +3. **UI evidence gate:** For a user-visible UI diff, produce the matched + Before/After evidence from Step 22 before push. If unavailable, STOP; do not + push or create/update the PR/MR. + +4. **Rationalization prevention:** - "Should work now" → RUN IT. - "I'm confident" → Confidence is not evidence. - "I already tested earlier" → Code changed since then. Test again. @@ -3207,3 +3211,64 @@ through `gstack-version-bump`; never hand-roll the VERSION/package.json write. - **Never push without fresh verification evidence.** If code changed after Step 5 tests, re-run before pushing. - **Step 7 generates coverage tests.** They must pass before committing. Never commit failing tests. - **The goal is: user says `/ship`, next thing they see is the review + PR URL + auto-synced docs.** + +--- + +## Step 22: Full engineering handoff + +End every ship attempt with this report, even when stopped. Use each heading; +write `Not applicable` briefly when a category does not apply. + +```markdown +## Engineering summary + +### Outcome +What shipped (version, branch, commit, PR/MR URL), or what blocked it. + +### Problem and root cause +What was wrong or missing, why, and the supporting evidence. + +### Investigation and decisions +Key findings, alternatives, decisions, and why this approach won. + +### Implementation +Changed behavior, components, data paths, APIs, migrations, infrastructure, and docs. + +### Verification +Fresh test/build/typecheck/lint/behavior/review/CI/deploy evidence; mark pass/fail/skipped. + +### Risks and operational impact +Tradeoffs, compatibility, rollout/migration, observability, rollback. + +### Remaining work +Unfinished work. Write `None` when the completion contract is fully satisfied. + +### Decision required +The user's next decision. Write `None` when no decision is required. +``` + +### UI before/after evidence + +For user-visible UI changes, include matched **Before** and **After** screenshots: + +1. Use a pre-change capture only if it faithfully represents the base; otherwise run the merge-base/base revision in an isolated worktree or use its known deployment. +2. Capture the current branch after implementation and verification. +3. Match route, application state, data, viewport, theme, and zoom. Use safe test data; never expose credentials, customer data, or private information. +4. Show `![Before](path)` and `![After](path)` inline; include both artifact paths. +5. Do not commit screenshot artifacts unless the repository explicitly requires it. + +If a faithful comparison is impossible, use the same report with Outcome blocked, +mark UI evidence incomplete, and STOP before Step 17. Do not push, create/update +the PR/MR, claim UI completion, or offer a waiver. + +### Put simply + +Final response section; nothing follows. Be intuitive, extremely concise, precise: + +```markdown +### Put simply + +- **Why:** The problem this solves. +- **What:** What is now different for the user or system. +- **How:** The essential mechanism, without unnecessary implementation detail. +``` diff --git a/test/helpers/touchfiles.ts b/test/helpers/touchfiles.ts index 33ba9ad97..7b61dc797 100644 --- a/test/helpers/touchfiles.ts +++ b/test/helpers/touchfiles.ts @@ -772,6 +772,8 @@ export const LLM_JUDGE_TOUCHFILES: Record = { // Ship & Release 'ship/SKILL.md workflow': ['ship/SKILL.md', 'ship/SKILL.md.tmpl'], + 'ship/SKILL.md completion handoff': ['ship/SKILL.md', 'ship/SKILL.md.tmpl'], + 'ship/SKILL.md completion behavior': ['ship/SKILL.md', 'ship/SKILL.md.tmpl'], 'document-release/SKILL.md workflow': ['document-release/SKILL.md', 'document-release/SKILL.md.tmpl'], // Plan Reviews diff --git a/test/ship-completion-report.test.ts b/test/ship-completion-report.test.ts new file mode 100644 index 000000000..c52291dea --- /dev/null +++ b/test/ship-completion-report.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, test } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const root = join(import.meta.dir, '..'); + +const skillPaths = [ + 'ship/SKILL.md.tmpl', + 'ship/SKILL.md', + 'test/fixtures/golden/claude-ship-SKILL.md', + 'test/fixtures/golden/codex-ship-SKILL.md', + 'test/fixtures/golden/factory-ship-SKILL.md', +]; + +const engineeringHeadings = [ + '### Outcome', + '### Problem and root cause', + '### Investigation and decisions', + '### Implementation', + '### Verification', + '### Risks and operational impact', + '### Remaining work', + '### Decision required', +]; + +describe('ship completion report', () => { + for (const relativePath of skillPaths) { + test(`${relativePath} requires the complete two-layer handoff`, () => { + const skill = readFileSync(join(root, relativePath), 'utf8'); + const handoff = skill.indexOf('## Step 22: Full engineering handoff'); + const engineeringSummary = skill.indexOf('## Engineering summary', handoff); + const uiEvidence = skill.indexOf('### UI before/after evidence', handoff); + const putSimply = skill.lastIndexOf('### Put simply'); + const uiEvidenceGate = skill.indexOf('3. **UI evidence gate:**'); + const pushStep = skill.indexOf('## Step 17: Push'); + + expect(handoff).toBeGreaterThan(-1); + expect(engineeringSummary).toBeGreaterThan(handoff); + expect(skill).toContain('Use each heading'); + + let previousHeading = engineeringSummary; + for (const heading of engineeringHeadings) { + const headingIndex = skill.indexOf(heading, previousHeading); + expect(headingIndex).toBeGreaterThan(previousHeading); + previousHeading = headingIndex; + } + + expect(uiEvidence).toBeGreaterThan(previousHeading); + expect(skill).toContain('Write `None` when the completion contract is fully satisfied.'); + expect(skill).toContain('Write `None` when no decision is required.'); + expect(skill).toContain('matched **Before** and **After** screenshots'); + expect(skill).toContain('merge-base/base revision in an isolated worktree'); + expect(skill).toContain('current branch after implementation and verification'); + expect(skill).toContain('route, application state, data, viewport, theme, and zoom'); + expect(skill).toContain('Use safe test data; never expose credentials, customer data, or private information.'); + expect(skill).toContain('`![Before](path)` and `![After](path)` inline'); + expect(skill).toContain('include both artifact paths'); + expect(skill).toContain('Do not commit screenshot artifacts unless the repository explicitly requires it.'); + expect(skill).toContain('claim UI completion'); + expect(uiEvidenceGate).toBeGreaterThan(-1); + expect(pushStep).toBeGreaterThan(uiEvidenceGate); + expect(skill).toMatch(/do not\s+push or create\/update the PR\/MR/); + expect(skill).toContain('STOP before Step 17'); + expect(skill).toContain('or offer a waiver'); + expect(putSimply).toBeGreaterThan(uiEvidence); + expect(skill.slice(putSimply)).toContain('- **Why:**'); + expect(skill.slice(putSimply)).toContain('- **What:**'); + expect(skill.slice(putSimply)).toContain('- **How:**'); + expect(skill.slice(handoff).match(/^### .+$/gm)?.at(-1)).toBe('### Put simply'); + expect(skill.slice(handoff).match(/^## Step /gm)?.length).toBe(1); + expect(skill).not.toContain('output the PR URL at the end'); + }); + } +}); diff --git a/test/skill-coverage-matrix.ts b/test/skill-coverage-matrix.ts index 7359afbce..0291597b9 100644 --- a/test/skill-coverage-matrix.ts +++ b/test/skill-coverage-matrix.ts @@ -39,7 +39,12 @@ export interface SkillCoverage { export const SKILL_COVERAGE: Record = { // ─── Core loop ────────────────────────────────────────────── ship: { - gate: ['test/skill-e2e-ship-idempotency.test.ts', 'test/skill-coverage-floor.test.ts'], + gate: [ + 'test/skill-e2e-ship-idempotency.test.ts', + 'test/ship-completion-report.test.ts', + 'test/skill-llm-eval.test.ts', + 'test/skill-coverage-floor.test.ts', + ], periodic: ['test/skill-e2e-workflow.test.ts'], }, review: { diff --git a/test/skill-llm-eval.test.ts b/test/skill-llm-eval.test.ts index 17b1adf0d..8914727b2 100644 --- a/test/skill-llm-eval.test.ts +++ b/test/skill-llm-eval.test.ts @@ -537,6 +537,7 @@ async function runWorkflowJudge(opts: { judgeContext: string; judgeGoal: string; thresholds?: { clarity: number; completeness: number; actionability: number }; + includeCarvedSections?: boolean; }) { const t0 = Date.now(); const defaults = { clarity: 4, completeness: 3, actionability: 4 }; @@ -549,7 +550,7 @@ async function runWorkflowJudge(opts: { let content = fs.readFileSync(path.join(ROOT, opts.skillPath), 'utf-8'); const secDir = path.join(ROOT, path.dirname(opts.skillPath), 'sections'); const sectionBodies: string[] = []; - if (fs.existsSync(secDir)) { + if (opts.includeCarvedSections !== false && fs.existsSync(secDir)) { for (const f of fs.readdirSync(secDir).sort()) { if (f.endsWith('.md') && !f.endsWith('.md.tmpl')) { const body = fs.readFileSync(path.join(secDir, f), 'utf-8'); @@ -617,7 +618,12 @@ ${section}`); } // Block 1: Ship & Release skills -describeIfSelected('Ship & Release skill evals', ['ship/SKILL.md workflow', 'document-release/SKILL.md workflow'], () => { +describeIfSelected('Ship & Release skill evals', [ + 'ship/SKILL.md workflow', + 'ship/SKILL.md completion handoff', + 'ship/SKILL.md completion behavior', + 'document-release/SKILL.md workflow', +], () => { testIfSelected('ship/SKILL.md workflow', async () => { await runWorkflowJudge({ testName: 'ship/SKILL.md workflow', @@ -630,6 +636,96 @@ describeIfSelected('Ship & Release skill evals', ['ship/SKILL.md workflow', 'doc }); }, 30_000); + testIfSelected('ship/SKILL.md completion handoff', async () => { + await runWorkflowJudge({ + testName: 'ship/SKILL.md completion handoff', + suite: 'Ship & Release skill evals', + skillPath: 'ship/SKILL.md', + startMarker: '## Step 22: Full engineering handoff', + endMarker: null, + judgeContext: 'the final user-facing handoff contract for a ship workflow', + judgeGoal: + 'how to report the complete engineering outcome and evidence, require matched before/after screenshots only for UI changes, disclose incomplete UI evidence, and finish with an extremely concise Why/What/How explanation', + thresholds: { clarity: 4, completeness: 4, actionability: 4 }, + includeCarvedSections: false, + }); + }, 30_000); + + testIfSelected('ship/SKILL.md completion behavior', async () => { + const t0 = Date.now(); + const skill = fs.readFileSync(path.join(ROOT, 'ship', 'SKILL.md'), 'utf-8'); + const handoffStart = skill.indexOf('## Step 22: Full engineering handoff'); + expect(handoffStart).toBeGreaterThan(-1); + const handoff = skill.slice(handoffStart); + const client = new Anthropic(); + const response = await client.messages.create({ + model: 'claude-sonnet-4-6', + max_tokens: 4096, + messages: [{ + role: 'user', + content: `Follow the ship handoff instructions below and produce three final user-facing responses. +Return ONLY JSON with string fields "non_ui", "ui_complete", and "ui_incomplete". Do not use a JSON code fence. + +SCENARIO non_ui: v1.2.3.4 shipped on feat/report at commit abc123; PR https://example.test/pr/1; root cause was a missing output contract; implementation added it; 505 tests passed; no UI changed; no remaining work or decision. + +SCENARIO ui_complete: v1.2.3.4 shipped on feat/ui at commit def456; PR https://example.test/pr/2; a button alignment bug was fixed; 505 tests passed; matched evidence is Before /tmp/before.png and After /tmp/after.png on the same route, state, data, viewport, theme, and zoom; no remaining work or decision. + +SCENARIO ui_incomplete: a UI changed, but the base revision cannot be rendered, so faithful Before evidence is unavailable; the workflow reached the UI evidence gate before push and must report the blocker without claiming completion or inventing a PR URL. + +INSTRUCTIONS: +${handoff}`, + }], + }); + + const text = response.content[0].type === 'text' ? response.content[0].text : ''; + const jsonMatch = text.match(/\{[\s\S]*\}/); + if (!jsonMatch) throw new Error(`Completion behavior eval returned non-JSON: ${text.slice(0, 300)}`); + const outputs = JSON.parse(jsonMatch[0]) as Record; + const headings = [ + '## Engineering summary', + '### Outcome', + '### Problem and root cause', + '### Investigation and decisions', + '### Implementation', + '### Verification', + '### Risks and operational impact', + '### Remaining work', + '### Decision required', + '### Put simply', + ]; + + for (const key of ['non_ui', 'ui_complete', 'ui_incomplete']) { + const output = outputs[key]; + expect(typeof output).toBe('string'); + for (const heading of headings) expect(output).toContain(heading); + expect(output.match(/^### .+$/gm)?.at(-1)).toBe('### Put simply'); + expect(output.slice(output.lastIndexOf('### Put simply'))).toContain('- **Why:**'); + expect(output.slice(output.lastIndexOf('### Put simply'))).toContain('- **What:**'); + expect(output.slice(output.lastIndexOf('### Put simply'))).toContain('- **How:**'); + } + + expect(outputs.non_ui).not.toMatch(/!\[[^\]]*(Before|After)/i); + expect(outputs.ui_complete).toContain('/tmp/before.png'); + expect(outputs.ui_complete).toContain('/tmp/after.png'); + expect(outputs.ui_complete).toMatch(/Before[\s\S]*After/i); + expect(outputs.ui_complete).toMatch(/!\[Before\]\([^)]*before\.png\)/i); + expect(outputs.ui_complete).toMatch(/!\[After\]\([^)]*after\.png\)/i); + expect(outputs.ui_incomplete).toMatch(/incomplete/i); + expect(outputs.ui_incomplete).toMatch(/stop|block|cannot claim|not complete/i); + expect(outputs.ui_incomplete).not.toContain('https://example.test/pr/'); + expect(outputs.ui_incomplete).not.toMatch(/waiv/i); + + evalCollector?.addTest({ + name: 'ship/SKILL.md completion behavior', + suite: 'Ship & Release skill evals', + tier: 'llm-judge', + passed: true, + duration_ms: Date.now() - t0, + cost_usd: 0.08, + judge_reasoning: 'Generated non-UI, complete-UI, and blocked-UI handoffs satisfied the deterministic output contract.', + }); + }, 60_000); + testIfSelected('document-release/SKILL.md workflow', async () => { await runWorkflowJudge({ testName: 'document-release/SKILL.md workflow',