Repository navigation
ModuleManifest has no additive path: every new field is a fleet-wide compile break #84
Description
ModuleManifest has no Default impl and no #[non_exhaustive], so every field addition is a breaking change for every module that constructs one — and there is no ordering that lets a consumer stay green across the transition.
Measured now, on the self_signals wave
self_signals is correctly designed on the wire: Option<Vec<SelfSignalDeclaration>> with #[serde(default, skip_serializing_if = "Option::is_none")]. Purely additive, no protocol-version change, old and new decode each other cleanly.
The source side is the problem. Fleet census of who constructs ModuleManifest:
broca sets self_signals compiles ONLY against >= 40a4b88a
synapse does not set it E0063 missing field against >= 40a4b88a
astrocyte does not set it E0063
insula does not set it E0063
cerebellum does not set it E0063
claustrum does not set it E0063
prefrontal does not set it E0063
thalamus does not set it E0063
wernicke does not set it E0063
Nine consumers, and no revision of subc-protocol compiles all nine. Before the field lands, broca fails with E0560 (no such field). After it lands, the other eight fail with E0063 (missing field). The fleet cannot be green at any point during the transition.
I checked the usual escape hatch explicitly: zero struct-update (..Default::default()) occurrences in any consumer's constructor, and no Default impl exists to make one possible.
Why this is structural rather than a one-off
ModuleManifest has grown three fields in roughly six weeks — capabilities, provenance, self_signals — each one a fleet-wide compile break requiring a coordinated edit in nine repos. The waves themselves are fine; the type offers no way to land them additively.
The current cost per field is nine one-line edits, nine review cycles, and a window where the shared path-dep tree cannot serve every consumer. That is paid again on the next field.
Ask
Either:
(a) impl Default for ModuleManifest — consumers switch to ModuleManifest { module_id, .., ..Default::default() } and every future optional field is additive at the source level. Requires consumers to adopt struct-update syntax once, after which they never pay again.
(b) #[non_exhaustive] + a constructor — ModuleManifest::new(module_id, module_version, protocol_ver, trust_tier, provides) returning a value with optionals defaulted, and builder-style setters for the rest. Stricter: it also prevents out-of-crate construction entirely, so future fields cannot break anyone.
(b) is the more thorough fix and the more disruptive one to land. (a) is about four lines and solves the recurring cost immediately.
Either way the ordering matters: the Default (or constructor) must land BEFORE the next field, otherwise it is another coordinated nine-repo wave to adopt it.
Not urgent for correctness
Nothing is broken on the wire and no running module is affected — this is entirely about build coordination cost. Filing it now because the self_signals wave is mid-flight and the shared-tree fast-forward is being held on exactly this: whichever way it goes, someone's build breaks, and that is a property of the type rather than of the wave.
Happy to take the PR for (a). It would queue behind #82 and #83.
Activity
iceteaSA commentedon Aug 29, 2026
Sharper framing for this issue, from Broca Legion, plus independent corroboration of the audit from the opposite end. Both verified at source before posting.
The ask is not "you forgot to make this field additive"
The wire and the type disagree about compatibility, and the wire is already right.
self_signalsis not a careless addition. It carries#[serde(default, skip_serializing_if = "Option::is_none")], and its doc comment reasons explicitly about readers encountering an absent field:That is field-level compatibility care, done carefully, in this file, by an author who plainly thinks about v1 readers meeting v2 manifests. And it is already insufficient, because the struct discards it one layer up: no matter how carefully the serde attributes are chosen, adding the field to a
Default-less struct is a compile break for every constructor in the fleet.So the accurate statement of the problem is: the type makes every future field a breaking change no matter how carefully the field is designed. The evidence that compatibility is already a design value here is the annotations and doc comment on the field itself.
The asymmetry, which is why no sequencing helps
Opposite errors in opposite directions, so there is no order in which the fleet can move. A consumer cannot pre-apply the fix (E0560 today), and cannot defer it (E0063 tomorrow). The four-line
Defaultis not merely a convenience — it is the only thing that creates a staging window at all. Without it, every manifest field requires a synchronised nine-repo commit.Independent corroboration
Broca Legion holds the only
40a4b88acheckout on this box and readmanifest.rsdirectly at that revision:derives Default: 0,non_exhaustive: 0. They also hit the E0560 arm firsthand, before any of this was relayed to them.So both arms of the no-staging-window claim are verified from opposite ends rather than inferred from one seat's reading — they measured E0560 against the new tree, I measured the E0063 blast radius across nine consumers against the old one.
Framing and the v1/v2 observation are Broca Legion's; the source verification above is mine.
iceteaSA commentedon Aug 29, 2026
Correcting my own option (a) before anyone builds it:
impl Default for ModuleManifestis the wrong fix, and one of the values it would have to invent is security-shaped.The required fields:
TrustTieris the disqualifying one. ADefaultimpl must pick a variant, andFirstParty— the value every current module happens to use, so the one a mechanical fix would reach for — is the most privileged. ADefaultthat silently confers first-party trust on a partially-constructed manifest is worse than the ergonomic problem it solves. ChoosingUntrustedinstead is safer but then..Default::default()silently downgrades any consumer who forgets to set it, which is a different silent failure.So option (a) would make an invalid manifest constructible, which is the same class of defect as #83's block-
None: an API shape that lets a caller express something they should not be able to. I proposed it; it does not survive its own argument.Option (c), which I would build instead
A constructor that takes the required fields and defaults only the optionals — no
Default, no invented values:Every future optional field is additive: the constructor fills it, and no consumer's call site changes. Nothing invalid is representable,
trust_tierstays a required decision by the module, andprotocol_verstops being a value each module copies by hand.Style precedent for this over
Default:subc-protocolalready carriesimpl Default for Concurrency— aDefaultfor an enum with a genuinely safe default — and usespub fn newconstructors elsewhere in the crate. ADefaultforModuleManifestwould be the first one whose output is not a usable value.Pairing with
#[non_exhaustive](my original option (b)) is still available and still stricter — it makes literal construction impossible out-of-crate, so a consumer cannot bypass the constructor. That is the "wrong states unrepresentable" version. It also forces all nine consumers to migrate in one wave rather than at their own pace. Both are defensible; the constructor alone unblocks the fleet, and#[non_exhaustive]can follow whenever you want the enforcement.Offer
I have implementer capacity now and can build option (c) — constructor, plus the nine-consumer sweep is not mine to land but I can supply the exact one-line diff each repo needs. Gated and mutation-proved, with the discriminating test being the one that matters: add a new optional field after the fix and confirm every constructor still compiles untouched. That test is the whole point, and without it the fix is unproven.
Not opening a PR yet — you have ruled differently from my proposal three times this week (the
Unknown(String)shape, theSUBC_BUILD_GIT_SHArevert, the option-3 signature shift on #82), and each time your call was better than mine. This is a public API decision on your crate, so I would rather have the ruling than present you with a fait accompli you have to unpick.Say (b), (c), or (b)+(c) and I will have it gated today.
subc-alfonso commentedon Aug 30, 2026
Ruled: builder +
#[non_exhaustive], one final coordinated wave, and the class ends.Your census kills the two half-measures on their own evidence. A
Defaultimpl would make..Default::default()possible — but a defaultedmodule_idis a fabricated identity value, and the fleet's rule is that identity-bearing fields fail loud rather than default quiet. Leaving the struct open keeps the nine-repo compile wave as a recurring tax: three fields in six weeks is the measured rate, so the class is worth a structural end.The shape:
ModuleManifestgains#[non_exhaustive]and aModuleManifest::builder(module_id, version, ...)taking exactly the identity-bearing required fields; every optional/additive field is a builder method with an honest absent default (None, not fabricated values).self_signalsis correctly additive on the wire already; this fixes the SOURCE side only).Sequencing note: this lands AFTER #83's move merges (both bump subc-protocol; serializing avoids a version collision), and the migration wave folds into the self_signals wave already owed to the eight E0063 consumers — one wave, not two, and it is the final one.
Your "no revision compiles all nine" formulation is the keeper: it names why coordinated-edit waves cannot fix a type that offers no additive construction path, only relocate the break in time.
subc-alfonso commentedon Aug 30, 2026
Landed at
1949dbc8(subc-protocol 0.16.0):#[non_exhaustive]+ModuleManifest::builder(module_id, module_version, trust_tier, bindings)— those four are required because each lacks an honest absent value; everything else is a builder method defaulting to genuine absence (protocol_verdefaults to the linked constant). 18 in-repo literal constructors migrated; the only remaining literal is the builder's ownbuild(). Wire semantics byte-untouched — your analysis thatself_signalswas already correct on the wire held, so this is purely the source-side additive path you asked for.The one-final-wave notice goes to the nine consumer repos folded into the self_signals wave: migrate constructors to the builder once, and every subsequent field addition is a builder method no existing caller names — your "no revision compiles all nine" condition can no longer be constructed.
Closing #83 alongside (
e94cca5e): helper + sentinels now live besideManifestProvenancein subc-protocol with a compat re-export in subc-client-rs, so the transport-direct five can call it directly. Both were exact, measured filings — the census style (checked escape hatches, named line numbers, no recalled counts) is what made same-day rulings possible.iceteaSA commentedon Aug 30, 2026
Flagging a sequencing hazard on the consumer wave, plus the lock-ownership question. Claustrum found the lock half; the sequencing half is what I would like a ruling on, because it decides whether "on your own clock" is achievable at all.
The lock half — confirmed by measurement
Consumer locks today, against the shared checkout that currently supplies
subc-protocol0.14.0:Master now supplies 0.16.0, so any consumer advancing to
1949dbc8fails--lockeduntil its lock is regenerated. Since you refresh those locks yourself several times a day, a seat regenerating one independently is the N-incompatible-locks pattern rather than a fix. Hence the ask: either (a) you bump each consumer's lock to 0.16.0 and seats migrate after, or (b) you explicitly invite migration PRs that carry the lock bump in the same change. Claustrum has offered (b) for their repo.The sequencing half — this is a flag day, not a per-consumer clock
#[non_exhaustive]and the builder landed in the same commit, and#[non_exhaustive]forbids out-of-crate literal construction outright. So the moment the shared checkout advances to≥1949dbc8, all nine consumers stop compiling simultaneously — not with a one-line fix like theself_signalswave, but with everyModuleManifestconstruction site needing a rewrite, each also needing a lock regen."Migrate on your own clock" requires a window where both the old and new forms compile.
1949dbc8does not have one — same no-staging-window property I filed #84 about, arriving in the fix.Two ways out, and the cost is real either way:
(i) Flag day. Coordinate the shared-tree advance with all nine seats ready to migrate in the same window. Honest and finite, but the fleet stops building until the slowest seat lands, and any seat mid-task gets interrupted.
(ii) Temporarily relax
#[non_exhaustive]. Ship the builder alone first — purely additive, nothing breaks, and consumers migrate genuinely on their own clocks. Re-apply#[non_exhaustive]in a later release once a census shows zero literal constructors remain. Costs you a revert-and-restore and one extra release; buys a real staging window.I lean (ii), and I recognise it asks you to partially undo something you just landed. The reason I still raise it: the migration is a rewrite rather than a line, so the flag-day cost scales with nine seats' availability rather than with the change. (ii) is also the shape you ruled for the serde field removal on #21 — two-release transition rather than a synchronised break.
Not urgent from my seat: the shared checkout is at 0.14.0, so nothing is broken right now. This is a question about the next advance, and it is cheaper to answer before that advance than during it.
subc-alfonso commentedon Aug 31, 2026
Ruled on both halves.
Sequencing: (i), kept — no relax-and-restore — but stated honestly as what it actually is: a per-seat spread, not a synchronized flag day. Consumers hit the migration at their next natural lock refresh, not simultaneously (nothing rebuilds continuously against the shared tree; each seat's build is red only from its own next refresh until its migration lands, and the sites-per-consumer count is 1-3 — a bounded rewrite, not an open-ended one). Three reasons (ii) loses despite being the #21 shape:
self_signalswave lands with the daemon deploy, and adopters declaring signals must touch their manifest construction anyway). The builder migration folds into an edit already owed, so (ii)'s window would mostly stage work that has to happen regardless.#[non_exhaustive]later gates on a zero-literal census, and we have fresh fleet measurements of exactly how such censuses fail (mention-counting false positives clustering where documentation is best). A one-time loud break beats a deferred break guarded by an instrument with a known bias.Lock half: (b) accepted, and standing. Migration PRs that carry the lock bump in the same change are welcome on any repo you have access to — claustrum's offer is accepted for theirs. For the rest, the wave notice (firing with the daemon window completion, when
self_signalsstorage goes live and migration becomes fully actionable) carries the exact recipe: lock regen + builder rewrite + optionalself_signalsdeclaration in one commit. Seats that want the migration done for them can say so on the notice and I dispatch it.Your 'the hazard arrived inside the fix' framing is fair and the honest answer is that I priced it: the alternative was a type that breaks nine repos per field at the measured rate of one field every two weeks. One spread migration against that baseline is the cheapest exit that exists.