lucas/omp-deck

Watch

Fork 0

CI wait tool blocks session #264

New issue

Closed

opened 2026-11-12 14:47:11 +01:00 by lucas · 0 comments

lucas commented

2026-11-12 14:47:11 +01:00

Owner

Problem

The "wait for CI" tool completely stalls the session. Long running CI therefore
blocks the session from responding to events like follow up comments or reviews.

The cause, read out of the code:

forgejo_wait_commit_status (apps/server/src/forgejo/tools/ci.ts:318-371)
runs waitForChecks (ci.ts:256-279), a for (;;) loop that exits only when
every check is terminal, the commit still has no checks after
EMPTY_GRACE_MS = 30_000, or timeout_seconds passes (default 600, max 3600).
Between fetches it sleeps in webhookWake (ci.ts:282-302) for up to
FALLBACK_POLL_MS = 60_000, waking early only on a commit-status webhook
for its own sha.
The agent turn stays in flight for the whole tool call. runTurn
(apps/server/src/forgejo/dispatcher.ts:2353-2384) settles only on agent_end.
A follow-up comment or review is not dropped: routes-forgejo-webhook.ts
inserts a pending row into forgejo_triggers and calls dispatcher.wake().
But pump (dispatcher.ts:550-568) skips any artifact key already in
this.running, and that key's drain (dispatcher.ts:571-589) is parked
inside runReady → runTurn. The row therefore waits in FIFO order until the
wait returns.
Nothing delivers the event mid-turn. The deck only ever uses
streamingBehavior: "followUp" (dispatcher.ts:2375), and the SDK reads
steering at tool-call boundaries anyway, so a blocking tool swallows it.
The only other exit is the dispatcher's turn timer, which calls
handle.abort() after 30 min for conversation/PR/watchdog sessions
(bridge/session-policy.ts:245) or 2 h for implementer sessions (:536).

The same stall hits a message queued onto the session from the cockpit: it sits
in the SDK queue until the turn ends.

Plan

End the wait early when the session has input it cannot see. The wait stays
blocking — this is not a park-and-resume redesign — but it gains an interrupt
predicate, so the turn ends, the dispatcher's drain loop advances, and the
pending comment or review is delivered as the next turn.

1. In-process hint bus for queued triggers

New file apps/server/src/forgejo/trigger-events.ts, deliberately modelled on
the existing apps/server/src/forgejo/ci-status-events.ts (read it first; copy
its structure, its doc comment style and its swallow-on-throw discipline). It
carries no payload — it is a hint, and the listener re-reads the database:

type Listener = () => void;

/** Calls every listener; one that throws does not stop the rest. */
export function notifyTriggerQueued(): void;

/** Subscribes `listener`; the returned function unsubscribes it. */
export function onTriggerQueued(listener: Listener): () => void;

Fire it from one place: ForgejoDispatcher.pump() (dispatcher.ts:550), right
after the keys set is built, guarded by if (keys.size > 0) notifyTriggerQueued();.
pump() is the single funnel every insert path reaches — the webhook route via
bridge.onForgejoTrigger → wake() (apps/server/src/index.ts:110, :125),
and the dispatcher's own synthetic rows via the this.pump() calls in the status
fan-out and sweeps (dispatcher.ts:1706, :1718, :1952, queueBranch at
:1721, queuePostMergeFix at :1866). Over-firing is harmless: the bus only
ends a sleep, and the predicate in step 4 is the authority.

Do not add the bus to db/forgejo-triggers.ts: db/ must not import forgejo/.

2. Narrow query for interrupting rows

Add to apps/server/src/db/forgejo-triggers.ts:

/**
 * How many `pending` rows target `repo` on one of `numbers` and were not
 * authored by `exceptAuthor`. Zero when `numbers` is empty. Both `repo` and
 * `exceptAuthor` are compared case-insensitively; the `repo` column stores the
 * payload's casing, unlike the `repo_key` columns elsewhere.
 */
export function countPendingForgejoTriggersFor(
	repo: string,
	numbers: readonly number[],
	exceptAuthor: string,
): number;

SQL: SELECT COUNT(*) AS n FROM forgejo_triggers WHERE status = 'pending' AND lower(repo) = ? AND number IN (<placeholders>) AND lower(comment_author) <> ?,
with one ? per number. Return 0 without touching the database when numbers
is empty.

Use this rather than the existing listPendingForgejoTriggers()
(forgejo-triggers.ts:182): that one loads every pending row's payload_json,
which the predicate never reads.

The author filter is load-bearing. An implementer pushes a commit and then
calls the wait tool; that push has already produced a pushed/synchronized
row for the same artifact with comment_author set to the agent's own login
(packages/protocol/src/index.ts:2231-2237). Without the filter the wait would
return instantly on every call and the fix would become a spin loop.

3. Session identity reaches the tool

The tool must know which artifacts belong to its session. listForgejoSessionNumbers(repo, sessionId)
(apps/server/src/db/forgejo-sessions.ts:31) answers that: the issue plus every
PR the session opened, because the bridge maps a new PR to the session as soon
as forgejo_create_pr succeeds (bridge/in-process.ts:889).

Do not use ctx.sessionManager.getSessionId(). The deck's session id is
session.sessionId, which resolves to #activeProviderSessionId()
(node_modules/@oh-my-pi/pi-coding-agent/src/session/agent-session.ts:6084) and
is not guaranteed to equal the session manager's id. Thread it explicitly,
following the filterRef / uiBridgeRef pattern already in this file:

apps/server/src/forgejo/tools/shared.ts: add an optional field to
ForgejoToolContext (optional so the existing stubs in
bridge/git-tools/repo-info.test.ts:41 and forgejo/tools/*.test.ts keep
compiling):
```
/** Deck session id of the session these tools belong to; null until the session is attached. */
sessionId?: () => string | null;
```
apps/server/src/forgejo/tools/index.ts: add a trailing optional parameter
sessionId?: () => string | null to both buildForgejoTools(cwd, signal?)
(index.ts:67) and buildForgejoToolsForRepo(repo, cwd, signal?) (:91), and
put it on the ForgejoToolContext built at index.ts:102.
apps/server/src/bridge/in-process.ts: declare
const sessionIdRef: { current: string | null } = { current: null }; beside
filterRef in both createSession (:291) and the resume path (:454), pass
() => sessionIdRef.current as the new argument at the two build sites
(:298-300, :454-456), add sessionIdRef as a parameter of attach()
(:929-940), and set sessionIdRef.current = sessionId immediately after
const sessionId = (session as any).sessionId as string; (:942). attach()
is the shared wiring point for create and resume, so one assignment covers both.

4. Interrupt predicate in the wait loop

apps/server/src/forgejo/tools/ci.ts:

Extend WaitOutcome (ci.ts:239) with "interrupted".

Add to WaitOptions (ci.ts:241-249):

/** Reason the wait must end so the session can handle input it cannot see, or null. */
interrupt?: () => string | null;

In waitForChecks (ci.ts:256-279), evaluate it after the done check and
before the no_checks check, so an all-terminal commit still reports "done"
but a comment arriving at second 5 is not held for the 30 s empty grace:
```
if (n > 0 && r.details.allTerminal) return done("done");
const stop = o.interrupt?.() ?? null;
if (stop !== null) return { ...done("interrupted"), reason: stop };
if (n === 0 && waitedMs >= EMPTY_GRACE_MS) return done("no_checks");
```
Carry the reason on the success branch's return type as reason?: string.
webhookWake (ci.ts:282-302) must also resolve on the new bus. Subscribe to
onTriggerQueued alongside onCommitStatus and unsubscribe both in finish.
Worst-case latency without the bus would be 60 s; with it the wait ends within
milliseconds, and the per-iteration predicate still catches anything the bus
missed.

In waitCommitStatus.run (ci.ts:336-370), build the predicate from two sources
and pass it as interrupt:

const sessionId = ctx.sessionId?.() ?? null;
const repoFull = `${ctx.owner}/${ctx.repo}`;
const interrupt = (): string | null => {
	if (call?.hasQueuedMessages?.()) {
		return "ended early: a message is queued for this session";
	}
	if (sessionId === null) return null;
	const numbers = listForgejoSessionNumbers(repoFull, sessionId);
	const n = countPendingForgejoTriggersFor(repoFull, numbers, ctx.agentUsername);
	return n === 0 ? null : `ended early: ${n} unhandled Forgejo event(s) on this session's issue or pull request`;
};

