[jun-mercury]

Draft. The four tests are red by design: they reproduce a bug this pull request does not fix, and a wrapper keeps the suite green until a fix lands.

Why

A ghc-worker server that outlives one build serves the first build's view of a unit to the next. The unit env in the make state's HUG is inserted by insertUnitEnv and never evicted; mergeModuleGraphNodes keeps the old ModuleNode, so a module's old ModSummary and preprocessed source survive; the interpreter's loader state is never unloaded; the HPT is trusted. The code assumes buck-proxy's lifecycle, one worker per build (socket named after BUCK_BUILD_ID, killed on SIGTERM unless --remain), and the comment in Cache/Hpt.hs says the state is "initially empty" when a new build starts. The suite models that too: Test.Env.newResumeSessionEnv creates a fresh WorkerState for the second build, "simulating a worker restart". A worker kept with --remain, or a pool of servers on remote execution (our case), breaks the assumption, and the second build's output is wrong without any diagnostic.

This pull request adds the missing shape to the suite: one WorkerState, two builds of the same unit with a change between them, the second build's interface compared with a fresh WorkerState's. It proposes no fix; the reviewers own that design.

Scope

• ghc-worker/test/StaleUnitTest.hs, new. Build (a unit's extra args, module sources, compiles in order) fed through computeMetadata and compileModuleWithDepsInHpt the way Test.Build does, Iface (sorted exports and ABI hash read with readBinIface), staleSequence, and the wrapper stillStale.
• ghc-worker/test/Main.hs and ghc-worker/ghc-worker.cabal register the module. The cabal edit is by hand; if the file is hix-generated at HEAD, regenerate it.
• No new dependency. tasty-expected-failure would have been the idiom, but adding a package touches ops/packages.nix, haskell/BUCK and the generated cabal files, so the wrapper is ten lines over Hedgehog.Internal.Property.runTestT: it passes while the inner assertions fail, prints their report to stderr so the stale output stays visible on a green run, and fails once a sequence comes out fresh, which is the signal to delete it. expectFailures = False shows the raw failures.

The four cases, group "stale unit state across builds":

1. "source changes, same args: the second build exports the new binding". K gains a binding; the recompiled K lacks it. The long-lived worker's ABI hash is the first build's.
2. "unit args change: -DFOO added, the second build exports the CPP-gated binding". Same source, -DFOO added to the unit's args; K recompiles with the old flags.
3. "TH splice reads a changed module: the second build's splice sees the new value". A declaration splice in M names a binding after K's value; K changes from 1 to 100; both recompile; the splice still produces spliced_1. A value-only change does not move the ABI hash under the suite's flags, which is why the splice writes the value into a name.
4. "module added to a known unit: the importer's second build sees the new module's binding". K2 joins the unit and M imports it; M recompiles from its first-build source and lacks the export that needs K2.

Blast Radius

Test-only change; nothing in the server or the libraries moves. The suite stays green because of the wrapper. The cost of leaving the bug is that any deployment with servers that outlive a build ships stale interfaces and objects, and the .hash file the server writes beside them agrees with the stale output, so a build system caching on it cannot notice.

Verification

Run on the 85f41f9 lineage with GHC 9.10.3 (this branch's base is upstream HEAD; see below):

nix build ... ghc-worker  # doCheck, testFlags ["-p", "/stale unit state/"]

With expectFailures = False, "4 out of 4 tests failed", every failure at longIface.exports === freshIface.exports:

source changes, same args: the second build exports the new binding:              FAIL
    │ - [ "value_1_1" ]
    │ + [ "value_1_1" , "value_1_1_1" ]
  fresh worker:      Iface {exports = ["value_1_1","value_1_1_1"], abi = "e93372c42d978dcc411506b56b5270fa"}
  long-lived worker: Iface {exports = ["value_1_1"],               abi = "46570b89cd1b11b2f91994f7f02a9dc8"}
unit args change: -DFOO added, the second build exports the CPP-gated binding:    FAIL
    │ - [ "value_1_1" ]
    │ + [ "value_1_1" , "value_1_1_foo" ]
  fresh worker:      Iface {exports = ["value_1_1","value_1_1_foo"], abi = "0dd85c65a5052a5cbaa20cf2de5e078a"}
  long-lived worker: Iface {exports = ["value_1_1"],                 abi = "46570b89cd1b11b2f91994f7f02a9dc8"}
TH splice reads a changed module: the second build's splice sees the new value:   FAIL
    │ - [ "spliced_1" ]
    │ + [ "spliced_100" ]
  fresh worker:      Iface {exports = ["spliced_100"], abi = "d48954426a065730d4e69a8f9f1d68a7"}
  long-lived worker: Iface {exports = ["spliced_1"],   abi = "a8454500a523cec22707fb934b5577e9"}
module added to a known unit: the importer's second build sees the new module's binding: FAIL
    │ - [ "value_1_2" ]
    │ + [ "value_1_2" , "value_1_2_3" ]
  fresh worker:      Iface {exports = ["value_1_2","value_1_2_3"], abi = "2905a95ffa2fc0ed87ddcc5a5988cb4f"}
  long-lived worker: Iface {exports = ["value_1_2"],               abi = "5d9250fbf6d48409457284cec83f2bf3"}

As shipped (expectFailures = True): "All 4 tests passed", "Test suite ghc-worker-test: PASS", with four still stale: <case> reports on stderr carrying the same two Iface values.

The same sequences on a real unit, outside the suite. src/Mercury/Sentry/Constants from our monorepo (three modules, no project deps, no package dbs), one hand-started server fed the plan buck2 wrote, each build compared with one-shot ghc -c of the same tree; the same result from an upstream 85f41f9 server and from our fork's:

• A, the plan as written, and B, Fingerprints.hs changes its export's type: server and one-shot agree (B: ABI 861d0cff..., exports defaultV2 default_).
• C, the unit's args gain -DSENTRY_V2 and Fingerprints.hs a CPP-gated export: server ABI 861d0cff... (unchanged from B), one-shot 0a5b2f81...; defaultV3 missing from the server's interface, and the .hash the server writes says 861d0cff....
• D, a module Extra joins the unit and Fingerprints imports it: -c Extra panics "Could not find ModSummary in the module graph for root-src-Mercury-Sentry-Constants-Constants:Mercury.Sentry.Constants.Extra", -c Fingerprints fails with "Could not find module 'Mercury.Sentry.Constants.Extra'". This is the static metadata path (--dep-units-static); the suite's harness runs the legacy path, where the fourth sequence recompiles the importer from its old source instead of panicking.

The same A, B, C, D driven by buck2 on remote execution through our pool of four servers per machine (ghc-worker.remote=true), edits made in the checkout as a developer would (extra_compiler_flags = ["-DSENTRY_V2"] in the BUCK file for C, a new file in the glob for D): B right (ABI 861d0cff..., .hash agrees), C stale (remote ABI 861d0cff... vs one-shot 0a5b2f81..., defaultV3 missing), D fails the build with the same panic. One machine, about two minutes, destroyed afterwards.

Not run: the suite at this branch's base, upstream experiments HEAD (3378700). HEAD needs two GHC patches our 9.10.3 lacks (ReachabilityIndex export, split getLinkDeps), so it cannot be built here. The commit was proven on 85f41f9, where Main.hs and the cabal file are identical to HEAD's and ghc-worker/BUCK globs test/**/*.hs; the same commit on that base is pushed as test/stale-unit-across-builds-85f41f93 on the fork for anyone who wants to rerun it. HEAD's incremental module graph commits (useIncrModGraph, mergeModuleGraphNodes append-only) do not evict either, but the failure content at HEAD is unverified.

Open question

The smallest change we can see that makes these pass is to key a unit's stored state on a fingerprint of its content (args file, source set, source hashes) rather than on the UnitId, evict the unit env, its module graph nodes and HPT entries when the fingerprint changes, and reinitialise the interpreter's loader state for that unit. About 150 lines in internal, worker only. Whether that is the right shape, or whether a worker should refuse a second build altogether, is a design call for the maintainers; this pull request only makes the current behaviour visible.

Assisted-by: Claude:claude-fable-5-1