lucas/omp-deck
lucas/omp-deck
Watch
1
Fork
You've already forked omp-deck
 0
Code Issues 9 Pull requests Projects Releases Packages Wiki Activity

Forgejo: CI monitoring on the agent's own pull requests (plan step 20) #109

New issue
Closed
opened 2026-10-27 16:55:09 +01:00 by omp-deck · 0 comments
omp-deck commented 2026-10-27 16:55:09 +01:00
Collaborator

Problem

The agent should watch CI on the pull requests it opened itself.

Today:

  • The implementer waits for CI only inside its own turn (CI_BULLET in apps/server/src/bridge/session-policy.ts tells it to call forgejo_wait_commit_status).
  • handleCommitStatus (apps/server/src/forgejo/dispatcher.ts) skips pull requests authored by the agent.
  • handleLifecycle archives the agent's PR session as soon as the PR is merged (comment "Until post-merge CI exists…").
  • parseCiWatchdog (apps/server/src/forgejo/repo-config.ts) ignores ci_watchdog.own_prs.

Before merge: some CI failures happen outside the in-turn wait: the wait timed out, the turn ended before CI finished, a check was re-run and failed, or someone else pushed to the branch. Each of these resumes the PR's mapped implementer session with a CI-failure seed. Each CI outcome is handled once, using the same dedupe as the others' PR watchdog. A trigger whose head has moved since CI finished is dropped, so a failure the running turn has already fixed and pushed is not fixed a second time.

After merge: the agent's own PR session is not archived at merge. Instead a durable record "waiting for post-merge CI on the merge commit" is stored, and it survives restarts. Status events for that SHA are matched against the record.

  • All checks finished and green: archive the session.
  • Any check failed: resume the session to fix the failure through a follow-up PR. The merged PR can't take more pushes, so the session policy allows exactly one new PR and branch in this state.
  • No checks 60 s after the merge: the repo has no CI, so archive.
  • Still pending after 24 h: archive, and post a deck inbox ticket.
  • Linked issue: the deck never touches it. Forgejo closes it at merge.
  • Reviewer sessions: still archived at merge.

Config: ci_watchdog.own_prs.enabled (boolean, default true). When it is false, own-PR CI is not monitored and merged own PRs are archived at merge, as they are today.

Plan

Context

Implement own-PR CI monitoring in the Forgejo dispatcher (apps/server):

  • a pre-merge CI-failure resume,
  • a durable post-merge CI wait with a green/red/no-CI/timeout outcome,
  • a policy exception that lets the implementer open a follow-up PR after a red post-merge CI,
  • the ci_watchdog.own_prs.enabled config key.

Approach

  1. Config: ci_watchdog.own_prs.enabled. Edit apps/server/src/forgejo/repo-config.ts.

    • Add the field ciWatchdogOwnPrs: boolean to RepoConfig, with /** Monitoring of CI on PRs authored by the agent; default true. */.
    • DEFAULT_REPO_CONFIG gets ciWatchdogOwnPrs: true.
    • Change parseCiWatchdog to return { others: PullFilters | null; own: boolean }. Missing or null section gives { others: null, own: true }.
    • For section.own_prs:
      • undefined/null gives own = true.
      • A non-mapping value is an error: `ci_watchdog.own_prs` must be a mapping.
      • enabled must be undefined or a boolean, otherwise the error is `ci_watchdog.own_prs.enabled` must be a boolean. own = enabled ?? true.
      • Other keys in own_prs are ignored.
    • Update the doc comment ("Only ci_watchdog.others_prs is read…") to say that own_prs.enabled is read too.
    • Wire the result into parseRepoConfig.
    • Update repo-config.test.ts:
      • Line 53 uses own_prs: {x: 1}. That stays valid and yields ciWatchdogOwnPrs: true.
      • Add own_prs: {enabled: false} gives false.
      • Add that own_prs: 1 and own_prs: {enabled: 1} give the errors above.
      • Add ciWatchdogOwnPrs: true to every expected config literal in that file (lines ~20, 32, 40, 93).

  2. Persistence: post-merge records.

    • Add the migration apps/server/src/db/migrations/019-forgejo-post-merge.sql: 
      -- An agent-authored PR that was merged and whose merge commit's CI the
