Coverage for src/keel/delegate.py: 100%
283 statements
« prev ^ index » next coverage.py v7.16.2, created at 2026-10-02 20:26 +0000
1"""How keel invokes a delegate — the pure planner behind ``keel delegate run`` (#1012).
3Nothing in keel dispatched a delegate. ``api_delegate.generate()`` had no caller; the
4``claude``/``codex``/``agy`` argv shapes, the stdin framing, the Ollama endpoint, the
5timeouts and the JSON return contract lived only as prose in ``ship.md`` s4/s7, so every
6host agent re-invented them and they drifted. This module is the single answer to *how do
7I invoke provider P for role R*, and :mod:`keel.delegaterun` is the only thing that runs
8the answer.
10The split is keel's usual one. Everything here is **pure**: it takes a
11:class:`keel.providers.Provider` (from #1011's registry) plus a role, and returns a frozen
12:class:`RunPlan` — an argv, or an HTTP request description, plus the stdin framing, the
13read-only verdict, the attribution record and any warnings. No subprocess, no network, no
14clock, no filesystem. That is what makes "a ``review`` run never carries a write-enabling
15flag" a property a unit test can assert per vendor instead of a sentence in a markdown
16file nobody can execute.
18**Role policy.** ``review``/``gate``/``chair`` are read-only: the delegate reads a diff
19and returns findings, and every mutation stays with the orchestrator. ``implement``/``fix``
20are tool-enabled. keel can *document* a read-only invocation for the three built-in CLIs
21and *offer* one (``review_args``) for a configured profile; it cannot **enforce** read-only
22for an arbitrary binary, which is why :attr:`RunPlan.read_only` reports the role's policy
23and a warning names the case where nothing backs it.
25**Why the prompt is never in argv.** Every transport that runs a binary delivers the
26prompt on stdin. A prompt carries the diff and the brief; an argv is world-readable in
27``ps`` for the life of the process, and a large diff can exceed ``ARG_MAX`` outright. The
28one exception is a profile that declares ``prompt_mode: arg`` — an operator-authored
29choice for CLIs whose usage requires it (``cursor-agent``'s is
30``agent [options] [command] [prompt...]``), where the plan carries ``stdin_mode: None`` and
31the executor appends the prompt as the final argument.
33**agy's framing.** ai-jury (``src/ai_jury/adapters.py``, read-only reference) verified two
34invocations against the shipped CLI: ``--print=<prompt>``, which takes the prompt as the
35flag's *value* because ``--print --model X`` swallows the next flag; and
36``--input-format stream-json --output-format stream-json``, which reads one NDJSON frame
37per line on stdin. keel takes the **stream-json** one. Three reasons, in order: the prompt
38stays out of ``ps`` and out of ``ARG_MAX`` (ai-jury's #287); it sidesteps the ``--print``
39arity trap entirely rather than working around it; and — decisively for this module — a
40*pure* planner is handed a prompt **path**, never the prompt text, so it could not build a
41``--print=<prompt>`` argv at all without reading a file and stopping being pure. The
42NDJSON reply is parsed back by :func:`parse_stream_json`.
44Pure and deterministic: no wall-clock, no randomness, no I/O. Failures raise
45:class:`DelegateError`, which carries a machine-readable ``code`` the executor turns into
46the fail-soft ``error_code`` of the JSON contract — a traceback never reaches an operator.
47"""
49from __future__ import annotations
51import json
52import posixpath
53import re
54from dataclasses import dataclass, field
55from typing import Any
57from . import agents
58from . import providers as providers_mod
59from .api_delegate import DEFAULT_MAX_TOKENS, OPENAI_COMPATIBLE
60from .config import DEFAULT_PROMPT_MODE, DelegateProfile
62# The effort vocabulary lives in the leaf :mod:`keel.vocab` so ``keel.team`` can validate
63# a seat's ``effort`` without importing the executor (#1050). ``EFFORT_VENDORS`` and
64# ``supports_effort`` are re-exported under the original names, which are what the rest
65# of the package and the tests read.
66from .vocab import EFFORT_VENDORS as EFFORT_VENDORS
67from .vocab import EFFORTS as EFFORTS
68from .vocab import supports_effort as supports_effort
70#: The module's public surface, in definition order (#1070). It is declared because the
71#: ``X as X`` re-exports above are read from *other* modules — a use CodeQL's
72#: ``py/unused-import`` cannot see, since it counts same-module uses only. A name listed
73#: in ``__all__`` is used by definition, so the declaration answers the scanner with the
74#: language's own statement of intent rather than with a dismissal. Being a real
75#: declaration it has to be the *whole* surface, not the re-exports alone;
76#: ``tests/test_reexport_surface.py`` holds it to that in both directions.
77__all__ = [
78 "EFFORT_VENDORS",
79 "EFFORTS",
80 "supports_effort",
81 "ROLES",
82 "READ_ONLY_ROLES",
83 "TRANSPORTS",
84 "STDIN_TEXT",
85 "STDIN_STREAM_JSON",
86 "DEFAULT_TIMEOUT_S",
87 "OLLAMA_GENERATE_URL",
88 "AGY_STREAM_ARGS",
89 "CLAUDE_ALLOWED_TOOLS",
90 "CODEX_EFFORT_CONFIG",
91 "ANTHROPIC_THINKING_BUDGET",
92 "GOOGLE_THINKING_BUDGET",
93 "THINKING_HEADROOM_TOKENS",
94 "DelegateError",
95 "Resolution",
96 "Effort",
97 "RunPlan",
98 "resolve_provider",
99 "is_safe_body_model_token",
100 "model_reaches_argv",
101 "model_token_issue",
102 "is_absolute_cwd",
103 "plan_run",
104 "stream_json_frame",
105 "parse_stream_json",
106 "parse_ollama_response",
107 "ollama_payload",
108]
110#: Roles a delegate can be dispatched for. ``chair`` is the jury's summariser; it reads
111#: reviews and writes nothing, so it sits with the read-only three.
112ROLES = ("implement", "fix", "review", "gate", "chair")
114#: Roles invoked read-only / findings-only. Everything not listed here is tool-enabled.
115READ_ONLY_ROLES = ("review", "gate", "chair")
117#: How the executor reaches a provider. ``cli`` is one of the three built-in agent CLIs,
118#: ``profile`` any other binary (a project ``delegate_profiles`` entry or a registry
119#: ``cli``/``local`` entry), ``api`` a hosted or OpenAI-compatible HTTP endpoint, and
120#: ``ollama`` the built-in local vendor served on loopback.
121TRANSPORTS = ("cli", "profile", "api", "ollama")
123#: Prompt framing on the child's stdin. ``None`` means "no stdin": the executor appends
124#: the prompt as the final argv element (a profile's ``prompt_mode: arg``).
125STDIN_TEXT = "text"
126STDIN_STREAM_JSON = "stream-json"
128#: Wall-clock seconds a delegate run may take when the caller names none. Delegated
129#: implementation is measured in tens of minutes, not in the 120 s a git call gets.
130DEFAULT_TIMEOUT_S = 1800
132#: Ollama's local generation endpoint. A **hardcoded loopback constant**, exactly like
133#: :data:`keel.providers.OLLAMA_TAGS_URL` and every other URL keel dials: the executor
134#: never reaches an endpoint named by config or by the registry.
135OLLAMA_GENERATE_URL = "http://127.0.0.1:11434/api/generate"
137#: agy's NDJSON stdin/stdout framing. See the module docstring for why keel uses it
138#: rather than ``--print=<prompt>``.
139AGY_STREAM_ARGS = ("--input-format", "stream-json", "--output-format", "stream-json")
141#: A Windows absolute path (``C:\\wt`` / ``C:/wt``) or a UNC share (``\\\\host\\share``).
142#: ``posixpath.isabs`` says False for both, and a plan is a document that may be read
143#: on a platform other than the one that wrote it.
144_WINDOWS_ABSOLUTE = re.compile(r"^(?:[A-Za-z]:[\\/]|\\\\)")
146#: The only tools a read-only ``claude`` invocation may use. An **allow-list**: a denylist
147#: of write tools has to be extended every time the CLI grows one, and is wrong in the
148#: window before someone notices. Reading a diff needs no more than these three — ``Glob``
149#: is how the current CLI lists a directory, so there is no separate listing tool to name.
150CLAUDE_ALLOWED_TOOLS = "Read,Grep,Glob"
152#: How ``codex`` spells reasoning effort. Not a flag on the shipped CLI (0.152.1) — it is
153#: a config override, and an unknown key is rejected under ``--strict-config``, which is
154#: how this spelling was verified rather than assumed.
155CODEX_EFFORT_CONFIG = "model_reasoning_effort"
157#: ``anthropic-api`` extended-thinking budget per effort level, in tokens.
158ANTHROPIC_THINKING_BUDGET = {"low": 2048, "medium": 8192, "high": 32768}
160#: ``google-api`` ``thinkingBudget`` per effort level, in tokens. Gemini's floor is lower
161#: than Claude's, so ``low`` is 1024 rather than 2048.
162GOOGLE_THINKING_BUDGET = {"low": 1024, "medium": 8192, "high": 32768}
164#: Room left for the answer *above* a thinking budget. Anthropic requires
165#: ``max_tokens > thinking.budget_tokens``; a budget of 32768 against the default 16384
166#: cap is rejected by the API before a token is generated.
167THINKING_HEADROOM_TOKENS = 4096
169#: Suffixes agy spells reasoning effort with, e.g. ``gemini-3.8-flash-high``.
170_EFFORT_SUFFIXES = tuple(f"-{level}" for level in EFFORTS)
173class DelegateError(Exception):
174 """A run that cannot be planned. ``code`` is the JSON contract's ``error_code``."""
176 def __init__(self, code: str, message: str) -> None:
177 super().__init__(message)
178 self.code = code
179 self.message = message
182@dataclass(frozen=True)
183class Resolution:
184 """Which provider a ``--provider`` token names, and the model it carries."""
186 provider: providers_mod.Provider
187 #: The project ``delegate_profiles`` entry behind a ``profile``-source provider.
188 #: :class:`keel.providers.Provider` keeps only ``review_args``, so the implementer's
189 #: ``args`` and the ``prompt_mode`` are reachable only through the profile itself.
190 profile: DelegateProfile | None = None
191 #: The per-run model from ``vendor:model``, already validated. ``None`` when the
192 #: token named no model.
193 model: str | None = None
196@dataclass(frozen=True)
197class Effort:
198 """How one ``--effort`` request landed on one vendor.
200 Returned as a record rather than a tuple because the *budget* is needed twice and in
201 two shapes: inside the vendor's payload fragment, and again as the number
202 ``max_tokens`` must exceed. Re-deriving it by digging back into the fragment means
203 one parser per vendor spelling, with branches nothing can reach.
204 """
206 model: str | None
207 payload: dict[str, Any]
208 #: Thinking-token budget the fragment asks for, ``None`` when it asks for none.
209 budget: int | None
210 applied: bool
211 warnings: tuple[str, ...]
212 #: Extra argv fragment, for a vendor that spells effort on its command line.
213 argv: tuple[str, ...] = ()
216@dataclass(frozen=True)
217class RunPlan:
218 """One fully-resolved delegate invocation. Frozen, JSON-stable, never executed here."""
220 provider: str
221 #: The vendor attribution names — a built-in's own token, or a profile/registry
222 #: entry's ``vendor_label`` when it declares one (#1129). Before that field existed
223 #: this said ``cli`` for *every* configured CLI, so two entries driving different
224 #: makers through one binary were indistinguishable here and in the label derived
225 #: from it. Which executor runs the plan stays in :attr:`transport`
226 #: (``cli``/``profile``/``api``/``ollama``); a declared label costs the finer
227 #: ``cli``-vs-``local`` split, which no caller branches on — both are run as a
228 #: configured binary.
229 vendor: str
230 role: str
231 transport: str
232 prompt_path: str
233 model: str | None = None
234 cwd: str | None = None
235 timeout: int = DEFAULT_TIMEOUT_S
236 #: The command line, **without** the prompt. Empty for ``api``/``ollama``.
237 argv: tuple[str, ...] = ()
238 #: The HTTP call description for ``api``/``ollama``; ``None`` for the argv transports.
239 request: dict[str, Any] | None = None
240 #: ``text`` | ``stream-json`` | ``None`` (append the prompt as the last argv element).
241 stdin_mode: str | None = None
242 #: True when the role's policy is read-only.
243 read_only: bool = False
244 #: True when something actually **backs** that policy: a built-in CLI's documented
245 #: read-only invocation, an operator's ``review_args``, or a transport with no tools
246 #: at all. False means the role says read-only and nothing enforces it — the caller
247 #: must decide whether to run at all. Split from :attr:`read_only` because a single
248 #: flag cannot distinguish "reviewer" from "reviewer holding the implementer's write
249 #: flags", and the second is the one that edits the checkout.
250 read_only_backed: bool = False
251 effort: str | None = None
252 effort_applied: bool = False
253 warnings: tuple[str, ...] = ()
254 attribution: dict[str, str | None] = field(default_factory=dict)
256 def as_dict(self) -> dict[str, Any]:
257 """JSON-stable record (carries no secret: ``api_key_env`` is a name)."""
258 return {
259 "provider": self.provider,
260 "vendor": self.vendor,
261 "role": self.role,
262 "transport": self.transport,
263 "prompt_path": self.prompt_path,
264 "model": self.model,
265 "cwd": self.cwd,
266 "timeout": self.timeout,
267 "argv": list(self.argv),
268 "request": dict(self.request) if self.request is not None else None,
269 "stdin_mode": self.stdin_mode,
270 "read_only": self.read_only,
271 "read_only_backed": self.read_only_backed,
272 "effort": self.effort,
273 "effort_applied": self.effort_applied,
274 "warnings": list(self.warnings),
275 "attribution": dict(self.attribution),
276 }
279def resolve_provider(
280 config,
281 registry: providers_mod.Registry | None,
282 token: str,
283) -> Resolution:
284 """Resolve ``name`` / ``vendor:model`` against the built-ins, profiles, the registry.
286 Precedence is **built-in > project profile > registry**. A built-in vendor always
287 wins and can never be redefined — the invariant
288 :func:`keel.agents.resolve_delegate_profile` states and
289 :func:`keel.config.parse_config` enforces up front, and the same rule
290 :func:`keel.providers.registry_clashes` applies to a machine-level entry. Resolving a
291 profile or a registry entry first would make ``claude`` mean whatever a file in
292 ``$HOME`` said it meant, which is exactly the shadowing those two checks exist to
293 refuse; dispatch must not be the one place the rule is inverted.
295 The model half of the token is validated before anything else looks at it: it arrives
296 per run, on the command line (``--provider <name>:<model>`` or ``--model``), which is
297 a lower-trust source than the operator-authored command beside it. **Which** rule
298 applies depends on where the model lands — see :func:`model_token_issue`.
299 """
300 name, model = agents.split_delegate((token or "").strip())
301 if not name:
302 raise DelegateError("bad-provider", "--provider is empty; pass name or vendor:model")
303 profiles = {} if config is None else config.knobs.delegate_profiles
304 registry = providers_mod.Registry(path="") if registry is None else registry
305 resolution = _lookup(name, config, profiles, registry)
306 if resolution is None:
307 known = ", ".join(sorted({*profiles, *registry.names(), *agents.BUILTIN_DELEGATE_VENDORS}))
308 raise DelegateError("unknown-provider", f"unknown provider {name!r}; known: {known}")
309 _check_model(resolution.provider, model)
310 return Resolution(resolution.provider, resolution.profile, model)
313def _lookup(name, config, profiles, registry: providers_mod.Registry) -> Resolution | None:
314 """The provider ``name`` means, in precedence order. ``None`` when nothing claims it."""
315 for entry in providers_mod.builtin_providers():
316 if entry.name == name:
317 return Resolution(entry, None, None)
318 if name in profiles:
319 return Resolution(_profile_provider(config, name), profiles[name], None)
320 for entry in registry.providers:
321 if entry.name == name:
322 return Resolution(entry, None, None)
323 return None
326#: Characters a model may contain when it travels in a **JSON request body**. Wider than
327#: :func:`keel.agents.is_safe_model_token` by ``:`` and ``/`` because real ids need both —
328#: an Ollama tag (``qwen2.5-coder:32b``) and a gateway's namespaced id
329#: (``deepseek/deepseek-r1``). Neither character can do anything in a JSON string value:
330#: the body is built by :func:`json.dumps`, so there is no argv to split and no URL path
331#: to retarget. A leading dash and ``..`` stay refused so the same token can never be
332#: mistaken for a flag or a path if it is later logged, echoed, or reused.
333_BODY_MODEL_OK = frozenset("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._:/-")
336def is_safe_body_model_token(model: str | None) -> bool:
337 """True when ``model`` is safe as a JSON body field (``ollama``/hosted/compatible)."""
338 if not model or model.startswith("-") or ".." in model:
339 return False
340 return _BODY_MODEL_OK.issuperset(model)
343def model_reaches_argv(provider: providers_mod.Provider) -> bool:
344 """Does this provider's model end up on a command line or in a URL path?
346 Two places demand the strict ``[A-Za-z0-9._-]`` token, and only two: a **subprocess
347 argv** (``cli``/``profile``), where a stray character could read as another flag; and
348 ``google-api``'s **URL path**, the one vendor that interpolates the model into its
349 endpoint, where a ``/`` or a ``?`` could retarget the request or smuggle query
350 parameters onto a URL that also carries an API key header.
352 Everywhere else the model is a JSON string value, and applying the argv rule there is
353 not caution but breakage: it refuses ``ollama:qwen2.5-coder:32b`` and
354 ``openrouter:deepseek/deepseek-r1``, two ids this repository's own documentation tells
355 operators to use.
356 """
357 return _transport_of(provider) in ("cli", "profile") or provider.vendor == "google-api"
360def model_token_issue(provider: providers_mod.Provider, model: str | None) -> str | None:
361 """Why ``model`` may not be used with ``provider``, or ``None`` when it may."""
362 if model is None:
363 return None
364 if model_reaches_argv(provider):
365 if not agents.is_safe_model_token(model):
366 return (
367 f"model {model!r} is not a safe token for {provider.name!r}: it reaches a "
368 "command line or a URL path, so only [A-Za-z0-9._-] with no leading dash "
369 "is accepted"
370 )
371 return None
372 if not is_safe_body_model_token(model):
373 return (
374 f"model {model!r} is not a safe token for {provider.name!r}: a request-body "
375 "model accepts [A-Za-z0-9._:/-] with no leading dash and no '..'"
376 )
377 return None
380def _check_model(provider: providers_mod.Provider, model: str | None) -> None:
381 issue = model_token_issue(provider, model)
382 if issue is not None:
383 raise DelegateError("bad-model", issue)
386def _profile_provider(config, name: str) -> providers_mod.Provider:
387 """The one :class:`keel.providers.Provider` this project's profile ``name`` maps to."""
388 for provider in providers_mod.profile_providers(config):
389 if provider.name == name:
390 return provider
391 raise DelegateError( # pragma: no cover - profile_providers covers every profile key
392 "unknown-provider", f"profile {name!r} produced no provider record"
393 )
396def plan_run(
397 provider: providers_mod.Provider,
398 role: str,
399 prompt_path: str,
400 cwd: str | None = None,
401 timeout: int = DEFAULT_TIMEOUT_S,
402 effort: str | None = None,
403 model: str | None = None,
404 *,
405 profile: DelegateProfile | None = None,
406) -> RunPlan:
407 """Plan one delegate invocation. Pure; raises :class:`DelegateError` on bad input.
409 ``model`` is the per-run override (``--model``, or the ``vendor:model`` half of the
410 provider token) and wins over the provider's configured ``model``, matching the
411 precedence ship.md s4 documents. ``profile`` supplies the project profile behind a
412 ``profile``-source provider — see :attr:`Resolution.profile`.
413 """
414 if role not in ROLES:
415 raise DelegateError("bad-role", f"unknown role {role!r}; valid: {', '.join(ROLES)}")
416 if effort is not None and effort not in EFFORTS:
417 raise DelegateError("bad-effort", f"unknown effort {effort!r}; valid: {', '.join(EFFORTS)}")
418 if timeout <= 0:
419 raise DelegateError(
420 "bad-timeout", f"timeout must be a positive number of seconds: {timeout!r}"
421 )
422 if not prompt_path:
423 raise DelegateError("no-prompt", "--prompt-file is required")
424 # Normalised once, here, because a :class:`RunPlan` is a frozen JSON document: a
425 # ``Path`` reaching this far would pass every predicate, land in ``cwd`` and in the
426 # ``--add-dir`` argv, and then raise ``TypeError: Object of type PosixPath is not
427 # JSON serializable`` out of ``as_dict`` — at the point where the contract is
428 # printed rather than where the wrong type entered. Found by the gate review of
429 # #1134.
430 cwd = None if cwd is None else str(cwd)
431 effective = model or provider.model
432 _check_model(provider, effective)
433 read_only = role in READ_ONLY_ROLES
434 transport = _transport_of(provider)
435 effort, warnings = _effective_effort(provider, effort)
436 applied = _apply_effort(provider.vendor, transport, effort, effective)
437 effective = applied.model
438 warnings = warnings + applied.warnings
439 argv: tuple[str, ...] = ()
440 request: dict[str, Any] | None = None
441 stdin_mode: str | None = None
442 backed = read_only
443 if transport == "cli":
444 argv, stdin_mode = _builtin_argv(
445 provider,
446 read_only=read_only,
447 model=effective,
448 effort_args=applied.argv,
449 cwd=cwd,
450 timeout=timeout,
451 )
452 if cwd and not is_absolute_cwd(cwd) and provider.name == "agy":
453 warnings = warnings + (
454 f"cwd {cwd!r} is relative; agy resolves --add-dir against the directory "
455 "it is started in, so it would name a path inside the worktree instead of "
456 "the worktree. The flag is omitted and agy will edit its own scratch copy "
457 "(#1134) — pass an absolute path.",
458 )
459 elif transport == "profile":
460 argv, stdin_mode, backed, profile_warnings = _profile_argv(
461 provider, profile, read_only=read_only, model=effective
462 )
463 warnings = warnings + profile_warnings
464 else:
465 request = _request(provider, transport, model=effective, timeout=timeout, effort=applied)
466 return RunPlan(
467 provider=provider.name,
468 vendor=provider.label_vendor(),
469 role=role,
470 transport=transport,
471 prompt_path=prompt_path,
472 model=effective,
473 cwd=cwd,
474 timeout=timeout,
475 argv=argv,
476 request=request,
477 stdin_mode=stdin_mode,
478 read_only=read_only,
479 read_only_backed=backed,
480 effort=effort,
481 effort_applied=applied.applied,
482 warnings=warnings,
483 attribution=_attribution(provider, profile, effective),
484 )
487def _effective_effort(
488 provider: providers_mod.Provider, effort: str | None
489) -> tuple[str | None, tuple[str, ...]]:
490 """``--effort`` if given, else the provider's own configured default.
492 ``Provider.effort`` exists so an operator can say "this entry is my high-effort seat"
493 once, in the registry or the profile, instead of on every dispatch. A per-run
494 ``--effort`` still wins. An unrecognised configured value is a **warning, not an
495 error**: the registry is fail-soft everywhere else in #1011, and a typo in a file in
496 ``$HOME`` should not make every run of an otherwise usable provider fail.
497 """
498 if effort is not None or not provider.effort:
499 return effort, ()
500 if provider.effort in EFFORTS:
501 return provider.effort, ()
502 return None, (
503 f"{provider.name}: ignoring configured effort {provider.effort!r}; "
504 f"valid: {', '.join(EFFORTS)}",
505 )
508def _transport_of(provider: providers_mod.Provider) -> str:
509 """Which executor path a provider record lands on.
511 :class:`keel.providers.Provider` records the *shape* of a provider (``cli``/``api``/
512 ``local``); this is the finer question of which code runs it. The built-in ``ollama``
513 vendor gets its own HTTP path, while a registry ``local`` entry names an arbitrary
514 binary and is run exactly like a configured CLI — keel does not dial an address a
515 file names, so a local entry's only reachable surface is its command.
516 """
517 if provider.transport == "api":
518 return "api"
519 if provider.vendor == "ollama":
520 return "ollama"
521 if provider.source == "builtin":
522 return "cli"
523 return "profile"
526def is_absolute_cwd(cwd: str | None) -> bool:
527 """Is ``cwd`` a path agy's ``--add-dir`` can be given? (pure — no filesystem)
529 The child is started **inside** ``cwd`` — :func:`keel.delegaterun._run_argv` passes it
530 to the runner — so agy resolves a relative ``--add-dir`` against that directory:
531 ``--cwd worktrees/foo`` would name ``<root>/worktrees/foo/worktrees/foo``, and the
532 real worktree would go untouched exactly as it did before #1134. ``--add-dir .`` is
533 not a way out; it was measured, and the run ended on agy's timeout with the tree
534 unchanged, the same as passing no flag at all.
536 So the path has to be absolute, and this module cannot make it so: ``abspath`` reads
537 the *host's* working directory into a :class:`RunPlan` that is frozen, JSON-stable and
538 may be executed somewhere else. The CLI resolves it; this refuses what is left, and
539 :func:`plan_run` says why in a warning. Found by the gate review of #1134.
541 Both separators are accepted, because a plan built on one platform is a document that
542 may be read on another.
543 """
544 # ``str(cwd)`` rather than ``cwd``: the annotation says ``str | None``, but
545 # ``posixpath.isabs`` accepts a ``PathLike`` while ``re.match`` does not, so a
546 # *relative* ``Path`` would fall through the first test and raise ``TypeError`` out of
547 # the second while an absolute one short-circuited to ``True``. A predicate that
548 # answers for half its inputs and crashes for the other half is worse than one that
549 # refuses both. Found by the gate review of #1134.
550 text = "" if cwd is None else str(cwd)
551 return bool(text) and (posixpath.isabs(text) or _WINDOWS_ABSOLUTE.match(text) is not None)
554def _builtin_argv(
555 provider: providers_mod.Provider,
556 *,
557 read_only: bool,
558 model: str | None,
559 effort_args: tuple[str, ...] = (),
560 cwd: str | None = None,
561 timeout: int = DEFAULT_TIMEOUT_S,
562) -> tuple[tuple[str, ...], str]:
563 """The argv + stdin framing for one of the three built-in agent CLIs.
565 **claude's read-only invocation is an allow-list, and carries no permission bypass.**
566 The first cut paired ``--disallowed-tools Edit,Write,NotebookEdit,Bash`` with
567 ``--dangerously-skip-permissions``, on the reasoning that a denylist wins over the
568 bypass. That is a denylist of four names against a tool surface that grows with every
569 release — ``WebFetch``, an MCP server's tools, whatever ships next — and it hands the
570 bypass to everything it failed to enumerate. ``--allowed-tools`` inverts the default:
571 anything not named is refused, so a new tool is refused on the day it appears rather
572 than on the day someone remembers this list. With no tool outside the read set
573 reachable there is nothing left for a permission prompt to ask about, so the bypass is
574 dropped as well. Flag spelling verified against ``claude --help``
575 (``--allowedTools, --allowed-tools <tools...>``).
577 ``agy`` keeps ``--sandbox`` plus ``--dangerously-skip-permissions``: it exposes no
578 allow-list, the sandbox is the only read-only mechanism it documents, and without the
579 skip flag an unattended reviewer stops at an approval prompt. This is the same pairing
580 ai-jury's ``privilege.enforce_read_only`` uses for the vendor, and the reason
581 :attr:`RunPlan.read_only_backed` reports what backs the promise rather than asserting
582 that writes are impossible.
583 """
584 command = provider.command or provider.name
585 if provider.name == "claude":
586 argv = [command, "-p"]
587 if read_only:
588 argv += ["--output-format", "text"]
589 if model:
590 argv += ["--model", model]
591 if read_only:
592 argv += ["--allowed-tools", CLAUDE_ALLOWED_TOOLS]
593 else:
594 argv += ["--dangerously-skip-permissions"]
595 return tuple(argv), STDIN_TEXT
596 if provider.name == "codex":
597 sandbox = "read-only" if read_only else "workspace-write"
598 argv = [command, "exec", "-s", sandbox, "--skip-git-repo-check"]
599 if model:
600 argv += ["-m", model]
601 argv += list(effort_args)
602 return tuple(argv), STDIN_TEXT
603 # agy — the only remaining built-in CLI vendor (agents.CLI_VENDORS).
604 argv = [command]
605 if read_only:
606 argv += ["--sandbox"]
607 argv += ["--dangerously-skip-permissions", *AGY_STREAM_ARGS]
608 if model:
609 argv += ["--model", model]
610 # `--add-dir` is what makes the working directory keel named the one agy edits
611 # (#1134). Without it agy works in `~/.gemini/antigravity-cli/scratch/<basename>` —
612 # its own copy — so an `implement` run returned prose about files it had changed
613 # while the worktree keel handed it stayed clean, and the first thing downstream
614 # would have seen is a pull request with no diff. Measured, not inferred: three
615 # runs of one prompt against a standalone clone, a linked worktree, and a linked
616 # worktree with this flag. Only the third edited the real file, and only it made
617 # no scratch copy — the other two never touched the directory at all and ended on
618 # agy's own print timeout.
619 # Absolute only — see :func:`is_absolute_cwd`. The caller makes it absolute; a plan
620 # that reached here with a relative one carries the warning instead of an argv the
621 # child would resolve against itself.
622 if is_absolute_cwd(cwd):
623 # ``str`` by the time it reaches here — :func:`plan_run` normalises it — and
624 # spelled again because this helper is reachable from a test with any value, and
625 # ``RunPlan.argv`` is ``tuple[str, ...]``.
626 argv += ["--add-dir", str(cwd)]
627 # And `--print-timeout` is why keel's own `--timeout` used to mean nothing here:
628 # agy's print mode defaults to 5m and stops on its own, so `--timeout 900` against
629 # a real brief died at 298s with agy's `timeout waiting for response` — keel's
630 # bound never reached the process it was bounding. Go duration spelling, per
631 # `agy --help` ("default 5m0s").
632 #
633 # Unconditional, because :func:`plan_run` refuses a non-positive timeout before
634 # reaching here — a guard on it would be a branch no input can take, which the
635 # 100 % coverage bar reports as exactly what it is. Found by the gate review.
636 argv += ["--print-timeout", f"{timeout}s"]
637 return tuple(argv), STDIN_STREAM_JSON
640def _profile_argv(
641 provider: providers_mod.Provider,
642 profile: DelegateProfile | None,
643 *,
644 read_only: bool,
645 model: str | None,
646) -> tuple[tuple[str, ...], str | None, bool, tuple[str, ...]]:
647 """The argv + framing + read-only backing for an operator-configured binary.
649 The dangerous case is a profile that has ``args`` and no ``review_args``:
650 :meth:`keel.config.DelegateProfile.role_args` **falls back to ``args``**, which is the
651 implementer's flag set — ``aider``'s ``--yes-always``, ``cursor-agent``'s ``--force``.
652 A reviewer invoked with those can edit the checkout it was asked to read.
654 So the question asked here is *"did the operator configure a read-only invocation?"* —
655 ``profile.review_args is None`` — and **not** *"is the argv empty?"*. The first cut
656 asked the second, so exactly the dangerous case produced a full implementer argv,
657 ``read_only: true``, and no warning at all: the fallback made ``role_args`` non-empty,
658 which read as "configured". A profile whose ``review_args`` is an explicit empty list
659 is a deliberate choice — "this CLI needs no flags to be a reviewer" — and is backed.
660 """
661 command = provider.command
662 if not command:
663 raise DelegateError("bad-provider", f"provider {provider.name!r} names no command")
664 if profile is not None:
665 role_args = profile.role_args(review=read_only)
666 configured = profile.review_args is not None
667 prompt_mode = profile.prompt_mode
668 else:
669 # A registry entry has no implementer `args` to fall back to, so an empty tuple
670 # here really is "nothing configured" rather than a fallback in disguise.
671 role_args = provider.review_args if read_only else ()
672 configured = bool(provider.review_args)
673 prompt_mode = DEFAULT_PROMPT_MODE
674 argv = [command, *role_args]
675 if model and provider.model_arg:
676 argv += [provider.model_arg, model]
677 warnings: tuple[str, ...] = ()
678 backed = read_only and configured
679 if read_only and not configured:
680 shared = " it is running with the implementer's own args" if role_args else ""
681 warnings = (
682 f"{provider.name}: no read-only invocation is configured (review_args), so"
683 f"{shared or ' the command runs with its default permissions'} — keel cannot "
684 "enforce read-only for an arbitrary CLI. Set review_args, treat this "
685 "reviewer's output as advisory, and re-check the worktree is clean afterwards",
686 )
687 stdin_mode = STDIN_TEXT if prompt_mode == DEFAULT_PROMPT_MODE else None
688 return tuple(argv), stdin_mode, backed, warnings
691def _request(
692 provider: providers_mod.Provider,
693 transport: str,
694 *,
695 model: str | None,
696 timeout: int,
697 effort: Effort,
698) -> dict[str, Any]:
699 """The HTTP call description for the ``api`` and ``ollama`` transports."""
700 if not model:
701 raise DelegateError(
702 "no-model",
703 f"provider {provider.name!r} generates text over HTTP and names no model; "
704 "pass --model or provider:model",
705 )
706 if transport == "ollama":
707 # Hardcoded loopback constant, never provider.endpoint: the tag listing #1011
708 # probes and the generation endpoint are two paths on the same fixed origin.
709 return {
710 "vendor": "ollama",
711 "model": model,
712 "endpoint": OLLAMA_GENERATE_URL,
713 "timeout": timeout,
714 }
715 max_tokens = DEFAULT_MAX_TOKENS
716 if effort.budget is not None:
717 # Anthropic rejects max_tokens <= thinking.budget_tokens before generating a
718 # token, and Gemini's answer shares the output cap with its thinking.
719 max_tokens = max(max_tokens, effort.budget + THINKING_HEADROOM_TOKENS)
720 return {
721 "vendor": provider.vendor,
722 "model": model,
723 "endpoint": provider.endpoint,
724 "api_key_env": provider.api_key_env,
725 "max_tokens": max_tokens,
726 "timeout": timeout,
727 "extra_payload": effort.payload,
728 }
731def _apply_effort(vendor: str, transport: str, effort: str | None, model: str | None) -> Effort:
732 """Map ``--effort`` onto the vendor's own spelling.
734 Every vendor spells reasoning effort differently and several cannot spell it at all;
735 an unsupported request is a warning plus ``applied: False``, never a silent no-op — a
736 caller that asked for ``high`` and got the default must be able to see that from the
737 JSON it parses.
738 """
739 if effort is None:
740 return Effort(model, {}, None, False, ())
741 if vendor == "agy":
742 return _agy_effort(effort, model)
743 if vendor == "codex":
744 # A config override rather than a flag: `codex exec` takes `-c key=value`, and
745 # `model_reasoning_effort` is the key it recognises (verified by round-tripping a
746 # real and a bogus key through `--strict-config`).
747 return Effort(model, {}, None, True, (), ("-c", f"{CODEX_EFFORT_CONFIG}={effort}"))
748 if vendor == "anthropic-api":
749 budget = ANTHROPIC_THINKING_BUDGET[effort]
750 payload = {"thinking": {"type": "enabled", "budget_tokens": budget}}
751 return Effort(model, payload, budget, True, ())
752 if vendor in ("openai-api", OPENAI_COMPATIBLE):
753 return Effort(model, {"reasoning_effort": effort}, None, True, ())
754 if vendor == "google-api":
755 budget = GOOGLE_THINKING_BUDGET[effort]
756 payload = {"generationConfig": {"thinkingConfig": {"thinkingBudget": budget}}}
757 return Effort(model, payload, budget, True, ())
758 return Effort(
759 model,
760 {},
761 None,
762 False,
763 (
764 f"--effort {effort} is not supported by {vendor} over the {transport} "
765 "transport; the run used the provider's default reasoning effort",
766 ),
767 )
770def _agy_effort(effort: str, model: str | None) -> Effort:
771 """agy spells effort as a model suffix (``gemini-3.8-flash-high``), not as a flag."""
772 if not model:
773 return Effort(
774 model,
775 {},
776 None,
777 False,
778 (
779 f"--effort {effort} needs a model for agy, which spells effort as a "
780 "model suffix; pass --model or provider:model",
781 ),
782 )
783 for suffix in _EFFORT_SUFFIXES:
784 if model.endswith(suffix):
785 if suffix == f"-{effort}":
786 return Effort(model, {}, None, True, ())
787 return Effort(
788 model,
789 {},
790 None,
791 True,
792 (
793 f"model {model!r} already selects effort {suffix[1:]!r}; the model's "
794 f"own suffix wins over --effort {effort}",
795 ),
796 )
797 return Effort(f"{model}-{effort}", {}, None, True, ())
800def _attribution(
801 provider: providers_mod.Provider,
802 profile: DelegateProfile | None,
803 model: str | None,
804) -> dict[str, str | None]:
805 """The attribution record, computed by :mod:`keel.agents` so a host cannot drift.
807 A configured provider also records **which** entry ran under ``delegate_profile``,
808 never ``profile``: the ship run record already means the workflow profile
809 (``standard``/``compound``) by that name, and writing the CLI's name there would
810 silently overwrite it.
812 A registry entry names its **label vendor** (#1129): ``vendor`` there is the
813 transport the schema requires — ``cli`` for every local coding-agent CLI — so two
814 entries driving Grok and GPT through the same binary both reported ``agent:cli``,
815 and ``review-vendor-distinctness``, which is the rule keel most depends on, could
816 not tell them apart. ``vendor_label`` is how an entry says which maker it reaches;
817 unset, it stays the generic token, which is what every built-in already is.
818 """
819 if profile is not None:
820 return agents.profile_attribution(provider.name, profile, model)
821 record = agents.attribution(provider.label_vendor(), model)
822 if provider.source != "builtin":
823 record["delegate_profile"] = provider.name
824 return record
827def stream_json_frame(prompt: str) -> str:
828 """One NDJSON user frame for agy's ``--input-format stream-json`` stdin.
830 Shape ported from ai-jury's ``AgyAdapter._stdin_for``, verified there against
831 agy 1.1.22.
832 """
833 return json.dumps({"event": "user", "message": {"role": "user", "content": prompt}}) + "\n"
836def parse_stream_json(raw: str) -> str:
837 """The response text out of agy's NDJSON stdout, falling back to the raw stream.
839 Falls back rather than returning empty: a truncated stream must surface as output an
840 operator can read, not as a silent abstention. An empty review counts as a review,
841 and a review read as an approval is the expensive failure (ai-jury #625).
842 """
843 response, saw_result = None, False
844 for line in (raw or "").splitlines():
845 line = line.strip()
846 if not line:
847 continue
848 try:
849 event = json.loads(line)
850 except ValueError:
851 continue
852 if isinstance(event, dict) and event.get("event") == "result":
853 saw_result = True
854 result = event.get("result")
855 if isinstance(result, dict):
856 response = result.get("response")
857 if saw_result and isinstance(response, str):
858 return response
859 return raw
862def parse_ollama_response(data: Any) -> str | None:
863 """The completion out of an Ollama ``/api/generate`` payload (``None`` if malformed)."""
864 if not isinstance(data, dict):
865 return None
866 text = data.get("response")
867 return text if isinstance(text, str) and text else None
870def ollama_payload(model: str, prompt: str) -> dict[str, Any]:
871 """The ``/api/generate`` body. ``stream: false`` — keel wants one document, not NDJSON."""
872 return {"model": model, "prompt": prompt, "stream": False}