Skip to content

fix(codex): share model cache with ACP run homes#51

Merged
jomeswang merged 5 commits into
mainfrom
agent/provider-runtime-home
Jul 18, 2026
Merged

fix(codex): share model cache with ACP run homes#51
jomeswang merged 5 commits into
mainfrom
agent/provider-runtime-home

Conversation

@jomeswang

@jomeswang jomeswang commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • link models_cache.json from the stable provider home into each transient Codex run home
  • preserve the existing isolated config, MCP, skills, and temporary-file boundary
  • cover cache write-through behavior in the launch-plan test

Verification

  • pnpm vitest run tests/providers/codex-launch-plan.test.ts
  • npm run typecheck

The full suite has two pre-existing timing failures unrelated to this change.


Open in Devin Review

@jomeswang
jomeswang force-pushed the agent/provider-runtime-home branch from d16d168 to b5bcac1 Compare July 18, 2026 03:38
@jomeswang
jomeswang merged commit b03854a into main Jul 18, 2026
1 of 2 checks passed
@jomeswang
jomeswang deleted the agent/provider-runtime-home branch July 18, 2026 03:40

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines +650 to +653
await linkWritableModelsCache(
join(sourceHome, "models_cache.json"),
join(runHome, "models_cache.json"),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Shared model-cache setup can abort an entire Codex run instead of falling back

The shared model catalog file is set up with a step that throws on failure (linkWritableModelsCache at src/providers/codex/index.ts:650-653) instead of being treated as an optional speed-up, so a run that used to work now aborts entirely when that file cannot be created or linked.
Impact: On a read-only or restricted provider home, Codex runs that previously succeeded (just with a slower cold model lookup) will fail to launch at all.

Why the optimization becomes a hard failure

linkWritableModelsCache (src/providers/codex/index.ts:197-209) is awaited directly in materializeCodexHome (src/providers/codex/index.ts:650-653) with no surrounding try/catch. If the source models_cache.json does not exist, it writes an empty file into sourceHome; if that write fails for any reason other than EEXIST (e.g. a read-only provider home) it throws. Likewise a failing linkFile (rm/symlink/hard-link) throws. Any throw rejects materializeCodexHome, which is surfaced via if (homeResult.status === "rejected") throw homeResult.reason; and fails the whole launch. This contrasts with the plugin cache immediately below (src/providers/codex/index.ts:656-661), which is wrapped in a best-effort try/catch precisely because it is described as an optimization. The comment on linkWritableModelsCache frames the model cache as an optimization to avoid cold-start discovery, yet its failure now blocks the run entirely rather than degrading to the prior cold-start behavior.

Suggested change
await linkWritableModelsCache(
join(sourceHome, "models_cache.json"),
join(runHome, "models_cache.json"),
);
try {
await linkWritableModelsCache(
join(sourceHome, "models_cache.json"),
join(runHome, "models_cache.json"),
);
} catch {
// Sharing the model cache is an optimization to avoid cold-start model
// discovery. If the shared file cannot be created or linked (e.g. a
// read-only provider home), fall back to per-run discovery.
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant