diff --git a/.codex/skills/codex-issue-digest/SKILL.md b/.codex/skills/codex-issue-digest/SKILL.md new file mode 100644 index 0000000000..b531748f8c --- /dev/null +++ b/.codex/skills/codex-issue-digest/SKILL.md @@ -0,0 +1,102 @@ +--- +name: codex-issue-digest +description: Run a GitHub issue digest for openai/codex by feature-area labels, all areas, and configurable time windows. Use when asked to summarize recent Codex bug reports or enhancement requests, especially for owner-specific labels such as tui, exec, app, or similar areas. +--- + +# Codex Issue Digest + +## Objective + +Produce a concise, insight-oriented digest of `openai/codex` issues for the requested feature-area labels over the previous 24 hours by default. Honor a different duration when the user asks for one, for example "past week" or "48 hours". + +Include only issues that currently have `bug` or `enhancement` plus at least one requested owner label. If the user asks for all areas or all labels, collect `bug`/`enhancement` issues across all labels. + +## Inputs + +- Feature-area labels, for example `tui exec` +- `all areas` / `all labels` to scan all current feature labels +- Optional repo override, default `openai/codex` +- Optional time window, default previous 24 hours; examples: `48h`, `7d`, `1w`, `past week` + +## Workflow + +1. Run the collector from a current Codex repo checkout: + +```bash +python3 .codex/skills/codex-issue-digest/scripts/collect_issue_digest.py --labels tui exec --window-hours 24 +``` + +Use `--window "past week"` or `--window-hours 168` when the user asks for a non-default duration. Use `--all-labels` when the user says all areas or all labels. + +2. Use the JSON as the source of truth. It includes new issues, new issue comments, new reactions/upvotes, current labels, current reaction counts, model-ready `summary_inputs`, and detailed `digest_rows`. +3. Start the report with `## Summary`, then `## Details`. +4. In `## Summary`, write skim-first headlines: + - Lead with the most important fact or judgment. Do not start with aggregate counts unless the aggregate itself is the story. + - Make the first 1-3 bullets answer "what should owners pay attention to right now?" + - Bold only the critical insight phrase in each high-priority bullet, for example `**GPT-5.5 context is the dominant pressure point**`. + - Keep summary bullets short enough to scan in about 20 seconds. + - Put broad stats near the end of the summary, after the owner-relevant takeaways. + - Say clearly when there is nothing significant to act on. + - Call out any areas or themes receiving lots of user attention. + - Cluster and name themes yourself from `summary_inputs`; the collector intentionally does not hard-code issue categories. + - Use a cluster only when the issues genuinely share the same product problem. If several issues merely share a broad platform or label, describe them individually. + - Do not omit a repeated theme just because its individual issues fall below the details table cutoff. Several similar reports should be called out as a repeated customer concern. + - For single-issue rows, summarize the concern directly instead of calling it a cluster. + - Use inline numbered issue links from each relevant row's `ref_markdown`. +5. In `## Details`, include a compact table only when useful: + - Prefer rows from `digest_rows`; include a `Refs` column using each row's `ref_markdown`. + - Keep the table short; omit low-signal rows when the summary already covers them. + - Use compact columns such as marker, area, type, description, interactions, and refs. + - The `Description` cell should be a short owner-readable phrase. Use row `description`, title, body excerpts, and recent comments, but do not mechanically copy the raw GitHub issue title when it contains incidental details. + - A clear quiet/no-concern sentence when there is no meaningful signal. +6. Use the JSON `attention_marker` exactly. It is empty for normal rows, `๐Ÿ”ฅ` for elevated rows, and `๐Ÿ”ฅ๐Ÿ”ฅ` for very high-attention rows. The actual cutoffs are in `attention_thresholds`. +7. Use inline numbered references where a row or bullet points to issues, for example `Compaction bugs [1](https://github.com/openai/codex/issues/123), [2](https://github.com/openai/codex/issues/456)`. Do not add a separate footnotes section. +8. Label `interactions` as `Interactions`; it counts posts/comments/reactions during the requested window, not unique people. +9. Mention the collector `script_version`, repo checkout `git_head`, and time window in the digest footer or final line. + +## Reaction Handling + +The collector uses GitHub reactions endpoints, which include `created_at`, to count reactions created during the digest window for hydrated issues. It reports both in-window reaction counts and current reaction totals. Treat current reaction totals as standing engagement, and treat `new_reactions` / `new_upvotes` as windowed activity. + +By default, the collector fetches issue comments with `since=` and caps the number of comment pages per issue. This keeps very long historical threads from dominating a digest run and focuses the report on recent posts. Use `--fetch-all-comments` only when exhaustive comment history is more important than runtime. + +GitHub issue search is still seeded by issue `updated_at`, so a purely reaction-only issue may be missed if reactions do not bump `updated_at`. Covering every reaction-only case would require either a persisted snapshot store or a broader scan of labeled issues. + +## Attention Markers + +The collector scales attention markers by the requested time window. The baseline is 10 human user interactions for `๐Ÿ”ฅ` and 20 for `๐Ÿ”ฅ๐Ÿ”ฅ` over 24 hours; longer or shorter windows scale those cutoffs linearly and round up. For example, a one-week report uses 70 and 140 interactions. Human user interactions are human-authored new issue posts, human-authored new comments, and human reactions created during the window, including upvotes. Bot posts and bot reactions are excluded. In prose, explain this as high user interaction rather than naming the emoji. + +## Freshness + +The automation should run from a repo checkout that contains this skill. For shared daily use, prefer one of these patterns: + +- Run the automation in a checkout that is refreshed before the automation starts, for example with `git pull --ff-only`. +- If the automation cannot safely mutate the checkout, have it report the current `git_head` from the collector output so readers know which skill/script version produced the digest. + +## Sample Owner Prompt + +```text +Use $codex-issue-digest to run the Codex issue digest for labels tui and exec over the previous 24 hours. +``` + +```text +Use $codex-issue-digest to run the Codex issue digest for all areas over the past week. +``` + +## Validation + +Dry run the collector against recent issues: + +```bash +python3 .codex/skills/codex-issue-digest/scripts/collect_issue_digest.py --labels tui exec --window-hours 24 +``` + +```bash +python3 .codex/skills/codex-issue-digest/scripts/collect_issue_digest.py --all-labels --window "past week" --limit-issues 10 +``` + +Run the focused script tests: + +```bash +pytest .codex/skills/codex-issue-digest/scripts/test_collect_issue_digest.py +``` diff --git a/.codex/skills/codex-issue-digest/agents/openai.yaml b/.codex/skills/codex-issue-digest/agents/openai.yaml new file mode 100644 index 0000000000..706ce5e11b --- /dev/null +++ b/.codex/skills/codex-issue-digest/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Codex Issue Digest" + short_description: "Summarize Codex issues by labels or all areas" + default_prompt: "Use $codex-issue-digest to run the Codex issue digest for labels tui and exec over the previous 24 hours." diff --git a/.codex/skills/codex-issue-digest/scripts/collect_issue_digest.py b/.codex/skills/codex-issue-digest/scripts/collect_issue_digest.py new file mode 100755 index 0000000000..e211af08f8 --- /dev/null +++ b/.codex/skills/codex-issue-digest/scripts/collect_issue_digest.py @@ -0,0 +1,988 @@ +#!/usr/bin/env python3 +"""Collect recent openai/codex issue activity for owner-focused digests.""" + +import argparse +import json +import math +import re +import subprocess +import sys +from datetime import datetime, timedelta, timezone +from pathlib import Path +from urllib.parse import quote + +SCRIPT_VERSION = 2 +QUALIFYING_KIND_LABELS = ("bug", "enhancement") +REACTION_KEYS = ("+1", "-1", "laugh", "hooray", "confused", "heart", "rocket", "eyes") +BASE_ATTENTION_WINDOW_HOURS = 24.0 +ONE_ATTENTION_INTERACTION_THRESHOLD = 10 +TWO_ATTENTION_INTERACTION_THRESHOLD = 20 +ALL_LABEL_PHRASES = {"all", "all areas", "all labels", "all-areas", "all-labels", "*"} + + +class GhCommandError(RuntimeError): + pass + + +def parse_args(): + parser = argparse.ArgumentParser( + description="Collect recent GitHub issue activity for a Codex owner digest." + ) + parser.add_argument( + "--repo", default="openai/codex", help="OWNER/REPO, default openai/codex" + ) + parser.add_argument( + "--labels", + nargs="+", + default=[], + help="Feature-area labels owned by the digest recipient, for example: tui exec", + ) + parser.add_argument( + "--all-labels", + action="store_true", + help="Collect bug/enhancement issues across all feature-area labels", + ) + parser.add_argument( + "--window", + help='Lookback duration such as "24h", "7d", "1w", or "past week"', + ) + parser.add_argument( + "--window-hours", type=float, default=24.0, help="Lookback window" + ) + parser.add_argument( + "--since", help="UTC ISO timestamp override for the window start" + ) + parser.add_argument("--until", help="UTC ISO timestamp override for the window end") + parser.add_argument( + "--limit-issues", + type=int, + default=200, + help="Maximum candidate issues to hydrate after search", + ) + parser.add_argument( + "--body-chars", type=int, default=1200, help="Issue body excerpt length" + ) + parser.add_argument( + "--comment-chars", type=int, default=900, help="Comment excerpt length" + ) + parser.add_argument( + "--max-comment-pages", + type=int, + default=3, + help=( + "Maximum pages of issue comments to hydrate per issue after applying the " + "window filter. Use 0 with --fetch-all-comments for no page cap." + ), + ) + parser.add_argument( + "--fetch-all-comments", + action="store_true", + help="Hydrate complete issue comment histories instead of only window-updated comments.", + ) + return parser.parse_args() + + +def parse_timestamp(value, arg_name): + if value is None: + return None + normalized = value.strip() + if not normalized: + return None + if normalized.endswith("Z"): + normalized = f"{normalized[:-1]}+00:00" + try: + parsed = datetime.fromisoformat(normalized) + except ValueError as err: + raise ValueError(f"{arg_name} must be an ISO timestamp") from err + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc) + + +def format_timestamp(value): + return ( + value.astimezone(timezone.utc) + .replace(microsecond=0) + .isoformat() + .replace("+00:00", "Z") + ) + + +def resolve_window(args): + until = parse_timestamp(args.until, "--until") or datetime.now(timezone.utc) + since = parse_timestamp(args.since, "--since") + if since is None: + hours = parse_duration_hours(getattr(args, "window", None)) + if hours is None: + hours = getattr(args, "window_hours", 24.0) + if hours <= 0: + raise ValueError("window duration must be > 0") + since = until - timedelta(hours=hours) + if since >= until: + raise ValueError("--since must be before --until") + return since, until + + +def parse_duration_hours(value): + if value is None: + return None + text = value.strip().casefold().replace("_", " ") + if not text: + return None + text = re.sub(r"^(past|last)\s+", "", text) + aliases = { + "day": 24.0, + "24h": 24.0, + "week": 168.0, + "7d": 168.0, + } + if text in aliases: + return aliases[text] + match = re.fullmatch(r"(\d+(?:\.\d+)?)\s*(h|hr|hrs|hour|hours)", text) + if match: + return float(match.group(1)) + match = re.fullmatch(r"(\d+(?:\.\d+)?)\s*(d|day|days)", text) + if match: + return float(match.group(1)) * 24.0 + match = re.fullmatch(r"(\d+(?:\.\d+)?)\s*(w|week|weeks)", text) + if match: + return float(match.group(1)) * 168.0 + raise ValueError(f"Unsupported duration: {value}") + + +def normalize_requested_labels(labels, all_labels=False): + out = [] + seen = set() + for raw in labels: + for piece in raw.split(","): + label = piece.strip() + if not label: + continue + key = label.casefold() + if key not in seen: + out.append(label) + seen.add(key) + phrase = " ".join(label.casefold() for label in out) + if all_labels or phrase in ALL_LABEL_PHRASES: + return [], True + if not out: + raise ValueError( + "At least one feature-area label is required, or use --all-labels" + ) + return out, False + + +def quote_label(label): + if re.fullmatch(r"[A-Za-z0-9_.:-]+", label): + return f"label:{label}" + escaped = label.replace('"', '\\"') + return f'label:"{escaped}"' + + +def build_search_queries( + repo, owner_labels, since, kind_labels=QUALIFYING_KIND_LABELS, all_labels=False +): + since_date = since.date().isoformat() + queries = [] + if all_labels: + for kind_label in kind_labels: + queries.append( + " ".join( + [ + f"repo:{repo}", + "is:issue", + f"updated:>={since_date}", + quote_label(kind_label), + ] + ) + ) + return queries + for owner_label in owner_labels: + for kind_label in kind_labels: + queries.append( + " ".join( + [ + f"repo:{repo}", + "is:issue", + f"updated:>={since_date}", + quote_label(owner_label), + quote_label(kind_label), + ] + ) + ) + return queries + + +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_json(args): + cmd = ["gh", *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 + raw = proc.stdout.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 gh_text(args): + cmd = ["gh", *args] + try: + proc = subprocess.run(cmd, check=True, capture_output=True, text=True) + except (FileNotFoundError, subprocess.CalledProcessError): + return "" + return proc.stdout.strip() + + +def git_head(): + try: + proc = subprocess.run( + ["git", "rev-parse", "--short=12", "HEAD"], + check=True, + capture_output=True, + text=True, + ) + except (FileNotFoundError, subprocess.CalledProcessError): + return None + return proc.stdout.strip() or None + + +def skill_relative_path(): + try: + return str(Path(__file__).resolve().relative_to(Path.cwd().resolve())) + except ValueError: + return str(Path(__file__).resolve()) + + +def gh_api_list_paginated(endpoint, per_page=100, max_pages=None, with_metadata=False): + items = [] + page = 1 + truncated = False + while True: + sep = "&" if "?" in endpoint else "?" + page_endpoint = f"{endpoint}{sep}per_page={per_page}&page={page}" + payload = gh_json(["api", page_endpoint]) + 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 + if max_pages is not None and page >= max_pages: + truncated = True + break + page += 1 + if with_metadata: + return { + "items": items, + "truncated": truncated, + "pages": page, + "max_pages": max_pages, + } + return items + + +def search_issue_numbers(queries, limit): + numbers = {} + for query in queries: + page = 1 + while True: + payload = gh_json( + [ + "api", + "search/issues", + "-X", + "GET", + "-f", + f"q={query}", + "-f", + "per_page=100", + "-f", + f"page={page}", + ] + ) + if not isinstance(payload, dict): + raise GhCommandError("Unexpected payload from GitHub issue search") + items = payload.get("items") or [] + if not isinstance(items, list): + raise GhCommandError("Expected search `items` to be a list") + for item in items: + if not isinstance(item, dict): + continue + number = item.get("number") + if isinstance(number, int): + numbers[number] = str(item.get("updated_at") or "") + if len(items) < 100 or len(numbers) >= limit: + break + page += 1 + ordered = sorted( + numbers, key=lambda number: (numbers[number], number), reverse=True + ) + return ordered[:limit] + + +def fetch_issue(repo, number): + payload = gh_json(["api", f"repos/{repo}/issues/{number}"]) + if not isinstance(payload, dict): + raise GhCommandError(f"Unexpected issue payload for #{number}") + return payload + + +def fetch_comments(repo, number, since=None, max_pages=None): + endpoint = f"repos/{repo}/issues/{number}/comments" + if since is not None: + endpoint = f"{endpoint}?since={quote(format_timestamp(since), safe='')}" + return gh_api_list_paginated( + endpoint, + max_pages=max_pages, + with_metadata=True, + ) + + +def fetch_reactions_for_item(endpoint, item): + if reaction_summary(item)["total"] <= 0: + return [] + return gh_api_list_paginated(endpoint) + + +def fetch_comment_reactions(repo, comments): + reactions_by_comment_id = {} + for comment in comments: + comment_id = comment.get("id") + if comment_id in (None, ""): + continue + endpoint = f"repos/{repo}/issues/comments/{comment_id}/reactions" + reactions_by_comment_id[comment_id] = fetch_reactions_for_item( + endpoint, comment + ) + return reactions_by_comment_id + + +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.lower().endswith("[bot]") + + +def is_human_user(user_obj): + login = extract_login(user_obj) + return bool(login) and not is_bot_login(login) + + +def label_names(issue): + labels = [] + for label in issue.get("labels") or []: + if isinstance(label, dict) and label.get("name"): + labels.append(str(label["name"])) + return sorted(labels, key=str.casefold) + + +def matching_labels(labels, requested): + labels_by_key = {label.casefold(): label for label in labels} + return [label for label in requested if label.casefold() in labels_by_key] + + +def area_labels(labels): + kind_keys = {label.casefold() for label in QUALIFYING_KIND_LABELS} + return [label for label in labels if label.casefold() not in kind_keys] + + +def attention_thresholds_for_window(window_hours): + if window_hours <= 0: + raise ValueError("window_hours must be > 0") + window_hours = round(window_hours, 6) + scale = window_hours / BASE_ATTENTION_WINDOW_HOURS + elevated = max(1, math.ceil(ONE_ATTENTION_INTERACTION_THRESHOLD * scale)) + very_high = max( + elevated + 1, math.ceil(TWO_ATTENTION_INTERACTION_THRESHOLD * scale) + ) + return { + "base_window_hours": BASE_ATTENTION_WINDOW_HOURS, + "window_hours": round(window_hours, 3), + "scale": round(scale, 3), + "elevated": elevated, + "very_high": very_high, + } + + +def attention_level_for(user_interactions, attention_thresholds=None): + thresholds = attention_thresholds or attention_thresholds_for_window( + BASE_ATTENTION_WINDOW_HOURS + ) + if user_interactions >= thresholds["very_high"]: + return 2 + if user_interactions >= thresholds["elevated"]: + return 1 + return 0 + + +def attention_marker_for(user_interactions, attention_thresholds=None): + return "๐Ÿ”ฅ" * attention_level_for(user_interactions, attention_thresholds) + + +def reaction_summary(item): + reactions = item.get("reactions") + if not isinstance(reactions, dict): + return {"total": 0, "counts": {}} + counts = {} + for key in REACTION_KEYS: + value = reactions.get(key, 0) + if isinstance(value, int) and value: + counts[key] = value + total = reactions.get("total_count") + if not isinstance(total, int): + total = sum(counts.values()) + return {"total": total, "counts": counts} + + +def reaction_event_summary(reactions, since, until): + counts = {} + total = 0 + for reaction in reactions or []: + if not isinstance(reaction, dict): + continue + if not is_in_window(str(reaction.get("created_at") or ""), since, until): + continue + if not is_human_user(reaction.get("user")): + continue + content = str(reaction.get("content") or "") + if not content: + continue + counts[content] = counts.get(content, 0) + 1 + total += 1 + return { + "total": total, + "counts": counts, + "upvotes": counts.get("+1", 0), + } + + +def compact_text(value, limit): + text = re.sub(r"\s+", " ", str(value or "")).strip() + if limit <= 0: + return "" + if len(text) <= limit: + return text + return f"{text[: max(limit - 1, 0)].rstrip()}..." + + +def clean_title_for_description(title): + cleaned = re.sub(r"\s+", " ", str(title or "")).strip() + cleaned = re.sub( + r"^(codex(?: desktop| app|\.app| cli)?|desktop|windows codex app)\s*[:,-]\s*", + "", + cleaned, + flags=re.IGNORECASE, + ) + cleaned = re.sub(r"^on windows,\s*", "Windows: ", cleaned, flags=re.IGNORECASE) + cleaned = cleaned.strip(" -:;") + return compact_text(cleaned, 80) or "Issue needs owner review" + + +def issue_description(issue): + return clean_title_for_description(issue.get("title")) + + +def is_in_window(timestamp, since, until): + parsed = parse_timestamp(timestamp, "timestamp") + if parsed is None: + return False + return since <= parsed < until + + +def summarize_comment( + comment, comment_chars, reaction_events=None, since=None, until=None +): + reactions = reaction_summary(comment) + new_reactions = ( + reaction_event_summary(reaction_events, since, until) + if since is not None and until is not None + else {"total": 0, "counts": {}, "upvotes": 0} + ) + human_user_interaction = is_human_user(comment.get("user")) + return { + "id": comment.get("id"), + "author": extract_login(comment.get("user")), + "author_association": str(comment.get("author_association") or ""), + "created_at": str(comment.get("created_at") or ""), + "updated_at": str(comment.get("updated_at") or ""), + "url": str(comment.get("html_url") or ""), + "human_user_interaction": human_user_interaction, + "reactions": reactions["counts"], + "reaction_total": reactions["total"], + "new_reactions": new_reactions["total"], + "new_upvotes": new_reactions["upvotes"], + "new_reaction_counts": new_reactions["counts"], + "body_excerpt": compact_text(comment.get("body"), comment_chars), + } + + +def summarize_issue( + issue, + comments, + requested_labels, + since, + until, + body_chars, + comment_chars, + issue_reaction_events=None, + comment_reactions_by_id=None, + all_labels=False, + comments_hydration=None, + attention_thresholds=None, +): + labels = label_names(issue) + labels_by_key = {label.casefold() for label in labels} + kind_labels = [ + label for label in QUALIFYING_KIND_LABELS if label.casefold() in labels_by_key + ] + if all_labels: + owner_labels = area_labels(labels) or ["unlabeled"] + else: + owner_labels = matching_labels(labels, requested_labels) + if not kind_labels or not owner_labels: + return None + + updated_at = str(issue.get("updated_at") or "") + if not is_in_window(updated_at, since, until): + return None + + new_issue = is_in_window(str(issue.get("created_at") or ""), since, until) + comment_reactions_by_id = comment_reactions_by_id or {} + new_comments = [ + summarize_comment( + comment, + comment_chars, + reaction_events=comment_reactions_by_id.get(comment.get("id")), + since=since, + until=until, + ) + for comment in comments + if is_in_window(str(comment.get("created_at") or ""), since, until) + ] + new_comments.sort(key=lambda item: (item["created_at"], str(item["id"]))) + + issue_reactions = reaction_summary(issue) + issue_reaction_events_summary = reaction_event_summary( + issue_reaction_events, since, until + ) + comment_reaction_events_summary = reaction_event_summary( + [ + reaction + for reactions in comment_reactions_by_id.values() + for reaction in reactions + ], + since, + until, + ) + new_reactions = ( + issue_reaction_events_summary["total"] + + comment_reaction_events_summary["total"] + ) + new_upvotes = ( + issue_reaction_events_summary["upvotes"] + + comment_reaction_events_summary["upvotes"] + ) + all_comment_reaction_total = sum( + reaction_summary(comment)["total"] for comment in comments + ) + new_comment_reaction_total = sum( + comment["reaction_total"] for comment in new_comments + ) + new_issue_user_interaction = new_issue and is_human_user(issue.get("user")) + new_comment_user_interactions = sum( + 1 for comment in new_comments if comment["human_user_interaction"] + ) + user_interactions = ( + int(new_issue_user_interaction) + new_comment_user_interactions + new_reactions + ) + attention_level = attention_level_for(user_interactions, attention_thresholds) + attention_marker = attention_marker_for(user_interactions, attention_thresholds) + updated_without_visible_new_post = ( + not new_issue and not new_comments and new_reactions == 0 + ) + + engagement_score = ( + len(new_comments) * 3 + + new_reactions + + issue_reactions["total"] + + new_comment_reaction_total + + min(int(issue.get("comments") or len(comments) or 0), 10) + ) + + return { + "number": issue.get("number"), + "title": str(issue.get("title") or ""), + "description": issue_description(issue), + "url": str(issue.get("html_url") or ""), + "state": str(issue.get("state") or ""), + "author": extract_login(issue.get("user")), + "author_association": str(issue.get("author_association") or ""), + "created_at": str(issue.get("created_at") or ""), + "updated_at": updated_at, + "labels": labels, + "kind_labels": kind_labels, + "owner_labels": owner_labels, + "comments_total": int(issue.get("comments") or len(comments) or 0), + "comments_hydration": comments_hydration + or { + "fetched": len(comments), + "since": None, + "truncated": False, + "max_pages": None, + }, + "issue_reactions": issue_reactions["counts"], + "issue_reaction_total": issue_reactions["total"], + "comment_reaction_total": all_comment_reaction_total, + "new_comment_reaction_total": new_comment_reaction_total, + "new_issue_reactions": issue_reaction_events_summary["total"], + "new_issue_upvotes": issue_reaction_events_summary["upvotes"], + "new_comment_reactions": comment_reaction_events_summary["total"], + "new_comment_upvotes": comment_reaction_events_summary["upvotes"], + "new_reactions": new_reactions, + "new_upvotes": new_upvotes, + "user_interactions": user_interactions, + "attention": attention_level > 0, + "attention_level": attention_level, + "attention_marker": attention_marker, + "engagement_score": engagement_score, + "activity": { + "new_issue": new_issue, + "new_comments": len(new_comments), + "new_human_comments": new_comment_user_interactions, + "new_reactions": new_reactions, + "new_upvotes": new_upvotes, + "updated_without_visible_new_post": updated_without_visible_new_post, + }, + "body_excerpt": compact_text(issue.get("body"), body_chars), + "new_comments": new_comments, + } + + +def count_by_label(issues, labels): + out = {} + for label in labels: + matching = [issue for issue in issues if label in issue["owner_labels"]] + out[label] = { + "issues": len(matching), + "new_issues": sum( + 1 for issue in matching if issue["activity"]["new_issue"] + ), + "new_comments": sum( + issue["activity"]["new_comments"] for issue in matching + ), + } + return out + + +def count_by_kind(issues): + out = {} + for kind in QUALIFYING_KIND_LABELS: + matching = [issue for issue in issues if kind in issue["kind_labels"]] + out[kind] = { + "issues": len(matching), + "new_issues": sum( + 1 for issue in matching if issue["activity"]["new_issue"] + ), + "new_comments": sum( + issue["activity"]["new_comments"] for issue in matching + ), + } + return out + + +def hot_items(issues, limit=8): + ranked = sorted( + issues, + key=lambda issue: ( + issue["attention"], + issue["attention_level"], + issue["user_interactions"], + issue["engagement_score"], + issue["activity"]["new_comments"], + issue["issue_reaction_total"] + issue["comment_reaction_total"], + issue["updated_at"], + ), + reverse=True, + ) + return [ + { + "number": issue["number"], + "title": issue["title"], + "url": issue["url"], + "owner_labels": issue["owner_labels"], + "kind_labels": issue["kind_labels"], + "attention": issue["attention"], + "attention_level": issue["attention_level"], + "attention_marker": issue["attention_marker"], + "user_interactions": issue["user_interactions"], + "new_reactions": issue["new_reactions"], + "new_upvotes": issue["new_upvotes"], + "engagement_score": issue["engagement_score"], + "new_comments": issue["activity"]["new_comments"], + "reaction_total": issue["issue_reaction_total"] + + issue["comment_reaction_total"], + } + for issue in ranked[:limit] + if issue["engagement_score"] > 0 + ] + + +def ranked_digest_issues(issues): + return sorted( + issues, + key=lambda issue: ( + issue["attention"], + issue["attention_level"], + issue["user_interactions"], + issue["engagement_score"], + issue["activity"]["new_comments"], + issue["updated_at"], + ), + reverse=True, + ) + + +def digest_rows(issues, limit=10, ref_map=None): + ranked = ranked_digest_issues(issues) + if ref_map is None: + ref_map = {issue["number"]: ref for ref, issue in enumerate(ranked, start=1)} + rows = [] + for issue in ranked[:limit]: + ref = ref_map[issue["number"]] + reaction_total = issue["issue_reaction_total"] + issue["comment_reaction_total"] + rows.append( + { + "ref": ref, + "ref_markdown": f"[{ref}]({issue['url']})", + "marker": issue["attention_marker"], + "attention_marker": issue["attention_marker"], + "number": issue["number"], + "description": issue["description"], + "title": issue["title"], + "url": issue["url"], + "area": ", ".join(issue["owner_labels"]), + "kind": ", ".join(issue["kind_labels"]), + "state": issue["state"], + "interactions": issue["user_interactions"], + "user_interactions": issue["user_interactions"], + "new_reactions": issue["new_reactions"], + "new_upvotes": issue["new_upvotes"], + "current_reactions": reaction_total, + } + ) + return rows + + +def issue_ref_markdown(issue, ref_map): + ref = ref_map[issue["number"]] + return f"[{ref}]({issue['url']})" + + +def summary_inputs(issues, limit=80, ref_map=None): + ranked = ranked_digest_issues(issues) + if ref_map is None: + ref_map = {issue["number"]: ref for ref, issue in enumerate(ranked, start=1)} + rows = [] + for issue in ranked[:limit]: + rows.append( + { + "ref": ref_map[issue["number"]], + "ref_markdown": issue_ref_markdown(issue, ref_map), + "number": issue["number"], + "title": issue["title"], + "description": issue["description"], + "url": issue["url"], + "labels": issue["labels"], + "owner_labels": issue["owner_labels"], + "kind_labels": issue["kind_labels"], + "state": issue.get("state", ""), + "attention_marker": issue.get("attention_marker", ""), + "interactions": issue["user_interactions"], + "new_comments": issue["activity"].get("new_comments", 0), + "new_reactions": issue.get("new_reactions", 0), + "new_upvotes": issue.get("new_upvotes", 0), + "current_reactions": issue.get("issue_reaction_total", 0) + + issue.get("comment_reaction_total", 0), + } + ) + return rows + + +def collect_digest(args): + since, until = resolve_window(args) + window_hours = (until - since).total_seconds() / 3600 + attention_thresholds = attention_thresholds_for_window(window_hours) + requested_labels, all_labels = normalize_requested_labels( + args.labels, all_labels=args.all_labels + ) + queries = build_search_queries( + args.repo, requested_labels, since, all_labels=all_labels + ) + numbers = search_issue_numbers(queries, args.limit_issues) + gh_version_output = gh_text(["--version"]) + + issues = [] + max_comment_pages = None if args.max_comment_pages <= 0 else args.max_comment_pages + for number in numbers: + issue = fetch_issue(args.repo, number) + comments_since = None if args.fetch_all_comments else since + comments_payload = fetch_comments( + args.repo, + number, + since=comments_since, + max_pages=max_comment_pages, + ) + comments = comments_payload["items"] + issue_reaction_events = fetch_reactions_for_item( + f"repos/{args.repo}/issues/{number}/reactions", issue + ) + comment_reactions_by_id = fetch_comment_reactions(args.repo, comments) + comments_hydration = { + "fetched": len(comments), + "total": int(issue.get("comments") or len(comments) or 0), + "since": format_timestamp(comments_since) if comments_since else None, + "truncated": comments_payload["truncated"], + "max_pages": comments_payload["max_pages"], + "fetch_all_comments": args.fetch_all_comments, + } + summary = summarize_issue( + issue, + comments, + requested_labels, + since, + until, + args.body_chars, + args.comment_chars, + issue_reaction_events=issue_reaction_events, + comment_reactions_by_id=comment_reactions_by_id, + all_labels=all_labels, + comments_hydration=comments_hydration, + attention_thresholds=attention_thresholds, + ) + if summary is not None: + issues.append(summary) + + issues.sort( + key=lambda issue: (issue["updated_at"], int(issue["number"] or 0)), reverse=True + ) + totals = { + "candidate_issues": len(numbers), + "included_issues": len(issues), + "new_issues": sum(1 for issue in issues if issue["activity"]["new_issue"]), + "issues_with_new_comments": sum( + 1 for issue in issues if issue["activity"]["new_comments"] > 0 + ), + "new_comments": sum(issue["activity"]["new_comments"] for issue in issues), + "comments_fetched": sum( + issue["comments_hydration"]["fetched"] for issue in issues + ), + "issues_with_truncated_comment_hydration": sum( + 1 for issue in issues if issue["comments_hydration"]["truncated"] + ), + "updated_without_visible_new_post": sum( + 1 + for issue in issues + if issue["activity"]["updated_without_visible_new_post"] + ), + "issue_reactions_current_total": sum( + issue["issue_reaction_total"] for issue in issues + ), + "comment_reactions_current_total": sum( + issue["comment_reaction_total"] for issue in issues + ), + "new_reactions": sum(issue["new_reactions"] for issue in issues), + "new_upvotes": sum(issue["new_upvotes"] for issue in issues), + "user_interactions": sum(issue["user_interactions"] for issue in issues), + } + ranked = ranked_digest_issues(issues) + ref_map = {issue["number"]: ref for ref, issue in enumerate(ranked, start=1)} + filter_label = "all" if all_labels else requested_labels + + return { + "generated_at": format_timestamp(datetime.now(timezone.utc)), + "source": { + "repo": args.repo, + "skill": "codex-issue-digest", + "collector": skill_relative_path(), + "script_version": SCRIPT_VERSION, + "git_head": git_head(), + "gh_version": gh_version_output.splitlines()[0] + if gh_version_output + else None, + }, + "window": { + "since": format_timestamp(since), + "until": format_timestamp(until), + "hours": round(window_hours, 3), + }, + "attention_thresholds": attention_thresholds, + "filters": { + "owner_labels": filter_label, + "all_labels": all_labels, + "kind_labels": list(QUALIFYING_KIND_LABELS), + }, + "collection_notes": [ + "Issues are selected when they currently have bug or enhancement plus at least one requested owner label and were updated during the window.", + "By default, issue comments are fetched with since=window_start and a max page cap to avoid long historical threads; use --fetch-all-comments when exhaustive comment history is needed.", + "New issue comments are filtered by comment creation time within the window from the fetched comment set.", + "Reaction events are counted by GitHub reaction created_at timestamps for hydrated issues and fetched comments.", + "Current reaction totals are standing engagement signals; new_reactions and new_upvotes are windowed activity.", + "The collector does not assign semantic clusters; use summary_inputs as model-ready evidence for report-time clustering.", + "Pure reaction-only issues may be missed if GitHub issue search does not surface them via updated_at.", + "Issues updated during the window without a new issue body or new comment are retained because label/status edits can still be useful owner signals.", + ], + "totals": totals, + "by_owner_label": count_by_label( + issues, + sorted( + {area for issue in issues for area in issue["owner_labels"]}, + key=str.casefold, + ) + if all_labels + else requested_labels, + ), + "by_kind_label": count_by_kind(issues), + "hot_items": hot_items(issues), + "summary_inputs": summary_inputs(issues, ref_map=ref_map), + "digest_rows": digest_rows(issues, ref_map=ref_map), + "issues": issues, + } + + +def main(): + args = parse_args() + try: + digest = collect_digest(args) + except (GhCommandError, RuntimeError, ValueError) as err: + sys.stderr.write(f"collect_issue_digest.py error: {err}\n") + return 1 + sys.stdout.write(json.dumps(digest, indent=2, sort_keys=True) + "\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.codex/skills/codex-issue-digest/scripts/test_collect_issue_digest.py b/.codex/skills/codex-issue-digest/scripts/test_collect_issue_digest.py new file mode 100644 index 0000000000..1c283ea2f6 --- /dev/null +++ b/.codex/skills/codex-issue-digest/scripts/test_collect_issue_digest.py @@ -0,0 +1,614 @@ +import importlib.util +from datetime import timezone +from pathlib import Path + + +MODULE_PATH = Path(__file__).with_name("collect_issue_digest.py") +MODULE_SPEC = importlib.util.spec_from_file_location( + "collect_issue_digest", MODULE_PATH +) +collect_issue_digest = importlib.util.module_from_spec(MODULE_SPEC) +assert MODULE_SPEC.loader is not None +MODULE_SPEC.loader.exec_module(collect_issue_digest) + + +def test_build_search_queries_uses_each_owner_and_kind_label(): + since = collect_issue_digest.parse_timestamp("2026-04-25T12:34:56Z", "--since") + + queries = collect_issue_digest.build_search_queries( + "openai/codex", ["tui", "exec"], since + ) + + assert queries == [ + "repo:openai/codex is:issue updated:>=2026-04-25 label:tui label:bug", + "repo:openai/codex is:issue updated:>=2026-04-25 label:tui label:enhancement", + "repo:openai/codex is:issue updated:>=2026-04-25 label:exec label:bug", + "repo:openai/codex is:issue updated:>=2026-04-25 label:exec label:enhancement", + ] + + +def test_build_search_queries_can_scan_all_labels(): + since = collect_issue_digest.parse_timestamp("2026-04-25T12:34:56Z", "--since") + + queries = collect_issue_digest.build_search_queries( + "openai/codex", [], since, all_labels=True + ) + + assert queries == [ + "repo:openai/codex is:issue updated:>=2026-04-25 label:bug", + "repo:openai/codex is:issue updated:>=2026-04-25 label:enhancement", + ] + + +def test_normalize_requested_labels_accepts_all_area_phrases(): + assert collect_issue_digest.normalize_requested_labels(["all", "areas"]) == ( + [], + True, + ) + assert collect_issue_digest.normalize_requested_labels(["all-labels"]) == ( + [], + True, + ) + + +def test_summarize_issue_keeps_new_comments_and_reaction_signals(): + since = collect_issue_digest.parse_timestamp("2026-04-25T00:00:00Z", "--since") + until = collect_issue_digest.parse_timestamp("2026-04-26T00:00:00Z", "--until") + issue = { + "number": 123, + "title": "TUI does not redraw", + "html_url": "https://github.com/openai/codex/issues/123", + "state": "open", + "created_at": "2026-04-24T20:00:00Z", + "updated_at": "2026-04-25T10:00:00Z", + "user": {"login": "alice"}, + "author_association": "NONE", + "comments": 2, + "body": "The terminal freezes after resize.", + "labels": [{"name": "bug"}, {"name": "tui"}], + "reactions": {"total_count": 3, "+1": 2, "rocket": 1}, + } + comments = [ + { + "id": 1, + "created_at": "2026-04-25T11:00:00Z", + "updated_at": "2026-04-25T11:00:00Z", + "html_url": "https://github.com/openai/codex/issues/123#issuecomment-1", + "user": {"login": "bob"}, + "author_association": "MEMBER", + "body": "I can reproduce this on main.", + "reactions": {"total_count": 4, "heart": 1, "+1": 3}, + }, + { + "id": 2, + "created_at": "2026-04-24T11:00:00Z", + "updated_at": "2026-04-24T11:00:00Z", + "html_url": "https://github.com/openai/codex/issues/123#issuecomment-2", + "user": {"login": "carol"}, + "author_association": "NONE", + "body": "Older comment.", + "reactions": {"total_count": 1, "eyes": 1}, + }, + ] + + summary = collect_issue_digest.summarize_issue( + issue, + comments, + ["tui", "exec"], + since, + until, + body_chars=200, + comment_chars=200, + ) + + assert summary == { + "number": 123, + "title": "TUI does not redraw", + "description": "TUI does not redraw", + "url": "https://github.com/openai/codex/issues/123", + "state": "open", + "author": "alice", + "author_association": "NONE", + "created_at": "2026-04-24T20:00:00Z", + "updated_at": "2026-04-25T10:00:00Z", + "labels": ["bug", "tui"], + "kind_labels": ["bug"], + "owner_labels": ["tui"], + "comments_total": 2, + "comments_hydration": { + "fetched": 2, + "since": None, + "truncated": False, + "max_pages": None, + }, + "issue_reactions": {"+1": 2, "rocket": 1}, + "issue_reaction_total": 3, + "comment_reaction_total": 5, + "new_comment_reaction_total": 4, + "new_issue_reactions": 0, + "new_issue_upvotes": 0, + "new_comment_reactions": 0, + "new_comment_upvotes": 0, + "new_reactions": 0, + "new_upvotes": 0, + "user_interactions": 1, + "attention": False, + "attention_level": 0, + "attention_marker": "", + "engagement_score": 12, + "activity": { + "new_issue": False, + "new_comments": 1, + "new_human_comments": 1, + "new_reactions": 0, + "new_upvotes": 0, + "updated_without_visible_new_post": False, + }, + "body_excerpt": "The terminal freezes after resize.", + "new_comments": [ + { + "id": 1, + "author": "bob", + "author_association": "MEMBER", + "created_at": "2026-04-25T11:00:00Z", + "updated_at": "2026-04-25T11:00:00Z", + "url": "https://github.com/openai/codex/issues/123#issuecomment-1", + "human_user_interaction": True, + "reactions": {"+1": 3, "heart": 1}, + "reaction_total": 4, + "new_reactions": 0, + "new_upvotes": 0, + "new_reaction_counts": {}, + "body_excerpt": "I can reproduce this on main.", + } + ], + } + + +def test_summarize_issue_filters_non_owner_or_non_kind_labels(): + since = collect_issue_digest.parse_timestamp("2026-04-25T00:00:00Z", "--since") + until = collect_issue_digest.parse_timestamp("2026-04-26T00:00:00Z", "--until") + base_issue = { + "number": 1, + "title": "Question", + "created_at": "2026-04-25T01:00:00Z", + "updated_at": "2026-04-25T01:00:00Z", + "labels": [{"name": "question"}, {"name": "tui"}], + } + + assert ( + collect_issue_digest.summarize_issue( + base_issue, + [], + ["tui"], + since, + until, + body_chars=100, + comment_chars=100, + ) + is None + ) + + issue_without_owner = dict(base_issue) + issue_without_owner["labels"] = [{"name": "bug"}, {"name": "app"}] + + assert ( + collect_issue_digest.summarize_issue( + issue_without_owner, + [], + ["tui"], + since, + until, + body_chars=100, + comment_chars=100, + ) + is None + ) + + +def test_resolve_window_defaults_to_previous_hours(): + class Args: + since = None + until = "2026-04-26T12:00:00Z" + window_hours = 24 + + since, until = collect_issue_digest.resolve_window(Args()) + + assert since.isoformat() == "2026-04-25T12:00:00+00:00" + assert until.tzinfo == timezone.utc + + +def test_parse_duration_hours_accepts_common_phrases(): + assert collect_issue_digest.parse_duration_hours("past week") == 168 + assert collect_issue_digest.parse_duration_hours("48h") == 48 + assert collect_issue_digest.parse_duration_hours("2 days") == 48 + assert collect_issue_digest.parse_duration_hours("1w") == 168 + + +def test_attention_thresholds_scale_by_window_length(): + one_day = collect_issue_digest.attention_thresholds_for_window(24) + assert one_day["elevated"] == 10 + assert one_day["very_high"] == 20 + + half_day = collect_issue_digest.attention_thresholds_for_window(12) + assert half_day["elevated"] == 5 + assert half_day["very_high"] == 10 + + week = collect_issue_digest.attention_thresholds_for_window(168) + assert week["elevated"] == 70 + assert week["very_high"] == 140 + assert collect_issue_digest.attention_marker_for(69, week) == "" + assert collect_issue_digest.attention_marker_for(107, week) == "๐Ÿ”ฅ" + assert collect_issue_digest.attention_marker_for(140, week) == "๐Ÿ”ฅ๐Ÿ”ฅ" + + +def test_fetch_comments_uses_since_filter_and_page_cap(monkeypatch): + calls = [] + + def fake_gh_json(args): + calls.append(args) + return [{"id": idx} for idx in range(100)] + + monkeypatch.setattr(collect_issue_digest, "gh_json", fake_gh_json) + since = collect_issue_digest.parse_timestamp("2026-04-25T00:00:00Z", "--since") + + payload = collect_issue_digest.fetch_comments( + "openai/codex", 123, since=since, max_pages=1 + ) + + assert len(payload["items"]) == 100 + assert payload["truncated"] is True + assert payload["max_pages"] == 1 + assert calls == [ + [ + "api", + "repos/openai/codex/issues/123/comments?since=2026-04-25T00%3A00%3A00Z&per_page=100&page=1", + ] + ] + + +def test_issue_description_prefers_title_over_body_noise(): + issue = { + "title": "Codex.app GUI: MCP child processes not reaped after task completion", + "body": "A later crash mention should not override the title-level symptom.", + "labels": [{"name": "app"}, {"name": "bug"}], + } + + description = collect_issue_digest.issue_description(issue) + assert "MCP child processes" in description + assert "crash" not in description.casefold() + + +def test_attention_markers_count_human_user_interactions(): + since = collect_issue_digest.parse_timestamp("2026-04-25T00:00:00Z", "--since") + until = collect_issue_digest.parse_timestamp("2026-04-26T00:00:00Z", "--until") + issue = { + "number": 456, + "title": "Agent context is exploding", + "html_url": "https://github.com/openai/codex/issues/456", + "state": "open", + "created_at": "2026-04-25T01:00:00Z", + "updated_at": "2026-04-25T12:00:00Z", + "user": {"login": "alice"}, + "labels": [{"name": "bug"}, {"name": "agent"}], + } + comments = [ + { + "id": idx, + "created_at": "2026-04-25T02:00:00Z", + "updated_at": "2026-04-25T02:00:00Z", + "user": {"login": f"user-{idx}"}, + "body": "same here", + } + for idx in range(9) + ] + comments.append( + { + "id": 99, + "created_at": "2026-04-25T02:00:00Z", + "updated_at": "2026-04-25T02:00:00Z", + "user": {"login": "github-actions[bot]"}, + "body": "duplicate bot note", + } + ) + + summary = collect_issue_digest.summarize_issue( + issue, + comments, + ["agent"], + since, + until, + body_chars=100, + comment_chars=100, + ) + + assert summary["user_interactions"] == 10 + assert summary["activity"]["new_human_comments"] == 9 + assert summary["attention"] is True + assert summary["attention_level"] == 1 + assert summary["attention_marker"] == "๐Ÿ”ฅ" + + issue["created_at"] = "2026-04-24T01:00:00Z" + comments.extend( + { + "id": idx, + "created_at": "2026-04-25T03:00:00Z", + "updated_at": "2026-04-25T03:00:00Z", + "user": {"login": f"extra-user-{idx}"}, + "body": "also seeing this", + } + for idx in range(11) + ) + + summary = collect_issue_digest.summarize_issue( + issue, + comments, + ["agent"], + since, + until, + body_chars=100, + comment_chars=100, + ) + + assert summary["user_interactions"] == 20 + assert summary["attention_level"] == 2 + assert summary["attention_marker"] == "๐Ÿ”ฅ๐Ÿ”ฅ" + + +def test_reactions_count_toward_attention_markers(): + since = collect_issue_digest.parse_timestamp("2026-04-25T00:00:00Z", "--since") + until = collect_issue_digest.parse_timestamp("2026-04-26T00:00:00Z", "--until") + issue = { + "number": 789, + "title": "Support 1M token context", + "html_url": "https://github.com/openai/codex/issues/789", + "state": "open", + "created_at": "2026-04-24T01:00:00Z", + "updated_at": "2026-04-25T12:00:00Z", + "user": {"login": "alice"}, + "labels": [{"name": "enhancement"}, {"name": "context"}], + "reactions": {"total_count": 20, "+1": 20}, + } + comments = [ + { + "id": 1, + "created_at": "2026-04-25T02:00:00Z", + "updated_at": "2026-04-25T02:00:00Z", + "user": {"login": "commenter"}, + "body": "please", + "reactions": {"total_count": 2, "+1": 2}, + } + ] + issue_reactions = [ + { + "content": "+1", + "created_at": "2026-04-25T03:00:00Z", + "user": {"login": f"reactor-{idx}"}, + } + for idx in range(18) + ] + comment_reactions_by_id = { + 1: [ + { + "content": "heart", + "created_at": "2026-04-25T04:00:00Z", + "user": {"login": "human-reactor"}, + }, + { + "content": "+1", + "created_at": "2026-04-25T04:00:00Z", + "user": {"login": "github-actions[bot]"}, + }, + ] + } + + summary = collect_issue_digest.summarize_issue( + issue, + comments, + ["context"], + since, + until, + body_chars=100, + comment_chars=100, + issue_reaction_events=issue_reactions, + comment_reactions_by_id=comment_reactions_by_id, + ) + + assert summary["new_reactions"] == 19 + assert summary["new_upvotes"] == 18 + assert summary["user_interactions"] == 20 + assert summary["attention_level"] == 2 + assert summary["attention_marker"] == "๐Ÿ”ฅ๐Ÿ”ฅ" + assert summary["new_comments"][0]["new_reactions"] == 1 + assert summary["new_comments"][0]["new_upvotes"] == 0 + + +def test_digest_rows_are_table_ready_with_concise_descriptions(): + rows = collect_issue_digest.digest_rows( + [ + { + "number": 1, + "title": "Quiet bug", + "description": "Quiet bug", + "url": "https://github.com/openai/codex/issues/1", + "owner_labels": ["context"], + "kind_labels": ["bug"], + "state": "open", + "attention": False, + "attention_level": 0, + "attention_marker": "", + "user_interactions": 1, + "new_reactions": 0, + "new_upvotes": 0, + "engagement_score": 3, + "issue_reaction_total": 0, + "comment_reaction_total": 0, + "updated_at": "2026-04-25T01:00:00Z", + "activity": { + "new_issue": True, + "new_comments": 0, + "new_reactions": 0, + "updated_without_visible_new_post": False, + }, + }, + { + "number": 2, + "title": "Busy bug", + "description": "High-volume bug report", + "url": "https://github.com/openai/codex/issues/2", + "owner_labels": ["agent"], + "kind_labels": ["bug"], + "state": "open", + "attention": True, + "attention_level": 1, + "attention_marker": "๐Ÿ”ฅ", + "user_interactions": 17, + "new_reactions": 3, + "new_upvotes": 2, + "engagement_score": 20, + "issue_reaction_total": 5, + "comment_reaction_total": 2, + "updated_at": "2026-04-25T02:00:00Z", + "activity": { + "new_issue": False, + "new_comments": 16, + "new_reactions": 3, + "updated_without_visible_new_post": False, + }, + }, + ] + ) + + assert rows[0] == { + "ref": 1, + "ref_markdown": "[1](https://github.com/openai/codex/issues/2)", + "marker": "๐Ÿ”ฅ", + "attention_marker": "๐Ÿ”ฅ", + "number": 2, + "description": "High-volume bug report", + "title": "Busy bug", + "url": "https://github.com/openai/codex/issues/2", + "area": "agent", + "kind": "bug", + "state": "open", + "interactions": 17, + "user_interactions": 17, + "new_reactions": 3, + "new_upvotes": 2, + "current_reactions": 7, + } + + +def test_summary_inputs_are_model_ready_without_preclustering(): + issues = [ + { + "number": 20, + "title": "Windows app Browser Use external navigation fails", + "description": "Browser Use navigation or app-server failure", + "url": "https://github.com/openai/codex/issues/20", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "attention": False, + "attention_level": 0, + "attention_marker": "", + "user_interactions": 3, + "new_reactions": 1, + "engagement_score": 8, + "updated_at": "2026-04-25T04:00:00Z", + "activity": {"new_comments": 2}, + }, + { + "number": 21, + "title": "On Windows, cmake output waits until timeout", + "description": "Windows command timeout/capture problem", + "url": "https://github.com/openai/codex/issues/21", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "attention": False, + "attention_level": 0, + "attention_marker": "", + "user_interactions": 3, + "new_reactions": 0, + "engagement_score": 7, + "updated_at": "2026-04-25T03:00:00Z", + "activity": {"new_comments": 3}, + }, + { + "number": 22, + "title": "Windows computer use tool fails to click buttons", + "description": "Computer-use workflow failure", + "url": "https://github.com/openai/codex/issues/22", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "attention": False, + "attention_level": 0, + "attention_marker": "", + "user_interactions": 3, + "new_reactions": 0, + "engagement_score": 6, + "updated_at": "2026-04-25T02:00:00Z", + "activity": {"new_comments": 3}, + }, + ] + + rows = collect_issue_digest.summary_inputs(issues, ref_map={20: 1, 21: 2, 22: 3}) + + assert rows == [ + { + "ref": 1, + "ref_markdown": "[1](https://github.com/openai/codex/issues/20)", + "number": 20, + "title": "Windows app Browser Use external navigation fails", + "description": "Browser Use navigation or app-server failure", + "url": "https://github.com/openai/codex/issues/20", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "state": "", + "attention_marker": "", + "interactions": 3, + "new_comments": 2, + "new_reactions": 1, + "new_upvotes": 0, + "current_reactions": 0, + }, + { + "ref": 2, + "ref_markdown": "[2](https://github.com/openai/codex/issues/21)", + "number": 21, + "title": "On Windows, cmake output waits until timeout", + "description": "Windows command timeout/capture problem", + "url": "https://github.com/openai/codex/issues/21", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "state": "", + "attention_marker": "", + "interactions": 3, + "new_comments": 3, + "new_reactions": 0, + "new_upvotes": 0, + "current_reactions": 0, + }, + { + "ref": 3, + "ref_markdown": "[3](https://github.com/openai/codex/issues/22)", + "number": 22, + "title": "Windows computer use tool fails to click buttons", + "description": "Computer-use workflow failure", + "url": "https://github.com/openai/codex/issues/22", + "labels": ["app", "bug"], + "owner_labels": ["app"], + "kind_labels": ["bug"], + "state": "", + "attention_marker": "", + "interactions": 3, + "new_comments": 3, + "new_reactions": 0, + "new_upvotes": 0, + "current_reactions": 0, + }, + ] diff --git a/.github/actions/setup-bazel-ci/action.yml b/.github/actions/setup-bazel-ci/action.yml index 881209fd81..6f9f99ff66 100644 --- a/.github/actions/setup-bazel-ci/action.yml +++ b/.github/actions/setup-bazel-ci/action.yml @@ -33,7 +33,9 @@ runs: run: Copy-Item (Get-Command dotslash).Source -Destination "$env:LOCALAPPDATA\Microsoft\WindowsApps\dotslash.exe" - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + # setup-bazelisk is archived; use the maintained successor and let it + # prefer the runner's pre-installed Bazelisk when available. + uses: bazel-contrib/setup-bazel@c5acdfb288317d0b5c0bbd7a396a3dc868bb0f86 # 0.19.0 - name: Configure Bazel repository cache id: configure_bazel_repository_cache diff --git a/.github/workflows/rusty-v8-release.yml b/.github/workflows/rusty-v8-release.yml index 29e7b3b1ae..ee92eff4fa 100644 --- a/.github/workflows/rusty-v8-release.yml +++ b/.github/workflows/rusty-v8-release.yml @@ -78,7 +78,9 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + uses: ./.github/actions/setup-bazel-ci + with: + target: ${{ matrix.target }} - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 diff --git a/.github/workflows/v8-canary.yml b/.github/workflows/v8-canary.yml index f5aa1d7c67..9e699e52f8 100644 --- a/.github/workflows/v8-canary.yml +++ b/.github/workflows/v8-canary.yml @@ -75,7 +75,9 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - name: Set up Bazel - uses: bazelbuild/setup-bazelisk@b39c379c82683a5f25d34f0d062761f62693e0b2 # v3 + uses: ./.github/actions/setup-bazel-ci + with: + target: ${{ matrix.target }} - name: Set up Python uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fbab962cbc..464b7d72a2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2370,6 +2370,8 @@ dependencies = [ "codex-hooks", "codex-login", "codex-mcp", + "codex-memories-read", + "codex-memories-write", "codex-model-provider", "codex-model-provider-info", "codex-models-manager", @@ -2921,6 +2923,40 @@ dependencies = [ "wiremock", ] +[[package]] +name = "codex-memories-read" +version = "0.0.0" +dependencies = [ + "codex-protocol", + "codex-shell-command", + "codex-utils-absolute-path", + "codex-utils-output-truncation", + "codex-utils-template", + "pretty_assertions", + "tempfile", + "tokio", +] + +[[package]] +name = "codex-memories-write" +version = "0.0.0" +dependencies = [ + "anyhow", + "chrono", + "codex-git-utils", + "codex-models-manager", + "codex-protocol", + "codex-state", + "codex-utils-absolute-path", + "codex-utils-output-truncation", + "codex-utils-template", + "pretty_assertions", + "tempfile", + "tokio", + "tracing", + "uuid", +] + [[package]] name = "codex-model-provider" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 648c184ec8..85c947aea2 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -46,6 +46,8 @@ members = [ "login", "codex-mcp", "mcp-server", + "memories/read", + "memories/write", "model-provider-info", "models-manager", "network-proxy", @@ -153,6 +155,8 @@ codex-keyring-store = { path = "keyring-store" } codex-linux-sandbox = { path = "linux-sandbox" } codex-lmstudio = { path = "lmstudio" } codex-login = { path = "login" } +codex-memories-read = { path = "memories/read" } +codex-memories-write = { path = "memories/write" } codex-mcp = { path = "codex-mcp" } codex-mcp-server = { path = "mcp-server" } codex-model-provider-info = { path = "model-provider-info" } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 736b996676..e0c507a6eb 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -7,6 +7,7 @@ use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE; use crate::error_code::INTERNAL_ERROR_CODE; use crate::error_code::INVALID_PARAMS_ERROR_CODE; use crate::error_code::INVALID_REQUEST_ERROR_CODE; +use crate::error_code::invalid_params; use crate::fuzzy_file_search::FuzzyFileSearchSession; use crate::fuzzy_file_search::run_fuzzy_file_search; use crate::fuzzy_file_search::start_fuzzy_file_search_session; @@ -1353,7 +1354,7 @@ impl CodexMessageProcessor { self.config.cli_auth_credentials_store_mode, ) { Ok(()) => { - self.auth_manager.reload(); + self.auth_manager.reload().await; Ok(()) } Err(err) => Err(JSONRPCErrorError { @@ -1504,7 +1505,7 @@ impl CodexMessageProcessor { .await; if success { - auth_manager.reload(); + auth_manager.reload().await; config_manager.replace_cloud_requirements_loader( auth_manager.clone(), chatgpt_base_url, @@ -1612,7 +1613,7 @@ impl CodexMessageProcessor { .await; if success { - auth_manager.reload(); + auth_manager.reload().await; config_manager.replace_cloud_requirements_loader( auth_manager.clone(), chatgpt_base_url, @@ -1748,7 +1749,7 @@ impl CodexMessageProcessor { self.outgoing.send_error(request_id, error).await; return; } - self.auth_manager.reload(); + self.auth_manager.reload().await; self.config_manager.replace_cloud_requirements_loader( self.auth_manager.clone(), self.config.chatgpt_base_url.clone(), @@ -6323,6 +6324,15 @@ impl CodexMessageProcessor { self.outgoing.send_error(request_id, error).await; } + async fn send_internal_error(&self, request_id: ConnectionRequestId, message: String) { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message, + data: None, + }; + self.outgoing.send_error(request_id, error).await; + } + fn input_too_large_error(actual_chars: usize) -> JSONRPCErrorError { JSONRPCErrorError { code: INVALID_PARAMS_ERROR_CODE, @@ -6345,41 +6355,6 @@ impl CodexMessageProcessor { Ok(()) } - async fn send_internal_error(&self, request_id: ConnectionRequestId, message: String) { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message, - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - - async fn send_marketplace_error( - &self, - request_id: ConnectionRequestId, - err: MarketplaceError, - action: &str, - ) { - match err { - MarketplaceError::MarketplaceNotFound { .. } => { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - } - MarketplaceError::Io { .. } => { - self.send_internal_error(request_id, format!("failed to {action}: {err}")) - .await; - } - MarketplaceError::InvalidMarketplaceFile { .. } - | MarketplaceError::PluginNotFound { .. } - | MarketplaceError::PluginNotAvailable { .. } - | MarketplaceError::PluginsDisabled - | MarketplaceError::InvalidPlugin(_) => { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - } - } - } - async fn wait_for_thread_shutdown(thread: &Arc) -> ThreadShutdownResult { match tokio::time::timeout(Duration::from_secs(10), thread.shutdown_and_wait()).await { Ok(Ok(())) => ThreadShutdownResult::Complete, @@ -6458,34 +6433,33 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: ThreadUnsubscribeParams, ) { - let thread_id = match ThreadId::from_string(¶ms.thread_id) { - Ok(id) => id, - Err(err) => { - self.send_invalid_request_error(request_id, format!("invalid thread id: {err}")) - .await; - return; - } - }; + let result = self + .thread_unsubscribe_response(params, request_id.connection_id) + .await; + self.outgoing.send_result(request_id, result).await; + } + + async fn thread_unsubscribe_response( + &self, + params: ThreadUnsubscribeParams, + connection_id: ConnectionId, + ) -> Result { + let thread_id = ThreadId::from_string(¶ms.thread_id) + .map_err(|err| invalid_request(format!("invalid thread id: {err}")))?; if self.thread_manager.get_thread(thread_id).await.is_err() { // Reconcile stale app-server bookkeeping when the thread has already been // removed from the core manager. This keeps loaded-status/subscription state // consistent with the source of truth before reporting NotLoaded. self.finalize_thread_teardown(thread_id).await; - self.outgoing - .send_response( - request_id, - ThreadUnsubscribeResponse { - status: ThreadUnsubscribeStatus::NotLoaded, - }, - ) - .await; - return; + return Ok(ThreadUnsubscribeResponse { + status: ThreadUnsubscribeStatus::NotLoaded, + }); }; let was_subscribed = self .thread_state_manager - .unsubscribe_connection_from_thread(thread_id, request_id.connection_id) + .unsubscribe_connection_from_thread(thread_id, connection_id) .await; let status = if was_subscribed { @@ -6493,9 +6467,7 @@ impl CodexMessageProcessor { } else { ThreadUnsubscribeStatus::NotSubscribed }; - self.outgoing - .send_response(request_id, ThreadUnsubscribeResponse { status }) - .await; + Ok(ThreadUnsubscribeResponse { status }) } async fn prepare_thread_for_archive(&self, thread_id: ThreadId) { @@ -6589,6 +6561,16 @@ impl CodexMessageProcessor { config: Config, environment_manager: Arc, ) { + let result = Self::apps_list_response(&outgoing, params, config, environment_manager).await; + outgoing.send_result(request_id, result).await; + } + + async fn apps_list_response( + outgoing: &Arc, + params: AppsListParams, + config: Config, + environment_manager: Arc, + ) -> Result { let AppsListParams { cursor, limit, @@ -6598,15 +6580,7 @@ impl CodexMessageProcessor { let start = match cursor { Some(cursor) => match cursor.parse::() { Ok(idx) => idx, - Err(_) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("invalid cursor: {cursor}"), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; - } + Err(_) => return Err(invalid_request(format!("invalid cursor: {cursor}"))), }, None => 0, }; @@ -6660,7 +6634,7 @@ impl CodexMessageProcessor { accessible_loaded, all_loaded, ) { - apps_list_helpers::send_app_list_updated_notification(&outgoing, merged.clone()) + apps_list_helpers::send_app_list_updated_notification(outgoing, merged.clone()) .await; last_notified_apps = Some(merged); } @@ -6670,25 +6644,13 @@ impl CodexMessageProcessor { let result = match tokio::time::timeout_at(app_list_deadline, rx.recv()).await { Ok(Some(result)) => result, Ok(None) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: "failed to load app lists".to_string(), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error("failed to load app lists")); } Err(_) => { let timeout_seconds = APP_LIST_LOAD_TIMEOUT.as_secs(); - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!( - "timed out waiting for app lists after {timeout_seconds} seconds" - ), - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(format!( + "timed out waiting for app lists after {timeout_seconds} seconds" + ))); } }; @@ -6698,26 +6660,14 @@ impl CodexMessageProcessor { accessible_loaded = true; } AppListLoadResult::Accessible(Err(err)) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: err, - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(err)); } AppListLoadResult::Directory(Ok(connectors)) => { all_connectors = Some(connectors); all_loaded = true; } AppListLoadResult::Directory(Err(err)) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: err, - data: None, - }; - outgoing.send_error(request_id, error).await; - return; + return Err(internal_error(err)); } } @@ -6747,27 +6697,26 @@ impl CodexMessageProcessor { all_loaded, ) && last_notified_apps.as_ref() != Some(&merged) { - apps_list_helpers::send_app_list_updated_notification(&outgoing, merged.clone()) + apps_list_helpers::send_app_list_updated_notification(outgoing, merged.clone()) .await; last_notified_apps = Some(merged.clone()); } if accessible_loaded && all_loaded { - match apps_list_helpers::paginate_apps(merged.as_slice(), start, limit) { - Ok(response) => { - outgoing.send_response(request_id, response).await; - return; - } - Err(error) => { - outgoing.send_error(request_id, error).await; - return; - } - } + return apps_list_helpers::paginate_apps(merged.as_slice(), start, limit); } } } async fn skills_list(&self, request_id: ConnectionRequestId, params: SkillsListParams) { + let result = self.skills_list_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn skills_list_response( + &self, + params: SkillsListParams, + ) -> Result { let SkillsListParams { cwds, force_reload, @@ -6792,17 +6741,13 @@ impl CodexMessageProcessor { let mut valid_extra_roots = Vec::new(); for root in entry.extra_user_roots { - let Ok(root) = AbsolutePathBuf::from_absolute_path_checked(root.as_path()) else { - self.send_invalid_request_error( - request_id, - format!( + let root = + AbsolutePathBuf::from_absolute_path_checked(root.as_path()).map_err(|_| { + invalid_request(format!( "skills/list perCwdExtraUserRoots extraUserRoots paths must be absolute: {}", root.display() - ), - ) - .await; - return; - }; + )) + })?; valid_extra_roots.push(root); } extra_roots_by_cwd @@ -6811,13 +6756,7 @@ impl CodexMessageProcessor { .extend(valid_extra_roots); } - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(error) => { - self.outgoing.send_error(request_id, error).await; - return; - } - }; + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; let auth = self.auth_manager.auth().await; let workspace_codex_plugins_enabled = self .workspace_codex_plugins_enabled(&config, auth.as_ref()) @@ -6896,9 +6835,7 @@ impl CodexMessageProcessor { errors, }); } - self.outgoing - .send_response(request_id, SkillsListResponse { data }) - .await; + Ok(SkillsListResponse { data }) } async fn marketplace_remove( &self, @@ -6911,27 +6848,16 @@ impl CodexMessageProcessor { marketplace_name: params.marketplace_name, }, ) - .await; - - match result { - Ok(outcome) => { - self.outgoing - .send_response( - request_id, - MarketplaceRemoveResponse { - marketplace_name: outcome.marketplace_name, - installed_root: outcome.removed_installed_root, - }, - ) - .await; - } - Err(MarketplaceRemoveError::InvalidRequest(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(MarketplaceRemoveError::Internal(message)) => { - self.send_internal_error(request_id, message).await; - } - } + .await + .map(|outcome| MarketplaceRemoveResponse { + marketplace_name: outcome.marketplace_name, + installed_root: outcome.removed_installed_root, + }) + .map_err(|err| match err { + MarketplaceRemoveError::InvalidRequest(message) => invalid_request(message), + MarketplaceRemoveError::Internal(message) => internal_error(message), + }); + self.outgoing.send_result(request_id, result).await; } async fn marketplace_upgrade( @@ -6939,53 +6865,38 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: MarketplaceUpgradeParams, ) { - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let result = self.marketplace_upgrade_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn marketplace_upgrade_response( + &self, + params: MarketplaceUpgradeParams, + ) -> Result { + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; let plugins_manager = self.thread_manager.plugins_manager(); let MarketplaceUpgradeParams { marketplace_name } = params; - let result = tokio::task::spawn_blocking(move || { + let outcome = tokio::task::spawn_blocking(move || { plugins_manager .upgrade_configured_marketplaces_for_config(&config, marketplace_name.as_deref()) }) - .await; + .await + .map_err(|err| internal_error(format!("failed to upgrade marketplaces: {err}")))? + .map_err(invalid_request)?; - match result { - Ok(Ok(outcome)) => { - self.outgoing - .send_response( - request_id, - MarketplaceUpgradeResponse { - selected_marketplaces: outcome.selected_marketplaces, - upgraded_roots: outcome.upgraded_roots, - errors: outcome - .errors - .into_iter() - .map(|err| MarketplaceUpgradeErrorInfo { - marketplace_name: err.marketplace_name, - message: err.message, - }) - .collect(), - }, - ) - .await; - } - Ok(Err(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(err) => { - self.send_internal_error( - request_id, - format!("failed to upgrade marketplaces: {err}"), - ) - .await; - } - } + Ok(MarketplaceUpgradeResponse { + selected_marketplaces: outcome.selected_marketplaces, + upgraded_roots: outcome.upgraded_roots, + errors: outcome + .errors + .into_iter() + .map(|err| MarketplaceUpgradeErrorInfo { + marketplace_name: err.marketplace_name, + message: err.message, + }) + .collect(), + }) } async fn marketplace_add(&self, request_id: ConnectionRequestId, params: MarketplaceAddParams) { @@ -6997,28 +6908,17 @@ impl CodexMessageProcessor { sparse_paths: params.sparse_paths.unwrap_or_default(), }, ) - .await; - - match result { - Ok(outcome) => { - self.outgoing - .send_response( - request_id, - MarketplaceAddResponse { - marketplace_name: outcome.marketplace_name, - installed_root: outcome.installed_root, - already_added: outcome.already_added, - }, - ) - .await; - } - Err(MarketplaceAddError::InvalidRequest(message)) => { - self.send_invalid_request_error(request_id, message).await; - } - Err(MarketplaceAddError::Internal(message)) => { - self.send_internal_error(request_id, message).await; - } - } + .await + .map(|outcome| MarketplaceAddResponse { + marketplace_name: outcome.marketplace_name, + installed_root: outcome.installed_root, + already_added: outcome.already_added, + }) + .map_err(|err| match err { + MarketplaceAddError::InvalidRequest(message) => invalid_request(message), + MarketplaceAddError::Internal(message) => internal_error(message), + }); + self.outgoing.send_result(request_id, result).await; } async fn skills_config_write( @@ -7026,6 +6926,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: SkillsConfigWriteParams, ) { + let result = self.skills_config_write_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn skills_config_write_response( + &self, + params: SkillsConfigWriteParams, + ) -> Result { let SkillsConfigWriteParams { path, name, @@ -7040,43 +6948,24 @@ impl CodexMessageProcessor { ConfigEdit::SetSkillConfigByName { name, enabled } } _ => { - let error = JSONRPCErrorError { - code: INVALID_PARAMS_ERROR_CODE, - message: "skills/config/write requires exactly one of path or name".to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; + return Err(invalid_params( + "skills/config/write requires exactly one of path or name", + )); } }; let edits = vec![edit]; - let result = ConfigEditsBuilder::new(&self.config.codex_home) + ConfigEditsBuilder::new(&self.config.codex_home) .with_edits(edits) .apply() - .await; - - match result { - Ok(()) => { + .await + .map(|()| { self.thread_manager.plugins_manager().clear_cache(); self.thread_manager.skills_manager().clear_cache(); - self.outgoing - .send_response( - request_id, - SkillsConfigWriteResponse { - effective_enabled: enabled, - }, - ) - .await; - } - Err(err) => { - let error = JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: format!("failed to update skill settings: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - } - } + SkillsConfigWriteResponse { + effective_enabled: enabled, + } + }) + .map_err(|err| internal_error(format!("failed to update skill settings: {err}"))) } async fn turn_start( diff --git a/codex-rs/app-server/src/codex_message_processor/plugins.rs b/codex-rs/app-server/src/codex_message_processor/plugins.rs index 8f0f4dea9a..e1d0fffad3 100644 --- a/codex-rs/app-server/src/codex_message_processor/plugins.rs +++ b/codex-rs/app-server/src/codex_message_processor/plugins.rs @@ -1,4 +1,6 @@ use super::*; +use crate::error_code::internal_error; +use crate::error_code::invalid_request; use codex_app_server_protocol::PluginInstallPolicy; impl CodexMessageProcessor { @@ -7,46 +9,33 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginListParams, ) { + let result = self.plugin_list_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_list_response( + &self, + params: PluginListParams, + ) -> Result { let plugins_manager = self.thread_manager.plugins_manager(); let PluginListParams { cwds } = params; let roots = cwds.unwrap_or_default(); - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; + let empty_response = || PluginListResponse { + marketplaces: Vec::new(), + marketplace_load_errors: Vec::new(), + featured_plugin_ids: Vec::new(), }; if !config.features.enabled(Feature::Plugins) { - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: Vec::new(), - marketplace_load_errors: Vec::new(), - featured_plugin_ids: Vec::new(), - }, - ) - .await; - return; + return Ok(empty_response()); } let auth = self.auth_manager.auth().await; if !self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await { - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: Vec::new(), - marketplace_load_errors: Vec::new(), - featured_plugin_ids: Vec::new(), - }, - ) - .await; - return; + return Ok(empty_response()); } plugins_manager.maybe_start_non_curated_plugin_cache_refresh(&roots); @@ -100,18 +89,11 @@ impl CodexMessageProcessor { .await { Ok(Ok(outcome)) => outcome, - Ok(Err(err)) => { - self.send_marketplace_error(request_id, err, "list marketplace plugins") - .await; - return; - } + Ok(Err(err)) => return Err(Self::marketplace_error(err, "list marketplace plugins")), Err(err) => { - self.send_internal_error( - request_id, - format!("failed to list marketplace plugins: {err}"), - ) - .await; - return; + return Err(internal_error(format!( + "failed to list marketplace plugins: {err}" + ))); } }; @@ -174,16 +156,11 @@ impl CodexMessageProcessor { Vec::new() }; - self.outgoing - .send_response( - request_id, - PluginListResponse { - marketplaces: data, - marketplace_load_errors, - featured_plugin_ids, - }, - ) - .await; + Ok(PluginListResponse { + marketplaces: data, + marketplace_load_errors, + featured_plugin_ids, + }) } pub(super) async fn plugin_read( @@ -191,6 +168,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginReadParams, ) { + let result = self.plugin_read_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_read_response( + &self, + params: PluginReadParams, + ) -> Result { let plugins_manager = self.thread_manager.plugins_manager(); let PluginReadParams { marketplace_path, @@ -201,30 +186,16 @@ impl CodexMessageProcessor { (Some(marketplace_path), None) => Ok(marketplace_path), (None, Some(remote_marketplace_name)) => Err(remote_marketplace_name), (Some(_), Some(_)) | (None, None) => { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "plugin/read requires exactly one of marketplacePath or remoteMarketplaceName".to_string(), - data: None, - }, - ) - .await; - return; + return Err(invalid_request( + "plugin/read requires exactly one of marketplacePath or remoteMarketplaceName", + )); } }; let config_cwd = read_source.as_ref().ok().and_then(|marketplace_path| { marketplace_path.as_path().parent().map(Path::to_path_buf) }); - let config = match self.load_latest_config(config_cwd).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let config = self.load_latest_config(config_cwd).await?; let plugin = match read_source { Ok(marketplace_path) => { @@ -232,17 +203,10 @@ impl CodexMessageProcessor { plugin_name, marketplace_path, }; - let outcome = match plugins_manager + let outcome = plugins_manager .read_plugin_for_config(&config, &request) .await - { - Ok(outcome) => outcome, - Err(err) => { - self.send_marketplace_error(request_id, err, "read plugin details") - .await; - return; - } - }; + .map_err(|err| Self::marketplace_error(err, "read plugin details"))?; let environment_manager = self.thread_manager.environment_manager(); let app_summaries = plugin_app_helpers::load_plugin_app_summaries( &config, @@ -287,19 +251,9 @@ impl CodexMessageProcessor { if !config.features.enabled(Feature::Plugins) || !config.features.enabled(Feature::RemotePlugin) { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "remote plugin read is not enabled for marketplace {remote_marketplace_name}" - ), - data: None, - }, - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin read is not enabled for marketplace {remote_marketplace_name}" + ))); } let auth = self.auth_manager.auth().await; let remote_plugin_service_config = RemotePluginServiceConfig { @@ -310,36 +264,20 @@ impl CodexMessageProcessor { .chars() .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~') { - self.send_invalid_request_error( - request_id, - "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed" - .to_string(), - ) - .await; - return; + return Err(invalid_request( + "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed", + )); } - let remote_detail = match codex_core_plugins::remote::fetch_remote_plugin_detail( + let remote_detail = codex_core_plugins::remote::fetch_remote_plugin_detail( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - Ok(remote_detail) => remote_detail, - Err(err) => { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc( - err, - "read remote plugin details", - ), - ) - .await; - return; - } - }; + .map_err(|err| { + remote_plugin_catalog_error_to_jsonrpc(err, "read remote plugin details") + })?; let plugin_apps = remote_detail .app_ids .iter() @@ -357,9 +295,7 @@ impl CodexMessageProcessor { } }; - self.outgoing - .send_response(request_id, PluginReadResponse { plugin }) - .await; + Ok(PluginReadResponse { plugin }) } pub(super) async fn plugin_install( @@ -367,6 +303,14 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginInstallParams, ) { + let result = self.plugin_install_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_install_response( + &self, + params: PluginInstallParams, + ) -> Result { let PluginInstallParams { marketplace_path, remote_marketplace_name, @@ -375,44 +319,27 @@ impl CodexMessageProcessor { let marketplace_path = match (marketplace_path, remote_marketplace_name) { (Some(marketplace_path), None) => marketplace_path, (None, Some(remote_marketplace_name)) => { - self.remote_plugin_install(request_id, remote_marketplace_name, plugin_name) + return self + .remote_plugin_install_response(remote_marketplace_name, plugin_name) .await; - return; } (Some(_), Some(_)) | (None, None) => { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "plugin/install requires exactly one of marketplacePath or remoteMarketplaceName".to_string(), - data: None, - }, - ) - .await; - return; + return Err(invalid_request( + "plugin/install requires exactly one of marketplacePath or remoteMarketplaceName", + )); } }; let config_cwd = marketplace_path.as_path().parent().map(Path::to_path_buf); - let config = match self.load_latest_config(config_cwd.clone()).await { - Ok(config) => config, - Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; - } - }; + let config = self.load_latest_config(config_cwd.clone()).await?; let auth = self.auth_manager.auth().await; if !self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await { - self.send_invalid_request_error( - request_id, - "Codex plugins are disabled for this workspace".to_string(), - ) - .await; - return; + return Err(invalid_request( + "Codex plugins are disabled for this workspace", + )); } let plugins_manager = self.thread_manager.plugins_manager(); @@ -421,197 +348,103 @@ impl CodexMessageProcessor { marketplace_path, }; - let install_result = plugins_manager.install_plugin(request).await; - - match install_result { - Ok(result) => { - let config = match self.load_latest_config(config_cwd).await { - Ok(config) => config, - Err(err) => { - warn!( - "failed to reload config after plugin install, using current config: {err:?}" - ); - config - } - }; - - self.clear_plugin_related_caches(); - - let plugin_mcp_servers = - load_plugin_mcp_servers(result.installed_path.as_path()).await; - - if !plugin_mcp_servers.is_empty() { - if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { - warn!( - plugin = result.plugin_id.as_key(), - "failed to queue MCP refresh after plugin install: {err:?}" - ); - } - self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) - .await; - } - - let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; - let auth = self.auth_manager.auth().await; - let apps_needing_auth = self - .plugin_apps_needing_auth_for_install( - &config, - auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), - &result.plugin_id.as_key(), - &plugin_apps, - ) - .await; - - self.outgoing - .send_response( - request_id, - PluginInstallResponse { - auth_policy: result.auth_policy.into(), - apps_needing_auth, - }, - ) - .await; - } - Err(err) => { - if err.is_invalid_request() { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - return; - } - - match err { - CorePluginInstallError::Marketplace(err) => { - self.send_marketplace_error(request_id, err, "install plugin") - .await; - } - CorePluginInstallError::Config(err) => { - self.send_internal_error( - request_id, - format!("failed to persist installed plugin config: {err}"), - ) - .await; - } - CorePluginInstallError::Remote(err) => { - self.send_internal_error( - request_id, - format!("failed to enable remote plugin: {err}"), - ) - .await; - } - CorePluginInstallError::Join(err) => { - self.send_internal_error( - request_id, - format!("failed to install plugin: {err}"), - ) - .await; - } - CorePluginInstallError::Store(err) => { - self.send_internal_error( - request_id, - format!("failed to install plugin: {err}"), - ) - .await; - } - } - } - } - } - - async fn remote_plugin_install( - &self, - request_id: ConnectionRequestId, - remote_marketplace_name: String, - plugin_name: String, - ) { - let config = match self.load_latest_config(/*fallback_cwd*/ None).await { + let result = plugins_manager + .install_plugin(request) + .await + .map_err(Self::plugin_install_error)?; + let config = match self.load_latest_config(config_cwd).await { Ok(config) => config, Err(err) => { - self.outgoing.send_error(request_id, err).await; - return; + warn!( + "failed to reload config after plugin install, using current config: {err:?}" + ); + config } }; + + self.clear_plugin_related_caches(); + + let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await; + + if !plugin_mcp_servers.is_empty() { + if let Err(err) = self.queue_mcp_server_refresh_for_config(&config).await { + warn!( + plugin = result.plugin_id.as_key(), + "failed to queue MCP refresh after plugin install: {err:?}" + ); + } + self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) + .await; + } + + let plugin_apps = load_plugin_apps(result.installed_path.as_path()).await; + let auth = self.auth_manager.auth().await; + let apps_needing_auth = self + .plugin_apps_needing_auth_for_install( + &config, + auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), + &result.plugin_id.as_key(), + &plugin_apps, + ) + .await; + + Ok(PluginInstallResponse { + auth_policy: result.auth_policy.into(), + apps_needing_auth, + }) + } + + async fn remote_plugin_install_response( + &self, + remote_marketplace_name: String, + plugin_name: String, + ) -> Result { + let config = self.load_latest_config(/*fallback_cwd*/ None).await?; if !config.features.enabled(Feature::Plugins) || !config.features.enabled(Feature::RemotePlugin) { - self.outgoing - .send_error( - request_id, - JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "remote plugin install is not enabled for marketplace {remote_marketplace_name}" - ), - data: None, - }, - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin install is not enabled for marketplace {remote_marketplace_name}" + ))); } if plugin_name.is_empty() || !plugin_name .chars() .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~') { - self.send_invalid_request_error( - request_id, - "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed" - .to_string(), - ) - .await; - return; + return Err(invalid_request( + "invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed", + )); } let auth = self.auth_manager.auth().await; let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), }; - let remote_detail = match codex_core_plugins::remote::fetch_remote_plugin_detail( + let remote_detail = codex_core_plugins::remote::fetch_remote_plugin_detail( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - Ok(remote_detail) => remote_detail, - Err(err) => { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc( - err, - "read remote plugin details before install", - ), - ) - .await; - return; - } - }; + .map_err(|err| { + remote_plugin_catalog_error_to_jsonrpc(err, "read remote plugin details before install") + })?; if remote_detail.summary.install_policy == PluginInstallPolicy::NotAvailable { - self.send_invalid_request_error( - request_id, - format!("remote plugin {plugin_name} is not available for install"), - ) - .await; - return; + return Err(invalid_request(format!( + "remote plugin {plugin_name} is not available for install" + ))); } - if let Err(err) = codex_core_plugins::remote::install_remote_plugin( + codex_core_plugins::remote::install_remote_plugin( &remote_plugin_service_config, auth.as_ref(), &remote_marketplace_name, &plugin_name, ) .await - { - self.outgoing - .send_error( - request_id, - remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"), - ) - .await; - return; - } + .map_err(|err| remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"))?; self.clear_plugin_related_caches(); @@ -629,15 +462,10 @@ impl CodexMessageProcessor { ) .await; - self.outgoing - .send_response( - request_id, - PluginInstallResponse { - auth_policy: remote_detail.summary.auth_policy, - apps_needing_auth, - }, - ) - .await; + Ok(PluginInstallResponse { + auth_policy: remote_detail.summary.auth_policy, + apps_needing_auth, + }) } async fn plugin_apps_needing_auth_for_install( @@ -709,59 +537,82 @@ impl CodexMessageProcessor { request_id: ConnectionRequestId, params: PluginUninstallParams, ) { + let result = self.plugin_uninstall_response(params).await; + self.outgoing.send_result(request_id, result).await; + } + + async fn plugin_uninstall_response( + &self, + params: PluginUninstallParams, + ) -> Result { let PluginUninstallParams { plugin_id } = params; let plugins_manager = self.thread_manager.plugins_manager(); - let uninstall_result = plugins_manager.uninstall_plugin(plugin_id).await; + plugins_manager + .uninstall_plugin(plugin_id) + .await + .map_err(Self::plugin_uninstall_error)?; + self.clear_plugin_related_caches(); + Ok(PluginUninstallResponse {}) + } - match uninstall_result { - Ok(()) => { - self.clear_plugin_related_caches(); - self.outgoing - .send_response(request_id, PluginUninstallResponse {}) - .await; - } - Err(err) => { - if err.is_invalid_request() { - self.send_invalid_request_error(request_id, err.to_string()) - .await; - return; - } + fn plugin_install_error(err: CorePluginInstallError) -> JSONRPCErrorError { + if err.is_invalid_request() { + return invalid_request(err.to_string()); + } - match err { - CorePluginUninstallError::Config(err) => { - self.send_internal_error( - request_id, - format!("failed to clear plugin config: {err}"), - ) - .await; - } - CorePluginUninstallError::Remote(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall remote plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::Join(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::Store(err) => { - self.send_internal_error( - request_id, - format!("failed to uninstall plugin: {err}"), - ) - .await; - } - CorePluginUninstallError::InvalidPluginId(_) => { - unreachable!("invalid plugin ids are handled above"); - } - } + match err { + CorePluginInstallError::Marketplace(err) => { + Self::marketplace_error(err, "install plugin") } + CorePluginInstallError::Config(err) => { + internal_error(format!("failed to persist installed plugin config: {err}")) + } + CorePluginInstallError::Remote(err) => { + internal_error(format!("failed to enable remote plugin: {err}")) + } + CorePluginInstallError::Join(err) => { + internal_error(format!("failed to install plugin: {err}")) + } + CorePluginInstallError::Store(err) => { + internal_error(format!("failed to install plugin: {err}")) + } + } + } + + fn plugin_uninstall_error(err: CorePluginUninstallError) -> JSONRPCErrorError { + if err.is_invalid_request() { + return invalid_request(err.to_string()); + } + + match err { + CorePluginUninstallError::Config(err) => { + internal_error(format!("failed to clear plugin config: {err}")) + } + CorePluginUninstallError::Remote(err) => { + internal_error(format!("failed to uninstall remote plugin: {err}")) + } + CorePluginUninstallError::Join(err) => { + internal_error(format!("failed to uninstall plugin: {err}")) + } + CorePluginUninstallError::Store(err) => { + internal_error(format!("failed to uninstall plugin: {err}")) + } + CorePluginUninstallError::InvalidPluginId(_) => { + unreachable!("invalid plugin ids are handled above"); + } + } + } + + fn marketplace_error(err: MarketplaceError, action: &str) -> JSONRPCErrorError { + match err { + MarketplaceError::MarketplaceNotFound { .. } + | MarketplaceError::InvalidMarketplaceFile { .. } + | MarketplaceError::PluginNotFound { .. } + | MarketplaceError::PluginNotAvailable { .. } + | MarketplaceError::PluginsDisabled + | MarketplaceError::InvalidPlugin(_) => invalid_request(err.to_string()), + MarketplaceError::Io { .. } => internal_error(format!("failed to {action}: {err}")), } } } diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index cc4e22e923..73332394f4 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -391,7 +391,8 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { let processor_outgoing = Arc::clone(&outgoing_message_sender); let auth_manager = - AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env); + AuthManager::shared_from_config(args.config.as_ref(), args.enable_codex_api_key_env) + .await; let config_manager = ConfigManager::new( args.config.codex_home.to_path_buf(), args.cli_overrides, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 52b6de0e0c..de89608101 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -469,7 +469,7 @@ pub async fn run_main_with_transport_options( config_manager .replace_thread_config_loader(Arc::clone(&discovered_thread_config_loader)); let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; config_manager.replace_cloud_requirements_loader(auth_manager, config.chatgpt_base_url); } Err(err) => { @@ -631,7 +631,7 @@ pub async fn run_main_with_transport_options( } let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; let remote_control_enabled = config.features.enabled(Feature::RemoteControl); if transport_accept_handles.is_empty() && !remote_control_enabled { @@ -712,7 +712,7 @@ pub async fn run_main_with_transport_options( let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(outgoing_tx)); let outbound_control_tx = outbound_control_tx; let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await; let processor = Arc::new(MessageProcessor::new(MessageProcessorArgs { outgoing: outgoing_message_sender, arg0_paths, diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index 507d7865b9..850d05c04e 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -127,7 +127,7 @@ impl TracingHarness { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; let config = Arc::new(build_test_config(codex_home.path(), &server.uri()).await?); - let (processor, outgoing_rx) = build_test_processor(config); + let (processor, outgoing_rx) = build_test_processor(config).await; let tracing = init_test_tracing(); tracing.exporter.reset(); tracing::callsite::rebuild_interest_cache(); @@ -257,7 +257,7 @@ async fn build_test_config(codex_home: &Path, server_uri: &str) -> Result, ) -> ( Arc, @@ -266,7 +266,7 @@ fn build_test_processor( let (outgoing_tx, outgoing_rx) = mpsc::channel(16); let outgoing = Arc::new(OutgoingMessageSender::new(outgoing_tx)); let auth_manager = - AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false).await; let config_manager = ConfigManager::new( config.codex_home.to_path_buf(), Vec::new(), diff --git a/codex-rs/app-server/src/transport/remote_control/tests.rs b/codex-rs/app-server/src/transport/remote_control/tests.rs index 6b0051f8db..82e02ae5de 100644 --- a/codex-rs/app-server/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server/src/transport/remote_control/tests.rs @@ -497,7 +497,8 @@ async fn remote_control_start_allows_missing_auth_when_enabled() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let (transport_event_tx, _transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let shutdown_token = CancellationToken::new(); @@ -1085,7 +1086,8 @@ async fn remote_control_waits_for_account_id_before_enrolling() { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let expected_server_name = gethostname().to_string_lossy().trim().to_string(); let expected_enrollment = RemoteControlEnrollment { account_id: "account_id".to_string(), diff --git a/codex-rs/app-server/src/transport/remote_control/websocket.rs b/codex-rs/app-server/src/transport/remote_control/websocket.rs index 464832e34a..4cd078455a 100644 --- a/codex-rs/app-server/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server/src/transport/remote_control/websocket.rs @@ -706,7 +706,7 @@ pub(crate) async fn load_remote_control_auth( "remote control requires ChatGPT authentication", )); } - auth_manager.reload(); + auth_manager.reload().await; reloaded = true; continue; }; @@ -714,7 +714,7 @@ pub(crate) async fn load_remote_control_auth( break auth; } if auth.get_account_id().is_none() && !reloaded { - auth_manager.reload(); + auth_manager.reload().await; reloaded = true; continue; } @@ -1090,7 +1090,8 @@ mod tests { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let mut auth_recovery = auth_manager.unauthorized_recovery(); let mut enrollment = Some(RemoteControlEnrollment { account_id: "account_id".to_string(), @@ -1172,7 +1173,8 @@ mod tests { /*enable_codex_api_key_env*/ false, AuthCredentialsStoreMode::File, /*chatgpt_base_url*/ None, - ); + ) + .await; let mut auth_recovery = auth_manager.unauthorized_recovery(); let mut enrollment = None; save_auth( diff --git a/codex-rs/chatgpt/src/chatgpt_client.rs b/codex-rs/chatgpt/src/chatgpt_client.rs index 42aac41138..05d8186686 100644 --- a/codex-rs/chatgpt/src/chatgpt_client.rs +++ b/codex-rs/chatgpt/src/chatgpt_client.rs @@ -21,7 +21,7 @@ pub(crate) async fn chatgpt_get_request_with_timeout( ) -> anyhow::Result { let chatgpt_base_url = &config.chatgpt_base_url; let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager .auth() .await diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 62e8040940..9dba71ce3a 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -26,7 +26,7 @@ const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60); async fn apps_enabled(config: &Config) -> bool { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; config .features @@ -35,7 +35,7 @@ async fn apps_enabled(config: &Config) -> bool { async fn connector_auth(config: &Config) -> anyhow::Result { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager .auth() .await diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 4fa7272ae4..5c60c20cbf 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -362,7 +362,9 @@ pub async fn run_login_with_device_code_fallback_to_browser( pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides).await; - match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) { + match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) + .await + { Ok(Some(auth)) => match auth.auth_mode() { AuthMode::ApiKey => match auth.get_token() { Ok(api_key) => { diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9f465521da..852dff616d 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1384,7 +1384,7 @@ async fn run_debug_models_command( .map_err(anyhow::Error::msg)?; let config = Config::load_with_cli_overrides(cli_overrides).await?; let auth_manager = - AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true); + AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true).await; let models_manager = build_models_manager(&config, auth_manager, CollaborationModesConfig::default()); models_manager diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 86a12e7d17..d38fbb0846 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -727,7 +727,7 @@ pub fn cloud_requirements_loader( }) } -pub fn cloud_requirements_loader_for_storage( +pub async fn cloud_requirements_loader_for_storage( codex_home: PathBuf, enable_codex_api_key_env: bool, credentials_store_mode: AuthCredentialsStoreMode, @@ -738,7 +738,8 @@ pub fn cloud_requirements_loader_for_storage( enable_codex_api_key_env, credentials_store_mode, Some(chatgpt_base_url.clone()), - ); + ) + .await; cloud_requirements_loader(auth_manager, chatgpt_base_url, codex_home) } @@ -853,7 +854,7 @@ mod tests { Ok(()) } - fn auth_manager_with_api_key() -> Arc { + async fn auth_manager_with_api_key() -> Arc { let tmp = tempdir().expect("tempdir"); let auth_json = json!({ "OPENAI_API_KEY": "sk-test-key", @@ -861,15 +862,18 @@ mod tests { "last_refresh": null, }); write_auth_json(tmp.path(), auth_json).expect("write auth"); - Arc::new(AuthManager::new( - tmp.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )) + Arc::new( + AuthManager::new( + tmp.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ) } - fn auth_manager_with_plan_and_identity( + async fn auth_manager_with_plan_and_identity( plan_type: &str, chatgpt_user_id: Option<&str>, account_id: Option<&str>, @@ -886,12 +890,15 @@ mod tests { ), ) .expect("write auth"); - Arc::new(AuthManager::new( - tmp.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )) + Arc::new( + AuthManager::new( + tmp.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ) } fn chatgpt_auth_json( @@ -975,7 +982,7 @@ mod tests { manager: Arc, } - fn managed_auth_context( + async fn managed_auth_context( plan_type: &str, chatgpt_user_id: Option<&str>, account_id: Option<&str>, @@ -995,18 +1002,22 @@ mod tests { ) .expect("write auth"); ManagedAuthContext { - manager: Arc::new(AuthManager::new( - home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )), + manager: Arc::new( + AuthManager::new( + home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ), _home: home, } } - fn auth_manager_with_plan(plan_type: &str) -> Arc { + async fn auth_manager_with_plan(plan_type: &str) -> Arc { auth_manager_with_plan_and_identity(plan_type, Some("user-12345"), Some("account-12345")) + .await } fn parse_for_fetch(contents: Option<&str>) -> Option { @@ -1118,7 +1129,7 @@ mod tests { #[tokio::test] async fn fetch_cloud_requirements_skips_non_chatgpt_auth() { - let auth_manager = auth_manager_with_api_key(); + let auth_manager = auth_manager_with_api_key().await; let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( auth_manager, @@ -1134,7 +1145,7 @@ mod tests { async fn fetch_cloud_requirements_skips_non_business_or_enterprise_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("pro"), + auth_manager_with_plan("pro").await, Arc::new(StaticFetcher { contents: None }), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1147,7 +1158,7 @@ mod tests { async fn fetch_cloud_requirements_skips_team_like_usage_based_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("self_serve_business_usage_based"), + auth_manager_with_plan("self_serve_business_usage_based").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1161,7 +1172,7 @@ mod tests { async fn fetch_cloud_requirements_allows_business_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1193,7 +1204,7 @@ mod tests { async fn fetch_cloud_requirements_allows_business_like_usage_based_plan() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise_cbp_usage_based"), + auth_manager_with_plan("enterprise_cbp_usage_based").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1225,7 +1236,7 @@ mod tests { async fn fetch_cloud_requirements_allows_hc_plan_as_enterprise() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("hc"), + auth_manager_with_plan("hc").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1329,7 +1340,7 @@ enabled = false #[tokio::test(start_paused = true)] async fn fetch_cloud_requirements_times_out() { - let auth_manager = auth_manager_with_plan("enterprise"); + let auth_manager = auth_manager_with_plan("enterprise").await; let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( auth_manager, @@ -1356,7 +1367,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1405,12 +1416,15 @@ enabled = false ), ) .expect("write initial auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); write_auth_json( auth_home.path(), @@ -1479,12 +1493,15 @@ enabled = false ), ) .expect("write initial auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); write_auth_json( auth_home.path(), @@ -1559,7 +1576,8 @@ enabled = false Some("account-12345"), "stale-access-token", "test-refresh-token", - ); + ) + .await; write_auth_json( auth._home.path(), chatgpt_auth_json( @@ -1611,12 +1629,15 @@ enabled = false ), ) .expect("write auth"); - let auth_manager = Arc::new(AuthManager::new( - auth_home.path().to_path_buf(), - /*enable_codex_api_key_env*/ false, - AuthCredentialsStoreMode::File, - /*chatgpt_base_url*/ None, - )); + let auth_manager = Arc::new( + AuthManager::new( + auth_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await, + ); let fetcher = Arc::new(UnauthorizedFetcher { message: @@ -1653,7 +1674,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1678,7 +1699,7 @@ enabled = false ))])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher, codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1700,7 +1721,7 @@ enabled = false async fn fetch_cloud_requirements_uses_cache_when_valid() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1711,7 +1732,7 @@ enabled = false let fetcher = Arc::new(SequenceFetcher::new(vec![Err(request_error())])); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1747,7 +1768,8 @@ enabled = false "business", /*chatgpt_user_id*/ None, Some("account-12345"), - ), + ) + .await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1790,7 +1812,7 @@ enabled = false async fn fetch_cloud_requirements_does_not_use_cache_when_auth_identity_is_incomplete() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1807,7 +1829,8 @@ enabled = false "business", /*chatgpt_user_id*/ None, Some("account-12345"), - ), + ) + .await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1843,7 +1866,8 @@ enabled = false "business", Some("user-12345"), Some("account-12345"), - ), + ) + .await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1860,7 +1884,8 @@ enabled = false "business", Some("user-99999"), Some("account-12345"), - ), + ) + .await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1892,7 +1917,7 @@ enabled = false async fn fetch_cloud_requirements_ignores_tampered_cache() { let codex_home = tempdir().expect("tempdir"); let prime_service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -1917,7 +1942,7 @@ enabled = false "allowed_approval_policies = [\"never\"]".to_string(), ))])); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -1975,7 +2000,7 @@ enabled = false "allowed_approval_policies = [\"never\"]".to_string(), ))])); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2007,7 +2032,7 @@ enabled = false async fn fetch_cloud_requirements_writes_signed_cache() { let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, Arc::new(StaticFetcher { contents: Some("allowed_approval_policies = [\"never\"]".to_string()), }), @@ -2070,7 +2095,7 @@ enabled = false let fetcher = Arc::new(SequenceFetcher::new(vec![Ok(None), Err(request_error())])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2088,7 +2113,7 @@ enabled = false ])); let codex_home = tempdir().expect("tempdir"); let service = CloudRequirementsService::new( - auth_manager_with_plan("enterprise"), + auth_manager_with_plan("enterprise").await, fetcher.clone(), codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, @@ -2121,7 +2146,7 @@ enabled = false )), ])); let service = CloudRequirementsService::new( - auth_manager_with_plan("business"), + auth_manager_with_plan("business").await, fetcher, codex_home.path().to_path_buf(), CLOUD_REQUIREMENTS_TIMEOUT, diff --git a/codex-rs/cloud-tasks/src/util.rs b/codex-rs/cloud-tasks/src/util.rs index e433b892e5..9a5056aa66 100644 --- a/codex-rs/cloud-tasks/src/util.rs +++ b/codex-rs/cloud-tasks/src/util.rs @@ -44,12 +44,15 @@ pub fn normalize_base_url(input: &str) -> String { pub async fn load_auth_manager(chatgpt_base_url: Option) -> Option { // TODO: pass in cli overrides once cloud tasks properly support them. let config = Config::load_with_cli_overrides(Vec::new()).await.ok()?; - Some(AuthManager::new( - config.codex_home.to_path_buf(), - /*enable_codex_api_key_env*/ false, - config.cli_auth_credentials_store_mode, - chatgpt_base_url.or(Some(config.chatgpt_base_url)), - )) + Some( + AuthManager::new( + config.codex_home.to_path_buf(), + /*enable_codex_api_key_env*/ false, + config.cli_auth_credentials_store_mode, + chatgpt_base_url.or(Some(config.chatgpt_base_url)), + ) + .await, + ) } /// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`, diff --git a/codex-rs/config/src/key_aliases.rs b/codex-rs/config/src/key_aliases.rs index 8d417e269f..07cb44fa6d 100644 --- a/codex-rs/config/src/key_aliases.rs +++ b/codex-rs/config/src/key_aliases.rs @@ -8,18 +8,11 @@ struct ConfigKeyAlias { canonical_key: &'static str, } -const CONFIG_KEY_ALIASES: &[ConfigKeyAlias] = &[ - ConfigKeyAlias { - table_path: &["memories"], - legacy_key: "no_memories_if_mcp_or_web_search", - canonical_key: "disable_on_external_context", - }, - ConfigKeyAlias { - table_path: &["agents"], - legacy_key: "max_concurrent_threads_per_session", - canonical_key: "max_threads", - }, -]; +const CONFIG_KEY_ALIASES: &[ConfigKeyAlias] = &[ConfigKeyAlias { + table_path: &["memories"], + legacy_key: "no_memories_if_mcp_or_web_search", + canonical_key: "disable_on_external_context", +}]; pub(crate) fn normalize_key_aliases(path: &[String], table: &mut TomlMap) { for alias in CONFIG_KEY_ALIASES { diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index b8d3b146f6..c95b57b718 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -39,6 +39,8 @@ codex-exec-server = { workspace = true } codex-features = { workspace = true } codex-feedback = { workspace = true } codex-login = { workspace = true } +codex-memories-read = { workspace = true } +codex-memories-write = { workspace = true } codex-mcp = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 3fbbfaf6eb..5727a4bdcf 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1310,6 +1310,11 @@ "hide_spawn_agent_metadata": { "type": "boolean" }, + "max_concurrent_threads_per_session": { + "format": "uint", + "minimum": 1.0, + "type": "integer" + }, "usage_hint_enabled": { "type": "boolean" }, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 273949e19f..e25c83e58d 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -7348,6 +7348,7 @@ async fn multi_agent_v2_config_from_feature_table() -> std::io::Result<()> { codex_home.path().join(CONFIG_TOML_FILE), r#"[features.multi_agent_v2] enabled = true +max_concurrent_threads_per_session = 5 usage_hint_enabled = false usage_hint_text = "Custom delegation guidance." hide_spawn_agent_metadata = true @@ -7361,6 +7362,8 @@ hide_spawn_agent_metadata = true .await?; assert!(config.features.enabled(Feature::MultiAgentV2)); + assert_eq!(config.multi_agent_v2.max_concurrent_threads_per_session, 5); + assert_eq!(config.agent_max_threads, Some(4)); assert!(!config.multi_agent_v2.usage_hint_enabled); assert_eq!( config.multi_agent_v2.usage_hint_text.as_deref(), @@ -7379,11 +7382,13 @@ async fn profile_multi_agent_v2_config_overrides_base() -> std::io::Result<()> { r#"profile = "no_hint" [features.multi_agent_v2] +max_concurrent_threads_per_session = 4 usage_hint_enabled = true usage_hint_text = "base hint" hide_spawn_agent_metadata = true [profiles.no_hint.features.multi_agent_v2] +max_concurrent_threads_per_session = 6 usage_hint_enabled = false usage_hint_text = "profile hint" hide_spawn_agent_metadata = false @@ -7396,6 +7401,7 @@ hide_spawn_agent_metadata = false .build() .await?; + assert_eq!(config.multi_agent_v2.max_concurrent_threads_per_session, 6); assert!(!config.multi_agent_v2.usage_hint_enabled); assert_eq!( config.multi_agent_v2.usage_hint_text.as_deref(), @@ -7406,6 +7412,80 @@ hide_spawn_agent_metadata = false Ok(()) } +#[tokio::test] +async fn multi_agent_v2_default_session_thread_cap_counts_root() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert_eq!(config.multi_agent_v2.max_concurrent_threads_per_session, 4); + assert_eq!(config.agent_max_threads, Some(3)); + + Ok(()) +} + +#[tokio::test] +async fn multi_agent_v2_rejects_agents_max_threads() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true + +[agents] +max_threads = 3 +"#, + )?; + + let err = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await + .expect_err("agents.max_threads should conflict with multi_agent_v2"); + + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert_eq!( + err.to_string(), + "agents.max_threads cannot be set when multi_agent_v2 is enabled" + ); + + Ok(()) +} + +#[tokio::test] +async fn multi_agent_v2_session_thread_cap_one_disallows_subagents() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true +max_concurrent_threads_per_session = 1 +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert_eq!(config.multi_agent_v2.max_concurrent_threads_per_session, 1); + assert_eq!(config.agent_max_threads, Some(0)); + + Ok(()) +} + #[tokio::test] async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 9635034dcc..47f2e4dd9f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1,7 +1,6 @@ use crate::agents_md::AgentsMdManager; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; -use crate::memories::memory_root; use crate::path_utils::normalize_for_native_workdir; use crate::unified_exec::DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS; use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS; @@ -63,6 +62,7 @@ use codex_features::MultiAgentV2ConfigToml; use codex_git_utils::resolve_root_git_project_for_trust; use codex_login::AuthManagerConfig; use codex_mcp::McpConfig; +use codex_memories_write::memory_root; use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::OLLAMA_CHAT_PROVIDER_REMOVED_ERROR; @@ -131,6 +131,7 @@ pub use codex_git_utils::GhostSnapshotConfig; /// the context window. pub(crate) const AGENTS_MD_MAX_BYTES: usize = 32 * 1024; // 32 KiB pub(crate) const DEFAULT_AGENT_MAX_THREADS: Option = Some(6); +pub(crate) const DEFAULT_MULTI_AGENT_V2_MAX_CONCURRENT_THREADS_PER_SESSION: usize = 4; pub(crate) const DEFAULT_AGENT_MAX_DEPTH: i32 = 1; pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option = None; const LOCAL_DEV_BUILD_VERSION: &str = "0.0.0"; @@ -704,6 +705,7 @@ pub struct Config { #[derive(Debug, Clone, PartialEq, Eq)] pub struct MultiAgentV2Config { + pub max_concurrent_threads_per_session: usize, pub usage_hint_enabled: bool, pub usage_hint_text: Option, pub hide_spawn_agent_metadata: bool, @@ -712,6 +714,8 @@ pub struct MultiAgentV2Config { impl Default for MultiAgentV2Config { fn default() -> Self { Self { + max_concurrent_threads_per_session: + DEFAULT_MULTI_AGENT_V2_MAX_CONCURRENT_THREADS_PER_SESSION, usage_hint_enabled: true, usage_hint_text: None, hide_spawn_agent_metadata: false, @@ -1579,6 +1583,10 @@ fn resolve_multi_agent_v2_config( let profile = multi_agent_v2_toml_config(config_profile.features.as_ref()); let default = MultiAgentV2Config::default(); + let max_concurrent_threads_per_session = profile + .and_then(|config| config.max_concurrent_threads_per_session) + .or_else(|| base.and_then(|config| config.max_concurrent_threads_per_session)) + .unwrap_or(default.max_concurrent_threads_per_session); let usage_hint_enabled = profile .and_then(|config| config.usage_hint_enabled) .or_else(|| base.and_then(|config| config.usage_hint_enabled)) @@ -1594,6 +1602,7 @@ fn resolve_multi_agent_v2_config( .unwrap_or(default.hide_spawn_agent_metadata); MultiAgentV2Config { + max_concurrent_threads_per_session, usage_hint_enabled, usage_hint_text, hide_spawn_agent_metadata, @@ -2078,17 +2087,35 @@ impl Config { let history = cfg.history.unwrap_or_default(); - let agent_max_threads = cfg - .agents - .as_ref() - .and_then(|agents| agents.max_threads) - .or(DEFAULT_AGENT_MAX_THREADS); - if agent_max_threads == Some(0) { + if multi_agent_v2.max_concurrent_threads_per_session == 0 { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, - "agents.max_threads must be at least 1", + "features.multi_agent_v2.max_concurrent_threads_per_session must be at least 1", )); } + let agent_max_threads_from_config = cfg.agents.as_ref().and_then(|agents| agents.max_threads); + let agent_max_threads = if features.enabled(Feature::MultiAgentV2) { + if agent_max_threads_from_config.is_some() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "agents.max_threads cannot be set when multi_agent_v2 is enabled", + )); + } + Some( + multi_agent_v2 + .max_concurrent_threads_per_session + .saturating_sub(1), + ) + } else { + let agent_max_threads = agent_max_threads_from_config.or(DEFAULT_AGENT_MAX_THREADS); + if agent_max_threads == Some(0) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "agents.max_threads must be at least 1", + )); + } + agent_max_threads + }; let agent_max_depth = cfg .agents .as_ref() diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 456f3a7eac..42cd415217 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -144,7 +144,7 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools( config: &Config, ) -> Option> { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; if !config .features @@ -216,7 +216,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( environment_manager: &EnvironmentManager, ) -> anyhow::Result { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager.auth().await; if !config .features @@ -434,7 +434,7 @@ async fn list_directory_connectors_for_tool_suggest_with_auth( Some(auth) } else { let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false); + AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; loaded_auth = auth_manager.auth().await; loaded_auth.as_ref() }; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index c6f879209d..c5e0da9b8f 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -57,7 +57,7 @@ pub use codex_mcp::SandboxState; mod mcp_openai_file; mod mcp_tool_call; mod memories; -pub use memories::clear_memory_roots_contents; +pub use codex_memories_write::clear_memory_roots_contents; pub(crate) mod mention_syntax; pub(crate) mod message_history; pub(crate) mod utils; @@ -200,4 +200,5 @@ pub mod compact; pub(crate) mod memory_trace; pub use memory_trace::BuiltMemory; pub use memory_trace::build_memories_from_trace_files; +mod memory_usage; pub mod otel_init; diff --git a/codex-rs/core/src/memories/extensions.rs b/codex-rs/core/src/memories/extensions.rs deleted file mode 100644 index 458197609f..0000000000 --- a/codex-rs/core/src/memories/extensions.rs +++ /dev/null @@ -1,251 +0,0 @@ -use crate::memories::memory_extensions_root; -use chrono::DateTime; -use chrono::Duration; -use chrono::NaiveDateTime; -use chrono::Utc; -use std::path::Path; -use std::path::PathBuf; -use tracing::warn; - -const FILENAME_TS_FORMAT: &str = "%Y-%m-%dT%H-%M-%S"; -pub(super) const EXTENSION_RESOURCE_RETENTION_DAYS: i64 = 7; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct RemovedExtensionResource { - pub(super) extension: String, - pub(super) resource_path: String, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct PendingExtensionResourceRemoval { - pub(super) removed: RemovedExtensionResource, - path: PathBuf, -} - -pub(super) async fn find_old_extension_resources( - memory_root: &Path, -) -> Vec { - find_old_extension_resources_with_now(memory_root, Utc::now()).await -} - -async fn find_old_extension_resources_with_now( - memory_root: &Path, - now: DateTime, -) -> Vec { - let mut pending = Vec::new(); - let cutoff = now - Duration::days(EXTENSION_RESOURCE_RETENTION_DAYS); - let extensions_root = memory_extensions_root(memory_root); - let mut extensions = match tokio::fs::read_dir(&extensions_root).await { - Ok(extensions) => extensions, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return pending, - Err(err) => { - warn!( - "failed reading memory extensions root {}: {err}", - extensions_root.display() - ); - return pending; - } - }; - - while let Ok(Some(extension_entry)) = extensions.next_entry().await { - let extension_path = extension_entry.path(); - let Ok(file_type) = extension_entry.file_type().await else { - continue; - }; - if !file_type.is_dir() { - continue; - } - let Some(extension) = extension_path - .file_name() - .and_then(|name| name.to_str()) - .map(ToOwned::to_owned) - else { - continue; - }; - if !tokio::fs::try_exists(extension_path.join("instructions.md")) - .await - .unwrap_or(false) - { - continue; - } - - let resources_path = extension_path.join("resources"); - let mut resources = match tokio::fs::read_dir(&resources_path).await { - Ok(resources) => resources, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, - Err(err) => { - warn!( - "failed reading memory extension resources {}: {err}", - resources_path.display() - ); - continue; - } - }; - - while let Ok(Some(resource_entry)) = resources.next_entry().await { - let resource_file_path = resource_entry.path(); - let Ok(file_type) = resource_entry.file_type().await else { - continue; - }; - if !file_type.is_file() { - continue; - } - let Some(file_name) = resource_file_path - .file_name() - .and_then(|name| name.to_str()) - else { - continue; - }; - if !file_name.ends_with(".md") { - continue; - } - let Some(resource_timestamp) = resource_timestamp(file_name) else { - continue; - }; - if resource_timestamp > cutoff { - continue; - } - - pending.push(PendingExtensionResourceRemoval { - removed: RemovedExtensionResource { - extension: extension.clone(), - resource_path: format!("resources/{file_name}"), - }, - path: resource_file_path, - }); - } - } - - pending.sort_by(|left, right| { - left.removed - .extension - .cmp(&right.removed.extension) - .then_with(|| left.removed.resource_path.cmp(&right.removed.resource_path)) - }); - pending -} - -pub(super) async fn remove_extension_resources(resources: &[PendingExtensionResourceRemoval]) { - for resource in resources { - if let Err(err) = tokio::fs::remove_file(&resource.path).await - && err.kind() != std::io::ErrorKind::NotFound - { - warn!( - "failed pruning old memory extension resource {}: {err}", - resource.path.display() - ); - } - } -} - -fn resource_timestamp(file_name: &str) -> Option> { - let timestamp = file_name.get(..19)?; - let naive = NaiveDateTime::parse_from_str(timestamp, FILENAME_TS_FORMAT).ok()?; - Some(DateTime::from_naive_utc_and_offset(naive, Utc)) -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - use tempfile::TempDir; - - #[tokio::test] - async fn finds_only_old_resources_from_extensions_with_instructions() { - let codex_home = TempDir::new().expect("create temp codex home"); - let memory_root = codex_home.path().join("memories"); - let extensions_root = memory_extensions_root(&memory_root); - let chronicle_resources = extensions_root.join("chronicle/resources"); - tokio::fs::create_dir_all(&chronicle_resources) - .await - .expect("create chronicle resources"); - tokio::fs::write( - extensions_root.join("chronicle/instructions.md"), - "instructions", - ) - .await - .expect("write chronicle instructions"); - - let now = DateTime::from_naive_utc_and_offset( - NaiveDateTime::parse_from_str("2026-04-14T12-00-00", FILENAME_TS_FORMAT) - .expect("parse now"), - Utc, - ); - let old_file = chronicle_resources.join("2026-04-06T11-59-59-abcd-10min-old.md"); - let exact_cutoff_file = - chronicle_resources.join("2026-04-07T12-00-00-abcd-10min-cutoff.md"); - let recent_file = chronicle_resources.join("2026-04-08T12-00-00-abcd-10min-recent.md"); - let invalid_file = chronicle_resources.join("not-a-timestamp.md"); - for file in [&old_file, &exact_cutoff_file, &recent_file, &invalid_file] { - tokio::fs::write(file, "resource") - .await - .expect("write chronicle resource"); - } - - let ignored_resources = extensions_root.join("ignored/resources"); - tokio::fs::create_dir_all(&ignored_resources) - .await - .expect("create ignored resources"); - let ignored_old_file = ignored_resources.join("2026-04-06T11-59-59-abcd-10min-old.md"); - tokio::fs::write(&ignored_old_file, "ignored") - .await - .expect("write ignored resource"); - - let pending = find_old_extension_resources_with_now(&memory_root, now).await; - - assert_eq!( - pending - .iter() - .map(|resource| resource.removed.clone()) - .collect::>(), - vec![ - RemovedExtensionResource { - extension: "chronicle".to_string(), - resource_path: "resources/2026-04-06T11-59-59-abcd-10min-old.md".to_string(), - }, - RemovedExtensionResource { - extension: "chronicle".to_string(), - resource_path: "resources/2026-04-07T12-00-00-abcd-10min-cutoff.md".to_string(), - }, - ] - ); - assert!( - tokio::fs::try_exists(&old_file) - .await - .expect("check old file before remove") - ); - assert!( - tokio::fs::try_exists(&exact_cutoff_file) - .await - .expect("check cutoff file before remove") - ); - - remove_extension_resources(&pending).await; - - assert!( - !tokio::fs::try_exists(&old_file) - .await - .expect("check old file") - ); - assert!( - !tokio::fs::try_exists(&exact_cutoff_file) - .await - .expect("check cutoff file") - ); - assert!( - tokio::fs::try_exists(&recent_file) - .await - .expect("check recent file") - ); - assert!( - tokio::fs::try_exists(&invalid_file) - .await - .expect("check invalid file") - ); - assert!( - tokio::fs::try_exists(&ignored_old_file) - .await - .expect("check ignored old file") - ); - } -} diff --git a/codex-rs/core/src/memories/mod.rs b/codex-rs/core/src/memories/mod.rs index d796063d2d..efa70f4bbb 100644 --- a/codex-rs/core/src/memories/mod.rs +++ b/codex-rs/core/src/memories/mod.rs @@ -1,37 +1,23 @@ -//! Memory subsystem for startup extraction and consolidation. +//! Memory startup extraction and consolidation orchestration. //! //! The startup memory pipeline is split into two phases: //! - Phase 1: select rollouts, extract stage-1 raw memories, persist stage-1 outputs, and enqueue consolidation. //! - Phase 2: claim a global consolidation lock, materialize consolidation inputs, and dispatch one consolidation agent. -pub(crate) mod citations; -mod control; mod phase1; mod phase2; -pub(crate) mod prompts; mod start; -mod storage; #[cfg(test)] mod tests; -pub(crate) mod usage; use codex_protocol::openai_models::ReasoningEffort; -pub use control::clear_memory_roots_contents; /// Starts the memory startup pipeline for eligible root sessions. /// This is the single entrypoint that `codex` uses to trigger memory startup. /// /// This is the entry point to read and understand this module. pub(crate) use start::start_memories_startup_task; -mod artifacts { - pub(super) const EXTENSIONS_SUBDIR: &str = "memories_extensions"; - pub(super) const ROLLOUT_SUMMARIES_SUBDIR: &str = "rollout_summaries"; - pub(super) const RAW_MEMORIES_FILENAME: &str = "raw_memories.md"; -} - -mod extensions; - /// Phase 1 (startup extraction). mod phase_one { /// Default model used for phase 1. @@ -39,21 +25,9 @@ mod phase_one { /// Default reasoning effort used for phase 1. pub(super) const REASONING_EFFORT: super::ReasoningEffort = super::ReasoningEffort::Low; /// Prompt used for phase 1. - pub(super) const PROMPT: &str = include_str!("../../templates/memories/stage_one_system.md"); + pub(super) const PROMPT: &str = codex_memories_write::STAGE_ONE_PROMPT; /// Concurrency cap for startup memory extraction and consolidation scheduling. pub(super) const CONCURRENCY_LIMIT: usize = 8; - /// Fallback stage-1 rollout truncation limit (tokens) when model metadata - /// does not include a valid context window. - pub(super) const DEFAULT_STAGE_ONE_ROLLOUT_TOKEN_LIMIT: usize = 150_000; - /// Maximum number of tokens from `memory_summary.md` injected into memory - /// tool developer instructions. - pub(super) const MEMORY_TOOL_DEVELOPER_INSTRUCTIONS_SUMMARY_TOKEN_LIMIT: usize = 5_000; - /// Portion of the model effective input window reserved for the stage-1 - /// rollout input. - /// - /// Keeping this below 100% leaves room for system instructions, prompt - /// framing, and model output. - pub(super) const CONTEXT_WINDOW_PERCENT: i64 = 70; /// Lease duration (seconds) for phase-1 job ownership. pub(super) const JOB_LEASE_SECONDS: i64 = 3_600; /// Backoff delay (seconds) before retrying a failed stage-1 extraction job. @@ -97,27 +71,3 @@ mod metrics { /// Histogram for aggregate token usage across one phase-2 consolidation run. pub(super) const MEMORY_PHASE_TWO_TOKEN_USAGE: &str = "codex.memory.phase2.token_usage"; } - -use codex_utils_absolute_path::AbsolutePathBuf; -use std::path::Path; -use std::path::PathBuf; - -pub fn memory_root(codex_home: &AbsolutePathBuf) -> AbsolutePathBuf { - codex_home.join("memories") -} - -fn rollout_summaries_dir(root: &Path) -> PathBuf { - root.join(artifacts::ROLLOUT_SUMMARIES_SUBDIR) -} - -fn memory_extensions_root(root: &Path) -> PathBuf { - root.with_file_name(artifacts::EXTENSIONS_SUBDIR) -} - -fn raw_memories_file(root: &Path) -> PathBuf { - root.join(artifacts::RAW_MEMORIES_FILENAME) -} - -async fn ensure_layout(root: &Path) -> std::io::Result<()> { - tokio::fs::create_dir_all(rollout_summaries_dir(root)).await -} diff --git a/codex-rs/core/src/memories/phase1.rs b/codex-rs/core/src/memories/phase1.rs index 8fed735c49..40f65c6db0 100644 --- a/codex-rs/core/src/memories/phase1.rs +++ b/codex-rs/core/src/memories/phase1.rs @@ -5,13 +5,13 @@ use crate::context::is_memory_excluded_contextual_user_fragment; use crate::memories::metrics; use crate::memories::phase_one; use crate::memories::phase_one::PRUNE_BATCH_SIZE; -use crate::memories::prompts::build_stage_one_input_message; use crate::rollout::INTERACTIVE_SESSION_SOURCES; use crate::rollout::policy::should_persist_response_item_for_memories; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use codex_api::ResponseEvent; use codex_config::types::MemoriesConfig; +use codex_memories_write::build_stage_one_input_message; use codex_otel::SessionTelemetry; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::config_types::ServiceTier; diff --git a/codex-rs/core/src/memories/phase2.rs b/codex-rs/core/src/memories/phase2.rs index 248e61dbab..d156d1dee5 100644 --- a/codex-rs/core/src/memories/phase2.rs +++ b/codex-rs/core/src/memories/phase2.rs @@ -1,20 +1,21 @@ use crate::agent::AgentStatus; use crate::agent::status::is_final as is_final_agent_status; use crate::config::Config; -use crate::memories::extensions::PendingExtensionResourceRemoval; -use crate::memories::extensions::find_old_extension_resources; -use crate::memories::extensions::remove_extension_resources; -use crate::memories::memory_root; use crate::memories::metrics; use crate::memories::phase_two; -use crate::memories::prompts::build_consolidation_prompt; -use crate::memories::storage::rebuild_raw_memories_file_from_memories; -use crate::memories::storage::rollout_summary_file_stem; -use crate::memories::storage::sync_rollout_summaries_from_memories; use crate::session::emit_subagent_session_started; use crate::session::session::Session; use codex_config::Constrained; use codex_features::Feature; +use codex_memories_write::build_consolidation_prompt; +use codex_memories_write::memory_root; +use codex_memories_write::prune_old_extension_resources; +use codex_memories_write::rebuild_raw_memories_file_from_memories; +use codex_memories_write::sync_rollout_summaries_from_memories; +use codex_memories_write::workspace::memory_workspace_diff; +use codex_memories_write::workspace::prepare_memory_workspace; +use codex_memories_write::workspace::reset_memory_workspace_baseline; +use codex_memories_write::workspace::write_workspace_diff; use codex_protocol::ThreadId; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -25,7 +26,7 @@ use codex_protocol::user_input::UserInput; use codex_state::Stage1Output; use codex_state::StateRuntime; use std::collections::HashMap; -use std::collections::HashSet; +use std::path::Path; use std::sync::Arc; use std::time::Duration; use tokio::sync::watch; @@ -59,7 +60,7 @@ pub(super) async fn run(session: &Arc, config: Arc) { let max_raw_memories = config.memories.max_raw_memories_for_consolidation; let max_unused_days = config.memories.max_unused_days; - // 1. Claim the job. + // 1. Claim the global Phase 2 lock before touching the memory workspace. let claim = match job::claim(session, db).await { Ok(claim) => claim, Err(e) => { @@ -72,71 +73,76 @@ pub(super) async fn run(session: &Arc, config: Arc) { } }; - // 2. Get the config for the agent - let Some(agent_config) = agent::get_config(config.clone()) else { + // 2. Ensure the memories root has a git baseline repository. + if let Err(err) = prepare_memory_workspace(&root).await { + tracing::error!("failed preparing memory workspace: {err}"); + job::failed(session, db, &claim, "failed_prepare_workspace").await; + return; + } + + // 3. Build the locked-down config used by the consolidation agent. + let Some(agent_config) = agent::get_config(config.as_ref()) else { // If we can't get the config, we can't consolidate. tracing::error!("failed to get agent config"); job::failed(session, db, &claim, "failed_sandbox_policy").await; return; }; - // 3. Query the memories - let selection = match db + // 4. Load current DB-backed Phase 2 inputs. + let raw_memories = match db .get_phase2_input_selection(max_raw_memories, max_unused_days) .await { - Ok(selection) => selection, + Ok(raw_memories) => raw_memories, Err(err) => { - tracing::error!("failed to list stage1 outputs from global: {}", err); + tracing::error!("failed to list stage1 outputs from global: {err}"); job::failed(session, db, &claim, "failed_load_stage1_outputs").await; return; } }; - let raw_memories = selection.selected.to_vec(); - let artifact_memories = artifact_memories_for_phase2(&selection); + let raw_memory_count = raw_memories.len(); let new_watermark = get_watermark(claim.watermark, &raw_memories); - // 4. Update the file system by syncing the raw memories with the one extracted from DB at - // step 3 - // [`rollout_summaries/`] - if let Err(err) = - sync_rollout_summaries_from_memories(&root, &artifact_memories, artifact_memories.len()) - .await - { - tracing::error!("failed syncing local memory artifacts for global consolidation: {err}"); - job::failed(session, db, &claim, "failed_sync_artifacts").await; + // 5. Sync the current inputs into the memory workspace. + if let Err(err) = sync_phase2_workspace_inputs(&root, &raw_memories).await { + tracing::error!("failed syncing phase2 workspace inputs: {err}"); + job::failed(session, db, &claim, "failed_sync_workspace_inputs").await; return; } - // [`raw_memories.md`] - if let Err(err) = - rebuild_raw_memories_file_from_memories(&root, &artifact_memories, artifact_memories.len()) - .await - { - tracing::error!("failed syncing local memory artifacts for global consolidation: {err}"); - job::failed(session, db, &claim, "failed_rebuild_raw_memories").await; - return; - } - let pending_extension_resource_removals = find_old_extension_resources(&root).await; - let removed_extension_resources = pending_extension_resource_removals - .iter() - .map(|resource| resource.removed.clone()) - .collect::>(); - if raw_memories.is_empty() && pending_extension_resource_removals.is_empty() { + + // 6. Use git to decide whether the synced workspace actually changed. + let workspace_diff = match memory_workspace_diff(&root).await { + Ok(diff) => diff, + Err(err) => { + tracing::error!("failed checking memory workspace changes: {err}"); + job::failed(session, db, &claim, "failed_workspace_status").await; + return; + } + }; + if !workspace_diff.has_changes() { + tracing::error!("Phase 2 no changes"); // We check only after sync of the file system. job::succeed( session, db, &claim, new_watermark, - &[], - "succeeded_no_input", + &raw_memories, + "succeeded_no_workspace_changes", ) .await; return; } - // 5. Spawn the agent - let prompt = agent::get_prompt(config, &selection, &removed_extension_resources); + // 7. Persist the diff for the consolidation agent to inspect. + if let Err(err) = write_workspace_diff(&root, &workspace_diff).await { + tracing::error!("failed writing memory workspace diff file: {err}"); + job::failed(session, db, &claim, "failed_workspace_diff_file").await; + return; + } + + // 8. Spawn the consolidation agent. + let prompt = agent::get_prompt(&root); let source = SessionSource::SubAgent(SubAgentSource::MemoryConsolidation); let agent_control = session.services.agent_control.detached_registry(); let thread_id = match agent_control @@ -172,39 +178,34 @@ pub(super) async fn run(session: &Arc, config: Arc) { warn!("failed to load memory consolidation thread config for analytics: {thread_id}"); } - // 6. Spawn the agent handler. + // 9. Hand off completion handling, heartbeats, and baseline reset. agent::handle( session, claim, new_watermark, raw_memories.clone(), - pending_extension_resource_removals, + root, thread_id, agent_control, phase_two_e2e_timer, ); - // 7. Metrics and logs. + // 10. Emit dispatch metrics. let counters = Counters { - input: raw_memories.len() as i64, + input: raw_memory_count as i64, }; emit_metrics(session, counters); } -fn artifact_memories_for_phase2( - selection: &codex_state::Phase2InputSelection, -) -> Vec { - let mut seen = HashSet::new(); - let mut memories = selection.selected.clone(); - for memory in &selection.selected { - seen.insert(rollout_summary_file_stem(memory)); - } - for memory in &selection.previous_selected { - if seen.insert(rollout_summary_file_stem(memory)) { - memories.push(memory.clone()); - } - } - memories +async fn sync_phase2_workspace_inputs( + root: &Path, + raw_memories: &[Stage1Output], +) -> std::io::Result<()> { + let raw_memory_count = raw_memories.len(); + sync_rollout_summaries_from_memories(root, raw_memories, raw_memory_count).await?; + rebuild_raw_memories_file_from_memories(root, raw_memories, raw_memory_count).await?; + prune_old_extension_resources(root).await; + Ok(()) } mod job { @@ -234,7 +235,9 @@ mod job { ); (ownership_token, input_watermark) } - codex_state::Phase2JobClaimOutcome::SkippedNotDirty => return Err("skipped_not_dirty"), + codex_state::Phase2JobClaimOutcome::SkippedRetryUnavailable => { + return Err("skipped_retry_unavailable"); + } codex_state::Phase2JobClaimOutcome::SkippedRunning => return Err("skipped_running"), }; @@ -293,9 +296,9 @@ mod job { mod agent { use super::*; - pub(super) fn get_config(config: Arc) -> Option { + pub(super) fn get_config(config: &Config) -> Option { let root = memory_root(&config.codex_home); - let mut agent_config = config.as_ref().clone(); + let mut agent_config = config.clone(); agent_config.cwd = root.clone(); // Consolidation threads must never feed back into phase-1 memory generation. @@ -342,13 +345,8 @@ mod agent { Some(agent_config) } - pub(super) fn get_prompt( - config: Arc, - selection: &codex_state::Phase2InputSelection, - removed_extension_resources: &[crate::memories::extensions::RemovedExtensionResource], - ) -> Vec { - let root = memory_root(&config.codex_home); - let prompt = build_consolidation_prompt(&root, selection, removed_extension_resources); + pub(super) fn get_prompt(root: &Path) -> Vec { + let prompt = build_consolidation_prompt(root); vec![UserInput::Text { text: prompt, text_elements: vec![], @@ -362,7 +360,7 @@ mod agent { claim: Claim, new_watermark: i64, selected_outputs: Vec, - pending_extension_resource_removals: Vec, + memory_root: codex_utils_absolute_path::AbsolutePathBuf, thread_id: ThreadId, agent_control: crate::agent::AgentControl, phase_two_e2e_timer: Option, @@ -386,20 +384,38 @@ mod agent { }; // Loop the agent until we have the final status. - let final_status = loop_agent( - db.clone(), - claim.token.clone(), - new_watermark, - thread_id, - rx, - ) - .await; + let final_status = loop_agent(db.clone(), claim.token.clone(), thread_id, rx).await; if matches!(final_status, AgentStatus::Completed(_)) { if let Some(token_usage) = agent_control.get_total_token_usage(thread_id).await { emit_token_usage_metrics(&session, &token_usage); } - if job::succeed( + // Do not reset the workspace baseline if we lost the lock. + let Ok(still_owns_lock) = db + .heartbeat_global_phase2_job(&claim.token, phase_two::JOB_LEASE_SECONDS) + .await + .inspect_err(|err| { + tracing::error!( + "failed confirming global memory consolidation ownership before resetting workspace baseline: {err}" + ); + }) + else { + job::failed(&session, &db, &claim, "failed_confirm_ownership").await; + return; + }; + if !still_owns_lock { + tracing::error!( + "lost global memory consolidation ownership before resetting workspace baseline" + ); + return; + } + + if let Err(err) = reset_memory_workspace_baseline(&memory_root).await { + tracing::error!("failed resetting memory workspace baseline: {err}"); + job::failed(&session, &db, &claim, "failed_workspace_commit").await; + return; + } + if !job::succeed( &session, &db, &claim, @@ -409,7 +425,9 @@ mod agent { ) .await { - remove_extension_resources(&pending_extension_resource_removals).await; + tracing::error!( + "failed marking global memory consolidation job succeeded after resetting workspace baseline" + ); } } else { job::failed(&session, &db, &claim, "failed_agent").await; @@ -433,7 +451,6 @@ mod agent { async fn loop_agent( db: Arc, token: String, - _new_watermark: i64, thread_id: ThreadId, mut rx: watch::Receiver, ) -> AgentStatus { @@ -491,7 +508,7 @@ pub(super) fn get_watermark( .map(|memory| memory.source_updated_at.timestamp()) .max() .unwrap_or(claimed_watermark) - .max(claimed_watermark) // todo double check the claimed here. + .max(claimed_watermark) } fn emit_metrics(session: &Arc, counters: Counters) { diff --git a/codex-rs/core/src/memories/prompts.rs b/codex-rs/core/src/memories/prompts.rs deleted file mode 100644 index 9425a53804..0000000000 --- a/codex-rs/core/src/memories/prompts.rs +++ /dev/null @@ -1,293 +0,0 @@ -use crate::memories::extensions::EXTENSION_RESOURCE_RETENTION_DAYS; -use crate::memories::extensions::RemovedExtensionResource; -use crate::memories::memory_extensions_root; -use crate::memories::memory_root; -use crate::memories::phase_one; -use crate::memories::storage::rollout_summary_file_stem_from_parts; -use codex_protocol::openai_models::ModelInfo; -use codex_state::Phase2InputSelection; -use codex_state::Stage1Output; -use codex_state::Stage1OutputRef; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_output_truncation::TruncationPolicy; -use codex_utils_output_truncation::truncate_text; -use codex_utils_template::Template; -use std::fmt::Write as _; -use std::path::Path; -use std::sync::LazyLock; -use tokio::fs; -use tracing::warn; - -static CONSOLIDATION_PROMPT_TEMPLATE: LazyLock