Claude finished @Yeraze's task —— View job

PR Review: refactor(config): centralize config/module-type registry (section B1)

Tasks

• Gather context and read changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess test coverage
• Provide final feedback

This is a clean, well-scoped refactor. The motivation is sound, the implementation is straightforward, and the tests are genuinely useful. A few observations:

Correctness

statusmessage field inconsistency (configTypes.ts:62): The statusmessage module entry has field: 'statusmessage' (lowercase, no camelCase), while every other multi-word field uses camelCase (storeForward, cannedMessage, trafficManagement, etc.). If the protobuf field is actually statusMessage (camelCase), this would silently mismatch. Worth verifying against the protobuf definition — but this would be a pre-existing issue if so, not introduced by this PR.

Remaining switch cases in server.ts: The local-node and remote-node GET handlers still have large switch blocks with hardcoded case lists (case 'serial': case 'extnotif': ... at lines 7785–7806 and 7930–7944). The registry is now used for the lookup/validation step, but the fallthrough cases listing every module id are still duplicated. This is not a regression — those cases just fall through to a common config = remoteConfig or config = finalConfig.moduleConfig[moduleKey] — but it means adding a new config type still requires editing those switch blocks in addition to configTypes.ts. Could be a follow-up: replace the fallthrough switch with a registry-driven branch (since the MODULE_FIELD_BY_ID lookup is already happening above the switch in the local-node path at line 7799).

Code quality

CONFIG_TYPES is missing mqtt in the switch fallthrough at line 7785 of server.ts: The local-node module switch covers serial through trafficmanagement but omits mqtt. Since mqtt has its own dedicated handling earlier in the switch (line 7770ish), this is likely intentional, but it's worth confirming the mqtt case is actually handled before the fallthrough block.

DEVICE_FIELD_BY_ID is imported but barely used in server.ts: It's used once at line 7617 for the needsRequest check, but the large switch block below doesn't use it (it hardcodes the field names). Not a bug, but indicates the consolidation is partial for the GET path.

Tests

The test file is excellent:

• The pinned enum/field snapshot (EXPECTED table) is exactly the right approach for protocol-contract regression testing.
• The drift guard (every VALID_MODULE_CONFIG_TYPES entry must be in the registry) directly prevents recurrence of Bug: Invalid module type errors for trafficmanagement and statusmessage configs #3464.
• The uniqueness check per kind is a nice sanity guard.

One minor gap: the test asserts CONFIG_TYPE_MAP has correct values for a couple of spot-checks, but doesn't iterate all entries to verify the derived map is complete. Given the snapshot test covers CONFIG_TYPES exhaustively, this is a low-priority gap.

Summary

This is ready to merge. The core goal — eliminating the copy-paste drift that caused #3464 — is achieved, and the drift guard test will catch future recurrences. The residual switch fallthrough blocks in server.ts are pre-existing and out of scope for this PR. The one item worth double-checking before merge is the statusmessage field casing.