diff --git a/bin/ocx.mjs b/bin/ocx.mjs index 8da4e1f724..5357985bca 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -9,6 +9,9 @@ * src/cli/index.ts — only the published npm `bin` routes through here.) */ import { spawn, spawnSync } from "node:child_process"; +import { STOP_HISTORY_INCOMPLETE_EXIT_CODE } from "../src/update/stop-contract.mjs"; +import { probeProxyLiveness } from "../src/update/proxy-liveness-probe.mjs"; +import { decidePostStopUpdate } from "../src/update/stop-decision.mjs"; import { randomBytes } from "node:crypto"; import { createRequire } from "node:module"; import { existsSync, readFileSync, readdirSync } from "node:fs"; @@ -17,6 +20,7 @@ import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { isRealBunBinary } from "../src/lib/bun-binary-validator.mjs"; import { npmInvocation } from "../src/update/npm-invocation.mjs"; +import { hasPendingTeardownIn } from "../src/config/pending-teardown-names.mjs"; import { npmCachePreflightFailureMessage, runNpmCachePreflight, @@ -202,7 +206,12 @@ function runNpmSelfUpdate() { // Capture listen target before stop clears runtime-port.json (mirrors GUI/CLI update worker). // Do not treat a live runtime port of 10100 as "missing" — track whether the read succeeded. let bakePort = 10100; + // The hostname travels with the port: a proxy bound to ::1 or a specific interface is + // invisible to a probe that assumes 127.0.0.1, and "no answer" would then read as + // "stopped" for exactly the proxy the probe exists to find. + let bakeHostname = "127.0.0.1"; let sawRuntimePort = false; + let sawRuntimeHostname = false; try { const rt = JSON.parse(readFileSync(join(configDir(), "runtime-port.json"), "utf8")); if (Number.isFinite(rt?.port) && rt.port > 0 && rt.port <= 65535) { @@ -219,16 +228,29 @@ function runNpmSelfUpdate() { } if (runtimeLive) { bakePort = Math.trunc(rt.port); + if (typeof rt?.hostname === "string" && rt.hostname.trim() !== "") { + bakeHostname = rt.hostname.trim(); + sawRuntimeHostname = true; + } sawRuntimePort = true; } } } catch { /* fall through to config */ } - if (!sawRuntimePort) { + // Port and hostname resolve INDEPENDENTLY: a legacy runtime record carries a port and no + // hostname, and skipping config in that case probed 127.0.0.1 for a proxy bound to ::1. + if (!sawRuntimePort || bakeHostname === "127.0.0.1") { try { const cfg = JSON.parse(readFileSync(join(configDir(), "config.json"), "utf8")); - if (Number.isFinite(cfg?.port) && cfg.port > 0 && cfg.port <= 65535) bakePort = Math.trunc(cfg.port); + if (!sawRuntimePort && Number.isFinite(cfg?.port) && cfg.port > 0 && cfg.port <= 65535) { + bakePort = Math.trunc(cfg.port); + } + if (!sawRuntimeHostname && typeof cfg?.hostname === "string" && cfg.hostname.trim() !== "") { + bakeHostname = cfg.hostname.trim(); + } } catch { /* keep default */ } } + // Wildcard and bracketed-IPv6 normalization lives in probeProxyLiveness, so both lanes + // get it from one place. const launcher = fileURLToPath(import.meta.url); @@ -343,17 +365,47 @@ function runNpmSelfUpdate() { } } - if (serviceWasInstalled || hasRuntimeState) { + // An outstanding pending-teardown receipt is a fourth reason to run the stop. After a + // parent crashed mid-deferral the service, pid and runtime records can all be absent + // while the shared client config still points at a proxy that is gone; installing over + // that silently skips the recovery the receipt was written to trigger (#3008). Presence + // is the whole test here — the launcher cannot parse it, and `ocx stop` is what decides + // whether the obligation is safe to finish. + const hasPendingTeardown = hasPendingTeardownIn(readdirSync, configDir()); + if (serviceWasInstalled || hasRuntimeState || hasPendingTeardown) { console.log("⏹ Stopping the running proxy before updating..."); const stopRes = spawnSync(process.execPath, [launcher, "stop"], { stdio: "inherit", windowsHide: true }); const stillHasRuntimeState = existsSync(join(configDir(), "ocx.pid")) || existsSync(join(configDir(), "runtime-port.json")); - if (stopRes.status !== 0 || stillHasRuntimeState) { + // A history-only failure means teardown succeeded and a backup manifest is waiting for + // review: the proxy is down and replacing package files is safe. Every other nonzero + // status is a stop that did not finish, and a signal kill (status null) says nothing + // about whether it did - both abort, because replacing files under a live server + // leaves it running mixed old and new modules (#3008). + // The same decision the Bun updater makes, from the same module (#3008). Absent PID and + // runtime files are weak evidence, so the captured endpoint is asked; "unknown" aborts + // because a silent listener is exactly the state where replacing files is dangerous. + const decision = decidePostStopUpdate({ + status: stopRes.status, + hasRuntimeState: stillHasRuntimeState, + // Re-checked AFTER the stop: a quarantined receipt lets the stop itself succeed + // (there is nothing left to stop), so a pre-stop check alone let the retry install + // over a teardown that never ran. + teardownOutstanding: hasPendingTeardownIn(readdirSync, configDir()), + liveness: probeProxyLiveness(bakePort, bakeHostname), + }); + const historyOnlyStop = decision.reason === "history-only"; + if (!decision.proceed) { if (trayBeforeUpdate.restoreOnFailure) runTrayLifecycle(launcher, "start"); - console.error("opencodex: could not stop the running proxy; aborting the update. Run 'ocx stop' and retry."); + if (decision.reason === "teardown-outstanding") { + console.error("opencodex: a shared teardown from an earlier stop is still outstanding and needs manual review; aborting the update."); + console.error("opencodex: confirm no proxy is running, run 'ocx restore', then remove the pending-teardown file in the opencodex home."); + } else console.error(decision.reason === "proxy-unknown" + ? `opencodex: could not confirm the proxy on ${bakeHostname}:${bakePort} is stopped; aborting the update. Run 'ocx stop' and retry.` + : "opencodex: could not stop the running proxy; aborting the update. Run 'ocx stop' and retry."); process.exit(1); } - if (historyRestoreIncomplete()) { + if (historyOnlyStop || historyRestoreIncomplete()) { console.warn( "opencodex: WARNING — Codex resume-history metadata restore is incomplete (a backup manifest remains).\n" + " The DB may be busy or the manifest/target may need review; untracked routed history is intentionally unchanged.\n" + diff --git a/devlog/_plan/260831_prio70_train_round2/051_wp5_outcome.md b/devlog/_plan/260831_prio70_train_round2/051_wp5_outcome.md new file mode 100644 index 0000000000..1f87650dd3 --- /dev/null +++ b/devlog/_plan/260831_prio70_train_round2/051_wp5_outcome.md @@ -0,0 +1,68 @@ +# 050 outcome — wp5 (#3008): what the implementation added beyond this plan + +The plan described one defect: `ocx update` aborting because `ocx stop` could not tell a +history-only failure from a real stop failure. That fix is in the first commit of the +branch. The other twenty-five came from adversarial review, and they are not incidental — +each one is the same defect wearing different clothes: **a teardown that reports success +while half of it did not happen.** + +## The shape that kept recurring + +Something cannot be determined, and the code treats "could not determine" as "determined +to be fine". Every instance authorized taking shared client config down while a proxy +might still be serving, which leaves Codex or Grok pointed at a process that is gone. + +| Where | What was read as proof | Round | +| --- | --- | --- | +| `POST /api/stop` | `stopServiceIfInstalled` false — collapsed "not installed" with "refused to stop" | 17 | +| `POST /api/stop` | Success decided from the native restore alone, Grok failure appended as text | 17 | +| `ocx service stop` / `uninstall` | Restore and strip failures logged, exit code still 0 | 17 | +| Route pre-check | Scheduler stopped first, refused second — mutate-then-refuse | 18 | +| Daemon exit | Drain success alone, ignoring the teardown result | 18 | +| Respawn predicate | `status === "present"`, so an unreadable probe passed as absent | 19 | +| `ocx restore` | Early return on the Codex no-op path, never reaching the Grok strip | 15 | +| `ocx restore --json` | Same, on the ordinary forward path | 16 | +| Receipt scan | Every `readdir` error read as "no obligations" | 16 | +| `ocx uninstall` | `stopServiceIfInstalled` false read as "not installed" | 22 | +| `ocx uninstall` | Missing pid file read as "no proxy serving" | 23 | +| `ocx uninstall` | `uninstallServiceIfInstalled` false — absence and removal failure | 24 | +| `ocx uninstall` | Registration removed, running wrapper assumed dead | 24 | +| `ocx uninstall` | `findLiveProxy` null read as proof of absence | 24 | +| `ocx uninstall` | One endpoint probed while two were candidates | 25 | +| `ocx uninstall` | `proxyStillLiveAfterStop` null read as a verified window | 25 | + +## The deferral, and why it needed four attempts + +`ocx stop` has to defer shared teardown to itself, because the proxy exits before anyone +can verify a Task Scheduler wrapper did not respawn it. Expressing that obligation took +four tries: + +1. A query flag. Any authenticated caller could set it and exit, and a parent that died + mid-stop left nothing on disk saying a restore was owed. +2. A receipt file. Presence is not ownership — another caller could ride on it. +3. A nonce inside one shared file. Read-compare-unlink is three syscalls, so a concurrent + stop replacing the file between the compare and the unlink got its obligation deleted. +4. **The nonce as the filename.** `unlink` names one specific obligation and cannot reach + another. Two concurrent stops hold two receipts, which is the truth of the situation. + +The receipt also carries the endpoint being stopped and how it was obtained. A configured +address is recorded as `guessed` and never authorizes automatic recovery: a proxy on an +explicit `--port` can be respawned there while the configured port refuses. + +## Tests that passed for the wrong reason + +Five times a regression was written as a source-text assertion, and five times reverting +the defect left it green. The reviewer caught each one. Where a rule mattered it was +extracted into something callable — `performStopTeardown`, `classifyWindowsServiceStop`, +`sharedTeardownAuthorized`, `endpointsToProve`, `everyEndpointProvenDown` — and the test +now executes the permutations. Source assertions remain only for wiring: that a route +delegates to the extracted rule rather than growing a second copy. + +Every fix on this branch was driven RED against the specific defect and restored. + +## Docs + +Sixteen files across eight locales carry the new refusal contract: `respawnable_service` +and `service_state_unknown`, and the fact that the dashboard Stop button refuses on the +Windows Task Scheduler backend rather than half-performing a stop it cannot verify. + diff --git a/docs-site/src/content/docs/fr/guides/web-dashboard.md b/docs-site/src/content/docs/fr/guides/web-dashboard.md index c6343f75b7..dc000a5a27 100644 --- a/docs-site/src/content/docs/fr/guides/web-dashboard.md +++ b/docs-site/src/content/docs/fr/guides/web-dashboard.md @@ -55,7 +55,7 @@ gestionnaire de mots de passe. | **Journaux** | Actualisez automatiquement les requêtes récentes et consultez les jetons, l'effort demandé et, lorsqu'il est disponible, l'effort sortant effectif, le modèle résolu, le fournisseur, l'état, l'identifiant de requête, la durée et les détails de l'erreur. La vue détaillée inclut le champ exact de raisonnement transmis lorsque l'adaptateur en émet un. Filtrez par identifiant opaque de conversation ou de session — si le client en fournit un — afin d'obtenir le total des jetons et le coût estimé au tarif catalogue pour l'anneau de journaux actuellement chargé. | | **Utilisation / Débogage** | Examinez la couverture et les tendances d'utilisation des jetons, ou activez à la demande les diagnostics de transport et d'extraction de l'utilisation propres aux fournisseurs. | | **Stockage** | Consultez en lecture seule la répartition du disque de CODEX_HOME — sessions, archives, bases de données et pièces jointes. Pour le nettoyage facultatif des archives, prévisualisez les N % les plus anciennes, puis placez-les en quarantaine dans `CODEX_HOME/.trash` (par défaut) ou supprimez-les définitivement après avoir coché une case explicite. **La stratégie de nettoyage automatique** est facultative et **désactivée par défaut** (`storageCleanupPolicy.enabled`) ; configurez son seuil, sa cible, sa planification et son mode sur la page **Stockage**, ou lancez **Exécuter maintenant**. Les entrées mises en quarantaine peuvent être restaurées depuis cette page (JSONL et fils). Les sessions actives restent en lecture seule. Le nettoyage et la restauration sont refusés tant que Codex verrouille le fichier `state_*.sqlite` le plus récent ou actif. | -| **Arrêter** | Arrêtez proprement le proxy et le service d'arrière-plan installé, restaurez Codex natif et quittez (`POST /api/stop`). | +| **Arrêter** | Arrêtez proprement le proxy et le service d'arrière-plan installé, restaurez Codex natif et quittez (`POST /api/stop`). Sur Windows avec le backend Planificateur de tâches, le tableau de bord refuse et vous demande d'exécuter `ocx stop` : le wrapper peut relancer le proxy après la fin de la tâche, et seul un stop exécuté hors du proxy peut vérifier cette fenêtre de redémarrage avant de restaurer votre configuration client. Rien n'est modifié en cas de refus. | ### Liens directs vers une section @@ -174,7 +174,7 @@ L'interface graphique est un client léger de l'API JSON de gestion du proxy. Pa | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | Ajouter un compte au groupe au moyen d’une connexion dans le navigateur. | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | Lire les métadonnées des requêtes récentes avec des filtres facultatifs de fin de journal, de fournisseur et d'état exact ou par classe. Avec `limit`/`offset`, la pagination remonte depuis la ligne la plus récente (`offset=0` renvoie la dernière page). Forme de la réponse : `{ timeZone, total, logs }`, où `total` est le nombre de lignes filtrées avant pagination. | | `GET` / `PUT /api/subagent-models` | Lire ou définir les cinq modèles de remplacement `spawn_agent` mis en avant. | -| `POST /api/stop` | Arrêter le proxy et le service, restaurer Codex natif et quitter. | +| `POST /api/stop` | Arrêter le proxy et le service, restaurer Codex natif et quitter. Refusé avec `respawnable_service` sur le backend Planificateur de tâches Windows, et avec `service_state_unknown` lorsque cet état ne peut pas être lu ; rien n'est modifié dans les deux cas. | :::tip L'ajout d'**Ollama Cloud** ou d'un autre fournisseur doté d'un catalogue depuis le tableau de bord copie sa diff --git a/docs-site/src/content/docs/fr/reference/cli/lifecycle.md b/docs-site/src/content/docs/fr/reference/cli/lifecycle.md index b93a23c298..8f6362e1e4 100644 --- a/docs-site/src/content/docs/fr/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/fr/reference/cli/lifecycle.md @@ -24,7 +24,7 @@ ocx start --port 8080 ### `ocx stop` -Arrête le proxy actif à partir de son PID, supprime le fichier de PID et rétablit le fonctionnement natif de Codex. Si un service d’arrière-plan géré est installé, `ocx stop` l’arrête d’abord afin qu’il ne puisse pas relancer le proxy. La même opération est disponible avec le bouton **Stop** du tableau de bord Web (`POST /api/stop`). +Arrête le proxy actif à partir de son PID, supprime le fichier de PID et rétablit le fonctionnement natif de Codex. Si un service d’arrière-plan géré est installé, `ocx stop` l’arrête d’abord afin qu’il ne puisse pas relancer le proxy. Le bouton **Stop** du tableau de bord Web exécute la même opération (`POST /api/stop`) sur tous les backends, sauf le Planificateur de tâches Windows : le wrapper peut y relancer le proxy après la fin de la tâche, donc le tableau de bord refuse avec `respawnable_service`, ne modifie rien et vous demande d'exécuter `ocx stop`. ### `ocx restart` diff --git a/docs-site/src/content/docs/fr/reference/management-api.md b/docs-site/src/content/docs/fr/reference/management-api.md index cde2aa143c..b5c730b837 100644 --- a/docs-site/src/content/docs/fr/reference/management-api.md +++ b/docs-site/src/content/docs/fr/reference/management-api.md @@ -227,7 +227,7 @@ lui-même s'il souhaite ajouter une étoile au dépôt. | --- | --- | --- | | `GET /api/system/memory` | Renvoyer les mesures scalaires du processus, du tas, des flux, de l'état des réponses, du mécanisme de surveillance et des tours actifs | — | | `POST /api/system/restart` | Amorcer un redémarrage du processus qui attend l'évacuation des requêtes, sans retirer l'injection du client | Renvoie 202 ; les appels répétés signalent l'évacuation déjà en cours | -| `POST /api/stop` | Arrêter le service, restaurer Codex en mode natif, retirer l'injection Grok gérée et évacuer les requêtes du proxy | 409 conflit de propriété du service | +| `POST /api/stop` | Arrêter le service, restaurer Codex en mode natif, retirer l'injection Grok gérée et évacuer les requêtes du proxy | 409 conflit de propriété du service; 409 `respawnable_service` lorsqu'un wrapper du Planificateur de tâches Windows pourrait relancer le proxy et que l'appelant n'est pas `ocx stop` (rien n'est modifié) ; 409 lorsque le gestionnaire installé refuse de s'arrêter ; 409 `service_state_unknown` lorsque l'état du Planificateur de tâches ne peut pas être lu (rien n'est modifié ; réparez la requête puis réessayez) | | `GET /api/system/codex-app-server` | Indiquer si les serveurs d'application Codex en cours d'exécution sont antérieurs au catalogue de modèles actuel | — | | `POST /api/system/codex-restart` | Actualiser le catalogue, puis demander aux serveurs d'application Codex obsolètes de s'arrêter afin que le sélecteur de modèles se recharge | Renvoie 200 avec `code: partially_stopped` lorsqu'une cible ne s'arrête pas | diff --git a/docs-site/src/content/docs/guides/web-dashboard.md b/docs-site/src/content/docs/guides/web-dashboard.md index 6c2ca81e05..8c9b589fc2 100644 --- a/docs-site/src/content/docs/guides/web-dashboard.md +++ b/docs-site/src/content/docs/guides/web-dashboard.md @@ -53,7 +53,7 @@ the browser or password manager's decision. | **Logs** | Auto-refresh recent requests with tokens, requested effort and (when available) effective outbound effort, resolved model, provider, status, request id, duration, and error details. The detail view includes the exact reasoning wire field when the adapter emits one. Filter by opaque conversation/session id (when the client sends one) to total tokens and estimated list-price cost for the currently loaded Logs ring. | | **Usage / Debug** | Inspect token-usage coverage and trends, or enable opt-in provider transport and usage-extraction diagnostics. | | **Storage** | Read-only CODEX_HOME disk breakdown (sessions, archives, DBs, attachments). Optional archived cleanup: preview the oldest N%, then quarantine to `CODEX_HOME/.trash` (default) or permanently delete behind an explicit checkbox. **Auto-cleanup policy** is opt-in and **default OFF** (`storageCleanupPolicy.enabled`); configure threshold/target/schedule/mode on the Storage page, or trigger **Run now**. Quarantined entries can be restored from the Storage page (JSONL + threads). Active sessions stay read-only. Cleanup and restore are refused while Codex holds the newest/active `state_*.sqlite` locked. | -| **Stop** | Gracefully stop the proxy and installed background service, restore native Codex, and exit (`POST /api/stop`). | +| **Stop** | Gracefully stop the proxy and installed background service, restore native Codex, and exit (`POST /api/stop`). On Windows with the Task Scheduler backend the dashboard refuses and asks you to run `ocx stop` instead: that wrapper can respawn the proxy after the task ends, and only a stop running outside this process can verify the restart window before restoring your client config. Nothing is changed when it refuses. | ### Linking to a section @@ -217,7 +217,7 @@ The GUI is a thin client over the proxy's JSON management API. Useful endpoints | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | Add a pool account through browser login. | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | Read recent request metadata with optional tail, provider, and exact/class status filters. With `limit`/`offset`, paging walks backward from the newest row (`offset=0` returns the latest page). Response shape: `{ timeZone, total, logs }` where `total` is the filtered row count before pagination. | | `GET` / `PUT /api/subagent-models` | Read or set the five featured `spawn_agent` override models. | -| `POST /api/stop` | Stop the proxy/service, restore native Codex, and exit. | +| `POST /api/stop` | Stop the proxy/service, restore native Codex, and exit. Refused with `respawnable_service` on the Windows Task Scheduler backend, and with `service_state_unknown` when that state cannot be read; nothing is changed either way. | :::tip Adding **Ollama Cloud** or another catalog provider from the dashboard copies its text-versus-vision diff --git a/docs-site/src/content/docs/ja/guides/web-dashboard.md b/docs-site/src/content/docs/ja/guides/web-dashboard.md index c63c7a77e4..41a9b4b2d5 100644 --- a/docs-site/src/content/docs/ja/guides/web-dashboard.md +++ b/docs-site/src/content/docs/ja/guides/web-dashboard.md @@ -46,7 +46,7 @@ bun run dev:gui | **ログ** | トークン、要求された強度と(利用可能な場合は)実際に送信された強度、実際のモデル、プロバイダー、状態、リクエスト ID、所要時間、エラー詳細を含む最近のリクエストを自動更新します。アダプターが reasoning パラメーターを送信した場合、詳細表示に正確な wire field も表示されます。 | | **使用量 / デバッグ** | トークン使用量の測定範囲と推移を見るか、オプションのプロバイダートランスポート/使用量抽出診断をオンにします。 | | **ストレージ** | CODEX_HOME のディスク内訳(セッション、アーカイブ、DB、添付)を読み取り専用で表示。任意のアーカイブクリーンアップ: 最古 N% をプレビューし、既定では `CODEX_HOME/.trash` へ隔離、または明示チェックで完全削除。**自動クリーンアップ方針**はオプトインで**既定 OFF**(`storageCleanupPolicy.enabled`)。Storage ページでしきい値/目標/スケジュール/モードを設定するか **今すぐ実行**。隔離エントリは Storage ページから復元可能(JSONL + スレッド)。アクティブセッションは読み取り専用。最新/アクティブな `state_*.sqlite` がロック中はクリーンアップと復元を拒否。 | -| **停止** | プロキシとインストールされたバックグラウンドサービスを正常終了しネイティブ Codex を復元した後終了します(`POST /api/stop`)。 | +| **停止** | プロキシとインストールされたバックグラウンドサービスを正常終了しネイティブ Codex を復元した後終了します(`POST /api/stop`)。ただし Windows のタスク スケジューラ バックエンドではダッシュボードが拒否し、`ocx stop` の実行を促します。タスク終了後もラッパーがプロキシを再起動しうるため、クライアント設定を戻す前にその再起動区間を確認できるのはプロキシの外で動く stop だけです。拒否されたときは何も変更されません。 | ### セクションへのリンク @@ -147,7 +147,7 @@ GUI はプロキシの JSON 管理 API を使うシンクライアントです | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | ブラウザログインでプールアカウントを追加します。 | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | tail、プロバイダー、正確な状態コードまたは状態等級で最近のリクエストメタデータを参照します。`limit`/`offset` は最新行から過去方向にページングします(`offset=0` が最新ページ)。応答は `{ timeZone, total, logs }` で、`total` はページング前の一致件数です。 | | `GET` / `PUT /api/subagent-models` | `spawn_agent` に優先公開するモデル 5 つを読むか設定します。 | -| `POST /api/stop` | プロキシ/サービスを停止しネイティブ Codex を復元した後終了します。 | +| `POST /api/stop` | プロキシ/サービスを停止しネイティブ Codex を復元した後終了します。Windows タスク スケジューラ バックエンドでは `respawnable_service`、その状態を読み取れない場合は `service_state_unknown` で拒否し、どちらの場合も何も変更されません。 | :::tip ダッシュボードで **Ollama Cloud** のようなカタログプロバイダーを追加するとテキスト/ビジョンモデル分類が保存された diff --git a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md index 4d6e7bcad7..6faca72d3a 100644 --- a/docs-site/src/content/docs/ja/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ja/reference/cli/lifecycle.md @@ -24,7 +24,7 @@ ocx start --port 8080 ### `ocx stop` -実行中のプロキシを (PID によって) 停止し、PID ファイルを削除して、ネイティブ Codex を復元します。マネージド バックグラウンド サービスがインストールされている場合、`ocx stop` はそれを最初に停止するため、プロキシを再起動できません。同じアクションは、Web ダッシュボードの **停止** ボタン (`POST /api/stop`) から実行できます。 +実行中のプロキシを (PID によって) 停止し、PID ファイルを削除して、ネイティブ Codex を復元します。マネージド バックグラウンド サービスがインストールされている場合、`ocx stop` はそれを最初に停止するため、プロキシを再起動できません。Web ダッシュボードの **停止** ボタンは同じ処理 (`POST /api/stop`) を実行しますが、Windows タスク スケジューラだけは例外です。タスク終了後もラッパーがプロキシを再起動しうるため、ダッシュボードは `respawnable_service` で拒否し、何も変更せずに `ocx stop` の実行を促します。 ### `ocx restart` diff --git a/docs-site/src/content/docs/ja/reference/management-api.md b/docs-site/src/content/docs/ja/reference/management-api.md index 7024b2ef45..8d1f652392 100644 --- a/docs-site/src/content/docs/ja/reference/management-api.md +++ b/docs-site/src/content/docs/ja/reference/management-api.md @@ -195,7 +195,7 @@ Authorization: Bearer | --- | --- | --- | | `GET /api/system/memory` |スカラー プロセス、ヒープ、ストリーム、応答状態、ウォッチドッグ、およびアクティブ ターン メトリックを返します。 — | | `POST /api/system/restart` |クライアント インジェクションを削除せずに、ドレイン対応プロセスの再起動を開始します。 202 を返します。繰り返しの呼び出しにより、既存の排水が報告されます。 -| `POST /api/stop` |サービスを停止し、ネイティブ Codex を復元し、マネージド Grok インジェクションを削除し、プロキシをドレインします。 409 サービス所有権の競合 | +| `POST /api/stop` | サービスを停止し、ネイティブ Codex を復元し、マネージド Grok インジェクションを削除し、プロキシをドレインします | 409 サービス所有権の競合、409 `respawnable_service`(Windows タスク スケジューラのラッパーがプロキシを再起動しうる状態で、呼び出し元が `ocx stop` でない場合。何も変更されません)、409 インストール済みマネージャが停止を拒否した場合、409 `service_state_unknown`(タスク スケジューラの状態を読み取れない場合。何も変更されません。クエリを修復して再試行してください) | ### Codex認証の委任 diff --git a/docs-site/src/content/docs/ko/guides/web-dashboard.md b/docs-site/src/content/docs/ko/guides/web-dashboard.md index 9ec0fe4092..f831016288 100644 --- a/docs-site/src/content/docs/ko/guides/web-dashboard.md +++ b/docs-site/src/content/docs/ko/guides/web-dashboard.md @@ -46,7 +46,7 @@ bun run dev:gui | **Logs** | 토큰, 요청한 강도와 (사용 가능한 경우) 실제 전송 강도, 실제 모델, 프로바이더, 상태, 요청 id, 소요 시간, 오류 상세가 포함된 최근 요청을 자동 갱신합니다. 어댑터가 reasoning 매개변수를 전송한 경우 상세 보기에 정확한 wire field도 표시됩니다. 클라이언트가 보낸 불투명 대화/세션 id로 필터하면 현재 로드된 Logs 링의 토큰·추정 정가 합계를 볼 수 있습니다. | | **Usage / Debug** | 토큰 사용량의 측정 범위와 추이를 보거나, 선택적 프로바이더 전송/사용량 추출 진단을 켭니다. | | **Storage** | CODEX_HOME 디스크 사용량(세션, 보관, DB, 첨부)을 읽기 전용으로 표시합니다. 선택적 보관 정리: 가장 오래된 N%를 미리본 뒤 기본으로 `CODEX_HOME/.trash`에 격리하거나, 명시 체크 후 영구 삭제합니다. **자동 정리 정책**은 opt-in이며 **기본 OFF**(`storageCleanupPolicy.enabled`)입니다. Storage 페이지에서 임계값/목표/일정/모드를 설정하거나 **지금 실행**하세요. Storage 페이지에서 격리 항목을 복원할 수 있습니다(JSONL + 스레드). 활성 세션은 읽기 전용입니다. Codex가 최신/활성 `state_*.sqlite`를 잠그면 정리와 복원을 거절합니다. | -| **Stop** | 프록시와 설치된 백그라운드 서비스를 정상 종료하고 네이티브 Codex를 복원한 뒤 끝냅니다(`POST /api/stop`). | +| **Stop** | 프록시와 설치된 백그라운드 서비스를 정상 종료하고 네이티브 Codex를 복원한 뒤 끝냅니다(`POST /api/stop`). 단, Windows 작업 스케줄러로 관리되는 경우에는 대시보드가 거절하고 `ocx stop`을 안내합니다. 작업이 끝나도 래퍼가 프록시를 다시 띄울 수 있어서, 클라이언트 설정을 되돌리기 전에 그 재시작 구간을 확인할 수 있는 건 프록시 바깥에서 도는 stop뿐입니다. 거절될 때는 아무것도 바뀌지 않습니다. | ### 섹션으로 바로 가기 @@ -168,7 +168,7 @@ GUI는 프록시의 JSON 관리 API를 사용하는 얇은 클라이언트입니 | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | 브라우저 로그인으로 pool 계정을 추가합니다. | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | tail, 프로바이더, 정확한 상태 코드 또는 상태 등급으로 최근 요청 메타데이터를 조회합니다. `limit`/`offset`은 최신 행에서 과거 방향으로 페이지네이션합니다(`offset=0`이 최신 페이지). 응답은 `{ timeZone, total, logs }`이며 `total`은 페이지네이션 전 필터 일치 건수입니다. | | `GET` / `PUT /api/subagent-models` | `spawn_agent`에 우선 노출할 모델 5개를 읽거나 설정합니다. | -| `POST /api/stop` | 프록시/서비스를 멈추고 네이티브 Codex를 복원한 뒤 종료합니다. | +| `POST /api/stop` | 프록시/서비스를 멈추고 네이티브 Codex를 복원한 뒤 종료합니다. Windows 작업 스케줄러 백엔드에서는 `respawnable_service`로, 그 상태를 읽을 수 없으면 `service_state_unknown`으로 거절하며, 두 경우 모두 아무것도 바뀌지 않습니다. | :::tip 대시보드에서 **Ollama Cloud** 같은 카탈로그 프로바이더를 추가하면 텍스트/비전 모델 분류가 저장된 diff --git a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md index f027979a48..0080f40942 100644 --- a/docs-site/src/content/docs/ko/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ko/reference/cli/lifecycle.md @@ -32,7 +32,7 @@ ocx start --port 8080 실행 중인 프록시를 PID 기준으로 중지하고, PID 파일을 삭제한 뒤 기본 Codex를 복원합니다. 관리형 백그라운드 서비스가 설치되어 있으면 `ocx stop`이 먼저 그 서비스를 중지하므로 프록시가 다시 -올라올 수 없습니다. 같은 동작은 웹 대시보드의 **Stop** 버튼(`POST /api/stop`)에서도 사용할 수 있습니다. +올라올 수 없습니다. 웹 대시보드의 **Stop** 버튼도 같은 동작(`POST /api/stop`)을 하지만, Windows 작업 스케줄러는 예외입니다. 작업이 끝나도 래퍼가 프록시를 다시 띄울 수 있어서, 대시보드는 `respawnable_service`로 거절하고 아무것도 바꾸지 않은 채 `ocx stop` 실행을 안내합니다. ### `ocx restart` diff --git a/docs-site/src/content/docs/ko/reference/management-api.md b/docs-site/src/content/docs/ko/reference/management-api.md index 5280b31cd4..10c8ff0694 100644 --- a/docs-site/src/content/docs/ko/reference/management-api.md +++ b/docs-site/src/content/docs/ko/reference/management-api.md @@ -198,7 +198,7 @@ Authorization: Bearer | --- | --- | --- | | `GET /api/system/memory` | 프로세스, heap, stream, response-state, watchdog, active-turn의 스칼라 메트릭을 반환합니다 | — | | `POST /api/system/restart` | 클라이언트 injection을 제거하지 않고 drain-aware 프로세스 재시작을 시작합니다 | 202 반환; 반복 호출은 기존 drain을 보고합니다 | -| `POST /api/stop` | 서비스를 중지하고, native Codex를 복원하며, 관리형 Grok injection을 제거하고, 프록시를 drain합니다 | 409 서비스 소유권 충돌 | +| `POST /api/stop` | 서비스를 중지하고, native Codex를 복원하며, 관리형 Grok injection을 제거하고, 프록시를 drain합니다 | 409 서비스 소유권 충돌; Windows 작업 스케줄러 래퍼가 프록시를 다시 띄울 수 있고 호출자가 `ocx stop`이 아니면 409 `respawnable_service`(아무것도 바뀌지 않음); 설치된 관리자가 정지를 거부하면 409; 작업 스케줄러 상태를 읽을 수 없으면 409 `service_state_unknown`(아무것도 바뀌지 않음, 조회를 고친 뒤 재시도) | ### Codex 인증 위임 diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index e728335fa4..cde46cf827 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -32,7 +32,11 @@ ocx start --port 8080 Stop the running proxy (by PID), remove the PID file, and restore native Codex. If a managed background service is installed, `ocx stop` also stops it first so it cannot respawn the proxy. -The same action is available from the web dashboard's **Stop** button (`POST /api/stop`). +The web dashboard's **Stop** button runs the same action (`POST /api/stop`) on every backend +except Windows Task Scheduler. There the wrapper can respawn the proxy after the task ends, +and only a stop running outside the proxy can verify that restart window before restoring +your client config — so the dashboard refuses with `respawnable_service`, changes nothing, +and asks you to run `ocx stop`. ### `ocx restart` diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index 1bfedb0bd6..2484e7e675 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -227,7 +227,7 @@ whether to star the repository. | --- | --- | --- | | `GET /api/system/memory` | Return scalar process, heap, stream, response-state, watchdog, and active-turn metrics | — | | `POST /api/system/restart` | Begin a drain-aware process restart without removing client injection | Returns 202; repeated calls report the existing drain | -| `POST /api/stop` | Stop the service, restore native Codex, remove managed Grok injection, and drain the proxy | 409 service ownership conflict | +| `POST /api/stop` | Stop the service, restore native Codex, remove managed Grok injection, and drain the proxy | 409 service ownership conflict; 409 `respawnable_service` when a Windows Task Scheduler wrapper could respawn the proxy and the caller is not `ocx stop` (nothing is changed); 409 when the installed manager refuses to stop; 409 `service_state_unknown` when the Task Scheduler state cannot be read (nothing is changed; repair the query and retry) | | `GET /api/system/codex-app-server` | Report whether running Codex app-servers predate the current model catalog | — | | `POST /api/system/codex-restart` | Refresh the catalog, then ask stale Codex app-servers to exit so the model picker reloads | Returns 200 with `code: partially_stopped` when a target survives | diff --git a/docs-site/src/content/docs/ru/guides/web-dashboard.md b/docs-site/src/content/docs/ru/guides/web-dashboard.md index 58f037dfd4..2779b167d7 100644 --- a/docs-site/src/content/docs/ru/guides/web-dashboard.md +++ b/docs-site/src/content/docs/ru/guides/web-dashboard.md @@ -46,7 +46,7 @@ bun run dev:gui | **Logs** | Автообновляемый список недавних запросов: токены, запрошенный и, когда доступен, фактически отправленный уровень рассуждений, фактическая модель, провайдер, статус, id запроса, длительность и подробности ошибок. Если адаптер отправляет параметр рассуждений, в подробностях также отображается точное wire-поле. Можно фильтровать по непрозрачному id диалога/сессии (если клиент его передаёт) и суммировать токены и оценочную стоимость по прайс-листу в пределах загруженного кольца Logs. | | **Usage / Debug** | Просмотр покрытия и трендов расхода токенов либо включение опциональной диагностики транспорта провайдеров и извлечения данных об использовании. | | **Storage** | Только чтение разбивки диска CODEX_HOME (сессии, архивы, БД, вложения). Опциональная очистка архива: предпросмотр самых старых N%, затем карантин в `CODEX_HOME/.trash` (по умолчанию) или безвозвратное удаление по явному флажку. **Политика автоочистки** — opt-in и **по умолчанию ВЫКЛ** (`storageCleanupPolicy.enabled`); порог/цель/расписание/режим на странице Storage или **Запустить сейчас**. Записи карантина можно восстановить со страницы Storage (JSONL + threads). Активные сессии только для чтения. Очистка и восстановление отклоняются, пока Codex держит блокировку новейшего/активного `state_*.sqlite`. | -| **Stop** | Корректная остановка прокси и установленного фонового сервиса, восстановление нативного Codex и выход (`POST /api/stop`). | +| **Stop** | Корректная остановка прокси и установленного фонового сервиса, восстановление нативного Codex и выход (`POST /api/stop`). На Windows с бэкендом планировщика заданий дашборд отказывает и просит выполнить `ocx stop`: обёртка может перезапустить прокси после завершения задачи, и проверить это окно перезапуска до восстановления клиентской конфигурации способен только stop, работающий вне прокси. При отказе ничего не изменяется. | ### Ссылки на разделы @@ -157,7 +157,7 @@ GUI — это тонкий клиент поверх JSON-API управлен | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | Добавление аккаунта пула через вход в браузере. | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | Чтение метаданных недавних запросов с необязательными фильтрами tail, провайдера и точного/классового статуса. `limit`/`offset` листают назад от самой новой строки (`offset=0` — последняя страница). Ответ: `{ timeZone, total, logs }`, где `total` — число совпадений до пагинации. | | `GET` / `PUT /api/subagent-models` | Чтение или настройка пяти выделенных моделей переопределения `spawn_agent`. | -| `POST /api/stop` | Остановка прокси/сервиса, восстановление нативного Codex и выход. | +| `POST /api/stop` | Остановка прокси/сервиса, восстановление нативного Codex и выход. Отклоняется с `respawnable_service` на бэкенде планировщика заданий Windows и с `service_state_unknown`, когда это состояние не удаётся прочитать; в обоих случаях ничего не изменяется. | :::tip Добавление **Ollama Cloud** или другого провайдера каталога из дашборда копирует его классификацию diff --git a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md index 362600d285..763fbf9afc 100644 --- a/docs-site/src/content/docs/ru/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/ru/reference/cli/lifecycle.md @@ -35,8 +35,7 @@ ocx start --port 8080 Остановить работающий прокси (по PID), удалить PID-file и восстановить native Codex. Если установлена managed background service, `ocx stop` сначала останавливает и её, чтобы она не -перезапустила прокси обратно. То же действие доступно из кнопки **Stop** в веб-дашборде -(`POST /api/stop`). +перезапустила прокси обратно. Кнопка **Stop** в веб-дашборде выполняет то же действие (`POST /api/stop`) на всех бэкендах, кроме планировщика заданий Windows: там обёртка может перезапустить прокси после завершения задачи, поэтому дашборд отказывает с `respawnable_service`, ничего не меняет и просит выполнить `ocx stop`. ### `ocx restart` diff --git a/docs-site/src/content/docs/ru/reference/management-api.md b/docs-site/src/content/docs/ru/reference/management-api.md index 5ab5788beb..516b30e530 100644 --- a/docs-site/src/content/docs/ru/reference/management-api.md +++ b/docs-site/src/content/docs/ru/reference/management-api.md @@ -221,7 +221,7 @@ Management-аутентификация доказывает доступ к п | --- | --- | --- | | `GET /api/system/memory` | Вернуть скалярные метрики процесса, heap, stream, response-state, watchdog и active-turn | — | | `POST /api/system/restart` | Начать restart процесса с учётом drain, не снимая client injection | Возвращает 202; повторные вызовы сообщают о текущем drain | -| `POST /api/stop` | Остановить службу, восстановить native Codex, убрать managed Grok injection и выполнить drain прокси | 409 service ownership conflict | +| `POST /api/stop` | Остановить службу, восстановить native Codex, убрать managed Grok injection и выполнить drain прокси | 409 service ownership conflict; 409 `respawnable_service`, когда обёртка планировщика заданий Windows может перезапустить прокси, а вызывающая сторона — не `ocx stop` (ничего не изменяется); 409, когда установленный менеджер отказывается останавливаться; 409 `service_state_unknown`, когда состояние планировщика заданий не удаётся прочитать (ничего не изменяется; исправьте запрос и повторите) | ### Делегирование аутентификации Codex diff --git a/docs-site/src/content/docs/tr/guides/web-dashboard.md b/docs-site/src/content/docs/tr/guides/web-dashboard.md index 5e5e16272d..955148a054 100644 --- a/docs-site/src/content/docs/tr/guides/web-dashboard.md +++ b/docs-site/src/content/docs/tr/guides/web-dashboard.md @@ -58,7 +58,7 @@ kararıdır. | **Günlükler** | Belirteçler, talep edilen çaba ve (varsa) etkili giden çaba, çözümlenen model, sağlayıcı, durum, istek kimliği, süre ve hata ayrıntılarıyla son istekleri otomatik yenileyin. Ayrıntı görünümü, adaptör bir tane yaydığında tam akıl yürütme hat alanını içerir. Yüklenen Günlükler halkası için toplam belirteçleri ve tahmini liste fiyatı maliyetini görmek üzere donuk görüşme/oturum kimliğine göre (istemci bir tane gönderdiğinde) filtreleyin. | | **Kullanım / Hata Ayıklama** | Belirteç kullanımı kapsamını ve eğilimlerini inceleyin veya isteğe bağlı sağlayıcı aktarımı ve kullanım çıkarma tanılamalarını etkinleştirin. | | **Depolama** | Salt okunur CODEX_HOME disk dökümü (oturumlar, arşivler, DB'ler, ekler). İsteğe bağlı arşivlenmiş temizleme: en eski %N'yi önizleyin, ardından `CODEX_HOME/.trash` konumuna karantinaya alın (varsayılan) veya açık bir onay kutusu arkasında kalıcı olarak silin. **Otomatik temizleme politikası** isteğe bağlıdır ve **varsayılan olarak KAPALIDIR** (`storageCleanupPolicy.enabled`); Depolama sayfasında eşik/hedef/zamanlama/mod yapılandırın veya **Şimdi çalıştır (Run now)**'ı tetikleyin. Karantinaya alınan girdiler Depolama sayfasından geri yüklenebilir (JSONL + iş parçacıkları). Aktif oturumlar salt okunur kalır. Codex en yeni/aktif `state_*.sqlite` dosyasını kilitli tuttuğu sürece temizleme ve geri yükleme reddedilir. | -| **Durdur** | Proxy'yi ve kurulu arka plan servisini zarif bir şekilde durdurun, yerel Codex'i geri yükleyin ve çıkın (`POST /api/stop`). | +| **Durdur** | Proxy'yi ve kurulu arka plan servisini zarif bir şekilde durdurun, yerel Codex'i geri yükleyin ve çıkın (`POST /api/stop`). Windows'ta Görev Zamanlayıcı arka ucunda panel reddeder ve `ocx stop` çalıştırmanızı ister: görev bittikten sonra sarmalayıcı proxy'yi yeniden başlatabilir ve bu yeniden başlatma penceresini istemci yapılandırmanız geri yüklenmeden önce yalnızca proxy dışında çalışan bir stop doğrulayabilir. Reddedildiğinde hiçbir şey değiştirilmez. | ### Bir bölüme bağlantı verme @@ -245,7 +245,7 @@ noktalar şunları içerir: | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | Tarayıcı girişi aracılığıyla bir havuz hesabı ekleyin. | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | İsteğe bağlı kuyruk, sağlayıcı ve tam/sınıf durum filtreleriyle son istek meta verilerini okuyun. `limit`/`offset` ile sayfalama en yeni satırdan geriye doğru ilerler (`offset=0` en son sayfayı döndürür). Yanıt şekli: `{ timeZone, total, logs }` burada `total`, sayfalamadan önceki filtrelenmiş satır sayısıdır. | | `GET` / `PUT /api/subagent-models` | Öne çıkan beş `spawn_agent` geçersiz kılma modelini okuyun veya ayarlayın. | -| `POST /api/stop` | Proxy'yi/servisi durdurun, yerel Codex'i geri yükleyin ve çıkın. | +| `POST /api/stop` | Proxy'yi/servisi durdurun, yerel Codex'i geri yükleyin ve çıkın. Windows Görev Zamanlayıcı arka ucunda `respawnable_service`, bu durum okunamadığında `service_state_unknown` ile reddedilir; her iki durumda da hiçbir şey değiştirilmez. | :::tip Kontrol panelinden **Ollama Cloud** veya başka bir katalog sağlayıcısı eklemek, diff --git a/docs-site/src/content/docs/tr/reference/cli/lifecycle.md b/docs-site/src/content/docs/tr/reference/cli/lifecycle.md index ff9089357e..425380b2fd 100644 --- a/docs-site/src/content/docs/tr/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/tr/reference/cli/lifecycle.md @@ -37,8 +37,7 @@ ocx start --port 8080 Çalışan proxy'yi (PID'ye göre) durdurun, PID dosyasını kaldırın ve yerel Codex'i geri yükleyin. Yönetilen bir arka plan servisi kuruluysa `ocx stop` proxy'yi -yeniden oluşturamaması için önce onu da durdurur. Aynı eylem web kontrol -panelinin **Durdur** düğmesinden de (`POST /api/stop`) kullanılabilir. +yeniden oluşturamaması için önce onu da durdurur. Web kontrol panelinin **Durdur** düğmesi aynı eylemi (`POST /api/stop`) Windows Görev Zamanlayıcı dışındaki tüm arka uçlarda çalıştırır: orada görev bittikten sonra sarmalayıcı proxy'yi yeniden başlatabilir, bu yüzden panel `respawnable_service` ile reddeder, hiçbir şeyi değiştirmez ve `ocx stop` çalıştırmanızı ister. ### `ocx restart` diff --git a/docs-site/src/content/docs/tr/reference/management-api.md b/docs-site/src/content/docs/tr/reference/management-api.md index cc5c293345..0ce4456cd9 100644 --- a/docs-site/src/content/docs/tr/reference/management-api.md +++ b/docs-site/src/content/docs/tr/reference/management-api.md @@ -240,7 +240,7 @@ dolaşmamalıdır. Depoya yıldız verip vermeyeceğini kullanıcı seçmelidir. | --- | --- | --- | | `GET /api/system/memory` | Skaler süreç, yığın (heap), akış, yanıt durumu, denetleyici ve aktif tur metriklerini döndürün | — | | `POST /api/system/restart` | İstemci enjeksiyonunu kaldırmadan boşaltma duyarlı bir süreç yeniden başlatması başlatın | 202 döndürür; tekrarlanan çağrılar mevcut boşaltmayı bildirir | -| `POST /api/stop` | Servisi durdurun, yerel Codex'i geri yükleyin, yönetilen Grok enjeksiyonunu kaldırın ve proxy'yi boşaltın | 409 servis sahipliği çakışması | +| `POST /api/stop` | Servisi durdurun, yerel Codex'i geri yükleyin, yönetilen Grok enjeksiyonunu kaldırın ve proxy'yi boşaltın | 409 servis sahipliği çakışması; çağıran `ocx stop` değilken bir Windows Görev Zamanlayıcı sarmalayıcısı proxy'yi yeniden başlatabiliyorsa 409 `respawnable_service` (hiçbir şey değiştirilmez); kurulu yönetici durmayı reddederse 409; Görev Zamanlayıcı durumu okunamıyorsa 409 `service_state_unknown` (hiçbir şey değiştirilmez; sorguyu onarıp yeniden deneyin) | ### Codex kimlik doğrulama yetkilendirmesi diff --git a/docs-site/src/content/docs/zh-cn/guides/web-dashboard.md b/docs-site/src/content/docs/zh-cn/guides/web-dashboard.md index e8f7e42d21..1c8fe541d4 100644 --- a/docs-site/src/content/docs/zh-cn/guides/web-dashboard.md +++ b/docs-site/src/content/docs/zh-cn/guides/web-dashboard.md @@ -45,7 +45,7 @@ bun run dev:gui | **Logs** | 自动刷新近期请求,显示 token、请求强度以及(可用时)实际发送强度、实际模型、provider、状态、request id、耗时和错误详情。适配器发送 reasoning 参数时,详情中还会显示准确的 wire field。可按不透明会话/对话 ID(客户端提供时)筛选,并对当前已加载的 Logs 环形缓冲合计 token 与估算标价成本。 | | **Usage / Debug** | 查看 token usage 覆盖率与趋势,或启用可选的 provider transport 和 usage 提取诊断。 | | **Storage** | 只读查看 CODEX_HOME 磁盘占用(会话、归档、数据库、附件)。可选归档清理:预览最旧 N%,默认隔离到 `CODEX_HOME/.trash`,或勾选后永久删除。**自动清理策略**为可选且**默认关闭**(`storageCleanupPolicy.enabled`);可在 Storage 页配置阈值/目标/计划/模式,或点「立即运行」。可在 Storage 页从隔离区恢复(JSONL + 线程)。活动会话保持只读。Codex 锁定最新/活动的 `state_*.sqlite` 时拒绝清理与恢复。 | -| **Stop** | 优雅地停止代理和已安装的后台服务,恢复原生 Codex 并退出(`POST /api/stop`)。 | +| **Stop** | 优雅地停止代理和已安装的后台服务,恢复原生 Codex 并退出(`POST /api/stop`)。在使用任务计划程序后端的 Windows 上,仪表板会拒绝并提示改用 `ocx stop`:任务结束后包装器仍可能重新拉起代理,只有运行在代理之外的 stop 才能在恢复客户端配置前确认这个重启窗口。被拒绝时不会做任何更改。 | ### 链接到某个部分 @@ -139,7 +139,7 @@ GUI 是代理 JSON 管理 API 之上的轻量客户端。常用 endpoint 包括 | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | 通过浏览器登录添加池账号。 | | `GET /api/logs?tail=50&limit=20&offset=0&provider=...&status=5xx` | 使用 tail、provider、精确状态码或状态类别筛选近期请求元数据。`limit`/`offset` 从最新一行向前分页(`offset=0` 为最新一页)。响应为 `{ timeZone, total, logs }`,其中 `total` 为分页前的匹配行数。 | | `GET` / `PUT /api/subagent-models` | 读取或设置五个置顶的 `spawn_agent` override 模型。 | -| `POST /api/stop` | 停止代理/服务,恢复原生 Codex 并退出。 | +| `POST /api/stop` | 停止代理/服务,恢复原生 Codex 并退出。在 Windows 任务计划程序后端会以 `respawnable_service` 拒绝,无法读取该状态时以 `service_state_unknown` 拒绝;两种情况都不会做任何更改。 | :::tip 从仪表盘添加 **Ollama Cloud** 或其他目录型 provider 时,其文本/视觉模型分类会写入保存的 diff --git a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md index 277b14981b..d6f5c6d219 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md @@ -24,7 +24,7 @@ ocx start --port 8080 ### `ocx stop` -停止正在运行的代理(按 PID),移除 PID 文件,并恢复原生 Codex。如果安装了受管后台服务,`ocx stop` 还会先停止该服务,这样它就无法重新拉起代理。Web 仪表盘中的 **Stop** 按钮也提供同样的操作(`POST /api/stop`)。 +停止正在运行的代理(按 PID),移除 PID 文件,并恢复原生 Codex。如果安装了受管后台服务,`ocx stop` 还会先停止该服务,这样它就无法重新拉起代理。Web 仪表盘的 **Stop** 按钮在多数后端执行同样的操作(`POST /api/stop`),但 Windows 任务计划程序除外:任务结束后包装器仍可能重新拉起代理,只有运行在代理之外的 stop 才能在恢复客户端配置前确认这个重启窗口,因此仪表盘会以 `respawnable_service` 拒绝、不做任何更改,并提示改用 `ocx stop`。 ### `ocx restart` diff --git a/docs-site/src/content/docs/zh-cn/reference/management-api.md b/docs-site/src/content/docs/zh-cn/reference/management-api.md index 09910a7a46..fe3568e3f0 100644 --- a/docs-site/src/content/docs/zh-cn/reference/management-api.md +++ b/docs-site/src/content/docs/zh-cn/reference/management-api.md @@ -195,7 +195,7 @@ Authorization: Bearer | --- | --- | --- | | `GET /api/system/memory` | 返回标量级的进程、堆、流、响应状态、看门狗和活跃回合指标 | — | | `POST /api/system/restart` | 在不移除客户端注入的情况下,开始一次考虑排空的进程重启 | 返回 202;重复调用会报告现有排空 | -| `POST /api/stop` | 停止服务、恢复原生 Codex、移除受管 Grok 注入并排空代理 | 409 服务所有权冲突 | +| `POST /api/stop` | 停止服务、恢复原生 Codex、移除受管 Grok 注入并排空代理 | 409 服务所有权冲突;当 Windows 任务计划程序包装器可能重新拉起代理且调用方不是 `ocx stop` 时返回 409 `respawnable_service`(不会做任何更改);已安装的管理器拒绝停止时返回 409;无法读取任务计划程序状态时返回 409 `service_state_unknown`(不会做任何更改;修复查询后重试) | ### Codex 身份验证委托 diff --git a/docs-site/src/content/docs/zh-tw/guides/web-dashboard.md b/docs-site/src/content/docs/zh-tw/guides/web-dashboard.md index 67828d56b0..4b22882f66 100644 --- a/docs-site/src/content/docs/zh-tw/guides/web-dashboard.md +++ b/docs-site/src/content/docs/zh-tw/guides/web-dashboard.md @@ -49,7 +49,7 @@ GUI session 簽發到服務的頁面中,並在到期或代理重啟時靜默 | **Models** | 開關原生 GPT 與路由模型,設定 provider allowlist、上下文上限、v1/base/v2 以及 v2 thread 數量。 | | **Logs** | 自動重新整理近期請求,顯示 token、請求強度、實際模型、provider、狀態、request id、耗時和錯誤詳情。 | | **Usage / Debug** | 檢視 token usage 覆蓋率與趨勢,或啟用可選的 provider transport 和 usage 提取診斷。 | -| **Stop** | 優雅地停止代理和已安裝的後臺服務,恢復原生 Codex 並退出(`POST /api/stop`)。 | +| **Stop** | 優雅地停止代理和已安裝的後臺服務,恢復原生 Codex 並退出(`POST /api/stop`)。在使用工作排程器後端的 Windows 上,儀表板會拒絕並提示改用 `ocx stop`:工作結束後包裝程序仍可能重新啟動 Proxy,只有執行在 Proxy 之外的 stop 才能在還原用戶端設定前確認這個重啟視窗。被拒絕時不會做任何變更。 | ### 連結到某個部分 @@ -135,7 +135,7 @@ GUI 是代理 JSON 管理 API 之上的輕量用戶端。常用 endpoint 包括 | `POST /api/codex-auth/login` · `GET /api/codex-auth/login-status` | 透過瀏覽器登入新增池帳號。 | | `GET /api/logs?tail=50&provider=...&status=5xx` | 使用 tail、provider、精確狀態碼或狀態類別篩選近期請求後設資料。 | | `GET` / `PUT /api/subagent-models` | 讀取或設定五個置頂的 `spawn_agent` override 模型。 | -| `POST /api/stop` | 停止代理/服務,恢復原生 Codex 並退出。 | +| `POST /api/stop` | 停止代理/服務,恢復原生 Codex 並退出。在 Windows 工作排程器後端會以 `respawnable_service` 拒絕,無法讀取該狀態時以 `service_state_unknown` 拒絕;兩種情況都不會做任何變更。 | :::tip 從儀表板新增 **Ollama Cloud** 或其他目錄型 provider 時,其文字/視覺模型分類會寫入儲存的 diff --git a/docs-site/src/content/docs/zh-tw/reference/cli/lifecycle.md b/docs-site/src/content/docs/zh-tw/reference/cli/lifecycle.md index a8fd9f5c08..11fa127e23 100644 --- a/docs-site/src/content/docs/zh-tw/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/zh-tw/reference/cli/lifecycle.md @@ -24,7 +24,7 @@ ocx start --port 8080 ### `ocx stop` -停止執行中的代理(依 PID)、移除 PID 檔案,並還原原生 Codex。若已安裝受管背景服務,`ocx stop` 也會先停止它,使其無法重新生成代理。相同動作亦可從網頁儀表板的 **Stop** 按鈕執行(`POST /api/stop`)。 +停止執行中的代理(依 PID)、移除 PID 檔案,並還原原生 Codex。若已安裝受管背景服務,`ocx stop` 也會先停止它,使其無法重新生成代理。網頁儀表板的 **Stop** 按鈕在多數後端執行相同動作(`POST /api/stop`),但 Windows 工作排程器除外:工作結束後包裝程序仍可能重新啟動 Proxy,因此儀表板會以 `respawnable_service` 拒絕、不做任何變更,並請你改用 `ocx stop`。 ### `ocx restart` diff --git a/docs-site/src/content/docs/zh-tw/reference/management-api.md b/docs-site/src/content/docs/zh-tw/reference/management-api.md index 3c996662cb..5dc158a98d 100644 --- a/docs-site/src/content/docs/zh-tw/reference/management-api.md +++ b/docs-site/src/content/docs/zh-tw/reference/management-api.md @@ -195,7 +195,7 @@ Session 簽發在需要 data-plane 認證時停用,這包含遠端綁定。遠 | --- | --- | --- | | `GET /api/system/memory` | 回傳純量行程、heap、串流、回應狀態、看門狗與活躍回合指標 | — | | `POST /api/system/restart` | 在不移除客戶端注入的情況下開始感知排空的行程重啟 | 回傳 202;重複呼叫回報既有的排空 | -| `POST /api/stop` | 停止服務、還原原生 Codex、移除受管 Grok 注入並排空代理 | 409 服務擁有權衝突 | +| `POST /api/stop` | 停止服務、還原原生 Codex、移除受管 Grok 注入並排空代理 | 409 服務擁有權衝突;當 Windows 工作排程器包裝程序可能重新啟動 Proxy 且呼叫端不是 `ocx stop` 時回傳 409 `respawnable_service`(不會做任何變更);已安裝的管理器拒絕停止時回傳 409;無法讀取工作排程器狀態時回傳 409 `service_state_unknown`(不會做任何變更;修復查詢後重試) | ### Codex 認證委派 diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index e4aa52f465..e21536ce89 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -121,14 +121,31 @@ const commandRunners: Record = { if (desired.status === "unchanged") { const { classifyNativeRoutedResidue } = await import("../codex/native-residue"); if (classifyNativeRoutedResidue().kind === "clean") { - const alreadyOff = "Codex integration is already OFF and native; no Codex files changed."; + // The Codex half being a no-op says nothing about the Grok half. Returning here + // without stripping the fence meant `ocx restore` could report success while Grok + // still pointed at a stopped proxy — and the deferred-teardown recovery path + // (#3008) tells operators to run exactly this command before deleting a receipt, + // so the incomplete teardown would be signed off and the obligation erased. + let grokNote = ""; + let grokCode = 0; + try { + const g = stripGrokConfig(); + if (g.changed) grokNote = ` ${g.message}`; + else if (!g.ok) { grokNote = ` Grok config cleanup failed: ${g.message}`; grokCode = 1; } + } catch (err) { + grokNote = ` Grok config cleanup failed: ${err instanceof Error ? err.message : String(err)}`; + grokCode = 1; + } + const alreadyOff = `Codex integration is already OFF and native; no Codex files changed.${grokNote}`; if (restoreJson) { const { skippedRestoreEnvelope } = await import("../codex/inject"); - console.log(JSON.stringify(skippedRestoreEnvelope(true, alreadyOff))); - } else { + console.log(JSON.stringify(skippedRestoreEnvelope(grokCode === 0, alreadyOff))); + } else if (grokCode === 0) { console.log(alreadyOff); + } else { + console.error(alreadyOff); } - return 0; + return grokCode; } } let r: { success: boolean; message: string }; @@ -137,26 +154,40 @@ const commandRunners: Record = { } catch (err) { r = { success: false, message: err instanceof Error ? err.message : String(err) }; } + // Grok BEFORE either output. The JSON path used to return here, so `ocx restore --json` + // (and `ocx eject --json`, the same runner) could report success while the fence still + // pointed at the stopped proxy — and the deferred-teardown recovery on this branch + // tells operators to run exactly this before deleting a receipt (#3008). + let grokFailure: string | null = null; + let grokChangedMessage: string | null = null; + try { + const g = stripGrokConfig(); + if (g.changed) grokChangedMessage = g.message; + else if (!g.ok) grokFailure = g.message; + } catch (err) { + grokFailure = err instanceof Error ? err.message : String(err); + } if (restoreJson) { // Spawned callers need the artifact-level result to distinguish a busy // history worker from a successful native restore. Keep stdout machine - // readable; human framing remains the default command contract. - console.log(JSON.stringify(r)); - return r.success ? 0 : 1; + // readable — the Codex artifact schema is unchanged; the Grok outcome is + // folded into success/message so a caller cannot read a half teardown as done. + const message = grokFailure + ? `${r.message} Grok config cleanup failed: ${grokFailure}` + : grokChangedMessage ? `${r.message} ${grokChangedMessage}` : r.message; + console.log(JSON.stringify({ ...r, success: r.success && !grokFailure, message })); + return r.success && !grokFailure ? 0 : 1; } if (r.success) console.log(`✅ ${r.message}`); else { console.error(`⚠️ ${r.message}`); } let code = r.success ? 0 : 1; - try { - const g = stripGrokConfig(); - if (g.changed) console.log(`✅ ${g.message}`); - else if (!g.ok) { - console.error(`⚠️ ${g.message}`); - code = 1; - } - } catch { /* best-effort */ } + if (grokChangedMessage) console.log(`✅ ${grokChangedMessage}`); + if (grokFailure) { + console.error(`⚠️ ${grokFailure}`); + code = 1; + } if (r.success) { console.log("Codex integration is OFF and plain `codex` now runs natively. Switch back with: ocx restore back"); } else { diff --git a/src/cli/index.ts b/src/cli/index.ts index 56abb5d05a..98e041a188 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -2,6 +2,7 @@ import { spawn } from "node:child_process"; import { currentExternalCodexModelProvider, restoreNativeCodex, restoreNativeCodexAsync, shouldInjectApiAuthHeader } from "../codex/inject"; import { stripGrokConfig } from "../grok/inject"; +import { STOP_HISTORY_INCOMPLETE_EXIT_CODE } from "../update/stop-contract.mjs"; import { describeHistoryJobFailure, resolveCodexHistoryJobTarget, @@ -25,7 +26,16 @@ import { writePid, writeRuntimePort, } from "../config/process-state"; +import { + claimPendingTeardown, + clearPendingTeardown, + isPendingTeardownAbandoned, + listPendingTeardowns, + pendingTeardownPathFor, + quarantinePendingTeardown, +} from "../config/pending-teardown"; import { collectStatus, unusedProxyWarningLines } from "./status"; +import { endpointsToProve, everyEndpointProvenDown, sharedTeardownAuthorized, type UninstallObservation } from "./uninstall-plan"; import { takeFlag } from "./runtime-api"; import { @@ -44,9 +54,9 @@ import { findLiveProxy, probeHostname, type LiveProxy } from "../server/proxy-li import { createReadinessGate } from "../server/readiness"; import { runReady, type ReadyArgs } from "./ready"; import { runCli } from "./root"; -import { ProxyOwnershipRefusedError, stopProxy } from "../lib/process-control"; +import { isProcessAlive, ProxyOwnershipRefusedError, stopProxy } from "../lib/process-control"; import { loadServiceTokenFromFile } from "../lib/service-secrets"; -import { assertNotAdminToken, diagnoseService, isServiceOwnershipError, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalled, uninstallServiceIfInstalled } from "../service"; +import { assertNotAdminToken, diagnoseService, isServiceOwnershipError, proxyStillLiveAfterStop, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalledDetailed, uninstallServiceIfInstalled, uninstallServiceDetailed } from "../service"; import { formatStartupRoutingDetail, startupHealthSummary } from "../codex/autostart-health"; import { drainAndShutdown, isRecyclingForExit, startServer } from "../server"; import { injectSystemEnv, reconcileShellHook, revertSystemEnv, uninstallShellHook } from "../server/system-env"; @@ -631,17 +641,35 @@ async function handleRestartStartWhenStopped(): Promise { return handleEnsure({ existingIsSuccess: false }); } -async function restoreSharedClientStateAfterStop(): Promise { - let restored = true; +/** + * Restore shared client state after a stop. + * + * Returns the two failure kinds separately. `historyOnly` means teardown succeeded and + * only Codex history metadata could not be finalized: the proxy is down, the service is + * stopped, and a manifest is waiting for review. `other` means something that actually + * removes state a client depends on. + * + * The distinction exists because `ocx update` must proceed for the first and abort for the + * second, and it can only see an exit code (#3008). + */ +async function restoreSharedClientStateAfterStop(): Promise<{ historyOnly: boolean; other: boolean }> { + let historyOnly = false; + let other = false; try { const result = await restoreNativeCodexAsync(); if (result.success) console.log(`↩️ ${result.message}`); else { - restored = false; + // Codex history is the one restore whose failure leaves the runtime consistent: the + // manifest is retained and the routed metadata is untouched. Config and catalog are + // not — a client reads those, so their failure is a real teardown failure. + const artifacts = result.artifacts; + const configOrCatalogFailed = artifacts.config.state === "failed" || artifacts.catalog.state === "failed"; + if (!configOrCatalogFailed && artifacts.history.state === "failed") historyOnly = true; + else other = true; console.error(`⚠️ ${result.message}`); } } catch (error) { - restored = false; + other = true; console.error(`⚠️ Native Codex restore failed: ${error instanceof Error ? error.message : String(error)}`); } @@ -649,16 +677,58 @@ async function restoreSharedClientStateAfterStop(): Promise { try { const grok = stripGrokConfig(); if (grok.changed) console.log(`↩️ ${grok.message}`); - else if (!grok.ok) { restored = false; console.error(`⚠️ ${grok.message}`); } + else if (!grok.ok) { other = true; console.error(`⚠️ ${grok.message}`); } } catch (error) { - restored = false; + other = true; console.error(`⚠️ Grok config restore failed: ${error instanceof Error ? error.message : String(error)}`); } - return restored; + return { historyOnly, other }; } async function handleStop() { + // The receipt must name the endpoint the owner was stopping — an obligation nobody can + // locate cannot be proven discharged. Only the runtime record knows it; a proxy started + // with an explicit --port is not on the configured one. + const endpointOf = (runtime: { port: number; hostname?: string } | null): { hostname: string; port: number } | null => + runtime?.port ? { hostname: runtime.hostname ?? "127.0.0.1", port: runtime.port } : null; + // Last-resort endpoint for a receipt: the address this home is configured to serve on, + // which is what a later recovery probe would ask about anyway. + const configuredEndpoint = (): { hostname: string; port: number } => { + try { + const config = loadConfig(); + return { + hostname: config.hostname ?? "127.0.0.1", + port: typeof config.port === "number" && config.port > 0 ? config.port : 10100, + }; + } catch { + return { hostname: "127.0.0.1", port: 10100 }; + } + }; + // Only a definitive "nothing is answering" authorizes finishing somebody else's + // abandoned teardown. The tri-state probe distinguishes that from "we could not tell" + // (timeout, a listener that withholds /healthz), which `findLiveProxy` collapses into + // the same null (#3008). + const abandonedTeardownIsSafeToFinish = async ( + endpoint: { hostname: string; port: number } | null, + ): Promise => { + // The endpoint has to come from the receipt. A crashed owner usually leaves no + // runtime-port record, and the configured port is the wrong question for a proxy + // started with an explicit --port: it refuses while the live one keeps serving. + // An obligation that cannot name its endpoint cannot be proven discharged. + if (!endpoint) return false; + try { + const { probeProxyLiveness } = await import("../update/proxy-liveness-probe.mjs"); + return probeProxyLiveness(endpoint.port, endpoint.hostname) === "dead"; + } catch { + // A probe that could not run is not evidence of absence. + return false; + } + }; let stopFailed = false; + let historyOnlyFailure = false; + // Only Task Scheduler respawns after a successful stop (#764), so only it earns the + // restart-window wait; launchd, systemd and WinSW are down when they say so. + let schedulerCanRespawn = false; let stoppedService = false; // An ownership mismatch means the service manager was never even contacted: the installed // service is still live and will respawn the proxy. Tearing down SHARED state in that @@ -666,9 +736,81 @@ async function handleStop() { // service — the exact failure this flag prevents. A plain stop failure is different: we // tried, so local teardown still proceeds. let ownershipBlocked = false; + // Deferring shared teardown to this process is an obligation, so record it on disk + // before asking for it (#3008). A parent that dies mid-stop would otherwise leave the + // client config routed at a proxy that is already gone, with nothing to find later. + // + // `inheritedTeardowns` is the inverse case: PREVIOUS stops that left obligations + // unfinished. Snapshot them BEFORE this run claims anything, so this run's own receipt + // is never mistaken for one it inherited. + const inheritedTeardowns = listPendingTeardowns() + .filter(read => isPendingTeardownAbandoned(read, isProcessAlive)); + let teardownNonce: string | undefined; + const claimTeardown = (endpoint: { hostname: string; port: number }, endpointSource: "exact" | "guessed") => { + if (teardownNonce) return; + try { + teardownNonce = claimPendingTeardown(endpoint, endpointSource).nonce; + } catch (err) { + // Without a receipt the proxy performs its own teardown, which is the pre-#3008 + // behaviour: correct for every backend that cannot respawn, and merely early for + // Task Scheduler. Losing the deferral is far better than losing the stop. + console.warn(`⚠️ Could not record the deferred-teardown receipt: ${err instanceof Error ? err.message : String(err)}`); + } + }; + /** + * One stop target, used for BOTH the receipt and the request. + * + * Deriving them separately meant the receipt could name a different endpoint than the + * one actually contacted, and a proxy with no runtime record got no receipt at all — + * silently reopening the parent-crash window on the path where the stop is a hard kill + * and no child teardown runs at all. + * + * So the caller supplies whatever endpoint it already discovered: the orphan path knows + * one from `findLiveProxy` even when the runtime record is gone. + * + * When nothing resolves, the graceful request cannot be made at all — `stopProxy` goes + * straight to the kill ladder, no child teardown runs, and there is no receipt to leave + * behind. A warning does not make that durable, so the receipt is claimed FIRST against + * the endpoint this process would restore anyway. It is the configured listen address, + * which is the same address every recovery probe would ask about, and an obligation + * recorded against it is strictly better than none: at worst the probe cannot confirm + * A guessed endpoint is NOT evidence, so the receipt records which kind it holds: a + * guessed one fails closed into manual recovery rather than letting a later probe read + * "the configured port refuses" as proof that the right proxy is down. + */ + const stopWithDeferral = async (pid: number, discovered?: { hostname: string; port: number } | null): Promise => { + // Resolve ONCE. Reading the runtime record twice let the receipt name the configured + // guess while the request went to a runtime endpoint that appeared in between. + const exact = discovered ?? endpointOf(readRuntimePort(pid)); + claimTeardown(exact ?? configuredEndpoint(), exact ? "exact" : "guessed"); + await stopProxy(pid, { + deferSharedTeardownNonce: teardownNonce, + // Only an exact endpoint may direct the request; the configured fallback is a guess + // good enough to record an obligation against, not to POST a stop to. + runtimeEndpoint: exact ?? undefined, + }); + }; try { - stoppedService = stopServiceIfInstalled(); - if (stoppedService) console.log("🛑 Service manager stopped (won't respawn)."); + const serviceStop = stopServiceIfInstalledDetailed(); + stoppedService = serviceStop === "stopped" || serviceStop === "stopped-respawnable"; + schedulerCanRespawn = serviceStop === "stopped-respawnable"; + // No "won't respawn" claim here: a stopped Task Scheduler can still respawn through + // its wrapper, which the verification below is what actually settles. + if (stoppedService) console.log("🛑 Service manager stopped."); + if (serviceStop === "failed") { + // A manager that would not stop can respawn the proxy. That is a real stop failure, + // not a history-only one, and an update must not replace files over it (#3008). + stopFailed = true; + console.error("❌ The installed service manager did not stop; it may respawn the proxy."); + } + if (serviceStop === "state-unknown") { + // Nothing refused to stop — the scheduler state could not be READ. Saying "did not + // stop" sends the operator looking for the wrong problem, and `/api/stop` answers + // the same case with service_state_unknown. + stopFailed = true; + console.error("❌ The Windows Task Scheduler state could not be read, so this stop cannot tell whether a wrapper would respawn the proxy."); + console.error(" Run 'ocx service status' to see the query error, repair Task Scheduler access, then retry."); + } } catch (err) { if (isServiceOwnershipError(err)) { ownershipBlocked = true; @@ -685,7 +827,11 @@ async function handleStop() { try { // Graceful-first (management-API drain) — on Windows this is the only path where // the proxy's shutdown handlers actually run; taskkill /F is the fallback inside. - await stopProxy(pid); + // Shared teardown is deferred to this process: it happens after the respawn + // verification below, so a survivor does not get its client config pulled first. + // The receipt goes down first — the proxy honours the deferral only when it can + // see one, so an unrecordable claim degrades to the child doing its own teardown. + await stopWithDeferral(pid); console.log(`✅ Proxy (PID ${pid}) stopped.`); removePid(pid); removeRuntimePort(pid); @@ -713,7 +859,9 @@ async function handleStop() { const live = await findLiveProxy(); if (live?.pid) { try { - await stopProxy(live.pid); + // The probe already found where it answers, and on this path the runtime record is + // typically what went missing in the first place. + await stopWithDeferral(live.pid, { hostname: live.hostname ?? "127.0.0.1", port: live.port }); console.log(`✅ Proxy (PID ${live.pid}) stopped.`); } catch (err) { stopFailed = true; @@ -725,6 +873,17 @@ async function handleStop() { console.error(" Skipping shared teardown (native Codex restore, Grok config): the foreign proxy is still running."); } } + } else if (live) { + // Identity-confirmed live, but no PID this process can kill: a legacy /healthz that + // reports no pid, or a pid that failed verification. Treating that as "nothing is + // running" purges the state records and then restores shared client config out from + // under a proxy that is still serving — the exact failure the deferral exists to + // prevent, arrived at from the other direction. + stopFailed = true; + ownershipBlocked = true; + console.error(`❌ A proxy is answering on port ${live.port}, but no process id could be resolved for it, so it cannot be stopped from here.`); + console.error(" Skipping shared teardown: restoring client config while it serves would leave both pointing at each other."); + console.error(" Stop it from the home that started it, or end the process manually, then rerun 'ocx stop'."); } else if (!stoppedService) { console.log("No running proxy found."); } @@ -739,16 +898,159 @@ async function handleStop() { // Environment ownership is independent from service ownership. Always roll back // current-home variables; the helper refuses foreign markers on its own. try { revertSystemEnv(); } catch { /* best-effort */ } - if (!ownershipBlocked) { - if (!await restoreSharedClientStateAfterStop()) stopFailed = true; + // A stopped Windows scheduler is not a proven-down proxy. `killWindowsSchedulerWrappers` + // is explicitly best-effort and the `:loop` wrapper respawns its child after ~5s, so an + // immediate probe can see a dead interval and an update can start replacing files right + // before the proxy comes back. Poll across the restart window before this stop is allowed + // to report anything but failure (#3008) — and ONLY for that backend, since making every + // launchd and systemd stop wait seven seconds would be a regression in ordinary use. + if (schedulerCanRespawn && !ownershipBlocked) { + const survivor = await proxyStillLiveAfterStop({ canRespawn: true }); + if (survivor) { + stopFailed = true; + console.error(`❌ A proxy is still listening on port ${survivor.port} after the service stop; it is being respawned.`); + console.error(" Skipping shared teardown: restoring client config while the proxy runs leaves both pointing at each other."); + ownershipBlocked = true; + } + } + // Recovering somebody else's abandoned obligation is not the same act as finishing this + // run's own. This run stopped a proxy and verified the result; the inherited case has no + // such evidence, and `findLiveProxy` returning null covers a timeout and a malformed + // answer as well as a genuinely dead port. Restoring client config under a proxy that is + // merely unresponsive is exactly the failure the deferral exists to prevent. + // + // So an inherited obligation this run did not claim GATES the restore itself, rather + // than only labelling it: without a definitive "dead" from the tri-state probe, the + // restore does not run, the receipt stays for the next stop, and the stop fails. A + // warning that lets the restore happen anyway is not a gate. + // + // An UNREADABLE obligation is a third case. It names no endpoint, so nothing can ever + // prove its proxy down. It is NOT waved through: it fails this stop and is set aside + // only afterwards, so the operator gets an explicit manual step instead of a silent + // restore backed by no evidence. Setting it aside is still necessary — left in place it + // makes both updater gates run a stop that fails on it every time, which is an update + // that can never proceed. + // + // Inherited obligations are evaluated whether or not this run claimed its own. A stop + // that finds a live proxy used to skip them entirely, so older abandoned receipts + // accumulated forever while each run cleared only its own nonce. + const recoveredNonces: string[] = []; + const unreadable: { nonce: string }[] = []; + let inheritedBlocks = false; + if (inheritedTeardowns.length > 0 && !ownershipBlocked) { + for (const read of inheritedTeardowns) { + if (read.state === "unscannable") { + // No file, no nonce: nothing to quarantine and nothing to remove. The home itself + // may be hiding an obligation, so block and ask for the directory to be fixed. + inheritedBlocks = true; + stopFailed = true; + console.error(`❌ ${read.detail}, so this stop cannot tell whether a shared teardown is still owed.`); + console.error(" Skipping shared teardown. Fix access to the opencodex home, then rerun 'ocx stop'."); + continue; + } + if (read.state === "invalid") { + unreadable.push(read); + inheritedBlocks = true; + stopFailed = true; + console.error(`❌ A pending-teardown receipt could not be read (${read.detail}).`); + console.error(" It names no endpoint, so this stop cannot prove the proxy it belonged to is down."); + console.error(" Confirm no proxy is running, then rerun 'ocx stop' to complete the teardown."); + continue; + } + if (read.receipt.endpointSource === "guessed") { + // The recorded address is the configured one, not the one that stop contacted. A + // proxy on an explicit --port can be respawned there while this address refuses, + // so "dead" here proves nothing and must not authorize a restore. + inheritedBlocks = true; + stopFailed = true; + console.error("❌ A shared teardown from an earlier stop is outstanding, but that stop could not record the address it was stopping."); + console.error(` Only the configured address (${read.receipt.endpoint.hostname}:${read.receipt.endpoint.port}) was recorded, which cannot prove the right proxy is down.`); + console.error(` Confirm no proxy is running, then run 'ocx restore' and remove ${pendingTeardownPathFor(read.receipt.nonce)}.`); + continue; + } + if (await abandonedTeardownIsSafeToFinish(read.receipt.endpoint)) { + recoveredNonces.push(read.receipt.nonce); + continue; + } + inheritedBlocks = true; + stopFailed = true; + console.error(`❌ A shared teardown from an earlier stop is still outstanding, and the proxy on ${read.receipt.endpoint.hostname}:${read.receipt.endpoint.port} could not be confirmed down.`); + console.error(" Skipping shared teardown: restoring client config under a proxy that may still be running is what the deferral exists to prevent."); + console.error(" The obligation is preserved; retry once the proxy is confirmed stopped."); + } + } + const restoreBlocked = ownershipBlocked || inheritedBlocks; + if (!restoreBlocked) { + if (recoveredNonces.length > 0) { + // A previous deferred stop died before restoring, and the probe says its endpoint is + // not answering. That is the whole point of leaving the receipt behind. + console.log("↩️ Finishing a shared teardown left unfinished by an earlier stop."); + } + const restore = await restoreSharedClientStateAfterStop(); + if (restore.other) stopFailed = true; + else if (restore.historyOnly) historyOnlyFailure = true; + // The obligation is discharged whether or not history metadata finalized: config and + // catalog are what a client reads, and `restore.other` already fails the stop. + // + // Each nonce names its own file, so a clear can only ever remove the obligation it + // names — never one a concurrent stop wrote. Both this run's claim and every inherited + // receipt it proved discharged are released together. + if (!restore.other) { + const discharged = teardownNonce ? [teardownNonce, ...recoveredNonces] : recoveredNonces; + for (const nonce of discharged) { + // A receipt that survives its discharge re-triggers recovery forever, so a failed + // removal is surfaced rather than swallowed. + if (!clearPendingTeardown(nonce)) { + stopFailed = true; + console.error(`❌ The shared teardown finished, but its receipt could not be removed: ${pendingTeardownPathFor(nonce)}`); + console.error(" Remove it manually; otherwise every later stop and update will try to recover it again."); + } + } + } + } + // Set an unreadable receipt aside only AFTER the outcome is known. Renaming it earlier + // would take it out of the recovery loop while the restore it stood for had not run. + // + // Setting aside is NOT discharging. The renamed file still counts as an outstanding + // obligation (`isAnyTeardownObligationFileName`), so both updaters keep refusing to + // install until an operator removes it — the rename only stops every later stop from + // re-reading the same garbage. Skipped under `ownershipBlocked` because a foreign + // service still owns this state and none of it is ours to move. + if (unreadable.length > 0 && !ownershipBlocked) { + for (const read of unreadable) { + const moved = quarantinePendingTeardown(read.nonce); + if (moved) { + console.error(`⚠️ That unreadable receipt was set aside at ${moved}. It still blocks 'ocx update', and 'ocx stop' has NOT restored on its behalf.`); + console.error(" To clear it: confirm no proxy is running, run 'ocx restore', then delete that file."); + } else { + console.error(`❌ It could not be set aside either: ${pendingTeardownPathFor(read.nonce)}. Remove it manually after running 'ocx restore'.`); + } + } } - // Set the code rather than exiting inline: `restart` and the tray coordinator call this - // function and need it to RETURN so they can decide what to do next. + // Set the code rather than exiting inline: this function returns a value its dispatcher + // reads, so exiting here would take that decision away from the caller. + // + // A history-only failure gets its own code so `ocx update` can tell "the proxy is down + // and a manifest needs review" from "the proxy would not stop" (#3008). Ordinary failure + // still wins: it is the stronger signal. if (stopFailed) process.exitCode = 1; + else if (historyOnlyFailure) process.exitCode = STOP_HISTORY_INCOMPLETE_EXIT_CODE; return !stopFailed; } async function handleUninstall() { + /** Definitive "nothing is answering" on the endpoint this home would serve. */ + const proxyEndpointProvenDown = async (): Promise => { + try { + const { probeProxyLiveness } = await import("../update/proxy-liveness-probe.mjs"); + // Every candidate, not just the preferred one: a stale runtime record pointing at a + // closed port would otherwise "prove" a live proxy on the configured port is gone. + const endpoints = endpointsToProve(readRuntimePort(), loadConfig()); + return everyEndpointProvenDown(endpoints, e => probeProxyLiveness(e.port, e.hostname)); + } catch { + return false; + } + }; const failures: string[] = []; const runStep = async (label: string, step: () => void | boolean | Promise) => { @@ -762,18 +1064,89 @@ async function handleUninstall() { } }; - await runStep("service stopped", () => stopServiceIfInstalled()); + // Consume the DETAILED outcome. The boolean helper returns false for "not installed", + // "refused to stop" and "state could not be read" alike, so this step used to print + // "not installed" for a manager that might still be running and then tear down shared + // config underneath it (#3008). + // The authorization rule lives in `uninstall-plan` so it can be exercised for every + // failure permutation by calling it, rather than by reading this function's source. + const observed: UninstallObservation = { + serviceStop: null, + proxyProvenDown: false, + serviceRemoval: null, + respawnWindowVerified: false, + }; + await runStep("service stopped", () => { + const outcome = stopServiceIfInstalledDetailed(); + observed.serviceStop = outcome; + if (outcome === "absent") return false; + if (outcome === "failed") { + throw new Error("the installed service manager did not stop; it may respawn the proxy"); + } + if (outcome === "state-unknown") { + throw new Error("the Windows Task Scheduler state could not be read, so this uninstall cannot tell whether a manager is still running. Run 'ocx service status' to see the query error"); + } + return true; + }); await runStep("proxy stopped", async () => { const pid = readPid(); - if (!pid) return false; + if (!pid) { + // A missing pid file is not proof that nothing is serving: a proxy can outlive its + // record (crash, manual delete, corrupt file), which is exactly why `ocx stop` falls + // back to identity-checked discovery. Without this, uninstall restored shared config + // and reported success while that proxy kept running (#3008). + const live = await findLiveProxy(); + if (!live) { + // A miss is not proof: `findLiveProxy` collapses a timeout and a transport failure + // into the same null as a dead endpoint. Ask the tri-state probe, which only says + // "dead" for a refused connection or a definitive non-OpenCodex answer (#3008). + observed.proxyProvenDown = await proxyEndpointProvenDown(); + if (!observed.proxyProvenDown) { + throw new Error("no proxy could be found, but its endpoint could not be confirmed down either; confirm nothing is serving, then rerun"); + } + return false; + } + if (!live.pid) { + throw new Error(`a proxy is answering on port ${live.port} but no process id could be resolved for it; stop it from the home that started it, then rerun`); + } + await stopProxy(live.pid); + observed.proxyProvenDown = true; + return true; + } await stopProxy(pid); removePid(pid); removeRuntimePort(pid); + observed.proxyProvenDown = true; return true; }); - await runStep("service removed", () => uninstallServiceIfInstalled()); + await runStep("service removed", () => { + const outcome = uninstallServiceDetailed(); + observed.serviceRemoval = outcome; + // "absent" and "removed" are both fine; a failure is not, and it used to look like + // absence on darwin and linux. + if (outcome === "failed") throw new Error("the installed service could not be removed"); + return outcome === "removed"; + }); + + // Only Task Scheduler can respawn through a surviving wrapper, and removing the + // registration does not prove the running one died. Poll the same window `ocx stop` does + // before shared config is allowed down (#764, #3008). + if (observed.serviceStop === "stopped-respawnable") { + await runStep("respawn window verified", async () => { + const survivor = await proxyStillLiveAfterStop({ canRespawn: true }); + if (survivor) throw new Error(`a proxy is still listening on port ${survivor.port} after the service was removed; it is being respawned`); + // A null from that poll is not proof either: its identity probe returns null on a + // timeout, so a respawned-but-unresponsive proxy looks the same as none. Require the + // tri-state probe to say dead on every candidate before calling the window verified. + if (!await proxyEndpointProvenDown()) { + throw new Error("no survivor answered after the service was removed, but the endpoint could not be confirmed down either; confirm nothing is serving, then rerun"); + } + observed.respawnWindowVerified = true; + return true; + }); + } if (process.platform === "win32") { await runStep("Windows tray removed", async () => { @@ -784,16 +1157,26 @@ async function handleUninstall() { }); } - await runStep("native Codex restored", async () => { - const r = await restoreNativeCodexAsync(); - if (!r.success) throw new Error(r.message); - }); + // Shared client config comes down only once nothing that could still be serving is + // unaccounted for. Restoring it under a live, still-managed proxy leaves both pointing + // at each other — the same failure `ocx stop` refuses (#3008). + if (sharedTeardownAuthorized(observed)) { + await runStep("native Codex restored", async () => { + const r = await restoreNativeCodexAsync(); + if (!r.success) throw new Error(r.message); + }); - await runStep("Grok Build config restored", () => { - const r = stripGrokConfig(); - if (!r.ok) throw new Error(r.message); - return r.changed; - }); + await runStep("Grok Build config restored", () => { + const r = stripGrokConfig(); + if (!r.ok) throw new Error(r.message); + return r.changed; + }); + } else { + failures.push("native Codex restored", "Grok Build config restored"); + console.error("⚠️ Skipping shared teardown (native Codex restore, Grok config): a service or proxy could not be proven stopped."); + console.error(" Resolve the failures above and rerun 'ocx uninstall' — service removal and local state cleanup are also unfinished."); + console.error(" 'ocx restore' is an interim step if you need native routing back before then."); + } await runStep("system env vars reverted", () => { const r = revertSystemEnv(); diff --git a/src/cli/uninstall-plan.ts b/src/cli/uninstall-plan.ts new file mode 100644 index 0000000000..0e1df1cd2d --- /dev/null +++ b/src/cli/uninstall-plan.ts @@ -0,0 +1,86 @@ +/** + * Whether an uninstall may take shared client config down (#3008). + * + * Extracted from `handleUninstall` because the rule is a decision, and a decision that + * only exists inside a long imperative command can only be tested by reading its source — + * which is how this shipped wrong twice: first trusting a boolean that collapsed "not + * installed" with "still running", then trusting a missing pid file as proof no proxy was + * serving. + * + * Native Codex and the Grok fence are SHARED. Restoring them while something may still be + * serving leaves the client and the proxy pointing at each other, so every step that could + * leave a live proxy behind has to be accounted for first. + */ +export type UninstallObservation = { + /** Detailed service-stop outcome, or null when the step threw. */ + serviceStop: "absent" | "stopped" | "stopped-respawnable" | "failed" | "state-unknown" | null; + /** + * Did the proxy step PROVE nothing is serving? + * + * A `findLiveProxy` miss is not that proof: it collapses a timeout and a transport + * failure into the same null as a dead endpoint, so an unresponsive proxy read as absent. + */ + proxyProvenDown: boolean; + /** Service removal outcome, or null when the step threw. */ + serviceRemoval: "absent" | "removed" | "failed" | null; + /** + * For a Task Scheduler backend: was the restart window verified AFTER removal? + * + * Deleting the registration does not prove an already-running `:loop` wrapper died — + * killing it is best-effort (#764). `ocx stop` polls across the window; uninstall has + * to do the same before it may take shared config down. + */ + respawnWindowVerified: boolean; +}; + +export function sharedTeardownAuthorized(o: UninstallObservation): boolean { + if (o.serviceStop === null) return false; + // "absent" and a clean stop are the only service states that prove nothing is managing + // the proxy. + if (o.serviceStop === "failed" || o.serviceStop === "state-unknown") return false; + // Removing the registration is not the same as proving the running wrapper is gone. + if (o.serviceStop === "stopped-respawnable" && !o.respawnWindowVerified) return false; + if (o.serviceRemoval === null || o.serviceRemoval === "failed") return false; + return o.proxyProvenDown; +} + +/** An endpoint an uninstall must account for before shared config comes down. */ +export type ProbeEndpoint = { hostname: string; port: number }; + +/** + * Every DISTINCT endpoint this home could be serving on. + * + * A runtime record and the configured port can disagree — a stale record pointing at a + * closed port while the live proxy sits on the configured one. Probing only the runtime + * candidate then reports "dead" for a port nobody is using and authorizes the teardown + * (#3008). `findLiveProxy` already probes both; the proof has to cover both too. + */ +export function endpointsToProve( + runtime: { port?: number; hostname?: string } | null, + config: { port?: number; hostname?: string }, +): ProbeEndpoint[] { + const out: ProbeEndpoint[] = []; + const push = (port: number | undefined, hostname: string | undefined) => { + if (!port || port <= 0 || port > 65535) return; + const endpoint = { hostname: hostname ?? "127.0.0.1", port }; + if (out.some(e => e.port === endpoint.port && e.hostname === endpoint.hostname)) return; + out.push(endpoint); + }; + push(runtime?.port, runtime?.hostname); + push(typeof config.port === "number" && config.port > 0 ? config.port : 10100, config.hostname); + return out; +} + +/** + * Proof requires EVERY candidate to be definitively dead. + * + * "unknown" is not absence: a listener that accepts connections but withholds /healthz, or + * one that times out, is exactly the state where restoring shared config is most harmful. + */ +export function everyEndpointProvenDown( + endpoints: readonly ProbeEndpoint[], + probe: (e: ProbeEndpoint) => "live" | "dead" | "unknown", +): boolean { + if (endpoints.length === 0) return false; + return endpoints.every(e => probe(e) === "dead"); +} diff --git a/src/config/pending-teardown-names.d.mts b/src/config/pending-teardown-names.d.mts new file mode 100644 index 0000000000..a5e540d55c --- /dev/null +++ b/src/config/pending-teardown-names.d.mts @@ -0,0 +1,8 @@ +export declare const PENDING_TEARDOWN_PREFIX: string; +export declare const PENDING_TEARDOWN_SUFFIX: string; +export declare const PENDING_TEARDOWN_UNREADABLE_SUFFIX: string; +export declare function isPendingTeardownFileName(name: unknown): boolean; +export declare function isQuarantinedTeardownFileName(name: unknown): boolean; +export declare function isAnyTeardownObligationFileName(name: unknown): boolean; +export declare function pendingTeardownNonceFromFileName(name: string): string | null; +export declare function hasPendingTeardownIn(readdir: (dir: string) => string[], dir: string): boolean; diff --git a/src/config/pending-teardown-names.mjs b/src/config/pending-teardown-names.mjs new file mode 100644 index 0000000000..6d3f33f6f5 --- /dev/null +++ b/src/config/pending-teardown-names.mjs @@ -0,0 +1,69 @@ +/** + * Naming rules for pending-teardown receipts, shared by both update lanes (#3008). + * + * Plain ESM because `bin/ocx.mjs` runs under Node before Bun exists and cannot import the + * TypeScript module. It lives here rather than being spelled out twice because that is + * exactly how this broke: the launcher kept checking the retired singleton filename after + * the receipts moved to one file per claim, so the npm lane silently stopped seeing every + * outstanding obligation. + */ + +export const PENDING_TEARDOWN_PREFIX = "pending-teardown-"; +export const PENDING_TEARDOWN_SUFFIX = ".json"; +/** + * Suffix for an obligation that could not be read. + * + * It is still an obligation. Quarantine renames the file so the ordinary recovery loop + * stops re-reading garbage, but it must NOT stop counting: an update that proceeds + * because the evidence was filed away is exactly the outcome the receipt exists to + * prevent. Both lanes treat this as outstanding until an operator removes it. + */ +export const PENDING_TEARDOWN_UNREADABLE_SUFFIX = ".unreadable.json"; +const NONCE_RE = /^[0-9a-f]{32}$/; + +/** A receipt the recovery loop should read and try to discharge. */ +export function isPendingTeardownFileName(name) { + if (typeof name !== "string") return false; + if (isQuarantinedTeardownFileName(name)) return false; + if (!name.startsWith(PENDING_TEARDOWN_PREFIX) || !name.endsWith(PENDING_TEARDOWN_SUFFIX)) return false; + return NONCE_RE.test(name.slice(PENDING_TEARDOWN_PREFIX.length, name.length - PENDING_TEARDOWN_SUFFIX.length)); +} + +/** A receipt that could not be read and is waiting on a human. */ +export function isQuarantinedTeardownFileName(name) { + if (typeof name !== "string") return false; + if (!name.startsWith(PENDING_TEARDOWN_PREFIX) || !name.endsWith(PENDING_TEARDOWN_UNREADABLE_SUFFIX)) return false; + return NONCE_RE.test(name.slice( + PENDING_TEARDOWN_PREFIX.length, + name.length - PENDING_TEARDOWN_UNREADABLE_SUFFIX.length, + )); +} + +/** Any obligation at all — readable or quarantined. Both block an update. */ +export function isAnyTeardownObligationFileName(name) { + return isPendingTeardownFileName(name) || isQuarantinedTeardownFileName(name); +} + +export function pendingTeardownNonceFromFileName(name) { + if (!isPendingTeardownFileName(name)) return null; + return name.slice(PENDING_TEARDOWN_PREFIX.length, name.length - PENDING_TEARDOWN_SUFFIX.length); +} + +/** + * Does the given config directory hold any outstanding obligation? + * + * Quarantined receipts count. Filing one away to unblock an update would let the very + * next `ocx update` install over a teardown that never ran — the enforcement has to + * survive until a human removes the file. + */ +export function hasPendingTeardownIn(readdir, dir) { + try { + return readdir(dir).some(isAnyTeardownObligationFileName); + } catch (error) { + // "There is no home yet" is the only honest empty answer. Any other failure — + // permissions, I/O, a file where the directory should be — means an obligation may be + // sitting there unread, and reporting "none" would let an update install over a + // teardown that never ran. Absence of proof is not proof of absence. + return error?.code !== "ENOENT"; + } +} diff --git a/src/config/pending-teardown.ts b/src/config/pending-teardown.ts new file mode 100644 index 0000000000..31082b0fbb --- /dev/null +++ b/src/config/pending-teardown.ts @@ -0,0 +1,286 @@ +import { createHash, randomBytes } from "node:crypto"; +import { existsSync, readdirSync, readFileSync, renameSync, unlinkSync } from "node:fs"; +import { join } from "node:path"; +import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; +import { atomicWriteFile } from "./atomic-write"; +import { getConfigDir } from "./paths"; + +/** + * Ownership receipt for a deferred shared teardown (#3008). + * + * `ocx stop` asks the proxy NOT to restore native Codex and the Grok fence, because a + * stopped Task Scheduler can respawn the proxy and a survivor must keep its client + * config. That hands one obligation to the parent — and a bare query flag cannot express + * an obligation: if the parent dies between the child's exit and its own restore, the + * shared config keeps pointing at a proxy that is gone, with nothing on disk saying so. + * + * The receipt is that missing state. The parent writes it BEFORE asking for a deferred + * stop and removes it only after its own restore, so a later `ocx stop`/`ocx update` can + * see the abandoned obligation and finish it once that proxy is proven down. + * + * ## Why the nonce is the FILENAME + * + * One shared file cannot be cleared safely. Read-compare-unlink is three syscalls, and a + * concurrent stop replacing the file between the compare and the unlink means this run + * deletes an obligation it never owned — the check passed against bytes that are already + * gone. Giving each claim its own path removes the race rather than serializing it: + * `unlink` names one specific obligation, so it can only ever delete that one. Two + * concurrent stops hold two receipts, which is the truth of the situation. + */ +export type PendingTeardownReceipt = { + /** Process that accepted the obligation, so a live owner is distinguishable from a dead one. */ + ownerPid: number; + /** Identity of this claim; also its filename, which is what makes a clear a single-syscall delete. */ + nonce: string; + /** ISO timestamp, for diagnostics only; recovery is decided by liveness, not by age. */ + createdAt: string; + /** + * Endpoint the owner was stopping. + * + * Recovery has to prove THAT proxy is down, and after a crash the runtime-port record + * is usually gone. Falling back to the configured port asks the wrong question for a + * proxy started with an explicit `--port`: the configured port refuses while the live + * one keeps serving, and its client config gets torn out from under it. + */ + endpoint: { hostname: string; port: number }; + /** + * How the endpoint was obtained. + * + * `exact` came from the runtime record or a successful liveness probe — the address the + * stop actually contacted. `guessed` is the configured listen address, recorded because + * an obligation with a weak address beats no obligation at all, but it is NOT evidence: + * a proxy on an explicit `--port` can be respawned there while the configured port + * refuses, and treating that refusal as proof would restore under a live proxy. A + * guessed receipt therefore fails closed into manual recovery. + */ + endpointSource: "exact" | "guessed"; +}; + +/** + * What is on disk, kept distinct from what it means. + * + * Collapsing a malformed file into "no receipt" loses the one fact recovery needs: an + * obligation may still be outstanding, and its owner can no longer be identified. That + * state must not silently authorize a deferral, and it must not wedge every later stop + * either — see {@link quarantinePendingTeardown}. + */ +export type PendingTeardownRead = + | { state: "missing" } + | { state: "valid"; receipt: PendingTeardownReceipt } + | { state: "invalid"; nonce: string; detail: string }; + +/** + * The home itself could not be listed. + * + * Distinct from an invalid receipt: there is no file to quarantine and no nonce to name, + * so it must never be fed to the receipt machinery. It blocks like any obligation, but the + * remedy is to fix the directory and retry, not to remove something. + */ +export type TeardownScanFailure = { state: "unscannable"; detail: string }; + +import { + isPendingTeardownFileName, + isAnyTeardownObligationFileName, + PENDING_TEARDOWN_PREFIX as PREFIX, + PENDING_TEARDOWN_SUFFIX as SUFFIX, + PENDING_TEARDOWN_UNREADABLE_SUFFIX as UNREADABLE_SUFFIX, + pendingTeardownNonceFromFileName, +} from "./pending-teardown-names.mjs"; + +const NONCE_RE = /^[0-9a-f]{32}$/; + +export function pendingTeardownPathFor(nonce: string): string { + return join(getConfigDir(), `${PREFIX}${nonce}${SUFFIX}`); +} + +function isReceipt(value: unknown, nonce: string): value is PendingTeardownReceipt { + if (!value || typeof value !== "object") return false; + const receipt = value as Record; + const endpoint = receipt.endpoint as Record | undefined; + const endpointOk = !!endpoint + && typeof endpoint === "object" + && typeof endpoint.hostname === "string" + && endpoint.hostname.trim() !== "" + && Number.isInteger(endpoint.port) + && Number(endpoint.port) > 0 + && Number(endpoint.port) <= 65535; + return Number.isSafeInteger(receipt.ownerPid) + && Number(receipt.ownerPid) > 0 + // The body must agree with the name: a receipt whose nonce was edited to name a + // different claim would let a request authorize a deferral it does not own. + && receipt.nonce === nonce + && typeof receipt.createdAt === "string" + && (receipt.endpointSource === "exact" || receipt.endpointSource === "guessed") + && endpointOk; +} + +/** Claim a deferred teardown for this process. Returns the receipt that was written. */ +export function claimPendingTeardown( + endpoint: { hostname: string; port: number }, + endpointSource: "exact" | "guessed", + ownerPid: number = process.pid, +): PendingTeardownReceipt { + const dir = getConfigDir(); + assertNotRealHomeUnderTest(dir); + const nonce = randomBytes(16).toString("hex"); + const receipt: PendingTeardownReceipt = { ownerPid, nonce, createdAt: new Date().toISOString(), endpoint, endpointSource }; + atomicWriteFile(pendingTeardownPathFor(nonce), JSON.stringify(receipt, null, 2) + "\n"); + return receipt; +} + +export function readPendingTeardown(nonce: string): PendingTeardownRead { + if (!NONCE_RE.test(nonce)) return { state: "missing" }; + let raw: string; + try { + raw = readFileSync(pendingTeardownPathFor(nonce), "utf-8"); + } catch (error) { + // Only "there is no file" is absence. A permission error, or a directory sitting where + // the receipt belongs, means something IS there and cannot be read; calling that + // missing hides an obligation that may still be outstanding. + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT") return { state: "missing" }; + return { state: "invalid", nonce, detail: `unreadable (${code ?? "unknown"})` }; + } + try { + const parsed: unknown = JSON.parse(raw); + if (isReceipt(parsed, nonce)) return { state: "valid", receipt: parsed }; + const digest = createHash("sha256").update(raw).digest("hex").slice(0, 12); + return { state: "invalid", nonce, detail: `malformed receipt (sha256 ${digest})` }; + } catch { + return { state: "invalid", nonce, detail: "unparseable JSON" }; + } +} + +/** An obligation that exists on disk — the "missing" case cannot occur in a listing. */ +export type OutstandingTeardown = Exclude | TeardownScanFailure; + +/** + * Every obligation currently on disk, attributable or not. + * + * A scan that FAILS is not an empty scan. Swallowing a permission or I/O error into `[]` + * would let `handleStop` restore client config with an unread obligation sitting right + * there, so anything but a missing home surfaces as one unreadable obligation the caller + * must treat like any other: blocking, and needing a human. + */ +export function listPendingTeardowns(): OutstandingTeardown[] { + let names: string[]; + try { + names = readdirSync(getConfigDir()); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return []; + // Not an invalid RECEIPT: there is no file here and no nonce to name. Synthesizing one + // would hand a fabricated identity to the quarantine and clear paths, which could then + // rename or delete a real receipt that happened to carry it. + return [{ state: "unscannable", detail: `the opencodex home could not be listed (${(error as NodeJS.ErrnoException).code ?? "unknown"})` }]; + } + const out: OutstandingTeardown[] = []; + for (const name of names) { + // One naming rule, shared with the npm launcher: the two lanes drifting apart is + // exactly how the Node updater stopped seeing receipts at all. + if (!isPendingTeardownFileName(name)) continue; + const nonce = pendingTeardownNonceFromFileName(name)!; + const read = readPendingTeardown(nonce); + if (read.state !== "missing") out.push(read); + } + return out; +} + +/** + * Is any obligation outstanding, whether or not it can still be attributed? + * + * Quarantined receipts count. Filing an unreadable one away must not let the next update + * install over a teardown that never ran — that would turn "we could not tell" into "it + * is fine", which is the failure this whole mechanism exists to prevent. + */ +export function pendingTeardownOutstanding(): boolean { + try { + return readdirSync(getConfigDir()).some(isAnyTeardownObligationFileName); + } catch (error) { + // Only a missing home is empty. Any other scan failure may be hiding an obligation, + // and reporting "none" would unblock an update over a teardown that never ran. + return (error as NodeJS.ErrnoException).code !== "ENOENT"; + } +} + +/** Paths of quarantined obligations awaiting a human. */ +export function listQuarantinedTeardowns(): string[] { + try { + return readdirSync(getConfigDir()) + .filter(name => name.startsWith(PREFIX) && name.endsWith(UNREADABLE_SUFFIX)) + .map(name => join(getConfigDir(), name)); + } catch { + return []; + } +} + +/** + * Remove exactly one obligation. + * + * The nonce is the filename, so this is a compare-and-delete in one syscall: it can never + * remove a receipt another process wrote, because that receipt lives at a different path. + * Returns whether the obligation is gone — a failed unlink is reported rather than + * swallowed, since a receipt that survives its discharge re-triggers recovery forever. + */ +export function clearPendingTeardown(nonce: string): boolean { + try { + unlinkSync(pendingTeardownPathFor(nonce)); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "ENOENT"; + } +} + +/** + * Move an unattributable obligation aside. + * + * An invalid receipt names no endpoint, so nothing can prove its proxy is down, so it can + * never be discharged the normal way. Left in place it is not merely useless: both + * updater gates treat an outstanding receipt as a reason to run the stop, and that stop + * would fail on the same receipt every time — an update that can never proceed. + * + * Renaming stops the recovery loop from re-reading garbage on every stop, but it + * deliberately does NOT stop the obligation from counting: `pendingTeardownOutstanding` + * still sees it, so both updaters keep refusing to install over a teardown that never + * ran. Only a human removing the file ends the enforcement. + * + * Returns the path it was moved to, or null when it could not be moved. + */ +export function quarantinePendingTeardown(nonce: string): string | null { + const from = pendingTeardownPathFor(nonce); + if (!existsSync(from)) return null; + const to = join(getConfigDir(), `${PREFIX}${nonce}${UNREADABLE_SUFFIX}`); + try { + renameSync(from, to); + return to; + } catch { + return null; + } +} + +/** + * True when a previous deferred stop left its obligation unfinished. + * + * A receipt whose owner is still alive belongs to a stop that is still running: leave it + * alone. Only an abandoned obligation is a candidate, and a VALID one still has to prove + * its endpoint is down before anything is restored — an invalid one never can, which is + * what {@link quarantinePendingTeardown} exists for. + */ +export function isPendingTeardownAbandoned( + read: PendingTeardownRead | TeardownScanFailure, + isAlive: (pid: number) => boolean, + selfPid: number = process.pid, +): boolean { + if (read.state === "missing") return false; + // A home that cannot be listed may be hiding an obligation. It is not recoverable and + // not removable; the caller blocks on it and asks for the directory to be fixed. + if (read.state === "unscannable") return true; + if (read.state === "invalid") return true; + if (read.receipt.ownerPid === selfPid) return false; + return !isAlive(read.receipt.ownerPid); +} + +/** Does this request name an obligation that exists and is readable? */ +export function deferralMatchesReceipt(nonce: string | null): boolean { + if (!nonce || !NONCE_RE.test(nonce)) return false; + return readPendingTeardown(nonce).state === "valid"; +} diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index 6c0f7082f9..3e296d6c72 100644 --- a/src/lib/process-control.ts +++ b/src/lib/process-control.ts @@ -29,6 +29,24 @@ export interface GracefulStopIo { waitExit?: (pid: number, timeoutMs: number) => boolean; env?: Record; exitTimeoutMs?: number; + /** + * Nonce of the pending-teardown receipt this caller claimed. + * + * `ocx stop` sets it because it restores shared client config itself, only after + * proving a stopped Task Scheduler did not respawn the proxy (#3008). The nonce is what + * makes the deferral an owned obligation rather than a flag anyone can set: the proxy + * honours it only when it names the receipt actually on disk. Direct callers omit it + * and keep the self-contained behaviour. + */ + deferSharedTeardownNonce?: string; + /** + * Endpoint the caller already resolved for this pid. + * + * `ocx stop` records this same snapshot in its pending-teardown receipt. Re-reading the + * runtime file here could pick up a different one, which would make the receipt name an + * endpoint the stop never contacted — and recovery probes exactly that endpoint. + */ + runtimeEndpoint?: { hostname: string; port: number }; } /** @@ -67,7 +85,7 @@ export class ProxyOwnershipRefusedError extends Error {} */ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): Promise { const readRuntime = io.readRuntime ?? readRuntimePort; - const runtime = readRuntime(pid); + const runtime = io.runtimeEndpoint ?? readRuntime(pid); if (!runtime?.port) return false; const env = io.env ?? process.env; const headers: Record = {}; @@ -75,7 +93,14 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): if (token) headers["x-opencodex-api-key"] = token; const fetchFn = io.fetchFn ?? fetch; try { - const res = await fetchFn(`http://${gracefulStopHost(runtime.hostname)}:${runtime.port}/api/stop`, { + // `ocx stop` asks the proxy NOT to restore shared client config: it does that itself, + // after verifying a stopped Task Scheduler did not respawn the proxy (#3008). Letting + // the child do it means a survivor found seconds later has already lost its config. + const stopUrl = `http://${gracefulStopHost(runtime.hostname)}:${runtime.port}/api/stop` + + (io.deferSharedTeardownNonce + ? `?deferSharedTeardown=1&teardownNonce=${encodeURIComponent(io.deferSharedTeardownNonce)}` + : ""); + const res = await fetchFn(stopUrl, { method: "POST", headers, // Hung proxies with many CLOSE_WAIT clients can be slow to accept; give them @@ -107,10 +132,10 @@ function drainDeadlineMs(): number { } /** Graceful-first stop: management-API drain, then the platform kill ladder. */ -export async function stopProxy(pid: number): Promise { +export async function stopProxy(pid: number, io: GracefulStopIo = {}): Promise { if (!isProcessAlive(pid)) return; - const runtime = readRuntimePort(pid); - const graceful = await stopProxyGracefully(pid); + const runtime = io.runtimeEndpoint ?? readRuntimePort(pid); + const graceful = await stopProxyGracefully(pid, io); if (graceful === "refused") { // The proxy refused on purpose (foreign service owns it). Forcing would strip shared // config while that service keeps the proxy alive. diff --git a/src/server/management-api.ts b/src/server/management-api.ts index 9f19831576..9e188c03ee 100644 --- a/src/server/management-api.ts +++ b/src/server/management-api.ts @@ -256,10 +256,46 @@ export async function handleManagementAPI( if (routed) return routed; if (url.pathname === "/api/stop" && req.method === "POST") { - const { restoreNativeCodexAsync } = await import("../codex/inject"); - const { stopServiceIfInstalled, isServiceOwnershipError } = await import("../service"); + const { installedServiceRespawnRisk, stopServiceIfInstalledDetailed, isServiceOwnershipError } = await import("../service"); + // `ocx stop` performs its own shared teardown AFTER verifying the scheduler did not + // respawn the proxy (#3008). Without this the child restores native Codex and strips + // the Grok fence here, so a survivor found moments later has already had the shared + // config pulled out from under it — and the parent's `ownershipBlocked` guard can + // only prevent a second, redundant teardown. A direct caller sends nothing and keeps + // the self-contained behaviour. + // + // The query flag alone is not enough to hand over the obligation: any authenticated + // caller could set it and simply exit, leaving client config pointed at a proxy that + // no longer exists. Honour the deferral only when the caller left a pending-teardown + // receipt on disk, which a later stop/update can find and finish. + // Decide BEFORE touching the manager. Stopping the Task Scheduler task and then + // refusing left the proxy running with its manager stopped — worse than either + // outcome. This process cannot verify its own post-exit respawn window; only the + // receipt-backed parent `ocx stop` can, which is what the deferral exists for. + const { deferralMatchesReceipt } = await import("../config/pending-teardown"); + const { deferralHonored, performStopTeardown } = await import("./stop-teardown"); + const holdsReceipt = deferralHonored(url, deferralMatchesReceipt); + const respawnRisk = holdsReceipt ? "none" : installedServiceRespawnRisk(); + if (respawnRisk === "respawnable") { + return jsonResponse({ + success: false, + code: "respawnable_service", + message: "This proxy is managed by a Task Scheduler wrapper that can respawn it, so the stop must be run by `ocx stop`, which verifies the respawn window. Nothing was changed.", + }, 409, req, config); + } + if (respawnRisk === "unknown") { + // Do NOT send them to `ocx stop`: it maps the same unanswerable probe to a stop + // failure, so that advice would be a loop. The scheduler query itself is what needs + // fixing (#3008). + return jsonResponse({ + success: false, + code: "service_state_unknown", + message: "The Windows Task Scheduler state could not be read, so this proxy cannot tell whether a wrapper would respawn it. Nothing was changed. Run `ocx service status` to see the query error, repair Task Scheduler access, then retry.", + }, 409, req, config); + } + let serviceStop: import("../service").ServiceStopOutcome; try { - stopServiceIfInstalled(); + serviceStop = stopServiceIfInstalledDetailed(); } catch (err) { if (isServiceOwnershipError(err)) { // The installed service belongs to another CODEX_HOME/OPENCODEX_HOME: it would respawn @@ -269,12 +305,30 @@ export async function handleManagementAPI( } throw err; } - const restore = await restoreNativeCodexAsync(); + // The boolean helper collapses "failed" into the same false as "no service installed", + // so this route used to tear down shared config and exit while a manager that refused + // to stop was still there to respawn the proxy (#3008). + if (serviceStop === "failed") { + return jsonResponse({ + success: false, + message: "The installed service manager did not stop; it may respawn the proxy. Shared client config was left alone. Run `ocx stop` from the home that owns the service.", + }, 409, req, config); + } + if (serviceStop === "state-unknown") { + // Same case, same remedy as the pre-check: the query is what needs fixing. + return jsonResponse({ + success: false, + code: "service_state_unknown", + message: "The Windows Task Scheduler state could not be read, so this proxy cannot tell whether a wrapper would respawn it. Shared client config was left alone. Run `ocx service status` to see the query error, repair Task Scheduler access, then retry.", + }, 409, req, config); + } + // The pre-check above already refused the respawnable case without a receipt, so + // reaching here with one means the parent owns the verification. // Both managed configs come down together on an explicit teardown. The daemon's own // syncCleanup skips this when OCX_SERVICE is set (so a crash/respawn keeps the fence), - // which is exactly why an intentional stop has to do it here. - const { stripGrokConfig } = await import("../grok/inject"); - const grok = stripGrokConfig(); + // which is exactly why an intentional stop has to do it here — unless the caller is + // `ocx stop`, which does it itself once the proxy is proven down. + const teardown = await performStopTeardown(url, { ownsReceipt: deferralMatchesReceipt }); setTimeout(async () => { let shutdownSucceeded = false; try { @@ -282,12 +336,12 @@ export async function handleManagementAPI( } catch { console.warn("[opencodex] shutdown drain failed"); } - process.exit(shutdownSucceeded ? 0 : 1); + // A drained proxy whose shared teardown failed did not finish the job. Exiting 0 + // told a supervisor the stop was clean while native Codex or the Grok fence was + // still pointed at this process (#3008). + process.exit(shutdownSucceeded && teardown.success ? 0 : 1); }, 200); - const grokNote = grok.ok ? "" : ` Grok config cleanup failed: ${grok.message}`; - return jsonResponse(restore.success - ? { success: true, message: `Proxy stopping, native Codex restored.${grokNote}` } - : { success: false, message: `Proxy stopping, but native Codex restore failed: ${restore.message}. Run \`ocx restore\`.${grokNote}` }); + return jsonResponse(teardown); } if (url.pathname.startsWith("/api/native-main-profiles")) { diff --git a/src/server/stop-teardown.ts b/src/server/stop-teardown.ts new file mode 100644 index 0000000000..e4aadc4996 --- /dev/null +++ b/src/server/stop-teardown.ts @@ -0,0 +1,84 @@ +import type { CodexNativeRestoreResult } from "../codex/inject"; +import { deferralMatchesReceipt } from "../config/pending-teardown"; + +/** + * Shared-teardown decision and execution for `POST /api/stop` (#3008). + * + * Lives outside the route handler because the handler schedules `process.exit` 200ms + * after it answers, which makes it uncallable from a test. The part worth testing is + * exactly this: whether the deferral is honoured, whether the restores actually run, and + * whether the response says what happened. + */ + +export type GrokStripResult = { ok: boolean; changed: boolean; message: string }; + +export type StopTeardownIo = { + /** Does the nonce this request carries name a readable obligation on disk? */ + ownsReceipt?: (nonce: string | null) => boolean; + restoreNativeCodex?: () => Promise; + stripGrok?: () => GrokStripResult; +}; + +export type StopTeardownBody = { + success: boolean; + message: string; + sharedTeardown: "deferred" | "performed"; +}; + +/** + * A deferral is honoured only when the caller proves it owns the obligation. + * + * The query flag names an intention; the receipt is the obligation. Without the second + * half any authenticated caller could ask the proxy to skip teardown and then exit, + * leaving native Codex and the Grok fence pointed at a proxy that no longer exists. + * + * "A receipt exists" is not that proof either: it would let any caller ride on another + * stop's outstanding obligation and get a deferral it never owns. The request has to name + * the receipt's nonce, which only the process that wrote it (and anything that can read + * the 0700 config directory, which is already the trust boundary for the admin token) + * can know. + */ +export function deferralHonored(url: URL, ownsReceipt: (nonce: string | null) => boolean): boolean { + if (url.searchParams.get("deferSharedTeardown") !== "1") return false; + return ownsReceipt(url.searchParams.get("teardownNonce")); +} + +/** Run (or skip) the shared teardown and describe the outcome truthfully. */ +export async function performStopTeardown(url: URL, io: StopTeardownIo = {}): Promise { + const ownsReceipt = io.ownsReceipt ?? deferralMatchesReceipt; + if (deferralHonored(url, ownsReceipt)) { + // Not "native Codex restored": nothing was restored here, and claiming otherwise + // would be a success message the operator cannot verify. + return { + success: true, + message: "Proxy stopping; shared teardown deferred to the stopping client.", + sharedTeardown: "deferred", + }; + } + const restore = io.restoreNativeCodex + ? await io.restoreNativeCodex() + : await (await import("../codex/inject")).restoreNativeCodexAsync(); + const grok = io.stripGrok + ? io.stripGrok() + : (await import("../grok/inject")).stripGrokConfig(); + // Success means BOTH halves came down. Deciding it from the native restore alone and + // appending the Grok text let a caller read `success: true` while the fence still + // pointed at a proxy that was exiting — the teardown reported done with half of it + // undone (#3008). + const grokNote = grok.ok ? "" : ` Grok config cleanup failed: ${grok.message}`; + if (restore.success && grok.ok) { + return { success: true, message: "Proxy stopping, native Codex restored.", sharedTeardown: "performed" }; + } + if (restore.success) { + return { + success: false, + message: `Proxy stopping, native Codex restored, but the Grok fence was not removed:${grokNote} Run \`ocx restore\`.`, + sharedTeardown: "performed", + }; + } + return { + success: false, + message: `Proxy stopping, but native Codex restore failed: ${restore.message}. Run \`ocx restore\`.${grokNote}`, + sharedTeardown: "performed", + }; +} diff --git a/src/service.ts b/src/service.ts index ab780711e8..67138d79fc 100644 --- a/src/service.ts +++ b/src/service.ts @@ -2998,6 +2998,22 @@ export function stopWindows(): void { if (isWindowsSchedulerEndBenign(error)) return; } } + +/** + * `stopWindows` for callers that need to know whether it worked. + * + * The void form swallows a non-benign `/end` failure, which is right for best-effort + * teardown and wrong for deciding whether an update may replace files: a scheduler that + * refused to stop can respawn the proxy on top of a half-written install (#3008). + */ +export function stopWindowsChecked(): boolean { + try { + schtasks(["/end", "/tn", TASK]); + return true; + } catch (error) { + return isWindowsSchedulerEndBenign(error); + } +} function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } } function statusWindowsXml(): string { try { return schtasks(["/query", "/tn", TASK, "/xml"]); } catch { return ""; } } @@ -3604,36 +3620,129 @@ export async function installFreshWindowsSchedulerSafely( } } +// `stopServiceIfInstalled` (boolean) is deliberately gone. It collapsed "not installed", +// "refused to stop" and "state could not be read" into the same `false`, and every caller +// that trusted it eventually read a live manager as absence — the route, then uninstall +// (#3008). Callers take `stopServiceIfInstalledDetailed` and handle the outcomes. /** - * If a service is installed, stop it so the process manager doesn't respawn after `ocx stop`. - * Returns true if a service was found and stopped. + * Would stopping the installed manager leave something that can respawn the proxy? + * + * Answered WITHOUT stopping anything, because a caller that must refuse the stop has to + * refuse before it acts: `POST /api/stop` briefly ended the Task Scheduler task and then + * returned 409, which left the proxy running with its manager stopped — worse than either + * outcome it was choosing between. + * + * Task Scheduler only. `schtasks /end` ends the task instance while the `cmd :loop` + * wrapper survives and respawns its child (#764); launchd, systemd and WinSW are down when + * they report stopped. */ -export function stopServiceIfInstalled(): boolean { +export function installedServiceRespawnRisk( + probe: () => WindowsSchedulerTaskProbe = probeWindowsSchedulerTask, + platform: NodeJS.Platform = process.platform, +): "none" | "respawnable" | "unknown" { + // launchd, systemd and WinSW are down when they report stopped; only the Task Scheduler + // wrapper survives its task ending (#764). + if (platform !== "win32") return "none"; + try { + // `probeWindowsSchedulerTask` returns "unknown" as an ordinary value when its queries + // fail — it does not throw — so testing for "present" let an unanswerable probe + // through, and the route then killed scheduler wrappers before refusing. + // + // "unknown" is kept SEPARATE from "respawnable" because the remedies differ. Telling + // an operator whose schtasks query is broken to run `ocx stop` is circular: that + // command maps the same unknown to a stop failure, so it cannot finish either. + const status = probe().status; + if (status === "absent") return "none"; + return status === "present" ? "respawnable" : "unknown"; + } catch { + // A probe that cannot answer is not evidence of absence either. + return "unknown"; + } +} + +/** + * Outcome of stopping an installed process manager. + * + * `stopServiceIfInstalled` collapses "no service was installed" and "a service was + * installed and would not stop" into the same `false`, which is fine for a caller that + * only wants to log. It is not fine for one deciding whether an update may replace package + * files: a manager that refused to stop can respawn the proxy on top of a half-written + * install (#3008). + */ +/** + * `stopped-respawnable` is Task Scheduler specifically: `schtasks /end` ends the task + * instance while the `cmd :loop` wrapper survives and respawns its child seconds later + * (#764). Only that backend needs the restart-window wait — launchd, systemd and WinSW + * are down when they report stopped, and making them pay a seven-second poll would be a + * regression in every ordinary `ocx stop`. + */ +/** + * `state-unknown` is kept apart from `failed` because the remedies differ. A manager that + * refused to stop is a stop failure the operator can retry; a scheduler whose state cannot + * be READ is a broken query, and telling that operator "the manager did not stop" sends + * them looking for the wrong thing (#3008). + */ +export type ServiceStopOutcome = "absent" | "stopped" | "stopped-respawnable" | "failed" | "state-unknown"; + +/** + * Collapse the Windows backend observations into one outcome. + * + * Extracted so the precedence is testable by calling it. The rule that matters: a readable + * failure outranks an unreadable state, and an unreadable state outranks success — a + * scheduler we cannot see may still respawn the proxy. + */ +export function classifyWindowsServiceStop(o: { + stopped: boolean; + failed: boolean; + schedulerStopped: boolean; + stateUnknown: boolean; +}): ServiceStopOutcome { + if (o.failed) return "failed"; + if (o.stateUnknown) return "state-unknown"; + if (o.stopped) return o.schedulerStopped ? "stopped-respawnable" : "stopped"; + return "absent"; +} + +export function stopServiceIfInstalledDetailed(): ServiceStopOutcome { assertServiceEnvironmentMatchesInstall(); if (process.platform === "darwin") { if (existsSync(plistPath())) { - try { stopLaunchd(); return true; } catch { return false; } + try { stopLaunchd(); return "stopped"; } catch { return "failed"; } } } else if (process.platform === "win32") { // Query BOTH backends regardless of state: a failed switch or stale state can leave // two managers installed, and either one would respawn the proxy after `ocx stop`. let stopped = false; - try { - const q = schtasks(["/query", "/tn", TASK]); - if (q.includes(TASK)) { stopWindows(); stopped = true; } - } catch { /* task not found */ } + let failed = false; + let schedulerStopped = false; + let stateUnknown = false; + // `probeWindowsSchedulerTask` is tri-state on purpose: a query that THROWS is not the + // same as a task that is absent, and treating it as absent lets a live scheduler + // survive a "successful" stop. + const probe = probeWindowsSchedulerTask(); + if (probe.status === "present") { + if (stopWindowsChecked()) { stopped = true; schedulerStopped = true; } + else failed = true; + } else if (probe.status === "unknown") { + // Not "failed": nothing refused to stop. The query itself could not answer, which is + // a different problem with a different fix. + stateUnknown = true; + } if (statusWinswRaw() !== "nonexistent") { - try { stopWinswService(); stopped = true; } catch { /* best-effort */ } + try { stopWinswService(); stopped = true; } catch { failed = true; } } // `schtasks /end` ends the task instance but the cmd `:loop` wrapper survives and // respawns its child seconds later (issue #764), resurrecting the proxy during a // stop or a tray restart. Kill the launcher/wrapper processes outright. killWindowsServiceWrapperProcesses(); - if (stopped) return true; + // A failure on either backend wins: the other one stopping does not make the live one + // safe to update over. + const outcome = classifyWindowsServiceStop({ stopped, failed, schedulerStopped, stateUnknown }); + if (outcome !== "absent") return outcome; } else if (process.platform === "linux" && isSystemd() && existsSync(unitPath())) { - try { stopSystemd(); return true; } catch { return false; } + try { stopSystemd(); return "stopped"; } catch { return "failed"; } } - return false; + return "absent"; } /** Delete install-state files; stale state would make `ocx update` "reinstall" a service that no longer exists. */ @@ -3666,13 +3775,22 @@ export function setUninstallServiceHooksForTests(hooks: UninstallServiceHooksFor * service or scheduler task that cannot be removed throws so the caller cannot erase state and * report success. */ -export function uninstallServiceIfInstalled(): boolean { +/** + * Outcome of removing an installed manager. + * + * `false` used to mean both "nothing was installed" and "removal failed" on darwin and + * linux, so a failed removal was reported as absence and authorized the shared teardown + * while the service assets were still there (#3008). + */ +export type ServiceUninstallOutcome = "absent" | "removed" | "failed"; + +export function uninstallServiceDetailed(): ServiceUninstallOutcome { const hooks = uninstallServiceHooksForTests; (hooks?.assertEnvironment ?? assertServiceEnvironmentMatchesInstall)(); const platform = hooks?.platform ?? process.platform; if (platform === "darwin") { if (existsSync(plistPath())) { - try { uninstallLaunchd(); removeServiceInstallState(); return true; } catch { return false; } + try { uninstallLaunchd(); removeServiceInstallState(); return "removed"; } catch { return "failed"; } } } else if (platform === "win32") { let removed = false; @@ -3688,13 +3806,20 @@ export function uninstallServiceIfInstalled(): boolean { (hooks?.uninstallNative ?? uninstallWinswService)(); removed = true; } - if (removed) { (hooks?.removeInstallState ?? removeServiceInstallState)(); return true; } + if (removed) { (hooks?.removeInstallState ?? removeServiceInstallState)(); return "removed"; } } else if (platform === "linux" && existsSync(unitPath())) { - try { uninstallSystemd(); removeServiceInstallState(); return true; } catch { - try { unlinkSync(unitPath()); removeServiceInstallState(); return true; } catch { return false; } + try { uninstallSystemd(); removeServiceInstallState(); return "removed"; } catch { + try { unlinkSync(unitPath()); removeServiceInstallState(); return "removed"; } catch { return "failed"; } } } - return false; + return "absent"; +} + +/** Boolean form for callers that only distinguish "something was removed". */ +export function uninstallServiceIfInstalled(): boolean { + const outcome = uninstallServiceDetailed(); + if (outcome === "failed") throw new Error("the installed service could not be removed"); + return outcome === "removed"; } /** True if a background service (launchd/systemd/Task Scheduler) is installed. */ @@ -4204,11 +4329,17 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { // modules after startup, so an in-place update leaves it executing mixed old/new code. // Gate on the service and the runtime-port record too, not just the pid file — a // service-managed or orphaned proxy can be live while ocx.pid is stale/missing. + // + // An outstanding pending-teardown receipt is a fourth reason to run the stop. After a + // parent crashed mid-deferral all three of the other signals can be absent while the + // shared client config still points at a proxy that is gone; installing over that + // silently skips the recovery the receipt was written to trigger (#3008). // Full `ocx stop` semantics (drain, service stop, restore). - if (serviceWasInstalled || readPid() || readRuntimePort()) { + if (serviceWasInstalled || readPid() || readRuntimePort() || pendingTeardownOutstanding()) { console.log("⏹ Stopping the running proxy before updating..."); const stopStdio = updateChildStdio(); const stop = spawnSync(process.execPath, selfLaunchArgv(["stop"]), { @@ -256,17 +266,39 @@ export async function runUpdate(): Promise { windowsHide: true, }); if (stopStdio === "pipe") logSpawnOutput("", stop); - if (stop.status !== 0 || readPid() || readRuntimePort()) { + // One decision, shared with the npm launcher (#3008). The two lanes disagreeing about + // the same situation is how this shipped fixed on one side only. Absent PID and runtime + // files are weak evidence - a crashed-but-listening proxy leaves none - so the captured + // endpoint is asked, and `null` from proxyIdentityAt covers refusal AND timeout alike. + const identity = await proxyIdentityAt(capturedListen.port, { hostname: capturedListen.hostname }); + const decision = decidePostStopUpdate({ + status: stop.status, + hasRuntimeState: !!(readPid() || readRuntimePort()), + // Re-checked AFTER the stop: a quarantined receipt lets the stop itself succeed + // (there is nothing left to stop), so a pre-stop check alone let the retry install + // over a teardown that never ran. + teardownOutstanding: pendingTeardownOutstanding(), + liveness: identity ? "live" : probeProxyLiveness(capturedListen.port, capturedListen.hostname), + }); + const historyOnlyStop = decision.reason === "history-only"; + if (!decision.proceed) { if (trayWasRunning) { try { const { startWindowsTray } = await import("../tray/windows"); startWindowsTray(); } catch { /* preserve the proxy stop failure */ } } - console.error("⚠️ Could not stop the running proxy; aborting the update. Run 'ocx stop' and retry."); + if (decision.reason === "teardown-outstanding") { + console.error("⚠️ A shared teardown from an earlier stop is still outstanding and needs manual review; aborting the update."); + console.error(" Confirm no proxy is running, run 'ocx restore', then remove the pending-teardown file in your opencodex home."); + } else { + console.error(decision.reason === "proxy-unknown" + ? `⚠️ Could not confirm the proxy on ${capturedListen.hostname}:${capturedListen.port} is stopped; aborting the update. Run 'ocx stop' and retry.` + : "⚠️ Could not stop the running proxy; aborting the update. Run 'ocx stop' and retry."); + } process.exit(1); } - if (historyRestoreIncomplete()) { + if (historyOnlyStop || historyRestoreIncomplete()) { console.warn( "⚠️ Codex resume-history metadata restore is incomplete (a backup manifest remains).\n" + " The DB may be busy or the manifest/target may need review; untracked routed history is intentionally unchanged.\n" + diff --git a/src/update/proxy-liveness-probe.d.mts b/src/update/proxy-liveness-probe.d.mts new file mode 100644 index 0000000000..a72c5fe028 --- /dev/null +++ b/src/update/proxy-liveness-probe.d.mts @@ -0,0 +1,6 @@ +/** Declaration for the plain-ESM liveness probe shared with `bin/ocx.mjs`. */ +export declare function probeProxyLiveness( + port: number, + hostname?: string, + timeoutMs?: number, +): "live" | "dead" | "unknown"; diff --git a/src/update/proxy-liveness-probe.mjs b/src/update/proxy-liveness-probe.mjs new file mode 100644 index 0000000000..6179156a32 --- /dev/null +++ b/src/update/proxy-liveness-probe.mjs @@ -0,0 +1,84 @@ +import { spawnSync } from "node:child_process"; + +/** + * Is something still answering `/healthz` as an opencodex proxy on this endpoint? + * + * Absent PID and runtime-port files are weak evidence that the proxy is gone: a crashed + * but still-listening process, or one supervised outside our records, leaves no files and + * keeps the port. Replacing package files under it leaves a server running a mix of old + * and new modules, which is the hazard `ocx update` stops the proxy to avoid (#3008). + * + * Synchronous and dependency-free because it runs inside the plain-Node launcher's + * `runNpmSelfUpdate`, which is not async and cannot import the TypeScript liveness module. + * A separate Node child does the fetch so the caller keeps its straight-line control flow. + * + * Returns `"live" | "dead" | "unknown"`, and the caller treats `unknown` as a reason to + * stop. Fail-open was wrong here: a listener that accepts connections but withholds + * `/healthz`, or a probe that times out, is exactly the state where replacing package + * files is most dangerous, and "we could not tell" is not evidence the proxy is gone. + * Only a refused connection or a definitive non-OpenCodex answer earns `"dead"`. + */ +export function probeProxyLiveness(port, hostname = "127.0.0.1", timeoutMs = 1500) { + // An unusable port is not an ambiguous probe: there is nothing to ask. + if (!Number.isFinite(port) || port <= 0 || port > 65535) return "dead"; + // Normalize HERE rather than at each call site. Leaving it to the callers put the fix in + // one lane and not the other, and a bracketed IPv6 literal handed to node:http answers + // nothing - which the tri-state correctly reports as "unknown" and the updater correctly + // treats as a reason to abort, turning a healthy stop into a refused update. + let host = typeof hostname === "string" && hostname.trim() !== "" ? hostname.trim() : "127.0.0.1"; + // A wildcard bind answers on loopback; `node:http` cannot dial the wildcard itself. + if (host === "0.0.0.0" || host === "*") host = "127.0.0.1"; + if (host === "::" ) host = "::1"; + // `[::1]` is a URL spelling; the socket layer wants the bare address. + if (host.startsWith("[") && host.endsWith("]")) host = host.slice(1, -1); + // `node:http` rather than `fetch`: the child inherits a parent whose event loop is + // blocked on `spawnSync`, and an aborted-before-dispatch fetch reports the same "not + // live" as a genuinely dead port. A request emitted on the socket cannot be confused + // with one that never left. + const script = [ + "const http = require('node:http');", + "const [host, port, timeout] = process.argv.slice(1);", + "const req = http.get({ host, port: Number(port), path: '/healthz', timeout: Number(timeout) }, res => {", + " let body = '';", + " res.setEncoding('utf8');", + " res.on('data', chunk => { body += chunk; });", + " res.on('end', () => {", + " try {", + " const parsed = JSON.parse(body);", + " // Mirrors isOpencodexHealthz in src/server/proxy-liveness.ts. A foreign server", + " // that happens to expose /healthz must not be read as our proxy, and a", + " // pre-identity build of ours must not be read as foreign.", + " const isOpencodex = parsed && typeof parsed === 'object'", + " && (parsed.service === 'opencodex'", + " || (parsed.service === undefined", + " && parsed.status === 'ok'", + " && typeof parsed.version === 'string'", + " && typeof parsed.uptime === 'number'));", + " // Only a clean 200 decides anything. Any other status means the endpoint is", + " // answering but not telling us what it is, which is not evidence of absence.", + " if (res.statusCode !== 200) process.stdout.write('UNKNOWN');", + " else process.stdout.write(isOpencodex ? 'LIVE' : 'DEAD');", + " } catch { process.stdout.write('UNKNOWN'); }", + " });", + "});", + "req.on('timeout', () => { process.stdout.write('UNKNOWN'); req.destroy(); });", + "// ECONNREFUSED is the one error that proves nothing is listening. Everything else -", + "// reset, unreachable host, TLS confusion - leaves the question open.", + "req.on('error', err => process.stdout.write(err && err.code === 'ECONNREFUSED' ? 'DEAD' : 'UNKNOWN'));", + ].join("\n"); + try { + const probe = spawnSync( + process.execPath, + ["-e", script, host, String(port), String(timeoutMs)], + { encoding: "utf8", timeout: timeoutMs + 1500, windowsHide: true }, + ); + const out = probe.stdout ?? ""; + if (out.includes("LIVE")) return "live"; + if (out.includes("DEAD")) return "dead"; + // A child that produced nothing, was killed by its own timeout, or failed to spawn + // leaves the question open rather than answering it. + return "unknown"; + } catch { + return "unknown"; + } +} diff --git a/src/update/stop-contract.d.mts b/src/update/stop-contract.d.mts new file mode 100644 index 0000000000..b077eb21b3 --- /dev/null +++ b/src/update/stop-contract.d.mts @@ -0,0 +1,2 @@ +/** Declaration for the plain-ESM stop contract shared with `bin/ocx.mjs`. */ +export declare const STOP_HISTORY_INCOMPLETE_EXIT_CODE: 79; diff --git a/src/update/stop-contract.mjs b/src/update/stop-contract.mjs new file mode 100644 index 0000000000..c72548b777 --- /dev/null +++ b/src/update/stop-contract.mjs @@ -0,0 +1,15 @@ +/** + * The exit code `ocx stop` uses to say "teardown succeeded, history cleanup did not". + * + * This is plain ESM rather than TypeScript because it has two consumers on opposite sides + * of a process boundary: `src/update/index.ts` and the Node launcher `bin/ocx.mjs`, which + * cannot import a `.ts` module. A TypeScript union would not survive `spawnSync` anyway — + * the value has to be on the wire, and an exit code is the wire. + * + * 79 is deliberate. It sits above the `sysexits.h` block (64-78), below `128 + signal`, + * and outside every code this CLI already uses: `src/cli/index.ts` emits 0, 1 and 130, + * and `src/cli/dispatch.ts` adds 2, 4 and 64. Picking one of those would have made a + * history-only stop indistinguishable from a config conflict, and `bin/ocx.mjs` mirrors + * the child's code faithfully enough to propagate the confusion. + */ +export const STOP_HISTORY_INCOMPLETE_EXIT_CODE = 79; diff --git a/src/update/stop-decision.d.mts b/src/update/stop-decision.d.mts new file mode 100644 index 0000000000..f773e786e6 --- /dev/null +++ b/src/update/stop-decision.d.mts @@ -0,0 +1,10 @@ +/** Declaration for the plain-ESM post-stop decision shared with `bin/ocx.mjs`. */ +export declare function decidePostStopUpdate(input: { + status: number | null; + hasRuntimeState: boolean; + liveness: "live" | "dead" | "unknown"; + teardownOutstanding?: boolean; +}): { + proceed: boolean; + reason: "stop-failed" | "runtime-state" | "teardown-outstanding" | "proxy-live" | "proxy-unknown" | "history-only" | "ok"; +}; diff --git a/src/update/stop-decision.mjs b/src/update/stop-decision.mjs new file mode 100644 index 0000000000..e96c11ae17 --- /dev/null +++ b/src/update/stop-decision.mjs @@ -0,0 +1,34 @@ +import { STOP_HISTORY_INCOMPLETE_EXIT_CODE } from "./stop-contract.mjs"; + +/** + * May an update replace package files after `ocx stop` returned? + * + * Both updaters ask this: `src/update/index.ts` on the Bun path and `bin/ocx.mjs` on the + * npm path the dashboard uses. It lives here as plain ESM so the Node launcher can import + * it, and so the two lanes cannot drift into disagreeing about the same situation — which + * is how #3008 shipped in the first place, with the fix on one side only. + * + * Returns `{ proceed, reason }`. The reasons are: + * + * - `stop-failed` — a nonzero status other than the history-only code, or a signal kill. + * A signal kill carries no evidence the teardown finished, so it is not a maybe. + * - `runtime-state` — a PID or runtime-port record survived the stop. + * - `teardown-outstanding` — a shared-teardown obligation survived the stop. That is a + * quarantined receipt awaiting a human: the stop itself can succeed (there was nothing + * left to stop), so checking only BEFORE the stop let the retry sail straight through + * and install over a teardown that never ran. + * - `proxy-live` — something is still answering as our proxy on the captured endpoint. + * - `proxy-unknown` — the probe could not answer. Absence of proof is not proof of + * absence, and replacing files under a live server leaves it running a mix of old and + * new modules. + * - `ok` / `history-only` — proceed; the second also prints the manifest warning. + */ +export function decidePostStopUpdate({ status, hasRuntimeState, liveness, teardownOutstanding = false }) { + const historyOnly = status === STOP_HISTORY_INCOMPLETE_EXIT_CODE; + if (status !== 0 && !historyOnly) return { proceed: false, reason: "stop-failed" }; + if (hasRuntimeState) return { proceed: false, reason: "runtime-state" }; + if (teardownOutstanding) return { proceed: false, reason: "teardown-outstanding" }; + if (liveness === "live") return { proceed: false, reason: "proxy-live" }; + if (liveness !== "dead") return { proceed: false, reason: "proxy-unknown" }; + return { proceed: true, reason: historyOnly ? "history-only" : "ok" }; +} diff --git a/tests/cli-ready.test.ts b/tests/cli-ready.test.ts index 577491e0ae..56a9875b56 100644 --- a/tests/cli-ready.test.ts +++ b/tests/cli-ready.test.ts @@ -861,8 +861,10 @@ describe("handleStart OCX_SERVICE exit guard (source-level)", () => { test("service.ts teardown kills surviving wrapper processes on stop", () => { const serviceSource = readFileSync(join(import.meta.dir, "../src/service.ts"), "utf8"); expect(serviceSource).toMatch(/killWindowsServiceWrapperProcesses/); - const callSite = serviceSource.match(/stopServiceIfInstalled[\s\S]{0,1200}?killWindowsServiceWrapperProcesses\(\)/); - expect(callSite, "wrapper kill must run during stopServiceIfInstalled").not.toBeNull(); + // The boolean `stopServiceIfInstalled` is gone — it collapsed a live manager into the + // same false as "not installed" (#3008). The stop itself is the detailed function. + const callSite = serviceSource.match(/stopServiceIfInstalledDetailed[\s\S]{0,1600}?killWindowsServiceWrapperProcesses\(\)/); + expect(callSite, "wrapper kill must run during stopServiceIfInstalledDetailed").not.toBeNull(); }); test("wrapper kill matches the canonical paths of THIS installation, not bare filenames", () => { diff --git a/tests/grok-lifecycle.test.ts b/tests/grok-lifecycle.test.ts index f245e917b5..b33c19d96c 100644 --- a/tests/grok-lifecycle.test.ts +++ b/tests/grok-lifecycle.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; import { join } from "node:path"; -import { isServiceOwnershipError, ServiceOwnershipError } from "../src/service"; +import { classifyWindowsServiceStop, installedServiceRespawnRisk, isServiceOwnershipError, ServiceOwnershipError } from "../src/service"; const CLI_SOURCE = readFileSync(join(import.meta.dir, "..", "src", "cli", "index.ts"), "utf8"); const ENSURE_SOURCE = readFileSync(join(import.meta.dir, "..", "src", "cli", "ensure-desired-integrations.ts"), "utf8"); @@ -95,20 +95,25 @@ describe("Grok fence lifecycle wiring", () => { expect(stopFn).toContain("isServiceOwnershipError(err)"); expect(stopFn).toContain("ownershipBlocked = true"); - expect(stopFn).toContain("if (!ownershipBlocked)"); + // Ownership is now one of two reasons to skip the restore; the other is an inherited + // obligation whose proxy could not be confirmed down (#3008). + expect(stopFn).toContain("const restoreBlocked = ownershipBlocked ||"); + expect(stopFn).toContain("if (!restoreBlocked) {"); expect(stopFn).toContain("await restoreSharedClientStateAfterStop()"); expect(restoreFn).toContain("restoreNativeCodexAsync()"); expect(restoreFn).not.toContain("revertSystemEnv()"); expect(restoreFn).toContain("stripGrokConfig()"); - expect(stopFn.indexOf("revertSystemEnv()")).toBeLessThan(stopFn.indexOf("if (!ownershipBlocked)")); + expect(stopFn.indexOf("revertSystemEnv()")).toBeLessThan(stopFn.indexOf("if (!restoreBlocked) {")); }); test("a refused Grok strip makes ocx stop fail instead of reporting success", () => { const restoreFn = sliceFn(CLI_SOURCE, "async function restoreSharedClientStateAfterStop(", "async function handleStop("); const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); - expect(restoreFn).toContain("else if (!grok.ok) { restored = false;"); + // A Grok strip failure is "other", never history-only: it points Grok at a dead proxy, + // so an update must abort rather than proceed (#3008). + expect(restoreFn).toContain("else if (!grok.ok) { other = true;"); expect(restoreFn).toContain("Grok config restore failed"); - expect(stopFn).toContain("if (!await restoreSharedClientStateAfterStop()) stopFailed = true"); + expect(stopFn).toContain("if (restore.other) stopFailed = true"); }); test("a refused proxy stop reports WHY, not just that it failed", () => { @@ -148,13 +153,161 @@ describe("Grok fence lifecycle wiring", () => { expect(restartHelper).toContain("requestBoundSystemRestart(previous, deadlineAt)"); }); + test("a stopped scheduler is verified across the respawn window before stop succeeds", () => { + const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); + // killWindowsSchedulerWrappers is best-effort and the `:loop` wrapper respawns after + // ~5s, so "stopped" alone is not a proven-down proxy. An update that trusts it can + // start replacing files during the dead interval (#3008). + expect(stopFn).toContain("proxyStillLiveAfterStop({ canRespawn: true })"); + // A survivor is an ordinary failure AND blocks shared teardown: restoring client + // config while the proxy runs leaves both pointing at each other. + expect(stopFn).toContain("stopFailed = true;"); + expect(stopFn).toContain("ownershipBlocked = true;"); + }); + + test("only Task Scheduler earns the respawn wait", () => { + const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); + const serviceSource = readFileSync(join(import.meta.dir, "..", "src", "service.ts"), "utf8"); + // schtasks /end leaves the `cmd :loop` wrapper alive to respawn its child (#764). + // launchd, systemd and WinSW are down when they report stopped, so charging them a + // seven-second poll on every ocx stop would be a regression in ordinary use. + expect(serviceSource).toContain('"absent" | "stopped" | "stopped-respawnable" | "failed"'); + expect(serviceSource).toContain('schedulerStopped ? "stopped-respawnable" : "stopped"'); + expect(stopFn).toContain("if (schedulerCanRespawn && !ownershipBlocked)"); + // The wait is gated on the scheduler flag, not on "a service stopped". + expect(stopFn).not.toContain("if (stoppedService && !ownershipBlocked)"); + }); + + test("ocx stop defers shared teardown so a respawn survivor keeps its config", () => { + const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); + const apiSource = readFileSync(join(import.meta.dir, "..", "src", "server", "management-api.ts"), "utf8"); + const controlSource = readFileSync(join(import.meta.dir, "..", "src", "lib", "process-control.ts"), "utf8"); + // POST /api/stop normally restores native Codex and strips the Grok fence itself. If + // ocx stop let it, a scheduler wrapper that respawns seconds later would already have + // lost its client config, and the parent ownershipBlocked guard could only prevent a + // second redundant teardown (#3008). + expect(stopFn).toContain("deferSharedTeardownNonce: teardownNonce"); + expect(controlSource).toContain("deferSharedTeardown"); + expect(apiSource).toContain("performStopTeardown(url, { ownsReceipt: deferralMatchesReceipt })"); + // The deferral is an obligation, so it is claimed on disk BEFORE it is requested and + // released only after THIS process has restored the shared config itself. A bare + // query flag could not survive the parent dying mid-stop. + const claimAt = stopFn.indexOf("claimTeardown(exact ?? configuredEndpoint()"); + expect(claimAt).toBeGreaterThan(-1); + expect(claimAt).toBeLessThan(stopFn.indexOf("deferSharedTeardownNonce: teardownNonce")); + // One resolved stop target feeds BOTH the receipt and the request, so the endpoint + // recorded is the endpoint contacted — recovery probes exactly that one. + // The endpoint is resolved ONCE: reading the runtime record twice let the receipt name + // the configured guess while the request went to one that appeared in between. + expect(stopFn).toContain("const exact = discovered ?? endpointOf(readRuntimePort(pid));"); + // Every stop claims a receipt, including the one that resolves no endpoint at all — + // that path goes straight to the kill ladder with no child teardown, so a warning + // instead of a receipt is exactly the parent-crash window this exists to close. + expect(stopFn).toContain('claimTeardown(exact ?? configuredEndpoint(), exact ? "exact" : "guessed");'); + // A guessed endpoint records an obligation but must not direct the stop request. + expect(stopFn).toContain("runtimeEndpoint: exact ?? undefined"); + // Nor may it authorize a later recovery: "the configured port refuses" is not proof + // that a proxy on an explicit --port is down. + expect(stopFn).toContain('if (read.receipt.endpointSource === "guessed")'); + const guessedBranch = stopFn.slice(stopFn.indexOf('if (read.receipt.endpointSource === "guessed")'), stopFn.indexOf("if (await abandonedTeardownIsSafeToFinish(")); + expect(guessedBranch).toContain("inheritedBlocks = true;"); + expect(guessedBranch).toContain("stopFailed = true;"); + expect(controlSource).toContain("io.runtimeEndpoint ?? readRuntime(pid)"); + // Inherited obligations are snapshotted BEFORE this run claims anything, so its own + // receipt is never mistaken for one it inherited. + expect(stopFn).toContain("isPendingTeardownAbandoned(read, isProcessAlive)"); + expect(stopFn.indexOf("listPendingTeardowns()")).toBeLessThan(claimAt); + expect(stopFn).toContain("clearPendingTeardown(nonce)"); + expect(stopFn.indexOf("await restoreSharedClientStateAfterStop()")) + .toBeLessThan(stopFn.indexOf("clearPendingTeardown(nonce)")); + // A receipt that survives its discharge would re-trigger recovery forever. + expect(stopFn).toContain("if (!clearPendingTeardown(nonce)) {"); + }); + + test("an unconfirmed inherited obligation blocks the restore, it does not merely warn", () => { + const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); + // Finishing SOMEBODY ELSE's obligation needs a definitive "dead", not findLiveProxy's + // null, which also covers a timeout and a listener that withholds /healthz. The first + // attempt at this only logged a warning and then restored anyway, which is not a gate. + expect(stopFn).toContain("await abandonedTeardownIsSafeToFinish(read.receipt.endpoint)"); + expect(stopFn).toContain("const restoreBlocked = ownershipBlocked || inheritedBlocks"); + expect(stopFn).toContain("if (!restoreBlocked) {"); + // The restore is reached only through that gate — no other call site may bypass it. + const restoreCalls = stopFn.split("await restoreSharedClientStateAfterStop()").length - 1; + expect(restoreCalls).toBe(1); + expect(stopFn.indexOf("const restoreBlocked")).toBeLessThan(stopFn.indexOf("await restoreSharedClientStateAfterStop()")); + // An obligation that cannot be discharged fails the stop and is preserved. + const gateBlock = stopFn.slice(stopFn.indexOf("const recoveredNonces"), stopFn.indexOf("const restoreBlocked")); + expect(gateBlock).toContain("inheritedBlocks = true;"); + expect(gateBlock).toContain("stopFailed = true;"); + expect(gateBlock).not.toContain("clearPendingTeardown"); + // An unreadable obligation names no endpoint, so it can never probe dead. It fails the + // stop rather than being waved through, and is set aside only AFTER the outcome is + // known — moving it earlier would erase it from every future scan while the restore it + // stood for had not run. + expect(gateBlock).not.toContain("quarantinePendingTeardown"); + // Setting aside is not discharging, and the message must not claim otherwise: the + // renamed file still blocks an update until an operator removes it. + const quarantineBlock = stopFn.slice(stopFn.indexOf("if (unreadable.length > 0"), stopFn.indexOf("// Set the code rather than exiting inline")); + expect(quarantineBlock).toContain("It still blocks 'ocx update'"); + expect(quarantineBlock).toContain("has NOT restored on its behalf"); + expect(quarantineBlock).not.toContain("no longer blocks an update"); + expect(stopFn.indexOf("await restoreSharedClientStateAfterStop()")) + .toBeLessThan(stopFn.indexOf("quarantinePendingTeardown(read.nonce)")); + // Inherited receipts are evaluated whether or not this run claimed one of its own, and + // every discharged nonce is released together — otherwise a stop that finds a live + // proxy clears only its own and older obligations accumulate forever. + expect(stopFn).toContain("if (inheritedTeardowns.length > 0 && !ownershipBlocked)"); + expect(stopFn).toContain("teardownNonce ? [teardownNonce, ...recoveredNonces] : recoveredNonces"); + // The orphan path hands over the endpoint the probe already found; its runtime record + // is typically what went missing in the first place. + expect(stopFn).toContain('stopWithDeferral(live.pid, { hostname: live.hostname ?? "127.0.0.1", port: live.port })'); + // A live proxy with no killable pid is not "no proxy found": purging state and + // restoring over it is the same failure arrived at from the other direction. + expect(stopFn).toContain("} else if (live) {"); + const noPidBranch = stopFn.slice(stopFn.indexOf("} else if (live) {"), stopFn.indexOf('} else if (!stoppedService) {')); + expect(noPidBranch).toContain("stopFailed = true;"); + expect(noPidBranch).toContain("ownershipBlocked = true;"); + const gateFn = sliceFn(CLI_SOURCE, "const abandonedTeardownIsSafeToFinish", "let stopFailed = false;"); + expect(gateFn).toContain('probeProxyLiveness(endpoint.port, endpoint.hostname) === "dead"'); + expect(gateFn).toContain("return false;"); + }); + + test("an outstanding teardown receipt makes both updaters run the stop", () => { + // After a parent crashed mid-deferral the service, pid and runtime records can all be + // absent while shared client config still points at a proxy that is gone. Installing + // over that skips the recovery the receipt exists to trigger (#3008). + const updateSource = readFileSync(join(import.meta.dir, "..", "src", "update", "index.ts"), "utf8"); + expect(updateSource).toContain("readPid() || readRuntimePort() || pendingTeardownOutstanding()"); + const launcherSource = readFileSync(join(import.meta.dir, "..", "bin", "ocx.mjs"), "utf8"); + // The launcher runs under plain Node, so it shares the naming rule as ESM rather than + // spelling it out — which is how it ended up watching the retired singleton filename + // after receipts moved to one file per claim, silently seeing none of them. + expect(launcherSource).toContain("hasPendingTeardownIn(readdirSync, configDir())"); + expect(launcherSource).not.toContain('"pending-teardown.json"'); + expect(launcherSource).toContain("serviceWasInstalled || hasRuntimeState || hasPendingTeardown"); + // Checked AFTER the stop too: a quarantined receipt lets the stop succeed, so a + // pre-stop check alone let the retry install over a teardown that never ran. + expect(launcherSource).toContain("teardownOutstanding: hasPendingTeardownIn(readdirSync, configDir())"); + const updateSource2 = readFileSync(join(import.meta.dir, "..", "src", "update", "index.ts"), "utf8"); + expect(updateSource2).toContain("teardownOutstanding: pendingTeardownOutstanding()"); + const decisionSource = readFileSync(join(import.meta.dir, "..", "src", "update", "stop-decision.mjs"), "utf8"); + expect(decisionSource).toContain('if (teardownOutstanding) return { proceed: false, reason: "teardown-outstanding" };'); + const receiptSource = readFileSync(join(import.meta.dir, "..", "src", "config", "pending-teardown.ts"), "utf8"); + expect(receiptSource).toContain('from "./pending-teardown-names.mjs"'); + expect(receiptSource).toContain("isPendingTeardownFileName(name)"); + }); + test("handleStop treats an incomplete native Codex restore as a stop failure", () => { const restoreFn = sliceFn(CLI_SOURCE, "async function restoreSharedClientStateAfterStop(", "async function handleStop("); const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); expect(restoreFn).toContain("if (result.success) console.log"); - expect(restoreFn).toContain("restored = false"); + // Config or catalog failure is a real teardown failure - a client reads those. Only a + // history-only failure is separable, and it still surfaces (#3008). + expect(restoreFn).toContain('artifacts.config.state === "failed" || artifacts.catalog.state === "failed"'); + expect(restoreFn).toContain("else other = true"); expect(restoreFn).toContain("console.error(`⚠️ ${result.message}`)"); - expect(stopFn).toContain("if (!await restoreSharedClientStateAfterStop()) stopFailed = true"); + expect(stopFn).toContain("if (restore.other) stopFailed = true"); }); test("the daemon's exit cleanup keeps the OCX_SERVICE exclusion and adds the ownership check", () => { @@ -217,15 +370,124 @@ describe("POST /api/stop teardown", () => { }); test("strips the Grok fence on an accepted stop", () => { + // The teardown moved to src/server/stop-teardown.ts so a test can call it: the route + // schedules process.exit 200ms after answering, which made the inline version + // unreachable. tests/stop-deferred-teardown.test.ts proves the behaviour; this proves + // the route still delegates to it rather than growing a second copy. + const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); + expect(handler).toContain("performStopTeardown(url, { ownsReceipt: deferralMatchesReceipt })"); + const teardownSource = readFileSync(join(import.meta.dir, "..", "src", "server", "stop-teardown.ts"), "utf8"); + expect(teardownSource).toContain('await import("../grok/inject")'); + expect(teardownSource).toContain("stripGrokConfig()"); + }); + + test("an unreadable scheduler state gets the same diagnosis from the CLI and the API", () => { + const serviceSource = readFileSync(join(import.meta.dir, "..", "src", "service.ts"), "utf8"); + // A manager that refused to stop and a query that could not answer are different + // problems: reporting the second as "did not stop" sends the operator looking for the + // wrong thing, and `ocx stop` was the command the API told them to run (#3008). + expect(serviceSource).toContain('"absent" | "stopped" | "stopped-respawnable" | "failed" | "state-unknown"'); + // Behavioural, because a source-text assertion cannot tell whether an unreadable probe + // is still being folded into the generic failure. + expect(classifyWindowsServiceStop({ stopped: false, failed: false, schedulerStopped: false, stateUnknown: true })) + .toBe("state-unknown"); + // A readable failure outranks it — something actually refused to stop. + expect(classifyWindowsServiceStop({ stopped: false, failed: true, schedulerStopped: false, stateUnknown: true })) + .toBe("failed"); + // And an unreadable state outranks success: a scheduler we cannot see may respawn. + expect(classifyWindowsServiceStop({ stopped: true, failed: false, schedulerStopped: true, stateUnknown: true })) + .toBe("state-unknown"); + expect(classifyWindowsServiceStop({ stopped: true, failed: false, schedulerStopped: true, stateUnknown: false })) + .toBe("stopped-respawnable"); + expect(classifyWindowsServiceStop({ stopped: true, failed: false, schedulerStopped: false, stateUnknown: false })) + .toBe("stopped"); + expect(classifyWindowsServiceStop({ stopped: false, failed: false, schedulerStopped: false, stateUnknown: false })) + .toBe("absent"); + const stopFn = sliceFn(CLI_SOURCE, "async function handleStop(", "async function handleUninstall("); + expect(stopFn).toContain('if (serviceStop === "state-unknown")'); + const unknownBranch = stopFn.slice(stopFn.indexOf('if (serviceStop === "state-unknown")'), stopFn.indexOf('if (serviceStop === "state-unknown")') + 700); + expect(unknownBranch).toContain("stopFailed = true;"); + expect(unknownBranch).toContain("ocx service status"); + expect(unknownBranch).not.toContain("did not stop"); const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); - expect(handler).toContain('await import("../grok/inject")'); - expect(handler).toContain("stripGrokConfig()"); + expect(handler).toContain('if (serviceStop === "state-unknown")'); + // The route answers the post-stop case with the same code as the pre-check. + expect((handler.match(/service_state_unknown/g) ?? []).length).toBeGreaterThanOrEqual(2); }); test("maps a failed shutdown drain to a nonzero process exit", () => { const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); expect(handler).toContain("shutdownSucceeded = await drainAndShutdown"); - expect(handler).toContain("process.exit(shutdownSucceeded ? 0 : 1)"); + expect(handler).toContain("process.exit(shutdownSucceeded && teardown.success ? 0 : 1)"); + }); + + test("the route consumes the detailed service outcome instead of the boolean", () => { + const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); + // stopServiceIfInstalled collapses "failed" into the same false as "not installed", so + // this route used to tear down shared config while a manager that refused to stop was + // still there to respawn the proxy (#3008). + expect(handler).toContain("stopServiceIfInstalledDetailed()"); + expect(handler).not.toContain("stopServiceIfInstalled();"); + expect(handler).toContain('if (serviceStop === "failed")'); + expect(handler.indexOf('if (serviceStop === "failed")')).toBeLessThan(handler.indexOf("await performStopTeardown")); + }); + + test("a respawnable backend is refused BEFORE the manager is touched", () => { + const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); + // Stopping the Task Scheduler task and then returning 409 left the proxy running with + // its manager stopped — worse than either outcome, and the dashboard's Stop button + // sends a bare request on every backend. + expect(handler).toContain('const respawnRisk = holdsReceipt ? "none" : installedServiceRespawnRisk();'); + expect(handler).toContain('code: "respawnable_service"'); + expect(handler.indexOf("installedServiceRespawnRisk()")).toBeLessThan(handler.indexOf("stopServiceIfInstalledDetailed()")); + // The refusal must say nothing was changed, because nothing was. + expect(handler).toContain("Nothing was changed."); + // An unreadable scheduler state is its own answer: sending that operator to `ocx stop` + // would be a loop, because it maps the same unknown probe to a stop failure. + expect(handler).toContain('code: "service_state_unknown"'); + const unknownBranch = handler.slice(handler.indexOf('code: "service_state_unknown"'), handler.indexOf('code: "service_state_unknown"') + 500); + expect(unknownBranch).toContain("ocx service status"); + expect(unknownBranch).not.toContain("run `ocx stop`"); + }); + + test("only a proven absence is safe to stop inline", () => { + // Behavioural, not source-shaped: the previous assertion matched an unrelated + // `return true` in the catch and therefore passed while "unknown" was let through. + expect(installedServiceRespawnRisk(() => ({ status: "present" }) as never, "win32")).toBe("respawnable"); + // "unknown" is an ordinary return value from the probe, not a throw. Treating it as + // absence let the route kill scheduler wrappers before refusing. + // It is also kept distinct from "respawnable", because the remedy differs: `ocx stop` + // maps the same unknown to a stop failure, so telling that operator to run it loops. + expect(installedServiceRespawnRisk(() => ({ status: "unknown" }) as never, "win32")).toBe("unknown"); + expect(installedServiceRespawnRisk(() => { throw new Error("schtasks unavailable"); }, "win32")).toBe("unknown"); + // A proven absence is the only case that proceeds. + expect(installedServiceRespawnRisk(() => ({ status: "absent" }) as never, "win32")).toBe("none"); + // Every other platform is down when it says so; no wrapper can respawn. + expect(installedServiceRespawnRisk(() => ({ status: "present" }) as never, "darwin")).toBe("none"); + expect(installedServiceRespawnRisk(() => ({ status: "present" }) as never, "linux")).toBe("none"); + }); + + test("the daemon's exit status reflects the shared teardown, not just the drain", () => { + const handler = sliceFn(MANAGEMENT_SOURCE, '"/api/stop"', "/api/codex-auth/"); + // A drained proxy whose restore failed did not finish the job; exiting 0 told a + // supervisor the stop was clean while client config still pointed at this process. + expect(handler).toContain("process.exit(shutdownSucceeded && teardown.success ? 0 : 1)"); + }); + + test("direct service stop and uninstall fail when a shared teardown half fails", () => { + const serviceSource = readFileSync(join(import.meta.dir, "..", "src", "service.ts"), "utf8"); + // These paths logged the failure and exited 0, so a script could not tell a complete + // teardown from one that left Grok aimed at a stopped proxy. + const stopCase = serviceSource.slice( + serviceSource.indexOf("service stopped + native Codex restored"), + serviceSource.indexOf('case "status": {'), + ); + expect(stopCase).toContain("if (!restore.success) process.exitCode = 1;"); + expect((stopCase.match(/process\.exitCode = 1;/g) ?? []).length).toBeGreaterThanOrEqual(2); + const uninstallStart = serviceSource.indexOf("`⚠️ native Codex restore FAILED:"); + expect(uninstallStart).toBeGreaterThan(-1); + const uninstallCase = serviceSource.slice(uninstallStart, uninstallStart + 700); + expect((uninstallCase.match(/process\.exitCode = 1;/g) ?? []).length).toBeGreaterThanOrEqual(2); }); test("a 409 does not escalate to a forced kill", () => { diff --git a/tests/restore-completes-shared-teardown.test.ts b/tests/restore-completes-shared-teardown.test.ts new file mode 100644 index 0000000000..4efd282e28 --- /dev/null +++ b/tests/restore-completes-shared-teardown.test.ts @@ -0,0 +1,177 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { dispatchCommand, type CliDispatchDeps } from "../src/cli/dispatch"; + +/** + * `ocx restore` must finish the WHOLE shared teardown, including when Codex is already + * off (#3008). + * + * The deferred-teardown recovery path prints "run 'ocx restore', then delete the receipt". + * If restore returns success on the Codex no-op path before touching the Grok fence, an + * operator following those instructions signs off an incomplete teardown and deletes the + * obligation that would have caught it — leaving Grok pointed at a proxy that is gone. + */ + +const BEGIN = "# >>> opencodex managed block — do not edit (removed by `ocx stop`) >>>"; +const END = "# <<< opencodex managed block <<<"; +const depsFor = (args: string[]) => ({ args } as unknown as CliDispatchDeps); + +let grokHome: string; +let opencodexHome: string; +let codexHome: string; +let previous: Record = {}; + +beforeEach(() => { + previous = { + GROK_HOME: process.env.GROK_HOME, + OPENCODEX_HOME: process.env.OPENCODEX_HOME, + CODEX_HOME: process.env.CODEX_HOME, + }; + grokHome = mkdtempSync(join(tmpdir(), "ocx-restore-grok-")); + opencodexHome = mkdtempSync(join(tmpdir(), "ocx-restore-home-")); + codexHome = mkdtempSync(join(tmpdir(), "ocx-restore-codex-")); + process.env.GROK_HOME = grokHome; + process.env.OPENCODEX_HOME = opencodexHome; + process.env.CODEX_HOME = codexHome; +}); + +afterEach(() => { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + for (const dir of [grokHome, opencodexHome, codexHome]) rmSync(dir, { recursive: true, force: true }); +}); + +async function seedOffConfig(): Promise { + // Codex already OFF in this home, so the desired-state write reports "unchanged" and the + // residue classifier reports clean — the no-op path under test. Written through the real + // saver so the file satisfies the same schema the CLI validates. + const { loadConfig, saveConfig } = await import("../src/config"); + const config = loadConfig(); + saveConfig({ ...config, clientIntegrations: { ...(config.clientIntegrations ?? {}), codex: false } }); +} + +async function seedOnConfig(): Promise { + // Codex ON, so the desired-state write is a real change and restore takes its ordinary + // forward path rather than the already-clean branch. + const { loadConfig, saveConfig } = await import("../src/config"); + const config = loadConfig(); + const integrations = { ...(config.clientIntegrations ?? {}) }; + delete integrations.codex; + saveConfig({ ...config, clientIntegrations: integrations }); +} + +function writeManagedGrokFence(): string { + mkdirSync(grokHome, { recursive: true }); + const configPath = join(grokHome, "config.toml"); + writeFileSync(configPath, [ + "# user content above", + BEGIN, + 'base_url = "http://127.0.0.1:10100/v1"', + END, + "", + ].join("\n")); + return configPath; +} + +test("restore strips the Grok fence even when Codex is already off and native", async () => { + await seedOffConfig(); + const configPath = writeManagedGrokFence(); + expect(readFileSync(configPath, "utf8")).toContain(BEGIN); + + // Codex is untouched in this home, so the desired-state write is "unchanged" and the + // residue classifier reports clean — the exact no-op path that used to return 0 before + // stripGrokConfig() ever ran. + const code = await dispatchCommand({ kind: "run", command: "restore", args: ["restore"] }, depsFor(["restore"])); + + const after = readFileSync(configPath, "utf8"); + expect(after).not.toContain(BEGIN); + expect(after).not.toContain(END); + expect(after).toContain("# user content above"); + expect(code).toBe(0); +}); + +test("the JSON envelope on that path reports the Grok cleanup too", async () => { + await seedOffConfig(); + writeManagedGrokFence(); + const lines: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { + await dispatchCommand({ kind: "run", command: "restore", args: ["restore", "--json"] }, depsFor(["restore", "--json"])); + } finally { + console.log = originalLog; + } + const envelope = JSON.parse(lines.at(-1)!); + // A machine caller must not read "already OFF and native" as "nothing was left to do". + expect(envelope.success).toBe(true); + expect(String(envelope.message)).toContain("already OFF and native"); + expect(String(envelope.message)).toMatch(/Grok|managed block/i); +}); + +test("the ordinary forward-restore path strips the fence before emitting JSON", async () => { + await seedOnConfig(); + // NOT the already-clean branch: Codex is ON here, so restore runs its real machinery + // and used to return the JSON envelope before stripGrokConfig() was ever called. + const configPath = writeManagedGrokFence(); + const lines: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { + await dispatchCommand({ kind: "run", command: "restore", args: ["restore", "--json"] }, depsFor(["restore", "--json"])); + } finally { + console.log = originalLog; + } + // The fence itself, not the wording: a message can claim a cleanup that never happened. + const after = readFileSync(configPath, "utf8"); + expect(after).not.toContain(BEGIN); + expect(after).toContain("# user content above"); + const envelope = JSON.parse(lines.at(-1)!); + expect(envelope).toHaveProperty("artifacts"); + expect(String(envelope.message)).toMatch(/Grok|managed block/i); +}); + +test("eject --json is the same runner and gets the same teardown", async () => { + await seedOnConfig(); + const configPath = writeManagedGrokFence(); + const lines: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { + await dispatchCommand({ kind: "run", command: "eject", args: ["eject", "--json"] }, depsFor(["eject", "--json"])); + } finally { + console.log = originalLog; + } + expect(readFileSync(configPath, "utf8")).not.toContain(BEGIN); +}); + +test("a Grok cleanup failure is not reported as a successful restore", async () => { + await seedOffConfig(); + // A directory where config.toml belongs: the strip cannot succeed, and the envelope + // must not say the teardown is done. + mkdirSync(join(grokHome, "config.toml"), { recursive: true }); + const lines: string[] = []; + const originalLog = console.log; + const originalError = console.error; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + console.error = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + let code: number; + try { + code = await dispatchCommand({ kind: "run", command: "restore", args: ["restore", "--json"] }, depsFor(["restore", "--json"])); + } finally { + console.log = originalLog; + console.error = originalError; + } + expect(code).toBe(1); +}); + +test("with no Grok home at all the no-op path still succeeds quietly", async () => { + await seedOffConfig(); + rmSync(grokHome, { recursive: true, force: true }); + const code = await dispatchCommand({ kind: "run", command: "restore", args: ["restore"] }, depsFor(["restore"])); + expect(code).toBe(0); + expect(existsSync(grokHome)).toBe(false); +}); diff --git a/tests/stop-deferred-teardown.test.ts b/tests/stop-deferred-teardown.test.ts new file mode 100644 index 0000000000..69f978c83d --- /dev/null +++ b/tests/stop-deferred-teardown.test.ts @@ -0,0 +1,457 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { stopProxyGracefully } from "../src/lib/process-control"; +import { performStopTeardown } from "../src/server/stop-teardown"; +import type { CodexNativeRestoreResult } from "../src/codex/inject"; + +/** + * Behavioural cover for the deferred shared teardown (#3008). + * + * The wiring assertions in tests/grok-lifecycle.test.ts read source text, which cannot + * tell a working deferral from a plausible-looking one. These tests call the real + * functions: the graceful-stop client that builds the URL, the teardown decision the + * route delegates to, and the on-disk receipts that decide whether a deferral is an owned + * obligation or an unbacked request. + */ + +const ENDPOINT = { hostname: "127.0.0.1", port: 10100 }; +const FOREIGN_NONCE = "ffffffffffffffffffffffffffffffff"; +let home: string; +let previousHome: string | undefined; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + home = mkdtempSync(join(tmpdir(), "ocx-deferred-teardown-")); + process.env.OPENCODEX_HOME = home; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + rmSync(home, { recursive: true, force: true }); +}); + +function restoreResult(success: boolean): CodexNativeRestoreResult { + return { + success, + message: success ? "native Codex restored" : "config restore failed", + artifacts: { + config: { state: success ? "restored" : "failed" }, + catalog: { state: "restored" }, + history: { state: "restored" }, + }, + } as unknown as CodexNativeRestoreResult; +} + +describe("stopProxyGracefully deferral flag", () => { + test("the default stop asks for no deferral", async () => { + const urls: string[] = []; + await stopProxyGracefully(11, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async (url: string | URL | Request) => { + urls.push(String(url)); + return new Response(JSON.stringify({ success: true }), { status: 200 }); + }) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(urls).toEqual(["http://127.0.0.1:10100/api/stop"]); + }); + + test("a claimed nonce is carried in the query the route reads", async () => { + const urls: string[] = []; + await stopProxyGracefully(11, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async (url: string | URL | Request) => { + urls.push(String(url)); + return new Response(JSON.stringify({ success: true }), { status: 200 }); + }) as typeof fetch, + waitExit: () => true, + env: {}, + deferSharedTeardownNonce: FOREIGN_NONCE, + }); + expect(urls).toEqual([`http://127.0.0.1:10100/api/stop?deferSharedTeardown=1&teardownNonce=${FOREIGN_NONCE}`]); + }); + + test("the caller's endpoint snapshot is used instead of re-reading the runtime file", async () => { + const urls: string[] = []; + // The receipt records the endpoint the stop contacted. If this call re-read the + // runtime record it could contact a different one, and recovery would then probe an + // endpoint that was never stopped. + await stopProxyGracefully(11, { + readRuntime: () => ({ port: 19999, hostname: "127.0.0.1" }), + runtimeEndpoint: { hostname: "127.0.0.1", port: 10100 }, + fetchFn: (async (url: string | URL | Request) => { + urls.push(String(url)); + return new Response(JSON.stringify({ success: true }), { status: 200 }); + }) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(urls).toEqual(["http://127.0.0.1:10100/api/stop"]); + }); +}); + +describe("performStopTeardown", () => { + test("an ordinary stop restores native Codex and strips the Grok fence", async () => { + let restored = 0; + let stripped = 0; + const body = await performStopTeardown(new URL("http://127.0.0.1:10100/api/stop"), { + ownsReceipt: () => false, + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => { stripped += 1; return { ok: true, changed: true, message: "Grok config restored" }; }, + }); + expect(restored).toBe(1); + expect(stripped).toBe(1); + expect(body.sharedTeardown).toBe("performed"); + expect(body.message).toContain("native Codex restored"); + }); + + test("a receipt-backed deferral touches neither config and says so", async () => { + let restored = 0; + let stripped = 0; + const body = await performStopTeardown(new URL(`http://127.0.0.1:10100/api/stop?deferSharedTeardown=1&teardownNonce=${FOREIGN_NONCE}`), { + ownsReceipt: nonce => nonce === FOREIGN_NONCE, + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => { stripped += 1; return { ok: true, changed: true, message: "Grok config restored" }; }, + }); + expect(restored).toBe(0); + expect(stripped).toBe(0); + expect(body.sharedTeardown).toBe("deferred"); + expect(body.message).toContain("deferred to the stopping client"); + // The old response claimed a restore that never happened; an operator reading it + // would believe native Codex was back while the deferral was still outstanding. + expect(body.message).not.toContain("native Codex restored"); + }); + + test("the real ownership check accepts only a nonce with a readable receipt on disk", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + let restored = 0; + const deferred = await performStopTeardown(new URL(`http://127.0.0.1:10100/api/stop?deferSharedTeardown=1&teardownNonce=${claimed.nonce}`), { + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => ({ ok: true, changed: true, message: "Grok config restored" }), + }); + expect(deferred.sharedTeardown).toBe("deferred"); + expect(restored).toBe(0); + + // Another caller riding on the existence of that obligation gets nothing: it does not + // own the nonce, so it cannot hand its teardown to anyone. + const ridden = await performStopTeardown(new URL(`http://127.0.0.1:10100/api/stop?deferSharedTeardown=1&teardownNonce=${FOREIGN_NONCE}`), { + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => ({ ok: true, changed: true, message: "Grok config restored" }), + }); + expect(ridden.sharedTeardown).toBe("performed"); + expect(restored).toBe(1); + }); + + test("the query alone does not buy a deferral without a receipt", async () => { + let restored = 0; + const body = await performStopTeardown(new URL("http://127.0.0.1:10100/api/stop?deferSharedTeardown=1"), { + ownsReceipt: () => false, + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => ({ ok: true, changed: true, message: "Grok config restored" }), + }); + // An authenticated caller that sets the flag and exits must not be able to leave + // client config pointed at a proxy that is going away. + expect(restored).toBe(1); + expect(body.sharedTeardown).toBe("performed"); + }); + + test("an unreadable receipt does not authorize a deferral", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + writeFileSync(mod.pendingTeardownPathFor(claimed.nonce), "{not json"); + let restored = 0; + const body = await performStopTeardown(new URL(`http://127.0.0.1:10100/api/stop?deferSharedTeardown=1&teardownNonce=${claimed.nonce}`), { + restoreNativeCodex: async () => { restored += 1; return restoreResult(true); }, + stripGrok: () => ({ ok: true, changed: true, message: "Grok config restored" }), + }); + expect(restored).toBe(1); + expect(body.sharedTeardown).toBe("performed"); + }); + + test("a failed restore still reports failure and the remediation", async () => { + const body = await performStopTeardown(new URL("http://127.0.0.1:10100/api/stop"), { + ownsReceipt: () => false, + restoreNativeCodex: async () => restoreResult(false), + stripGrok: () => ({ ok: false, changed: false, message: "grok home is read-only" }), + }); + expect(body.success).toBe(false); + expect(body.message).toContain("ocx restore"); + expect(body.message).toContain("Grok config cleanup failed"); + }); + + test("a Grok-only failure is not reported as a successful teardown", async () => { + // The native restore succeeding said nothing about the fence. Deciding success from + // the native half alone let a caller read success: true while Grok still pointed at a + // proxy that was exiting — the previous test masked it by failing both halves. + const body = await performStopTeardown(new URL("http://127.0.0.1:10100/api/stop"), { + ownsReceipt: () => false, + restoreNativeCodex: async () => restoreResult(true), + stripGrok: () => ({ ok: false, changed: false, message: "grok home is read-only" }), + }); + expect(body.success).toBe(false); + expect(body.sharedTeardown).toBe("performed"); + expect(body.message).toContain("Grok fence was not removed"); + expect(body.message).toContain("ocx restore"); + }); + + test("both halves succeeding is the only success", async () => { + const body = await performStopTeardown(new URL("http://127.0.0.1:10100/api/stop"), { + ownsReceipt: () => false, + restoreNativeCodex: async () => restoreResult(true), + stripGrok: () => ({ ok: true, changed: true, message: "Grok config restored" }), + }); + expect(body.success).toBe(true); + expect(body.message).not.toContain("Grok config cleanup failed"); + }); +}); + +describe("receipt naming is shared by both update lanes", () => { + test("the launcher's scan and the TypeScript listing agree on what is outstanding", async () => { + const mod = await import("../src/config/pending-teardown"); + const names = await import("../src/config/pending-teardown-names.mjs"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + + // bin/ocx.mjs runs under plain Node and cannot import the TypeScript module, so the + // naming rule lives in one shared .mjs. Spelling it twice is exactly how the npm lane + // ended up watching a filename that no longer existed. + expect(names.hasPendingTeardownIn(readdirSync, home)).toBe(true); + expect(mod.pendingTeardownOutstanding()).toBe(true); + + // The retired singleton name is not a receipt. + expect(names.isPendingTeardownFileName("pending-teardown.json")).toBe(false); + expect(names.isPendingTeardownFileName(`pending-teardown-${claimed.nonce}.json`)).toBe(true); + // A quarantined receipt is no longer READ by the recovery loop... + const quarantinedName = `pending-teardown-${claimed.nonce}.unreadable.json`; + expect(names.isPendingTeardownFileName(quarantinedName)).toBe(false); + // ...but it is still an obligation, so it still blocks an update. + expect(names.isQuarantinedTeardownFileName(quarantinedName)).toBe(true); + expect(names.isAnyTeardownObligationFileName(quarantinedName)).toBe(true); + + mod.quarantinePendingTeardown(claimed.nonce); + expect(mod.listPendingTeardowns()).toHaveLength(0); + // Both lanes still refuse to install over a teardown that never ran. + expect(names.hasPendingTeardownIn(readdirSync, home)).toBe(true); + expect(mod.pendingTeardownOutstanding()).toBe(true); + expect(mod.listQuarantinedTeardowns()).toHaveLength(1); + + // Only a human removing the file ends the enforcement. + rmSync(mod.listQuarantinedTeardowns()[0]!); + expect(names.hasPendingTeardownIn(readdirSync, home)).toBe(false); + expect(mod.pendingTeardownOutstanding()).toBe(false); + }); + + test("a scan that fails is not an empty scan", async () => { + const names = await import("../src/config/pending-teardown-names.mjs"); + // Only a missing home is honestly empty. Any other failure may be hiding an + // obligation, and reporting "none" would let an update install over a teardown that + // never ran — absence of proof is not proof of absence. + const enoent = Object.assign(new Error("no such directory"), { code: "ENOENT" }); + expect(names.hasPendingTeardownIn(() => { throw enoent; }, home)).toBe(false); + const denied = Object.assign(new Error("permission denied"), { code: "EACCES" }); + expect(names.hasPendingTeardownIn(() => { throw denied; }, home)).toBe(true); + expect(names.hasPendingTeardownIn(() => { throw new Error("no code at all"); }, home)).toBe(true); + }); + + test("a home that cannot be scanned is its own state, not a fabricated receipt", async () => { + const mod = await import("../src/config/pending-teardown"); + const previous = process.env.OPENCODEX_HOME; + // A file where the home should be: readdir fails with ENOTDIR, which is not absence. + const notADir = join(home, "not-a-directory"); + writeFileSync(notADir, ""); + process.env.OPENCODEX_HOME = notADir; + try { + const listed = mod.listPendingTeardowns(); + // handleStop must see something blocking rather than an empty set it would restore over. + expect(listed).toHaveLength(1); + // Not "invalid": that carries a nonce, and a synthesized one would be handed to the + // quarantine and clear paths, which could rename or delete a real receipt. + expect(listed[0]!.state).toBe("unscannable"); + expect(listed[0]).not.toHaveProperty("nonce"); + expect(mod.isPendingTeardownAbandoned(listed[0]!, () => false, 1)).toBe(true); + expect(mod.pendingTeardownOutstanding()).toBe(true); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + } + }); +}); + +describe("endpoint provenance", () => { + test("a guessed endpoint is recorded as such and is not exact evidence", async () => { + const mod = await import("../src/config/pending-teardown"); + const guessed = mod.claimPendingTeardown({ hostname: "127.0.0.1", port: 10100 }, "guessed", 1234); + const read = mod.readPendingTeardown(guessed.nonce); + expect(read.state === "valid" && read.receipt.endpointSource).toBe("guessed"); + + // A proxy started with an explicit --port can be respawned there while the configured + // address refuses, so a dead probe of THIS address proves nothing. handleStop reads + // the provenance and fails closed rather than restoring on it. + const exact = mod.claimPendingTeardown({ hostname: "127.0.0.1", port: 19999 }, "exact", 1234); + expect(mod.readPendingTeardown(exact.nonce)).toMatchObject({ state: "valid" }); + }); + + test("a receipt without provenance is invalid, so an old-format file cannot be trusted", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + writeFileSync( + mod.pendingTeardownPathFor(claimed.nonce), + JSON.stringify({ ownerPid: 1234, nonce: claimed.nonce, createdAt: "t", endpoint: ENDPOINT }), + ); + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + writeFileSync( + mod.pendingTeardownPathFor(claimed.nonce), + JSON.stringify({ ownerPid: 1234, nonce: claimed.nonce, createdAt: "t", endpoint: ENDPOINT, endpointSource: "maybe" }), + ); + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + }); +}); + +describe("post-stop update decision", () => { + test("an outstanding obligation aborts the install even when the stop succeeded", async () => { + const { decidePostStopUpdate } = await import("../src/update/stop-decision.mjs"); + // A quarantined receipt lets the stop itself succeed — there is nothing left to stop — + // so checking only BEFORE the stop let the retry sail through and install over a + // teardown that never ran. + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: false, liveness: "dead", teardownOutstanding: true })) + .toEqual({ proceed: false, reason: "teardown-outstanding" }); + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: false, liveness: "dead", teardownOutstanding: false })) + .toEqual({ proceed: true, reason: "ok" }); + // Omitting the field keeps the previous behaviour for any caller that has not adopted it. + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: false, liveness: "dead" })) + .toEqual({ proceed: true, reason: "ok" }); + // A real stop failure still wins: it is the stronger signal. + expect(decidePostStopUpdate({ status: 1, hasRuntimeState: false, liveness: "dead", teardownOutstanding: true })) + .toEqual({ proceed: false, reason: "stop-failed" }); + }); +}); + +describe("pending teardown receipts", () => { + test("a claim is durable and carries the endpoint it was stopping", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + expect(claimed.nonce).toMatch(/^[0-9a-f]{32}$/); + expect(existsSync(mod.pendingTeardownPathFor(claimed.nonce))).toBe(true); + const read = mod.readPendingTeardown(claimed.nonce); + expect(read.state).toBe("valid"); + expect(read.state === "valid" && read.receipt.endpoint).toEqual(ENDPOINT); + expect(mod.pendingTeardownOutstanding()).toBe(true); + }); + + test("a clear names one obligation, so a concurrent claim cannot be deleted by it", async () => { + const mod = await import("../src/config/pending-teardown"); + // Review round 8 reproduced the delete-the-wrong-receipt bug; round 10 pointed out + // that a read-compare-unlink against ONE shared path is still racy, because the file + // can be replaced between the compare and the unlink. The nonce is the filename now, + // so the replacement is a DIFFERENT file and the delete cannot reach it — no ordering + // of the two operations matters. + const abandoned = mod.claimPendingTeardown(ENDPOINT, "exact", 1111); + const concurrent = mod.claimPendingTeardown(ENDPOINT, "exact", 2222); + expect(mod.listPendingTeardowns()).toHaveLength(2); + + expect(mod.clearPendingTeardown(abandoned.nonce)).toBe(true); + const survivors = mod.listPendingTeardowns(); + expect(survivors).toHaveLength(1); + expect(survivors[0]!.state === "valid" && survivors[0]!.receipt.nonce).toBe(concurrent.nonce); + }); + + test("clearing reports whether the obligation is actually gone", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + expect(mod.clearPendingTeardown(claimed.nonce)).toBe(true); + // Already gone is still "gone" — an idempotent discharge is not a failure. + expect(mod.clearPendingTeardown(claimed.nonce)).toBe(true); + // A receipt that cannot be removed must be reported, or recovery repeats forever. + const stuck = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + rmSync(mod.pendingTeardownPathFor(stuck.nonce)); + mkdirSync(mod.pendingTeardownPathFor(stuck.nonce), { recursive: true }); + mkdirSync(join(mod.pendingTeardownPathFor(stuck.nonce), "child"), { recursive: true }); + expect(mod.clearPendingTeardown(stuck.nonce)).toBe(false); + rmSync(mod.pendingTeardownPathFor(stuck.nonce), { recursive: true, force: true }); + }); + + test("an unreadable receipt is invalid, outstanding, and quarantinable", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + writeFileSync(mod.pendingTeardownPathFor(claimed.nonce), "{not json"); + const read = mod.readPendingTeardown(claimed.nonce); + expect(read.state).toBe("invalid"); + expect(mod.pendingTeardownOutstanding()).toBe(true); + // It names no endpoint, so nothing can prove its proxy down. Quarantine stops the + // recovery loop from re-reading garbage on every stop, but the obligation REMAINS + // outstanding: filing it away to unblock an update would let the next install land + // over a teardown that never ran. + const moved = mod.quarantinePendingTeardown(claimed.nonce); + expect(moved).toBeTruthy(); + expect(existsSync(moved!)).toBe(true); + expect(mod.listPendingTeardowns()).toHaveLength(0); + expect(mod.pendingTeardownOutstanding()).toBe(true); + expect(readdirSync(home).some(n => n.endsWith(".unreadable.json"))).toBe(true); + }); + + test("a directory where a receipt belongs is invalid, not missing", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + rmSync(mod.pendingTeardownPathFor(claimed.nonce)); + mkdirSync(mod.pendingTeardownPathFor(claimed.nonce), { recursive: true }); + // Reading that as absence hides an obligation that may still be outstanding. + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + expect(mod.pendingTeardownOutstanding()).toBe(true); + rmSync(mod.pendingTeardownPathFor(claimed.nonce), { recursive: true, force: true }); + }); + + test("a receipt whose body disagrees with its filename is invalid", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + writeFileSync( + mod.pendingTeardownPathFor(claimed.nonce), + JSON.stringify({ ownerPid: 1234, nonce: FOREIGN_NONCE, createdAt: "t", endpoint: ENDPOINT, endpointSource: "exact" }), + ); + // Otherwise an edited body could claim an identity the file name does not carry. + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + expect(mod.deferralMatchesReceipt(claimed.nonce)).toBe(false); + }); + + test("a receipt without a usable endpoint is invalid, because recovery could not locate it", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 1234); + const path = mod.pendingTeardownPathFor(claimed.nonce); + const base = { ownerPid: 7, nonce: claimed.nonce, createdAt: "t", endpointSource: "exact" }; + writeFileSync(path, JSON.stringify(base)); + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + writeFileSync(path, JSON.stringify({ ...base, endpoint: { hostname: "", port: 10100 } })); + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + writeFileSync(path, JSON.stringify({ ...base, endpoint: { hostname: "127.0.0.1", port: 0 } })); + expect(mod.readPendingTeardown(claimed.nonce).state).toBe("invalid"); + }); + + test("only an abandoned receipt is recoverable", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 4242); + const live = mod.readPendingTeardown(claimed.nonce); + + // A stop that is still running owns its own obligation; finishing it from here would + // restore client config while that stop is still deciding whether a proxy survived. + expect(mod.isPendingTeardownAbandoned(live, () => true, 1)).toBe(false); + // This process's own receipt is not "abandoned" either. + expect(mod.isPendingTeardownAbandoned(live, () => false, 4242)).toBe(false); + // A dead owner left the obligation behind: recover it. + expect(mod.isPendingTeardownAbandoned(live, () => false, 1)).toBe(true); + expect(mod.isPendingTeardownAbandoned({ state: "missing" }, () => false, 1)).toBe(false); + }); + + test("deferralMatchesReceipt needs a well-formed nonce that names a readable receipt", async () => { + const mod = await import("../src/config/pending-teardown"); + const claimed = mod.claimPendingTeardown(ENDPOINT, "exact", 7); + expect(mod.deferralMatchesReceipt(claimed.nonce)).toBe(true); + expect(mod.deferralMatchesReceipt(FOREIGN_NONCE)).toBe(false); + expect(mod.deferralMatchesReceipt(null)).toBe(false); + // A path-shaped "nonce" must not be able to reach outside the receipt namespace. + expect(mod.deferralMatchesReceipt("../config")).toBe(false); + expect(mod.deferralMatchesReceipt("")).toBe(false); + }); +}); diff --git a/tests/uninstall.test.ts b/tests/uninstall.test.ts index 8988ed3431..a0d233a1f4 100644 --- a/tests/uninstall.test.ts +++ b/tests/uninstall.test.ts @@ -95,9 +95,165 @@ describe("full uninstall command", () => { expect(uninstallBody).toContain('runStep("proxy stopped"'); expect(uninstallBody).toContain('runStep("service removed"'); expect(uninstallBody).toContain("await stopProxy(pid);"); - expect(uninstallBody).toContain("uninstallServiceIfInstalled()"); + expect(uninstallBody).toContain("uninstallServiceDetailed()"); expect(uninstallBody.indexOf('runStep("service stopped"')).toBeLessThan(uninstallBody.indexOf('runStep("proxy stopped"')); expect(uninstallBody.indexOf('runStep("proxy stopped"')).toBeLessThan(uninstallBody.indexOf('runStep("service removed"')); - expect(uninstallBody.indexOf("await stopProxy(pid);")).toBeLessThan(uninstallBody.indexOf("uninstallServiceIfInstalled()")); + expect(uninstallBody.indexOf("await stopProxy(pid);")).toBeLessThan(uninstallBody.indexOf("uninstallServiceDetailed()")); }); }); +describe("uninstall gates shared teardown on a proven service stop", () => { + test("the authorization rule, exercised for every failure permutation", async () => { + const { sharedTeardownAuthorized } = await import("../src/cli/uninstall-plan"); + const base = { + serviceStop: "stopped" as const, + proxyProvenDown: true, + serviceRemoval: "removed" as const, + respawnWindowVerified: false, + }; + expect(sharedTeardownAuthorized(base)).toBe(true); + expect(sharedTeardownAuthorized({ ...base, serviceStop: "absent" })).toBe(true); + expect(sharedTeardownAuthorized({ ...base, serviceRemoval: "absent" })).toBe(true); + // Removing the registration does not prove an already-running wrapper died; killing it + // is best-effort (#764), so the restart window has to be polled first. + expect(sharedTeardownAuthorized({ ...base, serviceStop: "stopped-respawnable" })).toBe(false); + expect(sharedTeardownAuthorized({ ...base, serviceStop: "stopped-respawnable", respawnWindowVerified: true })).toBe(true); + // A manager that refused to stop, or one we could not read, may still be running. + expect(sharedTeardownAuthorized({ ...base, serviceStop: "failed" })).toBe(false); + expect(sharedTeardownAuthorized({ ...base, serviceStop: "state-unknown" })).toBe(false); + // The step itself threw: we know nothing. + expect(sharedTeardownAuthorized({ ...base, serviceStop: null })).toBe(false); + // A proxy that could not be PROVEN down — a live orphan with no pid, or an endpoint + // that would not answer — blocks it. A findLiveProxy miss is not proof. + expect(sharedTeardownAuthorized({ ...base, proxyProvenDown: false })).toBe(false); + // A removal that failed used to look like absence on darwin and linux. + expect(sharedTeardownAuthorized({ ...base, serviceRemoval: "failed" })).toBe(false); + expect(sharedTeardownAuthorized({ ...base, serviceRemoval: null })).toBe(false); + }); + + test("a removal failure is distinguishable from nothing being installed", async () => { + const { setUninstallServiceHooksForTests, uninstallServiceDetailed } = await import("../src/service"); + // Windows is the platform whose hooks are injectable; the darwin/linux catch arms that + // returned the same false as absence are now typed outcomes rather than a boolean. + setUninstallServiceHooksForTests({ + platform: "win32", + assertEnvironment: () => {}, + probeWindowsTask: () => ({ status: "absent" }) as never, + uninstallWindowsTask: () => {}, + nativeStatus: () => "nonexistent", + uninstallNative: () => {}, + removeInstallState: () => {}, + } as never); + expect(uninstallServiceDetailed()).toBe("absent"); + + const serviceSource = await readText("src/service.ts"); + // The darwin and linux arms return "failed", not the absence value. + expect(serviceSource).toContain('try { uninstallLaunchd(); removeServiceInstallState(); return "removed"; } catch { return "failed"; }'); + expect(serviceSource).toContain('try { unlinkSync(unitPath()); removeServiceInstallState(); return "removed"; } catch { return "failed"; }'); + }); + + test("a live orphan with no pid file blocks the teardown", async () => { + const cli = await readText("src/cli/index.ts"); + const at = cli.indexOf("async function handleUninstall("); + const fn = cli.slice(at, at + 9000); + // A missing pid file is not proof that nothing is serving — the same discovery + // `ocx stop` performs. Without it, uninstall restored shared config under a live proxy. + expect(fn).toContain("const live = await findLiveProxy();"); + expect(fn).toContain("observed.proxyProvenDown = await proxyEndpointProvenDown();"); + expect(fn).toContain("no process id could be resolved for it"); + // The orphan-with-no-pid branch THROWS, so `proxyProvenDown` stays false and the + // authorization rule refuses the shared teardown. + const orphanBranch = fn.slice(fn.indexOf("const live = await findLiveProxy();"), fn.indexOf("const live = await findLiveProxy();") + 600); + expect(orphanBranch).toContain("throw new Error("); + // A findLiveProxy miss is not proof either: it goes through the tri-state probe first. + expect(orphanBranch).toContain("could not be confirmed down either"); + }); + + async function uninstallFn(): Promise { + const cli = await readText("src/cli/index.ts"); + const at = cli.indexOf("async function handleUninstall("); + expect(at).toBeGreaterThan(-1); + return cli.slice(at, at + 9000); + } + + test("the detailed outcome is consumed, not the boolean collapse", async () => { + const fn = await uninstallFn(); + // stopServiceIfInstalled returns false for "not installed", "refused to stop" and + // "state could not be read" alike, so this step reported "not installed" for a manager + // that might still be running (#3008). + expect(fn).toContain("stopServiceIfInstalledDetailed()"); + expect(fn).not.toContain("stopServiceIfInstalled()"); + expect(fn).toContain('if (outcome === "absent") return false;'); + expect(fn).toContain('if (outcome === "failed")'); + expect(fn).toContain('if (outcome === "state-unknown")'); + }); + + test("shared teardown runs only when nothing that could still serve is unaccounted for", async () => { + const fn = await uninstallFn(); + // The rule itself is exercised by calling it above; this pins the wiring. + expect(fn).toContain("if (sharedTeardownAuthorized(observed)) {"); + // Every step that could leave something serving records what it observed, and the + // fields start pessimistic so a step that throws cannot look like a success. + expect(fn).toContain("serviceStop: null,"); + expect(fn).toContain("proxyProvenDown: false,"); + expect(fn).toContain("serviceRemoval: null,"); + expect(fn).toContain("respawnWindowVerified: false,"); + expect(fn).toContain("observed.serviceStop = outcome;"); + expect(fn).toContain("observed.serviceRemoval = outcome;"); + expect(fn).toContain('if (observed.serviceStop === "stopped-respawnable")'); + expect(fn).toContain("observed.respawnWindowVerified = true;"); + const gateAt = fn.indexOf("if (sharedTeardownAuthorized(observed)) {"); + expect(gateAt).toBeLessThan(fn.indexOf("native Codex restored", gateAt)); + // The skip is a failure, not a silent pass: the command must exit nonzero and say what + // to run once the blocker is resolved. + expect(fn).toContain('failures.push("native Codex restored", "Grok Build config restored");'); + expect(fn).toContain("Skipping shared teardown"); + // Naming only `ocx restore` was wrong: it restores client routing but leaves the + // service removal and local cleanup this command had not reached. + expect(fn).toContain("rerun 'ocx uninstall'"); + expect(fn).toContain("interim step"); + }); +}); + test("proof covers every distinct endpoint, not just the preferred one", async () => { + const { endpointsToProve, everyEndpointProvenDown } = await import("../src/cli/uninstall-plan"); + + // A stale runtime record pointing at a closed port, and the live proxy on the + // configured one. Probing only the runtime candidate reports "dead" for a port nobody + // is using and authorizes the teardown (#3008). + const endpoints = endpointsToProve({ port: 10999, hostname: "127.0.0.1" }, { port: 10100, hostname: "127.0.0.1" }); + expect(endpoints).toEqual([ + { hostname: "127.0.0.1", port: 10999 }, + { hostname: "127.0.0.1", port: 10100 }, + ]); + const closedRuntimeLiveConfig = (e: { port: number }) => (e.port === 10999 ? "dead" as const : "live" as const); + expect(everyEndpointProvenDown(endpoints, closedRuntimeLiveConfig)).toBe(false); + // A silent listener is not absence either. + expect(everyEndpointProvenDown(endpoints, e => (e.port === 10999 ? "dead" : "unknown"))).toBe(false); + // Both definitively dead is the only proof. + expect(everyEndpointProvenDown(endpoints, () => "dead")).toBe(true); + + // Identical candidates collapse to one; a missing runtime record leaves the config one. + expect(endpointsToProve({ port: 10100, hostname: "127.0.0.1" }, { port: 10100 })).toHaveLength(1); + expect(endpointsToProve(null, { port: 10100 })).toEqual([{ hostname: "127.0.0.1", port: 10100 }]); + // No configured port still yields the default, so the set is never empty in practice. + expect(endpointsToProve(null, {})).toEqual([{ hostname: "127.0.0.1", port: 10100 }]); + // An empty set is not proof of anything. + expect(everyEndpointProvenDown([], () => "dead")).toBe(false); + // A nonsense runtime port is skipped rather than probed. + expect(endpointsToProve({ port: 0 }, { port: 10100 })).toEqual([{ hostname: "127.0.0.1", port: 10100 }]); + }); + + test("the respawn window is verified by evidence, not by a silent poll", async () => { + const cli = await readText("src/cli/index.ts"); + const at = cli.indexOf("async function handleUninstall("); + const fn = cli.slice(at, at + 9000); + // proxyStillLiveAfterStop returns null on a timeout as well as on a genuinely dead + // endpoint, so a respawned-but-unresponsive proxy looked verified-down. + const windowStep = fn.slice(fn.indexOf('runStep("respawn window verified"'), fn.indexOf('runStep("respawn window verified"') + 900); + expect(windowStep).toContain("if (!await proxyEndpointProvenDown())"); + expect(windowStep).toContain("could not be confirmed down either"); + expect(windowStep.indexOf("if (!await proxyEndpointProvenDown())")) + .toBeLessThan(windowStep.indexOf("observed.respawnWindowVerified = true;")); + // And the proof itself asks every candidate. + expect(fn).toContain("endpointsToProve(readRuntimePort(), loadConfig())"); + expect(fn).toContain("everyEndpointProvenDown(endpoints, e => probeProxyLiveness(e.port, e.hostname))"); + }); diff --git a/tests/update-stop-classification.test.ts b/tests/update-stop-classification.test.ts new file mode 100644 index 0000000000..d9d4c73349 --- /dev/null +++ b/tests/update-stop-classification.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, test } from "bun:test"; +import { spawn } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { STOP_HISTORY_INCOMPLETE_EXIT_CODE } from "../src/update/stop-contract.mjs"; +import { probeProxyLiveness } from "../src/update/proxy-liveness-probe.mjs"; +import { decidePostStopUpdate } from "../src/update/stop-decision.mjs"; + +const repoRoot = join(import.meta.dir, ".."); +const read = (rel: string): string => readFileSync(join(repoRoot, rel), "utf8"); + +/** + * #3008: `ocx update` aborted after a stop that had already succeeded. + * + * `handleStop` sets a failure code AFTER history restoration — that is, after the proxy + * and service are already down — so a failed Codex-history cleanup was indistinguishable + * from a proxy that refused to die. The update aborted with the service stopped, no + * listener, and the old package still installed. + * + * The distinguishing signal has to survive `spawnSync`, so it is an exit code rather than + * a type. These assertions pin the contract at both ends of that process boundary, and the + * decision table each end implements. + */ +describe("stop failure classification (#3008)", () => { + test("the history-only code is outside every code this CLI already uses", () => { + // Picking an occupied code would make a history-only stop indistinguishable from + // whatever else emits it, and `bin/ocx.mjs` mirrors the child's status faithfully + // enough to propagate the confusion. + expect(STOP_HISTORY_INCOMPLETE_EXIT_CODE).toBe(79); + // sysexits.h occupies 64-78; 128+signal starts at 129. + expect(STOP_HISTORY_INCOMPLETE_EXIT_CODE).toBeGreaterThan(78); + expect(STOP_HISTORY_INCOMPLETE_EXIT_CODE).toBeLessThan(128); + + const cliCodes = [...read("src/cli/index.ts").matchAll(/process\.exit(?:Code)?\s*(?:=|\()\s*(\d+)/g)] + .map(match => Number(match[1])); + const dispatchCodes = [...read("src/cli/dispatch.ts").matchAll(/return (\d+);/g)] + .map(match => Number(match[1])); + expect(cliCodes).not.toContain(STOP_HISTORY_INCOMPLETE_EXIT_CODE); + expect(dispatchCodes).not.toContain(STOP_HISTORY_INCOMPLETE_EXIT_CODE); + }); + + test("the shared contract is plain ESM so the Node launcher can import it", () => { + // A .ts module would be unusable from bin/ocx.mjs, and inlining the number in two + // places is how the two ends drift. + const contract = read("src/update/stop-contract.mjs"); + expect(contract).toContain("export const STOP_HISTORY_INCOMPLETE_EXIT_CODE"); + expect(read("bin/ocx.mjs")).toContain("stop-contract.mjs"); + expect(read("src/update/index.ts")).toContain("stop-contract.mjs"); + }); + + test("the liveness probe sees a surviving proxy, and fails open when nothing is there", async () => { + // Behavioural, not textual: absent PID and runtime files are weak evidence, so the + // updaters ask the endpoint. The listener runs in a SEPARATE process because the probe + // uses spawnSync - an in-process server could never answer while the parent's event + // loop is blocked, which is also why the probe speaks node:http rather than fetch. + const listener = spawn(process.execPath, ["-e", [ + "const http = require('node:http');", + "const server = http.createServer((req, res) => {", + " if (req.url !== '/healthz') { res.writeHead(404); res.end(); return; }", + " res.writeHead(200, { 'content-type': 'application/json' });", + " res.end(JSON.stringify({ service: 'opencodex', pid: process.pid, version: 'test' }));", + "});", + "server.listen(0, '127.0.0.1', () => process.stdout.write(String(server.address().port)));", + ].join("\n")], { stdio: ["ignore", "pipe", "ignore"] }); + + const port = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("listener did not report a port")), 10_000); + listener.stdout.once("data", chunk => { clearTimeout(timer); resolve(Number(String(chunk))); }); + listener.once("error", error => { clearTimeout(timer); reject(error); }); + }); + + try { + expect(probeProxyLiveness(port)).toBe("live"); + } finally { + listener.kill(); + await new Promise(resolve => listener.once("exit", () => resolve())); + } + + // A refused connection is the one error that proves nothing is listening. + expect(probeProxyLiveness(port)).toBe("dead"); + // Nothing to ask is not ambiguity. + expect(probeProxyLiveness(0)).toBe("dead"); + expect(probeProxyLiveness(Number.NaN)).toBe("dead"); + }); + + test("identity decides live, and an unexpected status is unknown", async () => { + // Mirrors isOpencodexHealthz: a foreign server exposing /healthz is not our proxy, a + // pre-identity build of ours is, and any status other than 200 says the endpoint is + // answering without telling us what it is - which is not evidence of absence. + const cases: Array<[string, string, "live" | "dead" | "unknown"]> = [ + ["canonical", "{ service: 'opencodex', pid: 1 }", "live"], + ["legacy pre-identity", "{ status: 'ok', version: '2.0.0', uptime: 12 }", "live"], + ["foreign", "{ service: 'other', status: 'ok' }", "dead"], + ["foreign lookalike", "{ status: 'ok' }", "dead"], + ]; + for (const [name, body, expected] of cases) { + const listener = spawn(process.execPath, ["-e", [ + "const http = require('node:http');", + "const server = http.createServer((req, res) => {", + " res.writeHead(200, { 'content-type': 'application/json' });", + ` res.end(JSON.stringify(${body}));`, + "});", + "server.listen(0, '127.0.0.1', () => process.stdout.write(String(server.address().port)));", + ].join("\n")], { stdio: ["ignore", "pipe", "ignore"] }); + const port = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error(`${name} listener did not report a port`)), 10_000); + listener.stdout.once("data", chunk => { clearTimeout(timer); resolve(Number(String(chunk))); }); + listener.once("error", error => { clearTimeout(timer); reject(error); }); + }); + try { + expect(probeProxyLiveness(port)).toBe(expected); + } finally { + listener.kill(); + await new Promise(resolve => listener.once("exit", () => resolve())); + } + } + }); + + test("a non-200 from our own endpoint is unknown, never dead", async () => { + const listener = spawn(process.execPath, ["-e", [ + "const http = require('node:http');", + "const server = http.createServer((req, res) => {", + " res.writeHead(500, { 'content-type': 'application/json' });", + " res.end(JSON.stringify({ service: 'opencodex' }));", + "});", + "server.listen(0, '127.0.0.1', () => process.stdout.write(String(server.address().port)));", + ].join("\n")], { stdio: ["ignore", "pipe", "ignore"] }); + const port = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("listener did not report a port")), 10_000); + listener.stdout.once("data", chunk => { clearTimeout(timer); resolve(Number(String(chunk))); }); + listener.once("error", error => { clearTimeout(timer); reject(error); }); + }); + try { + expect(probeProxyLiveness(port)).toBe("unknown"); + } finally { + listener.kill(); + await new Promise(resolve => listener.once("exit", () => resolve())); + } + }); + + test("the shared probe normalizes wildcard and bracketed IPv6 hosts", async () => { + // Normalization lives in the probe, not at each call site: doing it per-lane fixed + // the npm launcher and left the TypeScript updater passing a bracketed literal + // straight to node:http, which answers nothing - read as "unknown", which aborts a + // healthy update and leaves the service down. That is the original failure shape. + const listener = spawn(process.execPath, ["-e", [ + "const http = require('node:http');", + "const server = http.createServer((req, res) => {", + " res.writeHead(200, { 'content-type': 'application/json' });", + " res.end(JSON.stringify({ service: 'opencodex', pid: process.pid }));", + "});", + "server.listen(0, () => process.stdout.write(String(server.address().port)));", + ].join("\n")], { stdio: ["ignore", "pipe", "ignore"] }); + const port = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("listener did not report a port")), 10_000); + listener.stdout.once("data", chunk => { clearTimeout(timer); resolve(Number(String(chunk))); }); + listener.once("error", error => { clearTimeout(timer); reject(error); }); + }); + + try { + // A wildcard bind cannot be dialled as a wildcard; it answers on loopback. + expect(probeProxyLiveness(port, "0.0.0.0")).toBe("live"); + expect(probeProxyLiveness(port, "*")).toBe("live"); + // A URL-spelled literal is unwrapped rather than handed to the socket layer. + expect(probeProxyLiveness(port, "[127.0.0.1]")).toBe("live"); + // Empty falls back to loopback rather than dialling "". + expect(probeProxyLiveness(port, "")).toBe("live"); + } finally { + listener.kill(); + await new Promise(resolve => listener.once("exit", () => resolve())); + } + }); + + test("an unreachable or silent endpoint is unknown, not dead", async () => { + // Fail-open was the wrong default: a listener that accepts connections but withholds + // /healthz, or a probe that times out, is exactly the state where replacing package + // files is most dangerous. "We could not tell" is not evidence the proxy is gone. + const listener = spawn(process.execPath, ["-e", [ + "const net = require('node:net');", + // Accepts the connection and never answers, so the request times out. + "const server = net.createServer(() => {});", + "server.listen(0, '127.0.0.1', () => process.stdout.write(String(server.address().port)));", + ].join("\n")], { stdio: ["ignore", "pipe", "ignore"] }); + + const port = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("listener did not report a port")), 10_000); + listener.stdout.once("data", chunk => { clearTimeout(timer); resolve(Number(String(chunk))); }); + listener.once("error", error => { clearTimeout(timer); reject(error); }); + }); + + try { + expect(probeProxyLiveness(port, "127.0.0.1", 400)).toBe("unknown"); + } finally { + listener.kill(); + await new Promise(resolve => listener.once("exit", () => resolve())); + } + }); + + test("the shared decision covers the whole post-stop matrix", () => { + // This is THE predicate both updaters call, not a copy of it: src/update/index.ts and + // bin/ocx.mjs each import decidePostStopUpdate. Testing a local reimplementation would + // stay green while either lane drifted, which is how #3008 shipped fixed on one side. + const dead = { hasRuntimeState: false, liveness: "dead" } as const; + + // Proceed: a clean stop, or the history-only code with everything else quiet. + expect(decidePostStopUpdate({ status: 0, ...dead })).toEqual({ proceed: true, reason: "ok" }); + expect(decidePostStopUpdate({ status: STOP_HISTORY_INCOMPLETE_EXIT_CODE, ...dead })) + .toEqual({ proceed: true, reason: "history-only" }); + + // Abort: a stop that did not finish. A signal kill carries no evidence that it did. + for (const status of [1, 2, 4, 64, 130, null]) { + expect(decidePostStopUpdate({ status, ...dead })) + .toEqual({ proceed: false, reason: "stop-failed" }); + } + + // Abort: records survived the stop, even on a clean exit. + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: true, liveness: "dead" })) + .toEqual({ proceed: false, reason: "runtime-state" }); + + // Abort: something still answers as our proxy, or the probe could not tell. Absence of + // proof is not proof of absence when the cost is a server running mixed modules. + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: false, liveness: "live" })) + .toEqual({ proceed: false, reason: "proxy-live" }); + expect(decidePostStopUpdate({ status: 0, hasRuntimeState: false, liveness: "unknown" })) + .toEqual({ proceed: false, reason: "proxy-unknown" }); + // The history-only code does not buy past a live or unclear proxy either. + expect(decidePostStopUpdate({ status: STOP_HISTORY_INCOMPLETE_EXIT_CODE, hasRuntimeState: false, liveness: "unknown" })) + .toEqual({ proceed: false, reason: "proxy-unknown" }); + }); + + test("both updater lanes call the shared decision", () => { + // The reported path is a dashboard npm update through the plain-Node launcher. Fixing + // only the Bun updater would leave that lane broken while every focused test went + // green, which is exactly how #3008 reached a release. + for (const lane of ["src/update/index.ts", "bin/ocx.mjs"]) { + const source = read(lane); + expect(source).toContain("decidePostStopUpdate({"); + // And neither lane keeps a private copy of the rule it was supposed to delegate. + expect(source).not.toMatch(/status !== 0 && !historyOnlyStop/); + } + }); + + test("handleStop emits the code only for a history-only failure and still returns", () => { + const cli = read("src/cli/index.ts"); + // Ordinary failure wins: it is the stronger signal. + expect(cli).toMatch(/if \(stopFailed\) process\.exitCode = 1;\s*\n\s*else if \(historyOnlyFailure\) process\.exitCode = STOP_HISTORY_INCOMPLETE_EXIT_CODE;/); + // The code is set rather than exited inline so the dispatcher still receives the + // return value and decides what happens next. + expect(cli).toMatch(/process\.exitCode = STOP_HISTORY_INCOMPLETE_EXIT_CODE;\s*\n\s*return !stopFailed;/); + // Config and catalog failures are real teardown failures: a client reads those. + expect(cli).toMatch(/artifacts\.config\.state === "failed" \|\| artifacts\.catalog\.state === "failed"/); + }); +}); diff --git a/tests/update-stop-first.test.ts b/tests/update-stop-first.test.ts index 0f7fd7ff55..d20eafb5c7 100644 --- a/tests/update-stop-first.test.ts +++ b/tests/update-stop-first.test.ts @@ -94,7 +94,7 @@ describe("update stops the running proxy before replacing files", () => { expect(stopAt).toBeGreaterThan(-1); expect(updateAt).toBeGreaterThan(-1); expect(stopAt).toBeLessThan(updateAt); - expect(updateSource).toContain("if (serviceWasInstalled || readPid() || readRuntimePort())"); + expect(updateSource).toContain("if (serviceWasInstalled || readPid() || readRuntimePort() || pendingTeardownOutstanding())"); }); test("integrity pre-flight runs BEFORE the stop so anomalous metadata never unloads the proxy", () => { @@ -306,10 +306,12 @@ esac // may claim a DB lock or that every routed thread is hidden. expect(updateSource).toContain("export function historyRestoreIncomplete("); expect(updateSource).toContain('name.startsWith("codex-history-backup-") && name.endsWith(".json")'); - expect(updateSource).toContain("if (historyRestoreIncomplete())"); + // The warning now also fires on the dedicated stop code, so the manifest check is one + // of two triggers rather than the whole condition (#3008). + expect(updateSource).toContain("if (historyOnlyStop || historyRestoreIncomplete())"); expect(launcherSource).toContain("function historyRestoreIncomplete()"); expect(launcherSource).toContain('name.startsWith("codex-history-backup-") && name.endsWith(".json")'); - expect(launcherSource).toContain("if (historyRestoreIncomplete())"); + expect(launcherSource).toContain("if (historyOnlyStop || historyRestoreIncomplete())"); const warnAt = launcherSource.indexOf("Codex resume-history metadata restore is incomplete"); const installAt = launcherSource.indexOf("transactionalNpmUpdate({"); expect(warnAt).toBeGreaterThan(-1); @@ -320,9 +322,16 @@ esac }); test("the stop gate covers service-managed and orphaned proxies whose pid file is stale/missing", () => { - expect(updateSource).toContain("if (serviceWasInstalled || readPid() || readRuntimePort())"); - expect(launcherSource).toContain("if (serviceWasInstalled || hasRuntimeState)"); - expect(launcherSource).toContain("stopRes.status !== 0 || stillHasRuntimeState"); + // A pending-teardown receipt is a fourth reason to stop: after a parent crashed + // mid-deferral the service, pid and runtime records can all be absent while shared + // client config still points at a proxy that is gone (#3008). + expect(updateSource).toContain("if (serviceWasInstalled || readPid() || readRuntimePort() || pendingTeardownOutstanding())"); + expect(launcherSource).toContain("if (serviceWasInstalled || hasRuntimeState || hasPendingTeardown)"); + // The rule now lives in the shared post-stop decision both lanes import (#3008): a + // history-only stop proceeds, every other nonzero status and any surviving runtime + // state aborts. Pinned by tests/update-stop-classification.test.ts. + expect(launcherSource).toContain("decidePostStopUpdate({"); + expect(launcherSource).toContain("hasRuntimeState: stillHasRuntimeState"); }); test("GUI worker update children use pipe stdio so background updates do not open consoles", () => {