diff --git a/AGENTS.md b/AGENTS.md index 6ef4e41..9b9c446 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,8 +18,9 @@ monitoring and informs a model when an observed event provides useful work. Installation defaults for GitHub accounts, AI accounts, roles, and selections are unconfigured (`NULL`). Add them explicitly through configuration, with no fixed count. See DESIGN.md §19.8 and docs/github-accounts.md for the implemented -GitHub configuration, remaining ambient-`gh` gaps (such as `agent a38` -visibility lookup), and the AI/role configuration gap. +GitHub configuration, and docs/ai-accounts.md for AI profiles, named roles and +explicit launch/usage selections. A38 visibility lookup requires a configured +GitHub session unless visibility is explicitly supplied. Draft publication is immediate after the first signed task commit; see the lifecycle. A draft plus local tests is not done. Ready for review is signed diff --git a/DESIGN.md b/DESIGN.md index 7abcc46..8b2d828 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -463,14 +463,23 @@ Owned-row runtime fields (start/stop set control and tmux; `keep-working` may al "rows": 24, "provider": "grok", "grok_session_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", - "model": "grok-4.6", + "model": "operator-selected-model", "keep_working": { "standing_sent": true } } ``` Start sets `control=attached` and the tmux name. Stop sets `control=stopped` and keeps the name. `agent session keep-working` updates `runtime.keep_working.standing_sent` on idle ticks so the standing instruction is sent once. Session `status` (`active` / `closed`) is separate; `session close` stays as it is. -**Grok Build launch** (`--provider grok` or control `{provider: "grok"}`) is not the store session id. The Grok CLI `--session-id` flag accepts only a UUID (`8-4-4-4-12`). A caller-chosen session id (including a ULID) is never passed through. First start mints `runtime.grok_session_id` and runs `grok --always-approve --session-id --model grok-4.6`. Later starts, if that field is set, run `grok --always-approve --resume --model grok-4.6`. An empty model becomes `grok-4.6`; it must not inherit a Claude default. The pane is started with `env -u ANTHROPIC_API_KEY -u CLAUDECODE -u CLAUDE_CODE_ENTRYPOINT` so Claude credentials do not leak into the Grok process. `--provider` and `--cmd` cannot be combined. +**Grok Build launch** (`--provider grok` or control `{provider: "grok"}`) resolves +an explicit interactive role from `ai-accounts.json`. The model and provider +profile directory are required configuration; there is no built-in model. +The CLI accepts only a UUID, so first start creates `runtime.grok_session_id`. +A later start may resume that ID only with the same recorded AI binding +(role, account, provider, model, access and configuration-directory hash). +Changing that binding, or adopting an old unbound ID, requires a new session. +Provider credential environment variables are cleared in the child process, +and `GROK_HOME` selects the configured profile. `--provider` and `--cmd` cannot +be combined; `--model`, when supplied, must match the configured role. **Vendors remain `grok` | `codex`.** A process running inside a tmux pane is not a store vendor. There is no `vendor=claude` and no shell-string tmux driver: the runtime invokes `tmux` with argv lists only. `runtime.provider` is launch metadata, not a review-gate vendor. @@ -658,14 +667,18 @@ executor verifies the selected login and never falls back to an ambient login or a different configured account. GitHub execution identities do not change the device's hub identity or transfer ownership of store rows. -**Remaining implementation boundary:** this GitHub configuration does not cover -every CLI path that may invoke `gh`. In particular, `agent a38` visibility -lookup still runs ambient host `gh repo view` when `--private` is omitted -(`src/agent_cli/a38.py`), without loading `github-accounts.json` or a session -binding. This manifest also does not yet implement configurable AI accounts or -user-defined roles. The role/vendor lists and model choices in `lane.py`, and -the Grok default in `runtime.py`, still contain fixed values. They must not be -presented as satisfying the complete empty-default configuration requirement. +[AI configuration](docs/ai-accounts.md) supplies arbitrarily named account +profiles and roles, explicit models/access and session-to-role bindings. It is +used by lane launches, interactive Grok starts and optional Grok usage reads. +Absent configuration authorizes no AI launch. Fixed workflow kinds and the +Grok/Codex provider adapters are protocol capabilities, not installed account +or role selections. A38 visibility lookup uses an explicitly bound GitHub +session, or operator-supplied visibility without a GitHub lookup. + +The standalone PR guard retains its explicit workflow-token configuration. +Trusted script APIs and raw terminal commands are not a sandbox. Complete +issue-to-PR orchestration and technical restrictions on model tools remain +separate work as documented in §19.7. ## 20. Refused: hub as a coding control plane @@ -808,7 +821,7 @@ Phase 1 is this loop plus a backlog. Smarter questions are later. ## 23. Document history -Recorded from the design thread that specified realtime team visibility, rejected a central write database and a mesh, rejected embedding the hub in the existing public API, chose GitHub login + git teams, and split the work into `agent` + `agent-core`. Control: local tmux ownership, hub control frames, ephemeral terminal bytes. Grok launch: own UUID in `runtime.grok_session_id`, `--resume` on later starts, default model `grok-4.6`, no Claude environment in the pane. +Recorded from the design thread that specified realtime team visibility, rejected a central write database and a mesh, rejected embedding the hub in the existing public API, chose GitHub login + git teams, and split the work into `agent` + `agent-core`. Control: local tmux ownership, hub control frames, ephemeral terminal bytes. Grok launch: own UUID in `runtime.grok_session_id`, `--resume` on later starts, the historical default model (now replaced by explicit AI configuration), no Claude environment in the pane. This revision replaces default complete pull with own events + inbox/subscription snapshots, moves the local engine to PostgreSQL, requires a session row, adds the `activity` catalog and opt-in skills, and adds session-addressed mail with a TUI knock of `da ist Post id ` only. diff --git a/README.md b/README.md index ac896c4..ead5d66 100644 --- a/README.md +++ b/README.md @@ -26,10 +26,13 @@ Accounts and roles must start unconfigured (`NULL`); see execution accounts and session bindings explicitly in [`github-accounts.json`](docs/github-accounts.md) for the executors that load it. There is no default GitHub account or fallback to the host login on those -paths. Some legacy CLI paths still use ambient host `gh` (for example -`agent a38` visibility lookup when `--private` is omitted); that gap is named -in the design and the accounts document. AI-account and role configuration -remain separate implementation work, as recorded in the design. +paths. A38 visibility lookup likewise requires an explicitly configured +`--github-session`, unless `--private` or `--public` supplies the report visibility. +[AI accounts and roles](docs/ai-accounts.md) configure provider profiles, models, +access and session bindings without built-in selections. `agent lane run` needs +`--session`; `agent run` uses its task session. Interactive Grok starts require +an interactive role binding. Automatic Grok usage reads are disabled until +`usage_session` is explicitly configured. ## Install diff --git a/docs/a38.md b/docs/a38.md index 72c30a1..4434c1a 100644 --- a/docs/a38.md +++ b/docs/a38.md @@ -85,7 +85,7 @@ agent a38 run --repo . --policy /tmp/a38-run/policy.json \ --output /tmp/a38-run/report.md --logs-dir /tmp/a38-run/logs --private ``` -The runner identifies the GitHub repository from `origin` by default; for a fork, pass `--repository OWNER/NAME` with the PR target repository. It queries that repository's visibility through `gh`; explicit `--private` or `--public` selects visibility when needed. A38 checks that visibility against the actual repository. The runner executes the complete job list sequentially, records each command's result, exit code, elapsed time and timeout, and continues independent jobs after failures. It exposes `A38_HEAD_SHA` and `A38_BASE_SHA` to commands, removes GitHub token variables from job environments, and retains per-job logs locally. It terminates timed-out process groups. An interrupted run or checkout drift must never produce a successful report. +The runner identifies the GitHub repository from `origin` by default; for a fork, pass `--repository OWNER/NAME` with the PR target repository. Without `--private` or `--public`, visibility lookup requires `--github-session SESSION` bound explicitly in `$AGENT_HOME/github-accounts.json`. The executor verifies the selected account and refuses an absent binding or mismatched login; it never uses ambient host authentication. Explicit `--private` or `--public` supplies the report visibility without a GitHub lookup. A38 checks that visibility against the actual repository. The runner executes the complete job list sequentially, records each command's result, exit code, elapsed time and timeout, and continues independent jobs after failures. It exposes `A38_HEAD_SHA` and `A38_BASE_SHA` to commands, removes GitHub token variables from job environments, and retains per-job logs locally. It terminates timed-out process groups. An interrupted run or checkout drift must never produce a successful report. Job adapter behavior is defined separately in [A38 job adapters](a38-job-adapters.md). Use `agent a38 job --config ''`; this standard intentionally does not duplicate adapter schemas. diff --git a/docs/ai-accounts.md b/docs/ai-accounts.md new file mode 100644 index 0000000..b1cf572 --- /dev/null +++ b/docs/ai-accounts.md @@ -0,0 +1,128 @@ +# AI accounts and roles + +Static scripts that load this manifest select AI provider accounts and +user-defined roles from `$AGENT_HOME/ai-accounts.json`. Installation creates no +accounts, roles, or session selections. A missing file, `{}`, or null/empty +`accounts`, `roles`, and `sessions` leaves the configuration empty. There is no +implicit provider profile, model, or role from the host environment, hub +pairing, or built-in defaults. + +This device-local manifest is used by `agent lane run`, the lane steps of +`agent run`, interactive Grok session starts, and optional Grok usage polling. +It does not implement the complete issue-to-PR coordinator or a sandbox. + +The following is an **operator-supplied example**, never an installed default: + +```json +{ + "accounts": { + "provider-profile": { + "provider": "grok", + "config_dir": "/operator/path" + } + }, + "roles": { + "my-builder": { + "account": "provider-profile", + "model": "operator-selected-model", + "access": "workspace-write" + } + }, + "sessions": { + "chosen-session": { + "interactive": "my-builder", + "lanes": { + "grok:implementer": "my-builder" + } + } + } +} +``` + +Add as many named accounts, roles, and session bindings as needed. No fixed +account list, role list, or count is built in. Configurable role names are +chosen by the operator; they are distinct from the fixed workflow kinds +(`implementer`, `reviewer`, `pr-reviewer-quality`, `pr-reviewer-logic`) and from +the supported provider adapters (`grok`, `codex`). Those workflow kinds and +adapters are protocol capabilities used when resolving a lane slot, not +preinstalled user roles. + +## Required fields + +Each account requires: + +- `provider`: `grok` or `codex` +- `config_dir`: absolute path to that profile's provider CLI configuration + directory (no NUL, newline, CR, or parent traversal) + +Each role requires: + +- `account`: name of a configured account +- `model`: explicit model id string (no default) +- `access`: `read-only` or `workspace-write` (no default) + +The optional top-level `usage_session` selects an explicitly configured +interactive Grok session for automatic billing reads. Missing/null disables +those reads; it never chooses the host profile or the first available account. +The selected session must exist locally, be owned and active. + +Each session may include: + +- `interactive`: role name for the interactive runner, or omit/null when none +- `lanes`: map of slot → role name, or omit/null/`{}` when none + +Lane slots are arbitrary non-empty single-line strings. Resolvers look up the +explicit key `vendor:workflow-kind` (for example `grok:implementer`) and require +the bound role's account provider to match `vendor`. For workflow kinds +`reviewer`, `pr-reviewer-quality`, and `pr-reviewer-logic`, the bound role must +use `access: read-only`; a writable binding is refused. + +Unknown top-level, account, role, or session fields are rejected. Dangling +account or role references are rejected. Credentials and API tokens must not +appear in this manifest; they belong only inside each `config_dir`. Error text +from the loader does not echo credential contents. + +## Process isolation prefix + +`AIRole.env_prefix()` returns an `env` argv prefix for child processes. It +removes ambient `XAI_API_KEY`, `GROK_API_KEY`, `OPENAI_API_KEY`, `CODEX_API_KEY`, +`ANTHROPIC_API_KEY`, `CLAUDECODE`, `CLAUDE_CODE_ENTRYPOINT`, `GROK_HOME`, and +`CODEX_HOME`, then sets `GROK_HOME` or `CODEX_HOME` to the role's account +`config_dir`. It does not mutate the parent `os.environ`. Unrelated environment +variables are left for the child to inherit. This is process configuration +isolation, not a sandbox and not a claim that the provider CLI is already +authenticated for that profile. + +## Interactive selection + +`AIAccounts.for_session(session_id)` resolves `sessions[session_id].interactive` +and fails when that binding is absent or null. The loader may represent Codex +roles in the manifest. Interactive runtime support remains Grok-only initially; +unsupported interactive providers are refused. The configured model is required, +and a supplied `--model` must agree with it. A resumed conversation is pinned to +the configured role, account, model, access and configuration-directory identity; +a changed or missing binding requires a new session. Existing unbound Grok +conversation IDs are not silently adopted under a newly configured account. + +## Migration from implicit defaults + +Before this manifest, lane launchers and the interactive runtime used fixed +vendor/role lists and built-in model choices in code. After adopting +`ai-accounts.json`: + +1. Create one account entry per provider CLI profile directory you intend to use. +2. Define roles with explicit `account`, `model`, and `access` (no omitted + fields). +3. Bind each session that should run lanes or an interactive runner: set + `lanes` keys such as `grok:implementer` and, when needed, `interactive`. +4. Existing sessions are unconfigured until those bindings are added. An empty + or missing file does not authorize a fallback identity. + +Configure sessions explicitly before enabling launch paths after upgrading. +For example, the static script invokes `agent lane run --session chosen-session +--role implementer --vendor grok --spec-file task.md --no-tmux`; it resolves the +`grok:implementer` slot to the operator-defined role. `agent run` resolves slots +using its task session. Interactive starts use `agent session start --id +chosen-session --provider grok`. Raw terminal `--cmd` remains an explicitly +supplied script command, outside provider-profile launch enforcement; it is not +a sandboxed model interface. No model lane may invoke these execution commands. diff --git a/docs/github-accounts.md b/docs/github-accounts.md index a1a5a07..da5536f 100644 --- a/docs/github-accounts.md +++ b/docs/github-accounts.md @@ -5,7 +5,9 @@ Static scripts that load this manifest select GitHub accounts from bindings. A missing file, `{}`, or null/empty `accounts` and `sessions` leaves those covered GitHub executors unconfigured. There is no implicit account from hub pairing, the host login, or environment tokens. CLI paths that never load -this file are outside this enforcement; see the remaining gaps below. +this file are outside this enforcement; see A38 visibility below for the +explicit `--github-session` path and the standalone PR guard's documented +workflow-token configuration. The following is an operator-supplied example, not an installed default: @@ -98,12 +100,13 @@ sessions explicitly before enabling these operations after upgrading. Hub pairing and event ownership are separate: using a second GitHub execution account does not re-pair the device or change ownership of its rows. -**Remaining gaps.** This manifest does not cover legacy ambient `gh` paths that -never load it. One reachable example is `agent a38` visibility lookup -(`gh repo view` when `--private` is omitted), which still uses the host `gh` -login. Configurable AI accounts and roles are part of the -[empty-default requirement](../DESIGN.md#198-configuration-starts-empty) and are -not implemented by this manifest. +**A38 visibility.** `agent a38 run` requires `--github-session SESSION` for +GitHub visibility lookup; the selected account is verified using this manifest. +Explicit `--private`/`--public` supplies report visibility without that lookup. +AI account and role selections use the separate [AI manifest](ai-accounts.md). +The standalone PR guard uses its documented explicitly supplied workflow-token +configuration; it does not select a host GitHub CLI login. Pure argv builders +and injected-runner libraries are not a sandbox; callers remain trusted scripts. Transfer options are deliberately limited to the explicit allowlists in `github_accounts.py`. Unknown options (including custom receive/upload programs), diff --git a/src/agent_cli/a38.py b/src/agent_cli/a38.py index a30184e..d2f1084 100644 --- a/src/agent_cli/a38.py +++ b/src/agent_cli/a38.py @@ -585,19 +585,27 @@ def _resolve_private( *, run: RunFn, repository: str, + github_session: str | None = None, + config_home: Path | None = None, ) -> bool: if private is not None: if not isinstance(private, bool): raise A38Error("private must be a boolean") return private - completed = run( - ["gh", "repo", "view", repository, "--json", "isPrivate"], - repo_path, - None, - ) + if not github_session: + raise A38Error("visibility lookup requires an explicitly configured --github-session, or --private/--public") + from .github_accounts import AccountError, load_accounts + try: + if config_home is None: + from .main import home + config_home = home() + account = load_accounts(config_home).for_session(github_session) + scoped = account.runner(lambda argv: run(argv, repo_path, None)) + completed = scoped(["gh", "repo", "view", repository, "--json", "isPrivate"]) + except AccountError as exc: + raise A38Error(str(exc)) from None if completed.returncode != 0: - detail = (completed.stderr or completed.stdout or "gh repo view failed").strip() - raise A38Error(detail or "gh repo view failed") + raise A38Error("configured GitHub account could not resolve repository visibility") try: payload = json.loads(completed.stdout) except json.JSONDecodeError as exc: @@ -872,6 +880,8 @@ def run_policy( run: RunFn | None = None, repository: str | None = None, policy_path: Path | None = None, + github_session: str | None = None, + config_home: Path | None = None, ) -> dict: """Execute policy jobs and write a complete ``dfx-local-ci/v1`` report.""" runner = run or _default_run @@ -906,7 +916,10 @@ def run_policy( head = _head_sha(root, run=runner) base = _ensure_commit_exists(root, base_sha, run=runner) repo = repository if repository is not None else _origin_repo(root, run=runner) - is_private = _resolve_private(root, private, run=runner, repository=repo) + is_private = _resolve_private( + root, private, run=runner, repository=repo, + github_session=github_session, config_home=config_home, + ) logs_dir.mkdir(parents=True, exist_ok=True) env = _job_env(head, base) @@ -1140,6 +1153,7 @@ def _cmd_run(args: argparse.Namespace) -> int: private=private, repository=args.repository, policy_path=Path(args.policy), + github_session=args.github_session, ) except (A38Error, OSError) as exc: print(f"a38: {exc}", file=sys.stderr) @@ -1167,6 +1181,7 @@ def build_parser() -> argparse.ArgumentParser: run_p.add_argument("--base-sha", required=True, dest="base_sha", help="Base commit SHA") run_p.add_argument("--private", action="store_true", help="Mark report private=true") run_p.add_argument("--public", action="store_true", help="Mark report private=false") + run_p.add_argument("--github-session", help="Explicit GitHub account session for visibility lookup") run_p.set_defaults(func=_cmd_run) verify_p = sub.add_parser("verify", help="Verify an author report against a trusted policy") diff --git a/src/agent_cli/ai_accounts.py b/src/agent_cli/ai_accounts.py new file mode 100644 index 0000000..0fef9a2 --- /dev/null +++ b/src/agent_cli/ai_accounts.py @@ -0,0 +1,229 @@ +"""Device-local AI account, role, and session bindings for static executors. + +Operators supply ``$AGENT_HOME/ai-accounts.json``. Installation creates no +accounts, roles, or selections. Credentials stay in each account's provider CLI +configuration directory; this manifest stores only directory paths. Static launch sites resolve these bindings before invoking a provider. +""" + +from __future__ import annotations + +import json +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from .store import StoreError + +CONFIG_FILE = "ai-accounts.json" +SUPPORTED_PROVIDERS = frozenset({"grok", "codex"}) +ACCESS_VALUES = frozenset({"read-only", "workspace-write"}) +# Fixed workflow kinds that must resolve to read-only access. These are protocol +# capabilities, not installed user-role names. +READ_ONLY_WORKFLOW_KINDS = frozenset( + {"reviewer", "pr-reviewer-quality", "pr-reviewer-logic"} +) +_CLEAR_ENV = ( + "XAI_API_KEY", + "GROK_API_KEY", + "OPENAI_API_KEY", + "CODEX_API_KEY", + "ANTHROPIC_API_KEY", + "CLAUDECODE", + "CLAUDE_CODE_ENTRYPOINT", + "GROK_HOME", + "CODEX_HOME", +) + + +class AccountError(StoreError): + """Configuration does not authorize this AI account or role selection.""" + + +def _text(value: Any, field: str) -> str: + if ( + not isinstance(value, str) + or not value.strip() + or "\x00" in value + or "\n" in value + or "\r" in value + ): + raise AccountError(f"{field} must be a non-empty single-line string") + return value + + +def _config_dir(value: Any, field: str) -> str: + text = _text(value, field) + path = Path(text) + if not path.is_absolute(): + raise AccountError(f"{field} must be an absolute path") + if ".." in path.parts: + raise AccountError(f"{field} must not contain parent traversal") + return text + + +@dataclass(frozen=True) +class AIAccount: + name: str + provider: str + config_dir: str + + +@dataclass(frozen=True) +class AIRole: + name: str + account: AIAccount + model: str + access: str + + def env_prefix(self) -> list[str]: + """Return an ``env`` argv prefix that isolates provider configuration. + + Clears ambient provider tokens and home variables for the child process + only, then sets ``GROK_HOME`` or ``CODEX_HOME`` to this role's account + ``config_dir``. Does not mutate ``os.environ``. This is process + configuration isolation, not a sandbox or a claim that the provider CLI + is already authenticated. + """ + argv = ["env"] + for key in _CLEAR_ENV: + argv.extend(["-u", key]) + if self.account.provider == "grok": + argv.append(f"GROK_HOME={self.account.config_dir}") + elif self.account.provider == "codex": + argv.append(f"CODEX_HOME={self.account.config_dir}") + else: + raise AccountError(f"Unsupported provider for role {self.name}") + return argv + + +@dataclass(frozen=True) +class AISessionBinding: + interactive: str | None + lanes: dict[str, str] + + +@dataclass(frozen=True) +class AIAccounts: + accounts: dict[str, AIAccount] + roles: dict[str, AIRole] + sessions: dict[str, AISessionBinding] + usage_session: str | None = None + + def for_lane(self, session_id: str, role: str, vendor: str) -> AIRole: + binding = self.sessions.get(session_id) + if binding is None: + raise AccountError(f"No AI session configured for {session_id}") + slot = f"{vendor}:{role}" + role_name = binding.lanes.get(slot) + if role_name is None: + raise AccountError(f"No AI lane configured for session {session_id} slot {slot}") + selected = self.roles[role_name] + if selected.account.provider != vendor: + raise AccountError( + f"Lane slot {slot} provider does not match configured account provider" + ) + if role in READ_ONLY_WORKFLOW_KINDS and selected.access != "read-only": + raise AccountError( + f"Workflow kind {role} requires read-only access; refusing writable binding" + ) + return selected + + def for_session(self, session_id: str) -> AIRole: + binding = self.sessions.get(session_id) + if binding is None: + raise AccountError(f"No AI session configured for {session_id}") + if binding.interactive is None: + raise AccountError(f"No interactive AI role configured for session {session_id}") + return self.roles[binding.interactive] + + +def load_ai_accounts(home: Path) -> AIAccounts: + path = home / CONFIG_FILE + if not path.exists(): + return AIAccounts({}, {}, {}) + try: + raw = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, ValueError) as exc: + raise AccountError(f"Cannot read {CONFIG_FILE}") from exc + if not isinstance(raw, dict) or set(raw) - {"accounts", "roles", "sessions", "usage_session"}: + raise AccountError(f"{CONFIG_FILE} accepts only accounts, roles, sessions, and usage_session") + accounts_raw = raw.get("accounts") + roles_raw = raw.get("roles") + sessions_raw = raw.get("sessions") + if accounts_raw is None: + accounts_raw = {} + if roles_raw is None: + roles_raw = {} + if sessions_raw is None: + sessions_raw = {} + if ( + not isinstance(accounts_raw, dict) + or not isinstance(roles_raw, dict) + or not isinstance(sessions_raw, dict) + ): + raise AccountError("accounts, roles, and sessions must be objects or null") + + accounts: dict[str, AIAccount] = {} + for name, entry in accounts_raw.items(): + _text(name, "account name") + if not isinstance(entry, dict) or set(entry) - {"provider", "config_dir"}: + raise AccountError(f"Invalid account fields for {name}") + provider = _text(entry.get("provider"), "provider") + if provider not in SUPPORTED_PROVIDERS: + raise AccountError(f"Unsupported provider for account {name}") + config_dir = _config_dir(entry.get("config_dir"), "config_dir") + accounts[name] = AIAccount(name, provider, config_dir) + + roles: dict[str, AIRole] = {} + for name, entry in roles_raw.items(): + _text(name, "role name") + if not isinstance(entry, dict) or set(entry) - {"account", "model", "access"}: + raise AccountError(f"Invalid role fields for {name}") + account_name = _text(entry.get("account"), "role account") + if account_name not in accounts: + raise AccountError(f"Role {name} references an unknown account") + model = _text(entry.get("model"), "model") + access = _text(entry.get("access"), "access") + if access not in ACCESS_VALUES: + raise AccountError(f"Invalid access for role {name}") + roles[name] = AIRole(name, accounts[account_name], model, access) + + sessions: dict[str, AISessionBinding] = {} + for session_id, entry in sessions_raw.items(): + _text(session_id, "session id") + if not isinstance(entry, dict) or set(entry) - {"interactive", "lanes"}: + raise AccountError(f"Invalid session fields for {session_id}") + interactive_raw = entry.get("interactive") + if interactive_raw is None: + interactive: str | None = None + else: + interactive = _text(interactive_raw, "session interactive") + if interactive not in roles: + raise AccountError( + f"Session {session_id} interactive references an unknown role" + ) + lanes_raw = entry.get("lanes") + if lanes_raw is None: + lanes_raw = {} + if not isinstance(lanes_raw, dict): + raise AccountError(f"Session {session_id} lanes must be an object or null") + lanes: dict[str, str] = {} + for slot, role_name in lanes_raw.items(): + _text(slot, "lane slot") + bound = _text(role_name, "lane role") + if bound not in roles: + raise AccountError( + f"Session {session_id} lane {slot} references an unknown role" + ) + lanes[slot] = bound + sessions[session_id] = AISessionBinding(interactive, lanes) + + usage_session = raw.get("usage_session") + if usage_session is not None: + usage_session = _text(usage_session, "usage_session") + if usage_session not in sessions or sessions[usage_session].interactive is None: + raise AccountError("usage_session requires a configured interactive session") + selected = roles[sessions[usage_session].interactive] + if selected.account.provider != "grok": + raise AccountError("usage_session currently supports the Grok billing adapter only") + return AIAccounts(accounts, roles, sessions, usage_session) diff --git a/src/agent_cli/lane.py b/src/agent_cli/lane.py index 0e46ee1..44fa9fe 100644 --- a/src/agent_cli/lane.py +++ b/src/agent_cli/lane.py @@ -15,9 +15,6 @@ LANE_ROLES = ("implementer", "reviewer", "pr-reviewer-quality", "pr-reviewer-logic") LANE_VENDORS = ("grok", "codex") -WRITE_ROLES = frozenset({"implementer"}) -GROK_LANE_MODEL = "grok-4.5" -CODEX_LANE_MODEL = "gpt-5.6-sol" NPROC_CAP = 800 GROK_STRIP_ENV = ("ANTHROPIC_API_KEY", "CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT") STATUS_VALUES = ("complete", "partial", "timeout", "unavailable") @@ -51,9 +48,9 @@ def _env_strip_prefix() -> list[str]: return argv -def grok_argv(*, spec_file: str, cwd: str, write: bool) -> list[str]: +def grok_argv(*, spec_file: str, cwd: str, write: bool, model: str) -> list[str]: argv = _env_strip_prefix() - argv.extend(["grok", "--prompt-file", spec_file, "-m", GROK_LANE_MODEL]) + argv.extend(["grok", "--prompt-file", spec_file, "-m", model]) if write: argv.extend( [ @@ -95,7 +92,7 @@ def grok_argv(*, spec_file: str, cwd: str, write: bool) -> list[str]: return argv -def codex_argv(*, cwd: str, write: bool, output_file: str) -> list[str]: +def codex_argv(*, cwd: str, write: bool, output_file: str, model: str) -> list[str]: sandbox = "workspace-write" if write else "read-only" argv = _env_strip_prefix() argv.extend( @@ -103,9 +100,7 @@ def codex_argv(*, cwd: str, write: bool, output_file: str) -> list[str]: "codex", "exec", "--model", - CODEX_LANE_MODEL, - "-c", - "model_reasoning_effort=high", + model, "--sandbox", sandbox, "--skip-git-repo-check", @@ -226,11 +221,17 @@ def launch( runner: Runner | None = None, dry_run: bool = False, tmux: bool = True, + config_home: Path | None = None, + session_id: str | None = None, ) -> LaneResult: if role not in LANE_ROLES: raise SystemExit(f"role must be {'|'.join(LANE_ROLES)}") if vendor not in LANE_VENDORS: raise SystemExit("vendor must be grok|codex") + from .ai_accounts import load_ai_accounts + if not session_id or config_home is None: + raise SystemExit("lane requires an explicit session and AI configuration home") + selected = load_ai_accounts(config_home).for_lane(session_id, role, vendor) path = Path(spec_file) if not path.is_file(): @@ -241,16 +242,17 @@ def launch( spec_file = str(path.resolve()) cwd = str(Path(cwd).resolve()) - write = role in WRITE_ROLES + write = selected.access == "workspace-write" codex_output_file: str | None = None if vendor == "grok": - argv = grok_argv(spec_file=spec_file, cwd=cwd, write=write) + argv = grok_argv(spec_file=spec_file, cwd=cwd, write=write, model=selected.model) else: if dry_run: argv = codex_argv( cwd=cwd, write=write, output_file="/tmp/agent-lane-codex-dry-run.txt", + model=selected.model, ) else: fd, codex_output_file = tempfile.mkstemp( @@ -258,7 +260,11 @@ def launch( suffix=".txt", ) os.close(fd) - argv = codex_argv(cwd=cwd, write=write, output_file=codex_output_file) + argv = codex_argv(cwd=cwd, write=write, output_file=codex_output_file, model=selected.model) + + # Apply the selected provider home inside the tmux child too. The builders' + # credential-clearing prefix is nested, never a process-global mutation. + argv = [*selected.env_prefix(), *argv] tmux_session: str | None = None if tmux: diff --git a/src/agent_cli/main.py b/src/agent_cli/main.py index e2d7652..6242f9e 100644 --- a/src/agent_cli/main.py +++ b/src/agent_cli/main.py @@ -3,6 +3,7 @@ from __future__ import annotations import base64 +import hashlib import json import os import re @@ -1931,8 +1932,6 @@ def _session_start( _require_owned(store, row, "session") if row.get("status") != "active": die(f"session {sid} is not active") - if not runtime.available(sid): - die("tmux is not installed") if provider is not None and provider != "grok": die("provider must be grok") if provider == "grok" and command: @@ -1945,18 +1944,40 @@ def _session_start( command_argv: list[str] | None = None start_command = command if provider == "grok": + from .ai_accounts import load_ai_accounts + selected = load_ai_accounts(store.home).for_session(sid) + if selected.account.provider != provider: + die("configured interactive role does not match the requested provider") + if model is not None and model != selected.model: + die("--model does not match the configured interactive role") + binding = { + "role": selected.name, "account": selected.account.name, + "provider": selected.account.provider, "model": selected.model, + "access": selected.access, + "configuration": hashlib.sha256(selected.account.config_dir.encode()).hexdigest(), + } existing = meta.get("grok_session_id") existing_s = existing if isinstance(existing, str) and existing else "" + if existing_s and meta.get("ai_binding") != binding: + die("interactive AI binding changed or is missing; configure a new session instead of resuming") + if not runtime.available(sid): + die("tmux is not installed") if runtime.exists(sid) and not existing_s: runtime.stop(sid) new_id = grok_new_session_id() if not existing_s else "" - resolved = grok_model(model) - command_argv = grok_tmux_command_argv(existing=existing_s, model=resolved, new_id=new_id) + resolved = grok_model(selected.model) + command_argv = [*selected.env_prefix(), *grok_tmux_command_argv( + existing=existing_s, model=resolved, new_id=new_id, + )] + if selected.access == "read-only": + command_argv.extend(["--deny", "Write", "--deny", "Edit"]) + command_argv.extend(["--deny", "Bash", "--no-subagents", "--disable-web-search"]) start_command = None if not existing_s: meta["grok_session_id"] = new_id meta["provider"] = "grok" meta["model"] = resolved + meta["ai_binding"] = binding runtime.start(sid, start_command, cols, rows, command_argv=command_argv, cwd=cwd) meta["tmux_session"] = name meta["control"] = "attached" @@ -2762,6 +2783,11 @@ def cmd_run(args: list[str]) -> None: round_num: int | None = None if role in ("implementer", "reviewer"): round_num = current_round + # Fail closed on AI lane binding before creating a working agent. + # launch() still resolves the same binding; no ambient fallback. + from .ai_accounts import load_ai_accounts + + load_ai_accounts(store.home).for_lane(session_id, role, vendor) working = _find_working_agent( store, tid, role=role, vendor=vendor, round_num=round_num ) @@ -2786,6 +2812,8 @@ def cmd_run(args: list[str]) -> None: spec_file=spec_file, cwd=cwd, tmux=tmux, + config_home=store.home, + session_id=session_id, ) _print_lane_result(result) if role == "implementer" and result.status == "complete": @@ -2999,11 +3027,12 @@ def cmd_lane(args: list[str]) -> None: if not args or args[0] != "run": die( "Usage: agent lane run --role ROLE --vendor grok|codex " - "--spec-file PATH [--cwd PATH] [--dry-run] [--no-tmux]" + "--session ID --spec-file PATH [--cwd PATH] [--dry-run] [--no-tmux]" ) rest = args[1:] role = require_flag(rest, "--role") vendor = require_flag(rest, "--vendor") + session_id = require_flag(rest, "--session") spec_file = require_flag(rest, "--spec-file") cwd = flag(rest, "--cwd") or os.getcwd() dry_run = "--dry-run" in rest @@ -3019,6 +3048,8 @@ def cmd_lane(args: list[str]) -> None: cwd=cwd, dry_run=dry_run, tmux=tmux, + config_home=home(), + session_id=session_id, ) if dry_run: print(" ".join(result.argv)) diff --git a/src/agent_cli/runtime.py b/src/agent_cli/runtime.py index 656a73e..4c0899a 100644 --- a/src/agent_cli/runtime.py +++ b/src/agent_cli/runtime.py @@ -16,7 +16,6 @@ TARGETS_FILE = "runtime-targets.json" -GROK_DEFAULT_MODEL = "grok-4.6" GROK_STRIP_ENV = ("ANTHROPIC_API_KEY", "CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT") _UUID_RE = re.compile( r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$" @@ -43,11 +42,11 @@ def grok_new_session_id() -> str: def grok_model(raw: str | None) -> str: - if raw is None: - return GROK_DEFAULT_MODEL + if not isinstance(raw, str): + raise StoreError("Grok model must be explicitly configured") stripped = raw.strip() if stripped == "": - return GROK_DEFAULT_MODEL + raise StoreError("Grok model must be explicitly configured") return stripped diff --git a/src/agent_cli/usage.py b/src/agent_cli/usage.py index 422eb92..d195208 100644 --- a/src/agent_cli/usage.py +++ b/src/agent_cli/usage.py @@ -4,7 +4,6 @@ import json import math -import os import uuid from collections.abc import Callable from datetime import datetime, timezone @@ -42,14 +41,6 @@ class AuthStale(StoreError): """Grok login missing or expired. Not a runtime failure of this scan.""" -def grok_auth_path() -> Path: - """$GROK_HOME/auth.json if GROK_HOME is set and non-empty, else ~/.grok/auth.json.""" - home = os.environ.get("GROK_HOME") - if isinstance(home, str) and home != "": - return Path(home) / "auth.json" - return Path.home() / ".grok" / "auth.json" - - def load_grok_bearer(auth_path: Path | None = None) -> tuple[str, str]: """ Read the SuperGrok OIDC entry. @@ -61,7 +52,9 @@ def load_grok_bearer(auth_path: Path | None = None) -> tuple[str, str]: Return (token, account_email). Never log the token. Do not refresh tokens. Do not read refresh_token except to ignore it. """ - path = auth_path if auth_path is not None else grok_auth_path() + if auth_path is None: + raise AuthStale("Grok authentication path must be explicitly configured") + path = auth_path if not path.is_file(): raise AuthStale(f"grok auth file not found: {path}") try: @@ -314,7 +307,7 @@ def scan_usage( now: Callable[[], str] | None = None, ) -> str | None: """ - Load bearer, call fetch or fetch_credits_and_settings, parse, pick session. + Resolve an explicit usage binding (or injected auth_path), fetch and record. If usage_unchanged(last, snapshot): return None. Else insert activity via store.write_with_advisory: lock_key='usage.snapshot:grok' @@ -326,11 +319,26 @@ def scan_usage( Raise StoreError on any failure. Never invent 0% on error. fetched_at = (now or utcnow)(). """ + session_id = None + if auth_path is None: + from .ai_accounts import load_ai_accounts + accounts = load_ai_accounts(store.home) + session_id = accounts.usage_session + if session_id is None: + raise AuthStale("AI usage monitoring is not configured") + row = store.row("session", session_id) + if row is None or row.get("_origin_device_id") != store.device_id() or row.get("status") != "active": + raise StoreError("AI usage session must be owned and active") + selected = accounts.for_session(session_id) + auth_path = Path(selected.account.config_dir) / "auth.json" token, account_email = load_grok_bearer(auth_path) credits, settings = (fetch or fetch_credits_and_settings)(token) fetched_at = (now or utcnow)() snapshot = snapshot_from_payloads(credits, settings, fetched_at, account_email) - session_id = pick_session_id(store) + # Explicit auth_path is an injection hook for callers supplying their own + # account path; automatic polling always uses the configured usage session. + if session_id is None: + session_id = pick_session_id(store) if usage_unchanged(last_usage_snapshot(store), snapshot): return None activity_id = str(uuid.uuid4()) diff --git a/tests/test_a38.py b/tests/test_a38.py index 74ca7d5..b8f4937 100644 --- a/tests/test_a38.py +++ b/tests/test_a38.py @@ -497,21 +497,32 @@ def test_fork_report_targets_base_repository(self) -> None: root = Path(tmp) repo = root / "repo" head = _init_repo(repo, origin="https://github.com/contributor/fork.git") + (root / 'github-accounts.json').write_text(json.dumps({ + 'accounts': {'configured': {'login': 'ExampleWorker', 'gh_config_dir': '/test/github'}}, + 'sessions': {'visibility': 'configured'}, + })) calls = [] def lookup(argv, cwd, env): - if argv[0] == "gh": - calls.append(argv) - return subprocess.CompletedProcess(argv, 0, '{"isPrivate":true}', "") + if argv[0] == 'env': + self.assertIn('GH_CONFIG_DIR=/test/github', argv) + command = argv[argv.index('gh'):] + calls.append(command) + output = 'ExampleWorker' if command[:3] == ['gh', 'api', 'user'] else '{"isPrivate":true}' + return subprocess.CompletedProcess(argv, 0, output, "") return subprocess.run(argv, cwd=cwd, env=env, text=True, capture_output=True) output = root / "report.md" verdict = run_policy( repo, _policy_dict(), output=output, logs_dir=root / "logs", base_sha=head, repository="example/app", run=lookup, + github_session='visibility', config_home=root, ) self.assertTrue(verdict["ok"]) - self.assertEqual(calls, [["gh", "repo", "view", "example/app", "--json", "isPrivate"]]) + self.assertEqual(calls, [ + ['gh', 'api', 'user', '--jq', '.login'], + ["gh", "repo", "view", "example/app", "--json", "isPrivate"], + ]) self.assertTrue(output.read_text().startswith("EN:\n")) self.assertIn("
\nDetails\n\n", output.read_text()) report = parse_comment(output.read_text()) @@ -1143,3 +1154,36 @@ def stat_mode(path: Path) -> int: if __name__ == "__main__": unittest.main() + + +@pytest.mark.parametrize('session', [None, 'unbound']) +def test_visibility_lookup_refuses_missing_account_before_network(tmp_path, monkeypatch, session): + from agent_cli.a38 import _resolve_private + monkeypatch.setenv('GH_TOKEN', 'synthetic-ambient-token') + def forbidden(*args): + pytest.fail('Unconfigured visibility lookup must not access GitHub') + with pytest.raises(A38Error): + _resolve_private(tmp_path, None, run=forbidden, repository='example/app', + github_session=session, config_home=tmp_path) + assert _resolve_private(tmp_path, False, run=forbidden, repository='example/app') is False + + +@pytest.mark.parametrize('auth_result', ['WrongWorker', None]) +def test_visibility_lookup_rejects_wrong_or_unavailable_account(tmp_path, auth_result): + from agent_cli.a38 import _resolve_private + (tmp_path / 'github-accounts.json').write_text(json.dumps({ + 'accounts': {'selected': {'login': 'ExpectedWorker', 'gh_config_dir': '/test/selected'}}, + 'sessions': {'chosen': 'selected'}, + })) + calls = [] + def runner(argv, cwd, env): + command = argv[argv.index('gh'):] + calls.append(command) + assert command == ['gh', 'api', 'user', '--jq', '.login'] + return subprocess.CompletedProcess(argv, 1 if auth_result is None else 0, + auth_result or '', 'synthetic-private-error') + with pytest.raises(A38Error) as exc: + _resolve_private(tmp_path, None, run=runner, repository='example/app', + github_session='chosen', config_home=tmp_path) + assert len(calls) == 1 + assert 'synthetic-private-error' not in str(exc.value) diff --git a/tests/test_ai_accounts.py b/tests/test_ai_accounts.py new file mode 100644 index 0000000..b5e3d46 --- /dev/null +++ b/tests/test_ai_accounts.py @@ -0,0 +1,383 @@ +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from agent_cli.ai_accounts import ( + ACCESS_VALUES, + READ_ONLY_WORKFLOW_KINDS, + SUPPORTED_PROVIDERS, + AccountError, + AIAccount, + AIAccounts, + AIRole, + load_ai_accounts, +) + +pytestmark = pytest.mark.no_pg + + +def write_config(home: Path, data: dict) -> None: + (home / "ai-accounts.json").write_text(json.dumps(data), encoding="utf-8") + + +def sample_config() -> dict: + return { + "accounts": { + "grok-primary": { + "provider": "grok", + "config_dir": "/operator/path/grok-primary", + }, + "codex-primary": { + "provider": "codex", + "config_dir": "/operator/path/codex-primary", + }, + "grok-secondary": { + "provider": "grok", + "config_dir": "/operator/path/grok-secondary", + }, + }, + "roles": { + "builder": { + "account": "grok-primary", + "model": "operator-selected-model", + "access": "workspace-write", + }, + "reader": { + "account": "grok-primary", + "model": "operator-selected-model", + "access": "read-only", + }, + "codex-builder": { + "account": "codex-primary", + "model": "operator-codex-model", + "access": "workspace-write", + }, + "codex-reader": { + "account": "codex-primary", + "model": "operator-codex-model", + "access": "read-only", + }, + "alt-builder": { + "account": "grok-secondary", + "model": "alt-model", + "access": "workspace-write", + }, + }, + "sessions": { + "chosen-session": { + "interactive": "builder", + "lanes": { + "grok:implementer": "builder", + "grok:reviewer": "reader", + "grok:pr-reviewer-quality": "reader", + "grok:pr-reviewer-logic": "reader", + "codex:implementer": "codex-builder", + "codex:pr-reviewer-quality": "codex-reader", + "codex:pr-reviewer-logic": "codex-reader", + }, + }, + "lanes-only": { + "lanes": { + "grok:implementer": "alt-builder", + }, + }, + }, + } + + +@pytest.mark.parametrize( + "data", + [ + None, + {}, + {"accounts": None, "roles": None, "sessions": None}, + {"accounts": {}, "roles": {}, "sessions": {}}, + ], +) +def test_installation_defaults_to_empty(tmp_path: Path, data) -> None: + if data is not None: + write_config(tmp_path, data) + accounts = load_ai_accounts(tmp_path) + assert accounts.accounts == {} + assert accounts.roles == {} + assert accounts.sessions == {} + with pytest.raises(AccountError, match="No AI session configured"): + accounts.for_session("chosen-session") + with pytest.raises(AccountError, match="No AI session configured"): + accounts.for_lane("chosen-session", "implementer", "grok") + + +def test_many_profiles_and_roles_load_without_fixed_count(tmp_path: Path) -> None: + write_config(tmp_path, sample_config()) + loaded = load_ai_accounts(tmp_path) + assert set(loaded.accounts) == {"grok-primary", "codex-primary", "grok-secondary"} + assert set(loaded.roles) == { + "builder", + "reader", + "codex-builder", + "codex-reader", + "alt-builder", + } + assert loaded.accounts["grok-primary"].provider == "grok" + assert loaded.accounts["codex-primary"].config_dir == "/operator/path/codex-primary" + assert loaded.roles["builder"].access == "workspace-write" + assert loaded.roles["reader"].account is loaded.accounts["grok-primary"] + assert SUPPORTED_PROVIDERS == frozenset({"grok", "codex"}) + assert ACCESS_VALUES == frozenset({"read-only", "workspace-write"}) + + +def test_explicit_session_and_lane_resolution(tmp_path: Path) -> None: + write_config(tmp_path, sample_config()) + loaded = load_ai_accounts(tmp_path) + interactive = loaded.for_session("chosen-session") + assert interactive.name == "builder" + assert interactive.account.provider == "grok" + implementer = loaded.for_lane("chosen-session", "implementer", "grok") + assert implementer.name == "builder" + assert implementer.access == "workspace-write" + reviewer = loaded.for_lane("chosen-session", "reviewer", "grok") + assert reviewer.name == "reader" + assert reviewer.access == "read-only" + alt = loaded.for_lane("lanes-only", "implementer", "grok") + assert alt.name == "alt-builder" + with pytest.raises(AccountError, match="No interactive AI role"): + loaded.for_session("lanes-only") + with pytest.raises(AccountError, match="No AI lane configured"): + loaded.for_lane("lanes-only", "reviewer", "grok") + + +def test_session_omitted_interactive_and_empty_lanes(tmp_path: Path) -> None: + data = sample_config() + data["sessions"]["optional"] = {"interactive": None, "lanes": None} + data["sessions"]["empty-lanes"] = {"lanes": {}} + write_config(tmp_path, data) + loaded = load_ai_accounts(tmp_path) + assert loaded.sessions["optional"].interactive is None + assert loaded.sessions["optional"].lanes == {} + assert loaded.sessions["empty-lanes"].lanes == {} + with pytest.raises(AccountError, match="No interactive AI role"): + loaded.for_session("optional") + + +@pytest.mark.parametrize("kind", sorted(READ_ONLY_WORKFLOW_KINDS)) +def test_read_only_workflow_kinds_reject_writable_bindings( + tmp_path: Path, kind: str +) -> None: + data = sample_config() + data["sessions"]["chosen-session"]["lanes"][f"grok:{kind}"] = "builder" + write_config(tmp_path, data) + loaded = load_ai_accounts(tmp_path) + with pytest.raises(AccountError, match="requires read-only access"): + loaded.for_lane("chosen-session", kind, "grok") + + +def test_wrong_provider_on_lane_resolution_fails(tmp_path: Path) -> None: + data = sample_config() + data["sessions"]["chosen-session"]["lanes"]["grok:implementer"] = "codex-builder" + write_config(tmp_path, data) + loaded = load_ai_accounts(tmp_path) + with pytest.raises(AccountError, match="provider does not match"): + loaded.for_lane("chosen-session", "implementer", "grok") + + +def test_env_prefix_clears_tokens_without_mutating_process( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_config(tmp_path, sample_config()) + loaded = load_ai_accounts(tmp_path) + monkeypatch.setenv("XAI_API_KEY", "parent-xai") + monkeypatch.setenv("GROK_API_KEY", "parent-grok") + monkeypatch.setenv("OPENAI_API_KEY", "parent-openai") + monkeypatch.setenv("CODEX_API_KEY", "parent-codex") + monkeypatch.setenv("ANTHROPIC_API_KEY", "parent-anthropic") + monkeypatch.setenv("CLAUDECODE", "1") + monkeypatch.setenv("CLAUDE_CODE_ENTRYPOINT", "cli") + monkeypatch.setenv("GROK_HOME", "/ambient/grok") + monkeypatch.setenv("CODEX_HOME", "/ambient/codex") + monkeypatch.setenv("UNRELATED_KEEP", "keep-me") + + grok_prefix = loaded.roles["builder"].env_prefix() + assert grok_prefix[0] == "env" + for key in ( + "XAI_API_KEY", + "GROK_API_KEY", + "OPENAI_API_KEY", + "CODEX_API_KEY", + "ANTHROPIC_API_KEY", + "CLAUDECODE", + "CLAUDE_CODE_ENTRYPOINT", + "GROK_HOME", + "CODEX_HOME", + ): + assert key in grok_prefix + assert grok_prefix[grok_prefix.index(key) - 1] == "-u" + assert "GROK_HOME=/operator/path/grok-primary" in grok_prefix + assert not any(part.startswith("CODEX_HOME=") for part in grok_prefix) + + codex_prefix = loaded.roles["codex-builder"].env_prefix() + assert "CODEX_HOME=/operator/path/codex-primary" in codex_prefix + assert not any(part.startswith("GROK_HOME=") for part in codex_prefix) + + assert os.environ["XAI_API_KEY"] == "parent-xai" + assert os.environ["GROK_API_KEY"] == "parent-grok" + assert os.environ["OPENAI_API_KEY"] == "parent-openai" + assert os.environ["CODEX_API_KEY"] == "parent-codex" + assert os.environ["ANTHROPIC_API_KEY"] == "parent-anthropic" + assert os.environ["CLAUDECODE"] == "1" + assert os.environ["CLAUDE_CODE_ENTRYPOINT"] == "cli" + assert os.environ["GROK_HOME"] == "/ambient/grok" + assert os.environ["CODEX_HOME"] == "/ambient/codex" + assert os.environ["UNRELATED_KEEP"] == "keep-me" + + +@pytest.mark.parametrize( + "change", + [ + "unknown-top-level", + "accounts-not-object", + "roles-not-object", + "sessions-not-object", + "account-unknown-field", + "role-unknown-field", + "session-unknown-field", + "missing-provider", + "missing-config-dir", + "missing-role-account", + "missing-model", + "missing-access", + "unsupported-provider", + "invalid-access", + "relative-config-dir", + "parent-traversal", + "config-dir-newline", + "config-dir-cr", + "config-dir-nul", + "empty-account-name", + "empty-role-name", + "empty-session-id", + "empty-lane-slot", + "dangling-role-account", + "dangling-interactive", + "dangling-lane-role", + "lanes-not-object", + "token-field", + "invalid-json", + ], +) +def test_malformed_configuration_is_rejected(tmp_path: Path, change: str) -> None: + data = sample_config() + if change == "unknown-top-level": + data["extra"] = {} + elif change == "accounts-not-object": + data["accounts"] = [] + elif change == "roles-not-object": + data["roles"] = "nope" + elif change == "sessions-not-object": + data["sessions"] = 1 + elif change == "account-unknown-field": + data["accounts"]["grok-primary"]["token"] = "not-allowed" + elif change == "role-unknown-field": + data["roles"]["builder"]["temperature"] = 0.2 + elif change == "session-unknown-field": + data["sessions"]["chosen-session"]["default"] = "builder" + elif change == "missing-provider": + del data["accounts"]["grok-primary"]["provider"] + elif change == "missing-config-dir": + del data["accounts"]["grok-primary"]["config_dir"] + elif change == "missing-role-account": + del data["roles"]["builder"]["account"] + elif change == "missing-model": + del data["roles"]["builder"]["model"] + elif change == "missing-access": + del data["roles"]["builder"]["access"] + elif change == "unsupported-provider": + data["accounts"]["grok-primary"]["provider"] = "claude" + elif change == "invalid-access": + data["roles"]["builder"]["access"] = "full" + elif change == "relative-config-dir": + data["accounts"]["grok-primary"]["config_dir"] = "./relative" + elif change == "parent-traversal": + data["accounts"]["grok-primary"]["config_dir"] = "/operator/path/../secret" + elif change == "config-dir-newline": + data["accounts"]["grok-primary"]["config_dir"] = "/operator/path\n/other" + elif change == "config-dir-cr": + data["accounts"]["grok-primary"]["config_dir"] = "/operator/path\r/other" + elif change == "config-dir-nul": + data["accounts"]["grok-primary"]["config_dir"] = "/operator/path\x00/other" + elif change == "empty-account-name": + data["accounts"][""] = { + "provider": "grok", + "config_dir": "/operator/path/empty", + } + elif change == "empty-role-name": + data["roles"][" "] = { + "account": "grok-primary", + "model": "m", + "access": "read-only", + } + elif change == "empty-session-id": + data["sessions"][""] = {"interactive": "builder"} + elif change == "empty-lane-slot": + data["sessions"]["chosen-session"]["lanes"][""] = "builder" + elif change == "dangling-role-account": + data["roles"]["builder"]["account"] = "missing" + elif change == "dangling-interactive": + data["sessions"]["chosen-session"]["interactive"] = "missing" + elif change == "dangling-lane-role": + data["sessions"]["chosen-session"]["lanes"]["grok:implementer"] = "missing" + elif change == "lanes-not-object": + data["sessions"]["chosen-session"]["lanes"] = ["builder"] + elif change == "token-field": + data["accounts"]["grok-primary"]["api_key"] = "secret-must-not-load" + if change == "invalid-json": + (tmp_path / "ai-accounts.json").write_text("{", encoding="utf-8") + else: + write_config(tmp_path, data) + with pytest.raises(AccountError) as excinfo: + load_ai_accounts(tmp_path) + message = str(excinfo.value) + assert "secret-must-not-load" not in message + assert "parent-xai" not in message + + +def test_errors_do_not_echo_credential_like_values(tmp_path: Path) -> None: + secret = "sk-live-should-never-appear" + write_config( + tmp_path, + { + "accounts": { + "probe": { + "provider": "grok", + "config_dir": "/operator/path", + "api_key": secret, + } + }, + "roles": {}, + "sessions": {}, + }, + ) + with pytest.raises(AccountError) as excinfo: + load_ai_accounts(tmp_path) + assert secret not in str(excinfo.value) + + +def test_account_error_extends_store_error() -> None: + from agent_cli.store import StoreError + + assert issubclass(AccountError, StoreError) + assert issubclass(AccountError, SystemExit) + + +def test_frozen_dataclasses_expose_required_fields() -> None: + account = AIAccount("n", "grok", "/absolute/config") + role = AIRole("r", account, "model-x", "read-only") + accounts = AIAccounts({"n": account}, {"r": role}, {}) + assert account.name == "n" + assert role.model == "model-x" + assert accounts.roles["r"].access == "read-only" + with pytest.raises(Exception): + account.name = "other" # type: ignore[misc] diff --git a/tests/test_ai_usage_binding.py b/tests/test_ai_usage_binding.py new file mode 100644 index 0000000..bc31304 --- /dev/null +++ b/tests/test_ai_usage_binding.py @@ -0,0 +1,58 @@ +import json + +import pytest + +from agent_cli.ai_accounts import AccountError, load_ai_accounts +from agent_cli.store import Store +from agent_cli.usage import AuthStale, load_grok_bearer, scan_usage +from test_usage import SETTINGS, _auth_file, _credits, _owned_grok_session + + +def test_automatic_usage_does_not_read_ambient_account(tmp_path, monkeypatch): + monkeypatch.setenv('GROK_HOME', str(tmp_path)) + _auth_file(tmp_path) + store = Store(tmp_path) + try: + _owned_grok_session(store) + def forbidden(token): + pytest.fail('Unconfigured usage must not contact a provider') + with pytest.raises(AuthStale, match='not configured'): + scan_usage(store, fetch=forbidden) + with pytest.raises(AuthStale, match='explicitly configured'): + load_grok_bearer() + finally: + store.close() + + +def test_usage_uses_explicit_session_account_instead_of_newest_session(tmp_path, monkeypatch): + chosen = tmp_path / 'chosen' + chosen.mkdir() + _auth_file(chosen, email='chosen@example.com') + ambient = tmp_path / 'ambient' + ambient.mkdir() + _auth_file(ambient, email='ambient@example.com') + monkeypatch.setenv('GROK_HOME', str(ambient)) + (tmp_path / 'ai-accounts.json').write_text(json.dumps({ + 'accounts': {'chosen': {'provider': 'grok', 'config_dir': str(chosen)}}, + 'roles': {'configured': {'account': 'chosen', 'model': 'explicit-model', 'access': 'read-only'}}, + 'sessions': {'chosen-session': {'interactive': 'configured'}}, + 'usage_session': 'chosen-session', + })) + store = Store(tmp_path) + try: + _owned_grok_session(store, 'chosen-session') + _owned_grok_session(store, 'newer-unselected-session') + aid = scan_usage(store, fetch=lambda token: (_credits(), SETTINGS)) + row = store.row('activity', aid) + assert row['session_id'] == 'chosen-session' + assert row['payload']['account_email'] == 'chosen@example.com' + finally: + store.close() + + +@pytest.mark.no_pg +@pytest.mark.parametrize('value', ['', 'missing', False, 42]) +def test_invalid_usage_selection_is_not_replaced_with_another_account(tmp_path, value): + (tmp_path / 'ai-accounts.json').write_text(json.dumps({'usage_session': value})) + with pytest.raises(AccountError): + load_ai_accounts(tmp_path) diff --git a/tests/test_control_cli.py b/tests/test_control_cli.py index bc13014..f2434ed 100644 --- a/tests/test_control_cli.py +++ b/tests/test_control_cli.py @@ -1,5 +1,7 @@ from __future__ import annotations +import hashlib +import json import re from pathlib import Path @@ -10,6 +12,90 @@ from agent_cli.runtime import Completed, Runtime from agent_cli.store import Store +OPERATOR_INTERACTIVE_MODEL = "operator-interactive-model" +OPERATOR_INTERACTIVE_MODEL_B = "operator-interactive-model-b" +OPERATOR_GROK_HOME = "/operator/path/interactive-grok-a" +OPERATOR_GROK_HOME_B = "/operator/path/interactive-grok-b" +SESSION_A = "sess-1" +SESSION_B = "sess-2" + + +def write_operator_interactive_ai_accounts( + home: Path, + *, + sessions: dict | None = None, + model: str = OPERATOR_INTERACTIVE_MODEL, + config_dir: str = OPERATOR_GROK_HOME, + account_name: str = "interactive-grok", + role_name: str = "interactive-builder", + access: str = "workspace-write", +) -> None: + """Write operator-supplied ai-accounts.json for positive interactive starts. + + Not installed by default. Call explicitly for positive grok provider cases; + raw-shell starts (provider=None) must not rely on this helper. + """ + data = { + "accounts": { + account_name: {"provider": "grok", "config_dir": config_dir}, + "interactive-grok-b": { + "provider": "grok", + "config_dir": OPERATOR_GROK_HOME_B, + }, + "interactive-codex": { + "provider": "codex", + "config_dir": "/operator/path/interactive-codex", + }, + }, + "roles": { + role_name: { + "account": account_name, + "model": model, + "access": access, + }, + "interactive-builder-b": { + "account": "interactive-grok-b", + "model": OPERATOR_INTERACTIVE_MODEL_B, + "access": "workspace-write", + }, + "interactive-codex-role": { + "account": "interactive-codex", + "model": "operator-codex-model", + "access": "workspace-write", + }, + "interactive-reader": { + "account": account_name, + "model": model, + "access": "read-only", + }, + }, + "sessions": sessions + or { + SESSION_A: {"interactive": role_name, "lanes": {}}, + SESSION_B: {"interactive": "interactive-builder-b", "lanes": {}}, + "s1": {"interactive": role_name, "lanes": {}}, + }, + } + (home / "ai-accounts.json").write_text(json.dumps(data), encoding="utf-8") + + +def expected_ai_binding( + *, + role: str = "interactive-builder", + account: str = "interactive-grok", + model: str = OPERATOR_INTERACTIVE_MODEL, + config_dir: str = OPERATOR_GROK_HOME, + access: str = "workspace-write", +) -> dict: + return { + "role": role, + "account": account, + "provider": "grok", + "model": model, + "access": access, + "configuration": hashlib.sha256(config_dir.encode()).hexdigest(), + } + def run(home: Path, argv: list[str]) -> None: import os @@ -42,6 +128,7 @@ def factory(*_a: object, **_k: object) -> Runtime: def test_cli_start_provider_grok_mints_uuid_and_resumes( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: + write_operator_interactive_ai_accounts(tmp_path) calls: list[list[str]] = [] def runner(argv: list[str]) -> Completed: @@ -58,37 +145,49 @@ def runner(argv: list[str]) -> Completed: monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) run(tmp_path, ["init"]) - run(tmp_path, ["session", "register", "--id", "sess-1", "--kind", "human"]) - run(tmp_path, ["session", "start", "--id", "sess-1", "--provider", "grok"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) out = capsys.readouterr().out store = Store(tmp_path) try: - row = store.row("session", "sess-1") + row = store.row("session", SESSION_A) assert row is not None gid = row["runtime"]["grok_session_id"] assert re.fullmatch(r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", gid) assert f"grok={gid}" in out + assert row["runtime"]["ai_binding"] == expected_ai_binding() + assert row["runtime"]["model"] == OPERATOR_INTERACTIVE_MODEL first = [c for c in calls if c[:2] == ["tmux", "new-session"]][-1] assert "env" in first assert "ANTHROPIC_API_KEY" in first + assert "XAI_API_KEY" in first + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in first assert "--session-id" in first assert gid in first - assert "--model" in first and "grok-4.6" in first - assert first[first.index("--session-id") + 1] != "sess-1" + assert "--model" in first and OPERATOR_INTERACTIVE_MODEL in first + assert "grok-4.6" not in first + assert first[first.index("--session-id") + 1] != SESSION_A + # Nested role env_prefix precedes the existing grok_tmux_command_argv env strip. + assert first[first.index("--") + 1] == "env" + second_env = first.index("env", first.index("--") + 2) + assert second_env > first.index(f"GROK_HOME={OPERATOR_GROK_HOME}") finally: store.close() - run(tmp_path, ["session", "stop", "--id", "sess-1"]) + run(tmp_path, ["session", "stop", "--id", SESSION_A]) calls.clear() - run(tmp_path, ["session", "start", "--id", "sess-1", "--provider", "grok"]) + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) resume = [c for c in calls if c[:2] == ["tmux", "new-session"]][-1] assert "--resume" in resume assert "--session-id" not in resume + assert OPERATOR_INTERACTIVE_MODEL in resume + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in resume store = Store(tmp_path) try: - row = store.row("session", "sess-1") + row = store.row("session", SESSION_A) assert row is not None assert resume[resume.index("--resume") + 1] == row["runtime"]["grok_session_id"] + assert row["runtime"]["ai_binding"] == expected_ai_binding() finally: store.close() @@ -96,6 +195,7 @@ def runner(argv: list[str]) -> Completed: def test_cli_start_grok_replaces_bare_tmux( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + write_operator_interactive_ai_accounts(tmp_path) calls: list[list[str]] = [] def runner(argv: list[str]) -> Completed: @@ -110,39 +210,47 @@ def runner(argv: list[str]) -> Completed: monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) run(tmp_path, ["init"]) - run(tmp_path, ["session", "register", "--id", "sess-1", "--kind", "human"]) - run(tmp_path, ["session", "start", "--id", "sess-1"]) - run(tmp_path, ["session", "start", "--id", "sess-1", "--provider", "grok"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + # Raw-shell start: provider=None must not require AI accounts. + run(tmp_path, ["session", "start", "--id", SESSION_A]) + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) assert any(c[:2] == ["tmux", "kill-session"] for c in calls) grok_news = [c for c in calls if c[:2] == ["tmux", "new-session"] and "grok" in c] assert len(grok_news) == 1 assert "--session-id" in grok_news[0] + assert OPERATOR_INTERACTIVE_MODEL in grok_news[0] store = Store(tmp_path) try: - gid = store.row("session", "sess-1")["runtime"]["grok_session_id"] + gid = store.row("session", SESSION_A)["runtime"]["grok_session_id"] assert gid in grok_news[0] + assert store.row("session", SESSION_A)["runtime"]["ai_binding"] == expected_ai_binding() finally: store.close() def test_cli_provider_and_cmd_dies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + write_operator_interactive_ai_accounts(tmp_path) factory, _ = _fake_runtime_factory() monkeypatch.setattr(main_mod, "Runtime", factory) run(tmp_path, ["init"]) - run(tmp_path, ["session", "register", "--id", "sess-1", "--kind", "human"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) with pytest.raises(SystemExit, match="cannot be used together"): - run(tmp_path, ["session", "start", "--id", "sess-1", "--provider", "grok", "--cmd", "bash"]) + run( + tmp_path, + ["session", "start", "--id", SESSION_A, "--provider", "grok", "--cmd", "bash"], + ) def test_cli_start_owned_writes_runtime(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: factory, _ = _fake_runtime_factory() monkeypatch.setattr(main_mod, "Runtime", factory) run(tmp_path, ["init"]) - run(tmp_path, ["session", "register", "--id", "sess-1", "--kind", "human"]) - run(tmp_path, ["session", "start", "--id", "sess-1", "--cols", "80", "--rows", "24"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + # Raw-shell path: no AI accounts required. + run(tmp_path, ["session", "start", "--id", SESSION_A, "--cols", "80", "--rows", "24"]) store = Store(tmp_path) try: - row = store.row("session", "sess-1") + row = store.row("session", SESSION_A) assert row is not None rt = row.get("runtime") assert isinstance(rt, dict) @@ -150,6 +258,8 @@ def test_cli_start_owned_writes_runtime(tmp_path: Path, monkeypatch: pytest.Monk assert rt["tmux_session"] == "agent-sess-1" assert rt["cols"] == 80 assert rt["rows"] == 24 + assert "ai_binding" not in rt + assert "grok_session_id" not in rt finally: store.close() @@ -178,6 +288,7 @@ def test_cli_start_foreign_dies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) def test_apply_control_start_stop_input(tmp_path: Path) -> None: + write_operator_interactive_ai_accounts(tmp_path) run(tmp_path, ["init"]) run(tmp_path, ["session", "register", "--id", "s1", "--kind", "human"]) store = Store(tmp_path) @@ -211,14 +322,19 @@ def runner(argv: list[str]) -> Completed: row = store.row("session", "s1") assert row is not None assert row["runtime"]["provider"] == "grok" + assert row["runtime"]["model"] == OPERATOR_INTERACTIVE_MODEL + assert row["runtime"]["ai_binding"] == expected_ai_binding() assert re.fullmatch( r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", row["runtime"]["grok_session_id"], ) grok_new = [c for c in calls if c[:2] == ["tmux", "new-session"]][-1] assert "--session-id" in grok_new - assert "grok-4.6" in grok_new + assert OPERATOR_INTERACTIVE_MODEL in grok_new + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in grok_new + assert "grok-4.6" not in grok_new + # Non-provider restart keeps the existing attached session (raw resize path). ack = apply_control( store, runtime, @@ -255,6 +371,7 @@ def runner(argv: list[str]) -> Completed: def test_apply_control_provider_and_command_not_ok(tmp_path: Path) -> None: + write_operator_interactive_ai_accounts(tmp_path) run(tmp_path, ["init"]) run(tmp_path, ["session", "register", "--id", "s1", "--kind", "human"]) store = Store(tmp_path) @@ -323,3 +440,237 @@ def test_apply_control_bad_quoting_acks_false(tmp_path: Path) -> None: def test_should_sync_on_ws_false_for_control_messages() -> None: for msg_type in ("control", "terminal", "control-ack", "control-ready"): assert should_sync_on_ws({"type": msg_type}) is False + + +def test_missing_interactive_account_refuses_before_process_start( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Genuinely empty AI config: provider=grok must not start tmux/grok.""" + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + with pytest.raises(SystemExit, match="No AI session configured"): + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + assert not any(c[:2] == ["tmux", "new-session"] for c in calls) + + +def test_two_sessions_use_different_interactive_accounts_and_models( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_interactive_ai_accounts(tmp_path) + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + if argv[:2] == ["tmux", "has-session"]: + return Completed(1, "", "") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + run(tmp_path, ["session", "register", "--id", SESSION_B, "--kind", "human"]) + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + run(tmp_path, ["session", "start", "--id", SESSION_B, "--provider", "grok"]) + news = [c for c in calls if c[:2] == ["tmux", "new-session"] and "grok" in c] + assert len(news) == 2 + argv_a, argv_b = news + assert OPERATOR_INTERACTIVE_MODEL in argv_a + assert OPERATOR_INTERACTIVE_MODEL_B in argv_b + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in argv_a + assert f"GROK_HOME={OPERATOR_GROK_HOME_B}" in argv_b + assert OPERATOR_INTERACTIVE_MODEL_B not in argv_a + assert OPERATOR_INTERACTIVE_MODEL not in argv_b + store = Store(tmp_path) + try: + assert store.row("session", SESSION_A)["runtime"]["ai_binding"] == expected_ai_binding() + assert store.row("session", SESSION_B)["runtime"]["ai_binding"] == expected_ai_binding( + role="interactive-builder-b", + account="interactive-grok-b", + model=OPERATOR_INTERACTIVE_MODEL_B, + config_dir=OPERATOR_GROK_HOME_B, + ) + finally: + store.close() + + +@pytest.mark.parametrize( + "mutate", + [ + "model", + "config_dir", + "account", + "role", + ], +) +def test_resume_refuses_account_config_role_or_model_change_before_process_start( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, mutate: str +) -> None: + write_operator_interactive_ai_accounts(tmp_path) + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + if argv[:2] == ["tmux", "has-session"]: + if any(c[:2] == ["tmux", "new-session"] for c in calls[:-1]) and not any( + c[:2] == ["tmux", "kill-session"] for c in calls[:-1] + ): + return Completed(0, "", "") + return Completed(1, "", "") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + run(tmp_path, ["session", "stop", "--id", SESSION_A]) + before = len([c for c in calls if c[:2] == ["tmux", "new-session"]]) + + raw = json.loads((tmp_path / "ai-accounts.json").read_text(encoding="utf-8")) + if mutate == "model": + raw["roles"]["interactive-builder"]["model"] = "changed-after-first-start" + elif mutate == "config_dir": + raw["accounts"]["interactive-grok"]["config_dir"] = "/operator/path/interactive-changed" + elif mutate == "account": + raw["roles"]["interactive-builder"]["account"] = "interactive-grok-b" + else: + raw["roles"]["interactive-builder-alt"] = { + "account": "interactive-grok", + "model": OPERATOR_INTERACTIVE_MODEL, + "access": "workspace-write", + } + raw["sessions"][SESSION_A]["interactive"] = "interactive-builder-alt" + (tmp_path / "ai-accounts.json").write_text(json.dumps(raw), encoding="utf-8") + + with pytest.raises(SystemExit, match="interactive AI binding changed"): + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + after = len([c for c in calls if c[:2] == ["tmux", "new-session"]]) + assert after == before + + +def test_legacy_grok_session_without_ai_binding_refuses_resume( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Legacy sessions with grok_session_id but no ai_binding must not silently resume.""" + write_operator_interactive_ai_accounts(tmp_path) + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + if argv[:2] == ["tmux", "has-session"]: + return Completed(1, "", "") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + store = Store(tmp_path) + try: + row = store.row("session", SESSION_A) + assert row is not None + row["runtime"] = { + "grok_session_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "provider": "grok", + "model": "grok-4.6", + "tmux_session": "agent-sess-1", + "control": "stopped", + } + store.write( + "session", + "update", + SESSION_A, + {k: v for k, v in row.items() if not str(k).startswith("_")}, + ) + finally: + store.close() + + with pytest.raises(SystemExit, match="interactive AI binding changed"): + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + assert not any(c[:2] == ["tmux", "new-session"] for c in calls) + + +def test_cli_model_flag_must_match_configured_interactive_role( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_interactive_ai_accounts(tmp_path) + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + if argv[:2] == ["tmux", "has-session"]: + return Completed(1, "", "session not found") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + # Old model-switch-on-resume behavior is gone: mismatched --model is refused. + with pytest.raises(SystemExit, match="--model does not match"): + run( + tmp_path, + [ + "session", + "start", + "--id", + SESSION_A, + "--provider", + "grok", + "--model", + "some-other-model", + ], + ) + assert not any(c[:2] == ["tmux", "new-session"] for c in calls) + + run( + tmp_path, + [ + "session", + "start", + "--id", + SESSION_A, + "--provider", + "grok", + "--model", + OPERATOR_INTERACTIVE_MODEL, + ], + ) + assert any(c[:2] == ["tmux", "new-session"] for c in calls) + + +def test_cli_interactive_codex_role_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_interactive_ai_accounts( + tmp_path, + sessions={SESSION_A: {"interactive": "interactive-codex-role", "lanes": {}}}, + ) + calls: list[list[str]] = [] + + def runner(argv: list[str]) -> Completed: + calls.append(list(argv)) + if argv[:2] == ["tmux", "-V"]: + return Completed(0, "tmux 3.3a", "") + return Completed(0, "", "") + + monkeypatch.setattr(main_mod, "Runtime", lambda *a, **k: Runtime(runner=runner)) + run(tmp_path, ["init"]) + run(tmp_path, ["session", "register", "--id", SESSION_A, "--kind", "human"]) + with pytest.raises(SystemExit, match="does not match the requested provider"): + run(tmp_path, ["session", "start", "--id", SESSION_A, "--provider", "grok"]) + assert not any(c[:2] == ["tmux", "new-session"] for c in calls) diff --git a/tests/test_lane.py b/tests/test_lane.py index 1a5c62e..3d07018 100644 --- a/tests/test_lane.py +++ b/tests/test_lane.py @@ -1,5 +1,7 @@ from __future__ import annotations +import json +import os from dataclasses import dataclass from pathlib import Path from subprocess import CompletedProcess @@ -7,6 +9,7 @@ import pytest +from agent_cli.ai_accounts import AccountError from agent_cli.lane import ( GROK_STRIP_ENV, LaneResult, @@ -21,15 +24,131 @@ pytestmark = pytest.mark.no_pg +OPERATOR_GROK_MODEL = "operator-lane-grok-model" +OPERATOR_CODEX_MODEL = "operator-lane-codex-model" +OPERATOR_GROK_HOME = "/operator/path/grok-lane-a" +OPERATOR_CODEX_HOME = "/operator/path/codex-lane-a" +OPERATOR_GROK_HOME_B = "/operator/path/grok-lane-b" +OPERATOR_GROK_MODEL_B = "operator-lane-grok-model-b" +DEFAULT_SESSION = "sess-1" +ALT_SESSION = "sess-2" + + +def write_operator_ai_accounts( + home: Path, + *, + sessions: dict | None = None, +) -> None: + """Write an operator-supplied ai-accounts.json for positive lane scenarios. + + Installation defaults stay empty. Callers must invoke this explicitly for + positive cases; negative tests leave the home unconfigured. + """ + data = { + "accounts": { + "grok-a": {"provider": "grok", "config_dir": OPERATOR_GROK_HOME}, + "codex-a": {"provider": "codex", "config_dir": OPERATOR_CODEX_HOME}, + "grok-b": {"provider": "grok", "config_dir": OPERATOR_GROK_HOME_B}, + }, + "roles": { + "lane-builder": { + "account": "grok-a", + "model": OPERATOR_GROK_MODEL, + "access": "workspace-write", + }, + "lane-reader": { + "account": "grok-a", + "model": OPERATOR_GROK_MODEL, + "access": "read-only", + }, + "codex-builder": { + "account": "codex-a", + "model": OPERATOR_CODEX_MODEL, + "access": "workspace-write", + }, + "codex-reader": { + "account": "codex-a", + "model": OPERATOR_CODEX_MODEL, + "access": "read-only", + }, + "lane-builder-b": { + "account": "grok-b", + "model": OPERATOR_GROK_MODEL_B, + "access": "workspace-write", + }, + }, + "sessions": sessions + or { + DEFAULT_SESSION: { + "interactive": "lane-builder", + "lanes": { + "grok:implementer": "lane-builder", + "grok:reviewer": "lane-reader", + "grok:pr-reviewer-quality": "lane-reader", + "grok:pr-reviewer-logic": "lane-reader", + "codex:implementer": "codex-builder", + "codex:reviewer": "codex-reader", + "codex:pr-reviewer-quality": "codex-reader", + "codex:pr-reviewer-logic": "codex-reader", + }, + }, + ALT_SESSION: { + "interactive": "lane-builder-b", + "lanes": { + "grok:implementer": "lane-builder-b", + }, + }, + }, + } + (home / "ai-accounts.json").write_text(json.dumps(data), encoding="utf-8") + def run(argv: list[str]) -> None: main(argv) -def test_grok_implementer_argv() -> None: - argv = grok_argv(spec_file="/tmp/spec.md", cwd="/work", write=True) +def _lane_inner(argv: list[str]) -> list[str]: + if "--" in argv: + return argv[argv.index("--") + 1 :] + return list(argv) + + +def _assert_nested_role_env_prefix( + argv: list[str], *, config_dir: str, provider: str +) -> None: + inner = _lane_inner(argv) + assert inner[0] == "env" + home_key = "GROK_HOME" if provider == "grok" else "CODEX_HOME" + assert f"{home_key}={config_dir}" in inner + for key in ( + "XAI_API_KEY", + "GROK_API_KEY", + "OPENAI_API_KEY", + "CODEX_API_KEY", + "ANTHROPIC_API_KEY", + "CLAUDECODE", + "CLAUDE_CODE_ENTRYPOINT", + "GROK_HOME", + "CODEX_HOME", + ): + assert key in inner + assert inner[inner.index(key) - 1] == "-u" + # Role env_prefix precedes the existing provider argv (which itself starts with env). + second_env = inner.index("env", 1) + assert second_env > inner.index(f"{home_key}={config_dir}") + provider_bin = "grok" if provider == "grok" else "codex" + assert provider_bin in inner[second_env:] + + +def test_grok_implementer_argv_requires_explicit_model() -> None: + argv = grok_argv( + spec_file="/tmp/spec.md", cwd="/work", write=True, model="explicit-grok-model" + ) assert "--session-id" not in argv assert "--always-approve" not in argv + assert "reasoning-effort" not in argv + assert "grok-4.5" not in argv + assert "grok-4.6" not in argv assert argv[0] == "env" for key in GROK_STRIP_ENV: assert "-u" in argv @@ -39,7 +158,7 @@ def test_grok_implementer_argv() -> None: "--prompt-file", "/tmp/spec.md", "-m", - "grok-4.5", + "explicit-grok-model", "--permission-mode", "acceptEdits", "--allow", @@ -51,23 +170,25 @@ def test_grok_implementer_argv() -> None: "--cwd", "/work", ] - # env strip order preserved strip_idx = [argv.index(k) for k in GROK_STRIP_ENV] assert strip_idx == sorted(strip_idx) -def test_grok_reviewer_argv() -> None: - argv = grok_argv(spec_file="/tmp/spec.md", cwd="/work", write=False) +def test_grok_reviewer_argv_requires_explicit_model() -> None: + argv = grok_argv( + spec_file="/tmp/spec.md", cwd="/work", write=False, model="explicit-readonly-model" + ) assert "--permission-mode" not in argv assert "acceptEdits" not in argv assert "--always-approve" not in argv assert "--session-id" not in argv + assert "reasoning-effort" not in argv assert argv[argv.index("grok") :] == [ "grok", "--prompt-file", "/tmp/spec.md", "-m", - "grok-4.5", + "explicit-readonly-model", "--allow", "Read", "--allow", @@ -90,6 +211,7 @@ def test_grok_reviewer_argv() -> None: def test_pr_reviewer_quality_uses_readonly_grok_argv(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("review this\n", encoding="utf-8") result = launch( @@ -99,27 +221,41 @@ def test_pr_reviewer_quality_uses_readonly_grok_argv(tmp_path: Path) -> None: cwd=str(tmp_path), dry_run=True, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert "--deny" in result.argv assert "Write" in result.argv assert "acceptEdits" not in result.argv assert "--permission-mode" not in result.argv + assert OPERATOR_GROK_MODEL in result.argv + _assert_nested_role_env_prefix( + result.argv, config_dir=OPERATOR_GROK_HOME, provider="grok" + ) -def test_codex_implementer_argv() -> None: - argv = codex_argv(cwd="/work", write=True, output_file="/tmp/out.txt") +def test_codex_implementer_argv_requires_explicit_model() -> None: + argv = codex_argv( + cwd="/work", write=True, output_file="/tmp/out.txt", model="explicit-codex-model" + ) assert "workspace-write" in argv - assert "gpt-5.6-sol" in argv + assert "explicit-codex-model" in argv + assert "gpt-5.6-sol" not in argv + assert "reasoning-effort" not in argv assert argv[-1] == "-" assert argv[0] == "env" for key in GROK_STRIP_ENV: assert key in argv + assert argv[argv.index("--model") + 1] == "explicit-codex-model" -def test_codex_reviewer_argv() -> None: - argv = codex_argv(cwd="/work", write=False, output_file="/tmp/out.txt") +def test_codex_reviewer_argv_requires_explicit_model() -> None: + argv = codex_argv( + cwd="/work", write=False, output_file="/tmp/out.txt", model="explicit-codex-ro" + ) assert "read-only" in argv assert "workspace-write" not in argv + assert argv[argv.index("--model") + 1] == "explicit-codex-ro" assert argv[-1] == "-" @@ -157,7 +293,184 @@ def test_parse_status_rc_zero_partial() -> None: assert parse_status("no status here", 0) == "partial" +def test_launch_requires_config_home_and_session_id(tmp_path: Path) -> None: + spec = tmp_path / "spec.md" + spec.write_text("do the thing\n", encoding="utf-8") + + def boom(argv: list[str], stdin_text: str | None) -> object: + raise AssertionError("runner must not be called without config") + + with pytest.raises(SystemExit, match="explicit session and AI configuration home"): + launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + dry_run=True, + tmux=False, + ) + with pytest.raises(SystemExit, match="explicit session and AI configuration home"): + launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + dry_run=True, + tmux=False, + config_home=tmp_path, + session_id=None, + ) + with pytest.raises(SystemExit, match="explicit session and AI configuration home"): + launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + dry_run=True, + tmux=False, + config_home=None, + session_id=DEFAULT_SESSION, + ) + + +def test_launch_unconfigured_lane_fails_before_runner(tmp_path: Path) -> None: + """Genuinely empty defaults: no ai-accounts.json, runner must not start.""" + spec = tmp_path / "spec.md" + spec.write_text("do the thing\n", encoding="utf-8") + called = {"n": 0} + + def boom(argv: list[str], stdin_text: str | None) -> object: + called["n"] += 1 + raise AssertionError("runner must not be called for unconfigured lane") + + with pytest.raises(AccountError, match="No AI session configured"): + launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + dry_run=False, + tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, + ) + assert called["n"] == 0 + + +def test_launch_provider_mismatch_refused_before_runner(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) + raw = json.loads((tmp_path / "ai-accounts.json").read_text(encoding="utf-8")) + raw["sessions"] = { + DEFAULT_SESSION: {"lanes": {"grok:implementer": "codex-builder"}}, + } + (tmp_path / "ai-accounts.json").write_text(json.dumps(raw), encoding="utf-8") + spec = tmp_path / "spec.md" + spec.write_text("implement\n", encoding="utf-8") + called = {"n": 0} + + def boom(argv: list[str], stdin_text: str | None) -> object: + called["n"] += 1 + raise AssertionError("runner must not start on provider mismatch") + + with pytest.raises(AccountError, match="provider does not match"): + launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, + ) + assert called["n"] == 0 + + +def test_launch_writable_reviewer_binding_refused_before_runner(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) + raw = json.loads((tmp_path / "ai-accounts.json").read_text(encoding="utf-8")) + raw["sessions"][DEFAULT_SESSION]["lanes"]["grok:reviewer"] = "lane-builder" + (tmp_path / "ai-accounts.json").write_text(json.dumps(raw), encoding="utf-8") + spec = tmp_path / "spec.md" + spec.write_text("review\n", encoding="utf-8") + called = {"n": 0} + + def boom(argv: list[str], stdin_text: str | None) -> object: + called["n"] += 1 + raise AssertionError("runner must not start on writable reviewer binding") + + with pytest.raises(AccountError, match="requires read-only access"): + launch( + role="reviewer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + runner=boom, + tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, + ) + assert called["n"] == 0 + + +def test_two_sessions_select_different_config_homes_and_models( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_ai_accounts(tmp_path) + spec = tmp_path / "spec.md" + spec.write_text("implement\n", encoding="utf-8") + monkeypatch.setenv("XAI_API_KEY", "ambient-xai-token") + monkeypatch.setenv("GROK_API_KEY", "ambient-grok-token") + monkeypatch.setenv("GROK_HOME", "/ambient/should-not-leak") + monkeypatch.setenv("OPENAI_API_KEY", "ambient-openai") + + result_a = launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + dry_run=True, + tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, + ) + result_b = launch( + role="implementer", + vendor="grok", + spec_file=str(spec), + cwd=str(tmp_path), + dry_run=True, + tmux=False, + config_home=tmp_path, + session_id=ALT_SESSION, + ) + + assert OPERATOR_GROK_MODEL in result_a.argv + assert OPERATOR_GROK_MODEL_B in result_b.argv + assert OPERATOR_GROK_MODEL_B not in result_a.argv + assert OPERATOR_GROK_MODEL not in result_b.argv + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in result_a.argv + assert f"GROK_HOME={OPERATOR_GROK_HOME_B}" in result_b.argv + assert "/ambient/should-not-leak" not in result_a.argv + assert "/ambient/should-not-leak" not in result_b.argv + assert "ambient-xai-token" not in " ".join(result_a.argv) + assert "ambient-grok-token" not in " ".join(result_b.argv) + assert os.environ["XAI_API_KEY"] == "ambient-xai-token" + assert os.environ["GROK_HOME"] == "/ambient/should-not-leak" + _assert_nested_role_env_prefix( + result_a.argv, config_dir=OPERATOR_GROK_HOME, provider="grok" + ) + _assert_nested_role_env_prefix( + result_b.argv, config_dir=OPERATOR_GROK_HOME_B, provider="grok" + ) + + def test_launch_dry_run_does_not_call_runner(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("do the thing\n", encoding="utf-8") @@ -172,15 +485,21 @@ def boom(argv: list[str], stdin_text: str | None) -> object: runner=boom, dry_run=True, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert result.status == "" assert result.returncode == 0 - assert "grok-4.5" in result.argv + assert OPERATOR_GROK_MODEL in result.argv + _assert_nested_role_env_prefix( + result.argv, config_dir=OPERATOR_GROK_HOME, provider="grok" + ) def test_launch_codex_dry_run_skips_mkstemp( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("codex dry run\n", encoding="utf-8") @@ -195,11 +514,18 @@ def boom_mkstemp(*_args: object, **_kwargs: object) -> tuple[int, str]: cwd=str(tmp_path), dry_run=True, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert "--output-last-message" in result.argv + assert OPERATOR_CODEX_MODEL in result.argv + _assert_nested_role_env_prefix( + result.argv, config_dir=OPERATOR_CODEX_HOME, provider="codex" + ) def test_launch_fake_runner_codex_stdin(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" contents = "codex please implement\n" spec.write_text(contents, encoding="utf-8") @@ -220,10 +546,13 @@ def fake(argv: list[str], stdin_text: str | None) -> object: cwd=str(tmp_path), runner=fake, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert len(seen) == 1 assert seen[0][1] == contents assert "codex" in seen[0][0] + assert OPERATOR_CODEX_MODEL in seen[0][0] assert result.status == "complete" assert result.returncode == 0 assert output_paths @@ -231,6 +560,7 @@ def fake(argv: list[str], stdin_text: str | None) -> object: def test_launch_codex_unlinks_output_file_on_runner_exception(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("codex please fail\n", encoding="utf-8") out_path: str | None = None @@ -249,12 +579,15 @@ def fake(argv: list[str], stdin_text: str | None) -> object: cwd=str(tmp_path), runner=fake, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert out_path is not None assert not Path(out_path).exists() def test_launch_fake_runner_grok_stdin_none_or_empty(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("grok please implement\n", encoding="utf-8") seen: list[str | None] = [] @@ -276,6 +609,8 @@ def fake(argv: list[str], stdin_text: str | None) -> object: cwd=str(tmp_path), runner=fake, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert seen == [None] or seen == [""] assert result.status == "complete" @@ -298,6 +633,7 @@ def test_tmux_wrap_argv_shape() -> None: def test_launch_default_wraps_tmux(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") result = launch( @@ -306,18 +642,24 @@ def test_launch_default_wraps_tmux(tmp_path: Path) -> None: spec_file=str(spec), cwd=str(tmp_path), dry_run=True, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert result.argv[:3] == ["tmux", "new-session", "-d"] assert "-s" in result.argv assert result.tmux_session is not None assert result.tmux_session.startswith("agent-lane-grok-implementer") assert "--" in result.argv - assert "grok-4.5" in result.argv + assert OPERATOR_GROK_MODEL in result.argv inner = result.argv[result.argv.index("--") + 1 :] assert inner[0] == "env" + _assert_nested_role_env_prefix( + result.argv, config_dir=OPERATOR_GROK_HOME, provider="grok" + ) def test_launch_no_tmux_starts_with_env(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") result = launch( @@ -327,13 +669,17 @@ def test_launch_no_tmux_starts_with_env(tmp_path: Path) -> None: cwd=str(tmp_path), dry_run=True, tmux=False, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert result.argv[0] == "env" assert "tmux" not in result.argv assert result.tmux_session is None + assert OPERATOR_GROK_MODEL in result.argv def test_launch_tmux_fake_runner_gets_wrapped_argv(tmp_path: Path) -> None: + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") seen: list[list[str]] = [] @@ -348,6 +694,8 @@ def fake(argv: list[str], stdin_text: str | None) -> object: spec_file=str(spec), cwd=str(tmp_path), runner=fake, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) assert len(seen) == 1 assert seen[0][:3] == ["tmux", "new-session", "-d"] @@ -469,6 +817,7 @@ def handler(argv: list[str], _calls: list[list[str]]) -> CompletedProcess[str]: def test_launch_tmux_passes_absolute_spec_file( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + write_operator_ai_accounts(tmp_path) monkeypatch.chdir(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") @@ -480,6 +829,8 @@ def test_launch_tmux_passes_absolute_spec_file( spec_file="spec.md", cwd=str(work), dry_run=True, + config_home=tmp_path, + session_id=DEFAULT_SESSION, ) inner = result.argv[result.argv.index("--") + 1 :] prompt = inner[inner.index("--prompt-file") + 1] @@ -506,10 +857,14 @@ def handler(argv: list[str], _calls: list[list[str]]) -> CompletedProcess[str]: def test_cli_lane_run_prints_vendor_stdout( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: + write_operator_ai_accounts(tmp_path) + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) spec = tmp_path / "spec.md" spec.write_text("review this\n", encoding="utf-8") + seen: dict = {} def fake_launch(**kwargs): # type: ignore[no-untyped-def] + seen.update(kwargs) return LaneResult( role="pr-reviewer-quality", vendor="grok", @@ -529,6 +884,8 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] "pr-reviewer-quality", "--vendor", "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(spec), "--cwd", @@ -540,11 +897,15 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] assert "distinctive-marker-abc123" in out assert "STATUS=complete" in out assert out.index("distinctive-marker-abc123") < out.index("STATUS=complete") + assert seen.get("session_id") == DEFAULT_SESSION + assert seen.get("config_home") == tmp_path def test_cli_lane_run_prints_vendor_stderr( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: + write_operator_ai_accounts(tmp_path) + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) spec = tmp_path / "spec.md" spec.write_text("review this\n", encoding="utf-8") @@ -569,6 +930,8 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] "pr-reviewer-quality", "--vendor", "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(spec), "--cwd", @@ -605,7 +968,11 @@ def test_sanitize_lane_output_strips_exact_range_boundaries() -> None: assert _sanitize_lane_output(kept) == kept -def test_cli_dry_run_implementer_grok(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: +def test_cli_dry_run_implementer_grok( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_ai_accounts(tmp_path) + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") run( @@ -616,6 +983,8 @@ def test_cli_dry_run_implementer_grok(tmp_path: Path, capsys: pytest.CaptureFixt "implementer", "--vendor", "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(spec), "--cwd", @@ -626,11 +995,16 @@ def test_cli_dry_run_implementer_grok(tmp_path: Path, capsys: pytest.CaptureFixt out = capsys.readouterr().out.strip() assert "tmux" in out assert "new-session" in out - assert "grok-4.5" in out + assert OPERATOR_GROK_MODEL in out + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in out assert "STATUS=" not in out -def test_cli_no_tmux_dry_run(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: +def test_cli_no_tmux_dry_run( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + write_operator_ai_accounts(tmp_path) + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") run( @@ -641,6 +1015,8 @@ def test_cli_no_tmux_dry_run(tmp_path: Path, capsys: pytest.CaptureFixture[str]) "implementer", "--vendor", "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(spec), "--cwd", @@ -652,10 +1028,32 @@ def test_cli_no_tmux_dry_run(tmp_path: Path, capsys: pytest.CaptureFixture[str]) out = capsys.readouterr().out.strip() assert out.startswith("env ") assert "new-session" not in out - assert "grok-4.5" in out + assert OPERATOR_GROK_MODEL in out + assert f"GROK_HOME={OPERATOR_GROK_HOME}" in out -def test_cli_missing_spec_dies(tmp_path: Path) -> None: +def test_cli_missing_session_dies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) + spec = tmp_path / "spec.md" + spec.write_text("x\n", encoding="utf-8") + with pytest.raises(SystemExit, match="--session is required"): + run( + [ + "lane", + "run", + "--role", + "implementer", + "--vendor", + "grok", + "--spec-file", + str(spec), + ] + ) + + +def test_cli_missing_spec_dies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + write_operator_ai_accounts(tmp_path) + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) with pytest.raises(SystemExit, match="spec"): run( [ @@ -665,13 +1063,16 @@ def test_cli_missing_spec_dies(tmp_path: Path) -> None: "implementer", "--vendor", "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(tmp_path / "missing.md"), ] ) -def test_cli_unknown_vendor_dies(tmp_path: Path) -> None: +def test_cli_unknown_vendor_dies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) spec = tmp_path / "spec.md" spec.write_text("x\n", encoding="utf-8") with pytest.raises(SystemExit, match="vendor"): @@ -683,7 +1084,39 @@ def test_cli_unknown_vendor_dies(tmp_path: Path) -> None: "implementer", "--vendor", "nope", + "--session", + DEFAULT_SESSION, + "--spec-file", + str(spec), + ] + ) + + +def test_cli_unconfigured_lane_dies_before_launch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Empty defaults: CLI lane run must fail without starting a vendor process.""" + monkeypatch.setenv("AGENT_HOME", str(tmp_path)) + spec = tmp_path / "spec.md" + spec.write_text("implement\n", encoding="utf-8") + # Do not mock launch: AccountError from real launch must surface via main. + # dry-run still resolves accounts before building argv, so no process starts. + with pytest.raises(SystemExit, match="No AI session configured"): + run( + [ + "lane", + "run", + "--role", + "implementer", + "--vendor", + "grok", + "--session", + DEFAULT_SESSION, "--spec-file", str(spec), + "--cwd", + str(tmp_path), + "--dry-run", + "--no-tmux", ] ) diff --git a/tests/test_run.py b/tests/test_run.py index 543ac5c..31fd917 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os from pathlib import Path @@ -11,6 +12,54 @@ from test_cli import _last_agent_id, _last_task_id, run from github_support import configure_accounts +OPERATOR_GROK_MODEL = "operator-run-grok-model" +OPERATOR_GROK_HOME = "/operator/path/grok-run-a" +OPERATOR_CODEX_HOME = "/operator/path/codex-run-a" +DEFAULT_SESSION = "sess-1" + + +def write_operator_ai_accounts(home: Path, *, sessions: dict | None = None) -> None: + """Write an explicit operator ai-accounts.json for positive cmd_run lane starts. + + Installation defaults stay empty. Call this only for configured success + paths; negative tests must leave the home unconfigured or deliberately + malformed/mismatched. + """ + data = { + "accounts": { + "grok-a": {"provider": "grok", "config_dir": OPERATOR_GROK_HOME}, + "codex-a": {"provider": "codex", "config_dir": OPERATOR_CODEX_HOME}, + }, + "roles": { + "run-builder": { + "account": "grok-a", + "model": OPERATOR_GROK_MODEL, + "access": "workspace-write", + }, + "run-reader": { + "account": "grok-a", + "model": OPERATOR_GROK_MODEL, + "access": "read-only", + }, + "codex-builder": { + "account": "codex-a", + "model": "operator-run-codex-model", + "access": "workspace-write", + }, + }, + "sessions": sessions + or { + DEFAULT_SESSION: { + "interactive": "run-builder", + "lanes": { + "grok:implementer": "run-builder", + "grok:reviewer": "run-reader", + }, + }, + }, + } + (home / "ai-accounts.json").write_text(json.dumps(data), encoding="utf-8") + def _store(home: Path) -> Store: os.environ["AGENT_HOME"] = str(home) @@ -292,10 +341,13 @@ def test_run_prints_vendor_stdout( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: tid = _bootstrap_implement(tmp_path, capsys) + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement this\n", encoding="utf-8") + seen: dict = {} def fake_launch(**kwargs): # type: ignore[no-untyped-def] + seen.update(kwargs) return LaneResult( role="implementer", vendor="grok", @@ -324,16 +376,24 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] marker_at = out.index("distinctive-marker-run456") summary_at = out.index("STATUS=complete") assert marker_at < summary_at + assert seen.get("session_id") == "sess-1" + assert seen.get("config_home") == tmp_path + assert seen.get("role") == "implementer" + assert seen.get("vendor") == "grok" + assert seen.get("tmux") is False def test_run_spec_file_implementer_complete( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: tid = _bootstrap_implement(tmp_path, capsys) + write_operator_ai_accounts(tmp_path) spec = tmp_path / "spec.md" spec.write_text("implement this\n", encoding="utf-8") + seen: dict = {} def fake_launch(**kwargs): # type: ignore[no-untyped-def] + seen.update(kwargs) return LaneResult( role="implementer", vendor="grok", @@ -364,6 +424,9 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] a.get("role") == "implementer" and a.get("status") == "done" for a in _agents(tmp_path, tid) ) + assert seen.get("session_id") == "sess-1" + assert seen.get("config_home") == tmp_path + assert seen.get("spec_file") == str(spec) def test_run_missing_spec_file_does_not_leave_working_agent( @@ -388,6 +451,120 @@ def test_run_missing_spec_file_does_not_leave_working_agent( assert not any(a.get("status") == "working" for a in _agents(tmp_path, tid)) +def _assert_run_config_failure_leaves_no_working_agent( + home: Path, + tid: str, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, + *, + spec: Path, + match: str, +) -> None: + called = {"n": 0} + + def fake_launch(**kwargs): # type: ignore[no-untyped-def] + called["n"] += 1 + raise AssertionError("model must not launch when AI lane binding fails") + + monkeypatch.setattr("agent_cli.main.launch", fake_launch) + with pytest.raises(SystemExit, match=match): + run( + home, + [ + "run", + "--task", + tid, + "--spec-file", + str(spec), + "--no-tmux", + "--cwd", + str(home), + ], + ) + capsys.readouterr() + assert called["n"] == 0 + assert not any(a.get("status") == "working" for a in _agents(home, tid)) + + +def test_run_unconfigured_ai_leaves_no_working_agent( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + """Empty installation default: cmd_run must not persist a working lane.""" + tid = _bootstrap_implement(tmp_path, capsys) + spec = tmp_path / "spec.md" + spec.write_text("implement this\n", encoding="utf-8") + _assert_run_config_failure_leaves_no_working_agent( + tmp_path, + tid, + capsys, + monkeypatch, + spec=spec, + match="No AI session configured", + ) + + +def test_run_malformed_ai_accounts_leaves_no_working_agent( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + tid = _bootstrap_implement(tmp_path, capsys) + (tmp_path / "ai-accounts.json").write_text("{", encoding="utf-8") + spec = tmp_path / "spec.md" + spec.write_text("implement this\n", encoding="utf-8") + _assert_run_config_failure_leaves_no_working_agent( + tmp_path, + tid, + capsys, + monkeypatch, + spec=spec, + match="Cannot read ai-accounts.json", + ) + + +def test_run_provider_mismatch_leaves_no_working_agent( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + tid = _bootstrap_implement(tmp_path, capsys) + write_operator_ai_accounts(tmp_path) + raw = json.loads((tmp_path / "ai-accounts.json").read_text(encoding="utf-8")) + raw["sessions"] = { + DEFAULT_SESSION: {"lanes": {"grok:implementer": "codex-builder"}}, + } + (tmp_path / "ai-accounts.json").write_text(json.dumps(raw), encoding="utf-8") + spec = tmp_path / "spec.md" + spec.write_text("implement this\n", encoding="utf-8") + _assert_run_config_failure_leaves_no_working_agent( + tmp_path, + tid, + capsys, + monkeypatch, + spec=spec, + match="provider does not match", + ) + + +def test_run_writable_reviewer_binding_leaves_no_working_agent( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + tid = _bootstrap_implement(tmp_path, capsys) + _finish_implementer(tmp_path, tid, capsys) + run(tmp_path, ["run", "--task", tid]) # implementer_done + capsys.readouterr() + write_operator_ai_accounts(tmp_path) + raw = json.loads((tmp_path / "ai-accounts.json").read_text(encoding="utf-8")) + raw["sessions"][DEFAULT_SESSION]["lanes"]["grok:reviewer"] = "run-builder" + (tmp_path / "ai-accounts.json").write_text(json.dumps(raw), encoding="utf-8") + spec = tmp_path / "review-spec.md" + spec.write_text("review this\n", encoding="utf-8") + _assert_run_config_failure_leaves_no_working_agent( + tmp_path, + tid, + capsys, + monkeypatch, + spec=spec, + match="requires read-only access", + ) + + def test_run_spec_file_reviewer_complete_no_auto_approve( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: @@ -395,10 +572,13 @@ def test_run_spec_file_reviewer_complete_no_auto_approve( _finish_implementer(tmp_path, tid, capsys) run(tmp_path, ["run", "--task", tid]) # implementer_done capsys.readouterr() + write_operator_ai_accounts(tmp_path) spec = tmp_path / "review-spec.md" spec.write_text("review this\n", encoding="utf-8") + seen: dict = {} def fake_launch(**kwargs): # type: ignore[no-untyped-def] + seen.update(kwargs) return LaneResult( role="reviewer", vendor="grok", @@ -430,6 +610,10 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] if agent.get("role") != "reviewer": continue assert agent.get("status") == "working" + assert seen.get("session_id") == "sess-1" + assert seen.get("config_home") == tmp_path + assert seen.get("role") == "reviewer" + assert seen.get("vendor") == "grok" def _advance_to_pushed( diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 97fe96f..1fcf946 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -117,30 +117,38 @@ def test_grok_session_id_is_uuid_not_ulid() -> None: assert "_" not in gid -def test_grok_empty_model_is_grok_46() -> None: - assert grok_model(None) == "grok-4.6" - assert grok_model("") == "grok-4.6" - assert grok_model(" ") == "grok-4.6" +def test_grok_model_none_or_blank_raises_store_error() -> None: + with pytest.raises(StoreError, match="explicitly configured"): + grok_model(None) + with pytest.raises(StoreError, match="explicitly configured"): + grok_model("") + with pytest.raises(StoreError, match="explicitly configured"): + grok_model(" ") assert grok_model("opus") == "opus" - assert grok_model("grok-4.5") == "grok-4.5" + assert grok_model("operator-selected-model") == "operator-selected-model" + assert grok_model(" operator-selected-model ") == "operator-selected-model" def test_grok_first_start_uses_session_id() -> None: - argv = grok_launch_argv(existing="", model="", new_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") + argv = grok_launch_argv( + existing="", + model="operator-selected-model", + new_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + ) assert argv == [ "grok", "--always-approve", "--session-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--model", - "grok-4.6", + "operator-selected-model", ] def test_grok_resume_does_not_use_session_id() -> None: argv = grok_launch_argv( existing="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", - model="grok-4.5", + model="operator-resume-model", new_id="should-not-appear", ) assert argv == [ @@ -149,14 +157,18 @@ def test_grok_resume_does_not_use_session_id() -> None: "--resume", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--model", - "grok-4.5", + "operator-resume-model", ] assert "--session-id" not in argv assert "should-not-appear" not in argv def test_grok_tmux_argv_unsets_claude_env() -> None: - argv = grok_tmux_command_argv(existing="", model="", new_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") + argv = grok_tmux_command_argv( + existing="", + model="operator-selected-model", + new_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + ) assert argv[:7] == [ "env", "-u", @@ -172,13 +184,24 @@ def test_grok_tmux_argv_unsets_claude_env() -> None: "--session-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--model", - "grok-4.6", + "operator-selected-model", ] def test_grok_rejects_non_uuid_session_id() -> None: with pytest.raises(SystemExit, match="UUID"): - grok_launch_argv(existing="", model="", new_id="01ARZ3NDEKTSV4RRFFQ69G5FAV") + grok_launch_argv( + existing="", + model="operator-selected-model", + new_id="01ARZ3NDEKTSV4RRFFQ69G5FAV", + ) + + with pytest.raises(StoreError, match="explicitly configured"): + grok_launch_argv( + existing="", + model="", + new_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + ) def test_start_cwd_argv() -> None: