BaseModulePlugin.initialise() (conductor) is not idempotent - affects every migrated module's method exports

[martin-henz]

Summary

BaseModulePlugin.initialise() (from @sourceacademy/conductor/module) is itself not idempotent - independent of anything any individual bundle does in its own override. A second call re-registers every exportedNames entry as a fresh export all over again. This is the root cause underneath the "initialise() isn't idempotent" nitpicks filed separately for rune (#827) and scrabble, and it means any bundle overriding initialise() has to guard the entire method body, not just its own additions, to actually be safe from a second call.

Not urgent, no user-facing symptom currently observed (nothing in this repo appears to call initialise() twice today) - opening this so the finding is tracked, since the actual fix belongs in @sourceacademy/conductor itself, a separate published package this repo doesn't own.

How this was found

While fixing csg's version of this nitpick (#826), the first attempt guarded only csg's own colour-constant loop:

override async initialise() {
  await super.initialise();

  if (this.__initialised) return;
  this.__initialised = true;

  for (const [name, value] of Object.entries(CSG_COLORS)) { ... }
}

A test asserting initialise() twice still produces the expected export count failed with 74 exports instead of the expected 45 (45 = 29 methods + 16 colours). 74 = 45 + 29 - i.e. super.initialise() alone had re-registered all 29 method exports a second time; the colour-loop guard had nothing to do with the extra 29. Moving the guard to wrap the whole method, including the super.initialise() call, fixed it:

override async initialise() {
  if (this.__initialised) return;
  this.__initialised = true;

  await super.initialise();
  for (const [name, value] of Object.entries(CSG_COLORS)) { ... }
}

This is base-class behaviour, not anything csg-specific - the same mechanism would reproduce in any bundle whose exportedNames array is non-empty.

Who's actually exposed

Module	Overrides initialise()?	Exposed?
rune	yes, no guard	yes - #827
scrabble	yes, no guard	yes - separately filed
csg	yes, now guards the whole method	fixed, #826
sound	no	not exposed via override, but a raw second call to initialise() would still re-register sound's ~38 method exports through the base class alone
binary_tree	no	same as sound
midi	no (pushes its 3 constants in the constructor instead)	same as sound, for its method exports
repeat	no (binds methods in the constructor instead)	same as sound, for its method exports

So every migrated module's method exports (registered by the base class from exportedNames) are equally exposed to a second initialise() call, regardless of whether that module also overrides initialise() for its own extra data. Modules that push extra data in the constructor (midi, repeat) only avoid duplicating that specific data, not the base class's own registration of their methods.

Suggested fix

The correct place for this fix is BaseModulePlugin.initialise() itself, in @sourceacademy/conductor (not this repo) - e.g. an internal guard so a second call is a no-op, matching what every affected bundle currently has to reinvent locally. Until that lands, the interim convention for this repo should be: any bundle overriding initialise() must wrap the entire method body (not just its own additions) in an __initialised-style guard, as csg now does.

[11suixing11]

Hi, I can take this on in the upstream package.

Since the root cause is in @sourceacademy/conductor, I will open a PR against source-academy/conductor that makes BaseModulePlugin.initialise() idempotent and adds a small regression test there, then link it back here.

[11suixing11]

Opened the upstream conductor PR here: source-academy/conductor#58