From dc074672ebb3d146e60fec7f1faa1ca7e654fe02 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 09:09:20 +0900 Subject: [PATCH 1/2] fix(claude): fall back to native launch when routing is off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ocx claude` hard-errored and returned 1 whenever Claude routing was disabled (`src/cli/claude.ts:420` on dev), so the command was unusable with the Claude toggle off even though a native `claude` binary was available. Launch natively in that case instead. Only an explicit `false` triggers the fallback — from config, or reported live by `GET /api/claude-code` — so a proxy predating the `enabled` field stays routed, and an absent proxy still starts one rather than falling back. The native session must not inherit proxy state, so it removes only values it can prove OpenCodex owns: `ANTHROPIC_BASE_URL` when it targets this proxy's own loopback host and configured port with a proxy-issued admission token, the `CLAUDE_CODE_*` discovery and auto-context levers, and model slots that resolve only through the proxy. An unrelated `http://localhost:8080` gateway and a user `sk-ant-` credential are preserved. Client-ownership preflight runs before any fallback, so an invalid or mismatched connected client still fails closed. Three fixes on top of the contributor's head: - Sync all eight `docs-site` `guides/claude-code.md` pages, which still described `ocx claude` as proxy-only. - Distinguish an absent `settings.json` from a corrupt one in `readPickerDefaultModel`. Swallowing both alike dropped the "saved model requires the proxy" warning exactly when the file was broken; a corrupt file now warns and names the path without echoing contents. - Restore the `#764 / SERVICE_STOP_LIVENESS` rationale comment above `ensureProxyForClaude`, which the diff deleted while keeping the behavior. Carry of #3519. Co-authored-by: everton-dgn <58889432+everton-dgn@users.noreply.github.com> --- .../src/content/docs/fr/guides/claude-code.md | 26 ++ .../src/content/docs/guides/claude-code.md | 25 ++ .../src/content/docs/ja/guides/claude-code.md | 27 ++ .../src/content/docs/ko/guides/claude-code.md | 25 ++ .../src/content/docs/ru/guides/claude-code.md | 28 ++ .../src/content/docs/tr/guides/claude-code.md | 31 ++ .../content/docs/zh-cn/guides/claude-code.md | 23 ++ .../content/docs/zh-tw/guides/claude-code.md | 23 ++ src/cli/claude.ts | 271 +++++++++++++++--- src/cli/registry.ts | 5 +- tests/claude-integration/claude-cli.test.ts | 133 ++++++++- 11 files changed, 575 insertions(+), 42 deletions(-) diff --git a/docs-site/src/content/docs/fr/guides/claude-code.md b/docs-site/src/content/docs/fr/guides/claude-code.md index 797f9d3554..9899797ea6 100644 --- a/docs-site/src/content/docs/fr/guides/claude-code.md +++ b/docs-site/src/content/docs/fr/guides/claude-code.md @@ -67,6 +67,32 @@ ignore les identifiants Anthropic introduits uniquement par un fichier dotenv du dans votre shell reste toujours prioritaire, quel que soit le mode d'authentification. Pour utiliser volontairement une clé API, exportez-la (`export ANTHROPIC_API_KEY=...`) au lieu de la laisser dans un fichier de projet. +### Lancement natif de repli quand le routage Claude est désactivé + +`ocx claude` échouait auparavant avec une erreur lorsque le routage Claude était désactivé. Il lance +désormais le binaire natif `claude` à la place, de sorte que la commande reste utile routage coupé : + +| Où le routage est désactivé | Ce qui se passe | +| --- | --- | +| `claudeCode.enabled: false` dans la configuration | Lancement natif, avec un avis indiquant que le routage est désactivé | +| Le proxy en cours renvoie `enabled: false` depuis `GET /api/claude-code` | Lancement natif, avec un avis de redémarrer le service après réactivation | +| `claudeCode.enabled` absent ou `true` | Routage par le proxy, inchangé | + +Seul un `false` explicite déclenche le repli : un proxy antérieur à ce champ reste donc routé. Un proxy +absent n'est pas non plus un déclencheur — routage activé, `ocx claude` démarre toujours le proxy. + +Une session native ne doit pas hériter de l'état du proxy. Le repli supprime donc uniquement les valeurs +dont OpenCodex peut **prouver** la propriété : `ANTHROPIC_BASE_URL` seulement lorsqu'elle pointe vers +l'adresse de bouclage et le port configuré de ce proxy *et* que le jeton d'admission associé a bien été +émis par lui ; les leviers `CLAUDE_CODE_*` de découverte et d'auto-contexte ; et les emplacements de +modèle qui ne se résolvent qu'à travers le proxy (alias routés et identifiants `provider/model`). Tout +le reste vous appartient et est préservé — une passerelle `http://localhost:8080` sans rapport et vos +propres identifiants `sk-ant-` survivent tous les deux. + +Si le modèle par défaut enregistré dans le sélecteur `/model` est réservé au proxy, la session native +bascule sur `claudeCode.model` lorsque celui-ci est utilisable nativement, et vous avertit sinon de +passer `--model `. Un argument `--model` explicite l'emporte toujours. + ## Mode d'authentification Claude Code a besoin d'un jeton dans `ANTHROPIC_AUTH_TOKEN` pour communiquer avec une passerelle, mais définir cette diff --git a/docs-site/src/content/docs/guides/claude-code.md b/docs-site/src/content/docs/guides/claude-code.md index 264d5d6fea..9dafc37c21 100644 --- a/docs-site/src/content/docs/guides/claude-code.md +++ b/docs-site/src/content/docs/guides/claude-code.md @@ -72,6 +72,31 @@ ignores Anthropic credentials that only a project dotenv introduced. A value you your shell still wins, in every auth mode. To use an API key deliberately, export it (`export ANTHROPIC_API_KEY=...`) rather than leaving it in a project file. +### Native fallback when Claude routing is off + +`ocx claude` used to exit with an error when Claude routing was disabled. It now launches the +native `claude` binary instead, so the command stays useful with routing off: + +| Where routing is off | What happens | +| --- | --- | +| `claudeCode.enabled: false` in config | Native launch, with a notice that routing is disabled | +| The running proxy reports `enabled: false` from `GET /api/claude-code` | Native launch, with a notice to restart the service after re-enabling | +| `claudeCode.enabled` absent or `true` | Routed through the proxy, unchanged | + +Only an explicit `false` triggers the fallback, so a proxy predating the field stays routed. A +missing proxy is not a trigger either — with routing on, `ocx claude` still starts the proxy. + +A native session must not inherit proxy state, so the fallback removes values it can **prove** +OpenCodex owns: `ANTHROPIC_BASE_URL` only when it points at this proxy's own loopback address +and configured port *and* the paired admission token is one the proxy issued; the +`CLAUDE_CODE_*` discovery and auto-context levers; and model slots that only resolve through the +proxy (routed aliases and `provider/model` ids). Anything else is yours and is preserved — an +unrelated `http://localhost:8080` gateway and your own `sk-ant-` credential both survive. + +If your saved `/model` picker default is a proxy-only model, the native session falls back to +`claudeCode.model` when that is natively usable, and otherwise warns you to pass +`--model `. An explicit `--model` argument always wins. + ## Auth mode Claude Code needs a token in `ANTHROPIC_AUTH_TOKEN` to talk to a gateway, but setting that diff --git a/docs-site/src/content/docs/ja/guides/claude-code.md b/docs-site/src/content/docs/ja/guides/claude-code.md index bc7e84e39d..6d45bf63af 100644 --- a/docs-site/src/content/docs/ja/guides/claude-code.md +++ b/docs-site/src/content/docs/ja/guides/claude-code.md @@ -28,6 +28,33 @@ ocx claude | `CLAUDE_CODE_MAX_CONTEXT_TOKENS` / `DISABLE_COMPACT` | `maxContextTokens` が設定された場合の従来コンテキスト上書き値 (条件付き) | 直接 export した変数が常に優先します。追加引数はそのまま渡されます: `ocx claude -p "hello"`。 +### Claude ルーティングが無効なときのネイティブフォールバック + +以前は Claude ルーティングが無効だと `ocx claude` はエラーで終了していました。現在は代わりに +ネイティブの `claude` バイナリを起動するため、ルーティングを切ったままでもこのコマンドを使えます。 + +| ルーティングが無効な場所 | 動作 | +| --- | --- | +| 設定の `claudeCode.enabled: false` | ルーティングが無効である旨の通知とともにネイティブ起動 | +| 実行中のプロキシが `GET /api/claude-code` で `enabled: false` を返す | ネイティブ起動 + 有効化後にサービスを再起動する案内 | +| `claudeCode.enabled` が無い、または `true` | 従来どおりプロキシ経由でルーティング | + +明示的な `false` のみがフォールバックの条件なので、このフィールドを持たない古いプロキシは +ルーティングのままです。プロキシが無いことも条件ではありません — ルーティングが有効なら +`ocx claude` はこれまでどおりプロキシを起動します。 + +ネイティブセッションがプロキシの状態を引き継いではならないため、フォールバックは OpenCodex の +所有だと**証明できる**値だけを削除します。`ANTHROPIC_BASE_URL` はこのプロキシ自身のループバック +アドレスと設定済みポートを指し、かつ対になる admission トークンがプロキシの発行したものである場合 +のみ削除します。加えて `CLAUDE_CODE_*` の検出・自動コンテキスト用スイッチと、プロキシ経由でしか +解決しないモデルスロット(ルーティング用エイリアスと `provider/model` 形式)も削除します。それ以外 +はあなたの値なので保持されます — 無関係な `http://localhost:8080` ゲートウェイと自分の +`sk-ant-` 資格情報はどちらも残ります。 + +保存された `/model` ピッカーの既定値がプロキシ専用モデルの場合、`claudeCode.model` がネイティブ +で使えるならそれにフォールバックし、使えなければ `--model ` を渡すよう警告します。 +明示的な `--model` 引数が常に優先します。 + ## システム環境統合(macOS) `claudeCode.systemEnv` を `true` に設定すると(デフォルト: **オフ`)`ocx start` が `launchctl setenv` を diff --git a/docs-site/src/content/docs/ko/guides/claude-code.md b/docs-site/src/content/docs/ko/guides/claude-code.md index 0183b5b72f..a7ed659c59 100644 --- a/docs-site/src/content/docs/ko/guides/claude-code.md +++ b/docs-site/src/content/docs/ko/guides/claude-code.md @@ -28,6 +28,31 @@ ocx claude | `CLAUDE_CODE_MAX_CONTEXT_TOKENS` / `DISABLE_COMPACT` | `maxContextTokens`가 설정된 경우 기존 컨텍스트 재정의 값 (조건부) | 직접 내보낸 변수가 항상 우선해요. 추가 인자는 그대로 전달돼요: `ocx claude -p "hello"`. +### Claude 라우팅이 꺼져 있을 때의 네이티브 폴백 + +예전에는 Claude 라우팅이 꺼져 있으면 `ocx claude`가 오류를 내고 종료했어요. 이제는 네이티브 +`claude` 실행 파일을 대신 실행하므로, 라우팅을 꺼 둔 상태에서도 이 명령을 그대로 쓸 수 있어요. + +| 라우팅이 꺼진 위치 | 동작 | +| --- | --- | +| 설정의 `claudeCode.enabled: false` | 라우팅이 비활성화되었다는 안내와 함께 네이티브 실행 | +| 실행 중인 프록시가 `GET /api/claude-code`에서 `enabled: false`를 보고 | 네이티브 실행 + 라우팅을 켠 뒤 서비스를 재시작하라는 안내 | +| `claudeCode.enabled`가 없거나 `true` | 기존과 동일하게 프록시로 라우팅 | + +명시적인 `false`만 폴백을 유발하므로, 이 필드를 모르는 예전 프록시는 계속 라우팅돼요. 프록시가 +없는 것도 폴백 조건이 아니에요 — 라우팅이 켜져 있으면 `ocx claude`가 프록시를 그대로 띄워요. + +네이티브 세션이 프록시 상태를 물려받으면 안 되므로, 폴백은 OpenCodex 소유임을 **증명할 수 있는** +값만 제거해요. `ANTHROPIC_BASE_URL`은 이 프록시의 루프백 주소와 설정된 포트를 정확히 가리키고 +짝이 되는 admission 토큰도 프록시가 발급한 것일 때만 제거하고, `CLAUDE_CODE_*` 검색·자동 컨텍스트 +레버와 프록시를 거쳐야만 해석되는 모델 슬롯(라우팅 별칭과 `provider/model` 형식)도 제거해요. +그 밖의 값은 사용자 것이라 그대로 유지돼요 — 관련 없는 `http://localhost:8080` 게이트웨이와 +직접 설정한 `sk-ant-` 자격 증명은 둘 다 살아남아요. + +저장된 `/model` 선택기 기본값이 프록시 전용 모델이면, `claudeCode.model`이 네이티브에서 쓸 수 +있을 때 그 값으로 대체하고, 그렇지 않으면 `--model `을 넘기라고 경고해요. +명시적인 `--model` 인자가 항상 우선해요. + ## 인증 모드 Claude Code가 게이트웨이와 통신하려면 `ANTHROPIC_AUTH_TOKEN`에 토큰이 필요해요. 그런데 이 변수를 diff --git a/docs-site/src/content/docs/ru/guides/claude-code.md b/docs-site/src/content/docs/ru/guides/claude-code.md index b04612625b..3f4285cfe6 100644 --- a/docs-site/src/content/docs/ru/guides/claude-code.md +++ b/docs-site/src/content/docs/ru/guides/claude-code.md @@ -29,6 +29,34 @@ ocx claude | `CLAUDE_CODE_MAX_CONTEXT_TOKENS` / `DISABLE_COMPACT` | Устаревшее переопределение контекста, когда задан `maxContextTokens` (условно) | Переменные, которые вы экспортируете сами, всегда имеют приоритет. Дополнительные аргументы передаются как есть: `ocx claude -p "hello"`. +### Нативный запасной запуск, когда маршрутизация Claude выключена + +Раньше `ocx claude` завершался с ошибкой, если маршрутизация Claude была выключена. Теперь вместо +этого запускается нативный бинарник `claude`, поэтому команда остаётся полезной и с выключенной +маршрутизацией: + +| Где выключена маршрутизация | Что происходит | +| --- | --- | +| `claudeCode.enabled: false` в конфигурации | Нативный запуск с уведомлением, что маршрутизация выключена | +| Запущенный прокси возвращает `enabled: false` из `GET /api/claude-code` | Нативный запуск и совет перезапустить службу после включения | +| `claudeCode.enabled` отсутствует или равен `true` | Маршрутизация через прокси, без изменений | + +Запасной запуск включает только явное `false`, поэтому прокси, не знающий об этом поле, остаётся +маршрутизируемым. Отсутствие прокси тоже не является триггером — при включённой маршрутизации +`ocx claude` по-прежнему запускает прокси. + +Нативная сессия не должна наследовать состояние прокси, поэтому запасной запуск удаляет только те +значения, принадлежность которых OpenCodex может **доказать**: `ANTHROPIC_BASE_URL` — лишь когда он +указывает на собственный локальный адрес и настроенный порт этого прокси, а парный admission-токен +выдан самим прокси; переключатели обнаружения и автоконтекста `CLAUDE_CODE_*`; и слоты моделей, +которые разрешаются только через прокси (маршрутные псевдонимы и форма `provider/model`). Всё +остальное — ваше и сохраняется: посторонний шлюз `http://localhost:8080` и ваши собственные +учётные данные `sk-ant-` остаются на месте. + +Если сохранённый выбор в селекторе `/model` — модель только для прокси, нативная сессия перейдёт на +`claudeCode.model`, когда та доступна нативно, а иначе предупредит передать +`--model <модель Anthropic>`. Явный аргумент `--model` всегда имеет приоритет. + ## Интеграция с системным окружением (macOS) Когда `claudeCode.systemEnv` установлен в `true` (по умолчанию: **выключено**), `ocx start` diff --git a/docs-site/src/content/docs/tr/guides/claude-code.md b/docs-site/src/content/docs/tr/guides/claude-code.md index bf955fba97..2b092dc621 100644 --- a/docs-site/src/content/docs/tr/guides/claude-code.md +++ b/docs-site/src/content/docs/tr/guides/claude-code.md @@ -80,6 +80,37 @@ kimlik doğrulama modunda her zaman geçerlidir. Bir API anahtarını kasıtlı kullanmak için, onu bir proje dosyasında bırakmak yerine dışa aktarın (`export ANTHROPIC_API_KEY=...`). +### Claude yönlendirmesi kapalıyken yerel geri dönüş + +`ocx claude` eskiden Claude yönlendirmesi kapalıyken hata vererek çıkardı. +Artık bunun yerine yerel `claude` ikili dosyasını başlatır; böylece komut, +yönlendirme kapalıyken de kullanışlı kalır: + +| Yönlendirmenin kapalı olduğu yer | Ne olur | +| --- | --- | +| Yapılandırmada `claudeCode.enabled: false` | Yönlendirmenin kapalı olduğunu bildiren bir uyarıyla yerel başlatma | +| Çalışan vekil `GET /api/claude-code` üzerinden `enabled: false` bildiriyor | Yerel başlatma ve yeniden etkinleştirdikten sonra servisi yeniden başlatma önerisi | +| `claudeCode.enabled` yok veya `true` | Değişmeden vekil üzerinden yönlendirme | + +Geri dönüşü yalnızca açık bir `false` tetikler; bu alandan önceki bir vekil +yönlendirilmiş kalır. Vekilin bulunmaması da bir tetikleyici değildir — +yönlendirme açıkken `ocx claude` vekili yine başlatır. + +Yerel bir oturum vekil durumunu devralmamalıdır; bu nedenle geri dönüş yalnızca +OpenCodex'in sahipliğini **kanıtlayabildiği** değerleri kaldırır: +`ANTHROPIC_BASE_URL` yalnızca bu vekilin kendi geri döngü adresini ve +yapılandırılmış bağlantı noktasını gösteriyorsa *ve* eşlenmiş kabul belirteci +vekilin verdiği bir belirteçse; `CLAUDE_CODE_*` keşif ve otomatik bağlam +anahtarları; ve yalnızca vekil üzerinden çözülen model yuvaları (yönlendirme +takma adları ve `provider/model` kimlikleri). Geri kalan her şey sizindir ve +korunur — ilgisiz bir `http://localhost:8080` ağ geçidi ve kendi `sk-ant-` +kimlik bilginiz birlikte hayatta kalır. + +Kaydedilmiş `/model` seçici varsayılanınız yalnızca vekile özgü bir modelse, +yerel oturum `claudeCode.model` yerel olarak kullanılabildiğinde ona döner; +aksi hâlde `--model ` geçmeniz için uyarır. Açık bir +`--model` argümanı her zaman kazanır. + ## Kimlik doğrulama modu (Auth mode) Claude Code'un bir ağ geçidiyle konuşabilmesi için `ANTHROPIC_AUTH_TOKEN` içinde diff --git a/docs-site/src/content/docs/zh-cn/guides/claude-code.md b/docs-site/src/content/docs/zh-cn/guides/claude-code.md index e5140cb8a7..e2db5ddec4 100644 --- a/docs-site/src/content/docs/zh-cn/guides/claude-code.md +++ b/docs-site/src/content/docs/zh-cn/guides/claude-code.md @@ -28,6 +28,29 @@ ocx claude | `CLAUDE_CODE_MAX_CONTEXT_TOKENS` / `DISABLE_COMPACT` | 设置 `maxContextTokens` 时使用的旧版上下文覆盖项（条件注入） | 你自行导出的变量始终优先。额外参数会直接透传：`ocx claude -p "hello"`。 +### Claude 路由关闭时的原生回退 + +以前当 Claude 路由被关闭时，`ocx claude` 会直接报错退出。现在它会改为启动原生 `claude` +可执行文件，因此在关闭路由的情况下该命令依然可用： + +| 路由关闭的位置 | 行为 | +| --- | --- | +| 配置中的 `claudeCode.enabled: false` | 原生启动，并提示路由已被禁用 | +| 运行中的代理在 `GET /api/claude-code` 中返回 `enabled: false` | 原生启动，并提示重新启用后重启服务 | +| `claudeCode.enabled` 缺失或为 `true` | 与以往一致，经代理路由 | + +只有显式的 `false` 才会触发回退，因此早于该字段的旧代理仍会保持路由。代理缺失同样不是触发条件 +——只要路由是开启的，`ocx claude` 仍会照常启动代理。 + +原生会话不应继承代理状态，因此回退只移除能够**证明**属于 OpenCodex 的值：仅当 +`ANTHROPIC_BASE_URL` 指向本代理自身的回环地址与配置端口、且配对的 admission 令牌确实由代理签发 +时才移除；此外还会移除 `CLAUDE_CODE_*` 的发现与自动上下文开关，以及只能经由代理解析的模型槽位 +（路由别名与 `provider/model` 形式）。其余都属于你自己的配置并被保留——无关的 +`http://localhost:8080` 网关和你自己的 `sk-ant-` 凭据都会保留。 + +如果保存的 `/model` 选择器默认值是仅限代理的模型，当 `claudeCode.model` 可在原生环境使用时会 +回退到它，否则会警告你传入 `--model `。显式的 `--model` 参数始终优先。 + ## 系统环境集成（macOS） 当 `claudeCode.systemEnv` 设置为 `true`（默认：**关闭**）时，`ocx start` 会使用 `launchctl setenv` diff --git a/docs-site/src/content/docs/zh-tw/guides/claude-code.md b/docs-site/src/content/docs/zh-tw/guides/claude-code.md index 7aea715953..f07b2f97c6 100644 --- a/docs-site/src/content/docs/zh-tw/guides/claude-code.md +++ b/docs-site/src/content/docs/zh-tw/guides/claude-code.md @@ -54,6 +54,29 @@ ocx claude | `CLAUDE_CODE_MAX_CONTEXT_TOKENS` / `DISABLE_COMPACT` | 設定 `maxContextTokens` 時使用的舊版上下文覆蓋項（條件注入） | 你自行匯出的變數始終優先。額外引數會直接透傳：`ocx claude -p "hello"`。 +### Claude 路由關閉時的原生回退 + +以前當 Claude 路由被關閉時，`ocx claude` 會直接報錯結束。現在它會改為啟動原生 `claude` +執行檔，因此在關閉路由的情況下該指令仍然可用： + +| 路由關閉的位置 | 行為 | +| --- | --- | +| 設定中的 `claudeCode.enabled: false` | 原生啟動，並提示路由已停用 | +| 執行中的代理在 `GET /api/claude-code` 回傳 `enabled: false` | 原生啟動，並提示重新啟用後重啟服務 | +| `claudeCode.enabled` 缺少或為 `true` | 與以往一致，經代理路由 | + +只有明確的 `false` 才會觸發回退，因此早於該欄位的舊代理仍會維持路由。代理不存在同樣不是觸發 +條件——只要路由是開啟的，`ocx claude` 仍會照常啟動代理。 + +原生工作階段不應繼承代理狀態，因此回退只移除能夠**證明**屬於 OpenCodex 的值：僅當 +`ANTHROPIC_BASE_URL` 指向本代理自身的回送位址與設定連接埠、且配對的 admission token 確實由代理 +簽發時才移除；此外還會移除 `CLAUDE_CODE_*` 的探索與自動上下文開關，以及只能經由代理解析的模型 +槽位（路由別名與 `provider/model` 形式）。其餘都屬於你自己的設定並會保留——無關的 +`http://localhost:8080` 閘道器和你自己的 `sk-ant-` 憑證都會保留。 + +若儲存的 `/model` 選擇器預設值是僅限代理的模型，當 `claudeCode.model` 可在原生環境使用時會 +回退到它，否則會警告你傳入 `--model `。明確的 `--model` 引數始終優先。 + ## 認證模式 Claude Code 需要在 `ANTHROPIC_AUTH_TOKEN` 中有 token 才能與閘道器通訊，但設定該變數也會停用 diff --git a/src/cli/claude.ts b/src/cli/claude.ts index a9e64fc1af..446485e2b7 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -1,5 +1,6 @@ /** - * `ocx claude [claude args...]` — launch Claude Code wired to the local proxy. + * `ocx claude [claude args...]` — launch Claude Code through the local proxy, + * or natively when Claude routing is explicitly disabled. * * Mirrors `ccr code` UX (devlog/260711_claude_inbound/020, 003 E1/E2/E5/G1): * ensures the proxy is running, injects the Anthropic env slots, then execs the @@ -9,8 +10,9 @@ import { spawn } from "node:child_process"; import { loadConfig } from "../config"; import { injectClaudeAgentDefs } from "../claude/agents-inject"; +import { CLAUDE_ALIAS_PREFIX_V1, CLAUDE_ALIAS_PREFIX_V2 } from "../claude/alias"; import { effectiveModelEnv, resolveAutoContext } from "../claude/context-windows"; -import { refreshGatewayModelCacheFromProxy } from "../claude/gateway-cache"; +import { claudeConfigDir, refreshGatewayModelCacheFromProxy } from "../claude/gateway-cache"; import { commandInvocation } from "../lib/win-exec"; import { isProxyAdmissionSecret } from "../server/auth-cors"; import { findLiveProxy } from "../server/proxy-liveness"; @@ -21,10 +23,11 @@ import { resolveClaudeAuthMode } from "../claude/auth-mode"; import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; import { selfLaunchArgv } from "../lib/self-launch-argv"; import { ANTHROPIC_PARENT_ENV_SLOTS, trustedNodeLauncherContext, type AnthropicParentEnvSlot } from "./launcher-context"; -import { readClientConnectionState } from "../client/state"; -import { readServiceApiTokenState } from "../lib/service-secrets"; +import { readClientConnectionState, type ClientConnectionState } from "../client/state"; +import { readServiceApiTokenState, type ServiceApiTokenState } from "../lib/service-secrets"; import { DEFAULT_CATALOG_PATH } from "../codex/paths"; import { readFileSync } from "node:fs"; +import { join } from "node:path"; import { aliasForNative, aliasForRoute } from "../claude/alias"; import { desktop3pAlias } from "../claude/desktop-3p"; @@ -49,6 +52,48 @@ export type ClaudeEnvDeps = { allowRootSkipPermissions?: boolean; }; +function deleteUntrustedAnthropicSlots(env: ClaudeLaunchEnv, deps: ClaudeEnvDeps): void { + const explicitSlots = deps.preBunAnthropicSlots; + const trustedSlots = explicitSlots === undefined + ? trustedNodeLauncherContext()?.anthropicEnvSlots ?? [] + : explicitSlots ?? []; + const exported = new Set(trustedSlots); + for (const name of ANTHROPIC_PARENT_ENV_SLOTS) { + const value = env[name]; + if (value !== undefined && value !== "" && !exported.has(name)) delete env[name]; + } + delete env.OCX_PRE_BUN_ANTHROPIC_ENV; + delete env.OCX_NODE_LAUNCH_CONTEXT; +} + +/** + * Read Claude Code's own persisted `/model` picker default. + * + * An absent `settings.json` is the ordinary fresh-install case and stays silent. A + * present-but-unparseable one is not: swallowing it would drop the "saved model requires + * the proxy" warning exactly when the file is broken, so the native session would start + * on a model the user never chose with no explanation. Name the file, never its contents. + */ +export function readPickerDefaultModel(configDir: string): string | null { + const file = join(configDir, "settings.json"); + let raw: string; + try { + raw = readFileSync(file, "utf8"); + } catch (err) { // no-excuse-ok: catch -- an absent picker settings file is the default install state. + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + console.warn(`⚠ Could not read Claude Code settings at ${file}; the saved model check is skipped this run.`); + } + return null; + } + try { + const parsed = JSON.parse(raw) as Record; + return typeof parsed.model === "string" && parsed.model.trim() !== "" ? parsed.model.trim() : null; + } catch { // no-excuse-ok: catch -- a corrupt picker file must warn, not abort the launch. + console.warn(`⚠ Claude Code settings at ${file} are not valid JSON; the saved model check is skipped this run.`); + return null; + } +} + function isClaudeLoopbackHostname(hostname: string): boolean { const normalized = hostname.toLowerCase().replace(/\.$/, ""); return normalized === "localhost" @@ -128,18 +173,7 @@ export function buildClaudeEnv( // Direct `bun src/cli/index.ts` therefore loses ambient Anthropic values. That is a // real cost to a documented entry point, and the escape hatch is the launcher: run // through `ocx` (the published bin) and genuine shell exports are preserved by proof. - const explicitSlots = deps.preBunAnthropicSlots; - const trustedSlots = explicitSlots === undefined - ? trustedNodeLauncherContext()?.anthropicEnvSlots ?? [] - : explicitSlots ?? []; - const exported = new Set(trustedSlots); - for (const name of ANTHROPIC_PARENT_ENV_SLOTS) { - const value = env[name]; - if (value !== undefined && value !== "" && !exported.has(name)) delete env[name]; - } - // Never forward old or current provenance seams to Claude Code. - delete env.OCX_PRE_BUN_ANTHROPIC_ENV; - delete env.OCX_NODE_LAUNCH_CONTEXT; + deleteUntrustedAnthropicSlots(env, deps); const setDefault = (name: string, value: string | undefined) => { if (value === undefined || value.length === 0) return; if (env[name] !== undefined && env[name] !== "") return; // user wins @@ -302,7 +336,12 @@ export function buildClaudeEnv( * daemon registers every selector form — audit R3#1). 3s bound + management auth header. * (no [1m] marking, conservative). */ -export async function fetchClaudeContextWindows(config: OcxConfig, port: number, timeoutMs = 3_000): Promise> { +export interface ClaudeCodeLiveState { + contextWindows: Record; + enabled?: boolean; +} + +export async function fetchClaudeCodeState(config: OcxConfig, port: number, timeoutMs = 3_000): Promise { try { const headers = new Headers(); const token = configuredAdminToken(); @@ -311,15 +350,22 @@ export async function fetchClaudeContextWindows(config: OcxConfig, port: number, headers, signal: AbortSignal.timeout(timeoutMs), }); - if (!res.ok) return {}; - const body = await res.json() as { contextWindows?: Record }; - return body.contextWindows && typeof body.contextWindows === "object" ? body.contextWindows : {}; + if (!res.ok) return { contextWindows: {} }; + const body = await res.json() as { contextWindows?: Record; enabled?: boolean }; + return { + contextWindows: body.contextWindows && typeof body.contextWindows === "object" ? body.contextWindows : {}, + ...(typeof body.enabled === "boolean" ? { enabled: body.enabled } : {}), + }; } catch { console.error("⚠ 모델 컨텍스트 정보를 불러오지 못했습니다 — 1M 자동 표시는 이번 실행에서 생략됩니다."); - return {}; + return { contextWindows: {} }; } } +export async function fetchClaudeContextWindows(config: OcxConfig, port: number, timeoutMs = 3_000): Promise> { + return (await fetchClaudeCodeState(config, port, timeoutMs)).contextWindows; +} + export function readConnectedClaudeContextWindows(path = DEFAULT_CATALOG_PATH): Record { try { const parsed = JSON.parse(readFileSync(path, "utf8")) as { models?: unknown }; @@ -384,6 +430,139 @@ export async function ensureProxyForClaude(deps: ClaudeProxyEnsureDeps = {}): Pr return null; } +export const CLAUDE_NATIVE_ROUTING_OFF = + "ℹ️ Claude Code routing is disabled in OpenCodex. Launching Claude Code natively. Enable Claude routing to use the proxy again."; + +export const CLAUDE_NATIVE_LIVE_DISABLED = + "ℹ️ The running OpenCodex proxy has Claude Code routing disabled. Launching Claude Code natively. Restart the service after enabling routing."; + +export type ClaudeLaunchPlan = + | { kind: "routed" } + | { kind: "native"; notice: string }; + +export function claudeLaunchPlan( + configuredEnabled: boolean, + liveEnabled: boolean | undefined, +): ClaudeLaunchPlan { + if (!configuredEnabled) return { kind: "native", notice: CLAUDE_NATIVE_ROUTING_OFF }; + if (liveEnabled === false) return { kind: "native", notice: CLAUDE_NATIVE_LIVE_DISABLED }; + return { kind: "routed" }; +} + +export type ClaudeLaunchPreflight = + | { kind: "continue" } + | { kind: "native"; notice: string } + | { kind: "error"; message: string }; + +/** Validate connected-client ownership before any native fallback can run. */ +export function claudeLaunchPreflight( + configuredEnabled: boolean, + clientState: ClientConnectionState, + tokenState?: ServiceApiTokenState, +): ClaudeLaunchPreflight { + if (clientState.kind === "invalid" || clientState.kind === "mismatched") { + return { kind: "error", message: `Client state is ${clientState.kind}: ${clientState.reason}` }; + } + if (clientState.kind === "connected") { + if (!clientState.value.selectedClients.includes("claude")) { + return { kind: "error", message: "Claude is not selected for this remote hub connection." }; + } + if (tokenState?.kind !== "present" || tokenState.fingerprint !== clientState.value.tokenFingerprint) { + return { + kind: "error", + message: tokenState?.kind === "absent" + ? "Connected service token is missing." + : "Connected service token ownership changed.", + }; + } + } + return configuredEnabled + ? { kind: "continue" } + : { kind: "native", notice: CLAUDE_NATIVE_ROUTING_OFF }; +} + +const NATIVE_STRIPPED_LEVERS = [ + "CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", + "CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST", + "CLAUDE_CODE_MAX_CONTEXT_TOKENS", + "CLAUDE_CODE_AUTO_COMPACT_WINDOW", + "CLAUDE_CODE_ALWAYS_ENABLE_EFFORT", + "DISABLE_COMPACT", +] as const; + +const MODEL_ENV_SLOT_NAMES = [ + "ANTHROPIC_MODEL", + "ANTHROPIC_DEFAULT_OPUS_MODEL", + "ANTHROPIC_DEFAULT_SONNET_MODEL", + "ANTHROPIC_DEFAULT_FABLE_MODEL", + "ANTHROPIC_DEFAULT_HAIKU_MODEL", + "ANTHROPIC_SMALL_FAST_MODEL", +] as const; + +const DESKTOP_3P_ALIAS = /^claude-opus-4(?:-8)?-[a-z][0-9a-z]{2}$/; + +export function isProxyOnlyModelId(value: string, providerNames: readonly string[] = []): boolean { + const id = value.trim().replace(/\[1m\]$/, ""); + if (!id) return false; + if (id.startsWith(CLAUDE_ALIAS_PREFIX_V1) || id.startsWith(CLAUDE_ALIAS_PREFIX_V2) || DESKTOP_3P_ALIAS.test(id)) { + return true; + } + const slash = id.indexOf("/"); + return slash > 0 && providerNames.includes(id.slice(0, slash)); +} + +export function buildNativeClaudeEnv( + config: OcxConfig, + base: ClaudeLaunchEnv, + deps: ClaudeEnvDeps = {}, +): ClaudeLaunchEnv { + const env: ClaudeLaunchEnv = { ...base }; + deleteUntrustedAnthropicSlots(env, deps); + + const admissionSlots = ["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"] as const; + const hasOwnedAdmission = admissionSlots.some(name => { + const value = env[name]?.trim(); + return Boolean(value && (value === PROXY_MARKER || isProxyAdmissionSecret(value, config))); + }); + const baseUrl = env.ANTHROPIC_BASE_URL; + if (hasOwnedAdmission && targetsLocalClaudeProxy(baseUrl, config.port)) { + delete env.ANTHROPIC_BASE_URL; + } + for (const name of admissionSlots) { + const value = env[name]?.trim(); + if (value && (value === PROXY_MARKER || isProxyAdmissionSecret(value, config))) delete env[name]; + } + + for (const name of NATIVE_STRIPPED_LEVERS) delete env[name]; + const providerNames = Object.keys(config.providers); + for (const name of MODEL_ENV_SLOT_NAMES) { + const value = env[name]; + if (value && isProxyOnlyModelId(value, providerNames)) delete env[name]; + } + if (deps.allowRootSkipPermissions === true && !env.IS_SANDBOX) env.IS_SANDBOX = "1"; + return env; +} + +export function nativeModelOverride( + pickedModel: string | null, + configuredModel: string | undefined, + args: readonly string[], + providerNames: readonly string[] = [], +): { flag?: string[]; warning?: string } { + if (!pickedModel || !isProxyOnlyModelId(pickedModel, providerNames)) return {}; + if (args.some(arg => arg === "--model" || arg.startsWith("--model="))) return {}; + const fallback = configuredModel?.trim(); + if (fallback && !isProxyOnlyModelId(fallback, providerNames)) { + return { + flag: ["--model", fallback], + warning: `ℹ️ The saved model (${pickedModel}) requires the proxy. This native session will use ${fallback}.`, + }; + } + return { + warning: `⚠ The saved model (${pickedModel}) requires the proxy. Use \`--model \` or select a native model in this session.`, + }; +} + const CLAUDE_INSTALL_HINT = "❌ `claude` CLI not found. Install it first: npm install -g @anthropic-ai/claude-code"; /** @@ -417,28 +596,19 @@ export function rootSkipPermissionsNotice(env: ClaudeLaunchEnv): string { export async function cmdClaude(args: string[]): Promise { const config = loadConfig(); - if (config.claudeCode?.enabled === false) { - console.error("Claude inbound is disabled (config.claudeCode.enabled=false — flip the Claude ON toggle in the GUI or edit config)."); - return 1; - } const clientState = readClientConnectionState(); - if (clientState.kind === "invalid" || clientState.kind === "mismatched") { - console.error(`Client state is ${clientState.kind}: ${clientState.reason}`); + const tokenState = clientState.kind === "connected" ? readServiceApiTokenState() : undefined; + const preflight = claudeLaunchPreflight(config.claudeCode?.enabled !== false, clientState, tokenState); + if (preflight.kind === "error") { + console.error(preflight.message); return 1; } + if (preflight.kind === "native") return launchNativeClaude(config, args, preflight.notice); let route: number | ClaudeRoutingTarget; let contextWindows: Record; if (clientState.kind === "connected") { - if (!clientState.value.selectedClients.includes("claude")) { - console.error("Claude is not selected for this remote hub connection."); - return 1; - } - const token = readServiceApiTokenState(); - if (token.kind !== "present" || token.fingerprint !== clientState.value.tokenFingerprint) { - console.error(token.kind === "absent" ? "Connected service token is missing." : "Connected service token ownership changed."); - return 1; - } - route = { baseUrl: clientState.value.serverUrl, admissionToken: token.token }; + if (tokenState?.kind !== "present") return 1; + route = { baseUrl: clientState.value.serverUrl, admissionToken: tokenState.token }; contextWindows = readConnectedClaudeContextWindows(); } else { const port = await ensureProxyForClaude(); @@ -446,8 +616,11 @@ export async function cmdClaude(args: string[]): Promise { console.error("❌ Proxy did not become healthy after starting."); return 1; } + const liveState = await fetchClaudeCodeState(config, port); + const plan = claudeLaunchPlan(true, liveState.enabled); + if (plan.kind === "native") return launchNativeClaude(config, args, plan.notice); route = port; - contextWindows = await fetchClaudeContextWindows(config, port); + contextWindows = liveState.contextWindows; } const allowRootSkipPermissions = shouldAllowRootSkipPermissions(args); const env = buildClaudeEnv(config, route, process.env, contextWindows, { allowRootSkipPermissions }); @@ -479,7 +652,27 @@ export async function cmdClaude(args: string[]): Promise { console.error(`⚠ Claude agent definitions could not be synced: ${message}`); } } - return await new Promise(resolve => { + return spawnClaude(args, env); +} + +async function launchNativeClaude(config: OcxConfig, args: string[], notice: string): Promise { + console.error(notice); + const providerNames = Object.keys(config.providers); + const override = nativeModelOverride( + readPickerDefaultModel(claudeConfigDir()), + config.claudeCode?.model, + args, + providerNames, + ); + if (override.warning) console.error(override.warning); + const allowRootSkipPermissions = shouldAllowRootSkipPermissions(args); + const env = buildNativeClaudeEnv(config, process.env, { allowRootSkipPermissions }); + if (allowRootSkipPermissions) console.error(rootSkipPermissionsNotice(env)); + return spawnClaude([...(override.flag ?? []), ...args], env); +} + +function spawnClaude(args: string[], env: ClaudeLaunchEnv): Promise { + return new Promise(resolve => { const inv = commandInvocation("claude", args); const child = spawn(inv.file, inv.args, { stdio: "inherit", env: env as NodeJS.ProcessEnv, ...inv.options }); child.on("error", (err: NodeJS.ErrnoException) => { diff --git a/src/cli/registry.ts b/src/cli/registry.ts index 5d6cd4391c..32d7481606 100644 --- a/src/cli/registry.ts +++ b/src/cli/registry.ts @@ -315,13 +315,14 @@ export const CLI_COMMANDS: CliCommandEntry[] = [ { name: "claude", usage: "ocx claude [claude args...]", - summary: "Launch Claude Code wired to the proxy (env injection + gateway model discovery).", + summary: "Launch Claude Code through the proxy, with native fallback when Claude routing is disabled.", details: [ "Ensures the proxy is running, then execs `claude` with ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN,", "CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=1 and model slots from config.claudeCode.", + "When Claude routing is explicitly disabled, it launches natively after removing proven OpenCodex-owned proxy state.", "Routed models appear in the native /model picker with stable claude-opus-4-8-2026MMDD slot aliases (Claude Code >= 2.1.129).", "Older versions: pick models via ANTHROPIC_MODEL or /model directly (any string passes through).", - "User-exported ANTHROPIC_* variables always take precedence.", + "User-exported ANTHROPIC_* variables take precedence for routed launches; native fallback removes only proven OpenCodex-owned proxy values.", "", "Claude Desktop profile:", " ocx claude desktop [apply] Save and apply the four-family profile", diff --git a/tests/claude-integration/claude-cli.test.ts b/tests/claude-integration/claude-cli.test.ts index 2e94faca5c..9bbe7cff95 100644 --- a/tests/claude-integration/claude-cli.test.ts +++ b/tests/claude-integration/claude-cli.test.ts @@ -1,6 +1,21 @@ import { describe, expect, test } from "bun:test"; -import { buildClaudeEnv, claudeNotFoundHint, ensureProxyForClaude, rootSkipPermissionsNotice, shouldAllowRootSkipPermissions } from "../../src/cli/claude"; +import { + buildClaudeEnv, + buildNativeClaudeEnv, + claudeLaunchPlan, + claudeLaunchPreflight, + claudeNotFoundHint, + ensureProxyForClaude, + isProxyOnlyModelId, + nativeModelOverride, + readPickerDefaultModel, + rootSkipPermissionsNotice, + shouldAllowRootSkipPermissions, +} from "../../src/cli/claude"; import { commandInvocation } from "../../src/lib/win-exec"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import type { LivenessIo, LiveProxy } from "../../src/server/proxy-liveness"; import type { OcxConfig } from "../../src/types"; @@ -40,6 +55,122 @@ describe("ocx claude proxy liveness", () => { }); }); +describe("ocx claude native fallback", () => { + test("routes unless configured or live Claude routing is explicitly disabled", () => { + expect(claudeLaunchPlan(true, true)).toEqual({ kind: "routed" }); + expect(claudeLaunchPlan(true, undefined)).toEqual({ kind: "routed" }); + expect(claudeLaunchPlan(false, true)).toMatchObject({ kind: "native" }); + expect(claudeLaunchPlan(true, false)).toMatchObject({ kind: "native" }); + }); + + test("rejects an invalid connected client before configuration-disabled fallback", () => { + expect(claudeLaunchPreflight(false, { kind: "invalid", reason: "bad client state" })) + .toEqual({ kind: "error", message: "Client state is invalid: bad client state" }); + expect(claudeLaunchPreflight(false, { + kind: "connected", + value: { + serverUrl: "https://hub.example.test", + apiKeyId: "remote", + tokenFingerprint: "expected", + selectedClients: ["claude"], + }, + }, { kind: "present", token: "secret", fingerprint: "changed" })) + .toEqual({ kind: "error", message: "Connected service token ownership changed." }); + }); + + test("removes proxy-owned state while preserving user credentials and native model ids", () => { + const config = cfg({ + apiKeys: [{ id: "local", name: "local", key: "ocx_data_local_key", createdAt: "2026-01-01" }], + providers: { mock: { adapter: "openai-chat", baseUrl: "http://x/v1" } }, + }); + const env = buildNativeClaudeEnv(config, { + PATH: "/usr/bin", + ANTHROPIC_BASE_URL: "http://127.0.0.1:10100", + ANTHROPIC_AUTH_TOKEN: "ocx_data_local_key", + ANTHROPIC_API_KEY: "sk-ant-user-key", + ANTHROPIC_MODEL: "claude-ocx-mock--model", + ANTHROPIC_DEFAULT_OPUS_MODEL: "mock/model", + ANTHROPIC_DEFAULT_SONNET_MODEL: "sonnet", + CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST: "1", + CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY: "1", + CLAUDE_CODE_AUTO_COMPACT_WINDOW: "829800", + }, { + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_API_KEY"], + }); + + expect(env.PATH).toBe("/usr/bin"); + expect(env.ANTHROPIC_BASE_URL).toBeUndefined(); + expect(env.ANTHROPIC_AUTH_TOKEN).toBeUndefined(); + expect(env.ANTHROPIC_API_KEY).toBe("sk-ant-user-key"); + expect(env.ANTHROPIC_MODEL).toBeUndefined(); + expect(env.ANTHROPIC_DEFAULT_OPUS_MODEL).toBeUndefined(); + expect(env.ANTHROPIC_DEFAULT_SONNET_MODEL).toBe("sonnet"); + expect(env.CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST).toBeUndefined(); + expect(env.CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY).toBeUndefined(); + expect(env.CLAUDE_CODE_AUTO_COMPACT_WINDOW).toBeUndefined(); + }); + + test("preserves an unrelated loopback gateway and its user credential", () => { + for (const baseUrl of ["http://localhost:8080", "http://127.0.0.1:10100"]) { + const env = buildNativeClaudeEnv(cfg({ port: 10100 }), { + ANTHROPIC_BASE_URL: baseUrl, + ANTHROPIC_API_KEY: "sk-ant-user-key", + }, { + preBunAnthropicSlots: ["ANTHROPIC_BASE_URL", "ANTHROPIC_API_KEY"], + }); + + expect(env.ANTHROPIC_BASE_URL).toBe(baseUrl); + expect(env.ANTHROPIC_API_KEY).toBe("sk-ant-user-key"); + } + }); + + test("keeps unrelated slash model ids and recognizes configured provider routes", () => { + expect(isProxyOnlyModelId("mock/model", ["mock"])).toBe(true); + expect(isProxyOnlyModelId("claude-ocx2-abcd")).toBe(true); + expect(isProxyOnlyModelId("arn:aws:bedrock:region:acct:inference-profile/us.anthropic.model", ["mock"])).toBe(false); + expect(isProxyOnlyModelId("claude-opus-5")).toBe(false); + }); + + test("overrides a persisted proxy model only with a configured native model", () => { + expect(nativeModelOverride("claude-ocx2-abcd", "opus", [], ["mock"])) + .toMatchObject({ flag: ["--model", "opus"] }); + expect(nativeModelOverride("claude-ocx2-abcd", "mock/model", [], ["mock"]).flag).toBeUndefined(); + expect(nativeModelOverride("claude-ocx2-abcd", "opus", ["--model", "sonnet"], ["mock"])) + .toEqual({}); + }); + + test("preserves the root opt-in on native fallback", () => { + const env = buildNativeClaudeEnv(cfg(), {}, { allowRootSkipPermissions: true }); + expect(env.IS_SANDBOX).toBe("1"); + }); + + // A corrupt settings.json used to be indistinguishable from an absent one, so the + // "saved model requires the proxy" warning vanished exactly when the file was broken. + test("an absent picker settings file is silent, a corrupt one warns and names the file", () => { + const dir = mkdtempSync(join(tmpdir(), "ocx-claude-picker-")); + const warnings: string[] = []; + const realWarn = console.warn; + console.warn = (...parts: unknown[]) => { warnings.push(parts.join(" ")); }; + try { + expect(readPickerDefaultModel(dir)).toBeNull(); + expect(warnings).toEqual([]); + + writeFileSync(join(dir, "settings.json"), '{"model": "claude-ocx2-abcd"'); + expect(readPickerDefaultModel(dir)).toBeNull(); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain(join(dir, "settings.json")); + expect(warnings[0]).not.toContain("claude-ocx2-abcd"); + + writeFileSync(join(dir, "settings.json"), '{"model": "claude-ocx2-abcd"}'); + expect(readPickerDefaultModel(dir)).toBe("claude-ocx2-abcd"); + expect(warnings).toHaveLength(1); + } finally { + console.warn = realWarn; + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + describe("ocx claude env assembly", () => { test("connected target injects only the hub base and client admission token", () => { const env = buildClaudeEnv(cfg(), { From 29182deb64918340d03da321f1b1c7b0d4649f2a Mon Sep 17 00:00:00 2001 From: t Date: Sat, 5 Sep 2026 09:29:29 +0900 Subject: [PATCH 2/2] fix(oauth): rebase startup reconciliation on the persisted config reconcileOAuthProviders mutated the in-memory config and called saveConfig(config), so a startup snapshot overwrote any operator edit made after loadConfig() returned. runModelRenameStartupMigration had the same shape. Both now project onto a clone and commit through mutatePersistedConfig, which rebases the write on the newest on-disk snapshot, so a concurrent edit survives. Persistence failure degrades rather than throws. Both functions run inside startServer (src/server/index.ts:651 and :663), which is synchronous by design and wraps neither call in try/catch, so a throw there takes the whole proxy down over a config file the operator can still repair. A missing, malformed or contended config now warns once and adopts the projection in memory, matching every other mutatePersistedConfig consumer (src/storage/policy.ts, src/codex/plan-from-token.ts, src/server/management/agent-settings-routes.ts). Adoption is key by key over the touched keys only. A clear-and-reassign preserves the top-level object identity while silently detaching every nested sub-object a caller still holds a reference to. Tests: the concurrent-edit cases are the RED-on-dev proof of the defect (they fail against unmodified dev, which clobbers). The degrade-not-throw assertions are RED against #3524's head, which threw. The new tests/server/server-startup-reconcile-resilience.test.ts covers the boot path; its /healthz case binds a listener and is skipped where Bun.serve cannot bind, so it is a hosted-CI-only assertion. Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com> --- scripts/test-layout/layout.json | 1 + src/oauth/index.ts | 144 ++++++++++--- src/providers/model-rename-startup.ts | 80 ++++++- tests/fixtures/test-layout-expected.json | 1 + tests/oauth/oauth-provider-reconcile.test.ts | 199 +++++++++++++++++- .../providers/model-rename-migration.test.ts | 147 ++++++++++++- ...erver-startup-reconcile-resilience.test.ts | 147 +++++++++++++ 7 files changed, 680 insertions(+), 39 deletions(-) create mode 100644 tests/server/server-startup-reconcile-resilience.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 0d04e08ac5..b16e32881c 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1047,6 +1047,7 @@ "server-rate-limit-retry-e2e.test.ts": "server", "server-request-body-size.test.ts": "server", "server-search.test.ts": "server", + "server-startup-reconcile-resilience.test.ts": "server", "server-stop-config-hardening.test.ts": "server", "server-xai-chat-reasoning-streaming.test.ts": "server", "server-xai-header-parity.test.ts": "server", diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 3ae6f4c01f..3623398309 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1,7 +1,7 @@ import type { KiroOAuthMetadata, OAuthController, OAuthCredentials } from "./types"; import { parseCallbackInput } from "./callback-server"; import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types"; -import { ConfigMutationLockError, loadConfig, saveConfig } from "../config"; +import { ConfigMutationLockError, loadConfig, mutatePersistedConfig, saveConfig } from "../config"; import { resolveProviderApiKey } from "../providers/key-store"; import { maskEmail } from "../lib/privacy"; import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro"; @@ -1247,42 +1247,134 @@ function migrateLegacyAntigravityStaticCatalog(config: OcxConfig): boolean { return true; } -export function reconcileOAuthProviders(config: OcxConfig): boolean { - let changed = migrateLegacyAntigravityStaticCatalog(config); - for (const [name, prov] of Object.entries(config.providers)) { +interface OAuthReconcileProjection { + config: OcxConfig; + changed: boolean; + touchedProviders: string[]; + touchedAntigravityVersion: boolean; +} + +/** Pure projection over a clone: apply every reconciliation rule and report what it touched. */ +function projectOAuthProviderReconciliation(config: OcxConfig): OAuthReconcileProjection { + const projected = structuredClone(config); + const touchedProviders = new Set(); + const beforeAntigravity = JSON.stringify(projected.providers[GOOGLE_ANTIGRAVITY_PROVIDER]); + const beforeAntigravityVersion = projected.googleAntigravityStaticCatalogVersion; + let changed = migrateLegacyAntigravityStaticCatalog(projected); + if (JSON.stringify(projected.providers[GOOGLE_ANTIGRAVITY_PROVIDER]) !== beforeAntigravity) { + touchedProviders.add(GOOGLE_ANTIGRAVITY_PROVIDER); + } + const touchedAntigravityVersion = projected.googleAntigravityStaticCatalogVersion !== beforeAntigravityVersion; + + for (const [name, prov] of Object.entries(projected.providers)) { + const beforeProvider = JSON.stringify(prov); const def = OAUTH_PROVIDERS[name]; if (name === "command-code" && isLegacyCommandCodeStaticCatalog(prov)) { // The former experimental preset was the exact three-model seed above. It was not a user // choice to disable discovery, so promote only that shape to the account live catalog. prov.liveModels = true; - changed = true; } - if (!def || prov.authMode !== "oauth") continue; - const preset = def.providerConfig; - for (const field of OAUTH_RECONCILE_FIELDS) { - if (JSON.stringify(prov[field]) === JSON.stringify(preset[field])) continue; - if (preset[field] !== undefined) { - prov[field] = cloneProviderField(preset[field]) as never; - } else { - delete prov[field]; + if (def && prov.authMode === "oauth") { + const preset = def.providerConfig; + for (const field of OAUTH_RECONCILE_FIELDS) { + if (JSON.stringify(prov[field]) === JSON.stringify(preset[field])) continue; + if (preset[field] !== undefined) { + prov[field] = cloneProviderField(preset[field]) as never; + } else { + delete prov[field]; + } + } + if (prov.liveModels === undefined && preset.liveModels !== undefined) { + prov.liveModels = preset.liveModels; + } + // Heal a defaultModel that no longer exists in the refreshed list (e.g. a deprecated snapshot). + // Skip providers without a static preset `models` list: for live-discovery providers + // (e.g. command-code OAuth) the account-scoped catalog is not enumerable here, so any + // persisted defaultModel is a user selection and must not be overwritten by the seed. + if (prov.defaultModel && preset.defaultModel && preset.models && preset.models.length > 0 && !(prov.models ?? []).includes(prov.defaultModel)) { + prov.defaultModel = preset.defaultModel; } - changed = true; - } - if (prov.liveModels === undefined && preset.liveModels !== undefined) { - prov.liveModels = preset.liveModels; - changed = true; } - // Heal a defaultModel that no longer exists in the refreshed list (e.g. a deprecated snapshot). - // Skip providers without a static preset `models` list: for live-discovery providers - // (e.g. command-code OAuth) the account-scoped catalog is not enumerable here, so any - // persisted defaultModel is a user selection and must not be overwritten by the seed. - if (prov.defaultModel && preset.defaultModel && preset.models && preset.models.length > 0 && !(prov.models ?? []).includes(prov.defaultModel)) { - prov.defaultModel = preset.defaultModel; + if (JSON.stringify(prov) !== beforeProvider) { changed = true; + touchedProviders.add(name); } } - if (changed) saveConfig(config); - return changed; + + return { + config: projected, + changed, + touchedProviders: [...touchedProviders], + touchedAntigravityVersion, + }; +} + +/** + * Copy only the keys the projection actually touched back onto the caller's live object. + * + * Deliberately key-by-key rather than a wholesale clear-and-reassign: a live reference held + * elsewhere to an untouched provider sub-object must survive startup reconciliation. + */ +function adoptOAuthReconciliation(config: OcxConfig, projection: OAuthReconcileProjection): void { + for (const name of projection.touchedProviders) { + const provider = projection.config.providers[name]; + if (provider) config.providers[name] = structuredClone(provider); + else delete config.providers[name]; + } + if (projection.touchedAntigravityVersion) { + config.googleAntigravityStaticCatalogVersion = projection.config.googleAntigravityStaticCatalogVersion; + } +} + +/** + * Union the keys the on-disk rebase touched with the keys the live projection touched. + * + * The rebase runs against the persisted snapshot, which may already carry a reconciliation + * another process committed. Adopting only its touched set would leave the live object stale + * for a key it decided was already correct on disk. + */ +function withOAuthReconciliationTouchedKeys( + projection: OAuthReconcileProjection, + required: OAuthReconcileProjection, +): OAuthReconcileProjection { + return { + ...projection, + touchedProviders: [...new Set([...projection.touchedProviders, ...required.touchedProviders])], + touchedAntigravityVersion: projection.touchedAntigravityVersion || required.touchedAntigravityVersion, + }; +} + +/** + * Refresh OAuth provider presets against the registry, rebasing the write on the persisted config. + * + * This runs on the boot path (`startServer`), so persistence failure must never be fatal: a + * missing, malformed or contended config degrades to a warning plus an in-memory adopt, exactly + * as every other `mutatePersistedConfig` consumer does (`src/storage/policy.ts`, + * `src/codex/plan-from-token.ts`, `src/server/management/agent-settings-routes.ts`). Throwing + * here would take the whole proxy down over a config file the operator can still repair. + */ +export function reconcileOAuthProviders(config: OcxConfig, persist = true): boolean { + const projection = projectOAuthProviderReconciliation(config); + if (!projection.changed) return false; + if (!persist) { + adoptOAuthReconciliation(config, projection); + return true; + } + const outcome = mutatePersistedConfig(fresh => { + const next = projectOAuthProviderReconciliation(fresh); + if (next.changed) adoptOAuthReconciliation(fresh, next); + return { changed: next.changed, value: next }; + }); + if (outcome.status === "unavailable") { + console.warn( + `[opencodex] OAuth provider reconciliation could not be persisted (${outcome.reason}); ` + + "applying it in memory for this run only.", + ); + adoptOAuthReconciliation(config, projection); + return true; + } + adoptOAuthReconciliation(config, withOAuthReconciliationTouchedKeys(outcome.value, projection)); + return true; } /** Runtime guards: provider config is intentionally passthrough, so persisted fields may be malformed. */ diff --git a/src/providers/model-rename-startup.ts b/src/providers/model-rename-startup.ts index 1f983a6acb..7112e7cdd8 100644 --- a/src/providers/model-rename-startup.ts +++ b/src/providers/model-rename-startup.ts @@ -1,10 +1,47 @@ -import { saveConfig } from "../config"; +import { mutatePersistedConfig } from "../config"; import { projectModelRenames } from "./model-rename-migration"; import type { OcxConfig } from "../types"; export interface ModelRenameStartupDeps { project: typeof projectModelRenames; - save: (config: OcxConfig) => void; + /** Injected writer for tests and callers that own their own persistence. */ + save?: (config: OcxConfig) => void; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Recursive key-by-key adopt: descend into a changed container, replace only changed leaves. */ +function adoptRecord(live: Record, next: Record): void { + for (const key of Object.keys(live)) { + if (!(key in next)) delete live[key]; + } + for (const [key, value] of Object.entries(next)) { + const current = live[key]; + if (JSON.stringify(current) === JSON.stringify(value)) continue; + if (isPlainObject(current) && isPlainObject(value)) { + adoptRecord(current, value); + continue; + } + live[key] = value; + } +} + +/** + * Copy `source` onto `target` in place, touching only the keys that actually differ. + * + * A clear-and-reassign preserves the top-level object identity while silently detaching every + * nested sub-object a caller still holds a live reference to. The renames rewrite one provider + * row at a time, so every sibling row — and the `providers` container itself — must survive + * with its identity intact. + */ +function adoptConfig(target: OcxConfig, source: OcxConfig): void { + if (target === source) return; + adoptRecord( + target as unknown as Record, + structuredClone(source) as unknown as Record, + ); } /** @@ -15,14 +52,41 @@ export interface ModelRenameStartupDeps { * recoverable from the config alone. This one only rewrites model ids that the * registry itself no longer seeds, and the pre-migration value is a string this * file still names, so the change is reversible by hand. + * + * Persistence failure is not fatal. This runs inside `startServer`, so a missing, malformed or + * contended config degrades to a warning plus the in-memory projection rather than taking the + * boot path down. */ export function runModelRenameStartupMigration( config: OcxConfig, - deps: ModelRenameStartupDeps = { project: projectModelRenames, save: saveConfig }, + deps: ModelRenameStartupDeps = { project: projectModelRenames }, ): OcxConfig { - const projection = deps.project(config); - for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); - if (!projection.changed) return projection.config; - deps.save(projection.config); - return projection.config; + const projection = deps.project(structuredClone(config)); + if (!projection.changed) { + for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; + } + if (deps.save) { + deps.save(projection.config); + adoptConfig(config, projection.config); + for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; + } + const outcome = mutatePersistedConfig(fresh => { + const next = deps.project(fresh); + if (next.changed) adoptConfig(fresh, next.config); + return { changed: next.changed, value: next }; + }); + if (outcome.status === "unavailable") { + console.warn( + `[model-rename-migration] persistence unavailable (${outcome.reason}); applying the renames ` + + "in memory for this run only.", + ); + adoptConfig(config, projection.config); + for (const warning of projection.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; + } + adoptConfig(config, outcome.value.config); + for (const warning of outcome.value.warnings) console.warn(`[model-rename-migration] ${warning}`); + return config; } diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index ef52b3720f..0b0c65d0ed 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -884,6 +884,7 @@ "server-rate-limit-retry-e2e.test.ts": "server", "server-request-body-size.test.ts": "server", "server-search.test.ts": "server", + "server-startup-reconcile-resilience.test.ts": "server", "server-stop-config-hardening.test.ts": "server", "server-xai-chat-reasoning-streaming.test.ts": "server", "server-xai-header-parity.test.ts": "server", diff --git a/tests/oauth/oauth-provider-reconcile.test.ts b/tests/oauth/oauth-provider-reconcile.test.ts index 4f1ef9768f..132dea3c7a 100644 --- a/tests/oauth/oauth-provider-reconcile.test.ts +++ b/tests/oauth/oauth-provider-reconcile.test.ts @@ -1,8 +1,13 @@ -import { afterEach, describe, expect, test } from "bun:test"; -import { mkdtempSync} from "node:fs"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { loadConfig } from "../../src/config"; +import { + getConfigPath, + loadConfig, + saveConfig, + setPersistedConfigMutationBeforeCommitForTests, +} from "../../src/config"; import { OAUTH_PROVIDERS, reconcileOAuthProviders, upsertOAuthProvider } from "../../src/oauth"; import { getCredential, saveCredential } from "../../src/oauth/store"; import { routeModel } from "../../src/router"; @@ -15,6 +20,7 @@ const originalHome = process.env.OPENCODEX_HOME; const homes: string[] = []; afterEach(() => { + setPersistedConfigMutationBeforeCommitForTests(null); if (originalHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = originalHome; for (const home of homes.splice(0)) removeTreeWithRetry(home); @@ -39,6 +45,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); @@ -48,6 +55,180 @@ describe("OAuth provider reconciliation", () => { expect(modelInList(config.providers.cursor.noVisionModels, "composer-2.5")).toBe(true); expect(reconcileOAuthProviders(config)).toBe(false); }); + + // RED on dev (#3524): dev mutates the live object then calls saveConfig(config), so the + // startup snapshot overwrites whatever an operator wrote after loadConfig() returned. + test("rebases startup reconciliation over a concurrent provider edit", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-race-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } satisfies OcxConfig; + saveConfig(config); + setPersistedConfigMutationBeforeCommitForTests(() => { + const concurrent = loadConfig(); + concurrent.providers.cursor.note = "concurrent-operator-edit"; + writeFileSync(getConfigPath(), JSON.stringify(concurrent, null, 2) + "\n"); + }); + + expect(reconcileOAuthProviders(config)).toBe(true); + + expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(config.providers.cursor.note).toBe("concurrent-operator-edit"); + const persisted = loadConfig(); + expect(persisted.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(persisted.providers.cursor.note).toBe("concurrent-operator-edit"); + }); + + // #3524 threw here, on a call site startServer() does not guard. Reconciliation is a + // best-effort startup refresh: an unwritable config must degrade, never kill boot. + test("a config removed between load and reconcile warns and degrades to an in-memory apply", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-unavailable-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } satisfies OcxConfig; + saveConfig(config); + rmSync(getConfigPath()); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + let changed: boolean | undefined; + expect(() => { changed = reconcileOAuthProviders(config); }).not.toThrow(); + expect(changed).toBe(true); + // The running process still gets a correct catalog even though nothing was written. + expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("OAuth provider reconciliation could not be persisted (missing)"))).toBe(true); + } finally { + warn.mockRestore(); + } + }); + + test("a malformed config degrades without writing and without throwing", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-invalid-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } satisfies OcxConfig; + saveConfig(config); + writeFileSync(getConfigPath(), "{ this is not json"); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(reconcileOAuthProviders(config)).toBe(true); + expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("OAuth provider reconciliation could not be persisted"))).toBe(true); + } finally { + warn.mockRestore(); + } + }); + + test("a fresh install with nothing to reconcile neither writes nor warns", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-fresh-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { cursor: { ...structuredClone(OAUTH_PROVIDERS.cursor.providerConfig), authMode: "oauth" } }, + } satisfies OcxConfig; + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + // No config.json on disk at all: an unchanged projection must short-circuit before + // persistence is ever consulted, so "missing" never becomes a startup warning. + expect(reconcileOAuthProviders(config)).toBe(false); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("OAuth provider reconciliation"))).toBe(false); + } finally { + warn.mockRestore(); + } + }); + + test("persist=false adopts the projection in memory and leaves the file untouched", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-no-persist-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } satisfies OcxConfig; + saveConfig(config); + const before = Bun.file(getConfigPath()); + const beforeBytes = before.size; + + expect(reconcileOAuthProviders(config, false)).toBe(true); + expect(config.providers.cursor.noVisionModels).toEqual(preset.noVisionModels); + expect(Bun.file(getConfigPath()).size).toBe(beforeBytes); + }); + + test("an untouched provider row keeps its live object identity across reconciliation", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-oauth-reconcile-identity-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + const preset = OAUTH_PROVIDERS.cursor.providerConfig; + const config = { + port: 10100, + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(preset), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + untouched: { + adapter: "openai", + baseUrl: "http://127.0.0.1:9999/v1", + allowPrivateNetwork: true, + models: ["local-live"], + }, + }, + } satisfies OcxConfig; + saveConfig(config); + const liveUntouched = config.providers.untouched; + + expect(reconcileOAuthProviders(config)).toBe(true); + // A wholesale clear-and-reassign would silently detach every reference a caller still + // holds; only the rows the projection actually changed may be replaced. + expect(config.providers.untouched).toBe(liveUntouched); + }); test("refreshes a saved Antigravity 3.5 preset without touching credentials or user fields", async () => { const home = mkdtempSync(join(tmpdir(), "ocx-gemini-36-reconcile-")); homes.push(home); @@ -76,6 +257,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); const provider = config.providers["google-antigravity"]; @@ -112,6 +294,9 @@ describe("OAuth provider reconciliation", () => { }); test("migrates the version-1 canonical Antigravity static row to live discovery", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-antigravity-static-reconcile-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; const config = { port: 10100, defaultProvider: "google-antigravity", @@ -134,6 +319,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers["google-antigravity"].liveModels).toBe(true); @@ -149,6 +335,9 @@ describe("OAuth provider reconciliation", () => { // healing branch. This one is the opposite claim, and the one that matters for an // additive rollout: a user who deliberately chose 3.7 must still be on 3.7 afterwards. // Google still serves it, so healing it onto 3.8 would be silently overriding a choice. + const home = mkdtempSync(join(tmpdir(), "ocx-antigravity-explicit-default-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; saveCredential("google-antigravity", { access: "a", refresh: "r", projectId: "p" }); const config = { port: 10100, @@ -165,6 +354,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); reconcileOAuthProviders(config); const provider = config.providers["google-antigravity"]; @@ -264,7 +454,7 @@ describe("OAuth provider reconciliation", () => { }, } satisfies OcxConfig; - reconcileOAuthProviders(config); + reconcileOAuthProviders(config, false); expect(config.providers.kimi.requiresReasoningPlaceholderModels).toEqual([]); }); @@ -288,6 +478,7 @@ describe("OAuth provider reconciliation", () => { }, }, } satisfies OcxConfig; + saveConfig(config); expect(reconcileOAuthProviders(config)).toBe(true); expect(config.providers.xai.modelReasoningEfforts?.["grok-4.6"]) diff --git a/tests/providers/model-rename-migration.test.ts b/tests/providers/model-rename-migration.test.ts index 40803a4ed1..34801e8b21 100644 --- a/tests/providers/model-rename-migration.test.ts +++ b/tests/providers/model-rename-migration.test.ts @@ -1,11 +1,17 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { MODEL_RENAMES, projectModelRenames, type ModelRename, } from "../../src/providers/model-rename-migration"; +import { runModelRenameStartupMigration } from "../../src/providers/model-rename-startup"; import { PROVIDER_REGISTRY } from "../../src/providers/registry"; +import { getConfigPath, loadConfig, saveConfig, setPersistedConfigMutationBeforeCommitForTests } from "../../src/config"; import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; const INTL_BASE_URL = "https://token-plan.ap-southeast-1.maas.aliyuncs.com/compatible-mode/v1"; @@ -131,3 +137,142 @@ describe("registry model rename migration (#1610)", () => { } }); }); + +describe("model rename startup persistence", () => { + const homes: string[] = []; + const originalHome = process.env.OPENCODEX_HOME; + + function isolate(prefix: string): void { + const home = mkdtempSync(join(tmpdir(), prefix)); + homes.push(home); + process.env.OPENCODEX_HOME = home; + } + + /** A saved config the renames actually rewrite, valid enough for loadConfig to accept. */ + function persistableStale(): OcxConfig { + return { port: 10100, defaultProvider: "alibaba-token-plan-intl", ...staleConfig() } as OcxConfig; + } + + afterEach(() => { + setPersistedConfigMutationBeforeCommitForTests(null); + if (originalHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = originalHome; + for (const home of homes.splice(0)) removeTreeWithRetry(home); + }); + + test("startup no-op preserves the live config object identity", () => { + const clean = projectModelRenames(staleConfig(), [RENAME]).config; + const returned = runModelRenameStartupMigration(clean, { + project: config => projectModelRenames(config, [RENAME]), + save: () => { throw new Error("no-op must not save"); }, + }); + + expect(returned).toBe(clean); + }); + + test("startup no-op still reports projection warnings", () => { + const clean = projectModelRenames(staleConfig(), [RENAME]).config; + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + const returned = runModelRenameStartupMigration(clean, { + project: config => ({ config, changed: false, warnings: ["rename target is unavailable"] }), + save: () => { throw new Error("no-op must not save"); }, + }); + + expect(returned).toBe(clean); + expect(warn).toHaveBeenCalledWith("[model-rename-migration] rename target is unavailable"); + } finally { + warn.mockRestore(); + } + }); + + // RED on dev: dev hands `projectModelRenames` the live object and saves the projection + // wholesale, so an operator edit written after loadConfig() is silently discarded. + test("rebases the startup rename over a concurrent provider edit", () => { + isolate("ocx-model-rename-race-"); + const live = persistableStale(); + saveConfig(live); + setPersistedConfigMutationBeforeCommitForTests(() => { + const concurrent = loadConfig(); + concurrent.providers["alibaba-token-plan-intl"]!.note = "concurrent-operator-edit"; + writeFileSync(getConfigPath(), JSON.stringify(concurrent, null, 2) + "\n"); + }); + + const returned = runModelRenameStartupMigration(live); + + expect(returned).toBe(live); + expect(live.providers["alibaba-token-plan-intl"]!.models).toContain("qwen3.8-max"); + expect(live.providers["alibaba-token-plan-intl"]!.note).toBe("concurrent-operator-edit"); + const persisted = loadConfig(); + expect(persisted.providers["alibaba-token-plan-intl"]!.models).toContain("qwen3.8-max"); + expect(persisted.providers["alibaba-token-plan-intl"]!.note).toBe("concurrent-operator-edit"); + }); + + // #3524 threw here, on the unguarded startServer() call site at src/server/index.ts:651. + test("a config removed between load and migrate warns and degrades to an in-memory apply", () => { + isolate("ocx-model-rename-unavailable-"); + const live = persistableStale(); + saveConfig(live); + rmSync(getConfigPath()); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + let returned: OcxConfig | undefined; + expect(() => { returned = runModelRenameStartupMigration(live); }).not.toThrow(); + + expect(returned).toBe(live); + expect(live.providers["alibaba-token-plan-intl"]!.models).toContain("qwen3.8-max"); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("[model-rename-migration] persistence unavailable (missing)"))).toBe(true); + } finally { + warn.mockRestore(); + } + }); + + test("a malformed config degrades without throwing", () => { + isolate("ocx-model-rename-invalid-"); + const live = persistableStale(); + saveConfig(live); + writeFileSync(getConfigPath(), "{ this is not json"); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(() => runModelRenameStartupMigration(live)).not.toThrow(); + expect(live.providers["alibaba-token-plan-intl"]!.models).toContain("qwen3.8-max"); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("[model-rename-migration] persistence unavailable"))).toBe(true); + } finally { + warn.mockRestore(); + } + }); + + test("a fresh install with nothing to rename neither writes nor warns about persistence", () => { + isolate("ocx-model-rename-fresh-"); + const clean = projectModelRenames(persistableStale()).config; + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(runModelRenameStartupMigration(clean)).toBe(clean); + expect(warn.mock.calls.some(([first]) => typeof first === "string" + && first.includes("persistence unavailable"))).toBe(false); + } finally { + warn.mockRestore(); + } + }); + + test("an untouched top-level branch keeps its live object identity across the migration", () => { + isolate("ocx-model-rename-identity-"); + const live = persistableStale(); + live.providers.untouched = { + adapter: "openai", + baseUrl: "http://127.0.0.1:9999/v1", + allowPrivateNetwork: true, + models: ["local-live"], + }; + saveConfig(live); + const liveUntouched = live.providers.untouched; + + runModelRenameStartupMigration(live); + + // adoptConfig copies key by key, so a reference a caller still holds to an unchanged + // branch survives; a clear-and-reassign would silently detach it. + expect(live.providers.untouched).toBe(liveUntouched); + }); +}); diff --git a/tests/server/server-startup-reconcile-resilience.test.ts b/tests/server/server-startup-reconcile-resilience.test.ts new file mode 100644 index 0000000000..4b5db8ba64 --- /dev/null +++ b/tests/server/server-startup-reconcile-resilience.test.ts @@ -0,0 +1,147 @@ +/** + * Startup reconciliation must never be able to kill boot (#3524). + * + * `startServer` is synchronous by design and calls `runModelRenameStartupMigration` + * (src/server/index.ts:651) and `reconcileOAuthProviders` (:663) with no try/catch. Both + * now rebase their write on the persisted config, and a persistence failure there is a + * degrade-and-warn, not a throw: an operator whose config.json vanished, was hand-edited + * into invalid JSON, or sits on an unreadable volume still gets a running proxy. + * + * RED against #3524's head, which threw "OAuth provider reconciliation persistence + * unavailable: missing" from exactly this sequence. It is NOT red on unmodified dev — dev + * never throws, it silently overwrites — so the defect itself is proven by the + * concurrent-edit tests in tests/oauth/oauth-provider-reconcile.test.ts and + * tests/providers/model-rename-migration.test.ts, not here. + */ +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { getConfigPath, loadConfig, saveConfig } from "../../src/config"; +import { OAUTH_PROVIDERS, reconcileOAuthProviders } from "../../src/oauth"; +import { runModelRenameStartupMigration } from "../../src/providers/model-rename-startup"; +import { startServer } from "../../src/server"; +import { CURSOR_STATIC_MODELS, cursorModelIds } from "../../src/adapters/cursor/discovery"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import type { OcxConfig } from "../../src/types"; + +/** + * Sandboxed agent environments deny `Bun.serve` outright ("Is port 0 in use?", EADDRINUSE on + * every port), which is an environment artifact and not a regression — the same class already + * documented for tests/server/server-combo-failover-e2e.test.ts. Probe once so the + * listener-bound assertion is hosted-CI-only while the boot-sequence assertions always run. + * + * The probe has to be `Bun.serve` itself: a `node:net` listener still binds in an environment + * where Bun's does not, so probing with the wrong API reports a false green and the skip never + * fires. + */ +function canBindLoopback(): boolean { + try { + const probe = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch: () => new Response("probe") }); + probe.stop(true); + return true; + } catch { + return false; + } +} + +const CAN_BIND = canBindLoopback(); + +let testDir = ""; +let previousHome: string | undefined; +let isolatedCodexHome: IsolatedCodexHome | null = null; + +/** A saved config that reconciliation genuinely rewrites, so the persistence path is reached. */ +function staleConfig(): OcxConfig { + return { + port: 0, + hostname: "127.0.0.1", + defaultProvider: "cursor", + providers: { + cursor: { + ...structuredClone(OAUTH_PROVIDERS.cursor.providerConfig), + authMode: "oauth", + noVisionModels: cursorModelIds(CURSOR_STATIC_MODELS), + }, + }, + } as OcxConfig; +} + +/** The exact startup sequence src/server/index.ts runs at :651 and :663, and nothing else. */ +function runStartupReconciliation(config: OcxConfig): void { + reconcileOAuthProviders(runModelRenameStartupMigration(config)); +} + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + isolatedCodexHome = installIsolatedCodexHome("ocx-startup-reconcile-codex-"); + testDir = mkdtempSync(join(tmpdir(), "ocx-startup-reconcile-")); + process.env.OPENCODEX_HOME = testDir; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (testDir) removeTreeWithRetry(testDir); +}); + +test("a config removed between loadConfig() and reconcile does not throw on the boot path", () => { + saveConfig(staleConfig()); + const config = loadConfig(); + rmSync(getConfigPath()); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(() => runStartupReconciliation(config)).not.toThrow(); + // Degrading is not the same as doing nothing: the running process still gets the + // reconciled catalog, it just never reaches disk. + expect(config.providers.cursor.noVisionModels) + .toEqual(OAUTH_PROVIDERS.cursor.providerConfig.noVisionModels); + } finally { + warn.mockRestore(); + } +}); + +test("a config hand-edited into invalid JSON does not throw on the boot path", () => { + saveConfig(staleConfig()); + const config = loadConfig(); + writeFileSync(getConfigPath(), "{ this is not json"); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(() => runStartupReconciliation(config)).not.toThrow(); + } finally { + warn.mockRestore(); + } +}); + +test("a fresh install with no config file at all does not throw on the boot path", () => { + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(() => runStartupReconciliation(loadConfig())).not.toThrow(); + } finally { + warn.mockRestore(); + } +}); + +// Hosted-CI-only: binds a listener, which sandboxed agent environments refuse (see +// canBindLoopback above). The three assertions above cover the same claim without a port. +test.skipIf(!CAN_BIND)( + "startServer completes and serves /healthz when the config disappears before reconcile", + async () => { + saveConfig(staleConfig()); + // Removing the file after the save reproduces the operator-visible shape: every persisted + // mutation under startServer is "unavailable" for the rest of the boot. + rmSync(getConfigPath()); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + const server = startServer(0); + try { + const response = await fetch(`http://127.0.0.1:${server.port}/healthz`); + expect(response.ok).toBe(true); + } finally { + await server.stop(true); + warn.mockRestore(); + } + }, +);