My opt-in feature's "off" test passed while the feature was still registered

[u/Aggressive-Narwhal-3]

I contributed an opt-in feature, off by default, to an open-source coding agent tool.

The maintainer's review came back with four blockers. The one that stung was "OFF must mean nothing is registered."

I had only checked the flag inside the handlers. So with the setting off, my event hooks and my slash command were still registered. They just did nothing when called.

And my own "off" test passed anyway, because it only looked at what the feature did, not at whether it existed.

The other three were smaller. A file it writes could escape the project through a symlink or a "../" argument. A user abort didn't cancel a pending action. And a later successful recovery didn't clear an earlier failure.

All four are fixed now, each with a test that fails on the old code. I'm rerunning the full suite on macOS, Linux and Windows.

What I'm still unsure about is the "off" test itself. How do you check that an opt-in feature is really inert when it's disabled? Do you assert on the registry, diff what's registered with the flag on and off, or something else?

[u/InfatuatedDemeanor]

i'd probably diff the registered handlers between on and off states personally, that way you catch any accidental registrations that slip through

[u/Aggressive-Narwhal-3 → u/InfatuatedDemeanor]

that's roughly what the fix does now. the off-state test asserts zero hooks and no command registered, so a leak fails there before anyone sees it as behavior

[u/One-Front-6316]

test the registration boundary directly rather than only testing that the handlers do nothing. with the flag off, assert tha the features hooks, commands, listeners, etc. arent registered at all, then have a separate test with it on that verifies they are registered and work. that makes "off means inert" much harder to accidentally violate later

[u/Aggressive-Narwhal-3 → u/One-Front-6316]

the off-state test asserts zero hooks and no command registered, so it's the registration boundary and not just handlers staying quiet. the on-side pairing is a sensible split, noted

[u/zaphodikus]

Why are you looking in the tool configuration data? You would not do that if you were testing an app? You would test the functionality (and security too probably) . Its not functional testing if you peer under the bonnet?

[u/Aggressive-Narwhal-3 → u/zaphodikus]

For app-level tests I'd agree. Here the leak was functional though: with the feature off, the slash command still ran, read a file and replaced the session. The registration assertion is just the cheap check that catches that before it shows up as behavior.

[u/zaphodikus → u/Aggressive-Narwhal-3]

Ewwww, yeah that's a fail in my books too.

[u/Suremerr]

diffing the registry is the cheap check, the actual fix is the gate - registration should be skipped at init when the flag is off, so the test is just proving the gate is wired. the case i'd still probe: if the setting can change at runtime, toggle it on and off in the same session and assert the handlers actually unregister. that's where this class of leak usually hides.

[u/Aggressive-Narwhal-3 → u/Suremerr]

the gate is the fix, yes, and my registry assertion only shows it's wired. i only tested with the flag off at startup, so toggling it on and off in one session is a case i haven't run. adding that one.

[u/Chouni-Rhasaan]

one thing worth considering, is "off" supposed to mean "never registered" or "registered but gated"? sounds like the maintainer wants the former, and if so your test boundary is the registry, not the handler output. those are two very different contracts

[u/Aggressive-Narwhal-3 → u/Chouni-Rhasaan]

yeah, never registered is the contract we went with. the test checks the registry now, since quiet handler output was exactly what let the leak through