-- dispatcher is waiting on. `failed` rows stay: they let the implementer
-- session open one follow-up PR (see session-policy.ts).
CREATE TABLE forgejo_post_merge (
  repo_key   TEXT NOT NULL,          -- lowercased owner/repo
  number     INTEGER NOT NULL,       -- the merged PR
  session_id TEXT NOT NULL,
  merge_sha  TEXT NOT NULL,          -- lowercased 40-hex
  base_ref   TEXT NOT NULL,
  state      TEXT NOT NULL CHECK (state IN ('waiting','failed')),
  merged_at  TEXT NOT NULL,          -- ISO, set when the record is stored
  updated_at TEXT NOT NULL,
  PRIMARY KEY (repo_key, number)
);
CREATE INDEX idx_forgejo_post_merge_sha ON forgejo_post_merge (merge_sha);
    • Add the new module apps/server/src/db/forgejo-post-merge.ts. Follow the style of db/forgejo-sessions.ts: getDb(), prepared statements, nowIso(), repo lowercased. Exports:
      • interface PostMergeRecord { repo: string; number: number; sessionId: string; mergeSha: string; baseRef: string; state: "waiting" | "failed"; mergedAt: string }. repo is the stored repo_key.
      • insertPostMerge(r: { repo: string; number: number; sessionId: string; mergeSha: string; baseRef: string }): void: INSERT OR REPLACE with state waiting.
      • listWaitingPostMergeBySha(repo: string, sha: string): PostMergeRecord[]
      • listWaitingPostMerge(): PostMergeRecord[]
      • settlePostMerge(repo: string, number: number, to: "done" | "failed"): boolean. This is an atomic claim, and only the caller that gets true acts.
        • done runs DELETE … WHERE repo_key=? AND number=? AND state='waiting'.
        • failed runs UPDATE … SET state='failed', updated_at=? WHERE … AND state='waiting'.
        • Both return changes === 1.
      • getPostMerge(repo: string, number: number): PostMergeRecord | null
    • Add apps/server/src/db/forgejo-post-merge.test.ts. Model it on forgejo-sessions.test.ts, with bootDb(). Cover these cases:
      • A second settlePostMerge returns false.
      • A failed row is not returned by either listWaiting* function.
      • getPostMerge still returns a failed row.

  3. API: the merge commit SHA. In apps/server/src/forgejo/dispatch-api.ts:

    • Add merge_commit_sha?: string | null to RawPull.
    • Add /** Merge commit; "" when not merged or omitted. */ mergeCommitSha: string to PullSummary.
    • Map it in readPull.
    • Update the fake pull fixtures in dispatcher.test.ts that build PullSummary literals, adding mergeCommitSha: "".
    • In conversation-detect.ts, extend the pull_request type of PullSyncPayloadLike with merge_commit_sha?: string | null.

  4. Before merge: fan-out to own PRs. Restructure handleCommitStatus in dispatcher.ts. Steps 4 and 5 share this method.

    • Keep: the API/agent checks, the workspace match, the config read/parse, and the SHA extraction.
    • Remove the early ciWatchdogOthersPrs === null → dropped return. Instead set:
      • others = parsed.config.ciWatchdogOthersPrs !== null
      • own = parsed.config.ciWatchdogOwnPrs
    • Read the commit status and handle the no status checks / pending / all checks passed drops exactly as today, after the post-merge branch from step 5.
    • If the run failed:
      • Compute outcome as today.
      • listOpenPulls.
      • The others' targets are filtered as today, but only when others is true.
      • The own targets (only when own is true) are pulls where headSha === sha, author is the agent, and getForgejoSession(row.repo, p.number) !== null.
      • For each own target, insert a row with the same payload shape as ci_failed:
        • action: "own_ci_failed", event: "status"
        • dedupeKey sha256(JSON.stringify(["own_ci_failed", repoLower, p.number, sha, outcome]))
    • Finish reasons:
      • queued ${n} CI analysis(es) and ${m} own CI fix(es) for ${sha7}
      • no open pull request has head ${sha7} when n + m === 0
      • CI watchdog not configured as dropped when !others && !own
    • Update the class doc comment (dispatcher.ts ~line 33) and the method doc so they describe both fan-outs and the post-merge lookup.

  5. After merge: matching a status event to a record. In handleCommitStatus, right after the checks are read:

    • Compute records = listWaitingPostMergeBySha(row.repo, sha).
    • Process records whether or not own is enabled now: a record exists only because own monitoring was enabled at merge.
    • For each record, call the new private resolvePostMerge(record, checks, row), which returns a finish-reason fragment:
      • Pending checks: post-merge #N: ${k} check(s) pending, no state change.
      • Empty checks: treat as pending, because the sweep decides "no CI".
      • All terminal, none failed:
        • If settlePostMerge(…, "done"), call archiveAfterIdle(record.sessionId). This is a new private helper extracted from the two lines at the end of handleLifecycle: waitUntilIdle with the implementer turn timeout, then archiveSession. handleLifecycle is changed to call it too.
        • Fragment: post-merge #N green: archived.
      • Any failed:
        • If settlePostMerge(…, "failed"), call insertForgejoTrigger with:
          • action: "post_merge_ci_failed", event: "status", number: record.number, isPull: true, commentAuthor: row.commentAuthor
          • dedupeKey sha256(JSON.stringify(["post_merge_ci_failed", repoLower, record.number, sha]))
        • payload: { sender: { login: <commentAuthor> }, pull_request: { number, title, user: { login: agent }, head: { ref: "", sha }, base: { ref: record.baseRef } }, ci: { sha, checks } }. Take title from api.readIssue(owner, repo, number), or "?" if that fails. Leave out body and html_url: the seed tells the agent to read the PR with forgejo_read_pr to get the Closes: line. Put this payload construction plus the insert in a private helper queuePostMergeFix(record, sha, checks, commentAuthor), which the sweep in step 7 reuses.
        • Fragment: post-merge #N red: queued fix.
    • The finish reason joins the fragments with ; , then continues with the pre-merge outcome.
    • Call this.pump() if anything was queued.

  6. After merge: storing the record instead of archiving. In handleLifecycle, inside the row.isPull branch, when payload.pull_request?.merged === true:

    • Store a record and finish the row without archiving when all of these hold:
      • trigger.kind === "forgejo_implementer"
      • payload.pull_request.user.login equals this.deps.agentUsername() (case-insensitive)
      • this.deps.api() is non-null
    • First read the repo config with api.readRepoFile + parseRepoConfig. On a read or parse failure, log a warning and treat ciWatchdogOwnPrs as true, its default. If it is false, archive as today.
    • Merge SHA:
      • Use payload.pull_request.merge_commit_sha.
      • If it is missing or not 40-hex, use api.readPull(...).data.mergeCommitSha.
      • If it is still empty, archive as today with the reason archived: merged (no merge commit sha).
    • Then:
      • insertPostMerge({ repo: row.repo, number: row.number, sessionId: mapped, mergeSha, baseRef: payload.pull_request.base?.ref ?? "" })
      • finishForgejoTrigger(row.id, "done", \waiting for post-merge CI on ${sha7}`)`
      • this.wake(). Not needed for correctness; the sweep handles it.
    • Replace the "Until post-merge CI exists…" comment with one saying that the agent's own merged PRs wait for post-merge CI, and that reviewer and conversation sessions archive here.
    • Closed-without-merge and the issue-close paths are unchanged.

  7. Sweep: no CI, and timeout. In ForgejoDispatcher:

    • Add private sweepTimer: ReturnType<typeof setInterval> | null.
    • In start(), after pump(), run this.track(this.sweepPostMerge()) and start a 60 000 ms setInterval that does the same.
    • dispose() clears the timer.
    • Expose the constants POST_MERGE_NO_CI_GRACE_MS = 60_000 and POST_MERGE_TIMEOUT_MS = 24 * 60 * 60_000. Make them overridable through the new optional deps postMergeGraceMs? and postMergeTimeoutMs? on ForgejoDispatcherDeps, the same way turnTimeoutMs is (tests need this).
    • sweepPostMerge(): for each listWaitingPostMerge() record with age = now - merged_at:
      • If age < grace, skip.
      • Otherwise read the commit status of mergeSha. On an API error, log a warning and skip.
      • Checks present, all terminal, none failed: green handling as in step 5.
      • Any failed: red handling as in step 5. Insert the trigger directly, with commentAuthor set to the agent username, and pump().
      • Checks empty: settlePostMerge(…, "done"), then archive.
      • Pending and age >= timeout: settlePostMerge(…, "done"), archive, and call captureInbox (the same API fallback uses; reuse its call shape) with:
        • kind: "ticket", source: INBOX_SOURCE
        • title: \Post-merge CI on ${repo}#${n} never finished``
        • body: the merge SHA, the pending contexts, and the session id.
    • To share the evaluation with step 5, factor it into classifyChecks(checks): "pending" | "green" | "red" | "none", using TERMINAL_CHECK_STATES / FAILED_CHECK_STATES. Step 5 maps none to pending; the sweep maps it to archive.

  8. Gate and seeds for the two new actions. In dispatcher.ts:

    • Add "own-ci-failed" | "post-merge-ci-failed" to ReadyReason and update its doc comment.
    • In gate(), add ownCi = row.action === "own_ci_failed" and postMerge = row.action === "post_merge_ci_failed". Before the reviewSubmitted branch:
      • ownCi: call the new private resolveOwnCi(job, payload): Promise<boolean>, modelled on resolveCiWatchdog:
        • Re-read the config. ciWatchdogOwnPrs === false → dropped "CI monitoring of own pull requests disabled".
        • readPull, with failure handled as in resolveCiWatchdog.
        • headSha !== payload.ci.sha → dropped "head moved since CI finished". This is the guard against fixing twice.
        • Author not the agent → dropped "pull request not authored by the agent".
        • getForgejoSession === null → dropped "no session is mapped to this pull request".
        • Sets reason = "own-ci-failed".
      • postMerge: if getPostMerge(row.repo, row.number)?.state !== "failed" → dropped "no failed post-merge record". Otherwise reason = "post-merge-ci-failed".
    • Add ownCi || postMerge to the author source list (payload.pull_request?.user?.login) and to the permission-check exclusion (!synchronized && !basePushed && !autoReview && !ciFailed && !ownCi && !postMerge).
    • In the agentPr block, skip readPull for postMerge: the PR is merged and its head branch may be gone. Set prHead = "merged" for postMerge so reuseMapped still requires an implementer session.
    • In resolveSession, extend the "only continues" rule from pull-synchronized to own-ci-failed and post-merge-ci-failed. Both throw the mapped session is unusable: … instead of creating a session.
    • In runReady, add two seed branches calling new builders in conversation-detect.ts:
      • buildOwnCiFailureSeed({ repo, number, payload: CiFailurePayloadLike }): string
        • Headline: CI failed on your pull request ${repo} #${number}, then the branch and SHA line, then the check list. Share the check-line formatting with buildCiFailureSeed by extracting formatCheckLines(payload).
        • Task: "Read the failing checks' logs (forgejo_read_commit_status, forgejo_list_run_jobs, forgejo_read_job_log). If the pull request's head has moved past this commit, or you already fixed this failure, end without changes. Otherwise fix the cause on the pull request's branch, run the relevant checks, commit and push, wait for CI with forgejo_wait_commit_status, and reply on #N with what you changed. If the failure is not caused by the change (flaky or infrastructure), say so in a comment on #N instead."
      • buildPostMergeCiFailureSeed({ repo, number, payload: CiFailurePayloadLike }): string
        • Headline: CI failed on the merge commit ${sha} of your merged pull request ${repo} #${number} (base ${base}), then the check list.
        • Task: "The pull request is merged, so its branch can't take fixes. Read the failing logs. Fetch origin, create a new branch from the merge commit ${sha} with git_branch (e.g. agent/fix-ci-${number}), fix the cause, commit, push, and open one follow-up pull request with forgejo_create_pr whose base is ${base}, and whose body says Follow-up to #${number} plus the original Closes: #N line if the merged pull request had one. Then comment on #${number} linking the follow-up pull request."
    • In runReady, add both reasons to finishForgejoTrigger(row.id, "done", reason) (already generic).

  9. Policy: allowing the follow-up PR. In apps/server/src/bridge/session-policy.ts:

    • Add function followUpAllowed(scope: ForgejoImplementerScope, sessionId: string): boolean. It returns true when the session's newest PR, Math.max(...implementerPrNumbers(scope, sessionId)), has getPostMerge(repo, pr)?.state === "failed".
    • In decide:
      • forgejo_create_pr is allowed when followUpAllowed. Otherwise the existing deny stays.
      • The scope.headBranch lock on git_branch is bypassed when followUpAllowed.
      • The git_push remote check stays.
    • Once the follow-up PR exists, the bridge maps it (in-process.ts:799). It becomes the newest PR with no record, so the one-PR rule applies again.
    • In systemContext, when followUpAllowed, replace the "Do not open another pull request" lines with: - Pull request #${pr} was merged but CI failed on its merge commit. Open exactly one follow-up pull request from a new branch to fix it.

Critical files & anchors

  • apps/server/src/forgejo/dispatcher.ts:
    • handleCommitStatus (~877): fan-out plus post-merge lookup.
    • handleLifecycle (~1218, the comment at 1259): stores the record.
    • gate (~352): new actions.
    • resolveCiWatchdog (~648): the pattern for resolveOwnCi.
    • resolveSession (~1002): the continue-only rule.
    • start/dispose (~266): the sweep.
  • apps/server/src/bridge/session-policy.ts: forgejoImplementerPolicy.decide (~294–314), implementerPrNumbers (~273).
  • apps/server/src/forgejo/repo-config.ts: parseCiWatchdog (~97).
  • apps/server/src/forgejo/conversation-detect.ts: buildCiFailureSeed (~273), PullSyncPayloadLike (~345).
  • apps/server/src/db/forgejo-sessions.ts: style template for the new db module.

Verification

Run from apps/server: bun test and bun run typecheck. Add dispatcher tests in src/forgejo/dispatcher.test.ts, reusing the existing harness (setup(), the fake api, enqueueEvent, implementerSession, ciRows-style queries). A new test file is not needed.

  • Pre-merge: readCommitStatus returns [{context:"ci",state:"failure",…}], and listOpenPulls returns an agent PR #7 with head sha, mapped to impl.
    • Expected: one own_ci_failed row is done with reason own-ci-failed, and impl received a prompt starting CI failed on your pull request.
    • A second identical status event inserts no new row (dedupe).
    • With readPull.headSha changed, the row is dropped "head moved since CI finished".
    • With own_prs: {enabled: false}, no own rows.
  • Merge: a closed event on agent PR #7 with merged: true and merge_commit_sha: "a"*40.
    • Expected: the session is not archived, getPostMerge is waiting, and the row reason is waiting for post-merge CI on aaaaaaa.
    • A reviewer session (forgejo_pull_request) on a merged PR is still archived.
  • Green: a status event for "a"*40 with all success archives impl and deletes the record.
  • Red: a status event with one failure leaves the record failed, and a post_merge_ci_failed row runs on impl with a seed containing follow-up pull request.
  • Sweep: with postMergeGraceMs: 0, postMergeTimeoutMs: 0, and the dispatcher started:
    • no checks → archived;
    • pending → archived plus an inbox ticket titled Post-merge CI on lucas/omp-deck#7 never finished.
  • Policy (session-policy.test.ts): with setForgejoSession("o/r", 12, "s1") and a failed post-merge record for #12, forgejo_create_pr is "allow". After setForgejoSession("o/r", 13, "s1"), it is denied again.

Assumptions & contingencies

  • merge_commit_sha is assumed to be present in Forgejo's pull_request closed payload (unverified; confirm first). If it is absent on the instance, the readPull fallback from step 6 covers it. If both are empty, the PR is archived as before.
  • Forgejo is assumed to close the linked issue at merge; the deck does not act on issues. Whether Forgejo's keyword parser accepts Closes: #N (with the colon) is unverified. If it does not, change the implementer prompt (session-policy.ts:369, plus the test at session-policy.test.ts:171) to Closes #${number}.
## Problem The agent should watch CI on the pull requests it opened itself. Today: - The implementer waits for CI only inside its own turn (`CI_BULLET` in `apps/server/src/bridge/session-policy.ts` tells it to call `forgejo_wait_commit_status`). - `handleCommitStatus` (`apps/server/src/forgejo/dispatcher.ts`) skips pull requests authored by the agent. - `handleLifecycle` archives the agent's PR session as soon as the PR is merged (comment "Until post-merge CI exists…"). - `parseCiWatchdog` (`apps/server/src/forgejo/repo-config.ts`) ignores `ci_watchdog.own_prs`. **Before merge:** some CI failures happen outside the in-turn wait: the wait timed out, the turn ended before CI finished, a check was re-run and failed, or someone else pushed to the branch. Each of these resumes the PR's mapped implementer session with a CI-failure seed. Each CI outcome is handled once, using the same dedupe as the others' PR watchdog. A trigger whose head has moved since CI finished is dropped, so a failure the running turn has already fixed and pushed is not fixed a second time. **After merge:** the agent's own PR session is not archived at merge. Instead a durable record "waiting for post-merge CI on the merge commit" is stored, and it survives restarts. Status events for that SHA are matched against the record. - **All checks finished and green:** archive the session. - **Any check failed:** resume the session to fix the failure through a follow-up PR. The merged PR can't take more pushes, so the session policy allows exactly one new PR and branch in this state. - **No checks 60 s after the merge:** the repo has no CI, so archive. - **Still pending after 24 h:** archive, and post a deck inbox ticket. - **Linked issue:** the deck never touches it. Forgejo closes it at merge. - **Reviewer sessions:** still archived at merge. **Config:** `ci_watchdog.own_prs.enabled` (boolean, default `true`). When it is `false`, own-PR CI is not monitored and merged own PRs are archived at merge, as they are today. ## Plan ### Context Implement own-PR CI monitoring in the Forgejo dispatcher (`apps/server`): - a pre-merge CI-failure resume, - a durable post-merge CI wait with a green/red/no-CI/timeout outcome, - a policy exception that lets the implementer open a follow-up PR after a red post-merge CI, - the `ci_watchdog.own_prs.enabled` config key. ### Approach 1. **Config: `ci_watchdog.own_prs.enabled`.** Edit `apps/server/src/forgejo/repo-config.ts`. - Add the field `ciWatchdogOwnPrs: boolean` to `RepoConfig`, with `/** Monitoring of CI on PRs authored by the agent; default true. */`. - `DEFAULT_REPO_CONFIG` gets `ciWatchdogOwnPrs: true`. - Change `parseCiWatchdog` to return `{ others: PullFilters | null; own: boolean }`. Missing or null `section` gives `{ others: null, own: true }`. - For `section.own_prs`: - `undefined`/`null` gives `own = true`. - A non-mapping value is an error: `` `ci_watchdog.own_prs` must be a mapping ``. - `enabled` must be `undefined` or a boolean, otherwise the error is `` `ci_watchdog.own_prs.enabled` must be a boolean ``. `own = enabled ?? true`. - Other keys in `own_prs` are ignored. - Update the doc comment ("Only `ci_watchdog.others_prs` is read…") to say that `own_prs.enabled` is read too. - Wire the result into `parseRepoConfig`. - Update `repo-config.test.ts`: - Line 53 uses `own_prs: {x: 1}`. That stays valid and yields `ciWatchdogOwnPrs: true`. - Add `own_prs: {enabled: false}` gives `false`. - Add that `own_prs: 1` and `own_prs: {enabled: 1}` give the errors above. - Add `ciWatchdogOwnPrs: true` to every expected config literal in that file (lines ~20, 32, 40, 93). 2. **Persistence: post-merge records.** - Add the migration `apps/server/src/db/migrations/019-forgejo-post-merge.sql`: ```sql -- An agent-authored PR that was merged and whose merge commit's CI the -- dispatcher is waiting on. `failed` rows stay: they let the implementer -- session open one follow-up PR (see session-policy.ts). CREATE TABLE forgejo_post_merge ( repo_key TEXT NOT NULL, -- lowercased owner/repo number INTEGER NOT NULL, -- the merged PR session_id TEXT NOT NULL, merge_sha TEXT NOT NULL, -- lowercased 40-hex base_ref TEXT NOT NULL, state TEXT NOT NULL CHECK (state IN ('waiting','failed')), merged_at TEXT NOT NULL, -- ISO, set when the record is stored updated_at TEXT NOT NULL, PRIMARY KEY (repo_key, number) ); CREATE INDEX idx_forgejo_post_merge_sha ON forgejo_post_merge (merge_sha); ``` - Add the new module `apps/server/src/db/forgejo-post-merge.ts`. Follow the style of `db/forgejo-sessions.ts`: `getDb()`, prepared statements, `nowIso()`, repo lowercased. Exports: - `interface PostMergeRecord { repo: string; number: number; sessionId: string; mergeSha: string; baseRef: string; state: "waiting" | "failed"; mergedAt: string }`. `repo` is the stored `repo_key`. - `insertPostMerge(r: { repo: string; number: number; sessionId: string; mergeSha: string; baseRef: string }): void`: `INSERT OR REPLACE` with state `waiting`. - `listWaitingPostMergeBySha(repo: string, sha: string): PostMergeRecord[]` - `listWaitingPostMerge(): PostMergeRecord[]` - `settlePostMerge(repo: string, number: number, to: "done" | "failed"): boolean`. This is an atomic claim, and only the caller that gets `true` acts. - `done` runs `DELETE … WHERE repo_key=? AND number=? AND state='waiting'`. - `failed` runs `UPDATE … SET state='failed', updated_at=? WHERE … AND state='waiting'`. - Both return `changes === 1`. - `getPostMerge(repo: string, number: number): PostMergeRecord | null` - Add `apps/server/src/db/forgejo-post-merge.test.ts`. Model it on `forgejo-sessions.test.ts`, with `bootDb()`. Cover these cases: - A second `settlePostMerge` returns `false`. - A `failed` row is not returned by either `listWaiting*` function. - `getPostMerge` still returns a `failed` row. 3. **API: the merge commit SHA.** In `apps/server/src/forgejo/dispatch-api.ts`: - Add `merge_commit_sha?: string | null` to `RawPull`. - Add `/** Merge commit; "" when not merged or omitted. */ mergeCommitSha: string` to `PullSummary`. - Map it in `readPull`. - Update the fake `pull` fixtures in `dispatcher.test.ts` that build `PullSummary` literals, adding `mergeCommitSha: ""`. - In `conversation-detect.ts`, extend the `pull_request` type of `PullSyncPayloadLike` with `merge_commit_sha?: string | null`. 4. **Before merge: fan-out to own PRs.** Restructure `handleCommitStatus` in `dispatcher.ts`. Steps 4 and 5 share this method. - Keep: the API/agent checks, the workspace match, the config read/parse, and the SHA extraction. - Remove the early `ciWatchdogOthersPrs === null → dropped` return. Instead set: - `others = parsed.config.ciWatchdogOthersPrs !== null` - `own = parsed.config.ciWatchdogOwnPrs` - Read the commit status and handle the `no status checks` / `pending` / `all checks passed` drops exactly as today, *after* the post-merge branch from step 5. - If the run failed: - Compute `outcome` as today. - `listOpenPulls`. - The others' targets are filtered as today, but only when `others` is true. - The own targets (only when `own` is true) are pulls where `headSha === sha`, `author` is the agent, and `getForgejoSession(row.repo, p.number) !== null`. - For each own target, insert a row with the same payload shape as `ci_failed`: - `action: "own_ci_failed"`, `event: "status"` - dedupeKey `sha256(JSON.stringify(["own_ci_failed", repoLower, p.number, sha, outcome]))` - Finish reasons: - `queued ${n} CI analysis(es) and ${m} own CI fix(es) for ${sha7}` - `no open pull request has head ${sha7}` when `n + m === 0` - `CI watchdog not configured` as `dropped` when `!others && !own` - Update the class doc comment (dispatcher.ts ~line 33) and the method doc so they describe both fan-outs and the post-merge lookup. 5. **After merge: matching a status event to a record.** In `handleCommitStatus`, right after the checks are read: - Compute `records = listWaitingPostMergeBySha(row.repo, sha)`. - Process records whether or not `own` is enabled now: a record exists only because own monitoring was enabled at merge. - For each record, call the new private `resolvePostMerge(record, checks, row)`, which returns a finish-reason fragment: - **Pending checks:** `post-merge #N: ${k} check(s) pending`, no state change. - **Empty checks:** treat as pending, because the sweep decides "no CI". - **All terminal, none failed:** - If `settlePostMerge(…, "done")`, call `archiveAfterIdle(record.sessionId)`. This is a new private helper extracted from the two lines at the end of `handleLifecycle`: `waitUntilIdle` with the implementer turn timeout, then `archiveSession`. `handleLifecycle` is changed to call it too. - Fragment: `post-merge #N green: archived`. - **Any failed:** - If `settlePostMerge(…, "failed")`, call `insertForgejoTrigger` with: - `action: "post_merge_ci_failed"`, `event: "status"`, `number: record.number`, `isPull: true`, `commentAuthor: row.commentAuthor` - dedupeKey `sha256(JSON.stringify(["post_merge_ci_failed", repoLower, record.number, sha]))` - payload: `{ sender: { login: <commentAuthor> }, pull_request: { number, title, user: { login: agent }, head: { ref: "", sha }, base: { ref: record.baseRef } }, ci: { sha, checks } }`. Take `title` from `api.readIssue(owner, repo, number)`, or `"?"` if that fails. Leave out `body` and `html_url`: the seed tells the agent to read the PR with `forgejo_read_pr` to get the `Closes:` line. Put this payload construction plus the insert in a private helper `queuePostMergeFix(record, sha, checks, commentAuthor)`, which the sweep in step 7 reuses. - Fragment: `post-merge #N red: queued fix`. - The finish reason joins the fragments with `; `, then continues with the pre-merge outcome. - Call `this.pump()` if anything was queued. 6. **After merge: storing the record instead of archiving.** In `handleLifecycle`, inside the `row.isPull` branch, when `payload.pull_request?.merged === true`: - Store a record and finish the row without archiving when all of these hold: - `trigger.kind === "forgejo_implementer"` - `payload.pull_request.user.login` equals `this.deps.agentUsername()` (case-insensitive) - `this.deps.api()` is non-null - First read the repo config with `api.readRepoFile` + `parseRepoConfig`. On a read or parse failure, log a warning and treat `ciWatchdogOwnPrs` as `true`, its default. If it is `false`, archive as today. - Merge SHA: - Use `payload.pull_request.merge_commit_sha`. - If it is missing or not 40-hex, use `api.readPull(...).data.mergeCommitSha`. - If it is still empty, archive as today with the reason `archived: merged (no merge commit sha)`. - Then: - `insertPostMerge({ repo: row.repo, number: row.number, sessionId: mapped, mergeSha, baseRef: payload.pull_request.base?.ref ?? "" })` - `finishForgejoTrigger(row.id, "done", \`waiting for post-merge CI on ${sha7}\`)` - `this.wake()`. Not needed for correctness; the sweep handles it. - Replace the "Until post-merge CI exists…" comment with one saying that the agent's own merged PRs wait for post-merge CI, and that reviewer and conversation sessions archive here. - Closed-without-merge and the issue-close paths are unchanged. 7. **Sweep: no CI, and timeout.** In `ForgejoDispatcher`: - Add `private sweepTimer: ReturnType<typeof setInterval> | null`. - In `start()`, after `pump()`, run `this.track(this.sweepPostMerge())` and start a 60 000 ms `setInterval` that does the same. - `dispose()` clears the timer. - Expose the constants `POST_MERGE_NO_CI_GRACE_MS = 60_000` and `POST_MERGE_TIMEOUT_MS = 24 * 60 * 60_000`. Make them overridable through the new optional deps `postMergeGraceMs?` and `postMergeTimeoutMs?` on `ForgejoDispatcherDeps`, the same way `turnTimeoutMs` is (tests need this). - `sweepPostMerge()`: for each `listWaitingPostMerge()` record with `age = now - merged_at`: - If `age < grace`, skip. - Otherwise read the commit status of `mergeSha`. On an API error, log a warning and skip. - Checks present, all terminal, none failed: green handling as in step 5. - Any failed: red handling as in step 5. Insert the trigger directly, with `commentAuthor` set to the agent username, and `pump()`. - Checks empty: `settlePostMerge(…, "done")`, then archive. - Pending and `age >= timeout`: `settlePostMerge(…, "done")`, archive, and call `captureInbox` (the same API `fallback` uses; reuse its call shape) with: - `kind: "ticket"`, `source: INBOX_SOURCE` - `title: \`Post-merge CI on ${repo}#${n} never finished\`` - `body`: the merge SHA, the pending contexts, and the session id. - To share the evaluation with step 5, factor it into `classifyChecks(checks): "pending" | "green" | "red" | "none"`, using `TERMINAL_CHECK_STATES` / `FAILED_CHECK_STATES`. Step 5 maps `none` to pending; the sweep maps it to archive. 8. **Gate and seeds for the two new actions.** In `dispatcher.ts`: - Add `"own-ci-failed" | "post-merge-ci-failed"` to `ReadyReason` and update its doc comment. - In `gate()`, add `ownCi = row.action === "own_ci_failed"` and `postMerge = row.action === "post_merge_ci_failed"`. Before the `reviewSubmitted` branch: - **`ownCi`:** call the new private `resolveOwnCi(job, payload): Promise<boolean>`, modelled on `resolveCiWatchdog`: - Re-read the config. `ciWatchdogOwnPrs === false` → `dropped "CI monitoring of own pull requests disabled"`. - `readPull`, with failure handled as in `resolveCiWatchdog`. - `headSha !== payload.ci.sha` → `dropped "head moved since CI finished"`. This is the guard against fixing twice. - Author not the agent → `dropped "pull request not authored by the agent"`. - `getForgejoSession === null` → `dropped "no session is mapped to this pull request"`. - Sets `reason = "own-ci-failed"`. - **`postMerge`:** if `getPostMerge(row.repo, row.number)?.state !== "failed"` → `dropped "no failed post-merge record"`. Otherwise `reason = "post-merge-ci-failed"`. - Add `ownCi || postMerge` to the `author` source list (`payload.pull_request?.user?.login`) and to the permission-check exclusion (`!synchronized && !basePushed && !autoReview && !ciFailed && !ownCi && !postMerge`). - In the `agentPr` block, skip `readPull` for `postMerge`: the PR is merged and its head branch may be gone. Set `prHead = "merged"` for `postMerge` so `reuseMapped` still requires an implementer session. - In `resolveSession`, extend the "only continues" rule from `pull-synchronized` to `own-ci-failed` and `post-merge-ci-failed`. Both throw `the mapped session is unusable: …` instead of creating a session. - In `runReady`, add two seed branches calling new builders in `conversation-detect.ts`: - **`buildOwnCiFailureSeed({ repo, number, payload: CiFailurePayloadLike }): string`** - Headline: `CI failed on your pull request ${repo} #${number}`, then the branch and SHA line, then the check list. Share the check-line formatting with `buildCiFailureSeed` by extracting `formatCheckLines(payload)`. - Task: "Read the failing checks' logs (forgejo_read_commit_status, forgejo_list_run_jobs, forgejo_read_job_log). If the pull request's head has moved past this commit, or you already fixed this failure, end without changes. Otherwise fix the cause on the pull request's branch, run the relevant checks, commit and push, wait for CI with forgejo_wait_commit_status, and reply on #N with what you changed. If the failure is not caused by the change (flaky or infrastructure), say so in a comment on #N instead." - **`buildPostMergeCiFailureSeed({ repo, number, payload: CiFailurePayloadLike }): string`** - Headline: `CI failed on the merge commit ${sha} of your merged pull request ${repo} #${number} (base ${base})`, then the check list. - Task: "The pull request is merged, so its branch can't take fixes. Read the failing logs. Fetch origin, create a new branch from the merge commit ${sha} with git_branch (e.g. `agent/fix-ci-${number}`), fix the cause, commit, push, and open one follow-up pull request with forgejo_create_pr whose base is ${base}, and whose body says `Follow-up to #${number}` plus the original `Closes: #N` line if the merged pull request had one. Then comment on #${number} linking the follow-up pull request." - In `runReady`, add both reasons to `finishForgejoTrigger(row.id, "done", reason)` (already generic). 9. **Policy: allowing the follow-up PR.** In `apps/server/src/bridge/session-policy.ts`: - Add `function followUpAllowed(scope: ForgejoImplementerScope, sessionId: string): boolean`. It returns true when the session's newest PR, `Math.max(...implementerPrNumbers(scope, sessionId))`, has `getPostMerge(repo, pr)?.state === "failed"`. - In `decide`: - `forgejo_create_pr` is allowed when `followUpAllowed`. Otherwise the existing deny stays. - The `scope.headBranch` lock on `git_branch` is bypassed when `followUpAllowed`. - The `git_push` remote check stays. - Once the follow-up PR exists, the bridge maps it (`in-process.ts:799`). It becomes the newest PR with no record, so the one-PR rule applies again. - In `systemContext`, when `followUpAllowed`, replace the "Do not open another pull request" lines with: `- Pull request #${pr} was merged but CI failed on its merge commit. Open exactly one follow-up pull request from a new branch to fix it.` ### Critical files & anchors - `apps/server/src/forgejo/dispatcher.ts`: - `handleCommitStatus` (~877): fan-out plus post-merge lookup. - `handleLifecycle` (~1218, the comment at 1259): stores the record. - `gate` (~352): new actions. - `resolveCiWatchdog` (~648): the pattern for `resolveOwnCi`. - `resolveSession` (~1002): the continue-only rule. - `start`/`dispose` (~266): the sweep. - `apps/server/src/bridge/session-policy.ts`: `forgejoImplementerPolicy.decide` (~294–314), `implementerPrNumbers` (~273). - `apps/server/src/forgejo/repo-config.ts`: `parseCiWatchdog` (~97). - `apps/server/src/forgejo/conversation-detect.ts`: `buildCiFailureSeed` (~273), `PullSyncPayloadLike` (~345). - `apps/server/src/db/forgejo-sessions.ts`: style template for the new db module. ### Verification Run from `apps/server`: `bun test` and `bun run typecheck`. Add dispatcher tests in `src/forgejo/dispatcher.test.ts`, reusing the existing harness (`setup()`, the fake `api`, `enqueueEvent`, `implementerSession`, `ciRows`-style queries). A new test file is not needed. - **Pre-merge:** `readCommitStatus` returns `[{context:"ci",state:"failure",…}]`, and `listOpenPulls` returns an agent PR #7 with head `sha`, mapped to `impl`. - Expected: one `own_ci_failed` row is `done` with reason `own-ci-failed`, and `impl` received a prompt starting `CI failed on your pull request`. - A second identical status event inserts no new row (dedupe). - With `readPull.headSha` changed, the row is `dropped "head moved since CI finished"`. - With `own_prs: {enabled: false}`, no own rows. - **Merge:** a `closed` event on agent PR #7 with `merged: true` and `merge_commit_sha: "a"*40`. - Expected: the session is not archived, `getPostMerge` is `waiting`, and the row reason is `waiting for post-merge CI on aaaaaaa`. - A reviewer session (`forgejo_pull_request`) on a merged PR is still archived. - **Green:** a status event for `"a"*40` with all `success` archives `impl` and deletes the record. - **Red:** a status event with one failure leaves the record `failed`, and a `post_merge_ci_failed` row runs on `impl` with a seed containing `follow-up pull request`. - **Sweep:** with `postMergeGraceMs: 0`, `postMergeTimeoutMs: 0`, and the dispatcher started: - no checks → archived; - pending → archived plus an inbox ticket titled `Post-merge CI on lucas/omp-deck#7 never finished`. - **Policy** (`session-policy.test.ts`): with `setForgejoSession("o/r", 12, "s1")` and a `failed` post-merge record for #12, `forgejo_create_pr` is `"allow"`. After `setForgejoSession("o/r", 13, "s1")`, it is denied again. ### Assumptions & contingencies - `merge_commit_sha` is assumed to be present in Forgejo's `pull_request` closed payload (unverified; confirm first). If it is absent on the instance, the `readPull` fallback from step 6 covers it. If both are empty, the PR is archived as before. - Forgejo is assumed to close the linked issue at merge; the deck does not act on issues. Whether Forgejo's keyword parser accepts `Closes: #N` (with the colon) is unverified. If it does not, change the implementer prompt (`session-policy.ts:369`, plus the test at `session-policy.test.ts:171`) to `Closes #${number}`.
lucas closed this issue 2026-10-27 18:51:39 +01:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
feat/update-check
fix/bun-execpath-resolution
feat/npm-publish
fix/dev-script-skips-telegram-bridge
v0.6.1
v0.6.0
v0.5.0
v0.4.0
v0.3.0
v0.2.0
v0.1.0
Labels
Clear labels   
Agent
Ignore
 omp-deck skips this pull request

  
Agent
In Progress
 An omp-deck implementation is running

  
Agent
Planned
 An approved plan has been written to the issue

  
Agent
Planning
 An omp-deck planning session is running

  
Agent
Ready
 Starts an omp-deck implementation of the issue

  
Agent
Triage
 Starts an omp-deck triage session on the issue

  
Compat
Breaking
 Breaking change that won't be backward compatible

  
Kind
Bug
 Something is not working

  
Kind
Documentation
 Documentation changes

  
Kind
Enhancement
 Improve existing functionality

  
Kind
Feature
 New functionality

  
Kind
Security
 This is security issue

  
Kind
Testing
 Issue or pull request related to testing

  
Priority
Critical
 The priority is critical

  
Priority
High
 The priority is high

  
Priority
Low
 The priority is low

  
Priority
Medium
 The priority is medium

  
Reviewed
Confirmed
 Issue has been confirmed

  
Reviewed
Duplicate
 This issue or pull request already exists

  
Reviewed
Invalid
 Invalid issue

  
Reviewed
Won't Fix
 This issue won't be fixed

  
Status
Abandoned
 Somebody has started to work on this but abandoned work

  
Status
Blocked
 Something is blocking this issue or pull request

  
Status
Need More Info
 Feedback is required to reproduce issue or to continue work

No labels
Agent
Ignore
Agent
In Progress
Agent
Planned
Agent
Planning
Agent
Ready
Agent
Triage
Compat
Breaking
Kind
Bug
Kind
Documentation
Kind
Enhancement
Kind
Feature
Kind
Security
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
lucas/omp-deck#109
No description provided.