theam / theam/facility

Turns interrupted by a dead worker are never counted against the project budget

Open
#399 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
71
Forks
64
Avg merge
15h 38m
Merged PRs (30d)
66

Description

In short: if the worker process dies while an engine is running, the recovery job marks the turn failed but never records its usage. The provider already charged for those tokens. The month total stays too low, so the budget check admits turns it should block. The Insights page, README and FAQ all say the opposite.

What happened

I installed Facility locally (main at d50f3c4) and read how budgets are enforced.

The Insights budget panel says (apps/web/app/(app)/projects/[projectId]/insights/page.tsx:117-119): "a provider call already in progress is allowed to finish and is accounted afterwards." README.md:92-93 and apps/docs/docs/faq.md:55-56 say the same.

That holds when the engine fails inside a live worker. It does not hold when the worker itself dies. Then the turn gets no turn_usage row, "spent this month" is lower than the real bill, and the worker_interrupted attention item asks the user to retry, which charges the provider a second time. The retry's spend is booked. The first turn's never is.

How to reproduce

I added one test to services/api/test/turn-dispatcher.integration.test.ts (diff below). No credentials, no network. It uses the file's FakeCodexEngine and the repo's FakeWorkspaceRuntime.

  • Turn A: the engine throws AgentEngineError with usage {100 in, 20 out} inside a live dispatcher. Result: failed, a turn_usage row exists, spentCents goes up.
  • Turn B: I set a turn to running with updatedAt five minutes old (what a dead worker leaves behind) and call recoverInterruptedTurns(). Result: failed, a worker_interrupted attention item is open, no turn_usage row, spentCents unchanged.

As committed, the last two assertions state today's behaviour and all 17 tests in the file pass. Flip them to .toHaveLength(1) and .toBeGreaterThan(spentAfterAccounted) and the test fails with: AssertionError: expected [] to have a length of 1 but got +0.

docker compose -f docker-compose.dev.yml exec -T postgres dropdb -U facility --if-exists facility_test
docker compose -f docker-compose.dev.yml exec -T postgres createdb -U facility facility_test
DATABASE_URL="postgres://facility:facility@127.0.0.1:5461/facility_test" pnpm --filter @facility/api exec vitest run -t "books provider spend" test/turn-dispatcher.integration.test.ts

Turn B runs no engine, so the test shows the missing call, not lost data from a real session.

Test diff (against d50f3c4; passes as written)
diff --git a/services/api/test/turn-dispatcher.integration.test.ts b/services/api/test/turn-dispatcher.integration.test.ts
index 1369ff9..77032c3 100644
--- a/services/api/test/turn-dispatcher.integration.test.ts
+++ b/services/api/test/turn-dispatcher.integration.test.ts
@@ -26,6 +26,7 @@ import postgres from "postgres";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 import { AgentCatalogService, type AgentCatalogSource } from "../src/agents/catalog.js";
 import { GithubWorkspaceCredentialBroker } from "../src/github/workspace-credentials.js";
+import { CostBudgetService } from "../src/insights/costs.js";
 import { StoryWorkspaceService } from "../src/stories/service.js";
 import { TurnDispatcher } from "../src/turns/dispatcher.js";
 import {
@@ -117,6 +118,7 @@ environment:
     renameNextBranch?: string;
     corruptResumeOnce = false;
     failNextRun = false;
+    failNextRunAfterSpending = false;
     liveFailureGate?: Promise<void>;
     throttleDuringRun = false;
     replacementPending = false;
@@ -175,6 +177,21 @@ environment:
           ],
         });
       }
+      if (this.failNextRunAfterSpending) {
+        this.failNextRunAfterSpending = false;
+        throw new AgentEngineError("agent_engine_failed", "codex exited with status 1", {
+          engine: "codex",
+          exitCode: 1,
+          events: [],
+          durationMs: 1_000,
+          usage: {
+            inputTokens: 100,
+            outputTokens: 20,
+            cacheReadTokens: 0,
+            cacheWriteTokens: 0,
+          },
+        });
+      }
       if (this.corruptResumeOnce && request.nativeSessionId) {
         this.corruptResumeOnce = false;
         this.replacementPending = true;
@@ -1029,6 +1046,99 @@ environment:
     ).toEqual(expect.arrayContaining(["Persist this message before the engine starts"]));
   });
 
