diff --git a/CHANGELOG.md b/CHANGELOG.md index f94d955..afc36b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes. ## 2026-07-19 — Epic workflow and review contract cleanup +- feat: add standalone ticket implementation skill - feat: integrate repository-owned review into epic execution + (28c3945b3db8f84a812cd2e498d54a6912bcd934) - feat: compose repository-owned code review (556fea80b6970b97c31e819693f43c251b7b3796) - feat: add local code simplicity review diff --git a/README.md b/README.md index 8aaea79..259706c 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,11 @@ A personal monorepo for agent skills and supporting scripts. Current skills: +- `skills/implement-ticket` — implement exactly one standalone ticket or named + epic child through isolated execution, repository-owned review, PR gates, and + authorized merge and cleanup; this is the canonical owner of generic + single-ticket execution rules, with duplication in `implement-epic-sequence` + temporary until its dependent orchestrator refactor - `skills/review-code-change` — orchestrate the repository-owned review lenses into one evidence-bound, deduplicated verdict - `skills/implement-epic-sequence` — execute GitHub or Linear epics through @@ -38,6 +43,7 @@ Run skill-specific tests: ```bash just test-prepare-changesets just test-review-suite +just test-implement-ticket ``` Validate a review packet and result together: diff --git a/justfile b/justfile index 94d89be..a7568b3 100644 --- a/justfile +++ b/justfile @@ -28,6 +28,9 @@ test: test-review-suite: python3 -m unittest discover -s review-suite/scripts/tests -p 'test_*.py' +test-implement-ticket: + python3 -m unittest discover -s {{skills_dir}}/implement-ticket/scripts/tests -p 'test_*.py' + test-prepare-changesets: python3 -m unittest discover -s {{skills_dir}}/prepare-changesets/scripts/tests -p 'test_*.py' diff --git a/skills/implement-ticket/SKILL.md b/skills/implement-ticket/SKILL.md new file mode 100644 index 0000000..ffe61ff --- /dev/null +++ b/skills/implement-ticket/SKILL.md @@ -0,0 +1,293 @@ +--- +name: implement-ticket +description: Implement exactly one standalone GitHub or Linear ticket, or one named child of a larger epic, through an isolated branch and pull request. Use when an agent should resolve live ticket and dependency context, enforce readiness and authority boundaries, implement and validate one coherent change, invoke the repository-owned review-code-change skill in a fresh read-only context, handle current-head remote gates, and optionally merge and clean up when explicitly authorized. Detect whole-epic requests before mutation and route them toward implement-epic without creating a circular skill dependency. +--- + +# Implement Ticket + +Implement one independently reviewable ticket without selecting sibling work or +claiming a parent epic is complete. Treat live tracker and repository evidence +as execution state; use old plans or summaries only for orientation. + +Treat this skill as the canonical owner of generic single-ticket readiness, +implementation, review, PR, merge, base-drift, feedback, tracker-transition, +cleanup, and terminal-reporting rules. Duplication in `implement-epic-sequence` +is temporary until the dependent epic-orchestrator refactor removes it. Do not +create a third shared workflow abstraction merely to eliminate that transition. + +## Load the applicable references + +- Read [the GitHub adapter](references/github.md) whenever GitHub owns issue + state or hosts the repository and pull request. +- Read [the Linear adapter](references/linear.md) whenever Linear owns ticket, + parent, dependency, or status state. +- Always read [review and merge gates](references/review-and-merge-gates.md) + before publishing the pull request. +- Always read [cleanup and result](references/cleanup-and-result.md) before a + merge or terminal handoff. + +For cross-system work, record which system owns issue status, dependency state, +source code, pull requests, checks, reviews, and merge. Never substitute a +same-numbered issue from the PR host for the real tracker ticket. + +## Require compatible runtime capabilities + +A compatible agentic runtime must be able to: + +- load this skill and repository-owned skill dependencies; +- read repository instructions, tracker state, and structured relationships; +- inspect and create isolated branch/worktree state; +- edit files, run commands, commit, push, and manage PRs when authorized; +- invoke a fresh read-only reviewer worker, subagent, or equivalent isolated + context; +- poll or wait for asynchronous CI and review gates; and +- read thread-aware PR feedback. + +Stop with an explicit missing-capability result when an applicable capability is +unavailable. Product-specific discovery metadata such as `agents/openai.yaml` +may exist, but it does not constrain the operating contract or require a +particular agent product. + +## Resolve the operating contract + +Before mutation, discover or receive and verify: + +- live ticket identity, body, state, scope-affecting comments, owning tracker, + and relevant native relationships; +- repository, PR host, current remote base, and repository instructions; +- parent outcome, closed-prerequisite evidence, and sibling-owned contracts + needed to understand whether this ticket can ship independently; +- named architecture, design, contract, migration, and rollout documents; +- completion policy: ready PR only, merge after gates, or merge plus manual + ticket transition; +- whether the owning tracker's PR reference will automatically close or + transition the ticket when merged, with that consequence stated explicitly in + the completion policy; +- required local, CI, human, connector, thread, build, integration, and manual + validation gates; and +- authority for ticket edits, dependency changes, follow-up creation, review + replies and resolution, merge, branch deletion, manual ticket transitions, + deployment, production mutation, and destructive operations. + +Use this default authority matrix unless the user or repository is stricter: + +- `ready PR only` permits isolated implementation, validation, commit, feature + branch push, PR creation or update, evidence-based review replies, and + resolution of fully addressed threads; +- `merge after gates` additionally permits merging this ticket's PR and safely + deleting its verified merged feature branch; +- `merge plus manual transition` additionally permits the explicitly requested + status or close transition for this ticket only; +- ticket-body edits, dependency mutations, and follow-up creation require + explicit ticket-management authority; and +- deployment, production mutation, destructive data operations, and parent + closure always require separate explicit authority. + +Do not infer merge, issue-close, parent-close, deployment, or production +authority from words such as `implement`, `finish`, `complete`, or `end to end`. +When merge authority is unclear, stop at a ready PR. + +Treat an automatic ticket transition caused by the selected closing syntax as a +disclosed consequence of authorized merge, not as an independently requested +manual transition. State that consequence before publication or merge. Do not +use automatic closing syntax when its effect conflicts with the resolved +completion policy. + +For a merge-inclusive run, verify before implementation that +`review-code-change` is available and readable. It is the only local +adversarial-review dependency. Return `blocked` when it is unavailable; do not +substitute a third-party skill, generic self-review, or unreviewed merge path. + +## Establish source-of-truth precedence + +Use this order: + +1. Current user instructions. +2. Live ticket, relationship, branch, PR, and review state. +3. Repository `AGENTS.md` and equivalent local instructions. +4. Named architecture, design, contract, migration, and rollout documents. +5. Current code and tests. +6. Prior summaries or memory. + +Stop on a material conflict. Do not choose the most convenient interpretation. + +## Guard whole-epic scope before mutation + +Determine whether the requested item is itself an epic whose requested outcome +requires implementing a child graph. Prefer authoritative structured evidence: +native issue type, native parent/sub-issue relationships, and explicit user +scope. Labels and prose may support the decision but must not silently override +contradictory native state. + +- Treat a named child of an epic as ordinary `implement-ticket` scope. +- Treat an epic with children, or an explicitly identified undecomposed epic + requested as a whole, as `implement-epic` scope. +- Permit work directly on a parent only when the user explicitly requests a + genuinely independent one-PR deliverable owned by that parent and the normal + readiness gate proves it can ship without implementing children. + +For whole-epic scope, stop before branch creation or any other mutation and +return `requires_epic`. Name `implement-epic`, preserve the resolved +tracker/repository context, and include the stable marker +`implement-ticket:requires-epic::`. If the same marker is +already present in the incoming handoff, return `blocked` with a routing-cycle +reason instead of redirecting again. + +Do not invoke or require `implement-epic` from this skill. The executing host or +caller may route the handoff when that skill is available. If it is unavailable, +report the missing capability explicitly without flattening the epic into one PR +or implementing children. + +## Apply the ticket readiness gate + +Proceed only when the selected ticket: + +- is open and is not already implemented, superseded, or represented by a + canonical open or merged PR or branch; +- has no unresolved native blocker; +- has every required closed-blocker outcome verified in its authoritative + repository, artifact registry, tracker, or environment; +- has a clear observable goal, acceptance criteria, non-goals, preserved + behavior, and required verification; +- contains no unresolved product, data, authorization, migration, destructive, + or architecture decision; +- represents one coherent, independently reviewable PR; and +- can merge without exposing incomplete, misleading, or unusable behavior. + +Treat a closed, canceled, or not-planned prerequisite as unresolved when its +required outcome is absent. Read parent and sibling context as evidence, not as +permission to widen scope. Return `blocked` with the missing outcome when an +unimplemented sibling is required; never absorb that sibling into this PR. + +When an open canonical PR or branch already owns the ticket, return `blocked` +with its identity and require explicit ownership transfer before modifying it; +do not report another worker's candidate as this run's `ready_pr`. When a merged +PR is verified on the base and the ticket is already complete, return `merged` +with that evidence without creating new implementation state. + +When ticket editing is authorized, make an unclear ticket implementation-ready +and re-read it. Otherwise stop with the missing decision rather than +improvising. + +## Execute one ticket + +### 1. Create exclusive implementation state + +- Confirm the primary checkout and registered worktrees. +- Fetch current remote state. +- Create one feature branch and clean isolated worktree from the verified base, + unless the current clean worktree is already the user's explicit ticket + workspace. +- Use one ticket per branch, worktree, and PR. +- Install documented dependencies and start required local services before + classifying missing-tool failures as feature failures. + +Standalone execution may mutate the primary context. A delegated worker, +subagent, or equivalent context must own exactly one verified worktree and +feature branch exclusively. Never allow two implementation contexts to mutate +the same candidate. Preserve unrelated branches, worktrees, and user changes. + +### 2. Implement only the live contract + +- Read nearby code and tests before editing. +- Preserve explicit non-goals and named existing behavior. +- Follow established architecture, idioms, shared modules, and extension points. +- Add focused behavior tests with the implementation. +- Update executable contract or contributor documentation when behavior changes. +- Avoid speculative backfills, compatibility layers, abstractions, or adjacent + ticket work for conditions not evidenced by the ticket or repository. + +Apply incidental changes only for a demonstrated ticket-scoped correctness, +security, acceptance, architecture, or validation need. Defer polish, broad +refactors, hypothetical hardening, and sibling work. + +### 3. Validate in layers + +Discover commands from repository instructions and tooling. Run: + +1. focused tests for changed behavior; +2. relevant static checks; +3. the complete required repository gate; +4. integration tests with documented real dependencies; and +5. required build, packaging, or manual checks. + +Report commands and exact outcomes. Distinguish bootstrap or environment +failures from feature failures. Do not claim completion while required +validation is failing or unavailable. + +### 4. Publish one focused PR + +- Recheck that no PR or branch already owns the ticket. +- Commit with repository conventions and push only the feature branch. +- Describe the branch-wide outcome, important non-goals, and actual validation. +- Use the owning tracker's correct closing or reference syntax. +- Confirm the PR base and head match the ticket worktree. + +Do not combine independently useful tickets in one PR. + +### 5. Run bounded repository-owned review + +Follow [review and merge gates](references/review-and-merge-gates.md). Keep +every mutation in the implementation context. Give `review-code-change` only raw +live ticket, repository, full diff, candidate identity, validation, and worktree +evidence in a fresh read-only context. Exclude the implementation transcript, +intended solution, prior conclusions, and suspected findings. + +Apply only material ticket-scoped blocking and strong-recommendation findings. +Preserve deferred findings without expanding scope. After a fix, rerun affected +and required validation, commit and push a new head, rebuild the evidence, and +follow the suite's re-review instruction. Use at most three full fix/re-review +cycles by default. + +Treat a missing dependency, malformed result, `blocked` verdict, reviewer +mutation, or unavailable required evidence as a failed local gate. The review +suite stays read-only; this skill owns accepted fixes, GitHub replies, thread +resolution, commits, pushes, merge, and cleanup within granted authority. + +### 6. Apply current-candidate remote and merge gates + +Do not equate CI success, stale approval, or zero threads with a clean review. +Require every applicable local, CI, human, connector, comment, formal-review, +and thread gate for the current candidate. Follow the risk-based base-drift +rules in the gate reference. + +When merge is authorized and every gate passes, merge using the repository's +approved method. Verify remote merge state, mainline representation, and the +owning tracker's ticket transition before cleanup. For an epic child, reread the +affected native dependency relationships after that transition and report newly +unblocked work without selecting or mutating it. Never close or verify a parent +epic from this skill. + +## Stop conditions + +Stop and return `blocked` when: + +- ticket scope and native relationships conflict materially; +- implementation requires an unresolved product or architecture choice; +- a prerequisite outcome, authority, credential, approval, or required + infrastructure is missing; +- correctness would materially exceed one-ticket scope; +- review feedback requires redesigning the ticket; or +- required validation remains unavailable after documented bootstrap attempts. + +Difficulty, a long test suite, ordinary CI wait time, or independently ready +sibling work is not a blocker. + +## Return one terminal handoff + +Follow [cleanup and result](references/cleanup-and-result.md). Return exactly +one terminal state: + +- `ready_pr`: the one-ticket PR exists at the reported candidate; state every + remaining remote or authority gate, and confirm this run owns or was + explicitly handed ownership of the candidate; +- `merged`: the PR is verified on the base, the ticket transition is verified, + and authorized cleanup is complete or precisely limited; +- `blocked`: give one concrete blocking reason and next action, preserving any + partial artifacts; or +- `requires_epic`: no mutation occurred and the handoff names `implement-epic` + with its stable routing marker. + +When this ticket is an epic child, report newly unblocked downstream work after +merge but do not select or implement it. Never claim whole-epic acceptance or +close a parent. diff --git a/skills/implement-ticket/agents/openai.yaml b/skills/implement-ticket/agents/openai.yaml new file mode 100644 index 0000000..18ede93 --- /dev/null +++ b/skills/implement-ticket/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Implement Ticket" + short_description: "Implement one ticket through a reviewed PR" + default_prompt: "Use $implement-ticket to implement this ticket through the authorized validation, review, PR, and merge gates." diff --git a/skills/implement-ticket/evals/cases.json b/skills/implement-ticket/evals/cases.json new file mode 100644 index 0000000..0e45e4e --- /dev/null +++ b/skills/implement-ticket/evals/cases.json @@ -0,0 +1,97 @@ +[ + { + "id": "standalone-ready-pr", + "request": "Implement GitHub issue G-21.", + "tracker_state": "Open standalone PR-sized ticket with complete requirements, no blockers, and no existing implementation.", + "authority": "ready_pr_only", + "capabilities": ["review-code-change", "isolated_worktree", "github_pr"] + }, + { + "id": "authorized-merge-cleanup", + "request": "Implement and merge GitHub issue G-22.", + "tracker_state": "Open ready standalone ticket with no competing implementation.", + "authority": "merge_after_gates", + "candidate_state": "Focused and full validation, local review, CI, and required remote review are clean for the current head; cleanup preconditions pass." + }, + { + "id": "named-epic-child-only", + "request": "Implement child G-23 only.", + "tracker_state": "G-23 is a native child of G-20, has no blockers, and can ship independently; siblings are also ready.", + "authority": "ready_pr_only" + }, + { + "id": "open-native-blocker", + "request": "Implement GitHub issue G-24.", + "tracker_state": "G-24 is open and natively blocked by open G-19; G-19's outcome is required." + }, + { + "id": "closed-prerequisite-outcome-missing", + "request": "Implement GitHub issue G-25.", + "tracker_state": "Blocker G-18 is closed as not planned, but its required API contract is absent from the authoritative main branch." + }, + { + "id": "sibling-outcome-missing", + "request": "Implement epic child G-36 only.", + "tracker_state": "G-36 belongs to epic G-33 and has no open native blocker, but it cannot ship independently because required shared-contract work owned by sibling G-37 is not implemented." + }, + { + "id": "canonical-pr-owned-elsewhere", + "request": "Implement GitHub issue G-26.", + "tracker_state": "An open canonical PR already implements G-26 and is owned by another active worker; no ownership transfer occurred." + }, + { + "id": "correctness-fix-and-rereview", + "request": "Implement and merge GitHub issue G-27.", + "authority": "merge_after_gates", + "candidate_state": "Validation passes; review-code-change returns one blocking correctness finding, then returns clean on the committed fixed head; CI passes and no other remote review is required." + }, + { + "id": "clean-local-review-remote-pending", + "request": "Implement GitHub issue G-28 to a ready PR.", + "authority": "ready_pr_only", + "candidate_state": "Local validation and review are clean; required current-head human and connector approvals remain pending; zero threads are unresolved." + }, + { + "id": "unrelated-base-drift-retained", + "request": "Merge the reviewed PR for G-29.", + "authority": "merge_after_gates", + "candidate_state": "The base advanced only in unrelated documentation; effective diff and resulting candidate tree are unchanged, no conflict or relevant overlap exists, and repository policy permits retention." + }, + { + "id": "linear-ticket-github-pr", + "request": "Implement Linear ticket LIN-30.", + "tracker_state": "Linear owns LIN-30, its parent, dependencies, and status; GitHub hosts code and PRs; unrelated GitHub issue 30 exists.", + "authority": "ready_pr_only" + }, + { + "id": "epic-with-children", + "request": "Implement GitHub epic G-31.", + "tracker_state": "G-31 has native epic type and three native subissues; the request means whole-epic delivery.", + "capabilities": ["implement-epic"] + }, + { + "id": "undecomposed-epic", + "request": "Implement GitHub epic G-32.", + "tracker_state": "G-32 has native epic type but no children yet; the request means whole-epic delivery.", + "capabilities": ["implement-epic"] + }, + { + "id": "explicit-child-not-redirected", + "request": "Implement G-34 only.", + "tracker_state": "G-34 is a native child of G-33, independently shippable, and explicitly selected without siblings.", + "authority": "ready_pr_only" + }, + { + "id": "missing-implement-epic", + "request": "Implement GitHub epic G-35.", + "tracker_state": "G-35 has children and requires whole-epic delivery.", + "capabilities": [] + }, + { + "id": "repeated-epic-handoff", + "request": "Implement GitHub epic G-35.", + "tracker_state": "G-35 has children and requires whole-epic delivery.", + "incoming_handoff": "implement-ticket:requires-epic:github:G-35", + "capabilities": [] + } +] diff --git a/skills/implement-ticket/evals/results.json b/skills/implement-ticket/evals/results.json new file mode 100644 index 0000000..e460cc7 --- /dev/null +++ b/skills/implement-ticket/evals/results.json @@ -0,0 +1,82 @@ +[ + { + "case_id": "standalone-ready-pr", + "terminal_state": "ready_pr", + "required_actions": ["implement one ticket", "validate", "run repository-owned review", "publish one PR", "report merge authority as pending"] + }, + { + "case_id": "authorized-merge-cleanup", + "terminal_state": "merged", + "required_actions": ["verify all current-candidate gates", "merge", "verify mainline and ticket transition", "clean only verified branch state"] + }, + { + "case_id": "named-epic-child-only", + "terminal_state": "ready_pr", + "required_actions": ["use parent context as evidence", "implement only G-23", "leave siblings and parent untouched"] + }, + { + "case_id": "open-native-blocker", + "terminal_state": "blocked", + "required_actions": ["name G-19 and its required outcome", "perform no implementation mutation"] + }, + { + "case_id": "closed-prerequisite-outcome-missing", + "terminal_state": "blocked", + "required_actions": ["verify the API contract is absent", "do not treat administrative closure as delivered outcome"] + }, + { + "case_id": "sibling-outcome-missing", + "terminal_state": "blocked", + "required_actions": ["name the missing G-37 shared-contract outcome", "perform no implementation mutation", "do not absorb or mutate sibling work"] + }, + { + "case_id": "canonical-pr-owned-elsewhere", + "terminal_state": "blocked", + "required_actions": ["report the canonical PR", "require explicit ownership transfer", "do not create a competing branch or claim ready_pr"] + }, + { + "case_id": "correctness-fix-and-rereview", + "terminal_state": "merged", + "required_actions": ["fix in the implementation context", "revalidate and commit a new head", "accept only the clean new-head review", "merge after current gates pass"] + }, + { + "case_id": "clean-local-review-remote-pending", + "terminal_state": "ready_pr", + "required_actions": ["report clean local review", "report human and connector gates pending", "do not merge"] + }, + { + "case_id": "unrelated-base-drift-retained", + "terminal_state": "merged", + "required_actions": ["record the evidence-retention basis", "reread final gates", "merge without unnecessary head change"] + }, + { + "case_id": "linear-ticket-github-pr", + "terminal_state": "ready_pr", + "required_actions": ["use Linear for ticket state", "use GitHub for PR state", "ignore unrelated GitHub issue 30", "do not merge"] + }, + { + "case_id": "epic-with-children", + "terminal_state": "requires_epic", + "required_actions": ["perform no mutation", "target implement-epic", "emit implement-ticket:requires-epic:github:G-31"] + }, + { + "case_id": "undecomposed-epic", + "terminal_state": "requires_epic", + "required_actions": ["perform no mutation", "do not flatten the epic", "emit implement-ticket:requires-epic:github:G-32"] + }, + { + "case_id": "explicit-child-not-redirected", + "terminal_state": "ready_pr", + "required_actions": ["remain in implement-ticket", "emit no epic routing marker", "implement only G-34"] + }, + { + "case_id": "missing-implement-epic", + "terminal_state": "requires_epic", + "required_actions": ["perform no mutation", "report implement-epic unavailable", "emit implement-ticket:requires-epic:github:G-35"] + }, + { + "case_id": "repeated-epic-handoff", + "terminal_state": "blocked", + "required_actions": ["report routing cycle detected", "perform no mutation", "do not emit another requires_epic"] + } +] diff --git a/skills/implement-ticket/references/cleanup-and-result.md b/skills/implement-ticket/references/cleanup-and-result.md new file mode 100644 index 0000000..ff6e211 --- /dev/null +++ b/skills/implement-ticket/references/cleanup-and-result.md @@ -0,0 +1,73 @@ +# Cleanup and terminal result + +Verify remote, mainline, tracker, and local state before deleting anything or +returning a terminal handoff. + +## Safe per-PR cleanup + +01. Confirm the PR is merged remotely. +02. Fetch and prune the remote. +03. Confirm the branch result is represented on the verified base. Use ancestry + first and patch equivalence after squash or rebase when needed. +04. Map the exact ticket worktree, local branch, upstream, PR branch, recorded + PR head, and base branch. Never rely on the current directory or a + branch-name guess. +05. Inspect tracked, staged, unstaged, untracked, and ignored state in that + exact worktree. Classify ignored and untracked paths as reproducible output + or non-reproducible/user-created data. Preserve credentials, `.env` files, + local databases, and all non-reproducible artifacts. +06. Confirm the local branch has no commits absent from its pushed PR branch. + When the remote branch is gone, compare it with the recorded PR head. +07. If the pushed branch exists, confirm it did not advance beyond the recorded + PR head and that the recorded result is represented on the base. +08. Remove only a clean disposable worktree. Never force removal. +09. Delete only the verified merged local feature branch, then its remote + feature branch when policy and authority permit. +10. Prune worktree metadata and verify the intended path and branches are gone. + +Stop cleanup and report exact dirty paths, ignored paths, unique commits, or +branch drift when any precondition fails. Preserve unrelated worktrees, +branches, ignored files, untracked files, and user edits. + +## Mainline and ticket verification + +After merge, verify: + +- the remote base advanced or otherwise contains the merged result; +- the implemented behavior and tests exist on the base; +- the owning tracker transitioned the ticket as expected; +- for an epic child, affected native dependency relationships were reread after + the transition and newly unblocked work was reported without selection or + mutation; +- no required check or review state invalidated the claimed result; and +- every performed cleanup action passed its preconditions. + +Do not close a parent epic, verify whole-epic acceptance, or implement newly +unblocked work. Report newly ready work only as context. + +## Result fields + +Return a concise documented handoff. Do not require a machine-readable schema +unless the caller has one. Include every applicable field: + +- `terminal_state`: `ready_pr`, `merged`, `blocked`, or `requires_epic`; +- ticket identity, tracker, repository, PR host, and base identity; +- branch, worktree, candidate head, and PR identity when created; +- completion policy and the authority actually used; +- focused and full validation commands, outcomes, and limitations; +- `review-code-change` verdict and reviewed candidate identity; +- applicable CI, human, connector, comment, formal-review, and thread state; +- merge, mainline, ticket transition, and cleanup state; +- deferred findings and intentionally unperformed work; and +- one concrete next action or blocking reason. + +For `requires_epic`, require all of: + +- no branch, worktree, ticket, or PR mutation occurred; +- `target_skill` is `implement-epic`; +- the resolved tracker, repository, ticket, native type, and sub-issue evidence; +- stable marker `implement-ticket:requires-epic::`; and +- an explicit missing-skill limitation when `implement-epic` is unavailable. + +If the incoming handoff already contains the same marker, return `blocked` with +`routing cycle detected` rather than another `requires_epic` result. diff --git a/skills/implement-ticket/references/github.md b/skills/implement-ticket/references/github.md new file mode 100644 index 0000000..68bf5c7 --- /dev/null +++ b/skills/implement-ticket/references/github.md @@ -0,0 +1,131 @@ +# GitHub ticket and PR adapter + +Use GitHub independently as an issue tracker, a PR host, or both. Apply only the +sections owned by its role for this ticket. + +## Ownership boundary + +- When GitHub owns ticket state, use the issue preflight, scope, relationship, + duplicate-work, closing-reference, and transition rules below. +- Whenever GitHub hosts the PR, use PR preflight, review, check, merge, and + branch rules. +- When Linear owns the ticket and GitHub hosts the PR, route parent, blocker, + status, and close decisions through the Linear adapter. Do not inspect or + mutate a same-numbered GitHub issue as a substitute. + +## Issue preflight and scope + +When GitHub owns ticket state: + +- read the live title, body, state, issue type, labels, and scope-affecting + comments; +- read native `parent`, `subIssues`, `blockedBy`, and `blocking` relationships + through GraphQL or an equivalent structured tool; +- use native issue type and sub-issues to apply the epic scope guard; +- verify every required closed-blocker outcome in its authoritative source; and +- search open and merged PRs plus plausible feature branches for an existing or + superseding implementation. + +Do not infer ownership, epic status, or dependency state from title, number, +Markdown task lists, or prose when native relationships exist. A label may +support but cannot override contradictory structured state. + +For a ticket that belongs to an epic, read the parent outcome and only the +sibling contracts or prerequisite results needed for correctness and independent +shipping. Never select another child or mutate the graph from this skill. + +When duplicate branches or PRs exist, compare actual patches or resulting trees +and retain one canonical implementation path. For an open canonical path owned +by another worker, return `blocked` with its identity unless ownership is +explicitly transferred; never claim its candidate as this run's `ready_pr`. When +a merged PR is verified on the base and the ticket is already complete, return +`merged` with that evidence without creating new state. Return `blocked` rather +than creating a competing PR when canonical ownership is unresolved. + +## PR-host preflight and contract + +- Resolve the repository from the request or checkout and confirm + authentication. +- Confirm the remote base, current checkout, branch, and worktree topology. +- Inspect open and merged PRs that reference the owning tracker ticket. +- Use one tracker ticket per branch and PR. +- When GitHub owns ticket state, use the repository's closing syntax, normally + `Fixes #`. +- Determine whether that syntax will automatically close or transition the + ticket on merge and disclose the consequence in the resolved completion policy + before publishing or merging. Use a non-closing reference when automatic + closure would conflict with that policy. +- When another tracker owns state, use its required reference and avoid GitHub + closing syntax unless a real GitHub issue is also intentionally in scope. +- Describe the branch as a whole, preserve material non-goals, and report actual + validation. +- Confirm the PR base and head match the ticket worktree. + +Use file-based commit and PR messages when shell interpolation could alter +Markdown. + +## Review state + +Capture exact PR head and base SHAs. Read conversation comments, formal reviews, +inline comments, and thread resolution state; use GraphQL or another +thread-aware API when flat PR output is insufficient. + +For required human review, record reviewer, state, reviewed commit, submission +time, and relevant base. Require current-candidate approval under repository +policy. Apply the generic base-drift gate after a base advance. + +For required connector review, discover and record before polling: + +- connector identity; +- automatic or request-driven initiation; +- exact initiation action and per-push policy; +- run-start evidence; +- accepted clean signal; and +- polling window and interval. + +Fail closed if this contract cannot be discovered. Accept a clean connector +result only when it is tied to the captured candidate by a current-head formal +review, a comment naming the head, a configured reaction on a request/result +naming the head, or another repository-documented completion signal with +equivalent candidate identity. Require zero unresolved connector-authored +threads. + +Do not infer current approval from timing, comment order, a generic bot message, +CI success, or a verdict on an earlier head. After every head change, require a +fresh candidate-bound signal. For base-only drift, use the generic drift gate +and the connector's documented retention policy. + +For every actionable comment, review, and thread: + +1. Verify it against the current code and ticket. +2. Fix it or reject it with concrete evidence. +3. Run affected and required validation. +4. Push when code changed. +5. Reply on the originating surface when possible and record disposition. +6. Resolve a thread only after disposition is complete. + +Require zero undispositioned actionable items before merge. Use at most three +connector feedback passes by default; do not spend passes on rejected, +out-of-scope, polish, or hypothetical findings. + +## Checks, merge, and ticket transition + +- Read required GitHub Actions checks and logs directly. +- Continue monitoring pending checks; ordinary wait time is not a user blocker. +- Separate in-scope failures from infrastructure, dependency, or + external-service failures. +- Fix only demonstrated ticket-scoped failures, revalidate, push, and restart + every invalidated current-head gate. +- Immediately before merge, reread head, base, checks, reviews, comments, + reactions, and threads. +- Apply the generic base-drift gate if the base advanced. +- Use the repository's approved merge method. +- Verify PR state, merged result, base content, and GitHub issue transition + before cleanup. +- When the ticket is an epic child, reread its affected native `blocking` and + sibling `blockedBy` relationships after the transition and report newly + unblocked work without selecting or mutating it. + +If local worktree ownership prevents the CLI from switching to the base, merge +through the GitHub API when authorized and perform local cleanup separately. +Never close a parent issue from this skill. diff --git a/skills/implement-ticket/references/linear.md b/skills/implement-ticket/references/linear.md new file mode 100644 index 0000000..85325af --- /dev/null +++ b/skills/implement-ticket/references/linear.md @@ -0,0 +1,51 @@ +# Linear ticket adapter + +Use live Linear issue relationships and repository evidence when Linear owns the +ticket, parent, dependency, or status state. + +## Preflight and scope + +- Read the live issue body, state, parent or epic, project context, comments, + and explicit blocking relationships. +- Read linked design documents and prior implementation PRs when they affect + scope. +- Search for completed, canceled, duplicate, or superseding issues and PRs. +- Resolve repository, PR host, base, and local instructions independently. +- Apply the epic scope guard from native issue type/parent-child state plus + explicit user scope. + +Do not use list order, priority, labels, or an old prompt as dependency state +when explicit relations are available. If Linear cannot express a required +relationship, record that limitation; do not silently treat prose as equivalent +to a native blocker. + +## Readiness and independent shipping + +Verify every required outcome from a completed, canceled, duplicate, or +superseded blocker in its authoritative repository, artifact registry, tracker, +or environment. For same-repository code, verify the outcome on the base. For +cross-repository or operational prerequisites, verify the consumer uses the +required contract, version, configuration, approval, or environment state. + +Treat a canceled or not-planned blocker with an unmet outcome as unresolved. +Read the parent outcome and relevant sibling contracts without selecting or +implementing sibling work. Return `blocked` if this ticket cannot ship +independently. + +## PR linkage and transition + +- Treat the live Linear issue body as the scope contract. +- Preserve exact product rules such as timing, timezone, idempotency, skip, + unavailable, migration, compatibility, and rollout behavior. +- Use the repository's required Linear reference in branch, commit, and PR + metadata. +- Keep one ticket per PR. +- Update Linear status only when that workflow state was actually reached and + the completion policy authorizes a manual transition. +- After merge, verify the result on the base and the expected Linear transition. +- When the ticket is an epic child, reread affected native dependency + relationships after transition and report newly unblocked work without + selecting or mutating it. + +Do not close or verify the parent epic. Report newly unblocked downstream work +without selecting it. diff --git a/skills/implement-ticket/references/review-and-merge-gates.md b/skills/implement-ticket/references/review-and-merge-gates.md new file mode 100644 index 0000000..003889f --- /dev/null +++ b/skills/implement-ticket/references/review-and-merge-gates.md @@ -0,0 +1,118 @@ +# Review and merge gates + +Apply these gates to the single ticket candidate. Repository instructions may +add stricter requirements but must not silently weaken them. + +## Contents + +- [Bounded review loop](#bounded-review-loop) +- [Revalidation](#revalidation) +- [Base-drift gate](#base-drift-gate) +- [Merge gate](#merge-gate) +- [Feedback that must not expand the PR](#feedback-that-must-not-expand-the-pr) + +## Bounded review loop + +Require the repository-owned `review-code-change` skill before a merge-inclusive +run. Fail closed when it is missing or unreadable. Do not substitute another +skill, a generic self-review, or an unreviewed merge path. + +Require every intended ticket change to be committed and the implementation +worktree to be clean before review. If unrelated user artifacts prevent a clean +state, classify and preserve them and prove they are irrelevant to the +candidate. + +Before delegation, capture HEAD, comparison base, commit history, and tracked, +staged, unstaged, untracked, and ignored state. Invoke `review-code-change` in a +fresh or minimally inherited read-only context with: + +- the live ticket and acceptance criteria; +- every named architecture, design, contract, migration, and rollout document; +- repository instructions and representative nearby code and tests; +- the exact captured head and comparison-base SHAs plus the complete + `base...HEAD` diff; and +- exact focused and full validation evidence, including unavailable checks. + +Exclude the implementation transcript, intended solution, prior conclusions, +suspected findings, and fixture expected outputs. After review, verify that +HEAD, history, and every captured worktree-state category remain unchanged. +Treat any mutation as an integrity failure; inspect and preserve it rather than +resetting or deleting user work. + +Consume the suite's validated aggregate result without restating or overriding +lens order, severity, deduplication, or correctness-versus-simplicity rules. +Apply only blocking and strong-recommendation findings that are material, +tractable, and ticket-scoped. Preserve deferred findings without expanding the +PR. Reply with evidence when a finding no longer applies. + +After a material fix, run affected and required validation, commit and push the +new head, rebuild the evidence packet, and follow the returned re-review +instruction. Use at most three full fix/re-review cycles by default. A clean +aggregate ends the local loop. If material findings remain after the final +cycle, keep the PR open and return `blocked` with the unresolved evidence. + +## Revalidation + +After every accepted fix: + +- run affected focused tests; +- rerun the repository-required gate; +- commit every intended ticket change; +- confirm `base...HEAD` contains the complete ticket implementation and no + unexplained artifact; +- push the new head; +- capture the new head and comparison-base identities; and +- reread current-candidate check and review state. + +Never carry head-bound evidence across an edit, push, rebase, conflict +resolution, or update operation that changes the head. + +## Base-drift gate + +Bind local review to the captured head and comparison base. Immediately before +merge, reread both identities and inspect the effective merge candidate when the +base advanced. + +Retain head-bound evidence across base-only drift only when all are true: + +- the effective diff and resulting tree are unchanged; +- no conflict or relevant overlap exists; +- repository policy permits retaining the evidence; and +- the reason is recorded. + +Otherwise invalidate and rerun every affected local-validation, CI, +repository-owned-review, human-review, connector-review, and +feedback-disposition gate. Any rebase, merge, conflict resolution, or update +that changes the head restarts all head-bound gates. + +## Merge gate + +Require every applicable condition: + +- every intended ticket change is committed and represented by the candidate + diff, with unrelated artifacts classified, preserved, and proven irrelevant; +- focused and full local validation passed; +- required remote checks passed; +- the repository-owned review is clean for the current head, with later base + drift explicitly retained or re-reviewed; +- every required human and connector review is current under repository policy; +- no undispositioned actionable conversation comment, formal review, connector + finding, or inline thread remains; +- no conflict or superseding implementation exists; +- the candidate still satisfies one ticket and its non-goals; and +- required rollout or migration prerequisites are complete. + +If the repository has no CI or a category of remote review, record that fact and +use the remaining documented gates. Do not infer absence merely from an empty +first read. + +## Feedback that must not expand the PR + +Keep these out unless the live ticket requires them: + +- speculative pre-release backfills; +- support for nonexistent legacy data; +- broad refactors unrelated to correctness; +- defensive abstraction without demonstrated duplication; +- product polish or future hardening; and +- changes owned by a sibling or parent epic. diff --git a/skills/implement-ticket/scripts/tests/test_implement_ticket_contract.py b/skills/implement-ticket/scripts/tests/test_implement_ticket_contract.py new file mode 100644 index 0000000..3ad43cd --- /dev/null +++ b/skills/implement-ticket/scripts/tests/test_implement_ticket_contract.py @@ -0,0 +1,198 @@ +from __future__ import annotations + +import json +import re +import unittest +from pathlib import Path + +SKILL_ROOT = Path(__file__).resolve().parents[2] +REPOSITORY_ROOT = SKILL_ROOT.parents[1] + + +def read(path: Path) -> str: + return path.read_text() + + +def compact(value: str) -> str: + return re.sub(r"\s+", " ", value).strip() + + +class ImplementTicketContractTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.skill = read(SKILL_ROOT / "SKILL.md") + cls.github = read(SKILL_ROOT / "references" / "github.md") + cls.linear = read(SKILL_ROOT / "references" / "linear.md") + cls.gates = read(SKILL_ROOT / "references" / "review-and-merge-gates.md") + cls.result = read(SKILL_ROOT / "references" / "cleanup-and-result.md") + cls.skill_compact = compact(cls.skill) + cls.github_compact = compact(cls.github) + cls.linear_compact = compact(cls.linear) + cls.gates_compact = compact(cls.gates) + cls.result_compact = compact(cls.result) + cls.all_contract = compact( + cls.skill + cls.github + cls.linear + cls.gates + cls.result + ) + cls.cases = { + item["id"]: item + for item in json.loads(read(SKILL_ROOT / "evals" / "cases.json")) + } + cls.results = { + item["case_id"]: item + for item in json.loads(read(SKILL_ROOT / "evals" / "results.json")) + } + + def test_frontmatter_and_product_neutral_contract(self): + self.assertTrue(self.skill.startswith("---\nname: implement-ticket\n")) + self.assertNotIn("Codex", self.all_contract) + self.assertNotIn("code-review-pro", self.all_contract) + self.assertIn("compatible agentic runtime", self.skill) + + def test_scope_is_exactly_one_ticket(self): + self.assertIn("one ticket per branch, worktree, and PR", self.skill_compact) + self.assertIn("do not select or implement it", self.skill_compact) + self.assertIn("Never claim whole-epic acceptance", self.skill_compact) + self.assertIn("parent and sibling context as evidence", self.skill_compact) + self.assertIn( + "canonical owner of generic single-ticket readiness", + self.skill_compact, + ) + self.assertIn( + "Duplication in `implement-epic-sequence` is temporary", + self.skill_compact, + ) + + def test_epic_routing_is_pre_mutation_and_acyclic(self): + self.assertIn("Guard whole-epic scope before mutation", self.skill_compact) + self.assertIn("Treat a named child of an epic", self.skill_compact) + self.assertIn("Do not invoke or require `implement-epic`", self.skill_compact) + self.assertIn( + "implement-ticket:requires-epic::", + self.all_contract, + ) + self.assertIn("routing cycle detected", self.all_contract) + + def test_readiness_fails_closed_without_widening_scope(self): + self.assertIn("no unresolved native blocker", self.skill_compact) + self.assertIn( + "closed, canceled, or not-planned prerequisite", self.skill_compact + ) + self.assertIn("never absorb that sibling", self.skill_compact) + self.assertIn("require explicit ownership transfer", self.skill_compact) + self.assertIn( + "never claim its candidate as this run's `ready_pr`", self.github_compact + ) + + def test_authority_does_not_expand(self): + self.assertIn("ready PR only", self.skill_compact) + self.assertIn("merge after gates", self.skill_compact) + self.assertIn("merge plus manual transition", self.skill_compact) + self.assertIn( + "parent closure always require separate explicit authority", + self.all_contract, + ) + self.assertIn( + "When merge authority is unclear, stop at a ready PR", self.skill_compact + ) + self.assertIn( + "automatic ticket transition caused by the selected closing syntax", + self.skill_compact, + ) + self.assertIn( + "disclose the consequence in the resolved completion policy", + self.github_compact, + ) + + def test_review_dependency_and_integrity_are_preserved(self): + self.assertIn("only local adversarial-review dependency", self.all_contract) + self.assertIn("Do not substitute another skill", self.gates_compact) + self.assertIn( + "fresh or minimally inherited read-only context", self.gates_compact + ) + self.assertIn("Exclude the implementation transcript", self.gates_compact) + self.assertIn("at most three full fix/re-review cycles", self.gates_compact) + self.assertIn("Treat any mutation as an integrity failure", self.gates_compact) + + def test_current_candidate_and_remote_gates_are_preserved(self): + self.assertIn( + "effective diff and resulting tree are unchanged", self.gates_compact + ) + self.assertIn("no conflict or relevant overlap exists", self.gates_compact) + self.assertIn("human and connector review is current", self.gates_compact) + self.assertIn("zero undispositioned actionable items", self.github_compact) + self.assertIn("Do not infer current approval", self.github_compact) + + def test_tracker_and_pr_host_ownership_are_separate(self): + self.assertIn("same-numbered GitHub issue", self.github_compact) + self.assertIn("Linear owns the ticket", self.github_compact) + self.assertIn("Resolve repository, PR host", self.linear_compact) + self.assertIn("Do not close or verify the parent epic", self.linear_compact) + + def test_cleanup_and_result_contract_are_complete(self): + for state in ("ready_pr", "merged", "blocked", "requires_epic"): + self.assertIn(state, self.result_compact) + self.assertIn( + "tracked, staged, unstaged, untracked, and ignored", self.result_compact + ) + self.assertIn("Never force removal", self.result_compact) + self.assertIn("Do not close a parent epic", self.result_compact) + self.assertIn( + "affected native dependency relationships were reread after the transition", + self.result_compact, + ) + self.assertIn("report newly unblocked work", self.github_compact) + self.assertIn("report newly unblocked work", self.linear_compact) + + def test_forward_cases_cover_required_boundaries(self): + required = { + "standalone-ready-pr", + "authorized-merge-cleanup", + "named-epic-child-only", + "open-native-blocker", + "closed-prerequisite-outcome-missing", + "sibling-outcome-missing", + "canonical-pr-owned-elsewhere", + "correctness-fix-and-rereview", + "clean-local-review-remote-pending", + "unrelated-base-drift-retained", + "linear-ticket-github-pr", + "epic-with-children", + "undecomposed-epic", + "explicit-child-not-redirected", + "missing-implement-epic", + "repeated-epic-handoff", + } + self.assertEqual(required, set(self.cases)) + self.assertEqual(required, set(self.results)) + + def test_forward_results_enforce_routing_and_authority(self): + self.assertEqual( + "ready_pr", self.results["standalone-ready-pr"]["terminal_state"] + ) + self.assertEqual( + "blocked", + self.results["canonical-pr-owned-elsewhere"]["terminal_state"], + ) + self.assertEqual( + "blocked", self.results["sibling-outcome-missing"]["terminal_state"] + ) + self.assertEqual( + "requires_epic", + self.results["missing-implement-epic"]["terminal_state"], + ) + self.assertEqual( + "blocked", self.results["repeated-epic-handoff"]["terminal_state"] + ) + self.assertIn( + "do not merge", + self.results["clean-local-review-remote-pending"]["required_actions"], + ) + + def test_ui_metadata_matches_skill(self): + metadata = read(SKILL_ROOT / "agents" / "openai.yaml") + self.assertIn('display_name: "Implement Ticket"', metadata) + self.assertIn("$implement-ticket", metadata) + + +if __name__ == "__main__": + unittest.main()