[kyle-sexton]

Closes #7603

Summary

/implementation:implement and /implementation:implement-dispatch now pass the PLAN.md pointer at each handoff, so a downstream /verification:confirm (rung 1 of its intent ladder, "a PLAN.md path or topic slug the caller named") reads the Brief even in a fresh session.

Fix

• implement Step 5 item 6 and the skill-chaining row: the hand-off is /verification:confirm <PLAN.md path> when a plan exists, the path resolved under Progress tracking (artifact protocol, <memory_dir>/<slug>/PLAN.md), so no .work/ layout is hard-coded.
• implement-dispatch brief item 5 carries the PLAN.md's absolute path in the orchestrator's checkout (none under --items, or when holdout tests fence the memory slice), and the "All phases complete" row hands the path to implement Step 5 (none under --items).
• Two eval cases: brief-names-plan-pointer (a worker brief names the absolute PLAN.md path unprompted, graded on the last message) and im-confirm-handoff-names-plan (the Step 5 confirm invocation carries the PLAN.md path).
• Changelog fragment for implementation (minor).

Out of scope: passing the Brief's sections verbatim (Phase 19).

Verification

• bash plugins/skill-quality/scripts/check-skill.sh plugins/implementation/skills: exit 0 (3 passed; warnings are pre-existing size warnings).
• python3 -I plugins/evals/skills/validate/scripts/validate-cases.py plugins/implementation/evals: exit 0, no warning on the new cases; their regex graders pass and fail their sample answers offline.
• scripts/check-affected.sh --tests on the merged head: exit 0 (7 passed, 2 skipped).
• Ready-flip review (fresh-context, code and security): fixed in this PR the missing "when a plan exists" / --items conditions, the changelog line's scope, and the plan-pointer grader's target (last_message). No security findings.
• Not run: the paid eval. "Fails on main, passes after" is not yet measured; the existing brief-passes-absolute-plan-paths case already gets an absolute path into a brief when asked, so the new case may pass on main too until measured.

Related

• Depends on feat(verification): look up intent from the PLAN.md Brief first #7624 (R1, the confirm intent ladder whose rung 1 takes this pointer): until it lands, /verification:confirm on main parses no path argument, so merge feat(verification): look up intent from the PLAN.md Brief first #7624 first.
• Part of epic Epic: absorb Lauren Tan's and Matt Pocock's agent workflows (poteto-review) #7555.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-16T18:24:11.108878Z	1433e69	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 1m 4s —— View job

---

Security review: /review:security-review

• Gather context (PR diff, changed files)
• Review changed skill files for security-relevant surface
• Check instruction-surface deletions against protected classes
• Report findings or clear result

Scope reviewed (git diff origin/main...HEAD, head 1433e69): 11 files, all in plugins/implementation/ — two SKILL.md edits (implement, implement-dispatch), two new eval cases (prompt/grader/sample fixtures), and one changelog fragment. No application code, scripts, workflows, or config changed.

Findings: none.

Reasoning:

• Both SKILL.md edits are purely additive natural-language instructions that tell the orchestrator/implementer to pass an already-resolved PLAN.md path (resolved under the existing artifact protocol, <memory_dir>/<slug>/PLAN.md) along at a handoff point. Nothing is removed, narrowed, or weakened — see
  implement-dispatch/SKILL.md#L93,
  implement-dispatch/SKILL.md#L273,
  implement/SKILL.md#L261, and
  implement/SKILL.md#L275.
• Checked against the instruction-surface-deletions lens (docs/conventions/instruction-exception-register/README.md, present in this checkout): no STOP/FORBIDDEN/guardrail language, permission, or check is deleted, narrowed, or softened anywhere in this diff — every change adds detail to an existing hand-off instruction, so no protected class is implicated.
• The new eval fixtures (brief-names-plan-pointer/*, im-confirm-handoff-names-plan/*) use fictional paths (/srv/dev/billing/...) in prompt/grader/sample files; no real paths, tokens, or credentials are present.
• The plan path being threaded through is produced internally by the existing artifact-protocol resolution (unchanged in this PR), not attacker-controlled input, so there's no new injection or path-traversal surface introduced by handing it to a downstream Skill invocation.
• No GitHub Actions, permissions, or dependency-pin changes in scope for zizmor's lane.

No security issues found in this pull request.

claude-lane-summary: lane=claude-security-review sha=1433e69ca73dc1f5a0bc61347c05f0cc7e36bb7b run=38946694210

[claude]

Claude finished @kyle-sexton's task in 3m 18s —— View job

---

Code review — /review:code-review

• Gather context (PR body, diff, REVIEW.md, standards index)
• Understand the request (CI code-review lane on PR feat(implementation): pass the PLAN.md pointer at each handoff #7630)
• Review changed files for correctness and convention adherence
• Report findings

Scope: 2 skill-text edits (implement/SKILL.md, implement-dispatch/SKILL.md), 2 new eval cases, 1 changelog fragment. No application code, no security-sensitive surface, no GitHub Actions changes — nothing for this lane to defer to the security lane.

What I checked:

• Traced the new "hand off the PLAN.md path" language in both SKILL.md files against the actual Progress tracking / Phase-boundary discipline sections they point to (plugins/implementation/skills/implement/SKILL.md:42,210,261) — the <memory_dir>/<slug>/PLAN.md path they reference matches, and the wording cites rather than restates it (no reference-dont-duplicate.md violation).
• Verified the --items and holdout-tests exclusions in implement-dispatch/SKILL.md:93 against reference/holdout-tests.md:36-41: holdout mode denies the worker read/write on the whole memory-slice root (where PLAN.md lives), so omitting the pointer there is correct, not an oversight.
• Checked the two new eval cases' regex graders by hand against their pass/fail samples (brief-names-plan-pointer/graders/plan-pointer.md, im-confirm-handoff-names-plan/graders/confirm-gets-plan.md): both patterns correctly accept the intended samples and reject the negative ones (relative-path-only, path split across a newline from the invocation, empty reply).
• Compared the new brief-names-plan-pointer case's skill-fired.md grader against the pre-existing brief-passes-absolute-plan-paths case — same established pattern for this eval shape, not a new convention.
• Changelog fragment accurately summarizes the diff; bump: minor matches an additive, backward-compatible doc change.

Not independently re-run (this lane's sandbox denies python3/script execution outright): the author's check-skill.sh, validate-cases.py, and check-affected.sh results. Treated as author-claimed, consistent with the PR's own "Not run: the paid eval" disclosure.

No correctness or maintainability issues found — nothing to flag at this lane's high-signal bar.

claude-lane-summary: lane=claude-review sha=1433e69ca73dc1f5a0bc61347c05f0cc7e36bb7b run=38946694264

[github-actions]

claude-security-review has reviewed this pull request through 42df8f7; a later push is reviewed from there.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1433e69ca7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

claude-review has reviewed this pull request through 42df8f7; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 1m 54s —— View job

---

Code review — /review:code-review (incremental)

• Read incremental diff and gather context
• Review changed lines in the 3 files since 5d6df02
• Report findings

Scope: incremental, limited to the diff since 5d6df02 in:

• plugins/implementation/skills/implement/SKILL.md
• plugins/implementation/evals/im-confirm-handoff-names-plan/graders/confirm-gets-plan.md
• plugins/implementation/evals/im-confirm-handoff-names-plan/samples/confirm-gets-plan.json

This commit (42df8f7) is exactly the fix for the two P2 findings Codex raised on the prior commit.

What I checked:

• implement/SKILL.md#L275: the table row now invokes bare /verification:confirm unconditionally (when the plugin is installed), adds the PLAN.md path as its argument only when a plan exists, and restricts self-verify to when the plugin is absent. That restores the outcome-verification gate for no-plan runs that Codex flagged as silently dropped, and it now matches the fuller Step 6 prose at SKILL.md#L261 (unchanged in this slice, already consistent).
• confirm-gets-plan.md: the new pattern verification:confirm`?[ \t]+`?[^\s`]*late-fee/PLAN\.md requires the path to immediately follow confirm as whitespace-separated (optionally backtick-wrapped) content, rather than matching anywhere within 80 chars. Traced it by hand against every sample in confirm-gets-plan.json:
  • Pass: `/verification:confirm .work/late-fee/PLAN.md` and the absolute-path variant — both match (backtick/space-then-path).
  • Fail: bare `/verification:confirm` (no pointer), the path on a separate line, and the new same-line-but-not-an-argument case Run `/verification:confirm`; the plan remains at `.work/late-fee/PLAN.md`. — none match, since the closing backtick right after confirm isn't followed by whitespace.
  • This is exactly the negative sample Codex's comment asked for, and it correctly fails under the new pattern.
• No new issues introduced by this fix; it's a narrow, correct correction to both prior findings.

Findings: none.

claude-lane-summary: lane=claude-review sha=42df8f727fdcfd5187eea2f14d63ca208867cb44 run=38953183023

[claude]

Claude finished @kyle-sexton's task in 41s —— View job

---

Security review: /review:security-review

• Gather context (incremental diff since 5d6df02, scoped to 3 files)
• Review implement/SKILL.md wording fix (confirm-gate restoration)
• Review eval grader/sample tightening (confirm-gets-plan.md)
• Report findings or clear result

Scope reviewed (incremental since 5d6df021558607b0cfee8e0e60e080d290e2a3e0, per .claude-lane/incremental.diff): 3 files — plugins/implementation/skills/implement/SKILL.md, plugins/implementation/evals/im-confirm-handoff-names-plan/graders/confirm-gets-plan.md, plugins/implementation/evals/im-confirm-handoff-names-plan/samples/confirm-gets-plan.json. This is the author's response to the two Codex P2 review comments on the prior revision.

Findings: none.

Reasoning:

• implement/SKILL.md#L275: the row now reads "then /verification:confirm, with the PLAN.md path as its argument when a plan exists (self-verify against intent only when the plugin is absent)". This restores the outcome-verification gate Codex flagged as silently dropped for plan-less tasks — /verification:confirm is invoked whenever the verification plugin is installed, plan or not; self-verify is now only the fallback when the plugin itself is absent. Still purely additive natural-language instruction; no STOP/guardrail language removed, narrowed, or weakened, so the instruction-surface-deletions lens doesn't apply.
• confirm-gets-plan.md#L4: the regex changed from verification:confirm[^\n]{0,80}late-fee/PLAN\.md to verification:confirm`?[ \t]+`?[^\s`]*late-fee/PLAN\.md — now anchors the path immediately after confirm (optionally backtick-wrapped) rather than allowing any same-line mention, closing the false-positive gap Codex identified. Checked for ReDoS: both character classes ([^\s\]*, [ \t]+`) are linear, non-overlapping, no nested quantifiers — no catastrophic-backtracking risk.
• confirm-gets-plan.json: new negative sample (Run `/verification:confirm`; the plan remains at `.work/late-fee/PLAN.md`.) is exactly Codex's counter-example, now a why: "path on the confirm line but not its argument" fail case — a fictional path, no real paths/tokens/credentials introduced. I hand-checked it against the new pattern: the backtick-paren-semicolon-space sequence after confirm does not match `?[ \t]+`?, so it correctly fails.
• No application code, scripts, workflows, or config in this incremental diff; nothing new for injection, path traversal, or secrets exposure.

No security issues found in this pull request.

claude-lane-summary: lane=claude-security-review sha=42df8f727fdcfd5187eea2f14d63ca208867cb44 run=38953182915