Skip to content

Commit f019e27

Browse files
NagyViktNagyVikt
andauthored
Keep branch start auto-transfer alive under pipefail (#397)
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 Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent fad45e8 commit f019e27

6 files changed

Lines changed: 110 additions & 12 deletions

File tree

frontend/scripts/agent-branch-start.sh

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,19 @@ has_local_changes() {
308308
return 1
309309
}
310310

311+
resolve_stash_ref_by_message() {
312+
local root="$1"
313+
local message="$2"
314+
local stash_list
315+
stash_list="$(git -C "$root" stash list 2>/dev/null || true)"
316+
if [[ -z "$stash_list" ]]; then
317+
printf ''
318+
return 0
319+
fi
320+
321+
awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list"
322+
}
323+
311324
resolve_protected_branches() {
312325
local root="$1"
313326
local raw
@@ -556,10 +569,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra
556569
if has_local_changes "$repo_root"; then
557570
auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}"
558571
if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then
559-
auto_transfer_stash_ref="$(
560-
git -C "$repo_root" stash list \
561-
| awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }'
562-
)"
572+
auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")"
563573
if [[ -n "$auto_transfer_stash_ref" ]]; then
564574
auto_transfer_source_branch="$current_branch"
565575
echo "[agent-branch-start] Detected local changes on protected branch '${current_branch}'. Moving them to '${branch_name}'..."
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-04-23
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# agent-codex-fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01 (minimal / T1)
2+
3+
Branch: `agent/codex/fix-branch-start-pipefail-stash-lookup-2026-04-23-20-01`
4+
5+
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.
6+
7+
Scope:
8+
- 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.
9+
- Replace the inline `git stash list | awk ... exit` lookup with the shared helper on the protected-branch auto-transfer path.
10+
- 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.
11+
12+
Verification:
13+
- `node --test test/branch.test.js`
14+
15+
## Handoff
16+
17+
- 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`.
18+
19+
## Cleanup
20+
21+
- [ ] 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`
22+
- [ ] Record PR URL + `MERGED` state in the completion handoff.
23+
- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`).

scripts/agent-branch-start.sh

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,19 @@ has_local_changes() {
370370
return 1
371371
}
372372

373+
resolve_stash_ref_by_message() {
374+
local root="$1"
375+
local message="$2"
376+
local stash_list
377+
stash_list="$(git -C "$root" stash list 2>/dev/null || true)"
378+
if [[ -z "$stash_list" ]]; then
379+
printf ''
380+
return 0
381+
fi
382+
383+
awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list"
384+
}
385+
373386
resolve_protected_branches() {
374387
local root="$1"
375388
local raw
@@ -616,10 +629,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra
616629
if has_local_changes "$repo_root"; then
617630
auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}"
618631
if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then
619-
auto_transfer_stash_ref="$(
620-
git -C "$repo_root" stash list \
621-
| awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }'
622-
)"
632+
auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")"
623633
if [[ -n "$auto_transfer_stash_ref" ]]; then
624634
auto_transfer_source_branch="$current_branch"
625635
echo "[agent-branch-start] Detected local changes on protected branch '${current_branch}'. Moving them to '${branch_name}'..."

templates/scripts/agent-branch-start.sh

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,19 @@ has_local_changes() {
370370
return 1
371371
}
372372

373+
resolve_stash_ref_by_message() {
374+
local root="$1"
375+
local message="$2"
376+
local stash_list
377+
stash_list="$(git -C "$root" stash list 2>/dev/null || true)"
378+
if [[ -z "$stash_list" ]]; then
379+
printf ''
380+
return 0
381+
fi
382+
383+
awk -v msg="$message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }' <<<"$stash_list"
384+
}
385+
373386
resolve_protected_branches() {
374387
local root="$1"
375388
local raw
@@ -616,10 +629,7 @@ if [[ -n "$current_branch" && "$current_branch" != "HEAD" ]] && is_protected_bra
616629
if has_local_changes "$repo_root"; then
617630
auto_transfer_message="guardex-auto-transfer-${timestamp}-${agent_slug}-${task_slug}"
618631
if git -C "$repo_root" stash push --include-untracked --message "$auto_transfer_message" >/dev/null 2>&1; then
619-
auto_transfer_stash_ref="$(
620-
git -C "$repo_root" stash list \
621-
| awk -v msg="$auto_transfer_message" '$0 ~ msg { ref=$1; sub(/:$/, "", ref); print ref; exit }'
622-
)"
632+
auto_transfer_stash_ref="$(resolve_stash_ref_by_message "$repo_root" "$auto_transfer_message")"
623633
if [[ -n "$auto_transfer_stash_ref" ]]; then
624634
auto_transfer_source_branch="$current_branch"
625635
echo "[agent-branch-start] Detected local changes on protected branch '${current_branch}'. Moving them to '${branch_name}'..."

test/branch.test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,49 @@ test('agent-branch-start restores protected-branch changes when startup fails af
198198
assert.doesNotMatch(stashList.stdout, /guardex-auto-transfer-/);
199199
});
200200

201+
test('installed agent-branch-start script survives auto-transfer stash lookup under pipefail', () => {
202+
const repoDir = initRepoOnBranch('main');
203+
seedCommit(repoDir);
204+
attachOriginRemoteForBranch(repoDir, 'main');
205+
206+
let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);
207+
assert.equal(result.status, 0, result.stderr || result.stdout);
208+
209+
result = runCmd('git', ['add', '.'], repoDir);
210+
assert.equal(result.status, 0, result.stderr || result.stdout);
211+
result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, {
212+
ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1',
213+
});
214+
assert.equal(result.status, 0, result.stderr || result.stdout);
215+
result = runCmd('git', ['push', 'origin', 'main'], repoDir);
216+
assert.equal(result.status, 0, result.stderr || result.stdout);
217+
218+
const packageJsonPath = path.join(repoDir, 'package.json');
219+
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8'));
220+
packageJson.name = 'demo-script-auto-transfer';
221+
fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}\n`, 'utf8');
222+
223+
const branchStartScript = path.resolve(__dirname, '..', 'scripts', 'agent-branch-start.sh');
224+
225+
result = runCmd('bash', [branchStartScript, 'script-auto-transfer', 'bot'], repoDir, {
226+
GUARDEX_CLI_ENTRY: cliPath,
227+
GUARDEX_NODE_BIN: process.execPath,
228+
});
229+
assert.equal(result.status, 0, result.stderr || result.stdout);
230+
assert.match(result.stdout, /Created branch: agent\/codex\/script-auto-transfer-/);
231+
232+
const agentWorktree = extractCreatedWorktree(result.stdout);
233+
assert.equal(fs.existsSync(path.join(agentWorktree, 'package.json')), true, 'worktree should be created');
234+
235+
const rootStatus = runCmd('git', ['status', '--short'], repoDir);
236+
assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout);
237+
assert.equal(rootStatus.stdout.trim(), '', 'base branch checkout should be clean after auto-transfer');
238+
239+
const stashList = runCmd('git', ['stash', 'list'], repoDir);
240+
assert.equal(stashList.status, 0, stashList.stderr || stashList.stdout);
241+
assert.doesNotMatch(stashList.stdout, /guardex-auto-transfer-/);
242+
});
243+
201244

202245
test('agent-branch-start leaves removed workflow helpers out of new worktrees', () => {
203246
const repoDir = initRepo();

0 commit comments

Comments
 (0)