refactor(registry): deepen registry subsystem — eliminate duplication, cleanup teardown, decompose hub

[VampireChicken12]

Problem Statement

The registry subsystem (src/features/_registry/) has grown to 15 files and ~2,500+ lines without corresponding improvement in navigability or testability. Several structural issues make it hard to reason about and harder to change safely:

• Duplicate safelyExecute: FeatureRegistry has its own copy of the error-handling wrapper that's nearly identical to FeatureManagerBase.safelyExecute, despite the base class being designed for exactly this purpose.
• False compile-time claim: featureContract.d.ts declares that every src/features/*/index.ts exports metadata, but only index.metadata.ts does. TypeScript accepts the import at compile time but it fails at runtime.
• Misleading file split: featureRegistryCore.ts exists only to break a circular dependency; its functions (hasState, isFeature, resolveEnabled) logically belong on the FeatureRegistry class.
• Bloated hub class: FeatureRegistry (300 lines) mixes registration, enable/disable orchestration, navigation callbacks, config dispatch, button lifecycle, concurrency guards, and caching.
• Three parallel cleanup systems: Features tear down via cleanupRegistry.run(), onDisable() lifecycle methods, and button remove() callbacks. These should unify.

Solution

Five refactoring passes on the registry, ordered from least to highest effort/risk:

1. Fix the false type contract
2. Eliminate the safelyExecute duplicate
3. Clean up the misleading file split
4. Decompose FeatureRegistry into smaller orchestrators
5. Unify the three cleanup systems into a single pattern

Commits

Commit 1: fix(registry): correct featureContract.d.ts module pattern to match actual exports

Effort: ~15 minutes — Risk: Very Low

The ambient declaration claims /src/features/*/index.ts exports metadata, but the actual export lives in index.metadata.ts. Change the module pattern to match /src/features/*/index.metadata.ts or, since metadataRegistry already validates schemas and the contract is redundant, remove the module augmentation entirely.

Pre-flight: grep for any import { metadata } from "./index" to confirm nothing breaks. Update those imports if they exist.

Verification: npm run typecheck passes.

---

Commit 2: refactor(registry): make FeatureRegistry extend FeatureManagerBase, remove safelyExecute duplicate

Effort: ~1-2 hours — Risk: Very Low

FeatureRegistry has its own safelyExecute (nearly identical to FeatureManagerBase.safelyExecute, ~40 lines). It's the only manager class that doesn't extend FeatureManagerBase.

• Make FeatureRegistry extend FeatureManagerBase
• Override getFeatureIdForErrorLogging() to return "registry"
• Delete the duplicated safelyExecute implementation
• Update all internal calls from this.safelyExecute(...) to the inherited version
• Align the minor rethrow-handling difference to FeatureManagerBase's version

Verification: npm run typecheck passes. Trace the enable/disable call chain to confirm inherited safelyExecute is used.

---

Commit 3: refactor(registry): merge featureRegistryCore into featureRegistry, move hasState cleanly

Effort: ~1 hour — Risk: Low

featureRegistryCore.ts (57 lines) exports hasState, isFeature, isFeatureKey, resolveEnabled. It exists solely because featureLifecycleManager.ts needed hasState without importing featureRegistry.ts. Now that the split is stable, the re-export from featureRegistry.ts is confusing.

• Move the four functions into featureRegistry.ts as file-level exports
• Update featureLifecycleManager.ts to import hasState from featureRegistry.ts directly
• Delete featureRegistryCore.ts
• Update autoRegister.ts imports if needed

Verification: npm run typecheck passes. npm run build succeeds. Confirm no circular dependency at import time.

---

Commit 4: refactor(registry): extract FeatureOrchestrator from FeatureRegistry hub

Effort: ~3-4 hours — Risk: Low-Medium

Extract the enable/disable state machine into a dedicated FeatureOrchestrator class.

FeatureOrchestrator owns:

• updateFeatureEnabledState(id, enabled, config)
• enableAll(options)
• disableAll()
• updateFeatureOnNavigation(id, navType)
• Private: updatingFeatures concurrency guard set
• Private: featureEnabledState map
• Private: sortedFeaturesCache / sortedFeaturesCacheDirty

FeatureRegistry keeps:

• register(feature, initialState)
• getAll(), getFeature(id)
• initialize(cb)
• setSchema(id), setStateSchema(id)
• destroyNavigationListener()
• Reference to FeatureOrchestrator as this.orchestrator

FeatureOrchestrator receives references to navigationManager, configManager, buttonManager, lifecycleManager, stateManager, perf, and the features map.

Verification: npm run typecheck. Manual toggle of features in the extension popup. Verify enable -> disable -> re-enable -> config change -> navigation transitions.

---

Commit 5: refactor(registry): unify teardown into single Disposer pattern

Effort: ~4-6 hours — Risk: Medium-High

Currently cleanup splits across:

1. cleanupRegistry.run(id) — for non-event resources
2. onDisable(config, stateAPI) — lifecycle method on the feature object
3. Button remove() callbacks in featureButtonManager

Design a lightweight approach without adding a fourth lifecycle hook. Explore options:

• onDisable itself returning a Disposer | Disposer[]
• Or keeping cleanupRegistry but routing it through the lifecycle manager instead of being called independently in featureRegistry.ts

Remove the standalone cleanupRegistry.run(id) call from FeatureRegistry and route all teardown through the lifecycle method instead.

Migration: features that currently call cleanupRegistry.add(...) (e.g., miniPlayer) register their cleanup in onDisable instead.

Verification: npm run typecheck. npm run build. Manual toggle of miniPlayer, loopButton, playerSpeed — verify no orphaned observers/timeouts after disable.

Decision Document

• Commit order: strictly least-effort-first, so each step validates the substrate for the next.
• safelyExecute behavioral alignment: FeatureManagerBase's version is canonical; the minor rethrow difference in FeatureRegistry's copy is discarded.
• No new abstractions until Commit 4: first three commits are pure deletion/reorganization with zero new types.
• Disposer approach: avoid adding a fourth lifecycle hook by exploring whether onDisable itself can return disposers, or by routing cleanupRegistry through the lifecycle manager.
• featureRegistryCore.ts deletion: safe because featureLifecycleManager.ts only imports hasState at module evaluation time, resolved before any FeatureRegistry instance creates a FeatureLifecycleManager.

Testing Decisions

• No tests exist for the registry subsystem today.
• Each commit verified by npm run typecheck + npm run build.
• Commits 4 and 5 require manual verification through the extension popup (enable/disable, navigate YouTube pages).
• Adding integration tests for the registry is a separate future effort.

Out of Scope

• Adding unit or integration tests for the registry.
• Refactoring feature-level onDisable implementations (except as needed for Commit 5 cleanup unification).
• Changing Vite import.meta.glob patterns in featureMetadataRegistry.ts or autoRegister.ts.
• The FeaturePerformanceTracker class (only touched in Commit 2 for the dedup).
• DOM querying abstraction (Candidate 4 in the architecture doc).