Event feed conformance: three corrections to what tier 2 claims (3/3) - #778
Event feed conformance: three corrections to what tier 2 claims (3/3)#778jeremy wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Tightens tier-2 event-feed conformance claims and closes a streaming deduplication coverage gap.
Changes:
- Clarifies redirect coverage at the poll seam.
- Rejects scheduling-dependent virtual-time advances.
- Adds fixture 31 for lower-ID live stragglers.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
go/pkg/basecamp/eventfeed/scenario_selftest_test.go |
Tests the new advance guard. |
go/pkg/basecamp/eventfeed/scenario_conformance_test.go |
Implements guarded virtual-time advancement. |
conformance/event-feed/README.md |
Revises tier-2 guarantees and fixture matrix. |
conformance/event-feed/fixtures/31-post-snapshot-straggler-below-served-id.json |
Adds streaming dedupe coverage. |
conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json |
Corrects the redirect fixture description. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
039aad6 to
4c8446c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
go/pkg/basecamp/eventfeed/scenario_conformance_test.go:494
- This observes only the net counts per timer name, not whether
NewTimerran. A timer can fire and be rearmed under the same name beforesettlesamples it (the connector does same-name rearming, e.g.repair-pollatcatchup.go:852), leaving the map equal tobeforeand allowing the scheduling-dependent advance. Conversely, removal alone is labeled as an arm. Track a monotonic timer-arm/firing generation infeedtest.Clockand add a same-name-rearm self-test so this guard detects activity independently of the snapshot schedule.
before := timerCounts(d.h.clock)
armed := false
d.h.clock.AdvanceSettling(millis(step.Ms), func() {
if !maps.Equal(timerCounts(d.h.clock), before) {
armed = true
conformance/event-feed/README.md:360
- The sentinel obligation is still present verbatim in the shared schema (
schema.json:906), and the fixture inventory at README line 239 still presents “zero foreign egress” without the seam qualification. As a result, the merged contract continues to make the exact tier-2 claim this PR says is withdrawn. Remove the sentinel requirement and qualify the inventory/schema text as driver construction rather than connector proof.
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**,
conformance/event-feed/README.md:131
- The shared schema still defines
advanceas firing timers armed inside the window (schema.json:1208) and does not mention this mandatory rejection. Drivers generated from or validated against that contract are therefore still instructed to implement the behavior this paragraph makes unscriptable. Update the schema description alongside the README so the cross-language contract is consistent.
Every driver must therefore FAIL an `advance` during which the connector arms
anything, naming `fireTimer` as the deterministic alternative — it fires one
named timer without moving the clock, so no re-selection is involved. This is
3eaf48a to
cf1d06b
Compare
4c8446c to
cfd9b3d
Compare
cfd9b3d to
be5a1c7
Compare
Round summary — 3 findings, 3 fixed, 0 openSwept all three surfaces. Both review threads answered and resolved; all three suppressed comments from the round on head are addressed, since two of them carried findings no thread did.
The one that matteredFinding 1 was a test that could not fail, and the review's sharper half was in the suppressed block rather than the thread. Proven by mutation rather than argued: with the connector's Findings 2 and 3 are one mechanism — a correction that reached the README but not Not absorbed here: #789 (arrival-strict matching)#789 is the same class as finding 1 — "the driver cannot fail the contract it enforces" — and it is now the third known instance. I am deliberately not fixing it in this PR, and the reason is mechanism, not appetite. Finding 1's remedy is a per-step verdict read from an instrument the clock already had: additive, locally provable, no new concurrency. #789's remedy is a harness-wide state machine — an active-step tracked under So it should land as its own change on top of this stack. #705 made the same call for the same reason, which is why it recorded rather than patched. One piece of design input I do owe #789, posted there: this PR's finding is evidence about the shape of the remedy. Stack / rebaseRebased twice —
Gates
No finding in this round argued from bc3 server behaviour. I re-checked anyway, since the pins moved today: between |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
go/pkg/basecamp/eventfeed/scenario_selftest_test.go:365
- This control makes the new guard timing-dependent. After
Advancedelivers the due timer, the connector handles it on another goroutine; the driver only pollsArmCountuntilscenarioWatchdogexpires (scenario_conformance_test.go:508-519). If that goroutine is delayed past the watchdog, the scenario passes and can arm a timer afteradvancereturns—the exact cross-language divergence this rule is meant to eliminate. A timeout cannot prove that no arm is coming; either reject everyadvancethat fires a timer (while still allowing truly quiet windows), or add an explicit synchronous completion protocol.
{"expectConnect":{"url":"{{CABLE_URL:2}}"}}],
"finally":{"state":"awaiting_welcome"}}`
be5a1c7 to
81414d7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
conformance/event-feed/README.md:356
- Adding row 16 leaves the matrix summary inaccurate in two ways: the heading still says “fifteen,” and the introductory sentence says every listed mutation is killed even though row 15 is now explicitly only partial/below-seam. Update the heading and introduction so this conformance document does not continue overstating the suite’s proof.
| 15 | `follow-cross-origin-redirect` (follows a 302 to a foreign Location) | 30 — **partially**, and the boundary is below 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) |
Stopping on the advance guard — third round, one classThe suppressed comment on the latest review ( The three rounds
Rounds 2 and 3 are not the same bug, but they are the same class: the driver cannot observe the quantity the rule names. Round 2 observed the wrong quantity; round 3 cannot observe the right one at all. A fourth selector on the same wait would be the treadmill. The real questionThe rule says: reject an advance during which the connector arms a timer. That predicate is decidable in a single-threaded driver and undecidable in a concurrent one — you can only ever conclude "nothing armed within N seconds". Which means the current rule has the disease it was written to cure. A rule that TypeScript decides exactly and Go decides by waiting is itself a cross-language divergence: the same fixture can be admitted in one language and rejected in another. So the question is not "how long should the wait be" — it is "what predicate can every driver decide identically and synchronously?" The candidate answer, and the measurement that makes it actionableCopilot's first alternative is the sound one: key the rule on FIRING, not arming. Every driver decides "did this advance fire a timer" synchronously, inside its own advance, on its own thread, with no waiting and no observation of the connector at all. It is sound as a conservative over-approximation because the reentrant divergence requires a firing to trigger it: no firing in the window ⇒ nothing can be armed in response ⇒ no divergence, provably, with no wait. It is strictly coarser than the true predicate, and the coarseness is the price of decidability. Its cost is that it forbids a shape currently legal — an advance whose window expires a timer that is not replaced. I measured that cost against the merged suite and it is zero. Fixture 05 is the only Both timers sit beyond the window, exactly as the fixture intends. Why I am not making the change hereIt rewrites the normative rule in What is shipping meanwhile, stated plainlyThe guard as it stands is conservative in the safe direction. Related: #789 is the same family — "the tier-2 driver cannot fail the contract it enforces" — and this is a further instance of its root shape, a predicate the driver cannot actually observe. If the firing-based rule is accepted, it should land with #789's work rather than as a fourth pass here. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
conformance/event-feed/README.md:358
- Calling this a “partial kill” contradicts the next sentence, which says the redirect-follow mutation lives below the seam and no tier-2 harness can reach it. Describe it as unreachable/not killed so the explanatory note agrees with the mutation matrix.
**Row 15 is the family's one partial kill, and the reason is structural.** In
conformance/event-feed/README.md:355
- The table still says fixture 30 partially kills
follow-cross-origin-redirect, but the note below and the PR description establish that this mutation is below the poll seam and cannot be reached at tier 2. In a mutation kill matrix, an unreachable mutant is not partially killed; fixture 30 instead covers separate verdict-handling failures above the seam. Mark this row as not killed at tier 2.
This issue also appears on line 358 of the same file.
| 15 | `follow-cross-origin-redirect` (follows a 302 to a foreign Location) | 30 — **partially**, and the boundary is below the seam. See the note under this table. |
conformance/event-feed/README.md:366
- Fixture 30 cannot detect Location over-echoing. The driver reduces the scripted
LocationtoCanonicalOriginand supplies a generic safe cause (scenario_conformance_test.go:1065-1081), while this fixture asserts only the terminal reason. A connector that echoed all data it received would therefore still pass. Remove this kill claim; the separate hostile-cause unit test owns the redaction proof.
and no tier-2 harness can reach it. What fixture 30 does kill is the half above
the seam: a connector that mishandles the verdict — retrying it, classifying it
as anything but Terminal(`invalid_continuation`), or echoing more of the
`Location` than its origin — diverges on `finally` and fails.
conformance/event-feed/fixtures/30-continuation-redirect-cross-origin.json:3
- The fixture description claims it kills Location over-echoing, but the driver strips the Location to its origin before constructing
PollErrorand the fixture asserts only the terminal reason. Thus an implementation that unsafely preserves its entire seam input still passes this scenario. Remove the redaction claim or extend the contract and driver with an assertion that exposes a secret-bearing cause.
"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) with zero egress to the foreign origin. {{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. The Location host is literal, never substituted, and never served. NOTE: 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 \u2014 this fixture kills the half above the seam (mishandling the verdict: retrying it, misclassifying it, or echoing more of the Location than its origin), not follow-cross-origin-redirect itself, which lives below the seam and is proven by the Layer-1 adapter's own 302 test. See the row-15 note in README.md.",
conformance/event-feed/schema.json:906
- This contract description still overclaims redaction coverage. The tier-2 driver passes the connector only the canonical origin plus
errors.New("poll refused a redirect"), and fixture 30 checks onlyinvalid_continuation; no path/query text reaches the connector for the fixture to prove it is not echoed. Remove that item from the pinned behaviors.
"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 half above the seam (mishandling the verdict — retrying it, misclassifying it, or echoing more of the Location than its origin), not redirect-following itself, which lives below the seam and is proven by the Layer-1 adapter's own 302 test. 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.",
Round 3's second suppressed finding — fixed in
|
e2cb78a to
9fe0a87
Compare
56ac9e7 to
6ac66a0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
spec/tracking-issues.yml:26
- The registry is not exhaustive, so the new gate still silently skips existing ownership promises. Current examples include
Makefile:183(#589),SPEC.md:640andtypescript/tests/retry-after.test.ts:285(#775), and the Python/Ruby #578 comments; the Ruby/Python #576 wording is also still present even though #576 is closed. Because the checker only reads this file and deliberately does not scan prose, none of those claims is queried. Register every live claim and rewrite/remove the stale #576 wording so the gate actually covers the repository it claims to cover.
prose_tracking_issues:
- issue: 792
site: "conformance/event-feed/README.md — the Go driver's arrival-strict gap"
- issue: 758
site: "go/pkg/basecamp/eventfeed/catchup.go awaitSupersededPoll — the superseded-poll bound's overshoot"
SPEC.md:3237
- Removing reentrant scheduling from this required checklist leaves the two normative documents inconsistent:
conformance/event-feed/README.md:140-142still requires every test clock's shared checklist to cover “reentrant scheduling within an advance,” while the next paragraph here says that clause remains normative (only fixture dependence is forbidden). Align the checklist requirement in both documents so other SDK drivers know whether that clock self-test is still mandatory.
asserted without a cross-language RNG seam. Each language's test clock passes a shared
semantics checklist (deadline order, creation-order tie-break) before its tier-2 results
count.
6ac66a0 to
390d293
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (4)
go/pkg/basecamp/eventfeed/scenario_conformance_test.go:611
DueWithinandAdvanceare separate lock acquisitions. The connector can callNewTimerafter this check releasesc.mubut before line 618 reacquires it; if that timer is due inside the window,Advancefires it even though the driver accepted the forbidden directive. This makes the new guard scheduling-dependent again. Combine the check and no-firing advance in one clock operation that holds the advance/clock locks throughout.
if due := d.h.clock.DueWithin(millis(step.Ms)); len(due) > 0 {
SPEC.md:3281
- This removes reentrant scheduling from the required clock checklist, but the clause is still normative immediately below,
conformance/event-feed/README.md:143-145still requires it in that checklist, and the Go clock still tests it directly. Fixture scripts must not depend on concurrent connector reentrancy, but clocks still need to prove the normative algorithm; keep the checklist consistent across both contract copies.
semantics checklist (deadline order, creation-order tie-break) before its tier-2 results
count.
spec/tracking-issues.yml:22
- The registry is empty even though the repository already contains present-tense tracking promises, including
SPEC.md:640for #775 andruby/lib/basecamp/services/todolists_extensions.rb:184for #576. #576 is already closed, so the new gate currently reports “no tracking issues referenced” and misses exactly the stale-prose case it claims to catch. Classify and register the existing promises, and update/remove any that already point to closed issues.
prose_tracking_issues: []
.triage.md:1
- This is a transient review ledger for PR #705, with point-in-time thread counts and unresolved-review status, rather than part of the event-feed conformance contract or implementation. It is already unrelated to this PR's stated changes and will become stale as those threads evolve; remove
.triage.mdfrom the committed SDK sources.
# PR #705 — review triage (workstream A)
…ion is withdrawn conformance/event-feed/README.md's mutation table said fixture 30 kills `follow-cross-origin-redirect` in part through "the harness obligation that the fixture's foreign origin is bound to a sentinel listener whose any-request fails the scenario". Fixture 30's own description said the same thing. No implementation meets that obligation, and meeting it would prove nothing. At tier 2 the poll lane is a SEAM: the driver receives the fixture's scripted 302 and hands the connector an already-formed redirect-refused verdict. The Go driver says so itself — "the driver is the seam, so the foreign origin is unreachable by construction". The connector never sees a Location header and never decides whether to follow one, so a silent sentinel would be a statement about the driver, not about the connector. That makes the mutation itself unreachable at this tier: it lives BELOW the seam. What fixture 30 does kill is the half above it — a connector that mishandles the verdict by retrying it, classifying it as anything but Terminal(invalid_continuation), or echoing more of the Location than its origin. The row now says that, and says where the real proof lives: the Layer-1 seam adapter's own 302 test, where a generated PollEvents call meets a real redirect. Tracked for G1b. The fixture's description is amended too. Fixtures are merged contract and are not edited lightly, but leaving the identical false claim in the file the other five SDKs read would make this a half-correction — and the edit is prose only, with no assertion changed: `make event-feed-fixtures-check` passes, both pin probes still verify, and all 22 fixtures still replay green. This also corrects PR #705's own kill-matrix, which inherited the claim.
#765 item 2. The Go driver ran `advance` through the plain clock Advance, with a comment noting that no current fixture scripts a chained firing and that one which did "would pass AdvanceSettling the rendezvous for the arming". That is an opt-in, and the next fixture author is exactly who would not take it. Stress-testing the settle killed it, and the reason is fundamental rather than effort: - Waiting for a firing to be CONSUMED deadlocks. The connector deliberately does not consume a staleness firing promptly — a window closing while the consumer is inside a delivery is latched and observed later, which is what §23 requires and what staleHolder implements. A clock that waited would hang against the behavior the spec mandates. - Waiting for the follow-on ARMING requires knowing one is coming. Nothing distinguishes "has not armed yet" from "will not arm", so such a wait is either a guess or the same hang. So the reentrant clause is not settleable where the connector runs concurrently, and the divergence is made unscriptable instead of unhittable: an advance during which the connector arms anything now FAILS, naming fireTimer as the deterministic alternative (one named timer, no clock movement, no re-selection). Checked on every advance — a schema field would let a fixture author take the divergence rather than avoid it, which is the thing being prevented. The family README carries the constraint, because it binds every driver and not just this one: a fixture relying on the reentrant clause means different things in a single-threaded test clock and a concurrent one, and the six SDKs are split across both. Self-tested in both directions, since a guard that cannot fail proves nothing: the mutant (an advance across the handshake deadline, which arms `backoff` inside the window) is rejected, and the control (an advance over a window that arms nothing) still passes — without which the guard would be rejecting every advance, including fixture 05's.
Closes the tier-2 hole where a dedupe that orders live ids against the highest poll-served id passes the whole suite. Fixture 20 already proves a post-snapshot straggler is delivered — but its entry page serves NO events, so a highest-served-id implementation passes it too: with nothing served the mark is unset and every id clears it. Every other straggler in the suite is buffered pre-cut (01, 12, 19), where the same implementation also survives. 31 serves id 99 on the entry page and pushes 41 afterwards, in the streaming lane. 41 must be delivered — dedupe tracks ACTUALLY-DELIVERED ids and 41 was never served by poll — while a re-push of 99 is still suppressed, which is what keeps this a dedupe fixture rather than a no-dedupe one. The kill was verified rather than asserted, and the first two attempts were wrong in instructive ways. A mutant in admitLive survived: that is the buffer-admission path, and a streaming straggler never goes through it. A mutant in the shared deliver() gate failed SIX fixtures, so it was broader than the hole and would not have justified a new fixture. The mutant that matches the claim — the ordering applied only to the streaming lane, which is the plausible shape, since a buffered straggler is visibly pre-cut while a streaming one looks like an old duplicate — passes all of 01–30 and is killed by 31 alone. The two "mechanically derived" fixture lists in the README are re-derived, per their own instruction.
The guard added for the scheduling-dependent advance compared snapshots of the clock's outstanding-timer set, which answers a different question than the rule it enforces — and gets the rule's own two cases backwards. A firing removes its timer from the registry before any observer runs, so an ordinary expiry that arms nothing reads as an arm. That is not a theoretical inversion: it is what the self-test was actually exercising. Deleting the connector's backoff arming entirely leaves the test passing, because the handshake deadline's own removal trips the comparison. The test asserted nothing about arming. The other direction is the one §23 cares about more. A timer rearmed under a name it already had leaves the set byte-identical, so the connector's same-name rearms — repair-poll, poll-retry — are exactly the arms a set comparison cannot see. Measure the event instead: feedtest.Clock exposes the monotonic count of timer creations it already maintained for tie-breaking, and the guard compares that across the window. Arming is then visible whatever the name, and an expiry is not mistaken for one. The wait after Advance is the other half — the connector arms on its own goroutine, so an arm caused by a firing inside the window can land just after Advance returns — and it ends the instant an arm appears, so only a legitimately quiet advance pays it. The suite contains one such advance. Three self-tests now, where there were two: the rejection, a quiet window, and the control that separates the instruments — an advance whose window fires a due timer that is not replaced, which must pass and which the previous guard rejected.
README says plainly that schema.json is the contract, and a driver is built from it. Two of this branch's corrections stopped at the prose, so the merged contract still issued the instructions they withdraw. The 302 respond variant still carried the sentinel obligation verbatim — the harness must bind the foreign origin to a listener whose any-request fails the scenario — which row 15 withdraws as unmeetable and, if met, a statement about the driver rather than the connector. Its description now carries the seam boundary instead, and the fixture inventory's row 30 no longer offers "zero foreign egress" unqualified: that property holds by construction of the seam and is proven at Layer 1. The advance directive still described only the reentrant clause, so a driver generated from the schema was still told to implement the behaviour this branch makes unscriptable. It now carries the mandatory rejection, and names the instrument: count timer creations, never compare outstanding-timer sets. That last sentence is the whole of the preceding commit, and it belongs here because every other language's driver will reach for the set first — it is the obvious thing to reach for, and it is wrong in both directions. Swept the family for the rest of the class rather than patching the two that were reported: these were the only remaining sites.
Row 16 arrived without its heading, and row 15 stopped being a whole kill without its introduction. Both were left overstating the suite, in a document whose entire job is to say precisely what tier 2 proves. The heading said fifteen against sixteen rows. The introduction said every mutation is shown red against at least one fixture before it counts, which row 15 no longer satisfies in the sense a reader would take: its mutation lives below the tier-2 seam, so this suite kills the half above it and the Layer-1 adapter's own 302 test kills the rest. Both now say so. Counting the partial row as a kill would have been exactly the class of claim this family exists to check, which is why the exception is stated at the top rather than left to be discovered in the row. Swept the family for the rest of the class first: these were the only two places the matrix's size or completeness is asserted.
…ot fail This is the third narrowing of row 15, so it is rewritten from the driver's code rather than trimmed again. Successive narrowing is why it kept overstating: each pass removed the overclaim it was shown and left the rest standing, so the row was never re-derived from what fixture 30 actually does. Two things were wrong. **"Partial kill" was the wrong classification.** The mutation lives below the poll seam and no tier-2 harness can reach it. An unreachable mutant is not half-killed; fixture 30 is named against it because it pins a different fault class at the same boundary, not because it kills a fraction of that mutant. The row, the matrix heading's exception, and the note now all say not killed at tier 2, and name Layer 1 as the owner. **The redaction claim could not fail.** The note, the fixture description and the schema all said fixture 30 kills a connector "echoing more of the Location than its origin". It cannot. `redirectRefusalFrom` reduces the scripted Location to `CanonicalOrigin(location)` and hands over a generic cause, so no path or query text ever reaches the connector: an implementation that echoed its entire input verbatim passes fixture 30 unchanged. A fixture that pins the exact class of claim this PR exists to remove is worse than one that pins nothing, because it is counted as coverage. The real proof already exists and is not vacuous — `TestRedirectRefusalExposesOnlyTheLocationOrigin` feeds a secret-bearing cause and asserts the terminal's whole rendering and cause chain never carry it — so the claim moves there rather than being invented anew. What fixture 30 does pin is now stated positively and checked against its `finally`: the verdict must be classified Terminal(invalid_continuation) and must not be retried, which `mintCount`/`connectCount`/`timers`/`socket` enforce. Swept for residual over-echo claims across the family: none remain.
Row 15 cited "the Layer-1 adapter's own 302 test" in the present tense, as a proof the repository already contains. It does not. `doc.go` lists the Layer-1 adapters over the generated CreateStreamTicket and PollEvents operations among the pieces still to land, AGENTS.md calls them pending, and no adapter file exists. The row's own closing line said "Tracked for G1b" — the sentence tracking the work and the sentence claiming it was done sat four lines apart. That is the same defect as the redaction claim removed in the previous commit, which is what makes it worth saying out loud rather than just fixing: the rewrite there was checked against the driver's code and came out right about tier 2, while every claim it made about ANOTHER layer went unverified. Verifying a document against the layer it lives in cannot catch a false claim about a layer it does not. So all four cross-layer proof claims — the fixture inventory's row 30, the kill matrix's row 15, the redaction paragraph's tail, and the zero-egress paragraph — now state an assigned obligation with its pending status, and the one test that does exist is marked as existing. The distinction is load-bearing for a reader deciding what this suite has established. Swept for residual present-tense Layer-1 proof claims across the family: none remain.
…can see The withdrawn sentinel obligation had a third home, in different words. The placeholder section told every harness to "assert those hosts receive zero requests" for any literal foreign origin — which is exactly the obligation row 15 withdraws for fixture 30, phrased without the word sentinel or egress, which is why two term-based sweeps walked past it. The rule is right for the fixtures it was written for and wrong only where it was over-extended. In 26 and 27 the connector itself holds the hostile URL as a continuation target: it could poll it, no expect step serves it, and zero requests is a real structural property of the connector. In 30 the foreign origin reaches the connector only through a Location the driver has already reduced to an origin, so there is no egress for a harness to observe and asserting its absence describes the driver. So the obligation is now scoped to connector-visible continuation targets, with the seam-converted case named and pointed at the row-15 note, rather than the blanket "any literal foreign origin" that contradicted its own withdrawal four hundred lines later. Found by sweeping the concept instead of the vocabulary: the literal hostile origin plus every phrasing of a harness obligation, which enumerates five sites and shows the other four are correct as written.
…oing The branch changed the README and the schema and never touched SPEC §23, so each of them now contradicted it outright. Both are the spec's side of a call already made and already enforced elsewhere. **Reentrant scheduling.** §23 listed it among the semantics every language's test clock must pass before its tier-2 results count, while the schema's `$defs.advance` requires every driver to HARD-FAIL an advance during which the connector arms a timer. The distinction the spec was missing is that the clause is normative for the ALGORITHM and forbidden as a FIXTURE DEPENDENCY: a clock that ignored it would fire the wrong set, but whether a timer armed during the window lands inside it depends on when the connector's goroutine got scheduled, which no fixture can pin. The checklist keeps deadline order and the creation-order tie-break; the reentrant clause moves to its own paragraph with the driver obligation, including the detection rule — count timer CREATIONS, never compare the outstanding set, because a firing removes its timer before any observer runs and a same-name rearm leaves the set identical. **Fixture 30's zero egress.** §23 required "zero requests to the foreign origin" as tier-2 coverage. Tier 2 cannot deliver it, and the README's row-15 note already says why: 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`, so the foreign origin is unreachable by construction of the harness — a harness asserting nothing reached it would be asserting something about itself. The requirement becomes what tier 2 can actually hold (terminal reason, no retry, no further poll), and the egress proof is named as the Layer-1 obligation it is, pending G1b. **The published memory ceiling was short by one frame.** It read (pump depth + LIVE_BUFFER_CAPACITY) × MAX_FRAME_BYTES. The pump is a single reader, so it can hold one frame it has already READ and is blocked handing off, over and above the queue's depth — which is precisely why the drain's protocol-fatal scan is budgeted at pumpDepth+1 rather than pumpDepth, and that comment has been in the tree explaining the same +1 since #760. It is one rather than unbounded for the same reason: one reader holds at most one frame outside the queue. The three code comments restating the formula are corrected with it.
Correcting pump depth to pump depth + 1 named the pump's in-flight frame and stopped there. There are two frames the queue's depth does not count, and daybreak's finding is that they are held at the SAME TIME rather than being alternatives: the pump's held frame, and the deferred socket outcome parked in the single slot the in-flight-poll servicing and the drain's scan share. The slot is retained while the queue behind it refills, so the stable bound is pump depth + 2 + live capacity. The scan's budget stays at pump depth + 1, and the SPEC now says why that 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. Both numbers are correct for their own question, which is exactly the confusion a bare "+1/+2" mismatch would otherwise invite. The advance guard also stops reading as a proof. It waits out the family's wall-clock watchdog for an arm and treats silence as "never armed", which is a heuristic and cannot be otherwise from outside — the connector arms on its own goroutine, and the only alternative is the rendezvous a scheduling-dependent fixture would need, which is the thing the guard refuses. The doc now names the bound and the failure direction: a missed arm lets such a fixture through, and it then fails non-deterministically rather than silently meaning different things in different languages.
The known-defect gate verifies that every issue the bc3 route allowlist leans on is still OPEN, because #588 auto-closed while nine live 404s pointed at it and "is an Integer" was the entire test. This branch and its siblings now make the same class of claim in PROSE — the README says the arrival-strict gap is "Tracked in #792", and awaitSupersededPoll's doc leans on #758 for the overshoot — and nothing checked those at all. Same failure, same remedy, same script: the issue closes, the sentence keeps promising someone owns the gap, and no one is told. spec/tracking-issues.yml registers the promises and the existing gate verifies them alongside the allowlist's, reusing its fail-closed behavior and its "name the site, not just the number" reporting unchanged. A registry rather than a scan, and the file says why: `#\d+` over prose cannot separate a tracking promise from an as-of citation — "shipped in #12380", "the fix in #760" — and telling them apart needs the author, not a matcher. That is the same distinction AGENTS.md draws for bc3 revisions in prose, for the same reason. Two things the extension had to fix in the test rig rather than add to it. The self-test never set the registry path, so the checker would have read the repository's real one mid-test, making hermetic cases depend on live issue state and the network — precisely what the gh stub exists to remove. And the summary line restated its case count by hand, so it said "9 cases" while running twelve; it now counts what ran. Verified red against a genuinely closed issue (#784) before wiring, not only against the stub.
…o fail closed
A missing registry file became `{}`, so the checker reported "nothing to
verify" and exited 0 with every prose claim in the repository unverified.
Delete the file, or typo the override, and the gate went quiet — the #588
failure with one extra step, in the script written to end it. daybreak
reproduced exit 0; the file is now required, and its absence is a failure with
the same wording as an unreachable API.
Rows are validated rather than skipped, and that differs from the allowlist on
purpose. There, `next unless Integer` is right: an entry with no issue number
is making no claim. Here the row IS the claim, so a malformed one is a promise
the author wrote and the gate silently dropped — worse than either a failure or
an absent row. A row now needs a positive integer `issue` and a non-empty
`site`, because a bare number tells the next reader nothing to go fix.
Four self-test cases cover it: a missing registry, a row with no site, a row
with no issue, and a non-list registry. The runner also had to gain the ability
NOT to write the file, which it previously always did — so the missing-registry
case is expressed without deleting the repository's real one.
…s with it #792 is implemented and closed, and the README sentence that leaned on it is gone. The registry entry has to go in the same breath: it names a site that no longer says anything, and the gate would have failed on the closed issue at the next run — which is the gate working, not the gate complaining. That coupling is the point of registering prose claims rather than scanning for them. A promise and its tracker are removed together or the gate objects.
It sampled the clock's arm count, advanced, waited out the family's wall-clock watchdog for the count to move, and failed if it did — so "no arm within five seconds" meant "no arm", and a later arm was simply missed. No wait makes that sound: there is no instant at which "nothing further will be armed" becomes knowable from outside a connector that arms on its own goroutine. daybreak was right to call it out; the previous round only wrote the weakness into a comment. The question changes instead. "Did the advance cause an arm?" is racy. "Can this advance fire anything at all?" is one atomic read of the clock, taken under the same lock the advance selects under, before time moves. The 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 never releases that lock, never wakes anything, and so cannot be the cause of any arm. There is nothing left to detect, which is why rejecting the shape is complete where detecting the divergence could only ever be probable. It is stricter than the arming rule it replaces: a firing that replaces nothing is now rejected too, where before it was expressly legal. That case had been the control distinguishing arm-counting from set-comparison, and it becomes a rejection — the third self-test arm now pins that. A script that wanted such a firing writes `fireTimer`, which fires one named timer without moving the clock and says which timer it means, so the loss is expressiveness nobody was using and legibility gained. Fixture 05 is the suite's only advance and it qualifies: Streaming, with staleness and repair-poll configured to ~11 days against a 121-second window, existing to age a ticket past its TTL rather than to fire anything. Verified before writing the rule rather than after. `ArmCount` and its clock test go with the guard that was its only caller — it was built for this and nothing else, and a test double keeping an API for a deleted mechanism invites the next reader to use it. README, schema and §23 move together, since the obligation binds five drivers still to be written.
#758 and #792 are implemented, and the two sentences that leaned on them are gone with them — `awaitSupersededPoll`'s overshoot paragraph replaced by the published bound, and the README's arrival-strict gap note removed. Their entries go in the same breath, which is the coupling the registry exists to enforce: a promise and its tracker are removed together, or the gate objects the moment the issue closes. The file stays, with an empty list. It is a habit rather than a one-off — the next sentence that promises an issue owns something registers here, and the first self-test case already pins that an empty registry is a legitimate no-op rather than a missing one.
390d293 to
805e074
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Suppressed comments (4)
go/pkg/basecamp/eventfeed/scenario_conformance_test.go:611
DueWithinandAdvanceare separate critical sections. AfterDueWithinreturns empty, the connector goroutine can arm a timer inside this window beforeAdvancereacquires the clock lock;Advancethen fires that timer even though the directive was accepted. A concurrent stop can produce the inverse false rejection. Make the due check and quiet-window advance one atomic clock operation so the new MUST is deterministic.
if due := d.h.clock.DueWithin(millis(step.Ms)); len(due) > 0 {
SPEC.md:3231
- This statement withdraws tier-2 zero-egress coverage for all three preceding hostile-URL cases, but the PR and family README retain that obligation for connector-visible
next/resumetargets (fixtures 26/27). Only fixture 30's redirectLocationis hidden below the poll seam. Scope the Layer-1-only statement to the redirect and retain zero foreign poll calls for fixtures 26/27.
**Zero egress to the foreign origin is a Layer-1 obligation, not tier-2 coverage**, and
this paragraph used to require it here. Tier 2 cannot deliver it: the poll lane IS the
SPEC.md:3281
- The family README still requires the shared clock checklist to cover “reentrant scheduling within an advance,” and the Go clock retains a direct
AdvanceSettlingtest for it. The following paragraph also says the clause remains normative; only fixture dependence is forbidden. Removing it from this checklist creates conflicting six-SDK conformance requirements, so keep the direct clock semantic in the checklist.
semantics checklist (deadline order, creation-order tie-break) before its tier-2 results
count.
SPEC.md:2856
- The
+ 2ceiling still misses one simultaneously retained raw frame.fatalScandeliberately continues whenl.deferredis occupied; when it dequeues from a full channel, the blocked pump refills that slot and can read/hold its next frame while the scan still holds the dequeueditem. The peak is therefore full queue + deferred + scan-local item + pump-held frame (pump depth + 3), alongside the live buffer. Either prevent the pump from reading again during that scan or update the bound and all mirrored comments/tests.
(pump depth + 2 + `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. The **+ 2** is two raw frames the queue's depth does
not count, and they are retained by different parties at the same time:
| rows = tracking['prose_tracking_issues'] | ||
| unless rows.nil? || 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| |
| # | ||
| # Add an entry when you write a sentence promising an issue owns something. | ||
| # Remove it when the sentence goes, and the gate stops asking. | ||
| prose_tracking_issues: [] |
Third of the split. Stacked on #705, which is stacked on #777 — review after
those. Three corrections to the tier-2 conformance family, all of them about the
suite claiming more than it proves.
1. Row 15's kill claim is partial, and the sentinel obligation is withdrawn
The mutation table said fixture 30 kills
follow-cross-origin-redirectpartlythrough "the harness obligation that the fixture's foreign origin is bound to a
sentinel listener whose any-request fails the scenario". Fixture 30's own
description said the same.
No implementation meets that obligation, and meeting it would prove nothing.
At tier 2 the poll lane is a seam: the driver receives the fixture's scripted
302 and hands the connector an already-formed redirect-refused verdict. The Go
driver says so itself — "the driver is the seam, so the foreign origin is
unreachable by construction". The connector never sees a
Locationheader andnever decides whether to follow one, so a silent sentinel is a statement about
the driver, not the connector.
That makes the mutation unreachable at this tier — it lives below the seam.
What fixture 30 does kill is the half above it: mishandling the verdict by
retrying it, misclassifying it, or echoing more of the
Locationthan itsorigin. Real zero-egress proof is the Layer-1 adapter's own 302 test, tracked
for G1b.
The fixture's description is amended too. Fixtures are merged contract and are
not edited lightly, but leaving the identical false claim in the file the other
five SDKs read would make this a half-correction. Prose only — no assertion
changed, both pin probes still verify.
This also corrects #705's own kill-matrix, which inherited the claim.
2. An advance that arms a timer is rejected, in every driver
The driver ran
advancethrough the plain clockAdvance, with a commentnoting that a fixture wanting a chained firing "would pass
AdvanceSettlingtherendezvous". That is an opt-in, and the next fixture author is exactly who would
not take it.
Stress-testing the settle killed it, for reasons that are fundamental rather
than effort:
does not consume a staleness firing promptly — a window closing during a
delivery is latched and observed later, which is what §23 requires. A clock
that waited would hang against the behavior the spec mandates.
distinguishes "has not armed yet" from "will not arm".
So the reentrant clause is not settleable where the connector runs
concurrently, and the divergence is made unscriptable instead: an advance
during which the connector arms anything now fails, naming
fireTimeras thedeterministic alternative. Unconditional — a schema field would let an author
take the divergence rather than avoid it.
The family README carries the constraint, because it binds all six SDKs and they
are split across single-threaded and concurrent test clocks.
Self-tested both ways: the mutant (an advance across the handshake deadline,
which arms
backoffinside the window) is rejected, and the control (a quietwindow) still passes — without which the guard would be rejecting every advance,
including fixture 05's.
3. Fixture 31 — a straggler below the entry page's served id
Closes the hole where a dedupe ordering live ids against the highest poll-served
id passes the whole suite.
Fixture 20 already proves a post-snapshot straggler is delivered, but its entry
page serves no events, so a highest-served-id implementation passes it too:
with nothing served the mark is unset and every id clears it. Every other
straggler in the suite is buffered pre-cut.
31 serves id 99 on the entry page and pushes 41 afterwards, in the streaming
lane. 41 must be delivered — dedupe tracks actually-delivered ids and 41 was
never served by poll — while a re-push of 99 is still suppressed, which keeps
this a dedupe fixture rather than a no-dedupe one.
The kill was verified, and the first two attempts were wrong in instructive
ways. A mutant in
admitLivesurvived — that is the buffer-admission path, anda streaming straggler never goes through it. A mutant in the shared
deliver()gate failed six fixtures, so it was broader than the hole and would not have
justified a new fixture. The mutant that matches the claim — the ordering applied
only to the streaming lane — passes all of 01–30 and is killed by 31 alone.
Verification
go build/go vet/go test -race -count=1pass; 23/23 fixtures green;5 driver self-test groups pass;
make event-feed-fixtures-checkclean with bothpin probes verified;
make go-lint0 issues.Summary by cubic
Aligns tier‑2 conformance to what the suite actually proves and removes unscriptable timing. Clarifies fixture 30’s scope, enforces deterministic virtual time, plugs a dedupe hole, corrects the memory ceiling, and adds a CI gate for prose tracking promises.
nextthat answers 302 with a cross‑origin Location must end Terminal(invalid_continuation), with no retry and no further poll. Zero‑egress and redaction are not tier‑2 claims; both move to Layer 1. Kill matrix marks row 15 (“follow‑cross‑origin‑redirect”) as not killed at tier 2.README,schema.json,SPEC.md, and fixture 30’s prose updated.advancewhose window would fire a timer, decided from the clock’s state before time moves; namefireTimeras the deterministic alternative.schema.json/SPEC.mdcodify this; the Go driver implements the pre‑advance “would fire” check with self‑tests.SPEC.mdand code comments updated.scripts/check-known-defect-issues-opennow also verifies prose “Tracked in #N” claims registered inspec/tracking-issues.yml, requires the registry file, validates rows, and fails closed. Self‑tests updated.Migration
advancewithfireTimer. Update drivers to reject anadvancethat would fire timers using a pre‑advance due‑timers check.spec/tracking-issues.ymlfor any prose “Tracked in #N” references.Written for commit 805e074. Summary will update on new commits.