Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
effect-codex-app-server
ApprovabilityVerdict: Needs human review Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Error recovery yields failure instead of fallback value
- Piped the Effect.tryPromise through Effect.orElseSucceed(() => "") so that a rejected response.text() recovers to an empty string success value instead of short-circuiting the generator with a bare empty string failure.
Or push these changes by commenting:
@cursor push cf803e8469
Preview (cf803e8469)
diff --git a/packages/effect-codex-app-server/scripts/generate.ts b/packages/effect-codex-app-server/scripts/generate.ts
--- a/packages/effect-codex-app-server/scripts/generate.ts
+++ b/packages/effect-codex-app-server/scripts/generate.ts
@@ -160,8 +160,9 @@
if (!response.ok) {
const detail = yield* Effect.tryPromise({
try: () => response.text(),
- catch: () => "",
- });
+ catch: (cause) =>
+ new GeneratorError({ detail: `Failed to read response body from ${url}`, cause }),
+ }).pipe(Effect.orElseSucceed(() => ""));
return yield* Effect.fail(
new GeneratorError({
detail: `Failed to download ${url}: ${response.status} ${detail}`,You can send follow-ups to the cloud agent here.
3d9f655 to
308dd65
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Removed null safety on JSON-RPC response access
- Restored
| null | undefinedunion type on theasassertion and replaced direct property access with optional chaining (response?.configOptions) in both the helper function and inline usage inmain().
- Restored
- ✅ Fixed: Fake getSession re-invokes startImpl on every access
- Replaced
getSession's delegation tostartImpl()with an independentEffect.syncthat constructs the session data directly, matching the real runtime'sRef.getbehavior and preventing inflatedstartImplcall counts.
- Replaced
Or push these changes by commenting:
@cursor push 418b113858
Preview (418b113858)
diff --git a/apps/server/scripts/cursor-acp-model-mismatch-probe.ts b/apps/server/scripts/cursor-acp-model-mismatch-probe.ts
--- a/apps/server/scripts/cursor-acp-model-mismatch-probe.ts
+++ b/apps/server/scripts/cursor-acp-model-mismatch-probe.ts
@@ -316,10 +316,10 @@
sessionId,
configId: option.id,
value,
- })) as SetConfigResult;
+ })) as SetConfigResult | null | undefined;
logSection(`SET_${label}_RESPONSE`, response);
- return response.configOptions ?? configOptions;
+ return response?.configOptions ?? configOptions;
}
async function main() {
@@ -377,10 +377,10 @@
sessionId,
configId: modelConfig.id,
value: targetModel,
- })) as SetConfigResult;
+ })) as SetConfigResult | null | undefined;
logSection("SET_MODEL_RESPONSE", setModelResponse);
- configOptions = setModelResponse.configOptions ?? configOptions;
+ configOptions = setModelResponse?.configOptions ?? configOptions;
configOptions = await setSelectOptionIfAdvertised(
rpc,
diff --git a/apps/server/src/provider/Layers/CodexAdapter.test.ts b/apps/server/src/provider/Layers/CodexAdapter.test.ts
--- a/apps/server/src/provider/Layers/CodexAdapter.test.ts
+++ b/apps/server/src/provider/Layers/CodexAdapter.test.ts
@@ -101,7 +101,19 @@
return Effect.promise(() => this.startImpl());
}
- getSession = Effect.promise(() => this.startImpl());
+ getSession = Effect.sync(
+ () =>
+ ({
+ provider: "codex" as const,
+ status: "ready" as const,
+ runtimeMode: this.options.runtimeMode,
+ threadId: this.options.threadId,
+ cwd: this.options.cwd,
+ ...(this.options.model ? { model: this.options.model } : {}),
+ createdAt: this.now,
+ updatedAt: this.now,
+ }) satisfies ProviderSession,
+ );
sendTurn(input: CodexSessionRuntimeSendTurnInput) {
return Effect.promise(() => this.sendTurnImpl(input));You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Thread resume lacks graceful fallback to fresh start
- Added isRecoverableThreadResumeError check with Effect.catchIf around thread/resume that falls back to thread/start for recoverable errors (not found, missing thread, etc.), restoring the old CodexAppServerManager behavior.
- ✅ Fixed: Account read failure now prevents session from starting
- Wrapped account/read in Effect.matchEffect so failures log a warning and fall back to the default account snapshot instead of crashing session startup.
Or push these changes by commenting:
@cursor push f93739f5a8
Preview (f93739f5a8)
diff --git a/apps/server/src/provider/codex/CodexSessionRuntime.ts b/apps/server/src/provider/codex/CodexSessionRuntime.ts
--- a/apps/server/src/provider/codex/CodexSessionRuntime.ts
+++ b/apps/server/src/provider/codex/CodexSessionRuntime.ts
@@ -46,7 +46,22 @@
"state db missing rollout path for thread",
"state db record_discrepancy: find_thread_path_by_id_str_in_subdir, falling_back",
];
+const RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS = [
+ "not found",
+ "missing thread",
+ "no such thread",
+ "unknown thread",
+ "does not exist",
+];
+function isRecoverableThreadResumeError(error: unknown): boolean {
+ const message = (error instanceof Error ? error.message : String(error)).toLowerCase();
+ if (!message.includes("thread/resume")) {
+ return false;
+ }
+ return RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS.some((snippet) => message.includes(snippet));
+}
+
export const CodexResumeCursorSchema = Schema.Struct({
threadId: Schema.String,
});
@@ -1078,10 +1093,16 @@
yield* client.request("initialize", buildCodexInitializeParams());
yield* client.notify("initialized", undefined);
- const accountSnapshot = readCodexAccountSnapshotResponse(
- yield* client.request("account/read", {}),
- );
- yield* Ref.set(accountRef, accountSnapshot);
+ const accountSnapshot = yield* Effect.matchEffect(client.request("account/read", {}), {
+ onSuccess: (response) =>
+ Effect.succeed(readCodexAccountSnapshotResponse(response)).pipe(
+ Effect.tap((snapshot) => Ref.set(accountRef, snapshot)),
+ ),
+ onFailure: (error) =>
+ Effect.logWarning("codex account/read failed, continuing with default account", {
+ cause: error instanceof Error ? error.message : String(error),
+ }).pipe(Effect.andThen(Ref.get(accountRef))),
+ });
const requestedModel = resolveCodexModelForAccount(
normalizeCodexModelSlug(options.model),
@@ -1089,26 +1110,34 @@
);
const resumedThreadId = readResumeCursorThreadId(options.resumeCursor);
+ const threadStartParams = buildThreadStartParams({
+ cwd: options.cwd,
+ runtimeMode: options.runtimeMode,
+ model: requestedModel,
+ serviceTier: options.serviceTier,
+ });
+ const startFreshThread = client.request("thread/start", threadStartParams);
const opened =
resumedThreadId !== undefined
- ? yield* client.request("thread/resume", {
- threadId: resumedThreadId,
- ...buildThreadStartParams({
- cwd: options.cwd,
- runtimeMode: options.runtimeMode,
- model: requestedModel,
- serviceTier: options.serviceTier,
- }),
- })
- : yield* client.request(
- "thread/start",
- buildThreadStartParams({
- cwd: options.cwd,
- runtimeMode: options.runtimeMode,
- model: requestedModel,
- serviceTier: options.serviceTier,
- }),
- );
+ ? yield* client
+ .request("thread/resume", { threadId: resumedThreadId, ...threadStartParams })
+ .pipe(
+ Effect.catchIf(isRecoverableThreadResumeError, (error) =>
+ emitSessionEvent(
+ "session/threadResumeFallback",
+ `Could not resume thread ${resumedThreadId}; starting a new thread instead.`,
+ ).pipe(
+ Effect.andThen(
+ Effect.logWarning("codex thread/resume fell back to fresh start", {
+ resumedThreadId,
+ cause: error.message,
+ }),
+ ),
+ Effect.andThen(startFreshThread),
+ ),
+ ),
+ )
+ : yield* startFreshThread;
const providerThreadId = opened.thread.id;
const session = {You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Overly permissive recoverable error matching hides real failures
- Added a guard requiring the error message to contain 'thread' before matching generic snippets, preventing unrelated errors like 'Config file not found' from being silently treated as recoverable.
Or push these changes by commenting:
@cursor push 4faeac1f09
Preview (4faeac1f09)
diff --git a/apps/server/src/provider/codex/CodexSessionRuntime.test.ts b/apps/server/src/provider/codex/CodexSessionRuntime.test.ts
--- a/apps/server/src/provider/codex/CodexSessionRuntime.test.ts
+++ b/apps/server/src/provider/codex/CodexSessionRuntime.test.ts
@@ -171,6 +171,27 @@
false,
);
});
+
+ it("ignores unrelated 'not found' errors that don't mention thread", () => {
+ assert.equal(
+ isRecoverableThreadResumeError(
+ new CodexErrors.CodexAppServerRequestError({
+ code: -32603,
+ errorMessage: "Config file not found",
+ }),
+ ),
+ false,
+ );
+ assert.equal(
+ isRecoverableThreadResumeError(
+ new CodexErrors.CodexAppServerRequestError({
+ code: -32603,
+ errorMessage: "Model does not exist",
+ }),
+ ),
+ false,
+ );
+ });
});
describe("openCodexThread", () => {
diff --git a/apps/server/src/provider/codex/CodexSessionRuntime.ts b/apps/server/src/provider/codex/CodexSessionRuntime.ts
--- a/apps/server/src/provider/codex/CodexSessionRuntime.ts
+++ b/apps/server/src/provider/codex/CodexSessionRuntime.ts
@@ -383,6 +383,9 @@
export function isRecoverableThreadResumeError(error: unknown): boolean {
const message = (error instanceof Error ? error.message : String(error)).toLowerCase();
+ if (!message.includes("thread")) {
+ return false;
+ }
return RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS.some((snippet) => message.includes(snippet));
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit d53e02b. Configure here.
Co-authored-by: codex <codex@users.noreply.github.com>
- Initialize provider as unchecked in a pending state - Update initial probe message to reflect session-local status
- Type the runtime effect with `Scope` - Build the ACP session runtime without wrapping it in `Effect.scoped`
- Use strict TurnId and ProviderItemId parsing in Codex session routing - Decode in-memory stdio chunks in streaming mode to avoid split UTF-8 corruption
- Transfer session-owned scopes into adapter state - Ensure runtime scopes close on stop and startup failure - Add regression coverage for scoped lifecycle cleanup
- Close the managed native event logger when the adapter layer tears down - Make session runtime close idempotent with an atomic closed flag - Add coverage for flushing thread native logs on shutdown
- Use codex app-server snapshots for auth, models, and skills - Remove legacy CLI/config discovery paths and related helpers - Update tests for the new provider status flow
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
d138a0b to
6414e08
Compare
- Move Codex runtime and app-server probe logic under `provider/Layers` - Centralize initialize params and provider snapshot types - Update tests and imports after the Codex provider split
Upstream additions: - fix(web): restore manual sort drag and keep per-group expand state (pingdotgg#2221) - fix: Change right panel sheet to be below title bar / action bar (pingdotgg#2224) - Refactor OpenCode lifecycle and structured output handling (pingdotgg#2218) - effect-codex-app-server (pingdotgg#1942) - Redesign model picker with favorites and search (pingdotgg#2153) - fix(server): prevent probeClaudeCapabilities from wasting API requests (pingdotgg#2192) - fix(server): handle OpenCode text response format in commit message gen (pingdotgg#2202) - Devcontainer / IDE updates (pingdotgg#2208) - Expand leading ~ in Codex home paths before exporting CODEX_HOME (pingdotgg#2210) - fix(release): use v<semver> tag format for nightly releases (pingdotgg#2186) Fork adaptations: - Took upstream's redesigned model picker with favorites and search - Removed deleted codexAppServerManager (replaced by effect-codex-app-server) - Stubbed fetchCodexUsage (manager-based readout no longer available) - Extended PROVIDER_ICON_BY_PROVIDER for all 8 fork providers - Extended modelOptionsByProvider test fixtures for all 8 providers - Inline ClaudeSlashCommand type (not yet re-exported from SDK) - Updated SettingsPanels imports for new picker module structure - Preserved fork's CI customizations (ubuntu-24.04 not Blacksmith)


Typesafe JSONRPC effect API for Codex App Server. Generated from the protocol's JSON schemas
Note
Medium Risk
Moderate risk because it replaces the Codex provider’s session/runtime implementation and event mapping, which can affect session lifecycle and downstream consumers of provider events.
Overview
Switches the server’s Codex integration from the in-repo
CodexAppServerManagerto theeffect-codex-app-servertyped JSON-RPC runtime, including per-thread scoped session lifecycles and a new schema-driven mapping from raw Codex notifications/requests to canonical runtime events.Removes the legacy manager implementation and its large test suite, adds shared
CodexDeveloperInstructionsconstants, and updates Codex git text generation to usefastModedirectly (dropping capability-based normalization). The Codex adapter tests are rewritten to exercise the new runtime factory/scoping behavior, native event log flushing on layer shutdown, and updated payload shapes (e.g., user-input answers, request resolution, plan item typing).Reviewed by Cursor Bugbot for commit 97a9354. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add
effect-codex-app-serverpackage with typed RPC client and migrate Codex provider to app-server protocolpackages/effect-codex-app-serverpackage providing a JSONL-over-stdio RPC client, generated protocol schemas, and structured error types for communicating with the Codex app-server subprocess.CodexProvider.tsto probe provider status by spawning the app-server and querying it via RPC for account, models, and skills, replacing all CLI-based auth parsing and config.toml logic.CodexAdapter.tsto manage per-threadCodexSessionRuntimeShapeinstances directly, streaming typed events into a queue via a per-session fiber, replacingCodexAppServerManagerand its event emitter.CodexSessionRuntime.tsdefining the session runtime interface, options, and schemas consumed by the adapter.Macroscope summarized 97a9354.