Forgejo: rest of the PR lifecycle (pushes, merge/close, reopen, issue close) #72

New issue

Closed

opened 2026-10-26 13:55:45 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-10-26 13:55:45 +01:00

Collaborator

Problem

Finish the pull request lifecycle for sessions mapped to a Forgejo issue or pull request (KB project/forgejo-integration-plan.md step 15; design project/forgejo-integration.md, sections "PR synchronized", "PR merged / closed", "Session Lifecycle"). Submitted reviews and inline review comments already trigger sessions; this covers the remaining events:

PR synchronized (new commits pushed): inject into the existing PR session. Harness precondition: fetch the updated branch in the session's clone before the turn. Seed: PR id, title, base/head branch, author, new commits (hash + title). Pushes by the agent itself must not trigger its session. No permission check on the pusher: an existing mapped session suffices. No mapped session → nothing (auto-review on push is out of scope, plan steps 18/19).
PR merged: archive the session and remove its clone — for reviewer sessions and, until post-merge CI exists (plan step 20), for the agent's own PR too.
PR closed without merge: not an agent trigger; quietly archive the session and remove its clone.
PR reopened and issue reopened: unarchive the mapped session, no agent turn; its clone is recreated lazily when the session is next resumed. No mapped session → nothing.
Issue closed: archive the issue's session and remove its clone — except an implementer session that has a PR, which is archived only by its PR's lifecycle (otherwise a merge with Closes #N would archive the implementer through the issue).
If the session to archive is mid-turn (an implementer session spans issue and PR, so per-artifact FIFO doesn't serialize them), the archive waits until the turn ends, bounded by that session kind's turn timeout, then archives regardless.
Reuse the archive behaviour of POST /sessions/:id/archive (apps/server/src/routes.ts): set archived, dispose the live session, remove the clone.

Plan

Context

Today classifyForgejoDelivery (apps/server/src/forgejo/webhook.ts) ignores every pull_request action except review_requested and every issues event, so pushes, merges, closes and reopens never reach ForgejoDispatcher (apps/server/src/forgejo/dispatcher.ts), and clones are only removed when a session is archived or deleted in the deck. End state: these events are queued as forgejo_triggers rows like other triggers; the dispatcher runs a seeded turn for pushes and performs archive/unarchive (no turn) for the others.

Approach

Steps are ordered so the tree builds and existing tests pass after each.

1. Shared archive helper

New file apps/server/src/session-archive.ts exporting:
```
/** Mark `sessionId` archived; if it runs in a per-artifact clone, dispose its live handle and delete the clone (resume re-creates it). Never checks existence. */
export async function archiveSession(bridge: Pick<AgentBridge, "getSession">, sessionId: string): Promise<void>
```
Body = the current route body after the existence check: setArchived(id, true); const checkout = getSessionPlacement(id)?.checkoutPath; if set, await bridge.getSession(id)?.dispose() then await removeCheckout(checkout). Move the existing comment ("The clone is the session's cwd: …") with it.
apps/server/src/routes.ts POST /sessions/:id/archive: keep the 404 existence check, replace the rest of the body with await archiveSession(bridge, id). Drop imports that become unused.

2. Protocol union

In packages/protocol/src/index.ts ForgejoTriggerSummary (≈ line 2038):

event gains "pull_request_sync" and "issues".
action gains "synchronized" | "closed" | "reopened".
Update the commentAuthor doc: "…, the reviewer for reviewed, or the sender (pusher / closer / reopener) for synchronized, closed, reopened."
No web code matches on these literals (apps/web/src has no review_requested/"reviewed" usage), so nothing else changes there.

3. Classifier

In apps/server/src/forgejo/webhook.ts:

Event types: Forgejo sends PR pushes as X-Forgejo-Event: pull_request with X-Forgejo-Event-Type: pull_request_sync, other PR actions with Event-Type pull_request, and issue state changes as issues (unverified — confirm against Forgejo modules/webhook/type.go HookEventPullRequestSync / HookEventIssues before coding; if the Event-Type for pushes is different, use that literal in place of pull_request_sync everywhere in this plan). Accept both pull_request and pull_request_sync for every PR action so the -Event-only fallback of resolveEventType also works.
classifyForgejoDelivery: replace the first branch with: if eventType is pull_request_review_request, pull_request or pull_request_sync → dispatch on payload.action (after the isPlainObject check): review_requested → existing classifyReviewRequest (widen its eventType parameter type to include "pull_request_sync"); synchronized, closed, reopened → new classifyLifecycle(eventType, action, payload, agentUsername, true); anything else → { kind: "ignore", reason: "unsupported action <action>" }. New branch: eventType === "issues" → actions closed/reopened → classifyLifecycle(…, false), else ignore with the same reason format. Update the module doc comment with these Forgejo facts.
New private classifyLifecycle(eventType, action: "synchronized" | "closed" | "reopened", payload, agentUsername, isPull: boolean): WebhookVerdict:
- sender.login required (malformed "missing sender.login" otherwise).
- Only for synchronized: sender equal to the agent (case-insensitive) → { kind: "ignore", reason: "self-authored" }. Close/reopen by the agent are NOT ignored (the agent may merge or close).
- repository.full_name validated exactly as the other classifiers. Number: PR → payload.number ?? payload.pull_request.number; issue → payload.issue.number; same integer/positive validation and error strings (missing or invalid pull_request.number / missing or invalid issue.number).
- Accept with dedupeKey = sha256 of JSON.stringify([action, payload]) (whole-body hash, same convention and comment as review requests), event: eventType, action, repo, number, isPull, commentAuthor: sender.

4. Checkout refresh for pushes

In apps/server/src/forgejo/checkout.ts:

Factor the git closure of createCheckout into a module-private runChecked(cwd: string, args: string[], step: string, ssh: …, network = false): Promise<string> (same behaviour: network calls get NETWORK_TIMEOUT_MS and the deck SSH env, failures throw withHostKeyHint(gitErrorText(…))), and use it in createCheckout.

New export:

export interface PushedCommit { sha: string; title: string }
/**
 * Bring a session's clone up to date with pull request `number` before a push trigger's turn and
 * list the commits that arrived. "detached" (reviewer clones): fetch refs/pull/<n>/head and move the
 * detached HEAD there when the working tree is clean. "branch" (implementer clones): `git fetch
 * --prune origin` only; local branches are left to the agent. `commits` is null when the previous
 * head is unknown. Throws with git's message.
 */
export async function refreshPullCheckout(dir: string, opts: { number: number; headBranch: string; mode: "detached" | "branch" }): Promise<{ commits: PushedCommit[] | null; movedHead: boolean }>

prepareDeckSsh() as in createCheckout (throw its error if not ok).
detached: old = rev-parse HEAD; fetch origin refs/pull/<n>/head (network); new = rev-parse FETCH_HEAD; if status --porcelain output is empty → checkout --detach FETCH_HEAD, movedHead = true, else movedHead = false.
branch: old = rev-parse --verify -q refs/remotes/origin/<headBranch> run via runGit directly (non-zero exit → old = null, not an error); fetch --prune origin (network); new = rev-parse refs/remotes/origin/<headBranch>; movedHead = false.
commits: old === null → null; else log --reverse --format=%H%x09%s <old>..<new> parsed into { sha, title } per non-empty line (split on the first tab).

5. Dispatcher: lifecycle (archive / unarchive, no turn)

In apps/server/src/forgejo/dispatcher.ts:

Deps: add archiveSession?: typeof archiveSession (default: the step-1 helper) and refreshPullCheckout?: typeof refreshPullCheckout (default: step 4), mirroring the existing createCheckout? override. Extend bridge Pick only if needed (getSession is already there).
gate: as the very first statement, if row.action === "closed" || row.action === "reopened" → await this.handleLifecycle(row); return null; (before the Forgejo-config check: these need no API).
New private handleLifecycle(row: StoredForgejoTrigger): Promise<void>; every path finishes the row:
1. mapped = getForgejoSession(row.repo, row.number); null → finishForgejoTrigger(row.id, "dropped", "no session is mapped to this artifact").
2. trigger = getSessionTrigger(mapped) (catch → null); null → dropped "the mapped session has no session trigger".
3. reopened: if getArchivedMap([mapped]).has(mapped) → setArchived(mapped, false), done "session unarchived"; else done "session already active".
4. closed, issue (!row.isPull): if trigger.kind === "forgejo_implementer" and it has a PR — trigger.scope.pull !== undefined || listForgejoSessionNumbers(row.repo, mapped).some((n) => n !== row.number) → dropped "an implementer session with a pull request is archived by its pull request". Otherwise archive (below), done "archived: issue closed".
5. closed, PR: archive, done "archived: merged" when payload.pull_request?.merged === true, else "archived: closed without merge". This applies to every session kind, including the agent's own PR (implementer), until post-merge CI (plan step 20) replaces it.
- Archive = await this.waitUntilIdle(mapped, timeoutMs) with timeoutMs = this.deps.turnTimeoutMs ?? SESSION_POLICIES[trigger.kind].turnTimeoutMs, then await (this.deps.archiveSession ?? archiveSession)(this.deps.bridge, mapped).
- Errors thrown here propagate to gateSafely → failInternal (existing behaviour).
New private waitUntilIdle(sessionId: string, timeoutMs: number): Promise<void>: handle = this.deps.bridge.getSession(sessionId); if absent or !handle.isStreamingNow() resolve immediately; else subscribe and resolve on a terminal agent_end once !handle.isStreamingNow() (same setTimeout(…, 0) re-check as runTurn), or after timeoutMs; always unsubscribe and clear the timer. Archive proceeds regardless of which fired.

6. Dispatcher: push turn (`synchronized`)

ReadyReason gains "pull-synchronized". Payload gets pull_request?.merged?: boolean via a new PullSyncPayloadLike (step 7) intersection.
gate, with const synchronized = row.action === "synchronized":
- reason: synchronized → "pull-synchronized" (no detect).
- Right after the reason is chosen: if synchronized and getForgejoSession(row.repo, row.number) === null → dropped "no session for this pull request".
- The sender/commentAuthor check stays (passes by construction).
- author: use payload.pull_request?.user?.login for synchronized too (add it to the reviewRequest || reviewSubmitted condition).
- Skip the permission check when synchronized (wrap the checkPermission block in if (!synchronized)).
- Workspace matching and the own-PR readPull/prHead block are unchanged (they then supply prHead for implementer sessions).
resolveSession: when ready.reason === "pull-synchronized" and reuseMapped returns a string (or mapping vanished), throw new Error(\the mapped session is unusable: ${reason}`)` instead of creating a new session.
runReady, synchronized branch (before building the seed): call (this.deps.refreshPullCheckout ?? refreshPullCheckout)(handle.cwd, { number: row.number, headBranch: payload.pull_request?.head?.ref ?? ready.prHead ?? "", mode: kind === "forgejo_implementer" ? "branch" : "detached" }). Empty headBranch in branch mode → await this.fail(job, "the pull request has no head branch", "forgejo", handle.sessionId) and return (dispose as the finally already does — place the refresh inside the existing try whose finally disposes, i.e. restructure so refresh + turn share it). A thrown git error → await this.fail(job, \could not update the checkout: ${msg}`, "internal", handle.sessionId). Then seed = buildPullSyncSeed({ repo: row.repo, number: row.number, payload, commits, movedHead, implementer: kind === "forgejo_implementer" }). Row finished donewith reason"pull-synchronized"` on success (existing path).

7. Push seed

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

export interface PullSyncPayloadLike extends ReviewRequestPayloadLike { pull_request?: ReviewRequestPayloadLike["pull_request"] & { merged?: boolean } }.
export function buildPullSyncSeed(t: { repo: string; number: number; payload: PullSyncPayloadLike; commits: { sha: string; title: string }[] | null; movedHead: boolean; implementer: boolean }): string — lines joined by \n:
1. New commits pushed to ${repo} PR #${n}: "${title ?? "?"}"
2. Author: @${pr.user.login ?? "?"} · ${base ?? "?"} ← ${head ?? "?"} (${head.sha?.slice(0,7) ?? "?"}) · ${html_url ?? "?"}
3. Pushed by @${sender ?? "?"}
4. New commits: then one - ${sha.slice(0, 7)} ${title} per commit; commits === null → - unknown: the previous head is not known in this checkout; empty list → - none found in the checkout (it may have been re-created); compare against your last review.
5. Task line — reviewer (!implementer): ${movedHead ? "The checkout is detached at the new head." : "The checkout has local changes, so HEAD was not moved; the new head is FETCH_HEAD."} Task: continue your review of this pull request: inspect what the new commits change and post a review with forgejo_post_review covering them. Post nothing if they don't affect your review. — implementer: Someone else pushed to your pull request branch. Task: integrate origin/${head} into your local branch ${head} (git_rebase action:"start" onto:"origin/${head}") before committing further, so your next git_push doesn't conflict. Reply on the pull request only if the push needs a response.

Critical files & anchors

apps/server/src/forgejo/webhook.ts — classifyForgejoDelivery first branch (≈ line 84): routes all pull_request events to classifyReviewRequest today.
apps/server/src/forgejo/dispatcher.ts — gate (≈ 271), runReady (≈ 369), resolveSession (≈ 439), isUsableTriggerSession (≈ 780).
apps/server/src/bridge/session-policy.ts — implementerPrNumbers (≈ 258) shows how an implementer's PRs are derived (scope.pull + mapped numbers); step 5.4 mirrors it.
apps/server/src/bridge/in-process.ts resumeSession (≈ 320) — restoreCheckout recreates a removed clone on resume, which is what makes "unarchive only" on reopen sufficient.

Verification

Run from apps/server (existing test command for the package; same as for the current *.test.ts).

webhook.test.ts, new cases with the existing classify(overrides) factory:
- Event-Type pull_request_sync, action synchronized, sender alice → accept, action: "synchronized", isPull: true, commentAuthor: "alice"; same with sender deck-bot → ignore "self-authored".
- pull_request action closed sender deck-bot → accept (not self-ignored).
- issues action closed → accept with isPull: false, number from issue.number; issues action opened → ignore "unsupported action opened".
- pull_request action edited → ignore "unsupported action edited".
dispatcher.test.ts, new describe("ForgejoDispatcher lifecycle") using bootDb, harness, insertSessionTrigger, setForgejoSession, a fake archiveSession and fake refreshPullCheckout dep recording calls:
- PR closed with merged: true on a mapped forgejo_pull_request session → archive called with that id, row done "archived: merged", no prompt.
- PR closed unmerged on a mapped implementer session → archived, "archived: closed without merge".
- Issue closed on implementer session mapped to #4 and #7 → not archived, row dropped with the step-5.4 reason; issue closed on a forgejo_conversation session → archived.
- Reopened on an archived mapped session → getArchivedMap no longer has it, row done "session unarchived", no prompt.
- Lifecycle on an unmapped artifact → dropped "no session is mapped to this artifact".
- Archive while the mapped FakeHandle is streaming: archive not called until the handle emits terminal agent_end (use the existing manual-mode harness), then called.
- synchronized on a mapped reviewer session → refresh called with mode: "detached", one prompt whose text contains New commits pushed to lucas/omp-deck PR #7 and - abc1234 Fix x (fake returns [{ sha: "abc1234…", title: "Fix x" }]), and no checkPermission call; on the agent's own PR mapped to an implementer session → mode: "branch" and seed contains integrate origin/<head>; unmapped → dropped "no session for this pull request", no session created.
checkout.test.ts (follow its real-git fixture pattern): refreshPullCheckout in detached mode on a clone whose remote gained a commit on refs/pull/<n>/head → returns that commit and moves HEAD; with a dirty file → movedHead: false, HEAD unchanged.
Manual end-to-end on a connected test repo: push a commit to a PR that has a reviewer session → session gets the push turn; merge it → session shows archived in the deck and <dataDir>/checkouts/<owner>/<repo>/pr-<n> is gone; reopen a closed PR → session unarchived.

Assumptions & contingencies

Merging the agent's own PR archives its implementer session immediately; plan step 20 (post-merge CI) will change this.
Issue reopened is handled like PR reopened, although the brief only named PR reopen.
If Forgejo's push payload Event-Type differs from pull_request_sync, substitute the real literal (step 3); if pull_request.merged is absent on closed payloads, use payload.pull_request.merged_at != null instead (unverified — confirm first).

## Problem Finish the pull request lifecycle for sessions mapped to a Forgejo issue or pull request (KB `project/forgejo-integration-plan.md` step 15; design `project/forgejo-integration.md`, sections "PR synchronized", "PR merged / closed", "Session Lifecycle"). Submitted reviews and inline review comments already trigger sessions; this covers the remaining events: - **PR `synchronized`** (new commits pushed): inject into the existing PR session. Harness precondition: fetch the updated branch in the session's clone before the turn. Seed: PR id, title, base/head branch, author, new commits (hash + title). Pushes by the agent itself must not trigger its session. No permission check on the pusher: an existing mapped session suffices. No mapped session → nothing (auto-review on push is out of scope, plan steps 18/19). - **PR merged**: archive the session and remove its clone — for reviewer sessions *and*, until post-merge CI exists (plan step 20), for the agent's own PR too. - **PR closed without merge**: not an agent trigger; quietly archive the session and remove its clone. - **PR reopened** and **issue reopened**: unarchive the mapped session, no agent turn; its clone is recreated lazily when the session is next resumed. No mapped session → nothing. - **Issue closed**: archive the issue's session and remove its clone — except an implementer session that has a PR, which is archived only by its PR's lifecycle (otherwise a merge with `Closes #N` would archive the implementer through the issue). - If the session to archive is mid-turn (an implementer session spans issue and PR, so per-artifact FIFO doesn't serialize them), the archive waits until the turn ends, bounded by that session kind's turn timeout, then archives regardless. - Reuse the archive behaviour of `POST /sessions/:id/archive` (`apps/server/src/routes.ts`): set archived, dispose the live session, remove the clone. ## Plan ### Context Today `classifyForgejoDelivery` (`apps/server/src/forgejo/webhook.ts`) ignores every `pull_request` action except `review_requested` and every `issues` event, so pushes, merges, closes and reopens never reach `ForgejoDispatcher` (`apps/server/src/forgejo/dispatcher.ts`), and clones are only removed when a session is archived or deleted in the deck. End state: these events are queued as `forgejo_triggers` rows like other triggers; the dispatcher runs a seeded turn for pushes and performs archive/unarchive (no turn) for the others. ### Approach Steps are ordered so the tree builds and existing tests pass after each. #### 1. Shared archive helper - New file `apps/server/src/session-archive.ts` exporting: ```ts /** Mark `sessionId` archived; if it runs in a per-artifact clone, dispose its live handle and delete the clone (resume re-creates it). Never checks existence. */ export async function archiveSession(bridge: Pick<AgentBridge, "getSession">, sessionId: string): Promise<void> ``` Body = the current route body after the existence check: `setArchived(id, true)`; `const checkout = getSessionPlacement(id)?.checkoutPath`; if set, `await bridge.getSession(id)?.dispose()` then `await removeCheckout(checkout)`. Move the existing comment ("The clone is the session's cwd: …") with it. - `apps/server/src/routes.ts` `POST /sessions/:id/archive`: keep the 404 existence check, replace the rest of the body with `await archiveSession(bridge, id)`. Drop imports that become unused. #### 2. Protocol union In `packages/protocol/src/index.ts` `ForgejoTriggerSummary` (≈ line 2038): - `event` gains `"pull_request_sync"` and `"issues"`. - `action` gains `"synchronized" | "closed" | "reopened"`. - Update the `commentAuthor` doc: "…, the reviewer for `reviewed`, or the sender (pusher / closer / reopener) for `synchronized`, `closed`, `reopened`." No web code matches on these literals (`apps/web/src` has no `review_requested`/`"reviewed"` usage), so nothing else changes there. #### 3. Classifier In `apps/server/src/forgejo/webhook.ts`: - Event types: Forgejo sends PR pushes as `X-Forgejo-Event: pull_request` with `X-Forgejo-Event-Type: pull_request_sync`, other PR actions with Event-Type `pull_request`, and issue state changes as `issues` (unverified — confirm against Forgejo `modules/webhook/type.go` `HookEventPullRequestSync` / `HookEventIssues` before coding; if the Event-Type for pushes is different, use that literal in place of `pull_request_sync` everywhere in this plan). Accept both `pull_request` and `pull_request_sync` for every PR action so the `-Event`-only fallback of `resolveEventType` also works. - `classifyForgejoDelivery`: replace the first branch with: if `eventType` is `pull_request_review_request`, `pull_request` or `pull_request_sync` → dispatch on `payload.action` (after the `isPlainObject` check): `review_requested` → existing `classifyReviewRequest` (widen its `eventType` parameter type to include `"pull_request_sync"`); `synchronized`, `closed`, `reopened` → new `classifyLifecycle(eventType, action, payload, agentUsername, true)`; anything else → `{ kind: "ignore", reason: "unsupported action <action>" }`. New branch: `eventType === "issues"` → actions `closed`/`reopened` → `classifyLifecycle(…, false)`, else ignore with the same reason format. Update the module doc comment with these Forgejo facts. - New private `classifyLifecycle(eventType, action: "synchronized" | "closed" | "reopened", payload, agentUsername, isPull: boolean): WebhookVerdict`: - `sender.login` required (malformed `"missing sender.login"` otherwise). - Only for `synchronized`: sender equal to the agent (case-insensitive) → `{ kind: "ignore", reason: "self-authored" }`. Close/reopen by the agent are NOT ignored (the agent may merge or close). - `repository.full_name` validated exactly as the other classifiers. Number: PR → `payload.number ?? payload.pull_request.number`; issue → `payload.issue.number`; same integer/positive validation and error strings (`missing or invalid pull_request.number` / `missing or invalid issue.number`). - Accept with `dedupeKey` = sha256 of `JSON.stringify([action, payload])` (whole-body hash, same convention and comment as review requests), `event: eventType`, `action`, `repo`, `number`, `isPull`, `commentAuthor: sender`. #### 4. Checkout refresh for pushes In `apps/server/src/forgejo/checkout.ts`: - Factor the `git` closure of `createCheckout` into a module-private `runChecked(cwd: string, args: string[], step: string, ssh: …, network = false): Promise<string>` (same behaviour: network calls get `NETWORK_TIMEOUT_MS` and the deck SSH env, failures throw `withHostKeyHint(gitErrorText(…))`), and use it in `createCheckout`. - New export: ```ts export interface PushedCommit { sha: string; title: string } /** * Bring a session's clone up to date with pull request `number` before a push trigger's turn and * list the commits that arrived. "detached" (reviewer clones): fetch refs/pull/<n>/head and move the * detached HEAD there when the working tree is clean. "branch" (implementer clones): `git fetch * --prune origin` only; local branches are left to the agent. `commits` is null when the previous * head is unknown. Throws with git's message. */ export async function refreshPullCheckout(dir: string, opts: { number: number; headBranch: string; mode: "detached" | "branch" }): Promise<{ commits: PushedCommit[] | null; movedHead: boolean }> ``` - `prepareDeckSsh()` as in `createCheckout` (throw its error if not ok). - detached: `old = rev-parse HEAD`; `fetch origin refs/pull/<n>/head` (network); `new = rev-parse FETCH_HEAD`; if `status --porcelain` output is empty → `checkout --detach FETCH_HEAD`, `movedHead = true`, else `movedHead = false`. - branch: `old = rev-parse --verify -q refs/remotes/origin/<headBranch>` run via `runGit` directly (non-zero exit → `old = null`, not an error); `fetch --prune origin` (network); `new = rev-parse refs/remotes/origin/<headBranch>`; `movedHead = false`. - commits: `old === null` → null; else `log --reverse --format=%H%x09%s <old>..<new>` parsed into `{ sha, title }` per non-empty line (split on the first tab). #### 5. Dispatcher: lifecycle (archive / unarchive, no turn) In `apps/server/src/forgejo/dispatcher.ts`: - Deps: add `archiveSession?: typeof archiveSession` (default: the step-1 helper) and `refreshPullCheckout?: typeof refreshPullCheckout` (default: step 4), mirroring the existing `createCheckout?` override. Extend `bridge` Pick only if needed (`getSession` is already there). - `gate`: as the very first statement, if `row.action === "closed" || row.action === "reopened"` → `await this.handleLifecycle(row); return null;` (before the Forgejo-config check: these need no API). - New private `handleLifecycle(row: StoredForgejoTrigger): Promise<void>`; every path finishes the row: 1. `mapped = getForgejoSession(row.repo, row.number)`; null → `finishForgejoTrigger(row.id, "dropped", "no session is mapped to this artifact")`. 2. `trigger = getSessionTrigger(mapped)` (catch → null); null → dropped `"the mapped session has no session trigger"`. 3. `reopened`: if `getArchivedMap([mapped]).has(mapped)` → `setArchived(mapped, false)`, done `"session unarchived"`; else done `"session already active"`. 4. `closed`, issue (`!row.isPull`): if `trigger.kind === "forgejo_implementer"` and it has a PR — `trigger.scope.pull !== undefined || listForgejoSessionNumbers(row.repo, mapped).some((n) => n !== row.number)` → dropped `"an implementer session with a pull request is archived by its pull request"`. Otherwise archive (below), done `"archived: issue closed"`. 5. `closed`, PR: archive, done `"archived: merged"` when `payload.pull_request?.merged === true`, else `"archived: closed without merge"`. This applies to every session kind, including the agent's own PR (implementer), until post-merge CI (plan step 20) replaces it. - Archive = `await this.waitUntilIdle(mapped, timeoutMs)` with `timeoutMs = this.deps.turnTimeoutMs ?? SESSION_POLICIES[trigger.kind].turnTimeoutMs`, then `await (this.deps.archiveSession ?? archiveSession)(this.deps.bridge, mapped)`. - Errors thrown here propagate to `gateSafely` → `failInternal` (existing behaviour). - New private `waitUntilIdle(sessionId: string, timeoutMs: number): Promise<void>`: `handle = this.deps.bridge.getSession(sessionId)`; if absent or `!handle.isStreamingNow()` resolve immediately; else subscribe and resolve on a terminal `agent_end` once `!handle.isStreamingNow()` (same `setTimeout(…, 0)` re-check as `runTurn`), or after `timeoutMs`; always unsubscribe and clear the timer. Archive proceeds regardless of which fired. #### 6. Dispatcher: push turn (`synchronized`) - `ReadyReason` gains `"pull-synchronized"`. `Payload` gets `pull_request?.merged?: boolean` via a new `PullSyncPayloadLike` (step 7) intersection. - `gate`, with `const synchronized = row.action === "synchronized"`: - reason: `synchronized` → `"pull-synchronized"` (no `detect`). - Right after the reason is chosen: if synchronized and `getForgejoSession(row.repo, row.number) === null` → dropped `"no session for this pull request"`. - The sender/commentAuthor check stays (passes by construction). - `author`: use `payload.pull_request?.user?.login` for synchronized too (add it to the `reviewRequest || reviewSubmitted` condition). - Skip the permission check when synchronized (wrap the `checkPermission` block in `if (!synchronized)`). - Workspace matching and the own-PR `readPull`/`prHead` block are unchanged (they then supply `prHead` for implementer sessions). - `resolveSession`: when `ready.reason === "pull-synchronized"` and `reuseMapped` returns a string (or mapping vanished), throw `new Error(\`the mapped session is unusable: ${reason}\`)` instead of creating a new session. - `runReady`, synchronized branch (before building the seed): call `(this.deps.refreshPullCheckout ?? refreshPullCheckout)(handle.cwd, { number: row.number, headBranch: payload.pull_request?.head?.ref ?? ready.prHead ?? "", mode: kind === "forgejo_implementer" ? "branch" : "detached" })`. Empty `headBranch` in branch mode → `await this.fail(job, "the pull request has no head branch", "forgejo", handle.sessionId)` and return (dispose as the `finally` already does — place the refresh inside the existing `try` whose `finally` disposes, i.e. restructure so refresh + turn share it). A thrown git error → `await this.fail(job, \`could not update the checkout: ${msg}\`, "internal", handle.sessionId)`. Then `seed = buildPullSyncSeed({ repo: row.repo, number: row.number, payload, commits, movedHead, implementer: kind === "forgejo_implementer" })`. Row finished `done` with reason `"pull-synchronized"` on success (existing path). #### 7. Push seed In `apps/server/src/forgejo/conversation-detect.ts`: - `export interface PullSyncPayloadLike extends ReviewRequestPayloadLike { pull_request?: ReviewRequestPayloadLike["pull_request"] & { merged?: boolean } }`. - `export function buildPullSyncSeed(t: { repo: string; number: number; payload: PullSyncPayloadLike; commits: { sha: string; title: string }[] | null; movedHead: boolean; implementer: boolean }): string` — lines joined by `\n`: 1. `New commits pushed to ${repo} PR #${n}: "${title ?? "?"}"` 2. `Author: @${pr.user.login ?? "?"} · ${base ?? "?"} ← ${head ?? "?"} (${head.sha?.slice(0,7) ?? "?"}) · ${html_url ?? "?"}` 3. `Pushed by @${sender ?? "?"}` 4. `New commits:` then one `- ${sha.slice(0, 7)} ${title}` per commit; `commits === null` → `- unknown: the previous head is not known in this checkout`; empty list → `- none found in the checkout (it may have been re-created); compare against your last review`. 5. Task line — reviewer (`!implementer`): `${movedHead ? "The checkout is detached at the new head." : "The checkout has local changes, so HEAD was not moved; the new head is FETCH_HEAD."} Task: continue your review of this pull request: inspect what the new commits change and post a review with forgejo_post_review covering them. Post nothing if they don't affect your review.` — implementer: `Someone else pushed to your pull request branch. Task: integrate origin/${head} into your local branch ${head} (git_rebase action:"start" onto:"origin/${head}") before committing further, so your next git_push doesn't conflict. Reply on the pull request only if the push needs a response.` ### Critical files & anchors - `apps/server/src/forgejo/webhook.ts` — `classifyForgejoDelivery` first branch (≈ line 84): routes all `pull_request` events to `classifyReviewRequest` today. - `apps/server/src/forgejo/dispatcher.ts` — `gate` (≈ 271), `runReady` (≈ 369), `resolveSession` (≈ 439), `isUsableTriggerSession` (≈ 780). - `apps/server/src/bridge/session-policy.ts` — `implementerPrNumbers` (≈ 258) shows how an implementer's PRs are derived (scope.pull + mapped numbers); step 5.4 mirrors it. - `apps/server/src/bridge/in-process.ts` `resumeSession` (≈ 320) — `restoreCheckout` recreates a removed clone on resume, which is what makes "unarchive only" on reopen sufficient. ### Verification Run from `apps/server` (existing test command for the package; same as for the current `*.test.ts`). - `webhook.test.ts`, new cases with the existing `classify(overrides)` factory: - Event-Type `pull_request_sync`, action `synchronized`, sender `alice` → accept, `action: "synchronized"`, `isPull: true`, `commentAuthor: "alice"`; same with sender `deck-bot` → ignore `"self-authored"`. - `pull_request` action `closed` sender `deck-bot` → accept (not self-ignored). - `issues` action `closed` → accept with `isPull: false`, number from `issue.number`; `issues` action `opened` → ignore `"unsupported action opened"`. - `pull_request` action `edited` → ignore `"unsupported action edited"`. - `dispatcher.test.ts`, new `describe("ForgejoDispatcher lifecycle")` using `bootDb`, `harness`, `insertSessionTrigger`, `setForgejoSession`, a fake `archiveSession` and fake `refreshPullCheckout` dep recording calls: - PR closed with `merged: true` on a mapped `forgejo_pull_request` session → archive called with that id, row `done` `"archived: merged"`, no prompt. - PR closed unmerged on a mapped implementer session → archived, `"archived: closed without merge"`. - Issue closed on implementer session mapped to #4 and #7 → not archived, row `dropped` with the step-5.4 reason; issue closed on a `forgejo_conversation` session → archived. - Reopened on an archived mapped session → `getArchivedMap` no longer has it, row `done` `"session unarchived"`, no prompt. - Lifecycle on an unmapped artifact → `dropped` `"no session is mapped to this artifact"`. - Archive while the mapped `FakeHandle` is streaming: archive not called until the handle emits terminal `agent_end` (use the existing manual-mode harness), then called. - `synchronized` on a mapped reviewer session → refresh called with `mode: "detached"`, one prompt whose text contains `New commits pushed to lucas/omp-deck PR #7` and `- abc1234 Fix x` (fake returns `[{ sha: "abc1234…", title: "Fix x" }]`), and no `checkPermission` call; on the agent's own PR mapped to an implementer session → `mode: "branch"` and seed contains `integrate origin/<head>`; unmapped → `dropped` `"no session for this pull request"`, no session created. - `checkout.test.ts` (follow its real-git fixture pattern): `refreshPullCheckout` in `detached` mode on a clone whose remote gained a commit on `refs/pull/<n>/head` → returns that commit and moves HEAD; with a dirty file → `movedHead: false`, HEAD unchanged. - Manual end-to-end on a connected test repo: push a commit to a PR that has a reviewer session → session gets the push turn; merge it → session shows archived in the deck and `<dataDir>/checkouts/<owner>/<repo>/pr-<n>` is gone; reopen a closed PR → session unarchived. ### Assumptions & contingencies - Merging the agent's own PR archives its implementer session immediately; plan step 20 (post-merge CI) will change this. - Issue reopened is handled like PR reopened, although the brief only named PR reopen. - If Forgejo's push payload Event-Type differs from `pull_request_sync`, substitute the real literal (step 3); if `pull_request.merged` is absent on closed payloads, use `payload.pull_request.merged_at != null` instead (`unverified — confirm first`).