Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
All notable changes to this project are documented here. Format loosely follows
[Keep a Changelog](https://keepachangelog.com/); this project uses semantic versioning.

## [Unreleased]

### Added
- **Per-advisor `default_model` + model aliases**: codex second opinions now
default to `gpt-6-astra` (GPT-6 Astra), with `gpt6`/`astra` as case-insensitive
aliases scoped to the codex advisor only. Override via
`advisors.json` (`"codex": {"default_model": null}`) or `--model default`,
which suppresses the `--model` flag entirely. Defaults are never injected on
resume, and the resolved model id is what gets persisted to the session
registry and `command.json`.

## [0.1.5] - 2026-07-20

### Added
Expand Down
34 changes: 33 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,45 @@ Create `~/.config/crossagent/advisors.json`:
{
"advisors": {
"codex": { "executable": "codex", "base_args": ["exec", "--full-auto"] },
"myllm": { "executable": "myllm", "prompt_delivery": "flag:-q", "model_flag": "--model" }
"myllm": { "executable": "myllm", "prompt_delivery": "flag:-q", "model_flag": "--model", "default_model": "myllm-pro" }
}
}
```

Fields layer onto the built-ins, so you only specify what differs. `prompt_delivery` is `dashdash` (prompt after `--`), `positional` (prompt as last arg), or `flag:<flag>` (prompt is the value of a flag).

### Model selection

`--model` takes a model id or a **per-advisor alias**. Aliases are scoped to one
advisor so a name never leaks across CLIs — for `codex`, `gpt6` and `astra` both
expand to `gpt-6-astra` (case-insensitive); anything else is passed through
verbatim, so each CLI still resolves its own shorthands.

Each advisor may declare a `default_model`, used when `--model` is omitted.
**Codex second opinions now default to `gpt-6-astra`** (GPT-6 Astra); every other
advisor still has no default and falls through to whatever its own CLI picks.

Resolution order for a fresh invocation:

1. explicit `--model <id-or-alias>` (alias-expanded),
2. the advisor's `default_model`,
3. no `--model` flag at all — the advisor CLI's own default.

Two escape hatches:

- `--model default` (any case) suppresses the flag entirely, so your own
`~/.codex/config.toml` decides.
- Clear the default permanently in `~/.config/crossagent/advisors.json`:

```json
{ "advisors": { "codex": { "default_model": null } } }
```

On **resume**, a `default_model` is never injected — switching models mid-thread
would change the advisor under an existing conversation. An explicit `--model`
still applies. The resolved id (not the alias) is what gets persisted to the
session registry and a job's `command.json`.

## Skill usage inside an agent

Once installed, the skill auto-triggers on phrases like *"ask Claude"*, *"debate with Claude"*, *"ask Codex"*, *"second opinion"*, *"hỏi ý với Claude"*. The agent packages context, runs `crossagent`, and reports both views. See [`skills/crossagent/SKILL.md`](skills/crossagent/SKILL.md).
Expand Down
61 changes: 61 additions & 0 deletions docs/plans/gpt6-astra-default-model.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Plan: GPT-6 Astra model support in crossagent (rev2, post fable-5.1 review)

## Context (verified 2026-09-08)
- OpenAI **GPT-6 Astra** released 2026-09-03. API model id: `gpt-6-astra`. Codex-native (`codex exec --model gpt-6-astra`). 1.05M ctx / 128K out, cutoff Apr 30 2026, $10/$50 per M. Successor to GPT-5.6 Sol.
- crossagent: `Advisor` frozen dataclass (`advisors.py`). Model flows only via `--model` at `cli.py:67-68`, placed on argv BEFORE the resume subcommand (`cli.py:91-101`). `args.model` is also persisted raw to the session registry (`cli.py:299`) and `command.json` (`cli.py:767`). `_coerce` (`advisors.py:146-151`) forwards any non-name key straight into `replace` — a `default_model` JSON key needs no special-casing (confirmed by review).

## Goal
Make GPT-6 Astra the default second-opinion model for the **codex** advisor, with a clean escape hatch and per-advisor aliases. Backward-compatible for every other advisor. Codex behaviour DOES change (documented).

## Design (revised per review)
1. `Advisor.default_model: str | None = None` (add after `model_flag`; all later fields have defaults and all constructions are keyword — safe).
2. **Per-advisor** aliases: `MODEL_ALIASES: dict[str, dict[str, str]]` keyed by advisor name. `{"codex": {"gpt6": "gpt-6-astra", "astra": "gpt-6-astra"}}`. **Drop any `fable` alias** — the Claude CLI resolves `fable` itself; pinning it is wrong.
3. `resolve_model(name: str | None, advisor_name: str) -> str | None`: guard `isinstance(name, str)`; `strip()`; return `None` if empty; else return per-advisor alias hit (case-insensitive) or the stripped value verbatim.
4. `codex` built-in gets `default_model="gpt-6-astra"`. All others stay `None`.
5. **Escape hatch:** the literal `--model default` (case-insensitive) suppresses the model flag entirely, so a user falls back to their own codex `config.toml`. `--model ""` (empty) also falls through to `default_model` (today's fall-through), while `--model default` means "no flag".
6. **Single source of truth:** `effective_model(advisor, args) -> str` in cli.py, used by `build_command`, the registry `record` call (`cli.py:299`), and `_write_command_info` (`cli.py:767`) — so the RESOLVED id (`gpt-6-astra`), not `""`/`gpt6`, is persisted everywhere.
7. **Do not switch model mid-thread on resume:** apply `default_model` only on a FRESH invocation. When a resume is being emitted (`args.resume` set, or stored_id used without `--new-session`), skip the default (explicit `--model` still applies exactly as today, unchanged position). This avoids `codex exec --model gpt-6-astra ... resume <thread>` forcing a switch on every stored thread.

## Tasks (implement as one coherent unit — coupled files)

### T1 — `advisors.py`
- Add `default_model` field.
- Add `MODEL_ALIASES` (per-advisor, codex only) + `resolve_model(name, advisor_name)` with the robustness guards above.
- Set codex `default_model="gpt-6-astra"`.
- Export `resolve_model`, `MODEL_ALIASES` (module-level; no `__all__` gymnastics needed).

### T2 — `cli.py`
- Add `effective_model(advisor, args, *, is_resume) -> str`:
- `SENTINEL "default"` → return `""` (suppress).
- `raw = args.model or ("" if is_resume else advisor.default_model or "")`.
- return `advisors.resolve_model(raw, advisor.name) or ""`.
- In `build_command` (line ~67): compute `is_resume` (mirror the resume conditions at 91-101), call `effective_model`, emit `[model_flag, chosen]` only when `chosen` and `advisor.model_flag`.
- At `cli.py:299` and `cli.py:767`, persist `effective_model(advisor, args, is_resume=...)` instead of raw `args.model`. (Both are post-build; reuse a value computed once and thread it, or recompute — recompute is fine, pure function.)
- Update `--model` help (line ~163): mention aliases, that empty falls back to advisor `default_model` then the CLI default, and that `default` suppresses the flag.
- `_print_advisors` (line ~208): print each advisor's `default_model` when set, so users see codex → gpt-6-astra.

### T3 — Tests (`tests/test_advisors.py`, `tests/test_cli.py`)
- `resolve_model`: codex alias hit (case-insensitive) → `gpt-6-astra`; same alias under `claude` → passthrough verbatim (no cross-advisor leak); non-str input → `None`; whitespace-only → `None`; unknown → verbatim stripped.
- codex `default_model == "gpt-6-astra"`; every other built-in `None`.
- `build_command`: codex + no `--model` (fresh) → includes `--model gpt-6-astra`; codex + `--model gpt6` → `gpt-6-astra`; codex + `--model default` → NO `--model` flag; claude + no model → no flag (unchanged); claude + `--model gpt6` → passes `gpt6` verbatim.
- Resume: codex with a stored thread (resume path) + no `--model` → NO forced `--model` (default skipped); explicit `--model X` on resume → `--model X` still present before `resume`.
- Persistence: registry `record` and `command.json` receive `gpt-6-astra` (resolved), not `""`/`gpt6` — assert via the `start`/dispatch path and `_write_command_info`.
- `"default_model": null` user-config override clears the codex default.
- Job `start` path via `_add_advisor_args` exercises `build_command` too — one test through that entry point.

### T4 — Docs
- `README.md`: model-selection subsection — per-advisor aliases, `default_model`, codex defaults to `gpt-6-astra`, `--model default` escape hatch, one JSON override example (`"codex": {"default_model": null}` to opt out). Update the line-282 example to show `default_model`.
- `CHANGELOG.md`: unreleased `feat(models): per-advisor default_model + aliases; codex second opinions default to gpt-6-astra (override via advisors.json or --model default)`.
- No version bump / release.

## Constraints
- Backward compatible for all advisors EXCEPT codex (documented behaviour change).
- No new deps. Python 3.9+ (`from __future__ import annotations` already present).
- Surgical diffs; no unrelated refactors/formatting. No secrets. No AI/tool attribution in commits/MR.

## Verification
- `python -m pytest -q` green (was 298 passed).
- Manual: `python -m crossagent --list` shows codex default; a dry command build (via test) shows `--model gpt-6-astra`.

## Out of scope (follow-up)
- Reasoning-effort passthrough (`--reasoning low|medium|high|xhigh|max`), cost/pricing tracking, wiring GPT-6 into Ringkas repos.
18 changes: 18 additions & 0 deletions src/crossagent/advisors.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class Advisor:
invoke_args: tuple[str, ...] = ()
prompt_delivery: str = "positional" # "dashdash" | "positional" | "flag:<flag>"
model_flag: str | None = None
default_model: str | None = None
stream_args: tuple[str, ...] = ()
json_args: tuple[str, ...] = ()
resume_flag: str | None = None
Expand Down Expand Up @@ -146,6 +147,7 @@ def supports_stream(self) -> bool:
base_args=("exec", "--skip-git-repo-check"),
prompt_delivery="positional",
model_flag="--model",
default_model="gpt-6-astra",
json_args=("--json",),
stream_args=("--json",),
result_parser="codex-jsonl",
Expand Down Expand Up @@ -219,6 +221,22 @@ def supports_stream(self) -> bool:
# Friendly aliases callers may type.
_ALIASES = {"cmd": "commandcode", "cc": "claude", "oc": "opencode"}

# Short model aliases, scoped per advisor so a name never leaks across CLIs
# (each advisor's own CLI resolves its own shorthands; we only expand ours).
MODEL_ALIASES: dict[str, dict[str, str]] = {
"codex": {"gpt6": "gpt-6-astra", "astra": "gpt-6-astra"},
}


def resolve_model(name: str | None, advisor_name: str) -> str | None:
"""Expand a per-advisor model alias. Returns None when no model was requested."""
if not isinstance(name, str):
return None
candidate = name.strip()
if not candidate:
return None
return MODEL_ALIASES.get(advisor_name, {}).get(candidate.lower(), candidate)


def _coerce(name: str, raw: dict[str, Any]) -> Advisor:
"""Build an Advisor from a user-config dict, layering onto a built-in if one exists."""
Expand Down
73 changes: 67 additions & 6 deletions src/crossagent/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,46 @@ def _redacted_command(cmd: list[str]) -> str:
return shlex.join([*cmd[:-1], "<prompt>"])


# Literal `--model default` (any case) means "emit no model flag at all" — the
# escape hatch back to whatever the advisor CLI's own config already picks.
_MODEL_SENTINEL_DEFAULT = "default"


def _is_resume(
advisor: Advisor, args: argparse.Namespace, registry: dict[str, Any]
) -> bool:
"""True when this invocation will continue an existing thread (mirrors build_command)."""
if not advisor.supports_sessions:
return False
if args.resume and advisor.resume_flag:
return True
key = reg.session_key(advisor.name, args.name)
stored_id = reg.stored_session_id(registry, key)
return bool(
stored_id
and not args.new_session
and (advisor.resume_command or advisor.resume_flag)
)


def effective_model(
advisor: Advisor, args: argparse.Namespace, *, is_resume: bool
) -> str:
"""The model id actually used (and persisted): explicit --model, else advisor default.

An advisor's ``default_model`` is only injected on a FRESH invocation — forcing
a model onto a resumed thread would switch models mid-conversation.
"""
requested = getattr(args, "model", "") or ""
if (
isinstance(requested, str)
and requested.strip().lower() == _MODEL_SENTINEL_DEFAULT
):
return ""
raw = requested or ("" if is_resume else advisor.default_model or "")
return advisors_mod.resolve_model(raw, advisor.name) or ""


def build_command(
advisor: Advisor,
args: argparse.Namespace,
Expand All @@ -76,8 +116,11 @@ def build_command(
) -> tuple[list[str], str]:
cmd = [advisor.executable, *advisor.base_args, *advisor.invoke_args]

if args.model and advisor.model_flag:
cmd.extend([advisor.model_flag, args.model])
chosen_model = effective_model(
advisor, args, is_resume=_is_resume(advisor, args, registry)
)
if chosen_model and advisor.model_flag:
cmd.extend([advisor.model_flag, chosen_model])

if advisor.supports_stream:
cmd.extend(args.stream and advisor.stream_args or advisor.json_args)
Expand Down Expand Up @@ -179,7 +222,11 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace:
parser.add_argument(
"--model",
default="",
help="Advisor model or alias (advisor-specific). Empty = advisor default.",
help=(
"Advisor model or per-advisor alias (codex: gpt6/astra -> gpt-6-astra). "
"Empty falls back to the advisor's default_model, then to the CLI's own "
"default. Use 'default' to send no --model flag at all."
),
)
parser.add_argument(
"--safe-mode",
Expand Down Expand Up @@ -240,6 +287,8 @@ def _print_advisors() -> int:
for name, adv in sorted(advisors_mod.available().items()):
tag = " (experimental)" if adv.experimental else ""
print(f"{name:14} -> {adv.executable}{tag}")
if adv.default_model:
print(f"{'':14} default model: {adv.default_model}")
if adv.notes:
print(f"{'':14} {adv.notes}")
write_desc = shlex.join(adv.write_args) if adv.write_args else "unsupported"
Expand Down Expand Up @@ -344,7 +393,9 @@ def _dispatch(
name=args.name,
cwd=args.cwd or os.getcwd(),
advisor=advisor.name,
model=args.model,
model=effective_model(
advisor, args, is_resume=_is_resume(advisor, args, registry)
),
)
print(
f"[crossagent] saved session name={key} id={parsed.session_id}",
Expand Down Expand Up @@ -697,7 +748,15 @@ def _cmd_start(args: argparse.Namespace) -> int:

job_dir = jobs_mod.create_job_dir(state_root, job_id)
_write_job_prompt(job_dir, args._prompt)
_write_command_info(job_dir, advisor, args, cmd, key, registry_path)
_write_command_info(
job_dir,
advisor,
args,
cmd,
key,
registry_path,
is_resume=_is_resume(advisor, args, registry),
)

now = datetime.now(timezone.utc).isoformat()
job = jobs_mod.Job(
Expand Down Expand Up @@ -1086,6 +1145,8 @@ def _write_command_info(
cmd: list[str],
key: str,
registry_path: Path,
*,
is_resume: bool = False,
) -> None:
info = {
"command": cmd,
Expand All @@ -1095,7 +1156,7 @@ def _write_command_info(
"registry_path": str(registry_path),
"key": key,
"name": args.name,
"model": args.model,
"model": effective_model(advisor, args, is_resume=is_resume),
"advisor": advisor.name,
"check": getattr(args, "check", None),
"check_timeout": getattr(
Expand Down
50 changes: 50 additions & 0 deletions tests/test_advisors.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,56 @@ def test_user_config_overrides_builtin(tmp_path):
assert registry["myllm"].prompt_delivery == "flag:-q"


def test_codex_defaults_to_gpt6_astra_and_others_have_no_default():
assert advisors.resolve("codex").default_model == "gpt-6-astra"
for name in ("claude", "opencode", "commandcode", "gemini"):
assert advisors.resolve(name).default_model is None


@pytest.mark.parametrize("alias", ["gpt6", "GPT6", "astra", "Astra", " gpt6 "])
def test_resolve_model_expands_codex_aliases_case_insensitively(alias):
assert advisors.resolve_model(alias, "codex") == "gpt-6-astra"


def test_codex_aliases_do_not_leak_to_other_advisors():
# The Claude CLI resolves its own shorthands; we must not rewrite them.
assert advisors.resolve_model("gpt6", "claude") == "gpt6"
assert advisors.resolve_model("astra", "gemini") == "astra"
assert advisors.resolve_model("fable", "claude") == "fable"


@pytest.mark.parametrize("value", [None, 0, 1.5, [], {}, object()])
def test_resolve_model_returns_none_for_non_strings(value):
assert advisors.resolve_model(value, "codex") is None


@pytest.mark.parametrize("value", ["", " ", "\t\n"])
def test_resolve_model_returns_none_for_blank_strings(value):
assert advisors.resolve_model(value, "codex") is None


def test_resolve_model_passes_unknown_models_through_stripped():
assert advisors.resolve_model(" gpt-5.6-sol ", "codex") == "gpt-5.6-sol"
assert advisors.resolve_model("opus", "claude") == "opus"


def test_user_config_can_clear_codex_default_model(tmp_path):
cfg = tmp_path / "advisors.json"
cfg.write_text(json.dumps({"advisors": {"codex": {"default_model": None}}}))
registry = advisors.available(cfg)
assert registry["codex"].default_model is None
# Other built-in fields survive the layering.
assert registry["codex"].executable == "codex"


def test_user_config_can_override_default_model(tmp_path):
cfg = tmp_path / "advisors.json"
cfg.write_text(
json.dumps({"advisors": {"codex": {"default_model": "gpt-5.6-sol"}}})
)
assert advisors.available(cfg)["codex"].default_model == "gpt-5.6-sol"


def test_malformed_user_config_is_ignored(tmp_path):
cfg = tmp_path / "advisors.json"
cfg.write_text("{ not json")
Expand Down
Loading