Skip to content

test(buyer): re-award an agent that actually won, not a presumed one (#230) - #287

Merged
orveth merged 1 commit into
devfrom
fix/230-overspend-test-assumes-race-winner
Jul 30, 2026
Merged

test(buyer): re-award an agent that actually won, not a presumed one (#230)#287
orveth merged 1 commit into
devfrom
fix/230-overspend-test-assumes-race-winner

Conversation

@orveth

@orveth orveth commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Unblocks the dev→main cut. The money-path row went red on #283 (640 passed; 1 failed) after #282 landed. It is a pre-existing test bug, not a #282 regression and not a production defect.

What was wrong

n_equal_agents_cannot_overspend_the_shared_budget spawns five agents that race for the money lock. The first three to acquire it win 30 sats each of a shared 100; the other two are correctly refused. Which three win is decided by scheduling order — nothing pins it.

The idempotency leg then re-awarded format!("{:064x}", 0), assuming agent 0 won. When scheduling leaves agent 0 among the two refused, it has no reservations.job_id row — so that call is a fresh reserve of 30 against the 10 the winners left, which reserve correctly refuses:

idempotent re-award: Reserve(InsufficientAvailable { requested: 30, available: 10, bound: Wallet })

available: 10 = min(cap, balance) 100 − the winners' 90. Those numbers only occur if agent 0 was not a winner, which is what pins the diagnosis.

The production path was never at fault

Worth stating outright, because a red row on a money guard reads as a broken guard until someone says otherwise. The two aggregate asserts three lines above the panicwinners.len() == 3 and reserved == 90 — were passing. The invariant held. What happened is the overspend guard refusing an overspend under adversarial scheduling.

The fix

Each task now returns the job id it won; the leg re-awards winners[0]. It therefore asserts idempotency against a job that was genuinely awarded, which is what it was always trying to assert. Strictly stronger than before, and no longer luck-dependent.

Verification

  • Red-proven: pointing the re-award at a never-awarded id reproduces the CI panic byte for byte (requested: 30, available: 10, bound: Wallet), so the mechanism is demonstrated on demand rather than argued.
  • Full money-path suite: 641 passed; 0 failed; 2 ignored — meets the structural acceptance rule (max N passed ≥ 641, 0 failed).
  • Three discriminators established it was non-deterministic before any fix: CI red / local same-commit-same-features 641-0 green / the test in isolation 1 passed, 642 filtered out.

Why it surfaced now, and why that is good news

#282 added two tests to the binary, which perturbed the parallel schedule. The bug has been latent since #230 — any scheduling change (a new test, a faster runner, a different core count) would do it.

It was structurally invisible on main, where the money suite still runs inside the acp job and gets skipped when that job is reaped. #276's decouple is the reason anyone can see it: the instrument caught a latent defect in an overspend guard on its first run with real counts.

Notes for reviewers

  • Coupling flagged by buyer-orch: their §1 chokepoint PR changes award_with_reservation's return type to an enum and touches every call site in this file, including the line this PR edits. This goes first (it gates the tag) and they rebase onto it — the cheap direction. Visible only from the file list, not from either PR alone.
  • Test file belongs to buyer-orch (buyer daemon: concurrency audit + no-overspend proof for N equal agents #230). They and the lead both ruled that I land it with the diagnosis attached and buyer-orch reviewing post-hoc.
  • ★ Generalisation worth a follow-up, raised by market-orch: assert the invariant, never the identity of a race winner. A luck-dependent test guarding overspend is the kind that eventually gets muted as "that flaky test," and then the guard is gone without anyone deciding to remove it. A scan of the other money tests for this shape is not in this PR.

…230)

`n_equal_agents_cannot_overspend_the_shared_budget` spawns five agents that race
for the money lock. The first three to acquire it win 30 sats each of a shared
100; the other two are correctly refused. Which three win is decided by
scheduling order, and nothing pins it.

The idempotency leg then re-awarded `format!("{:064x}", 0)`, assuming agent 0 was
among the winners. When scheduling leaves agent 0 among the two refused it has no
`reservations.job_id` row, so that call is not a re-award at all -- it is a fresh
reserve of 30 against the 10 the winners left, which `reserve` correctly refuses:

    idempotent re-award: Reserve(InsufficientAvailable {
        requested: 30, available: 10, bound: Wallet })

So the test read a correct overspend refusal as an idempotency failure. It was
green by scheduling luck since it was written, and #282 adding two tests to the
binary perturbed the schedule enough to surface it.

Each task now reports the job id it won, and the leg re-awards `winners[0]`, so
it asserts idempotency against a job that was genuinely awarded -- which is what
it was always trying to assert.

Worth stating because a red row on a money guard reads as a broken guard: the
production path was never at fault. The aggregate asserts three lines above the
panic (`winners.len() == 3`, `reserved == 90`) had already proven the invariant
held. The overspend guard was refusing an overspend under adversarial scheduling.

Red-proven: pointing the re-award at a never-awarded id reproduces the CI panic
byte for byte.

Reported-by: keeper:mobee-market-orch
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mobee Ready Ready Preview Jul 30, 2026 1:13am

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant