diff --git a/CLAUDE.md b/CLAUDE.md index eb38485..d89a936 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,7 +25,7 @@ There is intentionally no `.claude/` self-install in this repo: the plugin is en - **Dogfood the kit**: evolve dobby through its own stages (`/dobby:scope` → … → `/dobby:wrap`) or `/dobby:dispatch` for small fixes. Friction found while doing so is signal — fix the kit, not the workaround. - **Everything in English.** Three skill categories coexist. (1) The work-session **stage** skills (`scope` → … → `wrap`), the **side-path** skills that plug into the flow on demand (`handoff`, `triage`, `map`, `resolve-conflicts`, `wizard`, `teach`, `upgrade`), the worker agents, and their supporting skills are **methodology** — project-agnostic (no references to any specific codebase) but assuming the single **terminal execution host** (with optional cmux enrichment when `CMUX_WORKSPACE_ID` is set) and reaching the `@kvnwolf/dobby` CLI (`bunx dobby` — `env`/`instructions`/`check`/`up`/`down`/`dev`/`db:*`/`update`) for the mechanics it still executes and the instruction catalogue for what it cannot; the kit reaches the running app via the devUrl `dobby env` reports plus a curl liveness check, and drives the UI per environment — `dobby instructions browser`, which under cmux is its own cmux-browser → claude-in-chrome → curl ladder and under Claude Desktop / t3 code is that host's own MCP browser tools; see the statement above). (2) The kit ALSO carries **convention** skills (`data-processing`, `data-fetching`, `module-conventions`) that encode the user's standard application stack (TanStack Start + Drizzle/Neon + Better Auth, the `@/shared` form/data system) and intentionally reference its module file conventions — deep-path imports and the role-based file taxonomy (`{export}.server.ts` / `functions.ts` / `{descriptor}.browser.ts` / `schema.gen.ts`), no barrels. That coupling is deliberate, not a leak to genericize. (3) **Kit self-improvement tooling** (`mark`, `learn`) couples to the *host* — Claude Code session storage (`~/.claude/projects`, `CLAUDE_CODE_SESSION_ID`) — not to any project, and exists to evolve the kit from how it behaved in real field sessions. That host-coupling is intentional and each such `SKILL.md` must label itself as this category so it isn't mistaken for project-agnostic methodology. - **Skills carry NO `model:`/`effort:`; each agent's own frontmatter has one owner.** Skills inherit the interactive SESSION's model/effort; the maintainer chooses the main-thread Architect's intelligence manually for the task. Agent PROMPT BODIES in `plugin/agents/*.md` remain the authoritative role instructions, and their frontmatter is the SOLE source of that role's model/effort — there is no external recipe to mirror or keep in sync: researcher Sonnet/medium, test-author Opus/high, implementor Sonnet/high, reviewer Opus/high, qa Sonnet/medium. There is no normal execute reviewer loop; reviewer remains available for explicit dispatch and missing-work-log safety review. Claude Code's operator-level `CLAUDE_CODE_SUBAGENT_MODEL` may still override subagent pins externally; record that when evaluating a run because it is host control, not a Dobby setting. -- **Namespacing is mandatory**: cross-references between kit pieces are always `/dobby:` and `dobby:`. Bare names only for things outside the plugin. After any rename/addition, grep for bare references. +- **Namespacing is mandatory**: cross-references between kit pieces are always `/dobby:` and `dobby:`. Bare names only for things outside the plugin. The Agent tool's `name` is a separate per-task ADDRESS (`test-author-t` / `implementor-t` / `qa-t` — a `:` is rejected there), never the namespaced id. After any rename/addition, grep for bare references. - **The Architect never implements.** The interactive main thread owns planning, host mechanics, `AskUserQuestion`, persistence, routing, and worker dispatch. It delegates exploration and code changes to workers; a stage skill that makes main grep around or implement code is a regression. - **Spec prefers scoped evidence but keeps the original standalone path.** In a work session, `/dobby:spec` writes only the existing `STATE.md`; for an already-understood standalone task with no state, it may run `dobby state init` before persisting and linting the plan. It never invents missing understanding—when inputs are thin, route to interview/research instead. - **No interruptions mid-flow**: stages run to completion; gates exist only at stage handoffs (Next-step) and plan approval. No unsolicited explanations; teaching is opt-in via `/dobby:teach`. diff --git a/CONTEXT.md b/CONTEXT.md index 3181c31..a5335df 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -31,9 +31,9 @@ The vocabulary of the dobby kit. Use these terms exactly — in skills, agents, - **Design tree** — the dependency structure of a task's open decisions in `/dobby:interview`: every question hangs off the answers or research it depends on, and each answer pushes the **frontier** outward. - **Frontier** — every open decision in the **design tree** whose prerequisites are already settled: everything `/dobby:interview` could ask RIGHT NOW without guessing — what a **round** asks, split across consecutive rounds by the four-question popup cap and by vehicle; the interview closes only when it is empty. - **Round** — one turn of frontier questioning in `/dobby:interview`, homogeneous by vehicle: an `AskUserQuestion` batch of AT MOST four questions with anticipatable options, or a plain-text batch of open-ended ones. -- **Build loop** — the per-task loop that turns a spec into locally verified code: `test-author` (conditional — only when the repo has a test suite AND the spec marked the task test-first) → `implementor` (implements, then runs its own **Exit gate**) → **QA** (proves real behaviour and returns a verdict). A defect QA finds opens a **build conversation** with the implementor, or with the test-author when the failure traces to the test contract rather than the implementation; that conversation is capped at five rounds, after which QA reports the task `needs-human` instead of retrying further. Normal execute never dispatches the reviewer — holistic static review lives at the **External PR review boundary**. Encoded once in the `dobby:execute` skill's `references/build-protocol.md` — the dispatch protocol the Architect follows directly, for every task — and reused by `/dobby:dispatch` and `/dobby:address-review`, which follow that same protocol for a single ad-hoc task instead of a whole plan. +- **Build loop** — the per-task loop that turns a spec into locally verified code: `test-author` (conditional — only when the repo has a test suite AND the spec marked the task test-first) → `implementor` (implements, then runs its own **Exit gate**) → **QA** (proves real behaviour and returns a verdict). A defect QA finds opens a **build conversation** with the implementor, or with the test-author when the failure traces to the test contract rather than the implementation; that conversation is capped at five rounds, after which QA reports the task `needs-human` instead of retrying further. Each task's three workers are dispatched once and stay addressable under `-t` until the task reaches a terminal status, and the Architect prints a per-step status table (one row per task, one emoji per step) after every worker verdict or fix-round message. Normal execute never dispatches the reviewer — holistic static review lives at the **External PR review boundary**. Encoded once in the `dobby:execute` skill's `references/build-protocol.md` — the dispatch protocol the Architect follows directly, for every task — and reused by `/dobby:dispatch` and `/dobby:address-review`, which follow that same protocol for a single ad-hoc task instead of a whole plan. - **QA** — the worker (`dobby:qa`) that runs the build loop's last step: proves a task's real BEHAVIOUR ONLY, never lint, types, build, or the test suite — that mechanical layer is already closed by the **Edit hook** and the implementor's own **Exit gate** before QA is ever dispatched. It drives the browser (following `dobby instructions browser`, the environment's own **instruction catalogue** entry for the UI-verification topic — never `env`, which reports no browser guide) where an app exists, or exercises the artefact directly (a CLI, a skill, a library) where none does, and returns `{pass, failureKind, evidence, verificationKind, findings}`. A genuine defect opens a **build conversation** with the implementor or test-author; QA never edits code itself. _Avoid_: verifier. -- **Build conversation** — the direct sibling `SendMessage` exchange that closes a build-loop failure: QA reaches back to whichever worker can fix it — `dobby:implementor` for a code defect, `dobby:test-author` (describing expected behaviour only, never a code fragment) for a test-contract problem — rather than starting a fresh agent that would re-read everything. QA numbers every message in the text itself (round 1, round 2, …), so the count survives a fresh context on either side; the conversation is capped at five rounds, after which QA stops and reports the task `needs-human`. An environment failure (a dead browser, an expired credential, anything QA can't attribute to the code or the tests) never enters this conversation — it goes straight to the Architect and costs no round. +- **Build conversation** — the direct sibling `SendMessage` exchange that closes a build-loop failure: QA reaches back to whichever worker can fix it — the task's `implementor-t` for a code defect, `test-author-t` (describing expected behaviour only, never a code fragment) for a test-contract problem (the `dobby:` id is the `subagent_type`, never the address) — rather than starting a fresh agent that would re-read everything. QA numbers every message in the text itself (round 1, round 2, …), so the count survives a fresh context on either side; the conversation is capped at five rounds, after which QA stops and reports the task `needs-human`. An environment failure (a dead browser, an expired credential, anything QA can't attribute to the code or the tests) never enters this conversation — it goes straight to the Architect and costs no round; and the fixer messages the reporter back by name to close the round. - **Test-author** — the worker (`dobby:test-author`) that runs the conditional first step of the build loop: it writes a task's tests **from the spec alone, never seeing the implementation**, producing the fixed contract the implementor must satisfy. Blindness to the code is what makes the tests anti-tautological — an independent source of truth. It runs once per task; a **build conversation** round re-implements against the same tests without the test-author touching the implementation itself, the implementor's own **Exit gate** runs them, and the external PR reviewer later judges their static quality in the context of the complete change. - **Green baseline** — the record (`dobby baseline record`, stored at `.dobby/baseline.json`) of which test suites were already failing before a task's work began. The implementor's **Exit gate** consults it (`dobby check --fix --baseline`) so only suites newly red because of this change are reported — a pre-existing failure is exempt, not the implementor's to fix. It replaces hand-written KNOWN-RED exclusions, and an absent record is stated explicitly ("every failing suite counts") rather than passing silently. Distinct from the **gate cache**, which records only proven-GREEN verdicts, never pre-existing red ones. - **Exit gate** — the full `dobby check --fix --baseline` the implementor runs on itself before handing a task to QA, serialised to one implementor at a time across the shared tree (a task's other steps — its test-author step, its implementation, its QA proof — keep running in parallel with its siblings; only the gate itself queues, because it judges the whole tree). This INVERTS the earlier rule that forbade implementors from running any check: the mechanical layer is closed before QA ever sees the task, which is what lets QA prove behaviour only and never re-run it. @@ -44,7 +44,7 @@ The vocabulary of the dobby kit. Use these terms exactly — in skills, agents, - **blocked** — a terminal task state alongside done/needs-human: the task was skipped WITHOUT spawning agents because a direct or transitive dependency ended `needs-human`; the result names the blocker (`blockedBy`) and the task gets no work-log entry. Transitivity is structural — a blocked task is itself non-`done`, so its own dependents block in turn — while every task independent of the dead one keeps running exactly as if nothing had happened. _Avoid_: skipped, cancelled. - **Dispatch** — the lightweight ad-hoc path: a scoped task handed to one worker (or the single-task build loop), no `STATE.md`. - **Prototype** — throwaway code that answers ONE design question, then dies. Two branches: **logic** (a minimal TUI over a pure, portable module) and **UI** (3-5 radically different variants on one route with a floating switcher). -- **Namespacing** — inside the plugin, every cross-reference is fully qualified: `/dobby:` for skills, `dobby:` for `subagent_type`/`agentType`. Bare names are reserved for things OUTSIDE the plugin (`deep-research`, `find-docs`, built-in `Plan`/`Explore`). +- **Namespacing** — inside the plugin, every cross-reference is fully qualified: `/dobby:` for skills, `dobby:` for `subagent_type`/`agentType`. Bare names are reserved for things OUTSIDE the plugin (`deep-research`, `find-docs`, built-in `Plan`/`Explore`). The Agent tool's `name` is a separate, per-task ADDRESS (`-t`; `:` is rejected there), so the namespaced id is never used as a name. - **Session indicator** — the copy-pasteable pointer to a recorded Claude Code session that `/dobby:mark` emits (transcript `.jsonl` path + repo, worktree root, the `STATE.md` path, the `/dobby:*` skills it invoked, and a note). Consumed by `/dobby:learn`, which digests that session to improve a kit skill from how it actually behaved in the field. Together `mark` (capture, in the consumer project) and `learn` (digest + edit, in this repo) are the kit's **self-improvement loop**; they couple to host paths (`~/.claude/projects`, `CLAUDE_CODE_SESSION_ID`) on purpose. - **Teach** — the kit's on-demand pedagogy skill (`/dobby:teach`): the interactive Architect teaches the user ONE topic in-session — explains it from trusted resources, runs a tight feedback loop to verify understanding, and records the demonstrated understanding as evidence. It is conversational interaction, not planning or code work, so no worker is dispatched. It is NOT a work-session stage. Distinct from the `mark`/`learn` self-improvement loop, which evolves the kit rather than the user. - **Capability** — a detected fact about a project, derived from signals in its dependencies/marker files (e.g. `vite`, `tanstack-start`, `react`, `neon`, `drizzle`, `react-email`, `vitest`, `expo`). Capabilities DRIVE the mechanical layer — `dobby` infers each task from them (the check pipeline, the `dev` composition, the `db:*` set, the capability-filtered help); see **Task inference**. `dobby env` reports the detected list; a capability is triggered by one or more **detection signals**. diff --git a/README.md b/README.md index adaecb1..f6c3a94 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ The coordinator makes sure the app is up — `/dobby:execute` runs `bunx dobby u The implementor runs its own **Exit gate** (`bunx dobby check --fix --baseline`) before handing off — the full quality gate, serialised to one implementor at a time across the shared tree — so QA only ever proves real behaviour and never re-runs lint/types/build/the suite. The leading test step is conditional: when the repo has a test suite and the spec marked a task test-first, a `dobby:test-author` writes the failing tests before the implementor touches the code. There is no fixed batch grouping tasks into waves — independent tasks keep running the moment they're ready, and a destructive task (one that mutates shared backend state during its proof) runs alone, with nothing else touching that state at the same time. A genuine defect QA finds opens a **build conversation** with the implementor, or the test-author when the failure traces to the tests rather than the implementation; the conversation is capped at five rounds, after which the task reports `needs-human` instead of retrying further. Every task that depended on a dead one is skipped as `blocked` — no agents spawned, the blocker named in its row — while everything independent of it keeps running. The normal loop deliberately has no per-task reviewer: after commit/push, the repository's external reviewer (currently Greptile) reviews the complete PR, and merge readiness requires a review of the current HEAD rather than a stale summary or silence. -**You'll see:** the Architect narrating the run live as it works — a line when each task starts, one line per task the moment it lands (`✓ verified`, `✗ needs-human`, `⊘ blocked`), a line per build-conversation round, and extra detail for a task in trouble — while `STATE.md` stays current as the run advances, so a compaction or a fresh session can reconstruct exactly where things stand. Once every task in the plan has reached a terminal status, the run closes with one summary table: rounds per task, first-attempt success, what died and why, and wall clock. +**You'll see:** the Architect narrating the run live as it works — a line when each task starts, one line per task the moment it lands (`✓ verified`, `✗ needs-human`, `⊘ blocked`), a line per build-conversation round, and extra detail for a task in trouble — while `STATE.md` stays current as the run advances, so a compaction or a fresh session can reconstruct exactly where things stand. After every worker verdict or fix-round message, the Architect also prints a per-task status table — one row per task, one emoji per step for not-started/in-progress/failed/passed — so you always have a live view of the whole plan. Each task's workers are addressed as `test-author-t` / `implementor-t` / `qa-t` and stay addressable under those same names across fix rounds. Once every task in the plan has reached a terminal status, the run closes with one summary table: rounds per task, first-attempt success, what died and why, and wall clock. ### 6. Wrap diff --git a/cli/CONTEXT.md b/cli/CONTEXT.md index b32ebdf..57f0cdb 100644 --- a/cli/CONTEXT.md +++ b/cli/CONTEXT.md @@ -38,7 +38,7 @@ under `plugin/agents/`; this CLI carries no worker-consumption recipe. - `src/release-cask.ts` (+ `src/release-cask.test.ts`) — the **homebrew-cask release TARGET**: the `ReleaseAdapter` behind `release.type: "homebrew-cask"`, for a Tauri macOS app shipped through a Homebrew tap. It uses SIX of the seam's moments (the four every target has, plus BOTH optional hooks). **preflight** — `/src-tauri/tauri.conf.json` exists (else this is not a Tauri app), `rustup target list --installed` carries BOTH `aarch64-apple-darwin` and `x86_64-apple-darwin` (a missing one refuses with the literal `rustup target add …` fix), `gh auth status`, and `release.tap` + `release.cask` are configured (each refusal names the missing key) — plus, ONLY when the OPTIONAL `release.notaryProfile` is set, the two one-time human setups notarization needs: `security find-identity -v -p codesigning` listing a `Developer ID Application` certificate (the tool EXITS 0 while listing none, so the verdict is its OUTPUT; the refusal names Xcode → Settings → Accounts as where the certificate is made, and records that SIGNING is `tauri.conf.json`'s `signingIdentity`, never dobby's job) and `xcrun notarytool history --keychain-profile

` exiting 0 (the refusal carries the one-time `xcrun notarytool store-credentials

--apple-id … --team-id …`). **bumpExtras** — `src-tauri/Cargo.toml`'s version, rewritten byte-surgically and SCOPED to the `[package]` section (the window from the `[package]` header to the NEXT `[section]`: `version = ` also sits at the start of a line under `[dependencies.]`, and a whole-file regex bumps a dependency instead), then `cargo check` in the crate — ANY cargo command reconciles `Cargo.lock`, whose stale version would otherwise ride along in the release commit. **packGate** — `bun tauri build --bundles app,dmg --target universal-apple-darwin`, then exactly ONE `*.dmg` under `src-tauri/target/universal-apple-darwin/release/bundle/dmg/` (zero and many are separate refusals) and `PlistBuddy -c "Print :CFBundleShortVersionString"` on the built `.app` equal to the version being released (a bundle that predates the bump would ship an app reporting the old number while the cask advertises the new one) — PlistBuddy is spawned BARE with `/usr/libexec` APPENDED to the child's PATH, never by absolute path. With a `release.notaryProfile` configured, THREE more steps run here (last, AFTER the version gate — a stale bundle must never cost an Apple round trip — and still before any tag, the last place a release can be refused for free): `xcrun notarytool submit --keychain-profile

--wait` whose stdout must carry `status: Accepted` (the tool exits 0 on a REJECTED submission, and a refusal quotes its FULL log, never truncated), `xcrun stapler staple `, then `spctl -a -t open --context context:primary-signature -vv ` whose output must carry `Notarized Developer ID` (spctl exits 0 for a signed-but-un-notarized build and writes its assessment to STDERR, so the gate reads BOTH streams and matches CASE-SENSITIVELY — Gatekeeper's refusal reads `Unnotarized Developer ID`); each step gates the next, so a rejected submission is never stapled and an unstapled dmg is never assessed. **publish** — a NO-OP: the dmg is a local file until the GitHub release exists, so there is nothing this target could half-publish. **postRelease** — `gh release upload v `, `shasum -a 256` on that same file, then the TAP: `gh repo clone ` into a mkdtemp dir (a failed clone is the probe — `gh repo create --public` then clone again), the cask's `version` + `sha256` lines replaced in place (indentation captured, never assumed) or the whole file SCAFFOLDED from the module's template when the tap carries none (its `url` templates Homebrew's `#{version}` and SANITIZES the asset name the way GitHub serves it — spaces become dots — so later releases only ever move two lines), `ruby -c` before the commit (an absent ruby is a NOTE, a rejection is a refusal), then `git add` + `git commit -m " "` + `git push -u origin HEAD` in the tap checkout. **smoke** — REPORT-ONLY: it runs NOTHING and hands back `brew install --cask /` (Homebrew's own rule: `/homebrew-` is referred to as `/`), plus — CONDITIONALLY, only when nothing was notarized — the quarantine caveat (`xattr -dr com.apple.quarantine …`), which next to a notarized build would simply be a lie. **The credentials are keychain-only**: `notaryProfile` is the NAME of a notarytool keychain profile and the only credential fact dobby ever holds; there is deliberately NO `APPLE_ID`/`APPLE_PASSWORD`/`APPLE_TEAM_ID` env-var path (mad-eye ADR 0005 — an app-specific password in the environment is inherited by every child, shell history and CI log). `security`, `xcrun` and `spctl` are spawned BARE like the rest, which is also what keeps them stubbable in tests. Registered by the SPINE like the npm one (`registerReleaseAdapter("homebrew-cask", caskAdapter)` in release.ts), so this module imports only TYPES from it — no runtime cycle, and the target is reachable from `run.ts`'s graph through the spine. `node:*` only (ADR-0008). - `src/review.ts` (+ `src/review.test.ts`) — the **review domain**: `dobby review fetch|apply` + `dobby pr watch`, the whole `gh` surface of the address-review stage, behind the `command.ts` contract (JUDGMENT — validity triage, fix briefs, merge gates — stays in the skills; these commands only move DATA). Every gh call goes out as an ARGV ARRAY through `runner.runCapture` with cwd pinned to `requireWorkroot` (so a reply body carrying quotes/newlines/`$(…)` reaches gh as ONE argument and the skills' shell-hardening prose becomes structurally unnecessary). Five external-system incompatibilities are baked in and commented at their call sites: (1) `gh api graphql --paginate` REQUIRES the cursor variable to be named literally `$endCursor` (the predecessor `$cursor` hand-loop silently returns page 1), used with `--slurp` so the pages arrive as ONE parseable array; (2) `gh pr checks --watch --json` is a hard error and `--json` mode never signals the CHECKS' state through the exit code (the 1/8 exits live in the non-JSON path), so `pr watch` owns its OWN polling loop and derives every verdict from BUCKET COUNTS, never exit codes — while a nonzero exit THERE means "gh could not report at all" and is surfaced as a hard error rather than read as an empty (green) check list, gh's own "no checks reported on the '' branch" being the one nonzero that legitimately means zero checks; (3) `reviewThreads` exists only in GraphQL while the summary comment must be read over REST `issues/{n}/comments` (gh's comments JSON has no `updatedAt`, and the bot EDITS one comment in place, so only `updated_at` finds the freshest body); (4) bot logins differ by API (`greptile-apps` vs `greptile-apps[bot]`), so matching STRIPS a trailing `[bot]` on both sides as a plain string op — never a regex `test()`, where `[bot]` is a character class that matches nothing. The **adapter registry is DATA** (`ADAPTERS` + the `human_or_unknown` fallback: botLogins · reTrigger · intentionalReply · confidence) — adding a review tool is one entry and nothing else. `fetch` returns `{pr, adapter{id,matchedLogins,reTrigger,intentionalReply,confidence}, candidates (null unless several matched), threads[] (every page, `isResolved` filtered CLIENT-side, each carrying `comments(last: 5)`), summary{author,body,confidence,reviewedHeadOid,updatedAt}|null}`; a clean PR is `threads: []` + exit 0, never an error, and a FAILED thread read is never degraded to an empty list (inventing "no findings" would tell the kit to merge unreviewed work). Adapter detection reads the OPEN-THREAD authors first and falls back to the ISSUE-COMMENT authors only when they name nobody — a clean review posts a summary and NO threads, but Greptile additionally requires `reviewedHeadOid` to match the current PR HEAD before `merge-ready`. `apply --plan |--stdin` consumes `{pr, plan:[{threadId, disposition: fix|defer|dismiss|outdated, reply?}], reTrigger}` and answers `{resolved, replied, skipped, retriggered, failures, dryRun}` (arrays of THREAD IDS): fix/dismiss/outdated resolve through ONE batched mutation with `t1:`/`t2:` aliases while **defer stays unresolved ON PURPOSE**, replies go out over REST `pulls/{n}/comments` with `in_reply_to` = the thread's first comment id, a reply whose body already appears in the thread's last-5 comments is SKIPPED (the idempotency the `last: 5` selection exists for), a reply that FAILED blocks its thread's resolve (closing it would bury the finding with no answer), a threadId absent from the open set is skipped (so re-running a plan is a no-op), and `--dry-run` decides everything and writes nothing. `pr watch [--pr N] [--await-review] [--deadline ]` polls `gh pr checks --json name,state,bucket,link` → `ci-failed` (any `fail`/`cancel` bucket, with the failing `[{name, link}]`) · `ci-green` · `ci-pending` (deadline hit while pending), then — only when green and asked — polls the fetch until current review evidence lands → `merge-ready` | `feedback-present` | `open-unreviewed`, exposing `reviewFresh` and a diagnostic `reason` for Greptile; no PR (on main, or nobody opened one) is `skipped` + exit 0, and every verdict exits 0 (the watch REPORTS, it never inherits an outcome) — but a gh call that FAILED is not a verdict at all and exits 1, so "green" is only ever printed about checks that were actually read. `--deadline` (300s) is the budget for EACH wait PHASE, each starting a fresh clock: one shared ceiling would let a slow CI run eat the whole budget and answer `open-unreviewed` without ever having waited for the review. `ci-pending` is the one verdict beyond the spec's list — the CI wait's timeout answer, because the spec's unbounded "poll until terminal" would let the command hang forever on a queued run. There is NO merge path in this module, structurally. `DOBBY_POLL_INTERVAL_MS` is the documented poll-interval TEST SEAM (sibling of `DOBBY_LIVENESS_RETRIES`; 0 is valid) for both loops, which sleep via `Atomics.wait` because the handler seam is synchronous. `node:*` + the shared runner only (ADR-0008). - `src/kb.ts` + `src/adr.ts` (+ the shared `src/kb-adr.test.ts`) — the two DURABLE-ARTIFACT writers (`docs/` is where the kit's decisions outlive a session), both domain modules behind the `command.ts` contract and both ACTION commands resolving their directory under `runner.requireWorkroot`. **`kb list|record`** is ONE engine for BOTH knowledge bases — `docs/out-of-scope/` (triage's rejected concepts) and `docs/learn-discarded/` (learn's discarded frictions) — parameterized by `--kind`: the `KINDS` table is the whole difference (a directory + two heading strings, `## Why this is out of scope`/`## Prior requests` vs `## Why this was NOT turned into a skill edit`/`## Prior occurrences`), which is the "two real adapters, only the strings vary" bar the one-module decision was taken against (Research R5). `record` writes the kit's OWN published skeleton (`plugin/skills/triage/references/out-of-scope-kb.md`, "File format") and dedups BY CONCEPT — an existing concept file is APPENDED to (one more bullet after the last content line of its prior-section, every byte before that heading untouched), never replaced and never duplicated under a second filename — while `list` parses the same documents TOLERANTLY (the committed KB files lead with bold inline markers, not the skeleton, so each field degrades to its empty value rather than failing the scan). The `--concept` value is slug-normalized because it becomes a PATH (which is also what makes `../` impossible). **`adr new`** owns ADR NUMBERING, the part parallelism breaks: the max scan covers the local `docs/adr/` AND `origin/HEAD:docs/adr` via `runner.runCapture` (a sibling worktree's ADR is pushed before it ever lands here — tolerant of a missing origin/HEAD/git), and the create loop claims a NUMBER rather than a filename — each attempt re-reads the directory and skips the number if ANY `NNNN-*.md` already carries it (a sibling's ADR almost never shares our slug, so an exact-filename check alone would mint a second 0017), with `O_EXCL` (`flag: "wx"`) as the last-resort arbiter for the window in which the sibling picked our slug too; both guards retry at the next number. It writes a SKELETON only (`# NNNN. `, the optional `**Status:**` line, a placeholder paragraph) — the CLI never authors an ADR body, and never DECIDES to record one; wrap/address-review decide, the CLI executes. Both answer the spec's BARE payloads (`[]`/`{path, created, appended}`/`{number, slug, path}`) rather than `state`'s `{ok, …}` envelope, so refusals live only on stderr with exit 1. `node:*` + the shared runner only (ADR-0008). -- `src/artifact-lint.ts` (+ `src/artifact-lint.test.ts`, `src/artifact-lint-b.test.ts`) — the **ARTIFACT LINTERS**, ONE module for the whole family (`dobby spec lint`, `dobby map next|claim|lint`, `dobby skill lint`, `dobby wizard verify`, `dobby arch-report verify`, `dobby handoff finalize`, `dobby brief lint`), a domain module behind the `command.ts` contract. The command TOKEN is on the `CommandContext`, so each linter is a BRANCH inside this module — never a new dispatch arm in run.ts. They share ONE report contract: findings print one per line as `<where>: <message>` (`path:line`, 1-based, wherever a line is knowable), ANY finding exits 1, a clean artifact prints `ok` and exits 0, and `--json` renders `{ok, findings: [{check, message, where}]}` as the SOLE stdout line; a malformed INVOCATION (a target that does not exist, a slug the map never had) is NOT a finding but a hard error on stderr, so "I could not judge this" can never read as "this is clean". Two ADDITIVE extensions the second family needed: **notes** — a check that could not RUN (shellcheck absent, a repo with no `.github/workflows`) is never a finding and never touches the exit code, but rides along in BOTH channels so a caller can tell a shellcheck-checked wizard from an unchecked one — and **payload keys**, the extra `--json` keys a command answers with (`handoff finalize`'s `path`). What they judge is the MECHANICAL subset only — whether a decision is right, an answer good, or prose worth its tokens stays the architect's and the reviewer's call. Targets are POSITIONAL (`--file` accepted as an alias), and the default resolutions read the workroot (`runner.resolveWorkroot`, DEGRADING to the caller's cwd outside a repo: these READ, so unlike the action commands they never fail hard). **`spec lint [<file>]`** judges `<workroot>/STATE.md`'s `## Spec` section (a document with `## ` sections but no `## Spec` is ONE finding; a file with no `## ` at all is a spec fragment and is linted whole; every heading scan skips FENCED lines, so a spec that quotes markdown under `### Decisions` is not truncated at its own snippet): the nine required `###` sub-headings (`User flow` optional, extra ones free), EXACTLY one table under `### Tasks` carrying `#`/`Task`/`Depends on`/`Affected areas`/`Verify recipe` (plus `Test-first` exactly when the repo has the vitest capability — the SAME `detect.ts` the gate reads, so the column rule can never disagree with the gate about whether this repo has a suite; `Description`/`Destructive` optional), non-empty `Task`/`Affected areas`/`Verify recipe` cells, `Depends on` edges pointing BACKWARDS by ROW POSITION (a dangling ref and a forward ref are two different findings, and a cycle is unrepresentable), every `Affected areas` entry a REAL repository path (split on comma/semicolon the SAME way `Depends on` is, but WITHOUT dropping the `—`/`none` no-dependency spellings — this column has no "nothing here" value, so one fails like any other non-path; a fragment outside the `[\w./-]` path shape — a parenthetical mangled apart by that same comma — is one finding, a shaped fragment that resolves to neither an existing path NOR an existing parent directory is another, so a path the task will CREATE stays legal as long as its parent already exists; this is what makes the dispatch protocol's area-overlap check mean anything — a prose label for the same file no longer evades it), verify recipes free of the banned commands (`lint|format|typecheck|tsc|biome|knip|vitest|jest|npm test|bun test|dobby check|build`, WORD-BOUNDARY + case-insensitive, so an honest `rebuild the fixture` stays writable), the literal `Manual verify setup:` label under `### Testing Decisions` — found ANYWHERE on its line, since the spec skill also writes it inline at the end of the paragraph (`… no test suite involvement. **Manual verify setup: none.**`), and its value read past any trailing sentence punctuation (`none` or followed by numbered steps — `/dobby:execute`'s pre-verification gate reads it, so an omitted field is an unanswered question, not "nothing needed"), and fenced blocks ONLY under `### Decisions` (the snippet exception). **`map lint|next|claim [<file>]`** parses `## <slug>: Title` tickets (`Blocked by:`/`Status:`/`Type:` + `### Question`/`### Answer`; a `## ` section without a colon is prose, not a ticket; FENCED lines are prose too, so an `### Answer` quoting a ticket cannot invent a phantom one) from the named map or the NEWEST `docs/maps/*.md`: `lint` reports non-kebab and duplicate slugs, a Status outside `open|in-progress|resolved`, a Type outside `Research|Prototype|Grilling`, dangling `Blocked by` edges, blocking CYCLES (naming every ticket caught in one — none of them can ever unblock) and a `resolved` ticket with an empty `### Answer` (only `resolved` obliges one: the whole open frontier is answerless by definition); `next` answers `{path, question, slug, title, type}` for the FIRST `open` ticket in DOCUMENT order whose every blocker is `resolved` (explicit nulls when there is none) and NEVER writes — claiming is its own command, so two sessions asking "what's next" cannot both think they own the answer; `claim <slug>` is the family's ONE write: it rewrites that ticket's `Status:` line IN PLACE (indentation AND line ending preserved — the edit is made on the RAW text, so a CRLF-authored map stays CRLF; every other byte carried over, so the claim that makes parallel map sessions safe can never reformat someone else's ticket), writing for ANY non-`in-progress` status (a hand-typed `pending`, an empty value) rather than only `open` — the claim it reports is always the claim the file carries — and refusing an unknown slug (naming the known ones), an already-resolved ticket, and one with no `Status:` line. **`skill lint <dir>`** judges a skill directory against create-skill's craft rules: `SKILL.md` present, frontmatter keys ⊆ the VENDORED whitelist (copied as DATA from `plugin/skills/create-skill/references/frontmatter.md`, because a consumer has no plugin dir to read — the same reason the wizard template is vendored; `model`/`effort` ARE valid skill fields and are NOT findings here, since "kit skills carry no model/effort" is a DOBBY convention (ADR-0004) enforced by this repo's own `checks[]`, not a rule about skills in general), a kebab `name` ≤64 chars without `claude`/`anthropic`, a `description` ≤1024, an `effort` in `low|medium|high|xhigh|max`, ≤500 lines (decision 18 settled 500 over 200), a closing `## Acceptance checklist` as the LAST H2 (headings inside fences ignored), and the RESOURCE GRAPH — every `references/`/`examples/`/`scripts/` path the body cites exists on disk, every file on disk is cited (an orphan is sediment nothing loads), no resource points at another resource (one level deep — a reference behind a reference is never reliably reached), and paths are cited as inline code, never markdown links. Only THIS skill's paths are judged: a mention carrying a path prefix counts as local when the prefix spells this very directory (`skills/improve-architecture/references/…` from inside it), and a citation of another skill's material (`../backlog/references/trackers.md`) is a link, not a pointer into this directory — reading it as one made every cross-skill citation a false "does not exist". **`wizard verify <script>`** judges a generated wizard against the VENDORED template (`wizard/template.sh`, shipped in the `files` allowlist — a consumer install has no plugin dir to read, the same reason the frontmatter whitelist is vendored): the LIBRARY REGION (`set -euo pipefail` up to the `# STAGES` marker) must hash-equal the template's own region — measured from `set -euo pipefail` rather than byte 0 so the copy's provenance header cannot make an honest script mismatch — plus `bash -n` parses, `shellcheck` when it is installed (absent → a NOTE, never a finding), the executable bit, `TOTAL_STAGES` (the value assigned BELOW the marker) equal to the number of `stage` calls, every `ask`/`ask_secret` key persisted (`write_env`/`set_secret`/`set_var`) in the SAME stage (a stage is where a Ctrl-C lands), a secret-shaped key (`SECRET|TOKEN|KEY|PASSWORD|DSN`, `PUBLIC`/`PUBLISHABLE` exempt) never read with a visible `ask`, `open_url` before the stage's first `ask`, and the BIDIRECTIONAL `set_secret` ↔ `.github/workflows/*.yml` `secrets.*` diff (`GITHUB_TOKEN` exempt — Actions injects it; no workflows dir → a NOTE). `bash`/`shellcheck` are spawned BARE through the runner, so a missing tool comes back as a spawn error, which is a SKIP; `shellcheck` is asked for `--format=gcc`, so ONE diagnostic is one finding carrying the line it happened on (its default tty output spreads a single issue over five lines). A VENDORED template that lost its own library region is a hard ERROR naming the asset (reinstall dobby), never a silent pass — otherwise a corrupted install would answer `ok` for every script. **`arch-report verify <file>`** judges the architecture review's HTML against `improve-architecture/references/html-report.md`: the two modern CDNs present (`cdn.jsdelivr.net/npm/@tailwindcss/browser@4`, a `mermaid@11` ESM import INSIDE a `<script type="module">`), the three stale spellings absent (`cdn.tailwindcss.com`, `tailwind.config`, `mermaid.min.js`), no `integrity=` (the CDN URLs are version RANGES that move under a pinned hash), no external script/stylesheet beyond those two, a `#candidates` section carrying ≥1 `<article>`, a `#top-recommendation` section whose anchor resolves to an id the report actually has, the banned prose ("easier to maintain", "cleaner code", "it's worth noting"), and the LOCATION rule it shares with the handoff: the report is ephemeral, so living inside the git workroot (or being git-tracked) is a finding. **`handoff finalize <file> [--focus <s>]`** judges the fork doc `handoff/SKILL.md` writes: the same ephemeral-location rule, the five sections (`Focus`, `Where we are`, `Artifacts`, `Open questions`, `Suggested skills` — matched as PREFIXES, since the skill itself writes "Open questions / next moves"), a ONE-line `## Focus` that must mention `--focus` when it was given, NO fenced block anywhere (a handoff REFERENCES by path or URL and never pastes), every local artifact reference resolving against the WORKROOT (the doc lives in the OS temp dir, so its own directory could never resolve `STATE.md`), every `/dobby:<skill>` suggestion naming a skill the plugin carries (the list VENDORED as data), and the SECRET SCAN — `sk-`, GitHub `ghp_`/`github_pat_`, `AKIA…`, a credentialed `postgres://` DSN, an inlined private key, `xox…`, and the generic `api_key|token|password|secret = …` assignment — which FAILS CLOSED (a false positive costs one rewording; a miss costs a rotation) and reports the LINE and the SHAPE, never the value. On a clean doc the command PRINTS the absolute path (its output IS the skill's "echo the path" step) and carries it in the `--json` payload. **`brief lint (--file <f> | --issue N)`** judges a triage agent brief either as a draft on disk or as the NEWEST comment on an issue (read through `gh api --paginate --slurp repos/{owner}/{repo}/issues/N/comments`, the last comment picked HERE rather than in a `--jq` filter; a gh failure is a refusal, never a clean verdict): the AI disclaimer as the first line, the `## Agent Brief` heading, the seven `**Label:**` fields, a `Category` of `bug|enhancement`, a ONE-line `Summary`, ≥2 `- [ ]` acceptance criteria none of which is vacuous ("it should work", "works correctly", "no regressions"), ≥1 `Out of scope` bullet, no procedural `Files to change`/`What to do` section in either spelling, and `agent-brief.md`'s two durability rules — NO file paths (extension-gated so prose is not mistaken for a path; `dobby.config.json`/`package.json` allowlisted as contracts that cannot go stale) and NO line references (`line 150`, `file.ts:150`). `node:*` + the shared runner only (ADR-0008). +- `src/artifact-lint.ts` (+ `src/artifact-lint.test.ts`, `src/artifact-lint-b.test.ts`) — the **ARTIFACT LINTERS**, ONE module for the whole family (`dobby spec lint`, `dobby map next|claim|lint`, `dobby skill lint`, `dobby wizard verify`, `dobby arch-report verify`, `dobby handoff finalize`, `dobby brief lint`), a domain module behind the `command.ts` contract. The command TOKEN is on the `CommandContext`, so each linter is a BRANCH inside this module — never a new dispatch arm in run.ts. They share ONE report contract: findings print one per line as `<where>: <message>` (`path:line`, 1-based, wherever a line is knowable), ANY finding exits 1, a clean artifact prints `ok` and exits 0, and `--json` renders `{ok, findings: [{check, message, where}]}` as the SOLE stdout line; a malformed INVOCATION (a target that does not exist, a slug the map never had) is NOT a finding but a hard error on stderr, so "I could not judge this" can never read as "this is clean". Two ADDITIVE extensions the second family needed: **notes** — a check that could not RUN (shellcheck absent, a repo with no `.github/workflows`) is never a finding and never touches the exit code, but rides along in BOTH channels so a caller can tell a shellcheck-checked wizard from an unchecked one — and **payload keys**, the extra `--json` keys a command answers with (`handoff finalize`'s `path`). What they judge is the MECHANICAL subset only — whether a decision is right, an answer good, or prose worth its tokens stays the architect's and the reviewer's call. Targets are POSITIONAL (`--file` accepted as an alias), and the default resolutions read the workroot (`runner.resolveWorkroot`, DEGRADING to the caller's cwd outside a repo: these READ, so unlike the action commands they never fail hard). **`spec lint [<file>]`** judges `<workroot>/STATE.md`'s `## Spec` section (a document with `## ` sections but no `## Spec` is ONE finding; a file with no `## ` at all is a spec fragment and is linted whole; every heading scan skips FENCED lines, so a spec that quotes markdown under `### Decisions` is not truncated at its own snippet): the nine required `###` sub-headings (`User flow` optional, extra ones free), EXACTLY one table under `### Tasks` carrying `#`/`Task`/`Depends on`/`Affected areas`/`Verify recipe` (plus `Test-first` exactly when the repo has the vitest capability — the SAME `detect.ts` the gate reads, so the column rule can never disagree with the gate about whether this repo has a suite; `Description`/`Destructive` optional), non-empty `Task`/`Affected areas`/`Verify recipe` cells, a non-empty `#` cell shaped like a legal worker-address fragment (`[A-Za-z0-9][A-Za-z0-9_-]{0,20}`, so letters/digits/`_`/`-` only, max 21 chars — build-protocol.md names each worker `<role>-t<id>`, so a path-shaped (`api/v2`), spaced (`task 1`), or over-length id would produce an invalid Agent name and fail the first dispatch; an EMPTY cell keeps the row-position default unchecked), `Depends on` edges pointing BACKWARDS by ROW POSITION (a dangling ref and a forward ref are two different findings, and a cycle is unrepresentable), every `Affected areas` entry a REAL repository path (split on comma/semicolon the SAME way `Depends on` is, but WITHOUT dropping the `—`/`none` no-dependency spellings — this column has no "nothing here" value, so one fails like any other non-path; a fragment outside the `[\w./$-]` path shape (`$` allowed for file-based-router `$param` route directories) — a parenthetical mangled apart by that same comma — is one finding, a shaped fragment whose nearest existing ancestor directory is not strictly below the repo root is another, so a path the task will CREATE stays legal as long as SOME ancestor directory below the root already exists (any depth — a module a task creates two levels deep, or a file inside a directory an earlier task creates, is not forced to overlap that directory as its area); this is what makes the dispatch protocol's area-overlap check mean anything — a prose label for the same file no longer evades it), verify recipes free of the banned commands (`lint|format|typecheck|tsc|biome|knip|vitest|jest|npm test|bun test|dobby check|build`, WORD-BOUNDARY + case-insensitive, so an honest `rebuild the fixture` stays writable), the literal `Manual verify setup:` label under `### Testing Decisions` — found ANYWHERE on its line, since the spec skill also writes it inline at the end of the paragraph (`… no test suite involvement. **Manual verify setup: none.**`), and its value read past any trailing sentence punctuation (`none` or followed by numbered steps — `/dobby:execute`'s pre-verification gate reads it, so an omitted field is an unanswered question, not "nothing needed"), and fenced blocks ONLY under `### Decisions` (the snippet exception). **`map lint|next|claim [<file>]`** parses `## <slug>: Title` tickets (`Blocked by:`/`Status:`/`Type:` + `### Question`/`### Answer`; a `## ` section without a colon is prose, not a ticket; FENCED lines are prose too, so an `### Answer` quoting a ticket cannot invent a phantom one) from the named map or the NEWEST `docs/maps/*.md`: `lint` reports non-kebab and duplicate slugs, a Status outside `open|in-progress|resolved`, a Type outside `Research|Prototype|Grilling`, dangling `Blocked by` edges, blocking CYCLES (naming every ticket caught in one — none of them can ever unblock) and a `resolved` ticket with an empty `### Answer` (only `resolved` obliges one: the whole open frontier is answerless by definition); `next` answers `{path, question, slug, title, type}` for the FIRST `open` ticket in DOCUMENT order whose every blocker is `resolved` (explicit nulls when there is none) and NEVER writes — claiming is its own command, so two sessions asking "what's next" cannot both think they own the answer; `claim <slug>` is the family's ONE write: it rewrites that ticket's `Status:` line IN PLACE (indentation AND line ending preserved — the edit is made on the RAW text, so a CRLF-authored map stays CRLF; every other byte carried over, so the claim that makes parallel map sessions safe can never reformat someone else's ticket), writing for ANY non-`in-progress` status (a hand-typed `pending`, an empty value) rather than only `open` — the claim it reports is always the claim the file carries — and refusing an unknown slug (naming the known ones), an already-resolved ticket, and one with no `Status:` line. **`skill lint <dir>`** judges a skill directory against create-skill's craft rules: `SKILL.md` present, frontmatter keys ⊆ the VENDORED whitelist (copied as DATA from `plugin/skills/create-skill/references/frontmatter.md`, because a consumer has no plugin dir to read — the same reason the wizard template is vendored; `model`/`effort` ARE valid skill fields and are NOT findings here, since "kit skills carry no model/effort" is a DOBBY convention (ADR-0004) enforced by this repo's own `checks[]`, not a rule about skills in general), a kebab `name` ≤64 chars without `claude`/`anthropic`, a `description` ≤1024, an `effort` in `low|medium|high|xhigh|max`, ≤500 lines (decision 18 settled 500 over 200), a closing `## Acceptance checklist` as the LAST H2 (headings inside fences ignored), and the RESOURCE GRAPH — every `references/`/`examples/`/`scripts/` path the body cites exists on disk, every file on disk is cited (an orphan is sediment nothing loads), no resource points at another resource (one level deep — a reference behind a reference is never reliably reached), and paths are cited as inline code, never markdown links. Only THIS skill's paths are judged: a mention carrying a path prefix counts as local when the prefix spells this very directory (`skills/improve-architecture/references/…` from inside it), and a citation of another skill's material (`../backlog/references/trackers.md`) is a link, not a pointer into this directory — reading it as one made every cross-skill citation a false "does not exist". **`wizard verify <script>`** judges a generated wizard against the VENDORED template (`wizard/template.sh`, shipped in the `files` allowlist — a consumer install has no plugin dir to read, the same reason the frontmatter whitelist is vendored): the LIBRARY REGION (`set -euo pipefail` up to the `# STAGES` marker) must hash-equal the template's own region — measured from `set -euo pipefail` rather than byte 0 so the copy's provenance header cannot make an honest script mismatch — plus `bash -n` parses, `shellcheck` when it is installed (absent → a NOTE, never a finding), the executable bit, `TOTAL_STAGES` (the value assigned BELOW the marker) equal to the number of `stage` calls, every `ask`/`ask_secret` key persisted (`write_env`/`set_secret`/`set_var`) in the SAME stage (a stage is where a Ctrl-C lands), a secret-shaped key (`SECRET|TOKEN|KEY|PASSWORD|DSN`, `PUBLIC`/`PUBLISHABLE` exempt) never read with a visible `ask`, `open_url` before the stage's first `ask`, and the BIDIRECTIONAL `set_secret` ↔ `.github/workflows/*.yml` `secrets.*` diff (`GITHUB_TOKEN` exempt — Actions injects it; no workflows dir → a NOTE). `bash`/`shellcheck` are spawned BARE through the runner, so a missing tool comes back as a spawn error, which is a SKIP; `shellcheck` is asked for `--format=gcc`, so ONE diagnostic is one finding carrying the line it happened on (its default tty output spreads a single issue over five lines). A VENDORED template that lost its own library region is a hard ERROR naming the asset (reinstall dobby), never a silent pass — otherwise a corrupted install would answer `ok` for every script. **`arch-report verify <file>`** judges the architecture review's HTML against `improve-architecture/references/html-report.md`: the two modern CDNs present (`cdn.jsdelivr.net/npm/@tailwindcss/browser@4`, a `mermaid@11` ESM import INSIDE a `<script type="module">`), the three stale spellings absent (`cdn.tailwindcss.com`, `tailwind.config`, `mermaid.min.js`), no `integrity=` (the CDN URLs are version RANGES that move under a pinned hash), no external script/stylesheet beyond those two, a `#candidates` section carrying ≥1 `<article>`, a `#top-recommendation` section whose anchor resolves to an id the report actually has, the banned prose ("easier to maintain", "cleaner code", "it's worth noting"), and the LOCATION rule it shares with the handoff: the report is ephemeral, so living inside the git workroot (or being git-tracked) is a finding. **`handoff finalize <file> [--focus <s>]`** judges the fork doc `handoff/SKILL.md` writes: the same ephemeral-location rule, the five sections (`Focus`, `Where we are`, `Artifacts`, `Open questions`, `Suggested skills` — matched as PREFIXES, since the skill itself writes "Open questions / next moves"), a ONE-line `## Focus` that must mention `--focus` when it was given, NO fenced block anywhere (a handoff REFERENCES by path or URL and never pastes), every local artifact reference resolving against the WORKROOT (the doc lives in the OS temp dir, so its own directory could never resolve `STATE.md`), every `/dobby:<skill>` suggestion naming a skill the plugin carries (the list VENDORED as data), and the SECRET SCAN — `sk-`, GitHub `ghp_`/`github_pat_`, `AKIA…`, a credentialed `postgres://` DSN, an inlined private key, `xox…`, and the generic `api_key|token|password|secret = …` assignment — which FAILS CLOSED (a false positive costs one rewording; a miss costs a rotation) and reports the LINE and the SHAPE, never the value. On a clean doc the command PRINTS the absolute path (its output IS the skill's "echo the path" step) and carries it in the `--json` payload. **`brief lint (--file <f> | --issue N)`** judges a triage agent brief either as a draft on disk or as the NEWEST comment on an issue (read through `gh api --paginate --slurp repos/{owner}/{repo}/issues/N/comments`, the last comment picked HERE rather than in a `--jq` filter; a gh failure is a refusal, never a clean verdict): the AI disclaimer as the first line, the `## Agent Brief` heading, the seven `**Label:**` fields, a `Category` of `bug|enhancement`, a ONE-line `Summary`, ≥2 `- [ ]` acceptance criteria none of which is vacuous ("it should work", "works correctly", "no regressions"), ≥1 `Out of scope` bullet, no procedural `Files to change`/`What to do` section in either spelling, and `agent-brief.md`'s two durability rules — NO file paths (extension-gated so prose is not mistaken for a path; `dobby.config.json`/`package.json` allowlisted as contracts that cannot go stale) and NO line references (`line 150`, `file.ts:150`). `node:*` + the shared runner only (ADR-0008). - `src/tracker.ts` (+ `src/tracker.test.ts`) — the **issue-tracker surface** (`dobby tracker info|search|create|close`, `dobby claim`, `dobby goal parse`): ONE backend-agnostic contract over three backends — `github` (the `gh` CLI), `linear` (the MCP), `local` (a `BACKLOG.md` at the repo root) — selected by `dobby.config.json#tracker` (key ABSENT → `github`), mechanizing `plugin/skills/backlog/references/trackers.md` verbatim. A domain module behind the `command.ts` contract; every verb is an ACTION command (`runner.requireWorkroot` — outside a git repo it fails hard, and a MALFORMED config throws too since its `tracker` key is exactly what dispatch reads). THREE properties it exists to hold. (1) **No shell, ever**: every `gh` call is an ARGV ARRAY through `runner.runCapture` (cwd pinned to the workroot), so user-derived text — a search concept, an issue title — lands as ONE argv element whatever bytes it holds; the single-quote-binding / heredoc-escaping prose the skills used to carry is RETIRED, not simplified, and the body never reaches argv at all (`--body-file`, read by gh itself). (2) **Linear is never spawned**: a `linear` backend returns a DELEGATION DESCRIPTOR (`{delegate:"mcp", op, …}`) the SKILL executes through whichever tool it resolves via ToolSearch — naming a tool here would break trackers.md's tool-name agnosticism. (3) **Degradation is REPORTED, never performed**: D8 lives in `tracker info` (`available` from `gh auth status`, `degradedTo:"local"` + a reason naming gh) and in `goal parse`'s hard stop; `search`/`create`/`close`/`claim` never silently re-route a github call into a `BACKLOG.md` write nobody asked for. Two argument ORDERS are load-bearing and commented as such: `gh label create <role>` BEFORE `gh issue create`, and `gh label create status:in-progress` BEFORE `gh issue edit` (on a fresh repo an unknown label makes the edit fail outright and the whole in-progress signal is lost) — both ignore the label create's nonzero "Name has already been taken", and neither uses `--force`, which would overwrite a colour the repo's maintainers chose. `goal parse` emits the `lifecycleLink` (`Closes #<n>` / `Fixes <KEY>`) so commit/scope never re-derive it, plus the `slug` + `slugCollision` early warning (informational only — `scope` no longer computes a collision verdict of its own). The local backend parses/writes ONE line format (`- [ ] <title> — <body> (<role>)`), projects matches onto the github result shape, and marks a close by rewriting only the checkbox. `node:*` + the shared runner only (ADR-0008). - `src/run.test.ts` + `__fixtures__/` — the co-located vitest suite (run via `vitest`, discovered from the repo root by vitest's default globs) and its hand-written sample projects. Tests call `run()` in-process with a fixture path (or a throwaway temp git repo) as `cwd`; they never import `detect.ts`/`config.ts`/`envinfo.ts`/`check.ts` directly. The `check` integration slices build a throwaway git repo (inline biome.jsonc + tsconfig + hand-written lint/type errors) and run the REAL biome + tsc. New behavior goes into PER-DOMAIN suites beside it (`src/<domain>.test.ts`) rather than growing this file. - `src/test-helpers.ts` (+ `src/test-helpers.test.ts`) — the SHARED test scaffolding the per-domain suites import; imported ONLY by `*.test.ts`, never by production code, and it NEVER ships (see the `files` allowlist below). Two seams. (1) **Stub bins on PATH** — the established way to test a tool dobby spawns BARE (`gh`, `npm`, `cmux`, `curl`): `mkStubBins({ gh: [...], npm: [] })` creates a temp dir of executable `/bin/sh` stubs to prepend to PATH (`stubPath` / `withStubPath`, which restores PATH even on a throw — the in-process `run(argv, cwd)` seam spawns children off the PARENT's env, so that is how a stub reaches the code under test); each stub RECORDS its full argv and then answers the first matching `StubResponse` (patterns are LITERAL substrings of the space-joined argv — the generated `case` arm single-quotes them, so a `*`/`?`/`[…]` inside a pattern matches ITSELF and only the wildcards wrapped around it float the substring; several patterns = AND; canned stdout/stderr/exit code — e.g. `gh pr view --json …` → a fixture payload), with `mkStubBin` as the hand-written-script escape hatch and `mkRecorderBin` for one bin at a time. `readStubLog(dir, name)` replays the recorded argv VECTORS in order: the log is `<argc>\n` + argc NUL-terminated arguments per invocation, so an argument holding spaces, quotes or newlines round-trips verbatim — which is what makes "the injection string landed in argv unmangled" a real assertion instead of a shell-quoting artifact. (2) **Scratch git repos** — `makeScratchRepo({ branch, pkg, config, files, commit })` builds a throwaway real repo under the pinned `gitEnv()` (fixed identity + `GIT_CONFIG_GLOBAL`/`SYSTEM` → `/dev/null`, no prompts, so ambient signing/hooks/templates can never derail a commit), `gitIn` reads git facts back as the independent observer, `cleanupDirs` is the afterAll counterpart. `run.test.ts` deliberately KEEPS its own local copies of the git-repo/stub-bin makers (untouched by the extraction). diff --git a/cli/src/artifact-lint.test.ts b/cli/src/artifact-lint.test.ts index 07f7c74..1978ad2 100644 --- a/cli/src/artifact-lint.test.ts +++ b/cli/src/artifact-lint.test.ts @@ -127,6 +127,7 @@ const SAYS_NESTED_REFERENCE = /detail\.md|deeper\.md/; const SAYS_AREA_PATH = /real path|does not exist/i; const SAYS_AREA_CANONICAL = /canonically|canonical/i; const SAYS_AREA_EMPHASIS = /markdown emphasis/i; +const SAYS_TASK_ID = /worker address/i; // =========================================================================== // FIXTURES — the `## Spec` artifact. @@ -264,12 +265,16 @@ const AREA_PARENT_FILES: Record<string, string> = { "cli/src/.gitkeep": "" }; // into absent-by-design (the capability comes from the shared detector). function makeSpecRepo( sections: Sections = CLEAN_SECTIONS, - opts: { vitest?: boolean } = {} + opts: { files?: Record<string, string>; vitest?: boolean } = {} ): string { const devDependencies = opts.vitest === false ? {} : { vitest: "^3.2.0" as string }; return makeScratchRepo({ - files: { ...AREA_PARENT_FILES, "STATE.md": stateDoc(specBody(sections)) }, + files: { + ...AREA_PARENT_FILES, + ...opts.files, + "STATE.md": stateDoc(specBody(sections)), + }, pkg: { devDependencies, name: "fixture-project", private: true }, prefix: "dobby-artifact-spec-", track: scratchDirs, @@ -444,6 +449,49 @@ describe("dobby spec lint — the task table", () => { }); }); +// =========================================================================== +// Slice 3a — the `#` cell's shape. The dispatch protocol interpolates it +// verbatim into a worker's Agent `name` as `<role>-t<id>`, so a plan whose id +// carries a slash, a space, or too many characters would produce an invalid +// Agent name and fail the very first worker dispatch (Greptile finding on +// build-protocol.md:15) — this is the lint that catches that before build. +// =========================================================================== + +describe("dobby spec lint — the task id shape", () => { + const idRow = (id: string) => + `| ${id} | Command registry | Per-command flag allowlist plus one stub per session command. | — | cli/src/run.ts | yes | no | dobby env --fix → exit 1 naming the flag and the command |`; + + it("rejects a `#` cell shaped like a path (`api/v2`)", async () => { + const root = makeSpecRepo(withTable(taskTable([idRow("api/v2")]))); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(1); + expect(reportOf(result)).toMatch(SAYS_TASK_ID); + }); + + it("rejects a `#` cell containing a space (`task 1`)", async () => { + const root = makeSpecRepo(withTable(taskTable([idRow("task 1")]))); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(1); + expect(reportOf(result)).toMatch(SAYS_TASK_ID); + }); + + it("rejects a `#` cell longer than 21 characters", async () => { + const root = makeSpecRepo(withTable(taskTable([idRow("a".repeat(22))]))); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(1); + expect(reportOf(result)).toMatch(SAYS_TASK_ID); + }); + + it.each(["1", "t-2", "A_3"])( + "accepts a plain, worker-address-safe `#` cell (%s)", + async (id) => { + const root = makeSpecRepo(withTable(taskTable([idRow(id)]))); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(0); + } + ); +}); + // =========================================================================== // Slice 3b — `Affected areas` as real repository paths. The clean fixture // (CLEAN_TABLE, above) already doubles as the "path a task will CREATE" @@ -462,15 +510,57 @@ describe("dobby spec lint — Affected areas as real paths", () => { expect(result.exitCode).toBe(0); }); - it("reports an area whose parent directory does not exist either", async () => { + it("reports an area with no existing ancestor below the repo root", async () => { + // No segment of this path exists anywhere in the fixture — the walk + // upward from it reaches the repo root itself without finding an + // anchor, and the root is never an acceptable anchor. const nowhere = - "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | cli/nonexistent/preflight.ts | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; + "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | nonexistent/deeply/nested/preflight.ts | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; const root = makeSpecRepo(withTable(taskTable([ROW_1, ROW_2, nowhere]))); const result = await run(["spec", "lint"], root); expect(result.exitCode).toBe(1); expect(reportOf(result)).toMatch(SAYS_AREA_PATH); }); + it("accepts a path two levels below an existing directory — an area a later task's module will create", async () => { + // `cli/src` exists (AREA_PARENT_FILES); `cli/src/new-module` and + // `cli/src/new-module/server` do not. The nearest existing ancestor, + // `cli/src`, anchors the area even though it sits two levels up. + const twoLevelsDeep = + "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | cli/src/new-module/server | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; + const root = makeSpecRepo( + withTable(taskTable([ROW_1, ROW_2, twoLevelsDeep])) + ); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(0); + }); + + it("accepts an existing `$param` route directory as a real path", async () => { + const existingParam = + "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | src/routes/$id | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; + const root = makeSpecRepo( + withTable(taskTable([ROW_1, ROW_2, existingParam])), + { + files: { "src/routes/$id/.gitkeep": "" }, + } + ); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(0); + }); + + it("accepts a nonexistent path under an existing `$param` route directory", async () => { + const underParam = + "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | src/routes/$id/new-file.tsx | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; + const root = makeSpecRepo( + withTable(taskTable([ROW_1, ROW_2, underParam])), + { + files: { "src/routes/$id/.gitkeep": "" }, + } + ); + const result = await run(["spec", "lint"], root); + expect(result.exitCode).toBe(0); + }); + it("reports a bare label that names no real path in the repo", async () => { const bare = "| 3 | Session preflights | Read-only verdicts for scope and finish. | 1 | dispatch | yes | no | dobby scope preflight --slug demo → JSON reporting the collision |"; diff --git a/cli/src/artifact-lint.ts b/cli/src/artifact-lint.ts index 34e51af..9e8af80 100644 --- a/cli/src/artifact-lint.ts +++ b/cli/src/artifact-lint.ts @@ -358,15 +358,27 @@ const BANNED_VERIFY_COMMAND = /\b(?:lint|format|typecheck|tsc|biome|knip|vitest|jest|npm test|bun test|dobby check|build)\b/i; // The character shape a real repository path takes in this kit: word -// characters, dot, dash and slash only. Nothing else — no space, no -// parenthesis, no stray punctuation — belongs in a path, so anything outside -// this shape is prose that leaked into the cell (task-decomposition.md's -// `Affected areas` rule: name the path, not a description of it). This is what -// catches a parenthetical aside split apart by the SAME comma that separates -// areas, e.g. `plugin/skills (research, dispatch)` → `plugin/skills (research` -// + `dispatch)`, both of which fail this shape before existence is even -// checked. -const AREA_PATH_SHAPE = /^[\w./-]+$/; +// characters, dot, dash, slash and `$`. `$` is allowed because file-based +// routers (TanStack Start/Router, Remix) spell a dynamic route segment as a +// `$param` directory, so it is part of a real path in this kit's stack. +// Nothing else — no space, no parenthesis, no stray punctuation — belongs in +// a path, so anything outside this shape is prose that leaked into the cell +// (task-decomposition.md's `Affected areas` rule: name the path, not a +// description of it). This is what catches a parenthetical aside split apart +// by the SAME comma that separates areas, e.g. `plugin/skills (research, +// dispatch)` → `plugin/skills (research` + `dispatch)`, both of which fail +// this shape before existence is even checked. +const AREA_PATH_SHAPE = /^[\w./$-]+$/; + +// The `#` cell is interpolated verbatim into a worker's Agent `name` as +// `<role>-t<id>` (build-protocol.md), whose own shape is +// `^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$` — letters, digits, `_`, `-`, max 64 +// chars total. `test-author-t` is the longest role prefix at 13 chars, so +// capping the id at 21 chars keeps every address well under that ceiling +// with room to spare. An id like `api/v2` or `task 1` would produce an +// invalid Agent name and fail the very first worker dispatch, so this shape +// is enforced here rather than discovered at dispatch time. +const TASK_ID_SHAPE = /^[A-Za-z0-9][A-Za-z0-9_-]{0,20}$/; // A backtick or asterisk left INSIDE an area value — unlike `_`, which a real // snake_case path segment legitimately carries (`splitCellMembers` in @@ -627,6 +639,7 @@ function lintTaskTable( const columns = taskColumns(table); const rows = taskRows(table, columns); findings.push(...lintRowCells(rows, columns, context)); + findings.push(...lintRowIds(table, columns, context)); findings.push(...lintRowDependencies(rows, context)); findings.push(...lintRowAreas(rows, columns, context)); findings.push(...lintRowRecipes(rows, columns, context)); @@ -733,6 +746,35 @@ function lintRowCells( return findings; } +// The `#` cell must itself be a legal worker-address fragment (see +// TASK_ID_SHAPE above) — an empty cell keeps today's positional default +// (`taskRows` fills it in with `index + 1`, which is always shape-legal), so +// only a NON-EMPTY, badly-shaped id is a finding here. +function lintRowIds( + table: Table, + columns: TaskColumns, + context: SpecContext +): Finding[] { + if (columns.id < 0) { + return []; + } + const findings: Finding[] = []; + for (const row of table.rows) { + const raw = cellAt(row.cells, columns.id); + if (raw === "" || TASK_ID_SHAPE.test(raw)) { + continue; + } + findings.push( + finding( + "spec-task-id", + at(context.path, row.line), + `task ${raw}: \`#\` is \`${raw}\`, which cannot be a worker address — the dispatch protocol names workers \`<role>-t<id>\`, so an id may carry only letters, digits, \`_\` and \`-\` (max 21 chars); number the tasks 1, 2, 3…` + ) + ); + } + return findings; +} + // Dependencies point BACKWARDS, always. Comparing by the row's POSITION (not by // parsing the id as a number) keeps the rule honest for any id scheme, and it is // what makes a cycle unrepresentable: an edge can only ever reach a row that is @@ -847,14 +889,16 @@ function canonicalAreaPath(root: string, area: string): string { // Why an area is not usable, as a finding message, or null when it is fine: an // EXISTING file or directory, or one this task will CREATE — recognised by -// its parent directory already existing (task-decomposition.md's rule: a path -// a task creates is legitimate as long as its parent already exists). A cell -// that isn't shaped like a path at all (a comma-mangled fragment of a -// parenthetical, free prose) is rejected before existence is even checked, -// and one that IS shaped like a path but is not written canonically (a -// doubled separator, an interior `..`, an absolute path standing in for the -// same relative one) is rejected before existence too — see -// `canonicalAreaPath` above for why. +// its NEAREST EXISTING ANCESTOR being inside the repo (task-decomposition.md's +// rule: a path a task creates is legitimate as long as some ancestor +// directory below the repo root already exists — a module a task creates two +// levels deep, or a file inside a directory an earlier task creates, is not +// forced to overlap that directory as its area). A cell that isn't shaped +// like a path at all (a comma-mangled fragment of a parenthetical, free +// prose) is rejected before existence is even checked, and one that IS shaped +// like a path but is not written canonically (a doubled separator, an +// interior `..`, an absolute path standing in for the same relative one) is +// rejected before existence too — see `canonicalAreaPath` above for why. function areaPathProblem( root: string, rawArea: string, @@ -876,10 +920,19 @@ function areaPathProblem( if (existsSync(absolute)) { return null; } - if (area.includes("/") && existsSync(dirname(absolute))) { - return null; + // Walk upward from the nearest ancestor directory: any one of them + // existing anchors the area to real ground. `inside()` is the "strictly + // below root" predicate the walk needs on both ends — it stops the moment + // it leaves the repo (the root itself never counts as the anchor, or every + // top-level nonexistent path would pass unconditionally), and normalizes + // the slash so a trailing separator on `root` can never fool a bare `!==`. + for (let ancestor = dirname(absolute); inside(root, ancestor); ) { + if (existsSync(ancestor)) { + return null; + } + ancestor = dirname(ancestor); } - return `task ${taskId}: \`Affected areas\` names \`${rawArea}\`, which does not exist in this repo — name a real directory or file (a path the task will CREATE is fine as long as its parent directory already exists)`; + return `task ${taskId}: \`Affected areas\` names \`${rawArea}\`, which does not exist in this repo — name a real directory or file (a path the task will CREATE is fine as long as some ancestor directory below the repo root already exists, e.g. a module an earlier task creates)`; } function lintRowRecipes( diff --git a/cli/src/build-protocol.test.ts b/cli/src/build-protocol.test.ts index ec0e52d..49f9cb7 100644 --- a/cli/src/build-protocol.test.ts +++ b/cli/src/build-protocol.test.ts @@ -40,8 +40,10 @@ import { describe, expect, it } from "vitest"; // decisions name character-for-character — nothing is inferred about them. // - "This task must not describe a Workflow anywhere" is the constraint stated // as a ban, so the ban is what is asserted. -// - The `dobby:`-qualified agent ids are the kit's mandatory namespacing rule -// (CLAUDE.md, CONTEXT.md `Namespacing`). +// - `dobby:<role>` stays the kit's mandatory namespacing rule for cross- +// references (CLAUDE.md, CONTEXT.md `Namespacing`) and is what a dispatch's +// `subagent_type` names; the ADDRESS a sibling messages is the separate +// per-task `name`, `<role>-t<id>`. // =========================================================================== const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "..", ".."); @@ -557,6 +559,51 @@ describe("the dispatch protocol — the run record", () => { }); }); +// --------------------------------------------------------------------------- +// SLICE 7b — the status table's transitions: which cell moves at each routing +// and re-check step, so a code defect visibly resumes the implementor and a +// closed round visibly resumes the reporter. +// --------------------------------------------------------------------------- + +describe("the status table — transitions", () => { + it("moves the implementor to in progress when QA reports a code defect", () => { + expect( + statesJoinedRule( + readProtocol(), + /defect/i, + /implementor/i, + /in progress|🔄/i + ), + "a code defect resumes the implementor, so its cell must leave ✅/⚪ for in-progress" + ).toBe(true); + }); + + it("has the fixer message the reporter back to trigger the re-check", () => { + expect( + statesJoinedRule( + readProtocol(), + /message|SendMessage/i, + /back/i, + /re-check|recheck/i + ), + "the return message is what resumes the reporter for its re-check" + ).toBe(true); + }); + + it("routes a corrected test contract through the implementor's Exit gate before QA resumes", () => { + expect( + statesJoinedRule( + readProtocol(), + /test-author/i, + /implementor/i, + /exit gate|gate/i, + /before|then|only/i + ), + "QA never runs the suite, so a corrected contract must be proven by the implementor's own gate first" + ).toBe(true); + }); +}); + // =========================================================================== // THE FIX CONVERSATION — the loop that closes a failure. // @@ -573,10 +620,11 @@ describe("the dispatch protocol — the run record", () => { // consume a round" is the decision, stated as three separate facts. // - "the sender reports to the Architect rather than retrying blindly, and the // round still counts" is the constraint, verbatim. -// - The `dobby:`-qualified addressee ids are the kit's mandatory namespacing -// rule (CLAUDE.md, CONTEXT.md `Namespacing`) — and the name is literally the -// answer to "who does QA message", since a worker can only reach a sibling it -// can name. +// - `dobby:<role>` is the kit's mandatory namespacing rule (CLAUDE.md, +// CONTEXT.md `Namespacing`) for the `subagent_type`, never the address a +// sibling messages — the per-task `name`, `<role>-t<id>`, is literally the +// answer to "who does QA message", since a worker can only reach a sibling +// it can name. // =========================================================================== // --------------------------------------------------------------------------- @@ -591,7 +639,7 @@ describe("the fix conversation — routing a failure", () => { statesJoinedRule( readProtocol(), /\bQA\b/, - /dobby:implementor/, + /implementor-t<id>/, SENDS, /defect|failure|failing/i ), @@ -604,7 +652,7 @@ describe("the fix conversation — routing a failure", () => { statesJoinedRule( readProtocol(), /\bQA\b/, - /dobby:test-author/, + /test-author-t<id>/, SENDS, /contract/i ), @@ -616,7 +664,7 @@ describe("the fix conversation — routing a failure", () => { expect( statesJoinedRule( readProtocol(), - /dobby:test-author/, + /test-author-t<id>/, /behaviou?r/i, /quote|verbatim|snippet|fragment|implementation/i, NEGATION diff --git a/plugin/agents/implementor.md b/plugin/agents/implementor.md index 33ae0d6..136f1d7 100644 --- a/plugin/agents/implementor.md +++ b/plugin/agents/implementor.md @@ -10,7 +10,7 @@ effort: high You are the IMPLEMENTOR. You implement (or fix) ONE task, then run the Exit gate yourself before handing off. You do NOT prove behaviour against the running app — QA does that — and you do NOT review your own style. Don't claim it works; QA decides. ## Reach a sibling -`SendMessage` is a DEFERRED tool — load it before your first use with `ToolSearch({query: "select:SendMessage"})`, or you can never reach anyone. Use it to message the test-author directly when the Exit gate turns up a test-contract problem (see below), and expect QA to message YOU directly with a defect during the fix loop instead of routing through a fresh agent that would have to re-read everything. +`SendMessage` is a DEFERRED tool — load it before your first use with `ToolSearch({query: "select:SendMessage"})`, or you can never reach anyone. Use it to message the test-author directly when the Exit gate turns up a test-contract problem (see below), and expect QA to message YOU directly with a defect during the fix loop instead of routing through a fresh agent that would have to re-read everything. Once you've fixed a QA-reported defect and your Exit gate is green again, message QA back by name with the round number so it re-checks — a fix nobody is told about closes nothing. If the test-author instead messages YOU that a test contract was corrected, make whatever implementation change the corrected contract now demands, re-run your Exit gate, and only once it's green message QA back yourself — QA never runs the test suite, so it can't resume on the test-author's word alone. ## What you get The task (title, spec, decisions, constraints, affected areas) and, on a fix iteration, the SPECIFIC QA findings to apply, or a message from the test-author if they extended the contract. diff --git a/plugin/agents/qa.md b/plugin/agents/qa.md index cf36734..a68508b 100644 --- a/plugin/agents/qa.md +++ b/plugin/agents/qa.md @@ -12,7 +12,7 @@ You are QA (`dobby:qa`). You did NOT write or review this code. Prove the task a **You prove BEHAVIOUR ONLY.** Never run lint, typecheck, build, or the test suite — that whole mechanical layer is already closed before you're ever dispatched: the edit hook caught it file-by-file, and the implementor ran the full gate himself (the Exit gate) before handing the task to you. Re-running any of it just duplicates work that already happened and burns a round for nothing. You don't review code style either — that's the reviewer's and the PR's job, not yours. ## Reach the implementor when you find a defect -`SendMessage` is a DEFERRED tool — load it before your first use with `ToolSearch({query: "select:SendMessage"})`, or you will never reach anyone. When you find a genuine defect, message the implementor who still holds this task's full context directly by name, describing what you OBSERVED (not a guess at the fix or an implementation fragment) — that's cheaper and more accurate than a fresh agent re-reading everything. The round-count and hand-off rules for that conversation live in the dispatch protocol you were launched under; if you weren't dispatched with a name, or no addressable sibling exists, fall back to returning your verdict alone. +`SendMessage` is a DEFERRED tool — load it before your first use with `ToolSearch({query: "select:SendMessage"})`, or you will never reach anyone. When you find a genuine defect, message the implementor who still holds this task's full context directly by name, describing what you OBSERVED (not a guess at the fix or an implementation fragment) — that's cheaper and more accurate than a fresh agent re-reading everything. The round-count and hand-off rules for that conversation live in the dispatch protocol you were launched under; if you weren't dispatched with a name, or no addressable sibling exists, fall back to returning your verdict alone. After you send a defect, expect the implementor to message you back by name once the fix lands — re-check then, and number the next round in your own message. A re-check is triggered ONLY by the implementor's return message, never by the test-author's: if you reported a test-contract problem instead, the test-author fixes it and hands it to the implementor first, and you keep waiting until the implementor — having re-run its own Exit gate against the corrected contract — messages you. ## The app is already running — don't start it The dev server is ALREADY up at the `devUrl` you're given — `/dobby:execute` ensured it per `../skills/execute/references/bring-up.md` — the run is registered and live before you are dispatched. You NEVER start it yourself — parallel QA runs each starting a server would collide on the port. Verify against the given `devUrl`; if it's unreachable, report that rather than starting your own. diff --git a/plugin/agents/test-author.md b/plugin/agents/test-author.md index bca8af7..5e735d5 100644 --- a/plugin/agents/test-author.md +++ b/plugin/agents/test-author.md @@ -1,7 +1,7 @@ --- name: test-author description: Write the tests for ONE task from the SPEC ALONE — never seeing the implementation — as the fixed contract the implementor must satisfy, then return them. Does not implement, review, or verify. -tools: Read, Edit, Write, Grep, Glob, Bash +tools: Read, Edit, Write, Grep, Glob, Bash, ToolSearch, SendMessage # Model and effort are authoritative here — no external recipe supplies them. model: claude-opus-5 effort: high @@ -9,6 +9,9 @@ effort: high You are the TEST-AUTHOR. You write the tests for ONE task, from the SPEC ALONE, BEFORE any implementation exists. You do NOT implement, review, or verify — separate agents do that. The tests you write are the fixed contract: the implementor makes them pass through his own Exit gate, and QA proves the behaviour they describe against the running app. You run at the start of the task and outer-loop retries re-implement against your SAME tests, so get the contract right. The ONE way you are re-dispatched is a test-contract gap raised during the build loop — the implementor's Exit gate turning up a weak/tautological assertion, or a PR review finding one later: extend the contract with exactly what that finding names and leave the rest fixed. The implementor may message you directly with a suspected gap, described in terms of expected behaviour only, but he can never edit or skip your tests himself — only you rewrite the contract, and a re-dispatch is never a license to rewrite more of it than the finding names. +## Reach a sibling +`SendMessage` is a DEFERRED tool — load it before your first use with `ToolSearch({query: "select:SendMessage"})`, or you can never reach anyone. When the implementor (or QA) messages you about a test-contract problem for a round, fix the tests and message the IMPLEMENTOR back by name with the round number — never QA directly, even when QA was the one that raised the problem — describing expected BEHAVIOUR only, never a code fragment or snippet of the implementation. Only the implementor can run your corrected contract through its Exit gate; QA never runs the test suite, so it needs the implementor's proof, not yours, before it resumes. + ## Why you never see the implementation Your one job is to be the INDEPENDENT source of truth. If you derived a test's expected value the way the code computes it, the test could never disagree with the code — it would pass by construction and prove nothing (the tautology below). You are protected from that failure structurally: you write from the spec, the interface it names, and known-good examples — NOT from the implementation, which does not exist yet and which you must not reconstruct. diff --git a/plugin/skills/execute/SKILL.md b/plugin/skills/execute/SKILL.md index 54fdcb5..ce0e818 100644 --- a/plugin/skills/execute/SKILL.md +++ b/plugin/skills/execute/SKILL.md @@ -51,8 +51,9 @@ Follow **`references/build-protocol.md`** — the shared build-loop component For every task, write a self-contained instruction before dispatching: `TASK: <title>`, `Spec: <spec>`, plus the plan-level `decisions` / `constraints` you judge relevant to THIS task (they came back empty from `build-plan` by contract), and `Affected areas: <areas>`. On a later round of the same task, add the specific QA or test-author feedback being applied — nothing else. - **Start each task the moment its `dependsOn` are all `done`** — no fixed batch to wait on (`build-protocol.md`'s scheduling rule). A `destructive` task runs alone, nothing else touching the shared backend at the same time. -- **Every dispatch is NAMED** — `dobby:test-author` (conditional: only when `hasTestSuite.value` is true AND the task is marked test-first), `dobby:implementor`, `dobby:qa` — never anonymous; naming is what gives a worker a sibling roster and lets QA reach the implementor (or the implementor reach the test-author) directly instead of a fresh agent re-reading everything. +- **Every dispatch is NAMED** — `subagent_type: "dobby:test-author"` (conditional: only when `hasTestSuite.value` is true AND the task is marked test-first) / `"dobby:implementor"` / `"dobby:qa"`, with `name: "test-author-t<id>"` / `"implementor-t<id>"` / `"qa-t<id>"` (the Agent tool rejects a `:` in `name`, which is why the `subagent_type` id can never double as the address) — never anonymous. Open each dispatch's instruction with the task's roster (`You are implementor-t2; the test-author is test-author-t2; QA will be qa-t2.`) so the worker knows its siblings without discovering them. One dispatch per role per task: later rounds — a QA defect, a test-contract fix, a re-check — reach that SAME name by `SendMessage` until the task hits a terminal status; never a replacement worker mid-task. - **The Exit gate is serialised** — only one implementor runs it against the shared tree at a time; everything else about every task keeps running in parallel. `build-protocol.md` owns the exact turn-taking. +- **Report progress after every step** — after any worker returns a verdict or a fix-round message lands, print the per-step status table `build-protocol.md` defines (`Report progress after every step`): one row per planned task, one emoji per step (not started / in progress / failed / passed). - **A dead task stops only its dependents** — mark them blocked and keep dispatching everything else that is ready; say what died the moment it happens, by task id and terminal status, rather than only at the end. - **Every worker appends its own record and hands you back a short verdict only** — you never write a worker's work-log entry for it, and you never receive its full reasoning trail. @@ -94,7 +95,7 @@ User-facing output (status) in the user's language. Write all code, comments, do - [ ] `preconditions.ok === false` → STOPPED and routed back to `/dobby:spec`, quoting `missing[]` (+ dangling deps / cycles); `hasTestSuite.disagreement` surfaced - [ ] Workspace brought up per `references/bring-up.md`; `ok:false` STOPped naming the `reason` (+ `degradedCommand` when offered); non-empty `instructions[]` carried out in order (rename then start) and `up --json` re-run, with the stop rule honored on a repeated `start`; `devUrl` / `verifyMode` / `browserPane` / `workroot` taken from the payload - [ ] Manual-setup gate honored at end of Step 2: `none` skips silently; steps first run `bunx dobby instructions browser --json` and carry out its surface step, THEN prompt (AskUserQuestion, in-stage) and block every worker until the user confirms setup in that ONE deterministic surface -- [ ] Every task followed `references/build-protocol.md`: named dispatch throughout, a task started the moment its `dependsOn` cleared, the Exit gate serialised to one implementor at a time, a dead task's dependents blocked while independent tasks kept going, and each worker's death was named as it happened +- [ ] Every task followed `references/build-protocol.md`: named dispatch throughout (`<role>-t<id>`, never a bare role name or the `subagent_type` id), no worker replaced mid-task (later rounds reached the same name until a terminal status), a task started the moment its `dependsOn` cleared, the Exit gate serialised to one implementor at a time, a dead task's dependents blocked while independent tasks kept going, each worker's death was named as it happened, and the per-step status table was printed after every worker verdict or fix-round message - [ ] Test-author gated correctly: dispatched only when `hasTestSuite.value` AND the task is test-first; the implementor never edits the authored tests, only messages the test-author with a suspected gap - [ ] `STATE.md` kept current as the run advanced, so progress was reconstructable after a compaction - [ ] `done` reported as locally verified with external PR code review still pending diff --git a/plugin/skills/execute/references/build-protocol.md b/plugin/skills/execute/references/build-protocol.md index 8fe6067..02bf913 100644 --- a/plugin/skills/execute/references/build-protocol.md +++ b/plugin/skills/execute/references/build-protocol.md @@ -6,16 +6,22 @@ It is not a runtime for a tool to execute — it is what the interactive Archite ## Launch workers named -Every worker is dispatched as a NAMED subagent — the Agent tool's `name` argument, never an anonymous call: +Every worker is dispatched as a NAMED subagent — the Agent tool's `name` argument, never an anonymous call. `subagent_type` is the agent DEFINITION; `name` is the per-task ADDRESS, and the two are not interchangeable: -- Test-author → `dobby:test-author` -- Implementor → `dobby:implementor` -- QA → `dobby:qa` +- Test-author → `subagent_type: "dobby:test-author"`, `name: "test-author-t<id>"` +- Implementor → `subagent_type: "dobby:implementor"`, `name: "implementor-t<id>"` +- QA → `subagent_type: "dobby:qa"`, `name: "qa-t<id>"` -Naming is not a style choice. Only a NAMED dispatch produces a sibling roster the worker can read, and only a worker who can see that roster has anyone addressable to reach with `SendMessage` — dispatch a worker anonymously and it has no roster to consult and no sibling it can name, so it silently falls back to returning its verdict alone with no fix conversation possible. Every dispatch in this protocol carries a name for exactly this reason. +`<id>` is the task's own id from the plan (`tasks[].id` from `build-plan`). The Agent tool's `name` argument accepts only letters, digits, `_`, and `-` (max 64 chars) — a `:` is rejected, which is exactly why the definition id (`dobby:implementor`) can never double as the name. One name per role is not enough either: with several tasks in flight at once, a bare `implementor` would collide across tasks, so the name carries the task id and gives each task's workers their own distinguishable address. + +Naming is not a style choice. A NAMED worker is addressable by `SendMessage` under that name, and its dispatch instruction (below) is what tells it who its siblings are — an anonymous worker has no address of its own and no siblings named to it, so it silently falls back to returning its verdict alone with no fix conversation possible. Every dispatch in this protocol carries a name for exactly this reason. + +That sibling roster is exactly what belongs in the dispatch instruction: open it with the task's three names, so the worker knows exactly who its siblings are without having to discover them — e.g. `You are implementor-t2; the test-author is test-author-t2; QA will be qa-t2.` Before a worker's FIRST use of `SendMessage`, it must load the deferred tool with `ToolSearch({query: 'select:SendMessage'})` — the tool does not exist in a fresh worker's toolset until then, and a worker that skips this step cannot reach anyone. +**A task's three workers are dispatched ONCE each, and stay alive for the whole task.** Every later round — a QA defect, a test-contract problem, a re-check after a fix — reaches that SAME name by `SendMessage`; the Architect never dispatches a replacement worker for a role mid-task. "Alive" means NOT REPLACED, not necessarily still running: `SendMessage` to a name resumes that agent from its own transcript with its context intact, so a worker that has already returned a verdict is still the right one to message. Treat a worker as gone only once its task reaches a terminal status (`done`, `blocked`, `needs-human`) — never before. + ## Require CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS unset Before dispatching anyone, confirm the session's `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS` environment variable is unset. With it set, a named dispatch silently launches as a teammate instead of an ordinary worker, and the sibling roster and messaging this protocol depends on behave differently under that mode. This protocol assumes it stays off; if it is set, stop and tell the user rather than dispatching into a mode this design was never verified against. @@ -29,7 +35,7 @@ Two exceptions hold a task back even once its dependencies clear: - **Overlapping writers.** `dobby build-plan` emits each task's `areas` — compare a ready task's `Affected areas` against every task currently in flight before dispatching it, as NORMALISED PATHS (strip a trailing slash, strip a leading `./`) rather than as opaque strings matched for exact equality: one area being a PREFIX of the other counts as overlap, so a task naming a directory and a task naming a file inside it describe overlapping ground even though the labels aren't identical. If any pair overlaps, the ready task waits for the in-flight one to finish, exactly as if it depended on it, rather than starting alongside it. Two implementors mid-edit on the same file is not a hypothetical: without this check, their edits overwrite or interleave each other in the shared tree, and a gate run against that tree judges a mix of both tasks' unfinished code, not either task cleanly. - **A destructive task** (one that mutates shared backend state during its proof) is dispatched alone, with nothing else touching the shared backend at the same time, because two destructive proofs racing each other corrupt both. -This check only ever sees what the plan wrote down. It closes the common collision — two tasks naming the same code at different depths — and it closes the collision two tasks used to hide under genuinely UNRELATED labels for the same file, too: `dobby spec lint` requires every `Affected areas` entry to be a real repository path (an existing directory/file, or one whose parent already exists for a path a task will create), rejecting free-prose labels ("the gate", "CLI checks") at SPEC TIME, before the plan is ever dispatched — two tasks touching the same ground are forced to name the same path, so the overlap check above actually sees the overlap. Areas that name real paths are what make this check work at all; a task left vague here (naming a whole directory when a file would do) still weakens its own protection. What remains is genuinely irreducible: a task can still touch a file it never declared in its `Affected areas` at all, and no static comparison — lint or dispatch — can catch that, because prose written before anyone has touched the code can't know what an implementor will actually open. Where that residual gap is hit for real, nothing here PREVENTS the collision — but the serialised Exit gate below still DETECTS it: the gate always judges the current whole tree, so a sibling's edit to an undeclared file shows up there as a finding the implementor does not own, which it reports and leaves alone per the coordination guards. That is detection, not prevention, and the difference matters — don't read the area check as airtight just because most collisions never reach the gate to find out. +This check only ever sees what the plan wrote down. It closes the common collision — two tasks naming the same code at different depths — and it closes the collision two tasks used to hide under genuinely UNRELATED labels for the same file, too: `dobby spec lint` requires every `Affected areas` entry to be a real repository path (an existing directory/file, or one whose nearest existing ancestor is inside the repo, for a path a task will create — a task may create a path several levels deep under a directory that exists, or that an earlier task creates), rejecting free-prose labels ("the gate", "CLI checks") at SPEC TIME, before the plan is ever dispatched — two tasks touching the same ground are forced to name the same path, so the overlap check above actually sees the overlap. Prefer naming FILES over a whole directory from the first draft, too: a task whose area is an entire directory serialises every sibling task that also touches something in that directory, even one that never comes near the same file. Areas that name real paths are what make this check work at all; a task left vague here (naming a whole directory when a file would do) still weakens its own protection. What remains is genuinely irreducible: a task can still touch a file it never declared in its `Affected areas` at all, and no static comparison — lint or dispatch — can catch that, because prose written before anyone has touched the code can't know what an implementor will actually open. Where that residual gap is hit for real, nothing here PREVENTS the collision — but the serialised Exit gate below still DETECTS it: the gate always judges the current whole tree, so a sibling's edit to an undeclared file shows up there as a finding the implementor does not own, which it reports and leaves alone per the coordination guards. That is detection, not prevention, and the difference matters — don't read the area check as airtight just because most collisions never reach the gate to find out. ## The per-task loop @@ -47,8 +53,19 @@ This is the loop that closes a failure. QA does not stop at a verdict when it fi ### Route the failure to whoever can fix it -- **A code defect** — QA sends its findings directly to `dobby:implementor`, the implementor that still holds this task's context, rather than a fresh worker that would have to re-read everything from nothing. The message describes what QA OBSERVED — the failing behaviour — not a guess at the fix. -- **A test-contract problem** — when the failure traces back to the tests themselves rather than the implementation, QA sends its findings directly to `dobby:test-author` instead. A message to the test-author describes expected BEHAVIOUR only: it never quotes, pastes, or shows any snippet or fragment of the implementation. Quoting the code is exactly what the test-author's blindness to it is meant to prevent — a test-author who never sees the implementation writes tests that pin behaviour, not ones that tautologically confirm whatever the code already does. +- **A code defect** — QA sends its findings directly to `implementor-t<id>`, the implementor that still holds this task's context, rather than a fresh worker that would have to re-read everything from nothing. The message describes what QA OBSERVED — the failing behaviour — not a guess at the fix. +- **A test-contract problem** — when the failure traces back to the tests themselves rather than the implementation, QA sends its findings directly to `test-author-t<id>` instead. A message to the test-author describes expected BEHAVIOUR only: it never quotes, pastes, or shows any snippet or fragment of the implementation. Quoting the code is exactly what the test-author's blindness to it is meant to prevent — a test-author who never sees the implementation writes tests that pin behaviour, not ones that tautologically confirm whatever the code already does. + +### Close the round: the fixer messages whoever can prove the fix + +A round does not end when the fixer starts working — it ends when the fixer messages the fix onward, BY NAME, carrying the same round number, to whoever can PROVE it: for a code defect that is the reporter itself; for a test-contract problem it is always the implementor, never straight back to whichever worker reported the problem. + +- **A code defect**: the implementor messages `qa-t<id>` once its fix has passed its own Exit gate ("round N fix landed — re-check"). +- **A test-contract problem**: the test-author messages `implementor-t<id>` — never `qa-t<id>` directly, even when QA was the one that raised the problem — once the test contract is corrected, describing expected behaviour only, never a code fragment ("round N contract corrected — re-run your gate"). QA never runs the test suite, so a corrected contract that bypassed the implementor's Exit gate would leave QA marking the task done on an unproven change. The implementor then makes any implementation change the corrected contract now demands, re-runs its Exit gate, and only once it is green messages `qa-t<id>` itself ("round N fix landed — re-check") — the same code-defect return leg above, just reached one hop later. + +That return message is what resumes the next worker in the chain — `SendMessage` resumes a named agent from its own transcript — so a round is never left waiting on the Architect to notice a fix landed. QA — the only worker either return leg ever resumes — then re-checks and either passes the task on or opens round N+1 the same way. + +The Architect steps in only in the two cases already covered above: a message that cannot be delivered at all, or a worker that returns without closing its round — a fixer that reports done to the Architect but never messages the next worker in the chain. In that second case the Architect re-sends the return message itself, by name, rather than treating the round as stalled or redispatching either worker. ### Number every message, so the count lives in the text @@ -58,7 +75,7 @@ The fix conversation is capped at five rounds. If a round five message still doe ### Failures nothing a writer can fix -Not every failure belongs in this conversation. An environment failure — a dead browser, a missing session, an expired credential, anything QA can't attribute to the code or the tests — goes straight to the Architect, never to `dobby:implementor` or `dobby:test-author` or any other writer: there is nothing for either of them to implement or test away. Reporting an environment failure to the Architect does not spend a round; the five-round cap counts only rounds where a writer had a real chance to fix something. +Not every failure belongs in this conversation. An environment failure — a dead browser, a missing session, an expired credential, anything QA can't attribute to the code or the tests — goes straight to the Architect, never to `implementor-t<id>` or `test-author-t<id>` or any other writer: there is nothing for either of them to implement or test away. Reporting an environment failure to the Architect does not spend a round; the five-round cap counts only rounds where a writer had a real chance to fix something. If a message in this conversation can't be delivered at all — the addressed sibling has died mid-task — the sender reports that to the Architect rather than retrying blindly; a sibling that has already died isn't going to answer a second attempt either. That round still counts toward the five, even though the message never landed, so an unlucky death can't be used to dodge the cap. @@ -91,6 +108,35 @@ Every worker appends its own record before it returns — the Architect never tr What reaches the Architect is a short verdict only: pass, fail, or blocked, plus one line on why. Nothing longer. That short verdict is what preserves the context isolation this protocol depends on — the Architect never reads a worker's full reasoning trail, only its outcome, so its own context stays small enough to run the whole plan without drowning in every task's detail. +## Report progress after every step + +Print a per-step status table every time ANY worker of ANY task returns a verdict, and every time a fix-round message lands (a round starting or ending) — not only at task boundaries, and not only at the close of the run. This is the user's live view of the whole plan while it runs. + +One row per PLANNED task (its `#` and title), one column per step THAT TASK actually has — `Test-author` only for a test-first task in a repo with a suite, plus `Implementor` and `QA` — and one emoji per cell: + +- ⚪ not started +- 🔄 in progress (add the round number once past round 1, e.g. `🔄 round 2 (fix)`) +- ❌ failed, in fix conversation (name the round, e.g. `❌ round 2`) +- ✅ passed +- — not applicable (e.g. `Test-author` for a task that isn't test-first) + +**Transitions** — which cell moves at each routing and re-check step: + +- QA reports a code defect (round N) → QA `❌ round N`, Implementor `🔄 round N (fix)`. +- The implementor's fix passes its Exit gate and it messages QA back → Implementor `✅`, QA `🔄 round N (re-check)`. +- A test-contract problem (round N) → Test-author `🔄 round N (fix)`, the reporter — Implementor or QA — `❌ round N`; when the test-author messages the implementor back (never QA directly) → Test-author `✅`, Implementor `🔄 round N (gate)`; once the implementor's gate is green and it messages QA → Implementor `✅`, QA `🔄 round N (re-check)`. +- QA passes → QA `✅` (task `done`); a fifth round that still fails → QA `❌ round 5 — needs-human`. +- A blocked or dead task keeps its last cells and gets a trailing note (`needs-human`, or `blocked by <id>`). + +Example: + +| # | Task | Test-author | Implementor | QA | +| --- | --- | --- | --- | --- | +| 1 | Add rate limiter | ✅ | 🔄 round 2 (fix) | ❌ round 2 | +| 2 | Update docs | — | 🔄 | ⚪ | + +This table complements, rather than replaces, the one-line narration of task starts/deaths (unchanged) and the closing summary table below — it's the running view; the summary table is the final tally. + ## Keep the run's state in STATE.md As the run advances, keep `STATE.md` current — each task's status and which round of its loop it is on — so the record on disk always reflects where the run actually stands, not just what still fits in the Architect's own context. This is what lets the Architect reconstruct progress after a compaction: read `STATE.md` back, and it is clear which tasks are done, which are still running, and which never started, without replaying the conversation that got them there. diff --git a/plugin/skills/spec/SKILL.md b/plugin/skills/spec/SKILL.md index 483f52a..9496dee 100644 --- a/plugin/skills/spec/SKILL.md +++ b/plugin/skills/spec/SKILL.md @@ -39,7 +39,7 @@ The `## Spec` section of the work-session doc (the repo-root `STATE.md`, created 1. Write the plan body (the `###` sub-headings — no `## Spec` heading of its own) to a scratch file OUTSIDE the repo, e.g. `"$TMPDIR/spec-<slug>.md"`. It's ephemeral working memory, never a committed artifact. 2. **`bunx dobby state set Spec --file <that file>`** — it replaces that one section body and preserves every other byte of the document. `## Spec` is re-settable, so a revision is just another `set`. No `STATE.md` at all (spec run standalone)? Run `bunx dobby state init --goal "<the goal>"` first, then `set`. -3. **`bunx dobby spec lint`** — it reads `<workroot>/STATE.md`'s `## Spec` and checks the sub-heading inventory, the task table (required columns — including `Test-first` when the repo has a runnable suite — non-empty `Task` / `Affected areas` / `Verify recipe` cells, dependencies pointing backwards only, and every `Affected areas` entry a real repository path — an existing directory/file, or one a task will create whose parent already exists), banned quality-gate commands in verify recipes, the `Manual verify setup:` line, and fenced blocks outside `### Decisions`. **Exit 0 is required before the Step 4 approval gate.** Findings → fix the plan, re-run `state set Spec`, lint again; never present a spec the linter rejects. +3. **`bunx dobby spec lint`** — it reads `<workroot>/STATE.md`'s `## Spec` and checks the sub-heading inventory, the task table (required columns — including `Test-first` when the repo has a runnable suite — non-empty `Task` / `Affected areas` / `Verify recipe` cells, dependencies pointing backwards only, and every `Affected areas` entry a real repository path — an existing directory/file, or one a task will create under a directory that already exists (any depth — the nearest existing ancestor just has to be inside the repo)), banned quality-gate commands in verify recipes, the `Manual verify setup:` line, and fenced blocks outside `### Decisions`. **Exit 0 is required before the Step 4 approval gate.** Findings → fix the plan, re-run `state set Spec`, lint again; never present a spec the linter rejects. 4. **Paste the lint output into the conversation** — the clean `ok` line (and, if you had to repair anything, what you fixed). The user approves a plan that is already machine-clean. Executors append what they did to the doc's `## Work log` (change, decisions/deviations, verify evidence) as tasks complete, via `dobby state append-worklog`. ADRs still go to `docs/adr/` at wrap-up, not here. diff --git a/plugin/skills/spec/references/task-decomposition.md b/plugin/skills/spec/references/task-decomposition.md index bd2964c..ce44755 100644 --- a/plugin/skills/spec/references/task-decomposition.md +++ b/plugin/skills/spec/references/task-decomposition.md @@ -9,7 +9,7 @@ Decompose the work into a task table the executor can dispatch from. - **Incremental expansion** — task 1 = minimal working version; each subsequent task adds a capability on top. - **Test-first marker** — when the repo has a test suite the `Test-first` column is REQUIRED (`dobby spec lint` fails the plan without it), and each task carries the flag (`yes` for tasks with real logic/seams, `no` for trivial config/prose/scaffolding) from the plan's Testing Decisions. `/dobby:execute`'s test-author gate reads this column. Omit the column entirely only when the repo has no suite. - **Atomic** — small enough for one agent to complete within ~50% of its context window. 3-4 files beats 8-10. Prefer many small tasks over few large ones. -- **Affected areas** — each area is a REAL repository path: an existing directory or file, or (for ground the task will CREATE) a path whose parent directory already exists. Never a prose label ("the gate", "CLI checks") for the same file — `dobby spec lint` rejects one, naming the cell. This column is what decides parallelism: `dobby build-plan` puts two tasks in the same wave only when their areas are disjoint, and the dispatch protocol compares them AS PATHS (build-protocol.md's "Overlapping writers"), so overlapping areas serialize themselves — a prose label defeats that comparison silently, because two unrelated strings never compare equal or as a prefix of one another. List multiple areas comma-separated, one real path per fragment — a parenthetical aside inside one area ("plugin/skills (research, dispatch)") splits on that same comma into mangled fragments and fails lint; give the parenthetical its own area or drop it. +- **Affected areas** — each area is a REAL repository path: an existing directory or file, or (for ground the task will CREATE) a path under a directory that already exists — any depth, the nearest existing ancestor just has to be inside the repo, so a module a task creates two levels deep, or a file inside a directory an earlier task creates, is a legal area. A `$param` route directory (TanStack Start/Router, Remix) is a real path, not decoration — `dobby spec lint` accepts the `$`. Never a prose label ("the gate", "CLI checks") for the same file — `dobby spec lint` rejects one, naming the cell. This column is what decides parallelism: `dobby build-plan` puts two tasks in the same wave only when their areas are disjoint, and the dispatch protocol compares them AS PATHS (build-protocol.md's "Overlapping writers"), so overlapping areas serialize themselves — a prose label defeats that comparison silently, because two unrelated strings never compare equal or as a prefix of one another. **Name the specific files a task creates or edits, not the whole directory, from the first draft** — the dispatch protocol treats a directory as a prefix of every file inside it, so a whole-directory area overlaps every sibling task that touches anything in that directory and serializes them needlessly. List multiple areas comma-separated, one real path per fragment — a parenthetical aside inside one area ("plugin/skills (research, dispatch)") splits on that same comma into mangled fragments and fails lint; give the parenthetical its own area or drop it. - **Dependencies** — express which tasks depend on which, and **always point backwards**: a `Depends on` cell may only name tasks ABOVE it in the table (`—` for none). Ordering the table this way makes a forward reference — and a cycle — impossible; both are lint findings. - **Verify recipe** — each task declares how it will be verified against the running app, written as **`action → observable`**: the action you take (drive the UI, fire the seam, run the query under the right role) and, after the `→`, the effect you must SEE. For UI work that's what to drive in the browser and what renders; for backend/data work the programmatic check and its result. Verify recipes observe BEHAVIOR — they never run lint/format/typecheck/build/the test suite (the edit-time hook and the pre-commit gate `dobby check --fix` own those); `dobby spec lint` rejects a recipe naming one of those commands, and rejects an empty cell. This makes verification planned, not improvised. - **Destructive marker** — a task whose VERIFY mutates shared state (writes/deletes rows the whole local backend shares, flips global flags, runs a migration) carries `Destructive: yes`. It's the one scheduling fact the executor can't infer: `dobby build-plan` gives a destructive task a wave of its own, so nothing verifies against that state concurrently. Omit the column entirely when no task in the plan is destructive; an absent or empty cell means "no". @@ -27,7 +27,7 @@ Instead of one "Notification system" task: "Notification + list endpoint + empty ## How to present -ONE markdown table under the spec's `### Tasks` sub-heading. `#`, `Task`, `Depends on`, `Affected areas` and `Verify recipe` are required (`dobby spec lint` checks them, `dobby build-plan` reads them). `Test-first` joins them as required whenever the repo has a runnable test suite — lint enforces it there too (see the plan's Testing Decisions) — and is dropped only in a repo with no suite. `Description` is optional — without it the title stands in as the task's spec. `Destructive` is the one truly optional column: add it only when some task's verify mutates shared state: +ONE markdown table under the spec's `### Tasks` sub-heading. `#`, `Task`, `Depends on`, `Affected areas` and `Verify recipe` are required (`dobby spec lint` checks them, `dobby build-plan` reads them). `#` is a plain task number — or, if not, a short token of only letters, digits, `_` and `-` (max 21 characters) — because the dispatch protocol names each worker `<role>-t<id>` and that address must stay a legal Agent name; `dobby spec lint` rejects a path-shaped id (`api/v2`), a spaced one (`task 1`), or one over the length cap. Number the tasks 1, 2, 3… unless there's a real reason not to. `Test-first` joins them as required whenever the repo has a runnable test suite — lint enforces it there too (see the plan's Testing Decisions) — and is dropped only in a repo with no suite. `Description` is optional — without it the title stands in as the task's spec. `Destructive` is the one truly optional column: add it only when some task's verify mutates shared state: | # | Task | Description | Depends on | Affected areas | Test-first | Destructive | Verify recipe | |---|------|-------------|------------|----------------|------------|-------------|---------------| @@ -44,6 +44,6 @@ ONE markdown table under the spec's `### Tasks` sub-heading. `#`, `Task`, `Depen | 4 | Unread badge with polling | Header badge shows the unread count, refreshing on an interval. | 1 | src/notifications, src/app-header | Browser: badge shows 2; mark one read → shows 1 within the poll interval | | 5 | Cross-tab sync | Reading in one tab updates the badge/list in another. | 2, 4 | src/notifications | Two tabs; read in A → B's badge updates | -This example comes from a repo with NO test suite — that is the only reason it carries no `Test-first` column; in a repo with a suite lint requires one, with a `yes`/`no` on every row. Each row names the approach/tools to follow and a concrete observable, and every recipe reads `action → observable`. A backend-only row instead verifies programmatically (a query under the right role, or firing a seam and observing the effect) — never by running lint/typecheck/build/the test suite, which are not verification. Had one of these rows needed a destructive verify (say a "purge read notifications" job that empties the table), it would carry a `Destructive` column with `yes` on that row so it lands in a wave alone. Each `Affected areas` cell here is a real path in that hypothetical repo — `src/notifications` is the directory task 1 creates (its parent, `src`, already exists), not a description of one; a plan for a repo without a `src/` layout names its own real paths the same way. +This example comes from a repo with NO test suite — that is the only reason it carries no `Test-first` column; in a repo with a suite lint requires one, with a `yes`/`no` on every row. Each row names the approach/tools to follow and a concrete observable, and every recipe reads `action → observable`. A backend-only row instead verifies programmatically (a query under the right role, or firing a seam and observing the effect) — never by running lint/typecheck/build/the test suite, which are not verification. Had one of these rows needed a destructive verify (say a "purge read notifications" job that empties the table), it would carry a `Destructive` column with `yes` on that row so it lands in a wave alone. Each `Affected areas` cell here is a real path in that hypothetical repo — `src/notifications` is the directory task 1 creates (its ancestor, `src`, already exists), not a description of one; a plan for a repo without a `src/` layout names its own real paths the same way. If the user rejects or asks for changes, regenerate the plan with their feedback before any execution.