diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f6845714a2..8c0855385f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -134,8 +134,9 @@ jobs: with: ruby-version: "3.3" - # Offline and first: the gate below is a no-op whenever the allowlist - # references no issues, so this is what proves it rejects anything. + # Offline and first: a green run of the live gate below proves only that + # today's referenced issues are open, so this is what proves it REJECTS + # anything — closed issues, malformed rows, the fail-closed paths. - name: Self-test the known-defect gate run: make test-check-known-defect-issues-open diff --git a/MIGRATING.md b/MIGRATING.md index 34d8aaa0d1..a0ae565cb8 100644 --- a/MIGRATING.md +++ b/MIGRATING.md @@ -818,13 +818,12 @@ constructed. They now sleep the server's `Retry-After` — both wire forms, delta-seconds and HTTP-date — in place of the backoff, with no jitter and no ceiling beyond what the host can represent: a value the parser holds but the host cannot schedule saturates at 2147483647 seconds (~68 years) rather than -wrapping negative, and that figure does not vary by architecture. A value too -large for the parser's own `int64` is treated as malformed instead and falls -through to the backoff curve, as it always did. That split is Go's: SPEC §6's -parsing algorithm says only to parse a positive integer, #793 states the -two-tier rule (unrepresentable → malformed, unschedulable → saturate) in §6 -"Retry-After Honouring", and #799 tracks the cross-SDK convergence on -over-range values, which the six SDKs still answer differently. +wrapping negative, and that figure does not vary by architecture. Every +over-ceiling digit string saturates there, including one too large for the +parser's own `int64` — the earlier two-tier rule (unrepresentable → malformed, +unschedulable → saturate) is gone from every Go parser — and #799 tracks the +cross-SDK convergence on over-range values, which the six SDKs still answer +differently. **Two behaviours changed for `DownloadURL` and the rate-limiter hook as well**, because all three paths share `parseRetryAfter`: an HTTP-date's sub-second @@ -834,7 +833,7 @@ the backoff curve; and a delta-seconds above the schedulable ceiling saturates instead of wrapping. The wire operations those paths perform are unchanged, and they already honoured the header on 429 — it is what the header parses to that moved. Typed service methods run the generated retry loop, which has its own -copy of the parse and is untouched (#798). +copy of the parse; its clamping landed with #855 (the defect was #798). **Wrong behaviour you get if you ignore it:** none, but the wait between attempts on a throttled account can now be seconds or minutes where it used to diff --git a/Makefile b/Makefile index 4e0371166d..fec2599a9e 100644 --- a/Makefile +++ b/Makefile @@ -156,9 +156,10 @@ check-known-defect-issues-open: @echo "==> Checking known-defect tracking issues are open..." @./scripts/check-known-defect-issues-open -# Drive that gate from outside. Its live run is a no-op today — the allowlist -# references no issues — so without this NOTHING exercises the closed-issue -# rejection, the fail-closed path, or the second reference shape. Offline: PATH +# Drive that gate from outside. Its live run verifies whatever the allowlist +# and registry currently reference, and a green run proves only that those +# issues are open — so without this NOTHING exercises the closed-issue +# rejection, the fail-closed paths, or the second reference shape. Offline: PATH # is stripped to a stub `gh` answering from a canned table, because a self-test # that asked GitHub would assert against whatever is true this morning. # diff --git a/SPEC.md b/SPEC.md index 081981bb6b..97e5401005 100644 --- a/SPEC.md +++ b/SPEC.md @@ -3615,13 +3615,93 @@ Two dispatch clarifications, pinned: (implementation-chosen; the Go reference uses 256). At capacity the pump **blocks** — back-pressure propagates to the socket and TCP — rather than dropping: the state-machine-owned live buffer is the only place a frame can ever be dropped, and its - overflow signal is the only drop signal. Worst-case connector memory is therefore - bounded multiplicatively — every queued or buffered item is itself bounded by - `EVENT_FEED_MAX_FRAME_BYTES`, so the ceiling is - (pump depth + `EVENT_FEED_LIVE_BUFFER_CAPACITY`) × `EVENT_FEED_MAX_FRAME_BYTES` + overflow signal is the only drop signal. **The retention ceiling below is the GO + REFERENCE IMPLEMENTATION'S**, stated in its own terms — two goroutines, a + `json.RawMessage` copy, a copying decoder — **and it presumes a transport with + bounded reads**. Every SDK's cable lane inherits the shape (bounded queue, blocking + hand-off, single deferral slot, the buffer as the only drop point) but re-derives its + own weights, and one recorded divergence already breaks the per-item premise + elsewhere: TypeScript's default global-`WebSocket` lane cannot pre-bound a read, so a + single oversized message is allocated whole at receipt, before the + `EVENT_FEED_MAX_FRAME_BYTES` check drops it — the scenario-lane table in Appendix F + records that as an accepted divergence, and no universal cross-SDK byte ceiling is + published here. The ceiling also counts FRAMES, not errors: exactly one non-frame item + can ride the queue or the deferral slot — the read error that ends the pump, at most + one per attempt because the pump exits by sending it — and its SIZE is + transport-authored, unbounded by the seam contract. The built-in transport's errors + are bounded by construction (fixed shapes whose renderings are configured text or + placeholders, never server bytes); a custom transport's error is its author's to + bound. In the Go reference the accounting SPLITS, because two kinds of memory answer + different questions. + + **RETAINED storage is what a consumer sizes against** — raw frames and buffered + events held across blocking points, an enumeration by HOLDER, which is what closes + the count: the hand-off queue (≤ pump depth), the live buffer + (≤ `EVENT_FEED_LIVE_BUFFER_CAPACITY`), the single deferral slot (≤ 1), and one + in-hand frame for each of the exactly two goroutines that touch frames. The retained + worst case is + (pump depth + 3 + `EVENT_FEED_LIVE_BUFFER_CAPACITY`) × `EVENT_FEED_MAX_FRAME_BYTES` (≈ 10 GiB at the defaults' extreme, reached only if every slot holds a maximum-size - frame) — even under a slow consumer. Implementations MAY additionally impose a total - byte cap on the live buffer; if they do, eviction routes through the same overflow + frame) — even under a slow consumer. The **+ 3** is three frame-sized retentions the + queue's depth does not count, held by different parties at the same time: + - the **pump's own in-flight frame** — the pump is a single reader, so it may hold exactly + one frame it has already READ and not yet handed off. One rather than an unbounded + number for that reason: one reader holds at most one frame outside the queue. + - the **state machine's in-hand frame** — the protocol-fatal scan's dequeue is the very + receive that lets a blocked pump refill the queue, so while the scan still holds that + frame — examining, admitting, or parking it — the queue is full again and the pump may + already hold its next read. A single consumer, so one frame in hand, for the pump's + own reason. + - the **deferred socket outcome** — the single slot the in-flight-poll servicing and the + drain's scan park one receive in. It is retained while the queue behind it refills, so it + is concurrent with a full queue and with both in-hand frames, not an alternative to + any of them. + + **TRANSIENT decode-time allocation rides on top, per frame, in the state machine's + hands alone** (the pump never parses), bounded by a small implementation-topology + multiple of the frame being decoded rather than by a published constant. The multiple + covers the decode chain's representations — the wire bytes, `parseFrame`'s envelope + `json.RawMessage` copy, `decodeMessageEvent`'s per-field + `map[string]json.RawMessage`, the decoded `Event`'s strings — plus decoder overhead + proportional to member count: map bucket storage and copied keys, which a frame of + many tiny members inflates past any per-representation count. That is exactly why + the multiplier is NOT published: it is implementation topology, not contract — it + moved when the exact-spelling per-field decode was coded in, and pinning a number + would turn every decoder refactor into a spec change. What IS contract: transients + exist only between a frame's dequeue and its decode returning, one frame at a time, + so **peak frame-payload retention is the retained formula plus ONE frame's + transient allocation** — never a per-slot or per-queue multiplier. Payload, not + process memory: the equality deliberately excludes the two things it cannot + bound — the single transport-authored error item, whose size is its author's + (stated above), and runtime metadata (channel and `pumpItem` storage, slice and + map headers), which scales with the configured capacities, not with frame + bytes. + + The retained enumeration cannot grow by a further party being noticed: every + retained frame is in one of the three counted structures or in the hands of the pump + or the state machine. The live buffer's weight is one per slot: a buffered `Event` + retains only the chain's LAST representation — its strings are copies, since Go's + decoder never aliases its input buffer, and its `details` bytes are the decoder's own + clone of that member (a bounded slice of the frame, retained per slot alongside the + strings; never the frame itself) — so no transient survives admission, and a slot's + weight is the decoded event, details included. + + The formula is the cable lane's retention, and only that — every counted item is a + raw socket frame or a buffered live event. The poll lane sits outside it on purpose: + `PollSource.Poll` returns one page decoded whole, and the walk retains that page + until its rows are delivered. What bounds it is shape, not size: pages are fetched + sequentially, so a walk holds at most one live page (a superseded attempt's in-flight + poll may briefly hold another before its result is discarded), but the page's SIZE is + the server's pagination decision — `EVENT_FEED_MAX_FRAME_BYTES` governs socket + frames and says nothing about an HTTP body the generated layer decodes. A + total-connector memory bound would need a poll-page cap this contract deliberately + does not impose. + + The drain's protocol-fatal scan is budgeted at `pump depth + 1` and not at this figure, + which is not an inconsistency: the budget counts what the scan may DEQUEUE — the queue plus + the pump's held frame — while the ceiling counts what may be RETAINED, and the deferral slot + is retained without being dequeued by that scan. Implementations MAY additionally impose a + total byte cap on the live buffer; if they do, eviction routes through the same overflow signal, never a silent drop. - The transport negotiates subprotocol `actioncable-v1-json`, sends no `Origin` header (non-browser clients), and passes the mint URL through untouched, query string included. @@ -3991,9 +4071,26 @@ logged (Security Invariants below). Required tier-2 coverage: a hostile cross-origin `next` mid-walk, a hostile 410 `resume` URL, and a validated same-origin `next` answering 302 with a cross-origin -`Location` each terminate with `invalid_continuation` and zero requests to the foreign -origin; store-failure coverage proves Failed(load) terminates with zero wire attempts and -Failed(save) continues with the observer signal and a subsequent save attempt. +`Location` each terminate with `invalid_continuation`, are not retried, and issue no +further poll; store-failure coverage proves Failed(load) terminates with zero wire +attempts and Failed(save) continues with the observer signal and a subsequent save +attempt. + +**Zero egress to the foreign origin splits at the seam.** For the hostile `next` and +`resume` cases the target is connector-visible and tier 2 owns the coverage: a +connector that follows one hands the URL to the poll seam, which the driver observes +and fails — fixtures 26/27 assert zero requests to those hosts, structurally (no step +ever serves them, and the harness's servers own only their own origins). For the +redirect the obligation is Layer-1's, and this paragraph used to require it at tier 2: +the poll lane IS the seam, so the driver reduces the `Location` to its origin and +hands the connector a refusal verdict. The connector never sees a `Location` and never +decides whether to follow one, which makes the foreign origin unreachable by +construction of the harness — a harness that asserted no request reached it would be +asserting something about itself. That obligation belongs to the Layer-1 seam +adapter's own 302 test, where a real generated `PollEvents` call meets a real redirect +against an adapter with automatic redirect-following disabled. +`conformance/event-feed/README.md`'s row-15 note records it as a pending obligation +rather than a proof the repository contains; the adapters are tracked in #819. ### Clock, Timers, and Virtual Time `[conformance]` @@ -4048,8 +4145,44 @@ the advance whose deadlines land inside the window also fire; ties break by crea order.* A harness may additionally fire a named timer without advancing the clock, asserting its scheduled delay against a `{min, max}` envelope — that is how jitter is asserted without a cross-language RNG seam. Each language's test clock passes a shared -semantics checklist (deadline order, reentrant scheduling within an advance, creation-order -tie-break) before its tier-2 results count. +semantics checklist (deadline order, creation-order tie-break) before its tier-2 results +count. + +**The reentrant clause is normative for the algorithm and forbidden as a fixture +dependency.** It stays in the algorithm because a clock that ignored it would fire the +wrong set. But it is UNSCRIPTABLE wherever the connector runs concurrently with the +driver: whether a timer armed during the window lands inside it depends on when the +connector's goroutine, thread, or task got scheduled, which no fixture can pin. So **no +fixture may rely on it, and every driver MUST REJECT an `advance` whose window would fire +any timer**, naming `fireTimer` as the deterministic alternative. + +The test is what would FIRE, decided from the clock's state before time moves — not what +gets ARMED. Arming happens on the connector's schedule, so a driver can only look for it +by waiting and then assuming nothing further is coming, which is a heuristic wearing a +MUST and passes a late arm in silence. Firing is one atomic read under the same lock the +advance selects under. The inversion is sound because a test clock releases that lock only +across a firing's aftermath — so an advance that fires nothing never wakes anything and +cannot cause an arm, leaving nothing to detect. It is stricter than an arming rule (a +firing that replaces nothing is rejected too) and that is the trade: a script wanting that +firing writes `fireTimer` and names the timer. The due-set read also needs a settled set +to read — an action's completion can precede the timer arms its transition causes — so +every `advance` must be the scenario's first step or immediately follow the two-step +rendezvous `expectState` then `expectTimers`, enforced at fixture load (an empty +rendezvous set is rejected with it — it orders nothing). Neither step alone settles: a +set match can coincide with a transient mid-surgery set (timer surgery spans clock +acquisitions), and an announcement can precede a tail arm. Together they do — the +announcement bounds the surgery, and any timer still unarmed at the announcement is +exactly what the exact-set match then waits for, both blocking under the watchdog so +wrong authorship fails loudly on every schedule where the stale pair no longer holds — +a pre-action pair can pass on the schedule where the action is not yet processed, so a +wrong script is at worst flaky, never stably green; the settled guarantee is for +correctly authored pairs. A transition that announces no state change, or only +rearms a timer of the same kind and count, is invisible to this rendezvous — a served +live frame's pump-side `staleness` rearm is the concrete case. Such a script overrides +`stalenessMs` large so no deadline, old or new, sits inside a window it advances (the +schema's own guidance, and what the suite's one advance does), or uses `fireTimer` for +the firing it actually wants. `conformance/event-feed/schema.json`'s +`$defs.advance` states both, and the driver obligation is enforced there. Teardown discipline: disposing a connection attempt — deadline lapse, staleness, socket death, terminal — cancels the frame pump, **cancels any in-flight seam call belonging to diff --git a/conformance/event-feed/README.md b/conformance/event-feed/README.md index fb36716dcb..22f2760dd7 100644 --- a/conformance/event-feed/README.md +++ b/conformance/event-feed/README.md @@ -111,8 +111,16 @@ teeth): Literal origins (e.g. `https://attacker.example.com`) are intentional and must **not** be substituted — the hostile-continuation fixtures depend on them staying -foreign, and the harness must assert those hosts receive **zero** requests -(structurally guaranteed: no expect step ever serves them). +foreign, and **where the connector itself holds such a URL as a continuation +target** (fixtures 26 and 27) the harness must assert those hosts receive +**zero** requests (structurally guaranteed: no expect step ever serves them). + +That obligation is deliberately scoped to connector-visible targets. Where a +foreign origin reaches the connector only through a value the seam has already +converted — fixture 30's redirect `Location`, which the driver reduces to an +origin before the connector sees anything — there is no egress for a harness to +observe, and asserting its absence would be a statement about the driver rather +than about the connector. See the row-15 note under the mutation kill matrix. ## Count semantics: seam calls, never wire attempts @@ -133,8 +141,81 @@ asserting its scheduled delay against a `{min, max}` envelope — that is how ji is asserted without a cross-language RNG seam (Go additionally pins the full-jitter formula exactly in tier 3; a degenerate always-0 RNG is caught only there — a documented divergence). Each language's test clock passes the shared semantics -checklist (deadline order, reentrant scheduling within an advance, creation-order -tie-break) before its tier-2 results count. +checklist (deadline order, creation-order tie-break) before its tier-2 results +count; the reentrant clause stays normative for the algorithm — a clock that +ignored it would fire the wrong set — but no tier-2 fixture can reach it (next +paragraph), so it is not part of that gate. + +**The reentrant clause is unscriptable where the connector runs concurrently, +so no fixture may rely on it.** In a single-threaded test clock, "a timer armed +during the window also fires" is exact. Where the connector runs on its own +thread or goroutine it is a scheduling question: the same fixture can fire the +follow-on in one language and not in another. There is no settle that fixes +this — waiting for the firing to be CONSUMED deadlocks against §23's own +requirement that a staleness window closing during a delivery is latched and +observed later, and waiting for the follow-on ARMING requires knowing one is +coming, which nothing can tell you. + +**So an `advance` whose window would fire ANY timer is rejected**, and the +driver names `fireTimer` as the alternative — it fires one named timer without +moving the clock, so no re-selection is involved. This is unconditional, not a +per-fixture opt-in: a flag would let a fixture author take the divergence +instead of avoiding it. + +The rule asks what an advance would FIRE, not what it arms, and the difference +is the whole reason it is enforceable. Arming happens on the connector's +schedule, so a driver can only look for it by waiting and then guessing that +nothing more is coming — a heuristic wearing a MUST, which silently passes a +late arm. Firing is decided by the clock's own state before time moves: one +atomic read, under the same lock the advance selects under, answers it +completely. + +That inversion is sound because of what an advance does when it fires nothing. +A test clock holds its lock while selecting due timers and releases it only +across a firing's aftermath — deliberately, so a woken recipient can arm inside +the window. An advance with nothing due therefore never releases the lock, never +wakes anything, and cannot be the cause of any arm. There is nothing left to +detect. + +It is stricter than an arming rule, and deliberately: a firing that replaces +nothing is rejected too. A script that wants that firing writes `fireTimer` and +says which timer it means, which is more legible anyway. The Go driver +self-tests three arms — the rejection, an ordinary quiet-window advance still +passing, and a firing that arms nothing being rejected all the same. + +**And the due-set read needs a settled set to read.** An action's completion +can precede the timer arms its transition causes — a connect is observable +before the handshake deadline is armed on the connector's own thread — so an +advance placed right behind an action races those arms: rejected on one +schedule, accepted with time moved past a deadline about to arm on another. +The rendezvous is authored, not guessed, and it is TWO steps: every `advance` +must be the scenario's first step or immediately follow `expectState` then +`expectTimers`, enforced at fixture load (an empty `expectTimers` set is +rejected with it — it orders nothing). Neither step alone settles. A set match +can coincide with a transient mid-surgery set: the welcome transition stops +`handshake-deadline` and arms `confirmation-deadline` in separate clock +acquisitions, so an authored set can exist in the gap. An announcement can +precede a tail arm in a driver that announces first (the Go reference arms Backoff's +timer before announcing, precisely so the set is already exact — a port need not, and +the rendezvous must not depend on it). Together they +settle — the announcement bounds the surgery, and any timer still unarmed at +the announcement is exactly what the following exact-set match waits for. Both +steps block under the scenario watchdog, so a wrongly authored state or set +fails loudly on every schedule where the pair no longer holds. The precise +guarantee: a pair naming the PRE-action state can pass on the schedule where +the action has not yet been processed, so wrong authorship is at worst FLAKY — +red whenever the transition lands first — never stably green; the settled +guarantee belongs to correctly authored pairs, the ones the per-state tables +define. What stays outside this +rendezvous is a transition that announces no state change, or only rearms a +timer of the same kind and count — invisible to both barriers. The concrete +case is a live frame served in a socket-open state: its receipt rearms +`staleness` pump-side with no announcement, so an advance behind it would race +the rearm's deadline shift. A script that must advance across served frames +takes the schema's own `stalenessMs` guidance — override it large, so both the +old and the new deadline sit outside any window the script advances (fixture +05 does exactly this, at ~11.5 virtual days against a 121-second window) — and +a script that wants the staleness firing itself writes `fireTimer`. ## Contract notes the fixtures encode (SDK-owned, final) @@ -238,7 +319,8 @@ revoked-mint threshold). | 27 | `27-hostile-resume-cross-origin.json` | accepted 410 with a cross-origin `resume` → Terminal(`invalid_continuation`), zero foreign requests | | 28 | `28-checkpoint-load-failure.json` | store load Failed → Terminal(`checkpoint_load`) with ZERO wire attempts; distinct from Missing (which proceeds to a present entry) | | 29 | `29-checkpoint-save-failure-continues.json` | save Failed → feed continues and a SUBSEQUENT save is attempted (exact store-call script: no save circuit breaker) | -| 30 | `30-continuation-redirect-cross-origin.json` | validated same-origin `next` answering 302 + cross-origin Location → Terminal(`invalid_continuation`), zero foreign egress | +| 30 | `30-continuation-redirect-cross-origin.json` | validated same-origin `next` answering 302 + cross-origin Location → Terminal(`invalid_continuation`); zero foreign egress holds by construction of the seam here, and proving it against a real redirect is ASSIGNED to Layer 1, whose adapters are still pending, tracked in #819 — see the row-15 note | +| 31 | `31-post-snapshot-straggler-below-served-id.json` | post-snapshot straggler with an id BELOW the entry page's served id delivered live; the re-push of that served id still suppressed | | 34 | `34-filter-changed-409-reenters-at-the-present.json` | 409 with both digests → `Observer.filterConflict` (digests pinned) before `Observer.positionRejected(filter_changed)`; the held position is discarded and the walk re-enters at `since=now` (present-class, no poll-served id) | **Hostile-URL coverage note (stated author's choice, per the PR-1 review):** the @@ -265,7 +347,7 @@ reason via a constant, not the literal. | Disconnect reason literal `unauthorized` (arrives only pre-welcome) | 1 (+ 2 for the pre-welcome timing) | 07 | | Disconnect reason literal `invalid_event_stream_command`, `reconnect:false` | 1 | 06 | | Disconnect reason literal `remote`, `reconnect:true` | 1 — **no transcript capture exists**; source-verified against the pinned Rails; its freeze rides bc3's disconnect-matrix re-verification plus the one requested capture frame | 17 | -| Poll body envelope keys `events` / `position` / `next` | 1 | every fixture serving a 200 poll: 01, 02, 05, 07, 12, 16, 17, 19, 20, 22, 26, 29, 30 (mechanically derived from the fixture files; re-derive when the set changes) | +| Poll body envelope keys `events` / `position` / `next` | 1 | every fixture serving a 200 poll: 01, 02, 05, 07, 12, 16, 17, 19, 20, 22, 26, 29, 30, 31 (mechanically derived from the fixture files; re-derive when the set changes) | | Mint response body `{ticket, expires_in, url}`, status 200 | 1 | every fixture with `expectMint` (all but 28) | | Subscribe identifier literals: channel `EventsChannel`, param spellings `types`/`buckets`/`creators`/`performers`/`exclude_performers`/`actor_types`, comma-joined values | 1 | channel: every `expectSubscribe`; `types` spelling: 01 (its `expectSubscribe` pins `params` explicitly, single-valued); `buckets`/`creators` spellings + comma-joining: no PR-2 fixture — pinned at PR-4 (fixture 15, whose retransmit case also pins byte-identity of the identifier) | | 409 body: all three keys `error` / `position_digest` / `filters_digest` required; digest values bare 16-hex (no `srv2-` prefix), `error` content unconstrained | 1 | 34 (served, both digests forwarded to the connector and pinned on Observer.filterConflict); the tier-1 dispatch case additionally owns the wire pin | @@ -276,7 +358,7 @@ reason via a constant, not the literal. | Filter raw bounds: a filter list of > 1,000 elements or > 16 KB → filter 400 | 2 | unreachable through validated construction (the client caps at 100 ids); recorded, unpinned | | `since=now` / bare entry mints the cursor at the newest visible id; an empty entry page positions above an in-flight lower id N | 2 | 19, 20 | | Safety-horizon bound: position-relative, best-effort, ~30s — never wall-clock | 2 | premise of 19/20 (not directly assertable client-side; the entry-boundary fixtures encode its consequence) | -| Frozen-head `next` predicate: absent `next` = the walk reached its head | 2 | every fixture whose walk ends on a 200 page without `next`: 01, 02, 05, 07, 12, 16, 17, 19, 20, 22, 29 (mechanically derived; re-derive when the set changes) | +| Frozen-head `next` predicate: absent `next` = the walk reached its head | 2 | every fixture whose walk ends on a 200 page without `next`: 01, 02, 05, 07, 12, 16, 17, 19, 20, 22, 29, 31 (mechanically derived; re-derive when the set changes) | | 410 `resume` re-enters at the epoch (`since=`, in served history — a position-resume entry) with the canonical filter set preserved | 2 | 16 (resume URL followed verbatim); 27 (hostile variant) | | 400-position / 409 re-entry semantics (`since=`, present-class fallback) | 2 | 34 (409, present-class fallback); the 400-position and poll-served-id variants remain PR-4's | | Ticket statelessness + ~120s TTL (server-owned `expires_in`) | 2 | 05 (TTL-advance premise; `expires_in` never schedules anything) | @@ -322,10 +404,20 @@ when every line is done: 7. Any drifted row: fix fixtures, schema, and SPEC §23 together in the true-up PR — never fixture-only. -## Mutation kill matrix (fifteen) +## Mutation kill matrix (sixteen) + +Fifteen of the sixteen mutations are shown red against at least one fixture in +the reference implementation PR's body before they count. Row 15 is the +recorded exception — not killed at tier 2, pending the Layer-1 adapters #819 +tracks — and the note below is its account. -Each mutation is shown red against at least one fixture in the reference -implementation PR's body before it counts. +**One row is an exception, and it is the reason this heading is worth reading +twice.** Row 15's mutation is **not killed at tier 2 at all** — it lives below +the poll seam, where no tier-2 harness can reach it. Fixture 30 is named +against it because it pins a different fault class at the same boundary, not +because it kills the mutant. Every other row is a real kill. A matrix that +counted an unreachable mutant as killed — or as half-killed — would be making +exactly the class of claim this family exists to check. | # | Mutation | Killed by | |---|---|---| @@ -343,7 +435,45 @@ implementation PR's body before it counts. | 12 | `bypass-configured-handler` (handler registered but skipped; default-terminal applied) | 24, 25 (via `handlerInvocations` exact-set) | | 13 | `follow-cross-origin-continuation` (skips §8 validation, polls the hostile URL) | 26, 27 | | 14 | `collapse-load-error-to-missing` | 28 | -| 15 | `follow-cross-origin-redirect` (follows a 302 to a foreign Location) | 30 — killed by **outcome divergence**: the mutant's redirect-follow happens inside the poll seam call, which the harness cannot instrument, and leads to a divergent end state; PLUS the harness obligation that the fixture's foreign origin is bound to a sentinel listener whose any-request fails the scenario | +| 15 | `follow-cross-origin-redirect` (follows a 302 to a foreign Location) | **not killed at tier 2** — below the poll seam; assigned to Layer 1, whose adapters are still pending, tracked in #819. Fixture 30 pins a different fault class above the seam. See the note under this table. | +| 16 | `discard-live-id-at-or-below-served-id` (streaming lane orders live ids against the highest poll-served id) | 31 — and 31 alone: verified to pass all of 01–30, because every other straggler either arrives with nothing yet served (20) or is buffered pre-cut (01, 12, 19) | + +**Row 15 is not killed at tier 2, and the reason is structural.** In tier 2 the +poll lane is a SEAM. The driver receives the fixture's scripted 302, reduces +the `Location` to its origin with `CanonicalOrigin`, and hands the connector a +`PollRedirectRefused` verdict carrying that origin and a generic cause. The +connector never sees a `Location` header and never decides whether to follow +one, so `follow-cross-origin-redirect` is not merely hard to observe here — it +is **unreachable**, and an unreachable mutant is not a partial kill. + +What fixture 30 does pin is a different fault class, above the seam: given a +`PollRedirectRefused` verdict, a connector must classify it as +Terminal(`invalid_continuation`) and must not retry it. Its `finally` makes +both fail loudly — the reason is asserted exactly, and `mintCount: 1` / +`connectCount: 1` / `timers: {}` / `socket: closed` leave no room for a retry, +a reconnect, or a lingering timer. + +**Redaction is not among them**, and an earlier revision of this note said it +was. The driver performs the redaction itself, before the connector runs: no +path or query text from the `Location` ever reaches the connector, so a +connector that echoed its entire input verbatim would pass fixture 30 +unchanged. Claiming it here would have been a kill that cannot fail. That proof +belongs to `TestRedirectRefusalRendersNoServerValue`, which feeds a +secret-bearing cause and asserts the terminal's whole rendering and cause chain +never carry it — a test that exists today — and, for the real-adapter path, to +Layer 1 once its adapters land. + +An earlier revision of this row claimed a harness obligation to "bind the +foreign origin to a sentinel listener whose any-request fails the scenario". +That is withdrawn. No implementation met it, and meeting it would prove +nothing: the foreign origin is unreachable **by construction of the harness**, +because the harness is the seam, so a silent sentinel is a statement about the +driver rather than about the connector. Zero egress to a foreign redirect +target is a Layer-1 property, and proving it is ASSIGNED to the Layer-1 seam +adapter's own 302 test, where a real generated `PollEvents` call will meet a +real redirect. Those adapters have not landed — `go/pkg/basecamp/eventfeed/doc.go` +lists them among the pieces still to come — so this is a recorded obligation, +not a proof the repository contains today. Tracked in #819. Auto-continue-past-unhandled-gap needs no separate mutation — fixture 23's exact-set `finally` is its direct test. Fixture 29's exact store-call script is the diff --git a/conformance/event-feed/fixtures/05-fresh-ticket-reconnect-after-ttl.json b/conformance/event-feed/fixtures/05-fresh-ticket-reconnect-after-ttl.json index 6255dc1302..f35b8f3fea 100644 --- a/conformance/event-feed/fixtures/05-fresh-ticket-reconnect-after-ttl.json +++ b/conformance/event-feed/fixtures/05-fresh-ticket-reconnect-after-ttl.json @@ -63,6 +63,14 @@ "is": "streaming" } }, + { + "expectTimers": { + "exact": { + "staleness": 1, + "repair-poll": 1 + } + } + }, { "advance": { "ms": 121000 diff --git a/conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json b/conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json index aa7abaf3ee..317744fe00 100644 --- a/conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json +++ b/conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json @@ -1,7 +1,6 @@ { "name": "30-continuation-redirect-cross-origin", - "description": "A VALIDATED same-origin `next` answers 302 with a cross-origin Location: the poll seam suppresses automatic redirect-following, and the foreign Location is Terminal(invalid_continuation). {{NEXT:1}} substitutes same-origin, so the pre-poll validation PASSES and the second poll seam call is made (contrast fixture 26, where no request reaches the URL at all) — the redirect answer is where the per-hop rule bites. What tier 2 verifies is the per-hop refusal DECISION (the driver classifies the scripted Location through the shipped predicate) and the loop's response: the continuation terminal, the socket explicitly closed, and no further seam calls. That the adapter's HTTP client makes zero requests to the refused URL is below the poll seam and owned by Layer-1 adapter conformance; the Location host is literal, never substituted, and never served. Kills follow-cross-origin-redirect.", - "config": { +"description": "A VALIDATED same-origin `next` answers 302 with a cross-origin Location: the poll seam suppresses automatic redirect-following, and the foreign Location is Terminal(invalid_continuation). {{NEXT:1}} substitutes same-origin, so the pre-poll validation PASSES and the second poll seam call is made (contrast fixture 26, where no request reaches the URL at all) \u2014 the redirect answer is where the per-hop rule bites. What tier 2 verifies is the per-hop refusal DECISION \u2014 the driver classifies the scripted Location through the SHIPPED predicate (checkContinuation, via a test export), never an ad-hoc re-implementation \u2014 and the loop's response: the continuation terminal, the socket explicitly closed, and no further seam calls. That the adapter's HTTP client makes zero requests to the refused URL is below the poll seam and owned by Layer-1 adapter conformance \u2014 a recorded obligation, not an existing proof: those adapters are still pending (tracked in #819), so follow-cross-origin-redirect itself remains NOT killed at tier 2 (row 15). The Location host is literal, never substituted, and never served. Redaction is deliberately not claimed here: the driver reduces the Location to its origin best-effort (an unreducible Location carries none) before the connector runs, so no path or query text exists for a connector to over-echo; TestRedirectRefusalRendersNoServerValue owns that proof. See the row-15 note in README.md.", "config": { "position": "{{POS:0}}" }, "steps": [ diff --git a/conformance/event-feed/fixtures/31-post-snapshot-straggler-below-served-id.json b/conformance/event-feed/fixtures/31-post-snapshot-straggler-below-served-id.json new file mode 100644 index 0000000000..3a98bc7b12 --- /dev/null +++ b/conformance/event-feed/fixtures/31-post-snapshot-straggler-below-served-id.json @@ -0,0 +1,176 @@ +{ + "name": "31-post-snapshot-straggler-below-served-id", + "description": "The dedupe rule's other axis, and the one no other fixture covers. Fixture 20 proves a post-snapshot straggler is delivered when the entry page served NO events \u2014 which a highest-served-id implementation also passes, because with nothing served the mark is unset and every id clears it. Here the entry page serves id 99 and the straggler arriving afterwards is 41, BELOW it. It must still be delivered: dedupe tracks actually-delivered event ids, and 41 was never served by poll, so no ordering over ids or positions may suppress it. Re-pushing 99 IS suppressed, by id, which keeps this a dedupe fixture rather than a no-dedupe one. Kills discard-live-id-at-or-below-served-id in the streaming lane, which was verified to pass all of 01-30.", + "steps": [ + { + "expectMint": { + "respond": { + "status": 200, + "body": { + "ticket": "{{TICKET:1}}", + "expires_in": 120, + "url": "{{CABLE_URL:1}}" + } + } + } + }, + { + "expectConnect": { + "url": "{{CABLE_URL:1}}" + } + }, + { + "serve": { + "frame": "welcome" + } + }, + { + "expectSubscribe": { + "channel": "EventsChannel" + } + }, + { + "serve": { + "frame": "confirm" + } + }, + { + "expectPoll": { + "query": { + "exact": {} + }, + "respond": { + "status": 200, + "body": { + "events": [ + { + "id": 99, + "kind": "message", + "event_type": "message.created", + "action": "created", + "created_at": "2026-08-01T12:00:00Z", + "bucket_id": 2, + "creator_id": 3, + "performed_by_id": null, + "recording_id": 900 + } + ], + "position": "{{POS:1}}" + } + } + } + }, + { + "expectDelivered": { + "exact": [ + 99 + ] + } + }, + { + "expectCheckpoint": { + "position": "{{POS:1}}" + } + }, + { + "expectState": { + "is": "streaming" + } + }, + { + "serve": { + "frame": "message", + "event": { + "id": 41, + "kind": "message", + "event_type": "message.created", + "action": "created", + "created_at": "2026-08-01T12:00:00Z", + "bucket_id": 2, + "creator_id": 3, + "performed_by_id": null, + "actor_type": "person", + "recording_id": 900, + "visible_to_clients": false + } + } + }, + { + "expectDelivered": { + "exact": [ + 99, + 41 + ] + } + }, + { + "serve": { + "frame": "message", + "event": { + "id": 99, + "kind": "message", + "event_type": "message.created", + "action": "created", + "created_at": "2026-08-01T12:00:00Z", + "bucket_id": 2, + "creator_id": 3, + "performed_by_id": null, + "actor_type": "person", + "recording_id": 900, + "visible_to_clients": false + } + } + }, + { + "serve": { + "frame": "message", + "event": { + "id": 42, + "kind": "message", + "event_type": "message.created", + "action": "created", + "created_at": "2026-08-01T12:00:00Z", + "bucket_id": 2, + "creator_id": 3, + "performed_by_id": null, + "actor_type": "person", + "recording_id": 900, + "visible_to_clients": false + } + } + }, + { + "expectDelivered": { + "exact": [ + 99, + 41, + 42 + ] + } + } + ], + "finally": { + "state": "streaming", + "mintCount": 1, + "connectCount": 1, + "delivered": { + "exact": [ + 99, + 41, + 42 + ] + }, + "checkpoints": { + "exact": [ + "{{POS:1}}" + ] + }, + "timers": { + "exact": { + "staleness": 1, + "repair-poll": 1 + } + }, + "socket": "open" + } +} diff --git a/conformance/event-feed/schema.json b/conformance/event-feed/schema.json index ddd90eb873..d2b473e068 100644 --- a/conformance/event-feed/schema.json +++ b/conformance/event-feed/schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://basecamp.com/schemas/event-feed-scenario.json", "title": "Event Feed Connector Tier-2 Scenario", - "description": "One strictly-ordered interleaved script driving the SPEC §23 connector over its five seams: HTTP exchanges (mint/poll), cable frames, time directives, and observations, in one `steps` array. Strict-matched actions (mint, connect, poll, outbound frame, client close, checkpoint save) must each match their expect step under the per-action-class rules in this family's README ('Strictness semantics, per action class': saves and outbound frames are arrival-strict; mint/poll seam calls are parked and matched in order); observation directives (delivered, buffered, timers, state, signals, handler invocations) are rendezvous assertions evaluated cumulatively under a small wall-clock watchdog while virtual time is frozen. All counts (mintCount, connectCount) count SEAM CALLS — one fully-governed generated call each, with SPEC §7 retries inside — never wire attempts. Time flows only through the injected Clock seam; the virtual-advance algorithm and the ownership-cut/dequeued-frames semantics are normative prose in this family's README and SPEC §23. Validated by `make event-feed-fixtures-check`.", + "description": "One strictly-ordered interleaved script driving the SPEC §23 connector over its five seams. NUMERIC LITERALS: schema keywords judge mathematical value (1e3 and 1000.0 are integer instances) and drivers judge integrality and range on the literal's exact value — but a driver MAY refuse any numeric literal longer than 100000 characters regardless of value: a sanctioned resource bound on pathological spellings, not a value constraint. Every integer field carries an explicit, portable ceiling — 9223372036854775807 for ids and epoch positions (§10's 64-bit contract), 2147483647 for counts, capacities and seconds (the narrowest native integer any driver language carries), 65535 for a close code — so a schema-valid fixture is never one a driver must refuse at its own integer width. Continuing: HTTP exchanges (mint/poll), cable frames, time directives, and observations, in one `steps` array. Strict-matched actions (mint, connect, poll, outbound frame, client close, checkpoint save) must each match their expect step under the per-action-class rules in this family's README ('Strictness semantics, per action class': saves and outbound frames are arrival-strict; mint/poll seam calls are parked and matched in order); observation directives (delivered, buffered, timers, state, signals, handler invocations) are rendezvous assertions evaluated cumulatively under a small wall-clock watchdog while virtual time is frozen. All counts (mintCount, connectCount) count SEAM CALLS — one fully-governed generated call each, with SPEC §7 retries inside — never wire attempts. Time flows only through the injected Clock seam; the virtual-advance algorithm and the ownership-cut/dequeued-frames semantics are normative prose in this family's README and SPEC §23. Validated by `make event-feed-fixtures-check`.", "type": "object", "additionalProperties": false, "required": [ @@ -261,7 +261,8 @@ "maxItems": 100, "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "description": "Filter: bucket ids, ≤ 100, positive." }, @@ -271,7 +272,8 @@ "maxItems": 100, "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "description": "Filter: creator ids, ≤ 100, positive." }, @@ -281,7 +283,8 @@ "maxItems": 100, "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "description": "Filter: effective-performer ids, ≤ 100, positive (the server's `self` literal is caller-resolved to an id — SPEC §23 Consumer Surface)." }, @@ -291,7 +294,8 @@ "maxItems": 100, "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "description": "Filter: excluded effective-performer ids, ≤ 100, positive — the loop guard for an acting agent." }, @@ -314,37 +318,44 @@ "confirmationDeadlineMs": { "type": "integer", "minimum": 1, + "maximum": 315576000000, "description": "Default 10000." }, "repairPollBaseMs": { "type": "integer", "minimum": 1, + "maximum": 315576000000, "description": "Repair interval base. Default 60000, ±20% jitter per cycle. Override large to keep repair-poll out of an advance window." }, "backoffBaseMs": { "type": "integer", "minimum": 1, + "maximum": 315576000000, "description": "Reconnect/poll-retry full-jitter base. Default 1000." }, "backoffCapMs": { "type": "integer", "minimum": 1, + "maximum": 315576000000, "description": "Local jitter-draw cap. Default 60000. Server-directed Retry-After is exempt per §7." }, "stalenessMs": { "type": "integer", "minimum": 1, + "maximum": 315576000000, "description": "Default 7500. Override large when a scenario advances virtual time without scripting frames." }, "liveBufferCapacity": { "type": "integer", "minimum": 1, - "description": "Default 10000 events. Overflow fixtures set a small value so overflow is reachable in a handful of frames." + "description": "Default 10000 events. Overflow fixtures set a small value so overflow is reachable in a handful of frames. Capped at 1,000,000 — a resource ceiling, since a driver may allocate the capacity eagerly.", + "maximum": 1000000 }, "dedupeCapacity": { "type": "integer", "minimum": 1, - "description": "Default 10000 delivered ids. Deliberately decoupled from liveBufferCapacity." + "description": "Default 10000 delivered ids. Deliberately decoupled from liveBufferCapacity. Capped at 1,000,000 — a resource ceiling, since a driver may allocate the capacity eagerly.", + "maximum": 1000000 }, "signalDisposition": { "type": "object", @@ -549,7 +560,8 @@ "expires_in": { "type": "integer", "minimum": 1, - "description": "Server-owned (~120); NEVER used for client scheduling." + "description": "Server-owned (~120); NEVER used for client scheduling.", + "maximum": 2147483647 }, "url": { "type": "string", @@ -677,7 +689,9 @@ }, "message": { "type": "integer", - "description": "Optional epoch payload — both wire ping forms are legal." + "description": "Optional epoch payload — both wire ping forms are legal. A non-negative 64-bit integer, decoded at that width by every driver.", + "maximum": 9223372036854775807, + "minimum": 0 } } }, @@ -782,7 +796,24 @@ "description": "Server-initiated WebSocket close (with a close frame; contrast `sever`).", "properties": { "code": { - "type": "integer" + "type": "integer", + "minimum": 1000, + "maximum": 4999, + "anyOf": [ + { + "minimum": 1000, + "maximum": 1003 + }, + { + "minimum": 1007, + "maximum": 1014 + }, + { + "minimum": 3000, + "maximum": 4999 + } + ], + "description": "A WebSocket close status a server can actually send in a close frame, per the IANA WebSocket Close Code Number registry: the standard 1000–1003 and 1007–1014 (RFC 6455 §7.4.1 plus the registered 1012 Service Restart, 1013 Try Again Later, 1014 Bad Gateway), or the registered/private 3000–4999 range. The reserved 1004–1006 and 1015 are never sent in a frame, and 1016–2999 are unassigned. Decoded at a fixed 64-bit width by every driver." }, "reason": { "type": "string" @@ -989,7 +1020,7 @@ "status", "headers" ], - "description": "Redirect on a continuation (fixture 30): the seam suppresses automatic following; a cross-origin/downgraded Location is Terminal(invalid_continuation) with zero egress. Harness obligation: the fixture's foreign origin is bound to a sentinel listener — any request reaching it fails the scenario.", + "description": "Redirect on a continuation (fixture 30): the seam suppresses automatic following; a cross-origin/downgraded Location is Terminal(invalid_continuation). At tier 2 the poll lane is a SEAM, so the driver forms the redirect-refused verdict and the connector never sees a Location header: this pins the fault class above the seam (mishandling the verdict — retrying it or misclassifying it), not redirect-following itself, which lives below the seam, is unreachable at tier 2, and is ASSIGNED to the Layer-1 adapter's 302 test — a recorded obligation, not an existing proof: those adapters are still pending (tracked in #819). Redaction is not pinned here: the driver reduces the Location to its origin before the connector runs, so no path or query text reaches it. No sentinel-listener obligation is imposed on the harness: the foreign origin is unreachable by construction of the harness, so a silent sentinel would be a statement about the driver rather than about the connector. See the row-15 note in README.md.", "properties": { "status": { "const": 302 @@ -1089,7 +1120,8 @@ }, "epoch_after_id": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 9223372036854775807 }, "resume": { "type": "string", @@ -1190,7 +1222,8 @@ "properties": { "id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "kind": { "type": "string", @@ -1210,22 +1243,26 @@ }, "bucket_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "creator_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "performed_by_id": { "type": [ "integer", "null" ], - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "recording_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "details": { "type": "object", @@ -1253,7 +1290,8 @@ "properties": { "id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "kind": { "type": "string", @@ -1273,18 +1311,21 @@ }, "bucket_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "creator_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "performed_by_id": { "type": [ "integer", "null" ], - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "actor_type": { "enum": [ @@ -1294,7 +1335,8 @@ }, "recording_id": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 }, "visible_to_clients": { "type": "boolean" @@ -1322,11 +1364,13 @@ "required": [ "ms" ], - "description": "Advance virtual now by ms, firing due timers in deadline order per the normative virtual-advance algorithm (README): re-evaluate after each fire; timers scheduled during the advance whose deadlines land inside the window also fire; ties break by creation order.", + "description": "Advance virtual now by ms. The normative virtual-advance algorithm (README) fires due timers in deadline order, re-evaluating after each fire, with timers scheduled during the advance whose deadlines land inside the window also firing and ties breaking by creation order. That reentrant clause is UNSCRIPTABLE wherever the connector runs concurrently with the driver, so no fixture may rely on it: every driver MUST REJECT an advance whose window would fire ANY timer, naming fireTimer as the deterministic alternative. The test is what would FIRE, decided from the clock's state before time moves — not what gets armed, which happens on the connector's schedule and can only be sampled. An advance that fires nothing never wakes anything, so it cannot cause an arm; that is what makes the check complete rather than probabilistic. A firing that replaces nothing is rejected too. And an advance is deterministic only from a scripted rendezvous point: an action's completion can precede the timer arms its transition causes, so every advance must be the scenario's first step or immediately follow a TWO-STEP rendezvous: expectState, then expectTimers, enforced at load (an empty expectTimers set is rejected with it — it orders nothing). Neither step alone settles: a set match can coincide with a transient mid-surgery set (the welcome transition stops handshake-deadline and arms confirmation-deadline in separate clock acquisitions), and an announcement can precede a tail arm. Together they do: the announcement bounds the surgery, and any timer still unarmed at the announcement is exactly what the exact-set match then waits for. Both block under the watchdog, so wrong authorship fails on every schedule where the stale pair no longer holds — a pair naming the PRE-action state can pass on the schedule where the action is not yet processed, so a wrong script is at worst flaky, never stably green; the settled guarantee belongs to correctly authored pairs. A transition that announces no state change — or only rearms a timer of the same kind and count — is invisible to this rendezvous, and a script that would advance behind one uses fireTimer instead.", "properties": { "ms": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 315576000000, + "description": "Every ms field in this schema shares this maximum: 10 virtual years, a DOMAIN bound (scripts age tickets by minutes to days) chosen over the representation-derived 9223372036854 because the schema states what a script can mean, not one language's integer layout. It sits ~29× under the int64-nanosecond overflow line, so no conforming driver's duration representation can overflow — one past that line, a naive ms-to-duration multiply goes negative and an accepted advance would silently REWIND virtual time. Drivers enforce the range at load." } } }, @@ -1352,11 +1396,13 @@ "properties": { "min": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 315576000000 }, "max": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 315576000000 } } } @@ -1388,7 +1434,8 @@ }, "additionalProperties": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 2147483647 } } } @@ -1484,7 +1531,8 @@ "properties": { "epochAfterId": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 9223372036854775807 }, "resumeUrl": { "type": "string", @@ -1503,7 +1551,8 @@ "properties": { "count": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 2147483647 } } }, @@ -1527,13 +1576,15 @@ "minItems": 1, "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 } }, "droppedCount": { "type": "integer", "minimum": 1, - "description": "The harness asserts droppedCount == droppedIds.length at load — a disagreement fails the fixture, never passes silently." + "description": "The harness asserts droppedCount == droppedIds.length at load — a disagreement fails the fixture, never passes silently.", + "maximum": 2147483647 } } }, @@ -1550,7 +1601,8 @@ }, "epochAfterId": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 9223372036854775807 }, "resumeUrl": { "type": "string", @@ -1573,7 +1625,8 @@ "type": "array", "items": { "type": "integer", - "minimum": 1 + "minimum": 1, + "maximum": 9223372036854775807 } } } @@ -1643,16 +1696,19 @@ }, "mintCount": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 2147483647 }, "connectCount": { "type": "integer", - "minimum": 0 + "minimum": 0, + "maximum": 2147483647 }, "pollCount": { "type": "integer", "minimum": 0, - "description": "Poll SEAM calls — one fully-governed generated PollEvents call each, with SPEC §7 retries inside — never wire attempts." + "description": "Poll SEAM calls — one fully-governed generated PollEvents call each, with SPEC §7 retries inside — never wire attempts.", + "maximum": 2147483647 }, "timers": { "$ref": "#/$defs/timerSet" diff --git a/go/pkg/basecamp/eventfeed/buffer_test.go b/go/pkg/basecamp/eventfeed/buffer_test.go index 62fc0d1792..25124bea87 100644 --- a/go/pkg/basecamp/eventfeed/buffer_test.go +++ b/go/pkg/basecamp/eventfeed/buffer_test.go @@ -6,8 +6,9 @@ import ( ) // TestLiveBufferAddClearsEvictedSlots pins the eviction half of the live -// buffer's memory ceiling. SPEC.md §23 publishes the connector's worst case -// as (pump depth + EVENT_FEED_LIVE_BUFFER_CAPACITY) × EVENT_FEED_MAX_FRAME_BYTES; +// buffer's memory ceiling. SPEC.md §23 publishes the cable lane's worst case +// as (pump depth + 3 + EVENT_FEED_LIVE_BUFFER_CAPACITY) × EVENT_FEED_MAX_FRAME_BYTES +// retained, plus one frame's transient decode allocation; // a reslice alone removes the evicted event LOGICALLY while the slice that // results still points into the same backing array, whose prefix keeps that // event's strings reachable until a later reallocation. Under sustained diff --git a/go/pkg/basecamp/eventfeed/catchup.go b/go/pkg/basecamp/eventfeed/catchup.go index 1d4565333d..2fe3416485 100644 --- a/go/pkg/basecamp/eventfeed/catchup.go +++ b/go/pkg/basecamp/eventfeed/catchup.go @@ -916,7 +916,7 @@ func (l *loop) drain(at *attempt) (cycleOutcome, bool) { // one. Both halves matter. Scanning first is what keeps the // protocol-fatal carve-out ahead of every delivery; dequeuing singly is // what keeps the drain inside the live buffer's capacity, which is a - // bound on events held AT ONCE (SPEC.md §23 sizes the connector's whole + // bound on events held AT ONCE (SPEC.md §23 sizes the cable lane's whole // memory ceiling off it). Taking the buffer's whole contents into a batch // instead let the buffer read as empty while `capacity` events were still // pending in that batch, so the scan could admit another full capacity @@ -1116,9 +1116,17 @@ func (l *loop) probeFatal(at *attempt) (cycleOutcome, bool) { // is actually reported. // // This is why the scan needs no queue and no share of pumpDepth: it retains -// exactly what the single slot always retained. The connector's published -// memory bound — (pump depth + liveBufferCapacity) × MAX_FRAME_BYTES — is -// untouched, and so is the depth at which the pump blocks. +// exactly what the single slot always retained. The published RETAINED bound +// — (pump depth + 3 + liveBufferCapacity) × MAX_FRAME_BYTES — is untouched, +// and so is the depth at which the pump blocks. The slot IS one of that +// formula's frame-sized terms beyond the queue; the others are the frame the +// pump has read and not yet handed off, and the frame the scan itself has +// dequeued and not yet disposed of — the very receive that lets a blocked +// pump refill the queue behind it. All are retained WHILE the queue is full, +// which is why they are addends and not alternatives. Decode-time TRANSIENTS +// (the representation chain plus map/key overhead) ride per frame on top and +// are deliberately not in the formula — SPEC §23 splits the accounting: +// peak = retained + one frame's transient. func (l *loop) deferForDrain(d *deferredFrame) { if l.deferred == nil { l.deferred = d diff --git a/go/pkg/basecamp/eventfeed/catchup_test.go b/go/pkg/basecamp/eventfeed/catchup_test.go index 789b40fee0..f467666877 100644 --- a/go/pkg/basecamp/eventfeed/catchup_test.go +++ b/go/pkg/basecamp/eventfeed/catchup_test.go @@ -1395,8 +1395,8 @@ func TestDrainScanAdmissionIsNotStranded(t *testing.T) { } // TestDrainHoldsNoMoreThanTheLiveBufferCapacity: the live buffer's capacity -// is a bound on events HELD AT ONCE — SPEC §23 sizes the connector's whole -// memory ceiling off it, "(pump depth + EVENT_FEED_LIVE_BUFFER_CAPACITY) × +// is a bound on events HELD AT ONCE — SPEC §23 sizes the cable lane's whole +// RETAINED ceiling off it, "(pump depth + 3 + EVENT_FEED_LIVE_BUFFER_CAPACITY) × // EVENT_FEED_MAX_FRAME_BYTES" — so a drain must not be able to hold a batch // outside the buffer while the buffer refills to capacity behind it. // diff --git a/go/pkg/basecamp/eventfeed/feedtest/clock.go b/go/pkg/basecamp/eventfeed/feedtest/clock.go index 48bf72c9a0..3be277e5db 100644 --- a/go/pkg/basecamp/eventfeed/feedtest/clock.go +++ b/go/pkg/basecamp/eventfeed/feedtest/clock.go @@ -66,6 +66,75 @@ func (c *Clock) NewTimer(d time.Duration, name string) eventfeed.Timer { return t } +// DueWithin returns the names of live timers due within d of the current +// virtual time — the INITIAL set an Advance(d) would fire, read from the +// clock's present state — in creation order. It is not a complete firing +// prediction: Advance re-evaluates after each fire, so a timer armed +// reentrantly by a firing's recipient can fire inside the same window without +// ever appearing here (TestClock_AdvanceFiresATimerArmedByAFiringsRecipient +// shows one). The asymmetry is what makes the empty answer exact and the +// non-empty answer a floor. +// +// Read under the same lock advance selects under and NewTimer arms under, so +// the answer is atomic with respect to both. That is what lets a caller turn a +// racy question into a decidable one: an EMPTY result means the advance fires +// nothing, and advance never unlocks unless it fires something, so no +// recipient can be woken by it and no timer it could arm can land inside its +// window. A non-empty result means the script is asking for a firing whose +// aftermath races the re-selection, which is the thing no cross-language +// fixture can mean the same way twice. +func (c *Clock) DueWithin(d time.Duration) []string { + c.mu.Lock() + defer c.mu.Unlock() + return c.dueWithinLocked(c.now.Add(d)) +} + +// dueWithinLocked returns the names of live timers due at or before target, +// in creation order. The caller holds c.mu. +func (c *Clock) dueWithinLocked(target time.Time) []string { + var names []string + for _, t := range c.live { + if !t.deadline.After(target) { + names = append(names, t.name) + } + } + return names +} + +// AdvanceIfQuiet advances virtual time by d only if the window would fire +// nothing; otherwise it reports the INITIALLY due set — the same floor +// DueWithin reads, sufficient here because rejection needs only "non-empty", +// and an empty initial set means nothing fires at all — and leaves the clock +// untouched. +// It is DueWithin and Advance as ONE critical section, for the driver MUST +// in SPEC §23: an advance whose window would fire any timer is rejected. +// Deciding that with two separate lock acquisitions leaves a gap — a timer +// armed (or stopped) between the check and the movement changes what the +// accepted directive does, so an advance the guard accepted could fire. +// Under one hold of the clock's locks, an accepted advance provably fires +// nothing. +// +// What stays undecidable, stated honestly: whether a CONCURRENT arm lands +// before or after this critical section is still the arming goroutine's +// schedule — no clock operation can order another goroutine's lock +// acquisition. The invariant restored here is the decidable one: whichever +// side the arm lands, an ACCEPTED advance fired nothing, and an arm that +// lost the race is due at its own deadline, unfired and unharmed. +func (c *Clock) AdvanceIfQuiet(d time.Duration) ([]string, bool) { + c.advancing.Lock() + defer c.advancing.Unlock() + c.mu.Lock() + target := c.now.Add(d) + if due := c.dueWithinLocked(target); len(due) > 0 { + c.mu.Unlock() + return due, false + } + c.now = target + c.mu.Unlock() + c.cond.Broadcast() + return nil, true +} + // Outstanding returns the names of live (unfired, unstopped) timers, in // creation order. func (c *Clock) Outstanding() []string { diff --git a/go/pkg/basecamp/eventfeed/feedtest/clock_test.go b/go/pkg/basecamp/eventfeed/feedtest/clock_test.go index 5df3886988..4695df3ff6 100644 --- a/go/pkg/basecamp/eventfeed/feedtest/clock_test.go +++ b/go/pkg/basecamp/eventfeed/feedtest/clock_test.go @@ -121,6 +121,64 @@ func TestClock_AdvanceFiresATimerArmedByAFiringsRecipient(t *testing.T) { } } +// The reject half of the one-critical-section contract: a window that would +// fire is refused with the due set named, and the clock is untouched — time +// has not moved and the refused timer is still outstanding and firable. +func TestClock_AdvanceIfQuietRejectsAFiringWindowUntouched(t *testing.T) { + c := NewClock() + base := c.Now() + tm := c.NewTimer(5*time.Millisecond, "backoff") + c.NewTimer(30*time.Millisecond, "repair-poll") + + due, ok := c.AdvanceIfQuiet(10 * time.Millisecond) + + if ok { + t.Fatal("AdvanceIfQuiet accepted a window that would fire backoff") + } + if len(due) != 1 || due[0] != "backoff" { + t.Errorf("due = %v, want [backoff]", due) + } + if got := c.Now(); !got.Equal(base) { + t.Errorf("Now() = %v, want unmoved %v", got, base) + } + select { + case at := <-tm.C(): + t.Errorf("rejected advance fired the timer at %v", at) + default: + } + if got := c.Outstanding(); len(got) != 2 { + t.Errorf("Outstanding() = %v, want both timers still live", got) + } + // The refused timer is unharmed: it still fires on its own terms. + if delay, found := c.FireTimer("backoff"); !found || delay != 5*time.Millisecond { + t.Errorf("FireTimer(backoff) = (%v, %v), want (5ms, true)", delay, found) + } +} + +// The accept half: a genuinely quiet window moves time by exactly d and +// leaves undue timers outstanding. A deadline landing exactly ON the window +// edge counts as due — the same !After(target) boundary Advance fires at. +func TestClock_AdvanceIfQuietMovesTimeOnAQuietWindow(t *testing.T) { + c := NewClock() + base := c.Now() + c.NewTimer(30*time.Millisecond, "repair-poll") + + if due, ok := c.AdvanceIfQuiet(10 * time.Millisecond); !ok { + t.Fatalf("quiet window rejected: due = %v", due) + } + if got := c.Now(); !got.Equal(base.Add(10 * time.Millisecond)) { + t.Errorf("Now() = %v, want %v", got, base.Add(10*time.Millisecond)) + } + if got := c.Outstanding(); len(got) != 1 || got[0] != "repair-poll" { + t.Errorf("Outstanding() = %v, want [repair-poll]", got) + } + + // Exactly-on-the-edge is a firing window, not a quiet one. + if due, ok := c.AdvanceIfQuiet(20 * time.Millisecond); ok || len(due) != 1 || due[0] != "repair-poll" { + t.Errorf("edge deadline: AdvanceIfQuiet = (%v, %v), want ([repair-poll], false)", due, ok) + } +} + func TestClock_StopRemovesTimerAndSuppressesFiring(t *testing.T) { c := NewClock() timer := c.NewTimer(5*time.Millisecond, "staleness") diff --git a/go/pkg/basecamp/eventfeed/scenario_conformance_test.go b/go/pkg/basecamp/eventfeed/scenario_conformance_test.go index 8d43fb9d47..25291f5455 100644 --- a/go/pkg/basecamp/eventfeed/scenario_conformance_test.go +++ b/go/pkg/basecamp/eventfeed/scenario_conformance_test.go @@ -308,14 +308,7 @@ func (d *driver) runStep(step scenarioStep) error { case *expectClientCloseStep: return d.expectClientClose() case *advanceStep: - // Plain Advance: no fixture scripts a firing that arms a follow-on - // timer due inside the same window — 05, the suite's only advance, - // deliberately configures staleness and repair-poll out of it, so the - // window fires nothing. A script that did want a chained firing would - // pass feedtest.Clock.AdvanceSettling the rendezvous for the arming, - // since the connector arms on its own goroutine. - d.h.clock.Advance(millis(payload.Ms)) - return nil + return d.advance(payload) case *fireTimerStep: return d.fireTimer(payload) case *exactIDs: @@ -409,7 +402,7 @@ func (d *driver) serverClose(step *serverCloseStep) error { if d.peer == nil { return errors.New("no cable connection is open to close") } - return d.h.closePeer(d.peer, step.Code, step.Reason) + return d.h.closePeer(d.peer, int(step.Code), step.Reason) } // sever drops the TCP connection abruptly: no close frame, no disconnect @@ -605,6 +598,54 @@ func (d *driver) nextClientFrame(what string) (clientFrame, error) { // --- time ---------------------------------------------------------------- +// advance runs an `advance` directive under the family's virtual-advance +// algorithm, and REJECTS the one shape of script the algorithm cannot resolve +// identically in every language. +// +// The algorithm says timers armed during a window whose deadlines land inside +// it also fire. In a single-threaded test clock that is exact. In Go the +// connector arms on its own goroutine, so whether such a timer lands before +// the re-selection that would fire it is a scheduling outcome — the same +// script means two things, and a fixture cannot pin which. +// +// # Rejecting the shape, rather than detecting the divergence +// +// This used to detect: sample the clock's arm count, advance, wait out the +// family's wall-clock watchdog for the count to move, and fail if it did. That +// is a heuristic wearing a MUST, and it reads "no arm within five seconds" as +// "no arm" — an arm landing later is simply missed. No wait makes it sound, +// because there is no instant at which "nothing further will be armed" becomes +// knowable from outside. +// +// So the question changes from "did the advance cause an arm?", which is racy, +// to "can this advance fire anything at all?", which is decidable. feedtest's +// clock selects due timers under its own lock and unlocks ONLY across a +// firing's aftermath — deliberately, so a woken recipient can arm inside the +// window. An advance with nothing due therefore never unlocks, never wakes +// anything, and cannot be the cause of any arm. AdvanceIfQuiet decides the +// due set and moves time under one hold of the clock's locks — a check and a +// movement in two acquisitions would leave a gap where a concurrently armed +// timer turns an accepted advance into a firing one. +// +// A script that wants a firing writes `fireTimer`, which fires one named timer +// without advancing the clock and so involves no re-selection at all. The +// suite's only `advance` (fixture 05) sits in Streaming with staleness and +// repair-poll configured to ~11 days against a 121-second window: it exists to +// age a ticket past its TTL, not to fire anything, and it qualifies. +// +// AdvanceSettling remains for a caller that genuinely wants a chained firing +// with an explicit rendezvous. It is deliberately not reachable from a fixture. +func (d *driver) advance(step *advanceStep) error { + if due, ok := d.h.clock.AdvanceIfQuiet(millis(int64(step.Ms))); !ok { + return fmt.Errorf( + "advance of %dms would fire %v: whether a timer armed by one of those firings lands inside the "+ + "same window depends on goroutine scheduling, so this script cannot mean the same thing in "+ + "every language — use fireTimer, which fires one named timer without re-selecting", + step.Ms, due) + } + return nil +} + func (d *driver) fireTimer(step *fireTimerStep) error { if err := d.awaitTimerArmed(step.Kind); err != nil { return err @@ -613,10 +654,10 @@ func (d *driver) fireTimer(step *fireTimerStep) error { if !ok { return fmt.Errorf("no %s timer is outstanding: outstanding %v", step.Kind, d.h.clock.Outstanding()) } - if step.AssertDelayMs == nil { + if !step.AssertDelayMs.set { return nil } - low, high := millis(step.AssertDelayMs.Min), millis(step.AssertDelayMs.Max) + low, high := millis(step.AssertDelayMs.env.Min.v), millis(step.AssertDelayMs.env.Max.v) if delay < low || delay > high { return fmt.Errorf("the %s timer was armed for %s, want [%s, %s]", step.Kind, delay, low, high) } diff --git a/go/pkg/basecamp/eventfeed/scenario_fixture_test.go b/go/pkg/basecamp/eventfeed/scenario_fixture_test.go index 539cf50d8c..65e21ba826 100644 --- a/go/pkg/basecamp/eventfeed/scenario_fixture_test.go +++ b/go/pkg/basecamp/eventfeed/scenario_fixture_test.go @@ -16,6 +16,7 @@ import ( "errors" "fmt" "io" + "math/big" "regexp" "strconv" "strings" @@ -39,24 +40,72 @@ type scenarioStep struct { // scenarioConfig is the schema's `config` object — the connector construction // options a scenario selects. type scenarioConfig struct { - Types []string `json:"types"` - Buckets []int64 `json:"buckets"` - Creators []int64 `json:"creators"` - Performers []int64 `json:"performers"` - ExcludePerformers []int64 `json:"exclude_performers"` - ActorTypes []string `json:"actorTypes"` - Position string `json:"position"` - ConfirmationDeadlineMs int `json:"confirmationDeadlineMs"` - RepairPollBaseMs int `json:"repairPollBaseMs"` - BackoffBaseMs int `json:"backoffBaseMs"` - BackoffCapMs int `json:"backoffCapMs"` - StalenessMs int `json:"stalenessMs"` + Types []string `json:"types"` + Buckets []int64 `json:"buckets"` + Creators []int64 `json:"creators"` + Performers []int64 `json:"performers"` + ExcludePerformers []int64 `json:"exclude_performers"` + ActorTypes []string `json:"actorTypes"` + Position string `json:"position"` + // The five durations are THREE-STATE because presence is meaning twice + // over: a plain int64 read an explicit zero as "absent, use the + // default", and a pointer read an explicit JSON null the same way — + // each accepting a value the schema rejects ("minimum": 1 for zero, + // "type": "integer" for null) and silently substituting another. The + // driver enforces the schema's judgments portably, so all three states + // the wire distinguishes are preserved: absent, null, and value. + ConfirmationDeadlineMs optionalMs `json:"confirmationDeadlineMs"` + RepairPollBaseMs optionalMs `json:"repairPollBaseMs"` + BackoffBaseMs optionalMs `json:"backoffBaseMs"` + BackoffCapMs optionalMs `json:"backoffCapMs"` + StalenessMs optionalMs `json:"stalenessMs"` LiveBufferCapacity int `json:"liveBufferCapacity"` DedupeCapacity int `json:"dedupeCapacity"` SignalDisposition map[string]string `json:"signalDisposition"` CheckpointStore *storeScript `json:"checkpointStore"` } +// optionalMs is one config duration in the three JSON states the schema +// distinguishes: absent (the zero optionalMs — use the default), JSON null +// (set, null — rejected, "type": "integer" refuses it), and a value (set, +// ranged). encoding/json calls a value type's UnmarshalJSON for null where it +// short-circuits a pointer's, which is exactly why this is not a *int64. The +// number itself arrives already judged and rewritten to its integer spelling +// by normalizeNumbers, so only the STRING gate remains here: a quoted "1000" +// is a string instance the schema refuses, though json.Number's own +// Unmarshal would take it. +type optionalMs struct { + set bool + null bool + v int64 +} + +func (o *optionalMs) UnmarshalJSON(data []byte) error { + o.set = true + if string(data) == "null" { + o.null = true + return nil + } + return unmarshalScenarioInt(data, &o.v) +} + +// unmarshalScenarioInt decodes one already-normalized integer literal, +// refusing a string instance in the schema's own terms. +func unmarshalScenarioInt(data []byte, dst *int64) error { + trimmed := strings.TrimLeft(string(data), " \t\r\n") + if strings.HasPrefix(trimmed, `"`) { + return fmt.Errorf("%s is a string: the schema's type is integer — quote-wrapping a number makes it a different instance", trimmed) + } + return json.Unmarshal(data, dst) +} + +// scenarioMs is a required ms value under the same number model. +type scenarioMs int64 + +func (m *scenarioMs) UnmarshalJSON(data []byte) error { + return unmarshalScenarioInt(data, (*int64)(m)) +} + // storeScript is the schema's scripted CheckpointStore. type storeScript struct { Load string `json:"load"` @@ -134,7 +183,7 @@ type connectOutcome struct { type serveStep struct { Frame string `json:"frame"` - Message *int `json:"message"` + Message *int64 `json:"message"` Identifier *string `json:"identifier"` Reason string `json:"reason"` Reconnect *bool `json:"reconnect"` @@ -143,7 +192,7 @@ type serveStep struct { } type serverCloseStep struct { - Code int `json:"code"` + Code int64 `json:"code"` Reason string `json:"reason"` } @@ -195,17 +244,43 @@ type goneBody struct { } type advanceStep struct { - Ms int `json:"ms"` + Ms scenarioMs `json:"ms"` } type fireTimerStep struct { - Kind string `json:"kind"` - AssertDelayMs *delayEnvelope `json:"assertDelayMs"` + Kind string `json:"kind"` + AssertDelayMs optionalEnvelope `json:"assertDelayMs"` } +// delayEnvelope's members are three-state for the same reason the config +// durations are: min and max are schema-REQUIRED integers, and a plain int64 +// read an absent or null member as 0 — inside the allowed range, silently +// converting the authored envelope into a different one. type delayEnvelope struct { - Min int `json:"min"` - Max int `json:"max"` + Min optionalMs `json:"min"` + Max optionalMs `json:"max"` +} + +// optionalEnvelope is assertDelayMs in the three JSON states: absent (no +// assertion — the zero value), null (set, null — refused, the schema's type +// is object), and a value (set, decoded strictly: the outer decoder's +// DisallowUnknownFields does not reach inside a custom unmarshaler, so the +// strictness is re-established here). +type optionalEnvelope struct { + set bool + null bool + env delayEnvelope +} + +func (o *optionalEnvelope) UnmarshalJSON(data []byte) error { + o.set = true + if string(data) == "null" { + o.null = true + return nil + } + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + return dec.Decode(&o.env) } type expectCheckpointStep struct { @@ -253,12 +328,42 @@ type expectFilterConflictStep struct { // --- loading ------------------------------------------------------------- +// maxScenarioMs is the schema's shared `maximum` for every ms field: 10 +// virtual years. It is a DOMAIN bound — scripts age tickets by minutes to +// days, so the largest value today (~11 days) has 300× headroom — chosen over +// the representation-derived 9,223,372,036,854 (the largest ms count whose +// int64-nanosecond product does not overflow) because the schema should say +// what a script can MEAN, not restate one language's integer layout. It sits +// ~29× under that overflow line, so no conforming driver's duration +// representation can overflow — the failure this bound exists to make a +// fixture error rather than a representation accident: one past the int64 +// line, time.Duration(ms)*time.Millisecond goes negative and an accepted +// advance would silently REWIND virtual time. +const maxScenarioMs int64 = 315_576_000_000 + +// checkScenarioMs enforces the schema's [floor, maxScenarioMs] range on one +// ms field at load, so every driver rejects the same values for the same +// stated reason. ms values are int64 END TO END (fixture structs, this check, +// millis): the maximum exceeds MaxInt32, so a platform-width int fails to +// compile on 32-bit (an int64-typed constant alone would instead make +// schema-valid values above MaxInt32 fail decode into int structs). +func checkScenarioMs(what string, v, floor int64) error { + if v < floor || v > maxScenarioMs { + return fmt.Errorf("%s must be in [%d, %d] (10 virtual years): got %d", what, floor, maxScenarioMs, v) + } + return nil +} + // parseScenario decodes one substituted fixture, failing on anything the // driver does not model. func parseScenario(raw []byte, file string) (*scenario, error) { if err := rejectDuplicateKeys(raw); err != nil { return nil, err } + raw, err := normalizeNumbers(raw) + if err != nil { + return nil, err + } top, err := objectKeys(raw) if err != nil { return nil, err @@ -295,6 +400,47 @@ func parseScenario(raw []byte, file string) (*scenario, error) { } sc.Steps = append(sc.Steps, step) } + + // An advance is deterministic only from a scripted rendezvous point, and + // the rendezvous is TWO steps: expectState, then expectTimers. An + // action's completion can precede the timer arms its transition causes — + // expectConnect returns when the dial is recorded, while the handshake + // deadline arms on the connector's goroutine after — and a set match + // ALONE can coincide with a transient mid-surgery set (the welcome + // transition stops handshake-deadline and arms confirmation-deadline in + // separate clock acquisitions, so an authored set can exist in between). + // The state announcement bounds the surgery: expectState blocks until + // the transition announces, and in every announced state any timer still + // unarmed at the announcement is exactly what the following exact-set + // match waits for — so the pair settles where either alone races. Both + // blocks fail on the watchdog on every schedule where the stale pair no + // longer holds; a pair naming the PRE-action state can still pass on the + // schedule where the action is not yet processed, so wrong authorship is + // at worst flaky — never stably green (README "settle semantics"; the + // settled guarantee belongs to correctly authored pairs). + for i, step := range sc.Steps { + if step.Kind != "advance" || i == 0 { + continue + } + if i < 2 || sc.Steps[i-1].Kind != "expectTimers" || sc.Steps[i-2].Kind != "expectState" { + return nil, fmt.Errorf("step %d: an advance must be the scenario's first step or immediately follow "+ + "an expectState + expectTimers rendezvous — an action's completion can precede the timer arms "+ + "its transition causes, and a set match alone can coincide with a transient mid-surgery set; "+ + "the state announcement bounds the surgery and the exact-set match settles what follows it", i+1) + } + // The match must be able to MEAN settled: an empty set can never + // contain an arm of the preceding transition, so it matches before + // that transition is processed (a released failed mint has not armed + // backoff yet) exactly as if the rendezvous were absent. The limit + // this cannot close is stated in the contract: the authored set must + // include an arm of the preceding transition, and a same-kind rearm + // is invisible to set matching — such scripts use fireTimer. + if rv, ok := sc.Steps[i-1].Payload.(*timerSet); ok && len(rv.Exact) == 0 { + return nil, fmt.Errorf("step %d: an empty rendezvous orders nothing — an expectTimers set with no "+ + "timers cannot contain an arm of the preceding transition, so its match cannot prove the "+ + "transition settled; a scenario with nothing yet armed advances as its first step", i+1) + } + } finRaw, ok := top["finally"] if !ok { return nil, fmt.Errorf("fixture is missing its `finally` block") @@ -387,14 +533,37 @@ func decodeDirective(kind string, body json.RawMessage) (any, error) { return &expectClientCloseStep{}, decodeStrict(body, &empty) case "advance": step := &advanceStep{} - return step, decodeStrict(body, step) + if err := decodeStrict(body, step); err != nil { + return nil, err + } + return step, checkScenarioMs("advance ms", int64(step.Ms), 1) case "fireTimer": step := &fireTimerStep{} if err := decodeStrict(body, step); err != nil { return nil, err } - if step.AssertDelayMs != nil && step.AssertDelayMs.Min > step.AssertDelayMs.Max { - return nil, fmt.Errorf("assertDelayMs min %d exceeds max %d", step.AssertDelayMs.Min, step.AssertDelayMs.Max) + if step.AssertDelayMs.set { + if step.AssertDelayMs.null { + return nil, fmt.Errorf("assertDelayMs supplied as JSON null: the schema's type is object and null is not one — omit the key to fire without a delay assertion") + } + env := step.AssertDelayMs.env + for _, m := range []struct { + name string + o optionalMs + }{{"min", env.Min}, {"max", env.Max}} { + if !m.o.set { + return nil, fmt.Errorf("assertDelayMs needs both min and max — the schema requires them, and an absent %s is a different envelope than the one authored", m.name) + } + if m.o.null { + return nil, fmt.Errorf("assertDelayMs %s supplied as JSON null: the schema's type is integer and null is not one", m.name) + } + if err := checkScenarioMs("assertDelayMs "+m.name, m.o.v, 0); err != nil { + return nil, err + } + } + if env.Min.v > env.Max.v { + return nil, fmt.Errorf("assertDelayMs min %d exceeds max %d", env.Min.v, env.Max.v) + } } return step, validateTimerKind(step.Kind) case "expectDelivered": @@ -489,7 +658,27 @@ type ( // --- validation ---------------------------------------------------------- func validateConfig(cfg scenarioConfig) error { - if cfg.BackoffBaseMs != 0 || cfg.BackoffCapMs != 0 { + // Absence means "default"; everything SUPPLIED is judged — a value is + // ranged (explicit zero included) and null is refused outright. + for _, f := range []struct { + name string + o optionalMs + }{ + {"confirmationDeadlineMs", cfg.ConfirmationDeadlineMs}, + {"repairPollBaseMs", cfg.RepairPollBaseMs}, + {"stalenessMs", cfg.StalenessMs}, + } { + switch { + case !f.o.set: + case f.o.null: + return fmt.Errorf("%s supplied as JSON null: the schema's type is integer and null is not one — omit the key for the default", f.name) + default: + if err := checkScenarioMs(f.name, f.o.v, 1); err != nil { + return err + } + } + } + if cfg.BackoffBaseMs.set || cfg.BackoffCapMs.set { return fmt.Errorf("backoffBaseMs/backoffCapMs are not modeled: SPEC §23 pins the Go connector's full-jitter base and cap as constants, with no construction option to override") } for kind, disposition := range cfg.SignalDisposition { @@ -826,6 +1015,224 @@ func rejectDuplicateKeys(raw []byte) error { } } +// normalizeNumbers rewrites every number literal in raw to its integer +// spelling, judging each the way draft 2020-12's "integer" does: by +// MATHEMATICAL value, not spelling — 1000.0, 1e3 and 200.0 are integer +// instances a schema-valid fixture may carry, and the schema's every number +// is an integer. One walk over the whole document, before any typed decode, +// is what makes the schema's all-numeric cross-driver claim true for every +// field at once: a status, an id, a capacity and a duration are all judged +// here, and the typed script then reads plain integer spellings — so no +// field needs its own number type, and adding a field cannot reopen the +// class. Integrality is a fact about the TEXT, which json.Number preserves: +// a float64 detour rounds 1000.00000000000001 to exactly 1000 before any +// check can look, so the literal is judged exactly, with big.Rat. An +// exponent is read as a NUMBER'S text before anything is materialized, so an +// exponent bomb (1e999999999) is refused for its magnitude, never expanded. +// Strings pass through untouched: a quoted "1000" stays the STRING instance +// the schema refuses, for the typed decode to reject. +func normalizeNumbers(raw []byte) ([]byte, error) { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var doc any + if err := dec.Decode(&doc); err != nil { + return nil, err + } + if dec.More() { + return nil, fmt.Errorf("trailing JSON content") + } + if err := integralizeNumbers(doc, "fixture"); err != nil { + return nil, err + } + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if err := enc.Encode(doc); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// opaqueSubtree reports whether member, found under key in its parent +// object, is a subtree the driver forwards or ignores without decoding — +// an event's `details`, or a `respond.body` whose status the poll and mint +// seams answer by status alone (every status but the 200 page and the 409 +// and 410 bodies, whose members the driver does decode). The integer walk +// leaves such a subtree as spelled: the schema types it as an arbitrary +// object, so its numbers are the server's to spell. +func opaqueSubtree(key string, member any, parent map[string]any) bool { + if key == "details" { + return true + } + if key != "body" { + return false + } + if _, isObject := member.(map[string]any); !isObject { + return false + } + status, ok := parent["status"].(json.Number) + if !ok { + // A bodied respond with no status is the 200 page, which the driver + // decodes. + return false + } + // Judged on the status's VALUE, not its spelling: map iteration may + // reach the body before the walk has rewritten a `200.0` beside it, and + // a literal the walk will refuse anyway is not opaque either way. + lit, err := integerSpelling(status.String()) + if err != nil { + return false + } + switch lit { + case "200", "409", "410": + return false + } + return true +} + +// integralizeNumbers rewrites, in place, every json.Number under doc to its +// integer spelling, naming the member path in any refusal. +func integralizeNumbers(doc any, path string) error { + switch v := doc.(type) { + case map[string]any: + for key, member := range v { + if opaqueSubtree(key, member, v) { + // Server-owned or never-decoded: an event's details object, + // and the body of a response the driver forwards as a status + // without reading it. Their numbers are whatever the server + // publishes, judged by no field of this schema, and are left + // exactly as spelled. + continue + } + at := path + "." + key + if n, ok := member.(json.Number); ok { + lit, err := integerSpelling(n.String()) + if err != nil { + return fmt.Errorf("%s: %w", at, err) + } + v[key] = json.Number(lit) + continue + } + if err := integralizeNumbers(member, at); err != nil { + return err + } + } + case []any: + for i, member := range v { + at := fmt.Sprintf("%s[%d]", path, i) + if n, ok := member.(json.Number); ok { + lit, err := integerSpelling(n.String()) + if err != nil { + return fmt.Errorf("%s: %w", at, err) + } + v[i] = json.Number(lit) + continue + } + if err := integralizeNumbers(member, at); err != nil { + return err + } + } + case json.Number: + // A bare top-level number is not a fixture; the typed decode says so. + } + return nil +} + +// integerSpelling answers the plain decimal integer spelling of one JSON +// number literal, or why it has none. A bomb is refused before anything is +// materialized, and by the value's EFFECTIVE magnitude, never the exponent's +// spelling alone — draft 2020-12 constrains the mathematical value, and a +// significand can offset any exponent (1e44-digits × e-41 is exactly 1000). +// Two string judgments suffice, both exact: +// - the most significant nonzero digit's decimal place caps the value: +// above place 19 nothing fits int64, the widest integer any schema +// field decodes into (the ms fields' 10-year range is judged after, on +// the value, since its floor differs by field); +// - a least significant nonzero digit below the units place makes the +// value non-integral outright (decimal digits do not carry), which +// refuses 1e-999999999 without a 10^999999999 denominator. +// +// A length cap comes first so no multi-megabyte literal is ever walked into +// a rational — the schema's top-level description sanctions exactly this +// bound (a resource limit on spellings, not a value constraint), so refusing +// "1" + 100k zeros + e-100000 (mathematically 1) is conformant. +func integerSpelling(lit string) (string, error) { + if len(lit) > 100000 { + return "", fmt.Errorf("a %d-character number is beyond any modeled value (literals are capped at 100000 characters)", len(lit)) + } + mant, expText := lit, "" + if i := strings.IndexAny(lit, "eE"); i >= 0 { + mant = lit[:i] + expText = strings.TrimPrefix(lit[i+1:], "+") + } + digits := strings.TrimPrefix(mant, "-") + point := strings.IndexByte(digits, '.') + intLen := len(digits) + if point >= 0 { + intLen = point + digits = digits[:point] + digits[point+1:] + } + firstNZ, lastNZ := -1, -1 + for i := 0; i < len(digits); i++ { + if digits[i] >= '1' && digits[i] <= '9' { + if firstNZ < 0 { + firstNZ = i + } + lastNZ = i + } + } + if firstNZ < 0 { + // Zero, however spelled (0, 0.000, 0e200001): integral, and decided + // before the exponent is even parsed — an exponent multiplies a + // significand, and this one is zero. + return "0", nil + } + exp := 0 + if expText != "" { + // ParseInt at a FIXED width, not Atoi: int is 32 bits on some + // targets, where an exponent like 1e9223372036854775807 would fail + // as unreadable before reaching the magnitude judgment and change + // the diagnostic by platform. A 64-bit overflow can only mean the + // exponent is beyond the ±200000 bound below, so range errors take + // the bound's own verdict. + e, err := strconv.ParseInt(expText, 10, 64) + if err != nil { + if errors.Is(err, strconv.ErrRange) { + return "", fmt.Errorf("%s is beyond any modeled value", lit) + } + return "", fmt.Errorf("%s is not a number this driver can read", lit) + } + // Bound the exponent before any place arithmetic: at the platform's + // integer extremes, intLen - firstNZ + exp wraps and the magnitude + // judgments below judge garbage. The literal cap above bounds the + // significand at 100000 digits, so no in-range value needs an + // exponent beyond ±200000 to spell. + if e > 200000 || e < -200000 { + return "", fmt.Errorf("%s is beyond any modeled value", lit) + } + exp = int(e) + } + // Digit i occupies decimal place intLen - i + exp (units = 1). + if msd := intLen - firstNZ + exp; msd > 19 { + return "", fmt.Errorf("%s is beyond any modeled value", lit) + } + if lsd := intLen - lastNZ + exp; lsd < 1 { + return "", fmt.Errorf("%s is not an integer: the schema's type is integer — a number whose mathematical value is integral", lit) + } + r, ok := new(big.Rat).SetString(lit) + if !ok { + return "", fmt.Errorf("%s is not a number this driver can read", lit) + } + if !r.IsInt() { + return "", fmt.Errorf("%s is not an integer: the schema's type is integer — a number whose mathematical value is integral", lit) + } + num := r.Num() + if !num.IsInt64() { + return "", fmt.Errorf("%s is beyond any modeled value", lit) + } + return num.String(), nil +} + // decodeStrict decodes exactly one JSON value into dst, rejecting unknown // object keys and trailing content. func decodeStrict(raw json.RawMessage, dst any) error { diff --git a/go/pkg/basecamp/eventfeed/scenario_harness_test.go b/go/pkg/basecamp/eventfeed/scenario_harness_test.go index c153700e84..d7c4809f35 100644 --- a/go/pkg/basecamp/eventfeed/scenario_harness_test.go +++ b/go/pkg/basecamp/eventfeed/scenario_harness_test.go @@ -708,11 +708,11 @@ func (h *scenarioHarness) newConnector(cfg scenarioConfig) (*eventfeed.Connector }, }), } - if cfg.ConfirmationDeadlineMs > 0 { - opts = append(opts, eventfeed.WithConfirmationDeadline(millis(cfg.ConfirmationDeadlineMs))) + if cfg.ConfirmationDeadlineMs.set { + opts = append(opts, eventfeed.WithConfirmationDeadline(millis(cfg.ConfirmationDeadlineMs.v))) } - if cfg.RepairPollBaseMs > 0 { - opts = append(opts, eventfeed.WithRepairInterval(millis(cfg.RepairPollBaseMs))) + if cfg.RepairPollBaseMs.set { + opts = append(opts, eventfeed.WithRepairInterval(millis(cfg.RepairPollBaseMs.v))) } if cfg.LiveBufferCapacity > 0 { opts = append(opts, eventfeed.WithLiveBufferCapacity(cfg.LiveBufferCapacity)) @@ -728,8 +728,8 @@ func (h *scenarioHarness) newConnector(cfg scenarioConfig) (*eventfeed.Connector if err != nil { return nil, err } - if cfg.StalenessMs > 0 { - conn.SetStaleAfter(millis(cfg.StalenessMs)) + if cfg.StalenessMs.set { + conn.SetStaleAfter(millis(cfg.StalenessMs.v)) } conn.OnStateChanged(h.recordState) conn.OnBufferOccupancy(h.recordOccupancy) @@ -778,4 +778,4 @@ func (h *scenarioHarness) signalHandler(dispositions map[string]string) eventfee } } -func millis(ms int) time.Duration { return time.Duration(ms) * time.Millisecond } +func millis(ms int64) time.Duration { return time.Duration(ms) * time.Millisecond } diff --git a/go/pkg/basecamp/eventfeed/scenario_selftest_test.go b/go/pkg/basecamp/eventfeed/scenario_selftest_test.go index 21575083b3..4a7c2cda86 100644 --- a/go/pkg/basecamp/eventfeed/scenario_selftest_test.go +++ b/go/pkg/basecamp/eventfeed/scenario_selftest_test.go @@ -17,6 +17,7 @@ import ( "net/http" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -81,7 +82,7 @@ func TestScenarioDriverRejectsMutatedFixtures(t *testing.T) { { name: "reconnect dials the previous mint's url", fixture: "05-fresh-ticket-reconnect-after-ttl.json", - path: "steps.13.expectConnect.url", + path: "steps.14.expectConnect.url", value: "{{CABLE_URL:1}}", wants: "a cable dial", }, @@ -147,6 +148,25 @@ func TestScenarioDriverRejectsMutatedFixtures(t *testing.T) { // witness is retained because the lookahead that computes the current step errs // deliberately permissive (see stepSatisfiedLocked), and it is the check that // still fires if a step's arm is ever loosened. +// TestNormalizeNumbersLeavesDetailsAlone: an event's details object is +// forwarded verbatim and typed by the schema as an arbitrary object, so a +// fraction or an integer past int64 inside it is schema-valid fixture data +// the integer normalization must not judge — every other number in the +// document is an integer field of the schema and is. +func TestNormalizeNumbersLeavesDetailsAlone(t *testing.T) { + raw := []byte(`{"id":1e2,"details":{"ratio":0.5,"big":92233720368547758070,"nested":{"n":1.25}},"count":2.0,"respond":{"status":500,"body":{"retry_factor":0.5}},"page":{"status":200.0,"body":{"position":"p","events":[{"id":1e1}]}}}`) + out, err := normalizeNumbers(raw) + if err != nil { + t.Fatalf("normalizeNumbers: %v", err) + } + got := string(out) + for _, want := range []string{`"ratio":0.5`, `"big":92233720368547758070`, `"n":1.25`, `"id":100`, `"count":2`, `"retry_factor":0.5`, `"id":10`} { + if !strings.Contains(got, want) { + t.Errorf("normalized document %s lacks %s", got, want) + } + } +} + func TestScenarioDriverEnforcesDeliveryBeforeCheckpoint(t *testing.T) { const fixture = "02-confirmation-gating.json" control := readFixture(t, fixture) @@ -442,6 +462,145 @@ func TestScenarioDriverRejectsUnmodelledScripts(t *testing.T) { script: `{"name":"x","description":"d","steps":[{"fireTimer":{"kind":"backoff","assertDelayMs":{"min":1000,"max":10}}}],"finally":{"state":"closed"}}`, wants: "exceeds max", }, + { + // The schema's shared ms maximum, driver-enforced: one past Go's + // int64-nanosecond line, time.Duration(ms)*time.Millisecond goes + // NEGATIVE and an accepted advance would REWIND virtual time. The + // bound is checked at load so the overflow is a fixture error in + // every driver, not a representation accident in one. + name: "advance ms beyond the 10-virtual-year maximum", + script: `{"name":"x","description":"d","steps":[{"advance":{"ms":9223372036855}}],"finally":{"state":"closed"}}`, + wants: "10 virtual years", + }, + { + name: "fireTimer envelope beyond the 10-virtual-year maximum", + script: `{"name":"x","description":"d","steps":[{"fireTimer":{"kind":"backoff","assertDelayMs":{"min":0,"max":9223372036855}}}],"finally":{"state":"closed"}}`, + wants: "10 virtual years", + }, + { + name: "config stalenessMs beyond the 10-virtual-year maximum", + script: `{"name":"x","description":"d","config":{"stalenessMs":9223372036855},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "10 virtual years", + }, + { + // Explicit zero is not omission: the schema says minimum 1, and a + // plain int64 decode read {"stalenessMs":0} as "absent, use the + // default" — accepting a value the schema rejects and silently + // substituting another. Presence is preserved with pointers. + name: "config stalenessMs explicit zero", + script: `{"name":"x","description":"d","config":{"stalenessMs":0},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "must be in [1,", + }, + { + name: "config confirmationDeadlineMs explicit zero", + script: `{"name":"x","description":"d","config":{"confirmationDeadlineMs":0},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "must be in [1,", + }, + { + name: "config repairPollBaseMs explicit zero", + script: `{"name":"x","description":"d","config":{"repairPollBaseMs":0},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "must be in [1,", + }, + { + // The unmodeled pair is unmodeled at ANY supplied value: explicit + // zero used to slip past the != 0 check into silence. + name: "config backoffBaseMs explicit zero", + script: `{"name":"x","description":"d","config":{"backoffBaseMs":0},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "not modeled", + }, + { + // Explicit null is the presence saga's second act: *int64 read + // {"stalenessMs":null} and omission both as nil, silently + // defaulting a value the schema's "type": "integer" rejects. + name: "config stalenessMs explicit null", + script: `{"name":"x","description":"d","config":{"stalenessMs":null},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "supplied as JSON null", + }, + { + name: "config confirmationDeadlineMs explicit null", + script: `{"name":"x","description":"d","config":{"confirmationDeadlineMs":null},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "supplied as JSON null", + }, + { + name: "config repairPollBaseMs explicit null", + script: `{"name":"x","description":"d","config":{"repairPollBaseMs":null},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "supplied as JSON null", + }, + { + name: "config backoffBaseMs explicit null", + script: `{"name":"x","description":"d","config":{"backoffBaseMs":null},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "not modeled", + }, + { + name: "config backoffCapMs explicit null", + script: `{"name":"x","description":"d","config":{"backoffCapMs":null},"steps":[{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "not modeled", + }, + { + // The envelope has the same three states the config durations do: + // a pointer read "assertDelayMs": null as omission, skipping the + // assertion the script wrote. + name: "fireTimer assertDelayMs explicit null", + script: `{"name":"x","description":"d","steps":[{"fireTimer":{"kind":"backoff","assertDelayMs":null}}],"finally":{"state":"closed"}}`, + wants: "supplied as JSON null", + }, + { + // min and max are schema-required: an absent member decoded to + // int64(0), inside the allowed range, silently converting the + // authored envelope into a different one. + name: "fireTimer assertDelayMs missing min", + script: `{"name":"x","description":"d","steps":[{"fireTimer":{"kind":"backoff","assertDelayMs":{"max":10}}}],"finally":{"state":"closed"}}`, + wants: "needs both min and max", + }, + { + name: "fireTimer assertDelayMs null member", + script: `{"name":"x","description":"d","steps":[{"fireTimer":{"kind":"backoff","assertDelayMs":{"min":null,"max":10}}}],"finally":{"state":"closed"}}`, + wants: "supplied as JSON null", + }, + { + // draft 2020-12 judges the VALUE, not the spelling: 1000.5 is not + // an integer instance, and the refusal should say so in the + // schema's terms rather than in encoding/json's. + name: "advance ms non-integral number", + script: `{"name":"x","description":"d","steps":[{"advance":{"ms":1000.5}}],"finally":{"state":"closed"}}`, + wants: "is not an integer", + }, + { + // An advance is deterministic only from a scripted rendezvous: + // an action's completion can precede the timer arms its + // transition causes (expectConnect returns when the dial is + // recorded; the handshake deadline arms on the connector's + // goroutine after), so an unrendezvoused advance races the arm — + // accepted on one schedule, rejected on another. The rendezvous + // is authored, not guessed: expectTimers' exact-set match is the + // settle, and the load rule makes its absence unscriptable. + name: "an advance not preceded by an expectTimers rendezvous", + script: `{"name":"x","description":"d","steps":[{"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}},{"expectConnect":{"url":"{{CABLE_URL:1}}"}},{"advance":{"ms":30000}}],"finally":{"state":"closed"}}`, + wants: "expectState + expectTimers rendezvous", + }, + { + // An EMPTY rendezvous set can never contain an arm of the + // preceding transition, so its match orders nothing: after a + // released failed mint, {"exact":{}} matches before backoff is + // armed, and the advance races the arm exactly as if the + // rendezvous were absent. A scenario with nothing yet armed + // advances as its first step instead. + name: "an advance behind an empty expectTimers rendezvous", + script: `{"name":"x","description":"d","steps":[{"expectState":{"is":"backoff"}},{"expectTimers":{"exact":{}}},{"advance":{"ms":1}}],"finally":{"state":"closed"}}`, + wants: "an empty rendezvous orders nothing", + }, + { + // The set match alone can coincide with a TRANSIENT mid-surgery + // set (the welcome transition stops handshake-deadline and arms + // confirmation-deadline in separate clock acquisitions, so + // {staleness:1} exists in between). The state announcement bounds + // the surgery: expectState blocks until the transition announces, + // and every timer still unarmed at an announcement is exactly + // what the set match then waits for. + name: "an advance whose rendezvous lacks the state barrier", + script: `{"name":"x","description":"d","steps":[{"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}},{"expectConnect":{"url":"{{CABLE_URL:1}}"}},{"expectTimers":{"exact":{"handshake-deadline":1}}},{"advance":{"ms":30000}}],"finally":{"state":"closed"}}`, + wants: "expectState + expectTimers rendezvous", + }, { name: "droppedCount disagreeing with droppedIds", script: `{"name":"x","description":"d","steps":[{"expectSignal":{"kind":"bufferOverflow","droppedIds":[1],"droppedCount":2}}],"finally":{"state":"closed"}}`, @@ -511,8 +670,218 @@ func TestScenarioDriverRejectsUnmatchedActions(t *testing.T) { }) } -// underShortWatchdog runs a scenario that is EXPECTED to fail under a short -// rendezvous window. A hostile scenario often fails by never satisfying a +// TestScenarioDriverRejectsSchedulingDependentAdvance pins the advance guard. +// A driver that quietly took the scheduling-dependent path would produce a +// result that differs between languages for the same fixture, which is worse +// than a failure because nothing reports it. +// +// The rule is about what an advance would FIRE, not about what it arms, and +// the third case below is where those differ: a firing that replaces nothing +// is still rejected. That is stricter than the arming rule this replaced, and +// deliberately so — the arming rule could only be enforced by sampling, and a +// sampled MUST is not one. +// +// TestScenarioMsAcceptsIntegralNumberSpellings pins the schema's number +// model onto the loader: draft 2020-12's "integer" is any number whose +// MATHEMATICAL value is integral, so 1000.0 and 1e3 are integer instances a +// schema-valid fixture may carry, and only the Go driver was refusing them — +// the same float-spelled-int class FlexInt absorbs on the rich-text lane. +// The literal is judged exactly, as text, by the one normalization walk the +// loader runs before any typed decode; the ms fields' 10-year range is then +// the value check's verdict, which is why an out-of-range spelling here is +// named by the walk's int64 bound and an in-range-but-zero one by the range. +func TestScenarioMsAcceptsIntegralNumberSpellings(t *testing.T) { + base := `{"name":"x","description":"d","config":{"stalenessMs":%s},"steps":[{"advance":{"ms":%s}}],"finally":{"state":"closed"}}` + cases := []struct { + name, staleness, ms, wantErr string + want int64 + }{ + {"float spelling", "1000.0", "1000.0", "", 1000}, + {"exponent spelling", "1e3", "1e3", "", 1000}, + {"non-integral", "1000.5", "1000", "is not an integer", 0}, + {"non-integral ms", "1000", "1000.5", "is not an integer", 0}, + // Integrality is a fact about the TEXT: float64 rounds these to + // exactly 1000 and exactly the maximum before any Trunc can look. + {"rounding-boundary fraction", "1000.00000000000001", "1000", "is not an integer", 0}, + {"near-maximum fraction", "315575999999.99999", "1000", "is not an integer", 0}, + {"exact maximum float spelling", "315576000000.0", "1000", "", 315576000000}, + // A quoted "1000" is a STRING instance — schema type integer refuses + // it, and json.Number's Unmarshal would happily have taken it. + {"quoted number", `"1000"`, "1000", "is a string", 0}, + {"quoted ms", "1000", `"1000"`, "is a string", 0}, + // A bomb is refused by its EFFECTIVE magnitude — significand length + // plus exponent — never by materializing the number, and never by + // the exponent's spelling alone: a significand can offset it. + {"exponent bomb", "1e999999999", "1000", "beyond any modeled value", 0}, + {"significand offsets a negative exponent", "100000000000000000000000000000000000000000000e-41", "1000", "", 1000}, + {"significand offsets to one hundred", "1000000000000000000000000000000000000000000e-40", "1000", "", 100}, + {"fraction-led spelling of one", "0.00000000000000000000000000000000000000001e41", "1000", "", 1}, + {"negative exponent bomb", "1e-999999999", "1000", "beyond any modeled value", 0}, + {"parse-bomb literal", strings.Repeat("9", 100001), "1000", "characters", 0}, + // The platform's max integer as an exponent must not wrap the place + // arithmetic into acceptance. + {"max-int exponent", "1e9223372036854775807", "1000", "beyond any modeled value", 0}, + {"min-int exponent", "1e-9223372036854775808", "1000", "beyond any modeled value", 0}, + // Exponents past int64 overflow the fixed-width parse itself; the + // range error takes the bound's own verdict, so the diagnostic is + // the same on every platform width. + {"beyond-int64 exponent", "1e9223372036854775808", "1000", "beyond any modeled value", 0}, + {"beyond-int64 negative exponent", "1e-9223372036854775809", "1000", "beyond any modeled value", 0}, + // Zero short-circuits before any exponent expansion; a zero + // stalenessMs is then the RANGE check's refusal, not a parse error. + {"zero with a large exponent", "0e199999", "1000", "must be in [1,", 0}, + {"zero beyond the exponent cap", "0e200001", "1000", "must be in [1,", 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + raw := fmt.Sprintf(base, tc.staleness, tc.ms) + sc, err := parseScenario([]byte(raw), "x.json") + if tc.wantErr == "" { + if err != nil { + t.Fatalf("an integral spelling must load: %v", err) + } + if got := sc.Config.StalenessMs.v; got != tc.want { + t.Errorf("stalenessMs decoded to %d, want %d", got, tc.want) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("err = %v, want one naming %q", err, tc.wantErr) + } + }) + } +} + +// TestScenarioIntegerFieldsAcceptIntegralNumberSpellings pins the same +// number model onto EVERY schema integer, not only the ms fields: the +// schema's top-level description promises drivers judge each numeric +// literal by its exact value, and that promise is only true cross-driver if +// a status, an id and a capacity spelled 200.0 / 1e2 / 1.0 load as their +// integer spellings do. One normalization walk over the document is what +// makes it true at once, so no field type has to opt in — and a non-integral +// literal in any of them is refused in the schema's terms, naming the member. +func TestScenarioIntegerFieldsAcceptIntegralNumberSpellings(t *testing.T) { + base := `{"name":"x","description":"d","config":{"liveBufferCapacity":%s},"steps":[{"expectMint":{"respond":{"status":%s,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}}],"finally":{"state":"closed","delivered":{"exact":[%s]}}}` + load := func(t *testing.T, capacity, status, id string) (*scenario, error) { + t.Helper() + return parseScenario([]byte(fmt.Sprintf(base, capacity, status, id)), "x.json") + } + want, err := load(t, "1", "200", "100") + if err != nil { + t.Fatalf("the integer spellings must load: %v", err) + } + cases := []struct{ name, capacity, status, id string }{ + {"float spellings", "1.0", "200.0", "100.0"}, + {"exponent spellings", "1e0", "2e2", "1e2"}, + {"mixed spellings", "1.0", "2000e-1", "0.1e3"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := load(t, tc.capacity, tc.status, tc.id) + if err != nil { + t.Fatalf("an integral spelling must load: %v", err) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("loaded scenario differs from its integer-spelled twin:\n got %+v\nwant %+v", got, want) + } + }) + } + refused := []struct{ name, capacity, status, id, wants string }{ + {"non-integral capacity", "1.5", "200", "100", "config.liveBufferCapacity: 1.5 is not an integer"}, + {"non-integral status", "1", "200.5", "100", "respond.status: 200.5 is not an integer"}, + {"non-integral id", "1", "200", "100.25", "delivered.exact[0]: 100.25 is not an integer"}, + } + for _, tc := range refused { + t.Run(tc.name, func(t *testing.T) { + _, err := load(t, tc.capacity, tc.status, tc.id) + if err == nil || !strings.Contains(err.Error(), tc.wants) { + t.Fatalf("err = %v, want one naming %q", err, tc.wants) + } + }) + } +} + +// The control matters as much as the mutants: an advance over a window with +// nothing due is ordinary and must still pass, or the guard would be rejecting +// every advance and the suite's one real advance (fixture 05) would be failing +// for the wrong reason. +func TestScenarioDriverRejectsSchedulingDependentAdvance(t *testing.T) { + t.Run("an advance during which the connector arms a timer", func(t *testing.T) { + // Advancing past the handshake deadline fires it, and the teardown it + // causes arms `backoff` inside the same window — the reentrant clause + // the algorithm cannot resolve identically across languages when the + // recipient is another goroutine. The guard never has to observe that + // arming: the firing alone is enough to reject the script. + script := `{"name":"x","description":"d","steps":[ + {"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}}, + {"expectConnect":{"url":"{{CABLE_URL:1}}"}}, + {"expectState":{"is":"awaiting_welcome"}}, + {"expectTimers":{"exact":{"handshake-deadline":1,"staleness":1}}}, + {"advance":{"ms":30000}}], + "finally":{"state":"backoff"}}` + err := underShortWatchdog(func() error { return runScenarioBytes([]byte(script), "x.json") }) + if err == nil { + t.Fatal("an advance that changes the outstanding timer set must fail the scenario") + } + if !strings.Contains(err.Error(), "would fire") { + t.Fatalf("failed for the wrong reason: %v", err) + } + if !strings.Contains(err.Error(), "fireTimer") { + t.Errorf("the rejection must name the deterministic alternative: %v", err) + } + }) + + t.Run("an advance over a quiet window is ordinary", func(t *testing.T) { + // No connection yet, so nothing is armed and nothing is due: the + // guard must not reject an advance merely for existing. + script := `{"name":"x","description":"d","steps":[ + {"advance":{"ms":1000}}, + {"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}}, + {"expectConnect":{"url":"{{CABLE_URL:1}}"}}], + "finally":{"state":"awaiting_welcome"}}` + if err := underShortWatchdog(func() error { return runScenarioBytes([]byte(script), "x.json") }); err != nil { + t.Fatalf("an advance over a window that arms nothing must pass: %v", err) + } + }) + + // A firing that replaces nothing is STILL rejected, and this is the case + // that shows the rule changed rather than merely being reimplemented. + // Here the backoff deadline expires and the connector's next act is a mint, + // which parks inside the seam until the driver releases it, so nothing is + // armed anywhere in the window. Under the arming rule this was legal. It is + // not any more, because "did anything get armed?" can only be answered by + // waiting and hoping, while "is anything due?" is one atomic read — and the + // script that wanted this has `fireTimer`, which says which timer it means. + t.Run("an advance in which a due timer fires without replacement is still rejected", func(t *testing.T) { + script := `{"name":"x","description":"d","steps":[ + {"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:1}}","expires_in":120,"url":"{{CABLE_URL:1}}"}}}}, + {"expectConnect":{"url":"{{CABLE_URL:1}}"}}, + {"serve":{"frame":"welcome"}}, + {"expectSubscribe":{"channel":"EventsChannel"}}, + {"fireTimer":{"kind":"confirmation-deadline"}}, + {"expectClientClose":{}}, + {"expectState":{"is":"backoff"}}, + {"expectTimers":{"exact":{"backoff":1}}}, + {"advance":{"ms":1000}}, + {"expectMint":{"respond":{"status":200,"body":{"ticket":"{{TICKET:2}}","expires_in":120,"url":"{{CABLE_URL:2}}"}}}}, + {"expectConnect":{"url":"{{CABLE_URL:2}}"}}], + "finally":{"state":"awaiting_welcome"}}` + err := underShortWatchdog(func() error { return runScenarioBytes([]byte(script), "x.json") }) + if err == nil { + t.Fatal("an advance whose window fires a timer must fail, even when it replaces nothing") + } + if !strings.Contains(err.Error(), "would fire") { + t.Fatalf("failed for the wrong reason: %v", err) + } + if !strings.Contains(err.Error(), "fireTimer") { + t.Errorf("the rejection must name the deterministic alternative: %v", err) + } + }) +} + +// underShortWatchdog runs a scenario under a short rendezvous window. Its +// usual use is a scenario EXPECTED to fail, but it serves any case whose waits +// are all short by construction. A hostile scenario often fails by never satisfying a // rendezvous, and waiting the full window for each would cost more than the // whole conformance suite; every caller still pins the failure's reason, so a // mutant rejected for the wrong reason cannot pass as the pin firing. diff --git a/python/src/basecamp/services/todolists.py b/python/src/basecamp/services/todolists.py index 7150121554..f2ca9904c1 100644 --- a/python/src/basecamp/services/todolists.py +++ b/python/src/basecamp/services/todolists.py @@ -140,9 +140,9 @@ def _writable_string(body: dict[str, Any], key: str, *, non_empty: bool = False) makes the check explicit work here rather than something the layer below already did, and #544 did not change it: flattening the declared shape changes what the API returns, not what Python validates — ``get`` still - hands back the parsed JSON as ``dict[str, Any]``. The same shape is live in - the shipped Todos and Cards composites; that is tracked separately in #576, - and giving Python a decoder at all in #578. + hands back the parsed JSON as ``dict[str, Any]``. The same shape in the + shipped Todos and Cards composites is guarded by the ``_merge_safe`` checks + #576 closed with; giving Python a decoder at all is tracked in #578. """ if key not in body: raise ApiError( diff --git a/python/tests/services/test_todolists_service.py b/python/tests/services/test_todolists_service.py index 64957b416a..2bac84c2b4 100644 --- a/python/tests/services/test_todolists_service.py +++ b/python/tests/services/test_todolists_service.py @@ -278,7 +278,8 @@ class TestMalformedWritableFields: is full-replace, that value is then written back over the real one — the composite erases the field it exists to preserve, on a call that never mentioned it. Truthy non-strings are just as wrong: they reach the wire - verbatim. The shipped Todos/Cards analogue is tracked in #576. + verbatim. The shipped Todos/Cards analogue takes the same refusal from + the ``_merge_safe`` guards #576 closed with. """ @pytest.mark.parametrize("malformed", [False, 0, [], {}, 42, True, ["x"], {"a": 1}]) diff --git a/ruby/lib/basecamp/services/todolists_extensions.rb b/ruby/lib/basecamp/services/todolists_extensions.rb index a0324c062a..32f55b2d1a 100644 --- a/ruby/lib/basecamp/services/todolists_extensions.rb +++ b/ruby/lib/basecamp/services/todolists_extensions.rb @@ -180,9 +180,10 @@ def require_hash(body) # Ruby has no typed decoder between the GET and this read, unlike the Go, # Swift and Kotlin composites where a wrong-typed field fails at decode, # and flattening the shape did not add one: the generated method still - # returns http_get(...).json verbatim. The same shape is live in - # the shipped Todos composite; tracked in #576, with the generated - # validating layer that would retire this guard tracked in #578. + # returns http_get(...).json verbatim. The same shape in the + # shipped Todos composite is guarded by the MergeSafe checks #576 closed + # with; the generated validating layer that would retire this guard is + # tracked in #578. def writable_string(body, key, non_empty: false) raise_missing_field(key) unless body.key?(key) diff --git a/ruby/test/basecamp/services/todolists_service_test.rb b/ruby/test/basecamp/services/todolists_service_test.rb index 02d62be7d4..ad47141e95 100644 --- a/ruby/test/basecamp/services/todolists_service_test.rb +++ b/ruby/test/basecamp/services/todolists_service_test.rb @@ -159,7 +159,8 @@ def test_replace_sends_an_explicit_empty_description # straight through. This endpoint is full-replace, so either outcome is # written back over the real value — the composite erases or corrupts the # field it exists to preserve, on a call that never mentioned it. The shipped - # Todos analogue is tracked in #576. + # Todos analogue takes the same refusal from the MergeSafe guards #576 + # closed with. [ false, 0, [], {}, 42, true, [ "x" ], { "a" => 1 } ].each do |malformed| define_method("test_update_refuses_a_malformed_description_#{malformed.inspect}") do stub_todolist_get_and_put(todolist: full_todolist.merge("description" => malformed)) diff --git a/scripts/check-known-defect-issues-open b/scripts/check-known-defect-issues-open index 29f84fcff5..60f2a77307 100755 --- a/scripts/check-known-defect-issues-open +++ b/scripts/check-known-defect-issues-open @@ -9,6 +9,23 @@ # bc3_routes_not_modeled[].tracking_issue # "an OPEN issue owns absorbing it" # +# spec/tracking-issues.yml discharges the same kind of claim made in PROSE — +# "Tracked in #N" in a README or a code comment. Same failure, same remedy: a +# closed issue tracks nothing, and a sentence saying otherwise is a lie nobody +# is told about. It is a registry rather than a scan of the prose because +# "#\d+" cannot tell a tracking promise from an as-of citation ("shipped in +# #12380"), and sorting those needs the author, not a matcher. +# +# The registry is the judgment surface, and one DETECTOR backs it: the checker +# sweeps tracked text files for the canonical grammar "tracked [separately] in +# #N" (tolerating a comment-prefixed line wrap mid-phrase) and fails when a +# mention's [file, issue] pair is not registered — so the canonical form can +# never be written without being registered. Non-canonical phrasings ("structural +# safety ... is #N") are OUT OF THE DETECTOR'S REACH BY DESIGN: an open +# vocabulary cannot be enumerated, and a regex that tried would sweep up the +# as-of citations that must never be flagged. Those still go through the +# registry by hand. PROSE_SWEEP_ROOT overrides the swept tree (self-test). +# # check-bc3-route-parity already requires those to be numeric. A number is not a # tracker: #588 auto-closed while nine live 404s still pointed at it, and that # gate stayed green the whole time because "is an Integer" was the entire test. @@ -26,7 +43,11 @@ # fails rather than passing quietly, because an unverifiable "we know" claim is # indistinguishable from an untracked defect. # -# No-op when the allowlist references no issues at all, which is today's state. +# Every run verifies the registry's claims live (and the allowlist's, when it +# references issues). A green live run proves only that today's referenced +# issues are open; the offline self-test is what covers the failure branches — +# closed issues, malformed rows, the fail-closed paths — that a green run +# never exercises. # # Usage: ./check-known-defect-issues-open # BC3_ROUTE_ALLOWLIST=... KNOWN_DEFECT_ISSUE_REPO=... \ @@ -35,6 +56,7 @@ # Exercised offline by scripts/test-check-known-defect-issues-open. require 'json' +require 'set' require 'yaml' ROOT = File.expand_path('..', __dir__) @@ -54,7 +76,89 @@ unless File.exist?(allow_path) warn "ERROR: allowlist not found at #{allow_path}" exit 1 end -allow = YAML.safe_load(File.read(allow_path)) || {} +# YAML keeps only the LAST of duplicate mapping keys, so a second +# `sdk_routes_known_defective:` key would REPLACE the first list — every issue +# reference in it gone, with nothing malformed left for the shape validation +# below to see. Psych's parse tree still holds both, so duplicates are +# detected there, before safe_load flattens them. All mappings are walked, +# not just the root: a duplicated key inside one entry silently drops a field +# the same way. +def reject_duplicate_keys!(path, text) + # The STREAM is parsed, not the first document: Psych.parse and safe_load + # read only the first, so everything after a `---` separator — rows, + # entries, whole lists — would silently vanish from the check. A registry + # cannot mean "the first document of". + stream = Psych.parse_stream(text) + docs = stream ? stream.children : [] + if docs.length > 1 + warn "ERROR: #{path} holds #{docs.length} YAML documents — only the first is ever read, so everything " \ + 'after the `---` separator would silently vanish from the check' + exit 1 + end + # An empty file parses to no documents at all — nothing to walk; the shape + # validation below speaks. + return if docs.empty? + + stack = [ docs.first ] + until stack.empty? + node = stack.pop + if node.is_a?(Psych::Nodes::Mapping) + dup = node.children.each_slice(2) + .map { |k, _| k.value if k.respond_to?(:value) } + .compact.tally.select { |_, n| n > 1 }.keys + unless dup.empty? + warn "ERROR: #{path} repeats mapping key(s): #{dup.join(', ')} — YAML keeps only the last, " \ + 'silently discarding the first' + exit 1 + end + end + stack.concat(node.children) if node.children + end +end + +# Psych's exception family — syntax errors, forbidden tags, aliases (which +# safe_load refuses) — is the same fail-closed verdict as a malformed shape, +# and gets the same treatment: the gate's own words, naming the file to fix, +# never a psych.rb backtrace. +allow_text = File.read(allow_path) +begin + reject_duplicate_keys!(allow_path, allow_text) + allow = YAML.safe_load(allow_text) || {} +rescue Psych::Exception => e + warn "ERROR: #{allow_path} cannot be parsed as YAML: #{e.message}" + exit 1 +end +unless allow.is_a?(Hash) + warn "ERROR: #{allow_path} must be a YAML mapping of route lists, got #{allow.class}" + exit 1 +end + +# REQUIRED, not optional. Treating an absent registry as an empty one is a +# fail-OPEN path in a gate whose entire purpose is failing closed: delete the +# file (or typo the override) and the checker reports "nothing to verify" and +# exits 0, with every prose claim in the repository unverified. That is the +# #588 failure with an extra step, which is what this script exists to end. +tracking_path = ENV['TRACKING_ISSUES'] || File.join(ROOT, 'spec', 'tracking-issues.yml') +unless File.exist?(tracking_path) + warn "ERROR: tracking registry not found at #{tracking_path}" + warn ' This check fails closed by design: a missing registry is indistinguishable' + warn ' from one whose claims are all unverified. Restore it, or empty its list.' + exit 1 +end +tracking_text = File.read(tracking_path) +begin + reject_duplicate_keys!(tracking_path, tracking_text) + tracking = YAML.safe_load(tracking_text) || {} +rescue Psych::Exception => e + warn "ERROR: #{tracking_path} cannot be parsed as YAML: #{e.message}" + exit 1 +end +# A non-mapping top level ([] or a bare string) would crash on .key? below — +# nonzero, but a backtrace is not a diagnosis. Same fail-closed, better words. +unless tracking.is_a?(Hash) + warn "ERROR: #{tracking_path} must be a YAML mapping with a prose_tracking_issues list, got #{tracking.class}" + exit 1 +end # (issue number, human-readable site) pairs, so a failure names the entry that # has to change rather than just the number. @@ -68,8 +172,170 @@ end refs << [e['tracking_issue'], "bc3_routes_not_modeled #{e['method']} #{e['path']}"] end +# Every row is VALIDATED rather than skipped. `next unless Integer` is how the +# allowlist entries are read, and there it is tolerable because an entry with no +# issue number is making no claim. Here the row IS the claim: a malformed one is +# a promise the author wrote and the gate silently dropped, which is worse than +# either a failure or an absent row. +# The KEY is required, not just the file. A registry whose key is misspelled +# or dropped in an edit reads as nil and used to pass as empty — the same +# fail-open path as a deleted file, one typo later. Only the explicit empty +# list says "no promises" on purpose. +unless tracking.key?('prose_tracking_issues') + warn "ERROR: #{tracking_path} must define prose_tracking_issues (use [] when there are none)." + exit 1 +end +rows = tracking['prose_tracking_issues'] +unless rows.is_a?(Array) + warn "ERROR: prose_tracking_issues in #{tracking_path} must be a list, got #{rows.class}" + exit 1 +end +rows.each_with_index do |e, i| + unless e.is_a?(Hash) + warn "ERROR: prose_tracking_issues[#{i}] in #{tracking_path} must be a mapping, got #{e.class}" + exit 1 + end + unless e['issue'].is_a?(Integer) && e['issue'].positive? + warn "ERROR: prose_tracking_issues[#{i}] in #{tracking_path} needs a positive integer `issue`, got #{e['issue'].inspect}" + exit 1 + end + # String-typed, not merely stringifiable: to_s coerces [] and {} and 123 + # into non-empty text, which would register a promise into a shape nothing + # reads — the sweep's [file, issue] pair can never match a non-String file. + unless e['site'].is_a?(String) && !e['site'].strip.empty? + warn "ERROR: prose_tracking_issues[#{i}] (##{e['issue']}) in #{tracking_path} needs a `site` — a non-blank " \ + 'string naming the sentence that leans on it — got ' + e['site'].inspect + exit 1 + end + unless e['file'].is_a?(String) && !e['file'].strip.empty? + warn "ERROR: prose_tracking_issues[#{i}] (##{e['issue']}) in #{tracking_path} needs a `file` — a non-blank " \ + 'string naming the path the sentence lives in (the sweep keys coverage on [file, issue]) — got ' + e['file'].inspect + exit 1 + end + # The count is committed, exactly as doc-constants commits its marker + # counts: a bare [file, issue] pair would silently cover every FUTURE + # canonical mention in the file, so each row records how many it stands for + # (0 for a promise phrased outside the canonical grammar) and the sweep + # verifies the number in both directions. + unless e['mentions'].is_a?(Integer) && e['mentions'] >= 0 + warn "ERROR: prose_tracking_issues[#{i}] (##{e['issue']}) in #{tracking_path} needs a `mentions` count — " \ + 'the number of canonical "tracked in #N" mentions of this issue in `file` — got ' + e['mentions'].inspect + exit 1 + end + refs << [e['issue'], e['site']] +end + +# One row per [file, issue]: a duplicate would make the committed count +# ambiguous (which row does the sweep charge a mention to?). +dup = rows.map { |e| [ e['file'], e['issue'] ] }.tally.select { |_, n| n > 1 }.keys +unless dup.empty? + dup.each { |f, n| warn "ERROR: duplicate prose_tracking_issues rows for [#{f}, ##{n}] in #{tracking_path} — merge them and sum `mentions`" } + exit 1 +end + +# --- Discovery sweep -------------------------------------------------------------- +# +# Hand-enumeration cannot prove the registry covers the tree, so the one +# canonical grammar is swept for: "tracked [separately] in #N". A mention whose +# [file, issue] pair is not registered fails the gate. The GAP between the +# phrase's words tolerates a newline plus a comment leader, because the phrase +# wraps in real prose and a line-based match would read a wrapped promise as +# absence. Excluded: the registry itself and this gate's self-test (both quote +# the grammar as data — the header's example, the test's corpus literals) and +# spec/api-gaps/ (historical as-of citations, never promises). +# +# Files come from `git ls-files` — tracked text only, .gitignore respected — +# with a plain directory walk as the fallback for a sweep root that is not a +# repository (the self-test corpus). Binary files are skipped by NUL sniff. + +# The line leader is matched by CLASS, not by count: Swift/Kotlin doc +# comments are `///`, box comments stack `*`, Markdown blockquotes nest `>`, +# and a leader that consumed an exact number of marks would leave the surplus +# mark in front of the next word and read a wrapped promise as absence. `#+` +# refuses a following digit so an issue reference is never eaten as a leader. +SWEEP_GAP = /(?:[ \t]|\r?\n[ \t]*(?:#+(?!\d)|\/{2,}|\*+|-{2,}|(?:>[ \t]*)+)?[ \t]*)+/ +SWEEP_MENTION = /\btracked(?:#{SWEEP_GAP}separately)?#{SWEEP_GAP}in#{SWEEP_GAP}#(\d+)/i + +def sweep_files(root) + listing = begin + out = IO.popen([ 'git', '-C', root, 'ls-files', '-z' ], err: File::NULL, &:read) + $?.success? ? out.split("\0") : nil + rescue Errno::ENOENT + nil + end + return listing unless listing.nil? || listing.empty? + + require 'find' + files = [] + Find.find(root) do |p| + if File.directory?(p) + Find.prune if File.basename(p) == '.git' + next + end + files << p.delete_prefix("#{root}#{File::SEPARATOR}") + end + files +end + +sweep_root = ENV['PROSE_SWEEP_ROOT'] || ROOT +registered_mentions = rows.to_h { |e| [ [ e['file'], e['issue'] ], e['mentions'] ] } +swept = Hash.new { |h, k| h[k] = [] } +scanned = Set.new +sweep_files(sweep_root).each do |rel| + next if rel == 'spec/tracking-issues.yml' || + rel == 'scripts/test-check-known-defect-issues-open' || + rel.start_with?('spec/api-gaps/') + + path = File.join(sweep_root, rel) + next unless File.file?(path) + + content = File.binread(path) + next if content.index("\0".b) + + scanned << rel + content = content.force_encoding(Encoding::UTF_8).scrub + content.to_enum(:scan, SWEEP_MENTION).each do + m = Regexp.last_match + swept[[ rel, m[1].to_i ]] << content[0...m.begin(0)].count("\n") + 1 + end +end + +violations = [] +swept.each do |(rel, issue), lines| + unless registered_mentions.key?([ rel, issue ]) + lines.each { |line| violations << "#{rel}:#{line} — a canonical \"tracked in ##{issue}\" claim, and [#{rel}, ##{issue}] is not registered" } + end +end +# A mentions: 0 row guards a non-canonical promise by NAME alone, so a rename, +# delete, or typo makes swept.fetch empty, 0 == 0 passes, and the row silently +# guards nothing — green until the issue closes and CI blocks on a phantom. +# Every registered path must be in the sweep's view (present, text, and not +# excluded from the scan). +registered_mentions.each_key do |(rel, _issue)| + next if scanned.include?(rel) + + violations << "#{rel} is registered but the sweep cannot see it (missing, renamed, binary, or excluded) — " \ + 'a row for a file the sweep cannot read guards nothing; fix the path or remove the row' +end +# Both directions, so a new mention cannot ride an existing row and a deleted +# sentence cannot leave its count claiming more than the file holds. +registered_mentions.each do |(rel, issue), expected| + found = swept.fetch([ rel, issue ], []) + next if found.length == expected + + violations << "#{rel} ##{issue}: the registry records #{expected} canonical mention(s), the sweep found " \ + "#{found.length}#{found.empty? ? '' : " (line #{found.join(', ')})"} — update `mentions:` alongside the prose" +end +unless violations.empty? + warn 'ERROR: canonical tracking claims exist outside the registry:' + violations.each { |v| warn " - #{v}" } + warn ' Register each in spec/tracking-issues.yml (issue, file, site) — or, if the' + warn ' promise is already discharged, rewrite the sentence as an as-of fact.' + exit 1 +end + if refs.empty? - puts ' ✓ bc3 route allowlist references no tracking issues — nothing to verify' + puts ' ✓ no tracking issues referenced — nothing to verify' exit 0 end @@ -98,8 +364,8 @@ refs.each do |number, site| next if state.nil? || state == 'open' failures << "#{site} points at ##{number}, which is #{state.upcase}. " \ 'A closed issue tracks nothing: reopen it, repoint the entry at the ' \ - 'issue that now owns the defect, or remove the entry because the ' \ - 'route is fixed.' + 'issue that now owns the defect, or remove the entry because what it ' \ + 'tracked is fixed — and delete the claim that leaned on it.' end unless failures.empty? diff --git a/scripts/test-check-known-defect-issues-open b/scripts/test-check-known-defect-issues-open index 58ffdcf9ac..f1694c7da1 100755 --- a/scripts/test-check-known-defect-issues-open +++ b/scripts/test-check-known-defect-issues-open @@ -47,6 +47,7 @@ require "yaml" require "tmpdir" +require "fileutils" require "open3" require "rbconfig" @@ -89,7 +90,16 @@ failures = [] # all, when gh is meant to be absent), so the real gh can never answer. The # interpreter is invoked by absolute path for the same reason. Returns # [combined_output, status, gh_log_lines]. -def run_checker(allowlist:, states: nil, gh_fail: false, repo: nil, github_repository: nil) +# tracking defaults to the explicit empty registry — the one sanctioned no-op — +# because a file WITHOUT the key is an error (case N+7): most cases here are +# about the allowlist and just need the registry to say "no promises" on purpose. +# +# sweep is the discovery corpus: filename => content, written under a tmp dir +# that PROSE_SWEEP_ROOT points the checker's canonical-grammar sweep at. Every +# case gets a corpus — empty by default — so the sweep can never read the REAL +# repository against a synthetic registry, which would fail every case here on +# facts about the tree rather than about the checker. +def run_checker(allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, allowlist_raw: nil, tracking: { "prose_tracking_issues" => [] }, tracking_raw: nil, sweep: {}, states: nil, gh_fail: false, repo: nil, github_repository: nil) Dir.mktmpdir("known-defect-selftest") do |dir| path_dir = File.join(dir, "bin") Dir.mkdir(path_dir) @@ -102,11 +112,37 @@ def run_checker(allowlist:, states: nil, gh_fail: false, repo: nil, github_repos end allow_path = File.join(dir, "bc3-route-allowlist.yml") - File.write(allow_path, YAML.dump(allowlist)) + # allowlist_raw writes bytes verbatim — the malformed-SYNTAX cases need a + # file YAML.dump could never produce. + File.write(allow_path, allowlist_raw || YAML.dump(allowlist)) + + # Pointed at a temp file even when the case supplies nothing, so the + # checker can never read the repository's REAL registry mid-self-test — + # which would make these cases depend on live issue state and on the + # network, the two things the stub exists to remove. + tracking_path = File.join(dir, "tracking-issues.yml") + # nil means "do not create the file", which is how the missing-registry + # case is expressed without deleting the repository's real one. + # tracking_raw writes bytes verbatim, for the malformed-syntax case. + if tracking_raw + File.write(tracking_path, tracking_raw) + elsif !tracking.nil? + File.write(tracking_path, YAML.dump(tracking)) + end + + sweep_root = File.join(dir, "corpus") + Dir.mkdir(sweep_root) + sweep.each do |rel, content| + full = File.join(sweep_root, rel) + FileUtils.mkdir_p(File.dirname(full)) + File.write(full, content) + end env = { "PATH" => path_dir, "BC3_ROUTE_ALLOWLIST" => allow_path, + "TRACKING_ISSUES" => tracking_path, + "PROSE_SWEEP_ROOT" => sweep_root, "GH_STUB_LOG" => gh_log, "GH_STUB_STATES" => states, "GH_STUB_FAIL" => (gh_fail ? "1" : nil), @@ -120,7 +156,13 @@ def run_checker(allowlist:, states: nil, gh_fail: false, repo: nil, github_repos end end +# CASES counts what actually ran. The summary used to restate the number by +# hand, and it was wrong the moment a case was added — the same stale-constant +# failure the gates in this directory exist to catch, in the gate's own tests. +CASES = [ 0 ] + def expect_pass(failures, label, out, status, fragment = nil) + CASES[0] += 1 if !status.success? puts " FAIL #{label}" failures << "#{label}: expected PASS but checker exited #{status.exitstatus}:\n#{out}" @@ -133,6 +175,7 @@ def expect_pass(failures, label, out, status, fragment = nil) end def expect_fail(failures, label, out, status, fragment) + CASES[0] += 1 if status.success? puts " FAIL #{label}" failures << "#{label}: expected FAILURE but checker passed:\n#{out}" @@ -161,7 +204,7 @@ puts "==> known-defect tracking-issue self-test (checker: #{CHECKER.sub("#{ROOT} out, status, calls = run_checker(allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }) expect_pass(failures, "1. empty allowlist is a no-op", out, status, - "references no tracking issues") + "no tracking issues referenced") unless calls.empty? puts " FAIL 1a. empty allowlist makes no API calls" failures << "1a: expected no gh calls, got #{calls.inspect}" @@ -281,8 +324,397 @@ expect_fail(failures, "9. missing gh fails closed", out, status, # --- Report ------------------------------------------------------------------------ +# --- N. Prose tracking claims are collected from the registry -------------------- +# +# "Tracked in #N" in a README is the same claim the allowlist makes in +# structured form, and it fails the same way: the issue closes, the sentence +# keeps promising someone owns the gap, and nothing says otherwise. The registry +# exists because prose cannot be scanned for this — "#\d+" cannot separate a +# tracking promise from an as-of citation — so the author registers the promise +# and the gate verifies it alongside the allowlist's. + +out, status, calls = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303, "file" => "README.md", "site" => "README.md — a gap", "mentions" => 0 } ] }, + sweep: { "README.md" => "a gap, promised outside the canonical grammar\n" }, + states: "303=open" +) +expect_pass(failures, "N. prose registry entries are collected", out, status, + "README.md — a gap") +unless calls.any? { |c| c.end_with?("/issues/303") } + puts " FAIL Na. prose entry is looked up" + failures << "Na: expected a lookup of #303, got #{calls.inspect}" +end + +# --- N+1. A closed prose tracking issue fails the gate -------------------------- +# +# The whole point. A registry that reported a closed issue as fine would be the +# #588 failure with extra steps. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 404, "file" => "SPEC.md", "site" => "SPEC.md — a bound", "mentions" => 0 } ] }, + sweep: { "SPEC.md" => "a bound, promised outside the canonical grammar\n" }, + states: "404=closed" +) +expect_fail(failures, "N+1. a CLOSED prose tracking issue fails", out, status, + "SPEC.md — a bound points at #404, which is CLOSED") + +# --- N+2. Allowlist and registry are collected together ------------------------- +# +# Neither source may mask the other: a green allowlist must not make an unread +# registry look verified, which is what a single-source read would do. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [ defective(101) ], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303, "file" => "README.md", "site" => "README.md — a gap", "mentions" => 0 } ] }, + sweep: { "README.md" => "a gap, promised outside the canonical grammar\n" }, + states: "101=open,303=closed" +) +expect_fail(failures, "N+2. a closed registry entry fails even with a green allowlist", out, status, + "README.md — a gap points at #303, which is CLOSED") + +# --- N+3. A MISSING registry fails closed --------------------------------------- +# +# The fail-open path this gate cannot have. Treating an absent file as an empty +# one means deleting it (or typoing the override) reports "nothing to verify" +# and exits 0, with every prose claim unverified — the #588 failure with an +# extra step, in the script written to end it. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: nil +) +expect_fail(failures, "N+3. a missing registry fails closed", out, status, + "tracking registry not found") + +# --- N+4. Malformed rows fail rather than being skipped -------------------------- +# +# The allowlist skips entries without an issue number, and there that is right: +# such an entry makes no claim. Here the row IS the claim, so a malformed one is +# a promise the author wrote and the gate silently dropped. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303 } ] } +) +expect_fail(failures, "N+4. a row with no site fails", out, status, "needs a `site`") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "site" => "README.md — a gap" } ] } +) +expect_fail(failures, "N+5. a row with no issue number fails", out, status, + "needs a positive integer `issue`") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => "not a list" } +) +expect_fail(failures, "N+6. a non-list registry fails", out, status, "must be a list") + +# --- N+6a/b. site and file must be Strings, not merely stringifiable ------------- +# +# `to_s` coerces [] and {} and 123 into non-empty text, so a malformed row +# reached the issue check with no human-readable location. The row IS the +# claim: a value the sweep's [file, issue] pair can never match is a promise +# registered into a shape nothing reads. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303, "site" => 123, "file" => "README.md" } ] }, + states: "303=open" +) +expect_fail(failures, "N+6a. a non-string site fails", out, status, "needs a `site`") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303, "site" => "README.md — a gap", "file" => [ "README.md" ] } ] }, + states: "303=open" +) +expect_fail(failures, "N+6b. a non-string file fails", out, status, "needs a `file`") + +# --- N+6c/d. A non-mapping YAML top level fails with the gate's own words --------- +# +# YAML.safe_load of "- x" returns an Array, and Array#key? / Array#[String] +# raise — the gate exited nonzero, but through a backtrace rather than a +# diagnosis, telling the next reader nothing about which file is malformed or +# how. Fail closed WITH the diagnostics. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: [ "not", "a", "mapping" ] +) +expect_fail(failures, "N+6c. a non-mapping registry file fails with a diagnosis", out, status, + "must be a YAML mapping") + +out, status, _ = run_checker(allowlist: [ "not", "a", "mapping" ]) +expect_fail(failures, "N+6d. a non-mapping allowlist fails with a diagnosis", out, status, + "must be a YAML mapping") + +# --- N+6e/f. YAML that does not PARSE fails with the gate's words too ------------ +# +# Psych::SyntaxError (and safe_load's tag/alias refusals) exited through a +# backtrace: nonzero, fail-closed, and useless — naming a psych.rb line rather +# than the file to fix. Same verdict, the gate's own diagnosis. + +out, status, _ = run_checker(tracking_raw: "prose_tracking_issues: [unclosed\n") +expect_fail(failures, "N+6e. unparseable registry YAML fails with a diagnosis", out, status, + "cannot be parsed as YAML") + +out, status, _ = run_checker(allowlist_raw: "sdk_routes_known_defective: [unclosed\n") +expect_fail(failures, "N+6f. unparseable allowlist YAML fails with a diagnosis", out, status, + "cannot be parsed as YAML") + +# --- N+6g/h. Duplicate mapping keys fail instead of silently last-winning -------- +# +# YAML keeps only the LAST of duplicate keys, so a second +# `sdk_routes_known_defective:` REPLACES the first list — every issue +# reference in it gone, with nothing malformed left for shape validation to +# see. Psych's parse tree still holds both, so the gate looks there first. + +out, status, _ = run_checker( + allowlist_raw: "sdk_routes_known_defective:\n - method: GET\n path: /x\n issue: 101\nsdk_routes_known_defective: []\n" +) +expect_fail(failures, "N+6g. a duplicate allowlist key fails instead of last-winning", out, status, + "repeats mapping key") + +out, status, _ = run_checker( + tracking_raw: "prose_tracking_issues:\n - issue: 303\n file: \"a.md\"\n site: \"a gap\"\n mentions: 0\nprose_tracking_issues: []\n" +) +expect_fail(failures, "N+6h. a duplicate registry key fails instead of last-winning", out, status, + "repeats mapping key") + +# --- N+6i/j. An EMPTY file is an empty document, not a crash --------------------- +# +# Psych.parse("") returns false, not nil, so an empty file reached the parse +# tree walk and crashed on false.root — nonzero, but a NoMethodError is not +# the gate's diagnosis. Falsy parse results are empty documents; the shape +# validation below then speaks in its own words. + +out, status, _ = run_checker(tracking_raw: "") +expect_fail(failures, "N+6i. an empty registry file fails with the key diagnosis", out, status, + "must define prose_tracking_issues") + +out, status, _ = run_checker(allowlist_raw: "") +expect_pass(failures, "N+6j. an empty allowlist file is an empty allowlist", out, status, + "no tracking issues referenced") + +# --- N+6k/l. A multi-document file fails instead of dropping documents ----------- +# +# Psych.parse and safe_load read only the FIRST YAML document: everything +# after a `---` separator — rows, entries, whole lists — silently vanishes +# from the live check. A registry cannot mean "the first document of". + +out, status, _ = run_checker( + tracking_raw: "prose_tracking_issues: []\n---\nprose_tracking_issues:\n - issue: 999\n file: \"a.md\"\n site: \"a hidden promise\"\n mentions: 0\n" +) +expect_fail(failures, "N+6k. a multi-document registry fails instead of dropping rows", out, status, + "YAML documents") + +out, status, _ = run_checker( + allowlist_raw: "sdk_routes_known_defective: []\n---\nsdk_routes_known_defective:\n - method: GET\n path: /x\n issue: 101\n" +) +expect_fail(failures, "N+6l. a multi-document allowlist fails instead of dropping entries", out, status, + "YAML documents") + +# --- N+18. A row whose file the sweep cannot see fails --------------------------- +# +# A mentions: 0 row guards a non-canonical promise by NAME alone, so when its +# file is renamed, deleted, or mistyped, swept.fetch returns empty, 0 == 0, +# and the row silently stops guarding anything — green until the issue closes +# and CI blocks on a phantom promise. Every registered path must be in the +# sweep's view. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "ghost.md", "site" => "ghost.md — a promise whose file went away", "mentions" => 0 } + ] }, + states: "303=open" +) +expect_fail(failures, "N+18. a row for a file the sweep cannot see fails", out, status, + "cannot see") + +# --- N+7. A registry file without the key fails ----------------------------------- +# +# The missing-FILE case (N+3) fails closed; a file whose key is misspelled or +# dropped in an edit is the same accident one typo later, and it used to pass +# as an empty registry. Only the explicit empty list says "no promises" on +# purpose — a keyless file is indistinguishable from one the gate never read. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: {} +) +expect_fail(failures, "N+7. a registry without the prose_tracking_issues key fails", out, status, + "must define prose_tracking_issues") + +# --- N+8. An unregistered canonical mention fails the sweep ---------------------- +# +# The instrument case. Hand-enumeration cannot prove coverage, so the checker +# sweeps tracked text for the one canonical grammar — "tracked [separately] in +# #N" — and an occurrence whose [file, issue] pair is absent from the registry +# fails the gate. The canonical form can never again be written without being +# registered. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + sweep: { "README.md" => "This gap is tracked in #909 until the decoder lands.\n" } +) +expect_fail(failures, "N+8. an unregistered canonical mention fails the sweep", out, status, + "not registered") + +# --- N+9. A registered canonical mention passes ---------------------------------- +# +# Coverage is keyed on [file, issue]: the same issue registered for a DIFFERENT +# file must not cover this one, so the entry names the corpus file exactly. + +out, status, calls = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "docs/note.md", "site" => "docs/note.md — a gap", "mentions" => 1 } + ] }, + sweep: { "docs/note.md" => "The gap is tracked in #303.\n" }, + states: "303=open" +) +expect_pass(failures, "N+9. a registered canonical mention passes the sweep", out, status) +unless calls.any? { |c| c.end_with?("/issues/303") } + puts " FAIL N+9a. the registered mention is still looked up" + failures << "N+9a: expected a lookup of #303, got #{calls.inspect}" +end + +# --- N+10. A non-canonical phrasing does not trip the sweep ---------------------- +# +# By design, not by gap: "structural safety ... is #N" is an open vocabulary no +# regex can enumerate without also sweeping up as-of citations. The registry +# stays the judgment surface for those; the sweep guarantees only the canonical +# form. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + sweep: { "README.md" => "Structural safety for this SDK is #909.\n" } +) +expect_pass(failures, "N+10. a non-canonical phrasing does not trip the sweep", out, status, + "no tracking issues referenced") + +# --- N+11. The canonical grammar is caught across a comment line wrap ------------ +# +# The phrase wraps in real prose — MIGRATING.md's own mention is "tracked\n +# separately in #775" — and a line-based grep reads that as absence. The sweep +# must see through a newline plus a comment leader mid-phrase. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + sweep: { "lib/service.rb" => "# the six SDKs disagree and this is tracked\n# separately in #909.\n" } +) +expect_fail(failures, "N+11. a line-wrapped canonical mention is still caught", out, status, + "not registered") + +# --- N+11a. The wrap is caught behind a Swift doc-comment leader too ------------- +# +# `///` is three slashes; a leader that consumes exactly two leaves the third +# in front of "separately" and reads the promise as absence. The leader must +# take comment marks by class, not by count. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + sweep: { "Sources/Feed.swift" => "/// the six SDKs disagree and this is tracked\n/// separately in #909.\n" } +) +expect_fail(failures, "N+11a. a Swift doc-comment (///) wrapped mention is still caught", out, status, + "not registered") + +# --- N+11b. The wrap is caught behind a Markdown blockquote too ------------------ +# +# `> ` opens a blockquote continuation line, and rendered prose is exactly +# where a tracking promise reads most like a promise. A leader class that +# knows comments but not quotes reads the wrapped claim as absence. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + sweep: { "docs/note.md" => "> the six SDKs disagree and this is tracked\n> separately in #909.\n" } +) +expect_fail(failures, "N+11b. a blockquote-wrapped mention is still caught", out, status, + "not registered") + +# --- N+13..16. Counts are committed, like doc-constants' marker counts ----------- +# +# [file, issue] pair coverage silently absorbed every FUTURE matching promise +# in a covered file: a second "tracked in #N" could appear and ride the +# existing row unregistered. The repo's instrument for exactly this is the +# committed COUNT — a row records how many canonical mentions it stands for, +# the sweep verifies the number in both directions, and a new mention (or a +# deleted one) fails until someone updates the row. + +two = "one gap is tracked in #303 here.\nThe other is also tracked in #303.\n" + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "docs/note.md", "site" => "docs/note.md — a gap", "mentions" => 1 } + ] }, + sweep: { "docs/note.md" => two }, + states: "303=open" +) +expect_fail(failures, "N+13. a second canonical mention fails until the count moves", out, status, + "records 1 canonical mention(s), the sweep found 2") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "docs/note.md", "site" => "docs/note.md — two gaps", "mentions" => 2 } + ] }, + sweep: { "docs/note.md" => two }, + states: "303=open" +) +expect_pass(failures, "N+14. a matching count passes", out, status) + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "docs/note.md", "site" => "docs/note.md — a gap", "mentions" => 1 } + ] }, + sweep: { "docs/note.md" => "the sentence went away in an edit\n" }, + states: "303=open" +) +expect_fail(failures, "N+15. a deleted mention fails until the count moves", out, status, + "records 1 canonical mention(s), the sweep found 0") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "README.md", "site" => "README.md — a gap" } + ] }, + states: "303=open" +) +expect_fail(failures, "N+16. a row without a mentions count fails", out, status, + "needs a `mentions`") + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ + { "issue" => 303, "file" => "README.md", "site" => "README.md — a gap", "mentions" => 0 }, + { "issue" => 303, "file" => "README.md", "site" => "README.md — the same gap again", "mentions" => 0 } + ] }, + states: "303=open" +) +expect_fail(failures, "N+17. duplicate [file, issue] rows fail", out, status, + "duplicate") + +# --- N+12. A row with no file fails ---------------------------------------------- +# +# The sweep keys coverage on [file, issue], so an entry without a file could +# never cover the mention it registers — it would pass validation and leave the +# sweep red with no way to say which entry is wrong. + +out, status, _ = run_checker( + allowlist: { "sdk_routes_known_defective" => [], "bc3_routes_not_modeled" => [] }, + tracking: { "prose_tracking_issues" => [ { "issue" => 303, "site" => "README.md — a gap" } ] } +) +expect_fail(failures, "N+12. a row with no file fails", out, status, "needs a `file`") + if failures.empty? - puts "==> known-defect tracking-issue self-test passed — 9 cases" + puts "==> known-defect tracking-issue self-test passed — #{CASES[0]} cases" exit 0 else warn "known-defect tracking-issue self-test FAILED:" diff --git a/spec/tracking-issues.yml b/spec/tracking-issues.yml new file mode 100644 index 0000000000..94b15b08a3 --- /dev/null +++ b/spec/tracking-issues.yml @@ -0,0 +1,137 @@ +# Prose claims that discharge an obligation by pointing at an issue number. +# +# A sentence like "Tracked in #792" is a promise that someone owns the gap it +# describes. If that issue closes without the gap closing, the sentence becomes +# a documentation lie and nothing notices — which is exactly what happened to +# the bc3 route allowlist when #588 auto-closed with nine live 404s pointing at +# it (see scripts/check-known-defect-issues-open). +# +# The route allowlist gets that check because its entries are structured data. +# Prose is not, so the claims are registered here instead — one entry per +# [file, issue] pair, committing in `mentions` how many canonical sentences it +# stands for — verified OPEN by the same gate, failing closed the same way. +# (Duplicate [file, issue] rows are rejected: they would make the committed +# count ambiguous. Several sentences in one file leaning on one issue share +# the row and sum in its count.) +# +# This is deliberately a REGISTRY and not a scanner. A regex over prose for +# "#\d+" would sweep up every historical citation — "shipped in #12380", "the +# fix in #760" — which are as-of facts that must never be reopened, and would +# need an ever-growing exclusion list to tell the two apart. The registry makes +# the author say which kind they meant, once, where the reason is visible. +# +# Every entry names the `file` its sentence lives in and commits a `mentions` +# count, because one detector backs this registry: the checker sweeps tracked +# text for the canonical grammar "tracked [separately] in #N" and fails on any +# mention whose [file, issue] pair is not registered here — and on any pair +# whose swept count differs from the committed one, in either direction, so a +# NEW mention cannot ride an existing row and a deleted sentence cannot leave +# its row claiming more than the file holds (`mentions: 0` marks a promise +# phrased outside the canonical grammar, registered by hand). Non-canonical phrasings are out of the +# detector's reach by design (an open vocabulary cannot be enumerated); they +# still go through this registry by hand, on the honor the header describes. +# +# Add an entry — or bump an existing row's `mentions` — when you write a +# sentence promising an issue owns something. Decrement or remove when the +# sentence goes, and the gate stops asking. +prose_tracking_issues: + - issue: 589 + file: "Makefile" + site: "Makefile — the vendored bc3-route table's freshness gate needs BC3_REPO_PATH, so it is not in CI" + mentions: 1 + - issue: 775 + file: "SPEC.md" + site: "SPEC.md §6 Retry-After — which statuses honour the header is divergent across the six SDKs" + mentions: 0 + - issue: 775 + file: "typescript/tests/retry-after.test.ts" + site: "typescript/tests/retry-after.test.ts — pins TypeScript's side of the Retry-After status divergence" + mentions: 0 + - issue: 775 + file: "MIGRATING.md" + site: "MIGRATING.md — the Retry-After migration note repeats the status-divergence promise" + mentions: 1 + - issue: 578 + file: "ruby/lib/basecamp/services/todolists_extensions.rb" + site: "ruby/lib/basecamp/services/todolists_extensions.rb — the generated validating layer that would retire writable_string" + mentions: 1 + - issue: 578 + file: "python/src/basecamp/services/todolists.py" + site: "python/src/basecamp/services/todolists.py — making the flat todolist read structurally safe" + mentions: 2 + - issue: 578 + file: "typescript/src/services/todolists-extensions.ts" + site: "typescript/src/services/todolists-extensions.ts — structural safety for this SDK" + mentions: 0 + - issue: 578 + file: "python/tests/services/test_todolist_groups_service.py" + site: "python/tests/services/test_todolist_groups_service.py — giving Python a structural decoder" + mentions: 0 + - issue: 578 + file: "typescript/tests/services/todolists.test.ts" + site: "typescript/tests/services/todolists.test.ts — structural safety for this SDK" + mentions: 0 + - issue: 578 + file: "ruby/lib/basecamp/services/merge_safe.rb" + site: "ruby/lib/basecamp/services/merge_safe.rb — the generated validating layer as the guards' intended end state" + mentions: 0 + - issue: 578 + file: "python/src/basecamp/services/_merge_safe.py" + site: "python/src/basecamp/services/_merge_safe.py — the generated validating layer as the guards' intended end state" + mentions: 0 + - issue: 578 + file: "typescript/src/services/merge-safe.ts" + site: "typescript/src/services/merge-safe.ts — the generated validating layer as the guards' intended end state" + mentions: 0 + - issue: 775 + file: "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/Pagination.kt" + site: "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/Pagination.kt — #775 carries the six-SDK Retry-After table" + mentions: 0 + - issue: 775 + file: "kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/PaginationTest.kt" + site: "kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/PaginationTest.kt — #775 carries the six-SDK Retry-After table" + mentions: 0 + - issue: 775 + file: "typescript/src/errors.ts" + site: "typescript/src/errors.ts — the strictness gate is recorded against the #775 divergence" + mentions: 0 + - issue: 799 + file: "SPEC.md" + site: "SPEC.md §6 — Python and Ruby still truncate the delay; #799 tracks the rounding convergence" + mentions: 0 + - issue: 799 + file: "MIGRATING.md" + site: "MIGRATING.md — #799 tracks the cross-SDK convergence on Retry-After rounding" + mentions: 0 + - issue: 775 + file: "go/pkg/basecamp/client.go" + site: "go/pkg/basecamp/client.go — widening the set of statuses that carry a parsed Retry-After is #775's" + mentions: 0 + - issue: 799 + file: "go/pkg/basecamp/client.go" + site: "go/pkg/basecamp/client.go — the rounding and over-range halves of the divergence are #799's" + mentions: 0 + - issue: 799 + file: "go/pkg/basecamp/client_retry_after_test.go" + site: "go/pkg/basecamp/client_retry_after_test.go — pins Go's side pending #799's convergence" + mentions: 0 + - issue: 818 + file: "SPEC.md" + site: "SPEC.md Appendix F — OAuth endpoint address enforcement beyond Go (§16 req 5–6) is pending #818's umbrella" + mentions: 2 + - issue: 819 + file: "SPEC.md" + site: "SPEC.md §23 zero-egress paragraph — the Layer-1 302 test's adapters are pending #819" + mentions: 1 + - issue: 819 + file: "conformance/event-feed/README.md" + site: "conformance/event-feed/README.md — Layer-1 seam-adapter conformance (rows 15/30 and the row-15 note) is pending #819" + mentions: 3 + - issue: 819 + file: "conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json" + site: "fixture 30's description — the below-seam zero-request proof is pending #819's Layer-1 adapters" + mentions: 1 + - issue: 819 + file: "conformance/event-feed/schema.json" + site: "schema.json fixture-30 respond variant — the Layer-1 302 test is pending #819" + mentions: 1