Skip to content

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

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

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

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
@sonarqubecloud

Copy link
Copy Markdown

@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.

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