Skip to content

fix(autocoder): pin target SHA in merge-to-integration.sh, don't trust mutable HEAD - #133

Open
laird wants to merge 1 commit into
masterfrom
fix/athena-1736-pin-merge-sha
Open

fix(autocoder): pin target SHA in merge-to-integration.sh, don't trust mutable HEAD#133
laird wants to merge 1 commit into
masterfrom
fix/athena-1736-pin-merge-sha

Conversation

@laird

@laird laird commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Problem

Reported against athena2 as ey-org/athena2#1736 (P0, corroborated by a second, independent observer on that thread).

merge-to-integration.sh pushes with git push origin "HEAD:${INTEGRATION_BRANCH}" and later verifies with git rev-parse "HEAD^{tree}" — both re-read the worktree's mutable HEAD at the moment they run, not a value pinned when the script actually had the right commit checked out.

When a second /fix invocation reuses the same worktree directory for a different issue (git checkout -b feature/issue-N origin/<integration>) while the first script's --test-cmd is still running (which can take minutes), it silently repoints that worktree's .git/HEAD. The first script's later push and tree-check then operate on the hijacked branch instead of the commit it actually built — and in the reported incident the hijacked branch happened to already equal origin/main, so the push reported Everything up-to-date and the tree comparison matched trivially. The script printed ✅ ... merged and verified while nothing had landed.

This is the swarm's only proof a merge landed (branch protection has enforce_admins: false), so a verification step that can pass on a no-op run makes every "merged and verified" log line unreliable.

Fix

Capture the commit we intend to land (TARGET_SHA=$(git rev-parse HEAD)) immediately after each merge that produces it (the initial pre-integration sync, and again after each push-retry re-sync), and use $TARGET_SHA — not HEAD — for:

  • the push (git push origin "${TARGET_SHA}:${INTEGRATION_BRANCH}")
  • the GitHub API push fallback
  • the tree-verification (git rev-parse "${TARGET_SHA}^{tree}")

A captured SHA is a git object reference, valid regardless of what HEAD is repointed to afterward, so it can't be fooled by a worktree hijack the way re-reading HEAD was.

Also added a check right after the initial git checkout "$FEATURE" that fails loudly if the worktree isn't actually on $FEATURE, instead of silently proceeding on whatever happens to be checked out — this catches the same hazard if it occurs before the script's first merge.

Out of scope (flagged in the issue as a "consider"): a lock around worktree reuse in the dispatcher, so a new /fix invocation can't git checkout in a worktree that still has a live merge-to-integration.sh/merge-poll.sh process. That's a separate, larger change to the worker-loop dispatcher and doesn't block this fix — this fix makes the verification trustworthy even if that hazard isn't eliminated.

Testing

Added tests/test_merge_to_integration_worktree_hijack.sh: builds a real throwaway git repo, runs the script with --test-cmd 'git checkout -b feature/issue-8888 origin/main' (the hijack — --test-cmd runs bash -c in the script's own cwd, the same real repo the checkout repoints), and asserts the feature commit actually reaches origin/main.

  • Confirmed this test fails against the pre-fix script: exits 0, prints "merged and verified", but origin/main only contains the pre-existing content (reproduces the exact reported incident).
  • Confirmed it passes against the fixed script: origin/main contains the feature commit.
  • Full suite: bash tests/run-shell-suite.sh → 27/27 passed (including the new test).
  • Build verification: bash -n plugins/autocoder/scripts/*.sh && python3 -m py_compile plugins/autocoder/scripts/*.py → OK.

Closes the athena2-side defect once this merges; will comment back on ey-org/athena2#1736 with this PR link.

… of re-reading HEAD

A concurrent /fix invocation reusing the same worktree directory for a
different issue can run `git checkout -b feature/issue-N origin/<integration>`
while an in-flight merge-to-integration.sh for another issue is still mid-run
(its test-cmd can take minutes). Both the push (`HEAD:$INTEGRATION_BRANCH`)
and the later tree-verification (`HEAD^{tree}`) re-read the worktree's
mutable HEAD at the moment they run, so a hijack between the pre-integration
merge and those steps makes both silently push/verify nothing while the
script still reports "merged and verified".

Capture the intended commit's SHA once, immediately after each merge that
produces it, and use that SHA (not HEAD) for the push, the retry loop, the
GitHub API fallback, and the tree comparison. A git object hash is valid
regardless of what HEAD points to later, so it can't be fooled by the hijack
the way re-reading HEAD was. Also added a check right after the initial
checkout that fails loudly if the worktree isn't actually on $FEATURE,
instead of silently proceeding on whatever branch happens to be checked out.

Added tests/test_merge_to_integration_worktree_hijack.sh, which reproduces
the exact hijack (via --test-cmd, which runs in the script's own cwd — the
same real repo the checkout below repoints) against a real throwaway git
repo. Confirmed this fails against the pre-fix script (reports "merged and
verified" while main only has the pre-existing content) and passes against
the fix (main actually contains the feature commit).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant