fix(server): do not report a merged implementation as abandoned #251

Merged

lucas merged 1 commit from agent/issue-250-followup-close-not-abandoned into main

2026-11-11 09:38:57 +01:00

omp-deck commented

2026-11-11 09:21:52 +01:00

Collaborator

Closes: #250

What changed

hasMergedPull(repo, sessionId, exceptNumber) in db/forgejo-post-merge.ts: true when the session owns a forgejo_post_merge record for another pull request, i.e. it already merged one. No schema change — a record in either state proves the merge.
A closed follow-up archives silently. In handleLifecycle, an unmerged close of an agent PR whose session already merged a pull request archives the session and finishes the row as archived: follow-up closed without merge. Nothing is posted, no scrap row is queued, so queueScrap / buildScrappedTurn / handleAbandon stay exactly as they are for the genuine case.
forgejo_abandon_implementation is denied once the session has a merged pull request; the denial and the implementer system prompt both point a stuck agent at forgejo_create_issue.
finishMergedImplementation is now the single ending for an implementation that landed: it removes the repository's implementation.progress_label from the issue, then archives. Wired into the green post-merge resolve, the sweep's green/no-CI and pending-past-timeout verdicts, the merged-but-unmonitored lifecycle fall-through, and the new follow-up-close branch. Label problems are logged only — the implementation is merged, so no human decision waits on the label. Previously the abandon reset was the only code that ever removed that label, so every implementation that merged kept it forever.

Verification

bun test apps/server/src — 1284 pass, 0 fail (includes 4 new tests: the hasMergedPull cases, the silent follow-up archive, the progress-label removal on a merged implementation, and the merged-abandon denial plus its prompt bullet).
bun run typecheck — clean.
The existing "a human closing the agent's PR runs a final turn, then resets with a fixed notice" test is the regression guard that the genuine scrap path is untouched.

Note

Step 6 of the plan assumes a resumed implementer session rebuilds its system prompt, so the changed bullet reaches the post-merge turn. That assumption is not verified here; the behaviour is correct either way, because the tool denial carries the same instruction.

Closes: #250 ## What changed - **`hasMergedPull(repo, sessionId, exceptNumber)`** in `db/forgejo-post-merge.ts`: true when the session owns a `forgejo_post_merge` record for another pull request, i.e. it already merged one. No schema change — a record in either state proves the merge. - **A closed follow-up archives silently.** In `handleLifecycle`, an unmerged close of an agent PR whose session already merged a pull request archives the session and finishes the row as `archived: follow-up closed without merge`. Nothing is posted, no scrap row is queued, so `queueScrap` / `buildScrappedTurn` / `handleAbandon` stay exactly as they are for the genuine case. - **`forgejo_abandon_implementation` is denied** once the session has a merged pull request; the denial and the implementer system prompt both point a stuck agent at `forgejo_create_issue`. - **`finishMergedImplementation`** is now the single ending for an implementation that landed: it removes the repository's `implementation.progress_label` from the issue, then archives. Wired into the green post-merge resolve, the sweep's green/no-CI and pending-past-timeout verdicts, the merged-but-unmonitored lifecycle fall-through, and the new follow-up-close branch. Label problems are logged only — the implementation is merged, so no human decision waits on the label. Previously the abandon reset was the only code that ever removed that label, so every implementation that merged kept it forever. ## Verification - `bun test apps/server/src` — 1284 pass, 0 fail (includes 4 new tests: the `hasMergedPull` cases, the silent follow-up archive, the progress-label removal on a merged implementation, and the merged-abandon denial plus its prompt bullet). - `bun run typecheck` — clean. - The existing "a human closing the agent's PR runs a final turn, then resets with a fixed notice" test is the regression guard that the genuine scrap path is untouched. ## Note Step 6 of the plan assumes a resumed implementer session rebuilds its system prompt, so the changed bullet reaches the post-merge turn. That assumption is not verified here; the behaviour is correct either way, because the tool denial carries the same instruction.

omp-deck added 1 commit

2026-11-11 09:21:52 +01:00

fix(server): do not report a merged implementation as abandoned a0f3b07c22

A human closing an agent's post-merge follow-up pull request ran the
scrapped-implementation path on the original issue, which reported the
merged implementation as abandoned and swapped its progress label back
to the planning label. The merged work stands; only the CI fix was
dismissed.

Such a close now archives the implementer session silently, recognised
from the session owning a post-merge record for another pull request.
The same predicate denies `forgejo_abandon_implementation` once the
session has merged a pull request, and the implementer prompt points a
stuck agent at `forgejo_create_issue` instead.

Ending a merged implementation also removes the repository's
`implementation.progress_label` from its issue on every path, which so
far only the abandon reset ever did, leaving the label on every
implementation that landed.

Co-Authored-By: Claude Opus 5 via omp-deck