Skip to content

Commit 64ecd41

Browse files
chitcommitclaude
andcommitted
fix(mercury): close 4 critical silent failures — discriminated union, atomic audit, real sovereignty, fail-closed (PR #108)
Real-money attack/loss vectors fixed in the mercury_payment executor: 1. Mercury post() helper now returns a discriminated PostResult — every failure mode (network / http / parse / 409 idempotency_collision) is surfaced to the executor instead of collapsing to null. runMercuryPayment and the audit row carry httpStatus + bodySnippet + failureKind so operators can diagnose without re-calling Mercury. 2. Executor maps Mercury's 2xx body.status into the audit vocabulary: sent/posted/delivered -> completed, pending -> in_progress, failed -> failed (refusal), requires_review -> pending_review. No more blanket "completed" stamp on 2xx-with-error-envelope. 3. Audit row is PRE-WRITTEN as in_flight BEFORE the Mercury call and UPDATED in place afterwards. Idempotency key is now deterministic on intent_id only (NOT attempt), so the partial unique index on (intent_id, idempotency_key) blocks duplicate placeholders across retries and Mercury de-dupes on the same key end-to-end. A retry that finds a prior in_flight row refuses with in_flight_unknown (the only safe default — Mercury state must be reconciled by an operator). 4. Chat surface (src/agents/tools/actions.ts::execute_payment) no longer passes a synthetic { decision: 'autonomous' } snapshot to runMercuryPayment. The chat tool factory has no access to the chat actor ChittyID and therefore cannot perform a real assessSovereignty() call, so it REFUSES Mercury payments with a "use the dashboard" message and audits the attempt. This closes the silent-bypass of the money-path sovereignty gate. 5. failIntent() failures in dispatch.ts are no longer swallowed by .catch(() => null). A new safeFailIntent() wrapper logs a stable "audit_write_failed_during_failIntent" token to console.error and re-throws so the daemon's outer loop catches and applies backoff. 6. account_slug is normalized (lowercase + [a-z0-9-]) BEFORE the KV lookup; if normalization changes the value the input is refused with invalid_account_slug — no silent fallback to a different token. Tests: 8 new failure-path tests drive runMercuryPayment with an injected fetch double (real Response objects — no mocks of Mercury or the DB). Existing 3 DB integration tests untouched and still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c3509f6 commit 64ecd41

6 files changed

Lines changed: 688 additions & 146 deletions

File tree

meta/executors/dispatch.ts

Lines changed: 186 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,24 @@ function getSql(env: Env): NeonQueryFunction<false, false> {
4141

4242
/**
4343
* Stable, content-addressable idempotency key.
44-
* Formula: sha256("{intent.id}:{attempt}:{intent.intentType}")
44+
*
45+
* Formula: sha256("{intent.id}:{intent.intentType}")
46+
*
47+
* The key is deterministic on `intent.id` (NOT `attempt`) so that:
48+
* 1. The partial unique index on `cc_actions_log (intent_id, idempotency_key)`
49+
* prevents duplicate audit rows across daemon retries — every retry of
50+
* the same intent reuses the same key and the index rejects the second
51+
* INSERT.
52+
* 2. Mercury de-dupes on the same value end-to-end — a retry after a Neon
53+
* blip cannot cause double-spend because Mercury sees the same key.
54+
* 3. The pre-write in_flight row uses this key, and a successful run
55+
* UPDATEs that same row in place (see writePreAudit / updateAuditRow).
4556
*/
4657
async function computeIdempotencyKey(
4758
intentId: string,
48-
attempt: number,
4959
intentType: string,
5060
): Promise<string> {
51-
const data = new TextEncoder().encode(`${intentId}:${attempt}:${intentType}`);
61+
const data = new TextEncoder().encode(`${intentId}:${intentType}`);
5262
const digest = await crypto.subtle.digest('SHA-256', data);
5363
return [...new Uint8Array(digest)]
5464
.map((b) => b.toString(16).padStart(2, '0'))
@@ -83,18 +93,23 @@ export async function dispatch(
8393
): Promise<ExecutorResult> {
8494
const sql = getSql(env);
8595
const freshnessMs = options.freshnessMs ?? SOVEREIGNTY_FRESHNESS_MS;
96+
const idempotencyKey = await computeIdempotencyKey(intent.id, intent.intentType);
8697

87-
// 1. Replay short-circuit: if any prior cc_actions_log row exists for this
88-
// intent that hit a terminal status ('completed' or 'failed'), the intent
89-
// has already been dispatched and the second call must NOT re-execute.
90-
// The unique partial index on (intent_id, idempotency_key) backs this
91-
// invariant for retry attempts; the latest-terminal-row lookup backs the
92-
// "intent already done" case.
98+
// 1. Replay / safety lookup. With the key deterministic on intent_id, every
99+
// prior row for this intent shares the same idempotency_key. We look up
100+
// by (intent_id, idempotency_key) regardless of status because:
101+
// - 'completed' / 'failed' / 'pending_review' / 'in_progress' →
102+
// terminal-or-known; short-circuit and replay the prior outcome.
103+
// - 'in_flight' → the previous attempt's outcome is UNKNOWN. Mercury
104+
// may or may not have moved money. We MUST NOT re-call Mercury.
105+
// Refuse with `in_flight_unknown`; the operator runbook resolves by
106+
// querying Mercury directly using this idempotency key and then
107+
// manually setting the row to its true terminal state.
93108
const priorRows = (await sql`
94109
SELECT id, status, response_payload, error_message, idempotency_key, attempt
95110
FROM cc_actions_log
96111
WHERE intent_id = ${intent.id}::uuid
97-
AND status IN ('completed', 'failed')
112+
AND idempotency_key = ${idempotencyKey}
98113
ORDER BY executed_at DESC
99114
LIMIT 1
100115
`) as unknown as Array<{
@@ -107,29 +122,37 @@ export async function dispatch(
107122
}>;
108123
if (priorRows[0]) {
109124
const prior = priorRows[0];
125+
if (prior.status === 'in_flight') {
126+
const errMsg =
127+
`prior attempt is in_flight (audit_log id=${prior.id}, idempotency_key=${idempotencyKey}) — ` +
128+
`Mercury state is unknown; operator must reconcile before retry`;
129+
console.error(`[meta/executors/dispatch] in_flight_unknown for intent ${intent.id}: ${errMsg}`);
130+
return {
131+
ok: false,
132+
idempotencyKey,
133+
actionLogId: prior.id,
134+
error: errMsg,
135+
replayed: true,
136+
};
137+
}
138+
// Any other status → known terminal outcome, replay it.
110139
return {
111140
ok: prior.status === 'completed',
112-
idempotencyKey: prior.idempotency_key,
141+
idempotencyKey,
113142
actionLogId: prior.id,
114143
data: prior.response_payload ?? undefined,
115144
error: prior.error_message ?? undefined,
116145
replayed: true,
117146
};
118147
}
119148

120-
// 2. Compute attempt number (prior rows + 1) and idempotency key for the
121-
// new audit row. The partial unique index on (intent_id, idempotency_key)
122-
// prevents two concurrent dispatchers from writing duplicate rows for
123-
// the same attempt.
149+
// 2. Compute attempt number. With the deterministic key, `attempt` is now
150+
// purely audit metadata (the unique partial index dedupes us, not the
151+
// attempt number). Still useful for operators inspecting retry history.
124152
const [{ count: priorCount } = { count: 0 }] = (await sql`
125153
SELECT COUNT(*)::int AS count FROM cc_actions_log WHERE intent_id = ${intent.id}::uuid
126154
`) as unknown as Array<{ count: number }>;
127155
const attempt = (priorCount ?? 0) + 1;
128-
const idempotencyKey = await computeIdempotencyKey(
129-
intent.id,
130-
attempt,
131-
intent.intentType,
132-
);
133156

134157
// 3. Re-reckon sovereignty if snapshot stale.
135158
let sovereignty: SovereigntyAssessmentSnapshot;
@@ -161,7 +184,7 @@ export async function dispatch(
161184
requestPayload: intent.payload,
162185
metadata: { reason: 'no_actor_for_reckon' },
163186
});
164-
await failIntent(env, intent.id, errMsg).catch(() => null);
187+
await safeFailIntent(env, intent.id, errMsg);
165188
return { ok: false, idempotencyKey, error: errMsg };
166189
}
167190
const result = await assessSovereignty(
@@ -191,7 +214,7 @@ export async function dispatch(
191214
requestPayload: intent.payload,
192215
metadata: { sovereignty },
193216
});
194-
await failIntent(env, intent.id, refusal).catch(() => null);
217+
await safeFailIntent(env, intent.id, refusal);
195218
return {
196219
ok: false,
197220
idempotencyKey,
@@ -208,7 +231,43 @@ export async function dispatch(
208231
throw new Error(errMsg);
209232
}
210233

211-
// 5. Execute.
234+
// 5. PRE-WRITE the audit row as `in_flight` BEFORE invoking the executor.
235+
// This is the atomicity guarantee for the money path: if the executor
236+
// moves money and the post-update fails (e.g., Neon outage), the
237+
// in_flight row still exists and a retry will see it via the prior-row
238+
// lookup above and refuse with `in_flight_unknown`. The operator runbook
239+
// then reconciles by querying Mercury with `idempotencyKey`.
240+
//
241+
// The partial unique index on (intent_id, idempotency_key) prevents two
242+
// concurrent dispatchers from racing to create the same in_flight row —
243+
// the second INSERT will fail with a unique violation. We let that throw
244+
// so the daemon's outer loop treats it as a retryable error.
245+
let auditId: string;
246+
try {
247+
auditId = await writeAuditRow(sql, {
248+
intentId: intent.id,
249+
attempt,
250+
idempotencyKey,
251+
actionType: 'payment_in_flight',
252+
targetType: 'intent',
253+
targetId: intent.id,
254+
description: `mercury_payment in_flight for intent ${intent.id}`,
255+
status: 'in_flight',
256+
errorMessage: null,
257+
responsePayload: null,
258+
requestPayload: intent.payload,
259+
metadata: { sovereignty, canonicalUri: executor.canonicalUri, phase: 'pre_execute' },
260+
});
261+
} catch (err) {
262+
// Pre-write failed — money has NOT moved. Surface to caller; daemon
263+
// retries with backoff. failIntent is NOT called (the intent stays
264+
// claimable on the next pass).
265+
const errMsg = err instanceof Error ? err.message : String(err);
266+
console.error(`[meta/executors/dispatch] pre-write audit failed for intent ${intent.id}: ${errMsg}`);
267+
throw new Error(`audit_write_failed_pre_execute: ${errMsg}`);
268+
}
269+
270+
// 6. Execute.
212271
const ctx: ExecutorContext = {
213272
env,
214273
sql,
@@ -222,42 +281,61 @@ export async function dispatch(
222281
runOutput = await executor.run(ctx);
223282
} catch (err) {
224283
const errMsg = err instanceof Error ? err.message : String(err);
225-
const auditId = await writeAuditRow(sql, {
226-
intentId: intent.id,
227-
attempt,
228-
idempotencyKey,
229-
actionType: 'executor_error',
230-
targetType: 'intent',
231-
targetId: intent.id,
232-
description: `executor threw: ${errMsg}`,
233-
status: 'failed',
234-
errorMessage: errMsg,
235-
responsePayload: null,
236-
requestPayload: intent.payload,
237-
metadata: { sovereignty, canonicalUri: executor.canonicalUri },
238-
});
284+
// Executor threw — update the in_flight row to failed (NOT a new row;
285+
// same idempotency key, same record). If the update itself throws, we
286+
// surface to the daemon so the row remains `in_flight` and a retry will
287+
// hit the `in_flight_unknown` branch.
288+
try {
289+
await updateAuditRow(sql, auditId, {
290+
actionType: 'executor_error',
291+
description: `executor threw: ${errMsg}`,
292+
status: 'failed',
293+
errorMessage: errMsg,
294+
responsePayload: null,
295+
metadata: { sovereignty, canonicalUri: executor.canonicalUri, phase: 'executor_threw' },
296+
});
297+
} catch (updateErr) {
298+
const updateMsg = updateErr instanceof Error ? updateErr.message : String(updateErr);
299+
console.error(
300+
`[meta/executors/dispatch] AUDIT_UPDATE_FAILED_AFTER_EXECUTOR_THREW intent=${intent.id} key=${idempotencyKey}: ${updateMsg}`,
301+
);
302+
throw new Error(
303+
`audit_update_failed_after_executor_threw: original=${errMsg}; update_error=${updateMsg}`,
304+
);
305+
}
239306
return { ok: false, idempotencyKey, actionLogId: auditId, error: errMsg };
240307
}
241308

242-
// 6. Write audit row.
243-
const auditId = await writeAuditRow(sql, {
244-
intentId: intent.id,
245-
attempt,
246-
idempotencyKey,
247-
actionType: runOutput.actionType,
248-
targetType: runOutput.targetType,
249-
targetId: runOutput.targetId ?? null,
250-
description: runOutput.description,
251-
status: runOutput.status,
252-
errorMessage: runOutput.errorMessage ?? null,
253-
responsePayload: runOutput.responsePayload ?? null,
254-
requestPayload: intent.payload,
255-
metadata: {
256-
sovereignty,
257-
canonicalUri: executor.canonicalUri,
258-
...(runOutput.metadata ?? {}),
259-
},
260-
});
309+
// 7. UPDATE the same audit row in place with the executor result. This is
310+
// the atomicity completion: in_flight → terminal status. Failure here
311+
// leaves the row in_flight, which a retry will treat as
312+
// `in_flight_unknown` (safe — Mercury de-dupes on idempotencyKey, so
313+
// operator reconciliation reveals the true state).
314+
try {
315+
await updateAuditRow(sql, auditId, {
316+
actionType: runOutput.actionType,
317+
description: runOutput.description,
318+
status: runOutput.status,
319+
errorMessage: runOutput.errorMessage ?? null,
320+
responsePayload: runOutput.responsePayload ?? null,
321+
metadata: {
322+
sovereignty,
323+
canonicalUri: executor.canonicalUri,
324+
phase: 'post_execute',
325+
...(runOutput.metadata ?? {}),
326+
},
327+
});
328+
} catch (err) {
329+
const errMsg = err instanceof Error ? err.message : String(err);
330+
console.error(
331+
`[meta/executors/dispatch] AUDIT_UPDATE_FAILED_POST_EXECUTE intent=${intent.id} key=${idempotencyKey} executor_ok=${runOutput.ok}: ${errMsg}`,
332+
);
333+
// Surface to daemon — the row is still in_flight, so a retry will be
334+
// refused as in_flight_unknown (correct, since money may have moved).
335+
throw new Error(
336+
`audit_update_failed_post_execute: executor_ok=${runOutput.ok}; update_error=${errMsg}`,
337+
);
338+
}
261339

262340
return {
263341
ok: runOutput.ok,
@@ -268,6 +346,26 @@ export async function dispatch(
268346
};
269347
}
270348

349+
/**
350+
* failIntent wrapper that, if failIntent itself throws (e.g., Neon outage),
351+
* (a) logs to console.error with a stable token for operator alerting and
352+
* (b) re-throws so the daemon's outer loop catches and applies backoff.
353+
* Replaces the prior `.catch(() => null)` pattern which silently dropped
354+
* failure information.
355+
*/
356+
async function safeFailIntent(env: Env, intentId: string, reason: string): Promise<void> {
357+
try {
358+
await failIntent(env, intentId, reason);
359+
} catch (err) {
360+
const errMsg = err instanceof Error ? err.message : String(err);
361+
// Stable token so log aggregators / Alchemist alerting can match.
362+
console.error(
363+
`[meta/executors/dispatch] audit_write_failed_during_failIntent intent=${intentId} reason="${reason}" error=${errMsg}`,
364+
);
365+
throw new Error(`failIntent_threw: intent=${intentId}: ${errMsg}`);
366+
}
367+
}
368+
271369
interface AuditRow {
272370
intentId: string;
273371
attempt: number;
@@ -283,6 +381,38 @@ interface AuditRow {
283381
metadata: Record<string, unknown>;
284382
}
285383

384+
interface AuditUpdate {
385+
actionType: string;
386+
description: string;
387+
status: string;
388+
errorMessage: string | null;
389+
responsePayload: Record<string, unknown> | null;
390+
metadata: Record<string, unknown>;
391+
}
392+
393+
/**
394+
* UPDATE an existing audit row by id. Used to transition the in_flight
395+
* placeholder into its terminal status after the executor returns. We
396+
* intentionally do NOT touch intent_id / attempt / idempotency_key /
397+
* request_payload — those were set at pre-write and are immutable.
398+
*/
399+
async function updateAuditRow(
400+
sql: NeonQueryFunction<false, false>,
401+
id: string,
402+
patch: AuditUpdate,
403+
): Promise<void> {
404+
await sql`
405+
UPDATE cc_actions_log
406+
SET action_type = ${patch.actionType},
407+
description = ${patch.description},
408+
status = ${patch.status},
409+
error_message = ${patch.errorMessage},
410+
response_payload = ${patch.responsePayload ? JSON.stringify(patch.responsePayload) : null}::jsonb,
411+
metadata = ${JSON.stringify(patch.metadata)}::jsonb
412+
WHERE id = ${id}::uuid
413+
`;
414+
}
415+
286416
async function writeAuditRow(
287417
sql: NeonQueryFunction<false, false>,
288418
row: AuditRow,

0 commit comments

Comments
 (0)