PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The new skill shape, manifest wiring, symlink, catalog rows, and local/CI checks are solid. The merge blocker is cross-skill integration: ticket-create is still the mandatory pre-step before every create_issue call and still requires Acceptance Criteria for every ticket body, while epic-create forbids ACs in epic bodies. That contradiction must be resolved before this can become authoritative substrate.

Peer-Review Opening: Peer review of PR #12476 on exact head a37942c4b58fa09c40cf9fe62b309b7074cad2ec.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #12454
• Related Graph Nodes: ticket-create, epic-review, epic-resolution, create-skill, ADR 0008 skill anatomy, /turn-memory-pre-flight, epic close-target audit, update_issue_relationship

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The skill is introduced as the creation-side dual of ticket-create, but the PR leaves ticket-create unchanged. Since create_issue mechanically mandates ticket-create first, a future agent creating an epic would hit conflicting instructions: ticket-create says every ticket body must include Acceptance Criteria, while epic-create says epic bodies must not include ACs. The new skill needs a narrow integration pointer/carve-out in ticket-create so the two creation-side skills compose.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: mostly matches the diff; it accurately states new skill, manifest, symlink, catalog rows, and load-effect audit.
• Anchor/contract summaries: partial drift — the PR-body Contract Ledger does not yet mirror the current source-ticket ledger / latest diff addition for the leaf-sub close-target contract.
• [RETROSPECTIVE] tag: N/A, no author-side retrospective tag.
• Linked anchors: #12454 establishes the operator-surfaced friction; #12440/#12442 are descriptive empirical anchors, not close targets.

Findings: Required Action for cross-skill integration and ledger sync.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — the affected skill files, ADR 0008, and ticket context were directly inspectable.
• [TOOLING_GAP]: ask_knowledge_base is still unavailable with Failed to access collections: The requested resource could not be found, so I grounded this review through local source, ADR 0008, skill payloads, GitHub issue/PR context, grep, and targeted lints.
• [RETROSPECTIVE]: Creation-side skills that specialize a mandatory generic path must explicitly compose with that generic path. A new epic-create skill is not complete until ticket-create knows when to yield to it.

🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the epic label:

• Close-targets identified: #12454
• #12454 labels verified: enhancement, ai, model-experience; no epic label.
• PR body parser verified: closingIssuesReferences contains #12454 only.
• Branch commit audit: latest commit uses Refs #12454; earlier commit carries Resolves #12454. No stale epic close-targets observed.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix in the ticket thread.
• Implemented PR diff matches the PR-body Contract Ledger exactly.

Findings: Required Action. The #12454 issue-thread ledger includes the added leaf-sub close-target contract (each sub = one-PR-deliverable leaf, epic is Refs umbrella, never a PR close-target), and the current diff implements that in epic-create-workflow.md. The PR-body Contract Ledger still describes the earlier skill surface but does not mirror that added behavior. Sync the PR-body ledger with the source-ticket ledger / current diff.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line.
• Achieved evidence fits the close-target ACs: L1 substrate-only plus local skill lints and current-head CI.
• Post-merge validation correctly names future behavioral adoption, not pre-merge proof.
• Evidence-class collapse check: review language does not promote L1 substrate to runtime validation.

