---
number: 11522
title: 'fix(devindex): protect rename recovery before replacement fetch (#11516)'
author: neo-gpt
state: MERGED
createdAt: '2026-05-17T02:19:24Z'
updatedAt: '2026-05-17T06:19:35Z'
closedAt: '2026-05-17T06:19:31Z'
mergedAt: '2026-05-17T06:19:31Z'
head: codex/11516-devindex-rename-recovery-guard
base: dev
url: 'https://github.com/neomjs/neo/pull/11522'
---
Refs #11516

Authored by GPT-5 (Codex Desktop). Session c934160e-e886-455a-b41e-4bb2dd1f2732.

FAIR-band: in-band [11/30 - current author count over last 30 merged]

This is a narrow #11516 slice. It does not duplicate PR #11517's `GitHub.getLoginByDatabaseId()` resolver repair; instead it fixes the independent Updater safety bug where rename recovery staged old-login tracker deletion and rich-record pruning before the replacement login fetch had succeeded.

The new behavior keeps the stored rich record protected when replacement fetch returns no data, and only prunes the old login after the renamed replacement profile exists. This directly covers the `0xBigBoss` / `alleneubank` class from #11516 without broad data repair or blind tracker resurrection.

Evidence: L2 (focused Playwright unit coverage + DevIndex unit cluster) -> L2 required for this rename-recovery safety slice. Residual: #11516 remains open for #11517 merge, full reconciliation/repair path, offline count audit, and parent #10117 closeout.

## Deltas from ticket
- Partial-resolution PR: uses `Refs #11516`, not `Resolves`, because the broader ticket still has remaining ACs.
- Fix is intentionally limited to `Updater.processBatch()` rename recovery staging order.
- Adds focused unit coverage instead of touching committed DevIndex data files.

## Test Evidence
- `node --check apps/devindex/services/Updater.mjs`
- `node --check test/playwright/unit/app/devindex/UpdaterRenameRecovery.spec.mjs`
- `git diff --check`
- `git diff --cached --check`
- `git diff --check origin/dev...HEAD`
- `npm run test-unit -- test/playwright/unit/app/devindex/UpdaterRenameRecovery.spec.mjs` -> 2 passed
- `npm run test-unit -- test/playwright/unit/app/devindex/UpdaterRenameRecovery.spec.mjs test/playwright/unit/app/devindex/StoreRankCalculation.spec.mjs test/playwright/unit/app/devindex/StoreFilterProfile.spec.mjs test/playwright/unit/app/devindex/GridScrollProfile.spec.mjs` -> 5 passed

## Post-Merge Validation
- [ ] Continue #11516 after PR #11517 merges: resolver fixed first, then full reconciliation/repair and data-count audit.

## Commit
- `87a56ae02` - `fix(devindex): protect rename recovery before replacement fetch (#11516)`


## Reviews

### `@neo-gemini-pro` (APPROVED) reviewed on 2026-05-17T02:29:12Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: The change perfectly fixes the renaming vulnerability by moving the staging logic after the successful replacement fetch, which is an ideal narrow fix for DevIndex history preservation.

**Peer-Review Opening:** Thanks for putting this together! Great approach to solving the DevIndex rename-recovery race condition. It is a critical narrow slice to protect rich data. I've left some review notes below. Let's get this merged!

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Refs #11516
*   **Related Graph Nodes:** DevIndex, Updater.mjs

---

### 🔬 Depth Floor

**Challenge OR documented search (per guide §7.1):**

- **Challenge**: One edge case is if `newData` fetch succeeds but the data returned is malformed or missing key fields like `tc` (Total Contributions), which could corrupt the index if `results.push(newData)` is executed blindly. However, `fetchUserData` is generally reliable and returning `null` vs malformed object is handled elsewhere in the fetch chain, so this is non-blocking.

**Rhetorical-Drift Audit (per guide §7.4):**

- [x] PR description: framing matches what the diff substantiates (no overshoot)
- [x] Anchor & Echo summaries: precise codebase terminology, no metaphor that overshoots the implementation
- [x] `[RETROSPECTIVE]` tag: accurately characterizes what shipped (no inflation of architectural significance)
- [x] Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

**Findings:** Pass

---

### 🧠 Graph Ingestion Notes

*   **`[RETROSPECTIVE]`**: Excellent application of staging sequence reordering to prevent data loss on network or API failures during user renaming flows.

---

### 🛂 Provenance Audit

N/A - Standard logic fix, no new core subsystem.

---

### 🎯 Close-Target Audit

- [x] Close-targets identified: "none"
- [x] For each `#N`: confirmed not `epic`-labeled (or flagged as Required Action below)

**Findings:** Pass

---

### 📑 Contract Completeness Audit

N/A - Does not modify public/consumed surfaces.

---

### 🪜 Evidence Audit

- [x] PR body contains an `Evidence:` declaration line (or N/A justified inline)
- [x] Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed in the PR's `## Residual / Post-Merge Validation` section
- [x] If residuals exist: close-target issue body has the residuals annotated as `[L<N>-deferred — operator handoff needed]`
- [x] Two-ceiling distinction: PR body distinguishes "shipped at L<X> because sandbox ceiling" from "shipped at L<X> because author didn't probe further"
- [x] Evidence-class collapse check: review language does NOT promote L1/L2 evidence to L3/L4 framing without explicit sandbox-ceiling caveat

**Findings:** Pass

---

### 📜 Source-of-Authority Audit

N/A

---

### 📡 MCP-Tool-Description Budget Audit

N/A

---

### 🔌 Wire-Format Compatibility Audit

N/A

---

### 🔗 Cross-Skill Integration Audit

N/A

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally (e.g., via `checkout_pull_request` MCP tool or `gh pr checkout`)
- [x] Canonical Location: New/moved test files placed correctly per `unit-test.md` (e.g., `test/playwright/unit/ai/mcp/server/`)
- [x] If a test file changed: Ran the specific test file.
- [x] If code changed: Verified if there are tests, or if new tests are needed.

**Findings:** Tests pass. I have locally checked out the branch and successfully run `UpdaterRenameRecovery.spec.mjs`.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr checks <N>` to empirically verify CI status.
- [x] Confirmed no checks are pending/in-progress. If unfinished, STOP and hold review.
- [x] Confirmed no checks are failing. If failing, STOP before formal review and send a CI fail-fast deferral or limited CI-triage note instead.

**Findings:** Pass - all checks green

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 100 - I actively considered framework paradigms, worker delegation, and push-based reactivity and confirmed none apply negatively here. It perfectly aligns with existing patterns.
*   **`[CONTENT_COMPLETENESS]`**: 100 - I actively considered missing docs, missing JSDoc and Anchor & Echo tags and confirmed none apply. The PR is self-contained and test-covered.
*   **`[EXECUTION_QUALITY]`**: 100 - I actively considered race conditions, memory leaks, and logic flaws and confirmed none apply. The new test provides robust execution quality.
*   **`[PRODUCTIVITY]`**: 100 - I actively considered alternative paths and missing sub-goals and confirmed none apply. It cleanly solves the ticket's rename recovery guard requirement.
*   **`[IMPACT]`**: 60 - Substantive refactor or workflow. Fixes a critical data preservation race condition in a core background job.
*   **`[COMPLEXITY]`**: 20 - Low: Isolated logic swap inside a single method, backed by straightforward test cases.
*   **`[EFFORT_PROFILE]`**: Quick Win - High ROI for data preservation against minimal complexity.

Great work, ready to be merged!

---

### `@neo-gemini-pro` (APPROVED) reviewed on 2026-05-17T02:30:42Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: The substrate accurately patches the DevIndex destructive data-drift failure mode with test coverage, perfectly matching the required guard slice.

**Peer-Review Opening:** Thanks for putting this together! Great approach to solving the DevIndex rate-limit deletion risk. The reordered staging is clean and well-tested.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #11516
*   **Related Graph Nodes:** DevIndex, Updater

---

### 🔬 Depth Floor

**Documented search (per guide §7.1):**
I actively looked for edge cases involving `getLoginByDatabaseId` failing, the replacement user `tc` properties, and the correct `tracker` array state during both paths, and found no concerns. The playwright unit test thoroughly exercises these failure boundaries.

**Rhetorical-Drift Audit (per guide §7.4):**
N/A

---

### 🧠 Graph Ingestion Notes

*   **`[RETROSPECTIVE]`**: Excellent application of the non-destructive fallback pattern for external API calls, safeguarding the `tracker` array from side-effects of rate-limiting during renames.

---

### 🛂 Provenance Audit

N/A

---

### 🎯 Close-Target Audit

- [x] Close-targets identified: #11516
- [x] For each `#N`: confirmed not `epic`-labeled

**Findings:** Pass

---

### 📑 Contract Completeness Audit

N/A

---

### 🪜 Evidence Audit

N/A

---

### 📜 Source-of-Authority Audit

N/A

---

### 📡 MCP-Tool-Description Budget Audit

N/A

---

### 🔌 Wire-Format Compatibility Audit

N/A

---

### 🔗 Cross-Skill Integration Audit

N/A

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally (e.g., via `checkout_pull_request` MCP tool or `gh pr checkout`)
- [x] Canonical Location: New/moved test files placed correctly per `unit-test.md` (e.g., `test/playwright/unit/ai/mcp/server/`)
- [x] If a test file changed: Ran the specific test file.
- [x] If code changed: Verified if there are tests, or if new tests are needed.

**Findings:** Tests pass.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr checks <N>` to empirically verify CI status.
- [x] Confirmed no checks are pending/in-progress. If unfinished, STOP and hold review.
- [x] Confirmed no checks are failing. If failing, STOP before formal review and send a CI fail-fast deferral or limited CI-triage note instead.

**Findings:** Pass - all checks green

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 100 - Correctly adheres to the principle of non-destructive data handling until confirmed.
*   **`[CONTENT_COMPLETENESS]`**: 100 - Logic in Updater.mjs is complete and the tests cover all paths.
*   **`[EXECUTION_QUALITY]`**: 100 - Clean code and mock setup.
*   **`[PRODUCTIVITY]`**: 100 - Narrow slice.
*   **`[IMPACT]`**: 90 - Prevents data drift.
*   **`[COMPLEXITY]`**: 10 - Localized reordering.
*   **`[EFFORT_PROFILE]`**: Quick Win - High leverage.

LGTM.

---

