Repository navigation
Proposal: fix dual-instance bootstrapIncludes failure + misleading "root Include entry" error (with patch sketches) #12076
ArvinDevel
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Proposal: fix the dual-instance
bootstrapIncludesfailure + its misleading error message@tianyicui @LegGasai @imccyu — tagging the top contributors for visibility. Happy to provide more probe data or test repos if useful.
Background
A cluster of "settings can never be saved" reports (#8928, #11853, #9579, #11321, #10282, #9750, #10407, #7675, #9542, #10532…) share one failure signature:
I hit this on
0.2.1-rc.5(Linux/WSL-like path, Intel macOS) and root-caused it with runtime instrumentation. The root Include is actually registered fine —mountRootInclude()andreconcileProfilePatches()→recompose()were simply executing against two different module instances ofdsh-app-boot, each holding its own module-levelbootstrapIncludesWeakMap. Registration lands in copy A's map; the lookup reads copy B's map; thegetmisses; the write path (settings/mutate→writeFileAtomic→reconcileProfilePatches) catches, rolls back the file on disk, and surfaces the error above — which claims the root Include is missing. It isn't.Instrumented proof (two real copies loaded in one process):
Moving the profile's duplicated
node_modulesaway (30-second test) makes the error vanish andsettings/mutateimmediately works, with revisions and the optimistic lock behaving correctly — confirming the read-back validation and the optimistic lock are not involved (contrary to earlier hypotheses in #7959/#9299, which describe a different code path that passes on rc.5).Known triggers for dual instances:
dependenciesdeclaring installation-shipped bundles (my case: manualpnpm addofdsh-base/dsh-web-app— user error, but nothing warns or blocks it, andinitProfile()emitsdependencies: {}so the tooling itself never writes this shape)Fix 1 (root cause): share the registry across module instances via
Symbol.forThe failure exists because the registry is module-scoped. Hanging it off
globalThisunder aSymbol.forkey merges all copies in-process (Node workers each boot their own tree, so they're unaffected):Compatibility:
packages/boot/app-boot/tests/user-patches.spec.ts:351-358("rejects a context without the launcher root Include") still passes — globalizing merges the module-instance dimension, not the Context dimension. A context that never mounted still throws.Trade-off: cross-copy version skew is tolerated with a warning rather than a hard failure. A hard DI refactor (pass the root entry explicitly into ConfigEditor) is architecturally cleaner but touches cross-package signatures — probably too costly for preview stage.
Fix 2 (defense): make the error tell the truth + self-diagnose
Even with Fix 1, a
getmiss can legitimately mean "never mounted" (e.g. config-editor attached to a non-root ctx). The message should distinguish the two and hand the user an actionable hint:This is the highest value-per-line change: it doesn't eliminate the fault, but it turns "hours of misdirected debugging" (which misled #11853's original analysis and mine) into a 30-second diagnosis. If only one fix lands, land this one.
Fix 3 (prevention): block installation-shipped bundles at
plugin addlistBundles()already computesshipped = Object.hasOwn(installation.dependencies ?? {}, name). Hoist that check into the add/install entry point:Hard-block with a
--forceescape hatch beats today's behavior (silent install → runtime explosion with an unrelated error message) for both normal and advanced users.Suggested tests
recomposecopy; assert the write still succeeds (Fix 1) or at least the new diagnostic fires (Fix 2).user-patches.spec.ts:351-358green.--force→ allowed + boot warning about duplicates.Priority
Symbol.forshared registryshippedNote: master and
0.2.2-alpha.3still carry the original message atpackages/boot/app-boot/src/index.ts:334-335.All reactions