fix(genies): local-first resolution + existing PR handling

[Sillhouette]

How batch execution works

Understanding this context makes the bugs below make sense.

Each worker runs a full PDLC lifecycle:

deliver → discern → commit → done

• /commit creates the PR
• /done archives the backlog item (adds commits to the branch after the PR exists)

After all workers finish, the batch runner runs an integration phase for each succeeded item. It pushes any post-/commit commits (i.e. from /done) to the remote branch so they land in the PR, then ensures a PR exists.

---

Bugs fixed

1. Integration phase fails when /commit already created a PR

session_integrate_pr called gh pr create unconditionally. Since /commit already opened the PR, gh returned an error — and the batch marked the item as failed despite an APPROVED run.

Fix: check for an existing open PR first. If found, return it as success. The branch push still runs first so /done commits are included.

2. Global genies ignores project-local install

When genie-team is installed both globally and in a project, the global binary always ran. Project-level installs (e.g. to test a PR branch) were silently ignored.

Fix: path-based shim at startup. If running from ~/.claude/scripts/, check for a project-scoped .claude/scripts/genies and exec into it. Project copies skip this block entirely — no env guard, no recursion risk.

3. genies status required explicit --log-dir

Running genies status without --log-dir errored immediately, even during an active batch.

Fix: auto-detect the most recent logs/batch-*/ directory in the repo root.

4. Side-effect files appeared as batch items in status

The status scanner picked up all *.log files, including claude_stderr.log written by the runner itself.

Fix: skip files matching claude_* and batch-manifest*.

---

Test plan

• Parallel batch on items with existing open PRs → succeeded, not failed
• /done commits appear in the PR after integration
• genies from a project with a local install → uses local version
• genies from a dir with no local install → falls back to global
• genies status with no flags during active batch → auto-detects log dir
• Only item slugs in status table, not claude_stderr

🤖 Generated with Claude Code

[Sillhouette]

@npatterson I reworked the PR a bit to address 4 clear issues and bugs, please take a look when you have a chance!

[Sillhouette]

Critic Review — PR #16: fix(genies): local-first resolution + existing PR handling

All CI checks pass (lint ✓, test ✓). Four real production bugs fixed, all with tests, all commits clean and conventional.

---

Bug-by-Bug Assessment

Bug 1 — session_integrate_pr called gh pr create unconditionally ✅

Fix is correct. The guard queries gh pr list --head "$branch" --base "$default_branch" --json url --jq '.[0].url' and early-returns if a PR exists. Push still runs first, so /done commits land in the branch before the check.

One observation on the test: The mock gh in test_session.sh matches [[ "$1 $2" == "pr list" ]] but ignores all flags (--head, --base, --json, --jq). If the wrong branch name were passed to gh pr list, the test wouldn't catch it. Low risk in practice, but worth a follow-up.

Bug 2 — Global genies ignores project-local install ✅

Clean implementation using BASH_SOURCE[0] for path detection — more reliable than $0 under sourcing. The check only fires from ~/.claude/scripts/, so project installs skip it entirely. No recursion risk.

Edge case handled correctly: when git rev-parse --show-toplevel fails outside a git repo, the resulting _local_genies path won't be executable, so it falls through safely.

Bug 3 — genies status required explicit --log-dir ✅

Fix is correct. The || true guard on ls -1dt | head -1 properly handles pipefail when no batch dirs exist (the f3e4821 follow-up commit correctly added this).

The test update pinning STATUS_LOG_DIR="/nonexistent/no-batch-logs" is good defensive hygiene — without it, the test would silently pass if a real logs/batch-*/ dir existed during CI.

Bug 4 — Side-effect files appear as batch items in status ✅

The [[ "$slug" == claude_* || "$slug" == batch-manifest* ]] guard correctly uses [[ ]] for shell glob matching. Test covers both item presence and noise-file absence.

---

Minor Observations (non-blocking)

1. test_session.sh mock doesn't validate --head/--base flags — consider tightening to [[ "$*" == *"--head genie/existing-pr-item-deliver"* ]] in a follow-up.
2. No test for "global genies invoked outside a git repo" — the not-a-git-repo path is safe by inspection but untested.
3. gh pr list empty-array behavior — --jq '.[0].url' on [] returns empty string, correctly handled by [[ -n "$existing_pr" ]]. Confirmed safe.

---

Verdict: APPROVED — minimal, targeted fixes, green CI, good test coverage. Merge when ready.

[Sillhouette]

Both non-blocking observations addressed:

1. Mock tightened to validate --head flag (test_session.sh, squashed into commit 2)

The gh pr list mock now asserts the correct branch name is passed:

if [[ "$1 $2" == "pr list" && "$*" == *"--head genie/existing-pr-item-deliver"* ]]; then

A wrong branch name would now cause the mock to fall through to the exit 1 arm, catching the regression.

2. Outside-a-git-repo path now tested (test_run_pdlc.sh, squashed into commit 1)

New test invokes the global shim from a plain temp dir (no git init). git rev-parse --show-toplevel fails, _local_genies is empty/non-executable, shim falls through — global runs and exits 3 (no-input) as expected.

531 tests, all green.

[Sillhouette]

Follow-up Review Pass

Both observations from the first review were addressed:

1. test_session.sh mock now validates --head flag — the condition tightened to [[ "$1 $2" == "pr list" && "$*" == *"--head genie/existing-pr-item-deliver"* ]], with the fallthrough arm printing actual args for easier debugging. Exactly what was requested.

2. New test: global genies outside a git repo — the FAKE_HOME3/FAKE_NOGREPO case confirms the shim falls through safely when git rev-parse --show-toplevel fails. Covers the previously untested edge case.

CI still green (lint ✓, test ✓). No new concerns. All observations resolved.

Verdict: APPROVED — ready to merge.

[Sillhouette]

Full Review — PR #16 (final pass)

CI: lint ✓ test ✓ (531 assertions across 3 test suites, 0 failures)
Scope: 146 additions, 1 deletion across 5 files, 4 commits

---

Fix 1 — session_integrate_pr skips gh pr create when PR exists

Correct. The guard calls gh pr list --head "$branch" --base "$default_branch" --json url --jq '.[0].url', assigns to a local, and early-returns on non-empty. Push still runs first, so /done commits reach the remote branch before the check. URL is echoed on success, matching the normal-path contract.

Branch cleanup note (non-blocking, pre-existing gap): The early return at line 518 bypasses the branch -D at line 540. However, this is not a regression: before this PR, gh pr create would fail and return 1, also bypassing line 540. The branch-not-deleted behavior is identical in both cases. The underlying gap (no worktree/branch cleanup on integration success in batch mode) predates this PR and should be addressed separately.

Test: Mock now validates --head genie/existing-pr-item-deliver in addition to the pr list subcommand. Unexpected calls print the full args and exit 1. Solid.

---

Fix 2 — Local-first genies resolution

Correct. Uses BASH_SOURCE[0] for path detection (correct under sourcing). The shim only fires from ~/.claude/scripts/ — project installs skip it entirely. The exec "$_local_genies" "$@" path correctly propagates all arguments.

Outside-git-repo edge case: When git rev-parse --show-toplevel fails, _local_genies resolves to /.claude/scripts/genies. The [[ -x ]] check is false for this path, so it falls through safely. Now tested by the new FAKE_NOGREPO case.

Tests: All three cases covered — global execs local, global falls back when no local, global falls through outside git repo. Clean isolation via fake $HOME.

---

Fix 3 — Auto-detect batch log dir in genies status

Correct. ls -1dt "$repo_root"/logs/batch-* 2>/dev/null | head -1) || true handles both the no-match case (ls exit 1 under pipefail) and populates log_dir with the most recent directory. Falls through to the existing error path if still empty.

Test fix is important: LOG_DIR="" STATUS_LOG_DIR="/nonexistent/no-batch-logs" prevents CI from accidentally auto-detecting a real batch dir and masking the error path. Correct defensive hygiene.

---

Fix 4 — Skip non-item log files in status scanner

Correct. [[ "$slug" == claude_* || "$slug" == batch-manifest* ]] uses [[ ]] for glob matching. slug is derived from basename "$log_file" .log, so this filters claude_stderr.log, claude_stdout.log, batch-manifest.log out of the table.

Two minor dead-code observations:

1. batch-manifest* in the skip guard: the production batch runner writes batch-manifest.json, which has no .log extension and is never picked up by the *.log glob. The pattern is unreachable in production. The test creates batch-manifest.log artificially to exercise it. Harmless defensive code, but the comment could note that this anticipates future tooling rather than guarding an existing artifact.

2. claude_stdout.log: the production runner only redirects stderr to claude_stderr.log (lines 774, 868). There is no code path that writes claude_stdout.log. The exclusion pattern and test fixture for it are defensive. Also harmless but worth a comment.

Neither of these affects correctness or the tests.

---

Security

No new attack surface. The local-first exec dispatches to a path derived from git rev-parse --show-toplevel, not from PATH or user input. gh pr list output is assigned to a local and checked with [[ -n ]] — no shell injection risk from the URL string.

---

Summary

Fix	Correct	Tests
Skip gh pr create when PR exists	✅	✅ Branch name validated in mock
Local-first resolution	✅	✅ All 3 paths covered
Auto-detect batch log dir	✅	✅ Error path pinned correctly
Skip non-item log files	✅	✅ Item present, noise absent

Verdict: APPROVED. All four bugs fixed correctly, no regressions, CI green. The branch-cleanup and dead-code observations are follow-up items, not blockers.

[Sillhouette]

@npatterson Candidly, I'm not sure Fix 2 — Local-first genies resolution is the right move, I was assuming there is value in project-level genie-team but I'm not so sure there is much. As long as the global version of genie team knows where the target project directory is it shouldn't matter much, we could remove project-level dependencies on genie-team altogether.

Thoughts?