Repository navigation
merge: land B02-B04, fix the pnpm workspace, and add Gate P4 adapters - #24
Merged
Merged
Conversation
… classifier Implements B02 for the Coder B lane as pure adapter logic with no network, no durable state, and no credential. Canonical request (B02.1, B02.2). One intent produces byte-stable calldata, a keccak256 payload fingerprint, a provider idempotency key, and a reference ID. Serialization writes an explicit field order rather than relying on JSON.stringify over an object literal, whose key order depends on construction order; two workers building the same obligation must fingerprint identically or the provider key stops collapsing duplicates. Addresses are lowercased so checksummed and non-checksummed spellings cannot fingerprint apart. A golden vector pins the exact bytes, because changing any of them changes every in-flight idempotency key. The 24-hour provider window is documented as supplemental: OneShot durable state remains the authority past it. Only the direct transfer path is built. B01.3 recorded the Arc Memo forwarded call NOT_SUPPORTED, so no memo calldata builder exists to be reached by mistake. Policy fixture (B02.3). Expresses the required Privy policy as rules over the documented condition fields, terminated by an unconditional default deny. assessPolicySoundness catches the two ways such a policy silently stops protecting anything: losing its terminal deny, or dropping a constrained dimension. A keccak256 digest lets readiness detect drift, since the policy lives in Privy configuration outside this repository and can be edited without a commit. The digest ignores recipient ordering so an operator relisting the same addresses does not read as drift. Receipt verification (B02.4). Confirmation requires a final receipt and exactly one Transfer matching sender, recipient, and amount, emitted by the configured token. status: 1 alone is not confirmation, and tests cover the cases that would otherwise pass: no logs, a redirected recipient, an amount off by one atomic unit, a Transfer from an impostor contract, and two matching transfers, which would mean more value moved than was authorized. Outcome classifier (B02.5). Doubt is structural. DEFINITELY_NOT_SUBMITTED is granted only for narrow pre-flight proofs where nothing was broadcast; every ambiguous signal and every unrecognized response shape falls through to POSSIBLY_SUBMITTED, including a response kind from a future provider version. A receipt that fails to prove settlement is POSSIBLY_SUBMITTED, never DEFINITELY_NOT_SUBMITTED: absence of proof is not proof of absence.
Implements B03 offline closure for the Coder B lane. Live execution is not performed and is recorded as LIVE_NOT_RUN in docs/settlement/LIVE_EVIDENCE.md, which the packet explicitly permits. Harness (B03.2). Runs the whole adapter workflow against simulated providers with no network and no credential. Request identity is persisted before the provider call rather than after: the window between that write and the provider returning is exactly where a crash produces UNKNOWN, and the record is what makes it recoverable. Negative suite (B03.3). Every denial family asserts against a broadcast counter, not just the returned enum, because a denial that returned the right value while still broadcasting has not denied anything. Duplicate delivery, ten sequential retries, and ten parallel workers sharing durable state each produce exactly one broadcast. An ambiguous outcome does not grant a fresh submission right, so the dangerous retry after a possible payment cannot happen. Ambiguous scenarios deliberately increment the broadcast counter: counting them as non-broadcasts would understate exposure. Fixture capture (B03.5). Allowlists the fields needed to reproduce a decision so a new provider field cannot silently start appearing in committed fixtures, then redacts what survives and asserts no secrets at capture time. Fixes a redaction bug the round-trip test caught. The rule treating any 32-byte hex value as secret-shaped also matched transaction hashes, block hashes, event topics, ABI-encoded words, and payload fingerprints, so sanitizing a receipt destroyed the very evidence it was captured to preserve and the fixture no longer confirmed the settlement. Shape alone cannot separate a private key from a keccak hash, so the rule now applies only outside named hash-bearing fields; key material is still caught by field name regardless of shape, and a credential hidden inside a hash-bearing field is still redacted. Also adds the human-run provider setup guide, which keeps account creation, secret handling, policy mutation, and funding with a person and leaves only the read-only readiness check automatable.
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.
…, ports Implements B04 for the Coder B lane: the production adapter surface, plus the conservative classification behind it. Failure taxonomy (B04.1). Turns on one question: did the request reach the network? DNS failure, connection refusal, and a failed TLS handshake all happen before any application data is sent, so nothing can have been broadcast and a retry is safe. A reset, a timeout, a truncated response, an interrupted TLS connection, 429, and 5xx may all follow a delivered request, so none of them permit a retry. HTTP 429 stays ambiguous rather than counting as a rejection, because a rate limiter may reject before or after queuing the work. Any error code this build has never seen classifies post-send: an unknown failure cannot be proof that nothing happened. Evidence lookup (B04.2, B04.3). NOT_FOUND is the dangerous result and gets the strictest treatment. Absence can mean never broadcast, or invisible to this node, or replaced, so permitsResubmission returns false for every observation and no code path converts an absent result into a settlement right. Evidence is bound to the exact request before it is interpreted, so a receipt from another chain, another hash, or another wallet cannot resolve this intent. A receipt that exists for our hash but does not prove our settlement is contradictory and stays unbound rather than being resolved by guess. Hashless discovery is deliberately absent: that is Coder C's Subgraph MCP path, and a second, weaker way to decide a payment happened must not grow here. Drift hardening (B04.4). The settlement boundary depends on a Privy policy, a wallet, a chain, a token, and a cap that all live outside this repository and can change with no commit and no review. detectDrift compares observed identity against a reviewed baseline and fails closed on any difference, including a lowered cap: judging whether a change is benign is not this module's job. assertNoDrift throws rather than returning a value a caller could ignore. Webhooks stay disabled, since signature verification is unproven and polling is already complete. Production entry point (B04.5). Exports only the frozen ports and coarse sanitized error codes, so provider error text carrying bodies and headers never crosses the seam. A test asserts no import reaches an A- or C-owned package rather than trusting review to catch it. The compatibility manifest states what the host must supply, what the adapter does not do, and its live gaps, so fixtures cannot masquerade as live evidence.
The review noted that PersistedIdentity declared a nonce nobody reads, which implies a wrong-nonce defence that does not exist: binding is done on the transaction hash alone. Removed, with a comment recording why, rather than leaving a field that overstates what the type checks.
Brings B02, B03, and B04 onto a branch based on current develop, and wires the three B-lane packages into the workspace properly. Only B01 ever reached develop. The stacked pull requests merged into each other rather than into develop: #11 targeted develop, but #12 targeted the B01 branch, #14 the B02 branch, and #17 the B03 branch. So develop had the B01 files and none of the settlement request, receipt verification, harness, evidence lookup, failure taxonomy, drift hardening, or frozen ports. It also still carried the redaction bug that destroyed transaction hashes, since that fix landed in B03. The B01 merge also broke develop. Its package-local npm lockfiles are incompatible with the pnpm workspace that A02 introduced, and pnpm install --frozen-lockfile has failed on every develop commit since. develop currently keeps CI green by quarantining the three packages from the workspace, from eslint, and from vitest, so none of their code was linted, typechecked, or tested. This removes the quarantine rather than living inside it: - Deletes the three package-local package-lock.json, eslint.config.js, and tsconfig.build.json files. The root workspace supplies the toolchain. - Rewrites the manifests to match the conventions A established: no package devDependencies, workspace:* for cross-package references, and the same build, clean, lint, test, and typecheck scripts. - Adds the packages to the root tsconfig project references and removes their exclusions from pnpm-workspace.yaml, eslint.config.mjs, and vitest.config.ts. - Regenerates pnpm-lock.yaml so --frozen-lockfile passes again. Two code changes were required by the shared toolchain. permitsResubmission dropped its unused parameter, which the root eslint config rejects and which was misleading anyway: no input can change the answer. Tests that resolved paths relative to the working directory now anchor to their own file, because the suite runs both package-locally and from the workspace root. The packages compile under the root TypeScript 6.0.3 and lint under ESLint 10.10.0 without further change. Verified: pnpm install --frozen-lockfile, lint, typecheck, build, format:check, and 439 tests across 31 files, of which 345 are B-lane tests that CI has never executed until now.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
oneshot | 33d98b5 | Sep 07 2026, 08:27 PM |
The settlement-packages job ran npm ci and npm run check in each of the three B-lane package directories, using their package-local lockfiles. It was the compensating control that kept those packages tested while they were quarantined from the pnpm workspace. They are now workspace members covered by the root lint, typecheck, build, and vitest runs, and their npm lockfiles are gone, so the job can no longer resolve its cache paths or run npm ci. Keeping it would mean maintaining a second, divergent toolchain for the same code. Their 345 tests now run inside the root suite instead, alongside the rest of the workspace.
selezenart
changed the title
6 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands B02, B03, and B04 on
develop, fixes the CI breakage my B01 mergecaused, and adds the Gate P4 composition adapters so lane B is ready for P4.
Two problems, one fix.
Only B01 ever reached
develop. The stacked PRs merged into each otherrather than into
develop: #11 targeteddevelop, but #12 targeted the B01branch, #14 the B02 branch, and #17 the B03 branch. So
develophas the B01files and none of the canonical request, receipt verification, outcome
classifier, harness, restart durability, fixture capture, evidence lookup,
failure taxonomy, drift hardening, or frozen ports. It also still carries the
redaction bug that destroys transaction hashes, because that fix landed in B03.
The B01 merge broke
develop. Its package-local npm lockfiles areincompatible with the pnpm workspace A02 introduced:
Stack lintpassed on A02 (64d0a6f) and has failed on every commit since.developcurrently stays green only by quarantining all three B packages fromthe workspace, from ESLint, and from vitest — so none of that code is linted,
typechecked, or tested today.
This removes the quarantine rather than living inside it.
Scope and acceptance criteria
The change is limited to the stated milestone or issue.
Acceptance criteria are listed and satisfied.
No unrelated cleanup is included.
Deletes the three package-local
package-lock.json,eslint.config.js, andtsconfig.build.jsonfiles; the root workspace supplies the toolchain.Rewrites the manifests to A's conventions: no package devDependencies,
workspace:*cross-references, matching scripts.Adds the packages to root
tsconfig.jsonreferences and removes theirexclusions from
pnpm-workspace.yaml,eslint.config.mjs, andvitest.config.ts.Regenerates
pnpm-lock.yaml.Removes the now-redundant
settlement-packagesCI job, whosenpm ciperpackage can no longer run and whose coverage the root suite now provides.
Gate P4 readiness
docs/GATE_P4_CHECKLIST.mdreplaces the simulator ports with lane-Bimplementations, but lane B exported only interfaces and pure logic — there was
nothing concrete to inject. Added:
ArcSettlementAdapterandPrivyAuthorizationAdapterin@oneshot/privy-adapter, conforming to the canonical result shapes inpackages/contracts/src/ports.ts, declaringcontractVersion = '1.0.0'andthe enabled network.
src/p4-conformance.ts, a compile-time guard that mirrors the worker's portinterfaces and statically asserts assignability. Verified to bite: renaming
submitmakestscreportTS2344 ... does not satisfy the constraint.It mirrors rather than imports, because importing
apps/workerwould breakthe lane rule.
docs/settlement/GATE_P4_LANE_B_READINESS.mdwith the injection recipe andthe required
WalletProvidersurface.Also corrects the lane boundary test, which wrongly forbade
@oneshot/contracts.That is the sanctioned cross-lane seam per
milestones/CONTRACTS.md, not anowned implementation. Implementation packages stay forbidden.
Naming disagreement for you and Coder A to settle
The P4 checklist reserves
@oneshot/adapter-arcand@oneshot/adapter-privy.The merged packages are
@oneshot/arc-adapterand@oneshot/privy-adapter,already referenced by the workspace, root tsconfig, fixtures, and docs. The
classes have the names the checklist expects; the packages do not. This needs a
decision rather than being discovered during composition.
Product and security invariants
Invariant notes:
No durable product state or tenant boundary changes here. The settlement
invariants are unchanged from the reviewed milestone branches; this PR carries
them onto
developand makes CI actually execute them. Notably it restores theredaction fix, without which committed fixtures lose the transaction hashes and
Transfer topics that prove a settlement.
Two code changes were forced by the shared toolchain:
permitsResubmissiondropped its unused parameter. The root ESLint configrejects it, and it was misleading regardless: no input can change the answer.
own file, because the suite runs both package-locally and from the root.
Validation
368 of those 462 tests are B-lane tests that CI has never executed until now.
They compile under the root TypeScript 6.0.3 and lint under ESLint 10.10.0 with
no further change.
Independent review evidence
Gate A — exact candidate tree before push
1228c2f02b947cbffbd2bac8205c0bb77f736fb0(commit72f643d)free-pi-cli0.2.19glm-5.3-flashINTEGRATION_PASS)The reviewer was asked specifically whether the workspace integration silently
drops coverage the deleted
settlement-packagesjob provided, whether theadapters match what
composeWorkerexpects, whether the conformance mirror canpass vacuously, whether any error path that could have broadcast returns
DEFINITELY_NOT_SUBMITTED, and whether allowing@oneshot/contractsisjustified. No blocking findings.
Non-blocking finding, fixed in
93fe9f9after the reviewed tree:transferLogIndexreached aCONFIRMEDresult straight from provider data withno bounds check. Now validated at the boundary, failing closed to
POSSIBLY_SUBMITTED. That commit is not covered by the verdict above; it addsone guard and five tests.
B02, B03, and B04 were each also reviewed on their own trees before their PRs:
B02
PASS, B03FAILthenRECHECK_PASSafter both blocking findings werefixed, B04
PASS.Evidence caveat: the reviewer's literal
VERDICT:line was lost to terminalviewport truncation. The captured evidence is
REVIEWED_TARGET: milestone/b-lane-integration @ 72f643d, an empty blocking-findings section, andthe terminal
INTEGRATION_PASStoken.Risk and rollback
CI. That is the intent.
now built with 6.0.3 and 10.10.0. Full suite passes, but they have less
mileage on this toolchain.
NOT VERIFIEDuntildocs/settlement/LIVE_EVIDENCE.mdrecords a live run.Rollback: revert this merge.
developreturns to B01-only with the quarantineand the failing lockfile check.
Human merge