From 0ee45fb8817d52c03da623ae1cb03fab0faed73a Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:37:04 +0200 Subject: [PATCH 1/2] Re-sync agentic workflow guide + add the NetPace delta ("sidekick") MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs/agentic-workflow.md — the generic, stack-portable guide, refreshed to the current version and kept stack-neutral. - docs/agentic-workflow-NetPace.md — the NetPace delta ("sidekick"), recording where NetPace's concrete harness differs from the generic guide: CI + agent-review action (Review B), the NuGet + cross-platform release pipeline, Change-Intent-Records + memory as the decision ledger, the enforcement gates (green-gate, no-skipped, traceability, format-on-commit, PR pre-flight), and console-output snapshot verification. Flags one unreconciled divergence: format-on-commit vs the guide's ship-cadence recommendation. - Delete docs/agentic-software-development-workflow.md — superseded by the two above. Both docs stand alone: all framing is relative to the generic guide only, with no reference to any external/sibling repository. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01T6dd1XBNhEq42D6EsY6nPz --- docs/agentic-software-development-workflow.md | 250 ------------------ docs/agentic-workflow-NetPace.md | 88 ++++++ docs/agentic-workflow.md | 235 ++++++++++++++++ 3 files changed, 323 insertions(+), 250 deletions(-) delete mode 100644 docs/agentic-software-development-workflow.md create mode 100644 docs/agentic-workflow-NetPace.md create mode 100644 docs/agentic-workflow.md diff --git a/docs/agentic-software-development-workflow.md b/docs/agentic-software-development-workflow.md deleted file mode 100644 index e168b15..0000000 --- a/docs/agentic-software-development-workflow.md +++ /dev/null @@ -1,250 +0,0 @@ -# Agentic Software Development Workflow - -## Introduction - -Writing a spec before touching code, locking a test plan before writing a test, and -enforcing both mechanically — that is the discipline this workflow encodes. The result is a -harness: Claude Code constrained by context, feedback loops, and automated quality gates so -that the agent does the work and the engineer reviews it. - -**The workflow in one line:** write a rigorous spec → generate a test plan → review it → -generate tasks → implement → verify test coverage → PR review → merge. - -The one-time project setup is already done. See the [Appendix](#appendix--codebase-setup) -for the files that make the workflow run. - -*Inspired by:* -- [Harness Engineering](https://openai.com/index/harness-engineering/) — OpenAI, 2026. -- [Effective harnesses for long-running agents](https://www.anthropic.com/engineering/effective-harnesses-for-long-running-agents) - Anthropic, 2025. - - ---- - -## Workflow Execution Order - -### Per-feature — Spec & planning (on main branch) -1. `/speckit.draftissue` ← optional; turn an unstructured brief into a well-formed - GitHub issue before review -2. `/speckit.reviewissue` ← pre-specification gate; posts gaps + recommendations as an - issue comment. Re-run to expand any question the author - hedged on (`not sure`, `more options`, etc.) — the same - comment is edited in place -3. `/speckit.confirmissue` ← fold answered review comment into a `## Confirmed decisions` - section on the issue body, so spec consumes decisions, not - deliberation -4. `/speckit.specify` -5. `/speckit.clarify` ← iterate until spec feels complete -6. `/speckit.checklist` ← resolve all gaps before continuing -7. `/speckit.plan` -8. `/speckit.testplan` ← review output carefully before continuing -9. `powershell -ExecutionPolicy Bypass -File scripts\git-red-phase-commit.ps1` -10. `/speckit.tasks` -11. `/speckit.analyze` ← resolve HIGH/CRITICAL before branching; auto-runs - `/speckit.analyze.testplan` via the `after_analyze` hook, - appending a test-plan cross-check to the analyze report - -### Per-feature — Implementation -12. `/speckit.implement` ← agent runs to suite-green - -### Per-feature — Pre-PR -13. `/speckit.testchecklist` ← resolve CRITICAL before continuing -14. `/pr-review-toolkit:review-pr all` ← re-run dotnet test after simplifier -15. `/review-slop` - -### Per-feature — PR -16. `/raise-pr` -17. `/capture-learnings` ← optional; run after PR is raised - -### Periodic (not per-feature) -∞ `/audit-deadcode` ← run every few features or before a release -∞ `/context-gardening` ← run quarterly or after a big architectural shift - - ---- - -## Separation of Concerns - -- **Spec (what & why):** requirements as normative SHALL/MUST statements. No test scenarios. -- **Test plan (how you verify):** named scenarios derived from the spec. Generated after the - spec is complete, before tasks are decomposed. -- **Tasks (how you build):** implementation breakdown informed by the test plan, so the work - reflects the full verification surface. -- **Test checklist (did you honour it):** static analysis after implementation confirming - every scenario has an honest test. - - -## Design Principles - -- **Single branch per feature.** Tests and implementation on the same branch. Commit history - is the audit trail. -- **test-plan.md is the red-phase baseline.** In a statically-typed C# project, - pre-implementation test files cannot compile without the classes they reference. - test-plan.md committed on the feature branch serves as the locked intent. The test - checklist enforces honesty. -- **PR review as the integrity gate.** Reviewer diffs test-plan.md and checks the - testchecklist report, not a binary test pass/fail. - - -## Documentation Hierarchy - -- Tier 1: GOVERNANCE → `.specify/memory/constitution.md` (principles) -- Tier 2: IMPLEMENTATION → `CLAUDE.md` (essential patterns + signposts) -- Tier 3: DETAILED REFERENCES → `docs/conventions/*` (deep guidance) - - ---- - -## Appendix — Codebase Setup - -The following files were added to this repo to make the SDD workflow operational. They are -listed here as a reference — the setup is already complete. - -### Claude Code configuration (`.claude/`) - -**[`.claude/settings.json`](.claude/settings.json)** -Permissions allowlist/denylist and two inline hooks: -- *Format on commit* — runs `dotnet format style` and `dotnet format whitespace` on staged - `.cs` files before any `git commit` Claude Code issues -- *Build and test before PR* — runs `dotnet build` + `dotnet test` before any - `gh pr create` command executes, blocking PR creation if either fails -- Enables the `pr-review-toolkit` plugin - -**[`.claude/commands/speckit.draftissue.md`](.claude/commands/speckit.draftissue.md)** -Custom `/speckit.draftissue` command. Pre-issue gate that sits *before* -`/speckit.reviewissue`. Takes an unstructured feature brief, grounds it in the codebase, -surfaces the ~5–10 decisions the brief leaves open (with concrete leans), iterates with the -user to lock them, then writes a structured issue body to a transient file and posts it via -`gh issue create`. Output is an issue with substantive scope, acceptance criteria, and an -explicit out-of-scope list — the raw material `/speckit.reviewissue` needs to do useful -cross-checking. - -**[`.claude/commands/speckit.reviewissue.md`](.claude/commands/speckit.reviewissue.md)** -Custom `/speckit.reviewissue` command. Pre-specification gate that sits *before* -`/speckit.specify`. Reads an unrefined GitHub issue, cross-references it against the current -codebase (architecture, existing services, test data, docs), and surfaces ambiguities in -scope and undefined semantics (matching rules, thresholds, field lists) that would otherwise -block or distort a specification run. Each gap ends with a concrete `**Recommendation:**` -the author can accept or redirect. The posted comment carries a `` -marker; re-runs **edit the same comment in place** to expand any question where the author -hedged (`not sure`, `more options`, `idk`, etc.) — substantive answers are left untouched -for `/speckit.confirmissue` to pick up. - -**[`.claude/commands/speckit.confirmissue.md`](.claude/commands/speckit.confirmissue.md)** -Custom `/speckit.confirmissue` command. Sits *between* `/speckit.reviewissue` and -`/speckit.specify`. Reads the answered review comment, pairs each gap's recommendation with -the author's inline answer (accepted / accepted-with-rider / redirected / out-of-scope), -and appends a `## Confirmed decisions` section to the issue body so the spec author -consumes resolved decisions rather than re-reading deliberation. Stops if any answer is -empty or still hedging. The original review comment is left intact as the audit trail. - -**[`.claude/commands/speckit.testplan.md`](.claude/commands/speckit.testplan.md)** -Custom `/speckit.testplan` command. Translates completed spec.md requirements into named -WHEN/THEN test scenarios and writes them to `.specify/specs//test-plan.md`. -Includes pre-generation quality checks and a post-generation coverage summary table. - -**[`.claude/commands/speckit.testchecklist.md`](.claude/commands/speckit.testchecklist.md)** -Custom `/speckit.testchecklist` command. Static analysis (no test execution) that verifies -every scenario in test-plan.md has an honest, non-trivially-passing test with a matching -`// SCENARIO:` comment. Outputs a structured report with CRITICAL / HIGH / WARNING findings. - -**[`.claude/commands/review-slop.md`](.claude/commands/review-slop.md)** -Custom `/review-slop` command. Diffs the current branch against main and flags AI-generated -code patterns that compile and pass lint but degrade the codebase. - -**[`.claude/commands/audit-deadcode.md`](.claude/commands/audit-deadcode.md)** -Custom `/audit-deadcode` command. Walks the call graph from CLI entry points and produces -a `DEADCODE.md` report of unused code. - -**[`.claude/commands/context-gardening.md`](.claude/commands/context-gardening.md)** -Custom `/context-gardening` command. Audits `CLAUDE.md`, its linked docs, and -`.claude/memory/` against the patterns from "A good AGENTS.md is a model upgrade" — flags -bloat, stale references, mis-fit patterns, and verbose passages, then applies approved -edits. Advisory: candidates are surfaced with rationale; the user picks which to apply. -Run quarterly, after a big architectural shift, or whenever `CLAUDE.md` feels stale. - -**[`.claude/commands/raise-pr.md`](.claude/commands/raise-pr.md)** -Custom `/raise-pr` command. Reads the branch name, commit history, and changed files to -auto-generate a PR title and body (using `.github/pull_request_template.md`), then runs -`gh pr create`. - -**[`.claude/commands/capture-learnings.md`](.claude/commands/capture-learnings.md)** -Custom `/capture-learnings` command. Scans the current conversation (and, on a feature -branch, commits since the branch point as secondary corroboration) for corrections, -decisions, and gotchas, then surfaces candidates for the user to approve and persist as -memory entries. - -### spec-kit configuration (`.specify/`) - -**[`.specify/memory/constitution.md`](.specify/memory/constitution.md)** -Project governance: TDD as non-negotiable, library-first architecture, CLI excellence, -cross-platform compatibility, code quality standards, and semantic versioning. Supersedes -all other guides. Versioned; amendments require documented rationale. - -**[`.specify/extensions.yml`](.specify/extensions.yml)** + -**[`.specify/extensions/.registry`](.specify/extensions/.registry)** -Spec-kit extension configuration and registry. `extensions.yml` declares lifecycle hooks -(before_*/after_* phases) bound to extension commands; `.registry` tracks installed -extensions with version, priority, and registered commands. - -**[`.specify/extensions/git/`](.specify/extensions/git/)** -Git integration extension for spec-kit. Hooks into the spec-kit lifecycle to auto-commit -before and after each spec-kit command (e.g. commit after `/speckit.specify`, commit before -`/speckit.clarify`). Also provides `/speckit.git.feature` to create feature branches and -`/speckit.git.initialize` for project setup. Scripts provided in both Bash and PowerShell. - -**[`.specify/extensions/testplan/`](.specify/extensions/testplan/)** -Test-plan cross-check extension. Provides `/speckit.analyze.testplan`, which runs -automatically as an `after_analyze` hook. It cross-checks `test-plan.md` scenarios against -`spec.md` and `tasks.md` for the current feature and appends a *Test Plan Cross-Check* -findings table to the analyze report. - -### GitHub integration (`.github/`) - -**[`.github/workflows/dotnet.yml`](.github/workflows/dotnet.yml)** -CI: runs `dotnet build` and `dotnet test` on every PR targeting `main`. PRs cannot merge -without this passing. - -**[`.github/workflows/claude.yml`](.github/workflows/claude.yml)** -Claude Code GitHub Action. Triggers on: -- PR opened **by `claude-code[bot]`** — automatically runs a PR review -- `@claude` mentioned in any PR/issue comment, review, or issue body — responds inline - -**[`.github/pull_request_template.md`](.github/pull_request_template.md)** -Default PR body template with sections for: Summary, Spec (link to -`.specify/specs//`), Changed Files, and New Artifacts (spec folders and CIR -files). Pre-filled automatically by `/raise-pr`. - -### Conventions - -**[`docs/conventions/csharp-style.md`](docs/conventions/csharp-style.md)** -Detailed C# style reference: field naming, file-scoped namespaces, ConfigureAwait patterns, -collection expressions, brace style, member ordering. - -**[`docs/conventions/change-intent-records.md`](docs/conventions/change-intent-records.md)** -When and how to write a Change Intent Record (CIR) — for decisions involving viable -alternatives, constraint workarounds, or anything that affects future work. -CIRs live in `docs/change-intent-records/`. - - ---- - -## Claude Token Management Plugins - -### read-once - -https://github.com/Bande-a-Bonnot/Boucle-framework/blob/main/tools/read-once/README.md - -`C:\Users\frank\.claude\read-once\read-once.ps1 stats` -`~/.claude/read-once/read-once stats` - -### context-mode - -https://github.com/mksglu/context-mode - -`/context-mode:ctx-stats` - -### rkt - -https://github.com/rtk-ai/rtk - -`rtk gain` diff --git a/docs/agentic-workflow-NetPace.md b/docs/agentic-workflow-NetPace.md new file mode 100644 index 0000000..05e763d --- /dev/null +++ b/docs/agentic-workflow-NetPace.md @@ -0,0 +1,88 @@ +# Agentic Workflow — the NetPace delta + +Companion to [agentic-workflow.md](agentic-workflow.md), the generic, stack-portable guide. This file records **only where NetPace differs** from it, or makes a generic instruction concrete. If a practice is not listed here, NetPace follows the generic guide as written. + +## Why a generic guide plus this delta + +[agentic-workflow.md](agentic-workflow.md) is kept deliberately **stack-neutral** — the portable principles, with no NetPace specifics — so it stays reusable and easy to keep current. This file is where NetPace's concrete implementation, and any deviations from the guide, live. + +The rule that keeps the split honest: **NetPace-specific behaviour never edits the generic guide** — it belongs here. When the guide's principles themselves change, that diff belongs in `agentic-workflow.md`; only NetPace's concrete reading of them belongs in this delta. + +## Platform + +- **Cross-platform, developed on Windows + WSL.** NetPace targets `win`/`linux`/`osx` (`x64`/`arm64`) and is developed on Windows with a WSL sandbox for the agent (see [wsl-claude-sandbox.md](wsl-claude-sandbox.md)). +- **Both `sh` and PowerShell script variants are kept.** NetPace retains the spec-kit `.ps1` variants alongside the `.sh` ones. Spec-kit is initialised `--script sh`; the PowerShell copies stay for Windows-native use. +- **AOT-trimmable.** Production code must stay trim/AOT-safe (reflection-heavy APIs like `Spectre.Console.Cli` were deliberately replaced). A code constraint rather than a workflow one, but it shapes what "implement" may reach for. + +## CI + +The generic guide's "CI on PR" step **applies fully**; NetPace realises the whole `.github/workflows/` set the guide's Appendix describes: + +| Workflow | Trigger | Role | +|---|---|---| +| `dotnet.yml` — Build and Test | pull_request → main | the generic **CI-on-PR** gate: build + test every PR | +| `codeql.yml` — CodeQL | push/PR/weekly | security analysis (the supply-chain-hardening line — see the CIR) | +| `claude.yml` — Claude Code | `@claude` in an issue/PR comment (author-gated) | the generic **Agent review action** — this is **Review B** | +| `publish-nuget.yml` | tag push | publish `NetPace.Core` to NuGet | +| `release-binaries.yml` | tag push | cross-platform binary release matrix | + +**Review B is live:** the `@claude` action posts on the raised PR. `/ship` requests it via `/raise-pr` and never waits on it; a human reads it at merge, and `capture-learnings` can fold it in later. + +## Release pipeline + +NetPace ships: `NetPace.Core` as a **NuGet package**, and cross-platform **binaries** (6 RIDs × self-contained/framework-dependent) on tag push. The generic guide has no release step; for NetPace it is first-class. + +The contract — release matrix, runner-per-RID rationale, naming convention, smoke-test and size-assertion contracts — lives in [RELEASING.md](RELEASING.md); touching `release-binaries.yml` (or any release-pipeline scope) without updating it is a documented no-no (memory: `feedback_release_pipeline_doc`). Per-release "what changed" notes are GitHub-auto-generated from merged PRs — there is no `CHANGELOG.md` to maintain. + +## Decision ledger: Change-Intent-Records **and** memory + +Where the generic guide offers "Change-Intent-Records (or an equivalent decision ledger)", NetPace uses **both, for different jobs**: + +- **Change-Intent-Records** — [`docs/change-intent-records/`](change-intent-records/), dated `YYYY-MM-DD-slug.md` files, are the human-facing record of *why* a non-obvious change was made (the AOT release shape, the profile CLI switch, the speckit-file guard, supply-chain hardening). When to write one is governed by [`docs/conventions/change-intent-records.md`](conventions/change-intent-records.md). +- **Memory** — [`.claude/memory/`](../.claude/memory/), indexed by `MEMORY.md` and loaded via `CLAUDE.md`, holds the agent-facing facts and corrections (one fact per file). The generic guide's "prefer a gate to a memory entry" rule is live: several memories exist only as the *rationale* for a gate that now enforces them (`feedback_dotnet_test_no_build` → `green-gate.sh`; the skip ban → `no-skipped-tests.sh`). + +## The gates, concretely + +The generic enforcement layer, as NetPace wires it. Hooks live in [`.claude/hooks/`](../.claude/hooks/), wired in `.claude/settings.json`, each with a `.tests.sh` case matrix beside it. Documented per-hook in [`.claude/hooks/README.md`](../.claude/hooks/README.md). + +| Generic gate | NetPace implementation | Event | +|---|---|---| +| Stale-build guard | `green-gate.sh` — denies `dotnet test --no-build` when a `*.cs` under `src/` is newer than the built assembly | PreToolUse(Bash) | +| No skipped tests | `no-skipped-tests.sh` — blocks commits reintroducing the skip family (incl. xUnit-v3 `SkipUnless=`/`SkipWhen=`); `--check` mode for CI | PreToolUse(Bash) | +| Traceability gate | `traceability-gate.sh` — spec label ↔ test-plan scenario ↔ `// SCENARIO:` marker under `src/`, exact match; loop-guarded nudge, never a lock-out | Stop | +| Upstream-file guard | `permissions.deny` (Edit/Write/MultiEdit) on `.claude/skills/speckit-*/SKILL.md`, `.specify/templates/*.md`, `.specify/scripts/bash/*.sh` | settings | +| Format-on-commit | `dotnet format style/whitespace` on staged `*.cs` (see divergence below) | PreToolUse(Bash), `if git commit` | +| PR pre-flight | `dotnet build ./src && dotnet test ./src` before `gh pr create` | PreToolUse(Bash), `if gh pr create` | +| **Test-green gate** | **`/ship` step 1 — a real `dotnet build ./src && dotnet test ./src`. Not a hook.** | — | + +Every hook is **fail-open with an announced override** (`NETPACE_SKIP_GREEN_GATE=1`, `NETPACE_ALLOW_SKIPS=1`, `NETPACE_SKIP_TRACEABILITY_GATE=1`). For a harness edited with itself, a false block can lock out the tools that would fix it — so uncertain paths allow, and the override announces itself on stderr. + +**Two generic gates do not apply here.** NetPace has **no stack-guard** — there is no external service stack to orchestrate — and **no UI-automation denylist**: it is a console CLI, not a browser UI, so a denylist has nothing to guard. NetPace's console output *is* verified — see below — just not by a browser-automation framework. + +### Divergence to reconcile — format-on-commit + +NetPace formats on commit (the `dotnet format` hook above). The generic guide's *Formatting is not verification — do it at ship cadence* section argues the opposite: per-commit formatting taxes every commit to fix something no reviewer would catch, and the step belongs in `/ship`. This is an **unreconciled divergence** — either move formatting into the ship flow, or record here why per-commit is deliberately kept. Flagged, not silently retained. + +## `/ship` + +NetPace's `/ship` follows the generic *ship gate* section as written: + +- **Always runs the suite.** Step 1 is `dotnet build ./src && dotnet test ./src` — no docs-only skip. The suite is fast (no external stack), and the `gh pr create` pre-flight hook would re-run it anyway, so a skip would save nothing. +- **Review B posts.** Because `claude.yml` is wired, the async `@claude` review the generic flow describes actually appears on the PR. `/ship` still never waits on it. + +## Test-green gate & categories + +- The completion gate is the real suite run inside `/ship` (above), backed belt-and-braces by the `gh pr create` pre-flight hook — both are `dotnet build ./src && dotnet test ./src`. There is no ledger/Stop-hook proxy (the shape the generic guide's *Where the completion gate belongs* section warns against). +- **Fast/slow split.** Real-network integration tests live in a **separate test category**, excluded from the default run, so the inner loop stays seconds-fast; the whole (default) suite is the completion gate. +- **Console output is verified by snapshot.** `NetPace.Console.Tests` uses `Spectre.Console.Testing` with `Expectations/*.verified.txt` snapshots — that is how a CLI covers the generic guide's *verify* duty for rendered output. Check the `*.verified.txt` before reporting an output mode as untested (memory: `feedback_console_output_snapshot_coverage`). + +## Spec-kit + +Pinned at **0.12.10**, initialised `--script sh`. A `--force` re-init resets every stock skill's `disable-model-invocation` flag to `false` — the flip must be re-applied after any upgrade, and the upstream-file guard above exists to stop that regression recurring (CIR: `2026-07-10-guard-speckit-files`; memory: `speckit_upgrade_procedure`). NetPace's custom `speckit.*` commands (`draftissue`, `reviewissue`, `confirmissue`, `testplan`, `testchecklist`) are authored here, not stock — an upgrade does not touch them; the guarded files are the hyphenated `speckit-*` skills. + +## Related + +- [../.specify/memory/constitution.md](../.specify/memory/constitution.md) — governance; supersedes this file and the generic guide alike. +- [RELEASING.md](RELEASING.md) — the release matrix and its contracts. +- [conventions/change-intent-records.md](conventions/change-intent-records.md) — when a change warrants a CIR; [conventions/csharp-style.md](conventions/csharp-style.md) — C# style. +- [../.claude/hooks/README.md](../.claude/hooks/README.md) — per-hook documentation. diff --git a/docs/agentic-workflow.md b/docs/agentic-workflow.md new file mode 100644 index 0000000..775d11f --- /dev/null +++ b/docs/agentic-workflow.md @@ -0,0 +1,235 @@ + + +# Agentic Software Development Workflow + +## Introduction + +Writing a spec before touching code, locking a test plan before writing a test, and **enforcing both mechanically** — that is the discipline this workflow encodes. The result is a *harness*: an agent (Claude Code, Codex CLI, …) constrained by context, feedback loops, and automated quality gates so that the agent does the work and the engineer reviews it. + +**The workflow in one line:** write a rigorous spec → generate a test plan → review it → generate tasks → implement → verify test coverage → PR review → merge. + +This document is **stack-generic**. A concrete project implements it by adding the files in the [Appendix](#appendix--codebase-setup) and recording any project-specific deviations in a short companion "implementation delta" doc — keeping this guide as the single shared source of truth across repos. + +*Inspired by:* +- [Coding with AI](https://www.chrismdp.com/coding-with-ai/) — Chris Parsons, 2026. +- [Harness Engineering](https://openai.com/index/harness-engineering/) — OpenAI, 2026. +- [Effective harnesses for long-running agents](https://www.anthropic.com/engineering/effective-harnesses-for-long-running-agents) — Anthropic, 2025. +- Field notes — Andrej Karpathy (Dec 2025) and Boris Cherny (Feb 2026). + +--- + +## Why a harness, not better prompts + +When an agent struggles with trivial work, treat it as a **context/harness failure, not a prompt failure** (chrismdp). The model is rarely the bottleneck — GitHub Copilot running a frontier model underperforms a purpose-built CLI harness, because the wrapper (context selection, the agent loop, the gates) matters at least as much as the model. + +A useful frame is the **five duties of a harness** (OpenAI): the harness must **constrain** what the agent may do, **inform** it of what it should do, **verify** its work, **correct** its mistakes, and **keep humans in the loop at high-stakes decisions.** Every section below maps to one of these duties — and the duty teams most often under-build is *verify*. + +**The bottleneck has moved from generation to verification** (chrismdp). The question is no longer "how fast can we build?" but "how fast can we tell if it's right?" Two practical consequences run through this workflow: +- If verifying an AI change takes as long as writing it yourself, either present it differently, move the verification to an automated gate, or don't delegate that task. +- **Give the agent a way to verify its own work** — a test loop, a type checker, a browser check. With that feedback loop in hand, output quality rises sharply (Boris Cherny: ~2–3×). + +--- + +## Workflow Execution Order + +> Slash-command names below are the reference Claude-Code/spec-kit set; a project may rename +> them. The *sequence* is the contract, not the names. + +### Per-feature — Spec & planning (on the main branch) +1. `/speckit.draftissue` ← optional; turn an unstructured brief into a well-formed issue +2. `/speckit.reviewissue` ← pre-spec gate; posts gaps + recommendations as an issue comment +3. `/speckit.confirmissue` ← fold answered review into a `## Confirmed decisions` section +4. `/speckit.specify` +5. `/speckit.clarify` ← iterate until the spec feels complete +6. `/speckit.checklist` ← resolve all gaps before continuing +7. `/speckit.plan` +8. `/speckit.testplan` ← review output carefully before continuing +9. *red-phase commit* ← commit `test-plan.md` as locked intent (script per project) +10. `/speckit.tasks` +11. `/speckit.analyze` ← resolve HIGH/CRITICAL before branching; runs the test-plan cross-check via the `after_analyze` hook + +### Per-feature — Implementation +12. `/speckit.implement` ← agent runs to suite-green, keeping the suite green on the inner loop at its own discretion. This is a **soft standard, not a per-turn gate** — the binding "green before a PR" guarantee is the real suite run in step 13 (see *Where the completion gate belongs*). + +### Per-feature — Ship +13. `/ship` ← one orchestrator: **full suite (the gate) → clean-context review → fix → raise PR.** Runs unattended, so it can drive a loop. Composes what were previously separate manual steps (test-checklist, PR review, slop review, raise-PR). + +### Periodic (not per-feature) +- **capture learnings** — fold corrections back into memory/skills. Deliberately *not* part of `/ship`: it needs human curation and batches better across several features, so run it at a supervised checkpoint after a batch. +- **dead-code audit** — every few features or before a release; **not per-PR**. +- **context gardening** — quarterly or after a big architectural shift. + +--- + +## The ship gate + +`/ship` exists because the steps between "implementation looks done" and "PR raised" are a fixed sequence with one hard ordering constraint, and a human re-enacting them from prose gets it subtly wrong. + +**The suite runs first and everything else is downstream of its exit code.** This ordering is *structural*, not policed: review cannot begin against unverified or red code because it literally runs after the gate. Do not add a hook to enforce the ordering — the exit code **is** the gate. A hook that watches for the agent *claiming* green is exactly the anti-pattern the "gates attach to actions, not prose" rule warns against. + +Properties worth copying: + +- **Unattended by design.** No prompts anywhere in the flow, so `/ship` can be driven by an automated loop shipping features back-to-back, as well as invoked by hand. Anything that needs a human turns the pipeline into a wait. +- **Stop-on-failure is global.** Suite not green, a reviewer subagent errors, push rejected, PR already exists — stop at that step, report, and run nothing later. +- **Preconditions run before the expensive work.** Check the cheap things first (on a feature branch? any commits over main?), or a full suite and full review burn before a late guard trips. +- **Review runs in clean context.** Reviewers see the diff, not the conversation that produced it. The *deciding and fixing* legitimately happens in the orchestrator's own loop — "review in clean context" governs the reviewing, not the fixing. +- **Validate a finding before acting on it.** Reviewer severities are fickle; cross-check a "Critical" against the actual test and spec state rather than relaying it verbatim. Acting on a mislabelled finding is how a review pass makes code worse. +- **Re-verify what review changed.** Fixes applied after the gate are unverified code — re-run the suite before raising, or a bad fix ships green-unchecked. + +**Two reviews, not one.** *Review A* is synchronous and inside `/ship` — clean-context subagents over the diff, whose findings are in-conversation and therefore available to `capture-learnings` later. *Review B* is the asynchronous agent review on the raised PR, for a human to read at merge. `/ship` never waits on Review B: blocking a pipeline for minutes to fold in a second review of the same diff buys little. + +--- + +## Separation of Concerns + +- **Spec (what & why):** requirements as normative SHALL/MUST statements. No test scenarios. +- **Test plan (how you verify):** named scenarios derived from the spec, after the spec is complete, before tasks are decomposed. +- **Tasks (how you build):** implementation breakdown informed by the test plan. +- **Test checklist (did you honour it):** static analysis after implementation confirming every scenario has an honest test. + +### Spec the problem, not the solution +A spec fixes *outcomes and constraints*, not mechanism. "We need a searchable audit log retained seven years without slowing writes" beats "create table audit_log with these eight columns" (chrismdp). The same idea, stated as a rule: **acceptance criteria must read as user-observable outcomes that hold under any reasonable implementation.** Over-specifying the solution upfront is the waterfall mistake with new branding, and it produces brittle, implementation-mirroring tests. + +--- + +## Design Principles + +- **Single branch per feature, one mission.** Tests and implementation on the same branch; commit history is the audit trail. When a second mission surfaces mid-flight, ship the first with documented known-issues and open a separate branch. +- **The test plan is the red-phase baseline.** In a statically-typed project, pre-impl test files can't compile; the committed `test-plan.md` is the locked intent, and the test checklist enforces honesty. +- **PR review is the integrity gate.** The reviewer diffs the test plan and checks the checklist report — not a binary pass/fail. + +### Gates over rules +A rule the agent must *remember* is weaker than a gate the system *enforces* — Karpathy notes the common failure modes (silent assumptions, overcomplication, orthogonal edits, weak success criteria) persist "despite a few simple attempts to fix it via instructions in CLAUDE.md." When the same correction recurs, promote it from a written rule to a mechanical gate. **Crucially, gates attach to actions, not prose** — no hook can see the agent *say* "all tests pass," so enforcement must hang off concrete actions (a test run, a commit, a PR creation), never off claims. + +Three corollaries: + +- **Exclusions are amendment-level, not per-PR.** When the project deliberately excludes a tool or approach (a UI-automation framework, a whole dependency class), record it as a *standing, named exclusion* that changes only by explicit amendment — not a call an agent or a single review can reason past mid-task. Left as prose guidance, an agent will re-derive the "reasonable" case for the excluded tool every time a gate blocks it. Back the exclusion with a denylist gate so the excluded path is mechanically impossible. +- **Package operations as callable, self-documenting commands — not prose.** Anything you want the agent to do consistently, especially destructive or multi-step orchestration, belongs in a single script/command it invokes (with real `--help` output it can query), gated so the raw pieces can't be hand-assembled. Orchestration that lives only as prose in a skill or doc gets re-enacted imperfectly and drifts every time. +- **Guard the files an upgrade will overwrite.** Where the project vendors a scaffolding tool (spec-kit or similar) whose generated files carry local customisations, a `--force` re-init silently resets them — including the settings that stop an agent invoking things it shouldn't. Deny the agent's edit path on upstream-managed files so drift can't accumulate there, and keep genuine extension points editable. The upgrade itself writes files directly and is unaffected, which is the point: the guard stops unattended drift, not deliberate action. + +--- + +## Mechanical enforcement layer + +The *verify* and *correct* duties, made automatic. Minimum set: + +- **Stale-build guard.** Block running tests `--no-build` (or equivalent) when sources changed since the last build — stale binaries produce lying green results. +- **The test-green gate.** The full suite must pass on the code about to become a PR. Put this in the ship flow, not the implement turn — see *Where the completion gate belongs*, below. +- **Traceability gate.** The exact-match half of the test checklist — spec label ↔ test-plan scenario ↔ code marker, character-for-character — is a *deterministic* gate. The judgment-level half (mock self-satisfaction, trivial-pass, fuzzy matches) stays a human-run review command. This one *does* belong at the agent's turn-end, as a loop-guarded nudge rather than a lock-out. +- **No skipped tests.** Skipped / ignored / conditionally-skipped tests (including *runtime* skips) are banned by a static gate — they fake coverage and rot the spec→test trace. Genuinely-untestable scenarios go in a documented "untested branches" table, not a faked skipped test. +- **Fast/slow test categories.** Tag tests *unit* (fast, no external dependencies) vs *integration* (slow, real stack), so the agent gets seconds-fast inner-loop feedback while developing — the whole-suite run remains the completion gate, not the tagged subset. +- **CI on PR** *(where the suite can run in CI).* Build + test on every PR, blocking merge. If the real test stack can't run in hosted CI (heavy infra, private-repo limits), keep the full gate local and let CI cover the deterministic subset only — and say so explicitly. + +### Where the completion gate belongs — at ship, not at turn-end + +The instinct is to gate the *implement* agent: a turn-end hook that refuses to let it stop until the suite is green. It is the wrong seam, and the reason generalises. + +A turn-end hook cannot run the suite itself — a full run is minutes, and the agent's turn is not the place to spend them. So it does the only thing it can: it consults a **ledger** recording that a green run happened at some past moment, and gates on that. But a past green run is a *proxy*. It attests that the suite passed on some earlier state of the code, not that it passes on the code about to become a PR. The plumbing this proxy needs — the ledger, the file markers, the locking, the "was that a whole-suite run or a filtered one?" heuristic — is substantial, and it buys an attestation weaker than the thing you actually wanted. + +**Put the gate where the truth is: a real whole-suite run at ship time, immediately before the PR is raised.** It is more machinery removed than added, and the guarantee gets *stronger* — the suite passes **now**, on exactly the diff under review. During implementation, keeping the suite green becomes a soft standard the agent applies at its own discretion on the inner loop, which is where discretion is cheap and a hard gate is merely a tax. + +The general rule this instance teaches: **when a gate can only see a proxy for the property you care about, move the gate to where the property itself is observable.** A gate on a ledger is a gate on a claim about the past, which is a short step from the "gates attach to actions, not prose" failure it was meant to avoid. + +### How many green runs? One — and fix the flakiness instead + +A tempting bar is *N consecutive* green runs, on the reasoning that a non-deterministic stack makes a single green run luck rather than proof. Resist it. **A multi-run bar is a crutch for a flaky suite, and it prices every completion at N× the suite's wall-clock.** The honest reading of "we need three green runs to believe it" is "our suite lies one run in three" — and the fix for that is the flakiness, not the arithmetic. + +Set the bar at **one green whole-suite run since the last code change**, and *earn* it: if the suite is genuinely non-deterministic, hunt the flake. Where a multi-run rule already exists, retire it against evidence rather than taste — a provocation campaign (repeated runs under varied seed, concurrency, accumulated state, and CPU pressure) either demonstrates determinism, which retires the rule, or surfaces the flake, which is the thing you actually needed to find. + +### Formatting is not verification — do it at ship cadence + +Formatting is cosmetic, and cosmetic work does not belong on the inner loop. A format-on-commit hook taxes **every** commit — on a real codebase the tool's workspace load is measured in tens of seconds — to fix something no reviewer would have caught anyway. Fold the format step into the ship flow instead, where it runs once per PR at a cadence that already costs minutes. (Boris Cherny's "formatting handles the last 10%" is right about the value and silent about the cadence; per-commit is the wrong one.) + +> Pre-allow safe commands in checked-in settings rather than disabling permission prompts +> wholesale (Boris Cherny): the agent flows, but high-stakes actions still surface. + +--- + +## Context management + +Output quality follows a curve (chrismdp): too little context → generic slop; too much → the model drowns in noise and is confidently vague. Two habits: +- **Reset on drift.** Useless refactoring of working code, repeated wrong assumptions, or a "going in circles" feel are signs to start a fresh session — resetting is cheaper than correcting, and models tend to stay lost once they take a wrong turn. +- **Plan mode first for non-trivial work** (Boris Cherny / Karpathy): agree the plan before switching to auto-accept edits. A good plan is often the difference between a one-shot and a spiral. Reserve the heavyweight spec pipeline for features; use lightweight plan-mode for small changes. + +For **long-running / multi-session** work (Anthropic): keep a durable progress ledger and a feature list seeded as "all failing," so a later session gets its bearings from git history + progress notes rather than re-deriving state — and can't declare premature victory. + +**Prefer the tracker's own state as that ledger.** Before building a progress file, check what the issue tracker already models: issue open/closed, dependency relationships, labels, and PR links are durable, shared, human-visible, and already the thing the team edits. A modern tracker resolves the dependency graph server-side and reports it per issue, so "what is ready to work on now" is a *query*, not a parser and a graph walk you maintain. A bespoke ledger has to be kept true; the tracker is true by construction, and every hand-rolled mirror of it eventually disagrees with it. + +--- + +## The feedback loop (compounding engineering) + +The trainer's job is to make next week's agent better than this week's: +- **Capture corrections as durable artifacts** — project-specific rules into the standing instructions / memory; generic rules into reusable skill files (chrismdp). +- **Do it continuously.** The strongest teams add to their shared instructions *multiple times a week*, the moment the agent does something wrong (Boris Cherny — "compounding engineering"). During review, tag the agent on a PR to fold the lesson in as part of the PR. +- **Ship one skill file a week** as a baseline cadence (chrismdp): pick a recurring annoyance, formalise it. +- **Prefer a gate to a memory entry** where the learning can be mechanically enforced. A captured correction that *could* have been a hook is a rule you will re-teach. + +--- + +## Documentation Hierarchy + +- Tier 1: GOVERNANCE → constitution / principles (supersedes all else) +- Tier 2: IMPLEMENTATION → `CLAUDE.md` / `AGENTS.md` (essential patterns + signposts) +- Tier 3: DETAILED REFERENCES → `docs/…` deep guidance and/or a memory corpus + +Keep Tier 2 short and high-signal; symlink `CLAUDE.md`↔`AGENTS.md` so every tool reads the same rules. A project may realise Tier 3 as convention docs, a memory ledger, or both — record which in the project's delta doc. + +--- + +## Failure modes the harness defends against + +| Failure mode (Karpathy / chrismdp) | Harness defence | +|---|---| +| Silent assumptions, no clarifying questions | plan-mode first; review/confirm gates surface decisions before code | +| Overcomplication, bloated abstractions | slop review + simplifier sub-agent; "would a senior call this overcomplicated?" | +| Orthogonal edits (touching unrelated code) | one-mission branch; "mention dead code, don't delete it"; diff-scoped review | +| Weak success criteria | outcome-level ACs + test plan + the ship gate (a real suite run before the PR) | +| Accidental vibe coding (ship unverified) | the mechanical enforcement layer; the ship gate's structural ordering | +| Review fatigue | move recurring issues into skill files / gates, off the human's plate | +| Harness change locks out the harness | build gates fail-open with an override first, verify, then tighten | + +--- + +## Appendix — Codebase Setup + +The kinds of files a project adds to make this workflow operational (names illustrative): + +### Agent configuration (`.claude/` or `.agents/`) +- **settings** — checked-in permissions allowlist + hooks: *stale-build guard*, *traceability nudge*, plus any denylist gates backing a standing exclusion; and a deny path over upstream-managed vendored files. Note what is *not* here: the test-green gate is a real suite run inside the ship command, not a hook. +- **commands** — the custom slash commands above (`draftissue`, `reviewissue`, `confirmissue`, `testplan`, `testchecklist`, slop review, dead-code audit, context-gardening, raise-PR, capture-learnings, and the `ship` orchestrator that composes the pre-PR ones). +- **skills / sub-agents** — simplifier, verifier, and any stack-orchestration script. + +### spec-kit configuration (`.specify/`) +- **constitution** — project governance, versioned, supersedes other guides. +- **extensions** — lifecycle hooks (git auto-commit per phase; test-plan cross-check). + +### CI / SCM integration +- **CI workflow** — build + test on PRs (full suite or deterministic subset; see enforcement). +- **Agent review action** — auto-review on agent-authored PRs; respond to `@agent` mentions. +- **PR template** — Summary, Spec link, Changed files, New artifacts. + +### Conventions / reference +- Detailed style guide; optional Change-Intent-Records (or an equivalent decision ledger). + +--- + +## Modifying the harness itself + +Harness changes are the one class of work that can destroy the tool making the next change — a gate that blocks its own `git commit` is not a hypothetical. Four rules: + +1. **Test every hook standalone with synthetic input** before wiring it into settings. +2. **Build fail-open, with an override, first.** Verify no lock-out, *then* tighten to fail-closed. +3. **One mission per branch, its own PR.** Dogfood the workflow once the gate exists. +4. **A human reviews each hook before it lands in settings.** Non-negotiable. + +--- + +## Token / context management plugins +- **read-once** — avoid re-reading unchanged files. +- **context-mode** — context budgeting. +- **rtk** — token-saving CLI proxy (`rtk gain`). From d8ffa51620e15d8d024a642dbd9fd4c6c1239988 Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:46:01 +0200 Subject: [PATCH 2/2] Scrub remaining references to the private sibling repo from public files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three pre-existing references named a private sibling repo — invisible to external readers — in public NetPace files: - .specify/templates/overrides/README.md — "Why overrides it" table header, a copy-paste leftover; corrected to "Why this repo overrides it". - docs/change-intent-records/2026-07-10-guard-speckit-files.md — "downstream () trees" reworded to "other repos sharing this harness". - .claude/memory/feedback_read_source_before_designing.md — dropped the sibling-repo origin story; the concrete NetPace incident already carries the lesson. No meaning lost; all framing now stands on its own. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01T6dd1XBNhEq42D6EsY6nPz --- .claude/memory/feedback_read_source_before_designing.md | 2 +- .specify/templates/overrides/README.md | 2 +- docs/change-intent-records/2026-07-10-guard-speckit-files.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.claude/memory/feedback_read_source_before_designing.md b/.claude/memory/feedback_read_source_before_designing.md index 22af4af..888e45a 100644 --- a/.claude/memory/feedback_read_source_before_designing.md +++ b/.claude/memory/feedback_read_source_before_designing.md @@ -6,6 +6,6 @@ type: feedback Before designing a fix that depends on how a subsystem behaves, **read the source that implements that behaviour** — not the docs, not your prior mental model, not the test names, and **not a diagnosis handed to you in an issue, spec, or ticket**. A confident root-cause written by someone else is an assumption to verify against current HEAD, not a fact — especially when the artefact predates recent commits. Reading the actual code upfront routinely avoids multiple rounds of fix proposals that turn out to be solving the wrong problem. -**Why:** first ported from IMS's harness-hardening review (a sibling .NET/spec-kit repo) as a preventive practice, then confirmed by a NetPace incident. In IMS, a subsystem's behaviour was repeatedly guessed at ("the system probably does X") across three iterations of a proposed fix, when reading a single line of the actual implementation would have surfaced the real design gap immediately. In NetPace, a `/ship` road-test began from issue #220's confident "servers are probed concurrently" root-cause; a branch and test edit were made straight off it — but `GetFastestServerByLatencyAsync` is a sequential `for` loop, and the flake had already been fixed by #221's `SynchronousProgress`. Three clean-context reviewers had to catch that the whole branch premise was invalid. One `Read` of `OoklaSpeedtest.cs` before editing would have surfaced it immediately. See [[feedback_docs_no_forward_references]] for the sibling rule that docs (and by extension issues) can drift from current code. +**Why:** confirmed by a NetPace incident. A `/ship` road-test began from issue #220's confident "servers are probed concurrently" root-cause; a branch and test edit were made straight off it — but `GetFastestServerByLatencyAsync` is a sequential `for` loop, and the flake had already been fixed by #221's `SynchronousProgress`. Three clean-context reviewers had to catch that the whole branch premise was invalid. One `Read` of `OoklaSpeedtest.cs` before editing would have surfaced it immediately. See [[feedback_docs_no_forward_references]] for the related rule that docs (and by extension issues) can drift from current code. **How to apply:** when a proposed fix rests on "the system does X" / "I believe X happens because..." — or on a root-cause quoted from an issue/spec/ticket — stop and grep or read the exact code that implements X before sketching the fix. When the work *starts* from an issue, treat verifying its diagnosis against HEAD as the first step, before creating a branch or editing. Applies especially to `ISpeedTestService` implementations, provider-specific behaviour in `Clients/{ProviderName}/`, and anywhere a fix is being designed against assumed rather than confirmed behaviour. diff --git a/.specify/templates/overrides/README.md b/.specify/templates/overrides/README.md index 0ed3033..c17a957 100644 --- a/.specify/templates/overrides/README.md +++ b/.specify/templates/overrides/README.md @@ -13,7 +13,7 @@ Resolution order (highest → lowest precedence): ## Current overrides -| File | Why IMS overrides it | +| File | Why this repo overrides it | |------|----------------------| | `spec-template.md` | Bakes in the `**Scenario: [name]**` label convention required by [Constitution principle VIII](../../memory/constitution.md) for AC-to-test traceability. | diff --git a/docs/change-intent-records/2026-07-10-guard-speckit-files.md b/docs/change-intent-records/2026-07-10-guard-speckit-files.md index d2cedff..8a87a91 100644 --- a/docs/change-intent-records/2026-07-10-guard-speckit-files.md +++ b/docs/change-intent-records/2026-07-10-guard-speckit-files.md @@ -12,7 +12,7 @@ **Constraints:** - The protected set mirrors the two spec-kit manifests (`.specify/integrations/{speckit,claude}.manifest.json`): 6 core templates, 5 bash scripts, 10 SDD skills. - Extension points must stay editable — deny globs are single-level (`*` never crosses `/`), so `templates/*.md` excludes `templates/overrides/`. -- Enforcement must cherry-pick cleanly onto downstream (IMS) trees: config-only, no per-clone bootstrap. +- Enforcement must cherry-pick cleanly onto other repos sharing this harness: config-only, no per-clone bootstrap. **Decisions:** 1. **`deny`, not `ask`** — a firmer guarantee against headless drift. Trade-off: a deliberate human edit requires temporarily removing the rule. `ask` (human-approvable, headless-blocked) was rejected as too soft for the stated goal.