Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
## Why

- `scripts/agent-branch-finish.sh` currently treats `git branch -d <agent-branch>` as a hard cleanup failure even after the merge already succeeded.
- In the observed false-negative, `gh pr merge --delete-branch` reported that local branch deletion failed because of an active worktree, but by the time Guardex continued its own cleanup the local branch ref was already gone.
- That leaves the merge outcome correct but still exits non-zero, which forces needless manual bookkeeping follow-ups.

## What Changes

- Make finish cleanup tolerate an already-missing local source branch during the post-merge branch-delete step.
- Keep the existing warning for the GitHub CLI local-delete error, but continue cleanup when the branch ref has already disappeared.
- Add a focused finish regression for the race where the local branch is gone by the time Guardex reaches its own cleanup.

## Impact

- Affects only the post-merge cleanup path in `agent-branch-finish.sh`.
- Keeps true branch-delete failures fatal, but downgrades the already-deleted case to an informational cleanup warning.
- Reduces false-negative finish exits while preserving merge, remote-delete, and worktree-prune behavior.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
## ADDED Requirements

### Requirement: finish cleanup tolerates an already-missing local source branch after merge
The `gx branch finish` cleanup flow SHALL treat the local source-branch delete step as successful when the branch ref is already absent by the time post-merge cleanup runs.

#### Scenario: GitHub merge reports a local-branch delete problem but the branch is already gone during Guardex cleanup
- **GIVEN** `scripts/agent-branch-finish.sh` merges an `agent/*` branch through the PR flow
- **AND** the GitHub CLI reports a local branch delete problem during `gh pr merge --delete-branch`
- **AND** the local `refs/heads/<agent-branch>` ref is already missing by the time Guardex reaches its own cleanup branch-delete step
- **WHEN** Guardex continues cleanup
- **THEN** the finish command SHALL keep going without failing
- **AND** it SHALL emit an informational warning that the local branch was already deleted
- **AND** it SHALL still continue remote-branch cleanup and worktree pruning

#### Scenario: real local branch delete failures still fail finish cleanup
- **GIVEN** `scripts/agent-branch-finish.sh` reaches the local source-branch delete step
- **AND** the local `refs/heads/<agent-branch>` ref still exists
- **AND** `git branch -d <agent-branch>` fails for a reason other than the branch already being absent
- **WHEN** Guardex handles cleanup
- **THEN** the finish command SHALL still fail
- **AND** it SHALL preserve the underlying git error output
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
## 1. Specification

- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-codex-tolerate-already-deleted-local-branch-in-2026-04-22-20-43`.
- [x] 1.2 Define normative requirements in `specs/workflow-guardrails/spec.md`.

## 2. Implementation

- [x] 2.1 Update `scripts/agent-branch-finish.sh` so post-merge cleanup tolerates an already-missing local source branch.
- [x] 2.2 Mirror the same finish cleanup change into `templates/scripts/agent-branch-finish.sh`.
- [x] 2.3 Add a focused regression in `test/finish.test.js` for the local-branch-already-deleted race.

## 3. Verification

- [x] 3.1 Run focused finish verification (`node --test test/finish.test.js`, `node --check bin/multiagent-safety.js`).
- [x] 3.2 Run parity verification (`node --test test/metadata.test.js`).
- [x] 3.3 Run `openspec validate agent-codex-tolerate-already-deleted-local-branch-in-2026-04-22-20-43 --type change --strict`.
- [x] 3.4 Run `openspec validate --specs`.

## 4. Completion

- [ ] 4.1 Finish the agent branch via PR merge + cleanup (`gx finish --via-pr --wait-for-merge --cleanup` or `bash scripts/agent-branch-finish.sh --branch <agent-branch> --base <base-branch> --via-pr --wait-for-merge --cleanup`).
- [ ] 4.2 Record PR URL + final `MERGED` state in the completion handoff.
- [ ] 4.3 Confirm sandbox cleanup (`git worktree list`, `git branch -a`) or capture a `BLOCKED:` handoff if merge/cleanup is pending.
31 changes: 30 additions & 1 deletion scripts/agent-branch-finish.sh
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,33 @@ is_remote_branch_missing_error() {
return 1
}

local_branch_exists() {
local branch="$1"
git -C "$repo_root" show-ref --verify --quiet "refs/heads/${branch}"
}

delete_local_branch_for_cleanup() {
local branch="$1"
local delete_output=""

if ! local_branch_exists "$branch"; then
echo "[agent-branch-finish] Local branch '${branch}' was already deleted; continuing cleanup." >&2
return 0
fi

if delete_output="$(git -C "$repo_root" branch -d "$branch" 2>&1)"; then
return 0
fi

if ! local_branch_exists "$branch"; then
echo "[agent-branch-finish] Local branch '${branch}' was already deleted; continuing cleanup." >&2
return 0
fi

echo "$delete_output" >&2
return 1
}

read_pr_state() {
local state_line
state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | join("\u001f")' 2>/dev/null || true)"
Expand Down Expand Up @@ -607,7 +634,9 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
git -C "$repo_root" worktree remove "$source_worktree" --force >/dev/null 2>&1 || true
fi

git -C "$repo_root" branch -d "$SOURCE_BRANCH"
if ! delete_local_branch_for_cleanup "$SOURCE_BRANCH"; then
exit 1
fi

if [[ "$PUSH_ENABLED" -eq 1 && "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then
if git -C "$repo_root" ls-remote --exit-code --heads origin "$SOURCE_BRANCH" >/dev/null 2>&1; then
Expand Down
31 changes: 30 additions & 1 deletion templates/scripts/agent-branch-finish.sh
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,33 @@ is_remote_branch_missing_error() {
return 1
}

local_branch_exists() {
local branch="$1"
git -C "$repo_root" show-ref --verify --quiet "refs/heads/${branch}"
}

delete_local_branch_for_cleanup() {
local branch="$1"
local delete_output=""

if ! local_branch_exists "$branch"; then
echo "[agent-branch-finish] Local branch '${branch}' was already deleted; continuing cleanup." >&2
return 0
fi

if delete_output="$(git -C "$repo_root" branch -d "$branch" 2>&1)"; then
return 0
fi

if ! local_branch_exists "$branch"; then
echo "[agent-branch-finish] Local branch '${branch}' was already deleted; continuing cleanup." >&2
return 0
fi

echo "$delete_output" >&2
return 1
}

read_pr_state() {
local state_line
state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | join("\u001f")' 2>/dev/null || true)"
Expand Down Expand Up @@ -607,7 +634,9 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
git -C "$repo_root" worktree remove "$source_worktree" --force >/dev/null 2>&1 || true
fi

git -C "$repo_root" branch -d "$SOURCE_BRANCH"
if ! delete_local_branch_for_cleanup "$SOURCE_BRANCH"; then
exit 1
fi

if [[ "$PUSH_ENABLED" -eq 1 && "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then
if git -C "$repo_root" ls-remote --exit-code --heads origin "$SOURCE_BRANCH" >/dev/null 2>&1; then
Expand Down
83 changes: 83 additions & 0 deletions test/finish.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,89 @@ fi
assert.equal(result.stdout.trim(), '', 'agent branch should be absent on origin');
});

test('agent-branch-finish cleanup tolerates an already-deleted local branch after gh delete warning', () => {
const repoDir = initRepo();
seedCommit(repoDir);
attachOriginRemote(repoDir);

let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);
assert.equal(result.status, 0, result.stderr || result.stdout);
result = runCmd('git', ['add', '.'], repoDir);
assert.equal(result.status, 0, result.stderr);
result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, {
ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1',
});
assert.equal(result.status, 0, result.stderr);
result = runCmd('git', ['push', 'origin', 'dev'], repoDir);
assert.equal(result.status, 0, result.stderr);

const agentWorktreePath = path.join(repoDir, '.omx', 'agent-worktrees', 'agent__local-delete-race');
result = runCmd(
'git',
['worktree', 'add', '-b', 'agent/test-pr-local-delete-race', agentWorktreePath, 'dev'],
repoDir,
);
assert.equal(result.status, 0, result.stderr || result.stdout);

fs.writeFileSync(path.join(agentWorktreePath, 'local-delete-race.txt'), 'cleanup race\n', 'utf8');
result = runCmd('git', ['add', 'local-delete-race.txt'], agentWorktreePath);
assert.equal(result.status, 0, result.stderr);
result = runCmd('git', ['commit', '--no-verify', '-m', 'local delete race change'], agentWorktreePath);
assert.equal(result.status, 0, result.stderr || result.stdout);

const { fakePath: fakeGhPath } = createFakeGhScript(`
if [[ "$1" == "pr" && "$2" == "create" ]]; then
exit 0
fi
if [[ "$1" == "pr" && "$2" == "view" ]]; then
if [[ " $* " == *" --json url "* ]]; then
echo "https://example.test/pr/local-delete-race"
exit 0
fi
echo "unexpected gh pr view args: $*" >&2
exit 1
fi
if [[ "$1" == "pr" && "$2" == "merge" ]]; then
git_bin="$(command -v git)"
"$git_bin" -C "${'${GUARDEX_TEST_AGENT_WORKTREE}'}" checkout --detach >/dev/null 2>&1 || true
"$git_bin" -C "${'${GUARDEX_TEST_REPO_DIR}'}" branch -D "$3" >/dev/null 2>&1 || true
echo "failed to delete local branch $3: error: cannot delete branch '$3' used by worktree at '${'${GUARDEX_TEST_AGENT_WORKTREE}'}'" >&2
echo "/usr/bin/git: exit status 1" >&2
exit 1
fi
echo "unexpected gh args: $*" >&2
exit 1
`);

const finish = runBranchFinish(
['--branch', 'agent/test-pr-local-delete-race', '--base', 'dev', '--mode', 'pr', '--cleanup'],
repoDir,
{
GUARDEX_GH_BIN: fakeGhPath,
GUARDEX_TEST_REPO_DIR: repoDir,
GUARDEX_TEST_AGENT_WORKTREE: agentWorktreePath,
},
);
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
assert.match(
finish.stderr,
/PR merged but gh could not delete the local branch \(active worktree\); continuing local cleanup\./,
);
assert.match(
finish.stderr,
/Local branch 'agent\/test-pr-local-delete-race' was already deleted; continuing cleanup\./,
);
assert.match(
finish.stdout,
/Merged 'agent\/test-pr-local-delete-race' into 'dev' via pr flow and cleaned source branch\/worktree\./,
);

result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-local-delete-race'], repoDir);
assert.notEqual(result.status, 0, 'agent branch should stay deleted locally');
result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-local-delete-race'], repoDir);
assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin');
});


test('agent-branch-finish cleanup succeeds from active agent worktree when base branch is checked out elsewhere', () => {
const repoDir = initRepo();
Expand Down
1 change: 1 addition & 0 deletions test/metadata.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ test('frontend mirror workflow skips cleanly when the mirror PAT is missing', ()
test('critical runtime helper scripts and active-agents sources stay in sync with templates', () => {
const pairs = [
['templates/scripts/agent-branch-start.sh', 'scripts/agent-branch-start.sh'],
['templates/scripts/agent-branch-finish.sh', 'scripts/agent-branch-finish.sh'],
['templates/scripts/codex-agent.sh', 'scripts/codex-agent.sh'],
['templates/scripts/openspec/init-plan-workspace.sh', 'scripts/openspec/init-plan-workspace.sh'],
['templates/scripts/openspec/init-change-workspace.sh', 'scripts/openspec/init-change-workspace.sh'],
Expand Down