diff --git a/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/.openspec.yaml b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/.openspec.yaml new file mode 100644 index 0000000..81cd71f --- /dev/null +++ b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-11 diff --git a/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/proposal.md b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/proposal.md new file mode 100644 index 0000000..509ba24 --- /dev/null +++ b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/proposal.md @@ -0,0 +1,16 @@ +## Why + +- `gx branch finish` can push a parent branch that updates a submodule gitlink before the submodule branch containing that commit is on its remote. +- That strands the parent branch/PR with a gitlink that collaborators and CI cannot fetch, and it causes agents to ask for separate ad hoc `git push` approval outside the Guardex finish flow. + +## What Changes + +- During `gx branch finish`, changed submodule gitlinks are detected before the parent branch is pushed or merged. +- For each changed checked-out submodule, Guardex pushes the local branch containing the gitlink commit to that submodule's configured remote. +- If the submodule commit cannot be safely pushed, finish fails before pushing the parent branch. + +## Impact + +- Affects `scripts/agent-branch-finish.sh` PR and direct push flows. +- Keeps network approval consolidated around the single `gx branch finish` invocation. +- Does not bypass host approval policy; it only moves the required submodule push into the existing finish command. diff --git a/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/specs/agent-codex-codex-task-2026-05-11-09-45/spec.md b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/specs/agent-codex-codex-task-2026-05-11-09-45/spec.md new file mode 100644 index 0000000..ad1a5c7 --- /dev/null +++ b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/specs/agent-codex-codex-task-2026-05-11-09-45/spec.md @@ -0,0 +1,16 @@ +## ADDED Requirements + +### Requirement: Finish pushes changed submodule branches before parent branch publication +When an agent branch updates a submodule gitlink, `gx branch finish` SHALL push the checked-out submodule branch that contains the gitlink commit before pushing or merging the parent repository branch. + +#### Scenario: Changed submodule branch is local-only +- **GIVEN** a parent agent branch points a submodule gitlink at a commit that exists only on a local submodule branch +- **WHEN** `gx branch finish` runs with push enabled +- **THEN** the submodule branch is pushed to the submodule remote before the parent branch is pushed or merged +- **AND** the parent finish flow continues only after that submodule push succeeds. + +#### Scenario: Changed submodule commit cannot be safely published +- **GIVEN** a parent agent branch points a submodule gitlink at a commit without a checked-out submodule branch that contains it +- **WHEN** `gx branch finish` runs with push enabled +- **THEN** Guardex reports the unsafe submodule state +- **AND** the parent branch is not pushed by the finish flow. diff --git a/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/tasks.md b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/tasks.md new file mode 100644 index 0000000..1f33876 --- /dev/null +++ b/openspec/changes/agent-codex-codex-task-2026-05-11-09-45/tasks.md @@ -0,0 +1,34 @@ +## Definition of Done + +This change is complete only when **all** of the following are true: + +- Every checkbox below is checked. +- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff. +- If any step blocks (test failure, conflict, ambiguous result), append a `BLOCKED:` line under section 4 explaining the blocker and **STOP**. Do not tick remaining cleanup boxes; do not silently skip the cleanup pipeline. + +## Handoff + +- Handoff: change=`agent-codex-codex-task-2026-05-11-09-45`; branch=`agent/codex/codex-task-2026-05-11-09-45`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`. +- Copy prompt: Continue `agent-codex-codex-task-2026-05-11-09-45` on branch `agent/codex/codex-task-2026-05-11-09-45`. Work inside the existing sandbox, review `openspec/changes/agent-codex-codex-task-2026-05-11-09-45/tasks.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent/codex/codex-task-2026-05-11-09-45 --base main --via-pr --wait-for-merge --cleanup`. + +## 1. Specification + +- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-codex-codex-task-2026-05-11-09-45`. +- [x] 1.2 Define normative requirements in `specs/agent-codex-codex-task-2026-05-11-09-45/spec.md`. + +## 2. Implementation + +- [x] 2.1 Implement scoped behavior changes. +- [x] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [x] 3.1 Run targeted project verification commands. +- [x] 3.2 Run `openspec validate agent-codex-codex-task-2026-05-11-09-45 --type change --strict`. +- [x] 3.3 Run `openspec validate --specs`. + +## 4. Cleanup (mandatory; run before claiming completion) + +- [ ] 4.1 Run the cleanup pipeline: `gx branch finish --branch agent/codex/codex-task-2026-05-11-09-45 --base main --via-pr --wait-for-merge --cleanup`. This handles commit -> push -> PR create -> merge wait -> worktree prune in one invocation. +- [ ] 4.2 Record the PR URL and final merge state (`MERGED`) in the completion handoff. +- [ ] 4.3 Confirm the sandbox worktree is gone (`git worktree list` no longer shows the agent path; `git branch -a` shows no surviving local/remote refs for the branch). diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index 178c642..593d906 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -382,6 +382,7 @@ merge_completed=0 merge_status="pr" direct_push_error="" pr_url="" +changed_submodule_push_done=0 cleanup() { if [[ -n "$integration_worktree" && -d "$integration_worktree" ]]; then @@ -718,6 +719,77 @@ maybe_auto_commit_parent_gitlink() { echo "[agent-branch-finish] Parent gitlink auto-committed '${subrepo_rel}' in ${super_root}." } +maybe_push_changed_submodule_branches() { + local base_ref="${1:-}" + local source_ref="${2:-}" + local changed_path="" + local gitlink_mode="" + local gitlink_sha="" + local submodule_dir="" + local branch_name="" + local candidate_branch="" + local remote_name="" + local push_output="" + + if [[ "$PUSH_ENABLED" -ne 1 || "$changed_submodule_push_done" -eq 1 ]]; then + return 0 + fi + changed_submodule_push_done=1 + if [[ -z "$base_ref" || -z "$source_ref" ]]; then + return 0 + fi + + while IFS= read -r changed_path; do + [[ -n "$changed_path" ]] || continue + + gitlink_mode="$(git -C "$source_worktree" ls-tree "$source_ref" -- "$changed_path" | awk 'NR == 1 { print $1 }')" + if [[ "$gitlink_mode" != "160000" ]]; then + continue + fi + gitlink_sha="$(git -C "$source_worktree" ls-tree "$source_ref" -- "$changed_path" | awk 'NR == 1 { print $3 }')" + if [[ -z "$gitlink_sha" ]]; then + continue + fi + + submodule_dir="${source_worktree}/${changed_path}" + if ! git -C "$submodule_dir" rev-parse --is-inside-work-tree >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' has no checked-out submodule at ${submodule_dir}; cannot auto-push submodule commit ${gitlink_sha}." >&2 + return 1 + fi + if ! git -C "$submodule_dir" cat-file -e "${gitlink_sha}^{commit}" >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' points at ${gitlink_sha}, but that commit is not present in ${submodule_dir}; cannot auto-push it." >&2 + return 1 + fi + + branch_name="$(git -C "$submodule_dir" rev-parse --abbrev-ref HEAD 2>/dev/null || true)" + if [[ -z "$branch_name" || "$branch_name" == "HEAD" ]] || ! git -C "$submodule_dir" merge-base --is-ancestor "$gitlink_sha" "$branch_name" >/dev/null 2>&1; then + candidate_branch="$(git -C "$submodule_dir" for-each-ref --contains "$gitlink_sha" --format='%(refname:short)' refs/heads | head -n 1)" + branch_name="$candidate_branch" + fi + if [[ -z "$branch_name" || "$branch_name" == "HEAD" ]]; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' points at ${gitlink_sha}, but no local submodule branch contains it; cannot choose a safe remote branch to push." >&2 + return 1 + fi + + remote_name="$(git -C "$submodule_dir" config --get "branch.${branch_name}.remote" || true)" + if [[ -z "$remote_name" ]]; then + remote_name="origin" + fi + if ! git -C "$submodule_dir" remote get-url "$remote_name" >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' branch '${branch_name}' has no usable remote '${remote_name}'; cannot auto-push submodule commit ${gitlink_sha}." >&2 + return 1 + fi + + if push_output="$(git -C "$submodule_dir" push -u "$remote_name" "${branch_name}:${branch_name}" 2>&1)"; then + echo "[agent-branch-finish] Pushed changed submodule '${changed_path}' branch '${branch_name}' to '${remote_name}' before parent finish." + else + echo "[agent-branch-finish] Changed submodule '${changed_path}' must be pushed before the parent branch can be finished." >&2 + [[ -n "$push_output" ]] && echo "$push_output" >&2 + return 1 + fi + done < <(git -C "$source_worktree" diff --name-only "$base_ref" "$source_ref" -- 2>/dev/null || true) +} + wait_for_pr_merge() { local deadline deadline=$(( $(date +%s) + WAIT_TIMEOUT_SECONDS )) @@ -788,6 +860,7 @@ run_pr_flow() { return 0 fi + maybe_push_changed_submodule_branches "$start_ref" "$SOURCE_BRANCH" git -C "$source_worktree" push -u origin "$SOURCE_BRANCH" pr_title="$(git -C "$repo_root" log -1 --pretty=%s "$SOURCE_BRANCH" 2>/dev/null || true)" @@ -836,6 +909,7 @@ run_pr_flow() { if [[ "$PUSH_ENABLED" -eq 1 ]]; then if [[ "$MERGE_MODE" != "pr" ]]; then + maybe_push_changed_submodule_branches "$start_ref" "$SOURCE_BRANCH" if ! direct_push_output="$(git -C "$integration_worktree" push origin "HEAD:${BASE_BRANCH}" 2>&1)"; then direct_push_error="$direct_push_output" merge_completed=0 diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index 178c642..593d906 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -382,6 +382,7 @@ merge_completed=0 merge_status="pr" direct_push_error="" pr_url="" +changed_submodule_push_done=0 cleanup() { if [[ -n "$integration_worktree" && -d "$integration_worktree" ]]; then @@ -718,6 +719,77 @@ maybe_auto_commit_parent_gitlink() { echo "[agent-branch-finish] Parent gitlink auto-committed '${subrepo_rel}' in ${super_root}." } +maybe_push_changed_submodule_branches() { + local base_ref="${1:-}" + local source_ref="${2:-}" + local changed_path="" + local gitlink_mode="" + local gitlink_sha="" + local submodule_dir="" + local branch_name="" + local candidate_branch="" + local remote_name="" + local push_output="" + + if [[ "$PUSH_ENABLED" -ne 1 || "$changed_submodule_push_done" -eq 1 ]]; then + return 0 + fi + changed_submodule_push_done=1 + if [[ -z "$base_ref" || -z "$source_ref" ]]; then + return 0 + fi + + while IFS= read -r changed_path; do + [[ -n "$changed_path" ]] || continue + + gitlink_mode="$(git -C "$source_worktree" ls-tree "$source_ref" -- "$changed_path" | awk 'NR == 1 { print $1 }')" + if [[ "$gitlink_mode" != "160000" ]]; then + continue + fi + gitlink_sha="$(git -C "$source_worktree" ls-tree "$source_ref" -- "$changed_path" | awk 'NR == 1 { print $3 }')" + if [[ -z "$gitlink_sha" ]]; then + continue + fi + + submodule_dir="${source_worktree}/${changed_path}" + if ! git -C "$submodule_dir" rev-parse --is-inside-work-tree >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' has no checked-out submodule at ${submodule_dir}; cannot auto-push submodule commit ${gitlink_sha}." >&2 + return 1 + fi + if ! git -C "$submodule_dir" cat-file -e "${gitlink_sha}^{commit}" >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' points at ${gitlink_sha}, but that commit is not present in ${submodule_dir}; cannot auto-push it." >&2 + return 1 + fi + + branch_name="$(git -C "$submodule_dir" rev-parse --abbrev-ref HEAD 2>/dev/null || true)" + if [[ -z "$branch_name" || "$branch_name" == "HEAD" ]] || ! git -C "$submodule_dir" merge-base --is-ancestor "$gitlink_sha" "$branch_name" >/dev/null 2>&1; then + candidate_branch="$(git -C "$submodule_dir" for-each-ref --contains "$gitlink_sha" --format='%(refname:short)' refs/heads | head -n 1)" + branch_name="$candidate_branch" + fi + if [[ -z "$branch_name" || "$branch_name" == "HEAD" ]]; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' points at ${gitlink_sha}, but no local submodule branch contains it; cannot choose a safe remote branch to push." >&2 + return 1 + fi + + remote_name="$(git -C "$submodule_dir" config --get "branch.${branch_name}.remote" || true)" + if [[ -z "$remote_name" ]]; then + remote_name="origin" + fi + if ! git -C "$submodule_dir" remote get-url "$remote_name" >/dev/null 2>&1; then + echo "[agent-branch-finish] Warning: changed gitlink '${changed_path}' branch '${branch_name}' has no usable remote '${remote_name}'; cannot auto-push submodule commit ${gitlink_sha}." >&2 + return 1 + fi + + if push_output="$(git -C "$submodule_dir" push -u "$remote_name" "${branch_name}:${branch_name}" 2>&1)"; then + echo "[agent-branch-finish] Pushed changed submodule '${changed_path}' branch '${branch_name}' to '${remote_name}' before parent finish." + else + echo "[agent-branch-finish] Changed submodule '${changed_path}' must be pushed before the parent branch can be finished." >&2 + [[ -n "$push_output" ]] && echo "$push_output" >&2 + return 1 + fi + done < <(git -C "$source_worktree" diff --name-only "$base_ref" "$source_ref" -- 2>/dev/null || true) +} + wait_for_pr_merge() { local deadline deadline=$(( $(date +%s) + WAIT_TIMEOUT_SECONDS )) @@ -788,6 +860,7 @@ run_pr_flow() { return 0 fi + maybe_push_changed_submodule_branches "$start_ref" "$SOURCE_BRANCH" git -C "$source_worktree" push -u origin "$SOURCE_BRANCH" pr_title="$(git -C "$repo_root" log -1 --pretty=%s "$SOURCE_BRANCH" 2>/dev/null || true)" @@ -836,6 +909,7 @@ run_pr_flow() { if [[ "$PUSH_ENABLED" -eq 1 ]]; then if [[ "$MERGE_MODE" != "pr" ]]; then + maybe_push_changed_submodule_branches "$start_ref" "$SOURCE_BRANCH" if ! direct_push_output="$(git -C "$integration_worktree" push origin "HEAD:${BASE_BRANCH}" 2>&1)"; then direct_push_error="$direct_push_output" merge_completed=0 diff --git a/test/finish.test.js b/test/finish.test.js index bc7029b..d7bb963 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -231,6 +231,70 @@ test('agent-branch-finish auto-commits parent gitlink after nested repo finish', assert.equal(parentStatus.stdout.trim(), '', 'parent gitlink should be committed cleanly'); }); +test('agent-branch-finish pushes changed submodule branch before parent finish', () => { + const parentDir = initRepoOnBranch('main'); + seedCommit(parentDir); + attachOriginRemoteForBranch(parentDir, 'main'); + + const childDir = path.join(parentDir, 'apps', 'storefront'); + fs.mkdirSync(childDir, { recursive: true }); + let result = runCmd('git', ['init', '-b', 'main'], childDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + configureGitIdentity(childDir); + fs.writeFileSync(path.join(childDir, 'app.css'), '@tailwind utilities;\n', 'utf8'); + seedCommit(childDir); + const childOrigin = attachOriginRemoteForBranch(childDir, 'main'); + result = runCmd('git', ['push', 'origin', 'main'], childDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + const childMainCommit = runCmd('git', ['rev-parse', 'HEAD'], childDir).stdout.trim(); + + result = runCmd('git', ['update-index', '--add', '--cacheinfo', '160000', childMainCommit, 'apps/storefront'], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'track storefront submodule'], parentDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', 'origin', 'main'], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const childBranch = 'agent/codex/fix-css-import-order-2026-05-11'; + result = runCmd('git', ['checkout', '-b', childBranch], childDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + fs.writeFileSync(path.join(childDir, 'app.css'), '@import url("https://fonts.example/test.css");\n@tailwind utilities;\n', 'utf8'); + result = runCmd('git', ['add', 'app.css'], childDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'Keep font imports ahead of Tailwind output'], childDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + const childCommit = runCmd('git', ['rev-parse', 'HEAD'], childDir).stdout.trim(); + + const parentBranch = 'agent/codex/point-storefront-css-fix'; + result = runCmd('git', ['checkout', '-b', parentBranch], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['update-index', '--cacheinfo', '160000', childCommit, 'apps/storefront'], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'Point storefront at CSS import-order fix'], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runBranchFinish([ + '--branch', + parentBranch, + '--base', + 'main', + '--direct-only', + '--no-cleanup', + ], parentDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Pushed changed submodule 'apps\/storefront' branch 'agent\/codex\/fix-css-import-order-2026-05-11' to 'origin' before parent finish/); + + const remoteChildHead = runCmd('git', ['ls-remote', childOrigin, `refs/heads/${childBranch}`], parentDir); + assert.equal(remoteChildHead.status, 0, remoteChildHead.stderr || remoteChildHead.stdout); + assert.match(remoteChildHead.stdout.trim(), new RegExp(`^${childCommit}\\s+refs/heads/${escapeRegexLiteral(childBranch)}$`)); + + const parentMainGitlink = runCmd('git', ['ls-tree', 'origin/main', '--', 'apps/storefront'], parentDir); + assert.equal(parentMainGitlink.status, 0, parentMainGitlink.stderr || parentMainGitlink.stdout); + assert.match(parentMainGitlink.stdout, new RegExp(`160000 commit ${childCommit}\\s+apps/storefront`)); +}); + test('agent-branch-finish auto-syncs source branch when behind origin/dev', () => { const repoDir = initRepo();