From cc809c6a596615c39cd5ac44a42475e6b3616d79 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 16:27:53 +0200 Subject: [PATCH 1/8] feat(test-drive): add skill to boot a change, prove login, seed it, hand over the keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing a change by hand is mostly setup tax: boot a throwaway app from the branch, wait out a full build, find out whether login even works, work out which screens the diff actually touches, then discover the feature is invisible because nothing in the seeded data exercises it. `explain` deliberately stops at the diff — it says so itself ("not a verification run (a verify skill runs the app)"). `test-drive` is that verify step, aimed at a person rather than a pipeline: boot on a disposable instance, prove auth with a real HTTP round-trip, seed the data that makes the change visible through the app's own API, and hand back a click route with a live URL and credentials. Open-Mercato-aware — it knows `mercato test:ephemeral`, the ready line, the ephemeral state file, and the credentials that env pins — behind a discovery ladder so any other repo degrades to its own documented boot. No browser: login is curl'd, the walkthrough is written. The guardrails are the point. Never write to the database directly (a row inserted behind the app skips validation, events, and indexing). Never seed against a database that isn't the throwaway one. And don't demo a build you can't attribute: the ephemeral command silently attaches to an already-running instance based on file mtimes and a TTL, which after a branch switch hands you a running build of different code while everything looks fine. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 4 +- README.md | 12 ++- skills/test-drive/SKILL.md | 172 +++++++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 skills/test-drive/SKILL.md diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index cc9d3aa..1a3e6a8 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", - "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.0.0", + "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", + "version": "1.1.0", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/README.md b/README.md index 42b0a33..f8f0ce8 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ read what a counterparty changed in a contract. | Skill | Purpose | |-------|---------| | [`explain`](./skills/explain/SKILL.md) | Explain an existing change (working tree / branch / PR / commit) in plain language with a clearly-hedged merge recommendation. Read-only. | +| [`test-drive`](./skills/test-drive/SKILL.md) | `explain`'s hands-on counterpart: boot the change on a throwaway instance → prove login with a real HTTP round-trip → seed the data that makes it visible → hand back a click route with a live URL and credentials. Knows Open Mercato's `mercato test:ephemeral`; degrades to any repo's documented boot. No browser, posts nothing. | | [`brainstorm`](./skills/brainstorm/SKILL.md) | Divergent conversation before any artifact exists: question the idea, weigh alternatives incl. building nothing, reality-check the tracker, survive a fresh-context challenger — then converge on one routed next step (drop it / park as ticket / `kickoff` / `bug-hunt`) with a handoff brief. Read-only until the routing is confirmed. | | [`kickoff`](./skills/kickoff/SKILL.md) | Idea / brainstorm brief / ticket → ready-for-review PR: decides plan depth itself (spec in the repo's spec location, or straight to code), implements with tests, then runs `deliver --no-merge` for checks, PR, reviewer, and the feedback loop. Never merges. | | [`overnight`](./skills/overnight/SKILL.md) | A backlog → a stack of ready-for-review PRs, one per item: classify each item's *decision state*, order them into a dependency graph, batch every open question across every item into one interactive round, then run unattended — each item through `kickoff --base `. A failed item stops its descendants only. Never merges. | @@ -28,7 +29,7 @@ read what a counterparty changed in a contract. | [`bro`](./skills/bro/SKILL.md) | Restate the last message in plain human language — no jargon, one human talking to another. Manual-invoke only. | | [`zoom-out`](./skills/zoom-out/SKILL.md) | Break mid-task tunnel vision: restate the goal from the original request, mark sunk work ignorable, measure the decision space, get a fresh-context second opinion (subagent that never sees the current approach), present 2–3 options-in-kind + a recommendation. Analysis only until the user picks. | -`explain`, `brainstorm`, `kickoff`, `overnight`, `deliver`, `upstream-pr`, `pr-polish`, `ticket-refresh`, `ticket-polish`, `spec-polish`, `review-queue`, `bug-hunt`, `flake-hunt`, `project-status`, `design-polish`, and `design-explore` are **repo-agnostic** — they derive +`explain`, `test-drive`, `brainstorm`, `kickoff`, `overnight`, `deliver`, `upstream-pr`, `pr-polish`, `ticket-refresh`, `ticket-polish`, `spec-polish`, `review-queue`, `bug-hunt`, `flake-hunt`, `project-status`, `design-polish`, and `design-explore` are **repo-agnostic** — they derive project-specific commands, paths, and policy at runtime (see [Skill profile](#skill-profile) below). A repo with its own sharper, hardcoded variant can keep it in its `.claude/skills/` alongside these (plugin skills are namespaced, so they don't collide — see Install). @@ -85,8 +86,8 @@ between the two copies — that's expected, not a bug. ## Skill profile -The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`; `explain` to a -lesser extent) derive most specifics at runtime from the consuming repo's `CLAUDE.md` / +The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`, `test-drive`; +`explain` to a lesser extent) derive most specifics at runtime from the consuming repo's `CLAUDE.md` / `AGENTS.md` / `package.json` scripts. For knobs that aren't derivable from docs, add a **`## Skill profile`** section to the consuming repo's root `CLAUDE.md`. Recognized knobs: @@ -123,6 +124,11 @@ lesser extent) derive most specifics at runtime from the consuming repo's `CLAUD ` and `kickoff --base ` override it, which is how a stacked PR targets its parent instead of the base branch. - **Dev-server / port convention** (e.g. a Conductor worktree port rule) for repro/local runs. +- **Throwaway instance** (`test-drive`) — the command that boots a disposable app + database, + where it records its base URL, and the credentials it guarantees. Open Mercato repos need no + entry: `yarn test:integration:ephemeral:start` → `.ai/qa/ephemeral-env.json` → + `admin@acme.com` / `secret`. Say so explicitly if the only available environment is a + long-lived shared one — `test-drive` then asks before seeding anything into it. - **Status reporting** (`project-status`) — Slack status channel, tracker (Linear team/project IDs and/or Notion database), roadmap source (Linear projects/cycles or a Notion page), and audience (e.g. non-technical business owner). diff --git a/skills/test-drive/SKILL.md b/skills/test-drive/SKILL.md new file mode 100644 index 0000000..4be926e --- /dev/null +++ b/skills/test-drive/SKILL.md @@ -0,0 +1,172 @@ +--- +name: test-drive +description: Boot a change on a throwaway instance, prove login actually works, seed the data the change needs to be visible, then hand over a click-by-click route through it. Use when you want to see a change working before merging — "boot this PR so I can click through it", "set me up to review X by hand", "let me try this". Knows Open Mercato's ephemeral command; degrades to any repo's documented boot. Args: nothing (current worktree), or a PR number/URL to check out first. +--- + +# test-drive + +You are running the **test-drive** skill. Goal: put a running instance of the change in the user's hands, with working credentials, the data that makes the change visible, and a route through it — so they can form their own opinion by clicking, not by reading a diff. + +This is **not** `explain` (which translates a diff into a merge decision without running anything) and **not** an automated-QA skill (which drives a browser and posts pass/fail evidence to a pipeline). You do not drive the UI. You boot, you prove auth, you seed, you write the route — the human does the clicking. + +A URL handed over without a completed login round-trip and a named record to look at is a **failure of this skill**, not a success. "It's running on port 5001" is setup, not a test drive. + +## Project specifics — read these first + +This skill is repo-agnostic, with Open Mercato as its known case. Gather the concrete details from the repository you're running in: + +- **How to boot a throwaway instance** — the command that stands up a disposable app + database, where it records its base URL, and which credentials it guarantees. Open Mercato is resolved in Phase 2; for anything else, derive from the repo's `CLAUDE.md` / `AGENTS.md` / `package.json` scripts, or its **`## Skill profile`** section (the curated source) under the **Throwaway instance** knob. +- **How a changed file maps to a URL** — the route directory convention or a generated route manifest. The click route is *derived* from the diff, so you need this before Phase 4. +- **How records get created through the real path** — the API route convention per module, or the UI form that owns each entity. You need this before Phase 5. + +If a needed value isn't documented and you can't infer it, ask the user rather than guessing. + +## Arguments + +- **Empty** — the current worktree, as it stands. Diff target is the branch against the repo's default branch. +- **A PR number / URL** — `gh pr checkout ` first, and only into a clean tree. A dirty tree is a stop, not a stash: say what's uncommitted and let the user decide. +- **`--fresh`** — never attach to an already-running instance; always build a new one. +- **`--no-seed`** — skip Phase 5 and hand over against whatever data already exists. + +## Hard rules + +1. **Hand over nothing you haven't verified.** Every URL in the click route must have been fetched and returned 200. Every seeded record must have been read back. A route full of plausible-looking URLs you never requested is worse than no route. +2. **Never write to the database directly.** Seed through the real API or the real UI form. A row inserted behind the app skips validation, events, and index/search updates — the app then treats it differently from a real record, and the drive demonstrates something that can't happen in production. +3. **Never seed against a database that isn't the throwaway one.** Before the first write, confirm the base URL is the instance this run booted (or one recorded in the ephemeral state file). If you can't confirm it, stop and ask. Seeding into someone's dev database is not recoverable by apologising. +4. **Don't demo a build you can't attribute.** Prove the running instance was built from the commit under test, or restart it fresh. See the reuse trap in Phase 2 — it is the most likely way this skill lies to the user. +5. **Say what the drive can't show.** A throwaway instance runs with outbound email, the scheduler, and enterprise modules disabled — though queue workers *do* run, so job-backed flows work. A click route that omits that invites the user to conclude a feature is broken when it's merely switched off. +6. **Read-only on source.** Boot, seed, and write the handover — never edit files, commit, push, or merge. `gh pr checkout` with an explicit PR argument is the single exception. +7. **The handover goes to the user.** This skill posts nothing to GitHub, Slack, or a tracker. + +## Phases + +### 1. Resolve the target + +- **PR argument** → verify the tree is clean, `gh pr checkout `, then `gh pr diff ` for the change surface. +- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). + +State the target and the HEAD sha + subject in one line before doing anything expensive, so the user can stop you if you picked the wrong thing. You'll need that sha again in Phase 2 and in the handover. + +### 2. Boot a throwaway instance + +**Check preconditions first**, and report a failure with its fix rather than a stack trace: + +- `node -v` → Open Mercato's ephemeral runner requires Node 24 or newer, and says so with a fix; check it yourself so the user hears it before the build starts, not after. +- `docker info` → must succeed. A non-standard runtime (Colima and friends) is auto-detected from the active Docker context; a dead daemon is not. + +**Warn about the cost before starting, not after.** A cold boot runs the full initialize + production build pipeline: several minutes and RAM-heavy. Tell the user what they're waiting for. + +**Resolve the boot command** — first hit wins, and the last rung is a real probe rather than an assumption: + +1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. This is the Open Mercato monorepo; the script wraps `mercato test:ephemeral`. +2. `yarn mercato test:ephemeral` (equivalently `yarn mercato test ephemeral`). +3. A `create-mercato-app` scaffold — no root ephemeral script, but `@open-mercato/cli` is installed under the app. Probe for the alias before using it: `yarn --cwd apps/mercato exec mercato --help`, then `yarn --cwd apps/mercato exec mercato test:ephemeral`. +4. The repo's own documented boot (`## Skill profile` → **Throwaway instance**, or `CLAUDE.md` / `AGENTS.md`). If that resolves to a **long-lived shared dev stack** rather than a disposable one, say so explicitly and get consent before Phase 5 writes anything (Hard rule 3). + +Useful flags: `--verbose` (full bootstrap/build logs — reach for it the moment a boot fails silently), `--no-reuse-env` (always a brand-new instance on an isolated port), `--no-screenshots` (irrelevant here; this skill doesn't drive a browser). + +**Run it backgrounded.** The command holds the terminal until `Ctrl+C` — that's by design, it's what keeps the instance alive for the user. + +**The reuse trap.** The ephemeral command silently *attaches* to an already-running instance recorded in `.ai/qa/ephemeral-env.json` when source mtimes and a build-cache TTL say it's still valid. After a branch switch or a `gh pr checkout`, that can hand you a running build of **different code** while everything looks fine. Before trusting a reused instance, prove it was built from the commit under test — compare the state file's `startedAt` against the checkout and the working tree. If you can't prove it, pass `--no-reuse-env`. `--fresh` always passes it. + +**Capture the base URL from the ready line**, which is authoritative: + +``` +[ephemeral] Ready for QA exploration at http://127.0.0.1: +[ephemeral] Default credentials: admin@acme.com / secret +``` + +`.ai/qa/ephemeral-env.json` (`baseUrl`, `port`, `databaseUrl`, `startedAt`) is a useful bonus, but it lives at the *project root* — which differs between the monorepo and a scaffold. Find it; don't assume the path. The backend UI is at `/backend`. + +### 3. Prove login — a real round-trip, not a guess + +```bash +curl -s -X POST "$BASE_URL/api/auth/login" \ + -H 'content-type: application/x-www-form-urlencoded' \ + --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' +``` + +Expect 200 and a `token` in the body. Then **use the token** — one authenticated request against a route the change touches: + +```bash +curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" "$BASE_URL/api/" +``` + +A login that mints a token while every subsequent call returns 401 is exactly what this second request catches, and it's a failure the user would otherwise hit on their first click. + +- Credentials the ephemeral env guarantees: `admin@acme.com`, `superadmin@acme.com`, `employee@acme.com`, all with password `secret` (it pins the init passwords). **The guarantee is ephemeral-only** — in a normal environment the companion account passwords are randomly generated, so never present these as universal. +- The login route is rate-limited (a handful of attempts per minute per email). A `429` means back off, not bad credentials — do not loop on it. +- Keep the token for Phase 5. Never put it in the handover. + +### 4. Read the change + +Enough to say what it does in product terms — and, more importantly, to name **the user-visible surfaces it touches**. That list is what the click route is built from. + +- Derive routes from the changed files using the repo's route convention or generated manifest. Don't guess a URL; a 404 in the handover destroys the user's trust in the rest of it. +- Pull the linked ticket if the branch or PR references one — it usually states the change in exactly the user terms you want. +- If the change has **no UI surface** (a worker, a migration, an API-only change), say so plainly and route the drive through the API or CLI instead of inventing a screen. + +For a full merge-decision writeup — scenarios, what's tested, residual risk — that's `explain`. This is the short version that feeds the route. + +### 5. Seed what the change needs + +Skip if `--no-seed`. + +1. **Ask what state makes the change visible.** An order in a particular status, a product with a variant, a customer carrying the new field. The change is only demonstrable against data that exercises it. +2. **Check what already exists first.** Open Mercato's initialize seeds a tenant plus demo customers, catalog, sales, and todos. An existing record that fits beats a new one, and it keeps the click route shorter. (Outside an ephemeral instance, never *assume* demo data is present — verify.) +3. **Create through the real API.** Discover the route from the changed module's own `api/` directory rather than guessing it, then `POST` with the bearer token and `Content-Type: application/json`. +4. **Read every record back** with a separate `GET`. Trusting the create response is how you hand over a record that isn't really there — or one the app can't find because an index never updated. +5. **No API route for it?** Put the UI form into the click route as step 0, with the exact fields to fill. Do not fake it in the database (Hard rule 2). + +For each seeded record, note its **human-visible identifier** (name, number, title) and the **backend URL where the user will find it**. That pairing is what makes the handover clickable rather than a description. + +### 6. Write the handover + +The deliverable. Inline in your final message: + +```markdown +# Test drive: + +## Instance +- URL: /backend — port , built from "" +- Login: admin@acme.com / secret + Verified: POST /api/auth/login → 200; GET → 200 +- Stop it: — this destroys the container and everything seeded below. + +## What the change does +<2–4 sentences in product terms — the situation it addresses and what's different now> + +## Click route +1. → — +2. → <...> +- Worth poking at: + +## Seeded for this drive +| Record | Where to find it | Created via | +|---|---|---| +| | | | + +## What this drive can't show +- +``` + +Writing rules: +- Every URL is one you fetched. Every record is one you read back. +- Say what the user should *see*, not just where to go. "Order ACME-1042 now shows a Partially shipped badge" beats "check the orders list". +- Be honest about what you couldn't set up. A missing step named is useful; a missing step hidden wastes the user's afternoon. + +### 7. Hand over + +Leave the instance running — that's the point of the skill. State plainly how to stop it, and that stopping destroys the container along with everything seeded. If you started it backgrounded, give the user the exact way to kill it. + +## Things to remember + +- A reused instance is someone else's build until you've proven otherwise. +- A 200 from login is not proof of authorization — make one authenticated call. +- Demo data is a starting point, never an assumption. +- If the click route contains no seeded or named record, the drive probably shows nothing. +- The first boot is slow. Warn before, not after. +- A URL you didn't fetch is a guess wearing a link. +- If the change has no screen, say so — don't send the user hunting for one. From 4d2dd6899a13c9d6d4a029b763232263ea6ffe11 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 17:56:51 +0200 Subject: [PATCH 2/8] fix(test-drive): correct the boot sequence against a real Open Mercato PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated the skill end-to-end against fullstackhouse/open-mercato#93. The boot phase was wrong in ways that would have stopped the skill dead on its first run: four consecutive failures before an instance came up, none of which the skill gave any guidance for. The ephemeral command is not a from-zero installer. It assumes a repo already installed and built once, and it runs `initialize` *before* its own codegen and build steps — so a fresh tree, or any checkout that moves the lockfile, fails on missing artifacts with errors naming the symptom rather than the cause: Couldn't find the node_modules state file -> yarn install Cannot find module packages/cli/dist/bin.js -> yarn build:packages Cannot find module .../entities.ids.generated.js -> yarn generate + build:packages The second is a genuine chicken-and-egg: `yarn mercato` *is* the built CLI, so the thing that orchestrates the build must itself be built first. The third presents as a database failure — every migration applies, then bootstrap dies on codegen. Phase 2 now carries the sequence (which the root `build` script already encodes) and an error-to-missing-rung table. Also corrected, both observed rather than reasoned: - `gh pr checkout ` needs an explicit `--repo`. A checkout with a fork alongside its upstream has no default repo set and the bare form errors out. - Never derive the change surface from an unfetched local base ref. The local `develop` here was five weeks stale, turning a 15-file PR into a 4,141-file diff — a wrong answer that looks like a right one. - `--verbose` is not the first response to a failed boot; it adds log volume, not a missing artifact. It earns its place only once the tree is bootstrapped. Phase 4 gains the route mapping this PR demonstrated (backend//page.tsx is /backend/, and detail pages re-export each other, so one changed component surfaces under several routes). Phase 5 gains: read the change's own integration spec first — it is the author's recipe for the state the change needs. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- skills/test-drive/SKILL.md | 37 +++++++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 1a3e6a8..bb5787e 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.1.0", + "version": "1.1.1", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/skills/test-drive/SKILL.md b/skills/test-drive/SKILL.md index 4be926e..0cf807c 100644 --- a/skills/test-drive/SKILL.md +++ b/skills/test-drive/SKILL.md @@ -42,8 +42,8 @@ If a needed value isn't documented and you can't infer it, ask the user rather t ### 1. Resolve the target -- **PR argument** → verify the tree is clean, `gh pr checkout `, then `gh pr diff ` for the change surface. -- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). +- **PR argument** → verify the tree is clean, then `gh pr checkout --repo `. Pass `--repo` explicitly: a checkout with several remotes (a fork alongside its upstream) usually has no default repo set, and bare `gh pr checkout` just errors out. Take the change surface from `gh pr diff `. +- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. State the target and the HEAD sha + subject in one line before doing anything expensive, so the user can stop you if you picked the wrong thing. You'll need that sha again in Phase 2 and in the handover. @@ -56,6 +56,25 @@ State the target and the HEAD sha + subject in one line before doing anything ex **Warn about the cost before starting, not after.** A cold boot runs the full initialize + production build pipeline: several minutes and RAM-heavy. Tell the user what they're waiting for. +**Bootstrap the tree before you run the ephemeral command.** It is not a from-zero installer — it assumes a repo that has already been installed and built once, and it runs `initialize` *before* its own codegen and build steps. On a freshly-installed tree, or after a checkout that changed the lockfile, it fails on missing artifacts with errors that name the symptom and not the cause. In the monorepo the required sequence is exactly what the root `build` script already encodes: + +```bash +yarn install # after any checkout that touches yarn.lock +yarn build:packages # `yarn mercato` *is* packages/cli/dist/bin.js — it must exist to run at all +yarn generate # writes the generated entity registry +yarn build:packages # rebuild so the generated files land in dist/ +``` + +Map the error you get back to the rung you skipped: + +| Error | Missing rung | +|---|---| +| `Couldn't find the node_modules state file` | `yarn install` | +| `Cannot find module '.../packages/cli/dist/bin.js'` | `yarn build:packages` | +| `Cannot find module '.../packages/core/dist/generated/entities.ids.generated.js'` while "Bootstrapping application" | `yarn generate`, then `yarn build:packages` again | + +Note the trap in the third: `initialize` applies every migration successfully and *then* dies, so a codegen problem presents as a database one. Don't reach for `--verbose` on any of these — it adds log volume, not the missing artifact. + **Resolve the boot command** — first hit wins, and the last rung is a real probe rather than an assumption: 1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. This is the Open Mercato monorepo; the script wraps `mercato test:ephemeral`. @@ -63,7 +82,7 @@ State the target and the HEAD sha + subject in one line before doing anything ex 3. A `create-mercato-app` scaffold — no root ephemeral script, but `@open-mercato/cli` is installed under the app. Probe for the alias before using it: `yarn --cwd apps/mercato exec mercato --help`, then `yarn --cwd apps/mercato exec mercato test:ephemeral`. 4. The repo's own documented boot (`## Skill profile` → **Throwaway instance**, or `CLAUDE.md` / `AGENTS.md`). If that resolves to a **long-lived shared dev stack** rather than a disposable one, say so explicitly and get consent before Phase 5 writes anything (Hard rule 3). -Useful flags: `--verbose` (full bootstrap/build logs — reach for it the moment a boot fails silently), `--no-reuse-env` (always a brand-new instance on an isolated port), `--no-screenshots` (irrelevant here; this skill doesn't drive a browser). +Useful flags: `--verbose` (full bootstrap/build logs — worth a re-run once the tree is bootstrapped and a boot still fails silently), `--no-reuse-env` (always a brand-new instance on an isolated port), `--no-screenshots` (irrelevant here; this skill doesn't drive a browser). **Run it backgrounded.** The command holds the terminal until `Ctrl+C` — that's by design, it's what keeps the instance alive for the user. @@ -102,7 +121,8 @@ A login that mints a token while every subsequent call returns 401 is exactly wh Enough to say what it does in product terms — and, more importantly, to name **the user-visible surfaces it touches**. That list is what the click route is built from. -- Derive routes from the changed files using the repo's route convention or generated manifest. Don't guess a URL; a 404 in the handover destroys the user's trust in the rest of it. +- Take the changed-file list from `gh pr diff --name-only` (or a freshly-fetched base), then map each file to a route with the repo's convention. In Open Mercato a module's `backend//page.tsx` is `/backend/`, and detail pages re-export each other — `sales/orders/[id]` renders the `sales/documents/[id]` component, so one changed component surfaces under several routes. +- Don't guess a URL; a 404 in the handover destroys the user's trust in everything else in it. - Pull the linked ticket if the branch or PR references one — it usually states the change in exactly the user terms you want. - If the change has **no UI surface** (a worker, a migration, an API-only change), say so plainly and route the drive through the API or CLI instead of inventing a screen. @@ -113,10 +133,11 @@ For a full merge-decision writeup — scenarios, what's tested, residual risk Skip if `--no-seed`. 1. **Ask what state makes the change visible.** An order in a particular status, a product with a variant, a customer carrying the new field. The change is only demonstrable against data that exercises it. -2. **Check what already exists first.** Open Mercato's initialize seeds a tenant plus demo customers, catalog, sales, and todos. An existing record that fits beats a new one, and it keeps the click route shorter. (Outside an ephemeral instance, never *assume* demo data is present — verify.) -3. **Create through the real API.** Discover the route from the changed module's own `api/` directory rather than guessing it, then `POST` with the bearer token and `Content-Type: application/json`. -4. **Read every record back** with a separate `GET`. Trusting the create response is how you hand over a record that isn't really there — or one the app can't find because an index never updated. -5. **No API route for it?** Put the UI form into the click route as step 0, with the exact fields to fill. Do not fake it in the database (Hard rule 2). +2. **If the change ships an integration or e2e spec, read it first.** It is the author's own recipe for the state the change needs — exact route, exact payload, exact read-back — and reusing it means your seed exercises the path they intended rather than one you invented. +3. **Check what already exists first.** Open Mercato's initialize seeds a tenant plus demo customers, catalog, sales, and todos. An existing record that fits beats a new one, and it keeps the click route shorter. (Outside an ephemeral instance, never *assume* demo data is present — verify.) +4. **Create through the real API.** Discover the route from the changed module's own `api/` directory rather than guessing it, then `POST` with the bearer token and `Content-Type: application/json`. +5. **Read every record back** with a separate `GET`. Trusting the create response is how you hand over a record that isn't really there — or one the app can't find because an index never updated. +6. **No API route for it?** Put the UI form into the click route as step 0, with the exact fields to fill. Do not fake it in the database (Hard rule 2). For each seeded record, note its **human-visible identifier** (name, number, title) and the **backend URL where the user will find it**. That pairing is what makes the handover clickable rather than a description. From 594aac7092d8435a5333e47c1ee8f78b00bf3033 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 18:02:29 +0200 Subject: [PATCH 3/8] fix(test-drive): two more defects found booting PR 93 to a live instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The instance is now up and the full drive completed against fullstackhouse/open-mercato#93, which surfaced two problems the earlier fixes had not. A throwaway instance runs in production mode, and `apps/mercato/.env` ships `JWT_SECRET=change-me-dev-secret` straight from `.env.example` — a value the app's own guard rejects as a published placeholder. The build succeeds, the server starts, and only then does it exit, so the boot reports nothing but "Application process exited before readiness check" while the actual refusal sits in the app's stderr, visible only under --verbose. Phase 2 now names this class and supplies real secrets through the environment rather than editing the user's .env, which Hard rule 6 forbids. The larger correction is to Hard rule 1, which invited an overclaim. It demanded every click-route URL return 200 and every record be read back, and implied that constituted proof the change works. It does not, in two ways observed here: - `/backend` answers 307 anonymously. That is the login redirect behaving correctly, and the old rule would have read it as a broken route. - With the login cookie the order detail page answers 200 with 1.3 MB of HTML that contains none of the seeded values, because the backend is client- rendered and fetches its data after hydration. Grepping that HTML for the change finds nothing even when everything works. So curl can prove the route resolves and the data persisted; it cannot prove the UI paints it. That ceiling is now stated in the hard rule, in Phase 3 alongside the cookie-jar recipe and how to read 307 vs 200, and as the first line of the handover's "what this drive can't show". Verified end to end: instance on :60770 from c361d3caa4, login round-trip 200 with a working token, order ORDER-20260824-00001 seeded via the PR's own spec recipe and read back carrying phone, taxId and taxIdType. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- skills/test-drive/SKILL.md | 36 +++++++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index bb5787e..88eeb83 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.1.1", + "version": "1.1.2", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/skills/test-drive/SKILL.md b/skills/test-drive/SKILL.md index 0cf807c..715eee8 100644 --- a/skills/test-drive/SKILL.md +++ b/skills/test-drive/SKILL.md @@ -30,7 +30,7 @@ If a needed value isn't documented and you can't infer it, ask the user rather t ## Hard rules -1. **Hand over nothing you haven't verified.** Every URL in the click route must have been fetched and returned 200. Every seeded record must have been read back. A route full of plausible-looking URLs you never requested is worse than no route. +1. **Hand over nothing you haven't verified, and don't overclaim what you did verify.** Every URL in the click route must have been fetched *authenticated* and resolved. Every seeded record must have been read back through the API. But know the ceiling of a no-browser drive: you have proven the route resolves and the data persisted — **not** that the UI renders it. Say which of the two you checked; never let a 200 stand in for "the change works". 2. **Never write to the database directly.** Seed through the real API or the real UI form. A row inserted behind the app skips validation, events, and index/search updates — the app then treats it differently from a real record, and the drive demonstrates something that can't happen in production. 3. **Never seed against a database that isn't the throwaway one.** Before the first write, confirm the base URL is the instance this run booted (or one recorded in the ephemeral state file). If you can't confirm it, stop and ask. Seeding into someone's dev database is not recoverable by apologising. 4. **Don't demo a build you can't attribute.** Prove the running instance was built from the commit under test, or restart it fresh. See the reuse trap in Phase 2 — it is the most likely way this skill lies to the user. @@ -75,6 +75,15 @@ Map the error you get back to the rung you skipped: Note the trap in the third: `initialize` applies every migration successfully and *then* dies, so a codegen problem presents as a database one. Don't reach for `--verbose` on any of these — it adds log volume, not the missing artifact. +**A throwaway instance runs in production mode, so dev-safe placeholders become hard failures.** `apps/mercato/.env` ships `JWT_SECRET=change-me-dev-secret` (straight out of `.env.example`), and the app's own production guard refuses to start on a known placeholder secret. The build succeeds, the server starts, and *then* it exits — the boot reports only `Application process exited before readiness check`, with the actual refusal buried in the app's stderr where you'll only see it under `--verbose`. Supply real secrets for the run instead: + +```bash +JWT_SECRET=$(openssl rand -hex 32) AUTH_SECRET=$(openssl rand -hex 32) \ + yarn mercato test:ephemeral +``` + +Pass them in the environment; **do not edit the repo's `.env`** — it's the user's file and this skill is read-only on the tree (Hard rule 6). Treat any "exited before readiness" as this class of problem until `--verbose` proves otherwise: the app process failing *after* a clean build is a configuration refusal far more often than a code fault. + **Resolve the boot command** — first hit wins, and the last rung is a real probe rather than an assumption: 1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. This is the Open Mercato monorepo; the script wraps `mercato test:ephemeral`. @@ -113,6 +122,19 @@ curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" "$BAS A login that mints a token while every subsequent call returns 401 is exactly what this second request catches, and it's a failure the user would otherwise hit on their first click. +**Fetching pages, not just the API.** An authenticated page needs the login *cookie*, not the bearer token — grab a jar on login and reuse it: + +```bash +curl -s -c jar.txt -X POST "$BASE_URL/api/auth/login" \ + -H 'content-type: application/x-www-form-urlencoded' \ + --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' -o /dev/null +curl -s -b jar.txt -o /dev/null -w '%{http_code}\n' "$BASE_URL/backend/" +``` + +Read the status codes correctly, or you'll report working things as broken and broken things as fine: +- **`307`/`302` on an anonymous `/backend` fetch is correct** — that's the login redirect, not a failure. +- **`200` with the cookie means the route exists and you're authenticated. It does not mean the change rendered.** The backend is a client-rendered app: the detail page returns over a megabyte of shell HTML and fetches its data afterwards, so grepping that HTML for your seeded values finds nothing even when everything works. Prove the data at the API layer and be explicit in the handover that rendering is the user's job to confirm. + - Credentials the ephemeral env guarantees: `admin@acme.com`, `superadmin@acme.com`, `employee@acme.com`, all with password `secret` (it pins the init passwords). **The guarantee is ephemeral-only** — in a normal environment the companion account passwords are randomly generated, so never present these as universal. - The login route is rate-limited (a handful of attempts per minute per email). A `429` means back off, not bad credentials — do not loop on it. - Keep the token for Phase 5. Never put it in the handover. @@ -168,13 +190,15 @@ The deliverable. Inline in your final message: | | | | ## What this drive can't show -- +- Rendering is unverified — this drive proved the route resolves and the data persisted + through the API, not that the component paints it. That's the first thing to check. +- ``` Writing rules: -- Every URL is one you fetched. Every record is one you read back. +- Every URL is one you fetched authenticated. Every record is one you read back. Neither is a claim about what the screen looks like. - Say what the user should *see*, not just where to go. "Order ACME-1042 now shows a Partially shipped badge" beats "check the orders list". - Be honest about what you couldn't set up. A missing step named is useful; a missing step hidden wastes the user's afternoon. @@ -189,5 +213,7 @@ Leave the instance running — that's the point of the skill. State plainly how - Demo data is a starting point, never an assumption. - If the click route contains no seeded or named record, the drive probably shows nothing. - The first boot is slow. Warn before, not after. +- "Exited before readiness" is the app refusing its own config, not the harness failing. Read its stderr before blaming the change. +- A 200 from a client-rendered page proves routing and auth, nothing about the change. Don't grep the HTML and call it verified. - A URL you didn't fetch is a guess wearing a link. - If the change has no screen, say so — don't send the user hunting for one. From 0235e107a78b7f5570342820a163458a4d55d095 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 18:07:08 +0200 Subject: [PATCH 4/8] refactor(om-test-drive)!: rename test-drive to om-test-drive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill is Open Mercato-first in substance, not just by default: its boot ladder, bootstrap ordering table, credentials, ephemeral state file and config traps are all Mercato specifics earned by running it against a real PR. The name should say so rather than promising a generality the body doesn't carry. The generic fallback stays — a non-Mercato repo still degrades to its own documented boot via the Throwaway instance profile knob — but the skill is no longer listed among the repo-agnostic ones, where the name would contradict the roll-call. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- README.md | 13 +++++++------ skills/{test-drive => om-test-drive}/SKILL.md | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) rename skills/{test-drive => om-test-drive}/SKILL.md (98%) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 88eeb83..28d97bb 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.1.2", + "version": "1.2.0", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/README.md b/README.md index f8f0ce8..530f250 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ read what a counterparty changed in a contract. | Skill | Purpose | |-------|---------| | [`explain`](./skills/explain/SKILL.md) | Explain an existing change (working tree / branch / PR / commit) in plain language with a clearly-hedged merge recommendation. Read-only. | -| [`test-drive`](./skills/test-drive/SKILL.md) | `explain`'s hands-on counterpart: boot the change on a throwaway instance → prove login with a real HTTP round-trip → seed the data that makes it visible → hand back a click route with a live URL and credentials. Knows Open Mercato's `mercato test:ephemeral`; degrades to any repo's documented boot. No browser, posts nothing. | +| [`om-test-drive`](./skills/om-test-drive/SKILL.md) | `explain`'s hands-on counterpart, for Open Mercato: boot the change on a throwaway instance (`mercato test:ephemeral`) → prove login with a real HTTP round-trip → seed the data that makes it visible through the app's own API → hand back a click route with a live URL and credentials. Carries the bootstrap ordering and the production-mode config traps that make a first boot fail. No browser, so it proves routing and data, not rendering — and says so. Posts nothing. | | [`brainstorm`](./skills/brainstorm/SKILL.md) | Divergent conversation before any artifact exists: question the idea, weigh alternatives incl. building nothing, reality-check the tracker, survive a fresh-context challenger — then converge on one routed next step (drop it / park as ticket / `kickoff` / `bug-hunt`) with a handoff brief. Read-only until the routing is confirmed. | | [`kickoff`](./skills/kickoff/SKILL.md) | Idea / brainstorm brief / ticket → ready-for-review PR: decides plan depth itself (spec in the repo's spec location, or straight to code), implements with tests, then runs `deliver --no-merge` for checks, PR, reviewer, and the feedback loop. Never merges. | | [`overnight`](./skills/overnight/SKILL.md) | A backlog → a stack of ready-for-review PRs, one per item: classify each item's *decision state*, order them into a dependency graph, batch every open question across every item into one interactive round, then run unattended — each item through `kickoff --base `. A failed item stops its descendants only. Never merges. | @@ -29,7 +29,7 @@ read what a counterparty changed in a contract. | [`bro`](./skills/bro/SKILL.md) | Restate the last message in plain human language — no jargon, one human talking to another. Manual-invoke only. | | [`zoom-out`](./skills/zoom-out/SKILL.md) | Break mid-task tunnel vision: restate the goal from the original request, mark sunk work ignorable, measure the decision space, get a fresh-context second opinion (subagent that never sees the current approach), present 2–3 options-in-kind + a recommendation. Analysis only until the user picks. | -`explain`, `test-drive`, `brainstorm`, `kickoff`, `overnight`, `deliver`, `upstream-pr`, `pr-polish`, `ticket-refresh`, `ticket-polish`, `spec-polish`, `review-queue`, `bug-hunt`, `flake-hunt`, `project-status`, `design-polish`, and `design-explore` are **repo-agnostic** — they derive +`explain`, `brainstorm`, `kickoff`, `overnight`, `deliver`, `upstream-pr`, `pr-polish`, `ticket-refresh`, `ticket-polish`, `spec-polish`, `review-queue`, `bug-hunt`, `flake-hunt`, `project-status`, `design-polish`, and `design-explore` are **repo-agnostic** — they derive project-specific commands, paths, and policy at runtime (see [Skill profile](#skill-profile) below). A repo with its own sharper, hardcoded variant can keep it in its `.claude/skills/` alongside these (plugin skills are namespaced, so they don't collide — see Install). @@ -86,7 +86,7 @@ between the two copies — that's expected, not a bug. ## Skill profile -The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`, `test-drive`; +The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`; `explain` to a lesser extent) derive most specifics at runtime from the consuming repo's `CLAUDE.md` / `AGENTS.md` / `package.json` scripts. For knobs that aren't derivable from docs, add a **`## Skill profile`** section to the consuming repo's root `CLAUDE.md`. Recognized knobs: @@ -124,11 +124,12 @@ The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`, `t ` and `kickoff --base ` override it, which is how a stacked PR targets its parent instead of the base branch. - **Dev-server / port convention** (e.g. a Conductor worktree port rule) for repro/local runs. -- **Throwaway instance** (`test-drive`) — the command that boots a disposable app + database, +- **Throwaway instance** (`om-test-drive`) — the command that boots a disposable app + database, where it records its base URL, and the credentials it guarantees. Open Mercato repos need no entry: `yarn test:integration:ephemeral:start` → `.ai/qa/ephemeral-env.json` → - `admin@acme.com` / `secret`. Say so explicitly if the only available environment is a - long-lived shared one — `test-drive` then asks before seeding anything into it. + `admin@acme.com` / `secret`. Set it when the skill is pointed at a non-Mercato repo, and say + so explicitly if the only available environment is a long-lived shared one — `om-test-drive` + then asks before seeding anything into it. - **Status reporting** (`project-status`) — Slack status channel, tracker (Linear team/project IDs and/or Notion database), roadmap source (Linear projects/cycles or a Notion page), and audience (e.g. non-technical business owner). diff --git a/skills/test-drive/SKILL.md b/skills/om-test-drive/SKILL.md similarity index 98% rename from skills/test-drive/SKILL.md rename to skills/om-test-drive/SKILL.md index 715eee8..c190e8d 100644 --- a/skills/test-drive/SKILL.md +++ b/skills/om-test-drive/SKILL.md @@ -1,11 +1,11 @@ --- -name: test-drive +name: om-test-drive description: Boot a change on a throwaway instance, prove login actually works, seed the data the change needs to be visible, then hand over a click-by-click route through it. Use when you want to see a change working before merging — "boot this PR so I can click through it", "set me up to review X by hand", "let me try this". Knows Open Mercato's ephemeral command; degrades to any repo's documented boot. Args: nothing (current worktree), or a PR number/URL to check out first. --- -# test-drive +# om-test-drive -You are running the **test-drive** skill. Goal: put a running instance of the change in the user's hands, with working credentials, the data that makes the change visible, and a route through it — so they can form their own opinion by clicking, not by reading a diff. +You are running the **om-test-drive** skill. Goal: put a running instance of the change in the user's hands, with working credentials, the data that makes the change visible, and a route through it — so they can form their own opinion by clicking, not by reading a diff. This is **not** `explain` (which translates a diff into a merge decision without running anything) and **not** an automated-QA skill (which drives a browser and posts pass/fail evidence to a pipeline). You do not drive the UI. You boot, you prove auth, you seed, you write the route — the human does the clicking. From b641941c9b4c59327709cd3e20ba1f076f4d6e50 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 18:27:18 +0200 Subject: [PATCH 5/8] =?UTF-8?q?fix(om-test-drive):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20contradictions,=20ordering,=20and=20false=20promise?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's review landed ten findings; eight were right and are fixed here. Most of them are places where the skill contradicted itself or promised something it could not deliver. Contradictions between the hard rules and the phases: - Hard rule 3 forbade seeding a non-throwaway database while Phase 2 offered to do it "with consent". Consent does not make another developer's database disposable — the shared-stack path now drops to --no-seed or stops. - Hard rule 1 demanded API read-back of every seeded record while Phase 5 allows a UI-form fallback where no API route exists. Read-back is now required where a route exists, and the no-API case must be declared unverified. - Hard rule 6 said "never edit files" while the bootstrap it prescribes runs yarn install, generate and build:packages. That mutation is unavoidable — the instance is built from the tree — so the rule is now scoped to source, and the handover has to report what was installed, generated or rebuilt and which branch the checkout was left on. Ordering and provenance: - Phase 3 checked "a route the change touches" before Phase 4 discovers routes, and a UI-only change may add no API route at all. It now probes a known collection endpoint and re-checks the changed surfaces after Phase 4, accepting whatever success status each returns rather than demanding 200. - The reuse trap told the agent to prove provenance from the state file. The file records startedAt, a port and a database URL and no source SHA, so a process that started after the checkout can still serve stale build output. Provenance is now treated as unprovable: after any branch switch, always --no-reuse-env. - Node/Docker preconditions and the Mercato bootstrap ran before the boot command was resolved, so a non-Mercato repo would be rejected for lacking Node 24. Profile resolution now comes first. Correctness of what the skill tells the user: - The handover claimed stopping always destroys the instance. When the runner attaches to an existing environment, Ctrl+C only detaches and the data survives for its owner. Now conditional. - The handover hardcoded Open Mercato credentials and its switched-off surfaces even on the non-Mercato path, where both would be false. - The cookie jar was written to the working directory. It is a live session credential; it now goes to mktemp under an EXIT trap. - gh pr diff went unscoped while the checkout beside it used --repo. Two findings are not fixed, with evidence: - "The frontmatter is invalid YAML and will be rejected before discovery." PyYAML does reject the unquoted `Args: nothing`, but seven skills already on main carry the identical construct and load fine, so the claimed consequence does not hold. Real but repo-wide and pre-existing; fixing one skill of twenty would just make the set inconsistent. - "Secure cookies won't be sent over http, so the page probe proves nothing." Measured against the live instance: curl 8.7.1 sends both Secure cookies to 127.0.0.1 and the page returns 200, against 307 without the jar. curl treats loopback as a secure context. Noted inline so the next reader doesn't re-derive it. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- skills/om-test-drive/SKILL.md | 76 ++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 28d97bb..0dd92bf 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.2.0", + "version": "1.2.1", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/skills/om-test-drive/SKILL.md b/skills/om-test-drive/SKILL.md index c190e8d..3d1e11c 100644 --- a/skills/om-test-drive/SKILL.md +++ b/skills/om-test-drive/SKILL.md @@ -9,6 +9,8 @@ You are running the **om-test-drive** skill. Goal: put a running instance of the This is **not** `explain` (which translates a diff into a merge decision without running anything) and **not** an automated-QA skill (which drives a browser and posts pass/fail evidence to a pipeline). You do not drive the UI. You boot, you prove auth, you seed, you write the route — the human does the clicking. +**Scope: Open Mercato first, and honestly so.** The boot step degrades to any repo that documents a disposable environment, but the auth and seeding phases are written around Open Mercato's routes. On another stack this skill boots and then needs its profile to supply the auth contract — without that, stop and say so rather than half-driving an app you can't log into. + A URL handed over without a completed login round-trip and a named record to look at is a **failure of this skill**, not a success. "It's running on port 5001" is setup, not a test drive. ## Project specifics — read these first @@ -16,6 +18,7 @@ A URL handed over without a completed login round-trip and a named record to loo This skill is repo-agnostic, with Open Mercato as its known case. Gather the concrete details from the repository you're running in: - **How to boot a throwaway instance** — the command that stands up a disposable app + database, where it records its base URL, and which credentials it guarantees. Open Mercato is resolved in Phase 2; for anything else, derive from the repo's `CLAUDE.md` / `AGENTS.md` / `package.json` scripts, or its **`## Skill profile`** section (the curated source) under the **Throwaway instance** knob. +- **How to authenticate against it** — the login route, its method and payload shape, and whether it returns a bearer token or sets a session cookie. Phases 3 and 5 are written around Open Mercato's `POST /api/auth/login` → `{token}`; **a repo that authenticates differently will boot and then fail every later phase**, so if its profile doesn't document the auth contract, stop and ask rather than guessing at a login route. - **How a changed file maps to a URL** — the route directory convention or a generated route manifest. The click route is *derived* from the diff, so you need this before Phase 4. - **How records get created through the real path** — the API route convention per module, or the UI form that owns each entity. You need this before Phase 5. @@ -30,26 +33,28 @@ If a needed value isn't documented and you can't infer it, ask the user rather t ## Hard rules -1. **Hand over nothing you haven't verified, and don't overclaim what you did verify.** Every URL in the click route must have been fetched *authenticated* and resolved. Every seeded record must have been read back through the API. But know the ceiling of a no-browser drive: you have proven the route resolves and the data persisted — **not** that the UI renders it. Say which of the two you checked; never let a 200 stand in for "the change works". +1. **Hand over nothing you haven't verified, and don't overclaim what you did verify.** Every URL in the click route must have been fetched *authenticated* and resolved. Every seeded record must have been read back through the app — the API when a route exists, and when it doesn't (the UI-form fallback in Phase 5) say plainly that the record is unverified and the user is confirming it themselves. But know the ceiling of a no-browser drive: you have proven the route resolves and the data persisted — **not** that the UI renders it. Say which of the two you checked; never let a 200 stand in for "the change works". 2. **Never write to the database directly.** Seed through the real API or the real UI form. A row inserted behind the app skips validation, events, and index/search updates — the app then treats it differently from a real record, and the drive demonstrates something that can't happen in production. -3. **Never seed against a database that isn't the throwaway one.** Before the first write, confirm the base URL is the instance this run booted (or one recorded in the ephemeral state file). If you can't confirm it, stop and ask. Seeding into someone's dev database is not recoverable by apologising. -4. **Don't demo a build you can't attribute.** Prove the running instance was built from the commit under test, or restart it fresh. See the reuse trap in Phase 2 — it is the most likely way this skill lies to the user. -5. **Say what the drive can't show.** A throwaway instance runs with outbound email, the scheduler, and enterprise modules disabled — though queue workers *do* run, so job-backed flows work. A click route that omits that invites the user to conclude a feature is broken when it's merely switched off. -6. **Read-only on source.** Boot, seed, and write the handover — never edit files, commit, push, or merge. `gh pr checkout` with an explicit PR argument is the single exception. +3. **Never seed against a database that isn't the throwaway one — and consent doesn't change that.** Before the first write, confirm the base URL is the instance this run booted (or one recorded in the ephemeral state file). If it's a long-lived or shared environment, do not offer to seed it "with permission": another developer's database is not disposable just because someone said yes. Drive it read-only with `--no-seed` and say what that costs, or stop. Seeding into someone's dev database is not recoverable by apologising. +4. **Don't demo a build you can't attribute — and you usually can't.** The ephemeral state file records `startedAt`, a port and a database URL, but **no source SHA or build digest**, so a timestamp newer than your checkout is not evidence: a process can start after the checkout and still serve stale build output. Treat provenance as unprovable and force a fresh instance after any branch switch or PR checkout. See the reuse trap in Phase 2 — it is the most likely way this skill lies to the user. +5. **Say what the drive can't show, from the environment you actually booted.** Open Mercato's ephemeral env disables outbound email, the scheduler and enterprise modules, while queue workers *do* run — so job-backed flows work and scheduled ones don't. Those specifics are Open Mercato's, not a property of throwaway instances in general: on any other profile, read the limitations off that environment rather than repeating this list. A click route that omits them invites the user to call a feature broken when it's merely switched off. +6. **Read-only on source — not on build output.** Never edit source, commit, push, or merge. Bootstrapping deliberately writes build artifacts: `yarn install` rewrites `node_modules`, `yarn generate` writes the generated registry, `build:packages` fills `dist/`. That is expected and unavoidable — the instance is built from this tree — but it is a real mutation of the user's checkout, so **tell them in the handover what was installed, generated or rebuilt, and which branch the checkout is left on.** The two genuine exceptions to read-only are `gh pr checkout` with an explicit PR argument, and that bootstrap. Never edit tracked configuration to make a boot work: an `.env` that blocks startup is fixed with environment overrides for the run, not by rewriting the user's file. 7. **The handover goes to the user.** This skill posts nothing to GitHub, Slack, or a tracker. ## Phases ### 1. Resolve the target -- **PR argument** → verify the tree is clean, then `gh pr checkout --repo `. Pass `--repo` explicitly: a checkout with several remotes (a fork alongside its upstream) usually has no default repo set, and bare `gh pr checkout` just errors out. Take the change surface from `gh pr diff `. -- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. +- **PR argument** → verify the tree is clean, then `gh pr checkout --repo `. Pass `--repo` explicitly: a checkout with several remotes (a fork alongside its upstream) usually has no default repo set, and bare `gh pr checkout` just errors out. Take the change surface from `gh pr diff --repo ` — scope that call too, or an unqualified number can resolve a PR in the *other* repo of a fork/upstream pair. +- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. **Include uncommitted work** (`git diff` and `git diff --staged` on top of the branch range): the instance is built from the working tree, so anything staged or unstaged is running in the app you hand over, and a surface derived from committed history alone would omit exactly the code the user is about to click on. State the target and the HEAD sha + subject in one line before doing anything expensive, so the user can stop you if you picked the wrong thing. You'll need that sha again in Phase 2 and in the handover. ### 2. Boot a throwaway instance -**Check preconditions first**, and report a failure with its fix rather than a stack trace: +**Resolve the boot command before checking anything.** The preconditions and the bootstrap below are Open Mercato's, and applying them to a repo that boots some other way would reject a perfectly good environment for lacking Node or Docker. Run the discovery ladder first; only if it lands on the Mercato rungs do the rest of this phase's specifics apply. + +**Then check the preconditions**, reporting each failure with its fix rather than a stack trace: - `node -v` → Open Mercato's ephemeral runner requires Node 24 or newer, and says so with a fix; check it yourself so the user hears it before the build starts, not after. - `docker info` → must succeed. A non-standard runtime (Colima and friends) is auto-detected from the active Docker context; a dead daemon is not. @@ -89,13 +94,17 @@ Pass them in the environment; **do not edit the repo's `.env`** — it's the use 1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. This is the Open Mercato monorepo; the script wraps `mercato test:ephemeral`. 2. `yarn mercato test:ephemeral` (equivalently `yarn mercato test ephemeral`). 3. A `create-mercato-app` scaffold — no root ephemeral script, but `@open-mercato/cli` is installed under the app. Probe for the alias before using it: `yarn --cwd apps/mercato exec mercato --help`, then `yarn --cwd apps/mercato exec mercato test:ephemeral`. -4. The repo's own documented boot (`## Skill profile` → **Throwaway instance**, or `CLAUDE.md` / `AGENTS.md`). If that resolves to a **long-lived shared dev stack** rather than a disposable one, say so explicitly and get consent before Phase 5 writes anything (Hard rule 3). +4. The repo's own documented boot (`## Skill profile` → **Throwaway instance**, or `CLAUDE.md` / `AGENTS.md`). If that resolves to a **long-lived shared dev stack** rather than a disposable one, say so and drop to `--no-seed` for the rest of the run — do not ask for permission to write into it (Hard rule 3). Useful flags: `--verbose` (full bootstrap/build logs — worth a re-run once the tree is bootstrapped and a boot still fails silently), `--no-reuse-env` (always a brand-new instance on an isolated port), `--no-screenshots` (irrelevant here; this skill doesn't drive a browser). **Run it backgrounded.** The command holds the terminal until `Ctrl+C` — that's by design, it's what keeps the instance alive for the user. -**The reuse trap.** The ephemeral command silently *attaches* to an already-running instance recorded in `.ai/qa/ephemeral-env.json` when source mtimes and a build-cache TTL say it's still valid. After a branch switch or a `gh pr checkout`, that can hand you a running build of **different code** while everything looks fine. Before trusting a reused instance, prove it was built from the commit under test — compare the state file's `startedAt` against the checkout and the working tree. If you can't prove it, pass `--no-reuse-env`. `--fresh` always passes it. +**The reuse trap.** The ephemeral command silently *attaches* to an already-running instance recorded in `.ai/qa/ephemeral-env.json` when source mtimes and a build-cache TTL say it's still valid. After a branch switch or a `gh pr checkout`, that can hand you a running build of **different code** while everything looks fine. + +You cannot verify your way out of this: the state file carries `startedAt`, a port and a database URL, but **no source SHA or build digest**, and a process that started after your checkout may still be serving output built before it. So don't try to reason from the timestamp — **after any branch switch or PR checkout, always pass `--no-reuse-env`** (`--fresh` does it for you). Reuse is only safe when you booted the instance yourself, in this run, from this tree. + +One caveat on the escape hatch: `--no-reuse-env` takes the runner's setup lock and will not build a second instance while another ephemeral process holds it. If it blocks, report the lock owner and let the user stop it — don't promise a fresh instance you can't get. **Capture the base URL from the ready line**, which is authoritative: @@ -114,23 +123,36 @@ curl -s -X POST "$BASE_URL/api/auth/login" \ --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' ``` -Expect 200 and a `token` in the body. Then **use the token** — one authenticated request against a route the change touches: +Capture the token and fail loudly if it isn't there — a probe that prints the response and moves on can send `Bearer ` to the next call and still look like it passed: ```bash -curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" "$BASE_URL/api/" +TOKEN=$(curl -s -X POST "$BASE_URL/api/auth/login" \ + -H 'content-type: application/x-www-form-urlencoded' \ + --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' \ + | python3 -c 'import json,sys; print(json.load(sys.stdin).get("token",""))') +[ -n "$TOKEN" ] || { echo "login failed"; exit 1; } ``` -A login that mints a token while every subsequent call returns 401 is exactly what this second request catches, and it's a failure the user would otherwise hit on their first click. +Then **use the token** on a stable, always-present collection endpoint — *not* a route the change touches. Route discovery is Phase 4 and hasn't run yet, and a UI-only change may add no API route at all: + +```bash +curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" "$BASE_URL/api/" +``` + +A login that mints a token while every subsequent call returns 401 is exactly what this catches, and it's a failure the user would otherwise hit on their first click. Re-check the *changed* surfaces once Phase 4 has identified them, and accept whatever success status each one actually returns — a create is a 201, a delete often a 204, and demanding 200 everywhere would fail a working drive. **Fetching pages, not just the API.** An authenticated page needs the login *cookie*, not the bearer token — grab a jar on login and reuse it: ```bash -curl -s -c jar.txt -X POST "$BASE_URL/api/auth/login" \ +JAR=$(mktemp); trap 'rm -f "$JAR"' EXIT # a live session credential — never in the worktree +curl -s -c "$JAR" -X POST "$BASE_URL/api/auth/login" \ -H 'content-type: application/x-www-form-urlencoded' \ --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' -o /dev/null -curl -s -b jar.txt -o /dev/null -w '%{http_code}\n' "$BASE_URL/backend/" +curl -s -b "$JAR" -o /dev/null -w '%{http_code}\n' "$BASE_URL/backend/" ``` +The jar holds a working session for the whole drive. Put it in `mktemp` and delete it on exit — written into the repo it is one `git add -A` away from being published. (The cookies come back flagged `Secure` even on `http://127.0.0.1`; curl sends them anyway because it treats loopback as a secure context, so no HTTPS workaround is needed. If a future curl tightens that, the symptom is a 307 with the jar — not a silent pass.) + Read the status codes correctly, or you'll report working things as broken and broken things as fine: - **`307`/`302` on an anonymous `/backend` fetch is correct** — that's the login redirect, not a failure. - **`200` with the cookie means the route exists and you're authenticated. It does not mean the change rendered.** The backend is a client-rendered app: the detail page returns over a megabyte of shell HTML and fetches its data afterwards, so grepping that HTML for your seeded values finds nothing even when everything works. Prove the data at the API layer and be explicit in the handover that rendering is the user's job to confirm. @@ -171,10 +193,13 @@ The deliverable. Inline in your final message: # Test drive: ## Instance -- URL: /backend — port , built from "" -- Login: admin@acme.com / secret - Verified: POST /api/auth/login → 200; GET → 200 -- Stop it: — this destroys the container and everything seeded below. +- URL: / — port , built from "" +- Login: + Verified: → ; → +- Stop it: . +- Left in your checkout: ## What the change does <2–4 sentences in product terms — the situation it addresses and what's different now> @@ -192,9 +217,10 @@ The deliverable. Inline in your final message: ## What this drive can't show - Rendering is unverified — this drive proved the route resolves and the data persisted through the API, not that the component paints it. That's the first thing to check. -- +- +- ``` Writing rules: @@ -204,11 +230,13 @@ Writing rules: ### 7. Hand over -Leave the instance running — that's the point of the skill. State plainly how to stop it, and that stopping destroys the container along with everything seeded. If you started it backgrounded, give the user the exact way to kill it. +Leave the instance running — that's the point of the skill. Give the exact way to stop it, and describe stopping **as it will actually behave**: if this run booted the instance, stopping destroys the container and everything seeded; if it attached to one already running, `Ctrl+C` merely detaches and the runtime survives for whoever owns it. Telling a user their data was destroyed when it is still live — or the reverse — is a small lie with real consequences. + +Say what the run changed in their checkout, too: the branch it left them on, and whatever the bootstrap installed, generated or rebuilt (Hard rule 6). ## Things to remember -- A reused instance is someone else's build until you've proven otherwise. +- A reused instance is someone else's build, and the state file gives you nothing to prove otherwise. After a branch switch, boot fresh. - A 200 from login is not proof of authorization — make one authenticated call. - Demo data is a starting point, never an assumption. - If the click route contains no seeded or named record, the drive probably shows nothing. From d888c6218caab1fb2bb0b18392502623528a4dfc Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Mon, 24 Aug 2026 18:38:09 +0200 Subject: [PATCH 6/8] =?UTF-8?q?fix(om-test-drive):=20second=20review=20rou?= =?UTF-8?q?nd=20=E2=80=94=20scaffold=20layout,=20untracked=20files,=20fail?= =?UTF-8?q?ing=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight more findings, all correct. The most consequential is one I could not have reasoned my way to and had to go read the template for. The discovery ladder assumed a root `test:integration:ephemeral:start` meant the monorepo. It doesn't: the current `create-mercato-app` template defines that same script, has no `build:packages`, and is laid out flat with no `apps/mercato`. So the ladder mislabelled a scaffold as the monorepo and then prescribed a warm-up step that does not exist there. Bootstrap now branches on layout rather than script name — the doubled `build:packages` dance only applies where the CLI is in-tree and has to build itself; a scaffold gets `yarn install && yarn generate`, because its `mercato` ships built as a dependency and there is no chicken-and-egg. The rest: - Untracked source was invisible to the change surface. `git diff` in every form misses `??` rows, so a brand-new route or component could be live in the built instance while the handover said no such surface existed. - The authorization probe printed a status nobody checked. A 401 could scroll past into a handover claiming login was proven; it now exits non-zero. - `--no-seed` skipped the only steps that find or verify a record, so it could produce a handover with nothing concrete in it. It is now read-only, not verification-free: name an existing record that exercises the change, or say the route ends at an empty state. - The handover attributed the build to a HEAD SHA even though Phase 1 now folds in uncommitted work — on a dirty tree that SHA does not identify what is running, and the template says so. - The production-secret example hardcoded the monorepo's `yarn mercato` rather than whichever command the ladder selected. - Token parsing used `python3`, which is not a prerequisite anywhere; a Node-only machine would read a valid login as a failure. Uses `node` now. - The README knob contradicted the fixed hard rule, still promising the skill asks before seeding a shared environment. It refuses and drives read-only. - The same knob had no field for the auth contract the skill now requires, so a non-Mercato consumer had nowhere to supply it. Four fields now: boot command, URL state file, credentials, auth contract. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- README.md | 17 ++++++++----- skills/om-test-drive/SKILL.md | 47 ++++++++++++++++++++++++----------- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 0dd92bf..dd0d08d 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "fsh", "description": "Full Stack House Claude Code skills (idea brainstorming & kickoff, delivery workflow, bug/flake hunting, change explanation, hands-on test drives, spec/ticket/PR polishing, status reporting, design, document redlines)", - "version": "1.2.1", + "version": "1.2.2", "author": { "name": "Full Stack House", "url": "https://fullstack.house" diff --git a/README.md b/README.md index 530f250..3703e74 100644 --- a/README.md +++ b/README.md @@ -124,12 +124,17 @@ The repo-agnostic skills (`deliver`, `upstream-pr`, `bug-hunt`, `flake-hunt`; ` and `kickoff --base ` override it, which is how a stacked PR targets its parent instead of the base branch. - **Dev-server / port convention** (e.g. a Conductor worktree port rule) for repro/local runs. -- **Throwaway instance** (`om-test-drive`) — the command that boots a disposable app + database, - where it records its base URL, and the credentials it guarantees. Open Mercato repos need no - entry: `yarn test:integration:ephemeral:start` → `.ai/qa/ephemeral-env.json` → - `admin@acme.com` / `secret`. Set it when the skill is pointed at a non-Mercato repo, and say - so explicitly if the only available environment is a long-lived shared one — `om-test-drive` - then asks before seeding anything into it. +- **Throwaway instance** (`om-test-drive`) — how to stand up a disposable app + database, and + how to talk to it. Four fields: the **boot command**; where it **records its base URL**; the + **credentials** it guarantees; and the **auth contract** — login route, method, payload shape, + and whether it returns a bearer token or sets a session cookie. That last field is not + optional on a non-Mercato repo: the skill's verification and seeding phases are written around + Open Mercato's `POST /api/auth/login` → `{token}`, so without it the skill boots and then + stops rather than guessing at a login route. Open Mercato repos need no entry at all + (`yarn test:integration:ephemeral:start` → `.ai/qa/ephemeral-env.json` → `admin@acme.com` / + `secret`). If the only available environment is long-lived or shared, say so here — + `om-test-drive` then refuses to seed it and drives read-only, rather than asking for + permission it shouldn't act on. - **Status reporting** (`project-status`) — Slack status channel, tracker (Linear team/project IDs and/or Notion database), roadmap source (Linear projects/cycles or a Notion page), and audience (e.g. non-technical business owner). diff --git a/skills/om-test-drive/SKILL.md b/skills/om-test-drive/SKILL.md index 3d1e11c..fea2187 100644 --- a/skills/om-test-drive/SKILL.md +++ b/skills/om-test-drive/SKILL.md @@ -29,7 +29,7 @@ If a needed value isn't documented and you can't infer it, ask the user rather t - **Empty** — the current worktree, as it stands. Diff target is the branch against the repo's default branch. - **A PR number / URL** — `gh pr checkout ` first, and only into a clean tree. A dirty tree is a stop, not a stash: say what's uncommitted and let the user decide. - **`--fresh`** — never attach to an already-running instance; always build a new one. -- **`--no-seed`** — skip Phase 5 and hand over against whatever data already exists. +- **`--no-seed`** — don't create anything; drive what's already there. Read-only, **not** verification-free: you still have to find an existing record that exercises the change and name it in the click route. If no such record exists, say so plainly and hand over a route that ends at an empty state — never an unqualified "go and look". ## Hard rules @@ -46,7 +46,7 @@ If a needed value isn't documented and you can't infer it, ask the user rather t ### 1. Resolve the target - **PR argument** → verify the tree is clean, then `gh pr checkout --repo `. Pass `--repo` explicitly: a checkout with several remotes (a fork alongside its upstream) usually has no default repo set, and bare `gh pr checkout` just errors out. Take the change surface from `gh pr diff --repo ` — scope that call too, or an unqualified number can resolve a PR in the *other* repo of a fork/upstream pair. -- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. **Include uncommitted work** (`git diff` and `git diff --staged` on top of the branch range): the instance is built from the working tree, so anything staged or unstaged is running in the app you hand over, and a surface derived from committed history alone would omit exactly the code the user is about to click on. +- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. **Include everything the build will see** — the instance is compiled from the working tree, so the surface must cover `git diff`, `git diff --staged`, *and* untracked source (`git status --porcelain` rows starting `??`). Untracked files are the trap: a brand-new route or component is invisible to every `git diff` form, so the handover would claim the change has no such surface while that surface is live in the app. If untracked source is present, fold it in or stop and say why. State the target and the HEAD sha + subject in one line before doing anything expensive, so the user can stop you if you picked the wrong thing. You'll need that sha again in Phase 2 and in the handover. @@ -61,14 +61,27 @@ State the target and the HEAD sha + subject in one line before doing anything ex **Warn about the cost before starting, not after.** A cold boot runs the full initialize + production build pipeline: several minutes and RAM-heavy. Tell the user what they're waiting for. -**Bootstrap the tree before you run the ephemeral command.** It is not a from-zero installer — it assumes a repo that has already been installed and built once, and it runs `initialize` *before* its own codegen and build steps. On a freshly-installed tree, or after a checkout that changed the lockfile, it fails on missing artifacts with errors that name the symptom and not the cause. In the monorepo the required sequence is exactly what the root `build` script already encodes: +**Bootstrap the tree before you run the ephemeral command.** It is not a from-zero installer — it assumes a repo that has already been installed and built once, and it runs `initialize` *before* its own codegen and build steps. On a freshly-installed tree, or after a checkout that changed the lockfile, it fails on missing artifacts with errors that name the symptom and not the cause. -```bash -yarn install # after any checkout that touches yarn.lock -yarn build:packages # `yarn mercato` *is* packages/cli/dist/bin.js — it must exist to run at all -yarn generate # writes the generated entity registry -yarn build:packages # rebuild so the generated files land in dist/ -``` +**Which bootstrap depends on the layout, not on the script name.** Decide with `jq -e '.scripts["build:packages"]' package.json`: + +- **Monorepo** (`build:packages` at the root, `packages/cli` in-tree). `yarn mercato` *is* `packages/cli/dist/bin.js`, so the CLI that orchestrates the build must itself be built first — hence the doubled step, exactly as the root `build` script encodes it: + + ```bash + yarn install # after any checkout that touches yarn.lock + yarn build:packages # bootstraps the CLI binary itself + yarn generate # writes the generated entity registry + yarn build:packages # rebuild so the generated files land in dist/ + ``` + +- **Standalone scaffold** (flat `create-mercato-app` layout, no `build:packages`, `mercato` resolved from `node_modules/.bin`). There is no chicken-and-egg — the CLI ships built as a dependency — so the sequence is just: + + ```bash + yarn install + yarn generate # `mercato generate` + ``` + + Running the monorepo's sequence here fails on a `build:packages` script that doesn't exist, which is why the layout check comes first. Map the error you get back to the rung you skipped: @@ -84,16 +97,18 @@ Note the trap in the third: `initialize` applies every migration successfully an ```bash JWT_SECRET=$(openssl rand -hex 32) AUTH_SECRET=$(openssl rand -hex 32) \ - yarn mercato test:ephemeral + ``` +Prefix whichever command the ladder actually chose — on a scaffold that's `yarn test:integration:ephemeral:start` or the `yarn --cwd apps/mercato exec …` form, not the monorepo's `yarn mercato`. + Pass them in the environment; **do not edit the repo's `.env`** — it's the user's file and this skill is read-only on the tree (Hard rule 6). Treat any "exited before readiness" as this class of problem until `--verbose` proves otherwise: the app process failing *after* a clean build is a configuration refusal far more often than a code fault. **Resolve the boot command** — first hit wins, and the last rung is a real probe rather than an assumption: -1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. This is the Open Mercato monorepo; the script wraps `mercato test:ephemeral`. +1. Root `package.json` has `test:integration:ephemeral:start` → `yarn test:integration:ephemeral:start`. **Both** the monorepo and a current `create-mercato-app` scaffold define this, so the script's presence tells you nothing about which one you're in — see the layout check below before bootstrapping. 2. `yarn mercato test:ephemeral` (equivalently `yarn mercato test ephemeral`). -3. A `create-mercato-app` scaffold — no root ephemeral script, but `@open-mercato/cli` is installed under the app. Probe for the alias before using it: `yarn --cwd apps/mercato exec mercato --help`, then `yarn --cwd apps/mercato exec mercato test:ephemeral`. +3. An older scaffold laid out under `apps/mercato` with no root script — probe first, then use it: `yarn --cwd apps/mercato exec mercato --help`, then `yarn --cwd apps/mercato exec mercato test:ephemeral`. 4. The repo's own documented boot (`## Skill profile` → **Throwaway instance**, or `CLAUDE.md` / `AGENTS.md`). If that resolves to a **long-lived shared dev stack** rather than a disposable one, say so and drop to `--no-seed` for the rest of the run — do not ask for permission to write into it (Hard rule 3). Useful flags: `--verbose` (full bootstrap/build logs — worth a re-run once the tree is bootstrapped and a boot still fails silently), `--no-reuse-env` (always a brand-new instance on an isolated port), `--no-screenshots` (irrelevant here; this skill doesn't drive a browser). @@ -129,16 +144,19 @@ Capture the token and fail loudly if it isn't there — a probe that prints the TOKEN=$(curl -s -X POST "$BASE_URL/api/auth/login" \ -H 'content-type: application/x-www-form-urlencoded' \ --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' \ - | python3 -c 'import json,sys; print(json.load(sys.stdin).get("token",""))') + | node -pe 'JSON.parse(require("fs").readFileSync(0,"utf8")).token || ""') [ -n "$TOKEN" ] || { echo "login failed"; exit 1; } ``` Then **use the token** on a stable, always-present collection endpoint — *not* a route the change touches. Route discovery is Phase 4 and hasn't run yet, and a UI-only change may add no API route at all: ```bash -curl -s -o /dev/null -w '%{http_code}\n' -H "Authorization: Bearer $TOKEN" "$BASE_URL/api/" +CODE=$(curl -s -o /dev/null -w '%{http_code}' -H "Authorization: Bearer $TOKEN" "$BASE_URL/api/") +case "$CODE" in 2*) ;; *) echo "authorization failed: $CODE"; exit 1;; esac ``` +Check the status, don't just print it: a 401 or 403 that scrolls past is how a run reaches the handover claiming login was proven when it wasn't. + A login that mints a token while every subsequent call returns 401 is exactly what this catches, and it's a failure the user would otherwise hit on their first click. Re-check the *changed* surfaces once Phase 4 has identified them, and accept whatever success status each one actually returns — a create is a 201, a delete often a 204, and demanding 200 everywhere would fail a working drive. **Fetching pages, not just the API.** An authenticated page needs the login *cookie*, not the bearer token — grab a jar on login and reuse it: @@ -194,6 +212,7 @@ The deliverable. Inline in your final message: ## Instance - URL: / — port , built from "" + - Login: Verified: → ; → - Stop it: . Date: Tue, 25 Aug 2026 07:39:29 +0200 Subject: [PATCH 7/8] =?UTF-8?q?fix(om-test-drive):=20third=20review=20roun?= =?UTF-8?q?d=20=E2=80=94=20probe=20failures,=20stale=20refs,=20layout=20pa?= =?UTF-8?q?ths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six more findings, all correct. Two of them are mistakes I introduced while fixing the previous round, which is its own lesson. The worst was self-inflicted: after hitting the placeholder-JWT refusal during validation, I had written "treat any 'exited before readiness' as this class of problem". But that line is the harness's generic symptom, and a startup crash introduced by the change under test prints exactly the same thing — so the instruction told the agent to retry with fresh secrets and, in the case that matters most, hide the very bug it was sent to find. It now requires reading the actual stderr first and applying the override only when the refusal is shown. The subtlest: Phase 1 said to fetch the base and then diffed `...HEAD`. `git fetch origin main` updates `origin/main`, not the local `main`, so the stale-base failure this phase exists to prevent survived its own fix. Now diffs `origin/` and carries that ref into Phase 4. The rest: - The page probe printed its status and continued, unlike the API probe I had just fixed. A 307 (cookie didn't take) or 404 (route gone) would reach a handover whose click route Hard rule 1 claims was resolved. It exits now. - The `.env` recovery hardcoded `apps/mercato/.env`, which does not exist on the flat scaffold layout this skill started supporting last round. - The writing rule and the handover template both claimed every record was read back through the API, contradicting Phase 5's UI-form fallback and Hard rule 1's requirement to mark those unverified. Both are conditional on how the record was created now. Declined, verified rather than argued: "gh pr diff does not provide --name-only". It does — `gh pr diff 45 --repo fullstackhouse/skills --name-only` returns the file list and exits 0 on gh 2.90.0, and this session used it against another PR earlier. Also merges origin/main, which shipped docs-audit at 1.1.0; the manifest keeps both description additions and this branch's higher version. Co-Authored-By: Claude Opus 5 (1M context) --- skills/om-test-drive/SKILL.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/skills/om-test-drive/SKILL.md b/skills/om-test-drive/SKILL.md index fea2187..1a26f6f 100644 --- a/skills/om-test-drive/SKILL.md +++ b/skills/om-test-drive/SKILL.md @@ -46,7 +46,7 @@ If a needed value isn't documented and you can't infer it, ask the user rather t ### 1. Resolve the target - **PR argument** → verify the tree is clean, then `gh pr checkout --repo `. Pass `--repo` explicitly: a checkout with several remotes (a fork alongside its upstream) usually has no default repo set, and bare `gh pr checkout` just errors out. Take the change surface from `gh pr diff --repo ` — scope that call too, or an unqualified number can resolve a PR in the *other* repo of a fork/upstream pair. -- **No argument** → the current branch against the repo default (`git diff ...HEAD`; derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`). **Fetch the base first.** A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. **Include everything the build will see** — the instance is compiled from the working tree, so the surface must cover `git diff`, `git diff --staged`, *and* untracked source (`git status --porcelain` rows starting `??`). Untracked files are the trap: a brand-new route or component is invisible to every `git diff` form, so the handover would claim the change has no such surface while that surface is live in the app. If untracked source is present, fold it in or stop and say why. +- **No argument** → the current branch against the repo default, diffed against the **remote-tracking** ref: `git fetch origin ` then `git diff origin/...HEAD`. Derive the default via `gh repo view --json defaultBranchRef` or `git symbolic-ref refs/remotes/origin/HEAD`, and use that same `origin/` ref again in Phase 4. Fetching updates `origin/`, **not** your local `` branch — diffing the local one is how you get the stale surface even though you just fetched. A local base ref is only as fresh as your last fetch, and a stale one silently turns a 15-file change into a 4,000-file diff — every conclusion drawn after that is wrong. **Include everything the build will see** — the instance is compiled from the working tree, so the surface must cover `git diff`, `git diff --staged`, *and* untracked source (`git status --porcelain` rows starting `??`). Untracked files are the trap: a brand-new route or component is invisible to every `git diff` form, so the handover would claim the change has no such surface while that surface is live in the app. If untracked source is present, fold it in or stop and say why. State the target and the HEAD sha + subject in one line before doing anything expensive, so the user can stop you if you picked the wrong thing. You'll need that sha again in Phase 2 and in the handover. @@ -93,7 +93,7 @@ Map the error you get back to the rung you skipped: Note the trap in the third: `initialize` applies every migration successfully and *then* dies, so a codegen problem presents as a database one. Don't reach for `--verbose` on any of these — it adds log volume, not the missing artifact. -**A throwaway instance runs in production mode, so dev-safe placeholders become hard failures.** `apps/mercato/.env` ships `JWT_SECRET=change-me-dev-secret` (straight out of `.env.example`), and the app's own production guard refuses to start on a known placeholder secret. The build succeeds, the server starts, and *then* it exits — the boot reports only `Application process exited before readiness check`, with the actual refusal buried in the app's stderr where you'll only see it under `--verbose`. Supply real secrets for the run instead: +**A throwaway instance runs in production mode, so dev-safe placeholders become hard failures.** The app `.env` — `apps/mercato/.env` in the monorepo, the root `.env` on a flat scaffold — ships `JWT_SECRET=change-me-dev-secret` straight out of `.env.example`, and the app's own production guard refuses to start on a known placeholder secret. The build succeeds, the server starts, and *then* it exits — the boot reports only `Application process exited before readiness check`, with the actual refusal buried in the app's stderr where you'll only see it under `--verbose`. Supply real secrets for the run instead: ```bash JWT_SECRET=$(openssl rand -hex 32) AUTH_SECRET=$(openssl rand -hex 32) \ @@ -102,7 +102,7 @@ JWT_SECRET=$(openssl rand -hex 32) AUTH_SECRET=$(openssl rand -hex 32) \ Prefix whichever command the ladder actually chose — on a scaffold that's `yarn test:integration:ephemeral:start` or the `yarn --cwd apps/mercato exec …` form, not the monorepo's `yarn mercato`. -Pass them in the environment; **do not edit the repo's `.env`** — it's the user's file and this skill is read-only on the tree (Hard rule 6). Treat any "exited before readiness" as this class of problem until `--verbose` proves otherwise: the app process failing *after* a clean build is a configuration refusal far more often than a code fault. +Pass them in the environment; **do not edit the repo's `.env`** — it's the user's file and this skill is read-only on the tree (Hard rule 6). **Read the error before applying this.** `Application process exited before readiness check` is the harness's generic symptom, and a startup crash introduced *by the change under test* produces exactly the same line — that one is a finding, not an obstacle. So re-run with `--verbose` (or read the app's stderr) and apply the secret override only when the refusal is actually printed. Retrying with fresh credentials on reflex costs another cold boot and, worse, hides the bug you were sent to look at. **Resolve the boot command** — first hit wins, and the last rung is a real probe rather than an assumption: @@ -166,9 +166,12 @@ JAR=$(mktemp); trap 'rm -f "$JAR"' EXIT # a live session credential — nev curl -s -c "$JAR" -X POST "$BASE_URL/api/auth/login" \ -H 'content-type: application/x-www-form-urlencoded' \ --data-urlencode 'email=admin@acme.com' --data-urlencode 'password=secret' -o /dev/null -curl -s -b "$JAR" -o /dev/null -w '%{http_code}\n' "$BASE_URL/backend/" +CODE=$(curl -s -b "$JAR" -o /dev/null -w '%{http_code}' "$BASE_URL/backend/") +case "$CODE" in 2*) ;; *) echo "page probe failed: $CODE"; exit 1;; esac ``` +Fail on this one exactly as on the API probe. A 307 here means the cookie didn't take and a 404 means the route doesn't exist — either would otherwise walk straight into a handover whose click route Hard rule 1 claims was resolved. + The jar holds a working session for the whole drive. Put it in `mktemp` and delete it on exit — written into the repo it is one `git add -A` away from being published. (The cookies come back flagged `Secure` even on `http://127.0.0.1`; curl sends them anyway because it treats loopback as a secure context, so no HTTPS workaround is needed. If a future curl tightens that, the symptom is a 307 with the jar — not a silent pass.) Read the status codes correctly, or you'll report working things as broken and broken things as fine: @@ -234,8 +237,9 @@ The deliverable. Inline in your final message: | | | | ## What this drive can't show -- Rendering is unverified — this drive proved the route resolves and the data persisted - through the API, not that the component paints it. That's the first thing to check. +- Rendering is unverified — this drive proved the route resolves, and . Not that the component paints it. That's the first thing to check. - @@ -243,7 +247,7 @@ The deliverable. Inline in your final message: ``` Writing rules: -- Every URL is one you fetched authenticated. Every record is one you read back. Neither is a claim about what the screen looks like. +- Every URL is one you fetched authenticated. Every API-created record is one you read back; a record created through a UI form is **labelled unverified**, because you didn't watch it save. Neither is a claim about what the screen looks like. - Say what the user should *see*, not just where to go. "Order ACME-1042 now shows a Partially shipped badge" beats "check the orders list". - Be honest about what you couldn't set up. A missing step named is useful; a missing step hidden wastes the user's afternoon. From 2260890928218a32ffd10b7a06f725e0a149f838 Mon Sep 17 00:00:00 2001 From: Jacek Tomaszewski Date: Tue, 25 Aug 2026 07:48:25 +0200 Subject: [PATCH 8/8] fix(om-test-drive): reconcile the named-record rule with --no-seed's empty state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intro declared any handover without a named record a failure of the skill, while --no-seed — rewritten last round to be read-only rather than verification-free — explicitly permits a route that ends at an empty state. An agent could not satisfy both. The rule now holds wherever the environment could have carried such a record, with an honest empty state as the stated exception. Also lets a documented `Throwaway instance` profile short-circuit the discovery ladder. The rungs key on script names a non-Mercato repo can define too, so sniffing before reading the profile risks applying Open Mercato's bootstrap, auth and route assumptions to a stack that shares nothing with it but a script. Co-Authored-By: Claude Opus 5 (1M context) --- skills/om-test-drive/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/skills/om-test-drive/SKILL.md b/skills/om-test-drive/SKILL.md index 1a26f6f..d89be35 100644 --- a/skills/om-test-drive/SKILL.md +++ b/skills/om-test-drive/SKILL.md @@ -11,7 +11,7 @@ This is **not** `explain` (which translates a diff into a merge decision without **Scope: Open Mercato first, and honestly so.** The boot step degrades to any repo that documents a disposable environment, but the auth and seeding phases are written around Open Mercato's routes. On another stack this skill boots and then needs its profile to supply the auth contract — without that, stop and say so rather than half-driving an app you can't log into. -A URL handed over without a completed login round-trip and a named record to look at is a **failure of this skill**, not a success. "It's running on port 5001" is setup, not a test drive. +A URL handed over without a completed login round-trip is a **failure of this skill**, not a success — and so is one whose click route names nothing to look at, whenever the environment could have carried such a record. The single exception is an honest empty state: under `--no-seed`, or where the change genuinely has no record to point at, say so in as many words. "It's running on port 5001" is setup, not a test drive. ## Project specifics — read these first @@ -52,6 +52,8 @@ State the target and the HEAD sha + subject in one line before doing anything ex ### 2. Boot a throwaway instance +**A documented profile wins over sniffing.** If the repo's `## Skill profile` carries a **Throwaway instance** entry, use it and skip the ladder — the rungs below key on script names that a non-Mercato repo can define too, and losing that race means applying Open Mercato's bootstrap, auth and route assumptions to a stack that shares nothing but a script name. Only when no profile is documented should you infer the stack from the ladder. + **Resolve the boot command before checking anything.** The preconditions and the bootstrap below are Open Mercato's, and applying them to a repo that boots some other way would reject a perfectly good environment for lacking Node or Docker. Run the discovery ladder first; only if it lands on the Mercato rungs do the rest of this phase's specifics apply. **Then check the preconditions**, reporting each failure with its fix rather than a stack trace: