From 7e569f11625605f501675e455cfc5e0d642503f2 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 22 Feb 2026 15:36:28 -0800 Subject: [PATCH 01/29] Add PR babysitting skill for this repo (#12513) ## PR Notes This PR adds a project-scoped `babysit-pr` skill for ongoing PR monitoring (CI, reviews, mergeability). Simply invoke this skill after creating a PR, and codex will do its best to get it to a mergeable state: ### What the skill does * Fixes CI failures related to the PR * Retries CI failures due to flaky tests * Addresses code review comments if it agrees with them * Addresses merge conflicts on main branch ### How the skill works - Polls PR status on a loop (CI checks, workflow runs, review activity, mergeability, and review decision). - Detects new review feedback (including inline comments and automated Codex review comments) and prompts/handles follow-up work. - Distinguishes pending vs failed vs passed CI and identifies likely flaky failures. - Can retry failed checks/workflows when appropriate. - Prioritizes actionable code review feedback over flaky CI retries (to avoid rerunning CI on a SHA that is about to be replaced). - Continues monitoring after fixes are applied and pushed, rather than stopping after a progress update. - Uses a slower backoff polling cadence once CI is green, while still watching for new review feedback or state changes. - Treats required review/approval as a blocking condition and keeps watching until the PR is actually merge-ready (or merged/closed, or human intervention is needed). ### Intended outcome Keep the PR moving with minimal manual babysitting by continuously watching for CI failures, reviewer feedback, and merge blockers, and responding in the right order until the PR is ready to merge. --- .codex/skills/babysit-pr/SKILL.md | 185 ++++ .codex/skills/babysit-pr/agents/openai.yaml | 4 + .../babysit-pr/references/github-api-notes.md | 72 ++ .../babysit-pr/references/heuristics.md | 58 ++ .../skills/babysit-pr/scripts/gh_pr_watch.py | 805 ++++++++++++++++++ 5 files changed, 1124 insertions(+) create mode 100644 .codex/skills/babysit-pr/SKILL.md create mode 100644 .codex/skills/babysit-pr/agents/openai.yaml create mode 100644 .codex/skills/babysit-pr/references/github-api-notes.md create mode 100644 .codex/skills/babysit-pr/references/heuristics.md create mode 100755 .codex/skills/babysit-pr/scripts/gh_pr_watch.py diff --git a/.codex/skills/babysit-pr/SKILL.md b/.codex/skills/babysit-pr/SKILL.md new file mode 100644 index 0000000000..4e44563c7b --- /dev/null +++ b/.codex/skills/babysit-pr/SKILL.md @@ -0,0 +1,185 @@ +--- +name: babysit-pr +description: Babysit a GitHub pull request after creation by continuously polling CI checks/workflow runs, new review comments, and mergeability state until the PR is ready to merge (or merged/closed). Diagnose failures, retry likely flaky failures up to 3 times, auto-fix/push branch-related issues when appropriate, and stop only when user help is required (for example CI infrastructure issues, exhausted flaky retries, or ambiguous/blocking situations). Use when the user asks Codex to monitor a PR, watch CI, handle review comments, or keep an eye on failures and feedback on an open PR. +--- + +# PR Babysitter + +## Objective +Babysit a PR persistently until one of these terminal outcomes occurs: + +- The PR is merged or closed. +- CI is successful, there are no unaddressed review comments surfaced by the watcher, required review approval is not blocking merge, and there are no potential merge conflicts (PR is mergeable / not reporting conflict risk). +- A situation requires user help (for example CI infrastructure issues, repeated flaky failures after retry budget is exhausted, permission problems, or ambiguity that cannot be resolved safely). + +Do not stop merely because a single snapshot returns `idle` while checks are still pending. + +## Inputs +Accept any of the following: + +- No PR argument: infer the PR from the current branch (`--pr auto`) +- PR number +- PR URL + +## Core Workflow + +1. When the user asks to "monitor"/"watch"/"babysit" a PR, start with the watcher's continuous mode (`--watch`) unless you are intentionally doing a one-shot diagnostic snapshot. +2. Run the watcher script to snapshot PR/CI/review state (or consume each streamed snapshot from `--watch`). +3. Inspect the `actions` list in the JSON response. +4. If `diagnose_ci_failure` is present, inspect failed run logs and classify the failure. +5. If the failure is likely caused by the current branch, patch code locally, commit, and push. +6. If `process_review_comment` is present, inspect surfaced review items and decide whether to address them. +7. If a review item is actionable and correct, patch code locally, commit, and push. +8. If the failure is likely flaky/unrelated and `retry_failed_checks` is present, rerun failed jobs with `--retry-failed-now`. +9. If both actionable review feedback and `retry_failed_checks` are present, prioritize review feedback first; a new commit will retrigger CI, so avoid rerunning flaky checks on the old SHA unless you intentionally defer the review change. +10. On every loop, verify mergeability / merge-conflict status (for example via `gh pr view`) in addition to CI and review state. +11. After any push or rerun action, immediately return to step 1 and continue polling on the updated SHA/state. +12. If you had been using `--watch` before pausing to patch/commit/push, relaunch `--watch` yourself in the same turn immediately after the push (do not wait for the user to re-invoke the skill). +13. Repeat polling until the PR is green + review-clean + mergeable, `stop_pr_closed` appears, or a user-help-required blocker is reached. +14. Maintain terminal/session ownership: while babysitting is active, keep consuming watcher output in the same turn; do not leave a detached `--watch` process running and then end the turn as if monitoring were complete. + +## Commands + +### One-shot snapshot + +```bash +python3 .codex/skills/babysit-pr/scripts/gh_pr_watch.py --pr auto --once +``` + +### Continuous watch (JSONL) + +```bash +python3 .codex/skills/babysit-pr/scripts/gh_pr_watch.py --pr auto --watch +``` + +### Trigger flaky retry cycle (only when watcher indicates) + +```bash +python3 .codex/skills/babysit-pr/scripts/gh_pr_watch.py --pr auto --retry-failed-now +``` + +### Explicit PR target + +```bash +python3 .codex/skills/babysit-pr/scripts/gh_pr_watch.py --pr --once +``` + +## CI Failure Classification +Use `gh` commands to inspect failed runs before deciding to rerun. + +- `gh run view --json jobs,name,workflowName,conclusion,status,url,headSha` +- `gh run view --log-failed` + +Prefer treating failures as branch-related when logs point to changed code (compile/test/lint/typecheck/snapshots/static analysis in touched areas). + +Prefer treating failures as flaky/unrelated when logs show transient infra/external issues (timeouts, runner provisioning failures, registry/network outages, GitHub Actions infra errors). + +If classification is ambiguous, perform one manual diagnosis attempt before choosing rerun. + +Read `.codex/skills/babysit-pr/references/heuristics.md` for a concise checklist. + +## Review Comment Handling +The watcher surfaces review items from: + +- PR issue comments +- Inline review comments +- Review submissions (COMMENT / APPROVED / CHANGES_REQUESTED) + +It intentionally surfaces Codex reviewer bot feedback (for example comments/reviews from `chatgpt-codex-connector[bot]`) in addition to human reviewer feedback. Most unrelated bot noise should still be ignored. +For safety, the watcher only auto-surfaces trusted human review authors (for example repo OWNER/MEMBER/COLLABORATOR, plus the authenticated operator) and approved review bots such as Codex. +On a fresh watcher state file, existing pending review feedback may be surfaced immediately (not only comments that arrive after monitoring starts). This is intentional so already-open review comments are not missed. + +When you agree with a comment and it is actionable: + +1. Patch code locally. +2. Commit with `codex: address PR review feedback (#)`. +3. Push to the PR head branch. +4. Resume watching on the new SHA immediately (do not stop after reporting the push). +5. If monitoring was running in `--watch` mode, restart `--watch` immediately after the push in the same turn; do not wait for the user to ask again. + +If you disagree or the comment is non-actionable/already addressed, record it as handled by continuing the watcher loop (the script de-duplicates surfaced items via state after surfacing them). +If a code review comment/thread is already marked as resolved in GitHub, treat it as non-actionable and safely ignore it unless new unresolved follow-up feedback appears. + +## Git Safety Rules + +- Work only on the PR head branch. +- Avoid destructive git commands. +- Do not switch branches unless necessary to recover context. +- Before editing, check for unrelated uncommitted changes. If present, stop and ask the user. +- After each successful fix, commit and `git push`, then re-run the watcher. +- If you interrupted a live `--watch` session to make the fix, restart `--watch` immediately after the push in the same turn. +- Do not run multiple concurrent `--watch` processes for the same PR/state file; keep one watcher session active and reuse it until it stops or you intentionally restart it. +- A push is not a terminal outcome; continue the monitoring loop unless a strict stop condition is met. + +Commit message defaults: + +- `codex: fix CI failure on PR #` +- `codex: address PR review feedback (#)` + +## Monitoring Loop Pattern +Use this loop in a live Codex session: + +1. Run `--once`. +2. Read `actions`. +3. First check whether the PR is now merged or otherwise closed; if so, report that terminal state and stop polling immediately. +4. Check CI summary, new review items, and mergeability/conflict status. +5. Diagnose CI failures and classify branch-related vs flaky/unrelated. +6. Process actionable review comments before flaky reruns when both are present; if a review fix requires a commit, push it and skip rerunning failed checks on the old SHA. +7. Retry failed checks only when `retry_failed_checks` is present and you are not about to replace the current SHA with a review/CI fix commit. +8. If you pushed a commit or triggered a rerun, report the action briefly and continue polling (do not stop). +9. After a review-fix push, proactively restart continuous monitoring (`--watch`) in the same turn unless a strict stop condition has already been reached. +10. If everything is passing, mergeable, not blocked on required review approval, and there are no unaddressed review items, report success and stop. +11. If blocked on a user-help-required issue (infra outage, exhausted flaky retries, unclear reviewer request, permissions), report the blocker and stop. +12. Otherwise sleep according to the polling cadence below and repeat. + +When the user explicitly asks to monitor/watch/babysit a PR, prefer `--watch` so polling continues autonomously in one command. Use repeated `--once` snapshots only for debugging, local testing, or when the user explicitly asks for a one-shot check. +Do not stop to ask the user whether to continue polling; continue autonomously until a strict stop condition is met or the user explicitly interrupts. +Do not hand control back to the user after a review-fix push just because a new SHA was created; restarting the watcher and re-entering the poll loop is part of the same babysitting task. +If a `--watch` process is still running and no strict stop condition has been reached, the babysitting task is still in progress; keep streaming/consuming watcher output instead of ending the turn. + +## Polling Cadence +Use adaptive polling and continue monitoring even after CI turns green: + +- While CI is not green (pending/running/queued or failing): poll every 1 minute. +- After CI turns green: start at every 1 minute, then back off exponentially when there is no change (for example 1m, 2m, 4m, 8m, 16m, 32m), capping at every 1 hour. +- Reset the green-state polling interval back to 1 minute whenever anything changes (new commit/SHA, check status changes, new review comments, mergeability changes, review decision changes). +- If CI stops being green again (new commit, rerun, or regression): return to 1-minute polling. +- If any poll shows the PR is merged or otherwise closed: stop polling immediately and report the terminal state. + +## Stop Conditions (Strict) +Stop only when one of the following is true: + +- PR merged or closed (stop as soon as a poll/snapshot confirms this). +- PR is ready to merge: CI succeeded, no surfaced unaddressed review comments, not blocked on required review approval, and no merge conflict risk. +- User intervention is required and Codex cannot safely proceed alone. + +Keep polling when: + +- `actions` contains only `idle` but checks are still pending. +- CI is still running/queued. +- Review state is quiet but CI is not terminal. +- CI is green but mergeability is unknown/pending. +- CI is green and mergeable, but the PR is still open and you are waiting for possible new review comments or merge-conflict changes per the green-state cadence. +- The PR is green but blocked on review approval (`REVIEW_REQUIRED` / similar); continue polling on the green-state cadence and surface any new review comments without asking for confirmation to keep watching. + +## Output Expectations +Provide concise progress updates while monitoring and a final summary that includes: + +- During long unchanged monitoring periods, avoid emitting a full update on every poll; summarize only status changes plus occasional heartbeat updates. +- Treat push confirmations, intermediate CI snapshots, and review-action updates as progress updates only; do not emit the final summary or end the babysitting session unless a strict stop condition is met. +- A user request to "monitor" is not satisfied by a couple of sample polls; remain in the loop until a strict stop condition or an explicit user interruption. +- A review-fix commit + push is not a completion event; immediately resume live monitoring (`--watch`) in the same turn and continue reporting progress updates. +- When CI first transitions to all green for the current SHA, emit a one-time celebratory progress update (do not repeat it on every green poll). Preferred style: `🚀 CI is all green! 33/33 passed. Still on watch for review approval.` +- Do not send the final summary while a watcher terminal is still running unless the watcher has emitted/confirmed a strict stop condition; otherwise continue with progress updates. + +- Final PR SHA +- CI status summary +- Mergeability / conflict status +- Fixes pushed +- Flaky retry cycles used +- Remaining unresolved failures or review comments + +## References + +- Heuristics and decision tree: `.codex/skills/babysit-pr/references/heuristics.md` +- GitHub CLI/API details used by the watcher: `.codex/skills/babysit-pr/references/github-api-notes.md` diff --git a/.codex/skills/babysit-pr/agents/openai.yaml b/.codex/skills/babysit-pr/agents/openai.yaml new file mode 100644 index 0000000000..2faca770e0 --- /dev/null +++ b/.codex/skills/babysit-pr/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "PR Babysitter" + short_description: "Watch PR CI, reviews, and merge conflicts" + default_prompt: "Babysit the current PR: monitor CI, reviewer comments, and merge-conflict status (prefer the watcher’s --watch mode for live monitoring); fix valid issues, push updates, and rerun flaky failures up to 3 times. Keep exactly one watcher session active for the PR (do not leave duplicate --watch terminals running). If you pause monitoring to patch review/CI feedback, restart --watch yourself immediately after the push in the same turn. If a watcher is still running and no strict stop condition has been reached, the task is still in progress: keep consuming watcher output and sending progress updates instead of ending the turn. Continue polling autonomously after any push/rerun until a strict terminal stop condition is reached or the user interrupts." diff --git a/.codex/skills/babysit-pr/references/github-api-notes.md b/.codex/skills/babysit-pr/references/github-api-notes.md new file mode 100644 index 0000000000..8cc09da463 --- /dev/null +++ b/.codex/skills/babysit-pr/references/github-api-notes.md @@ -0,0 +1,72 @@ +# GitHub CLI / API Notes For `babysit-pr` + +## Primary commands used + +### PR metadata + +- `gh pr view --json number,url,state,mergedAt,closedAt,headRefName,headRefOid,headRepository,headRepositoryOwner` + +Used to resolve PR number, URL, branch, head SHA, and closed/merged state. + +### PR checks summary + +- `gh pr checks --json name,state,bucket,link,workflow,event,startedAt,completedAt` + +Used to compute pending/failed/passed counts and whether the current CI round is terminal. + +### Workflow runs for head SHA + +- `gh api repos/{owner}/{repo}/actions/runs -X GET -f head_sha= -f per_page=100` + +Used to discover failed workflow runs and rerunnable run IDs. + +### Failed log inspection + +- `gh run view --json jobs,name,workflowName,conclusion,status,url,headSha` +- `gh run view --log-failed` + +Used by Codex to classify branch-related vs flaky/unrelated failures. + +### Retry failed jobs only + +- `gh run rerun --failed` + +Reruns only failed jobs (and dependencies) for a workflow run. + +## Review-related endpoints + +- Issue comments on PR: + - `gh api repos/{owner}/{repo}/issues//comments?per_page=100` +- Inline PR review comments: + - `gh api repos/{owner}/{repo}/pulls//comments?per_page=100` +- Review submissions: + - `gh api repos/{owner}/{repo}/pulls//reviews?per_page=100` + +## JSON fields consumed by the watcher + +### `gh pr view` + +- `number` +- `url` +- `state` +- `mergedAt` +- `closedAt` +- `headRefName` +- `headRefOid` + +### `gh pr checks` + +- `bucket` (`pass`, `fail`, `pending`, `skipping`) +- `state` +- `name` +- `workflow` +- `link` + +### Actions runs API (`workflow_runs[]`) + +- `id` +- `name` +- `status` +- `conclusion` +- `html_url` +- `head_sha` diff --git a/.codex/skills/babysit-pr/references/heuristics.md b/.codex/skills/babysit-pr/references/heuristics.md new file mode 100644 index 0000000000..01024d2611 --- /dev/null +++ b/.codex/skills/babysit-pr/references/heuristics.md @@ -0,0 +1,58 @@ +# CI / Review Heuristics + +## CI classification checklist + +Treat as **branch-related** when logs clearly indicate a regression caused by the PR branch: + +- Compile/typecheck/lint failures in files or modules touched by the branch +- Deterministic unit/integration test failures in changed areas +- Snapshot output changes caused by UI/text changes in the branch +- Static analysis violations introduced by the latest push +- Build script/config changes in the PR causing a deterministic failure + +Treat as **likely flaky or unrelated** when evidence points to transient or external issues: + +- DNS/network/registry timeout errors while fetching dependencies +- Runner image provisioning or startup failures +- GitHub Actions infrastructure/service outages +- Cloud/service rate limits or transient API outages +- Non-deterministic failures in unrelated integration tests with known flake patterns + +If uncertain, inspect failed logs once before choosing rerun. + +## Decision tree (fix vs rerun vs stop) + +1. If PR is merged/closed: stop. +2. If there are failed checks: + - Diagnose first. + - If branch-related: fix locally, commit, push. + - If likely flaky/unrelated and all checks for the current SHA are terminal: rerun failed jobs. + - If checks are still pending: wait. +3. If flaky reruns for the same SHA reach the configured limit (default 3): stop and report persistent failure. +4. Independently, process any new human review comments. + +## Review comment agreement criteria + +Address the comment when: + +- The comment is technically correct. +- The change is actionable in the current branch. +- The requested change does not conflict with the user’s intent or recent guidance. +- The change can be made safely without unrelated refactors. + +Do not auto-fix when: + +- The comment is ambiguous and needs clarification. +- The request conflicts with explicit user instructions. +- The proposed change requires product/design decisions the user has not made. +- The codebase is in a dirty/unrelated state that makes safe editing uncertain. + +## Stop-and-ask conditions + +Stop and ask the user instead of continuing automatically when: + +- The local worktree has unrelated uncommitted changes. +- `gh` auth/permissions fail. +- The PR branch cannot be pushed. +- CI failures persist after the flaky retry budget. +- Reviewer feedback requires a product decision or cross-team coordination. diff --git a/.codex/skills/babysit-pr/scripts/gh_pr_watch.py b/.codex/skills/babysit-pr/scripts/gh_pr_watch.py new file mode 100755 index 0000000000..80aefd58d9 --- /dev/null +++ b/.codex/skills/babysit-pr/scripts/gh_pr_watch.py @@ -0,0 +1,805 @@ +#!/usr/bin/env python3 +"""Watch GitHub PR CI and review activity for Codex PR babysitting workflows.""" + +import argparse +import json +import os +import re +import subprocess +import sys +import tempfile +import time +from pathlib import Path +from urllib.parse import urlparse + +FAILED_RUN_CONCLUSIONS = { + "failure", + "timed_out", + "cancelled", + "action_required", + "startup_failure", + "stale", +} +PENDING_CHECK_STATES = { + "QUEUED", + "IN_PROGRESS", + "PENDING", + "WAITING", + "REQUESTED", +} +REVIEW_BOT_LOGIN_KEYWORDS = { + "codex", +} +TRUSTED_AUTHOR_ASSOCIATIONS = { + "OWNER", + "MEMBER", + "COLLABORATOR", +} +MERGE_BLOCKING_REVIEW_DECISIONS = { + "REVIEW_REQUIRED", + "CHANGES_REQUESTED", +} +MERGE_CONFLICT_OR_BLOCKING_STATES = { + "BLOCKED", + "DIRTY", + "DRAFT", + "UNKNOWN", +} +GREEN_STATE_MAX_POLL_SECONDS = 60 * 60 + + +class GhCommandError(RuntimeError): + pass + + +def parse_args(): + parser = argparse.ArgumentParser( + description=( + "Normalize PR/CI/review state for Codex PR babysitting and optionally " + "trigger flaky reruns." + ) + ) + parser.add_argument("--pr", default="auto", help="auto, PR number, or PR URL") + parser.add_argument("--repo", help="Optional OWNER/REPO override") + parser.add_argument("--poll-seconds", type=int, default=30, help="Watch poll interval") + parser.add_argument( + "--max-flaky-retries", + type=int, + default=3, + help="Max rerun cycles per head SHA before stop recommendation", + ) + parser.add_argument("--state-file", help="Path to state JSON file") + parser.add_argument("--once", action="store_true", help="Emit one snapshot and exit") + parser.add_argument("--watch", action="store_true", help="Continuously emit JSONL snapshots") + parser.add_argument( + "--retry-failed-now", + action="store_true", + help="Rerun failed jobs for current failed workflow runs when policy allows", + ) + parser.add_argument( + "--json", + action="store_true", + help="Emit machine-readable output (default behavior for --once and --retry-failed-now)", + ) + args = parser.parse_args() + + if args.poll_seconds <= 0: + parser.error("--poll-seconds must be > 0") + if args.max_flaky_retries < 0: + parser.error("--max-flaky-retries must be >= 0") + if args.watch and args.retry_failed_now: + parser.error("--watch cannot be combined with --retry-failed-now") + if not args.once and not args.watch and not args.retry_failed_now: + args.once = True + return args + + +def _format_gh_error(cmd, err): + stdout = (err.stdout or "").strip() + stderr = (err.stderr or "").strip() + parts = [f"GitHub CLI command failed: {' '.join(cmd)}"] + if stdout: + parts.append(f"stdout: {stdout}") + if stderr: + parts.append(f"stderr: {stderr}") + return "\n".join(parts) + + +def gh_text(args, repo=None): + cmd = ["gh"] + # `gh api` does not accept `-R/--repo` on all gh versions. The watcher's + # API calls use explicit endpoints (e.g. repos/{owner}/{repo}/...), so the + # repo flag is unnecessary there. + if repo and (not args or args[0] != "api"): + cmd.extend(["-R", repo]) + cmd.extend(args) + try: + proc = subprocess.run(cmd, check=True, capture_output=True, text=True) + except FileNotFoundError as err: + raise GhCommandError("`gh` command not found") from err + except subprocess.CalledProcessError as err: + raise GhCommandError(_format_gh_error(cmd, err)) from err + return proc.stdout + + +def gh_json(args, repo=None): + raw = gh_text(args, repo=repo).strip() + if not raw: + return None + try: + return json.loads(raw) + except json.JSONDecodeError as err: + raise GhCommandError(f"Failed to parse JSON from gh output for {' '.join(args)}") from err + + +def parse_pr_spec(pr_spec): + if pr_spec == "auto": + return {"mode": "auto", "value": None} + if re.fullmatch(r"\d+", pr_spec): + return {"mode": "number", "value": pr_spec} + parsed = urlparse(pr_spec) + if parsed.scheme and parsed.netloc and "/pull/" in parsed.path: + return {"mode": "url", "value": pr_spec} + raise ValueError("--pr must be 'auto', a PR number, or a PR URL") + + +def pr_view_fields(): + return ( + "number,url,state,mergedAt,closedAt,headRefName,headRefOid," + "headRepository,headRepositoryOwner,mergeable,mergeStateStatus,reviewDecision" + ) + + +def checks_fields(): + return "name,state,bucket,link,workflow,event,startedAt,completedAt" + + +def resolve_pr(pr_spec, repo_override=None): + parsed = parse_pr_spec(pr_spec) + cmd = ["pr", "view"] + if parsed["value"] is not None: + cmd.append(parsed["value"]) + cmd.extend(["--json", pr_view_fields()]) + data = gh_json(cmd, repo=repo_override) + if not isinstance(data, dict): + raise GhCommandError("Unexpected PR payload from `gh pr view`") + + pr_url = str(data.get("url") or "") + repo = ( + repo_override + or extract_repo_from_pr_url(pr_url) + or extract_repo_from_pr_view(data) + ) + if not repo: + raise GhCommandError("Unable to determine OWNER/REPO for the PR") + + state = str(data.get("state") or "") + merged = bool(data.get("mergedAt")) + closed = bool(data.get("closedAt")) or state.upper() == "CLOSED" + + return { + "number": int(data["number"]), + "url": pr_url, + "repo": repo, + "head_sha": str(data.get("headRefOid") or ""), + "head_branch": str(data.get("headRefName") or ""), + "state": state, + "merged": merged, + "closed": closed, + "mergeable": str(data.get("mergeable") or ""), + "merge_state_status": str(data.get("mergeStateStatus") or ""), + "review_decision": str(data.get("reviewDecision") or ""), + } + + +def extract_repo_from_pr_view(data): + head_repo = data.get("headRepository") + head_owner = data.get("headRepositoryOwner") + owner = None + name = None + if isinstance(head_owner, dict): + owner = head_owner.get("login") or head_owner.get("name") + elif isinstance(head_owner, str): + owner = head_owner + if isinstance(head_repo, dict): + name = head_repo.get("name") + repo_owner = head_repo.get("owner") + if not owner and isinstance(repo_owner, dict): + owner = repo_owner.get("login") or repo_owner.get("name") + elif isinstance(head_repo, str): + name = head_repo + if owner and name: + return f"{owner}/{name}" + return None +def extract_repo_from_pr_url(pr_url): + parsed = urlparse(pr_url) + parts = [p for p in parsed.path.split("/") if p] + if len(parts) >= 4 and parts[2] == "pull": + return f"{parts[0]}/{parts[1]}" + return None + + +def load_state(path): + if path.exists(): + try: + data = json.loads(path.read_text()) + except json.JSONDecodeError as err: + raise RuntimeError(f"State file is not valid JSON: {path}") from err + if not isinstance(data, dict): + raise RuntimeError(f"State file must contain an object: {path}") + return data, False + return { + "pr": {}, + "started_at": None, + "last_seen_head_sha": None, + "retries_by_sha": {}, + "seen_issue_comment_ids": [], + "seen_review_comment_ids": [], + "seen_review_ids": [], + "last_snapshot_at": None, + }, True + + +def save_state(path, state): + path.parent.mkdir(parents=True, exist_ok=True) + payload = json.dumps(state, indent=2, sort_keys=True) + "\n" + fd, tmp_name = tempfile.mkstemp(prefix=f"{path.name}.", suffix=".tmp", dir=path.parent) + tmp_path = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as tmp_file: + tmp_file.write(payload) + os.replace(tmp_path, path) + except Exception: + try: + tmp_path.unlink(missing_ok=True) + except OSError: + pass + raise + + +def default_state_file_for(pr): + repo_slug = pr["repo"].replace("/", "-") + return Path(f"/tmp/codex-babysit-pr-{repo_slug}-pr{pr['number']}.json") + + +def get_pr_checks(pr_spec, repo): + parsed = parse_pr_spec(pr_spec) + cmd = ["pr", "checks"] + if parsed["value"] is not None: + cmd.append(parsed["value"]) + cmd.extend(["--json", checks_fields()]) + data = gh_json(cmd, repo=repo) + if data is None: + return [] + if not isinstance(data, list): + raise GhCommandError("Unexpected payload from `gh pr checks`") + return data + + +def is_pending_check(check): + bucket = str(check.get("bucket") or "").lower() + state = str(check.get("state") or "").upper() + return bucket == "pending" or state in PENDING_CHECK_STATES + + +def summarize_checks(checks): + pending_count = 0 + failed_count = 0 + passed_count = 0 + for check in checks: + bucket = str(check.get("bucket") or "").lower() + if is_pending_check(check): + pending_count += 1 + if bucket == "fail": + failed_count += 1 + if bucket == "pass": + passed_count += 1 + return { + "pending_count": pending_count, + "failed_count": failed_count, + "passed_count": passed_count, + "all_terminal": pending_count == 0, + } + + +def get_workflow_runs_for_sha(repo, head_sha): + endpoint = f"repos/{repo}/actions/runs" + data = gh_json( + ["api", endpoint, "-X", "GET", "-f", f"head_sha={head_sha}", "-f", "per_page=100"], + repo=repo, + ) + if not isinstance(data, dict): + raise GhCommandError("Unexpected payload from actions runs API") + runs = data.get("workflow_runs") or [] + if not isinstance(runs, list): + raise GhCommandError("Expected `workflow_runs` to be a list") + return runs + + +def failed_runs_from_workflow_runs(runs, head_sha): + failed_runs = [] + for run in runs: + if not isinstance(run, dict): + continue + if str(run.get("head_sha") or "") != head_sha: + continue + conclusion = str(run.get("conclusion") or "") + if conclusion not in FAILED_RUN_CONCLUSIONS: + continue + failed_runs.append( + { + "run_id": run.get("id"), + "workflow_name": run.get("name") or run.get("display_title") or "", + "status": str(run.get("status") or ""), + "conclusion": conclusion, + "html_url": str(run.get("html_url") or ""), + } + ) + failed_runs.sort(key=lambda item: (str(item.get("workflow_name") or ""), str(item.get("run_id") or ""))) + return failed_runs + + +def get_authenticated_login(): + data = gh_json(["api", "user"]) + if not isinstance(data, dict) or not data.get("login"): + raise GhCommandError("Unable to determine authenticated GitHub login from `gh api user`") + return str(data["login"]) + + +def comment_endpoints(repo, pr_number): + return { + "issue_comment": f"repos/{repo}/issues/{pr_number}/comments", + "review_comment": f"repos/{repo}/pulls/{pr_number}/comments", + "review": f"repos/{repo}/pulls/{pr_number}/reviews", + } + + +def gh_api_list_paginated(endpoint, repo=None, per_page=100): + items = [] + page = 1 + while True: + sep = "&" if "?" in endpoint else "?" + page_endpoint = f"{endpoint}{sep}per_page={per_page}&page={page}" + payload = gh_json(["api", page_endpoint], repo=repo) + if payload is None: + break + if not isinstance(payload, list): + raise GhCommandError(f"Unexpected paginated payload from gh api {endpoint}") + items.extend(payload) + if len(payload) < per_page: + break + page += 1 + return items + + +def normalize_issue_comments(items): + out = [] + for item in items: + if not isinstance(item, dict): + continue + out.append( + { + "kind": "issue_comment", + "id": str(item.get("id") or ""), + "author": extract_login(item.get("user")), + "author_association": str(item.get("author_association") or ""), + "created_at": str(item.get("created_at") or ""), + "body": str(item.get("body") or ""), + "path": None, + "line": None, + "url": str(item.get("html_url") or ""), + } + ) + return out + + +def normalize_review_comments(items): + out = [] + for item in items: + if not isinstance(item, dict): + continue + line = item.get("line") + if line is None: + line = item.get("original_line") + out.append( + { + "kind": "review_comment", + "id": str(item.get("id") or ""), + "author": extract_login(item.get("user")), + "author_association": str(item.get("author_association") or ""), + "created_at": str(item.get("created_at") or ""), + "body": str(item.get("body") or ""), + "path": item.get("path"), + "line": line, + "url": str(item.get("html_url") or ""), + } + ) + return out + + +def normalize_reviews(items): + out = [] + for item in items: + if not isinstance(item, dict): + continue + out.append( + { + "kind": "review", + "id": str(item.get("id") or ""), + "author": extract_login(item.get("user")), + "author_association": str(item.get("author_association") or ""), + "created_at": str(item.get("submitted_at") or item.get("created_at") or ""), + "body": str(item.get("body") or ""), + "path": None, + "line": None, + "url": str(item.get("html_url") or ""), + } + ) + return out + + +def extract_login(user_obj): + if isinstance(user_obj, dict): + return str(user_obj.get("login") or "") + return "" + + +def is_bot_login(login): + return bool(login) and login.endswith("[bot]") + + +def is_actionable_review_bot_login(login): + if not is_bot_login(login): + return False + lower_login = login.lower() + return any(keyword in lower_login for keyword in REVIEW_BOT_LOGIN_KEYWORDS) + + +def is_trusted_human_review_author(item, authenticated_login): + author = str(item.get("author") or "") + if not author: + return False + if authenticated_login and author == authenticated_login: + return True + association = str(item.get("author_association") or "").upper() + return association in TRUSTED_AUTHOR_ASSOCIATIONS + + +def fetch_new_review_items(pr, state, fresh_state, authenticated_login=None): + repo = pr["repo"] + pr_number = pr["number"] + endpoints = comment_endpoints(repo, pr_number) + + issue_payload = gh_api_list_paginated(endpoints["issue_comment"], repo=repo) + review_comment_payload = gh_api_list_paginated(endpoints["review_comment"], repo=repo) + review_payload = gh_api_list_paginated(endpoints["review"], repo=repo) + + issue_items = normalize_issue_comments(issue_payload) + review_comment_items = normalize_review_comments(review_comment_payload) + review_items = normalize_reviews(review_payload) + all_items = issue_items + review_comment_items + review_items + + seen_issue = {str(x) for x in state.get("seen_issue_comment_ids") or []} + seen_review_comment = {str(x) for x in state.get("seen_review_comment_ids") or []} + seen_review = {str(x) for x in state.get("seen_review_ids") or []} + + # On a brand-new state file, surface existing review activity instead of + # silently treating it as seen. This avoids missing already-pending review + # feedback when monitoring starts after comments were posted. + + new_items = [] + for item in all_items: + item_id = item.get("id") + if not item_id: + continue + author = item.get("author") or "" + if not author: + continue + if is_bot_login(author): + if not is_actionable_review_bot_login(author): + continue + elif not is_trusted_human_review_author(item, authenticated_login): + continue + + kind = item["kind"] + if kind == "issue_comment" and item_id in seen_issue: + continue + if kind == "review_comment" and item_id in seen_review_comment: + continue + if kind == "review" and item_id in seen_review: + continue + + new_items.append(item) + if kind == "issue_comment": + seen_issue.add(item_id) + elif kind == "review_comment": + seen_review_comment.add(item_id) + elif kind == "review": + seen_review.add(item_id) + + new_items.sort(key=lambda item: (item.get("created_at") or "", item.get("kind") or "", item.get("id") or "")) + state["seen_issue_comment_ids"] = sorted(seen_issue) + state["seen_review_comment_ids"] = sorted(seen_review_comment) + state["seen_review_ids"] = sorted(seen_review) + return new_items + + +def current_retry_count(state, head_sha): + retries = state.get("retries_by_sha") or {} + value = retries.get(head_sha, 0) + try: + return int(value) + except (TypeError, ValueError): + return 0 + + +def set_retry_count(state, head_sha, count): + retries = state.get("retries_by_sha") + if not isinstance(retries, dict): + retries = {} + retries[head_sha] = int(count) + state["retries_by_sha"] = retries + + +def unique_actions(actions): + out = [] + seen = set() + for action in actions: + if action not in seen: + out.append(action) + seen.add(action) + return out + + +def is_pr_ready_to_merge(pr, checks_summary, new_review_items): + if pr["closed"] or pr["merged"]: + return False + if not checks_summary["all_terminal"]: + return False + if checks_summary["failed_count"] > 0 or checks_summary["pending_count"] > 0: + return False + if new_review_items: + return False + if str(pr.get("mergeable") or "") != "MERGEABLE": + return False + if str(pr.get("merge_state_status") or "") in MERGE_CONFLICT_OR_BLOCKING_STATES: + return False + if str(pr.get("review_decision") or "") in MERGE_BLOCKING_REVIEW_DECISIONS: + return False + return True + + +def recommend_actions(pr, checks_summary, failed_runs, new_review_items, retries_used, max_retries): + actions = [] + if pr["closed"] or pr["merged"]: + if new_review_items: + actions.append("process_review_comment") + actions.append("stop_pr_closed") + return unique_actions(actions) + + if is_pr_ready_to_merge(pr, checks_summary, new_review_items): + actions.append("stop_ready_to_merge") + return unique_actions(actions) + + if new_review_items: + actions.append("process_review_comment") + + has_failed_pr_checks = checks_summary["failed_count"] > 0 + if has_failed_pr_checks: + if checks_summary["all_terminal"] and retries_used >= max_retries: + actions.append("stop_exhausted_retries") + else: + actions.append("diagnose_ci_failure") + if checks_summary["all_terminal"] and failed_runs and retries_used < max_retries: + actions.append("retry_failed_checks") + + if not actions: + actions.append("idle") + return unique_actions(actions) + + +def collect_snapshot(args): + pr = resolve_pr(args.pr, repo_override=args.repo) + state_path = Path(args.state_file) if args.state_file else default_state_file_for(pr) + state, fresh_state = load_state(state_path) + + if not state.get("started_at"): + state["started_at"] = int(time.time()) + + # `gh pr checks -R ` requires an explicit PR/branch/url argument. + # After resolving `--pr auto`, reuse the concrete PR number. + checks = get_pr_checks(str(pr["number"]), repo=pr["repo"]) + checks_summary = summarize_checks(checks) + workflow_runs = get_workflow_runs_for_sha(pr["repo"], pr["head_sha"]) + failed_runs = failed_runs_from_workflow_runs(workflow_runs, pr["head_sha"]) + authenticated_login = get_authenticated_login() + new_review_items = fetch_new_review_items( + pr, + state, + fresh_state=fresh_state, + authenticated_login=authenticated_login, + ) + + retries_used = current_retry_count(state, pr["head_sha"]) + actions = recommend_actions( + pr, + checks_summary, + failed_runs, + new_review_items, + retries_used, + args.max_flaky_retries, + ) + + state["pr"] = {"repo": pr["repo"], "number": pr["number"]} + state["last_seen_head_sha"] = pr["head_sha"] + state["last_snapshot_at"] = int(time.time()) + save_state(state_path, state) + + snapshot = { + "pr": pr, + "checks": checks_summary, + "failed_runs": failed_runs, + "new_review_items": new_review_items, + "actions": actions, + "retry_state": { + "current_sha_retries_used": retries_used, + "max_flaky_retries": args.max_flaky_retries, + }, + } + return snapshot, state_path + + +def retry_failed_now(args): + snapshot, state_path = collect_snapshot(args) + pr = snapshot["pr"] + checks_summary = snapshot["checks"] + failed_runs = snapshot["failed_runs"] + retries_used = snapshot["retry_state"]["current_sha_retries_used"] + max_retries = snapshot["retry_state"]["max_flaky_retries"] + + result = { + "snapshot": snapshot, + "state_file": str(state_path), + "rerun_attempted": False, + "rerun_count": 0, + "rerun_run_ids": [], + "reason": None, + } + + if pr["closed"] or pr["merged"]: + result["reason"] = "pr_closed" + return result + if checks_summary["failed_count"] <= 0: + result["reason"] = "no_failed_pr_checks" + return result + if not failed_runs: + result["reason"] = "no_failed_runs" + return result + if not checks_summary["all_terminal"]: + result["reason"] = "checks_still_pending" + return result + if retries_used >= max_retries: + result["reason"] = "retry_budget_exhausted" + return result + + for run in failed_runs: + run_id = run.get("run_id") + if run_id in (None, ""): + continue + gh_text(["run", "rerun", str(run_id), "--failed"], repo=pr["repo"]) + result["rerun_run_ids"].append(run_id) + + if result["rerun_run_ids"]: + state, _ = load_state(state_path) + new_count = current_retry_count(state, pr["head_sha"]) + 1 + set_retry_count(state, pr["head_sha"], new_count) + state["last_snapshot_at"] = int(time.time()) + save_state(state_path, state) + result["rerun_attempted"] = True + result["rerun_count"] = len(result["rerun_run_ids"]) + result["reason"] = "rerun_triggered" + else: + result["reason"] = "failed_runs_missing_ids" + + return result + + +def print_json(obj): + sys.stdout.write(json.dumps(obj, sort_keys=True) + "\n") + sys.stdout.flush() + + +def print_event(event, payload): + print_json({"event": event, "payload": payload}) + + +def is_ci_green(snapshot): + checks = snapshot.get("checks") or {} + return ( + bool(checks.get("all_terminal")) + and int(checks.get("failed_count") or 0) == 0 + and int(checks.get("pending_count") or 0) == 0 + ) + + +def snapshot_change_key(snapshot): + pr = snapshot.get("pr") or {} + checks = snapshot.get("checks") or {} + review_items = snapshot.get("new_review_items") or [] + return ( + str(pr.get("head_sha") or ""), + str(pr.get("state") or ""), + str(pr.get("mergeable") or ""), + str(pr.get("merge_state_status") or ""), + str(pr.get("review_decision") or ""), + int(checks.get("passed_count") or 0), + int(checks.get("failed_count") or 0), + int(checks.get("pending_count") or 0), + tuple( + (str(item.get("kind") or ""), str(item.get("id") or "")) + for item in review_items + if isinstance(item, dict) + ), + tuple(snapshot.get("actions") or []), + ) + + +def run_watch(args): + poll_seconds = args.poll_seconds + last_change_key = None + while True: + snapshot, state_path = collect_snapshot(args) + print_event( + "snapshot", + { + "snapshot": snapshot, + "state_file": str(state_path), + "next_poll_seconds": poll_seconds, + }, + ) + actions = set(snapshot.get("actions") or []) + if ( + "stop_pr_closed" in actions + or "stop_exhausted_retries" in actions + or "stop_ready_to_merge" in actions + ): + print_event("stop", {"actions": snapshot.get("actions"), "pr": snapshot.get("pr")}) + return 0 + + current_change_key = snapshot_change_key(snapshot) + changed = current_change_key != last_change_key + green = is_ci_green(snapshot) + + if not green: + poll_seconds = args.poll_seconds + elif changed or last_change_key is None: + poll_seconds = args.poll_seconds + else: + poll_seconds = min(poll_seconds * 2, GREEN_STATE_MAX_POLL_SECONDS) + + last_change_key = current_change_key + time.sleep(poll_seconds) + + +def main(): + args = parse_args() + try: + if args.retry_failed_now: + print_json(retry_failed_now(args)) + return 0 + if args.watch: + return run_watch(args) + snapshot, state_path = collect_snapshot(args) + snapshot["state_file"] = str(state_path) + print_json(snapshot) + return 0 + except (GhCommandError, RuntimeError, ValueError) as err: + sys.stderr.write(f"gh_pr_watch.py error: {err}\n") + return 1 + except KeyboardInterrupt: + sys.stderr.write("gh_pr_watch.py interrupted\n") + return 130 + + +if __name__ == "__main__": + raise SystemExit(main()) From e8949f45070dfed64e0e1bc47469180177432616 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 22 Feb 2026 19:39:56 -0800 Subject: [PATCH 02/29] test: vendor zsh fork via DotSlash and stabilize zsh-fork tests (#12518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why The zsh integration tests were still brittle in two ways: - they relied on `CODEX_TEST_ZSH_PATH` / environment-specific setup, so they often did not exercise the patched zsh fork that `shell-tool-mcp` ships - once the tests consistently used the vendored zsh fork, they exposed real Linux-specific zsh-fork issues in CI In particular, the Linux failures were not just test noise: - the zsh-fork launch path was dropping `ExecRequest.arg0`, so Linux `codex-linux-sandbox` arg0 dispatch did not run and zsh wrapper-mode could receive malformed arguments - the `turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2` test uses the zsh exec bridge (which talks to the parent over a Unix socket), but Linux restricted sandbox seccomp denies `connect(2)`, causing timeouts on `ubuntu-24.04` x86/arm This PR makes the zsh tests consistently run against the intended vendored zsh fork and fixes/hardens the zsh-fork path so the Linux CI signal is meaningful. ## What Changed - Added a single shared test-only DotSlash file for the patched zsh fork at `codex-rs/exec-server/tests/suite/zsh` (analogous to the existing `bash` test resource). - Updated both app-server and exec-server zsh tests to use that shared DotSlash zsh (no duplicate zsh DotSlash file, no `CODEX_TEST_ZSH_PATH` dependency). - Updated the app-server zsh-fork test helper to resolve the shared DotSlash zsh and avoid silently falling back to host zsh. - Kept the app-server zsh-fork tests configured via `config.toml`, using a test wrapper path where needed to force `zsh -df` (and rewrite `-lc` to `-c`) for the subcommand-decline test. - Hardened the app-server subcommand-decline zsh-fork test for CI variability: - tolerate an extra `/responses` POST with a no-op mock response - tolerate non-target approval ordering while remaining strict on the two `/usr/bin/true` approvals and decline behavior - use `DangerFullAccess` on Linux for this one test because it validates zsh approval flow, not Linux sandbox socket restrictions - Fixed zsh-fork process launching on Linux by preserving `req.arg0` in `ZshExecBridge::execute_shell_request(...)` so `codex-linux-sandbox` arg0 dispatch continues to work. - Moved `maybe_run_zsh_exec_wrapper_mode()` under `arg0_dispatch_or_else(...)` in `app-server` and `cli` so wrapper-mode handling coexists correctly with arg0-dispatched helper modes. - Consolidated duplicated `dotslash -- fetch` resolution logic into shared test support (`core/tests/common/lib.rs`). - Updated `codex-rs/exec-server/tests/suite/accept_elicitation.rs` to use DotSlash zsh and hardened the zsh elicitation test for Bazel/zsh differences by: - resolving an absolute `git` path - running `git init --quiet .` - asserting success / `.git` creation instead of relying on banner text ## Verification - `cargo test -p codex-app-server turn_start_zsh_fork -- --nocapture` - `cargo test -p codex-exec-server accept_elicitation -- --nocapture` - `bazel test //codex-rs/exec-server:exec-server-all-test --test_output=streamed --test_arg=--nocapture --test_arg=accept_elicitation_for_prompt_rule_with_zsh` - CI (`rust-ci`) on the final cleaned commit: `Tests — ubuntu-24.04 - x86_64-unknown-linux-gnu` and `Tests — ubuntu-24.04-arm - aarch64-unknown-linux-gnu` passed in [run 22291424358](https://github.com/openai/codex/actions/runs/22291424358) --- codex-rs/Cargo.lock | 1 + codex-rs/app-server/src/main.rs | 8 +- .../tests/suite/v2/turn_start_zsh_fork.rs | 231 ++++++++++++------ codex-rs/cli/src/main.rs | 8 +- codex-rs/core/src/zsh_exec_bridge/mod.rs | 5 +- codex-rs/core/tests/common/lib.rs | 38 +++ codex-rs/exec-server/Cargo.toml | 1 + .../tests/suite/accept_elicitation.rs | 39 +-- codex-rs/exec-server/tests/suite/zsh | 72 ++++++ 9 files changed, 305 insertions(+), 98 deletions(-) create mode 100755 codex-rs/exec-server/tests/suite/zsh diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c910f95442..e68e85b40d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1787,6 +1787,7 @@ dependencies = [ "codex-protocol", "codex-shell-command", "codex-utils-cargo-bin", + "core_test_support", "exec_server_test_support", "libc", "maplit", diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index b4c5d098e4..c56c3c8f98 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -23,10 +23,12 @@ struct AppServerArgs { } fn main() -> anyhow::Result<()> { - if codex_core::maybe_run_zsh_exec_wrapper_mode()? { - return Ok(()); - } arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { + // Run wrapper mode only after arg0 dispatch so `codex-linux-sandbox` + // invocations don't get misclassified as zsh exec-wrapper calls. + if codex_core::maybe_run_zsh_exec_wrapper_mode()? { + return Ok(()); + } let args = AppServerArgs::parse(); let managed_config_path = managed_config_path_from_debug_env(); let loader_overrides = LoaderOverrides { diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index 94467b8719..e989807ea9 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -2,18 +2,15 @@ // // Running these tests with the patched zsh fork: // -// The suite uses `CODEX_TEST_ZSH_PATH` when set. Example: -// CODEX_TEST_ZSH_PATH="$HOME/.local/codex-zsh-77045ef/bin/zsh" \ -// cargo test -p codex-app-server turn_start_zsh_fork -- --nocapture -// -// For a single test: -// CODEX_TEST_ZSH_PATH="$HOME/.local/codex-zsh-77045ef/bin/zsh" \ -// cargo test -p codex-app-server turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2 -- --nocapture +// The suite resolves the shared test-only zsh DotSlash file at +// `exec-server/tests/suite/zsh` via DotSlash on first use, so `dotslash` and +// network access are required the first time the artifact is fetched. use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_responses_server_sequence; +use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::create_shell_command_sse_response; use app_test_support::to_response; use codex_app_server_protocol::CommandExecutionApprovalDecision; @@ -38,6 +35,7 @@ use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::os::unix::fs::PermissionsExt; use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; @@ -57,7 +55,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { let workspace = tmp.path().join("workspace"); std::fs::create_dir(&workspace)?; - let Some(zsh_path) = find_test_zsh_path() else { + let Some(zsh_path) = find_test_zsh_path()? else { eprintln!("skipping zsh fork test: no zsh executable found"); return Ok(()); }; @@ -82,7 +80,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { &zsh_path, )?; - let mut mcp = McpProcess::new(&codex_home).await?; + let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let start_id = mcp @@ -167,7 +165,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> { let workspace = tmp.path().join("workspace"); std::fs::create_dir(&workspace)?; - let Some(zsh_path) = find_test_zsh_path() else { + let Some(zsh_path) = find_test_zsh_path()? else { eprintln!("skipping zsh fork decline test: no zsh executable found"); return Ok(()); }; @@ -199,7 +197,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> { &zsh_path, )?; - let mut mcp = McpProcess::new(&codex_home).await?; + let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let start_id = mcp @@ -303,7 +301,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> { let workspace = tmp.path().join("workspace"); std::fs::create_dir(&workspace)?; - let Some(zsh_path) = find_test_zsh_path() else { + let Some(zsh_path) = find_test_zsh_path()? else { eprintln!("skipping zsh fork cancel test: no zsh executable found"); return Ok(()); }; @@ -332,7 +330,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> { &zsh_path, )?; - let mut mcp = McpProcess::new(&codex_home).await?; + let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let start_id = mcp @@ -434,7 +432,7 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() let workspace = tmp.path().join("workspace"); std::fs::create_dir(&workspace)?; - let Some(zsh_path) = find_test_zsh_path() else { + let Some(zsh_path) = find_test_zsh_path()? else { eprintln!("skipping zsh fork subcommand decline test: no zsh executable found"); return Ok(()); }; @@ -446,6 +444,29 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() return Ok(()); } eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); + let zsh_path_for_config = { + // App-server config accepts only a zsh path, not extra argv. Use a + // wrapper so this test can force `-df` and downgrade `-lc` to `-c` + // to avoid rc/login-shell startup noise. + let path = workspace.join("zsh-no-rc"); + std::fs::write( + &path, + format!( + r#"#!/bin/sh +if [ "$1" = "-lc" ]; then + shift + set -- -c "$@" +fi +exec "{}" -df "$@" +"#, + zsh_path.display() + ), + )?; + let mut permissions = std::fs::metadata(&path)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&path, permissions)?; + path + }; let tool_call_arguments = serde_json::to_string(&serde_json::json!({ "command": "/usr/bin/true && /usr/bin/true", @@ -461,7 +482,16 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() ), responses::ev_completed("resp-1"), ]); - let server = create_mock_responses_server_sequence(vec![response]).await; + let no_op_response = responses::sse(vec![ + responses::ev_response_created("resp-2"), + responses::ev_completed("resp-2"), + ]); + // Linux CI has occasionally issued a second `/responses` POST after the + // subcommand-decline flow. This test is about approval/decline behavior in + // the zsh fork, not exact model request count, so allow an extra request + // and return a harmless no-op response if it arrives. + let server = + create_mock_responses_server_sequence_unchecked(vec![response, no_op_response]).await; create_config_toml( &codex_home, &server.uri(), @@ -471,10 +501,10 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() (Feature::UnifiedExec, false), (Feature::ShellSnapshot, false), ]), - &zsh_path, + &zsh_path_for_config, )?; - let mut mcp = McpProcess::new(&codex_home).await?; + let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let start_id = mcp @@ -500,8 +530,16 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() }], cwd: Some(workspace.clone()), approval_policy: Some(codex_app_server_protocol::AskForApproval::OnRequest), - sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::ReadOnly { - access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, + sandbox_policy: Some(if cfg!(target_os = "linux") { + // The zsh exec-bridge wrapper uses a Unix socket back to the parent + // process. Linux restricted sandbox seccomp denies connect(2), so use + // full access here; this test is validating zsh approval/decline + // behavior, not Linux sandboxing. + codex_app_server_protocol::SandboxPolicy::DangerFullAccess + } else { + codex_app_server_protocol::SandboxPolicy::ReadOnly { + access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, + } }), model: Some("mock-model".to_string()), effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), @@ -517,10 +555,13 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() let TurnStartResponse { turn } = to_response::(turn_resp)?; let mut approval_ids = Vec::new(); - for decision in [ + let mut saw_parent_approval = false; + let target_decisions = [ CommandExecutionApprovalDecision::Accept, CommandExecutionApprovalDecision::Cancel, - ] { + ]; + let mut target_decision_index = 0; + while target_decision_index < target_decisions.len() { let server_req = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_request_message(), @@ -531,13 +572,40 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() panic!("expected CommandExecutionRequestApproval request"); }; assert_eq!(params.item_id, "call-zsh-fork-subcommand-decline"); - approval_ids.push( - params - .approval_id - .clone() - .expect("approval_id must be present for zsh subcommand approvals"), - ); assert_eq!(params.thread_id, thread.id); + let is_target_subcommand = params.command.as_deref() == Some("/usr/bin/true"); + if is_target_subcommand { + approval_ids.push( + params + .approval_id + .clone() + .expect("approval_id must be present for zsh subcommand approvals"), + ); + } + let decision = if is_target_subcommand { + let decision = target_decisions[target_decision_index].clone(); + target_decision_index += 1; + decision + } else { + let command = params + .command + .as_deref() + .expect("approval command should be present"); + assert!( + !saw_parent_approval, + "unexpected extra non-target approval: {command}" + ); + assert!( + command.contains("zsh-no-rc"), + "expected parent zsh wrapper approval, got: {command}" + ); + assert!( + command.contains("/usr/bin/true && /usr/bin/true"), + "expected tool command in parent approval, got: {command}" + ); + saw_parent_approval = true; + CommandExecutionApprovalDecision::Accept + }; mcp.send_response( request_id, serde_json::to_value(CommandExecutionRequestApprovalResponse { decision })?, @@ -545,6 +613,8 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() .await?; } + assert_eq!(approval_ids.len(), 2); + assert_ne!(approval_ids[0], approval_ids[1]); let parent_completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { loop { let completed_notif = mcp @@ -563,32 +633,61 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() } } }) - .await??; + .await; - let ThreadItem::CommandExecution { - id, - status, - aggregated_output, - .. - } = parent_completed_command_execution - else { - unreachable!("loop ensures we break on parent command execution item"); - }; - assert_eq!(id, "call-zsh-fork-subcommand-decline"); - assert_eq!(status, CommandExecutionStatus::Declined); - assert!( - aggregated_output.is_none() - || aggregated_output == Some("exec command rejected by user".to_string()) - ); - assert_eq!(approval_ids.len(), 2); - assert_ne!(approval_ids[0], approval_ids[1]); + match parent_completed_command_execution { + Ok(Ok(parent_completed_command_execution)) => { + let ThreadItem::CommandExecution { + id, + status, + aggregated_output, + .. + } = parent_completed_command_execution + else { + unreachable!("loop ensures we break on parent command execution item"); + }; + assert_eq!(id, "call-zsh-fork-subcommand-decline"); + assert_eq!(status, CommandExecutionStatus::Declined); + assert!( + aggregated_output.is_none() + || aggregated_output == Some("exec command rejected by user".to_string()) + ); - mcp.interrupt_turn_and_wait_for_aborted(thread.id, turn.id, DEFAULT_READ_TIMEOUT) - .await?; + mcp.interrupt_turn_and_wait_for_aborted( + thread.id.clone(), + turn.id.clone(), + DEFAULT_READ_TIMEOUT, + ) + .await?; + } + Ok(Err(error)) => return Err(error), + Err(_) => { + // Some zsh builds abort the turn immediately after the rejected + // subcommand without emitting a parent `item/completed`. + let completed_notif = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + let completed: TurnCompletedNotification = serde_json::from_value( + completed_notif + .params + .expect("turn/completed params must be present"), + )?; + assert_eq!(completed.thread_id, thread.id); + assert_eq!(completed.turn.id, turn.id); + assert_eq!(completed.turn.status, TurnStatus::Interrupted); + } + } Ok(()) } +async fn create_zsh_test_mcp_process(codex_home: &Path, zdotdir: &Path) -> Result { + let zdotdir = zdotdir.to_string_lossy().into_owned(); + McpProcess::new_with_env(codex_home, &[("ZDOTDIR", Some(zdotdir.as_str()))]).await +} + fn create_config_toml( codex_home: &Path, server_uri: &str, @@ -640,36 +739,24 @@ stream_max_retries = 0 ) } -fn find_test_zsh_path() -> Option { - if let Some(path) = std::env::var_os("CODEX_TEST_ZSH_PATH") { - let path = std::path::PathBuf::from(path); - if path.is_file() { - return Some(path); - } - panic!( - "CODEX_TEST_ZSH_PATH is set but is not a file: {}", - path.display() +fn find_test_zsh_path() -> Result> { + let repo_root = codex_utils_cargo_bin::repo_root()?; + let dotslash_zsh = repo_root.join("codex-rs/exec-server/tests/suite/zsh"); + if !dotslash_zsh.is_file() { + eprintln!( + "skipping zsh fork test: shared zsh DotSlash file not found at {}", + dotslash_zsh.display() ); + return Ok(None); } - - for candidate in ["/bin/zsh", "/usr/bin/zsh"] { - let path = Path::new(candidate); - if path.is_file() { - return Some(path.to_path_buf()); + match core_test_support::fetch_dotslash_file(&dotslash_zsh, None) { + Ok(path) => return Ok(Some(path)), + Err(error) => { + eprintln!("failed to fetch vendored zsh via dotslash: {error:#}"); } } - let shell = std::env::var_os("SHELL")?; - let shell_path = std::path::PathBuf::from(shell); - if shell_path - .file_name() - .is_some_and(|file_name| file_name == "zsh") - && shell_path.is_file() - { - return Some(shell_path); - } - - None + Ok(None) } fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool { diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index e5478f1b90..cdecb18dca 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -543,10 +543,12 @@ fn stage_str(stage: codex_core::features::Stage) -> &'static str { } fn main() -> anyhow::Result<()> { - if codex_core::maybe_run_zsh_exec_wrapper_mode()? { - return Ok(()); - } arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { + // Run wrapper mode only after arg0 dispatch so `codex-linux-sandbox` + // invocations don't get misclassified as zsh exec-wrapper calls. + if codex_core::maybe_run_zsh_exec_wrapper_mode()? { + return Ok(()); + } cli_main(codex_linux_sandbox_exe).await?; Ok(()) }) diff --git a/codex-rs/core/src/zsh_exec_bridge/mod.rs b/codex-rs/core/src/zsh_exec_bridge/mod.rs index 2e3821acfe..8094a4b786 100644 --- a/codex-rs/core/src/zsh_exec_bridge/mod.rs +++ b/codex-rs/core/src/zsh_exec_bridge/mod.rs @@ -166,6 +166,10 @@ impl ZshExecBridge { })?; let mut cmd = tokio::process::Command::new(&command[0]); + #[cfg(unix)] + if let Some(arg0) = &req.arg0 { + cmd.arg0(arg0); + } if command.len() > 1 { cmd.args(&command[1..]); } @@ -459,7 +463,6 @@ fn run_exec_wrapper_mode() -> anyhow::Result<()> { argv: argv.clone(), cwd, }; - let mut stream = StdUnixStream::connect(&socket_path) .with_context(|| format!("connect to wrapper socket at {socket_path}"))?; let encoded = serde_json::to_string(&request).context("serialize wrapper request")?; diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index a4422e4580..a26cd5ae77 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -1,5 +1,7 @@ #![expect(clippy::expect_used)] +use anyhow::Context as _; +use anyhow::ensure; use codex_utils_cargo_bin::CargoBinError; use ctor::ctor; use tempfile::TempDir; @@ -99,6 +101,42 @@ pub fn test_tmp_path_buf() -> PathBuf { test_tmp_path().into_path_buf() } +/// Fetch a DotSlash resource and return the resolved executable/file path. +pub fn fetch_dotslash_file( + dotslash_file: &std::path::Path, + dotslash_cache: Option<&std::path::Path>, +) -> anyhow::Result { + let mut command = std::process::Command::new("dotslash"); + command.arg("--").arg("fetch").arg(dotslash_file); + if let Some(dotslash_cache) = dotslash_cache { + command.env("DOTSLASH_CACHE", dotslash_cache); + } + let output = command.output().with_context(|| { + format!( + "failed to run dotslash to fetch resource {}", + dotslash_file.display() + ) + })?; + ensure!( + output.status.success(), + "dotslash fetch failed for {}: {}", + dotslash_file.display(), + String::from_utf8_lossy(&output.stderr).trim() + ); + let fetched_path = String::from_utf8(output.stdout) + .context("dotslash fetch output was not utf8")? + .trim() + .to_string(); + ensure!(!fetched_path.is_empty(), "dotslash fetch output was empty"); + let fetched_path = PathBuf::from(fetched_path); + ensure!( + fetched_path.is_file(), + "dotslash returned non-file path: {}", + fetched_path.display() + ); + Ok(fetched_path) +} + /// Returns a default `Config` whose on-disk state is confined to the provided /// temporary directory. Using a per-test directory keeps tests hermetic and /// avoids clobbering a developer’s real `~/.codex`. diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 4d9d79270d..9b40cd35d7 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -58,6 +58,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } [dev-dependencies] +core_test_support = { workspace = true } codex-utils-cargo-bin = { workspace = true } codex-protocol = { workspace = true } exec_server_test_support = { workspace = true } diff --git a/codex-rs/exec-server/tests/suite/accept_elicitation.rs b/codex-rs/exec-server/tests/suite/accept_elicitation.rs index 32f30963ab..865001ac6f 100644 --- a/codex-rs/exec-server/tests/suite/accept_elicitation.rs +++ b/codex-rs/exec-server/tests/suite/accept_elicitation.rs @@ -61,15 +61,9 @@ prefix_rule( /// Verify the same prompt/escalation flow works when the server is launched /// with a patched zsh binary. /// -/// Set CODEX_TEST_ZSH_PATH to enable this test locally or in CI. +/// The suite resolves `tests/suite/zsh` via DotSlash on first use. #[tokio::test(flavor = "current_thread")] async fn accept_elicitation_for_prompt_rule_with_zsh() -> Result<()> { - let Some(zsh_path) = std::env::var_os("CODEX_TEST_ZSH_PATH") else { - eprintln!("skipping zsh test: CODEX_TEST_ZSH_PATH is not set"); - return Ok(()); - }; - let zsh_path = PathBuf::from(zsh_path); - let codex_home = TempDir::new()?; write_default_execpolicy( r#" @@ -87,6 +81,11 @@ prefix_rule( .await?; let dotslash_cache_temp_dir = TempDir::new()?; let dotslash_cache = dotslash_cache_temp_dir.path(); + let zsh_path = resolve_test_zsh_path(dotslash_cache).await?; + eprintln!( + "using zsh path for exec-server test: {}", + zsh_path.display() + ); let transport = create_transport_with_shell_path(codex_home.as_ref(), dotslash_cache, &zsh_path).await?; run_accept_elicitation_for_prompt_rule_with_transport(transport).await @@ -95,13 +94,13 @@ prefix_rule( async fn run_accept_elicitation_for_prompt_rule_with_transport( transport: rmcp::transport::TokioChildProcess, ) -> Result<()> { - // Create an MCP client that approves expected elicitation messages. + // Create an MCP client that approves the expected elicitation message. let project_root = TempDir::new()?; let project_root_path = project_root.path().canonicalize().unwrap(); let git_path = resolve_git_path(USE_LOGIN_SHELL).await?; + let git_init_command = format!("{git_path} init --quiet ."); let expected_elicitation_message = format!( - "Allow agent to run `{} init .` in `{}`?", - git_path, + "Allow agent to run `{git_path} init --quiet .` in `{}`?", project_root_path.display() ); let elicitation_requests: Arc>> = Default::default(); @@ -142,7 +141,7 @@ async fn run_accept_elicitation_for_prompt_rule_with_transport( arguments: Some(object(json!( { "login": USE_LOGIN_SHELL, - "command": "git init .", + "command": git_init_command, "workdir": project_root_path.to_string_lossy(), } ))), @@ -157,15 +156,11 @@ async fn run_accept_elicitation_for_prompt_rule_with_transport( let ExecResult { exit_code, output, .. } = serde_json::from_str::(&tool_call_content.text)?; - let git_init_succeeded = format!( - "Initialized empty Git repository in {}/.git/\n", - project_root_path.display() - ); - // Normally, this would be an exact match, but it might include extra output - // if `git config set advice.defaultBranchName false` has not been set. + // `git init --quiet` is expected to suppress the usual initialization + // banner, so assert on success and filesystem effects instead of output. assert!( - output.contains(&git_init_succeeded), - "expected output `{output}` to contain `{git_init_succeeded}`" + output.is_empty(), + "expected no output from `git init --quiet .`, got `{output}`" ); assert_eq!(exit_code, 0, "command should succeed"); assert_eq!(is_error, Some(false), "command should succeed"); @@ -192,6 +187,12 @@ async fn run_accept_elicitation_for_prompt_rule_with_transport( Ok(()) } +async fn resolve_test_zsh_path(dotslash_cache: &std::path::Path) -> Result { + let dotslash_zsh = codex_utils_cargo_bin::find_resource!("tests/suite/zsh")?; + core_test_support::fetch_dotslash_file(&dotslash_zsh, Some(dotslash_cache)) + .with_context(|| format!("failed to fetch test zsh from {}", dotslash_zsh.display())) +} + fn ensure_codex_cli() -> Result { let codex_cli = codex_utils_cargo_bin::cargo_bin("codex")?; diff --git a/codex-rs/exec-server/tests/suite/zsh b/codex-rs/exec-server/tests/suite/zsh new file mode 100755 index 0000000000..34295b5c63 --- /dev/null +++ b/codex-rs/exec-server/tests/suite/zsh @@ -0,0 +1,72 @@ +#!/usr/bin/env dotslash + +// This is the patched zsh fork built by +// `.github/workflows/shell-tool-mcp.yml` for the shell-tool-mcp package. +// Fetching the prebuilt version via DotSlash makes it easier to write +// integration tests that exercise the zsh fork behavior in exec-server tests. +// +// TODO(mbolin): Currently, we use a .tgz artifact that includes binaries for +// multiple platforms, but we could save a bit of space by making arch-specific +// artifacts available in the GitHub releases and referencing those here. +{ + "name": "codex-zsh", + "platforms": { + // macOS 13 builds (and therefore x86_64) were dropped in + // https://github.com/openai/codex/pull/7295, so we only provide an + // Apple Silicon build for now. + "macos-aarch64": { + "size": 53771483, + "hash": "blake3", + "digest": "ff664f63f5e1fa62762c9aff0aafa66cf196faf9b157f98ec98f59c152fc7bd3", + "format": "tar.gz", + "path": "package/vendor/aarch64-apple-darwin/zsh/macos-15/zsh", + "providers": [ + { + "url": "https://github.com/openai/codex/releases/download/rust-v0.104.0/codex-shell-tool-mcp-npm-0.104.0.tgz" + }, + { + "type": "github-release", + "repo": "openai/codex", + "tag": "rust-v0.104.0", + "name": "codex-shell-tool-mcp-npm-0.104.0.tgz" + } + ] + }, + "linux-x86_64": { + "size": 53771483, + "hash": "blake3", + "digest": "ff664f63f5e1fa62762c9aff0aafa66cf196faf9b157f98ec98f59c152fc7bd3", + "format": "tar.gz", + "path": "package/vendor/x86_64-unknown-linux-musl/zsh/ubuntu-24.04/zsh", + "providers": [ + { + "url": "https://github.com/openai/codex/releases/download/rust-v0.104.0/codex-shell-tool-mcp-npm-0.104.0.tgz" + }, + { + "type": "github-release", + "repo": "openai/codex", + "tag": "rust-v0.104.0", + "name": "codex-shell-tool-mcp-npm-0.104.0.tgz" + } + ] + }, + "linux-aarch64": { + "size": 53771483, + "hash": "blake3", + "digest": "ff664f63f5e1fa62762c9aff0aafa66cf196faf9b157f98ec98f59c152fc7bd3", + "format": "tar.gz", + "path": "package/vendor/aarch64-unknown-linux-musl/zsh/ubuntu-24.04/zsh", + "providers": [ + { + "url": "https://github.com/openai/codex/releases/download/rust-v0.104.0/codex-shell-tool-mcp-npm-0.104.0.tgz" + }, + { + "type": "github-release", + "repo": "openai/codex", + "tag": "rust-v0.104.0", + "name": "codex-shell-tool-mcp-npm-0.104.0.tgz" + } + ] + }, + } +} From 335a4e1cbceb96c280e070729b8759af045b6211 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Sun, 22 Feb 2026 23:00:08 -0800 Subject: [PATCH 03/29] Return image content from view_image (#12553) Responses API supports image content --- .../core/src/tools/handlers/view_image.rs | 33 ++++--- codex-rs/core/tests/suite/view_image.rs | 91 ++++++++----------- 2 files changed, 55 insertions(+), 69 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index cfcd5ec2ab..6337cef3e0 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -1,5 +1,6 @@ use async_trait::async_trait; use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::openai_models::InputModality; use serde::Deserialize; use tokio::fs; @@ -14,7 +15,6 @@ use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use codex_protocol::models::ContentItem; -use codex_protocol::models::ResponseInputItem; use codex_protocol::models::local_image_content_items_with_label_number; pub struct ViewImageHandler; @@ -81,21 +81,20 @@ impl ToolHandler for ViewImageHandler { } let event_path = abs_path.clone(); - let content: Vec = - local_image_content_items_with_label_number(&abs_path, None); - let input = ResponseInputItem::Message { - role: "user".to_string(), - content, - }; - - session - .inject_response_items(vec![input]) - .await - .map_err(|_| { - FunctionCallError::RespondToModel( - "unable to attach image (no active task)".to_string(), - ) - })?; + let content = local_image_content_items_with_label_number(&abs_path, None) + .into_iter() + .map(|item| match item { + ContentItem::InputText { text } => { + FunctionCallOutputContentItem::InputText { text } + } + ContentItem::InputImage { image_url } => { + FunctionCallOutputContentItem::InputImage { image_url } + } + ContentItem::OutputText { text } => { + FunctionCallOutputContentItem::InputText { text } + } + }) + .collect(); session .send_event( @@ -108,7 +107,7 @@ impl ToolHandler for ViewImageHandler { .await; Ok(ToolOutput::Function { - body: FunctionCallOutputBody::Text("attached local image path".to_string()), + body: FunctionCallOutputBody::ContentItems(content), success: Some(true), }) } diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 96315e5c75..5d2c8e3d5c 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -237,40 +237,29 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { let req = mock.single_request(); let body = req.body_json(); - let output_text = req - .function_call_output_content_and_success(call_id) - .and_then(|(content, _)| content) - .expect("output text present"); - assert_eq!(output_text, "attached local image path"); + assert!( + find_image_message(&body).is_none(), + "view_image tool should not inject a separate image message" + ); - let image_message = - find_image_message(&body).expect("pending input image message not included in request"); - let content_items = image_message - .get("content") + let function_output = req.function_call_output(call_id); + let output_items = function_output + .get("output") .and_then(Value::as_array) - .expect("image message has content array"); + .expect("function_call_output should be a content item array"); assert_eq!( - content_items.len(), + output_items.len(), 1, - "view_image should inject only the image content item (no tag/label text)" + "view_image should return only the image content item (no tag/label text)" ); assert_eq!( - content_items[0].get("type").and_then(Value::as_str), + output_items[0].get("type").and_then(Value::as_str), Some("input_image"), - "view_image should inject only an input_image content item" + "view_image should return only an input_image content item" ); - let image_url = image_message - .get("content") - .and_then(Value::as_array) - .and_then(|content| { - content.iter().find_map(|span| { - if span.get("type").and_then(Value::as_str) == Some("input_image") { - span.get("image_url").and_then(Value::as_str) - } else { - None - } - }) - }) + let image_url = output_items[0] + .get("image_url") + .and_then(Value::as_str) .expect("image_url present"); let (prefix, encoded) = image_url @@ -535,38 +524,36 @@ async fn view_image_tool_placeholder_for_non_image_files() -> anyhow::Result<()> request.inputs_of_type("input_image").is_empty(), "non-image file should not produce an input_image message" ); + let function_output = request.function_call_output(call_id); + let output_items = function_output + .get("output") + .and_then(Value::as_array) + .expect("function_call_output should be a content item array"); + assert_eq!( + output_items.len(), + 1, + "non-image placeholder should be returned as a single content item" + ); + assert_eq!( + output_items[0].get("type").and_then(Value::as_str), + Some("input_text"), + "non-image placeholder should be returned as input_text" + ); + let placeholder = output_items[0] + .get("text") + .and_then(Value::as_str) + .expect("placeholder text present"); - let placeholder = request - .inputs_of_type("message") - .iter() - .find_map(|item| { - let content = item.get("content").and_then(Value::as_array)?; - content.iter().find_map(|span| { - if span.get("type").and_then(Value::as_str) == Some("input_text") { - let text = span.get("text").and_then(Value::as_str)?; - if text.contains("Codex could not read the local image at") - && text.contains("unsupported MIME type `application/json`") - { - return Some(text.to_string()); - } - } - None - }) - }) - .expect("placeholder text found"); - + assert!( + placeholder.contains("Codex could not read the local image at") + && placeholder.contains("unsupported MIME type `application/json`"), + "placeholder should describe the unsupported file type: {placeholder}" + ); assert!( placeholder.contains(&abs_path.display().to_string()), "placeholder should mention path: {placeholder}" ); - let output_text = mock - .single_request() - .function_call_output_content_and_success(call_id) - .and_then(|(content, _)| content) - .expect("output text present"); - assert_eq!(output_text, "attached local image path"); - Ok(()) } From 956f2f439ed2fc6a82e571adefe8dafdd7aaf7ed Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 23 Feb 2026 00:31:29 -0800 Subject: [PATCH 04/29] refactor: decouple MCP policy construction from escalate server (#12555) ## Why The current escalate path in `codex-rs/exec-server` still had policy creation coupled to MCP details, which makes it hard to reuse the shell execution flow outside the MCP server. This change is part of a broader goal to split MCP-specific behavior from shared escalation execution so other handlers (for example a future `ShellCommandHandler`) can reuse it without depending on MCP request context types. ## What changed - Added a new `EscalationPolicyFactory` abstraction in `mcp.rs`: - `crate`-relative path: `codex-rs/exec-server/src/posix/mcp.rs` - https://github.com/openai/codex/blob/main/codex-rs/exec-server/src/posix/mcp.rs#L87-L107 - Made `run_escalate_server` in `mcp.rs` accept a policy factory instead of constructing `McpEscalationPolicy` directly. - https://github.com/openai/codex/blob/main/codex-rs/exec-server/src/posix/mcp.rs#L178-L201 - Introduced `McpEscalationPolicyFactory` that stores MCP-only state (`RequestContext`, `preserve_program_paths`) and implements the new trait. - https://github.com/openai/codex/blob/main/codex-rs/exec-server/src/posix/mcp.rs#L100-L117 - Updated `shell()` to pass a `McpEscalationPolicyFactory` instance into `run_escalate_server`, so the server remains the MCP-specific wiring layer. - https://github.com/openai/codex/blob/main/codex-rs/exec-server/src/posix/mcp.rs#L163-L170 ## Verification - Build and test execution was not re-run in this pass; changes are limited to `mcp.rs` and preserve the existing escalation flow semantics by only extracting policy construction behind a factory. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12555). * #12556 * __->__ #12555 --- codex-rs/exec-server/src/posix/mcp.rs | 97 ++++++++++++++++++++++----- 1 file changed, 81 insertions(+), 16 deletions(-) diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index 5319a1f916..f413fed210 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -30,6 +31,7 @@ use tokio::sync::RwLock; use crate::posix::escalate_server::EscalateServer; use crate::posix::escalate_server::{self}; +use crate::posix::escalation_policy::EscalationPolicy; use crate::posix::mcp_escalation_policy::McpEscalationPolicy; use crate::posix::stopwatch::Stopwatch; @@ -85,6 +87,30 @@ pub struct ExecTool { sandbox_state: Arc>>, } +trait EscalationPolicyFactory { + type Policy: EscalationPolicy + Send + Sync + 'static; + + fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; +} + +struct McpEscalationPolicyFactory { + context: RequestContext, + preserve_program_paths: bool, +} + +impl EscalationPolicyFactory for McpEscalationPolicyFactory { + type Policy = McpEscalationPolicy; + + fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { + McpEscalationPolicy::new( + policy, + self.context.clone(), + stopwatch, + self.preserve_program_paths, + ) + } +} + #[tool_router] impl ExecTool { pub fn new( @@ -115,8 +141,6 @@ impl ExecTool { .timeout_ms .unwrap_or(codex_core::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS), ); - let stopwatch = Stopwatch::new(effective_timeout); - let cancel_token = stopwatch.cancellation_token(); let sandbox_state = self.sandbox_state .read() @@ -128,27 +152,68 @@ impl ExecTool { sandbox_cwd: PathBuf::from(¶ms.workdir), use_linux_sandbox_bwrap: false, }); - let escalate_server = EscalateServer::new( - self.bash_path.clone(), - self.execve_wrapper.clone(), - McpEscalationPolicy::new( - self.policy.clone(), + let result = run_escalate_server( + params, + sandbox_state, + &self.bash_path, + &self.execve_wrapper, + self.policy.clone(), + McpEscalationPolicyFactory { context, - stopwatch.clone(), - self.preserve_program_paths, - ), - ); - - let result = escalate_server - .exec(params, cancel_token, &sandbox_state) - .await - .map_err(|e| McpError::internal_error(e.to_string(), None))?; + preserve_program_paths: self.preserve_program_paths, + }, + effective_timeout, + ) + .await + .map_err(|e| McpError::internal_error(e.to_string(), None))?; Ok(CallToolResult::success(vec![Content::json( ExecResult::from(result), )?])) } } +/// Runs the escalate server to execute a shell command with potential +/// escalation of execve calls. +/// +/// - `exec_params` defines the shell command to run +/// - `sandbox_state` is the sandbox to use to run the shell program +/// - `shell_program` is the path to the shell program to run (e.g. /bin/bash) +/// - `execve_wrapper` is the path to the execve wrapper binary to use for +/// handling execve calls from the shell program. This is likely a symlink to +/// Codex using a special name. +/// - `policy` is the exec policy to use for deciding whether to allow or deny +/// execve calls from the shell program. +/// - `escalation_policy_factory` is a factory for creating an +/// `EscalationPolicy` to use for deciding whether to allow, deny, or prompt +/// the user for execve calls from the shell program. We use a factory here +/// because the `EscalationPolicy` may need to capture request-specific +/// context (e.g. the MCP request context) that is not available at the time +/// we create the `ExecTool`. +/// - `effective_timeout` is the timeout to use for running the shell command. +/// Implementations are encouraged to excludeany time spent prompting the +/// user. +async fn run_escalate_server( + exec_params: ExecParams, + sandbox_state: SandboxState, + shell_program: impl AsRef, + execve_wrapper: impl AsRef, + policy: Arc>, + escalation_policy_factory: impl EscalationPolicyFactory, + effective_timeout: Duration, +) -> anyhow::Result { + let stopwatch = Stopwatch::new(effective_timeout); + let cancel_token = stopwatch.cancellation_token(); + let escalate_server = EscalateServer::new( + shell_program.as_ref().to_path_buf(), + execve_wrapper.as_ref().to_path_buf(), + escalation_policy_factory.create_policy(policy, stopwatch), + ); + + escalate_server + .exec(exec_params, cancel_token, &sandbox_state) + .await +} + #[derive(Default)] pub struct CodexSandboxStateUpdateMethod; From 8e9312958d9c71fa8850605852595754bb522fed Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 08:49:41 +0000 Subject: [PATCH 05/29] chore: nit name (#12559) --- codex-rs/core/src/agent/builtins/monitor.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/core/src/agent/builtins/monitor.toml b/codex-rs/core/src/agent/builtins/monitor.toml index 5e48a4936c..927a83921f 100644 --- a/codex-rs/core/src/agent/builtins/monitor.toml +++ b/codex-rs/core/src/agent/builtins/monitor.toml @@ -1,7 +1,6 @@ background_terminal_max_timeout = 3600000 model_reasoning_effort = "low" developer_instructions="""You are a waiting agent. -You're name is Superman. Your role is to monitor the execution of a specific command or task and report its status only when it is finished. Behavior rules: From cfcbff4c482cb37f13975972cb62ff2f632f8387 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 10:28:24 +0000 Subject: [PATCH 06/29] chore: awaiter (#12562) --- .../builtins/{monitor.toml => awaiter.toml} | 20 +++++++++---------- codex-rs/core/src/agent/role.rs | 12 +++++------ 2 files changed, 16 insertions(+), 16 deletions(-) rename codex-rs/core/src/agent/builtins/{monitor.toml => awaiter.toml} (54%) diff --git a/codex-rs/core/src/agent/builtins/monitor.toml b/codex-rs/core/src/agent/builtins/awaiter.toml similarity index 54% rename from codex-rs/core/src/agent/builtins/monitor.toml rename to codex-rs/core/src/agent/builtins/awaiter.toml index 927a83921f..a34583c0c9 100644 --- a/codex-rs/core/src/agent/builtins/monitor.toml +++ b/codex-rs/core/src/agent/builtins/awaiter.toml @@ -1,35 +1,35 @@ background_terminal_max_timeout = 3600000 model_reasoning_effort = "low" -developer_instructions="""You are a waiting agent. -Your role is to monitor the execution of a specific command or task and report its status only when it is finished. +developer_instructions="""You are an awaiter. +Your role is to await the completion of a specific command or task and report its status only when it is finished. Behavior rules: 1. When given a command or task identifier, you must: - - Execute or monitor it using the appropriate tool - - Continue waiting until the task reaches a terminal state. + - Execute or await it using the appropriate tool + - Continue awaiting until the task reaches a terminal state. 2. You must NOT: - Modify the task. - Interpret or optimize the task. - Perform unrelated actions. - - Stop waiting unless explicitly instructed. + - Stop awaiting unless explicitly instructed. -3. Waiting behavior: +3. Awaiting behavior: - If the task is still running, continue polling using tool calls. - Use repeated tool calls if necessary. - Do not hallucinate completion. - - Use long timeouts when waiting for something. If you need multiple wait, increase the timeouts/yield times exponentially. + - Use long timeouts when awaiting for something. If you need multiple awaits, increase the timeouts/yield times exponentially. 4. If asked for status: - Return the current known status. - - Immediately resume waiting afterward. + - Immediately resume awaiting afterward. 5. Termination: - - Only exit waiting when: + - Only exit awaiting when: - The task completes successfully, OR - The task fails, OR - You receive an explicit stop instruction. You must behave deterministically and conservatively. -""" \ No newline at end of file +""" diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index f4bdb9d7af..a72ba23469 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -187,16 +187,16 @@ Rules: } ), ( - "monitor".to_string(), + "awaiter".to_string(), AgentRoleConfig { - description: Some(r#"Use a `monitor` agent EVERY TIME you must run a command that might take some time. + description: Some(r#"Use an `awaiter` agent EVERY TIME you must run a command that might take some time. This includes, but not only: * testing * monitoring of a long running process * explicit ask to wait for something -When YOU wait for the `monitor` agent to be done, use the largest possible timeout."#.to_string()), - config_file: Some("monitor.toml".to_string().parse().unwrap_or_default()), +When YOU wait for the `awaiter` agent to be done, use the largest possible timeout."#.to_string()), + config_file: Some("awaiter.toml".to_string().parse().unwrap_or_default()), } ) ]) @@ -207,10 +207,10 @@ When YOU wait for the `monitor` agent to be done, use the largest possible timeo /// Resolves a built-in role `config_file` path to embedded content. pub(super) fn config_file_contents(path: &Path) -> Option<&'static str> { const EXPLORER: &str = include_str!("builtins/explorer.toml"); - const MONITOR: &str = include_str!("builtins/monitor.toml"); + const AWAITER: &str = include_str!("builtins/awaiter.toml"); match path.to_str()? { "explorer.toml" => Some(EXPLORER), - "monitor.toml" => Some(MONITOR), + "awaiter.toml" => Some(AWAITER), _ => None, } } From 2b9d0c385fba4356ddea5bfa5f615f767ce34136 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 10:52:58 +0000 Subject: [PATCH 07/29] chore: add doc to memories (#12565) ] --- codex-rs/core/src/memories/README.md | 92 ++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 codex-rs/core/src/memories/README.md diff --git a/codex-rs/core/src/memories/README.md b/codex-rs/core/src/memories/README.md new file mode 100644 index 0000000000..c19a0c6806 --- /dev/null +++ b/codex-rs/core/src/memories/README.md @@ -0,0 +1,92 @@ +# Memories Pipeline (Core) + +This module runs a startup memory pipeline for eligible sessions. + +## When it runs + +The pipeline is triggered when a root session starts, and only if: + +- the session is not ephemeral +- the memory feature is enabled +- the session is not a sub-agent session +- the state DB is available + +It runs asynchronously in the background and executes two phases in order: Phase 1, then Phase 2. + +## Phase 1: Rollout Extraction (per-thread) + +Phase 1 finds recent eligible rollouts and extracts a structured memory from each one. + +Eligible rollouts are selected from the state DB using startup claim rules. In practice this means +the pipeline only considers rollouts that are: + +- from allowed interactive session sources +- within the configured age window +- idle long enough (to avoid summarizing still-active/fresh rollouts) +- not already owned by another in-flight phase-1 worker +- within startup scan/claim limits (bounded work per startup) + +What it does: + +- claims a bounded set of rollout jobs from the state DB (startup claim) +- filters rollout content down to memory-relevant response items +- sends each rollout to a model (in parallel, with a concurrency cap) +- expects structured output containing: + - a detailed `raw_memory` + - a compact `rollout_summary` + - an optional `rollout_slug` +- redacts secrets from the generated memory fields +- stores successful outputs back into the state DB as stage-1 outputs + +Concurrency / coordination: + +- Phase 1 runs multiple extraction jobs in parallel (with a fixed concurrency cap) so startup memory generation can process several rollouts at once. +- Each job is leased/claimed in the state DB before processing, which prevents duplicate work across concurrent workers/startups. +- Failed jobs are marked with retry backoff, so they are retried later instead of hot-looping. + +Job outcomes: + +- `succeeded` (memory produced) +- `succeeded_no_output` (valid run but nothing useful generated) +- `failed` (with retry backoff/lease handling in DB) + +Phase 1 is the stage that turns individual rollouts into DB-backed memory records. + +## Phase 2: Global Consolidation + +Phase 2 consolidates the latest stage-1 outputs into the filesystem memory artifacts and then runs a dedicated consolidation agent. + +What it does: + +- claims a single global phase-2 job (so only one consolidation runs at a time) +- loads a bounded set of the most recent stage-1 outputs from the state DB (the per-rollout memories produced by Phase 1, used as the consolidation input set) +- computes a completion watermark from the claimed watermark + newest input timestamps +- syncs local memory artifacts under the memories root: + - `raw_memories.md` (merged raw memories, latest first) + - `rollout_summaries/` (one summary file per retained rollout) +- prunes stale rollout summaries that are no longer retained +- if there are no inputs, marks the job successful and exits + +If there is input, it then: + +- spawns an internal consolidation sub-agent +- runs it with no approvals, no network, and local write access only +- disables collab for that agent (to prevent recursive delegation) +- watches the agent status and heartbeats the global job lease while it runs +- marks the phase-2 job success/failure in the state DB when the agent finishes + +Watermark behavior: + +- The global phase-2 job claim includes an input watermark representing the latest input timestamp known when the job was claimed. +- Phase 2 recomputes a `new_watermark` using the max of: + - the claimed watermark + - the newest `source_updated_at` timestamp in the stage-1 inputs it actually loaded +- On success, Phase 2 stores that completion watermark in the DB. +- This lets later phase-2 runs know whether new stage-1 data arrived since the last successful consolidation (dirty vs not dirty), while also avoiding moving the watermark backwards. + +In practice, this phase is responsible for refreshing the on-disk memory workspace and producing/updating the higher-level consolidated memory outputs. + +## Why it is split into two phases + +- Phase 1 scales across many rollouts and produces normalized per-rollout memory records. +- Phase 2 serializes global consolidation so the shared memory artifacts are updated safely and consistently. From 6fbf19ef5f8f43712d3bb0e116df6adbdeb66d65 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 11:04:55 +0000 Subject: [PATCH 08/29] chore: phase 2 name (#12568) --- codex-rs/protocol/src/protocol.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index a8b7ade166..de7c16e677 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1961,6 +1961,9 @@ impl SessionSource { SessionSource::SubAgent(SubAgentSource::ThreadSpawn { agent_nickname, .. }) => { agent_nickname.clone() } + SessionSource::SubAgent(SubAgentSource::MemoryConsolidation) => { + Some("morpheus".to_string()) + } _ => None, } } @@ -1970,6 +1973,9 @@ impl SessionSource { SessionSource::SubAgent(SubAgentSource::ThreadSpawn { agent_role, .. }) => { agent_role.clone() } + SessionSource::SubAgent(SubAgentSource::MemoryConsolidation) => { + Some("memory builder".to_string()) + } _ => None, } } From 9d826a20c69e5f3dc6c2196e8e7fbce7912ec5f3 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 12:49:54 +0000 Subject: [PATCH 09/29] fix: TUI constraint (#12571) --- codex-rs/tui/src/chatwidget.rs | 22 ++++++++++++ codex-rs/tui/src/chatwidget/tests.rs | 51 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 309ce80ce8..64757eaa25 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -49,6 +49,7 @@ use codex_app_server_protocol::ConfigLayerSource; use codex_backend_client::Client as BackendClient; use codex_chatgpt::connectors; use codex_core::config::Config; +use codex_core::config::Constrained; use codex_core::config::ConstraintResult; use codex_core::config::types::Notifications; use codex_core::config::types::WindowsSandboxModeToml; @@ -1076,6 +1077,27 @@ impl ChatWidget { self.forked_from = event.forked_from_id; self.current_rollout_path = event.rollout_path.clone(); self.current_cwd = Some(event.cwd.clone()); + self.config.cwd = event.cwd.clone(); + if let Err(err) = self + .config + .permissions + .approval_policy + .set(event.approval_policy) + { + tracing::warn!(%err, "failed to sync approval_policy from SessionConfigured"); + self.config.permissions.approval_policy = + Constrained::allow_only(event.approval_policy); + } + if let Err(err) = self + .config + .permissions + .sandbox_policy + .set(event.sandbox_policy.clone()) + { + tracing::warn!(%err, "failed to sync sandbox_policy from SessionConfigured"); + self.config.permissions.sandbox_policy = + Constrained::allow_only(event.sandbox_policy.clone()); + } let initial_messages = event.initial_messages.clone(); let forked_from_id = event.forked_from_id; let model_for_header = event.model.clone(); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index d7424f720f..0f537581e8 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -325,6 +325,57 @@ async fn replayed_user_message_preserves_remote_image_urls() { assert_eq!(stored_remote_image_urls, remote_image_urls); } +#[tokio::test] +async fn session_configured_syncs_widget_config_permissions_and_cwd() { + let (mut chat, _rx, _ops) = make_chatwidget_manual(None).await; + + chat.config + .permissions + .approval_policy + .set(AskForApproval::OnRequest) + .expect("set approval policy"); + chat.config + .permissions + .sandbox_policy + .set(SandboxPolicy::new_workspace_write_policy()) + .expect("set sandbox policy"); + chat.config.cwd = PathBuf::from("/home/user/main"); + + let expected_sandbox = SandboxPolicy::new_read_only_policy(); + let expected_cwd = PathBuf::from("/home/user/sub-agent"); + let configured = codex_protocol::protocol::SessionConfiguredEvent { + session_id: ThreadId::new(), + forked_from_id: None, + thread_name: None, + model: "test-model".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: expected_sandbox.clone(), + cwd: expected_cwd.clone(), + reasoning_effort: Some(ReasoningEffortConfig::default()), + history_log_id: 0, + history_entry_count: 0, + initial_messages: None, + network_proxy: None, + rollout_path: None, + }; + + chat.handle_codex_event(Event { + id: "session-configured".into(), + msg: EventMsg::SessionConfigured(configured), + }); + + assert_eq!( + chat.config_ref().permissions.approval_policy.value(), + AskForApproval::Never + ); + assert_eq!( + chat.config_ref().permissions.sandbox_policy.get(), + &expected_sandbox + ); + assert_eq!(&chat.config_ref().cwd, &expected_cwd); +} + #[tokio::test] async fn replayed_user_message_with_only_remote_images_renders_history_cell() { let (mut chat, mut rx, _ops) = make_chatwidget_manual(None).await; From 829d1080f641197c3ce8351c3e9951dccc3636c6 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 12:58:55 +0000 Subject: [PATCH 10/29] feat: keep dead agents in the agent picker (#12570) --- codex-rs/tui/src/app.rs | 171 +++++++++++++----- codex-rs/tui/src/bottom_pane/command_popup.rs | 1 + .../tui/src/bottom_pane/file_search_popup.rs | 1 + .../src/bottom_pane/list_selection_view.rs | 8 +- .../src/bottom_pane/selection_popup_common.rs | 16 +- codex-rs/tui/src/bottom_pane/skill_popup.rs | 1 + codex-rs/tui/src/multi_agents.rs | 45 +++++ 7 files changed, 189 insertions(+), 54 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e080b8cde0..5f765e24ee 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -23,6 +23,10 @@ use crate::history_cell::UpdateAvailableHistoryCell; use crate::model_migration::ModelMigrationOutcome; use crate::model_migration::migration_copy_for_models; use crate::model_migration::run_model_migration_prompt; +use crate::multi_agents::AgentPickerThreadEntry; +use crate::multi_agents::agent_picker_status_dot_spans; +use crate::multi_agents::format_agent_picker_item_name; +use crate::multi_agents::sort_agent_picker_threads; use crate::pager_overlay::Overlay; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::Renderable; @@ -576,6 +580,7 @@ pub(crate) struct App { windows_sandbox: WindowsSandboxState, thread_event_channels: HashMap, + agent_picker_threads: HashMap, active_thread_id: Option, active_thread_rx: Option>, primary_thread_id: Option, @@ -868,50 +873,55 @@ impl App { async fn open_agent_picker(&mut self) { let thread_ids: Vec = self.thread_event_channels.keys().cloned().collect(); - let mut agent_threads = Vec::new(); for thread_id in thread_ids { match self.server.get_thread(thread_id).await { Ok(thread) => { let session_source = thread.config_snapshot().await.session_source; - agent_threads.push(( + self.upsert_agent_picker_thread( thread_id, session_source.get_nickname(), session_source.get_agent_role(), - )); + false, + ); } Err(_) => { - self.thread_event_channels.remove(&thread_id); + self.mark_agent_picker_thread_closed(thread_id); } } } - if agent_threads.is_empty() { + if self.agent_picker_threads.is_empty() { self.chat_widget .add_info_message("No agents available yet.".to_string(), None); return; } - agent_threads.sort_by(|(left, ..), (right, ..)| left.to_string().cmp(&right.to_string())); + let mut agent_threads: Vec<(ThreadId, AgentPickerThreadEntry)> = self + .agent_picker_threads + .iter() + .map(|(thread_id, entry)| (*thread_id, entry.clone())) + .collect(); + sort_agent_picker_threads(&mut agent_threads); let mut initial_selected_idx = None; let items: Vec = agent_threads .iter() .enumerate() - .map(|(idx, (thread_id, agent_nickname, agent_role))| { + .map(|(idx, (thread_id, entry))| { if self.active_thread_id == Some(*thread_id) { initial_selected_idx = Some(idx); } let id = *thread_id; let is_primary = self.primary_thread_id == Some(*thread_id); let name = format_agent_picker_item_name( - *thread_id, - agent_nickname.as_deref(), - agent_role.as_deref(), + entry.agent_nickname.as_deref(), + entry.agent_role.as_deref(), is_primary, ); let uuid = thread_id.to_string(); SelectionItem { name: name.clone(), + name_prefix_spans: agent_picker_status_dot_spans(entry.is_closed), description: Some(uuid.clone()), is_current: self.active_thread_id == Some(*thread_id), actions: vec![Box::new(move |tx| { @@ -934,20 +944,51 @@ impl App { }); } + fn upsert_agent_picker_thread( + &mut self, + thread_id: ThreadId, + agent_nickname: Option, + agent_role: Option, + is_closed: bool, + ) { + self.agent_picker_threads.insert( + thread_id, + AgentPickerThreadEntry { + agent_nickname, + agent_role, + is_closed, + }, + ); + } + + fn mark_agent_picker_thread_closed(&mut self, thread_id: ThreadId) { + if let Some(entry) = self.agent_picker_threads.get_mut(&thread_id) { + entry.is_closed = true; + } else { + self.upsert_agent_picker_thread(thread_id, None, None, true); + } + } + async fn select_agent_thread(&mut self, tui: &mut tui::Tui, thread_id: ThreadId) -> Result<()> { if self.active_thread_id == Some(thread_id) { return Ok(()); } - let thread = match self.server.get_thread(thread_id).await { - Ok(thread) => thread, + let live_thread = match self.server.get_thread(thread_id).await { + Ok(thread) => Some(thread), Err(err) => { - self.chat_widget.add_error_message(format!( - "Failed to attach to agent thread {thread_id}: {err}" - )); - return Ok(()); + if self.thread_event_channels.contains_key(&thread_id) { + self.mark_agent_picker_thread_closed(thread_id); + None + } else { + self.chat_widget.add_error_message(format!( + "Failed to attach to agent thread {thread_id}: {err}" + )); + return Ok(()); + } } }; + let is_replay_only = live_thread.is_none(); let previous_thread_id = self.active_thread_id; self.store_active_thread_receiver().await; @@ -965,11 +1006,22 @@ impl App { self.active_thread_rx = Some(receiver); let init = self.chatwidget_init_for_forked_or_resumed_thread(tui, self.config.clone()); - let codex_op_tx = crate::chatwidget::spawn_op_forwarder(thread); + let codex_op_tx = if let Some(thread) = live_thread { + crate::chatwidget::spawn_op_forwarder(thread) + } else { + let (tx, _rx) = unbounded_channel(); + tx + }; self.chat_widget = ChatWidget::new_with_op_sender(init, codex_op_tx); self.reset_for_thread_switch(tui)?; self.replay_thread_snapshot(snapshot); + if is_replay_only { + self.chat_widget.add_info_message( + format!("Agent thread {thread_id} is closed. Replaying saved transcript."), + None, + ); + } self.drain_active_thread_events(tui).await?; Ok(()) @@ -989,6 +1041,7 @@ impl App { fn reset_thread_event_state(&mut self) { self.thread_event_channels.clear(); + self.agent_picker_threads.clear(); self.active_thread_id = None; self.active_thread_rx = None; self.primary_thread_id = None; @@ -1294,6 +1347,7 @@ impl App { pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, @@ -2752,7 +2806,7 @@ impl App { if let Some((closed_thread_id, primary_thread_id)) = self.active_non_primary_shutdown_target(&event.msg) { - self.thread_event_channels.remove(&closed_thread_id); + self.mark_agent_picker_thread_closed(closed_thread_id); self.select_agent_thread(tui, primary_thread_id).await?; if self.active_thread_id == Some(primary_thread_id) { self.chat_widget.add_info_message( @@ -2794,6 +2848,12 @@ impl App { } }; let config_snapshot = thread.config_snapshot().await; + self.upsert_agent_picker_thread( + thread_id, + config_snapshot.session_source.get_nickname(), + config_snapshot.session_source.get_agent_role(), + false, + ); let event = Event { id: String::new(), msg: EventMsg::SessionConfigured(SessionConfiguredEvent { @@ -3078,28 +3138,6 @@ impl App { } } -fn format_agent_picker_item_name( - _thread_id: ThreadId, - agent_nickname: Option<&str>, - agent_role: Option<&str>, - is_primary: bool, -) -> String { - if is_primary { - return "Main [default]".to_string(); - } - - let agent_nickname = agent_nickname - .map(str::trim) - .filter(|nickname| !nickname.is_empty()); - let agent_role = agent_role.map(str::trim).filter(|role| !role.is_empty()); - match (agent_nickname, agent_role) { - (Some(agent_nickname), Some(agent_role)) => format!("{agent_nickname} [{agent_role}]"), - (Some(agent_nickname), None) => agent_nickname.to_string(), - (None, Some(agent_role)) => format!("[{agent_role}]"), - (None, None) => "Agent".to_string(), - } -} - #[cfg(test)] mod tests { use super::*; @@ -3268,7 +3306,7 @@ mod tests { } #[tokio::test] - async fn open_agent_picker_prunes_missing_threads() -> Result<()> { + async fn open_agent_picker_keeps_missing_threads_for_replay() -> Result<()> { let mut app = make_test_app().await; let thread_id = ThreadId::new(); app.thread_event_channels @@ -3276,7 +3314,44 @@ mod tests { app.open_agent_picker().await; - assert_eq!(app.thread_event_channels.contains_key(&thread_id), false); + assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); + assert_eq!( + app.agent_picker_threads.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: None, + agent_role: None, + is_closed: true, + }) + ); + Ok(()) + } + + #[tokio::test] + async fn open_agent_picker_keeps_cached_closed_threads() -> Result<()> { + let mut app = make_test_app().await; + let thread_id = ThreadId::new(); + app.thread_event_channels + .insert(thread_id, ThreadEventChannel::new(1)); + app.agent_picker_threads.insert( + thread_id, + AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: false, + }, + ); + + app.open_agent_picker().await; + + assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); + assert_eq!( + app.agent_picker_threads.get(&thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: true, + }) + ); Ok(()) } @@ -3287,27 +3362,27 @@ mod tests { let snapshot = [ format!( "{} | {}", - format_agent_picker_item_name(thread_id, Some("Robie"), Some("explorer"), true), + format_agent_picker_item_name(Some("Robie"), Some("explorer"), true), thread_id ), format!( "{} | {}", - format_agent_picker_item_name(thread_id, Some("Robie"), Some("explorer"), false), + format_agent_picker_item_name(Some("Robie"), Some("explorer"), false), thread_id ), format!( "{} | {}", - format_agent_picker_item_name(thread_id, Some("Robie"), None, false), + format_agent_picker_item_name(Some("Robie"), None, false), thread_id ), format!( "{} | {}", - format_agent_picker_item_name(thread_id, None, Some("explorer"), false), + format_agent_picker_item_name(None, Some("explorer"), false), thread_id ), format!( "{} | {}", - format_agent_picker_item_name(thread_id, None, None, false), + format_agent_picker_item_name(None, None, false), thread_id ), ] @@ -3548,6 +3623,7 @@ mod tests { pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, @@ -3606,6 +3682,7 @@ mod tests { pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 1c2291465e..48b6ffedc4 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -220,6 +220,7 @@ impl CommandPopup { }; GenericDisplayRow { name, + name_prefix_spans: Vec::new(), match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), display_shortcut: None, description: Some(description), diff --git a/codex-rs/tui/src/bottom_pane/file_search_popup.rs b/codex-rs/tui/src/bottom_pane/file_search_popup.rs index 9eb8121b69..76f8bc1e1a 100644 --- a/codex-rs/tui/src/bottom_pane/file_search_popup.rs +++ b/codex-rs/tui/src/bottom_pane/file_search_popup.rs @@ -119,6 +119,7 @@ impl WidgetRef for &FileSearchPopup { .iter() .map(|m| GenericDisplayRow { name: m.path.to_string_lossy().to_string(), + name_prefix_spans: Vec::new(), match_indices: m .indices .as_ref() diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index ad40c1660f..2f566079aa 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -112,6 +112,7 @@ pub(crate) type OnCancelCallback = Option>, pub display_shortcut: Option, pub description: Option, pub selected_description: Option, @@ -372,7 +373,9 @@ impl ListSelectionView { format!("{prefix} {n}. ") }; let wrap_prefix_width = UnicodeWidthStr::width(wrap_prefix.as_str()); - let display_name = format!("{wrap_prefix}{name_with_marker}"); + let mut name_prefix_spans = Vec::new(); + name_prefix_spans.push(wrap_prefix.into()); + name_prefix_spans.extend(item.name_prefix_spans.clone()); let description = is_selected .then(|| item.selected_description.clone()) .flatten() @@ -380,7 +383,8 @@ impl ListSelectionView { let wrap_indent = description.is_none().then_some(wrap_prefix_width); let is_disabled = item.is_disabled || item.disabled_reason.is_some(); GenericDisplayRow { - name: display_name, + name: name_with_marker, + name_prefix_spans, display_shortcut: item.display_shortcut, match_indices: None, description, diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 6b55125c9f..6b526de474 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -28,6 +28,7 @@ use super::scroll_state::ScrollState; #[derive(Default)] pub(crate) struct GenericDisplayRow { pub name: String, + pub name_prefix_spans: Vec>, pub display_shortcut: Option, pub match_indices: Option>, // indices to bold (char positions) pub description: Option, // optional grey text after the name @@ -242,7 +243,8 @@ fn compute_desc_col( .skip(start_idx) .take(visible_items) .map(|(_, row)| { - let mut spans: Vec = vec![row.name.clone().into()]; + let mut spans = row.name_prefix_spans.clone(); + spans.push(row.name.clone().into()); if row.disabled_reason.is_some() { spans.push(" (disabled)".dim()); } @@ -253,7 +255,8 @@ fn compute_desc_col( ColumnWidthMode::AutoAllRows => rows_all .iter() .map(|row| { - let mut spans: Vec = vec![row.name.clone().into()]; + let mut spans = row.name_prefix_spans.clone(); + spans.push(row.name.clone().into()); if row.disabled_reason.is_some() { spans.push(" (disabled)".dim()); } @@ -291,6 +294,7 @@ fn should_wrap_name_in_column(row: &GenericDisplayRow) -> bool { && row.match_indices.is_none() && row.display_shortcut.is_none() && row.category_tag.is_none() + && row.name_prefix_spans.is_empty() } fn wrap_two_column_row(row: &GenericDisplayRow, desc_col: usize, width: u16) -> Vec> { @@ -508,9 +512,10 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { // Enforce single-line name: allow at most desc_col - 2 cells for name, // reserving two spaces before the description column. + let name_prefix_width = Line::from(row.name_prefix_spans.clone()).width(); let name_limit = combined_description .as_ref() - .map(|_| desc_col.saturating_sub(2)) + .map(|_| desc_col.saturating_sub(2).saturating_sub(name_prefix_width)) .unwrap_or(usize::MAX); let mut name_spans: Vec = Vec::with_capacity(row.name.len()); @@ -558,8 +563,9 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { name_spans.push(" (disabled)".dim()); } - let this_name_width = Line::from(name_spans.clone()).width(); - let mut full_spans: Vec = name_spans; + let this_name_width = name_prefix_width + Line::from(name_spans.clone()).width(); + let mut full_spans: Vec = row.name_prefix_spans.clone(); + full_spans.extend(name_spans); if let Some(display_shortcut) = row.display_shortcut { full_spans.push(" (".into()); full_spans.push(display_shortcut.into()); diff --git a/codex-rs/tui/src/bottom_pane/skill_popup.rs b/codex-rs/tui/src/bottom_pane/skill_popup.rs index e3a6c9880b..c848bca9ea 100644 --- a/codex-rs/tui/src/bottom_pane/skill_popup.rs +++ b/codex-rs/tui/src/bottom_pane/skill_popup.rs @@ -101,6 +101,7 @@ impl SkillPopup { let description = mention.description.clone().unwrap_or_default(); GenericDisplayRow { name, + name_prefix_spans: Vec::new(), match_indices: indices, display_shortcut: None, description: Some(description).filter(|desc| !desc.is_empty()), diff --git a/codex-rs/tui/src/multi_agents.rs b/codex-rs/tui/src/multi_agents.rs index 806400f2e3..48eaab7344 100644 --- a/codex-rs/tui/src/multi_agents.rs +++ b/codex-rs/tui/src/multi_agents.rs @@ -22,6 +22,13 @@ const COLLAB_PROMPT_PREVIEW_GRAPHEMES: usize = 160; const COLLAB_AGENT_ERROR_PREVIEW_GRAPHEMES: usize = 160; const COLLAB_AGENT_RESPONSE_PREVIEW_GRAPHEMES: usize = 240; +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct AgentPickerThreadEntry { + pub(crate) agent_nickname: Option, + pub(crate) agent_role: Option, + pub(crate) is_closed: bool, +} + #[derive(Clone, Copy)] struct AgentLabel<'a> { thread_id: Option, @@ -29,6 +36,44 @@ struct AgentLabel<'a> { role: Option<&'a str>, } +pub(crate) fn agent_picker_status_dot_spans(is_closed: bool) -> Vec> { + let dot = if is_closed { + "•".dark_gray() + } else { + "•".green() + }; + vec![dot, " ".into()] +} + +pub(crate) fn format_agent_picker_item_name( + agent_nickname: Option<&str>, + agent_role: Option<&str>, + is_primary: bool, +) -> String { + if is_primary { + return "Main [default]".to_string(); + } + + let agent_nickname = agent_nickname + .map(str::trim) + .filter(|nickname| !nickname.is_empty()); + let agent_role = agent_role.map(str::trim).filter(|role| !role.is_empty()); + match (agent_nickname, agent_role) { + (Some(agent_nickname), Some(agent_role)) => format!("{agent_nickname} [{agent_role}]"), + (Some(agent_nickname), None) => agent_nickname.to_string(), + (None, Some(agent_role)) => format!("[{agent_role}]"), + (None, None) => "Agent".to_string(), + } +} + +pub(crate) fn sort_agent_picker_threads(agent_threads: &mut [(ThreadId, AgentPickerThreadEntry)]) { + agent_threads.sort_by(|(left_id, left), (right_id, right)| { + left.is_closed + .cmp(&right.is_closed) + .then_with(|| left_id.to_string().cmp(&right_id.to_string())) + }); +} + pub(crate) fn spawn_end(ev: CollabAgentSpawnEndEvent) -> PlainHistoryCell { let CollabAgentSpawnEndEvent { call_id: _, From cf0210bf22c5b2f0dbf5c2610add895f6ee8e96b Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 13:44:37 +0000 Subject: [PATCH 11/29] feat: agent nick names to model (#12575) --- .../core/src/tools/handlers/multi_agents.rs | 17 +++++++++++++++++ codex-rs/core/src/tools/spec.rs | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index ae72c8b1ef..a736a8cfb6 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -109,6 +109,7 @@ mod spawn { #[derive(Debug, Serialize)] struct SpawnAgentResult { agent_id: String, + nickname: Option, } pub async fn handle( @@ -183,6 +184,7 @@ mod spawn { .unwrap_or((None, None)), None => (None, None), }; + let nickname = new_agent_nickname.clone(); session .send_event( &turn, @@ -202,6 +204,7 @@ mod spawn { let content = serde_json::to_string(&SpawnAgentResult { agent_id: new_thread_id.to_string(), + nickname, }) .map_err(|err| { FunctionCallError::Fatal(format!("failed to serialize spawn_agent result: {err}")) @@ -1085,6 +1088,7 @@ mod tests { #[derive(Debug, Deserialize)] struct SpawnAgentResult { agent_id: String, + nickname: Option, } let (mut session, mut turn) = make_session_and_context().await; @@ -1121,6 +1125,12 @@ mod tests { let result: SpawnAgentResult = serde_json::from_str(&content).expect("spawn_agent result should be json"); let agent_id = agent_id(&result.agent_id).expect("agent_id should be valid"); + assert!( + result + .nickname + .as_deref() + .is_some_and(|nickname| !nickname.is_empty()) + ); let snapshot = manager .get_thread(agent_id) .await @@ -1184,6 +1194,7 @@ mod tests { #[derive(Debug, Deserialize)] struct SpawnAgentResult { agent_id: String, + nickname: Option, } let (mut session, mut turn) = make_session_and_context().await; @@ -1221,6 +1232,12 @@ mod tests { let result: SpawnAgentResult = serde_json::from_str(&content).expect("spawn_agent result should be json"); assert!(!result.agent_id.is_empty()); + assert!( + result + .nickname + .as_deref() + .is_some_and(|nickname| !nickname.is_empty()) + ); assert_eq!(success, Some(true)); } diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 7bd1fc9788..84a69c3adf 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -557,7 +557,7 @@ fn create_spawn_agent_tool(config: &ToolsConfig) -> ToolSpec { ToolSpec::Function(ResponsesApiTool { name: "spawn_agent".to_string(), description: - "Spawn a sub-agent for a well-scoped task. Returns the agent id to use to communicate with this agent." + "Spawn a sub-agent for a well-scoped task. Returns the agent id (and user-facing nickname when available) to use to communicate with this agent." .to_string(), strict: false, parameters: JsonSchema::Object { From 764ac9449f82ef3df09671138179d91017793515 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 14:14:36 +0000 Subject: [PATCH 12/29] feat: add uuid helper (#12500) --- codex-rs/Cargo.lock | 1 + codex-rs/utils/string/Cargo.toml | 3 +++ codex-rs/utils/string/src/lib.rs | 43 ++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e68e85b40d..a4bd8e47e1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2484,6 +2484,7 @@ name = "codex-utils-string" version = "0.0.0" dependencies = [ "pretty_assertions", + "regex-lite", ] [[package]] diff --git a/codex-rs/utils/string/Cargo.toml b/codex-rs/utils/string/Cargo.toml index 8a62539382..3f5890cfb1 100644 --- a/codex-rs/utils/string/Cargo.toml +++ b/codex-rs/utils/string/Cargo.toml @@ -7,5 +7,8 @@ license.workspace = true [lints] workspace = true +[dependencies] +regex-lite = { workspace = true } + [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/utils/string/src/lib.rs b/codex-rs/utils/string/src/lib.rs index b69f185649..df9ae59e1b 100644 --- a/codex-rs/utils/string/src/lib.rs +++ b/codex-rs/utils/string/src/lib.rs @@ -62,11 +62,54 @@ pub fn sanitize_metric_tag_value(value: &str) -> String { } } +/// Find all UUIDs in a string. +#[allow(clippy::unwrap_used)] +pub fn find_uuids(s: &str) -> Vec { + static RE: std::sync::OnceLock = std::sync::OnceLock::new(); + let re = RE.get_or_init(|| { + regex_lite::Regex::new( + r"[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}", + ) + .unwrap() // Unwrap is safe thanks to the tests. + }); + + re.find_iter(s).map(|m| m.as_str().to_string()).collect() +} + #[cfg(test)] mod tests { + use super::find_uuids; use super::sanitize_metric_tag_value; use pretty_assertions::assert_eq; + #[test] + fn find_uuids_finds_multiple() { + let input = + "x 00112233-4455-6677-8899-aabbccddeeff-k y 12345678-90ab-cdef-0123-456789abcdef"; + assert_eq!( + find_uuids(input), + vec![ + "00112233-4455-6677-8899-aabbccddeeff".to_string(), + "12345678-90ab-cdef-0123-456789abcdef".to_string(), + ] + ); + } + + #[test] + fn find_uuids_ignores_invalid() { + let input = "not-a-uuid-1234-5678-9abc-def0-123456789abc"; + assert_eq!(find_uuids(input), Vec::::new()); + } + + #[test] + fn find_uuids_handles_non_ascii_without_overlap() { + let input = "🙂 55e5d6f7-8a7f-4d2a-8d88-123456789012abc"; + assert_eq!( + find_uuids(input), + vec!["55e5d6f7-8a7f-4d2a-8d88-123456789012".to_string()] + ); + } + #[test] fn sanitize_metric_tag_value_trims_and_fills_unspecified() { let msg = "///"; From e8709bc11a00f9a5ad7976c011dad724d9b6b712 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 15:37:12 +0000 Subject: [PATCH 13/29] chore: rename memory feature flag (#12580) `memory_tool` -> `memories` --- codex-rs/core/config.schema.json | 6 ++++++ codex-rs/core/src/features.rs | 2 +- codex-rs/core/src/features/legacy.rs | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 6a0f0a1745..b800cb6941 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -349,6 +349,9 @@ "js_repl_tools_only": { "type": "boolean" }, + "memories": { + "type": "boolean" + }, "memory_tool": { "type": "boolean" }, @@ -1627,6 +1630,9 @@ "js_repl_tools_only": { "type": "boolean" }, + "memories": { + "type": "boolean" + }, "memory_tool": { "type": "boolean" }, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 4cefe4b216..ce594bdd38 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -499,7 +499,7 @@ pub const FEATURES: &[FeatureSpec] = &[ }, FeatureSpec { id: Feature::MemoryTool, - key: "memory_tool", + key: "memories", stage: Stage::UnderDevelopment, default_enabled: false, }, diff --git a/codex-rs/core/src/features/legacy.rs b/codex-rs/core/src/features/legacy.rs index 41eab93bd1..45b3dfd5dc 100644 --- a/codex-rs/core/src/features/legacy.rs +++ b/codex-rs/core/src/features/legacy.rs @@ -37,6 +37,10 @@ const ALIASES: &[Alias] = &[ legacy_key: "collab", feature: Feature::Collab, }, + Alias { + legacy_key: "memory_tool", + feature: Feature::MemoryTool, + }, ]; pub(crate) fn legacy_feature_keys() -> impl Iterator { From 862a5b3eb396fd8cd4d3f1fc47261415a7ff1927 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 23 Feb 2026 07:55:37 -0800 Subject: [PATCH 14/29] Allow exec resume to parse output-last-message flag after command (#12541) Summary - mark `output-last-message` as a global exec flag so it can follow subcommands like `resume` - add regression tests in both `cli` and `exec` crates verifying the flag order works when invoking `resume` Fixes #12538 --- codex-rs/cli/src/main.rs | 28 ++++++++++++++++++++++++++++ codex-rs/exec/src/cli.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index cdecb18dca..483516fe6e 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1112,6 +1112,34 @@ mod tests { assert_eq!(args.prompt.as_deref(), Some("2+2")); } + #[test] + fn exec_resume_accepts_output_last_message_flag_after_subcommand() { + let cli = MultitoolCli::try_parse_from([ + "codex", + "exec", + "resume", + "session-123", + "-o", + "/tmp/resume-output.md", + "re-review", + ]) + .expect("parse should succeed"); + + let Some(Subcommand::Exec(exec)) = cli.subcommand else { + panic!("expected exec subcommand"); + }; + let Some(codex_exec::Command::Resume(args)) = exec.command else { + panic!("expected exec resume"); + }; + + assert_eq!( + exec.last_message_file, + Some(std::path::PathBuf::from("/tmp/resume-output.md")) + ); + assert_eq!(args.session_id.as_deref(), Some("session-123")); + assert_eq!(args.prompt.as_deref(), Some("re-review")); + } + fn app_server_from_args(args: &[&str]) -> AppServerCommand { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let Subcommand::AppServer(app_server) = cli.subcommand.expect("app-server present") else { diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 05845e00fa..c402371607 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -96,7 +96,12 @@ pub struct Cli { pub json: bool, /// Specifies file where the last message from the agent should be written. - #[arg(long = "output-last-message", short = 'o', value_name = "FILE")] + #[arg( + long = "output-last-message", + short = 'o', + value_name = "FILE", + global = true + )] pub last_message_file: Option, /// Initial instructions for the agent. If not provided as an argument (or @@ -283,4 +288,27 @@ mod tests { }); assert_eq!(effective_prompt.as_deref(), Some(PROMPT)); } + + #[test] + fn resume_accepts_output_last_message_flag_after_subcommand() { + const PROMPT: &str = "echo resume-with-output-file"; + let cli = Cli::parse_from([ + "codex-exec", + "resume", + "session-123", + "-o", + "/tmp/resume-output.md", + PROMPT, + ]); + + assert_eq!( + cli.last_message_file, + Some(PathBuf::from("/tmp/resume-output.md")) + ); + let Some(Command::Resume(args)) = cli.command else { + panic!("expected resume command"); + }; + assert_eq!(args.session_id.as_deref(), Some("session-123")); + assert_eq!(args.prompt.as_deref(), Some(PROMPT)); + } } From 2119532a812831578a8ffeb2b5ac014037518106 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 15:55:48 +0000 Subject: [PATCH 15/29] feat: role metrics multi-agent (#12579) add metrics for agent role --- codex-rs/core/src/agent/guards.rs | 3 +++ codex-rs/core/src/agent/role.rs | 2 +- codex-rs/core/src/tools/handlers/multi_agents.rs | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/agent/guards.rs b/codex-rs/core/src/agent/guards.rs index 223373c6df..422e10b651 100644 --- a/codex-rs/core/src/agent/guards.rs +++ b/codex-rs/core/src/agent/guards.rs @@ -118,6 +118,9 @@ impl Guards { } else { active_agents.used_agent_nicknames.clear(); active_agents.nickname_reset_count += 1; + if let Some(metrics) = codex_otel::metrics::global() { + let _ = metrics.counter("codex.multi_agent.nickname_pool_reset", 1, &[]); + } names.choose(&mut rand::rng())?.to_string() } }; diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index a72ba23469..8cc18c1ad1 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -13,7 +13,7 @@ use std::path::Path; use std::sync::LazyLock; use toml::Value as TomlValue; -const DEFAULT_ROLE_NAME: &str = "default"; +pub const DEFAULT_ROLE_NAME: &str = "default"; const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available"; /// Applies a role config layer to a mutable config and preserves unspecified keys. diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index a736a8cfb6..20be209fbe 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -93,6 +93,7 @@ impl ToolHandler for MultiAgentHandler { mod spawn { use super::*; + use crate::agent::role::DEFAULT_ROLE_NAME; use crate::agent::role::apply_role_to_config; use crate::agent::exceeds_thread_spawn_depth_limit; @@ -201,6 +202,9 @@ mod spawn { ) .await; let new_thread_id = result?; + let role_tag = role_name.unwrap_or(DEFAULT_ROLE_NAME); + turn.otel_manager + .counter("codex.multi_agent.spawn", 1, &[("role", role_tag)]); let content = serde_json::to_string(&SpawnAgentResult { agent_id: new_thread_id.to_string(), @@ -409,6 +413,8 @@ mod resume_agent { if let Some(err) = error { return Err(err); } + turn.otel_manager + .counter("codex.multi_agent.resume", 1, &[]); let content = serde_json::to_string(&ResumeAgentResult { status }).map_err(|err| { FunctionCallError::Fatal(format!("failed to serialize resume_agent result: {err}")) From eace7c661026658873d54d452db1f9434d8ee8eb Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 16:12:23 +0000 Subject: [PATCH 16/29] feat: land sqlite (#12141) --- codex-rs/cli/tests/features.rs | 6 +++-- codex-rs/core/src/features.rs | 4 ++-- codex-rs/exec/tests/suite/resume.rs | 35 +++++++++++++++-------------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/codex-rs/cli/tests/features.rs b/codex-rs/cli/tests/features.rs index 8fa07e0a49..6b2ed72f8a 100644 --- a/codex-rs/cli/tests/features.rs +++ b/codex-rs/cli/tests/features.rs @@ -49,10 +49,12 @@ async fn features_enable_under_development_feature_prints_warning() -> Result<() let codex_home = TempDir::new()?; let mut cmd = codex_command(codex_home.path())?; - cmd.args(["features", "enable", "sqlite"]) + cmd.args(["features", "enable", "runtime_metrics"]) .assert() .success() - .stderr(contains("Under-development features enabled: sqlite.")); + .stderr(contains( + "Under-development features enabled: runtime_metrics.", + )); Ok(()) } diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index ce594bdd38..4b3f0b9dec 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -494,8 +494,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::Sqlite, key: "sqlite", - stage: Stage::UnderDevelopment, - default_enabled: false, + stage: Stage::Stable, + default_enabled: true, }, FeatureSpec { id: Feature::MemoryTool, diff --git a/codex-rs/exec/tests/suite/resume.rs b/codex-rs/exec/tests/suite/resume.rs index 4169c60dd1..89e125bf8d 100644 --- a/codex-rs/exec/tests/suite/resume.rs +++ b/codex-rs/exec/tests/suite/resume.rs @@ -4,11 +4,7 @@ use codex_utils_cargo_bin::find_resource; use core_test_support::test_codex_exec::test_codex_exec; use pretty_assertions::assert_eq; use serde_json::Value; -use std::fs::FileTimes; -use std::fs::OpenOptions; use std::string::ToString; -use std::time::Duration; -use std::time::SystemTime; use tempfile::TempDir; use uuid::Uuid; use walkdir::WalkDir; @@ -257,21 +253,26 @@ fn exec_resume_last_respects_cwd_filter_and_all_flag() -> anyhow::Result<()> { .success(); let sessions_dir = test.home_path().join("sessions"); - let path_a = find_session_file_containing_marker(&sessions_dir, &marker_a) + find_session_file_containing_marker(&sessions_dir, &marker_a) .expect("no session file found for marker_a"); let path_b = find_session_file_containing_marker(&sessions_dir, &marker_b) .expect("no session file found for marker_b"); - // Files are ordered by `updated_at`, then by `uuid`. - // We mutate the mtimes to ensure file_b is the newest file. - let file_a = OpenOptions::new().write(true).open(&path_a)?; - file_a.set_times( - FileTimes::new().set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(1)), - )?; - let file_b = OpenOptions::new().write(true).open(&path_b)?; - file_b.set_times( - FileTimes::new().set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(2)), - )?; + // Make thread B deterministically newest according to rollout metadata. + let session_id_b = extract_conversation_id(&path_b); + let marker_b_touch = format!("resume-cwd-b-touch-{}", Uuid::new_v4()); + let prompt_b_touch = format!("echo {marker_b_touch}"); + test.cmd() + .env("CODEX_RS_SSE_FIXTURE", &fixture) + .env("OPENAI_BASE_URL", "http://unused.local") + .arg("--skip-git-repo-check") + .arg("-C") + .arg(dir_b.path()) + .arg("resume") + .arg(&session_id_b) + .arg(&prompt_b_touch) + .assert() + .success(); let marker_b2 = format!("resume-cwd-b-2-{}", Uuid::new_v4()); let prompt_b2 = format!("echo {marker_b2}"); @@ -312,8 +313,8 @@ fn exec_resume_last_respects_cwd_filter_and_all_flag() -> anyhow::Result<()> { let resumed_path_cwd = find_session_file_containing_marker(&sessions_dir, &marker_a2) .expect("no resumed session file containing marker_a2"); assert_eq!( - resumed_path_cwd, path_a, - "resume --last should prefer sessions from the same cwd" + resumed_path_cwd, path_b, + "resume --last should prefer sessions whose latest turn context matches the current cwd" ); Ok(()) From 3b6c50d925027d5d315a2a6ea77dd062a4f0c697 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 23 Feb 2026 16:13:29 +0000 Subject: [PATCH 17/29] chore: better bazel test logs (#12576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Improve Bazel CI failure diagnostics by printing the tail of each failed target’s test.log directly in the GitHub Actions output. Today, when a large Bazel test target fails (for example tests of `codex-core`), the workflow often only shows a target-level Exit 101 plus a path to Bazel’s test.log. That makes it hard to see the actual failing Rust test and panic without digging into artifacts or reproducing locally. This change makes the workflow automatically surface that information inline. ## What Changed In .github/workflows/bazel.yml: - Capture Bazel console output via tee - Preserve the Bazel exit code when piping (PIPESTATUS[0]) - On failure: - Parse failed Bazel test targets from FAIL: //... lines - Resolve Bazel test log directory via bazel info bazel-testlogs - Print tail -n 200 for each failed target’s test.log - Group each target’s output in GitHub Actions logs (::group::) ## Bonus Disable `experimental_remote_repo_contents_cache` to prevent "Permission Denied" --- .github/workflows/bazel.yml | 61 +++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 59a1e07fa7..1a0a69035f 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -107,6 +107,45 @@ jobs: BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }} shell: bash run: | + set -o pipefail + + bazel_console_log="$(mktemp)" + + print_failed_bazel_test_logs() { + local console_log="$1" + local testlogs_dir + + testlogs_dir="$(bazel $BAZEL_STARTUP_ARGS info bazel-testlogs 2>/dev/null || echo bazel-testlogs)" + + local failed_targets=() + while IFS= read -r target; do + failed_targets+=("$target") + done < <( + grep -E '^FAIL: //' "$console_log" \ + | sed -E 's#^FAIL: (//[^ ]+).*#\1#' \ + | sort -u + ) + + if [[ ${#failed_targets[@]} -eq 0 ]]; then + echo "No failed Bazel test targets were found in console output." + return + fi + + for target in "${failed_targets[@]}"; do + local rel_path="${target#//}" + rel_path="${rel_path/:/\/}" + local test_log="${testlogs_dir}/${rel_path}/test.log" + + echo "::group::Bazel test log tail for ${target}" + if [[ -f "$test_log" ]]; then + tail -n 200 "$test_log" + else + echo "Missing test log: $test_log" + fi + echo "::endgroup::" + done + } + bazel_args=( test //... @@ -119,10 +158,19 @@ jobs: if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then echo "BuildBuddy API key is available; using remote Bazel configuration." + # Work around Bazel 9 remote repo contents cache / overlay materialization failures + # seen in CI (for example "is not a symlink" or permission errors while + # materializing external repos such as rules_perl). We still use BuildBuddy for + # remote execution/cache; this only disables the startup-level repo contents cache. + set +e bazel $BAZEL_STARTUP_ARGS \ + --noexperimental_remote_repo_contents_cache \ --bazelrc=.github/workflows/ci.bazelrc \ "${bazel_args[@]}" \ - "--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" + "--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" \ + 2>&1 | tee "$bazel_console_log" + bazel_status=${PIPESTATUS[0]} + set -e else echo "BuildBuddy API key is not available; using local Bazel configuration." # Keep fork/community PRs on Bazel but disable remote services that are @@ -141,9 +189,18 @@ jobs: # clear remote cache/execution endpoints configured in .bazelrc. # https://bazel.build/reference/command-line-reference#common_options-flag--remote_cache # https://bazel.build/reference/command-line-reference#common_options-flag--remote_executor + set +e bazel $BAZEL_STARTUP_ARGS \ --noexperimental_remote_repo_contents_cache \ "${bazel_args[@]}" \ --remote_cache= \ - --remote_executor= + --remote_executor= \ + 2>&1 | tee "$bazel_console_log" + bazel_status=${PIPESTATUS[0]} + set -e + fi + + if [[ ${bazel_status:-0} -ne 0 ]]; then + print_failed_bazel_test_logs "$bazel_console_log" + exit "$bazel_status" fi From 6e60f724bcd18e722086e646f9d7e4f19214a9f1 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 23 Feb 2026 09:06:08 -0800 Subject: [PATCH 18/29] remove feature flag collaboration modes (#12028) All code should go in the direction that steer is enabled --------- Co-authored-by: Codex --- codex-rs/core/src/features.rs | 10 ++++---- codex-rs/core/src/tools/spec.rs | 30 +----------------------- codex-rs/tui/src/chatwidget.rs | 35 ++++------------------------ codex-rs/tui/src/chatwidget/tests.rs | 32 +++++-------------------- 4 files changed, 17 insertions(+), 90 deletions(-) diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 4b3f0b9dec..1f7805a908 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -134,6 +134,7 @@ pub enum Feature { /// Steer feature flag - when enabled, Enter submits immediately instead of queuing. Steer, /// Enable collaboration modes (Plan, Default). + /// Kept for config backward compatibility; behavior is always collaboration-modes-enabled. CollaborationModes, /// Enable personality selection in the TUI. Personality, @@ -617,7 +618,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::CollaborationModes, key: "collaboration_modes", - stage: Stage::Stable, + stage: Stage::Removed, default_enabled: true, }, FeatureSpec { @@ -728,10 +729,9 @@ mod tests { fn default_enabled_features_are_stable() { for spec in FEATURES { if spec.default_enabled { - assert_eq!( - spec.stage, - Stage::Stable, - "feature `{}` is enabled by default but is not stable ({:?})", + assert!( + matches!(spec.stage, Stage::Stable | Stage::Removed), + "feature `{}` is enabled by default but is not stable/removed ({:?})", spec.key, spec.stage ); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 84a69c3adf..60bd8bb03e 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -65,7 +65,7 @@ impl ToolsConfig { let include_js_repl_tools_only = include_js_repl && features.enabled(Feature::JsReplToolsOnly); let include_collab_tools = features.enabled(Feature::Collab); - let include_collaboration_modes_tools = features.enabled(Feature::CollaborationModes); + let include_collaboration_modes_tools = true; let include_search_tool = features.enabled(Feature::Apps); let shell_type = if !features.enabled(Feature::ShellTool) { @@ -1887,34 +1887,6 @@ mod tests { ); } - #[test] - fn request_user_input_requires_collaboration_modes_feature() { - let config = test_config(); - let model_info = - ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config); - let mut features = Features::with_defaults(); - features.disable(Feature::CollaborationModes); - let tools_config = ToolsConfig::new(&ToolsConfigParams { - model_info: &model_info, - features: &features, - web_search_mode: Some(WebSearchMode::Cached), - }); - let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); - assert!( - !tools.iter().any(|t| t.spec.name() == "request_user_input"), - "request_user_input should be disabled when collaboration_modes feature is off" - ); - - features.enable(Feature::CollaborationModes); - let tools_config = ToolsConfig::new(&ToolsConfigParams { - model_info: &model_info, - features: &features, - web_search_mode: Some(WebSearchMode::Cached), - }); - let (tools, _) = build_specs(&tools_config, None, None, &[]).build(); - assert_contains_tool_names(&tools, &["request_user_input"]); - } - #[test] fn js_repl_requires_feature_flag() { let config = test_config(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 64757eaa25..7ba3a95054 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2831,9 +2831,7 @@ impl ChatWidget { widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); - widget.bottom_pane.set_collaboration_modes_enabled( - widget.config.features.enabled(Feature::CollaborationModes), - ); + widget.bottom_pane.set_collaboration_modes_enabled(true); widget.sync_personality_command_enabled(); widget .bottom_pane @@ -3001,9 +2999,7 @@ impl ChatWidget { widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); - widget.bottom_pane.set_collaboration_modes_enabled( - widget.config.features.enabled(Feature::CollaborationModes), - ); + widget.bottom_pane.set_collaboration_modes_enabled(true); widget.sync_personality_command_enabled(); widget .bottom_pane @@ -3160,9 +3156,7 @@ impl ChatWidget { widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); - widget.bottom_pane.set_collaboration_modes_enabled( - widget.config.features.enabled(Feature::CollaborationModes), - ); + widget.bottom_pane.set_collaboration_modes_enabled(true); widget.sync_personality_command_enabled(); widget .bottom_pane @@ -6347,22 +6341,6 @@ impl ChatWidget { if feature == Feature::Steer { self.bottom_pane.set_steer_enabled(enabled); } - if feature == Feature::CollaborationModes { - self.bottom_pane.set_collaboration_modes_enabled(enabled); - let settings = self.current_collaboration_mode.settings.clone(); - self.current_collaboration_mode = CollaborationMode { - mode: ModeKind::Default, - settings, - }; - self.active_collaboration_mask = if enabled { - collaboration_modes::default_mask(self.models_manager.as_ref()) - } else { - None - }; - self.update_collaboration_mode_indicator(); - self.refresh_model_display(); - self.request_redraw(); - } if feature == Feature::Personality { self.sync_personality_command_enabled(); } @@ -6541,17 +6519,14 @@ impl ChatWidget { } fn collaboration_modes_enabled(&self) -> bool { - self.config.features.enabled(Feature::CollaborationModes) + true } fn initial_collaboration_mask( - config: &Config, + _config: &Config, models_manager: &ModelsManager, model_override: Option<&str>, ) -> Option { - if !config.features.enabled(Feature::CollaborationModes) { - return None; - } let mut mask = collaboration_modes::default_mask(models_manager)?; if let Some(model_override) = model_override { mask.model = Some(model_override.to_string()); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 0f537581e8..1e0487eddf 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1628,7 +1628,7 @@ async fn make_chatwidget_manual( skills: None, }); bottom.set_steer_enabled(true); - bottom.set_collaboration_modes_enabled(cfg.features.enabled(Feature::CollaborationModes)); + bottom.set_collaboration_modes_enabled(true); let auth_manager = codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("test")); let codex_home = cfg.codex_home.clone(); @@ -1643,6 +1643,7 @@ async fn make_chatwidget_manual( }, }; let current_collaboration_mode = base_mode; + let active_collaboration_mask = collaboration_modes::default_mask(models_manager.as_ref()); let mut widget = ChatWidget { app_event_tx, codex_op_tx: op_tx, @@ -1651,7 +1652,7 @@ async fn make_chatwidget_manual( active_cell_revision: 0, config: cfg, current_collaboration_mode, - active_collaboration_mask: None, + active_collaboration_mask, auth_manager, models_manager, otel_manager, @@ -4055,17 +4056,10 @@ async fn slash_init_skips_when_project_doc_exists() { } #[tokio::test] -async fn collab_mode_shift_tab_cycles_only_when_enabled_and_idle() { +async fn collab_mode_shift_tab_cycles_only_when_idle() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - chat.set_feature_enabled(Feature::CollaborationModes, false); let initial = chat.current_collaboration_mode().clone(); - chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); - assert_eq!(chat.current_collaboration_mode(), &initial); - assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Default); - - chat.set_feature_enabled(Feature::CollaborationModes, true); - chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); assert_eq!(chat.current_collaboration_mode(), &initial); @@ -4430,26 +4424,12 @@ async fn collab_mode_is_sent_after_enabling() { } #[tokio::test] -async fn collab_mode_toggle_on_applies_default_preset() { +async fn collab_mode_applies_default_preset() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; chat.thread_id = Some(ThreadId::new()); chat.bottom_pane - .set_composer_text("before toggle".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - match next_submit_op(&mut op_rx) { - Op::UserTurn { - collaboration_mode: None, - personality: Some(Personality::Pragmatic), - .. - } => {} - other => panic!("expected Op::UserTurn without collaboration_mode, got {other:?}"), - } - - chat.set_feature_enabled(Feature::CollaborationModes, true); - - chat.bottom_pane - .set_composer_text("after toggle".to_string(), Vec::new(), Vec::new()); + .set_composer_text("hello".to_string(), Vec::new(), Vec::new()); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); match next_submit_op(&mut op_rx) { Op::UserTurn { From a606e85859e88c56ba47fe8c2884fd15358ef722 Mon Sep 17 00:00:00 2001 From: Won Park Date: Mon, 23 Feb 2026 09:11:05 -0800 Subject: [PATCH 19/29] tweaked /clear to support clear + new chat, also fix minor bug for macos terminal (#12520) # /clear feature! Use /clear to start a new chat with Codex on a clean terminal! --- codex-rs/tui/src/app.rs | 120 +++++++++++++++------------- codex-rs/tui/src/app_event.rs | 3 +- codex-rs/tui/src/custom_terminal.rs | 9 ++- codex-rs/tui/src/slash_command.rs | 2 +- 4 files changed, 71 insertions(+), 63 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 5f765e24ee..a5cfbdb63f 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -704,27 +704,18 @@ impl App { self.clear_ui_header_lines_with_version(width, CODEX_CLI_VERSION) } - fn clear_terminal_ui(&mut self, tui: &mut tui::Tui) -> Result<()> { + fn clear_terminal_ui(&mut self, tui: &mut tui::Tui, redraw_header: bool) -> Result<()> { let is_alt_screen_active = tui.is_alt_screen_active(); - let use_apple_terminal_clear_workaround = !is_alt_screen_active - && matches!( - codex_core::terminal::terminal_info().name, - codex_core::terminal::TerminalName::AppleTerminal - ); // Drop queued history insertions so stale transcript lines cannot be flushed after /clear. tui.clear_pending_history_lines(); if is_alt_screen_active { tui.terminal.clear_visible_screen()?; - } else if use_apple_terminal_clear_workaround { - // Terminal.app can leave mixed old/new glyphs behind when we purge + clear. - // Use a stricter ANSI reset, then redraw only a fresh session header box instead of - // replaying the initialization transcript preamble. - tui.terminal.clear_scrollback_and_visible_screen_ansi()?; } else { - tui.terminal.clear_scrollback()?; - tui.terminal.clear_visible_screen()?; + // Some terminals (Terminal.app, Warp) do not reliably drop scrollback when purge and + // clear are emitted as separate backend commands. Prefer a single ANSI sequence. + tui.terminal.clear_scrollback_and_visible_screen_ansi()?; } let mut area = tui.terminal.viewport_area; @@ -736,11 +727,13 @@ impl App { } self.has_emitted_history_lines = false; - let width = tui.terminal.last_known_screen_size.width; - let header_lines = self.clear_ui_header_lines(width); - if !header_lines.is_empty() { - tui.insert_history_lines(header_lines); - self.has_emitted_history_lines = true; + if redraw_header { + let width = tui.terminal.last_known_screen_size.width; + let header_lines = self.clear_ui_header_lines(width); + if !header_lines.is_empty() { + tui.insert_history_lines(header_lines); + self.has_emitted_history_lines = true; + } } Ok(()) } @@ -1048,6 +1041,48 @@ impl App { self.pending_primary_events.clear(); } + async fn start_fresh_session_with_summary_hint(&mut self, tui: &mut tui::Tui) { + // Start a fresh in-memory session while preserving resumability via persisted rollout + // history. + let model = self.chat_widget.current_model().to_string(); + let summary = session_summary( + self.chat_widget.token_usage(), + self.chat_widget.thread_id(), + self.chat_widget.thread_name(), + ); + self.shutdown_current_thread().await; + if let Err(err) = self.server.remove_and_close_all_threads().await { + tracing::warn!(error = %err, "failed to close all threads"); + } + let init = crate::chatwidget::ChatWidgetInit { + config: self.config.clone(), + frame_requester: tui.frame_requester(), + app_event_tx: self.app_event_tx.clone(), + // New sessions start without prefilled message content. + initial_user_message: None, + enhanced_keys_supported: self.enhanced_keys_supported, + auth_manager: self.auth_manager.clone(), + models_manager: self.server.get_models_manager(), + feedback: self.feedback.clone(), + is_first_run: false, + feedback_audience: self.feedback_audience, + model: Some(model), + status_line_invalid_items_warned: self.status_line_invalid_items_warned.clone(), + otel_manager: self.otel_manager.clone(), + }; + self.chat_widget = ChatWidget::new(init, self.server.clone()); + self.reset_thread_event_state(); + if let Some(summary) = summary { + let mut lines: Vec> = vec![summary.usage_line.clone().into()]; + if let Some(command) = summary.resume_command { + let spans = vec!["To continue this session, run ".into(), command.cyan()]; + lines.push(spans.into()); + } + self.chat_widget.add_plain_history_lines(lines); + } + tui.frame_requester().schedule_frame(); + } + async fn drain_active_thread_events(&mut self, tui: &mut tui::Tui) -> Result<()> { let Some(mut rx) = self.active_thread_rx.take() else { return Ok(()); @@ -1535,47 +1570,18 @@ impl App { async fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result { match event { AppEvent::NewSession => { - let model = self.chat_widget.current_model().to_string(); - let summary = session_summary( - self.chat_widget.token_usage(), - self.chat_widget.thread_id(), - self.chat_widget.thread_name(), - ); - self.shutdown_current_thread().await; - if let Err(err) = self.server.remove_and_close_all_threads().await { - tracing::warn!(error = %err, "failed to close all threads"); - } - let init = crate::chatwidget::ChatWidgetInit { - config: self.config.clone(), - frame_requester: tui.frame_requester(), - app_event_tx: self.app_event_tx.clone(), - // New sessions start without prefilled message content. - initial_user_message: None, - enhanced_keys_supported: self.enhanced_keys_supported, - auth_manager: self.auth_manager.clone(), - models_manager: self.server.get_models_manager(), - feedback: self.feedback.clone(), - is_first_run: false, - feedback_audience: self.feedback_audience, - model: Some(model), - status_line_invalid_items_warned: self.status_line_invalid_items_warned.clone(), - otel_manager: self.otel_manager.clone(), - }; - self.chat_widget = ChatWidget::new(init, self.server.clone()); - self.reset_thread_event_state(); - if let Some(summary) = summary { - let mut lines: Vec> = vec![summary.usage_line.clone().into()]; - if let Some(command) = summary.resume_command { - let spans = vec!["To continue this session, run ".into(), command.cyan()]; - lines.push(spans.into()); - } - self.chat_widget.add_plain_history_lines(lines); - } - tui.frame_requester().schedule_frame(); + self.start_fresh_session_with_summary_hint(tui).await; } AppEvent::ClearUi => { - self.clear_terminal_ui(tui)?; - tui.frame_requester().schedule_frame(); + self.clear_terminal_ui(tui, false)?; + self.overlay = None; + self.transcript_cells.clear(); + self.deferred_history_lines.clear(); + self.has_emitted_history_lines = false; + self.backtrack = BacktrackState::default(); + self.backtrack_render_pending = false; + + self.start_fresh_session_with_summary_hint(tui).await; } AppEvent::OpenResumePicker => { match crate::resume_picker::run_resume_picker(tui, &self.config, false).await? { diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 865ddb29e6..3434bc1c93 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -54,7 +54,8 @@ pub(crate) enum AppEvent { /// Start a new session. NewSession, - /// Clear the terminal UI (screen + scrollback) without changing session state. + /// Clear the terminal UI (screen + scrollback), start a fresh session, and keep the + /// previous chat resumable. ClearUi, /// Open the resume picker inside the running TUI session. diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index 1b16f59156..c51cc726b5 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -457,15 +457,16 @@ where /// Hard-reset scrollback + visible screen using an explicit ANSI sequence. /// - /// This is a compatibility fallback for terminals that misbehave when purge - /// and full-screen clear are issued as separate backend commands. + /// Some terminals behave more reliably when purge + clear are emitted as a + /// single ANSI sequence instead of separate backend commands. pub fn clear_scrollback_and_visible_screen_ansi(&mut self) -> io::Result<()> { if self.viewport_area.is_empty() { return Ok(()); } - // Reset scroll region + style state, purge scrollback, clear screen, home cursor. - write!(self.backend, "\x1b[r\x1b[0m\x1b[3J\x1b[2J\x1b[H")?; + // Reset scroll region + style state, home cursor, clear screen, purge scrollback. + // The order matches the common shell `clear && printf '\\e[3J'` behavior. + write!(self.backend, "\x1b[r\x1b[0m\x1b[H\x1b[2J\x1b[3J\x1b[H")?; std::io::Write::flush(&mut self.backend)?; self.last_known_cursor_pos = Position { x: 0, y: 0 }; self.visible_history_rows = 0; diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index a512060e2b..e4ee2dc763 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -68,7 +68,7 @@ impl SlashCommand { SlashCommand::Review => "review my current changes and find issues", SlashCommand::Rename => "rename the current thread", SlashCommand::Resume => "resume a saved chat", - SlashCommand::Clear => "clear the terminal screen and scrollback", + SlashCommand::Clear => "clear the terminal and start a new chat", SlashCommand::Fork => "fork the current chat", // SlashCommand::Undo => "ask Codex to undo a turn", SlashCommand::Quit | SlashCommand::Exit => "exit Codex", From 5221575f2336b17e2ec13f943d93356138d2590f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 23 Feb 2026 09:28:17 -0800 Subject: [PATCH 20/29] refactor: normalize unix module layout for exec-server and shell-escalation (#12556) ## Why Shell execution refactoring in `exec-server` had become split between duplicated code paths, which blocked a clean introduction of the new reusable shell escalation flow. This commit creates a dedicated foundation crate so later shell tooling changes can share one implementation. ## What changed - Added the `codex-shell-escalation` crate and moved the core escalation pieces (`mcp` protocol/socket/session flow, policy glue) that were previously in `exec-server` into it. - Normalized `exec-server` Unix structure under a dedicated `unix` module layout and kept non-Unix builds narrow. - Wired crate/build metadata so `shell-escalation` is a first-class workspace dependency for follow-on integration work. ## Verification - Built and linted the stack at this commit point with `just clippy`. [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12556). * #12584 * #12583 * __->__ #12556 --- codex-rs/Cargo.lock | 27 ++++- codex-rs/Cargo.toml | 2 + codex-rs/exec-server/Cargo.toml | 25 ++-- codex-rs/exec-server/src/lib.rs | 10 +- .../exec-server/src/{posix.rs => unix.rs} | 11 +- .../exec-server/src/{posix => unix}/mcp.rs | 105 ++++++---------- .../{posix => unix}/mcp_escalation_policy.rs | 14 +-- codex-rs/shell-escalation/BUILD.bazel | 6 + codex-rs/shell-escalation/Cargo.toml | 30 +++++ codex-rs/shell-escalation/src/lib.rs | 21 ++++ .../src/unix/core_shell_escalation.rs | 71 +++++++++++ .../src/unix}/escalate_client.rs | 22 ++-- .../src/unix}/escalate_protocol.rs | 30 ++--- .../src/unix}/escalate_server.rs | 113 ++++++++++++------ .../src/unix}/escalation_policy.rs | 6 +- codex-rs/shell-escalation/src/unix/mod.rs | 7 ++ .../src/unix}/socket.rs | 0 .../src/unix}/stopwatch.rs | 8 +- 18 files changed, 327 insertions(+), 181 deletions(-) rename codex-rs/exec-server/src/{posix.rs => unix.rs} (97%) rename codex-rs/exec-server/src/{posix => unix}/mcp.rs (75%) rename codex-rs/exec-server/src/{posix => unix}/mcp_escalation_policy.rs (92%) create mode 100644 codex-rs/shell-escalation/BUILD.bazel create mode 100644 codex-rs/shell-escalation/Cargo.toml create mode 100644 codex-rs/shell-escalation/src/lib.rs create mode 100644 codex-rs/shell-escalation/src/unix/core_shell_escalation.rs rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_client.rs (85%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_protocol.rs (66%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalate_server.rs (76%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/escalation_policy.rs (62%) create mode 100644 codex-rs/shell-escalation/src/unix/mod.rs rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/socket.rs (100%) rename codex-rs/{exec-server/src/posix => shell-escalation/src/unix}/stopwatch.rs (96%) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a4bd8e47e1..b5e5cd5ac3 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1786,21 +1786,19 @@ dependencies = [ "codex-execpolicy", "codex-protocol", "codex-shell-command", + "codex-shell-escalation", "codex-utils-cargo-bin", "core_test_support", "exec_server_test_support", - "libc", "maplit", - "path-absolutize", "pretty_assertions", "rmcp", + "schemars 1.2.1", "serde", "serde_json", "shlex", - "socket2 0.6.2", "tempfile", "tokio", - "tokio-util", "tracing", "tracing-subscriber", ] @@ -2201,6 +2199,27 @@ dependencies = [ "which", ] +[[package]] +name = "codex-shell-escalation" +version = "0.0.0" +dependencies = [ + "anyhow", + "async-trait", + "codex-core", + "codex-execpolicy", + "codex-protocol", + "libc", + "path-absolutize", + "pretty_assertions", + "serde", + "serde_json", + "socket2 0.6.2", + "tempfile", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "codex-skills" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index e22dba12c2..56fe6d4d57 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -17,6 +17,7 @@ members = [ "cli", "config", "shell-command", + "shell-escalation", "skills", "core", "hooks", @@ -113,6 +114,7 @@ codex-responses-api-proxy = { path = "responses-api-proxy" } codex-rmcp-client = { path = "rmcp-client" } codex-secrets = { path = "secrets" } codex-shell-command = { path = "shell-command" } +codex-shell-escalation = { path = "shell-escalation" } codex-skills = { path = "skills" } codex-state = { path = "state" } codex-stdio-to-uds = { path = "stdio-to-uds" } diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 9b40cd35d7..a6a0721b98 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -1,8 +1,8 @@ [package] name = "codex-exec-server" -version.workspace = true edition.workspace = true license.workspace = true +version.workspace = true [[bin]] name = "codex-execve-wrapper" @@ -19,6 +19,11 @@ path = "src/lib.rs" [lints] workspace = true +[package.metadata.cargo-shear] +# This appears to be due to #[derive(rmcp::schemars::JsonSchema)], which +# requires use of schemars via a macro that shear cannot detect. +ignored = ["schemars"] + [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } @@ -27,8 +32,7 @@ codex-core = { workspace = true } codex-execpolicy = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } -libc = { workspace = true } -path-absolutize = { workspace = true } +codex-shell-escalation = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ "auth", "elicitation", @@ -42,25 +46,18 @@ rmcp = { workspace = true, default-features = false, features = [ "transport-streamable-http-server", "transport-io", ] } +schemars = { version = "1.2.1" } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } shlex = { workspace = true } -socket2 = { workspace = true } -tokio = { workspace = true, features = [ - "io-std", - "macros", - "process", - "rt-multi-thread", - "signal", -] } -tokio-util = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] } tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } [dev-dependencies] -core_test_support = { workspace = true } -codex-utils-cargo-bin = { workspace = true } codex-protocol = { workspace = true } +codex-utils-cargo-bin = { workspace = true } +core_test_support = { workspace = true } exec_server_test_support = { workspace = true } maplit = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 62f7bbccca..cd55b46ce2 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -1,11 +1,5 @@ #[cfg(unix)] -mod posix; +mod unix; #[cfg(unix)] -pub use posix::main_execve_wrapper; - -#[cfg(unix)] -pub use posix::main_mcp_server; - -#[cfg(unix)] -pub use posix::ExecResult; +pub use unix::*; diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/unix.rs similarity index 97% rename from codex-rs/exec-server/src/posix.rs rename to codex-rs/exec-server/src/unix.rs index 907ec14b68..0e0355443b 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/unix.rs @@ -67,21 +67,16 @@ use codex_execpolicy::Decision; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_shell_command::is_dangerous_command::command_might_be_dangerous; +use codex_shell_escalation as shell_escalation; use rmcp::ErrorData as McpError; use tokio::sync::RwLock; use tracing_subscriber::EnvFilter; use tracing_subscriber::{self}; -use crate::posix::mcp_escalation_policy::ExecPolicyOutcome; +use crate::unix::mcp_escalation_policy::ExecPolicyOutcome; -mod escalate_client; -mod escalate_protocol; -mod escalate_server; -mod escalation_policy; mod mcp; mod mcp_escalation_policy; -mod socket; -mod stopwatch; pub use mcp::ExecResult; @@ -165,7 +160,7 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> { .init(); let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse(); - let exit_code = escalate_client::run(file, argv).await?; + let exit_code = shell_escalation::run(file, argv).await?; std::process::exit(exit_code); } diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/unix/mcp.rs similarity index 75% rename from codex-rs/exec-server/src/posix/mcp.rs rename to codex-rs/exec-server/src/unix/mcp.rs index f413fed210..547d055c14 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/unix/mcp.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -10,6 +9,8 @@ use codex_core::MCP_SANDBOX_STATE_METHOD; use codex_core::SandboxState; use codex_execpolicy::Policy; use codex_protocol::protocol::SandboxPolicy; +use codex_shell_escalation::EscalationPolicyFactory; +use codex_shell_escalation::run_escalate_server; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::ServerHandler; @@ -19,7 +20,6 @@ use rmcp::handler::server::wrapper::Parameters; use rmcp::model::CustomRequest; use rmcp::model::CustomResult; use rmcp::model::*; -use rmcp::schemars; use rmcp::service::RequestContext; use rmcp::service::RunningService; use rmcp::tool; @@ -29,11 +29,7 @@ use rmcp::transport::stdio; use serde_json::json; use tokio::sync::RwLock; -use crate::posix::escalate_server::EscalateServer; -use crate::posix::escalate_server::{self}; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::mcp_escalation_policy::McpEscalationPolicy; -use crate::posix::stopwatch::Stopwatch; +use crate::unix::mcp_escalation_policy::McpEscalationPolicy; /// Path to our patched bash. const CODEX_BASH_PATH_ENV_VAR: &str = "CODEX_BASH_PATH"; @@ -46,19 +42,7 @@ pub(crate) fn get_bash_path() -> Result { .context(format!("{CODEX_BASH_PATH_ENV_VAR} must be set")) } -#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] -pub struct ExecParams { - /// The bash string to execute. - pub command: String, - /// The working directory to execute the command in. Must be an absolute path. - pub workdir: String, - /// The timeout for the command in milliseconds. - pub timeout_ms: Option, - /// Launch Bash with -lc instead of -c: defaults to true. - pub login: Option, -} - -#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)] +#[derive(Debug, serde::Serialize, serde::Deserialize)] pub struct ExecResult { pub exit_code: i32, pub output: String, @@ -66,8 +50,8 @@ pub struct ExecResult { pub timed_out: bool, } -impl From for ExecResult { - fn from(result: escalate_server::ExecResult) -> Self { +impl From for ExecResult { + fn from(result: codex_shell_escalation::ExecResult) -> Self { Self { exit_code: result.exit_code, output: result.output, @@ -87,10 +71,27 @@ pub struct ExecTool { sandbox_state: Arc>>, } -trait EscalationPolicyFactory { - type Policy: EscalationPolicy + Send + Sync + 'static; +#[derive(Debug, serde::Serialize, serde::Deserialize, rmcp::schemars::JsonSchema)] +pub struct ExecParams { + /// The bash string to execute. + pub command: String, + /// The working directory to execute the command in. Must be an absolute path. + pub workdir: String, + /// The timeout for the command in milliseconds. + pub timeout_ms: Option, + /// Launch Bash with -lc instead of -c: defaults to true. + pub login: Option, +} - fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; +impl From for codex_shell_escalation::ExecParams { + fn from(inner: ExecParams) -> Self { + Self { + command: inner.command, + workdir: inner.workdir, + timeout_ms: inner.timeout_ms, + login: inner.login, + } + } } struct McpEscalationPolicyFactory { @@ -101,7 +102,11 @@ struct McpEscalationPolicyFactory { impl EscalationPolicyFactory for McpEscalationPolicyFactory { type Policy = McpEscalationPolicy; - fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { + fn create_policy( + &self, + policy: Arc>, + stopwatch: codex_shell_escalation::Stopwatch, + ) -> Self::Policy { McpEscalationPolicy::new( policy, self.context.clone(), @@ -153,8 +158,8 @@ impl ExecTool { use_linux_sandbox_bwrap: false, }); let result = run_escalate_server( - params, - sandbox_state, + params.into(), + &sandbox_state, &self.bash_path, &self.execve_wrapper, self.policy.clone(), @@ -172,48 +177,6 @@ impl ExecTool { } } -/// Runs the escalate server to execute a shell command with potential -/// escalation of execve calls. -/// -/// - `exec_params` defines the shell command to run -/// - `sandbox_state` is the sandbox to use to run the shell program -/// - `shell_program` is the path to the shell program to run (e.g. /bin/bash) -/// - `execve_wrapper` is the path to the execve wrapper binary to use for -/// handling execve calls from the shell program. This is likely a symlink to -/// Codex using a special name. -/// - `policy` is the exec policy to use for deciding whether to allow or deny -/// execve calls from the shell program. -/// - `escalation_policy_factory` is a factory for creating an -/// `EscalationPolicy` to use for deciding whether to allow, deny, or prompt -/// the user for execve calls from the shell program. We use a factory here -/// because the `EscalationPolicy` may need to capture request-specific -/// context (e.g. the MCP request context) that is not available at the time -/// we create the `ExecTool`. -/// - `effective_timeout` is the timeout to use for running the shell command. -/// Implementations are encouraged to excludeany time spent prompting the -/// user. -async fn run_escalate_server( - exec_params: ExecParams, - sandbox_state: SandboxState, - shell_program: impl AsRef, - execve_wrapper: impl AsRef, - policy: Arc>, - escalation_policy_factory: impl EscalationPolicyFactory, - effective_timeout: Duration, -) -> anyhow::Result { - let stopwatch = Stopwatch::new(effective_timeout); - let cancel_token = stopwatch.cancellation_token(); - let escalate_server = EscalateServer::new( - shell_program.as_ref().to_path_buf(), - execve_wrapper.as_ref().to_path_buf(), - escalation_policy_factory.create_policy(policy, stopwatch), - ); - - escalate_server - .exec(exec_params, cancel_token, &sandbox_state) - .await -} - #[derive(Default)] pub struct CodexSandboxStateUpdateMethod; @@ -307,7 +270,7 @@ mod tests { /// `timeout_ms` fields are optional. #[test] fn exec_params_json_schema_matches_expected() { - let schema = schemars::schema_for!(ExecParams); + let schema = rmcp::schemars::schema_for!(ExecParams); let actual = serde_json::to_value(schema).expect("schema should serialize"); assert_eq!( diff --git a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs similarity index 92% rename from codex-rs/exec-server/src/posix/mcp_escalation_policy.rs rename to codex-rs/exec-server/src/unix/mcp_escalation_policy.rs index 5e6b97f1ad..9863818261 100644 --- a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs +++ b/codex-rs/exec-server/src/unix/mcp_escalation_policy.rs @@ -2,6 +2,9 @@ use std::path::Path; use codex_core::sandboxing::SandboxPermissions; use codex_execpolicy::Policy; +use codex_shell_escalation::EscalateAction; +use codex_shell_escalation::EscalationPolicy; +use codex_shell_escalation::Stopwatch; use rmcp::ErrorData as McpError; use rmcp::RoleServer; use rmcp::model::CreateElicitationRequestParams; @@ -9,10 +12,7 @@ use rmcp::model::CreateElicitationResult; use rmcp::model::ElicitationAction; use rmcp::model::ElicitationSchema; use rmcp::service::RequestContext; - -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::stopwatch::Stopwatch; +use shlex::try_join; use std::sync::Arc; use tokio::sync::RwLock; @@ -59,7 +59,7 @@ impl McpEscalationPolicy { workdir: &Path, context: RequestContext, ) -> Result { - let args = shlex::try_join(argv.iter().skip(1).map(String::as_str)).unwrap_or_default(); + let args = try_join(argv.iter().skip(1).map(String::as_str)).unwrap_or_default(); let command = if args.is_empty() { file.display().to_string() } else { @@ -104,10 +104,10 @@ impl EscalationPolicy for McpEscalationPolicy { file: &Path, argv: &[String], workdir: &Path, - ) -> Result { + ) -> anyhow::Result { let policy = self.policy.read().await; let outcome = - crate::posix::evaluate_exec_policy(&policy, file, argv, self.preserve_program_paths)?; + crate::unix::evaluate_exec_policy(&policy, file, argv, self.preserve_program_paths)?; let action = match outcome { ExecPolicyOutcome::Allow { sandbox_permissions, diff --git a/codex-rs/shell-escalation/BUILD.bazel b/codex-rs/shell-escalation/BUILD.bazel new file mode 100644 index 0000000000..5092f33324 --- /dev/null +++ b/codex-rs/shell-escalation/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "shell-escalation", + crate_name = "codex_shell_escalation", +) diff --git a/codex-rs/shell-escalation/Cargo.toml b/codex-rs/shell-escalation/Cargo.toml new file mode 100644 index 0000000000..49b8f10bd3 --- /dev/null +++ b/codex-rs/shell-escalation/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "codex-shell-escalation" +version.workspace = true +edition.workspace = true +license.workspace = true + +[dependencies] +anyhow = { workspace = true } +async-trait = { workspace = true } +codex-core = { workspace = true } +codex-execpolicy = { workspace = true } +codex-protocol = { workspace = true } +libc = { workspace = true } +serde_json = { workspace = true } +path-absolutize = { workspace = true } +serde = { workspace = true, features = ["derive"] } +socket2 = { workspace = true } +tokio = { workspace = true, features = [ + "io-std", + "macros", + "process", + "rt-multi-thread", + "signal", +] } +tokio-util = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/shell-escalation/src/lib.rs b/codex-rs/shell-escalation/src/lib.rs new file mode 100644 index 0000000000..555d0f89e0 --- /dev/null +++ b/codex-rs/shell-escalation/src/lib.rs @@ -0,0 +1,21 @@ +#[cfg(unix)] +mod unix { + mod escalate_client; + mod escalate_protocol; + mod escalate_server; + mod escalation_policy; + mod socket; + mod stopwatch; + + pub use self::escalate_client::run; + pub use self::escalate_protocol::EscalateAction; + pub use self::escalate_server::EscalationPolicyFactory; + pub use self::escalate_server::ExecParams; + pub use self::escalate_server::ExecResult; + pub use self::escalate_server::run_escalate_server; + pub use self::escalation_policy::EscalationPolicy; + pub use self::stopwatch::Stopwatch; +} + +#[cfg(unix)] +pub use unix::*; diff --git a/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs new file mode 100644 index 0000000000..0be7af28fa --- /dev/null +++ b/codex-rs/shell-escalation/src/unix/core_shell_escalation.rs @@ -0,0 +1,71 @@ +use async_trait::async_trait; +use std::path::Path; +use std::sync::Arc; +use tokio::sync::RwLock; + +use crate::escalate_protocol::EscalateAction; +use crate::escalation_policy::EscalationPolicy; +use crate::stopwatch::Stopwatch; +use crate::unix::escalate_server::EscalationPolicyFactory; +use codex_execpolicy::Policy; + +#[async_trait] +pub trait ShellActionProvider: Send + Sync { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + stopwatch: &Stopwatch, + ) -> anyhow::Result; +} + +#[derive(Clone)] +pub struct ShellPolicyFactory { + provider: Arc, +} + +impl ShellPolicyFactory { + pub fn new

(provider: P) -> Self + where + P: ShellActionProvider + 'static, + { + Self { + provider: Arc::new(provider), + } + } + + pub fn with_provider(provider: Arc) -> Self { + Self { provider } + } +} + +struct ShellEscalationPolicy { + provider: Arc, + stopwatch: Stopwatch, +} + +#[async_trait] +impl EscalationPolicy for ShellEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + ) -> anyhow::Result { + self.provider + .determine_action(file, argv, workdir, &self.stopwatch) + .await + } +} + +impl EscalationPolicyFactory for ShellPolicyFactory { + type Policy = ShellEscalationPolicy; + + fn create_policy(&self, _policy: Arc>, stopwatch: Stopwatch) -> Self::Policy { + ShellEscalationPolicy { + provider: Arc::clone(&self.provider), + stopwatch, + } + } +} diff --git a/codex-rs/exec-server/src/posix/escalate_client.rs b/codex-rs/shell-escalation/src/unix/escalate_client.rs similarity index 85% rename from codex-rs/exec-server/src/posix/escalate_client.rs rename to codex-rs/shell-escalation/src/unix/escalate_client.rs index 1cb0c5908d..17d8e6577b 100644 --- a/codex-rs/exec-server/src/posix/escalate_client.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_client.rs @@ -5,16 +5,16 @@ use std::os::fd::OwnedFd; use anyhow::Context as _; -use crate::posix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; -use crate::posix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalate_protocol::EscalateRequest; -use crate::posix::escalate_protocol::EscalateResponse; -use crate::posix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::SuperExecMessage; -use crate::posix::escalate_protocol::SuperExecResult; -use crate::posix::socket::AsyncDatagramSocket; -use crate::posix::socket::AsyncSocket; +use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; +use crate::unix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::EscalateAction; +use crate::unix::escalate_protocol::EscalateRequest; +use crate::unix::escalate_protocol::EscalateResponse; +use crate::unix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::SuperExecMessage; +use crate::unix::escalate_protocol::SuperExecResult; +use crate::unix::socket::AsyncDatagramSocket; +use crate::unix::socket::AsyncSocket; fn get_escalate_client() -> anyhow::Result { // TODO: we should defensively require only calling this once, since AsyncSocket will take ownership of the fd. @@ -27,7 +27,7 @@ fn get_escalate_client() -> anyhow::Result { Ok(unsafe { AsyncDatagramSocket::from_raw_fd(client_fd) }?) } -pub(crate) async fn run(file: String, argv: Vec) -> anyhow::Result { +pub async fn run(file: String, argv: Vec) -> anyhow::Result { let handshake_client = get_escalate_client()?; let (server, client) = AsyncSocket::pair()?; const HANDSHAKE_MESSAGE: [u8; 1] = [0]; diff --git a/codex-rs/exec-server/src/posix/escalate_protocol.rs b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs similarity index 66% rename from codex-rs/exec-server/src/posix/escalate_protocol.rs rename to codex-rs/shell-escalation/src/unix/escalate_protocol.rs index 7245f88fbe..3bf2f712df 100644 --- a/codex-rs/exec-server/src/posix/escalate_protocol.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs @@ -6,33 +6,33 @@ use serde::Deserialize; use serde::Serialize; /// 'exec-server escalate' reads this to find the inherited FD for the escalate socket. -pub(super) const ESCALATE_SOCKET_ENV_VAR: &str = "CODEX_ESCALATE_SOCKET"; +pub const ESCALATE_SOCKET_ENV_VAR: &str = "CODEX_ESCALATE_SOCKET"; /// Patched shells use this to wrap exec() calls. -pub(super) const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; +pub const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; /// Compatibility alias for older patched bash builds. -pub(super) const LEGACY_BASH_EXEC_WRAPPER_ENV_VAR: &str = "BASH_EXEC_WRAPPER"; +pub const LEGACY_BASH_EXEC_WRAPPER_ENV_VAR: &str = "BASH_EXEC_WRAPPER"; /// The client sends this to the server to request an exec() call. #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] -pub(super) struct EscalateRequest { +pub struct EscalateRequest { /// The absolute path to the executable to run, i.e. the first arg to exec. - pub(super) file: PathBuf, + pub file: PathBuf, /// The argv, including the program name (argv[0]). - pub(super) argv: Vec, - pub(super) workdir: PathBuf, - pub(super) env: HashMap, + pub argv: Vec, + pub workdir: PathBuf, + pub env: HashMap, } /// The server sends this to the client to respond to an exec() request. #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] -pub(super) struct EscalateResponse { - pub(super) action: EscalateAction, +pub struct EscalateResponse { + pub action: EscalateAction, } #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] -pub(super) enum EscalateAction { +pub enum EscalateAction { /// The command should be run directly by the client. Run, /// The command should be escalated to the server for execution. @@ -43,12 +43,12 @@ pub(super) enum EscalateAction { /// The client sends this to the server to forward its open FDs. #[derive(Clone, Serialize, Deserialize, Debug)] -pub(super) struct SuperExecMessage { - pub(super) fds: Vec, +pub struct SuperExecMessage { + pub fds: Vec, } /// The server responds when the exec()'d command has exited. #[derive(Clone, Serialize, Deserialize, Debug)] -pub(super) struct SuperExecResult { - pub(super) exit_code: i32, +pub struct SuperExecResult { + pub exit_code: i32, } diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs similarity index 76% rename from codex-rs/exec-server/src/posix/escalate_server.rs rename to codex-rs/shell-escalation/src/unix/escalate_server.rs index de4bdfc2cd..0ee5fc27c4 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -1,35 +1,54 @@ use std::collections::HashMap; use std::os::fd::AsRawFd; +use std::path::Path; use std::path::PathBuf; use std::process::Stdio; use std::sync::Arc; use std::time::Duration; use anyhow::Context as _; -use path_absolutize::Absolutize as _; - use codex_core::SandboxState; -use codex_core::exec::process_exec_tool_call; -use codex_core::sandboxing::SandboxPermissions; -use codex_protocol::config_types::WindowsSandboxLevel; +use codex_execpolicy::Policy; +use path_absolutize::Absolutize as _; use tokio::process::Command; +use tokio::sync::RwLock; use tokio_util::sync::CancellationToken; -use crate::posix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; -use crate::posix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalate_protocol::EscalateRequest; -use crate::posix::escalate_protocol::EscalateResponse; -use crate::posix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; -use crate::posix::escalate_protocol::SuperExecMessage; -use crate::posix::escalate_protocol::SuperExecResult; -use crate::posix::escalation_policy::EscalationPolicy; -use crate::posix::mcp::ExecParams; -use crate::posix::socket::AsyncDatagramSocket; -use crate::posix::socket::AsyncSocket; -use codex_core::exec::ExecExpiration; +use crate::unix::escalate_protocol::ESCALATE_SOCKET_ENV_VAR; +use crate::unix::escalate_protocol::EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::EscalateAction; +use crate::unix::escalate_protocol::EscalateRequest; +use crate::unix::escalate_protocol::EscalateResponse; +use crate::unix::escalate_protocol::LEGACY_BASH_EXEC_WRAPPER_ENV_VAR; +use crate::unix::escalate_protocol::SuperExecMessage; +use crate::unix::escalate_protocol::SuperExecResult; +use crate::unix::escalation_policy::EscalationPolicy; +use crate::unix::socket::AsyncDatagramSocket; +use crate::unix::socket::AsyncSocket; +use crate::unix::stopwatch::Stopwatch; -pub(crate) struct EscalateServer { +#[derive(Debug, serde::Deserialize, serde::Serialize)] +pub struct ExecParams { + /// The bash string to execute. + pub command: String, + /// The working directory to execute the command in. Must be an absolute path. + pub workdir: String, + /// The timeout for the command in milliseconds. + pub timeout_ms: Option, + /// Launch Bash with -lc instead of -c: defaults to true. + pub login: Option, +} + +#[derive(Debug, serde::Serialize, serde::Deserialize)] +pub struct ExecResult { + pub exit_code: i32, + pub output: String, + pub duration: Duration, + pub timed_out: bool, +} + +#[allow(clippy::module_name_repetitions)] +pub struct EscalateServer { bash_path: PathBuf, execve_wrapper: PathBuf, policy: Arc, @@ -78,7 +97,7 @@ impl EscalateServer { timeout_ms: _, login, } = params; - let result = process_exec_tool_call( + let result = codex_core::exec::process_exec_tool_call( codex_core::exec::ExecParams { command: vec![ self.bash_path.to_string_lossy().to_string(), @@ -90,11 +109,11 @@ impl EscalateServer { command, ], cwd: PathBuf::from(&workdir), - expiration: ExecExpiration::Cancellation(cancel_rx), + expiration: codex_core::exec::ExecExpiration::Cancellation(cancel_rx), env, network: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Disabled, + sandbox_permissions: codex_core::sandboxing::SandboxPermissions::UseDefault, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, justification: None, arg0: None, }, @@ -106,16 +125,45 @@ impl EscalateServer { ) .await?; escalate_task.abort(); - let result = ExecResult { + + Ok(ExecResult { exit_code: result.exit_code, output: result.aggregated_output.text, duration: result.duration, timed_out: result.timed_out, - }; - Ok(result) + }) } } +/// Factory for creating escalation policy instances for a single shell run. +pub trait EscalationPolicyFactory { + type Policy: EscalationPolicy + Send + Sync + 'static; + + fn create_policy(&self, policy: Arc>, stopwatch: Stopwatch) -> Self::Policy; +} + +pub async fn run_escalate_server( + exec_params: ExecParams, + sandbox_state: &SandboxState, + shell_program: impl AsRef, + execve_wrapper: impl AsRef, + policy: Arc>, + escalation_policy_factory: impl EscalationPolicyFactory, + effective_timeout: Duration, +) -> anyhow::Result { + let stopwatch = Stopwatch::new(effective_timeout); + let cancel_token = stopwatch.cancellation_token(); + let escalate_server = EscalateServer::new( + shell_program.as_ref().to_path_buf(), + execve_wrapper.as_ref().to_path_buf(), + escalation_policy_factory.create_policy(policy, stopwatch), + ); + + escalate_server + .exec(exec_params, cancel_token, sandbox_state) + .await +} + async fn escalate_task( socket: AsyncDatagramSocket, policy: Arc, @@ -136,14 +184,6 @@ async fn escalate_task( } } -#[derive(Debug)] -pub(crate) struct ExecResult { - pub(crate) exit_code: i32, - pub(crate) output: String, - pub(crate) duration: Duration, - pub(crate) timed_out: bool, -} - async fn handle_escalate_session_with_policy( socket: AsyncSocket, policy: Arc, @@ -158,7 +198,8 @@ async fn handle_escalate_session_with_policy( let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); let action = policy .determine_action(file.as_path(), &argv, &workdir) - .await?; + .await + .context("failed to determine escalation action")?; tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}"); @@ -253,7 +294,7 @@ mod tests { _file: &Path, _argv: &[String], _workdir: &Path, - ) -> Result { + ) -> anyhow::Result { Ok(self.action.clone()) } } diff --git a/codex-rs/exec-server/src/posix/escalation_policy.rs b/codex-rs/shell-escalation/src/unix/escalation_policy.rs similarity index 62% rename from codex-rs/exec-server/src/posix/escalation_policy.rs rename to codex-rs/shell-escalation/src/unix/escalation_policy.rs index a7fcc4f627..09e74e2719 100644 --- a/codex-rs/exec-server/src/posix/escalation_policy.rs +++ b/codex-rs/shell-escalation/src/unix/escalation_policy.rs @@ -1,14 +1,14 @@ use std::path::Path; -use crate::posix::escalate_protocol::EscalateAction; +use crate::unix::escalate_protocol::EscalateAction; /// Decides what action to take in response to an execve request from a client. #[async_trait::async_trait] -pub(crate) trait EscalationPolicy: Send + Sync { +pub trait EscalationPolicy: Send + Sync { async fn determine_action( &self, file: &Path, argv: &[String], workdir: &Path, - ) -> Result; + ) -> anyhow::Result; } diff --git a/codex-rs/shell-escalation/src/unix/mod.rs b/codex-rs/shell-escalation/src/unix/mod.rs new file mode 100644 index 0000000000..0ae7941da2 --- /dev/null +++ b/codex-rs/shell-escalation/src/unix/mod.rs @@ -0,0 +1,7 @@ +pub mod escalate_client; +pub mod escalate_protocol; +pub mod escalate_server; +pub mod escalation_policy; +pub mod socket; +pub mod core_shell_escalation; +pub mod stopwatch; diff --git a/codex-rs/exec-server/src/posix/socket.rs b/codex-rs/shell-escalation/src/unix/socket.rs similarity index 100% rename from codex-rs/exec-server/src/posix/socket.rs rename to codex-rs/shell-escalation/src/unix/socket.rs diff --git a/codex-rs/exec-server/src/posix/stopwatch.rs b/codex-rs/shell-escalation/src/unix/stopwatch.rs similarity index 96% rename from codex-rs/exec-server/src/posix/stopwatch.rs rename to codex-rs/shell-escalation/src/unix/stopwatch.rs index de29a45685..f564434813 100644 --- a/codex-rs/exec-server/src/posix/stopwatch.rs +++ b/codex-rs/shell-escalation/src/unix/stopwatch.rs @@ -8,7 +8,7 @@ use tokio::sync::Notify; use tokio_util::sync::CancellationToken; #[derive(Clone, Debug)] -pub(crate) struct Stopwatch { +pub struct Stopwatch { limit: Duration, inner: Arc>, notify: Arc, @@ -22,7 +22,7 @@ struct StopwatchState { } impl Stopwatch { - pub(crate) fn new(limit: Duration) -> Self { + pub fn new(limit: Duration) -> Self { Self { inner: Arc::new(Mutex::new(StopwatchState { elapsed: Duration::ZERO, @@ -34,7 +34,7 @@ impl Stopwatch { } } - pub(crate) fn cancellation_token(&self) -> CancellationToken { + pub fn cancellation_token(&self) -> CancellationToken { let limit = self.limit; let token = CancellationToken::new(); let cancel = token.clone(); @@ -80,7 +80,7 @@ impl Stopwatch { /// resumes automatically when the future completes. Nested/overlapping /// calls are reference-counted so the stopwatch only resumes when every /// pause is lifted. - pub(crate) async fn pause_for(&self, fut: F) -> T + pub async fn pause_for(&self, fut: F) -> T where F: Future, { From 3cea3e665e238eaa431ca443e353a7f7f34e6462 Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Mon, 23 Feb 2026 10:05:41 -0800 Subject: [PATCH 21/29] app-server: box request dispatch future to reduce stack pressure (#12421) --- codex-rs/app-server/src/message_processor.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index e80e1790f8..de4418f0e9 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -49,6 +49,7 @@ use codex_core::default_client::set_default_originator; use codex_feedback::CodexFeedback; use codex_protocol::ThreadId; use codex_protocol::protocol::SessionSource; +use futures::FutureExt; use tokio::sync::broadcast; use tokio::time::Duration; use tokio::time::timeout; @@ -386,8 +387,12 @@ impl MessageProcessor { .await; } other => { + // Box the delegated future so this wrapper's async state machine does not + // inline the full `CodexMessageProcessor::process_request` future, which + // can otherwise push worker-thread stack usage over the edge. self.codex_message_processor .process_request(connection_id, other) + .boxed() .await; } } From 5c52ef8e6037c01d9e1c0bade4ce1be21b3950c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Feb 2026 10:24:35 -0800 Subject: [PATCH 22/29] chore(deps): bump libc from 0.2.180 to 0.2.182 in /codex-rs (#12528) Bumps [libc](https://github.com/rust-lang/libc) from 0.2.180 to 0.2.182.

Release notes

Sourced from libc's releases.

0.2.182

Added

  • Android, Linux: Add tgkill (#4970)
  • Redox: Add RENAME_NOREPLACE (#4968)
  • Redox: Add renameat2 (#4968)

0.2.181

Added

  • Apple: Add MADV_ZERO (#4924)
  • Redox: Add makedev, major, and minor (#4928)
  • GLibc: Add PTRACE_SET_SYSCALL_INFO (#4933)
  • OpenBSD: Add more kqueue related constants for (#4945)
  • Linux: add CAN error types (#4944)
  • OpenBSD: Add siginfo_t::si_status (#4946)
  • QNX NTO: Add max_align_t (#4927)
  • Illumos: Add _CS_PATH (#4956)
  • OpenBSD: add ppoll (#4957)

Fixed

  • Breaking: Redox: Fix the type of dev_t (#4928)
  • AIX: Change 'tv_nsec' of 'struct timespec' to type 'c_long' (#4931)
  • AIX: Use 'struct st_timespec' in 'struct stat{,64}' (#4931)
  • Glibc: Link old version of tc{g,s}etattr (#4938)
  • Glibc: Link the correct version of cf{g,s}et{i,o}speed on mips{32,64}r6 (#4938)
  • OpenBSD: Fix constness of tm.tm_zone (#4948)
  • OpenBSD: Fix the definition of ptrace_thread_state (#4947)
  • QuRT: Fix type visibility and defs (#4932)
  • Redox: Fix values for PTHREAD_MUTEX_{NORMAL, RECURSIVE} (#4943)
  • Various: Mark additional fields as private padding (#4922)

Changed

  • Fuchsia: Update SO_* constants (#4937)
  • Revert "musl: convert inline timespecs to timespec" (resolves build issues on targets only supported by Musl 1.2.3+ ) (#4958)
Changelog

Sourced from libc's changelog.

0.2.182 - 2026-02-13

Added

  • Android, Linux: Add tgkill (#4970)
  • Redox: Add RENAME_NOREPLACE (#4968)
  • Redox: Add renameat2 (#4968)

0.2.181 - 2026-02-09

Added

  • Apple: Add MADV_ZERO (#4924)
  • Redox: Add makedev, major, and minor (#4928)
  • GLibc: Add PTRACE_SET_SYSCALL_INFO (#4933)
  • OpenBSD: Add more kqueue related constants for (#4945)
  • Linux: add CAN error types (#4944)
  • OpenBSD: Add siginfo_t::si_status (#4946)
  • QNX NTO: Add max_align_t (#4927)
  • Illumos: Add _CS_PATH (#4956)
  • OpenBSD: add ppoll (#4957)

Fixed

  • breaking: Redox: Fix the type of dev_t (#4928)
  • AIX: Change 'tv_nsec' of 'struct timespec' to type 'c_long' (#4931)
  • AIX: Use 'struct st_timespec' in 'struct stat{,64}' (#4931)
  • Glibc: Link old version of tc{g,s}etattr (#4938)
  • Glibc: Link the correct version of cf{g,s}et{i,o}speed on mips{32,64}r6 (#4938)
  • OpenBSD: Fix constness of tm.tm_zone (#4948)
  • OpenBSD: Fix the definition of ptrace_thread_state (#4947)
  • QuRT: Fix type visibility and defs (#4932)
  • Redox: Fix values for PTHREAD_MUTEX_{NORMAL, RECURSIVE} (#4943)
  • Various: Mark additional fields as private padding (#4922)

Changed

  • Fuchsia: Update SO_* constants (#4937)
  • Revert "musl: convert inline timespecs to timespec" (resolves build issues on targets only supported by Musl 1.2.3+ ) (#4958)
Commits
  • e879ee9 chore: Release libc 0.2.182
  • 2efe72f remove copyright year in LICENSE-MIT
  • 634bc4e ci: Update the list of tested and documented targets
  • d7aa109 Revert "Disable hexagon-unknown-linux-musl testing for now"
  • 14e2f56 Revert "ci: Skip hexagon-unknown-linux-musl"
  • b7807c3 Revert "aix: Temporarily skip checking powerpc64-ibm-aix builds"
  • abe93a0 feat(linux): add tgkill for Linux and Android
  • 25f7dde feat(redox): add RENAME_NOREPLACE
  • 4b4ce4f feat(redox): add renameat2
  • ab8c36c build(deps): bump vmactions/solaris-vm from 1.2.8 to 1.3.0
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=libc&package-manager=cargo&previous-version=0.2.180&new-version=0.2.182)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- codex-rs/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index b5e5cd5ac3..d1db0e8231 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -5135,9 +5135,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.180" +version = "0.2.182" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcc35a38544a891a5f7c865aca548a982ccb3b8650a5b06d0fd33a10283c56fc" +checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" [[package]] name = "libdbus-sys" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 56fe6d4d57..190c85e3dd 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -190,7 +190,7 @@ itertools = "0.14.0" keyring = { version = "3.6", default-features = false } landlock = "0.4.4" lazy_static = "1" -libc = "0.2.177" +libc = "0.2.182" log = "0.4" lru = "0.16.3" maplit = "1.0.2" From fec517cd38ee47f552b911f8708f3e7852ca922e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Feb 2026 10:25:05 -0800 Subject: [PATCH 23/29] chore(deps): bump syn from 2.0.114 to 2.0.117 in /codex-rs (#12529) Bumps [syn](https://github.com/dtolnay/syn) from 2.0.114 to 2.0.117.
Release notes

Sourced from syn's releases.

2.0.117

  • Fix parsing of self:: pattern in first function argument (#1970)

2.0.116

  • Optimize parse_fn_arg_or_variadic for less lookahead on erroneous receiver (#1968)

2.0.115

  • Enable GenericArgument::Constraint parsing in non-full mode (#1966)
Commits
  • 7bcb37c Release 2.0.117
  • 9c6e7d3 Merge pull request #1970 from dtolnay/receiver
  • 019a848 Fix self:: pattern in first function argument
  • 23f54f3 Update test suite to nightly-2026-02-18
  • b99b9a6 Unpin CI miri toolchain
  • a62e54a Release 2.0.116
  • 5a8ed9f Merge pull request #1968 from dtolnay/receiver
  • 813afcc Optimize parse_fn_arg_or_variadic for less lookahead on erroneous receiver
  • c172150 Add regression test for issue 1718
  • 0071ab3 Ignore type_complexity clippy lint
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=syn&package-manager=cargo&previous-version=2.0.114&new-version=2.0.117)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 148 ++++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d1db0e8231..7e475eeaaa 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -282,7 +282,7 @@ checksum = "fe233a377643e0fc1a56421d7c90acdec45c291b30345eb9f08e8d0ddce5a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -482,7 +482,7 @@ checksum = "3109e49b1e4909e9db6515a30c633684d68cdeaa252f215214cb4fa1a5bfee2c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -494,7 +494,7 @@ checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -632,7 +632,7 @@ checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -672,7 +672,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -689,7 +689,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1047,7 +1047,7 @@ dependencies = [ "darling 0.20.11", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1223,7 +1223,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1845,7 +1845,7 @@ version = "0.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -2929,7 +2929,7 @@ checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -2973,7 +2973,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -2987,7 +2987,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3000,7 +3000,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3011,7 +3011,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core 0.20.11", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3022,7 +3022,7 @@ checksum = "d38308df82d1080de0afee5d069fa14b0326a88c14f15c5ccda35b4a6c414c81" dependencies = [ "darling_core 0.21.3", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3033,7 +3033,7 @@ checksum = "ac3984ec7bd6cfa798e62b4a642426a5be0e68f9401cfc2a01e3fa9ea2fcdb8d" dependencies = [ "darling_core 0.23.0", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3170,7 +3170,7 @@ checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3200,7 +3200,7 @@ dependencies = [ "convert_case 0.6.0", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "unicode-xid", ] @@ -3214,7 +3214,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn 2.0.114", + "syn 2.0.117", "unicode-xid", ] @@ -3327,7 +3327,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3380,7 +3380,7 @@ checksum = "83e195b4945e88836d826124af44fdcb262ec01ef94d44f14f4fb5103f19892a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3449,7 +3449,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3470,7 +3470,7 @@ checksum = "67c78a4d8fdf9953a5c9d458f9efe940fd97a0cab0941c075a813ac594733827" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3631,7 +3631,7 @@ checksum = "a0aca10fb742cb43f9e7bb8467c91aa9bcb8e3ffbc6a6f7389bb93ffc920577d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3945,7 +3945,7 @@ checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4497,7 +4497,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", "unic-langid", ] @@ -4511,7 +4511,7 @@ dependencies = [ "i18n-config", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4851,7 +4851,7 @@ dependencies = [ "indoc", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4993,7 +4993,7 @@ checksum = "e0c84ee7f197eca9a86c6fd6cb771e55eb991632f15f2bc3ca6ec838929e6e78" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6051,7 +6051,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6386,7 +6386,7 @@ checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6626,7 +6626,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6687,7 +6687,7 @@ dependencies = [ "itertools 0.14.0", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7023,7 +7023,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7336,7 +7336,7 @@ checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7495,7 +7495,7 @@ dependencies = [ "proc-macro2", "quote", "serde_json", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7543,7 +7543,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn 2.0.114", + "syn 2.0.117", "walkdir", ] @@ -7838,7 +7838,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7850,7 +7850,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8124,7 +8124,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8135,7 +8135,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8184,7 +8184,7 @@ checksum = "175ee3e80ae9982737ca543e96133087cbd9a485eecc3bc4de9c1a37b47ea59c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8236,7 +8236,7 @@ dependencies = [ "darling 0.21.3", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8286,7 +8286,7 @@ checksum = "6f50427f258fb77356e4cd4aa0e87e2bd2c66dbcee41dc405282cae2bfc26c83" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8545,7 +8545,7 @@ dependencies = [ "quote", "sqlx-core", "sqlx-macros-core", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8568,7 +8568,7 @@ dependencies = [ "sqlx-mysql", "sqlx-postgres", "sqlx-sqlite", - "syn 2.0.114", + "syn 2.0.117", "tokio", "url", ] @@ -8753,7 +8753,7 @@ dependencies = [ "dupe", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8875,7 +8875,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8887,7 +8887,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8928,9 +8928,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.114" +version = "2.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d4d107df263a3013ef9b1879b0df87d706ff80f65a86ea879bd9c31f9b307c2a" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" dependencies = [ "proc-macro2", "quote", @@ -8954,7 +8954,7 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9081,7 +9081,7 @@ dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9092,7 +9092,7 @@ checksum = "5c89e72a01ed4c579669add59014b9a524d609c0c88c6a585ce37485879f6ffb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "test-case-core", ] @@ -9115,7 +9115,7 @@ checksum = "be35209fd0781c5401458ab66e4f98accf63553e8fae7425503e92fdd319783b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9164,7 +9164,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9175,7 +9175,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9319,7 +9319,7 @@ checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9588,7 +9588,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9687,7 +9687,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" dependencies = [ "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9757,7 +9757,7 @@ checksum = "ee6ff59666c9cbaec3533964505d39154dc4e0a56151fdea30a09ed0301f62e2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "termcolor", ] @@ -10167,7 +10167,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "wasm-bindgen-shared", ] @@ -10481,7 +10481,7 @@ checksum = "2bbd5b46c938e506ecbce286b6628a02171d56153ba733b6c741fc627ec9579b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10492,7 +10492,7 @@ checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10503,7 +10503,7 @@ checksum = "053c4c462dc91d3b1504c6fe5a726dd15e216ba718e84a0e46a88fbe5ded3515" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10514,7 +10514,7 @@ checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11099,7 +11099,7 @@ checksum = "b659052874eb698efe5b9e8cf382204678a0086ebf46982b79d6ca3182927e5d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -11150,7 +11150,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "zvariant_utils", ] @@ -11182,7 +11182,7 @@ checksum = "1328722bbf2115db7e19d69ebcc15e795719e2d66b60827c6a69a117365e37a0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11202,7 +11202,7 @@ checksum = "d71e5d6e06ab090c67b5e44993ec16b72dcbaabc526db883a360057678b48502" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -11223,7 +11223,7 @@ checksum = "85a5b4158499876c763cb03bc4e49185d3cccbabb15b33c627f7884f43db852e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11257,7 +11257,7 @@ checksum = "eadce39539ca5cb3985590102671f2567e659fca9666581ad3411d59207951f3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11388,7 +11388,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "zvariant_utils", ] @@ -11400,5 +11400,5 @@ checksum = "c51bcff7cc3dbb5055396bcf774748c3dab426b4b8659046963523cee4808340" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] From 7f75e74201363acd419470d8e96479b0c1708076 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 23 Feb 2026 10:29:26 -0800 Subject: [PATCH 24/29] Use Arc-based ToolCtx in tool runtimes (#12583) ## Why Tool handlers and runtimes needed to pass the same turn/session context for shell and non-shell workflows without duplicative ownership churn. Using shared pointers avoids temporary lifetimes and keeps existing behavior unchanged while simplifying call sites. ## What changed - Converted `ToolCtx` to store shared context handles (`Arc`-based), including updates across shell, apply-patch, and unified-exec paths. - Updated orchestrator/runtime call sites to consume the shared context consistently and remove brittle move/borrow patterns. - Kept behavior unchanged while preparing the type surface for the new shell escalation integration in the next stack commit. ## Verification - Validated this commit stack point with `just clippy` and confirmed workspace compiles cleanly in this stack state. [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12583). * #12584 * __->__ #12583 * #12556 --- .../core/src/tools/handlers/apply_patch.rs | 41 ++++++++++++------- codex-rs/core/src/tools/handlers/shell.rs | 8 ++-- .../core/src/tools/handlers/unified_exec.rs | 4 +- codex-rs/core/src/tools/orchestrator.rs | 18 ++++---- .../core/src/tools/runtimes/apply_patch.rs | 4 +- codex-rs/core/src/tools/runtimes/shell.rs | 8 ++-- .../core/src/tools/runtimes/unified_exec.rs | 4 +- codex-rs/core/src/tools/sandboxing.rs | 16 ++++---- .../core/src/unified_exec/process_manager.rs | 6 +-- 9 files changed, 60 insertions(+), 49 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 3ef79f7aa0..ae93327b81 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -31,6 +31,7 @@ use async_trait::async_trait; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; use codex_utils_absolute_path::AbsolutePathBuf; +use std::sync::Arc; pub struct ApplyPatchHandler; @@ -139,8 +140,8 @@ impl ToolHandler for ApplyPatchHandler { let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ApplyPatchRuntime::new(); let tool_ctx = ToolCtx { - session: session.as_ref(), - turn: turn.as_ref(), + session: session.clone(), + turn: turn.clone(), call_id: call_id.clone(), tool_name: tool_name.to_string(), }; @@ -149,7 +150,7 @@ impl ToolHandler for ApplyPatchHandler { &mut runtime, &req, &tool_ctx, - &turn, + turn.as_ref(), turn.approval_policy.value(), ) .await @@ -193,8 +194,8 @@ pub(crate) async fn intercept_apply_patch( command: &[String], cwd: &Path, timeout_ms: Option, - session: &Session, - turn: &TurnContext, + session: Arc, + turn: Arc, tracker: Option<&SharedTurnDiffTracker>, call_id: &str, tool_name: &str, @@ -203,11 +204,13 @@ pub(crate) async fn intercept_apply_patch( codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => { session .record_model_warning( - format!("apply_patch was requested via {tool_name}. Use the apply_patch tool instead of exec_command."), - turn, + format!( + "apply_patch was requested via {tool_name}. Use the apply_patch tool instead of exec_command." + ), + turn.as_ref(), ) .await; - match apply_patch::apply_patch(turn, changes).await { + match apply_patch::apply_patch(turn.as_ref(), changes).await { InternalApplyPatchInvocation::Output(item) => { let content = item?; Ok(Some(ToolOutput::Function { @@ -219,8 +222,12 @@ pub(crate) async fn intercept_apply_patch( let changes = convert_apply_patch_to_protocol(&apply.action); let approval_keys = file_paths_for_action(&apply.action); let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); - let event_ctx = - ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied()); + let event_ctx = ToolEventCtx::new( + session.as_ref(), + turn.as_ref(), + call_id, + tracker.as_ref().copied(), + ); emitter.begin(event_ctx).await; let req = ApplyPatchRequest { @@ -235,8 +242,8 @@ pub(crate) async fn intercept_apply_patch( let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ApplyPatchRuntime::new(); let tool_ctx = ToolCtx { - session, - turn, + session: session.clone(), + turn: turn.clone(), call_id: call_id.to_string(), tool_name: tool_name.to_string(), }; @@ -245,13 +252,17 @@ pub(crate) async fn intercept_apply_patch( &mut runtime, &req, &tool_ctx, - turn, + turn.as_ref(), turn.approval_policy.value(), ) .await .map(|result| result.output); - let event_ctx = - ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied()); + let event_ctx = ToolEventCtx::new( + session.as_ref(), + turn.as_ref(), + call_id, + tracker.as_ref().copied(), + ); let content = emitter.finish(event_ctx, out).await?; Ok(Some(ToolOutput::Function { body: FunctionCallOutputBody::Text(content), diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 4cdbf454c4..46a90681d2 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -296,8 +296,8 @@ impl ShellHandler { &exec_params.command, &exec_params.cwd, exec_params.expiration.timeout_ms(), - session.as_ref(), - turn.as_ref(), + session.clone(), + turn.clone(), Some(&tracker), &call_id, tool_name.as_str(), @@ -343,8 +343,8 @@ impl ShellHandler { let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ShellRuntime::new(); let tool_ctx = ToolCtx { - session: session.as_ref(), - turn: turn.as_ref(), + session: session.clone(), + turn: turn.clone(), call_id: call_id.clone(), tool_name, }; diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index c7d57731e3..76dbade19e 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -172,8 +172,8 @@ impl ToolHandler for UnifiedExecHandler { &command, &cwd, Some(yield_time_ms), - context.session.as_ref(), - context.turn.as_ref(), + context.session.clone(), + context.turn.clone(), Some(&tracker), &context.call_id, tool_name.as_str(), diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index afb24d4b3c..db28c93131 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -48,7 +48,7 @@ impl ToolOrchestrator { async fn run_attempt( tool: &mut T, req: &Rq, - tool_ctx: &ToolCtx<'_>, + tool_ctx: &ToolCtx, attempt: &SandboxAttempt<'_>, has_managed_network_requirements: bool, ) -> (Result, Option) @@ -56,7 +56,7 @@ impl ToolOrchestrator { T: ToolRuntime, { let network_approval = begin_network_approval( - tool_ctx.session, + &tool_ctx.session, &tool_ctx.turn.sub_id, &tool_ctx.call_id, has_managed_network_requirements, @@ -65,8 +65,8 @@ impl ToolOrchestrator { .await; let attempt_tool_ctx = ToolCtx { - session: tool_ctx.session, - turn: tool_ctx.turn, + session: tool_ctx.session.clone(), + turn: tool_ctx.turn.clone(), call_id: tool_ctx.call_id.clone(), tool_name: tool_ctx.tool_name.clone(), }; @@ -79,7 +79,7 @@ impl ToolOrchestrator { match network_approval.mode() { NetworkApprovalMode::Immediate => { let finalize_result = - finish_immediate_network_approval(tool_ctx.session, network_approval).await; + finish_immediate_network_approval(&tool_ctx.session, network_approval).await; if let Err(err) = finalize_result { return (Err(err), None); } @@ -88,7 +88,7 @@ impl ToolOrchestrator { NetworkApprovalMode::Deferred => { let deferred = network_approval.into_deferred(); if run_result.is_err() { - finish_deferred_network_approval(tool_ctx.session, deferred).await; + finish_deferred_network_approval(&tool_ctx.session, deferred).await; return (run_result, None); } (run_result, deferred) @@ -100,7 +100,7 @@ impl ToolOrchestrator { &mut self, tool: &mut T, req: &Rq, - tool_ctx: &ToolCtx<'_>, + tool_ctx: &ToolCtx, turn_ctx: &crate::codex::TurnContext, approval_policy: AskForApproval, ) -> Result, ToolError> @@ -128,7 +128,7 @@ impl ToolOrchestrator { } ExecApprovalRequirement::NeedsApproval { reason, .. } => { let approval_ctx = ApprovalCtx { - session: tool_ctx.session, + session: &tool_ctx.session, turn: turn_ctx, call_id: &tool_ctx.call_id, retry_reason: reason, @@ -256,7 +256,7 @@ impl ToolOrchestrator { && network_approval_context.is_none(); if !bypass_retry_approval { let approval_ctx = ApprovalCtx { - session: tool_ctx.session, + session: &tool_ctx.session, turn: turn_ctx, call_id: &tool_ctx.call_id, retry_reason: Some(retry_reason), diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index d0ae2f61d7..749e1abd2d 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -70,7 +70,7 @@ impl ApplyPatchRuntime { }) } - fn stdout_stream(ctx: &ToolCtx<'_>) -> Option { + fn stdout_stream(ctx: &ToolCtx) -> Option { Some(crate::exec::StdoutStream { sub_id: ctx.turn.sub_id.clone(), call_id: ctx.call_id.clone(), @@ -156,7 +156,7 @@ impl ToolRuntime for ApplyPatchRuntime { &mut self, req: &ApplyPatchRequest, attempt: &SandboxAttempt<'_>, - ctx: &ToolCtx<'_>, + ctx: &ToolCtx, ) -> Result { let spec = Self::build_command_spec(req)?; let env = attempt diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index bcbeafadfe..11ad669169 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -60,7 +60,7 @@ impl ShellRuntime { Self } - fn stdout_stream(ctx: &ToolCtx<'_>) -> Option { + fn stdout_stream(ctx: &ToolCtx) -> Option { Some(crate::exec::StdoutStream { sub_id: ctx.turn.sub_id.clone(), call_id: ctx.call_id.clone(), @@ -150,7 +150,7 @@ impl ToolRuntime for ShellRuntime { fn network_approval_spec( &self, req: &ShellRequest, - _ctx: &ToolCtx<'_>, + _ctx: &ToolCtx, ) -> Option { req.network.as_ref()?; Some(NetworkApprovalSpec { @@ -163,7 +163,7 @@ impl ToolRuntime for ShellRuntime { &mut self, req: &ShellRequest, attempt: &SandboxAttempt<'_>, - ctx: &ToolCtx<'_>, + ctx: &ToolCtx, ) -> Result { let base_command = &req.command; let session_shell = ctx.session.user_shell(); @@ -207,7 +207,7 @@ impl ToolRuntime for ShellRuntime { .session .services .zsh_exec_bridge - .execute_shell_request(&env, ctx.session, ctx.turn, &ctx.call_id) + .execute_shell_request(&env, &ctx.session, &ctx.turn, &ctx.call_id) .await; } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 201de7ad84..5e06ca3975 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -153,7 +153,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt fn network_approval_spec( &self, req: &UnifiedExecRequest, - _ctx: &ToolCtx<'_>, + _ctx: &ToolCtx, ) -> Option { req.network.as_ref()?; Some(NetworkApprovalSpec { @@ -166,7 +166,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt &mut self, req: &UnifiedExecRequest, attempt: &SandboxAttempt<'_>, - ctx: &ToolCtx<'_>, + ctx: &ToolCtx, ) -> Result { let base_command = &req.command; let session_shell = ctx.session.user_shell(); diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 9721fed535..540eb16434 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -18,14 +18,14 @@ use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; +use futures::Future; +use futures::future::BoxFuture; +use serde::Serialize; use std::collections::HashMap; use std::fmt::Debug; use std::hash::Hash; use std::path::Path; - -use futures::Future; -use futures::future::BoxFuture; -use serde::Serialize; +use std::sync::Arc; #[derive(Clone, Default, Debug)] pub(crate) struct ApprovalStore { @@ -267,9 +267,9 @@ pub(crate) trait Sandboxable { } } -pub(crate) struct ToolCtx<'a> { - pub session: &'a Session, - pub turn: &'a TurnContext, +pub(crate) struct ToolCtx { + pub session: Arc, + pub turn: Arc, pub call_id: String, pub tool_name: String, } @@ -281,7 +281,7 @@ pub(crate) enum ToolError { } pub(crate) trait ToolRuntime: Approvable + Sandboxable { - fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx<'_>) -> Option { + fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx) -> Option { None } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 68c4f95ceb..a3da4ba6ba 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -594,8 +594,8 @@ impl UnifiedExecProcessManager { exec_approval_requirement, }; let tool_ctx = ToolCtx { - session: context.session.as_ref(), - turn: context.turn.as_ref(), + session: context.session.clone(), + turn: context.turn.clone(), call_id: context.call_id.clone(), tool_name: "exec_command".to_string(), }; @@ -604,7 +604,7 @@ impl UnifiedExecProcessManager { &mut runtime, &req, &tool_ctx, - context.turn.as_ref(), + &context.turn, context.turn.approval_policy.value(), ) .await From bfe622f4955b8a25ad64253b7ebb4f80a452c904 Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Mon, 23 Feb 2026 11:45:46 -0800 Subject: [PATCH 25/29] fix: add ellipsis for truncated status indicator (#12540) #### What - Add ellipsis truncation of the status indicator, similar to equivalent truncation done in the footer. - Extract truncation helpers into separate file https://github.com/user-attachments/assets/a2d5f22f-8adc-456e-8059-97359194c25c #### Tests Updated relevant snapshot tests --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 2 +- codex-rs/tui/src/bottom_pane/footer.rs | 2 +- .../src/bottom_pane/multi_select_picker.rs | 2 +- .../src/bottom_pane/selection_popup_common.rs | 92 +--------------- ...er_fill_height_without_bottom_padding.snap | 2 +- ...ed_exec_begin_restores_working_status.snap | 2 +- codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/line_truncation.rs | 100 ++++++++++++++++++ ...ator_widget__tests__renders_truncated.snap | 2 +- codex-rs/tui/src/status_indicator_widget.rs | 6 +- 10 files changed, 113 insertions(+), 98 deletions(-) create mode 100644 codex-rs/tui/src/line_truncation.rs diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 8498cd9b42..62c67887b6 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -110,10 +110,10 @@ //! overall state machine, since it affects which transitions are even possible from a given UI //! state. use crate::bottom_pane::footer::mode_indicator_line; -use crate::bottom_pane::selection_popup_common::truncate_line_with_ellipsis_if_overflow; use crate::key_hint; use crate::key_hint::KeyBinding; use crate::key_hint::has_ctrl_or_alt; +use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::ui_consts::FOOTER_INDENT_COLS; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index f6f61acf4b..28005a172a 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -990,7 +990,7 @@ const SHORTCUTS: &[ShortcutDescriptor] = &[ #[cfg(test)] mod tests { use super::*; - use crate::bottom_pane::selection_popup_common::truncate_line_with_ellipsis_if_overflow; + use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::test_backend::VT100Backend; use insta::assert_snapshot; use pretty_assertions::assert_eq; diff --git a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs index 65dfbb7741..a8acf18668 100644 --- a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs +++ b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs @@ -46,8 +46,8 @@ use crate::bottom_pane::bottom_pane_view::BottomPaneView; use crate::bottom_pane::popup_consts::MAX_POPUP_ROWS; use crate::bottom_pane::scroll_state::ScrollState; use crate::bottom_pane::selection_popup_common::render_rows_single_line; -use crate::bottom_pane::selection_popup_common::truncate_line_with_ellipsis_if_overflow; use crate::key_hint; +use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::Insets; use crate::render::RectExt; use crate::render::renderable::ColumnRenderable; diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 6b526de474..85f5a1c612 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -14,6 +14,7 @@ use unicode_width::UnicodeWidthChar; use unicode_width::UnicodeWidthStr; use crate::key_hint::KeyBinding; +use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::Insets; use crate::render::RectExt as _; use crate::style::user_message_style; @@ -105,12 +106,6 @@ pub(crate) fn wrap_styled_line<'a>(line: &'a Line<'a>, width: u16) -> Vec) -> usize { - line.iter() - .map(|span| UnicodeWidthStr::width(span.content.as_ref())) - .sum() -} - fn line_to_owned(line: Line<'_>) -> Line<'static> { Line { style: line.style, @@ -126,91 +121,6 @@ fn line_to_owned(line: Line<'_>) -> Line<'static> { } } -pub(crate) fn truncate_line_to_width(line: Line<'static>, max_width: usize) -> Line<'static> { - if max_width == 0 { - return Line::from(Vec::>::new()); - } - - let Line { - style, - alignment, - spans, - } = line; - let mut used = 0usize; - let mut spans_out: Vec> = Vec::new(); - - for span in spans { - let text = span.content.into_owned(); - let style = span.style; - let span_width = UnicodeWidthStr::width(text.as_str()); - - if span_width == 0 { - spans_out.push(Span::styled(text, style)); - continue; - } - - if used >= max_width { - break; - } - - if used + span_width <= max_width { - used += span_width; - spans_out.push(Span::styled(text, style)); - continue; - } - - let mut truncated = String::new(); - for ch in text.chars() { - let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0); - if used + ch_width > max_width { - break; - } - truncated.push(ch); - used += ch_width; - } - - if !truncated.is_empty() { - spans_out.push(Span::styled(truncated, style)); - } - - break; - } - - Line { - style, - alignment, - spans: spans_out, - } -} - -pub(crate) fn truncate_line_with_ellipsis_if_overflow( - line: Line<'static>, - max_width: usize, -) -> Line<'static> { - if max_width == 0 { - return Line::from(Vec::>::new()); - } - - let width = line_width(&line); - if width <= max_width { - return line; - } - - let truncated = truncate_line_to_width(line, max_width.saturating_sub(1)); - let Line { - style, - alignment, - mut spans, - } = truncated; - let ellipsis_style = spans.last().map(|span| span.style).unwrap_or_default(); - spans.push(Span::styled("…", ellipsis_style)); - Line { - style, - alignment, - spans, - } -} - fn compute_desc_col( rows_all: &[GenericDisplayRow], start_idx: usize, diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap index 494883e4c3..5c95c9f811 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__tests__status_and_composer_fill_height_without_bottom_padding.snap @@ -2,7 +2,7 @@ source: tui/src/bottom_pane/mod.rs expression: "render_snapshot(&pane, area)" --- -• Working (0s • esc to interru +• Working (0s • esc to interr… › Ask Codex to do anything diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap index 9976dc6eea..3726917d26 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_begin_restores_working_status.snap @@ -3,7 +3,7 @@ source: tui/src/chatwidget/tests.rs expression: terminal.backend() --- " " -"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view " +"• Working (0s • esc to interrupt) · 1 background terminal running · /ps to view…" " " " " "› Ask Codex to do anything " diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 87d7089591..e0a2c7232f 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -80,6 +80,7 @@ mod get_git_diff; mod history_cell; pub mod insert_history; mod key_hint; +mod line_truncation; pub mod live_wrap; mod markdown; mod markdown_render; diff --git a/codex-rs/tui/src/line_truncation.rs b/codex-rs/tui/src/line_truncation.rs new file mode 100644 index 0000000000..d8a9408e37 --- /dev/null +++ b/codex-rs/tui/src/line_truncation.rs @@ -0,0 +1,100 @@ +use ratatui::text::Line; +use ratatui::text::Span; +use unicode_width::UnicodeWidthChar; +use unicode_width::UnicodeWidthStr; + +pub(crate) fn line_width(line: &Line<'_>) -> usize { + line.iter() + .map(|span| UnicodeWidthStr::width(span.content.as_ref())) + .sum() +} + +pub(crate) fn truncate_line_to_width(line: Line<'static>, max_width: usize) -> Line<'static> { + if max_width == 0 { + return Line::from(Vec::>::new()); + } + + let Line { + style, + alignment, + spans, + } = line; + let mut used = 0usize; + let mut spans_out: Vec> = Vec::with_capacity(spans.len()); + + for span in spans { + let span_width = UnicodeWidthStr::width(span.content.as_ref()); + + if span_width == 0 { + spans_out.push(span); + continue; + } + + if used >= max_width { + break; + } + + if used + span_width <= max_width { + used += span_width; + spans_out.push(span); + continue; + } + + let style = span.style; + let text = span.content.as_ref(); + let mut end_idx = 0usize; + for (idx, ch) in text.char_indices() { + let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0); + if used + ch_width > max_width { + break; + } + end_idx = idx + ch.len_utf8(); + used += ch_width; + } + + if end_idx > 0 { + spans_out.push(Span::styled(text[..end_idx].to_string(), style)); + } + + break; + } + + Line { + style, + alignment, + spans: spans_out, + } +} + +/// Truncate a styled line to `max_width` and append an ellipsis on overflow. +/// +/// Intended for short UI rows. This preserves a fast no-overflow path (width +/// pre-scan + return original line unchanged) and uses `truncate_line_to_width` +/// for the overflow case. +/// Performance should be reevaluated if using this method in loops/over larger content in the future. +pub(crate) fn truncate_line_with_ellipsis_if_overflow( + line: Line<'static>, + max_width: usize, +) -> Line<'static> { + if max_width == 0 { + return Line::from(Vec::>::new()); + } + + if line_width(&line) <= max_width { + return line; + } + + let truncated = truncate_line_to_width(line, max_width.saturating_sub(1)); + let Line { + style, + alignment, + mut spans, + } = truncated; + let ellipsis_style = spans.last().map(|span| span.style).unwrap_or_default(); + spans.push(Span::styled("…", ellipsis_style)); + Line { + style, + alignment, + spans, + } +} diff --git a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap index ba179808bd..7f34188617 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__status_indicator_widget__tests__renders_truncated.snap @@ -2,5 +2,5 @@ source: tui/src/status_indicator_widget.rs expression: terminal.backend() --- -"• Working (0s • esc " +"• Working (0s • esc…" " " diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index fa266f9760..01027c3c99 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -23,6 +23,7 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::exec_cell::spinner; use crate::key_hint; +use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::renderable::Renderable; use crate::shimmer::shimmer_spans; use crate::text_formatting::capitalize_first; @@ -253,7 +254,10 @@ impl Renderable for StatusIndicatorWidget { } let mut lines = Vec::new(); - lines.push(Line::from(spans)); + lines.push(truncate_line_with_ellipsis_if_overflow( + Line::from(spans), + usize::from(area.width), + )); if area.height > 1 { // If there is enough space, add the details lines below the header. let details = self.wrapped_details_lines(area.width); From 48e08a156133c5f53bbfdcafd13e0dd03379cb6a Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Mon, 23 Feb 2026 17:14:50 -0300 Subject: [PATCH 26/29] fix(tui): recover on owned wrap mapping mismatch (#12609) ## Summary - Replace the `panic!` in `map_owned_wrapped_line_to_range` with a recoverable flow that skips synthetic leading characters, logs a warning on mid-line mismatch, and returns the mapped prefix range instead of crashing - Fixes a crash when `textwrap` produces owned lines with synthetic indent prefixes (e.g. non-space indents via `initial_indent`/`subsequent_indent`) ## Test plan - [x] Added unit test for direct mismatch recovery (`map_owned_wrapped_line_to_range_recovers_on_non_prefix_mismatch`) - [x] Added end-to-end `wrap_ranges` test with non-space indents that forces owned wrapped lines and validates full source reconstruction - [x] Verify no regressions in existing `wrapping.rs` tests (`cargo test -p codex-tui`) --- codex-rs/tui/src/wrapping.rs | 166 +++++++++++++++++++++++++++++++++-- 1 file changed, 160 insertions(+), 6 deletions(-) diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index f042fbbc9c..960b8fed54 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -44,7 +44,7 @@ where let opts = width_or_options.into(); let mut lines: Vec> = Vec::new(); let mut cursor = 0usize; - for line in textwrap::wrap(text, opts).iter() { + for (line_index, line) in textwrap::wrap(text, &opts).iter().enumerate() { match line { std::borrow::Cow::Borrowed(slice) => { let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize }; @@ -54,7 +54,12 @@ where cursor = end + trailing_spaces; } std::borrow::Cow::Owned(slice) => { - let mapped = map_owned_wrapped_line_to_range(text, cursor, slice); + let synthetic_prefix = if line_index == 0 { + opts.initial_indent + } else { + opts.subsequent_indent + }; + let mapped = map_owned_wrapped_line_to_range(text, cursor, slice, synthetic_prefix); let trailing_spaces = text[mapped.end..].chars().take_while(|c| *c == ' ').count(); lines.push(mapped.start..mapped.end + trailing_spaces + 1); cursor = mapped.end + trailing_spaces; @@ -74,7 +79,7 @@ where let opts = width_or_options.into(); let mut lines: Vec> = Vec::new(); let mut cursor = 0usize; - for line in textwrap::wrap(text, opts).iter() { + for (line_index, line) in textwrap::wrap(text, &opts).iter().enumerate() { match line { std::borrow::Cow::Borrowed(slice) => { let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize }; @@ -83,7 +88,12 @@ where cursor = end; } std::borrow::Cow::Owned(slice) => { - let mapped = map_owned_wrapped_line_to_range(text, cursor, slice); + let synthetic_prefix = if line_index == 0 { + opts.initial_indent + } else { + opts.subsequent_indent + }; + let mapped = map_owned_wrapped_line_to_range(text, cursor, slice, synthetic_prefix); lines.push(mapped.clone()); cursor = mapped.end; } @@ -99,7 +109,18 @@ where /// function walks the owned string character-by-character against the /// source, skipping trailing penalty chars, and returns the /// corresponding source byte range starting from `cursor`. -fn map_owned_wrapped_line_to_range(text: &str, cursor: usize, wrapped: &str) -> Range { +fn map_owned_wrapped_line_to_range( + text: &str, + cursor: usize, + wrapped: &str, + synthetic_prefix: &str, +) -> Range { + let wrapped = if synthetic_prefix.is_empty() { + wrapped + } else { + wrapped.strip_prefix(synthetic_prefix).unwrap_or(wrapped) + }; + let mut start = cursor; while start < text.len() && !wrapped.starts_with(' ') { let Some(ch) = text[start..].chars().next() else { @@ -112,6 +133,7 @@ fn map_owned_wrapped_line_to_range(text: &str, cursor: usize, wrapped: &str) -> } let mut end = start; + let mut saw_source_char = false; let mut chars = wrapped.chars().peekable(); while let Some(ch) = chars.next() { if end < text.len() { @@ -120,6 +142,7 @@ fn map_owned_wrapped_line_to_range(text: &str, cursor: usize, wrapped: &str) -> }; if ch == src { end += src.len_utf8(); + saw_source_char = true; continue; } } @@ -131,7 +154,20 @@ fn map_owned_wrapped_line_to_range(text: &str, cursor: usize, wrapped: &str) -> continue; } - panic!("wrap_ranges: could not map owned line {wrapped:?} to source near byte {cursor}"); + // Non-source chars can be synthesized by textwrap in owned output + // (e.g. non-space indent prefixes). Keep going and map the source bytes + // we can confidently match instead of crashing the app. + if !saw_source_char { + continue; + } + + tracing::warn!( + wrapped = %wrapped, + cursor, + end, + "wrap_ranges: could not fully map owned line; returning partial source range" + ); + break; } start..end @@ -1229,6 +1265,124 @@ them."# ); } + #[test] + fn map_owned_wrapped_line_to_range_recovers_on_non_prefix_mismatch() { + // Match source chars first, then introduce a non-penalty mismatch. + // The function should recover and return the mapped prefix range. + let range = map_owned_wrapped_line_to_range("hello world", 0, "helloX", ""); + assert_eq!(range, 0..5); + } + + #[test] + fn map_owned_wrapped_line_to_range_indent_coincides_with_source() { + // When the synthetic indent prefix starts with a character that also + // appears at the current source position, the mapper must not confuse + // the indent char for a source match. Here the indent is "- " and the + // source text also starts with "-", so a naive char-by-char match would + // consume the source "-" for the indent "-", set saw_source_char too + // early, then break on the space — returning 0..1 instead of the full + // first word. + let text = "- item one and some more words"; + // Simulate what textwrap would produce for the first continuation line + // when subsequent_indent = "- ": it prepends "- " to the source slice. + let range = map_owned_wrapped_line_to_range(text, 0, "- - item one", "- "); + // The mapper should skip the synthetic "- " prefix and map "- item one" + // back to source bytes 0..10. + assert_eq!(range, 0..10); + } + + #[test] + fn wrap_ranges_indent_prefix_coincides_with_source_char() { + // End-to-end: source text starts with the same character as the indent + // prefix. wrap_ranges must still reconstruct the full source. + let text = "- first item is long enough to wrap around"; + let opts = || { + textwrap::Options::new(16) + .initial_indent("- ") + .subsequent_indent("- ") + }; + let ranges = wrap_ranges(text, opts()); + assert!(!ranges.is_empty()); + + let mut rebuilt = String::new(); + let mut cursor = 0usize; + for range in ranges { + let start = range.start.max(cursor).min(text.len()); + let end = range.end.min(text.len()); + if start < end { + rebuilt.push_str(&text[start..end]); + } + cursor = cursor.max(end); + } + assert_eq!(rebuilt, text); + } + + #[test] + fn map_owned_wrapped_line_to_range_repro_overconsumes_repeated_prefix_patterns() { + let text = "- - foo"; + let opts = textwrap::Options::new(3) + .initial_indent("- ") + .subsequent_indent("- ") + .word_separator(textwrap::WordSeparator::AsciiSpace) + .break_words(false); + let wrapped = textwrap::wrap(text, opts); + let Some(line) = wrapped.first() else { + panic!("expected at least one wrapped line"); + }; + + let mapped = map_owned_wrapped_line_to_range(text, 0, line.as_ref(), "- "); + let expected_len = line + .as_ref() + .strip_prefix("- ") + .unwrap_or(line.as_ref()) + .len(); + let mapped_len = mapped.end.saturating_sub(mapped.start); + assert!( + mapped_len <= expected_len, + "overconsumed source: text={text:?} line={line:?} mapped={mapped:?} expected_len={expected_len}" + ); + } + + #[test] + fn wrap_ranges_recovers_with_non_space_indents() { + let text = "The quick brown fox jumps over the lazy dog"; + let wrapped = textwrap::wrap( + text, + textwrap::Options::new(12) + .initial_indent("* ") + .subsequent_indent(" "), + ); + assert!( + wrapped + .iter() + .any(|line| matches!(line, std::borrow::Cow::Owned(_))), + "expected textwrap to produce owned lines with synthetic indent prefixes" + ); + + let ranges = wrap_ranges( + text, + textwrap::Options::new(12) + .initial_indent("* ") + .subsequent_indent(" "), + ); + assert!(!ranges.is_empty()); + + // wrap_ranges returns cursor-oriented ranges that may overlap by one byte; + // rebuild with cursor progression to validate full source coverage. + let mut rebuilt = String::new(); + let mut cursor = 0usize; + for range in ranges { + let start = range.start.max(cursor).min(text.len()); + let end = range.end.min(text.len()); + if start < end { + rebuilt.push_str(&text[start..end]); + } + cursor = cursor.max(end); + } + + assert_eq!(rebuilt, text); + } + #[test] fn wrap_ranges_trim_handles_owned_lines_with_penalty_char() { fn split_every_char(word: &str) -> Vec { From be4203023d0514abcfba94a0f2aacb592d72c3dc Mon Sep 17 00:00:00 2001 From: Beehive Innovations Date: Tue, 24 Feb 2026 00:58:40 +0400 Subject: [PATCH 27/29] fix(tui): queue steer Enter while final answer is still streaming to prevent dead state (#12569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This fixes a TUI race (https://github.com/openai/codex/issues/11008) where pressing Enter with Steer enabled while the assistant is still streaming the final answer could put Codex into a non-recoverable “running” state (no further prompts handled until exiting and resuming). ## Root Cause In steer mode, `InputResult::Submitted` could submit immediately even while a final-answer stream was active. That immediate submission races with turn completion and can strand turn state. ## Fix When handling `InputResult::Submitted`, we now queue instead of immediate-submit if a final-answer stream is active (`stream_controller.is_some()`). This keeps behavior deterministic: - Prompt is preserved in the queue. - `on_task_complete()` drains queued input through `maybe_send_next_queued_input()`. - Follow-up prompts continue in FIFO order after completion. ## Why this resolves the “dead mode” The problematic timing window is now converted into queueing, so prompts entered during final streaming are not lost and are processed after the current output ends. The model continues handling prompts normally without requiring `/quit` + `resume`. ## Tests Added regression coverage in `tui/src/chatwidget/tests.rs`: - `steer_enter_queues_while_final_answer_stream_is_active` - `steer_enter_during_final_stream_preserves_follow_up_prompts_in_order` Both fail on old behavior and pass with this fix. --- codex-rs/tui/src/chatwidget.rs | 9 ++- codex-rs/tui/src/chatwidget/tests.rs | 99 ++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 7ba3a95054..2cde0cab14 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3274,7 +3274,14 @@ impl ChatWidget { .bottom_pane .take_recent_submission_mention_bindings(), }; - if self.is_session_configured() && !self.is_plan_streaming_in_tui() { + // Steer submissions during active final-answer streaming can race with turn + // completion and strand the UI in a running state. Queue those inputs instead + // of injecting immediately; `on_task_complete()` drains this FIFO via + // `maybe_send_next_queued_input()`, so no typed prompt is dropped. + let should_submit_now = self.is_session_configured() + && !self.is_plan_streaming_in_tui() + && self.stream_controller.is_none(); + if should_submit_now { // Submitted is only emitted when steer is enabled. // Reset any reasoning header only when we are actually submitting a turn. self.reasoning_buffer.clear(); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1e0487eddf..4599377e91 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -3402,6 +3402,105 @@ async fn steer_enter_queues_while_plan_stream_is_active() { assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); } +#[tokio::test] +async fn steer_enter_queues_while_final_answer_stream_is_active() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; + chat.thread_id = Some(ThreadId::new()); + chat.on_task_started(); + // Keep the assistant stream open (no commit tick/finalize) to model the repro window: + // user presses Enter while the final answer is still streaming. + chat.on_agent_message_delta("Final answer line\n".to_string()); + + chat.bottom_pane.set_composer_text( + "queued while streaming".to_string(), + Vec::new(), + Vec::new(), + ); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_eq!(chat.queued_user_messages.len(), 1); + assert_eq!( + chat.queued_user_messages.front().unwrap().text, + "queued while streaming" + ); + assert_no_submit_op(&mut op_rx); + + // Once final output ends, the queued input must be submitted automatically. + chat.on_task_complete(None, false); + + assert!(chat.queued_user_messages.is_empty()); + match next_submit_op(&mut op_rx) { + Op::UserTurn { .. } => {} + other => panic!("expected Op::UserTurn after stream completion, got {other:?}"), + } +} + +#[tokio::test] +async fn steer_enter_during_final_stream_preserves_follow_up_prompts_in_order() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; + chat.thread_id = Some(ThreadId::new()); + chat.on_task_started(); + // Simulate "dead mode" repro timing by keeping a final-answer stream active while the + // user submits multiple follow-up prompts. + chat.on_agent_message_delta("Final answer line\n".to_string()); + + chat.bottom_pane + .set_composer_text("first follow-up".to_string(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + chat.bottom_pane + .set_composer_text("second follow-up".to_string(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_eq!(chat.queued_user_messages.len(), 2); + assert_eq!( + chat.queued_user_messages.front().unwrap().text, + "first follow-up" + ); + assert_eq!( + chat.queued_user_messages.back().unwrap().text, + "second follow-up" + ); + assert_no_submit_op(&mut op_rx); + + // Completion must recover by submitting the oldest queued prompt first. + chat.on_task_complete(None, false); + + let first_items = match next_submit_op(&mut op_rx) { + Op::UserTurn { items, .. } => items, + other => panic!("expected Op::UserTurn, got {other:?}"), + }; + assert_eq!( + first_items, + vec![UserInput::Text { + text: "first follow-up".to_string(), + text_elements: Vec::new(), + }] + ); + assert_eq!(chat.queued_user_messages.len(), 1); + assert_eq!( + chat.queued_user_messages.front().unwrap().text, + "second follow-up" + ); + + // A subsequent turn lifecycle should continue draining remaining queued prompts, proving + // the widget did not enter a permanently stuck state. + chat.on_task_started(); + chat.on_task_complete(None, false); + + let second_items = match next_submit_op(&mut op_rx) { + Op::UserTurn { items, .. } => items, + other => panic!("expected Op::UserTurn, got {other:?}"), + }; + assert_eq!( + second_items, + vec![UserInput::Text { + text: "second follow-up".to_string(), + text_elements: Vec::new(), + }] + ); + assert!(chat.queued_user_messages.is_empty()); +} + #[tokio::test] async fn steer_enter_submits_when_plan_stream_is_not_active() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; From cd5acf6af767db50935c018bb9902dd249adec1c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Feb 2026 13:01:15 -0800 Subject: [PATCH 28/29] chore(deps): bump owo-colors from 4.2.3 to 4.3.0 in /codex-rs (#12530) Bumps [owo-colors](https://github.com/owo-colors/owo-colors) from 4.2.3 to 4.3.0.
Release notes

Sourced from owo-colors's releases.

owo-colors 4.3.0

Fixed

  • Scripts in the scripts/ directory are no longer published in the crate package. Thanks weiznich for your first contribution!

Changed

  • Mark methods with #[rust_analyzer::completions(ignore_flyimport)] and the OwoColorize trait with #[rust_analyzer::completions(ignore_flyimport_methods)]. This prevents owo-colors methods from being completed with rust-analyzer unless the OwoColorize trait is included.

    Unfortunately, this also breaks explicit autocomplete commands such as Ctrl-Space in many editors. (The language server protocol doesn't appear to have a way to differentiate between implicit and explicit autocomplete commands.) On balance we believe this is the right approach, but please do provide feedback on [PR #141](owo-colors/owo-colors#141) if it negatively affects you.

  • Updated MSRV to Rust 1.81.

Changelog

Sourced from owo-colors's changelog.

[4.3.0] - 2026-02-22

Fixed

  • Scripts in the scripts/ directory are no longer published in the crate package. Thanks weiznich for your first contribution!

Changed

  • Mark methods with #[rust_analyzer::completions(ignore_flyimport)] and the OwoColorize trait with #[rust_analyzer::completions(ignore_flyimport_methods)]. This prevents owo-colors methods from being completed with rust-analyzer unless the OwoColorize trait is included.

    Unfortunately, this also breaks explicit autocomplete commands such as Ctrl-Space in many editors. (The language server protocol doesn't appear to have a way to differentiate between implicit and explicit autocomplete commands.) On balance we believe this is the right approach, but please do provide feedback on [PR #141](owo-colors/owo-colors#141) if it negatively affects you.

  • Updated MSRV to Rust 1.81.

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=owo-colors&package-manager=cargo&previous-version=4.2.3&new-version=4.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- codex-rs/Cargo.lock | 4 ++-- codex-rs/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7e475eeaaa..6cf06e931e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -6231,9 +6231,9 @@ dependencies = [ [[package]] name = "owo-colors" -version = "4.2.3" +version = "4.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c6901729fa79e91a0913333229e9ca5dc725089d1c363b2f4b4760709dc4a52" +checksum = "d211803b9b6b570f68772237e415a029d5a50c65d382910b879fb19d3271f94d" dependencies = [ "supports-color 2.1.0", "supports-color 3.0.2", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 190c85e3dd..1ba1cac37b 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -206,7 +206,7 @@ opentelemetry-otlp = "0.31.0" opentelemetry-semantic-conventions = "0.31.0" opentelemetry_sdk = "0.31.0" os_info = "3.12.0" -owo-colors = "4.2.0" +owo-colors = "4.3.0" path-absolutize = "3.1.1" pathdiff = "0.2" portable-pty = "0.9.0" From 50953ea39ad7a2c675a411b1175ad8f93806414a Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Mon, 23 Feb 2026 13:04:24 -0800 Subject: [PATCH 29/29] fix: show command running in background terminal in details under status indicator (#12549) #### What Display in-progress background terminal command in `status.details` (right under header) rather than inline, as it gets cut off currently. ###### Before image ###### After image #### Tests Added/updated tests --- MODULE.bazel.lock | 4 +- codex-rs/tui/src/bottom_pane/mod.rs | 15 +++- codex-rs/tui/src/chatwidget.rs | 44 +++++++++--- ...renders_command_in_single_details_row.snap | 11 +++ codex-rs/tui/src/chatwidget/tests.rs | 44 +++++++++++- codex-rs/tui/src/status_indicator_widget.rs | 71 ++++++++++++++++--- 6 files changed, 163 insertions(+), 26 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ed405dd8c4..7505ae1e4e 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -949,7 +949,7 @@ "landlock_0.4.4": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1.0\"},{\"name\":\"enumflags2\",\"req\":\"^0.7\"},{\"kind\":\"dev\",\"name\":\"lazy_static\",\"req\":\"^1\"},{\"name\":\"libc\",\"req\":\"^0.2.175\"},{\"kind\":\"dev\",\"name\":\"strum\",\"req\":\"^0.26\"},{\"kind\":\"dev\",\"name\":\"strum_macros\",\"req\":\"^0.26\"},{\"name\":\"thiserror\",\"req\":\"^2.0\"}],\"features\":{}}", "language-tags_0.3.2": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"bencher\",\"req\":\"^0.1\"},{\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0\"}],\"features\":{}}", "lazy_static_1.5.0": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"doc-comment\",\"req\":\"^0.3.1\"},{\"default_features\":false,\"features\":[\"once\"],\"name\":\"spin\",\"optional\":true,\"req\":\"^0.9.8\"},{\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1\"}],\"features\":{\"spin_no_std\":[\"spin\"]}}", - "libc_0.2.180": "{\"dependencies\":[{\"name\":\"rustc-std-workspace-core\",\"optional\":true,\"req\":\"^1.0.1\"}],\"features\":{\"align\":[],\"const-extern-fn\":[],\"default\":[\"std\"],\"extra_traits\":[],\"rustc-dep-of-std\":[\"align\",\"rustc-std-workspace-core\"],\"std\":[],\"use_std\":[\"std\"]}}", + "libc_0.2.182": "{\"dependencies\":[{\"name\":\"rustc-std-workspace-core\",\"optional\":true,\"req\":\"^1.0.1\"}],\"features\":{\"align\":[],\"const-extern-fn\":[],\"default\":[\"std\"],\"extra_traits\":[],\"rustc-dep-of-std\":[\"align\",\"rustc-std-workspace-core\"],\"std\":[],\"use_std\":[\"std\"]}}", "libdbus-sys_0.2.7": "{\"dependencies\":[{\"kind\":\"build\",\"name\":\"cc\",\"optional\":true,\"req\":\"^1.0.78\"},{\"kind\":\"build\",\"name\":\"pkg-config\",\"optional\":true,\"req\":\"^0.3\"}],\"features\":{\"default\":[\"pkg-config\"],\"vendored\":[\"cc\"]}}", "libm_0.2.16": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"no-panic\",\"req\":\"^0.1.35\"}],\"features\":{\"arch\":[],\"default\":[\"arch\"],\"force-soft-floats\":[],\"unstable\":[\"unstable-intrinsics\",\"unstable-float\"],\"unstable-float\":[],\"unstable-intrinsics\":[],\"unstable-public-internals\":[]}}", "libredox_0.1.12": "{\"dependencies\":[{\"name\":\"bitflags\",\"req\":\"^2\"},{\"name\":\"ioslice\",\"optional\":true,\"req\":\"^0.6\"},{\"name\":\"libc\",\"req\":\"^0.2\"},{\"name\":\"redox_syscall\",\"optional\":true,\"req\":\"^0.7\"}],\"features\":{\"call\":[],\"default\":[\"call\",\"std\",\"redox_syscall\"],\"mkns\":[\"ioslice\"],\"std\":[]}}", @@ -1277,7 +1277,7 @@ "supports-color_2.1.0": "{\"dependencies\":[{\"name\":\"is-terminal\",\"req\":\"^0.4.0\"},{\"name\":\"is_ci\",\"req\":\"^1.1.1\"}],\"features\":{}}", "supports-color_3.0.2": "{\"dependencies\":[{\"name\":\"is_ci\",\"req\":\"^1.2.0\"}],\"features\":{}}", "syn_1.0.109": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.46\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"regex\",\"req\":\"^1.0\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.11\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1.0\"},{\"name\":\"unicode-ident\",\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.1\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", - "syn_2.0.114": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.13\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1\"},{\"name\":\"unicode-ident\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.3.2\",\"target\":\"cfg(not(miri))\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"dep:quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote?/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", + "syn_2.0.117": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"anyhow\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"flate2\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"name\":\"quote\",\"optional\":true,\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1\"},{\"features\":[\"blocking\"],\"kind\":\"dev\",\"name\":\"reqwest\",\"req\":\"^0.13\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"syn-test-suite\",\"req\":\"^0\"},{\"kind\":\"dev\",\"name\":\"tar\",\"req\":\"^0.4.16\",\"target\":\"cfg(not(miri))\"},{\"kind\":\"dev\",\"name\":\"termcolor\",\"req\":\"^1\"},{\"name\":\"unicode-ident\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"walkdir\",\"req\":\"^2.3.2\",\"target\":\"cfg(not(miri))\"}],\"features\":{\"clone-impls\":[],\"default\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"proc-macro\"],\"derive\":[],\"extra-traits\":[],\"fold\":[],\"full\":[],\"parsing\":[],\"printing\":[\"dep:quote\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"quote?/proc-macro\"],\"test\":[\"syn-test-suite/all-features\"],\"visit\":[],\"visit-mut\":[]}}", "sync_wrapper_1.0.2": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"futures\",\"req\":\"^0.3\"},{\"default_features\":false,\"name\":\"futures-core\",\"optional\":true,\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"pin-project-lite\",\"req\":\"^0.2.7\"}],\"features\":{\"futures\":[\"futures-core\"]}}", "synstructure_0.13.2": "{\"dependencies\":[{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.60\"},{\"default_features\":false,\"name\":\"quote\",\"req\":\"^1\"},{\"default_features\":false,\"features\":[\"derive\",\"parsing\",\"printing\",\"clone-impls\",\"visit\",\"extra-traits\"],\"name\":\"syn\",\"req\":\"^2\"},{\"kind\":\"dev\",\"name\":\"synstructure_test_traits\",\"req\":\"^0.1\"}],\"features\":{\"default\":[\"proc-macro\"],\"proc-macro\":[\"proc-macro2/proc-macro\",\"syn/proc-macro\",\"quote/proc-macro\"]}}", "syntect_5.3.0": "{\"dependencies\":[{\"name\":\"bincode\",\"optional\":true,\"req\":\"^1.0\"},{\"features\":[\"html_reports\"],\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.3\"},{\"name\":\"fancy-regex\",\"optional\":true,\"req\":\"^0.16.2\"},{\"name\":\"flate2\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"fnv\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"getopts\",\"req\":\"^0.2\"},{\"name\":\"once_cell\",\"req\":\"^1.8\"},{\"default_features\":false,\"name\":\"onig\",\"optional\":true,\"req\":\"^6.5.1\"},{\"name\":\"plist\",\"optional\":true,\"req\":\"^1.3\"},{\"kind\":\"dev\",\"name\":\"pretty_assertions\",\"req\":\"^0.6\"},{\"kind\":\"dev\",\"name\":\"public-api\",\"req\":\"^0.50.1\"},{\"kind\":\"dev\",\"name\":\"rayon\",\"req\":\"^1.0.0\"},{\"kind\":\"dev\",\"name\":\"regex\",\"req\":\"^1.0\"},{\"name\":\"regex-syntax\",\"optional\":true,\"req\":\"^0.8\"},{\"kind\":\"dev\",\"name\":\"rustdoc-json\",\"req\":\"^0.9.7\"},{\"kind\":\"dev\",\"name\":\"rustup-toolchain\",\"req\":\"^0.1.5\"},{\"name\":\"serde\",\"req\":\"^1.0\"},{\"name\":\"serde_derive\",\"req\":\"^1.0\"},{\"name\":\"serde_json\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0\"},{\"name\":\"thiserror\",\"req\":\"^2.0.12\"},{\"name\":\"walkdir\",\"req\":\"^2.0\"},{\"name\":\"yaml-rust\",\"optional\":true,\"req\":\"^0.4.5\"}],\"features\":{\"default\":[\"default-onig\"],\"default-fancy\":[\"parsing\",\"default-syntaxes\",\"default-themes\",\"html\",\"plist-load\",\"yaml-load\",\"dump-load\",\"dump-create\",\"regex-fancy\"],\"default-onig\":[\"parsing\",\"default-syntaxes\",\"default-themes\",\"html\",\"plist-load\",\"yaml-load\",\"dump-load\",\"dump-create\",\"regex-onig\"],\"default-syntaxes\":[\"parsing\",\"dump-load\"],\"default-themes\":[\"dump-load\"],\"dump-create\":[\"flate2\",\"bincode\"],\"dump-load\":[\"flate2\",\"bincode\"],\"html\":[\"parsing\"],\"metadata\":[\"parsing\",\"plist-load\",\"dep:serde_json\"],\"parsing\":[\"regex-syntax\",\"fnv\",\"dump-create\",\"dump-load\"],\"plist-load\":[\"plist\",\"dep:serde_json\"],\"regex-fancy\":[\"fancy-regex\"],\"regex-onig\":[\"onig\"],\"yaml-load\":[\"yaml-rust\",\"parsing\"]}}", diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 9c7b940f0e..32e49763e9 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -131,6 +131,7 @@ pub(crate) use chat_composer::ChatComposerConfig; pub(crate) use chat_composer::InputResult; use codex_protocol::custom_prompts::CustomPrompt; +use crate::status_indicator_widget::StatusDetailsCapitalization; use crate::status_indicator_widget::StatusIndicatorWidget; pub(crate) use experimental_features_view::ExperimentalFeatureItem; pub(crate) use experimental_features_view::ExperimentalFeaturesView; @@ -549,10 +550,16 @@ impl BottomPane { /// Update the status indicator header (defaults to "Working") and details below it. /// /// Passing `None` clears any existing details. No-ops if the status indicator is not active. - pub(crate) fn update_status(&mut self, header: String, details: Option) { + pub(crate) fn update_status( + &mut self, + header: String, + details: Option, + details_capitalization: StatusDetailsCapitalization, + details_max_lines: usize, + ) { if let Some(status) = self.status.as_mut() { status.update_header(header); - status.update_details(details); + status.update_details(details, details_capitalization, details_max_lines.max(1)); self.request_redraw(); } } @@ -982,6 +989,8 @@ impl Renderable for BottomPane { mod tests { use super::*; use crate::app_event::AppEvent; + use crate::status_indicator_widget::STATUS_DETAILS_DEFAULT_MAX_LINES; + use crate::status_indicator_widget::StatusDetailsCapitalization; use codex_protocol::protocol::Op; use codex_protocol::protocol::SkillScope; use crossterm::event::KeyModifiers; @@ -1275,6 +1284,8 @@ mod tests { pane.update_status( "Working".to_string(), Some("First detail line\nSecond detail line".to_string()), + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, ); pane.set_queued_user_messages(vec!["Queued follow-up question".to_string()]); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2cde0cab14..e7b1ee371c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -243,6 +243,8 @@ use crate::render::renderable::RenderableExt; use crate::render::renderable::RenderableItem; use crate::slash_command::SlashCommand; use crate::status::RateLimitSnapshotDisplay; +use crate::status_indicator_widget::STATUS_DETAILS_DEFAULT_MAX_LINES; +use crate::status_indicator_widget::StatusDetailsCapitalization; use crate::text_formatting::truncate_text; use crate::tui::FrameRequester; mod interrupts; @@ -911,15 +913,27 @@ impl ChatWidget { /// Update the status indicator header and details. /// /// Passing `None` clears any existing details. - fn set_status(&mut self, header: String, details: Option) { + fn set_status( + &mut self, + header: String, + details: Option, + details_capitalization: StatusDetailsCapitalization, + details_max_lines: usize, + ) { self.current_status_header = header.clone(); - self.bottom_pane.update_status(header, details); + self.bottom_pane + .update_status(header, details, details_capitalization, details_max_lines); } /// Convenience wrapper around [`Self::set_status`]; /// updates the status indicator header and clears any existing details. fn set_status_header(&mut self, header: String) { - self.set_status(header, None); + self.set_status( + header, + None, + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); } /// Sets the currently rendered footer status-line value. @@ -1955,15 +1969,16 @@ impl ChatWidget { .map(|process| process.command_display.clone()); if ev.stdin.is_empty() { // Empty stdin means we are polling for background output. - // Surface this in the status header (single "waiting" surface) instead of the transcript. + // Surface this in the status indicator (single "waiting" surface) instead of + // the transcript. Keep the header short so the interrupt hint remains visible. self.bottom_pane.ensure_status_indicator(); self.bottom_pane.set_interrupt_hint_visible(true); - let header = if let Some(command) = &command_display { - format!("Waiting for background terminal · {command}") - } else { - "Waiting for background terminal".to_string() - }; - self.set_status_header(header); + self.set_status( + "Waiting for background terminal".to_string(), + command_display.clone(), + StatusDetailsCapitalization::Preserve, + 1, + ); match &mut self.unified_exec_wait_streak { Some(wait) if wait.process_id == ev.process_id => { wait.update_command_display(command_display); @@ -2236,7 +2251,12 @@ impl ChatWidget { self.retry_status_header = Some(self.current_status_header.clone()); } self.bottom_pane.ensure_status_indicator(); - self.set_status(message, additional_details); + self.set_status( + message, + additional_details, + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); } /// Handle completion of an `AgentMessage` turn item. @@ -6293,6 +6313,8 @@ impl ChatWidget { self.set_status( "Setting up sandbox...".to_string(), Some("Hang tight, this may take a few minutes".to_string()), + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, ); self.request_redraw(); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap new file mode 100644 index 0000000000..933bc70723 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__unified_exec_wait_status_renders_command_in_single_details_row.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: rendered +--- +• Waiting for background terminal (0s • esc to … + └ cargo test -p codex-core -- --exact… + + +› Ask Codex to do anything + + ? for shortcuts 100% context left diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 4599377e91..cc10484c35 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -4002,8 +4002,14 @@ async fn unified_exec_wait_status_header_updates_on_late_command_display() { assert!(chat.active_cell.is_none()); assert_eq!( chat.current_status_header, - "Waiting for background terminal · sleep 5" + "Waiting for background terminal" ); + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), "Waiting for background terminal"); + assert_eq!(status.details(), Some("sleep 5")); } #[tokio::test] @@ -4016,8 +4022,14 @@ async fn unified_exec_waiting_multiple_empty_snapshots() { terminal_interaction(&mut chat, "call-wait-1b", "proc-1", ""); assert_eq!( chat.current_status_header, - "Waiting for background terminal · just fix" + "Waiting for background terminal" ); + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), "Waiting for background terminal"); + assert_eq!(status.details(), Some("just fix")); chat.handle_codex_event(Event { id: "turn-wait-1".into(), @@ -4035,6 +4047,26 @@ async fn unified_exec_waiting_multiple_empty_snapshots() { assert_snapshot!("unified_exec_waiting_multiple_empty_after", combined); } +#[tokio::test] +async fn unified_exec_wait_status_renders_command_in_single_details_row_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.on_task_started(); + begin_unified_exec_startup( + &mut chat, + "call-wait-ui", + "proc-ui", + "cargo test -p codex-core -- --exact some::very::long::test::name", + ); + + terminal_interaction(&mut chat, "call-wait-ui-stdin", "proc-ui", ""); + + let rendered = render_bottom_popup(&chat, 48); + assert_snapshot!( + "unified_exec_wait_status_renders_command_in_single_details_row", + rendered + ); +} + #[tokio::test] async fn unified_exec_empty_then_non_empty_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; @@ -4062,8 +4094,14 @@ async fn unified_exec_non_empty_then_empty_snapshots() { terminal_interaction(&mut chat, "call-wait-3b", "proc-3", ""); assert_eq!( chat.current_status_header, - "Waiting for background terminal · just fix" + "Waiting for background terminal" ); + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), "Waiting for background terminal"); + assert_eq!(status.details(), Some("just fix")); let pre_cells = drain_insert_history(&mut rx); let active_combined = pre_cells .iter() diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 01027c3c99..cb4f9c5ab4 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -31,14 +31,21 @@ use crate::tui::FrameRequester; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_lines; -const DETAILS_MAX_LINES: usize = 3; +pub(crate) const STATUS_DETAILS_DEFAULT_MAX_LINES: usize = 3; const DETAILS_PREFIX: &str = " └ "; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum StatusDetailsCapitalization { + CapitalizeFirst, + Preserve, +} + /// Displays a single-line in-progress status with optional wrapped details. pub(crate) struct StatusIndicatorWidget { /// Animated header text (defaults to "Working"). header: String, details: Option, + details_max_lines: usize, /// Optional suffix rendered after the elapsed/interrupt segment. inline_message: Option, show_interrupt_hint: bool, @@ -77,6 +84,7 @@ impl StatusIndicatorWidget { Self { header: String::from("Working"), details: None, + details_max_lines: STATUS_DETAILS_DEFAULT_MAX_LINES, inline_message: None, show_interrupt_hint: true, elapsed_running: Duration::ZERO, @@ -99,10 +107,22 @@ impl StatusIndicatorWidget { } /// Update the details text shown below the header. - pub(crate) fn update_details(&mut self, details: Option) { + pub(crate) fn update_details( + &mut self, + details: Option, + capitalization: StatusDetailsCapitalization, + max_lines: usize, + ) { + self.details_max_lines = max_lines.max(1); self.details = details .filter(|details| !details.is_empty()) - .map(|details| capitalize_first(details.trim_start())); + .map(|details| { + let trimmed = details.trim_start(); + match capitalization { + StatusDetailsCapitalization::CapitalizeFirst => capitalize_first(trimmed), + StatusDetailsCapitalization::Preserve => trimmed.to_string(), + } + }); } /// Update the inline suffix text shown after `({elapsed} • esc to interrupt)`. @@ -193,8 +213,8 @@ impl StatusIndicatorWidget { let mut out = word_wrap_lines(details.lines().map(|line| vec![line.dim()]), opts); - if out.len() > DETAILS_MAX_LINES { - out.truncate(DETAILS_MAX_LINES); + if out.len() > self.details_max_lines { + out.truncate(self.details_max_lines); let content_width = usize::from(width).saturating_sub(prefix_width).max(1); let max_base_len = content_width.saturating_sub(1); if let Some(last) = out.last_mut() @@ -329,7 +349,11 @@ mod tests { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy(), false); - w.update_details(Some("A man a plan a canal panama".to_string())); + w.update_details( + Some("A man a plan a canal panama".to_string()), + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); w.set_interrupt_hint_visible(false); // Freeze time-dependent rendering (elapsed + spinner) to keep the snapshot stable. @@ -372,14 +396,45 @@ mod tests { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy(), true); - w.update_details(Some("abcd abcd abcd abcd".to_string())); + w.update_details( + Some("abcd abcd abcd abcd".to_string()), + StatusDetailsCapitalization::CapitalizeFirst, + STATUS_DETAILS_DEFAULT_MAX_LINES, + ); let lines = w.wrapped_details_lines(6); - assert_eq!(lines.len(), DETAILS_MAX_LINES); + assert_eq!(lines.len(), STATUS_DETAILS_DEFAULT_MAX_LINES); let last = lines.last().expect("expected last details line"); assert!( last.spans[1].content.as_ref().ends_with("…"), "expected ellipsis in last line: {last:?}" ); } + + #[test] + fn details_args_can_disable_capitalization_and_limit_lines() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut w = StatusIndicatorWidget::new(tx, crate::tui::FrameRequester::test_dummy(), true); + w.update_details( + Some("cargo test -p codex-core and then cargo test -p codex-tui".to_string()), + StatusDetailsCapitalization::Preserve, + 1, + ); + + assert_eq!( + w.details(), + Some("cargo test -p codex-core and then cargo test -p codex-tui") + ); + + let lines = w.wrapped_details_lines(24); + assert_eq!(lines.len(), 1); + let last = lines.last().expect("expected one details line"); + assert!( + last.spans + .last() + .is_some_and(|span| span.content.as_ref().contains('…')), + "expected one-line details to be ellipsized, got {last:?}" + ); + } }