Conversation
…ementation) Implements the verify-branch harness pre-tool-use hook designed in B-0191 (PR #1571 merged). Single TypeScript file under tools/orchestrator-checks/ with unit tests. Behavior: - If ZETA_EXPECTED_BRANCH is set and doesn't match `git branch --show-current`: print error to stderr, exit 1 (blocks commit when wired as a pre-tool-use hook on `git commit` Bash invocations) - If ZETA_EXPECTED_BRANCH is unset and current branch matches the worktree-suffix pattern: print INFO warning to stderr, exit 0 (informational only -- doesn't block, just nudges) - Otherwise: silent exit 0 Composes with: - memory/feedback_orchestrator_pre_commit_verify_branch_rule_aaron_2026_05_04.md (the manual discipline this mechanizes) - memory/feedback_dst_justifies_ts_quality_over_bash_and_harness_hooks_suffice_no_git_hooks_aaron_2026_05_03.md (TS-over-bash + harness-hooks-suffice; runs under bun, no git hooks needed) - memory/feedback_parallel_subagent_concurrency_lessons_cluster_aaron_2026_05_04.md (the orchestrator-CWD-bleed-over hazard this catches) Test plan covers all four behavior branches (no expectation, match, mismatch, worktree-warning). All 4 tests pass; 9 expect() calls. This closes B-0191 acceptance criterion #1 (harness pre-tool-use hook script exists) and #4 (test exists). Remaining acceptance criteria #2 (.claude/settings.json wiring) and #3 (CLAUDE.md documentation pointer) are separate follow-up PRs since they touch different files / surfaces. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Implements a new harness-side “verify current git branch” check intended to block wrong-branch commits in orchestrated/worktree-heavy workflows, with accompanying Bun unit tests.
Changes:
- Adds
tools/orchestrator-checks/verify-branch.ts, a Bun/TypeScript hook script that comparesZETA_EXPECTED_BRANCHtogit branch --show-currentand exits non-zero on mismatch. - Adds
tools/orchestrator-checks/verify-branch.test.tswith initial Bun tests for the exportedverifyBranchfunction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/orchestrator-checks/verify-branch.ts | New Bun hook that checks branch consistency and emits error/info messages with appropriate exit codes. |
| tools/orchestrator-checks/verify-branch.test.ts | New Bun test file covering basic match/mismatch cases and the “suppressed warning when expected is set” behavior. |
Comment on lines
+39
to
+44
| const result = spawnSync("git", ["branch", "--show-current"], { | ||
| encoding: "utf8", | ||
| }); | ||
| if (result.status !== 0) { | ||
| throw new Error(`git branch --show-current failed: ${result.stderr}`); | ||
| } |
| } | ||
|
|
||
| if (import.meta.main) { | ||
| process.exit(main()); |
Comment on lines
+3
to
+47
| // Per Otto-272 DST: tests are deterministic; we mock the env+git interaction | ||
| // at the function boundary (the verifyBranch function takes env explicitly, | ||
| // so we don't have to mock spawnSync, but we do need git to return SOMETHING | ||
| // since the test runner runs in a real git repo). | ||
|
|
||
| import { describe, expect, test } from "bun:test"; | ||
| import { verifyBranch } from "./verify-branch"; | ||
|
|
||
| describe("verifyBranch", () => { | ||
| test("no expectation set -> match=true regardless of current branch", () => { | ||
| const r = verifyBranch({}); | ||
| expect(r.match).toBe(true); | ||
| expect(r.expected).toBe(""); | ||
| // current is whatever git says; just verify the shape is non-empty when | ||
| // running inside a git repo. | ||
| expect(typeof r.current).toBe("string"); | ||
| }); | ||
|
|
||
| test("expectation set + matches current -> match=true", () => { | ||
| // First grab the actual current branch | ||
| const baseline = verifyBranch({}); | ||
| const r = verifyBranch({ ZETA_EXPECTED_BRANCH: baseline.current }); | ||
| expect(r.match).toBe(true); | ||
| expect(r.expected).toBe(baseline.current); | ||
| expect(r.current).toBe(baseline.current); | ||
| }); | ||
|
|
||
| test("expectation set + does NOT match current -> match=false", () => { | ||
| const r = verifyBranch({ | ||
| ZETA_EXPECTED_BRANCH: "definitely-not-the-current-branch-xyz", | ||
| }); | ||
| expect(r.match).toBe(false); | ||
| expect(r.expected).toBe("definitely-not-the-current-branch-xyz"); | ||
| }); | ||
|
|
||
| test("worktreeWarning fires only when expectation is unset AND branch matches the worktree-suffix pattern", () => { | ||
| // No expectation, on a non-worktree branch like 'main' -> no warning | ||
| const baseline = verifyBranch({}); | ||
| if (baseline.current === "main" || baseline.current === "master") { | ||
| expect(baseline.worktreeWarning).toBe(false); | ||
| } | ||
| // When we DO set an expectation, worktreeWarning is suppressed by design | ||
| // (the user asserted what they want, no need for the heuristic). | ||
| const r = verifyBranch({ ZETA_EXPECTED_BRANCH: baseline.current }); | ||
| expect(r.worktreeWarning).toBe(false); |
Comment on lines
+38
to
+48
| test("worktreeWarning fires only when expectation is unset AND branch matches the worktree-suffix pattern", () => { | ||
| // No expectation, on a non-worktree branch like 'main' -> no warning | ||
| const baseline = verifyBranch({}); | ||
| if (baseline.current === "main" || baseline.current === "master") { | ||
| expect(baseline.worktreeWarning).toBe(false); | ||
| } | ||
| // When we DO set an expectation, worktreeWarning is suppressed by design | ||
| // (the user asserted what they want, no need for the heuristic). | ||
| const r = verifyBranch({ ZETA_EXPECTED_BRANCH: baseline.current }); | ||
| expect(r.worktreeWarning).toBe(false); | ||
| }); |
AceHack
added a commit
that referenced
this pull request
May 5, 2026
…ME (B-0191 #2 + #3) (#1586) Closes B-0191 acceptance criterion #2 (hook wired -- but opt-in via settings.json edit, not auto-activated) and #3 (README in .claude/hooks/ explaining the wiring). Per Otto-364 search-first authority: WebSearched the canonical Anthropic Claude Code hooks reference at https://code.claude.com/docs/en/hooks before designing the wrapper. Schema: PreToolUse hook returns hookSpecificOutput JSON with permissionDecision: "allow" | "deny" | "ask" | "defer", or exits 0 (allow) / exits 2 (block with stderr as reason). Implementation choice: wrapper translates verify-branch.ts's exit code + stderr to the PreToolUse JSON contract. Default-off behavior preserved -- the script files exist on disk, but no harness behavior changes until .claude/settings.json is explicitly edited to wire the hook. This avoids unilaterally changing commit flow for any contributor running this repo. Wiring is documented in .claude/hooks/README.md with the exact settings.json snippet to add. The "if" clause restricts the hook to "Bash(git commit*)" so other Bash invocations are unaffected. Composes with: - tools/orchestrator-checks/verify-branch.ts (PR #1585) -- the underlying check that this wraps - memory/feedback_orchestrator_pre_commit_verify_branch_rule_aaron_2026_05_04.md (PR #1568) -- the manual discipline being mechanized - memory/feedback_dst_justifies_ts_quality_over_bash_and_harness_hooks_suffice_no_git_hooks_aaron_2026_05_03.md -- the harness-hooks-suffice rule Acceptance criterion #4 (test: deliberate wrong-branch commit blocked) already covered by tools/orchestrator-checks/verify-branch.test.ts (4 tests passing). The wrapper itself is thin and integration-tested when the hook is opted in. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 8, 2026
The verify-branch script and PreToolUse wrapper (PRs #1585, #1586) existed on disk but were never wired into .claude/settings.json. This commit activates the hook and fixes two issues found during wiring: 1. Remove invalid "if" field from README config snippet — Claude Code hooks API has no "if" field (verified against upstream docs at code.claude.com/docs/en/hooks-guide, 2026-05-08). The field was silently ignored. 2. Add stdin-based command filtering to the PreToolUse wrapper so it only runs verify-branch.ts on `git commit` commands. Three early-exit layers: (a) ZETA_EXPECTED_BRANCH unset → exit 0 without reading stdin, (b) command is not git-commit → exit 0 after parsing stdin, (c) branch matches → exit 0 from verify-branch.ts. This avoids spawning bun+git on every Bash tool call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AceHack
added a commit
that referenced
this pull request
May 8, 2026
…2151) * feat(B-0191): wire verify-branch PreToolUse hook into settings.json The verify-branch script and PreToolUse wrapper (PRs #1585, #1586) existed on disk but were never wired into .claude/settings.json. This commit activates the hook and fixes two issues found during wiring: 1. Remove invalid "if" field from README config snippet — Claude Code hooks API has no "if" field (verified against upstream docs at code.claude.com/docs/en/hooks-guide, 2026-05-08). The field was silently ignored. 2. Add stdin-based command filtering to the PreToolUse wrapper so it only runs verify-branch.ts on `git commit` commands. Three early-exit layers: (a) ZETA_EXPECTED_BRANCH unset → exit 0 without reading stdin, (b) command is not git-commit → exit 0 after parsing stdin, (c) branch matches → exit 0 from verify-branch.ts. This avoids spawning bun+git on every Bash tool call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * hook(B-0191): wire verify-branch harness PreToolUse hook (smallest safe slice) Wires the existing verify-branch.ts as Claude Code PreToolUse hook on Bash git commit invocations. Blocks wrong-branch commits when ZETA_EXPECTED_BRANCH is set. One bounded wiring step per task rules; no root checkout touched; worktree + claim branch used; focused test passed (4/4). Focused checks outcome: - bun test tools/orchestrator-checks/verify-branch.test.ts: 4 pass, 0 fail - dotnet build -c Release: 0 warnings 0 errors (pre-edit gate) - JSON valid, hook syntax per harness contract Composes with B-0191 acceptance (harness hook wiring). Co-Authored-By: Grok <noreply@x.ai> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Grok <noreply@x.ai> Co-authored-by: Cursor <cursoragent@cursor.com>
AceHack
added a commit
that referenced
this pull request
May 9, 2026
…(AC3) The core harness hook implementation (tools/orchestrator-checks/verify-branch.ts, .claude/hooks/verify-branch-pretooluse.ts, settings.json wiring) landed in PRs #1585/#1586/#2151. AC3 (CLAUDE.md/AGENTS.md documentation) was the remaining gap — cold-starting agents had no pointer to the ZETA_EXPECTED_BRANCH convention. This slice adds: - .claude/rules/zeta-expected-branch.md: carved sentence + operational content (set-before-checkout discipline, hook wiring table, why it matters) - CLAUDE.md pointer bullet: one-liner with the opt-in semantics and PR refs, discoverable at every session wake - Backlog row: status open → in-progress, pre-start checklist appended per backlog-item-start-gate rule Tests: 4/4 pass (bun test tools/orchestrator-checks/verify-branch.test.ts) Build: 0 warnings 0 errors (dotnet build -c Release) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
AceHack
added a commit
that referenced
this pull request
May 9, 2026
…(AC3) (#2239) * docs(B-0191): add ZETA_EXPECTED_BRANCH rule file + CLAUDE.md pointer (AC3) The core harness hook implementation (tools/orchestrator-checks/verify-branch.ts, .claude/hooks/verify-branch-pretooluse.ts, settings.json wiring) landed in PRs #1585/#1586/#2151. AC3 (CLAUDE.md/AGENTS.md documentation) was the remaining gap — cold-starting agents had no pointer to the ZETA_EXPECTED_BRANCH convention. This slice adds: - .claude/rules/zeta-expected-branch.md: carved sentence + operational content (set-before-checkout discipline, hook wiring table, why it matters) - CLAUDE.md pointer bullet: one-liner with the opt-in semantics and PR refs, discoverable at every session wake - Backlog row: status open → in-progress, pre-start checklist appended per backlog-item-start-gate rule Tests: 4/4 pass (bun test tools/orchestrator-checks/verify-branch.test.ts) Build: 0 warnings 0 errors (dotnet build -c Release) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(B-0191): add blank lines around lists to satisfy MD032 Four list blocks in the pre-start checklist were missing the required blank line between the bold-text label and the list body, triggering MD032 (blanks-around-lists) in markdownlint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(B-0191): address Copilot review — valid status + reconcile loading-taxonomy Two Copilot findings from PR #2239: 1. status: in-progress → status: open. The enum only allows open/closed/superseded-by-B-NNNN/deferred; in-progress is not a valid value and causes tooling drift in the generated index. Express in-progress state in the body. 2. Reconcile the CLAUDE.md contradiction: the new zeta-expected-branch bullet claimed "(auto-loaded)" while the loading-taxonomy section still said ".claude/rules/ auto-load is unverified — canary test pending". Both claims were in the same document. Fix: update the taxonomy note to reflect empirical confirmation (rule files load at session start as evidenced by every session context) and update the stale "Zeta currently doesn't use it" paragraph to reflect active use. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(B-0191): resolve review threads on PR #2239 - backlog row: revert status to 'open' (in-progress is not a valid status enum per tools/backlog/README.md; valid values are open / closed / superseded-by-B-NNNN / deferred) - CLAUDE.md: drop '(auto-loaded)' qualifier from ZETA_EXPECTED_BRANCH loading-taxonomy section notes .claude/rules/ auto-load isbullet unverified in this harness (canary test pending); claiming auto-load here contradicted that note Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(B-0191): align review notes on expected branch rule - keep backlog status wording consistent with supported enum - remove stale rules auto-load unverified sentence after canary confirmation - regenerate docs/BACKLOG.md Co-Authored-By: Codex <noreply@openai.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com>
3 tasks
AceHack
added a commit
that referenced
this pull request
May 9, 2026
All primary acceptance criteria are now met: - AC1: tools/orchestrator-checks/verify-branch.ts (PR #1585) - AC2: .claude/settings.json hook wiring (PR #2151) - AC3: .claude/rules/zeta-expected-branch.md + CLAUDE.md pointer (PR #2239) - AC4: unit tests in verify-branch.test.ts - AC5: tools/orchestrator-checks/check-orchestrator-state.ts (this PR) The state-check script (Rule 0: TS not bash) emits structured JSON with currentBranch, expectedBranch, branchMatch, dirtyFiles, worktrees, and driftedWorktrees — the last field flags the CWD-bleed-over hazard when another worktree is on the same branch as ZETA_EXPECTED_BRANCH. Tests: 12 pass / 0 fail across both orchestrator-checks files. Build: 0 warnings / 0 errors. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the verify-branch harness pre-tool-use hook designed in B-0191 (PR #1571, merged earlier this session). Single TypeScript file under
tools/orchestrator-checks/with unit tests.Behavior
ZETA_EXPECTED_BRANCHis set and doesn't matchgit branch --show-current: print error to stderr, exit 1 → blocks commit when wired as a pre-tool-use hookZETA_EXPECTED_BRANCHis unset and current branch matches the worktree-suffix pattern: print INFO to stderr, exit 0 → informational nudge, doesn't blockAcceptance criteria mapping (B-0191)
tools/orchestrator-checks/verify-branch.ts.claude/settings.jsonPreToolUseTest plan
Composes with
memory/feedback_orchestrator_pre_commit_verify_branch_rule_aaron_2026_05_04.md-- the manual discipline this mechanizesmemory/feedback_dst_justifies_ts_quality_over_bash_and_harness_hooks_suffice_no_git_hooks_aaron_2026_05_03.md-- TS-over-bash + harness-hooks-sufficememory/feedback_parallel_subagent_concurrency_lessons_cluster_aaron_2026_05_04.md-- the orchestrator-CWD-bleed-over hazard this catches🤖 Generated with Claude Code