Skip to content

fix(responses): expose continuation spill write health - #3525

Closed
Ingwannu wants to merge 1 commit into
devfrom
fix/3522-spill-health-diagnostics
Closed

fix(responses): expose continuation spill write health#3525
Ingwannu wants to merge 1 commit into
devfrom
fix/3522-spill-health-diagnostics

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a privacy-safe spill-write health projection to responseStateMetrics() and the existing authenticated GET /api/system/memory / ocx observe memory --json surface.
  • Preserve the cumulative write counters while adding initial / healthy / degraded status, a consecutive-failure streak, last failure/success timestamps, and a closed error-code union. Raw messages, paths, response ids, payloads, and causes never enter the response.
  • Reset the streak only after a durable spill publication is installed. Superseded/cancelled publications remain neutral.
  • Classify a second Windows ACL timeout as EACLRETRYEXHAUSTED without weakening ACL hardening, changing readiness, or adding restart behavior.
  • Cover repeated failure then success and Windows ACL exhaustion then healthy-runner recovery in the same process.
  • Document the diagnostic contract and the decision to keep it off unauthenticated /healthz.

This fixes the current observability gap identified in #3522. It does not claim that the original 2.39.0 incident root cause has been reproduced on the current release, so the issue should remain open for an exercised 2.42/2.43+ snapshot after this instrumentation lands.

Refs #3522

