Skip to content

fix(commands): fail fast on broken imports and enforce registry coverage - #69

Closed
syf2211 wants to merge 1 commit into
Wolfvin:mainfrom
syf2211:fix/39-strict-command-import-registry
Closed

fix(commands): fail fast on broken imports and enforce registry coverage#69
syf2211 wants to merge 1 commit into
Wolfvin:mainfrom
syf2211:fix/39-strict-command-import-registry

Conversation

@syf2211

@syf2211 syf2211 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Add strict command-import mode for CI/dev and a registry meta-test so broken or unregistered command modules cannot silently disappear from the CLI and MCP tool list.

Motivation

When a scripts/commands/*.py module fails to import, the auto-import loop logs an error but continues with exit code 0. CI only asserted len(COMMAND_REGISTRY) >= 41, so many commands could fail to load without failing the pipeline. MCP clients would then see missing tools with no clear signal.

Fixes #39

Changes

  • scripts/commands/__init__.py: honor CODELLENS_STRICT_COMMANDS=1 to re-raise import failures instead of only logging them
  • scripts/commands/self_analyze.py: register the missing self-analyze command (module loaded but never called register_command)
  • tests/conftest.py: enable strict imports during pytest runs
  • tests/test_command_registry.py: meta-test that every command module registers at least one command; subprocess test for strict fail-fast behavior
  • .gitlab-ci.yml: set CODELLENS_STRICT_COMMANDS=1 for all CI jobs

Tests

PYTHONPATH=scripts python3 -m pytest tests/test_command_registry.py -v
PYTHONPATH=scripts python3 -m pytest tests/ -q --ignore=tests/test_integration.py
  • tests/test_command_registry.py: 2 passed
  • Full suite (excluding slow integration): all passed

Notes

  • Production/default CLI behavior is unchanged; strict mode is opt-in via env var.
  • Composer-2.5 review: APPROVE

Add CODELLENS_STRICT_COMMANDS for CI/dev fail-fast when a command module
fails to import, plus a meta-test ensuring every commands/*.py registers
at least one CLI command. Also register the missing self-analyze command.

Fixes Wolfvin#39
@Wolfvin

Wolfvin commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Review - REQUEST CHANGES (critical typo)

Reviewed full diff (5 files, +88/-7). Implementation is correct and well-tested, but there's a critical typo that must be fixed before merge.

Critical issue: env var name typo

The env var is CODELLENS_STRICT_COMMANDS (double-L) everywhere in this PR. Should be CODELENS_STRICT_COMMANDS (single L).

File Current Should be
.gitlab-ci.yml CODELLENS_STRICT_COMMANDS CODELENS_STRICT_COMMANDS
scripts/commands/__init__.py CODELLENS_STRICT_COMMANDS CODELENS_STRICT_COMMANDS
tests/conftest.py CODELLENS_STRICT_COMMANDS CODELENS_STRICT_COMMANDS
tests/test_command_registry.py CODELLENS_STRICT_COMMANDS CODELENS_STRICT_COMMANDS

Why critical: Issue #39 body specifies CODELENS_STRICT_COMMANDS (single L). Anyone reading the issue and setting the env var with correct spelling gets no effect - strict mode silently won't activate. The bug #39 fixes (silent skip of broken command modules) will continue to bite.

The typo is internally consistent (all 4 sites use same wrong spelling), so the PR's own tests pass. But the env var name is a public API - it must match the issue spec and project branding (CODELENS_, not CODELLENS_).

Strengths (after typo fix, this is a great PR)

  1. Found a real bug: scripts/commands/self_analyze.py had COMMAND_INFO dict but never called register_command() - command was missing from registry. This PR fixes it. Excellent catch.
  2. CODELENS_STRICT_COMMANDS=1 env var re-raises import failures - matches issue [BUG-09] Command auto-import silently drops broken modules — registry missing commands with no CI signal #39 spec.
  3. tests/conftest.py enables strict mode for all pytest runs - defensive.
  4. test_every_command_module_registers meta-test - exactly what issue [BUG-09] Command auto-import silently drops broken modules — registry missing commands with no CI signal #39 suggested.
  5. test_strict_command_imports_fail_fast_on_broken_module - subprocess test with cleanup in finally. Good hygiene.

Verdict

Request changes: fix CODELLENS -> CODELENS typo in all 4 files. Once fixed, approved and ready to merge.

Reviewed by BOS (orchestrate-workers skill) - diff read in full before decision.

@Wolfvin

Wolfvin commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Follow-up: typo still not fixed

Checked the diff again - the env var is still CODELLENS_STRICT_COMMANDS (double L) in all 4 files. The correct spelling per issue #39 is CODELENS_STRICT_COMMANDS (single L).

This is a blocking issue: anyone who reads issue #39 and sets CODELENS_STRICT_COMMANDS=1 (correct spelling) will get no effect - strict mode silently won't activate.

Exact fix needed (4 files)

.gitlab-ci.yml line 10:

-  CODELLENS_STRICT_COMMANDS: "1"
+  CODELENS_STRICT_COMMANDS: "1"

scripts/commands/__init__.py line 30:

-_STRICT_COMMAND_IMPORTS = os.environ.get("CODELLENS_STRICT_COMMANDS", "").lower() in {
+_STRICT_COMMAND_IMPORTS = os.environ.get("CODELENS_STRICT_COMMANDS", "").lower() in {

tests/conftest.py line 6:

-os.environ.setdefault("CODELLENS_STRICT_COMMANDS", "1")
+os.environ.setdefault("CODELENS_STRICT_COMMANDS", "1")

tests/test_command_registry.py lines 43 + 51:

-    env["CODELLENS_STRICT_COMMANDS"] = "1"
+    env["CODELENS_STRICT_COMMANDS"] = "1"

and the docstring on line ~38.

Once fixed, this PR is approved and ready to merge. The implementation itself (strict mode fail-fast, self-analyze registration fix, meta-test) is correct and well-tested.

Wolfvin added a commit that referenced this pull request Jun 28, 2026
…v var

syf2211's PR #69 had the env var spelled CODELLENS_STRICT_COMMANDS (double
L) in all 4 files. Issue #39 specifies CODELENS_STRICT_COMMANDS (single L).
Anyone reading the issue and setting the env var with the correct spelling
would get no effect - strict mode silently wouldn't activate.

This fixup branch is based on syf2211's work (credited) with only the
typo corrected. All other changes (strict mode fail-fast, self_analyze
registration fix, meta-test, CI integration) are unchanged.

Closes #39
Supersedes #69
@Wolfvin Wolfvin closed this Jun 28, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG-09] Command auto-import silently drops broken modules — registry missing commands with no CI signal

2 participants