Skip to content

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

Merged
lidge-jun merged 3 commits into
devfrom
codex/260905-carry-3525
Sep 4, 2026
Merged

fix(responses): expose continuation spill write health (carry of #3525)#3542
lidge-jun merged 3 commits into
devfrom
codex/260905-carry-3525

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Continuation spill write failures accumulated silently behind a healthy readiness signal (#3522). This carry tracks spill write health in src/responses/state.ts as a privacy-safe failure-class enum and exposes it through the management system routes, distinguishing an active failure streak from a recovered one. Instrumentation only — #3522 stays open per the original author.

Supersedes #3525 (maintainer carry: GitHub reports the original CONFLICTING after the tests/<domain>/ migration, while git merge-tree is clean; the carry is the PR head merged with origin/dev, no source edits).

Stack (wp1 merge train, independent layers — each targets dev):

# PR Layer Review focus
1 this carry of #3525 this PR's diff only

Unit: devlog/_plan/260905_open_work_closeout/ (010, 011).

Verification

  • bun run typecheck — exit 0 on the carry head.
  • bun test tests/server/memory-watchdog.test.ts — tests/server/memory-watchdog.test.ts — 13 pass / 0 fail (six tests RED on dev before the change).
  • Original PR CI on 3525's exact head was full-matrix green; exact-head CI on this branch is the merge gate.

Checklist

  • Targets dev
  • Focused regression test present and green
  • Original author credited via Co-authored-by trailer

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

Summary by CodeRabbit

  • New Features

    • Added spill-write health diagnostics, including status, consecutive failure count, categorized failure details, and success/failure timestamps.
    • Added memory observability through the authenticated system memory endpoint, dashboard, and ocx observe memory --json CLI command.
    • Added confirm-gated drain-and-restart guidance for Windows memory troubleshooting.
  • Documentation

    • Expanded API and troubleshooting documentation with diagnostic fields, privacy protections, and recovery behavior.
    • Clarified that raw error messages and filesystem paths are never exposed.

@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: 902d5b4c-e72f-4926-bd08-18979cd128ae

📥 Commits

Reviewing files that changed from the base of the PR and between 980a9fb and 16c5df4.

📒 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/responses/continuation-dedup.test.ts
  • tests/responses/responses-state.test.ts
  • tests/server/memory-watchdog.test.ts

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


📝 Walkthrough

Walkthrough

The change adds privacy-safe spill-write health metrics to response-state diagnostics. Spill paths record classified failures and recovery timestamps. The authenticated memory endpoint and related documentation expose the new fields, with tests covering initial, degraded, and recovered states.

Changes

Spill-write health diagnostics

Layer / File(s) Summary
Health tracking and metric projection
src/responses/state.ts
Spill-write paths now record classified failures, consecutive failure counts, and success or failure timestamps. responseStateMetrics() exposes initial, healthy, or degraded status with fixed error codes.
Health metrics validation
tests/responses/responses-state.test.ts, tests/responses/continuation-dedup.test.ts, tests/server/memory-watchdog.test.ts
Tests cover metric shapes, ACL retry exhaustion, repeated failures, recovery, reset behavior, fixed error codes, and the 17-field memory response contract.
Authenticated diagnostic surface
src/server/management/system-routes.ts, docs-site/src/content/docs/reference/management-api.md, docs-site/src/content/docs/troubleshooting/windows-memory.md, structure/05_gui-and-management-api.md
Route comments and documentation describe the spill-write fields, privacy boundary, dashboard behavior, and authenticated memory snapshot command. /healthz remains a liveness-only surface.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 16c5d

The authenticated memory diagnostics expose the new spill-write health data without changing liveness behavior, and the covered status and recovery paths are ready to merge.

Sequence Diagram(s)

sequenceDiagram
  participant SpillWritePaths
  participant ResponseStateMetrics
  participant MemoryEndpoint
  SpillWritePaths->>ResponseStateMetrics: Record classified spill-write result
  ResponseStateMetrics->>MemoryEndpoint: Provide health status and timestamps
  MemoryEndpoint-->>MemoryEndpoint: Return authenticated memory diagnostics
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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: exposing continuation spill-write health in responses. The fix(responses) scope and carry reference are relevant and concise.
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.)

  • Fix all pre-merge checks with AI
✨ 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 codex/260905-carry-3525

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.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 70 / 80

설명

이 PR은 이슈 #3522에서 보이는 Windows continuation spill 쓰기 실패가 /healthz나 readiness 뒤에는 조용히 쌓이는 문제를, 고치지 않고 먼저 보이게 만드는 관측(instrumentation) 캐리입니다. 원본은 @Ingwannu 의 #3525이고, GitHub는 원본을 CONFLICTING으로 표시하지만(대개 tests/<domain>/ 레이아웃 이동 이후의 경로 리포트 노이즈) 본문대로 git merge-tree는 깨끗합니다. 그래서 소스 편집 없이 origin/dev와 합친 헤드를 codex/260905-carry-3525로 올린 메인테이너 캐리이며, 현재 dev HEAD 980a9fbed(#3537 Astra 설정·Fast 가격 정렬)와도 겹치지 않습니다. #3522 자체는 본문대로 열린 채로 둡니다.

왜 중요한지 한 줄로 말하면, continuation을 디스크에 넘기는 spill writer가 ACL 타임아웃·권한·용량 때문에 반복 실패해도 /healthz는 계속 살아 있고, 기존 spillWriteFailures누적 숫자만 있어서 “지금 계속 깨지고 있는지 / 이미 회복했는지 / 무슨 종류의 실패인지”를 구분할 수 없었습니다. 이 PR은 src/responses/state.tsspillWriteHealth를 두고, 성공·실패 경로를 noteSpillWriteSuccess / noteSpillWriteFailure로 모은 뒤 responseStateMetrics()에 다섯 필드를 더합니다. spillWriteStatusinitial → 실패 후 degraded → 같은 프로세스에서 다음 성공 시 healthy, spillWriteConsecutiveFailures는 연속 실패 횟수, spillLastWriteFailureCode는 경로·메시지 없이 고정 enum(EACLRETRYEXHAUSTED, ETIMEDOUT, EACCES, ENOSPC, EFBIG, EIO, ECAPACITY, ELOOP, EUNKNOWN), 그리고 마지막 실패·성공 시각입니다. 이 값은 이미 인증된 GET /api/system/memoryresponseState 블록으로 나가고, /healthz에는 넣지 않습니다. 문서(docs-site troubleshooting·management-api, structure/05_gui-and-management-api.md Decision Log)도 같은 경계를 분명히 합니다.

코드 경로를 현재 dev 체크아웃 기준으로 보면, 변경의 중심은 src/responses/state.ts입니다. 비동기 ACL 재시도가 소진되면 EACLRETRYEXHAUSTED로 분류하고, 대기열 용량·디스크 캡 거절은 ECAPACITY, 셧다운 터미널화 패스 한도는 ELOOP처럼 override 코드를 직접 넘깁니다. EPERMEACCES로, EDQUOTENOSPC로 접어 개인정보(경로·원문)를 빼면서도 운영자가 조치를 고를 수 있게 합니다. src/server/management/system-routes.ts는 주석만 보강하고, 실제 페이로드는 기존처럼 responseStateMetrics()를 그대로 실어 나릅니다. 테스트는 tests/responses/responses-state.test.ts에 Windows ACL 소진→회복, EACCES streak→회복(마지막 실패 코드는 지우지 않음)을 넣고, tests/responses/continuation-dedup.test.tstests/server/memory-watchdog.test.ts에서 memory 표면 필드 개수를 12에서 17로 고정합니다. 회귀 테스트가 원래 dev에서 RED였다는 본문 주장과도 맞습니다.

라인 수준에서 보면 설계 선택은 대체로 안전합니다. 성공해도 spillLastWriteFailureCode를 지우지 않는 것은 테스트로 고정되어 있고, “회복은 됐지만 직전에 무슨 실패였는지”를 남기려는 의도입니다. 대시보드 Memory 카드가 새 spill-health 필드를 그리는지까지는 이 PR이 주장하지 않고, API·ocx observe memory --json·문서에 먼저 올립니다. 루트 원인 수정(#3522)은 다음 레이어로 남겨 둔 점이 명확합니다.

tests/server/memory-watchdog.test.ts - 주석이 “17 after #3522 added”라고 이슈 번호를 적었는데, 실제 추가 PR은 #3525/#3542입니다. 이슈와 PR을 섞어 쓰면 나중에 추적할 때 헷갈립니다.

src/responses/state.ts noteSpillWriteFailure / classifySpillWriteFailure - cause를 최대 4단까지 걷지만, AggregateError의 errors[] 배열은 보지 않습니다. 지금 셧다운 경로는 override로 ELOOP/ETIMEDOUT을 직접 넣어서 당장 깨지진 않지만, 나중에 AggregateError만 throw하는 경로가 생기면 EUNKNOWN으로 뭉개질 수 있습니다.

docs-site/.../windows-memory.md - spill-health는 API에 있다고 잘 적었고, 대시보드 카드는 “memory and continuation-size fields”만 그린다고 범위를 좁혀 두었습니다. 운영자가 UI만 보면 새 필드를 못 볼 수 있으니, 머지 후 대시보드 노출은 별도 후속인지 한 줄로 적어 두면 더 좋습니다.

#3525 - 이 캐리가 머지되면 원본 #3525는 Landed via #3542 at <commit> + landed-via-maintainer로 닫아야 합니다. #3522는 본문대로 열어 둡니다.

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

너의 추천
CI(이 캐리 헤드 전체 매트릭스)만 초록이면 dev에 머지하세요. 카탈로그/#3537과 무관한 관측 전용 레이어이고 MERGEABLE입니다. 머지 후 #3525는 landed-via-maintainer로 닫고, #3522는 열어 둔 채 다음으로 “실패를 줄이는” 수정 PR을 받으면 됩니다. 주석의 #3522#3525/#3542 오기만 후속으로 고치면 충분합니다.

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer admin merge (ruleset bypass recorded per MAINTAINERS.md): carry of #3525, exact-head CI green on 16c5df4 (28 pass / 2 skipped / 0 fail; #3522 stays open per the original author (instrumentation only)). wp1 of the 260905 open-work closeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants