Skip to content

id(currency_poc): an Abandoned intent is not terminal — submit() never checks record state, so an identical execute() retry resurrects a conversion recovery deliberately abandoned #4678

Closed

Description

@dezren39

Problem

In pkgs/id/src/currency_poc, the terminal intent states are not enforced by the executor. A record the recovery scanner (or an operator) deliberately abandoned can be posted afterwards by nothing more than an identical retry — the abandon decision is silently undone and the money moves.

  • intent.rs:33-37 (origin/main) — IntentState::Abandoned is documented as "The scanner (or an operator) decided not to resubmit", and is_unresolved() (:41-43) treats it as resolved/terminal: the scanner will never look at the record again.
  • executor.rs:256-266 — prepare()'s never-replace guard compares a persisted record against the new request with the state deliberately normalised away (comparable.state = record.state, :262-263), so an identical request against an Abandoned (or already-Posted) record returns the existing record as-is, terminal state included.
  • executor.rs:284-307 — submit() never inspects record.state at all: it unconditionally overwrites it with Submitted (:285), submits the legs, and persists Posted (:302).
  • executor.rs:310-314 — execute() is prepare() + submit(), so the normal retry path composes the two gaps: retrying an abandoned conversion posts it.

recovery.rs:129-136 is where Abandon lands: the record is persisted Abandoned precisely so the conversion does not happen (e.g. the rate authorisation is stale, an operator intervened). Nothing downstream of that decision checks it.

Verified this run (laptop probe, verbatim origin/main sources)

Probe crate ~/git/fleet/probe-2026-11-02-laneA-abandoned/ on the laptop compiles the currency_poc modules extracted from origin/main (10e4ec1; blob-verified — executor.rs blob 0582e8ea, md5 500df828; intent.rs md5 d276f6cc; recovery.rs md5 8d40cf83) against the in-repo InMemoryLedger/InMemoryIntentStore:

  • Probe A (retry resurrects): prepare() a conversion (id 777) → reconcile(RecoveryPolicy::Abandon) returns [(777, Abandoned)], stored state Abandoned, 0 legs in the ledger → an identical execute() retry returns Ok with state Posted and 3 legs in the ledger.
  • Probe B (no guard in submit itself): a record hand-set to Abandoned and persisted, passed directly to submit(), returns Ok(()), ends Posted, 3 legs posted.

cargo test on the probe: 2 passed (both probes assert the buggy outcome).

Note on provenance: the laptop primary's working tree is currently a dirty repo-keeper snapshot (local HEAD 04286381a, off origin/main), so the probe was built from git show origin/main:... extractions, not the working tree; the cited line numbers are origin/main's.

Why it matters

Abandoned is the recovery system's only "do not move this money" verdict. If any later identical retry — a client timeout retry, a caller re-running execute() after a crash it misread, an operator script — silently reverses that verdict and posts the conversion, the state machine's terminal states are decorative. The asymmetry is the tell: the scanner respects Abandoned forever (is_unresolved excludes it), while the executor ignores it completely, so the two halves of the same module disagree about what the state means. (Re-submitting an already-Posted record is harmless today only because every leg returns Exists; the missing guard is the same hole.)

Acceptance criteria

  • ConversionExecutor::submit refuses a record in a terminal state (Posted, Abandoned) with an error naming the conversion and its state, instead of overwriting the state and submitting.
  • execute() on a conversion whose persisted intent is Abandoned fails (does not post, state stays Abandoned); on an already-Posted intent it returns the posted record without resubmitting, or fails explicitly — either way the behaviour is deliberate and tested.
  • Regression tests in currency_poc/tests.rs: abandon-then-retry (via reconcile + execute) posts nothing and leaves the record Abandoned; direct submit() on an Abandoned record errors; existing scenario tests still pass.

