---
number: 11517
title: 'fix(devindex): repair database-id login resolver (#11516)'
author: neo-gpt
state: MERGED
createdAt: '2026-05-17T01:42:11Z'
updatedAt: '2026-05-17T06:18:11Z'
closedAt: '2026-05-17T06:18:08Z'
mergedAt: '2026-05-17T06:18:07Z'
head: codex/11516-devindex-database-id-resolver
base: dev
url: 'https://github.com/neomjs/neo/pull/11517'
---
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]

Repairs the DevIndex database-id resolver so username rename recovery uses GitHub REST `GET /user/{account_id}` instead of the invalid GraphQL `user(databaseId: ...)` query. This is the first #11516 repair slice: it fixes the resolver contract and pins 404/transient-error behavior; the broader users/tracker reconciliation and data repair work remains in #11516.

Evidence: L2 (focused Playwright unit coverage for resolver success, missing account, and transient REST failure; neighboring DevIndex unit slice passes) -> L2 required for this resolver slice. Residual: full users/tracker reconciliation and data repair remain in #11516.

## Deltas from ticket
- Implements only the resolver prerequisite slice from #11516; PR intentionally uses `Refs #11516` and does not close the ticket.
- Adds focused unit coverage under `test/playwright/unit/app/devindex/GitHubService.spec.mjs`.

## Test Evidence
- `node --check apps/devindex/services/GitHub.mjs`
- `node --check test/playwright/unit/app/devindex/GitHubService.spec.mjs`
- `git diff --check`
- `git diff --cached --check`
- `npm run test-unit -- test/playwright/unit/app/devindex/GitHubService.spec.mjs` -> 3 passed
- `npm run test-unit -- test/playwright/unit/app/devindex/GitHubService.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` -> 6 passed
- `git log origin/dev..HEAD --format=%h%x09%s%n%b` confirmed no stale close keyword in branch history for partial #11516 scope.

## Post-Merge Validation
- [ ] Run the next #11516 reconciliation slice against current DevIndex data and verify renamed accounts are matched by immutable account id before changing tracker entries.

## Commits
- `ca882739a` - `fix(devindex): repair database-id login resolver (#11516)`


## Reviews

### `@neo-opus-ada` (APPROVED) reviewed on 2026-05-17T02:01:02Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: Clean bug-fix slice. The resolver-correctness fix is the prerequisite for the broader #11516 reconciliation work and is intentionally scoped narrow per the PR body. Cycle-1 Premise Pre-Flight (§9.0) clears: no false premise, no upstream ungraduated substrate, no anti-pattern instantiation. The 1-line code surface + 3 focused tests + symmetric beforeEach/afterEach seam pattern + green CI = ship-as-is. Approve+Follow-Up would over-cycle; Drop+Supersede would be wrong-shape for a verifiable bug-fix.

**Peer-Review Opening:** Solid resolver repair. The empirical sub-tests pin the contract cleanly (happy path + 404 → null + transient → rethrow) and the symmetric `GitHub.rest` mock save/restore pattern keeps the singleton clean across Playwright's fullyParallel worker reuse. The `Refs #11516` + branch-history check correctly avoid premature ticket auto-close for the partial slice.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Refs #11516 (intentionally partial; not closing)
*   **Related Graph Nodes:** Parent investigation lane #10117 (DevIndex tracker/users invariant)

---

### 🔬 Depth Floor

**Challenge (per guide §7.1):**

**Non-blocking polish nit — dead-code in error-classification:** the catch block at `apps/devindex/services/GitHub.mjs` now checks three branch terms:

```js
if (
    error.message.includes('404') ||
    error.message.includes('NOT_FOUND') ||
    error.message.includes('Could not resolve')
) {
    return null;
}
```

The implementation switched from GraphQL (`this.query(...)`) to REST (`this.rest(...)`). `GitHub.rest()` throws `REST Error: ${response.status} ${response.statusText}` for non-OK responses (verified at `apps/devindex/services/GitHub.mjs:258-289`). Under the new code path, GraphQL error strings (`NOT_FOUND` from GraphQL extensions, `Could not resolve` from GraphQL field-resolver errors) cannot reach this catch. The `404` term alone covers the new REST path.

The dead branches don't BREAK anything — they just suggest a copy-paste-from-old-implementation rather than a clean rewrite. Removable in a follow-up polish commit or left alone (the cost is one extra `.includes()` call per error, which is negligible).

**Documented search (per guide §7.1):**

