Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Proposal: deploy PR #546's fix to runtime via `templates/scripts/` + add `--auto-resolve=full` submodule pointer resolver

## Problem

Two issues, both surfaced by trying to use PR #546 in anger.

### 1. PR #546 did not actually deploy at runtime

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.

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.

### 2. Submodule pointer conflicts remain unresolved

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.

## Approach

### 1. Port Phase 1+2 to `templates/scripts/`

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.

### 2. Add `--auto-resolve=full` mode (submodule pointer resolver)

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:

- Detects whether a conflict path is a registered submodule via `.gitmodules`.
- Reads the three index stages (base/ours/theirs) via `git ls-files -u`.
- Determines ancestry using a working clone of the submodule. The clone is selected in order:
1. The checked-out submodule worktree (no network).
2. The cached internal clone at `.git/modules/<path>` (no network, present even after `git submodule deinit`).
3. A temporary bare clone of the submodule URL from `.gitmodules` (network, cleaned up via RETURN trap).
- 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.
- Claims the resolved submodule paths via `gx locks claim` before committing, matching the state-file flow.

### What's still out of scope

- Auto-fetching from remotes the user does not have credentials for.
- Resolving divergent submodule histories (refuse-by-design).
- AI-driven code-conflict resolution.

## Rationale

- 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.
- 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.
- Keeping the resolver opt-in (`--auto-resolve=full`) preserves the safe-by-default invariant from PR #546.

## Compatibility / Migration

- The `--auto-resolve=safe` value continues to behave exactly as in PR #546 (state files only).
- New value `--auto-resolve=full` is opt-in; default is still `none`.
- No env vars renamed. New default for `AUTO_RESOLVE_SAFE_GLOBS` remains unchanged.
- The `templates/` copy gains the same flags as the `scripts/` copy. No drift introduced in the touched regions.

## Risks / Known follow-ups

- `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.
- 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`.
- 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-`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Spec Delta: gitguardex-agent-lifecycle

## ADDED Requirements

### Requirement: PR #546 lifecycle flags MUST land in the runtime-invoked template scripts

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.

#### Scenario: Runtime invocation through gx honors the `--no-transfer` flag

- **GIVEN** `gx branch start --no-transfer "<task>" "<agent>"` is invoked
- **WHEN** the CLI dispatches to `templates/scripts/agent-branch-start.sh`
- **THEN** the script accepts the flag (does not exit with `Unknown option: --no-transfer`)
- **AND** no auto-transfer stash is created regardless of dirty state on the protected branch

#### Scenario: Runtime invocation through gx honors the auto-transfer exclude list

- **GIVEN** `gx branch start` is invoked from a repo containing the latest gitguardex tree
- **AND** the user is on the protected base branch with an untracked file under `.omc/`
- **WHEN** the CLI dispatches to its bundled `templates/scripts/agent-branch-start.sh`
- **THEN** the `.omc/...` file remains on the protected branch and does not appear in the new agent worktree

#### Scenario: `--auto-resolve=safe` is accepted by the bundled template script

- **GIVEN** the runtime path `templates/scripts/agent-branch-finish.sh` is invoked with `--auto-resolve=safe`
- **THEN** the flag parses without `Unknown argument`
- **AND** the state-file allowlist + `gx locks claim` + single-merge-commit flow execute as in PR #546

### Requirement: `gx branch finish` MUST accept `--auto-resolve=full` and resolve fast-forward-able submodule pointer conflicts

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.

#### Scenario: Submodule pointer conflict, one side is strict ancestor of the other

- **GIVEN** the agent branch and `origin/<base>` disagree on a registered submodule's gitlink
- **AND** the submodule's `.git/modules/<path>` cached clone contains both SHAs
- **AND** the agent-branch SHA is a strict ancestor of the base-branch SHA (or vice versa)
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
- **THEN** the resolver writes the descendant SHA via `git update-index --cacheinfo 160000,<sha>,<path>`
- **AND** claims the resolved submodule path via `gx locks claim`
- **AND** completes the merge with one commit
- **AND** the finish flow proceeds into the normal push/PR phase

#### Scenario: Submodule pointer conflict, divergent histories

- **GIVEN** the agent branch and `origin/<base>` disagree on a submodule's gitlink
- **AND** neither SHA is an ancestor of the other in any reachable clone
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
- **THEN** the resolver returns non-zero for that path
- **AND** the script aborts the merge and exits non-zero
- **AND** the unresolved submodule path is listed in the abort message

#### Scenario: Submodule conflict, no clone available locally

- **GIVEN** a submodule pointer conflict on a path with no checked-out worktree and no `.git/modules/<path>` cache
- **AND** `.gitmodules` records a usable URL for the submodule
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=full ...` runs
- **THEN** the resolver creates a temporary bare clone via `git clone --bare <url>`
- **AND** uses it to determine ancestry
- **AND** removes the temp clone before returning

#### Scenario: `--auto-resolve=safe` refuses submodule conflicts

- **GIVEN** a submodule pointer conflict on a registered submodule path
- **WHEN** `gx branch finish --branch <branch> --auto-resolve=safe ...` runs
- **THEN** the submodule conflict is classified as unresolved
- **AND** the abort message includes the hint "Submodule pointer auto-resolve requires --auto-resolve=full"

### Requirement: `--auto-resolve` MUST validate `full` as an accepted mode

`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.

#### Scenario: Invalid mode is rejected

- **GIVEN** the user invokes `gx branch finish --auto-resolve=aggressive ...`
- **THEN** the script exits non-zero with `Invalid --auto-resolve value: aggressive (expected none|safe|full)`
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Tasks

## 1. Spec

- [x] 1.1 Capture problem + approach in `proposal.md` (includes drift finding).
- [x] 1.2 Add ADDED/MODIFIED requirements + scenarios to `specs/gitguardex-agent-lifecycle/spec.md`.

## 2. Port Phase 1+2 to `templates/scripts/`

- [x] 2.1 `templates/scripts/agent-branch-start.sh`: add `AUTO_TRANSFER_ENABLED_RAW` / `AUTO_TRANSFER_EXCLUDE_RAW` env defaults.
- [x] 2.2 `templates/scripts/agent-branch-start.sh`: add `--no-transfer`, `--transfer`, `--transfer-exclude` flags.
- [x] 2.3 `templates/scripts/agent-branch-start.sh`: replace auto-transfer block with pathspec-exclude variant + log lines.
- [x] 2.4 `templates/scripts/agent-branch-finish.sh`: add `AUTO_RESOLVE_MODE_RAW` / `AUTO_RESOLVE_SAFE_GLOBS_RAW` env defaults.
- [x] 2.5 `templates/scripts/agent-branch-finish.sh`: add `--auto-resolve` / `--auto-resolve=*` / `--no-auto-resolve` flags + validation case for `none|safe|full`.
- [x] 2.6 `templates/scripts/agent-branch-finish.sh`: add `path_matches_auto_resolve_safe_glob` helper.
- [x] 2.7 `templates/scripts/agent-branch-finish.sh`: rewrite preflight conflict block with safe + full resolver + `gx locks claim` integration.

## 3. Phase 3 — `--auto-resolve=full` submodule pointer resolver

- [x] 3.1 `scripts/agent-branch-finish.sh`: extend `--auto-resolve` arg regex + validation to accept `full`.
- [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).
- [x] 3.3 `scripts/agent-branch-finish.sh`: integrate submodule resolver into the conflict-walk loop (only when mode == `full`).
- [x] 3.4 `scripts/agent-branch-finish.sh`: update commit message + summary lines for state + submodule counts.
- [x] 3.5 Mirror 3.1–3.4 into `templates/scripts/agent-branch-finish.sh`.

## 4. Tests / Verification

- [x] 4.1 `bash -n` clean on all four edited scripts.
- [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.
- [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).

## 5. Cleanup

- [ ] 5.1 Commit on `agent/claude/submodule-pointer-conflict-resolver-2026-05-11-10-34`.
- [ ] 5.2 Push branch and open PR against `main`.
- [ ] 5.3 PR merged (record URL + MERGED state).
- [ ] 5.4 Sandbox worktree pruned via `gx branch finish --cleanup`.
Loading
Loading