Findings from /issue:tackle 460 review-fix session. Five worth filing as separate fixes; rest as observations.
P1 — Build script rewrites shell variable refs as paths
File: scripts/build-claude-plugin.ts — rewriteMarkdownTarget
Problem: Walks every ](X) in markdown. Treats any non-URL target as a path to rewrite — including $URL2, $BODY, etc. Result: canonical ($URL2) becomes plugin (./$URL2), breaking generated link inside shell snippets.
Patch landed in PR #460 (fddd01d): skip targets containing $.
Recommendation: Tighten further. Markdown link targets inside fenced ```bash blocks should never be rewritten. Current logic is regex-on-whole-file and ignores fence context. Two options:
- Track fence state while walking lines; skip rewrite inside fences.
- Add explicit allowlist of file extensions for rewrite targets (
.md, .json, etc.) — refuse if extension absent.
P1 — agent-artifacts validator rejects runtime-created paths
File: tests/scripts/agent-artifacts.test.ts + validator under scripts/
Problem: validateAgentArtifacts() reports agent artifact references missing path: docs/onboarding/05-accept-and-merge-the-pr.md for .claude/skills/specorator-onboard/SKILL.md line 23. The skill's table inventories runtime-created files — they are not artifacts that must exist at validation time.
State on PR #460: failing on HEAD 349b91d, before review-fix commit. Pre-existing.
Recommendation: Add exemption. Options:
- Skill frontmatter flag
runtime_outputs: true for inventory-only references.
- Skip rows inside table sections labeled "Primary deliverable" / "Output paths".
- Require explicit angle-bracket placeholder syntax (
<docs/onboarding/05-...>) to opt out.
P1 — Tackle conductor worktree idempotency check is too narrow
File: .claude/skills/tackle-issue/SKILL.md — pre-flight Step 0
Problem: Skill globs .worktrees/issue-<N>-* to detect existing in-flight work. PR #460 was opened from an issue-457 worktree (.worktrees/issue-457-onboard); tackling PR #460 missed it because slug differed by issue number. Initial git worktree add failed with branch already used by worktree.
Recommendation: Match by branch not by path slug. For PR target with known headRefName:
existing=\$(git worktree list --porcelain | awk -v ref=\"refs/heads/\$headRefName\" '/^worktree/{p=\$2} /^branch /{if(\$2==ref) print p}')
Surface existing path and reuse instead of creating new one.
P2 — Worktree dependency bootstrap not automated
File: .claude/skills/tackle-issue/SKILL.md — Step 7
Problem: Skill says "Bootstrap deps (e.g. npm ci) — verify assumes deps installed." Conductor leaves this manual. PR #460 worktree had no node_modules; npm run verify failed on typecheck:scripts with TS2591 errors before any actual check ran.
Recommendation: Detect repo type (presence of package.json + lockfile) and auto-run npm ci after git worktree add. Or fail fast with a one-line hint when verify catches the missing-deps signature.
P2 — Codex thread resolution lifecycle
Observation: Three threads on PR #460 had author replies "Fixed." but isResolved=false. Codex doesn't auto-resolve on author replies; reviewer must mark resolved or push triggers re-review.
Recommendation: Document expectation in .claude/memory/feedback_pr_review_loop.md (or equivalent): author "Fixed" reply alone is not a closure signal. Resolution comes from either (a) Codex re-review on a new commit confirming the fix, or (b) reviewer marking the thread resolved manually. Conductors that consume isResolved should treat author claims as advisory, not authoritative.
P3 — Verify gate flake
Observation: cli.test.ts and project-init.test.ts hit 5s/15s test timeouts on Windows local runs (also flagged in PR #460 description). Not investigated this session.
Recommendation: Either raise per-test timeout where root cause is known-slow shellouts, or quarantine to a separate test run that doesn't gate verify.
Source: session notes from /issue:tackle 460, 2026-05-10. PR #460 commit fddd01d.
Findings from
/issue:tackle 460review-fix session. Five worth filing as separate fixes; rest as observations.P1 — Build script rewrites shell variable refs as paths
File:
scripts/build-claude-plugin.ts—rewriteMarkdownTargetProblem: Walks every
](X)in markdown. Treats any non-URL target as a path to rewrite — including$URL2,$BODY, etc. Result: canonical($URL2)becomes plugin(./$URL2), breaking generated link inside shell snippets.Patch landed in PR #460 (
fddd01d): skip targets containing$.Recommendation: Tighten further. Markdown link targets inside fenced ```bash blocks should never be rewritten. Current logic is regex-on-whole-file and ignores fence context. Two options:
.md,.json, etc.) — refuse if extension absent.P1 —
agent-artifactsvalidator rejects runtime-created pathsFile:
tests/scripts/agent-artifacts.test.ts+ validator underscripts/Problem:
validateAgentArtifacts()reportsagent artifact references missing path: docs/onboarding/05-accept-and-merge-the-pr.mdfor.claude/skills/specorator-onboard/SKILL.mdline 23. The skill's table inventories runtime-created files — they are not artifacts that must exist at validation time.State on PR #460: failing on HEAD
349b91d, before review-fix commit. Pre-existing.Recommendation: Add exemption. Options:
runtime_outputs: truefor inventory-only references.<docs/onboarding/05-...>) to opt out.P1 — Tackle conductor worktree idempotency check is too narrow
File:
.claude/skills/tackle-issue/SKILL.md— pre-flight Step 0Problem: Skill globs
.worktrees/issue-<N>-*to detect existing in-flight work. PR #460 was opened from an issue-457 worktree (.worktrees/issue-457-onboard); tackling PR #460 missed it because slug differed by issue number. Initialgit worktree addfailed withbranch already used by worktree.Recommendation: Match by branch not by path slug. For PR target with known
headRefName:Surface existing path and reuse instead of creating new one.
P2 — Worktree dependency bootstrap not automated
File:
.claude/skills/tackle-issue/SKILL.md— Step 7Problem: Skill says "Bootstrap deps (e.g.
npm ci) — verify assumes deps installed." Conductor leaves this manual. PR #460 worktree had nonode_modules;npm run verifyfailed ontypecheck:scriptswith TS2591 errors before any actual check ran.Recommendation: Detect repo type (presence of
package.json+ lockfile) and auto-runnpm ciaftergit worktree add. Or fail fast with a one-line hint when verify catches the missing-deps signature.P2 — Codex thread resolution lifecycle
Observation: Three threads on PR #460 had author replies "Fixed." but
isResolved=false. Codex doesn't auto-resolve on author replies; reviewer must mark resolved or push triggers re-review.Recommendation: Document expectation in
.claude/memory/feedback_pr_review_loop.md(or equivalent): author "Fixed" reply alone is not a closure signal. Resolution comes from either (a) Codex re-review on a new commit confirming the fix, or (b) reviewer marking the thread resolved manually. Conductors that consumeisResolvedshould treat author claims as advisory, not authoritative.P3 — Verify gate flake
Observation:
cli.test.tsandproject-init.test.tshit 5s/15s test timeouts on Windows local runs (also flagged in PR #460 description). Not investigated this session.Recommendation: Either raise per-test timeout where root cause is known-slow shellouts, or quarantine to a separate test run that doesn't gate
verify.Source: session notes from
/issue:tackle 460,2026-05-10. PR #460 commitfddd01d.