lucas/omp-deck

Watch

Fork 0

Closing a post-merge follow-up PR must not report the issue as abandoned #250

New issue

Closed

opened 2026-11-11 09:14:23 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-11-11 09:14:23 +01:00

Collaborator

Problem

When an agent-authored pull request is merged and the merge commit's CI goes red, omp-deck keeps the implementer session alive and has it open one follow-up pull request. If a human then closes that follow-up without merging, the deck runs the "scrapped implementation" path on the original issue: a final turn telling the agent to call forgejo_abandon_implementation, then handleAbandon closes remaining PRs, swaps the progress label back to the planning label and posts an "implementation abandoned" notice.

That report is wrong. The planned implementation was merged and stands; only the post-merge CI fix was dismissed.

The same false report is reachable a second way: forgejo_abandon_implementation stays permitted for the whole life of the session, and the implementer system prompt tells the agent to call it when it hits "CI you cannot fix" — exactly the situation of a post-merge follow-up turn.

A third, adjacent defect surfaced while tracing this: the deck adds implementation.progress_label when an implementation starts (dispatcher.ts:2764) and the only code that ever removes it is the abandon reset (dispatcher.ts:2414). An implementation that ends merged keeps the label forever, and after this change the follow-up close stops clearing it too.

Decisions taken for this work:

Closing a follow-up archives the implementer session silently: no final turn, no Forgejo comment, no inbox ticket. The Forgejo history already shows the human closed it.
A follow-up is recognised from the database alone: the session owns a forgejo_post_merge record for another pull request, i.e. it already merged one.
forgejo_abandon_implementation is denied once the session has a merged pull request; the prompt instead tells a stuck agent to file a new issue with forgejo_create_issue.
Every path where a merged implementation's session is archived removes the progress label.

Plan

1. Database predicate: has this session already merged a pull request?

In apps/server/src/db/forgejo-post-merge.ts, add next to getPostMerge:

/**
 * True when `sessionId` owns a `forgejo_post_merge` record in `repo` for a pull request other than
 * `exceptNumber` (null: any). A record in either state proves the session merged that pull request:
 * `waiting` rows are deleted only on resolution, `failed` rows are kept for the follow-up's lifetime.
 */
export function hasMergedPull(repo: string, sessionId: string, exceptNumber: number | null): boolean

Implementation: SELECT 1 FROM forgejo_post_merge WHERE repo_key = ? AND session_id = ? AND (? IS NULL OR number != ?) LIMIT 1, bound with repo.toLowerCase(); return row !== undefined/null in the style the neighbouring functions use (getPostMerge uses .get(...)).

No new table, index or migration: forgejo_post_merge already stores session_id and is keyed (repo_key, number).

This predicate is complete for live sessions, which is the only case that matters: a merged agent PR either produced a forgejo_post_merge record (and then the session stays alive, dispatcher.ts:2457-2486) or the session was archived at merge time — and the scrap path already skips archived sessions (dispatcher.ts:2336).

Add a unit test in apps/server/src/db/forgejo-post-merge.test.ts alongside the existing one: after insertPostMerge({repo:"o/r", number:7, sessionId:"s1", …}), hasMergedPull("O/R","s1",null) is true, hasMergedPull("o/r","s1",7) is false, hasMergedPull("o/r","s1",12) is true, hasMergedPull("o/r","s2",null) is false; still true after settlePostMerge("o/r",7,"failed").

2. Dispatcher: a closed follow-up archives silently instead of scrapping

In apps/server/src/forgejo/dispatcher.ts, handleLifecycle, inside the unmerged-implementer branch (:2336-2349), after the existing "closed by the agent while abandoning" drop and before this.queueScrap(...):

if (hasMergedPull(row.repo, mapped, row.number)) {
    await this.finishMergedImplementation(mapped, row.repo);
    finishForgejoTrigger(row.id, "done", "archived: follow-up closed without merge");
    return;
}

Import hasMergedPull from ../db/forgejo-post-merge.ts (that import block already exists at :80-87).

Nothing is posted and no row is queued, so queueScrap, buildScrappedTurn and handleAbandon are untouched for the genuine case (the session's only PR closed unmerged without any merge behind it).

3. Dispatcher: one ending for merged implementations that clears the progress label

Add a private method near archiveAfterIdle (dispatcher.ts:2489):

/**
 * End the implementer session of an implementation that landed: remove the repository's
 * `implementation.progress_label` from the issue it implements, then archive the session.
 * Label problems are logged and nothing else: the implementation is merged, so no human
 * decision waits on the label.
 */
private async finishMergedImplementation(sessionId: string, repo: string): Promise<void>

Behaviour, in order:

Resolve the issue: getSessionTrigger(sessionId) inside try/catch (same defensive shape as handleLifecycle:2293-2297). Skip the label work — go straight to archiving — when the lookup throws, the trigger is null, trigger.kind !== "forgejo_implementer", or trigger.scope.pull === trigger.scope.number (a PR-started session with no linked issue: its "number" is a pull request and carries no implementation label).
const api = this.deps.api(); — null means Forgejo is not configured; skip to archiving.
Read the config: api.readRepoFile(owner, repo, REPO_CONFIG_PATH) then parseRepoConfig, exactly as handleAbandon:2409-2410 does (do not reuse readImplementationConfig: it throws ImplementerStartError, which is wrong outside the manual-start path). On a failed read or parse, log.warn with the reason and skip to archiving.
When implementation.progressLabel !== null, call the existing this.swapLabels(api, { owner, repo, number }, { add: null, remove: progressLabel }) (:2792; owner/repo from repo.split("/"), number from the trigger scope). A non-empty return or a thrown error becomes a single log.warn; never a comment, never a ticket, never a thrown error.
await this.archiveAfterIdle(sessionId, "forgejo_implementer");

swapLabels already no-ops (no API write) when the label is absent, so repeated or late calls are harmless.

4. Dispatcher: route every merged ending through it

Replace archiveAfterIdle with finishMergedImplementation at the four places where a session ends because its implementation merged. Each takes (record.sessionId | mapped, record.repo | row.repo):

resolvePostMerge, green verdict: :1705.
sweepPostMerge, green/no-CI verdict: :1792.
sweepPostMerge, pending-past-timeout: :1795 (the merge landed; only CI never reported). Keep the existing this.ticket(...) call after it unchanged.
handleLifecycle, merged pull request that is not monitored: the fall-through archiveAfterIdle at :2351 is shared with the issue-closed and non-implementer paths, so gate it. In the isPull branch compute, right after const merged = … (:2327):
```
const agentUser = this.deps.agentUsername();
const mergedImplementation =
    merged && trigger.kind === "forgejo_implementer" && agentUser !== null && isAgent(payload.pull_request?.user?.login, agentUser);
```
declared with the other let/consts of handleLifecycle so it is visible at :2351, where:
```
if (mergedImplementation) await this.finishMergedImplementation(mapped, row.repo);
else await this.archiveAfterIdle(mapped, trigger.kind);
```
(awaitPostMergeCi returning "waiting" already returned earlier, so reaching :2351 with mergedImplementation true means monitoring was off or no merge sha was found.)

Step 2's new branch calls finishMergedImplementation too, so the label is cleared there as well.

5. Policy: refuse to abandon an implementation that already merged

In apps/server/src/bridge/session-policy.ts, forgejoImplementerPolicy.decide, replace the existing check at :545-547 with:

if (toolName === "forgejo_abandon_implementation") {
    if (paramNumber(params) !== scope.number) return { deny: `only #${scope.number} can be abandoned in this session` };
    if (hasMergedPull(`${scope.owner}/${scope.repo}`, sessionId, null)) {
        return {
            deny: `the implementation of #${scope.number} is already merged, so it cannot be abandoned; if the post-merge CI fix cannot be completed, open a new issue with \`forgejo_create_issue\` naming the failing checks, the merge commit and what you tried`,
        };
    }
}

decide's sessionId is a non-null string (SessionPolicy, :119), so no null guard is needed. Import hasMergedPull from ../db/forgejo-post-merge.ts; the file already imports listForgejoSessionNumbers from ../db/forgejo-sessions.ts, so synchronous DB access inside the policy is the established pattern.

6. Policy: tell a stuck agent what to do instead

In the same file's systemContext (:589), the abandon bullet is built unconditionally at :600. Make it state-dependent — compute once, next to the existing const [pr] = … line (:604), which already branches on sessionId === null:

const merged = sessionId !== null && hasMergedPull(`${scope.owner}/${scope.repo}`, sessionId, null);

When merged is true, the bullet in rules becomes:

- The implementation of #<number> is merged and stands; it cannot be abandoned any more. If the post-merge CI fix cannot be completed, open a new issue with `forgejo_create_issue` naming the failing checks, the merge commit, what you tried, and that #<number> is already merged — then end the turn.

Otherwise it stays exactly as today. The denial text of step 5 repeats the instruction, so an agent acting on a stale system prompt is still told where to go.

Critical files & anchors

apps/server/src/forgejo/dispatcher.ts — handleLifecycle (:2286, unmerged-implementer branch :2336, fall-through archive :2351), resolvePostMerge (:1695), sweepPostMerge (:1767), swapLabels (:2792), archiveAfterIdle (:2489), handleAbandon (:2382, the shape to copy for the config read).
apps/server/src/db/forgejo-post-merge.ts — getPostMerge (:97) is the query to copy; the module docstring explains who owns the table.
apps/server/src/bridge/session-policy.ts — forgejoImplementerPolicy.decide (:538) and systemContext (:589).
apps/server/src/forgejo/dispatcher.test.ts — describe("ForgejoDispatcher post-merge CI") setup/enqueueMerged/enqueueStatus (:3234-3300) and describe("ForgejoDispatcher abandoned implementations") setup/enqueueClose (:3497-3539) are the harnesses the new tests extend.
apps/server/src/db/migrations/019-forgejo-post-merge.sql — states why failed rows are kept, the invariant step 1 relies on.

Verification

Run from the repository root.

bun test apps/server/src/db/forgejo-post-merge.test.ts — the hasMergedPull cases from step 1.
bun test apps/server/src/forgejo/dispatcher.test.ts with three new tests:
- A closed follow-up archives without abandoning (in the post-merge describe). Drive the real sequence: await settle(d, enqueueMerged()), then the red-status trigger used by the existing "red post-merge CI resumes the session" test (:3368) so forgejo_post_merge for #7 is failed. Then map a follow-up: setForgejoSession(REPO, 12, "impl") and enqueue a pull_request/closed row for number 12 with pull_request.user.login = AGENT, merged: false, sender.login = "alice". Expect: rows("implementation_scrapped") is [], h.api.posted is [], h.archived contains "impl", and the closing row's reason is archived: follow-up closed without merge.
- The genuine scrap is unaffected: the existing test at :3570 (close of #7 by alice with no merge behind it) must still queue the scrapped turn and produce the abandon notice. No change expected; it is the regression guard for step 2's placement.
- A merged implementation loses the progress label: in the post-merge describe, set h.api.repoFile = ok(CONFIG) (the implementation.progress_label: Agent/In Progress config string from the abandoned-implementations describe, :3494) and seed h.api.issues.set(4, { title: "Do it", state: "closed", isPull: false, author: "alice", labels: [{ id: 2, name: "Agent/In Progress" }] }). Drive merge + green status so the session archives; expect h.api.labelPuts to contain { number: 4, ids: [] } and h.archived to contain "impl". The existing post-merge tests do not set repoFile to an implementation config, so they must keep passing with the label step logging and skipping — check the harness's default repoFile and fix the assertions only if the new config read makes an existing expectation (e.g. labelPuts) non-empty.
bun test apps/server/src/bridge/session-policy.test.ts with a new test next to "only the session's own issue can be abandoned" (:232): with no post-merge record, forgejo_abandon_implementation on #7 is still "allow"; after insertPostMerge({ repo: "o/r", number: 12, sessionId: "s1", mergeSha: <40-hex>, baseRef: "main" }) the same call denies with a message containing already merged and forgejo_create_issue, and policySystemContext(IMPL, "s1") no longer contains forgejo_abandon_implementation but does contain forgejo_create_issue.
bun test apps/server/src and bun run typecheck once at the end.

Assumptions & contingencies

The post-merge describe's harness currently leaves h.api.repoFile at its default. If that default is an error result, the new label step logs a warning and archives — existing assertions stay green; if it is a config that does define a progress label, adjust those tests' expectations to include the resulting labelPuts entry rather than suppressing the label step.
Resuming an implementer session for a post-merge turn is assumed to rebuild the system prompt, so step 6's bullet takes effect for that turn (dispatcher.ts:1930 describes prompt rebuilding on recycle) — unverified for this specific path. If it does not, the behaviour is still correct: the tool denial in step 5 carries the same instruction, and no extra work is warranted.

## Problem When an agent-authored pull request is merged and the merge commit's CI goes red, omp-deck keeps the implementer session alive and has it open one follow-up pull request. If a human then closes that follow-up without merging, the deck runs the "scrapped implementation" path on the **original issue**: a final turn telling the agent to call `forgejo_abandon_implementation`, then `handleAbandon` closes remaining PRs, swaps the progress label back to the planning label and posts an "implementation abandoned" notice. That report is wrong. The planned implementation was merged and stands; only the post-merge CI fix was dismissed. The same false report is reachable a second way: `forgejo_abandon_implementation` stays permitted for the whole life of the session, and the implementer system prompt tells the agent to call it when it hits "CI you cannot fix" — exactly the situation of a post-merge follow-up turn. A third, adjacent defect surfaced while tracing this: the deck adds `implementation.progress_label` when an implementation starts (`dispatcher.ts:2764`) and the *only* code that ever removes it is the abandon reset (`dispatcher.ts:2414`). An implementation that ends merged keeps the label forever, and after this change the follow-up close stops clearing it too. Decisions taken for this work: - Closing a follow-up archives the implementer session **silently**: no final turn, no Forgejo comment, no inbox ticket. The Forgejo history already shows the human closed it. - A follow-up is recognised from the database alone: the session owns a `forgejo_post_merge` record for another pull request, i.e. it already merged one. - `forgejo_abandon_implementation` is **denied** once the session has a merged pull request; the prompt instead tells a stuck agent to file a new issue with `forgejo_create_issue`. - Every path where a merged implementation's session is archived removes the progress label. ## Plan ### 1. Database predicate: has this session already merged a pull request? In `apps/server/src/db/forgejo-post-merge.ts`, add next to `getPostMerge`: ```ts /** * True when `sessionId` owns a `forgejo_post_merge` record in `repo` for a pull request other than * `exceptNumber` (null: any). A record in either state proves the session merged that pull request: * `waiting` rows are deleted only on resolution, `failed` rows are kept for the follow-up's lifetime. */ export function hasMergedPull(repo: string, sessionId: string, exceptNumber: number | null): boolean ``` Implementation: `SELECT 1 FROM forgejo_post_merge WHERE repo_key = ? AND session_id = ? AND (? IS NULL OR number != ?) LIMIT 1`, bound with `repo.toLowerCase()`; return `row !== undefined/null` in the style the neighbouring functions use (`getPostMerge` uses `.get(...)`). No new table, index or migration: `forgejo_post_merge` already stores `session_id` and is keyed `(repo_key, number)`. This predicate is complete for *live* sessions, which is the only case that matters: a merged agent PR either produced a `forgejo_post_merge` record (and then the session stays alive, `dispatcher.ts:2457-2486`) or the session was archived at merge time — and the scrap path already skips archived sessions (`dispatcher.ts:2336`). Add a unit test in `apps/server/src/db/forgejo-post-merge.test.ts` alongside the existing one: after `insertPostMerge({repo:"o/r", number:7, sessionId:"s1", …})`, `hasMergedPull("O/R","s1",null)` is true, `hasMergedPull("o/r","s1",7)` is false, `hasMergedPull("o/r","s1",12)` is true, `hasMergedPull("o/r","s2",null)` is false; still true after `settlePostMerge("o/r",7,"failed")`. ### 2. Dispatcher: a closed follow-up archives silently instead of scrapping In `apps/server/src/forgejo/dispatcher.ts`, `handleLifecycle`, inside the unmerged-implementer branch (`:2336-2349`), after the existing "closed by the agent while abandoning" drop and before `this.queueScrap(...)`: ```ts if (hasMergedPull(row.repo, mapped, row.number)) { await this.finishMergedImplementation(mapped, row.repo); finishForgejoTrigger(row.id, "done", "archived: follow-up closed without merge"); return; } ``` Import `hasMergedPull` from `../db/forgejo-post-merge.ts` (that import block already exists at `:80-87`). Nothing is posted and no row is queued, so `queueScrap`, `buildScrappedTurn` and `handleAbandon` are untouched for the genuine case (the session's only PR closed unmerged without any merge behind it). ### 3. Dispatcher: one ending for merged implementations that clears the progress label Add a private method near `archiveAfterIdle` (`dispatcher.ts:2489`): ```ts /** * End the implementer session of an implementation that landed: remove the repository's * `implementation.progress_label` from the issue it implements, then archive the session. * Label problems are logged and nothing else: the implementation is merged, so no human * decision waits on the label. */ private async finishMergedImplementation(sessionId: string, repo: string): Promise<void> ``` Behaviour, in order: 1. Resolve the issue: `getSessionTrigger(sessionId)` inside `try/catch` (same defensive shape as `handleLifecycle:2293-2297`). Skip the label work — go straight to archiving — when the lookup throws, the trigger is null, `trigger.kind !== "forgejo_implementer"`, or `trigger.scope.pull === trigger.scope.number` (a PR-started session with no linked issue: its "number" is a pull request and carries no implementation label). 2. `const api = this.deps.api();` — null means Forgejo is not configured; skip to archiving. 3. Read the config: `api.readRepoFile(owner, repo, REPO_CONFIG_PATH)` then `parseRepoConfig`, exactly as `handleAbandon:2409-2410` does (do **not** reuse `readImplementationConfig`: it throws `ImplementerStartError`, which is wrong outside the manual-start path). On a failed read or parse, `log.warn` with the reason and skip to archiving. 4. When `implementation.progressLabel !== null`, call the existing `this.swapLabels(api, { owner, repo, number }, { add: null, remove: progressLabel })` (`:2792`; `owner`/`repo` from `repo.split("/")`, `number` from the trigger scope). A non-empty return or a thrown error becomes a single `log.warn`; never a comment, never a ticket, never a thrown error. 5. `await this.archiveAfterIdle(sessionId, "forgejo_implementer");` `swapLabels` already no-ops (no API write) when the label is absent, so repeated or late calls are harmless. ### 4. Dispatcher: route every merged ending through it Replace `archiveAfterIdle` with `finishMergedImplementation` at the four places where a session ends because its implementation merged. Each takes `(record.sessionId | mapped, record.repo | row.repo)`: - `resolvePostMerge`, green verdict: `:1705`. - `sweepPostMerge`, green/no-CI verdict: `:1792`. - `sweepPostMerge`, pending-past-timeout: `:1795` (the merge landed; only CI never reported). Keep the existing `this.ticket(...)` call after it unchanged. - `handleLifecycle`, merged pull request that is not monitored: the fall-through `archiveAfterIdle` at `:2351` is shared with the issue-closed and non-implementer paths, so gate it. In the `isPull` branch compute, right after `const merged = …` (`:2327`): ```ts const agentUser = this.deps.agentUsername(); const mergedImplementation = merged && trigger.kind === "forgejo_implementer" && agentUser !== null && isAgent(payload.pull_request?.user?.login, agentUser); ``` declared with the other `let`/`const`s of `handleLifecycle` so it is visible at `:2351`, where: ```ts if (mergedImplementation) await this.finishMergedImplementation(mapped, row.repo); else await this.archiveAfterIdle(mapped, trigger.kind); ``` (`awaitPostMergeCi` returning `"waiting"` already returned earlier, so reaching `:2351` with `mergedImplementation` true means monitoring was off or no merge sha was found.) Step 2's new branch calls `finishMergedImplementation` too, so the label is cleared there as well. ### 5. Policy: refuse to abandon an implementation that already merged In `apps/server/src/bridge/session-policy.ts`, `forgejoImplementerPolicy.decide`, replace the existing check at `:545-547` with: ```ts if (toolName === "forgejo_abandon_implementation") { if (paramNumber(params) !== scope.number) return { deny: `only #${scope.number} can be abandoned in this session` }; if (hasMergedPull(`${scope.owner}/${scope.repo}`, sessionId, null)) { return { deny: `the implementation of #${scope.number} is already merged, so it cannot be abandoned; if the post-merge CI fix cannot be completed, open a new issue with \`forgejo_create_issue\` naming the failing checks, the merge commit and what you tried`, }; } } ``` `decide`'s `sessionId` is a non-null `string` (`SessionPolicy`, `:119`), so no null guard is needed. Import `hasMergedPull` from `../db/forgejo-post-merge.ts`; the file already imports `listForgejoSessionNumbers` from `../db/forgejo-sessions.ts`, so synchronous DB access inside the policy is the established pattern. ### 6. Policy: tell a stuck agent what to do instead In the same file's `systemContext` (`:589`), the abandon bullet is built unconditionally at `:600`. Make it state-dependent — compute once, next to the existing `const [pr] = …` line (`:604`), which already branches on `sessionId === null`: ```ts const merged = sessionId !== null && hasMergedPull(`${scope.owner}/${scope.repo}`, sessionId, null); ``` When `merged` is true, the bullet in `rules` becomes: ``` - The implementation of #<number> is merged and stands; it cannot be abandoned any more. If the post-merge CI fix cannot be completed, open a new issue with `forgejo_create_issue` naming the failing checks, the merge commit, what you tried, and that #<number> is already merged — then end the turn. ``` Otherwise it stays exactly as today. The denial text of step 5 repeats the instruction, so an agent acting on a stale system prompt is still told where to go. ## Critical files & anchors - `apps/server/src/forgejo/dispatcher.ts` — `handleLifecycle` (`:2286`, unmerged-implementer branch `:2336`, fall-through archive `:2351`), `resolvePostMerge` (`:1695`), `sweepPostMerge` (`:1767`), `swapLabels` (`:2792`), `archiveAfterIdle` (`:2489`), `handleAbandon` (`:2382`, the shape to copy for the config read). - `apps/server/src/db/forgejo-post-merge.ts` — `getPostMerge` (`:97`) is the query to copy; the module docstring explains who owns the table. - `apps/server/src/bridge/session-policy.ts` — `forgejoImplementerPolicy.decide` (`:538`) and `systemContext` (`:589`). - `apps/server/src/forgejo/dispatcher.test.ts` — `describe("ForgejoDispatcher post-merge CI")` `setup`/`enqueueMerged`/`enqueueStatus` (`:3234-3300`) and `describe("ForgejoDispatcher abandoned implementations")` `setup`/`enqueueClose` (`:3497-3539`) are the harnesses the new tests extend. - `apps/server/src/db/migrations/019-forgejo-post-merge.sql` — states why `failed` rows are kept, the invariant step 1 relies on. ## Verification Run from the repository root. 1. `bun test apps/server/src/db/forgejo-post-merge.test.ts` — the `hasMergedPull` cases from step 1. 2. `bun test apps/server/src/forgejo/dispatcher.test.ts` with three new tests: - **A closed follow-up archives without abandoning** (in the post-merge describe). Drive the real sequence: `await settle(d, enqueueMerged())`, then the red-status trigger used by the existing "red post-merge CI resumes the session" test (`:3368`) so `forgejo_post_merge` for #7 is `failed`. Then map a follow-up: `setForgejoSession(REPO, 12, "impl")` and enqueue a `pull_request`/`closed` row for number 12 with `pull_request.user.login = AGENT`, `merged: false`, `sender.login = "alice"`. Expect: `rows("implementation_scrapped")` is `[]`, `h.api.posted` is `[]`, `h.archived` contains `"impl"`, and the closing row's reason is `archived: follow-up closed without merge`. - **The genuine scrap is unaffected**: the existing test at `:3570` (close of #7 by `alice` with no merge behind it) must still queue the scrapped turn and produce the abandon notice. No change expected; it is the regression guard for step 2's placement. - **A merged implementation loses the progress label**: in the post-merge describe, set `h.api.repoFile = ok(CONFIG)` (the `implementation.progress_label: Agent/In Progress` config string from the abandoned-implementations describe, `:3494`) and seed `h.api.issues.set(4, { title: "Do it", state: "closed", isPull: false, author: "alice", labels: [{ id: 2, name: "Agent/In Progress" }] })`. Drive merge + green status so the session archives; expect `h.api.labelPuts` to contain `{ number: 4, ids: [] }` and `h.archived` to contain `"impl"`. The existing post-merge tests do not set `repoFile` to an implementation config, so they must keep passing with the label step logging and skipping — check the harness's default `repoFile` and fix the assertions only if the new config read makes an existing expectation (e.g. `labelPuts`) non-empty. 3. `bun test apps/server/src/bridge/session-policy.test.ts` with a new test next to "only the session's own issue can be abandoned" (`:232`): with no post-merge record, `forgejo_abandon_implementation` on `#7` is still `"allow"`; after `insertPostMerge({ repo: "o/r", number: 12, sessionId: "s1", mergeSha: <40-hex>, baseRef: "main" })` the same call denies with a message containing `already merged` and `forgejo_create_issue`, and `policySystemContext(IMPL, "s1")` no longer contains `forgejo_abandon_implementation` but does contain `forgejo_create_issue`. 4. `bun test apps/server/src` and `bun run typecheck` once at the end. ## Assumptions & contingencies - The post-merge describe's harness currently leaves `h.api.repoFile` at its default. If that default is an error result, the new label step logs a warning and archives — existing assertions stay green; if it is a config that *does* define a progress label, adjust those tests' expectations to include the resulting `labelPuts` entry rather than suppressing the label step. - Resuming an implementer session for a post-merge turn is assumed to rebuild the system prompt, so step 6's bullet takes effect for that turn (`dispatcher.ts:1930` describes prompt rebuilding on recycle) — *unverified for this specific path*. If it does not, the behaviour is still correct: the tool denial in step 5 carries the same instruction, and no extra work is warranted.

omp-deck added the

Agent

In Progress

label

2026-11-11 09:14:37 +01:00

omp-deck referenced this issue from a pull request that will close it,

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

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