Forgejo: ensure the agent resolves PR conflicts against the base branch #63

New issue

Closed

opened 2026-10-23 14:53:25 +02:00 by omp-deck · 0 comments

omp-deck commented

2026-10-23 14:53:25 +02:00

Collaborator

Problem

Agents working on Forgejo pull requests (implementer sessions, review-submitted follow-ups) never check whether their PR conflicts with the base branch, so PRs sit unmergeable. Add steering, and change tools, so the agent always notices and resolves conflicts against the base branch.

Decisions:

Resolution strategy: rebase onto origin/<base> with git_rebase, then git_push force_after_rebase (no merge commits; no new merge capability).
Detection: a local git merge-tree against a freshly fetched base is the dedicated, deterministic check; Forgejo's mergeable flag is used where a Forgejo tool already has it.
No turn-end gate; steering + tool signals only.
Steering goes into the implementer seed and the review-submitted seed, not the Forgejo system context.

Plan

Context

Tools that touch a PR branch report conflicts with the PR's base: git_push (after a successful push to a branch that has an open PR), forgejo_create_pr (after creating the PR; never refuses), forgejo_read_pr (lists conflicted files when Forgejo says mergeable: no). The implementer and review-submitted seeds tell the agent to check and resolve conflicts by rebasing. git_push's non-fast-forward message stops telling the agent the branches "need a human".

Approach

1. Shared conflict check helper

New file apps/server/src/bridge/git-tools/base-conflict.ts (no equivalent exists; rebase.ts#conflictedFiles only reads an in-progress rebase's index):

export type BaseConflictCheck = { ok: true; files: string[] } | { ok: false; error: string };

/**
 * Fetch `refspecs` from `remote`, then report the paths that conflict when
 * merging `head` into `base` (both revs resolvable after the fetch). Touches no
 * working tree, index, or HEAD. `files` is empty when they merge cleanly.
 */
export async function checkBaseConflict(
	root: string,
	opts: { remote: string; refspecs: string[]; base: string; head: string; signal?: AbortSignal },
): Promise<BaseConflictCheck>;

Implementation:

prepareDeckSsh() from ./ssh.ts; on !ok return { ok: false, error }.
runGit(root, ["fetch", "--no-tags", "--no-write-fetch-head", remote, ...refspecs], { timeoutMs: NETWORK_TIMEOUT_MS, signal, env: prepared.ssh?.env }); non-zero → { ok: false, error: withHostKeyHint(gitErrorText(res, "git fetch failed"), prepared.ssh) }. Copy the pattern from fetch.ts lines 90–96.
runGit(root, ["merge-tree", "--write-tree", "--name-only", "--no-messages", "-z", base, head], { signal }). Exit 0 → { ok: true, files: [] }. Exit 1 → split stdout on \0, drop the first entry (the tree oid) and empties, dedupe preserving order → { ok: true, files }. Any other exit → { ok: false, error: gitErrorText(res, "git merge-tree failed") }. (Requires git ≥ 2.38; dev box has 2.47.)

Also export from the same file the shared next-step text:

export function rebaseInstruction(remote: string, base: string): string
// → `Resolve it: git_fetch, then git_rebase action:"start" onto:"${remote}/${base}", resolve the conflicts, git_rebase action:"continue", then git_push force_after_rebase:true.`

2. `git_push` reports base conflicts for the branch's open PR

apps/server/src/bridge/git-tools/push.ts: change signature to buildPushTool(cwd: string, forgejo: ForgejoToolContext | null); update the caller in git-tools/index.ts (buildPushTool(cwd, forgejo)) and every test call in rebase.test.ts (buildPushTool(repo) → buildPushTool(repo, null); grep buildPushTool( to find all).
After the successful push (after clearMarker, before ok(result)), when forgejo !== null && remote === "origin":
- List open PRs: fetchAllPages<ApiPull>(forgejo.client, repoApiPath(forgejo.owner, forgejo.repo, "pulls"), { query: { state: "open" }, signal }) (imports from ../../forgejo/tools/shared.ts and the module that exports repoApiPath — the same one prs.ts imports it from).
- Pick the first PR with head.ref === branch and head.repo?.full_name === \${owner}/${repo}`` (fork PRs skipped). None → no check, no new fields.
- Run checkBaseConflict(root, { remote, refspecs: [\+refs/heads/${base}:refs/remotes/${remote}/${base}`], base: `refs/remotes/${remote}/${base}`, head: "HEAD", signal })withbase = pr.base.ref`.
- Result fields (added to GitPushResult in packages/protocol/src/git-tools.ts):
```
/** Open PR for this branch whose base conflicts with the pushed head. */
baseConflict?: { pr: number; base: string; files: string[]; next: string };
/** The base-conflict check could not run; the push itself succeeded. */
baseConflictCheckError?: string;
```
  Set baseConflict only when files.length > 0 (next = rebaseInstruction(remote, base)). PR listing failure (!ok, use the API error message) or helper { ok: false } → set baseConflictCheckError; the push still returns ok(...) (never fail).
Update the description: append "After pushing a branch that has an open PR, reports files that conflict with the PR's base (baseConflict)."
Non-fast-forward message (push.ts lines 84–88): replace with
push rejected: ${remote}/${branch} has commits you do not have locally. Run git_fetch with fast_forward:true; if the branches have diverged, git_rebase action:"start" onto:"${remote}/${branch}", then git_push.\n${out}
and change the description sentence "If the remote rejects it as non-fast-forward, call git_fetch with fast_forward:true first." to "If the remote rejects it as non-fast-forward, call git_fetch with fast_forward:true, or git_rebase onto the remote branch if they diverged."
apps/web/src/components/tools/git/GitPushView.tsx: when r.baseConflict is present, render a warning line conflicts with <base> (PR #<pr>): <files joined ", ">; when r.baseConflictCheckError is present, render it muted. Follow the existing Tag/row styling in that file. Add a baseConflict example to the push fixture in apps/web/src/preview-git.ts only if that file has multiple variants per tool; otherwise leave it.

3. `forgejo_create_pr` warns after creating a conflicting PR

In apps/server/src/forgejo/tools/prs.ts createPr.run, after res.ok and only when !head.includes(":"):

resolveRepo(ctx.cwd, signal) (from bridge/git-tools/run.ts); null → skip check.
checkBaseConflict(root, { remote: "origin", refspecs: [\+refs/heads/${base}:refs/remotes/origin/${base}`, `+refs/heads/${head}:refs/remotes/origin/${head}`], base: `refs/remotes/origin/${base}`, head: `refs/remotes/origin/${head}`, signal })`.
Conflicts → append to the text: \n\nWARNING: ${head} conflicts with ${base} in: ${files.join(", ")}. ${rebaseInstruction("origin", base)} and set baseConflictFiles: files on the details. Check failure → append \n\n(could not check for conflicts with ${base}: ${error}). Never turn the result into an error: the PR exists.
ForgejoCreatePrResult in packages/protocol/src/forgejo-tools.ts: add baseConflictFiles?: string[]. CreatePrView.tsx: show a warning row listing them when present.
Description: append "After creating it, warns if the head conflicts with the base."

4. `forgejo_read_pr` lists conflicted files when unmergeable

In readPr.run (prs.ts ~193): when pr.data.mergeable === false && pr.data.head?.sha && pr.data.base?.ref and the head is in this repo (head.repo?.full_name === \${ctx.owner}/${ctx.repo}``):

resolveRepo(ctx.cwd); null → skip.
checkBaseConflict(root, { remote: "origin", refspecs: [\+refs/heads/${base}:refs/remotes/origin/${base}`, `refs/pull/${n}/head`], base: `refs/remotes/origin/${base}`, head: pr.data.head.sha, signal }). (--no-write-fetch-head` is fine: the object is fetched into the odb and referenced by sha.)
Pass the outcome into formatPr via a new optional second parameter formatPr(pr: ApiPull, conflict?: { files: string[] } | { error: string }), which changes the mergeable: line to mergeable: no — conflicts with <base> in: a.ts, b.ts or mergeable: no (conflict check failed: <error>). Other formatPr callers unchanged. If the local check returns zero files (Forgejo stale), print mergeable: no (Forgejo); a local merge of the current heads is clean.
Description: replace mergeable/merged/draft with mergeable/merged/draft (when not mergeable, the conflicting files).

5. Seed steering

apps/server/src/forgejo/conversation-detect.ts:

buildImplementerSeed: append a paragraph: Before you finish: fetch the base branch and make sure your PR does not conflict with it (git_push and forgejo_create_pr report conflicts). If it does, rebase onto origin/<base> with git_rebase, resolve the conflicts, and git_push force_after_rebase:true. with <base> = t.baseBranch ?? "the default branch".
buildReviewSubmittedSeed Task line: after ...Address the feedback: change the code, commit, push. insert Then check the PR is not in conflict with ${pr?.base?.ref ?? "its base"} (forgejo_read_pr lists conflicting files; git_push reports them); if it is, rebase onto origin/<base> with git_rebase and git_push force_after_rebase:true. Replace the final sentence with If the verdict is approved and there is no feedback, post no comment; push only if you had to resolve conflicts with the base.

Critical files & anchors

apps/server/src/bridge/git-tools/push.ts — buildPushTool execute tail (lines ~78–97) and non-ff message.
apps/server/src/forgejo/tools/prs.ts — readPr (~186), createPr (~323).
apps/server/src/forgejo/tools/format.ts — formatPr mergeable line (~84).
apps/server/src/forgejo/conversation-detect.ts — buildImplementerSeed (~214), buildReviewSubmittedSeed task line (~263).
packages/protocol/src/git-tools.ts / forgejo-tools.ts — result types.

Verification

Run from /workspace/omp-deck with bun test <path>.

New apps/server/src/bridge/git-tools/base-conflict.test.ts (temp repos as in rebase.test.ts: a bare remote + clone): main and feature both edit a.txt differently, feature pushed → checkBaseConflict returns { ok: true, files: ["a.txt"] }; non-overlapping edits → files: []; nonexistent refspec → ok: false. Assert the clone's HEAD, index and working tree are unchanged after the call.
apps/server/src/bridge/git-tools/rebase.test.ts: existing push tests pass with buildPushTool(repo, null); add one case asserting the non-ff rejection text contains git_rebase action:"start" onto:"origin/<branch>".
apps/server/src/forgejo/conversation-detect.test.ts: implementer seed contains rebase onto origin/main for baseBranch: "main"; review-submitted seed contains push only if you had to resolve conflicts.
apps/server/src/forgejo/tools/format.test.ts: formatPr({ ...pr, mergeable: false }, { files: ["a.ts"] }) contains mergeable: no — conflicts with main in: a.ts; existing mergeable: no assertion still matches.
Manual end-to-end on this Forgejo instance: create a branch editing a line that main also changed, git_push → result JSON has baseConflict.files with that path; forgejo_create_pr → text contains WARNING: ... conflicts with main; forgejo_read_pr → mergeable: no — conflicts with main in: .... Then git_rebase onto origin/main, resolve, git_push force_after_rebase:true → no baseConflict.
bun run typecheck (or the repo's equivalent script in root package.json) passes for server, protocol and web.

Assumptions & contingencies

Forgejo exposes refs/pull/<n>/head over git fetch (Gitea-compatible). If the fetch fails in the manual check, fall back in step 4 to fetching +refs/heads/<head.ref>:refs/remotes/origin/<head.ref> and using that ref as head.
If the open-PR listing filter in step 2 cannot match because head.repo is absent in this Forgejo's payload, match on head.ref === branch alone.

## Problem Agents working on Forgejo pull requests (implementer sessions, review-submitted follow-ups) never check whether their PR conflicts with the base branch, so PRs sit unmergeable. Add steering, and change tools, so the agent always notices and resolves conflicts against the base branch. Decisions: - Resolution strategy: rebase onto `origin/<base>` with `git_rebase`, then `git_push force_after_rebase` (no merge commits; no new merge capability). - Detection: a local `git merge-tree` against a freshly fetched base is the dedicated, deterministic check; Forgejo's `mergeable` flag is used where a Forgejo tool already has it. - No turn-end gate; steering + tool signals only. - Steering goes into the implementer seed and the review-submitted seed, not the Forgejo system context. ## Plan ### Context Tools that touch a PR branch report conflicts with the PR's base: `git_push` (after a successful push to a branch that has an open PR), `forgejo_create_pr` (after creating the PR; never refuses), `forgejo_read_pr` (lists conflicted files when Forgejo says `mergeable: no`). The implementer and review-submitted seeds tell the agent to check and resolve conflicts by rebasing. `git_push`'s non-fast-forward message stops telling the agent the branches "need a human". ### Approach #### 1. Shared conflict check helper New file `apps/server/src/bridge/git-tools/base-conflict.ts` (no equivalent exists; `rebase.ts#conflictedFiles` only reads an in-progress rebase's index): ```ts export type BaseConflictCheck = { ok: true; files: string[] } | { ok: false; error: string }; /** * Fetch `refspecs` from `remote`, then report the paths that conflict when * merging `head` into `base` (both revs resolvable after the fetch). Touches no * working tree, index, or HEAD. `files` is empty when they merge cleanly. */ export async function checkBaseConflict( root: string, opts: { remote: string; refspecs: string[]; base: string; head: string; signal?: AbortSignal }, ): Promise<BaseConflictCheck>; ``` Implementation: - `prepareDeckSsh()` from `./ssh.ts`; on `!ok` return `{ ok: false, error }`. - `runGit(root, ["fetch", "--no-tags", "--no-write-fetch-head", remote, ...refspecs], { timeoutMs: NETWORK_TIMEOUT_MS, signal, env: prepared.ssh?.env })`; non-zero → `{ ok: false, error: withHostKeyHint(gitErrorText(res, "git fetch failed"), prepared.ssh) }`. Copy the pattern from `fetch.ts` lines 90–96. - `runGit(root, ["merge-tree", "--write-tree", "--name-only", "--no-messages", "-z", base, head], { signal })`. Exit 0 → `{ ok: true, files: [] }`. Exit 1 → split stdout on `\0`, drop the first entry (the tree oid) and empties, dedupe preserving order → `{ ok: true, files }`. Any other exit → `{ ok: false, error: gitErrorText(res, "git merge-tree failed") }`. (Requires git ≥ 2.38; dev box has 2.47.) Also export from the same file the shared next-step text: ```ts export function rebaseInstruction(remote: string, base: string): string // → `Resolve it: git_fetch, then git_rebase action:"start" onto:"${remote}/${base}", resolve the conflicts, git_rebase action:"continue", then git_push force_after_rebase:true.` ``` #### 2. `git_push` reports base conflicts for the branch's open PR - `apps/server/src/bridge/git-tools/push.ts`: change signature to `buildPushTool(cwd: string, forgejo: ForgejoToolContext | null)`; update the caller in `git-tools/index.ts` (`buildPushTool(cwd, forgejo)`) and every test call in `rebase.test.ts` (`buildPushTool(repo)` → `buildPushTool(repo, null)`; grep `buildPushTool(` to find all). - After the successful push (after `clearMarker`, before `ok(result)`), when `forgejo !== null && remote === "origin"`: - List open PRs: `fetchAllPages<ApiPull>(forgejo.client, repoApiPath(forgejo.owner, forgejo.repo, "pulls"), { query: { state: "open" }, signal })` (imports from `../../forgejo/tools/shared.ts` and the module that exports `repoApiPath` — the same one `prs.ts` imports it from). - Pick the first PR with `head.ref === branch` and `head.repo?.full_name === \`${owner}/${repo}\`` (fork PRs skipped). None → no check, no new fields. - Run `checkBaseConflict(root, { remote, refspecs: [\`+refs/heads/${base}:refs/remotes/${remote}/${base}\`], base: \`refs/remotes/${remote}/${base}\`, head: "HEAD", signal })` with `base = pr.base.ref`. - Result fields (added to `GitPushResult` in `packages/protocol/src/git-tools.ts`): ```ts /** Open PR for this branch whose base conflicts with the pushed head. */ baseConflict?: { pr: number; base: string; files: string[]; next: string }; /** The base-conflict check could not run; the push itself succeeded. */ baseConflictCheckError?: string; ``` Set `baseConflict` only when `files.length > 0` (`next = rebaseInstruction(remote, base)`). PR listing failure (`!ok`, use the API error message) or helper `{ ok: false }` → set `baseConflictCheckError`; the push still returns `ok(...)` (never `fail`). - Update the description: append `"After pushing a branch that has an open PR, reports files that conflict with the PR's base (baseConflict)."` - Non-fast-forward message (push.ts lines 84–88): replace with `push rejected: ${remote}/${branch} has commits you do not have locally. Run git_fetch with fast_forward:true; if the branches have diverged, git_rebase action:"start" onto:"${remote}/${branch}", then git_push.\n${out}` and change the description sentence `"If the remote rejects it as non-fast-forward, call git_fetch with fast_forward:true first."` to `"If the remote rejects it as non-fast-forward, call git_fetch with fast_forward:true, or git_rebase onto the remote branch if they diverged."` - `apps/web/src/components/tools/git/GitPushView.tsx`: when `r.baseConflict` is present, render a warning line `conflicts with <base> (PR #<pr>): <files joined ", ">`; when `r.baseConflictCheckError` is present, render it muted. Follow the existing `Tag`/row styling in that file. Add a `baseConflict` example to the `push` fixture in `apps/web/src/preview-git.ts` only if that file has multiple variants per tool; otherwise leave it. #### 3. `forgejo_create_pr` warns after creating a conflicting PR In `apps/server/src/forgejo/tools/prs.ts` `createPr.run`, after `res.ok` and only when `!head.includes(":")`: - `resolveRepo(ctx.cwd, signal)` (from `bridge/git-tools/run.ts`); null → skip check. - `checkBaseConflict(root, { remote: "origin", refspecs: [\`+refs/heads/${base}:refs/remotes/origin/${base}\`, \`+refs/heads/${head}:refs/remotes/origin/${head}\`], base: \`refs/remotes/origin/${base}\`, head: \`refs/remotes/origin/${head}\`, signal })`. - Conflicts → append to the text: `\n\nWARNING: ${head} conflicts with ${base} in: ${files.join(", ")}. ${rebaseInstruction("origin", base)}` and set `baseConflictFiles: files` on the details. Check failure → append `\n\n(could not check for conflicts with ${base}: ${error})`. Never turn the result into an error: the PR exists. - `ForgejoCreatePrResult` in `packages/protocol/src/forgejo-tools.ts`: add `baseConflictFiles?: string[]`. `CreatePrView.tsx`: show a warning row listing them when present. - Description: append `"After creating it, warns if the head conflicts with the base."` #### 4. `forgejo_read_pr` lists conflicted files when unmergeable In `readPr.run` (prs.ts ~193): when `pr.data.mergeable === false && pr.data.head?.sha && pr.data.base?.ref` and the head is in this repo (`head.repo?.full_name === \`${ctx.owner}/${ctx.repo}\``): - `resolveRepo(ctx.cwd)`; null → skip. - `checkBaseConflict(root, { remote: "origin", refspecs: [\`+refs/heads/${base}:refs/remotes/origin/${base}\`, \`refs/pull/${n}/head\`], base: \`refs/remotes/origin/${base}\`, head: pr.data.head.sha, signal })`. (`--no-write-fetch-head` is fine: the object is fetched into the odb and referenced by sha.) - Pass the outcome into `formatPr` via a new optional second parameter `formatPr(pr: ApiPull, conflict?: { files: string[] } | { error: string })`, which changes the `mergeable:` line to `mergeable: no — conflicts with <base> in: a.ts, b.ts` or `mergeable: no (conflict check failed: <error>)`. Other `formatPr` callers unchanged. If the local check returns zero files (Forgejo stale), print `mergeable: no (Forgejo); a local merge of the current heads is clean`. - Description: replace `mergeable/merged/draft` with `mergeable/merged/draft (when not mergeable, the conflicting files)`. #### 5. Seed steering `apps/server/src/forgejo/conversation-detect.ts`: - `buildImplementerSeed`: append a paragraph: `Before you finish: fetch the base branch and make sure your PR does not conflict with it (git_push and forgejo_create_pr report conflicts). If it does, rebase onto origin/<base> with git_rebase, resolve the conflicts, and git_push force_after_rebase:true.` with `<base>` = `t.baseBranch ?? "the default branch"`. - `buildReviewSubmittedSeed` Task line: after `...Address the feedback: change the code, commit, push.` insert `Then check the PR is not in conflict with ${pr?.base?.ref ?? "its base"} (forgejo_read_pr lists conflicting files; git_push reports them); if it is, rebase onto origin/<base> with git_rebase and git_push force_after_rebase:true.` Replace the final sentence with `If the verdict is approved and there is no feedback, post no comment; push only if you had to resolve conflicts with the base.` ### Critical files & anchors - `apps/server/src/bridge/git-tools/push.ts` — `buildPushTool` execute tail (lines ~78–97) and non-ff message. - `apps/server/src/forgejo/tools/prs.ts` — `readPr` (~186), `createPr` (~323). - `apps/server/src/forgejo/tools/format.ts` — `formatPr` mergeable line (~84). - `apps/server/src/forgejo/conversation-detect.ts` — `buildImplementerSeed` (~214), `buildReviewSubmittedSeed` task line (~263). - `packages/protocol/src/git-tools.ts` / `forgejo-tools.ts` — result types. ### Verification Run from `/workspace/omp-deck` with `bun test <path>`. - New `apps/server/src/bridge/git-tools/base-conflict.test.ts` (temp repos as in `rebase.test.ts`: a bare remote + clone): main and feature both edit `a.txt` differently, feature pushed → `checkBaseConflict` returns `{ ok: true, files: ["a.txt"] }`; non-overlapping edits → `files: []`; nonexistent refspec → `ok: false`. Assert the clone's HEAD, index and working tree are unchanged after the call. - `apps/server/src/bridge/git-tools/rebase.test.ts`: existing push tests pass with `buildPushTool(repo, null)`; add one case asserting the non-ff rejection text contains `git_rebase action:"start" onto:"origin/<branch>"`. - `apps/server/src/forgejo/conversation-detect.test.ts`: implementer seed contains `rebase onto origin/main` for `baseBranch: "main"`; review-submitted seed contains `push only if you had to resolve conflicts`. - `apps/server/src/forgejo/tools/format.test.ts`: `formatPr({ ...pr, mergeable: false }, { files: ["a.ts"] })` contains `mergeable: no — conflicts with main in: a.ts`; existing `mergeable: no` assertion still matches. - Manual end-to-end on this Forgejo instance: create a branch editing a line that `main` also changed, `git_push` → result JSON has `baseConflict.files` with that path; `forgejo_create_pr` → text contains `WARNING: ... conflicts with main`; `forgejo_read_pr` → `mergeable: no — conflicts with main in: ...`. Then `git_rebase` onto `origin/main`, resolve, `git_push force_after_rebase:true` → no `baseConflict`. - `bun run typecheck` (or the repo's equivalent script in root `package.json`) passes for server, protocol and web. ### Assumptions & contingencies - Forgejo exposes `refs/pull/<n>/head` over git fetch (Gitea-compatible). If the fetch fails in the manual check, fall back in step 4 to fetching `+refs/heads/<head.ref>:refs/remotes/origin/<head.ref>` and using that ref as `head`. - If the open-PR listing filter in step 2 cannot match because `head.repo` is absent in this Forgejo's payload, match on `head.ref === branch` alone.