--- name: implement-code description: "Sub-skill of agile-10-implement. Branch off base, implement the plan per ADR + Specs UI with full AC test coverage, finish only when the mode's gate is green, then commit and push. Does not open the PR. Also runs the fix pass for review findings. Not user-invoked." user-invocable: false --- # implement_code ## Host execution **Claude Code:** retain the agent-dispatch and concurrency behavior defined below. **Codex:** use only the inline behavior stated here. On Codex this sub-skill runs inline under `agile-10-implement` with `concurrency=0`; never spawn or assume a named agent. Perform its full gate and return its normal receipt to the caller. ## Purpose Build phase for `agile-10-implement`. Invoked with a planned ticket — and, on a fix pass, the numbered review findings. Posts the `🤖 agile:phase=implement` marker. **Does not open the PR**; `implement-pr` does that after this phase returns. **Build mode** — the orchestrator passes `mode=sequential | concurrent` (default `sequential`): - **sequential** — the only phase that touches the shared Docker Compose stack, so it runs strictly one ticket at a time and runs the **full** gate locally. - **concurrent** — runs inside **the ticket's** git worktree (created by the orchestrator, shared with that ticket's other phases) alongside other tickets' builds. A worktree isolates the filesystem, but the stack is a **shared external resource**, so this phase must not touch it: run the **stack-free** gate only (lint + unit + typecheck + migration linearity) and **defer integration + e2e + apply-on-fresh-DB to CI**. Their AC tests are still *written*, just not executed here. **Autonomous — never prompt the user.** Decide and document everything reversible, flagging it for the reviewer. The only stop is a *critical* decision (irreversible or high-blast-radius **and** not derivable from the ADR / PRD / Specs): return `critical` to the orchestrator, which parks that one ticket and asks. This holds in `concurrency=0` inline mode too, where no agent wraps this skill. ## Load the plan — implement *from* it Read the `🤖 agile:phase=plan` comment on the ticket (written by `implement-plan`). That plan is what you implement: its files-to-touch, its order (data → service → API → frontend → tests), its AC→test map. Do not re-derive the approach. On a resumed run the orchestrator passes only the ticket key, so always read the plan from Jira and re-read the ADR / Specs UI for the detail it references. A forced deviation gets noted (and follows the reversible/critical rule below). ## Set up workspace and branch **Sequential mode** — `git checkout && git pull`, then create or reuse `` branched **off ``** — never off another feature branch. Idempotent: `gh pr checkout` / `git checkout -B` only when no open PR branch exists for this ticket. **Concurrent mode — you are already in the ticket's worktree, already on its branch**, both created by the orchestrator before `validate`. Do **not** run `git checkout ` here: git refuses to check out a branch another worktree (the shared checkout) already holds, so it fails outright — and were it to succeed it would drag your worktree off the ticket's branch. Verify rather than switch: `git branch --show-current` matches the ticket's branch, and `git status --porcelain` + `git log --oneline @{u}..` tell you whether an earlier attempt already left work here — a re-dispatch after a crash re-enters this same tree, so build on what is there instead of redoing it. Need a fresher base? `git fetch origin ` then rebase; never `pull` a branch you are not on. ## Implement - **Follow the ADR exactly.** No new pattern, library, or architectural decision without flagging it (PR body + a `🤖` Jira comment). Never silently deviate — and **implement every Specs UI state** (default / loading / empty / error / success), flagging deviations rather than silently "improving". - **Cover every AC with a test** that exercises real behaviour, not the mock and not a restatement of the implementation. Each edge-case AC gets its own test. - **A test's expected value comes from the system under test or an invariant — never a hand-guessed constant.** A number you reasoned out by hand can encode a wrong premise that passes locally and fails at the stack tier. Assert a relationship, or capture the value from the system as a golden. - **Write the minimum that satisfies the spec — no slop, no dead code.** Match the surrounding file's idioms. No speculative abstraction, unused imports/variables/params, commented-out code, or "just in case" branches. A new file or export must be wired (imported, routed, referenced) or it is dead, and a change that makes existing code unreachable deletes it. Name from the PRD/ADR domain vocabulary. Comment only to state a constraint the code cannot. - **A change to a shared symbol ripples — update every site in lock-step.** When you edit a shared type, query/mutation, function signature, fixture, or mock, grep the whole repo for its other construction and call sites. A targeted local test on the file you touched passes while full CI fails on an untouched site; the grep-then-fix is what closes that green-locally / red-in-CI gap. - **A guard that asserts on source TEXT must parse it, not grep it.** A guard enforcing something *about* code — "nothing here calls `X`", "this literal appears once", "every entry is registered" — will eventually run against a file whose comments *discuss* `X`, and it then passes on the prose explaining why the rule exists. The rule disarms itself precisely when someone documents it well, and it fails silently: the guard is green, so nothing points at it. The same trap fires when a scan's own output contains its identifiers (a collected-test listing includes the guard's own name, so a scan of that listing matches itself). **Walk the AST, or strip comments and docstrings before scanning** — and prove it: mutate by moving the offending code *into a comment* and confirm the guard still reddens. - **A fix to a race, flake or ordering bug is proven by REPETITION, not by one green run.** The defining property of an intermittent failure is that it passes most of the time, so a single green run of its fix carries almost no information — it is the outcome the bug produces anyway. Run the affected tests **≥10 consecutive times under the conditions that made them flake** (full parallelism, the sibling load, the real scheduler) and report the count. Anything less is a fix asserted, not verified; a fix that is itself flaky is indistinguishable from a fix that worked until it is run enough times to tell them apart. - **Verify a runtime surface by exercising the flow, not just by green tests.** Green suites prove the covered code behaves; they do not prove the feature works. Drive the actual path — run the app, hit the endpoint, render the view — and observe the real result. A stale cache after a mutation, a missing affordance, an unwired handler: all pass every test. **Concurrent mode:** if that needs the shared stack, defer the runtime check to Phase 2 (`implement-monitor` holds the stack) or CI; a stack-free check (rendering a component, a CLI invocation) still runs in-worktree. **Forced ADR-uncovered decision:** **reversible** → take the lower-risk option, post a `🤖` comment with the choice + rationale, flag it in the PR, keep going. **Critical** (irreversible / high blast radius — destructive migration, auth/security, breaking a shared contract, a new paid or infra dependency, data-loss risk) → stop and return `critical`; the orchestrator parks the ticket and escalates. Never guess a critical decision. ### Finish gate Not done until all of these hold on the latest pushed commit. If any applicable gate fails, keep working (or return `critical`) — never return success and never hand off to `implement-pr`. 1. **All lint gates pass** — "lint" means *every command the CI lint job runs*, not just the formatter. CI lint jobs bundle extra checks (style/asset validators, i18n or dead-string checks, schema-drift and generated-file guards, bundle-size budgets, custom scripts): read the CI workflow and run each locally. Verify by **real exit code** — a piped or `xargs` exit can mask the tool's own status, and a gate can print a per-item `PASS` line while exiting non-zero on a different item, so never read a verdict out of the output. A missed gate fails CI and **skips** the downstream jobs. **A gate that reads build output verifies the build's own exit code first** — otherwise it silently reports on the previous build's artifacts. *(both modes)* 2. **Stack-free tests green** — unit + typecheck, locally, with no skips or xfails hiding a failure. *(both modes)* If this project's "unit" tests actually hit the DB, they are not stack-free: say so and stop rather than running them in a worktree — the run needs `concurrency=1`. 3. **Stack-bound tests** — integration + e2e. **Sequential:** run locally and green. **Concurrent:** written but **deferred to CI**; record the deferral in the marker. CI's run on the open PR is their gate, enforced at merge by `agile-11-merge-train`'s fresh-CI-green hard gate. 4. **Every AC satisfied and test-covered — and the load-bearing one MUTATION-PROVEN** — walk the plan's AC→test map. An AC with no test fails the gate; so does an AC whose test *cannot* fail. Take the AC whose silent breakage would cost most **among those this mode can execute**: break what it guards, watch the test go **RED**, revert. **Confirm the mutation applied** — a formatter that rewrote the target line, or an edit script that died before writing, yields a green run indistinguishable from a passing guard. **A mutation that applied cleanly and reddened ZERO tests is a FINDING, not a completed proof** — it is direct evidence the guard is decorative, so rewrite the test until the mutation reddens it (or re-plan the AC) and record both attempts in the marker; never report `0 RED` as a satisfied gate, and never conclude the code is fine because the suite stayed green. Expect it on the mutation you were most confident about: a framework that de-duplicates, batches or caches can absorb the injected defect and leave the assertion blind, which is itself the finding. **Sequential:** any AC. **Concurrent:** a stack-free one — a stack-bound AC cannot be run, so it cannot be proven here; say so in the marker. 5. **If the change crosses a producer→consumer boundary, the mutation targets the SEAM** — when this ticket adds, renames or drops a field that one side writes and another reads (a stored column surfaced by an API, a message or event payload, a serialized contract, a fixture two tiers share), the load-bearing mutation is **that field, broken at the PRODUCER**, and it must redden a test on the **CONSUMER's** side — or on a single artifact both sides read. **A red that only trips the producer's own test proves nothing**: that test restates the same literal, so a developer who renames both together — which is what a rename actually looks like — still ships it broken. Both sides being independently well tested is the *normal condition* for this defect, not a defence against it: each asserts against its own restatement of the contract and nothing compares them, so the seam is covered by neither. Where the two sides cannot be made to share a test, prefer **one artifact both read** (a golden generated from the producer, consumed by the other tier) over two assertions that merely happen to agree. If the mutation reddens nothing, that is the finding from gate 4 — the seam is untested, not fine. 6. **A fix to an intermittent failure is repeat-verified** — if this ticket fixes a race, flake, or ordering bug, run the affected tests **≥10 consecutive times** under the conditions that produced the flake and record the count (`10/10`). One green run is not evidence here. If the repeat run cannot be done in this mode (it needs the shared stack), say so and defer it to CI rather than claiming the fix verified. 7. **Migration history-linearity** (static, both modes) — if the change adds a migration, confirm the history resolves to a **single latest version**: no colliding or duplicate version identifiers, no two scripts sharing a parent. A split history makes the migrate step run an older or no-op version and **silently skip the new schema objects** while tests pass against a stale schema. 8. **Migration apply-on-fresh-DB** — apply on a clean database, confirm the expected objects exist, re-apply once for idempotency. **Sequential:** here. **Concurrent:** deferred to CI (7 is the local half). ## Fix pass (re-invoked after `implement-review`) Fix **every** numbered finding — Critical *and* Minor; Minor is a severity, not a deferral. The only acceptable unfixed finding is genuinely separate work (see the mid-phase test: would a reviewer accept the ticket as done without it?), never one that is merely more effort or touches more files: file a follow-up ticket inline and note it in the PR. **Point that ticket at creation** on the project's normal estimation scale — it never passes back through refinement, so unpointed here is unpointed forever; if it genuinely cannot be sized yet, label it `unsized` with a one-line reason rather than leaving the field empty. Re-read the changed files afterwards (fixes introduce bugs), then re-run the mode's gate green. ## Commit and push **Checkpoint early; the gate governs hand-off, not committing.** This phase can die mid-flight for reasons unrelated to the code — a session/usage limit, an API error, an OOM, a crash. In concurrent mode the ticket's worktree survives that and a re-dispatch re-enters it, so an uncommitted tree is recoverable — but only by a human or an orchestrator that thinks to look. A commit makes the work legible (`git log @{u}..` answers "how far did it get?" in one line), and a push makes it survive the worktree being cleaned up. So: **once the code compiles and the branch exists, commit a WIP checkpoint and push it** (`wip: …`, or amend as you go), then keep working toward the gate. Pushing early costs nothing — the PR is not opened until `implement-pr`, and the ticket only counts as handed off on the *gated* marker. Squash the WIP into the single intended conventional commit before hand-off, and never post the `🤖 implement` marker on a WIP push. Final commit: conventional, with `Refs: ` and the `Co-Authored-By` trailer in the body. **Stage explicitly, then verify the commit captured every intended file** — two silent-omission traps each cost a green-locally / red-in-CI round trip: - **A bad pathspec aborts the whole stage.** `git add a b c` where one path does not exist can stage *nothing*, yet a following commit of separately-staged files still succeeds and ships a partial change. Prefer `git add -A` (or real paths only), and after committing confirm `git status --porcelain` is empty of files belonging to this change. A leftover lockfile, generated file, or barrel/index export builds locally (your tree has it) and breaks CI (the branch does not). - **A dependency and its lockfile travel together**, plus any index/barrel that re-exports new modules. A component committed without its dependency, or without being exported, compiles in your tree and fails on a clean checkout. Confirm with `git show --stat HEAD` that every expected file is there and `git status` is clean. Only then post the marker. ## Marker — mandatory, exact format Post via `mcp__atlassian__addCommentToJiraIssue` (`contentFormat="markdown"`). The comment **must begin with the literal HTML comment** or resume detection (which greps `🤖 `) misses it and the phase re-runs. The gate receipt — each command with its **real exit code**, plus any `DEFERRED TO CI` line — is what the orchestrator verifies against the pushed branch; a marker asserting green with no per-command exit codes fails the gate, and one asserting AC coverage with no `Mutation:` line fails it too. Never delete prior markers. ``` 🤖 **implement — agile-10-implement — ** Mode: Gate receipt: lint: → exit 0 unit: → exit 0 integration: → exit 0 | DEFERRED TO CI (concurrent — worktree cannot hold the stack) migration: history-linear ✓ | apply-on-fresh-DB → exit 0 | DEFERRED TO CI repeat: → 10/10 green | n/a (no intermittent-failure fix) | DEFERRED TO CI (stack-bound; one green run is NOT the proof) AC coverage: / (stack-bound ACs: written, CI-gated) Mutation: AC — → RED, reverted (N ≥ 1, or it is not a proof) seam: broken at → RED at | n/a (no boundary crossed) | AC — → 0 RED — FINDING: guard was decorative; → RED, reverted | AC re-planned> | DEFERRED TO CI (concurrent — the load-bearing AC is stack-bound) | n/a — ```