Feat/extension catalog export#327
Feat/extension catalog export#327championVisionAI wants to merge 7 commits intoLight-Heart-Labs:mainfrom
Conversation
|
There's a critical issue: the diff shows two \ function definitions in \ — the second silently shadows the first. This is likely a merge conflict artifact. Please resolve the duplicate and rebase. |
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: Feat/extension catalog export
The catalog export is genuinely useful — machine-readable inventory of all extensions with metadata, status, features, and relationships. Clean dataclass design, pure functions, good filter/sort options. A few things need fixing.
Blocking
-
Duplicate
cmd_audit()in dream-cli. The diff introduces a secondcmd_audit()function definition that silently shadows the first. Almost certainly a merge conflict artifact. This will cause the original audit behavior to be lost. -
Removed audit validations without justification. The refactor drops several checks:
external_port_defaultpositive integer validation — goneexternal_port_envshell-friendliness check — goneenv_varentry validation (non-empty key) — gonehost-systemdservice warning whencompose_fileis declared — goneoverlay-backend-extrawarning — gone
If these were intentional simplifications, document why. Otherwise restore them — losing
env_varandexternal_port_envvalidation weakens the audit. -
Test coverage reduced. Alias collision test and strict-mode-fails-on-warnings test were removed. Only 4 scenarios remain where there were 6. Restore or replace.
Non-blocking
- Catalog duplicates manifest discovery logic.
extension-catalog.pyhas its owndiscover_services(),load_document(),find_manifest()rather than sharing withaudit-extensions.py. Two files now independently parse manifests with slightly different validation. Extract shared parsing into a common module. - No CLI integration for catalog. No
cmd_catalog()in dream-cli — users must invoke the Python script directly. Consider addingdream catalogcommand. - Document the enabled/disabled convention (
compose.yamlvscompose.yaml.disabled) used byservice_status().
The catalog itself is well-designed. Just needs the merge conflict resolved, dropped validations addressed, and test coverage restored.
No description provided.