-
Notifications
You must be signed in to change notification settings - Fork 500
Codex/app server restart #802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,14 +142,21 @@ opencodex local config only if all restore steps succeeded. `remove` is an alias | |
|
|
||
| ## Models & Codex | ||
|
|
||
| ### `ocx sync` | ||
| ### `ocx sync [--restart-codex]` | ||
|
|
||
| Fetch the live model list from every configured provider and re-inject the merged catalog into Codex. | ||
| Run it after adding a provider or to refresh available models. | ||
|
|
||
| ### `ocx sync-cache` | ||
| If long-lived Codex `app-server` processes are still running, `ocx sync` warns that they may keep | ||
| serving the previous in-memory model list even though `opencodex-catalog.json` / `models_cache.json` | ||
| were updated. Pass `--restart-codex` to send `SIGTERM` only to matching `codex … app-server` and | ||
| `codex-code-mode-host` processes owned by the current user (active turns may be interrupted). Broad | ||
| `pkill -f codex` matching is intentionally avoided. | ||
|
|
||
| Invalidate Codex's local model picker cache so it is rebuilt from the active opencodex catalog. | ||
| ### `ocx sync-cache [--restart-codex]` | ||
|
|
||
| Invalidate Codex's local model picker cache so it is rebuilt from the active opencodex catalog. The | ||
| same stale-`app-server` warning and optional `--restart-codex` behavior as `ocx sync` apply. | ||
|
Comment on lines
+145
to
+159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 'SIGTERM|process\.platform|win32|app-server|code-mode-host' \
src/codex/app-server-processes.ts src/cli/index.tsRepository: lidge-jun/opencodex Length of output: 25366 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- process discovery implementation ---'
sed -n '232,370p' src/codex/app-server-processes.ts
printf '%s\n' '--- platform-aware matching and restart flow ---'
sed -n '367,515p' src/codex/app-server-processes.ts
printf '%s\n' '--- related tests and documentation references ---'
rg -n -C 4 'listWindowsSnapshots|restartCodexAppServers|afterCatalogWriteHandleAppServers|restart-codex|sync-cache' \
test tests src docs-site 2>/dev/null || true
printf '%s\n' '--- locale CLI documentation files ---'
find docs-site/src/content/docs -type f -path '*/cli.md' -print 2>/dev/null | sortRepository: lidge-jun/opencodex Length of output: 49661 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
paths = [
Path("docs-site/src/content/docs/reference/cli.md"),
Path("docs-site/src/content/docs/ja/reference/cli.md"),
Path("docs-site/src/content/docs/ko/reference/cli.md"),
Path("docs-site/src/content/docs/ru/reference/cli.md"),
Path("docs-site/src/content/docs/zh-cn/reference/cli.md"),
]
for path in paths:
lines = path.read_text(encoding="utf-8").splitlines()
starts = [i for i, line in enumerate(lines) if line.startswith("### `ocx sync")]
print(f"--- {path} ---")
for start in starts:
end = next((i for i in range(start + 1, len(lines))
if lines[i].startswith("### ")), len(lines))
block = lines[start:end]
print(f"lines {start + 1}-{end}:")
print("\n".join(block))
print("has_restart_flag=", any("--restart-codex" in line for line in block))
print("has_stale_warning=", any("app-server" in line or "app-server" in line.lower() for line in block))
PY
printf '%s\n' '--- platform dispatch and signal call ---'
rg -n -C 3 'defaultListSnapshots|platform === "win32"|process\.kill\(pid, signal\)|powershell\.exe|GetOwner' \
src/codex/app-server-processes.tsRepository: lidge-jun/opencodex Length of output: 5886 🌐 Web query:
💡 Result: On Windows, POSIX signals are not supported, meaning there is no direct equivalent to the termination behavior triggered by signals on non-Windows platforms [1][2]. However, Node.js provides emulation for specific signals when using process.kill or subprocess.kill [1][2]. When you use process.kill(pid, 'SIGTERM') or subprocess.kill('SIGTERM') on Windows: 1. Forced Termination: The signal is treated as an instruction for unconditional and abrupt termination [1][2]. The process is killed immediately, similar to how it would be handled by 'SIGKILL' [2]. 2. Signal Reporting: Despite the forced nature of the termination, Node.js emulates the behavior for child processes, meaning the subprocess will correctly report that it was terminated by 'SIGTERM' [1][2]. 3. No Graceful Shutdown: Because Windows does not support POSIX signals, a Node.js process on Windows cannot listen for or handle 'SIGTERM' to perform a graceful shutdown [1][3]. While you can register a listener for 'SIGTERM' in your code, it will not be triggered when the process is killed in this manner [1][4]. In summary, 'SIGTERM' on Windows acts as a forced kill rather than a request for graceful exit [1][2]. This behavior applies specifically to 'SIGINT', 'SIGTERM', 'SIGQUIT', and 'SIGKILL', which are the signals supported by the Node.js emulation layer on Windows [2][5]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [
Path("docs-site/src/content/docs/reference/cli.md"),
Path("docs-site/src/content/docs/ja/reference/cli.md"),
Path("docs-site/src/content/docs/ko/reference/cli.md"),
Path("docs-site/src/content/docs/ru/reference/cli.md"),
Path("docs-site/src/content/docs/zh-cn/reference/cli.md"),
]
for path in paths:
lines = path.read_text(encoding="utf-8").splitlines()
starts = [i for i, line in enumerate(lines) if line.startswith("### `ocx sync")]
print(f"--- {path} ---")
for start in starts:
end = next(
(i for i in range(start + 1, len(lines)) if lines[i].startswith("### ")),
len(lines),
)
block = lines[start:end]
print(f"lines {start + 1}-{end}:")
print("\n".join(block))
print("has_restart_flag=", any("--restart-codex" in line for line in block))
print("has_stale_warning=", any("app-server" in line.lower() for line in block))
PY
printf '%s\n' '--- platform dispatch and signal call ---'
rg -n -C 3 'defaultListSnapshots|platform === "win32"|process\.kill\(pid, signal\)|powershell\.exe|GetOwner' \
src/codex/app-server-processes.tsRepository: lidge-jun/opencodex Length of output: 5886 Update the translated CLI pages.
🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| ### `ocx v2 [subcommand]` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -199,7 +199,7 @@ export const en = { | |
| "dash.syncModels": "Sync models", | ||
| "dash.syncing": "Syncing…", | ||
| "dash.syncOk": "Sync complete. {count} model(s) appended.", | ||
| "dash.syncStaleHint": "If Codex App still shows an older list, restart its long-lived app-server process.", | ||
| "dash.syncStaleHint": "If Codex still shows an older list, restart its long-lived app-server ({cmd}).", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Remove the conflicting “no restart is needed” guidance. This new message says that a long-lived app-server can retain an old model list. However, Update the model-page copy to state that cache invalidation normally avoids a restart, but a stale app-server requires the restart command. Apply the same correction to the other locale files. As per path instructions, translated locale pages must not contradict the English source. 🤖 Prompt for AI AgentsSource: Path instructions |
||
| "dash.syncFailed": "Sync failed: {error}", | ||
| "dash.projectConfigTitle": "Project Codex config bypasses OpenCodex", | ||
| "dash.projectConfigHint": "These repo-local settings override the OpenCodex proxy (e.g. route to OpenCode Go directly). Remove them so ~/.codex/config.toml routing applies in that project.", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,21 +94,6 @@ async function waitForProxy(timeoutMs = 8_000): Promise<LiveProxy | null> { | |
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * A Grok fence sync that throws is best-effort by design — it must never block startup. | ||
| * Reporting nothing, however, is what lets a STALE fence survive: `~/.grok/config.toml` | ||
| * keeps naming whatever port the last successful sync wrote, and once that listener is | ||
| * gone every grok turn retries against a refused connection while our own log stays | ||
| * silent (2026-07-27 field report: 8 entries pinned to a dead 127.0.0.1:4179). | ||
| * So say what failed and name the single command that repairs it. | ||
| */ | ||
| function grokSyncFailureMessage(err: unknown): string { | ||
| const detail = err instanceof Error ? err.message : String(err); | ||
| return `Grok Build config sync failed: ${detail}. ` | ||
| + "~/.grok/config.toml may still point at a previous proxy port — " | ||
| + "run 'ocx ensure' (or apply from the dashboard's Grok page) to repoint it."; | ||
| } | ||
|
|
||
| /** Argv for detached `start`, optionally hard-pinning the listen port. */ | ||
| function startArgv(port?: number): string[] { | ||
| const args = [process.argv[1], "start"]; | ||
|
|
@@ -314,14 +299,7 @@ async function handleStart(options: { block?: boolean } = {}) { | |
| const r = await syncGrokConfig(port, config, config.hostname ? { hostname: config.hostname } : {}); | ||
| if (r.changed) console.log(" + Grok Build config updated (~/.grok/config.toml)"); | ||
| else if (!r.ok) console.error(`⚠️ ${r.message}`); | ||
| } catch (err) { | ||
| // Best-effort: grok integration must never block startup. But swallowing the error | ||
| // silently is how a stale fence survives unnoticed — ~/.grok/config.toml keeps | ||
| // pointing at whatever port the LAST successful sync wrote, and if that listener is | ||
| // gone every grok turn retries against a refused connection with nothing in our log | ||
| // to explain it. Name the failure and the one command that repairs it. | ||
| console.error(`⚠️ ${grokSyncFailureMessage(err)}`); | ||
| } | ||
| } catch { /* best-effort — grok integration must never block startup */ } | ||
| if (options.block ?? true) { | ||
| setInterval(() => {}, 60_000); | ||
| await new Promise<void>(() => {}); | ||
|
|
@@ -349,7 +327,7 @@ async function handleEnsure() { | |
| const g = await syncGrokConfig(live.port, config, live.hostname ? { hostname: live.hostname } : {}); | ||
| if (g.changed) console.log(" + Grok Build config updated (~/.grok/config.toml)"); | ||
| else if (!g.ok) console.error(`⚠️ ${g.message}`); | ||
| } catch (err) { console.error(`⚠️ ${grokSyncFailureMessage(err)}`); } | ||
| } catch { /* best-effort */ } | ||
| console.log(`✅ Proxy running on port ${live.port}`); | ||
| return; | ||
| } | ||
|
|
@@ -376,7 +354,7 @@ async function handleEnsure() { | |
| const g = await syncGrokConfig(port, config, config.hostname ? { hostname: config.hostname } : {}); | ||
| if (g.changed) console.log(" + Grok Build config updated (~/.grok/config.toml)"); | ||
| else if (!g.ok) console.error(`⚠️ ${g.message}`); | ||
| } catch (err) { console.error(`⚠️ ${grokSyncFailureMessage(err)}`); } | ||
| } catch { /* best-effort */ } | ||
| // Always sync the LIVE port: after a fallback-port start, config.port still names the | ||
| // busy preferred port — syncing that would point Codex at a dead listener. | ||
| await syncModelsToCodex(port).catch(e => { | ||
|
|
@@ -766,7 +744,13 @@ switch (command) { | |
| break; | ||
| } | ||
| case "sync": { | ||
| await syncModelsToCodex((await findLiveProxy())?.port); | ||
| const restartCodex = args.slice(1).includes("--restart-codex"); | ||
| const syncResult = await syncModelsToCodex((await findLiveProxy())?.port); | ||
| // Only warn/restart when a catalog or models_cache write actually happened. | ||
| if (syncResult.catalogWritten || syncResult.cacheSynced) { | ||
| const { afterCatalogWriteHandleAppServers } = await import("../codex/app-server-processes"); | ||
| afterCatalogWriteHandleAppServers({ restart: restartCodex, log: console }); | ||
| } | ||
| break; | ||
| } | ||
| case "v2": { | ||
|
|
@@ -775,8 +759,13 @@ switch (command) { | |
| break; | ||
| } | ||
| case "sync-cache": { | ||
| const restartCodex = args.slice(1).includes("--restart-codex"); | ||
| const { invalidateCodexModelsCache } = await import("../codex/catalog"); | ||
| invalidateCodexModelsCache(); | ||
| // Only warn/restart when models_cache was actually rewritten from a readable catalog. | ||
| if (invalidateCodexModelsCache()) { | ||
| const { afterCatalogWriteHandleAppServers } = await import("../codex/app-server-processes"); | ||
| afterCatalogWriteHandleAppServers({ restart: restartCodex, log: console }); | ||
| } | ||
|
Comment on lines
+762
to
+768
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Print the full sync-cache CLI case to check user-facing output and exit codes.
set -euo pipefail
rg -n -A16 'case "sync-cache":' src/cli/index.tsRepository: lidge-jun/opencodex Length of output: 1058 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- CLI dispatch and sync-cache context ---'
sed -n '700,790p' src/cli/index.ts
printf '%s\n' '--- sync implementation ---'
sed -n '500,570p' src/codex/catalog/sync.ts
printf '%s\n' '--- command completion/error handling references ---'
rg -n -C4 'sync-cache|process\.exit|exitCode|invalidateCodexModelsCache|afterCatalogWriteHandleAppServers' src/cli src/codexRepository: lidge-jun/opencodex Length of output: 42692 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- CLI file outline and entrypoint ---'
ast-grep outline src/cli/index.ts
printf '%s\n' '--- CLI tail and top-level invocation ---'
tail -n 140 src/cli/index.ts
printf '%s\n' '--- all sync-cache references ---'
rg -n -C6 'sync-cache|invalidateCodexModelsCache' .Repository: lidge-jun/opencodex Length of output: 46857 Set a non-zero exit code when
🤖 Prompt for AI Agents |
||
| break; | ||
| } | ||
| case "gui": { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make all stale-process guidance reflect the write-status gate.
Stale warnings and restarts occur only after a successful catalog or cache write. No-op syncs do not terminate matching processes.
docs-site/src/content/docs/reference/cli.md#L150-L154: state that--restart-codexacts only after a successful write.docs-site/src/content/docs/guides/codex-integration.md#L194-L198: add the same condition to the warning and recovery steps.gui/src/pages/dashboard-overview-sections.tsx#L225-L225: ensureocx sync --restart-codexperforms a write after the API sync, or show a command that reliably rewrites the cache.📍 Affects 3 files
docs-site/src/content/docs/reference/cli.md#L150-L154(this comment)docs-site/src/content/docs/guides/codex-integration.md#L194-L198gui/src/pages/dashboard-overview-sections.tsx#L225-L225🤖 Prompt for AI Agents
Source: Path instructions