refactor: address code-review findings on the containment fixes

[steve-calvert-glean]

Follow-up to #28, addressing the Standards axis of the code review — which reported after that PR merged rather than before. No behaviour change: all 199 tests pass unchanged, and each of #28's three original reproductions was re-run and still resolves correctly.

Duplicated Code — the pointed one

listSourcePluginDirs was discoverSourcePlugins's scan loop copied verbatim: same exists guard, same readdir({ withFileTypes: true }), same dotfile/non-dir skip, same isSourcePluginDir call. Two copies of "what counts as a source plugin" will drift — which is the bug class this entire 1.0 review is about, and #28's own PR body argued that avoiding that duplication was the reason for putting the helper in config.ts. It then duplicated the loop anyway.

discoverSourcePlugins is now expressed over listSourcePluginDirs, so there is one definition.

Duplicated Code — second instance

managed.ts's pathsEqual(a, root) || isUnder(a, root) was fs.ts's isInside plus folding. Replaced with one exported isInsideFolded beside isInside. That also removes a Mysterious Name: isUnder vs isInside gave a reader no hint which of the two folded case.

Data Clump

All three call sites passed project.rootDir, project.config, project.configPath — which is exactly ResolvedProjectConfig. buildDeleteGuard(project, force) collapses #28's sync→async ripple to one signature.

It deliberately still takes the loaded config rather than plugin-discovery results. The reviewer's stronger suggestion — derive the guard at load time, since loadConfig already discovered those directories — would couple clean to a working build, and clean not requiring one is load-bearing (it's often why someone is cleaning). The shallow readdir is the price of that independence.

Smaller

• missing.reverse() was safe — it sat on the return line and the function exited immediately — but the safety depended on a reader noticing the adjacent return. [...missing].reverse() costs nothing and removes the reasoning step.
• Dropped incident framing from two comment blocks ("the shape of test that was missing when this class of bug shipped twice"), keeping the behavioural rationale.

Still open from the review

The Spec axis never reported — two escalations, no output. And one question neither axis resolved: whether main's existing test refuses to clean the default source.plugins root even when unset in config still asserts correct behaviour under the narrowed guard. My read is that it does, because its fixture's plugins/demo contains a skills/ component dir and so is a genuine discovered source plugin — but I wrote both that test and the fix, so it's noted rather than closed.

🤖 Generated with Claude Code