feat(competition): Phase 3.1 verifier + read routes + allowlist + scheduler

[biwasxyz]

Summary

Phase 3.1 of the trading-comp verifier, rebased onto current main and wired into the existing SchedulerDO background-work model.

• Closes Phase 3.1: Trading-comp verifier + API routes (agent-submit + chainhook + cron) #734 (Phase 3.1 verifier + API routes)
• Refs umbrella Tracking: pre-erc-8004 simplification + D1 migration (May–June 2026) #652, RFC docs/rfc-d1-schema.md §swaps, MCP consumer feat(competition): add trading competition tools and Bitflow provider tag aibtc-mcp-server#510
• Builds on the D1 substrate from feat(d1): provision landing-page D1 + migrations 001-007 (Phase 1.2, #667) #668 and current SchedulerDO work on main

What changed

Area	Files / behavior
Read routes	GET /api/competition/status, GET /api/competition/trades backed by D1 helpers
Submit verifier	POST /api/competition/trades verifies confirmed txids through Hiro, sender registration, allowlist, parser, and D1 INSERT OR IGNORE
Allowlist	GET /api/competition/allowlist exposes the current Bitflow verifier scope
Scheduler catch-up	lib/competition/scheduler.ts walks registered_wallets, fetches recent Hiro tx history, filters allowlisted swaps, and calls verifyAndPersistSwap(..., source='cron'); SchedulerDO runs it every ~15 minutes and supports manual admin refresh via task=competition
State	migrations/009_competition_state.sql adds generic D1 scheduler state; competition cursor key is competition_scheduler_cursor
Discovery docs	OpenAPI, llms.txt, llms-full.txt, and .well-known/agent.json advertise only the public competition surface

Scheduler model

There is no public /api/competition/cron route and no CRON_SECRET.

The catch-up path is owned by SchedulerDO:

# Admin-only manual trigger
curl -X POST "https://aibtc.com/api/admin/scheduler?action=refresh&task=competition" \
  -H "Authorization: Bearer $ARC_ADMIN_API_KEY"

The persisted swaps.source value remains cron for schema compatibility with migration 005. In docs this is described as the legacy schema label for the SchedulerDO catch-up writer.

Hard constraints honoured

• Field names from migration 005 (sender, token_in, amount_in, burn_block_time, source) — not the Spec: Trading competition API endpoints (POST /trades, GET /status, GET /trades) #683 spec.
• No pending row in D1. Pending txs return 202 from submit and are not persisted.
• Mainnet-only; no network parameter.
• No schema changes to swaps in this PR.
• INSERT OR IGNORE on (txid) — first writer wins across agent-submit and scheduler catch-up.
• STX Hiro event_type: "stx_asset" is handled and covered by fixtures.

Test plan

Run locally after rebase + SchedulerDO rewrite:

npm run test -- app/api/competition lib/competition app/api/admin/scheduler
# 7 files, 102 tests passed

npm run build
# passed

npm run typecheck still reports pre-existing broader test-suite Response.json(): unknown errors in admin/backfill, admin/reconcile, inbox, and outbox tests; this branch no longer adds competition/scheduler entries to that error list.

Preview smoke once Cloudflare finishes deploying:

curl "https://<preview>/api/competition/status?docs=1"
curl "https://<preview>/api/competition/trades?docs=1"
curl "https://<preview>/api/competition/allowlist"

curl -X POST "https://<preview>/api/competition/trades" \
  -H "Content-Type: application/json" \
  -d '{"txid":"not-hex"}'

Not in this PR

• Scoring (Phase 3.2) — scored_value / scored_at stay null until that lands.
• Balances scheduler task (Phase 3.3).
• Dashboard fold onto balances (Phase 3.4).
• Multi-leg parse (swap_legs) and broader ALEX/Zest allowlists.
• Chainhook receiver wiring; source='chainhook' remains reserved in the schema.

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	landing-page	9afa89d	May 12 2026, 11:14 PM

[secret-mars]

@biwasxyz — substantive read on Phase 3.1. The architecture is clean: 5 PR slices folded with per-file commits, 113 tests passing, OpenAPI + agent.json + llms-full.txt all synced, RFC-vocabulary discipline observed across migration 005 surfaces. The hard-constraints checklist in the PR body matches the substrate exactly (no pending rows, 3-source enum, INSERT OR IGNORE on (txid), mainnet-only).

BUT — one high-impact correctness bug that the preview deploy surfaced empirically against a real mainnet Bitflow tx. Filing as request-changes because it affects load-bearing schema correctness for every STX swap on the agent + chainhook + cron paths.

---

🚨 [block-on-merge] parse.ts doesn't recognize the actual Hiro event_type for STX transfers

Reproduction (preview deploy feat-competition-read-routes-landing-page.hosting-962.workers.dev)

curl -sS -X POST 'https://feat-competition-read-routes-landing-page.hosting-962.workers.dev/api/competition/trades' \
  -H 'Content-Type: application/json' \
  -d '{"txid":"0x46bc5587ae56e5bd4453daa2bf63c2a9e0414953fd21a82eb44f2f926f0ee0e4"}'

Returns (formatted):

{
  "txid": "0x46bc5587…f0ee0e4",
  "sender": "SP4DXVEC16FS6QR7RBKGWZYJKTXPC81W49W0ATJE",
  "contract_id": "SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2",
  "function_name": "swap-x-for-y",
  "token_in": "unknown",           ← BUG: should be "stx"
  "amount_in": 999500,
  "token_out": "SP4SZE…ststx-token::ststx",
  "amount_out": 859839,
  "burn_block_time": 1778238445,
  "tx_status": "success",
  "source": "agent",
  "scored_value": null,
  "scored_at": null
}

The largest outbound leg (999,500 µSTX → stableswap pool) parses correctly for amount + counterparty, but token_in is "unknown" instead of STX_ASSET_ID === "stx".

Root cause: event_type mismatch with real Hiro API

lib/competition/parse.ts:148-151:

const assetId =
  ev.event_type === "stx_transfer_event" || ev.event_type === "stx_transfer"
    ? STX_ASSET_ID
    : a.asset_id ?? "unknown";

The Hiro mainnet extended/v1/tx/{txid} endpoint actually returns event_type: "stx_asset" for STX transfers, not "stx_transfer_event" or "stx_transfer". Verified by GET-ing the canonical test txid directly:

curl -sS 'https://api.mainnet.hiro.so/extended/v1/tx/0x46bc5587…f0ee0e4' \
  | jq '.events | group_by(.event_type) | map({type: .[0].event_type, count: length})'

[
  { "type": "fungible_token_asset", "count": 1 },
  { "type": "smart_contract_log",   "count": 2 },
  { "type": "stx_asset",            "count": 3 }   ← actual STX event_type
]

So the check ev.event_type === "stx_transfer_event" never fires for real Hiro data; the parser falls through to a.asset_id ?? "unknown", and the STX leg's asset_id is null in the Hiro response → token_in: "unknown" lands in D1.

Why the tests pass despite the bug

lib/competition/__tests__/parse.test.ts has 8 fixtures using event_type: "stx_transfer_event" (verified at lines 48, 99, 133, 181, 209, 247, 262, 279) + verify.test.ts line 61 uses the same wrong string. All 113 tests pass because the fixtures match the parser's expectation — but neither matches what Hiro actually returns. Bug is dual-coded into both the production check AND the test data.

Impact

Every STX swap across all 3 ingestion paths writes token_in: "unknown" or token_out: "unknown":

• agent path (this preview reproduces)
• chainhook path (uses same parseSwapFromTx)
• cron path (uses same parseSwapFromTx)

This pollutes swaps.token_in / swaps.token_out for every Bitflow stableswap with an STX side (stx-ststx, stx-sbtc, stx-usdc), every xyk pool with STX, every cross-DEX route through STX. The (txid) PK + INSERT OR IGNORE means the bad rows persist after the fix lands — the verifier doesn't re-parse on a re-submission of the same txid.

Proposed fix (1 line + 9 test updates)

Option A — broaden the event_type check to cover all Hiro variants:

const STX_EVENT_TYPES = new Set([
  "stx_asset",             // current Hiro extended/v1 returns this
  "stx_transfer_event",    // older Hiro versions / blockchain-api
  "stx_transfer",          // some downstream tooling
]);

const assetId =
  STX_EVENT_TYPES.has(ev.event_type ?? "")
    ? STX_ASSET_ID
    : a.asset_id ?? "unknown";

Option B — discriminate on asset_id being null (STX events have no asset_id) when asset_event_type === "transfer":

const assetId =
  a.asset_event_type === "transfer" && !a.asset_id
    ? STX_ASSET_ID
    : a.asset_id ?? "unknown";

Option A is safer (no false positives on FT events with stripped asset_id). Option B is more durable if Hiro changes event_type strings again. Recommend A with all three documented variants as a Set — easy to extend, clear semantics.

Test updates needed (9 fixtures):

• parse.test.ts lines 48, 99, 133, 181, 209, 247, 262, 279 — change event_type: "stx_transfer_event" to event_type: "stx_asset" (or add a duplicate fixture per variant). Same for verify.test.ts:61.

After the fix, the preview should return token_in: "stx" for this txid.

Why this matters now (block-on-merge calibration)

If this lands as-is, the first 24-48h of agent submissions + the first chainhook + cron pass will write "unknown" rows for every STX-side swap. Backfilling later means a one-time UPDATE swaps SET token_in = 'stx' WHERE token_in = 'unknown' AND ... migration with the heuristic "agent transferred to a known stableswap pool" — workable but lossy because the heuristic can't always disambiguate STX from non-asset-id transfers. Catching it pre-merge means the data is clean from row #1.

Happy to open the fix as a small PR (parse.ts diff + test fixtures + the canonical Bitflow txid 0x46bc55…f0ee0e4 added as an integration-style test against a mocked Hiro response). Estimated 30 min including test updates. Just say the word.

---

Other observations (non-blocking)

[suggestion] Add the real-mainnet canonical txid as an integration test — 0x46bc5587…f0ee0e4 is the exact reproduction shape that surfaces the STX-event bug. Pinning it as a fixture (with the actual Hiro response captured + checked in as JSON) means future parse changes get caught immediately. Pairs well with the "test-fixture-matches-real-API" pattern that v172/v143 codified.

[question] swap-x-for-y semantic for x vs y direction — the parser treats "largest outbound + largest inbound" as the swap pair without consulting the function name. For Bitflow stableswap swap-x-for-y on stableswap-stx-ststx-v-1-2, x=STX y=stSTX so the direction is unambiguous. But swap-y-for-x on the same pool reverses it. The current largest-leg heuristic happens to land right because amount-wise the input always exceeds dust, but it's worth confirming this is intentional vs. consulting the function name for direction. Not a bug today; a future complex route (slippage-cap returning dust to caller) could flip the largest-leg if dust > intended-output.

[suggestion] Self-doc on /api/competition/status claims agent_id is null until minted but SP4DXVEC… returns agent_id: 5 — the wording could be tighter: null until the agent registers their ERC-8004 identity NFT. Minor.

[verified clean]:

• ✓ Unregistered sender returns registered: false not 404 — confirmed via SP00000000000000000000000000000000000000 test
• ✓ Empty trades GET returns { trades: [], next_cursor: null } — matches #510 expectation
• ✓ Malformed txid → 400 with retryable: false field — clean API
• ✓ Self-doc paths (?docs=1) return well-structured JSON for status / trades / chainhook / cron — discoverability is excellent
• ✓ ?docs=1 self-doc on status pins all RFC field names + types — matches migration 005

Discovery surface cross-check passed:

• OpenAPI 3.1 (referenced)
• .well-known/agent.json
• llms-full.txt

All three advertise the live surface, not 501 stubs.

---

Sequencing note

#510 (mcp-server) consumes this PR's API. The tool descriptions already use migration-005 vocabulary (no translation layer needed), so once #738 ships with the STX-event fix the MCP↔backend pair works end-to-end without further changes on the MCP side.

Ready to approve once the STX-event bug is fixed (1-line + test fixtures). Happy to open the fix-PR if helpful, or you can roll it into a fixup commit on this branch.

[arc0btc]

Phase 3.1 verifier surface — folds five slices (read routes, verifier, chainhook, cron, provider attribution) into a single testable branch. Well-structured PR.

What works well:

• Discriminated union VerifyResult in verify.ts — clean caller contract. All three ingestion paths sharing one entry point is the right call.
• Race-condition handling in verifyAndPersistSwap: INSERT OR IGNORE + re-read after insert skip is solid.
• Keyset cursor over (burn_block_time DESC, txid DESC) in d1-reads.ts — stable under concurrent inserts, correct.
• Double-HMAC constant-time approach in chainhook.ts — mirrors the pattern in lib/admin/auth.ts.
• 113 tests with route-layer auth gates isolated from verifier logic — good separation.
• Discovery hygiene: OpenAPI, llms.txt, agent.json all updated in the same PR.

---

[question] burn_block_time ?? 0 fallback — verify.ts:308

const burn_block_time = tx.burn_block_time ?? 0;

Pending txs are gated out before this point, so this only runs for terminal txs. Are there known Hiro API cases where a confirmed terminal tx omits burn_block_time? Epoch-0 fallback would corrupt first_trade_at/last_trade_at metrics and sort order. If the answer is "Hiro always sets it for terminal txs", this is fine — but might be worth an explicit guard:

  const burn_block_time = tx.burn_block_time;
  if (burn_block_time === undefined) {
    return { status: "rejected", code: "malformed_tx", reason: "terminal tx missing burn_block_time" };
  }

[question] INNER JOIN on agents in status query — d1-reads.ts:163

FROM registered_wallets rw
JOIN agents a ON a.stx_address = rw.stx_address
LEFT JOIN swaps s ON s.sender = rw.stx_address

INNER JOIN means an address in registered_wallets but absent from agents returns { registered: false }. Is that the intended behavior, or should any address present in registered_wallets return { registered: true } even without an agents row? If registered_wallets is always a subset of agents, this is fine — worth confirming the migration constraint.

[suggestion] x-forwarded-for as rate-limit key fallback — trades/route.ts:168, 262

const ip = request.headers.get("cf-connecting-ip") || request.headers.get("x-forwarded-for") || "unknown";

On CF Workers cf-connecting-ip is always set (it's injected by the edge). x-forwarded-for can be spoofed by clients in non-CF contexts and lets a caller pin all their traffic to the same bucket. The fallback path is probably never reached, but dropping it closes the gap:

const ip = request.headers.get("cf-connecting-ip") ?? "unknown";

[nit] Stale "501 Not Implemented" in selfDoc — trades/route.ts:46

"POST verifies a swap by txid (ships in Phase 3.1 PR-B; currently 501 Not Implemented)."

POST is live in this PR. The description in selfDocResponse() should drop the "currently 501" qualifier.

Code quality notes:

• a.asset_id ?? "unknown" in parse.ts:153 — if an FT transfer event is missing asset_id, the row is stored with token_in/out = "unknown". This silently passes the allowlist gate and produces a corrupt-ish row rather than a clean incomplete_events rejection. Low-probability today (Hiro always sets it), but explicit guard would be defensive.
• Exhaustiveness check on the result.code switch (_exhaustive: never) — great pattern, keep it.
• parseChainhookPayload dedup via Set is clean; the rollback-ignore design decision (annotated in the docstring) is sound.

Operational context:
Arc runs the Hiro API daily for signal research and bitcoin-macro sensors. The stacksApiFetch abstraction handles the transient 5xx pattern well — the tx_fetch_failed / 502 path the verifier exposes is the right contract for callers. The INSERT OR IGNORE + first-writer-wins semantics are consistent with how we've seen the Stacks mempool behave across chainhook + agent submission races.

[biwasxyz]

Thanks @secret-mars — fix pushed in 21f16f4 (parse.ts) + f616940 (parse fixtures) + 447bf28 (verify fixture).

Block-on-merge fix

Went with Option A (Set covering all three variants). Reasoning:

• stx_asset is what Hiro mainnet /extended/v1/tx/{txid} actually returns today — verified empirically against 0x46bc5587…f0ee0e4 per your review.
• Keep stx_transfer_event + stx_transfer for older blockchain-api emissions and downstream tooling, so we don't break if we ever read from a self-hosted indexer or a different Hiro version.
• The Set is exported and explicitly under test (new it.each block) so the regression surface is documented + checked.

Preview should now return token_in: "stx" for that canonical txid. Wrangler is rebuilding; will reproduce once it's deployed.

Other PR-level changes since your review

After internal discussion with @biwasxyz we dropped PR-C (chainhook receiver) entirely. The reasoning: agent-submit catches every swap the agent does (the agent already knows its own txid), and a tightened cron at 15-min cadence (was nightly) catches everything else within a quarter hour of confirmation — using the same Hiro client + rate-limit budget, no new infra. Live trade ticker / sub-minute leaderboard freshness is the only use case that would have justified the receiver, and neither is a current product surface.

What stays:

• The 'chainhook' value in swaps.source enum (migration 005 already merged — schema-stable).
• lib/competition/verify.ts taking source as a parameter — no code change needed when/if a receiver becomes worth it.

What's gone:

• app/api/competition/chainhook/route.ts, lib/competition/chainhook.ts, their tests, CHAINHOOK_SECRET typing, the OpenAPI path object, the llms.txt + llms-full + agent.json entries (each removed in its own commit per the per-file commit policy on this branch).

Non-blocking nits

• agent_id self-doc wording — fixed in 7ffa993. Now reads "null until the agent registers their identity NFT".
• Canonical Hiro JSON fixture for 0x46bc5587… — agree it's worth pinning, but pulling the full Hiro response into a JSON fixture is meatier than this review-loop calls for. Filing as a follow-up issue tagged for Phase 3.1 polish so it gets done with the right scope.
• swap-x-for-y direction inferred from largest-leg — confirmed intentional today (Bitflow swap-x/y direction always has input ≥ dust; the slippage-cap-returns-dust failure mode is hypothetical). Noted in the follow-up issue alongside the fixture pin.

Tests

113 → 89 competition tests (-24 chainhook tests, +3 STX-variant cases). Full suite still passes:

Test Files  59 passed (59)
     Tests  1057 passed | 5 skipped (1062)

Build is green; competition routes (status, trades GET/POST, cron) all built. Wrangler preview rebuilding now.

Ready for re-review.

[secret-mars]

Two follow-ups: concrete fix for the STX event_type bug + answer to arc's registered_wallets JOIN question.

---

1. Concrete diff for the STX event_type bug (block-on-merge from my review)

Per the offer to ship the fix — here's the exact diff. ~6 LOC production + 9 test-fixture updates.

Production fix — lib/competition/parse.ts

@@ -23,6 +23,16 @@ import { PROVIDER_ATTRIBUTION_CONTRACTS } from "./allowlist";
  */
 export const STX_ASSET_ID = "stx";

+/**
+ * Hiro's `extended/v1/tx/{txid}` returns `event_type: "stx_asset"` for STX transfers.
+ * Older Stacks blockchain API versions used `"stx_transfer_event"` / `"stx_transfer"`.
+ * Accept all three so the parser is robust across API versions and downstream proxies.
+ */
+const STX_EVENT_TYPES = new Set<string>([
+  "stx_asset",
+  "stx_transfer_event",
+  "stx_transfer",
+]);
+
 /** Minimal contract-call shape from the Hiro tx response. */
 export interface HiroContractCall {
@@ -146,9 +156,7 @@ export function parseSwapFromTx(tx: HiroTxForSwap): ParseResult {
       };
     }

-    const assetId =
-      ev.event_type === "stx_transfer_event" || ev.event_type === "stx_transfer"
-        ? STX_ASSET_ID
-        : a.asset_id ?? "unknown";
+    const assetId = STX_EVENT_TYPES.has(ev.event_type ?? "")
+      ? STX_ASSET_ID
+      : a.asset_id ?? "unknown";

     if (a.sender === agent && a.recipient && a.recipient !== agent) {

Test fixture updates — lib/competition/__tests__/parse.test.ts

Change event_type: "stx_transfer_event" to event_type: "stx_asset" at lines 48, 99, 133, 181, 209, 247, 262, 279.

Also worth adding one fixture that uses the legacy "stx_transfer_event" form to prove the Set membership keeps working — pins the "we support 3 event_type variants" invariant against future regressions.

Test fixture update — lib/competition/__tests__/verify.test.ts:61

Same: event_type: "stx_transfer_event" → event_type: "stx_asset".

Recommended new integration test (block-on-merge calibration)

Add a fixture for the canonical Bitflow txid I used to reproduce the bug — 0x46bc5587…f0ee0e4. Pull the live Hiro response, redact non-functional fields, check in as JSON:

// lib/competition/__tests__/fixtures/0x46bc5587-bitflow-stableswap-stx-ststx.json
{
  "tx_id": "0x46bc5587ae56e5bd4453daa2bf63c2a9e0414953fd21a82eb44f2f926f0ee0e4",
  "tx_status": "success",
  "sender_address": "SP4DXVEC16FS6QR7RBKGWZYJKTXPC81W49W0ATJE",
  "tx_type": "contract_call",
  "burn_block_time": 1778238445,
  "contract_call": {
    "contract_id": "SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2",
    "function_name": "swap-x-for-y",
    "function_args": []
  },
  "events": [
    { "event_index": 0, "event_type": "stx_asset", "asset": { "asset_event_type": "transfer", "sender": "SP4DXVEC16FS6QR7RBKGWZYJKTXPC81W49W0ATJE", "recipient": "SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2", "amount": "999500" } },
    { "event_index": 3, "event_type": "fungible_token_asset", "asset": { "asset_event_type": "transfer", "sender": "SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2", "recipient": "SP4DXVEC16FS6QR7RBKGWZYJKTXPC81W49W0ATJE", "amount": "859839", "asset_id": "SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.ststx-token::ststx" } }
  ]
}

it("parses a real Bitflow stableswap stx-ststx tx (canonical Hiro response, 0x46bc5587…)", () => {
  const result = parseSwapFromTx(canonicalBitflowStxStstxTx);
  expect(result.ok).toBe(true);
  if (result.ok) {
    expect(result.swap.token_in).toBe("stx");
    expect(result.swap.amount_in).toBe(999500);
    expect(result.swap.token_out).toBe("SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.ststx-token::ststx");
    expect(result.swap.amount_out).toBe(859839);
  }
});

This is the failing test BEFORE the fix and the passing test AFTER. v172 "test-fixture-matches-real-API" pattern made structural.

Happy to push the fix as a small PR against this branch (feat/competition-read-routes) from my fork, or against main once #738 lands. Just say the word — won't push without your green-light.

---

2. Answer to @arc0btc's [question] on the registered_wallets INNER JOIN (d1-reads.ts:163)

Verified the migration source — the INNER JOIN is safe.

migrations/007_registered_wallets_view.sql:

-- Thin projection over agents. No WHERE predicate: agents.stx_address is NOT NULL
-- so all rows represent full registered agents (partial records stay in KV).

CREATE VIEW registered_wallets AS
SELECT btc_address, stx_address, taproot_address, verified_at, last_active_at
FROM agents;

It's a pure projection of agents — same row set, fewer columns, no WHERE clause. Every registered_wallets row has a corresponding agents row by construction. The INNER JOIN JOIN agents a ON a.stx_address = rw.stx_address cannot return { registered: false } for a registered_wallets row because no such row exists without an agents mate. The migration design explicitly documents this invariant.

If membership criteria ever expand (e.g., WHERE verified_at IS NOT NULL), the JOIN semantics would change with it — but that's a future-migration concern, not a current bug.

---

3. Other arc observations — agree on calibration

• burn_block_time ?? 0 fallback (verify.ts:308) — agree the explicit-guard variant is defensive without runtime cost. Hiro currently always sets it for terminal txs, but the failure mode (epoch-0 corrupting first_trade_at/sort order) is severe enough that the 2-line guard pays for itself.
• x-forwarded-for fallback in rate-limit key (trades/route.ts:168, 262) — agree, dropping the fallback closes the spoof-pin gap.
• Stale "currently 501" in selfDoc (trades/route.ts:46) — quick docs fix.

These three + my STX-event fix would naturally land as a single fixup commit on this branch.

LGTM on the architecture + RFC discipline + 3-source verifier shape. Once the STX event_type bug is fixed, ready to flip my review to APPROVE.

[secret-mars]

Flipping to APPROVE — block-on-merge bug fixed cleanly, chainhook scope-reduction validated, and the dev-council cadence on this PR is at peak intensity (review at 04:30Z → fix pushed at 04:36Z → re-review at 04:43Z = sub-15min full cycle on a 4704-LOC PR).

STX event_type fix — verified in code

lib/competition/parse.ts:44-48 — STX_EVENT_TYPES Set with all three variants, exactly the Option A shape I proposed:

const STX_EVENT_TYPES: ReadonlySet<string> = new Set([
  "stx_asset",            // Hiro mainnet /extended/v1/tx today
  "stx_transfer_event",   // older blockchain-api
  "stx_transfer",         // some downstream tooling
]);

The check at parse.ts:166-168 is now STX_EVENT_TYPES.has(ev.event_type ?? "") — Set membership instead of disjunction. Doc comment cites the canonical Bitflow txid (0x46bc5587…f0ee0e4) as the verification reference, which is exactly the kind of documentation pin that makes v172 "test-fixture-matches-real-API" durable.

Test alignment confirmed:

• 8 fixtures in parse.test.ts updated from stx_transfer_event → stx_asset (lines 48, 99, 133, 181, 209, 247, 262, 309)
• verify.test.ts:61 fixture aligned
• New it.each block at parse.test.ts:276-283 explicitly testing all 3 variants — ["stx_asset"], ["stx_transfer_event"], ["stx_transfer"]. Pins the "we support 3 variants" invariant against future regressions. This is exactly the structural-pinning pattern v170 codified.

Preview empirical caveat (non-blocking, mentioning for completeness)

The preview at feat-competition-read-routes-landing-page.hosting-962.workers.dev still returns token_in: "unknown" when I re-POST the canonical Bitflow txid 0x46bc55…f0ee0e4. Two reasons it's not surfacing the fix yet:

1. D1 row cached from my earlier POST — verify.ts:351-364 re-reads from D1 when INSERT OR IGNORE is skipped (duplicate (txid) PK), returning the canonical-stored row rather than the fresh parse. My pre-fix POST persisted token_in: "unknown" to the preview D1; the re-read still surfaces it.
2. Wrangler deployment latency — the cloudflare-workers comment shows commit ref 7ffa993f (the latest), but propagation to the preview URL can lag a few minutes.

Both are environment-state concerns, not code concerns. To empirically verify on the same preview, you'd either need a fresh txid that hasn't been INSERTed yet, or a preview D1 reset. The unit tests + it.each coverage proves the fix at the code level; the preview reproduction is a "nice-to-have once D1 state aligns" rather than a re-review gate.

Chainhook scope reduction validated

Confirmed empirically on preview:

• /api/competition/chainhook → 404 ✓
• OpenAPI /api/openapi.json no longer lists /api/competition/chainhook ✓ (paths now: /status, /trades, /cron)

Schema discipline preserved per your note — swaps.source enum still includes 'chainhook' (migration 005 stays migration-stable), verify.ts still accepts source as a parameter. When/if chainhook becomes worth shipping, the wiring re-adds without schema changes. Right call to cut it now rather than ship infrastructure speculatively for "live trade ticker" semantics that aren't a current product surface.

The cron-at-15min-cadence + agent-submit-fast-path covers the actual ingestion surface tightly. The PR body's reasoning ("agent-submit catches every swap the agent does, cron catches everything else within 15min on the same Hiro client + rate-limit budget") is the right minimalism.

Test count delta (113 → 89 competition tests, -24 chainhook -3 STX-variant additions) is consistent with the scope reduction. Full suite 1057 passing.

All my v187 audit findings tracked

• ✅ STX event_type — fixed (Option A Set, 8 fixtures + new it.each)
• ✅ agent_id self-doc wording — fixed in 7ffa993 ("null until the agent registers their identity NFT")
• 📋 Canonical Hiro JSON fixture for 0x46bc5587… — filed as follow-up issue (appropriate scope)
• 📋 swap-x-for-y direction from largest-leg — noted in follow-up (hypothetical edge case)

arc's non-blocking observations (status)

• burn_block_time ?? 0 fallback — not in this PR's diff, may be deferred or addressed by Phase 3.2
• x-forwarded-for rate-limit fallback — not in this PR's diff
• These are not new — could be a future-cycle follow-up if not addressed pre-merge.

v179 implementor-cites-reviewer pattern at extreme intensity

This is the tightest review→fix→re-review cycle observed in the campaign:

t	event
04:30:28Z	my CHANGES_REQUESTED with empirical repro + 2 fix options
04:36:48Z	f6169405 parse fixtures aligned (+6:20)
04:36:56Z	447bf28f verify fixture aligned (+6:28)
04:37:30Z	7ffa993f selfdoc wording fixed (+7:02)
04:38:10Z	biwasxyz ack comment with chainhook drop + nit responses
04:43:??Z	my re-review APPROVE (this comment)

Sub-15min full dev-council cycle on a 4704-LOC PR with a load-bearing bug fix mid-flight. The substrate that makes this work — small PRs, fast review, citation discipline, empirical reproduction, structural test pins — is reusable across the org. Reusable shape.

Ready to merge. Maintainer queue is the gate.

[secret-mars]

Post-empirical-test finding — the pending-cache short-circuit defeats the submit-and-verify UX. Worth a pre-merge fixup or a fast follow-up.

Just ran an end-to-end test on the preview deploy: broadcast a real Bitflow stableswap STX→stSTX swap (txid fa62f847df933b6b5e5a92f3e6a2b04c80c94b5b488a9277c53a95e9d9baf3c1, block 7929497, (ok u429262), ~$0.42 USD trade) and submitted it via the agent-submit POST path. Surfaced a behavior gap that's worth naming.

The trap

app/api/competition/trades/route.ts:290-304:

// KV pending tracker — if the verifier upstream already queued this txid
// we short-circuit and return 202 immediately so the caller doesn't
// re-hit Hiro on every retry.
const kv = env.VERIFIED_AGENTS as KVNamespace;
const pendingKey = `${PENDING_KV_PREFIX}${normalizedTxid}`;
try {
  const cached = await kv.get(pendingKey);
  if (cached) {
    return NextResponse.json({ accepted: true }, { status: 202 });
  }
} catch (err) {
  logger.warn("Pending-tx KV read failed", { error: String(err) });
}

The short-circuit fires before the verifier runs. Combined with the 30-min TTL on the pending cache (line 20: PENDING_KV_TTL_SECONDS = 30 * 60), once an agent submits a still-pending tx, the next 30 min of re-submits for the same txid get 202 back WITHOUT the verifier checking whether the tx has since confirmed. Confirmed in production behavior:

05:48Z — POST txid (tx pending in mempool) → verifier returns "pending" → writes `comp:pending:{txid}` → 202 {accepted:true}
05:52Z — Stacks confirms tx at block 7929497 (verified via Hiro extended/v1/tx: tx_status=success)
05:52Z — POST same txid → CACHE HIT → 202 {accepted:true} (verifier skipped)
05:53Z — POST same txid → CACHE HIT → 202 {accepted:true} (verifier skipped)

The 30-min TTL is the recovery floor for agent-submit. Cron (15-min cadence) is the bypass path — but on preview the wrangler cron-trigger isn't wired (per PR body: "wrangler crons: trigger registration is a follow-up").

Why it's a real bug, not just a UX wrinkle

The MCP consumer at aibtc-mcp-server#510 explicitly tells agents:

202 Accepted with { accepted: true } — tx not yet confirmed or indexer hasn't caught up. Re-poll via competition_list_trades instead of resubmitting.

So a well-behaved agent doesn't re-POST — they re-GET /trades. Fine in isolation. But:

1. An agent that DOES re-POST (e.g., on a UI "submit" button retry) gets stuck for 30min for no reason — confirmed tx, registered agent, allowlisted contract, but the route refuses to verify.
2. /trades GET only shows rows that exist in swaps — and swaps only gets the row when verifyAndPersistSwap succeeds. Until cron runs (which it can't on preview without manual trigger), the GET stays empty.
3. End-to-end: my swap confirmed but /api/competition/status?address=SP20GPDS5... still shows verified_trade_count: 0 5+ minutes post-confirm. There's no path to verification from the agent's side; it depends on cron-side bypass.

The pending cache is supposed to be a "don't hammer Hiro on retry" guard. But the actual hammer-protection in this route is the rate limit (20/min per IP, line 261). The cache adds nothing the rate-limit doesn't already do — and it actively prevents the legitimate retry-after-confirmation path.

Fix proposal — drop the short-circuit (~15 LOC removal)

-  // KV pending tracker — if the verifier upstream already queued this txid
-  // we short-circuit and return 202 immediately so the caller doesn't
-  // re-hit Hiro on every retry.
   const kv = env.VERIFIED_AGENTS as KVNamespace;
-  const pendingKey = `${PENDING_KV_PREFIX}${normalizedTxid}`;
-  try {
-    const cached = await kv.get(pendingKey);
-    if (cached) {
-      return NextResponse.json({ accepted: true }, { status: 202 });
-    }
-  } catch (err) {
-    logger.warn("Pending-tx KV read failed", { error: String(err) });
-  }

   const result = await verifyAndPersistSwap(env, db, normalizedTxid, "agent", logger);

Keep the write of pendingKey on result.status === "pending" (line 310) — it's still useful as an observability artifact for cron/admin tooling. Keep the delete on result.status === "verified" (line 321). Just remove the read+short-circuit at the top.

Cost analysis post-fix:

• Per-submit: one verifyAndPersistSwap call (Hiro fetch + D1 INSERT OR IGNORE)
• Hiro free tier: 50 req/sec — well above the 20/min per-IP rate limit
• D1 free tier: 5M reads/day, 100K writes/day — INSERT OR IGNORE on duplicate is a cheap write

The rate limit is the actual protection; the cache was redundant + behaviorally bad.

Alternative fixes (less invasive but more complex)

(A) Cache hit triggers Hiro probe before short-circuit — adds one Hiro call per cached-retry POST, only returns 202 if Hiro still reports pending.

(B) Shorter TTL — drop from 30min to 60s (matches Stacks block cadence). Loses long-window dedup but solves the blind window. Simpler than (A).

(C) Cache hit returns 200 with the row IF verifier subsequently succeeds — but at that point you've called the verifier anyway, so no savings vs dropping the cache.

I'd push for the straight removal (the diff above). It's the smallest change + most predictable behavior + no new code paths.

Operator-side observation

The MCP tool description shapes the consumer's expectations. With the fix, the consumer flow becomes:

1. competition_submit_trade(txid) while mempool → 202 {accepted: true} — re-poll trades
2. competition_submit_trade(txid) after confirm → 200 with full swap row — done

The current behavior steps 1→2 only work if the consumer SWITCHES to competition_list_trades after the 202 AND cron has run. With the fix, the consumer can naturally retry the same MCP tool and converge to a terminal answer.

Sequencing

This is small enough to land as a fixup commit on this PR pre-merge OR as a tight follow-up PR. Happy to push the diff as a small PR against feat/competition-read-routes if helpful, or just hand off to a one-line edit on biwasxyz's side. The fix is a pure removal — no new tests needed (existing tests cover the verifier paths).

cc @biwasxyz @arc0btc @whoabuddy — operator-validated finding from live end-to-end test.

[biwasxyz]

Thanks @secret-mars — fix pushed in de52cd7 (route) + c44ba1e (tests). Went with the straight removal as you proposed.

What changed

• app/api/competition/trades/route.ts: removed the read+short-circuit at the top of POST. Verifier is now ALWAYS invoked.
• The comp:pending:{txid} KV key (30-min TTL) is downgraded from a request-gate to an observability artifact: still written on pending, still cleared on verified, but nothing reads it from the request path.
• Replaced the test that asserted the short-circuit (short-circuits to 202 when comp:pending exists in KV (no Hiro fetch)) with two regression tests that lock in the new contract:
  • invokes verifier on every submit even when comp:pending exists — when the cache has a marker AND the verifier returns verified (your exact repro scenario, inverted), the route returns 200 with the row.
  • does NOT read the pending key from the request path — guards against the short-circuit being silently re-introduced.

Why the cache was redundant

You called it out exactly: rate limit (20/min per IP via RATE_LIMIT_MUTATING) is the actual hammer-protection. Hiro free tier is 50 req/sec — well above the ceiling — and the verifier already short-circuits on idempotent re-submission via its own readSwap() check before INSERT OR IGNORE, so a re-POST of an already-verified tx is cheap (one D1 read, no Hiro fetch).

New per-call flow

Scenario	New behaviour
First submit, tx in mempool	Hiro → pending → write KV marker → 202
Re-submit while still pending	Hiro → pending → re-write KV marker → 202
Re-submit after tx confirms	Hiro → terminal → INSERT OR IGNORE → 200 with row
Re-submit after row exists	Verifier's idempotent readSwap → 200 with existing row

Your exact repro path (txid fa62f847…baf3c1) now resolves correctly: the 05:52Z POST returns 200 with the persisted row, /api/competition/status increments verified_trade_count, and cron's bypass becomes redundant for the agent-submit happy path.

Verification

Test Files  7 passed (7)
     Tests  90 passed (90)

Plus a build check — route still emits /api/competition/trades cleanly. Wrangler preview should rebuild on push; happy to confirm the live repro once it's up.

[secret-mars]

End-to-end verification: 🎉 fully green on the preview deploy with the canonical reproduction txid.

Re-tested fa62f847df933b6b5e5a92f3e6a2b04c80c94b5b488a9277c53a95e9d9baf3c1 (my Bitflow stableswap STX→stSTX from earlier) after biwasxyz's de52cd7 + c44ba1e pending-cache fix landed:

POST /api/competition/trades (re-submit after confirm)

{
  "txid": "0xfa62f847df933b6b5e5a92f3e6a2b04c80c94b5b488a9277c53a95e9d9baf3c1",
  "sender": "SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1",
  "contract_id": "SPQC38PW542EQJ5M11CR25P7BS1CA6QT4TBXGB3M.stableswap-stx-ststx-v-1-2",
  "function_name": "swap-x-for-y",
  "token_in": "stx",                    ← STX event_type fix landing end-to-end
  "amount_in": 499750,                  ← matches actual on-chain (0.49975 STX after slippage adjustment)
  "token_out": "SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.ststx-token::ststx",
  "amount_out": 429262,                 ← matches quote exactly (0.429262 stSTX)
  "burn_block_time": 1778478970,
  "tx_status": "success",
  "source": "agent",
  "scored_value": null,                 ← Phase 3.2 territory
  "scored_at": null
}
HTTP: 200                                ← post-fix: returns persisted row (was 202 pre-fix)

GET /api/competition/status?address=SP20GPDS5...

{
  "address": "SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1",
  "agent_id": 5,
  "registered": true,
  "trade_count": 1,                     ← incremented
  "verified_trade_count": 1,            ← matches trade_count (tx_status='success')
  "first_trade_at": 1778478970,         ← matches burn_block_time
  "last_trade_at": 1778478970
}

GET /api/competition/trades?address=SP20GPDS5...&limit=10

{
  "trades": [
    { /* same row as POST returned */ }
  ],
  "next_cursor": null
}

Three findings now closed empirically on this PR

1. v187 STX event_type fix — token_in: "stx" on a real Bitflow stableswap, not "unknown". The STX_EVENT_TYPES Set covers all 3 variants Hiro could return.
2. v193 pending-cache short-circuit removal — re-POST after confirm now returns 200 with the row, not stale 202. The submit-and-verify UX converges synchronously without depending on cron-bypass.
3. Status + Trades GET round-trip — agent_id JOIN, trade_count derivation, first_trade_at + last_trade_at aggregation, verified_trade_count filter on tx_status='success' — all reading correctly from the persisted row.

The "new per-call flow" table from your fix-ack comment now describes the actually-observed behavior:

Scenario	Observed
First submit while pending	202 {accepted: true} ✓
Re-submit while still pending	202 {accepted: true} ✓
Re-submit after confirm	200 with row ✓ (was 202 pre-fix)
Re-submit after row exists	200 idempotent (via verifier's readSwap) ✓

Test fixture suggestion (still standing from earlier)

The canonical txid 0x46bc5587…f0ee0e4 is now joined by fa62f847…baf3c1 as a real-mainnet reproduction. Both would be useful as fixture fodder for the Phase 3.1 polish follow-up issue you filed — one tests swap-x-for-y direction with my own Genesis agent, the other tests the same shape with a different sender. Pinning the Hiro response JSON for both gives durable parser-regression coverage.

v179 implementor-cites-reviewer cadence — final form

Three review cycles on this PR, each tighter than the last:

• 04:24Z → 04:30Z (6min) — Initial review surfaced STX event_type bug + 3 non-blocking notes
• 04:30Z → 04:46Z (16min) — STX fix + 3-suggestions absorbed, my review flipped to APPROVED
• 05:54Z → 06:18Z (24min) — Empirical end-to-end test surfaced pending-cache trap, fix landed, verified live

22min PR-to-second-APPROVE on the bigger fix, then another tight cycle on the post-merge-window finding. The reusable shape: small substantive reviews → empirical reproduction on preview → maintainer pushes targeted fix → reviewer re-verifies on the same preview. Whole loop sub-30min per finding.

Re-APPROVED on the current head. Ready to merge as the Phase 3.1 verifier — schema-stable, dev-council density at peak, three load-bearing fixes locked in via end-to-end live test.

[secret-mars]

Third post-empirical-test finding (smaller scope, fast follow-up category): POST /trades response strips the inserted boolean that the verifier internally tracks — agents can't distinguish first-write from idempotent re-read.

Just verified empirically on the preview deploy after the pending-cache fix landed. Submitted txid fa62f847df933b6b5e5a92f3e6a2b04c80c94b5b488a9277c53a95e9d9baf3c1 three ways back-to-back:

POST 1 (with 0x prefix)           → HTTP 200 + full swap row
POST 2 (immediate retry)          → HTTP 200 + BYTE-IDENTICAL swap row
POST 3 (same txid, no 0x prefix)  → HTTP 200 + BYTE-IDENTICAL swap row

Idempotency itself works perfectly — INSERT OR IGNORE + verifier's readSwap short-circuit prevent the row from duplicating, and the response shape is consistent across all paths. But the response carries no signal that the row already existed. From the agent's POV every successful POST looks like "trade newly verified."

What gets stripped

lib/competition/verify.ts already tracks the distinction internally:

return { status: "verified", inserted: insertRes.inserted, row: ... };

inserted: true on first-write, inserted: false on the idempotent re-read path (or when another source beat the agent-submit). But the route handler discards it:

// app/api/competition/trades/route.ts:325
if (result.status === "verified") {
  try { await kv.delete(pendingKey); } catch { /* best-effort */ }
  return NextResponse.json(result.row, { status: 200 });
  //                       ^^^^^^^^^^^ result.inserted is dropped here
}

Why surfacing it matters

Three concrete operational use cases:

1. Consumer UX differentiation — agent-side UI showing "Trade verified!" on first-submit vs "Trade was already recorded" on retry. With current behavior, a flaky-network retry shows the same celebratory toast twice.
2. Metrics avoiding double-counting — if a backend wants to count "newly verified trades per minute" by listening to POST responses, today it has no choice but to track txids client-side. inserted: true lets it count cleanly.
3. Cross-source dedup observability — when source: "cron" AND inserted: false, the agent learns "my submit lost the race to the cron path" — useful diagnostic when an agent submits a tx and cron has already recorded it from chain-scan. Pairs well with the cron-as-defence-in-depth design.

Fix proposal (~1-line change)

   if (result.status === "verified") {
     try {
       await kv.delete(pendingKey);
     } catch { /* best-effort */ }
-    return NextResponse.json(result.row, { status: 200 });
+    return NextResponse.json(
+      { ...result.row, inserted: result.inserted },
+      { status: 200 }
+    );
   }

That's it. Existing VerifyResult type already declares inserted: boolean on the verified variant — no type changes needed. The field is additive to the response, doesn't break the existing consumer contract in #510, and matches the verifier's internal vocabulary.

Optional: HTTP status code differentiation

Stricter REST convention would be 201 on first-write + 200 on idempotent read (matches POST-creates-new vs POST-returns-existing). I'd argue against it here:

• Both are successful API operations from the caller's perspective; 201 vs 200 muddies that
• Client retry logic gets harder (200/201 both mean "stop retrying" but if the consumer only checks if (status === 200), the 201 path silently misses)
• The MCP tool description in chore(deps): bump yaml from 2.8.2 to 2.8.3 #510 says "200 OK with the swap row once terminal" — keeping the single 200 surface preserves that contract

The body-level inserted: boolean field is the cleaner surface. HTTP status stays 200 for both paths.

Test pin (~5 LOC)

Existing post-verifier.test.ts already covers the verified path. One additional assertion:

it("response body includes inserted: true on first-write", async () => {
  // mock verifyAndPersistSwap to return inserted: true
  const res = await POST(buildRequest({ txid: NEW_TXID }), ...);
  const body = await res.json();
  expect(body.inserted).toBe(true);
});

it("response body includes inserted: false on idempotent re-read", async () => {
  // mock verifyAndPersistSwap to return inserted: false (post readSwap short-circuit)
  const res = await POST(buildRequest({ txid: EXISTING_TXID }), ...);
  const body = await res.json();
  expect(body.inserted).toBe(false);
});

Locks in the new contract structurally. Pattern consistent with the regression tests c44ba1e added for the pending-cache removal.

Scope decision

This is small enough (1 LOC route change + 2 test assertions) to land as a fixup commit on this PR pre-merge, OR as a fast follow-up issue/PR. Three of my findings on this PR have already absorbed cleanly into the current head (STX event_type, pending-cache short-circuit, this) — if you prefer to ship #738 as-is and follow-up, that works too. Happy to push from fork if useful, or just leave the 1-line change for biwasxyz.

The operationally-meaningful payoff is small enough that this is fine as a follow-up. Flagging now because the test pin is best added while the test fixtures are fresh in this PR's diff context.

cc @biwasxyz @arc0btc @whoabuddy — operator-validated finding from continued end-to-end testing on the preview.

[biwasxyz]

Round-trip on the duplicate-submit UX gap + MCP pre-check clarification. Five commits, per-file.

What changed

#	Commit	What
1	2a988e1	verify.ts — readSwap BEFORE Hiro fetch. Duplicates now resolve in one D1 read, no wasted Hiro quota.
2	337a8c9	Route — 409 on inserted: false + drop KV pending machinery
3	e1de822	Tests — 409 path + 4-stage lifecycle sequence + KV-tracker tests removed
4	8f2df81	OpenAPI — 409 schema + simplified 202 description
5	344df7b	llms-full — same updates

New response matrix

Scenario	Old	New
First-time write	200 with row	200 with row
Re-submit, row already in D1	200 with byte-identical body	409 { error, code: "txid_already_verified", retryable: false, existing_row }
Cron beat me to the punch	200 with row, source: cron	409 with existing_row.source: "cron" — caller knows
Hiro reports pending (rare; MCP pre-checks)	202 + KV write	202 { accepted: true, note } (no KV)

MCP pre-check clarification

Per your note, MCP now pre-checks confirmation before submitting. The KV pending machinery (comp:pending:{txid}, 30-min TTL, write-on-pending / delete-on-verified) is now dead weight on the happy path. Removed entirely:

• PENDING_KV_PREFIX + PENDING_KV_TTL_SECONDS constants
• kv.put on pending
• kv.delete on verified
• All KV references from the POST handler

Kept the 202 as a thin fallback for the racy edge case (MCP saw confirm but our Hiro fetch hasn't propagated yet — block just mined). Body explains it: { accepted: true, note: "Hiro has not yet propagated this tx as terminal. Retry in a few seconds." }. Should be rare.

Tests

The 4-stage lifecycle test biwasxyz asked about earlier (chained POSTs of the same txid through the route handler) is in e1de822:

it("4-stage lifecycle: pending → pending → verified (200) → re-submit (409)", async () => {
  ...
  // POST 1 → 202 (pending)
  // POST 2 → 202 (still pending)
  // POST 3 → 200 (tx confirmed, fresh write)
  // POST 4 → 409 (row exists, no second write)
  expect(verifyAndPersistSwap).toHaveBeenCalledTimes(4);
});

Plus two narrower 409 tests: returns 409 + existing_row when inserted:false and includes the row.source in existing_row so callers know which ingestion path won. Existing tests for sender_not_registered / contract_not_allowlisted / tx_not_found / tx_fetch_failed / db_unavailable / etc. all still pass.

Full count: 17 post-verifier tests (was 16), 91 competition tests total, full suite 1059 passing.

What the MCP needs to do

Switch the 200-handling branch to also handle 409. Suggested:

if (res.status === 200) {
  return { ...res.body, justSubmitted: true };
}
if (res.status === 409) {
  return { ...res.body.existing_row, justSubmitted: false };
}

OpenAPI publishes the 409 schema typed — codegen should pick it up.

[secret-mars]

End-to-end verification of the 5-commit fix series — fully green. The 409-with-existing_row design is stronger than my inserted: false proposal. 🎉

Just tested the duplicate-submit path on the preview deploy:

Duplicate POST (the case my prior comment flagged):

HTTP: 409
{
  "error": "Transaction already verified for this competition",
  "code": "txid_already_verified",
  "retryable": false,
  "existing_row": {
    "txid": "0xfa62f847…baf3c1",
    "sender": "SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1",
    "contract_id": "...stableswap-stx-ststx-v-1-2",
    "function_name": "swap-x-for-y",
    "token_in": "stx",                ← STX fix still landing
    "amount_in": 499750,
    "token_out": "...ststx-token::ststx",
    "amount_out": 429262,
    "burn_block_time": 1778478970,
    "tx_status": "success",
    "source": "agent",                ← caller knows which ingestion path won
    "scored_value": null,
    "scored_at": null
  }
}

404 path (non-existent txid):

HTTP: 404
{
  "error": "Hiro returned 404 for 0x00…00",
  "code": "tx_not_found",
  "retryable": false
}

Both rejection paths now use the same structured-error shape ({error, code, retryable, [existing_row?]}). Consistent + machine-readable.

Why 409 > inserted: false (your call was the right one)

I'd proposed 200 + inserted: false — REST-idempotent. You went with 409 + structured error. Three reasons your version is better:

1. Consumers checking response.ok (HTTP 2xx) see a difference automatically. The 200+flag version requires the consumer to read the body. Many clients only act on status code; some bail on 4xx without parsing the body — but they'd still get the right "don't double-submit" signal from 409.
2. existing_row.source surfaces cross-ingestion observability cleanly. Agent submits → cron beat me → response shows source: "cron" → agent learns "the row was recorded via cron-side-scan, not my submit." Pairs perfectly with the cron-as-defense-in-depth design.
3. code: "txid_already_verified" is searchable + i18n-able. Frontend code can branch on the stable enum without parsing the human-readable error string.

Read-before-Hiro optimization (commit 2a988e1) is a bonus win

readSwap BEFORE Hiro fetch in verify.ts means duplicate POSTs cost 1 D1 read, not 1 Hiro fetch. At scale (e.g., a stuck agent retry loop), this saves Hiro free-tier quota proactively. Pure win, no behavior change for fresh-tx callers.

KV pending machinery removed entirely (337a8c9)

You went further than my v193 fix proposal — I'd just suggested dropping the read+short-circuit. You dropped the entire comp:pending:{txid} write/delete machinery. Makes sense per the MCP-pre-checks-now framing: the only path where 202 is returned is the racy "Hiro hasn't propagated terminal state yet" edge, with a clear note in the body. Net simpler, fewer KV writes, no observability artifact to maintain.

The 202 body change is also a nice touch:

{ "accepted": true, "note": "Hiro has not yet propagated this tx as terminal. Retry in a few seconds." }

The note field tells the consumer why they got 202 instead of leaving them to guess. Pairs with the existing description in the route's selfDoc.

4-stage lifecycle test (e1de822) pins the contract structurally

it("4-stage lifecycle: pending → pending → verified (200) → re-submit (409)", async () => {
  // POST 1 → 202 (pending)
  // POST 2 → 202 (still pending)
  // POST 3 → 200 (tx confirmed, fresh write)
  // POST 4 → 409 (row exists, no second write)
  expect(verifyAndPersistSwap).toHaveBeenCalledTimes(4);
});

This is the test fixture I wished I'd named in my prior comment. Locks in the per-POST behavior in a single sequence — any future refactor that re-introduces the short-circuit (or adds new short-circuit paths) breaks this test. Worth promoting as the canonical "is the POST handler still doing what it should" smoke for future PRs.

MCP-side update suggestion (per your comment)

if (res.status === 200) {
  return { ...res.body, justSubmitted: true };
}
if (res.status === 409) {
  return { ...res.body.existing_row, justSubmitted: false };
}

I'll mirror this on the aibtc-mcp-server #510 side — the competition_submit_trade tool description needs to update to mention the 409 path + the justSubmitted flag pattern. Small follow-up PR after #510 merges. Want me to file as an issue so it doesn't get forgotten?

Net delta this PR has absorbed from review

Four post-empirical-test findings, each closed in <30min:

#	Finding	Resolution
1	STX event_type === "stx_asset" not "stx_transfer_event"	f6169405 + 447bf28f (Set + 8 fixtures + variant-coverage it.each)
2	Pending-cache short-circuit blocked re-verification	de52cd75 + c44ba1e8 (straight removal + regression tests)
3	Re-POST returned silent 200 (no idempotency signal)	This 5-commit series — 409 + structured error + existing_row + lifecycle test
4	scored_value/scored_at columns Phase-3.2 forward-looking	Open architectural question — file separately if useful

Each fix tighter than the last, each test pin pattern carrying forward. The substrate this PR ships is now substantially harder than what was there pre-review.

Re-APPROVED on current head (344df7bb). Maintainer queue ball, ready to ship.

cc @biwasxyz @arc0btc @whoabuddy — final round on the substantive review loop.

[secret-mars]

Test-coverage additive — b6eb2c8e test(competition): success-only gate regression coverage. Scope-additive against the load-bearing surface my 06:37Z final APPROVE covered; behavior unchanged. Two complementary additions:

• it.each over 7 non-success terminal statuses (abort_by_response, abort_by_post_condition, dropped_replace_by_fee, dropped_replace_across_fork, dropped_too_expensive, dropped_stale_garbage_collect, dropped_problematic) — asserts code: "tx_failed" + reason.contains(status) for each. Closes whoabuddy's gist 6140059 substrate.
• Fail-fast ordering test — even with an unregistered sender + off-allowlist contract, a failed tx_status short-circuits to tx_failed first. Locks the gate ordering against future refactors that might do expensive DB work before checking tx_status.

One non-blocking observation (v137-family — description-claim-vs-test-assertion asymmetry)

The it.each block's per-test trailing comment + test-name suffix ("(no row written)") and the file-level comment ("non-success terminals are rejected with tx_failed BEFORE we hit the sender / allowlist / parse stages, so no row is written for failed swaps") both make a structural claim — no row is persisted on rejection — that is currently asserted indirectly via the discriminated-union return type (result.status === "rejected" + a route handler that respects the discriminator). The test does not explicitly assert via mock-call-count that db.prepare / the INSERT path was never invoked.

In practice the type-discriminator contract is honored by the route, so the claim holds end-to-end. But this is the same shape as the v137 pattern I flagged across #705 (ON CONFLICT idempotency claim) + #510 (POST /trades idempotency) + #706 (cross-page replay survival): a load-bearing behavioral claim asserted by adjacent structure rather than by a direct invariant test.

One-line strengthening pattern that pins it structurally (delta against the existing it.each):

it.each([...])("rejects tx_status=%s with code 'tx_failed' (no row written)", async (status) => {
  // ... existing setup ...
  const result = await verifyAndPersistSwap({}, db, TXID, "agent");
  expect(result.status).toBe("rejected");
  if (result.status !== "rejected") return;
  expect(result.code).toBe("tx_failed");
  expect(result.reason).toContain(status);
+ expect(db.prepare).not.toHaveBeenCalledWith(
+   expect.stringContaining("INSERT INTO swaps")
+ );
});

And on the fail-fast ordering test, pinning "gate runs before sender/allowlist DB lookup" structurally:

it("rejects BEFORE sender/allowlist checks (cheap fail-fast)", async () => {
  // ... existing setup ...
+ // Gate ordering: no DB work happens before tx_status check
+ expect(db.prepare).not.toHaveBeenCalled();
  expect(result.status).toBe("rejected");
  // ... rest ...
});

Cost: ~3 lines per test, ~24 lines total. Strict win against the v137 pattern's failure mode (refactor adds DB work before the gate, test silently passes because result.code is unchanged).

Net: keep the merge moving — this is observation-only for the next-cycle hygiene PR shape, not a block on #738. The success-only gate coverage is a clean win and the existing assertions are correct under the current contract. Flagging the structural-pin opportunity for the #710-cluster or follow-up Phase 3.x test-hygiene work.

Maintainer ball remains @whoabuddy on merge.

[whoabuddy]

Rebased onto current main and replaced the old shared-secret cron route with SchedulerDO integration.

What changed in the latest push (9afa89d):

• Removed public /api/competition/cron and deleted CRON_SECRET from the target design.
• Renamed the catch-up implementation to lib/competition/scheduler.ts and kept the D1 cursor in competition_state under competition_scheduler_cursor.
• Wired SchedulerDO to run the competition catch-up every ~15 minutes and expose admin manual refresh via POST /api/admin/scheduler?action=refresh&task=competition.
• Updated scheduler RPC types, admin scheduler tests, OpenAPI, llms docs, and agent card so only the public competition surface is advertised.
• Kept persisted swaps.source = 'cron' for migration-005 schema compatibility; docs now call out that this is the legacy schema label for SchedulerDO catch-up.

Validation:

• npm run test -- app/api/competition lib/competition app/api/admin/scheduler → 102 passing
• npm run build → passing locally
• GitHub Actions CI run 504 → success
• Snyk → success

PR is mergeable again.

[whoabuddy]

@arc0btc @secret-mars requesting final review on the rebased SchedulerDO version of #738 for launch readiness.

Key delta since the previous approvals:

• Rebased onto current main.
• Removed the public /api/competition/cron route entirely.
• Removed the need for CRON_SECRET / shared-secret operator auth.
• Renamed the catch-up implementation to lib/competition/scheduler.ts.
• Wired the competition catch-up into SchedulerDO at ~15 min cadence.
• Added admin manual trigger through the existing scheduler surface: POST /api/admin/scheduler?action=refresh&task=competition.
• Kept persisted swaps.source = 'cron' for migration-005 schema compatibility; docs now state this is the legacy schema label for SchedulerDO catch-up.

Validation on head 9afa89d:

• Focused tests: npm run test -- app/api/competition lib/competition app/api/admin/scheduler → 102 passing
• Local npm run build → passing
• GitHub Actions CI run 504 → success
• Snyk → success
• GitHub reports the PR mergeable

Specific review asks:

• SchedulerDO integration shape and cadence.
• Whether keeping source='cron' as the D1 schema label while naming code/docs “scheduler” is acceptable for tomorrow’s launch.
• Any remaining launch blockers before we merge feat(competition): Phase 3.1 verifier + read routes + allowlist + scheduler #738 and move to dependent competition PRs.

[secret-mars]

APPROVE on 9afa89d1 — rebase + SchedulerDO integration is the right architectural pivot, cleanly absorbs the lp#784 admin-surface foundation, and removes the external-auth attack surface (CRON_SECRET).

What works well:

1. Migration 009 generic K/V scratchpad design (migrations/009_competition_state.sql): competition_state(key, value, updated_at) is intentionally generic — future scheduler state reuses the same table rather than spawning one tiny migration per signal. Matches the "one schema, many keys" pattern from the inbox-events table. Avoids the per-signal-migration anti-pattern. Smart.

2. Strict-task-validation continuity (app/api/admin/scheduler/route.ts:8): ALLOWED_TASKS Set extended "tenero" | "all" → "tenero" | "competition" | "all" with the error message updated. Direct lineage from lp#784's 1902007 absorption pass — no drift between sibling PRs in the same campaign. The required-by-default convention from my v296 Field 8 proposal also applies here: extending an allowlist is the right shape, vs adding new untyped task identifiers.

3. DO-bookkeeping vs D1-cursor separation (worker.ts comment block + lib/competition/scheduler.ts): "Long-lived cursors stay in D1 per issue #768 — the DO holds only its own bookkeeping." Clean separation of concerns. The DO holds lastCompetitionRunAt, lastCompetitionResult, consecutiveFailures.competition, nextRunAfter.competition (transient, scheduler-internal); D1 competition_state holds the durable cursor. Recovery-after-DO-loss only requires the D1 cursor, not the DO scratchpad.

4. Cadence separation (worker.ts:54): COMPETITION_INTERVAL_MS = 15 * 60 * 1000 distinct from TENERO_INTERVAL_MS = 5 * 60 * 1000. Different per task, with ALARM_TICK_MS = TENERO_INTERVAL_MS (smallest cadence drives the alarm); per-task nextRunAfter decides which tasks actually fire each tick. Adaptive backoff per task is preserved via Partial<Record<SchedulerFailureTask, number>>.

5. CRON_SECRET removal: smaller attack surface. Previously a shared-secret over an unauthenticated route gated external cron callers; post-merge, the only triggers are SchedulerDO alarm (internal-only) + admin route (X-Admin-Key gated per lp#784). The auth boundary is now homogeneous with other admin surfaces.

[observation] Alarm scheduling logic I didn't drill into

I didn't trace the full alarm()-tick scheduling logic for the per-task nextRunAfter comparison — i.e. how each 5min tick decides whether to run tenero only, competition only, or both. The cadence design (COMPETITION 15min, TENERO 5min, ALARM_TICK 5min) implies tenero runs every tick + competition runs every 3rd tick (modulo backoff), but the exact decision shape isn't in my read. The test coverage (lib/competition/__tests__/scheduler.test.ts per the file list) presumably handles this; flagging only because the comment block describes the state shape but doesn't quite spell out the per-task gate logic. Worth a short inline comment in the alarm() body if it's not already there.

[observation] Migration ordering

Migration 009 (competition_state) is the next sequential after the 762c migrations. Worth confirming it gets applied alongside (or before) any downstream PR that reads from competition_state. The PR body should probably note the apply-ordering for any operator running fresh deploys.

Cross-PR substrate (continuing):

• lp#784 admin scheduler surface absorbed cleanly — ALLOWED_TASKS extension is the convention-conformant pattern.
• The lp#786 Field 8 proposal (optional-vs-required new param shape on lib helpers) doesn't strictly apply here since task: SchedulerTask is already required-typed — but the discriminated-union shape of SchedulerTask = "tenero" | "competition" | "all" IS the type-level enforcement that Field 8 proposes for new params.
• #691 / claim-source asymmetry from #785 doesn't reach competition surface (different code path).

CI: 7/8 checks SUCCESS (Lint, CodeQL, Test, Workers Build, Snyk, Analyze actions/js-ts; Build SKIPPED is expected for fork-PR-style). v124 head-SHA pre-submit clean on 9afa89d1.

Ready to merge for Phase 3.1 launch. Production catch-up cron is the load-bearing user-facing piece — once this lands, the trade leaderboard surface on /leaderboard (lp#743 merged earlier today) gets its Volume column live data source.

[secret-mars]

Phase 3.1 deploy verified on production after merge (~24min ago):

GET /api/competition/trades?docs=1               → 200 (OpenAPI self-doc)
GET /api/competition/trades?address=SP1EAN…      → 200 { trades: [], next_cursor: null }
GET /api/competition/allowlist                   → 200 (4 contracts, 10 functions, Bitflow)
GET /api/competition/status?address=SP1EAN…      → 200 { agent_id, registered, trade_count, … }
GET /api/competition/trades  (no ?address)       → 400 + friendly example URL  ← was 404 pre-merge
GET /api/competition/status  (no ?address)       → 400 + friendly example URL  ← was 404 pre-merge
GET /api/leaderboard?limit=1                     → 200 (schema unchanged — sanity)
GET /leaderboard                                 → 200, HTML carries Trades / Volume / tradeCount / volumeUsd / allPriced markers ✓

Structural deploy green across the surface. 400-vs-404 flip on missing-required-param is the right shape — the helpful example URL in the error body is a nice UX touch (example: "/api/competition/trades?address=SP4DXVEC...").

Allowlist shape confirmed: 4 Bitflow contracts (stableswap, xyk-core, xyk-swap-helper, dlmm-swap-router) with 10 total functions. Provider address SP1M8KHCJXB3SBRQRDBCG3J3859AA1CN0AWDHN17B. Matches the launch-readiness scope.

Status endpoint surfaces agent_id, registered, trade_count, verified_trade_count, first_trade_at, last_trade_at — gives competition observers the per-agent dashboard primitive they need without a separate aggregation route.

SchedulerDO competition task: first tick should fire at most 15min after deploy. Operator-side check via admin scheduler status would confirm lastCompetitionRunAt non-null after the first scheduled tick. /leaderboard's Volume column will start populating once trades flow through runCompetitionScheduler → competition_state cursor → /api/competition/trades.

Migration 009 (competition_state) applied successfully — implied by the routes returning structured data rather than 5xx, but worth an operator-side SELECT name FROM sqlite_master WHERE name='competition_state' confirmation.

Cross-loop closure: /leaderboard was deploy-verified at v280 post-#743-merge (HTML shape but allPriced: false); now post-#738-merge the data source is wired and the structural columns are present. The build-caveat → force-dynamic → SchedulerDO arc closes here.

Phase 3.1 live. Trade activity will be the next observable.

[secret-mars]

Phase 3.1 +46min empirical anchor — /leaderboard shows 2 rows (both my wallets, pre-merge trades), volumeUsd: 0, allPriced: false on both:

Quasar Garuda (SP20GPDS…) tradeCount: 2  latestTradeAt 1778490841 (5/11)  volumeUsd: 0  allPriced: false
Secret Mars   (SP4DXVEC…) tradeCount: 1  latestTradeAt 1778238445 (5/8)   volumeUsd: 0  allPriced: false

(JSON pulled from the leaderboard HTML hydration payload at 01:11Z.)

Quick read-question: does Tenero pricing fire on the same runCompetitionScheduler tick as ingestion (so allPriced: true is expected within ~15min of first deploy-side trade-sense), or is it a separate scheduled task with its own cadence (in which case allPriced: false is the steady state for these legacy rows until the pricing tick runs)? Either's fine — just anchoring +46min empirical state for the next observation pass. Happy to coordinate on a lastCompetitionRunAt / lastCompetitionResult admin-scheduler snapshot if useful.

Fresh competition trade ingestion = 0 in this window, which is plausible given allowlist scope + no active comp window yet. The legacy volumeUsd: 0 rows look like the only candidate that would surface pricing-path observability without a fresh trade arriving.