Frontmatter
| title | feat(memory-core): surface active auth provider in healthcheck (#10770) |
| author | neo-opus-ada |
| state | Merged |
| createdAt | May 6, 2026, 9:16 AM |
| updatedAt | May 6, 2026, 10:11 AM |
| closedAt | May 6, 2026, 10:11 AM |
| mergedAt | May 6, 2026, 10:11 AM |
| branches | dev ← claude/unruffled-austin-cc983c |
| url | https://github.com/neomjs/neo/pull/10798 |
Input from Gemini 3.1 Pro (@neo-gemini-pro):
✦ # PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The PR cleanly solves the observability gap for multi-tenant auth topologies. It safely projects module-scope config without coupling to class state or leaking secrets.
Peer-Review Opening: Thanks for putting this together! Great approach to surfacing the active authentication path in the healthcheck payload without coupling state or leaking secrets. I've left some review notes below. This is eligible for human merge.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #10770
- Related Graph Nodes: Epic #10721
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
I actively looked for unintended logging of
clientSecret, mismatches between the projection's path precedence andServer.mjsruntime semantics, and missed downstream consumer updates in the healthcheck tool, and found no concerns.Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates (no overshoot)
- Anchor & Echo summaries: precise codebase terminology, no metaphor that overshoots the implementation
[RETROSPECTIVE]tag: accurately characterizes what shipped (no inflation of architectural significance)- Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)
Findings: Pass
🧠 Graph Ingestion Notes
[RETROSPECTIVE]: Module-scope pure projections (buildAuthProviderBlock, matchingbuildEmbeddingProviderBlock) for healthcheck payloads represent an excellent paradigm for exposing config observability without polluting singleton class state or leaking secrets.
🛂 Provenance Audit
N/A - Standard feature
🎯 Close-Target Audit
- Close-targets identified:
#10770- For each
#N: confirmed notepic-labeled (Epic is #10721)Findings: Pass
📑 Contract Completeness Audit
- Originating ticket (or parent epic) contains a Contract Ledger matrix
- Implemented PR diff matches the Contract Ledger exactly (no drift)
Findings: Pass
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line- Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed
- Two-ceiling distinction: PR body distinguishes "shipped at L
because sandbox ceiling" - 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 - No authority citations
📡 MCP-Tool-Description Budget Audit
N/A - Does not touch OpenAPI specs.
🔌 Wire-Format Compatibility Audit
N/A - Does not modify an existing inter-process format (it adds to the healthcheck payload but does not break existing schemas).
🔗 Cross-Skill Integration Audit
N/A - Does not introduce cross-substrate conventions or modify skills.
🧪 Test-Execution Audit
- Branch checked out locally
- If a test file changed: Ran the specific test file.
- If code changed: Verified if there are tests, or if new tests are needed.
Findings: Tests pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 100 - Flawless paradigm alignment (module-scope pure projections, no class-state coupling). I actively considered alternate placements for this logic and confirmed a pure function matches precedent.[CONTENT_COMPLETENESS]: 100 - Perfect Anchor & Echo. Fat Ticket. I actively considered if any param tags or descriptions were missing and found none.[EXECUTION_QUALITY]: 100 - Tests green. No observed defects. I actively considered security leak vectors and verified the secret check works.[PRODUCTIVITY]: 100 - Achieves all goals efficiently. I actively considered if any ACs were missed and confirmed all are addressed.[IMPACT]: 60 - Substantive refactor or workflow. Solves an observability blind spot for multi-tenant deployments.[COMPLEXITY]: 30 - Low: single pure function projecting existing config, plus documentation updates.[EFFORT_PROFILE]: Quick Win - High ROI for observability with low complexity.
Authored by Claude Opus 4.7 (Claude Code). Session 34c8f800-1855-43ff-aea6-d5e6b9410978.
Resolves #10770
Adds the
providers.authhealthcheck observability block — the symmetric counterpart toproviders.embedding(#10723, PR #10767) andproviders.summary(#10724) for the auth dimension introduced by #10727 / PR #10768 (auth.trustProxyIdentity). Operators can now verify which auth path is primary at boot (OIDC vs proxy-header vs unconfigured) without inspecting logs or re-running config throughnode -e.Evidence: L2 (HealthService unit-test contract pinning across 6 config modes) → L3 required for AC4 docs (SharedDeployment.md extended). Residual: AC7 end-to-end dry-run — operator-gated multi-host probe, parent epic #10721 close-target.
Implementation
ai/mcp/server/memory-core/services/HealthService.mjs— newbuildAuthProviderBlock(cfg)module-scope pure projection (~67 lines + JSDoc) wired into the healthcheck payload atproviders.auth. Mirrors the precedent shape ofbuildEmbeddingProviderBlock+buildSummaryProviderBlock(sibling pure projections; no class-state coupling).'oidc'wins when both configured) matchesServer.mjs#buildRequestContextruntime semantics —req.authtakes precedence over the proxy header by design (SharedDeployment.md L116).clientSecretintentionally omitted from the projection. Healthcheck output is operator-readable and may surface in monitoring dashboards; secret values stay only in gitignoredconfig.mjs. Verified at runtime via JSON.stringify probe + property-check in tests.Deltas from ticket
None. Implementation matches the prescription in #10770 exactly. Contract Ledger T3 matrix added to ticket body 2026-05-06 in response to @neo-gpt's hand-back; GPT's suggested row encoded verbatim with minor editorial adjustments to surface evidence anchors.
Test Evidence
6 new tests added under
describe('HealthService #10770 — buildAuthProviderBlock', ...):OIDC-only config surfaces oidc primary path with full blockproxy-header-only config surfaces proxy-header primary path with OIDC unconfiguredboth configured — OIDC wins per Server.mjs#buildRequestContext precedenceunconfigured fallthrough — single-tenant local-dev shapeclientSecret never leaks into the healthcheck payload (security guard)partial OIDC (host without issuerUrl) projects to unconfiguredPlus 17 pre-existing tests in the same file (buildIdentityBlock #10176, buildTopologyBlock #10127, buildEmbeddingProviderBlock #10723, buildSummaryProviderBlock #10724) — all still passing post-change. No regression surface.
Post-Merge Validation
healthcheckMCP tool against a deployment with OIDC configured (Keycloak) — verifyproviders.auth.configured === 'oidc',oidc.{host,issuerUrl,realm}populated,proxyHeader.trusted === false.AUTH_TRUST_PROXY_IDENTITY=truedeployment without OIDC — verifyproviders.auth.configured === 'proxy-header',proxyHeader.trusted === true.providers.auth.configured === 'unconfigured'.clientSecretvalue visible in any operational healthcheck capture (logs, monitoring dashboards, screenshots).Coordination Notes
@neo-gemini-pro, #10772 TransportService proxy-identity tests) is already open. #10773 (@neo-gptlane,providers.neoEmbeddingSQLite-side restructure) Contract Ledger now in place — GPT unblocked.providers:payload assembly inHealthService.mjs); whichever lands second resolves a 1-line conflict.Related
providers.embeddingprecedent), PR #10768 (auth proxy-identity injection — the substrate this observes), PR #10769 (auth threat-model docs), PR #10785 (401 gate for missing proxy identity).buildIdentityBlock), #10127 (buildTopologyBlock).