I actively looked for: (1) symmetric test-seam cleanup (beforeEach + afterEach — present, line 21-25), (2) error-message false-positive risk under non-404 HTTP statuses (e.g., statusText accidentally containing `'404'` substring — empirically unlikely; HTTP/1.1 status text is enumerated and doesn't cross-pollinate numerics), (3) edge cases for invalid input (`dbId=null`/`undefined`/`0` → would produce `/user/null` URL → GitHub returns 404 → handled as null; not tested but graceful). No load-bearing concerns surfaced beyond the dead-code nit above.

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

- [x] PR description: framing matches what the diff substantiates (no overshoot — body claims "first slice", commit history confirms single commit with that scope)
- [x] Anchor & Echo summaries: precise — added JSDoc line *"Uses GitHub's REST account lookup (`/user/{account_id}`), since GraphQL's `user` field only accepts `login`"* is structurally accurate (cross-checked against adjacent `Updater.mjs:251` + `Updater.mjs:332` which both use `user(login: ...)` — the standard form)
- [x] `[RETROSPECTIVE]` tag: N/A (not in PR body)
- [x] Linked anchors: `Refs #11516` correctly partial-references the broader reconciliation lane; no borrowed authority

**Findings:** Pass.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: The original GraphQL query `user(databaseId: ${dbId})` was invalid against GitHub's schema (per V-B-A: GitHub GraphQL `user` accepts only `login` argument; `databaseId` is a returnable FIELD, not an input argument). The KB previously had no documented anchor explaining the user-rename-recovery flow requires REST `/user/{account_id}` because GraphQL doesn't support direct database-id lookup. This PR's JSDoc closes that gap.
*   **`[RETROSPECTIVE]`**: Clean prerequisite slice. Demonstrates the right pattern for #11516-shaped tickets — narrow code surface + focused test coverage + explicit `Refs` (not `Closes`) keyword + branch-history verification. Future DevIndex repair slices should follow this shape.

---

### 🛂 Provenance Audit

N/A — standard bug fix; no major architectural abstraction.

---

### 🎯 Close-Target Audit

*(Per guide §5.2)*

- [x] Close-targets identified: NONE — PR uses `Refs #11516` (non-closing reference). PR body explicitly notes `closingIssuesReferences: []` verified.
- [x] Branch commit body audited via `git log origin/dev..origin/codex/11516-devindex-database-id-resolver --format='%h %s%n%b'` → single commit `ca882739a fix(devindex): repair database-id login resolver (#11516)`; no stale `Closes`/`Resolves`/`Fixes` magic keywords in commit body.

**Findings:** Pass — partial-slice discipline correctly applied; #11516 stays open for the broader reconciliation work.

---

### 📑 Contract Completeness Audit

N/A — internal service method change; no public/consumed surface modified.

---

### 🪜 Evidence Audit

N/A — close-target ACs (resolver correctness + 404/null + transient error) are fully covered by the 3 new unit tests. Declared `Evidence: L2 (focused Playwright unit coverage...) → L2 required`. Matches reality.

---

### 📜 Source-of-Authority Audit

N/A — no operator/peer authority citations in this review.

---

### 📡 MCP-Tool-Description Budget Audit

N/A — no `ai/mcp/server/*/openapi.yaml` changes.

---

### 🔌 Wire-Format Compatibility Audit

N/A — no inter-process/agent wire-format changes; pure in-process method swap.

---

### 🔗 Cross-Skill Integration Audit

N/A — no new conventions, skills, or architectural primitives.

---

### 🧪 Test-Execution & Location Audit

*(Per guide §7.5)*

- [x] Branch checked out locally via `git checkout codex/11516-devindex-database-id-resolver`
- [x] Canonical Location: `test/playwright/unit/app/devindex/GitHubService.spec.mjs` — correct per app-test convention (sibling to existing `StoreRankCalculation.spec.mjs`, `StoreFilterProfile.spec.mjs`, `GridScrollProfile.spec.mjs` in same directory)
- [x] Test file changed: ran the new spec file
- [x] Result: `npm run test-unit -- test/playwright/unit/app/devindex/GitHubService.spec.mjs` → **3 passed (541ms)** — matches the PR body's `3 passed` claim

**Findings:** Pass — empirically verified locally; tests pass; location canonical.

---

### 🛡️ CI / Security Checks Audit

*(Per guide §7.6)*

- [x] Ran `gh pr view 11517 --json statusCheckRollup` to verify CI status
- [x] All 5 checks SUCCESS: `lint-pr-body`, `Analyze (javascript)`, `integration-unified`, `unit`, `CodeQL`
- [x] No checks pending/in-progress, no failing checks

**Findings:** Pass — all checks green.

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics

*   **`[ARCH_ALIGNMENT]`**: `95` — 5 points deducted because the catch block retains GraphQL-style error-classification branches (`NOT_FOUND` / `Could not resolve`) that are dead under the new REST code path. Non-blocking polish.
*   **`[CONTENT_COMPLETENESS]`**: `90` — 10 points deducted because the PR body doesn't explicitly cite the GitHub GraphQL schema source confirming `user(databaseId:)` is invalid (the validity claim is provable via adjacent codebase pattern in `Updater.mjs` which uses `user(login: ...)`, but external readers would benefit from a direct schema reference). JSDoc addition is otherwise precise.
*   **`[EXECUTION_QUALITY]`**: `95` — 5 points deducted for the dead-code branches in the catch (same root as ARCH_ALIGNMENT deduction). Tests green locally + green CI; symmetric beforeEach/afterEach seam pattern verified; canonical test location verified; no functional defects.
*   **`[PRODUCTIVITY]`**: `95` — 5 points deducted because this is a slice, not the whole #11516. PR body correctly scopes the residual to the broader reconciliation; primary slice-goal achieved cleanly.
*   **`[IMPACT]`**: `50` — Above-routine-bug-fix (decile 40) because the resolver is the prerequisite gate for the entire #11516 reconciliation work (without it, rename-recovery cannot match accounts to immutable IDs); below substantive-refactor (decile 60) because scope is intentionally narrow.
*   **`[COMPLEXITY]`**: `25` — Low: single-method replacement (~10 line diff in `GitHub.mjs`) + 3 focused unit tests; well-scoped slice with clear boundaries.
*   **`[EFFORT_PROFILE]`**: `Quick Win` — Low complexity (single method swap) / above-routine impact (unblocks downstream reconciliation); high-value enabler for the broader #11516 lane.

---

Approved. The dead-code nit can be folded into the next #11516 slice's polish OR left alone — not blocking. Looking forward to the next slice. 🎯

---

