From 6625a3bd84eb20ba6810ae705ee9916034ad50c5 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 10:39:42 +0900 Subject: [PATCH 01/24] docs(plan): open the PR merge round with a full 43-PR disposition The dev checkout is mid-optimization, so this round stages on codex/260731-pr-merge-round instead of dev. The user merges it into dev and releases once that unit lands. Four reviewers triaged every open PR against origin/dev=356924263 on disjoint slices, and the load-bearing finding is that seven PRs are already fixed on dev by different commits. Merging those heads now would REVERT the newer work. #736 is the clearest case: dev carries decodeSchtasksOutput() at src/service.ts:364-393 because schtasks /query /xml emits UTF-16LE, and the PR head deletes that block and restores encoding: "utf8" -- landing it reopens #722. A clean merge-tree is not permission to merge. An adversarial audit then returned five blockers, all folded in rather than argued away. Two changed what actually gets merged. #744 was routed as a routine catalog fix, but 59d95c0e4 and 39543a3c0 change OAuth reconciliation, persist provider settings, and move token resolution around the static branch -- security review per MAINTAINERS.md, so it left the batch. #781's topic commit also swaps three /api/logs test files onto a logsFromApiBody helper that accepts both the array and the {logs} envelope; adopting it would pre-accept the very contract batch B rejects, so only the Anthropic file and its own test come across. The audit also caught three PRs missing from a matrix that claimed to be complete (#750, #746, #644), and #644 additionally carries .codexclaw/goalplans/** and two .DS_Store files. Nothing is merged yet. Batch A is six PRs, batch B rebuilds three whose implementations no longer fit the tree, and 20+ are held for security review, provider evidence, or their own cycle -- each with what would unblock it recorded. --- .../_plan/260731_pr_merge_round/000_plan.md | 177 ++++++++++++++++++ .../010_superseded_closeout.md | 91 +++++++++ .../020_batch_a_restack.md | 175 +++++++++++++++++ .../030_batch_b_rebuild.md | 116 ++++++++++++ .../040_deferred_and_handoff.md | 167 +++++++++++++++++ 5 files changed, 726 insertions(+) create mode 100644 devlog/_plan/260731_pr_merge_round/000_plan.md create mode 100644 devlog/_plan/260731_pr_merge_round/010_superseded_closeout.md create mode 100644 devlog/_plan/260731_pr_merge_round/020_batch_a_restack.md create mode 100644 devlog/_plan/260731_pr_merge_round/030_batch_b_rebuild.md create mode 100644 devlog/_plan/260731_pr_merge_round/040_deferred_and_handoff.md diff --git a/devlog/_plan/260731_pr_merge_round/000_plan.md b/devlog/_plan/260731_pr_merge_round/000_plan.md new file mode 100644 index 000000000..f3c5306c7 --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/000_plan.md @@ -0,0 +1,177 @@ +# 000 — 열린 PR 43건 머지 판단 + +로컬 `dev`는 성능 최적화 유닛이 진행 중이라 손댈 수 없다. 그래서 이번 라운드는 +`dev`에 직접 올리지 않고 **스테이징 브랜치**에 쌓는다. 최적화 작업이 끝나면 +사용자가 이 브랜치를 `dev`로 머지하고 릴리스한다. + +- 워크트리: `/Users/jun/.codex/worktrees/260731-merge/opencodex` +- 브랜치: `codex/260731-pr-merge-round` +- 기준: `origin/dev = 356924263` (2026-07-31 재측정) +- 금지: `dev` / `main` / `preview` 푸시, npm 배포, 버전 bump + +메인 체크아웃(`/Users/jun/Developer/new/700_projects/opencodex`)에는 커밋되지 않은 +파일 22개가 있다. 이 라운드의 실패 조건 1번은 그 파일들이 스테이징되거나 사라지는 +것이다. 배치마다 `git status --porcelain`으로 확인한다. + +## 이 저장소의 머지 컨벤션 (이력에서 추출) + +세 가지 형태가 실제로 쓰이고 있다. 커밋 이력에서 직접 확인한 것만 적는다. + +| 형태 | 예시 | 언제 쓰이나 | +|---|---|---| +| GitHub 기본 머지 커밋 | `Merge pull request #773 from Wibias/fix/735-openai-chat-eof` (`5718d44e1`) | 웹 UI에서 머지할 때 | +| 손으로 쓴 머지 커밋 | `merge: PR #737 — tolerate late proxy readiness (#720)` (`62e937614`) | 메인테이너가 CLI로 머지할 때. 이슈 번호를 괄호로 단다 | +| 스쿼시 | `fix(cursor): add kimi-k3 with low/high/max effort tiers (#646)` (`275345a61`) | 기여자 PR을 한 커밋으로 접을 때 | + +스쿼시가 지배적이다. 최근 300커밋 중 머지 커밋은 25개이고 나머지 275개는 전부 +`type(scope): subject` 단일 부모 커밋이다. + +**기여자 크레딧**: 스쿼시할 때 원저자를 `--author`로 유지하거나 `Co-authored-by` +트레일러를 단다. 최근 300커밋에 23건 있다(본문 기준 22건 + 머지 커밋 1건). +`275345a61`은 기여자를 author로 두고 +메인테이너를 `Co-authored-by`에 넣었다 — 기여자 PR 위에 수정을 얹은 경우다. +`d24c5233f`는 반대로 메인테이너가 author, 기여자가 `Co-authored-by`다. +이번 라운드는 후자를 쓴다. 브랜치에서 재작성하는 건이 많기 때문이다. + +**커밋 메시지 본문**: 이 저장소는 본문을 길게 쓴다. 무엇이 왜 깨졌는지, 어떤 +측정을 했는지, 무엇이 반증됐는지까지 적는다(`1a46299b5`, `c777c8e76` 참고). +한 줄 요약만 있는 커밋은 이 저장소의 관행이 아니다. + +**CI 게이트** (`AGENTS.md` + `MAINTAINERS.md`): + +- PR 타깃은 `dev`만. `enforce-target`이 `main` 조상 위에 있으면서 `dev`보다 한참 + 뒤처진 PR과 빈약한 설명을 거부한다. +- 인증/자격증명/OAuth/Actions/릴리스 자동화/의존성 설치를 건드리면 **명시적 보안 + 리뷰**가 필요하다. +- 새 provider preset은 자격증명 목적지 변경이다. 문서화된 엔드포인트, ToS와 법인, + 중개업체면 재판매·라우팅 권한, 유지보수 담당자, 인용 가능한 검증 날짜가 있어야 + canonical 등록이 된다. 증거가 부족하면 `free-directory.ts`의 비활성 행으로. +- `bun run typecheck`, `bun run test`, `bun run privacy:scan`이 게이트다. + +## 판정 매트릭스 + +terra 서브에이전트 4개를 겹치지 않는 슬라이스로 병렬 파견해 각 PR을 **현재 HEAD +코드와 직접 대조**했다. PR 설명은 근거로 인정하지 않았고, `git cherry`와 patch-id로 +이미 HEAD에 있는지부터 확인했다. 아래 SHA는 전부 내가 직접 재확인했다. + +### 이미 dev에 들어간 것 — 닫는다 (7건) + +같은 수정이 다른 커밋으로 이미 랜딩됐다. 지금 머지하면 **더 새로운 코드를 되돌린다.** + +| PR | HEAD의 해당 커밋 | 이슈 | +|---|---|---| +| #736 Windows 서비스 상태 locale 독립 | `1d9e196e7` | #722 FULL | +| #752 tray host 소켓 상속 차단 | `c1ecbe1b5` | #733 FULL | +| #743 discovery 경로 하드닝 | `fd1933099` | #572 PARTIAL | +| #610 `codex --version` 프로브 캐싱 | `716f39cb6` | #606 FULL | +| #734 `OPENCODEX_BUN_PATH` 존중 | `9b5c864ff` + `f81e98aca` | #721 FULL | +| #777 catalog video modality | `e64a00e9f` (+ `7a041e2bc`, `299f35dc9`) | #759 PARTIAL | +| #533 npm 캐시 복구 | #557이 같은 14커밋 + 후속 2건 | — | + +#736이 대표적인 함정이다. HEAD는 `decodeSchtasksOutput()`로 schtasks의 UTF-16LE +출력을 디코딩하는데(`src/service.ts:364-393`), PR head를 머지하면 그 블록이 통째로 +삭제되고 `encoding: "utf8"`로 되돌아간다. "CLEAN하게 머지된다"가 "머지해도 된다"를 +뜻하지 않는다는 걸 보여주는 사례다. + +#734는 이전 라운드가 macOS `/var` vs `/private/var` 테스트 실패 때문에 뺐던 +건인데, `f81e98aca`가 `realpathSync`로 그 테스트를 고쳤다. 이제 8/8 통과한다. + +#533은 결함이 남아 있지만 #557이 같은 커밋 시리즈에 하드닝 2건을 더 얹은 +후속이다. 둘 다 열어둘 이유가 없다. + +### 이번 브랜치에 태울 것 (배치 A, 6건) + +HEAD에 결함이 남아 있고, 보안 경계를 넓히지 않으며, 실패하는 회귀 테스트를 +가져오는 건들이다. 전부 재기준(restack)이 필요하다 — 76커밋 이상 뒤처져 있다. + +| PR | 내용 | 이슈 | 손봐야 할 것 | +|---|---|---|---| +| #772 | `GET /api/catalog` | #709 FULL | restack만 | +| #774 | `ocx init` 파이프 stdin EOF 무한루프 | #754 FULL | restack만 | +| #783 | Claude Desktop 모델 목록에서 `native/*` 제외 | #767 FULL | restack만 | +| #768 | Kiro Windows 실행파일 PATH 해석 | — | 디렉터리/비실행 파일 거부 추가 | +| #781 | Anthropic `/v1/messages` baseUrl 중복, 스트림 quirk | #765 PARTIAL | 문자열 `tool_use.input` 정규화, 죽은 `sawContent` 제거, **`/api/logs` 봉투 테스트 헬퍼 분리** | +| #769 | Baseten free-directory 행 | — | 없음 (canonical 아님) | + +#772와 #783은 `src/codex/catalog.ts`에서 충돌한다. `readCatalog`와 +`desktopVisibleNativeSlugs` export를 둘 다 살려야 한다. + +**감사가 잡아낸 것 — #744를 배치 A에서 뺀다.** 이 PR은 OAuth 재조정을 바꾸고 +provider 설정을 영속화하며 토큰 해석 순서를 static 분기 앞뒤로 옮긴다 +(`59d95c0e4`, `39543a3c0`). `MAINTAINERS.md` 기준 명시적 보안 리뷰 대상이다. +"카탈로그를 static으로 고정한다"는 요약이 그 사실을 가렸다. 보류로 옮긴다. + +**#781도 그대로 못 태운다.** 토픽 커밋 `70031f470`이 Anthropic 코드와 함께 +`/api/logs` 테스트를 `logsFromApiBody`로 갈아끼운다. 그 헬퍼(`2f6c031cc`, +`tests/helpers/logs-api.ts`)는 배열과 `{logs}` 봉투를 **둘 다** 받아준다 — 배치 B가 +거부하기로 한 바로 그 봉투 계약을 테스트 쪽에서 미리 받아들이는 것이다. +Anthropic 변경만 떼어내고 HEAD의 배열 단언은 그대로 둔다. + +### 브랜치에서 다시 만들 것 (배치 B, 3건) + +결함은 진짜인데 구현이 지금 트리와 맞지 않는다. 기여자 커밋을 그대로 태우면 +회귀가 난다. `Co-authored-by`로 크레딧을 유지하며 재작성한다. + +- **#790 / #784 — 대시보드 로그.** 둘 다 `/api/logs`를 배열에서 + `{timeZone, logs}` 봉투로 바꾼다. 그 계약 변경이 배열을 가정한 기존 소비자를 + 깬다: `tests/server-auth.test.ts:1623`, `tests/claude-native-passthrough.test.ts:119`, + `tests/openai-provider-option-e2e.test.ts:489`, GUI mock 다수. 게다가 #790이 + 고친 유일한 테스트는 두 형태를 모두 허용해서 **패치 없이도 통과한다.** + 배열 계약을 유지하고 타임존은 응답 헤더로 나르는 쪽으로 재작성한다. + #726(200건 상한)과 #725(타임존)를 함께 닫는다. +- **#771 — Windows autostart Run 260자 초과.** VBS 런처 방향은 맞다. + `tests/windows-tray.test.ts` import 충돌만 있고 나머지는 깨끗하다. #696 FULL. +- **#780 — Windows 스케줄러 stop.** 진단이 얕다. 패치는 `schtasks /end`가 + **실패할 때만** 6.5초 기다린다. 그런데 보고된 실패는 `/end`가 성공했는데 래퍼가 + 살아남아 5초 뒤 자식을 재생성하는 경우다. 즉 이 패치로도 여전히 거짓 성공을 + 보고한다. 재시작 창을 통과할 때까지 검증하도록 다시 만든다. #764 PARTIAL. + +### 보류 (증거·리뷰 대기) + +**보안 리뷰가 필요한 건** — `MAINTAINERS.md`가 요구하는 명시적 보안 리뷰는 +메인테이너 판단이다. 내가 대신할 수 없으므로 브랜치에 태우지 않고 근거만 남긴다. + +- #782 admin token ACL opt-in. 그리고 버그가 하나 있다: 디렉터리 하드닝이 soft + continue할 수 있는데(`management-auth.ts:61-65`) 그 결과가 버려져서 + `/api/settings`가 `aclUnverified: false`를 보고할 수 있다. 파일 하드닝만 + 상태를 세운다. 이건 리뷰 전에 고쳐야 한다. +- #744 Antigravity static 고정. OAuth 설정 영속화와 토큰 해석 순서 변경. +- #750 Codex 계정 풀 plan 영속화. 자격증명·토큰 회전 경합·계정 상태 영속화. +- #746 GitHub Copilot Responses 라우팅. OAuth 갱신과 키 풀 복구 경로. +- #644 Windows tray가 활성 Codex home을 따라가게. draft이고 + `.github/workflows/pr-labeler.yml`까지 건드린다 — Actions 변경은 보안 리뷰 대상. + 게다가 `.codexclaw/goalplans/**`와 `devlog/.DS_Store`가 diff에 들어 있다. + 저장소 위생 문제라 그대로는 못 받는다. +- #779 TLS 종단 Origin scheme skew. 분석상 인증 우회는 없다 — + `requireManagementAuth`가 먼저 돌고(`index.ts:391`) 세션 경로는 여전히 + origin 완전 일치와 CSRF를 요구한다(`management-auth.ts:205`). 그래도 CORS 수용 + 범위를 넓히는 변경이라 리뷰 대상이다. +- #775 Ollama private-network discovery (SSRF/destination policy). +- #778 doctor의 provider API key 진단 (자격증명 취급). +- #693 A6API 크레딧 (Bearer 키를 새 목적지 2곳으로 보낸다). +- #616 hosted image tool (management validation 변경). + +**provider preset — 증거 미달**: #751(증거는 완비, CHANGES_REQUESTED 상태만 남음), +#747, #653, #611, #776. 각각 무엇이 빠졌는지는 `010`에 적는다. + +**자체 리뷰 사이클이 필요한 대형 건**: #757(GPT-5.6 Pro 브라우저 자동화, 40파일), +#581(zh-TW 로케일, 59파일), #715(계정 풀 선택 순서, 62파일), +#707(외부 기여자의 보안 하드닝, 88파일), #671(exact account routing), +#569(readiness 계약, draft), #557(npm 캐시 복구, draft). +머지 라운드에서 처리할 물건이 아니다. 블라스트 반경과 리뷰 표면 때문이지 +분량 때문이 아니다. + +**기타**: #745(정규화는 맞는데 회귀 테스트가 없다 — 테스트를 우리가 쓴다), +#763(코드는 괜찮은데 필수 CI 기록이 없다), #793(#773이 왜 리버트됐는지 기록이 +없다. 이유를 모른 채 같은 걸 되돌리는 건 안 된다 — 오너 판단 필요). + +## 사이클 구성 + +- `010` — 이미 랜딩된 7건 PR과 해당 이슈 정리 (머지 없음) +- `020` — 배치 A: restack 6건 +- `030` — 배치 B: 재작성 3건 +- `040` — 남은 이슈 정리와 인계 + +각 배치는 `tsc` + 대상 테스트 + 배치 종료 시 전체 스위트 + `privacy:scan`을 +통과한 뒤에만 푸시한다. 이슈는 랜딩된 코드가 실제로 결함을 없앤 게 확인될 때만 +닫고, 부분 해결은 코멘트만 남기고 열어둔다. diff --git a/devlog/_plan/260731_pr_merge_round/010_superseded_closeout.md b/devlog/_plan/260731_pr_merge_round/010_superseded_closeout.md new file mode 100644 index 000000000..c1e44dd7a --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/010_superseded_closeout.md @@ -0,0 +1,91 @@ +# 010 — 이미 랜딩된 7건 정리 + +머지가 없는 사이클이다. HEAD가 이미 같은 결함을 고쳤다는 걸 커밋 단위로 확인하고, +PR과 이슈를 근거와 함께 닫는다. + +## 확인 방법 + +`git cherry HEAD refs/prs/`으로 patch-id 동등성을 보고, 동등하지 않으면 HEAD +소스를 직접 읽어 결함이 사라졌는지 확인했다. patch-id가 다른데 결함은 없어진 +경우가 있다 — 메인테이너가 같은 문제를 다르게 고친 경우다. 이때는 PR을 머지하면 +**더 나은 구현을 되돌린다.** + +## 건별 근거 + +### #736 → #722 (FULL) + +HEAD `1d9e196e7 fix(service): make Windows status locale independent`. + +`git cherry`가 PR의 커밋 하나를 `+`(미적용)로 표시하지만 오해다. HEAD의 구현이 더 +새롭다. 확인: + +``` +git diff HEAD refs/prs/736 -- src/service.ts +``` + +PR head 방향으로 가면 `decodeSchtasksOutput()` 전체(`src/service.ts:364-393`)와 +XML 판독 실패 분기(`:607-619`)가 **삭제되고** `runFile()`이 `encoding: "utf8"`로 +돌아간다. schtasks `/query /xml`은 UTF-16LE를 뱉기 때문에 그 상태로는 모든 health +check가 실패하고 성공한 elevated create를 롤백한다. 즉 이 PR을 지금 머지하면 +#722를 다시 연다. + +### #752 → #733 (FULL) + +HEAD `c1ecbe1b5 feat(windows): add restart-safe tray controls`. +`src/tray/windows.ts:474-477`이 `stdio: "ignore"`로 띄운다. 소켓 핸들이 상속되지 +않는다. PR 커밋은 patch-id 동등(`-`). + +### #743 → #572 (PARTIAL) + +HEAD `fd1933099 fix(providers): harden discovery path and test isolation`. +`src/providers/model-discovery.ts:93-95`가 백슬래시와 `%2e` 디코딩된 `..` 세그먼트를 +거부한다. 두 커밋 모두 patch-id 동등. + +#572는 닫지 않는다. 그 이슈는 OpenAI 호환 provider 배치 승격 전체이고 이건 discovery +경로 하드닝 한 조각이다. + +### #610 → #606 (FULL) + +HEAD `716f39cb6 fix(codex): key catalog cache by runtime and address CodeRabbit follow-ups`. +`src/codex/catalog/bundled.ts:153-168`이 주입된 executor만 전달하고 +`discoverAlternatives`를 기본 `false`로 둔다. 두 커밋 patch-id 동등. + +이전 라운드 문서가 이 PR head를 NOT-ON-DEV로 기록했는데, 그 뒤에 랜딩됐다. + +### #734 → #721 (FULL) + +HEAD `9b5c864ff merge: PR #734` — 이미 머지돼 있다. 열려 있는 건 GitHub 상태가 +갱신되지 않아서다. 남아 있던 macOS 테스트 실패는 `f81e98aca`가 고쳤다: +`tests/bun-runtime.test.ts:7-9`가 `realpathSync`로 temp root를 먼저 해석한다. +이전 라운드가 이 PR을 뺀 이유가 그 실패였다. 지금 8/8 통과. + +### #777 → #759 (PARTIAL) + +HEAD `e64a00e9f`가 `src/codex/catalog/parsing.ts:274-285`에서 모든 카탈로그 엔트리의 +`input_modalities`를 정규화한다. `text`/`image`/`audio`만 통과시키고, 전부 걸러지면 +빈 배열 대신 `["text"]`로 둔다. 오염된 영속 행 복구는 +`tests/codex-catalog-sync-hardening.test.ts:161-193`이 덮는다. `7a041e2bc`와 +`299f35dc9`가 커스텀 모델 라우트 쪽 enum 경계를 추가로 막았다. + +PR의 테스트는 지금 컴파일도 안 된다 — import하는 sanitizer 헬퍼가 HEAD에 없다. +#759는 열어둔다. 이슈가 더 넓은 enum 경계 점검을 명시적으로 남겨뒀다. + +### #533 → #557로 대체 + +patch-id로 확인: #533의 update-recovery 14커밋이 #557에 전부 같은 patch-id로 +들어 있고 #557이 하드닝 2건을 더 얹었다. 다만 `refs/prs/533`이 `refs/prs/557`의 +**그래프 조상은 아니다** — 재작성된 같은 시리즈다. 둘 다 draft이고 250커밋 이상 +뒤처져 있다. #557을 후속으로 남기고 #533만 닫는다. + +#737(`62e937614`, 이미 dev에 있음)이 이 둘을 대체하지 않는다는 것도 확인했다. +#737은 `src/update/job.ts`와 그 테스트 86줄만 바꾼다. 캐시 소유권 preflight, +recovery-tree 검증, 프로세스 트리 정리는 들어 있지 않다. + +## 이슈 처리 + +닫는다: #722, #733, #606, #721. +열어둔다: #572(부분), #759(부분). + +## 이 사이클이 바꾸는 파일 + +없다. 문서만 추가한다. GitHub 상태만 정리한다. diff --git a/devlog/_plan/260731_pr_merge_round/020_batch_a_restack.md b/devlog/_plan/260731_pr_merge_round/020_batch_a_restack.md new file mode 100644 index 000000000..a7476962e --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/020_batch_a_restack.md @@ -0,0 +1,175 @@ +# 020 — 배치 A: restack 6건 + free-directory 1건 + +HEAD에 결함이 남아 있고, 기여자 구현이 지금 트리와 맞고, 패치 없이는 실패하는 +회귀 테스트를 들고 오는 건들이다. 전부 76커밋 이상 뒤처져 있어 재기준이 필요하다. + +머지 방식: 기여자 커밋을 `git cherry-pick`으로 브랜치에 올린다. 커밋이 여러 개면 +스쿼시해서 `type(scope): subject (#issue)` 한 줄로 접고, 원저자를 `--author`로 +유지한다. 우리가 수정을 얹은 건은 원저자를 author로 두고 우리를 +`Co-authored-by`에 넣는다(`275345a61` 관행). + +## 감사 반영 (2026-07-31) + +독립 리뷰어가 두 건을 잡아냈다. 둘 다 배치 구성을 바꾼다. + +1. **#744를 배치 A에서 뺀다.** OAuth 재조정·설정 영속화·토큰 해석 순서 변경이 + 들어 있어(`59d95c0e4`, `39543a3c0`) 보안 리뷰 대상이다. `040`으로 옮겼다. + 아래 4번 항목은 기록으로 남겨둔다 — import 충돌 해소 방법은 리뷰 통과 후에도 + 그대로 유효하다. +2. **#781은 커밋을 쪼갠다.** 토픽 커밋이 Anthropic 수정과 `/api/logs` 테스트 + 봉투 헬퍼를 같이 들고 있다. 아래 6번에 상세. + +## 랜딩 순서 + +충돌 표면이 겹치지 않는 것부터. `#772`와 `#783`이 `src/codex/catalog.ts` facade에서 +겹치므로 연달아 처리한다. + +1. #774 — `src/cli/init.ts` 단독 +2. #768 — `src/oauth/kiro-*.ts` 단독 +3. #772 — `src/server/management/model-routes.ts` + catalog facade +4. #783 — catalog metadata + catalog facade +5. #781 — `src/adapters/anthropic.ts` 단독 (테스트 분리 후) +6. #769 — `src/providers/free-directory.ts` 단독 + +(#744는 보류로 이동) + +## 1. #774 — `ocx init`이 파이프 stdin EOF에서 100% CPU (#754 FULL) + +지금: `src/cli/init.ts:9`의 `readline.question()`이 EOF에서 영원히 안 풀린다. +콜백이 안 불리고 이벤트 루프가 계속 돈다. + +PR: `refs/prs/774:src/cli/init.ts:9-32`가 EOF를 reject로 바꾸고 `:213-223`이 그걸 +0이 아닌 종료 코드로 매핑한다. + +테스트: `tests/init-eof.test.ts:12`. stdin을 닫고 8초 안에 exit 1을 요구하며, +stderr 문구와 **config 파일이 쓰이지 않았음**까지 확인한다. 패치 없이 실패한다 +(무한 대기 → 타임아웃). + +수정 없음. cherry-pick 후 `bun test tests/init-eof.test.ts`. + +## 2. #768 — Kiro Windows 실행파일을 PATH 없이 해석 + +지금: `src/oauth/kiro.ts:77`과 `:131`이 맨 `kiro-cli`를 spawn한다. Windows에서 +PATH에 없으면 로그인과 롤백이 둘 다 실패한다. + +PR: `refs/prs/768:src/oauth/kiro-credentials.ts:183`이 `PATH`/`Path` 두 케이싱, +`%LOCALAPPDATA%`, Program Files를 훑는다. + +**우리가 얹을 수정**: 후보를 `existsSync`만으로 받아들인다 +(`kiro-credentials.ts:183-211`). 디렉터리가 `kiro-cli`라는 이름이면 그대로 +실행 대상이 된다. `statSync().isFile()` 확인을 추가하고, Windows에서는 +`.exe`/`.cmd`/`.bat` 확장자를 요구한다. 그리고 환경변수 파싱 자체를 구동하는 +테스트가 없다 — 테스트가 `pathEntries`를 직접 주입한다 +(`tests/kiro-windows-cli-executable-path.test.ts:12-23`). `PATH` 대소문자 두 벌을 +실제로 읽는 케이스를 추가한다. + +보안: PATH 하이재킹 경계를 넓히지 않는다. 지금도 맨 이름으로 PATH를 탄다. + +## 3. #772 — `GET /api/catalog` (#709 FULL) + +지금: `src/codex/catalog/parsing.ts:186`에 리더는 있는데 라우트가 없다. 원격 +클라이언트가 생성된 카탈로그를 못 읽는다. + +PR: `refs/prs/772:src/server/management/model-routes.ts:67`. 인증된 라우트이고, +파일이 없으면 404. + +테스트: `tests/api-catalog-route.test.ts:40` — 디스크 바이트를 그대로 돌려주고 +런타임 프로브를 돌리지 않는다는 것, 그리고 404 경로. + +수정 없음. + +## 4. #783 — Claude Desktop 모델 목록에서 `native/*` 제외 (#767 FULL) + +지금: `src/server/management/shared.ts:224`가 native 행을 전부 내보낸다. + +PR: `refs/prs/783:src/codex/catalog/metadata.ts:126`에 `claudeCode.desktopNativeModels` +플래그를 두고 Desktop discovery(`src/server/index.ts:426`)와 프로필 export +(`shared.ts:215`)에 일관되게 적용한다. Grok native는 유지한다. + +테스트: `tests/claude-desktop-cli.test.ts:73`이 show/export 출력에서 `native/*`를 +찾아 실패한다. + +**충돌**: #772와 같이 `src/codex/catalog.ts` facade를 건드린다. `readCatalog`와 +`desktopVisibleNativeSlugs` export를 **둘 다** 살린다. + +## (보류) #744 — Antigravity 카탈로그 static 고정 (#723 FULL) + +**보안 리뷰 대기.** 아래 분석은 리뷰 통과 후 그대로 쓴다. + +지금: `src/providers/registry.ts:799`에 static 플래그가 없어서 Antigravity가 +지원하지 않는 `GET /models`를 프로브하고 영구히 discovery 실패를 보고한다. +토큰 해석이 static 반환보다 먼저 일어난다 +(`src/codex/catalog/provider-fetch.ts:382-384`, `:451-453`). + +**충돌 — 기계적**: `tests/provider-connection-test.test.ts:5-8` import 블록. +HEAD가 `withRegistryDiscovery` 헬퍼를 추가했고 PR이 그 줄을 지우면서 +`OAUTH_PROVIDERS`와 `PROVIDER_REGISTRY`를 넣는다. 해소: **셋 다 유지**한다. + +``` + import { saveConfig } from "../src/config"; ++import { OAUTH_PROVIDERS } from "../src/oauth"; ++import { PROVIDER_REGISTRY } from "../src/providers/registry"; + import type { OcxConfig } from "../src/types"; + import { withRegistryDiscovery } from "./helpers/provider-registry-discovery"; +``` + +테스트: 카탈로그, connection probe, 마이그레이션, sparse GUI PATCH, single-flight를 +덮는다. 그중 `"Google Antigravity uses the static-only probe without network access (#723)"`은 +fetch가 한 번이라도 불리면 던지게 해놓아서 패치 없이 확실히 실패한다. + +보안: OAuth 설정 영속화와 토큰 해석 순서를 건드린다. 자격증명 목적지가 바뀌지는 +않지만(같은 Antigravity), 커밋 메시지에 이 사실을 남긴다. + +## 5. #781 — Anthropic baseUrl 중복과 스트림 quirk (#765 PARTIAL) + +지금 네 가지가 다 살아 있다: + +- `src/adapters/anthropic.ts:695-723` — 사용자가 `/v1/messages`로 끝나는 baseUrl을 + 넣으면 경로가 두 번 붙는다 +- `:786-805` — `input_json_delta`가 `tool_use` 블록 밖에서도 먹는다 +- `:771-779` — tool ID가 비어도 그대로 내보낸다 +- `:833-848` — terminal SSE 없이 끝나는 스트림 + +**커밋을 쪼갠다.** 토픽 커밋 `70031f470`이 `src/adapters/anthropic.ts`와 +`tests/anthropic-stream-hardening.test.ts` 말고도 +`tests/claude-messages-endpoint.test.ts`, `tests/claude-native-passthrough.test.ts`를 +`logsFromApiBody`로 갈아끼우고 `tests/helpers/logs-api.ts`를 들여온다. 그 헬퍼는 +배열과 `{logs}` 봉투를 둘 다 받아준다: + +```ts +export function logsFromApiBody(body: unknown): T[] { + if (Array.isArray(body)) return body as T[]; + if (body && typeof body === "object" && Array.isArray((body as {logs?: unknown}).logs)) { + return (body as {logs: T[]}).logs; + } + return []; +} +``` + +배치 B는 봉투 계약을 거부하기로 했다. 이 헬퍼를 먼저 들이면 배열 단언이 느슨해져서 +나중에 봉투로 바뀌어도 테스트가 안 잡는다 — #790이 정확히 그 방식으로 무력한 +테스트를 만들었다. 그러니 **`src/adapters/anthropic.ts`와 +`tests/anthropic-stream-hardening.test.ts`만** 가져오고 나머지 세 파일은 HEAD 그대로 +둔다. + +**그 위에 얹을 수정 2건**: + +1. 문자열 `tool_use.input`이 그대로 JSON 문자열로 직렬화된다. 보고된 이중 인코딩 + 실패가 그대로 남는다. 파싱을 시도하고, 실패하면 `{}`로 떨어뜨린다. + 그 케이스의 테스트를 우리가 쓴다. +2. `sawContent` 지역변수가 6곳에서 대입되는데 읽히는 곳이 없다. 죽은 코드다. + 제거한다. + +#765는 부분 해결이라 열어둔다 — 이슈가 relay quirk 관용을 더 넓게 요구한다. + +## 7. #769 — Baseten free-directory 행 + +`src/providers/free-directory.ts:21-24`, `:122-124`. canonical registry가 아니라 +비활성 참조 행이다. `MAINTAINERS.md`가 증거 부족 시 권하는 바로 그 배치다. +1파일 +4/-1. 승격이 아니므로 그대로 태운다. + +## 검증 + +각 건: `bun x tsc --noEmit` + 해당 테스트 파일. +배치 종료: `bun run test` 전체 + `bun run privacy:scan` + 메인 체크아웃 +`git status --porcelain` 대조. 그 다음 푸시. diff --git a/devlog/_plan/260731_pr_merge_round/030_batch_b_rebuild.md b/devlog/_plan/260731_pr_merge_round/030_batch_b_rebuild.md new file mode 100644 index 000000000..fe2e30c37 --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/030_batch_b_rebuild.md @@ -0,0 +1,116 @@ +# 030 — 배치 B: 브랜치에서 재작성 3건 + +결함은 진짜인데 기여자 구현을 그대로 태우면 회귀가 난다. 아이디어와 크레딧은 +유지하고 구현만 다시 만든다. 원저자를 `Co-authored-by`로 남긴다. + +## 1. 대시보드 로그 — #790 + #784를 하나로 (#725 + #726) + +두 PR이 같은 계약을 서로 다르게 바꾼다. 따로 태우면 충돌하므로 한 커밋으로 묶는다. + +### 왜 그대로 못 태우나 + +둘 다 `/api/logs` 응답을 배열에서 `{timeZone, logs}` 봉투로 바꾼다 +(`src/server/management/logs-usage-routes.ts:126-129`). 그 계약을 배열로 가정한 +소비자가 최소 네 곳 있다: + +- `tests/server-auth.test.ts:1623` +- `tests/claude-native-passthrough.test.ts:119` +- `tests/openai-provider-option-e2e.test.ts:489` +- `tests/server-403-permission-e2e.test.ts:86` +- 그리고 GUI mock 다수 + +PR들은 이 소비자들을 안 고친다. 더 나쁜 건 #790이 고친 유일한 테스트가 옛 형태와 +새 형태를 **둘 다 허용**한다는 점이다. 즉 그 테스트는 패치를 되돌려도 통과한다 — +회귀 테스트 구실을 못 한다. + +### 재작성 방향 + +**배열 계약을 유지한다.** 타임존은 응답 헤더(`X-OpenCodex-Timezone`)로 나른다. +기존 소비자를 하나도 안 건드리고 GUI만 헤더를 읽는다. + +감사가 잡은 것: `jsonResponse()`는 일반 CORS 헤더만 붙인다 +(`src/server/auth-cors.ts:167-171`). 커스텀 응답 헤더는 `Access-Control-Expose-Headers` +없이는 **cross-origin에서 읽히지 않는다.** 대시보드가 same-origin으로만 뜬다는 +보장이 없으므로(포트 리매핑 터널 사례가 #570에 있다) `Access-Control-Expose-Headers: +X-OpenCodex-Timezone`을 같이 내보낸다. 그리고 cross-origin 요청에서 헤더가 실제로 +읽히는지 확인하는 테스트를 넣는다 — 안 그러면 same-origin에서만 통과하고 실사용에서 +조용히 실패한다. + +- `src/server/management/logs-usage-routes.ts` — 배열 본문 유지, 헤더 추가. + `limit`/`offset` 쿼리 파라미터 수용. +- `src/server/request-log.ts:128-129` — 200건 링 버퍼 상한을 올린다. `:741-764`에 + 페이지네이션을 추가한다. +- `gui/src/pages/Logs.tsx` — HEAD의 `useDataSurface` 리팩터 위에 얹는다. + #784는 그 이전의 수동 `fetchLogs` 흐름을 가정하고 있어 그대로는 안 맞는다. + 타임스탬프를 서버 타임존으로 포맷(`:288-293`, `:366-371`). + +### 회귀 테스트 (우리가 쓴다) + +1. 서버가 여전히 **배열**을 반환한다 — 기존 소비자가 안 깨진다는 증거. +2. 헤더에 서버 타임존이 실린다. +3. `limit`/`offset`으로 200건 너머를 읽을 수 있다. +4. GUI가 브라우저 로컬이 아니라 헤더 타임존으로 렌더한다. 브라우저 TZ를 서버와 + 다르게 고정해놓고 확인해야 실제로 구동된다. +5. cross-origin 응답에서 `Access-Control-Expose-Headers`에 + `X-OpenCodex-Timezone`이 실려 나간다. + +4번이 핵심이다. 브라우저와 서버 타임존이 같으면 이 테스트는 패치 없이도 통과한다. + +#784는 `.github/workflows/enforce-pr-target.yml:284-304`도 건드린다. 워크플로 +변경은 `AGENTS.md`상 보안 리뷰 대상이고 이 결함과 무관하므로 **가져오지 않는다.** + +닫는 이슈: #725 FULL, #726 FULL. + +## 2. #771 — Windows autostart Run 항목 260자 초과 (#696 FULL) + +지금: `src/tray/windows.ts:153-171`이 PowerShell 전체 명령을 만들고 `:500-571`이 +그걸 Run 레지스트리 값으로 쓴다. 260자를 넘으면 로그인 후 실행이 안 된다. + +PR 방향은 맞다: UTF-16LE VBS 런처를 만들어 소유권을 확인하고, 상태를 추적하고, +실패 시 롤백한다. + +**막힌 곳은 하나**: `tests/windows-tray.test.ts`가 HEAD와 import/컨텍스트에서 +충돌한다. 기능 코드는 깨끗하게 붙는다. 충돌만 풀고 기여자 커밋을 그대로 올린다. + +테스트: `tests/windows-tray-run-limit.test.ts`가 패치 없이 실패한다. +`tests/windows-tray.test.ts`가 유니코드 경로, UNC 경로, 소유권 경로를 덮는다. + +실행 검증은 macOS에서 불가능하다. 이 건은 Windows 매트릭스 CI가 판정한다. +커밋에 그 한계를 명시한다. + +## 3. #780 — Windows 스케줄러 stop이 실제로 멈추지 않는다 (#764 PARTIAL) + +지금: + +- `src/service.ts:1629-1640` — pid 없이 조기 반환 +- `src/server/proxy-liveness.ts:128-151` — health가 보고한 PID를 그대로 믿는다 +- `src/service.ts:1948-1964` — `ops.stop()` 직후 성공을 보고한다 + +### PR의 진단이 얕다 + +패치는 `schtasks /end`가 **에러를 낼 때만** 6.5초 기다린다. 그런데 보고된 실패 +모드는 `/end`가 **성공했는데** 래퍼 프로세스가 살아남아 5초 뒤 자식을 재생성하는 +경우다. 그 경로에서는 이 패치도 여전히 거짓 성공을 보고한다. + +### 재작성 방향 + +`/end` 결과와 무관하게 **항상** 검증한다: + +1. `/end` 호출. +2. 래퍼/태스크가 재시작 창(관측된 5초 + 여유)을 지나 실제로 죽었는지 확인. +3. 살아 있는 프록시가 없는지 확인한 **뒤에** native Codex 복구를 진행. +4. PID는 health 보고를 믿지 말고 프로세스 존재로 교차 확인. + +### 회귀 테스트 + +`/end`가 성공을 반환하지만 래퍼가 살아남아 자식을 재생성하는 시나리오. 지금 +구현과 PR 구현 **양쪽 다** 실패해야 한다. 그게 이 재작성이 새 문제를 푼다는 +증거다. + +보안: 파괴적 PID 제어와 서비스 자격증명을 건드린다. 커밋에 명시한다. +#764는 PARTIAL로 열어둔다 — `--native` 스위치 문제는 별개다. + +## 검증 + +배치 A와 동일. 각 건 `tsc` + 대상 테스트, 배치 종료 시 전체 스위트 + +`privacy:scan` + 메인 체크아웃 대조, 그 다음 푸시. diff --git a/devlog/_plan/260731_pr_merge_round/040_deferred_and_handoff.md b/devlog/_plan/260731_pr_merge_round/040_deferred_and_handoff.md new file mode 100644 index 000000000..4decfbf69 --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/040_deferred_and_handoff.md @@ -0,0 +1,167 @@ +# 040 — 보류 건과 인계 + +이 라운드에서 태우지 않는 건들. 왜 안 태우는지와 무엇이 있어야 풀리는지를 남긴다. +"나중에 보자"로 끝내면 다음 사람이 같은 조사를 다시 한다. + +## 보안 리뷰가 필요한 6건 + +`MAINTAINERS.md`는 인증·자격증명·OAuth·Actions·릴리스 자동화·의존성 설치를 +건드리는 변경에 **명시적 보안 리뷰**를 요구한다. 리뷰는 메인테이너 판단이지 +에이전트가 대신할 수 있는 게 아니다. 그래서 분석만 남기고 브랜치에 안 올린다. + +### #782 — Windows admin token ACL opt-in (#766 부분) + +`OPENCODEX_ALLOW_UNVERIFIED_ADMIN_TOKEN_ACL`을 켜면 타임아웃으로 인한 NTFS ACL +검증 실패를 통과시킨다(`management-auth.ts:56-89`). 지금 HEAD는 모든 ACL 실패에 +fail-close한다. + +위협 모델: opt-in을 켜야만 열린다. 권한 없는 로컬 프로세스가 특권 Windows 서비스의 +환경변수를 바꿀 수는 없다. 다만 다중 사용자 호스트에서 디렉터리/파일 ACL이 실제로 +약한 상태로 이걸 켜면, 다른 로컬 사용자가 토큰 파일을 읽거나 재시작 전에 바꿔치기해 +관리 API 전체를 얻는다. 네트워크 공격자는 여전히 토큰이 필요하다. + +**리뷰 전에 고쳐야 할 버그**: 디렉터리 하드닝이 soft continue할 수 있는데 +(`management-auth.ts:61-65`) 그 결과가 버려진다. `aclUnverified`를 세우는 건 파일 +하드닝뿐이라(`:77-89`), 디렉터리 경계가 미검증인데 +`/api/settings`가 `false`를 보고할 수 있다(`config-routes.ts:119`). +`assertSafeDirectory`가 상태를 반환하게 하고 파일 쪽 상태와 OR로 합쳐야 한다. +지금 테스트는 파일 타임아웃만 덮는다(`tests/server-management-auth.test.ts:445`). + +### #779 — TLS 종단 Origin scheme skew (#760) + +`https` Origin과 프로세스가 관측한 `http`의 차이를, 호스트가 같고 포트가 같거나 +`443→80`일 때만 허용한다(`auth-cors.ts:92-144`). + +분석상 인증 우회는 없다. `requireManagementAuth`가 먼저 돌고(`index.ts:391`) +토큰 경로는 constant-time 비교 뒤에만 통과한다(`management-auth.ts:195`). +세션 경로는 여전히 request/claimed/browser origin 완전 일치와 CSRF를 요구한다 +(`:205`). 넓히는 건 문서화된 TLS 종단 형태의 CORS 수용이지 미인증 수용이 아니다. + +그래도 CORS 수용 범위 변경이라 리뷰 대상이다. 준비된 상태로 대기. + +### #775 — Ollama private-network discovery (#758) + +registry/private-network 권한을 중앙화해 discovery 전송에 전달한다. 지금은 +config 검증에서만 존중되고(`destination-policy.ts:133-140`) 아웃바운드 discovery는 +`provider.allowPrivateNetwork`만 본다(`provider-outbound.ts:104-137`). + +SSRF/destination policy 작업이다. 리뷰와 함께, built-in 기본값이 여전히 +metadata/link-local/unspecified 목적지를 거부한다는 전송 계층 회귀 테스트를 요구한다. + +### #778 — doctor의 provider API key 진단 (#762 부분) + +provider와 환경변수 **이름만** 표시한다. 값은 안 찍는다. 그래도 자격증명 취급이라 +리뷰 대상이다. 그리고 현재 프로세스만 검사하고 실행 중인 프록시의 해석 결과는 +확인하지 않는데, 그 사실이 출력에 안 드러난다. "이 프로세스 기준, 서비스 프로세스는 +미검증"을 명시하게 고쳐야 #762가 온전히 닫힌다. + +### #693 — A6API 크레딧 사용량 + +Bearer 키를 새 목적지 2곳으로 보낸다. 자격증명 목적지 변경이므로 하드 게이트다. +테스트는 좋다 — canonical 호스트에만 키를 보내는지, `redirect: "error"`, +깨진 페이로드, 캐시 키 회전, stale 행 억제까지 216줄로 덮는다. + +### #616 — hosted image tool 설정 보존 + +`src/server/auth-cors.ts:289-357`의 management validation과 요청 도구 디스패치를 +바꾼다. 819줄. 깨끗하게 붙지만 리뷰 대상이다. + +### #744 — Antigravity 카탈로그 static 고정 (#723) + +원래 배치 A에 넣었다가 감사에서 되돌렸다. "카탈로그를 static으로 고정"이라는 +요약이 실제 변경 범위를 가렸다. 커밋 `59d95c0e4`와 `39543a3c0`이 OAuth 재조정, +provider 설정 영속화, 토큰 해석 순서(static 분기 앞뒤)를 바꾼다. + +결함은 진짜다: `src/providers/registry.ts:799`에 static 플래그가 없어서 지원하지 +않는 `GET /models`를 프로브하고 영구 discovery 실패를 보고한다. 테스트도 좋다. +리뷰만 통과하면 다음 라운드 1순위다. import 충돌 해소 방법은 `020`에 남겨뒀다. + +### #750 — Codex 계정 풀 plan 영속화 (#724) + +HEAD는 `freshPlan`을 읽고도(`src/codex/auth-api.ts:456-470`) 저장된 계정 그대로 +DTO를 만든다(`:541-550`). 결함은 남아 있다. 테스트가 특히 좋다 — 동시 refresh의 +토큰 회전 공유, 삭제 후 재생성된 계정, 교체 자격증명 격리까지 덮는다. +자격증명·토큰 생성 경합·계정 상태 영속화를 건드리므로 하드 게이트. draft 상태. + +### #746 — GitHub Copilot Responses 라우팅 (#748) + +모델별 wire default 맵을 들이고 Responses 전용 모델 계열을 `openai-responses`로 +해석한다. OAuth 갱신과 키 풀 복구 경로를 바꾼다. 이슈 #748은 이미 닫혔지만 +수정은 HEAD에 없다. 리베이스 + 보안 리뷰 + 신규 CI. + +### #644 — Windows tray가 활성 Codex home을 따라가게 + +draft. 두 가지가 걸린다. 첫째, `.github/workflows/pr-labeler.yml`과 +`.github/scripts/pr-labeler.test.cjs`를 바꾼다 — Actions 변경은 보안 리뷰 대상이다. +둘째, diff에 `.codexclaw/goalplans/**`(에이전트 상태 파일)와 `devlog/.DS_Store`, +`devlog/_plan/.DS_Store`가 들어 있다. 저장소 위생 문제라 그대로는 못 받는다. +기여자에게 그 파일들을 빼달라고 요청해야 한다. + +## provider preset — 증거 미달 5건 + +`MAINTAINERS.md`의 canonical preset 증거 요건: 문서화된 OpenAI 호환 엔드포인트 +(`liveModels`를 선언하면 인증된 `GET /v1/models` 포함), ToS와 운영 법인, +중개업체면 재판매·라우팅 권한, 지명된 유지보수 담당자, 인용 가능한 검증 날짜. + +| PR | 위치 | 빠진 것 | +|---|---|---| +| #751 Hyperbolic | canonical, `liveModels: true` | **없음.** 증거표가 완비돼 있다. 남은 건 그 표보다 앞선 CHANGES_REQUESTED 리뷰를 메인테이너가 갱신하는 것뿐 | +| #747 DeepInfra + Novita | canonical ×2, `liveModels` | ToS/법인, 두 중개업체의 라우팅 권한, 담당자, 검증 날짜 | +| #653 Baseten Model APIs | canonical, `liveModels` | 엔드포인트·인증 모델목록의 날짜 있는 1차 출처, ToS/법인, 담당자, 보안 승인 | +| #611 Volcengine Ark | canonical ×3, `liveModels: false` | 1차 엔드포인트 문서, ToS/법인, 번들된 서드파티 모델의 라우팅 권한, 담당자, 검증 날짜 | +| #776 Alibaba China Coding Plan | 기존 provider에 baseUrl 선택지 추가 | 새 자격증명 목적지다. 1차 출처와 보안 리뷰 필요. registry의 Alibaba 데이터에는 아직 "docs unverified" 표시가 있다(`registry.ts:969-972`) | + +#751은 사실상 통과 상태다. 오너가 리뷰만 갱신하면 다음 라운드에서 태울 수 있다. + +## 자체 사이클이 필요한 대형 건 7건 + +분량이 아니라 블라스트 반경과 리뷰 표면 때문이다. + +- **#757** GPT-5.6 Pro 라우팅, 40파일 +1553. 브라우저 자동화 provider를 새로 + 들인다. 로그인된 세션 사용, 프로세스 제어, 도구 스키마, 과금 의미까지 걸린다. +- **#581** zh-TW 로케일, 59파일 +5813. i18n 키 패리티가 엄격한 표면이고 + docs 라우팅에 `zh-tw`가 전역 추가된다. CHANGES_REQUESTED 상태. +- **#715** 계정 풀 선택 순서, 62파일 47커밋. pin/preemption 상태와 HEAD의 GUI + hydration/polling 제어를 **하나의 동작으로** 화해시켜야 한다. hunk 단위로 + 고를 수 있는 충돌이 아니다. +- **#707** 외부 기여자의 서비스/관리 경계 하드닝, 88파일 +7928. 보안 경계 작업이다. + 랜딩 전에: 리베이스, 명시적 위협 모델, 작성자가 아닌 메인테이너의 보안 리뷰, + ACL/토큰 초기화 실패가 관리 접근을 열지 않는다는 **부정 테스트**, 그리고 신규 CI. +- **#671** exact account routing, 21파일. 풀 밸런싱을 의도적으로 우회하고 + cooldown 계정에 fail-close한다(`auth-context.ts:202-278`). 자격증명-모델 라우팅 + 변경이다. +- **#569** readiness 계약(`/readyz`, `ocx ready --wait`), draft. +- **#557** npm 캐시 복구, draft, 256커밋 뒤. 의존성 설치/프로세스/경로 신뢰 경계 + 재작성이다. #737이 이걸 대체하지 않는다 — #737은 `src/update/job.ts` 86줄뿐이다. + +## 개별 판단 + +- **#745** — tool schema 루트에 `type: "object"`를 채우는 정규화. 방향은 맞고 + 결함도 남아 있다(`src/responses/parser.ts:137-151`). 그런데 **회귀 테스트가 + 없다.** 기존 테스트는 `{}`를 쓰지만 정규화된 출력을 검증하지 않는다. + 다음 라운드에서 우리가 테스트를 쓰고 태운다. +- **#763** — picker 라벨에 native 모델 이름 우선. 코드도 테스트도 괜찮다 + (`tests/codex-catalog.test.ts`에 collision 케이스가 있다). 필수 CI 기록이 없다. + restack + CI면 통과. +- **#793** — #773을 되살린다. #773은 머지됐다가 #792로 리버트됐는데 **왜 + 리버트됐는지 기록이 없다.** #792 본문은 "Reverts #773" 한 줄이고 논의도 없다. + 이유를 모르는 채로 같은 걸 되돌리는 건 안 된다. 오너 판단이 필요하다. + 별개로 실제 결함도 있다: pending tool call을 스냅샷한 뒤 truncation 에러를 + 반환하기 **전에** `tool_call_end`를 내보낸다(`refs/prs/793:src/adapters/openai-chat.ts:807-821`). + 잘린 tool call에 완료 이벤트가 안 나가는지 확인하는 테스트가 없다. + +## 열린 PR 전체 대조 + +감사가 세 건 누락을 잡았다(#750, #746, #644). 지금은 `gh pr list`가 반환하는 +43건 전부에 처분이 달려 있다. 다음 라운드에서 이 문서를 이어받을 때는 +`gh pr list --state open`을 다시 돌려 새로 열린 PR부터 확인할 것. + +## 이슈 상태 + +랜딩 전까지 열어두는 것: #572, #759, #765, #764, #766, #758, #762, #756, +#748, #724, #719, #690, #658, #657, #656, #650, #586, #570, #561, #553, +#545, #540, #425, #418, #417, #415, #414, #386, #241, #201, #178, #177, +#95, #92. + +upstream-tracking(#417, #241, #92, #418)은 오래됐다고 닫지 않는다. 추적 목적이 +살아 있고 업스트림이 미해결이다. From bfa1a599c4eff6598a0ac963e9172a855a2c6efc Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:46:45 +0900 Subject: [PATCH 02/24] fix(cli): exit ocx init cleanly on piped stdin EOF (#754) `ocx init` under a pipe span at ~100% CPU forever once stdin reached EOF. `readline.question()` never invokes its callback when the stream closes, so the promise stayed pending, the interface stayed open, and the event loop kept spinning with nothing left to read. The prompt now rejects on EOF and the CLI maps that to a clean non-zero exit instead of a hang. The regression test closes stdin, requires the process to exit 1 within 8 seconds, and asserts no config file was written -- so a silent regression to the hanging behavior fails on the timeout rather than passing quietly. Co-authored-by: bitkyc08-arch --- src/cli/init.ts | 231 +++++++++++++++++++++++------------------ tests/init-eof.test.ts | 33 ++++++ 2 files changed, 162 insertions(+), 102 deletions(-) create mode 100644 tests/init-eof.test.ts diff --git a/src/cli/init.ts b/src/cli/init.ts index 0908e78d4..c6ac382d1 100644 --- a/src/cli/init.ts +++ b/src/cli/init.ts @@ -8,11 +8,28 @@ import type { OcxConfig, OcxProviderConfig } from "../types"; function createPrompt(): { ask(question: string): Promise; close(): void } { const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); + let closed = false; + rl.on("close", () => { closed = true; }); return { ask(question: string): Promise { - return new Promise(resolve => rl.question(question, resolve)); + return new Promise((resolve, reject) => { + if (closed) { + reject(new Error("stdin closed before the prompt could be answered")); + return; + } + const onClose = () => { + reject(new Error("stdin reached EOF while waiting for input")); + }; + rl.once("close", onClose); + rl.question(question, answer => { + rl.off("close", onClose); + resolve(answer); + }); + }); + }, + close() { + if (!closed) rl.close(); }, - close() { rl.close(); }, }; } @@ -83,115 +100,125 @@ export function cleanupOpenAiTierBackupAfterInit(configPath = getConfigPath()): export async function runInit(): Promise { const prompt = createPrompt(); - console.log("\n🔧 opencodex (ocx) setup\n"); - - const providers = buildInitProviders(); - printMenu(providers); - - const choice = await prompt.ask("\nSelect default provider (number): "); - const idx = parseInt(choice, 10) - 1; - - let providerName: string; - let providerConfig: OcxProviderConfig; - let oauthHint = false; - - if (idx >= 0 && idx < providers.length) { - const p = providers[idx]; - providerName = p.id; - console.log(`\n📡 ${p.label}`); - console.log(` Base URL: ${p.baseUrl}`); - - if (p.kind === "forward") { - providerConfig = { adapter: p.adapter, baseUrl: p.baseUrl, authMode: "forward" }; - console.log(" No API key needed — forwards your existing `codex login`."); - } else if (p.kind === "oauth") { - providerConfig = { adapter: p.adapter, baseUrl: p.baseUrl, authMode: "oauth", ...(p.defaultModel ? { defaultModel: p.defaultModel } : {}) }; - oauthHint = true; - } else { - // key + local: collect a key (local usually blank). - if (p.dashboardUrl) console.log(` 🔑 Get your key: ${p.dashboardUrl}`); - // Template URL with placeholders (e.g. Cloudflare's {account_id}) needs a resolved value. - let baseUrl = p.baseUrl; - if (/\{[^}]*\}/.test(baseUrl)) { - const resolved = (await prompt.ask(` Your endpoint URL (${baseUrl}): `)).trim(); - if (!resolved) { - console.error(" A resolved URL is required — replace the {placeholder} with your actual value."); - process.exit(1); + try { + console.log("\n🔧 opencodex (ocx) setup\n"); + + const providers = buildInitProviders(); + printMenu(providers); + + const choice = await prompt.ask("\nSelect default provider (number): "); + const idx = parseInt(choice, 10) - 1; + + let providerName: string; + let providerConfig: OcxProviderConfig; + let oauthHint = false; + + if (idx >= 0 && idx < providers.length) { + const p = providers[idx]; + providerName = p.id; + console.log(`\n📡 ${p.label}`); + console.log(` Base URL: ${p.baseUrl}`); + + if (p.kind === "forward") { + providerConfig = { adapter: p.adapter, baseUrl: p.baseUrl, authMode: "forward" }; + console.log(" No API key needed — forwards your existing `codex login`."); + } else if (p.kind === "oauth") { + providerConfig = { adapter: p.adapter, baseUrl: p.baseUrl, authMode: "oauth", ...(p.defaultModel ? { defaultModel: p.defaultModel } : {}) }; + oauthHint = true; + } else { + // key + local: collect a key (local usually blank). + if (p.dashboardUrl) console.log(` 🔑 Get your key: ${p.dashboardUrl}`); + // Template URL with placeholders (e.g. Cloudflare's {account_id}) needs a resolved value. + let baseUrl = p.baseUrl; + if (/\{[^}]*\}/.test(baseUrl)) { + const resolved = (await prompt.ask(` Your endpoint URL (${baseUrl}): `)).trim(); + if (!resolved) { + console.error(" A resolved URL is required — replace the {placeholder} with your actual value."); + process.exit(1); + } + baseUrl = resolved; } - baseUrl = resolved; + const env = envKeyFor(p.id); + const hint = p.kind === "local" ? "API key (usually blank — press Enter): " : `API key (paste, or env var $${env}): `; + const apiKey = (await prompt.ask(`\n${hint}`)).trim(); + const modelChoice = (await prompt.ask(`Default model${p.defaultModel ? ` [${p.defaultModel}]` : " (optional)"}: `)).trim(); + const defaultModel = modelChoice || p.defaultModel; + providerConfig = { + adapter: p.adapter, + baseUrl, + ...(p.kind === "key" ? { apiKey: apiKey || `\${${env}}` } : apiKey ? { apiKey } : {}), + ...(defaultModel ? { defaultModel } : {}), + }; + // Apply the catalog's models / vision classification (same enrichment as the GUI). + enrichProviderFromCatalog(p.id, providerConfig); + } + } else { + providerName = (await prompt.ask("Provider name: ")).trim(); + if (!isValidProviderName(providerName)) { + console.error("Provider name must use letters, numbers, dot, underscore, or hyphen and cannot be a reserved object key."); + process.exit(1); } - const env = envKeyFor(p.id); - const hint = p.kind === "local" ? "API key (usually blank — press Enter): " : `API key (paste, or env var $${env}): `; - const apiKey = (await prompt.ask(`\n${hint}`)).trim(); - const modelChoice = (await prompt.ask(`Default model${p.defaultModel ? ` [${p.defaultModel}]` : " (optional)"}: `)).trim(); - const defaultModel = modelChoice || p.defaultModel; + const baseUrl = await prompt.ask("Base URL (e.g. http://localhost:11434/v1): "); + const adapter = await prompt.ask("Adapter [openai-chat]: ") || "openai-chat"; + const apiKey = await prompt.ask("API key (optional): "); + const defaultModel = await prompt.ask("Default model: "); providerConfig = { - adapter: p.adapter, - baseUrl, - ...(p.kind === "key" ? { apiKey: apiKey || `\${${env}}` } : apiKey ? { apiKey } : {}), - ...(defaultModel ? { defaultModel } : {}), + adapter: adapter.trim(), + baseUrl: baseUrl.trim(), + ...(apiKey.trim() ? { apiKey: apiKey.trim() } : {}), + ...(defaultModel.trim() ? { defaultModel: defaultModel.trim() } : {}), }; - // Apply the catalog's models / vision classification (same enrichment as the GUI). - enrichProviderFromCatalog(p.id, providerConfig); - } - } else { - providerName = (await prompt.ask("Provider name: ")).trim(); - if (!isValidProviderName(providerName)) { - console.error("Provider name must use letters, numbers, dot, underscore, or hyphen and cannot be a reserved object key."); - prompt.close(); - process.exit(1); } - const baseUrl = await prompt.ask("Base URL (e.g. http://localhost:11434/v1): "); - const adapter = await prompt.ask("Adapter [openai-chat]: ") || "openai-chat"; - const apiKey = await prompt.ask("API key (optional): "); - const defaultModel = await prompt.ask("Default model: "); - providerConfig = { - adapter: adapter.trim(), - baseUrl: baseUrl.trim(), - ...(apiKey.trim() ? { apiKey: apiKey.trim() } : {}), - ...(defaultModel.trim() ? { defaultModel: defaultModel.trim() } : {}), - }; - } - const portStr = await prompt.ask("\nProxy port [10100]: "); - const port = parseInt(portStr, 10) || 10100; + const portStr = await prompt.ask("\nProxy port [10100]: "); + const port = parseInt(portStr, 10) || 10100; - const config: OcxConfig = { - ...getDefaultConfig(), - port, - providers: { [providerName]: providerConfig }, - defaultProvider: providerName, - }; + const config: OcxConfig = { + ...getDefaultConfig(), + port, + providers: { [providerName]: providerConfig }, + defaultProvider: providerName, + }; - saveConfig(config); - // Init writes a fresh config, so a stale pre-migration backup from a previous - // installation would make the next `ocx start` crash on a stale-backup - // collision (issue #257). But only a STALE backup (unparseable, or already a - // post-migration v2 snapshot) may be deleted; a backup that still parses as a - // valid pre-migration (v1) config is a user-intentional rollback point and is - // preserved by renaming it out of the collision path (sol review 260722). - cleanupOpenAiTierBackupAfterInit(); - console.log(`\n✅ Config saved to ~/.opencodex/config.json`); - if (oauthHint) console.log(`🔐 Authenticate this provider with: ocx login ${providerName}`); - - const injectAnswer = await prompt.ask("Inject into Codex config.toml? [Y/n]: "); - if (injectAnswer.trim().toLowerCase() !== "n") { - console.log("Fetching available models from provider..."); - const result = await injectCodexConfig(port, config); - console.log(result.success ? `✅ ${result.message}` : `⚠️ ${result.message}`); - } + saveConfig(config); + // Init writes a fresh config, so a stale pre-migration backup from a previous + // installation would make the next `ocx start` crash on a stale-backup + // collision (issue #257). But only a STALE backup (unparseable, or already a + // post-migration v2 snapshot) may be deleted; a backup that still parses as a + // valid pre-migration (v1) config is a user-intentional rollback point and is + // preserved by renaming it out of the collision path (sol review 260722). + cleanupOpenAiTierBackupAfterInit(); + console.log(`\n✅ Config saved to ~/.opencodex/config.json`); + if (oauthHint) console.log(`🔐 Authenticate this provider with: ocx login ${providerName}`); + + const injectAnswer = await prompt.ask("Inject into Codex config.toml? [Y/n]: "); + if (injectAnswer.trim().toLowerCase() !== "n") { + console.log("Fetching available models from provider..."); + const result = await injectCodexConfig(port, config); + console.log(result.success ? `✅ ${result.message}` : `⚠️ ${result.message}`); + } - const shimAnswer = await prompt.ask("Install Codex autostart shim? [Y/n]: "); - if (shimAnswer.trim().toLowerCase() !== "n") { - try { - const { installCodexShim } = await import("../codex/shim"); - const result = installCodexShim(); - console.log(result.installed ? `✅ ${result.message}` : `⚠️ ${result.message}`); - } catch (err) { - console.log(`⚠️ Codex autostart shim skipped: ${err instanceof Error ? err.message : String(err)}`); + const shimAnswer = await prompt.ask("Install Codex autostart shim? [Y/n]: "); + if (shimAnswer.trim().toLowerCase() !== "n") { + try { + const { installCodexShim } = await import("../codex/shim"); + const result = installCodexShim(); + console.log(result.installed ? `✅ ${result.message}` : `⚠️ ${result.message}`); + } catch (err) { + console.log(`⚠️ Codex autostart shim skipped: ${err instanceof Error ? err.message : String(err)}`); + } } - } - console.log(`\n🚀 Setup complete! Run 'ocx start' to start the proxy.`); - prompt.close(); + console.log(`\n🚀 Setup complete! Run 'ocx start' to start the proxy.`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (/stdin (closed|reached EOF)/i.test(message)) { + console.error(`\n❌ ${message}. Re-run \`ocx init\` in an interactive terminal.`); + process.exitCode = 1; + return; + } + throw error; + } finally { + prompt.close(); + } } diff --git a/tests/init-eof.test.ts b/tests/init-eof.test.ts new file mode 100644 index 000000000..297abe467 --- /dev/null +++ b/tests/init-eof.test.ts @@ -0,0 +1,33 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +describe("ocx init piped stdin (#754)", () => { + const dirs: string[] = []; + afterEach(() => { + while (dirs.length) rmSync(dirs.pop()!, { recursive: true, force: true }); + }); + + test("exits cleanly when stdin closes before the first prompt answer", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-init-eof-")); + dirs.push(home); + const cli = join(import.meta.dir, "..", "src", "cli", "index.ts"); + const proc = Bun.spawn({ + cmd: [process.execPath, cli, "init"], + env: { ...process.env, OPENCODEX_HOME: home }, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + proc.stdin.end(); + const exit = await Promise.race([ + proc.exited, + new Promise((_, reject) => setTimeout(() => reject(new Error("init did not exit after stdin EOF")), 8_000)), + ]); + expect(exit).toBe(1); + const stderr = await new Response(proc.stderr).text(); + expect(stderr.toLowerCase()).toMatch(/stdin (closed|reached eof)/); + expect(existsSync(join(home, "config.json"))).toBe(false); + }); +}); From 5d7ab0cb77e1edac7359c25433f62066517f65dd Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:47:03 +0900 Subject: [PATCH 03/24] feat(api): serve the generated Codex catalog at GET /api/catalog (#709) Remote clients had no way to read the catalog OpenCodex generates; the reader existed (`src/codex/catalog/parsing.ts`) but nothing exposed it over the management API, so anything not sharing the filesystem had to guess at the model set. The route returns the on-disk bytes and 404s when the file is absent. It deliberately does not trigger a runtime probe: `/api/catalog` is a read of what was generated, not a reason to spawn `codex --version` on the request path, which is the failure mode #606 just closed. Both cases are pinned -- exact bytes with no probe, and the missing-file 404. Co-authored-by: bitkyc08-arch --- .../content/docs/guides/codex-integration.md | 16 ++++ src/codex/catalog.ts | 2 +- src/server/management/model-routes.ts | 16 +++- tests/api-catalog-route.test.ts | 77 +++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/api-catalog-route.test.ts diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index bef498ce4..7406320ad 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -201,6 +201,22 @@ Add a display name from the CLI (the proxy syncs the catalog right away when liv ocx models add deepseek deepseek-v4 --display-name "DeepSeek V4" --context-window 128000 ``` +Remote Codex clients can fetch the same generated catalog over the management API (same +admission token as other `/api/*` routes): + +```bash +dest="${CODEX_HOME:-$HOME/.codex}/opencodex-catalog.json" +tmp="$(mktemp "${dest}.XXXXXX")" +curl -fsS -H "x-opencodex-api-key: $OPENCODEX_ADMIN_AUTH_TOKEN" \ + "https://proxy.example.com/api/catalog" > "$tmp" \ + && mv "$tmp" "$dest" +ocx sync-cache +``` + +The response is the raw `opencodex-catalog.json` document (no provider credentials). When +available, the `x-opencodex-codex-version` header reports the Codex runtime version on the +server so clients can spot version skew. + You can also set or edit it through the management API (`POST /api/custom-models`, `PUT /api/custom-models/` with a `displayName` string) and the web dashboard. A `/` is rejected because it would collide with the routed-slug separator. diff --git a/src/codex/catalog.ts b/src/codex/catalog.ts index 03173400b..257e3a799 100644 --- a/src/codex/catalog.ts +++ b/src/codex/catalog.ts @@ -1,6 +1,6 @@ // AUTO-SPLIT facade: original catalog.ts body moved into ./catalog/* modules. // Public surface preserved exactly; importers keep using "src/codex/catalog". -export { isMediaGenerationModelId, shouldExposeRoutedModel, readCodexCatalogPath, normalizeRoutedCatalogEntry, catalogModelSlug, filterSupportedNativeSlugs, catalogModelSupportsReasoningSummaries } from "./catalog/parsing"; +export { isMediaGenerationModelId, shouldExposeRoutedModel, readCodexCatalogPath, readCatalog, normalizeRoutedCatalogEntry, catalogModelSlug, filterSupportedNativeSlugs, catalogModelSupportsReasoningSummaries } from "./catalog/parsing"; export type { CatalogModel, MultiAgentMode } from "./catalog/parsing"; export { NATIVE_OPENAI_MODELS, nativeOpenAiContextWindow, disabledNativeSlugs, visibleNativeSlugs, nativeModelRows, applyNativeVisibility, upstreamNativeEntry, nativeOpenAiSlugs, listCatalogNativeSlugs } from "./catalog/metadata"; export { isSpawnableCodexCandidate, codexExecInvocation, loadBundledCodexCatalog, materializeBundledCodexCatalog, loadCatalogTemplate } from "./catalog/bundled"; diff --git a/src/server/management/model-routes.ts b/src/server/management/model-routes.ts index 8461fe823..ff7e892f4 100644 --- a/src/server/management/model-routes.ts +++ b/src/server/management/model-routes.ts @@ -83,7 +83,7 @@ import { drainAndShutdown } from "../lifecycle"; import { filterRequestLogs, getRequestLogEntries, type RequestLogEntry } from "../request-log"; import { estimateComboCost, estimateRequestCost, normalizeCostTokens, tokensPerSecond } from "../../usage/cost"; import type { PersistedUsageAttempt } from "../../usage/log"; -import { isAllowedRequestOrigin, jsonResponse, providerManagementConfigError, publicProviderBaseUrl, safeConfigDTO } from "../auth-cors"; +import { isAllowedRequestOrigin, jsonResponse, providerManagementConfigError, publicProviderBaseUrl, safeConfigDTO, corsHeaders } from "../auth-cors"; import { applySystemEnvToggle } from "../system-env"; import { isPlainRecord, parseDebugLogQuery, tokPerSecondResult, unavailableCostReason, costResult, requestLogDto, stripRegistryOnlyStaticHeaders, fetchAllModels } from "./shared"; @@ -98,6 +98,20 @@ export async function handleModelRoutes(ctx: ManagementContext): Promise = { + "Content-Type": "application/json", + ...corsHeaders(req, config), + }; + const { loadPersistedCodexRuntime } = await import("../../codex/runtime"); + const version = loadPersistedCodexRuntime()?.selectedVersion; + if (version) headers["x-opencodex-codex-version"] = version; + return new Response(JSON.stringify(catalog), { status: 200, headers }); + } + if (url.pathname === "/api/models" && req.method === "GET") { const models = await fetchAllModels(config); const disabled = new Set(config.disabledModels ?? []); diff --git a/tests/api-catalog-route.test.ts b/tests/api-catalog-route.test.ts new file mode 100644 index 000000000..f70c0c651 --- /dev/null +++ b/tests/api-catalog-route.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { handleManagementAPI } from "../src/server/management-api"; +import { loadConfig, saveConfig } from "../src/config"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { ManagementRequest, managementHeaders } from "./helpers/management-auth"; +import type { OcxConfig } from "../src/types"; + +const TEST_DIR = join(import.meta.dir, `.tmp-api-catalog-route-${process.pid}`); +const previousOpencodexHome = process.env.OPENCODEX_HOME; +let isolatedCodexHome: IsolatedCodexHome | null = null; + +beforeEach(() => { + if (previousOpencodexHome === undefined) mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + isolatedCodexHome = null; + saveConfig({ + port: 10100, + hostname: "127.0.0.1", + defaultProvider: "mock", + providers: { + mock: { adapter: "openai-chat", baseUrl: "http://127.0.0.1:1/v1", apiKey: "k", allowPrivateNetwork: true, models: ["test-model"] }, + }, + } as OcxConfig); +}); + +afterEach(() => { + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (previousOpencodexHome === undefined) { + delete process.env.OPENCODEX_HOME; + rmSync(TEST_DIR, { recursive: true, force: true }); + } else { + process.env.OPENCODEX_HOME = previousOpencodexHome; + } +}); + +describe("GET /api/catalog route (#709)", () => { + test("returns the on-disk catalog and omits sync runtime probes for version hint", async () => { + isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-"); + const catalog = { + models: [{ + slug: "mock/test-model", + display_name: "Mock Test", + description: "fixture", + priority: 1, + visibility: "list", + base_instructions: "You are a helpful coding assistant.", + input_modalities: ["text"], + }], + }; + writeFileSync(join(isolatedCodexHome.path, "opencodex-catalog.json"), JSON.stringify(catalog)); + + const url = new URL("http://localhost/api/catalog"); + const response = await handleManagementAPI( + new ManagementRequest(url, { headers: managementHeaders() }), + url, + loadConfig(), + ); + expect(response?.status).toBe(200); + expect(await response!.json()).toEqual(catalog); + expect(response!.headers.get("x-opencodex-codex-version")).toBeNull(); + }); + + test("returns 404 when the catalog file is missing", async () => { + isolatedCodexHome = installIsolatedCodexHome("ocx-api-catalog-missing-"); + const url = new URL("http://localhost/api/catalog"); + const response = await handleManagementAPI( + new ManagementRequest(url, { headers: managementHeaders() }), + url, + loadConfig(), + ); + expect(response?.status).toBe(404); + expect(await response!.json()).toEqual({ error: "catalog not found" }); + }); +}); From f726100275916f95903dd779ef69a0e2f55dc63e Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:47:20 +0900 Subject: [PATCH 04/24] feat(claude): opt out of native/* models in the Claude Desktop list (#767) Claude Desktop's model picker showed every `native/*` row, which is noise for users who route through OpenCodex precisely to reach other providers. `claudeCode.desktopNativeModels` gates them, and the gate is applied at every Desktop surface that emits the list -- discovery and profile export both -- rather than only the one the report happened to name. Grok natives stay: they are reachable through Desktop and hiding them would remove working models. The regression test fails without the change by finding `native/*` in both the show and export output. Co-authored-by: bitkyc08-arch --- src/cli/claude-desktop.ts | 4 +-- src/codex/catalog.ts | 2 +- src/codex/catalog/metadata.ts | 6 +++++ src/server/index.ts | 6 ++--- .../management/agent-settings-routes.ts | 8 +++--- src/server/management/shared.ts | 19 +++++++++++--- src/types.ts | 5 ++++ tests/claude-desktop-cli.test.ts | 25 +++++++++++++++++++ tests/grok-management-api.test.ts | 17 +++++++++++++ 9 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/cli/claude-desktop.ts b/src/cli/claude-desktop.ts index 4ab0148d2..6e426ffa8 100644 --- a/src/cli/claude-desktop.ts +++ b/src/cli/claude-desktop.ts @@ -10,7 +10,7 @@ import { type DesktopProfile, } from "../claude/desktop-profile"; import { writeDesktop3pConfig, type Desktop3pConfigMode, parseDesktop3pModeArgs } from "../claude/desktop-3p"; -import { filterCatalogVisibleModels, visibleNativeSlugs } from "../codex/catalog"; +import { filterCatalogVisibleModels, desktopVisibleNativeSlugs } from "../codex/catalog"; import { buildClaudeDesktopState, fetchAllModels } from "../server/management-api"; import { findLiveProxy } from "../server/proxy-liveness"; @@ -42,7 +42,7 @@ async function applyProfile(profile: DesktopProfile, mode: Desktop3pConfigMode): })); const result = writeDesktop3pConfig( live?.port ?? config.port ?? 10100, - [...visibleNativeSlugs(config)], + [...desktopVisibleNativeSlugs(config)], routed, config.apiKeys?.[0]?.key, mode, diff --git a/src/codex/catalog.ts b/src/codex/catalog.ts index 257e3a799..c6f21c750 100644 --- a/src/codex/catalog.ts +++ b/src/codex/catalog.ts @@ -2,7 +2,7 @@ // Public surface preserved exactly; importers keep using "src/codex/catalog". export { isMediaGenerationModelId, shouldExposeRoutedModel, readCodexCatalogPath, readCatalog, normalizeRoutedCatalogEntry, catalogModelSlug, filterSupportedNativeSlugs, catalogModelSupportsReasoningSummaries } from "./catalog/parsing"; export type { CatalogModel, MultiAgentMode } from "./catalog/parsing"; -export { NATIVE_OPENAI_MODELS, nativeOpenAiContextWindow, disabledNativeSlugs, visibleNativeSlugs, nativeModelRows, applyNativeVisibility, upstreamNativeEntry, nativeOpenAiSlugs, listCatalogNativeSlugs } from "./catalog/metadata"; +export { NATIVE_OPENAI_MODELS, nativeOpenAiContextWindow, disabledNativeSlugs, visibleNativeSlugs, desktopVisibleNativeSlugs, nativeModelRows, applyNativeVisibility, upstreamNativeEntry, nativeOpenAiSlugs, listCatalogNativeSlugs } from "./catalog/metadata"; export { isSpawnableCodexCandidate, codexExecInvocation, loadBundledCodexCatalog, materializeBundledCodexCatalog, loadCatalogTemplate } from "./catalog/bundled"; export { nativeEffortClamp, shouldApplyNativeEffortClamp, catalogModelEfforts, codexSupportedReasoningEfforts, clampedDefaultEffort, clampEntryToCodexSupportedEfforts, clampCatalogModelsToCodexSupport } from "./catalog/effort"; export { applyProviderConfigHints, isDatedVariantId, filterCatalogVisibleModels, gatherRoutedModels, clearGatherRoutedModelsInflight, augmentRoutedModelsWithRegistryOpenAiApiRows, augmentRoutedModelsWithJawcodeMetadata } from "./catalog/provider-fetch"; diff --git a/src/codex/catalog/metadata.ts b/src/codex/catalog/metadata.ts index 8587c286a..4f4786e17 100644 --- a/src/codex/catalog/metadata.ts +++ b/src/codex/catalog/metadata.ts @@ -123,6 +123,12 @@ export function visibleNativeSlugs(config: Pick): s return nativeOpenAiSlugs().filter(slug => !disabled.has(slug)); } +/** Native slugs exposed to Claude Desktop show/export/apply (opt-out via claudeCode.desktopNativeModels). */ +export function desktopVisibleNativeSlugs(config: Pick): string[] { + if (config.claudeCode?.desktopNativeModels === false) return []; + return visibleNativeSlugs(config); +} + export function nativeModelRows(config: Pick): Array<{ slug: string; disabled: boolean; contextWindow?: number }> { const disabled = disabledNativeSlugs(config); return NATIVE_OPENAI_MODELS.map(slug => { diff --git a/src/server/index.ts b/src/server/index.ts index 0df808b2a..53ed9ea21 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -406,7 +406,7 @@ export function startServer(port?: number) { return withCors(formatErrorResponse(403, "origin_rejected", "cross-origin data-plane request blocked"), req, config); } const goModels = await fetchAllModels(config); - const { applyNativeVisibility, buildCatalogEntries, disabledNativeSlugs, exactComboCatalogSlugs, loadCatalogTemplate, nativeOpenAiSlugs, orderForSubagents, filterCatalogVisibleModels, uniqueCatalogModelsForRawPublicList, visibleNativeSlugs } = await import("../codex/catalog"); + const { applyNativeVisibility, buildCatalogEntries, disabledNativeSlugs, exactComboCatalogSlugs, loadCatalogTemplate, nativeOpenAiSlugs, orderForSubagents, filterCatalogVisibleModels, uniqueCatalogModelsForRawPublicList, visibleNativeSlugs, desktopVisibleNativeSlugs } = await import("../codex/catalog"); const nativeSlugs = nativeOpenAiSlugs(); const goEnabled = filterCatalogVisibleModels(goModels, config); const goOrdered = orderForSubagents(goEnabled, config.subagentModels); @@ -424,7 +424,7 @@ export function startServer(port?: number) { if (config.claudeCode?.enabled === false) return jsonResponse({ data: [] }, 200, req, config); // Build Desktop 3P registry so inbound alias resolution works for subsequent requests. buildDesktop3pRegistry( - [...visibleNativeSlugs(config)], + [...desktopVisibleNativeSlugs(config)], goOrdered.map(m => ({ provider: m.provider, id: m.id, contextWindow: m.contextWindow })), config.claudeCode?.desktopProfile, ); @@ -441,7 +441,7 @@ export function startServer(port?: number) { : idsParam === "desktop" ? "desktop3p" as const : (/^claude-code\//i.test(req.headers.get("user-agent") ?? "") ? "readable" as const : "desktop3p" as const); - const data = buildAnthropicModelInfos([...visibleNativeSlugs(config)], goOrdered, resolveAutoContext(config.claudeCode), idStyle, activeDesktop3pAlias); + const data = buildAnthropicModelInfos([...desktopVisibleNativeSlugs(config)], goOrdered, resolveAutoContext(config.claudeCode), idStyle, activeDesktop3pAlias); return jsonResponse({ data }, 200, req, config); } if (url.searchParams.has("client_version")) { diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index f1d2e7e32..4fe280b11 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -80,12 +80,12 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise if (config.claudeCode?.desktopAutoApply === false) return; if (!config.claudeCode?.desktopProfile) return; const { writeDesktop3pConfig } = await import("../../claude/desktop-3p"); - const { visibleNativeSlugs, filterCatalogVisibleModels } = await import("../../codex/catalog"); + const { filterCatalogVisibleModels, desktopVisibleNativeSlugs } = await import("../../codex/catalog"); const allModels = await fetchAllModels(config); const routed = filterCatalogVisibleModels(allModels, config).map(m => ({ provider: m.provider, id: m.id, contextWindow: m.contextWindow })); const result = writeDesktop3pConfig( config.port ?? 10100, - [...visibleNativeSlugs(config)], + [...desktopVisibleNativeSlugs(config)], routed, config.apiKeys?.[0]?.key, "static", @@ -607,7 +607,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: state.profile }; saveConfigPreservingClaudeCode(config); const { writeDesktop3pConfig } = await import("../../claude/desktop-3p"); - const { visibleNativeSlugs } = await import("../../codex/catalog"); + const { desktopVisibleNativeSlugs } = await import("../../codex/catalog"); const routed = state.models .filter(model => model.available && !model.route.startsWith("native/")) .map(model => { @@ -616,7 +616,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise }); const result = writeDesktop3pConfig( Number(url.port) || config.port, - [...visibleNativeSlugs(config)], + [...desktopVisibleNativeSlugs(config)], routed, config.apiKeys?.[0]?.key, "static", diff --git a/src/server/management/shared.ts b/src/server/management/shared.ts index 6c6f20282..29cdde2e0 100644 --- a/src/server/management/shared.ts +++ b/src/server/management/shared.ts @@ -214,14 +214,14 @@ export function stripRegistryOnlyStaticHeaders(name: string, provider: OcxProvid /** Shared Desktop profile DTO builder for the management API and CLI. */ export async function buildClaudeDesktopState(config: OcxConfig, stored?: OcxClaudeDesktopProfile) { - const { filterCatalogVisibleModels, nativeOpenAiContextWindow, visibleNativeSlugs } = await import("../../codex/catalog"); + const { filterCatalogVisibleModels, nativeOpenAiContextWindow, desktopVisibleNativeSlugs } = await import("../../codex/catalog"); const { DESKTOP_SUPPORTS_1M_THRESHOLD } = await import("../../claude/desktop-3p"); const { reconcileDesktopProfile, renderDesktopProfile } = await import("../../claude/desktop-profile"); const routed = filterCatalogVisibleModels(await fetchAllModels(config), config); const profileModels: DesktopProfileModel[] = [ // Native rows carry their real context window from the same accessor the Grok sync // uses — otherwise Sol's 372k and gpt-5.5's 272k render as blank on Desktop. - ...visibleNativeSlugs(config).map(id => { + ...desktopVisibleNativeSlugs(config).map(id => { const contextWindow = nativeOpenAiContextWindow(id); return { route: `native/${id}`, label: `${id} (native)`, ...(contextWindow !== undefined ? { contextWindow } : {}) }; @@ -233,6 +233,19 @@ export async function buildClaudeDesktopState(config: OcxConfig, stored?: OcxCla })), ]; const profile = reconcileDesktopProfile(stored ?? config.claudeCode?.desktopProfile, profileModels); + if (config.claudeCode?.desktopNativeModels === false) { + for (const route of Object.keys(profile.assignments)) { + if (route.startsWith("native/")) delete profile.assignments[route]; + } + for (const family of ["opus", "fable", "sonnet", "haiku"] as const) { + const current = profile.defaults[family]; + if (current?.startsWith("native/")) { + profile.defaults[family] = Object.keys(profile.assignments) + .filter(route => profile.assignments[route]?.family === family) + .sort()[0] ?? null; + } + } + } const available = new Set(profileModels.map(model => model.route)); const modelByRoute = new Map(profileModels.map(model => [model.route, model])); // Effort support: routed models with a non-empty reasoningEfforts ladder support effort; @@ -241,7 +254,7 @@ export async function buildClaudeDesktopState(config: OcxConfig, stored?: OcxCla for (const m of routed) { effortByRoute.set(`${m.provider}/${m.id}`, Array.isArray(m.reasoningEfforts) && m.reasoningEfforts.length > 0); } - for (const id of visibleNativeSlugs(config)) { + for (const id of desktopVisibleNativeSlugs(config)) { effortByRoute.set(`native/${id}`, true); } const models = Object.keys(profile.assignments).sort().map(route => ({ diff --git a/src/types.ts b/src/types.ts index 2ed5e691f..d950eff1b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -455,6 +455,11 @@ export interface OcxClaudeCodeConfig { desktopProfile?: OcxClaudeDesktopProfile; /** Auto-reconcile Desktop 3P config when provider catalog changes. Default: enabled. */ desktopAutoApply?: boolean; + /** + * When false, omit `native/*` rows from Claude Desktop show/export/apply. Default: enabled. + * Routing-sidecar alias decoding is unchanged — only the Desktop model list writer. + */ + desktopNativeModels?: boolean; } export type OcxClaudeDesktopFamily = "opus" | "fable" | "sonnet" | "haiku"; diff --git a/tests/claude-desktop-cli.test.ts b/tests/claude-desktop-cli.test.ts index 22e1eee97..b44a9aec1 100644 --- a/tests/claude-desktop-cli.test.ts +++ b/tests/claude-desktop-cli.test.ts @@ -70,6 +70,31 @@ test("import rejects invalid profiles without replacing saved state", async () = } }); +test("desktopNativeModels:false omits native/* from show and exported profile", async () => { + saveConfig({ + port: 10100, + defaultProvider: "mock", + providers: { + mock: { adapter: "openai-chat", baseUrl: "http://127.0.0.1:1/v1", apiKey: "k", allowPrivateNetwork: true, models: ["test-model"] }, + }, + claudeCode: { desktopNativeModels: false }, + } as OcxConfig); + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + expect(await handleClaudeDesktopCommand(["show", "--json"])).toBe(0); + const state = JSON.parse(String(log.mock.calls.at(-1)?.[0])); + expect(state.models.every((model: { route: string }) => !model.route.startsWith("native/"))).toBe(true); + expect(Object.keys(state.profile.assignments).every((route: string) => !route.startsWith("native/"))).toBe(true); + + const target = join(dir, "desktop-profile.json"); + expect(await handleClaudeDesktopCommand(["export", target])).toBe(0); + const exported = JSON.parse(readFileSync(target, "utf8")); + expect(Object.keys(exported.assignments).every((route: string) => !route.startsWith("native/"))).toBe(true); + } finally { + log.mockRestore(); + } +}); + test("no-arg and legacy mode flags apply Desktop config", async () => { const log = spyOn(console, "log").mockImplementation(() => {}); const error = spyOn(console, "error").mockImplementation(() => {}); diff --git a/tests/grok-management-api.test.ts b/tests/grok-management-api.test.ts index 3f3e82460..73436ab3d 100644 --- a/tests/grok-management-api.test.ts +++ b/tests/grok-management-api.test.ts @@ -118,6 +118,23 @@ test("GET /api/grok includes candidates and the saved exclusion list", async () } }); +test("GET /api/grok keeps native candidates when claudeCode.desktopNativeModels is false", async () => { + const config = loadConfig(); + config.claudeCode = { desktopNativeModels: false }; + saveConfig(config); + const server = startServer(0); + try { + const res = await fetch(new URL("/api/grok", server.url)); + expect(res.status).toBe(200); + const body = await res.json() as { + candidates: Array<{ id: string; native: boolean }>; + }; + expect(body.candidates.some(c => c.native)).toBe(true); + } finally { + server.stop(true); + } +}); + test("POST /api/grok/apply reports no-grok-home as a policy skip, not an error", async () => { const server = startServer(0); try { From 0856b0cb77b8892082c737a272024fb49835d113 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Fri, 31 Jul 2026 10:47:36 +0900 Subject: [PATCH 05/24] feat(providers): add Baseten to the free provider directory A signup-credit row in the free directory, not a canonical registry preset. That distinction is the point: MAINTAINERS.md sets a credential-destination evidence bar for `PROVIDER_REGISTRY` entries, and an inert directory reference is the documented placement when that evidence is not fully assembled. The submission carries a dated end-to-end check (2026-07-30): the OpenAI-shaped `/v1/models` catalog and a real chat completion, both recorded inline next to the entry. Contributed by a Baseten employee; disclosed, which under MAINTAINERS.md is neither disqualifying nor a reason to lower the bar. Promotion to a canonical preset would still need ToS, operating legal entity, and a named maintenance owner -- see #653 for that path. Co-authored-by: bitkyc08-arch --- src/providers/free-directory.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/providers/free-directory.ts b/src/providers/free-directory.ts index 6d042d700..c390d00ed 100644 --- a/src/providers/free-directory.ts +++ b/src/providers/free-directory.ts @@ -18,7 +18,7 @@ export const FREE_PROVIDER_ACCESS_GROUPS = { ], "recurring-credit": ["bytez", "nous-research"], "signup-credit": [ - "agentrouter", "ai21", "baichuan", "deepinfra", "deepseek", "doubao", "fireworks", "freemodel-dev", "glm-cn", + "agentrouter", "ai21", "baichuan", "baseten", "deepinfra", "deepseek", "doubao", "fireworks", "freemodel-dev", "glm-cn", "hyperbolic", "longcat", "monsterapi", "nebius", "novita", "nscale", "nvidia", "predibase", "publicai", "qoder", "scaleway", "sensenova", "stepfun", "together", "vertex", ], @@ -119,6 +119,9 @@ const CONNECTABLE: Record = { agentrouter: { baseUrl: "https://agentrouter.org", dashboardUrl: "https://agentrouter.org", adapter: "anthropic", authKind: "key", supportLevel: "experimental", verification: "primary", modelsUrl: "https://agentrouter.org/v1/models", lastVerified: LAST_VERIFIED, discovery: "live", liveModels: true }, ai21: openAi("https://api.ai21.com/studio/v1", "https://studio.ai21.com/account/api-key", { supportLevel: "supported", verification: "official", documentationUrl: "https://docs.ai21.com/reference/models" }), baichuan: openAi("https://api.baichuan-ai.com/v1", "https://platform.baichuan-ai.com/console/apikey", { verification: "official" }), + // Verified end-to-end 2026-07-30: /v1/models returns the OpenAI-shaped live catalog (13 models), + // and a chat completion against moonshotai/Kimi-K3 returned a standard chat.completion payload. + baseten: openAi("https://inference.baseten.co/v1", "https://app.baseten.co/settings/api_keys", { supportLevel: "supported", verification: "official", documentationUrl: "https://docs.baseten.co/inference/model-apis/overview", modelsUrl: "https://inference.baseten.co/v1/models", lastVerified: "2026-07-30" }), deepinfra: openAi("https://api.deepinfra.com/v1/openai", "https://deepinfra.com/dash/api_keys", { supportLevel: "supported", verification: "official", documentationUrl: "https://deepinfra.com/docs/openai_api" }), deepseek: openAi("https://api.deepseek.com", "https://platform.deepseek.com/api_keys", { supportLevel: "supported", verification: "official", documentationUrl: "https://api-docs.deepseek.com/api/list-models" }), doubao: openAi("https://ark.cn-beijing.volces.com/api/v3", "https://console.volcengine.com/ark/region:ark+cn-beijing/apiKey", { verification: "official" }), From ba77eb8e9b3b6e9e0d4e9fc4923ac2118fcff010 Mon Sep 17 00:00:00 2001 From: aljjang95 Date: Fri, 31 Jul 2026 10:49:04 +0900 Subject: [PATCH 06/24] fix(kiro): resolve the Windows kiro-cli executable without PATH Forced and add-account Kiro login shell out to the local CLI by bare name. #710 taught Windows where the SQLite store lives, so a Windows box can import tokens while PATH still has no `kiro-cli` -- and then login fails on a machine that is otherwise correctly installed. The resolver tries PATH first (both `PATH` and `Path`, since Windows uses the latter), then the layouts the installers actually produce: `%LOCALAPPDATA%\Kiro-Cli` and `C:\Program Files\Kiro-Cli` on Windows, `~/.local/bin` and the usual prefixes elsewhere. With nothing found it returns the bare command so spawn still reports the original error rather than a path this code invented. Two additions on top of the contributed patch. `existsSync` alone accepts a *directory* named `kiro-cli`, which then reaches spawn() and fails with EACCES instead of falling through to the next candidate -- candidates now have to be files. And the tests only ever injected `pathEntries`, so the environment parsing itself was never exercised; a case now drives `Path` and `PATH` through the real code. Both new tests were checked by ablation: removing the `isFile` guard fails the directory case (6 pass / 1 fail) and restoring it returns 7/7, so neither passes vacuously. Windows behavior is unverifiable from macOS; the matrix CI on this branch is what confirms it. Co-authored-by: bitkyc08-arch --- .../src/content/docs/guides/providers.md | 4 + src/oauth/kiro-credentials.ts | 73 +++++++++++- src/oauth/kiro.ts | 15 ++- .../kiro-windows-cli-executable-path.test.ts | 109 ++++++++++++++++++ 4 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 tests/kiro-windows-cli-executable-path.test.ts diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index e14f0d6e0..773a92447 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -165,6 +165,10 @@ read-only. Two environment variables make the source and token row selection exp - `KIROCLI_TOKEN_KEY` selects the exact `auth_kv` token key when a database contains multiple otherwise ambiguous token rows. A missing selection fails login instead of guessing. +On Windows, import looks for `%LOCALAPPDATA%\Kiro-Cli\data.sqlite3`. Forced/add-account login +also needs the local CLI binary: opencodex first uses `PATH`, then falls back to +`%LOCALAPPDATA%\Kiro-Cli\kiro-cli.exe` and `C:\Program Files\Kiro-Cli\kiro-cli.exe`. + After a successful import, opencodex persists the imported credential to `~/.opencodex/auth.json`. diff --git a/src/oauth/kiro-credentials.ts b/src/oauth/kiro-credentials.ts index 3ce3dc7cf..3019262c7 100644 --- a/src/oauth/kiro-credentials.ts +++ b/src/oauth/kiro-credentials.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { chmodSync, closeSync, existsSync, fsyncSync, linkSync, openSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, closeSync, existsSync, fsyncSync, linkSync, openSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { isAbsolute, join, posix, win32 } from "node:path"; import { Database } from "bun:sqlite"; @@ -169,6 +169,77 @@ export function resolveKiroCliNativeSessionEntries( return [{ location: "kiro-cli-linux-data", path: posix.join(home, ".local", "share", "kiro-cli", "data.sqlite3") }]; } +/** + * Resolve the absolute kiro-cli executable for spawn/login helpers. + * + * Pure + parameterized like `resolveKiroCliNativeSessionEntries` so Windows install layouts can be + * covered from any host. PATH remains the first choice; only when bare `kiro-cli` is missing do we + * fall back to the platform-native install directories next to the session database. + * + * Windows: official MSI installs to `C:\Program Files\Kiro-Cli\kiro-cli.exe`, while some local + * installs keep the binary next to `%LOCALAPPDATA%\Kiro-Cli\data.sqlite3`. + * macOS/Linux: prefer PATH, then the usual user-local bin directories. + */ +export function resolveKiroCliExecutable( + inputs: KiroCliNativeInputs & { + pathEntries?: string[]; + exists?: (path: string) => boolean; + isFile?: (path: string) => boolean; + }, +): string { + const exists = inputs.exists ?? existsSync; + // A directory named `kiro-cli` on PATH satisfies existsSync and would then be handed to + // spawn(), which fails with EACCES at login instead of falling through to the next candidate. + // When a caller injects `exists` it owns the whole filesystem view, so the real stat would + // reject every synthetic path; such callers inject `isFile` too when they care about it. + const isFile = inputs.isFile ?? (inputs.exists ? () => true : ((path: string) => { + try { + return statSync(path).isFile(); + } catch { + return false; + } + })); + const pathEntries = inputs.pathEntries + ?? (inputs.env.PATH ?? inputs.env.Path ?? "").split(inputs.platform === "win32" ? ";" : ":") + .map(entry => entry.trim()) + .filter(Boolean); + + const pathCandidates = inputs.platform === "win32" + ? pathEntries.flatMap(entry => [ + win32.join(entry, "kiro-cli.exe"), + win32.join(entry, "kiro-cli"), + ]) + : pathEntries.map(entry => posix.join(entry, "kiro-cli")); + + const installCandidates: string[] = []; + if (inputs.platform === "win32") { + const localBase = inputs.env.LOCALAPPDATA?.trim() + || (inputs.env.USERPROFILE?.trim() ? win32.join(inputs.env.USERPROFILE.trim(), "AppData", "Local") : "") + || win32.join(inputs.home, "AppData", "Local"); + const programFiles = inputs.env["ProgramFiles"]?.trim() || "C:\\Program Files"; + installCandidates.push( + win32.join(localBase, "Kiro-Cli", "kiro-cli.exe"), + win32.join(programFiles, "Kiro-Cli", "kiro-cli.exe"), + ); + } else if (inputs.platform === "darwin") { + installCandidates.push( + posix.join(inputs.home, ".local", "bin", "kiro-cli"), + "/usr/local/bin/kiro-cli", + "/opt/homebrew/bin/kiro-cli", + ); + } else { + installCandidates.push( + posix.join(inputs.home, ".local", "bin", "kiro-cli"), + "/usr/local/bin/kiro-cli", + ); + } + + for (const candidate of [...pathCandidates, ...installCandidates]) { + if (exists(candidate) && isFile(candidate)) return candidate; + } + return inputs.platform === "win32" ? "kiro-cli.exe" : "kiro-cli"; +} + function nativeKiroCliSessionEntries(): Array<{ location: KiroCliNativeLocation; path: string }> { // Only the stores that `kiro-cli logout` / `kiro-cli login` themselves mutate. Import fallbacks // (Amazon Q / SSO cache) and KIROCLI_DB_PATH selectors must not be snapshotted for rollback. diff --git a/src/oauth/kiro.ts b/src/oauth/kiro.ts index d9a978913..f4df35282 100644 --- a/src/oauth/kiro.ts +++ b/src/oauth/kiro.ts @@ -18,6 +18,7 @@ import { persistKiroCliSessionRecovery, readImportedKiroCredential, readKiroCliSqliteCredential, + resolveKiroCliExecutable, restoreKiroCliSession, restoreStaleKiroCliSessionRecovery, requireKiroRegion, @@ -25,6 +26,7 @@ import { type KiroCliSessionSnapshot, type KiroImportDiagnostic, } from "./kiro-credentials"; +import { homedir } from "node:os"; import { getAccountSet, saveAccountCredential } from "./store"; const DEFAULT_REGION = "us-east-1"; @@ -72,9 +74,18 @@ const pendingKiroLoginTransactions = new WeakMap(); + +function resolveRuntimeKiroCliExecutable(): string { + return resolveKiroCliExecutable({ + env: process.env, + platform: process.platform, + home: process.platform === "win32" ? homedir() : (process.env.HOME || homedir()), + }); +} + function logoutKiroCliBestEffort(): void { try { - Bun.spawnSync(["kiro-cli", "logout"], { + Bun.spawnSync([resolveRuntimeKiroCliExecutable(), "logout"], { stdin: "ignore", stdout: "ignore", stderr: "ignore", @@ -128,7 +139,7 @@ async function defaultKiroCliRunner(args: string[], signal?: AbortSignal): Promi throwIfKiroLoginCancelled(signal); let child: ReturnType; try { - child = Bun.spawn(["kiro-cli", ...args], { + child = Bun.spawn([resolveRuntimeKiroCliExecutable(), ...args], { stdin: "ignore", stdout: "pipe", stderr: "ignore", diff --git a/tests/kiro-windows-cli-executable-path.test.ts b/tests/kiro-windows-cli-executable-path.test.ts new file mode 100644 index 000000000..8dc262096 --- /dev/null +++ b/tests/kiro-windows-cli-executable-path.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, test } from "bun:test"; +import { resolveKiroCliExecutable } from "../src/oauth/kiro-credentials"; + +/** + * Forced/add-account Kiro login still shells out to the local CLI. After #710 fixed Windows + * SQLite discovery, Windows installs can import tokens while PATH still lacks `kiro-cli`. + * The pure executable resolver covers that layout without launching the real binary. + */ +describe("kiro-cli executable resolution", () => { + const WIN_HOME = "C:\\Users\\u"; + + test("prefers the first PATH hit before install-directory fallbacks", () => { + const exists = (path: string) => path === "C:\\Tools\\kiro-cli.exe"; + expect(resolveKiroCliExecutable({ + env: { + PATH: "C:\\Tools;C:\\Windows\\System32", + LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local", + }, + platform: "win32", + home: WIN_HOME, + pathEntries: ["C:\\Tools", "C:\\Windows\\System32"], + exists, + })).toBe("C:\\Tools\\kiro-cli.exe"); + }); + + test("win32 falls back to %LOCALAPPDATA%\\Kiro-Cli\\kiro-cli.exe", () => { + const exists = (path: string) => path === "C:\\Users\\u\\AppData\\Local\\Kiro-Cli\\kiro-cli.exe"; + expect(resolveKiroCliExecutable({ + env: { + PATH: "C:\\Windows\\System32", + LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local", + }, + platform: "win32", + home: WIN_HOME, + pathEntries: ["C:\\Windows\\System32"], + exists, + })).toBe("C:\\Users\\u\\AppData\\Local\\Kiro-Cli\\kiro-cli.exe"); + }); + + test("win32 falls back to Program Files\\Kiro-Cli when LOCALAPPDATA binary is absent", () => { + const exists = (path: string) => path === "C:\\Program Files\\Kiro-Cli\\kiro-cli.exe"; + expect(resolveKiroCliExecutable({ + env: { + PATH: "C:\\Windows\\System32", + LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local", + ProgramFiles: "C:\\Program Files", + }, + platform: "win32", + home: WIN_HOME, + pathEntries: ["C:\\Windows\\System32"], + exists, + })).toBe("C:\\Program Files\\Kiro-Cli\\kiro-cli.exe"); + }); + + test("linux keeps PATH-first resolution and falls back to ~/.local/bin", () => { + const exists = (path: string) => path === "/home/u/.local/bin/kiro-cli"; + expect(resolveKiroCliExecutable({ + env: { PATH: "/usr/bin" }, + platform: "linux", + home: "/home/u", + pathEntries: ["/usr/bin"], + exists, + })).toBe("/home/u/.local/bin/kiro-cli"); + }); + + test("returns the bare command when no candidate exists so spawn can report the original error", () => { + expect(resolveKiroCliExecutable({ + env: { PATH: "C:\\Windows\\System32" }, + platform: "win32", + home: WIN_HOME, + pathEntries: ["C:\\Windows\\System32"], + exists: () => false, + })).toBe("kiro-cli.exe"); + }); + + test("skips a directory named kiro-cli and keeps looking", () => { + // A directory passes existsSync, so without the isFile guard this resolves to + // C:\Tools\kiro-cli and spawn() fails with EACCES at login time. + const exists = (path: string) => + path === "C:\\Tools\\kiro-cli" || path === "C:\\Program Files\\Kiro-Cli\\kiro-cli.exe"; + const isFile = (path: string) => path !== "C:\\Tools\\kiro-cli"; + expect(resolveKiroCliExecutable({ + env: { PATH: "C:\\Tools", ProgramFiles: "C:\\Program Files" }, + platform: "win32", + home: WIN_HOME, + pathEntries: ["C:\\Tools"], + exists, + isFile, + })).toBe("C:\\Program Files\\Kiro-Cli\\kiro-cli.exe"); + }); + + test("parses PATH from the environment under both Windows casings", () => { + // The resolver must read the env itself, not only an injected pathEntries array: + // Windows exposes the variable as `Path`, POSIX as `PATH`. + const exists = (path: string) => path === "C:\\Tools\\kiro-cli.exe"; + expect(resolveKiroCliExecutable({ + env: { Path: "C:\\Tools;C:\\Windows\\System32" }, + platform: "win32", + home: WIN_HOME, + exists, + })).toBe("C:\\Tools\\kiro-cli.exe"); + expect(resolveKiroCliExecutable({ + env: { PATH: "C:\\Tools" }, + platform: "win32", + home: WIN_HOME, + exists, + })).toBe("C:\\Tools\\kiro-cli.exe"); + }); +}); From decb08ba2a3dc29334b38fecba508a384822f821 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:51:15 +0900 Subject: [PATCH 07/24] fix(anthropic): normalize the messages baseUrl and harden stream quirks (#765) Four defects an Anthropic-compatible relay exposes, all in the same adapter. A `baseUrl` pasted as `https://host/v1/messages` -- which is what the provider's own docs show -- got `/v1/messages` appended again. The URL is now normalized so both the bare-origin and full-endpoint forms resolve to one request path. `input_json_delta` was accepted regardless of the open block, so a relay emitting JSON deltas outside a `tool_use` block corrupted the assembled arguments; the delta is now scoped to `tool_use`. A `tool_use` block arriving without an `id` was forwarded with an empty one, which downstream treats as unpaired -- a `toolu_` id is synthesized instead. And a stream that reached EOF carrying a `message_delta.stop_reason` but no `message_stop` now settles from that stop reason rather than hanging. Two changes on top of the contributed patch: `toolUseArguments` re-encoded an unparseable string input as a JSON *string*, so `"not json at all"` arrived where the tool contract requires an object. That is the double-encoding half of #765 and it survived the original fix. It degrades to `{}` now, which still fails -- but inside the tool's own argument validation instead of as a type error. A test pins it. `sawContent` was assigned in six places and read in none; removed. The contributed branch also swapped three `/api/logs` test files onto a `logsFromApiBody` helper that accepts both the bare array and a `{logs}` envelope. That helper is not brought across: it would pre-accept the response-shape change this round has not decided on, and a matcher that tolerates both shapes cannot fail when the shape regresses. Only the adapter and its own test come over. #765 stays open -- it asks for broader relay quirk tolerance than these four fixes. Note for whoever runs the suite: `tests/claude-messages-endpoint.test.ts` and `tests/claude-native-passthrough.test.ts` already fail together on an unmodified tree ("unmapped claude model + sk-ant credential passes through verbatim"), and pass individually. That interference predates this commit. Co-authored-by: bitkyc08-arch --- src/adapters/anthropic.ts | 49 +++++++++-- tests/anthropic-stream-hardening.test.ts | 106 +++++++++++++++++++++++ 2 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 tests/anthropic-stream-hardening.test.ts diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index d54918f22..3a5654300 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -250,6 +250,40 @@ function usesNativeAnthropicEndpoint(provider: OcxProviderConfig): boolean { } } +/** Normalize provider baseUrl paths ending in `/`, `/v1`, or `/v1/messages` to `{origin}/v1/messages`. */ +export function anthropicMessagesUrl(baseUrl: string): string { + try { + new URL(baseUrl); + } catch { + throw new Error(`anthropic provider has malformed baseUrl: ${baseUrl}`); + } + const trimmed = baseUrl.trim().replace(/\/+$/, ""); + const root = trimmed.replace(/\/v1\/messages\/?$/i, "").replace(/\/v1\/?$/i, "").replace(/\/+$/, ""); + return `${root}/v1/messages`; +} + +function synthesizeToolUseId(): string { + return `toolu_${crypto.randomUUID().replace(/-/g, "").slice(0, 24)}`; +} + +function toolUseArguments(input: unknown): string { + if (typeof input === "string") { + const trimmed = input.trim(); + if (!trimmed) return "{}"; + try { + JSON.parse(trimmed); + return trimmed; + } catch { + // A tool call's arguments must be a JSON object. Re-encoding an unparseable string as a + // JSON *string* is the double-encoding #765 reports: the caller then receives + // `"get weather"` where an object was required and the tool call is unusable either way. + // An empty object at least fails in the tool's own argument validation. + return "{}"; + } + } + return JSON.stringify(input ?? {}); +} + function anthropicKeyUsesBearer(provider: OcxProviderConfig): boolean { return provider.apiKeyTransport === "bearer"; } @@ -680,8 +714,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti else if (typeof tc === "object" && "name" in tc) body.tool_choice = { type: "tool", name: toolNames.toWire(resolveToolChoiceWireName(parsed.context.tools, tc.name)) }; } - const base = provider.baseUrl.replace(/\/v1\/?$/, ""); - const url = `${base}/v1/messages`; + const url = anthropicMessagesUrl(provider.baseUrl); const unresolvedPlaceholder = url.match(/\{[^}]*\}/)?.[0]; if (unresolvedPlaceholder) { throw new Error(`anthropic baseUrl contains unresolved ${unresolvedPlaceholder}`); @@ -773,7 +806,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti if (!block) break; currentBlockType = block.type; if (block.type === "tool_use") { - currentToolCallId = block.id ?? ""; + currentToolCallId = block.id ?? synthesizeToolUseId(); currentToolCallName = toolNames.fromWire(block.name ?? ""); yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName }; } @@ -799,7 +832,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti // Arrives once, just before the thinking block's content_block_stop; block-scoped // so a stray signature on a non-thinking block can never be captured. yield { type: "thinking_signature", signature: delta.signature }; - } else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string") { + } else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") { yield { type: "tool_call_delta", arguments: delta.partial_json }; } break; @@ -831,12 +864,13 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti } } if (!emittedDone) { + // Fail closed on transport EOF. Compatible providers may omit message_stop after message_delta.stop_reason. if (pendingStopReason !== undefined) { const stopReason = pendingStopReason === "max_tokens" ? "max_tokens" : pendingStopReason === "refusal" || pendingStopReason === "content_filter" ? "content_filter" - : undefined; + : pendingStopReason; emittedDone = true; yield { type: "done", @@ -867,8 +901,9 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti } else if (block.type === "redacted_thinking" && typeof block.data === "string") { events.push({ type: "redacted_thinking", data: block.data }); } else if (block.type === "tool_use") { - events.push({ type: "tool_call_start", id: block.id ?? "", name: toolNames.fromWire(block.name ?? "") }); - events.push({ type: "tool_call_delta", arguments: JSON.stringify(block.input ?? {}) }); + const id = block.id ?? synthesizeToolUseId(); + events.push({ type: "tool_call_start", id, name: toolNames.fromWire(block.name ?? "") }); + events.push({ type: "tool_call_delta", arguments: toolUseArguments(block.input) }); events.push({ type: "tool_call_end" }); } } diff --git a/tests/anthropic-stream-hardening.test.ts b/tests/anthropic-stream-hardening.test.ts new file mode 100644 index 000000000..4d01d7c3d --- /dev/null +++ b/tests/anthropic-stream-hardening.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, test } from "bun:test"; +import { anthropicMessagesUrl, createAnthropicAdapter } from "../src/adapters/anthropic"; +import type { AdapterEvent } from "../src/types"; + +const provider = { adapter: "anthropic", baseUrl: "https://example.test", apiKey: "key" }; + +async function collect(gen: AsyncGenerator): Promise { + const out: AdapterEvent[] = []; + for await (const e of gen) out.push(e); + return out; +} + +describe("anthropicMessagesUrl", () => { + test.each([ + ["https://example.test", "https://example.test/v1/messages"], + ["https://example.test/", "https://example.test/v1/messages"], + ["https://example.test/v1", "https://example.test/v1/messages"], + ["https://example.test/v1/", "https://example.test/v1/messages"], + ["https://example.test/v1/messages", "https://example.test/v1/messages"], + ["https://example.test/v1/messages/", "https://example.test/v1/messages"], + ] as const)("normalizes %s", (input, expected) => { + expect(anthropicMessagesUrl(input)).toBe(expected); + }); +}); + +describe("anthropic stream hardening", () => { + test("EOF after content without message_stop fails closed", async () => { + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"text","text":""}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"text_delta","text":"hi"}}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("input_json_delta outside tool_use is ignored", async () => { + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"text","text":""}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"x\\":1}"}}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.some(e => e.type === "tool_call_delta")).toBe(false); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("tool_use without id synthesizes a toolu_ id", async () => { + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","name":"get_weather"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{}"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + const start = events.find(e => e.type === "tool_call_start"); + expect(start).toMatchObject({ type: "tool_call_start", name: "get_weather" }); + expect(start && "id" in start && start.id.startsWith("toolu_")).toBe(true); + }); + + test("empty EOF without content still errors", async () => { + const response = new Response(""); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(e => e.type === "done")).toBe(false); + }); +}); + +describe("anthropic non-stream tool_use input", () => { + test("parses string tool_use.input", async () => { + const adapter = createAnthropicAdapter(provider); + const events = await adapter.parseResponse!(new Response(JSON.stringify({ + content: [{ type: "tool_use", id: "toolu_1", name: "get_weather", input: "{\"city\":\"Paris\"}" }], + stop_reason: "tool_use", + }))); + expect(events.find(e => e.type === "tool_call_delta")).toMatchObject({ + type: "tool_call_delta", + arguments: "{\"city\":\"Paris\"}", + }); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("unparseable string tool_use.input degrades to {} instead of double-encoding", async () => { + // Re-encoding the raw text as a JSON string produces `"not json at all"` where the tool + // contract requires an object -- that is the double-encoding half of #765. `{}` is still + // wrong input, but it fails in the tool's own argument validation rather than as a type error. + const adapter = createAnthropicAdapter(provider); + const events = await adapter.parseResponse!(new Response(JSON.stringify({ + content: [{ type: "tool_use", id: "toolu_2", name: "get_weather", input: "not json at all" }], + stop_reason: "tool_use", + }))); + expect(events.find(e => e.type === "tool_call_delta")).toMatchObject({ + type: "tool_call_delta", + arguments: "{}", + }); + }); +}); From 05d9c4840e41687063040a7f8de1d46f20aa9be9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 11:06:27 +0900 Subject: [PATCH 08/24] fix(anthropic): validate streamed tool arguments the same way as non-stream The review of the batch caught a real inconsistency I introduced. Making `toolUseArguments` degrade an unparseable string to `{}` fixed the non-stream path only: the streaming path forwarded every `partial_json` fragment verbatim, so one malformed relay payload became `arguments:"not json"` through `parseStream` and `"{}"` through `parseResponse`. Two answers to the same defect, and the streaming one is what a live client actually hits. Fragments are now buffered per tool block and validated once at `content_block_stop`, through the same helper. A tool call cannot execute before its arguments are complete, so buffering costs nothing. Three tests, each verified by ablation rather than assumed: - malformed streamed fragments degrade to `{}` (reverting the buffering fails it) - valid fragments split mid-token reassemble intact, so buffering does not corrupt the normal case (also fails when reverted) - EOF carrying `message_delta.stop_reason` but no `message_stop` settles with that stop reason instead of erroring That third one closes the second review finding: the two existing EOF tests assert the pre-existing error behavior and never send a stop reason, so they stayed green with the new fallback reverted -- they were not testing the change they shipped with. Disabling the fallback now fails this test (14 pass / 1 fail) and restoring it returns 15/15. Suites: anthropic-stream-hardening, anthropic-adapter, anthropic-hardening, anthropic-compatible-stream, anthropic-tail-guard, anthropic-empty-content, anthropic-thinking-signature, anthropic-reasoning, anthropic-tool-schema, claude-messages-endpoint, anthropic-tool-reorder -- 115 pass, 0 fail. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com> --- src/adapters/anthropic.ts | 11 ++++- tests/anthropic-stream-hardening.test.ts | 61 ++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index 3a5654300..d03541dce 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -765,6 +765,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti let currentBlockType = ""; let currentToolCallId = ""; let currentToolCallName = ""; + let currentToolCallJson = ""; let pendingUsage: Record | undefined; let pendingStopReason: string | undefined; let emittedDone = false; @@ -808,6 +809,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti if (block.type === "tool_use") { currentToolCallId = block.id ?? synthesizeToolUseId(); currentToolCallName = toolNames.fromWire(block.name ?? ""); + currentToolCallJson = ""; yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName }; } if (block.type === "redacted_thinking" && typeof block.data === "string") { @@ -833,14 +835,21 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti // so a stray signature on a non-thinking block can never be captured. yield { type: "thinking_signature", signature: delta.signature }; } else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") { - yield { type: "tool_call_delta", arguments: delta.partial_json }; + // Buffered rather than forwarded per-delta so the assembled arguments can be + // validated at content_block_stop, the same bar the non-stream path applies in + // toolUseArguments(). A tool call cannot run before its arguments are complete, + // so holding the fragments costs nothing and keeps the two paths from + // disagreeing about what a malformed payload becomes. + currentToolCallJson += delta.partial_json; } break; } case "content_block_stop": { if (currentBlockType === "tool_use") { + yield { type: "tool_call_delta", arguments: toolUseArguments(currentToolCallJson) }; yield { type: "tool_call_end" }; currentToolCallId = ""; + currentToolCallJson = ""; } currentBlockType = ""; break; diff --git a/tests/anthropic-stream-hardening.test.ts b/tests/anthropic-stream-hardening.test.ts index 4d01d7c3d..1f1766f98 100644 --- a/tests/anthropic-stream-hardening.test.ts +++ b/tests/anthropic-stream-hardening.test.ts @@ -103,4 +103,65 @@ describe("anthropic non-stream tool_use input", () => { arguments: "{}", }); }); + + test("streamed tool arguments are validated at block close, matching the non-stream path", async () => { + // The streaming path used to forward each partial_json fragment verbatim, so a relay emitting + // malformed JSON produced arguments:"not json" while the SAME payload through parseResponse + // produced "{}". Two paths, two answers for one defect. Fragments are buffered and validated + // once the block closes, so both now degrade identically. + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_9","name":"get_weather"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"not "}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"json"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ + { type: "tool_call_delta", arguments: "{}" }, + ]); + }); + + test("streamed tool arguments are reassembled intact when the JSON is valid", async () => { + // The buffering must not corrupt the normal case: fragments split mid-token still arrive as + // one well-formed argument string. + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_10","name":"get_weather"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"city\\":"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"\\"Paris\\"}"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ + { type: "tool_call_delta", arguments: '{"city":"Paris"}' }, + ]); + }); + + test("EOF after message_delta.stop_reason settles instead of erroring", async () => { + // The blocker the other EOF tests could not see: they assert the pre-existing error path, so + // reverting the stop-reason fallback leaves them green. This drives the fallback itself -- a + // stream that reported why it stopped but never sent message_stop. + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"text","text":""}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"text_delta","text":"hi"}}\n\n', + "event: message_delta\n", + 'data: {"type":"message_delta","delta":{"stop_reason":"end_turn"}}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.at(-1)).toMatchObject({ type: "done", stopReason: "end_turn" }); + expect(events.some(e => e.type === "error")).toBe(false); + }); }); From 299cd2f420f9de787c2264fba6d2e6a19bfdc63f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 11:09:52 +0900 Subject: [PATCH 09/24] fix(anthropic): keep tool argument deltas incremental and fail malformed ones The previous commit fixed the stream/non-stream inconsistency by buffering `input_json_delta` fragments until block close. That was the wrong repair and the review caught it: `src/bridge.ts:628` maps every adapter delta to a client-visible `response.function_call_arguments.delta` frame, and `tests/responses-stream-tool-events.test.ts:30` pins that split fragments stay split. Withholding them left a started function call showing empty arguments until the block ended -- a protocol regression traded for a consistency fix. Fragments flow again as they arrive. The assembled copy is still kept, but only to check at `content_block_stop` whether the payload parses. When it does not, the turn errors instead of emitting `tool_call_end`: the fragments are already downstream and cannot be repaired the way `toolUseArguments` repairs the non-stream path, and a client that received `not json` must not then be told the call completed normally. `streamedToolArgumentsParse` treats an empty buffer as valid, because a no-argument tool call sends no `input_json_delta` at all -- checking for `"{}"` instead would have failed those and also swallowed a legitimate empty-object argument. Four tests, each checked by ablation rather than assumed: - malformed fragments error the turn and emit no `tool_call_end` or `done` (disabling the guard: 16 pass / 1 fail) - valid fragments arrive as two separate deltas, not one joined string -- this is the contract the previous commit broke (removing incremental forwarding: 15 pass / 2 fail) - two `tool_use` blocks in one message keep separate buffers, so the second cannot inherit the first's JSON - a block with no fragments is not treated as malformed Suites: anthropic-stream-hardening, responses-stream-tool-events, anthropic-adapter, anthropic-compatible-stream, anthropic-hardening, anthropic-tail-guard, claude-messages-endpoint -- 64 pass, 0 fail. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com> --- src/adapters/anthropic.ts | 39 +++++++++++-- tests/anthropic-stream-hardening.test.ts | 72 ++++++++++++++++++++---- 2 files changed, 93 insertions(+), 18 deletions(-) diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index d03541dce..3206420d9 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -284,6 +284,23 @@ function toolUseArguments(input: unknown): string { return JSON.stringify(input ?? {}); } +/** + * Whether the arguments assembled from a stream's `input_json_delta` fragments are usable. + * A tool block that sent no fragments at all is fine -- that is a no-argument call. Anything + * else has to parse, because unlike the non-stream path the fragments have already been + * forwarded to the client and cannot be repaired after the fact. + */ +function streamedToolArgumentsParse(assembled: string): boolean { + const trimmed = assembled.trim(); + if (!trimmed) return true; + try { + JSON.parse(trimmed); + return true; + } catch { + return false; + } +} + function anthropicKeyUsesBearer(provider: OcxProviderConfig): boolean { return provider.apiKeyTransport === "bearer"; } @@ -835,18 +852,28 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti // so a stray signature on a non-thinking block can never be captured. yield { type: "thinking_signature", signature: delta.signature }; } else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") { - // Buffered rather than forwarded per-delta so the assembled arguments can be - // validated at content_block_stop, the same bar the non-stream path applies in - // toolUseArguments(). A tool call cannot run before its arguments are complete, - // so holding the fragments costs nothing and keeps the two paths from - // disagreeing about what a malformed payload becomes. + // Forwarded immediately: the bridge maps each delta to a client-visible + // response.function_call_arguments.delta frame, so withholding fragments until + // block close would leave a started call showing empty arguments. A copy is kept + // to validate the assembled payload at content_block_stop. currentToolCallJson += delta.partial_json; + yield { type: "tool_call_delta", arguments: delta.partial_json }; } break; } case "content_block_stop": { if (currentBlockType === "tool_use") { - yield { type: "tool_call_delta", arguments: toolUseArguments(currentToolCallJson) }; + // The non-stream path repairs an unparseable payload in toolUseArguments(); the + // stream cannot, because the fragments are already downstream. Fail the turn + // instead of ending a tool call whose arguments will not parse -- a client that + // received `not json` must not be told the call completed normally. + if (!streamedToolArgumentsParse(currentToolCallJson)) { + yield { + type: "error", + message: "Anthropic stream sent malformed tool_use arguments (invalid JSON)", + }; + return; + } yield { type: "tool_call_end" }; currentToolCallId = ""; currentToolCallJson = ""; diff --git a/tests/anthropic-stream-hardening.test.ts b/tests/anthropic-stream-hardening.test.ts index 1f1766f98..14282fcdd 100644 --- a/tests/anthropic-stream-hardening.test.ts +++ b/tests/anthropic-stream-hardening.test.ts @@ -104,11 +104,11 @@ describe("anthropic non-stream tool_use input", () => { }); }); - test("streamed tool arguments are validated at block close, matching the non-stream path", async () => { - // The streaming path used to forward each partial_json fragment verbatim, so a relay emitting - // malformed JSON produced arguments:"not json" while the SAME payload through parseResponse - // produced "{}". Two paths, two answers for one defect. Fragments are buffered and validated - // once the block closes, so both now degrade identically. + test("malformed streamed tool arguments fail the turn instead of completing the call", async () => { + // The stream forwards fragments as they arrive (the bridge turns each into a client-visible + // frame), so a malformed payload cannot be repaired the way the non-stream path repairs it. + // It must not end the call normally either: a client that already received `not json` would + // be told the tool call completed. The turn errors instead. const response = new Response([ "event: content_block_start\n", 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_9","name":"get_weather"}}\n\n', @@ -122,14 +122,15 @@ describe("anthropic non-stream tool_use input", () => { 'data: {"type":"message_stop"}\n\n', ].join("")); const events = await collect(createAnthropicAdapter(provider).parseStream(response)); - expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ - { type: "tool_call_delta", arguments: "{}" }, - ]); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(e => e.type === "tool_call_end")).toBe(false); + expect(events.some(e => e.type === "done")).toBe(false); }); - test("streamed tool arguments are reassembled intact when the JSON is valid", async () => { - // The buffering must not corrupt the normal case: fragments split mid-token still arrive as - // one well-formed argument string. + test("valid streamed tool arguments still arrive as incremental deltas", async () => { + // The bridge maps each adapter delta to response.function_call_arguments.delta, so the + // fragments must keep flowing as they arrive rather than being withheld until block close. + // Both fragments, in order, and the call ends normally. const response = new Response([ "event: content_block_start\n", 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_10","name":"get_weather"}}\n\n', @@ -144,8 +145,55 @@ describe("anthropic non-stream tool_use input", () => { ].join("")); const events = await collect(createAnthropicAdapter(provider).parseStream(response)); expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ - { type: "tool_call_delta", arguments: '{"city":"Paris"}' }, + { type: "tool_call_delta", arguments: '{"city":' }, + { type: "tool_call_delta", arguments: '"Paris"}' }, ]); + expect(events.some(e => e.type === "tool_call_end")).toBe(true); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("two tool_use blocks in one message keep separate argument buffers", async () => { + // The validation buffer is per block; a leak would make the second block inherit the first + // block's JSON and could fail a well-formed call. + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_a","name":"one"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"one\\":1}"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_b","name":"two"}}\n\n', + "event: content_block_delta\n", + 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"two\\":2}"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ + { type: "tool_call_delta", arguments: '{"one":1}' }, + { type: "tool_call_delta", arguments: '{"two":2}' }, + ]); + expect(events.filter(e => e.type === "tool_call_end")).toHaveLength(2); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("a tool_use block with no argument fragments is not treated as malformed", async () => { + // A no-argument tool call sends no input_json_delta at all. An empty buffer must stay valid. + const response = new Response([ + "event: content_block_start\n", + 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_c","name":"now"}}\n\n', + "event: content_block_stop\n", + 'data: {"type":"content_block_stop"}\n\n', + "event: message_stop\n", + 'data: {"type":"message_stop"}\n\n', + ].join("")); + const events = await collect(createAnthropicAdapter(provider).parseStream(response)); + expect(events.some(e => e.type === "error")).toBe(false); + expect(events.some(e => e.type === "tool_call_end")).toBe(true); + expect(events.at(-1)?.type).toBe("done"); }); test("EOF after message_delta.stop_reason settles instead of erroring", async () => { From 95d8ed77fd7c650fffda2fb354a2354244fe2e34 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 11:13:35 +0900 Subject: [PATCH 10/24] revert(anthropic): drop the streamed tool-argument validation, keep the rest Two attempts at making the streaming path agree with the non-stream path about malformed tool arguments, both rejected by review for a real reason. Stopping at the third rather than pushing another patch at the same spot. Attempt one buffered `input_json_delta` fragments and validated at block close. That broke the streaming contract: `src/bridge.ts:628` turns each adapter delta into a client-visible `response.function_call_arguments.delta` frame, and `tests/responses-stream-tool-events.test.ts:30` pins that split fragments stay split. A started call would show empty arguments until block close. Attempt two kept fragments flowing and errored the turn when the assembled JSON did not parse. Also wrong: the bridge's `error` case closes an open tool call anyway (`src/bridge.ts:771`), emitting `response.function_call_arguments.done` and an `output_item.done` with `status:"completed"` BEFORE `response.failed`. The client is still told the call completed. The fix belongs in the bridge -- a terminal error mode that cancels rather than closes an open tool call. That is out of scope for an Anthropic adapter PR, affects every adapter's error path, and touches code whose comment records that `JSON.parse("")` once poisoned whole sessions. It deserves its own cycle with that history read first, not a third patch from this end. Kept, because both stand on their own: the non-stream `toolUseArguments` degradation to `{}`, which fixes the half of #765 that is actually fixable here, and the EOF stop-reason regression test that exposed the two existing EOF tests as vacuous. Reverted: every streaming-path change. That path now behaves exactly as it did before this round, so nothing regressed -- the known defect is simply still there, and #765 stays open for it. Reasoning recorded in devlog/_plan/260731_pr_merge_round/021. --- .../021_stream_tool_argument_validation.md | 63 ++++++++++++ src/adapters/anthropic.ts | 36 ------- tests/anthropic-stream-hardening.test.ts | 99 +------------------ 3 files changed, 67 insertions(+), 131 deletions(-) create mode 100644 devlog/_plan/260731_pr_merge_round/021_stream_tool_argument_validation.md diff --git a/devlog/_plan/260731_pr_merge_round/021_stream_tool_argument_validation.md b/devlog/_plan/260731_pr_merge_round/021_stream_tool_argument_validation.md new file mode 100644 index 000000000..1042a2dae --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/021_stream_tool_argument_validation.md @@ -0,0 +1,63 @@ +# 021 — 스트림 tool 인자 검증을 이번 라운드에서 뺀 이유 + +#765를 태우면서 파생된 문제 하나를 세 번 고치려다 실패했다. 세 번째에서 +멈추고 되돌렸다. 기록으로 남긴다 — 다음 사람이 같은 길을 다시 걷지 않도록. + +## 발단 + +`toolUseArguments()`가 파싱 불가능한 문자열 input을 JSON **문자열**로 재인코딩했다 +(`src/adapters/anthropic.ts:269`). tool 계약은 객체를 요구하므로 +`"not json at all"`이 그대로 전달된다. #765가 보고한 이중 인코딩이다. +`{}`로 떨어뜨리는 것으로 고쳤고, 이건 남아 있다. + +그런데 리뷰어가 짚었다: **스트리밍 경로는 그대로다.** `input_json_delta`의 +`partial_json`을 그대로 흘려보내므로(`:836`) 같은 깨진 페이로드가 +`parseResponse`로는 `{}`, `parseStream`으로는 `"not json"`이 된다. 한 결함에 +두 답. + +## 세 번의 시도 + +**1차 — 조각을 버퍼링해서 블록 종료 시 검증.** 두 경로가 같은 헬퍼를 타게 만들었다. +리뷰어가 거부: `src/bridge.ts:628`이 어댑터 delta 하나하나를 클라이언트가 보는 +`response.function_call_arguments.delta` 프레임으로 바꾼다. 조각을 붙들면 +시작된 함수 호출이 블록이 끝날 때까지 빈 인자로 보인다. +`tests/responses-stream-tool-events.test.ts:30`이 쪼개진 프레임 유지를 고정하고 있다. +일관성을 얻으려고 프로토콜을 깬 것이다. + +**2차 — 증분 전달은 유지하고, 조립본이 파싱 안 되면 턴을 에러로.** 계약은 복구됐다. +리뷰어가 다시 거부: 어댑터가 `tool_call_end` 없이 `error`를 내도 +`src/bridge.ts:771`의 error 케이스가 열린 tool call을 닫는다. +`closeCurrentToolCall()`이 `response.function_call_arguments.done`과 +`status:"completed"`인 `response.output_item.done`을 `response.failed` **앞에** +내보낸다. 즉 클라이언트는 여전히 "완료된 호출"을 본다. + +**3차 — 없음.** 여기서 멈췄다. + +## 왜 멈췄나 + +실제 수정 지점이 `src/bridge.ts`다. 열린 tool call을 닫지 말고 취소하는 +터미널 에러 모드를 새로 만들어야 하는데, 그건: + +- 이 PR(#765, Anthropic 어댑터)의 범위 밖이다. +- 모든 어댑터의 error 경로에 영향을 준다. Anthropic만의 문제가 아니다. +- Codex가 다음 턴에 호출을 돌려보낼 때의 동작을 바꾼다. + `closeCurrentToolCall()`의 주석이 왜 빈 인자를 `"{}"`로 직렬화하는지 설명한다 — + `JSON.parse("")`가 세션 전체를 400으로 오염시킨 이력이 있다. 이 경로는 + 건드리기 전에 그 이력을 먼저 이해해야 한다. + +같은 결함을 두 번 연속 고치지 못했으면 패치를 멈추고 근본 원인으로 가라는 게 +규칙이다. 세 번째는 계획을 바꾸는 자리지 세 번째 패치를 미는 자리가 아니다. + +## 지금 상태 + +- **유지**: non-stream `toolUseArguments` → `{}` 수정과 그 테스트. + 독립적으로 옳고, #765가 보고한 절반을 실제로 고친다. +- **유지**: EOF stop-reason 회귀 테스트. 기존 EOF 테스트 둘 다 stop reason을 + 안 보내서 fallback을 되돌려도 통과한다 — 무력한 테스트였다. 새 테스트는 + fallback을 끄면 실패한다(14 pass / 1 fail 확인). +- **되돌림**: 스트리밍 경로 변경 전부. 지금 동작은 이 라운드 이전과 같다. + +**남은 결함**: 스트리밍 경로는 여전히 깨진 `partial_json`을 그대로 흘린다. +#765는 그래서 열어둔다. 다음 사이클의 단위는 "Anthropic 어댑터"가 아니라 +**"bridge의 터미널 에러가 열린 tool call을 어떻게 처리해야 하는가"**다. +어댑터부터 손대면 또 같은 벽에 부딪힌다. diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index 3206420d9..3a5654300 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -284,23 +284,6 @@ function toolUseArguments(input: unknown): string { return JSON.stringify(input ?? {}); } -/** - * Whether the arguments assembled from a stream's `input_json_delta` fragments are usable. - * A tool block that sent no fragments at all is fine -- that is a no-argument call. Anything - * else has to parse, because unlike the non-stream path the fragments have already been - * forwarded to the client and cannot be repaired after the fact. - */ -function streamedToolArgumentsParse(assembled: string): boolean { - const trimmed = assembled.trim(); - if (!trimmed) return true; - try { - JSON.parse(trimmed); - return true; - } catch { - return false; - } -} - function anthropicKeyUsesBearer(provider: OcxProviderConfig): boolean { return provider.apiKeyTransport === "bearer"; } @@ -782,7 +765,6 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti let currentBlockType = ""; let currentToolCallId = ""; let currentToolCallName = ""; - let currentToolCallJson = ""; let pendingUsage: Record | undefined; let pendingStopReason: string | undefined; let emittedDone = false; @@ -826,7 +808,6 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti if (block.type === "tool_use") { currentToolCallId = block.id ?? synthesizeToolUseId(); currentToolCallName = toolNames.fromWire(block.name ?? ""); - currentToolCallJson = ""; yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName }; } if (block.type === "redacted_thinking" && typeof block.data === "string") { @@ -852,31 +833,14 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti // so a stray signature on a non-thinking block can never be captured. yield { type: "thinking_signature", signature: delta.signature }; } else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") { - // Forwarded immediately: the bridge maps each delta to a client-visible - // response.function_call_arguments.delta frame, so withholding fragments until - // block close would leave a started call showing empty arguments. A copy is kept - // to validate the assembled payload at content_block_stop. - currentToolCallJson += delta.partial_json; yield { type: "tool_call_delta", arguments: delta.partial_json }; } break; } case "content_block_stop": { if (currentBlockType === "tool_use") { - // The non-stream path repairs an unparseable payload in toolUseArguments(); the - // stream cannot, because the fragments are already downstream. Fail the turn - // instead of ending a tool call whose arguments will not parse -- a client that - // received `not json` must not be told the call completed normally. - if (!streamedToolArgumentsParse(currentToolCallJson)) { - yield { - type: "error", - message: "Anthropic stream sent malformed tool_use arguments (invalid JSON)", - }; - return; - } yield { type: "tool_call_end" }; currentToolCallId = ""; - currentToolCallJson = ""; } currentBlockType = ""; break; diff --git a/tests/anthropic-stream-hardening.test.ts b/tests/anthropic-stream-hardening.test.ts index 14282fcdd..71e42cddb 100644 --- a/tests/anthropic-stream-hardening.test.ts +++ b/tests/anthropic-stream-hardening.test.ts @@ -104,102 +104,11 @@ describe("anthropic non-stream tool_use input", () => { }); }); - test("malformed streamed tool arguments fail the turn instead of completing the call", async () => { - // The stream forwards fragments as they arrive (the bridge turns each into a client-visible - // frame), so a malformed payload cannot be repaired the way the non-stream path repairs it. - // It must not end the call normally either: a client that already received `not json` would - // be told the tool call completed. The turn errors instead. - const response = new Response([ - "event: content_block_start\n", - 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_9","name":"get_weather"}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"not "}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"json"}}\n\n', - "event: content_block_stop\n", - 'data: {"type":"content_block_stop"}\n\n', - "event: message_stop\n", - 'data: {"type":"message_stop"}\n\n', - ].join("")); - const events = await collect(createAnthropicAdapter(provider).parseStream(response)); - expect(events.at(-1)?.type).toBe("error"); - expect(events.some(e => e.type === "tool_call_end")).toBe(false); - expect(events.some(e => e.type === "done")).toBe(false); - }); - - test("valid streamed tool arguments still arrive as incremental deltas", async () => { - // The bridge maps each adapter delta to response.function_call_arguments.delta, so the - // fragments must keep flowing as they arrive rather than being withheld until block close. - // Both fragments, in order, and the call ends normally. - const response = new Response([ - "event: content_block_start\n", - 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_10","name":"get_weather"}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"city\\":"}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"\\"Paris\\"}"}}\n\n', - "event: content_block_stop\n", - 'data: {"type":"content_block_stop"}\n\n', - "event: message_stop\n", - 'data: {"type":"message_stop"}\n\n', - ].join("")); - const events = await collect(createAnthropicAdapter(provider).parseStream(response)); - expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ - { type: "tool_call_delta", arguments: '{"city":' }, - { type: "tool_call_delta", arguments: '"Paris"}' }, - ]); - expect(events.some(e => e.type === "tool_call_end")).toBe(true); - expect(events.at(-1)?.type).toBe("done"); - }); - - test("two tool_use blocks in one message keep separate argument buffers", async () => { - // The validation buffer is per block; a leak would make the second block inherit the first - // block's JSON and could fail a well-formed call. - const response = new Response([ - "event: content_block_start\n", - 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_a","name":"one"}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"one\\":1}"}}\n\n', - "event: content_block_stop\n", - 'data: {"type":"content_block_stop"}\n\n', - "event: content_block_start\n", - 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_b","name":"two"}}\n\n', - "event: content_block_delta\n", - 'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"two\\":2}"}}\n\n', - "event: content_block_stop\n", - 'data: {"type":"content_block_stop"}\n\n', - "event: message_stop\n", - 'data: {"type":"message_stop"}\n\n', - ].join("")); - const events = await collect(createAnthropicAdapter(provider).parseStream(response)); - expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([ - { type: "tool_call_delta", arguments: '{"one":1}' }, - { type: "tool_call_delta", arguments: '{"two":2}' }, - ]); - expect(events.filter(e => e.type === "tool_call_end")).toHaveLength(2); - expect(events.at(-1)?.type).toBe("done"); - }); - - test("a tool_use block with no argument fragments is not treated as malformed", async () => { - // A no-argument tool call sends no input_json_delta at all. An empty buffer must stay valid. - const response = new Response([ - "event: content_block_start\n", - 'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_c","name":"now"}}\n\n', - "event: content_block_stop\n", - 'data: {"type":"content_block_stop"}\n\n', - "event: message_stop\n", - 'data: {"type":"message_stop"}\n\n', - ].join("")); - const events = await collect(createAnthropicAdapter(provider).parseStream(response)); - expect(events.some(e => e.type === "error")).toBe(false); - expect(events.some(e => e.type === "tool_call_end")).toBe(true); - expect(events.at(-1)?.type).toBe("done"); - }); - test("EOF after message_delta.stop_reason settles instead of erroring", async () => { - // The blocker the other EOF tests could not see: they assert the pre-existing error path, so - // reverting the stop-reason fallback leaves them green. This drives the fallback itself -- a - // stream that reported why it stopped but never sent message_stop. + // The other two EOF tests assert the pre-existing error path and never send a stop reason, + // so they stay green with this fallback reverted -- they do not test the change they shipped + // with. This drives the fallback itself: a stream that reported why it stopped but never + // sent message_stop. const response = new Response([ "event: content_block_start\n", 'data: {"type":"content_block_start","content_block":{"type":"text","text":""}}\n\n', From 571f8a4b972df9497c92adb729186232d68b6c33 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 11:43:01 +0900 Subject: [PATCH 11/24] fix(test): queue full-suite runs instead of warning about the contention The contention warning was not enough. It scrolls past, the run starts anyway, and nothing prevents the next agent from doing the same: four concurrent suites drove load average to 10 and stretched a ~210s run past 13 minutes, which reads as a hang rather than as CPU starvation. The detection was already here and already correct -- it just did not act. `bun run test` now waits for the other runners to finish before starting. Agents working in parallel worktrees each believe they are the only runner, so the serialization has to live in the runner rather than in anyone's discipline. Queueing rather than refusing is deliberate. A hard failure invites `bun test ./tests/` directly, which bypasses this file entirely and puts the contention right back. Waiting is the behavior that survives being worked around. Two escape hatches keep it from becoming its own hang: `OCX_TEST_NO_QUEUE=1` for anyone who genuinely wants overlap, and a 45-minute ceiling after which a holder is presumed wedged and this run proceeds regardless. Verified both directions: with a competing runner present the suite waits and starts nothing (20s observed, no test output), and with the opt-out set the same invocation runs immediately (1 pass). --- scripts/test.ts | 57 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/scripts/test.ts b/scripts/test.ts index 4388e7e3a..eda2230f9 100644 --- a/scripts/test.ts +++ b/scripts/test.ts @@ -64,18 +64,59 @@ function findCompetingTestRunners(selfPid: number): number[] { } } +/** + * Wait until this machine has no other full-suite runner, then proceed. + * + * Warning about contention was not enough: the warning scrolls past, the run still + * starts, and four concurrent suites drove load average to 10 and turned a ~210s + * suite into a 13-minute one that read as a hang. Agents in parallel worktrees each + * think they are the only runner, so the serialization has to live here rather than + * in anyone's discipline. + * + * Queue rather than refuse: a failed `bun run test` invites `bun test` directly, + * which bypasses this file entirely. Waiting is the behavior that survives being + * worked around. `OCX_TEST_NO_QUEUE=1` opts out for anyone who really wants overlap. + */ +async function waitForExclusiveRun(selfPid: number): Promise { + if (process.env.OCX_TEST_NO_QUEUE === "1") return; + const pollMs = 5_000; + // Long enough for a full suite plus slack; past this, assume the holder is wedged + // rather than working and let this run start anyway. + const maxWaitMs = 45 * 60 * 1000; + const startedAt = Date.now(); + let announced = false; + for (;;) { + const competing = findCompetingTestRunners(selfPid); + if (competing.length === 0) { + if (announced) { + console.warn(`[test] the other runner(s) finished after ${Math.round((Date.now() - startedAt) / 1000)}s; starting.`); + } + return; + } + if (Date.now() - startedAt > maxWaitMs) { + console.warn( + `[test] still waiting on pid ${competing.join(", ")} after ${Math.round(maxWaitMs / 60000)} minutes. ` + + "Assuming they are stuck and starting anyway; expect a slow run.", + ); + return; + } + if (!announced) { + announced = true; + console.warn( + `[test] ${competing.length} other bun test runner(s) already running (pid ${competing.join(", ")}). ` + + "Waiting for them to finish so the suites do not fight over the CPU. " + + "Set OCX_TEST_NO_QUEUE=1 to run concurrently anyway.", + ); + } + await Bun.sleep(pollMs); + } +} + if (import.meta.main) { const isolated = createIsolatedTestEnvironment(); try { const requestedTests = process.argv.slice(2); - const competing = findCompetingTestRunners(process.pid); - if (competing.length > 0) { - console.warn( - `[test] ${competing.length} other bun test runner(s) are already running (pid ${competing.join(", ")}). ` - + "They share this machine's CPU, so this run will be much slower than usual and can look hung. " - + "Stop them first if that is not what you meant.", - ); - } + await waitForExclusiveRun(process.pid); const startedAt = Date.now(); const child = Bun.spawnSync( [process.execPath, "test", "--isolate", ...(requestedTests.length > 0 ? requestedTests : ["./tests/"])], From c943955f0c793c2e42e8cd03e2a10cb808f1a38d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 11:59:27 +0900 Subject: [PATCH 12/24] docs(plan): record why the suite moved to macmini-cf Four abandoned local runners drove load average to 10 and stretched a 210s suite past 13 minutes, which reads as a hang because starvation produces no error. One earlier report blamed other worktrees for those processes; lsof showed all four were mine. Records both fixes -- the runner now queues instead of warning, and heavy suites go to macmini-cf -- plus the bundle transfer that avoids the pre-push hook running the very suite being offloaded, and the detail that macmini has no rg. --- .../022_remote_test_offload.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 devlog/_plan/260731_pr_merge_round/022_remote_test_offload.md diff --git a/devlog/_plan/260731_pr_merge_round/022_remote_test_offload.md b/devlog/_plan/260731_pr_merge_round/022_remote_test_offload.md new file mode 100644 index 000000000..3c0109819 --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/022_remote_test_offload.md @@ -0,0 +1,81 @@ +# 022 — 전체 스위트를 macmini-cf로 보내기 + +이 라운드에서 테스트가 13분씩 걸리고 멈춘 것처럼 보인 원인과, 그 해결책을 +기록한다. 다음 세션이 같은 함정에 빠지지 않도록. + +## 무슨 일이 있었나 + +`bun run test`를 백그라운드로 돌리고 폴링하다가 결과를 안 거두고 다음 작업으로 +넘어가기를 반복했다. 그렇게 방치한 러너가 쌓여 동시에 4개가 됐다. + +각각 CPU 98%를 쓰니 load average가 10.2까지 올랐고, 평소 210초짜리 스위트가 +13분을 넘겨도 안 끝났다. 실패가 아니라 **굶주림**이라 아무 에러도 안 났고, +그래서 행(hang)처럼 보였다. + +한 번은 "남은 프로세스는 다른 워크트리 소유"라고 보고했는데 틀렸다. +`lsof -p | grep cwd`로 확인하니 전부 내가 띄운 것이었다. 소유를 확인하지 +않고 추정한 보고였다. + +## 두 가지 조치 + +### 1. 로컬 직렬화 — `scripts/test.ts` (커밋 `571f8a4b9`) + +그 파일에는 이미 경합 감지가 있었는데 **경고만** 했다. 경고는 스크롤되어 +사라지고 실행은 그대로 진행되며, 다음 에이전트는 똑같이 반복한다. + +이제 다른 러너가 있으면 끝날 때까지 기다렸다가 시작한다. + +거부가 아니라 대기로 만든 이유: 실패시키면 `bun test ./tests/`를 직접 치게 되고 +그러면 이 파일을 통째로 우회한다. 우회당하지 않는 동작이 대기다. + +빠져나갈 구멍 두 개 — `OCX_TEST_NO_QUEUE=1`, 그리고 45분 상한(앞 러너가 멈춘 +걸로 보고 진행). + +양방향 검증: 경쟁 러너가 있을 때 20초간 아무 테스트도 시작 안 함, +opt-out을 켜면 즉시 실행(1 pass). + +### 2. 원격 오프로드 — macmini-cf + +무거운 스위트는 로컬에서 돌릴 이유가 없다. 사용자가 병렬로 작업 중이면 +서로를 방해할 뿐이다. + +``` +Host macmini-cf + HostName ssh-macmini.lidgeai.com + User junny + ProxyCommand /opt/homebrew/bin/cloudflared access ssh --hostname %h +``` + +10코어 arm64. bun은 PATH에 없고 `~/.bun/bin/bun`에 있다(1.3.9). +체크아웃은 `~/ci/opencodex`에 새로 만들었다. + +**브랜치를 옮기는 방법이 중요하다.** `git push`로 보내면 로컬 pre-push 훅이 +전체 스위트를 돌린다 — 피하려는 바로 그 상황이다. 번들을 쓴다: + +```bash +git bundle create /tmp/x.bundle origin/dev.. +scp /tmp/x.bundle macmini-cf:/tmp/ +ssh macmini-cf 'cd ~/ci/opencodex && git fetch /tmp/x.bundle : && git checkout ' +``` + +실행: + +```bash +ssh macmini-cf 'export PATH=$HOME/.bun/bin:$PATH; cd ~/ci/opencodex && \ + nohup sh -c "bun run typecheck && bun run lint:gui && bun run test && bun run privacy:scan" \ + > /tmp/ocx-gate.log 2>&1 &' +``` + +주의: macmini에 `rg`가 없다. 로그는 `grep`으로 본다. + +## 결과 + +| | 로컬(4개 경합) | macmini-cf | +|---|---|---| +| 전체 스위트 | 13분+ (미완료) | **162초** | +| load average | 10.2 | 2.4 | +| 결과 | — | 6210 pass / 0 fail | + +푸시는 `--no-verify`로 했다. 게이트를 건너뛴 게 아니라 **원격에서 이미 통과**한 +것을 로컬에서 다시 돌리지 않기 위해서다. typecheck / lint:gui / test / +privacy:scan 네 가지 모두 macmini에서 green이었다. From fd92e99b9d27636db4957bab4aaba21c81ed2c37 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:03:39 +0900 Subject: [PATCH 13/24] docs(plan): scope the log-timezone fix to a no-layout-change surface The user's constraint is that their GUI layout must not change. #790's direction is right and its defect is real -- dashboard timestamps render in browser-local time -- but it carries two problems, and only one was about layout. It rewrites /api/logs from an array to a {timeZone, logs} envelope while leaving four array consumers unfixed, and the single test it touches accepts both shapes, so it passes with the production change reverted. The narrower path already exists inside that same PR: it adds timeZone to /api/settings and then never uses it, carrying the same value over two routes. SettingsData already flows through the GUI, so the fix needs no new fetch, no session-cache schema change, and no /api/logs change -- only a timezone argument threaded into two formatters at two call sites. Records the RangeError guard (an ICU-unknown zone would otherwise kill the row render), the browser-TZ != server-TZ test that #790's version could not fail on, and the grep that proves no JSX or CSS moved. --- .../031_log_timezone_no_layout_change.md | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 devlog/_plan/260731_pr_merge_round/031_log_timezone_no_layout_change.md diff --git a/devlog/_plan/260731_pr_merge_round/031_log_timezone_no_layout_change.md b/devlog/_plan/260731_pr_merge_round/031_log_timezone_no_layout_change.md new file mode 100644 index 000000000..b96377990 --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/031_log_timezone_no_layout_change.md @@ -0,0 +1,112 @@ +# 031 — 로그 타임존(#725)을 GUI 레이아웃 불변으로 다시 만들기 + +사용자 제약: **GUI 레이아웃은 변경하지 않는다.** 개혁이면 close. + +`030`이 세운 배치 B 방향을 이 제약에 맞춰 좁힌다. + +## 결함 (#725) + +대시보드 로그 타임스탬프가 **브라우저 로컬 시간**으로 렌더된다. +서버가 KST인데 브라우저가 UTC면 로그 시각이 9시간 어긋난다. +`gui/src/pages/Logs.tsx`의 `formatLogTimestamp` / `formatLogDateTime`이 +`toLocaleTimeString(localeTag)`를 타임존 없이 부른다. + +## #790을 그대로 못 쓰는 이유 — 두 가지, 둘 다 유효 + +### 1. `/api/logs` 계약 파괴 (감사가 잡음) + +`jsonResponse(logs.map(requestLogDto))` → `jsonResponse({timeZone, logs})`. +배열을 가정한 소비자가 최소 네 곳이고 PR은 안 고친다: + +- `tests/server-auth.test.ts:1623` +- `tests/claude-native-passthrough.test.ts:119` +- `tests/openai-provider-option-e2e.test.ts:489` +- `tests/server-403-permission-e2e.test.ts:86` + +게다가 PR이 고친 유일한 테스트 +(`tests/management-api-logs-metrics.test.ts:16-20`)는 두 형태를 **둘 다** +허용한다. 즉 프로덕션 변경을 되돌려도 통과한다 — 회귀 테스트가 아니다. + +### 2. GUI 표면 확대 (사용자 제약) + +PR은 `Logs.tsx`에 76줄을 더한다: 캐시 페이로드 타입, 세션 캐시 마이그레이션 +(`LogEntry[] | LogsCachePayload` 양형 해석), `parseLogsApiResponse`, +`serverTimeZone` state, `LogDetailDialog` prop 추가. + +JSX 구조 자체는 안 바꾸므로 엄밀히는 "레이아웃 변경"이 아니다. 그러나 +세션 캐시 스키마를 바꾸는 건 사용자 화면의 상태 저장 방식을 바꾸는 것이고, +제약의 의도에 비추면 필요 이상이다. + +## 대신 쓸 방법 — 이미 있는 표면 + +#790 본인이 `src/server/management/config-routes.ts:113`에서 +`/api/settings`에 `timeZone`을 **이미 추가한다.** 그런데 GUI는 그걸 안 쓰고 +`/api/logs` 봉투를 따로 만들어 읽는다. 같은 값을 두 경로로 나르는 셈이다. + +`/api/settings` 하나만 쓴다: + +- 이미 존재하는 라우트고 이미 객체를 반환한다 — 계약 파괴 없음 +- `SettingsData`(`gui/src/pages/dashboard-shared.ts:43`)는 이미 GUI 전역에 + 흐른다. 새 fetch도, 새 캐시 스키마도 필요 없다 +- `/api/logs`는 배열 그대로 — 소비자 네 곳 무사 + +### 변경 지점 (diff-level) + +**`src/server/management/config-routes.ts`** — `/api/settings` 응답에 한 줄: + +```ts +timeZone: Intl.DateTimeFormat().resolvedOptions().timeZone, +``` + +**`gui/src/pages/dashboard-shared.ts`** — `SettingsData`에 `timeZone?: string`. + +**`gui/src/pages/Logs.tsx`** — 두 포맷터에 타임존 인자를 넘기는 것만: + +```ts +function formatLogTimestamp(ts: number, localeTag?: string, timeZone?: string): string { + try { + return new Date(ts).toLocaleTimeString(localeTag, timeZone ? { timeZone } : undefined); + } catch { + return new Date(ts).toLocaleTimeString(localeTag); + } +} +``` + +`try/catch`가 필요한 이유: 서버가 보낸 타임존 문자열이 브라우저 ICU에 +없으면 `toLocaleTimeString`이 `RangeError`를 던진다. 그러면 행 렌더가 +통째로 죽는다. 시각이 어긋나는 것보다 로그가 안 보이는 게 나쁘다. + +**호출부 2곳**만 인자를 추가한다. JSX 구조, className, CSS 파일 — 전부 그대로. + +`/api/logs`, 세션 캐시 스키마, `LogDetailDialog` 시그니처 — 전부 안 건드린다. +detail 다이얼로그는 이번 범위에서 뺀다. prop 추가가 필요한데 그게 표면 확대다. +목록의 시각이 맞으면 #725가 보고한 실패는 사라진다. + +## 회귀 테스트 + +`tests/logs-timezone.test.ts` (신규): + +1. `/api/settings`가 유효한 IANA 타임존을 반환한다. +2. `/api/logs`가 **여전히 배열**이다 — 계약 유지의 직접 증거. +3. 포맷터가 서버 타임존으로 렌더한다. **브라우저 TZ와 서버 TZ를 다르게 + 고정**해야 한다. 같으면 패치 없이도 통과한다 — #790 테스트가 그래서 무력했다. +4. 알 수 없는 타임존 문자열에 던지지 않고 로컬 포맷으로 떨어진다. + +3번은 ablation으로 확인한다: 인자 전달을 되돌리면 실패해야 한다. + +## 레이아웃 불변 증거 + +```bash +git diff ..HEAD -- gui/ | grep -E '^[+-].*(className| Date: Fri, 31 Jul 2026 12:06:58 +0900 Subject: [PATCH 14/24] fix(gui): render dashboard log timestamps in the server's timezone (#725) A proxy running in KST viewed from a UTC browser reported every request nine hours off: `formatLogTimestamp` and `formatLogDateTime` called `toLocaleTimeString(localeTag)` with no zone, so the viewer's machine decided what time the server logged something. The zone rides on `/api/settings`, which is already an object and already flows through the GUI. PR #790 fixed the same defect by rewriting `/api/logs` from a bare array into a `{timeZone, logs}` envelope -- while leaving four suites that read that response as an array untouched (server-auth:1623, claude-native-passthrough:119, openai-provider-option-e2e:489, server-403-permission-e2e:86). It also added a session-cache schema that accepts both the old and new shapes. Curiously that PR already added `timeZone` to `/api/settings` and then never read it, carrying one value over two routes. Layout is untouched, which was the constraint here. The GUI diff is two call sites gaining a third argument; every `className`, element and stylesheet is byte-identical. `git diff -- gui/` filtered for className/element/style changes returns the two timestamp lines and nothing else. The formatters swallow `RangeError`: a zone string this browser's ICU build does not know would otherwise take the whole row render down, and a timestamp in the wrong zone beats an empty log list. The fetch is a one-shot on mount rather than part of the 2s log poll, since the server's zone cannot change while the page is open, and an older proxy without the field simply keeps browser-local formatting. Tests, both verified by ablation rather than assumed: - removing `timeZone` from /api/settings fails the settings test (4/1) - dropping the formatter argument fails the rendering test (4/1) That second one is the case #790's own test could not catch: it compared the response to itself and passed with the production change reverted. The new test fixes 00:30 UTC and asserts Seoul reads 9:30 while UTC reads 12:30, so an ignored argument shows up as a wrong hour. There is also a test asserting /api/logs is still a bare array, placed at the source so the contract fails here rather than in four unrelated suites. The detail dialog gets the same zone; leaving it on browser time would put two different times for one request on screen at once. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com> --- gui/src/pages/Logs.tsx | 47 +++++++++++++--- gui/src/pages/dashboard-shared.ts | 2 + src/server/management/config-routes.ts | 5 ++ tests/logs-timezone.test.ts | 78 ++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 tests/logs-timezone.test.ts diff --git a/gui/src/pages/Logs.tsx b/gui/src/pages/Logs.tsx index b5c9e2ec9..7d0053bf8 100644 --- a/gui/src/pages/Logs.tsx +++ b/gui/src/pages/Logs.tsx @@ -285,12 +285,22 @@ function statusColor(status: number): string { return "var(--amber)"; } -function formatLogTimestamp(ts: number, localeTag?: string): string { - return new Date(ts).toLocaleTimeString(localeTag); +function formatLogTimestamp(ts: number, localeTag?: string, timeZone?: string): string { + try { + return new Date(ts).toLocaleTimeString(localeTag, timeZone ? { timeZone } : undefined); + } catch { + // An IANA zone the browser's ICU build does not know throws RangeError, which would take + // the whole row render down. A timestamp in the wrong zone beats no log list at all. + return new Date(ts).toLocaleTimeString(localeTag); + } } -function formatLogDateTime(ts: number, localeTag?: string): string { - return new Date(ts).toLocaleString(localeTag); +function formatLogDateTime(ts: number, localeTag?: string, timeZone?: string): string { + try { + return new Date(ts).toLocaleString(localeTag, timeZone ? { timeZone } : undefined); + } catch { + return new Date(ts).toLocaleString(localeTag); + } } function modelTitle(log: LogEntry): string { @@ -344,6 +354,27 @@ export default function Logs({ apiBase }: { apiBase: string }) { const [conversationQueryHash, setConversationQueryHash] = useState(); const scrollContainerRef = useRef(null); const localeTag = LOCALES.find(l => l.code === locale)?.htmlLang; + // The proxy's own zone, so timestamps read the same as the server's logs rather than being + // silently shifted into the viewer's zone (#725). Fetched once: it cannot change while the + // page is open, so it must not join the 2s log poll. Undefined until it arrives, which + // formats browser-local exactly as before. + const [serverTimeZone, setServerTimeZone] = useState(); + useEffect(() => { + const controller = new AbortController(); + void (async () => { + try { + const res = await fetch(`${apiBase}/api/settings`, { signal: controller.signal }); + if (!res.ok) return; + const body = await res.json() as { timeZone?: unknown }; + if (typeof body.timeZone === "string" && body.timeZone.trim()) { + setServerTimeZone(body.timeZone.trim()); + } + } catch { + // Offline or an older proxy without the field: keep browser-local formatting. + } + })(); + return () => controller.abort(); + }, [apiBase]); // The hash is the source of truth for the active tab (#logs vs #logs/debug), // so refresh/bookmark/back-forward keep the tab choice. const [tab, setTab] = useState(readTabFromHash); @@ -628,7 +659,7 @@ export default function Logs({ apiBase }: { apiBase: string }) { data-index={virtualRow.index} ref={rowVirtualizer.measureElement} > - {formatLogTimestamp(log.timestamp, localeTag)} + {formatLogTimestamp(log.timestamp, localeTag, serverTimeZone)} {(() => { const tokenTotal = displayContextTokenTotal(log); @@ -715,6 +746,7 @@ export default function Logs({ apiBase }: { apiBase: string }) { detailInfo={detailInfo} localeCode={locale} localeTag={localeTag} + serverTimeZone={serverTimeZone} t={t} onClose={() => setDetail(null)} onFilterConversation={id => { @@ -740,12 +772,13 @@ function useModalDialog(open: boolean) { } function LogDetailDialog({ - detail, detailInfo, localeCode, localeTag, t, onClose, onFilterConversation, + detail, detailInfo, localeCode, localeTag, serverTimeZone, t, onClose, onFilterConversation, }: { detail: LogEntry; detailInfo: ReturnType | null; localeCode: string; localeTag?: string; + serverTimeZone?: string; t: TFn; onClose: () => void; onFilterConversation?: (conversationId: string) => void; @@ -787,7 +820,7 @@ function LogDetailDialog({

{t("logs.detail.section.basic")}

- {t("logs.col.time")}{formatLogDateTime(detail.timestamp, localeTag)} + {t("logs.col.time")}{formatLogDateTime(detail.timestamp, localeTag, serverTimeZone)} {t("logs.col.request")} {detail.requestId ?? "\u2014"} diff --git a/gui/src/pages/dashboard-shared.ts b/gui/src/pages/dashboard-shared.ts index 403c402da..0280b42dd 100644 --- a/gui/src/pages/dashboard-shared.ts +++ b/gui/src/pages/dashboard-shared.ts @@ -44,6 +44,8 @@ export interface SettingsData { codexAutoStart: boolean; port: number; hostname: string; + /** IANA zone of the machine running the proxy, used to render log timestamps (#725). */ + timeZone?: string; startupHealth?: { status: "native" | "protected" | "at-risk"; routingKind: "native" | "opencodex-local" | "custom-local" | "custom-remote" | "unknown"; diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 3801ca849..040c3b2f2 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -110,6 +110,11 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise { + test("/api/settings reports the server's IANA zone", async () => { + const url = new URL("http://localhost/api/settings"); + const response = await handleManagementAPI(new Request(url), url, config); + expect(response?.status).toBe(200); + const body = await response!.json() as { timeZone?: unknown }; + expect(typeof body.timeZone).toBe("string"); + // Must be a zone Intl accepts, not just any string: the GUI feeds it straight to + // toLocaleTimeString, where an unknown zone throws RangeError. + expect(() => new Intl.DateTimeFormat("en-US", { timeZone: body.timeZone as string })).not.toThrow(); + }); + + test("/api/logs still returns a bare array", async () => { + // The contract four other suites depend on. If this ever becomes an object, those + // suites fail somewhere far from here, so assert it at the source. + const url = new URL("http://localhost/api/logs"); + const response = await handleManagementAPI(new Request(url), url, config); + expect(response?.status).toBe(200); + expect(Array.isArray(await response!.json())).toBe(true); + }); +}); + +/** + * The GUI-side formatters, mirrored here because Logs.tsx is not importable from a Bun test. + * The behavior under test is the timeZone argument being threaded through at all -- passing + * `undefined` reproduces the pre-fix rendering exactly. + */ +function formatLogTimestamp(ts: number, localeTag?: string, timeZone?: string): string { + try { + return new Date(ts).toLocaleTimeString(localeTag, timeZone ? { timeZone } : undefined); + } catch { + return new Date(ts).toLocaleTimeString(localeTag); + } +} + +describe("timestamp formatting", () => { + // 2026-07-31T00:30:00Z -- deliberately across a date line for most zones, so a zone that + // is ignored shows up as a different hour rather than a subtle offset. + const ts = Date.parse("2026-07-31T00:30:00Z"); + + test("renders in the server zone, not the viewer's", () => { + const seoul = formatLogTimestamp(ts, "en-US", "Asia/Seoul"); + const utc = formatLogTimestamp(ts, "en-US", "UTC"); + // 00:30 UTC is 09:30 KST. If the argument were dropped both calls would agree, which is + // exactly the failure #790's own test could not catch -- it compared a value to itself. + expect(seoul).not.toBe(utc); + expect(seoul).toContain("9:30"); + expect(utc).toContain("12:30"); + }); + + test("falls back to browser-local for a zone the runtime does not know", () => { + // An ICU build without this zone throws RangeError, which would take the whole row + // render down. A wrong hour beats an empty log list. + expect(() => formatLogTimestamp(ts, "en-US", "Mars/Olympus_Mons")).not.toThrow(); + expect(formatLogTimestamp(ts, "en-US", "Mars/Olympus_Mons")) + .toBe(formatLogTimestamp(ts, "en-US", undefined)); + }); + + test("no zone keeps the previous behavior", () => { + expect(formatLogTimestamp(ts, "en-US", undefined)).toBe(new Date(ts).toLocaleTimeString("en-US")); + }); +}); From d3abf43450c6e6af04f6cba5a901b5bc2623b6a9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:22:14 +0900 Subject: [PATCH 15/24] fix(providers): give Volcengine Ark non-empty assistant text on tool turns (#796) Ark validates an assistant message's text as a required parameter and reads `""` as absent, so any history containing a tool-call-only assistant fails with `MissingParameter: input.content.text`. Single-turn requests work, which is why this only shows up once an agent loop has run a tool. The `""` it rejects is not accidental. xAI rejects the opposite way -- "Each message must have at least one content element" -- and `""` is what satisfies that, per the comment already sitting on the line. The two provider contracts are in direct conflict, so there is no value that satisfies both and the placeholder has to be host-gated. A single space is non-empty for Ark's validator and semantically empty for the model. The gate follows the file's existing pattern (`isXaiSchemaTarget`, `isKimiSchemaTarget`): an exact hostname set, matched on `URL.hostname` so a provider that merely mentions the string in a path is unaffected. Both current Ark regions are covered. All three placeholder sites move together -- tool-call-only, reasoning-only, and the synthesized orphan assistant -- since Ark's validator sees no difference between them. Every test runs the same input through Ark and a generic host and asserts the two diverge, so a mistake in either direction fails. Verified by ablation: - collapse the gate to always `""` -> 2 pass / 3 fail (Ark cases) - apply the space globally -> 2 pass / 3 fail (control + lookalike cases) - restored -> 5/5 That second one is the failure mode worth guarding: a global placeholder would have made the Ark tests pass while silently breaking xAI. Adjacent suites green: openai-chat-dangling-toolcalls, adapter-usage, openai-chat-parallel-stream -- 50 pass, 0 fail. --- src/adapters/openai-chat.ts | 35 +++++- .../volcengine-ark-assistant-content.test.ts | 104 ++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 tests/volcengine-ark-assistant-content.test.ts diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 0e463c65f..64fb890a1 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -206,10 +206,10 @@ function messagesToChatFormat(parsed: OcxParsedRequest, provider: OcxProviderCon })); // "" instead of null: strict validators (xAI: "Each message must have at least one // content element", langchain#34140) reject content-less assistant history entries. - if (!chatMsg.content) chatMsg.content = ""; + if (!chatMsg.content) chatMsg.content = emptyAssistantContent(provider); } if (chatMsg.reasoning_content !== undefined && chatMsg.content === undefined && chatMsg.tool_calls === undefined) { - chatMsg.content = ""; + chatMsg.content = emptyAssistantContent(provider); } out.push(chatMsg); pendingToolCalls = wireToolCalls.map(({ tc, id }) => ({ id, name: namespacedToolName(tc.namespace, tc.name) })); @@ -238,7 +238,7 @@ function messagesToChatFormat(parsed: OcxParsedRequest, provider: OcxProviderCon const name = safeToolName(msg.toolName); out.push({ role: "assistant", - content: "", + content: emptyAssistantContent(provider), tool_calls: [{ id: toolCallId, type: "function", @@ -372,6 +372,35 @@ function isKimiSchemaTarget(provider: OcxProviderConfig): boolean { } } +// Volcengine Ark regional endpoints. Ark validates an assistant message's text field as a +// REQUIRED parameter and treats "" as absent, so a tool-call-only assistant in history 400s with +// `MissingParameter: input.content.text` (#796). Every other OpenAI-compatible provider accepts +// "", and xAI actively requires it ("Each message must have at least one content element"), so +// the two contracts are in direct conflict and this cannot be a global change. +const VOLCENGINE_ARK_HOSTNAMES = new Set([ + "ark.cn-beijing.volces.com", + "ark.ap-southeast.volces.com", +]); + +function isVolcengineArkTarget(provider: OcxProviderConfig): boolean { + try { + return VOLCENGINE_ARK_HOSTNAMES.has(new URL(provider.baseUrl).hostname); + } catch { + return false; + } +} + +/** + * Placeholder text for an assistant history entry that carries only tool calls or reasoning. + * + * A single space rather than "" for Ark: it satisfies Ark's non-empty text requirement while + * staying semantically empty for the model. Ark rejects "" outright, so there is no value that + * satisfies both contracts — hence the host gate. + */ +function emptyAssistantContent(provider: OcxProviderConfig): string { + return isVolcengineArkTarget(provider) ? " " : ""; +} + /** * Kimi requires function.parameters.type to be exactly "object" at the root. * Codex tools with oneOf/anyOf schemas omit the root type, causing 400 errors. diff --git a/tests/volcengine-ark-assistant-content.test.ts b/tests/volcengine-ark-assistant-content.test.ts new file mode 100644 index 000000000..b63363139 --- /dev/null +++ b/tests/volcengine-ark-assistant-content.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, test } from "bun:test"; +import { createOpenAIChatAdapter } from "../src/adapters/openai-chat"; +import type { OcxMessage, OcxParsedRequest, OcxProviderConfig } from "../src/types"; + +/** + * #796: Volcengine Ark validates an assistant message's text as a REQUIRED parameter and treats + * "" as absent, so any history containing a tool-call-only assistant 400s with + * `MissingParameter: input.content.text`. Single-turn requests are fine; the failure needs the + * tool-call turn. + * + * This cannot be fixed globally. xAI rejects the opposite way -- "Each message must have at least + * one content element" -- and the existing "" is what satisfies it. So the two contracts conflict + * and the fix is host-gated, which is exactly what these tests have to prove: every case runs the + * SAME input through Ark and a generic host and asserts the two diverge. + */ + +function providerFor(baseUrl: string): OcxProviderConfig { + return { adapter: "openai-chat", baseUrl, apiKey: "sk-test", authMode: "key" }; +} + +const ark = providerFor("https://ark.cn-beijing.volces.com/api/v3"); +const generic = providerFor("https://example.test/v1"); + +interface ChatMsg { + role: string; + content?: unknown; + reasoning_content?: unknown; + tool_calls?: { id: string; function: { name: string; arguments: string } }[]; + tool_call_id?: string; +} + +function wire(provider: OcxProviderConfig, messages: OcxMessage[]): ChatMsg[] { + const parsed: OcxParsedRequest = { + modelId: "kimi-k3", + context: { messages }, + stream: false, + options: {}, + }; + const req = createOpenAIChatAdapter(provider).buildRequest(parsed) as { body: string }; + return (JSON.parse(req.body) as { messages: ChatMsg[] }).messages; +} + +function assistantToolCall(): OcxMessage { + return { + role: "assistant", + content: [{ type: "toolCall" as const, id: "call_1", name: "shell", arguments: { cmd: "ls" } }], + timestamp: 0, + }; +} + +function assistantsOf(messages: ChatMsg[]): ChatMsg[] { + return messages.filter(m => m.role === "assistant"); +} + +describe("Volcengine Ark empty assistant content (#796)", () => { + const history: OcxMessage[] = [ + { role: "user", content: "list dir", timestamp: 0 }, + assistantToolCall(), + { role: "toolResult", toolCallId: "call_1", toolName: "shell", content: "file1.txt", isError: false, timestamp: 0 }, + ]; + + test("a tool-call-only assistant carries non-empty text for Ark", () => { + const [assistant] = assistantsOf(wire(ark, history)); + expect(assistant.tool_calls).toHaveLength(1); + // The exact reproduction from the issue. "" is what Ark rejects. + expect(assistant.content).toBe(" "); + expect(String(assistant.content).length).toBeGreaterThan(0); + }); + + test("the same history still uses \"\" for every other provider", () => { + // The control. Without it the Ark assertion would also pass if the placeholder were applied + // globally -- which would break xAI, whose validator requires the empty-string element. + const [assistant] = assistantsOf(wire(generic, history)); + expect(assistant.tool_calls).toHaveLength(1); + expect(assistant.content).toBe(""); + }); + + test("a synthesized orphan tool-call assistant follows the same rule", () => { + // A tool result with no matching call: the adapter fabricates the assistant turn, and that + // fabricated message hits the same Ark validator. + const orphan: OcxMessage[] = [ + { role: "user", content: "hi", timestamp: 0 }, + { role: "toolResult", toolCallId: "call_missing", toolName: "shell", content: "out", isError: false, timestamp: 0 }, + ]; + const arkOrphan = assistantsOf(wire(ark, orphan)).find(m => m.tool_calls?.length); + const genericOrphan = assistantsOf(wire(generic, orphan)).find(m => m.tool_calls?.length); + expect(arkOrphan?.content).toBe(" "); + expect(genericOrphan?.content).toBe(""); + }); + + test("the gate matches the host, not the path or a substring of it", () => { + // A lookalike host must not inherit the quirk: the fix keys on an exact hostname set, so a + // provider merely mentioning the string in its path stays on the default contract. + const lookalike = providerFor("https://example.test/ark.cn-beijing.volces.com/v1"); + const [assistant] = assistantsOf(wire(lookalike, history)); + expect(assistant.content).toBe(""); + }); + + test("Ark's regional sibling endpoint gets the same treatment", () => { + const sea = providerFor("https://ark.ap-southeast.volces.com/api/v3"); + const [assistant] = assistantsOf(wire(sea, history)); + expect(assistant.content).toBe(" "); + }); +}); From 38f711bebeaf9ec083e2b8a1e79a730da5304b47 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:24:26 +0900 Subject: [PATCH 16/24] fix(openai-chat): fail a truncated tool call before it is flushed as complete At reader EOF the adapter flushed buffered tool calls and only then judged whether the stream had terminated cleanly. `flushToolCalls()` emits `tool_call_end`, so a socket that died mid-argument handed the client `{"cmd":"l` as a COMPLETED call: a truncation reported as a successful tool invocation, which is worse than an error because nothing downstream can tell it apart from a real one. The integrity check now runs before the flush. This adapter can still decide at that point because it buffers tool calls; the Anthropic adapter forwards fragments live and genuinely cannot, which is why the equivalent fix there was reverted (see devlog 260731_pr_merge_round/021). A usage frame does not launder it either. Usage counts as a terminal signal for text streams, and without the separate branch a provider that sent usage after dying mid-tool would still flush the partial call. What this deliberately does NOT do is the other half of #735. PR #793 (and #773 before it, merged then reverted by #792 with no recorded reason) additionally completes text-only streams that end without a terminal frame. That is a policy change, not a bug fix: it inverts two existing fail-closed assertions in tests/openai-chat-eof.test.ts, which currently pin "content without a terminator is truncation". Whether a relay's dropped terminal frame should be forgiven is the maintainer's call, and the missing revert rationale on #792 is exactly the context needed to make it. Attempting both halves here is how the previous round burned three repair rounds on the Anthropic side. So the narrowing half lands and the widening half does not. #735 stays open. Tests, verified by ablation (disable the guard -> 13 pass / 2 fail, restore -> 15/15): - a half-assembled tool call at EOF errors, emitting no tool_call_end or done - a trailing usage frame does not rescue it - a tool call closed by finish_reason still completes normally -- the control, without which the guard could fire on every well-formed tool turn Adjacent suites green: adapter-usage, openai-chat-parallel-stream. --- src/adapters/openai-chat.ts | 16 ++++++++++++- tests/openai-chat-eof.test.ts | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 64fb890a1..e4387b1ee 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -829,12 +829,26 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd if (buffer.length > 0) { if ((yield* handleDataLine(buffer)) === "terminate") return; } - yield* flushToolCalls(); // Reader EOF. A graceful close shows at least one terminal signal: `[DONE]` (returns above), // a non-null finish_reason (sawFinish), or a trailing usage chunk (providers emit usage only // at end-of-generation). If NONE of those were seen, the stream was cut mid-flight — fail // closed so the bridge emits a classified response.failed rather than a silent truncation. + // + // Checked BEFORE flushToolCalls(), because that helper emits tool_call_end and there is no + // taking it back: a half-assembled argument string would reach the client as a completed + // call. Tool calls are buffered here (unlike the Anthropic adapter, which forwards + // fragments live), so this adapter can still decide. const sawFinish = finishReason !== undefined; + if (!sawFinish && pendingToolCalls.length > 0) { + debugProviderDiagnostic("openai-chat", "stream-truncated", { + finishReason: null, + hadUsage: pendingUsage !== undefined, + pendingToolCalls: pendingToolCalls.length, + }); + yield { type: "error", message: "upstream stream ended mid tool call without a terminal signal — possible truncation" }; + return; + } + yield* flushToolCalls(); if (!sawFinish && pendingUsage === undefined) { debugProviderDiagnostic("openai-chat", "stream-truncated", { finishReason: finishReason ?? null, diff --git a/tests/openai-chat-eof.test.ts b/tests/openai-chat-eof.test.ts index 696151637..db44e6314 100644 --- a/tests/openai-chat-eof.test.ts +++ b/tests/openai-chat-eof.test.ts @@ -136,3 +136,46 @@ describe("openai-chat stream EOF fail-closed", () => { expect(events.some(e => e.type === "done")).toBe(false); }); }); + +describe("openai-chat EOF mid tool call (#735)", () => { + test("a half-assembled tool call at EOF errors instead of being flushed as complete", async () => { + // The provider opened a tool call and sent part of its argument JSON, then the socket closed + // with no finish_reason and no [DONE]. flushToolCalls() emits tool_call_end, so running it + // first would hand the client `{"cmd":"l` as a COMPLETED call -- a truncation reported as a + // successful tool invocation. The check therefore runs before the flush. + const response = new Response( + 'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"shell","arguments":"{\\"cmd\\":\\"l"}}]}}]}\n\n', + ); + const events = await collect(createOpenAIChatAdapter(provider).parseStream(response)); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(e => e.type === "tool_call_end")).toBe(false); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("a usage frame does not launder a truncated tool call into success", async () => { + // Usage alone counts as a terminal signal for text streams, and before this guard it also + // let a mid-flight tool call through: the tool branch is checked on finish_reason only, so + // usage must not be able to substitute for it. + const response = new Response([ + 'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"shell","arguments":"{\\"cmd\\":\\"l"}}]}}]}\n\n', + 'data: {"choices":[],"usage":{"prompt_tokens":1,"completion_tokens":2,"total_tokens":3}}\n\n', + ].join("")); + const events = await collect(createOpenAIChatAdapter(provider).parseStream(response)); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(e => e.type === "tool_call_end")).toBe(false); + }); + + test("a tool call closed by finish_reason still completes normally", async () => { + // The control: the guard must fire on MISSING terminal signals only, never on a well-formed + // tool turn, or every tool call in the product breaks. + const response = new Response([ + 'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"shell","arguments":"{\\"cmd\\":\\"ls\\"}"}}]}}]}\n\n', + 'data: {"choices":[{"delta":{},"finish_reason":"tool_calls"}]}\n\n', + "data: [DONE]\n\n", + ].join("")); + const events = await collect(createOpenAIChatAdapter(provider).parseStream(response)); + expect(events.some(e => e.type === "error")).toBe(false); + expect(events.filter(e => e.type === "tool_call_end")).toHaveLength(1); + expect(events.at(-1)?.type).toBe("done"); + }); +}); From 2eebd9268ea4e2af5b8667bc3fb90fa4a5eb3c41 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:33:12 +0900 Subject: [PATCH 17/24] fix(providers): send Ark the structured content form, not a placeholder space The review blocked the previous commit on the right question: what evidence says Ark accepts a space? None. I had reasoned from "the field must be non-empty" and picked the smallest non-empty string, and a validator that trims whitespace would have rejected it just as it rejects "" -- the fix would have shipped, passed every local test, and still 400'd against the live endpoint. Re-reading Ark's error changes the diagnosis. It names `input.content.text`, a NESTED path. Ark is not asking for non-empty text; it is asking for the structured content form, `[{type:"text",text:""}]`, which is where a `content.text` field exists at all. A bare string of any value never provides that path. The inner text stays empty, so nothing extra reaches the model -- which also retires the "a space may tokenize" objection rather than arguing with it. Every other provider keeps the bare `""` that xAI's validator requires, so the host gate stays. Also closes the two smaller review findings: the reasoning-only site now has direct coverage (it is reachable through the public `preserveReasoningContentModels` option, not a theoretical branch), and the EOF suite gains a `[DONE]`-only control. That control passes with the guard reverted, deliberately -- it pins the early-return path the guard must never begin intercepting, which a guard-sensitive test cannot express. Still unverified: no request in this round reached a live Ark endpoint, and the regional hostname set is the two currently documented chat hosts. Both are recorded in the issue so a user with Ark credentials can confirm before this is relied on. 67 pass across volcengine-ark-assistant-content, openai-chat-eof, openai-chat-dangling-toolcalls, adapter-usage, openai-chat-parallel-stream. --- src/adapters/openai-chat.ts | 16 ++++++---- tests/openai-chat-eof.test.ts | 14 +++++++++ .../volcengine-ark-assistant-content.test.ts | 30 +++++++++++++++---- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index e4387b1ee..61f84e611 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -391,14 +391,18 @@ function isVolcengineArkTarget(provider: OcxProviderConfig): boolean { } /** - * Placeholder text for an assistant history entry that carries only tool calls or reasoning. + * Placeholder content for an assistant history entry carrying only tool calls or reasoning. * - * A single space rather than "" for Ark: it satisfies Ark's non-empty text requirement while - * staying semantically empty for the model. Ark rejects "" outright, so there is no value that - * satisfies both contracts — hence the host gate. + * Ark's error names the parameter it wants: `input.content.text`. It is not asking for non-empty + * text, it is asking for the STRUCTURED content form — a `[{type:"text",text:""}]` array — and a + * bare string of any kind, `""` or `" "`, does not provide that path. The empty text inside the + * array stays empty, so nothing extra is fed to the model. + * + * Every other provider keeps the bare `""`, which xAI's validator specifically requires ("Each + * message must have at least one content element"), so this cannot be applied globally. */ -function emptyAssistantContent(provider: OcxProviderConfig): string { - return isVolcengineArkTarget(provider) ? " " : ""; +function emptyAssistantContent(provider: OcxProviderConfig): string | { type: "text"; text: string }[] { + return isVolcengineArkTarget(provider) ? [{ type: "text", text: "" }] : ""; } /** diff --git a/tests/openai-chat-eof.test.ts b/tests/openai-chat-eof.test.ts index db44e6314..e9025144c 100644 --- a/tests/openai-chat-eof.test.ts +++ b/tests/openai-chat-eof.test.ts @@ -165,6 +165,20 @@ describe("openai-chat EOF mid tool call (#735)", () => { expect(events.some(e => e.type === "tool_call_end")).toBe(false); }); + test("a tool call closed by [DONE] alone still completes normally", async () => { + // [DONE] flushes and returns BEFORE the EOF block, so this exercises a different early-return + // path than the finish_reason control below. It passes with the guard reverted -- that is the + // point: it pins the path the guard must never start intercepting. + const response = new Response([ + 'data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"call_1","function":{"name":"shell","arguments":"{\\"cmd\\":\\"ls\\"}"}}]}}]}\n\n', + "data: [DONE]\n\n", + ].join("")); + const events = await collect(createOpenAIChatAdapter(provider).parseStream(response)); + expect(events.some(e => e.type === "error")).toBe(false); + expect(events.filter(e => e.type === "tool_call_end")).toHaveLength(1); + expect(events.at(-1)?.type).toBe("done"); + }); + test("a tool call closed by finish_reason still completes normally", async () => { // The control: the guard must fire on MISSING terminal signals only, never on a well-formed // tool turn, or every tool call in the product breaks. diff --git a/tests/volcengine-ark-assistant-content.test.ts b/tests/volcengine-ark-assistant-content.test.ts index b63363139..15497e6dc 100644 --- a/tests/volcengine-ark-assistant-content.test.ts +++ b/tests/volcengine-ark-assistant-content.test.ts @@ -59,12 +59,12 @@ describe("Volcengine Ark empty assistant content (#796)", () => { { role: "toolResult", toolCallId: "call_1", toolName: "shell", content: "file1.txt", isError: false, timestamp: 0 }, ]; - test("a tool-call-only assistant carries non-empty text for Ark", () => { + test("a tool-call-only assistant uses the structured content form for Ark", () => { const [assistant] = assistantsOf(wire(ark, history)); expect(assistant.tool_calls).toHaveLength(1); - // The exact reproduction from the issue. "" is what Ark rejects. - expect(assistant.content).toBe(" "); - expect(String(assistant.content).length).toBeGreaterThan(0); + // Ark's error names `input.content.text` -- a nested path, so it is asking for the array + // form, not merely for non-empty text. The inner text stays empty. + expect(assistant.content).toEqual([{ type: "text", text: "" }]); }); test("the same history still uses \"\" for every other provider", () => { @@ -84,7 +84,7 @@ describe("Volcengine Ark empty assistant content (#796)", () => { ]; const arkOrphan = assistantsOf(wire(ark, orphan)).find(m => m.tool_calls?.length); const genericOrphan = assistantsOf(wire(generic, orphan)).find(m => m.tool_calls?.length); - expect(arkOrphan?.content).toBe(" "); + expect(arkOrphan?.content).toEqual([{ type: "text", text: "" }]); expect(genericOrphan?.content).toBe(""); }); @@ -99,6 +99,24 @@ describe("Volcengine Ark empty assistant content (#796)", () => { test("Ark's regional sibling endpoint gets the same treatment", () => { const sea = providerFor("https://ark.ap-southeast.volces.com/api/v3"); const [assistant] = assistantsOf(wire(sea, history)); - expect(assistant.content).toBe(" "); + expect(assistant.content).toEqual([{ type: "text", text: "" }]); + }); + + test("a reasoning-only assistant takes the same path", () => { + // The third placeholder site, reachable whenever an Ark model is listed in + // preserveReasoningContentModels -- public config, so it is not a theoretical branch. + const arkReasoning = providerFor("https://ark.cn-beijing.volces.com/api/v3"); + arkReasoning.preserveReasoningContentModels = ["kimi-k3"]; + const reasoningHistory: OcxMessage[] = [ + { role: "user", content: "hi", timestamp: 0 }, + { role: "assistant", content: [{ type: "thinking" as const, thinking: "deliberating" }], timestamp: 0 }, + ]; + const arkAssistant = assistantsOf(wire(arkReasoning, reasoningHistory))[0]; + expect(arkAssistant.reasoning_content).toBe("deliberating"); + expect(arkAssistant.content).toEqual([{ type: "text", text: "" }]); + + const genericReasoning = providerFor("https://example.test/v1"); + genericReasoning.preserveReasoningContentModels = ["kimi-k3"]; + expect(assistantsOf(wire(genericReasoning, reasoningHistory))[0].content).toBe(""); }); }); From c4acc2e3b4bf05518d8c325592fd591e3f85dd00 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:36:04 +0900 Subject: [PATCH 18/24] docs(providers): label the Ark content shape as unverified in the code itself The commit message said the array form was unconfirmed while the code comment stated it as fact. The review caught the mismatch, and the code comment is the one a future reader will actually find. Ark publishes array content only for multimodal USER input; nothing in its reference covers an assistant history entry, so the shape is inferred from the error's nested parameter path and nothing more. That inference is better motivated than the whitespace guess it replaced, but it is still an inference, and the wording now says so at both the helper and the test header. No behavior change. --- src/adapters/openai-chat.ts | 12 ++++++++---- tests/volcengine-ark-assistant-content.test.ts | 9 +++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 61f84e611..0f94ff1ad 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -393,10 +393,14 @@ function isVolcengineArkTarget(provider: OcxProviderConfig): boolean { /** * Placeholder content for an assistant history entry carrying only tool calls or reasoning. * - * Ark's error names the parameter it wants: `input.content.text`. It is not asking for non-empty - * text, it is asking for the STRUCTURED content form — a `[{type:"text",text:""}]` array — and a - * bare string of any kind, `""` or `" "`, does not provide that path. The empty text inside the - * array stays empty, so nothing extra is fed to the model. + * UNVERIFIED HYPOTHESIS for Ark. The reported error names `input.content.text`, a nested path, + * which suggests Ark wants the structured content form `[{type:"text",text:""}]` rather than a + * bare string — no string value, `""` or `" "`, exposes a `content.text` path at all. But Ark's + * published examples only show array content for MULTIMODAL USER input, never for an assistant + * history entry, so this shape is inferred from the error message and not confirmed by the docs + * or by a live request. The empty inner text at least adds no tokens either way. + * + * Confirm against a real Ark endpoint before relying on this; #796 records what is still missing. * * Every other provider keeps the bare `""`, which xAI's validator specifically requires ("Each * message must have at least one content element"), so this cannot be applied globally. diff --git a/tests/volcengine-ark-assistant-content.test.ts b/tests/volcengine-ark-assistant-content.test.ts index 15497e6dc..bc4f989bf 100644 --- a/tests/volcengine-ark-assistant-content.test.ts +++ b/tests/volcengine-ark-assistant-content.test.ts @@ -12,6 +12,10 @@ import type { OcxMessage, OcxParsedRequest, OcxProviderConfig } from "../src/typ * one content element" -- and the existing "" is what satisfies it. So the two contracts conflict * and the fix is host-gated, which is exactly what these tests have to prove: every case runs the * SAME input through Ark and a generic host and asserts the two diverge. + * + * Scope of these tests: they verify the WIRE SHAPE we emit and that the host gate is real. They + * cannot verify that Ark accepts it -- no request here reaches Volcengine. The array form is + * inferred from the error's nested parameter path and is still unconfirmed; see #796. */ function providerFor(baseUrl: string): OcxProviderConfig { @@ -62,8 +66,9 @@ describe("Volcengine Ark empty assistant content (#796)", () => { test("a tool-call-only assistant uses the structured content form for Ark", () => { const [assistant] = assistantsOf(wire(ark, history)); expect(assistant.tool_calls).toHaveLength(1); - // Ark's error names `input.content.text` -- a nested path, so it is asking for the array - // form, not merely for non-empty text. The inner text stays empty. + // Ark's error names `input.content.text`, a nested path, which is why the array form is the + // working hypothesis -- no bare string exposes a `content.text` path. UNVERIFIED against a + // live endpoint: this test pins what we send, not that Ark accepts it. expect(assistant.content).toEqual([{ type: "text", text: "" }]); }); From 3f7a5045848cdb302454435c9c71f843f22d61d9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:43:03 +0900 Subject: [PATCH 19/24] fix(providers): honour the registry private-network default at the fetch too (#758) Ollama's registry entry is marked `allowPrivateNetworkByDefault` because a local model server is what it IS. Config validation read that flag; outbound discovery did not, reading only the operator's `allowPrivateNetwork`. So a stock Ollama provider validated fine and was then refused at the fetch -- one provider, two boundaries, two answers. Both now consult the same helper. This grants nothing new: a provider with neither a registry default nor an explicit flag is denied exactly as before, and the three registry entries carrying the default (Ollama, vLLM, LM Studio) are local by definition. The SSRF exclusions are unchanged and, importantly, sit in FRONT of this permission rather than inside it -- metadata, link-local and unspecified destinations are rejected before any private-network question is asked. The tests prove that for Ollama specifically, since it is the provider that gained access: if those exclusions were ever folded into the same check, `169.254.169.254` is where it would surface, and an SSRF there leaks cloud instance credentials. A note on how these tests were built, because the first attempt was wrong. They originally drove the `provider.fetch` executor branch, and the ablation did not fail -- that branch delegates to `providerDestinationConfigError`, which already consulted the registry, so the tests never touched the defect. Real discovery has no injected executor and goes through the pinned transport. The tests now inject `resolveAddresses` there and assert the permission value it receives, which is the actual fix. Ablation with the pinned-path change reverted: 5 pass / 1 fail. Restored: 6/6. Adjacent suites green -- provider-outbound, discovery-contract, provider-connection-test: 46 pass. This is a security-boundary change under MAINTAINERS.md and wants a maintainer's eyes before it reaches dev, even though it only re-uses a permission the config layer already granted. --- src/lib/destination-policy.ts | 20 ++++++ src/lib/provider-outbound.ts | 8 ++- .../provider-outbound-private-network.test.ts | 68 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 tests/provider-outbound-private-network.test.ts diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts index 0b060fb90..90faebbac 100644 --- a/src/lib/destination-policy.ts +++ b/src/lib/destination-policy.ts @@ -126,6 +126,26 @@ function assessDestination(baseUrl: string): DestinationAssessment | null { } } +/** + * Whether a provider may reach loopback/private addresses. + * + * Two sources, and both have to be consulted at every boundary: the operator's explicit + * `allowPrivateNetwork`, and the registry's `allowPrivateNetworkByDefault` for entries that are + * local BY DEFINITION (Ollama, vLLM, LM Studio). Config validation already read both; outbound + * discovery read only the first, so a stock Ollama entry passed validation and was then refused + * at the fetch (#758). + * + * This grants nothing new. Metadata, link-local and unspecified destinations are rejected before + * this is consulted, and a provider without either source still cannot reach a private address. + */ +export function providerAllowsPrivateNetwork( + name: string, + provider: Pick, +): boolean { + return provider.allowPrivateNetwork === true + || getProviderRegistryEntry(name)?.allowPrivateNetworkByDefault === true; +} + function registryAllowsPrivateNetwork(name: string): boolean { return getProviderRegistryEntry(name)?.allowPrivateNetworkByDefault === true; } diff --git a/src/lib/provider-outbound.ts b/src/lib/provider-outbound.ts index e4f7d0a5c..588c8c7dc 100644 --- a/src/lib/provider-outbound.ts +++ b/src/lib/provider-outbound.ts @@ -2,6 +2,7 @@ import type { OcxProviderConfig } from "../types"; import { assessUrlDestination, DestinationDnsResolutionError, + providerAllowsPrivateNetwork, providerDestinationConfigError, resolvePublicAddresses, } from "./destination-policy"; @@ -116,7 +117,10 @@ export async function providerOutboundGet( if (assessment?.kind === "metadata" || assessment?.kind === "link-local" || assessment?.kind === "unspecified") { throw new ProviderOutboundPolicyError(`provider URL targets ${assessment.detail}`); } - if (!provider.allowPrivateNetwork) { + // Registry defaults count here too, not just the operator flag: a stock Ollama entry is + // local by definition and previously passed config validation only to be refused at the + // fetch (#758). Metadata/link-local/unspecified were already rejected above. + if (!providerAllowsPrivateNetwork(name, provider)) { const destinationError = providerDestinationConfigError(name, { baseUrl: url, allowPrivateNetwork: false, @@ -133,7 +137,7 @@ export async function providerOutboundGet( try { resolved = await resolveAddresses(url, { context: "provider URL", - allowPrivateNetwork: provider.allowPrivateNetwork, + allowPrivateNetwork: providerAllowsPrivateNetwork(name, provider), }); } catch (error) { const dnsResolutionFailed = error instanceof DestinationDnsResolutionError diff --git a/tests/provider-outbound-private-network.test.ts b/tests/provider-outbound-private-network.test.ts new file mode 100644 index 000000000..3c17d307b --- /dev/null +++ b/tests/provider-outbound-private-network.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { providerOutboundGet, ProviderOutboundPolicyError } from "../src/lib/provider-outbound"; +import { resolvePublicAddresses } from "../src/lib/destination-policy"; + +/** + * #758: a stock Ollama provider passed config validation -- which consults the registry's + * `allowPrivateNetworkByDefault` -- and was then refused at the outbound fetch, which read only + * the operator's `allowPrivateNetwork` flag. Two boundaries disagreeing about one provider. + * + * These drive the PINNED transport path (no `provider.fetch`), because that is where discovery + * actually runs and where the permission was being dropped. `resolveAddresses` is injected so the + * decision is observable without a network: the value it receives IS the fix. + */ + +const localProvider = { baseUrl: "http://localhost:11434/v1" }; + +/** Captures the permission the resolver was called with, then fails the call. */ +function capturingResolver(): { calls: boolean[]; resolve: typeof resolvePublicAddresses } { + const calls: boolean[] = []; + const resolve = (async (_url: string, opts?: { allowPrivateNetwork?: boolean }) => { + calls.push(opts?.allowPrivateNetwork === true); + throw new ProviderOutboundPolicyError("stop here — the permission has already been decided"); + }) as unknown as typeof resolvePublicAddresses; + return { calls, resolve }; +} + +describe("provider outbound private-network permission (#758)", () => { + test("a registry-local provider passes allowPrivateNetwork even without an operator flag", async () => { + // The defect. Before the fix this resolver saw `false` and discovery was refused for a + // provider whose registry entry is local by definition. + const { calls, resolve } = capturingResolver(); + await providerOutboundGet("ollama", localProvider, "http://localhost:11434/v1/models", {}, { resolveAddresses: resolve }) + .catch(() => undefined); + expect(calls).toEqual([true]); + }); + + test("a provider with neither a registry default nor a flag is still denied", async () => { + // The permission has to come from somewhere; an arbitrary custom provider gains nothing. + const { calls, resolve } = capturingResolver(); + await providerOutboundGet("custom", { baseUrl: "http://192.168.1.10:8080/v1" }, "http://192.168.1.10:8080/v1/models", {}, { resolveAddresses: resolve }) + .catch(() => undefined); + expect(calls).toEqual([false]); + }); + + test("an explicit operator flag still stands on its own", async () => { + const { calls, resolve } = capturingResolver(); + await providerOutboundGet("custom", { baseUrl: "http://10.0.0.5:8080/v1", allowPrivateNetwork: true }, "http://10.0.0.5:8080/v1/models", {}, { resolveAddresses: resolve }) + .catch(() => undefined); + expect(calls).toEqual([true]); + }); + + test.each([ + ["cloud metadata", "http://169.254.169.254/latest/meta-data/"], + ["link-local", "http://169.254.1.1/v1/models"], + ["unspecified", "http://0.0.0.0/v1/models"], + ])("a registry-local provider still cannot reach %s", async (_label, url) => { + // The load-bearing case. Ollama is the provider that GAINED private-network access, so if the + // SSRF exclusions were ever folded into the same permission, it shows up here first. + // 169.254.169.254 is the AWS/GCP credential endpoint; an SSRF there leaks instance creds. + // These run through the executor branch, where the literal assessment rejects before any + // permission check -- proving the exclusions sit in FRONT of the widened permission. + await expect(providerOutboundGet( + "ollama", + { ...localProvider, fetch: (async () => new Response("{}")) as unknown as typeof fetch }, + url, + )).rejects.toThrow(ProviderOutboundPolicyError); + }); +}); From 13a76a38a987bb916fe0e29bcc46a49de5007707 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:49:50 +0900 Subject: [PATCH 20/24] fix(tray): keep the Windows autostart Run entry under 260 characters (#696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HKCU Run values are capped at 260 characters. The tray registered a full PowerShell invocation carrying the Bun path, the CLI entry, the tray script and both home directories, so a user whose npm prefix sits under a long Windows profile name blew the cap and the tray silently never launched after sign-in. An owned VBS launcher holds the long command instead, and the Run value becomes `wscript.exe //B //NoLogo `. The launcher is written to the config directory, tracked in tray state, ownership-checked before reuse, and rolled back on failure, so a stale or foreign file at that path is not executed. The regression test builds an entry with a realistically long profile path and asserts the Run value stays within 260 characters, names `wscript.exe`, points at the .vbs — and that the PowerShell form still exceeds 260, which is what makes the first three assertions mean something. Verified by ablation, and the first attempt at that was wrong: disabling the 260-character guard did not fail anything, because the guard is a startup assertion rather than the fix. Substituting the old PowerShell command back into `buildWindowsTrayRunCommand` -- the actual pre-fix behavior -- fails the test (0 pass / 1 fail), and restoring passes 18/18 across both tray suites. This PR was blocked earlier in the round by a conflict in tests/windows-tray.test.ts. Landing #768 first resolved it: the branch now merges clean. Windows behavior cannot be exercised from macOS, so the matrix CI is what confirms it; the local evidence is limited to the pure command-construction functions, which is exactly what these tests cover. Co-authored-by: bitkyc08-arch --- src/tray/windows.ts | 63 ++++++++++++++++++++++++---- tests/windows-tray-run-limit.test.ts | 25 +++++++++++ tests/windows-tray.test.ts | 32 +++++++++++++- 3 files changed, 110 insertions(+), 10 deletions(-) create mode 100644 tests/windows-tray-run-limit.test.ts diff --git a/src/tray/windows.ts b/src/tray/windows.ts index 65c568692..eec7251af 100644 --- a/src/tray/windows.ts +++ b/src/tray/windows.ts @@ -150,8 +150,16 @@ function quoteRunValue(value: string): string { return `\"${value}\"`; } -/** Command persisted under HKCU Run. Every value is an owned absolute package/home path. */ -export function buildWindowsTrayRunCommand(entry: WindowsTrayEntry, powershell = windowsPowerShellPath()): string { +function installedTrayLauncherPath(): string { + return join(getConfigDir(), "opencodex-tray.vbs"); +} + +function quoteVbsPath(value: string): string { + return value.replace(/"/g, '""'); +} + +/** Full PowerShell invocation used by the owned VBS launcher (not written to HKCU Run). */ +export function buildWindowsTrayPowerShellCommand(entry: WindowsTrayEntry, powershell = windowsPowerShellPath()): string { return [ quoteRunValue(powershell), "-NoLogo", @@ -169,6 +177,27 @@ export function buildWindowsTrayRunCommand(entry: WindowsTrayEntry, powershell = ].join(" "); } +/** Short HKCU Run command (must stay ≤260 chars under long Windows user/npm paths). */ +export function buildWindowsTrayRunCommand(entry: WindowsTrayEntry & { launcherPath: string }): string { + const wscript = join(process.env.SystemRoot ?? "C:\\Windows", "System32", "wscript.exe"); + return `${quoteRunValue(wscript)} //B //NoLogo ${quoteRunValue(entry.launcherPath)}`; +} + +export function buildWindowsTrayLauncherScript(entry: WindowsTrayEntry, powershell = windowsPowerShellPath()): string { + const command = buildWindowsTrayPowerShellCommand(entry, powershell); + // VBS CreateObject("WScript.Shell").Run command, 0, False — hidden, non-blocking. + return [ + "' OpenCodex owned tray launcher — do not edit by hand.", + `CreateObject("WScript.Shell").Run "${quoteVbsPath(command)}", 0, False`, + "", + ].join("\r\n"); +} + +/** @deprecated Prefer buildWindowsTrayPowerShellCommand; kept for callers that still expect the long form. */ +export function buildWindowsTrayLegacyRunCommand(entry: WindowsTrayEntry, powershell = windowsPowerShellPath()): string { + return buildWindowsTrayPowerShellCommand(entry, powershell); +} + function readState(): WindowsTrayState | null { try { const state = JSON.parse(readFileSync(trayStatePath(), "utf8")) as Partial; @@ -204,7 +233,11 @@ function replaceOwnedFile(path: string, contents: string | Buffer): void { } } -function writeState(entry: WindowsTrayEntry, runValue: string, runCommand: string): void { +function writeState( + entry: WindowsTrayEntry & { launcherPath: string }, + runValue: string, + runCommand: string, +): void { const path = trayStatePath(); replaceOwnedFile(path, JSON.stringify({ version: TRAY_STATE_VERSION, ...entry, runValue, runCommand }, null, 2) + "\n"); } @@ -376,7 +409,8 @@ function trayStatusFrom(registered: string | null): WindowsTrayStatus { const running = heartbeatProcessAlive(heartbeat); const registrationOwned = state !== null && registered === state.runCommand - && [state.bun, state.cli, state.script, ...installedTrayIconPaths()].every(path => existsSync(path)); + && [state.bun, state.cli, state.script, ...(state.launcherPath ? [state.launcherPath] : []), ...installedTrayIconPaths()] + .every(path => existsSync(path)); const stale = windowsTrayRegistrationIsStale({ registered: registered !== null, registrationOwned, @@ -507,7 +541,12 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { } recordOwnedConfigPath(getConfigDir(), trayStatePath()); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true, mode: 0o700 }); - const runCommand = buildWindowsTrayRunCommand(entry); + const launcherPath = installedTrayLauncherPath(); + const entryWithLauncher = { ...entry, launcherPath }; + const runCommand = buildWindowsTrayRunCommand(entryWithLauncher); + if (runCommand.length > 260) { + throw new Error(`Tray Run command exceeds the Windows 260-character limit (${runCommand.length} chars).`); + } const runValue = windowsTrayRunValue(entry.opencodexHome); const existing = readOwnedRunValue(runValue); const state = readState(); @@ -517,6 +556,9 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { if (existsSync(entry.script) && (!state || resolve(state.script) !== resolve(entry.script))) { throw new Error(`Refusing to overwrite an unowned tray script at ${entry.script}.`); } + if (existsSync(launcherPath) && (!state?.launcherPath || resolve(state.launcherPath) !== resolve(launcherPath))) { + throw new Error(`Refusing to overwrite an unowned tray launcher at ${launcherPath}.`); + } if (!state && iconPairs.some(pair => existsSync(pair.installed))) { throw new Error("Refusing to overwrite unowned Windows tray icon assets."); } @@ -531,6 +573,7 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { const previousStateBytes = existsSync(trayStatePath()) ? readFileSync(trayStatePath()) : null; const previousScriptBytes = existsSync(entry.script) ? readFileSync(entry.script) : null; + const previousLauncherBytes = existsSync(launcherPath) ? readFileSync(launcherPath) : null; const previousIconBytes = new Map(iconPairs.map(pair => [ pair.installed, existsSync(pair.installed) ? readFileSync(pair.installed) : null, @@ -540,6 +583,10 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { if (previousScriptBytes) replaceOwnedFile(entry.script, previousScriptBytes); else if (existsSync(entry.script)) unlinkSync(entry.script); } catch { /* rollback best-effort */ } + try { + if (previousLauncherBytes) replaceOwnedFile(launcherPath, previousLauncherBytes); + else if (existsSync(launcherPath)) unlinkSync(launcherPath); + } catch { /* rollback best-effort */ } for (const [path, contents] of previousIconBytes) { try { if (contents) replaceOwnedFile(path, contents); @@ -567,8 +614,9 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { if (!hardenedDir.ok) throw new Error("Windows tray directory ACL hardening did not complete; refusing to install persistence."); replaceOwnedFile(entry.script, readFileSync(sourceScript)); for (const pair of iconPairs) replaceOwnedFile(pair.installed, readFileSync(pair.source)); + replaceOwnedFile(launcherPath, Buffer.from("\uFEFF" + buildWindowsTrayLauncherScript(entry), "utf16le")); runRegistry(["add", RUN_KEY, "/v", runValue, "/t", "REG_SZ", "/d", runCommand, "/f", "/reg:64"]); - writeState(entry, runValue, runCommand); + writeState(entryWithLauncher, runValue, runCommand); } catch (error) { restorePreviousInstall(); throw error; @@ -578,9 +626,6 @@ export function installWindowsTray(startNow = true): WindowsTrayStatus { restorePreviousInstall(); throw new Error("The tray startup registration was installed, but the tray process did not become healthy."); } - if (state?.launcherPath && existsSync(state.launcherPath)) { - try { unlinkSync(state.launcherPath); } catch { /* old owned VBS is inert after a committed Run replacement */ } - } return getWindowsTrayStatus(); } diff --git a/tests/windows-tray-run-limit.test.ts b/tests/windows-tray-run-limit.test.ts new file mode 100644 index 000000000..115d84475 --- /dev/null +++ b/tests/windows-tray-run-limit.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from "bun:test"; +import { + buildWindowsTrayRunCommand, + buildWindowsTrayPowerShellCommand, +} from "../src/tray/windows"; + +describe("windows tray Run registration (#696)", () => { + test("short wscript Run command stays within the 260-character Windows limit under long paths", () => { + const longHome = "C:\\Users\\VeryLongWindowsUserNameForTestingPaths\\AppData\\Roaming\\npm\\node_modules\\@bitkyc08\\opencodex"; + const entry = { + bun: `${longHome}\\node_modules\\bun\\bin\\bun.exe`, + cli: `${longHome}\\src\\cli\\index.ts`, + script: "C:\\Users\\VeryLongWindowsUserNameForTestingPaths\\.opencodex\\opencodex-tray.ps1", + codexHome: "C:\\Users\\VeryLongWindowsUserNameForTestingPaths\\.codex", + opencodexHome: "C:\\Users\\VeryLongWindowsUserNameForTestingPaths\\.opencodex", + launcherPath: "C:\\Users\\VeryLongWindowsUserNameForTestingPaths\\.opencodex\\opencodex-tray.vbs", + }; + const runCommand = buildWindowsTrayRunCommand(entry); + expect(runCommand.length).toBeLessThanOrEqual(260); + expect(runCommand.toLowerCase()).toContain("wscript.exe"); + expect(runCommand).toContain("opencodex-tray.vbs"); + // The long PowerShell form still exists for the VBS body, but must not be the Run value. + expect(buildWindowsTrayPowerShellCommand(entry).length).toBeGreaterThan(260); + }); +}); diff --git a/tests/windows-tray.test.ts b/tests/windows-tray.test.ts index f00d8cb10..60383dd64 100644 --- a/tests/windows-tray.test.ts +++ b/tests/windows-tray.test.ts @@ -3,6 +3,8 @@ import { copyFileSync, existsSync, mkdtempSync, readFileSync, rmSync } from "nod import { tmpdir } from "node:os"; import { join } from "node:path"; import { + buildWindowsTrayLauncherScript, + buildWindowsTrayPowerShellCommand, buildWindowsTrayRunCommand, launchWindowsTrayHost, parseWindowsTrayRunValue, @@ -40,11 +42,39 @@ describe("Windows tray packaging and command safety", () => { }); test("quotes metacharacter and Unicode paths without shell interpolation", () => { - const powershellCommand = buildWindowsTrayRunCommand(entry, "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); + const powershellCommand = buildWindowsTrayPowerShellCommand(entry, "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); expect(powershellCommand).toContain(`-File "${entry.script}"`); expect(powershellCommand).toContain(`-OpenCodexHome "${entry.opencodexHome}"`); expect(powershellCommand).not.toContain("cmd /c"); expect(powershellCommand).not.toContain("-Command"); + const runCommand = buildWindowsTrayRunCommand({ + ...entry, + launcherPath: `${entry.opencodexHome}\\opencodex-tray.vbs`, + }); + expect(runCommand.toLowerCase()).toContain("wscript.exe"); + expect(runCommand.length).toBeLessThanOrEqual(260); + }); + test("keeps UNC backslashes literal in the VBS Run command", () => { + const uncRoot = "\\\\server\\share"; + const uncEntry: WindowsTrayEntry = { + bun: `${uncRoot}\\tools\\bun.exe`, + cli: `${uncRoot}\\repo\\src\\cli\\index.ts`, + script: `${uncRoot}\\repo\\src\\tray\\windows-tray.ps1`, + codexHome: "C:\\Users\\Test\\.codex", + opencodexHome: `${uncRoot}\\opencodex`, + }; + const launcher = buildWindowsTrayLauncherScript(uncEntry); + expect(launcher).toContain(`${uncRoot}\\tools\\bun.exe`); + expect(launcher).not.toMatch(/\\\\\\\\server/); + }); + + + test("preserves non-ASCII paths in the tray launcher script and UTF-16LE install encoding", () => { + const launcher = buildWindowsTrayLauncherScript(entry); + expect(launcher).toContain("사용자 공간"); + const encoded = Buffer.from("\uFEFF" + launcher, "utf16le"); + expect(encoded.subarray(0, 2).equals(Buffer.from([0xff, 0xfe]))).toBe(true); + expect(encoded.toString("utf16le")).toContain("사용자 공간"); }); test("rejects quote and control-character path injection", () => { From 2d0c6a99c4eb8384f01bf6a56b75a934f4ef0b76 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:51:30 +0900 Subject: [PATCH 21/24] fix(service): verify the proxy is really gone before reporting a stop (#764) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ocx service stop` reported success and restored native Codex while a proxy was still listening, leaving both pointing at each other. The cause is that `ops.stop()` reports the outcome of the STOP COMMAND, not of the process. A Windows scheduler task whose wrapper survives `schtasks /end` respawns its child a few seconds later, so a stop that returned success can still leave a live proxy. The tracked-pid cleanup does not catch it either: the respawned child writes a pid this process never recorded, and `stopTrackedProxyIfRunning` returns "none" the moment the pid file is absent. Stop is now verified instead of inferred. After the manager and the tracked proxy are stopped, the command polls for a live proxy across the observed restart window; if one answers, it reports the port, leaves native Codex alone, and exits non-zero. Restoring the config on top of a running proxy is the part that turns a failed stop into a broken installation, so that is the step that must not happen on an unverified stop. Deliberately different from PR #780, which waits only when `schtasks /end` returns an ERROR. The reported case is an /end that SUCCEEDS while the wrapper lives — on that path there is nothing for the command to report, so waiting on its exit status cannot help. That PR's other work (orphan discovery, PID verification, Microsoft-account preflight) is untouched here and still worth reviewing separately. Four tests on an injected clock, so no wall-clock sleeping. Ablation removing the restart-window polling in favour of a single probe: 1 pass / 3 fail, including the respawn case itself; restored 4/4. One test asserts a genuinely stopped proxy returns null — without it the check could report a survivor every time and strand native Codex unrestored, which is worse than the bug. Another asserts a throwing probe is not read as absence, since an unreachable health endpoint means unknown, not stopped. 87 pass across service, proxy-liveness and both tray suites. Windows behavior needs the matrix CI; the logic here is clock-injected and platform-neutral. --- src/service.ts | 47 +++++++++++++++ tests/service-stop-verification.test.ts | 77 +++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tests/service-stop-verification.test.ts diff --git a/src/service.ts b/src/service.ts index d3397df58..d55608997 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1626,6 +1626,39 @@ function platformOps(backend: ServiceBackend = "scheduler"): ServiceOps | null { type TrackedProxyCleanupResult = "none" | "stale" | "stopped"; +/** + * Whether a proxy is still answering after the service manager claimed to stop it. + * + * `ops.stop()` reports the outcome of the STOP COMMAND, not of the process. A Windows scheduler + * task whose wrapper survives `schtasks /end` respawns its child a few seconds later, so a stop + * that returned success can still leave a live proxy — and `ocx service stop` then restored + * native Codex on top of a running one (#764). The tracked-pid cleanup does not catch it either: + * the respawned child writes a different pid, or none this process knows about. + * + * Probed rather than assumed, and bounded: the observed respawn window is ~5s, so a few seconds + * of polling either sees it come back or it is genuinely gone. + */ +export async function proxyStillLiveAfterStop(deps: { + findProxy?: () => Promise<{ port: number } | null>; + sleep?: (ms: number) => Promise; + now?: () => number; +} = {}): Promise<{ port: number } | null> { + const findProxy = deps.findProxy ?? findLiveProxy; + const sleep = deps.sleep ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); + const now = deps.now ?? Date.now; + const deadline = now() + 7000; + for (;;) { + try { + const live = await findProxy(); + if (live) return live; + } catch { + // A probe failure is not proof the proxy is gone; keep polling until the deadline. + } + if (now() >= deadline) return null; + await sleep(1000); + } +} + async function stopTrackedProxyIfRunning(): Promise { const pid = readPid(); if (!pid) return "none"; @@ -1952,6 +1985,20 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise current, + sleep: async (ms: number) => { current += ms; }, + }; +} + +describe("service stop verification (#764)", () => { + test("a proxy that respawns inside the restart window is detected", async () => { + // The reported failure: /end succeeded, the wrapper lived, and the child came back at ~5s. + // A single probe immediately after the stop command would have seen nothing and passed. + const clock = fakeClock(); + let probes = 0; + const live = await proxyStillLiveAfterStop({ + findProxy: async () => { + probes += 1; + return probes >= 5 ? { port: 10100 } : null; + }, + ...clock, + }); + expect(live).toEqual({ port: 10100 }); + expect(probes).toBeGreaterThan(1); + }); + + test("a genuinely stopped proxy returns null within the bound", async () => { + // The control. Without it the check could report a survivor every time and block every + // legitimate stop -- worse than the bug, since it would strand native Codex unrestored. + const clock = fakeClock(); + let probes = 0; + const live = await proxyStillLiveAfterStop({ + findProxy: async () => { probes += 1; return null; }, + ...clock, + }); + expect(live).toBeNull(); + expect(probes).toBeGreaterThan(1); + }); + + test("a probe that throws does not count as proof the proxy is gone", async () => { + // A failing health probe means "unknown", not "stopped". Treating an exception as absence + // would restore native Codex on top of a proxy that is merely unreachable for a moment. + const clock = fakeClock(); + let probes = 0; + const live = await proxyStillLiveAfterStop({ + findProxy: async () => { + probes += 1; + if (probes < 4) throw new Error("connection refused"); + return { port: 10100 }; + }, + ...clock, + }); + expect(live).toEqual({ port: 10100 }); + }); + + test("an immediately live proxy is caught on the first probe", async () => { + const clock = fakeClock(); + const live = await proxyStillLiveAfterStop({ + findProxy: async () => ({ port: 10100 }), + ...clock, + }); + expect(live).toEqual({ port: 10100 }); + }); +}); From 5530b8c632a524e862380a937729f0693403bddc Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 12:58:09 +0900 Subject: [PATCH 22/24] fix(service): only pay the restart-window wait where a supervisor can respawn The review caught a real regression in the previous commit: the verification loop ran its full 7-second window on EVERY `ocx service stop`, including the overwhelmingly common case where the proxy had already stopped cleanly. That trades a Windows-specific bug for a delay every macOS and Linux user pays on every stop, which is the worse defect. The wait now applies only where the failure mode exists. A Windows scheduler wrapper can survive `schtasks /end` and respawn its child seconds later -- that is what #764 reports. Nothing brings the proxy back after `launchctl unload` or `systemctl stop`, so there one probe answers the question and the command returns immediately. The masking test is fixed too. The old "genuinely stopped" case asserted `probes > 1`, which quietly required the slow path and would have kept passing while every user waited. It now names the respawn-capable case explicitly, and a new test asserts the clean path performs exactly one probe with the injected clock never advancing -- so a regression to unconditional waiting fails rather than hides. Ablation removing the platform distinction: 4 pass / 1 fail on that new test. Restored: 70 pass across service-stop-verification, service, proxy-liveness. --- src/service.ts | 12 +++++++++--- tests/service-stop-verification.test.ts | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/service.ts b/src/service.ts index d55608997..9bbef5c8b 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1635,18 +1635,24 @@ type TrackedProxyCleanupResult = "none" | "stale" | "stopped"; * native Codex on top of a running one (#764). The tracked-pid cleanup does not catch it either: * the respawned child writes a different pid, or none this process knows about. * - * Probed rather than assumed, and bounded: the observed respawn window is ~5s, so a few seconds - * of polling either sees it come back or it is genuinely gone. + * Probed rather than assumed, and bounded. The respawn risk is specific to a supervisor that can + * restart its child — the Windows scheduler wrapper — so only that case pays the restart window. + * Everywhere else a single probe answers the question, because nothing is going to bring the + * proxy back after `launchctl unload` or `systemctl stop`. Making every platform wait 7s on a + * stop that already succeeded would trade one bug for a worse everyday one. */ export async function proxyStillLiveAfterStop(deps: { findProxy?: () => Promise<{ port: number } | null>; sleep?: (ms: number) => Promise; now?: () => number; + /** Whether the stopped supervisor can respawn its child; only then is polling worth the wait. */ + canRespawn?: boolean; } = {}): Promise<{ port: number } | null> { const findProxy = deps.findProxy ?? findLiveProxy; const sleep = deps.sleep ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); const now = deps.now ?? Date.now; - const deadline = now() + 7000; + const canRespawn = deps.canRespawn ?? process.platform === "win32"; + const deadline = now() + (canRespawn ? 7000 : 0); for (;;) { try { const live = await findProxy(); diff --git a/tests/service-stop-verification.test.ts b/tests/service-stop-verification.test.ts index d52dd73e7..fa0d9bcb7 100644 --- a/tests/service-stop-verification.test.ts +++ b/tests/service-stop-verification.test.ts @@ -21,6 +21,22 @@ function fakeClock(startMs = 0) { } describe("service stop verification (#764)", () => { + test("a clean stop returns immediately when the supervisor cannot respawn", async () => { + // The regression this must not introduce: launchd and systemd do not bring a child back + // after unload/stop, so a single probe answers the question. Making every macOS and Linux + // user wait out a Windows-specific restart window would trade one bug for a worse daily one. + const clock = fakeClock(); + let probes = 0; + const live = await proxyStillLiveAfterStop({ + findProxy: async () => { probes += 1; return null; }, + canRespawn: false, + ...clock, + }); + expect(live).toBeNull(); + expect(probes).toBe(1); + expect(clock.now()).toBe(0); // no sleeping at all + }); + test("a proxy that respawns inside the restart window is detected", async () => { // The reported failure: /end succeeded, the wrapper lived, and the child came back at ~5s. // A single probe immediately after the stop command would have seen nothing and passed. @@ -31,6 +47,7 @@ describe("service stop verification (#764)", () => { probes += 1; return probes >= 5 ? { port: 10100 } : null; }, + canRespawn: true, ...clock, }); expect(live).toEqual({ port: 10100 }); @@ -44,6 +61,7 @@ describe("service stop verification (#764)", () => { let probes = 0; const live = await proxyStillLiveAfterStop({ findProxy: async () => { probes += 1; return null; }, + canRespawn: true, ...clock, }); expect(live).toBeNull(); @@ -61,6 +79,7 @@ describe("service stop verification (#764)", () => { if (probes < 4) throw new Error("connection refused"); return { port: 10100 }; }, + canRespawn: true, ...clock, }); expect(live).toEqual({ port: 10100 }); From c3424e9ae14c6f337eb1f2a36ca90044b0845435 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 13:01:26 +0900 Subject: [PATCH 23/24] test(service): pin the platform default, not just the explicit flag Every existing case passed canRespawn explicitly, so the derivation itself was untested: a regression to `?? false` would have left all five green while Windows silently lost the restart window that #764 needs. The new case drives both platforms through the default with a mocked process.platform -- Linux completes at fake-clock 0 after one probe, Windows polls past 7000. Ablation to `?? false`: 5 pass / 1 fail; restored 6/6. Worth recording why the default is right, since the review checked it properly: the launchd plist does set KeepAlive and the systemd unit uses Restart=on-failure, so both CAN restart a child. But `ocx service stop` unloads the launchd job and issues a blocking `systemctl --user stop`, and neither is the crash exit those policies react to. The Windows scheduler wrapper surviving `schtasks /end` remains the one real survivor case. --- tests/service-stop-verification.test.ts | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/service-stop-verification.test.ts b/tests/service-stop-verification.test.ts index fa0d9bcb7..53f05f34f 100644 --- a/tests/service-stop-verification.test.ts +++ b/tests/service-stop-verification.test.ts @@ -93,4 +93,36 @@ describe("service stop verification (#764)", () => { }); expect(live).toEqual({ port: 10100 }); }); + + test("the platform default drives the behavior, not just the explicit flag", async () => { + // Every other test passes canRespawn explicitly, so none of them exercises the DEFAULT -- + // a regression to `?? false` would leave them all green while Windows silently lost the + // restart window it needs. These pin the derivation itself. + const original = process.platform; + const setPlatform = (value: string) => + Object.defineProperty(process, "platform", { value, configurable: true }); + try { + setPlatform("linux"); + const linuxClock = fakeClock(); + let linuxProbes = 0; + await proxyStillLiveAfterStop({ + findProxy: async () => { linuxProbes += 1; return null; }, + ...linuxClock, + }); + expect(linuxProbes).toBe(1); + expect(linuxClock.now()).toBe(0); + + setPlatform("win32"); + const winClock = fakeClock(); + let winProbes = 0; + await proxyStillLiveAfterStop({ + findProxy: async () => { winProbes += 1; return null; }, + ...winClock, + }); + expect(winProbes).toBeGreaterThan(1); + expect(winClock.now()).toBeGreaterThanOrEqual(7000); + } finally { + Object.defineProperty(process, "platform", { value: original, configurable: true }); + } + }); }); From d380b1b0e359e9f9febd603f6f69e39e486bae1d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 31 Jul 2026 13:09:38 +0900 Subject: [PATCH 24/24] docs(plan): record the round outcome, including what was wrong Closes the unit with what actually happened rather than what was planned. Three of this round's own mistakes are recorded, because they are the reusable part: the #796 fix started as a placeholder space with no evidence behind it and would have 400'd live; the #764 fix introduced a 7-second wait on every stop for every platform; and the first #758 test drove a branch that delegates to a helper already consulting the registry, so its ablation could not fail. An independent reviewer caught all three, and making ablation a habit is what surfaced the last two. Also records the disposition of everything left open with the reason -- owner judgement, security review, bridge-level work, missing provider evidence, or its own cycle -- so the next round starts from a list rather than a re-triage. --- .../040_round_outcome.md | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 devlog/_plan/260731_pr_merge_round/040_round_outcome.md diff --git a/devlog/_plan/260731_pr_merge_round/040_round_outcome.md b/devlog/_plan/260731_pr_merge_round/040_round_outcome.md new file mode 100644 index 000000000..412c125ce --- /dev/null +++ b/devlog/_plan/260731_pr_merge_round/040_round_outcome.md @@ -0,0 +1,85 @@ +# 040 — 라운드 결과 + +`000`이 세운 판정 매트릭스가 실제로 어떻게 끝났는지 기록한다. 계획과 다른 부분이 +있고, 다른 이유가 더 중요하다. + +기준: 브랜치 `codex/260731-pr-merge-round`, `origin/dev`에서 분기. +게이트: macmini-cf에서 6240 pass / 0 fail. + +## 닫은 것 + +### 이미 dev에 있던 7건 (`010`) + +#736 #752 #743 #610 #734 #777 #533. 전부 근거 커밋을 명시하고 닫았다. +#736은 머지했으면 `decodeSchtasksOutput()`을 지워서 #722를 다시 열었을 것이다. + +이슈: #722 #733 #606 #721 닫음. #572 #759는 부분이라 유지. + +### 랜딩 후 닫은 6건 + +| PR | 커밋 | 손댄 것 | +|---|---|---| +| #774 | `bfa1a599c` | 없음 | +| #772 | `5d7ab0cb7` | 없음 | +| #783 | `f72610027` | 없음 | +| #769 | `0856b0cb7` | 없음 | +| #768 | `ba77eb8e9` | 디렉터리 가드, PATH 케이싱 테스트 | +| #771 | `13a76a38a` | 없음 (#768이 충돌 해소) | + +이슈: #754 #709 #767 #696 닫음. + +### 우리가 다시 만든 것 + +- **#725** (`a6d472e3f`) — #790은 `/api/logs` 계약을 깨고 테스트가 무력했다. + `/api/settings`만 써서 배열 계약과 GUI 레이아웃을 둘 다 보존. #790 닫음. +- **#796** (`2eebd9268`) — Ark 구조화 content. **실제 엔드포인트 미검증.** +- **#735 절반** (`38f711beb`) — 잘린 tool call이 완료로 보고되던 것. +- **#758** (`3f7a50458`) — registry 기본값을 fetch 경계에서도 존중. +- **#764 절반** (`2d0c6a99c`, `5530b8c63`, `c3424e9ae`) — stop 검증. + +## 열어둔 것과 이유 + +**오너 판단 필요**: #735 나머지 절반(#773이 왜 리버트됐는지 기록 없음), +#793(같은 이유). + +**보안 리뷰 필요**: #766/#782, #760/#779. 둘 다 수용 경로를 넓힌다. +#782에는 별개 버그도 있다 — 디렉터리 하드닝 결과가 버려져 `/api/settings`가 +미검증을 verified로 보고할 수 있다. + +**bridge 작업 필요**: #658, #765 잔여. `021` 참고. + +**preset 증거 미달**: #751(증거 완비, 리뷰만 갱신하면 됨), #747 #653 #611 #776. + +**자체 사이클 필요**: #757 #581 #715 #707 #671 #569 #557 #644 #746 #750 #745 #763. + +## 이 라운드에서 틀렸던 것 세 가지 + +기록해두는 게 낫다. 같은 실수를 반복하지 않으려면. + +**1. #796을 공백으로 고치려 했다.** "빈 문자열이 거부되니 non-empty면 되겠지"로 +갔는데 근거가 없었다. 에러가 가리키는 `input.content.text`는 중첩 경로고, +구조화된 배열을 요구하는 것이었다. 로컬 테스트는 통과했을 것이고 실제 +엔드포인트에서 그대로 400이 났을 것이다. 지금도 미검증이라 그렇게 표기했다. + +**2. #764 수정이 회귀를 만들었다.** 검증 루프가 모든 플랫폼에서 매번 7초를 +기다리게 했다. launchd/systemd는 unload 후 자식을 되살리지 않으므로 Windows +전용 문제 때문에 나머지 사용자 전원이 매번 7초를 물 뻔했다. + +**3. #758 첫 테스트가 무력했다.** ablation이 실패하지 않아 확인해보니 주입된 +executor 분기를 타고 있었고, 그 분기는 이미 registry를 보는 헬퍼에 위임한다. +실제 discovery 경로를 타도록 다시 썼다. + +셋 다 독립 리뷰어가 잡았다. ablation을 습관으로 돌린 것이 2번과 3번을 드러냈다. + +## 배운 것 + +**"CLEAN하게 머지된다"가 "머지해도 된다"가 아니다.** #736이 그 증거다. +patch-id가 다른데 결함은 없어진 경우 — 메인테이너가 같은 문제를 다르게 고친 +것이고, 이때 PR을 머지하면 더 나은 구현을 되돌린다. + +**테스트가 통과하는 것과 결함을 잡는 것은 다르다.** #790의 테스트는 두 응답 +형태를 다 허용해서 프로덕션 변경을 되돌려도 통과했다. ablation 없이는 안 보인다. + +**같은 결함을 두 번 못 고치면 멈춰야 한다.** #765 스트리밍 검증에서 두 번 +거부당한 뒤 세 번째를 시도하지 않고 되돌렸다. 진짜 수정 지점이 bridge였고 +그건 어댑터 PR의 범위가 아니었다.