tech debt: extract shared command runner from learn.ts/decisions.ts

[dean0x]

Summary

decisions.ts (726 lines) is a structural duplicate of learn.ts (896 lines). Nearly every subcommand follows the same pattern with minor parametric differences. This tech debt blocks maintainability and introduces risk of divergence bugs between the two command implementations.

Deferred Items

1. Code Duplication

• decisions.ts and learn.ts share identical overall structure and logic flow
• Nearly every subcommand (e.g., --run-background, --enable, --disable) replicates the same control flow pattern with minor parameter differences
• Impact: Changes to one often need applying to both; bugs fixed in one may remain in the other

2. Pipeline Duplication

• Both --run-background handlers implement an identical 10-step pipeline sequence
• Should be extracted into a shared runBackgroundPipeline(opts) utility
• Currently duplicated across both files, reducing maintainability
• Impact: Pipeline improvements require updating two separate code paths

3. Monolithic Action Handler

• decisions.ts contains a 580-line action handler with all 12 subcommands inlined
• Each subcommand branch should be extracted into a named function (e.g., handleEnable(), handleDisable(), handleRun())
• Impact: High cognitive load for reviewers, difficult to test individual subcommands in isolation

4. Testing Methodology

• Several tests in cli-subcommands.test.ts re-implement inline logic instead of calling actual command handlers
• extractBatchMessages and loadExistingObservations lack direct unit tests
• Tests duplicate implementation logic, making them fragile to refactoring
• Impact: Test coverage is misleading; refactoring breaks tests that should still pass

5. Convergence Logic Triplication

• Convergence detection logic duplicated across 3 prose documentation surfaces: code-review.md, code-review-teams.md, review:orch/SKILL.md
• Currently mitigated by sync NOTEs and parity test suite (convergence-detection.test.ts)
• Impact: Changes to convergence algorithm require 3-surface updates; risk of desynchronization if prose drifts from test oracle
• Solution Path: Extract convergence logic to shared reference document with build-time distribution to all 3 surfaces (similar to skill/agent distribution model)
• Deferred From: PR feat: add review pipeline convergence detection (#223) #225, branch feat/223-review-pipeline-convergence-detection

Recommended Approach

1. Create a shared CommandRunner class/module for dual-command dispatch
2. Extract --run-background logic to runBackgroundPipeline()
3. Extract each subcommand into a named function with clear input/output contracts
4. Refactor tests to call actual handlers instead of duplicating logic
5. Apply to both learn.ts and decisions.ts simultaneously for consistency
6. Create a convergence logic reference document (.md) in shared/references/ with schema and decision tables; build-time script replicates to command prose and SKILL.md with parity tests

Related

• PR feat: decouple decisions agent + migrate to Pattern A (Phase 3) #203: Phase 3 — Decouple Decisions Agent
• PR feat: add review pipeline convergence detection (#223) #225: Review Pipeline Convergence Detection
• Issue: deferred pending architectural refactor of learn.ts/decisions.ts duplication