From 0c342ad07d4d4e089a435eb6d5213f046af1a974 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Wed, 26 Aug 2026 08:14:10 -0500 Subject: [PATCH] fix(app-hosting): at-most-once tail settle, stop/meter watermark discipline, payer alignment, wake seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four dark-shipped app-hosting billing gaps (APP_HOSTING_ENABLED off, zero production callers of wakePublishedApp today), fixed now before wiring makes them live: 1. HIGH: settleAbandonedTail re-billed the stranded tail on every failed or racing wake. It now claims the tail with a guarded CAS that clears awakeBilledThrough/awakeHoldId BEFORE charging, so a retried wake after start_failed, or two concurrent wakes, settle it at most once. 2. MEDIUM: stopPublishedApp planned its settle from the row read BEFORE the slow Fly stopMachine call, so a meter tick landing during that call got double-billed. It now re-reads the watermark after the Fly call returns and settles only what that fresh watermark still owes. 3. MEDIUM: the router's balance gate read published_apps.ownerId while the meter and wake gate charge drives.ownerId via resolveEnvPayerId. The router now resolves the payer through the IDENTICAL function (defaultAppBillingDeps.resolvePayerId), so the two can never drift, and fails closed on an unresolvable drive. 4. MEDIUM: a replay to a stopped app relied on Fly's autostart, which starts the machine with no status flip, watermark stamp, or hold — invisible to the awake meter (running rows only). SERVABLE_STATUSES no longer treats 'stopped' as replayable; router.ts now routes a stopped app through wakePublishedApp (gate + hold + start + bookkeeping) before deciding, consistent with #2491's "the router never writes" design — the write happens in the real wake seam, not in the pure decision. Also (LOW): trackAIUsage now releases a placed hold in both failure shapes (the inner writeAiUsage/consumeCredits throw, and the outer calculation throw), not just the writeAiUsage-returned-null branch — closing the last case where a stranded hold suppressed the payer's balance for its own TTL. Each fix is covered by a new test that fails when the fix is reverted (verified via mutation), plus the existing suites still pass. No changelog entry — the feature ships dark behind APP_HOSTING_ENABLED. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Eb48eMuZayhd9WNfE2VdFP --- .../__tests__/ai-monitoring.test.ts | 32 +++++ packages/lib/src/monitoring/ai-monitoring.ts | 13 +- .../__tests__/app-lifecycle-metering.test.ts | 92 ++++++++++++- .../app-hosting/__tests__/router-core.test.ts | 12 +- .../app-hosting/__tests__/router.test.ts | 130 +++++++++++++++++- .../app-hosting/app-lifecycle-metering.ts | 57 +++++++- .../src/services/app-hosting/router-core.ts | 18 ++- .../lib/src/services/app-hosting/router.ts | 85 ++++++++++-- 8 files changed, 404 insertions(+), 35 deletions(-) diff --git a/packages/lib/src/monitoring/__tests__/ai-monitoring.test.ts b/packages/lib/src/monitoring/__tests__/ai-monitoring.test.ts index 453152bd60..c0a8fd3a28 100644 --- a/packages/lib/src/monitoring/__tests__/ai-monitoring.test.ts +++ b/packages/lib/src/monitoring/__tests__/ai-monitoring.test.ts @@ -720,6 +720,38 @@ describe('trackAIUsage', () => { ); }); + it('should release the hold when writeAiUsage THROWS, not just when it resolves null', async () => { + // The gate's reservation would otherwise sit against the payer's spendable + // balance for its whole TTL — a throw here never confirmed a charge, so + // there is nothing this release could double-free. + mockWriteAiUsage.mockRejectedValueOnce(new Error('db error')); + const outcome = await trackAIUsage({ + userId: 'user-1', + provider: 'openai', + model: 'gpt-4o', + holdId: 'hold-thrown', + }); + expect(outcome).toEqual({ persisted: false, creditsSettled: false }); + expect(mockReleaseHold).toHaveBeenCalledWith('hold-thrown'); + }); + + it('should release the hold when the usage CALCULATION itself throws, before writeAiUsage is even reached', async () => { + // A getter that throws on `model` forces the OUTER catch — `calculateCost` + // reads it before `writeAiUsage` is ever called, so this exercises the + // catch that never even attempted a write. + const data: Record = { userId: 'user-1', provider: 'openai', holdId: 'hold-calc-thrown' }; + Object.defineProperty(data, 'model', { + get() { + throw new Error('boom'); + }, + }); + + const outcome = await trackAIUsage(data as never); + + expect(outcome).toEqual({ persisted: false, creditsSettled: false }); + expect(mockReleaseHold).toHaveBeenCalledWith('hold-calc-thrown'); + }); + it('given AI request completes, should NOT write prompt or completion content to ai_usage_logs (#957 — GDPR data minimization)', async () => { await trackAIUsage({ userId: 'user-1', diff --git a/packages/lib/src/monitoring/ai-monitoring.ts b/packages/lib/src/monitoring/ai-monitoring.ts index 1012f694e9..379169860f 100644 --- a/packages/lib/src/monitoring/ai-monitoring.ts +++ b/packages/lib/src/monitoring/ai-monitoring.ts @@ -1169,12 +1169,21 @@ export async function trackAIUsage(data: AIUsageData): Promise { const { deps, trackUsage } = makeDeps(); - seed(abandoned(), [[appRow({ status: 'running' })]]); + // Two returning-row entries: the tail's own CAS claim, then the wake's final CAS. + seed(abandoned(), [[{ id: 'app-1' }], [appRow({ status: 'running' })]]); await wakePublishedApp('app-1', deps); @@ -296,7 +297,7 @@ describe('wakePublishedApp — the abandoned tail a failed close left behind', ( lastStopAt: new Date('2026-08-20T10:10:00.000Z'), // zero-length window awakeHoldId: 'hold-stranded', }), - [[appRow({ status: 'running' })]], + [[{ id: 'app-1' }], [appRow({ status: 'running' })]], ); await wakePublishedApp('app-1', deps); @@ -310,7 +311,7 @@ describe('wakePublishedApp — the abandoned tail a failed close left behind', ( // point — two independent failures at two separate moments — and said so. const { deps, trackUsage, startMachine } = makeDeps(); trackUsage.mockRejectedValue(new Error('ledger down')); - seed(abandoned(), [[appRow({ status: 'running' })]]); + seed(abandoned(), [[{ id: 'app-1' }], [appRow({ status: 'running' })]]); const result = await wakePublishedApp('app-1', deps); @@ -327,6 +328,55 @@ describe('wakePublishedApp — the abandoned tail a failed close left behind', ( expect(trackUsage).not.toHaveBeenCalled(); }); + + it('claims the tail EXCLUSIVELY before charging: a wake that starts, fails, and retries does NOT re-bill it', async () => { + // `settleAbandonedTail` runs on EVERY wake attempt against a non-running row — + // including a retry of a wake whose `startMachine` just failed. Without an + // at-most-once claim, each retry would re-settle the same stranded span, + // writing a new `ai_usage_logs` row every time. + const { deps, trackUsage, startMachine } = makeDeps(); + startMachine.mockRejectedValueOnce(new Error('fly outage')); + const row = abandoned(); + seed(row, [[{ id: 'app-1' }]]); // the tail claim succeeds; nothing else needed — start fails first + + const first = await wakePublishedApp('app-1', deps); + expect(first.outcome).toBe('start_failed'); + expect(trackUsage).toHaveBeenCalledTimes(1); + + // Re-seed exactly as the CAS would have left the row: watermark and hold + // cleared by the first attempt's claim, regardless of the start failure. + seed(appRow({ status: 'stopped', driveId: row.driveId, awakeBilledThrough: null, lastStopAt: row.lastStopAt, awakeHoldId: null }), [ + [appRow({ status: 'running' })], + ]); + + const second = await wakePublishedApp('app-1', deps); + + assert({ + given: 'a wake retried after `start_failed` following a settled tail', + should: 'start cleanly and NOT re-bill the already-claimed tail', + actual: { outcome: second.outcome, trackUsageCalls: trackUsage.mock.calls.length }, + expected: { outcome: 'woken', trackUsageCalls: 1 }, + }); + }); + + it('two wakes racing the same abandoned tail bill it EXACTLY ONCE', async () => { + const { deps, trackUsage } = makeDeps(); + const row = abandoned(); + // Wake #1 wins the claim (returns the row) and its own final CAS. Wake #2 — + // reading the identical stale snapshot, as a genuine race would — loses the + // claim (returns no row) and must never reach `trackUsage`. + seed(row, [ + [{ id: 'app-1' }], + [appRow({ status: 'running' })], + [], + [appRow({ status: 'running' })], + ]); + + await wakePublishedApp('app-1', deps); + await wakePublishedApp('app-1', deps); + + expect(trackUsage).toHaveBeenCalledTimes(1); + }); }); describe('stopPublishedApp', () => { @@ -502,6 +552,42 @@ describe('stopPublishedApp', () => { expect(releaseHold).toHaveBeenCalledWith('hold-1'); }); + it('re-reads the watermark AFTER the (slow) Fly stop call, so a meter tick landing mid-call is not re-billed', async () => { + // `stopMachine` can take several seconds. If the awake meter ticks while we + // wait on it — settling a slice of the window and re-holding — settling + // against the SNAPSHOT READ BEFORE THE CALL would re-bill whatever the meter + // already collected. Settling against a fresh read bills only the remainder. + const { deps, trackUsage, stopMachine } = makeDeps(); + const staleRow = running(); // awakeBilledThrough: WOKEN_AT (11:00), hold-1 + const meterAdvancedAt = new Date(WOKEN_AT.getTime() + 30 * 60_000); // 11:30 + const afterMeterTick = appRow({ + status: 'running', + lastWakeAt: WOKEN_AT, + awakeBilledThrough: meterAdvancedAt, + awakeHoldId: 'hold-2', // the meter's re-hold, replacing the wake's own + }); + stopMachine.mockImplementation(async () => { + mockDb.__state.selectRows = [afterMeterTick]; + }); + seed(staleRow); + + const result = await stopPublishedApp('app-1', 'idle', deps); + + assert({ + given: 'a meter settle landing during the Fly stop call', + should: 'bill only the 30 minutes the watermark still owed, not the full hour', + actual: { outcome: result.outcome, billed: result.outcome === 'stopped' ? result.billedSeconds : null }, + expected: { outcome: 'stopped', billed: 1800 }, + }); + expect(trackUsage).toHaveBeenCalledWith({ + payerId: 'payer-1', + holdId: 'hold-2', + activeSeconds: 1800, + driveId: 'drive-1', + publishedAppId: 'app-1', + }); + }); + it('refuses to stop a row that is not running', async () => { const { deps } = makeDeps(); seed(appRow({ status: 'stopped' })); diff --git a/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts b/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts index 93058444b8..5a58215f28 100644 --- a/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts +++ b/packages/lib/src/services/app-hosting/__tests__/router-core.test.ts @@ -48,10 +48,14 @@ describe('decideAppRoute — the balance gate is the wake gate', () => { }); assert({ - given: 'a STOPPED metered app whose payer is out of credits', - should: 'still refuse — a stopped machine is exactly the one a replay would wake', - actual: route(app({ status: 'stopped' }), false), - expected: { kind: 'parked', reason: 'out_of_credits' }, + given: 'a STOPPED app, decided in isolation (no wake seam involved)', + should: + 'answer unavailable, never replay — a stopped row must go through wakePublishedApp first, ' + + 'which is why router.ts intercepts it before this function is ever asked; see the ' + + "SERVABLE_STATUSES comment for why replaying Fly's silent autostart would leave the " + + 'machine unmetered', + actual: route(app({ status: 'stopped' }), true), + expected: { kind: 'unavailable', reason: 'deploying' }, }); assert({ diff --git a/packages/lib/src/services/app-hosting/__tests__/router.test.ts b/packages/lib/src/services/app-hosting/__tests__/router.test.ts index 666679aab2..d2f0c28767 100644 --- a/packages/lib/src/services/app-hosting/__tests__/router.test.ts +++ b/packages/lib/src/services/app-hosting/__tests__/router.test.ts @@ -6,9 +6,11 @@ * could be lost without the pure test noticing: * * • the kill switch short-circuits BEFORE any database read; - * • the balance is asked about the row's OWN payer (`ownerId`), the same - * column the awake-seconds meter charges; + * • the balance is asked about the app's DRIVE-OWNER payer, resolved the SAME + * way the awake-seconds meter resolves it — never `published_apps.ownerId`; * • the ledger is not consulted at all for a dedicated app; + * • a STOPPED app is woken through the real wake seam (gate + hold + start + + * bookkeeping) rather than replayed to and left for Fly's silent autostart; * • a router that cannot derive a replay key refuses rather than emitting a * replay with a blank state; * • a real failure (database down) propagates instead of reading as a miss. @@ -23,7 +25,7 @@ vi.mock('@pagespace/db/schema/published-apps', () => ({ status: 'status', tier: 'tier', machineId: 'machineId', - ownerId: 'ownerId', + driveId: 'driveId', subdomain: 'subdomain', }, })); @@ -34,6 +36,16 @@ vi.mock('@pagespace/db/operators', () => ({ eq: (a: unknown, b: unknown) => ({ e // covered in `billing/__tests__/credit-gate.test.ts`. vi.mock('../../../billing/credit-gate', () => ({ hasSpendableBalance: vi.fn() })); vi.mock('../../../billing/credit-balance', () => ({ resolveTier: vi.fn() })); +// Same reasoning: `app-billing`'s default payer resolver drags in the whole +// credit/monitoring stack to build, and `app-lifecycle-metering`'s wake seam +// drags in Fly clients and the provisioner. Both are asserted BEHAVIOURALLY +// (does the router's default binding delegate to them with the right args), +// which needs a mock, not the real module. +vi.mock('../app-billing', () => ({ defaultAppBillingDeps: { resolvePayerId: vi.fn() } })); +vi.mock('../app-lifecycle-metering', () => ({ + wakePublishedApp: vi.fn(), + defaultAppLifecycleMeteringDeps: { __marker: 'default-lifecycle-deps' }, +})); import { defaultAppRouterDeps, @@ -45,6 +57,8 @@ import { db } from '@pagespace/db/db'; import { publishedApps } from '@pagespace/db/schema/published-apps'; import { hasSpendableBalance } from '../../../billing/credit-gate'; import { resolveTier } from '../../../billing/credit-balance'; +import { defaultAppBillingDeps } from '../app-billing'; +import { wakePublishedApp, defaultAppLifecycleMeteringDeps, type WakePublishedAppResult } from '../app-lifecycle-metering'; import { isAppHostingEnabled } from '../app-hosting-env'; import { resolveAppReplaySecret, resolvePublishedAppsApex } from '../routing-env'; @@ -57,7 +71,7 @@ function row(overrides: Partial = {}): PublishedAppRouteRo status: 'running', tier: 'metered', machineId: 'm-1', - ownerId: 'user_payer', + driveId: 'drive_payer', ...overrides, }; } @@ -68,8 +82,11 @@ function deps(overrides: Partial = {}): AppRouterDeps { apex: () => 'pagespace.app', replaySecret: () => SECRET, findAppBySubdomain: async () => row(), + resolvePayerId: async ({ driveId }) => (driveId === 'drive_payer' ? 'user_payer' : null), resolveTier: async () => 'pro', hasSpendableBalance: async () => true, + wakePublishedApp: async () => + ({ outcome: 'woken', app: {} }) as unknown as WakePublishedAppResult, ...overrides, }; } @@ -116,22 +133,39 @@ describe('resolveAppRoute — hostname resolution', () => { }); }); -describe('resolveAppRoute — the balance is asked about the row own payer', () => { - it("given a metered app, should ask about the row's ownerId, not any other user", async () => { +describe('resolveAppRoute — the balance is asked about the SAME payer the meter charges', () => { + it("given a metered app, should resolve the payer from the row's driveId, not published_apps.ownerId", async () => { + const resolvePayerId = vi.fn(async ({ driveId }: { driveId: string }) => + driveId === 'drive_xyz' ? 'user_drive_owner' : null, + ); const hasSpendableBalance = vi.fn(async () => true); const resolveTier = vi.fn(async () => 'pro'); await resolveAppRoute( 'acme.pagespace.app', deps({ - findAppBySubdomain: async () => row({ ownerId: 'user_drive_owner' }), + findAppBySubdomain: async () => row({ driveId: 'drive_xyz' }), + resolvePayerId, resolveTier, hasSpendableBalance, }), ); + expect(resolvePayerId).toHaveBeenCalledWith({ driveId: 'drive_xyz' }); expect(resolveTier).toHaveBeenCalledWith('user_drive_owner'); expect(hasSpendableBalance).toHaveBeenCalledWith('user_drive_owner', 'pro'); }); + it('given an unresolvable drive, should refuse (fail closed) rather than serve on an unverified balance', async () => { + const resolveTier = vi.fn(async () => 'pro'); + const hasSpendableBalance = vi.fn(async () => true); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ resolvePayerId: async () => null, resolveTier, hasSpendableBalance }), + ); + expect(decision).toEqual({ kind: 'parked', reason: 'out_of_credits' }); + expect(resolveTier).not.toHaveBeenCalled(); + expect(hasSpendableBalance).not.toHaveBeenCalled(); + }); + it('given an insolvent payer, should park rather than replay', async () => { const decision = await resolveAppRoute( 'acme.pagespace.app', @@ -167,6 +201,68 @@ describe('resolveAppRoute — the balance is asked about the row own payer', () }); }); +describe('resolveAppRoute — a STOPPED app is woken through the real seam, never autostarted blind', () => { + it('given a running app, should never call the wake seam at all', async () => { + const wakePublishedApp = vi.fn(); + await resolveAppRoute('acme.pagespace.app', deps({ findAppBySubdomain: async () => row({ status: 'running' }), wakePublishedApp })); + expect(wakePublishedApp).not.toHaveBeenCalled(); + }); + + it('given a stopped app that wakes cleanly, should decide as if it were running — a replay', async () => { + const wakePublishedApp = vi.fn(async () => ({ outcome: 'woken', app: {} }) as unknown as WakePublishedAppResult); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'stopped' }), wakePublishedApp }), + ); + expect(wakePublishedApp).toHaveBeenCalledWith('app_1'); + expect(decision.kind).toBe('replay'); + }); + + it('given the wake gate refuses (insolvent), should park rather than replay', async () => { + const wakePublishedApp = vi.fn( + async () => ({ outcome: 'parked', reason: 'insufficient_credits' }) as unknown as WakePublishedAppResult, + ); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'stopped' }), wakePublishedApp }), + ); + expect(decision).toEqual({ kind: 'parked', reason: 'out_of_credits' }); + }); + + it('given Fly refuses the start, should answer unavailable rather than replay to a machine that never came up', async () => { + const wakePublishedApp = vi.fn( + async () => ({ outcome: 'start_failed', error: 'capacity' }) as unknown as WakePublishedAppResult, + ); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'stopped' }), wakePublishedApp }), + ); + expect(decision).toEqual({ kind: 'unavailable', reason: 'failed' }); + }); + + it('given the wake refuses because hosting flipped off mid-request, should answer hosting_disabled', async () => { + const wakePublishedApp = vi.fn( + async () => ({ outcome: 'refused', reason: 'disabled' }) as unknown as WakePublishedAppResult, + ); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'stopped' }), wakePublishedApp }), + ); + expect(decision).toEqual({ kind: 'unavailable', reason: 'hosting_disabled' }); + }); + + it('given any other wake refusal, should answer unavailable rather than replay to an un-woken app', async () => { + const wakePublishedApp = vi.fn( + async () => ({ outcome: 'refused', reason: 'no_machine' }) as unknown as WakePublishedAppResult, + ); + const decision = await resolveAppRoute( + 'acme.pagespace.app', + deps({ findAppBySubdomain: async () => row({ status: 'stopped' }), wakePublishedApp }), + ); + expect(decision).toEqual({ kind: 'unavailable', reason: 'failed' }); + }); +}); + describe('resolveAppRoute — the replay key must exist before traffic is replayed', () => { it('given a solvent servable app, should emit a replay carrying a derived state key', async () => { const decision = await resolveAppRoute('acme.pagespace.app', deps()); @@ -277,6 +373,26 @@ describe('defaultAppRouterDeps — the real edge is wired to the real readers', expect(tier).toBe('pro'); }); + // The identical function `app-billing.ts` hands the meter and the wake gate — + // not an equivalent bound the same way, the same reference — so "the router + // asks the wrong payer" is structurally impossible rather than a convention + // that can drift. + it('resolves the payer through the IDENTICAL function the meter and wake gate use', () => { + expect(defaultAppRouterDeps.resolvePayerId).toBe(defaultAppBillingDeps.resolvePayerId); + }); + + // Behavioural, not identity: the default binding is an arrow closing over + // `defaultAppLifecycleMeteringDeps`, so it must be shown to delegate with the + // right deps rather than compared by reference. + it('wakes a stopped app through the real wake seam, with the real lifecycle deps', async () => { + vi.mocked(wakePublishedApp).mockResolvedValue({ outcome: 'woken', app: {} } as unknown as WakePublishedAppResult); + + const result = await defaultAppRouterDeps.wakePublishedApp('app_1'); + + expect(wakePublishedApp).toHaveBeenCalledWith('app_1', defaultAppLifecycleMeteringDeps); + expect(result).toEqual({ outcome: 'woken', app: {} }); + }); + // The row reader is module-private, so it is pinned by what it queries: the // published_apps table, keyed on `subdomain`, one row. it('reads the published_apps row for the subdomain it is asked about', async () => { diff --git a/packages/lib/src/services/app-hosting/app-lifecycle-metering.ts b/packages/lib/src/services/app-hosting/app-lifecycle-metering.ts index 3a13886963..eb2ddf9332 100644 --- a/packages/lib/src/services/app-hosting/app-lifecycle-metering.ts +++ b/packages/lib/src/services/app-hosting/app-lifecycle-metering.ts @@ -260,7 +260,21 @@ export async function stopPublishedApp( await mirrorRecentFlyEvents(ref, deps); } - const settled = await settleAndClose(row, stoppedAt, nextStatus, stoppedAt, deps); + // Re-read the row rather than settling against the one captured at the top of + // this function: `stopMachine` above is a slow Fly call, and the awake meter's + // heartbeat can tick — and advance `awakeBilledThrough` — while we were waiting + // on it. Settling against the STALE watermark would re-bill whatever span the + // meter already collected in between; a fresh read settles only what the + // watermark still owes. Falls back to the original row if the fresh read finds + // nothing (the row was deleted under us), which leaves `settleAndClose`'s own + // guards to no-op safely. + const [freshRow] = await db + .select() + .from(publishedApps) + .where(eq(publishedApps.id, row.id)) + .limit(1); + + const settled = await settleAndClose(freshRow ?? row, stoppedAt, nextStatus, stoppedAt, deps); return { outcome: 'stopped', status: nextStatus, billedSeconds: settled.billedSeconds }; } @@ -415,6 +429,31 @@ async function settleAndClose( return { billedSeconds, failed: false }; } +/** + * Claim an abandoned tail exclusively: a CAS that clears `awakeBilledThrough` + * and `awakeHoldId` off the row, guarded on the exact watermark this caller read. + * + * A concurrent wake reads the same stale row and computes the same plan, so + * without this guard both would bill the same span. Only the first write to land + * matches the guard — `awakeBilledThrough` on the row still equals what was read + * — and clears it; the second finds the column already `null` and matches no + * row, learning it lost the race before it ever calls `trackUsage`. + */ +async function claimAbandonedTail(row: PublishedApp): Promise { + const [claimed] = await db + .update(publishedApps) + .set({ awakeBilledThrough: null, awakeHoldId: null }) + .where( + and( + eq(publishedApps.id, row.id), + eq(publishedApps.status, row.status), + eq(publishedApps.awakeBilledThrough, row.awakeBilledThrough as Date), + ), + ) + .returning({ id: publishedApps.id }); + return !!claimed; +} + /** * Settle a tail that a FAILED close left behind on a non-running row, at the wake * that is about to overwrite it. @@ -443,6 +482,17 @@ async function settleAbandonedTail( if (row.awakeBilledThrough === null || row.lastStopAt === null) return; const plan = planAwakeSettle({ billedThrough: row.awakeBilledThrough, now: row.lastStopAt }); + + // Claim the tail EXCLUSIVELY, before doing anything with it: a guarded CAS that + // clears the watermark (and the hold riding with it) off the row. Whoever wins + // this write is the only caller that may bill or release this tail — a second + // wake racing this one, or this same wake retried after a `start_failed`, reads + // `awakeBilledThrough: null` and returns above before ever reaching here. That + // is what makes the tail settle at-most-once instead of "once per wake attempt + // until one finally succeeds". + const claimed = await claimAbandonedTail(row); + if (!claimed) return; // lost the race, or a previous attempt already cleared it + if (plan.action !== 'settle') { // Nothing billable was stranded; return the reservation the failed close kept. if (row.awakeHoldId) await deps.billing.releaseHold(row.awakeHoldId); @@ -451,8 +501,9 @@ async function settleAbandonedTail( const payerId = await deps.billing.resolvePayerId({ driveId: row.driveId }); if (!payerId) { - // Unresolvable drive — never substitute a payer. The tail stays on the row and - // the wake below overwrites it, so this IS the loss; say so rather than warn. + // Unresolvable drive — never substitute a payer. The watermark is already + // claimed off the row above, so there is no later retry that could recover + // this span; say so at ERROR rather than warn. loggers.ai.error( 'Published-app abandoned tail could not be billed: the owning drive did not resolve — this span is lost', new Error('abandoned tail payer unresolved'), diff --git a/packages/lib/src/services/app-hosting/router-core.ts b/packages/lib/src/services/app-hosting/router-core.ts index b7e9b03009..1dcca39f03 100644 --- a/packages/lib/src/services/app-hosting/router-core.ts +++ b/packages/lib/src/services/app-hosting/router-core.ts @@ -138,8 +138,22 @@ export function parseAppHost(rawHost: string, apex: string): AppHost { return { kind: 'subdomain', subdomain: label }; } -/** Statuses whose app has something live to serve. */ -const SERVABLE_STATUSES = new Set(['running', 'stopped', 'deploying']); +/** + * Statuses whose app has something live to serve THROUGH THIS DECISION ALONE. + * + * `'stopped'` is deliberately absent. Fly's proxy auto-starts a stopped target on + * replay (`autostart: true`), but that start is invisible to the app: no status + * flip, no `awakeBilledThrough` stamp, no hold — the awake meter only reads + * `status = 'running'` rows, so a machine Fly quietly started stays unmetered + * until something else stops it. `wakePublishedApp` is the seam that does all of + * that bookkeeping alongside the balance gate; a stopped app must go through it + * before this function is ever asked, which is why the router (`router.ts`) + * intercepts `'stopped'` and calls it BEFORE building the `RoutableApp` this + * decides on. A `'stopped'` row reaching here regardless — a caller that skipped + * that step — resolves through the unrecognized-status branch below rather than + * being replayed to and silently going unbilled. + */ +const SERVABLE_STATUSES = new Set(['running', 'deploying']); export interface AppRouteInput { /** The row the hostname resolved to, or null when nothing did. */ diff --git a/packages/lib/src/services/app-hosting/router.ts b/packages/lib/src/services/app-hosting/router.ts index e9d604803a..878bbfee03 100644 --- a/packages/lib/src/services/app-hosting/router.ts +++ b/packages/lib/src/services/app-hosting/router.ts @@ -39,7 +39,9 @@ import { eq } from '@pagespace/db/operators'; import { publishedApps } from '@pagespace/db/schema/published-apps'; import { hasSpendableBalance } from '../../billing/credit-gate'; import { resolveTier } from '../../billing/credit-balance'; +import { defaultAppBillingDeps } from './app-billing'; import { isAppHostingEnabled } from './app-hosting-env'; +import { defaultAppLifecycleMeteringDeps, wakePublishedApp, type WakePublishedAppResult } from './app-lifecycle-metering'; import { derivePublishedAppReplayKey } from './app-replay-key'; import { resolveAppReplaySecret, resolvePublishedAppsApex } from './routing-env'; import { @@ -58,6 +60,20 @@ export interface AppRouterDeps { replaySecret: () => string; /** `published_apps` row for a subdomain, or null. */ findAppBySubdomain: (subdomain: string) => Promise; + /** + * Wake a `stopped` app through the real seam — gate, hold, start, stamp — rather + * than letting Fly's `autostart` silently start an unmetered machine. See the + * `SERVABLE_STATUSES` comment in `router-core.ts` for why `'stopped'` never + * reaches the pure decision directly. + */ + wakePublishedApp: (publishedAppId: string) => Promise; + /** + * Who pays — resolved the SAME way the awake-seconds meter resolves it + * (`drives.ownerId`, via `resolveEnvPayerId`). Null means unresolvable (a stale + * read of a drive mid-delete); the caller refuses rather than substituting a + * payer or falling back to a denormalized column the meter does not charge. + */ + resolvePayerId: (input: { driveId: string }) => Promise; /** The payer's subscription tier — the allowance the balance is judged against. */ resolveTier: (userId: string) => Promise; /** Whether the payer can still spend. */ @@ -72,17 +88,15 @@ export interface PublishedAppRouteRow { tier: string; machineId: string | null; /** - * Who pays — `published_apps.ownerId`, denormalized at publish time to the - * drive owner (`resolveEnvPayerId` semantics). - * - * Read from the row rather than re-resolved through the env and drive on every - * request, and that is a correctness point as much as a performance one: the - * balance gate must ask about the SAME payer the awake-seconds meter charges, - * and the meter charges this column. Re-deriving the payer here could disagree - * with it mid-flight (a drive ownership transfer between the two reads) and - * park an app whose actual payer is solvent. + * The app's owning drive — NOT `published_apps.ownerId`. The balance gate must + * ask about the SAME payer the awake-seconds meter charges, and the meter + * charges `drives.ownerId` (via `resolveEnvPayerId`), never the denormalized + * `ownerId` column, which exists only for indexing and cascade reach and can + * drift from the drive's real owner (a transfer, a stale write). Gating on the + * wrong payer would decide admission against one person's balance while the + * charge lands on another's. */ - ownerId: string; + driveId: string; } async function findAppBySubdomainRow(subdomain: string): Promise { @@ -93,7 +107,7 @@ async function findAppBySubdomainRow(subdomain: string): Promise wakePublishedApp(publishedAppId, defaultAppLifecycleMeteringDeps), + // The SAME resolver `app-billing.ts`'s `defaultAppBillingDeps` hands the meter + // and the wake gate — not an equivalent, the identical function — so a drift + // between "who the router asks" and "who the meter charges" is structurally + // impossible rather than merely kept in sync by convention. + resolvePayerId: defaultAppBillingDeps.resolvePayerId, resolveTier: (userId) => resolveTier(userId), hasSpendableBalance: (userId, tier) => hasSpendableBalance(userId, tier as Parameters[1]), @@ -144,9 +164,41 @@ export async function resolveAppRoute( const app = await deps.findAppBySubdomain(host.subdomain); if (!app) return { kind: 'not_found', reason: 'no_such_app' }; + // A STOPPED app never goes to `decideAppRoute` as-is: replaying to it would let + // Fly's own `autostart` start the machine with no bookkeeping behind it — no + // status flip, no `awakeBilledThrough` stamp, no hold — and the awake meter, + // which only reads `status = 'running'` rows, would never see it running at + // all. `wakePublishedApp` is the seam that does that bookkeeping alongside the + // balance gate, so it runs here, BEFORE the row is ever handed to the pure + // decision. A woken app is decided on as `running`; anything else answers the + // request directly. + let effectiveStatus = app.status; + if (app.status === 'stopped') { + const wakeResult = await deps.wakePublishedApp(app.id); + switch (wakeResult.outcome) { + case 'woken': + effectiveStatus = 'running'; + break; + case 'parked': + return { kind: 'parked', reason: 'out_of_credits' }; + case 'start_failed': + return { kind: 'unavailable', reason: 'failed' }; + case 'refused': + // 'disabled' means hosting was switched off between this route's own + // `isEnabled()` check above and this call — answer exactly as that check + // would have. Every other refusal ('not_found', 'no_machine', + // 'not_wakeable', 'unresolved_payer') means this row cannot be woken + // right now, which a router that has never heard of it would call + // "not serving yet". + return wakeResult.reason === 'disabled' + ? { kind: 'unavailable', reason: 'hosting_disabled' } + : { kind: 'unavailable', reason: 'failed' }; + } + } + const routable: RoutableApp = { flyAppName: app.flyAppName, - status: app.status, + status: effectiveStatus, tier: app.tier, hasMachine: app.machineId !== null, }; @@ -167,8 +219,13 @@ export async function resolveAppRoute( // than a hope. `decideAppRoute` still re-checks the tier itself, so the skip // here can never quietly become the policy. if (app.tier === 'metered') { - const tier = await deps.resolveTier(app.ownerId); - const balanceOk = await deps.hasSpendableBalance(app.ownerId, tier); + const payerId = await deps.resolvePayerId({ driveId: app.driveId }); + // An unresolvable drive fails CLOSED here — the router has no honest payer to + // ask, so it refuses exactly as the wake gate does for the same condition, + // rather than assuming a balance nobody can vouch for. + const balanceOk = payerId + ? await deps.hasSpendableBalance(payerId, await deps.resolveTier(payerId)) + : false; if (!balanceOk) { return decideAppRoute({ app: routable, balanceOk: false, replayState: 'pending' }); }