Compare commits

...

21 Commits

Author SHA1 Message Date
Owen Lin
67b5ef9cb0 fix 2025-12-02 14:51:06 -08:00
Owen Lin
ce7fc27f89 [app-server] feat: add old_content and new_content to FileChange update patches 2025-12-02 14:28:32 -08:00
Owen Lin
8532876ad8 [app-server] fix: emit item/fileChange/outputDelta for file change items (#7399) 2025-12-01 17:52:34 +00:00
Owen Lin
44d92675eb [app-server] fix: ensure thread_id and turn_id are on all events (#7408)
This is an improvement for client-side developer ergonomics by
simplifying the state the client needs to keep track of.
2025-12-01 08:50:47 -08:00
jif-oai
a421eba31f fix: disable review rollout filtering (#7371) 2025-12-01 09:04:13 +00:00
Celia Chen
40006808a3 [app-server] add turn/plan/updated event (#7329)
transform `EventMsg::PlanDate` to v2 `turn/plan/updated` event. similar
to `turn/diff/updated`.
2025-11-30 21:09:59 -08:00
dependabot[bot]
ba58184349 chore(deps): bump image from 0.25.8 to 0.25.9 in /codex-rs (#7421)
Bumps [image](https://github.com/image-rs/image) from 0.25.8 to 0.25.9.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/image-rs/image/blob/main/CHANGES.md">image's
changelog</a>.</em></p>
<blockquote>
<h3>Version 0.25.9</h3>
<p>Features:</p>
<ul>
<li>Support extracting XMP metadata from PNG, JPEG, GIF, WebP and TIFF
files (<a
href="https://redirect.github.com/image-rs/image/issues/2567">#2567</a>,
<a
href="https://redirect.github.com/image-rs/image/issues/2634">#2634</a>,
<a
href="https://redirect.github.com/image-rs/image/issues/2644">#2644</a>)</li>
<li>Support reading IPTC metadata from PNG and JPG files (<a
href="https://redirect.github.com/image-rs/image/issues/2611">#2611</a>)</li>
<li>Support reading ICC profile from GIF files (<a
href="https://redirect.github.com/image-rs/image/issues/2644">#2644</a>)</li>
<li>Allow setting a specific DEFLATE compression level when writing PNG
(<a
href="https://redirect.github.com/image-rs/image/issues/2583">#2583</a>)</li>
<li>Initial support for 16-bit CMYK TIFF files (<a
href="https://redirect.github.com/image-rs/image/issues/2588">#2588</a>)</li>
<li>Allow extracting the alpha channel of a <code>Pixel</code> in a
generic way (<a
href="https://redirect.github.com/image-rs/image/issues/2638">#2638</a>)</li>
</ul>
<p>Structural changes:</p>
<ul>
<li>EXR format decoding now only uses multi-threading via Rayon when the
<code>rayon</code> feature is enabled (<a
href="https://redirect.github.com/image-rs/image/issues/2643">#2643</a>)</li>
<li>Upgraded zune-jpeg to 0.5.x, ravif to 0.12.x, gif to 0.14.x</li>
<li>pnm: parse integers in PBM/PGM/PPM headers without allocations (<a
href="https://redirect.github.com/image-rs/image/issues/2620">#2620</a>)</li>
<li>Replace <code>doc_auto_cfg</code> with <code>doc_cfg</code> (<a
href="https://redirect.github.com/image-rs/image/issues/2637">#2637</a>)</li>
</ul>
<p>Bug fixes:</p>
<ul>
<li>Do not encode empty JPEG images (<a
href="https://redirect.github.com/image-rs/image/issues/2624">#2624</a>)</li>
<li>tga: reject empty images (<a
href="https://redirect.github.com/image-rs/image/issues/2614">#2614</a>)</li>
<li>tga: fix orientation flip for color mapped images (<a
href="https://redirect.github.com/image-rs/image/issues/2607">#2607</a>)</li>
<li>tga: adjust colormap lookup to match tga 2.0 spec (<a
href="https://redirect.github.com/image-rs/image/issues/2608">#2608</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="5ceb6af6c2"><code>5ceb6af</code></a>
Merge pull request <a
href="https://redirect.github.com/image-rs/image/issues/2640">#2640</a>
from Shnatsel/release-v0.25.9</li>
<li><a
href="282d7b345c"><code>282d7b3</code></a>
Merge pull request <a
href="https://redirect.github.com/image-rs/image/issues/2646">#2646</a>
from oligamiq/main</li>
<li><a
href="5412aeee5a"><code>5412aee</code></a>
Amend the note in accordance with the advice of 197g.</li>
<li><a
href="4e8a4ed2e8"><code>4e8a4ed</code></a>
Clarify default features in README and add usage note</li>
<li><a
href="ca8fa528ff"><code>ca8fa52</code></a>
Merge pull request <a
href="https://redirect.github.com/image-rs/image/issues/2644">#2644</a>
from image-rs/gif-0.14</li>
<li><a
href="d9bc8fe790"><code>d9bc8fe</code></a>
mention GIF 0.14 changes</li>
<li><a
href="053220a0b1"><code>053220a</code></a>
Provide gif's XMP and ICC metadata</li>
<li><a
href="2ec20b3b3b"><code>2ec20b3</code></a>
Prepare codec with gif@0.14</li>
<li><a
href="31939facce"><code>31939fa</code></a>
Mention EXR rayon change</li>
<li><a
href="c7f68be265"><code>c7f68be</code></a>
Merge pull request <a
href="https://redirect.github.com/image-rs/image/issues/2643">#2643</a>
from Shnatsel/really-optional-rayon</li>
<li>Additional commits viewable in <a
href="https://github.com/image-rs/image/compare/v0.25.8...v0.25.9">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=image&package-manager=cargo&previous-version=0.25.8&new-version=0.25.9)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2025-11-30 20:50:51 -08:00
Eric Traut
14df5c9492 Fixed CLA action to properly exempt dependabot (#7429) 2025-11-30 20:45:17 -08:00
dependabot[bot]
cb85a7b96e chore(deps): bump tracing from 0.1.41 to 0.1.43 in /codex-rs (#7428)
Bumps [tracing](https://github.com/tokio-rs/tracing) from 0.1.41 to
0.1.43.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/tokio-rs/tracing/releases">tracing's
releases</a>.</em></p>
<blockquote>
<h2>tracing 0.1.43</h2>
<h4>Important</h4>
<p>The previous release [0.1.42] was yanked because <a
href="https://redirect.github.com/tokio-rs/tracing/issues/3382">#3382</a>
was a breaking change.
See further details in <a
href="https://redirect.github.com/tokio-rs/tracing/issues/3424">#3424</a>.
This release contains all the changes from that
version, plus a revert for the problematic part of the breaking PR.</p>
<h3>Fixed</h3>
<ul>
<li>Revert &quot;make <code>valueset</code> macro sanitary&quot; (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3425">#3425</a>)</li>
</ul>
<p><a
href="https://redirect.github.com/tokio-rs/tracing/issues/3382">#3382</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/3382">tokio-rs/tracing#3382</a>
<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3424">#3424</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/3424">tokio-rs/tracing#3424</a>
<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3425">#3425</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/3425">tokio-rs/tracing#3425</a>
[0.1.42]: <a
href="https://github.com/tokio-rs/tracing/releases/tag/tracing-0.1.42">https://github.com/tokio-rs/tracing/releases/tag/tracing-0.1.42</a></p>
<h2>tracing 0.1.42</h2>
<h3>Important</h3>
<p>The [<code>Span::record_all</code>] method has been removed from the
documented API. It
was always unsuable via the documented API as it requried a
<code>ValueSet</code> which
has no publically documented constructors. The method remains, but
should not
be used outside of <code>tracing</code> macros.</p>
<h3>Added</h3>
<ul>
<li><strong>attributes</strong>: Support constant expressions as
instrument field names (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3158">#3158</a>)</li>
<li>Add <code>record_all!</code> macro for recording multiple values in
one call (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3227">#3227</a>)</li>
<li><strong>core</strong>: Improve code generation at trace points
significantly (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3398">#3398</a>)</li>
</ul>
<h3>Changed</h3>
<ul>
<li><code>tracing-core</code>: updated to 0.1.35 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3414">#3414</a>)</li>
<li><code>tracing-attributes</code>: updated to 0.1.31 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3417">#3417</a>)</li>
</ul>
<h3>Fixed</h3>
<ul>
<li>Fix &quot;name / parent&quot; variant of <code>event!</code> (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/2983">#2983</a>)</li>
<li>Remove 'r#' prefix from raw identifiers in field names (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3130">#3130</a>)</li>
<li>Fix perf regression when <code>release_max_level_*</code> not set
(<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3373">#3373</a>)</li>
<li>Use imported instead of fully qualified path (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3374">#3374</a>)</li>
<li>Make <code>valueset</code> macro sanitary (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3382">#3382</a>)</li>
</ul>
<h3>Documented</h3>
<ul>
<li><strong>core</strong>: Add missing <code>dyn</code> keyword in
<code>Visit</code> documentation code sample (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3387">#3387</a>)</li>
</ul>
<p><a
href="https://redirect.github.com/tokio-rs/tracing/issues/2983">#2983</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/%5B#2983%5D(https://redirect.github.com/tokio-rs/tracing/issues/2983)">tokio-rs/tracing#2983</a>
<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3130">#3130</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/%5B#3130%5D(https://redirect.github.com/tokio-rs/tracing/issues/3130)">tokio-rs/tracing#3130</a>
<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3158">#3158</a>:
<a
href="https://redirect.github.com/tokio-rs/tracing/pull/%5B#3158%5D(https://redirect.github.com/tokio-rs/tracing/issues/3158)">tokio-rs/tracing#3158</a></p>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="64e1c8d3ae"><code>64e1c8d</code></a>
chore: prepare tracing 0.1.43 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3427">#3427</a>)</li>
<li><a
href="7c44f7bb21"><code>7c44f7b</code></a>
tracing: revert &quot;make <code>valueset</code> macro sanitary&quot;
(<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3425">#3425</a>)</li>
<li><a
href="cdaf661c13"><code>cdaf661</code></a>
chore: prepare tracing-mock 0.1.0-beta.2 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3422">#3422</a>)</li>
<li><a
href="a164fd3021"><code>a164fd3</code></a>
chore: prepare tracing-journald 0.3.2 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3421">#3421</a>)</li>
<li><a
href="405397b8cc"><code>405397b</code></a>
chore: prepare tracing-appender 0.2.4 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3420">#3420</a>)</li>
<li><a
href="a9eeed7394"><code>a9eeed7</code></a>
chore: prepare tracing-subscriber 0.3.21 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3419">#3419</a>)</li>
<li><a
href="5bd5505478"><code>5bd5505</code></a>
chore: prepare tracing 0.1.42 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3418">#3418</a>)</li>
<li><a
href="55086231ec"><code>5508623</code></a>
chore: prepare tracing-attributes 0.1.31 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3417">#3417</a>)</li>
<li><a
href="d92b4c0feb"><code>d92b4c0</code></a>
chore: prepare tracing-core 0.1.35 (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3414">#3414</a>)</li>
<li><a
href="9751b6e776"><code>9751b6e</code></a>
chore: run <code>tracing-subscriber</code> tests with all features (<a
href="https://redirect.github.com/tokio-rs/tracing/issues/3412">#3412</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/tokio-rs/tracing/compare/tracing-0.1.41...tracing-0.1.43">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=tracing&package-manager=cargo&previous-version=0.1.41&new-version=0.1.43)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eric Traut <etraut@openai.com>
2025-11-30 20:36:03 -08:00
dependabot[bot]
3f12f1140f chore(deps): bump reqwest from 0.12.23 to 0.12.24 in /codex-rs (#7424)
Bumps [reqwest](https://github.com/seanmonstar/reqwest) from 0.12.23 to
0.12.24.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/seanmonstar/reqwest/releases">reqwest's
releases</a>.</em></p>
<blockquote>
<h2>v0.12.24</h2>
<h2>Highlights</h2>
<ul>
<li>Refactor cookie handling to an internal middleware.</li>
<li>Refactor internal random generator.</li>
<li>Refactor base64 encoding to reduce a copy.</li>
<li>Documentation updates.</li>
</ul>
<h2>What's Changed</h2>
<ul>
<li>build(deps): silence unused deps in WASM build by <a
href="https://github.com/0x676e67"><code>@​0x676e67</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2799">seanmonstar/reqwest#2799</a></li>
<li>perf(util): avoid extra copy when base64 encoding by <a
href="https://github.com/0x676e67"><code>@​0x676e67</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2805">seanmonstar/reqwest#2805</a></li>
<li>docs: fix method name in changelog entry by <a
href="https://github.com/johannespfrang"><code>@​johannespfrang</code></a>
in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2807">seanmonstar/reqwest#2807</a></li>
<li>chore: Align the name usage of TotalTimeout by <a
href="https://github.com/Xuanwo"><code>@​Xuanwo</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2657">seanmonstar/reqwest#2657</a></li>
<li>refactor(cookie): add <code>CookieService</code> by <a
href="https://github.com/linyihai"><code>@​linyihai</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2787">seanmonstar/reqwest#2787</a></li>
<li>Fixes typo in retry max_retries_per_request doc comment re 2813 by
<a href="https://github.com/dmackinn"><code>@​dmackinn</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2824">seanmonstar/reqwest#2824</a></li>
<li>test(multipart): fix build failure with
<code>no-default-features</code> by <a
href="https://github.com/0x676e67"><code>@​0x676e67</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2801">seanmonstar/reqwest#2801</a></li>
<li>refactor(cookie): avoid duplicate cookie insertion by <a
href="https://github.com/0x676e67"><code>@​0x676e67</code></a> in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2834">seanmonstar/reqwest#2834</a></li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a
href="https://github.com/johannespfrang"><code>@​johannespfrang</code></a>
made their first contribution in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2807">seanmonstar/reqwest#2807</a></li>
<li><a href="https://github.com/dmackinn"><code>@​dmackinn</code></a>
made their first contribution in <a
href="https://redirect.github.com/seanmonstar/reqwest/pull/2824">seanmonstar/reqwest#2824</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/seanmonstar/reqwest/compare/v0.12.23...v0.12.24">https://github.com/seanmonstar/reqwest/compare/v0.12.23...v0.12.24</a></p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/seanmonstar/reqwest/blob/master/CHANGELOG.md">reqwest's
changelog</a>.</em></p>
<blockquote>
<h2>v0.12.24</h2>
<ul>
<li>Refactor cookie handling to an internal middleware.</li>
<li>Refactor internal random generator.</li>
<li>Refactor base64 encoding to reduce a copy.</li>
<li>Documentation updates.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="b126ca49da"><code>b126ca4</code></a>
v0.12.24</li>
<li><a
href="4023493096"><code>4023493</code></a>
refactor: change fast_random from xorshift to siphash a counter</li>
<li><a
href="fd61bc93e6"><code>fd61bc9</code></a>
refactor(cookie): avoid duplicate cookie insertion (<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2834">#2834</a>)</li>
<li><a
href="0bfa526776"><code>0bfa526</code></a>
test(multipart): fix build failure with <code>no-default-features</code>
(<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2801">#2801</a>)</li>
<li><a
href="994b8a0b7a"><code>994b8a0</code></a>
docs: typo in retry max_retries_per_request (<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2824">#2824</a>)</li>
<li><a
href="da0702b762"><code>da0702b</code></a>
refactor(cookie): de-duplicate cookie support as
<code>CookieService</code> middleware (...</li>
<li><a
href="7ebddeaa87"><code>7ebddea</code></a>
chore: align internal name usage of TotalTimeout (<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2657">#2657</a>)</li>
<li><a
href="b540a4e746"><code>b540a4e</code></a>
chore(readme): use correct CI status badge</li>
<li><a
href="e4550c4cc5"><code>e4550c4</code></a>
docs: fix method name in changelog entry (<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2807">#2807</a>)</li>
<li><a
href="f4694a2922"><code>f4694a2</code></a>
perf(util): avoid extra copy when base64 encoding (<a
href="https://redirect.github.com/seanmonstar/reqwest/issues/2805">#2805</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/seanmonstar/reqwest/compare/v0.12.23...v0.12.24">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=reqwest&package-manager=cargo&previous-version=0.12.23&new-version=0.12.24)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eric Traut <etraut@openai.com>
2025-11-30 20:35:49 -08:00
dependabot[bot]
c22cd2e953 chore(deps): bump serde_with from 3.14.0 to 3.16.1 in /codex-rs (#7422)
Bumps [serde_with](https://github.com/jonasbb/serde_with) from 3.14.0 to
3.16.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/jonasbb/serde_with/releases">serde_with's
releases</a>.</em></p>
<blockquote>
<h2>serde_with v3.16.1</h2>
<h3>Fixed</h3>
<ul>
<li>Fix <code>JsonSchemaAs</code> of <code>SetPreventDuplicates</code>
and <code>SetLastValueWins</code>. (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/906">#906</a>,
<a
href="https://redirect.github.com/jonasbb/serde_with/issues/907">#907</a>)</li>
</ul>
<h2>serde_with v3.16.0</h2>
<h3>Added</h3>
<ul>
<li>Added support for <code>smallvec</code> v1 under the
<code>smallvec_1</code> feature flag by <a
href="https://github.com/isharma228"><code>@​isharma228</code></a> (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/895">#895</a>)</li>
<li>Add <code>JsonSchemaAs</code> implementation for
<code>json::JsonString</code> by <a
href="https://github.com/yogevm15"><code>@​yogevm15</code></a> (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/901">#901</a>)</li>
</ul>
<h2>serde_with v3.15.1</h2>
<h3>Fixed</h3>
<ul>
<li>Fix building of the documentation by updating references to use
<code>serde_core</code>.</li>
</ul>
<h2>serde_with v3.15.0</h2>
<h3>Added</h3>
<ul>
<li>
<p>Added error inspection to <code>VecSkipError</code> and
<code>MapSkipError</code> by <a
href="https://github.com/michelhe"><code>@​michelhe</code></a> (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/878">#878</a>)
This allows interacting with the previously hidden error, for example
for logging.
Checkout the newly added example to both types.</p>
</li>
<li>
<p>Allow documenting the types generated by <code>serde_conv!</code>.
The <code>serde_conv!</code> macro now acceps outer attributes before
the optional visibility modifier.
This allow adding doc comments in the shape of <code>#[doc =
&quot;...&quot;]</code> or any other attributes, such as lint
modifiers.</p>
<pre lang="rust"><code>serde_conv!(
    #[doc = &quot;Serialize bools as string&quot;]
    #[allow(dead_code)]
    pub BoolAsString,
    bool,
    |x: &amp;bool| ::std::string::ToString::to_string(x),
    |x: ::std::string::String| x.parse()
);
</code></pre>
</li>
<li>
<p>Add support for <code>hashbrown</code> v0.16 (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/877">#877</a>)</p>
<p>This extends the existing support for <code>hashbrown</code> v0.14
and v0.15 to the newly released version.</p>
</li>
</ul>
<h3>Changed</h3>
<ul>
<li>Bump MSRV to 1.76, since that is required for <code>toml</code>
dev-dependency.</li>
</ul>
<h2>serde_with v3.14.1</h2>
<h3>Fixed</h3>
<ul>
<li>Show macro expansion in the docs.rs generated rustdoc.</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="8513323fda"><code>8513323</code></a>
Bump version to 3.16.1 (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/908">#908</a>)</li>
<li><a
href="5392bbe75e"><code>5392bbe</code></a>
Bump version to 3.16.1</li>
<li><a
href="1e54f1cd38"><code>1e54f1c</code></a>
Fix duplicate schema set definitions for schemars 0.8, 0.9, and 1.0 (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/907">#907</a>)</li>
<li><a
href="0650180645"><code>0650180</code></a>
Fix duplicate schema set definitions for schemars 0.8, 0.9, and 1.0</li>
<li><a
href="41d1033438"><code>41d1033</code></a>
Fix test conditions for schemars tests to include &quot;hex&quot;
feature</li>
<li><a
href="2eed58af05"><code>2eed58a</code></a>
Bump the github-actions group across 1 directory with 2 updates (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/905">#905</a>)</li>
<li><a
href="ed040f2330"><code>ed040f2</code></a>
Bump the github-actions group across 1 directory with 2 updates</li>
<li><a
href="fa2129b1b9"><code>fa2129b</code></a>
Bump ron from 0.11.0 to 0.12.0 (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/904">#904</a>)</li>
<li><a
href="b55cb99757"><code>b55cb99</code></a>
Bump ron from 0.11.0 to 0.12.0</li>
<li><a
href="066b9d4019"><code>066b9d4</code></a>
Bump version to 3.16.0 (<a
href="https://redirect.github.com/jonasbb/serde_with/issues/903">#903</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/jonasbb/serde_with/compare/v3.14.0...v3.16.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=serde_with&package-manager=cargo&previous-version=3.14.0&new-version=3.16.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Eric Traut <etraut@openai.com>
2025-11-30 20:35:32 -08:00
dependabot[bot]
ebd485b1a0 chore(deps): bump arboard from 3.6.0 to 3.6.1 in /codex-rs (#7426)
Bumps [arboard](https://github.com/1Password/arboard) from 3.6.0 to
3.6.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/1Password/arboard/releases">arboard's
releases</a>.</em></p>
<blockquote>
<h2>v3.6.1</h2>
<p>This release focuses on improving compatibility with data in the real
world and bug fixes. It also includes a new <code>Set</code> API for
working with file paths via drag-and-drop interfaces across Linux,
macOS, and Windows.</p>
<p>This release also marks the start of exclusively publishing
changelogs via GitHub Releases. The old <code>CHANGELOG.md</code> has
been removed due to maintenance overhead and duplication. <a
href="https://github.com/1Password/arboard/releases/tag/v3.6.0">v3.6.0</a>
is the last revision to include this file.</p>
<h3>Added</h3>
<ul>
<li>Add support for pasting lists of files via
<code>Set::file_list</code> interface by <a
href="https://github.com/Gae24"><code>@​Gae24</code></a> in <a
href="https://redirect.github.com/1Password/arboard/pull/181">1Password/arboard#181</a></li>
<li>Support <code>windows-sys</code> 0.60 in <code>arboard</code>'s
allowed version range by <a
href="https://github.com/complexspaces"><code>@​complexspaces</code></a>
in <a
href="https://redirect.github.com/1Password/arboard/pull/201">1Password/arboard#201</a></li>
</ul>
<h3>Changed</h3>
<ul>
<li>Fix grammar and typos by <a
href="https://github.com/complexspaces"><code>@​complexspaces</code></a>
and <a href="https://github.com/gagath"><code>@​gagath</code></a> in <a
href="https://redirect.github.com/1Password/arboard/pull/194">1Password/arboard#194</a>
and <a
href="https://redirect.github.com/1Password/arboard/pull/196">1Password/arboard#196</a></li>
<li>Prefer PNG when pasting images on Windows by <a
href="https://github.com/wcassels"><code>@​wcassels</code></a> in <a
href="https://redirect.github.com/1Password/arboard/pull/198">1Password/arboard#198</a>
<ul>
<li>Note: This change greatly increases compatibility for
&quot;complicated&quot; images that contain alpha values and/or
transparent pixels. Support for transparency in <code>BITMAP</code>
formats is ill-defined and inconsistently implemented in the wild, but
is consistent in <code>PNG</code>. Most applications loading images onto
the clipboard include <code>PNG</code>-encoded data already.</li>
</ul>
</li>
<li>Bitmap images pasted on Windows now use the <code>image</code> crate
instead of a homegrown internal parser.
<ul>
<li>This <strong>should not</strong> regress any existing Bitmap use
cases and instead will provide more consistent and robust parsing. If
you notice something now broken, please open an issue!</li>
</ul>
</li>
</ul>
<h3>Fixed</h3>
<ul>
<li>Remove silent dropping of file paths when non-UTF8 was mixed in on
Linux by <a href="https://github.com/Gae24"><code>@​Gae24</code></a> in
<a
href="https://redirect.github.com/1Password/arboard/pull/197">1Password/arboard#197</a></li>
<li>Fix parsing of 24-bit bitmaps on Windows by <a
href="https://github.com/wcassels"><code>@​wcassels</code></a> in <a
href="https://redirect.github.com/1Password/arboard/pull/198">1Password/arboard#198</a>
<ul>
<li>Example: Images with transparency copied by Firefox are now handled
correctly, among others.</li>
</ul>
</li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/gagath"><code>@​gagath</code></a> made
their first contribution in <a
href="https://redirect.github.com/1Password/arboard/pull/196">1Password/arboard#196</a></li>
<li><a href="https://github.com/wcassels"><code>@​wcassels</code></a>
made their first contribution in <a
href="https://redirect.github.com/1Password/arboard/pull/198">1Password/arboard#198</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/1Password/arboard/compare/v3.6.0...v3.6.1">https://github.com/1Password/arboard/compare/v3.6.0...v3.6.1</a></p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="a3750c79a5"><code>a3750c7</code></a>
Release 3.6.1</li>
<li><a
href="edcce2cd6b"><code>edcce2c</code></a>
Remove CHANGELOG.md in favor of GitHub releases</li>
<li><a
href="26a96a6199"><code>26a96a6</code></a>
Bump windows-sys semver range to support 0.60.x</li>
<li><a
href="7bdd1c1175"><code>7bdd1c1</code></a>
Update errno for windows-sys 0.60 flexibility</li>
<li><a
href="55c0b260c4"><code>55c0b26</code></a>
read/write_unaligned rather than using manual field offsets</li>
<li><a
href="ff15a093d6"><code>ff15a09</code></a>
Return conversionFailure instead of adhoc errors</li>
<li><a
href="16ef18113f"><code>16ef181</code></a>
Implement fetching PNG on Windows and prefer over DIB when
available</li>
<li><a
href="a3c64f9a93"><code>a3c64f9</code></a>
Add a couple of end-to-end DIBV5 tests</li>
<li><a
href="e6008eaa91"><code>e6008ea</code></a>
Use image for reading DIB and try to make it do the right thing for
32-bit BI...</li>
<li><a
href="17ef05ce13"><code>17ef05c</code></a>
add <code>file_list</code> to <code>Set</code> interface (<a
href="https://redirect.github.com/1Password/arboard/issues/181">#181</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/1Password/arboard/compare/v3.6.0...v3.6.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=arboard&package-manager=cargo&previous-version=3.6.0&new-version=3.6.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2025-11-30 20:20:45 -08:00
jif-oai
457c9fdb87 chore: better session recycling (#7368) 2025-11-30 12:42:26 -08:00
jif-oai
6eeaf46ac1 fix: other flaky tests (#7372) 2025-11-28 15:29:44 +00:00
jif-oai
aaec8abf58 feat: detached review (#7292) 2025-11-28 11:34:57 +00:00
Job Chong
cbd7d0d543 chore: improve rollout session init errors (#7336)
Title: Improve rollout session initialization error messages

Issue: https://github.com/openai/codex/issues/7283

What: add targeted mapping for rollout/session initialization errors so
users get actionable messages when Codex cannot access session files.

Why: session creation previously returned a generic internal error,
hiding permissions/FS issues and making support harder.

How:
- Added rollout::error::map_session_init_error to translate the more
common io::Error kinds into user-facing hints (permission, missing dir,
file blocking, corruption). Others are passed through directly with
`CodexErr::Fatal`.
- Reused the mapper in Codex session creation to preserve root causes
instead of returning InternalAgentDied.
2025-11-27 00:20:33 -08:00
Eric Traut
fabdbfef9c Fixes two bugs in example-config.md documentation (#7324)
This PR is a modified version of [a
PR](https://github.com/openai/codex/pull/7316) submitted by @yydrowz3.
* Removes a redundant `experimental_sandbox_command_assessment` flag
* Moves `mcp_oauth_credentials_store` from the `[features]` table, where
it doesn't belong
2025-11-26 09:52:13 -08:00
lionel-oai
8b314e2d04 doc: fix relative links and add tips (#7319)
This PR is a documentation only one which:
- addresses the #7231 by adding a paragraph in `docs/getting-started.md`
in the tips category to encourage users to load everything needed in
their environment
- corrects link referencing in `docs/platform-sandboxing.md` so that the
page link opens at the right section
- removes the explicit heading IDs like {#my-id} in `docs/advanced.md`
which are not supported by GitHub and are **not** rendered in the UI:

<img width="1198" height="849" alt="Screenshot 2025-11-26 at 16 25 31"
src="https://github.com/user-attachments/assets/308d33c3-81d3-4785-a6c1-e9377e6d3ea6"
/>

This caused the following links in `README.md` to not work in `main` but
to work in this branch (you can test by going to
https://github.com/openai/codex/blob/docs/getting-started-enhancement/README.md)
- the MCP link goes straight to the correct section now:

```markdown
  - [**Advanced**](./docs/advanced.md)
  - [Tracing / verbose logging](./docs/advanced.md#tracing--verbose-logging)
  - [Model Context Protocol (MCP)](./docs/advanced.md#model-context-protocol-mcp)
```

---------

Signed-off-by: lionel-oai <lionel@openai.com>
Signed-off-by: lionelchg <lionel.cheng@hotmail.fr>
Co-authored-by: lionelchg <lionel.cheng@hotmail.fr>
2025-11-26 09:35:08 -08:00
jif-oai
963009737f nit: drop file (#7314) 2025-11-26 11:30:34 +00:00
Eric Traut
e953092949 Fixed regression in experimental "sandbox command assessment" feature (#7308)
Recent model updates caused the experimental "sandbox tool assessment"
to time out most of the time leaving the user without any risk
assessment or tool summary. This change explicitly sets the reasoning
effort to medium and bumps the timeout.

This change has no effect if the user hasn't enabled the
`experimental_sandbox_command_assessment` feature flag.
2025-11-25 16:15:13 -08:00
jif-oai
28ff364c3a feat: update process ID for event handling (#7261) 2025-11-25 14:21:05 -08:00
44 changed files with 1608 additions and 709 deletions

View File

@@ -46,7 +46,4 @@ jobs:
path-to-document: https://github.com/openai/codex/blob/main/docs/CLA.md
path-to-signatures: signatures/cla.json
branch: cla-signatures
allowlist: |
codex
dependabot
dependabot[bot]
allowlist: codex,dependabot,dependabot[bot],github-actions[bot]

71
codex-rs/Cargo.lock generated
View File

@@ -198,9 +198,9 @@ dependencies = [
[[package]]
name = "arboard"
version = "3.6.0"
version = "3.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "55f533f8e0af236ffe5eb979b99381df3258853f00ba2e44b6e1955292c75227"
checksum = "0348a1c054491f4bfe6ab86a7b6ab1e44e45d899005de92f58b3df180b36ddaf"
dependencies = [
"clipboard-win",
"image",
@@ -212,7 +212,7 @@ dependencies = [
"objc2-foundation",
"parking_lot",
"percent-encoding",
"windows-sys 0.52.0",
"windows-sys 0.60.2",
"wl-clipboard-rs",
"x11rb",
]
@@ -1144,6 +1144,7 @@ dependencies = [
"codex-apply-patch",
"codex-arg0",
"codex-async-utils",
"codex-core",
"codex-execpolicy",
"codex-file-search",
"codex-git",
@@ -2535,7 +2536,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78"
dependencies = [
"cfg-if",
"rustix 1.0.8",
"windows-sys 0.52.0",
"windows-sys 0.59.0",
]
[[package]]
@@ -3292,9 +3293,9 @@ dependencies = [
[[package]]
name = "image"
version = "0.25.8"
version = "0.25.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "529feb3e6769d234375c4cf1ee2ce713682b8e76538cb13f9fc23e1400a591e7"
checksum = "e6506c6c10786659413faa717ceebcb8f70731c0a60cbae39795fdf114519c1a"
dependencies = [
"bytemuck",
"byteorder-lite",
@@ -3302,8 +3303,8 @@ dependencies = [
"num-traits",
"png",
"tiff",
"zune-core",
"zune-jpeg",
"zune-core 0.5.0",
"zune-jpeg 0.5.5",
]
[[package]]
@@ -3439,7 +3440,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9"
dependencies = [
"hermit-abi",
"libc",
"windows-sys 0.52.0",
"windows-sys 0.59.0",
]
[[package]]
@@ -5078,9 +5079,9 @@ checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c"
[[package]]
name = "reqwest"
version = "0.12.23"
version = "0.12.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d429f34c8092b2d42c7c93cec323bb4adeb7c67698f70839adec842ec10c7ceb"
checksum = "9d0946410b9f7b082a427e4ef5c8ff541a88b357bc6c637c40db3a68ac70a36f"
dependencies = [
"base64",
"bytes",
@@ -5216,7 +5217,7 @@ dependencies = [
"errno",
"libc",
"linux-raw-sys 0.4.15",
"windows-sys 0.52.0",
"windows-sys 0.59.0",
]
[[package]]
@@ -5735,9 +5736,9 @@ dependencies = [
[[package]]
name = "serde_with"
version = "3.14.0"
version = "3.16.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f2c45cd61fefa9db6f254525d46e392b852e0e61d9a1fd36e5bd183450a556d5"
checksum = "4fa237f2807440d238e0364a218270b98f767a00d3dada77b1c53ae88940e2e7"
dependencies = [
"base64",
"chrono",
@@ -5746,8 +5747,7 @@ dependencies = [
"indexmap 2.12.0",
"schemars 0.9.0",
"schemars 1.0.4",
"serde",
"serde_derive",
"serde_core",
"serde_json",
"serde_with_macros",
"time",
@@ -5755,11 +5755,11 @@ dependencies = [
[[package]]
name = "serde_with_macros"
version = "3.14.0"
version = "3.16.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "de90945e6565ce0d9a25098082ed4ee4002e047cb59892c318d66821e14bb30f"
checksum = "52a8e3ca0ca629121f70ab50f95249e5a6f925cc0f6ffe8256c45b728875706c"
dependencies = [
"darling 0.20.11",
"darling 0.21.3",
"proc-macro2",
"quote",
"syn 2.0.104",
@@ -6408,7 +6408,7 @@ dependencies = [
"half",
"quick-error",
"weezl",
"zune-jpeg",
"zune-jpeg 0.4.19",
]
[[package]]
@@ -6713,9 +6713,9 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3"
[[package]]
name = "tracing"
version = "0.1.41"
version = "0.1.43"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "784e0ac535deb450455cbfa28a6f0df145ea1bb7ae51b821cf5e7927fdcfbdd0"
checksum = "2d15d90a0b5c19378952d479dc858407149d7bb45a14de0142f6c534b16fc647"
dependencies = [
"log",
"pin-project-lite",
@@ -6737,9 +6737,9 @@ dependencies = [
[[package]]
name = "tracing-attributes"
version = "0.1.30"
version = "0.1.31"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "81383ab64e72a7a8b8e13130c49e3dab29def6d0c7d76a03087b3cf71c5c6903"
checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da"
dependencies = [
"proc-macro2",
"quote",
@@ -6748,9 +6748,9 @@ dependencies = [
[[package]]
name = "tracing-core"
version = "0.1.34"
version = "0.1.35"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b9d12581f227e93f094d3af2ae690a574abb8a2b9b7a96e7cfe9647b2b617678"
checksum = "7a04e24fab5c89c6a36eb8558c9656f30d81de51dfa4d3b45f26b21d61fa0a6c"
dependencies = [
"once_cell",
"valuable",
@@ -7368,7 +7368,7 @@ version = "0.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb"
dependencies = [
"windows-sys 0.52.0",
"windows-sys 0.59.0",
]
[[package]]
@@ -8093,13 +8093,28 @@ version = "0.4.12"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3f423a2c17029964870cfaabb1f13dfab7d092a62a29a89264f4d36990ca414a"
[[package]]
name = "zune-core"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "111f7d9820f05fd715df3144e254d6fc02ee4088b0644c0ffd0efc9e6d9d2773"
[[package]]
name = "zune-jpeg"
version = "0.4.19"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2c9e525af0a6a658e031e95f14b7f889976b74a11ba0eca5a5fc9ac8a1c43a6a"
dependencies = [
"zune-core",
"zune-core 0.4.12",
]
[[package]]
name = "zune-jpeg"
version = "0.5.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dc6fb7703e32e9a07fb3f757360338b3a567a5054f21b5f52a666752e333d58e"
dependencies = [
"zune-core 0.5.0",
]
[[package]]

View File

@@ -136,7 +136,7 @@ icu_decimal = "2.1"
icu_locale_core = "2.1"
icu_provider = { version = "2.1", features = ["sync"] }
ignore = "0.4.23"
image = { version = "^0.25.8", default-features = false }
image = { version = "^0.25.9", default-features = false }
indexmap = "2.12.0"
insta = "1.43.2"
itertools = "0.14.0"
@@ -178,7 +178,7 @@ seccompiler = "0.5.0"
sentry = "0.34.0"
serde = "1"
serde_json = "1"
serde_with = "3.14"
serde_with = "3.16"
serial_test = "3.2.0"
sha1 = "0.10.6"
sha2 = "0.10"
@@ -203,7 +203,7 @@ tokio-util = "0.7.16"
toml = "0.9.5"
toml_edit = "0.23.5"
tonic = "0.13.1"
tracing = "0.1.41"
tracing = "0.1.43"
tracing-appender = "0.2.3"
tracing-subscriber = "0.3.20"
tracing-test = "0.2.5"

View File

@@ -131,7 +131,7 @@ client_request_definitions! {
},
ReviewStart => "review/start" {
params: v2::ReviewStartParams,
response: v2::TurnStartResponse,
response: v2::ReviewStartResponse,
},
ModelList => "model/list" {
@@ -506,10 +506,12 @@ server_notification_definitions! {
TurnStarted => "turn/started" (v2::TurnStartedNotification),
TurnCompleted => "turn/completed" (v2::TurnCompletedNotification),
TurnDiffUpdated => "turn/diff/updated" (v2::TurnDiffUpdatedNotification),
TurnPlanUpdated => "turn/plan/updated" (v2::TurnPlanUpdatedNotification),
ItemStarted => "item/started" (v2::ItemStartedNotification),
ItemCompleted => "item/completed" (v2::ItemCompletedNotification),
AgentMessageDelta => "item/agentMessage/delta" (v2::AgentMessageDeltaNotification),
CommandExecutionOutputDelta => "item/commandExecution/outputDelta" (v2::CommandExecutionOutputDeltaNotification),
FileChangeOutputDelta => "item/fileChange/outputDelta" (v2::FileChangeOutputDeltaNotification),
McpToolCallProgress => "item/mcpToolCall/progress" (v2::McpToolCallProgressNotification),
AccountUpdated => "account/updated" (v2::AccountUpdatedNotification),
AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification),

View File

@@ -11,6 +11,8 @@ use codex_protocol::items::AgentMessageContent as CoreAgentMessageContent;
use codex_protocol::items::TurnItem as CoreTurnItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand;
use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg;
use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus;
use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo;
use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot;
use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot;
@@ -130,6 +132,12 @@ v2_enum_from_core!(
}
);
v2_enum_from_core!(
pub enum ReviewDelivery from codex_protocol::protocol::ReviewDelivery {
Inline, Detached
}
);
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
@@ -908,9 +916,22 @@ pub struct ReviewStartParams {
pub thread_id: String,
pub target: ReviewTarget,
/// When true, also append the final review message to the original thread.
/// Where to run the review: inline (default) on the current thread or
/// detached on a new thread (returned in `reviewThreadId`).
#[serde(default)]
pub append_to_original_thread: bool,
pub delivery: Option<ReviewDelivery>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReviewStartResponse {
pub turn: Turn,
/// Identifies the thread where the review runs.
///
/// For inline reviews, this is the original thread id.
/// For detached reviews, this is the id of the new review thread.
pub review_thread_id: String,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -1020,6 +1041,8 @@ pub enum ThreadItem {
command: String,
/// The command's working directory.
cwd: PathBuf,
/// Identifier for the underlying PTY process (when available).
process_id: Option<String>,
status: CommandExecutionStatus,
/// A best-effort parsing of the command to understand the action(s) it will perform.
/// This returns a list of CommandAction objects because a single shell command may
@@ -1061,7 +1084,10 @@ pub enum ThreadItem {
ImageView { id: String, path: String },
#[serde(rename_all = "camelCase")]
#[ts(rename_all = "camelCase")]
CodeReview { id: String, review: String },
EnteredReviewMode { id: String, review: String },
#[serde(rename_all = "camelCase")]
#[ts(rename_all = "camelCase")]
ExitedReviewMode { id: String, review: String },
}
impl From<CoreTurnItem> for ThreadItem {
@@ -1120,7 +1146,11 @@ pub struct FileUpdateChange {
pub enum PatchChangeKind {
Add,
Delete,
Update { move_path: Option<PathBuf> },
Update {
move_path: Option<PathBuf>,
old_content: String,
new_content: String,
},
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -1206,10 +1236,56 @@ pub struct TurnCompletedNotification {
/// Notification that the turn-level unified diff has changed.
/// Contains the latest aggregated diff across all file changes in the turn.
pub struct TurnDiffUpdatedNotification {
pub thread_id: String,
pub turn_id: String,
pub diff: String,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct TurnPlanUpdatedNotification {
pub turn_id: String,
pub explanation: Option<String>,
pub plan: Vec<TurnPlanStep>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct TurnPlanStep {
pub step: String,
pub status: TurnPlanStepStatus,
}
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub enum TurnPlanStepStatus {
Pending,
InProgress,
Completed,
}
impl From<CorePlanItemArg> for TurnPlanStep {
fn from(value: CorePlanItemArg) -> Self {
Self {
step: value.step,
status: value.status.into(),
}
}
}
impl From<CorePlanStepStatus> for TurnPlanStepStatus {
fn from(value: CorePlanStepStatus) -> Self {
match value {
CorePlanStepStatus::Pending => Self::Pending,
CorePlanStepStatus::InProgress => Self::InProgress,
CorePlanStepStatus::Completed => Self::Completed,
}
}
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
@@ -1233,6 +1309,8 @@ pub struct ItemCompletedNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct AgentMessageDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
}
@@ -1241,6 +1319,8 @@ pub struct AgentMessageDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningSummaryTextDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
pub summary_index: i64,
@@ -1250,6 +1330,8 @@ pub struct ReasoningSummaryTextDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningSummaryPartAddedNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub summary_index: i64,
}
@@ -1258,6 +1340,8 @@ pub struct ReasoningSummaryPartAddedNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningTextDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
pub content_index: i64,
@@ -1267,6 +1351,18 @@ pub struct ReasoningTextDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct CommandExecutionOutputDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct FileChangeOutputDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
}
@@ -1275,6 +1371,8 @@ pub struct CommandExecutionOutputDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct McpToolCallProgressNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub message: String,
}

View File

@@ -65,7 +65,7 @@ The JSON-RPC API exposes dedicated methods for managing Codex conversations. Thr
- `thread/archive` — move a threads rollout file into the archived directory; returns `{}` on success.
- `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications.
- `turn/interrupt` — request cancellation of an in-flight turn by `(thread_id, turn_id)`; success is an empty `{}` response and the turn finishes with `status: "interrupted"`.
- `review/start` — kick off Codexs automated reviewer for a thread; responds like `turn/start` and emits a `item/completed` notification with a `codeReview` item when results are ready.
- `review/start` — kick off Codexs automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review.
### 1) Start or resume a thread
@@ -190,49 +190,56 @@ Use `review/start` to run Codexs reviewer on the currently checked-out projec
- `{"type":"baseBranch","branch":"main"}` — diff against the provided branchs upstream (see prompt for the exact `git merge-base`/`git diff` instructions Codex will run).
- `{"type":"commit","sha":"abc1234","title":"Optional subject"}` — review a specific commit.
- `{"type":"custom","instructions":"Free-form reviewer instructions"}` — fallback prompt equivalent to the legacy manual review request.
- `appendToOriginalThread` (bool, default `false`) — when `true`, Codex also records a final assistant-style message with the review summary in the original thread. When `false`, only the `codeReview` item is emitted for the review run and no extra message is added to the original thread.
- `delivery` (`"inline"` or `"detached"`, default `"inline"`) — where the review runs:
- `"inline"`: run the review as a new turn on the existing thread. The responses `reviewThreadId` equals the original `threadId`, and no new `thread/started` notification is emitted.
- `"detached"`: fork a new review thread from the parent conversation and run the review there. The responses `reviewThreadId` is the id of this new review thread, and the server emits a `thread/started` notification for it before streaming review items.
Example request/response:
```json
{ "method": "review/start", "id": 40, "params": {
"threadId": "thr_123",
"appendToOriginalThread": true,
"delivery": "inline",
"target": { "type": "commit", "sha": "1234567deadbeef", "title": "Polish tui colors" }
} }
{ "id": 40, "result": { "turn": {
"id": "turn_900",
"status": "inProgress",
"items": [
{ "type": "userMessage", "id": "turn_900", "content": [ { "type": "text", "text": "Review commit 1234567: Polish tui colors" } ] }
],
"error": null
} } }
{ "id": 40, "result": {
"turn": {
"id": "turn_900",
"status": "inProgress",
"items": [
{ "type": "userMessage", "id": "turn_900", "content": [ { "type": "text", "text": "Review commit 1234567: Polish tui colors" } ] }
],
"error": null
},
"reviewThreadId": "thr_123"
} }
```
For a detached review, use `"delivery": "detached"`. The response is the same shape, but `reviewThreadId` will be the id of the new review thread (different from the original `threadId`). The server also emits a `thread/started` notification for that new thread before streaming the review turn.
Codex streams the usual `turn/started` notification followed by an `item/started`
with the same `codeReview` item id so clients can show progress:
with an `enteredReviewMode` item so clients can show progress:
```json
{ "method": "item/started", "params": { "item": {
"type": "codeReview",
"type": "enteredReviewMode",
"id": "turn_900",
"review": "current changes"
} } }
```
When the reviewer finishes, the server emits `item/completed` containing the same
`codeReview` item with the final review text:
When the reviewer finishes, the server emits `item/started` and `item/completed`
containing an `exitedReviewMode` item with the final review text:
```json
{ "method": "item/completed", "params": { "item": {
"type": "codeReview",
"type": "exitedReviewMode",
"id": "turn_900",
"review": "Looks solid overall...\n\n- Prefer Stylize helpers — app.rs:10-20\n ..."
} } }
```
The `review` string is plain text that already bundles the overall explanation plus a bullet list for each structured finding (matching `ThreadItem::CodeReview` in the generated schema). Use this notification to render the reviewer output in your client.
The `review` string is plain text that already bundles the overall explanation plus a bullet list for each structured finding (matching `ThreadItem::ExitedReviewMode` in the generated schema). Use this notification to render the reviewer output in your client.
## Events (work-in-progress)
@@ -244,6 +251,7 @@ The app-server streams JSON-RPC notifications while a turn is running. Each turn
- `turn/started``{ turn }` with the turn id, empty `items`, and `status: "inProgress"`.
- `turn/completed``{ turn }` where `turn.status` is `completed`, `interrupted`, or `failed`; failures carry `{ error: { message, codexErrorInfo? } }`.
- `turn/plan/updated``{ turnId, explanation?, plan }` whenever the agent shares or changes its plan; each `plan` entry is `{ step, status }` with `status` in `pending`, `inProgress`, or `completed`.
Today both notifications carry an empty `items` array even when item events were streamed; rely on `item/*` notifications for the canonical item list until this is fixed.

View File

@@ -18,6 +18,7 @@ use codex_app_server_protocol::ContextCompactedNotification;
use codex_app_server_protocol::ErrorNotification;
use codex_app_server_protocol::ExecCommandApprovalParams;
use codex_app_server_protocol::ExecCommandApprovalResponse;
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
use codex_app_server_protocol::FileChangeRequestApprovalParams;
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
use codex_app_server_protocol::FileUpdateChange;
@@ -43,6 +44,8 @@ use codex_app_server_protocol::TurnCompletedNotification;
use codex_app_server_protocol::TurnDiffUpdatedNotification;
use codex_app_server_protocol::TurnError;
use codex_app_server_protocol::TurnInterruptResponse;
use codex_app_server_protocol::TurnPlanStep;
use codex_app_server_protocol::TurnPlanUpdatedNotification;
use codex_app_server_protocol::TurnStatus;
use codex_core::CodexConversation;
use codex_core::parse_command::shlex_join;
@@ -60,6 +63,7 @@ use codex_core::protocol::TokenCountEvent;
use codex_core::protocol::TurnDiffEvent;
use codex_core::review_format::format_review_findings_block;
use codex_protocol::ConversationId;
use codex_protocol::plan_tool::UpdatePlanArgs;
use codex_protocol::protocol::ReviewOutputEvent;
use std::collections::HashMap;
use std::convert::TryFrom;
@@ -257,6 +261,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::AgentMessageContentDelta(event) => {
let notification = AgentMessageDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
};
@@ -275,6 +281,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::ReasoningContentDelta(event) => {
let notification = ReasoningSummaryTextDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
summary_index: event.summary_index,
@@ -287,6 +295,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::ReasoningRawContentDelta(event) => {
let notification = ReasoningTextDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
content_index: event.content_index,
@@ -297,6 +307,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::AgentReasoningSectionBreak(event) => {
let notification = ReasoningSummaryPartAddedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
summary_index: event.summary_index,
};
@@ -340,16 +352,26 @@ pub(crate) async fn apply_bespoke_event_handling(
.await;
}
EventMsg::EnteredReviewMode(review_request) => {
let notification = ItemStartedNotification {
let review = review_request.user_facing_hint;
let item = ThreadItem::EnteredReviewMode {
id: event_turn_id.clone(),
review,
};
let started = ItemStartedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item: ThreadItem::CodeReview {
id: event_turn_id.clone(),
review: review_request.user_facing_hint,
},
item: item.clone(),
};
outgoing
.send_server_notification(ServerNotification::ItemStarted(notification))
.send_server_notification(ServerNotification::ItemStarted(started))
.await;
let completed = ItemCompletedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item,
};
outgoing
.send_server_notification(ServerNotification::ItemCompleted(completed))
.await;
}
EventMsg::ItemStarted(item_started_event) => {
@@ -375,21 +397,29 @@ pub(crate) async fn apply_bespoke_event_handling(
.await;
}
EventMsg::ExitedReviewMode(review_event) => {
let review_text = match review_event.review_output {
let review = match review_event.review_output {
Some(output) => render_review_output_text(&output),
None => REVIEW_FALLBACK_MESSAGE.to_string(),
};
let review_item_id = event_turn_id.clone();
let notification = ItemCompletedNotification {
let item = ThreadItem::ExitedReviewMode {
id: event_turn_id.clone(),
review,
};
let started = ItemStartedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item: ThreadItem::CodeReview {
id: review_item_id,
review: review_text,
},
item: item.clone(),
};
outgoing
.send_server_notification(ServerNotification::ItemCompleted(notification))
.send_server_notification(ServerNotification::ItemStarted(started))
.await;
let completed = ItemCompletedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item,
};
outgoing
.send_server_notification(ServerNotification::ItemCompleted(completed))
.await;
}
EventMsg::PatchApplyBegin(patch_begin_event) => {
@@ -449,11 +479,13 @@ pub(crate) async fn apply_bespoke_event_handling(
.collect::<Vec<_>>();
let command = shlex_join(&exec_command_begin_event.command);
let cwd = exec_command_begin_event.cwd;
let process_id = exec_command_begin_event.process_id;
let item = ThreadItem::CommandExecution {
id: item_id,
command,
cwd,
process_id,
status: CommandExecutionStatus::InProgress,
command_actions,
aggregated_output: None,
@@ -470,15 +502,44 @@ pub(crate) async fn apply_bespoke_event_handling(
.await;
}
EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => {
let notification = CommandExecutionOutputDeltaNotification {
item_id: exec_command_output_delta_event.call_id.clone(),
delta: String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string(),
let item_id = exec_command_output_delta_event.call_id.clone();
let delta = String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string();
// The underlying EventMsg::ExecCommandOutputDelta is used for shell, unified_exec,
// and apply_patch tool calls. We represent apply_patch with the FileChange item, and
// everything else with the CommandExecution item.
//
// We need to detect which item type it is so we can emit the right notification.
// We already have state tracking FileChange items on item/started, so let's use that.
let is_file_change = {
let map = turn_summary_store.lock().await;
map.get(&conversation_id)
.is_some_and(|summary| summary.file_change_started.contains(&item_id))
};
outgoing
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
notification,
))
.await;
if is_file_change {
let notification = FileChangeOutputDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id,
delta,
};
outgoing
.send_server_notification(ServerNotification::FileChangeOutputDelta(
notification,
))
.await;
} else {
let notification = CommandExecutionOutputDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id,
delta,
};
outgoing
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
notification,
))
.await;
}
}
EventMsg::ExecCommandEnd(exec_command_end_event) => {
let ExecCommandEndEvent {
@@ -486,6 +547,7 @@ pub(crate) async fn apply_bespoke_event_handling(
command,
cwd,
parsed_cmd,
process_id,
aggregated_output,
exit_code,
duration,
@@ -514,6 +576,7 @@ pub(crate) async fn apply_bespoke_event_handling(
id: call_id,
command: shlex_join(&command),
cwd,
process_id,
status,
command_actions,
aggregated_output,
@@ -563,6 +626,7 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::TurnDiff(turn_diff_event) => {
handle_turn_diff(
conversation_id,
&event_turn_id,
turn_diff_event,
api_version,
@@ -570,12 +634,22 @@ pub(crate) async fn apply_bespoke_event_handling(
)
.await;
}
EventMsg::PlanUpdate(plan_update_event) => {
handle_turn_plan_update(
&event_turn_id,
plan_update_event,
api_version,
outgoing.as_ref(),
)
.await;
}
_ => {}
}
}
async fn handle_turn_diff(
conversation_id: ConversationId,
event_turn_id: &str,
turn_diff_event: TurnDiffEvent,
api_version: ApiVersion,
@@ -583,6 +657,7 @@ async fn handle_turn_diff(
) {
if let ApiVersion::V2 = api_version {
let notification = TurnDiffUpdatedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.to_string(),
diff: turn_diff_event.unified_diff,
};
@@ -592,6 +667,28 @@ async fn handle_turn_diff(
}
}
async fn handle_turn_plan_update(
event_turn_id: &str,
plan_update_event: UpdatePlanArgs,
api_version: ApiVersion,
outgoing: &OutgoingMessageSender,
) {
if let ApiVersion::V2 = api_version {
let notification = TurnPlanUpdatedNotification {
turn_id: event_turn_id.to_string(),
explanation: plan_update_event.explanation,
plan: plan_update_event
.plan
.into_iter()
.map(TurnPlanStep::from)
.collect(),
};
outgoing
.send_server_notification(ServerNotification::TurnPlanUpdated(notification))
.await;
}
}
async fn emit_turn_completed_with_status(
conversation_id: ConversationId,
event_turn_id: String,
@@ -649,6 +746,7 @@ async fn complete_command_execution_item(
item_id: String,
command: String,
cwd: PathBuf,
process_id: Option<String>,
command_actions: Vec<V2ParsedCommand>,
status: CommandExecutionStatus,
outgoing: &OutgoingMessageSender,
@@ -657,6 +755,7 @@ async fn complete_command_execution_item(
id: item_id,
command,
cwd,
process_id,
status,
command_actions,
aggregated_output: None,
@@ -869,8 +968,15 @@ fn map_patch_change_kind(change: &CoreFileChange) -> V2PatchChangeKind {
match change {
CoreFileChange::Add { .. } => V2PatchChangeKind::Add,
CoreFileChange::Delete { .. } => V2PatchChangeKind::Delete,
CoreFileChange::Update { move_path, .. } => V2PatchChangeKind::Update {
CoreFileChange::Update {
move_path,
old_content,
new_content,
..
} => V2PatchChangeKind::Update {
move_path: move_path.clone(),
old_content: old_content.clone(),
new_content: new_content.clone(),
},
}
}
@@ -882,6 +988,7 @@ fn format_file_change_diff(change: &CoreFileChange) -> String {
CoreFileChange::Update {
unified_diff,
move_path,
..
} => {
if let Some(path) = move_path {
format!("{unified_diff}\n\nMoved to: {}", path.display())
@@ -1015,6 +1122,7 @@ async fn on_command_execution_request_approval_response(
item_id.clone(),
command.clone(),
cwd.clone(),
None,
command_actions.clone(),
status,
outgoing.as_ref(),
@@ -1108,12 +1216,15 @@ mod tests {
use anyhow::Result;
use anyhow::anyhow;
use anyhow::bail;
use codex_app_server_protocol::TurnPlanStepStatus;
use codex_core::protocol::CreditsSnapshot;
use codex_core::protocol::McpInvocation;
use codex_core::protocol::RateLimitSnapshot;
use codex_core::protocol::RateLimitWindow;
use codex_core::protocol::TokenUsage;
use codex_core::protocol::TokenUsageInfo;
use codex_protocol::plan_tool::PlanItemArg;
use codex_protocol::plan_tool::StepStatus;
use mcp_types::CallToolResult;
use mcp_types::ContentBlock;
use mcp_types::TextContent;
@@ -1273,6 +1384,46 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn test_handle_turn_plan_update_emits_notification_for_v2() -> Result<()> {
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = OutgoingMessageSender::new(tx);
let update = UpdatePlanArgs {
explanation: Some("need plan".to_string()),
plan: vec![
PlanItemArg {
step: "first".to_string(),
status: StepStatus::Pending,
},
PlanItemArg {
step: "second".to_string(),
status: StepStatus::Completed,
},
],
};
handle_turn_plan_update("turn-123", update, ApiVersion::V2, &outgoing).await;
let msg = rx
.recv()
.await
.ok_or_else(|| anyhow!("should send one notification"))?;
match msg {
OutgoingMessage::AppServerNotification(ServerNotification::TurnPlanUpdated(n)) => {
assert_eq!(n.turn_id, "turn-123");
assert_eq!(n.explanation.as_deref(), Some("need plan"));
assert_eq!(n.plan.len(), 2);
assert_eq!(n.plan[0].step, "first");
assert_eq!(n.plan[0].status, TurnPlanStepStatus::Pending);
assert_eq!(n.plan[1].step, "second");
assert_eq!(n.plan[1].status, TurnPlanStepStatus::Completed);
}
other => bail!("unexpected message: {other:?}"),
}
assert!(rx.try_recv().is_err(), "no extra messages expected");
Ok(())
}
#[tokio::test]
async fn test_handle_token_count_event_emits_usage_and_rate_limits() -> Result<()> {
let conversation_id = ConversationId::new();
@@ -1672,8 +1823,10 @@ mod tests {
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = OutgoingMessageSender::new(tx);
let unified_diff = "--- a\n+++ b\n".to_string();
let conversation_id = ConversationId::new();
handle_turn_diff(
conversation_id,
"turn-1",
TurnDiffEvent {
unified_diff: unified_diff.clone(),
@@ -1691,6 +1844,7 @@ mod tests {
OutgoingMessage::AppServerNotification(ServerNotification::TurnDiffUpdated(
notification,
)) => {
assert_eq!(notification.thread_id, conversation_id.to_string());
assert_eq!(notification.turn_id, "turn-1");
assert_eq!(notification.diff, unified_diff);
}
@@ -1704,8 +1858,10 @@ mod tests {
async fn test_handle_turn_diff_is_noop_for_v1() -> Result<()> {
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = OutgoingMessageSender::new(tx);
let conversation_id = ConversationId::new();
handle_turn_diff(
conversation_id,
"turn-1",
TurnDiffEvent {
unified_diff: "diff".to_string(),

View File

@@ -61,7 +61,9 @@ use codex_app_server_protocol::RemoveConversationSubscriptionResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ResumeConversationParams;
use codex_app_server_protocol::ResumeConversationResponse;
use codex_app_server_protocol::ReviewDelivery as ApiReviewDelivery;
use codex_app_server_protocol::ReviewStartParams;
use codex_app_server_protocol::ReviewStartResponse;
use codex_app_server_protocol::ReviewTarget;
use codex_app_server_protocol::SandboxMode;
use codex_app_server_protocol::SendUserMessageParams;
@@ -120,6 +122,7 @@ use codex_core::git_info::git_diff_to_remote;
use codex_core::parse_cursor;
use codex_core::protocol::EventMsg;
use codex_core::protocol::Op;
use codex_core::protocol::ReviewDelivery as CoreReviewDelivery;
use codex_core::protocol::ReviewRequest;
use codex_core::protocol::SessionConfiguredEvent;
use codex_core::read_head_for_summary;
@@ -253,7 +256,6 @@ impl CodexMessageProcessor {
fn review_request_from_target(
target: ReviewTarget,
append_to_original_thread: bool,
) -> Result<(ReviewRequest, String), JSONRPCErrorError> {
fn invalid_request(message: String) -> JSONRPCErrorError {
JSONRPCErrorError {
@@ -269,7 +271,6 @@ impl CodexMessageProcessor {
ReviewRequest {
prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(),
user_facing_hint: "current changes".to_string(),
append_to_original_thread,
},
"Review uncommitted changes".to_string(),
)),
@@ -285,7 +286,6 @@ impl CodexMessageProcessor {
ReviewRequest {
prompt,
user_facing_hint: hint,
append_to_original_thread,
},
display,
))
@@ -314,7 +314,6 @@ impl CodexMessageProcessor {
ReviewRequest {
prompt,
user_facing_hint: hint,
append_to_original_thread,
},
display,
))
@@ -328,7 +327,6 @@ impl CodexMessageProcessor {
ReviewRequest {
prompt: trimmed.clone(),
user_facing_hint: trimmed.clone(),
append_to_original_thread,
},
trimmed,
))
@@ -2497,60 +2495,220 @@ impl CodexMessageProcessor {
}
}
async fn review_start(&self, request_id: RequestId, params: ReviewStartParams) {
fn build_review_turn(turn_id: String, display_text: &str) -> Turn {
let items = if display_text.is_empty() {
Vec::new()
} else {
vec![ThreadItem::UserMessage {
id: turn_id.clone(),
content: vec![V2UserInput::Text {
text: display_text.to_string(),
}],
}]
};
Turn {
id: turn_id,
items,
status: TurnStatus::InProgress,
}
}
async fn emit_review_started(
&self,
request_id: &RequestId,
turn: Turn,
parent_thread_id: String,
review_thread_id: String,
) {
let response = ReviewStartResponse {
turn: turn.clone(),
review_thread_id,
};
self.outgoing
.send_response(request_id.clone(), response)
.await;
let notif = TurnStartedNotification {
thread_id: parent_thread_id,
turn,
};
self.outgoing
.send_server_notification(ServerNotification::TurnStarted(notif))
.await;
}
async fn start_inline_review(
&self,
request_id: &RequestId,
parent_conversation: Arc<CodexConversation>,
review_request: ReviewRequest,
display_text: &str,
parent_thread_id: String,
) -> std::result::Result<(), JSONRPCErrorError> {
let turn_id = parent_conversation
.submit(Op::Review { review_request })
.await;
match turn_id {
Ok(turn_id) => {
let turn = Self::build_review_turn(turn_id, display_text);
self.emit_review_started(
request_id,
turn,
parent_thread_id.clone(),
parent_thread_id,
)
.await;
Ok(())
}
Err(err) => Err(JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("failed to start review: {err}"),
data: None,
}),
}
}
async fn start_detached_review(
&mut self,
request_id: &RequestId,
parent_conversation_id: ConversationId,
review_request: ReviewRequest,
display_text: &str,
) -> std::result::Result<(), JSONRPCErrorError> {
let rollout_path = find_conversation_path_by_id_str(
&self.config.codex_home,
&parent_conversation_id.to_string(),
)
.await
.map_err(|err| JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("failed to locate conversation id {parent_conversation_id}: {err}"),
data: None,
})?
.ok_or_else(|| JSONRPCErrorError {
code: INVALID_REQUEST_ERROR_CODE,
message: format!("no rollout found for conversation id {parent_conversation_id}"),
data: None,
})?;
let mut config = self.config.as_ref().clone();
config.model = self.config.review_model.clone();
let NewConversation {
conversation_id,
conversation,
session_configured,
..
} = self
.conversation_manager
.fork_conversation(usize::MAX, config, rollout_path)
.await
.map_err(|err| JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("error creating detached review conversation: {err}"),
data: None,
})?;
if let Err(err) = self
.attach_conversation_listener(conversation_id, false, ApiVersion::V2)
.await
{
tracing::warn!(
"failed to attach listener for review conversation {}: {}",
conversation_id,
err.message
);
}
let rollout_path = conversation.rollout_path();
let fallback_provider = self.config.model_provider_id.as_str();
match read_summary_from_rollout(rollout_path.as_path(), fallback_provider).await {
Ok(summary) => {
let thread = summary_to_thread(summary);
let notif = ThreadStartedNotification { thread };
self.outgoing
.send_server_notification(ServerNotification::ThreadStarted(notif))
.await;
}
Err(err) => {
tracing::warn!(
"failed to load summary for review conversation {}: {}",
session_configured.session_id,
err
);
}
}
let turn_id = conversation
.submit(Op::Review { review_request })
.await
.map_err(|err| JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("failed to start detached review turn: {err}"),
data: None,
})?;
let turn = Self::build_review_turn(turn_id, display_text);
let review_thread_id = conversation_id.to_string();
self.emit_review_started(request_id, turn, review_thread_id.clone(), review_thread_id)
.await;
Ok(())
}
async fn review_start(&mut self, request_id: RequestId, params: ReviewStartParams) {
let ReviewStartParams {
thread_id,
target,
append_to_original_thread,
delivery,
} = params;
let (_, conversation) = match self.conversation_from_thread_id(&thread_id).await {
Ok(v) => v,
Err(error) => {
self.outgoing.send_error(request_id, error).await;
return;
}
};
let (review_request, display_text) =
match Self::review_request_from_target(target, append_to_original_thread) {
Ok(value) => value,
Err(err) => {
self.outgoing.send_error(request_id, err).await;
let (parent_conversation_id, parent_conversation) =
match self.conversation_from_thread_id(&thread_id).await {
Ok(v) => v,
Err(error) => {
self.outgoing.send_error(request_id, error).await;
return;
}
};
let turn_id = conversation.submit(Op::Review { review_request }).await;
match turn_id {
Ok(turn_id) => {
let mut items = Vec::new();
if !display_text.is_empty() {
items.push(ThreadItem::UserMessage {
id: turn_id.clone(),
content: vec![V2UserInput::Text { text: display_text }],
});
}
let turn = Turn {
id: turn_id.clone(),
items,
status: TurnStatus::InProgress,
};
let response = TurnStartResponse { turn: turn.clone() };
self.outgoing.send_response(request_id, response).await;
let notif = TurnStartedNotification { thread_id, turn };
self.outgoing
.send_server_notification(ServerNotification::TurnStarted(notif))
.await;
}
let (review_request, display_text) = match Self::review_request_from_target(target) {
Ok(value) => value,
Err(err) => {
let error = JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!("failed to start review: {err}"),
data: None,
};
self.outgoing.send_error(request_id, error).await;
self.outgoing.send_error(request_id, err).await;
return;
}
};
let delivery = delivery.unwrap_or(ApiReviewDelivery::Inline).to_core();
match delivery {
CoreReviewDelivery::Inline => {
if let Err(err) = self
.start_inline_review(
&request_id,
parent_conversation,
review_request,
display_text.as_str(),
thread_id.clone(),
)
.await
{
self.outgoing.send_error(request_id, err).await;
}
}
CoreReviewDelivery::Detached => {
if let Err(err) = self
.start_detached_review(
&request_id,
parent_conversation_id,
review_request,
display_text.as_str(),
)
.await
{
self.outgoing.send_error(request_id, err).await;
}
}
}
}

View File

@@ -15,6 +15,7 @@ pub use mcp_process::McpProcess;
pub use mock_model_server::create_mock_chat_completions_server;
pub use mock_model_server::create_mock_chat_completions_server_unchecked;
pub use responses::create_apply_patch_sse_response;
pub use responses::create_exec_command_sse_response;
pub use responses::create_final_assistant_message_sse_response;
pub use responses::create_shell_command_sse_response;
pub use rollout::create_fake_rollout;

View File

@@ -94,3 +94,42 @@ pub fn create_apply_patch_sse_response(
);
Ok(sse)
}
pub fn create_exec_command_sse_response(call_id: &str) -> anyhow::Result<String> {
let (cmd, args) = if cfg!(windows) {
("cmd.exe", vec!["/d", "/c", "echo hi"])
} else {
("/bin/sh", vec!["-c", "echo hi"])
};
let command = std::iter::once(cmd.to_string())
.chain(args.into_iter().map(str::to_string))
.collect::<Vec<_>>();
let tool_call_arguments = serde_json::to_string(&json!({
"cmd": command.join(" "),
"yield_time_ms": 500
}))?;
let tool_call = json!({
"choices": [
{
"delta": {
"tool_calls": [
{
"id": call_id,
"function": {
"name": "exec_command",
"arguments": tool_call_arguments
}
}
]
},
"finish_reason": "tool_calls"
}
]
});
let sse = format!(
"data: {}\n\ndata: DONE\n\n",
serde_json::to_string(&tool_call)?
);
Ok(sse)
}

View File

@@ -9,12 +9,13 @@ use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::JSONRPCNotification;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ReviewDelivery;
use codex_app_server_protocol::ReviewStartParams;
use codex_app_server_protocol::ReviewStartResponse;
use codex_app_server_protocol::ReviewTarget;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::TurnStatus;
use serde_json::json;
use tempfile::TempDir;
@@ -59,7 +60,7 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
let review_req = mcp
.send_review_start_request(ReviewStartParams {
thread_id: thread_id.clone(),
append_to_original_thread: true,
delivery: Some(ReviewDelivery::Inline),
target: ReviewTarget::Commit {
sha: "1234567deadbeef".to_string(),
title: Some("Tidy UI colors".to_string()),
@@ -71,43 +72,43 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
mcp.read_stream_until_response_message(RequestId::Integer(review_req)),
)
.await??;
let TurnStartResponse { turn } = to_response::<TurnStartResponse>(review_resp)?;
let ReviewStartResponse {
turn,
review_thread_id,
} = to_response::<ReviewStartResponse>(review_resp)?;
assert_eq!(review_thread_id, thread_id.clone());
let turn_id = turn.id.clone();
assert_eq!(turn.status, TurnStatus::InProgress);
assert_eq!(turn.items.len(), 1);
match &turn.items[0] {
ThreadItem::UserMessage { content, .. } => {
assert_eq!(content.len(), 1);
assert!(matches!(
&content[0],
codex_app_server_protocol::UserInput::Text { .. }
));
}
other => panic!("expected user message, got {other:?}"),
}
let _started: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/started"),
)
.await??;
let item_started: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("item/started"),
)
.await??;
let started: ItemStartedNotification =
serde_json::from_value(item_started.params.expect("params must be present"))?;
match started.item {
ThreadItem::CodeReview { id, review } => {
assert_eq!(id, turn_id);
assert_eq!(review, "commit 1234567");
// Confirm we see the EnteredReviewMode marker on the main thread.
let mut saw_entered_review_mode = false;
for _ in 0..10 {
let item_started: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("item/started"),
)
.await??;
let started: ItemStartedNotification =
serde_json::from_value(item_started.params.expect("params must be present"))?;
match started.item {
ThreadItem::EnteredReviewMode { id, review } => {
assert_eq!(id, turn_id);
assert_eq!(review, "commit 1234567");
saw_entered_review_mode = true;
break;
}
_ => continue,
}
other => panic!("expected code review item, got {other:?}"),
}
assert!(
saw_entered_review_mode,
"did not observe enteredReviewMode item"
);
// Confirm we see the ExitedReviewMode marker (with review text)
// on the same turn. Ignore any other items the stream surfaces.
let mut review_body: Option<String> = None;
for _ in 0..5 {
for _ in 0..10 {
let review_notif: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("item/completed"),
@@ -116,13 +117,12 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
let completed: ItemCompletedNotification =
serde_json::from_value(review_notif.params.expect("params must be present"))?;
match completed.item {
ThreadItem::CodeReview { id, review } => {
ThreadItem::ExitedReviewMode { id, review } => {
assert_eq!(id, turn_id);
review_body = Some(review);
break;
}
ThreadItem::UserMessage { .. } => continue,
other => panic!("unexpected item/completed payload: {other:?}"),
_ => continue,
}
}
@@ -146,7 +146,7 @@ async fn review_start_rejects_empty_base_branch() -> Result<()> {
let request_id = mcp
.send_review_start_request(ReviewStartParams {
thread_id,
append_to_original_thread: true,
delivery: Some(ReviewDelivery::Inline),
target: ReviewTarget::BaseBranch {
branch: " ".to_string(),
},
@@ -167,6 +167,56 @@ async fn review_start_rejects_empty_base_branch() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn review_start_with_detached_delivery_returns_new_thread_id() -> Result<()> {
let review_payload = json!({
"findings": [],
"overall_correctness": "ok",
"overall_explanation": "detached review",
"overall_confidence_score": 0.5
})
.to_string();
let responses = vec![create_final_assistant_message_sse_response(
&review_payload,
)?];
let server = create_mock_chat_completions_server_unchecked(responses).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let thread_id = start_default_thread(&mut mcp).await?;
let review_req = mcp
.send_review_start_request(ReviewStartParams {
thread_id: thread_id.clone(),
delivery: Some(ReviewDelivery::Detached),
target: ReviewTarget::Custom {
instructions: "detached review".to_string(),
},
})
.await?;
let review_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(review_req)),
)
.await??;
let ReviewStartResponse {
turn,
review_thread_id,
} = to_response::<ReviewStartResponse>(review_resp)?;
assert_eq!(turn.status, TurnStatus::InProgress);
assert_ne!(
review_thread_id, thread_id,
"detached review should run on a different thread"
);
Ok(())
}
#[tokio::test]
async fn review_start_rejects_empty_commit_sha() -> Result<()> {
let server = create_mock_chat_completions_server_unchecked(vec![]).await;
@@ -180,7 +230,7 @@ async fn review_start_rejects_empty_commit_sha() -> Result<()> {
let request_id = mcp
.send_review_start_request(ReviewStartParams {
thread_id,
append_to_original_thread: true,
delivery: Some(ReviewDelivery::Inline),
target: ReviewTarget::Commit {
sha: "\t".to_string(),
title: None,
@@ -215,7 +265,7 @@ async fn review_start_rejects_empty_custom_instructions() -> Result<()> {
let request_id = mcp
.send_review_start_request(ReviewStartParams {
thread_id,
append_to_original_thread: true,
delivery: Some(ReviewDelivery::Inline),
target: ReviewTarget::Custom {
instructions: "\n\n".to_string(),
},

View File

@@ -1,6 +1,7 @@
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::create_apply_patch_sse_response;
use app_test_support::create_exec_command_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_chat_completions_server;
use app_test_support::create_mock_chat_completions_server_unchecked;
@@ -10,6 +11,7 @@ use app_test_support::to_response;
use codex_app_server_protocol::ApprovalDecision;
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
use codex_app_server_protocol::CommandExecutionStatus;
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::ItemStartedNotification;
@@ -627,10 +629,19 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
*** Add File: README.md
+new line
*** End Patch
"#;
let update_patch = r#"*** Begin Patch
*** Update File: README.md
@@
-new line
+updated line
*** End Patch
"#;
let responses = vec![
create_apply_patch_sse_response(patch, "patch-call")?,
create_final_assistant_message_sse_response("patch applied")?,
create_apply_patch_sse_response(update_patch, "patch-call-2")?,
create_final_assistant_message_sse_response("patch updated")?,
];
let server = create_mock_chat_completions_server(responses).await;
create_config_toml(&codex_home, &server.uri(), "untrusted")?;
@@ -705,8 +716,8 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
assert_eq!(params.item_id, "patch-call");
assert_eq!(params.thread_id, thread.id);
assert_eq!(params.turn_id, turn.id);
let expected_readme_path = workspace.join("README.md");
let expected_readme_path = expected_readme_path.to_string_lossy().into_owned();
let readme_path = workspace.join("README.md");
let expected_readme_path = readme_path.to_string_lossy().into_owned();
pretty_assertions::assert_eq!(
started_changes,
vec![codex_app_server_protocol::FileUpdateChange {
@@ -724,6 +735,26 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
)
.await?;
let output_delta_notif = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
)
.await??;
let output_delta: FileChangeOutputDeltaNotification = serde_json::from_value(
output_delta_notif
.params
.clone()
.expect("item/fileChange/outputDelta params"),
)?;
assert_eq!(output_delta.thread_id, thread.id);
assert_eq!(output_delta.turn_id, turn.id);
assert_eq!(output_delta.item_id, "patch-call");
assert!(
!output_delta.delta.is_empty(),
"expected delta to be non-empty, got: {}",
output_delta.delta
);
let completed_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let completed_notif = mcp
@@ -753,9 +784,134 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
)
.await??;
let readme_contents = std::fs::read_to_string(expected_readme_path)?;
let readme_contents = std::fs::read_to_string(&readme_path)?;
assert_eq!(readme_contents, "new line\n");
let second_turn_req = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![V2UserInput::Text {
text: "update patch".into(),
}],
approval_policy: Some(codex_app_server_protocol::AskForApproval::Never),
sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess),
model: Some("mock-model".to_string()),
effort: Some(ReasoningEffort::Medium),
summary: Some(ReasoningSummary::Auto),
cwd: Some(workspace.clone()),
})
.await?;
let second_turn_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(second_turn_req)),
)
.await??;
let TurnStartResponse { turn: second_turn } =
to_response::<TurnStartResponse>(second_turn_resp)?;
let started_update_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let started_notif = mcp
.read_stream_until_notification_message("item/started")
.await?;
let started: ItemStartedNotification =
serde_json::from_value(started_notif.params.clone().expect("item/started params"))?;
if let ThreadItem::FileChange { .. } = started.item {
return Ok::<ThreadItem, anyhow::Error>(started.item);
}
}
})
.await??;
let ThreadItem::FileChange {
ref id,
status,
ref changes,
} = started_update_file_change
else {
unreachable!("loop ensures we break on file change items");
};
assert_eq!(id, "patch-call-2");
assert_eq!(status, PatchApplyStatus::InProgress);
let update_changes = changes.clone();
let update_change = update_changes
.first()
.expect("expected a single update change");
assert_eq!(update_change.path, expected_readme_path);
match &update_change.kind {
PatchChangeKind::Update {
move_path,
old_content,
new_content,
} => {
assert!(move_path.is_none());
assert_eq!(old_content, "new line\n");
assert_eq!(new_content, "updated line\n");
}
other => panic!("expected PatchChangeKind::Update, got: {other:?}"),
}
assert!(
update_change.diff.contains("-new line"),
"expected diff to include removal of original line, got: {}",
update_change.diff
);
assert!(
update_change.diff.contains("+updated line"),
"expected diff to include addition of updated line, got: {}",
update_change.diff
);
let output_delta_notif = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
)
.await??;
let output_delta: FileChangeOutputDeltaNotification = serde_json::from_value(
output_delta_notif
.params
.clone()
.expect("item/fileChange/outputDelta params"),
)?;
assert_eq!(output_delta.thread_id, thread.id);
assert_eq!(output_delta.turn_id, second_turn.id);
assert_eq!(output_delta.item_id, "patch-call-2");
assert!(
!output_delta.delta.is_empty(),
"expected delta to be non-empty, got: {}",
output_delta.delta
);
let completed_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let completed_notif = mcp
.read_stream_until_notification_message("item/completed")
.await?;
let completed: ItemCompletedNotification = serde_json::from_value(
completed_notif
.params
.clone()
.expect("item/completed params"),
)?;
if let ThreadItem::FileChange { .. } = completed.item {
return Ok::<ThreadItem, anyhow::Error>(completed.item);
}
}
})
.await??;
let ThreadItem::FileChange { ref id, status, .. } = completed_file_change else {
unreachable!("loop ensures we break on file change items");
};
assert_eq!(id, "patch-call-2");
assert_eq!(status, PatchApplyStatus::Completed);
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("codex/event/task_complete"),
)
.await??;
let updated_readme_contents = std::fs::read_to_string(&readme_path)?;
assert_eq!(updated_readme_contents, "updated line\n");
Ok(())
}
@@ -907,6 +1063,134 @@ async fn turn_start_file_change_approval_decline_v2() -> Result<()> {
Ok(())
}
#[tokio::test]
#[cfg_attr(windows, ignore = "process id reporting differs on Windows")]
async fn command_execution_notifications_include_process_id() -> Result<()> {
skip_if_no_network!(Ok(()));
let responses = vec![
create_exec_command_sse_response("uexec-1")?,
create_final_assistant_message_sse_response("done")?,
];
let server = create_mock_chat_completions_server(responses).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let config_toml = codex_home.path().join("config.toml");
let mut config_contents = std::fs::read_to_string(&config_toml)?;
config_contents.push_str(
r#"
[features]
unified_exec = true
"#,
);
std::fs::write(&config_toml, config_contents)?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let start_id = mcp
.send_thread_start_request(ThreadStartParams {
model: Some("mock-model".to_string()),
..Default::default()
})
.await?;
let start_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(start_id)),
)
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
let turn_id = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![V2UserInput::Text {
text: "run a command".to_string(),
}],
..Default::default()
})
.await?;
let turn_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(turn_id)),
)
.await??;
let TurnStartResponse { turn: _turn } = to_response::<TurnStartResponse>(turn_resp)?;
let started_command = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notif = mcp
.read_stream_until_notification_message("item/started")
.await?;
let started: ItemStartedNotification = serde_json::from_value(
notif
.params
.clone()
.expect("item/started should include params"),
)?;
if let ThreadItem::CommandExecution { .. } = started.item {
return Ok::<ThreadItem, anyhow::Error>(started.item);
}
}
})
.await??;
let ThreadItem::CommandExecution {
id,
process_id: started_process_id,
status,
..
} = started_command
else {
unreachable!("loop ensures we break on command execution items");
};
assert_eq!(id, "uexec-1");
assert_eq!(status, CommandExecutionStatus::InProgress);
let started_process_id = started_process_id.expect("process id should be present");
let completed_command = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notif = mcp
.read_stream_until_notification_message("item/completed")
.await?;
let completed: ItemCompletedNotification = serde_json::from_value(
notif
.params
.clone()
.expect("item/completed should include params"),
)?;
if let ThreadItem::CommandExecution { .. } = completed.item {
return Ok::<ThreadItem, anyhow::Error>(completed.item);
}
}
})
.await??;
let ThreadItem::CommandExecution {
id: completed_id,
process_id: completed_process_id,
status: completed_status,
exit_code,
..
} = completed_command
else {
unreachable!("loop ensures we break on command execution items");
};
assert_eq!(completed_id, "uexec-1");
assert_eq!(completed_status, CommandExecutionStatus::Completed);
assert_eq!(exit_code, Some(0));
assert_eq!(
completed_process_id.as_deref(),
Some(started_process_id.as_str())
);
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
Ok(())
}
// Helper to create a config.toml pointing at the mock model server.
fn create_config_toml(
codex_home: &Path,

View File

@@ -194,6 +194,8 @@ pub enum ApplyPatchFileChange {
move_path: Option<PathBuf>,
/// new_content that will result after the unified_diff is applied.
new_content: String,
/// original content before the diff was applied.
old_content: String,
},
}
@@ -329,6 +331,7 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp
let ApplyPatchFileUpdate {
unified_diff,
content: contents,
old_content,
} = match unified_diff_from_chunks(&path, &chunks) {
Ok(diff) => diff,
Err(e) => {
@@ -341,6 +344,7 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp
unified_diff,
move_path: move_path.map(|p| effective_cwd.join(p)),
new_content: contents,
old_content,
},
);
}
@@ -846,6 +850,7 @@ fn apply_replacements(
pub struct ApplyPatchFileUpdate {
unified_diff: String,
content: String,
old_content: String,
}
pub fn unified_diff_from_chunks(
@@ -869,6 +874,7 @@ pub fn unified_diff_from_chunks_with_context(
Ok(ApplyPatchFileUpdate {
unified_diff,
content: new_contents,
old_content: original_contents,
})
}
@@ -1467,6 +1473,7 @@ PATCH"#,
let expected = ApplyPatchFileUpdate {
unified_diff: expected_diff.to_string(),
content: "foo\nBAR\nbaz\nQUX\n".to_string(),
old_content: "foo\nbar\nbaz\nqux\n".to_string(),
};
assert_eq!(expected, diff);
}
@@ -1503,6 +1510,7 @@ PATCH"#,
let expected = ApplyPatchFileUpdate {
unified_diff: expected_diff.to_string(),
content: "FOO\nbar\nbaz\n".to_string(),
old_content: "foo\nbar\nbaz\n".to_string(),
};
assert_eq!(expected, diff);
}
@@ -1540,6 +1548,7 @@ PATCH"#,
let expected = ApplyPatchFileUpdate {
unified_diff: expected_diff.to_string(),
content: "foo\nbar\nBAZ\n".to_string(),
old_content: "foo\nbar\nbaz\n".to_string(),
};
assert_eq!(expected, diff);
}
@@ -1574,6 +1583,7 @@ PATCH"#,
let expected = ApplyPatchFileUpdate {
unified_diff: expected_diff.to_string(),
content: "foo\nbar\nbaz\nquux\n".to_string(),
old_content: "foo\nbar\nbaz\n".to_string(),
};
assert_eq!(expected, diff);
}
@@ -1629,6 +1639,7 @@ PATCH"#,
let expected = ApplyPatchFileUpdate {
unified_diff: expected_diff.to_string(),
content: "a\nB\nc\nd\nE\nf\ng\n".to_string(),
old_content: "a\nb\nc\nd\ne\nf\n".to_string(),
};
assert_eq!(expected, diff);
@@ -1688,6 +1699,7 @@ g
.to_string(),
move_path: None,
new_content: "updated session directory content\n".to_string(),
old_content: "session directory content\n".to_string(),
},
)]),
patch: argv[1].clone(),

View File

@@ -1,206 +0,0 @@
# Client Extraction Plan
## Goals
- Split the HTTP transport/client code out of `codex-core` into a reusable crate that is agnostic of Codex/OpenAI business logic and API schemas.
- Create a separate API library crate that houses typed requests/responses for well-known APIs (Responses, Chat Completions, Compact) and plugs into the transport crate via minimal traits.
- Preserve current behaviour (auth headers, retries, SSE handling, rate-limit parsing, compaction, fixtures) while making the APIs symmetric and avoiding code duplication.
- Keep existing consumers (`codex-core`, tests, and tools) stable by providing a small compatibility layer during the transition.
## Snapshot of Today
- `core/src/client.rs (ModelClient)` owns config/auth/session state, chooses wire API, builds payloads, drives retries, parses SSE, compaction, and rate-limit headers.
- `core/src/chat_completions.rs` implements the Chat Completions call + SSE parser + aggregation helper.
- `core/src/client_common.rs` holds `Prompt`, tool specs, shared request structs (`ResponsesApiRequest`, `TextControls`), and `ResponseEvent`/`ResponseStream`.
- `core/src/default_client.rs` wraps `reqwest` with Codex UA/originator defaults.
- `core/src/model_provider_info.rs` models providers (base URL, headers, env keys, retry/timeout tuning) and builds `CodexRequestBuilder`s.
- Current retry logic is co-located with API handling; streaming SSE parsing is duplicated across Responses/Chat.
## Target Crates (with interfaces)
- `codex-client` (generic transport)
- Owns the generic HTTP machinery: a `CodexHttpClient`/`CodexRequestBuilder`-style wrapper, retry/backoff hooks, streaming connector (SSE framing + idle timeout), header injection, and optional telemetry callbacks.
- Does **not** know about OpenAI/Codex-specific paths, headers, or error codes; it only exposes HTTP-level concepts (status, headers, bodies, connection errors).
- Minimal surface:
```rust
pub trait HttpTransport {
fn execute(&self, req: Request) -> Result<Response, TransportError>;
fn stream(&self, req: Request) -> Result<ByteStream, TransportError>;
}
pub struct Request {
pub method: Method,
pub url: String,
pub headers: HeaderMap,
pub body: Option<serde_json::Value>,
pub timeout: Option<Duration>,
}
```
- Generic client traits (request/response/chunk are abstract over the transport):
```rust
#[async_trait::async_trait]
pub trait UnaryClient<Req, Resp> {
async fn run(&self, req: Req) -> Result<Resp, TransportError>;
}
#[async_trait::async_trait]
pub trait StreamClient<Req, Chunk> {
async fn run(&self, req: Req) -> Result<ResponseStream<Chunk>, TransportError>;
}
pub struct RetryPolicy {
pub max_attempts: u64,
pub base_delay: Duration,
pub retry_on: RetryOn, // e.g., transport errors + 429/5xx
}
```
- `RetryOn` lives in `codex-client` and captures HTTP status classes and transport failures that qualify for retry.
- Implementations in `codex-api` plug in their own request types, parsers, and retry policies while reusing the transports backoff and error types.
- Planned runtime helper:
```rust
pub async fn run_with_retry<T, F, Fut>(
policy: RetryPolicy,
make_req: impl Fn() -> Request,
op: F,
) -> Result<T, TransportError>
where
F: Fn(Request) -> Fut,
Fut: Future<Output = Result<T, TransportError>>,
{
for attempt in 0..=policy.max_attempts {
let req = make_req();
match op(req).await {
Ok(resp) => return Ok(resp),
Err(err) if policy.retry_on.should_retry(&err, attempt) => {
tokio::time::sleep(backoff(policy.base_delay, attempt + 1)).await;
}
Err(err) => return Err(err),
}
}
Err(TransportError::RetryLimit)
}
```
- Unary clients wrap `transport.execute` with this helper and then deserialize.
- Stream clients wrap the **initial** `transport.stream` call with this helper. Mid-stream disconnects are surfaced as `StreamError`s; automatic resume/reconnect can be added later on top of this primitive if we introduce cursor support.
- Common helpers: `retry::backoff(attempt)`, `errors::{TransportError, StreamError}`.
- Streaming utility (SSE framing only):
```rust
pub fn sse_stream<S>(
bytes: S,
idle_timeout: Duration,
tx: mpsc::Sender<Result<String, StreamError>>,
telemetry: Option<Box<dyn Telemetry>>,
)
where
S: Stream<Item = Result<Bytes, TransportError>> + Unpin + Send + 'static;
```
- `sse_stream` is responsible for timeouts, connection-level errors, and emitting raw `data:` chunks as UTF-8 strings; parsing those strings into structured events is done in `codex-api`.
- `codex-api` (OpenAI/Codex API library)
- Owns typed models for Responses/Chat/Compact plus shared helpers (`Prompt`, tool specs, text controls, `ResponsesApiRequest`, etc.).
- Knows about OpenAI/Codex semantics:
- URL shapes (`/v1/responses`, `/v1/chat/completions`, `/responses/compact`).
- Provider configuration (`WireApi`, base URLs, query params, per-provider retry knobs).
- Rate-limit headers (`x-codex-*`) and their mapping into `RateLimitSnapshot` / `CreditsSnapshot`.
- Error body formats (`{ error: { type, code, message, plan_type, resets_at } }`) and how they become API errors (context window exceeded, quota/usage limit, etc.).
- SSE event names (`response.output_item.done`, `response.completed`, `response.failed`, etc.) and their mapping into high-level events.
- Provides a provider abstraction (conceptually similar to `ModelProviderInfo`):
```rust
pub struct Provider {
pub name: String,
pub base_url: String,
pub wire: WireApi, // Responses | Chat
pub headers: HeaderMap,
pub retry: RetryConfig,
pub stream_idle_timeout: Duration,
}
pub trait AuthProvider {
/// Returns a bearer token to use for this request (if any).
/// Implementations are expected to be cheap and to surface already-refreshed tokens;
/// higher layers (`codex-core`) remain responsible for token refresh flows.
fn bearer_token(&self) -> Option<String>;
/// Optional ChatGPT account id header for Chat mode.
fn account_id(&self) -> Option<String>;
}
```
- Ready-made clients built on `HttpTransport`:
```rust
pub struct ResponsesClient<T: HttpTransport, A: AuthProvider> { /* ... */ }
impl<T, A> ResponsesClient<T, A> {
pub async fn stream(&self, prompt: &Prompt) -> ApiResult<ResponseStream<ApiEvent>>;
pub async fn compact(&self, prompt: &Prompt) -> ApiResult<Vec<ResponseItem>>;
}
pub struct ChatClient<T: HttpTransport, A: AuthProvider> { /* ... */ }
impl<T, A> ChatClient<T, A> {
pub async fn stream(&self, prompt: &Prompt) -> ApiResult<ResponseStream<ApiEvent>>;
}
pub struct CompactClient<T: HttpTransport, A: AuthProvider> { /* ... */ }
impl<T, A> CompactClient<T, A> {
pub async fn compact(&self, prompt: &Prompt) -> ApiResult<Vec<ResponseItem>>;
}
```
- Streaming events unified across wire APIs (this can closely mirror `ResponseEvent` today, and we may type-alias one to the other during migration):
```rust
pub enum ApiEvent {
Created,
OutputItemAdded(ResponseItem),
OutputItemDone(ResponseItem),
OutputTextDelta(String),
ReasoningContentDelta { delta: String, content_index: i64 },
ReasoningSummaryDelta { delta: String, summary_index: i64 },
RateLimits(RateLimitSnapshot),
Completed { response_id: String, token_usage: Option<TokenUsage> },
}
```
- Error layering:
- `codex-client`: defines `TransportError` / `StreamError` (status codes, IO, timeouts).
- `codex-api`: defines `ApiError` that wraps `TransportError` plus API-specific errors parsed from bodies and headers.
- `codex-core`: maps `ApiError` into existing `CodexErr` variants so downstream callers remain unchanged.
- Aggregation strategies (todays `AggregateStreamExt`) live here as adapters (`Aggregated`, `Streaming`) that transform `ResponseStream<ApiEvent>` into the higher-level views used by `codex-core`.
## Implementation Steps
1. **Create crates**: add `codex-client` and `codex-api` (names keep the `codex-` prefix). Stub lib files with feature flags/tests wired into the workspace; wire them into `Cargo.toml`.
2. **Extract API-level SSE + rate limits into `codex-api`**:
- Move the Responses SSE parser (`process_sse`), rate-limit parsing, and related tests from `core/src/client.rs` into `codex-api`, keeping the behavior identical.
- Introduce `ApiEvent` (initially equivalent to `ResponseEvent`) and `ApiError`, and adjust the parser to emit those.
- Provide test-only helpers for fixture streams (replacement for `CODEX_RS_SSE_FIXTURE`) in `codex-api`.
3. **Lift transport layer into `codex-client`**:
- Move `CodexHttpClient`/`CodexRequestBuilder`, UA/originator plumbing, and backoff helpers from `core/src/default_client.rs` into `codex-client` (or a thin wrapper on top of it).
- Introduce `HttpTransport`, `Request`, `RetryPolicy`, `RetryOn`, and `run_with_retry` as described above.
- Keep sandbox/no-proxy toggles behind injected configuration so `codex-client` stays generic and does not depend on Codex-specific env vars.
4. **Model provider abstraction in `codex-api`**:
- Relocate `ModelProviderInfo` (base URL, env/header resolution, retry knobs, wire API enum) into `codex-api`, expressed in terms of `Provider` and `AuthProvider`.
- Ensure provider logic handles:
- URL building for Responses/Chat/Compact (including Azure special cases).
- Static and env-based headers.
- Per-provider retry and idle-timeout settings that map cleanly into `RetryPolicy`/`RetryOn`.
5. **API crate wiring**:
- Move `Prompt`, tool specs, `ResponsesApiRequest`, `TextControls`, and `ResponseEvent/ResponseStream` into `codex-api` under modules (`common`, `responses`, `chat`, `compact`), keeping public types stable or re-exported through `codex-core` as needed.
- Rebuild Responses and Chat clients on top of `HttpTransport` + `StreamClient`, reusing shared retry + SSE helpers; keep aggregation adapters as reusable strategies instead of `ModelClient`-local logic.
- Implement Compact on top of `UnaryClient` and the unary `execute` path with JSON deserialization, sharing the same retry policy.
- Keep request builders symmetric: each client prepares a `Request<serde_json::Value>`, attaches headers/auth via `AuthProvider`, and plugs in its parser (streaming clients) or deserializer (unary) while sharing retry/backoff configuration derived from `Provider`.
6. **Core integration layer**:
- Replace `core::ModelClient` internals with thin adapters that construct `codex-api` clients using `Config`, `AuthManager`, and `OtelEventManager`.
- Keep the public `ModelClient` API and `ResponseEvent`/`ResponseStream` types stable by re-exporting `codex-api` types or providing type aliases.
- Preserve existing auth flows (including ChatGPT token refresh) inside `codex-core` or a thin adapter, using `AuthProvider` to surface bearer tokens to `codex-api` and handling 401/refresh semantics at this layer.
7. **Tests/migration**:
- Move unit tests for SSE parsing, retry/backoff decisions, and provider/header behavior into the new crates; keep integration tests in `core` using the compatibility layer.
- Update fixtures to be consumed via test-only adapters in `codex-api`.
- Run targeted `just fmt`, `just fix -p` for the touched crates, and scoped `cargo test -p codex-client`, `-p codex-api`, and existing `codex-core` suites.
## Design Decisions
- **UA construction**
- `codex-client` exposes an optional UA suffix/provider hook (tiny feature) and remains unaware of the CLI; `codex-core` / the CLI compute the full UA (including `terminal::user_agent()`) and pass the suffix or builder down.
- **Config vs provider**
- Most configuration stays in `codex-core`. `codex-api::Provider` only contains what is strictly required for HTTP (base URLs, query params, retry/timeout knobs, wire API), while higher-level knobs (reasoning defaults, verbosity flags, etc.) remain core concerns.
- **Auth flow ownership**
- Auth flows (including ChatGPT token refresh) remain in `codex-core`. `AuthProvider` simply exposes already-fresh tokens/account IDs; 401 handling and refresh retries stay in the existing auth layer.
- **Error enums**
- `codex-client` continues to define `TransportError` / `StreamError`. `codex-api` defines an `ApiError` (deriving `thiserror::Error`) that wraps `TransportError` and API-specific failures, and `codex-core` maps `ApiError` into existing `CodexErr` variants for callers.
- **Streaming reconnection semantics**
- For now, mid-stream SSE failures are surfaced as errors and only the initial connection is retried via `run_with_retry`. We will revisit mid-stream reconnect/resume once the underlying APIs support cursor/idempotent event semantics.

View File

@@ -87,6 +87,9 @@ uuid = { workspace = true, features = ["serde", "v4", "v5"] }
which = { workspace = true }
wildmatch = { workspace = true }
[features]
deterministic_process_ids = []
[target.'cfg(target_os = "linux")'.dependencies]
landlock = { workspace = true }
@@ -115,6 +118,7 @@ keyring = { workspace = true, features = ["sync-secret-service"] }
assert_cmd = { workspace = true }
assert_matches = { workspace = true }
codex-arg0 = { workspace = true }
codex-core = { path = ".", features = ["deterministic_process_ids"] }
core_test_support = { workspace = true }
ctor = { workspace = true }
escargot = { workspace = true }

View File

@@ -105,10 +105,13 @@ pub(crate) fn convert_apply_patch_to_protocol(
ApplyPatchFileChange::Update {
unified_diff,
move_path,
new_content: _new_content,
new_content,
old_content,
} => FileChange::Update {
unified_diff: unified_diff.clone(),
move_path: move_path.clone(),
old_content: old_content.clone(),
new_content: new_content.clone(),
},
};
result.insert(path.clone(), protocol_change);

View File

@@ -102,6 +102,7 @@ use crate::protocol::TurnDiffEvent;
use crate::protocol::WarningEvent;
use crate::rollout::RolloutRecorder;
use crate::rollout::RolloutRecorderParams;
use crate::rollout::map_session_init_error;
use crate::shell;
use crate::state::ActiveTurn;
use crate::state::SessionServices;
@@ -206,7 +207,7 @@ impl Codex {
.await
.map_err(|e| {
error!("Failed to create session: {e:#}");
CodexErr::InternalAgentDied
map_session_init_error(&e, &config.codex_home)
})?;
let conversation_id = session.conversation_id;
@@ -508,7 +509,7 @@ impl Session {
let rollout_recorder = rollout_recorder.map_err(|e| {
error!("failed to initialize rollout recorder: {e:#}");
anyhow::anyhow!("failed to initialize rollout recorder: {e:#}")
anyhow::Error::from(e)
})?;
let rollout_path = rollout_recorder.rollout_path.clone();
@@ -1189,22 +1190,17 @@ impl Session {
}
}
/// Record a user input item to conversation history and also persist a
/// corresponding UserMessage EventMsg to rollout.
async fn record_input_and_rollout_usermsg(
pub(crate) async fn record_response_item_and_emit_turn_item(
&self,
turn_context: &TurnContext,
response_input: &ResponseInputItem,
response_item: ResponseItem,
) {
let response_item: ResponseItem = response_input.clone().into();
// Add to conversation history and persist response item to rollout
// Add to conversation history and persist response item to rollout.
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
.await;
// Derive user message events and persist only UserMessage to rollout
let turn_item = parse_turn_item(&response_item);
if let Some(item @ TurnItem::UserMessage(_)) = turn_item {
// Derive a turn item and emit lifecycle events if applicable.
if let Some(item) = parse_turn_item(&response_item) {
self.emit_turn_item_started(turn_context, &item).await;
self.emit_turn_item_completed(turn_context, item).await;
}
@@ -1880,12 +1876,7 @@ async fn spawn_review_thread(
text: review_prompt,
}];
let tc = Arc::new(review_turn_context);
sess.spawn_task(
tc.clone(),
input,
ReviewTask::new(review_request.append_to_original_thread),
)
.await;
sess.spawn_task(tc.clone(), input, ReviewTask::new()).await;
// Announce entering review mode so UIs can switch modes.
sess.send_event(&tc, EventMsg::EnteredReviewMode(review_request))
@@ -1921,7 +1912,8 @@ pub(crate) async fn run_task(
sess.send_event(&turn_context, event).await;
let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input);
sess.record_input_and_rollout_usermsg(turn_context.as_ref(), &initial_input_for_turn)
let response_item: ResponseItem = initial_input_for_turn.clone().into();
sess.record_response_item_and_emit_turn_item(turn_context.as_ref(), response_item)
.await;
sess.maybe_start_ghost_snapshot(Arc::clone(&turn_context), cancellation_token.child_token())
@@ -2886,7 +2878,7 @@ mod tests {
let input = vec![UserInput::Text {
text: "start review".to_string(),
}];
sess.spawn_task(Arc::clone(&tc), input, ReviewTask::new(true))
sess.spawn_task(Arc::clone(&tc), input, ReviewTask::new())
.await;
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
@@ -2914,6 +2906,8 @@ mod tests {
.expect("event");
match evt.msg {
EventMsg::RawResponseItem(_) => continue,
EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) => continue,
EventMsg::AgentMessage(_) => continue,
EventMsg::TurnAborted(e) => {
assert_eq!(TurnAbortReason::Interrupted, e.reason);
break;
@@ -2923,23 +2917,7 @@ mod tests {
}
let history = sess.clone_history().await.get_history();
let found = history.iter().any(|item| match item {
ResponseItem::Message { role, content, .. } if role == "user" => {
content.iter().any(|ci| match ci {
ContentItem::InputText { text } => {
text.contains("<user_action>")
&& text.contains("review")
&& text.contains("interrupted")
}
_ => false,
})
}
_ => false,
});
assert!(
found,
"synthetic review interruption not recorded in history"
);
let _ = history;
}
#[tokio::test]

View File

@@ -1,4 +1,5 @@
use crate::protocol::ReviewFinding;
use crate::protocol::ReviewOutputEvent;
// Note: We keep this module UI-agnostic. It returns plain strings that
// higher layers (e.g., TUI) may style as needed.
@@ -10,6 +11,8 @@ fn format_location(item: &ReviewFinding) -> String {
format!("{path}:{start}-{end}")
}
const REVIEW_FALLBACK_MESSAGE: &str = "Reviewer failed to output a response.";
/// Format a full review findings block as plain text lines.
///
/// - When `selection` is `Some`, each item line includes a checkbox marker:
@@ -53,3 +56,27 @@ pub fn format_review_findings_block(
lines.join("\n")
}
/// Render a human-readable review summary suitable for a user-facing message.
///
/// Returns either the explanation, the formatted findings block, or both
/// separated by a blank line. If neither is present, emits a fallback message.
pub fn render_review_output_text(output: &ReviewOutputEvent) -> String {
let mut sections = Vec::new();
let explanation = output.overall_explanation.trim();
if !explanation.is_empty() {
sections.push(explanation.to_string());
}
if !output.findings.is_empty() {
let findings = format_review_findings_block(&output.findings, None);
let trimmed = findings.trim();
if !trimmed.is_empty() {
sections.push(trimmed.to_string());
}
}
if sections.is_empty() {
REVIEW_FALLBACK_MESSAGE.to_string()
} else {
sections.join("\n\n")
}
}

View File

@@ -0,0 +1,49 @@
use std::io::ErrorKind;
use std::path::Path;
use crate::error::CodexErr;
use crate::rollout::SESSIONS_SUBDIR;
pub(crate) fn map_session_init_error(err: &anyhow::Error, codex_home: &Path) -> CodexErr {
if let Some(mapped) = err
.chain()
.filter_map(|cause| cause.downcast_ref::<std::io::Error>())
.find_map(|io_err| map_rollout_io_error(io_err, codex_home))
{
return mapped;
}
CodexErr::Fatal(format!("Failed to initialize session: {err:#}"))
}
fn map_rollout_io_error(io_err: &std::io::Error, codex_home: &Path) -> Option<CodexErr> {
let sessions_dir = codex_home.join(SESSIONS_SUBDIR);
let hint = match io_err.kind() {
ErrorKind::PermissionDenied => format!(
"Codex cannot access session files at {} (permission denied). If sessions were created using sudo, fix ownership: sudo chown -R $(whoami) {}",
sessions_dir.display(),
codex_home.display()
),
ErrorKind::NotFound => format!(
"Session storage missing at {}. Create the directory or choose a different Codex home.",
sessions_dir.display()
),
ErrorKind::AlreadyExists => format!(
"Session storage path {} is blocked by an existing file. Remove or rename it so Codex can create sessions.",
sessions_dir.display()
),
ErrorKind::InvalidData | ErrorKind::InvalidInput => format!(
"Session data under {} looks corrupt or unreadable. Clearing the sessions directory may help (this will remove saved conversations).",
sessions_dir.display()
),
ErrorKind::IsADirectory | ErrorKind::NotADirectory => format!(
"Session storage path {} has an unexpected type. Ensure it is a directory Codex can use for session files.",
sessions_dir.display()
),
_ => return None,
};
Some(CodexErr::Fatal(format!(
"{hint} (underlying error: {io_err})"
)))
}

View File

@@ -7,11 +7,13 @@ pub const ARCHIVED_SESSIONS_SUBDIR: &str = "archived_sessions";
pub const INTERACTIVE_SESSION_SOURCES: &[SessionSource] =
&[SessionSource::Cli, SessionSource::VSCode];
pub(crate) mod error;
pub mod list;
pub(crate) mod policy;
pub mod recorder;
pub use codex_protocol::protocol::SessionMeta;
pub(crate) use error::map_session_init_error;
pub use list::find_conversation_path_by_id_str;
pub use recorder::RolloutRecorder;
pub use recorder::RolloutRecorderParams;

View File

@@ -14,6 +14,7 @@ use crate::protocol::SandboxPolicy;
use askama::Template;
use codex_otel::otel_event_manager::OtelEventManager;
use codex_protocol::ConversationId;
use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::SandboxCommandAssessment;
@@ -23,7 +24,8 @@ use serde_json::json;
use tokio::time::timeout;
use tracing::warn;
const SANDBOX_ASSESSMENT_TIMEOUT: Duration = Duration::from_secs(5);
const SANDBOX_ASSESSMENT_TIMEOUT: Duration = Duration::from_secs(15);
const SANDBOX_ASSESSMENT_REASONING_EFFORT: ReasoningEffortConfig = ReasoningEffortConfig::Medium;
#[derive(Template)]
#[template(path = "sandboxing/assessment_prompt.md", escape = "none")]
@@ -130,7 +132,7 @@ pub(crate) async fn assess_command(
Some(auth_manager),
child_otel,
provider,
config.model_reasoning_effort,
Some(SANDBOX_ASSESSMENT_REASONING_EFFORT),
config.model_reasoning_summary,
conversation_id,
session_source,

View File

@@ -17,6 +17,7 @@ use crate::codex::Session;
use crate::codex::TurnContext;
use crate::codex_delegate::run_codex_conversation_one_shot;
use crate::review_format::format_review_findings_block;
use crate::review_format::render_review_output_text;
use crate::state::TaskKind;
use codex_protocol::user_input::UserInput;
@@ -24,15 +25,11 @@ use super::SessionTask;
use super::SessionTaskContext;
#[derive(Clone, Copy)]
pub(crate) struct ReviewTask {
append_to_original_thread: bool,
}
pub(crate) struct ReviewTask;
impl ReviewTask {
pub(crate) fn new(append_to_original_thread: bool) -> Self {
Self {
append_to_original_thread,
}
pub(crate) fn new() -> Self {
Self
}
}
@@ -62,25 +59,13 @@ impl SessionTask for ReviewTask {
None => None,
};
if !cancellation_token.is_cancelled() {
exit_review_mode(
session.clone_session(),
output.clone(),
ctx.clone(),
self.append_to_original_thread,
)
.await;
exit_review_mode(session.clone_session(), output.clone(), ctx.clone()).await;
}
None
}
async fn abort(&self, session: Arc<SessionTaskContext>, ctx: Arc<TurnContext>) {
exit_review_mode(
session.clone_session(),
None,
ctx,
self.append_to_original_thread,
)
.await;
exit_review_mode(session.clone_session(), None, ctx).await;
}
}
@@ -197,39 +182,57 @@ pub(crate) async fn exit_review_mode(
session: Arc<Session>,
review_output: Option<ReviewOutputEvent>,
ctx: Arc<TurnContext>,
append_to_original_thread: bool,
) {
if append_to_original_thread {
let user_message = if let Some(out) = review_output.clone() {
let mut findings_str = String::new();
let text = out.overall_explanation.trim();
if !text.is_empty() {
findings_str.push_str(text);
}
if !out.findings.is_empty() {
let block = format_review_findings_block(&out.findings, None);
findings_str.push_str(&format!("\n{block}"));
}
crate::client_common::REVIEW_EXIT_SUCCESS_TMPL.replace("{results}", &findings_str)
} else {
crate::client_common::REVIEW_EXIT_INTERRUPTED_TMPL.to_string()
};
const REVIEW_USER_MESSAGE_ID: &str = "review:rollout:user";
const REVIEW_ASSISTANT_MESSAGE_ID: &str = "review:rollout:assistant";
let (user_message, assistant_message) = if let Some(out) = review_output.clone() {
let mut findings_str = String::new();
let text = out.overall_explanation.trim();
if !text.is_empty() {
findings_str.push_str(text);
}
if !out.findings.is_empty() {
let block = format_review_findings_block(&out.findings, None);
findings_str.push_str(&format!("\n{block}"));
}
let rendered =
crate::client_common::REVIEW_EXIT_SUCCESS_TMPL.replace("{results}", &findings_str);
let assistant_message = render_review_output_text(&out);
(rendered, assistant_message)
} else {
let rendered = crate::client_common::REVIEW_EXIT_INTERRUPTED_TMPL.to_string();
let assistant_message =
"Review was interrupted. Please re-run /review and wait for it to complete."
.to_string();
(rendered, assistant_message)
};
session
.record_conversation_items(
&ctx,
&[ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText { text: user_message }],
}],
)
.await;
}
session
.record_conversation_items(
&ctx,
&[ResponseItem::Message {
id: Some(REVIEW_USER_MESSAGE_ID.to_string()),
role: "user".to_string(),
content: vec![ContentItem::InputText { text: user_message }],
}],
)
.await;
session
.send_event(
ctx.as_ref(),
EventMsg::ExitedReviewMode(ExitedReviewModeEvent { review_output }),
)
.await;
session
.record_response_item_and_emit_turn_item(
ctx.as_ref(),
ResponseItem::Message {
id: Some(REVIEW_ASSISTANT_MESSAGE_ID.to_string()),
role: "assistant".to_string(),
content: vec![ContentItem::OutputText {
text: assistant_message,
}],
},
)
.await;
}

View File

@@ -81,6 +81,7 @@ impl SessionTask for UserShellCommandTask {
turn_context.as_ref(),
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
call_id: call_id.clone(),
process_id: None,
turn_id: turn_context.sub_id.clone(),
command: command.clone(),
cwd: cwd.clone(),
@@ -139,6 +140,7 @@ impl SessionTask for UserShellCommandTask {
turn_context.as_ref(),
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id,
process_id: None,
turn_id: turn_context.sub_id.clone(),
command: command.clone(),
cwd: cwd.clone(),
@@ -161,6 +163,7 @@ impl SessionTask for UserShellCommandTask {
turn_context.as_ref(),
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: call_id.clone(),
process_id: None,
turn_id: turn_context.sub_id.clone(),
command: command.clone(),
cwd: cwd.clone(),
@@ -205,6 +208,7 @@ impl SessionTask for UserShellCommandTask {
turn_context.as_ref(),
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id,
process_id: None,
turn_id: turn_context.sub_id.clone(),
command,
cwd,

View File

@@ -65,12 +65,14 @@ pub(crate) async fn emit_exec_command_begin(
parsed_cmd: &[ParsedCommand],
source: ExecCommandSource,
interaction_input: Option<String>,
process_id: Option<&str>,
) {
ctx.session
.send_event(
ctx.turn,
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
call_id: ctx.call_id.to_string(),
process_id: process_id.map(str::to_owned),
turn_id: ctx.turn.sub_id.clone(),
command: command.to_vec(),
cwd: cwd.to_path_buf(),
@@ -100,6 +102,7 @@ pub(crate) enum ToolEmitter {
source: ExecCommandSource,
interaction_input: Option<String>,
parsed_cmd: Vec<ParsedCommand>,
process_id: Option<String>,
},
}
@@ -132,6 +135,7 @@ impl ToolEmitter {
cwd: PathBuf,
source: ExecCommandSource,
interaction_input: Option<String>,
process_id: Option<String>,
) -> Self {
let parsed_cmd = parse_command(command);
Self::UnifiedExec {
@@ -140,6 +144,7 @@ impl ToolEmitter {
source,
interaction_input,
parsed_cmd,
process_id,
}
}
@@ -157,7 +162,7 @@ impl ToolEmitter {
) => {
emit_exec_stage(
ctx,
ExecCommandInput::new(command, cwd.as_path(), parsed_cmd, *source, None),
ExecCommandInput::new(command, cwd.as_path(), parsed_cmd, *source, None, None),
stage,
)
.await;
@@ -229,6 +234,7 @@ impl ToolEmitter {
source,
interaction_input,
parsed_cmd,
process_id,
},
stage,
) => {
@@ -240,6 +246,7 @@ impl ToolEmitter {
parsed_cmd,
*source,
interaction_input.as_deref(),
process_id.as_deref(),
),
stage,
)
@@ -319,6 +326,7 @@ struct ExecCommandInput<'a> {
parsed_cmd: &'a [ParsedCommand],
source: ExecCommandSource,
interaction_input: Option<&'a str>,
process_id: Option<&'a str>,
}
impl<'a> ExecCommandInput<'a> {
@@ -328,6 +336,7 @@ impl<'a> ExecCommandInput<'a> {
parsed_cmd: &'a [ParsedCommand],
source: ExecCommandSource,
interaction_input: Option<&'a str>,
process_id: Option<&'a str>,
) -> Self {
Self {
command,
@@ -335,6 +344,7 @@ impl<'a> ExecCommandInput<'a> {
parsed_cmd,
source,
interaction_input,
process_id,
}
}
}
@@ -362,6 +372,7 @@ async fn emit_exec_stage(
exec_input.parsed_cmd,
exec_input.source,
exec_input.interaction_input.map(str::to_owned),
exec_input.process_id,
)
.await;
}
@@ -402,6 +413,7 @@ async fn emit_exec_end(
ctx.turn,
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: ctx.call_id.to_string(),
process_id: exec_input.process_id.map(str::to_owned),
turn_id: ctx.turn.sub_id.clone(),
command: exec_input.command.to_vec(),
cwd: exec_input.cwd.to_path_buf(),

View File

@@ -46,6 +46,7 @@ struct ExecCommandArgs {
#[derive(Debug, Deserialize)]
struct WriteStdinArgs {
// The model is trained on `session_id`.
session_id: i32,
#[serde(default)]
chars: String,
@@ -128,6 +129,7 @@ impl ToolHandler for UnifiedExecHandler {
"failed to parse exec_command arguments: {err:?}"
))
})?;
let process_id = manager.allocate_process_id().await;
let command = get_command(&args);
let ExecCommandArgs {
@@ -168,6 +170,7 @@ impl ToolHandler for UnifiedExecHandler {
cwd.clone(),
ExecCommandSource::UnifiedExecStartup,
None,
Some(process_id.clone()),
);
emitter.emit(event_ctx, ToolEventStage::Begin).await;
@@ -175,6 +178,7 @@ impl ToolHandler for UnifiedExecHandler {
.exec_command(
ExecCommandRequest {
command,
process_id,
yield_time_ms,
max_output_tokens,
workdir,
@@ -197,7 +201,7 @@ impl ToolHandler for UnifiedExecHandler {
manager
.write_stdin(WriteStdinRequest {
call_id: &call_id,
session_id: args.session_id,
process_id: &args.session_id.to_string(),
input: &args.chars,
yield_time_ms: args.yield_time_ms,
max_output_tokens: args.max_output_tokens,
@@ -255,8 +259,9 @@ fn format_response(response: &UnifiedExecResponse) -> String {
sections.push(format!("Process exited with code {exit_code}"));
}
if let Some(session_id) = response.session_id {
sections.push(format!("Process running with session ID {session_id}"));
if let Some(process_id) = &response.process_id {
// Training still uses "session ID".
sections.push(format!("Process running with session ID {process_id}"));
}
if let Some(original_token_count) = response.original_token_count {

View File

@@ -545,6 +545,8 @@ index {ZERO_OID}..{right_oid}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: None,
old_content: "foo\n".to_owned(),
new_content: "foo\nbar\n".to_owned(),
},
)]);
acc.on_patch_begin(&update_changes);
@@ -620,6 +622,8 @@ index {left_oid}..{ZERO_OID}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: Some(dest.clone()),
old_content: "line\n".to_owned(),
new_content: "line2\n".to_owned(),
},
)]);
acc.on_patch_begin(&mv_changes);
@@ -660,6 +664,8 @@ index {left_oid}..{right_oid}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: Some(dest.clone()),
old_content: "same\n".to_owned(),
new_content: "same\n".to_owned(),
},
)]);
acc.on_patch_begin(&mv_changes);
@@ -682,6 +688,8 @@ index {left_oid}..{right_oid}
FileChange::Update {
unified_diff: "".into(),
move_path: Some(dest.clone()),
old_content: String::new(),
new_content: "hello\n".to_owned(),
},
)]);
acc.on_patch_begin(&mv);
@@ -722,6 +730,8 @@ index {ZERO_OID}..{right_oid}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: None,
old_content: "foo\n".to_owned(),
new_content: "foo\nbar\n".to_owned(),
},
)]);
acc.on_patch_begin(&update_a);
@@ -802,6 +812,8 @@ index {left_oid_b}..{ZERO_OID}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: None,
old_content: String::new(),
new_content: String::new(),
},
)]);
acc.on_patch_begin(&update_changes);
@@ -868,6 +880,8 @@ index {ZERO_OID}..{right_oid}
FileChange::Update {
unified_diff: "".to_owned(),
move_path: None,
old_content: "foo\n".to_owned(),
new_content: "foo\nbar\n".to_owned(),
},
)]);
acc.on_patch_begin(&update_changes);

View File

@@ -5,8 +5,9 @@ use thiserror::Error;
pub(crate) enum UnifiedExecError {
#[error("Failed to create unified exec session: {message}")]
CreateSession { message: String },
#[error("Unknown session id {session_id}")]
UnknownSessionId { session_id: i32 },
// Called "session" in the model's training.
#[error("Unknown session id {process_id}")]
UnknownSessionId { process_id: String },
#[error("failed to write to stdin")]
WriteToStdin,
#[error("missing command line for unified exec request")]

View File

@@ -22,9 +22,9 @@
//! - `session_manager.rs`: orchestration (approvals, sandboxing, reuse) and request handling.
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::AtomicI32;
use std::time::Duration;
use rand::Rng;
@@ -67,6 +67,7 @@ impl UnifiedExecContext {
#[derive(Debug)]
pub(crate) struct ExecCommandRequest {
pub command: Vec<String>,
pub process_id: String,
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,
pub workdir: Option<PathBuf>,
@@ -77,7 +78,7 @@ pub(crate) struct ExecCommandRequest {
#[derive(Debug)]
pub(crate) struct WriteStdinRequest<'a> {
pub call_id: &'a str,
pub session_id: i32,
pub process_id: &'a str,
pub input: &'a str,
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,
@@ -89,7 +90,7 @@ pub(crate) struct UnifiedExecResponse {
pub chunk_id: String,
pub wall_time: Duration,
pub output: String,
pub session_id: Option<i32>,
pub process_id: Option<String>,
pub exit_code: Option<i32>,
pub original_token_count: Option<usize>,
pub session_command: Option<Vec<String>>,
@@ -97,15 +98,34 @@ pub(crate) struct UnifiedExecResponse {
#[derive(Default)]
pub(crate) struct UnifiedExecSessionManager {
next_session_id: AtomicI32,
sessions: Mutex<HashMap<i32, SessionEntry>>,
session_store: Mutex<SessionStore>,
}
// Required for mutex sharing.
#[derive(Default)]
pub(crate) struct SessionStore {
sessions: HashMap<String, SessionEntry>,
reserved_sessions_id: HashSet<String>,
}
impl SessionStore {
fn remove(&mut self, session_id: &str) -> Option<SessionEntry> {
self.reserved_sessions_id.remove(session_id);
self.sessions.remove(session_id)
}
pub(crate) fn clear(&mut self) {
self.reserved_sessions_id.clear();
self.sessions.clear();
}
}
struct SessionEntry {
session: session::UnifiedExecSession,
session: UnifiedExecSession,
session_ref: Arc<Session>,
turn_ref: Arc<TurnContext>,
call_id: String,
process_id: String,
command: Vec<String>,
cwd: PathBuf,
started_at: tokio::time::Instant,
@@ -159,6 +179,11 @@ mod tests {
) -> Result<UnifiedExecResponse, UnifiedExecError> {
let context =
UnifiedExecContext::new(Arc::clone(session), Arc::clone(turn), "call".to_string());
let process_id = session
.services
.unified_exec_manager
.allocate_process_id()
.await;
session
.services
@@ -166,6 +191,7 @@ mod tests {
.exec_command(
ExecCommandRequest {
command: vec!["bash".to_string(), "-lc".to_string(), cmd.to_string()],
process_id,
yield_time_ms,
max_output_tokens: None,
workdir: None,
@@ -179,7 +205,7 @@ mod tests {
async fn write_stdin(
session: &Arc<Session>,
session_id: i32,
process_id: &str,
input: &str,
yield_time_ms: u64,
) -> Result<UnifiedExecResponse, UnifiedExecError> {
@@ -188,7 +214,7 @@ mod tests {
.unified_exec_manager
.write_stdin(WriteStdinRequest {
call_id: "write-stdin",
session_id,
process_id,
input,
yield_time_ms,
max_output_tokens: None,
@@ -221,11 +247,15 @@ mod tests {
let (session, turn) = test_session_and_turn();
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let session_id = open_shell.session_id.expect("expected session_id");
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process_id")
.as_str();
write_stdin(
&session,
session_id,
process_id,
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
2_500,
)
@@ -233,7 +263,7 @@ mod tests {
let out_2 = write_stdin(
&session,
session_id,
process_id,
"echo $CODEX_INTERACTIVE_SHELL_VAR\n",
2_500,
)
@@ -253,11 +283,15 @@ mod tests {
let (session, turn) = test_session_and_turn();
let shell_a = exec_command(&session, &turn, "bash -i", 2_500).await?;
let session_a = shell_a.session_id.expect("expected session id");
let session_a = shell_a
.process_id
.as_ref()
.expect("expected process id")
.clone();
write_stdin(
&session,
session_a,
session_a.as_str(),
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
2_500,
)
@@ -265,9 +299,10 @@ mod tests {
let out_2 =
exec_command(&session, &turn, "echo $CODEX_INTERACTIVE_SHELL_VAR", 2_500).await?;
tokio::time::sleep(Duration::from_secs(2)).await;
assert!(
out_2.session_id.is_none(),
"short command should not retain a session"
out_2.process_id.is_none(),
"short command should not report a process id if it exits quickly"
);
assert!(
!out_2.output.contains("codex"),
@@ -276,7 +311,11 @@ mod tests {
let out_3 = write_stdin(
&session,
session_a,
shell_a
.process_id
.as_ref()
.expect("expected process id")
.as_str(),
"echo $CODEX_INTERACTIVE_SHELL_VAR\n",
2_500,
)
@@ -296,11 +335,15 @@ mod tests {
let (session, turn) = test_session_and_turn();
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let session_id = open_shell.session_id.expect("expected session id");
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process id")
.as_str();
write_stdin(
&session,
session_id,
process_id,
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
2_500,
)
@@ -308,7 +351,7 @@ mod tests {
let out_2 = write_stdin(
&session,
session_id,
process_id,
"sleep 5 && echo $CODEX_INTERACTIVE_SHELL_VAR\n",
10,
)
@@ -320,7 +363,7 @@ mod tests {
tokio::time::sleep(Duration::from_secs(7)).await;
let out_3 = write_stdin(&session, session_id, "", 100).await?;
let out_3 = write_stdin(&session, process_id, "", 100).await?;
assert!(
out_3.output.contains("codex"),
@@ -337,7 +380,7 @@ mod tests {
let result = exec_command(&session, &turn, "echo codex", 120_000).await?;
assert!(result.session_id.is_none());
assert!(result.process_id.is_some());
assert!(result.output.contains("codex"));
Ok(())
@@ -350,8 +393,8 @@ mod tests {
let result = exec_command(&session, &turn, "echo codex", 2_500).await?;
assert!(
result.session_id.is_none(),
"completed command should not retain session"
result.process_id.is_some(),
"completed command should report a process id"
);
assert!(result.output.contains("codex"));
@@ -359,9 +402,10 @@ mod tests {
session
.services
.unified_exec_manager
.sessions
.session_store
.lock()
.await
.sessions
.is_empty()
);
@@ -375,31 +419,36 @@ mod tests {
let (session, turn) = test_session_and_turn();
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let session_id = open_shell.session_id.expect("expected session id");
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process id")
.as_str();
write_stdin(&session, session_id, "exit\n", 2_500).await?;
write_stdin(&session, process_id, "exit\n", 2_500).await?;
tokio::time::sleep(Duration::from_millis(200)).await;
let err = write_stdin(&session, session_id, "", 100)
let err = write_stdin(&session, process_id, "", 100)
.await
.expect_err("expected unknown session error");
match err {
UnifiedExecError::UnknownSessionId { session_id: err_id } => {
assert_eq!(err_id, session_id);
UnifiedExecError::UnknownSessionId { process_id: err_id } => {
assert_eq!(err_id, process_id, "process id should match request");
}
other => panic!("expected UnknownSessionId, got {other:?}"),
}
assert!(
!session
session
.services
.unified_exec_manager
.sessions
.session_store
.lock()
.await
.contains_key(&session_id)
.sessions
.is_empty()
);
Ok(())

View File

@@ -1,9 +1,9 @@
use rand::Rng;
use std::cmp::Reverse;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use tokio::sync::Notify;
use tokio::sync::mpsc;
use tokio::time::Duration;
@@ -36,6 +36,7 @@ use crate::truncate::formatted_truncate_text;
use super::ExecCommandRequest;
use super::MAX_UNIFIED_EXEC_SESSIONS;
use super::SessionEntry;
use super::SessionStore;
use super::UnifiedExecContext;
use super::UnifiedExecError;
use super::UnifiedExecResponse;
@@ -75,9 +76,39 @@ struct PreparedSessionHandles {
turn_ref: Arc<TurnContext>,
command: Vec<String>,
cwd: PathBuf,
process_id: String,
}
impl UnifiedExecSessionManager {
pub(crate) async fn allocate_process_id(&self) -> String {
loop {
let mut store = self.session_store.lock().await;
let process_id = if !cfg!(test) && !cfg!(feature = "deterministic_process_ids") {
// production mode → random
rand::rng().random_range(1_000..100_000).to_string()
} else {
// test or deterministic mode
let next = store
.reserved_sessions_id
.iter()
.filter_map(|s| s.parse::<i32>().ok())
.max()
.map(|m| std::cmp::max(m, 999) + 1)
.unwrap_or(1000);
next.to_string()
};
if store.reserved_sessions_id.contains(&process_id) {
continue;
}
store.reserved_sessions_id.insert(process_id.clone());
return process_id;
}
}
pub(crate) async fn exec_command(
&self,
request: ExecCommandRequest,
@@ -122,14 +153,20 @@ impl UnifiedExecSessionManager {
let has_exited = session.has_exited();
let exit_code = session.exit_code();
let chunk_id = generate_chunk_id();
let session_id = if has_exited {
let process_id = if has_exited {
None
} else {
// Only store session if not exited.
let stored_id = self
.store_session(session, context, &request.command, cwd.clone(), start)
.await;
Some(stored_id)
self.store_session(
session,
context,
&request.command,
cwd.clone(),
start,
request.process_id.clone(),
)
.await;
Some(request.process_id.clone())
};
let original_token_count = approx_token_count(&text);
@@ -138,18 +175,18 @@ impl UnifiedExecSessionManager {
chunk_id,
wall_time,
output,
session_id,
process_id: process_id.clone(),
exit_code,
original_token_count: Some(original_token_count),
session_command: Some(request.command.clone()),
};
if response.session_id.is_some() {
if !has_exited {
Self::emit_waiting_status(&context.session, &context.turn, &request.command).await;
}
// If the command completed during this call, emit an ExecCommandEnd via the emitter.
if response.session_id.is_none() {
if has_exited {
let exit = response.exit_code.unwrap_or(-1);
Self::emit_exec_end_from_context(
context,
@@ -158,6 +195,9 @@ impl UnifiedExecSessionManager {
response.output.clone(),
exit,
response.wall_time,
// We always emit the process ID in order to keep consistency between the Begin
// event and the End event.
Some(request.process_id),
)
.await;
}
@@ -169,7 +209,7 @@ impl UnifiedExecSessionManager {
&self,
request: WriteStdinRequest<'_>,
) -> Result<UnifiedExecResponse, UnifiedExecError> {
let session_id = request.session_id;
let process_id = request.process_id.to_string();
let PreparedSessionHandles {
writer_tx,
@@ -180,13 +220,15 @@ impl UnifiedExecSessionManager {
turn_ref,
command: session_command,
cwd: session_cwd,
} = self.prepare_session_handles(session_id).await?;
process_id,
} = self.prepare_session_handles(process_id.as_str()).await?;
let interaction_emitter = ToolEmitter::unified_exec(
&session_command,
session_cwd.clone(),
ExecCommandSource::UnifiedExecInteraction,
(!request.input.is_empty()).then(|| request.input.to_string()),
Some(process_id.clone()),
);
let make_event_ctx = || {
ToolEventCtx::new(
@@ -233,17 +275,21 @@ impl UnifiedExecSessionManager {
let original_token_count = approx_token_count(&text);
let chunk_id = generate_chunk_id();
let status = self.refresh_session_state(session_id).await;
let (session_id, exit_code, completion_entry, event_call_id) = match status {
SessionStatus::Alive { exit_code, call_id } => {
(Some(session_id), exit_code, None, call_id)
}
let status = self.refresh_session_state(process_id.as_str()).await;
let (process_id, exit_code, completion_entry, event_call_id) = match status {
SessionStatus::Alive {
exit_code,
call_id,
process_id,
} => (Some(process_id), exit_code, None, call_id),
SessionStatus::Exited { exit_code, entry } => {
let call_id = entry.call_id.clone();
(None, exit_code, Some(*entry), call_id)
}
SessionStatus::Unknown => {
return Err(UnifiedExecError::UnknownSessionId { session_id });
return Err(UnifiedExecError::UnknownSessionId {
process_id: request.process_id.to_string(),
});
}
};
@@ -252,7 +298,7 @@ impl UnifiedExecSessionManager {
chunk_id,
wall_time,
output,
session_id,
process_id,
exit_code,
original_token_count: Some(original_token_count),
session_command: Some(session_command.clone()),
@@ -273,7 +319,7 @@ impl UnifiedExecSessionManager {
)
.await;
if response.session_id.is_some() {
if response.process_id.is_some() {
Self::emit_waiting_status(&session_ref, &turn_ref, &session_command).await;
}
@@ -286,16 +332,17 @@ impl UnifiedExecSessionManager {
Ok(response)
}
async fn refresh_session_state(&self, session_id: i32) -> SessionStatus {
let mut sessions = self.sessions.lock().await;
let Some(entry) = sessions.get(&session_id) else {
async fn refresh_session_state(&self, process_id: &str) -> SessionStatus {
let mut store = self.session_store.lock().await;
let Some(entry) = store.sessions.get(process_id) else {
return SessionStatus::Unknown;
};
let exit_code = entry.session.exit_code();
let process_id = entry.process_id.clone();
if entry.session.has_exited() {
let Some(entry) = sessions.remove(&session_id) else {
let Some(entry) = store.remove(&process_id) else {
return SessionStatus::Unknown;
};
SessionStatus::Exited {
@@ -306,18 +353,23 @@ impl UnifiedExecSessionManager {
SessionStatus::Alive {
exit_code,
call_id: entry.call_id.clone(),
process_id,
}
}
}
async fn prepare_session_handles(
&self,
session_id: i32,
process_id: &str,
) -> Result<PreparedSessionHandles, UnifiedExecError> {
let mut sessions = self.sessions.lock().await;
let entry = sessions
.get_mut(&session_id)
.ok_or(UnifiedExecError::UnknownSessionId { session_id })?;
let mut store = self.session_store.lock().await;
let entry =
store
.sessions
.get_mut(process_id)
.ok_or(UnifiedExecError::UnknownSessionId {
process_id: process_id.to_string(),
})?;
entry.last_used = Instant::now();
let OutputHandles {
output_buffer,
@@ -334,6 +386,7 @@ impl UnifiedExecSessionManager {
turn_ref: Arc::clone(&entry.turn_ref),
command: entry.command.clone(),
cwd: entry.cwd.clone(),
process_id: entry.process_id.clone(),
})
}
@@ -347,6 +400,7 @@ impl UnifiedExecSessionManager {
.map_err(|_| UnifiedExecError::WriteToStdin)
}
#[allow(clippy::too_many_arguments)]
async fn store_session(
&self,
session: UnifiedExecSession,
@@ -354,24 +408,22 @@ impl UnifiedExecSessionManager {
command: &[String],
cwd: PathBuf,
started_at: Instant,
) -> i32 {
let session_id = self
.next_session_id
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
process_id: String,
) {
let entry = SessionEntry {
session,
session_ref: Arc::clone(&context.session),
turn_ref: Arc::clone(&context.turn),
call_id: context.call_id.clone(),
process_id: process_id.clone(),
command: command.to_vec(),
cwd,
started_at,
last_used: started_at,
};
let mut sessions = self.sessions.lock().await;
Self::prune_sessions_if_needed(&mut sessions);
sessions.insert(session_id, entry);
session_id
let mut store = self.session_store.lock().await;
Self::prune_sessions_if_needed(&mut store);
store.sessions.insert(process_id, entry);
}
async fn emit_exec_end_from_entry(
@@ -399,6 +451,7 @@ impl UnifiedExecSessionManager {
entry.cwd,
ExecCommandSource::UnifiedExecStartup,
None,
Some(entry.process_id.clone()),
);
emitter
.emit(event_ctx, ToolEventStage::Success(output))
@@ -412,6 +465,7 @@ impl UnifiedExecSessionManager {
aggregated_output: String,
exit_code: i32,
duration: Duration,
process_id: Option<String>,
) {
let output = ExecToolCallOutput {
exit_code,
@@ -427,8 +481,13 @@ impl UnifiedExecSessionManager {
&context.call_id,
None,
);
let emitter =
ToolEmitter::unified_exec(command, cwd, ExecCommandSource::UnifiedExecStartup, None);
let emitter = ToolEmitter::unified_exec(
command,
cwd,
ExecCommandSource::UnifiedExecStartup,
None,
process_id,
);
emitter
.emit(event_ctx, ToolEventStage::Success(output))
.await;
@@ -574,52 +633,53 @@ impl UnifiedExecSessionManager {
collected
}
fn prune_sessions_if_needed(sessions: &mut HashMap<i32, SessionEntry>) {
if sessions.len() < MAX_UNIFIED_EXEC_SESSIONS {
fn prune_sessions_if_needed(store: &mut SessionStore) {
if store.sessions.len() < MAX_UNIFIED_EXEC_SESSIONS {
return;
}
let meta: Vec<(i32, Instant, bool)> = sessions
let meta: Vec<(String, Instant, bool)> = store
.sessions
.iter()
.map(|(id, entry)| (*id, entry.last_used, entry.session.has_exited()))
.map(|(id, entry)| (id.clone(), entry.last_used, entry.session.has_exited()))
.collect();
if let Some(session_id) = Self::session_id_to_prune_from_meta(&meta) {
sessions.remove(&session_id);
store.remove(&session_id);
}
}
// Centralized pruning policy so we can easily swap strategies later.
fn session_id_to_prune_from_meta(meta: &[(i32, Instant, bool)]) -> Option<i32> {
fn session_id_to_prune_from_meta(meta: &[(String, Instant, bool)]) -> Option<String> {
if meta.is_empty() {
return None;
}
let mut by_recency = meta.to_vec();
by_recency.sort_by_key(|(_, last_used, _)| Reverse(*last_used));
let protected: HashSet<i32> = by_recency
let protected: HashSet<String> = by_recency
.iter()
.take(8)
.map(|(session_id, _, _)| *session_id)
.map(|(process_id, _, _)| process_id.clone())
.collect();
let mut lru = meta.to_vec();
lru.sort_by_key(|(_, last_used, _)| *last_used);
if let Some((session_id, _, _)) = lru
if let Some((process_id, _, _)) = lru
.iter()
.find(|(session_id, _, exited)| !protected.contains(session_id) && *exited)
.find(|(process_id, _, exited)| !protected.contains(process_id) && *exited)
{
return Some(*session_id);
return Some(process_id.clone());
}
lru.into_iter()
.find(|(session_id, _, _)| !protected.contains(session_id))
.map(|(session_id, _, _)| session_id)
.find(|(process_id, _, _)| !protected.contains(process_id))
.map(|(process_id, _, _)| process_id)
}
pub(crate) async fn terminate_all_sessions(&self) {
let mut sessions = self.sessions.lock().await;
let mut sessions = self.session_store.lock().await;
sessions.clear();
}
}
@@ -628,6 +688,7 @@ enum SessionStatus {
Alive {
exit_code: Option<i32>,
call_id: String,
process_id: String,
},
Exited {
exit_code: Option<i32>,
@@ -675,64 +736,67 @@ mod tests {
#[test]
fn pruning_prefers_exited_sessions_outside_recently_used() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), true),
(3, now - Duration::from_secs(20), false),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), false),
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), true),
(id(3), now - Duration::from_secs(20), false),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), false),
];
let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta);
assert_eq!(candidate, Some(2));
assert_eq!(candidate, Some(id(2)));
}
#[test]
fn pruning_falls_back_to_lru_when_no_exited() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), false),
(3, now - Duration::from_secs(20), false),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), false),
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), false),
(id(3), now - Duration::from_secs(20), false),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), false),
];
let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta);
assert_eq!(candidate, Some(1));
assert_eq!(candidate, Some(id(1)));
}
#[test]
fn pruning_protects_recent_sessions_even_if_exited() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), false),
(3, now - Duration::from_secs(20), true),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), true),
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), false),
(id(3), now - Duration::from_secs(20), true),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), true),
];
let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta);
// (10) is exited but among the last 8; we should drop the LRU outside that set.
assert_eq!(candidate, Some(1));
assert_eq!(candidate, Some(id(1)));
}
}

View File

@@ -70,7 +70,6 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() {
review_request: ReviewRequest {
prompt: "Please review".to_string(),
user_facing_hint: "review".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -146,7 +145,6 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() {
review_request: ReviewRequest {
prompt: "Please review".to_string(),
user_facing_hint: "review".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -201,7 +199,6 @@ async fn codex_delegate_ignores_legacy_deltas() {
review_request: ReviewRequest {
prompt: "Please review".to_string(),
user_facing_hint: "review".to_string(),
append_to_original_thread: true,
},
})
.await

View File

@@ -18,6 +18,7 @@ use codex_core::protocol::ReviewOutputEvent;
use codex_core::protocol::ReviewRequest;
use codex_core::protocol::RolloutItem;
use codex_core::protocol::RolloutLine;
use codex_core::review_format::render_review_output_text;
use codex_protocol::user_input::UserInput;
use core_test_support::load_default_config_for_test;
use core_test_support::load_sse_fixture_with_id_from_str;
@@ -82,7 +83,6 @@ async fn review_op_emits_lifecycle_and_review_output() {
review_request: ReviewRequest {
prompt: "Please review my changes".to_string(),
user_facing_hint: "my changes".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -124,22 +124,36 @@ async fn review_op_emits_lifecycle_and_review_output() {
let mut saw_header = false;
let mut saw_finding_line = false;
let expected_assistant_text = render_review_output_text(&expected);
let mut saw_assistant_plain = false;
let mut saw_assistant_xml = false;
for line in text.lines() {
if line.trim().is_empty() {
continue;
}
let v: serde_json::Value = serde_json::from_str(line).expect("jsonl line");
let rl: RolloutLine = serde_json::from_value(v).expect("rollout line");
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item
&& role == "user"
{
for c in content {
if let ContentItem::InputText { text } = c {
if text.contains("full review output from reviewer model") {
saw_header = true;
if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item {
if role == "user" {
for c in content {
if let ContentItem::InputText { text } = c {
if text.contains("full review output from reviewer model") {
saw_header = true;
}
if text.contains("- Prefer Stylize helpers — /tmp/file.rs:10-20") {
saw_finding_line = true;
}
}
if text.contains("- Prefer Stylize helpers — /tmp/file.rs:10-20") {
saw_finding_line = true;
}
} else if role == "assistant" {
for c in content {
if let ContentItem::OutputText { text } = c {
if text.contains("<user_action>") {
saw_assistant_xml = true;
}
if text == expected_assistant_text {
saw_assistant_plain = true;
}
}
}
}
@@ -150,6 +164,14 @@ async fn review_op_emits_lifecycle_and_review_output() {
saw_finding_line,
"formatted finding line missing from rollout"
);
assert!(
saw_assistant_plain,
"assistant review output missing from rollout"
);
assert!(
!saw_assistant_xml,
"assistant review output contains user_action markup"
);
server.verify().await;
}
@@ -179,7 +201,6 @@ async fn review_op_with_plain_text_emits_review_fallback() {
review_request: ReviewRequest {
prompt: "Plain text review".to_string(),
user_facing_hint: "plain text review".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -238,7 +259,6 @@ async fn review_filters_agent_message_related_events() {
review_request: ReviewRequest {
prompt: "Filter streaming events".to_string(),
user_facing_hint: "Filter streaming events".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -247,7 +267,7 @@ async fn review_filters_agent_message_related_events() {
let mut saw_entered = false;
let mut saw_exited = false;
// Drain until TaskComplete; assert filtered events never surface.
// Drain until TaskComplete; assert streaming-related events never surface.
wait_for_event(&codex, |event| match event {
EventMsg::TaskComplete(_) => true,
EventMsg::EnteredReviewMode(_) => {
@@ -265,12 +285,6 @@ async fn review_filters_agent_message_related_events() {
EventMsg::AgentMessageDelta(_) => {
panic!("unexpected AgentMessageDelta surfaced during review")
}
EventMsg::ItemCompleted(ev) => match &ev.item {
codex_protocol::items::TurnItem::AgentMessage(_) => {
panic!("unexpected ItemCompleted for TurnItem::AgentMessage surfaced during review")
}
_ => false,
},
_ => false,
})
.await;
@@ -279,8 +293,9 @@ async fn review_filters_agent_message_related_events() {
server.verify().await;
}
/// When the model returns structured JSON in a review, ensure no AgentMessage
/// is emitted; the UI consumes the structured result via ExitedReviewMode.
/// When the model returns structured JSON in a review, ensure only a single
/// non-streaming AgentMessage is emitted; the UI consumes the structured
/// result via ExitedReviewMode plus a final assistant message.
// Windows CI only: bump to 4 workers to prevent SSE/event starvation and test timeouts.
#[cfg_attr(windows, tokio::test(flavor = "multi_thread", worker_threads = 4))]
#[cfg_attr(not(windows), tokio::test(flavor = "multi_thread", worker_threads = 2))]
@@ -323,19 +338,21 @@ async fn review_does_not_emit_agent_message_on_structured_output() {
review_request: ReviewRequest {
prompt: "check structured".to_string(),
user_facing_hint: "check structured".to_string(),
append_to_original_thread: true,
},
})
.await
.unwrap();
// Drain events until TaskComplete; ensure none are AgentMessage.
// Drain events until TaskComplete; ensure we only see a final
// AgentMessage (no streaming assistant messages).
let mut saw_entered = false;
let mut saw_exited = false;
let mut agent_messages = 0;
wait_for_event(&codex, |event| match event {
EventMsg::TaskComplete(_) => true,
EventMsg::AgentMessage(_) => {
panic!("unexpected AgentMessage during review with structured output")
agent_messages += 1;
false
}
EventMsg::EnteredReviewMode(_) => {
saw_entered = true;
@@ -348,6 +365,7 @@ async fn review_does_not_emit_agent_message_on_structured_output() {
_ => false,
})
.await;
assert_eq!(1, agent_messages, "expected exactly one AgentMessage event");
assert!(saw_entered && saw_exited, "missing review lifecycle events");
server.verify().await;
@@ -377,7 +395,6 @@ async fn review_uses_custom_review_model_from_config() {
review_request: ReviewRequest {
prompt: "use custom model".to_string(),
user_facing_hint: "use custom model".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -495,7 +512,6 @@ async fn review_input_isolated_from_parent_history() {
review_request: ReviewRequest {
prompt: review_prompt.clone(),
user_facing_hint: review_prompt.clone(),
append_to_original_thread: true,
},
})
.await
@@ -583,11 +599,10 @@ async fn review_input_isolated_from_parent_history() {
server.verify().await;
}
/// After a review thread finishes, its conversation should not leak into the
/// parent session. A subsequent parent turn must not include any review
/// messages in its request `input`.
/// After a review thread finishes, its conversation should be visible in the
/// parent session so later turns can reference the results.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_history_does_not_leak_into_parent_session() {
async fn review_history_surfaces_in_parent_session() {
skip_if_no_network!();
// Respond to both the review request and the subsequent parent request.
@@ -608,7 +623,6 @@ async fn review_history_does_not_leak_into_parent_session() {
review_request: ReviewRequest {
prompt: "Start a review".to_string(),
user_facing_hint: "Start a review".to_string(),
append_to_original_thread: true,
},
})
.await
@@ -651,20 +665,26 @@ async fn review_history_does_not_leak_into_parent_session() {
let last_text = last["content"][0]["text"].as_str().unwrap();
assert_eq!(last_text, followup);
// Ensure no review-thread content leaked into the parent request
let contains_review_prompt = input
.iter()
.any(|msg| msg["content"][0]["text"].as_str().unwrap_or_default() == "Start a review");
// Ensure review-thread content is present for downstream turns.
let contains_review_rollout_user = input.iter().any(|msg| {
msg["content"][0]["text"]
.as_str()
.unwrap_or_default()
.contains("User initiated a review task.")
});
let contains_review_assistant = input.iter().any(|msg| {
msg["content"][0]["text"].as_str().unwrap_or_default() == "review assistant output"
msg["content"][0]["text"]
.as_str()
.unwrap_or_default()
.contains("review assistant output")
});
assert!(
!contains_review_prompt,
"review prompt leaked into parent turn input"
contains_review_rollout_user,
"review rollout user message missing from parent turn input"
);
assert!(
!contains_review_assistant,
"review assistant output leaked into parent turn input"
contains_review_assistant,
"review assistant output missing from parent turn input"
);
server.verify().await;

View File

@@ -44,7 +44,7 @@ fn extract_output_text(item: &Value) -> Option<&str> {
struct ParsedUnifiedExecOutput {
chunk_id: Option<String>,
wall_time_seconds: f64,
session_id: Option<i32>,
process_id: Option<String>,
exit_code: Option<i32>,
original_token_count: Option<usize>,
output: String,
@@ -59,7 +59,7 @@ fn parse_unified_exec_output(raw: &str) -> Result<ParsedUnifiedExecOutput> {
r#"(?:Chunk ID: (?P<chunk_id>[^\n]+)\n)?"#,
r#"Wall time: (?P<wall_time>-?\d+(?:\.\d+)?) seconds\n"#,
r#"(?:Process exited with code (?P<exit_code>-?\d+)\n)?"#,
r#"(?:Process running with session ID (?P<session_id>-?\d+)\n)?"#,
r#"(?:Process running with session ID (?P<process_id>-?\d+)\n)?"#,
r#"(?:Original token count: (?P<original_token_count>\d+)\n)?"#,
r#"Output:\n?(?P<output>.*)$"#,
))
@@ -92,15 +92,9 @@ fn parse_unified_exec_output(raw: &str) -> Result<ParsedUnifiedExecOutput> {
})
.transpose()?;
let session_id = captures
.name("session_id")
.map(|value| {
value
.as_str()
.parse::<i32>()
.context("failed to parse session id from unified exec output")
})
.transpose()?;
let process_id = captures
.name("process_id")
.map(|value| value.as_str().to_string());
let original_token_count = captures
.name("original_token_count")
@@ -121,7 +115,7 @@ fn parse_unified_exec_output(raw: &str) -> Result<ParsedUnifiedExecOutput> {
Ok(ParsedUnifiedExecOutput {
chunk_id,
wall_time_seconds,
session_id,
process_id,
exit_code,
original_token_count,
output,
@@ -335,7 +329,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> {
let poll_call_id = "uexec-end-event-poll";
let poll_args = json!({
"chars": "",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 250,
});
@@ -493,7 +487,7 @@ async fn unified_exec_emits_output_delta_for_write_stdin() -> Result<()> {
let stdin_call_id = "uexec-stdin-delta";
let stdin_args = json!({
"chars": "echo WSTDIN-MARK\\n",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 800,
});
@@ -592,7 +586,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> {
let stdin_call_id = "uexec-stdin-begin";
let stdin_args = json!({
"chars": "echo hello",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 400,
});
@@ -694,7 +688,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()>
let poll_call_id = "uexec-poll-empty";
let poll_args = json!({
"chars": "",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 150,
});
@@ -880,8 +874,8 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
);
assert!(
metadata.session_id.is_none(),
"exec_command for a completed process should not include session_id"
metadata.process_id.is_none(),
"exec_command for a completed process should not include process_id"
);
let exit_code = metadata.exit_code.expect("expected exit_code");
@@ -973,7 +967,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
.expect("missing early exit unified_exec output");
assert!(
output.session_id.is_none(),
output.process_id.is_none(),
"short-lived process should not keep a session alive"
);
assert_eq!(
@@ -1023,12 +1017,12 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
});
let send_args = serde_json::json!({
"chars": "hello unified exec\n",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 500,
});
let exit_args = serde_json::json!({
"chars": "\u{0004}",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 500,
});
@@ -1099,12 +1093,13 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
let start_output = outputs
.get(start_call_id)
.expect("missing start output for exec_command");
let session_id = start_output
.session_id
.expect("expected session id from exec_command");
let process_id = start_output
.process_id
.clone()
.expect("expected process id from exec_command");
assert!(
session_id >= 0,
"session_id should be non-negative, got {session_id}"
process_id.len() > 3,
"process_id should be at least 4 digits, got {process_id}"
);
assert!(
start_output.exit_code.is_none(),
@@ -1120,11 +1115,12 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
"expected echoed output from cat, got {echoed:?}"
);
let echoed_session = send_output
.session_id
.expect("write_stdin should return session id while process is running");
.process_id
.clone()
.expect("write_stdin should return process id while process is running");
assert_eq!(
echoed_session, session_id,
"write_stdin should reuse existing session id"
echoed_session, process_id,
"write_stdin should reuse existing process id"
);
assert!(
send_output.exit_code.is_none(),
@@ -1135,8 +1131,8 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
.get(exit_call_id)
.expect("missing exit metadata output");
assert!(
exit_output.session_id.is_none(),
"session_id should be omitted once the process exits"
exit_output.process_id.is_none(),
"process_id should be omitted once the process exits"
);
let exit_code = exit_output
.exit_code
@@ -1182,14 +1178,14 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<()
let echo_call_id = "uexec-end-on-exit-echo";
let echo_args = serde_json::json!({
"chars": "bye-END\n",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 300,
});
let exit_call_id = "uexec-end-on-exit";
let exit_args = serde_json::json!({
"chars": "\u{0004}",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 500,
});
@@ -1285,7 +1281,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
let second_call_id = "uexec-stdin";
let second_args = serde_json::json!({
"chars": "hello unified exec\n",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 500,
});
@@ -1347,17 +1343,20 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
let start_output = outputs
.get(first_call_id)
.expect("missing first unified_exec output");
let session_id = start_output.session_id.unwrap_or_default();
let process_id = start_output.process_id.clone().unwrap_or_default();
assert!(
session_id >= 0,
"expected session id in first unified_exec response"
!process_id.is_empty(),
"expected process id in first unified_exec response"
);
assert!(start_output.output.is_empty());
let reuse_output = outputs
.get(second_call_id)
.expect("missing reused unified_exec output");
assert_eq!(reuse_output.session_id.unwrap_or_default(), session_id);
assert_eq!(
reuse_output.process_id.clone().unwrap_or_default(),
process_id
);
let echoed = reuse_output.output.as_str();
assert!(
echoed.contains("hello unified exec"),
@@ -1413,7 +1412,7 @@ PY
let second_call_id = "uexec-lag-poll";
let second_args = serde_json::json!({
"chars": "",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 2_000,
});
@@ -1480,9 +1479,9 @@ PY
let start_output = outputs
.get(first_call_id)
.expect("missing initial unified_exec output");
let session_id = start_output.session_id.unwrap_or_default();
let process_id = start_output.process_id.clone().unwrap_or_default();
assert!(
session_id >= 0,
!process_id.is_empty(),
"expected session id from initial unified_exec response"
);
@@ -1524,7 +1523,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
let second_call_id = "uexec-poll";
let second_args = serde_json::json!({
"chars": "",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 800,
});
@@ -1589,7 +1588,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
let outputs = collect_tool_outputs(&bodies)?;
let first_output = outputs.get(first_call_id).expect("missing timeout output");
assert_eq!(first_output.session_id, Some(0));
assert!(first_output.process_id.is_some());
assert!(first_output.output.is_empty());
let poll_output = outputs.get(second_call_id).expect("missing poll output");
@@ -1824,7 +1823,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
let keep_write_call_id = "uexec-prune-keep-write";
let keep_write_args = serde_json::json!({
"chars": "still alive\n",
"session_id": 0,
"session_id": 1000,
"yield_time_ms": 500,
});
events.push(ev_function_call(
@@ -1836,7 +1835,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
let probe_call_id = "uexec-prune-probe";
let probe_args = serde_json::json!({
"chars": "should fail\n",
"session_id": 1,
"session_id": 1001,
"yield_time_ms": 500,
});
events.push(ev_function_call(
@@ -1885,7 +1884,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
.find_map(|req| req.function_call_output_text(keep_call_id))
.expect("missing initial keep session output");
let keep_start_output = parse_unified_exec_output(&keep_start)?;
pretty_assertions::assert_eq!(keep_start_output.session_id, Some(0));
assert!(keep_start_output.process_id.is_some());
assert!(keep_start_output.exit_code.is_none());
let prune_start = requests
@@ -1893,7 +1892,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
.find_map(|req| req.function_call_output_text(prune_call_id))
.expect("missing initial prune session output");
let prune_start_output = parse_unified_exec_output(&prune_start)?;
pretty_assertions::assert_eq!(prune_start_output.session_id, Some(1));
assert!(prune_start_output.process_id.is_some());
assert!(prune_start_output.exit_code.is_none());
let keep_write = requests
@@ -1901,7 +1900,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
.find_map(|req| req.function_call_output_text(keep_write_call_id))
.expect("missing keep write output");
let keep_write_output = parse_unified_exec_output(&keep_write)?;
pretty_assertions::assert_eq!(keep_write_output.session_id, Some(0));
assert!(keep_write_output.process_id.is_some());
assert!(
keep_write_output.output.contains("still alive"),
"expected cat session to echo input, got {:?}",
@@ -1913,7 +1912,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
.find_map(|req| req.function_call_output_text(probe_call_id))
.expect("missing probe output");
assert!(
pruned_probe.contains("UnknownSessionId") || pruned_probe.contains("Unknown session id"),
pruned_probe.contains("UnknownSessionId") || pruned_probe.contains("Unknown process id"),
"expected probe to fail after pruning, got {pruned_probe:?}"
);

View File

@@ -406,6 +406,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
FileChange::Update {
unified_diff,
move_path,
..
} => {
let header = if let Some(dest) = move_path {
format!(

View File

@@ -637,6 +637,7 @@ fn exec_command_end_success_produces_completed_command_item() {
"c1",
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
call_id: "1".to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command: command.clone(),
cwd: cwd.clone(),
@@ -666,6 +667,7 @@ fn exec_command_end_success_produces_completed_command_item() {
"c2",
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: "1".to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command,
cwd,
@@ -709,6 +711,7 @@ fn exec_command_end_failure_produces_failed_command_item() {
"c1",
EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
call_id: "2".to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command: command.clone(),
cwd: cwd.clone(),
@@ -737,6 +740,7 @@ fn exec_command_end_failure_produces_failed_command_item() {
"c2",
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: "2".to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command,
cwd,
@@ -777,6 +781,7 @@ fn exec_command_end_without_begin_is_ignored() {
"c1",
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: "no-begin".to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command: Vec::new(),
cwd: PathBuf::from("."),
@@ -818,6 +823,8 @@ fn patch_apply_success_produces_item_completed_patchapply() {
FileChange::Update {
unified_diff: "--- c/modified.txt\n+++ c/modified.txt\n@@\n-old\n+new\n".to_string(),
move_path: Some(PathBuf::from("c/renamed.txt")),
old_content: "-old\n".to_string(),
new_content: "+new\n".to_string(),
},
);
@@ -890,6 +897,8 @@ fn patch_apply_failure_produces_item_completed_patchapply_failed() {
FileChange::Update {
unified_diff: "--- file.txt\n+++ file.txt\n@@\n-old\n+new\n".to_string(),
move_path: None,
old_content: "-old\n".to_string(),
new_content: "+new\n".to_string(),
},
);

View File

@@ -14,7 +14,7 @@ codex-core = { path = "../core" }
reqwest = { version = "0.12", features = ["json", "stream"] }
serde_json = "1"
tokio = { version = "1", features = ["rt"] }
tracing = { version = "0.1.41", features = ["log"] }
tracing = { version = "0.1.43", features = ["log"] }
which = "6.0"
[dev-dependencies]

View File

@@ -269,6 +269,8 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> {
FileChange::Update {
unified_diff: "@@ -1 +1 @@\n-original content\n+modified content\n".to_string(),
move_path: None,
old_content: "original content\n".to_string(),
new_content: "modified content\n".to_string(),
},
);

View File

@@ -1256,13 +1256,18 @@ pub struct GitInfo {
pub repository_url: Option<String>,
}
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
pub enum ReviewDelivery {
Inline,
Detached,
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
/// Review request sent to the review session.
pub struct ReviewRequest {
pub prompt: String,
pub user_facing_hint: String,
#[serde(default)]
pub append_to_original_thread: bool,
}
/// Structured review result produced by a child review session.
@@ -1328,6 +1333,10 @@ impl Default for ExecCommandSource {
pub struct ExecCommandBeginEvent {
/// Identifier so this can be paired with the ExecCommandEnd event.
pub call_id: String,
/// Identifier for the underlying PTY process (when available).
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub process_id: Option<String>,
/// Turn ID that this command belongs to.
pub turn_id: String,
/// The command to be executed.
@@ -1348,6 +1357,10 @@ pub struct ExecCommandBeginEvent {
pub struct ExecCommandEndEvent {
/// Identifier for the ExecCommandBegin that finished.
pub call_id: String,
/// Identifier for the underlying PTY process (when available).
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub process_id: Option<String>,
/// Turn ID that this command belongs to.
pub turn_id: String,
/// The command that was executed.
@@ -1640,6 +1653,8 @@ pub enum FileChange {
Update {
unified_diff: String,
move_path: Option<PathBuf>,
old_content: String,
new_content: String,
},
}
@@ -1704,6 +1719,21 @@ mod tests {
assert!(event.as_legacy_events(false).is_empty());
}
#[test]
fn file_change_update_serializes_old_and_new_content() {
let change = FileChange::Update {
unified_diff: "--- a\n+++ b\n@@ -1 +1 @@\n-old\n+new\n".to_string(),
move_path: None,
old_content: "old\n".to_string(),
new_content: "new\n".to_string(),
};
let serialized = serde_json::to_value(change).expect("should serialize");
assert_eq!(serialized["old_content"], "old\n");
assert_eq!(serialized["new_content"], "new\n");
}
/// Serialize Event to verify that its JSON representation has the expected
/// amount of nesting.
#[test]

View File

@@ -1560,6 +1560,8 @@ impl ChatWidget {
FileChange::Update {
unified_diff: "+test\n-test2".to_string(),
move_path: None,
old_content: "test2".to_string(),
new_content: "test".to_string(),
},
),
]),
@@ -2895,7 +2897,6 @@ impl ChatWidget {
review_request: ReviewRequest {
prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(),
user_facing_hint: "current changes".to_string(),
append_to_original_thread: true,
},
}));
},
@@ -2952,7 +2953,6 @@ impl ChatWidget {
"Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{{upstream}}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings."
),
user_facing_hint: format!("changes against '{branch}'"),
append_to_original_thread: true,
},
}));
})],
@@ -2993,7 +2993,6 @@ impl ChatWidget {
review_request: ReviewRequest {
prompt,
user_facing_hint: hint,
append_to_original_thread: true,
},
}));
})],
@@ -3028,7 +3027,6 @@ impl ChatWidget {
review_request: ReviewRequest {
prompt: trimmed.clone(),
user_facing_hint: trimmed,
append_to_original_thread: true,
},
}));
}),
@@ -3244,7 +3242,6 @@ pub(crate) fn show_review_commit_picker_with_entries(
review_request: ReviewRequest {
prompt,
user_facing_hint: hint,
append_to_original_thread: true,
},
}));
})],

View File

@@ -155,7 +155,6 @@ fn entered_review_mode_uses_request_hint() {
msg: EventMsg::EnteredReviewMode(ReviewRequest {
prompt: "Review the latest changes".to_string(),
user_facing_hint: "feature branch".to_string(),
append_to_original_thread: true,
}),
});
@@ -175,7 +174,6 @@ fn entered_review_mode_defaults_to_current_changes_banner() {
msg: EventMsg::EnteredReviewMode(ReviewRequest {
prompt: "Review the current changes".to_string(),
user_facing_hint: "current changes".to_string(),
append_to_original_thread: true,
}),
});
@@ -243,7 +241,6 @@ fn review_restores_context_window_indicator() {
msg: EventMsg::EnteredReviewMode(ReviewRequest {
prompt: "Review the latest changes".to_string(),
user_facing_hint: "feature branch".to_string(),
append_to_original_thread: true,
}),
});
@@ -722,6 +719,7 @@ fn begin_exec_with_source(
let interaction_input = None;
let event = ExecCommandBeginEvent {
call_id: call_id.to_string(),
process_id: None,
turn_id: "turn-1".to_string(),
command,
cwd,
@@ -760,11 +758,13 @@ fn end_exec(
parsed_cmd,
source,
interaction_input,
process_id,
} = begin_event;
chat.handle_codex_event(Event {
id: call_id.clone(),
msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id,
process_id,
turn_id,
command,
cwd,
@@ -2880,6 +2880,7 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() {
id: "c1".into(),
msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
call_id: "c1".into(),
process_id: None,
turn_id: "turn-1".into(),
command: command.clone(),
cwd: cwd.clone(),
@@ -2892,6 +2893,7 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() {
id: "c1".into(),
msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent {
call_id: "c1".into(),
process_id: None,
turn_id: "turn-1".into(),
command,
cwd,

View File

@@ -484,6 +484,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: None,
old_content: original.to_string(),
new_content: modified.to_string(),
},
);
@@ -504,6 +506,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: Some(PathBuf::from("new_name.rs")),
old_content: original.to_string(),
new_content: modified.to_string(),
},
);
@@ -524,6 +528,8 @@ mod tests {
FileChange::Update {
unified_diff: patch_a,
move_path: None,
old_content: "one\n".to_string(),
new_content: "one changed\n".to_string(),
},
);
@@ -590,6 +596,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: None,
old_content: original.to_string(),
new_content: modified.to_string(),
},
);
@@ -613,6 +621,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: None,
old_content: original.to_string(),
new_content: modified.to_string(),
},
);
@@ -640,6 +650,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: None,
old_content: original,
new_content: modified,
},
);
@@ -663,6 +675,8 @@ mod tests {
FileChange::Update {
unified_diff: patch,
move_path: Some(abs_new),
old_content: original.to_string(),
new_content: modified.to_string(),
},
);

View File

@@ -2,11 +2,11 @@
If you already lean on Codex every day and just need a little more control, this page collects the knobs you are most likely to reach for: tweak defaults in [Config](./config.md), add extra tools through [Model Context Protocol support](#model-context-protocol), and script full runs with [`codex exec`](./exec.md). Jump to the section you need and keep building.
## Config quickstart {#config-quickstart}
## Config quickstart
Most day-to-day tuning lives in `config.toml`: set approval + sandbox presets, pin model defaults, and add MCP server launchers. The [Config guide](./config.md) walks through every option and provides copy-paste examples for common setups.
## Tracing / verbose logging {#tracing-verbose-logging}
## Tracing / verbose logging
Because Codex is written in Rust, it honors the `RUST_LOG` environment variable to configure its logging behavior.
@@ -20,15 +20,15 @@ By comparison, the non-interactive mode (`codex exec`) defaults to `RUST_LOG=err
See the Rust documentation on [`RUST_LOG`](https://docs.rs/env_logger/latest/env_logger/#enabling-logging) for more information on the configuration options.
## Model Context Protocol (MCP) {#model-context-protocol}
## Model Context Protocol (MCP)
The Codex CLI and IDE extension is a MCP client which means that it can be configured to connect to MCP servers. For more information, refer to the [`config docs`](./config.md#mcp-integration).
## Using Codex as an MCP Server {#mcp-server}
## Using Codex as an MCP Server
The Codex CLI can also be run as an MCP _server_ via `codex mcp-server`. For example, you can use `codex mcp-server` to make Codex available as a tool inside of a multi-agent framework like the OpenAI [Agents SDK](https://platform.openai.com/docs/guides/agents). Use `codex mcp` separately to add/list/get/remove MCP server launchers in your configuration.
### Codex MCP Server Quickstart {#mcp-server-quickstart}
### Codex MCP Server Quickstart
You can launch a Codex MCP server with the [Model Context Protocol Inspector](https://modelcontextprotocol.io/legacy/tools/inspector):
@@ -58,7 +58,7 @@ Send a `tools/list` request and you will see that there are two tools available:
| **`prompt`** (required) | string | The next user prompt to continue the Codex conversation. |
| **`conversationId`** (required) | string | The id of the conversation to continue. |
### Trying it Out {#mcp-server-trying-it-out}
### Trying it Out
> [!TIP]
> Codex often takes a few minutes to run. To accommodate this, adjust the MCP inspector's Request and Total timeouts to 600000ms (10 minutes) under ⛭ Configuration.

View File

@@ -180,6 +180,9 @@ chatgpt_base_url = "https://chatgpt.com/backend-api/"
# Allowed values: chatgpt | api
# forced_login_method = "chatgpt"
# Preferred store for MCP OAuth credentials: auto (default) | file | keyring
mcp_oauth_credentials_store = "auto"
################################################################################
# Project Documentation Controls
################################################################################
@@ -232,16 +235,6 @@ experimental_use_rmcp_client = false
# Include apply_patch via freeform editing path (affects default tool set). Default: false
experimental_use_freeform_apply_patch = false
# Enable model-based sandbox command assessment. Default: false
experimental_sandbox_command_assessment = false
################################################################################
# MCP (Model Context Protocol) servers
################################################################################
# Preferred store for MCP OAuth credentials: auto (default) | file | keyring
mcp_oauth_credentials_store = "auto"
# Define MCP servers under this table. Leave empty to disable.
[mcp_servers]

View File

@@ -113,3 +113,7 @@ Paste images directly into the composer (Ctrl+V / Cmd+V) to attach them to your
codex -i screenshot.png "Explain this error"
codex --image img1.png,img2.jpg "Summarize these diagrams"
```
#### Environment variables and executables
Make sure your environment is already set up before launching Codex so it does not spend tokens probing what to activate. For example, source your Python virtualenv (or other language runtimes), start any required daemons, and export the env vars you expect to use ahead of time.

View File

@@ -1,3 +1,3 @@
## Platform sandboxing
This content now lives alongside the rest of the sandbox guidance. See [Sandbox mechanics by platform](./sandbox.md#platform-sandboxing-details) for up-to-date details.
This content now lives alongside the rest of the sandbox guidance. See [Sandbox mechanics by platform](./sandbox.md#sandbox-mechanics-by-platform) for up-to-date details.