Repository navigation
#28 Echo info_source_id on both facts; announce the blob horizon - #33
Merged
Merged
Conversation
co-core 0.8.0 makes info_source_id required on ContentFetchCommand and on both facts. Replicator's part is mechanical: copy it across, interpret nothing. The charter is what actually changed. tests/test_boundaries.py forbids domain vocabulary anywhere in src/, and it was right to — the rule it protects is that Replicator never learns what a domain key means. But the wire now requires naming one to carry it, so the token is exempted in exactly the three modules on the emit path and a second scan takes over the real invariant: the value may never appear in a comparison, a branch test, a subscript, or an f-string. Naming it is mechanics; keying on it is a domain model arriving one commit at a time. Also populates blob_expires_at (cannobserv#301). The clock it reports runs from the last fetch reference — store() touches the mtime on its content-addressed short-circuit — which is an event no consumer can observe, leaving them to re-derive the horizon from the contract's MUST-7 TTL and get the start point wrong. The stamp is read before store() rather than after, so the announced horizon can only fall earlier than the real one. Pin floor is 0.8.0, and unlike its predecessors this one fails loudly: the earlier floors guarded against extra="ignore" discarding kwargs silently, while a required field makes a skew a ValidationError at construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….8.0 MUST-2 is narrowed rather than dropped, and phrased by deploy epoch so it does not need revising again when watcher#252 lands: from that deploy onward a crash between minting a command_id and recording it costs the link to the occasion, not the domain object, but nothing is retroactive and a 0.7.x issuer is unaffected. The durable record stays required on narrower grounds — request options, audit, the MUST-6 reaper. MUST-3 keeps its rule and loses its last excuse: url was never a key because Archiver's url is one-to-many against InfoSources, and now the correct key is on the fact directly. The envelope-key table gains the re-key, which is a delivery-behaviour change on content.blobs rather than a rename — emissions that used to collapse now all deliver. The bare fingerprint also left the second command for identical bytes permanently unclosed, which looked like a slow origin. The boundaries charter records why info_source_id was allowed through a rule written to forbid exactly it: the field is not small, it is unread. That is the test a future proposal has to pass, so it is stated as one — does Replicator have to understand this to act on it — rather than as a list of accepted names. Three reference docs are at or over the repo's 10k context budget after this; the contract half was already over at HEAD. Flagged, not papered over. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rest The charter cited a scan that caught three of nine realistic violations. A probe of the detector found it missed a dict-literal key, a .get() lookup, a membership call, string concatenation into a Redis key, a path built from the value, and iteration over an info_source* collection — which is to say it missed every shape by which a carried value quietly becomes state, while catching the ones nobody was going to write. It now covers dict keys, positional call arguments, BinOp and JoinedStr, with a planted case for each; positional arguments prune at nested calls, or the emit path's own _publish(..., BlobAvailableEvent(info_source_id=...)) reads as its caller's violation. The vocabulary exemption was one word wide when the carve-out was one field wide, so info_source_policy or info_sources would have entered an allowlisted module unchallenged. It is now the exact identifier. blob_ttl_seconds is bounded. Since the horizon is arithmetic, an unbounded float raised OverflowError after the bytes were stored — dead-lettering a command whose blob was fine and leaving it orphaned, from a config typo. The seed harness refuses the placeholder domain key on the live target, under the same conjunction as the --production guard: a scratch run reaches no consumer, so inventing an id there is what the placeholder is for, while a real fetch broadcasts it to the cluster. AGENTS.md and both contract halves are back inside their budgets. Most of what came out was duplication the #24 split left behind — the enriched-field rules, the reason-token table and the silent-condition list each stated twice, in the read-through half and again in the lookup half — plus below-floor co-core version annotations that no longer tell an issuer anything actionable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard added last round broke the live-loop invocation documented in COMMANDS.md — the one an operator copies to exercise the real thing end to end. The suite proved the doc wrong (a test asserts exactly that refusal) and nothing noticed, which is the failure mode of testing a rule without re-reading what the docs promise about it. Exempting keyword arguments wholesale left the same hole CR #1 closed, reached by different syntax: client.get(name=command.info_source_id) reads the value under a parameter name that is not the echo. Only info_source_id= is exempt now, which is the shape the emit path actually uses, and the nested-call pruning keeps _publish(pub, topic, Event(info_source_id=...), command=...) clean at both levels. The harness refuses a blank domain key alongside the placeholder. The worker deliberately does not check this — reading the value is interpretation — but the harness is a producer, so MUST-1's "an empty id is not an id" applies to it directly. The TTL bound is documented where an operator setting it would look, since out of range now fails at startup rather than mid-handler. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three review rounds each found reading positions the deny-list had not enumerated — six, then one, then two — and each fix was correct while leaving the next gap unknown until somebody probed again. The charter meanwhile cited the scan as what made the vocabulary carve-out safe, so it advertised a guarantee stronger than it held. It now asks the opposite question. A verbatim echo has exactly four legal shapes: a field declaration, a parameter, the info_source_id= keyword, and that keyword's value. Everything else fails, including shapes nobody has thought of — probed against fifteen, among them assert, comprehension filters, set literals, del, sort keys, async for, and setdefault, none of which the scan is told about. All fifteen caught; the three legal shapes stay clean. Binding the value to a local is deliberately not legal. It is the first half of doing something with it, so a change that needs one argues with the test rather than slipping past it. Allowing a parameter is safe for the same structural reason the inversion works: uses of that name are themselves occurrences, so a helper cannot take the value and then key on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two instances of CR #10 in a second file the review had not looked at. The orientation paragraph asserted the wire carries no domain identity, which cannobserv#300 made false, and the smoke-test invocation is the same one the new guard refuses — a command a newcomer copies first and that now exits 2. Found by the ship-time doc spot-check, which is the check that exists for exactly this: a behaviour change lands with its own docs updated and the *other* file that repeated the claim keeps the old one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gregoryfoster
added a commit
that referenced
this pull request
Aug 14, 2026
Two of these were bugs in the wiring rather than the guards, and both were verified by probe before being written down. **Writers are keyed by alias, not by provider (CR #26).** AsyncGcsDriver takes a bucket in its constructor and never sees another, so a driver *is* a bucket and the key has to be whatever selects one. Keyed by provider, two gcs bindings collapsed onto a single dict entry: a command naming `public` reached the driver holding `co-gcs-internal` and wrote outside the root its own binding declared — validate_destination guards the prefix half of T3, and nothing downstream re-checks the bucket because by then it is baked into the driver. The loser was also dropped with its HTTP session open, since shutdown iterates that dict. Invisible today only because no host provisions two aliases. **Provider failures classify by HTTP status (CR #27).** Everything used to become TransientReplicateError, which loop.py exempts from the delivery ceiling — so a 403 on a misprovisioned bucket retried forever and published *no fact at all*, leaving the issuer waiting on a command that could never succeed. Reachable from the wire, too: object_options is an opaque dict, so a bad storage_class is a 400 any bus writer could use to park a command in the PEL indefinitely. Now 4xx closes (400 is the issuer's — invalid_destination; the rest are the host's — provider_disabled) while 5xx, 408, 429 and statusless failures stay open, which T4 makes safe. Read as a number rather than caught as google.api_core classes: that package is an undeclared transitive, ConditionalWriter is provider-agnostic, and 408 has no named class there at all. ValueError is left to propagate — it is the driver's own seam guard, a defect on our side, and the ceiling should see it. **"The first non-terminal fact" was never emitted (CR #28, #34).** Claimed in five places, including the previous commit message and what I reported. A transient outcome returns RETRY and publishes nothing; build_replicate_reporter still stamps terminal=True on every fact. The decision not to close an INDETERMINATE stands; the description of how it reaches the issuer was wrong. Whether that silence should become a terminal=false fact is a contract change, and is not made here. Also: a driver that cannot be built is skipped and logged rather than fatal, so an expired key file no longer stops the fetch path — load_alias_table promises exactly that one line earlier (#29). The shutdown close loop is guarded per writer; unguarded it ran before client.aclose() and one raising driver leaked the Redis client (#30). A blob swept between locate_blob and the open is blob_expired instead of a ceiling-burning handler_error (#33). The write timeout is a setting rather than the SDK's unchosen 120s (#38). Stale prose in replicate.py, the reporter, ARCHITECTURE.md, AGENTS.md and README corrected (#31, #36); read_blob deleted as dead code (#32); the object_options isinstance hoisted out of its comprehension (#35); _TERMINAL_OUTCOMES inlined (#37). Two found while fixing the above. ty caught that ReplicationCompleteEvent .public_url is a required str while GcsCreateResult types it str | None — a success carrying no URL raised ValidationError inside a publisher that re-raises, so the command retried forever; now refused unclassified before the publish. And build_completion_publisher had no test at all: every write-path test substitutes its own complete seam, so the envelope, the topic and the three echoed correlators were exercised nowhere. 683 tests, coverage 99.83%, ruff clean. Not re-run against the live bucket: the GCS interaction is unchanged apart from timeout_seconds, which a unit test asserts, and a verification run leaves an object objectCreator cannot delete (#38). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #28 once deployed. Do not merge-and-restart alone — see the deploy note below.
What lands
info_source_idechoed verbatim ontoblob_availableandfetch_failed, from the three emit-path modules. Not parsed, deduped on, keyed on, or read by any routing or policy decision.blob_expires_atpopulated (cannobserv#301). Stamped from a clock read beforestore(), so the announced horizon can only fall earlier than the real reap — a consumer acting on it re-fetches early rather than opening a deadblob_uri.>=0.8.0,<0.9, which fails loudly rather than silently: a skew is aValidationErrorat construction, not a field quietly discarded byextra="ignore".blob_ttl_secondsbounded (0 < n ≤ 10y). The horizon is arithmetic now, and an unbounded float raisedOverflowErrorafter the bytes were stored — dead-lettering a command whose blob was fine and orphaning it, from a config typo.--info-source-id, and the live target refuses both the placeholder default and a blank value.The charter is the interesting part
tests/test_boundaries.pyforbids domain vocabulary anywhere insrc/, and was right to — the rule it protects is that Replicator never learns what a domain key means. The wire now requires naming one to carry it, so the token is exempted in exactly the three emit-path modules, as the exact identifier, and a second scan took over the real invariant.That scan started as a deny-list of reading positions and three review rounds each found positions it had not enumerated (six, then one, then two). It is now an allow-list: an echo has four legal shapes — field declaration, parameter, the
info_source_id=keyword, that keyword's value — and everything else fails, including shapes nobody has thought of. Probed against fifteen violations it was never told about (assert, comprehension filters, set literals,del, sort keys,async for,setdefault, …): all fifteen caught, the three legal shapes clean.Verification
498 unit + 18 live-broker integration tests; coverage 99.77%; ruff clean;
tyat its pre-existing baseline. AGENTS.md and all three contract docs back inside their context budgets.Deploy ordering — this PR is not independently deployable
Required fields mean a half-deployed cluster does not degrade, it dead-letters: a Replicator on 0.8.0 fails
from_wireon any command from a Watcher still on 0.7.x, destroying thecommand_idcorrelator before any fact can name it. Ships with CannObserv/watcher#252, streams flushed between. Full order of operations indocs/DEPLOYMENT.md.🤖 Generated with Claude Code