Skip to content

Commit f1cc0ea

Browse files
NagyViktNagyViktclaude
authored
fix(gx): deploy PR #546 fix to runtime via templates/scripts/ + add --auto-resolve=full submodule pointer resolver (#547)
PR #546 only edited scripts/agent-branch-{start,finish}.sh, but the gx CLI invokes templates/scripts/agent-branch-{start,finish}.sh at runtime (src/context.js:247). The leak fix and --auto-resolve=safe flag never deployed in practice. This change ports the full PR #546 surface to templates/scripts/ (auto-transfer pathspec excludes, --no-transfer flag, --auto-resolve[=none|safe] with state-file allowlist, gx locks claim integration, single merge commit). On top of that, adds a new --auto-resolve=full mode that extends the resolver to submodule pointer conflicts via strict-ancestor (fast-forward) detection. The submodule clone used for ancestry is selected in order: the checked-out worktree, the cached .git/modules/<name> clone, or a temporary bare clone of the .gitmodules URL (cleaned up via RETURN trap). Divergent histories refuse; the script aborts with the unresolved list. Both copies (scripts/ and templates/scripts/) are kept in sync in the touched regions. The structural drift between the two directories is acknowledged in the OpenSpec change but tracked as a separate follow-up. OpenSpec change scaffolded at openspec/changes/agent-claude-submodule-pointer-conflict-resolver-2026-05-11-10-34/. Co-authored-by: NagyVikt <nagy.viktordp@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 896cabf commit f1cc0ea

6 files changed