Related: #2289 (the PoC), #2063 (design), #4133 (leg-vector vacuous Posted, fixed). Distinct from #4669 (short result vector), #4664 (preflight gaps), #4578 (FileIntentStore listing).

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    task/triageQueue packet kind: verify, reproduce, and route this item
    source/creatorFiled by the new-issue-creator job
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    app/idPackage/app: id (pkgs/ or apps/ in this repo)
    effort/sSmall: expected under 2 hours of work
    on Nov 3, 2026
  2. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

  3. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 3, 2026
  4. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · done

    • Agent: queue worker (repo-worker-code), lane H (code-h)
    • Session / ID: task 6653
    • Found via: queue claim
    • Task: Triage id(currency_poc): an Abandoned intent is not terminal — submit() never checks record state, so an identical execute() retry resurrects a conversion recovery deliberately abandoned #4678 — Abandoned currency_poc intent resurrected by an identical execute() retry.
    • Verdict: CONFIRMED on current main (origin/main 10e4ec1), routed to fix as task 6657. My confirmation is an independent direct code read (laptop git show origin/main:), not a re-run of the creator's probe: in pkgs/id/src/currency_poc/executor.rs, prepare() handles an identical retry by cloning the persisted record, setting comparable.state = record.state (normalising its state away for the equality check), and returning Ok(existing) whatever that state is — including Abandoned. submit() then unconditionally sets Submitted, calls backend.create_transfers(&record.legs), and sets Posted; it never reads the incoming record.state. And recovery.rs:129-134 shows RecoveryPolicy::Abandon deliberately persisting IntentState::Abandoned — the terminal decision the executor silently overrides. This matches the creator's probe results on the same head (abandon-then-retry → Ok/Posted, 3 legs; direct submit on Abandoned → Ok/Posted, 3 legs).
    • Routing: task 6657 (small_fix, dedupe repo-health:v1:fix:developing-today/code:issue:4678) — submit() refuses terminal-state records (Posted/Abandoned) naming conversion + state; execute() on an Abandoned intent fails without posting and leaves it Abandoned; already-Posted behavior made deliberate and tested; regression tests in currency_poc/tests.rs. The id(currency_poc): preflight passes requests build_legs cannot build — fee_amount>0 with no fee_account, and transfer_ids count never checked (extra IDs silently dropped) #4664 preflight gap stays on its own track.
  5. 61 remaining items

  6. dezren39 commented on Nov 5, 2026

    @dezren39
    MemberAuthor

    agent-note:v1
    state: triaged
    task: 8123
    queue: repo-health-code-pilot
    worker: repo-worker-code
    execution_lane: opencode
    session: ses_e5e55953603ffennLm9a7k769C
    at: 2026-11-05T06:46:00Z
    verdict: confirmed-open; routed-to-fix
    follow_up: task 9912

    Triage verdict: CONFIRMED on fresh current main (origin/main e5d459e). prepare() still normalizes the persisted state during identity comparison and reuses an identical terminal intent; submit() still overwrites to Submitted and calls the ledger without checking state. The issue is OPEN. Fresh checks found no hard-stop labels, no open PR referencing #4678, no matching remote branch, and no live fix task using the standard key. The earlier fix handoff (task 6657) had already been reported as evaporated; no current branch/PR supersedes it.

    Routing: enqueued task 9912 (small_fix) with dedupe key repo-health:v1:fix:developing-today/code:issue:4678. It covers terminal-state refusal, deliberate Posted retry behavior, and the abandon-then-retry/direct-submit regression tests. #2063, #2289, #4133, #4664, and #4578 are closed context; #4669 remains open but is explicitly separate. No code edits were made in this triage task; verification was the live source/repo/PR/queue check plus the probe evidence recorded above.

  7. dezren39 commented on Nov 5, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

    • Agent: queue worker (repo-worker-code), lane code-i
    • Session / ID: task 9912
    • Found via: queue claim
    • Task: Refuse terminal currency_poc intents: submit() rejects Posted/Abandoned before any write or ledger call; add regression tests (direct submit on Abandoned; reconcile(Abandon) then identical execute() retry fails, stays Abandoned, zero legs).
    • Expectation: effort s — done when a PR is open (merged if verified) with the focused currency_poc tests passing.
    • Links: task 9912
  8. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 5, 2026
  9. added 2 commits that reference this issue on Nov 5, 2026
  10. dezren39 commented on Nov 5, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · done

    • Agent: queue worker (repo-worker-code), lane code-i
    • Session / ID: task 9912
    • Found via: queue claim
    • Task: submit() refuses Posted/Abandoned records before any state write or ledger call (error names the conversion ID and state); identical execute() retry after reconcile(Abandon) fails and stays Abandoned with zero legs; already-Posted execute fails explicitly.
    • Expectation: met — PR fix(id): refuse terminal currency_poc intents in submit() (#4678) #6105 merged (merge c4783fb). Laptop: cargo test --manifest-path pkgs/id/Cargo.toml currency_poc 47 passed / 0 failed incl. 3 new regressions; feature-gated live test updated per Codex P2 (dispositioned on its thread, reply 4477548618) and compile-verified with --features currency-poc-tb; Codex re-review clean. PR-tier checks stayed QUEUED in the runner backlog — merged under merge authority on laptop verification; main-tier CI is the backstop.
    • Links: PR fix(id): refuse terminal currency_poc intents in submit() (#4678) #6105 · branch queue/9912-4678-terminal-intent-guard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)source/creatorFiled by the new-issue-creator jobtask/fixhas a fix task queuedtask/small-fixQueue packet kind: bounded fix with reproduction and targeted tests

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions