You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After #6170 extracted the memory-maintenance-loop spawn wiring out of build_acp_deps (src/acp.rs) and run_daemon (src/daemon.rs) into standalone functions (spawn_acp_memory_maintenance_loops, spawn_daemon_memory_maintenance_loops), there are now four near-identical copies of this wiring across the codebase:
src/runner.rs — inline (CLI/TUI entry point)
src/serve/deps.rs::spawn_memory_maintenance_loops — extracted function (pre-existing, the reference pattern)
Flagged by impl-critic during #6170's review cycle as a significant-but-explicitly-out-of-scope finding (S1): a new loop added to serve/deps.rs's version could easily be forgotten in the acp.rs/daemon.rs copies (or vice versa), since nothing enforces the four wiring blocks stay in sync. This is the same class of duplication tracked by #5887/#5886 (shared Agent skill-config builder chain across runner/daemon/acp/serve), though neither of those covers this specific memory-loop wiring.
Reproduction Steps
Compare src/runner.rs's inline memory-loop spawn block, src/serve/deps.rs::spawn_memory_maintenance_loops, src/acp.rs::spawn_acp_memory_maintenance_loops, and src/daemon.rs::spawn_daemon_memory_maintenance_loops.
Observe all four gate and spawn the same ten loops (mem-eviction, mem-tier-promotion, mem-scene-consolidation, mem-consolidation, mem-forgetting, mem-guidelines, mem-tree-consolidation, mem-hebbian-consolidation, mem-episodic-consolidation, mem-optical-forgetting) with only cosmetic parameter differences (acp passes None for the hebbian loop's status sender; daemon passes Some(status_tx); runner.rs is inline rather than a function).
Note nothing enforces these four copies stay in sync when a new loop is added or an existing gate changes.
Expected Behavior
A single shared function (e.g. taking status_tx: Option<&UnboundedSender<String>> to unify the acp/daemon parameter difference) that all four entry points call, so a new memory-maintenance loop or a changed gating condition only needs to be written once.
Actual Behavior
Four separately-maintained copies of the same wiring logic, one of which (runner.rs) isn't even extracted into a function.
Features: full (desktop, ide, server, chat, pdf, scheduler)
Notes
Related: #5887/#5886 (broader shared Agent skill-config builder chain dedup across runner/daemon/acp/serve) track the same class of duplication but not this specific wiring. See also the project's tracked "wire-X-into-ACP/serve/daemon" defect class (19+ prior instances of this pattern across other subsystems).
Description
After #6170 extracted the memory-maintenance-loop spawn wiring out of
build_acp_deps(src/acp.rs) andrun_daemon(src/daemon.rs) into standalone functions (spawn_acp_memory_maintenance_loops,spawn_daemon_memory_maintenance_loops), there are now four near-identical copies of this wiring across the codebase:src/runner.rs— inline (CLI/TUI entry point)src/serve/deps.rs::spawn_memory_maintenance_loops— extracted function (pre-existing, the reference pattern)src/acp.rs::spawn_acp_memory_maintenance_loops— extracted function (new in acp.rs/daemon.rs memory-loop regression tests reconstruct production wiring instead of calling it #6170), structurally identical toserve/deps.rs's versionsrc/daemon.rs::spawn_daemon_memory_maintenance_loops— extracted function (new in acp.rs/daemon.rs memory-loop regression tests reconstruct production wiring instead of calling it #6170), differs from the others only in threading an extrastatus_tx: &UnboundedSender<String>parameter through to the hebbian loopFlagged by impl-critic during #6170's review cycle as a significant-but-explicitly-out-of-scope finding (S1): a new loop added to
serve/deps.rs's version could easily be forgotten in theacp.rs/daemon.rscopies (or vice versa), since nothing enforces the four wiring blocks stay in sync. This is the same class of duplication tracked by #5887/#5886 (shared Agent skill-config builder chain across runner/daemon/acp/serve), though neither of those covers this specific memory-loop wiring.Reproduction Steps
src/runner.rs's inline memory-loop spawn block,src/serve/deps.rs::spawn_memory_maintenance_loops,src/acp.rs::spawn_acp_memory_maintenance_loops, andsrc/daemon.rs::spawn_daemon_memory_maintenance_loops.mem-eviction,mem-tier-promotion,mem-scene-consolidation,mem-consolidation,mem-forgetting,mem-guidelines,mem-tree-consolidation,mem-hebbian-consolidation,mem-episodic-consolidation,mem-optical-forgetting) with only cosmetic parameter differences (acp passesNonefor the hebbian loop's status sender; daemon passesSome(status_tx); runner.rs is inline rather than a function).Expected Behavior
A single shared function (e.g. taking
status_tx: Option<&UnboundedSender<String>>to unify the acp/daemon parameter difference) that all four entry points call, so a new memory-maintenance loop or a changed gating condition only needs to be written once.Actual Behavior
Four separately-maintained copies of the same wiring logic, one of which (
runner.rs) isn't even extracted into a function.Environment
Notes
Related: #5887/#5886 (broader shared Agent skill-config builder chain dedup across runner/daemon/acp/serve) track the same class of duplication but not this specific wiring. See also the project's tracked "wire-X-into-ACP/serve/daemon" defect class (19+ prior instances of this pattern across other subsystems).