Skip to content

Commit b6fcd25

Browse files
committed
fix(b03): address both blocking review findings
An independent FreePi review of tree 5aacf17 returned VERDICT: FAIL with two blocking findings. Both were correct. 1. The setup guide told a human to run `npm run check` in packages/arc-adapter and called it "the readiness probe". It is not. That command runs lint, typecheck, and unit tests against stubbed endpoints; it proves the probe's logic and says nothing about whether a particular operator's endpoint, chain, and token are correct. A human following the guide would have believed their setup was verified when nothing had contacted it. Adds a real read-only probe. createViemProbe implements the RpcProbe interface over viem, and `npm run probe` loads configuration from the environment, asks the configured endpoint which chain it is actually on, checks the USDC address holds bytecode, and prints a per-check report. It cannot sign, send, or mutate, and it prints no credential. Section 7 now points at it and explains why `npm run check` is not a substitute. 2. AttemptLog was in-memory, so B03.2's requirement to persist request identity in ignored test runtime state before submission was not met, and the at-most-once claims were an artifact of a shared process rather than a demonstration of durability. Restart is a boundary the failure-injection skill requires be exercised. Adds FileAttemptStore, writing through to JSON under an ignored tmp/ directory, and moves the store behind an AttemptStore interface. Writes are immediate: buffering would reopen the crash window the store exists to close. New restart tests build a fresh store from the same file, as a restarted process would, and prove a restarted worker is refused a second submission right, including after an outcome that was never learned, and that ten restarted workers still broadcast exactly once.
1 parent e6a15ee commit b6fcd25

11 files changed

Lines changed: 399 additions & 52 deletions

File tree

‎docs/settlement/LIVE_EVIDENCE.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ providers with a deterministic broadcast counter:
3030
- Every policy denial family produces **zero** external broadcasts.
3131
- Duplicate delivery, ten sequential retries, and ten parallel workers sharing
3232
durable state each produce **exactly one** broadcast.
33+
- Process restart is exercised against file-backed durable state: an attempt is
34+
written to disk before the provider is called, and a restarted worker reading
35+
that file is refused a second submission right, including after an outcome
36+
that was never learned.
3337
- An ambiguous outcome does not grant a fresh submission right, so the
3438
dangerous retry after a possible payment cannot happen.
3539
- Every ambiguous or unrecognized provider response classifies as

‎docs/settlement/PROVIDER_SETUP.md‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,30 @@ They are both called USDC. Read balances carefully.
7878
## 7. Verification (read-only, safe to automate)
7979

8080
Set the variables from `packages/arc-adapter/.env.example` in your shell or
81-
secret store, then run the readiness probe. It performs no mutation and prints
82-
no credential:
81+
secret store, then run the readiness probe:
8382

8483
```bash
85-
cd packages/arc-adapter
86-
npm run check
84+
cd packages/arc-adapter && npm run probe
8785
```
8886

89-
A `MISMATCH` result means a value is wrong and a human must fix it. It must
90-
never be retried into working. An `UNAVAILABLE` result means the endpoint could
91-
not be reached and may resolve on its own.
87+
This contacts the endpoint you configured, asks it which chain it is actually
88+
on, and checks that the configured USDC address holds contract bytecode. It is
89+
read-only: it cannot sign, send, or mutate anything, and it prints no
90+
credential. Exit code 0 means ready, 1 means not ready.
91+
92+
Read the result carefully, because the two failure modes need opposite
93+
responses:
94+
95+
- **`MISMATCH`** — the endpoint answered and the answer was wrong. Your
96+
configuration points somewhere it should not. A human must fix it. Do not
97+
retry; it will not resolve on its own.
98+
- **`UNAVAILABLE`** — the endpoint could not be reached. Your configuration may
99+
be perfectly correct. Retrying later is reasonable.
100+
101+
Note that `npm run check` is a different thing: it runs lint, typecheck, and
102+
the unit tests against stubbed endpoints. It proves the probe's logic is
103+
correct and tells you nothing about whether *your* setup is correct. Only
104+
`npm run probe` does that.
92105

93106
## 8. Storing the values
94107

‎packages/arc-adapter/bin/probe.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#!/usr/bin/env node
2+
import { runProbe } from '../dist/probe-cli.js';
3+
4+
const code = await runProbe(process.env);
5+
process.exit(code);

‎packages/arc-adapter/package.json‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,17 @@
1717
}
1818
},
1919
"files": [
20-
"dist"
20+
"dist",
21+
"bin"
2122
],
2223
"scripts": {
2324
"build": "tsc --project tsconfig.build.json",
2425
"typecheck": "tsc --noEmit",
2526
"lint": "eslint src test",
2627
"test": "vitest run",
2728
"test:watch": "vitest",
28-
"check": "npm run lint && npm run typecheck && npm run test && npm run build"
29+
"check": "npm run lint && npm run typecheck && npm run test && npm run build",
30+
"probe": "npm run build && node bin/probe.js"
2931
},
3032
"dependencies": {
3133
"viem": "2.56.3"
@@ -38,4 +40,4 @@
3840
"typescript-eslint": "8.69.0",
3941
"vitest": "5.0.0"
4042
}
41-
}
43+
}

‎packages/arc-adapter/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,5 @@ export * from './config.js';
55
export * from './readiness.js';
66
export * from './receipt.js';
77
export * from './outcome.js';
8+
export * from './viem-probe.js';
9+
export * from './probe-cli.js';
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* Read-only readiness command (B03.1 §7).
3+
*
4+
* Loads configuration from the environment, probes the configured Arc
5+
* endpoint, and prints a per-check report. It performs no mutation, signs
6+
* nothing, and prints no credential.
7+
*
8+
* This exists because `npm run check` runs unit tests against stubs. Those
9+
* prove the probe's logic; they say nothing about whether a particular
10+
* operator's endpoint, chain, and token are correct. Only this command does.
11+
*
12+
* Exit codes: 0 ready, 1 not ready.
13+
*/
14+
15+
import { loadSettlementConfig } from './config.js';
16+
import { probeReadiness } from './readiness.js';
17+
import { createViemProbe } from './viem-probe.js';
18+
19+
const SYMBOL: Readonly<Record<string, string>> = {
20+
PASS: 'PASS ',
21+
MISMATCH: 'MISMATCH',
22+
UNAVAILABLE: 'UNAVAIL',
23+
SKIPPED: 'SKIP ',
24+
};
25+
26+
export async function runProbe(env: NodeJS.ProcessEnv): Promise<number> {
27+
let config;
28+
try {
29+
config = loadSettlementConfig(env);
30+
} catch (error) {
31+
// Configuration errors name the variable, never its value.
32+
console.error(`Configuration error: ${(error as Error).message}`);
33+
return 1;
34+
}
35+
36+
console.log(`profile : ${config.profile.id} (chain ${config.profile.chainId})`);
37+
console.log(`token : ${config.profile.tokenContract}`);
38+
console.log(`settle : ${config.profile.tokenDecimals} decimals`);
39+
console.log(`gas : ${config.profile.nativeDecimals} decimals`);
40+
console.log('');
41+
42+
const report = await probeReadiness(config, createViemProbe(config));
43+
44+
for (const check of report.checks) {
45+
console.log(`${SYMBOL[check.status] ?? check.status} ${check.name}: ${check.detail}`);
46+
}
47+
48+
console.log('');
49+
if (report.ready) {
50+
console.log('READY');
51+
return 0;
52+
}
53+
54+
if (report.hasMismatch) {
55+
// The distinction that matters to whoever is reading this output.
56+
console.log('NOT READY: identity mismatch. A human must fix the configuration.');
57+
console.log('Do not retry; a mismatch will not resolve on its own.');
58+
} else {
59+
console.log('NOT READY: endpoint unavailable. Configuration may be correct.');
60+
console.log('Retrying later is reasonable.');
61+
}
62+
return 1;
63+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Live RPC probe backed by viem.
3+
*
4+
* The offline `RpcProbe` stubs prove the probe's logic. This implementation is
5+
* what actually contacts a configured endpoint, and it is the only thing that
6+
* can tell an operator whether THEIR setup is correct.
7+
*
8+
* Read-only by construction: it exposes `eth_chainId` and `eth_getCode` and
9+
* nothing that can sign, send, or mutate.
10+
*/
11+
12+
import { createPublicClient, http } from 'viem';
13+
import type { SettlementConfig } from './config.js';
14+
import type { RpcProbe } from './readiness.js';
15+
16+
/**
17+
* Build a probe against the configured endpoint.
18+
*
19+
* The chain is declared from the profile, but `checkChainId` still asks the
20+
* endpoint what chain it is actually on. Trusting the declared value would
21+
* defeat the check entirely.
22+
*/
23+
export function createViemProbe(config: SettlementConfig): RpcProbe {
24+
const client = createPublicClient({
25+
transport: http(config.rpcUrl, { timeout: config.rpcTimeoutMs }),
26+
});
27+
28+
return {
29+
async getChainId(): Promise<number> {
30+
return await client.getChainId();
31+
},
32+
33+
async getCode(address: `0x${string}`): Promise<string | null> {
34+
const code = await client.getCode({ address });
35+
return code ?? null;
36+
},
37+
};
38+
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/**
2+
* Durable attempt store for the harness (B03.2).
3+
*
4+
* B03.2 requires request identity to be persisted in ignored test runtime
5+
* state before submission. An in-memory map cannot satisfy that: it proves
6+
* concurrency inside one process and disappears on restart, which is exactly
7+
* the boundary `.agents/skills/oneshot-failure-injection/SKILL.md` requires be
8+
* exercised ("restart services between durable transitions and external
9+
* responses").
10+
*
11+
* This writes to a JSON file under an ignored `tmp/` directory. It is a stand-in
12+
* for the PostgreSQL state Coder A owns, not a production store, and it exists
13+
* so restart recovery can actually be tested rather than assumed.
14+
*/
15+
16+
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
17+
import { dirname } from 'node:path';
18+
19+
export interface PersistedAttempt {
20+
readonly businessIntentId: string;
21+
readonly payloadFingerprint: string;
22+
readonly idempotencyKey: string;
23+
readonly referenceId: string;
24+
/** Set before the external boundary is crossed, never after. */
25+
submissionAttempted: boolean;
26+
}
27+
28+
export interface AttemptStore {
29+
recordOrGet(attempt: PersistedAttempt): { entry: PersistedAttempt; isNew: boolean };
30+
get(businessIntentId: string): PersistedAttempt | undefined;
31+
readonly size: number;
32+
}
33+
34+
/** In-memory store. Fine for single-process tests; cannot survive a restart. */
35+
export class MemoryAttemptStore implements AttemptStore {
36+
protected readonly entries = new Map<string, PersistedAttempt>();
37+
38+
recordOrGet(attempt: PersistedAttempt): { entry: PersistedAttempt; isNew: boolean } {
39+
const existing = this.entries.get(attempt.businessIntentId);
40+
if (existing) return { entry: existing, isNew: false };
41+
this.entries.set(attempt.businessIntentId, attempt);
42+
return { entry: attempt, isNew: true };
43+
}
44+
45+
get(businessIntentId: string): PersistedAttempt | undefined {
46+
return this.entries.get(businessIntentId);
47+
}
48+
49+
get size(): number {
50+
return this.entries.size;
51+
}
52+
}
53+
54+
/**
55+
* File-backed store.
56+
*
57+
* Every mutation is written through immediately. Buffering writes would
58+
* reintroduce the crash window the store exists to close: an attempt that was
59+
* recorded but not yet flushed is, after a kill, indistinguishable from one
60+
* that never happened.
61+
*/
62+
export class FileAttemptStore implements AttemptStore {
63+
constructor(private readonly filePath: string) {
64+
mkdirSync(dirname(filePath), { recursive: true });
65+
}
66+
67+
private read(): Record<string, PersistedAttempt> {
68+
try {
69+
const raw = readFileSync(this.filePath, 'utf8');
70+
return JSON.parse(raw) as Record<string, PersistedAttempt>;
71+
} catch {
72+
// A missing or unreadable file means no attempts recorded yet. It must
73+
// never be treated as "no attempt exists" for an intent we have not
74+
// checked; callers only reach this through recordOrGet, which writes.
75+
return {};
76+
}
77+
}
78+
79+
private write(state: Record<string, PersistedAttempt>): void {
80+
writeFileSync(this.filePath, JSON.stringify(state, null, 2), 'utf8');
81+
}
82+
83+
recordOrGet(attempt: PersistedAttempt): { entry: PersistedAttempt; isNew: boolean } {
84+
const state = this.read();
85+
const existing = state[attempt.businessIntentId];
86+
if (existing) return { entry: existing, isNew: false };
87+
88+
state[attempt.businessIntentId] = attempt;
89+
this.write(state);
90+
return { entry: attempt, isNew: true };
91+
}
92+
93+
/** Persist a mutation made to an entry the caller already holds. */
94+
update(attempt: PersistedAttempt): void {
95+
const state = this.read();
96+
state[attempt.businessIntentId] = attempt;
97+
this.write(state);
98+
}
99+
100+
get(businessIntentId: string): PersistedAttempt | undefined {
101+
return this.read()[businessIntentId];
102+
}
103+
104+
get size(): number {
105+
return Object.keys(this.read()).length;
106+
}
107+
}

0 commit comments

Comments
 (0)