An interactive cockpit session needs no special case: it has no
forgejo_sessions mapping, so numbers is empty and only the queued-message
source is live there — which is the correct behaviour for a human-driven session.

On outcome === "interrupted", return an error result so a re-call is
discouraged, with the check report still attached as details:

const tail = `${r.reason} — end your turn now so omp-deck can deliver it; call forgejo_wait_commit_status again afterwards`;
return errorResult(`${tail}\n${r.text}`, details);

The reason goes first because errorResult prefixes error: to the whole
message. The predicate is checked on entry as well (it runs before the first
wake), so a re-call while the same rows are pending returns the same error
immediately instead of spinning — the rows stay pending until the turn ends,
because drain is parked in runTurn.

Update the tool description (ci.ts:321) to state that the wait also ends as
soon as the session has an unhandled Forgejo event or a queued message.

5. Result plumbing for the error case

apps/server/src/forgejo/tools/shared.ts:139: give errorResult an optional
second parameter, mirroring textResult (:128):

export function errorResult(message: string, details?: unknown): ToolResult {
	const content: ToolResult["content"] = [{ type: "text", text: `error: ${message}` }];
	return details === undefined ? { content, isError: true } : { content, details, isError: true };
}

No existing caller passes a second argument, so this is purely additive.

apps/server/src/forgejo/tools/shared.ts:55-59: add to ForgejoCallContext:
```
/** Whether the session has messages queued behind the running turn. */
hasQueuedMessages?: () => boolean;
```
and forward it in defineForgejoTool's execute (shared.ts:100-110), which
today keeps only localProtocolOptions from sdkCtx:
```
hasQueuedMessages: sdkCtx?.hasQueuedMessages ? () => sdkCtx.hasQueuedMessages() : undefined,
```
CustomToolContext.hasQueuedMessages() is real and wired
(node_modules/@oh-my-pi/pi-coding-agent/src/sdk.ts:1442 →
session/session-tools.ts:448 → queuedMessageCount() > 0).
packages/protocol/src/forgejo-tools.ts:166-170: add to
ForgejoWaitCommitStatusResult:
```
/** The wait ended early because the session has input it cannot see while the turn runs. */
interrupted: boolean;
```
Set it at both construction sites in ci.ts — the onPoll partial (ci.ts:354,
interrupted: false) and the final details (ci.ts:368).
apps/web/src/components/tools/forgejo/CommitStatusView.tsx:20: add a tag
beside the existing timedOut one:
```
{r.interrupted === true && <Tag tone="accent">interrupted</Tag>}
```

6. Stale prompt wording

apps/server/src/bridge/session-policy.ts:156 (CI_BULLET) currently tells every
trigger session that forgejo_wait_commit_status "blocks until a commit's checks
finish". Reword that clause to say the wait ends when the checks finish or as
soon as the session has an unhandled comment, review or queued message, in which
case the turn must end and the wait be re-issued in the next turn. Keep the
rest of the bullet and its tool list unchanged.

Critical files & anchors

apps/server/src/forgejo/tools/ci.ts — waitForChecks (256-279), webhookWake
(282-302), waitCommitStatus (318-371). The whole behavioural change lands here.
apps/server/src/forgejo/ci-status-events.ts — the template the new
trigger-events.ts must follow; read before writing it.
apps/server/src/forgejo/dispatcher.ts — pump (550-568), the one site that
fires the bus; drain (571-589) and runTurn (2353-2384) explain why an
interrupting row stays pending for the whole turn.
apps/server/src/bridge/in-process.ts — createSession (290-352), resume path
(454-456), attach (929-942). The three places the session-id ref is threaded.
apps/server/src/forgejo/tools/shared.ts — ForgejoToolContext (30-41),
ForgejoCallContext (55-59), defineForgejoTool (86-115), errorResult (139).

Verification

Working directory: repository root. Runner: bun test.

Unit — the loop. Extend apps/server/src/forgejo/tools/ci.test.ts, whose
harness (lines 20-37) already injects fetch, wake, now and onPoll
over a fake clock; add interrupt to the overrides. New cases:
- A predicate returning null then "ended early: 1 unhandled Forgejo event(s) …"
  on the second iteration, over perpetually-pending checks, yields
  { ok: true, outcome: "interrupted", waitedMs: 60_000 } with the reason carried.
- A predicate that always returns a reason, over checks that are already all
  terminal, still yields outcome: "done" — done wins over interrupted.
- A predicate that always returns a reason, over a commit with no checks,
  yields outcome: "interrupted" at waitedMs: 0, i.e. it does not wait out
  the 30 s empty grace.
  Run: bun test apps/server/src/forgejo/tools/ci.test.ts
Unit — the query. Add cases to the existing
apps/server/src/db/forgejo-triggers.test.ts: rows in another repo, rows on an
unlisted number, rows authored by the agent (mixed casing on both repo and
comment_author), and rows in a non-pending status are all excluded; an
empty numbers array returns 0 and touches nothing.
Run: bun test apps/server/src/db/forgejo-triggers.test.ts
End-to-end — new behaviour, concrete input to observable output. Throwaway
script (delete afterwards; do not leave a permanent test). Open an isolated
SQLite via the openDb helper the db tests use, then:
- setForgejoSession("acme/widgets", 7, "sess-1");
- insert a pending trigger row with repo: "Acme/Widgets", number: 7,
  action: "created", comment_author: "alice";
- build the tool: take the entry named forgejo_wait_commit_status from the
  exported CI_TOOLS (ci.ts:579) and call .build(ctx) with a stub
  ForgejoToolContext whose client.json always reports one pending check
  (the stubForgejo shape in bridge/git-tools/repo-info.test.ts:39-42),
  agentUsername: "bot", and sessionId: () => "sess-1";
- await tool.execute("id", { ref: "a".repeat(40), timeout_seconds: 600 }, undefined, {} as never).
Expected: the call returns in well under a second (not 600 s) with
isError === true, text starting
error: ended early: 1 unhandled Forgejo event(s) on this session's issue or pull request
followed by the normal check report, and details.interrupted === true.
Then flip comment_author to "bot" and re-run: the call must now block (give
it timeout_seconds: 1 and assert outcome text timed out after 1s), proving
the agent's own events do not self-interrupt.
Whole suite. bun test — dispatcher.test.ts and session-policy.test.ts
exercise the files touched in steps 1, 3 and 6.

Assumptions & contingencies

attach() (in-process.ts:929) is assumed to be the single wiring point for
both session creation and resume. If a third construction path turns out to
bypass it, set sessionIdRef.current at that path too rather than moving the
assignment — the ref must never stay null for a trigger session.
CustomToolContext.hasQueuedMessages() is assumed to count the dispatcher's
followUp seeds as well as cockpit messages. If it turns out to count only one
of them, keep the call as is: the trigger-row predicate is the load-bearing
source for #264, and the queued-message source is an additional safety net.
Turn timeouts in session-policy.ts (30 min / 2 h) are deliberately unchanged.

## Problem The "wait for CI" tool completely stalls the session. Long running CI therefore blocks the session from responding to events like follow up comments or reviews. The cause, read out of the code: 1. `forgejo_wait_commit_status` (`apps/server/src/forgejo/tools/ci.ts:318-371`) runs `waitForChecks` (`ci.ts:256-279`), a `for (;;)` loop that exits only when every check is terminal, the commit still has no checks after `EMPTY_GRACE_MS = 30_000`, or `timeout_seconds` passes (default 600, max 3600). Between fetches it sleeps in `webhookWake` (`ci.ts:282-302`) for up to `FALLBACK_POLL_MS = 60_000`, waking early only on a **commit-status** webhook for its own sha. 2. The agent turn stays in flight for the whole tool call. `runTurn` (`apps/server/src/forgejo/dispatcher.ts:2353-2384`) settles only on `agent_end`. 3. A follow-up comment or review is not dropped: `routes-forgejo-webhook.ts` inserts a `pending` row into `forgejo_triggers` and calls `dispatcher.wake()`. But `pump` (`dispatcher.ts:550-568`) skips any artifact key already in `this.running`, and that key's `drain` (`dispatcher.ts:571-589`) is parked inside `runReady` → `runTurn`. The row therefore waits in FIFO order until the wait returns. 4. Nothing delivers the event mid-turn. The deck only ever uses `streamingBehavior: "followUp"` (`dispatcher.ts:2375`), and the SDK reads steering at tool-call boundaries anyway, so a blocking tool swallows it. 5. The only other exit is the dispatcher's turn timer, which calls `handle.abort()` after 30 min for conversation/PR/watchdog sessions (`bridge/session-policy.ts:245`) or 2 h for implementer sessions (`:536`). The same stall hits a message queued onto the session from the cockpit: it sits in the SDK queue until the turn ends. ## Plan End the wait early when the session has input it cannot see. The wait stays blocking — this is not a park-and-resume redesign — but it gains an interrupt predicate, so the turn ends, the dispatcher's `drain` loop advances, and the pending comment or review is delivered as the next turn. ### 1. In-process hint bus for queued triggers New file `apps/server/src/forgejo/trigger-events.ts`, deliberately modelled on the existing `apps/server/src/forgejo/ci-status-events.ts` (read it first; copy its structure, its doc comment style and its swallow-on-throw discipline). It carries no payload — it is a hint, and the listener re-reads the database: ```ts type Listener = () => void; /** Calls every listener; one that throws does not stop the rest. */ export function notifyTriggerQueued(): void; /** Subscribes `listener`; the returned function unsubscribes it. */ export function onTriggerQueued(listener: Listener): () => void; ``` Fire it from one place: `ForgejoDispatcher.pump()` (`dispatcher.ts:550`), right after the `keys` set is built, guarded by `if (keys.size > 0) notifyTriggerQueued();`. `pump()` is the single funnel every insert path reaches — the webhook route via `bridge.onForgejoTrigger` → `wake()` (`apps/server/src/index.ts:110`, `:125`), and the dispatcher's own synthetic rows via the `this.pump()` calls in the status fan-out and sweeps (`dispatcher.ts:1706`, `:1718`, `:1952`, `queueBranch` at `:1721`, `queuePostMergeFix` at `:1866`). Over-firing is harmless: the bus only ends a sleep, and the predicate in step 4 is the authority. Do not add the bus to `db/forgejo-triggers.ts`: `db/` must not import `forgejo/`. ### 2. Narrow query for interrupting rows Add to `apps/server/src/db/forgejo-triggers.ts`: ```ts /** * How many `pending` rows target `repo` on one of `numbers` and were not * authored by `exceptAuthor`. Zero when `numbers` is empty. Both `repo` and * `exceptAuthor` are compared case-insensitively; the `repo` column stores the * payload's casing, unlike the `repo_key` columns elsewhere. */ export function countPendingForgejoTriggersFor( repo: string, numbers: readonly number[], exceptAuthor: string, ): number; ``` SQL: `SELECT COUNT(*) AS n FROM forgejo_triggers WHERE status = 'pending' AND lower(repo) = ? AND number IN (<placeholders>) AND lower(comment_author) <> ?`, with one `?` per number. Return `0` without touching the database when `numbers` is empty. Use this rather than the existing `listPendingForgejoTriggers()` (`forgejo-triggers.ts:182`): that one loads every pending row's `payload_json`, which the predicate never reads. The author filter is load-bearing. An implementer pushes a commit and *then* calls the wait tool; that push has already produced a `pushed`/`synchronized` row for the same artifact with `comment_author` set to the agent's own login (`packages/protocol/src/index.ts:2231-2237`). Without the filter the wait would return instantly on every call and the fix would become a spin loop. ### 3. Session identity reaches the tool The tool must know which artifacts belong to its session. `listForgejoSessionNumbers(repo, sessionId)` (`apps/server/src/db/forgejo-sessions.ts:31`) answers that: the issue plus every PR the session opened, because the bridge maps a new PR to the session as soon as `forgejo_create_pr` succeeds (`bridge/in-process.ts:889`). Do **not** use `ctx.sessionManager.getSessionId()`. The deck's session id is `session.sessionId`, which resolves to `#activeProviderSessionId()` (`node_modules/@oh-my-pi/pi-coding-agent/src/session/agent-session.ts:6084`) and is not guaranteed to equal the session manager's id. Thread it explicitly, following the `filterRef` / `uiBridgeRef` pattern already in this file: - `apps/server/src/forgejo/tools/shared.ts`: add an **optional** field to `ForgejoToolContext` (optional so the existing stubs in `bridge/git-tools/repo-info.test.ts:41` and `forgejo/tools/*.test.ts` keep compiling): ```ts /** Deck session id of the session these tools belong to; null until the session is attached. */ sessionId?: () => string | null; ``` - `apps/server/src/forgejo/tools/index.ts`: add a trailing optional parameter `sessionId?: () => string | null` to both `buildForgejoTools(cwd, signal?)` (`index.ts:67`) and `buildForgejoToolsForRepo(repo, cwd, signal?)` (`:91`), and put it on the `ForgejoToolContext` built at `index.ts:102`. - `apps/server/src/bridge/in-process.ts`: declare `const sessionIdRef: { current: string | null } = { current: null };` beside `filterRef` in both `createSession` (`:291`) and the resume path (`:454`), pass `() => sessionIdRef.current` as the new argument at the two build sites (`:298-300`, `:454-456`), add `sessionIdRef` as a parameter of `attach()` (`:929-940`), and set `sessionIdRef.current = sessionId` immediately after `const sessionId = (session as any).sessionId as string;` (`:942`). `attach()` is the shared wiring point for create and resume, so one assignment covers both. ### 4. Interrupt predicate in the wait loop `apps/server/src/forgejo/tools/ci.ts`: - Extend `WaitOutcome` (`ci.ts:239`) with `"interrupted"`. - Add to `WaitOptions` (`ci.ts:241-249`): ```ts /** Reason the wait must end so the session can handle input it cannot see, or null. */ interrupt?: () => string | null; ``` - In `waitForChecks` (`ci.ts:256-279`), evaluate it after the `done` check and before the `no_checks` check, so an all-terminal commit still reports `"done"` but a comment arriving at second 5 is not held for the 30 s empty grace: ```ts if (n > 0 && r.details.allTerminal) return done("done"); const stop = o.interrupt?.() ?? null; if (stop !== null) return { ...done("interrupted"), reason: stop }; if (n === 0 && waitedMs >= EMPTY_GRACE_MS) return done("no_checks"); ``` Carry the reason on the success branch's return type as `reason?: string`. - `webhookWake` (`ci.ts:282-302`) must also resolve on the new bus. Subscribe to `onTriggerQueued` alongside `onCommitStatus` and unsubscribe both in `finish`. Worst-case latency without the bus would be 60 s; with it the wait ends within milliseconds, and the per-iteration predicate still catches anything the bus missed. In `waitCommitStatus.run` (`ci.ts:336-370`), build the predicate from two sources and pass it as `interrupt`: ```ts const sessionId = ctx.sessionId?.() ?? null; const repoFull = `${ctx.owner}/${ctx.repo}`; const interrupt = (): string | null => { if (call?.hasQueuedMessages?.()) { return "ended early: a message is queued for this session"; } if (sessionId === null) return null; const numbers = listForgejoSessionNumbers(repoFull, sessionId); const n = countPendingForgejoTriggersFor(repoFull, numbers, ctx.agentUsername); return n === 0 ? null : `ended early: ${n} unhandled Forgejo event(s) on this session's issue or pull request`; }; ``` An interactive cockpit session needs no special case: it has no `forgejo_sessions` mapping, so `numbers` is empty and only the queued-message source is live there — which is the correct behaviour for a human-driven session. On `outcome === "interrupted"`, return an **error** result so a re-call is discouraged, with the check report still attached as `details`: ```ts const tail = `${r.reason} — end your turn now so omp-deck can deliver it; call forgejo_wait_commit_status again afterwards`; return errorResult(`${tail}\n${r.text}`, details); ``` The reason goes first because `errorResult` prefixes `error: ` to the whole message. The predicate is checked on entry as well (it runs before the first `wake`), so a re-call while the same rows are pending returns the same error immediately instead of spinning — the rows stay `pending` until the turn ends, because `drain` is parked in `runTurn`. Update the tool description (`ci.ts:321`) to state that the wait also ends as soon as the session has an unhandled Forgejo event or a queued message. ### 5. Result plumbing for the error case - `apps/server/src/forgejo/tools/shared.ts:139`: give `errorResult` an optional second parameter, mirroring `textResult` (`:128`): ```ts export function errorResult(message: string, details?: unknown): ToolResult { const content: ToolResult["content"] = [{ type: "text", text: `error: ${message}` }]; return details === undefined ? { content, isError: true } : { content, details, isError: true }; } ``` No existing caller passes a second argument, so this is purely additive. - `apps/server/src/forgejo/tools/shared.ts:55-59`: add to `ForgejoCallContext`: ```ts /** Whether the session has messages queued behind the running turn. */ hasQueuedMessages?: () => boolean; ``` and forward it in `defineForgejoTool`'s `execute` (`shared.ts:100-110`), which today keeps only `localProtocolOptions` from `sdkCtx`: ```ts hasQueuedMessages: sdkCtx?.hasQueuedMessages ? () => sdkCtx.hasQueuedMessages() : undefined, ``` `CustomToolContext.hasQueuedMessages()` is real and wired (`node_modules/@oh-my-pi/pi-coding-agent/src/sdk.ts:1442` → `session/session-tools.ts:448` → `queuedMessageCount() > 0`). - `packages/protocol/src/forgejo-tools.ts:166-170`: add to `ForgejoWaitCommitStatusResult`: ```ts /** The wait ended early because the session has input it cannot see while the turn runs. */ interrupted: boolean; ``` Set it at both construction sites in `ci.ts` — the `onPoll` partial (`ci.ts:354`, `interrupted: false`) and the final `details` (`ci.ts:368`). - `apps/web/src/components/tools/forgejo/CommitStatusView.tsx:20`: add a tag beside the existing `timedOut` one: ```tsx {r.interrupted === true && <Tag tone="accent">interrupted</Tag>} ``` ### 6. Stale prompt wording `apps/server/src/bridge/session-policy.ts:156` (`CI_BULLET`) currently tells every trigger session that `forgejo_wait_commit_status` "blocks until a commit's checks finish". Reword that clause to say the wait ends when the checks finish **or as soon as the session has an unhandled comment, review or queued message, in which case the turn must end and the wait be re-issued in the next turn**. Keep the rest of the bullet and its tool list unchanged. ## Critical files & anchors - `apps/server/src/forgejo/tools/ci.ts` — `waitForChecks` (256-279), `webhookWake` (282-302), `waitCommitStatus` (318-371). The whole behavioural change lands here. - `apps/server/src/forgejo/ci-status-events.ts` — the template the new `trigger-events.ts` must follow; read before writing it. - `apps/server/src/forgejo/dispatcher.ts` — `pump` (550-568), the one site that fires the bus; `drain` (571-589) and `runTurn` (2353-2384) explain why an interrupting row stays `pending` for the whole turn. - `apps/server/src/bridge/in-process.ts` — `createSession` (290-352), resume path (454-456), `attach` (929-942). The three places the session-id ref is threaded. - `apps/server/src/forgejo/tools/shared.ts` — `ForgejoToolContext` (30-41), `ForgejoCallContext` (55-59), `defineForgejoTool` (86-115), `errorResult` (139). ## Verification Working directory: repository root. Runner: `bun test`. 1. **Unit — the loop.** Extend `apps/server/src/forgejo/tools/ci.test.ts`, whose `harness` (lines 20-37) already injects `fetch`, `wake`, `now` and `onPoll` over a fake clock; add `interrupt` to the overrides. New cases: - A predicate returning `null` then `"ended early: 1 unhandled Forgejo event(s) …"` on the second iteration, over perpetually-pending checks, yields `{ ok: true, outcome: "interrupted", waitedMs: 60_000 }` with the reason carried. - A predicate that always returns a reason, over checks that are already all terminal, still yields `outcome: "done"` — `done` wins over `interrupted`. - A predicate that always returns a reason, over a commit with **no** checks, yields `outcome: "interrupted"` at `waitedMs: 0`, i.e. it does not wait out the 30 s empty grace. Run: `bun test apps/server/src/forgejo/tools/ci.test.ts` 2. **Unit — the query.** Add cases to the existing `apps/server/src/db/forgejo-triggers.test.ts`: rows in another repo, rows on an unlisted number, rows authored by the agent (mixed casing on both `repo` and `comment_author`), and rows in a non-`pending` status are all excluded; an empty `numbers` array returns `0` and touches nothing. Run: `bun test apps/server/src/db/forgejo-triggers.test.ts` 3. **End-to-end — new behaviour, concrete input to observable output.** Throwaway script (delete afterwards; do not leave a permanent test). Open an isolated SQLite via the `openDb` helper the db tests use, then: - `setForgejoSession("acme/widgets", 7, "sess-1")`; - insert a `pending` trigger row with `repo: "Acme/Widgets"`, `number: 7`, `action: "created"`, `comment_author: "alice"`; - build the tool: take the entry named `forgejo_wait_commit_status` from the exported `CI_TOOLS` (`ci.ts:579`) and call `.build(ctx)` with a stub `ForgejoToolContext` whose `client.json` always reports one `pending` check (the `stubForgejo` shape in `bridge/git-tools/repo-info.test.ts:39-42`), `agentUsername: "bot"`, and `sessionId: () => "sess-1"`; - `await tool.execute("id", { ref: "a".repeat(40), timeout_seconds: 600 }, undefined, {} as never)`. Expected: the call returns in well under a second (not 600 s) with `isError === true`, text starting `error: ended early: 1 unhandled Forgejo event(s) on this session's issue or pull request` followed by the normal check report, and `details.interrupted === true`. Then flip `comment_author` to `"bot"` and re-run: the call must now block (give it `timeout_seconds: 1` and assert `outcome` text `timed out after 1s`), proving the agent's own events do not self-interrupt. 4. **Whole suite.** `bun test` — `dispatcher.test.ts` and `session-policy.test.ts` exercise the files touched in steps 1, 3 and 6. ## Assumptions & contingencies - `attach()` (`in-process.ts:929`) is assumed to be the single wiring point for both session creation and resume. If a third construction path turns out to bypass it, set `sessionIdRef.current` at that path too rather than moving the assignment — the ref must never stay `null` for a trigger session. - `CustomToolContext.hasQueuedMessages()` is assumed to count the dispatcher's `followUp` seeds as well as cockpit messages. If it turns out to count only one of them, keep the call as is: the trigger-row predicate is the load-bearing source for #264, and the queued-message source is an additional safety net. - Turn timeouts in `session-policy.ts` (30 min / 2 h) are deliberately unchanged.

lucas added the

Kind

Bug

label

2026-11-12 14:47:11 +01:00

lucas added the

Agent

Ready

label

2026-11-12 17:46:14 +01:00