+  it("books provider spend for an in-process engine failure but not for a worker-interrupted turn", async () => {
+    const costs = new CostBudgetService(db);
+    await db
+      .insert(projectBudgets)
+      .values({
+        id: newId("bud"),
+        orgId,
+        projectId,
+        monthlyLimitCents: 100_000,
+        warningPercent: 80,
+        enabled: true,
+      })
+      .onConflictDoUpdate({
+        target: projectBudgets.projectId,
+        set: { monthlyLimitCents: 100_000, enabled: true },
+      });
+    const spentBefore = (await costs.budgetState(orgId, projectId)).spentCents;
+
+    engine.failNextRunAfterSpending = true;
+    const accounted = await storiesService.start({
+      orgId,
+      projectId,
+      provider: "manual",
+      externalId: `spend-accounted-${suffix}`,
+      title: "Fail in process after spending",
+      agent: builder,
+      message: "Burn tokens, then fail where the dispatcher can see it",
+      messageDedupeKey: `spend-accounted-start-${suffix}`,
+      actor: { type: "user", id: "user_test" },
+      workspace: { image: "facility-runner:test", ports: [] },
+    });
+    const accountedTurn = accounted.queued.turn;
+    if (!accountedTurn) throw new Error("expected an accounted turn");
+    await expect(
+      dispatcher.dispatch({ orgId, projectId, turnId: accountedTurn.id }),
+    ).resolves.toMatchObject({ state: "failed" });
+    expect(await db.select().from(turnUsage).where(eq(turnUsage.turnId, accountedTurn.id))).toEqual(
+      [
+        expect.objectContaining({
+          inputTokens: 100,
+          outputTokens: 20,
+          priced: true,
+          status: "failed",
+        }),
+      ],
+    );
+    const spentAfterAccounted = (await costs.budgetState(orgId, projectId)).spentCents;
+    expect(spentAfterAccounted).toBeGreaterThan(spentBefore);
+
+    const interrupted = await storiesService.start({
+      orgId,
+      projectId,
+      provider: "manual",
+      externalId: `spend-interrupted-${suffix}`,
+      title: "Lose the worker after spending",
+      agent: builder,
+      message: "Burn the same tokens, then lose the worker",
+      messageDedupeKey: `spend-interrupted-start-${suffix}`,
+      actor: { type: "user", id: "user_test" },
+      workspace: { image: "facility-runner:test", ports: [] },
+    });
+    const interruptedTurn = interrupted.queued.turn;
+    if (!interruptedTurn) throw new Error("expected an interrupted turn");
+    const now = new Date();
+    const stale = new Date(now.getTime() - 5 * 60_000);
+    await db
+      .update(turns)
+      .set({ state: "running", startedAt: stale, updatedAt: stale })
+      .where(eq(turns.id, interruptedTurn.id));
+    await expect(recoverInterruptedTurns(db, storiesService, now, 60_000)).resolves.toBe(1);
+
+    await expect(storiesService.get(orgId, projectId, interrupted.story.id)).resolves.toMatchObject(
+      {
+        turns: expect.arrayContaining([
+          expect.objectContaining({ id: interruptedTurn.id, state: "failed" }),
+        ]),
+        attention: [expect.objectContaining({ kind: "worker_interrupted", status: "open" })],
+      },
+    );
+
+    // Recovery reclaims the row, stops the orphan and raises attention, but books no usage:
+    // whatever the provider charged before the worker died never reaches the budget.
+    expect(
+      await db.select().from(turnUsage).where(eq(turnUsage.turnId, interruptedTurn.id)),
+    ).toEqual([]);
+    expect((await costs.budgetState(orgId, projectId)).spentCents).toBe(spentAfterAccounted);
+
+    await db
+      .update(projectBudgets)
+      .set({ enabled: false })
+      .where(eq(projectBudgets.projectId, projectId));
+  });
+
   it("keeps the agent's renamed branch and native workspace on the next turn", async () => {
     const branch = `chore/retained-${randomUUID()}`;
     const started = await storiesService.start({
How I run Facility

Local development (pnpm dev). Node 24.21.0, pnpm 11.20.0, Docker Desktop 4.85, macOS.

Version

main at d50f3c4 (2026-09-16). Also on b7a9212.

Evidence

Rows left in facility_test after the test (join of stories, turns and turn_usage for the two test stories):

             title              | turn_state | in_tok | out_tok | cost_cents |  accounting  |                           error
--------------------------------+------------+--------+---------+------------+--------------+-----------------------------------------------------------
 Fail in process after spending | failed     |    100 |      20 |       0.16 | booked       | codex exited with status 1
 Lose the worker after spending | failed     |      0 |       0 |          0 | NO USAGE ROW | Worker heartbeat expired before the agent turn completed.
Why it happens
  • The only insert(turnUsage) is in CostBudgetService.record() (services/api/src/insights/costs.ts:71). It is called from two places, both inside TurnDispatcher.dispatch(): services/api/src/turns/dispatcher.ts:337 (success) and :457 (engine error). Both need the dispatcher to be alive.
  • A dead worker's turn is handled instead by recoverInterruptedTurns() (services/api/src/worker.ts:193, run every minute via :119 and :142), which calls StoryWorkspaceService.recoverInterruptedTurn() (services/api/src/stories/service.ts:609). That marks the turn failed (:626), opens the attention item (:659) and kills the orphaned engine process (:682). It never touches CostBudgetService.
  • Engines only report usage on a completion event (engines.ts:338, :379). A killed process never sends one.
  • The budget gate assertTurnAllowed() (costs.ts:29, called at dispatcher.ts:145) sums turn_usage.cost_cents. A missing row means a lower sum.

A worker that is still alive but loses its lease cancels itself through dispatcher.ts:455-457 and can still book usage. This issue is only about a worker that is gone.

Possible fix
  1. Reconcile in recovery. recoverInterruptedTurn() already reaches the workspace (service.ts:682). Read usage from the retained native session files and book it through record() with a source such as "recovered". That needs a migration: turn_usage_source_check (packages/db/src/schema.ts:884-886) allows only provider, price_book, unpriced. Claude Code session files do carry a per-message usage block (72 of 74 transcripts on my machine). I have not checked Codex.
  2. Or say so. Change the attention text at service.ts:659 to state that the interrupted turn's spend is not booked, and change the panel, README and FAQ to say interrupted turns are excluded.

Which contract do you want: every provider charge counts (1), or only turns the dispatcher finishes (2)? I can send the PR with tests once you pick. AGENTS.md:5 asks for unit and integration tests on budget changes; the test above already covers the integration side.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with services/api/test/turn-dispatcher.integration.test.ts and run the named Vitest command to reproduce the missing turn_usage row. Read CostBudgetService.record/assertTurnAllowed, TurnDispatcher.dispatch, and recoverInterruptedTurns/StoryWorkspaceService.recoverInterruptedTurn to trace both paths. Done means the chosen accounting contract is explicit and the test, schema, and referenced documentation agree with it.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend, databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.