feat(executor): mercury_payment executor — sovereignty + cap + idempotency (REAL MONEY PATH)#108
feat(executor): mercury_payment executor — sovereignty + cap + idempotency (REAL MONEY PATH)#108chitcommit wants to merge 4 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittycommand | 70819c1 | Jun 04 2026, 10:15 AM |
📝 WalkthroughWalkthroughThis PR implements autonomous Mercury payment execution through a new canonical executor, refactors the dispatcher's idempotency and audit semantics to handle indeterminate outcomes, and blocks chat-surface payments. The feature enforces sovereignty autonomy, freshness windows, and amount caps while managing deterministic idempotency and in-flight audit rows. ChangesMercury Payment Autonomous Execution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
To use Codex here, create a Codex account and connect to github. |
Pre-merge validation required (post-#106)This PR's three DB-only refusal tests (sovereignty-stale, amount-cap, replay) were written, typecheck-clean, and skip cleanly without Reviewer / maintainer hand-off — do before merge:
Until that runs, this PR has typecheck + skip-clean test evidence only — a lower bar than #106 (live Neon validated) and #107 (live Neon validated). Flagging explicitly so this doesn't merge on equal footing without that gap closed. |
|
|
… 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>
|
|
To use Codex here, create a Codex account and connect to github. |
fix: 6 critical silent failures closed (commit 64ecd41)Real-money paths now fail loud and visible. Summary of behavior changes: 1. Mercury
|
#108 review C1) Mercury's ACH/wire/check transaction API dedupes creations by the `Idempotency-Key` HTTP header, not by a field in the JSON body. Prior to this fix, `createPayment` sent the key only in the body, so a transport-level retry (network blip, edge timeout, etc.) could create a duplicate ACH transfer — money out twice. Changes: - `mercuryClient.post` accepts an `opts.idempotencyKey` and sets the `Idempotency-Key` HTTP header on the outbound fetch when present. - `createPayment` strips `idempotencyKey` from the request body and forwards it via `opts`, so the header is set on every Mercury POST. - Regression test in `tests/meta/executors/mercury-payment-failures.spec.ts` captures the outbound `Request` via the existing injected-fetch pattern and asserts `headers.get('Idempotency-Key')` equals the executor's computed idempotency key. No mocks beyond the FetchImpl injection already used by the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review C1 fix landed — Mercury
|
|
To use Codex here, create a Codex account and connect to github. |
chitcommit
force-pushed
the
feat/meta-executors-registry
branch
from
747a995 to
fc60bfc
Compare
…mpotency REAL MONEY PATH. Stacked on #106 (feat/meta-executors-registry). Adds `meta/executors/mercury-payment.ts` — concrete executor for the mercury_payment intent type, plus a shared pure runner that ActionAgent's chat tool (`execute_payment`) now delegates to so chat + autonomous surfaces share one implementation. Refusal gates (executor returns ok=false, dispatcher writes a 'payment_refusal' row to cc_actions_log; Mercury is NOT called): - sovereignty_not_autonomous — decision !== 'autonomous' - sovereignty_stale — snapshot older than MERCURY_SOVEREIGNTY_FRESHNESS_MS (60s, tighter than dispatch's default 5min) - amount_cap_exceeded — payload.amount_cents > MERCURY_AUTONOMOUS_AMOUNT_CAP_USD * 100 - invalid_payload — zod schema rejected the intent payload - missing_token — KV mercury:token:{account_slug} absent - mercury_api_failure — Mercury HTTP returned null Idempotency uses the dispatcher's content-addressable key (sha256(intent.id:attempt:intent_type)) as supplied via ctx.idempotencyKey. Because intent.id is immutable and attempt is deterministic from prior audit rows, this is functionally equivalent to a payload-derived key for replay protection. The Mercury client receives the same key as its own idempotencyKey so Mercury de-dupes on it too. Files: - meta/executors/mercury-payment.ts (new) - meta/executors/index.ts (barrel: side-effect import) - src/agents/tools/actions.ts (execute_payment delegates to runMercuryPayment) - src/index.ts (Env: MERCURY_AUTONOMOUS_AMOUNT_CAP_USD) - tests/meta/executors/mercury-payment.spec.ts (3 DB-only refusal cases) - docs/runbooks/mercury-payment-executor.md (operator runbook) Does NOT modify meta/executors/{dispatch,registry,types}.ts from #106. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 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>
#108 review C1) Mercury's ACH/wire/check transaction API dedupes creations by the `Idempotency-Key` HTTP header, not by a field in the JSON body. Prior to this fix, `createPayment` sent the key only in the body, so a transport-level retry (network blip, edge timeout, etc.) could create a duplicate ACH transfer — money out twice. Changes: - `mercuryClient.post` accepts an `opts.idempotencyKey` and sets the `Idempotency-Key` HTTP header on the outbound fetch when present. - `createPayment` strips `idempotencyKey` from the request body and forwards it via `opts`, so the header is set on every Mercury POST. - Regression test in `tests/meta/executors/mercury-payment-failures.spec.ts` captures the outbound `Request` via the existing injected-fetch pattern and asserts `headers.get('Idempotency-Key')` equals the executor's computed idempotency key. No mocks beyond the FetchImpl injection already used by the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n, not terminal failed Adversarial review (silent-failure-hunter) + ChittyConnect concierge ruling: recording 409-collision / network-lost-response / 5xx / unparseable-2xx as terminal `failed` buries payments that MAY have moved money — executeIntent then blocks retry on terminal state and dispatch replays `failed` instead of triggering the in_flight_unknown reconciliation runbook. Two-part fix (both mandatory): - meta/executors/mercury-payment.ts: classify the "money may have moved" set (409 idempotency_collision, network, parse-on-2xx, http>=500) as `indeterminate`; the run() mapping records them as `in_flight` with action_type `payment_indeterminate`. Definite 4xx (!=409) and explicit Mercury status:"failed" stay terminal `failed` (no money moved). - meta/intent.ts: executeIntent skips failIntent when result.indeterminate, leaving the intent claimable so the next dispatch pass reaches in_flight_unknown. Without this the audit-status change is inert. - meta/executors/types.ts + dispatch.ts: propagate `indeterminate` on ExecutorRunOutput -> ExecutorResult. - mercury-payment.ts docstring: correct the idempotency-key formula to sha256(intent.id:intent_type) (NO attempt) — a per-attempt key defeats Mercury dedup and is a double-spend vector. Do not reintroduce. Tests: - tests/meta/executors/mercury-payment-failures.spec.ts: assert indeterminate true for 5xx/409/network, falsy for explicit 2xx status:"failed". 10/10 pass locally (injected fetch, real Response, no mocks). - tests/meta/executor-pr106-criticals.spec.ts: re-assert to the deterministic- key contract — attempt is COUNT(*)+1 (2 after a seeded row), sovereignty refusal returns replayed:falsy (not true); outcome guarantees (one refusal row, terminal intent) preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chitcommit
force-pushed
the
feat/executor-mercury-payment
branch
from
70819c1 to
a2e159d
Compare
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittycommand | a2e159d | Jun 10 2026, 12:51 PM |
|
|
To use Codex here, create a Codex account and connect to github. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
meta/executors/dispatch.ts (1)
403-418: 💤 Low valueConsider verifying the UPDATE affected a row.
updateAuditRowdoesn't check if the UPDATE matched any rows. If theiddoesn't exist (unexpected, but possible due to race or bug), the call succeeds silently with zero rows affected. Adding aRETURNING idand checking for an empty result would surface this early rather than leaving an inconsistent state.♻️ Suggested defensive check
async function updateAuditRow( sql: NeonQueryFunction<false, false>, id: string, patch: AuditUpdate, ): Promise<void> { - await sql` + const rows = await sql` UPDATE cc_actions_log SET action_type = ${patch.actionType}, description = ${patch.description}, status = ${patch.status}, error_message = ${patch.errorMessage}, response_payload = ${patch.responsePayload ? JSON.stringify(patch.responsePayload) : null}::jsonb, metadata = ${JSON.stringify(patch.metadata)}::jsonb WHERE id = ${id}::uuid + RETURNING id - `; + ` as unknown as Array<{ id: string }>; + if (!rows.length) { + throw new Error(`updateAuditRow: no row found for id=${id}`); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@meta/executors/dispatch.ts` around lines 403 - 418, updateAuditRow currently issues an UPDATE against cc_actions_log but never verifies any rows were affected; modify updateAuditRow (the async function taking sql: NeonQueryFunction, id: string, patch: AuditUpdate) to append RETURNING id to the query, inspect the query result, and throw or surface an error if no id is returned (indicating zero rows updated) so callers can detect a missing/invalid id rather than silently proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/runbooks/mercury-payment-executor.md`:
- Around line 88-117: The manual-approval section is outdated: the chat surface
`execute_payment` path now unconditionally refuses Mercury payments (`refusal
reason: chat_surface_refuses_mercury`, see
tests/meta/executors/mercury-payment-failures.spec.ts) so you must remove or
rewrite any instructions that tell operators to approve via the chat runner;
instead state that operators must use the dashboard or an out-of-band payment
mechanism, record the payment in `cc_actions_log` (since chat will not create an
approval row with `intent_id`), and then manually update the failed intent
metadata (the existing `UPDATE cc_intents ...` guidance and keys
`manually_approved_via_chat`, `approved_by`, `approved_at` should be revised to
reflect that the approval will reference the dashboard/out-of-band
`cc_actions_log.id` and not a chat action); also add a brief note referencing
`chat_surface_refuses_mercury` and the test file to justify the change.
- Around line 193-210: Update the idempotency key formula in the runbook to
match the implementation and tests: change the documented SHA-256 input from
sha256("{intent.id}:{attempt}:{intent_type}") to
sha256("{intent.id}:{intent_type}"), removing the "{attempt}" segment so the
idempotency key is deterministic on intent.id and intent_type only and aligns
with the executor contract and the test expectations referenced by the executor
PR.
In `@meta/intent.ts`:
- Around line 555-564: The code in executeIntent uses await failIntent(env,
intentId, result.error ?? 'unknown error').catch(() => null) which silently
swallows failures; replace this pattern by calling the safer helper
safeFailIntent(env, intentId, result.error ?? 'unknown error') (or import/export
safeFailIntent from the dispatch module if not available) so that failures to
mark the intent are surfaced/handled, or at minimum log the caught error before
swallowing (use processLogger or the module logger) instead of .catch(() =>
null); update the executeIntent call site to reference safeFailIntent and ensure
intentId, env and result.error are passed through unchanged.
---
Nitpick comments:
In `@meta/executors/dispatch.ts`:
- Around line 403-418: updateAuditRow currently issues an UPDATE against
cc_actions_log but never verifies any rows were affected; modify updateAuditRow
(the async function taking sql: NeonQueryFunction, id: string, patch:
AuditUpdate) to append RETURNING id to the query, inspect the query result, and
throw or surface an error if no id is returned (indicating zero rows updated) so
callers can detect a missing/invalid id rather than silently proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 905461f3-f431-4201-9773-75c8032b4a39
📒 Files selected for processing (12)
docs/runbooks/mercury-payment-executor.mdmeta/executors/dispatch.tsmeta/executors/index.tsmeta/executors/mercury-payment.tsmeta/executors/types.tsmeta/intent.tssrc/agents/tools/actions.tssrc/index.tssrc/lib/integrations.tstests/meta/executor-pr106-criticals.spec.tstests/meta/executors/mercury-payment-failures.spec.tstests/meta/executors/mercury-payment.spec.ts
| ## How to manually approve a `requires_human` mercury_payment | ||
|
|
||
| When sovereignty produces `requires_human` for a mercury_payment intent: | ||
|
|
||
| 1. The dispatcher writes a `sovereignty_refusal` row to `cc_actions_log` | ||
| (or the executor writes `payment_refusal` if dispatch's freshness | ||
| window was longer than 60s and let the snapshot through). | ||
| 2. The intent status moves to `failed`. The current build does NOT | ||
| auto-create a parallel approval queue entry; manual approval requires: | ||
| a. Inspecting the refusal via the dashboard or | ||
| `SELECT ... FROM cc_actions_log WHERE intent_id = '<id>'`. | ||
| b. Performing the payment via the ActionAgent chat surface | ||
| (`execute_payment` tool), which routes through the same pure | ||
| runner but supplies an `autonomous` + fresh snapshot because the | ||
| human typing in chat IS the approval event. The chat path writes | ||
| its own `cc_actions_log` row without `intent_id`. | ||
| c. Then manually updating the failed intent's metadata to reference | ||
| the chat-executed `cc_actions_log.id` for audit linkage: | ||
| ```sql | ||
| UPDATE cc_intents | ||
| SET metadata = COALESCE(metadata, '{}'::jsonb) || jsonb_build_object( | ||
| 'manually_approved_via_chat', '<chat_actions_log_id>', | ||
| 'approved_by', '<chitty_id>', | ||
| 'approved_at', NOW()::text | ||
| ) | ||
| WHERE id = '<intent_id>'; | ||
| ``` | ||
|
|
||
| A dedicated dashboard approval flow is a follow-up (tracked in the | ||
| ADR-001 amendment notes). |
There was a problem hiding this comment.
Manual approval procedure is outdated and incorrect.
Lines 100-103 instruct operators to perform manual approval via the chat surface execute_payment tool, stating it "routes through the same pure runner but supplies an autonomous + fresh snapshot."
However, per the PR objectives and the test at tests/meta/executors/mercury-payment-failures.spec.ts:217-253, the chat surface was changed to refuse Mercury payments unconditionally (refusal reason: chat_surface_refuses_mercury) and directs users to the dashboard. The described chat-based approval flow no longer works.
This section must be rewritten to reflect the current implementation, which blocks Mercury payments from the chat surface entirely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/runbooks/mercury-payment-executor.md` around lines 88 - 117, The
manual-approval section is outdated: the chat surface `execute_payment` path now
unconditionally refuses Mercury payments (`refusal reason:
chat_surface_refuses_mercury`, see
tests/meta/executors/mercury-payment-failures.spec.ts) so you must remove or
rewrite any instructions that tell operators to approve via the chat runner;
instead state that operators must use the dashboard or an out-of-band payment
mechanism, record the payment in `cc_actions_log` (since chat will not create an
approval row with `intent_id`), and then manually update the failed intent
metadata (the existing `UPDATE cc_intents ...` guidance and keys
`manually_approved_via_chat`, `approved_by`, `approved_at` should be revised to
reflect that the approval will reference the dashboard/out-of-band
`cc_actions_log.id` and not a chat action); also add a brief note referencing
`chat_surface_refuses_mercury` and the test file to justify the change.
| ## Idempotency | ||
|
|
||
| The dispatcher computes a deterministic idempotency key per attempt: | ||
|
|
||
| ``` | ||
| sha256("{intent.id}:{attempt}:{intent_type}") | ||
| ``` | ||
|
|
||
| The partial unique index | ||
| `cc_actions_log (intent_id, idempotency_key) WHERE intent_id IS NOT NULL | ||
| AND idempotency_key IS NOT NULL` enforces single-row-per-attempt at the | ||
| database level. The dispatcher also short-circuits on any prior terminal | ||
| (`completed` or `failed`) row for the same `intent_id`, so a replay of a | ||
| finished intent returns the prior result without re-executing. | ||
|
|
||
| The Mercury API call passes `ctx.idempotencyKey` as Mercury's own | ||
| `idempotencyKey`, so Mercury de-dupes on the same value if the executor | ||
| is re-invoked between writing the audit row and Mercury responding. |
There was a problem hiding this comment.
Idempotency key formula is incorrect.
The formula at lines 197-199 includes {attempt}:
sha256("{intent.id}:{attempt}:{intent_type}")
However, per the PR objectives and the test comment at tests/meta/executor-pr106-criticals.spec.ts:101-102, the idempotency key was refactored to exclude the attempt number and is deterministic on intent.id + intent_type only:
sha256("{intent.id}:{intent_type}")
The runbook must be corrected to remove {attempt} from the formula to match the implemented contract.
📝 Proposed fix
-sha256("{intent.id}:{attempt}:{intent_type}")
+sha256("{intent.id}:{intent_type}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Idempotency | |
| The dispatcher computes a deterministic idempotency key per attempt: | |
| ``` | |
| sha256("{intent.id}:{attempt}:{intent_type}") | |
| ``` | |
| The partial unique index | |
| `cc_actions_log (intent_id, idempotency_key) WHERE intent_id IS NOT NULL | |
| AND idempotency_key IS NOT NULL` enforces single-row-per-attempt at the | |
| database level. The dispatcher also short-circuits on any prior terminal | |
| (`completed` or `failed`) row for the same `intent_id`, so a replay of a | |
| finished intent returns the prior result without re-executing. | |
| The Mercury API call passes `ctx.idempotencyKey` as Mercury's own | |
| `idempotencyKey`, so Mercury de-dupes on the same value if the executor | |
| is re-invoked between writing the audit row and Mercury responding. | |
| ## Idempotency | |
| The dispatcher computes a deterministic idempotency key per attempt: | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 197-197: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/runbooks/mercury-payment-executor.md` around lines 193 - 210, Update the
idempotency key formula in the runbook to match the implementation and tests:
change the documented SHA-256 input from
sha256("{intent.id}:{attempt}:{intent_type}") to
sha256("{intent.id}:{intent_type}"), removing the "{attempt}" segment so the
idempotency key is deterministic on intent.id and intent_type only and aligns
with the executor contract and the test expectations referenced by the executor
PR.
| } else if (!result.replayed && !result.indeterminate) { | ||
| // Only mark failed on a fresh, DETERMINATE failure. Replays must not | ||
| // overwrite terminal state, and indeterminate outcomes (money may have | ||
| // moved — Mercury 409/network/5xx/unparseable-2xx) must NOT go terminal: | ||
| // the audit row is left `in_flight` so the next dispatch pass reaches the | ||
| // `in_flight_unknown` reconciliation branch instead of being buried as | ||
| // failed. See meta/executors/mercury-payment.ts + dispatch.ts. | ||
| await failIntent(env, intentId, result.error ?? 'unknown error').catch( | ||
| () => null, | ||
| ); |
There was a problem hiding this comment.
The failIntent error swallowing pattern persists here.
Line 562-564 still uses .catch(() => null), which silently drops errors if failIntent throws (e.g., Neon outage). The dispatcher now uses safeFailIntent to surface these failures, but executeIntent retains the old pattern. If marking the intent failed is important, this swallows failures and can leave intents stuck in claimed/running without progressing.
Consider using safeFailIntent here as well, or at minimum logging the error before swallowing.
🛠️ Suggested improvement
- await failIntent(env, intentId, result.error ?? 'unknown error').catch(
- () => null,
- );
+ await failIntent(env, intentId, result.error ?? 'unknown error').catch((err) => {
+ console.error(
+ `[meta/intent] failIntent threw for intent=${intentId}: ${err instanceof Error ? err.message : String(err)}`,
+ );
+ });Or import and use safeFailIntent from the dispatch module (would need to export it first).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (!result.replayed && !result.indeterminate) { | |
| // Only mark failed on a fresh, DETERMINATE failure. Replays must not | |
| // overwrite terminal state, and indeterminate outcomes (money may have | |
| // moved — Mercury 409/network/5xx/unparseable-2xx) must NOT go terminal: | |
| // the audit row is left `in_flight` so the next dispatch pass reaches the | |
| // `in_flight_unknown` reconciliation branch instead of being buried as | |
| // failed. See meta/executors/mercury-payment.ts + dispatch.ts. | |
| await failIntent(env, intentId, result.error ?? 'unknown error').catch( | |
| () => null, | |
| ); | |
| } else if (!result.replayed && !result.indeterminate) { | |
| // Only mark failed on a fresh, DETERMINATE failure. Replays must not | |
| // overwrite terminal state, and indeterminate outcomes (money may have | |
| // moved — Mercury 409/network/5xx/unparseable-2xx) must NOT go terminal: | |
| // the audit row is left `in_flight` so the next dispatch pass reaches the | |
| // `in_flight_unknown` reconciliation branch instead of being buried as | |
| // failed. See meta/executors/mercury-payment.ts + dispatch.ts. | |
| await failIntent(env, intentId, result.error ?? 'unknown error').catch((err) => { | |
| console.error( | |
| `[meta/intent] failIntent threw for intent=${intentId}: ${err instanceof Error ? err.message : String(err)}`, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@meta/intent.ts` around lines 555 - 564, The code in executeIntent uses await
failIntent(env, intentId, result.error ?? 'unknown error').catch(() => null)
which silently swallows failures; replace this pattern by calling the safer
helper safeFailIntent(env, intentId, result.error ?? 'unknown error') (or
import/export safeFailIntent from the dispatch module if not available) so that
failures to mark the intent are surfaced/handled, or at minimum log the caught
error before swallowing (use processLogger or the module logger) instead of
.catch(() => null); update the executeIntent call site to reference
safeFailIntent and ensure intentId, env and result.error are passed through
unchanged.
… build Replaying #105 onto current main (executor registry from #106) broke the daemon build two ways. Fix both so #105 ships a daemon that builds on the VM. 1. entrypoint.ts passed an `executor` callback to runLeaderLoop — removed in #106 when the loop began dispatching through the canonical executor registry (executeIntent). Drop the stub: an empty registry routes every claimed intent through dispatch's "no executor registered" throw → failIntent → `failed`, never silent `done`, preserving the PR #105 Codex-P1 safety property without a callback. 2. The executor registry (meta/executors/{types,dispatch,update-obligation- status}.ts) hard-imported the Workers `Env` from src/index, neither available nor compilable in the daemon's NodeNext + node-types build. Per ADR-001 the registry is consumed by BOTH the Worker and the daemon, so it must not depend on Workers-only types. Introduce a minimal structural `ExecutorEnv { DATABASE_URL?, HYPERDRIVE? }` (the only env the registry reads; getSql already cast to exactly this). Worker `Env` stays structurally assignable — ActionAgent callers unchanged. No index signature: keeps typo-safety on env reads for the real-money mercury path. Also add the explicit `/index.js` extension to meta/intent.ts's dynamic `import('./executors')` (required under NodeNext; accepted by the Worker's Bundler resolution). Verified: `npm run typecheck` (Worker, Bundler) and `npm run build:daemon` (NodeNext) both exit 0. Follow-up (needs `workflow` scope, separate push): add `tsc -p daemon/runtime/tsconfig.daemon.json --noEmit` as a CI step so the daemon build is gated — catches the #108 ripple (mercury also imports Workers `Env`) at PR time instead of VM install. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Adds
meta/executors/mercury-payment.ts— a concreteIntentExecutorfor themercury_paymentintent_type, plus a shared pure runner (runMercuryPayment) thatActionAgent's chat toolexecute_paymentnow delegates to. Chat and autonomous surfaces share one implementation; only the audit-row plumbing differs (dispatcher writes audit withintent_idfor autonomous, chat writes its existing audit row withoutintent_id).Refusal conditions (exhaustive)
The executor refuses (returns
ok: false, dispatcher writes apayment_refusalrow tocc_actions_log, Mercury is NOT called) in any of these cases:sovereignty_not_autonomousctx.sovereignty.decision !== 'autonomous'(evenrequires_humanrefuses for money path)sovereignty_staleassessedAtolder thanMERCURY_SOVEREIGNTY_FRESHNESS_MS(60s) at executor entryamount_cap_exceededpayload.amount_cents > env.MERCURY_AUTONOMOUS_AMOUNT_CAP_USD * 100(default cap 500 USD)invalid_payloadmissing_tokenmercury:token:{account_slug}absent fromCOMMAND_KVmercury_api_failurenull(5xx / network)Sovereignty: belt-and-suspenders
The executor's 60s freshness check is the second gate. Callers of
executeIntentformercury_paymentSHOULD passfreshnessMs: MERCURY_SOVEREIGNTY_FRESHNESS_MSso the dispatcher re-reckons againsttrust.chitty.ccfirst. Documented in the runbook and the executor's module comment.Idempotency — note on key derivation
Task spec asked for an idempotency key derived from
(intent.id, recipient_id, amount_cents, currency). PR #106's dispatcher is frozen by constraint and supplies a content-addressable key viactx.idempotencyKey:sha256(\"{intent.id}:{attempt}:{intent_type}\"). Becauseintent.idis immutable andattemptis deterministic from priorcc_actions_logrows, dispatch's key is functionally equivalent for replay protection — a retry of the same intent reuses the same key. We pass that same value as Mercury'sidempotencyKeyso Mercury also de-dupes on it.If reviewers prefer the payload-derived formula, that would require a change to
meta/executors/dispatch.ts(out of scope for this PR per constraint).What is NOT modified from PR #106
meta/executors/dispatch.tsmeta/executors/registry.tsmeta/executors/types.tsmeta/executors/index.tsbarrel adds one side-effect import (./mercury-payment).Files
meta/executors/mercury-payment.ts(new) — executor +runMercuryPaymentpure runnermeta/executors/index.ts— barrel side-effect importsrc/agents/tools/actions.ts—execute_paymentdelegates torunMercuryPaymentsrc/index.ts—Env.MERCURY_AUTONOMOUS_AMOUNT_CAP_USDtests/meta/executors/mercury-payment.spec.ts— 3 DB-only refusal-path integration testsdocs/runbooks/mercury-payment-executor.md— operator runbookTest evidence
npx tsc --noEmitclean.SKIP_INTEGRATION=1 npx vitest rungreen (15 passed, 15 skipped — mercury-payment.spec.ts skips correctly withoutDATABASE_URL).Real-Neon DB-only tests covered (all assert single
cc_actions_logrow withaction_type='payment_refusal',status='failed',attempt=1, idempotency key is 64-hex):error_messagematches/sovereignty snapshot older than/.amount_cents=50_001, cap is 50_000 cents → refusal withmetadata.refusal_reason='amount_cap_exceeded'.missing_token; secondexecuteIntentfor the same intent ID short-circuits on dispatcher's prior-terminal-row lookup. Only one row incc_actions_log; second call hasreplayed=trueand the sameactionLogIdandidempotencyKey.These tests skip without
DATABASE_URLand require a Neon branch that has the PR #106 migration applied (migrations/0004_chief_skin.sql— addsintent_id,attempt,idempotency_keycolumns plus the partial unique index). Per the task constraint that I do not autonomously provision destructive Neon ops, I did not run them against live Neon from this session — reviewer should run on a branch off the #106 migration or wait for #106 merge then run on a dev branch.Mercury sandbox path
MERCURY_INTEGRATION_TEST=1branch is not exercised — the chittycommand repo has no Mercury sandbox harness and the executor's stop condition triggered: I was not asked to invent one. The runbook documents the manual operator path for testing against Mercury sandbox.Stop-condition outcomes
src/lib/integrations.ts::mercuryClient— reused, not duplicated.cc_actions_log.target_idis auuidcolumn so the executor setstargetId = payload.obligation_id ?? nulland carriesrecipient_idinresponse_payloadinstead — recipient is still indexed via metadata andtarget_type='recipient'.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
Refactor