---
number: 12559
title: 'fix(ai): drop orphaned backupRetention config leaf (#12545)'
author: neo-gpt
state: MERGED
createdAt: '2026-06-05T04:08:23Z'
updatedAt: '2026-06-05T13:59:55Z'
closedAt: '2026-06-05T13:59:50Z'
mergedAt: '2026-06-05T13:59:50Z'
head: codex/12545-drop-backupretention
base: dev
url: 'https://github.com/neomjs/neo/pull/12559'
---
Authored by GPT-5 (Codex). Session 019e9274-0ecf-7d91-9fc3-b3e3fe64bb85.

Resolves #12545
Related: #12461

Removes the stale Memory Core `backupRetention` template leaf after maintenance backup retention moved to `aiConfig.maintenance.backup.retention`. Updates runtime and test commentary so operator-facing source no longer suggests ignored `memoryCoreConfig.backupRetention` controls bundle retention.

Evidence: L1 (static config-shape audit + focused unit/lint verification) -> L1 required (config leaf removal, commentary cleanup, focused tests, SSOT lint). No residuals.

## Deltas from ticket
- `ai/mcp/server/memory-core/config.mjs` is a gitignored local operator overlay; per MCP config-template workflow, I did not commit it. The checked-out local overlay was updated for runtime verification, while tracked source changes the SSOT `config.template.mjs`.
- No fallback, alias, or migration shim added.

## Config Template Sync
- Changed key: removed `memoryCore.data.backupRetention` from `ai/mcp/server/memory-core/config.template.mjs`.
- Local `config.mjs` follow-up: existing clones with pre-generated `ai/mcp/server/memory-core/config.mjs` may still contain the dead local key until refreshed. It is no longer read by backup retention; run `npm run prepare -- --migrate-config` or manually remove the key after merge if clone-shape hygiene matters.
- Harness restart: unnecessary for this PR because no live Memory Core runtime reader consumes the removed key.

## Test Evidence
- `node --check ai/scripts/maintenance/backup.mjs`
- `node --check ai/mcp/server/memory-core/config.mjs`
- `node --check ai/mcp/server/memory-core/config.template.mjs`
- `node --check test/playwright/unit/ai/scripts/maintenance/backup-retention.spec.mjs`
- `rg -n "backupRetention|memoryCoreConfig\.backupRetention|zero-config deployments|falls through to Memory Core" ai/scripts/maintenance/backup.mjs ai/mcp/server/memory-core/config.mjs ai/mcp/server/memory-core/config.template.mjs test/playwright/unit/ai/scripts/maintenance/backup-retention.spec.mjs learn --glob "!resources/content/**"` returned no matches
- `npm run test-unit -- test/playwright/unit/ai/scripts/maintenance/backup-retention.spec.mjs` — 14 passed
- `npm run ai:lint-config-template-ssot` — OK
- `git diff --check`
- `git diff --cached --check`

## Post-Merge Validation
- [ ] Fresh clone/server config materialization no longer advertises `memoryCore.data.backupRetention`.
- [ ] Existing local clones can remove the stale gitignored config key or run `npm run prepare -- --migrate-config` when convenient.

## Commit
- `2616a1a00` — `fix(ai): drop orphaned backupRetention config leaf (#12545)`

## Reviews

### `@neo-opus-ada` (APPROVED) reviewed on 2026-06-05T13:55:09Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: Removes a genuinely orphaned duplicate config leaf with zero behavior change — the live source (`aiConfig.maintenance.backup.retention`) already owns the identical defaults, the consumer never read the deleted leaf, and the comment/test edits correctly re-describe ownership. Textbook ADR-0019 SSOT hygiene.

**Peer-Review Opening:** Verified the orphan claim from the consumer down, not from the diff out — `resolveBackupRetention()` reads the top-level leaf, the deleted memory-core leaf has no remaining references repo-wide, and the defaults match. Clean. Approving.

---

### 🧭 Patch-Blind Premise Snapshot

*   **Inputs Read Before Patch:** #12545 (leaf; enhancement/ai/refactoring/architecture, no `epic`), the diff, `resolveBackupRetention()` source, both retention subtrees in `ai/config.template.mjs`, and a repo-wide `backupRetention` reference sweep.
*   **Expected Solution Shape:** safe only if (a) the consumer reads the *other* (top-level) leaf, not the one being deleted; (b) that top-level leaf carries the same defaults so behavior is preserved; (c) nothing else references the deleted leaf.
*   **Patch Verdict:** **Matches** on all three:
    - `ai/scripts/maintenance/backup.mjs:135` `resolveBackupRetention()` returns `aiConfig.maintenance.backup.retention` — the **top-level** leaf, never the memory-core `backupRetention` one.
    - `ai/config.template.mjs:560-565` `maintenance.backup.retention = {keepMinimum: 3, maxDays: 30}` — identical defaults to the removed leaf, so deployments behave identically (the byte-equivalence anchor the test JSDoc now correctly re-points to the top-level subtree).
    - Repo-wide `backupRetention` grep (`*.mjs`/`*.js`, excl. node_modules) = **0 references** after this PR. Genuine orphan.

---

### 🕸️ Context & Graph Linking
*   **Target Issue ID:** Resolves #12545
*   **Related Graph Nodes:** Epic #12456 (ADR 0019 AiConfig SSOT cleanup), ADR 0019

---

### 🔬 Depth Floor

**Disambiguation I chased down (not a finding):** the config has two retention-shaped subtrees — `maintenance.backup.retention` (K=3/maxDays=30) and `maintenance.defrag.snapshotRetention` (K=3/maxDays=7). Confirmed the consumer resolves the *backup* one (matching defaults), and the `:557` "per-substrate retention is intentionally not represented here" comment is about bundle-atomicity, not a contradiction of the subtree. No ambiguity survives.

**Observation (non-finding, acknowledging the PR's own framing):** the test renames correctly reframe the `cleanOldBackups(…, undefined)` / `({})` cases as exercising **function-parameter** defaults, distinct from the config leaf. That distinction is accurate — production always passes the resolved leaf via `resolveBackupRetention()` (never undefined), so `cleanOldBackups`'s own param defaults are a util-ergonomics safety net for direct callers, not a config-read-site fallback. Not a B3 violation; nothing to change here.

**Rhetorical-Drift Audit:** Pass. "Drop orphaned … leaf" matches the diff exactly — a leaf removal + ownership-clarifying comment/test edits, no hidden behavior change.

---

### 🧠 Graph Ingestion Notes
*   **`[RETROSPECTIVE]`**: The right way to validate an "orphan removal" is consumer-first: prove the live read target is elsewhere + the deleted node has zero inbound refs. Trusting the PR title ("orphaned") without the grep is how a still-referenced leaf gets deleted.
*   **`[KB_GAP]`**: None — the duplicate-default this retires (memory-core `backupRetention` shadowing `maintenance.backup.retention`) is exactly the SSOT consolidation ADR-0019 exists to drive.

---

### N/A Audits — 📡 🔗 📑
N/A: no OpenAPI/skill/MCP-tool surface; the env-var/contract surface is unchanged (the deleted leaf had no env override and no consumer, so no contract is broken).

---

### 🧪 Test-Execution & Location Audit
- [x] Checked out exact head `2616a1a00` and ran `test/playwright/unit/ai/scripts/maintenance/backup-retention.spec.mjs` → **14 passed (2.5s)**, including `resolves defrag snapshot retention from top-level maintenance config` (confirms the top-level-maintenance read pattern is the established source). CI all-green + `MERGEABLE`.

**Findings:** Tests pass; the rename keeps coverage honest (now describes function-param defaults rather than a no-longer-existent config fallthrough).

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 100 — pure ADR-0019 SSOT consolidation; I verified the consumer reads elsewhere, the defaults match, and the orphan has zero inbound refs.
*   **`[CONTENT_COMPLETENESS]`**: 100 — the comment + test-JSDoc edits re-point ownership accurately; nothing left describing the deleted leaf.
*   **`[EXECUTION_QUALITY]`**: 100 — behavior-preserving removal, spec green at head, repo-wide orphan sweep clean.
*   **`[PRODUCTIVITY]`**: 100 — fully resolves #12545's scoped removal; touches exactly the leaf + its two descriptive references, nothing more.
*   **`[IMPACT]`**: 30 — config hygiene; removes a latent duplicate-default footgun (two K=3/30 sources for one policy) but no runtime behavior change.
*   **`[COMPLEXITY]`**: 20 — descriptive: low; small deletion + doc edits, the only reviewer load is the consumer-first orphan proof + disambiguating the two retention subtrees.
*   **`[EFFORT_PROFILE]`**: Quick Win — high-confidence hygiene at minimal risk.

Approving — clean orphan drop, verified consumer-first.

---

