From d7b719f478c648123321b1a145030f500190f459 Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 21:19:54 +0100 Subject: [PATCH 01/23] Skill: tdd, test-first loop with CertShell seams and real test helpers --- .claude/skills/tdd/SKILL.md | 70 +++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .claude/skills/tdd/SKILL.md diff --git a/.claude/skills/tdd/SKILL.md b/.claude/skills/tdd/SKILL.md new file mode 100644 index 0000000..a0eedde --- /dev/null +++ b/.claude/skills/tdd/SKILL.md @@ -0,0 +1,70 @@ +--- +name: tdd +description: > + Test-first development for CertShell: red → green in vertical slices, tests + only at agreed seams, real dependencies over mocks. Use when the user wants + to build a feature or fix a bug test-first, says "tdd", "red-green", or + "/tdd", or when `/implement` or `/implement-spec` calls it. +--- + +The red → green loop, plus the rules that make its tests worth keeping. + +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). + +## A good test + +Tests check behaviour through a public interface, never internals. The code +underneath can be rewritten; the test shouldn't change. Name it like a spec: +`"sign rejects a login not in the access map"`, not `"calls checkAccess"`. + +## Seams + +A **seam** is the public boundary a test observes: a route +(`app.fetch(new Request(...))`), a WebSocket, an exported function. + +**Test only at agreed seams.** Before the first test, list the seams, each +with one line on what it catches and what it misses, and get them confirmed: + +- Called from `/implement` or directly: ask the user once. +- Called from an `/implement-spec` implementer: the seams come from the + ticket. No seams in the ticket → stop and report; don't invent them. + +## Where tests go + +- API: `apps/api/test/.test.ts`. Web unit: `apps/web/test/`. + Browser flows: `apps/web/e2e/`. Repo-level: `test/`. All end in `.test.ts`. +- Run one file while looping: `bun test ` (from the app dir for app + tests). Typecheck often: `bun run typecheck`. +- No `find`/`grep` in tests; use Bun's `Glob` and built-ins. + +## Real, not mocked + +Use the real thing; the helpers already exist: + +- Server + WS + session + throwaway CA: `startApp()` in `apps/api/test/helpers/app.ts`. +- OIDC: `startMockIdp()` in `apps/api/test/helpers/mock-idp.ts`. +- SQLite: `openDb(":memory:")`. `ssh-keygen`: the real binary. +- Target host: the sshd container in `apps/api/test/ssh-target/`. + +Mock only time and outbound alerts (`apps/api/test/fixtures/shoutrrr.ts`). +Never mock our own modules. + +## Anti-patterns + +- **Implementation-coupled**: mocks an internal module, tests a private + function, or checks through a side channel (querying SQLite instead of the + route). Tell: a refactor breaks it with no behaviour change. +- **Tautological**: the expected value is computed the way the code computes + it, so it can't fail. Expected values are literals, worked examples, or + SPEC.md. +- **Horizontal slicing**: all tests first, then all code. Tests written that + way check an imagined shape, not behaviour. + +## Loop rules + +- **Red first.** Write one failing test, watch it fail for the right reason, + then write just enough code to pass. +- **One slice per cycle**: one seam, one test, the minimal code. +- **No refactoring in the loop.** That's `/change-review`'s job. +- Security behaviour (authz, input checks, headers, rate limits, audit + events) gets a test that fails without the check. From a7db8cbad56acf8c7b36b01b76d6618c4e0017fc Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 21:19:54 +0100 Subject: [PATCH 02/23] Skill: change-review, standards/spec/security axes against origin/main --- .claude/skills/change-review/SKILL.md | 104 ++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 .claude/skills/change-review/SKILL.md diff --git a/.claude/skills/change-review/SKILL.md b/.claude/skills/change-review/SKILL.md new file mode 100644 index 0000000..d0edadf --- /dev/null +++ b/.claude/skills/change-review/SKILL.md @@ -0,0 +1,104 @@ +--- +name: change-review +description: > + Reviews CertShell changes since a fixed point (default `origin/main`) on + separate axes run in parallel sub-agents: Standards (CLAUDE.md, + CONTRIBUTING.md, code smells), Spec (does it do what the issue / SPEC.md + asked), and Security (`security-reviewer`, when sensitive paths changed). + Use when the user says "change-review", "/change-review", "review my + branch", "review since X", or when `/implement` or `/implement-spec` + calls it. Reports findings, applies nothing. +--- + +Review `git diff ...HEAD` on separate axes, side by side: + +- **Standards**: does the code follow this repo's documented rules? +- **Spec**: does it do what the issue or SPEC.md asked, no more, no less? +- **Security**: does it hold SPEC.md §3, §5, §6? Only when sensitive paths changed. + +A change can pass one axis and fail another. Each axis runs in its own +sub-agent so one can't mask the other. + +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). + +## Steps + +1. **Pin the base.** Whatever the user named (SHA, branch, tag, `HEAD~3`), + else `origin/main`. `git fetch -q origin`, then `git rev-parse ` + must resolve and `git diff ...HEAD` must be non-empty; fail here, + not in the sub-agents. If `git status --porcelain` shows changes, say they + aren't reviewed (commit first). Note `git log ..HEAD --oneline`. +2. **Find the spec**, in order: + - `#N` / `Closes #N` in commit messages or the branch's PR body + (`gh pr view --json body`): `gh issue view `, plus its parent + (`gh api repos/{owner}/{repo}/issues//parent`) if it has one. + - A path or `.audit//NN-slug.md` ticket the user passed. + - The plan agreed in this conversation. + - Nothing → ask. "No spec" → skip the Spec axis and say so. + SPEC.md is always context for the Spec axis. +3. **Security axis needed?** Yes if the diff touches `apps/api/src/`, + `Dockerfile`, `compose*.yaml` or `.github/workflows/`. +4. **Spawn the axes in parallel**, foreground, in one message. Each prompt + carries the diff command, commit list, and its brief below. +5. **Report** (see Output). Don't merge, dedupe or re-rank across axes. + +## Standards brief + +Sources: `CLAUDE.md`, `CONTRIBUTING.md`, the neighbouring code's patterns. +Skip anything Biome (`bun run lint`) or `tsc` enforces. A documented repo +rule beats the smells below; where the repo endorses something a smell +would flag, drop the smell. + +Repo rules worth checking by hand: + +- Tests come with every feature and fix, in `apps/*/test/` or `apps/web/e2e/`, ending `.test.ts`. +- No `find`/`grep` in tests (Bun `Glob` instead). +- Comments only where the next reader would otherwise spend several tool calls; a paragraph justifying a workaround means the code is wrong. +- Nothing changed beyond what was asked. +- One logical change per commit. +- Design change → SPEC.md updated in the same branch. + +Smells (always `judgement`, never `hard`): Mysterious Name, Duplicated +Code, Feature Envy, Data Clumps, Primitive Obsession, Repeated Switches, +Shotgun Surgery, Divergent Change, Speculative Generality, Message Chains, +Middle Man, Refused Bequest (Fowler, _Refactoring_ ch. 3). Name the smell, +quote the hunk, give the fix. + +Brief: "Report every breach of a repo rule (`hard`, cite file + rule) and +every smell (`judgement`). Use the finding format given. Under 400 words." + +## Spec brief + +Pass the spec contents (issue body, ticket file, or agreed plan) and SPEC.md. + +Brief: "Report (a) asked-for requirements missing or partial, (b) behaviour +in the diff nobody asked for, (c) requirements that look done but are +wrong. Quote the spec line for each. Use the finding format given. Under +400 words." + +## Security axis + +Launch the `security-reviewer` agent on `...HEAD`. Pass its findings +through as they come. + +## Output + +``` +## Standards +. [hard|judgement] : . . [path:line] + +## Spec +. [missing|unasked|wrong] . . [path:line] (spec: "") + +## Security + +``` + +Number findings across the whole report so the user can say "fix 2 and 5". +End with one line per axis: `: findings, worst: ` (or +`clean`). Never pick a winner across axes. + +## Boundaries + +Reviews, applies nothing. Bug-hunting is the built-in `/code-review`; +whole-repo audits are `/project-audit`. From df06eeffe5413edbb9ad6715665cb7dce642277b Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 21:19:54 +0100 Subject: [PATCH 03/23] Skill: implement, one ticket test-first through review to PR --- .claude/skills/implement/SKILL.md | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .claude/skills/implement/SKILL.md diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md new file mode 100644 index 0000000..e67f67c --- /dev/null +++ b/.claude/skills/implement/SKILL.md @@ -0,0 +1,62 @@ +--- +name: implement +description: > + Builds one already-decided piece of CertShell work (a GH issue, a local sec + ticket, or the plan agreed in this chat) test-first, reviews it, and opens + a PR. Use when the user says "implement #N", "/implement". +disable-model-invocation: true +--- + +Build work that's already decided. Don't reopen the plan: no new design, no +alternative approach. If the input is unclear, ask; don't redesign. + +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). + +## Input + +One of: + +- `#N` or an issue URL: `gh issue view `. State its title before starting. +- A sec ticket path `.audit//NN-slug.md` (from `/audit-to-tickets`). +- "the plan in this chat": restate it in 3–5 lines first. + +A bare number that could mean something else in context (a report finding, +a checklist item) → ask which. + +**Sec tickets never leave this machine** until the user says so: local +branch, no push, no PR (SECURITY.md: the fix would disclose the bug before +release). Same for any issue whose fix turns out to be a vulnerability. + +## Steps + +1. **Branch.** On `main` → `git fetch -q origin && git switch -c / origin/main` + (`fix/`, `feat/`, `docs/`, `skills/` …). Already on a feature branch → stay. +2. **Seams.** Read the code the ticket touches. Propose the seams (one line + each: what it catches, what it misses) and ask the user once. Seams + already written in the ticket → state them and go. +3. **Build** with the `tdd` skill, slice by slice. Typecheck and run single + test files as you go; full suite once at the end. +4. **Commit** per logical change (test + code for one slice can share a + commit). No attribution lines. +5. **Review.** Run the `change-review` skill against `origin/main`. Fix every + `hard`, `missing`, `wrong` and security finding; fix `judgement` and + `unasked` ones unless they contradict the ticket, and list any you skip. + Fixes are extra commits. +6. **Verify.** `bun run verify` must pass. Don't report done otherwise. +7. **PR** (not for sec tickets): `git push -u origin HEAD`, then + `gh pr create` with a body: what changed, how it was tested, and + `Closes #N`. Never push to `main`. + +## Output + +``` +built: +branch: · commits: · verify: pass +review: · skipped: | none +pr: | local only (sec) +``` + +## Boundaries + +One ticket per run. Doesn't tick the issue's checkboxes or close it (the +PR's `Closes #N` does that on merge). Doesn't merge. From b5e76bf2c363f86295fe3ac7f414d7d410bb3386 Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 21:19:54 +0100 Subject: [PATCH 04/23] Skill: implement-spec, parent issue's sub-issues on one integration branch --- .claude/skills/implement-spec/SKILL.md | 86 ++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 .claude/skills/implement-spec/SKILL.md diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md new file mode 100644 index 0000000..aa64799 --- /dev/null +++ b/.claude/skills/implement-spec/SKILL.md @@ -0,0 +1,86 @@ +--- +name: implement-spec +description: > + Builds a whole parent GH issue's sub-issues on one integration branch, + running up to 3 implementer sub-agents in parallel worktrees in + `blocked_by` order, then reviews and opens one PR. Use when the user says + "implement-spec #N", "/implement-spec". +disable-model-invocation: true +--- + +Turn a parent issue and its sub-issues into one integration branch and one +PR, every ticket done. + +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). + +## Input + +A parent issue `#N` (e.g. the `Audit ` parent from `/audit-to-tickets`). + +- Tickets: `gh api repos/{owner}/{repo}/issues//sub_issues`, open ones only. +- Edges: `gh api repos/{owner}/{repo}/issues//dependencies/blocked_by` + per ticket; fall back to the body's `## Blocked by` section. +- Spec: the parent body plus SPEC.md. + +The tickets are a **task graph**, not a list. The **frontier** is every open +ticket whose blockers are all done on the integration branch. + +Every ticket must name its seams (`## Seams` or equivalent). List the +tickets without them up front and ask the user to add them or name them +now; don't start those until they have seams. + +Sec tickets live in `.audit/`, never on GH, so they're never in the graph. + +## Context pointers + +Talk to sub-agents in pointers: issue numbers, file paths, commit SHAs, +the notes dir. Don't paste what they can read. + +## Steps + +1. **Read** every ticket and its edges; print the graph as + `#n ← blocked by #m, #k` and the first frontier. +2. **Explore** (optional). One sub-agent reads the code and docs the tickets + need and writes notes to `<scratchpad>/spec-<N>/notes/`. Implementers + read those instead of re-exploring. +3. **Integration branch**: `git fetch -q origin && git switch -c spec/<N>-<slug> origin/main`, + push it. +4. **Implement.** One background sub-agent per frontier ticket, max 3 at a + time, each with `isolation: "worktree"`. Each implementer: + - checks its worktree is on the integration branch tip, resets onto it if not; + - runs `bun install --frozen-lockfile`; + - builds the ticket with the `tdd` skill at the ticket's seams (none → + stop and report, don't invent); + - commits per logical change, no attribution lines; + - merges the integration tip into its branch, runs `bun run verify`; + - reports: branch, commits, verify result, anything it couldn't do. +5. **Merge.** As each implementer reports, a merger sub-agent merges its + branch into the integration branch, fixing only trivial conflicts + (imports, adjacent hunks), then runs `bun run verify`. A non-trivial + conflict or a failed verify → stop and ask the user. Push. After the + first merge, open a draft PR: `gh pr create --draft`, body lists every + ticket as `Closes #n` plus `Part of #N`. +6. **Advance.** Each merge can widen the frontier: launch implementers for + newly unblocked tickets, keeping the max of 3. +7. **Review.** All tickets merged → run the `change-review` skill on the + integration branch against `origin/main`. One implementer sub-agent fixes + every `hard`, `missing`, `wrong` and security finding (and the rest + unless they contradict a ticket), commits, runs verify. +8. **Ready.** `bun run verify` on the integration branch passes → + `gh pr ready`. Never push to `main`; the user merges. +9. **Clean up** every implementer worktree and its local branch. + +## Output + +``` +spec: #<N> <title> · branch: <name> +done: #n, #n … · stuck: #n — <why> | none +review: fixed <n> · skipped: <finding — why> | none +verify: pass · pr: <url> +``` + +## Boundaries + +Doesn't merge the PR, close issues by hand, or edit the tickets. A ticket +whose fix turns out to be a vulnerability → stop and ask before anything +about it is pushed. From f63ee739955032fd887054e9c9159875a35ab88e Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:23:07 +0100 Subject: [PATCH 05/23] Skill: audit-to-tickets tickets carry agreed seams for implement-spec --- .claude/skills/audit-to-tickets/SKILL.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.claude/skills/audit-to-tickets/SKILL.md b/.claude/skills/audit-to-tickets/SKILL.md index bd8f6ba..8c00ccd 100644 --- a/.claude/skills/audit-to-tickets/SKILL.md +++ b/.claude/skills/audit-to-tickets/SKILL.md @@ -27,15 +27,19 @@ They go to local files (step 8) for the user to file as security advisories. already covered by an open issue gets that number, not a new issue. 4. **Draft tickets.** Merge findings that one fix closes. Order them so a ticket comes after anything it depends on (a refactor before the fix that - uses it). Each ticket fits one PR. + uses it). Each ticket fits one PR. Name its seams: the public boundaries + its tests observe (a route, the WebSocket, an exported function), each + with one line on what it catches. `cut:` tickets with no behaviour + change: the existing tests that must stay green. 5. **Quiz the user** with the draft, numbered: ``` <T>. <title> [<labels>] (findings <N>, <N>) blocked by: <T|none> + seams: <seam — what it catches>; … ``` Ask: is the granularity right (merge or split?), are the blocking edges - right, anything to drop? Iterate until the user approves. Nothing is + and seams right, anything to drop? Iterate until the user approves. Nothing is published before that. 6. **Labels.** Create `audit` if missing: `gh label create audit --color 5319e7 --description "Found by /project-audit"`. @@ -67,6 +71,9 @@ Where: path:line ## What to build <the Fix from the report, plus anything the grill decided> +## Seams +- <seam>: <what it catches> + ## Acceptance criteria - [ ] <observable outcome, e.g. "POST /x without session → 401"> - [ ] test covering it (for bug/load/test findings) From d907097ae7f69217f31bc11978f82fcd7bcb1fd8 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:35:52 +0100 Subject: [PATCH 06/23] Skill: tdd matches the repo's ssh fake and documents e2e --- .claude/skills/tdd/SKILL.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.claude/skills/tdd/SKILL.md b/.claude/skills/tdd/SKILL.md index a0eedde..224625c 100644 --- a/.claude/skills/tdd/SKILL.md +++ b/.claude/skills/tdd/SKILL.md @@ -35,6 +35,8 @@ with one line on what it catches and what it misses, and get them confirmed: Browser flows: `apps/web/e2e/`. Repo-level: `test/`. All end in `.test.ts`. - Run one file while looping: `bun test <path>` (from the app dir for app tests). Typecheck often: `bun run typecheck`. +- E2E: `bun run e2e` (Playwright). `bun run verify` doesn't run it: run it + whenever you touch `apps/web/e2e/` or the flows it covers. - No `find`/`grep` in tests; use Bun's `Glob` and built-ins. ## Real, not mocked @@ -44,10 +46,13 @@ Use the real thing; the helpers already exist: - Server + WS + session + throwaway CA: `startApp()` in `apps/api/test/helpers/app.ts`. - OIDC: `startMockIdp()` in `apps/api/test/helpers/mock-idp.ts`. - SQLite: `openDb(":memory:")`. `ssh-keygen`: the real binary. -- Target host: the sshd container in `apps/api/test/ssh-target/`. +- ssh: swap the command through `startApp`'s terminal deps for a local `sh` + that inspects the real cert (`fakeSsh` in `apps/api/test/terminal.test.ts`). +- Real target host only for sshd-side behaviour: `apps/api/test/ssh-target.test.ts`, + skipped unless `SSH_TARGET` is set; its line 2 has the compose command. -Mock only time and outbound alerts (`apps/api/test/fixtures/shoutrrr.ts`). -Never mock our own modules. +Fake only those boundaries, time (`setSystemTime`) and outbound alerts +(`apps/api/test/fixtures/shoutrrr.ts`). Never mock our own modules. ## Anti-patterns From 762a935e1f3f834bad85f0c4f951a4c4956ec07d Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:35:52 +0100 Subject: [PATCH 07/23] Skill: tdd doesn't re-ask confirmed seams, points refactors at review fixes --- .claude/skills/tdd/SKILL.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.claude/skills/tdd/SKILL.md b/.claude/skills/tdd/SKILL.md index 224625c..8773be4 100644 --- a/.claude/skills/tdd/SKILL.md +++ b/.claude/skills/tdd/SKILL.md @@ -25,7 +25,8 @@ A **seam** is the public boundary a test observes: a route **Test only at agreed seams.** Before the first test, list the seams, each with one line on what it catches and what it misses, and get them confirmed: -- Called from `/implement` or directly: ask the user once. +- Called directly: ask the user once. +- Called from `/implement`: it already confirmed them; state them and go. - Called from an `/implement-spec` implementer: the seams come from the ticket. No seams in the ticket → stop and report; don't invent them. @@ -70,6 +71,7 @@ Fake only those boundaries, time (`setSystemTime`) and outbound alerts - **Red first.** Write one failing test, watch it fail for the right reason, then write just enough code to pass. - **One slice per cycle**: one seam, one test, the minimal code. -- **No refactoring in the loop.** That's `/change-review`'s job. +- **No refactoring in the loop.** Refactor in the review-fix step, after + `/change-review`. - Security behaviour (authz, input checks, headers, rate limits, audit events) gets a test that fails without the check. From bd7ad13d38e59c2e067aacff8b55869b0c44f540 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:36:03 +0100 Subject: [PATCH 08/23] Skill: change-review waits on background axes and hands them the line format --- .claude/skills/change-review/SKILL.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.claude/skills/change-review/SKILL.md b/.claude/skills/change-review/SKILL.md index d0edadf..ed8a443 100644 --- a/.claude/skills/change-review/SKILL.md +++ b/.claude/skills/change-review/SKILL.md @@ -38,8 +38,10 @@ Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). SPEC.md is always context for the Spec axis. 3. **Security axis needed?** Yes if the diff touches `apps/api/src/`, `Dockerfile`, `compose*.yaml` or `.github/workflows/`. -4. **Spawn the axes in parallel**, foreground, in one message. Each prompt - carries the diff command, commit list, and its brief below. +4. **Spawn the axes in parallel**: launch all of them in one message and + wait for every report before writing the output. Each prompt carries the + diff command, commit list, its brief below, and its line format from + Output pasted in literally (sub-agents can't see this file). 5. **Report** (see Output). Don't merge, dedupe or re-rank across axes. ## Standards brief @@ -65,7 +67,7 @@ Middle Man, Refused Bequest (Fowler, _Refactoring_ ch. 3). Name the smell, quote the hunk, give the fix. Brief: "Report every breach of a repo rule (`hard`, cite file + rule) and -every smell (`judgement`). Use the finding format given. Under 400 words." +every smell (`judgement`), one line each in the format below. Under 400 words." ## Spec brief @@ -73,8 +75,8 @@ Pass the spec contents (issue body, ticket file, or agreed plan) and SPEC.md. Brief: "Report (a) asked-for requirements missing or partial, (b) behaviour in the diff nobody asked for, (c) requirements that look done but are -wrong. Quote the spec line for each. Use the finding format given. Under -400 words." +wrong. Quote the spec line for each, one line each in the format below. +Under 400 words." ## Security axis From 48ac698a7ca14ef7bf4993d1d3a6f4a53b3d507b Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:36:03 +0100 Subject: [PATCH 09/23] Skill: change-review triggers on spec checks, not generic branch review --- .claude/skills/change-review/SKILL.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.claude/skills/change-review/SKILL.md b/.claude/skills/change-review/SKILL.md index ed8a443..f59e2f2 100644 --- a/.claude/skills/change-review/SKILL.md +++ b/.claude/skills/change-review/SKILL.md @@ -1,13 +1,14 @@ --- name: change-review description: > - Reviews CertShell changes since a fixed point (default `origin/main`) on - separate axes run in parallel sub-agents: Standards (CLAUDE.md, - CONTRIBUTING.md, code smells), Spec (does it do what the issue / SPEC.md - asked), and Security (`security-reviewer`, when sensitive paths changed). - Use when the user says "change-review", "/change-review", "review my - branch", "review since X", or when `/implement` or `/implement-spec` - calls it. Reports findings, applies nothing. + Checks a CertShell branch against its issue / SPEC.md and the repo's + CLAUDE.md rules, on separate axes run in parallel sub-agents: Standards + (CLAUDE.md, CONTRIBUTING.md, code smells), Spec (missing, unasked or wrong + vs the issue / SPEC.md), and Security (`security-reviewer`, when sensitive + paths changed). Default base `origin/main`. Use when the user says + "change-review", "/change-review", "does this match the issue", "review + against the spec", "review since X", or when `/implement` or + `/implement-spec` calls it. Reports findings, applies nothing. --- Review `git diff <base>...HEAD` on separate axes, side by side: From 79aa81568af87539e4bc54754f7a96e8a4900a89 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:36:13 +0100 Subject: [PATCH 10/23] Skill: implement skills re-run security-reviewer on sensitive review fixes --- .claude/skills/implement-spec/SKILL.md | 5 ++++- .claude/skills/implement/SKILL.md | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index aa64799..600171e 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -65,7 +65,10 @@ the notes dir. Don't paste what they can read. 7. **Review.** All tickets merged → run the `change-review` skill on the integration branch against `origin/main`. One implementer sub-agent fixes every `hard`, `missing`, `wrong` and security finding (and the rest - unless they contradict a ticket), commits, runs verify. + unless they contradict a ticket), commits, runs verify. If its commits + touch `apps/api/src/`, `Dockerfile`, `compose*.yaml` or + `.github/workflows/`, run the `security-reviewer` agent on just those + commits and fix what it finds. 8. **Ready.** `bun run verify` on the integration branch passes → `gh pr ready`. Never push to `main`; the user merges. 9. **Clean up** every implementer worktree and its local branch. diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index e67f67c..8a31d93 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -41,7 +41,9 @@ release). Same for any issue whose fix turns out to be a vulnerability. 5. **Review.** Run the `change-review` skill against `origin/main`. Fix every `hard`, `missing`, `wrong` and security finding; fix `judgement` and `unasked` ones unless they contradict the ticket, and list any you skip. - Fixes are extra commits. + Fixes are extra commits. If they touch `apps/api/src/`, `Dockerfile`, + `compose*.yaml` or `.github/workflows/`, run the `security-reviewer` + agent on just those commits and fix what it finds. 6. **Verify.** `bun run verify` must pass. Don't report done otherwise. 7. **PR** (not for sec tickets): `git push -u origin HEAD`, then `gh pr create` with a body: what changed, how it was tested, and From 1e0d5afdf7c7f0cad204821e031f66ee8f0812fe Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:36:13 +0100 Subject: [PATCH 11/23] Skill: implement-spec review fixer works in its own worktree --- .claude/skills/implement-spec/SKILL.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index 600171e..38f3bb9 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -63,12 +63,13 @@ the notes dir. Don't paste what they can read. 6. **Advance.** Each merge can widen the frontier: launch implementers for newly unblocked tickets, keeping the max of 3. 7. **Review.** All tickets merged → run the `change-review` skill on the - integration branch against `origin/main`. One implementer sub-agent fixes + integration branch against `origin/main`. One implementer sub-agent, in + its own worktree on the integration tip like any ticket, fixes every `hard`, `missing`, `wrong` and security finding (and the rest unless they contradict a ticket), commits, runs verify. If its commits touch `apps/api/src/`, `Dockerfile`, `compose*.yaml` or `.github/workflows/`, run the `security-reviewer` agent on just those - commits and fix what it finds. + commits and fix what it finds. The merger merges it as in step 5. 8. **Ready.** `bun run verify` on the integration branch passes → `gh pr ready`. Never push to `main`; the user merges. 9. **Clean up** every implementer worktree and its local branch. From 24ffeb7d9ef430c968e026d3c3b6f4715ab5999a Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:39:06 +0100 Subject: [PATCH 12/23] Biome and git ignore .claude/worktrees so verify passes with agent worktrees --- .gitignore | 1 + biome.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 8f27fd3..f8331bf 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ apps/web/skills-lock.json .wrangler/ .claude/settings.local.json .audit/ +.claude/worktrees/ diff --git a/biome.json b/biome.json index f3193f0..2fe6c1a 100644 --- a/biome.json +++ b/biome.json @@ -7,7 +7,7 @@ }, "files": { "ignoreUnknown": false, - "includes": ["**", "!**/.claude/skills", "!**/skills-lock.json"] + "includes": ["**", "!**/.claude/skills", "!**/.claude/worktrees", "!**/skills-lock.json"] }, "formatter": { "enabled": true, From ec752e1d850ead8b4b7f2ea3550e5179ebd98b86 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:40:33 +0100 Subject: [PATCH 13/23] Format biome.json --- biome.json | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/biome.json b/biome.json index 2fe6c1a..9496003 100644 --- a/biome.json +++ b/biome.json @@ -7,7 +7,12 @@ }, "files": { "ignoreUnknown": false, - "includes": ["**", "!**/.claude/skills", "!**/.claude/worktrees", "!**/skills-lock.json"] + "includes": [ + "**", + "!**/.claude/skills", + "!**/.claude/worktrees", + "!**/skills-lock.json" + ] }, "formatter": { "enabled": true, From c5a506379e2b2f2d396c51639808991e8f0b34e3 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:51:00 +0100 Subject: [PATCH 14/23] Skill: ponytail adapted for CertShell (smallest complete change, reuse map, never-cut) --- .claude/skills/ponytail/SKILL.md | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 .claude/skills/ponytail/SKILL.md diff --git a/.claude/skills/ponytail/SKILL.md b/.claude/skills/ponytail/SKILL.md new file mode 100644 index 0000000..f129b8b --- /dev/null +++ b/.claude/skills/ponytail/SKILL.md @@ -0,0 +1,87 @@ +--- +name: ponytail +description: > + CertShell's coding stance: the smallest change that fully solves the task, + reuse before stdlib before dependencies, no speculative code, and a reply a + busy human reads once. Auto-loaded via CLAUDE.md for every session and + sub-agent; invoke it again on any coding task (writing, fixing, + refactoring, choosing a dependency) or when the user says "ponytail", + "be lazy", "simplest solution", "yagni", or complains about bloat. +--- + +# Ponytail + +Be a lazy senior developer: the best code is the code never written. Solve +the whole problem with the least new code. End every reply that changed +code with one line: `skipped / risk: <what you skipped or didn't check, and +any risk the user must know>`. + +Always on. The user saying "normal mode" drops it for the session. + +Adapted from [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail) (MIT). + +## Precedence + +CLAUDE.md and SPEC.md, then the `tdd` skill, then this file. + +## Before you write + +Read the task and the code it touches. List every place the change must +reach: callers, tests, fixtures, config, exports, SPEC.md. Check what it +could break: data it destroys or exposes, callers that stop working. That's +scope. Extra features aren't. + +## The smallest complete change + +Take the first option that fully works: + +1. **Does it need to exist?** Skip features, options and flexibility nobody + asked for; name them in one line. +2. **Already in this repo?** Use it the way the surrounding code does (see + Reuse first). +3. **Bun or the platform?** `Bun.spawn`/`run()` argv, Bun `Glob`, `bun:test`, + Web Crypto (`crypto.getRandomValues`, `crypto.subtle`). Never `$` for `ssh`/`ssh-keygen` (it drops + empty args). +4. **An installed dependency?** Use it. A new one needs a reason a reviewer + accepts (CONTRIBUTING), never for a few lines. +5. **One line a reader gets at a glance?** One line. +6. Otherwise, the minimum code that works. + +- Lazy about the solution, never about the change: finish every part it + needs, including the callers, tests and fixtures it breaks. +- No abstraction, wrapper, option, config or "for later" code nobody asked + for. Keep values in the form the platform gives you. Deletion beats + addition. Keep the repo's structure: `routes/` → `lib/` / `ssh/`, shared + types in `packages/shared`. +- The shortest working diff wins, once you know everything it must touch. A + one-liner that needs decoding isn't short. +- Comments: CLAUDE.md's rule. Only the why the code can't show, one line. +- Bug fix: grep every caller of the function first, then fix the root cause + once in the shared code. +- Moved or merged code keeps its error handling and validation. +- Between options of equal size, take the one correct on edge cases. +- Every change is tested (CLAUDE.md), trivial or not; non-trivial work goes + through `tdd`. +- A shortcut with a known limit gets `// shortcut: <the limit>, <when to upgrade>`. + `/project-audit` treats these as recorded decisions. + +## Reuse first + +- argv commands: `run()` in `apps/api/src/ssh/exec.ts`. +- Input checks: `valid*` and `parseHostKey` in `apps/api/src/lib/validate.ts`. +- Access: `accessibleHosts` / `allowedHost` in `apps/api/src/lib/hosts.ts`. +- Audit events: `audit()` in `apps/api/src/lib/audit.ts`. +- Rate/concurrency limits: `apps/api/src/lib/ratelimit.ts`. +- JSON body + write guards: `apps/api/src/lib/http.ts`. +- Shell quoting and host-side snippets: `sq`, `forAnyShell` in `packages/shared/src/index.ts`. +- API/web types: `packages/shared/src/index.ts`. +- Web fetch: `api()` in `apps/web/src/api.ts`. UI: `apps/web/src/components/ui/` + (shadcn) before any native widget. +- Tests: `startApp`, `startMockIdp` in `apps/api/test/helpers/`. + +## Never cut + +Anything SPEC.md §3, §5 or §6 requires, or the user asked for: auth and +access-map checks, validation at trust boundaries, rate limits, security +headers, audit events, tests security relies on, error handling that +prevents data loss, accessibility. Don't re-propose SPEC.md §9. From 2e54d11b0fd5df4a5bd245501a607e1c347f6d2a Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:51:10 +0100 Subject: [PATCH 15/23] CLAUDE.md auto-loads the ponytail skill --- CLAUDE.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 527613c..0d57236 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,3 +18,7 @@ - **Run `bun run verify` before saying done** - lint, typecheck, build and tests, same as CI minus `bun audit`. - **Security review** - any change under `apps/api/src/`, `Dockerfile`, `compose*.yaml` or `.github/workflows/` gets a `security-reviewer` agent pass before the PR. - **Never push to `main`** - it is protected by a ruleset (PR required, CI `check` must pass). Branch, push, `gh pr create`; the user merges once green. + +## Coding stance + +@.claude/skills/ponytail/SKILL.md From 0948ea1a45f44e8ba0567766d590c60d86f4873d Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:51:10 +0100 Subject: [PATCH 16/23] Drop the ponytail plugin; the in-repo skill replaces it --- .claude/settings.json | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 946b4c3..0967ef4 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,13 +1 @@ -{ - "enabledPlugins": { - "ponytail@ponytail": true - }, - "extraKnownMarketplaces": { - "ponytail": { - "source": { - "source": "github", - "repo": "DietrichGebert/ponytail" - } - } - } -} +{} From 5727bcebfcafc1da097938cd3fce5af5eb2c7895 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 21:51:10 +0100 Subject: [PATCH 17/23] Skill: implement skills end with a skipped / risk line --- .claude/skills/implement-spec/SKILL.md | 1 + .claude/skills/implement/SKILL.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index 38f3bb9..1773388 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -81,6 +81,7 @@ spec: #<N> <title> · branch: <name> done: #n, #n … · stuck: #n — <why> | none review: fixed <n> · skipped: <finding — why> | none verify: pass · pr: <url> +skipped / risk: <what wasn't done or checked> | none ``` ## Boundaries diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index 8a31d93..a177b13 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -56,6 +56,7 @@ built: <ticket title> branch: <name> · commits: <n> · verify: pass review: <fixed n> · skipped: <finding — why> | none pr: <url> | local only (sec) +skipped / risk: <what wasn't done or checked> | none ``` ## Boundaries From 8656f7f85501176dd3ba15ee3fa14e3a9abb7c1a Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:03:33 +0100 Subject: [PATCH 18/23] Skill: implement handles foreign branches, verify failure, mid-run vulns, push errors --- .claude/skills/implement/SKILL.md | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index a177b13..0e73424 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -3,7 +3,7 @@ name: implement description: > Builds one already-decided piece of CertShell work (a GH issue, a local sec ticket, or the plan agreed in this chat) test-first, reviews it, and opens - a PR. Use when the user says "implement #N", "/implement". + a PR. User-invoked only: `/implement <#N | ticket path | "the plan">`. disable-model-invocation: true --- @@ -29,8 +29,10 @@ release). Same for any issue whose fix turns out to be a vulnerability. ## Steps -1. **Branch.** On `main` → `git fetch -q origin && git switch -c <type>/<slug> origin/main` - (`fix/`, `feat/`, `docs/`, `skills/` …). Already on a feature branch → stay. +1. **Branch.** `git fetch -q origin` first. On `main` → + `git switch -c <type>/<slug> origin/main` (`fix/`, `feat/`, `docs/`, + `skills/` …). On a branch already made for this ticket → stay. Any other + branch → ask. 2. **Seams.** Read the code the ticket touches. Propose the seams (one line each: what it catches, what it misses) and ask the user once. Seams already written in the ticket → state them and go. @@ -41,21 +43,28 @@ release). Same for any issue whose fix turns out to be a vulnerability. 5. **Review.** Run the `change-review` skill against `origin/main`. Fix every `hard`, `missing`, `wrong` and security finding; fix `judgement` and `unasked` ones unless they contradict the ticket, and list any you skip. - Fixes are extra commits. If they touch `apps/api/src/`, `Dockerfile`, - `compose*.yaml` or `.github/workflows/`, run the `security-reviewer` - agent on just those commits and fix what it finds. -6. **Verify.** `bun run verify` must pass. Don't report done otherwise. -7. **PR** (not for sec tickets): `git push -u origin HEAD`, then + Fixes are extra commits. The Security axis covered the earlier commits; + if the fix commits touch `apps/api/src/`, `Dockerfile`, `compose*.yaml` + or `.github/workflows/`, run the `security-reviewer` agent on just the + fix commits and fix what it finds. +6. **Verify.** `bun run verify` must pass. After 3 failed fix attempts, + stop before pushing: `verify: fail`, and report what fails. +7. **Vuln gate.** If the review, tests or your own reading show the bug is + a vulnerability, switch to sec-ticket rules (local only) and ask the + user before anything is pushed. +8. **PR** (not for sec tickets): `git push -u origin HEAD`, then `gh pr create` with a body: what changed, how it was tested, and - `Closes #N`. Never push to `main`. + `Closes #N` when the input was an issue. Never push to `main`. A PR for + the branch already exists → push, report its URL. Push or `gh` fails → + stop and report the error. ## Output ``` built: <ticket title> -branch: <name> · commits: <n> · verify: pass +branch: <name> · commits: <n> · verify: pass | fail review: <fixed n> · skipped: <finding — why> | none -pr: <url> | local only (sec) +pr: <url> | local only (sec) | not opened: <why> skipped / risk: <what wasn't done or checked> | none ``` From d5244edc3fa1fc7fdc3fbc69e83d82659caff422 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:03:48 +0100 Subject: [PATCH 19/23] Skill: implement-spec serial merger, vuln stop, PR after review, stuck tickets kept --- .claude/skills/implement-spec/SKILL.md | 70 +++++++++++++++----------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index 1773388..33346fc 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -3,8 +3,8 @@ name: implement-spec description: > Builds a whole parent GH issue's sub-issues on one integration branch, running up to 3 implementer sub-agents in parallel worktrees in - `blocked_by` order, then reviews and opens one PR. Use when the user says - "implement-spec #N", "/implement-spec". + `blocked_by` order, then reviews and opens one PR. User-invoked only: + `/implement-spec #N`. disable-model-invocation: true --- @@ -17,13 +17,15 @@ Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). A parent issue `#N` (e.g. the `Audit <date>` parent from `/audit-to-tickets`). -- Tickets: `gh api repos/{owner}/{repo}/issues/<N>/sub_issues`, open ones only. +- Tickets: `gh api --paginate repos/{owner}/{repo}/issues/<N>/sub_issues --jq '.[] | select(.state == "open")'`. - Edges: `gh api repos/{owner}/{repo}/issues/<n>/dependencies/blocked_by` per ticket; fall back to the body's `## Blocked by` section. - Spec: the parent body plus SPEC.md. The tickets are a **task graph**, not a list. The **frontier** is every open -ticket whose blockers are all done on the integration branch. +ticket whose blockers are all done: merged into the integration branch, or a +closed issue. A blocker outside this parent that's still open → report it, +don't start that ticket. A cycle → print it and stop. Every ticket must name its seams (`## Seams` or equivalent). List the tickets without them up front and ask the user to add them or name them @@ -40,11 +42,14 @@ the notes dir. Don't paste what they can read. 1. **Read** every ticket and its edges; print the graph as `#n <title> ← blocked by #m, #k` and the first frontier. -2. **Explore** (optional). One sub-agent reads the code and docs the tickets - need and writes notes to `<scratchpad>/spec-<N>/notes/`. Implementers - read those instead of re-exploring. -3. **Integration branch**: `git fetch -q origin && git switch -c spec/<N>-<slug> origin/main`, - push it. +2. **Explore** when 3+ tickets touch the same files, or the tickets need + docs outside the repo. One sub-agent reads the code and docs and writes + notes to `<scratchpad>/spec-<N>/notes/`. Implementers read those instead + of re-exploring. +3. **Integration branch**: `git fetch -q origin`, then + `git switch -c spec/<N>-<slug> origin/main` and push it. The branch + already exists (a re-run after a stop) → switch to it; tickets already + merged into it count as done. 4. **Implement.** One background sub-agent per frontier ticket, max 3 at a time, each with `isolation: "worktree"`. Each implementer: - checks its worktree is on the integration branch tip, resets onto it if not; @@ -53,26 +58,32 @@ the notes dir. Don't paste what they can read. stop and report, don't invent); - commits per logical change, no attribution lines; - merges the integration tip into its branch, runs `bun run verify`; - - reports: branch, commits, verify result, anything it couldn't do. -5. **Merge.** As each implementer reports, a merger sub-agent merges its - branch into the integration branch, fixing only trivial conflicts - (imports, adjacent hunks), then runs `bun run verify`. A non-trivial - conflict or a failed verify → stop and ask the user. Push. After the - first merge, open a draft PR: `gh pr create --draft`, body lists every - ticket as `Closes #n` plus `Part of #N`. + - reports: branch, commits, verify result, `vuln: yes|no` (does the fix + reveal a vulnerability?), anything it couldn't do. +5. **Merge**, one at a time: a single merger sub-agent; reports that arrive + while it runs wait in a queue. `vuln: yes` → don't merge; stop and ask the + user (sec-ticket rules: nothing about it is pushed). Otherwise it merges + the branch into the integration branch, fixing only trivial conflicts + (imports, adjacent hunks), runs `bun run verify`, and pushes. A + non-trivial conflict or a failed verify → stop and ask the user. 6. **Advance.** Each merge can widen the frontier: launch implementers for - newly unblocked tickets, keeping the max of 3. -7. **Review.** All tickets merged → run the `change-review` skill on the - integration branch against `origin/main`. One implementer sub-agent, in - its own worktree on the integration tip like any ticket, fixes - every `hard`, `missing`, `wrong` and security finding (and the rest - unless they contradict a ticket), commits, runs verify. If its commits - touch `apps/api/src/`, `Dockerfile`, `compose*.yaml` or - `.github/workflows/`, run the `security-reviewer` agent on just those - commits and fix what it finds. The merger merges it as in step 5. -8. **Ready.** `bun run verify` on the integration branch passes → - `gh pr ready`. Never push to `main`; the user merges. -9. **Clean up** every implementer worktree and its local branch. + newly unblocked tickets, keeping the max of 3. Stop when the frontier is + empty; tickets left unmerged are **stuck**. +7. **Review.** Run the `change-review` skill on the integration branch + against `origin/main`. One implementer sub-agent, in its own worktree on + the integration tip like any ticket, fixes every `hard`, `missing`, + `wrong` and security finding (and the rest unless they contradict a + ticket), commits, runs verify. If its commits touch `apps/api/src/`, + `Dockerfile`, `compose*.yaml` or `.github/workflows/`, run the + `security-reviewer` agent on just those commits and fix what it finds. + The merger merges it as in step 5. +8. **PR.** Only now, after review and the security pass (CLAUDE.md: before + the PR). `bun run verify` on the integration branch passes → + `gh pr create`, body: `Part of #N` plus `Closes #n` for each **merged** + ticket only. Any stuck ticket → `--draft`, and the body lists the stuck + ones. Never push to `main`; the user merges. +9. **Clean up** the worktrees and local branches of merged tickets. Keep + stuck tickets' branches and name them in the output. ## Output @@ -80,7 +91,8 @@ the notes dir. Don't paste what they can read. spec: #<N> <title> · branch: <name> done: #n, #n … · stuck: #n — <why> | none review: fixed <n> · skipped: <finding — why> | none -verify: pass · pr: <url> +verify: pass | fail · pr: <url> (ready | draft) | not opened: <why> +kept branches: <branch — #n> | none skipped / risk: <what wasn't done or checked> | none ``` From 21b72f78860742e56e7d676bd9e57cc9a24747db Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:04:08 +0100 Subject: [PATCH 20/23] Skill: change-review pins both SHAs, numbers security findings, read-only briefs --- .claude/skills/change-review/SKILL.md | 57 ++++++++++++++++----------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/.claude/skills/change-review/SKILL.md b/.claude/skills/change-review/SKILL.md index f59e2f2..09c9db7 100644 --- a/.claude/skills/change-review/SKILL.md +++ b/.claude/skills/change-review/SKILL.md @@ -2,13 +2,13 @@ name: change-review description: > Checks a CertShell branch against its issue / SPEC.md and the repo's - CLAUDE.md rules, on separate axes run in parallel sub-agents: Standards - (CLAUDE.md, CONTRIBUTING.md, code smells), Spec (missing, unasked or wrong - vs the issue / SPEC.md), and Security (`security-reviewer`, when sensitive - paths changed). Default base `origin/main`. Use when the user says - "change-review", "/change-review", "does this match the issue", "review - against the spec", "review since X", or when `/implement` or - `/implement-spec` calls it. Reports findings, applies nothing. + CLAUDE.md rules. Use when the user says "change-review", "/change-review", + "does this match the issue", "review against the spec", "review since X", + or when `/implement` or `/implement-spec` calls it. Separate axes in + parallel sub-agents: Standards (CLAUDE.md, CONTRIBUTING.md, code smells), + Spec (missing, unasked or wrong), Security (`security-reviewer`, when + sensitive paths changed). Default base `origin/main`. Not a bug hunt (that's + the built-in `/code-review`). Reports findings, applies nothing. --- Review `git diff <base>...HEAD` on separate axes, side by side: @@ -24,11 +24,13 @@ Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). ## Steps -1. **Pin the base.** Whatever the user named (SHA, branch, tag, `HEAD~3`), - else `origin/main`. `git fetch -q origin`, then `git rev-parse <base>` - must resolve and `git diff <base>...HEAD` must be non-empty; fail here, - not in the sub-agents. If `git status --porcelain` shows changes, say they - aren't reviewed (commit first). Note `git log <base>..HEAD --oneline`. +1. **Pin the range.** Base = whatever the user named (SHA, branch, tag, + `HEAD~3`), else `origin/main`. `git fetch -q origin` (if it fails, warn + and use local refs). Resolve both ends once: `B=$(git merge-base <base> HEAD)`, + `H=$(git rev-parse HEAD)`; every axis reviews `git diff $B..$H`, so all + see the same tree. Empty diff or bad ref → fail here, not in the + sub-agents. If `git status --porcelain` shows changes, continue and list + those paths as not reviewed. Note `git log $B..$H --oneline`. 2. **Find the spec**, in order: - `#N` / `Closes #N` in commit messages or the branch's PR body (`gh pr view --json body`): `gh issue view <N>`, plus its parent @@ -41,20 +43,23 @@ Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). `Dockerfile`, `compose*.yaml` or `.github/workflows/`. 4. **Spawn the axes in parallel**: launch all of them in one message and wait for every report before writing the output. Each prompt carries the - diff command, commit list, its brief below, and its line format from - Output pasted in literally (sub-agents can't see this file). + diff command with both SHAs, commit list, its brief below, its line + format from Output pasted in literally (sub-agents can't see this file), + and: "Read-only: edit, stage and commit nothing." Can't spawn sub-agents + → run the axes one after another yourself and say isolation was reduced. 5. **Report** (see Output). Don't merge, dedupe or re-rank across axes. ## Standards brief Sources: `CLAUDE.md`, `CONTRIBUTING.md`, the neighbouring code's patterns. +The list below is a reminder; where it differs from CLAUDE.md, CLAUDE.md wins. Skip anything Biome (`bun run lint`) or `tsc` enforces. A documented repo rule beats the smells below; where the repo endorses something a smell would flag, drop the smell. Repo rules worth checking by hand: -- Tests come with every feature and fix, in `apps/*/test/` or `apps/web/e2e/`, ending `.test.ts`. +- Tests come with every feature and fix, in `apps/*/test/` or `apps/web/e2e/`, ending `.test.<ext>`. - No `find`/`grep` in tests (Bun `Glob` instead). - Comments only where the next reader would otherwise spend several tool calls; a paragraph justifying a workaround means the code is wrong. - Nothing changed beyond what was asked. @@ -76,13 +81,13 @@ Pass the spec contents (issue body, ticket file, or agreed plan) and SPEC.md. Brief: "Report (a) asked-for requirements missing or partial, (b) behaviour in the diff nobody asked for, (c) requirements that look done but are -wrong. Quote the spec line for each, one line each in the format below. -Under 400 words." +wrong. Quote the spec line for each (`spec: none` for unasked behaviour), +one line each in the format below. Under 400 words." ## Security axis -Launch the `security-reviewer` agent on `<base>...HEAD`. Pass its findings -through as they come. +Launch the `security-reviewer` agent on `$B..$H`. Rewrite each of its +findings into the Security line format, keeping its severity. ## Output @@ -91,15 +96,19 @@ through as they come. <N>. [hard|judgement] <rule or smell>: <defect>. <fix>. [path:line] ## Spec -<N>. [missing|unasked|wrong] <defect>. <fix>. [path:line] (spec: "<quote>") +<N>. [missing|unasked|wrong] <defect>. <fix>. [path:line] (spec: "<quote>" | none) ## Security -<security-reviewer findings | not run: no sensitive paths changed> +<N>. [CRIT|HIGH|MED|LOW] <defect>. <fix>. [path:line] +(or: not run: no sensitive paths changed) ``` -Number findings across the whole report so the user can say "fix 2 and 5". -End with one line per axis: `<axis>: <count> findings, worst: <N>` (or -`clean`). Never pick a winner across axes. +Number findings across the whole report, Security included, so the user +can say "fix 2 and 5". End with one line per axis: +`<axis>: <count> findings, worst: #<N>` (or `clean`), where worst is the +finding number of the most serious one in that axis: `hard` > `judgement`; +`wrong` > `missing` > `unasked`; CRIT > HIGH > MED > LOW. Never pick a +winner across axes. ## Boundaries From 4ba4f6ea7c21d4f95a08dc9df98a1721e4f3a863 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:04:19 +0100 Subject: [PATCH 21/23] Skill: tdd defines done, right-reason red, seam confirmation, full fake list --- .claude/skills/tdd/SKILL.md | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/.claude/skills/tdd/SKILL.md b/.claude/skills/tdd/SKILL.md index 8773be4..07dd0fb 100644 --- a/.claude/skills/tdd/SKILL.md +++ b/.claude/skills/tdd/SKILL.md @@ -25,10 +25,12 @@ A **seam** is the public boundary a test observes: a route **Test only at agreed seams.** Before the first test, list the seams, each with one line on what it catches and what it misses, and get them confirmed: -- Called directly: ask the user once. -- Called from `/implement`: it already confirmed them; state them and go. -- Called from an `/implement-spec` implementer: the seams come from the - ticket. No seams in the ticket → stop and report; don't invent them. +- The invoking prompt or ticket lists seams → they're confirmed; state + them and go (`/implement` confirmed them already; `/implement-spec` + tickets carry them). +- Otherwise, called by the user directly → ask them once. +- Otherwise, inside an `/implement-spec` implementer → stop and report + `BLOCKED: no seams in ticket #<n>`; don't invent them. ## Where tests go @@ -50,10 +52,11 @@ Use the real thing; the helpers already exist: - ssh: swap the command through `startApp`'s terminal deps for a local `sh` that inspects the real cert (`fakeSsh` in `apps/api/test/terminal.test.ts`). - Real target host only for sshd-side behaviour: `apps/api/test/ssh-target.test.ts`, - skipped unless `SSH_TARGET` is set; its line 2 has the compose command. + skipped unless `SSH_TARGET` is set; its header comment has the compose command. -Fake only those boundaries, time (`setSystemTime`) and outbound alerts -(`apps/api/test/fixtures/shoutrrr.ts`). Never mock our own modules. +The only fakes: ssh (above), the OIDC IdP (`startMockIdp`), time +(`setSystemTime`) and outbound alerts (`apps/api/test/fixtures/shoutrrr.ts`). +Never mock our own modules. ## Anti-patterns @@ -68,10 +71,18 @@ Fake only those boundaries, time (`setSystemTime`) and outbound alerts ## Loop rules -- **Red first.** Write one failing test, watch it fail for the right reason, - then write just enough code to pass. +- **Red first.** Write one failing test, watch it fail for the right reason + (its assertion fails, not a compile, import or setup error), then write + just enough code to pass. - **One slice per cycle**: one seam, one test, the minimal code. -- **No refactoring in the loop.** Refactor in the review-fix step, after - `/change-review`. +- **No refactoring in the loop.** Called from `/implement` or + `/implement-spec`: refactor in their review-fix step, after + `/change-review`. Called directly: one refactor pass once everything is + green, keeping it green. - Security behaviour (authz, input checks, headers, rate limits, audit events) gets a test that fails without the check. + +## Done when + +Every agreed seam has a passing test, `bun run verify` passes, and `bun run e2e` +passes if you touched `apps/web/e2e/` or the flows it covers. From 8b002ee4121545a42fbf71fde6bb2b937c80bf5e Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:04:29 +0100 Subject: [PATCH 22/23] Skill: ponytail always on, no re-invoke trigger, precedence and SPEC titles --- .claude/skills/ponytail/SKILL.md | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/.claude/skills/ponytail/SKILL.md b/.claude/skills/ponytail/SKILL.md index f129b8b..f8b1819 100644 --- a/.claude/skills/ponytail/SKILL.md +++ b/.claude/skills/ponytail/SKILL.md @@ -3,10 +3,9 @@ name: ponytail description: > CertShell's coding stance: the smallest change that fully solves the task, reuse before stdlib before dependencies, no speculative code, and a reply a - busy human reads once. Auto-loaded via CLAUDE.md for every session and - sub-agent; invoke it again on any coding task (writing, fixing, - refactoring, choosing a dependency) or when the user says "ponytail", - "be lazy", "simplest solution", "yagni", or complains about bloat. + busy human reads once. Always on: CLAUDE.md imports it into every session + and sub-agent. Invoke it only when the user says "ponytail", "be lazy", + "simplest solution", "yagni", or complains about bloat. --- # Ponytail @@ -14,15 +13,17 @@ description: > Be a lazy senior developer: the best code is the code never written. Solve the whole problem with the least new code. End every reply that changed code with one line: `skipped / risk: <what you skipped or didn't check, and -any risk the user must know>`. +any risk the user must know>`, or `skipped / risk: none`. When a skill +defines its own Output block, the line goes inside it. -Always on. The user saying "normal mode" drops it for the session. +Always on, for every session and sub-agent; there's no off switch. Adapted from [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail) (MIT). ## Precedence -CLAUDE.md and SPEC.md, then the `tdd` skill, then this file. +The user's current request, then the rest of CLAUDE.md and SPEC.md, then +the `tdd` skill, then this file. ## Before you write @@ -51,8 +52,7 @@ Take the first option that fully works: needs, including the callers, tests and fixtures it breaks. - No abstraction, wrapper, option, config or "for later" code nobody asked for. Keep values in the form the platform gives you. Deletion beats - addition. Keep the repo's structure: `routes/` → `lib/` / `ssh/`, shared - types in `packages/shared`. + addition. Keep the repo's structure: `routes/` → `lib/` / `ssh/`. - The shortest working diff wins, once you know everything it must touch. A one-liner that needs decoding isn't short. - Comments: CLAUDE.md's rule. Only the why the code can't show, one line. @@ -60,8 +60,9 @@ Take the first option that fully works: once in the shared code. - Moved or merged code keeps its error handling and validation. - Between options of equal size, take the one correct on edge cases. -- Every change is tested (CLAUDE.md), trivial or not; non-trivial work goes - through `tdd`. +- Every change is tested (CLAUDE.md), trivial or not. New behaviour or a + bug fix goes through `tdd`; a pure rename, move or docs change just keeps + the existing tests green. - A shortcut with a known limit gets `// shortcut: <the limit>, <when to upgrade>`. `/project-audit` treats these as recorded decisions. @@ -81,7 +82,8 @@ Take the first option that fully works: ## Never cut -Anything SPEC.md §3, §5 or §6 requires, or the user asked for: auth and +Anything SPEC.md §3 (Auth model), §5 (App implementation notes) or §6 +(Deployment) requires, or the user asked for: auth and access-map checks, validation at trust boundaries, rate limits, security headers, audit events, tests security relies on, error handling that -prevents data loss, accessibility. Don't re-propose SPEC.md §9. +prevents data loss, accessibility. Don't re-propose SPEC.md §9 (Rejected alternatives). From 125da6c79680c19932a39ed2336fa2849bf2afb8 Mon Sep 17 00:00:00 2001 From: Matthew McConnell <maca134@googlemail.com> Date: Mon, 19 Oct 2026 22:04:46 +0100 Subject: [PATCH 23/23] Skills: full MIT copyright notices for adapted upstream skills --- .claude/skills/change-review/SKILL.md | 4 +++- .claude/skills/implement-spec/SKILL.md | 4 +++- .claude/skills/implement/SKILL.md | 4 +++- .claude/skills/ponytail/SKILL.md | 4 +++- .claude/skills/tdd/SKILL.md | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.claude/skills/change-review/SKILL.md b/.claude/skills/change-review/SKILL.md index 09c9db7..ca4e28e 100644 --- a/.claude/skills/change-review/SKILL.md +++ b/.claude/skills/change-review/SKILL.md @@ -20,7 +20,9 @@ Review `git diff <base>...HEAD` on separate axes, side by side: A change can pass one axis and fail another. Each axis runs in its own sub-agent so one can't mask the other. -Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills). +Copyright (c) 2026 Matt Pocock, MIT License: see the upstream +[LICENSE](https://github.com/mattpocock/skills/blob/main/LICENSE). ## Steps diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index 33346fc..187cd15 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -11,7 +11,9 @@ disable-model-invocation: true Turn a parent issue and its sub-issues into one integration branch and one PR, every ticket done. -Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills). +Copyright (c) 2026 Matt Pocock, MIT License: see the upstream +[LICENSE](https://github.com/mattpocock/skills/blob/main/LICENSE). ## Input diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index 0e73424..d7f572b 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -10,7 +10,9 @@ disable-model-invocation: true Build work that's already decided. Don't reopen the plan: no new design, no alternative approach. If the input is unclear, ask; don't redesign. -Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills). +Copyright (c) 2026 Matt Pocock, MIT License: see the upstream +[LICENSE](https://github.com/mattpocock/skills/blob/main/LICENSE). ## Input diff --git a/.claude/skills/ponytail/SKILL.md b/.claude/skills/ponytail/SKILL.md index f8b1819..3be2175 100644 --- a/.claude/skills/ponytail/SKILL.md +++ b/.claude/skills/ponytail/SKILL.md @@ -18,7 +18,9 @@ defines its own Output block, the line goes inside it. Always on, for every session and sub-agent; there's no off switch. -Adapted from [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail) (MIT). +Adapted from [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail). +Copyright (c) 2026 DietrichGebert, MIT License: see the upstream +[LICENSE](https://github.com/DietrichGebert/ponytail/blob/main/LICENSE). ## Precedence diff --git a/.claude/skills/tdd/SKILL.md b/.claude/skills/tdd/SKILL.md index 07dd0fb..14ee793 100644 --- a/.claude/skills/tdd/SKILL.md +++ b/.claude/skills/tdd/SKILL.md @@ -9,7 +9,9 @@ description: > The red → green loop, plus the rules that make its tests worth keeping. -Adapted from [mattpocock/skills](https://github.com/mattpocock/skills) (MIT). +Adapted from [mattpocock/skills](https://github.com/mattpocock/skills). +Copyright (c) 2026 Matt Pocock, MIT License: see the upstream +[LICENSE](https://github.com/mattpocock/skills/blob/main/LICENSE). ## A good test