diff --git a/.planning/rfcs/007-model-tier-routing.md b/.planning/rfcs/007-model-tier-routing.md index c1e1bc5..eaa783c 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 @@ -64,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 @@ -202,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. @@ -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 @@ -353,10 +382,25 @@ changing the config schema. - **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 @@ -481,14 +525,41 @@ 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 -- [ ] 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 +**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) +- [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 + *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 +- [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 -- [ ] C2. Doctor flags missing `model_tiers` block -- [ ] C3. README "Cost-aware dispatch" section +- [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. +- [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 + 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/.planning/rfcs/008-pre-rfc-completion-check.md b/.planning/rfcs/008-pre-rfc-completion-check.md index 4f210a1..f0a1928 100644 --- a/.planning/rfcs/008-pre-rfc-completion-check.md +++ b/.planning/rfcs/008-pre-rfc-completion-check.md @@ -1,5 +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 @@ -382,13 +384,29 @@ 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 -- [ ] 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/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md index 15339c1..7116c6f 100644 --- a/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md +++ b/.planning/rfcs/012-fleet-parallelism-and-context-handoff.md @@ -9,13 +9,15 @@ author: operator source: interactive_seed estimated_effort: ~5h across 3 phases phase_a: implemented +phase_b: implemented +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 @@ -256,15 +258,37 @@ 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 -- [ ] B3. Worktree creation honors `max_worktrees` -- [ ] B4. Six host skills: fan-out-to-cap guidance -- [ ] B5. Admission tests - -**Phase C — Handoff protocol** -- [ ] C1. Keepalive threshold comparison + prompt injection -- [ ] C2. `handoff:` section in `digest.md` -- [ ] C3. Briefing handoff counts -- [ ] C4. Six host skills: two-threshold protocol text +**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) +- [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`. +- [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 + +**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. +- [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/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/src/nightly_core/briefing.py b/packages/nightly-core/src/nightly_core/briefing.py index 6dc99c7..de5edf7 100644 --- a/packages/nightly-core/src/nightly_core/briefing.py +++ b/packages/nightly-core/src/nightly_core/briefing.py @@ -21,7 +21,10 @@ from __future__ import annotations -from dataclasses import dataclass +import json +import re +import subprocess +from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -30,6 +33,9 @@ from markdown_it import MarkdownIt from markupsafe import Markup +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__ = [ @@ -98,6 +104,121 @@ 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`. + + 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. + + 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.""" + + +_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. + + 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 +414,9 @@ 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), + auto_ticks=find_auto_ticks(run.path.parent.parent.parent), + handoffs=[{"slug": slug, "summary": summary} for slug, summary in find_handoffs(run.path)], ) @@ -315,6 +439,9 @@ 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, + handoffs=ctx.handoffs, + auto_ticks=ctx.auto_ticks, ) diff --git a/packages/nightly-core/src/nightly_core/cascade.py b/packages/nightly-core/src/nightly_core/cascade.py index 10060a7..929c834 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,9 +423,69 @@ 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? + 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`) @@ -478,9 +539,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/src/nightly_core/cli.py b/packages/nightly-core/src/nightly_core/cli.py index 7fb60f6..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 +from typing import TYPE_CHECKING, Annotated, cast import typer @@ -62,6 +62,7 @@ from nightly_core.contract import ( HostId, InstallScope, + ModelTier, NightlyHostIntegration, SpecialistRole, ) @@ -101,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.", @@ -112,6 +116,106 @@ _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. + + 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 +308,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 @@ -431,10 +535,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: @@ -698,13 +807,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() @@ -1418,6 +1555,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( @@ -1426,8 +1566,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 @@ -1518,8 +1664,26 @@ 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( +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."), @@ -1558,6 +1722,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. @@ -1571,11 +1742,40 @@ 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" + + 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, @@ -1584,6 +1784,9 @@ def dispatch_start_cmd( prompt=body, root=root, 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) @@ -1595,6 +1798,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( @@ -1656,37 +1862,61 @@ 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) + _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 + 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}" ) @@ -2014,10 +2244,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: @@ -2032,12 +2265,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 3c84d5e..86d3426 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", @@ -34,6 +35,7 @@ "ParallelismConfig", "TierBinding", "VaultConfig", + "VerifyConfig", "WorktreeConfig", "load_agents_config", "load_compact_config", @@ -42,7 +44,9 @@ "load_model_tier_config", "load_parallelism_config", "load_vault_config", + "load_verify_config", "load_worktree_config", + "render_config_yml", ] _log = logging.getLogger(__name__) @@ -402,6 +406,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 +466,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 +478,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 +519,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), ) @@ -725,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).""" @@ -784,7 +858,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 +965,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 @@ -952,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 @@ -960,12 +1029,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/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/dispatch.py b/packages/nightly-core/src/nightly_core/dispatch.py index 934f318..57b98b5 100644 --- a/packages/nightly-core/src/nightly_core/dispatch.py +++ b/packages/nightly-core/src/nightly_core/dispatch.py @@ -49,19 +49,25 @@ 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__ = [ "DEFAULT_LOG_FILENAME", "DEFAULT_STATE_FILENAME", + "HEADLESS_HOSTS", "BackgroundDispatchResult", "DispatchStatus", + "active_dispatches", + "admission_blocked", "build_argv", "is_alive", "list_dispatches", "read_dispatch_state", "start_background", + "tier_utilization", + "unsupported_host_message", "wait_for", "write_dispatch_state", ] @@ -72,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 @@ -102,17 +118,42 @@ 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 ──────────────────────────────────────────────────────── -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 +168,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 +178,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,28 +186,75 @@ 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 - # 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 ──────────────────────────────────────────────────────────────── @@ -178,6 +268,9 @@ 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, + tier: ModelTier | None = None, popen_factory: object | None = None, ) -> BackgroundDispatchResult: """Spawn the host's headless CLI as a detached background process. @@ -190,16 +283,9 @@ 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}'. " - "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() @@ -237,6 +323,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 @@ -315,6 +402,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"), @@ -450,3 +538,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/src/nightly_core/doctor.py b/packages/nightly-core/src/nightly_core/doctor.py index dcf3740..0d9b496 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 @@ -33,6 +50,7 @@ from __future__ import annotations import asyncio +import subprocess from collections.abc import Callable, Iterable, Mapping from dataclasses import dataclass from pathlib import Path @@ -159,6 +177,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`. @@ -202,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", @@ -221,11 +299,155 @@ 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)" ), ) +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_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? + + 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. @@ -583,7 +805,10 @@ 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)) 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/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/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/src/nightly_core/model_probe.py b/packages/nightly-core/src/nightly_core/model_probe.py new file mode 100644 index 0000000..87bf32b --- /dev/null +++ b/packages/nightly-core/src/nightly_core/model_probe.py @@ -0,0 +1,465 @@ +"""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", + "tier_of_model", +] + + +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(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]): + 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(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/src/nightly_core/rules.py b/packages/nightly-core/src/nightly_core/rules.py index dc48276..6280cec 100644 --- a/packages/nightly-core/src/nightly_core/rules.py +++ b/packages/nightly-core/src/nightly_core/rules.py @@ -253,6 +253,67 @@ 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. + +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 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/src/nightly_core/templates/briefing.html.j2 b/packages/nightly-core/src/nightly_core/templates/briefing.html.j2 index df53d1d..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,41 @@ {% 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 %} +
+
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 %} +
+
dispatches by tier
+
+ {{ tier_breakdown }} +
+
+ {% endif %} + {# ── session narrative (agent-authored, optional) ────────────────── #} {% if session_narrative %}
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/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 new file mode 100644 index 0000000..acd3d87 --- /dev/null +++ b/packages/nightly-core/tests/test_admission.py @@ -0,0 +1,281 @@ +"""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() == [] + + +# ── 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) + + +# ── 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 diff --git a/packages/nightly-core/tests/test_briefing.py b/packages/nightly-core/tests/test_briefing.py index c4afce4..f86960d 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,227 @@ 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) + + +# ── 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) + + +# ── 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 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) 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" 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 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}" 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_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) 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) diff --git a/packages/nightly-core/tests/test_doctor.py b/packages/nightly-core/tests/test_doctor.py index e1e3e39..8b200b9 100644 --- a/packages/nightly-core/tests/test_doctor.py +++ b/packages/nightly-core/tests/test_doctor.py @@ -580,3 +580,102 @@ 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 + + +# ── 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)) 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 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 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) 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) 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" 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_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) 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 diff --git a/packages/nightly-core/tests/test_routing.py b/packages/nightly-core/tests/test_routing.py index a070a74..9d70ba2 100644 --- a/packages/nightly-core/tests/test_routing.py +++ b/packages/nightly-core/tests/test_routing.py @@ -350,3 +350,89 @@ 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 + + +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 diff --git a/packages/nightly-core/tests/test_rules.py b/packages/nightly-core/tests/test_rules.py index 926f17b..d1bd76d 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,84 @@ 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 + + +# ── 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) 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 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