The commit dispatcher claims an op with an atomic pending -> committing CAS; if the process dies after side-effects post but before the terminal committed write (or that write fails, the PR #841 log line), the row sat in status='committing' forever: the expire cron only sweeps 'pending'. Add lib/pending-operations/recover-stuck-committing.ts, invoked from the existing daily expire cron (no new vercel.json entry): - Only rows whose updated_at (the claim timestamp: the CAS bumps it via the update_updated_at_column trigger) is older than 15 minutes, well past the 300s Vercel function ceiling, so in-flight executors are never raced. - Positive evidence that side-effects posted finalizes the row to committed with result_data.recovered=true. Evidence exists only where params identify a target with an unambiguous posted state: categorize_transaction (is_transaction_booked RPC, skipped for allow_duplicate), link_transaction_journal_entry (exact tx+entry link), match_transaction_invoice (invoice_payments pair row). - No evidence: terminal rejected with an explanatory result_data, never back to pending (re-execution could duplicate side-effects that posted without a trace). Reason 'stuck_committing' is distinct from 'expired' so the UI badge never claims these rows. - Every terminal write is CAS-guarded on status='committing'; probe errors skip the row for the next run. - One structured 'pending_op_recovery' warn per row (count by outcome); runbook comment added next to the #841 finalize-failure log line. Tests: unit coverage for the decision logic and cron wiring (401, sweep invoked, failure isolation), plus a pg-real test proving row selection, the trustworthy updated_at anchor, committing -> terminal transitions through the real immutability/input-frozen triggers, and the is_transaction_booked evidence substrate. Fixes #843 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
8294899543
commit
25a7261eda
@@ -3,6 +3,10 @@
|
||||
* staged operations are auto-rejected with the commit dispatcher's
|
||||
* result_data shape ({ auto_rejected: true, reason: 'expired' }) so the
|
||||
* /pending UI can render them as "Utgick automatiskt".
|
||||
*
|
||||
* The same run also invokes the stuck-'committing' recovery sweep (#843,
|
||||
* lib/pending-operations/recover-stuck-committing.ts), mocked here: its
|
||||
* decision logic has its own unit + pg-real tests.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
@@ -11,6 +15,15 @@ vi.mock('@/lib/auth/cron', () => ({
|
||||
verifyCronSecret: vi.fn(() => null),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/pending-operations/recover-stuck-committing', () => ({
|
||||
recoverStuckCommittingOperations: vi.fn(async () => ({
|
||||
scanned: 0,
|
||||
committed: 0,
|
||||
rejected: 0,
|
||||
skipped: 0,
|
||||
})),
|
||||
}))
|
||||
|
||||
interface FilterCall {
|
||||
method: string
|
||||
args: unknown[]
|
||||
@@ -57,6 +70,7 @@ vi.mock('@/lib/supabase/server', () => ({
|
||||
import { GET } from '../route'
|
||||
import { verifyCronSecret } from '@/lib/auth/cron'
|
||||
import { createServiceClient } from '@/lib/supabase/server'
|
||||
import { recoverStuckCommittingOperations } from '@/lib/pending-operations/recover-stuck-committing'
|
||||
|
||||
function cronRequest(): Request {
|
||||
return new Request('http://localhost:3000/api/pending-operations/expire/cron')
|
||||
@@ -117,7 +131,45 @@ describe('GET /api/pending-operations/expire/cron', () => {
|
||||
const response = await GET(cronRequest())
|
||||
const json = await response.json()
|
||||
|
||||
expect(json).toEqual({ success: true, expired: 0, cutoff: expect.any(String) })
|
||||
expect(json).toEqual({
|
||||
success: true,
|
||||
expired: 0,
|
||||
cutoff: expect.any(String),
|
||||
recovery: { scanned: 0, committed: 0, rejected: 0, skipped: 0 },
|
||||
})
|
||||
})
|
||||
|
||||
it('invokes the stuck-committing recovery sweep with the service client', async () => {
|
||||
updateResults = [{ data: [], error: null }]
|
||||
vi.mocked(recoverStuckCommittingOperations).mockResolvedValueOnce({
|
||||
scanned: 3,
|
||||
committed: 1,
|
||||
rejected: 2,
|
||||
skipped: 0,
|
||||
})
|
||||
|
||||
const response = await GET(cronRequest())
|
||||
const json = await response.json()
|
||||
|
||||
expect(vi.mocked(recoverStuckCommittingOperations)).toHaveBeenCalledTimes(1)
|
||||
const [client, opts] = vi.mocked(recoverStuckCommittingOperations).mock.calls[0]
|
||||
expect(client).toBe(vi.mocked(createServiceClient).mock.results[0].value)
|
||||
expect(opts?.log).toBeDefined()
|
||||
expect(json.recovery).toEqual({ scanned: 3, committed: 1, rejected: 2, skipped: 0 })
|
||||
})
|
||||
|
||||
it('a recovery sweep failure never masks a successful expiry pass', async () => {
|
||||
updateResults = [{ data: [{ id: 'op-1', company_id: 'c-1' }], error: null }]
|
||||
vi.mocked(recoverStuckCommittingOperations).mockRejectedValueOnce(new Error('sweep boom'))
|
||||
|
||||
const response = await GET(cronRequest())
|
||||
const json = await response.json()
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
expect(json.success).toBe(true)
|
||||
expect(json.expired).toBe(1)
|
||||
// Static marker only: the raw error stays in the structured log.
|
||||
expect(json.recovery).toEqual({ error: 'recovery_sweep_failed' })
|
||||
})
|
||||
|
||||
it('returns 401 without touching the database when cron auth fails', async () => {
|
||||
@@ -129,6 +181,7 @@ describe('GET /api/pending-operations/expire/cron', () => {
|
||||
|
||||
expect(response.status).toBe(401)
|
||||
expect(vi.mocked(createServiceClient)).not.toHaveBeenCalled()
|
||||
expect(vi.mocked(recoverStuckCommittingOperations)).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns an error envelope when the update fails', async () => {
|
||||
|
||||
@@ -2,12 +2,20 @@ import { createServiceClient } from '@/lib/supabase/server'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { withCronContext } from '@/lib/api/with-cron-context'
|
||||
import { errorResponse } from '@/lib/errors/get-structured-error'
|
||||
import { recoverStuckCommittingOperations } from '@/lib/pending-operations/recover-stuck-committing'
|
||||
|
||||
/**
|
||||
* GET /api/pending-operations/expire/cron, daily 02:30 UTC.
|
||||
*
|
||||
* Auto-rejects staged operations that have sat at status='pending' for more
|
||||
* than 30 days. AI agents stage operations for human review; when the chat
|
||||
* Two sweeps per run:
|
||||
*
|
||||
* 1. Expiry: auto-rejects staged operations that have sat at status='pending'
|
||||
* for more than 30 days.
|
||||
* 2. Recovery (#843): drives rows stuck in status='committing' beyond the
|
||||
* safe threshold to a terminal status; see
|
||||
* lib/pending-operations/recover-stuck-committing.ts for the semantics.
|
||||
*
|
||||
* AI agents stage operations for human review; when the chat
|
||||
* session is abandoned the proposal would otherwise linger in the worklist
|
||||
* forever, asking the user to Godkänn/Avvisa something whose context they no
|
||||
* longer remember. A 30-day-old proposal has lost its context regardless of
|
||||
@@ -58,5 +66,18 @@ export const GET = withCronContext('cron.pending_operations_expire', async (_req
|
||||
cutoff: cutoff.toISOString(),
|
||||
})
|
||||
|
||||
return NextResponse.json({ success: true, expired, cutoff: cutoff.toISOString() })
|
||||
// Recovery sweep for rows stuck in 'committing' (#843). Isolated so a
|
||||
// recovery failure never masks a successful expiry pass: the expiry update
|
||||
// above has already been applied at this point.
|
||||
let recovery: Record<string, unknown>
|
||||
try {
|
||||
recovery = { ...(await recoverStuckCommittingOperations(supabase, { log: ctx.log })) }
|
||||
} catch (err) {
|
||||
// Detail goes to the structured log only; the response carries a static
|
||||
// marker so an operator sees the run partially failed.
|
||||
ctx.log.error('pending_op_recovery sweep failed', err as Error)
|
||||
recovery = { error: 'recovery_sweep_failed' }
|
||||
}
|
||||
|
||||
return NextResponse.json({ success: true, expired, cutoff: cutoff.toISOString(), recovery })
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user