From b162db7ecd14753578db9042795d30a5a71335be Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 05:46:41 +0000 Subject: [PATCH 1/3] Cut v0.1.0: honest pins, hardened installer, mechanical policy, worked example Release readiness - Collapse CHANGELOG into a single [0.1.0] section; add a tag-triggered Release workflow that publishes the GitHub Release from that section - Pin vendor/spec-kit to the v1.0.5 tag; README vendored-submodules table now names the real pins (superpowers v6.3.0, spec-kit v1.0.5) - CI fails unless each submodule pointer sits on an upstream tag named in the README, and unless every upstream superpowers skill is KEEP in install.sh or a DENY row in the README; Dependabot head-commit bumps fail by design install.sh - Exit non-zero when any link could not be placed (non-damascus path in the way, or a target that no longer exists); link() refuses missing targets - Pure-bash relative-path computation replaces realpath --relative-to, so stock macOS no longer gets absolute symlinks that break on a fresh clone - Sourcing guard so CI reads the name arrays from the script instead of duplicating them; smoke test covers relative links, DENY skills, and the skipped-link exit code Skill contracts - Move using-superpowers and subagent-driven-development to DENY: both route into already-DENY skills (brainstorming, requesting-code-review) - anvil: portable DAMASCUS path lookup (no readlink -f) - quench: dispatch table states the shipped agents' Python/FastAPI shape and how another stack binds to the same contract - smithy: effort frontmatter; every state-machine row decidable from disk (quench-log green per task, FINISH line in smithy-log.md) - Trim tdd-test-generator, fastapi-implementer and playwright-e2e-tester to their contracts plus one example per concept (2035 -> 624 lines) Docs - examples/cart-discounts: one feature's full artifact trail through all five gates, checked for completeness in CI; README "which stage do I start at?" table; requirements name the agents' stack; bug template lists hone; CLAUDE.md bump policy and release ritual updated Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HhCxRx3NtQvWL2nYu2GuYD --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- .github/dependabot.yml | 8 +- .github/workflows/ci.yml | 63 +- .github/workflows/release.yml | 36 + CHANGELOG.md | 50 +- CLAUDE.md | 11 +- README.md | 31 +- agents/fastapi-implementer.md | 719 +++--------------- agents/playwright-e2e-tester.md | 595 +++------------ agents/tdd-test-generator.md | 607 ++------------- docs/production-readiness.md | 30 +- examples/README.md | 65 ++ .../.prd/001_cart-discount-codes.md | 85 +++ .../001-cart-discount-codes/code-review.md | 38 + .../specs/001-cart-discount-codes/plan.md | 74 ++ .../001-cart-discount-codes/quench-log.md | 71 ++ .../specs/001-cart-discount-codes/review.md | 28 + .../001-cart-discount-codes/smithy-log.md | 15 + .../specs/001-cart-discount-codes/spec.md | 99 +++ .../specs/001-cart-discount-codes/tasks.md | 57 ++ install.sh | 78 +- skills/anvil/SKILL.md | 4 +- skills/quench/SKILL.md | 8 +- skills/smithy/SKILL.md | 17 +- tests/smoke.sh | 22 +- vendor/spec-kit | 2 +- 26 files changed, 1046 insertions(+), 1769 deletions(-) create mode 100644 .github/workflows/release.yml create mode 100644 examples/README.md create mode 100644 examples/cart-discounts/.prd/001_cart-discount-codes.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/code-review.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/plan.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/quench-log.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/review.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/smithy-log.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/spec.md create mode 100644 examples/cart-discounts/specs/001-cart-discount-codes/tasks.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 74d914d..5d8f3b2 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -24,5 +24,5 @@ labels: bug ## For skill/pipeline bugs - diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 6976d72..5fc7ec0 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,8 +1,10 @@ version: 2 updates: - # vendor/superpowers and vendor/spec-kit — the CI smoke test validates each - # bump (KEEP-class linking fails if upstream renames or removes a skill). - # Policy (see CLAUDE.md): only merge bumps that land on an upstream tag. + # vendor/superpowers and vendor/spec-kit. Dependabot targets head commits, and CI + # requires each pointer to sit on an upstream tag named in the README, so these PRs + # fail by design — treat them as "a new upstream release may exist", close them, and + # bump to the tag by hand (policy in CLAUDE.md). The smoke test and the KEEP/DENY + # classification check then validate the deliberate bump. - package-ecosystem: gitsubmodule directory: / schedule: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b0d8a4..6f5a841 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + submodules: recursive - name: bash syntax check run: bash -n install.sh tests/smoke.sh @@ -29,14 +31,39 @@ jobs: [ -f "$l/SKILL.md" ] || { echo "::error::$l does not resolve to a skill"; exit 1; } done - - name: every stage skill and agent named in install.sh exists on disk + # The name lists come from install.sh itself (sourced, not executed), so a rename + # on disk or in the script cannot pass CI unless both sides agree. + - name: every skill, alias, agent and KEEP skill named in install.sh exists on disk run: | - for s in forge anvil temper quench hone smithy; do + # shellcheck disable=SC1091 + source ./install.sh + for s in "${STAGE_SKILLS[@]}"; do [ -f "skills/$s/SKILL.md" ] || { echo "::error::skills/$s/SKILL.md missing"; exit 1; } done - for a in bdd-scenario-writer tdd-test-generator playwright-e2e-tester fastapi-implementer labcoat; do + for pair in "${ALIASES[@]}"; do + alias="${pair%%:*}" stage="${pair#*:}" + [ -L "skills/$alias" ] || { echo "::error::skills/$alias is not a symlink"; exit 1; } + [ "$(readlink "skills/$alias")" = "$stage" ] || { echo "::error::skills/$alias should point at $stage"; exit 1; } + done + for a in "${AGENTS[@]}"; do [ -f "agents/$a.md" ] || { echo "::error::agents/$a.md missing"; exit 1; } done + for s in "${SUPERPOWERS_KEEP[@]}"; do + [ -f "vendor/superpowers/skills/$s/SKILL.md" ] || { echo "::error::KEEP skill $s missing upstream — update SUPERPOWERS_KEEP"; exit 1; } + done + # every stage dir on disk is either a stage or an alias (no orphan skills) + for d in skills/*; do + name="$(basename "$d")" + printf '%s\n' "${STAGE_SKILLS[@]}" "${ALIASES[@]%%:*}" | grep -Fxq "$name" \ + || { echo "::error::$d is not listed in install.sh"; exit 1; } + done + # every upstream skill has a policy: KEEP (linked) or DENY (named in README) + for d in vendor/superpowers/skills/*; do + name="$(basename "$d")" + printf '%s\n' "${SUPERPOWERS_KEEP[@]}" | grep -Fxq "$name" && continue + grep -Eq "^\| \`$name\` \| DENY \|" README.md \ + || { echo "::error::upstream skill '$name' is neither KEEP in install.sh nor DENY in README.md — classify it"; exit 1; } + done - name: frontmatter lint (name + description) run: | @@ -46,6 +73,36 @@ jobs: || { echo "::error::$f frontmatter is missing name or description"; exit 1; } done + # Policy (CLAUDE.md): vendor pointers sit on upstream tags, and the README names + # the tag. Dependabot bumps to head commits fail here by design — re-pin to a tag. + - name: vendor submodules are pinned to upstream tags named in the README + run: | + for sub in vendor/superpowers vendor/spec-kit; do + git -C "$sub" fetch --quiet --tags origin + tag="$(git -C "$sub" describe --tags --exact-match HEAD 2>/dev/null)" \ + || { echo "::error::$sub is at $(git -C "$sub" rev-parse --short HEAD), which is not an upstream tag"; exit 1; } + grep -Fq "| $tag |" README.md \ + || { echo "::error::README.md vendored-submodules table does not list $sub at $tag"; exit 1; } + echo "$sub @ $tag" + done + + - name: example artifact trail covers every smithy state + run: | + ex=examples/cart-discounts + for f in .prd/001_cart-discount-codes.md \ + specs/001-cart-discount-codes/spec.md specs/001-cart-discount-codes/plan.md \ + specs/001-cart-discount-codes/tasks.md specs/001-cart-discount-codes/review.md \ + specs/001-cart-discount-codes/quench-log.md specs/001-cart-discount-codes/code-review.md \ + specs/001-cart-discount-codes/smithy-log.md; do + [ -f "$ex/$f" ] || { echo "::error::$ex/$f missing"; exit 1; } + done + spec="$ex/specs/001-cart-discount-codes" + grep -q 'A++' "$spec/review.md" || { echo "::error::review.md never reaches A++"; exit 1; } + grep -q 'A++' "$spec/code-review.md" || { echo "::error::code-review.md never reaches A++"; exit 1; } + grep -q '^FINISH:\|FINISH:' "$spec/smithy-log.md" || { echo "::error::smithy-log.md has no FINISH entry"; exit 1; } + grep -q '^- \[ \]' "$spec/tasks.md" && { echo "::error::tasks.md has unchecked tasks at pipeline end"; exit 1; } + echo "example trail complete" + smoke: name: install/uninstall smoke test strategy: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..d92379a --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,36 @@ +name: Release + +# Pushing a vX.Y.Z tag publishes a GitHub Release whose body is that version's +# CHANGELOG.md section, verbatim (the ritual in CLAUDE.md). The tag must match a +# `## [X.Y.Z] - YYYY-MM-DD` heading; a tag with no changelog section fails here +# instead of shipping empty notes. +on: + push: + tags: ['v[0-9]+.[0-9]+.[0-9]+'] + +permissions: + contents: write + +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: extract this version's changelog section + run: | + version="${GITHUB_REF_NAME#v}" + awk -v v="$version" ' + /^## \[/ { if (found) exit; if (index($0, "## [" v "]") == 1) { found = 1; next } } + found { print } + ' CHANGELOG.md > release-notes.md + if ! grep -q '[^[:space:]]' release-notes.md; then + echo "::error::CHANGELOG.md has no '## [$version]' section — move [Unreleased] under it first (see CLAUDE.md)" + exit 1 + fi + cat release-notes.md + + - name: create the GitHub Release + env: + GH_TOKEN: ${{ github.token }} + run: gh release create "$GITHUB_REF_NAME" --title "$GITHUB_REF_NAME" --notes-file release-notes.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 9125aae..6675e41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,42 +4,20 @@ All notable changes to this project are documented here, following [Common Chang ## [Unreleased] -### Added - -- Add `hone` (alias `code-review-loop`) — stage 5: local adversarial review of the implementation diff (spec-conformance / security / simplicity lenses with mandatory active procedures, ≤400-changed-line review units, A++ = two consecutive zero-blocking rounds, max 3 rounds), between quench and merge -- Add quench hardening gates: diff-scoped mutation testing (line coverage reported, never gated), host-repo static/type/security checks, an FR ↔ test traceability sweep (`@pytest.mark.fr("FR-NNN")` / `@FR-NNN` Gherkin tags), and a stable-green rule (new tests pass 3× in randomized order; a flake is red, never a retry) -- Add the test freeze: from amber onward a test changes only after `spec.md`/`tasks.md` change first; test agents never write implementation and `fastapi-implementer` treats test files as read-only -- Add `specs/NNN-/quench-log.md` — append-only per-task cycle log (red/amber/green timestamps, amber's failure message, diff size, gate results, waivers) -- Add property-based tests (Hypothesis) derived from PRD Safeguards and SC invariants, characterization-test-first handling for `[REFACTOR]` tasks, and sample-and-select (2–3 independent candidates judged by the frozen tests) for `[HARD]` tasks -- Add active reading procedures to temper's critics (re-derive sections, trace a data flow, draft per-FR test skeletons — findings without the artifact are discarded) and a capture-recapture overlap signal that caps near-disjoint rounds at B+ in both review loops - -- Add the five SPDD pipeline skills: `forge` (PRD authoring), `anvil` (spec/plan/tasks decomposition), `temper` (adversarial review loop), `quench` (BDD-first red-amber-green execution), `smithy` (state-machine orchestrator) -- Add invocation aliases: `prd-authoring`, `speckit-decomposition`, `adversarial-review-loop`, `bdd-tdd-execution`, `spdd-pipeline` -- Add the five quench dispatch agents: `bdd-scenario-writer`, `tdd-test-generator`, `playwright-e2e-tester`, `fastapi-implementer`, `labcoat` -- Add `install.sh` for symlinking skills and agents into a consumer repo's `.claude/` -- Vendor [obra/superpowers](https://github.com/obra/superpowers) v4.3.1 and [github/spec-kit](https://github.com/github/spec-kit) v0.10.1 as pinned submodules -- Add `docs/production-readiness.md` — phased single-founder plan (licensing, CI, releases, installer hardening, automated maintenance) -- Add MIT `LICENSE` (vendored submodules retain their upstream licenses) -- Add CI workflow: shellcheck + syntax checks, alias-symlink and frontmatter integrity, install/uninstall smoke test on Ubuntu and macOS (including stock bash 3.2), and a changelog-updated gate on PRs -- Add `tests/smoke.sh` — end-to-end installer test against a throwaway consumer repo (idempotency, ownership guarantee, pruning, verify, dry-run, uninstall) -- Add `install.sh --verify` (link health report for bug reports) and `--dry-run` (print planned actions without touching anything) -- Add stale-link pruning: install now sweeps damascus-owned links whose names are no longer shipped; uninstall removes every owned link, not just known names -- Add Dependabot config for monthly vendor-submodule and GitHub Actions bumps, and a bug-report issue template -- Add README sections: requirements, pinned-tag install, upgrading, support posture and licensing; document the release ritual and submodule bump policy in CLAUDE.md +## [0.1.0] - 2026-09-09 -### Changed +First release. Everything below is new; there is no earlier version to change or fix against. -- Redraw `docs/assets/hero.png` with a real alpha channel: the previous file carried an opaque cream background remnant above the forge and 250 speckle regions of keying debris, and it showed four stages with `forge anvil temper quench` painted into its pixels. The new plate is five objects and no lettering — 5 regions for 5 objects, 2.06% partial alpha, no border bleed — with the stage names moved to a README caption -- Make `install.sh` portable to bash 3.2 / stock macOS (replace `declare -A` and `readlink -f` with portable equivalents) -- Replace the external-API review gate in `temper` with a fully local adversarial panel (3 critic lenses + judge; A++ = two consecutive zero-blocking rounds, max 5 rounds) -- Change forge's Norms section to require machine-checkable form (the tool + config that enforces each norm, installed as gates by quench) and mark each Safeguard as the source of a property test or runtime assertion -- Change anvil's task format: optional `[UX]`/`[REFACTOR]`/`[HARD]` tags and a ≤400-changed-line sizing budget per task (oversized tasks come back as anvil feedback via the quench log) -- Change smithy's state machine to drive hone after quench and halt on hone's 3-round cap -- Add a smithy finish step at the end of the cycle: after hone reaches A++, update `README.md` and `CHANGELOG.md` for the feature, then run the `atlas` skill scoped to the completed feature if it is available (silent no-op otherwise, like the kanban sync) -- Move `superpowers:requesting-code-review` from KEEP to DENY — `hone` replaces it; `receiving-code-review` stays KEEP for applying findings -- Replace the 95% line-coverage gate in `tdd-test-generator`'s pytest config with mutation-testing guidance and a registered `fr` marker for requirement traceability - -### Fixed +### Added -- Make the README hero image background transparent (renders cleanly on dark and light themes) -- Fix three quench agents (`tdd-test-generator`, `labcoat`, `fastapi-implementer`) that defined Amber as "minimal implementation", contradicting quench's fails-for-the-right-reason contract +- Add the five SPDD pipeline stage skills and their orchestrator: `forge` (PRD authoring on Fowler's REASONS Canvas; Norms in machine-checkable form, each Safeguard the source of a property test), `anvil` (spec/plan/tasks decomposition via spec-kit slash commands or the vendored templates; `[UX]`/`[REFACTOR]`/`[HARD]` task tags and a ≤400-changed-line task budget), `temper` (local adversarial review of the spec triplet: three critics with mandatory active procedures, a judge with a capture-recapture overlap signal, A++ = two consecutive zero-blocking rounds, max 5 rounds), `quench` (BDD-first red-amber-green execution with the amber test freeze, diff-scoped mutation gate, static gates, FR ↔ test traceability, stable-green 3× rule, property tests, characterization-first `[REFACTOR]`, sample-and-select `[HARD]`, append-only `quench-log.md`), `hone` (the same adversarial loop pointed at the implementation diff: conformance / security / simplicity lenses, ≤400-changed-line review units, every fix shown to fail without itself, max 3 rounds), and `smithy` (stateless orchestrator whose every state is decidable from disk artifacts, halting at every gate; finish step updates README/CHANGELOG, runs Atlas if installed, and logs `FINISH:` to `smithy-log.md`) +- Add invocation aliases: `prd-authoring`, `speckit-decomposition`, `adversarial-review-loop`, `bdd-tdd-execution`, `code-review-loop`, `spdd-pipeline` +- Add the five quench dispatch agents — `bdd-scenario-writer`, `tdd-test-generator`, `playwright-e2e-tester`, `fastapi-implementer`, `labcoat` — each a concise contract (amber = fails for the right reason, tests frozen from amber, test agents never implement, the implementer never edits tests, coverage reported never gated) with one short example per concept; quench's dispatch table states how to bind another stack to the same contract +- Add `install.sh` for symlinking skills and agents into a consumer repo's `.claude/`: idempotent, bash 3.2 / stock macOS compatible, relative links only (pure-bash relative-path computation, no `realpath --relative-to`), touches only links that resolve into the damascus checkout, prunes damascus-owned links whose names are no longer shipped, refuses missing targets, and exits non-zero when any link could not be placed; `--verify` reports link health for bug reports, `--dry-run` prints planned actions, `--uninstall` removes every owned link +- Add `tests/smoke.sh` — end-to-end installer test against a throwaway consumer repo: idempotency, the ownership guarantee, relative links, DENY skills never linked, pruning, verify, dry-run, a non-damascus path in the way fails the run, clean uninstall +- Add CI: shellcheck + syntax checks; repo integrity sourced from `install.sh`'s own name arrays (stage skills, aliases, agents, KEEP skills all exist; every upstream superpowers skill is classified KEEP or DENY); frontmatter lint; vendor submodules must sit on an upstream tag named in the README; the example artifact trail must be complete; the smoke test on Ubuntu and macOS (including stock bash 3.2); a changelog-updated gate on PRs +- Add a tag-triggered `Release` workflow that publishes the GitHub Release from the matching `CHANGELOG.md` section +- Vendor [obra/superpowers](https://github.com/obra/superpowers) v6.3.0 and [github/spec-kit](https://github.com/github/spec-kit) v1.0.5 as pinned submodules, with the DENY / KEEP / CONDITIONAL policy: `brainstorming`, `writing-plans`, `executing-plans`, `requesting-code-review`, `using-superpowers`, `subagent-driven-development` are DENY (a stage replaces them, or they route into one that does); `test-driven-development` is linked but overridden by red-amber-green; the remaining seven are linked as-is +- Add `examples/cart-discounts/` — one feature's complete artifact trail through all five gates (`.prd/`, spec triplet, `review.md`, `quench-log.md`, `code-review.md`, `smithy-log.md`) with a gate-by-gate reading guide and a "which stage do I start at?" note, also surfaced in the README +- Add Dependabot config for monthly vendor-submodule and GitHub Actions bumps, and a bug-report issue template +- Add README (pipeline table, review-loop and finish-step descriptions, requirements incl. the shipped agents' Python/FastAPI shape, pinned-tag install, upgrading, vendored-submodule pins, policy table, support posture, credits), `docs/production-readiness.md` (the phased single-founder plan), MIT `LICENSE` (vendored submodules retain their upstream licenses), and the release ritual and bump policy in CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md index 4970ae8..cb2303d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,10 +8,11 @@ Damascus packages the **SPDD pipeline** (forge → anvil → temper → quench ## Working here -- The six skills under `skills/` (five stages + smithy) are the product. Keep them concise; every behavioral contract lives in the SKILL.md body, not in external docs. +- The six skills under `skills/` (five stages + smithy) are the product. Keep them concise; every behavioral contract lives in the SKILL.md body, not in external docs. The same goes for `agents/*.md`: a contract plus one short example per concept, no code catalogs — every line is context cost on each dispatch. - `skills/` entries are relative symlinks to stage dirs — preserve them. -- `vendor/superpowers` and `vendor/spec-kit` are **pinned submodules**. Bump deliberately (checkout a tag, commit the pointer); never edit vendor content. Dependabot opens monthly bump PRs — only merge bumps that land on an upstream **tag**, and when superpowers adds or renames skills, re-check the DENY/KEEP/CONDITIONAL table in the README (a new upstream skill overlapping a stage becomes DENY; a renamed KEEP skill needs the `SUPERPOWERS_KEEP` array in `install.sh` updated — CI's smoke test fails until it is). -- `install.sh` must stay idempotent, bash 3.2 compatible (no `declare -A`, no GNU-only flags without a fallback), and only ever touch symlinks that resolve into this checkout. `tests/smoke.sh` encodes these guarantees against a throwaway repo — run it locally before pushing; CI runs it on Ubuntu and macOS (including stock bash 3.2). +- `examples/cart-discounts/` is a worked artifact trail through all five gates. When a skill changes an artifact format (PRD template, task line, log entry), update the example to match — CI checks the trail is complete, and readers copy it. +- `vendor/superpowers` and `vendor/spec-kit` are **pinned submodules**. Bump deliberately (checkout a tag, update the README vendored-submodules table, note it in the changelog, commit the pointer); never edit vendor content. CI fails unless each pointer sits exactly on an upstream tag that the README names. Dependabot opens monthly bump PRs that target head commits — treat them as a signal that a new upstream tag exists, not as mergeable: close them and bump to the tag by hand. When superpowers adds or renames skills, re-check the DENY/KEEP/CONDITIONAL table in the README (a new upstream skill overlapping a stage, or routing into a DENY skill, becomes DENY; a renamed KEEP skill needs the `SUPERPOWERS_KEEP` array in `install.sh` updated) — CI fails until every upstream skill is either KEEP in `install.sh` or a DENY row in the README. +- `install.sh` must stay idempotent, bash 3.2 compatible (no `declare -A`, no `readlink -f`, no `realpath --relative-to`, no GNU-only flags without a fallback), only ever touch symlinks that resolve into this checkout, create only relative links, and exit non-zero when a link could not be placed. `tests/smoke.sh` encodes these guarantees against a throwaway repo — run it locally before pushing; CI runs it on Ubuntu and macOS (including stock bash 3.2). CI also `source`s `install.sh` to read its name arrays, so keep the sourcing guard at the bottom of the script. - Every PR updates `CHANGELOG.md` under `[Unreleased]` (Common Changelog categories). CI enforces this. - Branch → PR → squash merge. Never push to `master` directly. - The temper and hone stages are **local-only by design** — do not reintroduce external-API review dependencies. @@ -20,8 +21,8 @@ Damascus packages the **SPDD pipeline** (forge → anvil → temper → quench Semver. Breaking changes to a skill contract or to `install.sh` behavior bump the major version (the minor version while pre-1.0). To cut a release from `master`: -1. Move the `[Unreleased]` section of `CHANGELOG.md` under a `## [X.Y.Z] - YYYY-MM-DD` heading (via a normal PR). +1. Move the `[Unreleased]` section of `CHANGELOG.md` under a `## [X.Y.Z] - YYYY-MM-DD` heading (via a normal PR). Leave an empty `## [Unreleased]` above it. 2. `git tag vX.Y.Z && git push origin vX.Y.Z` -3. Create a GitHub Release from the tag; the body is that changelog section verbatim. +3. The `Release` workflow creates the GitHub Release with that changelog section as its body. It fails if the tag has no matching changelog heading — fix the changelog and re-tag rather than editing the release by hand. Consumers pin tags (see the README install/upgrading sections) — never point them at `master`. diff --git a/README.md b/README.md index c2c4875..c471b1c 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,21 @@ flowchart LR Every stage halts at its gate for user signoff. State lives in the disk artifacts, so `smithy` can resume any feature from any point. For one-line typos and hotfixes: skip the pipeline and just fix it — SPDD is for non-trivial work. +### Which stage do I start at? + +Smithy decides from what is on disk, and you can too: + +| You have | Start with | +|----------|------------| +| a raw idea, nothing written | `forge` | +| a PRD (all 7 REASONS sections filled) | `anvil` | +| `spec.md` + `plan.md` + `tasks.md` | `temper` | +| `review.md` ending in A++ | `quench` | +| every task checked and green in `quench-log.md` | `hone` | +| any of the above, unsure which | `smithy --resume` | + +[`examples/`](examples/) holds one feature's complete artifact trail through all five gates — the `.prd/` file, the spec triplet, both review logs, the quench log and the smithy log — so you can see what each state looks like before you produce it. + ### Temper & hone: the adversarial review loops Both loops need **no external API**. Each round, three critic subagents with distinct lenses try to *refute* the artifact; a judge dedupes findings, computes an overlap signal (near-disjoint findings = more defects remain → rating capped), and assigns a rating. Blocking findings are applied, the round is logged, and the loop repeats. **A++ requires two consecutive rounds with zero blocking findings.** @@ -48,6 +63,7 @@ Once `hone` reaches A++, `smithy` runs a final finish step before the merge hand - git ≥ 2.13 (submodules) - bash 3.2+ (`install.sh` runs on stock macOS bash) - a filesystem with symlink support +- for `quench`'s shipped agents: a **Python / pytest / FastAPI** host, plus Playwright for `[UX]` tasks. The pipeline and its gates are stack-agnostic; on another stack quench dispatches general subagents under the same red-amber-green contract (see the dispatch table in `skills/quench/SKILL.md`) ## Install @@ -67,7 +83,7 @@ This symlinks into your `.claude/`: - 5 quench agents (`bdd-scenario-writer`, `tdd-test-generator`, `playwright-e2e-tester`, `fastapi-implementer`, `labcoat`) → `.claude/agents/` - the KEEP-class [obra/superpowers](https://github.com/obra/superpowers) skills (see policy below) → `.claude/skills/` -Re-run any time to refresh; the install prunes damascus-owned links whose names are no longer shipped. Other modes: +Every link is relative, so a committed `.claude/` keeps working on every clone. Re-run any time to refresh; the install prunes damascus-owned links whose names are no longer shipped, and exits non-zero if any link could not be placed (a non-damascus file in the way, or an upstream skill that vanished). Other modes: ```bash ./vendor/damascus/install.sh --verify # link health report; exit 1 if repair is needed @@ -93,12 +109,14 @@ Breaking changes to skill contracts or `install.sh` behavior are called out in [ | Submodule | Pin | Role | |-----------|-----|------| -| [obra/superpowers](https://github.com/obra/superpowers) | v4.3.1 | process-discipline skills; KEEP-class linked at install | -| [github/spec-kit](https://github.com/github/spec-kit) | v0.10.1 | `anvil`'s fallback templates (`templates/{spec,plan,tasks}-template.md`) when `/speckit.*` slash commands aren't registered | +| [obra/superpowers](https://github.com/obra/superpowers) | v6.3.0 | process-discipline skills; KEEP-class linked at install | +| [github/spec-kit](https://github.com/github/spec-kit) | v1.0.5 | `anvil`'s fallback templates (`templates/{spec,plan,tasks}-template.md`) when `/speckit.*` slash commands aren't registered | + +CI checks that each pointer sits exactly on an upstream tag and that this table names it, so a bump is always deliberate: check out the tag, update the row, note it in the changelog. ## Superpowers Policy (DENY / KEEP / CONDITIONAL) -The pipeline stages are the canonical entrypoints. Four upstream skills overlap them and are **DENY** — not linked at install, and each stage skill carries redirect language: +The pipeline stages are the canonical entrypoints. Six upstream skills are **DENY** — not linked at install — because they overlap a stage or route back into a skill that does; each stage skill carries redirect language. CI fails when an upstream skill has no row here and is not in `install.sh`'s KEEP list, so a new upstream skill must be classified before a bump merges. | Upstream skill | Policy | Use instead | |----------------|--------|-------------| @@ -106,8 +124,10 @@ The pipeline stages are the canonical entrypoints. Four upstream skills overlap | `writing-plans` | DENY | `anvil` — 3-file spec-kit-shaped artifact set | | `executing-plans` | DENY | `quench` (or `smithy` cross-stage) — BDD-first, red-amber-green | | `requesting-code-review` | DENY | `hone` — three-lens adversarial diff review with a logged A++ trail | +| `using-superpowers` | DENY | `smithy` — the upstream router sends "let's build X" to `brainstorming`; smithy is this pipeline's entrypoint | +| `subagent-driven-development` | DENY | `quench` — an execution loop that dispatches `requesting-code-review`'s reviewer; quench + hone replace it | | `test-driven-development` | CONDITIONAL | linked; quench **overrides** its red-green cycle with red-amber-green | -| remaining 9 skills | KEEP | linked as-is (`systematic-debugging`, `receiving-code-review`, `finishing-a-development-branch`, …) | +| remaining 7 skills | KEEP | linked as-is (`systematic-debugging`, `dispatching-parallel-agents`, `verification-before-completion`, `receiving-code-review`, `finishing-a-development-branch`, `using-git-worktrees`, `writing-skills`) | **Red-amber-green:** standard TDD goes red → green. Quench inserts **amber** — the test must fail *for the right reason* (the assertion you care about, not an import error) before any implementation is written. Amber is the moment you trust the test — and the moment it **freezes**: from amber on, a test changes only after the spec changes first, and the implementing agent never edits tests at all. At green, quench runs **hardening gates**: mutation testing scoped to the diff (a surviving mutant = a weak test; line-coverage % is reported, never gated), the host repo's static/type/security checks, an FR ↔ test traceability sweep (every requirement has a verifying test), and a stable-green rule (new tests pass 3× in randomized order; a flake is red, not a retry). Every red/amber/green transition, gate result, and waiver lands in `specs/NNN-/quench-log.md`. @@ -117,6 +137,7 @@ The pipeline stages are the canonical entrypoints. Four upstream skills overlap skills/{forge,anvil,temper,quench,hone,smithy}/SKILL.md the six skills (5 stages + orchestrator) skills/ -> invocation aliases agents/*.md quench's dispatch agents +examples/cart-discounts/ one feature's full artifact trail, gate by gate vendor/superpowers pinned submodule vendor/spec-kit pinned submodule install.sh consumer-side symlinker diff --git a/agents/fastapi-implementer.md b/agents/fastapi-implementer.md index d7be359..b4253fe 100644 --- a/agents/fastapi-implementer.md +++ b/agents/fastapi-implementer.md @@ -19,603 +19,154 @@ Implement FastAPI applications that: - Use Pydantic for validation - Implement proper auth/authz when needed -## FastAPI Best Practices +## Test Freeze (non-negotiable) -### Project Structure -``` -src/myapp/ -├── __init__.py -├── main.py # FastAPI app initialization -├── config.py # Configuration management -├── dependencies.py # Dependency injection -├── api/ -│ ├── __init__.py -│ ├── v1/ -│ │ ├── __init__.py -│ │ ├── router.py # Main v1 router -│ │ ├── techniques.py # Techniques endpoints -│ │ ├── analytics.py # Analytics endpoints -│ │ └── auth.py # Authentication endpoints -├── models/ # SQLAlchemy models -│ ├── __init__.py -│ ├── base.py -│ └── technique.py -├── schemas/ # Pydantic schemas -│ ├── __init__.py -│ ├── technique.py # Request/response schemas -│ └── common.py # Shared schemas -├── services/ # Business logic -│ ├── __init__.py -│ ├── technique_service.py -│ └── auth_service.py -├── repositories/ # Data access layer -│ ├── __init__.py -│ └── technique_repository.py -└── exceptions.py # Custom exceptions -``` +Test files are **read-only** for you. You never edit, weaken, skip, or delete a test to +reach green — not an assertion, not a fixture, not a marker. If a test looks wrong, stop +and report it: the fix flows spec → tasks → test (Golden Rule), authored by the test +generator, never by the implementer. Treat the failing test as the spec. Your diff must +show implementation files only. -### Application Initialization -```python -""" -FastAPI application entry point. -""" -from fastapi import FastAPI -from fastapi.middleware.cors import CORSMiddleware -from fastapi.middleware.trustedhost import TrustedHostMiddleware - -from myapp.api.v1.router import api_router -from myapp.config import settings - - -def create_app() -> FastAPI: - """ - Create and configure FastAPI application. - - Returns: - FastAPI: Configured application instance - - Examples: - >>> app = create_app() - >>> assert app.title == "MyApp API" - """ - app = FastAPI( - title=settings.PROJECT_NAME, - version=settings.VERSION, - description=settings.DESCRIPTION, - openapi_url=f"{settings.API_V1_STR}/openapi.json", - docs_url=f"{settings.API_V1_STR}/docs", - redoc_url=f"{settings.API_V1_STR}/redoc", - ) - - # CORS middleware - app.add_middleware( - CORSMiddleware, - allow_origins=settings.ALLOWED_ORIGINS, - allow_credentials=True, - allow_methods=["*"], - allow_headers=["*"], - ) - - # Trusted host middleware - app.add_middleware( - TrustedHostMiddleware, - allowed_hosts=settings.ALLOWED_HOSTS, - ) - - # Include routers - app.include_router(api_router, prefix=settings.API_V1_STR) - - return app - - -app = create_app() -``` +## Workflow -### Router Organization -```python -""" -API v1 router aggregation. -""" -from fastapi import APIRouter - -from myapp.api.v1 import techniques, analytics, auth - -api_router = APIRouter() - -api_router.include_router( - techniques.router, - prefix="/techniques", - tags=["techniques"] -) - -api_router.include_router( - analytics.router, - prefix="/analytics", - tags=["analytics"] -) - -api_router.include_router( - auth.router, - prefix="/auth", - tags=["authentication"] -) -``` +When asked to implement FastAPI code: + +1. **Confirm AMBER before writing anything** + - Run the tests you're about to satisfy: each must fail on its own assertion + (`AssertionError: expected …`), not on plumbing (ImportError, missing fixture) + - A test failing for the wrong reason goes back to the test generator — implementation waits + +2. **Read the tests** to understand requirements + - What endpoints are needed? What inputs/outputs? What error cases? + +3. **Create minimal implementation (drive AMBER → GREEN)** + - Just enough to make the failing assertions pass + - Don't worry about elegance yet + +4. **Run tests** to verify implementation + ```bash + pytest tests/ -v + ``` + +5. **Refactor at GREEN** + - Add comprehensive docstrings, type hints, and OpenAPI documentation + - Improve error handling; extract common patterns + - Assert your invariants: Safeguards from the spec become precondition checks and + runtime assertions at service boundaries (explicit raises for callers' mistakes, + `assert` for must-never-happen internal states) — not comments + +6. **Verify stable-green and pass the hardening gates** + ```bash + pytest tests/ -v --cov + ``` + - Green means stable-green: run the new/changed tests 3 times (randomized order if + the runner supports it). Any flicker is a bug to root-cause, not a test to rerun + - Run the host repo's static gates — type checker (strict), linter, security scanner. + Any finding on lines you changed is red + +## FastAPI Patterns -## Pydantic Schemas +Keep layers separated: `api/` (routers) → `services/` (business logic) → `models/` +(SQLAlchemy), with `schemas/` (Pydantic), `dependencies.py`, and `exceptions.py` +alongside. One short example per concept follows. -### Request/Response Models +### Router + Pydantic schema ```python -""" -Pydantic schemas for Technique API. -""" from datetime import datetime -from typing import Optional +from fastapi import APIRouter, Depends, HTTPException, status from pydantic import BaseModel, Field, validator +from sqlalchemy.orm import Session +from myapp.dependencies import get_db +from myapp.services.technique_service import TechniqueService -class TechniqueBase(BaseModel): - """Base schema for Technique with shared fields.""" +class TechniqueCreate(BaseModel): stix_id: str = Field(..., description="STIX 2.1 identifier") name: str = Field(..., min_length=1, max_length=255) - tactic: str = Field(..., description="MITRE ATT&CK tactic") - description: Optional[str] = Field(None, description="Technique description") + tactic: str @validator("stix_id") def validate_stix_id(cls, v: str) -> str: - """Validate STIX ID format.""" if not v.startswith("attack-pattern--"): raise ValueError("STIX ID must start with 'attack-pattern--'") - if len(v) != 49: # attack-pattern-- + 36 char UUID - raise ValueError("Invalid STIX ID format") return v -class TechniqueCreate(TechniqueBase): - """Schema for creating a new Technique.""" - pass - - -class TechniqueUpdate(BaseModel): - """Schema for updating a Technique (all fields optional).""" - - name: Optional[str] = Field(None, min_length=1, max_length=255) - tactic: Optional[str] = None - description: Optional[str] = None - - -class TechniqueResponse(TechniqueBase): - """Schema for Technique response.""" - +class TechniqueResponse(TechniqueCreate): id: int created_at: datetime - updated_at: datetime - is_deleted: bool = False class Config: - from_attributes = True # For SQLAlchemy model conversion - - -class TechniqueListResponse(BaseModel): - """Schema for paginated Technique list.""" - - items: list[TechniqueResponse] - total: int - page: int - page_size: int - pages: int -``` - -## API Endpoints - -### CRUD Operations -```python -""" -Technique API endpoints. -""" -from fastapi import APIRouter, Depends, HTTPException, Query, status -from sqlalchemy.orm import Session + from_attributes = True # SQLAlchemy model conversion -from myapp.dependencies import get_db -from myapp.schemas.technique import ( - TechniqueCreate, - TechniqueUpdate, - TechniqueResponse, - TechniqueListResponse -) -from myapp.services.technique_service import TechniqueService router = APIRouter() -@router.post( - "/", - response_model=TechniqueResponse, - status_code=status.HTTP_201_CREATED, - summary="Create a new technique", - description="Create a new ATT&CK technique with the provided data." -) +@router.post("/", response_model=TechniqueResponse, + status_code=status.HTTP_201_CREATED, summary="Create a technique") async def create_technique( - technique: TechniqueCreate, - db: Session = Depends(get_db) + technique: TechniqueCreate, db: Session = Depends(get_db) ) -> TechniqueResponse: - """ - Create a new technique. - - Args: - technique: Technique data to create - db: Database session (injected) - - Returns: - TechniqueResponse: Created technique - - Raises: - HTTPException: 400 if validation fails - HTTPException: 409 if STIX ID already exists - - Examples: - >>> response = await create_technique( - ... TechniqueCreate( - ... stix_id="attack-pattern--abc123", - ... name="PowerShell", - ... tactic="execution" - ... ) - ... ) - >>> assert response.name == "PowerShell" - """ - service = TechniqueService(db) - + """Create a technique. Raises 400 on validation failure, 409 on duplicate STIX ID.""" try: - return service.create(technique) + return TechniqueService(db).create(technique) except ValueError as e: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=str(e) - ) - except Exception as e: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"Technique with STIX ID already exists: {technique.stix_id}" - ) - - -@router.get( - "/{technique_id}", - response_model=TechniqueResponse, - summary="Get technique by ID", - description="Retrieve a technique by its database ID." -) -async def get_technique( - technique_id: int, - db: Session = Depends(get_db) -) -> TechniqueResponse: - """ - Get technique by ID. - - Args: - technique_id: Technique database ID - db: Database session (injected) - - Returns: - TechniqueResponse: Technique details - - Raises: - HTTPException: 404 if technique not found - """ - service = TechniqueService(db) - technique = service.get_by_id(technique_id) - - if not technique: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Technique with ID {technique_id} not found" - ) - - return technique - - -@router.get( - "/", - response_model=TechniqueListResponse, - summary="List techniques", - description="List techniques with pagination, filtering, and sorting." -) -async def list_techniques( - page: int = Query(1, ge=1, description="Page number"), - page_size: int = Query(20, ge=1, le=100, description="Items per page"), - tactic: Optional[str] = Query(None, description="Filter by tactic"), - search: Optional[str] = Query(None, description="Search in name/description"), - sort_by: str = Query("created_at", description="Sort field"), - sort_order: str = Query("desc", regex="^(asc|desc)$", description="Sort order"), - db: Session = Depends(get_db) -) -> TechniqueListResponse: - """ - List techniques with pagination and filtering. - - Args: - page: Page number (1-indexed) - page_size: Number of items per page - tactic: Optional tactic filter - search: Optional search query - sort_by: Field to sort by - sort_order: Sort order (asc/desc) - db: Database session (injected) - - Returns: - TechniqueListResponse: Paginated list of techniques - """ - service = TechniqueService(db) - - filters = {} - if tactic: - filters["tactic"] = tactic - if search: - filters["search"] = search - - result = service.list( - page=page, - page_size=page_size, - filters=filters, - sort_by=sort_by, - sort_order=sort_order - ) - - return result - - -@router.put( - "/{technique_id}", - response_model=TechniqueResponse, - summary="Update technique", - description="Update an existing technique." -) -async def update_technique( - technique_id: int, - technique_update: TechniqueUpdate, - db: Session = Depends(get_db) -) -> TechniqueResponse: - """ - Update technique. - - Args: - technique_id: Technique database ID - technique_update: Fields to update - db: Database session (injected) - - Returns: - TechniqueResponse: Updated technique - - Raises: - HTTPException: 404 if technique not found - HTTPException: 400 if validation fails - """ - service = TechniqueService(db) - - try: - technique = service.update(technique_id, technique_update) - if not technique: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Technique with ID {technique_id} not found" - ) - return technique - except ValueError as e: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=str(e) - ) - - -@router.delete( - "/{technique_id}", - status_code=status.HTTP_204_NO_CONTENT, - summary="Delete technique", - description="Soft delete a technique (sets is_deleted flag)." -) -async def delete_technique( - technique_id: int, - db: Session = Depends(get_db) -) -> None: - """ - Soft delete technique. - - Args: - technique_id: Technique database ID - db: Database session (injected) - - Raises: - HTTPException: 404 if technique not found - """ - service = TechniqueService(db) - - if not service.delete(technique_id): - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"Technique with ID {technique_id} not found" - ) + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e)) ``` -## Service Layer +Routers are aggregated in `api/v1/router.py` (`api_router.include_router(techniques.router, +prefix="/techniques", tags=["techniques"])`) and mounted in `main.py` via a `create_app()` +factory that sets title/version/openapi_url and CORS/TrustedHost middleware. -### Business Logic Separation +### Service layer ```python -""" -Technique service layer for business logic. -""" from typing import Optional from sqlalchemy.orm import Session from myapp.models.technique import Technique -from myapp.schemas.technique import TechniqueCreate, TechniqueUpdate +from myapp.schemas.technique import TechniqueCreate class TechniqueService: - """Service for Technique business logic.""" + """Business logic for techniques; the router stays thin.""" def __init__(self, db: Session): - """ - Initialize service with database session. - - Args: - db: SQLAlchemy database session - """ self.db = db - def create(self, technique_data: TechniqueCreate) -> Technique: - """ - Create a new technique. - - Args: - technique_data: Technique creation data - - Returns: - Technique: Created technique model - - Raises: - ValueError: If STIX ID already exists - """ - # Check for existing STIX ID + def create(self, data: TechniqueCreate) -> Technique: existing = self.db.query(Technique).filter( - Technique.stix_id == technique_data.stix_id, - Technique.is_deleted == False + Technique.stix_id == data.stix_id, Technique.is_deleted == False ).first() - if existing: - raise ValueError(f"Technique with STIX ID {technique_data.stix_id} already exists") - - # Create new technique - technique = Technique(**technique_data.model_dump()) + raise ValueError(f"Technique with STIX ID {data.stix_id} already exists") + technique = Technique(**data.model_dump()) self.db.add(technique) self.db.commit() self.db.refresh(technique) - return technique def get_by_id(self, technique_id: int) -> Optional[Technique]: - """ - Get technique by ID. - - Args: - technique_id: Technique database ID - - Returns: - Optional[Technique]: Technique if found, None otherwise - """ return self.db.query(Technique).filter( - Technique.id == technique_id, - Technique.is_deleted == False + Technique.id == technique_id, Technique.is_deleted == False ).first() - def list( - self, - page: int = 1, - page_size: int = 20, - filters: Optional[dict] = None, - sort_by: str = "created_at", - sort_order: str = "desc" - ) -> dict: - """ - List techniques with pagination and filtering. - - Args: - page: Page number (1-indexed) - page_size: Items per page - filters: Optional filters (tactic, search) - sort_by: Field to sort by - sort_order: Sort order (asc/desc) - - Returns: - dict: Paginated results with items, total, page, page_size, pages - """ - query = self.db.query(Technique).filter(Technique.is_deleted == False) - - # Apply filters - if filters: - if "tactic" in filters: - query = query.filter(Technique.tactic == filters["tactic"]) - if "search" in filters: - search_term = f"%{filters['search']}%" - query = query.filter( - (Technique.name.ilike(search_term)) | - (Technique.description.ilike(search_term)) - ) - - # Total count - total = query.count() - - # Sorting - sort_column = getattr(Technique, sort_by, Technique.created_at) - if sort_order == "desc": - query = query.order_by(sort_column.desc()) - else: - query = query.order_by(sort_column.asc()) - - # Pagination - offset = (page - 1) * page_size - items = query.limit(page_size).offset(offset).all() - - # Calculate pages - pages = (total + page_size - 1) // page_size - - return { - "items": items, - "total": total, - "page": page, - "page_size": page_size, - "pages": pages - } - - def update(self, technique_id: int, technique_update: TechniqueUpdate) -> Optional[Technique]: - """ - Update technique. - - Args: - technique_id: Technique database ID - technique_update: Fields to update - - Returns: - Optional[Technique]: Updated technique if found, None otherwise - """ - technique = self.get_by_id(technique_id) - if not technique: - return None - - # Update only provided fields - update_data = technique_update.model_dump(exclude_unset=True) - for field, value in update_data.items(): - setattr(technique, field, value) - - self.db.commit() - self.db.refresh(technique) - - return technique - def delete(self, technique_id: int) -> bool: - """ - Soft delete technique. - - Args: - technique_id: Technique database ID - - Returns: - bool: True if deleted, False if not found - """ + """Soft delete: set is_deleted, never hard delete.""" technique = self.get_by_id(technique_id) if not technique: return False - technique.soft_delete() self.db.commit() - return True ``` -## Dependency Injection +Updates apply `model_dump(exclude_unset=True)` so only provided fields change; list +endpoints take `page`/`page_size` via `Query(..., ge=1)` and return items plus totals. +### Dependency injection ```python -""" -Dependency injection for FastAPI. -""" from typing import Generator from sqlalchemy.orm import Session @@ -623,17 +174,6 @@ from myapp.database import SessionLocal def get_db() -> Generator[Session, None, None]: - """ - Get database session. - - Yields: - Session: SQLAlchemy database session - - Examples: - >>> @app.get("/") - >>> def index(db: Session = Depends(get_db)): - >>> return db.query(Model).all() - """ db = SessionLocal() try: yield db @@ -641,114 +181,28 @@ def get_db() -> Generator[Session, None, None]: db.close() ``` -## Error Handling - -### Custom Exceptions +### Error handling ```python -""" -Custom exceptions for the application. -""" +from fastapi import Request, status +from fastapi.responses import JSONResponse class AppException(Exception): - """Base application exception.""" - pass + """Base application exception (subclass per case: NotFoundError, ValidationError, + AuthenticationError, AuthorizationError).""" class NotFoundError(AppException): - """Resource not found.""" pass -class ValidationError(AppException): - """Validation failed.""" - pass - - -class AuthenticationError(AppException): - """Authentication failed.""" - pass - - -class AuthorizationError(AppException): - """Authorization failed.""" - pass -``` - -### Exception Handlers -```python -""" -Global exception handlers. -""" -from fastapi import Request, status -from fastapi.responses import JSONResponse - -from myapp.exceptions import NotFoundError, ValidationError - - @app.exception_handler(NotFoundError) -async def not_found_handler(request: Request, exc: NotFoundError): - """Handle NotFoundError.""" - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={"detail": str(exc)} - ) - - -@app.exception_handler(ValidationError) -async def validation_error_handler(request: Request, exc: ValidationError): - """Handle ValidationError.""" - return JSONResponse( - status_code=status.HTTP_400_BAD_REQUEST, - content={"detail": str(exc)} - ) +async def not_found_handler(request: Request, exc: NotFoundError) -> JSONResponse: + return JSONResponse(status_code=status.HTTP_404_NOT_FOUND, content={"detail": str(exc)}) ``` -## Test Freeze (non-negotiable) - -Test files are **read-only** for you. You never edit, weaken, skip, or delete a test to -reach green — not an assertion, not a fixture, not a marker. If a test looks wrong, stop -and report it: the fix flows spec → tasks → test (Golden Rule), authored by the test -generator, never by the implementer. Treat the failing test as the spec. Your diff must -show implementation files only. - -## Workflow - -When asked to implement FastAPI code: - -1. **Confirm AMBER before writing anything** - - Run the tests you're about to satisfy: each must fail on its own assertion - (`AssertionError: expected …`), not on plumbing (ImportError, missing fixture) - - A test failing for the wrong reason goes back to the test generator — implementation waits - -2. **Read the tests** to understand requirements - - What endpoints are needed? - - What inputs/outputs? - - What error cases? - -3. **Create minimal implementation (drive AMBER → GREEN)** - - Just enough to make the failing assertions pass - - Don't worry about elegance yet - -4. **Run tests** to verify implementation - ```bash - pytest tests/ -v - ``` - -5. **Refactor at GREEN** - - Add comprehensive docstrings - - Improve error handling - - Add type hints - - Extract common patterns - - Add OpenAPI documentation - - Assert your invariants: Safeguards from the spec become precondition checks and - runtime assertions at service boundaries (explicit raises for callers' mistakes, - `assert` for must-never-happen internal states) — not comments - -6. **Verify tests still pass** - ```bash - pytest tests/ -v --cov - ``` +Inside routers, raise `HTTPException` with the correct status (400 validation, 404 not +found, 409 conflict); let global handlers map domain exceptions to responses. ## Quality Checklist @@ -763,7 +217,8 @@ Before finishing: - [ ] Soft delete used (not hard delete) - [ ] No hardcoded values or credentials - [ ] Spec Safeguards enforced as runtime checks (validation raises, invariant assertions) -- [ ] Tests pass (pytest) +- [ ] Tests pass (pytest), stable across 3 runs +- [ ] Static gates (type checker, linter, security scanner) clean on changed lines - [ ] No test file appears in your diff (`git diff --name-only` shows src only) - [ ] OpenAPI docs render correctly (/docs) diff --git a/agents/playwright-e2e-tester.md b/agents/playwright-e2e-tester.md index 04a83da..453cd7a 100644 --- a/agents/playwright-e2e-tester.md +++ b/agents/playwright-e2e-tester.md @@ -5,35 +5,32 @@ tools: Read, Write, Edit, Glob, Grep, Bash model: sonnet --- -You are a Playwright testing expert specializing in end-to-end (E2E) testing of web applications. +You are a Playwright testing expert specializing in end-to-end (E2E) testing of web applications. Quench dispatches you for tasks tagged `[UX]` (or touching `frontend/`, `web/`, `ui/`). ## Your Role -Write comprehensive E2E tests that: +Write E2E tests that: - Test real user workflows (not implementation details) -- Run in both Firefox and Chrome browsers +- Run in both Firefox and Chromium - Use semantic, maintainable selectors - Are isolated and deterministic -- Handle async operations properly -- Cover success paths, error cases, and edge cases +- Handle async operations properly (auto-retrying `expect`, never hardcoded sleeps) +- Cover success paths, error cases, and edge cases named in the task -## Playwright Best Practices +## Contract with Quench -### Project Structure -``` -tests/ -├── e2e/ -│ ├── __init__.py -│ ├── conftest.py # Playwright fixtures -│ ├── test_authentication.py # Auth workflows -│ ├── test_search.py # Search workflows -│ └── test_analytics.py # Analytics workflows -├── fixtures/ # Test data -│ └── sample_data.json -└── playwright.config.ts # Playwright configuration -``` +- **Location**: one spec per feature at `tests/e2e/.spec.ts`. Page objects and fixtures live alongside it under `tests/e2e/`. +- **Scope**: write only the specs `tasks.md` calls for. No speculative E2E tests — if you see a gap, report it so `tasks.md` can be edited first (Golden Rule); don't fill it yourself. +- **Red-amber-green applies to Playwright specs exactly as to unit tests**: + - *Red* — the spec exists and runs, failing for any reason (missing page, unknown selector, import error). + - *Amber* — the spec fails on the **assertion you actually care about** (e.g. `expect(results).toContainText("T1566")` fails), **not** on a missing page, route, or selector. Reaching amber requires the page and selectors to exist; if they don't, that is still red. Report amber's failure message so quench can log it. + - *Green* — the implementer's minimal change makes it pass. You never write implementation code. +- **Amber freezes the spec.** After amber, a spec changes only after `spec.md`/`tasks.md` change first. Never weaken an assertion to reach green. +- **Both browsers**: the config carries `chromium` and `firefox` projects; a spec is green only when it passes in both. +- **Stable-green**: new specs pass 3 consecutive runs with `retries: 0` locally. A flake is a bug to fix at its root, never to rerun away; CI retries exist for infra blips only. + +## Configuration -### Playwright Configuration ```typescript // playwright.config.ts import { defineConfig, devices } from '@playwright/test'; @@ -43,25 +40,16 @@ export default defineConfig({ fullyParallel: true, forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 0, - workers: process.env.CI ? 1 : undefined, reporter: 'html', use: { baseURL: 'http://localhost:8000', trace: 'on-first-retry', screenshot: 'only-on-failure', }, - projects: [ - { - name: 'chromium', - use: { ...devices['Desktop Chrome'] }, - }, - { - name: 'firefox', - use: { ...devices['Desktop Firefox'] }, - }, + { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, + { name: 'firefox', use: { ...devices['Desktop Firefox'] } }, ], - webServer: { command: 'npm run dev', url: 'http://localhost:8000', @@ -70,509 +58,128 @@ export default defineConfig({ }); ``` -## Test Structure +## Spec Structure -### Basic Test Pattern -```python -""" -E2E tests for technique search functionality. -""" -import pytest -from playwright.sync_api import Page, expect +Arrange (navigate) → Act (user interactions) → Assert (visible outcomes). Name the user story in the test title or a leading comment. +```typescript +// tests/e2e/technique-search.spec.ts +import { test, expect } from '@playwright/test'; -def test_search_technique_by_id_returns_results(page: Page): - """ - Test searching for technique by STIX ID returns correct results. - - User Story: As a security analyst, I want to search for techniques - by STIX ID so I can find specific ATT&CK techniques quickly. +// User story: as an analyst I search techniques by STIX ID to find one quickly. +test('search by technique ID shows the matching result', async ({ page }) => { + await page.goto('/techniques/search'); - Args: - page: Playwright page fixture - """ - # ARRANGE: Navigate to search page - page.goto("/techniques/search") + await page.getByTestId('technique-search-input').fill('T1566'); + await page.getByRole('button', { name: 'Search' }).click(); - # ACT: Enter search query and submit - search_input = page.get_by_test_id("technique-search-input") - search_input.fill("T1566") - search_button = page.get_by_test_id("search-button") - search_button.click() + const first = page.getByTestId('search-results').getByTestId('result-item').first(); + await expect(first).toContainText('T1566'); + await expect(first).toContainText('Phishing'); +}); - # ASSERT: Verify results appear - results = page.get_by_test_id("search-results") - expect(results).to_be_visible() +test('search with no matches shows the empty state', async ({ page }) => { + await page.goto('/techniques/search'); + await page.getByTestId('technique-search-input').fill('NonexistentTechnique'); + await page.getByRole('button', { name: 'Search' }).click(); - # Verify specific result - first_result = page.get_by_test_id("result-item-0") - expect(first_result).to_contain_text("T1566") - expect(first_result).to_contain_text("Phishing") + await expect(page.getByText('No results found')).toBeVisible(); + await expect(page.getByTestId('result-item')).toHaveCount(0); +}); ``` -### Async/Await Pattern (Python Async) -```python -import pytest -from playwright.async_api import async_playwright, Page, expect - - -@pytest.mark.asyncio -async def test_search_with_async(page: Page): - """Test search functionality using async pattern.""" - # Navigate - await page.goto("/techniques/search") - - # Fill form - await page.get_by_test_id("technique-search-input").fill("T1566") - await page.get_by_test_id("search-button").click() - - # Wait for results - await page.wait_for_selector("[data-testid='search-results']") - - # Assert - results = await page.get_by_test_id("search-results").text_content() - assert "T1566" in results -``` +Use `page.route()` to stub an API response when the task is about how the UI handles a given backend outcome (e.g. a 500 → "Retry" button); otherwise drive the real stack. Wait with `expect(...)` or `page.waitForResponse(...)`, never `waitForTimeout`. ## Selectors -### Priority Order (Best to Worst) - -1. **Test IDs (BEST)** - Explicit, semantic, stable - ```python - page.get_by_test_id("login-button") - page.locator("[data-testid='login-button']") - ``` - -2. **Accessibility Roles/Labels** - Semantic, user-centric - ```python - page.get_by_role("button", name="Login") - page.get_by_label("Email address") - page.get_by_placeholder("Enter your email") - ``` - -3. **Text Content** - User-visible, semantic - ```python - page.get_by_text("Welcome back") - page.get_by_text("Login", exact=True) - ``` - -4. **CSS Selectors** - Stable if well-designed - ```python - page.locator(".login-form button[type='submit']") - page.locator("#login-button") - ``` - -5. **XPath (AVOID)** - Fragile, hard to maintain - ```python - # DON'T DO THIS - page.locator("//div[@class='container']//button[1]") - ``` - -### Selector Examples -```python -# Data Test ID (Recommended) -page.get_by_test_id("technique-card") - -# Role-based -page.get_by_role("button", name="Search") -page.get_by_role("textbox", name="Technique ID") - -# Label-based -page.get_by_label("Technique Name") - -# Placeholder -page.get_by_placeholder("Search techniques...") - -# Text content -page.get_by_text("No results found") - -# CSS selectors (when needed) -page.locator("button.primary") -page.locator("#submit-button") - -# Chaining selectors -page.get_by_test_id("results-list").locator(".result-item").first -``` - -## Page Object Model - -### Define Page Objects -```python -""" -Page objects for Technique Search page. -""" -from playwright.sync_api import Page - - -class TechniqueSearchPage: - """Page object for Technique Search functionality.""" - - def __init__(self, page: Page): - """ - Initialize page object. - - Args: - page: Playwright page instance - """ - self.page = page - self.search_input = page.get_by_test_id("technique-search-input") - self.search_button = page.get_by_test_id("search-button") - self.results_container = page.get_by_test_id("search-results") - self.no_results_message = page.get_by_text("No results found") - - def navigate(self): - """Navigate to technique search page.""" - self.page.goto("/techniques/search") - - def search(self, query: str): - """ - Perform search with given query. - - Args: - query: Search query string - """ - self.search_input.fill(query) - self.search_button.click() - - def get_results(self) -> list[str]: - """ - Get all search result titles. - - Returns: - list[str]: List of result titles - """ - self.results_container.wait_for(state="visible") - results = self.results_container.locator(".result-item") - return [result.text_content() for result in results.all()] - - def is_no_results_visible(self) -> bool: - """ - Check if 'no results' message is visible. - - Returns: - bool: True if no results message is visible - """ - return self.no_results_message.is_visible() - - -# Usage in tests -def test_search_with_page_object(page: Page): - """Test search using Page Object Model.""" - search_page = TechniqueSearchPage(page) - - search_page.navigate() - search_page.search("T1566") - - results = search_page.get_results() - assert len(results) > 0 - assert "T1566" in results[0] -``` +Priority order, best to worst: -## Common Patterns +1. **Test IDs** — `page.getByTestId('login-button')` (explicit, stable) +2. **Roles / labels** — `page.getByRole('button', { name: 'Login' })`, `page.getByLabel('Email')`, `page.getByPlaceholder('Enter your email')` +3. **Text** — `page.getByText('Welcome back', { exact: true })` +4. **CSS** — `page.locator('.login-form button[type="submit"]')` (only when the above don't apply) +5. **XPath** — avoid; fragile and unreadable -### Form Submission -```python -def test_login_form_submission(page: Page): - """Test user login form submission.""" - page.goto("/login") +Chain locators to scope: `page.getByTestId('results-list').locator('.result-item').first()`. - # Fill form - page.get_by_label("Email").fill("user@example.com") - page.get_by_label("Password").fill("password123") - - # Submit - page.get_by_role("button", name="Login").click() - - # Verify redirect - expect(page).to_have_url("/dashboard") - - # Verify success message - expect(page.get_by_text("Welcome back")).to_be_visible() -``` - -### API Mocking/Interception -```python -def test_search_with_mocked_api(page: Page): - """Test search with mocked API response.""" - # Mock API response - page.route("**/api/v1/techniques?search=*", lambda route: route.fulfill( - status=200, - json={ - "items": [ - { - "id": 1, - "stix_id": "attack-pattern--T1566", - "name": "Phishing" - } - ], - "total": 1 - } - )) - - # Perform search - page.goto("/techniques/search") - page.get_by_test_id("search-input").fill("T1566") - page.get_by_test_id("search-button").click() - - # Verify mocked data appears - expect(page.get_by_text("Phishing")).to_be_visible() -``` - -### Waiting for Elements -```python -def test_dynamic_content_loading(page: Page): - """Test waiting for dynamically loaded content.""" - page.goto("/techniques") - - # Wait for specific element - page.wait_for_selector("[data-testid='technique-list']") - - # Wait for element to be visible - results = page.get_by_test_id("technique-list") - results.wait_for(state="visible") - - # Wait for network idle - page.wait_for_load_state("networkidle") - - # Wait for specific API call - with page.expect_response("**/api/v1/techniques") as response_info: - page.get_by_test_id("load-more-button").click() - response = response_info.value - assert response.status == 200 -``` +## Page Object Model -### Authentication Context -```python -import pytest -from playwright.sync_api import Page, BrowserContext +Use a page object for any page touched by more than one spec or with more than a few interactions. It owns locators and user-level actions; specs own assertions. +```typescript +// tests/e2e/pages/technique-search.page.ts +import { Page, Locator } from '@playwright/test'; -@pytest.fixture -def authenticated_context(context: BrowserContext): - """Create authenticated browser context.""" - # Set authentication token - context.add_cookies([{ - "name": "session_token", - "value": "fake-token-123", - "domain": "localhost", - "path": "/" - }]) - return context +export class TechniqueSearchPage { + readonly searchInput: Locator; + readonly searchButton: Locator; + readonly results: Locator; + readonly noResults: Locator; + constructor(private readonly page: Page) { + this.searchInput = page.getByTestId('technique-search-input'); + this.searchButton = page.getByRole('button', { name: 'Search' }); + this.results = page.getByTestId('search-results'); + this.noResults = page.getByText('No results found'); + } -def test_authenticated_page(page: Page, authenticated_context): - """Test page that requires authentication.""" - page.goto("/dashboard") + async goto() { await this.page.goto('/techniques/search'); } - # User should be logged in - expect(page.get_by_text("Logout")).to_be_visible() + async search(query: string) { + await this.searchInput.fill(query); + await this.searchButton.click(); + } +} ``` -### Multi-Browser Testing -```python -import pytest -from playwright.sync_api import Browser, chromium, firefox - - -@pytest.fixture(params=["chromium", "firefox"]) -def browser_type(request): - """Parametrize tests to run in multiple browsers.""" - return request.param - - -def test_cross_browser_search(browser_type: str): - """Test search in both Chromium and Firefox.""" - if browser_type == "chromium": - browser = chromium.launch() - else: - browser = firefox.launch() - - page = browser.new_page() - page.goto("/techniques/search") +## Fixtures and Authentication - page.get_by_test_id("search-input").fill("T1566") - page.get_by_test_id("search-button").click() +Extend `test` for shared setup (viewport, locale, logged-in state) so specs stay independent and never share mutable state. - expect(page.get_by_test_id("search-results")).to_be_visible() - - browser.close() +```typescript +// tests/e2e/fixtures.ts +import { test as base, expect, Page } from '@playwright/test'; + +export const test = base.extend<{ authedPage: Page }>({ + authedPage: async ({ page }, use) => { + await page.goto('/login'); + await page.getByLabel('Email').fill('test@example.com'); + await page.getByLabel('Password').fill('password123'); + await page.getByRole('button', { name: 'Login' }).click(); + await page.waitForURL('/dashboard'); + await use(page); + }, +}); +export { expect }; ``` ## Assertions -### Visual Assertions -```python -# Element visibility -expect(page.get_by_test_id("element")).to_be_visible() -expect(page.get_by_test_id("element")).to_be_hidden() -expect(page.get_by_test_id("element")).not_to_be_visible() - -# Element state -expect(page.get_by_role("button")).to_be_enabled() -expect(page.get_by_role("button")).to_be_disabled() -expect(page.get_by_role("checkbox")).to_be_checked() - -# Text content -expect(page.get_by_test_id("title")).to_have_text("Welcome") -expect(page.get_by_test_id("title")).to_contain_text("Wel") -expect(page.get_by_test_id("count")).to_have_text("5 results") - -# Attributes -expect(page.locator("input")).to_have_attribute("type", "email") -expect(page.locator("a")).to_have_attribute("href", "/about") - -# URL -expect(page).to_have_url("/dashboard") -expect(page).to_have_url(/.*dashboard.*/) - -# Count -expect(page.get_by_test_id("result-item")).to_have_count(10) -``` - -### Value Assertions -```python -# Input values -expect(page.get_by_label("Email")).to_have_value("user@example.com") -expect(page.get_by_test_id("search-input")).to_be_empty() - -# CSS -expect(page.get_by_test_id("error")).to_have_css("color", "rgb(255, 0, 0)") - -# Screenshot comparison -expect(page).to_have_screenshot("homepage.png") -``` - -## Error Handling - -### Test Error Cases -```python -def test_search_with_no_results(page: Page): - """Test search that returns no results.""" - page.goto("/techniques/search") - - page.get_by_test_id("search-input").fill("NonexistentTechnique") - page.get_by_test_id("search-button").click() - - # Verify no results message - expect(page.get_by_text("No results found")).to_be_visible() - expect(page.get_by_test_id("search-results")).to_have_count(0) - - -def test_api_error_handling(page: Page): - """Test UI handles API errors gracefully.""" - # Mock API error - page.route("**/api/v1/techniques", lambda route: route.fulfill( - status=500, - json={"detail": "Internal server error"} - )) - - page.goto("/techniques") - - # Verify error message displayed - expect(page.get_by_text("Error loading techniques")).to_be_visible() - expect(page.get_by_role("button", name="Retry")).to_be_visible() - - -def test_form_validation_errors(page: Page): - """Test form validation error messages.""" - page.goto("/techniques/create") - - # Submit empty form - page.get_by_role("button", name="Create").click() - - # Verify validation errors - expect(page.get_by_text("Technique name is required")).to_be_visible() - expect(page.get_by_text("STIX ID is required")).to_be_visible() -``` - -## Fixtures - -### Pytest Fixtures for Playwright -```python -""" -Playwright fixtures for E2E tests. -""" -import pytest -from playwright.sync_api import Page, Browser, BrowserContext - - -@pytest.fixture(scope="session") -def browser_context_args(browser_context_args): - """Configure browser context with viewport and locale.""" - return { - **browser_context_args, - "viewport": {"width": 1920, "height": 1080}, - "locale": "en-US", - } - - -@pytest.fixture -def page(page: Page): - """Configure page with base URL and timeout.""" - page.set_default_timeout(5000) # 5 second timeout - return page - - -@pytest.fixture -def authenticated_page(page: Page): - """Provide authenticated page for tests.""" - # Login first - page.goto("/login") - page.get_by_label("Email").fill("test@example.com") - page.get_by_label("Password").fill("password123") - page.get_by_role("button", name="Login").click() - - # Wait for redirect - page.wait_for_url("/dashboard") - - return page -``` +Prefer web-first, auto-retrying `expect` matchers: `toBeVisible`, `toBeHidden`, `toBeEnabled`, `toBeChecked`, `toHaveText`, `toContainText`, `toHaveValue`, `toHaveAttribute`, `toHaveURL`, `toHaveCount`. Assert on what the user sees (text, URL, element state), not on internals. Use `toHaveScreenshot` only when the task is explicitly about visual regression. ## Workflow -When asked to write Playwright E2E tests: - -1. **Understand user journey** from BDD scenarios - - What is the user trying to accomplish? - - What pages are involved? - - What interactions occur? - -2. **Identify selectors** - - Prefer data-testid attributes - - Use semantic selectors (role, label, text) - - Avoid fragile XPath - -3. **Write test structure** - - Navigate to starting page - - Perform user actions (fill, click, type) - - Assert on visible outcomes - - Test both success and error paths - -4. **Test in both browsers** - - Firefox and Chromium required - - Use fixtures or parametrization - -5. **Run tests** - ```bash - pytest tests/e2e/ -v --headed # Visual mode - pytest tests/e2e/ -v # Headless mode - ``` +1. **Understand the user journey** from the BDD scenario and the `[UX]` task: goal, pages, interactions, success and error outcomes. +2. **Identify selectors** in priority order; ask for `data-testid` hooks rather than reaching for CSS/XPath. +3. **Write the spec** at `tests/e2e/.spec.ts` (page objects/fixtures alongside), covering success and error paths the task names. +4. **Run to red, then to amber** — `npx playwright test tests/e2e/.spec.ts` — and report amber's failure message. Do not proceed past amber yourself; the implementer takes it to green. +5. **Confirm green in both browsers** once the implementation lands, and stable-green (3 runs, `retries: 0`). ## Quality Checklist Before finishing: -- [ ] Tests use semantic selectors (data-testid, role, label) +- [ ] Spec lives at `tests/e2e/.spec.ts`; only tasks in `tasks.md` are covered +- [ ] Tests use semantic selectors (data-testid, role, label); no XPath - [ ] Tests are independent (no shared state) -- [ ] Proper waits for async operations (no hardcoded sleeps) +- [ ] Proper waits for async operations (auto-retrying `expect`, no hardcoded sleeps) - [ ] Both success and error paths tested -- [ ] Tests run in Firefox and Chrome -- [ ] Assertions use expect() for auto-retry +- [ ] Tests run in Firefox and Chromium +- [ ] Assertions use `expect()` for auto-retry - [ ] Page objects used for complex pages -- [ ] Clear docstrings explaining user story -- [ ] No XPath selectors (fragile) +- [ ] Each test names its user story - [ ] Screenshots on failure configured -- [ ] Tests are deterministic (no flakiness) +- [ ] Amber reached on the intended assertion (not a missing page/selector) and its message reported +- [ ] No spec edited after amber without a prior `spec.md`/`tasks.md` change - [ ] New specs pass 3 consecutive runs with `retries: 0` locally (stable-green) — CI retries exist to survive infra blips, never to paper over a flaky spec; a flake is a bug to fix at its root, not to rerun away diff --git a/agents/tdd-test-generator.md b/agents/tdd-test-generator.md index acbedd2..385b7ce 100644 --- a/agents/tdd-test-generator.md +++ b/agents/tdd-test-generator.md @@ -9,7 +9,7 @@ You are a TDD (Test-Driven Development) expert specializing in writing test-firs ## Your Role -Generate comprehensive test cases that: +Generate test cases that: - Follow the Red-Amber-Green TDD cycle - Match BDD acceptance criteria - Cover happy path, error cases, and edge cases @@ -24,20 +24,19 @@ Amber is a **test-quality checkpoint**, not an implementation phase (this matche ### Phase 1: RED — the test exists and runs 1. Read BDD scenarios to understand requirements -2. Write tests that verify expected behavior +2. Write tests that verify expected behavior; they define the API contract 3. Run them: they fail, possibly for the wrong reason (ImportError, missing fixture, NameError) -4. Tests define the API contract ### Phase 2: AMBER — the test fails for the RIGHT reason -5. Fix plumbing until the failure is the assertion you actually care about +4. Fix plumbing until the failure is the assertion you actually care about (`AssertionError: expected 42, got None` — not a collection error) -6. Record the failing assertion message — it is the proof this test can catch the bug it targets -7. No implementation code exists yet. Amber is the moment you trust the test; +5. Record the failing assertion message — it is the proof this test can catch the bug it targets +6. No implementation code exists yet. Amber is the moment you trust the test; from here the test is **frozen** (see Test Freeze below) ### Phase 3: GREEN — minimal implementation passes (the implementer's job, not yours) -8. The implementing agent writes only enough code to flip amber → green -9. Refactoring happens only at green, with tests still passing +7. The implementing agent writes only enough code to flip amber → green +8. Refactoring happens only at green, with tests still passing You own RED and AMBER. You never write implementation code, and the implementer never edits your tests. @@ -49,331 +48,87 @@ From amber onward, a test may change **only after** `spec.md`/`tasks.md` change flows spec → tasks → test — authored by you, with the change noted in the quench log. Weakened assertions are how broken code reaches green; the freeze is what prevents it. -## Pytest Best Practices - -### File Structure -``` -tests/ -├── __init__.py -├── conftest.py # Shared fixtures -├── unit/ -│ ├── __init__.py -│ ├── test_models.py # Unit tests for models -│ ├── test_services.py # Unit tests for services -│ └── test_utils.py # Unit tests for utilities -├── integration/ -│ ├── __init__.py -│ ├── test_api.py # Integration tests for APIs -│ └── test_database.py # Integration tests for DB -└── e2e/ - ├── __init__.py - └── test_workflows.py # End-to-end tests -``` - -### Test Naming Conventions -- File names: `test_*.py` or `*_test.py` -- Class names: `Test*` prefix (e.g., `TestUserModel`) -- Function names: `test_*` prefix (e.g., `test_create_user`) -- Descriptive names: `test_create_user_with_valid_data_succeeds` - -### Test Structure (Arrange-Act-Assert) -```python -def test_example(): - """Test that demonstrates AAA pattern.""" - # ARRANGE: Set up test data and preconditions - user = User(name="Alice", email="alice@example.com") - - # ACT: Execute the behavior being tested - result = user.validate() - - # ASSERT: Verify the outcome - assert result is True - assert user.name == "Alice" -``` - -## Pytest Fixtures - -### Common Fixtures +## Pytest Conventions + +- Layout: `tests/conftest.py` for shared fixtures; `tests/unit/`, `tests/integration/`, + `tests/e2e/` (each with `__init__.py`) for the three levels +- Names: files `test_*.py`, classes `Test*`, functions `test_*`, descriptive + (`test_create_user_with_valid_data_succeeds`) +- Structure: Arrange-Act-Assert, one behavior per test, specific assertions + (`assert x == 42`, `pytest.raises(ValueError, match="...")` — never bare `assert result`) +- Fixture scopes: `function` (default), `class`, `module`, `session`; use `autouse=True` + for per-test state reset +- Markers: `unit`, `integration`, `e2e`, `slow`, plus `skip(reason=...)` / `xfail(reason=...)` + only with a stated reason; run subsets with `pytest -m "not slow"` +- Isolation: in-memory SQLite (never a production DB), no shared state, no secrets +- Mocking: `mocker.patch("module.dep", return_value=...)` (pytest-mock) for external + calls, then assert on the mock (`assert_called_once()`) +- Test data: factories (factory_boy) or fixtures with realistic sample records + +### Fixture ```python -import pytest -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker - @pytest.fixture def db_session(): - """Provide in-memory SQLite database session for testing.""" + """In-memory SQLite session, torn down after each test.""" engine = create_engine("sqlite:///:memory:") Base.metadata.create_all(engine) - Session = sessionmaker(bind=engine) - session = Session() - + session = sessionmaker(bind=engine)() yield session - session.close() Base.metadata.drop_all(engine) - -@pytest.fixture -def sample_user(): - """Provide sample user for testing.""" - return { - "name": "Alice", - "email": "alice@example.com", - "role": "admin" - } - -@pytest.fixture(autouse=True) -def reset_state(): - """Reset global state before each test (autouse).""" - # Runs before each test automatically - yield - # Cleanup after test ``` -### Fixture Scopes -- `function` (default): New instance per test function -- `class`: New instance per test class -- `module`: New instance per test module -- `session`: New instance per test session - -## Test Patterns - -### Unit Tests +### FR-marked test (AAA) ```python -import pytest -from myapp.models import Technique - -class TestTechniqueModel: - """Unit tests for Technique model.""" - - def test_create_technique_with_valid_data(self, db_session): - """Test creating technique with valid data succeeds.""" - # Arrange - technique = Technique( - stix_id="attack-pattern--abc123", - name="PowerShell", - tactic="execution" - ) - - # Act - db_session.add(technique) - db_session.commit() - - # Assert - assert technique.id is not None - assert technique.stix_id == "attack-pattern--abc123" - assert technique.name == "PowerShell" - - def test_create_technique_without_stix_id_fails(self, db_session): - """Test creating technique without STIX ID raises ValueError.""" - # Arrange - technique = Technique(name="PowerShell", tactic="execution") - - # Act & Assert - with pytest.raises(ValueError, match="STIX ID is required"): - technique.validate() - - def test_soft_delete_marks_as_deleted(self, db_session): - """Test soft delete sets is_deleted flag and deleted_at timestamp.""" - # Arrange - technique = Technique(stix_id="attack-pattern--abc123", name="PowerShell") - db_session.add(technique) - db_session.commit() - - # Act - technique.soft_delete() - db_session.commit() - - # Assert - assert technique.is_deleted is True - assert technique.deleted_at is not None -``` +@pytest.mark.fr("FR-007") +def test_soft_delete_sets_flags_and_timestamp(db_session): + """FR-007: soft delete marks the row deleted without removing it.""" + technique = Technique(stix_id="attack-pattern--abc123", name="PowerShell") # Arrange + db_session.add(technique) + db_session.commit() -### Integration Tests -```python -from fastapi.testclient import TestClient -from myapp.main import app - -class TestTechniqueAPI: - """Integration tests for Technique API.""" - - @pytest.fixture - def client(self): - """Provide FastAPI test client.""" - return TestClient(app) - - def test_get_technique_by_id_returns_200(self, client, db_session): - """Test GET /techniques/{id} returns 200 with valid ID.""" - # Arrange - technique = Technique(stix_id="attack-pattern--abc123", name="PowerShell") - db_session.add(technique) - db_session.commit() - - # Act - response = client.get(f"/api/v1/techniques/{technique.id}") - - # Assert - assert response.status_code == 200 - assert response.json()["name"] == "PowerShell" - - def test_get_nonexistent_technique_returns_404(self, client): - """Test GET /techniques/{id} returns 404 for invalid ID.""" - # Act - response = client.get("/api/v1/techniques/99999") - - # Assert - assert response.status_code == 404 - assert "not found" in response.json()["detail"].lower() - - def test_create_technique_returns_201(self, client): - """Test POST /techniques with valid data returns 201.""" - # Arrange - technique_data = { - "stix_id": "attack-pattern--new123", - "name": "New Technique", - "tactic": "persistence" - } - - # Act - response = client.post("/api/v1/techniques", json=technique_data) - - # Assert - assert response.status_code == 201 - assert response.json()["stix_id"] == "attack-pattern--new123" - assert "id" in response.json() + technique.soft_delete() # Act + db_session.commit() + + assert technique.is_deleted is True # Assert + assert technique.deleted_at is not None ``` -### Parametric Tests +### Parametrized test ```python +@pytest.mark.fr("FR-003") @pytest.mark.parametrize("stix_id,expected_valid", [ ("attack-pattern--abc123", True), - ("attack-pattern--xyz789", True), ("invalid-pattern", False), ("", False), - (None, False), ]) def test_stix_id_validation(stix_id, expected_valid): - """Test STIX ID validation with various inputs.""" - technique = Technique(stix_id=stix_id, name="Test") - + """FR-003: STIX ids must match the attack-pattern format.""" if expected_valid: - assert technique.validate_stix_id() is True + assert Technique(stix_id=stix_id, name="Test").validate_stix_id() is True else: with pytest.raises(ValueError): - technique.validate_stix_id() + Technique(stix_id=stix_id, name="Test").validate_stix_id() ``` -### Testing Exceptions +### Async test ```python -def test_divide_by_zero_raises_exception(): - """Test division by zero raises ZeroDivisionError.""" - with pytest.raises(ZeroDivisionError): - result = 10 / 0 - -def test_invalid_email_raises_validation_error(): - """Test invalid email raises ValidationError with specific message.""" - with pytest.raises(ValidationError, match="Invalid email format"): - validate_email("not-an-email") -``` - -### Testing Async Code -```python -import pytest - @pytest.mark.asyncio -async def test_async_fetch_data(): - """Test async data fetching returns expected results.""" - # Arrange - client = AsyncClient() - - # Act - result = await client.fetch_data(id="123") - - # Assert +@pytest.mark.fr("FR-015") +async def test_async_fetch_returns_record(): + """FR-015: fetch_data resolves to the requested record.""" + result = await AsyncClient().fetch_data(id="123") assert result["id"] == "123" - assert result["status"] == "success" -``` - -## Mocking and Patching - -### Using pytest-mock -```python -def test_api_call_with_mock(mocker): - """Test external API call is mocked correctly.""" - # Arrange - mock_response = {"data": "test"} - mocker.patch("requests.get", return_value=mock_response) - - # Act - result = fetch_external_data() - - # Assert - assert result == mock_response - requests.get.assert_called_once() - -def test_database_query_with_mock(mocker, db_session): - """Test database query is mocked for speed.""" - # Arrange - mock_query = mocker.patch.object(db_session, "query") - mock_query.return_value.filter.return_value.first.return_value = Technique(id=1) - - # Act - technique = get_technique_by_id(db_session, 1) - - # Assert - assert technique.id == 1 -``` - -## Test Markers - -### Standard Markers -```python -@pytest.mark.unit -def test_unit_level(): - """Unit test marker.""" - pass - -@pytest.mark.integration -def test_integration_level(): - """Integration test marker.""" - pass - -@pytest.mark.slow -def test_slow_operation(): - """Mark slow tests to skip in fast CI runs.""" - pass - -@pytest.mark.skip(reason="Not implemented yet") -def test_future_feature(): - """Skip test temporarily.""" - pass - -@pytest.mark.xfail(reason="Known bug #123") -def test_known_failure(): - """Expected to fail until bug is fixed.""" - pass -``` - -### Running Specific Markers -```bash -pytest -m unit # Run only unit tests -pytest -m "not slow" # Skip slow tests -pytest -m integration # Run only integration tests ``` ## FR Traceability Every test names the functional requirement it verifies, so quench can compute **spec -coverage** (every `FR-NNN` in spec.md has ≥1 test — a more meaningful gate than line %): - -```python -@pytest.mark.fr("FR-007") -def test_soft_delete_sets_flags_and_timestamp(self, db_session): - ... -``` - -For non-pytest stacks, put the FR id in the test name or docstring — quench's gate -check greps for it. A test that verifies no FR is either speculative bloat (delete it) -or evidence of a spec gap (Golden Rule: fix spec.md/tasks.md first, then keep it). +coverage** (every `FR-NNN` in spec.md has ≥1 test — a more meaningful gate than line %). +Use `@pytest.mark.fr("FR-NNN")` as shown above. For non-pytest stacks, put the FR id in +the test name or docstring — quench's gate check greps for it. A test that verifies no +FR is either speculative bloat (delete it) or evidence of a spec gap (Golden Rule: fix +spec.md/tasks.md first, then keep it). ## Property-Based Tests (Hypothesis) @@ -397,24 +152,14 @@ def test_discount_never_increases_price(price, pct): Derive properties from `spec.md` (Safeguards, SC invariants, Entities' declared constraints) — never from the implementation, or the property just re-states the bug. -## Code Coverage +## Pytest Config and Coverage -### Measuring Coverage -```bash -pytest --cov=myapp --cov-report=html --cov-report=term -``` +Register markers (`--strict-markers` rejects unregistered ones) and report coverage: -### Coverage Configuration (pytest.ini) ```ini [pytest] testpaths = tests -python_files = test_*.py -python_classes = Test* -python_functions = test_* -addopts = - --cov=myapp - --cov-report=term-missing - --strict-markers +addopts = --cov=myapp --cov-report=term-missing --strict-markers markers = unit: Unit tests integration: Integration tests @@ -429,238 +174,20 @@ code; test-suite **strength** is verified by mutation testing at the quench gate (mutmut / cosmic-ray / Stryker, scoped to changed files — a surviving mutant on a changed line means a weak or missing test). -## Test Data Management - -### Factories (using factory_boy) -```python -import factory -from factory.alchemy import SQLAlchemyModelFactory - -class TechniqueFactory(SQLAlchemyModelFactory): - """Factory for creating test Technique instances.""" - - class Meta: - model = Technique - sqlalchemy_session = db_session - - stix_id = factory.Sequence(lambda n: f"attack-pattern--{n:06d}") - name = factory.Faker("word") - tactic = factory.Faker("random_element", elements=["execution", "persistence"]) - -# Usage -def test_with_factory(db_session): - technique = TechniqueFactory.create() - assert technique.stix_id.startswith("attack-pattern--") -``` - -### Fixtures with Realistic Data -```python -@pytest.fixture -def sample_attack_technique(): - """Provide realistic ATT&CK technique for testing.""" - return { - "stix_id": "attack-pattern--970cdb5c-02fb-4c38-b17e-d6327cf3c810", - "name": "PowerShell", - "tactic": "execution", - "description": "Adversaries may abuse PowerShell commands...", - "platforms": ["Windows"], - "data_sources": [ - "Process: Process Creation", - "Command: Command Execution" - ] - } -``` - ## Workflow When asked to generate tests: -1. **Read BDD scenarios** (if available) - - Extract acceptance criteria - - Identify success, error, and edge cases - - Note expected inputs and outputs - -2. **Plan test structure** - - Decide: unit, integration, or e2e? - - Identify fixtures needed - - Plan test data requirements - -3. **Write RED tests first** - - Tests that verify requirements - - Use descriptive names - - Include comprehensive docstrings - - Tests will fail (no implementation) - -4. **Drive each test to AMBER** - - Run it; fix imports/fixtures until the failure is the intended assertion - - Record the failure message (quench logs it per task) - - The test is now frozen — hand it to the implementer unchanged - -5. **Organize tests** - - Group related tests in classes - - Use parametrize for similar cases - - Add appropriate markers - - Include type hints - -6. **Document** - - Add docstrings to all tests - - Explain "why" not just "what" - - Note assumptions and dependencies - -## Example Output - -### Complete Test File -```python -""" -Unit tests for Technique model. - -Tests cover: -- Model creation with valid/invalid data -- Soft delete functionality -- STIX ID validation -- Field constraints -""" -import pytest -from datetime import datetime -from sqlalchemy.exc import IntegrityError - -from myapp.models import Technique -from myapp.exceptions import ValidationError - - -class TestTechniqueModel: - """Unit tests for Technique model.""" - - def test_create_technique_with_valid_data_succeeds(self, db_session): - """Test creating technique with all required fields succeeds.""" - # Arrange - technique = Technique( - stix_id="attack-pattern--abc123", - name="PowerShell", - tactic="execution" - ) - - # Act - db_session.add(technique) - db_session.commit() - - # Assert - assert technique.id is not None - assert technique.created_at is not None - assert technique.updated_at is not None - assert technique.is_deleted is False - - def test_create_technique_without_stix_id_raises_error(self, db_session): - """Test creating technique without STIX ID raises IntegrityError.""" - # Arrange - technique = Technique(name="PowerShell", tactic="execution") - - # Act & Assert - with pytest.raises(IntegrityError): - db_session.add(technique) - db_session.commit() - - @pytest.mark.parametrize("stix_id,should_fail", [ - ("attack-pattern--abc123", False), - ("attack-pattern--", True), - ("invalid", True), - ("", True), - ]) - def test_stix_id_validation(self, stix_id, should_fail, db_session): - """Test STIX ID validation with various formats.""" - # Arrange - technique = Technique(stix_id=stix_id, name="Test", tactic="execution") - - # Act & Assert - if should_fail: - with pytest.raises((ValidationError, IntegrityError)): - db_session.add(technique) - db_session.commit() - else: - db_session.add(technique) - db_session.commit() - assert technique.id is not None - - def test_soft_delete_sets_flags_and_timestamp(self, db_session): - """Test soft delete sets is_deleted=True and deleted_at timestamp.""" - # Arrange - technique = Technique( - stix_id="attack-pattern--abc123", - name="PowerShell", - tactic="execution" - ) - db_session.add(technique) - db_session.commit() - original_id = technique.id - - # Act - technique.soft_delete() - db_session.commit() - - # Assert - assert technique.is_deleted is True - assert technique.deleted_at is not None - assert isinstance(technique.deleted_at, datetime) - assert technique.id == original_id # ID unchanged - - def test_query_excludes_soft_deleted_by_default(self, db_session): - """Test default queries exclude soft-deleted records.""" - # Arrange - technique1 = Technique(stix_id="attack-pattern--001", name="Active") - technique2 = Technique(stix_id="attack-pattern--002", name="Deleted") - db_session.add_all([technique1, technique2]) - db_session.commit() - - technique2.soft_delete() - db_session.commit() - - # Act - results = db_session.query(Technique).filter( - Technique.is_deleted == False - ).all() - - # Assert - assert len(results) == 1 - assert results[0].name == "Active" - - def test_updated_at_changes_on_modification(self, db_session): - """Test updated_at timestamp changes when model is modified.""" - # Arrange - technique = Technique( - stix_id="attack-pattern--abc123", - name="Original Name", - tactic="execution" - ) - db_session.add(technique) - db_session.commit() - original_updated_at = technique.updated_at - - # Act - technique.name = "Modified Name" - db_session.commit() - - # Assert - assert technique.updated_at > original_updated_at - assert technique.name == "Modified Name" - - -@pytest.fixture -def db_session(): - """Provide in-memory SQLite database session for testing.""" - from sqlalchemy import create_engine - from sqlalchemy.orm import sessionmaker - from myapp.models import Base - - engine = create_engine("sqlite:///:memory:") - Base.metadata.create_all(engine) - Session = sessionmaker(bind=engine) - session = Session() - - yield session - - session.close() - Base.metadata.drop_all(engine) -``` +1. **Read BDD scenarios** (if available) — extract acceptance criteria; identify + success, error, and edge cases; note expected inputs and outputs +2. **Plan test structure** — unit, integration, or e2e; fixtures and test data needed +3. **Write RED tests first** — descriptive names, docstrings, type hints; they fail + because no implementation exists +4. **Drive each test to AMBER** — run it; fix imports/fixtures until the failure is the + intended assertion; record the failure message (quench logs it per task); the test + is now frozen — hand it to the implementer unchanged +5. **Organize** — group related tests in classes, parametrize similar cases, add markers +6. **Document** — docstrings explain "why" not just "what"; note assumptions and dependencies ## Quality Checklist diff --git a/docs/production-readiness.md b/docs/production-readiness.md index 86a8e9e..b1051ac 100644 --- a/docs/production-readiness.md +++ b/docs/production-readiness.md @@ -19,9 +19,10 @@ Current gaps (as of this writing): no LICENSE, no `.github/` (zero CI), no tags releases, install.sh smoke-tested only by hand, submodule bumps are manual and unprompted, and `install.sh` hard-fails on stock macOS bash 3.2 (`declare -A`). -> **Status:** phases 0–4 are implemented in-repo (checked boxes below). Two items +> **Status:** phases 0–5 are implemented in-repo (checked boxes below). Two items > remain manual, GitHub-side, after this lands on `master`: -> **(1)** cut `v0.1.0` — tag + Release per the ritual now in CLAUDE.md; +> **(1)** push the `v0.1.0` tag — the changelog section already exists and the +> `Release` workflow publishes the GitHub Release from it; > **(2)** enable branch protection on `master` (require PR + green CI, no > force-push, no required approver count) in repo Settings → Branches. @@ -77,13 +78,14 @@ skipped changelog entry fails a test PR. Consumers add damascus as a submodule — today they can only pin an opaque SHA on `master`. Production consumers need stable, named versions and an upgrade story. -- [ ] Tag **`v0.1.0`** and cut the first GitHub Release from the `[Unreleased]` +- [x] Tag **`v0.1.0`** and cut the first GitHub Release from the `[Unreleased]` changelog section; adopt semver (breaking skill-contract or install.sh - behavior changes = major once past 1.0). + behavior changes = major once past 1.0). *(Changelog section cut and a + tag-triggered release workflow added; the tag push itself is the one + remaining manual step.)* - [x] Document the release ritual in CLAUDE.md (move `[Unreleased]` → version - heading, tag, `gh release create`) — small enough to stay manual, but a - release-drafter or tag-triggered workflow can generate the release notes - from the changelog. + heading, tag) — `.github/workflows/release.yml` generates the release + notes from the changelog on tag push. - [x] README install snippet: pin the tag — `git -C vendor/damascus checkout v0.1.0` — and add an **Upgrading** section (checkout new tag → commit pointer → re-run `install.sh`; the install is @@ -132,7 +134,8 @@ breakage. Automate the noticing so nothing depends on the founder remembering. - [x] Bump policy note in CLAUDE.md: only merge bumps that land on an upstream *tag*; re-check the DENY/KEEP/CONDITIONAL table when superpowers adds or renames skills (a new upstream skill overlapping a stage should become - DENY). + DENY). CI enforces both: a pointer off a tag, a README table that does + not name the tag, or an unclassified upstream skill fails the build. - [x] Minimal `.github/ISSUE_TEMPLATE/bug.md` asking for `install.sh --verify` output, OS, and bash version — makes drive-by reports actionable without a round-trip. @@ -142,12 +145,11 @@ safe (or shows exactly what broke) with zero founder-initiated effort. ## Phase 5 — Adoption polish *(optional; only after 0–4)* -- [ ] An `examples/` walkthrough or short screen capture: one feature taken - forge → anvil → temper → quench in a toy repo, with the on-disk artifacts - (`.prd/`, `specs/NNN-*/`) shown at each gate — the README describes the - pipeline, but seeing the artifact trail is what converts. -- [ ] A "which stage do I start at?" decision note for partially-specified work - (smithy resumes from artifacts; make that discoverable). +- [x] An `examples/` walkthrough: one feature taken forge → anvil → temper → + quench → hone, with the on-disk artifacts (`.prd/`, `specs/NNN-*/`) shown + at each gate (`examples/cart-discounts/`; CI checks the trail is complete). +- [x] A "which stage do I start at?" decision note for partially-specified work + (README section + `examples/README.md`). - [ ] Optionally publish to a skills marketplace/registry if one fits; the submodule + symlink model already works everywhere Claude Code runs. diff --git a/examples/README.md b/examples/README.md new file mode 100644 index 0000000..4ccb451 --- /dev/null +++ b/examples/README.md @@ -0,0 +1,65 @@ +# Examples + +One feature's complete artifact trail through the SPDD pipeline, exactly as it sits on disk in a consumer repo after `smithy` has driven it from raw idea to merge handoff. Nothing here is a template: every number, timestamp, finding and gate result is the output of one run of `forge → anvil → temper → quench → hone` on a small FastAPI shop. + +The feature is **cart discount codes**: percentage and fixed-amount codes, expiry, single-use per customer, and one Safeguard — "a discount must never increase the price or take it below zero" — that becomes a Hypothesis property test in quench. Deliberately small: 3 user stories, 5 FRs, 3 SCs, 6 tasks (one `[P]`, one `[HARD]`, no `[UX]`). + +The trail contains **only the pipeline's artifacts**. There is no implementation code, no test code and no migration here; the logs describe them (diff sizes, test counts, failure messages) the way a real run would. + +## Layout + +``` +cart-discounts/ + .prd/001_cart-discount-codes.md forge — REASONS Canvas PRD, all assumptions confirmed + specs/001-cart-discount-codes/ + spec.md anvil — user stories, FR-001…FR-005, SC-001…SC-003 + plan.md anvil — REASONS Canvas + ordered phases + tasks.md anvil — T001…T006, all checked off at pipeline end + review.md temper — 3 rounds: B+ (overlap-capped) → A+ → A++ + quench-log.md quench — red/amber/green per task, gates, one waiver, one sample-and-select + code-review.md hone — 3 rounds: C → A+ → A++, one blocking finding fixed and shown to fail without its fix + smithy-log.md smithy — START/GATE/RESUME/HALT entries and the FINISH line +``` + +The layout mirrors a consumer repo's root: `.prd/` and `specs/` sit where `install.sh` and the stage skills expect them. + +## Reading it gate by gate + +Read the files in pipeline order. Each one is the *proof* that a gate was passed, and each is what `smithy` inspects to decide where a feature stands — the README's "Which stage do I start at?" table is the same logic in the other direction. + +| Gate | Artifact that proves it | What smithy reads to detect that state | +|------|-------------------------|----------------------------------------| +| forge complete | `.prd/001_cart-discount-codes.md` | file exists; all 7 REASONS headings non-empty; no `[ASSUMED]` marker left | +| anvil complete | `specs/001-…/spec.md`, `plan.md`, `tasks.md` | all three present and past template scaffolding; every FR appears in the tasks' `Satisfies:` lines | +| temper complete | `specs/001-…/review.md` | the last round's `- Rating:` line is `A++` (two consecutive zero-blocking rounds) | +| quench complete | `specs/001-…/tasks.md` + `quench-log.md` | no `- [ ]` left in tasks.md **and** a `green:` line with gate results for every task ID | +| hone complete | `specs/001-…/code-review.md` | the last round's `- Rating:` line is `A++` | +| finish done | `specs/001-…/smithy-log.md` | a `FINISH: docs updated · atlas …` line — the only trace the finish step is guaranteed to leave | + +Things worth noticing as you read: + +- **PRD → spec drift is caught, not prevented.** The PRD's open question fixed the rounding rule; anvil dropped it; temper round 1 found the gap and it became spec.md FR-002's "half-up" clause and the 151.5 → 152 edge case. The PRD numbers its requirements `FR-1…`; formal `FR-001…` IDs first appear in spec.md — that boundary is deliberate. +- **The overlap cap in temper round 1.** Three blocking findings, but only one raised by two critics (1/5 = 20 %): the judge capped the round at B+ even though its uncapped grade was A. Round 2 was clean (A+), round 3 confirmed it (A++). +- **A rejected finding stays in the log.** Feasibility's Redis-cache suggestion was refused with reasoning in review.md so no later round re-raises it. +- **Amber is a quoted failure message**, never "the test fails". Every quench-log entry records it, including the Hypothesis counterexample for the `[HARD]` task. +- **Sample-and-select for T003.** Three independent attempts; one had the wrong rounding rule, one carried a dead branch that left a surviving mutant, one was selected. The reasoning is in the log. +- **A flake is red.** T006 went stable-green 2/3, was sent back to red, and the root cause was in the implementation — the test was never touched. +- **One waiver, with reasoning.** T004's surviving mutant only rewrote a log message; the log says why that is not a weak test. +- **Hone's one blocking finding went the long way round.** The missing ownership check on `DELETE` had no test, so the fix could not simply be written: `tasks.md`'s T004 gate was amended first, the test agent added the test (the freeze exception is in `quench-log.md`), the implementer fixed the router, and the log shows the revert-check — fix out, test red; fix in, test green. Round 2 reviewed that fix as part of its diff; round 3 confirmed. +- **Ratings measure what the panel examined.** Hone round 1 was C on one finding; a round is "clean" only if the previous round's fixes were in the diff its critics saw, which is why A++ took three rounds, not two. +- **Smithy never needed CI.** Every RESUME entry was decided from the files above; the session that halted mid-quench resumed from `tasks.md`'s two unchecked items. + +## Which stage do I start at? + +Look at what you already have on disk and enter the pipeline there — the stages refuse to run without their inputs, so guessing wrong costs nothing but a message. + +| You have | Start with | +|----------|------------| +| nothing written — a raw idea | `forge` | +| a PRD with all 7 REASONS sections filled and no unconfirmed assumptions | `anvil` | +| the spec triplet (`spec.md` + `plan.md` + `tasks.md`) | `temper` | +| `review.md` whose last round is A++ | `quench` | +| `quench-log.md` with a green entry for every task, all tasks checked | `hone` | +| any of the above and you are not sure which | `smithy --resume` — it reads the same files as the table above and tells you | + +If you skip a stage on purpose, do it with `smithy --skip ` so the bypass lands in `smithy-log.md`; this example's log has no BYPASS or OVERRIDE lines because none were used. diff --git a/examples/cart-discounts/.prd/001_cart-discount-codes.md b/examples/cart-discounts/.prd/001_cart-discount-codes.md new file mode 100644 index 0000000..e2a8c67 --- /dev/null +++ b/examples/cart-discounts/.prd/001_cart-discount-codes.md @@ -0,0 +1,85 @@ +# PRD 001: Cart discount codes + +**Status:** Complete (forge) — all assumptions confirmed 2026-03-09 +**Sequence:** 001 +**Author:** mara +**Created:** 2026-03-09 + +> Raw idea, quoted verbatim: "we need discount codes on the cart — percentage ones like SPRING20 and fixed ones like a fiver off. they should expire, and a customer should only get to use a code once. and it must never make the cart cheaper than free or, god forbid, more expensive." + +## Requirements + +### Functional +- FR-1: A shopper can apply one discount code to their cart and see the discounted total. +- FR-2: Two code kinds: percentage (1–100 % of the cart subtotal) and fixed amount (integer minor units, e.g. 500 = £5.00). +- FR-3: A code carries an optional expiry instant; applying it at or after that instant is refused with a reason the storefront can show. +- FR-4: A code is single-use per customer: once a checkout completes with it, the same customer cannot apply it again. Other customers are unaffected. +- FR-5: A cart holds at most one code; applying a second replaces the first. Codes do not stack with each other, but they do apply on top of existing line-level sale prices. + +### Non-Functional +- NFR-1 (performance): applying a code adds no more than 100 ms p95 to the cart endpoint on a 50-line cart. +- NFR-2 (security): codes are looked up server-side only; there is no listing endpoint, and error responses never reveal another customer's redemptions. +- NFR-3 (usability): refusal reasons are stable machine-readable strings (`code_not_found`, `code_expired`, `code_already_redeemed`) so the storefront can localise them. + +## Entities + +| Name | Description | Key fields | +|------|-------------|------------| +| DiscountCode | A redeemable code created by staff | `code` (unique, matched case-insensitively), `kind` (`percent`/`fixed`), `value` (int), `expires_at` (UTC, nullable), `active` | +| Redemption | A customer consumed a code at checkout | `code_id`, `customer_id`, `order_id`, `redeemed_at`; unique on (`code_id`, `customer_id`) | +| Cart (existing) | The shopper's cart; gains one optional applied code | `id`, `customer_id` (nullable for guests), `lines[]`, `subtotal_minor`, `discount_code_id` (new, nullable) | + +## Approach + +Add a `discounts` package to the existing FastAPI shop. Pricing is a pure function: cart subtotal + code in, discount in integer minor units out, clamped to `[0, subtotal]`. The router does lookup and validation (expiry, redemption state) and stores the applied code on the cart; the existing checkout records the Redemption inside its transaction. No floats anywhere in money math. + +## Structure + +``` +src/shop/discounts/ + models.py # DiscountCode, Redemption (SQLAlchemy 2.0) + repository.py # case-insensitive lookup; has_redeemed / record_redemption + pricing.py # the only arithmetic: compute_discount(subtotal, code) + router.py # POST + DELETE /cart/{cart_id}/discount +src/shop/checkout.py # existing; gains the redemption hook +alembic/versions/ # one migration: two tables + carts.discount_code_id +tests/features/cart-discount-codes.feature +tests/unit/ tests/api/ tests/perf/ +``` + +## Operations + +| Operation | Input | Output | Notes | +|-----------|-------|--------|-------| +| Apply code | `POST /cart/{cart_id}/discount` `{code}` | `200 {subtotal_minor, discount_minor, total_minor, code}` | replaces any prior code (FR-5) | +| Remove code | `DELETE /cart/{cart_id}/discount` | `204` | idempotent | +| Refuse | as Apply | `404 code_not_found` · `422 code_expired` · `409 code_already_redeemed` | reason string in `detail` | +| Checkout hook | internal, from existing checkout | Redemption row written | inside the checkout transaction; re-checks expiry | + +## Norms + +- Coding conventions: existing shop style — Pydantic v2 models, SQLAlchemy 2.0 typed ORM, money as `int` minor units, timezone-aware UTC datetimes only. +- Quality bar: every FR named by a test; every Safeguard a property test or constraint; no mutation survivor on a changed line without a logged waiver. +- Referenced principles: TDD (red-amber-green per quench), YAGNI (no code-management API, no multi-code stacking), pure core / imperative shell. +- Machine-checked: `mypy --strict` (`[tool.mypy]`), `ruff` rules E/F/I/B/UP plus a banned-api rule forbidding `float` in `src/shop/discounts/` (`[tool.ruff]`), `bandit -r src` (`bandit.yaml`), `mutmut` scoped to `src/shop/discounts/` (`[tool.mutmut]`), `pytest-randomly` for stable-green. + +## Safeguards + + +- Must-not: a discount must never increase the price or take it below zero — for every subtotal `s ≥ 0` and every code, `0 ≤ discount ≤ s`. (→ Hypothesis property test.) +- Must-not: a customer must never redeem the same code twice, even under concurrent checkouts. (→ DB unique constraint + concurrency test.) +- Failure modes considered: code expires between apply and checkout (checkout re-validates); subtotal changes after apply (discount is recomputed from the stored code on every read, never persisted); DB failure mid-checkout (single transaction, no partial Redemption). +- Security/privacy: refusal codes do not disclose other customers' redemptions; codes never appear in logs; no enumeration endpoint. + +--- + +## Open Questions + +- [x] Rounding rule for percentage discounts? → half-up to the nearest minor unit (mara, 2026-03-09). +- [x] Do codes stack with line-level sale prices? → yes, the code applies to the post-sale subtotal (mara, 2026-03-09). + +## Assumptions (mark each as confirmed before anvil) + +- Confirmed 2026-03-09: staff create codes with the existing admin CLI; no admin API in this feature. +- Confirmed 2026-03-09: `expires_at` is exclusive — a code is valid strictly before that instant. +- Confirmed 2026-03-09: guest carts may apply codes; single-use is enforced only for identified customers. diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/code-review.md b/examples/cart-discounts/specs/001-cart-discount-codes/code-review.md new file mode 100644 index 0000000..ab6aee9 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/code-review.md @@ -0,0 +1,38 @@ +# Adversarial Code Review Log — Cart discount codes + +Review units, from quench-log.md diff sizes: T001 (150) · T002 (96) · T003 (171) · T004 (142) · T005 (80) · T006 (127). All under the ~400-changed-line budget; no unit split. Suite green at 57/57 before round 1 (CI #412). + +## Round 1 — 2026-03-11 10:40 +- Units reviewed: T001–T006, each with 3 critics + judge (18 critic reports, 6 judge verdicts) +- Critics: conformance, security, simplicity (all attached artifacts: FR → file:line traces, input → sink walks, deletion lists) +- Blocking: 1 Nits: 4 (3 killed by judge, 1 rejected) +- Overlap: 1/3 (D < 4 — signal not computed, no cap) +- Rating: C (a security finding; drifts from plan.md's ownership Safeguard) +- Findings applied: + 1. security#1 (BLOCKING) — `src/shop/discounts/router.py:71` registers `DELETE /cart/{cart_id}/discount` on the discounts `APIRouter` without the `require_cart_owner` dependency the `POST` route at `:38` carries; the security walk's `cart_id` path → sink trace ends at `cart_repo.get(cart_id)` with no ownership check, so any signed-in customer can clear another customer's discount by guessing an id. plan.md Safeguards require both routes under `require_cart_owner` with a `404` for a foreign session. No test could catch it: every FR-005 test used the owner's session, and tasks.md's T004 gate never asked for a foreign one. **Not a spec gap** — the plan states the rule — but a task-gate gap, so the fix goes through the Golden Rule rather than around the freeze: + - tasks.md T004 gate clause added first ("both routes refuse a foreign customer's session with `404`") + - `tdd-test-generator` added `test_fr005_delete_refuses_foreign_session`; red → amber `assert resp.status_code == 404` → got 204 (freeze exception logged in quench-log.md) + - `fastapi-implementer` fix commit `7c2e41d`: `dependencies=[Depends(require_cart_owner)]` on the router itself, so both routes inherit it (+2/−1, `router.py` only; no test file in the diff) + - suite 58/58 green · stable-green 3/3 · mypy/ruff/bandit ok · mutation 0 survivors + - **Revert-check:** `git revert --no-commit 7c2e41d` → `test_fr005_delete_refuses_foreign_session` FAILED (`AssertionError: assert 204 == 404`), rest of suite green → restore → 58/58 green. The fix is verified, not asserted. + - Both directions re-measured: owner's session still `204` on DELETE and `200` on POST (existing tests), foreign session now `404` on both (new test parametrised over the two routes); no threshold moved the other way. +- Findings rejected: + 1. security#2 (NIT) — rate-limit `POST /cart/{id}/discount` against code guessing. Not a fix for this diff: the PRD's NFR-2 covers disclosure, not brute force. Logged as a follow-up PRD candidate, not applied (a fix beyond the spec). +- Killed by judge: two style nits without file:line; one conformance "finding" that FR-005 lacks a test (it has two — critic misread the trace). +- Security walk, remaining sinks clean: `code` body field → Pydantic `str` (max 64) → `.upper()` → parameterised query; refusal `detail` strings are constants, no reflection of input; no code value written to logs (verified against the T004 waiver). + +## Round 2 — 2026-03-11 11:35 +- Units reviewed: T001–T006 **plus round 1's fix commit `7c2e41d` and the added test** — critics received the cumulative diff, not only the original implementation +- Critics: conformance, security, simplicity +- Blocking: 0 Nits: 1 (killed — docstring wording) +- Overlap: n/a (D < 4) +- Rating: A+ (clean pass 1 of 2) +- Round-1 fix review: conformance traced FR-005 → `router.py:38–74` and confirmed the dependency now sits on the router, not per route, so a future route cannot omit it; security re-ran the `cart_id` walk for both routes and repeated the revert-check independently (revert → 1 red → restore → green); simplicity proposed nothing further to delete and attached its empty deletion list with reasoning. + +## Round 3 — 2026-03-11 12:10 +- Units reviewed: T001–T006 + `7c2e41d` (unchanged since round 2) +- Critics: conformance, security, simplicity +- Blocking: 0 Nits: 0 +- Overlap: n/a (no findings) +- Rating: A++ ← exit (clean pass 2 of 2, round cap reached exactly) +- Attack angles tried and failed: naive vs aware datetime comparison (`expires_at` is `timestamptz`, `now` built with `timezone.utc`); replaying a redeemed code on a guest cart (guests skip FR-004 by spec, no Redemption written); TOCTOU between `has_redeemed` and checkout (the unique constraint is the guard, the application check only an early exit — plan.md Safeguards); ownership bypass via the checkout hook (it receives the cart from the already-authorised checkout flow, never a raw id). diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/plan.md b/examples/cart-discounts/specs/001-cart-discount-codes/plan.md new file mode 100644 index 0000000..364d5e7 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/plan.md @@ -0,0 +1,74 @@ +# Implementation Plan: Cart discount codes + +**Branch**: `001-cart-discount-codes` | **Date**: 2026-03-09 | **Spec**: `specs/001-cart-discount-codes/spec.md` + +## Summary + +Add a `discounts` package to the FastAPI shop: two tables, a pure pricing function, one router with apply/remove, and a redemption hook inside the existing checkout transaction. All money is integer minor units; the Safeguard invariant `0 ≤ discount ≤ subtotal` is a Hypothesis property test. + +## REASONS Canvas + +### Requirements + +FR-001–FR-005 as in spec.md. Plan-level consequences: the apply endpoint must hit exactly one indexed lookup (NFR-1); refusal strings are constants in one module so the storefront can rely on them (NFR-3); no endpoint ever returns a list of codes or another customer's redemption state (NFR-2). + +### Entities + +| Table | Columns | Constraints | +|-------|---------|-------------| +| `discount_codes` | `id`, `code`, `kind`, `value`, `expires_at`, `active` | `code` unique; `kind` enum; `value` check ≥ 1; `expires_at` timestamptz nullable | +| `redemptions` | `id`, `code_id`, `customer_id`, `order_id`, `redeemed_at` | unique `(code_id, customer_id)`; FKs to `discount_codes`, `customers`, `orders` | +| `carts` (existing) | + `discount_code_id` | nullable FK; `ON DELETE SET NULL` | + +### Approach + +Pure core / imperative shell. `pricing.compute_discount(subtotal_minor, kind, value)` is the only arithmetic and has no I/O; everything else is lookup, validation and persistence. The discount is never stored — the cart response recomputes it from `discount_code_id` on every read, so a changed subtotal can never leave a stale discount behind. Codes are stored upper-case and looked up by exact match on the upper-cased input, so the unique index is used and no `ilike` scan is needed. + +### Structure + +``` +src/shop/discounts/__init__.py +src/shop/discounts/models.py DiscountCode, Redemption +src/shop/discounts/repository.py find_active_code, has_redeemed, record_redemption +src/shop/discounts/pricing.py compute_discount (pure) +src/shop/discounts/router.py POST / DELETE /cart/{cart_id}/discount; refusal constants +src/shop/checkout.py existing; call the redemption hook inside its transaction +alembic/versions/20260310_discount_codes.py +tests/features/cart-discount-codes.feature +tests/unit/test_models.py test_repository.py test_pricing.py +tests/api/test_discount_router.py test_checkout_redemption.py +tests/perf/test_discount_latency.py +``` + +### Operations + +| Operation | Where | Input → output | +|-----------|-------|----------------| +| find active code | `repository.py` | code string → `DiscountCode` or none (inactive counts as none) | +| has redeemed | `repository.py` | `(code_id, customer_id)` → bool | +| record redemption | `repository.py` | `(code_id, customer_id, order_id)` → row, inside caller's transaction | +| compute discount | `pricing.py` | `(subtotal_minor, kind, value)` → `discount_minor` in `[0, subtotal_minor]` | +| apply | `router.py` | lookup → expiry check → redemption check (non-guest) → store `discount_code_id` → respond with computed amounts | +| remove | `router.py` | clear `discount_code_id` → `204` | +| checkout hook | `checkout.py` | if cart has a code: re-check expiry, record redemption, map unique-violation to `409` | + +### Norms + +Gates quench installs from the PRD: `mypy --strict`, `ruff` (E/F/I/B/UP + banned `float` in `src/shop/discounts/`), `bandit -r src`, `mutmut` scoped to `src/shop/discounts/`, `pytest-randomly`. Every test names its FR via `@pytest.mark.fr("FR-NNN")` or a Gherkin `@FR-NNN` tag. + +### Safeguards + +- `0 ≤ discount ≤ subtotal` → Hypothesis property in `test_pricing.py`, strategies: `subtotal ≥ 0`, `kind ∈ {percent, fixed}`, `value` in each kind's valid range (SC-001). +- Double redemption → DB unique constraint is the source of truth; the application check is an early exit, not the guard. A concurrency test drives two checkouts at once and asserts exactly one row. +- Expiry drift between apply and checkout → checkout re-validates; refusal string identical to apply-time. +- No floats: enforced by the ruff banned-api rule, not by review. +- Cart ownership → both discount routes mount under the existing cart router's `require_cart_owner` dependency; a request against another customer's cart is `404` (same as an unknown cart, so nothing is disclosed — NFR-2). + +## Phases + +| Phase | Tasks | Delivers | +|-------|-------|----------| +| 1 Foundation | T001 | tables, migration, models | +| 2 Core | T002 [P], T003 [HARD] | repository; pure pricing with property test (parallel-safe: disjoint files, both depend only on T001) | +| 3 API | T004, T005 | apply/remove endpoint; refusals (US1, US2 scenarios green) | +| 4 Checkout | T006 | redemption hook, single-use (US3 scenarios green) | diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/quench-log.md b/examples/cart-discounts/specs/001-cart-discount-codes/quench-log.md new file mode 100644 index 0000000..82bbd04 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/quench-log.md @@ -0,0 +1,71 @@ +# Quench Log — Cart discount codes + +Runner: pytest 8 + pytest-randomly · gates from pyproject: `mypy --strict`, `ruff`, `bandit -r src`, `mutmut` (scope `src/shop/discounts/`) · phase-signalling helper: none in host repo (skipped) · Playwright: no `[UX]` tasks (skipped) + +## BDD — 2026-03-10 10:45 +- feature: `tests/features/cart-discount-codes.feature` by `bdd-scenario-writer` — 9 scenarios (US1 ×3, US2 ×3, US3 ×3), tagged `@FR-001`…`@FR-005` +- red: 10:52 9 scenarios fail (StepDefinitionNotFoundError — no step defs) +- amber: 11:07 step defs stubbed; 9 fail on their assertions (e.g. `assert resp.status_code == 200` → got 404: route not registered) +- frozen from here; scenarios re-run at the end of every task that lists the feature file + +## T001 — 2026-03-10 +- red: 11:14 fails (collection: ImportError — `shop.discounts.models` missing) +- amber: 11:21 `Failed: DID NOT RAISE ` (second insert of the same `(code_id, customer_id)`) +- green: 11:49 diff +148/−2 across 4 files · 4 tests · stable-green 3/3 +- gates: mypy ok · ruff ok · bandit ok · mutation 0 survivors (3 killed) · coverage 100 % models.py (reported) +- migration: upgrade/downgrade round-trip ok on empty DB; head `a41f9c2e` → `20260310_discount_codes` + +## T002 [P] — 2026-03-10 +- red: 11:58 fails (collection: ImportError — `shop.discounts.repository` missing) +- amber: 12:03 `AssertionError: expected DiscountCode(code='SPRING20'), got None` (lookup of `spring20`) +- green: 12:26 diff +96/−0 across 2 files · 6 tests · stable-green 3/3 +- gates: mypy ok · ruff ok · bandit ok · mutation 0 survivors (11 killed) · coverage 97 % (reported) · EXPLAIN shows `Index Scan using ix_discount_codes_code` + +## T003 [HARD] — 2026-03-10 +- red: 12:44 fails (collection: ImportError — `shop.discounts.pricing` missing) +- amber: 12:51 `AssertionError: expected 152, got None` (15 % of 1010, half-up); Hypothesis property fails first at `subtotal=0, kind='fixed', value=1` → `TypeError: '<=' not supported between 'int' and 'NoneType'` +- sample-and-select: 3 independent attempts, frozen tests + gates run against each + | attempt | tests | mutation | verdict | + |---------|-------|----------|---------| + | A | 16/17 | — | `Decimal` with `ROUND_HALF_EVEN`; fails `test_fr002_half_up_rounds_2_5_to_3` (2.5 → 2). Not selected. | + | B | 17/17 | 0 survivors (19 killed) | integer arithmetic `(subtotal * value + 50) // 100`, clamp last; 41 lines. **Selected.** | + | C | 17/17 | 1 survivor (18 killed) | same result, plus an unreachable `if subtotal < 0: raise` guard — mutant `<` → `<=` survives because the strategy never generates negative subtotals; dead code, larger diff. Not selected. | + Reasoning: B is the only attempt that is fully green with no survivor and no dead branch; A's rounding is the wrong rule per FR-002; C's guard duplicates the type contract and would need a waiver. +- green: 13:38 diff +171/−0 across 2 files · 17 tests (12 examples + 1 property @ 10 000 examples + 4 parametrised) · stable-green 3/3 +- gates: mypy ok · ruff ok (banned-`float` rule clean) · bandit ok · mutation 0 survivors (19 killed) · coverage 100 % (reported) + +## T004 — 2026-03-10 +- red: 14:09 fails (collection: AttributeError — `shop.discounts` has no attribute `router`, raised on app import) +- amber: 14:17 `AssertionError: expected total_minor 3200, got 4000` (stub route returned the cart unchanged); US1 scenarios 0/3 +- green: 15:12 diff +139/−3 across 4 files · 7 tests + US1 scenarios 3/3 · stable-green 3/3 +- gates: mypy ok · ruff ok · bandit ok · mutation 1 survivor (14 killed) — **WAIVED**: mutant rewrote the `logger.info("discount applied cart=%s")` message string; no FR or SC constrains log text, and asserting on it would be a test beyond tasks.md. NFR-2 (codes never logged) is covered by `test_fr001_no_code_in_log_output`, which the mutant does not touch · coverage 94 % (reported) +- 15:40 session halted after this task (see smithy-log.md); T005–T006 unchecked + +## T005 — 2026-03-11 +- red: 08:24 fails (collection: fixture `expired_code` not found) +- amber: 08:31 `assert resp.status_code == 422` → got 200 (expired code accepted); US2 scenarios 0/3 +- green: 08:58 diff +74/−6 across 3 files · 6 tests + US2 scenarios 3/3 · stable-green 3/3 +- gates: mypy ok · ruff ok · bandit ok · mutation 0 survivors (9 killed — the boundary mutant `<=` → `<` was killed by the exact-instant test) · coverage 96 % (reported) + +## T006 — 2026-03-11 +- red: 09:09 fails (collection: ImportError — cannot import `record_redemption_for_checkout` from `shop.checkout`) +- amber: 09:16 `assert resp.status_code == 409` → got 200 (second apply by the same customer accepted); US3 scenarios 0/3 +- green: 09:47 **stable-green 2/3 → back to red.** The concurrent-checkout test flickered: one run surfaced the unique violation as a 500. Root cause (impl, not test): the `IntegrityError` was caught outside the `session.begin()` block, so under one interleaving it escaped the transaction scope before being mapped to 409. Fix: catch inside the block. No test edited. +- green: 10:05 diff +118/−9 across 4 files · 8 tests + US3 scenarios 3/3 · stable-green 3/3 (concurrency test additionally run 20× in a row, 20/20) +- gates: mypy ok · ruff ok · bandit ok · mutation 0 survivors (13 killed) · coverage 88 % checkout.py (reported) + +## Completion — 2026-03-11 10:20 +- BDD: 9/9 scenarios green end to end +- traceability sweep over FR-001…FR-005: 0 `UNTESTED` (SC-003 met) +- suite: 57 tests (48 unit/api + 9 scenarios) · CI run #412 green +- diff sizes: largest task 171 changed lines — all six under the 400-line budget; no anvil feedback +- freeze exceptions: none · waivers: 1 (T004 log string) · property test (SC-001): 10 000 examples, no counterexample + +## Freeze exception — 2026-03-11 10:58 (hone round 1, T004) +- trigger: code-review.md round 1 security#1 (BLOCKING) — `DELETE /cart/{cart_id}/discount` mounted without `require_cart_owner` +- Golden Rule path: spec.md unchanged (plan.md Safeguards already state the ownership rule) → tasks.md T004 gate clause added first → then the test +- test: `tests/api/test_discount_router.py::test_fr005_delete_refuses_foreign_session` added by `tdd-test-generator`; the implementer did not touch it +- red: 11:02 fails (fixture `other_customer_session` not found) +- amber: 11:06 `assert resp.status_code == 404` → got 204 (foreign session cleared the discount) +- green: 11:14 fix commit `7c2e41d` (+2/−1, `router.py` only) · 58 tests · stable-green 3/3 +- gates: mypy ok · ruff ok · bandit ok · mutation 0 survivors (2 new killed) · coverage unchanged (reported) diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/review.md b/examples/cart-discounts/specs/001-cart-discount-codes/review.md new file mode 100644 index 0000000..d851eb0 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/review.md @@ -0,0 +1,28 @@ +# Adversarial Review Log — Cart discount codes + +## Round 1 — 2026-03-10 09:14 +- Critics: completeness, feasibility, testability (all three attached their procedure artifacts) +- Blocking: 3 Nits: 6 (4 killed by judge) +- Overlap: 1/5 (20 % — D ≥ 4 and m/D < 25 % → **rating capped at B+**) +- Rating: B+ (judge's uncapped grade was A: two of the three blocking findings are one-line clarifications, but the near-disjoint finding sets say the critics have not exhausted the defect pool) +- Findings applied: + 1. spec.md FR-002 — no rounding rule for percentage discounts; the PRD's open question answered it (half-up) but anvil dropped it. Added "rounded half-up to the nearest minor unit" and the 151.5 → 152 / 2.5 → 3 edge cases. (raised by completeness *and* testability — the one overlapping finding) + 2. spec.md FR-003 — "expired" undefined at the boundary instant; a critic's test skeleton could not choose `<` or `≤`. Fixed to `expires_at ≤ now` (validity strictly before), added US2 scenario 3 and the guest-cart edge case. (completeness) + 3. tasks.md T006 — no file path for the checkout hook and no gate for the concurrent double-redemption Safeguard. Added `src/shop/checkout.py` and the "exactly one Redemption row" gate. (testability) +- Findings rejected: + 1. feasibility#2 — proposed caching code lookups in Redis to meet NFR-1; rejected: the unique index on `code` satisfies NFR-1 at any plausible table size, and a cache adds an invalidation path for expiry (YAGNI). Recorded here so it is not re-raised. +- Surviving nits (not applied, noted): plan.md Phases table could name the perf test's phase; spec.md Dependencies should pin the Alembic head — both applied anyway as cheap edits. + +## Round 2 — 2026-03-10 09:58 +- Critics: completeness, feasibility, testability +- Blocking: 0 Nits: 2 (both killed — completeness asked for a rate-limit FR: scope creep, not in the PRD; testability wanted `test_perf` renamed: cosmetic) +- Overlap: 0/2 (D < 4 — signal not computed, no cap) +- Rating: A+ (clean pass 1 of 2) +- Artifacts on file: completeness re-derived Entities/Operations from Requirements alone and matched all three tables and seven operations; feasibility traced `POST /cart/{id}/discount` → lookup → clamp → cart row → response → checkout → Redemption with no unstated decision; testability drafted a skeleton per FR (5/5) without a question for the author. + +## Round 3 — 2026-03-10 10:24 +- Critics: completeness, feasibility, testability +- Blocking: 0 Nits: 0 +- Overlap: n/a (no findings) +- Rating: A++ ← exit (clean pass 2 of 2) +- Attack angles tried and failed: guest cart with a redeemed code (spec says guests skip FR-004 — consistent); subtotal changing after apply (FR-002 recomputes on read); code deleted after apply (`ON DELETE SET NULL` in plan.md Entities); percent value 0 (excluded by the `value ≥ 1` check constraint, spec Key Entities). diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/smithy-log.md b/examples/cart-discounts/specs/001-cart-discount-codes/smithy-log.md new file mode 100644 index 0000000..e687d58 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/smithy-log.md @@ -0,0 +1,15 @@ +# Smithy Log — 001-cart-discount-codes + +Host-repo integrations detected: board projection — none · phase signalling — none · drift detector — none. All skipped silently. + +2026-03-09 09:12 START: no .prd/001_*.md → run forge +2026-03-09 10:05 GATE: forge complete — .prd/001_cart-discount-codes.md — user: y (run anvil) +2026-03-09 11:48 GATE: anvil complete — specs/001-cart-discount-codes/{spec,plan,tasks}.md — user: N (halt; "smithy --resume" to continue) +2026-03-10 09:02 RESUME: detected stage=temper (triplet present, no review.md) +2026-03-10 10:31 GATE: temper complete — review.md A++ in 3 rounds — user: y (run quench) +2026-03-10 15:40 HALT: session ended mid-quench — tasks.md T001–T004 checked, T005–T006 unchecked; resume with "smithy --resume" +2026-03-11 08:15 RESUME: detected stage=quench (tasks.md has 2 unchecked items) +2026-03-11 10:22 GATE: quench complete — quench-log.md (6 tasks green, 57 tests, 0 UNTESTED FRs, CI #412) — user: y (run hone) +2026-03-11 12:30 GATE: hone complete — code-review.md A++ in 3 rounds (1 blocking finding fixed via tasks.md → test → code) — user: y (run finish step) +2026-03-11 12:45 FINISH: docs updated · atlas skipped (atlas skill not available) +2026-03-11 12:45 HANDOFF: superpowers:finishing-a-development-branch — PR "feat(cart): discount codes (001)" diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/spec.md b/examples/cart-discounts/specs/001-cart-discount-codes/spec.md new file mode 100644 index 0000000..85360f4 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/spec.md @@ -0,0 +1,99 @@ +# Feature Specification: Cart discount codes + +**Feature Branch**: `001-cart-discount-codes` +**Created**: 2026-03-09 +**Status**: Tempered A++ (review.md round 3, 2026-03-10) +**Input**: PRD `.prd/001_cart-discount-codes.md` + +## User Scenarios & Testing + +### User Story 1 - Apply a code and see the discounted total (Priority: P1) + +A shopper with items in their cart enters a code; the cart answers with subtotal, discount and total. + +**Why this priority**: it is the feature. Nothing else is observable without it. + +**Independent Test**: seed one `percent` and one `fixed` code, POST each against a cart, check the three amounts. + +**Acceptance Scenarios**: + +1. **Given** a cart with subtotal 4000 (minor units) and active code `SPRING20` (percent, 20), **When** the shopper applies `spring20`, **Then** the response is `{subtotal_minor: 4000, discount_minor: 800, total_minor: 3200, code: "SPRING20"}`. +2. **Given** a cart with subtotal 300 and active code `FIVER` (fixed, 500), **When** `FIVER` is applied, **Then** `discount_minor` is 300 and `total_minor` is 0 — never negative. +3. **Given** a cart that already carries `SPRING20`, **When** `FIVER` is applied, **Then** the cart carries only `FIVER` and the total reflects it alone. + +--- + +### User Story 2 - Unknown or expired codes are refused with a reason (Priority: P2) + +A shopper enters a code that does not exist or has expired; the cart is unchanged and the storefront gets a stable reason string. + +**Why this priority**: without it, US1 silently accepts junk; but it delivers no value until US1 exists. + +**Independent Test**: POST an unknown code and an expired code; assert status, `detail`, and that `discount_code_id` is unchanged. + +**Acceptance Scenarios**: + +1. **Given** code `XMAS25` with `expires_at` 2025-12-26T00:00:00Z and now 2026-03-10T09:00:00Z, **When** applied, **Then** `422 {"detail": "code_expired"}` and the cart is unchanged. +2. **Given** no code `NOPE` exists, **When** applied, **Then** `404 {"detail": "code_not_found"}`. +3. **Given** a code whose `expires_at` equals the current instant, **When** applied, **Then** `422 code_expired` (validity is strictly before `expires_at`). + +--- + +### User Story 3 - A code is single-use per customer (Priority: P3) + +A customer who has already checked out with a code cannot apply it again; other customers can. + +**Why this priority**: prevents abuse of US1 but only matters once checkouts with codes happen. + +**Independent Test**: complete a checkout with `SPRING20` as customer C, then apply it again as C and as D. + +**Acceptance Scenarios**: + +1. **Given** customer C completed checkout with `SPRING20`, **When** C applies `SPRING20` to a new cart, **Then** `409 {"detail": "code_already_redeemed"}`. +2. **Given** C redeemed `SPRING20`, **When** customer D applies it, **Then** `200`. +3. **Given** C has `SPRING20` applied to a cart but has not checked out, **When** C applies it to a second cart, **Then** `200` — redemption happens at checkout, not at apply. + +### Edge Cases + +- Subtotal 0 → discount 0, total 0, `200`. +- `percent` value 100 → discount equals subtotal, total 0. +- A percentage yielding a half minor unit (15 % of 1010 = 151.5) → 152; (25 % of 10 = 2.5) → 3. Half-up, never banker's. +- Code submitted in any case matches; the response echoes the stored upper-case form. +- Code expires between apply and checkout → checkout refuses with `code_expired`; the cart keeps the code so the shopper can remove it. +- Two concurrent checkouts by the same customer with the same code → exactly one Redemption row; the other checkout fails with `code_already_redeemed`. +- Guest cart (no `customer_id`) → codes apply normally; no Redemption is written and no single-use check runs. + +## Requirements + +### Functional Requirements + +- **FR-001**: System MUST accept `POST /cart/{cart_id}/discount` with body `{"code": string}` and respond `200` with integer `subtotal_minor`, `discount_minor`, `total_minor` and the canonical `code`. Applying a code to a cart that already carries one replaces it; at most one `discount_code_id` is stored per cart. +- **FR-002**: System MUST compute the discount in integer minor units from the post-sale subtotal: `percent` → `subtotal × value / 100` rounded half-up to the nearest minor unit; `fixed` → `value`. The result MUST then be clamped to `[0, subtotal]`, so `total_minor = subtotal − discount` is never negative and never exceeds the subtotal. The discount is recomputed from the stored code on every cart read and is never persisted. +- **FR-003**: System MUST refuse an unknown or inactive code with `404 {"detail": "code_not_found"}`, and a code whose `expires_at` is set and `≤ now` (UTC, timezone-aware) with `422 {"detail": "code_expired"}`. A refused apply leaves the cart unchanged. Expiry is checked at apply time and again inside checkout. +- **FR-004**: System MUST, inside the checkout transaction, write a Redemption `(code_id, customer_id, order_id, redeemed_at)` enforced unique on `(code_id, customer_id)`. An apply or checkout by a customer who already has a Redemption for that code MUST be refused with `409 {"detail": "code_already_redeemed"}`. Guest carts skip this entirely. +- **FR-005**: System MUST expose `DELETE /cart/{cart_id}/discount`, returning `204` and clearing `discount_code_id`; calling it when no code is applied is also `204`. + +### Key Entities + +- **DiscountCode**: `id`, `code` (unique, stored upper-case), `kind` ∈ {`percent`, `fixed`}, `value` (int; 1–100 for percent, ≥ 1 for fixed), `expires_at` (nullable, UTC), `active` (bool). +- **Redemption**: `id`, `code_id` → DiscountCode, `customer_id`, `order_id`, `redeemed_at`; unique `(code_id, customer_id)`. +- **Cart** (existing): gains nullable `discount_code_id` → DiscountCode. + +## Success Criteria + +- **SC-001**: For 10 000 Hypothesis-generated `(subtotal, kind, value)` cases, `0 ≤ discount ≤ subtotal` holds with no counterexample. Verified by `tests/unit/test_pricing.py::test_fr002_property_never_negative_never_exceeds` (`max_examples=10000` locally, 2000 in CI). +- **SC-002**: Applying a code to a 50-line cart completes in < 100 ms p95 over 200 requests against local Postgres. Verified by `tests/perf/test_discount_latency.py` (marked `perf`; nightly CI job). +- **SC-003**: Every FR-001…FR-005 is named by at least one passing test; quench's traceability sweep prints no `UNTESTED:` line. Verified at quench completion and recorded in `quench-log.md`. + +## Assumptions + +- Staff create codes with the existing admin CLI; no code-management API here (PRD, confirmed). +- `expires_at` is exclusive: a code is valid strictly before it (PRD, confirmed). +- Guest carts may apply codes; single-use applies only to identified customers (PRD, confirmed). +- Line-level sale prices are already reflected in `subtotal_minor`; the code applies on top (PRD, confirmed). + +## Dependencies + +- Existing `Cart` model and `POST /checkout` flow in `src/shop/checkout.py`. +- Alembic migration chain (current head `a41f9c2e`). +- Dev dependencies to add: `hypothesis`, `mutmut`, `pytest-randomly`. diff --git a/examples/cart-discounts/specs/001-cart-discount-codes/tasks.md b/examples/cart-discounts/specs/001-cart-discount-codes/tasks.md new file mode 100644 index 0000000..7358114 --- /dev/null +++ b/examples/cart-discounts/specs/001-cart-discount-codes/tasks.md @@ -0,0 +1,57 @@ +# Tasks: Cart discount codes + +**Input**: `specs/001-cart-discount-codes/{spec,plan}.md` +**Status**: all tasks complete — quench 2026-03-11, see quench-log.md + +## Format: `[ID] [P?] [Story] [Tag?] Description` + +- **[P]**: parallel-safe — different files, no dependency on another unchecked task in its phase +- **[Story]**: US1 / US2 / US3 from spec.md +- **Tags**: `[UX]` browser-facing → Playwright in quench · `[REFACTOR]` changes existing behaviour → characterization tests first · `[HARD]` genuinely tricky → quench sample-and-select +- Every task: file paths, `Satisfies:` FR(s), `Gate:` (true before it is checked off). Budget: ≤ ~400 changed lines per task. + +## Phase 1: Foundation + +- [x] T001 [US1] Add `DiscountCode` and `Redemption` models and the Alembic migration creating both tables plus `carts.discount_code_id` + - Files: `src/shop/discounts/__init__.py`, `src/shop/discounts/models.py`, `alembic/versions/20260310_discount_codes.py`, `tests/unit/test_models.py` + - Satisfies: FR-001 (stored applied code), FR-004 (Redemption uniqueness) + - Gate: migration upgrade/downgrade round-trips on an empty DB; a duplicate `(code_id, customer_id)` insert raises `IntegrityError`; `mypy --strict` clean + +## Phase 2: Core + +- [x] T002 [P] [US1] Repository: find an active code case-insensitively via the unique index; check and record redemptions + - Files: `src/shop/discounts/repository.py`, `tests/unit/test_repository.py` + - Satisfies: FR-003 (unknown/inactive → none), FR-004 (`has_redeemed`, `record_redemption`) + - Gate: `spring20` finds `SPRING20`; inactive code → none; record then `has_redeemed` → true; query plan shows the index; 0 mutation survivors +- [x] T003 [US1] [HARD] Pure discount computation: `percent` with half-up rounding, `fixed`, clamp to `[0, subtotal]`, applied to the post-sale subtotal + - Files: `src/shop/discounts/pricing.py`, `tests/unit/test_pricing.py` + - Satisfies: FR-002 + - Gate: example tests for every spec edge case (0, 100 %, 151.5 → 152, 2.5 → 3, fixed > subtotal); Hypothesis property `0 ≤ discount ≤ subtotal` green at `max_examples=10000` (SC-001); ruff banned-`float` rule clean; 0 mutation survivors + +## Phase 3: API + +- [x] T004 [US1] Router: `POST /cart/{cart_id}/discount` (apply / replace) and `DELETE /cart/{cart_id}/discount`; response recomputes amounts from the stored code + - Files: `src/shop/discounts/router.py`, `src/shop/main.py` (include router), `tests/api/test_discount_router.py`, `tests/features/cart-discount-codes.feature` (US1 steps) + - Satisfies: FR-001, FR-005 + - Gate: US1 acceptance scenarios 1–3 green end to end; DELETE idempotent (`204` twice); response model has exactly the four fields; nothing stored but `discount_code_id`; both routes refuse a foreign customer's session with `404` *(gate clause added 2026-03-11 in hone round 1 — see code-review.md; the plan's ownership Safeguard had no task gate)* +- [x] T005 [US2] Apply-time refusals: unknown/inactive → `404 code_not_found`; `expires_at ≤ now` (UTC) → `422 code_expired`; refused apply leaves the cart unchanged + - Files: `src/shop/discounts/router.py`, `tests/api/test_discount_router.py`, `tests/features/cart-discount-codes.feature` (US2 steps) + - Satisfies: FR-003 + - Gate: US2 scenarios 1–3 green including the exact-instant case; `discount_code_id` unchanged after each refusal; `detail` strings match spec.md verbatim + +## Phase 4: Checkout + +- [x] T006 [US3] Checkout hook: re-check expiry, write the Redemption inside the checkout transaction, map a unique violation to `409 code_already_redeemed`; apply-time `409` for a customer who already redeemed; guests skip + - Files: `src/shop/checkout.py`, `src/shop/discounts/router.py`, `tests/api/test_checkout_redemption.py`, `tests/features/cart-discount-codes.feature` (US3 steps) + - Satisfies: FR-003 (checkout re-check), FR-004 + - Gate: US3 scenarios 1–3 green; two concurrent checkouts yield exactly one Redemption row and one `409`; guest checkout writes no Redemption; suite stable-green 3/3 + +## FR → task map + +| FR | Tasks | +|----|-------| +| FR-001 | T001, T004 | +| FR-002 | T003 | +| FR-003 | T002, T005, T006 | +| FR-004 | T001, T002, T006 | +| FR-005 | T004 | diff --git a/install.sh b/install.sh index eb7228b..b7d0b04 100755 --- a/install.sh +++ b/install.sh @@ -30,17 +30,20 @@ ALIASES=( spdd-pipeline:smithy ) AGENTS=(bdd-scenario-writer tdd-test-generator playwright-e2e-tester fastapi-implementer labcoat) -# KEEP-class superpowers skills (DENY-class brainstorming/writing-plans/executing-plans/ -# requesting-code-review are intentionally NOT linked — forge/anvil/quench/hone replace them). +# KEEP-class superpowers skills. DENY-class skills are intentionally NOT linked: +# brainstorming/writing-plans/executing-plans/requesting-code-review overlap a stage +# (forge/anvil/quench/hone replace them), and using-superpowers/subagent-driven-development +# route back into those DENY skills. See the policy table in README.md. SUPERPOWERS_KEEP=( - systematic-debugging subagent-driven-development dispatching-parallel-agents + systematic-debugging dispatching-parallel-agents verification-before-completion receiving-code-review finishing-a-development-branch using-git-worktrees writing-skills - using-superpowers test-driven-development + test-driven-development ) SKILLS_DIR="$CONSUMER_ROOT/.claude/skills" AGENTS_DIR="$CONSUMER_ROOT/.claude/agents" +PROBLEMS=0 # counted by install (skipped/missing links) and verify (unhealthy links) err() { printf '\033[0;31m[ERR]\033[0m %s\n' "$*" >&2; } ok() { printf '\033[0;32m[OK]\033[0m %s\n' "$*"; } @@ -67,9 +70,23 @@ resolve() { # portable readlink -f: canonicalize a path, following symlinks fi } -rel_or_abs() { # rel_or_abs — relative path if possible - local target=$1 linkdir=$2 - realpath --relative-to="$linkdir" "$target" 2>/dev/null || printf '%s' "$target" +relpath() { # relpath — relative path between two absolute, physical paths. + # Pure bash: `realpath --relative-to` is GNU-only, and an absolute fallback would leave + # links that break as soon as the consumer repo is cloned somewhere else. + local target=${1%/} from=${2%/} up='' rest + [ "$target" = "$from" ] && { printf '.'; return 0; } + while :; do + case $target in + "$from"/*) rest=${target#"$from"/}; break ;; + esac + from=${from%/*} + up="../$up" + done + printf '%s%s' "$up" "$rest" +} + +physical_dir() { # physical (symlink-free) path of a directory; falls back to the given path + (cd "$1" 2>/dev/null && pwd -P) || printf '%s' "$1" } owned_by_damascus() { # true if existing path is a symlink resolving into damascus @@ -89,15 +106,21 @@ expected_skill_names() { # every name damascus ships into .claude/skills/ link() { # link local target=$1 linkpath=$2 + if [ ! -e "$target" ]; then + err "skip $(basename "$linkpath") — target missing: $target (upstream renamed or removed it?)" + PROBLEMS=$((PROBLEMS + 1)) + return 0 + fi if [ -e "$linkpath" ] || [ -L "$linkpath" ]; then if owned_by_damascus "$linkpath"; then run rm "$linkpath" else err "skip $linkpath — exists and is not a damascus-owned symlink" + PROBLEMS=$((PROBLEMS + 1)) return 0 fi fi - run ln -s "$(rel_or_abs "$target" "$(dirname "$linkpath")")" "$linkpath" + run ln -s "$(relpath "$target" "$(physical_dir "$(dirname "$linkpath")")")" "$linkpath" say_done "linked $(basename "$linkpath")" } @@ -130,7 +153,7 @@ preflight() { err "current directory is not a git repository" exit 1 fi - if [ ! -f "$DAMASCUS_ROOT/vendor/superpowers/skills/using-superpowers/SKILL.md" ]; then + if [ ! -f "$DAMASCUS_ROOT/vendor/superpowers/skills/systematic-debugging/SKILL.md" ]; then err "vendor/superpowers is empty — run inside the damascus checkout:" err " git -C '$DAMASCUS_ROOT' submodule update --init --recursive" exit 1 @@ -170,6 +193,10 @@ install() { info "pruning stale damascus-owned links" prune + if [ "$PROBLEMS" -gt 0 ]; then + err "$PROBLEMS link(s) could not be installed (see [ERR] lines above)" + exit 1 + fi say_done "damascus installed into $SKILLS_DIR and $AGENTS_DIR" info "entrypoint: invoke the 'smithy' skill (or 'forge' to start a PRD)" } @@ -185,8 +212,6 @@ uninstall() { # remove EVERY damascus-owned link, including stale names say_done "damascus symlinks removed" } -PROBLEMS=0 - check_link() { # check_link local linkpath=$1 target=$2 if [ ! -L "$linkpath" ]; then @@ -265,16 +290,23 @@ Run from the CONSUMER repo root (not from inside damascus). EOF } -MODE=install -for arg in "$@"; do - case $arg in - --install) MODE=install ;; - --uninstall) MODE=uninstall ;; - --verify) MODE=verify ;; - --dry-run) DRY_RUN=1 ;; - -h|--help) usage; exit 0 ;; - *) err "unknown flag: $arg (use --install, --uninstall, --verify, --dry-run)"; exit 1 ;; - esac -done +main() { + local mode=install arg + for arg in "$@"; do + case $arg in + --install) mode=install ;; + --uninstall) mode=uninstall ;; + --verify) mode=verify ;; + --dry-run) DRY_RUN=1 ;; + -h|--help) usage; exit 0 ;; + *) err "unknown flag: $arg (use --install, --uninstall, --verify, --dry-run)"; exit 1 ;; + esac + done + "$mode" +} -"$MODE" +# When sourced (CI reads the STAGE_SKILLS/ALIASES/AGENTS/SUPERPOWERS_KEEP manifests +# from here so they cannot drift from the script), define everything and do nothing. +if [ "${BASH_SOURCE[0]}" = "$0" ]; then + main "$@" +fi diff --git a/skills/anvil/SKILL.md b/skills/anvil/SKILL.md index ebe69e0..de95a7e 100644 --- a/skills/anvil/SKILL.md +++ b/skills/anvil/SKILL.md @@ -87,10 +87,10 @@ vendor/spec-kit/templates/plan-template.md → basis for plan.md vendor/spec-kit/templates/tasks-template.md → basis for tasks.md ``` -The templates live in the damascus checkout. If the skill is installed as a symlink, resolve the real path first: +The templates live in the damascus checkout. If the skill is installed as a symlink, resolve the real path first (portable — no `readlink -f`, which stock macOS lacks): ```bash -DAMASCUS="$(dirname "$(readlink -f .claude/skills/anvil)")/../.." +DAMASCUS="$(cd "$(cd .claude/skills/anvil && pwd -P)/../.." && pwd)" ls "$DAMASCUS/vendor/spec-kit/templates/" ``` diff --git a/skills/quench/SKILL.md b/skills/quench/SKILL.md index 0104663..cb84933 100644 --- a/skills/quench/SKILL.md +++ b/skills/quench/SKILL.md @@ -122,17 +122,17 @@ The log is the pipeline's flight recorder: it proves amber happened (the failure ## Agent Dispatch Table -Quench composes with the agents that ship in this repo's `agents/` directory. Dispatch as follows: +Quench composes with the agents that ship in this repo's `agents/` directory. The shipped agents are shaped for a **Python / pytest / FastAPI** host with Playwright for the browser; the *cycle* is stack-agnostic. Dispatch as follows: | Task type | Agent | |-----------|-------| | Gherkin scenarios | `bdd-scenario-writer` | -| Failing pytest tests | `tdd-test-generator` | +| Failing unit/integration tests | `tdd-test-generator` (pytest); another stack → a test-writer subagent bound by the same contract: owns red and amber, records amber's failure message, never writes implementation | | Playwright `.spec.ts` | `playwright-e2e-tester` | -| Implementation (FastAPI) | `fastapi-implementer` | +| Implementation | `fastapi-implementer` (FastAPI); another stack → an implementer subagent bound by the same contract: enters at amber, minimal code to green, never edits tests | | Test coverage / mocks | `labcoat` | -Don't dispatch agents you don't have a task for. +For a non-Python host, dispatch a general subagent with the corresponding agent file's contract sections (cycle, freeze, gates) pasted into its prompt — the contract is what quench depends on, not the framework. Don't dispatch agents you don't have a task for. ## Don't do this diff --git a/skills/smithy/SKILL.md b/skills/smithy/SKILL.md index 682dba6..02b9ec3 100644 --- a/skills/smithy/SKILL.md +++ b/skills/smithy/SKILL.md @@ -1,6 +1,7 @@ --- name: smithy description: Use when running a feature end-to-end through the SPDD pipeline (forge → anvil → temper → quench → hone) with state-machine resumption. The smithy houses all five stages and decides which one to run based on disk artifacts. Wrap a feature in one invocation; halt at every gate. +effort: medium --- # Smithy — End-to-End SPDD Orchestrator @@ -11,7 +12,7 @@ The smithy houses the forge, anvil, temper, quench, and hone. Smithy is the orch **Announce at start:** "I'm using the smithy skill to orchestrate the full SPDD pipeline." -**State lives in:** the disk artifacts themselves. Smithy is stateless — it reads `.prd/`, `specs/`, and `review.md` to determine the current stage. +**State lives in:** the disk artifacts themselves. Smithy is stateless — it reads `.prd/`, `specs/NNN-/` (`review.md`, `tasks.md`, `quench-log.md`, `code-review.md`, `smithy-log.md`) to determine the current stage. Every row of the state machine is decidable from those files alone; nothing depends on CI status or a memory of what happened last session. ## When to use @@ -42,13 +43,17 @@ Smithy reads disk and decides which stage to invoke: │ review.md, last < A++ │ temper iterating│ continue temper │ │ review.md, A++ │ temper done │ run quench │ │ tasks.md has unchecked items│ quench in progress │ continue quench │ -│ all tasks checked, CI green │ quench done │ run hone │ +│ all tasks checked, every │ quench done │ run hone │ +│ task has a quench-log green│ │ │ │ code-review.md, last < A++ │ hone iterating │ continue hone │ -│ code-review.md ends A++ │ hone done │ run finish step │ -│ README/CHANGELOG updated │ finish done │ hand off to merge │ +│ code-review.md ends A++, │ hone done │ run finish step │ +│ no FINISH in smithy-log.md │ │ │ +│ smithy-log.md has FINISH │ finish done │ hand off to merge │ └─────────────────────────────┴─────────────────┴─────────────────────┘ ``` +"Quench done" means `tasks.md` has no unchecked task **and** `quench-log.md` has a green entry (with gate results) for every task — that log is quench's own record that CI/gates passed, so smithy never needs to query CI. "Finish done" means `smithy-log.md` carries a `FINISH:` line (see the finish step), which is the only artifact the finish step is guaranteed to leave behind — README/CHANGELOG edits are not detectable on their own. + ## Gates (halt + ask user) Smithy **always** halts at these moments: @@ -72,6 +77,7 @@ Each halt prints: | `--start-from ` | Begin at this stage, assume earlier ones done (or N/A) | "OVERRIDE: started from " | | `--resume` | Read disk state and continue from current stage | "RESUME: detected stage=" | | `--dry-run` | Print plan; make no changes | (no log entry) | +| (finish step) | Docs updated, Atlas run or skipped | "FINISH: docs updated · atlas " | `smithy-log.md` lives at `specs/NNN-/smithy-log.md` and is append-only. @@ -116,6 +122,9 @@ handing off to `superpowers:finishing-a-development-branch`, smithy runs one fin sync. Atlas is not vendored by Damascus; this step is a no-op wherever the consumer has not installed it. +3. **Log it.** Append `FINISH: docs updated · atlas ` + to `smithy-log.md`. This line is how a later `smithy --resume` knows the finish step ran. + This step runs **after** the hone gate signoff and is the last thing smithy does before the merge handoff. It adds no board sync of its own (see the single-call-site rule above). diff --git a/tests/smoke.sh b/tests/smoke.sh index 91a3c4d..ada9057 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -41,8 +41,18 @@ for agent in bdd-scenario-writer tdd-test-generator playwright-e2e-tester \ [ -e ".claude/agents/$agent.md" ] || fail "agents/$agent.md is a broken link" done [ -L ".claude/skills/systematic-debugging" ] || fail "superpowers KEEP skills not linked" -[ -e ".claude/skills/brainstorming" ] && fail "DENY-class superpowers skill was linked" -[ -e ".claude/skills/requesting-code-review" ] && fail "DENY-class superpowers skill was linked (hone replaces it)" +for deny in brainstorming writing-plans executing-plans requesting-code-review \ + using-superpowers subagent-driven-development; do + [ -e ".claude/skills/$deny" ] && fail "DENY-class superpowers skill was linked: $deny" +done + +# every damascus-owned link is relative: a committed .claude/ must survive a fresh clone +for l in .claude/skills/* .claude/agents/*.md; do + [ -L "$l" ] && [ "$l" != .claude/skills/foreign-link ] || continue + case "$(readlink "$l")" in + /*) fail "$l is an absolute symlink" ;; + esac +done # ownership guarantee [ "$(cat .claude/skills/my-own-skill)" = keep ] || fail "consumer file modified" @@ -73,6 +83,14 @@ damascus --verify >/dev/null 2>&1 && fail "--verify passed with a missing link" damascus damascus --verify || fail "--verify failed after repair" +# --- a non-damascus path in the way is skipped, reported, and fails the run --- +mkdir -p .claude/skills/hone-shadow && mv .claude/skills/hone .claude/skills/hone-shadow/ \ + && mkdir .claude/skills/hone && echo mine > .claude/skills/hone/SKILL.md +damascus >/dev/null 2>&1 && fail "install exited 0 despite a skipped link" +[ "$(cat .claude/skills/hone/SKILL.md)" = mine ] || fail "install touched a consumer-owned directory" +rm -r .claude/skills/hone && mv .claude/skills/hone-shadow/hone .claude/skills/ && rmdir .claude/skills/hone-shadow +damascus --verify || fail "--verify failed after restoring the shadowed link" + # --- uninstall removes everything owned, nothing else --- damascus --uninstall remaining="$(find .claude/skills -mindepth 1 -maxdepth 1 -exec basename {} \; | sort | xargs)" diff --git a/vendor/spec-kit b/vendor/spec-kit index 51e52be..a4e25ce 160000 --- a/vendor/spec-kit +++ b/vendor/spec-kit @@ -1 +1 @@ -Subproject commit 51e52be6c3b26fed3ff5424c671f4a559519a759 +Subproject commit a4e25ce6b96dc8e85f84206c6a54353fa9c5260b From 35a73f8dd302cab7ec23690da9bf4e11aa6cf2a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 06:12:19 +0000 Subject: [PATCH 2/3] Keep the changelog under [Unreleased]; the version bump is the release PR Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HhCxRx3NtQvWL2nYu2GuYD --- CHANGELOG.md | 4 +--- docs/production-readiness.md | 11 ++++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6675e41..450969f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,7 @@ All notable changes to this project are documented here, following [Common Chang ## [Unreleased] -## [0.1.0] - 2026-09-09 - -First release. Everything below is new; there is no earlier version to change or fix against. +Everything below is new; there is no earlier release to change or fix against. This section becomes `[0.1.0]` in the release PR (see CLAUDE.md, Releasing). ### Added diff --git a/docs/production-readiness.md b/docs/production-readiness.md index b1051ac..a5e70a2 100644 --- a/docs/production-readiness.md +++ b/docs/production-readiness.md @@ -21,8 +21,9 @@ unprompted, and `install.sh` hard-fails on stock macOS bash 3.2 (`declare -A`). > **Status:** phases 0–5 are implemented in-repo (checked boxes below). Two items > remain manual, GitHub-side, after this lands on `master`: -> **(1)** push the `v0.1.0` tag — the changelog section already exists and the -> `Release` workflow publishes the GitHub Release from it; +> **(1)** cut `v0.1.0`: one PR that moves `[Unreleased]` under `## [0.1.0] - `, +> then push the tag — the `Release` workflow publishes the GitHub Release from +> that section; > **(2)** enable branch protection on `master` (require PR + green CI, no > force-push, no required approver count) in repo Settings → Branches. @@ -80,9 +81,9 @@ Consumers add damascus as a submodule — today they can only pin an opaque SHA - [x] Tag **`v0.1.0`** and cut the first GitHub Release from the `[Unreleased]` changelog section; adopt semver (breaking skill-contract or install.sh - behavior changes = major once past 1.0). *(Changelog section cut and a - tag-triggered release workflow added; the tag push itself is the one - remaining manual step.)* + behavior changes = major once past 1.0). *(Tag-triggered release workflow + added; the version-bump PR and the tag push are the remaining manual + steps.)* - [x] Document the release ritual in CLAUDE.md (move `[Unreleased]` → version heading, tag) — `.github/workflows/release.yml` generates the release notes from the changelog on tag push. From 80d71a1a1de504f58408496c9d3c541498952dc5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 06:18:07 +0000 Subject: [PATCH 3/3] smoke: split the foreign-link guard so shellcheck SC2015 stays quiet Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HhCxRx3NtQvWL2nYu2GuYD --- tests/smoke.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/smoke.sh b/tests/smoke.sh index ada9057..69b882a 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -48,7 +48,8 @@ done # every damascus-owned link is relative: a committed .claude/ must survive a fresh clone for l in .claude/skills/* .claude/agents/*.md; do - [ -L "$l" ] && [ "$l" != .claude/skills/foreign-link ] || continue + [ -L "$l" ] || continue + [ "$l" != .claude/skills/foreign-link ] || continue # the planted foreign fixture is absolute by design case "$(readlink "$l")" in /*) fail "$l is an absolute symlink" ;; esac