Pin the bundled allowlist/exclude-set disjointness invariant with a test (#761)

[philcunliffe]

Closes item 2 of #761. Item 1 of #761 (the JSDoc correction in
hypaware-core/smoke/flows/walkthrough_picker_to_first_query.js:581-584)
only exists on PR #757's branch, which is open and held for a human to
merge; there is nothing to edit on master for it, so this PR does not
touch it. #761 should stay open (or be otherwise tracked) until #757 lands
and item 1 can be applied there.

What this adds

src/core/runtime/bundled.js exports V1_BUNDLED_PLUGIN_ALLOWLIST (default
activation) and V1_EXCLUDED_FROM_DEFAULT (bundled plugins requiring
explicit opt-in). Several call sites are correct only because those two sets
are disjoint, and nothing asserted it:

• discoverBundledPlugins (src/core/runtime/bundled.js:151-157) checks
  allowlist.has(name) before excludeSet.has(name), so a name present in
  both sets lands in loaded.
• ridersInDefaultSet (src/core/cli/walkthrough.js:2083-2091) filters on
  non-membership of V1_EXCLUDED_FROM_DEFAULT.

A name in both sets would make those two reads disagree, and the resulting
failure would surface downstream (a red release smoke pointing at a golden
config) rather than at the actual cause.

New test: test/core/bundled-sets.test.js, asserting
V1_BUNDLED_PLUGIN_ALLOWLIST and V1_EXCLUDED_FROM_DEFAULT are disjoint,
with a failure message that names the offending plugin(s) and explains which
two call sites would disagree.

Where it lives, and why

The issue suggested either test/core/compose-picker-config.test.js (next
to its existing corpus guards) or a new test/core/bundled-sets.test.js. I
went with the new file: the assertion is a pure Set comparison over the
two constants bundled.js exports, and needs none of
compose-picker-config.test.js's picker infrastructure (discoverBundledPlugins
manifest loading, buildPluginCatalog, composePickerConfig). That file's
own framing is pinning composePickerConfig's output against the retired
wantsAnthropic/wantsCodex switch; its two corpus-wide tests near the
bottom (no bundled plugin manifest fails validation,
no excluded bundled manifest declares compose_with) earn their place there
because they matter for the rider-composition behavior under test in that
file. The disjointness invariant belongs to bundled.js itself, not to a
consumer's test suite, so it gets its own small file.

Scope: one assertion, not more

I considered two further candidate invariants: that every name in
V1_EXCLUDED_FROM_DEFAULT corresponds to a real bundled plugin on disk, and
likewise for V1_BUNDLED_PLUGIN_ALLOWLIST. Neither is load-bearing for the
failure mode this issue is about: a stale or typo'd name in either set with
no matching manifest simply never appears in loaded/excluded (it has no
effect on unknownDirs either, since that bucket is populated from
discovered manifests, not from set membership). It is not a landmine the
way an in-both-sets name is, so it would be padding rather than a guard
against the documented failure class. I skipped it.

Verifying the test discriminates

Per the task, I temporarily added @hypaware/hermes (already in
V1_BUNDLED_PLUGIN_ALLOWLIST) to V1_EXCLUDED_FROM_DEFAULT and reran the
new test in isolation:

$ node --test test/core/bundled-sets.test.js
# Subtest: the default-activation allowlist and the excluded set are disjoint
not ok 1 - the default-activation allowlist and the excluded set are disjoint
  ---
  error: |-
    plugin(s) @hypaware/hermes appear in both V1_BUNDLED_PLUGIN_ALLOWLIST and V1_EXCLUDED_FROM_DEFAULT: discoverBundledPlugins checks the allowlist first, so these would load by default while ridersInDefaultSet treats them as excluded. Remove the name from whichever set does not match its intended default-activation boundary.
    + actual - expected

    + [
    +   '@hypaware/hermes'
    + ]
    - []
  code: 'ERR_ASSERTION'
  ...
# pass 0
# fail 1

Reverted the injection and reran: back to green (# pass 1, # fail 0).
The working tree diff on src/core/runtime/bundled.js was clean after the
revert.

Checks (fresh npm install)

• npm test - 4030 passed, 1 skipped (unrelated), 0 failed.
• npm run typecheck - clean.
• node --test test/core/bundled-sets.test.js - 1 passed.

package_bin_boot and walkthrough_picker_to_first_query smokes are known
red on master for unrelated reasons (#758, #750, both with open PRs); not
touched by this change.

[philcunliffe]

neutral review - round 1

Head reviewed: 1caec29f51ca7fe89dad52deb89350a13b6926ac. MERGEABLE, all 9 checks
SUCCESS. Reviewed in a detached worktree; nothing was written to the branch.

One nit, on the failure message, which is the whole point of a test like this.

Two results worth pulling out of the record below:

• The test earns its file. The author verified discrimination with
  @hypaware/hermes, which happens to carry a per-plugin spot check, so the suite
  showed 2 failures. The reviewer re-ran it with @hypaware/openclaw, an
  allowlisted plugin with no spot check: 1 failure out of 4030, the new test
  alone. For the ten-odd allowlisted plugins without a bespoke assertion, nothing
  else in the repo notices.
• The author was right to skip the "names resolve on disk" check, and the
  reviewer tested that rather than accepting it: bogus names added to both sets
  simultaneously leave the suite at 4030 pass / 0 fail, and reading confirms why -
  every bucket is populated by iterating discovered manifests, so nothing anywhere
  iterates either set expecting a disk match. A stale name is genuinely inert.

---

Worktree clean, my scratch file removed (other files in the shared scratchpad belong to other sessions and were left alone). Here is the review record.

---

VERDICT: findings

The test discriminates, fails informatively, and is the only thing in the suite that catches the general case. One nit on the failure message's wording, which is one clause away from exact.

---

1. nit - test/core/bundled-sets.test.js:22 - the failure message says an in-both plugin "would load by default"; nothing default-activates it

The message reads:

discoverBundledPlugins checks the allowlist first, so these would load by default
while ridersInDefaultSet treats them as excluded.

The asymmetry claim is correct, and I verified both halves. But "load by default" overstates what happens. Every site that decides activation re-checks the exclude set and sides with it:

• src/core/runtime/boot.js:591-592 and 601-603 (computeSelectedPlugins) filter on V1_BUNDLED_PLUGIN_ALLOWLIST.has(name) && !V1_EXCLUDED_FROM_DEFAULT.has(name). That !exclude term is load-bearing, not redundant, because selectBootPlugins (boot.js:533) builds its pool as [...discovered.loaded, ...excludedAvailable, ...installed.loaded]. An in-both plugin is in loaded, so it reaches the filter, and is then dropped.
• src/core/cli/dispatch.js:866-884 (computeBootSelection, which backs hyp --help per LLP 0009 #layered-help) routes through the same selectBootPlugins, so help agrees with boot.
• loadPickerCatalog (src/core/cli/walkthrough.js:2043-2044) builds its catalog from [...bundled.loaded, ...bundled.excluded], so the bucket assignment is invisible to it either way.

So an in-both plugin is never actually activated by default. What genuinely diverges is narrower and is exactly what the test's own header comment (lines 8-11) already states precisely: the name lands in discoverBundledPlugins's loaded bucket, which the picker smoke reads as a proxy for the default set, while ridersInDefaultSet drops it. The header comment is exact; only the user-facing message drifted, which is the half a reader sees in six months.

Why it matters: someone hitting this message would go looking for a plugin that activated when it should not have, and find that it did not activate anywhere. Low cost to fix, and the message is the whole point of the test.

Exact fix, line 22:

    + 'these land in its `loaded` bucket while ridersInDefaultSet treats them as excluded. '

(replacing + 'these would load by default while ridersInDefaultSet treats them as excluded. ')

Everything else in the message, including the remediation sentence, is right and needs no change.

---

What this test would catch

Mutation 1 (the author's, re-derived). Added '@hypaware/hermes' to V1_EXCLUDED_FROM_DEFAULT in src/core/runtime/bundled.js. Reproduces the reported output exactly:

not ok 1 - the default-activation allowlist and the excluded set are disjoint
  ---
  location: '.../test/core/bundled-sets.test.js:15:1'
  failureType: 'testCodeFailure'
  error: |-
    plugin(s) @hypaware/hermes appear in both V1_BUNDLED_PLUGIN_ALLOWLIST and
    V1_EXCLUDED_FROM_DEFAULT: discoverBundledPlugins checks the allowlist first, so
    these would load by default while ridersInDefaultSet treats them as excluded.
    Remove the name from whichever set does not match its intended default-activation
    boundary.
    + actual - expected
    + [ '@hypaware/hermes' ]
    - []
  code: 'ERR_ASSERTION'
  operator: 'deepStrictEqual'

Full suite under this mutation: 2 failures - the new test, plus hermes is bundled and default-activated beside claude and codex (test/plugins/hermes-manifest.test.js:67). The second only fires because hermes happens to carry a per-plugin spot check.

Mutation 2 (the one that matters). Same edit using '@hypaware/openclaw', an allowlisted plugin with no spot check. Full suite: 1 failure, the new test alone, out of 4030. This is the evidence the test earns its file: for any of the ten-odd allowlisted plugins without a bespoke membership assertion, nothing else in the repo notices.

Vacuity. Cannot pass vacuously by import failure: [...undefined] throws TypeError: s is not iterable, so a broken import fails loudly rather than yielding an empty overlap. It would pass on two empty sets, but emptying either set is caught elsewhere by direct membership assertions (test/plugins/hermes-manifest.test.js:64-66, test/plugins/iceberg-plugin.test.js:47, test/plugins/vector-search-manifests.test.js:40-41), so that degenerate state is guarded. Not a gap.

Mutation 3 (testing the author's skip, judgement call 3). Added a bogus '@hypaware/ghost-allow' to the allowlist and a bogus '@hypaware/ghost-excl' to the exclude set simultaneously. Full suite: 4030 pass, 0 fail. The author's reasoning holds and their skip was correct. Confirmed by reading as well as by running: discoverBundledPlugins populates all three buckets by iterating discovered manifests (bundled.js:143-156), so unknownDirs is fed from disk, never from set membership; computeSelectedPlugins filters available, itself derived from discovered manifests; ridersInDefaultSet iterates the composeWith map. Nothing anywhere iterates either set expecting a disk match - every reader is a membership test. A stale name is genuinely inert, so a "names resolve on disk" check would be hygiene, not a guard. No finding here.

---

Also checked, clean

• Scope. git diff origin/master...HEAD --stat is exactly test/core/bundled-sets.test.js | 26 ++++++, one file, +26/-0. (Note for anyone re-running this: the worktree's local master ref is stale and diffs against ~380 files; use origin/master.)
• Item 1 of Smoke JSDoc justifies the loaded-only cut by coverage; the load-bearing property is disjointness, and no test guards it #761 correctly left alone. The target text (Reading only 'loaded'... / restating it: it agrees) is absent from hypaware-core/smoke/flows/walkthrough_picker_to_first_query.js on this base, confirming it exists only on PR walkthrough_picker_to_first_query golden predates compose_with riders (#750) #757's branch. walkthrough_picker_to_first_query golden predates compose_with riders (#750) #757 is OPEN (walkthrough_picker_to_first_query golden predates compose_with riders (#750)). The PR body explains this and says Smoke JSDoc justifies the loaded-only cut by coverage; the load-bearing property is disjointness, and no test guards it #761 should stay open until walkthrough_picker_to_first_query golden predates compose_with riders (#750) #757 lands, so the issue does not read as half-abandoned. Correct call.
• Mechanism, verified at source. bundled.js:150-156 checks allowlist.has(name) then else if (excludeSet.has(name)), so allowlist wins the bucket. walkthrough.js:2087 is if (V1_EXCLUDED_FROM_DEFAULT.has(rider)) continue. Both halves of the asymmetry are as described.
• No duplicate coverage. grep -rn "disjoint" across test/ src/ hypaware-core/ llp/ finds nothing pre-existing for these two sets; the only prior readers in test/ are the three per-plugin spot checks named above. Nothing in test/core/ covered this.
• Placement. The author's argument holds. The two corpus guards in test/core/compose-picker-config.test.js:534+ genuinely need discoverBundledPlugins manifest loading, buildPluginCatalog, and composedNames; this assertion is a pure Set intersection over two imported constants and uses none of it. A new file is defensible, and the file is small and single-purpose.
• Conventions. Matches the repo test idiom exactly (// @ts-check, node:test, node:assert/strict, blank line, then source imports - identical to test/core/manifest-semver.test.js and test/core/verb-registry.test.js). No semicolons. No em dash (U+2014) anywhere in the file. No @typedef, no inline import('...') types. The one import is a root-anchored .js specifier (../../src/core/runtime/bundled.js).
• Missing @ref considered, and correctly absent. The sibling guard at compose-picker-config.test.js:533 carries @ref LLP 0213#d1 [tests], so I checked whether this test should too. It should not: LLP 0213 #d1 decides that the graph pair composes by default, and LLP 0024 #embedding-is-a-separate-capability (annotated on V1_EXCLUDED_FROM_DEFAULT itself) decides the embedder opt-in. Neither states disjointness. No LLP documents this invariant, so an @ref would be mechanical, which CLAUDE.md explicitly warns against. Citing issue Smoke JSDoc justifies the loaded-only cut by coverage; the load-bearing property is disjointness, and no test guards it #761 and PR walkthrough_picker_to_first_query golden predates compose_with riders (#750) #757 in the comment is the honest attribution.
• Ran, on a fresh npm install: npm test clean at 4030 pass / 0 fail / 1 skipped; npm run typecheck (tsc -p tsconfig.json --noEmit) clean, no output; node --test test/core/bundled-sets.test.js green in isolation. Smokes not run; package_bin_boot and walkthrough_picker_to_first_query are the known-red pair (package_bin_boot is red on master: the --help banner changed and the smoke still pins the old copy #758, walkthrough_picker_to_first_query is red on master, and no smoke runs in CI #750) and are untouched by a test-only change.
• Working tree left clean. All three mutations reverted from src/core/runtime/bundled.js; git status --porcelain empty at HEAD 1caec29. My one scratch file (a bundled.js backup) was removed; nothing else under the shared scratchpad was touched.

[philcunliffe]

neutral review - round 2 (final)

Head reviewed: 1bdb55336e9298a3753771bb132ffc5806c29e11. Base master, one file,
test/core/bundled-sets.test.js, +26/-0 against origin/master. The round-2 delta is
exactly one line (git diff 1caec29 1bdb553): the failure-message clause, nothing else.
Reviewed in a detached worktree, removed at the end; nothing written to the branch.

---

VERDICT: clean

Nothing ship-blocking, and nothing below blocker either. The round-1 nit is fixed, the
replacement clause is true, and I could not find a way to make the old wording the more
accurate of the two.

---

Round-1 finding, re-derived

Fixed. The message now reads:

plugin(s) @hypaware/openclaw appear in both V1_BUNDLED_PLUGIN_ALLOWLIST and
V1_EXCLUDED_FROM_DEFAULT: discoverBundledPlugins checks the allowlist first, so these
land in its `loaded` bucket while ridersInDefaultSet treats them as excluded. Remove the
name from whichever set does not match its intended default-activation boundary.

I re-derived the mechanism from source rather than from the round-1 record or the
fixer's account, by enumerating every consumer of discoverBundledPlugins and every
reader of the two sets (grep over src/ bin/ hypaware-core/ test/; 10 call sites in
src/, 2 set readers outside bundled.js).

The question that decides it - is there any site that acts on a plugin purely because
it is in loaded, without re-checking the exclude set? Answer: no site activates on
that basis. Three findings:

1. Every activation path re-checks. computeSelectedPlugins
   (src/core/runtime/boot.js:585-629) is the only thing that produces the selected
   set. Its all-bundled branch (:589-596) and all-available branch (:599-612)
   both filter V1_BUNDLED_PLUGIN_ALLOWLIST.has(name) && !V1_EXCLUDED_FROM_DEFAULT.has(name) && !installedNames.has(name). The !exclude term is load-bearing rather than
   redundant, because selectBootPlugins (:525-544) builds its pool as
   [...discovered.loaded, ...excludedAvailable, ...installed.loaded], so an in-both
   plugin does arrive at the filter via loaded and is dropped there. The other two
   profiles ({ activate: [...] } and the default config) select strictly by name from
   the profile or the config and never consult either set, so they cannot diverge on
   bucket at all. computeBootSelection (src/core/cli/dispatch.js:866-885) routes
   through the same selectBootPlugins, so hyp --help agrees with boot.
2. Every catalog consumer is bucket-blind. All nine remaining callers build from
   [...bundled.loaded, ...bundled.excluded]: walkthrough.js:2009, :2044, :2173;
   wizard/index.js:974; commands/plugin.js:69; commands/clients.js:2009;
   daemon/status.js:1499; config/apply_deps.js:62 and :103; boot.js:445. The
   bucket assignment is invisible to every one of them.
3. Two sites do read loaded alone, and neither activates anything.
   detectShadowedPlugins (boot.js:479-489) builds bundledNames from
   discovered.loaded only, and boot.js:244 walks discovered.loaded to emit
   plugin.skipped telemetry. The first is bucket-sensitive in a real way (an in-both
   plugin with a same-named installed plugin would reject boot as a shadow collision,
   instead of the installed one replacing the skeleton via the excludedAvailable
   filter at :530-532), but that is a rejection, not a default activation, and it is
   contingent on a third-party install. The second is a log line. Neither makes "would
   load by default" true.

So the old clause was false and the new one is true. ridersInDefaultSet
(src/core/cli/walkthrough.js:2083-2091) is if (V1_EXCLUDED_FROM_DEFAULT.has(rider)) continue, so the second half holds as stated.

Fixed without introducing a new problem. I judged the emitted text as a reader with
no context (full TAP output read end to end, below). It names the problem (this plugin is
in both sets), the mechanism (the two readers disagree about it), and the remedy (drop it
from whichever set does not match intent), and it now sends nobody hunting for a plugin
that activated when it should not have - because none did. ridersInDefaultSet is
unqualified by file in the message, but it is a unique grep hit and the test's header
comment two lines up names src/core/cli/walkthrough.js, so a reader who opens the file
lands correctly. The clause is arguably now narrower than the worst consequence
(shadow-collision behavior, item 3 above), but a message that understates by omission is
strictly better than one that overstates by assertion, and the shadow case is not the
failure class the test is about. Not worth another round.

One correction to the record, since I was asked not to inherit accounts: the round-1
narrative describes "the picker smoke reads loaded as a proxy for the default set."
grep -rn "\.loaded" hypaware-core/smoke/ returns nothing - no smoke flow reads that
bucket. Neither the test's header comment nor the failure message makes that claim, so
this does not touch the artifact under review; it just means the mechanism's blast radius
is narrower than round 1 painted it, which if anything strengthens the case for the
fixer's narrower wording.

---

Also checked, clean

• Regression: the openclaw mutation still yields exactly one failure. Added
  '@hypaware/openclaw' (allowlisted, no per-plugin spot check) to
  V1_EXCLUDED_FROM_DEFAULT. Full npm test: 4029 pass / 1 fail / 1 skipped - the
  new test alone, out of 4030. Round 1's result reproduces at this head. The emitted
  failure is the message quoted above, with a clean + [ '@hypaware/openclaw' ] / - []
  diff and a location: pointing at test/core/bundled-sets.test.js:15:1.
• Regression: still cannot pass vacuously. Two probes. (a) Renamed the export to
  V1_BUNDLED_PLUGIN_ALLOWLIST_RENAMED: the file fails loudly with
  SyntaxError: ... does not provide an export named 'V1_BUNDLED_PLUGIN_ALLOWLIST'
  (# fail 1), not silently on an empty overlap. (b) Emptied
  V1_EXCLUDED_FROM_DEFAULT entirely - the degenerate state where the assertion is
  trivially true: caught by test/plugins/vector-search-manifests.test.js (both plugins are bundled but excluded from default activation, # fail 1). Both round-1 claims hold.
• Clean runs, on a fresh npm install (43 packages, 0 vulnerabilities):
  npm test 4030 pass / 0 fail / 1 skipped; npm run typecheck
  (tsc -p tsconfig.json --noEmit) clean, no output. Smokes not run - test-only change,
  and package_bin_boot / walkthrough_picker_to_first_query are the known-red pair
  (package_bin_boot is red on master: the --help banner changed and the smoke still pins the old copy #758, walkthrough_picker_to_first_query is red on master, and no smoke runs in CI #750).
• Scope. Still exactly one file. git show --stat 1bdb553 is
  test/core/bundled-sets.test.js | 2 +-, and the round-2 diff touches only the one
  string literal. No source file was modified in either commit; all three of my mutations
  were reverted with git checkout -- and verified.
• Item 1 of Smoke JSDoc justifies the loaded-only cut by coverage; the load-bearing property is disjointness, and no test guards it #761 correctly still untouched, consistent with the PR body: it lives on
  PR walkthrough_picker_to_first_query golden predates compose_with riders (#750) #757's branch and there is nothing on master to edit.
• Conventions. No em dash (U+2014) anywhere in the file (grep -nP "\x{2014}":
  none). No trailing semicolons. // @ts-check, node:test + node:assert/strict,
  blank line, then a root-anchored .js source import - the repo test idiom. Typecheck
  passes, so the JSDoc/type conventions are satisfied. No @ref: I re-checked and agree
  with round 1 that none is warranted, since neither LLP 0024
  #embedding-is-a-separate-capability (annotated on V1_EXCLUDED_FROM_DEFAULT itself)
  nor LLP 0213 #d1 states disjointness, and CLAUDE.md warns against mechanical refs.
• Cleanup. git status --porcelain empty at 1bdb553 before teardown; my worktree
  wt-767-r2 removed with git worktree remove. No file under the shared scratchpad
  outside my own worktree was created, moved, or deleted; the other sessions' worktrees
  (wt-740c-X1eu, wt-751-fix, wt-767-fix) are intact. /work/hypaware untouched.