diff --git a/commands/do/pr-better.md b/commands/do/pr-better.md index c57a52a..719a5c6 100644 --- a/commands/do/pr-better.md +++ b/commands/do/pr-better.md @@ -78,7 +78,7 @@ The only Phase 7-equivalent housekeeping that applies: After Phase A leaves all fixes committed on `{CURRENT_BRANCH}`, hand off to the workflow defined in `~/.claude/commands/do/pr.md`, forwarding all five review flags extracted in argument forwarding — `REVIEW_AGENT_ARG` (the `--review-with ` pair, possibly comma-separated), `REVIEW_STOP_ARG` (the stop-mode flag, if any), `REVIEW_MODE_ARG` (the `--review-mode ` pair, if passed), `REVIEWER_APPLIES_ARG` (the `--reviewer-applies` token, if passed), and `REVIEW_ITERATIONS_ARG` (the `--review-iterations ` pair, if passed) — so the chosen reviewer(s), stop-mode, dispatch mode (series/parallel), editing mode, and copilot iteration cap all run on the combined PR: 1. **Detect branches** — already done in pre-flight, reuse those values -2. **Commit and push** — commit any remaining staged changes (e.g., the PLAN.md update), then run `do:pr`'s "Commit and Push" step verbatim: fast-forward the **local** `{default_branch}` ref to origin and rebase the branch onto it (`git fetch origin {default_branch}:{default_branch} && git rebase {default_branch}`, aborting and surfacing conflicts rather than guessing) so the reviewers below — which diff against the local `{default_branch}` — see a current base, then `git push -u origin {current_branch}` (`--force-with-lease` if the rebase rewrote pushed history) +2. **Commit and push** — commit any remaining staged changes (e.g., the PLAN.md update), then run `do:pr`'s "Commit and Push" step verbatim: fast-forward the **local** `{default_branch}` ref to origin and rebase the branch onto it (`git fetch origin {default_branch}:{default_branch} && git rebase {default_branch}`), resolving and continuing through conflicts with [lib/rebase-conflict-resolution.md](../../lib/rebase-conflict-resolution.md), including regeneration of generated artifacts from their canonical inputs, so the reviewers below — which diff against the local `{default_branch}` — see a current base; then push using `do:pr`'s upstream-derived rules (`--force-with-lease` if the rebase rewrote pushed history). A rebase conflict is not a handoff or stop condition. 3. **Local Code Review (REQUIRED GATE)** — run the full review gate from `do:pr`. The do:better Phase 4b internal review covered the worktree diff against the default branch, but the do:pr review gate also covers any prior commits on the feature branch that predate this run. Do not skip it. 4. **Open the PR** — create a single PR with a description that summarizes both: - The original feature work on the branch (from prior commits) diff --git a/commands/do/pr.md b/commands/do/pr.md index d966da8..8de8c86 100644 --- a/commands/do/pr.md +++ b/commands/do/pr.md @@ -86,7 +86,7 @@ Print: `PR flow: {current_branch} → {default_branch}` - **Sync the branch onto the latest `origin/{default_branch}` first.** Reviewers below diff the branch against `{REVIEW_BASE}` (`git diff {REVIEW_BASE}...HEAD`) — the **local** `{default_branch}` ref normally, `origin/{default_branch}` on the linked-worktree fallback below — anchored on the merge-base. If `origin/{default_branch}` moved forward since this branch was cut and the branch was never rebased, reviewers evaluate against a stale base and flag unrelated changes that landed on the default branch outside this branch's work. Rebase to eliminate that noise: - `git fetch origin {default_branch}:{default_branch}` to fast-forward the **local** `{default_branch}` ref (and its remote-tracking ref) to match origin. A plain `git fetch origin {default_branch}` only moves the remote-tracking ref — the reviewers diff against the *local* ref, so it must be the one advanced, or the merge-base stays stale and the noise remains. This works because the flow already moved you to a feature branch (Detect Branches step 3), so the default branch isn't checked out **here**. **In a linked worktree it can still fail** — git's refusal is repo-wide, so `fatal: refusing to fetch into branch 'refs/heads/{default_branch}' checked out at …` means the *parent* repo holds it, which is the normal state when `/do:next` or a claim flow invoked `/do:pr`. That is not a divergence and not a reason to stop: fall back to `git fetch origin {default_branch}` and rebase onto the remote-tracking ref (`git rebase origin/{default_branch}`), and have the reviewers diff `origin/{default_branch}...HEAD` — the local ref is the parent repo's to advance, and its cleanup phase does that. **Record the resolved base as `{REVIEW_BASE}`** — `{default_branch}` normally, `origin/{default_branch}` on this fallback — and pass it to the review loops as their `{BASE_BRANCH}` input; that is the only name they read, so a fallback that isn't recorded under it leaves them diffing the stale local ref anyway. If your local `{default_branch}` has diverged from origin and the fetch can't fast-forward it (unusual), surface that and stop rather than forcing it. - `git rebase {default_branch}` to replay this branch's commits on top of the now-current default branch. - - If the rebase hits conflicts, **abort** (`git rebase --abort`) and stop — print the conflicting files and ask the user to resolve them, rather than guessing at a merge. Do not proceed to review against a half-rebased tree. + - If the rebase hits conflicts, **resolve them and continue the rebase**. Do not abort or stop merely because conflicts exist. Read and follow [lib/rebase-conflict-resolution.md](../../lib/rebase-conflict-resolution.md): inspect the replayed commit and both sides, resolve human-authored sources semantically, regenerate generated artifacts from their canonical inputs (for example `server/lib/apiRouteCatalog.generated.json`), run the focused checks, stage the resolution, and repeat `git rebase --continue` until complete. Only the playbook's last-resort, evidence-backed ambiguity may abort the rebase; a generated-file conflict alone never qualifies. - After a clean rebase the branch's merge-base with `{REVIEW_BASE}` is current, so `git diff {REVIEW_BASE}...HEAD` shows only this branch's own changes. - Push the branch to remote (use `--force-with-lease` on any of these if the rebase above rewrote already-pushed history; never a bare `--force`). **Which form depends on whether the branch's upstream names a remote** — `-u` *rewrites* `branch..remote`/`.merge`, so using it unconditionally would re-point an existing upstream at `origin/{current_branch}` and defeat the config-derived guard under "Open the PR" at its source. Discriminate on `branch..remote`, **not** on whether `@{u}` resolves: a branch tracking a *local* ref (`branch..remote=.`, what `git branch --set-upstream-to=main` produces) resolves `@{u}` perfectly well, so an `@{u}`-based test would route it into the second case and push into the local repository: @@ -169,7 +169,7 @@ This phase drives the **multi-reviewer wrapper** (defined under "Reviewer loop b The `"$PUSH_REMOTE" = "."` guard is **load-bearing, not defensive**: on a local upstream `@{u}` resolves fine, so the no-upstream carve-out below never fires, and an unguarded push runs `git push . HEAD:refs/heads/main` — which silently fast-forwards the *local* default branch onto this branch's HEAD, exits 0, and leaves `@{u}..HEAD` empty. The assertion would then report the branch "reached the remote" and open a PR for a branch that was never pushed to any remote at all. - Never a bare `git push`, which under `push.default=matching` fans out to every same-named local branch, and never `git push origin {current_branch}`, which hardcodes the *local* branch name as the destination: on a branch whose upstream is named differently (or lives on another remote) that pushes a spurious remote branch, leaves the real PR head stale, and `@{u}..HEAD` is still non-empty afterward while the push itself "succeeded". On a non-fast-forward, retry once behind `git pull --rebase --autostash` — and if that rebase **conflicts, abort it** (`git rebase --abort 2>/dev/null`) before reporting the failure, exactly as `lib/multi-reviewer-loop.md` step 5 does. `/do:pr` is invoked programmatically by `/do:next` and `/do:pr-better`, so a branch left detached mid-rebase becomes the tree their next step runs against — and a later `@{u}` assertion silently skips on a detached HEAD. Then create the PR. **If the push still fails after that one retry, do NOT create the PR** — print the unpushed SHAs and the push error and stop, exactly as the unpushed-commits merge gate below refuses to merge; a PR opened from a tree missing the review fixes is the precise failure this check exists to prevent. Skip the check when the branch has no upstream (`git rev-parse --abbrev-ref --symbolic-full-name @{u} >/dev/null 2>&1` fails — detached HEAD or no origin): there is nothing to compare against. A **local** upstream is the case that does *not* skip — `@{u}` resolves, so the check runs and the `"."` guard above stops it rather than letting it push into the local repo. Note `git status` is **not** a substitute — a clean working tree says nothing about committed-but-unpushed commits, which is exactly the state this catches. + Never a bare `git push`, which under `push.default=matching` fans out to every same-named local branch, and never `git push origin {current_branch}`, which hardcodes the *local* branch name as the destination: on a branch whose upstream is named differently (or lives on another remote) that pushes a spurious remote branch, leaves the real PR head stale, and `@{u}..HEAD` is still non-empty afterward while the push itself "succeeded". On a non-fast-forward, retry once behind `git pull --rebase --autostash`. If that rebase conflicts, **resolve it through [lib/rebase-conflict-resolution.md](../../lib/rebase-conflict-resolution.md), continue until the rebase completes, rerun the focused checks affected by the resolution, then retry the same upstream-derived push**. Do not classify an active rebase conflict as a push failure and do not stop merely to ask the user to resolve it. `/do:pr` is invoked programmatically by `/do:next` and `/do:pr-better`, so the resolution must leave the expected branch checked out and fully rebased before the workflow continues. **If the push still fails after that one retry, do NOT create the PR** — this means after any conflict was resolved and the rebase completed; print the unpushed SHAs and the push error and stop, exactly as the unpushed-commits merge gate below refuses to merge. A PR opened from a tree missing the review fixes is the precise failure this check exists to prevent. Skip the check when the branch has no upstream (`git rev-parse --abbrev-ref --symbolic-full-name @{u} >/dev/null 2>&1` fails — detached HEAD or no origin): there is nothing to compare against. A **local** upstream is the case that does *not* skip — `@{u}` resolves, so the check runs and the `"."` guard above stops it rather than letting it push into the local repo. Note `git status` is **not** a substitute — a clean working tree says nothing about committed-but-unpushed commits, which is exactly the state this catches. - Create a PR / merge request from `{current_branch}` to `{default_branch}`: - GitHub: `gh pr create --base {default_branch} --head {current_branch} --title "..." --body "..."` - GitLab: `glab mr create --source-branch {current_branch} --target-branch {default_branch} --title "..." --description "..."` (add `--yes` to skip the interactive prompt; `--remove-source-branch` if the project deletes merged branches) diff --git a/install.sh b/install.sh index b38f7dc..fb79bd4 100755 --- a/install.sh +++ b/install.sh @@ -178,7 +178,7 @@ LIBS=( gh-host github-reviewer-loop graphql-escaping local-agent-review-loop model-tiers multi-reviewer-loop next-swarm ollama-review-loop per-finding-root-cause plan-id-format plan-issue-mode - post-review-doc-recommendations remediation-agent-template + post-review-doc-recommendations rebase-conflict-resolution remediation-agent-template review-agent-selection review-config-defaults review-convergence-gate swift-review-checklist swift-gotchas review-surface-scan review-surface-quality review-security-audit diff --git a/lib/local-agent-review-loop.md b/lib/local-agent-review-loop.md index 5a95ea5..0c352e7 100644 --- a/lib/local-agent-review-loop.md +++ b/lib/local-agent-review-loop.md @@ -395,7 +395,7 @@ Initialize `ITERATION=0`, `STATUS=""`, and `MAX_ITERATIONS` / `MAX_EXPLICIT` fro ```bash git push origin {BRANCH_NAME} ``` - If the push fails (e.g., non-fast-forward), run `git pull --rebase --autostash && git push origin {BRANCH_NAME}` once before reporting failure. + If the push fails (e.g., non-fast-forward), run `git pull --rebase --autostash` and then retry the push once. If the pull stops on conflicts, do not abort or report failure merely because the conflict exists: read and follow [rebase-conflict-resolution.md](./rebase-conflict-resolution.md), resolve and continue the rebase, rerun the build/tests affected by the resolution, then push. Report failure only after the completed resolution and retry still cannot publish the branch. 6. **Re-loop or stop**: - `ITERATION=$((ITERATION + 1))` diff --git a/lib/multi-reviewer-loop.md b/lib/multi-reviewer-loop.md index 5346caf..9b27b8f 100644 --- a/lib/multi-reviewer-loop.md +++ b/lib/multi-reviewer-loop.md @@ -110,20 +110,20 @@ This is the default path. Iterate `REVIEW_AGENTS` in order, running each reviewe # empty-string remote: a fatal error, reported as a bogus push-failed. if [ -n "$UNPUSHED" ]; then if ! git push "$PUSH_REMOTE" "HEAD:$PUSH_BRANCH"; then - # Never leave a conflicted rebase behind: `push-failed` is a continue-signal, - # so the next reviewer would otherwise run against a detached, mid-rebase HEAD - # with this pass's fix commit unapplied — worse than the unpushed state. + # A conflict is a resolution handoff, not push-failed. The caller MUST apply + # rebase-conflict-resolution.md and rerun this config-derived block before it + # records a status or dispatches the next reviewer. if git pull --rebase --autostash; then git push "$PUSH_REMOTE" "HEAD:$PUSH_BRANCH" else - git rebase --abort 2>/dev/null # no-op when the pull failed for another reason + echo "REBASE_CONFLICT_NEEDS_RESOLUTION" false fi fi fi ``` - If `UNPUSHED` was non-empty the inner loop's push step did not run, and the block above already pushed it — `git push "$PUSH_REMOTE" "HEAD:$PUSH_BRANCH"`, retried once behind `git pull --rebase --autostash` on a non-fast-forward, exactly the retry the loop files use. **Run the whole thing as one block**: `PUSH_REMOTE`/`PUSH_BRANCH` live only in the shell that set them. If the retry's rebase hits a conflict it is **aborted**, leaving the branch exactly as the reviewer left it, and the pass is recorded `push-failed` — never left mid-rebase. That matters more here than in the inner loops, which run the same `pull --rebase && push` idiom as their last act before reporting failure: `push-failed` is a *continue-signal*, so execution goes on to the next reviewer, and a detached mid-rebase HEAD would both corrupt that reviewer's tree and silently disable its own assertion (no resolvable `@{u}` on a detached HEAD ⇒ the check skips). If the push still fails, record the pass per the rules below. + If `UNPUSHED` was non-empty the inner loop's push step did not run, and the block above already attempted to push it — `git push "$PUSH_REMOTE" "HEAD:$PUSH_BRANCH"`, retried once behind `git pull --rebase --autostash` on a non-fast-forward, exactly the retry the loop files use. **Run the whole thing as one block**: `PUSH_REMOTE`/`PUSH_BRANCH` live only in the shell that set them. `REBASE_CONFLICT_NEEDS_RESOLUTION` is an explicit handoff to the orchestrator, not the final result of the block: while Git reports an active rebase or unmerged paths, do **not** record `push-failed` and do not dispatch another reviewer. Read and follow [rebase-conflict-resolution.md](./rebase-conflict-resolution.md), complete the rebase and its focused validation, then rerun this entire config-derived block so the variables and destination are fresh. Only a non-conflict pull error, the playbook's last-resort abort after a genuine ambiguity, or a push that still fails after the completed resolution becomes `push-failed`. This prevents both premature abandonment and a next reviewer inheriting a detached, mid-rebase HEAD. **Push to the ref the upstream names — not a bare `git push`, and not `git push origin HEAD`.** A bare push fans out under `push.default=matching` to every local branch with a same-named remote (publishing unrelated branches — including a `release` branch that may auto-tag and publish), and errors outright under `push.default=nothing`. `git push origin HEAD` is the subtler trap: with no ``, git resolves `HEAD` to the **local** branch name and pushes to `refs/heads/`, ignoring the upstream entirely — so on a branch whose upstream has a different name (or lives on another remote) it creates a spurious remote branch, leaves the real PR head stale, and `@{u}..HEAD` is *still* non-empty afterward. That is issue #134's exact failure, reintroduced by the guard meant to prevent it. Deriving the destination from `branch..merge` is what makes the push land on the branch the PR was opened from — and because that value is already fully qualified, it is `HEAD:$PUSH_BRANCH`, never `HEAD:refs/heads/$PUSH_BRANCH`. diff --git a/lib/ollama-review-loop.md b/lib/ollama-review-loop.md index 0550aee..ee3d77d 100644 --- a/lib/ollama-review-loop.md +++ b/lib/ollama-review-loop.md @@ -163,7 +163,7 @@ Initialize `ITERATION=0`, `STATUS=""`, and `MAX_ITERATIONS` / `MAX_EXPLICIT` fro ```bash git push origin {BRANCH_NAME} ``` - If the push fails (non-fast-forward), run `git pull --rebase --autostash && git push origin {BRANCH_NAME}` once before reporting failure. + If the push fails (non-fast-forward), run `git pull --rebase --autostash` and then retry the push once. If the pull stops on conflicts, do not abort or report failure merely because the conflict exists: read and follow [rebase-conflict-resolution.md](./rebase-conflict-resolution.md), resolve and continue the rebase, rerun the build/tests affected by the resolution, then push. Report failure only after the completed resolution and retry still cannot publish the branch. 6. **Re-loop or stop**: - `ITERATION=$((ITERATION + 1))` - **Apply the convergence gate** (`~/.claude/lib/review-convergence-gate.md`) before another round: if the round just completed made zero commits or landed only *marginal* findings (edge-case guards, hypotheticals with no concrete wrong outcome), **converge — set `STATUS=clean` (or `STATUS=incomplete` if the round had any coverage gap, `REVIEW_ERRORS + PARSE_ERRORS + TRUNCATED > 0`) and exit**, noting the diminishing-returns convergence in the report. The coverage-gap exception is the same invariant step 3 enforces: a partially-reviewed diff is never `clean`, even when the gate converges. Only a round with at least one *substantive* finding earns another pass. diff --git a/lib/rebase-conflict-resolution.md b/lib/rebase-conflict-resolution.md new file mode 100644 index 0000000..94deb68 --- /dev/null +++ b/lib/rebase-conflict-resolution.md @@ -0,0 +1,89 @@ +# Autonomous Rebase Conflict Resolution + +Use this playbook whenever a required `git rebase` or `git pull --rebase` stops +with conflicts. A conflict is a resolution step in the PR workflow, not by itself +a reason to abort, stop, or ask the user to take over. + +## Required posture + +- **Resolve and continue by default.** Do not abort merely because Git reports + conflicts. Preserve both the latest base branch contract and the intent of the + commit being replayed. +- Keep the resolution scoped to the conflicted paths and any source-of-truth files + that must change with them. Preserve unrelated user changes and commits. +- Never blanket-select `--ours` or `--theirs`, never hard-reset the branch, and + never use a bare `--force`. During a rebase those labels describe Git's replay + mechanics, not "the user's work" versus "upstream," so choosing by label alone + can silently discard the feature. +- A generated-file conflict alone is never an ambiguity that needs user input. + Regenerate it from the resolved canonical sources. + +## Resolution loop + +Repeat these steps for every commit at which the rebase stops: + +1. **Inspect the replay context before editing.** Run: + + ```bash + git status --short + git diff --name-only --diff-filter=U + git rebase --show-current-patch + ``` + + Read every conflicted file in full plus the related callers, tests, schemas, and + recent history. When useful, inspect the index stages with `git show :1:`, + `git show :2:`, and `git show :3:`; interpret them in the context of + the current patch rather than trusting the ours/theirs labels. + +2. **Resolve human-authored sources semantically.** Incorporate upstream's current + API, schema, or structure while retaining the behavior the replayed commit was + adding. Trace renamed/moved files and producer-consumer contracts instead of + resolving only the marker lines. Use tests and call sites to decide the combined + result. + +3. **Regenerate generated artifacts; do not hand-merge them.** For a path identified + by a generated header, filename, repository docs, or adjacent tooling (for + example `server/lib/apiRouteCatalog.generated.json`): + + - Locate its canonical inputs and generator using nearby docs, package scripts, + build scripts, and repository search. + - Resolve the human-authored inputs first. + - If the generator needs the output to be parseable, temporarily replace the + conflicted output with either complete index stage only as a seed. That choice + is not the resolution. + - Run the repository's canonical generator and use its complete output as the + resolution. Run the corresponding generated-drift, schema, or catalog check. + - If no generator exists, reconstruct the output from the canonical sources and + validate it with the repository's existing checks. Do not guess from conflict + markers alone. + +4. **Stage and continue only when the stop is fully resolved.** Stage only the + files resolved for the current replayed commit, then run: + + ```bash + test -z "$(git diff --name-only --diff-filter=U)" + git diff --check + git diff --cached --check + GIT_EDITOR=true git rebase --continue + ``` + + If Git stops again, return to step 1. If a replayed commit becomes empty, verify + from the current patch, history, and tests that its full intent is already on the + new base; only then use `git rebase --skip`. + +5. **Verify the completed rebase.** Confirm Git is no longer mid-rebase, the expected + feature branch is checked out, and there are no unmerged paths. Run the focused + generator/schema checks and tests for every resolution, then the caller's normal + build/test gate. Continue the push, review, and PR-creation workflow from the + step that encountered the conflict; a resolved rebase is not a terminal status. + +## Last-resort abort + +Abort only after the inspection above exposes a genuine, material ambiguity that +cannot be answered from the replayed commit, canonical sources, generator, callers, +tests, or repository history, or after required resolution tooling remains +unavailable with no deterministic fallback. Before reporting that blocker, run +`git rebase --abort`, verify the original branch/commit and clean state were +restored, and report the exact incompatible choices or missing tool plus the checks +performed. Never stop with only "rebase conflict" or "generated file conflict" as +the reason. diff --git a/src/transformer.js b/src/transformer.js index 0348de9..4370f3e 100644 --- a/src/transformer.js +++ b/src/transformer.js @@ -160,6 +160,8 @@ const ON_DEMAND_LIBS = new Map([ { what: 'draft-enhancement loop', when: '`--enhance-with` was passed' }], ['ci-flake-handling.md', { what: 'CI flake triage rules', when: 'a CI check fails in a way that looks like a flake' }], + ['rebase-conflict-resolution.md', + { what: 'autonomous rebase-conflict resolution playbook', when: 'a rebase stops on conflicts' }], // Review lenses: review-agent-selection.md dispatches only the lenses a diff // actually signals — often one or two, sometimes none. diff --git a/test/installer.test.js b/test/installer.test.js index ef564f8..24c95af 100644 --- a/test/installer.test.js +++ b/test/installer.test.js @@ -794,8 +794,9 @@ describe('bundled lib docs', () => { } it('writes each deferred lib beside the SKILL.md that cites it', () => { - // Only the libs a command actually cites — /do:pr runs reviewers, so it bundles - // the four backends; it never opens the tracker, so it gets no issue-mode doc. + // Only the libs a command actually cites — /do:pr bundles its four reviewer + // backends plus the conflict resolver; it never opens the tracker, so it gets + // no issue-mode doc. const { tmpDir, env } = makeSkillEnv(); try { install({ env, packageDir: PACKAGE_DIR, dryRun: false }); @@ -803,7 +804,8 @@ describe('bundled lib docs', () => { assert.ok(fs.existsSync(bundleDir), 'do-pr must get a bundle dir'); const written = fs.readdirSync(bundleDir); for (const name of ['copilot-review-loop.md', 'github-reviewer-loop.md', - 'local-agent-review-loop.md', 'ollama-review-loop.md']) { + 'local-agent-review-loop.md', 'ollama-review-loop.md', + 'rebase-conflict-resolution.md']) { assert.ok(written.includes(name), `${name} must be bundled with /do:pr`); } assert.ok(!written.includes('plan-issue-mode.md'), '/do:pr does not use issue mode'); diff --git a/test/review-loop-contract.test.js b/test/review-loop-contract.test.js index bda14e9..31a13ec 100644 --- a/test/review-loop-contract.test.js +++ b/test/review-loop-contract.test.js @@ -342,19 +342,18 @@ describe('review-loop parse contracts', () => { // The point is that the push lives inside the guard, in the same shell — not // that it sits on any particular line. assert.match(wrapper, /if \[ -n "\$UNPUSHED" \]; then[\s\S]{0,600}?git push "\$PUSH_REMOTE" "HEAD:\$PUSH_BRANCH"/); - // A conflicted retry must not strand the branch mid-rebase: push-failed is a - // continue-signal, so the next reviewer would inherit a detached HEAD whose own - // assertion then silently skips (no resolvable @{u} to compare against). - // Anchored to the else-branch, not a bare substring: naming `git rebase --abort` - // anywhere in the prose would otherwise satisfy this while the code path is gone. - // The trailing `false` is what makes a failed push observable to the orchestrator - // — without it the abort's own success flips the block to exit 0, the pass is - // never recorded push-failed, and the stranded commits reach the merge gate. + // A conflicted retry is a resolution handoff, not an automatic abort or a + // push-failed verdict. The trailing false keeps the shell block from falling + // through, while the explicit marker tells the orchestrator to resolve and + // rerun the config-derived block before it dispatches another reviewer. assert.match( wrapper, - /else\n\s*git rebase --abort 2>\/dev\/null[^\n]*\n\s*false\n/, - 'a conflicted retry must abort the rebase AND still exit non-zero so the pass records push-failed', + /else\n\s*echo "REBASE_CONFLICT_NEEDS_RESOLUTION"\n\s*false\n/, + 'a conflicted retry must hand control to autonomous resolution without falling through', ); + assert.doesNotMatch(wrapper, /git rebase --abort/, 'the shared retry must not abort at the first conflict'); + assert.match(wrapper, /do \*\*not\*\* record `push-failed`/); + assert.match(wrapper, /complete the rebase[\s\S]{0,180}?rerun this entire config-derived block/); // do:pr's pre-PR push must derive its destination the SAME way, from the branch's // upstream config — `git push origin {current_branch}` hardcodes the local branch @@ -410,9 +409,39 @@ describe('review-loop parse contracts', () => { // re-pointing a differently-named or non-origin upstream at origin/. // Without this, reintroducing `-u` in the second bullet passes every other test. assert.match(pr, /\*\*A genuine remote upstream\*\*[^\n]*never `-u`/); - // A conflicted retry must not strand the branch mid-rebase for /do:next and - // /do:pr-better, which invoke /do:pr programmatically. - assert.match(pr, /conflicts, abort it\*\* \(`git rebase --abort 2>\/dev\/null`\)/); + // A conflicted retry must be resolved before /do:next or /do:pr-better resumes. + assert.match(pr, /If that rebase conflicts, \*\*resolve it through/); + assert.match(pr, /Do not classify an active rebase conflict as a push failure/); + assert.doesNotMatch(pr, /conflicts, abort it\*\*/); + }); + + it('resolves do:pr rebase conflicts autonomously and regenerates derived files', () => { + const pr = readCommand('pr.md'); + const prBetter = readCommand('pr-better.md'); + const phaseB = prBetter.slice(prBetter.indexOf('## Phase B')); + const resolver = readLib('rebase-conflict-resolution.md'); + + assert.match(pr, /If the rebase hits conflicts, \*\*resolve them and continue the rebase\*\*/); + assert.match(pr, /rebase-conflict-resolution\.md/); + assert.doesNotMatch(pr, /ask the user to resolve them/); + assert.match(phaseB, /resolving and continuing through conflicts/); + assert.match(phaseB, /A rebase conflict is not a handoff or stop condition/); + assert.doesNotMatch(phaseB, /aborting and surfacing conflicts/); + + assert.match(resolver, /A conflict is a resolution step in the PR workflow/); + assert.match(resolver, /Do not abort merely because Git reports\s+conflicts/); + assert.match(resolver, /apiRouteCatalog\.generated\.json/); + assert.match(resolver, /Resolve the human-authored inputs first/); + assert.match(resolver, /Run the repository's canonical generator/); + assert.match(resolver, /GIT_EDITOR=true git rebase --continue/); + assert.match(resolver, /only then use `git rebase --skip`/); + assert.match(resolver, /a resolved rebase is not a terminal status/); + + for (const name of ['local-agent-review-loop.md', 'ollama-review-loop.md']) { + const loop = readLib(name); + assert.match(loop, /rebase-conflict-resolution\.md/, `${name} must use the shared resolver`); + assert.match(loop, /resolve and continue the rebase/, `${name} must continue after resolving`); + } }); it('files issues inside the scan-only gate, not after it', () => { diff --git a/uninstall.sh b/uninstall.sh index 9ad2c4c..3cfae5f 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -103,7 +103,7 @@ LIBS=( gh-host github-reviewer-loop graphql-escaping local-agent-review-loop model-tiers multi-reviewer-loop next-swarm ollama-review-loop per-finding-root-cause plan-id-format plan-issue-mode - post-review-doc-recommendations remediation-agent-template + post-review-doc-recommendations rebase-conflict-resolution remediation-agent-template review-agent-selection review-config-defaults review-convergence-gate swift-review-checklist swift-gotchas review-surface-scan review-surface-quality review-security-audit