diff --git a/openspec/changes/fix-branch-start-stash-leak/proposal.md b/openspec/changes/fix-branch-start-stash-leak/proposal.md new file mode 100644 index 00000000..2ab1c4f5 --- /dev/null +++ b/openspec/changes/fix-branch-start-stash-leak/proposal.md @@ -0,0 +1,7 @@ +# Proposal: restore auto-transfer stashes when branch start fails + +`gx branch start` currently leaks `guardex-auto-transfer-*` stashes if startup fails after local changes are stashed off a protected branch. That leaves duplicate stash entries for paths like `memory-bank/` even though the branch never started. The safer behavior is to restore those changes back to the original checkout on failure and drop the temporary stash. + +- add failure-safe auto-restore for temporary branch-start transfer stashes +- keep the success path unchanged when the new worktree is created and the stash applies cleanly +- lock the `memory-bank/` failure case with a focused regression diff --git a/openspec/changes/fix-branch-start-stash-leak/tasks.md b/openspec/changes/fix-branch-start-stash-leak/tasks.md new file mode 100644 index 00000000..1d4d6c09 --- /dev/null +++ b/openspec/changes/fix-branch-start-stash-leak/tasks.md @@ -0,0 +1,25 @@ +## 1. Spec + +- [x] 1.1 Capture why failed branch-start attempts must restore temporary auto-transfer stashes instead of leaking them. + +## 2. Implementation + +- [x] 2.1 Update `templates/scripts/agent-branch-start.sh` and `scripts/agent-branch-start.sh` with failure-safe auto-restore. +- [x] 2.2 Add a focused regression in `test/branch.test.js` for the `memory-bank/` failure path. + +## 3. Verification + +- [x] 3.1 Run targeted Guardex branch tests. +- [x] 3.2 Run script parity/diff hygiene. +- [x] 3.3 Run `openspec validate --specs`. + +Verification evidence: +- `node --test test/branch.test.js` (pass) +- `node --test test/metadata.test.js` (pass) +- `bash -n scripts/agent-branch-start.sh` and `bash -n templates/scripts/agent-branch-start.sh` (pass) +- `git diff --check` (pass) +- `openspec validate --specs` (no items found to validate) + +## 4. Cleanup + +- [ ] 4.1 Commit, push, open/update PR, merge, and clean up the worktree. diff --git a/scripts/agent-branch-start.sh b/scripts/agent-branch-start.sh index 9da6d603..0559d8d4 100755 --- a/scripts/agent-branch-start.sh +++ b/scripts/agent-branch-start.sh @@ -155,6 +155,15 @@ env_flag_truthy() { esac } +maybe_fail_after_auto_transfer_stash() { + if env_flag_truthy "${GUARDEX_TEST_FAIL_AFTER_AUTO_TRANSFER_STASH:-}"; then + echo "[agent-branch-start] Simulated failure after capturing auto-transfer stash." >&2 + return 1 + fi + + return 0 +} + default_worktree_root_rel() { local raw_agent="$1" local override="${GUARDEX_AGENT_TYPE:-}" @@ -580,6 +589,27 @@ fi auto_transfer_stash_ref="" auto_transfer_message="" auto_transfer_source_branch="" +auto_transfer_completed=0 + +restore_auto_transfer_stash_on_failure() { + local exit_code="${1:-0}" + if [[ "$exit_code" -eq 0 ]] || [[ -z "$auto_transfer_stash_ref" ]] || [[ "$auto_transfer_completed" -eq 1 ]]; then + return 0 + fi + + local transfer_label="${auto_transfer_source_branch:-$BASE_BRANCH}" + if git -C "$repo_root" stash apply "$auto_transfer_stash_ref" >/dev/null 2>&1; then + git -C "$repo_root" stash drop "$auto_transfer_stash_ref" >/dev/null 2>&1 || true + auto_transfer_stash_ref="" + echo "[agent-branch-start] Restored moved changes back to '${transfer_label}' after startup failure." >&2 + else + echo "[agent-branch-start] Startup failed and auto-restore also failed." >&2 + echo "[agent-branch-start] Changes are preserved in ${auto_transfer_stash_ref} on ${transfer_label}." >&2 + fi +} + +trap 'restore_auto_transfer_stash_on_failure "$?"' EXIT + current_branch="$(git -C "$repo_root" rev-parse --abbrev-ref HEAD 2>/dev/null || true)" protected_branches_raw="$(resolve_protected_branches "$repo_root")" if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_branch_name "$current_branch" "$protected_branches_raw"; then @@ -593,6 +623,9 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra if [[ -n "$auto_transfer_stash_ref" ]]; then auto_transfer_source_branch="$current_branch" echo "[agent-branch-start] Detected local changes on protected branch '${current_branch}'. Moving them to '${branch_name}'..." + if ! maybe_fail_after_auto_transfer_stash; then + exit 1 + fi fi fi fi @@ -610,7 +643,9 @@ git -C "$worktree_path" branch --unset-upstream "$branch_name" >/dev/null 2>&1 | if [[ -n "$auto_transfer_stash_ref" ]]; then if git -C "$worktree_path" stash apply "$auto_transfer_stash_ref" >/dev/null 2>&1; then + auto_transfer_completed=1 git -C "$repo_root" stash drop "$auto_transfer_stash_ref" >/dev/null 2>&1 || true + auto_transfer_stash_ref="" transfer_label="${auto_transfer_source_branch:-$BASE_BRANCH}" echo "[agent-branch-start] Moved local changes from '${transfer_label}' into '${branch_name}'." else diff --git a/templates/scripts/agent-branch-start.sh b/templates/scripts/agent-branch-start.sh index 9da6d603..0559d8d4 100755 --- a/templates/scripts/agent-branch-start.sh +++ b/templates/scripts/agent-branch-start.sh @@ -155,6 +155,15 @@ env_flag_truthy() { esac } +maybe_fail_after_auto_transfer_stash() { + if env_flag_truthy "${GUARDEX_TEST_FAIL_AFTER_AUTO_TRANSFER_STASH:-}"; then + echo "[agent-branch-start] Simulated failure after capturing auto-transfer stash." >&2 + return 1 + fi + + return 0 +} + default_worktree_root_rel() { local raw_agent="$1" local override="${GUARDEX_AGENT_TYPE:-}" @@ -580,6 +589,27 @@ fi auto_transfer_stash_ref="" auto_transfer_message="" auto_transfer_source_branch="" +auto_transfer_completed=0 + +restore_auto_transfer_stash_on_failure() { + local exit_code="${1:-0}" + if [[ "$exit_code" -eq 0 ]] || [[ -z "$auto_transfer_stash_ref" ]] || [[ "$auto_transfer_completed" -eq 1 ]]; then + return 0 + fi + + local transfer_label="${auto_transfer_source_branch:-$BASE_BRANCH}" + if git -C "$repo_root" stash apply "$auto_transfer_stash_ref" >/dev/null 2>&1; then + git -C "$repo_root" stash drop "$auto_transfer_stash_ref" >/dev/null 2>&1 || true + auto_transfer_stash_ref="" + echo "[agent-branch-start] Restored moved changes back to '${transfer_label}' after startup failure." >&2 + else + echo "[agent-branch-start] Startup failed and auto-restore also failed." >&2 + echo "[agent-branch-start] Changes are preserved in ${auto_transfer_stash_ref} on ${transfer_label}." >&2 + fi +} + +trap 'restore_auto_transfer_stash_on_failure "$?"' EXIT + current_branch="$(git -C "$repo_root" rev-parse --abbrev-ref HEAD 2>/dev/null || true)" protected_branches_raw="$(resolve_protected_branches "$repo_root")" if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_branch_name "$current_branch" "$protected_branches_raw"; then @@ -593,6 +623,9 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra if [[ -n "$auto_transfer_stash_ref" ]]; then auto_transfer_source_branch="$current_branch" echo "[agent-branch-start] Detected local changes on protected branch '${current_branch}'. Moving them to '${branch_name}'..." + if ! maybe_fail_after_auto_transfer_stash; then + exit 1 + fi fi fi fi @@ -610,7 +643,9 @@ git -C "$worktree_path" branch --unset-upstream "$branch_name" >/dev/null 2>&1 | if [[ -n "$auto_transfer_stash_ref" ]]; then if git -C "$worktree_path" stash apply "$auto_transfer_stash_ref" >/dev/null 2>&1; then + auto_transfer_completed=1 git -C "$repo_root" stash drop "$auto_transfer_stash_ref" >/dev/null 2>&1 || true + auto_transfer_stash_ref="" transfer_label="${auto_transfer_source_branch:-$BASE_BRANCH}" echo "[agent-branch-start] Moved local changes from '${transfer_label}' into '${branch_name}'." else diff --git a/test/branch.test.js b/test/branch.test.js index 08ad4650..02a3e3eb 100644 --- a/test/branch.test.js +++ b/test/branch.test.js @@ -157,6 +157,47 @@ test('agent-branch-start moves protected-branch local changes into the new agent assert.doesNotMatch(stashList.stdout, /guardex-auto-transfer-/); }); +test('agent-branch-start restores protected-branch changes when startup fails after auto-transfer stash capture', () => { + const repoDir = initRepoOnBranch('main'); + seedCommit(repoDir); + attachOriginRemoteForBranch(repoDir, 'main'); + + 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.stdout); + result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', 'origin', 'main'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const packageJsonPath = path.join(repoDir, 'package.json'); + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')); + packageJson.name = 'demo-failed-auto-transfer'; + fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}\n`, 'utf8'); + fs.mkdirSync(path.join(repoDir, 'memory-bank'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, 'memory-bank', 'note.md'), 'keep me local\n', 'utf8'); + + result = runBranchStart(['fail-after-auto-transfer', 'bot'], repoDir, { + GUARDEX_TEST_FAIL_AFTER_AUTO_TRANSFER_STASH: '1', + }); + assert.equal(result.status, 1, 'branch start should fail after the simulated post-stash error'); + assert.match(result.stderr, /Simulated failure after capturing auto-transfer stash/); + assert.match(result.stderr, /Restored moved changes back to 'main' after startup failure/); + + const rootStatus = runCmd('git', ['status', '--short'], repoDir); + assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); + assert.match(rootStatus.stdout, / M package\.json/); + assert.match(rootStatus.stdout, /\?\? memory-bank\//); + + const stashList = runCmd('git', ['stash', 'list'], repoDir); + assert.equal(stashList.status, 0, stashList.stderr || stashList.stdout); + assert.doesNotMatch(stashList.stdout, /guardex-auto-transfer-/); +}); + test('agent-branch-start leaves removed workflow helpers out of new worktrees', () => { const repoDir = initRepo();