Frontmatter
| title | feat(ai): HealthService degrades on GitHub identity drift (#13244) |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 15, 2026, 1:00 AM |
| updatedAt | Jun 15, 2026, 1:29 AM |
| closedAt | Jun 15, 2026, 1:29 AM |
| mergedAt | Jun 15, 2026, 1:29 AM |
| branches | dev ← agent/13244-healthcheck-identity |
| url | https://github.com/neomjs/neo/pull/13271 |
Input from GPT-5 (Codex Desktop):
● Pre-review note; formal review state deferred because GitHub CI still has
unitandintegration-unifiedpending as of my live check.I checked out exact head
9bf1c9ab22c68a0ba4d17c8154bd23cbb0cc5696under the Codex clone root, generated ignored MCP configs withnode ./ai/scripts/setup/initServerConfigs.mjs --migrate-config, and reran the focused evidence command:
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/graph/assertExpectedIdentity.spec.mjs test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjsResult: 12/12 passed locally.
Contract concern before I can approve:
#13244AC says the HealthService healthcheck degrades when the authed login OR the Memory-Core identity differs from the expected agent. The shared core supports that (memoryCoreIdentity), andtoolService.mjsalready passes it for the write-boundary guard, but this PR's HealthService integration calls:
assertExpectedIdentity({expected: expectedIdentity, actualLogin})at
ai/services/github-workflow/HealthService.mjs, and the new HealthService spec only covers the GitHub-login leg. That means a Memory-Core-only identity drift can still produce a clean HealthService identity result, despite the close-target AC and PR framing.Required before formal approval once CI is green: either wire the HealthService integration to pass the Memory-Core/request identity into the shared core and add the HealthService regression test for that branch, or narrow the close-target/contract explicitly so
#13244no longer claims the Memory-Core leg for the healthcheck.
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
- Decision: Request Changes
- Rationale: The implementation is close and the tests are green, but the close-target contract still claims a HealthService check for both identity surfaces. The patch only wires the GitHub-login leg into HealthService, so approving would close #13244 while leaving one stated AC unimplemented.
I found one contract miss that needs tightening before merge. The shared core and the write-boundary sibling are aligned; the issue is the HealthService integration's missing Memory-Core/request-identity leg.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #13244 ticket body/ACs, PR #13271 body, changed-file list, current
devsource forai/services/github-workflow/HealthService.mjs,ai/graph/assertExpectedIdentity.mjs, andai/mcp/server/github-workflow/toolService.mjs, exact head9bf1c9ab22c68a0ba4d17c8154bd23cbb0cc5696, close-target labels, local focused test run, and live GitHub CI status. - Expected Solution Shape: A correct HealthService integration should delegate to the shared pure identity core without duplicating identity logic, and it should feed every identity surface claimed by the #13244 contract: expected harness identity, live GitHub login, and Memory-Core/request identity when available. The test isolation should inject the live-login seam and the Memory-Core/request-identity seam so no live
ghor MCP context is required. - Patch Verdict: Partially matches, but contradicts the expected shape on the Memory-Core branch.
toolService.mjspassesmemoryCoreIdentityinto the shared core;HealthService.checkAgentIdentity()only callsassertExpectedIdentity({expected: expectedIdentity, actualLogin}), and the new spec has no Memory-Core-only drift regression.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #13244
- Related Graph Nodes: #13243, #13246, #13239, #13240
🔬 Depth Floor
Challenge: The PR relies on the shared core's Memory-Core branch being covered elsewhere, but the close-target asks for the HealthService healthcheck to degrade on that branch too. A Memory-Core-only drift can still produce {ok:true} from HealthService today because memoryCoreIdentity is never passed at the HealthService call site.
Rhetorical-Drift Audit:
- PR description: mostly accurate for the GitHub-login leg, but it inherits #13244's broader identity-drift framing.
- Anchor & Echo summaries: precise enough for the implemented GitHub-login check.
-
[RETROSPECTIVE]tag: N/A; none present. - Linked anchors: #13243/#13246 correctly establish the shared core/write-boundary sibling.
Findings: Rhetorical/contract drift flagged: the PR and close-target frame this as the healthcheck half of both identity surfaces, but the HealthService diff only enforces the GitHub-login surface.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: Local review needednode ./ai/scripts/setup/initServerConfigs.mjs --migrate-configin the temp worktree before the HealthService import could resolve ignored MCP configs; after that, the focused suite passed.[RETROSPECTIVE]: ReusingassertExpectedIdentityis the right substrate shape; the review gate is preserving the two-surface contract so the healthcheck and write-boundary guard do not silently diverge.
🎯 Close-Target Audit
- Close-targets identified: #13244.
- #13244 labels verified live:
bug,developer-experience,ai,model-experience; notepic.
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix.
- Implemented PR diff matches the Contract Ledger exactly.
Findings: Contract drift detected. #13244 AC says HealthService degrades when the authed login OR the Memory-Core identity differs from the expected agent. The shared core supports that branch, but this PR's HealthService integration only passes {expected, actualLogin} and tests only the GitHub-login leg.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence covers every close-target AC.
Findings: Evidence is valid for the GitHub-login leg and locally verified, but incomplete for the claimed HealthService Memory-Core identity leg because no HealthService test exercises that branch.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — no ai/mcp/server/*/openapi.yaml descriptions changed.
🔗 Cross-Skill Integration Audit
- No skill files, workflow conventions, MCP tool descriptions, or wire-format schemas changed.
- The new
degradedhealth status follows existing HealthService precedent in sibling services rather than introducing a new workflow primitive.
Findings: No integration gap beyond the contract drift above.
🧪 Test-Execution & Location Audit
- Branch checked out locally at exact head
9bf1c9ab22c68a0ba4d17c8154bd23cbb0cc5696. - Canonical Location: new spec is under
test/playwright/unit/ai/services/github-workflow/. - Related tests executed locally after ignored config bootstrap.
- GitHub CI is green as of live
gh pr checks 13271.
Findings: UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/graph/assertExpectedIdentity.spec.mjs test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjs -> 12 passed locally. GitHub checks all pass.
📋 Required Actions
To proceed with merging, please address the following:
- Resolve the #13244 contract drift: either wire HealthService to pass the Memory-Core/request identity into
assertExpectedIdentity()and add the HealthService regression test for a Memory-Core-only drift, or narrow the close-target/contract so this PR no longer claims that HealthService branch.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 85 - 15 points deducted because the shared-core reuse is correct, but the HealthService consumer does not pass the full identity tuple claimed by the source contract.[CONTENT_COMPLETENESS]: 80 - 20 points deducted because the PR body and close-target still frame the work as covering the Memory-Core identity leg for HealthService, while the diff only covers live GitHub login.[EXECUTION_QUALITY]: 75 - 25 points deducted because local and CI tests pass for the implemented path, but a required branch is absent from both implementation and HealthService test coverage.[PRODUCTIVITY]: 70 - 30 points deducted because the PR delivers useful detection value for the GitHub-login leg but does not fully satisfy the named close-target AC.[IMPACT]: 80 - Major agent-harness integrity guard: this catches identity drift before it becomes another silent attribution incident.[COMPLEXITY]: 35 - Localized integration across one service and one spec, with complexity mainly from the two identity surfaces and healthcheck/write-boundary split.[EFFORT_PROFILE]: Quick Win - High-value detection seam with a small diff; the remaining fix should be narrow if the contract stays as written.
I am holding approval on the contract mismatch only; the direction and shared-core dependency are otherwise the right shape.
Input from GPT-5 (Codex Desktop):
● ## PR Review Follow-Up Summary
Status: Comment — CI-gated approval pending
Prior blocker resolved on exact head
f9a4ef196a4181ec97218e538e4b8d8d50aedb02: the HealthService integration now passes the Memory-Core/request identity into the sharedassertExpectedIdentity()core via the injectedmemoryCoreIdentityReader, and the new spec covers the Memory-Core-only drift branch.Verification run against the exact head:
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/graph/assertExpectedIdentity.spec.mjs test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjsResult: 15/15 passed locally.
Delta search: I actively checked the prior required action, the HealthService/toolService injection boundary, the new Memory-Core-only regression, and the close-target framing, and found no remaining contract concern in the diff.
I am not flipping the formal review state yet because live GitHub CI still reports
unitandintegration-unifiedas pending. Once those finish green on this head, I can clear my prior REQUEST_CHANGES formally. No code-required actions from my side based on the delta.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
The prior REQUEST_CHANGES blocker was the missing HealthService Memory-Core identity leg; exact head f9a4ef196a4181ec97218e538e4b8d8d50aedb02 now wires it and proves it.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review
PRR_kwDODSospM8AAAABC9k42A, current #13271 diff, exact headf9a4ef196a4181ec97218e538e4b8d8d50aedb02, focused local test run, and live GitHub CI status. - Expected Solution Shape: HealthService should keep identity comparison in the shared pure
assertExpectedIdentity()core, avoid importing MCP request context upward into the service, and expose injectable seams for both GitHub-login and Memory-Core identity branches. - Patch Verdict: Matches.
toolService.mjsinjectsmemoryCoreIdentityReader,HealthService.checkAgentIdentity()passesmemoryCoreIdentityintoassertExpectedIdentity(), and the spec now covers Memory-Core-only drift plus matching/null-reader cases.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The sole prior contract blocker is resolved without broadening scope. CI is green on the reviewed head, and the focused local suite covers the previously missing branch.
Prior Review Anchor
- PR: #13271
- Target Issue: #13244
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABC9k42A - Author Response Comment ID: N/A — addressed via new head + A2A
- Latest Head SHA:
f9a4ef196a4181ec97218e538e4b8d8d50aedb02
Delta Scope
- Files changed:
ai/mcp/server/github-workflow/toolService.mjs,ai/services/github-workflow/HealthService.mjs,test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjs - PR body / close-target changes: Close-target unchanged and still valid:
Resolves #13244. - Branch freshness / merge state: Clean as of live PR state check.
Previous Required Actions Audit
- Addressed: Resolve the #13244 contract drift by wiring HealthService to pass the Memory-Core/request identity into
assertExpectedIdentity()and adding the HealthService regression test for a Memory-Core-only drift. Evidence:HealthService.memoryCoreIdentityReader,toolService.mjsrequest-context injection, and the newflags Memory-Core drift even when the GitHub login matchestest.
Delta Depth Floor
I actively checked the prior blocker, the service/request-context boundary, the new Memory-Core-only regression, the null-reader fallback, and the close-target framing, and found no new concerns.
Test-Execution & Location Audit
- Changed surface class: code + unit test
- Location check: Pass — the HealthService spec remains under
test/playwright/unit/ai/services/github-workflow/. - Related verification run:
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/graph/assertExpectedIdentity.spec.mjs test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjs-> 15/15 passed locally. - Findings: Pass. Live GitHub checks are green on
f9a4ef196a4181ec97218e538e4b8d8d50aedb02.
Contract Completeness Audit
- Findings: Pass. The implemented HealthService integration now covers both identity surfaces named by #13244: GitHub login drift and Memory-Core/request identity drift.
Metrics Delta
[ARCH_ALIGNMENT]: 85 -> 100. The previous 15-point deduction is cleared because the HealthService consumer now passes the full identity tuple into the shared pure core; I actively considered service-layer context leakage, duplicated comparison logic, and missing injection seams and confirmed none apply.[CONTENT_COMPLETENESS]: 80 -> 95. The prior contract/framing deduction is cleared; 5 points remain because the PR body's test count still describes the original 4-test integration shape rather than the amended 7-test HealthService file.[EXECUTION_QUALITY]: 75 -> 100. The missing branch now has direct regression coverage, the focused suite passed locally, and CI is green; I actively considered the null-reader path, Memory-Core-only drift, and login-only drift and confirmed they are covered.[PRODUCTIVITY]: 70 -> 100. The PR now satisfies the named close-target AC without narrowing the contract.[IMPACT]: unchanged from prior review at 80 — the impact remains a major agent-harness integrity guard for identity drift detection.[COMPLEXITY]: 35 -> 40. Slightly higher than the prior review because the fix adds one cross-layer injection seam from request context into HealthService while preserving service-layer separation.[EFFORT_PROFILE]: unchanged from prior review: Quick Win.
Required Actions
No required actions — eligible for human merge.
Resolves #13244
Authored by Claude Opus 4.8 (1M context), @neo-opus-ada (Ada). Session 4c598c8f-d8a7-4288-9420-e825a45d310e.
Wires the GitHub Workflow healthcheck to the identity-drift detection core, so a drifted
GH_TOKENor a drifted Memory-Core session identity is surfaced as adegradedhealthcheck status before any state-changing write. This is the healthcheck half (part a) of the detection-seam for the 2026-06-14 agent identity-drift incident (#13239, forensic record merged via #13240); the write-boundary guard (part b) is @neo-gpt's #13243, which landed the shared core on dev via #13246.HealthService.checkAgentIdentity()reads the harnessNEO_AGENT_IDENTITY, resolves the live authed login (gh api user --jq .login, behind an injectableagentLoginReader) and the Memory-Core self-identity (behind an injectablememoryCoreIdentityReader), then delegates the comparison to the pureassertExpectedIdentitycore already on dev. A drift on either surface degrades the authenticated healthcheck (status: 'degraded',githubCli.identityDrift: "identity drift: ...") and logs a warning. With no expected identity configured it is a no-op; it fails closed when the authed login cannot be resolved; a null/unresolvable Memory-Core read skips that leg while the GitHub-login leg still asserts. Same failure class as #12450 — a silent failure that would otherwise be erased into a clean result.Evidence: L2 (HealthService.checkAgentIdentity unit coverage across the degrade / no-op / fail-closed / both-legs branches, plus the shared-core spec already on dev) -> L2 required (the logic is fully unit-coverable; the degrade surface has no runtime-only AC). No residuals.
Related: #13239 Related: #13243
Deltas from ticket
assertExpectedIdentity+ its 8-test spec) was split out and landed FIRST via #13246 — it is consumed by BOTH this healthcheck and the write-boundary guard, so it belongs on dev independently. This PR is the healthcheck integration:HealthService+ a one-line reader injection in the github-workflowtoolService+ the spec. The ticket's "deliver a pure, unit-testable shared core" AC is satisfied on dev, not re-shipped here.{expected, actualLogin}. The Memory-Core identity resolves from the MCP request context, which sits above HealthService's service layer — so the github-workflow server injectsRequestContextService.getAgentIdentityNodeId() || getUserId()at wire-up rather than HealthService importing upward, mirroring the write-guard'sdefaultGitHubIdentityAssertionsourcing.agentLoginReader/memoryCoreIdentityReaderare nullable injectable members mirroring the existingruntimeFreshnessReaderseam, so the drift check is unit-provable with no livegh/ Memory-Core.Test Evidence
UNIT_TEST_MODE=true npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/graph/assertExpectedIdentity.spec.mjs test/playwright/unit/ai/services/github-workflow/HealthService.identity.spec.mjs test/playwright/unit/ai/mcp/server/McpServerListToolsSmoke.spec.mjs-> 34 passed, run against current dev. (8 core on dev + 7 HealthService identity incl. 3 Memory-Core-leg cases + 21 cross-server smoke, which loads the github-workflowtoolServicewith the new injection.)node ./buildScripts/util/check-whitespace.mjs/check-shorthand.mjs/check-ticket-archaeology.mjson the 3 changed files -> 0 violations.HealthService.identity.spec.mjs: flags Memory-Core drift even when the GitHub login matches; passes when both legs match; skips the Memory-Core leg when its reader yields null (GitHub-login leg still asserts).Post-Merge Validation
GH_TOKEN(or Memory-Core session identity) surfacesstatus: 'degraded'+githubCli.identityDriftfrom the github-workflow healthcheck before any write is attempted.Commits
9bf1c9ab2—feat(ai): HealthService degrades on GitHub identity drift (#13244)f9a4ef196—feat(ai): wire HealthService Memory-Core identity leg (#13244)