feat(delegate): unsubscribe, subscription introspection, and pin every wire tag

[sanity]

Problem

Three things, all in the delegate API surface, all of the same shape: something that looks like it is guaranteed and isn't.

1. The wire-format pin covered one variant out of sixteen. inbound_delegate_msg_wire_format_is_stable asserted that InboundDelegateMsg's variant 0 is ApplicationMessage. Nothing covered OutboundDelegateMsg at all, and nothing covered any variant after the first. So any reorder that happened to leave ApplicationMessage in front was undetected — swapping UserResponse and GetContractResponse, for instance, reassigns two bincode tags and makes already-deployed delegate WASM decode each as the other. Silently: the bytes still parse, into the wrong variant.

That is not a hypothetical. Exactly that swap was written and staged during the work that led to this PR, in a change whose stated purpose was to protect the wire format.

2. InboundDelegateMsg's doc comment asserted a fact about the code that was false. It said OutboundDelegateMsg was already #[non_exhaustive]. It never has been. Anyone reasoning about whether a variant addition was source-breaking got the wrong answer from the documentation.

3. A delegate cannot ask what it is subscribed to. A delegate's subscription set lives in the node, not in the delegate: the WASM is instantiated per invocation and dropped immediately after, and the node replays subscriptions across a restart without running the delegate at all. So after a restart a delegate has no way to learn its own state. It can keep a parallel record in its secrets — which drifts from the node's exactly in the cases that matter — or re-subscribe to everything on every wake. freenet/freenet-core#5467 names this as blocking restart-replay.

Approach

Pin every tag, and fail closed in both directions. delegate_msg_variant_tags_are_pinned asserts the bincode tag of every variant of both enums. Two mechanisms keep it honest, because a pin that can rot is not a pin:

• The tag map is an exhaustive match. #[non_exhaustive] has no effect inside the defining crate, so adding a variant without pinning it is a compile error.
• A probe asserts that the tag one past the last known variant does not decode. Add a variant and update nothing else, and that tag becomes decodable and the test fails. Without this, the variant-count constants would be checked only against a list written by the same hand in the same commit, which is a restatement rather than a check.

PutContractRequest is covered too. The obvious shortcut is to skip it because building a ContractContainer is awkward; it is four lines, and a pin with a hole in it reads as coverage while providing none.

Assert the compatibility rules instead of stating them. delegate_wire_compat and struct_field_wire_compat establish what bincode actually does, because appending an enum variant and appending a struct field break in opposite directions and it is easy to carry the wrong intuition from one to the other:

change	old sender → new receiver	new sender → old receiver
append an enum variant	fine, old tags unchanged	hard error, unknown tag
append a struct field	hard error, unexpected end of input	silently ignored if the struct is terminal in its message; silent corruption if it is not

A struct field is the more dangerous of the two: bincode is positional and carries no field tags, so there is nothing for a decoder to skip. #[serde(default)] does not make a field optional on this path — it is a self-describing-format feature and protects serde_json only, which is easy to misread given ContractState::size_bytes carries it. The practical rule that follows is to prefer a new enum variant over a new field on an existing wire struct, since a variant is only ever seen by a peer that asked for it.

NodeDiagnosticsResponse is terminal in its message, which is the only reason a field can be appended to it without corrupting what follows. That property is invisible at the definition site, so it is now pinned rather than assumed.

Introspection as a host function, not a message variant. DelegateCtx::list_subscriptions is backed by two new V2 host functions in the existing freenet_delegate_contracts namespace. Host functions resolve by name at module instantiation, so this is additive for every existing delegate — one that does not import it is unaffected, and one that does fails to load on a node too old to provide it, with a named missing-import error. Compare a new enum variant, which fails mid-protocol at bincode decode with no way for the delegate to have checked first. Where a capability can be expressed either way, the host function has the better failure mode.

It returns Result<Vec<[u8; 32]>, i64>, not a bare Vec. An empty list and a failed enumeration mean opposite things to a caller deciding whether to re-subscribe, and collapsing them is how a delegate concludes its user's content is unpinned because a host call failed. For the same reason the off-WASM stub returns Err, so a host-side test cannot read "no subscriptions" out of a stub that never had any.

OutboundDelegateMsg stays not #[non_exhaustive], deliberately. The false doc comment is fixed by correcting it, not by making it true. freenet-core dispatches this enum in exhaustive matches with no wildcard (contract.rs, in the request loop and again in the app-message filter). Marking it would force those to grow _ => arms, and a newly added variant would then compile against the host with no handler — the delegate's request silently swallowed while the call reports success. That is the failure mode a delegate SubscribeContractRequest has today (freenet/freenet-core#4669), and the compile error is what stops the next one. InboundDelegateMsg keeps the attribute because its consumers are third-party delegate WASM, which can reasonably ignore an unknown variant. The asymmetry is now documented on both enums, and matches the position already taken in #82.

Also corrects subscribe_contract's doc comment, which said notification delivery was "a follow-up" (it works) while saying nothing about the fact that a delegate subscription registers no demand in the network — it does not pin the contract, enter the renewal set, or exempt it from eviction, so a delegate sees remote updates only while some other route keeps the node subscribed. The call succeeds either way and nothing distinguishes the two, which is why it belongs at the call site rather than in an issue.

Compatibility

Wire change: two variants appended, nothing moved. UnsubscribeContractRequest joins OutboundDelegateMsg and UnsubscribeContractResponse joins InboundDelegateMsg, both at tag 8. Every pre-existing tag is exactly where it was, so deployed delegate WASM built against an older stdlib is unaffected — the append-safe direction of the table above. A hand-built pre-0.9.0 ContractNotification payload is asserted to still decode unchanged.

Tag 8 is a deliberate allocation, coordinated with #82: that PR appends ScheduleWakeup / WakeupFired and moves to tag 9. Recorded as a comment on #82 so it does not live only in a working session.

The pin was made to fail before it was made to pass. Adding the two variants with nothing else changed produced five E0004 non-exhaustive-pattern errors — three in production code, including the FlatBuffers encoder that the OutboundDelegateMsg doc cites as the reason that enum is deliberately not #[non_exhaustive], and two in the tag map itself:

error[E0004]: non-exhaustive patterns: `OutboundDelegateMsg::UnsubscribeContractRequest(_)` not covered
    --> rust/src/client_api/client_events.rs:1543:52   <- production dispatch
    --> rust/src/delegate_interface.rs:1481:15          <- pinned_inbound_tag
    --> rust/src/delegate_interface.rs:1496:15          <- pinned_outbound_tag

So the guard is demonstrated rather than asserted: a variant cannot be appended without both the host-dispatch site and the wire pin refusing to compile.

The two new host imports require a node whose freenet-core registers them. That is a load-time failure by design, not a silent one. The host half is not in this PR — for host functions the usual stdlib-first order is inverted, since core's linker registration references no stdlib type and can land first. Coordinated with the agent owning crates/core/src/contract.rs.

Testing

New: delegate_msg_variant_tags_are_pinned, every_variant_is_covered_by_the_pin, an_unpinned_variant_fails_this_test, an_old_payload_still_decodes_after_appending_a_variant, a_new_variant_does_not_decode_on_an_old_receiver, the five struct_field_wire_compat cases including node_diagnostics_response_is_terminal_in_its_message, and three round-trip cases for the contract-id codec.

The old-payload tests use hand-built byte strings rather than values produced by this crate's own encoder. An encoder-produced payload would only prove the code agrees with itself, which is not the property under test.

[AI-assisted - Claude]

Also in this PR: two fixes in memory/buf.rs

Flagged here because it is a file otherwise unrelated to this change, so a reviewer should not meet it by surprise.

StreamingBuffer::from_ptr is a pub unsafe fn that had no documentation, because its entire doc comment — including the # Safety contract — was attached to total_remaining, a safe getter two lines above that takes no pointer. So the getter was documented as "Create a streaming reader from a buffer pointer" with a safety contract about a ptr it does not have, while the constructor whose callers must uphold that invariant said nothing at all. A method had been inserted into the middle of another method's doc block. Splitting them back apart also clears clippy::missing_safety_doc.

The second is a false positive: the non-WASM stub must keep the mangled __frnt__fill_buffer name to match the WASM import it stands in for, so it takes a narrow #[allow(non_snake_case)] with the reason recorded.

Neither is reachable by the lint gate. ci.yml:99 passes no --features. The clippy matrix does cover both wasm32-unknown-unknown and x86_64-unknown-linux-gnu, so target coverage is fine — but default = [], so feature = "contract" is off in both legs, and both of these live behind #[cfg(feature = "contract")]. The contract-side API has therefore never been linted. Filed as #100 rather than fixed here, because CI is a shared Full-tier surface and widening the lint surfaces a backlog of unknown size.

Verification

Run locally, both the gate CI actually applies and a stricter one, because those are different questions — the first is what merges, the second is what stops the next person inheriting warnings:

• cargo clippy --target wasm32-unknown-unknown -- -D warnings — clean (CI's exact invocation)
• cargo clippy --all-targets --features contract,net,testing,trace -- -D warnings — clean
• cargo test --features contract,net,testing,trace — 111 passed, 0 failed; doc-tests 1 passed
• cargo build --target wasm32-unknown-unknown --features contract — succeeds, which is the first time anything has compiled the cfg(target_family = "wasm") branch of list_subscriptions

Note for whoever rebases #82

It appends ScheduleWakeup / WakeupFired to these same two enums, at the tag this PR leaves free. Two things to know:

• The new pin will catch a colliding append, which is the point — it fails as a compile error, not as a silent renumbering.
• feat: scheduled wakeup primitive for delegates (ScheduleWakeup / WakeupFired) #82 currently reverts four fixes merged to main since July, including the fixed_size_field decode-panic hardening and the unknown_union_discriminant change. That is branch staleness rather than intent, but it needs handling in the rebase regardless of which order the two land in.

The pins were verified to run, and to fail

A filtered cargo test that matches nothing prints 0 passed and exits 0. That is indistinguishable from a clean run by exit code, and a reviewer cannot tell the two apart by reading the output either. So every check below asserts a test count, never rc:

wire-compat tests executed:            8   (each named in the output)
struct-field tests executed:           6
list_subscriptions guard tests:       10
rc for a filter matching NOTHING:      0   <- why rc is not evidence

Counting is necessary and not sufficient — a test that runs can still be one that cannot fail. That is exactly what review found in two of these guards, so both were mutated to confirm they now fail when the thing they guard is broken:

Mutation	Result
INBOUND_VARIANT_COUNT left stale at 8 after appending a 9th variant	probe fails ✓
terminality fixture reverted to the all-default (all-zero) shape	non-zero-tail assertion fires ✓

Restored tree: 125 passed, 0 failed.

The first mutation is the drift scenario the count constants exist for; the second is the precise regression three reviewers found independently. Neither guard could previously fail for its stated reason.

A note on the external review

codex review converged on the unknown-tag probe defect independently — the strongest signal in the review, since three Claude lenses and a non-Claude model found the same hole from different directions.

Its proposed remedy would never have worked. It recommended asserting ErrorKind::InvalidTagEncoding. bincode never produces that for an enum tag: there is exactly one construction site, in deserialize_option (bincode-1.3.3/src/de/mod.rs:340). An out-of-range variant index goes through idx.into_deserializer() into serde's derived visitor and comes back as ErrorKind::Custom("invalid value: integer ..., expected variant index 0 <= i < N").

The trap is that bincode's description string for that variant reads "tag for enum is not valid" — so the external model was misled by the description exactly as this PR's original code was misled by the name. A test written to its suggestion would have failed permanently while looking correct.

Worth stating as a general point about external review: treat its diagnosis as signal and its remedy as a hypothesis. The diagnosis here was right and valuable; the fix was wrong in a way that would have been easy to apply without checking.

The host half: owner, and why it cannot be linked yet

The unsubscribe variants are useless without a freenet-core handler, and shipping a variant no host handles is the exact defect this workstream exists to remove. So, plainly:

• Owner: the agent holding crates/core/src/contract.rs, by the file-ownership rule — contract.rs:915's TODO(#2830) and the V1 outbound dispatch at contract.rs:660-679 are both in that file. Assignment confirmed by the team lead and accepted.
• Tracked as: freenet-core#2830, which specified subscribe and unsubscribe together; only subscribe was built.
• There is deliberately no PR number here, because one cannot exist yet. freenet-core cannot name UnsubscribeContractRequest until 0.9.0 is published, so the handler is necessarily downstream of this merge and release. Requiring the number as a merge condition would be a deadlock.

The handler will route both teardowns — the ring demand registration and the DELEGATE_SUBSCRIPTIONS notification hook — through a single helper rather than removing them in two places, so the two records cannot drift apart. That is the same desync class as freenet-core#5487, arriving from the opposite direction.

A correction worth recording, because it is the kind of reassurance that quietly replaces a check. An earlier version of this PR said the exhaustive match in freenet-core means a new variant "cannot compile against a host with no handler". That is too strong. It cannot compile without an arm; an arm returning Ok(()) compiles perfectly and is precisely the silent stub in question. This crate proves it — the FlatBuffers encoder has five arms that log the message and drop it. The compile error is a backstop against forgetting, and no backstop at all against stubbing.

Two findings from review that are the same lesson from opposite directions

Recorded because both are cheap to repeat and neither is visible in a passing test run.

A cosmetic edit inverted a guard. Replacing expect_err with unwrap_or_else to make a panic message interpolate turned the assertion into its opposite — unwrap_or_else unwraps Ok and runs the closure on Err, so the test would have demanded the probe succeed. The compiler caught it, but nothing about the change looked like a behavioural one.

Twelve tests existed and had never executed. The test count went 113 → 125 not because tests were added, but because the list_subscriptions guards were extracted out of #[cfg(target_family = "wasm")]. CI runs cargo test on the host only; the wasm32 matrix entries build and lint and execute nothing. The guards were type-checked and unrun — which reads as coverage in the config and provides none. See #100 for the related lint-gate blindness and #101 for four sibling sites with the same unvalidated-length shape.

[sanity]

Comprehensive PR Review: #98

Summary

• PR Title: feat(delegate): subscription introspection, and pin every wire tag
• Type: feat (wire-format guards + a new V2 host function)
• CI Status: green on the reviewed HEAD; re-running on the fix commit
• Linked Issues: freenet-core#5467, #4669, #5487; freenet-stdlib #99, #100
• Review tier: Full — touches wire format / protocol serialization on the delegate enums, an always-Full surface
• Reviewers run: code-first, testing, skeptical, big-picture (four parallel Claude lenses, each blind to the others), plus codex review as the external non-Claude pass. All five read the checked-out code in an isolated worktree.
• Disclosure: I am the author. Every finding below was raised by a reviewer that did not write the code, and I verified each against source before acting on it.

---

Code-First Analysis

Independent Understanding: four separable things — a new DelegateCtx::list_subscriptions plus two WASM imports and a shared codec; corrections to three doc comments; two test modules pinning bincode variant tags and demonstrating struct-field compat; two unrelated fixes in memory/buf.rs.

Stated Intent: matches, with one exception found by the big-picture lens (below).

Alignment: good on the wire-guard half. The list_subscriptions half was justified by a false premise — now corrected.

---

Testing Assessment

Coverage Level: adequate after fixes; two guards were vacuous before them.

Test Type	Status	Notes
Unit	✅	112 pass (was 111; off-WASM contract test added during review)
Integration	N/A	library crate; the host half lands in freenet-core
Simulation	N/A	no routing/topology surface
E2E	⚠️	the two new WASM imports cannot be exercised until the core half exists — stated in the PR rather than papered over

Regression Test: present — the shipped ContractState::size_bytes append is pinned as a concrete instance of the rule.

---

Skeptical Findings

Risk Level: medium before fixes, low after.

The wire-safety claim itself held under adversarial check: both delegate enums are byte-identical to origin/main, no variant or payload field added, removed or reordered, and no generated-file churn rode in. The problems were in the guards and the prose, not the wire.

Concern	Severity	Location	Status
Terminality pin vacuous — all-default fixture encodes as 27 zero bytes, so ends_with asserted only "ends in zeros", which survives appending any zero-encoding field	High	client_events.rs:4126	Fixed — non-zero fixture, an assertion that the fixture is not all-zero, and a length equality
Unknown-tag probe asserted only is_err(), so a variant whose payload rejects zeros fails for the wrong reason and drift goes undetected	High	delegate_interface.rs:1591	Fixed — asserts the error names an invalid variant index, plus a control that the last known tag still decodes
list_subscriptions could silently under-report: unchecked i64 as usize (32-bit on wasm32), written > len ignored by truncate, and a set that grew between the two calls returning a short list that looks complete	Medium	delegate_host.rs:743	Fixed — length validated against a new MAX_SUBSCRIPTION_LIST_BYTES, written > len rejected, exactly-full buffer re-checked, and the import contract now specifies ERR_BUFFER_TOO_SMALL
#[allow(non_snake_case)] claimed to suppress nothing	Low	buf.rs:369	Rejected — false positive. rustc rejects consecutive interior underscores; I have the clippy output showing it fires

---

Big Picture Assessment

Goal Alignment: yes for the wire guards. The list_subscriptions justification had drifted and is corrected.

The most valuable finding in the review, and the one I would not have caught: the doc claimed "the node replays subscriptions across a restart without running the delegate at all" — the entire motivation for the API. It is false. DELEGATE_SUBSCRIPTIONS is an in-memory LazyLock<DashMap> with no persistence, so a restart loses delegate subscriptions rather than replaying them. Verified in freenet-core. The API is still worth adding — it is the read side of that capability — but it does not deliver restart-replay until #4669 part 3's durable store lands, and it now says so.

Two further false claims, both verified and corrected:

• The #[non_exhaustive] rationale cited SubscribeContractRequest as a variant that compiles with no handler. It is handled (contract.rs:916-940); its defect is that it registers no demand. A different bug with a different fix. The argument stands on its own and now states two honest limits: the compile error forces an arm to exist, not a working handler — this crate's own encoder has arms that log and drop.
• The version floor named a stdlib version. Host functions register by name and reference no stdlib type, so that guarantees nothing; corrected to name the core requirement, and to say no released node provides these imports yet.

Anti-Patterns Detected: none of the CI-chasing kind. No test removed, skipped, or loosened; inbound_delegate_msg_wire_format_is_stable is present and byte-identical to main.

Scope Assessment: some creep, acknowledged. Five arguably-separable things. The buf.rs fixes and the CONTRIBUTING line were flagged in the PR body deliberately; the big-picture lens correctly notes that flagging is transparency, not focus. Splitting list_subscriptions out would be defensible.

---

External Model (codex)

Converged independently on the probe defect (P2, same location), which is the strongest signal in the review — three Claude lenses and the external model found the same hole from different directions.

Its suggested remedy was wrong, instructively. It recommended asserting ErrorKind::InvalidTagEncoding. bincode never produces that for an enum tag: it has exactly one construction site, in deserialize_option (de/mod.rs:340). The trap is that its description string reads "tag for enum is not valid" — codex was misled by the description exactly as the original code was misled by the name. A test written to codex's suggestion would never have passed. The fix asserts the error names an invalid variant index instead, which the passing test empirically confirms.

---

Documentation

• Code docs: complete, and now accurate — four claims corrected against source.
• CHANGELOG: corrected. The "additive for every existing delegate" claim is now evidence rather than prose: River's shipped chat_delegate.wasm builds against stdlib 0.8.5, which declares the five freenet_delegate_contracts externs, and wasm-objdump -x shows it imports none of them. (A first attempt to demonstrate this with a synthetic delegate was itself vacuous — the harness exported no entry point, so nothing could import, and an empty module reports zero the same way.)
• Noted, not fixed: the repo-root examples/delegate.rs no longer compiles against DelegateInterface — it uses a 5-parameter signature with a SecretsStore argument the trait has not had for some time. It is not wired into any Cargo target, so nothing catches it. Worth its own issue.

---

Recommendations

Must Fix (Blocking)

All resolved in be638f3:

1. Terminality pin vacuous.
2. Unknown-tag probe asserted only is_err().
3. Restart-replay premise false.
4. SubscribeContractRequest cited as unhandled.
5. Version floor named a stdlib version.

Should Fix (Important)

6. FFI hardening for the truncation and cast paths — done.
7. Best-effort/lossy notification delivery now documented — done.
8. "Old delegate → new host always fine" qualified: true for appended variants, not for fields appended to their payload structs — done.

Consider (Suggestions)

9. Split list_subscriptions into its own PR. Not done — it is the piece the host half is written against, and the ABI is what chokepoint needs.
10. Pin the request/response enums (HostResponse, QueryResponse, ContractRequest) the same way. Genuinely worth doing and out of scope here; the response direction is entirely unpinned today.
11. A HIGHEST_TAG_EVER_USED watermark, since removing the last variant is invisible to the whole scheme.
12. File the stale examples/delegate.rs.

10–12 are follow-ups, not merge blockers.

---

Verdict

State: Needs Changes — Re-review Required After Fix (fixes are pushed; a re-review pass on the new HEAD is required before merge, per the per-code-content rule — five blocking findings were addressed, which is well past the threshold)

HEAD SHA reviewed: 42ecc979ac9bce4e1ba80e4b27f94baaf55e570d
Fixes pushed as: be638f3

I am not merging this. The review found real defects — including two guards that would have passed while measuring nothing, in a PR whose entire purpose is to stop exactly that — and the code has changed materially since the reviewed SHA.

[AI-assisted - Claude]