diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4dfd7596d1..7cbb1a1983 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,12 +27,26 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile - # Run all tasks using workspace filters + # build_npm_package.py requires DotSlash when staging releases. + - uses: facebook/install-dotslash@v2 - - name: Ensure staging a release works. + - name: Stage npm package env: GH_TOKEN: ${{ github.token }} - run: ./codex-cli/scripts/stage_release.sh + run: | + set -euo pipefail + CODEX_VERSION=0.40.0 + PACK_OUTPUT="${RUNNER_TEMP}/codex-npm.tgz" + python3 ./codex-cli/scripts/build_npm_package.py \ + --release-version "$CODEX_VERSION" \ + --pack-output "$PACK_OUTPUT" + echo "PACK_OUTPUT=$PACK_OUTPUT" >> "$GITHUB_ENV" + + - name: Upload staged npm package artifact + uses: actions/upload-artifact@v4 + with: + name: codex-npm-staging + path: ${{ env.PACK_OUTPUT }} - name: Ensure root README.md contains only ASCII and certain Unicode code points run: ./scripts/asciicheck.py README.md diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index ebf55649b8..3d49986791 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -22,7 +22,7 @@ jobs: - name: Annotate locations with typos uses: codespell-project/codespell-problem-matcher@b80729f885d32f78a716c2f107b4db1025001c42 # v1 - name: Codespell - uses: codespell-project/actions-codespell@406322ec52dd7b488e48c1c4b82e2a8b3a1bf630 # v2 + uses: codespell-project/actions-codespell@406322ec52dd7b488e48c1c4b82e2a8b3a1bf630 # v2.1 with: ignore_words_file: .codespellignore skip: frame*.txt diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 280939c611..846408f3a8 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -57,7 +57,7 @@ jobs: working-directory: codex-rs steps: - uses: actions/checkout@v5 - - uses: dtolnay/rust-toolchain@1.89 + - uses: dtolnay/rust-toolchain@1.90 with: components: rustfmt - name: cargo fmt @@ -75,7 +75,7 @@ jobs: working-directory: codex-rs steps: - uses: actions/checkout@v5 - - uses: dtolnay/rust-toolchain@1.89 + - uses: dtolnay/rust-toolchain@1.90 - uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 with: tool: cargo-shear @@ -143,7 +143,7 @@ jobs: steps: - uses: actions/checkout@v5 - - uses: dtolnay/rust-toolchain@1.89 + - uses: dtolnay/rust-toolchain@1.90 with: targets: ${{ matrix.target }} components: clippy diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index e3776f149c..c808216d96 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -77,7 +77,7 @@ jobs: steps: - uses: actions/checkout@v5 - - uses: dtolnay/rust-toolchain@1.89 + - uses: dtolnay/rust-toolchain@1.90 with: targets: ${{ matrix.target }} @@ -173,6 +173,8 @@ jobs: outputs: version: ${{ steps.release_name.outputs.name }} tag: ${{ github.ref_name }} + should_publish_npm: ${{ steps.npm_publish_settings.outputs.should_publish }} + npm_tag: ${{ steps.npm_publish_settings.outputs.npm_tag }} steps: - name: Checkout repository @@ -193,21 +195,37 @@ jobs: version="${GITHUB_REF_NAME#rust-v}" echo "name=${version}" >> $GITHUB_OUTPUT + - name: Determine npm publish settings + id: npm_publish_settings + env: + VERSION: ${{ steps.release_name.outputs.name }} + run: | + set -euo pipefail + version="${VERSION}" + + if [[ "${version}" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "should_publish=true" >> "$GITHUB_OUTPUT" + echo "npm_tag=" >> "$GITHUB_OUTPUT" + elif [[ "${version}" =~ ^[0-9]+\.[0-9]+\.[0-9]+-alpha\.[0-9]+$ ]]; then + echo "should_publish=true" >> "$GITHUB_OUTPUT" + echo "npm_tag=alpha" >> "$GITHUB_OUTPUT" + else + echo "should_publish=false" >> "$GITHUB_OUTPUT" + echo "npm_tag=" >> "$GITHUB_OUTPUT" + fi + + # build_npm_package.py requires DotSlash when staging releases. + - uses: facebook/install-dotslash@v2 - name: Stage npm package env: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail TMP_DIR="${RUNNER_TEMP}/npm-stage" - python3 codex-cli/scripts/stage_rust_release.py \ + ./codex-cli/scripts/build_npm_package.py \ --release-version "${{ steps.release_name.outputs.name }}" \ - --tmp "${TMP_DIR}" - mkdir -p dist/npm - # Produce an npm-ready tarball using `npm pack` and store it in dist/npm. - # We then rename it to a stable name used by our publishing script. - (cd "$TMP_DIR" && npm pack --pack-destination "${GITHUB_WORKSPACE}/dist/npm") - mv "${GITHUB_WORKSPACE}"/dist/npm/*.tgz \ - "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ steps.release_name.outputs.name }}.tgz" + --staging-dir "${TMP_DIR}" \ + --pack-output "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ steps.release_name.outputs.name }}.tgz" - name: Create GitHub Release uses: softprops/action-gh-release@v2 @@ -230,8 +248,8 @@ jobs: # July 31, 2025: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/ # npm docs: https://docs.npmjs.com/trusted-publishers publish-npm: - # Skip this step for pre-releases (alpha/beta). - if: ${{ !contains(needs.release.outputs.version, '-') }} + # Publish to npm for stable releases and alpha pre-releases with numeric suffixes. + if: ${{ needs.release.outputs.should_publish_npm == 'true' }} name: publish-npm needs: release runs-on: ubuntu-latest @@ -266,7 +284,17 @@ jobs: # No NODE_AUTH_TOKEN needed because we use OIDC. - name: Publish to npm - run: npm publish "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${{ needs.release.outputs.version }}.tgz" + env: + VERSION: ${{ needs.release.outputs.version }} + NPM_TAG: ${{ needs.release.outputs.npm_tag }} + run: | + set -euo pipefail + tag_args=() + if [[ -n "${NPM_TAG}" ]]; then + tag_args+=(--tag "${NPM_TAG}") + fi + + npm publish "${GITHUB_WORKSPACE}/dist/npm/codex-npm-${VERSION}.tgz" "${tag_args[@]}" update-branch: name: Update latest-alpha-cli branch diff --git a/codex-cli/.gitignore b/codex-cli/.gitignore index f886e64f46..57872d0f1e 100644 --- a/codex-cli/.gitignore +++ b/codex-cli/.gitignore @@ -1,7 +1 @@ -# Added by ./scripts/install_native_deps.sh -/bin/codex-aarch64-apple-darwin -/bin/codex-aarch64-unknown-linux-musl -/bin/codex-linux-sandbox-arm64 -/bin/codex-linux-sandbox-x64 -/bin/codex-x86_64-apple-darwin -/bin/codex-x86_64-unknown-linux-musl +/vendor/ diff --git a/codex-cli/bin/codex.js b/codex-cli/bin/codex.js index f24065170d..3621f48c12 100755 --- a/codex-cli/bin/codex.js +++ b/codex-cli/bin/codex.js @@ -1,6 +1,7 @@ #!/usr/bin/env node // Unified entry point for the Codex CLI. +import { existsSync } from "fs"; import path from "path"; import { fileURLToPath } from "url"; @@ -40,10 +41,10 @@ switch (platform) { case "win32": switch (arch) { case "x64": - targetTriple = "x86_64-pc-windows-msvc.exe"; + targetTriple = "x86_64-pc-windows-msvc"; break; case "arm64": - targetTriple = "aarch64-pc-windows-msvc.exe"; + targetTriple = "aarch64-pc-windows-msvc"; break; default: break; @@ -57,7 +58,10 @@ if (!targetTriple) { throw new Error(`Unsupported platform: ${platform} (${arch})`); } -const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`); +const vendorRoot = path.join(__dirname, "..", "vendor"); +const archRoot = path.join(vendorRoot, targetTriple); +const codexBinaryName = process.platform === "win32" ? "codex.exe" : "codex"; +const binaryPath = path.join(archRoot, "codex", codexBinaryName); // Use an asynchronous spawn instead of spawnSync so that Node is able to // respond to signals (e.g. Ctrl-C / SIGINT) while the native binary is @@ -66,23 +70,6 @@ const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`); // receives a fatal signal, both processes exit in a predictable manner. const { spawn } = await import("child_process"); -async function tryImport(moduleName) { - try { - // eslint-disable-next-line node/no-unsupported-features/es-syntax - return await import(moduleName); - } catch (err) { - return null; - } -} - -async function resolveRgDir() { - const ripgrep = await tryImport("@vscode/ripgrep"); - if (!ripgrep?.rgPath) { - return null; - } - return path.dirname(ripgrep.rgPath); -} - function getUpdatedPath(newDirs) { const pathSep = process.platform === "win32" ? ";" : ":"; const existingPath = process.env.PATH || ""; @@ -94,9 +81,9 @@ function getUpdatedPath(newDirs) { } const additionalDirs = []; -const rgDir = await resolveRgDir(); -if (rgDir) { - additionalDirs.push(rgDir); +const pathDir = path.join(archRoot, "path"); +if (existsSync(pathDir)) { + additionalDirs.push(pathDir); } const updatedPath = getUpdatedPath(additionalDirs); diff --git a/codex-cli/bin/rg b/codex-cli/bin/rg new file mode 100755 index 0000000000..5a992570a9 --- /dev/null +++ b/codex-cli/bin/rg @@ -0,0 +1,79 @@ +#!/usr/bin/env dotslash + +{ + "name": "rg", + "platforms": { + "macos-aarch64": { + "size": 1787248, + "hash": "blake3", + "digest": "8d9942032585ea8ee805937634238d9aee7b210069f4703c88fbe568e26fb78a", + "format": "tar.gz", + "path": "ripgrep-14.1.1-aarch64-apple-darwin/rg", + "providers": [ + { + "url": "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/ripgrep-14.1.1-aarch64-apple-darwin.tar.gz" + } + ] + }, + "linux-aarch64": { + "size": 2047405, + "hash": "blake3", + "digest": "0b670b8fa0a3df2762af2fc82cc4932f684ca4c02dbd1260d4f3133fd4b2a515", + "format": "tar.gz", + "path": "ripgrep-14.1.1-aarch64-unknown-linux-gnu/rg", + "providers": [ + { + "url": "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/ripgrep-14.1.1-aarch64-unknown-linux-gnu.tar.gz" + } + ] + }, + "macos-x86_64": { + "size": 2082672, + "hash": "blake3", + "digest": "e9b862fc8da3127f92791f0ff6a799504154ca9d36c98bf3e60a81c6b1f7289e", + "format": "tar.gz", + "path": "ripgrep-14.1.1-x86_64-apple-darwin/rg", + "providers": [ + { + "url": "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/ripgrep-14.1.1-x86_64-apple-darwin.tar.gz" + } + ] + }, + "linux-x86_64": { + "size": 2566310, + "hash": "blake3", + "digest": "f73cca4e54d78c31f832c7f6e2c0b4db8b04fa3eaa747915727d570893dbee76", + "format": "tar.gz", + "path": "ripgrep-14.1.1-x86_64-unknown-linux-musl/rg", + "providers": [ + { + "url": "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/ripgrep-14.1.1-x86_64-unknown-linux-musl.tar.gz" + } + ] + }, + "windows-x86_64": { + "size": 2058893, + "hash": "blake3", + "digest": "a8ce1a6fed4f8093ee997e57f33254e94b2cd18e26358b09db599c89882eadbd", + "format": "zip", + "path": "ripgrep-14.1.1-x86_64-pc-windows-msvc/rg.exe", + "providers": [ + { + "url": "https://github.com/BurntSushi/ripgrep/releases/download/14.1.1/ripgrep-14.1.1-x86_64-pc-windows-msvc.zip" + } + ] + }, + "windows-aarch64": { + "size": 1667740, + "hash": "blake3", + "digest": "47b971a8c4fca1d23a4e7c19bd4d88465ebc395598458133139406d3bf85f3fa", + "format": "zip", + "path": "rg.exe", + "providers": [ + { + "url": "https://github.com/microsoft/ripgrep-prebuilt/releases/download/v13.0.0-13/ripgrep-v13.0.0-13-aarch64-pc-windows-msvc.zip" + } + ] + } + } +} diff --git a/codex-cli/package-lock.json b/codex-cli/package-lock.json index a1c840ade0..a28bb9cd9f 100644 --- a/codex-cli/package-lock.json +++ b/codex-cli/package-lock.json @@ -2,118 +2,17 @@ "name": "@openai/codex", "version": "0.0.0-dev", "lockfileVersion": 3, - "requires": true, "packages": { "": { "name": "@openai/codex", "version": "0.0.0-dev", "license": "Apache-2.0", - "dependencies": { - "@vscode/ripgrep": "^1.15.14" - }, "bin": { "codex": "bin/codex.js" }, "engines": { "node": ">=20" } - }, - "node_modules/@vscode/ripgrep": { - "version": "1.15.14", - "resolved": "https://registry.npmjs.org/@vscode/ripgrep/-/ripgrep-1.15.14.tgz", - "integrity": "sha512-/G1UJPYlm+trBWQ6cMO3sv6b8D1+G16WaJH1/DSqw32JOVlzgZbLkDxRyzIpTpv30AcYGMkCf5tUqGlW6HbDWw==", - "hasInstallScript": true, - "license": "MIT", - "dependencies": { - "https-proxy-agent": "^7.0.2", - "proxy-from-env": "^1.1.0", - "yauzl": "^2.9.2" - } - }, - "node_modules/agent-base": { - "version": "7.1.4", - "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-7.1.4.tgz", - "integrity": "sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==", - "license": "MIT", - "engines": { - "node": ">= 14" - } - }, - "node_modules/buffer-crc32": { - "version": "0.2.13", - "resolved": "https://registry.npmjs.org/buffer-crc32/-/buffer-crc32-0.2.13.tgz", - "integrity": "sha512-VO9Ht/+p3SN7SKWqcrgEzjGbRSJYTx+Q1pTQC0wrWqHx0vpJraQ6GtHx8tvcg1rlK1byhU5gccxgOgj7B0TDkQ==", - "license": "MIT", - "engines": { - "node": "*" - } - }, - "node_modules/debug": { - "version": "4.4.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.1.tgz", - "integrity": "sha512-KcKCqiftBJcZr++7ykoDIEwSa3XWowTfNPo92BYxjXiyYEVrUQh2aLyhxBCwww+heortUFxEJYcRzosstTEBYQ==", - "license": "MIT", - "dependencies": { - "ms": "^2.1.3" - }, - "engines": { - "node": ">=6.0" - }, - "peerDependenciesMeta": { - "supports-color": { - "optional": true - } - } - }, - "node_modules/fd-slicer": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.1.0.tgz", - "integrity": "sha512-cE1qsB/VwyQozZ+q1dGxR8LBYNZeofhEdUNGSMbQD3Gw2lAzX9Zb3uIU6Ebc/Fmyjo9AWWfnn0AUCHqtevs/8g==", - "license": "MIT", - "dependencies": { - "pend": "~1.2.0" - } - }, - "node_modules/https-proxy-agent": { - "version": "7.0.6", - "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz", - "integrity": "sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==", - "license": "MIT", - "dependencies": { - "agent-base": "^7.1.2", - "debug": "4" - }, - "engines": { - "node": ">= 14" - } - }, - "node_modules/ms": { - "version": "2.1.3", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", - "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", - "license": "MIT" - }, - "node_modules/pend": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/pend/-/pend-1.2.0.tgz", - "integrity": "sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==", - "license": "MIT" - }, - "node_modules/proxy-from-env": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", - "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "license": "MIT" - }, - "node_modules/yauzl": { - "version": "2.10.0", - "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz", - "integrity": "sha512-p4a9I6X6nu6IhoGmBqAcbJy1mlC4j27vEPZX9F4L4/vZT3Lyq1VkFHw/V/PUcB9Buo+DG3iHkT0x3Qya58zc3g==", - "license": "MIT", - "dependencies": { - "buffer-crc32": "~0.2.3", - "fd-slicer": "~1.1.0" - } } } } diff --git a/codex-cli/package.json b/codex-cli/package.json index 02124f3257..03f234d677 100644 --- a/codex-cli/package.json +++ b/codex-cli/package.json @@ -11,17 +11,11 @@ }, "files": [ "bin", - "dist" + "vendor" ], "repository": { "type": "git", "url": "git+https://github.com/openai/codex.git", "directory": "codex-cli" - }, - "dependencies": { - "@vscode/ripgrep": "^1.15.14" - }, - "devDependencies": { - "prettier": "^3.3.3" } } diff --git a/codex-cli/scripts/README.md b/codex-cli/scripts/README.md index 21e4f3e883..fd2265b299 100644 --- a/codex-cli/scripts/README.md +++ b/codex-cli/scripts/README.md @@ -5,5 +5,7 @@ Run the following: To build the 0.2.x or later version of the npm module, which runs the Rust version of the CLI, build it as follows: ```bash -./codex-cli/scripts/stage_rust_release.py --release-version 0.6.0 +./codex-cli/scripts/build_npm_package.py --release-version 0.6.0 ``` + +Note this will create `./codex-cli/vendor/` as a side-effect. diff --git a/codex-cli/scripts/build_npm_package.py b/codex-cli/scripts/build_npm_package.py new file mode 100755 index 0000000000..0401f10385 --- /dev/null +++ b/codex-cli/scripts/build_npm_package.py @@ -0,0 +1,269 @@ +#!/usr/bin/env python3 +"""Stage and optionally package the @openai/codex npm module.""" + +import argparse +import json +import re +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +SCRIPT_DIR = Path(__file__).resolve().parent +CODEX_CLI_ROOT = SCRIPT_DIR.parent +REPO_ROOT = CODEX_CLI_ROOT.parent +GITHUB_REPO = "openai/codex" + +# The docs are not clear on what the expected value/format of +# workflow/workflowName is: +# https://cli.github.com/manual/gh_run_list +WORKFLOW_NAME = ".github/workflows/rust-release.yml" + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Build or stage the Codex CLI npm package.") + parser.add_argument( + "--version", + help="Version number to write to package.json inside the staged package.", + ) + parser.add_argument( + "--release-version", + help=( + "Version to stage for npm release. When provided, the script also resolves the " + "matching rust-release workflow unless --workflow-url is supplied." + ), + ) + parser.add_argument( + "--workflow-url", + help="Optional GitHub Actions workflow run URL used to download native binaries.", + ) + parser.add_argument( + "--staging-dir", + type=Path, + help=( + "Directory to stage the package contents. Defaults to a new temporary directory " + "if omitted. The directory must be empty when provided." + ), + ) + parser.add_argument( + "--tmp", + dest="staging_dir", + type=Path, + help=argparse.SUPPRESS, + ) + parser.add_argument( + "--pack-output", + type=Path, + help="Path where the generated npm tarball should be written.", + ) + return parser.parse_args() + + +def main() -> int: + args = parse_args() + + version = args.version + release_version = args.release_version + if release_version: + if version and version != release_version: + raise RuntimeError("--version and --release-version must match when both are provided.") + version = release_version + + if not version: + raise RuntimeError("Must specify --version or --release-version.") + + staging_dir, created_temp = prepare_staging_dir(args.staging_dir) + + try: + stage_sources(staging_dir, version) + + workflow_url = args.workflow_url + resolved_head_sha: str | None = None + if not workflow_url: + if release_version: + workflow = resolve_release_workflow(version) + workflow_url = workflow["url"] + resolved_head_sha = workflow.get("headSha") + else: + workflow_url = resolve_latest_alpha_workflow_url() + elif release_version: + try: + workflow = resolve_release_workflow(version) + resolved_head_sha = workflow.get("headSha") + except Exception: + resolved_head_sha = None + + if release_version and resolved_head_sha: + print(f"should `git checkout {resolved_head_sha}`") + + if not workflow_url: + raise RuntimeError("Unable to determine workflow URL for native binaries.") + + install_native_binaries(staging_dir, workflow_url) + + if release_version: + staging_dir_str = str(staging_dir) + print( + f"Staged version {version} for release in {staging_dir_str}\n\n" + "Verify the CLI:\n" + f" node {staging_dir_str}/bin/codex.js --version\n" + f" node {staging_dir_str}/bin/codex.js --help\n\n" + ) + else: + print(f"Staged package in {staging_dir}") + + if args.pack_output is not None: + output_path = run_npm_pack(staging_dir, args.pack_output) + print(f"npm pack output written to {output_path}") + finally: + if created_temp: + # Preserve the staging directory for further inspection. + pass + + return 0 + + +def prepare_staging_dir(staging_dir: Path | None) -> tuple[Path, bool]: + if staging_dir is not None: + staging_dir = staging_dir.resolve() + staging_dir.mkdir(parents=True, exist_ok=True) + if any(staging_dir.iterdir()): + raise RuntimeError(f"Staging directory {staging_dir} is not empty.") + return staging_dir, False + + temp_dir = Path(tempfile.mkdtemp(prefix="codex-npm-stage-")) + return temp_dir, True + + +def stage_sources(staging_dir: Path, version: str) -> None: + bin_dir = staging_dir / "bin" + bin_dir.mkdir(parents=True, exist_ok=True) + + shutil.copy2(CODEX_CLI_ROOT / "bin" / "codex.js", bin_dir / "codex.js") + rg_manifest = CODEX_CLI_ROOT / "bin" / "rg" + if rg_manifest.exists(): + shutil.copy2(rg_manifest, bin_dir / "rg") + + readme_src = REPO_ROOT / "README.md" + if readme_src.exists(): + shutil.copy2(readme_src, staging_dir / "README.md") + + with open(CODEX_CLI_ROOT / "package.json", "r", encoding="utf-8") as fh: + package_json = json.load(fh) + package_json["version"] = version + + with open(staging_dir / "package.json", "w", encoding="utf-8") as out: + json.dump(package_json, out, indent=2) + out.write("\n") + + +def install_native_binaries(staging_dir: Path, workflow_url: str | None) -> None: + cmd = ["./scripts/install_native_deps.py"] + if workflow_url: + cmd.extend(["--workflow-url", workflow_url]) + cmd.append(str(staging_dir)) + subprocess.check_call(cmd, cwd=CODEX_CLI_ROOT) + + +def resolve_latest_alpha_workflow_url() -> str: + version = determine_latest_alpha_version() + workflow = resolve_release_workflow(version) + return workflow["url"] + + +def determine_latest_alpha_version() -> str: + releases = list_releases() + best_key: tuple[int, int, int, int] | None = None + best_version: str | None = None + pattern = re.compile(r"^rust-v(\d+)\.(\d+)\.(\d+)-alpha\.(\d+)$") + for release in releases: + tag = release.get("tag_name", "") + match = pattern.match(tag) + if not match: + continue + key = tuple(int(match.group(i)) for i in range(1, 5)) + if best_key is None or key > best_key: + best_key = key + best_version = ( + f"{match.group(1)}.{match.group(2)}.{match.group(3)}-alpha.{match.group(4)}" + ) + + if best_version is None: + raise RuntimeError("No alpha releases found when resolving workflow URL.") + return best_version + + +def list_releases() -> list[dict]: + stdout = subprocess.check_output( + ["gh", "api", f"/repos/{GITHUB_REPO}/releases?per_page=100"], + text=True, + ) + try: + releases = json.loads(stdout or "[]") + except json.JSONDecodeError as exc: + raise RuntimeError("Unable to parse releases JSON.") from exc + if not isinstance(releases, list): + raise RuntimeError("Unexpected response when listing releases.") + return releases + + +def resolve_release_workflow(version: str) -> dict: + stdout = subprocess.check_output( + [ + "gh", + "run", + "list", + "--branch", + f"rust-v{version}", + "--json", + "workflowName,url,headSha", + "--workflow", + WORKFLOW_NAME, + "--jq", + "first(.[])", + ], + text=True, + ) + workflow = json.loads(stdout or "[]") + if not workflow: + raise RuntimeError(f"Unable to find rust-release workflow for version {version}.") + return workflow + + +def run_npm_pack(staging_dir: Path, output_path: Path) -> Path: + output_path = output_path.resolve() + output_path.parent.mkdir(parents=True, exist_ok=True) + + with tempfile.TemporaryDirectory(prefix="codex-npm-pack-") as pack_dir_str: + pack_dir = Path(pack_dir_str) + stdout = subprocess.check_output( + ["npm", "pack", "--json", "--pack-destination", str(pack_dir)], + cwd=staging_dir, + text=True, + ) + try: + pack_output = json.loads(stdout) + except json.JSONDecodeError as exc: + raise RuntimeError("Failed to parse npm pack output.") from exc + + if not pack_output: + raise RuntimeError("npm pack did not produce an output tarball.") + + tarball_name = pack_output[0].get("filename") or pack_output[0].get("name") + if not tarball_name: + raise RuntimeError("Unable to determine npm pack output filename.") + + tarball_path = pack_dir / tarball_name + if not tarball_path.exists(): + raise RuntimeError(f"Expected npm pack output not found: {tarball_path}") + + shutil.move(str(tarball_path), output_path) + + return output_path + + +if __name__ == "__main__": + import sys + + sys.exit(main()) diff --git a/codex-cli/scripts/install_native_deps.py b/codex-cli/scripts/install_native_deps.py new file mode 100755 index 0000000000..7fbb44393a --- /dev/null +++ b/codex-cli/scripts/install_native_deps.py @@ -0,0 +1,318 @@ +#!/usr/bin/env python3 +"""Install Codex native binaries (Rust CLI plus ripgrep helpers).""" + +import argparse +import json +import os +import shutil +import subprocess +import tarfile +import tempfile +import zipfile +from concurrent.futures import ThreadPoolExecutor, as_completed +from pathlib import Path +from typing import Iterable, Sequence +from urllib.parse import urlparse +from urllib.request import urlopen + +SCRIPT_DIR = Path(__file__).resolve().parent +CODEX_CLI_ROOT = SCRIPT_DIR.parent +DEFAULT_WORKFLOW_URL = "https://github.com/openai/codex/actions/runs/17952349351" # rust-v0.40.0 +VENDOR_DIR_NAME = "vendor" +RG_MANIFEST = CODEX_CLI_ROOT / "bin" / "rg" +CODEX_TARGETS = ( + "x86_64-unknown-linux-musl", + "aarch64-unknown-linux-musl", + "x86_64-apple-darwin", + "aarch64-apple-darwin", + "x86_64-pc-windows-msvc", + "aarch64-pc-windows-msvc", +) + +RG_TARGET_PLATFORM_PAIRS: list[tuple[str, str]] = [ + ("x86_64-unknown-linux-musl", "linux-x86_64"), + ("aarch64-unknown-linux-musl", "linux-aarch64"), + ("x86_64-apple-darwin", "macos-x86_64"), + ("aarch64-apple-darwin", "macos-aarch64"), + ("x86_64-pc-windows-msvc", "windows-x86_64"), + ("aarch64-pc-windows-msvc", "windows-aarch64"), +] +RG_TARGET_TO_PLATFORM = {target: platform for target, platform in RG_TARGET_PLATFORM_PAIRS} +DEFAULT_RG_TARGETS = [target for target, _ in RG_TARGET_PLATFORM_PAIRS] + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Install native Codex binaries.") + parser.add_argument( + "--workflow-url", + help=( + "GitHub Actions workflow URL that produced the artifacts. Defaults to a " + "known good run when omitted." + ), + ) + parser.add_argument( + "root", + nargs="?", + type=Path, + help=( + "Directory containing package.json for the staged package. If omitted, the " + "repository checkout is used." + ), + ) + return parser.parse_args() + + +def main() -> int: + args = parse_args() + + codex_cli_root = (args.root or CODEX_CLI_ROOT).resolve() + vendor_dir = codex_cli_root / VENDOR_DIR_NAME + vendor_dir.mkdir(parents=True, exist_ok=True) + + workflow_url = (args.workflow_url or DEFAULT_WORKFLOW_URL).strip() + if not workflow_url: + workflow_url = DEFAULT_WORKFLOW_URL + + workflow_id = workflow_url.rstrip("/").split("/")[-1] + + with tempfile.TemporaryDirectory(prefix="codex-native-artifacts-") as artifacts_dir_str: + artifacts_dir = Path(artifacts_dir_str) + _download_artifacts(workflow_id, artifacts_dir) + install_codex_binaries(artifacts_dir, vendor_dir, CODEX_TARGETS) + + fetch_rg(vendor_dir, DEFAULT_RG_TARGETS, manifest_path=RG_MANIFEST) + + print(f"Installed native dependencies into {vendor_dir}") + return 0 + + +def fetch_rg( + vendor_dir: Path, + targets: Sequence[str] | None = None, + *, + manifest_path: Path, +) -> list[Path]: + """Download ripgrep binaries described by the DotSlash manifest.""" + + if targets is None: + targets = DEFAULT_RG_TARGETS + + if not manifest_path.exists(): + raise FileNotFoundError(f"DotSlash manifest not found: {manifest_path}") + + manifest = _load_manifest(manifest_path) + platforms = manifest.get("platforms", {}) + + vendor_dir.mkdir(parents=True, exist_ok=True) + + targets = list(targets) + if not targets: + return [] + + task_configs: list[tuple[str, str, dict]] = [] + for target in targets: + platform_key = RG_TARGET_TO_PLATFORM.get(target) + if platform_key is None: + raise ValueError(f"Unsupported ripgrep target '{target}'.") + + platform_info = platforms.get(platform_key) + if platform_info is None: + raise RuntimeError(f"Platform '{platform_key}' not found in manifest {manifest_path}.") + + task_configs.append((target, platform_key, platform_info)) + + results: dict[str, Path] = {} + max_workers = min(len(task_configs), max(1, (os.cpu_count() or 1))) + + with ThreadPoolExecutor(max_workers=max_workers) as executor: + future_map = { + executor.submit( + _fetch_single_rg, + vendor_dir, + target, + platform_key, + platform_info, + manifest_path, + ): target + for target, platform_key, platform_info in task_configs + } + + for future in as_completed(future_map): + target = future_map[future] + results[target] = future.result() + + return [results[target] for target in targets] + + +def _download_artifacts(workflow_id: str, dest_dir: Path) -> None: + cmd = [ + "gh", + "run", + "download", + "--dir", + str(dest_dir), + "--repo", + "openai/codex", + workflow_id, + ] + subprocess.check_call(cmd) + + +def install_codex_binaries( + artifacts_dir: Path, vendor_dir: Path, targets: Iterable[str] +) -> list[Path]: + targets = list(targets) + if not targets: + return [] + + results: dict[str, Path] = {} + max_workers = min(len(targets), max(1, (os.cpu_count() or 1))) + + with ThreadPoolExecutor(max_workers=max_workers) as executor: + future_map = { + executor.submit(_install_single_codex_binary, artifacts_dir, vendor_dir, target): target + for target in targets + } + + for future in as_completed(future_map): + target = future_map[future] + results[target] = future.result() + + return [results[target] for target in targets] + + +def _install_single_codex_binary(artifacts_dir: Path, vendor_dir: Path, target: str) -> Path: + artifact_subdir = artifacts_dir / target + archive_name = _archive_name_for_target(target) + archive_path = artifact_subdir / archive_name + if not archive_path.exists(): + raise FileNotFoundError(f"Expected artifact not found: {archive_path}") + + dest_dir = vendor_dir / target / "codex" + dest_dir.mkdir(parents=True, exist_ok=True) + + binary_name = "codex.exe" if "windows" in target else "codex" + dest = dest_dir / binary_name + dest.unlink(missing_ok=True) + extract_archive(archive_path, "zst", None, dest) + if "windows" not in target: + dest.chmod(0o755) + return dest + + +def _archive_name_for_target(target: str) -> str: + if "windows" in target: + return f"codex-{target}.exe.zst" + return f"codex-{target}.zst" + + +def _fetch_single_rg( + vendor_dir: Path, + target: str, + platform_key: str, + platform_info: dict, + manifest_path: Path, +) -> Path: + providers = platform_info.get("providers", []) + if not providers: + raise RuntimeError(f"No providers listed for platform '{platform_key}' in {manifest_path}.") + + url = providers[0]["url"] + archive_format = platform_info.get("format", "zst") + archive_member = platform_info.get("path") + + dest_dir = vendor_dir / target / "path" + dest_dir.mkdir(parents=True, exist_ok=True) + + is_windows = platform_key.startswith("win") + binary_name = "rg.exe" if is_windows else "rg" + dest = dest_dir / binary_name + + with tempfile.TemporaryDirectory() as tmp_dir_str: + tmp_dir = Path(tmp_dir_str) + archive_filename = os.path.basename(urlparse(url).path) + download_path = tmp_dir / archive_filename + _download_file(url, download_path) + + dest.unlink(missing_ok=True) + extract_archive(download_path, archive_format, archive_member, dest) + + if not is_windows: + dest.chmod(0o755) + + return dest + + +def _download_file(url: str, dest: Path) -> None: + dest.parent.mkdir(parents=True, exist_ok=True) + with urlopen(url) as response, open(dest, "wb") as out: + shutil.copyfileobj(response, out) + + +def extract_archive( + archive_path: Path, + archive_format: str, + archive_member: str | None, + dest: Path, +) -> None: + dest.parent.mkdir(parents=True, exist_ok=True) + + if archive_format == "zst": + output_path = archive_path.parent / dest.name + subprocess.check_call( + ["zstd", "-f", "-d", str(archive_path), "-o", str(output_path)] + ) + shutil.move(str(output_path), dest) + return + + if archive_format == "tar.gz": + if not archive_member: + raise RuntimeError("Missing 'path' for tar.gz archive in DotSlash manifest.") + with tarfile.open(archive_path, "r:gz") as tar: + try: + member = tar.getmember(archive_member) + except KeyError as exc: + raise RuntimeError( + f"Entry '{archive_member}' not found in archive {archive_path}." + ) from exc + tar.extract(member, path=archive_path.parent, filter="data") + extracted = archive_path.parent / archive_member + shutil.move(str(extracted), dest) + return + + if archive_format == "zip": + if not archive_member: + raise RuntimeError("Missing 'path' for zip archive in DotSlash manifest.") + with zipfile.ZipFile(archive_path) as archive: + try: + with archive.open(archive_member) as src, open(dest, "wb") as out: + shutil.copyfileobj(src, out) + except KeyError as exc: + raise RuntimeError( + f"Entry '{archive_member}' not found in archive {archive_path}." + ) from exc + return + + raise RuntimeError(f"Unsupported archive format '{archive_format}'.") + + +def _load_manifest(manifest_path: Path) -> dict: + cmd = ["dotslash", "--", "parse", str(manifest_path)] + stdout = subprocess.check_output(cmd, text=True) + try: + manifest = json.loads(stdout) + except json.JSONDecodeError as exc: + raise RuntimeError(f"Invalid DotSlash manifest output from {manifest_path}.") from exc + + if not isinstance(manifest, dict): + raise RuntimeError( + f"Unexpected DotSlash manifest structure for {manifest_path}: {type(manifest)!r}" + ) + + return manifest + + +if __name__ == "__main__": + import sys + + sys.exit(main()) diff --git a/codex-cli/scripts/install_native_deps.sh b/codex-cli/scripts/install_native_deps.sh deleted file mode 100755 index b9fda2543e..0000000000 --- a/codex-cli/scripts/install_native_deps.sh +++ /dev/null @@ -1,94 +0,0 @@ -#!/usr/bin/env bash - -# Install native runtime dependencies for codex-cli. -# -# Usage -# install_native_deps.sh [--workflow-url URL] [CODEX_CLI_ROOT] -# -# The optional RELEASE_ROOT is the path that contains package.json. Omitting -# it installs the binaries into the repository's own bin/ folder to support -# local development. - -set -euo pipefail - -# ------------------ -# Parse arguments -# ------------------ - -CODEX_CLI_ROOT="" - -# Until we start publishing stable GitHub releases, we have to grab the binaries -# from the GitHub Action that created them. Update the URL below to point to the -# appropriate workflow run: -WORKFLOW_URL="https://github.com/openai/codex/actions/runs/17417194663" # rust-v0.28.0 - -while [[ $# -gt 0 ]]; do - case "$1" in - --workflow-url) - shift || { echo "--workflow-url requires an argument"; exit 1; } - if [ -n "$1" ]; then - WORKFLOW_URL="$1" - fi - ;; - *) - if [[ -z "$CODEX_CLI_ROOT" ]]; then - CODEX_CLI_ROOT="$1" - else - echo "Unexpected argument: $1" >&2 - exit 1 - fi - ;; - esac - shift -done - -# ---------------------------------------------------------------------------- -# Determine where the binaries should be installed. -# ---------------------------------------------------------------------------- - -if [ -n "$CODEX_CLI_ROOT" ]; then - # The caller supplied a release root directory. - BIN_DIR="$CODEX_CLI_ROOT/bin" -else - # No argument; fall back to the repo’s own bin directory. - # Resolve the path of this script, then walk up to the repo root. - SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - CODEX_CLI_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" - BIN_DIR="$CODEX_CLI_ROOT/bin" -fi - -# Make sure the destination directory exists. -mkdir -p "$BIN_DIR" - -# ---------------------------------------------------------------------------- -# Download and decompress the artifacts from the GitHub Actions workflow. -# ---------------------------------------------------------------------------- - -WORKFLOW_ID="${WORKFLOW_URL##*/}" - -ARTIFACTS_DIR="$(mktemp -d)" -trap 'rm -rf "$ARTIFACTS_DIR"' EXIT - -# NB: The GitHub CLI `gh` must be installed and authenticated. -gh run download --dir "$ARTIFACTS_DIR" --repo openai/codex "$WORKFLOW_ID" - -# x64 Linux -zstd -d "$ARTIFACTS_DIR/x86_64-unknown-linux-musl/codex-x86_64-unknown-linux-musl.zst" \ - -o "$BIN_DIR/codex-x86_64-unknown-linux-musl" -# ARM64 Linux -zstd -d "$ARTIFACTS_DIR/aarch64-unknown-linux-musl/codex-aarch64-unknown-linux-musl.zst" \ - -o "$BIN_DIR/codex-aarch64-unknown-linux-musl" -# x64 macOS -zstd -d "$ARTIFACTS_DIR/x86_64-apple-darwin/codex-x86_64-apple-darwin.zst" \ - -o "$BIN_DIR/codex-x86_64-apple-darwin" -# ARM64 macOS -zstd -d "$ARTIFACTS_DIR/aarch64-apple-darwin/codex-aarch64-apple-darwin.zst" \ - -o "$BIN_DIR/codex-aarch64-apple-darwin" -# x64 Windows -zstd -d "$ARTIFACTS_DIR/x86_64-pc-windows-msvc/codex-x86_64-pc-windows-msvc.exe.zst" \ - -o "$BIN_DIR/codex-x86_64-pc-windows-msvc.exe" -# ARM64 Windows -zstd -d "$ARTIFACTS_DIR/aarch64-pc-windows-msvc/codex-aarch64-pc-windows-msvc.exe.zst" \ - -o "$BIN_DIR/codex-aarch64-pc-windows-msvc.exe" - -echo "Installed native dependencies into $BIN_DIR" diff --git a/codex-cli/scripts/stage_release.sh b/codex-cli/scripts/stage_release.sh deleted file mode 100755 index 96236fc53c..0000000000 --- a/codex-cli/scripts/stage_release.sh +++ /dev/null @@ -1,120 +0,0 @@ -#!/usr/bin/env bash -# ----------------------------------------------------------------------------- -# stage_release.sh -# ----------------------------------------------------------------------------- -# Stages an npm release for @openai/codex. -# -# Usage: -# -# --tmp : Use instead of a freshly created temp directory. -# -h|--help : Print usage. -# -# ----------------------------------------------------------------------------- - -set -euo pipefail - -# Helper - usage / flag parsing - -usage() { - cat <&2 - usage 1 - ;; - *) - echo "Unexpected extra argument: $1" >&2 - usage 1 - ;; - esac - shift -done - -# Fallback when the caller did not specify a directory. -# If no directory was specified create a fresh temporary one. -if [[ -z "$TMPDIR" ]]; then - TMPDIR="$(mktemp -d)" -fi - -# Ensure the directory exists, then resolve to an absolute path. -mkdir -p "$TMPDIR" -TMPDIR="$(cd "$TMPDIR" && pwd)" - -# Main build logic - -echo "Staging release in $TMPDIR" - -# The script lives in codex-cli/scripts/ - change into codex-cli root so that -# relative paths keep working. -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -CODEX_CLI_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" - -pushd "$CODEX_CLI_ROOT" >/dev/null - -# 1. Build the JS artifacts --------------------------------------------------- - -# Paths inside the staged package -mkdir -p "$TMPDIR/bin" - -cp -r bin/codex.js "$TMPDIR/bin/codex.js" -cp ../README.md "$TMPDIR" || true # README is one level up - ignore if missing - -# Modify package.json - bump version and optionally add the native directory to -# the files array so that the binaries are published to npm. - -jq --arg version "$VERSION" \ - '.version = $version' \ - package.json > "$TMPDIR/package.json" - -# 2. Native runtime deps (sandbox plus optional Rust binaries) - -./scripts/install_native_deps.sh --workflow-url "$WORKFLOW_URL" "$TMPDIR" - -popd >/dev/null - -echo "Staged version $VERSION for release in $TMPDIR" - -echo "Verify the CLI:" -echo " node ${TMPDIR}/bin/codex.js --version" -echo " node ${TMPDIR}/bin/codex.js --help" - -# Print final hint for convenience -echo "Next: cd \"$TMPDIR\" && npm publish" diff --git a/codex-cli/scripts/stage_rust_release.py b/codex-cli/scripts/stage_rust_release.py deleted file mode 100755 index 9a554b77d0..0000000000 --- a/codex-cli/scripts/stage_rust_release.py +++ /dev/null @@ -1,70 +0,0 @@ -#!/usr/bin/env python3 - -import json -import subprocess -import sys -import argparse -from pathlib import Path - - -def main() -> int: - parser = argparse.ArgumentParser( - description="""Stage a release for the npm module. - -Run this after the GitHub Release has been created and use -`--release-version` to specify the version to release. - -Optionally pass `--tmp` to control the temporary staging directory that will be -forwarded to stage_release.sh. -""" - ) - parser.add_argument( - "--release-version", required=True, help="Version to release, e.g., 0.3.0" - ) - parser.add_argument( - "--tmp", - help="Optional path to stage the npm package; forwarded to stage_release.sh", - ) - args = parser.parse_args() - version = args.release_version - - gh_run = subprocess.run( - [ - "gh", - "run", - "list", - "--branch", - f"rust-v{version}", - "--json", - "workflowName,url,headSha", - "--jq", - 'first(.[] | select(.workflowName == "rust-release"))', - ], - stdout=subprocess.PIPE, - check=True, - ) - gh_run.check_returncode() - workflow = json.loads(gh_run.stdout) - sha = workflow["headSha"] - - print(f"should `git checkout {sha}`") - - current_dir = Path(__file__).parent.resolve() - cmd = [ - str(current_dir / "stage_release.sh"), - "--version", - version, - "--workflow-url", - workflow["url"], - ] - if args.tmp: - cmd.extend(["--tmp", args.tmp]) - - stage_release = subprocess.run(cmd) - stage_release.check_returncode() - - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 71681aa57f..5b74ec3263 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -78,12 +78,6 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" -[[package]] -name = "android-tzdata" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" - [[package]] name = "android_system_properties" version = "0.1.5" @@ -316,6 +310,17 @@ dependencies = [ "syn 2.0.104", ] +[[package]] +name = "async-trait" +version = "0.1.89" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "atomic-waker" version = "1.1.2" @@ -485,17 +490,16 @@ checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" [[package]] name = "chrono" -version = "0.4.41" +version = "0.4.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c469d952047f47f91b68d1cba3f10d63c11d73e4636f24f08daf0278abf01c4d" +checksum = "145052bdd345b87320e369255277e3fb5152762ad123a901ef5c262dd38fe8d2" dependencies = [ - "android-tzdata", "iana-time-zone", "js-sys", "num-traits", "serde", "wasm-bindgen", - "windows-link", + "windows-link 0.2.0", ] [[package]] @@ -584,7 +588,6 @@ version = "0.0.0" dependencies = [ "anyhow", "assert_cmd", - "once_cell", "pretty_assertions", "similar", "tempfile", @@ -778,6 +781,16 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-git-tooling" +version = "0.0.0" +dependencies = [ + "pretty_assertions", + "tempfile", + "thiserror 2.0.16", + "walkdir", +] + [[package]] name = "codex-linux-sandbox" version = "0.0.0" @@ -795,6 +808,7 @@ dependencies = [ name = "codex-login" version = "0.0.0" dependencies = [ + "anyhow", "base64", "chrono", "codex-core", @@ -875,6 +889,7 @@ dependencies = [ name = "codex-protocol" version = "0.0.0" dependencies = [ + "anyhow", "base64", "icu_decimal", "icu_locale_core", @@ -919,6 +934,7 @@ dependencies = [ "codex-common", "codex-core", "codex-file-search", + "codex-git-tooling", "codex-login", "codex-ollama", "codex-protocol", @@ -932,7 +948,6 @@ dependencies = [ "lazy_static", "libc", "mcp-types", - "once_cell", "path-clean", "pathdiff", "pretty_assertions", @@ -954,11 +969,21 @@ dependencies = [ "tracing-appender", "tracing-subscriber", "unicode-segmentation", - "unicode-width 0.1.14", + "unicode-width 0.2.1", "url", "vt100", ] +[[package]] +name = "codex-utils-readiness" +version = "0.0.0" +dependencies = [ + "async-trait", + "thiserror 2.0.16", + "time", + "tokio", +] + [[package]] name = "color-eyre" version = "0.6.5" @@ -1076,6 +1101,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" name = "core_test_support" version = "0.0.0" dependencies = [ + "anyhow", "codex-core", "serde_json", "tempfile", @@ -1253,12 +1279,12 @@ dependencies = [ [[package]] name = "deranged" -version = "0.4.0" +version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c9e6a11ca8224451684bc0d7d5a7adbf8f2fd6887261a1cfc3c0432f9d4068e" +checksum = "a41953f86f8a05768a6cda24def994fd2f424b04ec5c719cf89989779f199071" dependencies = [ "powerfmt", - "serde", + "serde_core", ] [[package]] @@ -2658,9 +2684,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.27" +version = "0.4.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13dc2df351e3202783a1fe0d44375f7295ffb4049267b0f3018346dc122a1d94" +checksum = "34080505efa8e45a4b816c349525ebe327ceaa8559756f0356cba97ef3bf7432" [[package]] name = "logos" @@ -3901,9 +3927,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.224" +version = "1.0.226" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6aaeb1e94f53b16384af593c71e20b095e958dab1d26939c1b70645c5cfbcc0b" +checksum = "0dca6411025b24b60bfa7ec1fe1f8e710ac09782dca409ee8237ba74b51295fd" dependencies = [ "serde_core", "serde_derive", @@ -3911,18 +3937,18 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.224" +version = "1.0.226" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32f39390fa6346e24defbcdd3d9544ba8a19985d0af74df8501fbfe9a64341ab" +checksum = "ba2ba63999edb9dac981fb34b3e5c0d111a69b0924e253ed29d83f7c99e966a4" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.224" +version = "1.0.226" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87ff78ab5e8561c9a675bfc1785cb07ae721f0ee53329a595cefd8c04c2ac4e0" +checksum = "8db53ae22f34573731bafa1db20f04027b2d25e02d8205921b569171699cdb33" dependencies = [ "proc-macro2", "quote", @@ -4425,15 +4451,15 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.20.0" +version = "3.23.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8a64e3985349f2441a1a9ef0b853f869006c3855f2cda6862a94d26ebb9d6a1" +checksum = "2d31c77bdf42a745371d260a26ca7163f1e0924b64afa0b688e61b5a9fa02f16" dependencies = [ "fastrand", "getrandom 0.3.3", "once_cell", "rustix 1.0.8", - "windows-sys 0.59.0", + "windows-sys 0.60.2", ] [[package]] @@ -4557,9 +4583,9 @@ dependencies = [ [[package]] name = "time" -version = "0.3.41" +version = "0.3.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a7619e19bc266e0f9c5e6686659d394bc57973859340060a69221e57dbc0c40" +checksum = "91e7d9e3bb61134e77bde20dd4825b97c010155709965fedf0f49bb138e52a9d" dependencies = [ "deranged", "itoa", @@ -4574,15 +4600,15 @@ dependencies = [ [[package]] name = "time-core" -version = "0.1.4" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9e9a38711f559d9e3ce1cdb06dd7c5b8ea546bc90052da6d06bb76da74bb07c" +checksum = "40868e7c1d2f0b8d73e4a8c7f0ff63af4f6d19be117e90bd73eb1d62cf831c6b" [[package]] name = "time-macros" -version = "0.2.22" +version = "0.2.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3526739392ec93fd8b359c8e98514cb3e8e021beb4e5f597b00a0221f8ed8a49" +checksum = "30cfb0125f12d9c277f35663a0a33f8c30190f4e4574868a330595412d34ebf3" dependencies = [ "num-conv", "time-core", @@ -5310,7 +5336,7 @@ checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" dependencies = [ "windows-implement", "windows-interface", - "windows-link", + "windows-link 0.1.3", "windows-result", "windows-strings", ] @@ -5343,13 +5369,19 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" +[[package]] +name = "windows-link" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45e46c0661abb7180e7b9c281db115305d49ca1709ab8242adf09666d2173c65" + [[package]] name = "windows-registry" version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b8a9ed28765efc97bbc954883f4e6796c33a06546ebafacbabee9696967499e" dependencies = [ - "windows-link", + "windows-link 0.1.3", "windows-result", "windows-strings", ] @@ -5360,7 +5392,7 @@ version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" dependencies = [ - "windows-link", + "windows-link 0.1.3", ] [[package]] @@ -5369,7 +5401,7 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" dependencies = [ - "windows-link", + "windows-link 0.1.3", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 7e0d562074..57e606cbb9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -9,6 +9,7 @@ members = [ "exec", "execpolicy", "file-search", + "git-tooling", "linux-sandbox", "login", "mcp-client", @@ -18,6 +19,7 @@ members = [ "protocol", "protocol-ts", "tui", + "utils/readiness", ] resolver = "2" @@ -39,6 +41,7 @@ codex-common = { path = "common" } codex-core = { path = "core" } codex-exec = { path = "exec" } codex-file-search = { path = "file-search" } +codex-git-tooling = { path = "git-tooling" } codex-linux-sandbox = { path = "linux-sandbox" } codex-login = { path = "login" } codex-mcp-client = { path = "mcp-client" } @@ -47,6 +50,7 @@ codex-ollama = { path = "ollama" } codex-protocol = { path = "protocol" } codex-protocol-ts = { path = "protocol-ts" } codex-tui = { path = "tui" } +codex-utils-readiness = { path = "utils/readiness" } core_test_support = { path = "core/tests/common" } mcp-types = { path = "mcp-types" } mcp_test_support = { path = "mcp-server/tests/common" } @@ -60,9 +64,10 @@ askama = "0.12" assert_cmd = "2" async-channel = "2.3.1" async-stream = "0.3.6" +async-trait = "0.1.89" base64 = "0.22.1" bytes = "1.10.1" -chrono = "0.4.40" +chrono = "0.4.42" clap = "4" clap_complete = "4" color-eyre = "0.6.3" @@ -89,7 +94,6 @@ maplit = "1.0.2" mime_guess = "2.0.5" multimap = "0.10.0" nucleo-matcher = "0.3.1" -once_cell = "1" openssl-sys = "*" os_info = "3.12.0" owo-colors = "4.2.0" @@ -118,7 +122,7 @@ strum = "0.27.2" strum_macros = "0.27.2" supports-color = "3.0.2" sys-locale = "0.3.2" -tempfile = "3.13.0" +tempfile = "3.23.0" textwrap = "0.16.2" thiserror = "2.0.16" time = "0.3" @@ -136,7 +140,7 @@ tree-sitter = "0.25.9" tree-sitter-bash = "0.25.0" ts-rs = "11" unicode-segmentation = "1.12.0" -unicode-width = "0.1" +unicode-width = "0.2" url = "2" urlencoding = "2.1" uuid = "1" @@ -188,7 +192,7 @@ unwrap_used = "deny" # cargo-shear cannot see the platform-specific openssl-sys usage, so we # silence the false positive here instead of deleting a real dependency. [workspace.metadata.cargo-shear] -ignored = ["openssl-sys"] +ignored = ["openssl-sys", "codex-utils-readiness"] [profile.release] lto = "fat" diff --git a/codex-rs/apply-patch/Cargo.toml b/codex-rs/apply-patch/Cargo.toml index d37404c15b..9445ae08b7 100644 --- a/codex-rs/apply-patch/Cargo.toml +++ b/codex-rs/apply-patch/Cargo.toml @@ -20,7 +20,6 @@ similar = { workspace = true } thiserror = { workspace = true } tree-sitter = { workspace = true } tree-sitter-bash = { workspace = true } -once_cell = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 189c1a07d0..3737c6ea6d 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -6,10 +6,10 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; use std::str::Utf8Error; +use std::sync::LazyLock; use anyhow::Context; use anyhow::Result; -use once_cell::sync::Lazy; pub use parser::Hunk; pub use parser::ParseError; use parser::ParseError::*; @@ -351,7 +351,7 @@ fn extract_apply_patch_from_bash( // also run an arbitrary query against the AST. This is useful for understanding // how tree-sitter parses the script and whether the query syntax is correct. Be sure // to test both positive and negative cases. - static APPLY_PATCH_QUERY: Lazy = Lazy::new(|| { + static APPLY_PATCH_QUERY: LazyLock = LazyLock::new(|| { let language = BASH.into(); #[expect(clippy::expect_used)] Query::new( diff --git a/codex-rs/code b/codex-rs/code new file mode 100644 index 0000000000..e69de29bb2 diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 0afc06e99c..1ebbe5d738 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -1,13 +1,12 @@ use crate::codex::Session; use crate::codex::TurnContext; +use crate::function_tool::FunctionCallError; use crate::protocol::FileChange; use crate::protocol::ReviewDecision; use crate::safety::SafetyCheck; use crate::safety::assess_patch_safety; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; -use codex_protocol::models::FunctionCallOutputPayload; -use codex_protocol::models::ResponseInputItem; use std::collections::HashMap; use std::path::PathBuf; @@ -17,7 +16,7 @@ pub(crate) enum InternalApplyPatchInvocation { /// The `apply_patch` call was handled programmatically, without any sort /// of sandbox, because the user explicitly approved it. This is the /// result to use with the `shell` function call that contained `apply_patch`. - Output(ResponseInputItem), + Output(Result), /// The `apply_patch` call was approved, either automatically because it /// appears that it should be allowed based on the user's sandbox policy @@ -33,12 +32,6 @@ pub(crate) struct ApplyPatchExec { pub(crate) user_explicitly_approved_this_action: bool, } -impl From for InternalApplyPatchInvocation { - fn from(item: ResponseInputItem) -> Self { - InternalApplyPatchInvocation::Output(item) - } -} - pub(crate) async fn apply_patch( sess: &Session, turn_context: &TurnContext, @@ -77,25 +70,15 @@ pub(crate) async fn apply_patch( }) } ReviewDecision::Denied | ReviewDecision::Abort => { - ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_owned(), - output: FunctionCallOutputPayload { - content: "patch rejected by user".to_string(), - success: Some(false), - }, - } - .into() + InternalApplyPatchInvocation::Output(Err(FunctionCallError::RespondToModel( + "patch rejected by user".to_string(), + ))) } } } - SafetyCheck::Reject { reason } => ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_owned(), - output: FunctionCallOutputPayload { - content: format!("patch rejected: {reason}"), - success: Some(false), - }, - } - .into(), + SafetyCheck::Reject { reason } => InternalApplyPatchInvocation::Output(Err( + FunctionCallError::RespondToModel(format!("patch rejected: {reason}")), + )), } } diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index a32b59b552..eddc786484 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -35,6 +35,12 @@ pub(crate) async fn stream_chat_completions( client: &reqwest::Client, provider: &ModelProviderInfo, ) -> Result { + if prompt.output_schema.is_some() { + return Err(CodexErr::UnsupportedOperation( + "output_schema is not supported for Chat Completions API".to_string(), + )); + } + // Build messages array let mut messages = Vec::::new(); diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 72ca770abd..39a6aff8da 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -42,7 +42,8 @@ use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::WireApi; use crate::openai_model_info::get_model_info; use crate::openai_tools::create_tools_json_for_responses_api; -use crate::protocol::RateLimitSnapshotEvent; +use crate::protocol::RateLimitSnapshot; +use crate::protocol::RateLimitWindow; use crate::protocol::TokenUsage; use crate::token_data::PlanType; use crate::util::backoff; @@ -183,19 +184,23 @@ impl ModelClient { let input_with_instructions = prompt.get_formatted_input(); - // Only include `text.verbosity` for GPT-5 family models - let text = if self.config.model_family.family == "gpt-5" { - create_text_param_for_request(self.config.model_verbosity) - } else { - if self.config.model_verbosity.is_some() { - warn!( - "model_verbosity is set but ignored for non-gpt-5 model family: {}", - self.config.model_family.family - ); + let verbosity = match &self.config.model_family.family { + family if family == "gpt-5" => self.config.model_verbosity, + _ => { + if self.config.model_verbosity.is_some() { + warn!( + "model_verbosity is set but ignored for non-gpt-5 model family: {}", + self.config.model_family.family + ); + } + + None } - None }; + // Only include `text.verbosity` for GPT-5 family models + let text = create_text_param_for_request(verbosity, &prompt.output_schema); + // In general, we want to explicitly send `store: false` when using the Responses API, // but in practice, the Azure Responses API rejects `store: false`: // @@ -330,6 +335,7 @@ impl ModelClient { } if status == StatusCode::TOO_MANY_REQUESTS { + let rate_limit_snapshot = parse_rate_limit_snapshot(res.headers()); let body = res.json::().await.ok(); if let Some(ErrorResponse { error }) = body { if error.r#type.as_deref() == Some("usage_limit_reached") { @@ -343,6 +349,7 @@ impl ModelClient { return Err(CodexErr::UsageLimitReached(UsageLimitReachedError { plan_type, resets_in_seconds, + rate_limits: rate_limit_snapshot, })); } else if error.r#type.as_deref() == Some("usage_not_included") { return Err(CodexErr::UsageNotIncluded); @@ -412,9 +419,6 @@ struct SseEvent { delta: Option, } -#[derive(Debug, Deserialize)] -struct ResponseCreated {} - #[derive(Debug, Deserialize)] struct ResponseCompleted { id: String, @@ -485,20 +489,40 @@ fn attach_item_ids(payload_json: &mut Value, original_items: &[ResponseItem]) { } } -fn parse_rate_limit_snapshot(headers: &HeaderMap) -> Option { - let primary_used_percent = parse_header_f64(headers, "x-codex-primary-used-percent")?; - let secondary_used_percent = parse_header_f64(headers, "x-codex-secondary-used-percent")?; - let primary_to_secondary_ratio_percent = - parse_header_f64(headers, "x-codex-primary-over-secondary-limit-percent")?; - let primary_window_minutes = parse_header_u64(headers, "x-codex-primary-window-minutes")?; - let secondary_window_minutes = parse_header_u64(headers, "x-codex-secondary-window-minutes")?; +fn parse_rate_limit_snapshot(headers: &HeaderMap) -> Option { + let primary = parse_rate_limit_window( + headers, + "x-codex-primary-used-percent", + "x-codex-primary-window-minutes", + "x-codex-primary-reset-after-seconds", + ); - Some(RateLimitSnapshotEvent { - primary_used_percent, - secondary_used_percent, - primary_to_secondary_ratio_percent, - primary_window_minutes, - secondary_window_minutes, + let secondary = parse_rate_limit_window( + headers, + "x-codex-secondary-used-percent", + "x-codex-secondary-window-minutes", + "x-codex-secondary-reset-after-seconds", + ); + + if primary.is_none() && secondary.is_none() { + return None; + } + + Some(RateLimitSnapshot { primary, secondary }) +} + +fn parse_rate_limit_window( + headers: &HeaderMap, + used_percent_header: &str, + window_minutes_header: &str, + resets_header: &str, +) -> Option { + let used_percent = parse_header_f64(headers, used_percent_header)?; + + Some(RateLimitWindow { + used_percent, + window_minutes: parse_header_u64(headers, window_minutes_header), + resets_in_seconds: parse_header_u64(headers, resets_header), }) } diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 15bfb5d400..b695581deb 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -1,7 +1,7 @@ use crate::error::Result; use crate::model_family::ModelFamily; use crate::openai_tools::OpenAiTool; -use crate::protocol::RateLimitSnapshotEvent; +use crate::protocol::RateLimitSnapshot; use crate::protocol::TokenUsage; use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; @@ -10,6 +10,7 @@ use codex_protocol::config_types::Verbosity as VerbosityConfig; use codex_protocol::models::ResponseItem; use futures::Stream; use serde::Serialize; +use serde_json::Value; use std::borrow::Cow; use std::ops::Deref; use std::pin::Pin; @@ -32,6 +33,9 @@ pub struct Prompt { /// Optional override for the built-in BASE_INSTRUCTIONS. pub base_instructions_override: Option, + + /// Optional the output schema for the model's response. + pub output_schema: Option, } impl Prompt { @@ -79,7 +83,7 @@ pub enum ResponseEvent { WebSearchCallBegin { call_id: String, }, - RateLimits(RateLimitSnapshotEvent), + RateLimits(RateLimitSnapshot), } #[derive(Debug, Serialize)] @@ -90,14 +94,31 @@ pub(crate) struct Reasoning { pub(crate) summary: Option, } +#[derive(Debug, Serialize, Default, Clone)] +#[serde(rename_all = "snake_case")] +pub(crate) enum TextFormatType { + #[default] + JsonSchema, +} + +#[derive(Debug, Serialize, Default, Clone)] +pub(crate) struct TextFormat { + pub(crate) r#type: TextFormatType, + pub(crate) strict: bool, + pub(crate) schema: Value, + pub(crate) name: String, +} + /// Controls under the `text` field in the Responses API for GPT-5. -#[derive(Debug, Serialize, Default, Clone, Copy)] +#[derive(Debug, Serialize, Default, Clone)] pub(crate) struct TextControls { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) verbosity: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) format: Option, } -#[derive(Debug, Serialize, Default, Clone, Copy)] +#[derive(Debug, Serialize, Default, Clone)] #[serde(rename_all = "lowercase")] pub(crate) enum OpenAiVerbosity { Low, @@ -156,9 +177,20 @@ pub(crate) fn create_reasoning_param_for_request( pub(crate) fn create_text_param_for_request( verbosity: Option, + output_schema: &Option, ) -> Option { - verbosity.map(|v| TextControls { - verbosity: Some(v.into()), + if verbosity.is_none() && output_schema.is_none() { + return None; + } + + Some(TextControls { + verbosity: verbosity.map(std::convert::Into::into), + format: output_schema.as_ref().map(|schema| TextFormat { + r#type: TextFormatType::JsonSchema, + strict: true, + schema: schema.clone(), + name: "codex_output_schema".to_string(), + }), }) } @@ -255,6 +287,7 @@ mod tests { prompt_cache_key: None, text: Some(TextControls { verbosity: Some(OpenAiVerbosity::Low), + format: None, }), }; @@ -267,6 +300,52 @@ mod tests { ); } + #[test] + fn serializes_text_schema_with_strict_format() { + let input: Vec = vec![]; + let tools: Vec = vec![]; + let schema = serde_json::json!({ + "type": "object", + "properties": { + "answer": {"type": "string"} + }, + "required": ["answer"], + }); + let text_controls = + create_text_param_for_request(None, &Some(schema.clone())).expect("text controls"); + + let req = ResponsesApiRequest { + model: "gpt-5", + instructions: "i", + input: &input, + tools: &tools, + tool_choice: "auto", + parallel_tool_calls: false, + reasoning: None, + store: false, + stream: true, + include: vec![], + prompt_cache_key: None, + text: Some(text_controls), + }; + + let v = serde_json::to_value(&req).expect("json"); + let text = v.get("text").expect("text field"); + assert!(text.get("verbosity").is_none()); + let format = text.get("format").expect("format field"); + + assert_eq!( + format.get("name"), + Some(&serde_json::Value::String("codex_output_schema".into())) + ); + assert_eq!( + format.get("type"), + Some(&serde_json::Value::String("json_schema".into())) + ); + assert_eq!(format.get("strict"), Some(&serde_json::Value::Bool(true))); + assert_eq!(format.get("schema"), Some(&schema)); + } + #[test] fn omits_text_when_not_set() { let input: Vec = vec![]; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 08b28bdef5..84a04d7533 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -10,7 +10,9 @@ use std::time::Duration; use crate::AuthManager; use crate::client_common::REVIEW_PROMPT; use crate::event_mapping::map_response_item_to_event_messages; +use crate::function_tool::FunctionCallError; use crate::review_format::format_review_findings_block; +use crate::user_notification::UserNotifier; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; @@ -30,6 +32,7 @@ use mcp_types::CallToolResult; use serde::Deserialize; use serde::Serialize; use serde_json; +use serde_json::Value; use tokio::sync::Mutex; use tokio::sync::oneshot; use tokio::task::AbortHandle; @@ -98,7 +101,7 @@ use crate::protocol::ListCustomPromptsResponseEvent; use crate::protocol::Op; use crate::protocol::PatchApplyBeginEvent; use crate::protocol::PatchApplyEndEvent; -use crate::protocol::RateLimitSnapshotEvent; +use crate::protocol::RateLimitSnapshot; use crate::protocol::ReviewDecision; use crate::protocol::ReviewOutputEvent; use crate::protocol::SandboxPolicy; @@ -186,7 +189,7 @@ impl Codex { base_instructions: config.base_instructions.clone(), approval_policy: config.approval_policy, sandbox_policy: config.sandbox_policy.clone(), - notify: config.notify.clone(), + notify: UserNotifier::new(config.notify.clone()), cwd: config.cwd.clone(), }; @@ -259,7 +262,7 @@ struct State { pending_input: Vec, history: ConversationHistory, token_info: Option, - latest_rate_limits: Option, + latest_rate_limits: Option, } /// Context for an initialized model agent @@ -274,9 +277,7 @@ pub(crate) struct Session { session_manager: ExecSessionManager, unified_exec_manager: UnifiedExecSessionManager, - /// External notifier command (will be passed as args to exec()). When - /// `None` this feature is disabled. - notify: Option>, + notifier: UserNotifier, /// Optional rollout recorder for persisting the conversation transcript so /// sessions can be replayed or inspected later. @@ -303,6 +304,7 @@ pub(crate) struct TurnContext { pub(crate) shell_environment_policy: ShellEnvironmentPolicy, pub(crate) tools_config: ToolsConfig, pub(crate) is_review_mode: bool, + pub(crate) final_output_json_schema: Option, } impl TurnContext { @@ -335,10 +337,7 @@ struct ConfigureSession { /// How to sandbox commands executed in the system sandbox_policy: SandboxPolicy, - /// Optional external notifier command tokens. Present only when the - /// client wants the agent to spawn a program after each completed - /// turn. - notify: Option>, + notify: UserNotifier, /// Working directory that should be treated as the *root* of the /// session. All relative paths supplied by the model as well as the @@ -473,6 +472,7 @@ impl Session { shell_environment_policy: config.shell_environment_policy.clone(), cwd, is_review_mode: false, + final_output_json_schema: None, }; let sess = Arc::new(Session { conversation_id, @@ -480,7 +480,7 @@ impl Session { mcp_connection_manager, session_manager: ExecSessionManager::default(), unified_exec_manager: UnifiedExecSessionManager::default(), - notify, + notifier: notify, state: Mutex::new(state), rollout: Mutex::new(Some(rollout_recorder)), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), @@ -586,7 +586,7 @@ impl Session { command: Vec, cwd: PathBuf, reason: Option, - ) -> oneshot::Receiver { + ) -> ReviewDecision { // Add the tx_approve callback to the map before sending the request. let (tx_approve, rx_approve) = oneshot::channel(); let event_id = sub_id.clone(); @@ -608,7 +608,7 @@ impl Session { }), }; self.send_event(event).await; - rx_approve + rx_approve.await.unwrap_or_default() } pub async fn request_patch_approval( @@ -740,31 +740,42 @@ impl Session { async fn update_token_usage_info( &self, + sub_id: &str, turn_context: &TurnContext, token_usage: Option<&TokenUsage>, ) { - let mut state = self.state.lock().await; - if let Some(token_usage) = token_usage { - let info = TokenUsageInfo::new_or_append( - &state.token_info, - &Some(token_usage.clone()), - turn_context.client.get_model_context_window(), - ); - state.token_info = info; + { + let mut state = self.state.lock().await; + if let Some(token_usage) = token_usage { + let info = TokenUsageInfo::new_or_append( + &state.token_info, + &Some(token_usage.clone()), + turn_context.client.get_model_context_window(), + ); + state.token_info = info; + } } + self.send_token_count_event(sub_id).await; } - async fn update_rate_limits(&self, new_rate_limits: RateLimitSnapshotEvent) { - let mut state = self.state.lock().await; - state.latest_rate_limits = Some(new_rate_limits); + async fn update_rate_limits(&self, sub_id: &str, new_rate_limits: RateLimitSnapshot) { + { + let mut state = self.state.lock().await; + state.latest_rate_limits = Some(new_rate_limits); + } + self.send_token_count_event(sub_id).await; } - async fn get_token_count_event(&self) -> TokenCountEvent { - let state = self.state.lock().await; - TokenCountEvent { - info: state.token_info.clone(), - rate_limits: state.latest_rate_limits.clone(), - } + async fn send_token_count_event(&self, sub_id: &str) { + let (info, rate_limits) = { + let state = self.state.lock().await; + (state.token_info.clone(), state.latest_rate_limits.clone()) + }; + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::TokenCount(TokenCountEvent { info, rate_limits }), + }; + self.send_event(event).await; } /// Record a user input item to conversation history and also persist a @@ -1034,33 +1045,8 @@ impl Session { } } - /// Spawn the configured notifier (if any) with the given JSON payload as - /// the last argument. Failures are logged but otherwise ignored so that - /// notification issues do not interfere with the main workflow. - fn maybe_notify(&self, notification: UserNotification) { - let Some(notify_command) = &self.notify else { - return; - }; - - if notify_command.is_empty() { - return; - } - - let Ok(json) = serde_json::to_string(¬ification) else { - error!("failed to serialise notification payload"); - return; - }; - - let mut command = std::process::Command::new(¬ify_command[0]); - if notify_command.len() > 1 { - command.args(¬ify_command[1..]); - } - command.arg(json); - - // Fire-and-forget – we do not wait for completion. - if let Err(e) = command.spawn() { - warn!("failed to spawn notifier '{}': {e}", notify_command[0]); - } + pub(crate) fn notifier(&self) -> &UserNotifier { + &self.notifier } } @@ -1146,16 +1132,13 @@ impl AgentTask { turn_context: Arc, sub_id: String, input: Vec, - compact_instructions: String, ) -> Self { let handle = { let sess = sess.clone(); let sub_id = sub_id.clone(); let tc = Arc::clone(&turn_context); - tokio::spawn(async move { - compact::run_compact_task(sess, tc, sub_id, input, compact_instructions).await - }) - .abort_handle() + tokio::spawn(async move { compact::run_compact_task(sess, tc, sub_id, input).await }) + .abort_handle() }; Self { sess, @@ -1269,6 +1252,7 @@ async fn submission_loop( shell_environment_policy: prev.shell_environment_policy.clone(), cwd: new_cwd.clone(), is_review_mode: false, + final_output_json_schema: None, }; // Install the new persistent context for subsequent tasks/turns. @@ -1303,6 +1287,7 @@ async fn submission_loop( model, effort, summary, + final_output_json_schema, } => { // attempt to inject input into current task if let Err(items) = sess.inject_input(items).await { @@ -1353,6 +1338,7 @@ async fn submission_loop( shell_environment_policy: turn_context.shell_environment_policy.clone(), cwd, is_review_mode: false, + final_output_json_schema, }; // if the environment context has changed, record it in the conversation history @@ -1463,7 +1449,7 @@ async fn submission_loop( // Attempt to inject input into current task if let Err(items) = sess .inject_input(vec![InputItem::Text { - text: compact::COMPACT_TRIGGER_TEXT.to_string(), + text: compact::SUMMARIZATION_PROMPT.to_string(), }]) .await { @@ -1607,6 +1593,7 @@ async fn spawn_review_thread( shell_environment_policy: parent_turn_context.shell_environment_policy.clone(), cwd: parent_turn_context.cwd.clone(), is_review_mode: true, + final_output_json_schema: None, }; // Seed the child task with the review prompt as the initial user message. @@ -1883,11 +1870,12 @@ async fn run_task( last_agent_message = get_last_assistant_message_from_turn( &items_to_record_in_conversation_history, ); - sess.maybe_notify(UserNotification::AgentTurnComplete { - turn_id: sub_id.clone(), - input_messages: turn_input_messages, - last_assistant_message: last_agent_message.clone(), - }); + sess.notifier() + .notify(&UserNotification::AgentTurnComplete { + turn_id: sub_id.clone(), + input_messages: turn_input_messages, + last_assistant_message: last_agent_message.clone(), + }); break; } continue; @@ -1972,6 +1960,7 @@ async fn run_turn( input, tools, base_instructions_override: turn_context.base_instructions.clone(), + output_schema: turn_context.final_output_json_schema.clone(), }; let mut retries = 0; @@ -1980,9 +1969,14 @@ async fn run_turn( Ok(output) => return Ok(output), Err(CodexErr::Interrupted) => return Err(CodexErr::Interrupted), Err(CodexErr::EnvVar(var)) => return Err(CodexErr::EnvVar(var)), - Err(e @ (CodexErr::UsageLimitReached(_) | CodexErr::UsageNotIncluded)) => { - return Err(e); + Err(CodexErr::UsageLimitReached(e)) => { + let rate_limits = e.rate_limits.clone(); + if let Some(rate_limits) = rate_limits { + sess.update_rate_limits(&sub_id, rate_limits).await; + } + return Err(CodexErr::UsageLimitReached(e)); } + Err(CodexErr::UsageNotIncluded) => return Err(CodexErr::UsageNotIncluded), Err(e) => { // Use the configured provider-specific stream retry budget. let max_retries = turn_context.client.get_provider().stream_max_retries(); @@ -2155,20 +2149,13 @@ async fn try_run_turn( ResponseEvent::RateLimits(snapshot) => { // Update internal state with latest rate limits, but defer sending until // token usage is available to avoid duplicate TokenCount events. - sess.update_rate_limits(snapshot).await; + sess.update_rate_limits(sub_id, snapshot).await; } ResponseEvent::Completed { response_id: _, token_usage, } => { - sess.update_token_usage_info(turn_context, token_usage.as_ref()) - .await; - let token_event = sess.get_token_count_event().await; - let _ = sess - .send_event(Event { - id: sub_id.to_string(), - msg: EventMsg::TokenCount(token_event), - }) + sess.update_token_usage_info(sub_id, turn_context, token_usage.as_ref()) .await; let unified_diff = turn_diff_tracker.get_unified_diff(); @@ -2246,18 +2233,41 @@ async fn handle_response_item( .. } => { info!("FunctionCall: {name}({arguments})"); - Some( - handle_function_call( + if let Some((server, tool_name)) = sess.mcp_connection_manager.parse_tool_name(&name) { + let resp = handle_mcp_tool_call( + sess, + sub_id, + call_id.clone(), + server, + tool_name, + arguments, + ) + .await; + Some(resp) + } else { + let result = handle_function_call( sess, turn_context, turn_diff_tracker, sub_id.to_string(), name, arguments, - call_id, + call_id.clone(), ) - .await, - ) + .await; + + let output = match result { + Ok(content) => FunctionCallOutputPayload { + content, + success: Some(true), + }, + Err(FunctionCallError::RespondToModel(msg)) => FunctionCallOutputPayload { + content: msg, + success: Some(false), + }, + }; + Some(ResponseInputItem::FunctionCallOutput { call_id, output }) + } } ResponseItem::LocalShellCall { id, @@ -2290,17 +2300,32 @@ async fn handle_response_item( }; let exec_params = to_exec_params(params, turn_context); - Some( - handle_container_exec_with_params( + { + let result = handle_container_exec_with_params( exec_params, sess, turn_context, turn_diff_tracker, sub_id.to_string(), - effective_call_id, + effective_call_id.clone(), ) - .await, - ) + .await; + + let output = match result { + Ok(content) => FunctionCallOutputPayload { + content, + success: Some(true), + }, + Err(FunctionCallError::RespondToModel(msg)) => FunctionCallOutputPayload { + content: msg, + success: Some(false), + }, + }; + Some(ResponseInputItem::FunctionCallOutput { + call_id: effective_call_id, + output, + }) + } } ResponseItem::CustomToolCall { id: _, @@ -2308,18 +2333,24 @@ async fn handle_response_item( name, input, status: _, - } => Some( - handle_custom_tool_call( + } => { + let result = handle_custom_tool_call( sess, turn_context, turn_diff_tracker, sub_id.to_string(), name, input, - call_id, + call_id.clone(), ) - .await, - ), + .await; + + let output = match result { + Ok(content) => content, + Err(FunctionCallError::RespondToModel(msg)) => msg, + }; + Some(ResponseInputItem::CustomToolCallOutput { call_id, output }) + } ResponseItem::FunctionCallOutput { .. } => { debug!("unexpected FunctionCallOutput from stream"); None @@ -2356,22 +2387,17 @@ async fn handle_response_item( async fn handle_unified_exec_tool_call( sess: &Session, - call_id: String, session_id: Option, arguments: Vec, timeout_ms: Option, -) -> ResponseInputItem { +) -> Result { let parsed_session_id = if let Some(session_id) = session_id { match session_id.parse::() { Ok(parsed) => Some(parsed), Err(output) => { - return ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - content: format!("invalid session_id: {session_id} due to error {output}"), - success: Some(false), - }, - }; + return Err(FunctionCallError::RespondToModel(format!( + "invalid session_id: {session_id} due to error {output:?}" + ))); } } } else { @@ -2384,40 +2410,29 @@ async fn handle_unified_exec_tool_call( timeout_ms, }; - let result = sess.unified_exec_manager.handle_request(request).await; + let value = sess + .unified_exec_manager + .handle_request(request) + .await + .map_err(|err| { + FunctionCallError::RespondToModel(format!("unified exec failed: {err:?}")) + })?; - let output_payload = match result { - Ok(value) => { - #[derive(Serialize)] - struct SerializedUnifiedExecResult<'a> { - session_id: Option, - output: &'a str, - } - - match serde_json::to_string(&SerializedUnifiedExecResult { - session_id: value.session_id.map(|id| id.to_string()), - output: &value.output, - }) { - Ok(serialized) => FunctionCallOutputPayload { - content: serialized, - success: Some(true), - }, - Err(err) => FunctionCallOutputPayload { - content: format!("failed to serialize unified exec output: {err}"), - success: Some(false), - }, - } - } - Err(err) => FunctionCallOutputPayload { - content: format!("unified exec failed: {err}"), - success: Some(false), - }, - }; - - ResponseInputItem::FunctionCallOutput { - call_id, - output: output_payload, + #[derive(Serialize)] + struct SerializedUnifiedExecResult { + session_id: Option, + output: String, } + + serde_json::to_string(&SerializedUnifiedExecResult { + session_id: value.session_id.map(|id| id.to_string()), + output: value.output, + }) + .map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to serialize unified exec output: {err:?}" + )) + }) } async fn handle_function_call( @@ -2428,15 +2443,10 @@ async fn handle_function_call( name: String, arguments: String, call_id: String, -) -> ResponseInputItem { +) -> Result { match name.as_str() { "container.exec" | "shell" => { - let params = match parse_container_exec_arguments(arguments, turn_context, &call_id) { - Ok(params) => params, - Err(output) => { - return *output; - } - }; + let params = parse_container_exec_arguments(arguments, turn_context, &call_id)?; handle_container_exec_with_params( params, sess, @@ -2457,74 +2467,41 @@ async fn handle_function_call( timeout_ms: Option, } - let args = match serde_json::from_str::(&arguments) { - Ok(args) => args, - Err(err) => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {err}"), - success: Some(false), - }, - }; - } - }; + let args: UnifiedExecArgs = serde_json::from_str(&arguments).map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {err:?}" + )) + })?; - handle_unified_exec_tool_call( - sess, - call_id, - args.session_id, - args.input, - args.timeout_ms, - ) - .await + handle_unified_exec_tool_call(sess, args.session_id, args.input, args.timeout_ms).await } "view_image" => { #[derive(serde::Deserialize)] struct SeeImageArgs { path: String, } - let args = match serde_json::from_str::(&arguments) { - Ok(a) => a, - Err(e) => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: Some(false), - }, - }; - } - }; + let args: SeeImageArgs = serde_json::from_str(&arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {e:?}" + )) + })?; let abs = turn_context.resolve_path(Some(args.path)); - let output = match sess - .inject_input(vec![InputItem::LocalImage { path: abs }]) + sess.inject_input(vec![InputItem::LocalImage { path: abs }]) .await - { - Ok(()) => FunctionCallOutputPayload { - content: "attached local image path".to_string(), - success: Some(true), - }, - Err(_) => FunctionCallOutputPayload { - content: "unable to attach image (no active task)".to_string(), - success: Some(false), - }, - }; - ResponseInputItem::FunctionCallOutput { call_id, output } + .map_err(|_| { + FunctionCallError::RespondToModel( + "unable to attach image (no active task)".to_string(), + ) + })?; + + Ok("attached local image path".to_string()) } "apply_patch" => { - let args = match serde_json::from_str::(&arguments) { - Ok(a) => a, - Err(e) => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: None, - }, - }; - } - }; + let args: ApplyPatchToolArgs = serde_json::from_str(&arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {e:?}" + )) + })?; let exec_params = ExecParams { command: vec!["apply_patch".to_string(), args.input.clone()], cwd: turn_context.cwd.clone(), @@ -2546,69 +2523,39 @@ async fn handle_function_call( "update_plan" => handle_update_plan(sess, arguments, sub_id, call_id).await, EXEC_COMMAND_TOOL_NAME => { // TODO(mbolin): Sandbox check. - let exec_params = match serde_json::from_str::(&arguments) { - Ok(params) => params, - Err(e) => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: Some(false), - }, - }; - } - }; + let exec_params: ExecCommandParams = serde_json::from_str(&arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {e:?}" + )) + })?; let result = sess .session_manager .handle_exec_command_request(exec_params) .await; - let function_call_output = crate::exec_command::result_into_payload(result); - ResponseInputItem::FunctionCallOutput { - call_id, - output: function_call_output, + match result { + Ok(output) => Ok(output.to_text_output()), + Err(err) => Err(FunctionCallError::RespondToModel(err)), } } WRITE_STDIN_TOOL_NAME => { - let write_stdin_params = match serde_json::from_str::(&arguments) { - Ok(params) => params, - Err(e) => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: Some(false), - }, - }; - } - }; + let write_stdin_params = + serde_json::from_str::(&arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {e:?}" + )) + })?; + let result = sess .session_manager .handle_write_stdin_request(write_stdin_params) - .await; - let function_call_output: FunctionCallOutputPayload = - crate::exec_command::result_into_payload(result); - ResponseInputItem::FunctionCallOutput { - call_id, - output: function_call_output, - } - } - _ => { - match sess.mcp_connection_manager.parse_tool_name(&name) { - Some((server, tool_name)) => { - handle_mcp_tool_call(sess, &sub_id, call_id, server, tool_name, arguments).await - } - None => { - // Unknown function: reply with structured failure so the model can adapt. - ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("unsupported call: {name}"), - success: None, - }, - } - } - } + .await + .map_err(FunctionCallError::RespondToModel)?; + + Ok(result.to_text_output()) } + _ => Err(FunctionCallError::RespondToModel(format!( + "unsupported call: {name}" + ))), } } @@ -2620,7 +2567,7 @@ async fn handle_custom_tool_call( name: String, input: String, call_id: String, -) -> ResponseInputItem { +) -> Result { info!("CustomToolCall: {name} {input}"); match name.as_str() { "apply_patch" => { @@ -2632,7 +2579,8 @@ async fn handle_custom_tool_call( with_escalated_permissions: None, justification: None, }; - let resp = handle_container_exec_with_params( + + handle_container_exec_with_params( exec_params, sess, turn_context, @@ -2640,26 +2588,13 @@ async fn handle_custom_tool_call( sub_id, call_id, ) - .await; - - // Convert function-call style output into a custom tool call output - match resp { - ResponseInputItem::FunctionCallOutput { call_id, output } => { - ResponseInputItem::CustomToolCallOutput { - call_id, - output: output.content, - } - } - // Pass through if already a custom tool output or other variant - other => other, - } + .await } _ => { debug!("unexpected CustomToolCall from stream"); - ResponseInputItem::CustomToolCallOutput { - call_id, - output: format!("unsupported custom tool call: {name}"), - } + Err(FunctionCallError::RespondToModel(format!( + "unsupported custom tool call: {name}" + ))) } } } @@ -2678,23 +2613,13 @@ fn to_exec_params(params: ShellToolCallParams, turn_context: &TurnContext) -> Ex fn parse_container_exec_arguments( arguments: String, turn_context: &TurnContext, - call_id: &str, -) -> Result> { - // parse command - match serde_json::from_str::(&arguments) { - Ok(shell_tool_call_params) => Ok(to_exec_params(shell_tool_call_params, turn_context)), - Err(e) => { - // allow model to re-sample - let output = ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: None, - }, - }; - Err(Box::new(output)) - } - } + _call_id: &str, +) -> Result { + serde_json::from_str::(&arguments) + .map(|p| to_exec_params(p, turn_context)) + .map_err(|e| { + FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e:?}")) + }) } pub struct ExecInvokeArgs<'a> { @@ -2731,20 +2656,14 @@ async fn handle_container_exec_with_params( turn_diff_tracker: &mut TurnDiffTracker, sub_id: String, call_id: String, -) -> ResponseInputItem { +) -> Result { if params.with_escalated_permissions.unwrap_or(false) && !matches!(turn_context.approval_policy, AskForApproval::OnRequest) { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!( - "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", - policy = turn_context.approval_policy - ), - success: None, - }, - }; + return Err(FunctionCallError::RespondToModel(format!( + "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", + policy = turn_context.approval_policy + ))); } // check if this was a patch, and apply it if so @@ -2761,13 +2680,9 @@ async fn handle_container_exec_with_params( // It looks like an invocation of `apply_patch`, but we // could not resolve it into a patch that would apply // cleanly. Return to model for resample. - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("error: {parse_error:#}"), - success: None, - }, - }; + return Err(FunctionCallError::RespondToModel(format!( + "error: {parse_error:#?}" + ))); } MaybeApplyPatchVerified::ShellParseError(error) => { trace!("Failed to parse shell command, {error:?}"); @@ -2785,13 +2700,9 @@ async fn handle_container_exec_with_params( .ok() .map(|p| p.to_string_lossy().to_string()); let Some(path_to_codex) = path_to_codex else { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: "failed to determine path to codex executable".to_string(), - success: None, - }, - }; + return Err(FunctionCallError::RespondToModel( + "failed to determine path to codex executable".to_string(), + )); }; let params = ExecParams { @@ -2842,7 +2753,7 @@ async fn handle_container_exec_with_params( let sandbox_type = match safety { SafetyCheck::AutoApprove { sandbox_type } => sandbox_type, SafetyCheck::AskUser => { - let rx_approve = sess + let decision = sess .request_command_approval( sub_id.clone(), call_id.clone(), @@ -2851,19 +2762,15 @@ async fn handle_container_exec_with_params( params.justification.clone(), ) .await; - match rx_approve.await.unwrap_or_default() { + match decision { ReviewDecision::Approved => (), ReviewDecision::ApprovedForSession => { sess.add_approved_command(params.command.clone()).await; } ReviewDecision::Denied | ReviewDecision::Abort => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: "exec command rejected by user".to_string(), - success: None, - }, - }; + return Err(FunctionCallError::RespondToModel( + "exec command rejected by user".to_string(), + )); } } // No sandboxing is applied because the user has given @@ -2873,13 +2780,9 @@ async fn handle_container_exec_with_params( SandboxType::None } SafetyCheck::Reject { reason } => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("exec command rejected: {reason}"), - success: None, - }, - }; + return Err(FunctionCallError::RespondToModel(format!( + "exec command rejected: {reason:?}" + ))); } }; @@ -2926,15 +2829,11 @@ async fn handle_container_exec_with_params( match output_result { Ok(output) => { let ExecToolCallOutput { exit_code, .. } = &output; - - let is_success = *exit_code == 0; let content = format_exec_output(&output); - ResponseInputItem::FunctionCallOutput { - call_id: call_id.clone(), - output: FunctionCallOutputPayload { - content, - success: Some(is_success), - }, + if *exit_code == 0 { + Ok(content) + } else { + Err(FunctionCallError::RespondToModel(content)) } } Err(CodexErr::Sandbox(error)) => { @@ -2949,13 +2848,9 @@ async fn handle_container_exec_with_params( ) .await } - Err(e) => ResponseInputItem::FunctionCallOutput { - call_id: call_id.clone(), - output: FunctionCallOutputPayload { - content: format!("execution error: {e}"), - success: None, - }, - }, + Err(e) => Err(FunctionCallError::RespondToModel(format!( + "execution error: {e:?}" + ))), } } @@ -2967,35 +2862,23 @@ async fn handle_sandbox_error( sandbox_type: SandboxType, sess: &Session, turn_context: &TurnContext, -) -> ResponseInputItem { +) -> Result { let call_id = exec_command_context.call_id.clone(); let sub_id = exec_command_context.sub_id.clone(); let cwd = exec_command_context.cwd.clone(); if let SandboxErr::Timeout { output } = &error { let content = format_exec_output(output); - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content, - success: Some(false), - }, - }; + return Err(FunctionCallError::RespondToModel(content)); } // Early out if either the user never wants to be asked for approval, or // we're letting the model manage escalation requests. Otherwise, continue match turn_context.approval_policy { AskForApproval::Never | AskForApproval::OnRequest => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!( - "failed in sandbox {sandbox_type:?} with execution error: {error}" - ), - success: Some(false), - }, - }; + return Err(FunctionCallError::RespondToModel(format!( + "failed in sandbox {sandbox_type:?} with execution error: {error:?}" + ))); } AskForApproval::UnlessTrusted | AskForApproval::OnFailure => (), } @@ -3012,7 +2895,7 @@ async fn handle_sandbox_error( sess.notify_background_event(&sub_id, format!("Execution failed: {error}")) .await; - let rx_approve = sess + let decision = sess .request_command_approval( sub_id.clone(), call_id.clone(), @@ -3022,7 +2905,7 @@ async fn handle_sandbox_error( ) .await; - match rx_approve.await.unwrap_or_default() { + match decision { ReviewDecision::Approved | ReviewDecision::ApprovedForSession => { // Persist this command as pre‑approved for the // remainder of the session so future @@ -3061,36 +2944,23 @@ async fn handle_sandbox_error( match retry_output_result { Ok(retry_output) => { let ExecToolCallOutput { exit_code, .. } = &retry_output; - - let is_success = *exit_code == 0; let content = format_exec_output(&retry_output); - - ResponseInputItem::FunctionCallOutput { - call_id: call_id.clone(), - output: FunctionCallOutputPayload { - content, - success: Some(is_success), - }, + if *exit_code == 0 { + Ok(content) + } else { + Err(FunctionCallError::RespondToModel(content)) } } - Err(e) => ResponseInputItem::FunctionCallOutput { - call_id: call_id.clone(), - output: FunctionCallOutputPayload { - content: format!("retry failed: {e}"), - success: None, - }, - }, + Err(e) => Err(FunctionCallError::RespondToModel(format!( + "retry failed: {e}" + ))), } } ReviewDecision::Denied | ReviewDecision::Abort => { // Fall through to original failure handling. - ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: "exec command rejected by user".to_string(), - success: None, - }, - } + Err(FunctionCallError::RespondToModel( + "exec command rejected by user".to_string(), + )) } } } @@ -3635,6 +3505,7 @@ mod tests { shell_environment_policy: config.shell_environment_policy.clone(), tools_config, is_review_mode: false, + final_output_json_schema: None, }; let session = Session { conversation_id, @@ -3642,7 +3513,7 @@ mod tests { mcp_connection_manager: McpConnectionManager::default(), session_manager: ExecSessionManager::default(), unified_exec_manager: UnifiedExecSessionManager::default(), - notify: None, + notifier: UserNotifier::default(), rollout: Mutex::new(None), state: Mutex::new(State { history: ConversationHistory::new(), @@ -3811,8 +3682,8 @@ mod tests { ) .await; - let ResponseInputItem::FunctionCallOutput { output, .. } = resp else { - panic!("expected FunctionCallOutput"); + let Err(FunctionCallError::RespondToModel(output)) = resp else { + panic!("expected error result"); }; let expected = format!( @@ -3820,7 +3691,7 @@ mod tests { policy = turn_context.approval_policy ); - pretty_assertions::assert_eq!(output.content, expected); + pretty_assertions::assert_eq!(output, expected); // Now retry the same command WITHOUT escalated permissions; should succeed. // Force DangerFullAccess to avoid platform sandbox dependencies in tests. @@ -3836,9 +3707,7 @@ mod tests { ) .await; - let ResponseInputItem::FunctionCallOutput { output, .. } = resp2 else { - panic!("expected FunctionCallOutput on retry"); - }; + let output = resp2.expect("expected Ok result"); #[derive(Deserialize, PartialEq, Eq, Debug)] struct ResponseExecMetadata { @@ -3852,10 +3721,9 @@ mod tests { } let exec_output: ResponseExecOutput = - serde_json::from_str(&output.content).expect("valid exec output json"); + serde_json::from_str(&output).expect("valid exec output json"); pretty_assertions::assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 }); assert!(exec_output.output.contains("hi")); - pretty_assertions::assert_eq!(output.success, Some(true)); } } diff --git a/codex-rs/core/src/codex/compact.rs b/codex-rs/core/src/codex/compact.rs index d1547a4818..8f213d4e7e 100644 --- a/codex-rs/core/src/codex/compact.rs +++ b/codex-rs/core/src/codex/compact.rs @@ -27,8 +27,7 @@ use codex_protocol::models::ResponseItem; use codex_protocol::protocol::RolloutItem; use futures::prelude::*; -pub(super) const COMPACT_TRIGGER_TEXT: &str = "Start Summarization"; -const SUMMARIZATION_PROMPT: &str = include_str!("../../templates/compact/prompt.md"); +pub const SUMMARIZATION_PROMPT: &str = include_str!("../../templates/compact/prompt.md"); const COMPACT_USER_MESSAGE_MAX_TOKENS: usize = 20_000; #[derive(Template)] @@ -44,13 +43,7 @@ pub(super) async fn spawn_compact_task( sub_id: String, input: Vec, ) { - let task = AgentTask::compact( - sess.clone(), - turn_context, - sub_id, - input, - SUMMARIZATION_PROMPT.to_string(), - ); + let task = AgentTask::compact(sess.clone(), turn_context, sub_id, input); sess.set_task(task).await; } @@ -60,17 +53,9 @@ pub(super) async fn run_inline_auto_compact_task( ) { let sub_id = sess.next_internal_sub_id(); let input = vec![InputItem::Text { - text: COMPACT_TRIGGER_TEXT.to_string(), + text: SUMMARIZATION_PROMPT.to_string(), }]; - run_compact_task_inner( - sess, - turn_context, - sub_id, - input, - SUMMARIZATION_PROMPT.to_string(), - false, - ) - .await; + run_compact_task_inner(sess, turn_context, sub_id, input, false).await; } pub(super) async fn run_compact_task( @@ -78,7 +63,6 @@ pub(super) async fn run_compact_task( turn_context: Arc, sub_id: String, input: Vec, - compact_instructions: String, ) { let start_event = Event { id: sub_id.clone(), @@ -87,15 +71,7 @@ pub(super) async fn run_compact_task( }), }; sess.send_event(start_event).await; - run_compact_task_inner( - sess.clone(), - turn_context, - sub_id.clone(), - input, - compact_instructions, - true, - ) - .await; + run_compact_task_inner(sess.clone(), turn_context, sub_id.clone(), input, true).await; let event = Event { id: sub_id, msg: EventMsg::TaskComplete(TaskCompleteEvent { @@ -110,19 +86,16 @@ async fn run_compact_task_inner( turn_context: Arc, sub_id: String, input: Vec, - compact_instructions: String, remove_task_on_completion: bool, ) { let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input); - let instructions_override = compact_instructions; let turn_input = sess .turn_input_with_history(vec![initial_input_for_turn.clone().into()]) .await; let prompt = Prompt { input: turn_input, - tools: Vec::new(), - base_instructions_override: Some(instructions_override), + ..Default::default() }; let max_retries = turn_context.client.get_provider().stream_max_retries(); diff --git a/codex-rs/core/src/is_safe_command.rs b/codex-rs/core/src/command_safety/is_safe_command.rs similarity index 98% rename from codex-rs/core/src/is_safe_command.rs rename to codex-rs/core/src/command_safety/is_safe_command.rs index 12eb36e0a5..c3321540d1 100644 --- a/codex-rs/core/src/is_safe_command.rs +++ b/codex-rs/core/src/command_safety/is_safe_command.rs @@ -2,6 +2,14 @@ use crate::bash::try_parse_bash; use crate::bash::try_parse_word_only_commands_sequence; pub fn is_known_safe_command(command: &[String]) -> bool { + #[cfg(target_os = "windows")] + { + use super::windows_safe_commands::is_safe_command_windows; + if is_safe_command_windows(command) { + return true; + } + } + if is_safe_to_call_with_exec(command) { return true; } @@ -24,7 +32,6 @@ pub fn is_known_safe_command(command: &[String]) -> bool { { return true; } - false } diff --git a/codex-rs/core/src/command_safety/mod.rs b/codex-rs/core/src/command_safety/mod.rs new file mode 100644 index 0000000000..b3095c86a1 --- /dev/null +++ b/codex-rs/core/src/command_safety/mod.rs @@ -0,0 +1,3 @@ +pub mod is_safe_command; +#[cfg(target_os = "windows")] +pub mod windows_safe_commands; diff --git a/codex-rs/core/src/command_safety/windows_safe_commands.rs b/codex-rs/core/src/command_safety/windows_safe_commands.rs new file mode 100644 index 0000000000..c6e781f8ae --- /dev/null +++ b/codex-rs/core/src/command_safety/windows_safe_commands.rs @@ -0,0 +1,25 @@ +// This is a WIP. This will eventually contain a real list of common safe Windows commands. +pub fn is_safe_command_windows(_command: &[String]) -> bool { + false +} + +#[cfg(test)] +mod tests { + use super::is_safe_command_windows; + + fn vec_str(args: &[&str]) -> Vec { + args.iter().map(ToString::to_string).collect() + } + + #[test] + fn everything_is_unsafe() { + for cmd in [ + vec_str(&["powershell.exe", "-NoLogo", "-Command", "echo hello"]), + vec_str(&["copy", "foo", "bar"]), + vec_str(&["del", "file.txt"]), + vec_str(&["powershell.exe", "Get-ChildItem"]), + ] { + assert!(!is_safe_command_windows(&cmd)); + } + } +} diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 77b447e5cf..1fe7e1150a 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -2,6 +2,7 @@ use crate::exec::ExecToolCallOutput; use crate::token_data::KnownPlan; use crate::token_data::PlanType; use codex_protocol::mcp_protocol::ConversationId; +use codex_protocol::protocol::RateLimitSnapshot; use reqwest::StatusCode; use serde_json; use std::io; @@ -104,6 +105,9 @@ pub enum CodexErr { #[error("codex-linux-sandbox was required but not provided")] LandlockSandboxExecutableNotProvided, + #[error("unsupported operation: {0}")] + UnsupportedOperation(String), + // ----------------------------------------------------------------- // Automatic conversions for common external error types // ----------------------------------------------------------------- @@ -135,6 +139,7 @@ pub enum CodexErr { pub struct UsageLimitReachedError { pub(crate) plan_type: Option, pub(crate) resets_in_seconds: Option, + pub(crate) rate_limits: Option, } impl std::fmt::Display for UsageLimitReachedError { @@ -262,12 +267,29 @@ pub fn get_error_message_ui(e: &CodexErr) -> String { #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::RateLimitWindow; + + fn rate_limit_snapshot() -> RateLimitSnapshot { + RateLimitSnapshot { + primary: Some(RateLimitWindow { + used_percent: 50.0, + window_minutes: Some(60), + resets_in_seconds: Some(3600), + }), + secondary: Some(RateLimitWindow { + used_percent: 30.0, + window_minutes: Some(120), + resets_in_seconds: Some(7200), + }), + } + } #[test] fn usage_limit_reached_error_formats_plus_plan() { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Plus)), resets_in_seconds: None, + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -280,6 +302,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Free)), resets_in_seconds: Some(3600), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -292,6 +315,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: None, resets_in_seconds: None, + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -304,6 +328,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Team)), resets_in_seconds: Some(3600), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -316,6 +341,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Business)), resets_in_seconds: None, + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -328,6 +354,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Pro)), resets_in_seconds: None, + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -340,6 +367,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: None, resets_in_seconds: Some(5 * 60), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -352,6 +380,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: Some(PlanType::Known(KnownPlan::Plus)), resets_in_seconds: Some(3 * 3600 + 32 * 60), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -364,6 +393,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: None, resets_in_seconds: Some(2 * 86_400 + 3 * 3600 + 5 * 60), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), @@ -376,6 +406,7 @@ mod tests { let err = UsageLimitReachedError { plan_type: None, resets_in_seconds: Some(30), + rate_limits: Some(rate_limit_snapshot()), }; assert_eq!( err.to_string(), diff --git a/codex-rs/core/src/exec_command/mod.rs b/codex-rs/core/src/exec_command/mod.rs index 103b76ad67..2cfd022510 100644 --- a/codex-rs/core/src/exec_command/mod.rs +++ b/codex-rs/core/src/exec_command/mod.rs @@ -12,4 +12,3 @@ pub use responses_api::WRITE_STDIN_TOOL_NAME; pub use responses_api::create_exec_command_tool_for_responses_api; pub use responses_api::create_write_stdin_tool_for_responses_api; pub use session_manager::SessionManager as ExecSessionManager; -pub use session_manager::result_into_payload; diff --git a/codex-rs/core/src/exec_command/session_manager.rs b/codex-rs/core/src/exec_command/session_manager.rs index 61c70f0cc2..cd1c5329a2 100644 --- a/codex-rs/core/src/exec_command/session_manager.rs +++ b/codex-rs/core/src/exec_command/session_manager.rs @@ -21,7 +21,6 @@ use crate::exec_command::exec_command_params::WriteStdinParams; use crate::exec_command::exec_command_session::ExecCommandSession; use crate::exec_command::session_id::SessionId; use crate::truncate::truncate_middle; -use codex_protocol::models::FunctionCallOutputPayload; #[derive(Debug, Default)] pub struct SessionManager { @@ -38,7 +37,7 @@ pub struct ExecCommandOutput { } impl ExecCommandOutput { - fn to_text_output(&self) -> String { + pub(crate) fn to_text_output(&self) -> String { let wall_time_secs = self.wall_time.as_secs_f32(); let termination_status = match self.exit_status { ExitStatus::Exited(code) => format!("Process exited with code {code}"), @@ -68,19 +67,6 @@ pub enum ExitStatus { Ongoing(SessionId), } -pub fn result_into_payload(result: Result) -> FunctionCallOutputPayload { - match result { - Ok(output) => FunctionCallOutputPayload { - content: output.to_text_output(), - success: Some(true), - }, - Err(err) => FunctionCallOutputPayload { - content: err, - success: Some(false), - }, - } -} - impl SessionManager { /// Processes the request and is required to send a response via `outgoing`. pub async fn handle_exec_command_request( diff --git a/codex-rs/core/src/flags.rs b/codex-rs/core/src/flags.rs index 60201a3b18..de693868df 100644 --- a/codex-rs/core/src/flags.rs +++ b/codex-rs/core/src/flags.rs @@ -1,16 +1,6 @@ -use std::time::Duration; - use env_flags::env_flags; env_flags! { - pub OPENAI_API_BASE: &str = "https://api.openai.com/v1"; - - /// Fallback when the provider-specific key is not set. - pub OPENAI_API_KEY: Option<&str> = None; - pub OPENAI_TIMEOUT_MS: Duration = Duration::from_millis(300_000), |value| { - value.parse().map(Duration::from_millis) - }; - /// Fixture path for offline tests (see client.rs). pub CODEX_RS_SSE_FIXTURE: Option<&str> = None; } diff --git a/codex-rs/core/src/function_tool.rs b/codex-rs/core/src/function_tool.rs new file mode 100644 index 0000000000..756cef3e3e --- /dev/null +++ b/codex-rs/core/src/function_tool.rs @@ -0,0 +1,7 @@ +use thiserror::Error; + +#[derive(Debug, Error, PartialEq)] +pub enum FunctionCallError { + #[error("{0}")] + RespondToModel(String), +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index f73bef4051..2db1e6e763 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -15,6 +15,7 @@ pub mod codex; mod codex_conversation; pub mod token_data; pub use codex_conversation::CodexConversation; +mod command_safety; pub mod config; pub mod config_edit; pub mod config_profile; @@ -29,7 +30,6 @@ pub mod exec_env; mod flags; pub mod git_info; pub mod internal_storage; -mod is_safe_command; pub mod landlock; mod mcp_connection_manager; mod mcp_tool_call; @@ -75,10 +75,12 @@ pub use rollout::find_conversation_path_by_id_str; pub use rollout::list::ConversationItem; pub use rollout::list::ConversationsPage; pub use rollout::list::Cursor; +mod function_tool; mod user_notification; pub mod util; pub use apply_patch::CODEX_APPLY_PATCH_ARG1; +pub use command_safety::is_safe_command; pub use safety::get_platform_sandbox; // Re-export the protocol types from the standalone `codex-protocol` crate so existing // `codex_core::protocol::...` references continue to work across the workspace. diff --git a/codex-rs/core/src/openai_model_info.rs b/codex-rs/core/src/openai_model_info.rs index 6fbe99fffc..d1d2305aaa 100644 --- a/codex-rs/core/src/openai_model_info.rs +++ b/codex-rs/core/src/openai_model_info.rs @@ -7,13 +7,14 @@ use crate::model_family::ModelFamily; /// Though this would help present more accurate pricing information in the UI. #[derive(Debug)] pub(crate) struct ModelInfo { - /// Size of the context window in tokens. + /// Size of the context window in tokens. This is the maximum size of the input context. pub(crate) context_window: u64, /// Maximum number of output tokens that can be generated for the model. pub(crate) max_output_tokens: u64, - /// Token threshold where we should automatically compact conversation history. + /// Token threshold where we should automatically compact conversation history. This considers + /// input tokens + output tokens of this turn. pub(crate) auto_compact_token_limit: Option, } @@ -61,6 +62,12 @@ pub(crate) fn get_model_info(model_family: &ModelFamily) -> Option { // https://platform.openai.com/docs/models/gpt-3.5-turbo "gpt-3.5-turbo" => Some(ModelInfo::new(16_385, 4_096)), + _ if slug.starts_with("gpt-5-codex") => Some(ModelInfo { + context_window: 272_000, + max_output_tokens: 128_000, + auto_compact_token_limit: Some(350_000), + }), + _ if slug.starts_with("gpt-5") => Some(ModelInfo::new(272_000, 128_000)), _ if slug.starts_with("codex-") => Some(ModelInfo::new(272_000, 128_000)), diff --git a/codex-rs/core/src/plan_tool.rs b/codex-rs/core/src/plan_tool.rs index a712b8b8dd..c81675698f 100644 --- a/codex-rs/core/src/plan_tool.rs +++ b/codex-rs/core/src/plan_tool.rs @@ -2,13 +2,12 @@ use std::collections::BTreeMap; use std::sync::LazyLock; use crate::codex::Session; +use crate::function_tool::FunctionCallError; use crate::openai_tools::JsonSchema; use crate::openai_tools::OpenAiTool; use crate::openai_tools::ResponsesApiTool; use crate::protocol::Event; use crate::protocol::EventMsg; -use codex_protocol::models::FunctionCallOutputPayload; -use codex_protocol::models::ResponseInputItem; // Use the canonical plan tool types from the protocol crate to ensure // type-identity matches events transported via `codex_protocol`. @@ -67,44 +66,20 @@ pub(crate) async fn handle_update_plan( session: &Session, arguments: String, sub_id: String, - call_id: String, -) -> ResponseInputItem { - match parse_update_plan_arguments(arguments, &call_id) { - Ok(args) => { - let output = ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: "Plan updated".to_string(), - success: Some(true), - }, - }; - session - .send_event(Event { - id: sub_id.to_string(), - msg: EventMsg::PlanUpdate(args), - }) - .await; - output - } - Err(output) => *output, - } + _call_id: String, +) -> Result { + let args = parse_update_plan_arguments(&arguments)?; + session + .send_event(Event { + id: sub_id.to_string(), + msg: EventMsg::PlanUpdate(args), + }) + .await; + Ok("Plan updated".to_string()) } -fn parse_update_plan_arguments( - arguments: String, - call_id: &str, -) -> Result> { - match serde_json::from_str::(&arguments) { - Ok(args) => Ok(args), - Err(e) => { - let output = ResponseInputItem::FunctionCallOutput { - call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - content: format!("failed to parse function arguments: {e}"), - success: None, - }, - }; - Err(Box::new(output)) - } - } +fn parse_update_plan_arguments(arguments: &str) -> Result { + serde_json::from_str::(arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e}")) + }) } diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 6befdb1fe9..23e645413f 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -7,8 +7,6 @@ use std::path::Path; use std::path::PathBuf; use codex_protocol::mcp_protocol::ConversationId; -use serde::Deserialize; -use serde::Serialize; use serde_json::Value; use time::OffsetDateTime; use time::format_description::FormatItem; @@ -28,7 +26,6 @@ use super::policy::is_persisted_response_item; use crate::config::Config; use crate::default_client::ORIGINATOR; use crate::git_info::collect_git_info; -use codex_protocol::models::ResponseItem; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; @@ -36,19 +33,6 @@ use codex_protocol::protocol::RolloutLine; use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; -#[derive(Serialize, Deserialize, Default, Clone)] -pub struct SessionStateSnapshot {} - -#[derive(Serialize, Deserialize, Default, Clone)] -pub struct SavedSession { - pub session: SessionMeta, - #[serde(default)] - pub items: Vec, - #[serde(default)] - pub state: SessionStateSnapshot, - pub session_id: ConversationId, -} - /// Records all [`ResponseItem`]s for a session and flushes them to disk after /// every update. /// diff --git a/codex-rs/core/src/tool_apply_patch.rs b/codex-rs/core/src/tool_apply_patch.rs index d4039fd192..164a1e127f 100644 --- a/codex-rs/core/src/tool_apply_patch.rs +++ b/codex-rs/core/src/tool_apply_patch.rs @@ -10,11 +10,6 @@ use crate::openai_tools::ResponsesApiTool; const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("tool_apply_patch.lark"); -#[derive(Serialize, Deserialize)] -pub(crate) struct ApplyPatchToolArgs { - pub(crate) input: String, -} - #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] #[serde(rename_all = "snake_case")] pub enum ApplyPatchToolType { diff --git a/codex-rs/core/src/user_notification.rs b/codex-rs/core/src/user_notification.rs index 0a3cb49e78..5eb9e98058 100644 --- a/codex-rs/core/src/user_notification.rs +++ b/codex-rs/core/src/user_notification.rs @@ -1,4 +1,45 @@ use serde::Serialize; +use tracing::error; +use tracing::warn; + +#[derive(Debug, Default)] +pub(crate) struct UserNotifier { + notify_command: Option>, +} + +impl UserNotifier { + pub(crate) fn notify(&self, notification: &UserNotification) { + if let Some(notify_command) = &self.notify_command + && !notify_command.is_empty() + { + self.invoke_notify(notify_command, notification) + } + } + + fn invoke_notify(&self, notify_command: &[String], notification: &UserNotification) { + let Ok(json) = serde_json::to_string(¬ification) else { + error!("failed to serialise notification payload"); + return; + }; + + let mut command = std::process::Command::new(¬ify_command[0]); + if notify_command.len() > 1 { + command.args(¬ify_command[1..]); + } + command.arg(json); + + // Fire-and-forget – we do not wait for completion. + if let Err(e) = command.spawn() { + warn!("failed to spawn notifier '{}': {e}", notify_command[0]); + } + } + + pub(crate) fn new(notify: Option>) -> Self { + Self { + notify_command: notify, + } + } +} /// User can configure a program that will receive notifications. Each /// notification is serialized as JSON and passed as an argument to the @@ -21,9 +62,10 @@ pub(crate) enum UserNotification { #[cfg(test)] mod tests { use super::*; + use anyhow::Result; #[test] - fn test_user_notification() { + fn test_user_notification() -> Result<()> { let notification = UserNotification::AgentTurnComplete { turn_id: "12345".to_string(), input_messages: vec!["Rename `foo` to `bar` and update the callsites.".to_string()], @@ -31,10 +73,11 @@ mod tests { "Rename complete and verified `cargo build` succeeds.".to_string(), ), }; - let serialized = serde_json::to_string(¬ification).unwrap(); + let serialized = serde_json::to_string(¬ification)?; assert_eq!( serialized, r#"{"type":"agent-turn-complete","turn-id":"12345","input-messages":["Rename `foo` to `bar` and update the callsites."],"last-assistant-message":"Rename complete and verified `cargo build` succeeds."}"# ); + Ok(()) } } diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 0a33be39dc..f85aaa9e38 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -1,12 +1,13 @@ [package] +edition = "2024" name = "core_test_support" version = { workspace = true } -edition = "2024" [lib] path = "lib.rs" [dependencies] +anyhow = { workspace = true } codex-core = { workspace = true } serde_json = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 9b86694e00..0fdd60387b 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -8,6 +8,7 @@ use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; pub mod responses; +pub mod test_codex; /// Returns a default `Config` whose on-disk state is confined to the provided /// temporary directory. Using a per-test directory keeps tests hermetic and diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs new file mode 100644 index 0000000000..65b25b13c9 --- /dev/null +++ b/codex-rs/core/tests/common/test_codex.rs @@ -0,0 +1,75 @@ +use std::mem::swap; +use std::sync::Arc; + +use codex_core::CodexAuth; +use codex_core::CodexConversation; +use codex_core::ConversationManager; +use codex_core::ModelProviderInfo; +use codex_core::NewConversation; +use codex_core::built_in_model_providers; +use codex_core::config::Config; +use codex_core::protocol::SessionConfiguredEvent; +use tempfile::TempDir; + +use crate::load_default_config_for_test; + +type ConfigMutator = dyn FnOnce(&mut Config); + +pub struct TestCodexBuilder { + config_mutators: Vec>, +} + +impl TestCodexBuilder { + pub fn with_config(mut self, mutator: T) -> Self + where + T: FnOnce(&mut Config) + 'static, + { + self.config_mutators.push(Box::new(mutator)); + self + } + + pub async fn build(&mut self, server: &wiremock::MockServer) -> anyhow::Result { + // Build config pointing to the mock server and spawn Codex. + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + let home = TempDir::new()?; + let cwd = TempDir::new()?; + let mut config = load_default_config_for_test(&home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + let mut mutators = vec![]; + swap(&mut self.config_mutators, &mut mutators); + + for mutator in mutators { + mutator(&mut config) + } + let conversation_manager = ConversationManager::with_auth(CodexAuth::from_api_key("dummy")); + let NewConversation { + conversation, + session_configured, + .. + } = conversation_manager.new_conversation(config).await?; + + Ok(TestCodex { + home, + cwd, + codex: conversation, + session_configured, + }) + } +} + +pub struct TestCodex { + pub home: TempDir, + pub cwd: TempDir, + pub codex: Arc, + pub session_configured: SessionConfiguredEvent, +} + +pub fn test_codex() -> TestCodexBuilder { + TestCodexBuilder { + config_mutators: vec![], + } +} diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index d885faafee..867accd78d 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -23,6 +23,7 @@ use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; use core_test_support::non_sandbox_test; use core_test_support::responses; +use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use futures::StreamExt; use serde_json::json; @@ -762,9 +763,10 @@ async fn token_count_includes_rate_limits_snapshot() { .insert_header("content-type", "text/event-stream") .insert_header("x-codex-primary-used-percent", "12.5") .insert_header("x-codex-secondary-used-percent", "40.0") - .insert_header("x-codex-primary-over-secondary-limit-percent", "75.0") .insert_header("x-codex-primary-window-minutes", "10") .insert_header("x-codex-secondary-window-minutes", "60") + .insert_header("x-codex-primary-reset-after-seconds", "1800") + .insert_header("x-codex-secondary-reset-after-seconds", "7200") .set_body_raw(sse_body, "text/event-stream"); Mock::given(method("POST")) @@ -797,7 +799,38 @@ async fn token_count_includes_rate_limits_snapshot() { .await .unwrap(); - let token_event = wait_for_event(&codex, |msg| matches!(msg, EventMsg::TokenCount(_))).await; + let first_token_event = + wait_for_event(&codex, |msg| matches!(msg, EventMsg::TokenCount(_))).await; + let rate_limit_only = match first_token_event { + EventMsg::TokenCount(ev) => ev, + _ => unreachable!(), + }; + + let rate_limit_json = serde_json::to_value(&rate_limit_only).unwrap(); + pretty_assertions::assert_eq!( + rate_limit_json, + json!({ + "info": null, + "rate_limits": { + "primary": { + "used_percent": 12.5, + "window_minutes": 10, + "resets_in_seconds": 1800 + }, + "secondary": { + "used_percent": 40.0, + "window_minutes": 60, + "resets_in_seconds": 7200 + } + } + }) + ); + + let token_event = wait_for_event( + &codex, + |msg| matches!(msg, EventMsg::TokenCount(ev) if ev.info.is_some()), + ) + .await; let final_payload = match token_event { EventMsg::TokenCount(ev) => ev, _ => unreachable!(), @@ -826,11 +859,16 @@ async fn token_count_includes_rate_limits_snapshot() { "model_context_window": 272000 }, "rate_limits": { - "primary_used_percent": 12.5, - "secondary_used_percent": 40.0, - "primary_to_secondary_ratio_percent": 75.0, - "primary_window_minutes": 10, - "secondary_window_minutes": 60 + "primary": { + "used_percent": 12.5, + "window_minutes": 10, + "resets_in_seconds": 1800 + }, + "secondary": { + "used_percent": 40.0, + "window_minutes": 60, + "resets_in_seconds": 7200 + } } }) ); @@ -841,11 +879,103 @@ async fn token_count_includes_rate_limits_snapshot() { let final_snapshot = final_payload .rate_limits .expect("latest rate limit snapshot should be retained"); - assert_eq!(final_snapshot.primary_used_percent, 12.5); + assert_eq!( + final_snapshot + .primary + .as_ref() + .map(|window| window.used_percent), + Some(12.5) + ); + assert_eq!( + final_snapshot + .primary + .as_ref() + .and_then(|window| window.resets_in_seconds), + Some(1800) + ); wait_for_event(&codex, |msg| matches!(msg, EventMsg::TaskComplete(_))).await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn usage_limit_error_emits_rate_limit_event() -> anyhow::Result<()> { + let server = MockServer::start().await; + + let response = ResponseTemplate::new(429) + .insert_header("x-codex-primary-used-percent", "100.0") + .insert_header("x-codex-secondary-used-percent", "87.5") + .insert_header("x-codex-primary-over-secondary-limit-percent", "95.0") + .insert_header("x-codex-primary-window-minutes", "15") + .insert_header("x-codex-secondary-window-minutes", "60") + .set_body_json(json!({ + "error": { + "type": "usage_limit_reached", + "message": "limit reached", + "resets_in_seconds": 42, + "plan_type": "pro" + } + })); + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(response) + .expect(1) + .mount(&server) + .await; + + let mut builder = test_codex(); + let codex_fixture = builder.build(&server).await?; + let codex = codex_fixture.codex.clone(); + + let expected_limits = json!({ + "primary": { + "used_percent": 100.0, + "window_minutes": 15, + "resets_in_seconds": null + }, + "secondary": { + "used_percent": 87.5, + "window_minutes": 60, + "resets_in_seconds": null + } + }); + + let submission_id = codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello".into(), + }], + }) + .await + .expect("submission should succeed while emitting usage limit error events"); + + let token_event = wait_for_event(&codex, |msg| matches!(msg, EventMsg::TokenCount(_))).await; + let EventMsg::TokenCount(event) = token_event else { + unreachable!(); + }; + + let event_json = serde_json::to_value(&event).expect("serialize token count event"); + pretty_assertions::assert_eq!( + event_json, + json!({ + "info": null, + "rate_limits": expected_limits + }) + ); + + let error_event = wait_for_event(&codex, |msg| matches!(msg, EventMsg::Error(_))).await; + let EventMsg::Error(error_event) = error_event else { + unreachable!(); + }; + assert!( + error_event.message.to_lowercase().contains("usage limit"), + "unexpected error message for submission {submission_id}: {}", + error_event.message + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn azure_overrides_assign_properties_used_for_responses_url() { let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" }; diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 3cae9841c5..9e456e58c8 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -19,6 +19,7 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; +use codex_core::codex::compact::SUMMARIZATION_PROMPT; use core_test_support::non_sandbox_test; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -37,7 +38,6 @@ use std::sync::atomic::Ordering; pub(super) const FIRST_REPLY: &str = "FIRST_REPLY"; pub(super) const SUMMARY_TEXT: &str = "SUMMARY_ONLY_CONTEXT"; -pub(super) const SUMMARIZE_TRIGGER: &str = "Start Summarization"; const THIRD_USER_MSG: &str = "next turn"; const AUTO_SUMMARY_TEXT: &str = "AUTO_SUMMARY"; const FIRST_AUTO_MSG: &str = "token limit start"; @@ -77,13 +77,13 @@ async fn summarize_context_three_requests_and_instructions() { let first_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains("\"text\":\"hello world\"") - && !body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) + && !body.contains("You have exceeded the maximum number of tokens") }; mount_sse_once(&server, first_matcher, sse1).await; let second_matcher = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); - body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) + body.contains("You have exceeded the maximum number of tokens") }; mount_sse_once(&server, second_matcher, sse2).await; @@ -121,7 +121,7 @@ async fn summarize_context_three_requests_and_instructions() { .unwrap(); wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; - // 2) Summarize – second hit with summarization instructions. + // 2) Summarize – second hit should include the summarization prompt. codex.submit(Op::Compact).await.unwrap(); wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; @@ -148,16 +148,12 @@ async fn summarize_context_three_requests_and_instructions() { let body2 = req2.body_json::().unwrap(); let body3 = req3.body_json::().unwrap(); - // System instructions should change for the summarization turn. + // Manual compact should keep the baseline developer instructions. let instr1 = body1.get("instructions").and_then(|v| v.as_str()).unwrap(); let instr2 = body2.get("instructions").and_then(|v| v.as_str()).unwrap(); - assert_ne!( + assert_eq!( instr1, instr2, - "summarization should override base instructions" - ); - assert!( - instr2.contains("You have exceeded the maximum number of tokens"), - "summarization instructions not applied" + "manual compact should keep the standard developer instructions" ); // The summarization request should include the injected user input marker. @@ -167,14 +163,14 @@ async fn summarize_context_three_requests_and_instructions() { assert_eq!(last2.get("type").unwrap().as_str().unwrap(), "message"); assert_eq!(last2.get("role").unwrap().as_str().unwrap(), "user"); let text2 = last2["content"][0]["text"].as_str().unwrap(); - assert!( - text2.contains(SUMMARIZE_TRIGGER), + assert_eq!( + text2, SUMMARIZATION_PROMPT, "expected summarize trigger, got `{text2}`" ); // Third request must contain the refreshed instructions, bridge summary message and new user msg. let input3 = body3.get("input").and_then(|v| v.as_array()).unwrap(); - println!("third request body: {body3}"); + assert!( input3.len() >= 3, "expected refreshed context and new user message in third request" @@ -215,13 +211,13 @@ async fn summarize_context_three_requests_and_instructions() { "bridge should capture earlier user messages" ); assert!( - !bridge_text.contains(SUMMARIZE_TRIGGER), + !bridge_text.contains(SUMMARIZATION_PROMPT), "bridge text should not echo the summarize trigger" ); assert!( !messages .iter() - .any(|(_, text)| text.contains(SUMMARIZE_TRIGGER)), + .any(|(_, text)| text.contains(SUMMARIZATION_PROMPT)), "third request should not include the summarize trigger" ); @@ -395,6 +391,7 @@ async fn auto_compact_runs_after_token_limit_hit() { "auto compact should add a third request" ); + let body_first = requests[0].body_json::().unwrap(); let body3 = requests[auto_compact_index] .body_json::() .unwrap(); @@ -402,9 +399,32 @@ async fn auto_compact_runs_after_token_limit_hit() { .get("instructions") .and_then(|v| v.as_str()) .unwrap_or_default(); - assert!( - instructions.contains("You have exceeded the maximum number of tokens"), - "auto compact should reuse summarization instructions" + let baseline_instructions = body_first + .get("instructions") + .and_then(|v| v.as_str()) + .unwrap_or_default() + .to_string(); + assert_eq!( + instructions, baseline_instructions, + "auto compact should keep the standard developer instructions", + ); + + let input3 = body3.get("input").and_then(|v| v.as_array()).unwrap(); + let last3 = input3 + .last() + .expect("auto compact request should append a user message"); + assert_eq!(last3.get("type").and_then(|v| v.as_str()), Some("message")); + assert_eq!(last3.get("role").and_then(|v| v.as_str()), Some("user")); + let last_text = last3 + .get("content") + .and_then(|v| v.as_array()) + .and_then(|items| items.first()) + .and_then(|item| item.get("text")) + .and_then(|text| text.as_str()) + .unwrap_or_default(); + assert_eq!( + last_text, SUMMARIZATION_PROMPT, + "auto compact should send the summarization prompt as a user message", ); } @@ -635,13 +655,25 @@ async fn auto_compact_stops_after_failed_attempt() { ); let last_body = requests[2].body_json::().unwrap(); - let instructions = last_body - .get("instructions") - .and_then(|v| v.as_str()) - .unwrap_or_default(); + let input = last_body + .get("input") + .and_then(|v| v.as_array()) + .unwrap_or_else(|| panic!("unexpected request format: {last_body}")); + let contains_prompt = input.iter().any(|item| { + item.get("type").and_then(|v| v.as_str()) == Some("message") + && item.get("role").and_then(|v| v.as_str()) == Some("user") + && item + .get("content") + .and_then(|v| v.as_array()) + .and_then(|items| items.first()) + .and_then(|entry| entry.get("text")) + .and_then(|text| text.as_str()) + .map(|text| text == SUMMARIZATION_PROMPT) + .unwrap_or(false) + }); assert!( - !instructions.contains("You have exceeded the maximum number of tokens"), - "third request should be the follow-up turn, not another summarization" + !contains_prompt, + "third request should be the follow-up turn, not another summarization", ); } @@ -785,7 +817,7 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ ); assert!( request_bodies[1].contains("You have exceeded the maximum number of tokens"), - "first auto compact request should use summarization instructions" + "first auto compact request should include the summarization prompt" ); assert!( request_bodies[3].contains(&format!("unsupported call: {DUMMY_FUNCTION_NAME}")), @@ -793,6 +825,6 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_ ); assert!( request_bodies[4].contains("You have exceeded the maximum number of tokens"), - "second auto compact request should reuse summarization instructions" + "second auto compact request should include the summarization prompt" ); } diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 43d6192f5f..73ad091fc9 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -8,7 +8,6 @@ //! model-visible history matches the expected sequence of messages. use super::compact::FIRST_REPLY; -use super::compact::SUMMARIZE_TRIGGER; use super::compact::SUMMARY_TEXT; use codex_core::CodexAuth; use codex_core::CodexConversation; @@ -16,6 +15,7 @@ use codex_core::ConversationManager; use codex_core::ModelProviderInfo; use codex_core::NewConversation; use codex_core::built_in_model_providers; +use codex_core::codex::compact::SUMMARIZATION_PROMPT; use codex_core::config::Config; use codex_core::protocol::ConversationPathResponseEvent; use codex_core::protocol::EventMsg; @@ -183,11 +183,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { let compact_1 = json!( { "model": "gpt-5-codex", - "instructions": "You have exceeded the maximum number of tokens, please stop coding and instead write a short memento message for the next agent. Your note should: -- Summarize what you finished and what still needs work. If there was a recent update_plan call, repeat its steps verbatim. -- List outstanding TODOs with file paths / line numbers so they're easy to find. -- Flag code that needs more tests (edge cases, performance, integration, etc.). -- Record any open bugs, quirks, or setup steps that will make it easier for the next agent to pick up where you left off.", + "instructions": prompt, "input": [ { "type": "message", @@ -235,7 +231,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "content": [ { "type": "input_text", - "text": "Start Summarization" + "text": SUMMARIZATION_PROMPT } ] } @@ -488,13 +484,14 @@ SUMMARY_ONLY_CONTEXT" ], "prompt_cache_key": fork_prompt_cache_key }); - let expected = json!([ + let mut expected = json!([ user_turn_1, compact_1, user_turn_2_after_compact, usert_turn_3_after_resume, user_turn_3_after_fork ]); + normalize_line_endings(&mut expected); assert_eq!(requests.len(), 5); assert_eq!(json!(requests), expected); } @@ -580,7 +577,7 @@ async fn compact_resume_after_second_compaction_preserves_history() { .unwrap_or_default() .to_string(); - let expected = json!([ + let mut expected = json!([ { "instructions": prompt, "input": [ @@ -637,6 +634,7 @@ async fn compact_resume_after_second_compaction_preserves_history() { ], } ]); + normalize_line_endings(&mut expected); let last_request_after_2_compacts = json!([{ "instructions": requests[requests.len() -1]["instructions"], "input": requests[requests.len() -1]["input"], @@ -698,7 +696,8 @@ async fn mount_initial_flow(server: &MockServer) { let match_first = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); body.contains("\"text\":\"hello world\"") - && !body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) + && !body.contains("You have exceeded the maximum number of tokens") + && !body.contains(&format!("\"text\":\"{SUMMARY_TEXT}\"")) && !body.contains("\"text\":\"AFTER_COMPACT\"") && !body.contains("\"text\":\"AFTER_RESUME\"") && !body.contains("\"text\":\"AFTER_FORK\"") @@ -707,7 +706,7 @@ async fn mount_initial_flow(server: &MockServer) { let match_compact = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); - body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) + body.contains("You have exceeded the maximum number of tokens") }; mount_sse_once(server, match_compact, sse2).await; @@ -741,7 +740,8 @@ async fn mount_second_compact_flow(server: &MockServer) { let match_second_compact = |req: &wiremock::Request| { let body = std::str::from_utf8(&req.body).unwrap_or(""); - body.contains(&format!("\"text\":\"{SUMMARIZE_TRIGGER}\"")) && body.contains("AFTER_FORK") + body.contains("You have exceeded the maximum number of tokens") + && body.contains("AFTER_FORK") }; mount_sse_once(server, match_second_compact, sse6).await; diff --git a/codex-rs/core/tests/suite/json_result.rs b/codex-rs/core/tests/suite/json_result.rs new file mode 100644 index 0000000000..86da33f7a0 --- /dev/null +++ b/codex-rs/core/tests/suite/json_result.rs @@ -0,0 +1,106 @@ +#![cfg(not(target_os = "windows"))] + +use codex_core::protocol::AskForApproval; +use codex_core::protocol::EventMsg; +use codex_core::protocol::InputItem; +use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::ReasoningSummary; +use core_test_support::non_sandbox_test; +use core_test_support::responses; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use responses::ev_assistant_message; +use responses::ev_completed; +use responses::sse; +use responses::start_mock_server; + +const SCHEMA: &str = r#" +{ + "type": "object", + "properties": { + "explanation": { "type": "string" }, + "final_answer": { "type": "string" } + }, + "required": ["explanation", "final_answer"], + "additionalProperties": false +} +"#; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn codex_returns_json_result_for_gpt5() -> anyhow::Result<()> { + codex_returns_json_result("gpt-5".to_string()).await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn codex_returns_json_result_for_gpt5_codex() -> anyhow::Result<()> { + codex_returns_json_result("gpt-5-codex".to_string()).await +} + +async fn codex_returns_json_result(model: String) -> anyhow::Result<()> { + non_sandbox_test!(result); + + let server = start_mock_server().await; + + let sse1 = sse(vec![ + ev_assistant_message( + "m2", + r#"{"explanation": "explanation", "final_answer": "final_answer"}"#, + ), + ev_completed("r1"), + ]); + + let expected_schema: serde_json::Value = serde_json::from_str(SCHEMA)?; + let match_json_text_param = move |req: &wiremock::Request| { + let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap_or_default(); + let Some(text) = body.get("text") else { + return false; + }; + let Some(format) = text.get("format") else { + return false; + }; + + format.get("name") == Some(&serde_json::Value::String("codex_output_schema".into())) + && format.get("type") == Some(&serde_json::Value::String("json_schema".into())) + && format.get("strict") == Some(&serde_json::Value::Bool(true)) + && format.get("schema") == Some(&expected_schema) + }; + responses::mount_sse_once(&server, match_json_text_param, sse1).await; + + let TestCodex { codex, cwd, .. } = test_codex().build(&server).await?; + + // 1) Normal user input – should hit server once. + codex + .submit(Op::UserTurn { + items: vec![InputItem::Text { + text: "hello world".into(), + }], + final_output_json_schema: Some(serde_json::from_str(SCHEMA)?), + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let message = wait_for_event(&codex, |ev| matches!(ev, EventMsg::AgentMessage(_))).await; + if let EventMsg::AgentMessage(message) = message { + let json: serde_json::Value = serde_json::from_str(&message.message)?; + assert_eq!( + json.get("explanation"), + Some(&serde_json::Value::String("explanation".into())) + ); + assert_eq!( + json.get("final_answer"), + Some(&serde_json::Value::String("final_answer".into())) + ); + } else { + anyhow::bail!("expected agent message event"); + } + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 4c217b63f4..2d91e330a8 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -7,6 +7,7 @@ mod compact_resume_fork; mod exec; mod exec_stream_events; mod fork_conversation; +mod json_result; mod live_cli; mod model_overrides; mod prompt_caching; @@ -15,3 +16,4 @@ mod rollout_list_find; mod seatbelt; mod stream_error_allows_next_turn; mod stream_no_completed; +mod user_notification; diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 6cfe6f4d78..147cf27d34 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -546,6 +546,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() { model: "o3".to_string(), effort: Some(ReasoningEffort::High), summary: ReasoningSummary::Detailed, + final_output_json_schema: None, }) .await .unwrap(); @@ -655,6 +656,7 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() { model: default_model.clone(), effort: default_effort, summary: default_summary, + final_output_json_schema: None, }) .await .unwrap(); @@ -671,6 +673,7 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() { model: default_model.clone(), effort: default_effort, summary: default_summary, + final_output_json_schema: None, }) .await .unwrap(); @@ -766,6 +769,7 @@ async fn send_user_turn_with_changes_sends_environment_context() { model: default_model, effort: default_effort, summary: default_summary, + final_output_json_schema: None, }) .await .unwrap(); @@ -782,6 +786,7 @@ async fn send_user_turn_with_changes_sends_environment_context() { model: "o3".to_string(), effort: Some(ReasoningEffort::High), summary: ReasoningSummary::Detailed, + final_output_json_schema: None, }) .await .unwrap(); diff --git a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs index f41534ab75..14bff33406 100644 --- a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs +++ b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs @@ -1,17 +1,15 @@ use std::time::Duration; -use codex_core::CodexAuth; -use codex_core::ConversationManager; use codex_core::ModelProviderInfo; use codex_core::WireApi; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; -use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture_with_id; use core_test_support::non_sandbox_test; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event_with_timeout; -use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -78,18 +76,14 @@ async fn continue_after_stream_error() { requires_openai_auth: false, }; - let home = TempDir::new().unwrap(); - let mut config = load_default_config_for_test(&home); - config.base_instructions = Some("You are a helpful assistant".to_string()); - config.model_provider = provider; - - let conversation_manager = - ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key")); - let codex = conversation_manager - .new_conversation(config) + let TestCodex { codex, .. } = test_codex() + .with_config(move |config| { + config.base_instructions = Some("You are a helpful assistant".to_string()); + config.model_provider = provider; + }) + .build(&server) .await - .unwrap() - .conversation; + .unwrap(); codex .submit(Op::UserInput { diff --git a/codex-rs/core/tests/suite/stream_no_completed.rs b/codex-rs/core/tests/suite/stream_no_completed.rs index 4801376df1..db55c7a450 100644 --- a/codex-rs/core/tests/suite/stream_no_completed.rs +++ b/codex-rs/core/tests/suite/stream_no_completed.rs @@ -3,17 +3,16 @@ use std::time::Duration; -use codex_core::CodexAuth; -use codex_core::ConversationManager; use codex_core::ModelProviderInfo; +use codex_core::WireApi; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; -use core_test_support::load_default_config_for_test; use core_test_support::load_sse_fixture; use core_test_support::load_sse_fixture_with_id; use core_test_support::non_sandbox_test; -use tempfile::TempDir; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; use tokio::time::timeout; use wiremock::Mock; use wiremock::MockServer; @@ -74,7 +73,7 @@ async fn retries_on_early_close() { // provider is not set. env_key: Some("PATH".into()), env_key_instructions: None, - wire_api: codex_core::WireApi::Responses, + wire_api: WireApi::Responses, query_params: None, http_headers: None, env_http_headers: None, @@ -85,16 +84,13 @@ async fn retries_on_early_close() { requires_openai_auth: false, }; - let codex_home = TempDir::new().unwrap(); - let mut config = load_default_config_for_test(&codex_home); - config.model_provider = model_provider; - let conversation_manager = - ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key")); - let codex = conversation_manager - .new_conversation(config) + let TestCodex { codex, .. } = test_codex() + .with_config(move |config| { + config.model_provider = model_provider; + }) + .build(&server) .await - .unwrap() - .conversation; + .unwrap(); codex .submit(Op::UserInput { diff --git a/codex-rs/core/tests/suite/user_notification.rs b/codex-rs/core/tests/suite/user_notification.rs new file mode 100644 index 0000000000..c7acf35ecd --- /dev/null +++ b/codex-rs/core/tests/suite/user_notification.rs @@ -0,0 +1,73 @@ +#![cfg(not(target_os = "windows"))] + +use std::os::unix::fs::PermissionsExt; + +use codex_core::protocol::EventMsg; +use codex_core::protocol::InputItem; +use codex_core::protocol::Op; +use core_test_support::non_sandbox_test; +use core_test_support::responses; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use tempfile::TempDir; +use wiremock::matchers::any; + +use responses::ev_assistant_message; +use responses::ev_completed; +use responses::sse; +use responses::start_mock_server; +use tokio::time::Duration; +use tokio::time::sleep; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn summarize_context_three_requests_and_instructions() -> anyhow::Result<()> { + non_sandbox_test!(result); + + let server = start_mock_server().await; + + let sse1 = sse(vec![ev_assistant_message("m1", "Done"), ev_completed("r1")]); + + responses::mount_sse_once(&server, any(), sse1).await; + + let notify_dir = TempDir::new()?; + // write a script to the notify that touches a file next to it + let notify_script = notify_dir.path().join("notify.sh"); + std::fs::write( + ¬ify_script, + r#"#!/bin/bash +set -e +echo -n "${@: -1}" > $(dirname "${0}")/notify.txt"#, + )?; + std::fs::set_permissions(¬ify_script, std::fs::Permissions::from_mode(0o755))?; + + let notify_file = notify_dir.path().join("notify.txt"); + let notify_script_str = notify_script.to_str().unwrap().to_string(); + + let TestCodex { codex, .. } = test_codex() + .with_config(move |cfg| cfg.notify = Some(vec![notify_script_str])) + .build(&server) + .await?; + + // 1) Normal user input – should hit server once. + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello world".into(), + }], + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // We fork the notify script, so we need to wait for it to write to the file. + for _ in 0..100u32 { + if notify_file.exists() { + break; + } + sleep(Duration::from_millis(100)).await; + } + + assert!(notify_file.exists()); + + Ok(()) +} diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 19093ec925..2f5fe4fb1c 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -52,6 +52,10 @@ pub struct Cli { #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, + /// Path to a JSON Schema file describing the model's final response shape. + #[arg(long = "output-schema", value_name = "FILE")] + pub output_schema: Option, + #[clap(skip)] pub config_overrides: CliConfigOverrides, @@ -63,6 +67,10 @@ pub struct Cli { #[arg(long = "json", default_value_t = false)] pub json: bool, + /// Whether to include the plan tool in the conversation. + #[arg(long = "include-plan-tool", default_value_t = false)] + pub include_plan_tool: bool, + /// Specifies file where the last message from the agent should be written. #[arg(long = "output-last-message")] pub last_message_file: Option, diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index b22d4b7289..cb972bf93e 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -539,8 +539,37 @@ impl EventProcessor for EventProcessorWithHumanOutput { } EventMsg::PlanUpdate(plan_update_event) => { let UpdatePlanArgs { explanation, plan } = plan_update_event; - ts_println!(self, "explanation: {explanation:?}"); - ts_println!(self, "plan: {plan:?}"); + + // Header + ts_println!(self, "{}", "Plan update".style(self.magenta)); + + // Optional explanation + if let Some(explanation) = explanation + && !explanation.trim().is_empty() + { + ts_println!(self, "{}", explanation.style(self.italic)); + } + + // Pretty-print the plan items with simple status markers. + for item in plan { + use codex_core::plan_tool::StepStatus; + match item.status { + StepStatus::Completed => { + ts_println!(self, " {} {}", "✓".style(self.green), item.step); + } + StepStatus::InProgress => { + ts_println!(self, " {} {}", "→".style(self.cyan), item.step); + } + StepStatus::Pending => { + ts_println!( + self, + " {} {}", + "•".style(self.dimmed), + item.step.style(self.dimmed) + ); + } + } + } } EventMsg::GetHistoryEntryResponse(_) => { // Currently ignored in exec output. diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 097231da60..9c561cfb1f 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -25,6 +25,7 @@ use codex_ollama::DEFAULT_OSS_MODEL; use codex_protocol::config_types::SandboxMode; use event_processor_with_human_output::EventProcessorWithHumanOutput; use event_processor_with_json_output::EventProcessorWithJsonOutput; +use serde_json::Value; use tracing::debug; use tracing::error; use tracing::info; @@ -51,6 +52,8 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any json: json_mode, sandbox_mode: sandbox_mode_cli_arg, prompt, + output_schema: output_schema_path, + include_plan_tool, config_overrides, } = cli; @@ -96,6 +99,8 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any } }; + let output_schema = load_output_schema(output_schema_path); + let (stdout_with_ansi, stderr_with_ansi) = match color { cli::Color::Always => (true, true), cli::Color::Never => (false, false), @@ -157,7 +162,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any model_provider, codex_linux_sandbox_exe, base_instructions: None, - include_plan_tool: None, + include_plan_tool: Some(include_plan_tool), include_apply_patch_tool: None, include_view_image_tool: None, show_raw_agent_reasoning: oss.then_some(true), @@ -193,7 +198,14 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any // is using. event_processor.print_config_summary(&config, &prompt); - if !skip_git_repo_check && get_git_repo_root(&config.cwd.to_path_buf()).is_none() { + let default_cwd = config.cwd.to_path_buf(); + let default_approval_policy = config.approval_policy; + let default_sandbox_policy = config.sandbox_policy.clone(); + let default_model = config.model.clone(); + let default_effort = config.model_reasoning_effort; + let default_summary = config.model_reasoning_summary; + + if !skip_git_repo_check && get_git_repo_root(&default_cwd).is_none() { eprintln!("Not inside a trusted directory and --skip-git-repo-check was not specified."); std::process::exit(1); } @@ -288,7 +300,18 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any // Send the prompt. let items: Vec = vec![InputItem::Text { text: prompt }]; - let initial_prompt_task_id = conversation.submit(Op::UserInput { items }).await?; + let initial_prompt_task_id = conversation + .submit(Op::UserTurn { + items, + cwd: default_cwd, + approval_policy: default_approval_policy, + sandbox_policy: default_sandbox_policy, + model: default_model, + effort: default_effort, + summary: default_summary, + final_output_json_schema: output_schema, + }) + .await?; info!("Sent prompt with event ID: {initial_prompt_task_id}"); // Run the loop until the task is complete. @@ -327,3 +350,29 @@ async fn resolve_resume_path( Ok(None) } } + +fn load_output_schema(path: Option) -> Option { + let path = path?; + + let schema_str = match std::fs::read_to_string(&path) { + Ok(contents) => contents, + Err(err) => { + eprintln!( + "Failed to read output schema file {}: {err}", + path.display() + ); + std::process::exit(1); + } + }; + + match serde_json::from_str::(&schema_str) { + Ok(value) => Some(value), + Err(err) => { + eprintln!( + "Output schema file {} is not valid JSON: {err}", + path.display() + ); + std::process::exit(1); + } + } +} diff --git a/codex-rs/exec/tests/suite/mod.rs b/codex-rs/exec/tests/suite/mod.rs index 5748fba1e8..758f39ed71 100644 --- a/codex-rs/exec/tests/suite/mod.rs +++ b/codex-rs/exec/tests/suite/mod.rs @@ -1,5 +1,6 @@ // Aggregates all former standalone integration tests as modules. mod apply_patch; mod common; +mod output_schema; mod resume; mod sandbox; diff --git a/codex-rs/exec/tests/suite/output_schema.rs b/codex-rs/exec/tests/suite/output_schema.rs new file mode 100644 index 0000000000..954e12587c --- /dev/null +++ b/codex-rs/exec/tests/suite/output_schema.rs @@ -0,0 +1,76 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::expect_used, clippy::unwrap_used)] + +use assert_cmd::prelude::*; +use core_test_support::responses; +use serde_json::Value; +use std::process::Command; +use tempfile::TempDir; +use wiremock::matchers::any; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_includes_output_schema_in_request() -> anyhow::Result<()> { + let home = TempDir::new()?; + let workspace = TempDir::new()?; + + let schema_contents = serde_json::json!({ + "type": "object", + "properties": { + "answer": { "type": "string" } + }, + "required": ["answer"], + "additionalProperties": false + }); + let schema_path = workspace.path().join("schema.json"); + std::fs::write(&schema_path, serde_json::to_vec_pretty(&schema_contents)?)?; + let expected_schema: Value = schema_contents; + + let server = responses::start_mock_server().await; + let body = responses::sse(vec![ + serde_json::json!({ + "type": "response.created", + "response": {"id": "resp1"} + }), + responses::ev_assistant_message("m1", "fixture hello"), + responses::ev_completed("resp1"), + ]); + responses::mount_sse_once(&server, any(), body).await; + + Command::cargo_bin("codex-exec")? + .current_dir(workspace.path()) + .env("CODEX_HOME", home.path()) + .env("OPENAI_API_KEY", "dummy") + .env("OPENAI_BASE_URL", format!("{}/v1", server.uri())) + .arg("--skip-git-repo-check") + .arg("-C") + .arg(workspace.path()) + .arg("--output-schema") + .arg(&schema_path) + .arg("-m") + .arg("gpt-5") + .arg("tell me a joke") + .assert() + .success(); + + let requests = server + .received_requests() + .await + .expect("failed to capture requests"); + assert_eq!(requests.len(), 1, "expected exactly one request"); + let payload: Value = serde_json::from_slice(&requests[0].body)?; + let text = payload.get("text").expect("request missing text field"); + let format = text + .get("format") + .expect("request missing text.format field"); + assert_eq!( + format, + &serde_json::json!({ + "name": "codex_output_schema", + "type": "json_schema", + "strict": true, + "schema": expected_schema, + }) + ); + + Ok(()) +} diff --git a/codex-rs/execpolicy/src/execv_checker.rs b/codex-rs/execpolicy/src/execv_checker.rs index 1c03f591b1..e84b92cac0 100644 --- a/codex-rs/execpolicy/src/execv_checker.rs +++ b/codex-rs/execpolicy/src/execv_checker.rs @@ -146,6 +146,8 @@ mod tests { use super::*; use crate::MatchedArg; use crate::PolicyParser; + use anyhow::Result; + use anyhow::anyhow; fn setup(fake_cp: &Path) -> ExecvChecker { let source = format!( @@ -164,7 +166,7 @@ system_path=[{fake_cp:?}] #[test] fn test_check_valid_input_files() -> Result<()> { - let temp_dir = TempDir::new().unwrap(); + let temp_dir = TempDir::new()?; // Create an executable file that can be used with the system_path arg. let fake_cp = temp_dir.path().join("cp"); @@ -172,14 +174,14 @@ system_path=[{fake_cp:?}] { use std::os::unix::fs::PermissionsExt; - let fake_cp_file = std::fs::File::create(&fake_cp).unwrap(); - let mut permissions = fake_cp_file.metadata().unwrap().permissions(); + let fake_cp_file = std::fs::File::create(&fake_cp)?; + let mut permissions = fake_cp_file.metadata()?.permissions(); permissions.set_mode(0o755); - std::fs::set_permissions(&fake_cp, permissions).unwrap(); + std::fs::set_permissions(&fake_cp, permissions)?; } #[cfg(windows)] { - std::fs::File::create(&fake_cp).unwrap(); + std::fs::File::create(&fake_cp)?; } // Create root_path and reference to files under the root. @@ -199,7 +201,7 @@ system_path=[{fake_cp:?}] program: "cp".into(), args: vec![source, dest.clone()], }; - let valid_exec = match checker.r#match(&exec_call)? { + let valid_exec = match checker.r#match(&exec_call).map_err(|e| anyhow!("{e:?}"))? { MatchedExec::Match { exec } => exec, unexpected => panic!("Expected a safe exec but got {unexpected:?}"), }; @@ -244,7 +246,10 @@ system_path=[{fake_cp:?}] program: "cp".into(), args: vec![root.clone(), root], }; - let valid_exec_call_folders_as_args = match checker.r#match(&exec_call_folders_as_args)? { + let valid_exec_call_folders_as_args = match checker + .r#match(&exec_call_folders_as_args) + .map_err(|e| anyhow!("{e:?}"))? + { MatchedExec::Match { exec } => exec, _ => panic!("Expected a safe exec"), }; @@ -266,8 +271,9 @@ system_path=[{fake_cp:?}] 0, ArgType::ReadableFile, root_path.parent().unwrap().to_str().unwrap(), - )?, - MatchedArg::new(1, ArgType::WriteableFile, &dest)?, + ) + .map_err(|e| anyhow!("{e:?}"))?, + MatchedArg::new(1, ArgType::WriteableFile, &dest).map_err(|e| anyhow!("{e:?}"))?, ], ..Default::default() }; diff --git a/codex-rs/git-tooling/Cargo.toml b/codex-rs/git-tooling/Cargo.toml new file mode 100644 index 0000000000..674a5e527f --- /dev/null +++ b/codex-rs/git-tooling/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "codex-git-tooling" +version.workspace = true +edition.workspace = true +readme = "README.md" + +[lib] +name = "codex_git_tooling" +path = "src/lib.rs" + +[dependencies] +tempfile = "3" +thiserror = "2" +walkdir = "2" + +[lints] +workspace = true + +[dev-dependencies] +pretty_assertions = "1.4.1" diff --git a/codex-rs/git-tooling/README.md b/codex-rs/git-tooling/README.md new file mode 100644 index 0000000000..8833fc5850 --- /dev/null +++ b/codex-rs/git-tooling/README.md @@ -0,0 +1,20 @@ +# codex-git-tooling + +Helpers for interacting with git. + +```rust,no_run +use std::path::Path; + +use codex_git_tooling::{create_ghost_commit, restore_ghost_commit, CreateGhostCommitOptions}; + +let repo = Path::new("/path/to/repo"); + +// Capture the current working tree as an unreferenced commit. +let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + +// Later, undo back to that state. +restore_ghost_commit(repo, &ghost)?; +``` + +Pass a custom message with `.message("…")` or force-include ignored files with +`.force_include(["ignored.log".into()])`. diff --git a/codex-rs/git-tooling/src/errors.rs b/codex-rs/git-tooling/src/errors.rs new file mode 100644 index 0000000000..41f7b6df49 --- /dev/null +++ b/codex-rs/git-tooling/src/errors.rs @@ -0,0 +1,35 @@ +use std::path::PathBuf; +use std::process::ExitStatus; +use std::string::FromUtf8Error; + +use thiserror::Error; +use walkdir::Error as WalkdirError; + +/// Errors returned while managing git worktree snapshots. +#[derive(Debug, Error)] +pub enum GitToolingError { + #[error("git command `{command}` failed with status {status}: {stderr}")] + GitCommand { + command: String, + status: ExitStatus, + stderr: String, + }, + #[error("git command `{command}` produced non-UTF-8 output")] + GitOutputUtf8 { + command: String, + #[source] + source: FromUtf8Error, + }, + #[error("{path:?} is not a git repository")] + NotAGitRepository { path: PathBuf }, + #[error("path {path:?} must be relative to the repository root")] + NonRelativePath { path: PathBuf }, + #[error("path {path:?} escapes the repository root")] + PathEscapesRepository { path: PathBuf }, + #[error("failed to process path inside worktree")] + PathPrefix(#[from] std::path::StripPrefixError), + #[error(transparent)] + Walkdir(#[from] WalkdirError), + #[error(transparent)] + Io(#[from] std::io::Error), +} diff --git a/codex-rs/git-tooling/src/ghost_commits.rs b/codex-rs/git-tooling/src/ghost_commits.rs new file mode 100644 index 0000000000..06b4321122 --- /dev/null +++ b/codex-rs/git-tooling/src/ghost_commits.rs @@ -0,0 +1,494 @@ +use std::ffi::OsString; +use std::path::Path; +use std::path::PathBuf; + +use tempfile::Builder; + +use crate::GhostCommit; +use crate::GitToolingError; +use crate::operations::apply_repo_prefix_to_force_include; +use crate::operations::ensure_git_repository; +use crate::operations::normalize_relative_path; +use crate::operations::repo_subdir; +use crate::operations::resolve_head; +use crate::operations::resolve_repository_root; +use crate::operations::run_git_for_status; +use crate::operations::run_git_for_stdout; + +/// Default commit message used for ghost commits when none is provided. +const DEFAULT_COMMIT_MESSAGE: &str = "codex snapshot"; + +/// Options to control ghost commit creation. +pub struct CreateGhostCommitOptions<'a> { + pub repo_path: &'a Path, + pub message: Option<&'a str>, + pub force_include: Vec, +} + +impl<'a> CreateGhostCommitOptions<'a> { + /// Creates options scoped to the provided repository path. + pub fn new(repo_path: &'a Path) -> Self { + Self { + repo_path, + message: None, + force_include: Vec::new(), + } + } + + /// Sets a custom commit message for the ghost commit. + pub fn message(mut self, message: &'a str) -> Self { + self.message = Some(message); + self + } + + /// Supplies the entire force-include path list at once. + pub fn force_include(mut self, paths: I) -> Self + where + I: IntoIterator, + { + self.force_include = paths.into_iter().collect(); + self + } + + /// Adds a single path to the force-include list. + pub fn push_force_include

(mut self, path: P) -> Self + where + P: Into, + { + self.force_include.push(path.into()); + self + } +} + +/// Create a ghost commit capturing the current state of the repository's working tree. +pub fn create_ghost_commit( + options: &CreateGhostCommitOptions<'_>, +) -> Result { + ensure_git_repository(options.repo_path)?; + + let repo_root = resolve_repository_root(options.repo_path)?; + let repo_prefix = repo_subdir(repo_root.as_path(), options.repo_path); + let parent = resolve_head(repo_root.as_path())?; + + let normalized_force = options + .force_include + .iter() + .map(|path| normalize_relative_path(path)) + .collect::, _>>()?; + let force_include = + apply_repo_prefix_to_force_include(repo_prefix.as_deref(), &normalized_force); + let index_tempdir = Builder::new().prefix("codex-git-index-").tempdir()?; + let index_path = index_tempdir.path().join("index"); + let base_env = vec![( + OsString::from("GIT_INDEX_FILE"), + OsString::from(index_path.as_os_str()), + )]; + + let mut add_args = vec![OsString::from("add"), OsString::from("--all")]; + if let Some(prefix) = repo_prefix.as_deref() { + add_args.extend([OsString::from("--"), prefix.as_os_str().to_os_string()]); + } + + run_git_for_status(repo_root.as_path(), add_args, Some(base_env.as_slice()))?; + if !force_include.is_empty() { + let mut args = Vec::with_capacity(force_include.len() + 2); + args.push(OsString::from("add")); + args.push(OsString::from("--force")); + args.extend( + force_include + .iter() + .map(|path| OsString::from(path.as_os_str())), + ); + run_git_for_status(repo_root.as_path(), args, Some(base_env.as_slice()))?; + } + + let tree_id = run_git_for_stdout( + repo_root.as_path(), + vec![OsString::from("write-tree")], + Some(base_env.as_slice()), + )?; + + let mut commit_env = base_env; + commit_env.extend(default_commit_identity()); + let message = options.message.unwrap_or(DEFAULT_COMMIT_MESSAGE); + let commit_args = { + let mut result = vec![OsString::from("commit-tree"), OsString::from(&tree_id)]; + if let Some(parent) = parent.as_deref() { + result.extend([OsString::from("-p"), OsString::from(parent)]); + } + result.extend([OsString::from("-m"), OsString::from(message)]); + result + }; + + // Retrieve commit ID. + let commit_id = run_git_for_stdout( + repo_root.as_path(), + commit_args, + Some(commit_env.as_slice()), + )?; + + Ok(GhostCommit::new(commit_id, parent)) +} + +/// Restore the working tree to match the provided ghost commit. +pub fn restore_ghost_commit(repo_path: &Path, commit: &GhostCommit) -> Result<(), GitToolingError> { + restore_to_commit(repo_path, commit.id()) +} + +/// Restore the working tree to match the given commit ID. +pub fn restore_to_commit(repo_path: &Path, commit_id: &str) -> Result<(), GitToolingError> { + ensure_git_repository(repo_path)?; + + let repo_root = resolve_repository_root(repo_path)?; + let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); + + let mut restore_args = vec![ + OsString::from("restore"), + OsString::from("--source"), + OsString::from(commit_id), + OsString::from("--worktree"), + OsString::from("--staged"), + OsString::from("--"), + ]; + if let Some(prefix) = repo_prefix.as_deref() { + restore_args.push(prefix.as_os_str().to_os_string()); + } else { + restore_args.push(OsString::from(".")); + } + + run_git_for_status(repo_root.as_path(), restore_args, None)?; + Ok(()) +} + +/// Returns the default author and committer identity for ghost commits. +fn default_commit_identity() -> Vec<(OsString, OsString)> { + vec![ + ( + OsString::from("GIT_AUTHOR_NAME"), + OsString::from("Codex Snapshot"), + ), + ( + OsString::from("GIT_AUTHOR_EMAIL"), + OsString::from("snapshot@codex.local"), + ), + ( + OsString::from("GIT_COMMITTER_NAME"), + OsString::from("Codex Snapshot"), + ), + ( + OsString::from("GIT_COMMITTER_EMAIL"), + OsString::from("snapshot@codex.local"), + ), + ] +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::operations::run_git_for_stdout; + use pretty_assertions::assert_eq; + use std::process::Command; + + /// Runs a git command in the test repository and asserts success. + fn run_git_in(repo_path: &Path, args: &[&str]) { + let status = Command::new("git") + .current_dir(repo_path) + .args(args) + .status() + .expect("git command"); + assert!(status.success(), "git command failed: {args:?}"); + } + + /// Runs a git command and returns its trimmed stdout output. + fn run_git_stdout(repo_path: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .current_dir(repo_path) + .args(args) + .output() + .expect("git command"); + assert!(output.status.success(), "git command failed: {args:?}"); + String::from_utf8_lossy(&output.stdout).trim().to_string() + } + + /// Initializes a repository with consistent settings for cross-platform tests. + fn init_test_repo(repo: &Path) { + run_git_in(repo, &["init", "--initial-branch=main"]); + run_git_in(repo, &["config", "core.autocrlf", "false"]); + } + + #[test] + /// Verifies a ghost commit can be created and restored end to end. + fn create_and_restore_roundtrip() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + std::fs::write(repo.join("tracked.txt"), "initial\n")?; + std::fs::write(repo.join("delete-me.txt"), "to be removed\n")?; + run_git_in(repo, &["add", "tracked.txt", "delete-me.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "init", + ], + ); + + let tracked_contents = "modified contents\n"; + std::fs::write(repo.join("tracked.txt"), tracked_contents)?; + std::fs::remove_file(repo.join("delete-me.txt"))?; + let new_file_contents = "hello ghost\n"; + std::fs::write(repo.join("new-file.txt"), new_file_contents)?; + std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?; + let ignored_contents = "ignored but captured\n"; + std::fs::write(repo.join("ignored.txt"), ignored_contents)?; + + let options = + CreateGhostCommitOptions::new(repo).force_include(vec![PathBuf::from("ignored.txt")]); + let ghost = create_ghost_commit(&options)?; + + assert!(ghost.parent().is_some()); + let cat = run_git_for_stdout( + repo, + vec![ + OsString::from("show"), + OsString::from(format!("{}:ignored.txt", ghost.id())), + ], + None, + )?; + assert_eq!(cat, ignored_contents.trim()); + + std::fs::write(repo.join("tracked.txt"), "other state\n")?; + std::fs::write(repo.join("ignored.txt"), "changed\n")?; + std::fs::remove_file(repo.join("new-file.txt"))?; + std::fs::write(repo.join("ephemeral.txt"), "temp data\n")?; + + restore_ghost_commit(repo, &ghost)?; + + let tracked_after = std::fs::read_to_string(repo.join("tracked.txt"))?; + assert_eq!(tracked_after, tracked_contents); + let ignored_after = std::fs::read_to_string(repo.join("ignored.txt"))?; + assert_eq!(ignored_after, ignored_contents); + let new_file_after = std::fs::read_to_string(repo.join("new-file.txt"))?; + assert_eq!(new_file_after, new_file_contents); + assert_eq!(repo.join("delete-me.txt").exists(), false); + assert!(repo.join("ephemeral.txt").exists()); + + Ok(()) + } + + #[test] + /// Ensures ghost commits succeed in repositories without an existing HEAD. + fn create_snapshot_without_existing_head() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + let tracked_contents = "first contents\n"; + std::fs::write(repo.join("tracked.txt"), tracked_contents)?; + let ignored_contents = "ignored but captured\n"; + std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?; + std::fs::write(repo.join("ignored.txt"), ignored_contents)?; + + let options = + CreateGhostCommitOptions::new(repo).force_include(vec![PathBuf::from("ignored.txt")]); + let ghost = create_ghost_commit(&options)?; + + assert!(ghost.parent().is_none()); + + let message = run_git_stdout(repo, &["log", "-1", "--format=%s", ghost.id()]); + assert_eq!(message, DEFAULT_COMMIT_MESSAGE); + + let ignored = run_git_stdout(repo, &["show", &format!("{}:ignored.txt", ghost.id())]); + assert_eq!(ignored, ignored_contents.trim()); + + Ok(()) + } + + #[test] + /// Confirms custom messages are used when creating ghost commits. + fn create_ghost_commit_uses_custom_message() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join("tracked.txt"), "contents\n")?; + run_git_in(repo, &["add", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let message = "custom message"; + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo).message(message))?; + let commit_message = run_git_stdout(repo, &["log", "-1", "--format=%s", ghost.id()]); + assert_eq!(commit_message, message); + + Ok(()) + } + + #[test] + /// Rejects force-included paths that escape the repository. + fn create_ghost_commit_rejects_force_include_parent_path() { + let temp = tempfile::tempdir().expect("tempdir"); + let repo = temp.path(); + init_test_repo(repo); + let options = CreateGhostCommitOptions::new(repo) + .force_include(vec![PathBuf::from("../outside.txt")]); + let err = create_ghost_commit(&options).unwrap_err(); + assert!(matches!(err, GitToolingError::PathEscapesRepository { .. })); + } + + #[test] + /// Restoring a ghost commit from a non-git directory fails. + fn restore_requires_git_repository() { + let temp = tempfile::tempdir().expect("tempdir"); + let err = restore_to_commit(temp.path(), "deadbeef").unwrap_err(); + assert!(matches!(err, GitToolingError::NotAGitRepository { .. })); + } + + #[test] + /// Restoring from a subdirectory affects only that subdirectory. + fn restore_from_subdirectory_restores_files_relatively() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::create_dir_all(repo.join("workspace"))?; + let workspace = repo.join("workspace"); + std::fs::write(repo.join("root.txt"), "root contents\n")?; + std::fs::write(workspace.join("nested.txt"), "nested contents\n")?; + run_git_in(repo, &["add", "."]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + std::fs::write(repo.join("root.txt"), "root modified\n")?; + std::fs::write(workspace.join("nested.txt"), "nested modified\n")?; + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(&workspace))?; + + std::fs::write(repo.join("root.txt"), "root after\n")?; + std::fs::write(workspace.join("nested.txt"), "nested after\n")?; + + restore_ghost_commit(&workspace, &ghost)?; + + let root_after = std::fs::read_to_string(repo.join("root.txt"))?; + assert_eq!(root_after, "root after\n"); + let nested_after = std::fs::read_to_string(workspace.join("nested.txt"))?; + assert_eq!(nested_after, "nested modified\n"); + assert!(!workspace.join("codex-rs").exists()); + + Ok(()) + } + + #[test] + /// Restoring from a subdirectory preserves ignored files in parent folders. + fn restore_from_subdirectory_preserves_parent_vscode() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + let workspace = repo.join("codex-rs"); + std::fs::create_dir_all(&workspace)?; + std::fs::write(repo.join(".gitignore"), ".vscode/\n")?; + std::fs::write(workspace.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", "."]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + std::fs::write(workspace.join("tracked.txt"), "snapshot delta\n")?; + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(&workspace))?; + + std::fs::write(workspace.join("tracked.txt"), "post-snapshot\n")?; + let vscode = repo.join(".vscode"); + std::fs::create_dir_all(&vscode)?; + std::fs::write(vscode.join("settings.json"), "{\n \"after\": true\n}\n")?; + + restore_ghost_commit(&workspace, &ghost)?; + + let tracked_after = std::fs::read_to_string(workspace.join("tracked.txt"))?; + assert_eq!(tracked_after, "snapshot delta\n"); + assert!(vscode.join("settings.json").exists()); + let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?; + assert_eq!(settings_after, "{\n \"after\": true\n}\n"); + + Ok(()) + } + + #[test] + /// Restoring from the repository root keeps ignored files intact. + fn restore_preserves_ignored_files() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), ".vscode/\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + let vscode = repo.join(".vscode"); + std::fs::create_dir_all(&vscode)?; + std::fs::write(vscode.join("settings.json"), "{\n \"before\": true\n}\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + std::fs::write(repo.join("tracked.txt"), "snapshot delta\n")?; + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + std::fs::write(repo.join("tracked.txt"), "post-snapshot\n")?; + std::fs::write(vscode.join("settings.json"), "{\n \"after\": true\n}\n")?; + std::fs::write(repo.join("temp.txt"), "new file\n")?; + + restore_ghost_commit(repo, &ghost)?; + + let tracked_after = std::fs::read_to_string(repo.join("tracked.txt"))?; + assert_eq!(tracked_after, "snapshot delta\n"); + assert!(vscode.join("settings.json").exists()); + let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?; + assert_eq!(settings_after, "{\n \"after\": true\n}\n"); + assert!(repo.join("temp.txt").exists()); + + Ok(()) + } +} diff --git a/codex-rs/git-tooling/src/lib.rs b/codex-rs/git-tooling/src/lib.rs new file mode 100644 index 0000000000..f41d104385 --- /dev/null +++ b/codex-rs/git-tooling/src/lib.rs @@ -0,0 +1,43 @@ +use std::fmt; + +mod errors; +mod ghost_commits; +mod operations; +mod platform; + +pub use errors::GitToolingError; +pub use ghost_commits::CreateGhostCommitOptions; +pub use ghost_commits::create_ghost_commit; +pub use ghost_commits::restore_ghost_commit; +pub use ghost_commits::restore_to_commit; +pub use platform::create_symlink; + +/// Details of a ghost commit created from a repository state. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GhostCommit { + id: String, + parent: Option, +} + +impl GhostCommit { + /// Create a new ghost commit wrapper from a raw commit ID and optional parent. + pub fn new(id: String, parent: Option) -> Self { + Self { id, parent } + } + + /// Commit ID for the snapshot. + pub fn id(&self) -> &str { + &self.id + } + + /// Parent commit ID, if the repository had a `HEAD` at creation time. + pub fn parent(&self) -> Option<&str> { + self.parent.as_deref() + } +} + +impl fmt::Display for GhostCommit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.id) + } +} diff --git a/codex-rs/git-tooling/src/operations.rs b/codex-rs/git-tooling/src/operations.rs new file mode 100644 index 0000000000..3b387f8498 --- /dev/null +++ b/codex-rs/git-tooling/src/operations.rs @@ -0,0 +1,218 @@ +use std::ffi::OsStr; +use std::ffi::OsString; +use std::path::Component; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +use crate::GitToolingError; + +pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> { + match run_git_for_stdout( + path, + vec![ + OsString::from("rev-parse"), + OsString::from("--is-inside-work-tree"), + ], + None, + ) { + Ok(output) if output.trim() == "true" => Ok(()), + Ok(_) => Err(GitToolingError::NotAGitRepository { + path: path.to_path_buf(), + }), + Err(GitToolingError::GitCommand { status, .. }) if status.code() == Some(128) => { + Err(GitToolingError::NotAGitRepository { + path: path.to_path_buf(), + }) + } + Err(err) => Err(err), + } +} + +pub(crate) fn resolve_head(path: &Path) -> Result, GitToolingError> { + match run_git_for_stdout( + path, + vec![ + OsString::from("rev-parse"), + OsString::from("--verify"), + OsString::from("HEAD"), + ], + None, + ) { + Ok(sha) => Ok(Some(sha)), + Err(GitToolingError::GitCommand { status, .. }) if status.code() == Some(128) => Ok(None), + Err(other) => Err(other), + } +} + +pub(crate) fn normalize_relative_path(path: &Path) -> Result { + let mut result = PathBuf::new(); + let mut saw_component = false; + for component in path.components() { + saw_component = true; + match component { + Component::Normal(part) => result.push(part), + Component::CurDir => {} + Component::ParentDir => { + if !result.pop() { + return Err(GitToolingError::PathEscapesRepository { + path: path.to_path_buf(), + }); + } + } + Component::RootDir | Component::Prefix(_) => { + return Err(GitToolingError::NonRelativePath { + path: path.to_path_buf(), + }); + } + } + } + + if !saw_component { + return Err(GitToolingError::NonRelativePath { + path: path.to_path_buf(), + }); + } + + Ok(result) +} + +pub(crate) fn resolve_repository_root(path: &Path) -> Result { + let root = run_git_for_stdout( + path, + vec![ + OsString::from("rev-parse"), + OsString::from("--show-toplevel"), + ], + None, + )?; + Ok(PathBuf::from(root)) +} + +pub(crate) fn apply_repo_prefix_to_force_include( + prefix: Option<&Path>, + paths: &[PathBuf], +) -> Vec { + if paths.is_empty() { + return Vec::new(); + } + + match prefix { + Some(prefix) => paths.iter().map(|path| prefix.join(path)).collect(), + None => paths.to_vec(), + } +} + +pub(crate) fn repo_subdir(repo_root: &Path, repo_path: &Path) -> Option { + if repo_root == repo_path { + return None; + } + + repo_path + .strip_prefix(repo_root) + .ok() + .and_then(non_empty_path) + .or_else(|| { + let repo_root_canon = repo_root.canonicalize().ok()?; + let repo_path_canon = repo_path.canonicalize().ok()?; + repo_path_canon + .strip_prefix(&repo_root_canon) + .ok() + .and_then(non_empty_path) + }) +} + +fn non_empty_path(path: &Path) -> Option { + if path.as_os_str().is_empty() { + None + } else { + Some(path.to_path_buf()) + } +} + +pub(crate) fn run_git_for_status( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result<(), GitToolingError> +where + I: IntoIterator, + S: AsRef, +{ + run_git(dir, args, env)?; + Ok(()) +} + +pub(crate) fn run_git_for_stdout( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + let run = run_git(dir, args, env)?; + String::from_utf8(run.output.stdout) + .map(|value| value.trim().to_string()) + .map_err(|source| GitToolingError::GitOutputUtf8 { + command: run.command, + source, + }) +} + +fn run_git( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + let iterator = args.into_iter(); + let (lower, upper) = iterator.size_hint(); + let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower)); + for arg in iterator { + args_vec.push(OsString::from(arg.as_ref())); + } + let command_string = build_command_string(&args_vec); + let mut command = Command::new("git"); + command.current_dir(dir); + if let Some(envs) = env { + for (key, value) in envs { + command.env(key, value); + } + } + command.args(&args_vec); + let output = command.output()?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Err(GitToolingError::GitCommand { + command: command_string, + status: output.status, + stderr, + }); + } + Ok(GitRun { + command: command_string, + output, + }) +} + +fn build_command_string(args: &[OsString]) -> String { + if args.is_empty() { + return "git".to_string(); + } + let joined = args + .iter() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>() + .join(" "); + format!("git {joined}") +} + +struct GitRun { + command: String, + output: std::process::Output, +} diff --git a/codex-rs/git-tooling/src/platform.rs b/codex-rs/git-tooling/src/platform.rs new file mode 100644 index 0000000000..06feff7532 --- /dev/null +++ b/codex-rs/git-tooling/src/platform.rs @@ -0,0 +1,37 @@ +use std::path::Path; + +use crate::GitToolingError; + +#[cfg(unix)] +pub fn create_symlink( + _source: &Path, + link_target: &Path, + destination: &Path, +) -> Result<(), GitToolingError> { + use std::os::unix::fs::symlink; + + symlink(link_target, destination)?; + Ok(()) +} + +#[cfg(windows)] +pub fn create_symlink( + source: &Path, + link_target: &Path, + destination: &Path, +) -> Result<(), GitToolingError> { + use std::os::windows::fs::FileTypeExt; + use std::os::windows::fs::symlink_dir; + use std::os::windows::fs::symlink_file; + + let metadata = std::fs::symlink_metadata(source)?; + if metadata.file_type().is_symlink_dir() { + symlink_dir(link_target, destination)?; + } else { + symlink_file(link_target, destination)?; + } + Ok(()) +} + +#[cfg(not(any(unix, windows)))] +compile_error!("codex-git-tooling symlink support is only implemented for Unix and Windows"); diff --git a/codex-rs/login/Cargo.toml b/codex-rs/login/Cargo.toml index c0f4a2dd6f..5d358361c1 100644 --- a/codex-rs/login/Cargo.toml +++ b/codex-rs/login/Cargo.toml @@ -30,5 +30,6 @@ urlencoding = { workspace = true } webbrowser = { workspace = true } [dev-dependencies] +anyhow = { workspace = true } core_test_support = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/login/tests/suite/login_server_e2e.rs b/codex-rs/login/tests/suite/login_server_e2e.rs index 1acba00932..6d8dbd6ded 100644 --- a/codex-rs/login/tests/suite/login_server_e2e.rs +++ b/codex-rs/login/tests/suite/login_server_e2e.rs @@ -5,6 +5,7 @@ use std::net::TcpListener; use std::thread; use std::time::Duration; +use anyhow::Result; use base64::Engine; use codex_login::ServerOptions; use codex_login::run_login_server; @@ -76,13 +77,13 @@ fn start_mock_issuer() -> (SocketAddr, thread::JoinHandle<()>) { } #[tokio::test] -async fn end_to_end_login_flow_persists_auth_json() { - non_sandbox_test!(); +async fn end_to_end_login_flow_persists_auth_json() -> Result<()> { + non_sandbox_test!(result); let (issuer_addr, issuer_handle) = start_mock_issuer(); let issuer = format!("http://{}:{}", issuer_addr.ip(), issuer_addr.port()); - let tmp = tempdir().unwrap(); + let tmp = tempdir()?; let codex_home = tmp.path().to_path_buf(); // Seed auth.json with stale API key + tokens that should be overwritten. @@ -97,9 +98,8 @@ async fn end_to_end_login_flow_persists_auth_json() { }); std::fs::write( codex_home.join("auth.json"), - serde_json::to_string_pretty(&stale_auth).unwrap(), - ) - .unwrap(); + serde_json::to_string_pretty(&stale_auth)?, + )?; let state = "test_state_123".to_string(); @@ -114,25 +114,24 @@ async fn end_to_end_login_flow_persists_auth_json() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts).unwrap(); + let server = run_login_server(opts)?; let login_port = server.actual_port; // Simulate browser callback, and follow redirect to /success let client = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::limited(5)) - .build() - .unwrap(); + .build()?; let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=test_state_123"); - let resp = client.get(&url).send().await.unwrap(); + let resp = client.get(&url).send().await?; assert!(resp.status().is_success()); // Wait for server shutdown - server.block_until_done().await.unwrap(); + server.block_until_done().await?; // Validate auth.json let auth_path = codex_home.join("auth.json"); - let data = std::fs::read_to_string(&auth_path).unwrap(); - let json: serde_json::Value = serde_json::from_str(&data).unwrap(); + let data = std::fs::read_to_string(&auth_path)?; + let json: serde_json::Value = serde_json::from_str(&data)?; // The following assert is here because of the old oauth flow that exchanges tokens for an // API key. See obtain_api_key in server.rs for details. Once we remove this old mechanism // from the code, this test should be updated to expect that the API key is no longer present. @@ -143,16 +142,17 @@ async fn end_to_end_login_flow_persists_auth_json() { // Stop mock issuer drop(issuer_handle); + Ok(()) } #[tokio::test] -async fn creates_missing_codex_home_dir() { - non_sandbox_test!(); +async fn creates_missing_codex_home_dir() -> Result<()> { + non_sandbox_test!(result); let (issuer_addr, _issuer_handle) = start_mock_issuer(); let issuer = format!("http://{}:{}", issuer_addr.ip(), issuer_addr.port()); - let tmp = tempdir().unwrap(); + let tmp = tempdir()?; let codex_home = tmp.path().join("missing-subdir"); // does not exist let state = "state2".to_string(); @@ -167,31 +167,32 @@ async fn creates_missing_codex_home_dir() { open_browser: false, force_state: Some(state), }; - let server = run_login_server(opts).unwrap(); + let server = run_login_server(opts)?; let login_port = server.actual_port; let client = reqwest::Client::new(); let url = format!("http://127.0.0.1:{login_port}/auth/callback?code=abc&state=state2"); - let resp = client.get(&url).send().await.unwrap(); + let resp = client.get(&url).send().await?; assert!(resp.status().is_success()); - server.block_until_done().await.unwrap(); + server.block_until_done().await?; let auth_path = codex_home.join("auth.json"); assert!( auth_path.exists(), "auth.json should be created even if parent dir was missing" ); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn cancels_previous_login_server_when_port_is_in_use() { - non_sandbox_test!(); +async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { + non_sandbox_test!(result); let (issuer_addr, _issuer_handle) = start_mock_issuer(); let issuer = format!("http://{}:{}", issuer_addr.ip(), issuer_addr.port()); - let first_tmp = tempdir().unwrap(); + let first_tmp = tempdir()?; let first_codex_home = first_tmp.path().to_path_buf(); let first_opts = ServerOptions { @@ -203,13 +204,13 @@ async fn cancels_previous_login_server_when_port_is_in_use() { force_state: Some("cancel_state".to_string()), }; - let first_server = run_login_server(first_opts).unwrap(); + let first_server = run_login_server(first_opts)?; let login_port = first_server.actual_port; let first_server_task = tokio::spawn(async move { first_server.block_until_done().await }); tokio::time::sleep(Duration::from_millis(100)).await; - let second_tmp = tempdir().unwrap(); + let second_tmp = tempdir()?; let second_codex_home = second_tmp.path().to_path_buf(); let second_opts = ServerOptions { @@ -221,7 +222,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() { force_state: Some("cancel_state_2".to_string()), }; - let second_server = run_login_server(second_opts).unwrap(); + let second_server = run_login_server(second_opts)?; assert_eq!(second_server.actual_port, login_port); let cancel_result = first_server_task @@ -232,11 +233,12 @@ async fn cancels_previous_login_server_when_port_is_in_use() { let client = reqwest::Client::new(); let cancel_url = format!("http://127.0.0.1:{login_port}/cancel"); - let resp = client.get(cancel_url).send().await.unwrap(); + let resp = client.get(cancel_url).send().await?; assert!(resp.status().is_success()); second_server .block_until_done() .await .expect_err("second login server should report cancellation"); + Ok(()) } diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index 1d4a1b0fbb..eec3a62a2c 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -1009,6 +1009,7 @@ impl CodexMessageProcessor { model, effort, summary, + final_output_json_schema: None, }) .await; @@ -1410,13 +1411,13 @@ fn extract_conversation_summary( #[cfg(test)] mod tests { use super::*; + use anyhow::Result; use pretty_assertions::assert_eq; use serde_json::json; #[test] - fn extract_conversation_summary_prefers_plain_user_messages() { - let conversation_id = - ConversationId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0").unwrap(); + fn extract_conversation_summary_prefers_plain_user_messages() -> Result<()> { + let conversation_id = ConversationId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0")?; let timestamp = Some("2025-09-05T16:53:11.850Z".to_string()); let path = PathBuf::from("rollout.jsonl"); @@ -1456,5 +1457,6 @@ mod tests { ); assert_eq!(summary.path, path); assert_eq!(summary.preview, "Count to 5"); + Ok(()) } } diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 90e7f9d26a..6f376b1df5 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -256,6 +256,7 @@ pub(crate) struct OutgoingError { #[cfg(test)] mod tests { + use anyhow::Result; use codex_core::protocol::EventMsg; use codex_core::protocol::SessionConfiguredEvent; use codex_protocol::config_types::ReasoningEffort; @@ -269,12 +270,12 @@ mod tests { use super::*; #[tokio::test] - async fn test_send_event_as_notification() { + async fn test_send_event_as_notification() -> Result<()> { let (outgoing_tx, mut outgoing_rx) = mpsc::unbounded_channel::(); let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let conversation_id = ConversationId::new(); - let rollout_file = NamedTempFile::new().unwrap(); + let rollout_file = NamedTempFile::new()?; let event = Event { id: "1".to_string(), msg: EventMsg::SessionConfigured(SessionConfiguredEvent { @@ -302,15 +303,16 @@ mod tests { panic!("Event must serialize"); }; assert_eq!(params, Some(expected_params)); + Ok(()) } #[tokio::test] - async fn test_send_event_as_notification_with_meta() { + async fn test_send_event_as_notification_with_meta() -> Result<()> { let (outgoing_tx, mut outgoing_rx) = mpsc::unbounded_channel::(); let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let conversation_id = ConversationId::new(); - let rollout_file = NamedTempFile::new().unwrap(); + let rollout_file = NamedTempFile::new()?; let session_configured_event = SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), @@ -353,6 +355,7 @@ mod tests { } }); assert_eq!(params.unwrap(), expected_params); + Ok(()) } #[test] diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 9b530712cd..294f9982f7 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -31,6 +31,7 @@ ts-rs = { workspace = true, features = [ uuid = { workspace = true, features = ["serde", "v7"] } [dev-dependencies] +anyhow = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/protocol/src/mcp_protocol.rs b/codex-rs/protocol/src/mcp_protocol.rs index 52418b2755..c63bf2aecc 100644 --- a/codex-rs/protocol/src/mcp_protocol.rs +++ b/codex-rs/protocol/src/mcp_protocol.rs @@ -702,11 +702,12 @@ impl ServerNotification { #[cfg(test)] mod tests { use super::*; + use anyhow::Result; use pretty_assertions::assert_eq; use serde_json::json; #[test] - fn serialize_new_conversation() { + fn serialize_new_conversation() -> Result<()> { let request = ClientRequest::NewConversation { request_id: RequestId::Integer(42), params: NewConversationParams { @@ -730,8 +731,9 @@ mod tests { "approvalPolicy": "on-request" } }), - serde_json::to_value(&request).unwrap(), + serde_json::to_value(&request)?, ); + Ok(()) } #[test] @@ -741,23 +743,25 @@ mod tests { } #[test] - fn conversation_id_serializes_as_plain_string() { - let id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(); + fn conversation_id_serializes_as_plain_string() -> Result<()> { + let id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?; assert_eq!( json!("67e55044-10b1-426f-9247-bb680e5fe0c8"), - serde_json::to_value(id).unwrap() + serde_json::to_value(id)? ); + Ok(()) } #[test] - fn conversation_id_deserializes_from_plain_string() { + fn conversation_id_deserializes_from_plain_string() -> Result<()> { let id: ConversationId = - serde_json::from_value(json!("67e55044-10b1-426f-9247-bb680e5fe0c8")).unwrap(); + serde_json::from_value(json!("67e55044-10b1-426f-9247-bb680e5fe0c8"))?; assert_eq!( - ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(), + ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?, id, ); + Ok(()) } } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 08cd64f600..f6e5599cb3 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -318,9 +318,10 @@ impl std::ops::Deref for FunctionCallOutputPayload { #[cfg(test)] mod tests { use super::*; + use anyhow::Result; #[test] - fn serializes_success_as_plain_string() { + fn serializes_success_as_plain_string() -> Result<()> { let item = ResponseInputItem::FunctionCallOutput { call_id: "call1".into(), output: FunctionCallOutputPayload { @@ -329,15 +330,16 @@ mod tests { }, }; - let json = serde_json::to_string(&item).unwrap(); - let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + let json = serde_json::to_string(&item)?; + let v: serde_json::Value = serde_json::from_str(&json)?; // Success case -> output should be a plain string assert_eq!(v.get("output").unwrap().as_str().unwrap(), "ok"); + Ok(()) } #[test] - fn serializes_failure_as_string() { + fn serializes_failure_as_string() -> Result<()> { let item = ResponseInputItem::FunctionCallOutput { call_id: "call1".into(), output: FunctionCallOutputPayload { @@ -346,21 +348,22 @@ mod tests { }, }; - let json = serde_json::to_string(&item).unwrap(); - let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + let json = serde_json::to_string(&item)?; + let v: serde_json::Value = serde_json::from_str(&json)?; assert_eq!(v.get("output").unwrap().as_str().unwrap(), "bad"); + Ok(()) } #[test] - fn deserialize_shell_tool_call_params() { + fn deserialize_shell_tool_call_params() -> Result<()> { let json = r#"{ "command": ["ls", "-l"], "workdir": "/tmp", "timeout": 1000 }"#; - let params: ShellToolCallParams = serde_json::from_str(json).unwrap(); + let params: ShellToolCallParams = serde_json::from_str(json)?; assert_eq!( ShellToolCallParams { command: vec!["ls".to_string(), "-l".to_string()], @@ -371,5 +374,6 @@ mod tests { }, params ); + Ok(()) } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 478fcd5f10..bf0b192548 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -24,6 +24,7 @@ use mcp_types::CallToolResult; use mcp_types::Tool as McpTool; use serde::Deserialize; use serde::Serialize; +use serde_json::Value; use serde_with::serde_as; use strum_macros::Display; use ts_rs::TS; @@ -87,6 +88,8 @@ pub enum Op { /// Will only be honored if the model is configured to use reasoning. summary: ReasoningSummaryConfig, + // The JSON schema to use for the final assistant message + final_output_json_schema: Option, }, /// Override parts of the persistent turn context for subsequent turns. @@ -589,21 +592,23 @@ impl TokenUsageInfo { #[derive(Debug, Clone, Deserialize, Serialize, TS)] pub struct TokenCountEvent { pub info: Option, - pub rate_limits: Option, + pub rate_limits: Option, } #[derive(Debug, Clone, Deserialize, Serialize, TS)] -pub struct RateLimitSnapshotEvent { - /// Percentage (0-100) of the primary window that has been consumed. - pub primary_used_percent: f64, - /// Percentage (0-100) of the secondary window that has been consumed. - pub secondary_used_percent: f64, - /// Size of the primary window relative to secondary (0-100). - pub primary_to_secondary_ratio_percent: f64, - /// Rolling window duration for the primary limit, in minutes. - pub primary_window_minutes: u64, - /// Rolling window duration for the secondary limit, in minutes. - pub secondary_window_minutes: u64, +pub struct RateLimitSnapshot { + pub primary: Option, + pub secondary: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, TS)] +pub struct RateLimitWindow { + /// Percentage (0-100) of the window that has been consumed. + pub used_percent: f64, + /// Rolling window duration, in minutes. + pub window_minutes: Option, + /// Seconds until the window resets. + pub resets_in_seconds: Option, } // Includes prompts, tools and space to call compact. @@ -1261,16 +1266,16 @@ pub enum TurnAbortReason { #[cfg(test)] mod tests { use super::*; + use anyhow::Result; use serde_json::json; use tempfile::NamedTempFile; /// Serialize Event to verify that its JSON representation has the expected /// amount of nesting. #[test] - fn serialize_event() { - let conversation_id = - ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(); - let rollout_file = NamedTempFile::new().unwrap(); + fn serialize_event() -> Result<()> { + let conversation_id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8")?; + let rollout_file = NamedTempFile::new()?; let event = Event { id: "1234".to_string(), msg: EventMsg::SessionConfigured(SessionConfiguredEvent { @@ -1296,23 +1301,25 @@ mod tests { "rollout_path": format!("{}", rollout_file.path().display()), } }); - assert_eq!(expected, serde_json::to_value(&event).unwrap()); + assert_eq!(expected, serde_json::to_value(&event)?); + Ok(()) } #[test] - fn vec_u8_as_base64_serialization_and_deserialization() { + fn vec_u8_as_base64_serialization_and_deserialization() -> Result<()> { let event = ExecCommandOutputDeltaEvent { call_id: "call21".to_string(), stream: ExecOutputStream::Stdout, chunk: vec![1, 2, 3, 4, 5], }; - let serialized = serde_json::to_string(&event).unwrap(); + let serialized = serde_json::to_string(&event)?; assert_eq!( r#"{"call_id":"call21","stream":"stdout","chunk":"AQIDBAU="}"#, serialized, ); - let deserialized: ExecCommandOutputDeltaEvent = serde_json::from_str(&serialized).unwrap(); + let deserialized: ExecCommandOutputDeltaEvent = serde_json::from_str(&serialized)?; assert_eq!(deserialized, event); + Ok(()) } } diff --git a/codex-rs/rust-toolchain.toml b/codex-rs/rust-toolchain.toml index 0a9f6e8169..7187a33189 100644 --- a/codex-rs/rust-toolchain.toml +++ b/codex-rs/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.89.0" -components = [ "clippy", "rustfmt", "rust-src"] +channel = "1.90.0" +components = ["clippy", "rustfmt", "rust-src"] diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 38546d0bc0..ffc49fd3b6 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -35,6 +35,7 @@ codex-common = { workspace = true, features = [ ] } codex-core = { workspace = true } codex-file-search = { workspace = true } +codex-git-tooling = { workspace = true } codex-login = { workspace = true } codex-ollama = { workspace = true } codex-protocol = { workspace = true } @@ -52,7 +53,6 @@ image = { workspace = true, features = [ itertools = { workspace = true } lazy_static = { workspace = true } mcp-types = { workspace = true } -once_cell = { workspace = true } path-clean = { workspace = true } rand = { workspace = true } ratatui = { workspace = true, features = [ diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index fc36809672..ec749f4b7b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -30,7 +30,7 @@ use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::Op; use codex_core::protocol::PatchApplyBeginEvent; -use codex_core::protocol::RateLimitSnapshotEvent; +use codex_core::protocol::RateLimitSnapshot; use codex_core::protocol::ReviewRequest; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; @@ -76,7 +76,9 @@ use crate::history_cell::AgentMessageCell; use crate::history_cell::CommandOutput; use crate::history_cell::ExecCell; use crate::history_cell::HistoryCell; +use crate::history_cell::McpToolCallCell; use crate::history_cell::PatchEventType; +use crate::history_cell::RateLimitSnapshotDisplay; use crate::markdown::append_markdown; use crate::slash_command::SlashCommand; use crate::text_formatting::truncate_text; @@ -90,9 +92,10 @@ use self::agent::spawn_agent; use self::agent::spawn_agent_from_existing; mod session_header; use self::session_header::SessionHeader; -use crate::streaming::controller::AppEventHistorySink; use crate::streaming::controller::StreamController; -// +use std::path::Path; + +use chrono::Local; use codex_common::approval_presets::ApprovalPreset; use codex_common::approval_presets::builtin_approval_presets; use codex_common::model_presets::ModelPreset; @@ -103,7 +106,13 @@ use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; use codex_file_search::FileMatch; -use std::path::Path; +use codex_git_tooling::CreateGhostCommitOptions; +use codex_git_tooling::GhostCommit; +use codex_git_tooling::GitToolingError; +use codex_git_tooling::create_ghost_commit; +use codex_git_tooling::restore_ghost_commit; + +const MAX_TRACKED_GHOST_COMMITS: usize = 20; // Track information about an in-flight exec command. struct RunningCommand { @@ -111,7 +120,7 @@ struct RunningCommand { parsed_cmd: Vec, } -const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [50.0, 75.0, 90.0]; +const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [75.0, 90.0, 95.0]; #[derive(Default)] struct RateLimitWarningState { @@ -122,29 +131,46 @@ struct RateLimitWarningState { impl RateLimitWarningState { fn take_warnings( &mut self, - secondary_used_percent: f64, - primary_used_percent: f64, + secondary_used_percent: Option, + primary_used_percent: Option, ) -> Vec { - let mut warnings = Vec::new(); - - while self.secondary_index < RATE_LIMIT_WARNING_THRESHOLDS.len() - && secondary_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.secondary_index] - { - let threshold = RATE_LIMIT_WARNING_THRESHOLDS[self.secondary_index]; - warnings.push(format!( - "Heads up, you've used over {threshold:.0}% of your weekly limit. Run /status for a breakdown." - )); - self.secondary_index += 1; + let reached_secondary_cap = + matches!(secondary_used_percent, Some(percent) if percent == 100.0); + let reached_primary_cap = matches!(primary_used_percent, Some(percent) if percent == 100.0); + if reached_secondary_cap || reached_primary_cap { + return Vec::new(); } - while self.primary_index < RATE_LIMIT_WARNING_THRESHOLDS.len() - && primary_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.primary_index] - { - let threshold = RATE_LIMIT_WARNING_THRESHOLDS[self.primary_index]; - warnings.push(format!( - "Heads up, you've used over {threshold:.0}% of your 5h limit. Run /status for a breakdown." - )); - self.primary_index += 1; + let mut warnings = Vec::new(); + + if let Some(secondary_used_percent) = secondary_used_percent { + let mut highest_secondary: Option = None; + while self.secondary_index < RATE_LIMIT_WARNING_THRESHOLDS.len() + && secondary_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.secondary_index] + { + highest_secondary = Some(RATE_LIMIT_WARNING_THRESHOLDS[self.secondary_index]); + self.secondary_index += 1; + } + if let Some(threshold) = highest_secondary { + warnings.push(format!( + "Heads up, you've used over {threshold:.0}% of your weekly limit. Run /status for a breakdown." + )); + } + } + + if let Some(primary_used_percent) = primary_used_percent { + let mut highest_primary: Option = None; + while self.primary_index < RATE_LIMIT_WARNING_THRESHOLDS.len() + && primary_used_percent >= RATE_LIMIT_WARNING_THRESHOLDS[self.primary_index] + { + highest_primary = Some(RATE_LIMIT_WARNING_THRESHOLDS[self.primary_index]); + self.primary_index += 1; + } + if let Some(threshold) = highest_primary { + warnings.push(format!( + "Heads up, you've used over {threshold:.0}% of your 5h limit. Run /status for a breakdown." + )); + } } warnings @@ -166,13 +192,13 @@ pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, bottom_pane: BottomPane, - active_exec_cell: Option, + active_cell: Option>, config: Config, auth_manager: Arc, session_header: SessionHeader, initial_user_message: Option, token_info: Option, - rate_limit_snapshot: Option, + rate_limit_snapshot: Option, rate_limit_warnings: RateLimitWarningState, // Stream lifecycle controller stream_controller: Option, @@ -197,6 +223,9 @@ pub(crate) struct ChatWidget { pending_notification: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, + // List of ghost commits corresponding to each turn. + ghost_snapshots: Vec, + ghost_snapshots_disabled: bool, } struct UserMessage { @@ -223,11 +252,13 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio impl ChatWidget { fn flush_answer_stream_with_separator(&mut self) { - if let Some(mut controller) = self.stream_controller.take() { - let sink = AppEventHistorySink(self.app_event_tx.clone()); - controller.finalize(&sink); + if let Some(mut controller) = self.stream_controller.take() + && let Some(cell) = controller.finalize() + { + self.add_boxed_history(cell); } } + // --- Small event handlers --- fn on_session_configured(&mut self, event: codex_core::protocol::SessionConfiguredEvent) { self.bottom_pane @@ -317,12 +348,8 @@ impl ChatWidget { } fn on_task_complete(&mut self, last_agent_message: Option) { - // If a stream is currently active, finalize only that stream to flush any tail - // without emitting stray headers for other streams. - if let Some(mut controller) = self.stream_controller.take() { - let sink = AppEventHistorySink(self.app_event_tx.clone()); - controller.finalize(&sink); - } + // If a stream is currently active, finalize it. + self.flush_answer_stream_with_separator(); // Mark task stopped and request redraw now that all content is in history. self.bottom_pane.set_task_running(false); self.running_commands.clear(); @@ -337,29 +364,39 @@ impl ChatWidget { } pub(crate) fn set_token_info(&mut self, info: Option) { - self.bottom_pane.set_token_usage(info.clone()); - self.token_info = info; + if info.is_some() { + self.bottom_pane.set_token_usage(info.clone()); + self.token_info = info; + } } - fn on_rate_limit_snapshot(&mut self, snapshot: Option) { + fn on_rate_limit_snapshot(&mut self, snapshot: Option) { if let Some(snapshot) = snapshot { let warnings = self.rate_limit_warnings.take_warnings( - snapshot.secondary_used_percent, - snapshot.primary_used_percent, + snapshot + .secondary + .as_ref() + .map(|window| window.used_percent), + snapshot.primary.as_ref().map(|window| window.used_percent), ); - self.rate_limit_snapshot = Some(snapshot); + + let display = history_cell::rate_limit_snapshot_display(&snapshot, Local::now()); + self.rate_limit_snapshot = Some(display); + if !warnings.is_empty() { for warning in warnings { self.add_to_history(history_cell::new_warning_event(warning)); } self.request_redraw(); } + } else { + self.rate_limit_snapshot = None; } } /// Finalize any active exec as failed and stop/clear running UI state. fn finalize_turn(&mut self) { // Ensure any spinner is replaced by a red ✗ and flushed into history. - self.finalize_active_exec_cell_as_failed(); + self.finalize_active_cell_as_failed(); // Reset running state and clear streaming buffers. self.bottom_pane.set_task_running(false); self.running_commands.clear(); @@ -515,14 +552,18 @@ impl ChatWidget { self.add_to_history(history_cell::new_stream_error_event(message)); self.request_redraw(); } + /// Periodic tick to commit at most one queued line to history with a small delay, /// animating the output. pub(crate) fn on_commit_tick(&mut self) { if let Some(controller) = self.stream_controller.as_mut() { - let sink = AppEventHistorySink(self.app_event_tx.clone()); - let finished = controller.on_commit_tick(&sink); - if finished { - self.handle_stream_finished(); + let (cell, is_idle) = controller.on_commit_tick(); + if let Some(cell) = cell { + self.bottom_pane.set_task_running(false); + self.add_boxed_history(cell); + } + if is_idle { + self.app_event_tx.send(AppEvent::StopCommitAnimation); } } } @@ -561,14 +602,15 @@ impl ChatWidget { #[inline] fn handle_streaming_delta(&mut self, delta: String) { // Before streaming agent content, flush any active exec cell group. - self.flush_active_exec_cell(); + self.flush_active_cell(); if self.stream_controller.is_none() { self.stream_controller = Some(StreamController::new(self.config.clone())); } - if let Some(controller) = self.stream_controller.as_mut() { - let sink = AppEventHistorySink(self.app_event_tx.clone()); - controller.push_and_maybe_commit(&delta, &sink); + if let Some(controller) = self.stream_controller.as_mut() + && controller.push(&delta) + { + self.app_event_tx.send(AppEvent::StartCommitAnimation); } self.request_redraw(); } @@ -580,16 +622,25 @@ impl ChatWidget { None => (vec![ev.call_id.clone()], Vec::new()), }; - if self.active_exec_cell.is_none() { - // This should have been created by handle_exec_begin_now, but in case it wasn't, - // create it now. - self.active_exec_cell = Some(history_cell::new_active_exec_command( + let needs_new = self + .active_cell + .as_ref() + .map(|cell| cell.as_any().downcast_ref::().is_none()) + .unwrap_or(true); + if needs_new { + self.flush_active_cell(); + self.active_cell = Some(Box::new(history_cell::new_active_exec_command( ev.call_id.clone(), command, parsed, - )); + ))); } - if let Some(cell) = self.active_exec_cell.as_mut() { + + if let Some(cell) = self + .active_cell + .as_mut() + .and_then(|c| c.as_any_mut().downcast_mut::()) + { cell.complete_call( &ev.call_id, CommandOutput { @@ -601,7 +652,7 @@ impl ChatWidget { ev.duration, ); if cell.should_flush() { - self.flush_active_exec_cell(); + self.flush_active_cell(); } } } @@ -668,50 +719,68 @@ impl ChatWidget { parsed_cmd: ev.parsed_cmd.clone(), }, ); - if let Some(exec) = &self.active_exec_cell { - if let Some(new_exec) = exec.with_added_call( + if let Some(cell) = self + .active_cell + .as_mut() + .and_then(|c| c.as_any_mut().downcast_mut::()) + && let Some(new_exec) = cell.with_added_call( ev.call_id.clone(), ev.command.clone(), ev.parsed_cmd.clone(), - ) { - self.active_exec_cell = Some(new_exec); - } else { - // Make a new cell. - self.flush_active_exec_cell(); - self.active_exec_cell = Some(history_cell::new_active_exec_command( - ev.call_id.clone(), - ev.command.clone(), - ev.parsed_cmd, - )); - } + ) + { + *cell = new_exec; } else { - self.active_exec_cell = Some(history_cell::new_active_exec_command( + self.flush_active_cell(); + + self.active_cell = Some(Box::new(history_cell::new_active_exec_command( ev.call_id.clone(), ev.command.clone(), ev.parsed_cmd, - )); + ))); } - // Request a redraw so the working header and command list are visible immediately. self.request_redraw(); } pub(crate) fn handle_mcp_begin_now(&mut self, ev: McpToolCallBeginEvent) { self.flush_answer_stream_with_separator(); - self.add_to_history(history_cell::new_active_mcp_tool_call(ev.invocation)); + self.flush_active_cell(); + self.active_cell = Some(Box::new(history_cell::new_active_mcp_tool_call( + ev.call_id, + ev.invocation, + ))); + self.request_redraw(); } pub(crate) fn handle_mcp_end_now(&mut self, ev: McpToolCallEndEvent) { self.flush_answer_stream_with_separator(); - self.add_boxed_history(history_cell::new_completed_mcp_tool_call( - 80, - ev.invocation, - ev.duration, - ev.result - .as_ref() - .map(|r| !r.is_error.unwrap_or(false)) - .unwrap_or(false), - ev.result, - )); + + let McpToolCallEndEvent { + call_id, + invocation, + duration, + result, + } = ev; + + let extra_cell = match self + .active_cell + .as_mut() + .and_then(|cell| cell.as_any_mut().downcast_mut::()) + { + Some(cell) if cell.call_id() == call_id => cell.complete(duration, result), + _ => { + self.flush_active_cell(); + let mut cell = history_cell::new_active_mcp_tool_call(call_id, invocation); + let extra_cell = cell.complete(duration, result); + self.active_cell = Some(Box::new(cell)); + extra_cell + } + }; + + self.flush_active_cell(); + if let Some(extra) = extra_cell { + self.add_boxed_history(extra); + } } fn layout_areas(&self, area: Rect) -> [Rect; 3] { @@ -719,7 +788,7 @@ impl ChatWidget { let remaining = area.height.saturating_sub(bottom_min); let active_desired = self - .active_exec_cell + .active_cell .as_ref() .map_or(0, |c| c.desired_height(area.width) + 1); let active_height = active_desired.min(remaining); @@ -764,7 +833,7 @@ impl ChatWidget { placeholder_text: placeholder, disable_paste_burst: config.disable_paste_burst, }), - active_exec_cell: None, + active_cell: None, config: config.clone(), auth_manager, session_header: SessionHeader::new(config.model), @@ -787,6 +856,8 @@ impl ChatWidget { suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, + ghost_snapshots: Vec::new(), + ghost_snapshots_disabled: true, } } @@ -823,7 +894,7 @@ impl ChatWidget { placeholder_text: placeholder, disable_paste_burst: config.disable_paste_burst, }), - active_exec_cell: None, + active_cell: None, config: config.clone(), auth_manager, session_header: SessionHeader::new(config.model), @@ -846,13 +917,15 @@ impl ChatWidget { suppress_session_configured_redraw: true, pending_notification: None, is_review_mode: false, + ghost_snapshots: Vec::new(), + ghost_snapshots_disabled: true, } } pub fn desired_height(&self, width: u16) -> u16 { self.bottom_pane.desired_height(width) + self - .active_exec_cell + .active_cell .as_ref() .map_or(0, |c| c.desired_height(width) + 1) } @@ -978,6 +1051,9 @@ impl ChatWidget { } self.app_event_tx.send(AppEvent::ExitRequest); } + SlashCommand::Undo => { + self.undo_last_snapshot(); + } SlashCommand::Diff => { self.add_diff_in_progress(); let tx = self.app_event_tx.clone(); @@ -1067,10 +1143,9 @@ impl ChatWidget { } } - fn flush_active_exec_cell(&mut self) { - if let Some(active) = self.active_exec_cell.take() { - self.app_event_tx - .send(AppEvent::InsertHistoryCell(Box::new(active))); + fn flush_active_cell(&mut self) { + if let Some(active) = self.active_cell.take() { + self.app_event_tx.send(AppEvent::InsertHistoryCell(active)); } } @@ -1081,13 +1156,19 @@ impl ChatWidget { fn add_boxed_history(&mut self, cell: Box) { if !cell.display_lines(u16::MAX).is_empty() { // Only break exec grouping if the cell renders visible lines. - self.flush_active_exec_cell(); + self.flush_active_cell(); } self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); } fn submit_user_message(&mut self, user_message: UserMessage) { let UserMessage { text, image_paths } = user_message; + if text.is_empty() && image_paths.is_empty() { + return; + } + + self.capture_ghost_snapshot(); + let mut items: Vec = Vec::new(); if !text.is_empty() { @@ -1098,10 +1179,6 @@ impl ChatWidget { items.push(InputItem::LocalImage { path }); } - if items.is_empty() { - return; - } - self.codex_op_tx .send(Op::UserInput { items }) .unwrap_or_else(|e| { @@ -1123,6 +1200,57 @@ impl ChatWidget { } } + fn capture_ghost_snapshot(&mut self) { + if self.ghost_snapshots_disabled { + return; + } + + let options = CreateGhostCommitOptions::new(&self.config.cwd); + match create_ghost_commit(&options) { + Ok(commit) => { + self.ghost_snapshots.push(commit); + if self.ghost_snapshots.len() > MAX_TRACKED_GHOST_COMMITS { + self.ghost_snapshots.remove(0); + } + } + Err(err) => { + self.ghost_snapshots_disabled = true; + let (message, hint) = match &err { + GitToolingError::NotAGitRepository { .. } => ( + "Snapshots disabled: current directory is not a Git repository." + .to_string(), + None, + ), + _ => ( + format!("Snapshots disabled after error: {err}"), + Some( + "Restart Codex after resolving the issue to re-enable snapshots." + .to_string(), + ), + ), + }; + self.add_info_message(message, hint); + tracing::warn!("failed to create ghost snapshot: {err}"); + } + } + } + + fn undo_last_snapshot(&mut self) { + let Some(commit) = self.ghost_snapshots.pop() else { + self.add_info_message("No snapshot available to undo.".to_string(), None); + return; + }; + + if let Err(err) = restore_ghost_commit(&self.config.cwd, &commit) { + self.add_error_message(format!("Failed to restore snapshot: {err}")); + self.ghost_snapshots.push(commit); + return; + } + + let short_id: String = commit.id().chars().take(8).collect(); + self.add_info_message(format!("Restored workspace to snapshot {short_id}"), None); + } + /// Replay a subset of initial events into the UI to seed the transcript when /// resuming an existing session. This approximates the live event flow and /// is intentionally conservative: only safe-to-replay items are rendered to @@ -1249,7 +1377,7 @@ impl ChatWidget { if let Some(output) = review.review_output { self.flush_answer_stream_with_separator(); self.flush_interrupt_queue(); - self.flush_active_exec_cell(); + self.flush_active_cell(); if output.findings.is_empty() { let explanation = output.overall_explanation.trim().to_string(); @@ -1318,12 +1446,16 @@ impl ChatWidget { } } - /// Mark the active exec cell as failed (✗) and flush it into history. - fn finalize_active_exec_cell_as_failed(&mut self) { - if let Some(cell) = self.active_exec_cell.take() { - let cell = cell.into_failed(); - // Insert finalized exec into history and keep grouping consistent. - self.add_to_history(cell); + /// Mark the active cell as failed (✗) and flush it into history. + fn finalize_active_cell_as_failed(&mut self) { + if let Some(mut cell) = self.active_cell.take() { + // Insert finalized cell into history and keep grouping consistent. + if let Some(exec) = cell.as_any_mut().downcast_mut::() { + exec.mark_failed(); + } else if let Some(tool) = cell.as_any_mut().downcast_mut::() { + tool.mark_failed(); + } + self.add_boxed_history(cell); } } @@ -1798,12 +1930,16 @@ impl WidgetRef for &ChatWidget { let [_, active_cell_area, bottom_pane_area] = self.layout_areas(area); (&self.bottom_pane).render(bottom_pane_area, buf); if !active_cell_area.is_empty() - && let Some(cell) = &self.active_exec_cell + && let Some(cell) = &self.active_cell { - let mut active_cell_area = active_cell_area; - active_cell_area.y = active_cell_area.y.saturating_add(1); - active_cell_area.height -= 1; - cell.render_ref(active_cell_area, buf); + let mut area = active_cell_area; + area.y = area.y.saturating_add(1); + area.height = area.height.saturating_sub(1); + if let Some(exec) = cell.as_any().downcast_ref::() { + exec.render_ref(area, buf); + } else if let Some(tool) = cell.as_any().downcast_ref::() { + tool.render_ref(area, buf); + } } } } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index b983f6f553..7805e07e7b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -314,7 +314,7 @@ fn make_chatwidget_manual() -> ( app_event_tx, codex_op_tx: op_tx, bottom_pane: bottom, - active_exec_cell: None, + active_cell: None, config: cfg.clone(), auth_manager, session_header: SessionHeader::new(cfg.model), @@ -335,6 +335,8 @@ fn make_chatwidget_manual() -> ( suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, + ghost_snapshots: Vec::new(), + ghost_snapshots_disabled: false, }; (widget, rx, op_rx) } @@ -382,41 +384,30 @@ fn rate_limit_warnings_emit_thresholds() { let mut state = RateLimitWarningState::default(); let mut warnings: Vec = Vec::new(); - warnings.extend(state.take_warnings(10.0, 55.0)); - warnings.extend(state.take_warnings(55.0, 10.0)); - warnings.extend(state.take_warnings(10.0, 80.0)); - warnings.extend(state.take_warnings(80.0, 10.0)); - warnings.extend(state.take_warnings(10.0, 95.0)); - warnings.extend(state.take_warnings(95.0, 10.0)); + warnings.extend(state.take_warnings(Some(10.0), Some(55.0))); + warnings.extend(state.take_warnings(Some(55.0), Some(10.0))); + warnings.extend(state.take_warnings(Some(10.0), Some(80.0))); + warnings.extend(state.take_warnings(Some(80.0), Some(10.0))); + warnings.extend(state.take_warnings(Some(10.0), Some(95.0))); + warnings.extend(state.take_warnings(Some(95.0), Some(10.0))); assert_eq!( - warnings.len(), - 6, - "expected one warning per threshold per limit" - ); - assert!( - warnings - .iter() - .any(|w| w.contains("Heads up, you've used over 50% of your 5h limit.")), - "expected hourly 50% warning (new copy)" - ); - assert!( - warnings - .iter() - .any(|w| w.contains("Heads up, you've used over 50% of your weekly limit.")), - "expected weekly 50% warning (new copy)" - ); - assert!( - warnings - .iter() - .any(|w| w.contains("Heads up, you've used over 90% of your 5h limit.")), - "expected hourly 90% warning (new copy)" - ); - assert!( - warnings - .iter() - .any(|w| w.contains("Heads up, you've used over 90% of your weekly limit.")), - "expected weekly 90% warning (new copy)" + warnings, + vec![ + String::from( + "Heads up, you've used over 75% of your 5h limit. Run /status for a breakdown." + ), + String::from( + "Heads up, you've used over 75% of your weekly limit. Run /status for a breakdown.", + ), + String::from( + "Heads up, you've used over 95% of your 5h limit. Run /status for a breakdown." + ), + String::from( + "Heads up, you've used over 95% of your weekly limit. Run /status for a breakdown.", + ), + ], + "expected one warning per limit for the highest crossed threshold" ); } @@ -560,9 +551,9 @@ fn end_exec(chat: &mut ChatWidget, call_id: &str, stdout: &str, stderr: &str, ex fn active_blob(chat: &ChatWidget) -> String { let lines = chat - .active_exec_cell + .active_cell .as_ref() - .expect("active exec cell present") + .expect("active cell present") .display_lines(80); lines_to_single_string(&lines) } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 9eccf29ab0..a63da9ead1 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -11,6 +11,9 @@ use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; use crate::wrapping::word_wrap_lines; use base64::Engine; +use chrono::DateTime; +use chrono::Duration as ChronoDuration; +use chrono::Local; use codex_ansi_escape::ansi_escape_line; use codex_common::create_config_summary_entries; use codex_common::elapsed::format_duration; @@ -24,7 +27,8 @@ use codex_core::plan_tool::UpdatePlanArgs; use codex_core::project_doc::discover_project_doc_paths; use codex_core::protocol::FileChange; use codex_core::protocol::McpInvocation; -use codex_core::protocol::RateLimitSnapshotEvent; +use codex_core::protocol::RateLimitSnapshot; +use codex_core::protocol::RateLimitWindow; use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_core::protocol::TokenUsage; @@ -47,6 +51,7 @@ use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; use std::any::Any; use std::collections::HashMap; +use std::convert::TryFrom; use std::io::Cursor; use std::path::Path; use std::path::PathBuf; @@ -100,6 +105,10 @@ impl dyn HistoryCell { pub(crate) fn as_any(&self) -> &dyn Any { self } + + pub(crate) fn as_any_mut(&mut self) -> &mut dyn Any { + self + } } #[derive(Debug)] @@ -537,9 +546,7 @@ impl WidgetRef for &ExecCell { } impl ExecCell { - /// Convert an active exec cell into a failed, completed exec cell. - /// Any call without output is marked as failed with a red ✗. - pub(crate) fn into_failed(mut self) -> ExecCell { + pub(crate) fn mark_failed(&mut self) { for call in self.calls.iter_mut() { if call.output.is_none() { let elapsed = call @@ -556,7 +563,6 @@ impl ExecCell { }); } } - self } pub(crate) fn new(call: ExecCall) -> Self { @@ -633,6 +639,50 @@ impl HistoryCell for CompletedMcpToolCallWithImageOutput { const TOOL_CALL_MAX_LINES: usize = 5; const SESSION_HEADER_MAX_INNER_WIDTH: usize = 56; // Just an eyeballed value +fn card_inner_width(width: u16, max_inner_width: usize) -> Option { + if width < 4 { + return None; + } + let inner_width = std::cmp::min(width.saturating_sub(4) as usize, max_inner_width); + Some(inner_width) +} + +fn with_border(lines: Vec>) -> Vec> { + let content_width = lines + .iter() + .map(|line| { + line.iter() + .map(|span| UnicodeWidthStr::width(span.content.as_ref())) + .sum::() + }) + .max() + .unwrap_or(0); + + let mut out = Vec::with_capacity(lines.len() + 2); + let border_inner_width = content_width + 2; + out.push(vec![format!("╭{}╮", "─".repeat(border_inner_width)).dim()].into()); + + for line in lines.into_iter() { + let used_width: usize = line + .iter() + .map(|span| UnicodeWidthStr::width(span.content.as_ref())) + .sum(); + let span_count = line.spans.len(); + let mut spans: Vec> = Vec::with_capacity(span_count + 4); + spans.push(Span::from("│ ").dim()); + spans.extend(line.into_iter()); + if used_width < content_width { + spans.push(Span::from(" ".repeat(content_width - used_width)).dim()); + } + spans.push(Span::from(" │").dim()); + out.push(Line::from(spans)); + } + + out.push(vec![format!("╰{}╯", "─".repeat(border_inner_width)).dim()].into()); + + out +} + fn title_case(s: &str) -> String { if s.is_empty() { return String::new(); @@ -816,46 +866,20 @@ impl SessionHeaderHistoryCell { impl HistoryCell for SessionHeaderHistoryCell { fn display_lines(&self, width: u16) -> Vec> { - let mut out: Vec> = Vec::new(); - if width < 4 { - return out; - } + let Some(inner_width) = card_inner_width(width, SESSION_HEADER_MAX_INNER_WIDTH) else { + return Vec::new(); + }; - let inner_width = std::cmp::min( - width.saturating_sub(2) as usize, - SESSION_HEADER_MAX_INNER_WIDTH, - ); - // Top border without a title on the border - let mut top = String::with_capacity(inner_width + 2); - top.push('╭'); - top.push_str(&"─".repeat(inner_width)); - top.push('╮'); - out.push(Line::from(top.dim())); + let make_row = |spans: Vec>| Line::from(spans); - // Title line rendered inside the box: " >_ OpenAI Codex (vX)" - let title_text = format!(" >_ OpenAI Codex (v{})", self.version); - let title_w = UnicodeWidthStr::width(title_text.as_str()); - let pad_w = inner_width.saturating_sub(title_w); - let mut title_spans: Vec> = vec![ - Span::from("│").dim(), - Span::from(" ").dim(), + // Title line rendered inside the box: ">_ OpenAI Codex (vX)" + let title_spans: Vec> = vec![ Span::from(">_ ").dim(), Span::from("OpenAI Codex").bold(), Span::from(" ").dim(), Span::from(format!("(v{})", self.version)).dim(), ]; - if pad_w > 0 { - title_spans.push(Span::from(" ".repeat(pad_w)).dim()); - } - title_spans.push(Span::from("│").dim()); - out.push(Line::from(title_spans)); - // Spacer row between title and details - out.push(Line::from(vec![ - Span::from(format!("│{}│", " ".repeat(inner_width))).dim(), - ])); - - // Model line: " model: (change with /model)" const CHANGE_MODEL_HINT_COMMAND: &str = "/model"; const CHANGE_MODEL_HINT_EXPLANATION: &str = " to change"; const DIR_LABEL: &str = "directory:"; @@ -866,59 +890,33 @@ impl HistoryCell for SessionHeaderHistoryCell { label_width = label_width ); let reasoning_label = self.reasoning_label(); - let mut model_value_for_width = self.model.clone(); - if let Some(reasoning) = reasoning_label { - model_value_for_width.push(' '); - model_value_for_width.push_str(reasoning); - } - let model_text_for_width_calc = format!( - " {model_label} {model_value_for_width} {CHANGE_MODEL_HINT_COMMAND}{CHANGE_MODEL_HINT_EXPLANATION}", - ); - let model_w = UnicodeWidthStr::width(model_text_for_width_calc.as_str()); - let pad_w = inner_width.saturating_sub(model_w); - let mut spans: Vec> = vec![ - Span::from(format!("│ {model_label} ")).dim(), + let mut model_spans: Vec> = vec![ + Span::from(format!("{model_label} ")).dim(), Span::from(self.model.clone()), ]; if let Some(reasoning) = reasoning_label { - spans.push(Span::from(" ")); - spans.push(Span::from(reasoning)); + model_spans.push(Span::from(" ")); + model_spans.push(Span::from(reasoning)); } - spans.push(Span::from(" ").dim()); - spans.push(Span::from(CHANGE_MODEL_HINT_COMMAND).cyan()); - spans.push(Span::from(CHANGE_MODEL_HINT_EXPLANATION).dim()); - if pad_w > 0 { - spans.push(Span::from(" ".repeat(pad_w)).dim()); - } - spans.push(Span::from("│").dim()); - out.push(Line::from(spans)); + model_spans.push(" ".dim()); + model_spans.push(CHANGE_MODEL_HINT_COMMAND.cyan()); + model_spans.push(CHANGE_MODEL_HINT_EXPLANATION.dim()); - // Directory line: " Directory: " let dir_label = format!("{DIR_LABEL:> = vec![ - Span::from("│").dim(), - Span::from(" ").dim(), - Span::from(dir_label).dim(), - Span::from(" ").dim(), - Span::from(dir), + let dir_spans = vec![Span::from(dir_prefix).dim(), Span::from(dir)]; + + let lines = vec![ + make_row(title_spans), + make_row(Vec::new()), + make_row(model_spans), + make_row(dir_spans), ]; - if pad_w > 0 { - spans.push(Span::from(" ".repeat(pad_w)).dim()); - } - spans.push(Span::from("│").dim()); - out.push(Line::from(spans)); - // Bottom border - let bottom = format!("╰{}╯", "─".repeat(inner_width)); - out.push(Line::from(bottom.dim())); - - out + with_border(lines) } } @@ -945,6 +943,179 @@ impl HistoryCell for CompositeHistoryCell { } } +#[derive(Debug)] +pub(crate) struct McpToolCallCell { + call_id: String, + invocation: McpInvocation, + start_time: Instant, + duration: Option, + result: Option>, +} + +impl McpToolCallCell { + pub(crate) fn new(call_id: String, invocation: McpInvocation) -> Self { + Self { + call_id, + invocation, + start_time: Instant::now(), + duration: None, + result: None, + } + } + + pub(crate) fn call_id(&self) -> &str { + &self.call_id + } + + pub(crate) fn complete( + &mut self, + duration: Duration, + result: Result, + ) -> Option> { + let image_cell = try_new_completed_mcp_tool_call_with_image_output(&result) + .map(|cell| Box::new(cell) as Box); + self.duration = Some(duration); + self.result = Some(result); + image_cell + } + + fn success(&self) -> Option { + match self.result.as_ref() { + Some(Ok(result)) => Some(!result.is_error.unwrap_or(false)), + Some(Err(_)) => Some(false), + None => None, + } + } + + pub(crate) fn mark_failed(&mut self) { + let elapsed = self.start_time.elapsed(); + self.duration = Some(elapsed); + self.result = Some(Err("interrupted".to_string())); + } + + fn render_content_block(block: &mcp_types::ContentBlock, width: usize) -> String { + match block { + mcp_types::ContentBlock::TextContent(text) => { + format_and_truncate_tool_result(&text.text, TOOL_CALL_MAX_LINES, width) + } + mcp_types::ContentBlock::ImageContent(_) => "".to_string(), + mcp_types::ContentBlock::AudioContent(_) => "