From 4b1a38345950568191562a7ebaaca3b8fe4a9809 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Thu, 23 Apr 2026 20:11:24 +0200 Subject: [PATCH] Keep branch start auto-transfer alive under pipefail The protected-branch auto-transfer path used \, which can trip \ when \ exits after finding the matching stash message. That leaves the stash ref lookup fragile during \. Use a helper that reads the stash list once and resolves the matching ref without a pipeline, then mirror it across the installed, template, and frontend copies. Add focused regression coverage for the installed script path. Constraint: Shipped branch-start script copies must stay behavior-identical across installed, template, and frontend surfaces Rejected: Disable pipefail for the stash lookup | would hide real bootstrap failures on protected branches Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep stash-ref lookup shared and pipeline-free across all branch-start copies Tested: node --test test/branch.test.js; git diff --check Not-tested: Full gx branch finish/PR flow against GitHub --- frontend/scripts/agent-branch-start.sh | 18 ++++++-- .../.openspec.yaml | 2 + .../notes.md | 23 ++++++++++ scripts/agent-branch-start.sh | 18 ++++++-- templates/scripts/agent-branch-start.sh | 18 ++++++-- test/branch.test.js | 43 +++++++++++++++++++ 6 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/.openspec.yaml create mode 100644 openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/notes.md diff --git a/frontend/scripts/agent-branch-start.sh b/frontend/scripts/agent-branch-start.sh index ef8cc11f..cc7eb5c8 100755 --- a/frontend/scripts/agent-branch-start.sh +++ b/frontend/scripts/agent-branch-start.sh @@ -308,6 +308,19 @@ has_local_changes() { return 1 } +resolve_stash_ref_by_message() { + local root="$1" + local message="$2" + local stash_list + stash_list="$(git -C "$root" stash list 2>/dev/null || true)" + if [[ -z "$stash_list" ]]; then + printf '' + return 0 + fi + + awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list" +} + resolve_protected_branches() { local root="$1" local raw @@ -556,10 +569,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra if has_local_changes "$repo_root"; then auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}" if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then - auto_transfer_stash_ref="$( - git -C "$repo_root" stash list \ - | awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' - )" + auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")" 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}'..." diff --git a/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/.openspec.yaml b/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/.openspec.yaml new file mode 100644 index 00000000..8b394c66 --- /dev/null +++ b/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-23 diff --git a/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/notes.md b/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/notes.md new file mode 100644 index 00000000..ebe41c59 --- /dev/null +++ b/openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/notes.md @@ -0,0 +1,23 @@ +# agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01 (minimal / T1) + +Branch: `agent/codex/fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01` + +Protected-branch auto-transfer can misread the new stash ref under `set -o pipefail` because the `git stash list | awk ... exit` pipeline exits early once `awk` finds the matching message. Resolve the ref by reading the stash list once into a helper and matching there so `gx branch start` keeps moving local changes into the new agent worktree without tripping the pipefail path. + +Scope: +- Add `resolve_stash_ref_by_message` to `scripts/agent-branch-start.sh`, `templates/scripts/agent-branch-start.sh`, and `frontend/scripts/agent-branch-start.sh` so all shipped copies use the same safe stash lookup. +- Replace the inline `git stash list | awk ... exit` lookup with the shared helper on the protected-branch auto-transfer path. +- Add focused regression coverage in `test/branch.test.js` that exercises the installed script path and proves the base checkout ends clean with no leftover `guardex-auto-transfer-*` stash entry. + +Verification: +- `node --test test/branch.test.js` + +## Handoff + +- Handoff: change=`agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01`; branch=`agent/codex/fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01`; scope=`scripts/agent-branch-start.sh, templates/scripts/agent-branch-start.sh, frontend/scripts/agent-branch-start.sh, test/branch.test.js, openspec/changes/agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01/*`; action=`land the pipefail-safe stash lookup fix, verify with focused branch tests, then finish via PR merge + cleanup`. + +## Cleanup + +- [ ] Run: `gx branch finish --branch agent/codex/fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01 --base main --via-pr --wait-for-merge --cleanup` +- [ ] Record PR URL + `MERGED` state in the completion handoff. +- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`). diff --git a/scripts/agent-branch-start.sh b/scripts/agent-branch-start.sh index 0559d8d4..7fbff298 100755 --- a/scripts/agent-branch-start.sh +++ b/scripts/agent-branch-start.sh @@ -370,6 +370,19 @@ has_local_changes() { return 1 } +resolve_stash_ref_by_message() { + local root="$1" + local message="$2" + local stash_list + stash_list="$(git -C "$root" stash list 2>/dev/null || true)" + if [[ -z "$stash_list" ]]; then + printf '' + return 0 + fi + + awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list" +} + resolve_protected_branches() { local root="$1" local raw @@ -616,10 +629,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra if has_local_changes "$repo_root"; then auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}" if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then - auto_transfer_stash_ref="$( - git -C "$repo_root" stash list \ - | awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' - )" + auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")" 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}'..." diff --git a/templates/scripts/agent-branch-start.sh b/templates/scripts/agent-branch-start.sh index 0559d8d4..7fbff298 100755 --- a/templates/scripts/agent-branch-start.sh +++ b/templates/scripts/agent-branch-start.sh @@ -370,6 +370,19 @@ has_local_changes() { return 1 } +resolve_stash_ref_by_message() { + local root="$1" + local message="$2" + local stash_list + stash_list="$(git -C "$root" stash list 2>/dev/null || true)" + if [[ -z "$stash_list" ]]; then + printf '' + return 0 + fi + + awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list" +} + resolve_protected_branches() { local root="$1" local raw @@ -616,10 +629,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra if has_local_changes "$repo_root"; then auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}" if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then - auto_transfer_stash_ref="$( - git -C "$repo_root" stash list \ - | awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' - )" + auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")" 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}'..." diff --git a/test/branch.test.js b/test/branch.test.js index 02a3e3eb..129c04a1 100644 --- a/test/branch.test.js +++ b/test/branch.test.js @@ -198,6 +198,49 @@ test('agent-branch-start restores protected-branch changes when startup fails af assert.doesNotMatch(stashList.stdout, /guardex-auto-transfer-/); }); +test('installed agent-branch-start script survives auto-transfer stash lookup under pipefail', () => { + 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-script-auto-transfer'; + fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}\n`, 'utf8'); + + const branchStartScript = path.resolve(__dirname, '..', 'scripts', 'agent-branch-start.sh'); + + result = runCmd('bash', [branchStartScript, 'script-auto-transfer', 'bot'], repoDir, { + GUARDEX_CLI_ENTRY: cliPath, + GUARDEX_NODE_BIN: process.execPath, + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Created branch: agent\/codex\/script-auto-transfer-/); + + const agentWorktree = extractCreatedWorktree(result.stdout); + assert.equal(fs.existsSync(path.join(agentWorktree, 'package.json')), true, 'worktree should be created'); + + const rootStatus = runCmd('git', ['status', '--short'], repoDir); + assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); + assert.equal(rootStatus.stdout.trim(), '', 'base branch checkout should be clean after auto-transfer'); + + 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();