Lines changed: 638 additions & 43 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# Proposal: deploy PR #546's fix to runtime via `templates/scripts/` + add `--auto-resolve=full` submodule pointer resolver
2+
3+
## Problem
4+
5+
Two issues, both surfaced by trying to use PR #546 in anger.
6+
7+
### 1. PR #546 did not actually deploy at runtime
8+
9+
The Guardex Node CLI invokes `templates/scripts/agent-branch-{start,finish}.sh` (see `src/context.js:247`), not `scripts/agent-branch-{start,finish}.sh`. Both copies are tracked in git, drift independently, and PR #546 modified only the `scripts/` copy. At runtime the leak the PR claimed to fix is still live.
10+
11+
Reproduction: in a worktree containing PR #546's merge commit, run `gx branch start --no-transfer ...`. The CLI errors with `Unknown option: --no-transfer` from `templates/scripts/agent-branch-start.sh`. The fix needs to land in the `templates/` copy.
12+
13+
### 2. Submodule pointer conflicts remain unresolved
14+
15+
PR #546's `--auto-resolve=safe` only matches state-file globs. Real-world PR conflicts (see the user's downstream `Webu-PRO/lifted.sk-backend` PR #3) include submodule pointers (`apps/backend`, `apps/storefront`), which `safe` mode correctly refuses. There is no auto-resolution path for them today; the PR sits blocked.
16+
17+
## Approach
18+
19+
### 1. Port Phase 1+2 to `templates/scripts/`
20+
21+
Apply PR #546's auto-transfer exclude + `--no-transfer` flag set to `templates/scripts/agent-branch-start.sh`. Apply PR #546's `--auto-resolve=safe` resolver (with `gx locks claim` integration and pre-commit-hook compliance) to `templates/scripts/agent-branch-finish.sh`. After this PR, both `scripts/` and `templates/scripts/` carry the same fix.
22+
23+
### 2. Add `--auto-resolve=full` mode (submodule pointer resolver)
24+
25+
Extend the existing `--auto-resolve` enum from `{none, safe}` to `{none, safe, full}`. `full` keeps safe's state-file behavior and adds a submodule-pointer-only resolver:
26+
27+
- Detects whether a conflict path is a registered submodule via `.gitmodules`.
28+
- Reads the three index stages (base/ours/theirs) via `git ls-files -u`.
29+
- Determines ancestry using a working clone of the submodule. The clone is selected in order:
30+
1. The checked-out submodule worktree (no network).
31+
2. The cached internal clone at `.git/modules/<path>` (no network, present even after `git submodule deinit`).
32+
3. A temporary bare clone of the submodule URL from `.gitmodules` (network, cleaned up via RETURN trap).
33+
- If one SHA is a strict ancestor of the other, picks the descendant and writes it via `git update-index --cacheinfo 160000,<sha>,<path>`. Otherwise refuses.
34+
- Claims the resolved submodule paths via `gx locks claim` before committing, matching the state-file flow.
35+
36+
### What's still out of scope
37+
38+
- Auto-fetching from remotes the user does not have credentials for.
39+
- Resolving divergent submodule histories (refuse-by-design).
40+
- AI-driven code-conflict resolution.
41+
42+
## Rationale
43+
44+
- Modern git already auto-fast-forwards submodule pointers when both SHAs are reachable locally — so the resolver is most useful when no clone is present (shallow CI agents, deinit'd submodules, GitHub-server-side merge attempts). Path 3 (temp bare clone) closes the gap.
45+
- Strict ancestry-only matches the user's PR #3 shape: GitHub flags the conflict because *it* doesn't have a clone; once a clone exists, ancestry is trivially decidable.
46+
- Keeping the resolver opt-in (`--auto-resolve=full`) preserves the safe-by-default invariant from PR #546.
47+
48+
## Compatibility / Migration
49+
50+
- The `--auto-resolve=safe` value continues to behave exactly as in PR #546 (state files only).
51+
- New value `--auto-resolve=full` is opt-in; default is still `none`.
52+
- No env vars renamed. New default for `AUTO_RESOLVE_SAFE_GLOBS` remains unchanged.
53+
- The `templates/` copy gains the same flags as the `scripts/` copy. No drift introduced in the touched regions.
54+
55+
## Risks / Known follow-ups
56+
57+
- `templates/scripts/` and `scripts/` continue to drift in untouched regions. This PR keeps the touched regions in sync but does not solve the structural duplication. Recommended follow-up: a parity-check CI script or a build step that makes one the canonical source. Track as a separate change.
58+
- The temp bare-clone path executes `git clone` against the submodule URL — for HTTPS submodules the user's existing credentials carry over; for `file://` test fixtures, callers need `protocol.file.allow=always`.
59+
- The trap-based cleanup of the temp clone runs on function return; if the shell is killed mid-function the temp dir leaks. Acceptable given the size and the mktemp prefix `gx-submod-resolve-`.
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Spec Delta: gitguardex-agent-lifecycle
2+
3+
## ADDED Requirements
4+
5+
### Requirement: PR #546 lifecycle flags MUST land in the runtime-invoked template scripts
6+
7+
The gx CLI invokes `templates/scripts/agent-branch-start.sh` and `templates/scripts/agent-branch-finish.sh` at runtime (see `src/context.js`). All flags, env vars, and behavior introduced by PR #546 (`--no-transfer`, `--transfer`, `--transfer-exclude`, `GUARDEX_AUTO_TRANSFER*`, `--auto-resolve[=none|safe]`, `--no-auto-resolve`, `GUARDEX_FINISH_AUTO_RESOLVE*`, `path_matches_auto_resolve_safe_glob`, and the conflict-walk + `gx locks claim` + single-merge-commit flow) MUST be present in the `templates/scripts/` copies. The script copies in `scripts/` are kept in sync as a development convenience but the `templates/` copies are the source of truth at runtime.
8+
9+
#### Scenario: Runtime invocation through gx honors the `--no-transfer` flag
10+
11+
- **GIVEN** `gx branch start --no-transfer "<task>" "<agent>"` is invoked
12+
- **WHEN** the CLI dispatches to `templates/scripts/agent-branch-start.sh`
13+
- **THEN** the script accepts the flag (does not exit with `Unknown option: --no-transfer`)
14+
- **AND** no auto-transfer stash is created regardless of dirty state on the protected branch
15+
16+
#### Scenario: Runtime invocation through gx honors the auto-transfer exclude list
17+
18+
- **GIVEN** `gx branch start` is invoked from a repo containing the latest gitguardex tree
19+
- **AND** the user is on the protected base branch with an untracked file under `.omc/`
20+
- **WHEN** the CLI dispatches to its bundled `templates/scripts/agent-branch-start.sh`
21+
- **THEN** the `.omc/...` file remains on the protected branch and does not appear in the new agent worktree
22+
23+
#### Scenario: `--auto-resolve=safe` is accepted by the bundled template script
24+
25+
- **GIVEN** the runtime path `templates/scripts/agent-branch-finish.sh` is invoked with `--auto-resolve=safe`
26+
- **THEN** the flag parses without `Unknown argument`
27+
- **AND** the state-file allowlist + `gx locks claim` + single-merge-commit flow execute as in PR #546
28+
29+
### Requirement: `gx branch finish` MUST accept `--auto-resolve=full` and resolve fast-forward-able submodule pointer conflicts
30+
31+
When `--auto-resolve=full` is set, `gx branch finish` MUST handle conflicts on registered submodule paths in addition to the state-file allowlist. For each submodule pointer conflict, the script MUST determine the merge direction by checking ancestry of the three index stages against a working clone of the submodule, picking the strict descendant when one exists. The script MUST refuse and abort when the submodule histories are divergent, when the submodule URL is missing, or when no clone is reachable.
32+
33+
#### Scenario: Submodule pointer conflict, one side is strict ancestor of the other
34+
35+
- **GIVEN** the agent branch and `origin/<base>` disagree on a registered submodule's gitlink
36+
- **AND** the submodule's `.git/modules/<path>` cached clone contains both SHAs
37+
- **AND** the agent-branch SHA is a strict ancestor of the base-branch SHA (or vice versa)
38+
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
39+
- **THEN** the resolver writes the descendant SHA via `git update-index --cacheinfo 160000,<sha>,<path>`
40+
- **AND** claims the resolved submodule path via `gx locks claim`
41+
- **AND** completes the merge with one commit
42+
- **AND** the finish flow proceeds into the normal push/PR phase
43+
44+
#### Scenario: Submodule pointer conflict, divergent histories
45+
46+
- **GIVEN** the agent branch and `origin/<base>` disagree on a submodule's gitlink
47+
- **AND** neither SHA is an ancestor of the other in any reachable clone
48+
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
49+
- **THEN** the resolver returns non-zero for that path
50+
- **AND** the script aborts the merge and exits non-zero
51+
- **AND** the unresolved submodule path is listed in the abort message
52+
53+
#### Scenario: Submodule conflict, no clone available locally
54+
55+
- **GIVEN** a submodule pointer conflict on a path with no checked-out worktree and no `.git/modules/<path>` cache
56+
- **AND** `.gitmodules` records a usable URL for the submodule
57+
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
58+
- **THEN** the resolver creates a temporary bare clone via `git clone --bare <url>`
59+
- **AND** uses it to determine ancestry
60+
- **AND** removes the temp clone before returning
61+
62+
#### Scenario: `--auto-resolve=safe` refuses submodule conflicts
63+
64+
- **GIVEN** a submodule pointer conflict on a registered submodule path
65+
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=safe ...` runs
66+
- **THEN** the submodule conflict is classified as unresolved
67+
- **AND** the abort message includes the hint "Submodule pointer auto-resolve requires --auto-resolve=full"
68+
69+
### Requirement: `--auto-resolve` MUST validate `full` as an accepted mode
70+
71+
`gx branch finish` MUST accept exactly three `--auto-resolve` values: `none`, `safe`, `full`. Any other value MUST cause the script to exit non-zero before performing any merge or push.
72+
73+
#### Scenario: Invalid mode is rejected
74+
75+
- **GIVEN** the user invokes `gx branch finish --auto-resolve=aggressive ...`
76+
- **THEN** the script exits non-zero with `Invalid --auto-resolve value: aggressive (expected none|safe|full)`
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Tasks
2+
3+
## 1. Spec
4+
5+
- [x] 1.1 Capture problem + approach in `proposal.md` (includes drift finding).
6+
- [x] 1.2 Add ADDED/MODIFIED requirements + scenarios to `specs/gitguardex-agent-lifecycle/spec.md`.
7+
8+
## 2. Port Phase 1+2 to `templates/scripts/`
9+
10+
- [x] 2.1 `templates/scripts/agent-branch-start.sh`: add `AUTO_TRANSFER_ENABLED_RAW` / `AUTO_TRANSFER_EXCLUDE_RAW` env defaults.
11+
- [x] 2.2 `templates/scripts/agent-branch-start.sh`: add `--no-transfer`, `--transfer`, `--transfer-exclude` flags.
12+
- [x] 2.3 `templates/scripts/agent-branch-start.sh`: replace auto-transfer block with pathspec-exclude variant + log lines.
13+
- [x] 2.4 `templates/scripts/agent-branch-finish.sh`: add `AUTO_RESOLVE_MODE_RAW` / `AUTO_RESOLVE_SAFE_GLOBS_RAW` env defaults.
14+
- [x] 2.5 `templates/scripts/agent-branch-finish.sh`: add `--auto-resolve` / `--auto-resolve=*` / `--no-auto-resolve` flags + validation case for `none|safe|full`.
15+
- [x] 2.6 `templates/scripts/agent-branch-finish.sh`: add `path_matches_auto_resolve_safe_glob` helper.
16+
- [x] 2.7 `templates/scripts/agent-branch-finish.sh`: rewrite preflight conflict block with safe + full resolver + `gx locks claim` integration.
17+
18+
## 3. Phase 3 — `--auto-resolve=full` submodule pointer resolver
19+
20+
- [x] 3.1 `scripts/agent-branch-finish.sh`: extend `--auto-resolve` arg regex + validation to accept `full`.
21+
- [x] 3.2 `scripts/agent-branch-finish.sh`: add `try_resolve_submodule_pointer_conflict` helper with 3-source clone lookup (worktree → `.git/modules/<name>` → temp bare clone).
22+
- [x] 3.3 `scripts/agent-branch-finish.sh`: integrate submodule resolver into the conflict-walk loop (only when mode == `full`).
23+
- [x] 3.4 `scripts/agent-branch-finish.sh`: update commit message + summary lines for state + submodule counts.
24+
- [x] 3.5 Mirror 3.1–3.4 into `templates/scripts/agent-branch-finish.sh`.
25+
26+
## 4. Tests / Verification
27+
28+
- [x] 4.1 `bash -n` clean on all four edited scripts.
29+
- [x] 4.2 Arg-parser smoke: `bash <finish> --auto-resolve=full --branch x` parses without "Unknown argument"; `--auto-resolve=bogus` is rejected with the right enum error.
30+
- [x] 4.3 Manual reproduction of the drift bug: confirm `gx branch start --no-transfer ...` now works against `templates/scripts/` (verified by dogfooding `--no-transfer` in this session to start this branch).
31+
32+
## 5. Cleanup
33+
34+
- [ ] 5.1 Commit on `agent/claude/submodule-pointer-conflict-resolver-2026-05-11-10-34`.
35+
- [ ] 5.2 Push branch and open PR against `main`.
36+
- [ ] 5.3 PR merged (record URL + MERGED state).
37+
- [ ] 5.4 Sandbox worktree pruned via `gx branch finish --cleanup`.

0 commit comments

Comments
 (0)