Skip to content

fix(diagnostics): distinguish spill ACL timeout origins - #3790

Merged
lidge-jun merged 9 commits into
devfrom
codex/track3-windows-spill-origins-a4b0
Sep 6, 2026
Merged

fix(diagnostics): distinguish spill ACL timeout origins#3790
lidge-jun merged 9 commits into
devfrom
codex/track3-windows-spill-origins-a4b0

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

  • Distinguish a spill retry returning a timeout from the ACL helper refusing through its remembered timeout state. Add a closed last-origin field and separate cumulative terminal-publication counters to existing authenticated memory diagnostics.
  • Preserve retry predicates, limits, ACL enforcement, memo consumption, cancellation and readiness. Add stable-directory refusal, producer/decoder/privacy and strict scalar-consumer regressions.
  • Refs [Bug][Windows] Continuation spill failures accumulate behind healthy readiness after #3011 (observed on 2.39.0) #3522. This is diagnostic discrimination, not a runtime recovery fix; the issue must remain open.

Track 3 manual stack, layer 4, based on #3789. Earlier layers are #3786 and #3788. No native stack registration.

Verification

  • Independent Astra high code/security review: PASS, zero blockers. It checked unchanged retry/auth semantics, bounded cause decoding, effective error precedence, stable-directory fixture and scalar consumers.
  • git diff --cached --check passed. No local tests, typecheck, builds or suites, per explicit maintainer instruction.
  • Runtime validation: the cumulative exact-source CI evidence is recorded below; lower-layer tests are deferred under the explicit maintainer policy.
  • Additional command-stage/OS-launch diagnostics and actual production recovery are out of scope. Tests have been authored but not executed locally.

Cumulative tested source: 9ea896737bd17085ebb8a69b3320c9cace61889e, based on dev eff908e0fb9452d5ff2952af1c5dafc1c4c35dd9.

  • Cross-platform CI: all 25 jobs successful. Attempt 1 failed only the existing Windows cli-models first-child 15-second watchdog. One unchanged-head rerun of that shard completed 3,489 cases (3,465 pass, 24 skip, 0 fail); the first models case took 943 ms. The initial failure is retained; the slow phase remains unproven and no flakiness fix is claimed.
  • Service lifecycle: Linux, macOS and Windows passed on the same source head.
  • Documentation build: passed after checking out and asserting that exact source. Its isolated workflow-definition commit is different and is not part of this stack; no deployment occurred.
  • Independent scoped code/security and integration reviews: PASS. The two earlier Windows fixture-budget changes retain production behavior and original assertions, backed by readiness controls and quota restart controls. Diagnostic workflows, instrumentation and mutations are excluded from delivery.

The maintainer explicitly authorizes dev admin integration and final-head-first validation for this manual chain: #3786#3788#3789#3790#3796#3802. Lower automatic tests are deferred with [skip ci], not represented as passing. No local tests, typecheck, builds or suites were run; pushes used --no-verify. Merge commits preserve source ancestry and suppress duplicate lower CI; fresh head/base/actor/review checks and resulting tree checks accompany each merge.

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.

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

Landing confirmed: merged into dev as c2b4dc0696f0367ae21d4225e82f75884ec8b70e. The final chain merge is f89b815090020f52089801b50f569a777bdfdc0a; its tree 846b8ffc383fc0ba99486917bdfb820a0fa432ba equals the tested cumulative source. Source ancestry and every merge were verified after fetching dev.

Summary by CodeRabbit

  • New Features

    • Added response-spill diagnostics to the system memory endpoint, including failure origins and cumulative terminal-failure counters.
    • Clarified that diagnostics are process-local and privacy-safe, and distinguish timeout categories.
  • Documentation

    • Expanded Windows troubleshooting guidance for spill-write failures and recovery behavior.
    • Added safeguards and human-handoff instructions for secret-bearing access-key creation and rotation workflows.
  • Tests

    • Added coverage for diagnostic fields, bounded origin handling, timeout memo behavior, and secret-bearing command detection.

t and others added 2 commits September 7, 2026 01:50
Carry #3324 from 2933cc5 and 7734e75. Preserve prior exact revocation authority, benign commit/abort, and managed connect rotation. Add bounded literal recipe regression detector; this is guidance and static warning, not runtime enforcement.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Add closed origin and cumulative terminal-publication counters without changing ACL decisions, retry limits, memo handling, cancellation or readiness. Refs #3522; runtime recovery remains unresolved. Final stack CI pending.

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

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 88a11f4b-7491-48f8-a3a3-d9e8c786beec

📥 Commits

Reviewing files that changed from the base of the PR and between ad5285e and cc7568a.

📒 Files selected for processing (12)
  • docs-site/src/content/docs/reference/management-api.md
  • docs-site/src/content/docs/troubleshooting/windows-memory.md
  • skills/ocx/SKILL.md
  • skills/ocx/references/03_recipes.md
  • skills/ocx/references/05_remote_hub.md
  • src/lib/windows-secret-acl.ts
  • src/responses/state.ts
  • tests/ci-workflows/skill-ocx.test.ts
  • tests/responses/continuation-dedup.test.ts
  • tests/responses/responses-state.test.ts
  • tests/server/memory-watchdog.test.ts
  • tests/windows/windows-secret-acl.test.ts

📝 Walkthrough

Walkthrough

The change adds privacy-safe Windows spill-failure origins and cumulative metrics to response-state diagnostics. It documents the new memory fields. It also restricts plaintext access-key operations in ocx agent guidance and adds tests that detect unsafe command examples.

Changes

Windows spill diagnostics

Layer / File(s) Summary
ACL timeout classification
src/lib/windows-secret-acl.ts, tests/windows/windows-secret-acl.test.ts
previousTimeoutError marks memo refusals with timeout_memo_refusal at lines 718-733. Sync and async tests cover inspection, memo retention, recovery, and ACL-call limits at lines 379-414 and 1053-1097.
Spill failure metrics and publication
src/responses/state.ts, tests/responses/responses-state.test.ts, tests/responses/continuation-dedup.test.ts, tests/server/memory-watchdog.test.ts
Response state classifies bounded ACL causes, records the last origin, increments terminal-failure counters, exposes the fields, and resets them. Tests cover metric shapes, decoding bounds, recovery, and the memory endpoint.
Spill diagnostics documentation
docs-site/src/content/docs/reference/management-api.md, docs-site/src/content/docs/troubleshooting/windows-memory.md
The documentation names both fixed origins and counters, and describes retention, process locality, and non-disclosure behavior at management-api.md:350 and windows-memory.md:60-75.

ocx secret-bearing command safeguards

Layer / File(s) Summary
Secret-bearing command policy
skills/ocx/SKILL.md, skills/ocx/references/03_recipes.md, skills/ocx/references/05_remote_hub.md
The guidance at SKILL.md:93-113 and the referenced recipe sections keeps plaintext key creation and rotation-start operations outside agent sessions. It separates configuration confirmation from explicit revocation approval and distinguishes raw operations from ocx connect rotate.
Secret command documentation validation
tests/ci-workflows/skill-ocx.test.ts
The tests at lines 166-296 detect secret-bearing CLI and HTTP commands across aliases, wrappers, continuations, and inline code. They allow commit, abort, list, remove, delete, and managed-connect operations.

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

Sequence Diagram(s)

sequenceDiagram
  participant SpillWriter
  participant WindowsSecretACL
  participant ResponseState
  participant MemoryAPI
  SpillWriter->>WindowsSecretACL: publish response spill
  WindowsSecretACL-->>SpillWriter: classified timeout result
  SpillWriter->>ResponseState: record failure origin and counter
  MemoryAPI->>ResponseState: request responseStateMetrics()
  ResponseState-->>MemoryAPI: return privacy-safe diagnostics
Loading
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/track3-windows-spill-origins-a4b0

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 6, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

Final CI 34047664926 macOS job 101526036532 found one remaining 17-field assertion. Assert the complete reviewed 20-field API contract instead of only relaxing its count; unexpected private/replay fields still fail. Production projection is unchanged.
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 58 / 80

지금 dev HEAD(24c761a05, package 2.45.0)의 Windows spill 쓰기 진단은 spillLastWriteFailureCodeETIMEDOUT / EACLRETRYEXHAUSTED 같은 코드만 보여 줍니다. 운영자가 보면 “타임아웃이 또 났다”는 것만 보이고, 이번에 두 번째 spill 시도가 진짜로 타임아웃을 돌려준 건지, ACL 헬퍼가 예전에 기억해 둔 timeout memo 때문에 아예 명령을 안 돌리고 거절한 건지 구분할 수 없습니다. 이슈 #3522가 말한 “healthy readiness 뒤에 spill 실패가 쌓인다”를 조사하려면 그 두 origin이 갈라져야 합니다. 이 PR은 런타임 복구를 고치지 않고, 관측만 나눕니다. windows-secret-acl의 memo 거절 경로에 aclFailureOrigin: "timeout_memo_refusal"을 붙이고, spill writer는 cause를 최대 4단까지 훑어 memo 표식을 찾거나, 재시도가 실제로 타임아웃을 돌려준 경우 retry_returned_timeout으로 기록합니다. 누적 카운터 spillAclRetryReturnedTimeouts / spillAclTimeoutMemoRefusals는 터미널 게시 실패만 세고, 성공해도 last-failure 필드와 누적은 지우지 않습니다. management API·windows-memory 문서에 고정 enum과 프로세스-로컬 의미를 적었고, 응답 필드 allowlist를 17에서 20으로 핀했습니다.

경로를 확인했습니다. 실파일만 건드립니다: src/lib/windows-secret-acl.ts, src/responses/state.ts, docs-site management-api/windows-memory, 그리고 네 개 테스트. 없는 파일을 새로 만들지 않았습니다. base는 dev가 아니라 codex/track3-agent-key-recipes-a4b0(#3789)입니다. Track 3 수동 체인 #3786 → #3788 → #3789 → #3790 → #3796 → #3802이고, 아래 층 #3786·#3788·#3789가 모두 OPEN DRAFT로 열려 있어 바닥 PR이 빠진 상태는 아닙니다. 다만 이 층만 단독으로 dev에 올리려면 base 재지정이 필요합니다. types/config 분할 캠페인과는 겹치지 않습니다. #3522는 본문대로 열어 두어야 합니다(진단만, 복구 아님).

src/lib/windows-secret-acl.ts previousTimeoutError - memo 거절에만 aclFailureOrigin을 붙인다. 첫 실제 타임아웃에는 붙이지 않아서 origin 혼선을 줄인다.
src/responses/state.ts spillAclMemoRefusalOrigin / noteSpillWriteFailure - cause 체인·override·retryOrigin 우선순위가 문서와 맞다. 터미널 게시만 카운트한다는 주석이 코드와 일치한다.
src/responses/state.ts responseStateMetrics - spillLastWriteFailureOrigin과 두 누적 카운터가 authenticated memory 표면에만 노출된다. /healthz에는 안 탄다.
docs windows-memory.md - origin이 OS 명령을 가리키지 않는다는 한계(예산 만료, compliance 선검사)를 솔직히 적었다.
tests/... 20-field allowlist - 새 필드는 privacy 리뷰 게이트다. 예상 밖 private/replay 필드는 여전히 실패한다.
Track 3 base - dev가 아니다. 아래 #3786·#3788·#3789가 열려 있어 스택은 완전하다.

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

  • Track 3을 #3786 → … → #3790 순으로 묶을지, 이 진단 층만 dev로 retarget해 먼저 올릴지
  • #3522를 이 PR 머지 후에도 계속 열어 둘지(본문 권고: 유지). 복구/재시작은 별 이슈·PR
  • 아래 층 [skip ci] 정책 유지 여부. 검증 증거는 스택 헤드 Cross-platform CI(93184f64c)에 달려 있다
  • Windows Desktop lifecycle(fix: unblock Windows lifecycle tests and reduce Cursor blob admission cost #3771)과 관측 표면을 어떻게 나란히 둘지(이 PR은 spill ACL origin만)

너의 추천
드래프트 유지한 채 Track 3 순서대로 가져가라. #3522 조사에 필요한 origin 분리고, 재시도·ACL·memo·취소 의미는 그대로다. 단독 급하면 skills/storage와 무관한 파일이라 retarget은 가능하지만, 스택 정본이면 #3789 다음에 올리는 편이 이력상 깔끔하다. #3522는 닫지 말 것. types/config 분할로 무효화될 PR이 아니다.

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

@lidge-jun
lidge-jun changed the base branch from codex/track3-agent-key-recipes-a4b0 to dev September 6, 2026 20:19
@lidge-jun
lidge-jun marked this pull request as ready for review September 6, 2026 20:20
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 6, 2026 20:20
@lidge-jun
lidge-jun merged commit c2b4dc0 into dev Sep 6, 2026
19 of 22 checks passed
@lidge-jun
lidge-jun deleted the codex/track3-windows-spill-origins-a4b0 branch September 6, 2026 20:20
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-06T20:21:55.632213Z cc7568a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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.

1 participant