Forgejo: re-check agent PRs for conflicts when their base branch is pushed #89

New issue

Closed

opened 2026-10-26 16:03:41 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-10-26 16:03:41 +01:00

Collaborator

Problem

Today the user has to tell the agent manually that its pull requests have conflicts. When commits land on a branch that open agent-authored PRs target (e.g. a merge into main), the server should check those PRs for conflicts and, when one conflicts, run a turn in the PR's implementer session so the agent rebases.

Decisions:

Source event: Forgejo push (pushes to a PR's head already arrive as pull_request_sync; pushes/merges to a base branch only arrive as push). Pushes by the agent's own account are processed too (its merges move the base for other PRs).
Scope: open PRs authored by the agent whose base is the pushed branch. If no usable session is mapped, an implementer session is created (same as for a review on the agent's PR).
Detection: read Forgejo mergeable once when the event is handled; anything but false → nothing to do. On false, confirm with a local git merge-tree check in the session's checkout; clean → nothing to do.
Repeats: every base push re-checks; each confirmed conflict runs a turn (FIFO per PR, so a check queued behind the agent's fix drops as clean).

Plan

Approach

Steps in order; each leaves the tree building.

Protocol types — packages/protocol/src/index.ts, ForgejoTriggerSummary: add "push" to event, add "pushed" | "base_pushed" to action, and extend the commentAuthor doc: "the pusher for pushed / base_pushed". pushed = the repo-level push row (number 0, isPull false); base_pushed = one per-PR check row derived from it. No migration: the columns are untyped TEXT.
Classify push deliveries — apps/server/src/forgejo/webhook.ts:
- In classifyForgejoDelivery, before the review/comment branches, if (eventType === "push") return classifyPush(payload);. (resolveEventType already falls back to X-Forgejo-Event, which is push.)
- New classifyPush(payload): not object → malformed "payload is not an object". ref must be a string starting with refs/heads/, else { kind: "ignore", reason: "not a branch push" }. after equal to 40 zeros (or /^0+$/) → ignore "branch deleted". sender.login non-empty string else malformed "missing sender.login" (do NOT ignore the agent as self-authored). repository.full_name validated as in classifyLifecycle (same malformed message). Accept: { dedupeKey: sha256(JSON.stringify(["pushed", payload])), event: "push", action: "pushed", repo, number: 0, isPull: false, commentAuthor: sender }.
- Update the module header comment with a bullet describing push handling.
Dispatch API: mergeable + open PR listing — apps/server/src/forgejo/dispatch-api.ts:
- PullSummary gains mergeable: boolean | null (RawPull.mergeable?: boolean | null; map raw.mergeable ?? null) and baseBranch: string (raw.base?.ref ?? ""; add ref?: string to RawPull.base).
- New DispatchApi.listOpenPulls(owner, repo, signal?): Promise<ApiResult<OpenPull[]>> with export interface OpenPull { number: number; title: string; htmlUrl: string; author: string; headBranch: string; headSha: string; headRepo: string; baseBranch: string; body: string }. Implement with fetchAllPages<RawOpenPull>(client, repoApiPath(owner, repo, "pulls"), { signal, query: { state: "open" } }) — check fetchAllPages's option for query params in apps/server/src/forgejo/tools/shared.ts and use its existing mechanism (if it takes a path only, append ?state=open).
- Update the fake in apps/server/src/forgejo/dispatcher.test.ts (pull default gains mergeable: true, baseBranch: "main"; add openPulls: ApiResult<OpenPull[]> = ok([]) and listOpenPulls() returning it).
Fan out a pushed row — apps/server/src/forgejo/dispatcher.ts:
- In gate, right after the closed/reopened branch: if (row.action === "pushed") { await this.handleBasePush(row); return null; }.
- New private handleBasePush(row) (finishes the row itself, never runs a turn): api/agent missing → finishForgejoTrigger(row.id, "failed", "Forgejo is not configured") and fall back with this.fallback(row, reason, null) (no thread to comment on: number 0; do NOT call surfaceFailure, which posts a comment). Parse payload; branch = ref.slice("refs/heads/".length). listOpenPulls; on error → finish failed with could not list pull requests: <status> <message> + fallback. Filter p.baseBranch === branch && p.author.toLowerCase() === agent.toLowerCase(). For each, insertForgejoTrigger({ deliveryId: row.deliveryId, dedupeKey: sha256(JSON.stringify(["base_pushed", row.id, p.number])), event: "push", action: "base_pushed", repo: row.repo, number: p.number, isPull: true, commentAuthor: row.commentAuthor, payloadJson: JSON.stringify(<synthetic>) }) (the push row's id is the parent identity; StoredForgejoTrigger exposes no dedupe key). Synthetic payload so existing Payload readers work: { ref, before, after, sender: { login: row.commentAuthor }, pull_request: { number, title, body, html_url, user: { login: author }, head: { ref: headBranch, sha: headSha }, base: { ref: branch } } } (copy ref/before/after from the push payload). Finish the push row done with reason queued <n> conflict check(s) for <branch> (n may be 0). Then this.pump() so the new per-PR keys are drained.
- Push rows serialize per repo through artifactKey (repo#0); no change needed.
Gate a base_pushed row — in gate:
- const basePushed = row.action === "base_pushed"; Add "base-pushed" to ReadyReason; set reason = "base-pushed" in the reason chain.
- Include basePushed everywhere synchronized skips work: author taken from payload.pull_request.user.login (line ~344 condition), and skip the write-permission check (a push by anyone with push rights to the base only re-checks the agent's own PR). Do NOT add it to the "no session for this pull request" drop: a missing session is created.
- In the prHead block (already runs because isPull && agentPr): after head checks, if (basePushed && pull.data.mergeable !== false) { finishForgejoTrigger(row.id, "dropped", "mergeable per Forgejo"); return null; }. If pull.data.baseBranch differs from the payload's base.ref, drop with "pull request base changed".
Run a base_pushed row — runReady / resolveSession:
- resolveSession already creates an implementer session when prHead !== null and nothing usable is mapped; base_pushed is not pull-synchronized, so creation is allowed. No change there.
- In runReady, add else if (reason === "base-pushed") seed = await this.baseConflictSeed(ready, handle, created, linkedIssue);. Change the comment A null outcome means pullSyncSeed already failed the row. to A null outcome means the seed builder already finished the row.
- New private baseConflictSeed(ready, handle, created, linkedIssue): Promise<string | null>: base = payload.pull_request.base.ref, head = ready.prHead. Call checkBaseConflict(handle.cwd, { 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} }) (import from ../bridge/git-tools/base-conflict.ts; same refspec pattern as forgejo_create_pr in forgejo/tools/prs.ts). Make it injectable: add checkBaseConflict?: typeof checkBaseConflict to ForgejoDispatcherDeps (doc: "Local merge check for base pushes. Default: base-conflict.ts checkBaseConflict."), used as (this.deps.checkBaseConflict ?? checkBaseConflict). Result !ok → await this.fail(job, could not check for conflicts: ${error}, "internal", handle.sessionId); return null. files.length === 0 → finishForgejoTrigger(row.id, "dropped", "no conflict in a local merge"); return null. Otherwise return buildBaseConflictSeed(...).
- New exported buildBaseConflictSeed(t: { repo: string; number: number; payload: PullSyncPayloadLike; files: string[]; catchUp: boolean; linkedIssue: number | null }) in apps/server/src/forgejo/conversation-detect.ts, next to buildPullSyncSeed. Lines: `${base} moved: your pull request ${repo} #${number} "${title}" now conflicts with it`; `Branch: ${base} ← ${head} · ${html_url}`; `Pushed to ${base} by @${sender}`; Conflicting files: then - <file> per file; when catchUp, the same catch-up sentence shape as buildReviewSubmittedSeed (read with forgejo_read_pr / forgejo_list_pr_comments_and_reviews, plus the linked issue; checkout is detached at the PR head, create local branch <head> with git_branch first); final line Task: ${rebaseInstruction("origin", base)} Post no comment unless the conflict cannot be resolved without input; then explain on the pull request. (import rebaseInstruction from base-conflict.ts).
- finishForgejoTrigger(row.id, "done", reason) already records base-pushed on success.
Docs — docs/configuration.md line ~231: add Push to the webhook's custom events and one sentence: pushes to a branch re-check the agent's open PRs targeting it for conflicts and prompt the PR's session when one conflicts.

Critical files & anchors

apps/server/src/forgejo/webhook.ts — classifyForgejoDelivery, new classifyPush.
apps/server/src/forgejo/dispatcher.ts — gate (~290–401), runReady (~404), new handleBasePush, baseConflictSeed.
apps/server/src/forgejo/dispatch-api.ts — PullSummary, RawPull, new listOpenPulls.
apps/server/src/bridge/git-tools/base-conflict.ts — checkBaseConflict, rebaseInstruction (reuse, unchanged).
apps/server/src/forgejo/conversation-detect.ts — new buildBaseConflictSeed.

Verification

Run from /workspace/omp-deck (check package.json for the exact test script; bun test apps/server/src/forgejo works for the server package):

webhook.test.ts: push to refs/heads/main by the agent → accept { event: "push", action: "pushed", number: 0, isPull: false, commentAuthor: <agent> }; refs/tags/v1 → ignore not a branch push; after all zeros → ignore branch deleted; missing sender.login → malformed.
dispatcher.test.ts (existing fake API/bridge patterns around line 1340):
- A pushed row with openPulls = [agent PR #5 base main, agent PR #6 base dev, human PR #7 base main] → push row done queued 1 conflict check(s) for main; exactly one base_pushed row for #5.
- base_pushed with pull.mergeable: true → dropped mergeable per Forgejo, no turn.
- mergeable: false, injected checkBaseConflict → { ok: true, files: [] } → dropped no conflict in a local merge.
- mergeable: false, files ["a.ts"], mapped implementer session → one prompt containing a.ts and git_rebase; row done base-pushed.
- Same with no mapped session → implementer session created, seed contains the catch-up text, mapping set for #5.
- checkBaseConflict → { ok: false } → row failed, failure comment posted.
Manual: with the webhook's Push event enabled, merge a PR into main that conflicts with an open agent PR; the deck's Forgejo settings panel shows a push/pushed row done plus a base_pushed row done, and the agent's PR session receives the conflict turn.

Assumptions & contingencies

Forgejo's API mergeable may read false while the PR is still being checked right after a push (unverified — confirm against the instance); the local merge-tree check filters those out, so no polling.
If fetchAllPages cannot pass query params, append ?state=open to the path; if Forgejo ignores it, filter state === "open" client-side as well.

## Problem Today the user has to tell the agent manually that its pull requests have conflicts. When commits land on a branch that open agent-authored PRs target (e.g. a merge into `main`), the server should check those PRs for conflicts and, when one conflicts, run a turn in the PR's implementer session so the agent rebases. Decisions: - Source event: Forgejo `push` (pushes to a PR's head already arrive as `pull_request_sync`; pushes/merges to a base branch only arrive as `push`). Pushes by the agent's own account are processed too (its merges move the base for other PRs). - Scope: open PRs authored by the agent whose base is the pushed branch. If no usable session is mapped, an implementer session is created (same as for a review on the agent's PR). - Detection: read Forgejo `mergeable` once when the event is handled; anything but `false` → nothing to do. On `false`, confirm with a local `git merge-tree` check in the session's checkout; clean → nothing to do. - Repeats: every base push re-checks; each confirmed conflict runs a turn (FIFO per PR, so a check queued behind the agent's fix drops as clean). ## Plan ### Approach Steps in order; each leaves the tree building. 1. **Protocol types** — `packages/protocol/src/index.ts`, `ForgejoTriggerSummary`: add `"push"` to `event`, add `"pushed" | "base_pushed"` to `action`, and extend the `commentAuthor` doc: "the pusher for `pushed` / `base_pushed`". `pushed` = the repo-level push row (`number` 0, `isPull` false); `base_pushed` = one per-PR check row derived from it. No migration: the columns are untyped `TEXT`. 2. **Classify `push` deliveries** — `apps/server/src/forgejo/webhook.ts`: - In `classifyForgejoDelivery`, before the review/comment branches, `if (eventType === "push") return classifyPush(payload);`. (`resolveEventType` already falls back to `X-Forgejo-Event`, which is `push`.) - New `classifyPush(payload)`: not object → malformed `"payload is not an object"`. `ref` must be a string starting with `refs/heads/`, else `{ kind: "ignore", reason: "not a branch push" }`. `after` equal to 40 zeros (or `/^0+$/`) → ignore `"branch deleted"`. `sender.login` non-empty string else malformed `"missing sender.login"` (do NOT ignore the agent as self-authored). `repository.full_name` validated as in `classifyLifecycle` (same malformed message). Accept: `{ dedupeKey: sha256(JSON.stringify(["pushed", payload])), event: "push", action: "pushed", repo, number: 0, isPull: false, commentAuthor: sender }`. - Update the module header comment with a bullet describing push handling. 3. **Dispatch API: mergeable + open PR listing** — `apps/server/src/forgejo/dispatch-api.ts`: - `PullSummary` gains `mergeable: boolean | null` (`RawPull.mergeable?: boolean | null`; map `raw.mergeable ?? null`) and `baseBranch: string` (`raw.base?.ref ?? ""`; add `ref?: string` to `RawPull.base`). - New `DispatchApi.listOpenPulls(owner, repo, signal?): Promise<ApiResult<OpenPull[]>>` with `export interface OpenPull { number: number; title: string; htmlUrl: string; author: string; headBranch: string; headSha: string; headRepo: string; baseBranch: string; body: string }`. Implement with `fetchAllPages<RawOpenPull>(client, repoApiPath(owner, repo, "pulls"), { signal, query: { state: "open" } })` — check `fetchAllPages`'s option for query params in `apps/server/src/forgejo/tools/shared.ts` and use its existing mechanism (if it takes a path only, append `?state=open`). - Update the fake in `apps/server/src/forgejo/dispatcher.test.ts` (`pull` default gains `mergeable: true, baseBranch: "main"`; add `openPulls: ApiResult<OpenPull[]> = ok([])` and `listOpenPulls()` returning it). 4. **Fan out a `pushed` row** — `apps/server/src/forgejo/dispatcher.ts`: - In `gate`, right after the closed/reopened branch: `if (row.action === "pushed") { await this.handleBasePush(row); return null; }`. - New private `handleBasePush(row)` (finishes the row itself, never runs a turn): api/agent missing → `finishForgejoTrigger(row.id, "failed", "Forgejo is not configured")` and fall back with `this.fallback(row, reason, null)` (no thread to comment on: number 0; do NOT call `surfaceFailure`, which posts a comment). Parse payload; `branch = ref.slice("refs/heads/".length)`. `listOpenPulls`; on error → finish `failed` with `could not list pull requests: <status> <message>` + `fallback`. Filter `p.baseBranch === branch && p.author.toLowerCase() === agent.toLowerCase()`. For each, `insertForgejoTrigger({ deliveryId: row.deliveryId, dedupeKey: sha256(JSON.stringify(["base_pushed", row.id, p.number])), event: "push", action: "base_pushed", repo: row.repo, number: p.number, isPull: true, commentAuthor: row.commentAuthor, payloadJson: JSON.stringify(<synthetic>) })` (the push row's id is the parent identity; `StoredForgejoTrigger` exposes no dedupe key). Synthetic payload so existing `Payload` readers work: `{ ref, before, after, sender: { login: row.commentAuthor }, pull_request: { number, title, body, html_url, user: { login: author }, head: { ref: headBranch, sha: headSha }, base: { ref: branch } } }` (copy `ref`/`before`/`after` from the push payload). Finish the push row `done` with reason `queued <n> conflict check(s) for <branch>` (n may be 0). Then `this.pump()` so the new per-PR keys are drained. - Push rows serialize per repo through `artifactKey` (`repo#0`); no change needed. 5. **Gate a `base_pushed` row** — in `gate`: - `const basePushed = row.action === "base_pushed";` Add `"base-pushed"` to `ReadyReason`; set `reason = "base-pushed"` in the reason chain. - Include `basePushed` everywhere `synchronized` skips work: author taken from `payload.pull_request.user.login` (line ~344 condition), and skip the write-permission check (a push by anyone with push rights to the base only re-checks the agent's own PR). Do NOT add it to the "no session for this pull request" drop: a missing session is created. - In the `prHead` block (already runs because `isPull && agentPr`): after head checks, `if (basePushed && pull.data.mergeable !== false) { finishForgejoTrigger(row.id, "dropped", "mergeable per Forgejo"); return null; }`. If `pull.data.baseBranch` differs from the payload's `base.ref`, drop with `"pull request base changed"`. 6. **Run a `base_pushed` row** — `runReady` / `resolveSession`: - `resolveSession` already creates an implementer session when `prHead !== null` and nothing usable is mapped; `base_pushed` is not `pull-synchronized`, so creation is allowed. No change there. - In `runReady`, add `else if (reason === "base-pushed") seed = await this.baseConflictSeed(ready, handle, created, linkedIssue);`. Change the comment `A null outcome means pullSyncSeed already failed the row.` to `A null outcome means the seed builder already finished the row.` - New private `baseConflictSeed(ready, handle, created, linkedIssue): Promise<string | null>`: `base = payload.pull_request.base.ref`, `head = ready.prHead`. Call `checkBaseConflict(handle.cwd, { 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}` })` (import from `../bridge/git-tools/base-conflict.ts`; same refspec pattern as `forgejo_create_pr` in `forgejo/tools/prs.ts`). Make it injectable: add `checkBaseConflict?: typeof checkBaseConflict` to `ForgejoDispatcherDeps` (doc: "Local merge check for base pushes. Default: base-conflict.ts `checkBaseConflict`."), used as `(this.deps.checkBaseConflict ?? checkBaseConflict)`. Result `!ok` → `await this.fail(job, `could not check for conflicts: ${error}`, "internal", handle.sessionId); return null`. `files.length === 0` → `finishForgejoTrigger(row.id, "dropped", "no conflict in a local merge"); return null`. Otherwise return `buildBaseConflictSeed(...)`. - New exported `buildBaseConflictSeed(t: { repo: string; number: number; payload: PullSyncPayloadLike; files: string[]; catchUp: boolean; linkedIssue: number | null })` in `apps/server/src/forgejo/conversation-detect.ts`, next to `buildPullSyncSeed`. Lines: `` `${base} moved: your pull request ${repo} #${number} "${title}" now conflicts with it` ``; `` `Branch: ${base} ← ${head} · ${html_url}` ``; `` `Pushed to ${base} by @${sender}` ``; `Conflicting files:` then `- <file>` per file; when `catchUp`, the same catch-up sentence shape as `buildReviewSubmittedSeed` (read with forgejo_read_pr / forgejo_list_pr_comments_and_reviews, plus the linked issue; checkout is detached at the PR head, create local branch `<head>` with git_branch first); final line `Task: ${rebaseInstruction("origin", base)} Post no comment unless the conflict cannot be resolved without input; then explain on the pull request.` (import `rebaseInstruction` from base-conflict.ts). - `finishForgejoTrigger(row.id, "done", reason)` already records `base-pushed` on success. 7. **Docs** — `docs/configuration.md` line ~231: add **Push** to the webhook's custom events and one sentence: pushes to a branch re-check the agent's open PRs targeting it for conflicts and prompt the PR's session when one conflicts. ### Critical files & anchors - `apps/server/src/forgejo/webhook.ts` — `classifyForgejoDelivery`, new `classifyPush`. - `apps/server/src/forgejo/dispatcher.ts` — `gate` (~290–401), `runReady` (~404), new `handleBasePush`, `baseConflictSeed`. - `apps/server/src/forgejo/dispatch-api.ts` — `PullSummary`, `RawPull`, new `listOpenPulls`. - `apps/server/src/bridge/git-tools/base-conflict.ts` — `checkBaseConflict`, `rebaseInstruction` (reuse, unchanged). - `apps/server/src/forgejo/conversation-detect.ts` — new `buildBaseConflictSeed`. ### Verification Run from `/workspace/omp-deck` (check `package.json` for the exact test script; `bun test apps/server/src/forgejo` works for the server package): - `webhook.test.ts`: `push` to `refs/heads/main` by the agent → accept `{ event: "push", action: "pushed", number: 0, isPull: false, commentAuthor: <agent> }`; `refs/tags/v1` → ignore `not a branch push`; `after` all zeros → ignore `branch deleted`; missing `sender.login` → malformed. - `dispatcher.test.ts` (existing fake API/bridge patterns around line 1340): - A `pushed` row with `openPulls` = [agent PR #5 base `main`, agent PR #6 base `dev`, human PR #7 base `main`] → push row `done` `queued 1 conflict check(s) for main`; exactly one `base_pushed` row for #5. - `base_pushed` with `pull.mergeable: true` → dropped `mergeable per Forgejo`, no turn. - `mergeable: false`, injected `checkBaseConflict` → `{ ok: true, files: [] }` → dropped `no conflict in a local merge`. - `mergeable: false`, files `["a.ts"]`, mapped implementer session → one prompt containing `a.ts` and `git_rebase`; row `done` `base-pushed`. - Same with no mapped session → implementer session created, seed contains the catch-up text, mapping set for #5. - `checkBaseConflict` → `{ ok: false }` → row `failed`, failure comment posted. - Manual: with the webhook's **Push** event enabled, merge a PR into `main` that conflicts with an open agent PR; the deck's Forgejo settings panel shows a `push/pushed` row `done` plus a `base_pushed` row `done`, and the agent's PR session receives the conflict turn. ### Assumptions & contingencies - Forgejo's API `mergeable` may read `false` while the PR is still being checked right after a push (unverified — confirm against the instance); the local merge-tree check filters those out, so no polling. - If `fetchAllPages` cannot pass query params, append `?state=open` to the path; if Forgejo ignores it, filter `state === "open"` client-side as well.