--- name: Implement slug: implement-3 category: Automation description: Implement takes MCP work items from backlog to merged PR. It handles branching, schema-driven planning, implementation, review, and PR creation when you need to pick up a task or fix bugs. github: "https://github.com/jpicklyk/task-orchestrator/tree/main/.claude/skills/implement" language: Kotlin stars: 206 forks: 20 install: "npx degit https://github.com/jpicklyk/task-orchestrator/tree/main/.claude/skills/implement ~/.claude/skills/implement" installs_to: ~/.claude/skills/implement source_path: .claude/skills/implement/SKILL.md collection_size: 25 category_size: 1956 collection_url: "https://dirskills.com/collections/jpicklyk/task-orchestrator" added: 2026-09-05T05:28:54.221Z last_synced: 2026-09-05T05:28:54.221Z canonical_url: "https://dirskills.com/skills/implement-3" --- # Implement Implement takes MCP work items from backlog to merged PR. It handles branching, schema-driven planning, implementation, review, and PR creation when you need to pick up a task or fix bugs. **Install:** ```bash npx degit https://github.com/jpicklyk/task-orchestrator/tree/main/.claude/skills/implement ~/.claude/skills/implement ``` ## README # Implement End-to-end workflow for taking MCP work items from queue to PR. This skill composes the schema-driven planning (spec-quality), implementation, review (review-quality), and git/PR workflow into a single pipeline. **Usage:** - `/implement ` — work on a specific item - `/implement` — with context about what to work on - Can process single items or multiple items in batch --- ## Step 1 — Assess the Work Load the item(s) and determine the execution tier and interaction mode. For each item, call `get_context(itemId=...)` to understand: - Current role and gate status - Schema tag (feature-implementation, bug-fix, etc.) - Existing notes already filled - Dependencies and blocked status **Execution tier** — classify by this table (canonical source shared with the Workflow Orchestrator output style; edit the fragment, not this copy): | Criteria | Tier | Pipeline | |----------|------|----------| | 1-2 files, known fix, no migration/new API | **Direct** | Orchestrator edits, tests, reviews inline | | 3-10 files, single logical unit, clear or explorable scope | **Delegated** | Single subagent, separate review agent | | 11+ files, multiple independent work streams, dependency edges | **Parallel** | Worktree agents, full pipeline | **Force-UP signals** (bump tier regardless of file count): - Database migration → min Delegated - New public API surface → min Delegated - Multiple independent work streams → Parallel - User says "let's plan" / collaborative language → min Delegated **Force-DOWN signals:** - User says "just fix it" / "quick" → Direct (unless complexity contradicts) - Schema tag is `default` or absent → eligible for Direct If the item has no schema tag, apply `quick-fix` for Direct tier or leave untagged for Delegated/Parallel (the `default` schema catches these). **Trait application on classification.** When the tier resolves to Delegated or Parallel and the workspace defines a `delegated` trait (it appears in `availableTraits` on create responses), apply it before any dispatch — `traits: "delegated"` at item creation, or `manage_items(operation="update", items=[{itemId: "", traits: "delegated"}])` for an existing item. This makes the orchestrator-filled `delegation-metadata` note schema-visible instead of convention-only. Direct tier: do not apply it — nothing is delegated. **Test-author trigger rule.** `bug-fix.default_traits` already includes `needs-test-author` — no action needed, it applies automatically. For `feature-task` items, apply `needs-test-author` per item (`manage_items(operation="update", items=[{itemId: "", traits: "needs-test-author"}])`) when acceptance criteria involve a predicate, algorithm, parser, validator, or state transition, or when any force-ON signal is present: new public API surface, a database migration, a security predicate, or a prior vacuous-test finding in this item's area. Other schema tags (`feature-implementation`, `plugin-change`, `quick-fix`, and the global floor) do not carry the trait. Direct tier: apply the trait only in its **temporal-only degraded mode**, and only on bug-fixes — the `test-plan` gate and red-first rule still apply, `test-manifest` declares a single actor, and there is no separate test-author dispatch. Other Direct-tier items are exempt regardless of the criteria above — the tier is too small to separate authorship into a second dispatch. **Interaction mode** — orthogonal to tier: | Signal | Mode | |--------|------| | User says "work with me on", "let's plan", or similar collaborative language | **Collaborative** — user participates in planning and key decisions | | Scope is clear, no user participation needed | **Autonomous** — agent handles the pipeline | | Unclear scope or ambiguous complexity | **Ask the user** | When processing multiple items, evaluate whether related items (e.g., bugs in the same module, fixes that touch the same files) should be grouped into a single branch and PR. Group when the changes are cohesive and independent fixes would create merge conflicts. Keep items separate when they're unrelated or when isolation makes review cleaner. --- ## Step 2 — Prepare the Branch and Worktree Sync local main before any implementation begins. ```bash git checkout main git pull origin main --tags ``` The branching/worktree strategy depends on tier: **Direct tier** (orchestrator implements 1–2 files inline) — create a working branch on the main directory: ```bash git checkout -b ``` **Delegated tier** (single subagent) — same as Direct: orchestrator creates the branch on the main directory, the subagent works against it. No worktree. **If the main checkout is unavailable** (another branch checked out, uncommitted changes present) **or the orchestrator itself runs from a worktree** — Direct and Delegated tiers both: do not touch the occupied checkout. Create a dedicated worktree instead: ```bash git worktree add .claude/worktrees/ -b origin/main ``` Work there using absolute paths and `git -C` (never `cd`); after the PR merges, remove the worktree and delete the branch. Sync local `main` via `git fetch origin main:main` while `main` is not checked out anywhere, or a normal `git pull` from the main checkout when it is. **Parallel tier** (parent feature with multiple children) — create a **single feature worktree** that all child agents share: ```bash FEATURE_SLUG= # e.g. issue-117-followup FEATURE_BRANCH=feat/$FEATURE_SLUG FEATURE_WORKTREE=.claude/worktrees/feat-$FEATURE_SLUG # Resume detection — if the branch/worktree already exist (orchestrator restart # mid-feature), reuse them rather than recreating: if git show-ref --verify --quiet "refs/heads/$FEATURE_BRANCH"; then echo "Resuming existing feature branch $FEATURE_BRANCH" else git branch "$FEATURE_BRANCH" main fi if [ ! -d "$FEATURE_WORKTREE" ]; then git worktree add "$FEATURE_WORKTREE" "$FEATURE_BRANCH" fi ``` All child-task agents will be dispatched into this **shared** worktree (Step 4). The feature branch is pushed and PR'd **once**, when the parent feature reaches terminal (Step 6). **Why one worktree per feature, not per child:** the feature is the natural PR boundary. Per-child PRs created cross-PR test contamination and PR-body staleness during the #117 follow-up (see retro `a7f6024f`). Shared worktree means one commit history, one CI cycle, one PR — and the parent feature's review-checklist gives a coherent point at which to finalize. **Branch naming:** - `feat/` — feature-implementation parents (the integration branch) - `fix/` — bug-fix items (Direct or Delegated tier) - `fix/` — batch of related bug fixes (Delegated tier) - `chore/` — tech debt, refactoring (Direct tier) --- ## Step 3 — Queue Phase: Planning This step is **tier-conditional**: **Direct tier:** Skip this step entirely. No plan mode. No queue-phase notes. Call `advance_item(trigger="start")` immediately to move queue→work. The `quick-fix` schema has no queue-phase required notes, so the gate passes. **Exception:** if the item carries `needs-test-author` (temporal-only degraded mode, Step 1), `test-plan` is a queue-phase note and must be filled before this advance — see the seat-timing rule below. **Delegated tier:** Fill queue-phase notes per schema. Use `get_context(itemId=...)` to see `expectedNotes` and `guidancePointer`. Pre-plan-workflow is optional — use only if scope needs exploration. Post-plan-workflow only if child items need materialization. Advance: `advance_item(trigger="start")`. **Parallel tier:** Full planning pipeline: **Collaborative mode:** 1. Tell the user the item is ready for planning and ask them to enter plan mode. The `pre-plan-workflow` and `post-plan-workflow` hooks fire automatically on plan mode entry and exit, handling context gathering and materialization. 2. During planning, follow the `guidancePointer` for each required note — this will reference the spec-quality skill where applicable. 3. After plan approval and post-plan materialization, advance the item: `advance_item(trigger="start")` **Autonomous mode:** 1. Read and follow the `pre-plan-workflow` skill — gather existing MCP state, check schema requirements, and understand the definition floor. 2. Research the codebase — explore relevant files, understand current state. 3. Fill all queue-phase notes following the `guidancePointer` for each. The spec-quality framework applies regardless of mode. 4. Read and follow the `post-plan-workflow` skill — materialize child items if the plan calls for them. 5. Advance the item: `advance_item(trigger="start")` The gate will reject advancement if required notes are missing. If rejected, fill the missing notes and retry. **Do not confuse this with resource-lease contention.** A queue→work `advance_item` can also fail with `applied: false`, `errorCode: "resource_unavailable"`, `errorKind: "transient"` — a resource a trait on this item declares (`resources:`) is currently held by another item. This is not a note gate failure: filling notes will not fix it. Work a different item and retry later (`retryAfterMs` is a hint), or report the contended key(s) to the user. **Seat timing for `needs-test-author` items.** The queue-phase `test-plan` note is filled by the **planning seat** — the orchestrator (Direct tier) or the plan author (Delegated/Parallel tier), invoking the `test-author` skill's scenario-derivation and oracle-derivation sections — and this happens **before** `advance_item(trigger="start")` moves the item queue→work. This is by design: `test-plan` gates work entry, the same way any other required queue-phase note does. This is a distinct seat from Step 4b's **test author** seat, which writes test code and fills `test-manifest` at work phase — see Step 4b's intro for the two-seat distinction. --- ## Step 4 — Work Phase: Implementation **Verification commands.** Throughout this step, "run tests" means running BOTH the test suite AND the project linter: ```bash ./gradlew :current:test ./gradlew :current:ktlintCheck ``` CI enforces both — a green test run with failing lint will still block the PR. If `ktlintCheck` fails, run `./gradlew :current:ktlintFormat` to auto-fix formatting violations, then verify with `ktlintCheck` again and re-run tests. Include both commands in every implementation-agent and review-agent prompt. **Who runs the lint cycle** (proposal `ee6f5d32`, ~9 sessions of evidence): - **Delegated tier** (agent owns gradle): the implementation agent runs the `ktlintCheck` → `ktlintFormat` → re-verify cycle itself before committing. Validated 2026-07-13 (PRs #213/#214/#215): zero orchestrator fix-up commits. - **Parallel tier** (orchestrator owns gradle): agents include `:current:ktlintFormat` in their compile self-check (see the dispatch template below); the orchestrator additionally runs `ktlintFormat` before `ktlintCheck` after each commit batch — historically 3-5 fix-up commits per multi-phase run when skipped. **Capturing gradle's real exit code (use this pattern, not `2>&1 | tail -N`).** Piping gradle into `tail` discards gradle's exit code — `tail` always exits 0 on a successful read of the log, so `BUILD FAILED` at the end of the gradle output is reported by you as a successful run. Combined with gradle's daemon incremental cache, this can hide broken compilation through entire CI cycles (retro `568a8584`: 9 silently-failing tests shipped on PR #151 before the followup audit caught it). The reliable pattern, especially when running in `run_in_background`: ```bash ./gradlew :current:test > /tmp/gradle-out.log 2>&1; EXIT=$? echo "EXIT=$EXIT" tail -30 /tmp/gradle-out.log ``` **Redirect ordering matters:** use `> /tmp/log 2>&1`, NOT `2>&1 > /tmp/log` — the latter is evaluated left-to-right and leaks stderr (where gradle writes compile errors) to the terminal instead of the log (retro `ac25db89`: a compileTestKotlin failure was invisible in the captured log until the ordering was fixed). `EXIT=$?` captures gradle's actual exit code before any pipe consumes it. Read the captured exit code AND the tail of the log; never trust the tail alone. This applies to every orchestrator-owned gradle invocation throughout this step. **Use `--rerun-tasks` after dependency upgrades or large refactors.** Gradle's incremental compile cache retains class files from prior good builds. After a `gradle/libs.versions.toml` bump or a refactor that changes public API surfaces (removed methods, renamed types, sealed-class arms, generic-parameter shifts), incremental compilation can keep the OLD class files alongside source that no longer compiles, producing apparent BUILD SUCCESSFUL on stale bytecode. Run `./gradlew :current:test --rerun-tasks` once after such changes to force a clean run; ordinary incremental builds are safe afterwards. This step is **tier-conditional**: **Direct tier:** Implement directly. Edit the files, run the test suite. No subagent dispatch. No `/simplify` pass. **Exception — a Direct-tier `bug-fix` carrying `needs-test-author`:** follow Step 4b's Direct-tier test-first-then-fix sequence (test-plan frozen at queue → regression test observed red → fix → green) instead of implementing first. Fill the `session-tracking` note (required by both `quick-fix` and `default` schemas) with a brief summary of what changed and test results. Advance to review: `advance_item(trigger="start")`. **Delegated and Parallel tiers:** Use `get_context(itemId=...)` to see work-phase `expectedNotes` and `guidancePointer` values. Fill each required note following its guidance. Follow the delegation model from your output style (model selection, return formats, UUID inclusion). The key decisions at this step are: - **Single item (Delegated):** delegate to one implementation subagent or implement directly. Subagent works in the main directory on the working branch. - **Multiple child tasks, independent (Parallel):** dispatch parallel subagents into the **shared feature worktree** created in Step 2. Each agent receives the worktree path and branch name. Do **not** use `isolation: "worktree"` on the Agent tool — that would spawn a separate worktree per dispatch, which is the deprecated per-child PR pattern. - **Multiple child tasks, dependent:** dispatch sequentially into the shared feature worktree. Wait for each agent's commit to land before dispatching the next. **Test-file ownership boundary.** Every implementation dispatch (Delegated single agent or Parallel per-child agent) excludes `src/test/**` from scope — implementers do not create or modify test files. When a change surfaces a needed test update, the agent reports it in its return (or in `implementation-notes`) rather than editing the test itself; the test author (Step 4b) owns that file tree exclusively on items carrying `needs-test-author`. **Parallel dispatch into a shared feature worktree:** ``` Agent( prompt=""" Working directory: Branch (already checked out): feat/ Scope (modify ONLY these files): Do NOT create or modify any file under src/test/** — test authoring is a separate, independent dispatch (Step 4b) on items carrying needs-test-author. If your change surfaces a needed test update, report it in your return; never edit the test yourself. Format + compile self-check (REQUIRED before returning): ./gradlew -p :current:ktlintFormat :current:compileKotlin :current:compileTestKotlin > /tmp/agent-compile.log 2>&1; EXIT=$? If EXIT != 0, fix the reported error — a compile error OR a lint violation ktlintFormat could not auto-correct (e.g. line >140 chars, colons in backticked test names) — before committing and returning. Do NOT run :current:test or :current:ktlintCheck — orchestrator owns full build verification. ktlintFormat is formatting-only and idempotent; it prevents the recurring lint fix-up commits (proposal ee6f5d32). The compile self-check is fast (~3s) and catches type-mismatch / signature errors that gradle's incremental cache may otherwise mask in the orchestrator's later test run (retro `568a8584`: H3's `dbNow()` shipped with a Result vs Instant return type mismatch that was hidden for ~6 hours). After self-check passes, commit your changes with a descriptive message. """, model="sonnet", subagent_type="general-purpose" // NOTE: no isolation parameter — agents share the feature worktree ) ``` **File-edit overlap discipline:** Parallel agents in a shared worktree must operate on non-overlapping files. The orchestrator scopes each agent's prompt to a specific file list (per MEMORY.md §"Parallel File-Edit Delegation"). When inherent overlap exists, dispatch sequentially. **Contract-change sweep discipline.** When a child task tightens a contract — making a parameter required, adding `validate()` invariants, narrowing a sealed-class arm, or otherwise rejecting inputs that earlier passed — the orchestrator must sweep the rest of the codebase before advancing to review. Two recurrences (retros `a7f6024f` and `568a8584`) showed that: - Pre-existing test fixtures constructed under the old contract will fail under the new one. Example: H2's `WorkItem.validate()` claim-field invariants broke 8+ test fixtures that constructed mixed-state items via separate `Instant.now()` calls (microsecond drift) or partial claim fields. - The failure typically surfaces on a *different* PR's merge commit, not the PR that introduced the contract change — the original PR's tests passed because they used the new contract correctly. For each contract-tightening change in this run: 1. Identify the affected tool / class / method. 2. Grep all test files (and other call sites) for usages: `grep -rn "\|\|"`. 3. Verify every usage is consistent with the new contract. Update any that are not. 4. Re-run the full `:current:test` suite (orchestrator-owned, not the agent) to confirm no fixture-vs-contract conflicts surfaced elsewhere. 5. **Fixture repairs are orchestrator-owned and construction-only.** When a fixture fails under the tightened contract, the orchestrator may adjust how the fixture is *constructed* (fix the stale call site) but must never adjust what it *asserts*. Anything that would touch an expectation instead of a construction call is not a fixture repair — re-dispatch the test author (this preserves the independence the trait exists to protect). **Declare these commits in the review handoff:** list each orchestrator fixture-repair commit's SHA alongside the impl-range/author-range pair (see Step 4's tracking table and Step 4b's disjointness check) so review-quality can tell a declared fixture repair apart from a silent implementer edit to `src/test/**`. 6. **Doc-claims sweep after behavior-changing fixes:** when an orchestrator-owned bug-fix changes shipped behavior after documentation was authored (e.g. a tokenizer or default flips mid-run), grep all changed docs for claims about the OLD behavior before finalizing (retro `ac25db89`: "case-insensitive" survived in 3 places after the fix made search case-sensitive). This sweep is part of the orchestrator's verification step between waves, not the implementing agent's responsibility — agents are file-scoped and can't see the full fixture surface. **Model selection — always set `model` explicitly on every Agent dispatch:** | Agent purpose | Model | |--------------|-------| | Implementation (production code only) | `model="sonnet"` | | Independent test authoring (Step 4b) | `mo