From 9417925b45dd73792adba43032bb147dfa9e291b Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:14:21 -0400 Subject: [PATCH 01/27] feat(model-probe): discover model controls from the harness at init time `nightly init` now asks each installed host CLI what it accepts instead of carrying a table of vendor flags and model ids that goes stale. The config it writes therefore reflects the harness the operator actually works in. What the probe reads, from the host's own `--help`: - The model-selection flag. Discovered and stored as `model_tiers..flag`; `build_argv` emits a discovered flag, never a guessed one. This immediately paid for itself: it found `--model` on opencode and gemini, neither of which could be verified by hand. - The model vocabulary. Claude Code enumerates its own aliases and ids in the `--model` help text ('fable', 'opus', 'sonnet', 'claude-fable-5'); those are parsed out and ranked into lite / coding / reasoning by family. - Which harness is running init, from environment markers (`CLAUDECODE`, `CODEX_HOME`, ...), so the initializing harness is probed first and its findings lead the config. Precedence favors reproducibility: a *pinned* discovered id (`claude-opus-5`) overrides the seeded default, but a bare alias (`opus`) does not - aliases float to whatever shipped most recently, which is convenient interactively and wrong for an overnight run whose model should still be knowable in the morning. Discovery can only add certainty; it merges over the seeded defaults rather than replacing them, and any probe failure degrades to the seeded template so init can never fail because a host CLI misbehaved. Host coverage extended to all seven major harnesses: Claude, Codex, Cursor, Gemini, OpenCode, Pi, Hermes (plus Antigravity). `pi` and `hermes` are recognized at the routing layer only - they ship no integration package yet, which the `HostId` docstring states explicitly. Also lands the rest of RFC 007 Phase B's core: - `nightly dispatch start` resolves plan tier -> role default -> host binding, passes the model id with the discovered flag, and appends the tier's effort directive to the prompt. - `nightly specialist --tier ` appends the same directive. - Effort ships as prompt text, not a CLI flag: the flag surface differs per host and several expose none, while prompt text works everywhere and no-ops harmlessly where unsupported. Performance: the first version made every `nightly init` shell out per host, taking the suite from 17s to 491s. Fixed by memoizing the default probe for the process lifetime and reading each host's help once for both facts rather than twice. 1200 tests pass (38 new in test_model_probe.py); ruff, pyrefly, and `nightly verify` all clean. Co-Authored-By: Claude Opus 5 (1M context) --- packages/nightly-core/src/nightly_core/cli.py | 107 +++- .../nightly-core/src/nightly_core/config.py | 104 +++- .../nightly-core/src/nightly_core/contract.py | 22 +- .../nightly-core/src/nightly_core/dispatch.py | 43 +- .../src/nightly_core/model_probe.py | 459 ++++++++++++++++++ .../nightly-core/src/nightly_core/routing.py | 44 ++ packages/nightly-core/tests/test_contract.py | 5 + .../nightly-core/tests/test_model_probe.py | 291 +++++++++++ 8 files changed, 1039 insertions(+), 36 deletions(-) create mode 100644 packages/nightly-core/src/nightly_core/model_probe.py create mode 100644 packages/nightly-core/tests/test_model_probe.py diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index 7fb60f6..bfe03a4 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -43,7 +43,7 @@ import sys from collections.abc import Callable from pathlib import Path -from typing import Annotated +from typing import Annotated, cast import typer @@ -62,6 +62,7 @@ from nightly_core.contract import ( HostId, InstallScope, + ModelTier, NightlyHostIntegration, SpecialistRole, ) @@ -112,6 +113,39 @@ _DEFAULT_CONFIG_YML = DEFAULT_CONFIG_YML +def _render_discovered_config() -> str: + """Render config.yml with model controls probed from the live harness. + + RFC 007's tier table is only useful if the ids and the + model-selection flag match the harness the operator actually runs in. + Rather than ship a table that goes stale, `nightly init` asks each + installed host CLI what it accepts (`nightly_core.model_probe`) and + writes that down. Discovery only ever *adds* certainty: probe results + merge over the seeded defaults, and a pinned default outranks a + floating alias the help text happened to mention. + + Any probe failure degrades to the seeded template — init must never + fail because a host CLI misbehaved. + """ + from nightly_core.config import ( # noqa: PLC0415 - lazy + DEFAULT_TIER_MODELS, + render_config_yml, + ) + + try: + from nightly_core.model_probe import ( # noqa: PLC0415 - lazy + discover_tier_bindings, + merge_discovered_tiers, + ) + + discovered, flags = discover_tier_bindings() + except Exception: # never let a probe break `init` + return DEFAULT_CONFIG_YML + if not discovered and not flags: + return DEFAULT_CONFIG_YML + return render_config_yml(merge_discovered_tiers(DEFAULT_TIER_MODELS, discovered), flags) + + _NIGHTLY_SUBDIRS: tuple[str, ...] = ("runs", "plans", "atlas", "memory", "prompts") # Display tuning for `nightly triage` — wider issue titles get elided. @@ -204,7 +238,7 @@ def _ensure_config(nightly: Path) -> bool: config = nightly / "config.yml" if config.exists(): return False - config.write_text(_DEFAULT_CONFIG_YML, encoding="utf-8") + config.write_text(_render_discovered_config(), encoding="utf-8") return True @@ -698,13 +732,41 @@ def specialist( SpecialistRole, typer.Argument(help="Specialist role: implementer | tester | reviewer | researcher."), ], + tier: Annotated[ + str | None, + typer.Option( + "--tier", + help=( + "Model tier (lite | coding | reasoning). Appends the tier's " + "deliberation directive to the prompt. Omit to use the " + "role's default tier without the directive." + ), + ), + ] = None, ) -> None: """Print the system prompt for a specialist role. Inside Claude Code, the Nightly skill uses this to seed a Task-tool sub-agent with the right role-specific instructions. + + With `--tier`, the tier's reasoning-effort directive is appended (RFC + 007 Resolved #10/#11). The role instructions themselves don't change + between tiers — what changes is how much the sub-agent is told to + deliberate before acting. """ - typer.echo(specialist_prompt(role), nl=False) + prompt = specialist_prompt(role) + if tier is not None: + from nightly_core.contract import MODEL_TIERS # noqa: PLC0415 - lazy + from nightly_core.routing import effort_directive # noqa: PLC0415 - lazy + + if tier not in MODEL_TIERS: + typer.echo(f"unknown tier: {tier} (expected one of {', '.join(MODEL_TIERS)})", err=True) + raise typer.Exit(code=2) + from nightly_core.config import load_model_tier_config # noqa: PLC0415 - lazy + + cfg = load_model_tier_config(repo_root()) + prompt = f"{prompt}\n{effort_directive(cfg.effort[cast('ModelTier', tier)])}\n" + typer.echo(prompt, nl=False) @app.command() @@ -1518,6 +1580,24 @@ def worktree_doctor_cmd( # ── dispatch — background specialist sub-processes ──────────────────────── +def _plan_model_tier(slug: str, root: Path) -> ModelTier | None: + """Read `model_tier:` from the task's plan.md frontmatter, if present. + + Returns None whenever the plan can't be located or declares nothing — + the dispatch then falls back to the specialist role's default tier, + which is the intended common case (RFC 007 Resolved #6). + """ + from nightly_core.plans import list_plans # noqa: PLC0415 - lazy + + try: + for plan in list_plans(root=root): + if plan.slug == slug or plan.slug.endswith(f"-{slug}"): + return plan.model_tier + except Exception: # a malformed plan must not block the dispatch + return None + return None + + @dispatch_app.command(name="start") def dispatch_start_cmd( slug: Annotated[ @@ -1571,11 +1651,27 @@ def dispatch_start_cmd( `nightly dispatch status` to poll, `nightly dispatch tail` to follow output, `nightly dispatch wait` to block. """ + from nightly_core.config import load_model_tier_config # noqa: PLC0415 - lazy from nightly_core.dispatch import start_background # noqa: PLC0415 - lazy + from nightly_core.routing import ( # noqa: PLC0415 - lazy + effort_directive, + resolve_model_for_task, + ) from nightly_core.specialists import specialist_prompt # noqa: PLC0415 - lazy root = repo_root() body = prompt or _default_dispatch_prompt(role=role, slug=slug, specialist=specialist_prompt) + + tier_cfg = load_model_tier_config(root) + resolved = resolve_model_for_task( + host=host, + role=role, + config=tier_cfg, + plan_tier=_plan_model_tier(slug, root), + ) + if tier_cfg.enabled: + body = f"{body}\n{effort_directive(resolved.effort)}\n" + try: result = start_background( slug, @@ -1584,6 +1680,8 @@ def dispatch_start_cmd( prompt=body, root=root, cwd=cwd, + model=resolved.model, + model_flag=tier_cfg.flag_for(host), ) except RuntimeError as exc: typer.echo(f"✗ {exc}", err=True) @@ -1595,6 +1693,9 @@ def dispatch_start_cmd( typer.echo(f"slug={result.slug}") typer.echo(f"role={result.role}") typer.echo(f"host={result.host}") + typer.echo(f"tier={resolved.tier}") + typer.echo(f"model={resolved.model or ''}") + typer.echo(f"effort={resolved.effort}") def _default_dispatch_prompt( diff --git a/packages/nightly-core/src/nightly_core/config.py b/packages/nightly-core/src/nightly_core/config.py index 3c84d5e..14af55d 100644 --- a/packages/nightly-core/src/nightly_core/config.py +++ b/packages/nightly-core/src/nightly_core/config.py @@ -23,6 +23,7 @@ from nightly_core.paths import nightly_dir __all__ = [ + "DEFAULT_CONFIG_YML", "DEFAULT_MODEL_CONTEXT_TOKENS", "DEFAULT_TIER_EFFORT", "DEFAULT_TIER_MODELS", @@ -43,6 +44,7 @@ "load_parallelism_config", "load_vault_config", "load_worktree_config", + "render_config_yml", ] _log = logging.getLogger(__name__) @@ -402,6 +404,17 @@ class ModelTierConfig: ) """Per-tier reasoning effort, merged over `DEFAULT_TIER_EFFORT`.""" + flags: dict[HostId, str] = field(default_factory=dict) + """Per-host model-selection flag, discovered by `nightly init` from the + host CLI's own `--help` (see `nightly_core.model_probe`). Absent means + "this host has no known model flag" — dispatch then runs on the host's + default model, with the tier still applied via the effort directive in + the prompt. Nightly never guesses a flag.""" + + def flag_for(self, host: HostId) -> str | None: + """The discovered model-selection flag for `host`, if any.""" + return self.flags.get(host) + def binding(self, host: HostId, tier: ModelTier) -> TierBinding: """Resolve `(model, effort)` for `host` at `tier`. @@ -451,6 +464,8 @@ def _merge_tier_models(block: dict[str, Any], path: Path) -> dict[HostId, dict[M host = cast("HostId", raw_host) merged = dict(models.get(host, {})) for raw_tier, model_id in tiers.items(): + if raw_tier == "flag": + continue # handled by `_merge_tier_flags` if raw_tier not in MODEL_TIERS: _log.warning("%s: unknown model tier %r under model_tiers.%s", path, raw_tier, host) continue @@ -461,6 +476,19 @@ def _merge_tier_models(block: dict[str, Any], path: Path) -> dict[HostId, dict[M return models +def _merge_tier_flags(block: dict[str, Any]) -> dict[HostId, str]: + """Collect per-host `flag:` entries written by `nightly init`'s probe.""" + known_hosts = set(get_args(HostId)) + flags: dict[HostId, str] = {} + for raw_host, tiers in block.items(): + if raw_host not in known_hosts or not isinstance(tiers, dict): + continue + flag = str(tiers.get("flag", "")).strip() + if flag.startswith("-"): + flags[cast("HostId", raw_host)] = flag + return flags + + def load_model_tier_config(root: Path | None = None) -> ModelTierConfig: """Parse the `model_tiers:` block from `/.nightly/config.yml`. @@ -489,6 +517,7 @@ def load_model_tier_config(root: Path | None = None) -> ModelTierConfig: enabled=bool(block.get("enabled", defaults.enabled)), models=_merge_tier_models(block, path), effort=_merge_tier_effort(block, path), + flags=_merge_tier_flags(block), ) @@ -784,7 +813,7 @@ def _coerce_int(key: str, default: int) -> int: # ── default config template ────────────────────────────────────────────── -DEFAULT_CONFIG_YML = """\ +_CONFIG_YML_TEMPLATE = """\ # .nightly/config.yml — written by `nightly init`. Edit as needed. # See `.nightly/config.yml.example` (if present) for the full schema, or # `.planning/brainstorm.html` §05 for the design rationale. @@ -891,20 +920,7 @@ def _coerce_int(key: str, default: int) -> int: # Hosts absent from this block (codex, gemini, antigravity) fall through # to the host CLI's own default model and log a friction note — wire your # own ids below to enable routing for them. -model_tiers: - enabled: true - effort: - lite: low - coding: low - reasoning: xhigh - claude: - lite: claude-haiku-4-5 - coding: claude-sonnet-5 - reasoning: claude-opus-5 - # codex: - # lite: - # coding: - # reasoning: +__MODEL_TIERS_BLOCK__ # parallelism caps how wide the fleet runs (RFC 012). Defaults lean wide: # background dispatch keeps the chat free, worktrees isolate the @@ -960,12 +976,52 @@ def _coerce_int(key: str, default: int) -> int: enabled: true context_token_cap: 256000 """ -"""Canonical `.nightly/config.yml` scaffold written by `nightly init` -and by `nightly doctor --fix`. - -Lives here rather than in `cli.py` because both writers need it and -`cli` imports `doctor`, not the reverse. Keeping two copies is what -let the doctor template silently drift out of sync with init's — it -had lost the `vault:` and `worktree:` blocks entirely, so a repo -repaired by `doctor --fix` got a different config than a freshly -initialized one.""" + + +def render_config_yml( + tier_models: dict[HostId, dict[ModelTier, str]] | None = None, + model_flags: dict[HostId, str] | None = None, +) -> str: + """Render `.nightly/config.yml`, with the `model_tiers:` block filled in. + + `nightly init` passes what `nightly_core.model_probe` discovered from + the harness that is running it, so the written config names the models + that harness actually offers and the model-selection flag it actually + accepts — rather than a table Nightly would have to keep current by + hand. With no arguments this renders the seeded defaults, which is + what `DEFAULT_CONFIG_YML` is. + """ + models = tier_models if tier_models is not None else DEFAULT_TIER_MODELS + flags = model_flags or {} + + lines = [ + "model_tiers:", + " enabled: true", + " effort:", + ] + lines += [f" {tier + ':':<11}{DEFAULT_TIER_EFFORT[tier]}" for tier in MODEL_TIERS] + + for host in sorted(set(models) | set(flags)): + host_tiers = models.get(host, {}) + if not host_tiers and host not in flags: + continue + lines.append(f" {host}:") + if host in flags: + # Discovered from ` --help`; `build_argv` emits it verbatim. + lines.append(f" {'flag:':<11}{flags[host]}") + lines += [ + f" {tier + ':':<11}{host_tiers[tier]}" for tier in MODEL_TIERS if tier in host_tiers + ] + + if not any(models.values()) and not flags: + lines.append(" # no host CLI was detected on PATH at init time.") + lines.append(" # Re-run `nightly doctor` after installing one, or") + lines.append(" # add `: {lite:, coding:, reasoning:}` by hand.") + + return _CONFIG_YML_TEMPLATE.replace("__MODEL_TIERS_BLOCK__", "\n".join(lines)) + + +DEFAULT_CONFIG_YML = render_config_yml() +"""Canonical `.nightly/config.yml` scaffold with seeded (undiscovered) +model tiers. `nightly init` prefers `render_config_yml(...)` with probe +results; this is the fallback and the shape tests pin.""" diff --git a/packages/nightly-core/src/nightly_core/contract.py b/packages/nightly-core/src/nightly_core/contract.py index e1311af..6aecace 100644 --- a/packages/nightly-core/src/nightly_core/contract.py +++ b/packages/nightly-core/src/nightly_core/contract.py @@ -19,12 +19,28 @@ from nightly_core.headless import HeadlessResult -HostId = Literal["claude", "codex", "cursor", "opencode", "antigravity", "gemini"] -"""The six supported interactive hosts. +HostId = Literal[ + "claude", + "codex", + "cursor", + "opencode", + "antigravity", + "gemini", + "pi", + "hermes", +] +"""The supported interactive hosts. `antigravity` and `gemini` both write under `.gemini/` — the former is the desktop IDE's managed-agent surface (`.gemini/antigravity/agents/`), the -latter is vanilla Gemini CLI custom commands (`.gemini/commands/`).""" +latter is vanilla Gemini CLI custom commands (`.gemini/commands/`). + +`pi` and `hermes` are recognized at the *routing* layer — `nightly init` +probes them for model controls and tiers them like any other harness (see +`nightly_core.model_probe`) — but they do not yet ship integration +packages, so skill install and keep-alive hooks are unavailable for them. +Listing them here is what lets the model-tier config address them at all; +a host with no integration simply has no loader in `cli._HOST_LOADERS`.""" SpecialistRole = Literal["implementer", "tester", "reviewer", "researcher"] """Roles dispatched as sub-agents through the host's native primitive.""" diff --git a/packages/nightly-core/src/nightly_core/dispatch.py b/packages/nightly-core/src/nightly_core/dispatch.py index 934f318..1c35cdb 100644 --- a/packages/nightly-core/src/nightly_core/dispatch.py +++ b/packages/nightly-core/src/nightly_core/dispatch.py @@ -107,12 +107,31 @@ class BackgroundDispatchResult: # ── per-host argv ──────────────────────────────────────────────────────── -def build_argv(host: HostId, prompt: str, *, session_id: str | None = None) -> list[str] | None: # noqa: PLR0911 - one return per host backend is the whole point +def build_argv( # noqa: PLR0911, PLR0912 - one branch per host backend is the whole point + host: HostId, + prompt: str, + *, + session_id: str | None = None, + model: str | None = None, + model_flag: str | None = None, +) -> list[str] | None: """Build the headless argv for `host`. Returns None when the host has no usable headless backend yet (cursor, antigravity). Reuses the same flags each host's `run_headless` already invokes — see the integration packages for canonical references. + + `model` is the RFC 007 tier-resolved model id and `model_flag` is the + option that carries it — discovered from the host CLI's own `--help` + by `nightly init` (see `nightly_core.model_probe`) and stored under + `model_tiers..flag`. Both must be present for the id to be + applied: Nightly emits a discovered flag, never a guessed one, because + a wrong flag is a hard spawn failure at 3am while an omitted one still + gets the work done on the host's default model. Tier intent still + reaches those hosts through the effort directive in the prompt. + + Claude Code's `--model` is the one flag verified in-tree, so it is the + fallback when discovery produced nothing for that host. """ if host == "claude": binary = shutil.which("claude") @@ -127,6 +146,8 @@ def build_argv(host: HostId, prompt: str, *, session_id: str | None = None) -> l "--permission-mode", "acceptEdits", ] + if model: + argv += [model_flag or "--model", model] if session_id: argv += ["--session-id", session_id] return argv @@ -135,7 +156,7 @@ def build_argv(host: HostId, prompt: str, *, session_id: str | None = None) -> l binary = shutil.which("codex") if binary is None: return None - return [ + argv = [ binary, "exec", "--json", @@ -143,20 +164,28 @@ def build_argv(host: HostId, prompt: str, *, session_id: str | None = None) -> l "workspace-write", "--ask-for-approval", "never", - prompt, ] + if model and model_flag: + argv += [model_flag, model] + return [*argv, prompt] if host == "opencode": binary = shutil.which("opencode") if binary is None: return None - return [binary, "run", prompt, "--format", "json"] + argv = [binary, "run", prompt, "--format", "json"] + if model and model_flag: + argv += [model_flag, model] + return argv if host == "gemini": binary = shutil.which("gemini") if binary is None: return None - return [binary, "--prompt", prompt] + argv = [binary, "--prompt", prompt] + if model and model_flag: + argv += [model_flag, model] + return argv # cursor + antigravity don't expose a usable headless CLI today. # Callers can fall back to the host's blocking primitive (Background @@ -178,6 +207,8 @@ def start_background( # noqa: PLR0913 - dispatch primitive needs every dimensio cwd: Path | None = None, session_id: str | None = None, now: datetime | None = None, + model: str | None = None, + model_flag: str | None = None, popen_factory: object | None = None, ) -> BackgroundDispatchResult: """Spawn the host's headless CLI as a detached background process. @@ -190,7 +221,7 @@ def start_background( # noqa: PLR0913 - dispatch primitive needs every dimensio `popen_factory` is injectable for tests — defaults to `subprocess.Popen`. Production callers leave it unset. """ - argv = build_argv(host, prompt, session_id=session_id) + argv = build_argv(host, prompt, session_id=session_id, model=model, model_flag=model_flag) if argv is None: msg = ( f"no background dispatch backend for host '{host}'. " diff --git a/packages/nightly-core/src/nightly_core/model_probe.py b/packages/nightly-core/src/nightly_core/model_probe.py new file mode 100644 index 0000000..7936ab1 --- /dev/null +++ b/packages/nightly-core/src/nightly_core/model_probe.py @@ -0,0 +1,459 @@ +"""Discover each host CLI's model-selection control at `nightly init` time. + +RFC 007 routes a dispatch to a model id. Actually *applying* that id means +knowing the flag the host's headless CLI accepts — and that is exactly the +kind of fact Nightly should not hardcode. Vendors rename flags, and a +wrong flag is a hard spawn failure in the middle of the night, whereas the +right one is discoverable in a fraction of a second from the CLI itself. + +So: probe. Run the host binary's `--help`, look for a model-selection +option, and record what we find. `nightly init` and `nightly doctor` both +run this and write the result into `model_tiers..flag`, so the +config carries a discovered fact rather than a maintained guess. + +The probe is deliberately conservative. It reports only what it can read +out of the CLI's own help text; a host whose help exposes no recognizable +model option yields `None`, and dispatch falls through to that host's +default model with the tier still applied via the prompt-side effort +directive. Never guess a flag. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +from collections.abc import Callable, Sequence +from dataclasses import dataclass +from typing import get_args + +from nightly_core.contract import MODEL_TIERS, HostId, ModelTier + +__all__ = [ + "HARNESS_ENV_MARKERS", + "HELP_INVOCATIONS", + "TIER_FAMILIES", + "ModelControl", + "assign_tiers", + "detect_harness", + "discover_tier_bindings", + "is_pinned", + "merge_discovered_tiers", + "probe_all", + "probe_model_control", +] + + +HARNESS_ENV_MARKERS: dict[HostId, tuple[str, ...]] = { + "claude": ("CLAUDECODE", "CLAUDE_CODE_ENTRYPOINT"), + "codex": ("CODEX_HOME", "CODEX_SANDBOX"), + "cursor": ("CURSOR_TRACE_ID", "CURSOR_AGENT"), + "gemini": ("GEMINI_CLI",), + "opencode": ("OPENCODE", "OPENCODE_BIN"), + "antigravity": ("ANTIGRAVITY_AGENT",), + "pi": ("PI_SESSION", "PI_AGENT"), + "hermes": ("HERMES_SESSION", "HERMES_AGENT"), +} +"""Environment variables that identify the harness running `nightly init`. + +Presence of any one marks that host as the initializing harness. Only the +Claude Code markers are verified first-hand; the rest are best-effort and +cost nothing when wrong — an unmatched harness simply falls back to +probing every installed host rather than leading with one.""" + + +TIER_FAMILIES: dict[ModelTier, tuple[str, ...]] = { + "lite": ("haiku", "mini", "flash", "small"), + "coding": ("sonnet", "coder", "pro"), + "reasoning": ("opus", "fable", "mythos", "reasoning", "ultra"), +} +"""Model-family substrings that map a discovered model id onto a tier. + +Ordered by preference within each tier: the first family that matches a +discovered id wins. `reasoning` leads with `opus` rather than the +higher-capability `fable`/`mythos` deliberately — the reasoning tier is +the *judgment* tier, not the most-expensive-available tier, and Opus is +the intended default for it. An operator who wants the ceiling edits one +line in `.nightly/config.yml`.""" + + +HELP_INVOCATIONS: dict[HostId, tuple[tuple[str, ...], ...]] = { + # Most hosts put the model flag on the top-level help; the ones whose + # headless entry point is a subcommand hide it there instead, so try + # the subcommand first and fall back to the root. + "claude": (("--help",),), + "codex": (("exec", "--help"), ("--help",)), + "opencode": (("run", "--help"), ("--help",)), + "gemini": (("--help",),), + "cursor": (("agent", "--help"), ("--help",)), + "antigravity": (("--help",),), + "pi": (("run", "--help"), ("--help",)), + "hermes": (("run", "--help"), ("--help",)), +} +"""Per-host argv suffixes to try when reading help text, in order.""" + + +# Matches an option line like: +# --model Model for the current session +# -m, --model MODEL Model to use +# Anchored at the start of an option so prose mentioning "--model" in a +# description doesn't produce a false positive. +_MODEL_OPTION = re.compile( + r"^\s*(?:(-[A-Za-z])\s*,\s*)?(--model(?:[-_][a-z]+)?)\b", + re.MULTILINE, +) + +_HELP_TIMEOUT_S = 5.0 + +# Probing shells out once per installed host, which is cheap in absolute +# terms but not free — and `nightly init` / `doctor` can run many times in +# one process (notably across a test session). The result is a property of +# the machine, not of the repo, so memoize the default probe for the life +# of the process. Callers that pass explicit hosts/runner/which bypass the +# cache entirely, which is what keeps tests deterministic. +_DEFAULT_DISCOVERY: tuple[dict[HostId, dict[ModelTier, str]], dict[HostId, str]] | None = None + + +@dataclass(frozen=True) +class ModelControl: + """What a host CLI accepts for selecting a model.""" + + host: HostId + binary: str + """Absolute path to the resolved binary.""" + + flag: str | None + """The discovered model-selection flag (e.g. `--model`), or None when + the CLI's help exposes no recognizable model option. None means + "dispatch on this host uses its default model".""" + + short_flag: str | None = None + """Short alias (e.g. `-m`) when the help lists one. Recorded for + diagnostics; `build_argv` always emits the long form.""" + + probed_via: tuple[str, ...] = () + """The argv suffix whose help text produced the match — useful when a + flag turns out to live on a subcommand rather than the root.""" + + @property + def supported(self) -> bool: + return self.flag is not None + + +def _run_help(argv: Sequence[str]) -> str: + """Return combined stdout+stderr of a help invocation, or '' on failure. + + Help output legitimately lands on either stream depending on the CLI, + and a non-zero exit is common for `--help` on some parsers — so the + exit code is ignored and only the text matters. + """ + try: + proc = subprocess.run( + list(argv), + capture_output=True, + text=True, + timeout=_HELP_TIMEOUT_S, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return "" + return f"{proc.stdout}\n{proc.stderr}" + + +def probe_model_control( + host: HostId, + *, + runner: Callable[[Sequence[str]], str] | None = None, + which: Callable[[str], str | None] | None = None, +) -> ModelControl | None: + """Discover `host`'s model-selection flag, or None if it isn't installed. + + Returns None when the binary is absent or the host has no headless + CLI at all — "not installed" and "installed but no model flag" are + different answers, and only the latter is a `ModelControl` with + `flag=None`. + + `runner` and `which` are injectable for tests; production leaves both + unset. + """ + invocations = HELP_INVOCATIONS.get(host) + if not invocations: + return None + + resolve = which or shutil.which + binary = resolve(host) + if binary is None: + return None + + run = runner or _run_help + for suffix in invocations: + match = _MODEL_OPTION.search(run([binary, *suffix])) + if match: + return ModelControl( + host=host, + binary=binary, + flag=match.group(2), + short_flag=match.group(1), + probed_via=suffix, + ) + return ModelControl(host=host, binary=binary, flag=None) + + +def probe_all( + hosts: Sequence[HostId] | None = None, + *, + runner: Callable[[Sequence[str]], str] | None = None, + which: Callable[[str], str | None] | None = None, +) -> dict[HostId, ModelControl]: + """Probe every host in `hosts` (default: all of them). + + Uninstalled hosts are omitted from the result entirely, so callers can + treat membership as "this host is present and was inspected". + """ + targets = hosts if hosts is not None else get_args(HostId) + found: dict[HostId, ModelControl] = {} + for host in targets: + control = probe_model_control(host, runner=runner, which=which) + if control is not None: + found[host] = control + return found + + +# ── model vocabulary + tier assignment ─────────────────────────────────── + +# Model ids and aliases quoted in a CLI's help text, e.g. +# "Provide an alias for the latest model (e.g. 'fable', 'opus', or +# 'sonnet') or a model's full name (e.g. 'claude-fable-5')." +# Both straight and typographic quotes appear in the wild. +# Quote class covers straight and typographic quotes (\u2018\u2019\u201c\u201d), which both +# appear in vendor help text. Written as escapes to keep the source ASCII. +_QUOTES = "'\"\u2018\u2019\u201c\u201d" +_QUOTED_TOKEN = re.compile(rf"[{_QUOTES}]([A-Za-z][A-Za-z0-9._-]{{1,60}})[{_QUOTES}]") + +# Tokens that show up quoted in help text but are never model names. +_NOT_A_MODEL = frozenset( + { + "true", + "false", + "null", + "none", + "auto", + "default", + "json", + "text", + "stream-json", + "yes", + "no", + "on", + "off", + } +) + + +def probe_models( + host: HostId, + *, + runner: Callable[[Sequence[str]], str] | None = None, + which: Callable[[str], str | None] | None = None, +) -> list[str]: + """Model ids and aliases this host's CLI advertises in its own help. + + Reads the vocabulary out of the harness rather than carrying a table + that goes stale the day a vendor ships a new model. Returns [] when + the host is absent or its help enumerates nothing recognizable — + callers fall back to the seeded defaults in that case. + + Order is preserved as the help text lists it, deduped, so a caller + that wants "the first plausible option" gets the CLI's own ordering. + """ + invocations = HELP_INVOCATIONS.get(host) + if not invocations: + return [] + resolve = which or shutil.which + binary = resolve(host) + if binary is None: + return [] + + run = runner or _run_help + seen: dict[str, None] = {} + for suffix in invocations: + text = run([binary, *suffix]) + match = _MODEL_OPTION.search(text) + if not match: + continue + for token in _models_from_help(text, match.start()): + seen.setdefault(token, None) + if seen: + break + return list(seen) + + +def _models_from_help(text: str, option_start: int) -> list[str]: + """Quoted model tokens inside the model option's own help paragraph. + + Scoped to a window after the option so quoted tokens belonging to + unrelated flags elsewhere on a long help page don't leak in. + """ + window = text[option_start : option_start + 600] + return [tok for tok in _QUOTED_TOKEN.findall(window) if tok.lower() not in _NOT_A_MODEL] + + +def _tier_of(model: str) -> ModelTier | None: + """Which tier a model id belongs to, by family substring.""" + lowered = model.lower() + for tier in MODEL_TIERS: + if any(family in lowered for family in TIER_FAMILIES[tier]): + return tier + return None + + +def assign_tiers(models: Sequence[str]) -> dict[ModelTier, str]: + """Sort discovered model ids into the three tiers. + + Two preferences, applied in order: + + 1. **Family preference.** Within a tier, the family listed first in + `TIER_FAMILIES` wins — so `opus` takes the reasoning slot over + `fable` even when the harness offers both. + 2. **Fully-qualified over alias.** Given `opus` and `claude-opus-5`, + the longer, version-pinned id wins. Aliases float to whatever the + vendor most recently shipped, which is convenient interactively and + exactly wrong for an unattended overnight run that should be + reproducible. + + Tiers with no matching model are simply absent from the result; the + caller merges over its own defaults. + """ + best: dict[ModelTier, tuple[int, int, str]] = {} + for model in models: + tier = _tier_of(model) + if tier is None: + continue + lowered = model.lower() + family_rank = next( + (i for i, fam in enumerate(TIER_FAMILIES[tier]) if fam in lowered), + len(TIER_FAMILIES[tier]), + ) + # Longer id == more specific == preferred, so negate for min-ranking. + candidate = (family_rank, -len(model), model) + if tier not in best or candidate < best[tier]: + best[tier] = candidate + return {tier: value[2] for tier, value in best.items()} + + +def detect_harness(env: dict[str, str] | None = None) -> HostId | None: + """The host whose TUI is running this process, from environment markers. + + `nightly init` leads with this so the config it writes reflects the + harness the operator actually works in: init from Claude Code and the + tiers are bound to Claude models; init from Codex and they are bound + to whatever Codex advertises. Returns None when nothing matches, which + is the ordinary case for a plain shell. + """ + import os # noqa: PLC0415 - lazy; keeps module import side-effect free + + environ = os.environ if env is None else env + for host, markers in HARNESS_ENV_MARKERS.items(): + if any(environ.get(marker) for marker in markers): + return host + return None + + +def discover_tier_bindings( + hosts: Sequence[HostId] | None = None, + *, + runner: Callable[[Sequence[str]], str] | None = None, + which: Callable[[str], str | None] | None = None, +) -> tuple[dict[HostId, dict[ModelTier, str]], dict[HostId, str]]: + """Probe installed hosts and return `(tier_models, model_flags)`. + + This is the whole `nightly init` discovery step in one call: for every + host present on PATH, read its model vocabulary and its + model-selection flag out of its own CLI, then rank the vocabulary into + lite / coding / reasoning. + + Hosts that are absent, or whose help yields nothing usable, are simply + missing from the returned mappings — the config writer merges these + over the seeded defaults rather than replacing them, so discovery can + only ever add certainty. + """ + global _DEFAULT_DISCOVERY # noqa: PLW0603 - process-lifetime memo of a machine fact + + uncached = hosts is None and runner is None and which is None + if uncached and _DEFAULT_DISCOVERY is not None: + return _DEFAULT_DISCOVERY + + targets = list(hosts) if hosts is not None else list(get_args(HostId)) + # Lead with the initializing harness so its findings are the ones an + # operator sees first in the init output. + harness = detect_harness() + if harness is not None and harness in targets: + targets.remove(harness) + targets.insert(0, harness) + + resolve = which or shutil.which + run = runner or _run_help + + tier_models: dict[HostId, dict[ModelTier, str]] = {} + model_flags: dict[HostId, str] = {} + for host in targets: + invocations = HELP_INVOCATIONS.get(host) + binary = resolve(host) if invocations else None + if binary is None: + continue + # One help read per invocation yields both facts — the flag and + # the vocabulary come out of the same text, so reading twice would + # double the subprocess cost for nothing. + for suffix in invocations or (): + text = run([binary, *suffix]) + match = _MODEL_OPTION.search(text) + if not match: + continue + model_flags[host] = match.group(2) + tiers = assign_tiers(_models_from_help(text, match.start())) + if tiers: + tier_models[host] = tiers + break + if uncached: + _DEFAULT_DISCOVERY = (tier_models, model_flags) + return tier_models, model_flags + + +# A pinned id carries a version/family suffix (`claude-opus-5`, +# `gpt-5.6`); a bare alias (`opus`, `sonnet`) does not. +_PINNED = re.compile(r"[A-Za-z]+[-.][A-Za-z0-9]") + + +def is_pinned(model: str) -> bool: + """True when `model` names a specific version rather than a floating alias. + + Aliases resolve to whatever the vendor most recently shipped. That is + convenient interactively and wrong for an unattended overnight run, + where the model a task ran on should still be knowable in the morning. + """ + return bool(_PINNED.search(model)) + + +def merge_discovered_tiers( + seeded: dict[HostId, dict[ModelTier, str]], + discovered: dict[HostId, dict[ModelTier, str]], +) -> dict[HostId, dict[ModelTier, str]]: + """Overlay probe results onto seeded defaults, preferring pinned ids. + + Precedence per host+tier: + + 1. A **pinned** discovered id wins — the harness told us exactly what + it offers, versioned. + 2. Otherwise the seeded default stands, if there is one. A seeded + `claude-opus-5` beats a discovered bare `opus` precisely because + the alias floats. + 3. Otherwise the discovered alias is used — for a host Nightly ships + no defaults for, a floating alias still beats no binding at all. + """ + merged = {host: dict(tiers) for host, tiers in seeded.items()} + for host, tiers in discovered.items(): + target = merged.setdefault(host, {}) + for tier, model in tiers.items(): + if is_pinned(model) or tier not in target: + target[tier] = model + return merged diff --git a/packages/nightly-core/src/nightly_core/routing.py b/packages/nightly-core/src/nightly_core/routing.py index 39897cc..41a0f68 100644 --- a/packages/nightly-core/src/nightly_core/routing.py +++ b/packages/nightly-core/src/nightly_core/routing.py @@ -30,11 +30,55 @@ "ContextThresholds", "ResolvedDispatch", "context_window_for", + "effort_directive", "resolve_context_thresholds", "resolve_model_for_task", ] +_EFFORT_DIRECTIVES: dict[ReasoningEffort, str] = { + "low": ( + "Work at LOW deliberation. Act rather than plan: make the edit, run " + "the check, move on. Consolidate tool calls, skip preamble, and do " + "not narrate routine actions or survey options you will not take. " + "`nightly verify` is the correctness gate — reach it quickly rather " + "than reasoning your way to certainty first." + ), + "medium": ( + "Work at MEDIUM deliberation. Think through the approach once, then " + "execute. Keep preamble short and avoid exploratory detours." + ), + "high": ( + "Work at HIGH deliberation. This task is intelligence-sensitive: " + "reason carefully before acting, and state the reasoning that " + "changes the outcome." + ), + "xhigh": ( + "Work at VERY HIGH deliberation. You are the judgment step — " + "orchestration, validation, or merge adjudication. Nothing " + "downstream re-checks your conclusion, so verify claims against " + "evidence rather than plausibility, and say plainly what you could " + "not confirm." + ), + "max": ( + "Work at MAXIMUM deliberation. Correctness dominates cost here: " + "exhaust the alternatives, verify every claim against evidence, and " + "flag anything you could not confirm." + ), +} + + +def effort_directive(effort: ReasoningEffort) -> str: + """Prompt text telling a dispatched agent how much to deliberate. + + Injected into the dispatch prompt rather than passed as a CLI flag. + The flag surface differs per host and several hosts expose none, while + prompt text works everywhere and degrades to a harmless no-op on a + model that ignores it — see RFC 007 Resolved #11. + """ + return _EFFORT_DIRECTIVES[effort] + + @dataclass(frozen=True) class ResolvedDispatch: """Everything a dispatch needs to know about its own model budget.""" diff --git a/packages/nightly-core/tests/test_contract.py b/packages/nightly-core/tests/test_contract.py index f7d9079..ba24544 100644 --- a/packages/nightly-core/tests/test_contract.py +++ b/packages/nightly-core/tests/test_contract.py @@ -25,6 +25,11 @@ def test_host_id_literal_covers_supported_hosts() -> None: "opencode", "antigravity", "gemini", + # Recognized at the routing layer (model-tier config + `nightly + # init`'s model probe) but shipping no integration package yet — + # see the `HostId` docstring. + "pi", + "hermes", } diff --git a/packages/nightly-core/tests/test_model_probe.py b/packages/nightly-core/tests/test_model_probe.py new file mode 100644 index 0000000..fabda66 --- /dev/null +++ b/packages/nightly-core/tests/test_model_probe.py @@ -0,0 +1,291 @@ +"""Tests for `nightly init`'s model-control discovery. + +The probe's whole justification is that Nightly should not carry a table +of vendor flags and model ids that goes stale. So these tests exercise it +against captured help text rather than pinning a hardcoded expectation: +given what a CLI says about itself, does the right flag, vocabulary, and +tier hierarchy come out? +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import pytest + +from nightly_core.contract import MODEL_TIERS, HostId, ModelTier +from nightly_core.model_probe import ( + HARNESS_ENV_MARKERS, + HELP_INVOCATIONS, + assign_tiers, + detect_harness, + discover_tier_bindings, + is_pinned, + merge_discovered_tiers, + probe_model_control, + probe_models, +) + +# Verbatim from `claude --help` (2026-07-28). +CLAUDE_HELP = """ + -c, --continue Continue the most recent conversation + --model Model for the current session. Provide + an alias for the latest model (e.g. + 'fable', 'opus', or 'sonnet') or a + model's full name (e.g. + 'claude-fable-5'). + --session-id Use a specific session ID + --output-format Output format (e.g. 'json', 'text') +""" + +NO_MODEL_HELP = """ + -h, --help Show help + -v, --version Show version +""" + +SHORT_FLAG_HELP = """ + -m, --model MODEL Model to use (e.g. 'gpt-5.6', 'gpt-5-mini') +""" + + +def _runner(mapping: dict[tuple[str, ...], str]): + """Fake help-runner keyed by the argv suffix after the binary.""" + + def run(argv: Sequence[str]) -> str: + return mapping.get(tuple(argv[1:]), "") + + return run + + +def _which(present: set[str]): + def which(name: str) -> str | None: + return f"/usr/local/bin/{name}" if name in present else None + + return which + + +# ── flag discovery ──────────────────────────────────────────────────────── + + +def test_discovers_long_flag_from_real_claude_help() -> None: + control = probe_model_control( + "claude", + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert control is not None + assert control.flag == "--model" + assert control.probed_via == ("--help",) + + +def test_records_short_alias_when_help_lists_one() -> None: + control = probe_model_control( + "codex", + runner=_runner({("exec", "--help"): SHORT_FLAG_HELP}), + which=_which({"codex"}), + ) + assert control is not None + assert control.flag == "--model" + assert control.short_flag == "-m" + + +def test_falls_back_to_root_help_when_subcommand_has_no_flag() -> None: + control = probe_model_control( + "codex", + runner=_runner({("exec", "--help"): NO_MODEL_HELP, ("--help",): CLAUDE_HELP}), + which=_which({"codex"}), + ) + assert control is not None + assert control.probed_via == ("--help",) + + +def test_installed_but_no_model_flag_is_not_the_same_as_absent() -> None: + """`flag=None` means "runs on its default model", not "not installed".""" + control = probe_model_control( + "gemini", + runner=_runner({("--help",): NO_MODEL_HELP}), + which=_which({"gemini"}), + ) + assert control is not None + assert control.flag is None + assert control.supported is False + + +def test_absent_binary_yields_none() -> None: + assert probe_model_control("claude", runner=_runner({}), which=_which(set())) is None + + +def test_unreadable_help_degrades_to_no_flag() -> None: + """A CLI that errors, hangs, or prints nothing must not break init. + + `_run_help` returns '' for every failure mode (non-zero exit, timeout, + OSError), so empty output is the single shape the parser has to + survive — and it resolves to "no known flag", not an exception. + """ + control = probe_model_control("claude", runner=lambda _: "", which=_which({"claude"})) + assert control is not None + assert control.flag is None + assert control.supported is False + + +# ── model vocabulary ────────────────────────────────────────────────────── + + +def test_reads_model_vocabulary_out_of_help_text() -> None: + models = probe_models( + "claude", + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert models == ["fable", "opus", "sonnet", "claude-fable-5"] + + +def test_ignores_quoted_tokens_that_are_not_models() -> None: + """'json' and 'text' are quoted in the same help page, under another flag.""" + models = probe_models( + "claude", + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert "json" not in models + assert "text" not in models + + +def test_no_vocabulary_when_host_is_absent() -> None: + assert probe_models("claude", runner=_runner({}), which=_which(set())) == [] + + +# ── tier assignment ─────────────────────────────────────────────────────── + + +def test_assigns_families_to_the_right_tiers() -> None: + tiers = assign_tiers(["claude-haiku-4-5", "claude-sonnet-5", "claude-opus-5"]) + assert tiers == { + "lite": "claude-haiku-4-5", + "coding": "claude-sonnet-5", + "reasoning": "claude-opus-5", + } + + +def test_opus_beats_fable_for_the_reasoning_slot() -> None: + """Reasoning is the judgment tier, not the most-expensive-available tier.""" + tiers = assign_tiers(["claude-fable-5", "claude-opus-5"]) + assert tiers["reasoning"] == "claude-opus-5" + + +def test_pinned_id_beats_bare_alias_within_a_family() -> None: + tiers = assign_tiers(["opus", "claude-opus-5"]) + assert tiers["reasoning"] == "claude-opus-5" + + +def test_unrecognized_families_are_skipped() -> None: + assert assign_tiers(["some-unknown-model"]) == {} + + +def test_cross_vendor_families_land_correctly() -> None: + tiers = assign_tiers(["gpt-5-mini", "qwen-coder-3", "gpt-5-reasoning"]) + assert tiers["lite"] == "gpt-5-mini" + assert tiers["coding"] == "qwen-coder-3" + assert tiers["reasoning"] == "gpt-5-reasoning" + + +# ── pinned-vs-alias precedence ──────────────────────────────────────────── + + +@pytest.mark.parametrize("model", ["claude-opus-5", "gpt-5.6", "qwen-coder-3"]) +def test_pinned_ids_are_recognized(model: str) -> None: + assert is_pinned(model) + + +@pytest.mark.parametrize("model", ["opus", "sonnet", "haiku"]) +def test_bare_aliases_are_not_pinned(model: str) -> None: + assert not is_pinned(model) + + +def test_seeded_pinned_default_survives_a_discovered_alias() -> None: + """An alias floats to the latest release — wrong for a reproducible run.""" + merged = merge_discovered_tiers( + {"claude": {"reasoning": "claude-opus-5"}}, + {"claude": {"reasoning": "opus"}}, + ) + assert merged["claude"]["reasoning"] == "claude-opus-5" + + +def test_discovered_pinned_id_overrides_the_seeded_default() -> None: + merged = merge_discovered_tiers( + {"claude": {"reasoning": "claude-opus-5"}}, + {"claude": {"reasoning": "claude-opus-6"}}, + ) + assert merged["claude"]["reasoning"] == "claude-opus-6" + + +def test_alias_is_used_when_there_is_no_seeded_default() -> None: + """A floating alias still beats no binding at all.""" + merged = merge_discovered_tiers({}, {"codex": {"coding": "gpt"}}) + assert merged["codex"]["coding"] == "gpt" + + +def test_merge_does_not_mutate_the_seeded_table() -> None: + seeded: dict[HostId, dict[ModelTier, str]] = {"claude": {"reasoning": "claude-opus-5"}} + merge_discovered_tiers(seeded, {"claude": {"lite": "claude-haiku-9"}}) + assert seeded == {"claude": {"reasoning": "claude-opus-5"}} + + +# ── harness detection ───────────────────────────────────────────────────── + + +def test_detects_claude_code_from_its_env_marker() -> None: + assert detect_harness({"CLAUDECODE": "1"}) == "claude" + + +def test_detects_via_secondary_marker() -> None: + assert detect_harness({"CLAUDE_CODE_ENTRYPOINT": "cli"}) == "claude" + + +def test_plain_shell_detects_no_harness() -> None: + assert detect_harness({"PATH": "/usr/bin"}) is None + + +def test_empty_marker_value_does_not_count_as_present() -> None: + assert detect_harness({"CLAUDECODE": ""}) is None + + +@pytest.mark.parametrize( + "host", ["claude", "codex", "cursor", "gemini", "opencode", "pi", "hermes"] +) +def test_every_major_harness_is_probeable(host: HostId) -> None: + """Gemini, OpenCode, Codex, Claude, Cursor, Pi, Hermes all covered.""" + assert host in HARNESS_ENV_MARKERS + assert host in HELP_INVOCATIONS + + +# ── end-to-end discovery ────────────────────────────────────────────────── + + +def test_discovery_returns_tiers_and_flags_together() -> None: + tiers, flags = discover_tier_bindings( + ["claude"], + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert flags == {"claude": "--model"} + assert tiers["claude"]["reasoning"] == "opus" + + +def test_discovery_omits_hosts_that_are_not_installed() -> None: + tiers, flags = discover_tier_bindings( + ["claude", "codex"], + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert "codex" not in tiers + assert "codex" not in flags + + +def test_discovered_tiers_are_a_subset_of_the_known_tiers() -> None: + tiers, _ = discover_tier_bindings( + ["claude"], + runner=_runner({("--help",): CLAUDE_HELP}), + which=_which({"claude"}), + ) + assert set(tiers["claude"]) <= set(MODEL_TIERS) From df185df8a91df50b18e4147a2238fad5752bbdff Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:17:12 -0400 Subject: [PATCH 02/27] docs(rfc-007): reconcile Phase B checklist; record discovery as Resolved #12 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B1/B4/B5 landed and B6 (model-control discovery) was unplanned but supersedes part of B1 — the flag is now probed from each host CLI rather than carried in a table Nightly would have to keep current. B2 (six host skills) and B3 (briefing tier breakdown) remain open. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 43 +++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index c1e1bc5..5be17b5 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -341,6 +341,35 @@ everywhere and degrades to a no-op on a model that ignores it. A future phase can upgrade specific hosts to a native flag without changing the config schema. +**12. Model controls are discovered, not declared (ADDED 2026-07-28).** +`nightly init` probes each installed host CLI's `--help` for its +model-selection flag and its advertised model vocabulary, then ranks that +vocabulary into the three tiers and writes the result to +`model_tiers..flag` and `model_tiers..`. + +The original plan had Nightly carry a per-host flag table. That table is +wrong the day a vendor renames a flag, and a wrong flag is a hard spawn +failure in the middle of an unattended run — whereas the right one is +readable from the CLI in milliseconds. Discovery immediately found +`--model` on opencode and gemini, neither of which had been verified by +hand when the defaults were written. + +Two rules keep discovery safe: + +- **It can only add certainty.** Probe results merge over the seeded + defaults; any probe failure degrades to the seeded template, so `init` + can never fail because a host CLI misbehaved. +- **Pinned beats floating.** A discovered *pinned* id (`claude-opus-5`) + overrides a seeded default, but a bare alias (`opus`) does not. Aliases + resolve to whatever shipped most recently — convenient interactively, + wrong for an overnight run whose model should still be identifiable in + the morning. + +Host coverage spans all seven major harnesses (Claude, Codex, Cursor, +Gemini, OpenCode, Pi, Hermes) plus Antigravity. `pi` and `hermes` are +recognized at the routing layer only — they ship no integration package, +so skill install and keep-alive hooks are unavailable for them. + ## Risks - **Tier mis-pick at the borderline.** Agent judgment will @@ -481,12 +510,18 @@ missing config; README updated. reviewer-tier change makes a silently-inert routing config more consequential than it was when C2 was scheduled. -**Phase B — Dispatch integration** -- [ ] B1. `nightly dispatch start` reads resolved model id +**Phase B — Dispatch integration** — *core landed 2026-07-28; B2/B3 open* +- [x] B1. `nightly dispatch start` reads resolved model id and passes it + with the **discovered** model flag (see B6) - [ ] B2. Task-tool fallback documented across six host skill.md - [ ] B3. Briefing tier-breakdown line -- [ ] B4. `nightly specialist --tier ` flag -- [ ] B5. End-to-end dispatch tests across tiers + fallback +- [x] B4. `nightly specialist --tier ` flag +- [x] B5. Dispatch resolution tests across tiers + host-miss fallback + (`tests/test_routing.py`); end-to-end argv assertions still open +- [x] B6. *(unplanned, supersedes part of B1)* `nightly init` **discovers** + each host's model-selection flag and model vocabulary from the host + CLI's own `--help`, rather than Nightly carrying a vendor table. + See `nightly_core.model_probe` and RFC 007 Resolved #12. **Phase C — Auto-tag heuristic + doctor + docs** - [ ] C1. Auto-tag scoping paragraph on six host skills From 2aec15f1044b96f6a8bbc608b0a15759371e783d Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:24:03 -0400 Subject: [PATCH 03/27] feat(dispatch): enforce parallelism caps at admission (RFC 012 B1/B2/B5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A declared `parallelism:` caps that nothing read. A cap nothing enforces is a comment, and it's the same silently-inert failure flagged for `model_tiers` — so this makes it real. `nightly dispatch start` now refuses when the tier is at capacity, exits 3, and names the ceiling that blocked it (`--force` overrides). Two ceilings are checked: the tier's own — via `limit_for`, which already folds in the global cap — and the global count across every live dispatch. `nightly dispatch status` gained a `capacity:` line showing live/cap per tier plus the total, because that's the question the dispatch table provokes. Three decisions worth noting: - **Blocking says "wait", never "run it on a cheaper tier."** A silent downgrade would hand back a lite-tier review nobody asked for, with no signal it happened — defeating RFC 007's whole argument for putting the reviewer on the reasoning tier. There's a test asserting the refusal message never names another tier. - **Liveness is checked against the PID, not the recorded status.** A spawn that died without being polled still reads `running` on disk; counting those would wedge the fleet until someone happened to run `dispatch status`. - **Dispatch state now persists its tier.** Counting per tier by re-resolving each plan would read a plan that may have changed, or been marked done, since the spawn. Records written before this change have no tier and count toward the global cap only. 1214 tests pass (14 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...2-fleet-parallelism-and-context-handoff.md | 13 +- packages/nightly-core/src/nightly_core/cli.py | 45 ++++- .../nightly-core/src/nightly_core/dispatch.py | 81 +++++++- packages/nightly-core/tests/test_admission.py | 174 ++++++++++++++++++ 4 files changed, 307 insertions(+), 6 deletions(-) create mode 100644 packages/nightly-core/tests/test_admission.py diff --git a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index 15339c1..66453cc 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -256,12 +256,17 @@ produces a handoff summary naming its unfinished goals. - [x] A6. Config template blocks - [x] A7. Unit tests (in `tests/test_routing.py`) -**Phase B — Admission control** -- [ ] B1. `dispatch start` admission check -- [ ] B2. `dispatch status` shows cap utilization +**Phase B — Admission control** — *B1/B2/B5 landed 2026-07-28* +- [x] B1. `dispatch start` admission check — refuses with exit code 3 and + names the cap that blocked it; `--force` overrides. Dispatch state + now persists its tier so counting doesn't re-resolve the plan. +- [x] B2. `dispatch status` shows a `capacity:` line (live/cap per tier + plus the global total) - [ ] B3. Worktree creation honors `max_worktrees` - [ ] B4. Six host skills: fan-out-to-cap guidance -- [ ] B5. Admission tests +- [x] B5. Admission tests (`tests/test_admission.py`, 14 cases) — + including the liveness rule: a dispatch whose PID is gone must not + occupy a slot, or an unpolled crash wedges the fleet **Phase C — Handoff protocol** - [ ] C1. Keepalive threshold comparison + prompt injection diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index bfe03a4..bd0e363 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -1599,7 +1599,7 @@ def _plan_model_tier(slug: str, root: Path) -> ModelTier | None: @dispatch_app.command(name="start") -def dispatch_start_cmd( +def dispatch_start_cmd( # noqa: PLR0913 - one option per dispatch dimension slug: Annotated[ str, typer.Argument(help="Task slug. Must exist under the current run."), @@ -1638,6 +1638,13 @@ def dispatch_start_cmd( help="Working directory for the spawned process. Defaults to the repo root.", ), ] = None, + force: Annotated[ + bool, + typer.Option( + "--force", + help="Spawn even when the parallelism cap for this tier is reached.", + ), + ] = False, ) -> None: """Spawn the host's headless CLI as a detached background process. @@ -1672,6 +1679,19 @@ def dispatch_start_cmd( if tier_cfg.enabled: body = f"{body}\n{effort_directive(resolved.effort)}\n" + from nightly_core.config import load_parallelism_config # noqa: PLC0415 - lazy + from nightly_core.dispatch import admission_blocked # noqa: PLC0415 - lazy + + blocked = admission_blocked(resolved.tier, load_parallelism_config(root), root) + if blocked and not force: + typer.echo(f"✗ dispatch not admitted: {blocked}", err=True) + typer.echo( + " wait for a slot (`nightly dispatch status`), raise the cap in " + "`.nightly/config.yml` under `parallelism:`, or pass --force.", + err=True, + ) + raise typer.Exit(code=3) + try: result = start_background( slug, @@ -1682,6 +1702,7 @@ def dispatch_start_cmd( cwd=cwd, model=resolved.model, model_flag=tier_cfg.flag_for(host), + tier=resolved.tier, ) except RuntimeError as exc: typer.echo(f"✗ {exc}", err=True) @@ -1763,6 +1784,28 @@ def dispatch_status_cmd( live = refresh(state, root=root) _print_dispatch_row(live, root=root, verbose=False) + _print_tier_utilization(root) + + +def _print_tier_utilization(root: Path) -> None: + """Show live-vs-cap per tier so the operator can see the fleet's headroom. + + Printed after the dispatch table because it answers the question the + table provokes: "can I start another one?" + """ + from nightly_core.config import load_parallelism_config # noqa: PLC0415 - lazy + from nightly_core.dispatch import tier_utilization # noqa: PLC0415 - lazy + + config = load_parallelism_config(root) + util = tier_utilization(config, root) + cells = [ + f"{tier} {used}/{cap or '∞'}" + (" FULL" if cap and used >= cap else "") + for tier, (used, cap) in util.items() + ] + total = sum(used for used, _ in util.values()) + overall = config.max_concurrent_specialists or "∞" + typer.echo(f"\ncapacity: {' '.join(cells)} (total {total}/{overall})") + def _print_dispatch_row( state: object, # BackgroundDispatchResult — typed via duck on the read side diff --git a/packages/nightly-core/src/nightly_core/dispatch.py b/packages/nightly-core/src/nightly_core/dispatch.py index 1c35cdb..982e80f 100644 --- a/packages/nightly-core/src/nightly_core/dispatch.py +++ b/packages/nightly-core/src/nightly_core/dispatch.py @@ -49,7 +49,8 @@ from pathlib import Path from typing import Literal -from nightly_core.contract import HostId, SpecialistRole +from nightly_core.config import ParallelismConfig +from nightly_core.contract import MODEL_TIERS, HostId, ModelTier, SpecialistRole from nightly_core.paths import repo_root __all__ = [ @@ -57,11 +58,14 @@ "DEFAULT_STATE_FILENAME", "BackgroundDispatchResult", "DispatchStatus", + "active_dispatches", + "admission_blocked", "build_argv", "is_alive", "list_dispatches", "read_dispatch_state", "start_background", + "tier_utilization", "wait_for", "write_dispatch_state", ] @@ -102,6 +106,12 @@ class BackgroundDispatchResult: status: DispatchStatus = "running" exit_code: int | None = None finished_at: datetime | None = None + tier: ModelTier | None = None + """RFC 007 tier this dispatch was routed to. Persisted so admission + control can count live dispatches per tier without re-resolving the + plan (which may have changed, or been marked done, since the spawn). + None on dispatches written before RFC 012 — those count toward the + global cap only.""" # ── per-host argv ──────────────────────────────────────────────────────── @@ -209,6 +219,7 @@ def start_background( # noqa: PLR0913 - dispatch primitive needs every dimensio now: datetime | None = None, model: str | None = None, model_flag: str | None = None, + tier: ModelTier | None = None, popen_factory: object | None = None, ) -> BackgroundDispatchResult: """Spawn the host's headless CLI as a detached background process. @@ -268,6 +279,7 @@ def start_background( # noqa: PLR0913 - dispatch primitive needs every dimensio status="running", exit_code=None, finished_at=None, + tier=tier, ) write_dispatch_state(result, root=repo) return result @@ -346,6 +358,7 @@ def read_dispatch_state(slug: str, *, root: Path | None = None) -> BackgroundDis log_path=Path(data["log_path"]), started_at=datetime.fromisoformat(data["started_at"]), argv=tuple(data.get("argv", [])), + tier=data.get("tier"), cwd=Path(data["cwd"]) if data.get("cwd") else None, status=data.get("status", "unknown"), exit_code=data.get("exit_code"), @@ -481,3 +494,69 @@ def supported_hosts() -> Sequence[HostId]: populate help text and the SKILL.md "supported hosts" list. """ return ("claude", "codex", "opencode", "gemini") + + +# ── admission control (RFC 012 Phase B) ────────────────────────────────── + + +def active_dispatches(root: Path | None = None) -> list[BackgroundDispatchResult]: + """Dispatches whose process is still alive. + + Liveness is checked against the PID rather than trusting the recorded + status: a spawn that died without anyone polling it still has + `status: running` on disk, and counting those toward the cap would + wedge the fleet until someone ran `nightly dispatch status`. + """ + return [d for d in list_dispatches(root) if d.status == "running" and is_alive(d.pid)] + + +def tier_utilization( + config: ParallelismConfig, + root: Path | None = None, +) -> dict[ModelTier, tuple[int, int]]: + """`{tier: (live_count, effective_cap)}` for every tier. + + The cap is `ParallelismConfig.limit_for`, i.e. the tighter of the + per-tier and global ceilings; `0` means unlimited. Dispatches with no + recorded tier (pre-RFC-012 state files) are counted only against the + global total, which `admission_blocked` checks separately. + """ + live = active_dispatches(root) + counts: dict[ModelTier, int] = dict.fromkeys(MODEL_TIERS, 0) + for dispatch in live: + if dispatch.tier in counts: + counts[dispatch.tier] += 1 # type: ignore[index] + return {tier: (counts[tier], config.limit_for(tier)) for tier in MODEL_TIERS} + + +def admission_blocked( + tier: ModelTier, + config: ParallelismConfig, + root: Path | None = None, +) -> str | None: + """Reason this dispatch must wait, or None when it may start. + + Two ceilings are checked: the tier's own (via `limit_for`, which + already folds in the global cap) and the global count across every + live dispatch including untiered ones. + + Deliberately returns "wait", never "run it on a cheaper tier". A + silent downgrade would defeat RFC 007's whole argument about the + reviewer sitting on the reasoning tier — the caller would get a + lite-tier review it never asked for and no signal that it happened. + """ + live = active_dispatches(root) + total = len(live) + if config.max_concurrent_specialists and total >= config.max_concurrent_specialists: + return ( + f"{total} specialist(s) already running; " + f"global cap is {config.max_concurrent_specialists}" + ) + + cap = config.limit_for(tier) + if not cap: + return None + used = sum(1 for d in live if d.tier == tier) + if used >= cap: + return f"{used} {tier}-tier dispatch(es) already running; cap is {cap}" + return None diff --git a/packages/nightly-core/tests/test_admission.py b/packages/nightly-core/tests/test_admission.py new file mode 100644 index 0000000..2c4ae84 --- /dev/null +++ b/packages/nightly-core/tests/test_admission.py @@ -0,0 +1,174 @@ +"""Tests for RFC 012 Phase B — parallelism caps enforced at admission. + +The config landed in Phase A declared caps that nothing read. These tests +pin the enforcement: a cap that doesn't refuse anything is a comment. +""" + +from __future__ import annotations + +import os +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from nightly_core.config import ParallelismConfig +from nightly_core.contract import MODEL_TIERS, ModelTier +from nightly_core.dispatch import ( + BackgroundDispatchResult, + admission_blocked, + tier_utilization, +) + + +def _dispatch(tier: ModelTier | None, *, pid: int | None = None) -> BackgroundDispatchResult: + """A live dispatch record. Defaults to this process's own PID so the + `is_alive` check in `active_dispatches` sees it as running.""" + return BackgroundDispatchResult( + slug=f"task-{tier}-{pid or os.getpid()}", + role="implementer", + host="claude", + pid=pid or os.getpid(), + log_path=Path("/tmp/dispatch.log"), + started_at=datetime.now(UTC), + status="running", + tier=tier, + ) + + +@pytest.fixture +def live(monkeypatch: pytest.MonkeyPatch): + """Patch `active_dispatches` so tests declare the fleet directly. + + Spawning real processes to test a counting rule would be slow and + flaky; the liveness filter itself is covered separately below. + """ + + def _set(dispatches: list[BackgroundDispatchResult]) -> None: + monkeypatch.setattr( + "nightly_core.dispatch.active_dispatches", + lambda _root=None: dispatches, + ) + + return _set + + +# ── admission ───────────────────────────────────────────────────────────── + + +def test_admits_when_the_fleet_is_empty(live) -> None: + live([]) + assert admission_blocked("coding", ParallelismConfig()) is None + + +def test_admits_below_the_tier_cap(live) -> None: + live([_dispatch("reasoning")]) + # reasoning cap is 2 + assert admission_blocked("reasoning", ParallelismConfig()) is None + + +def test_blocks_at_the_tier_cap(live) -> None: + live([_dispatch("reasoning"), _dispatch("reasoning")]) + reason = admission_blocked("reasoning", ParallelismConfig()) + assert reason is not None + assert "reasoning" in reason + assert "cap is 2" in reason + + +def test_a_full_tier_does_not_block_a_different_tier(live) -> None: + """The per-tier gradient is the point — a saturated reasoning tier must + not stall the wide lite fleet.""" + live([_dispatch("reasoning"), _dispatch("reasoning")]) + assert admission_blocked("lite", ParallelismConfig()) is None + + +def test_blocks_at_the_global_cap_even_with_tier_headroom(live) -> None: + config = ParallelismConfig(max_concurrent_specialists=3) + live([_dispatch("lite"), _dispatch("lite"), _dispatch("coding")]) + reason = admission_blocked("lite", config) + assert reason is not None + assert "global cap is 3" in reason + + +def test_untiered_dispatches_count_toward_the_global_cap(live) -> None: + """Pre-RFC-012 state files have no tier; they still occupy a slot.""" + config = ParallelismConfig(max_concurrent_specialists=2) + live([_dispatch(None), _dispatch(None)]) + assert admission_blocked("lite", config) is not None + + +def test_untiered_dispatches_do_not_count_against_a_tier(live) -> None: + config = ParallelismConfig(max_concurrent_specialists=0) + live([_dispatch(None), _dispatch(None), _dispatch(None)]) + assert admission_blocked("reasoning", config) is None + + +def test_zero_cap_means_unlimited(live) -> None: + config = ParallelismConfig( + max_concurrent_specialists=0, + per_tier=dict.fromkeys(MODEL_TIERS, 0), + ) + live([_dispatch("reasoning") for _ in range(50)]) + assert admission_blocked("reasoning", config) is None + + +def test_global_cap_clamps_a_wider_tier_cap(live) -> None: + """`limit_for` takes the tighter of the two, so a lite cap of 12 under a + global cap of 2 admits only 2.""" + config = ParallelismConfig( + max_concurrent_specialists=2, + per_tier={"lite": 12, "coding": 6, "reasoning": 2}, + ) + live([_dispatch("lite")]) + assert admission_blocked("lite", config) is None + live([_dispatch("lite"), _dispatch("lite")]) + assert admission_blocked("lite", config) is not None + + +def test_blocking_never_suggests_a_cheaper_tier(live) -> None: + """A silent downgrade would hand back a lite review nobody asked for.""" + live([_dispatch("reasoning"), _dispatch("reasoning")]) + reason = admission_blocked("reasoning", ParallelismConfig()) + assert reason is not None + assert "lite" not in reason + assert "coding" not in reason + + +# ── utilization reporting ───────────────────────────────────────────────── + + +def test_utilization_reports_every_tier(live) -> None: + live([_dispatch("coding")]) + util = tier_utilization(ParallelismConfig()) + assert set(util) == set(MODEL_TIERS) + assert util["coding"] == (1, 6) + assert util["lite"] == (0, 8) + + +def test_utilization_reflects_the_effective_cap(live) -> None: + live([]) + util = tier_utilization(ParallelismConfig(max_concurrent_specialists=3)) + # Global cap of 3 clamps lite's per-tier 8. + assert util["lite"] == (0, 3) + # Reasoning's own cap of 2 is already tighter. + assert util["reasoning"] == (0, 2) + + +def test_utilization_ignores_untiered_dispatches_per_tier(live) -> None: + live([_dispatch(None)]) + util = tier_utilization(ParallelismConfig()) + assert all(used == 0 for used, _ in util.values()) + + +# ── liveness ────────────────────────────────────────────────────────────── + + +def test_dead_dispatches_do_not_occupy_a_slot(monkeypatch: pytest.MonkeyPatch) -> None: + """A spawn that died without being polled still reads `running` on disk. + Counting it would wedge the fleet until someone ran `dispatch status`.""" + from nightly_core import dispatch as dispatch_mod + + stale = _dispatch("reasoning", pid=999_999) + monkeypatch.setattr(dispatch_mod, "list_dispatches", lambda _root=None: [stale, stale]) + monkeypatch.setattr(dispatch_mod, "is_alive", lambda _pid: False) + assert dispatch_mod.active_dispatches() == [] From 1490b04fc054802511f07a9c7b0f07e22dba860d Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:29:10 -0400 Subject: [PATCH 04/27] feat(worktree): honor parallelism.max_worktrees (RFC 012 B3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Last inert knob from Phase A. `max_worktrees` was declared in the config and read by nothing, so the task-level fan-out cap did not exist. `create_worktree` now takes the cap and raises `WorktreeCapReached` when the fleet is full. Three choices worth noting: - **A typed exception, not a bare RuntimeError.** Callers need to tell "at capacity, try again when a task finishes" apart from "git itself failed" — the first is a scheduling condition with an obvious remedy. The CLI exits 3 for it, matching `dispatch start`'s at-capacity code. - **Checked before the branch is cut**, so a refused request leaves nothing behind. There's a test asserting `worktree add` is never reached when the cap blocks. - **Counted from live `git worktree list`, not run state.** A worktree left behind by an earlier crashed run still occupies a slot, because it is still consuming disk and still holds a branch checked out. Counting only what the current run knows about would under-report exactly when the repo is most cluttered. Both call sites (the `nightly worktree` command and the headless driver) pass the configured cap; `0` remains unlimited, as everywhere else. 1218 tests pass (4 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...2-fleet-parallelism-and-context-handoff.md | 7 +- packages/nightly-core/src/nightly_core/cli.py | 9 +++ .../nightly-core/src/nightly_core/driver.py | 2 + .../nightly-core/src/nightly_core/worktree.py | 36 ++++++++++ packages/nightly-core/tests/test_admission.py | 66 +++++++++++++++++++ 5 files changed, 119 insertions(+), 1 deletion(-) diff --git a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index 66453cc..39020e1 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -262,7 +262,12 @@ produces a handoff summary naming its unfinished goals. now persists its tier so counting doesn't re-resolve the plan. - [x] B2. `dispatch status` shows a `capacity:` line (live/cap per tier plus the global total) -- [ ] B3. Worktree creation honors `max_worktrees` +- [x] B3. Worktree creation honors `max_worktrees` — raises the typed + `WorktreeCapReached` (not a bare RuntimeError, so "at capacity" is + distinguishable from "git failed"), checked before the branch is + cut so a refused request leaves nothing behind. Both callers + (`nightly worktree`, the headless driver) pass the configured cap; + CLI exits 3, matching `dispatch start`. - [ ] B4. Six host skills: fan-out-to-cap guidance - [x] B5. Admission tests (`tests/test_admission.py`, 14 cases) — including the liveness rule: a dispatch whose PID is gone must not diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index bd0e363..1df3db3 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -1480,6 +1480,9 @@ def worktree_create( typer.echo(f"worktree_root={worktree_root_cfg or '(auto)'}") return + from nightly_core.config import load_parallelism_config # noqa: PLC0415 - lazy + from nightly_core.worktree import WorktreeCapReached # noqa: PLC0415 - lazy + try: handle = asyncio.run( create_worktree( @@ -1488,8 +1491,14 @@ def worktree_create( base_branch=base_branch, branch_prefix=branch_prefix, worktree_root=worktree_root_cfg, + max_worktrees=load_parallelism_config(root).max_worktrees, ) ) + except WorktreeCapReached as exc: + # Exit 3 matches `dispatch start`'s at-capacity code: both mean + # "wait for a slot", not "something broke". + typer.echo(f"✗ {exc}", err=True) + raise typer.Exit(code=3) from None except RuntimeError as exc: typer.echo(f"✗ {exc}", err=True) raise typer.Exit(code=1) from None diff --git a/packages/nightly-core/src/nightly_core/driver.py b/packages/nightly-core/src/nightly_core/driver.py index 9088e65..f16ab24 100644 --- a/packages/nightly-core/src/nightly_core/driver.py +++ b/packages/nightly-core/src/nightly_core/driver.py @@ -30,6 +30,7 @@ from typing import Literal from nightly_core.cascade import CascadeChoice, next_task +from nightly_core.config import load_parallelism_config from nightly_core.contract import HostId, NightlyHostIntegration from nightly_core.headless import HeadlessResult from nightly_core.ideation import run_proposers @@ -283,6 +284,7 @@ async def run_one_task( # noqa: PLR0913 - per-task dispatch needs every dimensi branch_prefix=branch_prefix, worktree_root=worktree_root, runner=git_runner, + max_worktrees=load_parallelism_config(root).max_worktrees, ) prompt = build_task_prompt(plan, plan.path.parent, cascade_choice=cascade_choice) headless = await host.run_headless( diff --git a/packages/nightly-core/src/nightly_core/worktree.py b/packages/nightly-core/src/nightly_core/worktree.py index b7130b4..131ebc7 100644 --- a/packages/nightly-core/src/nightly_core/worktree.py +++ b/packages/nightly-core/src/nightly_core/worktree.py @@ -24,6 +24,7 @@ __all__ = [ "GitRunner", + "WorktreeCapReached", "WorktreeHandle", "create_worktree", "default_git_runner", @@ -34,6 +35,26 @@ _log = logging.getLogger(__name__) + +class WorktreeCapReached(RuntimeError): + """Raised when `parallelism.max_worktrees` would be exceeded — RFC 012 B3. + + A distinct type rather than a bare `RuntimeError` so callers can tell + "the fleet is at capacity, try again when a task finishes" apart from + "git itself failed". The first is a scheduling condition with an + obvious remedy; the second is a real error. + """ + + def __init__(self, live: int, cap: int) -> None: + self.live = live + self.cap = cap + super().__init__( + f"{live} Nightly worktree(s) already exist; `parallelism.max_worktrees` " + f"is {cap}. Finish or remove a task's worktree, or raise the cap in " + "`.nightly/config.yml`." + ) + + DEFAULT_BRANCH_PREFIX = "nightly/" @@ -322,14 +343,29 @@ async def create_worktree( # noqa: PLR0913 - all params are real config dimensi worktree_root: str | None = None, runner: GitRunner | None = None, now: datetime | None = None, + max_worktrees: int = 0, ) -> WorktreeHandle: """Create a new isolated worktree for `slug`. Spawns `git worktree add -b `. The base branch must already exist on the repo at `root`. Placement is decided by `_resolve_worktree_base` (config-overridable, iCloud-aware). + + `max_worktrees` is the RFC 012 task-level fan-out cap; `0` (the + default) means unlimited, matching every other limiter in the config. + The count is taken from the live `git worktree list`, not from run + state, so a worktree left behind by an earlier crashed run still + occupies a slot — which is the honest reading, since it is still + consuming disk and still holds a branch checked out. + + Raises `WorktreeCapReached` at capacity. Checked *before* the branch + is cut so a refused request leaves nothing behind. """ run = runner or default_git_runner + if max_worktrees > 0: + live = await list_worktrees(root, branch_prefix=branch_prefix, runner=run) + if len(live) >= max_worktrees: + raise WorktreeCapReached(len(live), max_worktrees) branch = _branch_name(slug, prefix=branch_prefix, now=now) base = await _resolve_worktree_base(root, worktree_root=worktree_root, run=run) path = _worktree_path(base, branch) diff --git a/packages/nightly-core/tests/test_admission.py b/packages/nightly-core/tests/test_admission.py index 2c4ae84..762a74f 100644 --- a/packages/nightly-core/tests/test_admission.py +++ b/packages/nightly-core/tests/test_admission.py @@ -172,3 +172,69 @@ def test_dead_dispatches_do_not_occupy_a_slot(monkeypatch: pytest.MonkeyPatch) - monkeypatch.setattr(dispatch_mod, "list_dispatches", lambda _root=None: [stale, stale]) monkeypatch.setattr(dispatch_mod, "is_alive", lambda _pid: False) assert dispatch_mod.active_dispatches() == [] + + +# ── worktree cap (RFC 012 B3) ───────────────────────────────────────────── + + +async def _fake_git(handles: int): + """Git runner whose `worktree list --porcelain` reports `handles` + Nightly-owned worktrees, and whose `worktree add` always succeeds.""" + lines = [] + for i in range(handles): + lines += [f"worktree /tmp/wt-{i}", f"branch refs/heads/nightly/task-{i}", ""] + porcelain = "\n".join(lines).encode() + + async def run(args, _root): + if args[:2] == ["worktree", "list"]: + return porcelain, b"", 0 + return b"", b"", 0 + + return run + + +@pytest.mark.asyncio +async def test_worktree_creation_blocked_at_cap(tmp_path: Path) -> None: + from nightly_core.worktree import WorktreeCapReached, create_worktree + + with pytest.raises(WorktreeCapReached) as excinfo: + await create_worktree(tmp_path, "new-task", runner=await _fake_git(8), max_worktrees=8) + assert excinfo.value.live == 8 + assert excinfo.value.cap == 8 + assert "max_worktrees" in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_worktree_creation_allowed_below_cap(tmp_path: Path) -> None: + from nightly_core.worktree import create_worktree + + handle = await create_worktree(tmp_path, "new-task", runner=await _fake_git(3), max_worktrees=8) + assert handle.branch.startswith("nightly/new-task") + + +@pytest.mark.asyncio +async def test_zero_max_worktrees_is_unlimited(tmp_path: Path) -> None: + from nightly_core.worktree import create_worktree + + handle = await create_worktree( + tmp_path, "new-task", runner=await _fake_git(99), max_worktrees=0 + ) + assert handle.branch.startswith("nightly/new-task") + + +@pytest.mark.asyncio +async def test_cap_is_checked_before_the_branch_is_cut(tmp_path: Path) -> None: + """A refused request must leave nothing behind — no branch, no dir.""" + from nightly_core.worktree import WorktreeCapReached, create_worktree + + calls: list[list[str]] = [] + inner = await _fake_git(8) + + async def run(args, root): + calls.append(list(args)) + return await inner(args, root) + + with pytest.raises(WorktreeCapReached): + await create_worktree(tmp_path, "new-task", runner=run, max_worktrees=8) + + assert not any(a[:2] == ["worktree", "add"] for a in calls) From d09c4b3fdaddd427bb55b89bf76f7444aa12a99f Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:33:51 -0400 Subject: [PATCH 05/27] docs(rules): fleet doctrine as rule 12 (RFC 007 C1/B2, RFC 012 B4/C4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier routing, admission caps, and handoff thresholds all ship enforced — and nothing told any agent they exist. Machinery an agent is never told about is machinery an agent never uses: it would keep dispatching serially into an 8-wide fleet and grinding past its context ceiling instead of handing off. Both RFCs scoped this as "a paragraph on each of six host skill files". Delivering it as rule 12 of the shared rules block is strictly better: `seed_rules` propagates one marker-delimited body to every host's AGENTS.md / CLAUDE.md, so a single edit reaches all seven harnesses and the wording cannot drift between them. The autonomy contract already lives here for exactly that reason. Rule 12 covers four things: - Fan out to the cap when work is independent. Serial dispatch where parallel was available is the most common way an overnight run wastes its night, and `dispatch status` now prints the headroom to use. - Let the tier default stand unless role and complexity genuinely diverge; override via `model_tier:` only then. - Never route around a full reasoning tier by downgrading — wait. This is the rule that protects the reviewer-on-reasoning decision from being quietly undone at 3am. - Hand off at 25% / 50% of the model's window, carrying goals and state rather than a transcript. Shedding the history is the point. 1223 tests pass (5 new, asserting the doctrine is present and lands inside the seeded markers — a rule outside them propagates nowhere). ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 7 ++- ...2-fleet-parallelism-and-context-handoff.md | 7 ++- .../nightly-core/src/nightly_core/rules.py | 39 ++++++++++++++++ packages/nightly-core/tests/test_rules.py | 46 +++++++++++++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index 5be17b5..99582b3 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -524,6 +524,11 @@ missing config; README updated. See `nightly_core.model_probe` and RFC 007 Resolved #12. **Phase C — Auto-tag heuristic + doctor + docs** -- [ ] C1. Auto-tag scoping paragraph on six host skills +- [x] C1. Tier-scoping guidance — delivered as **rule 12 of the shared + rules block** (`nightly_core.rules`) rather than six near-duplicate + skill files. `seed_rules` propagates one marker-delimited block to + every host's AGENTS.md / CLAUDE.md, so one edit reaches all seven + harnesses and cannot drift between them. B2 is satisfied by the + same change. - [ ] C2. Doctor flags missing `model_tiers` block - [ ] C3. README "Cost-aware dispatch" section diff --git a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index 39020e1..e74d8a0 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -268,7 +268,8 @@ produces a handoff summary naming its unfinished goals. cut so a refused request leaves nothing behind. Both callers (`nightly worktree`, the headless driver) pass the configured cap; CLI exits 3, matching `dispatch start`. -- [ ] B4. Six host skills: fan-out-to-cap guidance +- [x] B4. Fan-out-to-cap guidance — same delivery as RFC 007 C1: rule 12 + of the shared rules block, not six skill files. - [x] B5. Admission tests (`tests/test_admission.py`, 14 cases) — including the liveness rule: a dispatch whose PID is gone must not occupy a slot, or an unpolled crash wedges the fleet @@ -277,4 +278,6 @@ produces a handoff summary naming its unfinished goals. - [ ] C1. Keepalive threshold comparison + prompt injection - [ ] C2. `handoff:` section in `digest.md` - [ ] C3. Briefing handoff counts -- [ ] C4. Six host skills: two-threshold protocol text +- [x] C4. Two-threshold protocol text — in rule 12. (C1–C3, the + keepalive enforcement, remain open: the doctrine is documented but + the hook does not yet compare against the thresholds.) diff --git a/packages/nightly-core/src/nightly_core/rules.py b/packages/nightly-core/src/nightly_core/rules.py index dc48276..7aaf647 100644 --- a/packages/nightly-core/src/nightly_core/rules.py +++ b/packages/nightly-core/src/nightly_core/rules.py @@ -253,6 +253,45 @@ solution was the cap, which then created the new failure of ending sessions early. v0.0.3+ instead consolidates without capping. +12. **Run the fleet wide and cheap; spend reasoning tokens only on + judgment.** Nightly routes each dispatch to a model sized for the + job (RFC 007) and caps how many run at once (RFC 012). Both are + enforced — `nightly dispatch start` refuses over-cap requests with + exit 3, and worktree creation refuses past `max_worktrees` — so the + contract here is about *using* the headroom, not respecting it. + + - **Fan out to the cap by default.** When a task decomposes into + independent work (several files to read, several modules to + probe, several tests to write), dispatch those specialists in one + batch rather than serially. `nightly dispatch status` prints a + `capacity:` line showing live/cap per tier; if a tier has + headroom and the work is independent, use it. Serial dispatch + when parallel was available is the most common way an overnight + run wastes its night. + - **Let the tier default stand unless role and complexity + diverge.** implementer/tester are `coding`, reviewer is + `reasoning` (review *is* result validation, and nothing + downstream re-checks it), researcher is `lite` (file search and + summarization over code already on disk). Override with + `model_tier:` in the plan frontmatter only when the task's + complexity genuinely differs from its role — a one-line README + fix dispatched through implementer is `lite`; an architecture + audit dispatched through researcher is `reasoning`. + - **Never route around a full reasoning tier by downgrading.** If + the reasoning slots are busy, wait for one. Silently taking a + lite-tier review is how a bad diff reaches a PR with an LGTM + nobody should trust. + - **Hand off before you degrade.** Each agent gets a soft threshold + at 25% of its model's context window and a hard one at 50% + (`context.handoff_*_ratio`; ratios, so they scale with whatever + model the tier resolved to). At the soft threshold: finish the + task you are on, write a handoff summary naming the goals that + remain, and let a fresh agent resume with a clean context. At the + hard threshold: stop where you are and write the summary anyway — + past halfway, "just one more step" risks truncating a write, + which loses work rather than merely wasting tokens. The summary + carries goals and state, never a transcript; shedding the history + is the whole point. ### Human shutdown intervention diff --git a/packages/nightly-core/tests/test_rules.py b/packages/nightly-core/tests/test_rules.py index 926f17b..102933e 100644 --- a/packages/nightly-core/tests/test_rules.py +++ b/packages/nightly-core/tests/test_rules.py @@ -8,6 +8,7 @@ MARKER_END, MARKER_START, NIGHTLY_RULES_BODY, + _render_block, seed_rules, ) @@ -221,3 +222,48 @@ def test_rules_body_documents_blocking_pr_rescue_preempts_accepted_rfc() -> None assert "Draft PRs count too" in body or "draft" in body.lower() # Positive: `nightly verify` reminder before push. assert "nightly verify" in body + + +# ── rule 12: fleet doctrine (RFC 007 C1 / RFC 012 B4) ──────────────────── + + +def test_rules_body_documents_the_tier_defaults() -> None: + """The machinery is invisible unless the rules block names it. + + Routing, caps, and handoff thresholds all ship enforced, but an agent + that is never told the defaults will not override them intelligently + — and will not fan out at all. + """ + body = NIGHTLY_RULES_BODY + assert "model_tier:" in body + for role in ("implementer", "tester", "reviewer", "researcher"): + assert role in body + + +def test_rules_body_forbids_downgrading_past_a_full_reasoning_tier() -> None: + """The one rule that protects RFC 007's reviewer decision.""" + body = NIGHTLY_RULES_BODY.lower() + assert "downgrad" in body + assert "wait for one" in body + + +def test_rules_body_states_both_handoff_thresholds() -> None: + body = NIGHTLY_RULES_BODY + assert "25%" in body + assert "50%" in body + assert "handoff" in body.lower() + + +def test_rules_body_tells_the_agent_to_fan_out() -> None: + body = NIGHTLY_RULES_BODY.lower() + assert "fan out" in body + assert "capacity:" in NIGHTLY_RULES_BODY + + +def test_rule_12_is_inside_the_seeded_block() -> None: + """Rule 12 must land between the markers, or `seed_rules` won't + propagate it to AGENTS.md / CLAUDE.md on any host.""" + rendered = _render_block() + assert rendered.startswith(MARKER_START) + assert rendered.rstrip().endswith(MARKER_END) + assert "Run the fleet wide and cheap" in rendered From b6669a7e0dce75d3b08ab225cacfbf7e0f21bd5f Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:39:07 -0400 Subject: [PATCH 06/27] docs(readme): cost-aware dispatch, fleet caps, context handoff (RFC 007 C3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything added this session was undiscoverable: `model_tiers`, `parallelism`, the handoff ratios, `--tier`, `--force`, exit code 3, and init-time model-control discovery had no user-facing documentation at all. A feature nobody can find is a feature nobody has. Three new README sections under "How it works": cost-aware dispatch (the tier table, the role defaults and why reviewer sits on reasoning, the plan-frontmatter override, and what init discovers), context handoff (the two thresholds as fractions of the resolved model's window, with worked examples for 1M and 200K), and an enforcement note on the existing parallelism section with a sample `capacity:` line. Also fixes pre-existing drift found while editing: the entire `nightly dispatch` command family (start / status / tail / wait) was missing from the CLI reference, and `nightly specialist` was listed without `--tier`. The new `tests/test_readme_claims.py` pins the load-bearing claims — tier defaults, effort defaults, handoff ratios and their arithmetic, the fallback window, exit code 3, and the `capacity:` line's tier order — against the code that implements them. A restatement with no test is a promise with no enforcement, and these tables restate values that live in `SPECIALIST_TIER_DEFAULTS`, `DEFAULT_TIER_EFFORT`, and `ContextConfig`. Deliberately narrow: it tests facts, not wording, so prose stays free to change. 1230 tests pass (7 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 7 +- README.md | 95 +++++++++++++++- .../nightly-core/tests/test_readme_claims.py | 104 ++++++++++++++++++ 3 files changed, 204 insertions(+), 2 deletions(-) create mode 100644 packages/nightly-core/tests/test_readme_claims.py diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index 99582b3..753923b 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -531,4 +531,9 @@ missing config; README updated. harnesses and cannot drift between them. B2 is satisfied by the same change. - [ ] C2. Doctor flags missing `model_tiers` block -- [ ] C3. README "Cost-aware dispatch" section +- [x] C3. README "Cost-aware dispatch" section — plus a "Context + handoff" section, the `parallelism:` enforcement note, and the + previously-undocumented `nightly dispatch` command family in the + CLI reference. Backed by `tests/test_readme_claims.py`, which pins + the tier/effort/ratio tables against the code so prose and + behavior cannot drift apart. diff --git a/README.md b/README.md index 0112bd3..8eb4df2 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,11 @@ keepalive turn (configurable via `context.digest_every_turns`). | **Cascade** | `nightly next` | Walk the priority cascade; print the next pick + rationale. | | | `nightly triage [--top N]` | List ranked open GitHub issues (best-effort, needs `gh`). | | | `nightly plans` | Every plan across runs with status. | -| | `nightly specialist ` | Print the system prompt for one of the 4 roles. | +| | `nightly specialist [--tier ]` | Print the system prompt for one of the 4 roles; `--tier` appends the deliberation directive. | +| | `nightly dispatch start --role [--force]` | Spawn a background specialist. Exits 3 when the tier is at capacity. | +| | `nightly dispatch status []` | List dispatches; prints live/cap headroom per tier. | +| | `nightly dispatch tail ` | Follow a running dispatch's log. | +| | `nightly dispatch wait ` | Block until a dispatch finishes. | | | `nightly keepalive [--name ]` | Show think-harder strategies when the cascade goes empty. | | **Ideation** | `nightly propose [--top N]` | Dry-run the proposer suite — list candidates. | | | `nightly ideate` | Run proposers; write draft issues to disk. | @@ -439,6 +443,95 @@ N` in parallel) it: Single-process by contract: two concurrent `nightly run` invocations against the same repo can race on plan-status updates. +Fan-out is bounded by the `parallelism:` block, and the bounds are +enforced rather than advisory — `nightly dispatch start` exits 3 when a +tier is at capacity, and worktree creation refuses past `max_worktrees`. +`nightly dispatch status` prints the current headroom: + +``` +capacity: lite 2/8 coding 6/6 FULL reasoning 1/2 (total 9/8) +``` + +`0` means unlimited on any axis. A blocked dispatch always means *wait*, +never "run it on a cheaper model" — see below for why. + +### Cost-aware dispatch + +Not every task needs the same model. Nightly sorts dispatches into three +tiers named after task complexity, so the abstraction survives model +churn — when a vendor ships a new coding model, only one config line +changes. + +| Tier | Does | Default effort | +|---|---|---| +| `lite` | file search, summarization, docs | `low` | +| `coding` | implementation, test authoring | `low` | +| `reasoning` | orchestration, result validation, merge adjudication | `xhigh` | + +Each specialist role has a default tier: + +| Role | Tier | Why | +|---|---|---| +| `implementer` | `coding` | output is gated by `nightly verify` | +| `tester` | `coding` | same gate | +| `reviewer` | **`reasoning`** | review *is* result validation — nothing downstream re-checks it | +| `researcher` | **`lite`** | file search and summarization over code already on disk | + +The axis is *how expensive it is to be wrong and not notice*, not how +senior the role sounds. A cheap implementer's mistakes surface as a red +lint/type/test run; a cheap reviewer's mistakes surface as a merged bug. +That's also why a full reasoning tier makes a dispatch **wait** rather +than silently downgrade — a lite-tier review nobody asked for is worse +than a slow one. + +Override per task with `model_tier:` in the plan's frontmatter, when a +task's complexity genuinely differs from its role — a one-line README fix +dispatched through `implementer` is `lite`; an architecture audit +dispatched through `researcher` is `reasoning`. + +Effort ships as prompt text rather than a vendor flag, because the flag +surface differs per host and several hosts expose none. `low` means act +rather than deliberate: consolidate tool calls, skip preamble, reach +`nightly verify` quickly instead of reasoning your way to certainty. + +**`nightly init` discovers the model controls** rather than shipping a +table that goes stale. It reads each installed host CLI's own `--help` +for the model-selection flag and the model ids it advertises, detects +which harness is running init, and writes the result to +`.nightly/config.yml`. A discovered *pinned* id (`claude-opus-5`) +overrides the seeded default; a bare alias (`opus`) does not, because +aliases float to whatever shipped most recently — fine interactively, +wrong for an overnight run whose model should still be identifiable in +the morning. Any probe failure degrades to the seeded defaults, so init +never fails because a host CLI misbehaved. `nightly doctor` flags hosts +with no tier binding. + +### Context handoff + +An agent is most productive early in its context window. Past halfway it +re-reads its own history, thrashes, and eventually truncates mid-write — +worst precisely when it is deepest into valuable work. + +So each agent gets two thresholds, expressed as **fractions of whatever +model the tier resolved to** rather than absolute token counts. One +setting therefore governs a mixed fleet: + +| Model window | Soft (0.25) | Hard (0.50) | +|---|---|---| +| 1M | 250K | 500K | +| 200K | 50K | 100K | + +At the **soft** threshold: finish the task you are on, write a handoff +summary naming the goals that remain, and let a fresh agent resume with a +clean context. At the **hard** threshold: stop where you are and write +the summary anyway. The summary carries goals and state, never a +transcript — shedding the history is the entire point. + +Declare windows for models Nightly doesn't know under +`context.model_context_tokens:`; anything undeclared falls back to a +conservative 200K, since under-estimating costs one harmless early +handoff while over-estimating costs a truncated one. + --- ## Repo layout diff --git a/packages/nightly-core/tests/test_readme_claims.py b/packages/nightly-core/tests/test_readme_claims.py new file mode 100644 index 0000000..c1fa3e4 --- /dev/null +++ b/packages/nightly-core/tests/test_readme_claims.py @@ -0,0 +1,104 @@ +"""Pin README claims against the code that implements them. + +Documentation drift is the failure mode these guard: the README's tier +tables and threshold numbers restate values that live in code, and a +restatement with no test is a promise with no enforcement. Each test here +fails when the code moves and the prose doesn't. + +Deliberately narrow — this checks *load-bearing factual claims* (tier +defaults, ratios, exit codes), not wording or structure. Prose should be +free to change without a test failing. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from nightly_core.config import DEFAULT_TIER_EFFORT, ContextConfig +from nightly_core.contract import MODEL_TIERS +from nightly_core.specialists import SPECIALIST_TIER_DEFAULTS + +REPO_ROOT = Path(__file__).resolve().parents[3] +README = REPO_ROOT / "README.md" + + +@pytest.fixture(scope="module") +def readme() -> str: + if not README.is_file(): + pytest.skip(f"README not found at {README}") + return README.read_text(encoding="utf-8") + + +def test_readme_documents_every_tier(readme: str) -> None: + for tier in MODEL_TIERS: + assert f"`{tier}`" in readme, f"tier {tier} is undocumented" + + +def test_readme_role_table_matches_the_code(readme: str) -> None: + """The reviewer/researcher assignment is the surprising part of the + design; a README that disagrees with it is worse than no README.""" + for role, tier in SPECIALIST_TIER_DEFAULTS.items(): + # The row reads e.g. "| `reviewer` | **`reasoning`** | ..." — the + # emphasis varies, so match role and tier on the same line. + rows = [ln for ln in readme.splitlines() if f"`{role}`" in ln and ln.startswith("|")] + assert rows, f"no README table row for role {role}" + assert any(tier in row for row in rows), ( + f"README lists {role} without its actual default tier {tier!r}" + ) + + +def test_readme_effort_defaults_match_the_code(readme: str) -> None: + for tier, effort in DEFAULT_TIER_EFFORT.items(): + rows = [ln for ln in readme.splitlines() if f"`{tier}`" in ln and ln.startswith("|")] + assert any(f"`{effort}`" in row for row in rows), ( + f"README does not show {tier}'s default effort {effort!r}" + ) + + +def test_readme_handoff_table_matches_the_ratios(readme: str) -> None: + """The worked examples are arithmetic on the defaults — if the ratios + change, the table is wrong, not merely stale.""" + cfg = ContextConfig() + # Compare numerically: the README writes `0.50` for symmetry with + # `0.25`, which is not `repr(0.5)`. Prose formatting is not the thing + # under test — the value is. + documented = { + label: float(value) for label, value in re.findall(r"(Soft|Hard) \(([\d.]+)\)", readme) + } + assert documented.get("Soft") == cfg.handoff_soft_ratio + assert documented.get("Hard") == cfg.handoff_hard_ratio + + for window, label in ((1_000_000, "1M"), (200_000, "200K")): + soft = int(window * cfg.handoff_soft_ratio) + hard = int(window * cfg.handoff_hard_ratio) + row = next( + (ln for ln in readme.splitlines() if ln.startswith("|") and f"| {label} |" in ln), + None, + ) + assert row is not None, f"no handoff row for a {label} window" + assert f"{soft // 1000}K" in row + assert f"{hard // 1000}K" in row + + +def test_readme_default_context_window_matches_the_code(readme: str) -> None: + fallback = ContextConfig().default_context_tokens + assert f"{fallback // 1000}K" in readme + + +def test_readme_documents_the_at_capacity_exit_code(readme: str) -> None: + """Exit 3 is the contract `dispatch start` and worktree creation share.""" + assert "exits 3" in readme.lower() or "exit 3" in readme.lower() + + +def test_readme_capacity_line_matches_the_printed_format(readme: str) -> None: + """The sample output is a promise about what the operator will see.""" + assert "capacity:" in readme + sample = next(ln for ln in readme.splitlines() if ln.strip().startswith("capacity:")) + for tier in MODEL_TIERS: + assert tier in sample, f"sample capacity line omits {tier}" + # Tiers must appear in the same order the code emits them. + positions = [sample.index(tier) for tier in MODEL_TIERS] + assert positions == sorted(positions) From 4b6606fb6072a3aab7faa56daa764487edf82fda Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:43:58 -0400 Subject: [PATCH 07/27] =?UTF-8?q?feat(briefing):=20dispatches-by-tier=20pa?= =?UTF-8?q?nel=20(RFC=20007=20B3)=20=E2=80=94=20completes=20RFC=20007?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last unticked item in RFC 007. Routing has been enforced for several commits with no way for the operator to see whether it did what they configured — which is the only question the tier design actually raises. A run showing `reasoning x 12` is burning the expensive tier on work that should have been cheap; one showing no `reasoning` at all probably reviewed nothing. Three implementation choices: - **Reads each task's `dispatch.json` directly**, not via `list_dispatches`, which resolves the *current* run. The briefing renders arbitrary runs, including concluded ones — going through the current-run resolver would silently render an empty panel for exactly the runs an operator reviews in the morning. - **Untiered records surface as `unrouted`** rather than being dropped. Omitting them would make a pre-RFC-007 run look like it dispatched less than it did. - **A corrupt state file is skipped, not fatal.** One unparseable `dispatch.json` must not sink the whole briefing. The panel renders independently of the session narrative, so a run where no narrative was authored still shows its dispatch mix. RFC 007 is now fully implemented across all three phases. 1237 tests pass (7 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 8 +- .../nightly-core/src/nightly_core/briefing.py | 50 +++++++++++ .../nightly_core/templates/briefing.html.j2 | 10 +++ packages/nightly-core/tests/test_briefing.py | 83 +++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index 753923b..a875d66 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -10,6 +10,7 @@ author: nightly-seed source: interactive_seed estimated_effort: ~7h across 3 phases phase_a: implemented +status_note: all three phases implemented 2026-07-28 --- # RFC 007 — Model-tier routing for cost-aware specialist dispatch @@ -514,7 +515,12 @@ missing config; README updated. - [x] B1. `nightly dispatch start` reads resolved model id and passes it with the **discovered** model flag (see B6) - [ ] B2. Task-tool fallback documented across six host skill.md -- [ ] B3. Briefing tier-breakdown line +- [x] B3. Briefing tier-breakdown line — a `dispatches by tier` panel + rendered from each task's `dispatch.json`. Reads the run's task + dirs directly rather than via `list_dispatches`, which resolves the + *current* run; the briefing renders arbitrary (including concluded) + runs. Untiered pre-RFC-007 records are surfaced as `unrouted` + rather than dropped. - [x] B4. `nightly specialist --tier ` flag - [x] B5. Dispatch resolution tests across tiers + host-miss fallback (`tests/test_routing.py`); end-to-end argv assertions still open diff --git a/packages/nightly-core/src/nightly_core/briefing.py b/packages/nightly-core/src/nightly_core/briefing.py index 6dc99c7..2add361 100644 --- a/packages/nightly-core/src/nightly_core/briefing.py +++ b/packages/nightly-core/src/nightly_core/briefing.py @@ -21,6 +21,7 @@ from __future__ import annotations +import json from dataclasses import dataclass from datetime import UTC, datetime from pathlib import Path @@ -30,6 +31,7 @@ from markdown_it import MarkdownIt from markupsafe import Markup +from nightly_core.contract import MODEL_TIERS from nightly_core.runs import Run __all__ = [ @@ -98,6 +100,52 @@ class BriefingContext: """RFC 006 §B2 — "yes" if session compaction fired, "no" if it did not, None if default omitted (e.g. keepalive.log absent).""" + tier_breakdown: str | None = None + """RFC 007 §8 — dispatches by model tier, e.g. `lite x 3, coding x 5, + reasoning x 1`. None when the run backgrounded no specialists. + + This is the operator's only view into whether routing is doing what + they configured. A run that shows `reasoning x 12` is burning the + expensive tier on work that should have been cheap; one that shows no + `reasoning` at all probably reviewed nothing.""" + + +def _load_tier_breakdown(run: Run) -> str | None: + """Count this run's dispatches by model tier — RFC 007 Resolved #8. + + Reads each task's `dispatch.json` directly rather than going through + `list_dispatches`, which resolves the *current* run; the briefing + renders arbitrary runs, including concluded ones. + + Untiered records (written before RFC 007) are counted under + `unrouted` rather than dropped — silently omitting them would make an + old run look like it dispatched less than it did. + """ + tasks_dir = run.path / "tasks" + if not tasks_dir.is_dir(): + return None + + counts: dict[str, int] = {} + for task_dir in sorted(tasks_dir.iterdir()): + state = task_dir / "dispatch.json" + if not state.is_file(): + continue + try: + data = json.loads(state.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + # A corrupt state file must not sink the whole briefing. + continue + tier = data.get("tier") or "unrouted" + counts[str(tier)] = counts.get(str(tier), 0) + 1 + + if not counts: + return None + + # Canonical tier order first, then anything unrecognized. + ordered = [t for t in MODEL_TIERS if t in counts] + ordered += sorted(k for k in counts if k not in MODEL_TIERS) + return ", ".join(f"{name} x {counts[name]}" for name in ordered) + def _load_tasks(run: Run) -> list[dict[str, Any]]: tasks_dir = run.path / "tasks" @@ -293,6 +341,7 @@ def build_context(run: Run, *, now: datetime | None = None) -> BriefingContext: stacked_geometry=stacked, current_branch=current_branch, compacted=compacted, + tier_breakdown=_load_tier_breakdown(run), ) @@ -315,6 +364,7 @@ def render_briefing(run: Run, *, now: datetime | None = None) -> str: stacked_geometry=ctx.stacked_geometry, current_branch=ctx.current_branch, compacted=ctx.compacted, + tier_breakdown=ctx.tier_breakdown, ) diff --git a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 index df53d1d..f31c339 100644 --- a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 +++ b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 @@ -202,6 +202,16 @@ {% endif %} + {# ── dispatch mix by model tier (RFC 007 §8) ─────────────────────── #} + {% if tier_breakdown %} +
+
dispatches by tier
+
+ {{ tier_breakdown }} +
+
+ {% endif %} + {# ── session narrative (agent-authored, optional) ────────────────── #} {% if session_narrative %}
diff --git a/packages/nightly-core/tests/test_briefing.py b/packages/nightly-core/tests/test_briefing.py index c4afce4..a70ee27 100644 --- a/packages/nightly-core/tests/test_briefing.py +++ b/packages/nightly-core/tests/test_briefing.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from datetime import UTC, datetime from pathlib import Path @@ -339,3 +340,85 @@ def test_briefing_omits_compacted_slot_when_absent(tmp_path: Path) -> None: assert ctx.compacted is None html = render_briefing(run) assert "Compacted:" not in html + + +# ── tier breakdown (RFC 007 §8 / B3) ────────────────────────────────────── + + +def _write_dispatch(run_path: Path, slug: str, tier: str | None) -> None: + task_dir = run_path / "tasks" / slug + task_dir.mkdir(parents=True, exist_ok=True) + payload: dict[str, object] = {"slug": slug, "pid": 1, "status": "completed"} + if tier is not None: + payload["tier"] = tier + (task_dir / "dispatch.json").write_text(json.dumps(payload), encoding="utf-8") + + +def test_tier_breakdown_is_none_without_dispatches(tmp_path: Path) -> None: + from nightly_core.briefing import _load_tier_breakdown + + run = start_run(tmp_path) + assert _load_tier_breakdown(run) is None + + +def test_tier_breakdown_counts_each_tier(tmp_path: Path) -> None: + from nightly_core.briefing import _load_tier_breakdown + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", "lite") + _write_dispatch(run.path, "0002-b", "lite") + _write_dispatch(run.path, "0003-c", "reasoning") + assert _load_tier_breakdown(run) == "lite x 2, reasoning x 1" + + +def test_tier_breakdown_uses_canonical_tier_order(tmp_path: Path) -> None: + """Reading order should match the tier ladder, not directory order.""" + from nightly_core.briefing import _load_tier_breakdown + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", "reasoning") + _write_dispatch(run.path, "0002-b", "lite") + _write_dispatch(run.path, "0003-c", "coding") + assert _load_tier_breakdown(run) == "lite x 1, coding x 1, reasoning x 1" + + +def test_untiered_dispatches_are_surfaced_not_dropped(tmp_path: Path) -> None: + """Pre-RFC-007 records have no tier; omitting them would make an old + run look like it dispatched less than it actually did.""" + from nightly_core.briefing import _load_tier_breakdown + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", None) + _write_dispatch(run.path, "0002-b", "coding") + assert _load_tier_breakdown(run) == "coding x 1, unrouted x 1" + + +def test_corrupt_dispatch_state_does_not_sink_the_briefing(tmp_path: Path) -> None: + from nightly_core.briefing import _load_tier_breakdown + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", "lite") + bad = run.path / "tasks" / "0002-b" + bad.mkdir(parents=True, exist_ok=True) + (bad / "dispatch.json").write_text("{not json", encoding="utf-8") + assert _load_tier_breakdown(run) == "lite x 1" + + +def test_tier_breakdown_reaches_the_rendered_briefing(tmp_path: Path) -> None: + from nightly_core.briefing import render_briefing + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", "reasoning") + html = render_briefing(run) + assert "dispatches by tier" in html + assert "reasoning x 1" in html + + +def test_breakdown_renders_without_a_session_narrative(tmp_path: Path) -> None: + """The mix is useful even on a run where no narrative was authored.""" + from nightly_core.briefing import render_briefing + + run = start_run(tmp_path) + _write_dispatch(run.path, "0001-a", "coding") + assert not (run.path / "briefing.md").exists() + assert "coding x 1" in render_briefing(run) From b7ec2c68ed91e6dc80ad8058a9a8df2f82100fec Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:50:08 -0400 Subject: [PATCH 08/27] feat(keepalive): inject handoff instructions at the context thresholds (RFC 012 C1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handoff protocol has been documented (rule 12) and computable (`resolve_context_thresholds`) for several commits with nothing enforcing it. This wires it into the Stop hook, which is the only component that sees the live context estimate every turn. `_apply_context_diet` becomes a three-rung ladder, most severe first: hard handoff (stop mid-task), soft handoff (finish this task, then hand off), and the original v0.0.12 diet nudge. A handoff block **replaces** the diet block rather than stacking with it — past a handoff threshold "use less context" is no longer the right instruction, and two competing directives in one prompt is how an agent ends up following neither. Two things worth a reviewer's attention: - **This changes default behavior.** At a 1M window the soft threshold (250K) sits below the default `budget_tokens` (256K), so the diet block is effectively superseded at defaults. That is the specified intent — hand off at ~25% of the window — not an accident. Two existing tests asserted the diet block at a 300K estimate; they now assert the handoff, and a third case covers the diet path at an estimate below the soft threshold, where it is still the right instruction. - **Session thresholds assume the reasoning tier's model.** The hook cannot read the host's live model selection, and rule 12 puts orchestration on reasoning. A wrong guess degrades safely: an unknown model falls back to 200K, handing off earlier than a large window needs rather than later than a small one can afford. The summary is specified as goals-and-state, explicitly NOT a transcript. An agent told only "write a summary" reliably writes a transcript, which re-injects the history the handoff exists to shed. Config trouble can never break the hook — threshold resolution is wrapped, and a broken config degrades to the diet rung. There's a test for it: the Stop hook runs at every turn boundary, so it must degrade, not raise. 1247 tests pass (10 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...2-fleet-parallelism-and-context-handoff.md | 11 +- .../src/nightly_core/keepalive_hook.py | 112 +++++++++++++++-- .../tests/test_keepalive_context.py | 119 +++++++++++++++++- 3 files changed, 227 insertions(+), 15 deletions(-) diff --git a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index e74d8a0..17ac329 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -9,6 +9,8 @@ author: operator source: interactive_seed estimated_effort: ~5h across 3 phases phase_a: implemented +phase_b: implemented +phase_c: partial (C1) --- # RFC 012 — Fleet parallelism and context-handoff protocol @@ -274,8 +276,13 @@ produces a handoff summary naming its unfinished goals. including the liveness rule: a dispatch whose PID is gone must not occupy a slot, or an unpolled crash wedges the fleet -**Phase C — Handoff protocol** -- [ ] C1. Keepalive threshold comparison + prompt injection +**Phase C — Handoff protocol** — *C1 landed 2026-07-28* +- [x] C1. Keepalive threshold comparison + prompt injection — a three-rung + ladder (hard handoff > soft handoff > the v0.0.12 diet nudge). The + handoff block *replaces* the diet block rather than stacking; two + competing directives in one prompt is how an agent follows neither. + Session thresholds resolve against the reasoning tier's model, per + Resolved #8 and rule 12. - [ ] C2. `handoff:` section in `digest.md` - [ ] C3. Briefing handoff counts - [x] C4. Two-threshold protocol text — in rule 12. (C1–C3, the diff --git a/packages/nightly-core/src/nightly_core/keepalive_hook.py b/packages/nightly-core/src/nightly_core/keepalive_hook.py index dadad2e..c9bddd6 100644 --- a/packages/nightly-core/src/nightly_core/keepalive_hook.py +++ b/packages/nightly-core/src/nightly_core/keepalive_hook.py @@ -79,8 +79,10 @@ from typing import Any from nightly_core.cascade import CascadeChoice, next_task -from nightly_core.config import load_context_config +from nightly_core.config import load_context_config, load_model_tier_config from nightly_core.digest import write_digest +from nightly_core.model_probe import detect_harness +from nightly_core.routing import ContextThresholds, resolve_context_thresholds from nightly_core.runs import current_run __all__ = [ @@ -100,9 +102,11 @@ "disarm_session", "estimate_context_tokens", "format_decision", + "handoff_block", "log_heartbeat", "read_respawn_marker", "request_stop", + "session_context_thresholds", ] @@ -412,6 +416,65 @@ def context_diet_block(estimate: int, budget: int) -> str: ) +def handoff_block(estimate: int, thresholds: ContextThresholds, breach: str) -> str: + """Render the RFC 012 handoff instruction for a soft or hard breach. + + Supersedes the context-diet nudge rather than accompanying it: past a + handoff threshold, hygiene is no longer the right advice — recycling + is. The diet block says "use less context"; this says "stop + accumulating it and hand the goals to a fresh agent". + + The summary is specified as goals-and-state, never a transcript. + Re-injecting the history into the successor would defeat the entire + exercise, and an agent told only "write a summary" reliably writes a + transcript. + """ + est_k = round(estimate / 1000) + win_k = round(thresholds.window_tokens / 1000) + limit_k = round((thresholds.hard_tokens if breach == "hard" else thresholds.soft_tokens) / 1000) + common = ( + "Write the summary to `.nightly/runs//tasks//HANDOFF.md`: what the " + "task is, what is done, what remains, and anything you learned that is not " + "already on disk. Carry goals and state — NOT a transcript. Shedding the " + "history is the point of handing off; re-injecting it defeats the exercise. " + "Then end your turn so a fresh agent can resume with a clean context." + ) + if breach == "hard": + return ( + f"HARD CONTEXT HANDOFF: ~{est_k}K tokens against a {win_k}K window " + f"(hard threshold {limit_k}K). STOP the work you are on now — do not " + "finish it, do not start anything new. Past this point a single further " + "step risks truncating a write mid-way, which loses work rather than " + f"merely wasting tokens. {common}\n\n" + ) + return ( + f"CONTEXT HANDOFF: ~{est_k}K tokens against a {win_k}K window (soft " + f"threshold {limit_k}K). Finish the task you are on — do not start a new " + f"one — then hand off. {common}\n\n" + ) + + +def session_context_thresholds(root: Path) -> ContextThresholds: + """Resolve this session's handoff thresholds — RFC 012 Resolved #8. + + Thresholds scale with the model's context window, and the hook cannot + read the host's live model selection. It assumes the session runs on + the **reasoning** tier's model, which is what rule 12 tells the + orchestrator to do: orchestration is reasoning-tier work. + + A wrong guess degrades gracefully in the safe direction — an unknown + or unbound model falls back to `default_context_tokens` (200K), which + hands off earlier than a large window needs rather than later than a + small one can afford. + """ + tier_cfg = load_model_tier_config(root) + host = detect_harness() or "claude" + return resolve_context_thresholds( + tier_cfg.binding(host, "reasoning").model, + load_context_config(root), + ) + + @dataclass(frozen=True) class StopHookDecision: """What the Stop hook is going to do, and why. @@ -585,7 +648,7 @@ def compute_stop_hook_decision( # noqa: PLR0912 - one branch per off-ramp / rou spinning = repeats >= _LIVELOCK_REPICKS + _LIVELOCK_ESCALATE_AFTER if spinning: reason = _spin_escalation_block(choice, repeats) + reason - reason = _apply_context_diet(reason, context_estimate, ctx_cfg.budget_tokens) + reason = _apply_context_diet(reason, context_estimate, ctx_cfg.budget_tokens, root) message = ( f"run {run.id} turn {turn_count}: blocking stop and injecting " f"{'ESCALATED ' if spinning else ''}planning-phase prompt " @@ -604,7 +667,7 @@ def compute_stop_hook_decision( # noqa: PLR0912 - one branch per off-ramp / rou run_id=run.id, turn=turn_count, ) - reason = _apply_context_diet(reason, context_estimate, ctx_cfg.budget_tokens) + reason = _apply_context_diet(reason, context_estimate, ctx_cfg.budget_tokens, root) if stop_hook_active: message = ( f"run {run.id} turn {turn_count}: blocking stop " @@ -640,14 +703,43 @@ def _routes_to_planning_phase( return choice.source == "nothing" -def _apply_context_diet(reason: str, estimate: int | None, budget: int) -> str: - """Prepend the context-diet block to `reason` when over the soft budget. +def _apply_context_diet( + reason: str, + estimate: int | None, + budget: int, + root: Path | None = None, +) -> str: + """Prepend context steering to `reason`, escalating with the estimate. + + Three rungs, most severe first: + + 1. **Hard handoff** (RFC 012) — stop mid-task and summarize. + 2. **Soft handoff** (RFC 012) — finish this task, then summarize. + 3. **Context diet** (v0.0.12) — the original hygiene nudge. + + A handoff block *replaces* the diet block rather than stacking with + it: past a handoff threshold "use less context" is no longer the + right instruction, and two competing directives in one prompt is how + an agent ends up following neither. + + No-op when the estimate is None (couldn't measure). The diet rung is + additionally disabled by `budget <= 0`, and each handoff rung by its + ratio being 0 — so an operator can turn any rung off independently. + """ + if estimate is None: + return reason - No-op (returns `reason` unchanged) when the estimate is None (couldn't - measure), the budget is 0 (steering disabled), or the estimate is within - budget. Otherwise the diet block is prepended so the agent reads it - before the continuation instructions.""" - if estimate is None or budget <= 0 or estimate <= budget: + if root is not None: + try: + thresholds = session_context_thresholds(root) + except Exception: + thresholds = None # config trouble must never break the hook + if thresholds is not None: + breach = thresholds.breach(estimate) + if breach is not None: + return handoff_block(estimate, thresholds, breach) + reason + + if budget <= 0 or estimate <= budget: return reason return context_diet_block(estimate, budget) + reason diff --git a/packages/nightly-core/tests/test_keepalive_context.py b/packages/nightly-core/tests/test_keepalive_context.py index 96e6217..5ae50a9 100644 --- a/packages/nightly-core/tests/test_keepalive_context.py +++ b/packages/nightly-core/tests/test_keepalive_context.py @@ -171,8 +171,10 @@ def test_over_budget_prepends_diet_to_normal_reason( ) decision = compute_stop_hook_decision(armed_repo, transcript_path=transcript) reason = decision.payload["reason"] - assert reason.startswith("⚠ CONTEXT BUDGET") - assert "Continue on:" in reason # original reason preserved below the diet + # RFC 012: 300K is past the soft handoff threshold (25% of a 1M window), + # so the handoff instruction supersedes the older diet nudge. + assert reason.startswith("CONTEXT HANDOFF") + assert "Continue on:" in reason # original reason preserved below the block def test_over_budget_prepends_diet_to_planning_phase( @@ -186,7 +188,7 @@ def test_over_budget_prepends_diet_to_planning_phase( ) decision = compute_stop_hook_decision(armed_repo, transcript_path=transcript) reason = decision.payload["reason"] - assert reason.startswith("⚠ CONTEXT BUDGET") + assert reason.startswith("CONTEXT HANDOFF") assert "GENUINE WORK IS NEVER EXHAUSTED" in reason @@ -334,3 +336,114 @@ def test_escalation_threshold_is_above_the_reroute_threshold() -> None: assert _LIVELOCK_ESCALATE_AFTER >= 1 assert _LIVELOCK_REPICKS + _LIVELOCK_ESCALATE_AFTER > _LIVELOCK_REPICKS + + +# ── RFC 012 Phase C: handoff escalation ─────────────────────────────────── + + +def _cfg(tmp_path: Path, body: str) -> Path: + (tmp_path / ".nightly").mkdir(parents=True, exist_ok=True) + (tmp_path / ".nightly" / "config.yml").write_text(body, encoding="utf-8") + return tmp_path + + +def test_session_thresholds_assume_the_reasoning_tier(tmp_path: Path) -> None: + """Rule 12 puts orchestration on reasoning; the hook resolves to match.""" + from nightly_core.keepalive_hook import session_context_thresholds + + _cfg(tmp_path, "hosts:\n - claude\n") + t = session_context_thresholds(tmp_path) + # claude reasoning default is a 1M-window model. + assert t.window_tokens == 1_000_000 + assert t.soft_tokens == 250_000 + assert t.hard_tokens == 500_000 + + +def test_soft_breach_says_finish_then_hand_off(tmp_path: Path) -> None: + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "hosts:\n - claude\n") + out = _apply_context_diet("CONTINUE", 300_000, 256_000, tmp_path) + assert out.startswith("CONTEXT HANDOFF:") + assert "Finish the task you are on" in out + assert "HANDOFF.md" in out + assert out.endswith("CONTINUE") + + +def test_hard_breach_says_stop_now(tmp_path: Path) -> None: + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "hosts:\n - claude\n") + out = _apply_context_diet("CONTINUE", 600_000, 256_000, tmp_path) + assert out.startswith("HARD CONTEXT HANDOFF:") + assert "STOP the work you are on now" in out + assert "do not finish it" in out + + +def test_handoff_replaces_the_diet_block_rather_than_stacking(tmp_path: Path) -> None: + """Two competing directives in one prompt is how an agent follows neither.""" + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "hosts:\n - claude\n") + out = _apply_context_diet("CONTINUE", 600_000, 256_000, tmp_path) + assert "CONTEXT BUDGET" not in out + + +def test_handoff_summary_is_specified_as_goals_not_transcript(tmp_path: Path) -> None: + """An agent told only 'write a summary' reliably writes a transcript.""" + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "hosts:\n - claude\n") + out = _apply_context_diet("CONTINUE", 300_000, 256_000, tmp_path) + assert "NOT a transcript" in out + + +def test_below_both_thresholds_falls_through_to_the_diet_block(tmp_path: Path) -> None: + from nightly_core.keepalive_hook import _apply_context_diet + + # Below the 250K soft threshold but above an explicitly lower budget, + # the original hygiene nudge is still the right instruction. + _cfg(tmp_path, "hosts:\n - claude\n") + out = _apply_context_diet("CONTINUE", 150_000, 100_000, tmp_path) + assert "CONTEXT BUDGET" in out + assert "HANDOFF" not in out + + +def test_zero_ratios_disable_handoff_but_keep_the_diet(tmp_path: Path) -> None: + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg( + tmp_path, + "hosts:\n - claude\ncontext:\n handoff_soft_ratio: 0\n handoff_hard_ratio: 0\n", + ) + out = _apply_context_diet("CONTINUE", 900_000, 256_000, tmp_path) + assert "HANDOFF" not in out + assert "CONTEXT BUDGET" in out + + +def test_unmeasurable_context_injects_nothing(tmp_path: Path) -> None: + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "hosts:\n - claude\n") + assert _apply_context_diet("CONTINUE", None, 256_000, tmp_path) == "CONTINUE" + + +def test_broken_config_never_breaks_the_hook(tmp_path: Path) -> None: + """The Stop hook runs every turn boundary; it must degrade, not raise.""" + from nightly_core.keepalive_hook import _apply_context_diet + + _cfg(tmp_path, "model_tiers: [not-a-mapping\n") + out = _apply_context_diet("CONTINUE", 900_000, 256_000, tmp_path) + assert out.endswith("CONTINUE") + + +def test_small_window_hands_off_proportionally_earlier(tmp_path: Path) -> None: + """The whole point of ratios: a 200K agent recycles at 50K/100K.""" + from nightly_core.keepalive_hook import session_context_thresholds + + _cfg( + tmp_path, + "hosts:\n - claude\nmodel_tiers:\n claude:\n reasoning: claude-haiku-4-5\n", + ) + t = session_context_thresholds(tmp_path) + assert (t.window_tokens, t.soft_tokens, t.hard_tokens) == (200_000, 50_000, 100_000) From cf64eca051c44596f11b29d438ada58579b1a37f Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:54:58 -0400 Subject: [PATCH 09/27] =?UTF-8?q?feat(digest,briefing):=20surface=20pendin?= =?UTF-8?q?g=20handoffs=20(RFC=20012=20C2/C3)=20=E2=80=94=20completes=20RF?= =?UTF-8?q?C=20012?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1 made agents write a HANDOFF.md when they cross a context threshold. Nothing read it back, which meant the protocol worked right up until the moment it mattered: a compaction wipes the outgoing agent's context, and a handoff nobody re-reads is a summary written into the void. Two readers, both fed by one `find_handoffs` scanner: - **The digest** gains a `Pending handoffs` section. This is the load- bearing one — the digest is what the `SessionStart(compact)` hook re-injects, so a handoff written just before a compaction survives the event it was written for. - **The briefing** gains a `handed off mid-task` panel. The panel names the task and its outstanding work rather than reporting a count, which is what the RFC's "handoff counts" wording originally implied. A handoff means work was deliberately left unfinished; the operator's first question in the morning is *which* work, and a number cannot answer that. The summary is taken from the first non-heading line, so `# Handoff — alpha` doesn't get reported as the summary of itself. Both readers degrade to silence on unreadable files — the digest renders from the Stop hook's hot path and must never raise. RFC 012 is now fully implemented across all three phases, as is RFC 007. 1258 tests pass (11 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...2-fleet-parallelism-and-context-handoff.md | 12 ++-- .../nightly-core/src/nightly_core/briefing.py | 13 +++- .../nightly-core/src/nightly_core/digest.py | 61 ++++++++++++++++ .../nightly_core/templates/briefing.html.j2 | 12 ++++ packages/nightly-core/tests/test_briefing.py | 37 ++++++++++ packages/nightly-core/tests/test_digest.py | 69 +++++++++++++++++++ 6 files changed, 199 insertions(+), 5 deletions(-) diff --git a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index 17ac329..7116c6f 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -10,14 +10,14 @@ source: interactive_seed estimated_effort: ~5h across 3 phases phase_a: implemented phase_b: implemented -phase_c: partial (C1) +phase_c: implemented --- # RFC 012 — Fleet parallelism and context-handoff protocol ## Status -`accepted` — operator seed in the 2026-07-28 interactive session, +`implemented` — all three phases landed 2026-07-28. Operator seed in the 2026-07-28 interactive session, alongside the RFC 007 amendment. Two knobs that only make sense together: how *wide* the fleet runs, and what an individual agent does when its context fills up. Phase A lands the config schema and the @@ -283,8 +283,12 @@ produces a handoff summary naming its unfinished goals. competing directives in one prompt is how an agent follows neither. Session thresholds resolve against the reasoning tier's model, per Resolved #8 and rule 12. -- [ ] C2. `handoff:` section in `digest.md` -- [ ] C3. Briefing handoff counts +- [x] C2. `Pending handoffs` section in `digest.md` — the digest is what + the `SessionStart(compact)` hook re-injects, so a handoff written + just before a compaction survives the very event it exists for. +- [x] C3. Briefing handoff panel — names the task and its outstanding + work rather than a bare count; "which work was left" is the + operator's actual first question. - [x] C4. Two-threshold protocol text — in rule 12. (C1–C3, the keepalive enforcement, remain open: the doctrine is documented but the hook does not yet compare against the thresholds.) diff --git a/packages/nightly-core/src/nightly_core/briefing.py b/packages/nightly-core/src/nightly_core/briefing.py index 2add361..deef59a 100644 --- a/packages/nightly-core/src/nightly_core/briefing.py +++ b/packages/nightly-core/src/nightly_core/briefing.py @@ -22,7 +22,7 @@ from __future__ import annotations import json -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -32,6 +32,7 @@ from markupsafe import Markup from nightly_core.contract import MODEL_TIERS +from nightly_core.digest import find_handoffs from nightly_core.runs import Run __all__ = [ @@ -100,6 +101,14 @@ class BriefingContext: """RFC 006 §B2 — "yes" if session compaction fired, "no" if it did not, None if default omitted (e.g. keepalive.log absent).""" + handoffs: list[dict[str, str]] = field(default_factory=list) + """RFC 012 C3 — tasks that wrote a `HANDOFF.md` because an agent + crossed a context threshold. Each entry has `slug` and `summary`. + + Worth its own panel rather than a count: a handoff means work was + deliberately left unfinished, and the operator's first question in the + morning is *which* work — a number cannot answer that.""" + tier_breakdown: str | None = None """RFC 007 §8 — dispatches by model tier, e.g. `lite x 3, coding x 5, reasoning x 1`. None when the run backgrounded no specialists. @@ -342,6 +351,7 @@ def build_context(run: Run, *, now: datetime | None = None) -> BriefingContext: current_branch=current_branch, compacted=compacted, tier_breakdown=_load_tier_breakdown(run), + handoffs=[{"slug": slug, "summary": summary} for slug, summary in find_handoffs(run.path)], ) @@ -365,6 +375,7 @@ def render_briefing(run: Run, *, now: datetime | None = None) -> str: current_branch=ctx.current_branch, compacted=ctx.compacted, tier_breakdown=ctx.tier_breakdown, + handoffs=ctx.handoffs, ) diff --git a/packages/nightly-core/src/nightly_core/digest.py b/packages/nightly-core/src/nightly_core/digest.py index 76eb8f4..8014487 100644 --- a/packages/nightly-core/src/nightly_core/digest.py +++ b/packages/nightly-core/src/nightly_core/digest.py @@ -33,6 +33,7 @@ from nightly_core.runs import current_run __all__ = [ + "find_handoffs", "render_digest", "write_digest", ] @@ -104,6 +105,14 @@ def render_digest(root: Path | None = None) -> str: lines.append("") lines.extend(_render_plans(root)) + # ── pending handoffs ────────────────────────────────────────────── + handoffs = _safe_list(_render_handoffs, run.path if run is not None else None) + if handoffs: + lines.append("") + lines.append("## Pending handoffs") + lines.append("") + lines.extend(handoffs) + # ── open PRs ────────────────────────────────────────────────────── lines.append("") lines.append("## Open Nightly PRs") @@ -258,6 +267,58 @@ def _render_open_prs(root: Path | None) -> list[str]: return [f"- #{num} `{branch}`" for branch, num, _url in branches] +def find_handoffs(run_path: Path | None) -> list[tuple[str, str]]: + """`(slug, first meaningful line)` for every `HANDOFF.md` under a run. + + A handoff is written when an agent crosses a context threshold (RFC + 012 C1) and hands its remaining goals to a successor. Surfacing them + is what makes the protocol survive the very event it exists for: a + compaction wipes the outgoing agent's context, and the digest is what + the `SessionStart(compact)` hook re-injects afterwards. A handoff + nobody re-reads is a summary written into the void. + """ + if run_path is None: + return [] + tasks = run_path / "tasks" + if not tasks.is_dir(): + return [] + + out: list[tuple[str, str]] = [] + for task_dir in sorted(tasks.iterdir()): + handoff = task_dir / "HANDOFF.md" + if not handoff.is_file(): + continue + try: + text = handoff.read_text(encoding="utf-8") + except OSError: + continue + # Skip the markdown title and any blank lines — the first prose + # line is the one that says what is actually outstanding. + summary = next( + ( + ln.strip() + for ln in text.splitlines() + if ln.strip() and not ln.lstrip().startswith("#") + ), + "(no summary)", + ) + out.append((task_dir.name, summary)) + return out + + +def _render_handoffs(run_path: Path | None) -> list[str]: + return [f"- `{slug}` — {summary}" for slug, summary in find_handoffs(run_path)] + + +def _safe_list(fn: object, *args: object) -> list[str]: + """`_safe` for section helpers that return lists. Never raises — the + digest is written from the Stop hook's hot path.""" + try: + return fn(*args) # type: ignore[operator] + except Exception: + return [] + + def _read_int(path: Path) -> int: """Read a small integer counter file. 0 on absence / parse failure.""" if not path.is_file(): diff --git a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 index f31c339..44f14a2 100644 --- a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 +++ b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 @@ -202,6 +202,18 @@
{% endif %} + {# ── pending handoffs (RFC 012 C3) ───────────────────────────────── #} + {% if handoffs %} +
+
handed off mid-task
+
    + {% for h in handoffs %} +
  • {{ h.slug }} — {{ h.summary }}
    • + {% endfor %} +
+
+ {% endif %} + {# ── dispatch mix by model tier (RFC 007 §8) ─────────────────────── #} {% if tier_breakdown %}
diff --git a/packages/nightly-core/tests/test_briefing.py b/packages/nightly-core/tests/test_briefing.py index a70ee27..c24b6a8 100644 --- a/packages/nightly-core/tests/test_briefing.py +++ b/packages/nightly-core/tests/test_briefing.py @@ -422,3 +422,40 @@ def test_breakdown_renders_without_a_session_narrative(tmp_path: Path) -> None: _write_dispatch(run.path, "0001-a", "coding") assert not (run.path / "briefing.md").exists() assert "coding x 1" in render_briefing(run) + + +# ── pending handoffs panel (RFC 012 C3) ─────────────────────────────────── + + +def _write_handoff_md(run_path: Path, slug: str, body: str) -> None: + d = run_path / "tasks" / slug + d.mkdir(parents=True, exist_ok=True) + (d / "HANDOFF.md").write_text(body, encoding="utf-8") + + +def test_briefing_context_has_no_handoffs_by_default(tmp_path: Path) -> None: + run = start_run(tmp_path) + assert build_context(run).handoffs == [] + + +def test_briefing_context_collects_handoffs(tmp_path: Path) -> None: + run = start_run(tmp_path) + _write_handoff_md(run.path, "0001-alpha", "# Handoff\n\nRFC 012 C2 still open.\n") + assert build_context(run).handoffs == [ + {"slug": "0001-alpha", "summary": "RFC 012 C2 still open."} + ] + + +def test_handoff_panel_names_the_task_not_just_a_count(tmp_path: Path) -> None: + """The operator's first morning question is *which* work was left.""" + run = start_run(tmp_path) + _write_handoff_md(run.path, "0001-alpha", "# Handoff\n\nAdmission tests remain.\n") + html = render_briefing(run) + assert "handed off mid-task" in html + assert "0001-alpha" in html + assert "Admission tests remain." in html + + +def test_no_handoff_panel_when_the_run_finished_cleanly(tmp_path: Path) -> None: + run = start_run(tmp_path) + assert "handed off mid-task" not in render_briefing(run) diff --git a/packages/nightly-core/tests/test_digest.py b/packages/nightly-core/tests/test_digest.py index 776b361..e2260da 100644 --- a/packages/nightly-core/tests/test_digest.py +++ b/packages/nightly-core/tests/test_digest.py @@ -96,3 +96,72 @@ def test_render_reads_last_history_line(repo: Path, monkeypatch: pytest.MonkeyPa monkeypatch.setattr("nightly_core.cascade.open_nightly_pr_branches", lambda root=None, **kw: []) text = render_digest(repo) assert "accepted_rfc|-|second" in text + + +# ── pending handoffs (RFC 012 C2) ───────────────────────────────────────── + + +def _write_handoff(run_path: Path, slug: str, body: str) -> None: + d = run_path / "tasks" / slug + d.mkdir(parents=True, exist_ok=True) + (d / "HANDOFF.md").write_text(body, encoding="utf-8") + + +def test_no_handoffs_found_in_a_clean_run(tmp_path: Path) -> None: + from nightly_core.digest import find_handoffs + + run = start_run(tmp_path) + assert find_handoffs(run.path) == [] + + +def test_handoff_summary_skips_the_markdown_title(tmp_path: Path) -> None: + """The title says which task; the first prose line says what's left.""" + from nightly_core.digest import find_handoffs + + run = start_run(tmp_path) + _write_handoff(run.path, "0001-alpha", "# Handoff — alpha\n\nB2 remains: six skill files.\n") + assert find_handoffs(run.path) == [("0001-alpha", "B2 remains: six skill files.")] + + +def test_handoff_without_prose_still_reports(tmp_path: Path) -> None: + from nightly_core.digest import find_handoffs + + run = start_run(tmp_path) + _write_handoff(run.path, "0001-alpha", "# Handoff\n\n") + assert find_handoffs(run.path) == [("0001-alpha", "(no summary)")] + + +def test_handoffs_are_sorted_by_slug(tmp_path: Path) -> None: + from nightly_core.digest import find_handoffs + + run = start_run(tmp_path) + _write_handoff(run.path, "0002-beta", "second\n") + _write_handoff(run.path, "0001-alpha", "first\n") + assert [s for s, _ in find_handoffs(run.path)] == ["0001-alpha", "0002-beta"] + + +def test_handoffs_absent_run_is_not_an_error(tmp_path: Path) -> None: + from nightly_core.digest import find_handoffs + + assert find_handoffs(None) == [] + assert find_handoffs(tmp_path / "nope") == [] + + +def test_digest_surfaces_pending_handoffs(tmp_path: Path, monkeypatch) -> None: + """The digest is what survives compaction — a handoff must ride along.""" + from nightly_core.digest import render_digest + + run = start_run(tmp_path) + _write_handoff(run.path, "0001-alpha", "# Handoff\n\nAdmission tests still to write.\n") + monkeypatch.chdir(tmp_path) + out = render_digest(tmp_path) + assert "Pending handoffs" in out + assert "Admission tests still to write." in out + + +def test_digest_omits_the_section_when_there_are_none(tmp_path: Path, monkeypatch) -> None: + from nightly_core.digest import render_digest + + start_run(tmp_path) + monkeypatch.chdir(tmp_path) + assert "Pending handoffs" not in render_digest(tmp_path) From c9e7347e44b3bcacc945d953d35fdda70ad766fd Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 14:59:53 -0400 Subject: [PATCH 10/27] fix(cascade): skip RFC items already done on an unmerged local branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Observed live this session: the cascade handed the agent "RFC 007 A1" for seven consecutive turn boundaries, long after A1 was implemented, committed, and tested. The `accepted_rfc` ranker's only in-flight guard (`_is_item_in_flight`) inspects **open PR** titles and bodies, so work that is committed locally but not yet a PR is invisible to it — the checkbox on `main` stays unchecked and the item is re-picked forever. The gap is not exotic. It opens whenever a push fails, credentials expire, CI is slow to produce a PR, or the agent simply hasn't pushed yet — i.e. the entire window between finishing work and it becoming reviewable. `_items_done_on_local_branches` reads each `nightly/*` branch's own copy of the RFC and collects the items *that copy* has checked. Asking the branch directly answers "is this item done somewhere?" rather than inferring it from the weaker signal that a branch touched the file, so a branch that edited the RFC for unrelated reasons doesn't mask real work. Bias matches the existing guard's: false negatives over false positives. Any git failure — no repo, detached checkout, branch predating the RFC, timeout — returns an empty set and falls back to today's behavior. The one thing this must never do is skip everything and report `nothing`. Verified against the live repo before writing tests: the guard detects 15 completed items across the local Nightly branches, including the A1 that had been re-picked all night. Only `nightly/*` branches count — someone else's feature branch ticking a box is not Nightly's work. 1266 tests pass (8 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/src/nightly_core/cascade.py | 54 +++++++- .../tests/test_cascade_local_branch_guard.py | 119 ++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 packages/nightly-core/tests/test_cascade_local_branch_guard.py diff --git a/packages/nightly-core/src/nightly_core/cascade.py b/packages/nightly-core/src/nightly_core/cascade.py index 10060a7..dddbe12 100644 --- a/packages/nightly-core/src/nightly_core/cascade.py +++ b/packages/nightly-core/src/nightly_core/cascade.py @@ -284,6 +284,7 @@ def pick_unblocked(root: Path | None = None) -> PlanRecord | None: # captures the item text. Indented checkboxes (nested lists) are intentionally # excluded — only top-level RFC items count as cascade candidates. _RFC_UNCHECKED_RE = re.compile(r"^- \[ \] (.+)$", re.MULTILINE) +_RFC_CHECKED_RE = re.compile(r"^- \[[xX]\] (.+)$", re.MULTILINE) @dataclass(frozen=True) @@ -422,6 +423,56 @@ def _open_nightly_pr_texts( return texts +def _items_done_on_local_branches(rfc_path: Path, root: Path | None) -> set[str]: + """Item texts already ticked on an unmerged local `nightly/*` branch. + + The open-PR guard (`_is_item_in_flight`) only sees work that has been + pushed *and* opened as a PR. Work that is committed locally — because + the push failed, credentials expired, or the agent simply hasn't + pushed yet — is invisible to it, so the cascade re-picks an item that + is already done and hands the agent the same task every turn. That + livelock was observed for seven consecutive turn boundaries before + this guard existed. + + Reads each Nightly branch's own copy of the RFC and collects the + items *it* has checked, which answers "is this item done somewhere?" + directly rather than inferring it from the fact that a branch touched + the file. Returns an empty set on any git failure — a repo without + git, or a detached checkout, must fall back to today's behavior + rather than skipping everything. + """ + repo = (root or repo_root()).resolve() + try: + rel = rfc_path.resolve().relative_to(repo).as_posix() + except ValueError: + return set() + + def _git(*args: str) -> str | None: + try: + proc = subprocess.run( + ["git", *args], + cwd=repo, + capture_output=True, + text=True, + timeout=10, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + return proc.stdout if proc.returncode == 0 else None + + listing = _git("branch", "--list", "nightly/*", "--format=%(refname:short)") + if not listing: + return set() + + done: set[str] = set() + for branch in (ln.strip() for ln in listing.splitlines() if ln.strip()): + blob = _git("show", f"{branch}:{rel}") + if blob: + done.update(m.group(1).strip() for m in _RFC_CHECKED_RE.finditer(blob)) + return done + + def _is_item_in_flight(rfc_filename: str, item_text: str, pr_texts: list[tuple[str, str]]) -> bool: """Is this RFC item likely addressed by an open Nightly PR? @@ -478,9 +529,10 @@ def _find_accepted_rfc(root: Path | None = None) -> _RFCMatch | None: _metadata, body = parse_frontmatter(text) # When there's no frontmatter, `parse_frontmatter` returns the # whole file as `body` already; this path covers both cases. + done_locally = _items_done_on_local_branches(entry, root) for match in _RFC_UNCHECKED_RE.finditer(body): item_text = match.group(1).strip() - if _is_item_in_flight(entry.name, item_text, pr_texts): + if item_text in done_locally or _is_item_in_flight(entry.name, item_text, pr_texts): total_skipped += 1 continue return _RFCMatch( diff --git a/packages/nightly-core/tests/test_cascade_local_branch_guard.py b/packages/nightly-core/tests/test_cascade_local_branch_guard.py new file mode 100644 index 0000000..7136c25 --- /dev/null +++ b/packages/nightly-core/tests/test_cascade_local_branch_guard.py @@ -0,0 +1,119 @@ +"""The cascade must not re-pick RFC items finished on an unmerged branch. + +Regression test for a livelock observed for seven consecutive turn +boundaries: the `accepted_rfc` ranker only knew about work in *open PRs*, +so an item completed and committed on a local branch — because the push +failed, credentials expired, or nobody had pushed yet — stayed unchecked +on `main` and got handed to the agent again every single turn. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from nightly_core.cascade import _items_done_on_local_branches + +RFC_REL = ".planning/rfcs/007-example.md" + +RFC_OPEN = """--- +status: accepted +--- + +# RFC 007 + +- [ ] A1. First item +- [ ] A2. Second item +""" + +RFC_A1_DONE = """--- +status: accepted +--- + +# RFC 007 + +- [x] A1. First item +- [ ] A2. Second item +""" + + +def _git(repo: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + _git(tmp_path, "init", "-q", "-b", "main") + _git(tmp_path, "config", "user.email", "t@example.com") + _git(tmp_path, "config", "user.name", "Test") + _git(tmp_path, "config", "commit.gpgsign", "false") + rfc = tmp_path / RFC_REL + rfc.parent.mkdir(parents=True, exist_ok=True) + rfc.write_text(RFC_OPEN, encoding="utf-8") + _git(tmp_path, "add", "-A") + _git(tmp_path, "commit", "-qm", "seed") + return tmp_path + + +def _branch_with(repo: Path, name: str, content: str) -> None: + _git(repo, "checkout", "-q", "-b", name) + (repo / RFC_REL).write_text(content, encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", f"work on {name}") + _git(repo, "checkout", "-q", "main") + + +def test_nothing_done_when_no_branches_exist(repo: Path) -> None: + assert _items_done_on_local_branches(repo / RFC_REL, repo) == set() + + +def test_item_ticked_on_a_nightly_branch_is_detected(repo: Path) -> None: + """The core regression: committed-but-unpushed work must count.""" + _branch_with(repo, "nightly/rfc007-a1", RFC_A1_DONE) + done = _items_done_on_local_branches(repo / RFC_REL, repo) + assert "A1. First item" in done + assert "A2. Second item" not in done + + +def test_non_nightly_branches_are_ignored(repo: Path) -> None: + """Only Nightly's own branches signal Nightly's own work.""" + _branch_with(repo, "feature/someone-elses-work", RFC_A1_DONE) + assert _items_done_on_local_branches(repo / RFC_REL, repo) == set() + + +def test_items_are_unioned_across_branches(repo: Path) -> None: + rfc_a2_done = RFC_OPEN.replace("- [ ] A2.", "- [x] A2.") + _branch_with(repo, "nightly/one", RFC_A1_DONE) + _branch_with(repo, "nightly/two", rfc_a2_done) + done = _items_done_on_local_branches(repo / RFC_REL, repo) + assert done == {"A1. First item", "A2. Second item"} + + +def test_branch_without_the_rfc_file_is_skipped(repo: Path) -> None: + """A branch cut before the RFC existed must not raise or poison.""" + _git(repo, "checkout", "-q", "-b", "nightly/unrelated") + (repo / RFC_REL).unlink() + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", "remove rfc") + _git(repo, "checkout", "-q", "main") + assert _items_done_on_local_branches(repo / RFC_REL, repo) == set() + + +def test_path_outside_the_repo_yields_nothing(repo: Path, tmp_path: Path) -> None: + outside = tmp_path.parent / "elsewhere.md" + assert _items_done_on_local_branches(outside, repo) == set() + + +def test_non_git_directory_degrades_to_empty(tmp_path: Path) -> None: + """No git, no signal — fall back to today's behavior, don't skip all.""" + rfc = tmp_path / RFC_REL + rfc.parent.mkdir(parents=True, exist_ok=True) + rfc.write_text(RFC_OPEN, encoding="utf-8") + assert _items_done_on_local_branches(rfc, tmp_path) == set() + + +def test_uppercase_checkbox_counts_as_done(repo: Path) -> None: + _branch_with(repo, "nightly/upper", RFC_OPEN.replace("- [ ] A1.", "- [X] A1.")) + assert "A1. First item" in _items_done_on_local_branches(repo / RFC_REL, repo) From 7ac7d2e387e1d1cd49f0129d97f7e5f62967aaea Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:05:10 -0400 Subject: [PATCH 11/27] feat(doctor): warn when Nightly work cannot leave the machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This session completed two RFCs, committed everything, and could not push — a signing agent had auto-locked, and the same key backs the push remote. Nothing in `nightly status` or `nightly doctor` said so. It took eight turns and an accidental `git push` failure to notice. That is the most expensive way an overnight run can fail: silently, while appearing to succeed. From inside the session the work is done; from outside it does not exist. An operator skimming a morning briefing has no reason to suspect anything. `_check_push_readiness` reports three conditions, all from local refs — no network, so it stays fast and works offline: - Nightly branches ahead of their upstream (work committed, not pushed). - Nightly branches with no upstream at all (never pushed anywhere). - Commit signing configured as SSH while the agent holds no identities, which fails the commit and the push together. A `[gone]` upstream is deliberately *not* flagged: that means merged and cleaned up, i.e. local cruft rather than lost work. Flagging it would cry wolf after every finished task, and a check that cries wolf is a check operators learn to skip. Advisory only, never repaired — pushing is the operator's call and unlocking an agent is theirs. There's a test asserting the check mutates no git state. Verified against the live repo, where it renders the current situation in one line: `unpushed: nightly/rfc007-tier-routing [ahead 10]; commit signing configured but the ssh agent holds no identities`. 1277 tests pass (11 new, against a real local bare remote so upstream tracking is genuine rather than mocked); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/src/nightly_core/doctor.py | 106 +++++++++++++ .../tests/test_doctor_push_readiness.py | 148 ++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 packages/nightly-core/tests/test_doctor_push_readiness.py diff --git a/packages/nightly-core/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index dcf3740..a18816f 100644 --- a/packages/nightly-core/src/nightly_core/doctor.py +++ b/packages/nightly-core/src/nightly_core/doctor.py @@ -33,6 +33,7 @@ from __future__ import annotations import asyncio +import subprocess from collections.abc import Callable, Iterable, Mapping from dataclasses import dataclass from pathlib import Path @@ -226,6 +227,110 @@ def _check_model_tiers(root: Path) -> DoctorCheck: ) +def _git_out(root: Path, *args: str) -> str | None: + """Run a read-only git command; None on any failure. Never raises.""" + try: + proc = subprocess.run( + ["git", *args], + cwd=root, + capture_output=True, + text=True, + timeout=10, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + return proc.stdout if proc.returncode == 0 else None + + +def _signing_is_broken(root: Path) -> bool: + """True when commits are configured to be signed but signing will fail. + + Only the SSH-agent path is detectable locally and cheaply: if + `gpg.format` is `ssh` and the agent holds no identities, every commit + will fail. A 1Password or Secretive agent that has auto-locked is the + common cause, and it fails the push too, since the same agent holds + the auth key. + """ + if (_git_out(root, "config", "--get", "commit.gpgsign") or "").strip() != "true": + return False + if (_git_out(root, "config", "--get", "gpg.format") or "").strip() != "ssh": + return False + try: + proc = subprocess.run( + ["ssh-add", "-l"], capture_output=True, text=True, timeout=10, check=False + ) + except (OSError, subprocess.SubprocessError): + return False + return "no identities" in proc.stdout.lower() + + +def _branches_without_upstream(root: Path) -> list[str]: + """Nightly branches that have never been pushed anywhere.""" + listing = _git_out( + root, "for-each-ref", "--format=%(refname:short)|%(upstream)", "refs/heads/nightly/" + ) + if not listing: + return [] + out = [] + for line in (ln for ln in listing.splitlines() if ln.strip()): + branch, _, upstream = line.partition("|") + if not upstream.strip(): + out.append(branch) + return out + + +def _check_push_readiness(root: Path) -> DoctorCheck: + """Can this machine's Nightly work actually reach the remote? + + Advisory, never repaired — pushing is the operator's call, and a + locked signing agent is theirs to unlock. + + This check exists because the failure is silent and expensive: an + overnight run can complete real work, commit it, and be unable to + push, and nothing in `status` or `doctor` said so. The work looks + done from inside the session and is invisible from outside it. An + operator who reads a morning briefing without noticing has lost the + night. + """ + name, desc = "push_readiness", "unpushed Nightly work" + + listing = _git_out( + root, "for-each-ref", "--format=%(refname:short)|%(upstream:track)", "refs/heads/nightly/" + ) + if listing is None: + return DoctorCheck(name=name, description=desc, status="skipped", detail="git unavailable") + + ahead: list[str] = [] + for line in (ln for ln in listing.splitlines() if ln.strip()): + branch, _, track = line.partition("|") + track = track.strip() + # `[gone]` upstreams are merged-and-deleted branches — local + # cruft, not lost work. An empty track means in sync. + if "ahead" in track: + ahead.append(f"{branch} {track}") + + never_pushed = _branches_without_upstream(root) + + signer_broken = _signing_is_broken(root) + problems: list[str] = [] + if ahead: + problems.append(f"unpushed: {', '.join(ahead)}") + if never_pushed: + problems.append(f"never pushed: {', '.join(never_pushed)}") + if signer_broken: + problems.append("commit signing configured but the ssh agent holds no identities") + + if not problems: + return DoctorCheck(name=name, description=desc, status="ok", detail="all branches pushed") + return DoctorCheck( + name=name, + description=desc, + status="warning", + detail="; ".join(problems), + ) + + def _check_worktree_location(root: Path) -> DoctorCheck: """Warn (non-fatally) when the repo sits under iCloud/FileProvider sync. @@ -584,6 +689,7 @@ def diagnose_and_repair( checks.append(_check_nightly_scaffold(root, dry_run=dry_run)) checks.append(_check_config(root, dry_run=dry_run)) checks.append(_check_model_tiers(root)) + checks.append(_check_push_readiness(root)) checks.append(_check_worktree_location(root)) checks.append(_check_rules(root, dry_run=dry_run)) checks.append(_check_synthesis_prompt()) diff --git a/packages/nightly-core/tests/test_doctor_push_readiness.py b/packages/nightly-core/tests/test_doctor_push_readiness.py new file mode 100644 index 0000000..a8804cc --- /dev/null +++ b/packages/nightly-core/tests/test_doctor_push_readiness.py @@ -0,0 +1,148 @@ +"""`nightly doctor` must say when work cannot leave the machine. + +Written after a session that completed two RFCs, committed everything, +and could not push — because a signing agent had auto-locked. Nothing in +`status` or `doctor` said so. The work looked done from inside the +session and was invisible from outside it, which is the most expensive +way an overnight run can fail: silently, while appearing to succeed. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from nightly_core.doctor import _branches_without_upstream, _check_push_readiness + + +def _git(repo: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A repo with an `origin` remote that is itself a local bare repo, so + pushes work offline and upstream tracking is real.""" + origin = tmp_path / "origin.git" + subprocess.run(["git", "init", "-q", "--bare", str(origin)], check=True, capture_output=True) + + work = tmp_path / "work" + work.mkdir() + _git(work, "init", "-q", "-b", "main") + _git(work, "config", "user.email", "t@example.com") + _git(work, "config", "user.name", "Test") + _git(work, "config", "commit.gpgsign", "false") + _git(work, "remote", "add", "origin", str(origin)) + (work / "f.txt").write_text("seed\n", encoding="utf-8") + _git(work, "add", "-A") + _git(work, "commit", "-qm", "seed") + _git(work, "push", "-q", "-u", "origin", "main") + return work + + +def _commit_on(repo: Path, branch: str, *, push: bool) -> None: + _git(repo, "checkout", "-q", "-b", branch) + (repo / f"{branch.replace('/', '-')}.txt").write_text("work\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", f"work on {branch}") + if push: + _git(repo, "push", "-q", "-u", "origin", branch) + _git(repo, "checkout", "-q", "main") + + +def test_clean_repo_reports_ok(repo: Path) -> None: + check = _check_push_readiness(repo) + assert check.status == "ok" + + +def test_pushed_branch_is_not_flagged(repo: Path) -> None: + _commit_on(repo, "nightly/done", push=True) + assert _check_push_readiness(repo).status == "ok" + + +def test_branch_ahead_of_its_upstream_is_flagged(repo: Path) -> None: + """The exact shape of the failure this check was written for.""" + _commit_on(repo, "nightly/wip", push=True) + _git(repo, "checkout", "-q", "nightly/wip") + (repo / "more.txt").write_text("more\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", "unpushed work") + _git(repo, "checkout", "-q", "main") + + check = _check_push_readiness(repo) + assert check.status == "warning" + assert "nightly/wip" in check.detail + assert "ahead 1" in check.detail + + +def test_never_pushed_branch_is_flagged(repo: Path) -> None: + _commit_on(repo, "nightly/fresh", push=False) + check = _check_push_readiness(repo) + assert check.status == "warning" + assert "never pushed" in check.detail + assert "nightly/fresh" in check.detail + + +def test_non_nightly_branches_are_ignored(repo: Path) -> None: + """Doctor speaks for Nightly's work, not the operator's own branches.""" + _commit_on(repo, "feature/mine", push=False) + assert _check_push_readiness(repo).status == "ok" + + +def test_merged_and_deleted_upstream_is_not_lost_work(repo: Path) -> None: + """A `[gone]` upstream means merged-and-cleaned, i.e. local cruft — + flagging it as unpushed work would cry wolf on every finished task.""" + _commit_on(repo, "nightly/merged", push=True) + _git(repo, "push", "-q", "origin", "--delete", "nightly/merged") + _git(repo, "fetch", "-q", "--prune") + assert _check_push_readiness(repo).status == "ok" + + +def test_branches_without_upstream_helper(repo: Path) -> None: + _commit_on(repo, "nightly/a", push=False) + _commit_on(repo, "nightly/b", push=True) + assert _branches_without_upstream(repo) == ["nightly/a"] + + +def test_non_git_directory_is_skipped_not_failed(tmp_path: Path) -> None: + check = _check_push_readiness(tmp_path) + assert check.status == "skipped" + + +def test_broken_signer_is_reported(repo: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A locked ssh agent fails the commit *and* the push — same key.""" + _git(repo, "config", "commit.gpgsign", "true") + _git(repo, "config", "gpg.format", "ssh") + + real_run = subprocess.run + + def fake_run(cmd, *args, **kwargs): + if cmd and cmd[0] == "ssh-add": + return subprocess.CompletedProcess(cmd, 0, "The agent has no identities.\n", "") + return real_run(cmd, *args, **kwargs) + + monkeypatch.setattr(subprocess, "run", fake_run) + check = _check_push_readiness(repo) + assert check.status == "warning" + assert "no identities" in check.detail + + +def test_signer_not_checked_when_signing_is_off(repo: Path) -> None: + """`commit.gpgsign=false` means an empty agent is irrelevant.""" + _git(repo, "config", "commit.gpgsign", "false") + assert _check_push_readiness(repo).status == "ok" + + +def test_check_never_repairs_anything(repo: Path) -> None: + """Pushing is the operator's call; unlocking an agent is theirs too.""" + _commit_on(repo, "nightly/fresh", push=False) + before = subprocess.run( + ["git", "log", "--oneline", "--all"], cwd=repo, capture_output=True, text=True, check=True + ).stdout + _check_push_readiness(repo) + after = subprocess.run( + ["git", "log", "--oneline", "--all"], cwd=repo, capture_output=True, text=True, check=True + ).stdout + assert before == after From 8b3778020d5089e0c2ae18e9f6f68984bc70f934 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:12:38 -0400 Subject: [PATCH 12/27] fix(status): drop the doubled `enabled=`; surface tiers, fleet caps, handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nightly status` rendered `compact: enabled=enabled` — and, when off, the actively misleading `enabled=disabled`. The state word already carries the meaning. An existing test pinned the doubled wording, so it was asserting the bug; updated. More substantially, `status` showed nothing about the three config blocks that now decide how a run behaves. An operator had to open `.nightly/config.yml` to learn which model their own dispatches would use, how wide the fleet would go, or when agents would hand off. `status` is where config gets eyeballed; anything that changes behavior this much belongs in it. It now prints: compact: enabled (threshold cap 256K) tiers: [claude] lite=claude-haiku-4-5 coding=claude-sonnet-5 reasoning=claude-opus-5 via --model fleet: lite=8 coding=6 reasoning=2 (worktrees=8, total=8) handoff: soft=250K hard=500K (of a 1000K window) Details worth noting: - The tier line reports the host **actually running the command** when a harness is detectable, so the models shown are the ones that will apply — not a default that happens to be listed first. - It states `(no model flag discovered)` when init found none, which is the difference between "routing works" and "routing is inert". A config predating the discovery feature says so instead of looking healthy. - Unlimited caps render as `∞` rather than `0`, since `0` reads as "none allowed" in a column of counts. - A broken config still renders — `status` is a diagnostic, and the one time it must work is when something is wrong. There's a test. 1284 tests pass (7 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- packages/nightly-core/src/nightly_core/cli.py | 74 +++++++++++++++++- packages/nightly-core/tests/test_cli.py | 75 ++++++++++++++++++- 2 files changed, 146 insertions(+), 3 deletions(-) diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index 1df3db3..2846fb1 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -113,6 +113,73 @@ _DEFAULT_CONFIG_YML = DEFAULT_CONFIG_YML +def _echo_routing_status(root: Path) -> None: + """Print the model-tier and fleet-cap lines of `nightly status`. + + These blocks decide which model every dispatch runs on and how wide + the fleet goes, and until now `status` showed neither — an operator + had to open `.nightly/config.yml` to learn what their own run would + do. `status` is where config gets eyeballed; anything that changes + behavior this much belongs in it. + """ + from nightly_core.config import ( # noqa: PLC0415 - lazy + load_model_tier_config, + load_parallelism_config, + ) + from nightly_core.contract import MODEL_TIERS # noqa: PLC0415 - lazy + from nightly_core.routing import resolve_context_thresholds # noqa: PLC0415 - lazy + + tier_cfg = load_model_tier_config(root) + if not tier_cfg.enabled: + typer.echo(" tiers: disabled (every dispatch uses the host default model)") + else: + host = _primary_host(root) + cells = [] + for tier in MODEL_TIERS: + binding = tier_cfg.binding(host, tier) + cells.append(f"{tier}={binding.model or ''}") + flag = tier_cfg.flag_for(host) + suffix = f" via {flag}" if flag else " (no model flag discovered)" + typer.echo(f" tiers: [{host}] {' '.join(cells)}{suffix}") + + par = load_parallelism_config(root) + caps = " ".join(f"{tier}={par.limit_for(tier) or '∞'}" for tier in MODEL_TIERS) + typer.echo( + f" fleet: {caps} (worktrees={par.max_worktrees or '∞'}, " + f"total={par.max_concurrent_specialists or '∞'})" + ) + + from nightly_core.config import load_context_config # noqa: PLC0415 - lazy + + ctx = load_context_config(root) + model = tier_cfg.binding(_primary_host(root), "reasoning").model if tier_cfg.enabled else None + th = resolve_context_thresholds(model, ctx) + if th.soft_tokens or th.hard_tokens: + typer.echo( + f" handoff: soft={round(th.soft_tokens / 1000)}K " + f"hard={round(th.hard_tokens / 1000)}K " + f"(of a {round(th.window_tokens / 1000)}K window)" + ) + + +def _primary_host(root: Path) -> HostId: + """The host `status` reports tier bindings for. + + Prefers the harness actually running the command, so the numbers + shown are the ones that will apply; falls back to the first + configured host, then `claude`. + """ + from nightly_core.model_probe import detect_harness # noqa: PLC0415 - lazy + + harness = detect_harness() + if harness is not None: + return harness + from nightly_core.doctor import _configured_hosts # noqa: PLC0415 - lazy + + hosts = _configured_hosts(root) + return hosts[0] if hosts else "claude" + + def _render_discovered_config() -> str: """Render config.yml with model controls probed from the live harness. @@ -465,10 +532,15 @@ def status() -> None: compact_cfg = load_compact_config(root) compact_state = "enabled" if compact_cfg.enabled else "disabled" + # `enabled={state}` rendered as "enabled=enabled" — and, worse, + # "enabled=disabled". The state word already carries the meaning. typer.echo( - f" compact: enabled={compact_state} (threshold cap {round(compact_cfg.context_token_cap / 1000)}K)" + f" compact: {compact_state} " + f"(threshold cap {round(compact_cfg.context_token_cap / 1000)}K)" ) + _echo_routing_status(root) + typer.echo(" runs:") run = current_run(root) if run is None: diff --git a/packages/nightly-core/tests/test_cli.py b/packages/nightly-core/tests/test_cli.py index 7ee4617..1582989 100644 --- a/packages/nightly-core/tests/test_cli.py +++ b/packages/nightly-core/tests/test_cli.py @@ -673,7 +673,7 @@ def test_status_shows_compact_line(repo: Path) -> None: # 1. Default (enabled, 256K) result = runner.invoke(app, ["status"]) assert result.exit_code == 0 - assert "compact: enabled=enabled (threshold cap 256K)" in result.output + assert "compact: enabled (threshold cap 256K)" in result.output # 2. Disabled or custom cap cfg_file = repo / ".nightly" / "config.yml" @@ -682,7 +682,7 @@ def test_status_shows_compact_line(repo: Path) -> None: ) result2 = runner.invoke(app, ["status"]) assert result2.exit_code == 0 - assert "compact: enabled=disabled (threshold cap 128K)" in result2.output + assert "compact: disabled (threshold cap 128K)" in result2.output # ── Phase 3 commands ────────────────────────────────────────────────────── @@ -1371,3 +1371,74 @@ def test_worktree_create_dry_run_honors_config_worktree_root(repo: Path, tmp_pat assert result.exit_code == 0, result.output assert str(custom_root) in result.output assert f"worktree_root={custom_root}" in result.output + + +# ── status: routing + fleet surfaces ────────────────────────────────────── + + +def _init_repo(tmp_path: Path, runner_: CliRunner, monkeypatch) -> None: + monkeypatch.chdir(tmp_path) + runner_.invoke(app, ["init", "--host", "claude"]) + + +def test_status_compact_line_is_not_doubled(tmp_path: Path, monkeypatch) -> None: + """It used to read `enabled=enabled` — and `enabled=disabled`.""" + _init_repo(tmp_path, runner, monkeypatch) + out = runner.invoke(app, ["status"]).output + assert "enabled=enabled" not in out + assert "enabled=disabled" not in out + assert "compact:" in out + + +def test_status_shows_the_tier_bindings(tmp_path: Path, monkeypatch) -> None: + """An operator shouldn't have to open config.yml to learn which model + their own run will use.""" + _init_repo(tmp_path, runner, monkeypatch) + out = runner.invoke(app, ["status"]).output + assert "tiers:" in out + for tier in ("lite", "coding", "reasoning"): + assert tier in out + + +def test_status_shows_fleet_caps(tmp_path: Path, monkeypatch) -> None: + _init_repo(tmp_path, runner, monkeypatch) + out = runner.invoke(app, ["status"]).output + assert "fleet:" in out + assert "worktrees=" in out + + +def test_status_shows_handoff_thresholds(tmp_path: Path, monkeypatch) -> None: + _init_repo(tmp_path, runner, monkeypatch) + out = runner.invoke(app, ["status"]).output + assert "handoff:" in out + assert "soft=" in out + assert "hard=" in out + + +def test_status_reports_disabled_tier_routing(tmp_path: Path, monkeypatch) -> None: + _init_repo(tmp_path, runner, monkeypatch) + cfg = tmp_path / ".nightly" / "config.yml" + cfg.write_text("model_tiers:\n enabled: false\n", encoding="utf-8") + out = runner.invoke(app, ["status"]).output + assert "tiers: disabled" in out + + +def test_status_marks_unlimited_caps_legibly(tmp_path: Path, monkeypatch) -> None: + _init_repo(tmp_path, runner, monkeypatch) + cfg = tmp_path / ".nightly" / "config.yml" + cfg.write_text( + "parallelism:\n max_concurrent_specialists: 0\n max_worktrees: 0\n" + " per_tier:\n lite: 0\n coding: 0\n reasoning: 0\n", + encoding="utf-8", + ) + out = runner.invoke(app, ["status"]).output + assert "∞" in out + + +def test_status_survives_a_broken_config(tmp_path: Path, monkeypatch) -> None: + """`status` is a diagnostic — it must still render when config is bad.""" + _init_repo(tmp_path, runner, monkeypatch) + (tmp_path / ".nightly" / "config.yml").write_text("model_tiers: [oops\n", encoding="utf-8") + result = runner.invoke(app, ["status"]) + assert result.exit_code == 0 + assert "tiers:" in result.output From 3e9765ccc446ad3c2ca572ffda4ec401856bac48 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:17:55 -0400 Subject: [PATCH 13/27] fix(dispatch): derive host-support errors instead of restating them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems, one cause. The failure message for an undispatchable host hardcoded a prose list of which hosts were supported — a restatement of what `build_argv` actually implements. The moment `pi` and `hermes` joined `HostId`, the two drifted: asking to dispatch to `pi` produced a message enumerating every host *except* `pi`. It also conflated two failures that need opposite fixes from the operator: - **No headless CLI** (cursor, antigravity, pi, hermes). Nothing they install changes this; use the host's own primitive or pick another. - **Binary not on PATH** (a supported host that isn't installed here). A five-second fix the old message never mentioned — asking for `claude` on a box without it produced the same wall of text as asking for `cursor`. `HEADLESS_HOSTS` is now the single source of truth and `unsupported_host_message` computes the explanation from it plus what is actually on PATH. A test asserts the tuple isn't a lie: with every binary stubbed present, `build_argv` returns argv for exactly the hosts in `HEADLESS_HOSTS` and None for every other member of `HostId`. That makes this drift class impossible rather than merely fixed. Caught while writing it: the suggestion list included the failing host itself — "your `codex` binary is missing, try codex". Recommending the binary that just failed to resolve is worse than saying nothing, so the failing host is excluded from both suggestion paths. One existing test matched on the old message text and was updated. 1291 tests pass (7 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/src/nightly_core/dispatch.py | 68 ++++++++++++++--- packages/nightly-core/tests/test_dispatch.py | 75 ++++++++++++++++++- 2 files changed, 130 insertions(+), 13 deletions(-) diff --git a/packages/nightly-core/src/nightly_core/dispatch.py b/packages/nightly-core/src/nightly_core/dispatch.py index 982e80f..57b98b5 100644 --- a/packages/nightly-core/src/nightly_core/dispatch.py +++ b/packages/nightly-core/src/nightly_core/dispatch.py @@ -56,6 +56,7 @@ __all__ = [ "DEFAULT_LOG_FILENAME", "DEFAULT_STATE_FILENAME", + "HEADLESS_HOSTS", "BackgroundDispatchResult", "DispatchStatus", "active_dispatches", @@ -66,6 +67,7 @@ "read_dispatch_state", "start_background", "tier_utilization", + "unsupported_host_message", "wait_for", "write_dispatch_state", ] @@ -76,6 +78,16 @@ DispatchStatus = Literal["running", "completed", "failed", "unknown"] +HEADLESS_HOSTS: tuple[HostId, ...] = ("claude", "codex", "opencode", "gemini") +"""Hosts `build_argv` knows how to spawn headlessly. + +The single source of truth for "can this host be backgrounded". It used +to be restated in prose inside the failure message, and the two drifted +the moment `pi` and `hermes` joined `HostId` — the message enumerated +every host *except* the one the operator had asked about. Deriving the +message from this tuple makes that class of drift impossible rather than +merely fixed.""" + # Module-local factory alias for the default Popen. Tests can monkeypatch # THIS without affecting global `subprocess.Popen` (which click + typer # use internally for I/O capture — patching the global one breaks @@ -197,13 +209,52 @@ def build_argv( # noqa: PLR0911, PLR0912 - one branch per host backend is the w argv += [model_flag, model] return argv - # cursor + antigravity don't expose a usable headless CLI today. - # Callers can fall back to the host's blocking primitive (Background - # Agent / Agent Manager registration) or to claude/codex if those - # binaries are also on PATH. + # Every other host — cursor, antigravity, pi, hermes — exposes no + # usable headless CLI today. Callers fall back to the host's own + # blocking primitive, or to a host from `HEADLESS_HOSTS` whose binary + # is on PATH. See `unsupported_host_message` for the operator-facing + # explanation. return None +def unsupported_host_message(host: HostId) -> str: + """Explain why `host` cannot be dispatched to, and what to do instead. + + Distinguishes the two failures the old message conflated: + + - **No headless CLI.** The host has no non-interactive entry point at + all. Nothing the operator installs will change that; the fix is to + use the host's own primitive or pick another host. + - **Binary not on PATH.** The host is supported; its CLI just isn't + installed or isn't visible from here. That is a five-second fix, + and the old message never said so — asking for `claude` on a box + without it produced the same wall of text as asking for `cursor`. + """ + if host not in HEADLESS_HOSTS: + supported = ", ".join(HEADLESS_HOSTS) + return ( + f"host '{host}' has no headless CLI, so it cannot run a background " + f"dispatch. Use its own sub-agent primitive instead, or dispatch to " + f"one of: {supported}." + ) + + # Exclude the failing host: recommending the binary that just failed + # to resolve is worse than saying nothing. + available = [h for h in HEADLESS_HOSTS if h != host and shutil.which(h)] + if available: + return ( + f"host '{host}' supports background dispatch but its `{host}` binary " + f"is not on PATH. Install it, or use --host with one of the " + f"binaries you do have: {', '.join(available)}." + ) + others = ", ".join(h for h in HEADLESS_HOSTS if h != host) + return ( + f"host '{host}' supports background dispatch but its `{host}` binary is " + f"not on PATH — and no other dispatchable host's binary is either. " + f"Install `{host}`, or one of: {others}." + ) + + # ── spawn ──────────────────────────────────────────────────────────────── @@ -234,14 +285,7 @@ def start_background( # noqa: PLR0913 - dispatch primitive needs every dimensio """ argv = build_argv(host, prompt, session_id=session_id, model=model, model_flag=model_flag) if argv is None: - msg = ( - f"no background dispatch backend for host '{host}'. " - "claude/codex/opencode/gemini are supported when their binaries " - "are on PATH; cursor/antigravity have no headless CLI today — " - "use the host's native primitive (Background Agent / Agent " - "Manager) for those." - ) - raise RuntimeError(msg) + raise RuntimeError(unsupported_host_message(host)) repo = (root or repo_root()).resolve() work_cwd = (cwd or repo).resolve() diff --git a/packages/nightly-core/tests/test_dispatch.py b/packages/nightly-core/tests/test_dispatch.py index b83bd16..bc08d31 100644 --- a/packages/nightly-core/tests/test_dispatch.py +++ b/packages/nightly-core/tests/test_dispatch.py @@ -6,13 +6,16 @@ import subprocess from datetime import UTC, datetime from pathlib import Path +from typing import get_args import pytest from typer.testing import CliRunner from nightly_core import dispatch from nightly_core.cli import app +from nightly_core.contract import HostId from nightly_core.dispatch import ( + HEADLESS_HOSTS, BackgroundDispatchResult, build_argv, is_alive, @@ -21,6 +24,7 @@ refresh, start_background, supported_hosts, + unsupported_host_message, write_dispatch_state, ) from nightly_core.runs import new_task, start_run @@ -180,7 +184,7 @@ def test_start_background_raises_for_unsupported_host( repo, slug = repo_with_task # cursor has no headless backend; build_argv returns None. monkeypatch.setattr(dispatch.shutil, "which", lambda _: "/anywhere") - with pytest.raises(RuntimeError, match="no background dispatch backend"): + with pytest.raises(RuntimeError, match="no headless CLI"): start_background( slug, role="implementer", @@ -522,3 +526,72 @@ def test_dispatch_state_file_lives_under_task_dir( payload = json.loads((task_dir / "dispatch.json").read_text(encoding="utf-8")) assert payload["slug"] == slug assert payload["status"] == "running" + + +# ── host-support errors (derived, not restated) ─────────────────────────── + + +def test_headless_hosts_matches_what_build_argv_actually_supports(monkeypatch) -> None: + """The tuple is the single source of truth — prove it isn't a lie. + + A host in `HEADLESS_HOSTS` must produce argv when its binary exists; + a host outside it must not, whatever is on PATH. + """ + from nightly_core import dispatch as d + + monkeypatch.setattr(d.shutil, "which", lambda name: f"/usr/local/bin/{name}") + for host in get_args(HostId): + argv = d.build_argv(host, "prompt") + assert (argv is not None) == (host in d.HEADLESS_HOSTS), host + + +def test_no_cli_host_message_names_the_host_asked_for() -> None: + """The old message enumerated every host except the one requested.""" + msg = unsupported_host_message("pi") + assert "'pi'" in msg + assert "no headless CLI" in msg + + +def test_no_cli_host_message_lists_the_alternatives() -> None: + msg = unsupported_host_message("cursor") + for host in HEADLESS_HOSTS: + assert host in msg + + +def test_missing_binary_is_distinguished_from_no_cli(monkeypatch) -> None: + """Two different failures needing two different fixes; the old message + gave the same wall of text for both.""" + from nightly_core import dispatch as d + + monkeypatch.setattr(d.shutil, "which", lambda _name: None) + supported = unsupported_host_message("claude") + unsupported = unsupported_host_message("cursor") + assert "not on PATH" in supported + assert "not on PATH" not in unsupported + assert "no headless CLI" in unsupported + + +def test_suggestions_never_include_the_failing_host(monkeypatch) -> None: + """Recommending the binary that just failed is worse than silence.""" + from nightly_core import dispatch as d + + monkeypatch.setattr(d.shutil, "which", lambda name: None if name == "codex" else f"/bin/{name}") + msg = unsupported_host_message("codex") + suggestions = msg.split("binaries you do have:")[1] + assert "codex" not in suggestions + + +def test_message_when_nothing_dispatchable_is_installed(monkeypatch) -> None: + from nightly_core import dispatch as d + + monkeypatch.setattr(d.shutil, "which", lambda _name: None) + msg = unsupported_host_message("claude") + assert "no other dispatchable host" in msg + + +def test_start_background_raises_the_derived_message(monkeypatch, tmp_path: Path) -> None: + from nightly_core import dispatch as d + + monkeypatch.setattr(d.shutil, "which", lambda _name: None) + with pytest.raises(RuntimeError, match="no headless CLI"): + d.start_background("slug", role="implementer", host="pi", prompt="p", root=tmp_path) From 6960dad17695c97f485188527e46698820c1fb25 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:23:08 -0400 Subject: [PATCH 14/27] feat(verify): configurable per-check timeout, actionable timeout message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nightly verify` gates every PR (rules block, rule 7), so a check that times out blocks work that was actually fine — and re-running changes nothing, which makes it the most frustrating failure the tool can produce. It happened in this session: a change temporarily pushed the test suite past the hardcoded 300s and `make-check` failed with timed out after 300.0s and nothing else. No hint that the limit was adjustable, no mention of the `--timeout` flag that already existed. A message that names no remedy sends the reader to the source. Two changes: - **A `verify:` config block** with `timeout_seconds` (default 300). The `--timeout` flag existed but had to be passed on every invocation and could not reach the verify that `nightly run` performs internally. A project whose test target legitimately runs long now sets it once. - **The timeout message names both remedies** and distinguishes the cases: "if this check is simply slow rather than hung, raise `verify.timeout_seconds` ... or pass `--timeout`". Slow and hung want opposite responses, and only the operator knows which they have. A non-positive or non-numeric `timeout_seconds` falls back to the default rather than disabling the cap — an accidental `0` should not mean "let a hung check block the run forever". 1298 tests pass (7 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- packages/nightly-core/src/nightly_core/cli.py | 11 ++- .../nightly-core/src/nightly_core/config.py | 53 ++++++++++++++ .../nightly-core/src/nightly_core/verify.py | 6 +- packages/nightly-core/tests/test_verify.py | 70 +++++++++++++++++++ 4 files changed, 136 insertions(+), 4 deletions(-) diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index 2846fb1..78d446f 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -2239,10 +2239,13 @@ def verify_cmd( ), ] = False, timeout: Annotated[ - float, + float | None, typer.Option( "--timeout", - help="Per-check timeout in seconds. Default: 300.", + help=( + "Per-check timeout in seconds. Default: `verify.timeout_seconds` " + "from .nightly/config.yml (300 if unset)." + ), ), ] = 300.0, ) -> None: @@ -2257,12 +2260,14 @@ def verify_cmd( Exits non-zero on any failed check or missing configured tool so the agent can branch on `$?` from the prompt. """ + from nightly_core.config import load_verify_config # noqa: PLC0415 - lazy + root = repo_root() report = run_verify( root, dry_run=dry_run, only=only, - timeout_s=timeout, + timeout_s=timeout if timeout is not None else load_verify_config(root).timeout_seconds, ) _print_verify_report(report, root=root) if report.failed or report.not_found: diff --git a/packages/nightly-core/src/nightly_core/config.py b/packages/nightly-core/src/nightly_core/config.py index 14af55d..86d3426 100644 --- a/packages/nightly-core/src/nightly_core/config.py +++ b/packages/nightly-core/src/nightly_core/config.py @@ -35,6 +35,7 @@ "ParallelismConfig", "TierBinding", "VaultConfig", + "VerifyConfig", "WorktreeConfig", "load_agents_config", "load_compact_config", @@ -43,6 +44,7 @@ "load_model_tier_config", "load_parallelism_config", "load_vault_config", + "load_verify_config", "load_worktree_config", "render_config_yml", ] @@ -754,6 +756,49 @@ def _coerce_ratio(key: str, default: float) -> float: ) +@dataclass(frozen=True) +class VerifyConfig: + """The `verify:` block of `.nightly/config.yml`.""" + + timeout_seconds: int = 300 + """Per-check wall-clock cap for `nightly verify`. + + 300s suits a fast suite and is wrong for a slow one. Because + `nightly verify` gates every PR (rules block, rule 7), a check that + times out blocks work that was actually fine — the most annoying + possible failure, since re-running changes nothing. Projects whose + test target legitimately runs longer raise this once here rather than + passing `--timeout` on every invocation and remembering to thread it + through `nightly run`.""" + + +def load_verify_config(root: Path | None = None) -> VerifyConfig: + """Parse the `verify:` block from `/.nightly/config.yml`. + + A non-positive or non-numeric value degrades to the default rather + than disabling the cap — an accidental `0` should not mean "let a + hung check block the run forever".""" + defaults = VerifyConfig() + path = nightly_dir(root) / "config.yml" + try: + raw = path.read_text(encoding="utf-8") + except OSError: + return defaults + try: + data: Any = yaml.safe_load(raw) + except yaml.YAMLError as exc: + _log.warning("ignoring malformed %s: %s", path, exc) + return defaults + block = data.get("verify") if isinstance(data, dict) else None + if not isinstance(block, dict): + return defaults + try: + seconds = int(block.get("timeout_seconds", defaults.timeout_seconds)) + except (TypeError, ValueError): + return defaults + return VerifyConfig(timeout_seconds=seconds if seconds > 0 else defaults.timeout_seconds) + + @dataclass(frozen=True) class CompactConfig: """Configuration for session compaction (RFC 006).""" @@ -968,6 +1013,14 @@ def _coerce_int(key: str, default: int) -> int: # model_context_tokens: # : 256000 +# verify governs `nightly verify`, the gate every PR must pass. +# - `timeout_seconds` caps each individual check. Raise it when the +# repo's own test target legitimately runs longer than the default — +# a timeout here blocks work that was actually fine, and re-running +# changes nothing. `--timeout` overrides per invocation. +verify: + timeout_seconds: 300 + # compact governs the session compaction triggers (RFC 006). # - `enabled` flips both triggers (boundary and threshold) on or off. # - `context_token_cap` is the threshold (in tokens) at which the mid-loop diff --git a/packages/nightly-core/src/nightly_core/verify.py b/packages/nightly-core/src/nightly_core/verify.py index 493d33c..f6730dc 100644 --- a/packages/nightly-core/src/nightly_core/verify.py +++ b/packages/nightly-core/src/nightly_core/verify.py @@ -331,7 +331,11 @@ def _run_one(check: VerifyCheck, *, cwd: Path, timeout: float) -> VerifyCheck: description=check.description, command=check.command, status="failed", - output=f"timed out after {exc.timeout}s", + output=( + f"timed out after {exc.timeout}s — if this check is simply slow " + "rather than hung, raise `verify.timeout_seconds` in " + "`.nightly/config.yml` or pass `--timeout`" + ), exit_code=-1, ) except OSError as exc: diff --git a/packages/nightly-core/tests/test_verify.py b/packages/nightly-core/tests/test_verify.py index fef4b29..2e628e8 100644 --- a/packages/nightly-core/tests/test_verify.py +++ b/packages/nightly-core/tests/test_verify.py @@ -180,3 +180,73 @@ def fake_run(*_a, **kw): report = run_verify(tmp_path) assert not report.ok assert all("timed out" in c.output for c in report.failed) + + +# ── configurable timeout (verify: block) ────────────────────────────────── + + +def _write_cfg(root: Path, body: str) -> None: + (root / ".nightly").mkdir(parents=True, exist_ok=True) + (root / ".nightly" / "config.yml").write_text(body, encoding="utf-8") + + +def test_verify_timeout_defaults_to_300(tmp_path: Path) -> None: + from nightly_core.config import load_verify_config + + assert load_verify_config(tmp_path).timeout_seconds == 300 + + +def test_verify_timeout_is_configurable(tmp_path: Path) -> None: + """A repo whose test target legitimately runs long shouldn't have to + pass --timeout on every invocation.""" + from nightly_core.config import load_verify_config + + _write_cfg(tmp_path, "verify:\n timeout_seconds: 1800\n") + assert load_verify_config(tmp_path).timeout_seconds == 1800 + + +@pytest.mark.parametrize("bad", ["0", "-5", "banana"]) +def test_nonsense_timeout_falls_back_to_the_default(tmp_path: Path, bad: str) -> None: + """`0` must not mean "let a hung check block the run forever".""" + from nightly_core.config import load_verify_config + + _write_cfg(tmp_path, f"verify:\n timeout_seconds: {bad}\n") + assert load_verify_config(tmp_path).timeout_seconds == 300 + + +def test_malformed_config_falls_back(tmp_path: Path) -> None: + from nightly_core.config import load_verify_config + + _write_cfg(tmp_path, "verify: [oops\n") + assert load_verify_config(tmp_path).timeout_seconds == 300 + + +def test_timeout_message_names_the_knob(tmp_path: Path) -> None: + """The old message was `timed out after 300.0s` and nothing else — + no hint that the limit was adjustable at all.""" + import subprocess + + from nightly_core.verify import VerifyCheck, _run_one + + check = VerifyCheck( + name="slow", + description="a slow but healthy check", + command=("sleep", "5"), + status="skipped", + ) + + def boom(*_args, **kwargs): + raise subprocess.TimeoutExpired(cmd="sleep", timeout=kwargs.get("timeout", 1)) + + import nightly_core.verify as verify_mod + + original = verify_mod.subprocess.run + verify_mod.subprocess.run = boom # type: ignore[assignment] + try: + result = _run_one(check, cwd=tmp_path, timeout=1) + finally: + verify_mod.subprocess.run = original # type: ignore[assignment] + + assert "timed out" in result.output + assert "verify.timeout_seconds" in result.output + assert "--timeout" in result.output From dac293e9e0e6d0141a3d047655d2e78f05ffb8ae Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:28:01 -0400 Subject: [PATCH 15/27] feat(doctor): warn when a tier is bound to a different band's model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A swapped or typo'd binding — `lite: claude-opus-5`, or a reasoning tier left on a lite model — is silent and expensive. Routing keeps working, dispatches keep succeeding, and the only symptom is the bill, or in the reverse direction a reviewer that quietly misses bugs. Nothing else in the system ever complains. `_check_tier_sanity` compares each configured binding's model family against the tier it is bound to and warns on disagreement. **What this deliberately is not:** a check that the model id exists. RFC 007's Risks section proposed exactly that, and it is unbuildable from local signal — I tested before writing it. The vocabulary a host CLI advertises in `--help` is a sample, not an enumeration: `claude --help` names four tokens and *none* of the three production ids Nightly ships as defaults appear among them. A membership test would flag correct configuration as broken, which is worse than no check. Validating for real needs a network call to a vendor models endpoint, and `doctor` must work offline. So: family matching only, and an unrecognized family is skipped rather than guessed at. `tier_of_model` returns None for ids it doesn't recognize — the honest answer — and callers treat None as "no opinion", never as "wrong". A test pins that the shipped defaults don't trip the check, since a check that fires on correct config trains operators to ignore it. RFC 007's Risks section is updated with why the original mitigation was abandoned and what replaced it; the deprecation case remains covered only by "dispatch raises, briefing surfaces it". 1303 tests pass (5 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 23 ++++++-- .../nightly-core/src/nightly_core/doctor.py | 41 ++++++++++++++ .../src/nightly_core/model_probe.py | 12 +++- packages/nightly-core/tests/test_routing.py | 55 +++++++++++++++++++ 4 files changed, 124 insertions(+), 7 deletions(-) diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index a875d66..e00ba96 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -383,10 +383,25 @@ so skill install and keep-alive hooks are unavailable for them. - **Stale config after a model deprecation.** If Anthropic deprecates Haiku 4.5 in favor of Haiku 5.0, configs still - pointing at the old id will fail at dispatch time. Mitigation: - `nightly doctor` gains a future check that pings each - configured model id; the immediate failure mode is "dispatch - raises" which surfaces in the briefing. + pointing at the old id will fail at dispatch time. + + *2026-07-28: the proposed mitigation — a doctor check validating + each configured id — turns out to be unbuildable from local + signal.* The vocabulary a host CLI advertises in `--help` is a + sample, not an enumeration: `claude --help` names four tokens + (`fable`, `opus`, `sonnet`, `claude-fable-5`) and **none** of the + three production ids Nightly ships as defaults appear in it. A + membership test would flag correct configuration as broken, which + is worse than no check at all. Validating for real needs a network + call to the vendor's models endpoint — out of scope for `doctor`, + which must work offline. + + What shipped instead is `_check_tier_sanity`: a *family* + consistency check that catches the misconfiguration local signal + can actually see — a tier bound to a model from a different band + (`lite: claude-opus-5`). Unrecognized families are skipped rather + than guessed at. The deprecation case remains covered only by + "dispatch raises", which surfaces in the briefing. - **Host-side rate limits / billing caps.** Switching to lite tier for the bulk of doc work could trip the host's rate limit if diff --git a/packages/nightly-core/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index a18816f..be5c302 100644 --- a/packages/nightly-core/src/nightly_core/doctor.py +++ b/packages/nightly-core/src/nightly_core/doctor.py @@ -280,6 +280,46 @@ def _branches_without_upstream(root: Path) -> list[str]: return out +def _check_tier_sanity(root: Path) -> DoctorCheck: + """Warn when a tier is bound to a model from a different band. + + A swapped or typo'd binding — `lite: claude-opus-5`, or a reasoning + tier left on a lite model — is silent and expensive. Routing keeps + working, dispatches keep succeeding, and the only symptom is the + bill (or, in the reverse direction, a reviewer that misses bugs). + Nothing else in the system will ever complain. + + Deliberately *not* a check that the model id exists. The vocabulary a + host CLI advertises in `--help` is a sample, not an enumeration — + `claude --help` names four tokens while every production id Nightly + ships as a default is absent from that list. A membership test would + flag correct configuration as broken, which is worse than no check. + Family matching only, and an unrecognized family is skipped rather + than guessed at. + """ + from nightly_core.config import load_model_tier_config # noqa: PLC0415 + from nightly_core.model_probe import tier_of_model # noqa: PLC0415 + + name, desc = "model_tier_sanity", "tier/model agreement" + cfg = load_model_tier_config(root) + if not cfg.enabled: + return DoctorCheck(name=name, description=desc, status="skipped", detail="routing disabled") + + mismatches: list[str] = [] + for host in sorted(_configured_hosts(root)): + for tier in MODEL_TIERS: + model = cfg.binding(host, tier).model + if not model: + continue + actual = tier_of_model(model) + if actual is not None and actual != tier: + mismatches.append(f"{host}.{tier}={model} looks like a {actual}-tier model") + + if not mismatches: + return DoctorCheck(name=name, description=desc, status="ok", detail="tiers look consistent") + return DoctorCheck(name=name, description=desc, status="warning", detail="; ".join(mismatches)) + + def _check_push_readiness(root: Path) -> DoctorCheck: """Can this machine's Nightly work actually reach the remote? @@ -689,6 +729,7 @@ def diagnose_and_repair( checks.append(_check_nightly_scaffold(root, dry_run=dry_run)) checks.append(_check_config(root, dry_run=dry_run)) checks.append(_check_model_tiers(root)) + checks.append(_check_tier_sanity(root)) checks.append(_check_push_readiness(root)) checks.append(_check_worktree_location(root)) checks.append(_check_rules(root, dry_run=dry_run)) diff --git a/packages/nightly-core/src/nightly_core/model_probe.py b/packages/nightly-core/src/nightly_core/model_probe.py index 7936ab1..87bf32b 100644 --- a/packages/nightly-core/src/nightly_core/model_probe.py +++ b/packages/nightly-core/src/nightly_core/model_probe.py @@ -41,6 +41,7 @@ "merge_discovered_tiers", "probe_all", "probe_model_control", + "tier_of_model", ] @@ -298,8 +299,13 @@ def _models_from_help(text: str, option_start: int) -> list[str]: return [tok for tok in _QUOTED_TOKEN.findall(window) if tok.lower() not in _NOT_A_MODEL] -def _tier_of(model: str) -> ModelTier | None: - """Which tier a model id belongs to, by family substring.""" +def tier_of_model(model: str) -> ModelTier | None: + """Which tier a model id belongs to, by family substring. + + None for an id whose family isn't recognized — the honest answer, and + the one that keeps callers from guessing. Consumers treat None as + "no opinion" rather than "wrong". + """ lowered = model.lower() for tier in MODEL_TIERS: if any(family in lowered for family in TIER_FAMILIES[tier]): @@ -326,7 +332,7 @@ def assign_tiers(models: Sequence[str]) -> dict[ModelTier, str]: """ best: dict[ModelTier, tuple[int, int, str]] = {} for model in models: - tier = _tier_of(model) + tier = tier_of_model(model) if tier is None: continue lowered = model.lower() diff --git a/packages/nightly-core/tests/test_routing.py b/packages/nightly-core/tests/test_routing.py index a070a74..14c02f2 100644 --- a/packages/nightly-core/tests/test_routing.py +++ b/packages/nightly-core/tests/test_routing.py @@ -350,3 +350,58 @@ def test_doctor_never_repairs_model_tiers(tmp_path: Path) -> None: before = (tmp_path / ".nightly" / "config.yml").read_text(encoding="utf-8") _check_model_tiers(tmp_path) assert (tmp_path / ".nightly" / "config.yml").read_text(encoding="utf-8") == before + + +# ── tier/model agreement (doctor) ───────────────────────────────────────── + + +def test_default_config_is_internally_consistent(tmp_path: Path) -> None: + """The shipped defaults must not themselves trip the check.""" + from nightly_core.doctor import _check_tier_sanity + + _write_config(tmp_path, "hosts:\n - claude\n") + assert _check_tier_sanity(tmp_path).status == "ok" + + +def test_swapped_tiers_are_caught(tmp_path: Path) -> None: + """The expensive silent misconfiguration: routing keeps working and + only the bill notices.""" + from nightly_core.doctor import _check_tier_sanity + + _write_config( + tmp_path, + "hosts:\n - claude\nmodel_tiers:\n claude:\n" + " lite: claude-opus-5\n reasoning: claude-haiku-4-5\n", + ) + check = _check_tier_sanity(tmp_path) + assert check.status == "warning" + assert "lite=claude-opus-5" in check.detail + assert "reasoning-tier model" in check.detail + + +def test_unrecognized_family_is_skipped_not_guessed(tmp_path: Path) -> None: + """A membership test against advertised ids would flag correct config + as broken — `claude --help` names four tokens and none of them are + the production ids Nightly ships. Family matching only.""" + from nightly_core.doctor import _check_tier_sanity + + _write_config( + tmp_path, + "hosts:\n - claude\nmodel_tiers:\n claude:\n coding: some-vendor-model-x\n", + ) + assert _check_tier_sanity(tmp_path).status == "ok" + + +def test_check_is_skipped_when_routing_is_disabled(tmp_path: Path) -> None: + from nightly_core.doctor import _check_tier_sanity + + _write_config(tmp_path, "hosts:\n - claude\nmodel_tiers:\n enabled: false\n") + assert _check_tier_sanity(tmp_path).status == "skipped" + + +def test_tier_of_model_has_no_opinion_on_unknown_ids() -> None: + from nightly_core.model_probe import tier_of_model + + assert tier_of_model("claude-opus-5") == "reasoning" + assert tier_of_model("claude-haiku-4-5") == "lite" + assert tier_of_model("mystery-model-9") is None From 2aa3b482b5554eaee7e7ab3d64a62a901947fcfc Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:32:49 -0400 Subject: [PATCH 16/27] feat(doctor): report config blocks an existing config.yml never learned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_check_config` writes the full template only when `config.yml` is *absent*. A repo initialized before a feature shipped therefore never learns that feature's knobs exist: every loader defaults gracefully, so nothing breaks, nothing complains, and the operator's config quietly diverges from the schema for as long as the repo lives. This is not hypothetical or new. Run against this very repo, the check reports eight missing blocks — `compact`, `context`, `ideate`, `model_tiers`, `parallelism`, `vault`, `verify`, `worktree` — and half of them predate tonight's work entirely. Nobody here has been able to configure the vault, the worktree probe, or ideation, because nothing ever mentioned they were configurable. The expected set is derived from `DEFAULT_CONFIG_YML` itself rather than a hand-maintained list, so the check cannot drift from the schema the way the thing it detects did. A test asserts the template satisfies its own check — otherwise the check is the broken party. Wording matters here: missing is not broken. The detail reads "not configured (defaults apply)", because every one of these has a working default and an operator who reads this as an error will go fix a non-problem. Advisory, and deliberately never repaired. `config.yml` is hand-edited and comment-rich; appending risks clobbering ordering or re-adding a key the operator deliberately deleted. A wrong merge into the file that governs every other behavior is worse than a message saying what to copy. There's a test asserting it writes nothing. 1309 tests pass (6 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/src/nightly_core/doctor.py | 52 +++++++++++++++ packages/nightly-core/tests/test_doctor.py | 63 +++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/packages/nightly-core/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index be5c302..9e37c98 100644 --- a/packages/nightly-core/src/nightly_core/doctor.py +++ b/packages/nightly-core/src/nightly_core/doctor.py @@ -160,6 +160,57 @@ def _check_config(root: Path, *, dry_run: bool) -> DoctorCheck: ) +def _check_config_blocks(root: Path) -> DoctorCheck: + """Name the config blocks an existing `config.yml` has never heard of. + + `_check_config` writes the full template only when the file is + *absent*. A repo initialized before a feature shipped therefore never + learns that feature's knobs exist: every loader defaults gracefully, + so nothing breaks and nothing complains. The operator's config quietly + diverges from the schema for as long as the repo lives — this repo was + missing eight blocks, half of them predating the current release. + + Advisory, and deliberately never repaired. `config.yml` is + hand-edited and comment-rich; appending to it risks clobbering + ordering or duplicating a key the operator deliberately removed. A + wrong merge into the file that governs every other behavior is worse + than a message telling them what to copy. + """ + import yaml # noqa: PLC0415 - lazy, doctor is not a hot path + + name, desc = "config_blocks", "config.yml schema drift" + config = nightly_dir(root) / "config.yml" + if not config.is_file(): + # `_check_config` owns the absent case and writes the template. + return DoctorCheck(name=name, description=desc, status="skipped", detail="no config yet") + + try: + present = yaml.safe_load(config.read_text(encoding="utf-8")) + expected = yaml.safe_load(DEFAULT_CONFIG_YML) + except (OSError, yaml.YAMLError): + return DoctorCheck( + name=name, description=desc, status="skipped", detail="config unreadable" + ) + if not isinstance(present, dict) or not isinstance(expected, dict): + return DoctorCheck(name=name, description=desc, status="skipped", detail="not a mapping") + + # Derived from the template itself, so this can never drift from the + # schema the way a hand-maintained list would. + missing = sorted(set(expected) - set(present)) + if not missing: + return DoctorCheck(name=name, description=desc, status="ok", detail="all blocks present") + return DoctorCheck( + name=name, + description=desc, + status="warning", + detail=( + f"not configured (defaults apply): {', '.join(missing)} — " + "see `.nightly/config.yml` in a freshly-initialized repo for the " + "annotated blocks to copy" + ), + ) + + def _configured_hosts(root: Path) -> tuple[HostId, ...]: """Host ids listed under `hosts:` in `.nightly/config.yml`. @@ -728,6 +779,7 @@ def diagnose_and_repair( checks: list[DoctorCheck] = [] checks.append(_check_nightly_scaffold(root, dry_run=dry_run)) checks.append(_check_config(root, dry_run=dry_run)) + checks.append(_check_config_blocks(root)) checks.append(_check_model_tiers(root)) checks.append(_check_tier_sanity(root)) checks.append(_check_push_readiness(root)) diff --git a/packages/nightly-core/tests/test_doctor.py b/packages/nightly-core/tests/test_doctor.py index e1e3e39..b6eea8a 100644 --- a/packages/nightly-core/tests/test_doctor.py +++ b/packages/nightly-core/tests/test_doctor.py @@ -580,3 +580,66 @@ def test_doctor_requires_context_token_cap_only_on_claude(repo: Path) -> None: report2 = diagnose_and_repair(repo, host_loader=_make_loaders({"cursor": cursor})) host_check2 = next(c for c in report2.checks if c.name == "host:cursor") assert host_check2.status == "ok" + + +# ── config schema drift ─────────────────────────────────────────────────── + + +def _cfg_at(root: Path, body: str) -> None: + (root / ".nightly").mkdir(parents=True, exist_ok=True) + (root / ".nightly" / "config.yml").write_text(body, encoding="utf-8") + + +def test_full_template_reports_no_drift(tmp_path: Path) -> None: + """The template must satisfy its own check, or the check is wrong.""" + from nightly_core.config import DEFAULT_CONFIG_YML + from nightly_core.doctor import _check_config_blocks + + _cfg_at(tmp_path, DEFAULT_CONFIG_YML) + assert _check_config_blocks(tmp_path).status == "ok" + + +def test_old_config_names_the_blocks_it_never_learned(tmp_path: Path) -> None: + """A repo initialized before a feature shipped defaults silently + forever — nothing breaks, so nothing ever says so.""" + from nightly_core.doctor import _check_config_blocks + + _cfg_at(tmp_path, "hosts:\n - claude\ngit:\n base_branch: main\n") + check = _check_config_blocks(tmp_path) + assert check.status == "warning" + for block in ("model_tiers", "parallelism", "verify"): + assert block in check.detail + + +def test_drift_detail_says_defaults_apply(tmp_path: Path) -> None: + """Missing is not broken — the wording must not read as an error.""" + from nightly_core.doctor import _check_config_blocks + + _cfg_at(tmp_path, "hosts:\n - claude\n") + assert "defaults apply" in _check_config_blocks(tmp_path).detail + + +def test_absent_config_defers_to_the_writer_check(tmp_path: Path) -> None: + from nightly_core.doctor import _check_config_blocks + + (tmp_path / ".nightly").mkdir(parents=True, exist_ok=True) + assert _check_config_blocks(tmp_path).status == "skipped" + + +def test_malformed_config_is_skipped_not_failed(tmp_path: Path) -> None: + from nightly_core.doctor import _check_config_blocks + + _cfg_at(tmp_path, "hosts: [unclosed\n") + assert _check_config_blocks(tmp_path).status == "skipped" + + +def test_drift_check_never_writes(tmp_path: Path) -> None: + """config.yml is hand-edited and comment-rich; a wrong merge into the + file that governs everything is worse than a message.""" + from nightly_core.doctor import _check_config_blocks + + _cfg_at(tmp_path, "hosts:\n - claude\n") + path = tmp_path / ".nightly" / "config.yml" + before = path.read_text(encoding="utf-8") + _check_config_blocks(tmp_path) + assert path.read_text(encoding="utf-8") == before From 4aa11660a5fd09b1db77897b39dbae30e3d62d10 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:37:37 -0400 Subject: [PATCH 17/27] refactor(cli): type the dispatch row printer; show each dispatch's tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems in one function. `_print_dispatch_row` was annotated `state: object` with the comment "typed via duck on the read side", which forced **thirteen** `# type: ignore[attr-defined]` suppressions — one per field access. The type was never actually unknown; it is `BackgroundDispatchResult`. A `TYPE_CHECKING` import expresses that with no runtime cost and no import cycle, and pyrefly now genuinely checks the function instead of being told to look away. Repo-wide suppressions drop from 59 to 44. Separately, the row omitted `tier` — so `dispatch status` printed per-tier capacity totals while giving the operator no way to see which tier any individual dispatch was on. Adding the totals without the per-row value that produces them was incoherent; both views now agree. status pid host role tier slug running 4242 claude reviewer reasoning 0001-reviewer running 4242 claude researcher lite 0001-researcher running 4242 claude implementer - 0001-implementer Untiered records (written before RFC 007) render `-` rather than `None`, which is noise in a table column. One of the new tests was wrong on first write — it asserted `"None" not in ` while the fixture's own slug is `task-None-`, so it would have passed for the wrong reason and failed for a real one. It now asserts on the tier field itself. 1313 tests pass (4 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- packages/nightly-core/src/nightly_core/cli.py | 39 ++++++++++-------- packages/nightly-core/tests/test_admission.py | 41 +++++++++++++++++++ 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/packages/nightly-core/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index 78d446f..91c9088 100644 --- a/packages/nightly-core/src/nightly_core/cli.py +++ b/packages/nightly-core/src/nightly_core/cli.py @@ -43,7 +43,7 @@ import sys from collections.abc import Callable from pathlib import Path -from typing import Annotated, cast +from typing import TYPE_CHECKING, Annotated, cast import typer @@ -102,6 +102,9 @@ ) from nightly_core.verify import VerifyReport, run_verify +if TYPE_CHECKING: # import only for annotations — keeps the CLI's cold start fast + from nightly_core.dispatch import BackgroundDispatchResult + app = typer.Typer( name="nightly", help="Nightly — continuously-running, host-native coding agent.", @@ -1859,8 +1862,8 @@ def dispatch_status_cmd( if not states: typer.echo("· no dispatches in the current run") return - typer.echo(f"{'status':<10} {'pid':<8} {'host':<10} {'role':<13} {'slug':<32} log") - typer.echo("-" * 78) + typer.echo(f"{'status':<10} {'pid':<8} {'host':<10} {'role':<13} {'tier':<10} {'slug':<32} log") + typer.echo("-" * 88) for state in states: live = refresh(state, root=root) _print_dispatch_row(live, root=root, verbose=False) @@ -1889,29 +1892,31 @@ def _print_tier_utilization(root: Path) -> None: def _print_dispatch_row( - state: object, # BackgroundDispatchResult — typed via duck on the read side + state: BackgroundDispatchResult, *, root: Path, verbose: bool, ) -> None: """Render one dispatch as either a compact table row or a verbose block.""" - log = _format_path_for_display(state.log_path, root) # type: ignore[attr-defined] + log = _format_path_for_display(state.log_path, root) + tier = state.tier or "-" if verbose: - typer.echo(f"slug: {state.slug}") # type: ignore[attr-defined] - typer.echo(f"role: {state.role}") # type: ignore[attr-defined] - typer.echo(f"host: {state.host}") # type: ignore[attr-defined] - typer.echo(f"pid: {state.pid}") # type: ignore[attr-defined] - typer.echo(f"status: {state.status}") # type: ignore[attr-defined] - if state.exit_code is not None: # type: ignore[attr-defined] - typer.echo(f"exit_code: {state.exit_code}") # type: ignore[attr-defined] - typer.echo(f"started: {state.started_at.strftime('%Y-%m-%dT%H:%M:%SZ')}") # type: ignore[attr-defined] - if state.finished_at is not None: # type: ignore[attr-defined] - typer.echo(f"finished: {state.finished_at.strftime('%Y-%m-%dT%H:%M:%SZ')}") # type: ignore[attr-defined] + typer.echo(f"slug: {state.slug}") + typer.echo(f"role: {state.role}") + typer.echo(f"host: {state.host}") + typer.echo(f"tier: {tier}") + typer.echo(f"pid: {state.pid}") + typer.echo(f"status: {state.status}") + if state.exit_code is not None: + typer.echo(f"exit_code: {state.exit_code}") + typer.echo(f"started: {state.started_at.strftime('%Y-%m-%dT%H:%M:%SZ')}") + if state.finished_at is not None: + typer.echo(f"finished: {state.finished_at.strftime('%Y-%m-%dT%H:%M:%SZ')}") typer.echo(f"log: {log}") return typer.echo( - f"{state.status:<10} {state.pid:<8} {state.host:<10} " # type: ignore[attr-defined] - f"{state.role:<13} {state.slug:<32} {log}" # type: ignore[attr-defined] + f"{state.status:<10} {state.pid:<8} {state.host:<10} " + f"{state.role:<13} {tier:<10} {state.slug:<32} {log}" ) diff --git a/packages/nightly-core/tests/test_admission.py b/packages/nightly-core/tests/test_admission.py index 762a74f..acd3d87 100644 --- a/packages/nightly-core/tests/test_admission.py +++ b/packages/nightly-core/tests/test_admission.py @@ -238,3 +238,44 @@ async def run(args, root): await create_worktree(tmp_path, "new-task", runner=run, max_worktrees=8) assert not any(a[:2] == ["worktree", "add"] for a in calls) + + +# ── dispatch status rows (typing + tier column) ─────────────────────────── + + +def _capture_row(dispatch: BackgroundDispatchResult, *, verbose: bool) -> str: + import io + from contextlib import redirect_stdout + + from nightly_core.cli import _print_dispatch_row + + buf = io.StringIO() + with redirect_stdout(buf): + _print_dispatch_row(dispatch, root=Path("/tmp"), verbose=verbose) + return buf.getvalue() + + +def test_compact_row_shows_the_tier() -> None: + """`dispatch status` reported per-tier totals while giving no way to + see which tier any individual dispatch was on.""" + assert "reasoning" in _capture_row(_dispatch("reasoning"), verbose=False) + + +def test_verbose_row_shows_the_tier() -> None: + assert "tier: lite" in _capture_row(_dispatch("lite"), verbose=True) + + +def test_untiered_dispatch_renders_a_placeholder_not_none() -> None: + """Pre-RFC-007 records have no tier; `None` in a table column is noise.""" + out = _capture_row(_dispatch(None), verbose=True) + # Assert on the tier field itself — the fixture's slug legitimately + # contains the word, so a whole-line check would test nothing. + tier_line = next(ln for ln in out.splitlines() if ln.startswith("tier:")) + assert tier_line.split(":", 1)[1].strip() == "-" + + +def test_row_columns_stay_aligned_across_tiers() -> None: + """A ragged table is worse than no column — pin the widths.""" + rows = [_capture_row(_dispatch(t), verbose=False) for t in ("lite", "coding", "reasoning")] + starts = [r.index("task-") for r in rows] + assert len(set(starts)) == 1 From 45222ac5a8a818f62a7c4afea636e0427872881c Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:43:34 -0400 Subject: [PATCH 18/27] feat(rules): pre-flight verification doctrine (RFC 008 A); reconcile RFC 007 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 008 was accepted with eight unchecked items and zero done — the oldest untouched human-blessed work in the repo. Its premise is that an unchecked box means "nobody ticked it", not "nobody did it", and this commit demonstrates the premise on itself. **RFC 007 B2 and C2 were both already implemented and never ticked.** B2 shipped as rule 12 in `173a7e8`; C2 shipped as `_check_model_tiers` in `9b23ef1`, recorded there as "A8, pulled forward from C2" while the C2 box stayed unchecked. I reported RFC 007 as complete on that basis — true in substance, unverifiable from the checklist. Both are now ticked with the SHA that implemented them, which is the format rule 13 asks for. Phase A ships the doctrine as **rule 13** of the shared rules block: before implementing an item, check whether the deliverable exists — the symbol, an unmerged `nightly/*` branch, an open PR — and if it does, tick the box and commit the reconciliation alone rather than re-implementing. This completes a three-layer guard against re-doing finished work, and `_is_item_in_flight`'s docstring now names all three: the open-PR skip (in flight), the local-branch guard from `1791974` (done but unmerged), and this verifier (done and unrecorded — the case neither can see). Two deviations from the RFC as written, both stated in its checklist: - **A2** specifies the paragraph in six host `skill.md` files. Delivered via the shared rules block instead — the RFC itself calls that duplication "a doctor-monitored drift surface" (Resolved #9), which concedes it is a liability. One marker-delimited copy reaches all hosts and cannot drift between them. - **A3** wants a per-host skill token for drift detection. With A2's delivery there is no per-host copy to drift, so the item is **left open, not silently reinterpreted** — a human should confirm the substitution before it is ticked. Ticking it myself would be the exact failure rule 13 exists to prevent, in reverse. 1318 tests pass (5 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 10 ++++- .../rfcs/008-pre-rfc-completion-check.md | 23 ++++++++--- .../nightly-core/src/nightly_core/cascade.py | 10 +++++ .../nightly-core/src/nightly_core/rules.py | 22 +++++++++++ .../src/nightly_core/specialists.py | 38 ++++++++++++++++++- packages/nightly-core/tests/test_rules.py | 36 ++++++++++++++++++ 6 files changed, 131 insertions(+), 8 deletions(-) diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index e00ba96..8d76b35 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -529,7 +529,10 @@ missing config; README updated. **Phase B — Dispatch integration** — *core landed 2026-07-28; B2/B3 open* - [x] B1. `nightly dispatch start` reads resolved model id and passes it with the **discovered** model flag (see B6) -- [ ] B2. Task-tool fallback documented across six host skill.md +- [x] B2. Task-tool fallback documented — delivered as rule 12 of the + shared rules block alongside C1, not six skill files. *(Ticked + 2026-07-28 by RFC 008's own doctrine: implemented in `173a7e8`, + checklist never reconciled.)* - [x] B3. Briefing tier-breakdown line — a `dispatches by tier` panel rendered from each task's `dispatch.json`. Reads the run's task dirs directly rather than via `list_dispatches`, which resolves the @@ -551,7 +554,10 @@ missing config; README updated. every host's AGENTS.md / CLAUDE.md, so one edit reaches all seven harnesses and cannot drift between them. B2 is satisfied by the same change. -- [ ] C2. Doctor flags missing `model_tiers` block +- [x] C2. Doctor flags missing `model_tiers` block — shipped early as + `_check_model_tiers` in `9b23ef1`. *(Ticked 2026-07-28; recorded + there as "A8, pulled forward from C2" but the C2 box was left + unchecked — exactly the drift RFC 008 exists to catch.)* - [x] C3. README "Cost-aware dispatch" section — plus a "Context handoff" section, the `parallelism:` enforcement note, and the previously-undocumented `nightly dispatch` command family in the diff --git a/.planning/rfcs/008-pre-rfc-completion-check.md b/.planning/rfcs/008-pre-rfc-completion-check.md index 4f210a1..2079137 100644 --- a/.planning/rfcs/008-pre-rfc-completion-check.md +++ b/.planning/rfcs/008-pre-rfc-completion-check.md @@ -1,5 +1,6 @@ --- status: accepted +phase_a: implemented (A3 deferred — see note) sized: true title: Pre-RFC completion check — verify deliverable doesn't already exist before dispatching created: 2026-06-04 @@ -382,11 +383,23 @@ both branches. ## Sized checklist **Phase A — Verifier paragraph + auto-tick documentation** -- [ ] A1. `RFC_008_VERIFIER_PARAGRAPH` constant in `nightly_core.specialists` (or `skill_blocks` if extracted) -- [ ] A2. Verifier paragraph added to all six host `skill.md` files in the SCOPE step -- [ ] A3. `_REQUIRED_SKILL_TOKENS` extended with `pre-flight verification` per host -- [ ] A4. `_is_item_in_flight` docstring cross-references this RFC's verifier -- [ ] A5. Tests covering presence + doctor drift detection +- [x] A1. `RFC_008_VERIFIER_PARAGRAPH` constant in `nightly_core.specialists` +- [x] A2. Verifier doctrine reaches every host — as **rule 13 of the + shared rules block**, not six duplicated skill files. This RFC + itself calls the per-host duplication "a doctor-monitored drift + surface" (Resolved #9), which concedes the duplication is a + liability; `seed_rules` propagates one marker-delimited copy to + every host's AGENTS.md / CLAUDE.md and cannot drift between them. + Same delivery as RFC 007 C1/B2. +- [ ] A3. `_REQUIRED_SKILL_TOKENS` per-host token — **not applicable as + written**, given A2's delivery: there is no per-host copy to drift. + The equivalent guard is doctor's existing `_check_rules` plus a test + asserting the doctrine is inside the seeded markers (A5). Left open + rather than silently reinterpreted — a human should confirm the + substitution before it is ticked. +- [x] A4. `_is_item_in_flight` docstring cross-references the verifier + and the RFC 012 local-branch guard as the three composing layers +- [x] A5. Tests covering presence, marker placement, and content **Phase B — Briefing surface + auto-tick commit format** - [ ] B1. Briefing template gains "Auto-ticked RFC items" optional section diff --git a/packages/nightly-core/src/nightly_core/cascade.py b/packages/nightly-core/src/nightly_core/cascade.py index dddbe12..929c834 100644 --- a/packages/nightly-core/src/nightly_core/cascade.py +++ b/packages/nightly-core/src/nightly_core/cascade.py @@ -476,6 +476,16 @@ def _git(*args: str) -> str | None: def _is_item_in_flight(rfc_filename: str, item_text: str, pr_texts: list[tuple[str, str]]) -> bool: """Is this RFC item likely addressed by an open Nightly PR? + One of three guards against re-doing finished work, and the only one + that runs in the cascade walker. The other two are + `_items_done_on_local_branches` (committed but unmerged) and RFC + 008's agent-side pre-flight verification + (`specialists.RFC_008_VERIFIER_PARAGRAPH`), which catches the case + neither can see: an item implemented under a different name, or one + whose checklist was simply never reconciled. They compose — this + skips what is *in flight*, the branch guard skips what is *done + locally*, and the verifier catches what is done and unrecorded. + Two heuristics, biased toward false negatives (we'd rather re-pick an in-flight item than silently skip one that isn't addressed): - PR title or body contains the RFC filename (sans `.md`) diff --git a/packages/nightly-core/src/nightly_core/rules.py b/packages/nightly-core/src/nightly_core/rules.py index 7aaf647..6280cec 100644 --- a/packages/nightly-core/src/nightly_core/rules.py +++ b/packages/nightly-core/src/nightly_core/rules.py @@ -293,6 +293,28 @@ carries goals and state, never a transcript; shedding the history is the whole point. +13. **Pre-flight verification — check the deliverable doesn't already exist.** + Before implementing an RFC checklist item, verify it is actually + outstanding. An unchecked box means "nobody ticked it", not "nobody did + it": work lands in a branch that hasn't merged, an item gets implemented + under a different name, or a phase ships and the checklist is never + reconciled. Re-implementing it wastes the night and risks a conflicting + second implementation. + + Check, in ascending cost: + 1. Does the named symbol / file / flag already exist? (`grep`, `ls`) + 2. Does an unmerged `nightly/*` branch already tick this item? + (`git show :`) + 3. Does an open PR's title or body reference this RFC or item? + + If the deliverable exists, do NOT re-implement it. Tick the box and + commit the reconciliation alone: + + docs(rfc-NNN): tick . — already implemented in + + Then take the next item. Reconciling a stale checklist IS progress; it + is what stops the next agent from burning its night on the same item. + ### Human shutdown intervention The keep-alive must never trap the operator. Three independent diff --git a/packages/nightly-core/src/nightly_core/specialists.py b/packages/nightly-core/src/nightly_core/specialists.py index 6424d4b..1dc3886 100644 --- a/packages/nightly-core/src/nightly_core/specialists.py +++ b/packages/nightly-core/src/nightly_core/specialists.py @@ -17,7 +17,13 @@ from nightly_core.contract import ModelTier, SpecialistRole -__all__ = ["SPECIALIST_TIER_DEFAULTS", "all_roles", "specialist_prompt", "tier_for_role"] +__all__ = [ + "RFC_008_VERIFIER_PARAGRAPH", + "SPECIALIST_TIER_DEFAULTS", + "all_roles", + "specialist_prompt", + "tier_for_role", +] _IMPLEMENTER = """\ @@ -116,6 +122,36 @@ } +RFC_008_VERIFIER_PARAGRAPH = """\ +**Pre-flight verification — check the deliverable doesn't already exist.** +Before implementing an RFC checklist item, verify it is actually +outstanding. An unchecked box means "nobody ticked it", not "nobody did +it": work lands in a branch that hasn't merged, an item gets implemented +under a different name, or a phase ships and the checklist is never +reconciled. Re-implementing it wastes the night and risks a conflicting +second implementation. + +Check, in ascending cost: +1. Does the named symbol / file / flag already exist? (`grep`, `ls`) +2. Does an unmerged `nightly/*` branch already tick this item? + (`git show :`) +3. Does an open PR's title or body reference this RFC or item? + +If the deliverable exists, do NOT re-implement it. Tick the box and +commit the reconciliation alone: + + docs(rfc-NNN): tick . — already implemented in + +Then take the next item. Reconciling a stale checklist IS progress; it +is what stops the next agent from burning its night on the same item. +""" +"""Agent-facing doctrine for RFC 008 — verify before implementing. + +Lives here beside the specialist prompts because it is prompt text, not +behavior. `nightly_core.rules` embeds it in the shared rules block so a +single copy reaches every host.""" + + SPECIALIST_TIER_DEFAULTS: dict[SpecialistRole, ModelTier] = { "implementer": "coding", "tester": "coding", diff --git a/packages/nightly-core/tests/test_rules.py b/packages/nightly-core/tests/test_rules.py index 102933e..d1bd76d 100644 --- a/packages/nightly-core/tests/test_rules.py +++ b/packages/nightly-core/tests/test_rules.py @@ -267,3 +267,39 @@ def test_rule_12_is_inside_the_seeded_block() -> None: assert rendered.startswith(MARKER_START) assert rendered.rstrip().endswith(MARKER_END) assert "Run the fleet wide and cheap" in rendered + + +# ── rule 13: pre-flight verification (RFC 008 A5) ───────────────────────── + + +def test_verifier_doctrine_is_in_the_rules_body() -> None: + from nightly_core.specialists import RFC_008_VERIFIER_PARAGRAPH + + first_line = RFC_008_VERIFIER_PARAGRAPH.splitlines()[0] + assert first_line in NIGHTLY_RULES_BODY + + +def test_verifier_states_the_unchecked_box_fallacy() -> None: + """The whole premise: unchecked means unticked, not undone.""" + assert 'means "nobody ticked it", not "nobody did' in NIGHTLY_RULES_BODY + + +def test_verifier_gives_the_reconciliation_commit_format() -> None: + """Without a format, agents write freeform messages and the audit + trail for auto-ticks becomes ungreppable.""" + assert "docs(rfc-NNN): tick" in NIGHTLY_RULES_BODY + assert "already implemented in" in NIGHTLY_RULES_BODY + + +def test_verifier_names_all_three_checks() -> None: + body = NIGHTLY_RULES_BODY + assert "grep" in body + assert "nightly/*` branch" in body + assert "open PR" in body + + +def test_verifier_lands_inside_the_seeded_markers() -> None: + """Outside the markers it propagates to no host at all.""" + rendered = _render_block() + assert "Pre-flight verification" in rendered + assert rendered.startswith(MARKER_START) From 6e91197af2ce4ab16f59f70866fdf748a16fa0b0 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:48:37 -0400 Subject: [PATCH 19/27] feat(briefing): auto-ticked RFC items panel (RFC 008 Phase B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An auto-tick is a claim that work already existed, made by an agent that chose not to redo it. That is exactly the judgment an operator should audit: a wrong one silently drops an item nobody implemented, and it leaves no other trace — the box is ticked, the RFC looks done, and the work simply never happened. The briefing now surfaces them as "ticked as already-done (verify these)", scanning `main..HEAD` for rule 13's commit format and naming the RFC, the item, and the SHA the agent claims implemented it. A tick with no SHA renders as `unstated`, which is the case most worth opening. Rule 13 caught a real instance on its first use: **B2 was already satisfied.** The commit format shipped with A1/A2 in `15fb1bc`, so this commit ticks it rather than re-implementing it — the doctrine working on its own RFC, one commit after landing. The regex deliberately requires the literal `tick` verb: the sibling commit `docs(rfc-007): reconcile Phase B checklist` is an ordinary docs change, not a claim about pre-existing work, and there is a test pinning that it is not swept up. RFC 008 is now implemented except A3, which remains open pending a human decision on whether the shared-rules delivery substitutes for the per-host skill token it specifies. 1324 tests pass (6 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../rfcs/008-pre-rfc-completion-check.md | 11 +- .../nightly-core/src/nightly_core/briefing.py | 66 +++++++++++ .../nightly_core/templates/briefing.html.j2 | 13 +++ packages/nightly-core/tests/test_briefing.py | 105 ++++++++++++++++++ 4 files changed, 192 insertions(+), 3 deletions(-) diff --git a/.planning/rfcs/008-pre-rfc-completion-check.md b/.planning/rfcs/008-pre-rfc-completion-check.md index 2079137..f0a1928 100644 --- a/.planning/rfcs/008-pre-rfc-completion-check.md +++ b/.planning/rfcs/008-pre-rfc-completion-check.md @@ -1,6 +1,7 @@ --- status: accepted phase_a: implemented (A3 deferred — see note) +phase_b: implemented sized: true title: Pre-RFC completion check — verify deliverable doesn't already exist before dispatching created: 2026-06-04 @@ -402,6 +403,10 @@ both branches. - [x] A5. Tests covering presence, marker placement, and content **Phase B — Briefing surface + auto-tick commit format** -- [ ] B1. Briefing template gains "Auto-ticked RFC items" optional section -- [ ] B2. Commit message format documented in the verifier paragraph -- [ ] B3. Tests covering rendered briefing with and without auto-ticks +- [x] B1. Briefing gains a "ticked as already-done (verify these)" + panel, populated by scanning `main..HEAD` for rule 13's commit + format. Omitted when empty. +- [x] B2. Commit message format documented in the verifier paragraph — + shipped with A1/A2 in `15fb1bc`. *(Ticked by rule 13's own process: + verified present before implementing, not re-implemented.)* +- [x] B3. Tests covering parse, render-with, and render-without diff --git a/packages/nightly-core/src/nightly_core/briefing.py b/packages/nightly-core/src/nightly_core/briefing.py index deef59a..de5edf7 100644 --- a/packages/nightly-core/src/nightly_core/briefing.py +++ b/packages/nightly-core/src/nightly_core/briefing.py @@ -22,6 +22,8 @@ from __future__ import annotations import json +import re +import subprocess from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path @@ -33,6 +35,7 @@ from nightly_core.contract import MODEL_TIERS from nightly_core.digest import find_handoffs +from nightly_core.paths import repo_root from nightly_core.runs import Run __all__ = [ @@ -101,6 +104,14 @@ class BriefingContext: """RFC 006 §B2 — "yes" if session compaction fired, "no" if it did not, None if default omitted (e.g. keepalive.log absent).""" + auto_ticks: list[dict[str, str]] = field(default_factory=list) + """RFC 008 B1 — checklist items an agent ticked as already-done rather + than implementing. Each entry has `rfc`, `item`, `sha`, `source`. + + Deliberately its own panel: an auto-tick asserts that work already + existed, and a wrong assertion silently drops an item. That is the + one decision in the loop most worth a human's eye.""" + handoffs: list[dict[str, str]] = field(default_factory=list) """RFC 012 C3 — tasks that wrote a `HANDOFF.md` because an agent crossed a context threshold. Each entry has `slug` and `summary`. @@ -119,6 +130,59 @@ class BriefingContext: `reasoning` at all probably reviewed nothing.""" +_AUTO_TICK_RE = re.compile( + r"^docs\(rfc-(\d+)\):\s*tick\s+([^\s—-]+)(?:.*?already implemented in\s+(\S+?)\.?)?$", + re.IGNORECASE, +) +"""Matches the reconciliation commit format rule 13 asks agents to use: +`docs(rfc-NNN): tick . — already implemented in `.""" + + +def find_auto_ticks(root: Path | None = None) -> list[dict[str, str]]: + """Checklist items reconciled rather than re-implemented — RFC 008 B1. + + An auto-tick is a claim that work already existed, made by an agent + that chose not to redo it. That is exactly the judgment call an + operator should audit: a wrong one silently drops an item nobody + implemented. Surfacing them in the briefing turns an invisible + decision into a reviewable line. + + Scans `main..HEAD` for rule 13's commit format. Returns [] on any git + failure — the briefing must render for a repo without git history. + """ + repo = (root or repo_root()).resolve() + try: + proc = subprocess.run( + ["git", "log", "--format=%h%x00%s", "main..HEAD"], + cwd=repo, + capture_output=True, + text=True, + timeout=10, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return [] + if proc.returncode != 0: + return [] + + out: list[dict[str, str]] = [] + for line in proc.stdout.splitlines(): + sha, _, subject = line.partition("\x00") + match = _AUTO_TICK_RE.match(subject.strip()) + if not match: + continue + rfc, item, source = match.groups() + out.append( + { + "rfc": f"RFC {rfc}", + "item": item.rstrip(".,"), + "sha": sha, + "source": source or "unstated", + } + ) + return out + + def _load_tier_breakdown(run: Run) -> str | None: """Count this run's dispatches by model tier — RFC 007 Resolved #8. @@ -351,6 +415,7 @@ def build_context(run: Run, *, now: datetime | None = None) -> BriefingContext: current_branch=current_branch, compacted=compacted, tier_breakdown=_load_tier_breakdown(run), + auto_ticks=find_auto_ticks(run.path.parent.parent.parent), handoffs=[{"slug": slug, "summary": summary} for slug, summary in find_handoffs(run.path)], ) @@ -376,6 +441,7 @@ def render_briefing(run: Run, *, now: datetime | None = None) -> str: compacted=ctx.compacted, tier_breakdown=ctx.tier_breakdown, handoffs=ctx.handoffs, + auto_ticks=ctx.auto_ticks, ) diff --git a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 index 44f14a2..a194f9b 100644 --- a/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 +++ b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 @@ -202,6 +202,19 @@
{% endif %} + {# ── auto-ticked RFC items (RFC 008 B1) ──────────────────────────── #} + {% if auto_ticks %} +
+
ticked as already-done (verify these)
+
    + {% for a in auto_ticks %} +
  • {{ a.rfc }} {{ a.item }} — claimed present in + {{ a.source }} ({{ a.sha }})
    • + {% endfor %} +
+
+ {% endif %} + {# ── pending handoffs (RFC 012 C3) ───────────────────────────────── #} {% if handoffs %}
diff --git a/packages/nightly-core/tests/test_briefing.py b/packages/nightly-core/tests/test_briefing.py index c24b6a8..f86960d 100644 --- a/packages/nightly-core/tests/test_briefing.py +++ b/packages/nightly-core/tests/test_briefing.py @@ -459,3 +459,108 @@ def test_handoff_panel_names_the_task_not_just_a_count(tmp_path: Path) -> None: def test_no_handoff_panel_when_the_run_finished_cleanly(tmp_path: Path) -> None: run = start_run(tmp_path) assert "handed off mid-task" not in render_briefing(run) + + +# ── auto-ticked RFC items (RFC 008 B1/B3) ───────────────────────────────── + + +def _git_repo(root: Path) -> None: + import subprocess + + def g(*a: str) -> None: + subprocess.run(["git", *a], cwd=root, check=True, capture_output=True) + + g("init", "-q", "-b", "main") + g("config", "user.email", "t@example.com") + g("config", "user.name", "T") + g("config", "commit.gpgsign", "false") + (root / "f.txt").write_text("seed\n", encoding="utf-8") + g("add", "-A") + g("commit", "-qm", "seed") + g("checkout", "-q", "-b", "nightly/work") + + +def _commit(root: Path, subject: str) -> None: + import subprocess + + (root / "f.txt").write_text(subject, encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=root, check=True, capture_output=True) + subprocess.run(["git", "commit", "-qm", subject], cwd=root, check=True, capture_output=True) + + +def test_no_auto_ticks_in_a_plain_branch(tmp_path: Path) -> None: + from nightly_core.briefing import find_auto_ticks + + _git_repo(tmp_path) + _commit(tmp_path, "feat(x): ordinary work") + assert find_auto_ticks(tmp_path) == [] + + +def test_auto_tick_commit_is_captured(tmp_path: Path) -> None: + from nightly_core.briefing import find_auto_ticks + + _git_repo(tmp_path) + _commit(tmp_path, "docs(rfc-008): tick A2 — already implemented in 173a7e8") + ticks = find_auto_ticks(tmp_path) + assert len(ticks) == 1 + assert ticks[0]["rfc"] == "RFC 008" + assert ticks[0]["item"] == "A2" + assert ticks[0]["source"] == "173a7e8" + + +def test_tick_without_a_source_sha_is_marked_unstated(tmp_path: Path) -> None: + """A tick claiming no evidence is the one most worth auditing.""" + from nightly_core.briefing import find_auto_ticks + + _git_repo(tmp_path) + _commit(tmp_path, "docs(rfc-012): tick C1") + assert find_auto_ticks(tmp_path)[0]["source"] == "unstated" + + +def test_ordinary_docs_commits_are_not_mistaken_for_ticks(tmp_path: Path) -> None: + from nightly_core.briefing import find_auto_ticks + + _git_repo(tmp_path) + _commit(tmp_path, "docs(rfc-007): reconcile Phase B checklist") + assert find_auto_ticks(tmp_path) == [] + + +def test_non_git_directory_yields_no_ticks(tmp_path: Path) -> None: + from nightly_core.briefing import find_auto_ticks + + assert find_auto_ticks(tmp_path) == [] + + +def test_auto_tick_panel_tells_the_operator_to_verify(tmp_path: Path) -> None: + """The panel exists because a wrong auto-tick silently drops an item.""" + run = start_run(tmp_path) + ctx_html = render_briefing(run) + assert "ticked as already-done" not in ctx_html # none present + + from nightly_core.briefing import _ENV, BriefingContext + + template = _ENV.get_template("briefing.html.j2") + html = template.render( + run_id="r", + is_concluded=False, + tasks=[], + approvals=[], + planning=[], + issues=[], + issues_by_strategic_category=[], + ready_count=0, + generated_at="now", + session_narrative=None, + lessons=None, + stacked_geometry=[], + current_branch="", + compacted=None, + tier_breakdown=None, + handoffs=[], + auto_ticks=[{"rfc": "RFC 008", "item": "A2", "sha": "abc1234", "source": "173a7e8"}], + ) + assert "ticked as already-done" in html + assert "verify these" in html + assert "RFC 008" in html + assert "173a7e8" in html + assert BriefingContext is not None From f17f76e9329935279e81a4fc8b39207183146943 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:53:15 -0400 Subject: [PATCH 20/27] test(doctor): pin that every check helper is actually wired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `diagnose_and_repair` builds its report from a hand-edited list of `checks.append(...)` calls. A `_check_*` helper that gets written but never appended runs zero times and reports nothing — silently, forever, with a passing test suite and a green doctor. That list grew by four in a single session. Nothing caught omissions except remembering, which is not a mechanism. `test_every_check_helper_is_wired_into_the_report` introspects the module for `_check_*` functions and asserts each appears in `diagnose_and_repair`'s source. `_check_host` is excluded — it is called per host inside the loop rather than appended once. Mutation-tested rather than assumed: deleting the `_check_tier_sanity` append makes it fail with the offending name, and restoring it makes it pass. A guard that cannot fail is decoration. Also added: report check names must be unique, since duplicates make the output ambiguous to read and impossible to filter. The module docstring claimed four checks; there are nine. It now separates the four that **repair** from the five that are **advisory and never write**, and states the two-step requirement for adding one. 1326 tests pass (2 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/src/nightly_core/doctor.py | 19 +++++++++- packages/nightly-core/tests/test_doctor.py | 36 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/nightly-core/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index 9e37c98..1203f82 100644 --- a/packages/nightly-core/src/nightly_core/doctor.py +++ b/packages/nightly-core/src/nightly_core/doctor.py @@ -8,7 +8,7 @@ back together without the user having to remember the exact sequence of `init` flags that produced their setup. -What it checks (and repairs by default): +Checks fall into two groups. **Repairing** checks fix what they find: 1. `.nightly/` scaffold — the five canonical subdirs from `cli.py` (`runs`, `plans`, `atlas`, `memory`, `prompts`). @@ -22,6 +22,23 @@ left alone unless the caller explicitly passes them via `extra_hosts`. +**Advisory** checks report and never write, because the right fix is a +judgment call the operator owns: + +5. Config schema drift — blocks an existing `config.yml` never learned, + which default silently forever. +6. Model-tier bindings — hosts with no tier→model mapping, where + routing is inert. +7. Tier/model agreement — a tier bound to a different band's model, + which is silent and expensive. +8. Push readiness — Nightly work that cannot leave the machine + (unpushed branches, a locked signing agent). +9. Worktree location — a repo under iCloud, where git state corrupts. + +Adding a check means writing a `_check_*` helper **and** appending it in +`diagnose_and_repair`; a helper that exists but is never called runs zero +times and reports nothing. `test_every_check_helper_is_wired` pins that. + Design parallels `update.refresh_repo_install` — both walk host loaders and call `install("project")` — but doctor's contract is broader: it also reconciles the non-host scaffold (`.nightly/`, config, rules) and diff --git a/packages/nightly-core/tests/test_doctor.py b/packages/nightly-core/tests/test_doctor.py index b6eea8a..8b200b9 100644 --- a/packages/nightly-core/tests/test_doctor.py +++ b/packages/nightly-core/tests/test_doctor.py @@ -643,3 +643,39 @@ def test_drift_check_never_writes(tmp_path: Path) -> None: before = path.read_text(encoding="utf-8") _check_config_blocks(tmp_path) assert path.read_text(encoding="utf-8") == before + + +# ── check registry ──────────────────────────────────────────────────────── + + +def test_every_check_helper_is_wired_into_the_report() -> None: + """A `_check_*` helper that nobody calls runs zero times and reports + nothing — silently, forever. The wiring is a hand-edited list that + grew by four in one session; this makes the omission structural + rather than a matter of remembering. + """ + import inspect + + from nightly_core import doctor as doctor_mod + + wiring = inspect.getsource(doctor_mod.diagnose_and_repair) + helpers = [ + name + for name, obj in vars(doctor_mod).items() + if name.startswith("_check_") + and inspect.isfunction(obj) + # `_check_host` is called per host inside the loop, not appended + # once like the rest. + and name != "_check_host" + ] + assert helpers, "no check helpers found — did the naming convention change?" + unwired = [n for n in helpers if n not in wiring] + assert not unwired, f"defined but never called in diagnose_and_repair: {unwired}" + + +def test_report_check_names_are_unique(repo: Path) -> None: + """Two checks sharing a name makes the report ambiguous to read and + impossible to filter.""" + report = diagnose_and_repair(repo, dry_run=True, host_loader={}) + names = [c.name for c in report.checks] + assert len(names) == len(set(names)) From 48af086b1e6913c3bbf4c37ea58672d59e51397a Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 15:58:21 -0400 Subject: [PATCH 21/27] test: exercise every Literal member through its dispatching function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tables are keyed by a `Literal` and indexed directly — `_PROMPTS`, `TIER_FAMILIES`, `DEFAULT_TIER_EFFORT`, `_EFFORT_DIRECTIVES`. Adding a member to the `Literal` without adding a row raises `KeyError` the moment that member is first used, which for a dispatch table means 3am, inside a spawned subprocess, with the traceback in a log nobody is reading. The RFCs discuss future specialist roles, so this is a change someone will actually make. The tests call the **public function** for every member rather than asserting on dict keys. A key-presence assertion passes for a row whose value is empty, wrong-typed, or unreachable behind a guard; calling the function is the actual contract. All four registries are total as of writing — this is prevention, not a fix. Two of the assertions encode invariants nobody had written down: - **Family round-trip.** A family listed under a tier must classify back to that tier, or `assign_tiers` files a model under one band while `tier_of_model` reports another. - **No family overlaps between tiers.** A shared substring makes classification order-dependent — the same model id would land in a different band depending on which tier happened to be checked first. Mutation-tested: adding `pro` (already under `coding`) to `reasoning` fails with both tier names, and removing it passes. Also asserts each role's prompt names its own role, which catches the copy-paste-a-sibling mistake that a non-empty check would not. 1405 tests pass (79 new, mostly parametrized); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/test_registry_totality.py | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 packages/nightly-core/tests/test_registry_totality.py diff --git a/packages/nightly-core/tests/test_registry_totality.py b/packages/nightly-core/tests/test_registry_totality.py new file mode 100644 index 0000000..e1422cf --- /dev/null +++ b/packages/nightly-core/tests/test_registry_totality.py @@ -0,0 +1,110 @@ +"""Every `Literal` member must survive the functions that dispatch on it. + +Several tables are keyed by a `Literal` and indexed directly — `_PROMPTS`, +`TIER_FAMILIES`, `DEFAULT_TIER_EFFORT`, `_EFFORT_DIRECTIVES`. Adding a +member to the `Literal` without adding a row raises `KeyError` at the +moment the new member is first used, which for a dispatch table means +3am, inside a spawned subprocess, with the traceback in a log nobody is +reading. + +These tests exercise the **public function** for every member rather than +asserting on dict keys. The distinction matters: a key-presence assertion +passes for a row whose value is empty, wrong-typed, or unreachable behind +a guard. Calling the function is the actual contract. + +All registries are total as of writing — this is prevention, not a fix. +""" + +from __future__ import annotations + +from typing import get_args + +import pytest + +from nightly_core.config import ( + DEFAULT_TIER_EFFORT, + ModelTierConfig, + ParallelismConfig, +) +from nightly_core.contract import ( + MODEL_TIERS, + HostId, + ModelTier, + ReasoningEffort, + SpecialistRole, +) +from nightly_core.model_probe import TIER_FAMILIES, assign_tiers, tier_of_model +from nightly_core.routing import effort_directive, resolve_model_for_task +from nightly_core.specialists import specialist_prompt, tier_for_role + + +@pytest.mark.parametrize("role", get_args(SpecialistRole)) +def test_every_role_has_a_usable_prompt(role: SpecialistRole) -> None: + prompt = specialist_prompt(role) + assert prompt.strip(), f"{role} has an empty prompt" + # A prompt that never names its own role is almost certainly a + # copy-paste of a sibling's. + assert role in prompt.lower() + + +@pytest.mark.parametrize("role", get_args(SpecialistRole)) +def test_every_role_resolves_to_a_real_tier(role: SpecialistRole) -> None: + assert tier_for_role(role) in MODEL_TIERS + + +@pytest.mark.parametrize("tier", MODEL_TIERS) +def test_every_tier_has_families_and_an_effort(tier: ModelTier) -> None: + assert TIER_FAMILIES[tier], f"{tier} has no model families" + assert DEFAULT_TIER_EFFORT[tier] in get_args(ReasoningEffort) + + +@pytest.mark.parametrize("tier", MODEL_TIERS) +def test_every_tier_round_trips_through_family_matching(tier: ModelTier) -> None: + """A family listed under a tier must classify back to that tier — + otherwise `assign_tiers` would file a model under one band and + `tier_of_model` report it as another.""" + sample = f"vendor-{TIER_FAMILIES[tier][0]}-1" + assert tier_of_model(sample) == tier + assert assign_tiers([sample]) == {tier: sample} + + +@pytest.mark.parametrize("effort", get_args(ReasoningEffort)) +def test_every_effort_level_has_a_directive(effort: ReasoningEffort) -> None: + text = effort_directive(effort) + assert text.strip(), f"{effort} has no directive" + + +@pytest.mark.parametrize("host", get_args(HostId)) +@pytest.mark.parametrize("tier", MODEL_TIERS) +def test_binding_resolves_for_every_host_and_tier(host: HostId, tier: ModelTier) -> None: + """Hosts without a seeded map must yield `model=None`, never raise — + an unbound host falls through to its CLI default by design.""" + binding = ModelTierConfig().binding(host, tier) + assert binding.tier == tier + assert binding.effort in get_args(ReasoningEffort) + + +@pytest.mark.parametrize("host", get_args(HostId)) +@pytest.mark.parametrize("role", get_args(SpecialistRole)) +def test_dispatch_resolves_for_every_host_and_role(host: HostId, role: SpecialistRole) -> None: + resolved = resolve_model_for_task(host=host, role=role, config=ModelTierConfig()) + assert resolved.tier in MODEL_TIERS + assert resolved.source in {"plan", "role", "disabled"} + + +@pytest.mark.parametrize("tier", MODEL_TIERS) +def test_parallelism_answers_for_every_tier(tier: ModelTier) -> None: + assert ParallelismConfig().limit_for(tier) >= 0 + + +def test_tier_families_do_not_overlap() -> None: + """A family string in two tiers makes classification order-dependent, + so the same model id would land in different bands depending on which + tier happened to be checked first.""" + seen: dict[str, ModelTier] = {} + for tier in MODEL_TIERS: + for family in TIER_FAMILIES[tier]: + assert family not in seen, ( + f"family {family!r} appears in both {seen.get(family)} and {tier}" + ) + seen[family] = tier From 84a95adb740e93ce341c94b153db091998abe176 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:04:05 -0400 Subject: [PATCH 22/27] test(conclude_skill): pin the companion skills' human-only doctrine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nightly_core.conclude_skill` holds the markdown for `/nightly-conclude`, `/nightly-update`, `/nightly-init`, and `/nightly-bug` — 301 lines that every host install writes verbatim, and that no test referenced. A malformed constant breaks skill loading on all seven harnesses at once; a subtly edited one changes what the agent believes it may do. Two are human-only off-ramps, and the rules block records that an agent has **already self-concluded in production** — running `nightly conclude` on its own initiative, freezing the cascade and ending a session with unblocked work still on disk. The only thing preventing a repeat is the warning text inside these constants, and nothing asserted it was still there. Covered: frontmatter parses; `name:` matches the `/command` an operator types; body is non-empty; the prohibition appears in the **description** specifically, not just the body — the description is what a host shows in its skill listing, and often all the agent reads before deciding to invoke; each off-ramp names the correct wrap-up (`ideate` → `brief`), because "don't do this" without an alternative is how an agent talks itself back into doing it. Also pins that no fifth companion skill can be added without a test row, which is exactly how all four went uncovered. Method note: the first mutation I ran to validate the self-invocation guard did not actually apply — the literal I targeted didn't match — and the test passed for that reason, not because the guard was weak. Re-run with a regex that provably edited the file, it fails with the offending description and passes on restore. Worth recording: I nearly reported a verified guard on false evidence. 1428 tests pass (23 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/tests/test_conclude_skill.py | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 packages/nightly-core/tests/test_conclude_skill.py diff --git a/packages/nightly-core/tests/test_conclude_skill.py b/packages/nightly-core/tests/test_conclude_skill.py new file mode 100644 index 0000000..b250ff0 --- /dev/null +++ b/packages/nightly-core/tests/test_conclude_skill.py @@ -0,0 +1,134 @@ +"""The four companion skill files, which nothing tested until now. + +`nightly_core.conclude_skill` holds the markdown for `/nightly-conclude`, +`/nightly-update`, `/nightly-init`, and `/nightly-bug`. Every host +install writes these verbatim, so a malformed one breaks skill loading on +all seven harnesses at once, and a subtly edited one changes what the +agent believes it is allowed to do. + +Two of them are **human-only off-ramps**. The rules block records that an +agent has already self-concluded in production — running `nightly +conclude` on its own initiative, freezing the cascade and ending a +session with unblocked work still on disk. The only thing standing +between that failure and a repeat is the warning text inside these +constants. Nothing asserted it was still there. +""" + +from __future__ import annotations + +import pytest + +from nightly_core.conclude_skill import ( + BUG_SKILL_MD, + CONCLUDE_SKILL_MD, + INIT_SKILL_MD, + UPDATE_SKILL_MD, +) + +ALL_SKILLS = { + "nightly-conclude": CONCLUDE_SKILL_MD, + "nightly-update": UPDATE_SKILL_MD, + "nightly-init": INIT_SKILL_MD, + "nightly-bug": BUG_SKILL_MD, +} + +# The off-ramps an agent must never invoke on its own initiative. +HUMAN_ONLY = ("nightly-conclude", "nightly-bug") + + +def _frontmatter(text: str) -> dict[str, str]: + """Parse the leading `---` fenced block into a flat dict.""" + assert text.startswith("---\n"), "skill must open with a frontmatter fence" + end = text.index("\n---\n", 3) + out: dict[str, str] = {} + for line in text[4:end].splitlines(): + key, sep, value = line.partition(":") + if sep: + out[key.strip()] = value.strip() + return out + + +@pytest.mark.parametrize("name", sorted(ALL_SKILLS)) +def test_skill_has_parseable_frontmatter(name: str) -> None: + """A malformed fence breaks skill loading on every host at once.""" + meta = _frontmatter(ALL_SKILLS[name]) + assert meta.get("name"), f"{name} has no `name:`" + assert meta.get("description"), f"{name} has no `description:`" + + +@pytest.mark.parametrize("name", sorted(ALL_SKILLS)) +def test_frontmatter_name_matches_the_invocable_command(name: str) -> None: + """The `name:` is what the operator types as `/`. A mismatch + installs a skill nobody can call.""" + assert _frontmatter(ALL_SKILLS[name])["name"] == name + + +@pytest.mark.parametrize("name", sorted(ALL_SKILLS)) +def test_skill_body_is_more_than_frontmatter(name: str) -> None: + text = ALL_SKILLS[name] + body = text[text.index("\n---\n", 3) + 5 :] + assert body.strip(), f"{name} has an empty body" + + +@pytest.mark.parametrize("name", HUMAN_ONLY) +def test_human_only_skills_forbid_self_invocation(name: str) -> None: + """This is the regression guard that matters. + + An agent has already self-concluded in production. The warning lives + only in this text; if an edit drops it, nothing else in the system + objects and the failure recurs silently. + """ + text = ALL_SKILLS[name] + lowered = text.lower() + assert "human" in lowered + assert "never" in lowered + # The prohibition must be visible in the *description*, not buried in + # the body — the description is what a host surfaces in its skill + # listing, and often all the agent reads before deciding to invoke. + description = _frontmatter(text)["description"].lower() + assert "never" in description or "human-only" in description + + +@pytest.mark.parametrize("name", HUMAN_ONLY) +def test_human_only_skills_name_the_correct_wrap_up(name: str) -> None: + """Saying "don't do this" without saying what to do instead is how an + agent talks itself back into doing it.""" + lowered = ALL_SKILLS[name].lower() + assert "ideate" in lowered or "brief" in lowered + + +def test_conclude_skill_explains_the_consequence() -> None: + """The rules block records the exact production failure; the skill + should carry the same reasoning so it survives independently.""" + lowered = CONCLUDE_SKILL_MD.lower() + assert "cascade" in lowered + assert "concluded" in lowered + + +def test_bug_skill_says_the_agent_must_not_self_file() -> None: + """Self-filing masks whatever the agent was about to do wrong.""" + lowered = BUG_SKILL_MD.lower() + assert "nightly bug" in lowered + assert "operator" in lowered or "human" in lowered + + +@pytest.mark.parametrize("name", sorted(ALL_SKILLS)) +def test_skills_do_not_end_mid_sentence(name: str) -> None: + """A truncated constant is the failure mode a string literal invites, + and it reads as valid markdown right up to the cut.""" + assert ALL_SKILLS[name].rstrip().endswith((".", "`", ")", "]", ":", "—", "!")) + + +def test_every_exported_skill_is_covered_here() -> None: + """A fifth companion skill added without a row would go untested the + same way all four were until now.""" + from nightly_core import conclude_skill + + exported = {n for n in conclude_skill.__all__ if n.endswith("_SKILL_MD")} + covered = { + "CONCLUDE_SKILL_MD", + "UPDATE_SKILL_MD", + "INIT_SKILL_MD", + "BUG_SKILL_MD", + } + assert exported == covered, f"untested companion skills: {exported - covered}" From ed0400092ccfe74ced06f445de3a4564d921b3c1 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:09:55 -0400 Subject: [PATCH 23/27] fix(doctor,rfc-007): partial tier bindings; stale ids in the RFC sample MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all verified against current code before acting. **Doctor treated a host as bound if its tier map was non-empty.** A host bound for only `coding` passed as configured while `lite` and `reasoning` silently fell through to the host CLI's default model. That partial shape is more dangerous than an empty map — an empty one at least looks unconfigured. `_check_model_tiers` now reports which tiers are missing per host, e.g. `codex (lite, reasoning)`. **The success detail sampled `sorted(hosts)[:1]`**, so with two hosts configured the second's bindings were never shown. It now lists every configured host. **RFC 007's Resolved #3 YAML presented four vendors' ids as shipped defaults.** Only `claude`, `cursor`, and `opencode` ship bindings; `codex`, `gemini`, and `antigravity` deliberately resolve to none. The sample now separates the shipped block from a commented, explicitly illustrative one. Worse than the finding reported: that sample also still carried `claude-sonnet-4-6` and `claude-opus-4-7`, superseded when the model table was amended earlier. A reader copying the sample would have configured two stale models and three phantom hosts. Zero stale vendor ids remain in the file. **The Context bullets still mapped reviewer→coding and researcher→reasoning**, directly contradicting the amended Resolved #4 five sections below. Both now match, and cite #4. Also fixes a pyrefly `bad-raise` in the new pr_feedback test file: `exc: object` should have been `BaseException | None`. 1446 tests pass (2 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .planning/rfcs/007-model-tier-routing.md | 45 +++--- .../nightly-core/src/nightly_core/doctor.py | 21 ++- .../tests/test_pr_feedback_failures.py | 141 ++++++++++++++++++ packages/nightly-core/tests/test_routing.py | 31 ++++ 4 files changed, 209 insertions(+), 29 deletions(-) create mode 100644 packages/nightly-core/tests/test_pr_feedback_failures.py diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index 8d76b35..eaa783c 100644 --- a/.planning/rfcs/007-model-tier-routing.md +++ b/.planning/rfcs/007-model-tier-routing.md @@ -65,9 +65,10 @@ task shape to choose a tier: - `nightly specialist implementer` — coding tier - `nightly specialist tester` — coding tier -- `nightly specialist reviewer` — coding tier (could be lite for - trivial diffs; conservative default is coding) -- `nightly specialist researcher` — reasoning tier +- `nightly specialist reviewer` — reasoning tier (review *is* result + validation; nothing downstream re-checks it — see Resolved #4) +- `nightly specialist researcher` — lite tier (file search and + summarization over code already on disk — see Resolved #4) - Plan body says "audit-only" / "doc-only" / "briefing-only" → lite tier - Plan frontmatter declares `model_tier: reasoning` → override @@ -203,31 +204,29 @@ Sonnet 4.7 doesn't break the config; only the model id under pointing at concrete model ids: ```yaml +# Shipped defaults — the only ids Nightly states authoritatively. model_tiers: claude: lite: claude-haiku-4-5 - coding: claude-sonnet-4-6 - reasoning: claude-opus-4-7 - codex: - lite: gpt-5-mini - coding: gpt-5 - reasoning: gpt-5-reasoning - cursor: + coding: claude-sonnet-5 + reasoning: claude-opus-5 + cursor: # Anthropic-backed, same ids lite: claude-haiku-4-5 - coding: claude-sonnet-4-6 - reasoning: claude-opus-4-7 - gemini: - lite: gemini-2.5-flash - coding: gemini-2.5-pro - reasoning: gemini-3.5-pro - antigravity: - lite: gemini-2.5-flash - coding: gemini-2.5-pro - reasoning: gemini-3.5-pro - opencode: + coding: claude-sonnet-5 + reasoning: claude-opus-5 + opencode: # Anthropic-backed, same ids lite: claude-haiku-4-5 - coding: claude-sonnet-4-6 - reasoning: claude-opus-4-7 + coding: claude-sonnet-5 + reasoning: claude-opus-5 + +# ILLUSTRATIVE ONLY — not shipped, and the ids below are placeholders. +# `codex`, `gemini`, and `antigravity` resolve to no binding by default; +# their dispatches fall through to the host CLI's own model. Wire real +# ids yourself, or let `nightly init` discover them (Resolved #12). +# codex: +# lite: +# coding: +# reasoning: ``` `nightly init` and `nightly doctor` write this default block. diff --git a/packages/nightly-core/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index 1203f82..0d9b496 100644 --- a/packages/nightly-core/src/nightly_core/doctor.py +++ b/packages/nightly-core/src/nightly_core/doctor.py @@ -271,12 +271,21 @@ def _check_model_tiers(root: Path) -> DoctorCheck: ) hosts = _configured_hosts(root) - unbound = sorted(h for h in hosts if not cfg.models.get(h)) + # A host is unbound if *any* tier is missing, not only if the whole map + # is. A partial map is the more dangerous shape: routing looks + # configured, and only the unbound tiers silently fall through to the + # host CLI's default model. + unbound: list[str] = [] + for host in sorted(hosts): + missing = [tier for tier in MODEL_TIERS if not cfg.binding(host, tier).model] + if missing: + unbound.append(f"{host} ({', '.join(missing)})") + if not unbound: - bound = ", ".join( - f"{tier}={cfg.binding(host, tier).model}" - for host in sorted(hosts)[:1] - for tier in MODEL_TIERS + bound = "; ".join( + f"{host}: " + + " ".join(f"{tier}={cfg.binding(host, tier).model}" for tier in MODEL_TIERS) + for host in sorted(hosts) ) return DoctorCheck( name="model_tiers", @@ -290,7 +299,7 @@ def _check_model_tiers(root: Path) -> DoctorCheck: status="warning", detail=( f"no tier→model binding for: {', '.join(unbound)} " - "(dispatches use the host CLI's default model)" + "(those tiers use the host CLI's default model)" ), ) diff --git a/packages/nightly-core/tests/test_pr_feedback_failures.py b/packages/nightly-core/tests/test_pr_feedback_failures.py new file mode 100644 index 0000000..9152365 --- /dev/null +++ b/packages/nightly-core/tests/test_pr_feedback_failures.py @@ -0,0 +1,141 @@ +"""The `gh` wrappers in `pr_feedback` must degrade, never raise. + +`pr_rescue` is cascade slot 5 and the rules block makes it a priority — +getting open PRs back to green outranks fresh work. It runs unattended +against a network service, so every failure here is a realistic 3am +event: `gh` not installed, an expired token, a rate limit, a timeout, a +truncated or non-JSON body. + +If any of those propagates, the cascade dies mid-run instead of moving to +the next rung. `pr_feedback` was the lowest-coverage module in the +package (70%), and the uncovered lines were precisely these +failure branches — the ones that only execute when something is already +going wrong. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest + +from nightly_core import pr_feedback +from nightly_core.pr_feedback import _gh_pr_review_comments, _gh_pr_view + + +def _stub_run( + monkeypatch: pytest.MonkeyPatch, *, stdout: str = "", exc: BaseException | None = None +): + """Replace `subprocess.run` inside pr_feedback only.""" + + def fake(*_args, **_kwargs): + if exc is not None: + raise exc + return subprocess.CompletedProcess(args=["gh"], returncode=0, stdout=stdout, stderr="") + + monkeypatch.setattr(pr_feedback.subprocess, "run", fake) + + +# ── _gh_pr_view ─────────────────────────────────────────────────────────── + + +def test_pr_view_parses_a_normal_response(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_run(monkeypatch, stdout=json.dumps({"number": 36, "title": "x"})) + assert _gh_pr_view("nightly/x", None) == {"number": 36, "title": "x"} + + +@pytest.mark.parametrize( + "exc", + [ + subprocess.CalledProcessError(1, "gh"), + subprocess.TimeoutExpired("gh", 30), + OSError("gh not found"), + ], + ids=["gh-failed", "gh-timed-out", "gh-missing"], +) +def test_pr_view_swallows_every_subprocess_failure( + monkeypatch: pytest.MonkeyPatch, exc: Exception +) -> None: + """No PR for the branch, an expired token, and `gh` not installed are + all indistinguishable here — and all mean "no feedback to act on".""" + _stub_run(monkeypatch, exc=exc) + assert _gh_pr_view("nightly/x", None) is None + + +def test_pr_view_treats_empty_output_as_no_pr(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_run(monkeypatch, stdout=" \n") + assert _gh_pr_view("nightly/x", None) is None + + +def test_pr_view_survives_non_json_output(monkeypatch: pytest.MonkeyPatch) -> None: + """A proxy error page or a truncated body must not raise.""" + _stub_run(monkeypatch, stdout="502 Bad Gateway") + assert _gh_pr_view("nightly/x", None) is None + + +def test_pr_view_rejects_json_that_is_not_an_object(monkeypatch: pytest.MonkeyPatch) -> None: + """Valid JSON of the wrong shape is the subtlest case — it parses, so + only the isinstance guard stops it reaching callers as a PR dict.""" + _stub_run(monkeypatch, stdout=json.dumps([{"number": 36}])) + assert _gh_pr_view("nightly/x", None) is None + + +# ── _gh_pr_review_comments ──────────────────────────────────────────────── + + +@pytest.mark.parametrize("pr_number", [0, -1]) +def test_review_comments_skips_the_api_for_a_bogus_number( + monkeypatch: pytest.MonkeyPatch, pr_number: int +) -> None: + """Guard before the call — a malformed PR number should not spend a + network round trip to learn it was malformed.""" + + def explode(*_a, **_k): + raise AssertionError("subprocess should not have been invoked") + + monkeypatch.setattr(pr_feedback.subprocess, "run", explode) + assert _gh_pr_review_comments(pr_number, None) == [] + + +def test_review_comments_parses_a_normal_response(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_run(monkeypatch, stdout=json.dumps([{"body": "nit"}, {"body": "bug"}])) + assert _gh_pr_review_comments(36, None) == [{"body": "nit"}, {"body": "bug"}] + + +@pytest.mark.parametrize( + "exc", + [ + subprocess.CalledProcessError(1, "gh"), + subprocess.TimeoutExpired("gh", 30), + OSError("gh not found"), + ], + ids=["gh-failed", "gh-timed-out", "gh-missing"], +) +def test_review_comments_swallows_every_subprocess_failure( + monkeypatch: pytest.MonkeyPatch, exc: Exception +) -> None: + _stub_run(monkeypatch, exc=exc) + assert _gh_pr_review_comments(36, None) == [] + + +def test_review_comments_survives_non_json_output(monkeypatch: pytest.MonkeyPatch) -> None: + _stub_run(monkeypatch, stdout="rate limit exceeded") + assert _gh_pr_review_comments(36, None) == [] + + +def test_review_comments_rejects_json_that_is_not_a_list(monkeypatch: pytest.MonkeyPatch) -> None: + """`--paginate` returns an array; an object means an error envelope.""" + _stub_run(monkeypatch, stdout=json.dumps({"message": "Not Found"})) + assert _gh_pr_review_comments(36, None) == [] + + +def test_wrappers_return_empty_rather_than_raising_on_a_real_missing_binary( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """End-to-end shape check with `gh` genuinely absent from PATH: both + wrappers must return their empty value, not propagate FileNotFoundError.""" + monkeypatch.setenv("PATH", str(tmp_path)) + assert _gh_pr_view("nightly/x", None) is None + assert _gh_pr_review_comments(36, None) == [] diff --git a/packages/nightly-core/tests/test_routing.py b/packages/nightly-core/tests/test_routing.py index 14c02f2..9d70ba2 100644 --- a/packages/nightly-core/tests/test_routing.py +++ b/packages/nightly-core/tests/test_routing.py @@ -405,3 +405,34 @@ def test_tier_of_model_has_no_opinion_on_unknown_ids() -> None: assert tier_of_model("claude-opus-5") == "reasoning" assert tier_of_model("claude-haiku-4-5") == "lite" assert tier_of_model("mystery-model-9") is None + + +def test_partially_bound_host_is_reported_as_unbound(tmp_path: Path) -> None: + """A host bound for only one tier looks configured while the other two + silently fall through to the host CLI's default — the more dangerous + shape than an entirely empty map.""" + from nightly_core.doctor import _check_model_tiers + + _write_config( + tmp_path, + "hosts:\n - codex\nmodel_tiers:\n codex:\n coding: some-vendor-model\n", + ) + check = _check_model_tiers(tmp_path) + assert check.status == "warning" + assert "codex" in check.detail + assert "lite" in check.detail + assert "reasoning" in check.detail + # The bound tier must not be listed as missing. + assert "coding" not in check.detail.split("(")[1].split(")")[0] + + +def test_ok_detail_covers_every_configured_host(tmp_path: Path) -> None: + """The detail used to sample only the first host, so a second host's + bindings were never shown.""" + from nightly_core.doctor import _check_model_tiers + + _write_config(tmp_path, "hosts:\n - claude\n - cursor\n") + check = _check_model_tiers(tmp_path) + assert check.status == "ok" + assert "claude:" in check.detail + assert "cursor:" in check.detail From efe1ba5cd165275b780d071bf84cf7f9c66481d7 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:13:37 -0400 Subject: [PATCH 24/27] test(plans): cover append_pr_feedback, the pr_rescue write path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plans.py` was the second-weakest module at 74%, and the entire gap was one function: `append_pr_feedback`. That is an uncomfortable thing to have untested, because unlike the read paths it **mutates the operator's task state**. Get the round numbering wrong and each round overwrites the last reviewer's comments; get the frontmatter wrong and the plan loses `status`, dropping out of the cascade entirely. Both are now pinned, along with the details that only bite in rendering: - Rounds accumulate rather than reusing round 1. - `status` and `slug` survive the rewrite, and the plan still parses through `read_plan` after two appends. - `pr_last_reconciled_at` is stamped — `pick_pr_rescue` skips on it, so without it the same feedback is re-applied forever. - Groups render blocking → humans → bots. The order is the point: a blocking review is *why* the agent was routed here and must not sit below bot chatter. - A multi-line comment is quoted on every line; a half-quoted body breaks the blockquote and the remainder renders as plan prose. - An empty body still emits a quote line — `"".splitlines()` is `[]`, so the `or [""]` fallback is load-bearing and now has a test saying so. 74% → 98%; the three remaining statements are `list_plans` filesystem guards. Package total is 90%. Also closes task 0026, whose work landed in df9442d but was never marked done — the cascade surfaced it as in-flight this turn, which is the mechanism working. 1462 tests pass (16 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/tests/test_plans_feedback.py | 197 ++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 packages/nightly-core/tests/test_plans_feedback.py diff --git a/packages/nightly-core/tests/test_plans_feedback.py b/packages/nightly-core/tests/test_plans_feedback.py new file mode 100644 index 0000000..815e99e --- /dev/null +++ b/packages/nightly-core/tests/test_plans_feedback.py @@ -0,0 +1,197 @@ +"""`append_pr_feedback` — the pr_rescue write path into a plan. + +Cascade slot 5 reads PR feedback and this writes it into `plan.md`. It +was the largest uncovered block in `plans.py` (74% before these tests), +which is uncomfortable for a function that *mutates the operator's task +state*: get the round numbering wrong and rounds overwrite each other, +get the frontmatter wrong and the cascade loses the plan's status. + +Feedback objects are duck-typed by the function (`getattr(f, +"is_blocking", False)`, `f.author_is_bot`, ...), so the fake below only +needs those attributes. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from nightly_core.plans import PR_LAST_RECONCILED_KEY, append_pr_feedback, read_plan + +FIXED = datetime(2026, 7, 28, 21, 30, tzinfo=UTC) + +PLAN = """\ +--- +status: in_progress +slug: 0001-example +--- + +# Task + +Original body text. +""" + + +@dataclass +class FakeFeedback: + author_login: str = "reviewer" + author_is_bot: bool = False + is_blocking: bool = False + kind: str = "review" + state: str = "CHANGES_REQUESTED" + body: str = "please fix" + url: str = "https://example.test/1" + file_ref: str | None = None + line_ref: int | None = None + + +@pytest.fixture +def plan_path(tmp_path: Path) -> Path: + p = tmp_path / "plan.md" + p.write_text(PLAN, encoding="utf-8") + return p + + +# ── round numbering ─────────────────────────────────────────────────────── + + +def test_first_call_writes_round_one(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback()], now=FIXED) + assert "## Feedback round 1" in plan_path.read_text(encoding="utf-8") + + +def test_second_call_increments_rather_than_overwriting(plan_path: Path) -> None: + """Rounds accumulate. Reusing round 1 would silently destroy the + previous reviewer's comments.""" + append_pr_feedback(plan_path, [FakeFeedback(body="first")], now=FIXED) + append_pr_feedback(plan_path, [FakeFeedback(body="second")], now=FIXED) + text = plan_path.read_text(encoding="utf-8") + assert "## Feedback round 1" in text + assert "## Feedback round 2" in text + assert "first" in text + assert "second" in text + + +# ── state preservation ──────────────────────────────────────────────────── + + +def test_frontmatter_and_body_survive(plan_path: Path) -> None: + """Losing `status` would drop the plan out of the cascade entirely.""" + append_pr_feedback(plan_path, [FakeFeedback()], now=FIXED) + plan = read_plan(plan_path) + assert plan.metadata["status"] == "in_progress" + assert plan.metadata["slug"] == "0001-example" + assert "Original body text." in plan.body + + +def test_reconciliation_timestamp_is_stamped(plan_path: Path) -> None: + """`pick_pr_rescue` skips plans whose PR has had no feedback since + this stamp — without it the same feedback is re-applied forever.""" + append_pr_feedback(plan_path, [FakeFeedback()], now=FIXED) + plan = read_plan(plan_path) + assert plan.metadata[PR_LAST_RECONCILED_KEY] == "2026-07-28T21:30:00Z" + assert plan.metadata["updated"] == plan.metadata[PR_LAST_RECONCILED_KEY] + + +def test_returned_record_matches_what_was_written(plan_path: Path) -> None: + returned = append_pr_feedback(plan_path, [FakeFeedback()], now=FIXED) + assert returned.metadata == read_plan(plan_path).metadata + + +# ── grouping ────────────────────────────────────────────────────────────── + + +def test_groups_render_blocking_then_humans_then_bots(plan_path: Path) -> None: + """Order is the point: a blocking review is why the agent was routed + here, so it must not be buried under bot chatter.""" + append_pr_feedback( + plan_path, + [ + FakeFeedback(author_login="botty", author_is_bot=True, body="nit"), + FakeFeedback(author_login="human", body="thought"), + FakeFeedback( + author_login="ci", is_blocking=True, kind="check_failure", state="FAILURE" + ), + ], + now=FIXED, + ) + text = plan_path.read_text(encoding="utf-8") + assert text.index("### Blocking") < text.index("### Human reviewers") + assert text.index("### Human reviewers") < text.index("### Bot reviewers") + + +def test_empty_groups_are_omitted(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback(author_login="human")], now=FIXED) + text = plan_path.read_text(encoding="utf-8") + assert "### Human reviewers" in text + assert "### Blocking" not in text + assert "### Bot reviewers" not in text + + +def test_no_feedback_says_so_explicitly(plan_path: Path) -> None: + """An empty section with no explanation reads as a rendering bug.""" + append_pr_feedback(plan_path, [], now=FIXED) + assert "_(no feedback returned)_" in plan_path.read_text(encoding="utf-8") + + +# ── per-item rendering ──────────────────────────────────────────────────── + + +def test_check_failure_renders_its_state(plan_path: Path) -> None: + append_pr_feedback( + plan_path, + [FakeFeedback(kind="check_failure", state="FAILURE", author_login="ci")], + now=FIXED, + ) + assert "(check: FAILURE)" in plan_path.read_text(encoding="utf-8") + + +def test_review_state_is_lowercased(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback(state="APPROVED")], now=FIXED) + assert "(approved)" in plan_path.read_text(encoding="utf-8") + + +def test_file_and_line_render_as_a_locator(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback(file_ref="src/a.py", line_ref=42)], now=FIXED) + assert "on `src/a.py:42`" in plan_path.read_text(encoding="utf-8") + + +def test_file_without_line_omits_the_colon(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback(file_ref="src/a.py")], now=FIXED) + text = plan_path.read_text(encoding="utf-8") + assert "on `src/a.py`" in text + assert "src/a.py:" not in text + + +def test_multiline_body_is_quoted_on_every_line(plan_path: Path) -> None: + """A half-quoted body breaks the markdown blockquote and the rest of + the comment renders as plan prose.""" + append_pr_feedback(plan_path, [FakeFeedback(body="line one\nline two")], now=FIXED) + text = plan_path.read_text(encoding="utf-8") + assert " > line one" in text + assert " > line two" in text + + +def test_empty_body_still_emits_a_quote_line(plan_path: Path) -> None: + """`"".splitlines()` is `[]`, so the `or [""]` fallback is what stops + the entry rendering as a bare bullet with no content.""" + append_pr_feedback(plan_path, [FakeFeedback(body="")], now=FIXED) + assert " > " in plan_path.read_text(encoding="utf-8") + + +def test_link_is_included_for_every_entry(plan_path: Path) -> None: + append_pr_feedback(plan_path, [FakeFeedback(url="https://example.test/9")], now=FIXED) + assert "[link](https://example.test/9)" in plan_path.read_text(encoding="utf-8") + + +def test_plan_remains_parseable_after_append(plan_path: Path) -> None: + """The whole point of writing through `render_frontmatter`: the next + `read_plan` must still see a valid plan.""" + append_pr_feedback(plan_path, [FakeFeedback(body="a\nb")], now=FIXED) + append_pr_feedback(plan_path, [FakeFeedback(author_is_bot=True)], now=FIXED) + plan = read_plan(plan_path) + assert plan.status == "in_progress" + assert plan.slug == "0001-example" From c0813e1a37f03158d552bdeddf2ad6869197056b Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:20:36 -0400 Subject: [PATCH 25/27] =?UTF-8?q?test(headless):=20cover=20default=5Fsubpr?= =?UTF-8?q?ocess=5Frunner=20(76%=20=E2=86=92=20100%)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `nightly run` task ultimately lands in this function, and none of it was exercised — the whole body was the module's coverage gap. Tested against real processes rather than a mocked asyncio, because the behaviour that matters is exactly what a mock would assume: stdout and stderr captured separately, exit code preserved (a swallowed non-zero reads as task success), stdin delivered, `cwd` honoured (a wrong one operates on the main checkout instead of the worktree), `TimeoutError` raised promptly rather than blocking the run behind one wedged task, `FileNotFoundError` propagated so per-host wrappers can name the missing CLI, and `timeout_s=None` meaning no deadline rather than zero. **One test written, then deleted, and the deletion is the point.** I wrote a check that the timed-out child is actually killed — poll `ps` for a uniquely-marked process after the timeout. It passed. Then I mutated `proc.kill()` out and it *still* passed: the event loop reaps the child during teardown regardless, so the assertion proved nothing about the kill. It is removed rather than shipped, with a comment in its place recording what was tried and why it does not work. A test that looks like a guard and isn't is worse than an acknowledged gap — it discourages anyone from writing the real one. Verifying that path needs the child's PID, which the runner does not expose; the honest fix is to surface it, not to assert around it. Related: the first version of that test used a fixed process marker and so matched orphans from earlier runs, including one it had leaked while being mutation-tested. Made unique per invocation before I concluded anything from it — a non-hermetic test would have "failed" on state it did not create. 1470 tests pass (8 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/test_headless_runner.py | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 packages/nightly-core/tests/test_headless_runner.py diff --git a/packages/nightly-core/tests/test_headless_runner.py b/packages/nightly-core/tests/test_headless_runner.py new file mode 100644 index 0000000..992ea23 --- /dev/null +++ b/packages/nightly-core/tests/test_headless_runner.py @@ -0,0 +1,109 @@ +"""`default_subprocess_runner` — the spawn behind every host's run_headless. + +Every `nightly run` task ultimately lands here, and the module's own +docstring documents a contract the callers depend on: return +`(stdout, stderr, rc)`; raise `TimeoutError` past the deadline; let +`FileNotFoundError` / `PermissionError` through so per-host wrappers can +surface them in `HeadlessResult.error`. + +None of that was exercised — the whole function was uncovered (76% +module coverage, and every missing line was this body). It is tested here +against **real processes** rather than a mocked asyncio: the behaviour +that matters (does the child actually die on timeout, do file +descriptors drain) is precisely what a mock would assume rather than +verify. +""" + +from __future__ import annotations + +import time +from pathlib import Path + +import pytest + +from nightly_core.headless import default_subprocess_runner + + +@pytest.mark.asyncio +async def test_captures_stdout_and_exit_code() -> None: + out, err, rc = await default_subprocess_runner(["sh", "-c", "printf hello"], None, None, 10) + assert out == b"hello" + assert err == b"" + assert rc == 0 + + +@pytest.mark.asyncio +async def test_captures_stderr_separately() -> None: + out, err, rc = await default_subprocess_runner(["sh", "-c", "printf oops >&2"], None, None, 10) + assert out == b"" + assert err == b"oops" + assert rc == 0 + + +@pytest.mark.asyncio +async def test_reports_a_non_zero_exit_code() -> None: + """`run_headless` infers task outcome from this when the agent did not + update the plan itself — a swallowed exit code would read as success.""" + _out, _err, rc = await default_subprocess_runner(["sh", "-c", "exit 3"], None, None, 10) + assert rc == 3 + + +@pytest.mark.asyncio +async def test_stdin_is_delivered_to_the_child() -> None: + out, _err, rc = await default_subprocess_runner(["cat"], None, b"piped", 10) + assert out == b"piped" + assert rc == 0 + + +@pytest.mark.asyncio +async def test_cwd_is_honoured(tmp_path: Path) -> None: + """Tasks run inside their worktree; a wrong cwd would silently operate + on the main checkout.""" + (tmp_path / "marker.txt").write_text("x", encoding="utf-8") + out, _err, rc = await default_subprocess_runner(["ls"], tmp_path, None, 10) + assert rc == 0 + assert b"marker.txt" in out + + +@pytest.mark.asyncio +async def test_timeout_raises_rather_than_hanging() -> None: + """The contract callers rely on: past the deadline this raises instead + of blocking the whole run behind one wedged task.""" + started = time.monotonic() + with pytest.raises(TimeoutError): + await default_subprocess_runner(["sleep", "30"], None, None, 0.3) + # Must return promptly — not after the child's own 30s. + assert time.monotonic() - started < 10 + + +# NOT TESTED HERE: that the timed-out child is actually killed. +# +# The `proc.kill()` + drain in the timeout handler exists so an overnight +# run spawning many tasks does not accumulate orphans. An attempt to +# verify it behaviourally — poll `ps` for a uniquely-marked child after +# the timeout — passed with `proc.kill()` mutated out, so it proved +# nothing: the event loop reaps the child during teardown regardless. +# +# Left unwritten rather than shipped looking like a guard. Verifying it +# needs the child's PID, which the runner does not expose; the honest fix +# is to surface it, not to assert around it. + + +@pytest.mark.asyncio +async def test_missing_binary_propagates(tmp_path: Path) -> None: + """Deliberately *not* swallowed here — per-host `run_headless` + catches it and reports which host CLI is absent, which is a better + message than an empty result.""" + with pytest.raises(FileNotFoundError): + await default_subprocess_runner([str(tmp_path / "definitely-not-a-binary")], None, None, 10) + + +@pytest.mark.asyncio +async def test_no_timeout_means_wait_indefinitely() -> None: + """`timeout_s=None` is the documented "no deadline" case — it must not + be mistaken for zero and fire immediately.""" + out, _err, rc = await default_subprocess_runner( + ["sh", "-c", "sleep 0.2; printf done"], None, None, None + ) + assert out == b"done" + assert rc == 0 From 48daf7da98877a7ca15ee11d6a8ba879269d9588 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:24:27 -0400 Subject: [PATCH 26/27] =?UTF-8?q?test(ci=5Fwatch):=20cover=20the=20classif?= =?UTF-8?q?ier=20that=20decides=20red=20vs=20green=20(86%=20=E2=86=92=2097?= =?UTF-8?q?%)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_bucket_for` maps GitHub's `state` / `conclusion` onto six buckets and `summarize_status` collapses a PR's checks into one verdict. Everything downstream keys off that verdict: `nightly ci` prints it and `pr_rescue` — cascade slot 5, a stated priority — routes on it. Neither function was covered. A mis-bucketed conclusion here is not a display bug. Reading `startup_failure` as anything but `fail` makes a broken PR look healthy and silently drops it from the rescue queue: the agent moves on to fresh work while the PR stays red all night. `stale` and `startup_failure` are the two easiest to get wrong — neither contains the word "fail". Pinned, in rough order of how expensive the mistake would be: - Every failure conclusion buckets as `fail`. - An unrecognised conclusion is `unknown`, never `pass`. When GitHub adds a new one, the failure mode must be "I can't tell", not "it's fine". - A PR with **no checks** is `unknown`, not passing-by-default — the difference between "CI has not reported" and "CI approved". - State beats conclusion while in flight, so a queued check carrying a previous run's `success` does not read green (or its `failure` red). - One red check outranks any number of green ones, following the declared `CHECK_STATUS_RANK` rather than re-deriving it. Process note: I wrote this against a `worst_bucket` function I assumed existed. It does not — the real name is `summarize_status`, and it takes `CICheck` objects rather than bucket strings. Caught at import, but the lesson is the same one as the mutation tests: check the API, don't predict it. 1493 tests pass (23 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/test_ci_watch_buckets.py | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 packages/nightly-core/tests/test_ci_watch_buckets.py diff --git a/packages/nightly-core/tests/test_ci_watch_buckets.py b/packages/nightly-core/tests/test_ci_watch_buckets.py new file mode 100644 index 0000000..d377aeb --- /dev/null +++ b/packages/nightly-core/tests/test_ci_watch_buckets.py @@ -0,0 +1,128 @@ +"""The classifier that decides whether a PR's CI reads as red or green. + +`_bucket_for` maps GitHub's `state` / `conclusion` strings onto six +buckets, and `summarize_status` collapses a PR's checks into one verdict. +Everything downstream keys off that verdict: `nightly ci` prints it, and +`pr_rescue` — cascade slot 5, which the rules block makes a priority — +routes on it. + +A mis-bucketed conclusion is therefore not a display bug. Classifying +`startup_failure` as anything but `fail` makes a broken PR read as +healthy and quietly removes it from the rescue queue; the agent moves on +to fresh work while the PR stays red. The classifier was uncovered. +""" + +from __future__ import annotations + +import pytest + +from nightly_core.ci_watch import ( + CHECK_STATUS_RANK, + CheckBucket, + CICheck, + _bucket_for, + summarize_status, +) + + +def _check(bucket: CheckBucket) -> CICheck: + return CICheck(name=f"check-{bucket}", bucket=bucket, state=bucket.upper()) + + +# ── failure conclusions ─────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "conclusion", + ["failure", "timed_out", "action_required", "stale", "startup_failure"], +) +def test_every_failure_conclusion_buckets_as_fail(conclusion: str) -> None: + """These are the ones that must never read as healthy. `stale` and + `startup_failure` are the easy ones to overlook — neither contains + the word 'fail'.""" + assert _bucket_for("completed", conclusion) == "fail" + + +def test_success_buckets_as_pass() -> None: + assert _bucket_for("completed", "success") == "pass" + + +@pytest.mark.parametrize("conclusion", ["neutral", "skipped"]) +def test_non_verdict_conclusions_bucket_as_skipping(conclusion: str) -> None: + """Neither ran nor failed — treating these as `fail` would put PRs in + the rescue queue that have nothing to rescue.""" + assert _bucket_for("completed", conclusion) == "skipping" + + +def test_cancelled_is_its_own_bucket() -> None: + """Distinct from `fail`: a cancelled run usually means someone pushed + again, not that the code is broken.""" + assert _bucket_for("completed", "cancelled") == "cancel" + + +# ── in-flight states ────────────────────────────────────────────────────── + + +@pytest.mark.parametrize("state", ["in_progress", "queued", "pending", "waiting"]) +def test_in_flight_states_bucket_as_pending(state: str) -> None: + """State wins over conclusion while a check is still running — a + stale `success` from a previous attempt must not mark it green.""" + assert _bucket_for(state, "success") == "pending" + + +def test_state_is_checked_before_conclusion() -> None: + """The ordering is load-bearing: a queued check carrying an old + `failure` conclusion is pending, not failed.""" + assert _bucket_for("queued", "failure") == "pending" + + +# ── unknown / malformed input ───────────────────────────────────────────── + + +@pytest.mark.parametrize("conclusion", ["", "some_new_github_conclusion"]) +def test_unrecognized_conclusion_is_unknown_not_pass(conclusion: str) -> None: + """The safe default. If GitHub adds a conclusion Nightly has not seen, + the failure mode must be 'I can't tell', never 'it's fine'.""" + assert _bucket_for("completed", conclusion) == "unknown" + + +def test_empty_inputs_do_not_raise() -> None: + assert _bucket_for("", "") == "unknown" + + +@pytest.mark.parametrize( + ("state", "conclusion", "expected"), + [("COMPLETED", "SUCCESS", "pass"), ("IN_PROGRESS", "", "pending")], +) +def test_matching_is_case_insensitive(state: str, conclusion: str, expected: str) -> None: + """gh has emitted both cases across versions.""" + assert _bucket_for(state, conclusion) == expected + + +# ── collapsing a PR's checks into one verdict ───────────────────────────── + + +def test_one_failure_makes_the_whole_pr_red() -> None: + """However many green checks surround it.""" + checks = tuple(_check(b) for b in ("pass", "pass", "fail", "pass")) + assert summarize_status(checks) == "fail" + + +def test_summary_follows_the_declared_rank() -> None: + """Rank order is the contract; pinned rather than re-derived.""" + for i, bucket in enumerate(CHECK_STATUS_RANK): + lower = CHECK_STATUS_RANK[i + 1 :] + if lower: + checks = tuple(_check(b) for b in (bucket, *lower)) + assert summarize_status(checks) == bucket + + +def test_a_pr_with_no_checks_is_unknown_not_passing() -> None: + """No signal is not a green light — the module's own docstring calls + this out, and it is the difference between "CI has not reported" and + "CI approved".""" + assert summarize_status(()) == "unknown" + + +def test_all_passing_is_pass() -> None: + assert summarize_status((_check("pass"), _check("pass"))) == "pass" From ba8686a0d0dabb1514f9510b4c795404384198f6 Mon Sep 17 00:00:00 2001 From: ulmentflam Date: Tue, 28 Jul 2026 16:37:00 -0400 Subject: [PATCH 27/27] =?UTF-8?q?test(triage):=20cover=20the=20open-PR=20i?= =?UTF-8?q?ssue-reference=20guard=20(82%=20=E2=86=92=2097%)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fetch_open_pr_issue_refs_via_gh` is the v0.0.11 fix for issue #27: an issue an open PR already claims must not be handed to the agent again. It was uncovered — which is uncomfortable given the session that wrote these tests spent most of a night inside the livelock that guard exists to prevent, and issue #30 documents the same shape 130 reroutes deep. Both failure directions are now pinned, because they fail differently: - **Under-reporting** re-picks covered work every boundary. All eleven closing-keyword forms GitHub documents are tested (close/closes/closed, fix/fixes/fixed, resolve/resolves/resolved, `Fixes:` with a colon, uppercase). A form this misses is an issue the cascade will grind on while a PR sits open against it. - **Over-reporting** silently drops real work from triage forever, which is quieter and worse. A bare `#9` must not read as a closing ref, and a bare mention on a *human* branch must not count — that asymmetry is deliberate and now has a test saying so. Also covered: the `gh` failure paths around it. No `gh` on PATH, expired token, timeout, rate-limit text, `null` title/body (gh emits null for an empty body — string-concatenating it would crash the scan and take the cascade with it). Every one degrades to "no refs known". Under-reporting is bad; raising into the cascade is worse. Process note, and it is the same note as last commit: I wrote the whole file calling `fetch_via_gh()` and `fetch_open_pr_issue_refs_via_gh()` with no arguments. Both require `root`. That is the fourth API I have predicted rather than read tonight, after two mutation tests that never applied and a `worst_bucket` that does not exist. Naming the pattern last turn did not stop it; reading the signature first is the only thing that will. 1529 tests pass (31 new); ruff, pyrefly, and `nightly verify` clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../nightly-core/tests/test_triage_pr_refs.py | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 packages/nightly-core/tests/test_triage_pr_refs.py diff --git a/packages/nightly-core/tests/test_triage_pr_refs.py b/packages/nightly-core/tests/test_triage_pr_refs.py new file mode 100644 index 0000000..b87005a --- /dev/null +++ b/packages/nightly-core/tests/test_triage_pr_refs.py @@ -0,0 +1,240 @@ +"""The guard that stops the cascade re-picking an issue a PR already covers. + +`fetch_open_pr_issue_refs_via_gh` is the v0.0.11 fix for issue #27: an +issue claimed by an open PR must not be handed to the agent again. When +it under-reports, the cascade re-picks covered work every boundary — the +exact livelock shape issue #30 documents, 130 identical reroutes deep. +When it over-reports, real work is skipped and never surfaces at all. + +It was uncovered, along with the `gh` failure paths around it. Those +failures are ordinary overnight events — no `gh` on PATH, an expired +token, a rate limit, a truncated body — and every one of them must +degrade to "no refs known", never raise into the cascade. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest + +from nightly_core import triage +from nightly_core.triage import ( + OpenPRRefs, + fetch_open_pr_issue_refs_via_gh, + fetch_via_gh, +) + + +def _stub(monkeypatch: pytest.MonkeyPatch, *, stdout: str = "", exc: BaseException | None = None): + def fake(*_a, **_k): + if exc is not None: + raise exc + return subprocess.CompletedProcess(args=["gh"], returncode=0, stdout=stdout, stderr="") + + monkeypatch.setattr(triage.subprocess, "run", fake) + monkeypatch.setattr(triage.shutil, "which", lambda _n: "/usr/local/bin/gh") + + +ROOT = Path("/tmp") + + +def _pr(title: str = "", body: str = "", head: str = "feature/x") -> dict: + return {"title": title, "body": body, "headRefName": head} + + +# ── closing keywords (any author, any branch) ───────────────────────────── + + +@pytest.mark.parametrize( + "phrase", + [ + "closes #30", + "close #30", + "closed #30", + "fix #30", + "fixes #30", + "fixed #30", + "resolve #30", + "resolves #30", + "resolved #30", + "Fixes: #30", + "FIXES #30", + ], +) +def test_every_closing_keyword_form_is_recognised( + monkeypatch: pytest.MonkeyPatch, phrase: str +) -> None: + """GitHub's documented grammar. A form this misses is an issue the + cascade will keep re-picking while a PR sits open against it.""" + _stub(monkeypatch, stdout=json.dumps([_pr(body=phrase)])) + assert 30 in fetch_open_pr_issue_refs_via_gh(ROOT).closing_refs + + +def test_closing_keyword_counts_from_any_branch(monkeypatch: pytest.MonkeyPatch) -> None: + """A human's PR closing an issue is just as disqualifying as a + Nightly one — the work is in flight either way.""" + _stub(monkeypatch, stdout=json.dumps([_pr(body="fixes #12", head="someone/else")])) + assert 12 in fetch_open_pr_issue_refs_via_gh(ROOT).closing_refs + + +def test_closing_keyword_found_in_the_title(monkeypatch: pytest.MonkeyPatch) -> None: + _stub(monkeypatch, stdout=json.dumps([_pr(title="fix #5: the thing")])) + assert 5 in fetch_open_pr_issue_refs_via_gh(ROOT).closing_refs + + +def test_a_bare_mention_is_not_a_closing_ref(monkeypatch: pytest.MonkeyPatch) -> None: + """ "See #9" is a cross-reference, not a claim to be fixing it. + Treating it as closing would silently drop #9 from triage forever.""" + _stub(monkeypatch, stdout=json.dumps([_pr(body="see #9 for context")])) + assert 9 not in fetch_open_pr_issue_refs_via_gh(ROOT).closing_refs + + +# ── bare mentions (Nightly-authored branches only) ──────────────────────── + + +def test_bare_mention_on_a_nightly_branch_counts(monkeypatch: pytest.MonkeyPatch) -> None: + """A bare `#N` in an orchestrator-owned PR means the issue is in + flight even without a closing keyword — the issue #27 fix.""" + _stub(monkeypatch, stdout=json.dumps([_pr(body="context: #27", head="nightly/thing")])) + refs = fetch_open_pr_issue_refs_via_gh(ROOT) + assert 27 in refs.nightly_mention_refs + + +def test_bare_mention_on_a_human_branch_does_not_count( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Deliberately asymmetric. Over-matching on human PRs would let one + stray `#N` in a description hide a real issue from triage.""" + _stub(monkeypatch, stdout=json.dumps([_pr(body="context: #27", head="feature/x")])) + assert 27 not in fetch_open_pr_issue_refs_via_gh(ROOT).nightly_mention_refs + + +def test_the_two_channels_stay_separate(monkeypatch: pytest.MonkeyPatch) -> None: + """They carry different evidentiary weight, so the cascade must be + able to tell them apart.""" + _stub( + monkeypatch, + stdout=json.dumps( + [ + _pr(body="fixes #1", head="human/a"), + _pr(body="touches #2", head="nightly/b"), + ] + ), + ) + refs = fetch_open_pr_issue_refs_via_gh(ROOT) + assert refs.closing_refs == frozenset({1}) + assert 2 in refs.nightly_mention_refs + assert 1 not in refs.nightly_mention_refs + + +# ── malformed and hostile input ─────────────────────────────────────────── + + +def test_non_dict_entries_are_skipped(monkeypatch: pytest.MonkeyPatch) -> None: + _stub(monkeypatch, stdout=json.dumps(["not-a-pr", 42, None, _pr(body="fixes #3")])) + assert 3 in fetch_open_pr_issue_refs_via_gh(ROOT).closing_refs + + +def test_null_title_and_body_do_not_raise(monkeypatch: pytest.MonkeyPatch) -> None: + """gh emits `null` for an empty body; string-concatenating it would + crash the whole scan and take the cascade with it.""" + _stub( + monkeypatch, + stdout=json.dumps([{"title": None, "body": None, "headRefName": "nightly/x"}]), + ) + assert fetch_open_pr_issue_refs_via_gh(ROOT) == OpenPRRefs() + + +def test_non_json_output_yields_no_refs(monkeypatch: pytest.MonkeyPatch) -> None: + _stub(monkeypatch, stdout="rate limit exceeded") + assert fetch_open_pr_issue_refs_via_gh(ROOT) == OpenPRRefs() + + +@pytest.mark.parametrize( + "exc", + [ + subprocess.CalledProcessError(1, "gh"), + subprocess.TimeoutExpired("gh", 30), + OSError("gh missing"), + ], + ids=["gh-failed", "gh-timed-out", "gh-missing"], +) +def test_subprocess_failures_degrade_to_no_refs( + monkeypatch: pytest.MonkeyPatch, exc: BaseException +) -> None: + """Under-reporting here re-picks covered issues, which is bad — but + raising kills the cascade entirely, which is worse.""" + _stub(monkeypatch, exc=exc) + assert fetch_open_pr_issue_refs_via_gh(ROOT) == OpenPRRefs() + + +def test_empty_output_is_treated_as_an_empty_list(monkeypatch: pytest.MonkeyPatch) -> None: + _stub(monkeypatch, stdout="") + assert fetch_open_pr_issue_refs_via_gh(ROOT) == OpenPRRefs() + + +# ── issue fetch failure paths ───────────────────────────────────────────── + + +def test_issue_fetch_returns_empty_without_gh(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(triage.shutil, "which", lambda _n: None) + assert fetch_via_gh(ROOT) == [] + + +@pytest.mark.parametrize( + "exc", + [ + subprocess.CalledProcessError(1, "gh"), + subprocess.TimeoutExpired("gh", 30), + OSError("gh missing"), + ], + ids=["gh-failed", "gh-timed-out", "gh-missing"], +) +def test_issue_fetch_swallows_subprocess_failures( + monkeypatch: pytest.MonkeyPatch, exc: BaseException +) -> None: + _stub(monkeypatch, exc=exc) + assert fetch_via_gh(ROOT) == [] + + +def test_issue_parser_skips_entries_missing_required_fields( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One malformed issue must not discard the whole triage list.""" + good = { + "number": 7, + "title": "t", + "body": "b", + "labels": [{"name": "bug"}], + "createdAt": "2026-01-01T00:00:00Z", + "updatedAt": "2026-01-02T00:00:00Z", + "url": "u", + "author": {"login": "me"}, + } + _stub(monkeypatch, stdout=json.dumps([{"title": "no number"}, good])) + issues = fetch_via_gh(ROOT) + assert [i.number for i in issues] == [7] + + +def test_issue_parser_survives_non_json(monkeypatch: pytest.MonkeyPatch) -> None: + _stub(monkeypatch, stdout="502") + assert fetch_via_gh(ROOT) == [] + + +def test_updated_at_falls_back_to_created_at(monkeypatch: pytest.MonkeyPatch) -> None: + """Ranking sorts on recency; a missing `updatedAt` must not crash it.""" + entry = { + "number": 8, + "title": "t", + "body": "", + "labels": [], + "createdAt": "2026-01-01T00:00:00Z", + "url": "u", + "author": {"login": "me"}, + } + _stub(monkeypatch, stdout=json.dumps([entry])) + issue = fetch_via_gh(ROOT)[0] + assert issue.updated_at == issue.created_at