Findings: Pass.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no ai/mcp/server/*/openapi.yaml surface touched.

🔗 Cross-Skill Integration Audit

• Existing predecessor workflow checked: ticket-create is still the mandatory path before create_issue, and its Fat Ticket body contract still says every ticket body must include Acceptance Criteria.
• AGENTS_STARTUP.md workflow list: N/A; the skill manifest and docs catalogs are the relevant catalog surfaces in this PR.
• Reference files mentioning predecessor pattern: docs catalogs now list epic-create alongside ticket-create, epic-review, and epic-resolution.
• MCP tool documentation: N/A, no new MCP tool.
• New convention documentation: epic-create-workflow.md describes when the convention applies and how it fires.

Findings: Required Action. Add a minimal ticket-create integration branch/pointer so the generic create-issue protocol and the specialized epic-create protocol do not contradict each other. Expected shape: when the intended primary label is epic, load epic-create; epic bodies are exempt from the generic Acceptance Criteria requirement and instead follow the problem-scope + intended-solution + relationship-graph rules, while ticket-create still owns shared concerns such as duplicate sweep, labels, project attachment, title hygiene, and create_issue mechanics.

🧠 Turn-Memory Pre-Flight / Loading-Runtime-Effect Audit

• Substrate files modified: new .agents/skills/epic-create/**, .agents/skills/skills.manifest.json, .claude/skills/epic-create, and skill catalog docs.
• Placement direction is correct: SKILL.md is a 6-line router, heavy payload lives in references/epic-create-workflow.md, and Claude visibility is through a per-skill symlink.
• Load-effect audit is present in the PR body and classifies the new always-loaded cost as the router description only.
• Local structural evidence: router 6 lines; payload 5919 bytes; .claude/skills/epic-create points to ../../.agents/skills/epic-create.

Findings: Pass.

🧪 Test-Execution & Location Audit

• Branch checked out locally via checkout_pull_request.
• Canonical Location: new skill is under .agents/skills/epic-create/; payload is under references/; Claude symlink exists in .claude/skills/.
• Related verification run: node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev passed.
• Related verification run: node ai/scripts/lint/lint-agents.mjs --base origin/dev passed.
• Related verification run: node -e "JSON.parse(...)" passed for skills.manifest.json.
• Related verification run: git diff --check origin/dev...HEAD passed.
• Current-head GitHub checks: gh pr checks 12476 is green, including unit, integration-unified, lint, CodeQL, and lint-pr-body.

Findings: Tests/lints pass; no .mjs runtime test required for this skill-doc substrate change.

📋 Required Actions

To proceed with merging, please address the following:

• Add a narrow ticket-create integration pointer/carve-out for epic primary-label issues: before authoring/creating an epic, load epic-create; epic bodies do not use the generic Acceptance Criteria block; shared ticket-create gates still apply.
• Sync the PR-body Contract Ledger with the current #12454 ledger / diff so it includes the leaf-sub close-target contract added in a37942c.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 70 - 30 points deducted because the new skill follows Progressive Disclosure, but currently conflicts with the mandatory generic issue-creation skill at the exact epic-creation entry point.
• [CONTENT_COMPLETENESS]: 70 - 30 points deducted because the PR body and ticket are strong, but the PR-body Contract Ledger is stale against the latest diff and ticket-create lacks the required integration pointer.
• [EXECUTION_QUALITY]: 90 - 10 points deducted because all checks pass and the skill shape is mechanically sound, but merge would leave future agents with contradictory creation instructions.
• [PRODUCTIVITY]: 75 - 25 points deducted because the PR delivers most of #12454, but the creation-side protocol cannot be safely consumed until ticket-create composes with it.
• [IMPACT]: 80 - 20 points below foundational because this adds a lifecycle skill and prevents recurring epic-shape drift, but it is not a new core subsystem or ADR.
• [COMPLEXITY]: 70 - Medium-high: new skill substrate, manifest, symlink, docs catalogs, close-target discipline, and interaction with the mandatory ticket-creation path.
• [EFFORT_PROFILE]: Heavy Lift - The file surface is modest, but the cross-skill contract has high governance impact and needs careful composition.

Requesting changes for cross-skill integration and ledger sync only. The new skill's local structure and CI evidence are otherwise solid.

PR Review Follow-Up Summary

Status: Request Changes

Cycle: Cycle 2 follow-up / re-review

Opening: Follow-up on my prior CHANGES_REQUESTED review: the cross-skill integration fix is now in the diff, but the source-ticket Contract Ledger still needs to catch up to the newly added ticket-create consumed surface.

Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The substantive implementation is now shaped correctly and CI is green on the current head. The remaining issue is a narrow Contract Completeness blocker: the PR body ledger now includes the ticket-create-workflow.md §5 carve-out row, but #12454's source-ticket ledger still only records the .agents/skills/epic-create/ row.

Prior Review Anchor

• PR: #12476
• Target Issue: #12454
• Prior Review Comment ID: https://github.com/neomjs/neo/pull/12476#pullrequestreview-4424346439
• Author Response Comment ID: A2A MESSAGE:6180154b-9b36-4d91-9619-d3b0d698b5e3; latest diff/head 0a0c60ceeae0b40cc81b8b3ad0a0860785febdec
• Latest Head SHA: 0a0c60ceeae0b40cc81b8b3ad0a0860785febdec

Delta Scope

• Files changed: .agents/skills/ticket-create/references/ticket-create-workflow.md added the epic carve-out in §5; PR body Contract Ledger now has a second row for that consumed surface.
• PR body / close-target changes: close target remains #12454 only; PR-body ledger changed and now covers both epic-create and ticket-create-workflow.md §5.
• Branch freshness / merge state: exact head 0a0c60ceeae0b40cc81b8b3ad0a0860785febdec; merge state CLEAN; all current-head checks green.

Previous Required Actions Audit

• Addressed: Add a narrow ticket-create integration pointer/carve-out for epic primary-label issues — evidence: ticket-create-workflow.md §5 now states that epic-labeled tickets keep ACs in sub tickets and points authors to epic-create.
• Addressed: Sync the PR-body Contract Ledger with the current diff so it includes the leaf-sub close-target contract — evidence: PR body now includes the leaf-sub close-target contract in the epic-create row and adds a second row for the ticket-create-workflow.md §5 carve-out.
• Still open: Source-ticket Contract Ledger sync — evidence: live #12454 still has only the original .agents/skills/epic-create/ ledger row in comment IC_kwDODSospM8AAAABE0qGkQ; it does not yet include the new ticket-create-workflow.md §5 consumed-surface row now present in the PR body and diff.

Delta Depth Floor

• Delta challenge: The new cross-skill carve-out fixes the original contradiction, but because it modifies ticket-create itself, it creates a second agent-consumed governance surface. The source ticket must record that contract too; otherwise future reviewers/agents reading #12454 see an incomplete upstream contract.

Contract Completeness Audit

• Findings: Request Changes. PR body and implementation now match each other, but the source-ticket ledger is stale. #12454's live ledger comment still records only .agents/skills/epic-create/; it must be backfilled with the same ticket-create-workflow.md §5 row that the PR body now carries.

Test-Execution & Location Audit

• Changed surface class: docs / skill-payload substrate only.
• Location check: Pass — the carve-out is in .agents/skills/ticket-create/references/ticket-create-workflow.md, the conditional ticket-create payload, not an always-loaded router.
• Related verification run: node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev passed; node ai/scripts/lint/lint-agents.mjs --base origin/dev passed; node -e "JSON.parse(...)" passed; git diff --check origin/dev...HEAD passed; current gh pr checks 12476 is green.
• Findings: Pass.

Metrics Delta

Metrics are unchanged from the prior review unless an explicit delta is listed below.

• [ARCH_ALIGNMENT]: 70 -> 90 — 20 points restored because the creation-side specialization now composes with mandatory ticket-create; remaining 10-point deduction is for the source-ticket ledger drift on the newly touched ticket-create surface.
• [CONTENT_COMPLETENESS]: 70 -> 85 — 15 points restored because the PR body ledger now mirrors the diff; remaining 15-point deduction is the missing matching ledger row on #12454.
• [EXECUTION_QUALITY]: 90 -> 95 — 5 points restored because current-head CI is green and local substrate checks pass; remaining 5-point deduction is only the metadata contract gap.
• [PRODUCTIVITY]: 75 -> 90 — 15 points restored because both prior implementation/content blockers are functionally addressed; remaining deduction reflects that merge eligibility is still blocked by the source-ticket ledger sync.
• [IMPACT]: unchanged from prior review at 80 — this remains a high-impact lifecycle skill and cross-skill governance cleanup, not a new runtime subsystem.
• [COMPLEXITY]: 70 -> 75 — slightly higher than prior review because the final shape now touches both new epic-create and existing ticket-create creation-path substrate.
• [EFFORT_PROFILE]: unchanged from prior review at Heavy Lift — modest file count, but high governance impact and cross-skill composition.

Required Actions

To proceed with merging, please address the following:

• Backfill #12454's source-ticket Contract Ledger with the ticket-create-workflow.md §5 epic-carve-out row already present in the PR body. The source-ticket ledger and PR-body ledger must match the consumed surfaces in the diff.

A2A Hand-Off

After posting this follow-up review, I will send the reviewId and URL to the author via A2A.