Skip to content

memory-maintenance-loop wiring is a 4-way near-identical duplicate across runner.rs/serve/deps.rs/acp.rs/daemon.rs #6180

Description

@bug-ops

Description

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:

  1. src/runner.rs — inline (CLI/TUI entry point)
  2. src/serve/deps.rs::spawn_memory_maintenance_loops — extracted function (pre-existing, the reference pattern)
  3. 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 to serve/deps.rs's version
  4. src/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 extra status_tx: &UnboundedSender<String> parameter through to the hebbian loop

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

  1. 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.
  2. 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).
  3. 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.

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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P3Research — medium-high complexityarchArchitecture and designbugSomething isn't workingmemoryzeph-memory crate (SQLite)tech-debtTechnical debt

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions