diff --git a/README.md b/README.md index a4e0de5..f4daa82 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,10 @@ Phase 6 Writes the docs you keep (optional), then cleans up the session The workflow stops at every phase boundary and waits for you. Say `status` to see where a session stands, `back` to revisit a phase, `pause` to save and exit, or `abort` to discard. +A complete finished session — every artifact from a real run, unedited — lives +in [`examples/001-api-rate-limiter`](examples/001-api-rate-limiter). Read it to +see what each phase produces before you run one. + ## How it works Six phases run in order. Each one produces artifacts, and each transition waits for your approval. @@ -140,6 +144,8 @@ The skill repository itself: ├── GUIDED-TOUR.md # Named overview that points to SKILL.md ├── phases/01-06 # Instructions per phase ├── templates/ # Output formats (requirement, plan, step, log) +├── examples/ # A complete worked session (001-api-rate-limiter) +├── hooks/ # Optional harness-enforced verify checks └── references/ ├── step-format.md # Self-containment specification ├── research-fanout.md # Subagent dispatch contract @@ -161,9 +167,9 @@ The skill repository itself: - [x] Six-phase workflow with human-gated checkpoints (documentation and cleanup merged into one wrap-up phase) - [x] Self-contained step files and artifact memory - [x] Quality loops in research, planning, and execution -- [ ] Commit a `LICENSE` file -- [ ] Ship a worked example session in the repo -- [ ] Optional hook-enforced checks for the execution verify loop +- [x] Commit a `LICENSE` file +- [x] Ship a worked example session in the repo +- [x] Optional hook-enforced checks for the execution verify loop ## Contributing diff --git a/SKILL.md b/SKILL.md index 0182127..73f4695 100644 --- a/SKILL.md +++ b/SKILL.md @@ -113,3 +113,4 @@ When in doubt, ask the user: *"This looks non-trivial — want me to run Guided- - `phases/01-06` — Phase instructions - `templates/` — Output formats - `references/` — Step format specification, session schema +- `hooks/` — Optional PreToolUse guard enforcing the evidence rule (see `hooks/README.md`) diff --git a/examples/001-api-rate-limiter/execution-log.md b/examples/001-api-rate-limiter/execution-log.md new file mode 100644 index 0000000..68fe5e2 --- /dev/null +++ b/examples/001-api-rate-limiter/execution-log.md @@ -0,0 +1,18 @@ +# Execution Log + +`Verified` records the check that closed the step — the command run and its result, not "looks done." + +| Step | Status | Verified | Started | Completed | Notes | +|------|--------|----------|---------|-----------|-------| +| 001 | complete | `npx jest token-bucket` → 8 passed, 0 failed | 10:05 | 10:31 | Tests written first; initial run failed on missing module as expected | +| 002 | complete | `npx jest --detectOpenHandles` → 51 passed, no open handles | 10:33 | 11:02 | Two discoveries folded back (fake-timer options, `.unref()` on sweep) — see research/notes.md | +| 003 | complete | `npx jest` → 54 passed, 0 failed | 11:04 | 11:27 | Skipped RateLimit-Reset deliberately; noted as follow-up in docs | + +## Final Review + +Full diff reviewed in a fresh context against requirement.md (2 rounds max): + +- Round 1: one correctness gap — `RateLimit-Remaining` was set from + `floor(tokens)` *before* the decrement on the allow branch, off by one + versus the header contract. Fixed; `npx jest` → 54 passed. +- Round 2: no correctness or requirements gaps. Converged, review closed. diff --git a/examples/001-api-rate-limiter/plan.md b/examples/001-api-rate-limiter/plan.md new file mode 100644 index 0000000..4276601 --- /dev/null +++ b/examples/001-api-rate-limiter/plan.md @@ -0,0 +1,44 @@ +# Implementation Plan + +**Session**: 001-api-rate-limiter +**Granularity**: medium +**Total Steps**: 3 + +## Overview + +Build a token-bucket limiter as a standalone module, wrap it in an Express +middleware following the project's factory convention, then add the standard +`RateLimit-*` response headers. Each step lands with its own tests; the bucket +logic is testable without HTTP. + +## Prerequisites + +- [ ] `npx jest` green on main before starting (confirmed: 42 passed) +- [ ] Node 20 toolchain available + +## Steps + +| # | Title | Dependencies | Scope | +|---|-------|--------------|-------| +| 001 | Token-bucket module | - | `src/lib/token-bucket.js`, `tests/token-bucket.test.js` | +| 002 | Rate-limit middleware, wired into the app | 001 | `src/middleware/rate-limit.js`, `src/app.js`, `tests/rate-limit.test.js` | +| 003 | Standard RateLimit headers and Retry-After | 002 | `src/middleware/rate-limit.js`, `tests/rate-limit.test.js` | + +## Dependency Graph + +``` +001 ─► 002 ─► 003 +``` + +## Rollback Strategy + +Each step is one commit on branch `feat/rate-limiter`; roll back a step with +`git revert ` or drop the branch entirely. No migrations, no config +changes outside the repo, so reverting the code fully reverts the feature. + +## Critique + +Reviewed for blocking gaps before execution: 1 gap found and fixed +- Gap: plan tested refill timing against wall-clock `Date.now()`, which makes + the bucket untestable with fake timers → fix: the bucket takes an injectable + `now` function (defaults to `Date.now`), steps 001-002 updated. diff --git a/examples/001-api-rate-limiter/requirement.md b/examples/001-api-rate-limiter/requirement.md new file mode 100644 index 0000000..220fdbd --- /dev/null +++ b/examples/001-api-rate-limiter/requirement.md @@ -0,0 +1,68 @@ +# API Rate Limiter + +## Problem Statement + +The orders-api has no request throttling. One misbehaving client script +recently sent 40 requests per second for an hour and degraded response times +for everyone. We need a per-client rate limit that rejects excess requests +early, before they reach the database. + +## Scope + +### In Scope +- Token-bucket limiter applied to all `/api/*` routes +- Client identity: `X-API-Key` header when present, source IP otherwise +- `429 Too Many Requests` with standard `RateLimit-*` headers and `Retry-After` +- Unit tests for the bucket, integration tests through the HTTP layer + +### Out of Scope +- Distributed rate limiting (Redis or similar) — the API runs as a single instance +- Per-route or per-plan limit tiers — one global limit for now +- Client dashboards or usage reporting +- Changes to authentication + +## Success Criteria + +- [ ] A client staying under 60 requests/minute is never throttled +- [ ] A burst above the bucket capacity gets `429` with `Retry-After` +- [ ] Every response on `/api/*` carries `RateLimit-Limit` and `RateLimit-Remaining` +- [ ] `npx jest` passes with the new tests included +- [ ] Existing endpoints behave unchanged below the limit + +## Context + +### Relevant Files +- `src/app.js` - Express app assembly; middleware order lives here +- `src/routes/orders.js` - the main API surface being protected +- `tests/orders.test.js` - existing supertest patterns to follow + +### Related Systems +- None. The limiter is in-process; no external store. + +## Constraints + +- Node 20, Express 4, Jest + supertest already in the project — no new runtime dependencies +- Must not add measurable latency to requests under the limit +- Memory bounded: idle client buckets must be evicted + +## Q&A Transcript + +
+Q&A + +**Q:** What are we building or fixing, and why? +**A:** Rate limiting for orders-api; a runaway client degraded the service last week. + +**Q:** Limit per what — API key, IP, or both? +**A:** API key when the header is present, otherwise IP. + +**Q:** What limit and burst? +**A:** 60 requests/minute sustained, bursts up to 60 allowed (bucket capacity 60, refill 1/sec). + +**Q:** External store acceptable? +**A:** No, keep it in-process; we run one instance. + +**Q:** Standard headers? +**A:** Yes, RateLimit-Limit/Remaining/Reset plus Retry-After on 429. + +
diff --git a/examples/001-api-rate-limiter/research/decisions.md b/examples/001-api-rate-limiter/research/decisions.md new file mode 100644 index 0000000..d4d8b6f --- /dev/null +++ b/examples/001-api-rate-limiter/research/decisions.md @@ -0,0 +1,17 @@ +# Decisions + +## Decision: Limiter algorithm and storage + +**Context**: Need per-client throttling with burst tolerance, single-instance API, no new runtime dependencies allowed. +**Options**: A: token bucket in an in-process Map | B: sliding-window counter in Redis | C: fixed-window counter in-process +**Chosen**: A +**Rationale**: Token bucket gives burst-then-sustain semantics that match the stated limit (burst 60, refill 1/sec). Redis is out of scope (single instance, no new dependencies). Fixed windows allow 2x bursts at window edges. + +--- + +## Decision: Idle bucket eviction + +**Context**: An in-process Map grows with every distinct client key; memory must stay bounded. +**Options**: A: sweep on a timer every 10 minutes | B: evict lazily on access when a bucket is full and stale +**Chosen**: A +**Rationale**: Lazy eviction never removes buckets for clients that stop calling. A 10-minute sweep dropping buckets idle for more than 2 minutes keeps the Map proportional to active clients; the sweep is O(entries) over a small set. diff --git a/examples/001-api-rate-limiter/research/express-middleware-patterns.md b/examples/001-api-rate-limiter/research/express-middleware-patterns.md new file mode 100644 index 0000000..9a63dc8 --- /dev/null +++ b/examples/001-api-rate-limiter/research/express-middleware-patterns.md @@ -0,0 +1,39 @@ +# Express Middleware Patterns in orders-api + +**Researched**: 2026-07-05T09:41:00Z +**Source**: codebase + +## Question + +Where does middleware mount in this app, and what conventions do existing +middleware and tests follow? + +## Key Findings + +- `src/app.js` mounts middleware in a fixed order: `express.json()`, then + `requestId`, then the routers under `/api`. A limiter must mount after + `requestId` (so 429s carry a request id) and before the routers. +- Existing middleware lives one file per concern in `src/middleware/` + (`request-id.js`, `error-handler.js`), exported as a factory function + taking an options object: `module.exports = (options = {}) => (req, res, next) => {...}`. +- Tests build the app per suite via `require('../src/app')` and drive it with + supertest; no server listens during tests. Time-sensitive tests use + `jest.useFakeTimers({ doNotFake: ['nextTick'] })` in `tests/orders.test.js:12`. +- Client IP: `app.set('trust proxy', 1)` is already configured, so + `req.ip` is the real client behind the reverse proxy. + +## Sources + +- `src/app.js:14-22` - middleware mount order +- `src/middleware/request-id.js:1` - factory-function convention +- `tests/orders.test.js:8-16` - supertest + fake-timers pattern + +## Relevance + +The limiter becomes `src/middleware/rate-limit.js`, a factory following the +existing convention, mounted between `requestId` and the routers. Tests can +control refill timing with the fake-timers pattern already in use. + +## Open Questions + +- None blocking. Eviction cadence for idle buckets decided in decisions.md. diff --git a/examples/001-api-rate-limiter/research/notes.md b/examples/001-api-rate-limiter/research/notes.md new file mode 100644 index 0000000..a2b2d5a --- /dev/null +++ b/examples/001-api-rate-limiter/research/notes.md @@ -0,0 +1,11 @@ +# Notes + +Running discoveries during execution, folded into the step files they affect. + +- 2026-07-05 10:22 — `jest.useFakeTimers()` without options broke supertest + (requests never resolved because `setImmediate` was faked). The working call + is `jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] })`. + Folded into step-001 and step-002 Validation sections. +- 2026-07-05 10:54 — The eviction sweep timer kept the Jest process alive + after tests. Fixed by calling `.unref()` on the interval; folded into + step-002 Instructions. diff --git a/examples/001-api-rate-limiter/research/summary.md b/examples/001-api-rate-limiter/research/summary.md new file mode 100644 index 0000000..d745172 --- /dev/null +++ b/examples/001-api-rate-limiter/research/summary.md @@ -0,0 +1,15 @@ +# Research Summary + +## Key Findings +- Middleware convention: factory functions in `src/middleware/`, mounted in `src/app.js` between `requestId` and the `/api` routers +- Tests use supertest against the exported app plus `jest.useFakeTimers` for time control — the limiter's refill logic can be tested the same way +- `trust proxy` is already set, so `req.ip` is safe as the fallback client key + +## Decisions Made +| Decision | Choice | Rationale | +|----------|--------|-----------| +| Algorithm and storage | Token bucket, in-process Map | Burst-then-sustain matches the requirement; no new dependencies | +| Idle bucket eviction | Timed sweep every 10 min | Bounded memory even for clients that vanish | + +## Open Questions +- None blocking. diff --git a/examples/001-api-rate-limiter/session.json b/examples/001-api-rate-limiter/session.json new file mode 100644 index 0000000..efd17da --- /dev/null +++ b/examples/001-api-rate-limiter/session.json @@ -0,0 +1,49 @@ +{ + "slug": "001-api-rate-limiter", + "phase": "wrapup", + "status": "complete", + "created": "2026-07-05T09:12:04Z", + "updated": "2026-07-05T11:48:31Z", + "requirement": { + "summary": "Per-client token-bucket rate limiting for the orders-api, keyed by API key with IP fallback, returning 429 with standard RateLimit headers.", + "minimal": false + }, + "research": { + "findings_count": 1, + "decisions_count": 1, + "last_topic": "express-middleware-patterns" + }, + "planning": { + "total_steps": 3, + "steps_generated": 3, + "granularity": "medium" + }, + "execution": { + "current_step": 3, + "completed_steps": [1, 2, 3], + "step_results": { + "1": { + "status": "complete", + "notes": "token-bucket module, 8 tests green", + "started": "2026-07-05T10:05:12Z", + "completed": "2026-07-05T10:31:40Z" + }, + "2": { + "status": "complete", + "notes": "middleware wired before routes, 5 tests green", + "started": "2026-07-05T10:33:02Z", + "completed": "2026-07-05T11:02:19Z" + }, + "3": { + "status": "complete", + "notes": "RateLimit headers + Retry-After, full suite green", + "started": "2026-07-05T11:04:55Z", + "completed": "2026-07-05T11:27:08Z" + } + } + }, + "documentation": { + "type": "feature", + "path": "docs/features/rate-limiting/README.md" + } +} diff --git a/examples/001-api-rate-limiter/steps/step-001.md b/examples/001-api-rate-limiter/steps/step-001.md new file mode 100644 index 0000000..a8ad59c --- /dev/null +++ b/examples/001-api-rate-limiter/steps/step-001.md @@ -0,0 +1,94 @@ +# Step 001: Token-bucket module + +**Session**: 001-api-rate-limiter +**Step**: 1 of 3 +**Dependencies**: none +**Scope**: create `src/lib/token-bucket.js`, create `tests/token-bucket.test.js` + +## Context + +### Requirement Summary +The orders-api needs per-client rate limiting: 60 requests/minute sustained +with bursts up to 60, keyed by API key or IP. Excess requests get `429`. This +step builds only the bucket logic — no HTTP, no Express. + +### Relevant Decisions +- **Algorithm and storage**: Token bucket in an in-process Map — burst-then-sustain + semantics match the requirement (capacity 60, refill 1 token/sec); no new + dependencies allowed. +- **Testability (from plan critique)**: The bucket takes an injectable `now` + function defaulting to `Date.now`, so tests control time without fake timers. + +### Prior Step Outputs +- None. This is the first step. + +### Files to Read +- `tests/orders.test.js` - Jest conventions in this project (describe/it naming, no snapshots) + +### Patterns to Follow +```js +// From src/lib/request-id.js — modules in src/lib export plain functions or +// classes via module.exports, no default-export interop: +module.exports = { generateRequestId }; +``` + +## Task + +### Objective +Implement a token bucket with take/refill semantics and an idle-entry sweep, +driven by an injectable clock. + +### Instructions +1. Create `src/lib/token-bucket.js` exporting a class `TokenBucketMap`: + - `constructor({ capacity = 60, refillPerSecond = 1, now = Date.now } = {})` + - `take(key)` → `{ allowed, remaining, retryAfterSeconds }`. Lazily creates + a bucket per key at full capacity. Refill is continuous: + `tokens = min(capacity, tokens + elapsedSeconds * refillPerSecond)`, + computed from the stored `lastRefill` timestamp on each call. + - When `tokens >= 1`: decrement, `allowed: true`, `remaining: floor(tokens)`. + - When `tokens < 1`: `allowed: false`, `remaining: 0`, + `retryAfterSeconds: ceil((1 - tokens) / refillPerSecond)`. + - `sweep(maxIdleMs = 120000)` → deletes entries whose `lastRefill` is older + than `maxIdleMs`; returns the number evicted. +2. Create `tests/token-bucket.test.js` with a manual fake clock + (`let t = 0; const now = () => t;`) covering: + - a fresh key allows `capacity` consecutive takes, then denies + - a denied take reports `retryAfterSeconds: 1` at refill 1/sec + - advancing the clock 1000ms allows exactly one more take + - tokens never exceed capacity after a long idle period + - `sweep` evicts only entries idle past the threshold + - two keys do not share tokens +3. Write the tests first and run them against an empty module to confirm they + fail for the right reason (missing implementation, not typos), then implement. + +### Expected Output +- `src/lib/token-bucket.js` - the bucket, no Express imports +- `tests/token-bucket.test.js` - 6-8 focused unit tests + +## Validation + +### Commands +```bash +npx jest token-bucket --verbose +npx jest # full suite still green +``` + +### Expected Results +- All new tests pass; suite total rises from 42 passed, 0 failures +- No open handles warning (the module itself starts no timers) + +### Manual Checks +- [ ] `take` never returns negative `remaining` + +## Acceptance Criteria + +- [ ] `npx jest token-bucket` exits 0 with all listed cases covered +- [ ] Full `npx jest` run stays green +- [ ] `src/lib/token-bucket.js` imports nothing from Express or `src/middleware` + +## Rollback + +```bash +git checkout -- src/lib/token-bucket.js tests/token-bucket.test.js 2>/dev/null || \ + rm -f src/lib/token-bucket.js tests/token-bucket.test.js +``` diff --git a/examples/001-api-rate-limiter/steps/step-002.md b/examples/001-api-rate-limiter/steps/step-002.md new file mode 100644 index 0000000..a39243c --- /dev/null +++ b/examples/001-api-rate-limiter/steps/step-002.md @@ -0,0 +1,112 @@ +# Step 002: Rate-limit middleware, wired into the app + +**Session**: 001-api-rate-limiter +**Step**: 2 of 3 +**Dependencies**: 001 +**Scope**: create `src/middleware/rate-limit.js`, create `tests/rate-limit.test.js`, modify `src/app.js` + +## Context + +### Requirement Summary +The orders-api needs per-client rate limiting: 60 requests/minute sustained +with bursts up to 60. Client identity is the `X-API-Key` header when present, +source IP otherwise. Requests over the limit get `429`. This step turns the +step-001 bucket into mounted Express middleware; response headers come in +step 003. + +### Relevant Decisions +- **Algorithm and storage**: Token bucket in an in-process `TokenBucketMap` + (built in step 001) — no external store, single-instance API. +- **Idle bucket eviction**: A sweep every 10 minutes evicts buckets idle for + more than 2 minutes, keeping memory proportional to active clients. + +### Prior Step Outputs +- Step 001 created: `src/lib/token-bucket.js` — `TokenBucketMap` with + `take(key) → { allowed, remaining, retryAfterSeconds }` and `sweep(maxIdleMs)`. + +### Files to Read +- `src/app.js` - current middleware order: `express.json()` → `requestId` → routers +- `src/middleware/request-id.js` - the factory convention to copy + +### Patterns to Follow +```js +// From src/middleware/request-id.js — middleware are factories taking options: +module.exports = (options = {}) => { + return (req, res, next) => { + // ... + next(); + }; +}; +``` + +```js +// From tests/orders.test.js:8-16 — supertest against the exported app, +// fake timers scoped so supertest still resolves: +jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] }); +const request = require('supertest'); +const app = require('../src/app'); +``` + +## Task + +### Objective +Mount a rate-limit middleware on `/api` that rejects over-limit clients with +`429`, keyed by API key with IP fallback. + +### Instructions +1. Create `src/middleware/rate-limit.js` as a factory: + `module.exports = ({ capacity = 60, refillPerSecond = 1, sweepIntervalMs = 600000, now } = {}) => ...` + - Instantiate one `TokenBucketMap` per factory call (not per request). + - Client key: `req.get('x-api-key') || req.ip`. `trust proxy` is already + set in `src/app.js`, so `req.ip` is the real client address. + - On `take(key).allowed === false`: respond + `res.status(429).json({ error: 'rate_limited' })` and do not call `next()`. + - Start the eviction sweep with + `setInterval(() => buckets.sweep(), sweepIntervalMs).unref()` — the + `.unref()` matters: without it the interval keeps Jest (and any short-lived + process) alive after the suite finishes. +2. Mount it in `src/app.js` between `requestId` and the routers: + ```js + const rateLimit = require('./middleware/rate-limit'); + app.use('/api', rateLimit()); + ``` +3. Create `tests/rate-limit.test.js` (write the failing tests first, confirm + they fail because the middleware is absent, then implement): + - 60 requests with one API key all return 200; the 61st returns 429 + - a second API key still gets 200 while the first is throttled + - no API key → keyed by IP: the 61st anonymous request returns 429 + - a throttled key gets 200 again after advancing fake timers 1000ms +4. Build the middleware with an injectable `now` passed through to + `TokenBucketMap`, matching the step-001 design, so tests drive time. + +### Expected Output +- `src/middleware/rate-limit.js` - factory following the project convention +- `src/app.js` - one new `app.use('/api', rateLimit())` line, order preserved +- `tests/rate-limit.test.js` - 4-5 integration tests through supertest + +## Validation + +### Commands +```bash +npx jest rate-limit --verbose +npx jest --detectOpenHandles # full suite; confirms the .unref() sweep leaks nothing +``` + +### Expected Results +- New tests pass; full suite green; no open-handles report + +### Manual Checks +- [ ] `curl -i localhost:3000/api/orders` (dev server) shows 200 and normal body under the limit + +## Acceptance Criteria + +- [ ] `npx jest rate-limit` exits 0 covering the four listed cases +- [ ] Full `npx jest --detectOpenHandles` run is green with no leaked handles +- [ ] `src/app.js` mounts the limiter after `requestId`, before the routers + +## Rollback + +```bash +git checkout -- src/app.js +rm -f src/middleware/rate-limit.js tests/rate-limit.test.js +``` diff --git a/examples/001-api-rate-limiter/steps/step-003.md b/examples/001-api-rate-limiter/steps/step-003.md new file mode 100644 index 0000000..623db94 --- /dev/null +++ b/examples/001-api-rate-limiter/steps/step-003.md @@ -0,0 +1,87 @@ +# Step 003: Standard RateLimit headers and Retry-After + +**Session**: 001-api-rate-limiter +**Step**: 3 of 3 +**Dependencies**: 002 +**Scope**: modify `src/middleware/rate-limit.js`, modify `tests/rate-limit.test.js` + +## Context + +### Requirement Summary +The orders-api rate limiter (built in steps 001-002) must expose its state to +clients: every `/api/*` response carries `RateLimit-Limit` and +`RateLimit-Remaining`, and a `429` additionally carries `Retry-After` so +well-behaved clients can back off instead of retrying blind. + +### Relevant Decisions +- **Algorithm and storage**: Token bucket (`TokenBucketMap`, step 001); + `take(key)` already returns `remaining` and `retryAfterSeconds` — this step + only surfaces them as headers, no bucket changes. + +### Prior Step Outputs +- Step 001 created: `src/lib/token-bucket.js` — `take(key) → { allowed, remaining, retryAfterSeconds }` +- Step 002 created: `src/middleware/rate-limit.js` mounted on `/api` in + `src/app.js`; `tests/rate-limit.test.js` with 4 passing integration tests + +### Files to Read +- `src/middleware/rate-limit.js` - the `take()` call site where headers attach + +### Patterns to Follow +```js +// Headers set before either branch responds, so 200s and 429s both carry them: +const result = buckets.take(key); +res.set('RateLimit-Limit', String(capacity)); +res.set('RateLimit-Remaining', String(result.remaining)); +``` + +## Task + +### Objective +Attach `RateLimit-Limit`/`RateLimit-Remaining` to every limited response and +`Retry-After` to 429s. + +### Instructions +1. In `src/middleware/rate-limit.js`, after `take(key)`: + - always set `RateLimit-Limit: ` and + `RateLimit-Remaining: ` + - on the deny branch, also set `Retry-After: ` + (integer seconds, per RFC 9110) before sending the 429 body +2. Extend `tests/rate-limit.test.js` (failing first, then implement): + - a 200 response carries `RateLimit-Limit: 60` and a numeric + `RateLimit-Remaining` + - `RateLimit-Remaining` decreases by 1 across two consecutive requests + - a 429 response carries `Retry-After: 1` at refill 1/sec +3. Do not add `RateLimit-Reset`: the requirement's success criteria name only + Limit/Remaining plus Retry-After, and the draft-standard Reset semantics + (window vs delay) invite client confusion. Note it as a possible follow-up + in the wrap-up docs instead. + +### Expected Output +- `src/middleware/rate-limit.js` - header lines added, logic otherwise unchanged +- `tests/rate-limit.test.js` - 3 new assertions/tests + +## Validation + +### Commands +```bash +npx jest rate-limit --verbose +npx jest # full suite +``` + +### Expected Results +- All rate-limit tests green, including the three new header cases +- Full suite green + +### Manual Checks +- [ ] `curl -i` against the dev server shows both headers on a normal 200 + +## Acceptance Criteria + +- [ ] `npx jest` exits 0 with the header assertions included +- [ ] A throttled response includes all three headers: Limit, Remaining (0), Retry-After + +## Rollback + +```bash +git checkout -- src/middleware/rate-limit.js tests/rate-limit.test.js +``` diff --git a/examples/README.md b/examples/README.md new file mode 100644 index 0000000..e8d204b --- /dev/null +++ b/examples/README.md @@ -0,0 +1,34 @@ +# Examples + +A complete, finished Guided-Tour session, copied verbatim from a project's +`.claude/workflows/guided-tour/artifacts/` folder after Phase 6 chose "Keep". +Use it to see what the workflow produces before you run it, and as a reference +for the shape each artifact should take. + +## 001-api-rate-limiter + +Adds a token-bucket rate limiter to a small Express API (`orders-api`). The +session ran all six phases: requirements in two rounds, research with one +recorded decision, a three-step plan that survived one critique pass, execution +with a test-first loop per step, and a wrap-up that kept the artifacts. + +Read it in the order the workflow wrote it: + +| Order | File | What to look at | +|-------|------|-----------------| +| 1 | `session.json` | Final state: phase `wrapup`, status `complete` | +| 2 | `requirement.md` | Scope kept small; success criteria are checkable | +| 3 | `research/` | One topic file, one decision, a summary, and `notes.md` from execution | +| 4 | `plan.md` | Step table, dependency graph, and the critique result | +| 5 | `steps/step-001..003.md` | Self-containment: each file repeats the context it needs | +| 6 | `execution-log.md` | Every step closed by a named check, not an assertion | + +The step files are the most useful part. Each one copies the requirement +summary and relevant decisions inline, names exact paths, and shows the +patterns to follow — so a fresh agent with no history can execute the step +from the file alone. That is the standard `references/step-format.md` sets; +these files show what it looks like in practice. + +The target codebase (`orders-api`) is fictional but consistent across all +artifacts: an Express app with `src/app.js`, an orders router, and a +Jest + supertest test suite. diff --git a/hooks/README.md b/hooks/README.md new file mode 100644 index 0000000..dee7aa0 --- /dev/null +++ b/hooks/README.md @@ -0,0 +1,98 @@ +# Hook-enforced checks (optional) + +The execution phase already tells the agent to close a step only on a real, +observed check. That rule lives in prose, so a rushed agent can still write +`complete` into `execution-log.md` on an assertion. This optional hook moves +the rule from prose into the harness: the edit is blocked unless evidence +exists on disk. + +## How it works + +1. `check_step_verified.py` runs as a Claude Code **PreToolUse** hook on every + Edit/Write call. +2. It ignores everything except writes to a file named `execution-log.md`. +3. When the new content marks step `NNN` as `complete`, the hook requires + `.verified/step-NNN` in the session folder (next to the log). Missing + marker → the tool call is blocked (exit 2) and the agent is told to run the + step's Validation command first. +4. The marker is created by chaining it onto the step's real check, so it can + only exist when that check exited 0: + + ```bash + npx jest rate-limit && mkdir -p .verified && touch .verified/step-002 + ``` + + (Paths relative to the session folder; use the session's absolute path when + running from the project root.) + +The hook fails open: malformed input, other files, or log edits that mark +nothing `complete` all pass through. It can block exactly one thing — claiming +completion without a marker — and nothing else. + +## Install + +Per project. Copy the script in and register it: + +```bash +mkdir -p .claude/hooks +cp ~/.claude/skills/guided-tour/hooks/check_step_verified.py .claude/hooks/ +``` + +Add to `.claude/settings.json` (project) or `.claude/settings.local.json`: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "python3 \"$CLAUDE_PROJECT_DIR/.claude/hooks/check_step_verified.py\"", + "timeout": 10 + } + ] + } + ] + } +} +``` + +On Windows, hooks run through `cmd`, so use `python` and `%CLAUDE_PROJECT_DIR%`: + +```json +"command": "python \"%CLAUDE_PROJECT_DIR%\\.claude\\hooks\\check_step_verified.py\"" +``` + +Requires Python 3 on PATH; the script uses only the standard library. On +Windows, `python` may be a Microsoft Store stub that opens the store instead +of running anything — install Python from python.org (or use the `py` +launcher) and verify `python --version` prints a version before enabling the +hook. + +## Test it + +From a project with the hook installed and a session folder in place: + +```bash +# No marker: this hook invocation must exit 2 +echo '{"tool_name":"Edit","tool_input":{"file_path":"/abs/path/to/session/execution-log.md","new_string":"| 001 | complete | test | 10:00 | 10:05 | ok |"}}' \ + | python3 .claude/hooks/check_step_verified.py; echo "exit: $?" + +# With marker: must exit 0 +mkdir -p /abs/path/to/session/.verified && touch /abs/path/to/session/.verified/step-001 +echo '{"tool_name":"Edit","tool_input":{"file_path":"/abs/path/to/session/execution-log.md","new_string":"| 001 | complete | test | 10:00 | 10:05 | ok |"}}' \ + | python3 .claude/hooks/check_step_verified.py; echo "exit: $?" +``` + +## Honest limits + +The hook checks that a marker exists, not that the check truly ran — an agent +could `touch` the marker directly. Phase 5 forbids that, and the marker's +mtime leaves an audit trail, but this is friction plus evidence, not proof. +Keep the final review loop; the hook narrows the gap between "said done" and +"is done", it does not close it alone. + +Markers live inside the session folder, so Phase 6 cleanup removes them with +everything else. diff --git a/hooks/check_step_verified.py b/hooks/check_step_verified.py new file mode 100644 index 0000000..36e1107 --- /dev/null +++ b/hooks/check_step_verified.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +"""PreToolUse guard: block marking a step complete without a verify marker. + +Enforces the Loop Discipline evidence rule mechanically. When the agent edits +an execution-log.md to mark a step ``complete``, this hook requires a marker +file ``.verified/step-{NNN}`` next to the log. The marker is created by +chaining it onto the step's real Validation command, so it exists only when +that command has exited 0: + + npx jest rate-limit && mkdir -p .verified && touch .verified/step-002 + +Fail-open by design: anything unexpected (malformed input, a different file, +no ``complete`` rows in the new content) allows the tool call. The hook only +blocks the one action it understands — writing ``complete`` for a step whose +marker is missing — so it can never wedge unrelated work. + +Install and settings snippet: see hooks/README.md. +""" +import json +import os +import re +import sys + + +def main(): + try: + payload = json.load(sys.stdin) + except Exception: + return 0 # malformed input: never block on our own bug + + tool_input = payload.get("tool_input") or {} + file_path = tool_input.get("file_path") or "" + if os.path.basename(file_path) != "execution-log.md": + return 0 + + # Write sends the whole file as `content`; Edit sends the replacement + # string as `new_string` (older builds: `new_str`). Check all shapes. + new_text = ( + tool_input.get("content") + or tool_input.get("new_string") + or tool_input.get("new_str") + or "" + ) + + # Table rows marking a step complete: | 002 | complete | ... + steps = re.findall(r"\|\s*(\d{3})\s*\|\s*complete\b", new_text) + if not steps: + return 0 + + session_dir = os.path.dirname(os.path.abspath(file_path)) + missing = sorted( + { + step + for step in steps + if not os.path.exists( + os.path.join(session_dir, ".verified", "step-" + step) + ) + } + ) + if not missing: + return 0 + + marker_dir = os.path.join(session_dir, ".verified") + sys.stderr.write( + "Blocked: step(s) {steps} marked complete without a verify marker.\n" + "Run the step's Validation command and let success create the marker, " + "for example:\n" + " && mkdir -p \"{marker_dir}\" && " + "touch \"{marker_dir}/step-{first}\"\n" + "then retry this edit. Do not create the marker without running the " + "check; that defeats the audit trail.\n".format( + steps=", ".join(missing), + marker_dir=marker_dir, + first=missing[0], + ) + ) + return 2 # exit 2 blocks the tool call; stderr is fed back to the agent + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/phases/05-execution.md b/phases/05-execution.md index c0b3ee6..2fae655 100644 --- a/phases/05-execution.md +++ b/phases/05-execution.md @@ -31,6 +31,22 @@ Each step runs as a verify loop, not a single pass. The step is done when its ch 8. Present checkpoint options. 9. Repeat until every step's check has passed. +## Optional Hook Enforcement + +Projects can install the guard hook from the skill's `hooks/` directory (see +`hooks/README.md`). With it enabled, the harness blocks any edit that marks a +step `complete` in `execution-log.md` unless `.verified/step-{NNN}` exists in +the session folder. Create the marker only by chaining it onto the step's real +check, so it exists only when the check exited 0: + +```bash + && mkdir -p {session}/.verified && touch {session}/.verified/step-{NNN} +``` + +Never touch a marker without running the check — that defeats the audit trail +the hook exists to keep. Without the hook installed, nothing changes; the +evidence rule still applies, enforced by discipline instead of the harness. + ## Execution Log Update after each step: