Skip to content

Only a directory named after the module is a module - #332

Closed
jakub-przepiora wants to merge 1 commit into
developfrom
fix/module-discovery
Closed

jakub-przepiora wants to merge 1 commit into
developfrom
fix/module-discovery

Conversation

@jakub-przepiora

Copy link
Copy Markdown
Contributor

An updater that parks the previous version next to the module — as Enterprise.poprzednia-20260930195044, a full copy including module.json — made discover() report it as a second install of the same module, and as an ENABLED one, because enablement matches on the manifest name and the copy carries the same name.

What the operator saw:

OpenMES Enterprise  v0.3.5  ENABLED     Disable | Uninstall
OpenMES Enterprise  v0.3.4  ENABLED     Disable | Uninstall

Two cards, two directories, the same provider class, and no way to tell which one Uninstall would remove. Worse than cosmetic: two directories claiming the same provider is a state nothing good comes out of.

Fix

A module lives in a directory named after itself — that is how installFromZip() puts it there and how loadEnabled() finds it. Anything else carrying a module.json is a leftover: an updater's backup, a half-finished copy, an unpacked archive.

Those are now skipped and logged (module.directory_name_mismatch), so a mismatch is visible rather than silently shaping the modules list.

Tests

✓ katalog nazwany jak modul jest wykrywany
✓ kopia obok modulu nie jest drugim modulem
✓ katalog deklarujacy obca nazwe jest pomijany

Full suite: 2954 passed, 3 failed — all InstallPresetTest, pre-existing on this branch's base.

Uwaga

The module side is being fixed in parallel (backups move to storage/), but core should not depend on every module getting that right — especially third-party ones.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QRWsLoNeVWdsSHve6vUmxs

An updater that parks the previous version next to the module — as
`Enterprise.poprzednia-20260930195044`, a full copy including module.json —
made discover() report it as a second install of the same module, and as an
ENABLED one, because enablement matches on the manifest name and the copy
carries the same name.

The operator saw two "OpenMES Enterprise" cards, v0.3.5 and v0.3.4, both
enabled, both declaring the same provider class, with no way to tell which
one "Uninstall" would remove. Worse than cosmetic: two directories claiming
the same provider is a state nothing good comes out of.

A module lives in a directory named after itself — that is how
installFromZip() puts it there and how loadEnabled() finds it. Anything else
carrying a module.json is a leftover: an updater's backup, a half-finished
copy, an unpacked archive. Those are skipped now and logged, so a mismatch
is visible rather than silently shaping the modules list.

The module side is being fixed too — backups move out of modules/ — but core
should not depend on every module getting that right.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QRWsLoNeVWdsSHve6vUmxs
@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • cla-signed

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: Mes-Open/OpenMes/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 60bf5d30-e1ac-406f-893f-02db7199c248

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jakub-przepiora

Copy link
Copy Markdown
Contributor Author

Zamykam na rzecz #333 — opisuję tam problem, a decyzję, czy rdzeń ma to łatać, zostawiam do rozstrzygnięcia. Gałąź fix/module-discovery z poprawką i testami zostaje, gdyby podejście zostało przyjęte.

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.

1 participant