Verification

  • Exact-head focused suite after rebasing onto dev b20af6668: bun test tests/responses/responses-state.test.ts tests/responses/continuation-dedup.test.ts tests/memory-watchdog.test.ts — 172 pass, 0 fail.
  • Exact-head bun run typecheck — pass.
  • Exact-head bun run privacy:scan — pass.
  • Full repository suite on the identical implementation before the test-layout-only test(layout): move adapters, responses, lab, gui into tests/<domain>/ (#3497) #3513 rebase: 17,824 pass / 16 skip / 0 fail, followed by every serialized gate passing.
  • docs-site build — 425 pages built successfully.
  • All tests used fresh temporary HOME, OPENCODEX_HOME, and CODEX_HOME; inherited data/admin auth tokens were cleared only inside the test processes.
  • Protected local Codex/OpenCodex/Paseo config mode, size, and SHA-256 remained unchanged.
  • git diff --check — pass.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features

    • Added spill-write health diagnostics to the authenticated system memory endpoint and dashboard, including status, failure streaks, classified errors, and timestamps.
    • Added the ocx observe memory --json command for viewing memory and spill-write health details.
    • Added privacy-safe reporting that excludes raw errors and filesystem paths.
  • Bug Fixes

    • Spill-write failures now preserve diagnostic state and report recovery after successful publication.
  • Documentation

    • Expanded Management API and Windows memory troubleshooting guidance, including the confirm-gated drain-and-restart workflow.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 448a5778-45c1-47fd-be95-5552147624f1

📥 Commits

Reviewing files that changed from the base of the PR and between b20af66 and 58760d5.

📒 Files selected for processing (8)
  • docs-site/src/content/docs/reference/management-api.md
  • docs-site/src/content/docs/troubleshooting/windows-memory.md
  • src/responses/state.ts
  • src/server/management/system-routes.ts
  • structure/05_gui-and-management-api.md
  • tests/memory-watchdog.test.ts
  • tests/responses/continuation-dedup.test.ts
  • tests/responses/responses-state.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Response spill writes now track classified health state, failure streaks, and event timestamps. The authenticated memory endpoint exposes these fields without raw errors or paths. Tests and documentation cover failure, recovery, privacy, CLI snapshots, and dashboard behavior.

Changes

Response spill-write health

Layer / File(s) Summary
Centralized spill-write health tracking
src/responses/state.ts:173-243, src/responses/state.ts:420-2012
Spill paths now record classified failures and successes, including ACL retry exhaustion, capacity failures, shutdown ELOOP failures, timestamps, and consecutive-failure counts.
Metrics and authenticated memory surface
src/responses/state.ts:2188-2366, src/server/management/system-routes.ts:18-20, structure/05_gui-and-management-api.md:132-153, docs-site/src/content/docs/reference/management-api.md:247
ResponseStateMetrics now exposes status, failure metadata, and success metadata. The authenticated memory surface documents privacy-safe fields, while /healthz remains liveness-only.
Validation and operational documentation
tests/responses/responses-state.test.ts:1079-3361, tests/memory-watchdog.test.ts:196-241, tests/responses/continuation-dedup.test.ts:313-315, docs-site/src/content/docs/troubleshooting/windows-memory.md:53-74
Tests cover field shape, failure classification, tombstones, recovery, timestamps, and reset behavior. Documentation adds dashboard behavior and ocx observe memory --json usage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 28850

This change adds privacy-safe spill-write health diagnostics to the authenticated memory interface while keeping liveness behavior unchanged. The documented status, counters, error classes, timestamps, and recovery behavior are covered with no remaining merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant SpillPublication
  participant ResponseState
  participant ManagementMemoryAPI
  participant MemoryDashboard
  SpillPublication->>ResponseState: record classified write success or failure
  ManagementMemoryAPI->>ResponseState: read responseState metrics
  ResponseState-->>ManagementMemoryAPI: return status, streak, code, and timestamps
  MemoryDashboard->>ManagementMemoryAPI: request authenticated memory snapshot
  ManagementMemoryAPI-->>MemoryDashboard: render privacy-safe spill-write health
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: exposing continuation spill-write health diagnostics. It is concise, specific, and directly matches the implementation and documented API changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/3522-spill-health-diagnostics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 70 / 80

설명

이 PR은 #3522에서 잡힌 “프로세스는 살아 있는데 continuation spill 쓰기는 계속 실패하는” 상태를, 운영자가 숫자만으로 구분할 수 있게 만드는 관측성 패치입니다. 지금 dev HEAD(b20af6668, #3513 레이아웃 슬라이스·#3523 oauth rotator pin까지)의 src/responses/state.ts에는 spillCounters.writes / writeFailures / readFailures만 있고, 실패가 누적되는지·같은 프로세스에서 회복됐는지·마지막 실패가 어떤 고정 클래스인지가 없습니다. #3522 사고(2.39.0 Windows, 성공 spill이 1,988에 고정된 채 실패가 1만 건 넘게 증가, /healthz는 계속 healthy)가 바로 그 구멍입니다.

브랜치는 ResponseSpillWriteHealth를 프로세스 로컬로 두고, 쓰기가 실제로 설치된 뒤(swapResidentForSpill / atomic replace / admit oversized가 성공한 뒤)에만 noteSpillWriteSuccess로 streak를 0으로 돌리고 lastSuccessAt을 갱신합니다. 실패는 noteSpillWriteFailure로 누적 카운터·연속 실패·고정 코드·실패 시각을 갱신합니다. classifySpillWriteFailure는 message/path를 버리고 cause 체인을 최대 4단까지 올라가 EACCES/ENOSPC/ETIMEDOUT/EACLRETRYEXHAUSTED 같은 닫힌 union만 남깁니다. Windows ACL 재시도가 한 번 더 타임아웃이면 override로 EACLRETRYEXHAUSTED를 찍고, ACL 강화·readiness·자동 재시작은 건드리지 않습니다. 용량 거부는 ECAPACITY, shutdown pass limit은 ELOOP으로 분류해 예전의 “그냥 +1”보다 원인 축이 보입니다.

노출면은 기존 인증 경로만 씁니다. responseStateMetrics()spillWriteStatus(initial/healthy/degraded)·연속 실패·마지막 실패 코드/시각·성공 시각을 더하고, GET /api/system/memoryocx observe memory --json이 그대로 실어 나릅니다. /healthz에는 넣지 않기로 structure Decision Log와 windows-memory 문서에 못 박았습니다. “한 번 실패한 demotion 때문에 프록시를 죽이지 않는다”는 현재 dev의 drain/restart 경계와도 맞습니다. 필드 개수 핀도 12→17로 올리고(tests/memory-watchdog.test.ts, tests/responses/continuation-dedup.test.ts), 반복 실패 후 성공·Windows ACL 소진 후 같은 프로세스 회복을 tests/responses/responses-state.test.ts에 고정했습니다. 작성자 기준 exact-head focused 172 pass, typecheck/privacy/전체 스위트·docs-site까지 통과했다고 했고, base는 dev, #3497 레이아웃 열차와 파일 충돌이 거의 없습니다(테스트가 이미 tests/responses/ 아래).

점수를 70으로 둔 이유는 #3522가 실제 Windows 운영 사고이고, 이 PR이 그 사고의 최소 관측 계약을 정확히 메우기 때문입니다. 루트 원인 재현·자동 재시작·ACL 완화는 의도적으로 범위 밖이고, 이슈를 닫지 말라는 PR 본문 판단도 맞습니다. 대시보드 Memory 카드(gui/src/components/MemoryObservabilityCard.tsx)는 여전히 entries/total/largest/age 위주라 GUI만 보는 운영자는 새 필드를 못 봅니다. 문서가 CLI/JSON을 가리키므로 이번 PR 범위를 넘지는 않지만, #3522 후속 UI는 별도 이슈가 됩니다.

라인 - src/responses/state.ts noteSpillWriteSuccess - 성공은 누적 writes와 streak 리셋·lastSuccessAt만 갱신하고 lastFailureCode/At은 지우지 않습니다. 테스트가 의도적으로 고정한 포렌식 설계입니다. 다만 degraded가 끝난 뒤에도 코드가 남아 “지금 실패 중”으로 오해하기 쉬우니, 운영 문서에 status+streak를 먼저 보라는 한 줄이 있으면 더 안전합니다(windows-memory에는 이미 흐름 설명이 있음).
src/responses/state.ts runPendingResponseSpill catch - states.get(job.id) === candidate && !job.cancelled일 때만 실패를 기록합니다. superseded/cancelled는 중립이라는 요약과 맞습니다. 회귀 테스트로 cancelled 경로가 health를 안 건드리는지 한 줄 더 있으면 리뷰어 부담이 줄어듭니다.
gui/src/components/MemoryObservabilityCard.tsx ResponseState - 타입에 spillWrites/Failures만 있고, 이번 5필드와 예전 replayScopeMismatchDrops도 없습니다. 구조적 타이핑이라 API 추가가 깨지지는 않지만, 대시보드는 spill health를 렌더하지 않습니다. PR 문서가 의도적으로 CLI를 가리키므로 버그는 아니지만 #3522 “운영자가 구분” 목표의 GUI 절반은 아직 비어 있습니다.
docs-site/.../windows-memory.md - spill health를 설명하면서 Memory 카드는 “memory and continuation-size fields”만 렌더한다고 정직히 썼습니다. 좋지만, 카드에 spill 카운터조차 안 나오는 현재 GUI와 맞추면 “대시보드에 status가 보인다”는 오해가 덜합니다.
tests/responses/responses-state.test.ts Windows ACL 회복 시나리오 - 소진 후 성공하면 status=healthy, consecutiveFailures=0, lastFailureCode는 EACLRETRYEXHAUSTED 유지입니다. #3522 가설(고립 테스트는 통과·라이브 프로세스는 실패 누적)과 맞는 계약입니다. 라이브에서 이 코드가 찍히면 재시작/ drain 판단으로 이어질 수 있으니, 자동 재시작을 넣지 않은 선택이 맞습니다.

메인테이너의 판단이 필요한 지점

  • #3522를 이 PR 머지 후에도 열린 채로 두고, 2.42/2.43+ Windows 스냅샷에서 spillWriteStatus/EACLRETRYEXHAUSTED가 실제로 찍히는지 확인한 뒤에만 닫을지(PR 본문 제안)
  • Memory 카드에 status/streak/code를 바로 이어서 넣을지, CLI/ocx observe memory --json만으로 1차 관측을 끝낼지
  • 회복 후에도 lastFailureCode를 남길지, healthy 전환 시 null로 지울지(현재는 유지)
  • degraded가 N회 이상이면 drain-and-restart를 문서/가이드로만 권할지, 제품 신호가 될지(이 PR은 신호만)

너의 추천
CI(특히 Windows/macOS 스위트)만 초록이면 머지하세요. #3522는 닫지 말고, 랜딩 후 Windows에서 ocx observe memory --json으로 degraded→healthy 전환과 failure class를 한 번 실측하세요. GUI 노출은 후속 PR로 두면 이 브랜치 범위를 깨지 않습니다. #3497 레이아웃 열차와 독립이라 끼워 넣어도 됩니다.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu

Ingwannu commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

CI triage update for head 58760d504:

The only completed failure is not in this PR's runtime or focused tests. Linux test 2/4 failed in tests/test-layout.test.ts because current dev contains two concurrent copies of anthropic-quorum-cache.test.ts and reports:

routing/anthropic-quorum-cache.test.ts -> adapters/anthropic

#3526 deletes only the byte-equivalent routing duplicate while retaining all six cases under tests/adapters/anthropic/; I independently normalized the import-depth differences and confirmed the two bodies have the same SHA-256, then approved that exact head. Once #3526 lands on dev, I will rebase this branch and let exact-head CI rerun. I am not changing this PR to work around a base-branch layout defect.

@Ingwannu
Ingwannu force-pushed the fix/3522-spill-health-diagnostics branch from 58760d5 to 288506d Compare September 4, 2026 21:02
@Ingwannu

Ingwannu commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto the current dev containing #3526. New exact head: 288506dc6.

Post-rebase local verification used isolated runtime homes:

  • focused continuation/memory suite: 172 pass, 0 fail
  • TypeScript typecheck: pass
  • privacy scan: pass
  • docs-site production build: 425 pages, pass

The protected local runtime configuration files were unchanged across validation. This remains unmerged pending exact-head CI and independent maintainer review; #3522 also remains open because these diagnostics do not yet prove the reporter’s underlying stall cause.

lidge-jun added a commit that referenced this pull request Sep 4, 2026
… (#3542)

* fix(responses): expose continuation spill write health

* chore: carry #3525 onto current dev

Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com>

---------

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Co-authored-by: jun <jun@lidge.dev>
Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed via #3542 at 7eddfb3

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 4, 2026
@lidge-jun lidge-jun closed this Sep 4, 2026
@lidge-jun
lidge-jun deleted the fix/3522-spill-health-diagnostics branch September 5, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants