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()) 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 diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 9739760884..4bd95c5406 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -959,7 +959,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\"]}}", "libloading_0.8.9": "{\"dependencies\":[{\"name\":\"cfg-if\",\"req\":\"^1\",\"target\":\"cfg(unix)\"},{\"kind\":\"dev\",\"name\":\"libc\",\"req\":\"^0.2\"},{\"kind\":\"dev\",\"name\":\"static_assertions\",\"req\":\"^1.1\"},{\"name\":\"windows-link\",\"req\":\"^0.2\",\"target\":\"cfg(windows)\"},{\"features\":[\"Win32_Foundation\"],\"kind\":\"dev\",\"name\":\"windows-sys\",\"req\":\"^0.61\",\"target\":\"cfg(windows)\"}],\"features\":{}}", "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\":[]}}", @@ -1296,7 +1296,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/Cargo.lock b/codex-rs/Cargo.lock index cfb6f7b8dc..550ff0f805 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]] @@ -504,7 +504,7 @@ checksum = "3109e49b1e4909e9db6515a30c633684d68cdeaa252f215214cb4fa1a5bfee2c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -516,7 +516,7 @@ checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -654,7 +654,7 @@ checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -694,7 +694,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -711,7 +711,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1087,7 +1087,7 @@ dependencies = [ "darling 0.20.11", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1283,7 +1283,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -1846,20 +1846,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", ] @@ -1906,7 +1905,7 @@ version = "0.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -2260,6 +2259,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" @@ -2544,6 +2564,7 @@ name = "codex-utils-string" version = "0.0.0" dependencies = [ "pretty_assertions", + "regex-lite", ] [[package]] @@ -3012,7 +3033,7 @@ checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3056,7 +3077,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3070,7 +3091,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3083,7 +3104,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3094,7 +3115,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core 0.20.11", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3105,7 +3126,7 @@ checksum = "d38308df82d1080de0afee5d069fa14b0326a88c14f15c5ccda35b4a6c414c81" dependencies = [ "darling_core 0.21.3", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3116,7 +3137,7 @@ checksum = "ac3984ec7bd6cfa798e62b4a642426a5be0e68f9401cfc2a01e3fa9ea2fcdb8d" dependencies = [ "darling_core 0.23.0", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3259,7 +3280,7 @@ checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3289,7 +3310,7 @@ dependencies = [ "convert_case 0.6.0", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "unicode-xid", ] @@ -3303,7 +3324,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn 2.0.114", + "syn 2.0.117", "unicode-xid", ] @@ -3416,7 +3437,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3469,7 +3490,7 @@ checksum = "83e195b4945e88836d826124af44fdcb262ec01ef94d44f14f4fb5103f19892a" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3538,7 +3559,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3559,7 +3580,7 @@ checksum = "67c78a4d8fdf9953a5c9d458f9efe940fd97a0cab0941c075a813ac594733827" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -3720,7 +3741,7 @@ checksum = "a0aca10fb742cb43f9e7bb8467c91aa9bcb8e3ffbc6a6f7389bb93ffc920577d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4034,7 +4055,7 @@ checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4592,7 +4613,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.11.1", - "syn 2.0.114", + "syn 2.0.117", "unic-langid", ] @@ -4606,7 +4627,7 @@ dependencies = [ "i18n-config", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -4946,7 +4967,7 @@ dependencies = [ "indoc", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -5088,7 +5109,7 @@ checksum = "e0c84ee7f197eca9a86c6fd6cb771e55eb991632f15f2bc3ca6ec838929e6e78" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -5230,9 +5251,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" @@ -6244,7 +6265,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6424,9 +6445,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", @@ -6579,7 +6600,7 @@ checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6819,7 +6840,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -6880,7 +6901,7 @@ dependencies = [ "itertools 0.14.0", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7216,7 +7237,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7529,7 +7550,7 @@ checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7688,7 +7709,7 @@ dependencies = [ "proc-macro2", "quote", "serde_json", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -7736,7 +7757,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn 2.0.114", + "syn 2.0.117", "walkdir", ] @@ -8031,7 +8052,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8043,7 +8064,7 @@ dependencies = [ "proc-macro2", "quote", "serde_derive_internals", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8317,7 +8338,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8328,7 +8349,7 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8377,7 +8398,7 @@ checksum = "175ee3e80ae9982737ca543e96133087cbd9a485eecc3bc4de9c1a37b47ea59c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8429,7 +8450,7 @@ dependencies = [ "darling 0.21.3", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8479,7 +8500,7 @@ checksum = "6f50427f258fb77356e4cd4aa0e87e2bd2c66dbcee41dc405282cae2bfc26c83" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8738,7 +8759,7 @@ dependencies = [ "quote", "sqlx-core", "sqlx-macros-core", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -8761,7 +8782,7 @@ dependencies = [ "sqlx-mysql", "sqlx-postgres", "sqlx-sqlite", - "syn 2.0.114", + "syn 2.0.117", "tokio", "url", ] @@ -8946,7 +8967,7 @@ dependencies = [ "dupe", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9068,7 +9089,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9080,7 +9101,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9121,9 +9142,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", @@ -9147,7 +9168,7 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9274,7 +9295,7 @@ dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9285,7 +9306,7 @@ checksum = "5c89e72a01ed4c579669add59014b9a524d609c0c88c6a585ce37485879f6ffb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "test-case-core", ] @@ -9308,7 +9329,7 @@ checksum = "be35209fd0781c5401458ab66e4f98accf63553e8fae7425503e92fdd319783b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9357,7 +9378,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9368,7 +9389,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9512,7 +9533,7 @@ checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9781,7 +9802,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9880,7 +9901,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" dependencies = [ "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -9950,7 +9971,7 @@ checksum = "ee6ff59666c9cbaec3533964505d39154dc4e0a56151fdea30a09ed0301f62e2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "termcolor", ] @@ -10360,7 +10381,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "wasm-bindgen-shared", ] @@ -10694,7 +10715,7 @@ checksum = "2bbd5b46c938e506ecbce286b6628a02171d56153ba733b6c741fc627ec9579b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10705,7 +10726,7 @@ checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10716,7 +10737,7 @@ checksum = "053c4c462dc91d3b1504c6fe5a726dd15e216ba718e84a0e46a88fbe5ded3515" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -10727,7 +10748,7 @@ checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11321,7 +11342,7 @@ checksum = "b659052874eb698efe5b9e8cf382204678a0086ebf46982b79d6ca3182927e5d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -11372,7 +11393,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "zvariant_utils", ] @@ -11404,7 +11425,7 @@ checksum = "1328722bbf2115db7e19d69ebcc15e795719e2d66b60827c6a69a117365e37a0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11424,7 +11445,7 @@ checksum = "d71e5d6e06ab090c67b5e44993ec16b72dcbaabc526db883a360057678b48502" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "synstructure", ] @@ -11445,7 +11466,7 @@ checksum = "85a5b4158499876c763cb03bc4e49185d3cccbabb15b33c627f7884f43db852e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11479,7 +11500,7 @@ checksum = "eadce39539ca5cb3985590102671f2567e659fca9666581ad3411d59207951f3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] [[package]] @@ -11610,7 +11631,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", "zvariant_utils", ] @@ -11622,5 +11643,5 @@ checksum = "c51bcff7cc3dbb5055396bcf774748c3dab426b4b8659046963523cee4808340" dependencies = [ "proc-macro2", "quote", - "syn 2.0.114", + "syn 2.0.117", ] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 076edd92b8..e9ed2c4e38 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" } @@ -189,7 +191,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" @@ -205,7 +207,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" 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/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; } } 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..483516fe6e 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(()) }) @@ -1110,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/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/config.schema.json b/codex-rs/core/config.schema.json index 53de190722..3a1b8f546e 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" }, @@ -1630,6 +1633,9 @@ "js_repl_tools_only": { "type": "boolean" }, + "memories": { + "type": "boolean" + }, "memory_tool": { "type": "boolean" }, diff --git a/codex-rs/core/src/agent/builtins/monitor.toml b/codex-rs/core/src/agent/builtins/awaiter.toml similarity index 53% rename from codex-rs/core/src/agent/builtins/monitor.toml rename to codex-rs/core/src/agent/builtins/awaiter.toml index 5e48a4936c..a34583c0c9 100644 --- a/codex-rs/core/src/agent/builtins/monitor.toml +++ b/codex-rs/core/src/agent/builtins/awaiter.toml @@ -1,36 +1,35 @@ 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. +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/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 f4bdb9d7af..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. @@ -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, } } diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index c6693a9214..4051b727b8 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, @@ -496,12 +497,12 @@ 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, - key: "memory_tool", + key: "memories", stage: Stage::UnderDevelopment, default_enabled: false, }, @@ -619,7 +620,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::CollaborationModes, key: "collaboration_modes", - stage: Stage::Stable, + stage: Stage::Removed, default_enabled: true, }, FeatureSpec { @@ -736,10 +737,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/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 { 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. 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/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index ae72c8b1ef..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; @@ -109,6 +110,7 @@ mod spawn { #[derive(Debug, Serialize)] struct SpawnAgentResult { agent_id: String, + nickname: Option, } pub async fn handle( @@ -183,6 +185,7 @@ mod spawn { .unwrap_or((None, None)), None => (None, None), }; + let nickname = new_agent_nickname.clone(); session .send_event( &turn, @@ -199,9 +202,13 @@ 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(), + nickname, }) .map_err(|err| { FunctionCallError::Fatal(format!("failed to serialize spawn_agent result: {err}")) @@ -406,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}")) @@ -1085,6 +1094,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 +1131,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 +1200,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 +1238,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/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/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/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/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 7bd1fc9788..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) { @@ -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 { @@ -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/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 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/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(()) } diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 4d9d79270d..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,24 +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] -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 82% rename from codex-rs/exec-server/src/posix/mcp.rs rename to codex-rs/exec-server/src/unix/mcp.rs index 5319a1f916..547d055c14 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/unix/mcp.rs @@ -9,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; @@ -18,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; @@ -28,10 +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::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"; @@ -44,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, @@ -64,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, @@ -85,6 +71,51 @@ pub struct ExecTool { sandbox_state: Arc>>, } +#[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, +} + +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 { + context: RequestContext, + preserve_program_paths: bool, +} + +impl EscalationPolicyFactory for McpEscalationPolicyFactory { + type Policy = McpEscalationPolicy; + + fn create_policy( + &self, + policy: Arc>, + stopwatch: codex_shell_escalation::Stopwatch, + ) -> Self::Policy { + McpEscalationPolicy::new( + policy, + self.context.clone(), + stopwatch, + self.preserve_program_paths, + ) + } +} + #[tool_router] impl ExecTool { pub fn new( @@ -115,8 +146,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,21 +157,20 @@ 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.into(), + &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), )?])) @@ -242,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/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" + } + ] + }, + } +} 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)); + } } 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(()) 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, } } 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, { diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e080b8cde0..a5cfbdb63f 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, @@ -699,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; @@ -731,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(()) } @@ -868,50 +866,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 +937,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 +999,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,12 +1034,55 @@ 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; 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(()); @@ -1294,6 +1382,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, @@ -1481,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? { @@ -2752,7 +2812,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 +2854,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 +3144,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 +3312,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 +3320,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 +3368,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 +3629,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 +3688,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/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/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 017a10c6a9..431ed103a7 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/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 32dcb1dddd..1852085aff 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -222,6 +222,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/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/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/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index d58d79f1a9..42965d9afb 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; @@ -554,10 +555,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(); } } @@ -987,6 +994,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; @@ -1280,6 +1289,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/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 6b55125c9f..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; @@ -28,6 +29,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 @@ -104,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, @@ -125,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, @@ -242,7 +153,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 +165,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 +204,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 +422,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 +473,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/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.rs b/codex-rs/tui/src/chatwidget.rs index 9e6306238e..adc05d34af 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; @@ -250,6 +251,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; @@ -927,15 +930,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. @@ -1093,6 +1108,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(); @@ -1950,15 +1986,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); @@ -2231,7 +2268,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. @@ -2828,9 +2870,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.sync_realtime_command_enabled(); widget @@ -3001,9 +3041,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.sync_realtime_command_enabled(); widget @@ -3163,9 +3201,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.sync_realtime_command_enabled(); widget @@ -3284,7 +3320,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(); @@ -6412,6 +6455,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(); } @@ -6467,22 +6512,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(); } @@ -6683,17 +6712,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/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/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 614aa8f8b2..6a59d3591b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -331,6 +331,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; @@ -1583,7 +1634,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(); @@ -1598,6 +1649,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, @@ -1606,7 +1658,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, @@ -3358,6 +3410,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; @@ -3859,8 +4010,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] @@ -3873,8 +4030,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(), @@ -3892,6 +4055,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; @@ -3919,8 +4102,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() @@ -4012,17 +4201,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); @@ -4387,26 +4569,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 { 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/lib.rs b/codex-rs/tui/src/lib.rs index ada4d01e11..b76392ce11 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/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: _, diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 07802a562f..30fca57944 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -69,7 +69,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", 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..cb4f9c5ab4 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; @@ -30,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, @@ -76,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, @@ -98,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)`. @@ -192,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() @@ -253,7 +274,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); @@ -325,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. @@ -368,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:?}" + ); + } } 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 { 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 = "///";