feat: add standalone verify-business-logic skill

[ahoym]

Summary

• Adds /verify-business-logic as a standalone-first skill that checks discipline rules and intent alignment on PRs after review/address cycles
• Standalone mode self-captures intent from PR metadata with operator confirmation; director mode reads locked intent files
• Integrates with director: Phase 1 intent capture negotiation, Phase 5 verifier invocation with CLARIFY protocol
• Includes convergence-verifier persona, 4 sibling reference files (discipline-rules, prompt-template, director-mode, SKILL.md)

Test plan

• Run /verify-business-logic standalone on a PR with review comments — confirm discipline checks and intent alignment output
• Verify director SKILL.md Phase 1 and Phase 5 additions are coherent with existing phases
• Edge case: PR with no review comments (discipline section minimal)
• Edge case: vague PR description triggers operator intent confirmation

🤖 Generated with Claude Code

[ahoym]

Team Review: feat: add standalone verify-business-logic skill

Solid standalone-first design with correct conditional reference loading, clean mode separation, and a well-scoped CLARIFY protocol. The main themes to address before merge: eager-loading inconsistency in the reference files section (affects Step 5 runtime behavior), missing error handling at the intent file I/O boundary, and a CLARIFY escalation gap where a degraded 'intent too vague' outcome silently reaches the operator without a clear blocking signal.

Reviewed by: claude-config-reviewer, correctness-reviewer

Findings

• [claude-config-reviewer, correctness-reviewer] Reference Files section declares discipline-rules.md and prompt-template.md as "always loaded" but uses backtick notation — not @ eager loading. Step 5 inherits a false assumption from this, and prompt-template.md's {{DISCIPLINE_RULES}} placeholder is inconsistent with reference-file loading semantics.
• [correctness-reviewer] Partial-flags argument error (--intent-file without --session-dir) has no defined operational behavior — halt, message, or silent fallthrough are all plausible and each produces a different outcome for the director.
• [correctness-reviewer] Step 4 director branch reads the intent file with no error handling for missing file, absent Source field, or unrecognized Source enum value — all silently fall through to confidence-framing logic.
• [correctness-reviewer] CLARIFY is triggered at Step 7 after all discipline checks ran; re-invocation re-fetches PR state from a potentially different snapshot, producing inconsistent discipline findings between runs.
• [correctness-reviewer] After one CLARIFY round, if intent is still vague, the verifier posts a degraded report with no explicit operator escalation — the director only logs to decisions.md and the session can converge on an unverified PR.
• [claude-config-reviewer, correctness-reviewer] Step 2 activates the persona mid-skill, after Step 1 already ran without the persona lens, and via slash-command syntax that may not resolve in director mode.
• [claude-config-reviewer] Phase 5 invokes the verifier via slash-command syntax (/verify-business-logic) — director is headless, Phase 2 correctly uses the Skill tool. Phase 5 should match.
• [correctness-reviewer] Director Phase 1 Step 8c waits for operator confirmation with no timeout or fallback — operator non-response deadlocks the session.
• [correctness-reviewer] Phase 5 constructs the intent file path using <id> but provides no mapping rule from <pr-number> (integer) to pr-<N> (prefixed string); wrong path causes an unhandled file-not-found in the verifier.
• [correctness-reviewer] Rule 3's operator-reopen exception is undefined — no criteria for what constitutes a "reopen" vs. a question or acknowledgment, making the rule subjective where it claims to be deterministic.
• [correctness-reviewer] Rule 4's timing caveat (check if address runner converged) is unevaluable in standalone mode — the verifier has no access to runner state.
• [claude-config-reviewer] Director SKILL.md Phase 1 references the intent file schema via prose cross-reference to verify-business-logic's director-mode.md — schema drift between the two files is a silent failure mode.

Positive Signals

• Standalone-first design is correctly implemented: standalone gathers intent interactively, director mode accepts pre-computed intent files and skips gathering — exactly the right composability pattern.
• Conditional reference file pattern applied correctly: director-mode.md loaded only when director-mode flags are present; base context stays lean for standalone invocations.
• Intent file locking protocol (no silent mutations after confirmation, append-only revision sections, decisions.md logging) is correct state-machine design.
• The discipline/intent alignment separation — assertions vs. advisory framing — is the right architectural split and prevents false precision in the intent section.
• CLARIFY one-round cap prevents infinite loops; its presence as a design constraint is correct even if the escalation path after the cap needs tightening.
• Superseded plan doc preserved with a clear header and pointers to the implementation — good institutional knowledge trail.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer, correctness-reviewer
• Role: Team-Reviewer

[ahoym]

11 suggestions implemented — see inline comments.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Team Re-review: feat: add standalone verify-business-logic skill

Both fix commits (7b4044f, 0b50066) comprehensively address all HIGH and MEDIUM findings from cycle 1. The updated SKILL.md is substantially better: intent validation runs before PR data fetch, CLARIFY fires early, persona activation is Step 0 with Skill tool invocation, and the structured VERIFIED:<status> completion signal makes the director/verifier contract machine-readable.

Reviewers (this cycle): claude-config-reviewer, correctness-reviewer
Carried forward: n/a (all domains affected by fixes)

Previous Findings

• ✅ 11 resolved (code change verified)
• 👍 0 acknowledged
• 🔄 0 partially addressed
• ❌ 0 not addressed

New Findings

None from the new commits. A few LOW items noted in the cycle 1 summary remain unaddressed (prompt-template.md {{DISCIPLINE_RULES}} placeholder still inconsistent with eager-loaded reference, ## Context block still present but unused, ## Usage section duplicates frontmatter) — these are cosmetic and don't block correctness.

Positive Signals

• Intent validation moved to Step 2 (before PR fetch) eliminates the TOCTOU risk and makes errors bounded — a clean structural fix, not just a safeguard bolted on.
• The VERIFIED:intent-unclear blocking signal closes the session-convergence gap cleanly: the director can now gate on verifier status without free-text parsing.
• Rule 3 and Rule 4 in discipline-rules.md are now deterministic assertions: reopen criteria are enumerated, and timing caveats are mode-specific. The discipline section can now genuinely claim pass/fail without subjective judgment.
• The 5-minute intent-capture timeout with inferred-from-pr-description fallback is the right pattern: autonomous at low confidence, logged, never deadlocking.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer, correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Team Review: feat: add standalone verify-business-logic skill

Solid standalone-first design with clean mode separation, explicit error halts, and a well-scoped CLARIFY protocol. The main themes to address before merge: (1) a concrete infinite-loop risk in director Phase 5 where the one-CLARIFY rule has no enforcement mechanism across fresh Skill re-invocations, (2) missing error handlers in Phase 5 — no handler for verifier error output and no check for missing intent files on mid-session PRs, and (3) a cluster of always-on token costs that should be trimmed or converted to conditional loading.

Reviewed by: claude-config-reviewer, correctness-reviewer

Findings

16 finding(s) — see inline comments.

Positive Signals

• Standalone-first design is correctly implemented: standalone gathers intent interactively, director mode accepts pre-computed intent files and skips gathering — exactly the right composability pattern. [claude-config-reviewer]
• Explicit halt-with-error in Step 2 director mode (file not found, missing Source, unrecognized Source value) produces no parseable success output on error — prevents silent state corruption downstream. [correctness-reviewer]
• CLARIFY check placed at Step 2 before PR data fetch correctly minimizes wasted API calls on ambiguous intents. [correctness-reviewer]
• Intent file locking semantics are clean: written once, never silently mutated, revision history append-only with dated sections and incremented counter. [correctness-reviewer]
• Discipline rules correctly framed as assertions (pass/fail with cited evidence) distinct from advisory intent alignment — well-maintained across all four files. [claude-config-reviewer]
• Rule 4 (Loop Closure) timing caveat correctly differentiates standalone vs director mode, avoiding false positives while treating orphaned threads as real findings when Phase 5 ordering guarantees post-convergence execution. [correctness-reviewer]
• Structured VERIFIED:<status> completion signal lets the director parse status without free-text matching. [correctness-reviewer]
• Pre-filter for converged items on director relaunch cycles (Phase 2) avoids wasted session startup cost. [claude-config-reviewer]

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer, correctness-reviewer
• Role: Team-Reviewer

[ahoym]

[CRITICAL] [correctness-reviewer] Phase 5 Step 3b has no exit condition — infinite CLARIFY loop risk

Step 3b: if verifier outputs CLARIFY:, route through Decision Framework, re-invoke verifier. If the re-invoked verifier outputs another CLARIFY: — which it can, because the verifier is a fresh Skill instance with zero memory of prior runs — Step 3b loops again with no counter, no fallback, no terminal condition.

Worst-case: vague intent → CLARIFY → director answers silently (simple/silent route, no operator escalation) → fresh re-invoke → same vagueness → CLARIFY → repeat. Spins invisibly.

Root cause: the one-CLARIFY rule has no protocol enforcement across independent invocations (see companion finding in verify-business-logic/SKILL.md Important Notes).

Fix: Add exit condition to Step 3b: "If re-invoked verifier outputs a second CLARIFY:, treat as VERIFIED:intent-unclear, log verifier-clarification-loop to decisions.md, do not re-invoke." Pairs with --clarify-answered flag on the verifier side.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — this is a real infinite-loop risk. A fresh Skill instance has zero memory of prior CLARIFY rounds, so the one-round cap is unenforceable without a protocol-level signal.

I'll add: (1) --clarify-answered flag the director passes on re-invocation, (2) exit condition in Step 3b — if re-invoked verifier outputs a second CLARIFY:, treat as VERIFIED:intent-unclear, log verifier-clarification-loop to decisions.md, do not re-invoke.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 1cda38a

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[HIGH] [correctness-reviewer] No handler for verifier error output — only CLARIFY: and implicit VERIFIED:* are handled

Verifier SKILL.md Step 2 explicitly halts on three error conditions and produces "no output the director could parse as success." But director Phase 5 only handles two output shapes: CLARIFY: (Step 3b) and (implicitly) VERIFIED:* (proceeds to Step 4).

If the verifier halts with an error, the director receives error text matching neither. Most likely outcome: silently treats invocation as done and moves to Step 4, omitting verification.

Fix: Add Step 3c: "If verifier output is neither CLARIFY: nor VERIFIED:*, treat as verifier error. Log to decisions.md as verifier-error. Do not proceed to Step 4 for this PR without operator decision — either retry with corrected args or mark as verification-skipped with explicit acknowledgment."

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — the verifier's explicit error halts produce non-parseable output, but the director has no handler for that case. Silent proceed-to-Step-4 is exactly the wrong outcome.

I'll add Step 3c: if verifier output is neither CLARIFY: nor VERIFIED:*, treat as verifier error. Log to decisions.md as verifier-error, do not proceed to Step 4 without operator decision.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 1cda38a

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[HIGH] [correctness-reviewer] Phase 5 invokes verifier for all converged PRs, but mid-session additions may lack intent files

Phase 1 Step 8 captures intent for PRs in scope at session start. The lock-and-update cycle appends to existing intent files — it does not create new ones for PRs added mid-session. If such a PR converges, Phase 5 invokes --intent-file <session_dir>/intents/<id>.md pointing to a file that does not exist → verifier halts with error → director has no error handler (see companion finding).

Fix: Before invoking verifier for a PR in Phase 5, check whether <session_dir>/intents/<id>.md exists. If missing: (a) run Phase 1 Step 8 intent capture for this PR first, OR (b) invoke verifier in standalone mode and log the gap to decisions.md.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — Phase 1 Step 8 only captures intent for PRs in scope at session start. Mid-session additions that converge hit a missing intent file → verifier error → no handler (companion finding).

I'll add a pre-invocation check to Phase 5: before invoking verifier for a PR, check whether <session_dir>/intents/<id>.md exists. If missing, run Phase 1 Step 8 intent capture for that PR first. This is cleaner than falling back to standalone mode since the director has the session context to negotiate intent properly.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 1cda38a

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[HIGH] [claude-config-reviewer, correctness-reviewer] One-CLARIFY-per-run rule has no enforcement mechanism across fresh re-invocations

"Maximum one CLARIFY per verifier run" is stated here (Important Notes) and in director-mode.md. But when the director re-invokes the verifier as a fresh Skill tool call, the new instance has zero memory of prior runs. Nothing in the intent file, args, or session dir signals "this is attempt 2." The new instance reads the same vague intent file, hits the same vagueness at Step 2, and can legally emit another CLARIFY:.

This is the protocol-level root cause of the infinite-loop risk in director Phase 5 Step 3b.

Fix: Add a --clarify-answered flag the director passes on re-invocation. In Step 2, when this flag is present and intent is still vague, skip CLARIFY and emit VERIFIED:intent-unclear directly. Document in both SKILL.md Step 2 and director-mode.md.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer, correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — the one-CLARIFY rule is stated but not enforced. A fresh Skill instance can't know it's attempt 2 without explicit signaling.

I'll add: when --clarify-answered is present in $ARGUMENTS and Step 2 finds intent still vague, skip CLARIFY and emit VERIFIED:intent-unclear directly. This pairs with the director-side exit condition (companion finding on Phase 5 Step 3b).

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[HIGH] [correctness-reviewer] Standalone mode missing VERIFIED:intent-unclear exit status on too-vague path

Step 2 standalone says "produce report with 'intent too vague to evaluate' section" after one failed revision round. But Step 7 defines VERIFIED:intent-unclear as a blocking signal — the director must not converge on a PR with this status.

Step 7's description anchors intent-unclear to "after CLARIFY limit reached" — but CLARIFY is director-mode-only. Standalone's terminal state is underdefined: a report is produced but no machine-readable exit status.

Fix: Add VERIFIED:intent-unclear to standalone Step 2's too-vague path. Update Step 7 description to generalize: "emitted when intent is too vague to evaluate, regardless of mode."

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — standalone's too-vague path produces a report but no machine-readable exit status. The VERIFIED:intent-unclear signal should be mode-independent.

I'll add VERIFIED:intent-unclear to standalone Step 2's too-vague path and update Step 7's description to generalize: "emitted when intent is too vague to evaluate, regardless of mode."

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[MEDIUM] [claude-config-reviewer] @./ prefix has known resolution issues — and both eagerly-loaded files exceed the 50-line guideline

From skill-references-and-loading.md: "@./ relative paths may have resolution issues. Use @filename.md (skill-directory-relative without ./) instead."

Beyond path syntax, both files exceed the less-than-50-line threshold for @ eager loading: discipline-rules.md is 97 lines (consumed only at Step 4, skipped on CLARIFY early-exit) and prompt-template.md is 75 lines (consumed only at Step 6).

Fix: (1) Drop the ./: use @discipline-rules.md and @prompt-template.md. (2) Consider converting both to conditional references read at Steps 4 and 6 — saves ~172 lines of context on CLARIFY early-exit paths.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed on both points:

1. @./ → @: Dropping the ./ per skill-references-and-loading.md guidance.
2. Conditional loading: Both files exceed the 50-line threshold and are only consumed at Steps 4 and 6 respectively. On CLARIFY early-exit, eager loading wastes ~172 tokens of context. Converting both to conditional references with explicit Read instructions at their consumption points.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[LOW] [claude-config-reviewer] Conditional reference request-interaction-base.md missing explicit Read instruction at Step 6

From skill-references-and-loading.md: include an explicit Read instruction in the step that consumes a conditional reference, not just in the ## Reference Files (conditional) header.

Step 6 says "per request-interaction-base.md format" but does not tell the agent to read the file there.

Fix: Add to Step 6: "Read ~/.claude/skill-references/request-interaction-base.md for the footnote format."

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — Step 6 references request-interaction-base.md for the footnote format but doesn't include an explicit Read instruction. Adding Read ~/.claude/skill-references/request-interaction-base.md for the footnote format to Step 6.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[LOW] [claude-config-reviewer] ## Usage section duplicates argument-hint frontmatter — always-loaded redundancy

The argument-hint frontmatter already declares the same two forms. Both load on every invocation.

Fix: Delete ## Usage (lines 20-23). The frontmatter covers discovery; Step 1 covers mode parsing.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — the argument-hint frontmatter already declares both usage forms. The ## Usage section is redundant always-loaded content. Removing it.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[LOW] [correctness-reviewer] Phase 1 timeout Source: inferred-from-pr-description misrepresents actual provenance

Step 8e uses Source: inferred-from-pr-description on timeout. But by then the director has drafted intent from metadata AND presented the draft to the operator (Steps 8b-c). The operator saw it, just did not reply.

inferred-from-pr-description implies the operator never saw the intent. The verifier will apply lowest-confidence framing to an intent the operator actually reviewed.

Fix (minor): Add Source: draft-timeout with confidence tier between agent-prompt and inferred-from-pr-description. Update director-mode.md and prompt-template.md confidence framing sections to match.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: correctness-reviewer
• Role: Team-Reviewer

[ahoym]

Good catch — the operator did see the intent draft (Steps 8b-c), so inferred-from-pr-description understates the confidence tier. I'll add Source: draft-timeout with a confidence tier between agent-prompt and inferred-from-pr-description, and update director-mode.md and prompt-template.md confidence framing to include it.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 1cda38a (director/SKILL.md) and 02fe19b (director-mode.md, prompt-template.md)

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

[LOW] [claude-config-reviewer] 178-line SUPERSEDED plan doc — delete or truncate to pointer-only

A SUPERSEDED file committed with full historical content is dead weight. Git history already preserves this content.

Options: (1) Delete the file. (2) Keep only the 9-line SUPERSEDED header pointing to the implementation, remove the rest.

---

• Co-Authored with Claude Code (Claude Sonnet 4.6)
• Persona: claude-config-reviewer
• Role: Team-Reviewer

[ahoym]

Agreed — 178 lines of SUPERSEDED content is dead weight when git history preserves it. Going with option 2: keep only the 9-line SUPERSEDED header with pointers to the implementation, remove the rest.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser

[ahoym]

Fixed in 02fe19b — truncated to pointer-only header.

---

• Co-Authored with Claude Code (Claude Opus 4.6)
• Persona: claude-config-expert
• Role: Addresser