feat(ats): freeze the Spec 1 broker connector and approval contracts - #160
Conversation
Gate 1.0 of the trading integrations finale: the cross-repository shapes for local broker connectors, account onboarding and the approval bridge. No connector write path is enabled — src/core/ats_contracts/ performs no I/O, holds no credential and places no order, and a test asserts that by scanning the modules for node:fs/node:net/node:http/node:child_process/fetch/process.env. Eight schemas frozen, each a CLOSED document (an unknown key is a refusal): execution-state, connector-capability, account-binding, delegated-trading-grant, equity-order-intent, order-review-receipt, operator-approval, execution-receipt. The invariants are enforced by the shapes rather than described in prose: - A capability snapshot pins grants_execution_authority:false. Implementation support is not permission. - Effective mode can never exceed requested mode on the ladder offline<observe<review_only<paper<approve<auto; the halt states may follow any request, and a differing effective mode must carry a reason. - An empty symbol allowlist permits nothing. There is no wildcard token, so an allowlist cannot degrade into one on a half-failed load. - confirmation is pinned to per_order; standing or session approval is not expressible even though the provider offers it. - Raw account numbers cannot leave the connector core: opaque refs require a namespace prefix, a masked label with 5+ consecutive digits is refused, and redactBindingForExport() is the only exported projection. - Preview, approval and commit must name the same adapter, endpoint schema digest, execution environment and account binding generation. verifyApprovalChain() returns a terminal verdict and there is deliberately no re-preview or reroute path to reach for. - Fill facts are only expressible when broker-confirmed, and a receipt cannot fill more than it ordered. - An ambiguous commit is first-class and requires a reconciliation state, so a commit that may have started is never retried. Canonicalization reuses the existing device-runtime encoder rather than adding a second one. The specs say RFC 8785; what is implemented is a stricter subset that refuses non-integer and non-finite numbers, and docs/CONTRACTS.md states that difference plainly rather than overclaiming. Every monetary field is an integer count of minor units and every quantity a whole share, so a preview and a commit cannot disagree about a number that looked equal across languages. test/fixtures/ats_contracts_golden.json carries one validated example per schema with its canonical digest, following the bridge_conformance.json drift-detector pattern; the ATSv2 Python suite mirrors it so a canonicalization divergence fails a test instead of a trade.
Addresses the review on #160. Four blockers, none cosmetic. 1. THE APPROVAL CHAIN WAS NOT FULLY BOUND. verifyApprovalChain() trusted an intentDigest supplied by its caller, so it proved only that three documents agreed about a number the caller chose — a forged intent plus a matching digest satisfied every comparison. It now derives both the intent and review digests itself and takes no digest argument, and it checks approval.review_digest against the recomputed review, which catches a tampered review that kept its review_id. ConnectorBindingRef carried only binding_generation, which cannot tell two accounts apart: both are generation 1 the day they are linked. It now carries provider_id and a stable opaque account_binding_id, and connectorBindingMatches derives its comparison from the field list so adding a field cannot leave the check silently testing the old set. BrokerAccountBindingV1 gained the matching account_binding_id. The chain also refuses a refused or expired review and a spent or expired approval, rather than validating a structurally perfect chain over a dead one. New verifyCommitAuthority() is the gate immediately before a broker commit: it proves the order is aimed at the account the local binding names, under a grant that still permits it, in a mode that still allows submission. Kill and pause are re-checked there rather than inherited from whatever the review said minutes earlier. The mutation test now classifies EVERY approval field as cross-bound or recorded-only and asserts each behaves as classified. A field missing from that map fails the test, so adding one to the contract forces a decision instead of landing unchecked. 2. THE CANONICALIZATION WAS NOT AN RFC 8785 SUBSET. The previous module re-exported the device-runtime encoder and called the result "jcs-integer-subset/1". That encoder escapes every non-ASCII character (ensure_ascii=True), while JCS requires non-control Unicode emitted literally and encoded as UTF-8 — different bytes, and a different digest, for any document containing a non-ASCII character. The name implied a narrowing when the difference was the string encoding. ats_contracts now has a real JCS implementation: literal Unicode, keys sorted by UTF-16 code unit, ECMAScript number-to-string, -0 normalized, lone surrogates refused. Profile renamed to rfc8785/1. device_runtime's encoder is untouched and must stay that way — it is pinned to the Cloud's Python ensure_ascii and changing it would invalidate device signatures. Float rejection moves to where it belongs: JCS serializes floats fine, so the encoder accepts them and the schema validators keep refusing them. Integer minor units remain the contract rule. The fixture gains encoder vectors for Unicode, astral characters, property ordering, -0, control escapes and large integers, plus lone-surrogate rejects. test/fixtures/ats_contracts_golden_verify.py is an independent Python implementation that reproduces all 18 checks byte-for-byte; it is the mirror ATSv2 lifts. It is not wired into npm test because CI has no guaranteed Python and a conditional skip would give false assurance. Worth knowing for anyone writing the Python side: sorted() is WRONG. Python compares code points, RFC 8785 requires UTF-16 code units, and they disagree when an astral character meets a BMP character at or above U+E000. The fixture contains a vector that fails loudly on exactly that and passes everywhere else. 3. A MALFORMED ABUSE FLAG READ AS NOT ABUSIVE. abuse_flagged used `raw.abuse_flagged === true`, turning a missing key, a null or the string "true" into false — silently downgrading an abuse flag in the one direction it must never fail in. Now a strict boolean. 4. PRIMITIVE VALIDATION. timestamp() re-derives the calendar fields from the parsed instant and compares them back, because Date.parse rolls 2026-02-30 over to 2 March. An expiry that quietly moves is not an expiry. CONTROL now rejects every C0 character including tab, newline and carriage return, matching what its comment always claimed. Fields validated by text() are single-line by construction and reach logs and support bundles, where an embedded newline lets one record forge a second. multilineText() is added for genuine prose (journal notes, compiler diagnostics): it permits newline and tab and REFUSES carriage return rather than normalizing it, because these strings are digested and folding CRLF would let identical-looking text hash two different ways. The opaque-reference comment no longer overclaims. A prefix cannot prove an account number is absent from the body; acct_000123456789 satisfies the shape. Non-reversibility has to be produced upstream by the connector core minting these identifiers, and the comment now says so.
|
All four required corrections are in 1. Approval chain
2. Canonicalization — you were right, and the name was the worst part
Float rejection moved as you specified: the encoder accepts floats (JCS serializes them fine), the schema validators keep refusing them. A test asserts both halves so neither drifts into the other. Cross-language verification, done before freezing as you required. New vectors: Unicode, astral character, property ordering, One finding worth flagging for the ATSv2 side. Python's The fixture carries a vector that fails loudly on exactly this and passes everywhere else, so a Python mirror that gets it wrong finds out at test time rather than at reconciliation time. 3. Abuse flag
4. Primitives
Verification
One unrelated observation. |
Caught by the Spec 2 lane reviewing the freeze: every contract document in the fixture was pure ASCII, and the old ensure_ascii encoder and a correct JCS encoder produce byte-identical output on ASCII. So all eight contract digests would still have matched after a regression to \u-escaping — the schema layer pinned nothing about the encoding change this PR exists to make. Measured before and after, by re-digesting every fixture entry under the old escaping rule and counting which recorded digests disagree: before: canonical 3/8 catch it, contract 0/8 after: canonical 3/8 catch it, contract 3/8 The canonical vectors were already covering the encoder, so the fixture was not blind — but a fixture where only one of two layers can detect the bug is one edit away from being blind, and regenerating contract vectors under a regressed encoder would have produced a self-consistent, silently wrong set. Two contract documents now carry non-ASCII in fields that would realistically hold it: an execution-state `effective_reason` with an em dash and a euro sign, and an account-binding `masked_label` with a middle dot and bullets. That also exercises text() accepting legitimate non-ASCII while still refusing control characters, which nothing covered before. Added a test that fails if someone tidies the non-ASCII back out. It does not merely assert the characters are present — it re-digests a covered document under the old escaping rule and requires the recorded digest to disagree, so the coverage is proven rather than assumed. Python mirror re-verified: 18 checks still reproduced byte-for-byte.
|
Follow-up in Raised by the Spec 2 lane reviewing the freeze. I measured it rather than taking it at face value: I re-digested every fixture entry under the old
The encoder vectors were never blind — That's not just missing redundancy. Regenerating contract vectors under a regressed encoder would have produced a self-consistent, silently wrong set, with only the canonical vectors objecting. A fixture where one of two layers can detect the bug is one edit from being blind. Fixed by putting non-ASCII in two fields that would realistically carry it — an execution-state Added a test that fails if the non-ASCII is ever tidied back out. It doesn't assert "these characters are present" — that would pass even if the encoder stopped caring. It re-digests a covered document under the old escaping rule and requires the recorded digest to disagree, so the coverage is proven on every run rather than assumed. Re-verified after the change:
|
…sion The Spec 2 lane found a self-lowering floor in its own guard — a threshold derived from the list it was checking, so it dropped as the list shrank and stayed green to one. Checking mine for the same shape turned up the same defect by a different mechanism. My floors were constants, so they could not lower themselves. But the contract floor was `>= 1` against actual coverage of 3, which is the same failure with extra steps: a constant set far below reality never rises to meet it. Measured rather than reasoned about — tidy the non-ASCII out of two of the three covered documents and regenerate their digests honestly, and coverage falls 3 -> 1 while the guard stays green. That lands in the worst available state. One catching vector that nobody knows is the only one is strictly worse than zero, because zero is unambiguous and one looks like coverage. Fixed: - MIN_CANONICAL_COVERAGE and MIN_CONTRACT_COVERAGE are named constants at the top of the file, independent of the fixture, both set to the coverage that actually exists. Raising them is a deliberate edit; lowering one should be argued for in review. - The digest disagreement is now asserted for EVERY covered document, not just contractCovered[0]. Checking only the first would let the other two be regenerated under a regressed encoder with the test still green, and the failure message names the schema that stopped pinning the encoding. Re-ran all four scenarios against the corrected guard: baseline -> passes, coverage 3 honest tidy-up (strip AND regenerate) -> fails the contract floor erosion 3 -> 1 (regenerated honestly) -> fails the contract floor regenerated under a regressed encoder -> fails the digest check, by schema control: mutate with stale digests -> fails for the wrong reason, proves nothing The control is worth keeping in mind: it disagrees because any document change breaks a stale digest, not because the guard works. That is the third instance today of a check that was green while proving nothing, so it is recorded here rather than left as folklore. Full suite 2630 tests, 2620 pass, 0 fail, 10 skipped. verify:production ok at 4,556,505. Python mirror still reproduces 18 checks byte-for-byte.
|
They hit a self-lowering floor: a threshold derived from the list it guarded, so it dropped as the list shrank and stayed green all the way to one. My floors were constants, so they couldn't lower themselves, and I nearly stopped at "different structure, not affected." Then I ran it. The contract floor was Same failure, extra steps. Theirs lowered itself to meet a shrinking list; mine was set so far below reality it never had to move. A constant floor is only safe if it equals the coverage that actually exists — otherwise it's a derived floor that someone derived once, badly, and froze. It also lands in the worst available state: one catching vector that nobody knows is the only one is strictly worse than zero, because zero is unambiguous and one looks like coverage. Fixed:
All four scenarios re-run against the corrected guard:
That control is worth keeping: it disagrees because any document change breaks a stale digest, not because the guard works. The pattern, now four instances across the two lanes. Every one was a check that was green while proving nothing, and the tell was identical each time — the check's own correctness depended on something the check did not verify:
What worked every time wasn't more assertions — it was running the scenario the check claims to catch, and confirming it fails for the stated reason. That last clause is what caught #2. Verification: full suite 2630 tests, 2620 pass, 0 fail, 10 skipped · |
1 participant
Gate 1.0 of the trading integrations finale — the shared schema freeze that Spec 1 §17 and Spec 2 PR 2.1 both put first. It lands the shapes and nothing else: no connector write path is enabled.
Why this shape
Both specs' primary invariant is that authority lives in exactly one place. The cheapest way to keep that true is to make the unsafe states unrepresentable rather than merely forbidden in review. So these contracts are closed documents with real validators, not bare TypeScript interfaces — an unknown key is a refusal, and a model-originated proposal is treated as attacker-controlled input throughout.
What is frozen
Eight schemas under
src/core/ats_contracts/:execution-state,connector-capability,account-binding,delegated-trading-grant,equity-order-intent,order-review-receipt,operator-approval,execution-receipt.Invariants the shapes enforce, each with a test:
grants_execution_authority: false; any other value fails to parse.offline < observe < review_only < paper < approve < auto. Halt states may follow any request; a differing effective mode must carry a reason.confirmationis pinned toper_order. Robinhood can permit unattended placement; that mode is not expressible here.redactBindingForExport()is the only exported projection.verifyApprovalChain()returns a terminal verdict — there is deliberately no re-preview or reroute path, because that path is how an approval minted against a paper preview would end up authorizing a live order.ambiguousis a first-class outcome requiring a reconciliation state, so a commit that may have started is never retried.Two judgement calls worth your review
1. Canonicalization is an honest subset, not RFC 8785. The specs say RFC 8785. I reused the existing
src/core/device_runtime/canonical_json.tsrather than adding a second encoder, and it is stricter: it refuses non-integer and non-finite numbers outright. On integers it agrees byte-for-byte with 8785 and with Python'sjson.dumps(sort_keys=True, separators=(",",":"), ensure_ascii=True).docs/CONTRACTS.mdstates the difference plainly rather than claiming full 8785. Consequence: every monetary field is integer minor units and every quantity a whole share — floats are how a preview and a commit come to disagree about a number that looked equal.2.
effective_modeis the union of both specs' lists. Spec 2 §7.2 givesoffline|observe|paper|approve|auto; Spec 1 §11.8 givesobserve|review_only|orders_paused|emergency_locked. They describe one projection from two vantage points, so this freezes the union. Splitting them would let a runtime-authoredorders_pausedfail to parse on the connector side — a halt that does not deserialize is a halt that does not stop anything.Test plan
npm run typecheckcleannpm run verify:production→{"ok":true,"packedFiles":852,"packedBytes":4539531}test/fixtures/ats_contracts_golden.json— one validated document per schema with its canonical digest, following the existingbridge_conformance.jsondrift-detector pattern. The ATSv2 Python suite mirrors it, so canonicalization divergence fails a test instead of a trade.node:fs/node:net/node:http/node:child_process/fetch(/process.env— this is what makes "no write path enabled" checkable rather than asserted.Correction to an earlier revision of this description
An earlier version of this body claimed 5 pre-existing test failures and that the package was 433 KB over its size budget. Both claims were wrong and are withdrawn. They came from my own broken build:
buildistsc -p tsconfig.json && node dist/scripts/copy-skill-assets.js, and I had been running barenpx tsc, which silently skipped the asset copy. That produced adistwith nodist/src/skills/builtin/, which is what failed the four skill tests — including in the "clean main baseline" worktree I used to attribute them, so the attribution was wrong too.The size claim was a second, independent error: I summed raw
dist/srcbytes on disk, butMAX_UNPACKED_BYTESis compared against npm'sunpackedSizefor the packed set. Those differ sharply because sourcemaps are not packed at all — 0.mapfiles ship. Real figures on this branch:unpackedSize(what the gate checks)MAX_UNPACKED_BYTESSo there is no size breach, this PR does not approach one, and the earlier suggestion to strip
.js.mapwas doubly unnecessary — they were never in the package. Thanks to the Spec 2 lane for pushing back on the number rather than taking it.Scope and coordination
Deliberately not in this PR:
strategyCount()atsrc/commands/ats_agent.ts:139returns every scanned file includingrejected/needs_conversion, so setup can report "6 strategies" with 0 compiled — the dishonest-readiness case Spec 2 §2.1 names. It feeds the zero-strategy check at :313, the persisted count at :327 and the journal count at :391. Confirmed real; it belongs to Spec 2 PR 2.3, and that lane has taken it.Next gates consume these shapes: 1.1 headless connector core, 1.5 local operator gateway, 1.6 delegated paper hardening.
Merge order: #161 targets this branch, so GitHub Actions does not run on it (
ci.ymltriggers onpull_requestwithbranches: [main]). Land this first; #161 then retargets tomainand gets the full matrix before approval.