Keep submodule commits publishable during finish (#544)

NagyVikt · NagyVikt · web-flow · commit e8e7651bf03f · 2026-05-11T09:56:55.000+02:00
Guardex finish already owns parent branch publication, but parent repos can update a submodule gitlink before the nested repo branch is available remotely. The finish path now detects changed gitlinks, pushes the checked-out submodule branch that contains the referenced commit, and stops before parent publication when the nested commit cannot be safely published.

Constraint: Codex host approval policy cannot be bypassed; approval must stay on the gx finish command surface.

Rejected: Tell agents to run standalone git push in submodules | repeats approval prompts and can strand parent branches with local-only gitlinks

Confidence: high

Scope-risk: moderate

Directive: Keep scripts/agent-branch-finish.sh and templates/scripts/agent-branch-finish.sh in sync for finish-flow changes.

Tested: bash -n scripts/agent-branch-finish.sh templates/scripts/agent-branch-finish.sh

Tested: node --test --test-name-pattern 'pushes changed submodule branch' test/finish.test.js

Tested: openspec validate agent-codex-codex-task-2026-05-11-09-45 --type change --strict

Tested: openspec validate --specs

Not-tested: Full test/finish.test.js remains blocked by existing expectation drift in unrelated subtests.

Co-authored-by: NagyVikt &lt;nagy.viktordp@gmail.com&gt;
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+schema: spec-driven`
	`2`	`+created: 2026-05-11`