fix(build): stale LEGACY_DELEGATES from undeclared registry input; missing registry now fatal - #616
Conversation
…stry fatal Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BEgtjegwuJPWSaAnVJ3z4e
Fable 5 Full-tier review — no blocking findings. 7 mutations, 7 reds.Verified empirically in a scratch real clone at this head, plus a git worktree of that clone for the dual-layout checks. That distinction matters here more than anywhere: in a worktree an unresolvable The risk this PR introduces was the main thing to check, and it holdsThe old code deliberately emitted no directives so Cargo's default heuristic would keep It doesn't. Repo-wide grep (plus skills and The publish path still stamps correctly, verified rather than reasoned — including a subtlety the PR body didn't spell out: The core fix, in the layout where it actually bitEditing a Not always-dirty: a no-op build stays a no-op (~1.8s warm, generated mtime and timestamp byte-identical across runs), so the fix doesn't re-mask the bug class it removes — which is the trap the original comment was trying to avoid. Both layouts now behave identically. Guards, all made to fail
Both pins run in CI (
|
Problem
ui/build.rsbakesLEGACY_DELEGATES— the delegate migration table — from../legacy_delegates.toml, but deliberately emitted nocargo:rerun-if-changeddirectives (.rerun_if_changed(false)), on the theory that Cargo's default heuristic "always regenerates". That theory is false: the default heuristic re-runs a build script when files inside the package change, and the registry lives at the workspace root, outside theuipackage. So editing the registry does not invalidate the cached build script.Reproduced in the main (non-worktree) checkout on this repo, before the fix:
scripts/add-migration.shedits exactly that file. So the normal migration ritual — record the outgoing delegate's hash, rebuild, publish — can ship a bundle whose bakedLEGACY_DELEGATESomits the outgoing delegate. The startup sweep then probes nobody and every returning user loses their rooms, with no error anywhere. This is the freenet/delta#52 failure mode (which stranded Delta users), reproduced in the app with the most users.A worktree cannot reproduce this: there
.gitis a file, git-derived paths don't resolve, and Cargo treats the script as always-dirty — everything looks correct from exactly the vantage point the repo's own conventions mandate. That is why it survived this long.Second route to the same outcome:
.allow_missing_registry(true)meant a missing registry file yielded an empty table with at most acargo:warning(which scrolls past unread in a release build). Same silent total-migration-failure, different trigger.Approach
Copied the shape Delta landed for this exact problem (
delta/ui/build.rson main), rather than inventing a new one:ui/build.rsnow declares every input it reads:build.rsitself, the registry (via the crate's directive,.rerun_if_changed(true)), and the gitHEAD/indexfiles that decideGIT_COMMIT_HASH— resolved viagit rev-parse --git-path, which works in both a normal checkout and a worktree (a literal../.git/HEADdoesn't resolve in a worktree and makes the script permanently dirty there)..allow_missing_registry(docs_rs_build())in bothui/build.rsandcommon/build.rs: a missing registry is now a hard build failure everywhere except docs.rs (the one environment where../legacy_delegates.tomllegitimately isn't shipped;common's TOML ships inside theriver-corepackage, so its leniency is even narrower in practice).common/build.rswas already fresh (the crate emits the registry directive by default) — it only gains the missing/empty gates.Behaviour change:
BUILD_TIMESTAMP_ISOfreshnessReviewers should see this rather than discover it. Emitting any
rerun-if-changeddisables Cargo's default heuristic, so the timestamp is no longer stamped on literally every compile. The new guarantee is: fresh whenever any declared input changes (build.rs, the registry, git HEAD/index — i.e. every commit or checkout), plus on demand viaFORCE_REBUILD_TIMESTAMP. Publishes follow a commit, so a published build is always freshly stamped. This is the same trade Delta made.Testing
Verified in a fresh clone (real
.gitdirectory — the layout where the bug lives and wherecargo make publish-riverruns; a worktree structurally cannot show it):legacy_delegates.toml→ build re-runs (4.85s), generatedlegacy_delegates.rsmtime advances, const content changes (// V1: VERIFY-REBUILD probein the diff),BUILD_TIMESTAMP_ISOrefreshes.FORCE_REBUILD_TIMESTAMP=1(21:50:46Z → 21:50:51Z).mv legacy_delegates.toml→ exit 101 withreading ../legacy_delegates.toml: No such file or directory;DOCS_RS=1builds (empty-table fallback, docs.rs only); same hard failure forcommon/legacy_room_contracts.toml(exit 101).--git-pathresolves the per-worktree paths, no-op build is 0.30s with the script not re-running — both layouts behave identically now.Pin tests (source scans — both build shapes compile identically, so only a pin can hold this):
ui/src/util.rs::build_script_declares_registry_and_timestamp_inputs— pins.rerun_if_changed(true), thebuild.rsself-declaration,FORCE_REBUILD_TIMESTAMP,--git-path(and no literal../.git/), and the docs.rs-gated leniency. Anchored to non-comment lines only, so a commented-out directive does not satisfy it (Delta's first pin had exactly that hole).common/tests/migration_test.rs::build_script_keeps_registry_fresh_and_missing_registry_fatal— same pins forcommon/build.rs.All four meaningful mutations were applied and observed to turn the pins red (
.rerun_if_changed(false), commenting out the self-declaration,.allow_missing_registry(true)in each file), then reverted.Suites:
cargo test -p river-ui --bins895 passed;river-coremigration_test5 passed,room_contract_migration_test4 passed. fmt/clippy clean.No WASM changes: no delegate/contract/common source is touched;
ui/public/contracts/chat_delegate.wasmis byte-identical (b3a44c64014d60fd245fe8fb5172f8fd7397039b248b3b6a8e95e89a0bb539fb5e), so no migration entry is needed for this PR.Scope note: this is a precursor to the freenet-migrate delegate-half adoption; it deliberately does not touch the sweep, the registry contents, or any delegate/contract source.
Risk tier: Full by surface — build/publish configuration on the data-loss-critical migration path.
[AI-assisted - Claude]