Repository navigation
fix(server): slow-starting Codex app-server no longer reports Unavailable #27262
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
base: main
Are you sure you want to change the base?
Changes from all commits
000fe03
0d7d647
fc6af35
0e65b16
88365fa
a6c7657
004f6a7
707ad77
3c54476
9b55e5c
7139fa1
0909016
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 |
|---|---|---|
|
|
@@ -125,8 +125,9 @@ const CODEX_CATALOG_REFRESH_TIMEOUT_MS = 3_000; | |
| const CODEX_CONFIG_PROBE_TIMEOUT_MS = 3_000; | ||
| const CODEX_CATALOG_REFRESH_POLL = "100 millis" as const; | ||
| const CODEX_CLIENT_NAME = "T3 Code"; | ||
| // Handshake plus account/models/skills requests can exceed the shared 10s probe budget. | ||
| const CODEX_AUTH_PROBE_TIMEOUT_MS = 20_000; | ||
| // Old machines can take ~25s to start app-server and answer initialize, before | ||
| // account/models/skills discovery runs. | ||
| export const CODEX_APP_SERVER_PROBE_TIMEOUT_MS = 60_000; | ||
|
Contributor
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. 🟡 Medium An unanswered 🚀 Reply "fix it for me" or copy this AI Prompt for your agent: |
||
| const CODEX_DISCOVERY_MARGIN_MS = 250; | ||
| // `codex-cli 0.159.0`, `codex-cli 0.160.0-alpha.1` or the npm CLI's bare `0.1.4161006`. | ||
| // Keep a prerelease suffix so compatibility stays unknown for it, as it does for `initialize`. | ||
|
|
@@ -953,7 +954,7 @@ export const checkCodexProviderStatus = Effect.fn("checkCodexProviderStatus")(fu | |
|
|
||
| const probeTimeoutMs = resolveProviderProbeTimeoutMs( | ||
| "codex", | ||
| CODEX_AUTH_PROBE_TIMEOUT_MS, | ||
| CODEX_APP_SERVER_PROBE_TIMEOUT_MS, | ||
| resolvedEnvironment, | ||
| ); | ||
| const probeResult = yield* probe({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -509,10 +509,10 @@ it.layer( | |
| Effect.gen(function* () { | ||
| const fiber = yield* checkCodexProviderStatus( | ||
| defaultCodexSettings, | ||
| () => Effect.sleep("22 seconds").pipe(Effect.as(makeCodexProbeSnapshot())), | ||
| { T3CODE_CODEX_PROBE_TIMEOUT_MS: "60000" }, | ||
| () => Effect.sleep("65 seconds").pipe(Effect.as(makeCodexProbeSnapshot())), | ||
| { T3CODE_CODEX_PROBE_TIMEOUT_MS: "90000" }, | ||
| ).pipe(Effect.forkChild); | ||
| yield* TestClock.adjust("22 seconds"); | ||
| yield* TestClock.adjust("65 seconds"); | ||
| const status = yield* Fiber.join(fiber); | ||
| assert.strictEqual(status.status, "ready"); | ||
| assert.strictEqual(status.auth.status, "authenticated"); | ||
|
|
@@ -604,6 +604,35 @@ it.layer( | |
| }), | ||
| ); | ||
|
|
||
| it.effect("returns ready when app-server answers slowly but correctly", () => | ||
| Effect.gen(function* () { | ||
| const fiber = yield* checkCodexProviderStatus(defaultCodexSettings, () => | ||
| Effect.sleep("25 seconds").pipe(Effect.as(makeCodexProbeSnapshot())), | ||
|
Comment on lines
+609
to
+610
Contributor
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. Test the real discovery deadline. This test replaces the entire probe, so it passes even if discovery still expires 20 seconds after spawn. Add a focused test that delays the actual As per coding guidelines: “Test behavior that can break,” including “the logic a bug fix changed.”
|
||
| ).pipe(Effect.forkChild); | ||
| yield* TestClock.adjust("25 seconds"); | ||
| const status = yield* Fiber.join(fiber); | ||
|
|
||
| assert.strictEqual(status.message, undefined); | ||
| assert.strictEqual(status.status, "ready"); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("times out when app-server never answers", () => | ||
| Effect.gen(function* () { | ||
| const fiber = yield* checkCodexProviderStatus(defaultCodexSettings, () => | ||
| Effect.never.pipe(Effect.as(makeCodexProbeSnapshot())), | ||
| ).pipe(Effect.forkChild); | ||
| yield* TestClock.adjust("60 seconds"); | ||
| const status = yield* Fiber.join(fiber); | ||
|
|
||
| assert.strictEqual(status.status, "error"); | ||
| assert.strictEqual( | ||
| status.message, | ||
| "Timed out while checking Codex app-server provider status.", | ||
| ); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("offers cyber access only on catalog models that advertise it", () => | ||
| Effect.gen(function* () { | ||
| const catalogModel = (slug: string, cyber?: Array<"standard" | "daybreakBlue">) => ({ | ||
|
|
||
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.
Change the Codex recovery timeout example to
120000.With the Codex default now
60_000, an override of60000is not greater than the default, so the value is ignored and the deadline stays at 60 seconds. Users who follow this instruction get the same timeout they already have.docs/user/install.mdalready uses120000for this variable.📝 Suggested fix