From b64df03db577d623909c313097a722160c63918d Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Fri, 2 Oct 2026 12:32:08 +0200 Subject: [PATCH 1/4] fix(cleanup): reclaim squash-merged lanes by default; add `gx agents start --cleanup-only` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gx cleanup` judged "merged" by git ancestry unless --include-pr-merged was passed. In squash/rebase-merge repos the branch tip never lands on the base, so every merged lane looked unmerged and its worktree was kept forever — on one KFB station repo 21 merged worktrees (~2.5 GB each) filled the disk. - cleanup: --include-pr-merged is now the default; --no-include-pr-merged restores ancestry-only. A missing or failing `gh` still disables the lookup (fail-closed: nothing is treated as merged). - agents start --cleanup-only: run just the cleanup bot, without the token-spending review bot. The bot inherits the new default. Tests: parse defaults + opt-out, the cleanup->prune path queries merged PRs (fake GUARDEX_GH_BIN) and skips it on opt-out, --cleanup-only bot lifecycle. Full suite: 1452/1458 pass, the one failure is identical on origin/main. Co-Authored-By: Claude Opus 5.5 --- src/cli/args.js | 15 +++++- src/cli/commands/agents.js | 24 +++++++--- src/context.js | 5 +- src/finish/index.js | 3 +- test/agents.test.js | 88 ++++++++++++++++++++++++++++++++++ test/cli-args-dispatch.test.js | 13 +++++ 6 files changed, 138 insertions(+), 10 deletions(-) diff --git a/src/cli/args.js b/src/cli/args.js index fbbc8709..8e0a337a 100644 --- a/src/cli/args.js +++ b/src/cli/args.js @@ -390,6 +390,7 @@ function parseAgentsArgs(rawArgs) { dryRun: false, reviewIntervalSeconds: 30, cleanupIntervalSeconds: 60, + cleanupOnly: false, idleMinutes: DEFAULT_SHADOW_CLEANUP_IDLE_MINUTES, staleAgeMinutes: 24 * 60, pid: null, @@ -551,6 +552,10 @@ function parseAgentsArgs(rawArgs) { index += 1; continue; } + if (arg === '--cleanup-only') { + options.cleanupOnly = true; + continue; + } if (arg === '--cleanup-interval') { const next = rest[index + 1]; if (!next) { @@ -1029,7 +1034,11 @@ function parseCleanupArgs(rawArgs) { keepRemote: false, keepCleanWorktrees: true, includeCleanLinkedWorktrees: false, - includePrMerged: false, + // Squash/rebase merges leave the branch tip off the base's history, so + // ancestry alone reports a merged PR's branch as unmerged and cleanup + // never reclaims it. Consult merged PRs by default; `gh` missing or failing + // disables the lookup (fail-closed: nothing is treated as merged). + includePrMerged: true, idleMinutes: 0, watch: false, intervalSeconds: 60, @@ -1094,6 +1103,10 @@ function parseCleanupArgs(rawArgs) { options.includePrMerged = true; continue; } + if (arg === '--no-include-pr-merged') { + options.includePrMerged = false; + continue; + } if (arg === '--idle-minutes') { const next = rawArgs[index + 1]; if (!next) { diff --git a/src/cli/commands/agents.js b/src/cli/commands/agents.js index 9934866a..20ab0ddf 100644 --- a/src/cli/commands/agents.js +++ b/src/cli/commands/agents.js @@ -254,7 +254,15 @@ function agents(rawArgs) { const reviewRunning = processAlive(existingReviewPid); const cleanupRunning = processAlive(existingCleanupPid); - if (reviewRunning && cleanupRunning) { + // --cleanup-only: run just the cleanup bot. The review bot spends model + // tokens on every PR; reclaiming merged lanes should not require it. + const reviewWanted = !options.cleanupOnly; + if ((reviewRunning || !reviewWanted) && cleanupRunning) { + if (!reviewWanted) { + console.log(`[${TOOL_NAME}] Cleanup bot already running (pid=${existingCleanupPid}).`); + process.exitCode = 0; + return; + } console.log( `[${TOOL_NAME}] Repo agents already running (review pid=${existingReviewPid}, cleanup pid=${existingCleanupPid}).`, ); @@ -271,7 +279,7 @@ function agents(rawArgs) { let reusedAny = false; const mainScriptPath = require.resolve('../main.js'); - if (!reviewRunning) { + if (reviewWanted && !reviewRunning) { reviewPid = spawnDetachedAgentProcess({ command: process.execPath, args: [ @@ -288,7 +296,7 @@ function agents(rawArgs) { logPath: reviewLogPath, }); startedAny = true; - } else { + } else if (reviewRunning) { reusedAny = true; } @@ -333,7 +341,7 @@ function agents(rawArgs) { repoRoot, startedAt: new Date().toISOString(), review: { - pid: reviewPid, + pid: reviewWanted || reviewRunning ? reviewPid : null, intervalSeconds: reviewIntervalSeconds, script: mainScriptPath, logPath: reviewLogPath, @@ -348,12 +356,16 @@ function agents(rawArgs) { }); console.log( - `[${TOOL_NAME}] Started repo agents in ${repoRoot} (review pid=${reviewPid}, cleanup pid=${cleanupPid}).`, + reviewWanted + ? `[${TOOL_NAME}] Started repo agents in ${repoRoot} (review pid=${reviewPid}, cleanup pid=${cleanupPid}).` + : `[${TOOL_NAME}] Started the cleanup bot in ${repoRoot} (cleanup pid=${cleanupPid}; review bot not started: --cleanup-only).`, ); if (reusedAny && startedAny) { console.log(`[${TOOL_NAME}] Reused healthy bot process(es) and started only missing ones.`); } - console.log(`[${TOOL_NAME}] Logs: ${reviewLogPath}, ${cleanupLogPath}`); + console.log( + `[${TOOL_NAME}] Logs: ${reviewWanted ? `${reviewLogPath}, ` : ''}${cleanupLogPath}`, + ); process.exitCode = 0; return; } diff --git a/src/context.js b/src/context.js index 2b2ac8be..d88d68f8 100644 --- a/src/context.js +++ b/src/context.js @@ -674,7 +674,7 @@ const CLI_COMMAND_HELP = { summary: 'Repo-scoped review and cleanup bots, plus the agent inbox and lane inspection.', subcommands: [ ['status', 'Show the running bots and the agent lanes (the default with no subcommand)'], - ['start', 'Start the review + cleanup bots for this repo'], + ['start', 'Start the review + cleanup bots for this repo (--cleanup-only: just the cleanup bot)'], ['stop', 'Stop them'], ['finish', 'Finish an agent lane by session or branch'], ['files', 'Show which files each lane has touched'], @@ -693,8 +693,9 @@ const CLI_COMMAND_HELP = { ['--session ', 'Address one agent session'], ['--branch ', 'Address one agent branch'], ['--dry-run', 'Print actions without applying them'], + ['--cleanup-only', 'start: run only the cleanup bot (no token-spending review bot)'], ], - examples: ['gx agents status --json', 'gx agents files --branch agent/claude/my-lane'], + examples: ['gx agents status --json', 'gx agents start --cleanup-only', 'gx agents files --branch agent/claude/my-lane'], }, 'pr-review': { diff --git a/src/finish/index.js b/src/finish/index.js index 552145f4..11c89151 100644 --- a/src/finish/index.js +++ b/src/finish/index.js @@ -235,7 +235,8 @@ OPTIONS --keep-clean-worktrees Preserve clean, unmerged agent worktrees (default) --include-clean-linked-worktrees Also prune clean linked worktrees outside managed agent directories - --include-pr-merged Treat branches from merged PRs as merged + --include-pr-merged Treat branches from merged PRs as merged (default; needs gh) + --no-include-pr-merged Judge merged by git ancestry only (misses squash merges) --idle-minutes Only consider worktrees idle for at least n minutes --watch Repeat cleanup cycles (defaults idle threshold to 60) --interval Watch interval in seconds (minimum: 5; default: 60) diff --git a/test/agents.test.js b/test/agents.test.js index cb24f970..0a04433e 100644 --- a/test/agents.test.js +++ b/test/agents.test.js @@ -275,6 +275,94 @@ test('agents command starts review+cleanup bots for the target repo and stops th }); +test('agents start --cleanup-only starts only the cleanup bot', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + const scriptsDir = path.join(repoDir, 'scripts'); + fs.mkdirSync(scriptsDir, { recursive: true }); + const reviewScriptPath = path.join(scriptsDir, 'review-bot-watch.sh'); + fs.writeFileSync(reviewScriptPath, fakeReviewBotDaemonScript(), 'utf8'); + fs.chmodSync(reviewScriptPath, 0o755); + const pruneScriptPath = path.join(scriptsDir, 'agent-worktree-prune.sh'); + fs.writeFileSync( + pruneScriptPath, + '#!/usr/bin/env bash\n' + + 'set -euo pipefail\n' + + 'exit 0\n', + 'utf8', + ); + fs.chmodSync(pruneScriptPath, 0o755); + + let result = runNode( + ['agents', 'start', '--target', repoDir, '--cleanup-only', '--cleanup-interval', '47', '--idle-minutes', '12'], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Started the cleanup bot/); + assert.match(result.stdout, /review bot not started: --cleanup-only/); + + const statePath = path.join(repoDir, '.omx', 'state', 'agents-bots.json'); + const state = JSON.parse(fs.readFileSync(statePath, 'utf8')); + assert.equal(state.review.pid, null, 'no review bot under --cleanup-only'); + assert.equal(isPidAlive(state.cleanup.pid), true, 'cleanup bot pid should be alive after start'); + assert.equal( + fs.existsSync(path.join(repoDir, '.omx', 'logs', 'agent-review.log')), + false, + 'no review log under --cleanup-only', + ); + + // A second start is a no-op while the cleanup bot runs. + result = runNode(['agents', 'start', '--target', repoDir, '--cleanup-only'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Cleanup bot already running/); + + result = runNode(['agents', 'stop', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.equal(waitForPidExit(state.cleanup.pid), true, 'cleanup bot pid should exit after stop'); +}); + +test('gx cleanup consults merged PRs by default and skips them under --no-include-pr-merged', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + const execGit = (dir, args) => { + const out = runCmd('git', args, dir); + assert.equal(out.status, 0, out.stderr || out.stdout); + return String(out.stdout || '').trim(); + }; + const base = execGit(repoDir, ['rev-parse', '--abbrev-ref', 'HEAD']); + // An agent lane whose tip is NOT an ancestor of the base — what a squash + // merge leaves behind — so only the PR lookup can call it merged. + execGit(repoDir, ['checkout', '-q', '-b', 'agent/claude/squashed-lane']); + fs.writeFileSync(path.join(repoDir, 'lane.txt'), 'lane\n', 'utf8'); + execGit(repoDir, ['add', 'lane.txt']); + execGit(repoDir, ['commit', '-q', '-m', 'lane work']); + execGit(repoDir, ['checkout', '-q', base]); + + const ghLog = path.join(repoDir, '.gh-calls.log'); + const fakeGh = path.join(repoDir, 'fake-gh.sh'); + fs.writeFileSync(fakeGh, `#!/usr/bin/env bash\necho "$*" >> ${JSON.stringify(ghLog)}\nexit 0\n`, 'utf8'); + fs.chmodSync(fakeGh, 0o755); + + let result = runNodeWithEnv( + ['cleanup', '--target', repoDir, '--base', base, '--dry-run'], + repoDir, + { GUARDEX_GH_BIN: fakeGh }, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + const calls = fs.existsSync(ghLog) ? fs.readFileSync(ghLog, 'utf8') : ''; + assert.match(calls, /pr list --state merged/, 'default cleanup must ask for merged PRs'); + + fs.rmSync(ghLog, { force: true }); + result = runNodeWithEnv( + ['cleanup', '--target', repoDir, '--base', base, '--dry-run', '--no-include-pr-merged'], + repoDir, + { GUARDEX_GH_BIN: fakeGh }, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + const optOut = fs.existsSync(ghLog) ? fs.readFileSync(ghLog, 'utf8') : ''; + assert.doesNotMatch(optOut, /--state merged/, 'opt-out must not query merged PRs'); +}); + test('agents start reuses running review bot when only cleanup bot is missing', () => { const repoDir = initRepo(); seedCommit(repoDir); diff --git a/test/cli-args-dispatch.test.js b/test/cli-args-dispatch.test.js index c4d286c0..015ed3d0 100644 --- a/test/cli-args-dispatch.test.js +++ b/test/cli-args-dispatch.test.js @@ -136,6 +136,7 @@ test('parseAgentsArgs applies interval overrides and validates the subcommand', dryRun: false, reviewIntervalSeconds: 15, cleanupIntervalSeconds: 45, + cleanupOnly: false, idleMinutes: 12, staleAgeMinutes: 1440, pid: null, @@ -328,6 +329,18 @@ test('parseCleanupArgs defaults idle minutes when watch mode is enabled', () => assert.equal(options.idleMinutes, DEFAULT_SHADOW_CLEANUP_IDLE_MINUTES); }); +test('parseCleanupArgs consults merged PRs by default so squash-merged lanes are reclaimed', () => { + assert.equal(parseCleanupArgs([]).includePrMerged, true); + assert.equal(parseCleanupArgs(['--include-pr-merged']).includePrMerged, true); + assert.equal(parseCleanupArgs(['--no-include-pr-merged']).includePrMerged, false); + assert.equal(parseCleanupArgs(['--watch']).includePrMerged, true); +}); + +test('parseAgentsArgs accepts --cleanup-only for a cleanup bot without the review bot', () => { + assert.equal(parseAgentsArgs(['start']).cleanupOnly, false); + assert.equal(parseAgentsArgs(['start', '--cleanup-only']).cleanupOnly, true); +}); + test('parseCleanupArgs preserves clean worktrees unless pruning is explicitly requested', () => { assert.equal(parseCleanupArgs([]).keepCleanWorktrees, true); assert.equal(parseCleanupArgs(['--prune-clean-worktrees']).keepCleanWorktrees, false); From 1e2fdcee6288f12b120409e79458550fe9bfa271 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Fri, 2 Oct 2026 12:40:49 +0200 Subject: [PATCH 2/4] fix(cleanup): a merged PR only counts when the lane tip is its head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#816): with --include-pr-merged on by default, a name-only match would let cleanup delete a reused branch name, or a lane with commits pushed after its PR merged. The merged-PR lookup now records each PR's headRefOid; a lane counts as merged only when its tip equals that SHA or is contained in it. Unknown/unrelated SHAs fail closed. Also: `agents start --cleanup-only` no longer claims the review bot was not started when one is already running — it says it was left as is. Tests: tip == PR head → reclaimed; commits after merge → kept; unrelated SHA for a reused name → kept. The existing squash-merge cleanup test's fake gh now reports name + head SHA like the real one. Full suite: 1453/1459, the one failure identical on origin/main. Co-Authored-By: Claude Opus 5.5 --- src/cli/commands/agents.js | 4 +- templates/scripts/agent-worktree-prune.sh | 33 +++++++++++--- test/agents.test.js | 55 +++++++++++++++++++++++ test/finish.test.js | 4 +- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/cli/commands/agents.js b/src/cli/commands/agents.js index 20ab0ddf..a09aa728 100644 --- a/src/cli/commands/agents.js +++ b/src/cli/commands/agents.js @@ -358,7 +358,9 @@ function agents(rawArgs) { console.log( reviewWanted ? `[${TOOL_NAME}] Started repo agents in ${repoRoot} (review pid=${reviewPid}, cleanup pid=${cleanupPid}).` - : `[${TOOL_NAME}] Started the cleanup bot in ${repoRoot} (cleanup pid=${cleanupPid}; review bot not started: --cleanup-only).`, + : reviewRunning + ? `[${TOOL_NAME}] Started the cleanup bot in ${repoRoot} (cleanup pid=${cleanupPid}); the review bot already running (pid=${existingReviewPid}) is left as is.` + : `[${TOOL_NAME}] Started the cleanup bot in ${repoRoot} (cleanup pid=${cleanupPid}; review bot not started: --cleanup-only).`, ); if (reusedAny && startedAny) { console.log(`[${TOOL_NAME}] Reused healthy bot process(es) and started only missing ones.`); diff --git a/templates/scripts/agent-worktree-prune.sh b/templates/scripts/agent-worktree-prune.sh index 91cafed2..35ace9fd 100644 --- a/templates/scripts/agent-worktree-prune.sh +++ b/templates/scripts/agent-worktree-prune.sh @@ -228,14 +228,18 @@ load_merged_pr_branches() { return 1 fi + # Name -> space-separated merged PR head SHAs. The SHA is what makes the + # name match safe: a reused branch name, or commits pushed after the PR + # merged, must not read as "merged" (see branch_has_merged_pr). local merged_branches="" merged_branches="$( - "$GH_BIN" pr list --state merged --base "$BASE_BRANCH" --limit 200 --json headRefName --jq '.[].headRefName' 2>/dev/null || true + "$GH_BIN" pr list --state merged --base "$BASE_BRANCH" --limit 200 --json headRefName,headRefOid --jq '.[] | "\(.headRefName) \(.headRefOid)"' 2>/dev/null || true )" if [[ -n "$merged_branches" ]]; then - while IFS= read -r merged_branch; do - [[ -z "$merged_branch" ]] && continue - MERGED_PR_BRANCHES["$merged_branch"]=1 + local merged_branch merged_oid + while IFS=' ' read -r merged_branch merged_oid; do + [[ -z "$merged_branch" || -z "$merged_oid" ]] && continue + MERGED_PR_BRANCHES["$merged_branch"]="${MERGED_PR_BRANCHES[$merged_branch]:-} ${merged_oid}" done <<< "$merged_branches" fi PR_MERGED_LOOKUP_LOADED=1 @@ -288,7 +292,26 @@ branch_has_merged_pr() { return 1 fi load_merged_pr_branches || return 1 - [[ -n "${MERGED_PR_BRANCHES[$branch]:-}" ]] + local merged_oids="${MERGED_PR_BRANCHES[$branch]:-}" + [[ -n "$merged_oids" ]] || return 1 + # Merged only if the branch tip IS a merged PR's head, or is contained in + # it. Anything newer on the branch (post-merge commits, a reused name) is + # unmerged work — fail closed. A PR head we do not have locally cannot + # prove containment, so only the exact-SHA match applies then. + local tip="" + tip="$(git -C "$repo_root" rev-parse --verify --quiet "refs/heads/${branch}^{commit}" 2>/dev/null || true)" + if [[ -z "$tip" ]]; then + tip="$(git -C "$repo_root" rev-parse --verify --quiet "refs/remotes/origin/${branch}^{commit}" 2>/dev/null || true)" + fi + [[ -n "$tip" ]] || return 1 + local oid + for oid in $merged_oids; do + [[ "$tip" == "$oid" ]] && return 0 + if git -C "$repo_root" merge-base --is-ancestor "$tip" "$oid" >/dev/null 2>&1; then + return 0 + fi + done + return 1 } load_lane_pr_metadata() { diff --git a/test/agents.test.js b/test/agents.test.js index 0a04433e..ebfa5ba1 100644 --- a/test/agents.test.js +++ b/test/agents.test.js @@ -363,6 +363,61 @@ test('gx cleanup consults merged PRs by default and skips them under --no-includ assert.doesNotMatch(optOut, /--state merged/, 'opt-out must not query merged PRs'); }); +test('gx cleanup treats a lane as PR-merged only when its tip matches the merged PR head', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + const execGit = (dir, args) => { + const out = runCmd('git', args, dir); + assert.equal(out.status, 0, out.stderr || out.stdout); + return String(out.stdout || '').trim(); + }; + const base = execGit(repoDir, ['rev-parse', '--abbrev-ref', 'HEAD']); + const lane = 'agent/claude/squashed-lane'; + execGit(repoDir, ['checkout', '-q', '-b', lane]); + fs.writeFileSync(path.join(repoDir, 'lane.txt'), 'lane\n', 'utf8'); + execGit(repoDir, ['add', 'lane.txt']); + execGit(repoDir, ['commit', '-q', '-m', 'lane work']); + const prHead = execGit(repoDir, ['rev-parse', 'HEAD']); + execGit(repoDir, ['checkout', '-q', base]); + + const fakeGhFor = (oid) => { + const fakeGh = path.join(repoDir, 'fake-gh.sh'); + fs.writeFileSync( + fakeGh, + '#!/usr/bin/env bash\n' + + `if [[ "$*" == *"--state merged"* ]]; then echo "${lane} ${oid}"; fi\n` + + 'exit 0\n', + 'utf8', + ); + fs.chmodSync(fakeGh, 0o755); + return fakeGh; + }; + const dryRun = (fakeGh) => + runNodeWithEnv(['cleanup', '--target', repoDir, '--base', base, '--dry-run'], repoDir, { + GUARDEX_GH_BIN: fakeGh, + }); + + // Tip == merged PR head: the squash-merged lane is reclaimed. + let result = dryRun(fakeGhFor(prHead)); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(`${result.stdout}${result.stderr}`, new RegExp(`merged PR branch: ${lane}`)); + + // Work pushed after the PR merged: the tip is past the PR head — keep it. + execGit(repoDir, ['checkout', '-q', lane]); + fs.writeFileSync(path.join(repoDir, 'later.txt'), 'later\n', 'utf8'); + execGit(repoDir, ['add', 'later.txt']); + execGit(repoDir, ['commit', '-q', '-m', 'work after merge']); + execGit(repoDir, ['checkout', '-q', base]); + result = dryRun(fakeGhFor(prHead)); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.doesNotMatch(`${result.stdout}${result.stderr}`, new RegExp(`merged PR branch: ${lane}`)); + + // A reused name whose merged PR head is unrelated to this tip — keep it. + result = dryRun(fakeGhFor('0123456789abcdef0123456789abcdef01234567')); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.doesNotMatch(`${result.stdout}${result.stderr}`, new RegExp(`merged PR branch: ${lane}`)); +}); + test('agents start reuses running review bot when only cleanup bot is missing', () => { const repoDir = initRepo(); seedCommit(repoDir); diff --git a/test/finish.test.js b/test/finish.test.js index 807901d7..7bfdee1f 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -1834,9 +1834,11 @@ test('cleanup command can remove squash-merged agent branches via merged PR dete result = runCmd('git', ['-C', worktreePath, 'commit', '-m', 'feature commit'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); + // A squash-merged PR's head is the lane tip; gh reports name + head SHA. + const laneTip = runCmd('git', ['-C', worktreePath, 'rev-parse', 'HEAD'], repoDir).stdout.trim(); const { fakePath: fakeGhPath } = createFakeGhScript( 'if [[ "$1" == "pr" && "$2" == "list" ]]; then\n' + - ' printf \'%s\\n\' "agent/test-cleanup-pr-merged"\n' + + ` printf '%s %s\\n' "agent/test-cleanup-pr-merged" "${laneTip}"\n` + ' exit 0\n' + 'fi\n' + 'exit 1', From e12a660885565f660cb1d146005b221930fd51df Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Fri, 2 Oct 2026 12:45:02 +0200 Subject: [PATCH 3/4] fix(cleanup): a PR-merged lane needs its origin tip covered too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#816): only the local tip was checked, with origin as a fallback. A stale local branch at the merged PR head while origin carries newer work read as merged, and cleanup (keepRemote defaults to false) would delete the remote work. Every visible tip — local and origin — must now equal or be contained in a merged PR head. Test: local at PR head + origin past it → kept. Full suite 1453/1459, the one failure identical on origin/main. Co-Authored-By: Claude Opus 5.5 --- templates/scripts/agent-worktree-prune.sh | 39 +++++++++++++---------- test/agents.test.js | 11 +++++++ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/templates/scripts/agent-worktree-prune.sh b/templates/scripts/agent-worktree-prune.sh index 35ace9fd..7b6bf33f 100644 --- a/templates/scripts/agent-worktree-prune.sh +++ b/templates/scripts/agent-worktree-prune.sh @@ -294,24 +294,29 @@ branch_has_merged_pr() { load_merged_pr_branches || return 1 local merged_oids="${MERGED_PR_BRANCHES[$branch]:-}" [[ -n "$merged_oids" ]] || return 1 - # Merged only if the branch tip IS a merged PR's head, or is contained in - # it. Anything newer on the branch (post-merge commits, a reused name) is - # unmerged work — fail closed. A PR head we do not have locally cannot - # prove containment, so only the exact-SHA match applies then. - local tip="" - tip="$(git -C "$repo_root" rev-parse --verify --quiet "refs/heads/${branch}^{commit}" 2>/dev/null || true)" - if [[ -z "$tip" ]]; then - tip="$(git -C "$repo_root" rev-parse --verify --quiet "refs/remotes/origin/${branch}^{commit}" 2>/dev/null || true)" - fi - [[ -n "$tip" ]] || return 1 - local oid - for oid in $merged_oids; do - [[ "$tip" == "$oid" ]] && return 0 - if git -C "$repo_root" merge-base --is-ancestor "$tip" "$oid" >/dev/null 2>&1; then - return 0 - fi + # Merged only if every tip we can see — the local branch AND origin's — + # IS a merged PR's head or is contained in one. Anything newer (commits + # pushed after the merge, locally or to origin; a reused name) is unmerged + # work — fail closed. A PR head we do not have locally cannot prove + # containment, so only the exact-SHA match applies then. + local tips=() ref resolved + for ref in "refs/heads/${branch}" "refs/remotes/origin/${branch}"; do + resolved="$(git -C "$repo_root" rev-parse --verify --quiet "${ref}^{commit}" 2>/dev/null || true)" + [[ -n "$resolved" ]] && tips+=("$resolved") done - return 1 + [[ ${#tips[@]} -gt 0 ]] || return 1 + local tip oid contained + for tip in "${tips[@]}"; do + contained=0 + for oid in $merged_oids; do + if [[ "$tip" == "$oid" ]] || git -C "$repo_root" merge-base --is-ancestor "$tip" "$oid" >/dev/null 2>&1; then + contained=1 + break + fi + done + [[ "$contained" -eq 1 ]] || return 1 + done + return 0 } load_lane_pr_metadata() { diff --git a/test/agents.test.js b/test/agents.test.js index ebfa5ba1..6cd797eb 100644 --- a/test/agents.test.js +++ b/test/agents.test.js @@ -412,6 +412,17 @@ test('gx cleanup treats a lane as PR-merged only when its tip matches the merged assert.equal(result.status, 0, result.stderr || result.stdout); assert.doesNotMatch(`${result.stdout}${result.stderr}`, new RegExp(`merged PR branch: ${lane}`)); + // Local branch at the PR head, but origin moved past it after the merge: + // the remote carries unmerged work — keep it. + execGit(repoDir, ['branch', '-f', lane, prHead]); + const laterOnOrigin = execGit(repoDir, ['rev-parse', `${lane}@{1}`]); + execGit(repoDir, ['update-ref', `refs/remotes/origin/${lane}`, laterOnOrigin]); + result = dryRun(fakeGhFor(prHead)); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.doesNotMatch(`${result.stdout}${result.stderr}`, new RegExp(`merged PR branch: ${lane}`)); + execGit(repoDir, ['update-ref', '-d', `refs/remotes/origin/${lane}`]); + execGit(repoDir, ['branch', '-f', lane, laterOnOrigin]); + // A reused name whose merged PR head is unrelated to this tip — keep it. result = dryRun(fakeGhFor('0123456789abcdef0123456789abcdef01234567')); assert.equal(result.status, 0, result.stderr || result.stdout); From 3f0c1ade4d3c6be045039d6376f56534856b331b Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Fri, 2 Oct 2026 12:47:49 +0200 Subject: [PATCH 4/4] fix(agents): list the review log when --cleanup-only leaves a running review bot Review follow-up (#816, LOW): the Logs line now matches the state file. Co-Authored-By: Claude Opus 5.5 --- src/cli/commands/agents.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/commands/agents.js b/src/cli/commands/agents.js index a09aa728..08733b73 100644 --- a/src/cli/commands/agents.js +++ b/src/cli/commands/agents.js @@ -366,7 +366,7 @@ function agents(rawArgs) { console.log(`[${TOOL_NAME}] Reused healthy bot process(es) and started only missing ones.`); } console.log( - `[${TOOL_NAME}] Logs: ${reviewWanted ? `${reviewLogPath}, ` : ''}${cleanupLogPath}`, + `[${TOOL_NAME}] Logs: ${reviewWanted || reviewRunning ? `${reviewLogPath}, ` : ''}${cleanupLogPath}`, ); process.exitCode = 0; return;