diff --git a/.autoskillit/test-filter-manifest.yaml b/.autoskillit/test-filter-manifest.yaml index b95a8c0c1..a654e1c88 100644 --- a/.autoskillit/test-filter-manifest.yaml +++ b/.autoskillit/test-filter-manifest.yaml @@ -245,6 +245,8 @@ scripts/check_pyi_stub_format.py: - infra/ scripts/check_pyi_stub_symbols.py: - infra/ +scripts/measure_codex_read_repetition.py: + - infra/ scripts/sync_versions.py: - infra/ diff --git a/docs/decisions/0005-output-budget-protocol.md b/docs/decisions/0005-output-budget-protocol.md index 48a9404e9..20f76ea5a 100644 --- a/docs/decisions/0005-output-budget-protocol.md +++ b/docs/decisions/0005-output-budget-protocol.md @@ -203,11 +203,13 @@ insertion control, not a measurement of remaining context. vs 258,400 in cli 0.144.1, but the effective window is server/catalog-controlled and oscillated during July 2026 (openai/codex#31860, #32806) — re-verify, do not assume an upgrade restores headroom. -- openai/codex#25458 / #27830: `fork_turns "none"` task-envelope delivery bug gates - the intake digest's sub-agent spawn rule — until the upstream bug is fixed, - `codex --json` sessions cannot reliably deliver task-envelope context to - sub-agents, so the intake discipline's "do not spawn sub-agents" guard remains - the operative constraint. +- openai/codex#25458 / #27830: `fork_turns "none"` task-envelope delivery bug — until + the upstream bug is fixed, `codex --json` sessions cannot reliably deliver + task-envelope context to sub-agents, so the intake discipline requires sub-agents to + be spawned with `fork_turns "none"` passed explicitly and given an explicit narrow + brief; sub-agents return a summary of their own task work, not raw file contents, + and must not be delegated the reading or interpretation of the caller's instruction + files. - openai/codex#33881: agent-TOML `model`/`model_reasoning_effort` reportedly ignored on 0.144.5 — affects the pins `_generate_agent_tomls` writes. - Upstream auto-compact semantics are scope-dependent @@ -222,3 +224,10 @@ insertion control, not a measurement of remaining context. - Recipe and ordinary result decisions are explicit and independent from history retention. - Artifact/invariant failures become explicit bounded errors instead of context floods. - The accepted gaps above remain visible work rather than implicit guarantees. +- The Codex context-intake policy (#4351) is advisory prose with no runtime gate, so it + is governed by the evidence-bound `CODEX_INTAKE_RULES` registry rather than by + `INVARIANT_REGISTRY`, which maps prohibitions to runtime enforcement targets. +- The intake digest is deliberately excluded from `PROBE_POLICY_IDENTITY`. Wiring it in + would make every intake-prose edit probe-gated under this ADR's Forward Obligations, + a real recurring operational cost; excluding it keeps that a conscious future decision + rather than a side effect of this change. diff --git a/scripts/measure_codex_read_repetition.py b/scripts/measure_codex_read_repetition.py new file mode 100644 index 000000000..f38b7b87f --- /dev/null +++ b/scripts/measure_codex_read_repetition.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python3 +"""Measure the Codex intake-discipline repeat-read rate across rollout sessions (#4351). + +Three repeat-read definitions have been published for this signal; this tool +implements the REPORT method, and states here how the other two differ: + + - report method (implemented here): counts, per session, only bounded-read + command shapes whose *leading* verb (before any pipe) is `sed -n`, + `head`/`tail` applied directly to a path, `nl -ba | sed -n`, or `rg -n`. + A trailing `| head -c N` output-safety wrapper is NOT itself a + bounded-read signal — nearly every command in this harness carries one, + so counting it naively would classify the vast majority of all commands + as "reads" and destroy the signal. A repeat is the 2nd+ bounded read of + the same resolved path within one session (rollout file). + - leading-command method (2026-07-24, 60-rollout sample): the same + leading-shape classifier described above, applied to a smaller, earlier + sample. This tool reproduces that method exactly at full corpus size, so + its output supersedes that one-off run. + - #4351 issue-body method (22,282 exec calls, 07-22 -> 07-24): counted + every exec_command call as a "read" with no bounded-shape filter and no + repeat-of-same-file grouping. It is not reproduced here because it does + not isolate repeat reads of a file already resident in context, which is + the friction signal this tool exists to measure. +""" + +from __future__ import annotations + +import argparse +import json +import re +from collections import defaultdict +from pathlib import Path +from typing import Any + +DEFAULT_LOG_ROOT = Path("~/.local/share/autoskillit/logs/codex-sessions").expanduser() +DEFAULT_SESSIONS_INDEX = Path("~/.local/share/autoskillit/logs/sessions.jsonl").expanduser() +DEFAULT_OUT = Path(".autoskillit/temp/codex_read_repetition_report.json") + +# Leading-command bounded-read shapes. Anchored at ^ so a trailing +# `| head -c N` safety wrapper elsewhere in the command does not qualify it. +_LEADING_SED_N_PAT = re.compile(r"^\s*(?:\{\s*)?sed\s+-n\s+'[^']*'\s+\S+") +_LEADING_HEAD_PAT = re.compile(r"^\s*(?:\{\s*)?head\s+-[cn]\s*\d+\s+\S+") +_LEADING_TAIL_PAT = re.compile(r"^\s*(?:\{\s*)?tail\s+-[cn]\s*\d+\s+\S+") +_LEADING_NL_BA_PAT = re.compile(r"^\s*(?:\{\s*)?nl\s+-ba\s+\S+\s*\|\s*sed\s+-n\b") +_LEADING_RG_N_PAT = re.compile(r"^\s*(?:\{\s*)?rg\s+(?:-\S+\s+)*(-n\b|--line-number\b)") + +_BOUNDED_PATTERNS = ( + _LEADING_SED_N_PAT, + _LEADING_HEAD_PAT, + _LEADING_TAIL_PAT, + _LEADING_NL_BA_PAT, + _LEADING_RG_N_PAT, +) + +_PATH_TOKEN_PAT = re.compile(r"(?:[./~][\w./\-]+|[\w\-]+/[\w./\-]+)") + +# A quoted span (single- or double-) is consumed whole, so a literal `|` inside an +# rg search pattern -- POSIX ERE alternation, this repo's documented idiom for +# `pattern` arguments -- does not truncate the match. Only an unquoted `|`/`;` +# (a real shell pipe/separator) ends the span. Unquantified: callers apply their +# own `*`/`*?` to match the surrounding regex's greediness. +_QUOTED_OR_UNPIPED = r"(?:'[^']*'|\"[^\"]*\"|[^|;])" + +# The rarer custom_tool_call/exec shape embeds the command in a JS-template +# string: tools.exec_command({cmd:"..."}). +_CUSTOM_TOOL_CALL_CMD_PAT = re.compile( + r"tools\.exec_command\(\{\s*cmd\s*:\s*[\"']((?:[^\"'\\]|\\.)*)[\"']" +) + +_VERSION_V2_MARKER = "Context Intake Discipline v2:" +_VERSION_V1_MARKER = "Context Intake Discipline v1:" + +# The YYYY/MM directory pair is month precision only; day precision lives in the +# filename itself, e.g. rollout-2026-05-26T07-30-33-.jsonl. +_ROLLOUT_DATE_PAT = re.compile(r"rollout-(\d{4}-\d{2}-\d{2})T") + + +def is_bounded_read(cmd: str) -> bool: + """Return True when cmd's *leading* command (before any pipe) is a bounded file read.""" + return any(pattern.search(cmd) for pattern in _BOUNDED_PATTERNS) + + +def extract_target_path(cmd: str) -> str | None: + """Best-effort extraction of the file path a bounded-read command targets.""" + m = re.search(r"sed\s+-n\s+'[^']*'\s+([^\s|;]+)", cmd) + if m: + return m.group(1) + m = re.search( + rf"\brg\s+{_QUOTED_OR_UNPIPED}*?(-n\b|--line-number\b){_QUOTED_OR_UNPIPED}*", cmd + ) + if m: + # Strip a trailing `2>&1` (or `2>/dev/null`, `>&2`, ...) redirect token + # before tokenizing -- an unstripped redirect was mis-attributed as a + # path in 8.5% of matches during calibration. + segment = re.sub(r"\d*[<>]&?\d*(/\S+)?\s*$", "", m.group(0)).strip() + tokens = [t for t in segment.split() if not t.startswith("-")] + candidates = [t for t in tokens if t != "rg" and not re.match(r"^\d*[<>]", t)] + if candidates: + return candidates[-1].strip("'\"") + m = re.search(r"\b(?:head|tail)\s+-[cn]\s*\d+\s+([^\s|;]+)", cmd) + if m: + return m.group(1) + m = re.search(r"\bnl\s+-ba\s+([^\s|;]+)", cmd) + if m: + return m.group(1) + tokens = [t for t in _PATH_TOKEN_PAT.findall(cmd) if len(t) > 3] + return max(tokens, key=len) if tokens else None + + +def classify_rollout_records(rollout_path: Path) -> tuple[list[tuple[int, str]], int]: + """Return (exec_commands, unclassified_count) for one rollout JSONL file. + + Malformed JSONL lines are skipped, never fatal. A record that is + exec-shaped (function_call/exec_command or custom_tool_call/exec) but + whose command could not be extracted increments the unclassified count + instead of silently vanishing. + """ + commands: list[tuple[int, str]] = [] + unclassified = 0 + with rollout_path.open("r", encoding="utf-8") as f: + for idx, line in enumerate(f): + line = line.strip() + if not line: + continue + try: + rec = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(rec, dict) or rec.get("type") != "response_item": + continue + payload = rec.get("payload") + if not isinstance(payload, dict): + continue + ptype = payload.get("type") + if ptype == "function_call" and payload.get("name") == "exec_command": + cmd = _extract_function_call_cmd(payload.get("arguments")) + if cmd: + commands.append((idx, cmd)) + else: + unclassified += 1 + elif ptype == "custom_tool_call" and payload.get("name") == "exec": + raw_input = payload.get("input") + match = ( + _CUSTOM_TOOL_CALL_CMD_PAT.search(raw_input) + if isinstance(raw_input, str) + else None + ) + if match: + commands.append((idx, match.group(1))) + else: + unclassified += 1 + return commands, unclassified + + +def _extract_function_call_cmd(raw_args: Any) -> str | None: + if not raw_args: + return None + try: + parsed = json.loads(raw_args) + except (json.JSONDecodeError, TypeError): + return None + return parsed.get("cmd") if isinstance(parsed, dict) else None + + +def classify_policy_cohort(rollout_path: Path) -> str: + """Return 'v2', 'v1', or 'none' from the intake-discipline header seen on the wire. + + An unreadable rollout is treated as 'none' rather than raised -- one bad file + in a batch of thousands must not abort the whole measurement run. + """ + try: + text = rollout_path.read_text(encoding="utf-8", errors="ignore") + except OSError: + return "none" + if _VERSION_V2_MARKER in text: + return "v2" + if _VERSION_V1_MARKER in text: + return "v1" + return "none" + + +def measure_rollout(rollout_path: Path) -> dict[str, Any]: + """Measure one rollout's bounded-read and repeat-read counts.""" + commands, unclassified = classify_rollout_records(rollout_path) + seen: dict[str, int] = defaultdict(int) + bounded = 0 + repeats = 0 + for _idx, cmd in commands: + if not is_bounded_read(cmd): + continue + bounded += 1 + target = extract_target_path(cmd) + if target is None: + continue + seen[target] += 1 + if seen[target] > 1: + repeats += 1 + return { + "session": rollout_path.name, + "cohort": classify_policy_cohort(rollout_path), + "exec_commands": len(commands), + "bounded_reads": bounded, + "repeat_reads": repeats, + "unclassified": unclassified, + "worst_paths": sorted(seen.items(), key=lambda kv: -kv[1])[:5], + } + + +def aggregate_report(rollout_paths: list[Path]) -> dict[str, Any]: + """Aggregate per-rollout measurements into a per-cohort report.""" + cohorts: dict[str, dict[str, Any]] = {} + worst_overall: list[tuple[str, str, int]] = [] + total_unclassified = 0 + for path in rollout_paths: + row = measure_rollout(path) + agg = cohorts.setdefault( + row["cohort"], {"session_count": 0, "bounded_read_count": 0, "repeat_count": 0} + ) + agg["session_count"] += 1 + agg["bounded_read_count"] += row["bounded_reads"] + agg["repeat_count"] += row["repeat_reads"] + total_unclassified += row["unclassified"] + for path_name, count in row["worst_paths"]: + if count > 1: + worst_overall.append((row["session"], path_name, count)) + for cohort_stats in cohorts.values(): + bounded = cohort_stats["bounded_read_count"] + cohort_stats["repeat_read_rate"] = ( + cohort_stats["repeat_count"] / bounded if bounded else None + ) + worst_overall.sort(key=lambda row: -row[2]) + return { + "cohorts": cohorts, + "worst_offenders": worst_overall[:20], + "unclassified_record_count": total_unclassified, + } + + +def _date_within_bound(date: str, bound: str, *, is_lower: bool) -> bool: + """Compare a day-precision date against a since/until bound of either precision. + + A month-precision bound (`YYYY-MM`) is matched against the date's month only, so + every day in that month satisfies it. A day-precision bound (`YYYY-MM-DD`) + compares in full, per the inclusive-boundary contract documented on --since/--until. + """ + if len(bound) == len("YYYY-MM"): + date = date[: len(bound)] + return date >= bound if is_lower else date <= bound + + +def _find_rollouts(log_root: Path, since: str | None, until: str | None) -> list[Path]: + paths = sorted(log_root.glob("*/*/rollout-*.jsonl")) + if since is None and until is None: + return paths + filtered = [] + for p in paths: + m = _ROLLOUT_DATE_PAT.search(p.name) + date = m.group(1) if m else None + if date is not None: + if since is not None and not _date_within_bound(date, since, is_lower=True): + continue + if until is not None and not _date_within_bound(date, until, is_lower=False): + continue + filtered.append(p) + return filtered + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument("--log-root", type=Path, default=DEFAULT_LOG_ROOT) + parser.add_argument("--since", default=None, help="Inclusive date prefix, e.g. 2026-07-18") + parser.add_argument("--until", default=None, help="Inclusive date prefix, e.g. 2026-07-24") + parser.add_argument("--sessions-index", type=Path, default=DEFAULT_SESSIONS_INDEX) + parser.add_argument("--out", type=Path, default=DEFAULT_OUT) + args = parser.parse_args(argv) + + rollouts = _find_rollouts(args.log_root, args.since, args.until) + report = aggregate_report(rollouts) + report["rollouts_scanned"] = len(rollouts) + args.out.parent.mkdir(parents=True, exist_ok=True) + args.out.write_text(json.dumps(report, indent=2)) + print(f"CODEX_READ_REPETITION=PASS rollouts={len(rollouts)} out={args.out}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/autoskillit/core/__init__.pyi b/src/autoskillit/core/__init__.pyi index 053bf3507..0eced8ce9 100644 --- a/src/autoskillit/core/__init__.pyi +++ b/src/autoskillit/core/__init__.pyi @@ -177,9 +177,16 @@ from .types import CODEX_ACTIVE_VIEWS_SUBDIR as CODEX_ACTIVE_VIEWS_SUBDIR from .types import CODEX_ARCHIVED_SESSIONS_SUBDIR as CODEX_ARCHIVED_SESSIONS_SUBDIR from .types import CODEX_CONTEXT_EXHAUSTION_MARKER as CODEX_CONTEXT_EXHAUSTION_MARKER from .types import CODEX_COOK_RESERVED_ENV_VARS as CODEX_COOK_RESERVED_ENV_VARS +from .types import ( + CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET as CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET, +) from .types import CODEX_EFFORT_MAPPING as CODEX_EFFORT_MAPPING +from .types import ( + CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET as CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET, +) from .types import CODEX_INTAKE_DISCIPLINE_DIGEST as CODEX_INTAKE_DISCIPLINE_DIGEST from .types import CODEX_INTAKE_DISCIPLINE_VERSION as CODEX_INTAKE_DISCIPLINE_VERSION +from .types import CODEX_INTAKE_RULES as CODEX_INTAKE_RULES from .types import CODEX_INTERACTIVE_REQUIRED_ENV as CODEX_INTERACTIVE_REQUIRED_ENV from .types import CODEX_MCP_ENV_FORWARD_VARS as CODEX_MCP_ENV_FORWARD_VARS from .types import CODEX_MODEL_ALIASES as CODEX_MODEL_ALIASES @@ -439,6 +446,7 @@ from .types import InputSpecType as InputSpecType from .types import InspectorCallback as InspectorCallback from .types import InspectorEvidence as InspectorEvidence from .types import InspectorVerdict as InspectorVerdict +from .types import IntakeRuleDef as IntakeRuleDef from .types import InvariantDef as InvariantDef from .types import IssueLabelState as IssueLabelState from .types import KillReason as KillReason @@ -619,6 +627,7 @@ from .types import parse_plan_paths as parse_plan_paths from .types import recipe_section_digest as recipe_section_digest from .types import recipe_section_element_digest as recipe_section_element_digest from .types import recipe_section_plan_digest as recipe_section_plan_digest +from .types import render_intake_digest as render_intake_digest from .types import render_target_skill_command as render_target_skill_command from .types import resolve_payload_field as resolve_payload_field from .types import resolve_skill_name as resolve_skill_name diff --git a/src/autoskillit/core/types/AGENTS.md b/src/autoskillit/core/types/AGENTS.md index f874c91d2..1d4b7fc80 100644 --- a/src/autoskillit/core/types/AGENTS.md +++ b/src/autoskillit/core/types/AGENTS.md @@ -37,6 +37,7 @@ Type re-export hub and all typed building blocks for the autoskillit package (IL | `_type_helpers.py` | Text processing, skill-name extraction, and shared content-free validation utilities | | `_type_inspector.py` | Health Inspector types: `InspectorEvidence`, `InspectorVerdict`, `InspectorCallback` (issue #3533) | | `_type_invariant_registry.py` | Invariant registry: `InvariantDef` dataclass and `INVARIANT_REGISTRY` mapping prose prohibitions to runtime gates | +| `_type_intake_policy.py` | Codex context-intake rule registry: `IntakeRuleDef`, `CODEX_INTAKE_RULES`, rendered digest, byte budgets | | `_type_phoropter.py` | Phoropter family/phase types: `PhoropterPrescription`, `ReadingToken`, `READING_TOKEN_PATTERN`, `PhoropterPhaseSkip`, `CrossDomainPrescription`, `CrossDomainAssessment` | | `_type_resume.py` | `ResumeSpec` discriminated union: `NoResume | BareResume | NamedResume` | | `_type_plugin_source.py` | `DirectInstall` (projection input) and `ProjectedPluginRoot` (the sole `PluginSource`) | @@ -45,7 +46,7 @@ Type re-export hub and all typed building blocks for the autoskillit package (IL ## Architecture Notes -Internal dependency DAG: enums -> constants_registries -> constants_features; enums -> results -> protocols -> helpers; enums -> phoropter; enums + phoropter -> tradition_manifest. All modules have zero `autoskillit` imports outside this sub-package (IL-0 hard constraint). Production code imports from `autoskillit.core`, not from this package directly. +Internal dependency DAG: enums -> constants_registries -> constants_features; enums -> results -> protocols -> helpers; enums -> phoropter; enums + phoropter -> tradition_manifest. `_type_intake_policy` is a DAG leaf — stdlib-only, zero sibling imports. All modules have zero `autoskillit` imports outside this sub-package (IL-0 hard constraint). Production code imports from `autoskillit.core`, not from this package directly. ## Extension Bundle Pattern diff --git a/src/autoskillit/core/types/__init__.py b/src/autoskillit/core/types/__init__.py index e97ee409a..61a051ca7 100644 --- a/src/autoskillit/core/types/__init__.py +++ b/src/autoskillit/core/types/__init__.py @@ -36,6 +36,8 @@ from ._type_helpers import __all__ as _helpers_all from ._type_inspector import * # noqa: F401, F403 from ._type_inspector import __all__ as _inspector_all +from ._type_intake_policy import * # noqa: F401, F403 +from ._type_intake_policy import __all__ as _intake_policy_all from ._type_invariant_registry import * # noqa: F401, F403 from ._type_invariant_registry import __all__ as _invariant_registry_all from ._type_phoropter import * # noqa: F401, F403 @@ -93,6 +95,7 @@ + _figure_spec_all + _helpers_all + _inspector_all + + _intake_policy_all + _invariant_registry_all + _phoropter_all + _plugin_source_all diff --git a/src/autoskillit/core/types/_type_constants.py b/src/autoskillit/core/types/_type_constants.py index 536b1dd29..8f0dc719b 100644 --- a/src/autoskillit/core/types/_type_constants.py +++ b/src/autoskillit/core/types/_type_constants.py @@ -19,8 +19,6 @@ "OUTPUT_DISCIPLINE_COMBINED_SHA256", "OUTPUT_DISCIPLINE_DIGEST", "OUTPUT_DISCIPLINE_REQUIRED_SKILLS", - "CODEX_INTAKE_DISCIPLINE_VERSION", - "CODEX_INTAKE_DISCIPLINE_DIGEST", "RETIRED_SKILL_NAMES", "RETIRED_AGENT_NAMES", "RETIRED_INSTALL_ARTIFACT_SHAPES", @@ -165,37 +163,6 @@ {"investigate", "rectify", "audit-bugs", "audit-friction"} ) -CODEX_INTAKE_DISCIPLINE_VERSION: int = 1 - -CODEX_INTAKE_DISCIPLINE_DIGEST = "\n".join( - ( - "Context Intake Discipline v1:", - ( - "- Read at most 2 files per exec command; never chain whole-file dumps " - "(`cat`/`sed`/`nl`) across a list of files." - ), - ( - "- Never read a file end-to-end. Use `rg -n` with context flags, or " - "`sed -n` ranges of at most 250 lines." - ), - "- Never pass max_output_tokens above 10000.", - ( - "- After listing a directory, open at most 2 of the listed files before " - "deciding what to read next." - ), - ( - "- Package tables in AGENTS.md files are an index, not required reading; " - "consult a per-package AGENTS.md only for packages you are modifying." - ), - ( - '- Spawn sub-agents with fresh context: pass fork_turns "none" explicitly ' - '(omitting fork_turns silently defaults to "all", forking the full parent ' - "conversation). Give each sub-agent an explicit narrow brief; sub-agents " - "return a summary, not raw file contents." - ), - ) -) - RETIRED_SKILL_NAMES: frozenset[str] = frozenset( { # Skill directory names that have been renamed or removed. diff --git a/src/autoskillit/core/types/_type_intake_policy.py b/src/autoskillit/core/types/_type_intake_policy.py new file mode 100644 index 000000000..8c3f628c8 --- /dev/null +++ b/src/autoskillit/core/types/_type_intake_policy.py @@ -0,0 +1,167 @@ +"""Codex context-intake rule registry — evidence-bound Codex instruction-reading policy. + +Zero autoskillit imports. +""" + +from __future__ import annotations + +from typing import Final, Literal, NamedTuple + +__all__ = [ + "IntakeRuleDef", + "CODEX_INTAKE_RULES", + "CODEX_INTAKE_DISCIPLINE_VERSION", + "CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET", + "CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET", + "render_intake_digest", + "CODEX_INTAKE_DISCIPLINE_DIGEST", +] + +CODEX_INTAKE_DISCIPLINE_VERSION: Final[int] = 2 + +# Always-on injection: every byte is replicated into 11 bundled agent TOMLs at session +# setup plus one copy per session prompt across 5 delivery surfaces. Raising either +# ceiling is a decision, not a consequence — record measured before/after in the PR. +CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET: Final[int] = 1200 +CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET: Final[int] = 3000 + + +class IntakeRuleDef(NamedTuple): + """One context-intake rule injected into Codex sessions. + + ``basis``/``evidence``/``evidence_anchor`` are what make the rule mergeable: a + guard resolves the evidence and fails when it does not exist or no longer says + what the rule claims. ``exception`` is mandatory whenever the text states an + absolute — an unqualified imperative is the failure mode behind #4351, #4265 and + #4373. ``path_classes`` names the file classes the rule directs the agent to read, + so a guard can prove none of them is in the harness's denied set. + """ + + id: str + subject: str + text: str + basis: Literal["backend-capability", "adr", "upstream-aligned", "local-policy"] + evidence: str + evidence_anchor: str + exception: str + path_classes: tuple[str, ...] + + +CODEX_INTAKE_RULES: Final[tuple[IntakeRuleDef, ...]] = ( + IntakeRuleDef( + id="instruction-file-completeness", + subject="instruction-file-intake", + text=( + "Instruction files you are about to act on — the SKILL.md you selected and " + "any plan file your task names — must be read completely before you act on " + "them; if a read is truncated or paginated, continue until EOF." + ), + basis="upstream-aligned", + evidence="openai/codex#27044", + evidence_anchor="must be read completely", + exception=( + "Recipe YAML and bundled source-tree SKILL.md paths are never read from disk " + "at all — recipes arrive via load_recipe / get_recipe_section and skills via " + "the Skill tool." + ), + path_classes=("session-skill-md", "plan-file"), + ), + IntakeRuleDef( + id="data-file-bounded-read", + subject="data-file-intake", + text=( + "For data, log, and source files, locate the region first with `rg -n` and " + "read only that region; keep each `sed -n` range to at most 250 lines." + ), + basis="local-policy", + evidence="#4280", + evidence_anchor="at most 250 lines", + exception=( + "Instruction files covered by the completeness rule above are exempt from this bound." + ), + path_classes=("data-file",), + ), + IntakeRuleDef( + id="outer-result-token-ceiling", + subject="tool-result-budget", + text="Never pass max_output_tokens above 10000.", + basis="backend-capability", + evidence="codex.unnegotiated_tool_result_token_limit", + evidence_anchor="max_output_tokens above 10000", + exception=( + "The single attested recipe-delivery call named in the recipe delivery " + "calling contract is the only exemption." + ), + path_classes=(), + ), + IntakeRuleDef( + id="agents-md-package-table", + subject="package-table-orientation", + text=( + "Package tables in AGENTS.md files are an index, not required reading; " + "consult a per-package AGENTS.md only for packages you are modifying — " + "except `src/autoskillit/agents/`, whose definitions reach you as configured " + "sub-agents and are not read from disk." + ), + basis="local-policy", + evidence="AGENTS.md", + evidence_anchor="an index, not required reading", + exception="Read a package's own AGENTS.md when you are modifying that package.", + path_classes=("agents-md",), + ), + IntakeRuleDef( + id="subagent-fresh-context", + subject="subagent-spawning", + text=( + 'Spawn sub-agents with fresh context: pass fork_turns "none" explicitly ' + '(omitting fork_turns silently defaults to "all", forking the full parent ' + "conversation). Give each sub-agent an explicit narrow brief; sub-agents " + "return a summary of their own task work, not raw file contents, and never " + "read or interpret your instruction files on your behalf." + ), + basis="adr", + evidence="docs/decisions/0005-output-budget-protocol.md", + evidence_anchor='fork_turns "none"', + exception="Sub-agents may still perform task work when the selected skill allows it.", + path_classes=(), + ), +) + + +def render_intake_digest( + rules: tuple[IntakeRuleDef, ...] = CODEX_INTAKE_RULES, + version: int = CODEX_INTAKE_DISCIPLINE_VERSION, +) -> str: + """Render the injected wire text from the rule registry.""" + return "\n".join( + (f"Context Intake Discipline v{version}:", *(f"- {rule.text}" for rule in rules)) + ) + + +CODEX_INTAKE_DISCIPLINE_DIGEST: Final[str] = render_intake_digest() + +_RULE_IDS = [rule.id for rule in CODEX_INTAKE_RULES] +if len(_RULE_IDS) != len(set(_RULE_IDS)): + raise AssertionError(f"CODEX_INTAKE_RULES ids must be unique: {_RULE_IDS}") +del _RULE_IDS + +_ANCHOR_NOT_IN_TEXT = [ + rule.id for rule in CODEX_INTAKE_RULES if rule.evidence_anchor not in rule.text +] +if _ANCHOR_NOT_IN_TEXT: + raise AssertionError( + "evidence_anchor must be a literal substring of the rule's own text: " + f"{_ANCHOR_NOT_IN_TEXT}" + ) +del _ANCHOR_NOT_IN_TEXT + +if "'''" in CODEX_INTAKE_DISCIPLINE_DIGEST: + raise AssertionError("CODEX_INTAKE_DISCIPLINE_DIGEST must not contain triple single-quotes") + +_DIGEST_BYTES = len(CODEX_INTAKE_DISCIPLINE_DIGEST.encode("utf-8")) +if _DIGEST_BYTES > CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET: + raise AssertionError( + f"CODEX_INTAKE_DISCIPLINE_DIGEST is {_DIGEST_BYTES} bytes, exceeding the " + f"{CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET}-byte budget" + ) +del _DIGEST_BYTES diff --git a/src/autoskillit/execution/backends/_claude_prompt.py b/src/autoskillit/execution/backends/_claude_prompt.py index dd2e978b0..1b390e13d 100644 --- a/src/autoskillit/execution/backends/_claude_prompt.py +++ b/src/autoskillit/execution/backends/_claude_prompt.py @@ -187,6 +187,61 @@ def _inject_narration_suppression(skill_command: str, *, has_skill_prefix: bool return skill_command + directive +class CoInjectedPolicyDef(NamedTuple): + """One policy text co-injected into a Codex session by codex_discipline_suffix(). + + ``subjects`` must be disjoint across every entry: two texts making claims about the + same subject is the shape that produced #4351, where one digest said "directly read + only known-small files" and the next said "Never read a file end-to-end". + ``scope_marker`` must appear in the text, so a text cannot silently widen past the + subject it declares. + + Not to be confused with ``InjectorDef`` below, which names a *stage of the + prompt-injection chain* ("intake-discipline"). This names a *constant whose text + is injected* ("CODEX_INTAKE_DISCIPLINE_DIGEST"). The field is ``constant_name``, not + ``name``, precisely so the two cannot be mistaken at a call site. + """ + + constant_name: str + subjects: frozenset[str] + scope_marker: str + + +CODEX_CO_INJECTED_POLICIES: tuple[CoInjectedPolicyDef, ...] = ( + CoInjectedPolicyDef( + constant_name="OUTPUT_DISCIPLINE_DIGEST", + subjects=frozenset({"producer-byte-bounding", "saved-artifact-inspection"}), + scope_marker="saved output", + ), + CoInjectedPolicyDef( + constant_name="CODEX_INTAKE_DISCIPLINE_DIGEST", + subjects=frozenset( + { + "instruction-file-intake", + "data-file-intake", + "tool-result-budget", + "package-table-orientation", + "subagent-spawning", + } + ), + scope_marker="Instruction files", + ), + CoInjectedPolicyDef( + constant_name="CODEX_RECIPE_DELIVERY_CALLING_CONTRACT", + subjects=frozenset({"recipe-delivery-attestation"}), + scope_marker="delivery_request", + ), +) + +# Explicit lookup, not globals()/getattr: a constant renamed out from under the matrix +# becomes an import error at module load, not a silently missing key at test time. +_CO_INJECTED_POLICY_TEXTS: dict[str, str] = { + "OUTPUT_DISCIPLINE_DIGEST": OUTPUT_DISCIPLINE_DIGEST, + "CODEX_INTAKE_DISCIPLINE_DIGEST": CODEX_INTAKE_DISCIPLINE_DIGEST, + "CODEX_RECIPE_DELIVERY_CALLING_CONTRACT": CODEX_RECIPE_DELIVERY_CALLING_CONTRACT, +} + + def codex_discipline_suffix() -> str: """Canonical combined discipline suffix: output-discipline + intake-discipline.""" return ( diff --git a/src/autoskillit/skills_extended/resolve-review/SKILL.md b/src/autoskillit/skills_extended/resolve-review/SKILL.md index c8987c827..680670265 100644 --- a/src/autoskillit/skills_extended/resolve-review/SKILL.md +++ b/src/autoskillit/skills_extended/resolve-review/SKILL.md @@ -512,6 +512,7 @@ classified `REJECT` with `category: "arch_violation"`. | Issue URL extraction guard | `test_issue_url_extraction_guard.py` | Raw `.get("issue_url")` / `.get("issue_urls")` in `fleet/` outside `_issue_url_helpers.py` and `state_types.py` — must use `extract_issue_urls()`; `fleet_claim_guard.py` must retain both key variants | | Pipeline ordering | `test_pipeline_ordering.py` | Moving `run_semantic_rules` before `_prune_skipped_steps` in `load_and_validate` — semantic rules must run on post-prune recipe | | Hook env-var authority | `test_hook_env_var_authority.py` | Hook scripts that read `AUTOSKILLIT_PROVIDER_PROFILE` without also reading `AUTOSKILLIT_AGENT_BACKEND` — provider profile is a credentials label, not a backend-identity signal | +| Evidence-bound intake rules | `test_intake_rule_registry.py` | `CODEX_INTAKE_RULES` entries stating an absolute imperative without a declared `exception`; `basis`/`evidence`/`evidence_anchor` that does not resolve to a live backend capability, ADR, or issue; a `path_classes` entry naming a file class `recipe_read_guard` denies | When a reviewer suggestion would cause a change matching any row above, classify the finding as `REJECT` with `category: "arch_violation"` and `evidence` referencing diff --git a/tests/_test_filter.py b/tests/_test_filter.py index f84ca1675..53ae02c52 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -208,6 +208,7 @@ class ImportContext(enum.StrEnum): {"_llm_triage", "cli", "core", "execution", "fleet", "pipeline", "server", "workspace"} ), "_type_inspector": frozenset({"core", "execution"}), + "_type_intake_policy": frozenset({"core", "execution"}), "_type_invariant_registry": frozenset({"core"}), "_install_detect": frozenset({"core", "cli", "config"}), "_linux_proc": frozenset({"core", "execution", "fleet", "cli"}), diff --git a/tests/arch/AGENTS.md b/tests/arch/AGENTS.md index d130d3821..afb6ad96b 100644 --- a/tests/arch/AGENTS.md +++ b/tests/arch/AGENTS.md @@ -121,6 +121,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests. | `test_subagent_filter_guard.py` | AST guard: all assistant-record NDJSON processing sites must use _is_parent_assistant_record or _is_parent_assistant predicate | | `test_swap_labels_guard.py` | AST guard: direct swap_labels calls in fleet/ must go through cleanup_orphaned_labels | | `test_invariant_registry_coverage.py` | Meta-test: every InvariantDef has resolvable gate_target, non-empty fields, and source_doc containing prohibition prose | +| `test_intake_rule_registry.py` | Structural guards for the Codex intake-rule registry (#4351): evidence-bound basis resolution, mandatory exceptions for absolute imperatives, and no rule naming a harness-denied path class | | `test_issue_url_extraction_guard.py` | AST guard: raw `.get('issue_url')` / `.get('issue_urls')` banned in fleet/; dual-key asserted in fleet_claim_guard.py | | `test_enqueue_ready_type_enforcement.py` | AST guard: mutation methods (_enqueue_direct, _enable_auto_merge_direct) must accept EnqueueReady, not str | | `test_origin_isolation_contract.py` | AST + shell lint guard: no hardcoded "origin" in git remote operations outside allowlist; shell scripts must try upstream before origin | diff --git a/tests/arch/test_intake_rule_registry.py b/tests/arch/test_intake_rule_registry.py new file mode 100644 index 000000000..d23f32998 --- /dev/null +++ b/tests/arch/test_intake_rule_registry.py @@ -0,0 +1,214 @@ +"""Structural guards for the Codex intake-rule registry (#4351). + +Every injected instruction rule must be evidence-bound, exception-qualified, and +must not name a path class the AutoSkillit harness denies. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from autoskillit.core import ( + CODEX_INTAKE_DISCIPLINE_DIGEST, + CODEX_INTAKE_DISCIPLINE_VERSION, + CODEX_INTAKE_RULES, + render_intake_digest, +) +from autoskillit.execution.backends._claude_prompt import CODEX_CO_INJECTED_POLICIES +from autoskillit.hooks._command_classification import command_has_blocked_protected_path_read +from autoskillit.hooks.guards.recipe_read_guard import _CMD_PATH_PATTERNS + +pytestmark = [pytest.mark.layer("arch"), pytest.mark.small] + +_ABSOLUTE_RE = re.compile( + r"(? list[str]: + found = [_PROJECT_ROOT / "AGENTS.md", *_SRC_ROOT.rglob("AGENTS.md")] + return sorted( + p.relative_to(_PROJECT_ROOT).as_posix() + for p in found + if p.is_file() and "__pycache__" not in str(p) + ) + + +def test_rule_ids_are_unique_kebab_case() -> None: + ids = [rule.id for rule in CODEX_INTAKE_RULES] + assert len(ids) == len(set(ids)), f"Duplicate rule ids: {ids}" + bad = [i for i in ids if not re.fullmatch(r"^[a-z][a-z0-9]*(-[a-z0-9]+)*$", i)] + assert not bad, f"Rule ids must be kebab-case: {bad}" + + +def test_no_absolute_imperative_without_a_declared_exception() -> None: + for rule in CODEX_INTAKE_RULES: + if _ABSOLUTE_RE.search(rule.text): + assert len(rule.exception.strip()) >= 20, ( + f"Rule {rule.id!r} states an absolute but has no declared exception" + ) + + +def test_every_evidence_anchor_is_a_fragment_of_its_own_rule() -> None: + for rule in CODEX_INTAKE_RULES: + assert len(rule.evidence_anchor) >= _MIN_ANCHOR_LEN, ( + f"Rule {rule.id!r} evidence_anchor is shorter than {_MIN_ANCHOR_LEN} chars" + ) + assert rule.evidence_anchor in rule.text, ( + f"Rule {rule.id!r} evidence_anchor is not a substring of its own text" + ) + + +@pytest.mark.medium +def test_backend_capability_basis_matches_live_value() -> None: + from autoskillit.execution.backends import BACKEND_REGISTRY + + for rule in CODEX_INTAKE_RULES: + if rule.basis != "backend-capability": + continue + backend_name, _, field_name = rule.evidence.partition(".") + assert backend_name and field_name, ( + f"Rule {rule.id!r} evidence must be '.': {rule.evidence!r}" + ) + capabilities = BACKEND_REGISTRY[backend_name]().capabilities + value = getattr(capabilities, field_name) + assert value, f"Rule {rule.id!r} evidence field {field_name!r} resolved falsy" + assert str(value) in rule.text, ( + f"Rule {rule.id!r} text does not contain the live capability value {value!r}" + ) + + +def test_doc_basis_resolves_and_anchor_appears_in_the_cited_doc() -> None: + for rule in CODEX_INTAKE_RULES: + if rule.basis not in {"adr", "local-policy"}: + continue + if re.fullmatch(r"^#\d+$", rule.evidence): + continue + doc_path = _PROJECT_ROOT / rule.evidence + assert doc_path.is_file(), f"Rule {rule.id!r} evidence doc does not exist: {rule.evidence}" + doc_text = doc_path.read_text(encoding="utf-8") + assert rule.evidence_anchor.lower() in doc_text.lower(), ( + f"Rule {rule.id!r} evidence_anchor {rule.evidence_anchor!r} not found in " + f"{rule.evidence}" + ) + + +def test_issue_basis_cites_a_plausible_issue_number() -> None: + for rule in CODEX_INTAKE_RULES: + if rule.basis != "local-policy" or not re.fullmatch(r"^#\d+$", rule.evidence): + continue + assert int(rule.evidence.lstrip("#")) >= 1, ( + f"Rule {rule.id!r} evidence issue number must be >= 1: {rule.evidence}" + ) + + +def test_upstream_aligned_basis_carries_a_citation() -> None: + for rule in CODEX_INTAKE_RULES: + if rule.basis != "upstream-aligned": + continue + assert re.fullmatch(r"openai/codex#\d+", rule.evidence) or re.match( + r"codex-rs/\S+", rule.evidence + ), ( + f"Rule {rule.id!r} upstream-aligned evidence must cite an issue or file: " + f"{rule.evidence}" + ) + + +def test_no_rule_names_a_path_class_the_harness_denies() -> None: + """A False result means the path class is not in the harness's denied set — + + not that the harness has affirmatively authorised the read. That narrower claim + is the one that matters: it is exactly what would have blocked a recipe-yaml + carve-out. + """ + seen_agents_md = False + for rule in CODEX_INTAKE_RULES: + for path_class in rule.path_classes: + if path_class == "agents-md": + probe_paths = _agents_md_probe_paths() + seen_agents_md = True + else: + probe_paths = list(_PATH_CLASS_PROBE_PATHS[path_class]) + for probe_path in probe_paths: + cmd = _PROBE_TEMPLATE.format(path=probe_path) + is_blocked = command_has_blocked_protected_path_read(cmd, _CMD_PATH_PATTERNS) + if probe_path in KNOWN_BLOCKED_AGENTS_MD: + assert is_blocked, ( + f"{probe_path} is a declared exception but is no longer blocked — " + "remove it from KNOWN_BLOCKED_AGENTS_MD" + ) + else: + assert not is_blocked, ( + f"Rule {rule.id!r} path_class {path_class!r} probe {probe_path} is " + "blocked by the harness and has no declared exception" + ) + + if seen_agents_md: + for blocked_path in KNOWN_BLOCKED_AGENTS_MD: + cmd = _PROBE_TEMPLATE.format(path=blocked_path) + assert command_has_blocked_protected_path_read(cmd, _CMD_PATH_PATTERNS), ( + f"{blocked_path} must still be blocked by the harness" + ) + + for control_path in _BLOCKED_CONTROLS: + cmd = _PROBE_TEMPLATE.format(path=control_path) + assert command_has_blocked_protected_path_read(cmd, _CMD_PATH_PATTERNS), ( + f"Positive control {control_path} must be blocked — the predicate may be dead" + ) + + +def test_rendered_digest_equals_registry_render() -> None: + assert CODEX_INTAKE_DISCIPLINE_DIGEST == render_intake_digest( + CODEX_INTAKE_RULES, CODEX_INTAKE_DISCIPLINE_VERSION + ) + + +def test_digest_header_carries_the_version() -> None: + assert CODEX_INTAKE_DISCIPLINE_DIGEST.startswith( + f"Context Intake Discipline v{CODEX_INTAKE_DISCIPLINE_VERSION}:" + ) + + +def test_every_rule_subject_is_declared_for_the_intake_digest() -> None: + rule_subjects = {rule.subject for rule in CODEX_INTAKE_RULES} + (matrix_entry,) = ( + entry + for entry in CODEX_CO_INJECTED_POLICIES + if entry.constant_name == "CODEX_INTAKE_DISCIPLINE_DIGEST" + ) + assert rule_subjects == set(matrix_entry.subjects) diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 6b8c87a32..7fc624282 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -94,6 +94,10 @@ def _get_call_func_name(node: ast.Call) -> str | None: # Canonical output-discipline block/digest and their SHA-256 cache identity are # deliberately derived once at import time from the single source of truth. "_type_constants", + # CODEX_INTAKE_DISCIPLINE_DIGEST is rendered once from CODEX_INTAKE_RULES at + # import time, and its byte length is checked against the budget in the same + # module-load self-check block (#4351). + "_type_intake_policy", "_type_constants_registries", # measured response-exemption registry digest "_codex_config", # Codex output ceiling derived from measured exemptions "_fmt_response_spill", # standalone spill schema and exemption mirror digests @@ -896,7 +900,7 @@ def test_no_subpackage_exceeds_10_files() -> None: "recipe": 42, # was 33; +9 from CI/graph/dataflow splits "execution": 18, "core": 26, # +_context_admission pure reducer - "core/types": 36, # +_type_recipe_sections +_type_skill_contract +context admission + "core/types": 37, # +_type_intake_policy evidence-bound Codex intake rule registry "cli": 21, "cli/doctor": 11, # +_doctor_skills capability declaration authenticity checks "workspace": 14, # +_install_state (single install-state consistency authority, diff --git a/tests/arch/test_subpackage_structure.py b/tests/arch/test_subpackage_structure.py index 8533cfa56..ba91a60ef 100644 --- a/tests/arch/test_subpackage_structure.py +++ b/tests/arch/test_subpackage_structure.py @@ -30,6 +30,7 @@ def test_core_types_has_all_type_modules(self): "_type_capture", "_type_helpers", "_type_inspector", + "_type_intake_policy", "_type_invariant_registry", "_type_phoropter", "_type_plugin_source", @@ -65,8 +66,8 @@ def test_type_constants_split_completeness(self): assert len(combined) == len(remaining) + len(env) + len(features) + len(registries), ( "Duplicate symbols across split modules" ) - assert len(combined) == 132, ( - f"Expected 132 symbols total, got {len(combined)} " + assert len(combined) == 130, ( + f"Expected 130 symbols total, got {len(combined)} " f"(remaining={len(remaining)}, env={len(env)}, " f"features={len(features)}, registries={len(registries)})" ) diff --git a/tests/contracts/AGENTS.md b/tests/contracts/AGENTS.md index 12e06eaa9..7ddcbc492 100644 --- a/tests/contracts/AGENTS.md +++ b/tests/contracts/AGENTS.md @@ -53,6 +53,7 @@ Protocol satisfaction, package gateway, and skill contract compliance tests. | `test_implement_experiment_contracts.py` | Contract tests for implement-experiment SKILL.md — test infrastructure requirements | | `test_input_type_semantic_correctness.py` | Cross-validate skill_contracts.yaml path input types against SKILL.md content | | `test_install_state_consistency.py` | `verify_install_state()` invariants, the doctor checks that consume it, and the retired-artifact-shape registry double bind | +| `test_injected_policy_consistency.py` | Contracts on the relationship between co-injected Codex policy texts (#4351): disjoint subjects, honest scope markers, byte budgets on the intake digest and composed suffix | | `test_instruction_surface.py` | Contracts for the authoritative instruction surfaces that own pipeline native-tool restrictions and prohibition framing | | `test_issue_body_discipline.py` | Cross-skill contract: no SKILL.md may append validation summaries to issue bodies | | `test_issue_content_fidelity.py` | Cross-skill contract: content fidelity for issue body assembly | diff --git a/tests/contracts/test_discipline_delivery_matrix.py b/tests/contracts/test_discipline_delivery_matrix.py index ff8e5cee2..fd42ebb8b 100644 --- a/tests/contracts/test_discipline_delivery_matrix.py +++ b/tests/contracts/test_discipline_delivery_matrix.py @@ -6,19 +6,22 @@ from __future__ import annotations +import tomllib from pathlib import Path import pytest from autoskillit.core import ( + CODEX_INTAKE_DISCIPLINE_DIGEST, SESSION_TYPE_ENV_VAR, SESSION_TYPE_FLEET, SESSION_TYPE_ORCHESTRATOR, SESSION_TYPE_SKILL, ProjectedPluginRoot, ) +from autoskillit.core.paths import pkg_root from autoskillit.execution.backends.claude import ClaudeCodeBackend -from autoskillit.execution.backends.codex import CodexBackend # noqa: F401 +from autoskillit.execution.backends.codex import CodexBackend, _generate_agent_tomls pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] @@ -31,6 +34,28 @@ def _assert_interactive_primary_channel(backend, spec) -> None: assert any("developer_instructions=" in arg for arg in spec.cmd) +def _assert_interactive_intake_digest(backend, spec) -> None: + """Assert the intake digest is present for Codex, absent for Claude. + + The interactive channel delivers the digest inside a `-c developer_instructions=...` + TOML config-override, which escapes newlines to literal `\\n` sequences — the digest's + single-line header survives that escaping unchanged, so it is the anchor here. + """ + header = CODEX_INTAKE_DISCIPLINE_DIGEST.splitlines()[0] + if isinstance(backend, ClaudeCodeBackend): + assert not any(header in arg for arg in spec.cmd) + else: + assert any("developer_instructions=" in arg and header in arg for arg in spec.cmd) + + +def _assert_headless_intake_digest(backend, spec) -> None: + """Assert the intake digest is present in the final prompt arg for Codex, absent for Claude.""" + if isinstance(backend, ClaudeCodeBackend): + assert CODEX_INTAKE_DISCIPLINE_DIGEST not in spec.cmd[-1] + else: + assert CODEX_INTAKE_DISCIPLINE_DIGEST in spec.cmd[-1] + + class TestFleetInteractive: @pytest.mark.parametrize( "backend", @@ -56,6 +81,18 @@ def test_session_type_fleet_in_env(self, backend) -> None: ) assert spec.env.get(SESSION_TYPE_ENV_VAR) == SESSION_TYPE_FLEET + @pytest.mark.parametrize( + "backend", + [ClaudeCodeBackend(), CodexBackend()], + ids=["claude-code", "codex"], + ) + def test_intake_digest_delivery(self, backend) -> None: + spec = backend.build_interactive_cmd( + system_prompt="Fleet discipline prompt", + env_extras={SESSION_TYPE_ENV_VAR: SESSION_TYPE_FLEET}, + ) + _assert_interactive_intake_digest(backend, spec) + class TestOrchestratorInteractive: @pytest.mark.parametrize( @@ -80,6 +117,17 @@ def test_no_session_type_assertion(self, backend) -> None: ) assert not spec.env.get(SESSION_TYPE_ENV_VAR, "") + @pytest.mark.parametrize( + "backend", + [ClaudeCodeBackend(), CodexBackend()], + ids=["claude-code", "codex"], + ) + def test_intake_digest_delivery(self, backend) -> None: + spec = backend.build_interactive_cmd( + system_prompt="Orchestrator discipline prompt", + ) + _assert_interactive_intake_digest(backend, spec) + class TestOrchestratorHeadless: @pytest.mark.parametrize( @@ -111,6 +159,20 @@ def test_session_type_orchestrator_in_env(self, backend) -> None: ) assert spec.env.get(SESSION_TYPE_ENV_VAR) == SESSION_TYPE_ORCHESTRATOR + @pytest.mark.parametrize( + "backend", + [ClaudeCodeBackend(), CodexBackend()], + ids=["claude-code", "codex"], + ) + def test_intake_digest_delivery(self, backend) -> None: + spec = backend.build_food_truck_cmd( + orchestrator_prompt="Run the pipeline", + plugin_source=ProjectedPluginRoot(plugin_dir=Path("/tmp")), + cwd="/tmp", + completion_marker="%%DONE%%", + ) + _assert_headless_intake_digest(backend, spec) + class TestSkillSession: @pytest.mark.parametrize( @@ -131,6 +193,63 @@ def test_session_type_skill_in_env(self, backend) -> None: spec = backend.build_skill_session_cmd("/investigate foo", "/tmp") assert spec.env.get(SESSION_TYPE_ENV_VAR) == SESSION_TYPE_SKILL + @pytest.mark.parametrize( + "backend", + [ClaudeCodeBackend(), CodexBackend()], + ids=["claude-code", "codex"], + ) + def test_intake_digest_delivery(self, backend) -> None: + spec = backend.build_skill_session_cmd("/investigate foo", "/tmp") + _assert_headless_intake_digest(backend, spec) + + +class TestResumeDelivery: + @pytest.mark.parametrize( + "backend", + [ClaudeCodeBackend(), CodexBackend()], + ids=["claude-code", "codex"], + ) + def test_intake_digest_delivery(self, backend) -> None: + spec = backend.build_resume_cmd( + resume_session_id="abc123", + prompt="CALLER PROMPT MARKER", + ) + _assert_headless_intake_digest(backend, spec) + + def test_intake_digest_is_prepended_before_the_caller_prompt_for_codex(self) -> None: + backend = CodexBackend() + spec = backend.build_resume_cmd( + resume_session_id="abc123", + prompt="CALLER PROMPT MARKER", + ) + assert spec.cmd[-1].index(CODEX_INTAKE_DISCIPLINE_DIGEST) < spec.cmd[-1].index( + "CALLER PROMPT MARKER" + ) + + +class TestAgentTomlDelivery: + @pytest.mark.medium + def test_every_bundled_agent_toml_carries_the_composed_suffix(self, tmp_path) -> None: + from autoskillit.execution.backends._claude_prompt import codex_discipline_suffix + + agents_src = pkg_root() / "agents" + expected_count = sum( + 1 + for md_path in agents_src.glob("*.md") + if md_path.name not in ("AGENTS.md", "CLAUDE.md") + ) + + count = _generate_agent_tomls(tmp_path) + + assert count == expected_count + toml_files = sorted((tmp_path / "agents").glob("*.toml")) + assert len(toml_files) == expected_count + suffix = codex_discipline_suffix() + for toml_path in toml_files: + parsed = tomllib.loads(toml_path.read_text(encoding="utf-8")) + # TOML keeps the trailing newline before the closing ''' delimiter. + assert parsed["developer_instructions"].endswith(f"{suffix}\n") + class TestSousChefDelivery: def test_sous_chef_in_orchestrator_prompt(self) -> None: diff --git a/tests/contracts/test_injected_policy_consistency.py b/tests/contracts/test_injected_policy_consistency.py new file mode 100644 index 000000000..5e7078c11 --- /dev/null +++ b/tests/contracts/test_injected_policy_consistency.py @@ -0,0 +1,78 @@ +"""Contracts on the relationship between co-injected Codex policy texts (#4351). + +Every existing guard asserts a property of one constant in isolation. These tests +assert properties of the *relationship* between the co-injected texts: disjoint +subjects, an honest scope marker, and a byte budget on both the intake digest and +the composed suffix. +""" + +from __future__ import annotations + +import itertools +import tomllib + +import pytest + +from autoskillit.core import ( + CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET, + CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET, + CODEX_INTAKE_DISCIPLINE_DIGEST, +) +from autoskillit.execution.backends._claude_prompt import ( + _CO_INJECTED_POLICY_TEXTS, + CODEX_CO_INJECTED_POLICIES, + codex_discipline_suffix, +) + +pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] + + +def test_composed_suffix_contains_no_triple_quote() -> None: + assert "'''" not in codex_discipline_suffix() + + +def test_composed_suffix_round_trips_through_agent_toml_literal() -> None: + suffix = codex_discipline_suffix() + literal = f"developer_instructions = '''\n{suffix}\n'''\n" + parsed = tomllib.loads(literal) + # TOML trims a newline immediately following the opening ''' delimiter but + # keeps the trailing one — this must match what _generate_agent_tomls writes. + assert parsed["developer_instructions"] == f"{suffix}\n" + + +def test_intake_digest_within_byte_budget() -> None: + assert ( + len(CODEX_INTAKE_DISCIPLINE_DIGEST.encode("utf-8")) <= CODEX_INTAKE_DISCIPLINE_BYTE_BUDGET + ) + + +def test_composed_suffix_within_byte_budget() -> None: + assert len(codex_discipline_suffix().encode("utf-8")) <= CODEX_DISCIPLINE_SUFFIX_BYTE_BUDGET + + +def test_co_injected_policies_claim_disjoint_subjects() -> None: + for entry_a, entry_b in itertools.combinations(CODEX_CO_INJECTED_POLICIES, 2): + overlap = entry_a.subjects & entry_b.subjects + assert overlap == frozenset(), ( + f"{entry_a.constant_name!r} and {entry_b.constant_name!r} claim overlapping " + f"subjects: {overlap}" + ) + + +def test_every_co_injected_text_evidences_its_declared_scope() -> None: + for entry in CODEX_CO_INJECTED_POLICIES: + live_text = _CO_INJECTED_POLICY_TEXTS[entry.constant_name] + assert entry.scope_marker in live_text, ( + f"{entry.constant_name!r} scope_marker {entry.scope_marker!r} is not present in " + "the live constant text" + ) + + +def test_subject_matrix_covers_every_co_injected_text() -> None: + expected = { + "OUTPUT_DISCIPLINE_DIGEST", + "CODEX_INTAKE_DISCIPLINE_DIGEST", + "CODEX_RECIPE_DELIVERY_CALLING_CONTRACT", + } + assert {entry.constant_name for entry in CODEX_CO_INJECTED_POLICIES} == expected + assert set(_CO_INJECTED_POLICY_TEXTS.keys()) == expected diff --git a/tests/contracts/test_output_budget_discipline.py b/tests/contracts/test_output_budget_discipline.py index 30835de64..87cbdf195 100644 --- a/tests/contracts/test_output_budget_discipline.py +++ b/tests/contracts/test_output_budget_discipline.py @@ -84,10 +84,10 @@ def test_digest_is_safe_for_agent_toml_multiline_literal_guard() -> None: def test_intake_digest_pins_numeric_rules() -> None: anchors = [ - "at most 2 files per exec command", + "must be read completely", + "continue until EOF", "at most 250 lines", "max_output_tokens above 10000", - "at most 2 of the listed files", "an index, not required reading", "fresh context", 'fork_turns "none"', diff --git a/tests/docs/test_output_budget_protocol_decision.py b/tests/docs/test_output_budget_protocol_decision.py index 46b11ebbb..6ec8946c0 100644 --- a/tests/docs/test_output_budget_protocol_decision.py +++ b/tests/docs/test_output_budget_protocol_decision.py @@ -166,6 +166,18 @@ def test_decision_defines_the_recipe_section_byte_budget(decision_text: str) -> assert "token×4" in decision_text or "token x 4" in decision_text +def test_adr_describes_the_subagent_rule_it_actually_gates(decision_text: str) -> None: + """The Forward Obligations bullet must describe the shipped rule, not a retired one. + + R6 of the v1 intake digest said sub-agents "return a summary, not raw file + contents" while this ADR described the gate as a "do not spawn sub-agents" guard — + the two texts never matched (#4351). + """ + assert "do not spawn sub-agents" not in decision_text + assert 'fork_turns "none"' in decision_text + assert "not raw file contents" in decision_text + + def test_decision_requires_exact_bounded_recipe_section_rendering(decision_text: str) -> None: for required in ( "complete outer response", diff --git a/tests/infra/AGENTS.md b/tests/infra/AGENTS.md index eaa82aa76..00753df3e 100644 --- a/tests/infra/AGENTS.md +++ b/tests/infra/AGENTS.md @@ -21,6 +21,7 @@ CI/CD configuration, security, guard coverage, and release sanity tests. | `test_ci_dev_config.py` | Structural enforcement: CI workflow and pre-commit configuration must contain required quality gates | | `test_ci_shard_config.py` | Tests for CI shard directory configuration consistency | | `test_ci_workflow.py` | CI workflow structural tests | +| `test_codex_read_repetition_measure.py` | Tests for scripts/measure_codex_read_repetition.py (#4351) — bounded-read classifier, repeat-read counting, cohort split, malformed-record handling | | `test_claude_md_critical_rules.py` | Tests that effective `CLAUDE.md` (resolved via the `@AGENTS.md` include) contains required critical rules from friction analysis; direct ownership of shared rules lives in `AGENTS.md` and is covered by `test_docs_critical_rules.py` and `tests/docs/test_agents_md_content.py` | | `test_command_guard_completeness.py` | Structural meta-test: command-inspecting guards must cover all command-executing tools | | `test_command_guard_verb_position.py` | Structural ratchet: command-inspecting guards must not perform raw substring membership against shell command text — guards must tokenize evaluated payloads and compare verb/argument positions | diff --git a/tests/infra/test_codex_read_repetition_measure.py b/tests/infra/test_codex_read_repetition_measure.py new file mode 100644 index 000000000..8cf41c001 --- /dev/null +++ b/tests/infra/test_codex_read_repetition_measure.py @@ -0,0 +1,208 @@ +"""Tests for scripts/measure_codex_read_repetition.py (#4351). + +Exercises the extractor, bounded-read classifier, and cohort aggregator against a +synthetic rollout corpus so the measurement tool is covered code, not an unwired +artifact. +""" + +from __future__ import annotations + +import importlib.util +import json +from pathlib import Path +from types import ModuleType + +import pytest + +pytestmark = pytest.mark.small + +SCRIPT = Path(__file__).parents[2] / "scripts" / "measure_codex_read_repetition.py" + + +def _load_measurer() -> ModuleType: + spec = importlib.util.spec_from_file_location("codex_read_repetition", SCRIPT) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +measurer = _load_measurer() + + +def _exec_record(cmd: str) -> dict: + return { + "type": "response_item", + "payload": { + "type": "function_call", + "name": "exec_command", + "arguments": json.dumps({"cmd": cmd}), + }, + } + + +def _custom_tool_call_record(raw_input: str) -> dict: + return { + "type": "response_item", + "payload": {"type": "custom_tool_call", "name": "exec", "input": raw_input}, + } + + +def _cohort_marker_record(text: str) -> dict: + return {"type": "response_item", "payload": {"type": "message", "content": text}} + + +def _write_rollout(tmp_path: Path, name: str, records: list[dict]) -> Path: + path = tmp_path / name + with path.open("w", encoding="utf-8") as f: + for rec in records: + f.write(json.dumps(rec) + "\n") + return path + + +def test_classifier_counts_only_leading_bounded_reads() -> None: + assert measurer.is_bounded_read("sed -n '1,10p' /some/path") + assert not measurer.is_bounded_read("gh pr view 123 | head -c 18000") + + +def test_repeat_reads_are_counted_per_session_not_per_corpus(tmp_path: Path) -> None: + same_session = _write_rollout( + tmp_path, + "rollout-a.jsonl", + [ + _exec_record("sed -n '1,10p' /a/b.py"), + _exec_record("sed -n '11,20p' /a/b.py"), + ], + ) + row = measurer.measure_rollout(same_session) + assert row["bounded_reads"] == 2 + assert row["repeat_reads"] == 1 + + rollout_1 = _write_rollout( + tmp_path, "rollout-b.jsonl", [_exec_record("sed -n '1,10p' /a/b.py")] + ) + rollout_2 = _write_rollout( + tmp_path, "rollout-c.jsonl", [_exec_record("sed -n '1,10p' /a/b.py")] + ) + report = measurer.aggregate_report([rollout_1, rollout_2]) + assert report["cohorts"]["none"]["bounded_read_count"] == 2 + assert report["cohorts"]["none"]["repeat_count"] == 0 + + +def test_policy_version_cohort_split(tmp_path: Path) -> None: + v1 = _write_rollout( + tmp_path, + "rollout-v1.jsonl", + [ + _cohort_marker_record("Context Intake Discipline v1:\n- Never read end-to-end."), + _exec_record("sed -n '1,10p' /a.py"), + _exec_record("sed -n '11,20p' /a.py"), + ], + ) + v2 = _write_rollout( + tmp_path, + "rollout-v2.jsonl", + [ + _cohort_marker_record("Context Intake Discipline v2:\n- Read completely."), + _exec_record("sed -n '1,10p' /b.py"), + ], + ) + report = measurer.aggregate_report([v1, v2]) + assert report["cohorts"]["v1"]["bounded_read_count"] == 2 + assert report["cohorts"]["v1"]["repeat_count"] == 1 + assert report["cohorts"]["v1"]["repeat_read_rate"] == pytest.approx(0.5) + assert report["cohorts"]["v2"]["bounded_read_count"] == 1 + assert report["cohorts"]["v2"]["repeat_count"] == 0 + assert report["cohorts"]["v2"]["repeat_read_rate"] == pytest.approx(0.0) + + +def test_unparseable_records_are_skipped_not_fatal(tmp_path: Path) -> None: + path = tmp_path / "rollout-corrupt.jsonl" + path.write_text( + "{not valid json\n" + json.dumps(_exec_record("sed -n '1,10p' /a.py")) + "\n", + encoding="utf-8", + ) + commands, unclassified = measurer.classify_rollout_records(path) + assert len(commands) == 1 + assert unclassified == 0 + + +def test_unclassified_exec_shapes_are_counted_and_reported(tmp_path: Path) -> None: + records = [_custom_tool_call_record("this does not match the exec_command JS template")] + path = _write_rollout(tmp_path, "rollout-unclassified.jsonl", records) + commands, unclassified = measurer.classify_rollout_records(path) + assert commands == [] + assert unclassified == 1 + + +def test_find_rollouts_matches_the_real_two_level_yyyy_mm_layout(tmp_path: Path) -> None: + # _codex_session_storage.py lays out rollouts as /YYYY/MM/rollout-*.jsonl -- + # never a third YYYY/MM/DD level. + rollout = tmp_path / "2026" / "07" / "rollout-a.jsonl" + rollout.parent.mkdir(parents=True) + rollout.write_text("{}") + found = measurer._find_rollouts(tmp_path, None, None) + assert found == [rollout] + + +def _write_dated_rollout(tmp_path: Path, year: str, month: str, day: str, thread: str) -> Path: + directory = tmp_path / year / month + directory.mkdir(parents=True, exist_ok=True) + path = directory / f"rollout-{year}-{month}-{day}T10-00-00-{thread}.jsonl" + path.write_text("{}") + return path + + +def test_find_rollouts_date_filter_actually_filters(tmp_path: Path) -> None: + june = _write_dated_rollout(tmp_path, "2026", "06", "15", "thread-june") + july_01 = _write_dated_rollout(tmp_path, "2026", "07", "01", "thread-july-01") + july_15 = _write_dated_rollout(tmp_path, "2026", "07", "15", "thread-july-15") + july_31 = _write_dated_rollout(tmp_path, "2026", "07", "31", "thread-july-31") + august = _write_dated_rollout(tmp_path, "2026", "08", "05", "thread-august") + + # Month-precision bounds still work at day-precision resolution: every day + # inside the bounded month is included. + found = measurer._find_rollouts(tmp_path, "2026-07", "2026-07") + assert found == [july_01, july_15, july_31] + + # Day-precision --since must not silently drop the whole target month. Before + # the fix, the extracted date key was month precision ("2026-07"), which + # string-sorts before any day-precision --since ("2026-07" < "2026-07-18"), + # so every rollout in the target month was incorrectly dropped. + found = measurer._find_rollouts(tmp_path, "2026-07-18", None) + assert found == [july_31, august] + + # Day-precision --until must not symmetrically over-include the whole target + # month ("2026-07" > "2026-07-18" was False, so all of July was kept). + found = measurer._find_rollouts(tmp_path, None, "2026-07-18") + assert found == [june, july_01, july_15] + + # --since/--until are documented as inclusive: the exact boundary date itself + # must be kept, not excluded. + found = measurer._find_rollouts(tmp_path, "2026-07-15", "2026-07-15") + assert found == [july_15] + + +def test_extract_target_path_survives_pipe_alternation_in_rg_pattern() -> None: + # AGENTS.md documents `|` alternation as this repo's ripgrep idiom -- the + # extractor must not truncate at a `|` that is inside the quoted pattern. + assert ( + measurer.extract_target_path("rg -n 'foo|bar' src/autoskillit/file.py") + == "src/autoskillit/file.py" + ) + assert ( + measurer.extract_target_path('rg -n "foo|bar|baz" src/autoskillit/file.py') + == "src/autoskillit/file.py" + ) + assert ( + measurer.extract_target_path("rg -n 'foo|bar' src/autoskillit/file.py | head -c 18000") + == "src/autoskillit/file.py" + ) + + +def test_classify_policy_cohort_survives_an_unreadable_file(tmp_path: Path) -> None: + # A directory named like a rollout file can never be read_text'd -- this must + # not abort a batch run over many rollouts. + unreadable = tmp_path / "rollout-unreadable.jsonl" + unreadable.mkdir() + assert measurer.classify_policy_cohort(unreadable) == "none" diff --git a/tests/test_test_filter_core_cascade.py b/tests/test_test_filter_core_cascade.py index ff7468d90..fb913d873 100644 --- a/tests/test_test_filter_core_cascade.py +++ b/tests/test_test_filter_core_cascade.py @@ -106,6 +106,7 @@ def test_all_entries_present(self) -> None: "_type_session_env", "_type_capture", "_type_inspector", + "_type_intake_policy", "_type_invariant_registry", "_type_phoropter", "_type_token",