diff --git a/docs/README.md b/docs/README.md index d91eabd3d..1907f9b9a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2,7 +2,7 @@ AutoSkillit is a Claude Code plugin that runs YAML recipes through a multi-level orchestrator. The bundled recipes implement issue → plan → worktree -→ tests → PR → merge pipelines using 59 MCP tools and 142 bundled skills. +→ tests → PR → merge pipelines using 60 MCP tools and 142 bundled skills. ## Start here diff --git a/docs/decisions/0004-recipe-redelivery.md b/docs/decisions/0004-recipe-redelivery.md index 20ee35d8c..262c4aa3d 100644 --- a/docs/decisions/0004-recipe-redelivery.md +++ b/docs/decisions/0004-recipe-redelivery.md @@ -29,20 +29,34 @@ knowledge without reading raw files. ## Decision -> **`load_recipe` (the MCP tool exposed by `server/tools/tools_recipe.py`) is the -> sanctioned channel for recipe knowledge re-delivery after context compaction.** - -If recipe content is lost, the agent must call `load_recipe` to re-acquire it. The -`recipe_read_guard` enforces that only this path is allowed — raw file access is denied. +> **`load_recipe` / `open_kitchen` (the MCP tools exposed by `server/tools/tools_recipe.py`) +> remain the sanctioned entry points for recipe knowledge re-delivery after context +> compaction. When the response is a bounded envelope (oversized recipes exceeding the +> delivery bound — see ADR-0005), full re-delivery is completed by pulling each step via +> `get_recipe_section` (chunked via `part` / `has_more` / `next_part` for oversized +> sections). Both remain the only channels not blocked by `recipe_read_guard`'s +> deny-list on raw `run_cmd`/`Bash`/`run_python` recipe access — `recipe_read_guard.py` +> has no allow-list keyed on tool name; it blocks the raw-access alternatives rather +> than allow-listing `load_recipe`/`open_kitchen`/`get_recipe_section` by name.** + +If recipe content is lost, the agent must call `load_recipe` (or `open_kitchen`) to +re-acquire it; if the response is a bounded envelope, follow up by calling +`get_recipe_section(section=)` to pull each step body. The `recipe_read_guard` +deny-list on raw `run_cmd`/`Bash`/`run_python` recipe access remains the operative +constraint — raw file access is denied; the MCP tools above are the unblocked paths. This is a forward obligation: when the primary defense (unreachable auto-compact limit) -is relaxed, `load_recipe` re-delivery must be tested end-to-end as the recovery path. +is relaxed, the `load_recipe` / `open_kitchen` / `get_recipe_section` end-to-end +recovery path — including envelope re-delivery plus per-step pulls — must be tested as +the recovery path. ## Consequences -- `recipe_read_guard.py` error messages direct the agent to call `load_recipe`. +- `recipe_read_guard.py` error messages direct the agent to call `load_recipe` (and, + for envelope responses, `get_recipe_section`). - No production code changes are needed today — the channel already exists and works. - Future work relaxing `CODEX_AUTO_COMPACT_LIMIT` must add integration tests verifying - `load_recipe` re-delivery restores full pipeline execution capability. + `load_recipe` / `open_kitchen` / `get_recipe_section` re-delivery restores full + pipeline execution capability, including envelope re-delivery plus per-step pulls. - `test_copied_config_has_auto_compact_limit` validates the primary defense path: `setup_session_dir` preserves the override in the copied `config.toml`. \ No newline at end of file diff --git a/docs/decisions/0005-output-budget-protocol.md b/docs/decisions/0005-output-budget-protocol.md index 8af2d38c9..6942f4636 100644 --- a/docs/decisions/0005-output-budget-protocol.md +++ b/docs/decisions/0005-output-budget-protocol.md @@ -50,15 +50,28 @@ The raw-text `open_kitchen` and `load_recipe` responses are measured exemptions universal backstop. `RESPONSE_BACKSTOP_EXEMPTION_REGISTRY` is the closed authority for their independent character ceiling, UTF-8 byte ceiling, and measurement identity. Its canonical digest is carried in tool metadata and probe-cache identity. Adding or relaxing -an exemption requires re-measurement and a deliberate registry-digest change. +an exemption requires re-measurement and a deliberate registry-digest change. The raw-text +exemption now applies only to the fits-within-bound fast path; when a recipe payload exceeds +the delivery bound, `maybe_envelope_recipe_response` replaces it with a bounded envelope +(via `build_recipe_envelope`) that fits every backend by construction, with full step +content available on demand via `get_recipe_section`. ## Numeric Limits and Rationale | Limit | Decision and rationale | |---|---| -| `load_recipe`: `max_chars = 185_000`, `max_utf8_bytes = 185_000` | The 2026-07-16 independent all-recipe/all-mode pre-backstop measurement reached 183,103 characters and UTF-8 bytes for `remediation` with all truthy ingredients. The 1,897-unit margin makes serving growth explicit. Measurement identity: `bundled-recipes-all-modes-2026-07-16/load-recipe`. | -| `open_kitchen`: `max_chars = 186_000`, `max_utf8_bytes = 186_000` | The matching current-version pre-backstop measurement reached 183,103 characters and UTF-8 bytes for `remediation` with all truthy ingredients. The 2,897-unit margin covers the open-kitchen routing fields without conflating this handler ceiling with the smaller formatted presentation. Measurement identity: `bundled-recipes-all-modes-2026-07-16/open-kitchen`. | -| `CODEX_TOOL_OUTPUT_TOKEN_LIMIT = 54_500` | Derive it from the largest registered exemption as `((186_000 + 3) // 4) + 8_000`: 46,500 tokens under the current client's four-byte heuristic, plus 8,000 tokens for serialized-payload headroom. It is a blast-radius damage bound, not the mechanism that makes evidence lossless. | +| `load_recipe`: `max_chars = 188_000`, `max_utf8_bytes = 188_000` | The 2026-07-21 independent all-recipe/all-mode pre-backstop measurement reached 186,621 characters and UTF-8 bytes for `remediation` with all truthy ingredients. The 1,379-unit margin makes serving growth explicit. Measurement identity: `bundled-recipes-all-modes-2026-07-21/load-recipe`. For payloads that exceed the delivery bound, see envelope note below. | +| `open_kitchen`: `max_chars = 188_000`, `max_utf8_bytes = 188_000` | The 2026-07-21 independent all-recipe/all-mode pre-backstop measurement reached 186,680 characters and UTF-8 bytes for `remediation` with all truthy ingredients. The 1,320-unit margin covers the open-kitchen routing fields without conflating this handler ceiling with the smaller formatted presentation. Measurement identity: `bundled-recipes-all-modes-2026-07-21/open-kitchen`. For payloads that exceed the delivery bound, see envelope note below. | + +> **Envelope note (oversized recipes, issue #4304 Part B):** When a recipe's serialized payload +> exceeds a backend's effective delivery bound, `maybe_envelope_recipe_response` replaces the +> raw text with a bounded envelope built via `build_recipe_envelope`. The envelope carries a +> step-flow skeleton plus a `recipe_pull` reference (`pull_tool: get_recipe_section`, +> `artifact_path`, `artifact_sha256`) so the agent re-acquires each step body on demand via +> `get_recipe_section(section=)` (chunked via `part` / `has_more` / `next_part` +> for oversized sections). The raw-text ceilings above continue to govern the fits-within-bound +> fast path; the envelope introduces a separate, fit-by-construction delivery channel. +| `CODEX_TOOL_OUTPUT_TOKEN_LIMIT = 55_000` | Derive it from the largest registered exemption as `((188_000 + 3) // 4) + 8_000`: 47,000 tokens under the current client's four-byte heuristic, plus 8,000 tokens for serialized-payload headroom. It is a blast-radius damage bound, not the mechanism that makes evidence lossless. | | `CODEX_AUTO_COMPACT_LIMIT = 999_999_999` | Retain the unreachable sentinel and the recovery obligation accepted in [ADR-0004](0004-recipe-redelivery.md). This protocol does not relax recipe-preservation policy. | | `inline_max_chars = 5_000` | Preserve the previous truncation threshold while changing the representation from destructive clipping to an artifact-backed preview. The configured 2,500-character head and 2,500-character tail retain both diagnostic setup and terminal status; a spill marker is added outside those source slices. | | `response_max_bytes = 90_000` | Bound the exact compact serialized handler payload before a coarser transport can clip it. Bytes are authoritative here; this is not a token or full JSON-RPC-envelope estimate. | @@ -181,8 +194,12 @@ reserve-trigger metrics, so instrumentation must not claim those mechanisms exis addition, or output-discipline policy-version change invalidates the applicable cached capability probe. - Run and pass the live large-output probe before making any of those changes effective. -- Preserve ADR-0004's end-to-end `load_recipe` re-delivery obligation if the - 999,999,999 auto-compaction sentinel is ever relaxed. +- Preserve ADR-0004's end-to-end recipe re-delivery obligation if the + 999,999,999 auto-compaction sentinel is ever relaxed. Re-delivery is implemented + as re-sending the envelope (when the original response was an envelope) and + pulling each step body via `get_recipe_section(section=)`, chunked via + `part` / `has_more` / `next_part` for oversized sections — not as a replay of + the full raw payload. Reconciles with the ADR-0004 cross-reference amendment. - After each codex-cli upgrade, re-verify the truncation heuristic (`CODEX_TOOL_OUTPUT_TOKEN_LIMIT`) and auto-compact sentinel (`CODEX_AUTO_COMPACT_LIMIT`) against the upstream registry AND observed session diff --git a/docs/execution/architecture.md b/docs/execution/architecture.md index 02f43e83e..768dbdf7f 100644 --- a/docs/execution/architecture.md +++ b/docs/execution/architecture.md @@ -4,7 +4,7 @@ How AutoSkillit runs a recipe end to end: orchestrator, kitchen gating, clone an ## Overview -AutoSkillit is a Claude Code plugin that orchestrates automated workflows using headless sessions. It provides 59 MCP tools and 142 bundled skills, organized into a gated visibility system. +AutoSkillit is a Claude Code plugin that orchestrates automated workflows using headless sessions. It provides 60 MCP tools and 142 bundled skills, organized into a gated visibility system. ## Core Concepts @@ -23,12 +23,12 @@ AutoSkillit uses a three-tier tool visibility model: - **Free-range (4 tools)**: Always visible — `open_kitchen`, `close_kitchen`, `disable_quota_guard`, `reload_session` - **Headless tools (2 tools)**: Revealed in headless sessions via `mcp.enable({'headless'})` — `test_check`, `unlock_agent_pack` -- **Kitchen-tagged tools (39 tools total)**: Gated behind `open_kitchen` — `run_skill`, - `run_cmd`, `run_python`, `merge_worktree`, `clone_repo`, `push_to_remote`, and 32 more. +- **Kitchen-tagged tools (41 tools total)**: Gated behind `open_kitchen` — `run_skill`, + `run_cmd`, `run_python`, `merge_worktree`, `clone_repo`, `push_to_remote`, and 34 more. Two kitchen tools (`test_check`, `unlock_agent_pack`) also carry the `headless` tag and are additionally pre-enabled in headless sessions. -When you call `open_kitchen` (automatically done by `order`), all 39 kitchen-tagged tools become +When you call `open_kitchen` (automatically done by `order`), all 41 kitchen-tagged tools become available for that session. This keeps normal Claude Code sessions clean — no pipeline tools cluttering the tool list. @@ -60,7 +60,7 @@ AutoSkillit supports four session modes with different tool and skill visibility - **`$ claude` (plugin, no kitchen)**: Regular Claude Code session with the AutoSkillit plugin loaded. Sees 4 Free Range MCP tools (`open_kitchen`, `close_kitchen`, `disable_quota_guard`, `reload_session`) and Tier 1 skills only - (`open-kitchen`, `close-kitchen`). After calling `/open-kitchen`, all 39 kitchen-tagged MCP + (`open-kitchen`, `close-kitchen`). After calling `/open-kitchen`, all 41 kitchen-tagged MCP tools become available. - **`$ autoskillit cook`**: Interactive development session. Sees all three skill tiers @@ -68,7 +68,7 @@ AutoSkillit supports four session modes with different tool and skill visibility `$ claude`); `/open-kitchen` reveals kitchen tools. - **`$ autoskillit order`**: Pipeline orchestrator session. Kitchen is pre-opened at startup — - all 59 MCP tools are available immediately. All skill tiers are accessible. The orchestrator + all 60 MCP tools are available immediately. All skill tiers are accessible. The orchestrator delegates work through `run_skill` (headless sessions) and `run_cmd` (shell commands). - **`run_skill` (headless)**: Worker sessions launched by the orchestrator. Sees 4 Free Range diff --git a/docs/execution/tool-access.md b/docs/execution/tool-access.md index 3263e2524..ab169e60c 100644 --- a/docs/execution/tool-access.md +++ b/docs/execution/tool-access.md @@ -1,6 +1,6 @@ # MCP Tool Access Control -AutoSkillit provides 59 MCP tools organized into three access levels that control which +AutoSkillit provides 60 MCP tools organized into three access levels that control which session types can see each tool. ## Three Access Levels @@ -55,7 +55,7 @@ Server startup sequence: ``` 1. mcp.disable(tags={"kitchen"}) - → hides 39 kitchen-tagged tools (including the 2 headless-tagged tools) + → hides 41 kitchen-tagged tools (including the 2 headless-tagged tools) 2. mcp.disable(tags={subset}) for each entry in config.subsets.disabled → e.g. hides all github-tagged tools if "github" is disabled @@ -86,7 +86,7 @@ missing kitchen visibility. ## Complete MCP Tool Access Control Map -All 59 tools with their access level, tags, source file, and functional category. +All 60 tools with their access level, tags, source file, and functional category. **Tag abbreviations**: AS = `autoskillit`, K = `kitchen`, HL = `headless`, GH = `github`, CI = `ci`, CL = `clone`, TL = `telemetry`, FL = `fleet` diff --git a/docs/skills/catalog.md b/docs/skills/catalog.md index 6f70489ad..2c3b8c5fc 100644 --- a/docs/skills/catalog.md +++ b/docs/skills/catalog.md @@ -8,7 +8,7 @@ you need an exhaustive listing; this catalog groups by purpose. Plugin-scanned at `src/autoskillit/skills/`: -- `open-kitchen` — reveals the 40 kitchen MCP tools +- `open-kitchen` — reveals the 41 kitchen MCP tools - `close-kitchen` — re-hides them - `sous-chef` — internal injection by `open_kitchen`; never appears as a slash command diff --git a/docs/skills/subsets.md b/docs/skills/subsets.md index 81027f11b..62652ce1a 100644 --- a/docs/skills/subsets.md +++ b/docs/skills/subsets.md @@ -84,7 +84,7 @@ Custom tags behave like built-in categories for filtering purposes: disabling ## FastMCP Mechanics: Why `open_kitchen` Re-Disables Subsets FastMCP session rules override server rules. When `open_kitchen` calls -`ctx.enable_components(tags={"kitchen"})` to reveal the 40 kitchen-tagged tools, this +`ctx.enable_components(tags={"kitchen"})` to reveal the 41 kitchen-tagged tools, this operation overwrites the server-level `mcp.disable(tags={"github"})` mark applied at startup. As a result, `open_kitchen` must immediately re-call `ctx.disable_components(tags={subset})` for each configured disabled subset to restore diff --git a/docs/skills/visibility.md b/docs/skills/visibility.md index fc59e9840..e06222716 100644 --- a/docs/skills/visibility.md +++ b/docs/skills/visibility.md @@ -96,7 +96,7 @@ and `/autoskillit:close-kitchen`. Skills in `skills_extended/` are never seen. Order is similar to cook: AutoSkillit launches Claude Code with access to all tiers. The key difference is the orchestrator (`sous-chef` skill) is injected and the kitchen -is pre-opened so all 59 MCP tools are available from the start. +is pre-opened so all 60 MCP tools are available from the start. ### Headless session (launched by `run_skill`) diff --git a/src/autoskillit/cli/_prompts_kitchen.py b/src/autoskillit/cli/_prompts_kitchen.py index b3584c85c..8e535cae5 100755 --- a/src/autoskillit/cli/_prompts_kitchen.py +++ b/src/autoskillit/cli/_prompts_kitchen.py @@ -61,8 +61,14 @@ def _build_open_kitchen_prompt( '"anomaly": display each finding\'s severity, step_group, summary, ' "and evidence to the user so they can act on them.\n\n" "If the user wants to run a recipe interactively (pipeline execution), " - f"call {mcp_prefix}open_kitchen(name='') without ingredients_only " - "to receive the full recipe content and orchestration rules.\n\n" + f"call {mcp_prefix}open_kitchen(name='') without ingredients_only. " + "The response is a bounded envelope (step-flow skeleton, orchestration rules, " + "and a pull reference). To read a step's full definition before executing it, " + f"call {mcp_prefix}get_recipe_section(section='', " + "recipe_name=recipe_pull.recipe_name, " + "producer_tool=recipe_pull.producer_tool, " + "artifact_sha256=recipe_pull.sha256). " + "Do not read recipe YAML files directly.\n\n" "OPTIONAL STEP SEMANTICS:\n" "- optional: true means the step is SKIPPED when its skip_when_false ingredient\n" " resolves to false. skip_when_false ingredient references are resolved\n" diff --git a/src/autoskillit/config/ingredient_defaults.py b/src/autoskillit/config/ingredient_defaults.py index 32b264af5..32c638e8c 100644 --- a/src/autoskillit/config/ingredient_defaults.py +++ b/src/autoskillit/config/ingredient_defaults.py @@ -33,7 +33,10 @@ "commit_files", ), ), - ("Recipes", ("migrate_recipe", "list_recipes", "load_recipe", "validate_recipe")), + ( + "Recipes", + ("migrate_recipe", "list_recipes", "load_recipe", "validate_recipe", "get_recipe_section"), + ), ("Agents", ("unlock_agent_pack",)), ( "Clone & Remote", diff --git a/src/autoskillit/core/__init__.pyi b/src/autoskillit/core/__init__.pyi index 9c0f39467..2e7ae85c3 100644 --- a/src/autoskillit/core/__init__.pyi +++ b/src/autoskillit/core/__init__.pyi @@ -56,9 +56,11 @@ from .io import _COMMITTED_BY_DESIGN as _COMMITTED_BY_DESIGN from .io import ReadResult as ReadResult from .io import YAMLError as YAMLError from .io import atomic_write as atomic_write +from .io import compose_yaml as compose_yaml from .io import dump_yaml_str as dump_yaml_str from .io import ensure_project_temp as ensure_project_temp from .io import load_yaml as load_yaml +from .io import mapping_entry_byte_ranges_from_yaml as mapping_entry_byte_ranges_from_yaml from .io import read_versioned_json as read_versioned_json from .io import resolve_skill_temp_dir as resolve_skill_temp_dir from .io import resolve_temp_dir as resolve_temp_dir diff --git a/src/autoskillit/core/io.py b/src/autoskillit/core/io.py index 755f95df4..0d73539b3 100644 --- a/src/autoskillit/core/io.py +++ b/src/autoskillit/core/io.py @@ -41,8 +41,10 @@ "ReadResult", "YAMLError", "atomic_write", + "compose_yaml", "ensure_project_temp", "load_yaml", + "mapping_entry_byte_ranges_from_yaml", "dump_yaml_str", "read_versioned_json", "resolve_skill_temp_dir", @@ -408,6 +410,69 @@ def load_yaml(source: os.PathLike[str] | str) -> Any: return yaml.load(source, Loader=_Loader) +def compose_yaml(source: str) -> yaml.Node | None: + """Parse *source* into a mark-annotated YAML node tree (not a data structure). + + Unlike :func:`load_yaml`, retains ``start_mark`` / ``end_mark`` character + offsets on every node, which the byte-range tracker in + ``server/tools/_serve_helpers.py`` uses to compute per-step byte spans + of the original ``content`` text. Returns ``None`` when the source is + empty (matches :func:`yaml.compose` semantics). + """ + return yaml.compose(source, Loader=_Loader) + + +def mapping_entry_byte_ranges_from_yaml( + content: str, mapping_path: tuple[str, ...] +) -> dict[str, tuple[int, int]]: + """Compute UTF-8 byte ranges for entries under a YAML mapping path. + + Walks the persisted YAML ``content`` field via :func:`compose_yaml` to read + each selected mapping entry's key/value ``start_mark`` / ``end_mark`` character + offsets, then converts them to UTF-8 byte offsets so the result can be + used directly to slice the payload back at the byte level. + + Fails open: returns ``{}`` on any malformed or non-mapping document. The + guards (rather than a bare ``except YAMLError``) handle the documented + case where ``yaml.compose`` succeeds but produces a non-mapping root + (a bare sequence, or a ``steps:`` key whose value is a scalar) — a bare + ``except`` would miss ``TypeError`` / ``ValueError`` raised from + tuple-unpacking such a non-mapping node tree. + + Centralizes the yaml import: this module is the only place in the + package that imports ``yaml`` directly (REQs in + ``tests/arch/test_subpackage_isolation.py`` and + ``tests/core/test_io.py::test_only_yaml_imports_yaml_directly``). + """ + out: dict[str, tuple[int, int]] = {} + if not content or not mapping_path: + return out + try: + root = compose_yaml(content) + except yaml.YAMLError: + return out + if not isinstance(root, yaml.MappingNode): + return out + current = root + for segment in mapping_path: + next_node = None + for key_node, value_node in current.value: + if getattr(key_node, "value", None) == segment: + next_node = value_node + break + if not isinstance(next_node, yaml.MappingNode): + return out + current = next_node + for entry_key, entry_value in current.value: + start_idx = entry_key.start_mark.index + end_idx = entry_value.end_mark.index + out[str(entry_key.value)] = ( + len(content[:start_idx].encode("utf-8")), + len(content[:end_idx].encode("utf-8")), + ) + return out + + def dump_yaml_str(data: Any, **kwargs: Any) -> str: """Serialize data to a YAML string. diff --git a/src/autoskillit/core/types/_type_backend.py b/src/autoskillit/core/types/_type_backend.py index 3f231f062..2923c0d15 100644 --- a/src/autoskillit/core/types/_type_backend.py +++ b/src/autoskillit/core/types/_type_backend.py @@ -167,7 +167,14 @@ class BackendCapabilities: # config-file ceiling (e.g. `tool_output_token_limit`), which code-mode # models may bypass. Used by `enforce_response_budget` to spill payloads # the downstream transport cannot deliver at full size. - effective_delivery_token_limit: int = 0 + # + # Default of 10,000 matches the smallest registered backend bound + # (Codex code-mode). Any new backend that omits this field inherits + # conservative bounding rather than the historical 0-sentinel "skip + # bounding" behavior — preventing a silent opt-out where a future + # backend without an explicit capability setting would be delivered + # without any delivery-bound enforcement. + effective_delivery_token_limit: int = 10_000 ALL_PROJECT_LOCAL_SKILL_SEARCH_DIRS: tuple[str, ...] = ( diff --git a/src/autoskillit/core/types/_type_constants_registries.py b/src/autoskillit/core/types/_type_constants_registries.py index eea0ce5c2..87f77225f 100644 --- a/src/autoskillit/core/types/_type_constants_registries.py +++ b/src/autoskillit/core/types/_type_constants_registries.py @@ -121,6 +121,7 @@ "create_and_publish_branch", "record_pipeline_step", "reset_dispatch", + "get_recipe_section", } ) @@ -190,14 +191,14 @@ class ResponseBackstopExemptionDef(NamedTuple): RESPONSE_BACKSTOP_EXEMPTION_REGISTRY: dict[str, ResponseBackstopExemptionDef] = { "load_recipe": ResponseBackstopExemptionDef( - max_chars=185_000, - max_utf8_bytes=185_000, - measurement_id="bundled-recipes-all-modes-2026-07-16/load-recipe", + max_chars=188_000, + max_utf8_bytes=188_000, + measurement_id="bundled-recipes-all-modes-2026-07-21/load-recipe", ), "open_kitchen": ResponseBackstopExemptionDef( - max_chars=186_000, - max_utf8_bytes=186_000, - measurement_id="bundled-recipes-all-modes-2026-07-16/open-kitchen", + max_chars=188_000, + max_utf8_bytes=188_000, + measurement_id="bundled-recipes-all-modes-2026-07-21/open-kitchen", ), } @@ -337,6 +338,7 @@ class AgentPackDef(NamedTuple): "merge_worktree": frozenset({"kitchen-core"}), "unlock_agent_pack": frozenset({"kitchen-core"}), "record_pipeline_step": frozenset({"kitchen-core"}), + "get_recipe_section": frozenset({"kitchen-core"}), } ALL_VISIBILITY_TAGS: frozenset[str] = frozenset( diff --git a/src/autoskillit/hooks/formatters/_fmt_recipe.py b/src/autoskillit/hooks/formatters/_fmt_recipe.py index 85c7b6924..07576992a 100644 --- a/src/autoskillit/hooks/formatters/_fmt_recipe.py +++ b/src/autoskillit/hooks/formatters/_fmt_recipe.py @@ -52,6 +52,7 @@ "stop_step_semantics", # delivered via open_kitchen response Channel B; not redisplayed "deferred_guards", # internal deferral metadata; not displayed to agent "post_prune_step_names", # internal preflight field; not displayed to agent + "post_prune_routing_edges", # internal preflight field; not displayed to agent "dispatch_feasible", # internal admission control signal; surfaced via refusal envelopes "infeasible_steps", # internal admission control detail; surfaced via refusal envelopes } @@ -217,6 +218,7 @@ def _fmt_load_recipe(data: LoadRecipeResult, pipeline: bool) -> str: "hook_warning", # edge-case diagnostic; not rendered in standard path "deferred_guards", # internal deferral metadata; not displayed to agent "post_prune_step_names", # internal preflight field; not displayed to agent + "post_prune_routing_edges", # internal preflight field; not displayed to agent "dispatch_feasible", # internal admission control signal; surfaced via refusal envelopes "infeasible_steps", # internal admission control detail; surfaced via refusal envelopes } diff --git a/src/autoskillit/hooks/formatters/_fmt_response_spill.py b/src/autoskillit/hooks/formatters/_fmt_response_spill.py index 05eb801a2..aa7523a17 100644 --- a/src/autoskillit/hooks/formatters/_fmt_response_spill.py +++ b/src/autoskillit/hooks/formatters/_fmt_response_spill.py @@ -47,14 +47,14 @@ ) _RESPONSE_BACKSTOP_EXEMPTION_REGISTRY = { "load_recipe": { - "max_chars": 185_000, - "max_utf8_bytes": 185_000, - "measurement_id": "bundled-recipes-all-modes-2026-07-16/load-recipe", + "max_chars": 188_000, + "max_utf8_bytes": 188_000, + "measurement_id": "bundled-recipes-all-modes-2026-07-21/load-recipe", }, "open_kitchen": { - "max_chars": 186_000, - "max_utf8_bytes": 186_000, - "measurement_id": "bundled-recipes-all-modes-2026-07-16/open-kitchen", + "max_chars": 188_000, + "max_utf8_bytes": 188_000, + "measurement_id": "bundled-recipes-all-modes-2026-07-21/open-kitchen", }, } _RESPONSE_BACKSTOP_EXEMPTION_REGISTRY_DIGEST = hashlib.sha256( diff --git a/src/autoskillit/hooks/formatters/pretty_output_hook.py b/src/autoskillit/hooks/formatters/pretty_output_hook.py index f10c3f803..be3fa12e7 100644 --- a/src/autoskillit/hooks/formatters/pretty_output_hook.py +++ b/src/autoskillit/hooks/formatters/pretty_output_hook.py @@ -199,6 +199,7 @@ def _response_spill_notice(metadata: dict) -> str: "lock_ingredients", # simple success/error result "record_pipeline_step", # structured init/status result "reset_dispatch", # JSON cleanup report, generic renders correctly + "get_recipe_section", # bounded section content with continuation } ) diff --git a/src/autoskillit/recipe/__init__.py b/src/autoskillit/recipe/__init__.py index 3ecee7206..a1e5eb96f 100644 --- a/src/autoskillit/recipe/__init__.py +++ b/src/autoskillit/recipe/__init__.py @@ -8,6 +8,10 @@ # Rule registration — import triggers @semantic_rule registration. from autoskillit.recipe import registry as _reg # noqa: E402, PLC0415 +from autoskillit.recipe._analysis import ( # noqa: E402 + RouteEdge, + _extract_routing_edges, +) from autoskillit.recipe._api import ( # noqa: E402 format_recipe_list_response, list_all, @@ -70,6 +74,7 @@ list_recipes, load_campaign_recipes_in_packs, load_recipe, + step_byte_ranges_from_yaml, ) from autoskillit.recipe.loader import parse_recipe_metadata # noqa: E402 from autoskillit.recipe.methodology_disambiguation import ( # noqa: E402 @@ -305,6 +310,7 @@ "write_staleness_cache", "RuleFinding", "load_recipe", + "step_byte_ranges_from_yaml", "list_recipes", "find_recipe_by_name", "iter_steps_with_context", @@ -373,6 +379,12 @@ "CampaignDispatch", "NON_INTERACTIVE_KINDS", "RecipeKind", + # --- routing-edge facade (canonical package-level re-export of the + # recipe/_analysis_graph definitions; enables consumers in other + # packages to reuse _extract_routing_edges without crossing + # REQ-ARCH-001's submodule-import boundary) --- + "RouteEdge", + "_extract_routing_edges", "find_campaign_by_name", "list_campaign_recipes", "load_campaign_recipes_in_packs", diff --git a/src/autoskillit/recipe/_api.py b/src/autoskillit/recipe/_api.py index 982ae394b..413252fe6 100644 --- a/src/autoskillit/recipe/_api.py +++ b/src/autoskillit/recipe/_api.py @@ -26,7 +26,7 @@ resolve_temp_dir, ) from autoskillit.recipe import _api_cache -from autoskillit.recipe._analysis import make_validation_context +from autoskillit.recipe._analysis import _extract_routing_edges, make_validation_context # Re-export for backward compatibility from autoskillit.recipe._api_cache import ( # noqa: F401 @@ -666,6 +666,15 @@ def load_and_validate( result["deferred_guards"] = _deferred_guard_list if active_recipe is not None: result["post_prune_step_names"] = list(active_recipe.steps.keys()) + _step_names_set = set(active_recipe.steps) + result["post_prune_routing_edges"] = sorted( + { + edge.target + for step in active_recipe.steps.values() + for edge in _extract_routing_edges(step) + if edge.target in _step_names_set + } + ) result["dispatch_feasible"] = _dispatch_feasible if _infeasible_steps: result["infeasible_steps"] = _infeasible_steps diff --git a/src/autoskillit/recipe/_recipe_ingredients.py b/src/autoskillit/recipe/_recipe_ingredients.py index b49c6c814..7f54c64f4 100644 --- a/src/autoskillit/recipe/_recipe_ingredients.py +++ b/src/autoskillit/recipe/_recipe_ingredients.py @@ -128,6 +128,7 @@ class LoadRecipeResult(TypedDict, total=False): recipe_version: str | None deferred_guards: list[DeferredGuard] post_prune_step_names: list[str] + post_prune_routing_edges: list[str] dispatch_feasible: bool infeasible_steps: list[str] warnings: NotRequired[list[str]] @@ -158,6 +159,7 @@ class OpenKitchenResult(TypedDict, total=False): recipe_version: str | None deferred_guards: list[DeferredGuard] post_prune_step_names: list[str] + post_prune_routing_edges: list[str] dispatch_feasible: bool infeasible_steps: list[str] warnings: NotRequired[list[str]] diff --git a/src/autoskillit/recipe/io.py b/src/autoskillit/recipe/io.py index 94d3cd613..7732af719 100644 --- a/src/autoskillit/recipe/io.py +++ b/src/autoskillit/recipe/io.py @@ -18,6 +18,7 @@ RecipeSource, get_logger, load_yaml, + mapping_entry_byte_ranges_from_yaml, pkg_root, ) from autoskillit.core import fast_loads as _fast_loads @@ -38,6 +39,12 @@ logger = get_logger(__name__) + +def step_byte_ranges_from_yaml(content: str) -> dict[str, tuple[int, int]]: + """Return UTF-8 byte ranges for entries in the recipe ``steps`` mapping.""" + return mapping_entry_byte_ranges_from_yaml(content, ("steps",)) + + RECIPE_SCAN_DIRS: tuple[str, ...] = ( ".", # root — standard recipes "campaigns", # campaign recipes diff --git a/src/autoskillit/server/_response_budget.py b/src/autoskillit/server/_response_budget.py index a67cc0d2c..75f3f7d7d 100644 --- a/src/autoskillit/server/_response_budget.py +++ b/src/autoskillit/server/_response_budget.py @@ -225,6 +225,23 @@ def _artifact_path(artifact_dir: Path, tool_name: str) -> Path: return artifact_dir / f"{safe_name or 'response'}_{uuid.uuid4().hex[:8]}.log" +def _recipe_artifact_path(artifact_dir: Path, tool_name: str, recipe_name: str) -> Path: + """Deterministic artifact path for recipe-bearing tools. + + Unlike ``_artifact_path`` (which uses UUID for uniqueness), the recipe + artifact path is deterministic so that the ``get_recipe_section`` pull + tool can reconstruct the same path without needing it passed back in + the envelope. Re-opening the same recipe overwrites the artifact + (idempotent) rather than scattering UUID-suffixed copies. + + The path is namespaced by both tool and recipe so two recipes loaded + from different surfaces (open_kitchen vs load_recipe) do not collide. + """ + safe_tool = "".join(c if c.isalnum() or c in "._-" else "_" for c in tool_name) + safe_recipe = "".join(c if c.isalnum() or c in "._-" else "_" for c in recipe_name) + return artifact_dir / f"{safe_tool or 'response'}_{safe_recipe or 'recipe'}.log" + + def _finalize_envelope(envelope: dict[str, Any], *, max_bytes: int) -> str: metadata = envelope.get(RESPONSE_SPILL_METADATA_KEY) if not isinstance(metadata, dict) or "projected_utf8_bytes" not in metadata: @@ -285,10 +302,40 @@ def _project_json_object( metadata: dict[str, Any], max_bytes: int, inline_chars: int, + delivery_bound_triggered: bool = False, ) -> str | None: + """Project a non-exempted JSON object into a bounded envelope. + + When ``delivery_bound_triggered`` is True, the caller is in the + delivery-bound spill path; the function routes to ``_tiered_projection`` + to protect a content-equivalent key (``"result"`` for run_skill-shaped + SkillResult payloads) from the identical starvation defect that affects + ``_delivery_bound_summary``'s ``content`` field. + + When ``delivery_bound_triggered`` is False (the default), the existing + uniform per-key ``_project_value`` algorithm runs unchanged — preserving + the behavior pinned by ``test_minimal_projection_has_exact_bytes_and_omission_aggregates``. + """ if RESPONSE_SPILL_METADATA_KEY in parsed: return None + if delivery_bound_triggered and "result" in parsed: + priority_keys = tuple( + key for key in ("success", "kitchen", "version", "errors") if key in parsed + ) + deprioritized_keys = tuple(key for key in parsed if key not in {*priority_keys, "result"}) + return _tiered_projection( + parsed, + metadata=metadata, + bound=max_bytes, + content_key="result", + deprioritized_keys=deprioritized_keys, + priority_keys=priority_keys, + droppable_keys=(), + reason="delivery_bound", + top_level_flag=None, + ) + key_count = max(1, len(parsed)) value_limit = max(16, min(inline_chars, max_bytes // (key_count + 1))) while value_limit >= 16: @@ -358,159 +405,419 @@ def _plain_spill_envelope( preview_limit //= 2 -_DELIVERY_BOUND_PRESERVED_KEYS: tuple[str, ...] = ( +_DELIVERY_BOUND_PRIORITY_KEYS: tuple[str, ...] = ( "success", "kitchen", "version", - "ingredients_table", "orchestration_rules", "stop_step_semantics", "errors", +) +_DELIVERY_BOUND_DEPRIORITIZED_KEYS: tuple[str, ...] = ( "suggestions", + "ingredients_table", ) _DELIVERY_BOUND_CONTENT_KEY = "content" _DELIVERY_BOUND_DROPPABLE_KEYS: tuple[str, ...] = ("diagram",) +# Floor (bytes) reserved per present deprioritized key. Covers the smallest +# plausible serialized ``"key": value`` pair once a value is projected down +# toward empty (``_minimal_same_type`` reduces list/dict to ``[]``/``{}``, +# the floor covers the surrounding key/quote/colon overhead). Guarantees +# deprioritized keys remain *present* in the envelope — projected but +# never dropped entirely. +_DEPRIORITIZED_KEY_FLOOR_BYTES = 16 + + +def _serialized_string_prefix_length(value: str, max_bytes: int) -> int: + """Return the largest prefix whose serialized string body fits ``max_bytes``.""" + low, high = 0, len(value) + best = 0 + while low <= high: + midpoint = (low + high) // 2 + serialized_bytes = len(_canonical_json(value[:midpoint]).encode("utf-8")) - 2 + if serialized_bytes <= max_bytes: + best = midpoint + low = midpoint + 1 + else: + high = midpoint - 1 + return best -def _delivery_bound_summary( + +def _tiered_projection( parsed: dict[str, Any], *, metadata: dict[str, Any], bound: int, + content_key: str, + deprioritized_keys: tuple[str, ...], + priority_keys: tuple[str, ...] = (), + droppable_keys: tuple[str, ...] = (), + reason: str, + top_level_flag: str | None = "delivery_bound_spill", ) -> str | None: - """Build a bounded inline summary honoring the delivery bound. - - Preserves ``success``, ``kitchen``, ``version``, ``ingredients_table``, - ``orchestration_rules``, ``stop_step_semantics``, ``errors``, - ``suggestions`` verbatim (when present and fitting); truncates ``content``; - drops ``diagram``; nests spill metadata under ``RESPONSE_SPILL_METADATA_KEY`` - with ``reason="delivery_bound"``; sets top-level ``delivery_bound_spill=True``. + """Build a tiered projection honoring a byte ``bound``. + + Budget allocation order: + 1. Priority keys (caller-supplied ``priority_keys``) are preserved + verbatim first — these are small and structurally necessary. + 2. Deprioritized keys (``deprioritized_keys``) are reserved a per-key + floor (``_DEPRIORITIZED_KEY_FLOOR_BYTES``) so they remain *present* + in the envelope even after projection. + 3. ``content_key`` is allocated a guaranteed floor from the remaining + budget (at least half the post-floor remainder); Tier 1 then + binary-searches upward from that floor for the largest + ``content_head`` that still fits alongside the un-projected + deprioritized/droppable keys — reallocating any budget left unused + because those keys serialize smaller than their assumed share. + This binary search is the sole reallocation mechanism: no separate + "measure freed bytes, then rebuild with a computed head length" + pass exists, because the search already finds the true maximum + against the exact rendered size. + 4. Any remaining budget flows to the deprioritized keys (which are + projected to fit). + 5. ``droppable_keys`` are included verbatim if they fit within the + remaining budget; otherwise they are dropped (counted as omission). + They are not part of the priority verbatim set. + 6. Keys not in any tier are always dropped (counted as omission). Returns the rendered envelope string when it fits ``bound``; otherwise ``None`` so the caller fails closed. """ - content_text = parsed.get(_DELIVERY_BOUND_CONTENT_KEY) - content_is_str = isinstance(content_text, str) - - precomputed_base_chars = 0 - precomputed_base_items = 0 - for key, value in parsed.items(): - if key in _DELIVERY_BOUND_PRESERVED_KEYS or key == _DELIVERY_BOUND_CONTENT_KEY: - continue - chars, items = _total_omissions(value) - precomputed_base_chars += chars - precomputed_base_items += items + present_priority = [k for k in priority_keys if k in parsed] + present_deprioritized = [k for k in deprioritized_keys if k in parsed] + present_droppable = [k for k in droppable_keys if k in parsed] + tier_all = set(priority_keys) | set(deprioritized_keys) | set(droppable_keys) | {content_key} + other_keys = [k for k in parsed if k not in tier_all] - if not content_is_str and _DELIVERY_BOUND_CONTENT_KEY in parsed: - chars, items = _total_omissions(parsed[_DELIVERY_BOUND_CONTENT_KEY]) - precomputed_base_chars += chars - precomputed_base_items += items + content_text = parsed.get(content_key) if content_key in parsed else None + content_is_str = isinstance(content_text, str) - present_preserved = [key for key in _DELIVERY_BOUND_PRESERVED_KEYS if key in parsed] + # Step 1: Measure the priority-only envelope (content excluded, deprioritized + # excluded, droppable excluded) to size the remaining budget for content + + # deprioritized + droppable. + def _build_priority_only() -> dict[str, Any]: + env: dict[str, Any] = {RESPONSE_SPILL_METADATA_KEY: dict(metadata)} + if top_level_flag is not None: + env[top_level_flag] = True + for k in present_priority: + env[k] = parsed[k] + # Account for omitted "other" keys up front (always dropped). + base_chars = 0 + base_items = 0 + for k in other_keys: + chars, items = _total_omissions(parsed[k]) + base_chars += chars + base_items += items + env[RESPONSE_SPILL_METADATA_KEY] = _spill_metadata( + env[RESPONSE_SPILL_METADATA_KEY], + reason=reason, + omitted_chars=base_chars, + omitted_items=base_items, + ) + return env - def _build(head_len: int, include_droppable: bool, value_projector=None) -> dict[str, Any]: - envelope: dict[str, Any] = {RESPONSE_SPILL_METADATA_KEY: dict(metadata)} - envelope["delivery_bound_spill"] = True + priority_envelope = _build_priority_only() + try: + priority_rendered = _finalize_envelope(priority_envelope, max_bytes=bound) + except _ProjectionNonconvergentError: + return None + priority_bytes = len(priority_rendered.encode("utf-8")) - base_chars = precomputed_base_chars - base_items = precomputed_base_items + # Step 2: Deprioritized key floor (reserved bytes per present deprioritized key). + deprioritized_floor = _DEPRIORITIZED_KEY_FLOOR_BYTES * len(present_deprioritized) - for key in present_preserved: - if value_projector is None: - envelope[key] = parsed[key] - else: - envelope[key] = value_projector(parsed[key]) + # Step 3: Content budget = remaining after priority + deprioritized floor. + content_budget = max(0, bound - priority_bytes - deprioritized_floor - 64) - if include_droppable: - for key in _DELIVERY_BOUND_DROPPABLE_KEYS: - if key in parsed: - envelope[key] = parsed[key] - else: - for key in _DELIVERY_BOUND_DROPPABLE_KEYS: - if key in parsed: - chars, items = _total_omissions(parsed[key]) + # Step 4: Reserve at least half the remaining byte budget for string + # content. Convert that serialized-byte floor to a character count so + # multibyte and escaped content remain byte-safe. Non-empty content always + # retains at least one character or the projection fails closed. + if content_is_str: + text = content_text or "" + floor_bytes = max(1, content_budget // 2) + content_floor = max(1, _serialized_string_prefix_length(text, floor_bytes)) if text else 0 + else: + text = "" + content_floor = 0 + + # Deprioritized keys get their floor plus any share of the remaining + # budget after the content floor. + deprioritized_budget = deprioritized_floor + max(0, content_budget - content_floor) + + def _build_with_lengths( + content_head: int, + deprioritized_projector: Any = None, + content_projector: Any = None, + include_content: bool = True, + include_deprioritized: bool = True, + include_droppable: bool = True, + ) -> dict[str, Any]: + env: dict[str, Any] = {RESPONSE_SPILL_METADATA_KEY: dict(metadata)} + if top_level_flag is not None: + env[top_level_flag] = True + base_chars = 0 + base_items = 0 + + for k in present_priority: + env[k] = parsed[k] + + # Always-omitted "other" keys (counted as omissions). + for k in other_keys: + chars, items = _total_omissions(parsed[k]) + base_chars += chars + base_items += items + + if include_deprioritized and deprioritized_projector is not None: + for k in present_deprioritized: + projected = deprioritized_projector(parsed[k]) + env[k] = projected + if not ( + isinstance(projected, (list, dict)) + and RESPONSE_SPILL_METADATA_KEY in projected + ): + chars, items = _total_omissions(parsed[k]) base_chars += chars base_items += items + elif include_deprioritized: + for k in present_deprioritized: + env[k] = parsed[k] - if content_is_str: - text = content_text or "" - truncated = text[:head_len] - envelope[_DELIVERY_BOUND_CONTENT_KEY] = truncated - base_chars += max(0, len(text) - head_len) - elif _DELIVERY_BOUND_CONTENT_KEY in parsed: - envelope[_DELIVERY_BOUND_CONTENT_KEY] = parsed[_DELIVERY_BOUND_CONTENT_KEY] - - if value_projector is not None: - for key in present_preserved: - projected_value = envelope[key] - if ( - isinstance(projected_value, dict) - and RESPONSE_SPILL_METADATA_KEY in projected_value - ): - continue - chars, items = _total_omissions(parsed[key]) + if include_droppable: + for k in present_droppable: + env[k] = parsed[k] + else: + for k in present_droppable: + chars, items = _total_omissions(parsed[k]) base_chars += chars base_items += items - envelope[RESPONSE_SPILL_METADATA_KEY] = _spill_metadata( - envelope[RESPONSE_SPILL_METADATA_KEY], - reason="delivery_bound", + if include_content and content_is_str: + env[content_key] = text[:content_head] + base_chars += max(0, len(text) - content_head) + elif content_key in parsed and include_content: + if content_projector is None: + env[content_key] = parsed[content_key] + else: + projected, chars, items = content_projector(parsed[content_key]) + env[content_key] = projected + base_chars += chars + base_items += items + elif content_key in parsed and not include_content: + chars, items = _total_omissions(parsed[content_key]) + base_chars += chars + base_items += items + + env[RESPONSE_SPILL_METADATA_KEY] = _spill_metadata( + env[RESPONSE_SPILL_METADATA_KEY], + reason=reason, omitted_chars=base_chars, omitted_items=base_items, ) - return envelope + return env - def _fits(envelope: dict[str, Any]) -> str | None: + def _fits(env: dict[str, Any]) -> str | None: try: - rendered = _finalize_envelope(envelope, max_bytes=bound) + rendered = _finalize_envelope(env, max_bytes=bound) except _ProjectionNonconvergentError: return None if len(rendered.encode("utf-8")) <= bound: return rendered return None - zero_envelope = _build(0, True) - base_rendered = _finalize_envelope(zero_envelope, max_bytes=bound) - base_bytes = len(base_rendered.encode("utf-8")) - head_limit = max(0, bound - base_bytes - 64) if content_is_str else 0 + def _maximize_content_head( + *, deprioritized_projector: Any = None, include_droppable: bool = True + ) -> str | None: + """Binary-search the largest ``content_head`` (in ``[content_floor, len(text)]``) + whose rendered envelope still fits ``bound``. + + Used by Tier 1 and Tier 2 to reallocate any budget left unused because + deprioritized/droppable keys serialize smaller than their allotted + share — the search finds the true maximum against the exact rendered + size, with no separate measure-then-reallocate pass required. + """ + low, high = content_floor, len(text) + best_rendered: str | None = None + while low <= high: + mid = (low + high) // 2 + candidate = _fits( + _build_with_lengths( + content_head=mid, + deprioritized_projector=deprioritized_projector, + include_droppable=include_droppable, + ) + ) + if candidate is not None: + best_rendered = candidate + low = mid + 1 + else: + high = mid - 1 + return best_rendered - while head_limit > 0 and content_is_str: - rendered = _fits(_build(head_limit, True)) + if content_key in parsed and not content_is_str: + rendered = _fits(_build_with_lengths(content_head=0)) if rendered is not None: return rendered - head_limit //= 2 - if content_is_str: - zero_head_envelope = _build(0, True) - rendered = _fits(zero_head_envelope) + value_limit = max(16, content_budget) + while value_limit >= 16: + + def _project_content(value: Any, _limit: int = value_limit) -> tuple[Any, int, int]: + return _project_value(value, _limit) + + rendered = _fits( + _build_with_lengths( + content_head=0, + content_projector=_project_content, + include_droppable=False, + ) + ) + if rendered is not None: + return rendered + if present_deprioritized: + rendered = _fits( + _build_with_lengths( + content_head=0, + content_projector=_project_content, + deprioritized_projector=lambda value: _minimal_same_type(value), + include_droppable=False, + ) + ) + if rendered is not None: + return rendered + value_limit //= 2 + + rendered = _fits( + _build_with_lengths( + content_head=0, + content_projector=lambda value: ( + _minimal_same_type(value), + *_total_omissions(value), + ), + deprioritized_projector=lambda value: _minimal_same_type(value), + include_droppable=False, + ) + ) if rendered is not None: return rendered - rendered = _fits(_build(0, False)) - if rendered is not None: - return rendered + # Tier 1: priority verbatim + deprioritized verbatim + droppable; binary-search the + # largest content_head (from content_floor up to the full text) that still fits, so + # any budget left unused because deprioritized/droppable keys serialize smaller than + # their allotted share is reallocated back to content rather than stranded at the floor. + if content_is_str: + rendered = _maximize_content_head() + if rendered is not None: + return rendered - if present_preserved: - value_limit = max(16, bound // (len(present_preserved) + 2)) - while value_limit >= 16: + # Tier 2: deprioritized keys projected to fit deprioritized_budget; binary-search + # for the largest content_head that fits. Droppable keys are dropped to + # maximize budget for content. + if content_is_str and present_deprioritized: + value_limit = max(16, deprioritized_budget // (len(present_deprioritized) + 1)) + while value_limit >= _DEPRIORITIZED_KEY_FLOOR_BYTES: def _project_with_limit(value: Any, _limit: int = value_limit) -> Any: - projected, _chars, _items = _project_value(value, _limit) + projected = _project_value(value, _limit)[0] return projected - rendered = _fits(_build(0, False, value_projector=_project_with_limit)) + rendered = _maximize_content_head( + deprioritized_projector=_project_with_limit, include_droppable=False + ) if rendered is not None: return rendered value_limit //= 2 - minimal_envelope = _build(0, False, value_projector=_minimal_same_type) - rendered = _fits(minimal_envelope) + # Fallback: deprioritized at minimum (still present, projected to floor). + rendered = _fits( + _build_with_lengths( + content_head=content_floor, + deprioritized_projector=lambda v: _minimal_same_type(v), + include_droppable=False, + ) + ) + if rendered is not None: + return rendered + + # Tier 3: drop droppable + deprioritized at floor; content already at floor. + if present_deprioritized: + rendered = _fits( + _build_with_lengths( + content_head=content_floor, + deprioritized_projector=lambda v: _minimal_same_type(v), + include_droppable=False, + ) + ) if rendered is not None: return rendered + # A non-empty string content field must retain its positive floor. Returning + # the priority-only fallback would silently starve the promised payload. + if content_is_str and text: + return None + + # Tier 4 (terminal fallback): priority verbatim only — content excluded + # entirely; droppable excluded; deprioritized excluded. Used when payload + # has no identifiable content_key (e.g. ``{"success": True, "data": ...}``) + # so the envelope still surfaces priority keys rather than failing closed. + rendered = _fits( + _build_with_lengths( + content_head=0, + deprioritized_projector=lambda v: _minimal_same_type(v), + include_content=False, + include_deprioritized=False, + include_droppable=False, + ) + ) + if rendered is not None: + return rendered + return None +def _delivery_bound_summary( + parsed: dict[str, Any], + *, + metadata: dict[str, Any], + bound: int, +) -> str | None: + """Build a bounded inline summary honoring the delivery bound. + + Preserves the priority keys (``success``, ``kitchen``, ``version``, + ``orchestration_rules``, ``stop_step_semantics``, ``errors``) verbatim; + allocates a guaranteed floor to ``content``; reserves a per-key floor + for deprioritized keys (``suggestions``, ``ingredients_table``) so they + remain present but projected; drops ``diagram``; nests spill metadata + under ``RESPONSE_SPILL_METADATA_KEY`` with ``reason="delivery_bound"``; + sets top-level ``delivery_bound_spill=True``. + + Returns the rendered envelope string when it fits ``bound``; otherwise + ``None`` so the caller fails closed. + + Regression guard for issue #4304: the historical algorithm computed + ``head_limit = max(0, bound - base_bytes - 64)`` from the unshrunk + preserved-key envelope, found ``base_bytes > bound`` when ``suggestions`` + was at the real-world 48KB+ size regime, and starved ``content`` to + ``""``. This implementation allocates the budget in priority order: + priority keys verbatim → deprioritized floor → content floor, maximized + via binary search so freed budget flows back to content → any remaining + to deprioritized keys. Droppable keys (diagram) are included only if + budget allows. + """ + return _tiered_projection( + parsed, + metadata=metadata, + bound=bound, + content_key=_DELIVERY_BOUND_CONTENT_KEY, + deprioritized_keys=_DELIVERY_BOUND_DEPRIORITIZED_KEYS, + priority_keys=_DELIVERY_BOUND_PRIORITY_KEYS, + droppable_keys=_DELIVERY_BOUND_DROPPABLE_KEYS, + reason="delivery_bound", + top_level_flag="delivery_bound_spill", + ) + + def _spill_for_delivery_bound( result: Any, *, @@ -758,6 +1065,7 @@ def enforce_response_budget( metadata=metadata, max_bytes=projection_max_bytes, inline_chars=projection_inline_chars, + delivery_bound_triggered=over_delivery_bound, ) if rendered is None and not isinstance(parsed, dict): rendered = _plain_spill_envelope( diff --git a/src/autoskillit/server/tools/_serve_helpers.py b/src/autoskillit/server/tools/_serve_helpers.py index c77fbcf84..711c4a216 100644 --- a/src/autoskillit/server/tools/_serve_helpers.py +++ b/src/autoskillit/server/tools/_serve_helpers.py @@ -9,6 +9,7 @@ from __future__ import annotations +import hashlib import json from pathlib import Path from typing import TYPE_CHECKING, Any @@ -16,11 +17,18 @@ from autoskillit.core import ( RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, RESPONSE_BACKSTOP_EXEMPTION_REGISTRY_DIGEST, + atomic_write, + get_logger, +) +from autoskillit.recipe import ( # noqa: F401 — canonical extractor reused by build_routing_edges_by_step default + _extract_routing_edges, + step_byte_ranges_from_yaml, ) if TYPE_CHECKING: from autoskillit.core import BackendCapabilities, CodingAgentBackend from autoskillit.pipeline.context import ToolContext + from autoskillit.recipe.schema import RecipeStep def build_backend_capabilities_map( @@ -71,7 +79,7 @@ def response_backstop_tool_meta( def render_served_response(payload: dict[str, Any]) -> str: """Render the authoritative pre-backstop response used by recipe serve tools.""" - return json.dumps(payload) + return json.dumps(payload, ensure_ascii=False, separators=(",", ":")) def build_open_kitchen_recipe_payload(result: dict[str, Any], *, version: str) -> dict[str, Any]: @@ -185,3 +193,526 @@ def serve_recipe( if ctx.recipes is None: raise RuntimeError("serve_recipe() called with ctx.recipes=None") return ctx.recipes.load_and_validate(name, ctx.project_dir, **kwargs) + + +# --------------------------------------------------------------------------- +# Bounded envelope (Part B of #4304) +# --------------------------------------------------------------------------- + + +def _step_one_line_summary(step: RecipeStep) -> str: + """Return a compact one-line summary of *step* for the envelope skeleton. + + Used by the open_kitchen / load_recipe integration paths to populate + ``step_summaries`` before calling ``extract_step_skeleton``. Prefers + an explicit ``description`` field; falls back to a 160-char head of + the step's ``message`` or a tool/action signature. Always single-line. + """ + desc = getattr(step, "description", "") or "" + if desc.strip(): + return desc.strip().splitlines()[0][:160] + msg = getattr(step, "message", None) + if isinstance(msg, str) and msg.strip(): + return msg.strip().splitlines()[0][:160] + tool = getattr(step, "tool", None) + action = getattr(step, "action", None) + if tool: + return f"tool={tool}" + if action: + return f"action={action}" + return "" + + +def _compute_step_byte_ranges(content: str) -> dict[str, tuple[int, int]]: + """Return ``{step_name: (start, end)}`` UTF-8 byte offsets within *content*. + + Thin wrapper over + :func:`autoskillit.recipe.step_byte_ranges_from_yaml` that keeps the + ``_serve_helpers``-local symbol stable for downstream callers and + tests. The yaml-handling and ``isinstance(..., MappingNode)`` + fail-open logic lives in ``core.io`` so that yaml imports are + confined to the single canonical module (the package-wide invariant + enforced by ``tests/arch/test_subpackage_isolation.py`` and + ``tests/core/test_io.py::TestYamlConsolidationArchitecture``). + """ + return step_byte_ranges_from_yaml(content) + + +def extract_step_skeleton( + post_prune_step_names: list[str], + routing_edges_by_step: dict[str, list[tuple[str, str]]], + step_summaries: dict[str, str] | None = None, + byte_ranges: dict[str, tuple[int, int]] | None = None, +) -> dict[str, Any]: + """Build a compact step-flow skeleton from parsed post-prune step data. + + The skeleton is a list of per-step dicts (name + one-line summary + + outgoing routing edges + optional byte range). Combined with the + byte-range index in the persisted artifact, the orchestrator can + route without pulling a full step body, and pull-on-demand only the + step it is about to execute. + + *post_prune_step_names* — order-preserving list of step names (from + ``load_and_validate``'s ``post_prune_step_names`` result field). + *routing_edges_by_step* — name → list of (edge_type, target) tuples + derived from ``_extract_routing_edges`` for each step. + *step_summaries* — optional name → one-line summary override. + *byte_ranges* — optional name → (start, end) UTF-8 byte offsets + within the persisted ``content`` field. When provided for a + given step, the skeleton's per-step entry gains a + ``byte_range`` field carrying ``[start, end]``; steps absent + from the map get no such key (fail-open, matching the + edges/summary fallback pattern above). + """ + skeleton: list[dict[str, Any]] = [] + for name in post_prune_step_names: + edges = routing_edges_by_step.get(name) or [] + summary = (step_summaries or {}).get(name) or "" + entry: dict[str, Any] = { + "name": name, + "summary": summary, + "edges": [ + {"type": edge_type, "target": target} for edge_type, target in edges if target + ], + } + span = (byte_ranges or {}).get(name) + if span is not None: + entry["byte_range"] = list(span) + skeleton.append(entry) + return { + "step_count": len(skeleton), + "steps": skeleton, + } + + +def build_recipe_envelope( + payload: dict[str, Any], + *, + recipe_name: str, + artifact_path: str, + artifact_sha256: str, + skeleton: dict[str, Any], + bound_bytes: int, + producer_tool: str = "open_kitchen", +) -> dict[str, Any]: + """Build a bounded envelope that fits the smallest backend delivery bound. + + The envelope is the orchestrator-visible replacement for the full + recipe payload when the payload exceeds a backend's effective delivery + token limit. It carries: + + - routing metadata (``success``, ``kitchen``, ``version``, ``valid``, + ``dispatch_feasible``); + - verbatim priority content (``orchestration_rules``, + ``stop_step_semantics``, ``errors``, ``warnings``, ``hooks``); + - ingredients schema (``ingredients_table``, ``suggestions``); + - the step-flow skeleton (post-prune step names, summaries, routing + edges) — enough for the orchestrator to route between steps without + pulling a full step body; + - the post-prune step list and routing edge list (so callers that + only need the routing graph don't need to parse the skeleton); + - a pull reference pointing to the deterministic artifact path + (overwritten by every open_kitchen / load_recipe call) and the + ``get_recipe_section`` MCP tool name. + + The envelope omits the full ``content`` field. The orchestrator pulls + each step's body via ``get_recipe_section(section=)`` at + execution time, bounded to the delivery limit and chunked with a + continuation token for steps larger than one chunk. + + ``bound_bytes`` is the effective delivery byte ceiling (smallest + backend bound × 4). Large string fields like ``orchestration_rules`` + are projected to fit alongside the skeleton + pull reference via a + two-phase allocator: phase 1 drops (omits, never emits as ``""``) + any key that can't even afford its JSON-key overhead under the + current ``remaining`` budget; phase 2 water-fills the leftover + content budget across survivors so neither priority field can + monopolize the pool. Truncation goes through ``_safe_utf8_truncate`` + so a multi-byte UTF-8 codepoint is never split into an + undecodable byte sequence. ``recipe_name`` is required (keyword-only) + to match the sibling-function convention in this module. + """ + envelope: dict[str, Any] = {} + envelope_bytes = 0 + serialized = json.dumps(payload.get("success", True)).encode("utf-8") + envelope_bytes += len(serialized) + envelope["success"] = payload.get("success", True) + for key in ( + "kitchen", + "version", + "valid", + "dispatch_feasible", + "errors", + "warnings", + "hooks", + "post_prune_step_names", + "post_prune_routing_edges", + "requires_packs", + "requires_features", + ): + if key in payload and payload[key] is not None: + envelope[key] = payload[key] + envelope_bytes += len( + json.dumps({key: payload[key]}, ensure_ascii=False).encode("utf-8") + ) + + # Fixed-size overhead: skeleton JSON + pull reference + delivery_bound_spill. + skeleton_overhead = len( + json.dumps({"step_flow_skeleton": skeleton}, ensure_ascii=False).encode("utf-8") + ) + pull_overhead = len( + json.dumps( + { + "recipe_pull": { + "recipe_name": recipe_name, + "producer_tool": producer_tool, + "artifact_path": artifact_path, + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + }, + "delivery_bound_spill": True, + }, + ensure_ascii=False, + ).encode("utf-8") + ) + + remaining = max( + 0, + bound_bytes - skeleton_overhead - pull_overhead - envelope_bytes - 64, + ) + + def _project_priority_strings(keys: tuple[str, ...]) -> None: + nonlocal remaining + candidates = [k for k in keys if isinstance(payload.get(k), str) and payload.get(k)] + if not candidates: + return + + overhead = { + k: len(json.dumps({k: ""}, ensure_ascii=False).encode("utf-8")) for k in candidates + } + # Phase 1: a key survives only if its JSON-key overhead plus at least + # one content byte fits what's left, checked in priority (declaration) + # order. Keys that don't clear this bar are omitted entirely — never + # emitted as "". + present: list[str] = [] + budget = remaining + for k in candidates: + if overhead[k] < budget: + present.append(k) + budget -= overhead[k] + if not present: + return + + # Phase 2: water-fill the leftover content budget evenly across + # survivors, round by round. A key whose value is shorter than its + # share drops out and its unused share carries over to the *other* + # survivors in the next round — no single key can monopolize the + # pool the way a sequential first-come-first-served allocator can. + content_budget = remaining - sum(overhead[k] for k in present) + lengths = {k: len(payload[k].encode("utf-8")) for k in present} + alloc: dict[str, int] = dict.fromkeys(present, 0) + active = list(present) + pool = content_budget + while active and pool > 0: + share, extra = divmod(pool, len(active)) + still_active: list[str] = [] + for index, key in enumerate(active): + give = share + (1 if index < extra else 0) + need = lengths[key] - alloc[key] + take = min(give, need, pool) + alloc[key] += take + pool -= take + if alloc[key] < lengths[key]: + still_active.append(key) + active = still_active + + for key in present: + take = alloc[key] + if take <= 0: + continue # no content budget survives for this field; omit it + value_bytes = payload[key].encode("utf-8") + if len(value_bytes) <= take: + envelope[key] = payload[key] + remaining -= len(value_bytes) + overhead[key] + else: + envelope[key] = _safe_utf8_truncate(value_bytes[:take]) + remaining -= take + overhead[key] + get_logger(__name__).warning( + "recipe_envelope_priority_field_truncated", + recipe_name=recipe_name, + field=key, + alloc_bytes=take, + ) + + _project_priority_strings(("orchestration_rules", "stop_step_semantics")) + + # ingredients_table and suggestions are deprioritized — serialize them + # only if budget allows; otherwise omit. The orchestrator can pull the + # full ingredients_table via ``get_recipe_section(section="ingredients_table")``. + for key in ("ingredients_table", "suggestions"): + value = payload.get(key) + if value is None: + continue + serialized_value = json.dumps(value, ensure_ascii=False).encode("utf-8") + if len(serialized_value) + 32 <= remaining: + envelope[key] = value + remaining -= len(serialized_value) + 32 + + envelope["step_flow_skeleton"] = skeleton + envelope["recipe_pull"] = { + "recipe_name": recipe_name, + "producer_tool": producer_tool, + "artifact_path": artifact_path, + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + } + envelope["delivery_bound_spill"] = True + if ( + len(json.dumps(envelope, ensure_ascii=False, separators=(",", ":")).encode("utf-8")) + <= bound_bytes + ): + return envelope + + fallback_candidates: tuple[dict[str, Any], ...] = ( + { + "success": payload.get("success", True), + "step_flow_skeleton": skeleton, + "recipe_pull": { + "recipe_name": recipe_name, + "producer_tool": producer_tool, + "artifact_path": artifact_path, + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + }, + "delivery_bound_spill": True, + }, + { + "success": False, + "error": "recipe_envelope_exceeds_delivery_bound", + "recipe_pull": { + "recipe_name": recipe_name, + "producer_tool": producer_tool, + "artifact_path": artifact_path, + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + }, + }, + {"success": False, "error": "recipe_envelope_exceeds_delivery_bound"}, + {}, + ) + for fallback in fallback_candidates: + if ( + len(json.dumps(fallback, ensure_ascii=False, separators=(",", ":")).encode("utf-8")) + <= bound_bytes + ): + return fallback + raise ValueError("delivery bound is too small for a JSON object") + + +def _safe_utf8_truncate(data: bytes) -> str: + """Decode *data* as UTF-8, backing off byte-by-byte from the end on failure. + + Handles both a dangling continuation byte and a dangling multi-byte lead + byte — the two ways a naive byte-index slice can land mid-codepoint. + Used by the envelope priority-field allocator to guarantee that a + multi-byte codepoint is never split into an undecodable byte sequence. + """ + while data: + try: + return data.decode("utf-8") + except UnicodeDecodeError as exc: + data = data[: exc.start] + return "" + + +def persist_recipe_artifact( + artifact_dir: Path, + *, + tool_name: str, + recipe_name: str, + payload: dict[str, Any], +) -> tuple[str, str]: + """Atomically persist the full recipe payload to the deterministic path. + + Returns (artifact_path, sha256) for inclusion in the envelope's + ``recipe_pull`` block. Uses ``atomic_write`` so concurrent open_kitchen + / load_recipe calls do not see a half-written file. + + The path is deterministic per (tool, recipe_name) so the pull tool + can reconstruct it from ``tool_ctx.recipe_name`` without the caller + having to thread it through every surface. Re-opening the same recipe + overwrites the artifact (idempotent). + """ + from autoskillit.server._response_budget import _recipe_artifact_path # circular-break + + path = _recipe_artifact_path(artifact_dir, tool_name, recipe_name) + serialized = json.dumps(payload, ensure_ascii=False) + atomic_write(path, serialized) + sha256 = hashlib.sha256(serialized.encode("utf-8")).hexdigest() + return str(path.resolve()), sha256 + + +def build_step_summaries(active_recipe_steps: Any) -> dict[str, str]: + """Build a {step_name: one_line_summary} dict from the parsed Recipe.steps. + + Falls back to an empty string per step if the parsed structure is + unavailable (e.g. the recipe was loaded but post-prune filtering + stripped the steps out, or the serve path passed no Recipe object). + """ + if not isinstance(active_recipe_steps, dict) or not active_recipe_steps: + return {} + summaries: dict[str, str] = {} + for name, step in active_recipe_steps.items(): + if not isinstance(name, str) or not name: + continue + summaries[name] = _step_one_line_summary(step) + return summaries + + +def build_routing_edges_by_step( + active_recipe_steps: Any, + *, + edge_extractor: Any = _extract_routing_edges, +) -> dict[str, list[tuple[str, str]]]: + """Build a {step_name: [(edge_type, target), ...]} dict via edge_extractor. + + ``edge_extractor`` is the existing ``_extract_routing_edges`` callable + from ``recipe/_analysis_graph.py``. We avoid importing it here to keep + the helper importable from lightweight contexts; the caller passes the + callable in. Steps whose extractor returns nothing map to an empty + list (not omitted) so callers don't have to ``.get(name) or []``. + """ + if not isinstance(active_recipe_steps, dict) or not active_recipe_steps: + return {} + edges_by_step: dict[str, list[tuple[str, str]]] = {} + for name, step in active_recipe_steps.items(): + if not isinstance(name, str) or not name: + continue + extracted = edge_extractor(step) if edge_extractor is not None else [] + edges_by_step[name] = [ + (edge.edge_type, edge.target) + for edge in (extracted or []) + if getattr(edge, "target", None) + ] + return edges_by_step + + +def maybe_envelope_recipe_response( + payload: dict[str, Any], + *, + tool_name: str, + recipe_name: str, + tool_ctx: ToolContext, + effective_delivery_token_limit: int | None, +) -> dict[str, Any]: + """Conditionally replace a recipe payload with a bounded envelope. + + Persists the full payload to the deterministic artifact path + UNCONDITIONALLY (gated only by ``temp_dir`` being a ``Path``), so the + ``get_recipe_section`` pull tool always has a backing store. If the + payload's estimated token count exceeds + ``effective_delivery_token_limit``, returns ``build_recipe_envelope(...)`` + so the orchestrator can pull each step's body on demand. + + Otherwise returns the payload unchanged (Claude backend path: the + full payload fits inline; backward compatible) — with the artifact + already on disk for the pull tool. + + On persistence failure: returns the original payload unchanged — + the caller is then subject to ``track_response_size``'s spill path + (which is more permissive but loses the pull guarantee). This is + a fail-open at the persistence layer; the spill path itself remains + fail-closed for shape violations. + + Persistence happens exactly once per call: the single early call's + return value is reused by ``build_recipe_envelope`` via its + ``artifact_path`` / ``artifact_sha256`` keyword args. + """ + artifact_dir = tool_ctx.temp_dir + if not isinstance(artifact_dir, Path): + return payload + + try: + artifact_path, artifact_sha256 = persist_recipe_artifact( + artifact_dir, + tool_name=tool_name, + recipe_name=recipe_name, + payload=payload, + ) + except OSError: + return payload + + if effective_delivery_token_limit is None or effective_delivery_token_limit <= 0: + return payload + + serialized = json.dumps(payload, ensure_ascii=False) + estimated_tokens = (len(serialized.encode("utf-8")) + 3) // 4 + if estimated_tokens <= effective_delivery_token_limit: + return payload + + # existing skeleton/edges construction (post_prune_names, summaries, + # edges via build_routing_edges_by_step) feeds extract_step_skeleton + # plus the new byte-range field computed from the persisted + # ``content`` payload field. + post_prune_raw = payload.get("post_prune_step_names") or [] + if not isinstance(post_prune_raw, list): + post_prune_raw = [] + post_prune_names = [n for n in post_prune_raw if isinstance(n, str)] + # ``active_recipe_steps`` is the kitchen's currently-open recipe — NOT + # necessarily the recipe being delivered here. When the caller + # (e.g. ``load_recipe``) targets a different recipe than the active + # one, mixing the other recipe's parsed step summary/edges into the + # skeleton bloats the envelope past the delivery bound even for a + # small payload. Use the active recipe's parsed steps only when it + # matches the payload's recipe_name; otherwise emit a name-only + # skeleton (orchestrator still has the artifact + pull reference). + active_recipe_steps: dict[str, Any] | None = None + active_recipe_name = getattr(tool_ctx, "recipe_name", "") or "" + if active_recipe_name and active_recipe_name == recipe_name: + active_recipe_steps = tool_ctx.active_recipe_steps + summaries = build_step_summaries(active_recipe_steps) + edges = build_routing_edges_by_step(active_recipe_steps) + byte_ranges = _compute_step_byte_ranges(payload.get("content") or "") + bound_bytes = effective_delivery_token_limit * 4 + skeleton = extract_step_skeleton(post_prune_names, edges, summaries, byte_ranges=byte_ranges) + + def _pullable_skeleton_size(candidate: dict[str, Any]) -> int: + pullable = { + "success": payload.get("success", True), + "step_flow_skeleton": candidate, + "recipe_pull": { + "recipe_name": recipe_name, + "producer_tool": tool_name, + "artifact_path": artifact_path, + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + }, + "delivery_bound_spill": True, + } + return len(json.dumps(pullable, ensure_ascii=False, separators=(",", ":")).encode("utf-8")) + + if _pullable_skeleton_size(skeleton) > bound_bytes: + for summary_limit in (120, 80, 64, 48, 32, 24, 16, 8, 0): + bounded_summaries = ( + {name: summary[:summary_limit] for name, summary in summaries.items()} + if summary_limit + else {} + ) + skeleton = extract_step_skeleton( + post_prune_names, + edges, + bounded_summaries, + byte_ranges=byte_ranges, + ) + if _pullable_skeleton_size(skeleton) <= bound_bytes: + break + + return build_recipe_envelope( + payload, + artifact_path=artifact_path, + artifact_sha256=artifact_sha256, + skeleton=skeleton, + bound_bytes=bound_bytes, + recipe_name=recipe_name, + producer_tool=tool_name, + ) diff --git a/src/autoskillit/server/tools/tools_kitchen.py b/src/autoskillit/server/tools/tools_kitchen.py index c572b2180..d6d67d2d0 100644 --- a/src/autoskillit/server/tools/tools_kitchen.py +++ b/src/autoskillit/server/tools/tools_kitchen.py @@ -27,6 +27,7 @@ from autoskillit.core import ( DISPATCH_ID_ENV_VAR, PIPELINE_FORBIDDEN_TOOLS, + BackendCapabilities, CapabilityResolutionDetail, ProcessStaleError, _collect_disabled_feature_tags, @@ -42,6 +43,7 @@ read_active_kitchens_registry, read_marker, register_active_kitchen, + resolve_effective_delivery_bound, resolve_kitchen_id, sweep_stale_markers, unregister_active_kitchen, @@ -85,6 +87,7 @@ from autoskillit.server.tools._serve_helpers import ( build_backend_capabilities_map, build_open_kitchen_recipe_payload, + maybe_envelope_recipe_response, render_served_response, response_backstop_tool_meta, serve_recipe, @@ -1059,6 +1062,28 @@ async def open_kitchen( if overrides is not None: tool_ctx.session_serve_overrides = dict(overrides) tool_ctx.session_serve_defer_unresolved = not bool(overrides) + if not ingredients_only: + _backend_caps = ( + tool_ctx.backend.capabilities + if tool_ctx.backend is not None + and isinstance( + getattr(tool_ctx.backend, "capabilities", None), + BackendCapabilities, + ) + else None + ) + _edtl = ( + resolve_effective_delivery_bound(_backend_caps) + if _backend_caps is not None + else None + ) + result = maybe_envelope_recipe_response( + result, + tool_name="open_kitchen", + recipe_name=name, + tool_ctx=tool_ctx, + effective_delivery_token_limit=_edtl, + ) return render_served_response(result) try: result = serve_recipe( @@ -1211,6 +1236,29 @@ async def open_kitchen( ) return _validation_err + if not ingredients_only: + _backend_caps_normal = ( + tool_ctx.backend.capabilities + if tool_ctx.backend is not None + and isinstance( + getattr(tool_ctx.backend, "capabilities", None), + BackendCapabilities, + ) + else None + ) + _edtl_normal = ( + resolve_effective_delivery_bound(_backend_caps_normal) + if _backend_caps_normal is not None + else None + ) + result = maybe_envelope_recipe_response( + result, + tool_name="open_kitchen", + recipe_name=name, + tool_ctx=tool_ctx, + effective_delivery_token_limit=_edtl_normal, + ) + return render_served_response(result) text = ( diff --git a/src/autoskillit/server/tools/tools_recipe.py b/src/autoskillit/server/tools/tools_recipe.py index 82cb6d777..a862f2c0b 100644 --- a/src/autoskillit/server/tools/tools_recipe.py +++ b/src/autoskillit/server/tools/tools_recipe.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import json from pathlib import Path from typing import Any @@ -10,12 +11,20 @@ from fastmcp import Context from fastmcp.dependencies import CurrentContext +from autoskillit import __version__ from autoskillit.config import ( build_config_authoritative_layer, build_config_default_layer, resolve_ingredient_defaults, ) -from autoskillit.core import FLEET_DISPATCH_TOOLS, get_logger, temp_dir_display_str # noqa: F401 +from autoskillit.core import ( + BackendCapabilities, + fast_dumps, + get_logger, + load_yaml, + resolve_effective_delivery_bound, + temp_dir_display_str, +) # noqa: F401 from autoskillit.pipeline import GATED_TOOLS, UNGATED_TOOLS # noqa: F401 from autoskillit.server import mcp from autoskillit.server._guards import _require_enabled @@ -35,6 +44,9 @@ from autoskillit.server.tools._cancellation_shield import _cancellation_shield from autoskillit.server.tools._serve_helpers import ( build_backend_capabilities_map, + build_open_kitchen_recipe_payload, + maybe_envelope_recipe_response, + persist_recipe_artifact, render_served_response, response_backstop_tool_meta, serve_recipe, @@ -44,6 +56,69 @@ logger = get_logger(__name__) +class _RecipeSectionError(Exception): + """Structured artifact failure surfaced by ``get_recipe_section``.""" + + def __init__(self, code: str, detail: str) -> None: + super().__init__(detail) + self.code = code + + +def _bounded_recipe_section_response( + section: str, content: str, *, part: int, bound_bytes: int +) -> str: + """Render one continuation chunk whose serialized UTF-8 size fits the bound.""" + + def _render(start: int, end: int) -> str: + has_more = end < len(content) + response: dict[str, Any] = { + "success": True, + "section": section, + "content": content[start:end], + "has_more": has_more, + } + if has_more: + response["next_part"] = chunk_index + 1 + response["total_size"] = len(content) + return json.dumps(response, ensure_ascii=False) + + start = 0 + for chunk_index in range(part + 1): + full = _render(start, len(content)) + if len(full.encode("utf-8")) <= bound_bytes: + end = len(content) + rendered = full + else: + low = start + 1 + high = len(content) - 1 + end = start + rendered = "" + while low <= high: + candidate_end = (low + high) // 2 + candidate = _render(start, candidate_end) + if len(candidate.encode("utf-8")) <= bound_bytes: + end = candidate_end + rendered = candidate + low = candidate_end + 1 + else: + high = candidate_end - 1 + if end == start: + return json.dumps({"success": False, "error": "recipe_section_bound_too_small"}) + if chunk_index == part: + return rendered + start = end + if start >= len(content): + return json.dumps( + { + "success": True, + "section": section, + "content": "", + "has_more": False, + } + ) + raise AssertionError("continuation loop must return") + + @mcp.tool( tags={"autoskillit", "kitchen-core", "fleet-dispatch"}, annotations={"readOnlyHint": True}, @@ -324,12 +399,351 @@ async def load_recipe( stage="validate_result", ) return _validation_err + if not ingredients_only: + _lr_backend_caps = ( + tool_ctx.backend.capabilities + if tool_ctx.backend is not None + and isinstance( + getattr(tool_ctx.backend, "capabilities", None), + BackendCapabilities, + ) + else None + ) + _lr_edtl = ( + resolve_effective_delivery_bound(_lr_backend_caps) + if _lr_backend_caps is not None + else None + ) + result = maybe_envelope_recipe_response( + result, + tool_name="load_recipe", + recipe_name=name, + tool_ctx=tool_ctx, + effective_delivery_token_limit=_lr_edtl, + ) return render_served_response(result) except Exception as exc: logger.error("load_recipe unhandled exception", exc_info=True) return json.dumps({"success": False, "error": f"{type(exc).__name__}: {exc}"}) +@mcp.tool(tags={"autoskillit", "kitchen", "kitchen-core"}, annotations={"readOnlyHint": True}) +@_cancellation_shield() +@track_response_size("get_recipe_section") +async def get_recipe_section( + section: str, + part: int = 0, + recipe_name: str | None = None, + producer_tool: str | None = None, + artifact_path: str | None = None, + artifact_sha256: str | None = None, +) -> str: + """Retrieve a recipe step or section from the persisted recipe artifact. + + Returns the body for the named step or section, bounded to the + effective delivery limit. The body is the same YAML that + ``open_kitchen`` / ``load_recipe`` would have returned inline — the + envelope omits ``content`` and the pull tool serves it on demand. + + For sections larger than the bound, use ``part=1``, ``part=2``, + etc. to retrieve continuation chunks. The response includes + ``has_more=True`` and ``next_part=N`` when more chunks remain. + + Args: + section: The step or section name to retrieve. Must match a + ``post_prune_step_names`` entry from the envelope, or the + special values ``"content"`` (full raw recipe YAML), + ``"ingredients_table"``, or ``"orchestration_rules"``. + part: Continuation index (0-based). Default 0 returns the first + chunk; pass the value from the previous response's + ``next_part`` to retrieve the next chunk. + recipe_name: Recipe identity copied from the envelope's + ``recipe_pull.recipe_name`` field. + producer_tool: Producer identity copied from the envelope's + ``recipe_pull.producer_tool`` field. + artifact_sha256: Artifact digest copied from the envelope's + ``recipe_pull.sha256`` field. Required to bind the pull to + the exact composition that produced the envelope. + + Returns: + JSON with ``success``, ``section``, ``content``, ``has_more``, + and (when more chunks remain) ``next_part``. + + This tool requires the kitchen to be open (gated by open_kitchen). + + Never raises. + """ + if (gate := _require_enabled()) is not None: + return gate + try: + with structlog.contextvars.bound_contextvars(tool="get_recipe_section"): + tool_ctx = _get_ctx_or_none() + if tool_ctx is None or tool_ctx.recipes is None: + return json.dumps({"success": False, "error": "kitchen not open"}) + + if not recipe_name or producer_tool is None or not artifact_sha256: + return json.dumps({"success": False, "error": "recipe_artifact_identity_required"}) + requested_recipe_name = recipe_name + + if producer_tool not in {"open_kitchen", "load_recipe"}: + return json.dumps({"success": False, "error": "invalid_recipe_artifact_identity"}) + + artifact_dir = getattr(tool_ctx, "temp_dir", None) + if not isinstance(artifact_dir, Path): + return json.dumps({"success": False, "error": "kitchen temp_dir not available"}) + + from autoskillit.server._response_budget import _recipe_artifact_path # circular-break + + resolved_artifact_path = _recipe_artifact_path( + artifact_dir, producer_tool, requested_recipe_name + ) + if ( + artifact_path is not None + and Path(artifact_path).resolve() != resolved_artifact_path.resolve() + ): + return json.dumps({"success": False, "error": "invalid_recipe_artifact_identity"}) + + if not resolved_artifact_path.exists(): + if producer_tool != "open_kitchen": + return json.dumps({"success": False, "error": "recipe_artifact_unavailable"}) + # Recreation path: re-invoke the same serve pipeline that + # built the artifact originally. This handles the case + # where the artifact was pruned/garbage-collected between + # open_kitchen and get_recipe_section calls. Use the + # session_serve_overrides snapshot to preserve idempotence + # — re-serving with the same overrides must produce the + # same content (issue #4208 hardening). + _recreate_envelope_err = None + try: + _defaults = resolve_ingredient_defaults(tool_ctx.project_dir) + _config_layer = build_config_authoritative_layer(_defaults) + _config_default = build_config_default_layer(_defaults) + _session_overrides: dict[str, str] = { + "kitchen_id": tool_ctx.kitchen_id, + "diagnostics_log_dir": str( + resolve_log_dir(tool_ctx.config.linux_tracing.log_dir) + ), + } + _caller_overrides = ( + dict(tool_ctx.session_serve_overrides) + if tool_ctx.session_serve_overrides is not None + else None + ) + _recreate = serve_recipe( + tool_ctx, + requested_recipe_name, + caller_overrides=_caller_overrides, + config_default=_config_default, + session_overrides=_session_overrides, + config_layer=_config_layer, + resolved_defaults=_defaults, + ingredients_only=False, + ) + if not _recreate.get("valid", False): + return json.dumps( + { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": "recreation returned invalid recipe", + } + ) + _recreate = build_open_kitchen_recipe_payload(_recreate, version=__version__) + + try: + persist_recipe_artifact( + artifact_dir, + tool_name=producer_tool, + recipe_name=requested_recipe_name, + payload=_recreate, + ) + except OSError: + return json.dumps( + { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": "recreation write failed", + } + ) + except Exception as exc: + logger.warning( + "get_recipe_section_recreate_failed", + recipe_name=requested_recipe_name, + exc_info=True, + ) + _recreate_envelope_err = f"{type(exc).__name__}: {exc}" + if _recreate_envelope_err is not None: + return json.dumps( + { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": _recreate_envelope_err, + } + ) + + # Read the persisted full payload. The pull tool returns a + # bounded chunk from the requested section of the recipe YAML. + try: + persisted_raw = resolved_artifact_path.read_text(encoding="utf-8") + if hashlib.sha256(persisted_raw.encode("utf-8")).hexdigest() != artifact_sha256: + return json.dumps( + {"success": False, "error": "invalid_recipe_artifact_identity"} + ) + persisted = json.loads(persisted_raw) + except (OSError, json.JSONDecodeError) as exc: + return json.dumps( + { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": f"{type(exc).__name__}: {exc}", + } + ) + if not isinstance(persisted, dict): + return json.dumps( + { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": "persisted recipe artifact is not a mapping", + } + ) + + content = "" + if section == "content": + content = persisted.get("content", "") or "" + elif section == "ingredients_table": + _it = persisted.get("ingredients_table") + content = json.dumps(_it) if _it is not None else "" + elif section == "orchestration_rules": + content = persisted.get("orchestration_rules", "") or "" + elif section == "stop_step_semantics": + content = persisted.get("stop_step_semantics", "") or "" + elif section == "errors": + content = json.dumps(persisted.get("errors", []) or []) + elif section == "warnings": + content = json.dumps(persisted.get("warnings", []) or []) + else: + if not _is_post_prune_step(persisted, section): + return json.dumps( + { + "success": False, + "error": "section_not_found", + "section": section, + } + ) + # Treat as a step name; extract the step definition from + # the post-prune step slice of the YAML content. The + # full recipe YAML uses the step name as a key; we parse + # the YAML and pull out only the requested step's subtree. + try: + content = _extract_step_body_from_persisted(persisted, section) + except _RecipeSectionError as exc: + return json.dumps( + { + "success": False, + "error": exc.code, + "detail": str(exc), + } + ) + + if not content: + return json.dumps( + { + "success": False, + "error": "section_not_found", + "section": section, + } + ) + + # Bound the chunk size to the effective delivery limit + # (smallest backend bound by construction) so the response + # itself never re-enters the spill path. ``track_response_size`` + # still enforces the response-side ceiling, but bounding + # here means a single response never accidentally exceeds + # even the Codex 40KB bound. + _backend_caps = ( + tool_ctx.backend.capabilities + if tool_ctx.backend is not None + and isinstance( + getattr(tool_ctx.backend, "capabilities", None), + BackendCapabilities, + ) + else None + ) + bound_tokens = ( + resolve_effective_delivery_bound(_backend_caps) + if _backend_caps is not None + else 10_000 + ) + bound_bytes = bound_tokens * 4 + + if part < 0: + part = 0 + return _bounded_recipe_section_response( + section, content, part=part, bound_bytes=bound_bytes + ) + except Exception as exc: + logger.error("get_recipe_section unhandled exception", exc_info=True) + return json.dumps({"success": False, "error": f"{type(exc).__name__}: {exc}"}) + + +def _is_post_prune_step(persisted: dict[str, Any], section: str) -> bool: + """Return whether ``section`` is an executable step in the persisted payload.""" + post_prune_step_names = persisted.get("post_prune_step_names") + if not isinstance(post_prune_step_names, list): + return False + return any(name == section for name in post_prune_step_names if isinstance(name, str)) + + +def _extract_step_body_from_persisted(persisted: dict[str, Any], step_name: str) -> str: + """Extract a single step's YAML subtree from the persisted full payload. + + The persisted payload's ``content`` field is the full recipe YAML + rendered as a string (from ``load_and_validate``). We re-parse it + via ``load_yaml`` to access the structured steps dict, then return + only the requested step's sub-mapping serialized back to YAML. + Returns an empty string only when the step is not present. Artifact parse + and section serialization failures raise ``_RecipeSectionError`` so the + caller can distinguish them from an absent section. + """ + content = persisted.get("content", "") or "" + if not content or not step_name: + return "" + try: + parsed = load_yaml(content) + except Exception as exc: + logger.warning( + "get_recipe_section_step_yaml_parse_failed", + step_name=step_name, + exc_info=True, + ) + raise _RecipeSectionError( + "recipe_artifact_parse_failed", f"{type(exc).__name__}: {exc}" + ) from exc + if not isinstance(parsed, dict): + raise _RecipeSectionError( + "recipe_artifact_parse_failed", "recipe content is not a mapping" + ) + steps = parsed.get("steps") + if not isinstance(steps, dict): + raise _RecipeSectionError("recipe_artifact_parse_failed", "recipe steps are not a mapping") + step_obj = steps.get(step_name) + if step_obj is None: + return "" + if not isinstance(step_obj, dict): + return str(step_obj) + # Render just this step's subtree as compact YAML. + try: + return fast_dumps({step_name: step_obj}) + except Exception as exc: + logger.warning( + "get_recipe_section_step_yaml_serialize_failed", + step_name=step_name, + exc_info=True, + ) + raise _RecipeSectionError( + "recipe_section_serialization_failed", f"{type(exc).__name__}: {exc}" + ) from exc + + @mcp.tool(tags={"autoskillit", "kitchen", "kitchen-core"}, annotations={"readOnlyHint": True}) @_cancellation_shield() @track_response_size("validate_recipe") diff --git a/src/autoskillit/skills/sous-chef/SKILL.md b/src/autoskillit/skills/sous-chef/SKILL.md index e5b37bf0e..211eecb8e 100644 --- a/src/autoskillit/skills/sous-chef/SKILL.md +++ b/src/autoskillit/skills/sous-chef/SKILL.md @@ -970,6 +970,13 @@ resolved state. The `load_recipe` tool is the ONLY authoritative source of recip it runs the full composition pipeline (sub-recipe merging, skip-guard resolution, hidden ingredient interpolation) before serving content to you. +When `load_recipe`/`open_kitchen` return a bounded envelope (large recipes exceeding +the backend's delivery bound), the envelope omits full step bodies. Call +`get_recipe_section(section=)` to pull the body of a specific step on +demand — this is still the authoritative channel (backed by the same persisted +artifact `load_recipe` wrote), not a raw-file read. For sections spanning multiple +chunks, follow `has_more`/`next_part` until exhausted. + Similarly, NEVER read SKILL.md files directly from the filesystem. Use the Skill tool to load skill instructions — it applies runtime transformations (namespace rewriting, temp directory substitution, disable-model-invocation injection) that raw files lack. diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 7a53d3eeb..a385c1409 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -475,7 +475,7 @@ class ImportContext(enum.StrEnum): "_analysis_bfs": frozenset({"recipe"}), "_analysis_blocks": frozenset({"recipe"}), "_analysis_detectors": frozenset({"recipe"}), - "_analysis_graph": frozenset({"recipe"}), + "_analysis_graph": frozenset({"recipe", "server"}), "_git_helpers": frozenset({"recipe"}), "_skill_helpers": frozenset({"recipe"}), "_skill_placeholder_parser": frozenset( @@ -653,6 +653,8 @@ class ImportContext(enum.StrEnum): "cli", "fleet", "infra/test_pretty_output_hook_infra.py", + # file-level: Part C envelope-fit test imports execution.backends.BACKEND_REGISTRY + "infra/test_pretty_output_recipe.py", "_llm_triage", "smoke_utils", } @@ -785,6 +787,7 @@ class ImportContext(enum.StrEnum): "server/test_admission_dispatch_agreement.py", "server/test_pipeline_deps_derivation.py", "server/test_pipeline_tracker.py", + "server/test_tools_recipe_pull.py", # CLI file-level entries (6 of 38 import autoskillit.recipe): "cli/test_cli_prompts.py", "cli/test_l3_orchestrator_prompt.py", diff --git a/tests/arch/test_canonical_constant_consumption.py b/tests/arch/test_canonical_constant_consumption.py index 58a1a6d9c..dd84ed0f9 100644 --- a/tests/arch/test_canonical_constant_consumption.py +++ b/tests/arch/test_canonical_constant_consumption.py @@ -85,6 +85,12 @@ def test_env_forward_constants_have_production_consumer() -> None: "test_lifespan_fleet_boot.py for fleet tool-tag parity guards; " "no runtime production consumer needed" ), + "FLEET_DISPATCH_TOOLS": ( + "alias-derived: subset of GATED_TOOLS exposed as a separate constant " + "for session-type visibility dispatch; production consumers access " + "it via GATED_TOOLS membership (see test_canonical_constant_consumption.py " + "test_fleet_dispatch_tools_subset_of_gated_tools)" + ), } diff --git a/tests/arch/test_pyright_suppression_allowlist.py b/tests/arch/test_pyright_suppression_allowlist.py index 2f32e59c6..7adcb6b88 100644 --- a/tests/arch/test_pyright_suppression_allowlist.py +++ b/tests/arch/test_pyright_suppression_allowlist.py @@ -20,7 +20,7 @@ PRODUCTION_ALLOWLIST: dict[tuple[str, int], str] = { ( "recipe/__init__.py", - 278, + 283, ): "lazy-registry: method added by _register_rule_module() side effects", ("recipe/_api.py", 286): "lazy-registry: RULE_REGISTRY_HASH set by _finalize_registry()", } diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 11852b50b..8ab6f62d4 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -991,7 +991,7 @@ def test_data_directories_are_not_python_packages() -> None: "closure-scoped _spawn_error, and _write_pid fail-closed contract add ~33 lines", ), "tools_kitchen.py": ( - 1610, + 1660, "REQ-CNST-010-E7: kitchen tool handlers — open_kitchen and lock_ingredients require " "inline validation helpers (_check_override_keys, _build_ingredient_key_suggestions) " "for ingredient key validation; splitting would cross import-layer boundaries; " @@ -1030,7 +1030,10 @@ def test_data_directories_are_not_python_packages() -> None: "(+21 net lines); response artifact temp-root bridge (+2 net lines)" "; prune_stale_kitchen_state liveness-gated tracker pruning wired into both " "fresh-open and deferred-recall open_kitchen paths, plus overlay lock sidecar " - "cleanup at close_kitchen (#4293 pipeline tracker split-brain, +42 net lines)", + "cleanup at close_kitchen (#4293 pipeline tracker split-brain, +42 net lines)" + "; envelope integration on both deferred-recall and normal open_kitchen paths: " + "resolve_effective_delivery_bound + BackendCapabilities isinstance guard + " + "maybe_envelope_recipe_response call (#4304 Part B, +24 net lines)", ), "tools_execution.py": ( 1650, @@ -1115,6 +1118,18 @@ def test_data_directories_are_not_python_packages() -> None: "in _skill_placeholder_parser.py and re-used by both rules_skill_content.py " "and the tests/skills/ contract linters (+~60 net lines)", ), + "_response_budget.py": ( + 1500, + "REQ-CNST-010-E12: lossless response spill — atomic_write, projection " + "(uniform and tiered), exact canonical projection finalization, " + "measured exemptions, both exempted and non-exempted spill paths, " + "spill metadata schema, bounded-failure rendering, and shared " + "_tiered_projection helper for the exempted (recipe/load_recipe) and " + "non-exempted (run_skill) spill paths (issue #4304 priority-tier " + "delivery-bound summary); splitting would scatter the priority-tier " + "algorithm across modules that must remain the single source of " + "truth for the bound-vs-deprioritized budget allocation order.", + ), } diff --git a/tests/contracts/test_delivery_bound_fitness.py b/tests/contracts/test_delivery_bound_fitness.py index 010681005..396ca2201 100644 --- a/tests/contracts/test_delivery_bound_fitness.py +++ b/tests/contracts/test_delivery_bound_fitness.py @@ -15,18 +15,27 @@ import json from pathlib import Path +from typing import cast import pytest from autoskillit.config import OutputBudgetConfig -from autoskillit.core import RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, resolve_effective_delivery_bound -from autoskillit.execution.backends import BACKEND_REGISTRY +from autoskillit.core import ( + RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, + BackendCapabilities, + resolve_effective_delivery_bound, +) +from autoskillit.execution.backends import BACKEND_REGISTRY, CODEX_TOOL_OUTPUT_TOKEN_LIMIT from autoskillit.recipe import all_validated_recipe_names, load_and_validate from autoskillit.server._response_budget import ( RESPONSE_SPILL_METADATA_KEY, enforce_response_budget, ) -from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload +from autoskillit.server.tools._serve_helpers import ( + build_open_kitchen_recipe_payload, + build_recipe_envelope, + extract_step_skeleton, +) pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] @@ -70,11 +79,73 @@ def _full_open_kitchen_payload(recipe_name: str) -> dict[str, object]: @pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) -def test_bundled_recipe_open_kitchen_fits_or_spills_per_backend( +@pytest.mark.parametrize("backend_name", sorted(_backend_capabilities().keys()), ids=lambda n: n) +def test_bundled_recipe_open_kitchen_envelope_fits_per_backend( + recipe_name: str, backend_name: str, tmp_path: Path +) -> None: + """System-level fitness contract (issue #4304 Part B REQ-B-T7): the bounded + envelope built unconditionally for every bundled recipe via + ``build_recipe_envelope`` must fit every registered backend's effective + delivery bound by construction — independent of whether the raw + ``open_kitchen`` payload itself happens to fit today. + + This test mirrors the unit-level invariant already exercised by + ``test_envelope_fits_every_backend_by_construction`` in + ``tests/server/test_tools_recipe_pull.py`` at the contracts layer: the + two layers overlap in scope by design (test-pyramid convention) and the + envelope-fit-by-construction guarantee is the post-#4304-Part-B invariant + that this file exists to defend. + """ + payload = _full_open_kitchen_payload(recipe_name) + step_names = [ + str(n) + for n in cast(list[object], payload.get("post_prune_step_names") or []) + if isinstance(n, str) + ] + skeleton = extract_step_skeleton( + step_names, + routing_edges_by_step={}, + step_summaries={name: f"summary-{name}" for name in step_names}, + ) + artifact_path = tmp_path / f"{recipe_name}.log" + sha256 = "0" * 64 + + caps = _backend_capabilities()[backend_name] + bound_tokens = resolve_effective_delivery_bound(caps) + bound_bytes = _effective_bound_bytes(bound_tokens) + envelope = build_recipe_envelope( + payload, + recipe_name=recipe_name, + artifact_path=str(artifact_path), + artifact_sha256=sha256, + skeleton=skeleton, + bound_bytes=bound_bytes, + ) + serialized = json.dumps(envelope, ensure_ascii=False) + assert len(serialized.encode("utf-8")) <= bound_bytes, ( + f"{backend_name}: envelope for {recipe_name} exceeds " + f"{bound_bytes} bytes (effective delivery bound)" + ) + assert envelope["recipe_pull"]["pull_tool"] == "get_recipe_section" + + +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_bundled_recipe_open_kitchen_raw_spill_projection_fits_per_backend( recipe_name: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """For each backend, the ``open_kitchen`` payload either fits the - effective delivery bound or spills to a projection that fits.""" + """Standalone regression guard for ``enforce_response_budget``'s generic spill + path. Independent of whether ``open_kitchen``/``load_recipe`` still reach it + in production for recipe payloads (Part B routes oversized recipes through + ``build_recipe_envelope`` instead), ``enforce_response_budget``'s projection + path remains valid coverage for the non-recipe response types that still + flow through the backstop. + + For every bundled recipe × registered backend: if the raw serialized + ``open_kitchen`` payload outgrows a backend's effective delivery bound, + ``enforce_response_budget`` must spill it to a projection that fits. When + the raw payload already fits, nothing more to verify here — envelope-fit + coverage lives in ``test_bundled_recipe_open_kitchen_envelope_fits_per_backend``. + """ monkeypatch.chdir(tmp_path) payload = _full_open_kitchen_payload(recipe_name) serialized = json.dumps(payload) @@ -141,3 +212,234 @@ def test_exemption_registry_tools_have_measured_ceiling() -> None: assert definition.max_chars > 0 assert definition.max_utf8_bytes > 0 assert definition.measurement_id, f"{tool_name} has no measurement_id" + + +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_delivery_bound_summary_carries_all_step_names( + recipe_name: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """When spilling occurs, ``content`` must be non-empty (issue #4304 + regression: the deprioritized ``suggestions`` key previously starved + ``content`` to ``""``). When the bound is large enough to admit the full + recipe body, every post-prune step name appears in the envelope. + + Note: the issue #4304 starvation bug had only two classes of effect — + (1) content was reduced to ``""``, and (2) step names disappeared because + they live inside ``content``. The first invariant is the primary + regression gate; the second (every step name visible) is a softer + property that depends on bound vs. recipe size. Recipes whose payload + materially exceeds the bound (e.g. ``research`` at ~106KB on the Codex + 40KB bound) inherently lose some step names to head-truncation. The + fix guarantees the *budget allocation order* is correct — priority + keys verbatim, deprioritized projected, content receives a guaranteed + floor (not zero) — not that the entire step graph is preserved at any + arbitrary bound. + """ + monkeypatch.chdir(tmp_path) + payload = _full_open_kitchen_payload(recipe_name) + serialized = json.dumps(payload) + serialized_bytes = len(serialized.encode("utf-8")) + step_names_raw = payload.get("post_prune_step_names") + step_names = [ + str(name) for name in cast(list[object], step_names_raw or []) if isinstance(name, str) + ] + for backend_name, caps in _backend_capabilities().items(): + bound_tokens = resolve_effective_delivery_bound(caps) + bound_bytes = _effective_bound_bytes(bound_tokens) + if serialized_bytes <= bound_bytes: + continue + result = enforce_response_budget( + serialized, + tool_name="open_kitchen", + artifact_dir=tmp_path / backend_name, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str), ( + f"{backend_name}: expected str result for {recipe_name} payload" + ) + assert len(result.encode("utf-8")) <= bound_bytes, ( + f"{backend_name}: projection for {recipe_name} exceeds " + f"{bound_bytes} bytes (effective delivery bound)" + ) + data = json.loads(result) + content = data.get("content", "") + assert len(content) > 0, ( + f"{backend_name}: content starved to empty for {recipe_name} — " + f"delivery-bound summary bug regressed" + ) + if not step_names: + # Recipe's load path bypassed active_recipe composition (sub-recipe + # composition); post_prune_step_names is absent. Skip the + # step-name coverage assertion — the content > 0 invariant is + # the primary regression gate for issue #4304. + continue + assert "post_prune_routing_edges" in payload, ( + f"post_prune_routing_edges missing from open_kitchen payload for " + f"{recipe_name} — routing-edge coverage cannot be verified" + ) + routing_edges_raw = payload.get("post_prune_routing_edges") + routing_targets = [ + str(t) for t in cast(list[object], routing_edges_raw or []) if isinstance(t, str) + ] + envelope_text = json.dumps(data) + # Step names live inside ``content`` (the recipe body). When the bound + # is small relative to the recipe body, head-truncation inherently + # drops some step names — assert coverage proportional to how much of + # the body fits within the bound. This tolerance is NOT a flat + # percentage: it scales with (1 - coverage_ratio), so at small bounds + # against large recipe bodies the tolerated miss count can be a large + # fraction of all step names. The ``content > 0`` assertion above is + # the primary regression gate for issue #4304; this is a softer, + # best-effort coverage check. + full_body_chars = len(payload.get("content", "") or "") # type: ignore[arg-type] # TypedDict access + body_chars_in_envelope = len(content) + if full_body_chars <= body_chars_in_envelope: + coverage_ratio = 1.0 + else: + coverage_ratio = body_chars_in_envelope / full_body_chars + max_missing = max(1, int(round(len(step_names) * (1.0 - coverage_ratio) + 1))) + missing = [sn for sn in step_names if sn not in envelope_text] + assert len(missing) <= max_missing, ( + f"{backend_name}: {len(missing)} of {len(step_names)} step names " + f"missing from spilled envelope for {recipe_name} (body fit " + f"{coverage_ratio:.1%}, tolerated misses {max_missing}); first " + f"missing: {missing[:5]}" + ) + if routing_targets: + # Routing-edge targets live inside the same truncatable ``content`` + # field as step names, so they are subject to the identical + # proportional body-fit tolerance derived above. + max_missing_edges = max( + 1, int(round(len(routing_targets) * (1.0 - coverage_ratio) + 1)) + ) + missing_edges = [t for t in routing_targets if t not in envelope_text] + assert len(missing_edges) <= max_missing_edges, ( + f"{backend_name}: {len(missing_edges)} of {len(routing_targets)} " + f"routing edge targets missing from spilled envelope for " + f"{recipe_name} (body fit {coverage_ratio:.1%}, tolerated misses " + f"{max_missing_edges}); first missing: {missing_edges[:5]}" + ) + + +def test_codex_configured_limit_not_less_than_effective_delivery_bound() -> None: + """Cross-relational invariant: the configured Codex tool-output token + limit must not drift below the operative delivery bound, and must not + drift below the upstream Codex code-mode default output bound (~10,000 + tokens, per issue #4300).""" + codex_caps = BACKEND_REGISTRY["codex"]().capabilities + codex_effective = resolve_effective_delivery_bound(codex_caps) + assert codex_effective > 0 + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT >= codex_effective, ( + f"CODEX_TOOL_OUTPUT_TOKEN_LIMIT ({CODEX_TOOL_OUTPUT_TOKEN_LIMIT}) is " + f"below Codex effective delivery bound ({codex_effective}); config " + f"and capability field have drifted apart" + ) + # Upstream floor: Codex code-mode default ~10,000 tokens. + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT >= 10_000, ( + f"CODEX_TOOL_OUTPUT_TOKEN_LIMIT ({CODEX_TOOL_OUTPUT_TOKEN_LIMIT}) is " + f"below the upstream Codex code-mode default output bound (10,000); " + f"config value must not drift below this floor" + ) + + +def test_capability_default_uses_conservative_bound() -> None: + """BackendCapabilities() with no effective_delivery_token_limit set must + default to a conservative worst-case bound (the smallest registered + backend bound). The historical 0-sentinel silently disabled delivery + bounding for any future backend that omitted the field, leading to + unbounded transport delivery.""" + caps = BackendCapabilities() + bound = caps.effective_delivery_token_limit + assert bound > 0, ( + f"BackendCapabilities() default effective_delivery_token_limit must " + f"be conservative (non-zero); got {bound}" + ) + # The default must be at most the smallest registered backend bound, + # so the worst-case delivery is bounded to the strictest transport. + min_registered = min( + resolve_effective_delivery_bound(cls().capabilities) for cls in BACKEND_REGISTRY.values() + ) + assert bound <= min_registered, ( + f"BackendCapabilities() default ({bound}) must be at most the " + f"smallest registered backend bound ({min_registered}); a larger " + f"default would silently bypass the strictest transport" + ) + + +def test_non_exempted_delivery_bound_preserves_result_field(tmp_path: Path) -> None: + """A run_skill-shaped payload with a large ``result`` field must survive + projection when the payload exceeds the effective delivery bound. The + existing fitness test only checks projection size, not that ``result`` + survives — the same starvation defect as ``_delivery_bound_summary``'s + ``content`` bug applies to the sibling ``_project_json_object`` path.""" + payload = { + "success": True, + "kitchen": "open", + "version": "1.2.3", + "result": "x" * 100_000, + **{f"key_{index:03d}": "y" * 200 for index in range(15)}, + } + serialized = json.dumps(payload) + bound_tokens = 10_000 + bound_bytes = bound_tokens * 4 + assert len(serialized.encode("utf-8")) > bound_bytes + result = enforce_response_budget( + serialized, + tool_name="run_skill", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= bound_bytes, ( + f"projection for non-exempted payload exceeds {bound_bytes} bytes" + ) + data = json.loads(result) + assert "result" in data, ( + "result field dropped from non-exempted projection — content-equivalent " + "starvation defect (sibling of _delivery_bound_summary's content bug)" + ) + assert len(data["result"]) > 0, ( + f"result field starved to empty ({len(data['result'])} chars); " + f"non-exempted projection must protect the content-equivalent key" + ) + for key in ("success", "kitchen", "version"): + assert key in data, f"structural field {key!r} was dropped from the projection" + + +def test_non_exempted_delivery_bound_preserves_non_string_result(tmp_path: Path) -> None: + payload = { + "success": True, + "result": {"records": [{"value": "\u2603" * 500} for _ in range(80)]}, + } + serialized = json.dumps(payload) + bound_tokens = 500 + result = enforce_response_budget( + serialized, + tool_name="run_skill", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= bound_tokens * 4 + projected = json.loads(result) + assert projected["success"] is True + assert isinstance(projected["result"], dict) + + +def test_non_exempted_delivery_bound_finds_multibyte_result_prefix(tmp_path: Path) -> None: + payload = {"success": True, "result": "\u2603" * 100_000} + bound_tokens = 500 + result = enforce_response_budget( + json.dumps(payload, ensure_ascii=False), + tool_name="run_skill", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= bound_tokens * 4 + projected = json.loads(result) + assert projected["result"] diff --git a/tests/core/test_type_constants.py b/tests/core/test_type_constants.py index 58f701569..9bd72034a 100644 --- a/tests/core/test_type_constants.py +++ b/tests/core/test_type_constants.py @@ -32,14 +32,14 @@ def test_response_backstop_exemption_registry_is_closed_and_pinned() -> None: assert RESPONSE_BACKSTOP_EXEMPTION_REGISTRY == { "load_recipe": ResponseBackstopExemptionDef( - max_chars=185_000, - max_utf8_bytes=185_000, - measurement_id="bundled-recipes-all-modes-2026-07-16/load-recipe", + max_chars=188_000, + max_utf8_bytes=188_000, + measurement_id="bundled-recipes-all-modes-2026-07-21/load-recipe", ), "open_kitchen": ResponseBackstopExemptionDef( - max_chars=186_000, - max_utf8_bytes=186_000, - measurement_id="bundled-recipes-all-modes-2026-07-16/open-kitchen", + max_chars=188_000, + max_utf8_bytes=188_000, + measurement_id="bundled-recipes-all-modes-2026-07-21/open-kitchen", ), } @@ -60,7 +60,7 @@ def test_response_backstop_exemption_registry_digest_is_canonical() -> None: ) assert ( RESPONSE_BACKSTOP_EXEMPTION_REGISTRY_DIGEST - == "01c05239140445c277920d48b8df5745f5840fa22d97b3611bf6f37e8af5f127" + == "5acb77e003aa6cb54242ccf8dc6af2776f6a03fd5bcc993552ca71f8285d4842" ) diff --git a/tests/docs/test_doc_counts.py b/tests/docs/test_doc_counts.py index 1c3e8b311..98ac9b079 100644 --- a/tests/docs/test_doc_counts.py +++ b/tests/docs/test_doc_counts.py @@ -202,9 +202,9 @@ def _count_semantic_rule_files() -> int: # ----- tests ------------------------------------------------------------------ -def test_kitchen_tagged_tool_count_is_40() -> None: +def test_kitchen_tagged_tool_count_is_41() -> None: count = _count_kitchen_tools() - assert count == 40, f"Expected 40 kitchen-tagged tools; found {count}" + assert count == 41, f"Expected 41 kitchen-tagged tools; found {count}" def test_free_range_tool_count_is_17() -> None: @@ -297,8 +297,8 @@ def _assert_doc_states_number(doc: Path, label: str, expected: int) -> None: DOCS_DIR / "execution" / "tool-access.md", ], ) -def test_docs_state_59_mcp_tools(doc_path: Path) -> None: - _assert_doc_states_number(doc_path, "MCP tools", 59) +def test_docs_state_60_mcp_tools(doc_path: Path) -> None: + _assert_doc_states_number(doc_path, "MCP tools", 60) @pytest.mark.parametrize( @@ -308,8 +308,8 @@ def test_docs_state_59_mcp_tools(doc_path: Path) -> None: DOCS_DIR / "execution" / "tool-access.md", ], ) -def test_docs_state_39_kitchen_tools(doc_path: Path) -> None: - _assert_doc_states_number(doc_path, "kitchen tools", 39) +def test_docs_state_41_kitchen_tools(doc_path: Path) -> None: + _assert_doc_states_number(doc_path, "kitchen tools", 41) def test_skill_visibility_states_142_skills() -> None: diff --git a/tests/docs/test_output_budget_protocol_decision.py b/tests/docs/test_output_budget_protocol_decision.py index fa4d0888e..aa3d5e485 100644 --- a/tests/docs/test_output_budget_protocol_decision.py +++ b/tests/docs/test_output_budget_protocol_decision.py @@ -49,14 +49,14 @@ def test_decision_names_all_four_layers(decision_text: str, required: str) -> No @pytest.mark.parametrize( "required", [ - "max_chars = 185_000", - "max_utf8_bytes = 185_000", - "183,103 characters and UTF-8 bytes", - "max_chars = 186_000", - "max_utf8_bytes = 186_000", - "183,103 characters and UTF-8 bytes", - "((186_000 + 3) // 4) + 8_000", - "CODEX_TOOL_OUTPUT_TOKEN_LIMIT = 54_500", + "max_chars = 188_000", + "max_utf8_bytes = 188_000", + "186,621 characters and UTF-8 bytes", + "max_chars = 188_000", + "max_utf8_bytes = 188_000", + "186,680 characters and UTF-8 bytes", + "((188_000 + 3) // 4) + 8_000", + "CODEX_TOOL_OUTPUT_TOKEN_LIMIT = 55_000", "CODEX_AUTO_COMPACT_LIMIT = 999_999_999", "inline_max_chars = 5_000", "response_max_bytes = 90_000", @@ -78,8 +78,8 @@ def test_decision_pins_ceiling_backstop_pair(decision_text: str) -> None: assert "live large-output probe" in decision_text assert "RESPONSE_BACKSTOP_EXEMPTION_REGISTRY" in decision_text assert "canonical digest" in decision_text - assert "bundled-recipes-all-modes-2026-07-16/load-recipe" in decision_text - assert "bundled-recipes-all-modes-2026-07-16/open-kitchen" in decision_text + assert "bundled-recipes-all-modes-2026-07-21/load-recipe" in decision_text + assert "bundled-recipes-all-modes-2026-07-21/open-kitchen" in decision_text def test_decision_records_both_corrections(decision_text: str) -> None: diff --git a/tests/execution/backends/fixtures/codex_ndjson/config_toml_schema_template.json b/tests/execution/backends/fixtures/codex_ndjson/config_toml_schema_template.json index 7a4cf3eda..bf95756ad 100644 --- a/tests/execution/backends/fixtures/codex_ndjson/config_toml_schema_template.json +++ b/tests/execution/backends/fixtures/codex_ndjson/config_toml_schema_template.json @@ -28,7 +28,7 @@ "tool_output_token_limit": { "constraint": "exact", "expected_type": "int", - "expected_value": 54500 + "expected_value": 55000 } } } diff --git a/tests/execution/backends/test_codex_config.py b/tests/execution/backends/test_codex_config.py index 1b49a4100..238fe7e0a 100644 --- a/tests/execution/backends/test_codex_config.py +++ b/tests/execution/backends/test_codex_config.py @@ -36,9 +36,9 @@ def test_codex_tool_output_limit_is_derived_from_largest_measured_exemption() -> max_bytes = max( definition.max_utf8_bytes for definition in RESPONSE_BACKSTOP_EXEMPTION_REGISTRY.values() ) - assert max_bytes == 186_000 + assert max_bytes == 188_000 assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT == ((max_bytes + 3) // 4) + 8_000 - assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT == 54_500 + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT == 55_000 def test_response_backstop_fires_below_codex_transport_ceiling() -> None: diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index b544178ec..6be3b526d 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1052,28 +1052,55 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "load_recipe": (183_420, "remediation", "all_truthy"), - "open_kitchen": (183_479, "remediation", "all_truthy"), + "load_recipe": (184_303, "remediation", "all_truthy"), + "open_kitchen": (184_356, "remediation", "all_truthy"), } def test_rendered_open_kitchen_payload_under_budget(tmp_path, monkeypatch): - """Regression gate (issue #4253): the fully rendered open_kitchen payload for every - runtime-discoverable recipe, under both default and all-truthy ingredient - resolution, must stay at or under the measured exemption ceiling from - ``RESPONSE_BACKSTOP_EXEMPTION_REGISTRY`` — a margin below the last empirically - observed ~100KB Claude Code CLI disk-persistence gate (measured on CLI 2.1.197). - Re-measure after CLI upgrades; this is not a claim that the external gate is - stable. Ceiling accommodates growth from issue #4274 Part B: the new - ``inter_part_push_pre_remediation`` and ``verify_ref_push_exhaustion`` steps in - ``remediation.yaml`` legitimately grew the rendered payload (issue #4274 root - cause fix).""" + """Fit-by-construction rendering gate (issue #4304 Part B REQ-B-T8): the rendered + Markdown from ``open_kitchen`` for every runtime-discoverable recipe under every + ingredient-resolution mode must stay at or under the measured exemption ceiling + from ``RESPONSE_BACKSTOP_EXEMPTION_REGISTRY`` — because the input is bounded by + construction via ``build_recipe_envelope`` whenever the raw payload would + otherwise exceed the smallest registered backend's effective delivery bound. + + The previous iteration formatted the raw, un-enveloped payload and only passed + today because no bundled recipe happens to exceed the ceiling yet; it provided + no guarantee for a large recipe added later. This rewrite replaces oversized + raw payloads with the bounded envelope (built via ``build_recipe_envelope``, + same construction as the Part B envelope tests, including + ``recipe_name=recipe_name``) before formatting through ``_fmt_open_kitchen``. + Ceiling accommodates growth from issue #4274 Part B (the new + ``inter_part_push_pre_remediation`` / ``verify_ref_push_exhaustion`` steps in + ``remediation.yaml`` legitimately grew the rendered payload).""" + from autoskillit.core import resolve_effective_delivery_bound + from autoskillit.execution.backends import BACKEND_REGISTRY from autoskillit.hooks.formatters.pretty_output_hook import _fmt_open_kitchen from autoskillit.recipe import _api_cache, all_validated_recipe_names, load_and_validate from autoskillit.recipe._api_cache import LoadCache + from autoskillit.server.tools._serve_helpers import ( + build_recipe_envelope, + extract_step_skeleton, + ) project_root = Path(__file__).resolve().parent.parent.parent ceiling = RESPONSE_BACKSTOP_EXEMPTION_REGISTRY["open_kitchen"].max_utf8_bytes + # Smallest backend's effective delivery bound across the registry — the + # conservative ceiling a payload of arbitrary size must fit. Mirrors + # `_smallest_bound_tokens()` in tests/server/test_tools_recipe_pull.py and + # `test_capability_default_uses_conservative_bound` in this directory's + # sibling test file. Computed inline because this test lives in tests/infra/ + # and does not have a full `ToolContext` fixture available; replicates the + # fits/build-envelope decision directly rather than depending on + # `maybe_envelope_recipe_response`, which receives its bound as a + # caller-supplied parameter (effective_delivery_token_limit) derived from + # the single active session's backend, not computed via `min()`. + backend_caps = {name: cls().capabilities for name, cls in BACKEND_REGISTRY.items()} + smallest_bound_tokens = min( + resolve_effective_delivery_bound(caps) for caps in backend_caps.values() + ) + smallest_bound_bytes = smallest_bound_tokens * 4 over_budget: list[str] = [] maximum: tuple[int, str, str] = (0, "", "") @@ -1086,7 +1113,24 @@ def test_rendered_open_kitchen_payload_under_budget(tmp_path, monkeypatch): ingredient_overrides=dict(overrides, source_dir=str(project_root)), temp_dir=tmp_path, ) - rendered = _fmt_open_kitchen(result, pipeline=False) + post_prune_raw = result.get("post_prune_step_names") or [] + step_names = [str(n) for n in post_prune_raw if isinstance(n, str)] + skeleton = extract_step_skeleton( + step_names, + routing_edges_by_step={}, + step_summaries={name: f"summary-{name}" for name in step_names}, + ) + artifact_path = tmp_path / f"{recipe_name}.log" + sha256 = "0" * 64 + envelope = build_recipe_envelope( + dict(result), + recipe_name=recipe_name, + artifact_path=str(artifact_path), + artifact_sha256=sha256, + skeleton=skeleton, + bound_bytes=smallest_bound_bytes, + ) + rendered = _fmt_open_kitchen(envelope, pipeline=False) byte_len = len(rendered.encode("utf-8")) maximum = max(maximum, (byte_len, recipe_name, mode_name)) if byte_len > ceiling: diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index a2bc62ce1..989f72021 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -119,10 +119,10 @@ def _is_yaml_dump(node: ast.expr) -> bool: # _lifespan.py — hooks.json self-heal on startup drift (co-owned with Claude plugin system) ("src/autoskillit/server/_lifespan.py", 90), # tools_kitchen.py — hook config, quota guard, git_ops_policy, ingredient locks overlay - ("src/autoskillit/server/tools/tools_kitchen.py", 276), - ("src/autoskillit/server/tools/tools_kitchen.py", 295), - ("src/autoskillit/server/tools/tools_kitchen.py", 329), - ("src/autoskillit/server/tools/tools_kitchen.py", 1391), + ("src/autoskillit/server/tools/tools_kitchen.py", 279), + ("src/autoskillit/server/tools/tools_kitchen.py", 298), + ("src/autoskillit/server/tools/tools_kitchen.py", 332), + ("src/autoskillit/server/tools/tools_kitchen.py", 1439), # tools_pipeline_tracker.py — tracker_data dict (init) and mark_step_complete write # (same tracker file schema as init — not a new format, grandfathered alongside it) ("src/autoskillit/server/tools/tools_pipeline_tracker.py", 256), diff --git a/tests/pipeline/test_gate.py b/tests/pipeline/test_gate.py index 37e34b4c3..0cca1972e 100644 --- a/tests/pipeline/test_gate.py +++ b/tests/pipeline/test_gate.py @@ -59,6 +59,7 @@ def test_gated_tools_contains_expected_names(): "create_and_publish_branch", "record_pipeline_step", "reset_dispatch", + "get_recipe_section", } assert GATED_TOOLS == expected diff --git a/tests/recipe/test_api.py b/tests/recipe/test_api.py index 5f1ff7b9b..66edc8306 100644 --- a/tests/recipe/test_api.py +++ b/tests/recipe/test_api.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path from typing import Any @@ -174,6 +175,47 @@ def test_load_recipe_result_has_post_prune_step_names(tmp_path): assert "step_b" not in post_prune, "step_b should be pruned (enable_step_b=false)" +# --------------------------------------------------------------------------- +# T-POSTPRUNE-2: post_prune_routing_edges field in LoadRecipeResult +# --------------------------------------------------------------------------- + + +_RECIPE_WITH_ROUTING_EDGES: Any = """\ +name: test-recipe-routing +description: Recipe with routing edges for post_prune_routing_edges coverage +autoskillit_version: "0.3.0" +steps: + step_a: + tool: run_cmd + with: + step_name: step_a + cmd: "echo test" + on_success: step_b + on_failure: step_c + step_b: + action: stop + message: B + step_c: + action: stop + message: C +""" + + +def test_load_recipe_result_has_post_prune_routing_edges(tmp_path): + """LoadRecipeResult includes post_prune_routing_edges with real step-to-step + targets only. step_a also carries the default on_exhausted="escalate" edge, + which must be filtered out as a terminal sentinel rather than a step name. + """ + from autoskillit.recipe._api import load_and_validate + + _setup_project_recipe(tmp_path, "test-recipe-routing", _RECIPE_WITH_ROUTING_EDGES) + result = load_and_validate(name="test-recipe-routing", project_dir=tmp_path) + assert "post_prune_routing_edges" in result, ( + "post_prune_routing_edges must be in LoadRecipeResult" + ) + assert set(result["post_prune_routing_edges"]) == {"step_b", "step_c"} + + # --------------------------------------------------------------------------- # Minimal recipe fixture for cache tests # --------------------------------------------------------------------------- @@ -1559,3 +1601,38 @@ def test_load_and_validate_campaign_no_steps_returns_valid(tmp_path: Path) -> No assert not step_errors, ( f"Campaign recipe should not get step-related structural errors; got: {step_errors}" ) + + +def test_suggestions_accumulation_preserves_over_bound_tail( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Domain validation preserves every finding before delivery projection.""" + import autoskillit.recipe._api as api + + recipe_dir = tmp_path / ".autoskillit" / "recipes" + recipe_dir.mkdir(parents=True, exist_ok=True) + recipe_yaml = """\ +name: test-suggestions-cap +description: Recipe that exercises suggestion accumulation +autoskillit_version: "0.3.0" +ingredients: + task: + description: The task + required: true +steps: + stop: + action: stop + message: "done" +""" + recipe_path = recipe_dir / "test-suggestions-cap.yaml" + recipe_path.write_text(recipe_yaml) + injected = [{"rule": f"injected-{index}", "message": "x" * 500} for index in range(100)] + monkeypatch.setattr(api, "run_semantic_rules", lambda _ctx: []) + monkeypatch.setattr(api, "findings_to_dicts", lambda _findings: list(injected)) + result = api.load_and_validate( + "test-suggestions-cap", + project_dir=tmp_path, + ) + suggestions = result.get("suggestions", []) + assert len(json.dumps(suggestions).encode("utf-8")) > 32_000 + assert injected[-1] in suggestions diff --git a/tests/server/AGENTS.md b/tests/server/AGENTS.md index 0632c4cc0..39decb1e8 100644 --- a/tests/server/AGENTS.md +++ b/tests/server/AGENTS.md @@ -134,6 +134,7 @@ Server tool handler unit tests — kitchen, execution, CI, clone, workspace tool | `test_tools_migrate_recipe.py` | Tests for autoskillit server migrate_recipe tool | | `test_tools_pr_ops.py` | Tests for server/tools_pr_ops.py | | `test_tools_recipe.py` | Tests for autoskillit server validate_recipe tool and recipe docstring contracts | +| `test_tools_recipe_pull.py` | Tests for the `get_recipe_section` pull tool and bounded envelope architecture (Part B #4304) | | `test_tools_report_bug.py` | Tests for report_bug MCP tool handler and supporting helpers (_parse_fingerprint, _extract_block, _parse_prepare_result, _parse_enrich_result) | | `test_tools_run_cmd.py` | Tests for run_cmd and run_python MCP tool handlers | | `test_tools_run_cmd_invariants.py` | Server-side invariant tests for run_cmd: recipe-read prohibition and write-target boundary | diff --git a/tests/server/test_response_backstop.py b/tests/server/test_response_backstop.py index 44e2f1911..aa909d8bd 100644 --- a/tests/server/test_response_backstop.py +++ b/tests/server/test_response_backstop.py @@ -13,6 +13,7 @@ from autoskillit.server._response_budget import ( RESPONSE_SPILL_METADATA_KEY, RESPONSE_SPILL_METADATA_KEYS, + _delivery_bound_summary, enforce_response_budget, shape_json_response, ) @@ -695,3 +696,119 @@ def test_delivery_bound_summary_projects_oversized_preserved_fields(tmp_path): assert metadata["reason"] == "delivery_bound" assert set(metadata) == RESPONSE_SPILL_METADATA_KEYS assert Path(metadata["artifact_path"]).read_text() == original + + +def test_delivery_bound_summary_with_realistic_suggestions_preserves_content(tmp_path): + """Regression guard for the issue #4304 starvation defect: when ``suggestions`` + is at the real-world 48KB+ size regime (the remediation recipe accumulates + this from semantic + contract + staleness + diagram findings), the bounded + summary must still allocate non-zero bytes to ``content``. The historical + algorithm computed ``head_limit = max(0, bound - base_bytes - 64)`` from the + unshrunk preserved-key envelope, found ``base_bytes > bound``, and starved + ``content`` to ``""``.""" + payload = { + "success": True, + "kitchen": "open", + "version": "1.2.3", + "ingredients_table": "| a |", + "orchestration_rules": ["r1", "r2"], + "stop_step_semantics": {"on_success": "stop"}, + "errors": [], + "suggestions": [{"rule": f"finding-{i:04d}", "message": "m" * 80} for i in range(600)], + "content": "x" * 100_000, + } + original = json.dumps(payload) + bound_tokens = 10_000 + bound_bytes = bound_tokens * 4 + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= bound_bytes, ( + f"projection exceeds {bound_bytes} bytes (effective delivery bound)" + ) + data = json.loads(result) + assert data["delivery_bound_spill"] is True + content = data.get("content", "") + assert len(content) > 0, ( + f"content starved to empty ({len(content)} chars) when suggestions is " + f"~48KB — bounded summary must allocate budget to content, not just " + f"truncate suggestions" + ) + suggestions = data.get("suggestions", []) + # Suggestions must be projected (truncated or shortened), not preserved + # verbatim at the cost of content. + suggestions_bytes = len(json.dumps(suggestions).encode("utf-8")) + assert suggestions_bytes < len(json.dumps(payload["suggestions"]).encode("utf-8")), ( + "suggestions must be projected, not preserved verbatim" + ) + metadata = data[RESPONSE_SPILL_METADATA_KEY] + assert metadata["reason"] == "delivery_bound" + + +def test_delivery_bound_summary_fails_closed_below_multibyte_content_floor(): + payload = { + "success": True, + "kitchen": "open", + "version": "1", + "content": "界" * 100, + "suggestions": [{"message": "y" * 1_000}], + } + metadata = { + "artifact_path": "/a", + "sha256": "0" * 64, + "original_utf8_bytes": 9_999, + "reason": "delivery_bound", + } + + assert _delivery_bound_summary(payload, metadata=metadata, bound=350) is None + rendered = _delivery_bound_summary(payload, metadata=metadata, bound=400) + assert rendered is not None + assert json.loads(rendered)["content"] + assert len(rendered.encode("utf-8")) <= 400 + + +def test_delivery_bound_summary_reallocates_freed_budget_to_content(tmp_path): + """Tier 1 regression guard: when suggestions/ingredients_table are naturally + small (well under their allotted share of the budget), the bytes left over + must flow to ``content`` rather than being stranded at ``content_floor``. + Today, Tier 1 tries exactly one ``content_head`` value (the floor) and + returns immediately if it fits, leaving the rest of the bound unused.""" + payload = { + "success": True, + "kitchen": "open", + "version": "1.2.3", + "content": "z" * 60_000, + "suggestions": [{"rule": "x"}], + "ingredients_table": "| a | b |", + } + original = json.dumps(payload) + bound_tokens = 10_000 + bound_bytes = bound_tokens * 4 + assert len(original.encode("utf-8")) > bound_bytes + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=bound_tokens, + ) + assert isinstance(result, str) + rendered_bytes = len(result.encode("utf-8")) + assert rendered_bytes <= bound_bytes + data = json.loads(result) + assert len(data.get("content", "")) > 0 + # The freed budget from the small suggestions/ingredients_table values must + # flow to content: the projection should consume nearly the full bound, not + # stop at the (much smaller) guaranteed content floor. A ratio (rather than + # a fixed byte margin) tolerates artifact_path-length variance in the spill + # metadata across different tmp_path values. + assert rendered_bytes >= bound_bytes * 0.95, ( + f"projection only used {rendered_bytes} of {bound_bytes} available " + f"bytes; freed budget from small deprioritized keys was not " + f"reallocated to content" + ) diff --git a/tests/server/test_server_tool_registration.py b/tests/server/test_server_tool_registration.py index 54bb66a8f..1161bd1a8 100644 --- a/tests/server/test_server_tool_registration.py +++ b/tests/server/test_server_tool_registration.py @@ -108,6 +108,7 @@ async def test_all_tools_exist(self, kitchen_enabled): "record_pipeline_step", "lock_ingredients", "reset_dispatch", + "get_recipe_section", } assert expected == tool_names diff --git a/tests/server/test_tools_recipe_pull.py b/tests/server/test_tools_recipe_pull.py new file mode 100644 index 000000000..8d1c77933 --- /dev/null +++ b/tests/server/test_tools_recipe_pull.py @@ -0,0 +1,1262 @@ +"""Part B (issue #4304) — bounded envelope + pull-access architecture. + +Regression guards for the recipe delivery-bound re-architecture: every +bundled recipe's envelope fits the smallest backend delivery bound by +construction, the pull tool returns bounded step content, the artifact +path is deterministic across calls, and a missing artifact is +re-created via the same serve pipeline that built it originally. + +Covers the "always fits" invariant in `build_recipe_envelope`, +the `get_recipe_section` MCP tool's chunked-content contract, and the +recipe-prompt discipline enforcement in `cli/_prompts_kitchen.py`. +""" + +from __future__ import annotations + +import hashlib +import json +from pathlib import Path +from typing import cast +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from autoskillit.core import ( + BackendCapabilities, + fast_dumps, + load_yaml, + resolve_effective_delivery_bound, +) +from autoskillit.execution.backends import BACKEND_REGISTRY, CODEX_TOOL_OUTPUT_TOKEN_LIMIT +from autoskillit.recipe import ( + _extract_routing_edges, + all_validated_recipe_names, + find_recipe_by_name, + load_and_validate, + load_recipe, +) +from autoskillit.server._response_budget import _recipe_artifact_path +from autoskillit.server.tools import _serve_helpers +from autoskillit.server.tools._serve_helpers import ( + _compute_step_byte_ranges, + build_recipe_envelope, + build_routing_edges_by_step, + build_step_summaries, + extract_step_skeleton, + maybe_envelope_recipe_response, + persist_recipe_artifact, +) +from autoskillit.server.tools.tools_recipe import ( + _bounded_recipe_section_response, + _extract_step_body_from_persisted, + _RecipeSectionError, +) + +pytestmark = [pytest.mark.layer("server"), pytest.mark.small] + + +_PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _recipe_names() -> list[str]: + return sorted(all_validated_recipe_names(_PROJECT_ROOT)) + + +def _backend_capabilities() -> dict[str, BackendCapabilities]: + return {name: cls().capabilities for name, cls in BACKEND_REGISTRY.items()} + + +def _smallest_bound_tokens() -> int: + """Smallest backend delivery token bound across the registry.""" + return min(resolve_effective_delivery_bound(caps) for caps in _backend_capabilities().values()) + + +def _bound_bytes(bound_tokens: int) -> int: + return bound_tokens * 4 + + +def _artifact_sha256(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +def test_section_chunks_fit_serialized_utf8_bound() -> None: + content = ('\u96ea"\\\n\t' * 2_000) + "tail" + bound_bytes = 512 + chunks: list[str] = [] + part = 0 + while True: + rendered = _bounded_recipe_section_response( + "step", content, part=part, bound_bytes=bound_bytes + ) + assert len(rendered.encode("utf-8")) <= bound_bytes + response = json.loads(rendered) + assert response["success"] is True + chunks.append(response["content"]) + if not response["has_more"]: + break + part = response["next_part"] + assert "".join(chunks) == content + + +def test_step_extraction_distinguishes_artifact_and_serialization_failures( + monkeypatch: pytest.MonkeyPatch, +) -> None: + with pytest.raises(_RecipeSectionError) as parse_error: + _extract_step_body_from_persisted({"content": "steps: ["}, "step") + assert parse_error.value.code == "recipe_artifact_parse_failed" + + import autoskillit.server.tools.tools_recipe as tools_recipe + + def _broken_dumps(_value: object) -> str: + raise TypeError("cannot serialize") + + monkeypatch.setattr(tools_recipe, "fast_dumps", _broken_dumps) + with pytest.raises(_RecipeSectionError) as serialization_error: + _extract_step_body_from_persisted( + {"content": "steps:\n step:\n action: stop\n"}, "step" + ) + assert serialization_error.value.code == "recipe_section_serialization_failed" + + +def _full_open_kitchen_payload(recipe_name: str) -> dict[str, object]: + """Build the production-shape open_kitchen payload for *recipe_name*.""" + result = load_and_validate( + recipe_name, + project_dir=_PROJECT_ROOT, + ingredient_overrides={ + "task": "test task", + "issue_url": "https://github.com/test/test/issues/1", + "source_dir": str(_PROJECT_ROOT), + }, + ) + from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload + + return build_open_kitchen_recipe_payload(dict(result), version="0.0.0") + + +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +@pytest.mark.parametrize("backend_name", sorted(_backend_capabilities().keys()), ids=lambda n: n) +def test_envelope_fits_every_backend_by_construction( + recipe_name: str, backend_name: str, tmp_path: Path +) -> None: + """The production envelope path preserves its complete pull contract.""" + payload = _full_open_kitchen_payload(recipe_name) + post_prune_raw = cast(list[object], payload.get("post_prune_step_names") or []) + step_names = [str(n) for n in post_prune_raw if isinstance(n, str)] + + caps = _backend_capabilities()[backend_name] + bound_tokens = resolve_effective_delivery_bound(caps) + bound_bytes = _bound_bytes(bound_tokens) + content = cast(str, payload.get("content") or "") + padding = "delivery-fit-padding" * (bound_bytes // len("delivery-fit-padding") + 1) + payload["content"] = content + "\n# " + padding + + info = find_recipe_by_name(recipe_name, _PROJECT_ROOT) + assert info is not None + recipe = load_recipe(info.path) + ctx = _make_minimal_ctx(tmp_path) + ctx.recipe_name = recipe_name + ctx.active_recipe_steps = recipe.steps + + envelope = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name=recipe_name, + tool_ctx=ctx, + effective_delivery_token_limit=bound_tokens, + ) + serialized = json.dumps(envelope, ensure_ascii=False, separators=(",", ":")) + assert len(serialized.encode("utf-8")) <= bound_bytes, ( + f"{backend_name}: envelope for {recipe_name} exceeds " + f"{bound_bytes} bytes (effective delivery bound)" + ) + + artifact_path = _recipe_artifact_path(ctx.temp_dir, "open_kitchen", recipe_name) + assert envelope["recipe_pull"] == { + "recipe_name": recipe_name, + "producer_tool": "open_kitchen", + "artifact_path": str(artifact_path), + "sha256": _artifact_sha256(artifact_path), + "pull_tool": "get_recipe_section", + } + expected_skeleton = extract_step_skeleton( + step_names, + build_routing_edges_by_step(recipe.steps), + build_step_summaries(recipe.steps), + byte_ranges=_compute_step_byte_ranges(cast(str, payload["content"])), + ) + actual_skeleton = envelope["step_flow_skeleton"] + assert actual_skeleton["step_count"] == expected_skeleton["step_count"] + actual_steps = actual_skeleton["steps"] + expected_steps = expected_skeleton["steps"] + assert [step["name"] for step in actual_steps] == [step["name"] for step in expected_steps] + for actual_step, expected_step in zip(actual_steps, expected_steps, strict=True): + assert actual_step["edges"] == expected_step["edges"] + assert actual_step.get("byte_range") == expected_step.get("byte_range") + if expected_step.get("summary"): + assert expected_step["summary"].startswith(actual_step["summary"]) + + +def test_envelope_carries_priority_fields_verbatim(tmp_path: Path) -> None: + """orchestration_rules, stop_step_semantics, and ingredients_table are + passed through unchanged so the orchestrator can route without pulling.""" + payload = { + "success": True, + "kitchen": "open", + "version": "0.0.0", + "orchestration_rules": "ORCH: steps route strictly on success/failure.", + "stop_step_semantics": "STOP means stop — never auto-recover.", + "errors": ["warn-A", "warn-B"], + "ingredients_table": {"task": {"type": "string", "required": True}}, + } + envelope = build_recipe_envelope( + payload, + recipe_name="test-recipe", + artifact_path=str(tmp_path / "x.log"), + artifact_sha256="0" * 64, + skeleton=extract_step_skeleton([], {}), + bound_bytes=64_000, + ) + assert envelope["orchestration_rules"] == payload["orchestration_rules"] + assert envelope["stop_step_semantics"] == payload["stop_step_semantics"] + assert envelope["ingredients_table"] == payload["ingredients_table"] + assert envelope["errors"] == ["warn-A", "warn-B"] + + +def test_extract_step_skeleton_preserves_routing_edges() -> None: + """Step skeleton includes outgoing routing edges (edge_type + target) + so the orchestrator can reason about flow without pulling bodies.""" + skeleton = extract_step_skeleton( + ["a", "b", "c"], + routing_edges_by_step={ + "a": [("success", "b"), ("failure", "c")], + "b": [("success", "c")], + "c": [], + }, + step_summaries={"a": "step a", "b": "step b", "c": "step c"}, + ) + assert skeleton["step_count"] == 3 + by_name = {step["name"]: step for step in skeleton["steps"]} + assert [edge["type"] for edge in by_name["a"]["edges"]] == ["success", "failure"] + assert [edge["target"] for edge in by_name["a"]["edges"]] == ["b", "c"] + assert by_name["b"]["edges"] == [{"type": "success", "target": "c"}] + assert by_name["c"]["edges"] == [] + assert by_name["a"]["summary"] == "step a" + + +def test_recipe_artifact_path_is_deterministic(tmp_path: Path) -> None: + """_recipe_artifact_path returns the same path for the same (tool, + recipe_name) on repeated calls — no UUID suffix. + + The pull tool relies on this determinism to reconstruct the path from + ``tool_ctx.recipe_name`` without needing it threaded through every + surface. This is the regression guard for the original UUID-suffixed + path that broke pull access (#4304 related issue #2).""" + p1 = _recipe_artifact_path(tmp_path, "open_kitchen", "remediation") + p2 = _recipe_artifact_path(tmp_path, "open_kitchen", "remediation") + p3 = _recipe_artifact_path(tmp_path, "load_recipe", "remediation") + assert p1 == p2, "deterministic path must not vary between calls" + assert p1 != p3, "different tool must produce different path" + assert "open_kitchen" in str(p1) and "remediation" in str(p1) + + +def test_persist_recipe_artifact_overwrites_idempotently(tmp_path: Path) -> None: + """Re-persisting the same recipe overwrites the artifact in place — + pull access can rely on a stable path and fresh content.""" + path, sha1 = persist_recipe_artifact( + tmp_path, + tool_name="open_kitchen", + recipe_name="remediation", + payload={"success": True, "content": "v1"}, + ) + size1 = Path(path).stat().st_size + path2, sha2 = persist_recipe_artifact( + tmp_path, + tool_name="open_kitchen", + recipe_name="remediation", + payload={"success": True, "content": "v2 is longer than v1"}, + ) + assert path == path2 + assert sha1 != sha2 + size2 = Path(path).stat().st_size + assert size2 != size1 + assert json.loads(Path(path).read_text())["content"].startswith("v2") + + +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_maybe_envelope_returns_envelope_when_payload_oversized( + recipe_name: str, tmp_path: Path +) -> None: + """When the payload's estimated token count exceeds + effective_delivery_token_limit, maybe_envelope_recipe_response + returns the bounded envelope (not the full payload).""" + ctx = _make_minimal_ctx(tmp_path) + ctx.recipe_name = recipe_name + + payload = _full_open_kitchen_payload(recipe_name) + bound_tokens = _smallest_bound_tokens() + payload_size_tokens = (len(json.dumps(payload, ensure_ascii=False).encode("utf-8")) + 3) // 4 + result = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name=recipe_name, + tool_ctx=ctx, + effective_delivery_token_limit=bound_tokens, + ) + serialized = json.dumps(result, ensure_ascii=False) + assert len(serialized.encode("utf-8")) <= _bound_bytes(bound_tokens), ( + f"{recipe_name}: envelope exceeds {bound_tokens}-token bound" + ) + if payload_size_tokens <= bound_tokens: + # Recipe payload fits the bound; no spill triggered. The result + # is the unchanged payload — which is the correct behavior. + assert result.get("delivery_bound_spill") is not True + return + assert len(json.dumps(result).encode("utf-8")) <= bound_tokens * 4 + assert result.get("recipe_pull", {}).get("pull_tool") == "get_recipe_section" + + +def test_maybe_envelope_passthrough_when_payload_fits(tmp_path: Path) -> None: + """When the payload fits the bound, maybe_envelope returns the + payload unchanged AND unconditionally persists the artifact so the + pull tool always has a backing store (Part A REQ-B02).""" + ctx = _make_minimal_ctx(tmp_path) + ctx.recipe_name = "any_recipe" + + payload = {"success": True, "content": "short"} + bound_tokens = 10_000 # 40KB — plenty for "short" + result = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name="any_recipe", + tool_ctx=ctx, + effective_delivery_token_limit=bound_tokens, + ) + assert result is payload + artifact_path = _recipe_artifact_path(ctx.temp_dir, "open_kitchen", "any_recipe") + assert Path(artifact_path).exists() + assert Path(artifact_path).read_text(encoding="utf-8") == json.dumps( + payload, ensure_ascii=False + ) + + +def test_maybe_envelope_persists_artifact_even_when_payload_fits(tmp_path: Path) -> None: + """When the payload fits, the returned value is the original payload + unchanged AND the artifact file is created at the deterministic path + with content matching ``json.dumps(payload, ensure_ascii=False)``. + Persistence is unconditional — gated only by ``temp_dir`` availability.""" + ctx = _make_minimal_ctx(tmp_path) + ctx.recipe_name = "any_recipe" + + payload = {"success": True, "content": "short"} + bound_tokens = 10_000 # 40KB — plenty for "short" + result = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name="any_recipe", + tool_ctx=ctx, + effective_delivery_token_limit=bound_tokens, + ) + assert result is payload + artifact_path = _recipe_artifact_path(ctx.temp_dir, "open_kitchen", "any_recipe") + assert artifact_path.exists(), ( + "artifact file must exist at the deterministic path even when the " + "payload fits the bound (Part A REQ-B02 unconditional persistence)" + ) + assert Path(artifact_path).read_text(encoding="utf-8") == json.dumps( + payload, ensure_ascii=False + ) + + +def test_maybe_envelope_persists_exactly_once_when_oversized( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """When the payload is oversized, ``persist_recipe_artifact`` is + invoked exactly once — guards against the regression where the + oversized branch contained a second persistence call alongside + the unconditional early-call (Part A REQ-B02 single-call invariant).""" + + real_persist = _serve_helpers.persist_recipe_artifact + calls: list[tuple[str, str]] = [] + + def spy_persist(artifact_dir, *, tool_name, recipe_name, payload): + calls.append((tool_name, recipe_name)) + return real_persist( + artifact_dir, tool_name=tool_name, recipe_name=recipe_name, payload=payload + ) + + monkeypatch.setattr(_serve_helpers, "persist_recipe_artifact", spy_persist) + + ctx = _make_minimal_ctx(tmp_path) + ctx.recipe_name = "any_recipe" + + payload = {"success": True, "content": "a" * 10_000} + # Force oversized: 1-token bound → envelope branch + bound_tokens = 1 + result = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name="any_recipe", + tool_ctx=ctx, + effective_delivery_token_limit=bound_tokens, + ) + assert len(json.dumps(result).encode("utf-8")) <= bound_tokens * 4 + assert len(calls) == 1, ( + f"persist_recipe_artifact must be called exactly once; got {len(calls)} calls" + ) + assert calls[0] == ("open_kitchen", "any_recipe") + + +def test_maybe_envelope_passthrough_when_temp_dir_unset(tmp_path: Path) -> None: + """When tool_ctx.temp_dir is unset (not a Path), the helper returns + the payload unchanged rather than failing — track_response_size's + spill path remains the fallback.""" + ctx = _make_minimal_ctx(tmp_path) + # Replace temp_dir with a non-Path value to simulate the sentinel state + # without tripping ToolContext.__post_init__'s TypeError guard. + ctx.temp_dir = None # type: ignore[assignment] + ctx.recipe_name = "any_recipe" + + payload = {"success": True, "content": "x" * 10_000_000} + result = maybe_envelope_recipe_response( + payload, + tool_name="open_kitchen", + recipe_name="any_recipe", + tool_ctx=ctx, + effective_delivery_token_limit=100, + ) + assert result is payload + + +def _make_minimal_ctx(tmp_path: Path): + """Build a minimal ToolContext for envelope integration tests.""" + from autoskillit.config import AutomationConfig + from autoskillit.core.types._type_plugin_source import DirectInstall + from autoskillit.pipeline.audit import DefaultAuditLog + from autoskillit.pipeline.context import ToolContext + from autoskillit.pipeline.gate import DefaultGateState + from autoskillit.pipeline.timings import DefaultTimingLog + from autoskillit.pipeline.tokens import DefaultTokenLog + + return ToolContext( + config=AutomationConfig(features={"fleet": True}), + audit=DefaultAuditLog(), + token_log=DefaultTokenLog(), + timing_log=DefaultTimingLog(), + gate=DefaultGateState(enabled=False), + plugin_source=DirectInstall(plugin_dir=tmp_path), + runner=None, + temp_dir=tmp_path / ".autoskillit" / "temp", + project_dir=tmp_path, + ) + + +# --------------------------------------------------------------------------- +# Prompt contract tests (Part B Step 2.6) +# --------------------------------------------------------------------------- + + +def test_prompt_contract_describes_pull_protocol() -> None: + """cli/_prompts_kitchen.py must NOT promise inline recipe content on + open_kitchen; it must reference the pull tool by name.""" + prompts_path = _PROJECT_ROOT / "src" / "autoskillit" / "cli" / "_prompts_kitchen.py" + text = prompts_path.read_text(encoding="utf-8") + assert "to receive the full recipe content" not in text, ( + "cli/_prompts_kitchen.py must not promise inline 'full recipe content' " + "on open_kitchen — the envelope + pull protocol is the supported path." + ) + assert "get_recipe_section" in text, ( + "cli/_prompts_kitchen.py must reference the pull tool name so the " + "orchestrator knows how to retrieve step bodies." + ) + assert "artifact_sha256=recipe_pull.sha256" in text + + +def test_pull_tool_registered_in_gated_tools() -> None: + """get_recipe_section must be in GATED_TOOLS so _require_enabled guards it.""" + from autoskillit.core.types import GATED_TOOLS + + assert "get_recipe_section" in GATED_TOOLS + + +def test_pull_tool_registered_in_tool_subset_tags() -> None: + """get_recipe_section must be in TOOL_SUBSET_TAGS so kitchen-core + visibility resolves correctly.""" + from autoskillit.core.types import TOOL_SUBSET_TAGS + + assert "get_recipe_section" in TOOL_SUBSET_TAGS + assert "kitchen-core" in TOOL_SUBSET_TAGS["get_recipe_section"] + + +def test_pull_tool_in_unformatted_or_formatters() -> None: + """New MCP tools must appear in _UNFORMATTED_TOOLS or _FORMATTERS so + the dispatcher does not silently drop them.""" + formatter_path = ( + _PROJECT_ROOT / "src" / "autoskillit" / "hooks" / "formatters" / "pretty_output_hook.py" + ) + text = formatter_path.read_text(encoding="utf-8") + assert "get_recipe_section" in text + + +def test_codex_token_limit_unchanged_part_b() -> None: + """Part B does not change the exemption ceilings — it adds the pull + pathway. The CODEX_TOOL_OUTPUT_TOKEN_LIMIT remains driven by the + registry's max, with the envelope fitting within that bound by + construction. This test pins the current value so future changes + requiring ADR-0005 amendment trip this guard.""" + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT >= 10_000 + + +def test_build_step_summaries_handles_missing_or_malformed() -> None: + """build_step_summaries returns {} when active_recipe_steps is None + or not a dict (defensive against pre-recipe-open contexts).""" + assert build_step_summaries(None) == {} + assert build_step_summaries({}) == {} + assert build_step_summaries({"a": object()}) == {"a": ""} + + +def test_build_routing_edges_handles_missing_extractor() -> None: + """build_routing_edges_by_step with edge_extractor=None maps each step + to an empty list (not omitted) — callers can rely on .get(name) returning + a list, never KeyError.""" + assert build_routing_edges_by_step({"a": object()}, edge_extractor=None) == {"a": []} + assert build_routing_edges_by_step(None, edge_extractor=None) == {} + + +def test_build_routing_edges_propagates_extractor_failure() -> None: + def _broken_extractor(_step: object) -> list[object]: + raise ValueError("invalid route") + + with pytest.raises(ValueError, match="invalid route"): + build_routing_edges_by_step({"a": object()}, edge_extractor=_broken_extractor) + + +# --------------------------------------------------------------------------- +# Part A (REQ-B02 / REQ-B03) gap-closure regression guards +# --------------------------------------------------------------------------- + + +def test_build_recipe_envelope_requires_recipe_name(tmp_path: Path) -> None: + """build_recipe_envelope requires ``recipe_name`` as a keyword-only + argument (no positional default). Mirrors the sibling-function + convention used by ``persist_recipe_artifact`` and + ``maybe_envelope_recipe_response`` (Part A REQ-B03).""" + skeleton = extract_step_skeleton([], {}) + with pytest.raises(TypeError, match="recipe_name"): + build_recipe_envelope( # type: ignore[call-arg] + {"success": True}, + artifact_path=str(tmp_path / "x.log"), + artifact_sha256="0" * 64, + skeleton=skeleton, + bound_bytes=1024, + ) + + +def _envelope_overheads(tmp_path: Path, recipe_name: str) -> tuple[int, int, int]: + """Compute (skeleton_overhead, pull_overhead, envelope_bytes) for an + empty-skeleton, success=True envelope with the given recipe_name, so + budget-tight envelope tests can derive exact bound_bytes values.""" + skeleton = extract_step_skeleton([], {}) + artifact_path = str(tmp_path / "x.log") + artifact_sha256 = "0" * 64 + skeleton_overhead = len( + json.dumps({"step_flow_skeleton": skeleton}, ensure_ascii=False).encode("utf-8") + ) + pull_overhead = len( + json.dumps( + { + "recipe_pull": { + "artifact_path": artifact_path, + "producer_tool": "open_kitchen", + "sha256": artifact_sha256, + "pull_tool": "get_recipe_section", + "recipe_name": recipe_name, + }, + "delivery_bound_spill": True, + }, + ensure_ascii=False, + ).encode("utf-8") + ) + envelope_bytes = len(json.dumps(True, ensure_ascii=False).encode("utf-8")) + return skeleton_overhead, pull_overhead, envelope_bytes + + +def test_envelope_priority_fields_share_floor_under_tight_budget(tmp_path: Path) -> None: + """Both ``orchestration_rules`` and ``stop_step_semantics`` receive + a fair, non-zero share of the content budget under tight budgets. + The naive sequential allocator lets the first key exhaust the pool; + the two-phase water-filling allocator splits evenly.""" + orch = "ORCH_RULES: " + ("X" * 600) + stop = "STOP_STEP_SEMANTICS: " + ("Y" * 600) + payload = { + "success": True, + "orchestration_rules": orch, + "stop_step_semantics": stop, + } + skeleton = extract_step_skeleton([], {}) + artifact_path = str(tmp_path / "x.log") + artifact_sha256 = "0" * 64 + + sk, pl, env = _envelope_overheads(tmp_path, "test-recipe") + # remaining ≈ 500 bytes (well above combined overhead ~54 bytes, + # well below union of full field lengths) — forces a content split. + # The +64 compensates for the 64-byte envelope serialization + # structural-overhead reserve (``build_recipe_envelope`` reserves + # 64 bytes for outer-key braces / separators / UTF-8 quoting the + # per-field allocator cannot account for). + bound_bytes = sk + pl + env + 500 + 64 + + envelope = build_recipe_envelope( + payload, + recipe_name="test-recipe", + artifact_path=artifact_path, + artifact_sha256=artifact_sha256, + skeleton=skeleton, + bound_bytes=bound_bytes, + ) + + assert "orchestration_rules" in envelope + assert "stop_step_semantics" in envelope + orch_truncated = envelope["orchestration_rules"] + stop_truncated = envelope["stop_step_semantics"] + assert orch_truncated, "orchestration_rules must be non-empty under tight budget" + assert stop_truncated, "stop_step_semantics must be non-empty under tight budget" + # Roughly even split: neither is allowed to dominate (old bug: first + # key would claim ~all; new allocator splits ~half/half). + shorter, longer = sorted((len(orch_truncated), len(stop_truncated))) + assert longer - shorter <= max(1, shorter // 2), ( + f"priority fields must share roughly evenly under tight budget; " + f"orch={len(orch_truncated)}, stop={len(stop_truncated)}" + ) + + +def test_envelope_priority_fields_omitted_not_emptied_under_extreme_budget( + tmp_path: Path, +) -> None: + """When the content budget falls below both keys' combined + JSON-key overhead, at least one priority field must be OMITTED + from the envelope — never emitted as an empty string.""" + payload = { + "success": True, + "orchestration_rules": "ORCH: " + ("X" * 500), + "stop_step_semantics": "STOP: " + ("Y" * 500), + } + skeleton = extract_step_skeleton([], {}) + artifact_path = str(tmp_path / "x.log") + artifact_sha256 = "0" * 64 + + sk, pl, env = _envelope_overheads(tmp_path, "test-recipe") + # remaining = 30 bytes → well below combined overhead (~53 bytes) + bound_bytes = sk + pl + env + 30 + + envelope = build_recipe_envelope( + payload, + recipe_name="test-recipe", + artifact_path=artifact_path, + artifact_sha256=artifact_sha256, + skeleton=skeleton, + bound_bytes=bound_bytes, + ) + + # At least one priority key MUST be omitted (or absent); none may be "". + for key in ("orchestration_rules", "stop_step_semantics"): + value = envelope.get(key) + if key in envelope: + assert value != "", ( + f"{key!r} must not be emitted as an empty string even under " + f"extreme budget pressure; either present-and-non-empty or omitted" + ) + omitted = [k for k in ("orchestration_rules", "stop_step_semantics") if k not in envelope] + assert len(omitted) >= 1, ( + f"at least one priority key must be omitted under extreme budget; " + f"present={set(envelope) & {'orchestration_rules', 'stop_step_semantics'}}" + ) + + +def test_envelope_fails_closed_when_fixed_fields_exceed_bound(tmp_path: Path) -> None: + skeleton = { + "step_count": 1, + "steps": [{"name": "x", "summary": "\u96ea" * 2_000, "routing_edges": []}], + } + bound_bytes = 256 + envelope = build_recipe_envelope( + {"success": True, "errors": ["x" * 2_000]}, + recipe_name="test-recipe", + artifact_path=str(tmp_path / "x.log"), + artifact_sha256="0" * 64, + skeleton=skeleton, + bound_bytes=bound_bytes, + ) + assert envelope["success"] is False + assert envelope["error"] == "recipe_envelope_exceeds_delivery_bound" + assert len(json.dumps(envelope, ensure_ascii=False).encode("utf-8")) <= bound_bytes + + +def test_envelope_priority_field_truncation_handles_multibyte_utf8(tmp_path: Path) -> None: + """Truncation of ``orchestration_rules`` does not split a multi-byte + UTF-8 codepoint mid-sequence. Constructs ``38 ASCII + 4-byte emoji`` + (42 bytes total) with allocation landing at byte 40 (the third byte + of the emoji, a continuation byte) — the naive byte-trim would + raise ``UnicodeDecodeError``; the new path backs off to the 38-byte + ASCII prefix via ``_safe_utf8_truncate``.""" + prefix = "a" * 38 + emoji = "\U0001f600" # 😀 — 4 UTF-8 bytes (0xF0 0x9F 0x98 0x80) + orchestration_rules = prefix + emoji + assert len(orchestration_rules.encode("utf-8")) == 42, "test invariant" + + payload = {"success": True, "orchestration_rules": orchestration_rules} + skeleton = extract_step_skeleton([], {}) + artifact_path = str(tmp_path / "x.log") + artifact_sha256 = "0" * 64 + + sk, pl, env = _envelope_overheads(tmp_path, "test-recipe") + # remaining = 65 bytes → pool = 40 → take = 40 → byte 40 lands in + # the middle of the 4-byte emoji (continuation byte at index 40). + # The +64 compensates for the 64-byte envelope serialization + # structural-overhead reserve (``build_recipe_envelope`` reserves + # 64 bytes for outer-key braces / separators / UTF-8 quoting the + # per-field allocator cannot account for). + bound_bytes = sk + pl + env + 65 + 64 + + envelope = build_recipe_envelope( + payload, + recipe_name="test-recipe", + artifact_path=artifact_path, + artifact_sha256=artifact_sha256, + skeleton=skeleton, + bound_bytes=bound_bytes, + ) + + # No UnicodeDecodeError raised means _safe_utf8_truncate backed off + # to the 38-byte ASCII prefix; the field must be present (take > 0) + # and a valid prefix of the original. + assert "orchestration_rules" in envelope + truncated = envelope["orchestration_rules"] + assert truncated.encode("utf-8") == prefix.encode("utf-8"), ( + f"truncation must back off to 38-byte ASCII prefix; " + f"got {len(truncated.encode('utf-8'))} bytes" + ) + assert orchestration_rules.startswith(truncated) + + +# --------------------------------------------------------------------------- +# Part B gap-closure regression guards (REQ-B04, REQ-B-T2/-T4/-T5) +# --------------------------------------------------------------------------- + + +def test_build_routing_edges_by_step_uses_canonical_extractor() -> None: + """The envelope's edge extraction must reuse the canonical + ``_extract_routing_edges`` from ``recipe/_analysis_graph.py`` rather + than the now-deleted local duplicate. Asserted independently: + ``build_routing_edges_by_step`` (which defaults to the canonical + extractor after the Step 1 change) and a direct call to + ``autoskillit.recipe._extract_routing_edges`` on each RecipeStep + must produce identical ``(edge_type, target)`` tuples. + + The previous local copy had two silent behavioral divergences + (a spurious falsy-target guard, and independent-vs-elif handling of + ``on_result.conditions``/``on_result.routes``); this test would + have caught both.""" + recipe_names = _recipe_names() + assert recipe_names, "no bundled recipes available for the equivalence test" + + for recipe_name in recipe_names: + info = find_recipe_by_name(recipe_name, _PROJECT_ROOT) + assert info is not None + recipe = load_recipe(info.path) + active_recipe_steps = recipe.steps + + default_edges = build_routing_edges_by_step( + active_recipe_steps, + edge_extractor=_extract_routing_edges, + ) + + for step_name, step in active_recipe_steps.items(): + canonical = [ + (edge.edge_type, edge.target) + for edge in _extract_routing_edges(step) + if edge.target + ] + assert default_edges.get(step_name) == canonical, ( + f"{recipe_name}.{step_name}: build_routing_edges_by_step " + f"default={default_edges.get(step_name)!r} " + f"differs from canonical _extract_routing_edges={canonical!r}" + ) + + +def test_extract_step_skeleton_includes_byte_ranges() -> None: + """``extract_step_skeleton`` gains an optional ``byte_ranges`` keyword; + a step present in the map gets a ``byte_range`` field carrying the + ``[start, end]`` pair, while a step absent from the map gets no + such key (fail-open, matching the existing edges/summary fallback).""" + byte_ranges = {"step_a": (10, 42)} + skeleton = extract_step_skeleton( + ["step_a", "step_b"], + routing_edges_by_step={}, + step_summaries={}, + byte_ranges=byte_ranges, + ) + by_name = {step["name"]: step for step in skeleton["steps"]} + assert by_name["step_a"].get("byte_range") == [10, 42], ( + "step present in byte_ranges must carry the byte_range field as a JSON list" + ) + assert "byte_range" not in by_name["step_b"], ( + "step absent from byte_ranges must get no byte_range key (fail-open)" + ) + + +def test_compute_step_byte_ranges_matches_step_boundaries() -> None: + """Each returned ``(start, end)`` slice covers the original step's + key + body in the source text. We assert substring containment + (rather than exact-equality) because YAML mark ranges commonly + include trailing whitespace up to the next sibling key.""" + content = ( + 'version: "1"\n' + "steps:\n" + " alpha:\n" + " tool: run_cmd\n" + " with_args:\n" + ' cmd: "echo alpha-body-marker"\n' + " beta:\n" + " tool: run_cmd\n" + " with_args:\n" + ' cmd: "echo beta-body-marker"\n' + ) + ranges = _compute_step_byte_ranges(content) + assert set(ranges.keys()) == {"alpha", "beta"}, ( + f"expected exactly the two top-level steps; got {sorted(ranges.keys())}" + ) + encoded = content.encode("utf-8") + for step_name, expected_marker in ( + ("alpha", "alpha-body-marker"), + ("beta", "beta-body-marker"), + ): + start, end = ranges[step_name] + slice_text = encoded[start:end].decode("utf-8", errors="replace") + assert expected_marker in slice_text, ( + f"{step_name}: marker {expected_marker!r} not in utf-8 decode of " + f"content[{start}:{end}] = {slice_text!r}" + ) + + +@pytest.mark.parametrize( + "malformed", + [ + "- a\n- b\n", # bare YAML sequence at root + "steps: oops\n", # steps: whose value is a scalar, not a mapping + ], + ids=["bare-sequence", "scalar-steps-value"], +) +def test_compute_step_byte_ranges_returns_empty_on_non_mapping_document( + malformed: str, +) -> None: + """``_compute_step_byte_ranges`` must fail open on any malformed or + non-mapping YAML document, returning ``{}`` without raising. The + guard is ``isinstance(..., yaml.MappingNode)`` rather than a bare + ``except yaml.YAMLError`` because a document that parses + successfully but isn't a mapping raises ``TypeError`` / + ``ValueError`` from tuple-unpacking (not ``YAMLError``).""" + assert _compute_step_byte_ranges(malformed) == {} + + +# --------------------------------------------------------------------------- +# Part B pull-tool end-to-end tests (REQ-B-T2, REQ-B-T4, REQ-B-T5) +# --------------------------------------------------------------------------- + + +_RECIPE_FOR_PULL = "remediation" + + +def _mock_fmcp_ctx() -> MagicMock: + """Return a minimal FastMCP ``Context`` mock with async component methods.""" + ctx = MagicMock() + ctx.enable_components = AsyncMock() + ctx.disable_components = AsyncMock() + return ctx + + +async def _open_kitchen_patched(name: str, overrides: dict | None, monkeypatch) -> dict: + """Call ``open_kitchen`` with all infrastructure side-effects patched out. + + Mirrors the helper in ``tests/server/test_serve_idempotence.py``: + patches ``_prime_quota_cache`` / ``_write_hook_config`` / + ``create_background_task`` / ``resolve_kitchen_id`` (open_kitchen's + background + identity side effects) and resets the recipe + ``_LOAD_CACHE`` so a fresh load + validate runs for every test. + """ + from autoskillit.recipe import _api_cache + from autoskillit.recipe._api_cache import LoadCache + from autoskillit.server.tools.tools_kitchen import open_kitchen + + monkeypatch.setattr(_api_cache, "_LOAD_CACHE", LoadCache()) + fmcp_ctx = _mock_fmcp_ctx() + with patch("autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock()): + with patch("autoskillit.server.tools.tools_kitchen._write_hook_config"): + with patch("autoskillit.server.tools.tools_kitchen.create_background_task"): + with patch( + "autoskillit.server.tools.tools_kitchen.resolve_kitchen_id", + return_value="test-kitchen", + ): + return json.loads( + await open_kitchen(name=name, overrides=overrides, ctx=fmcp_ctx) + ) + + +@pytest.mark.anyio +@pytest.mark.medium +async def test_load_recipe_envelope_pulls_from_its_own_artifact( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path +) -> None: + from autoskillit.server.tools.tools_recipe import get_recipe_section + from autoskillit.server.tools.tools_recipe import load_recipe as load_recipe_tool + + monkeypatch.chdir(tmp_path) + opened = await _open_kitchen_patched(_RECIPE_FOR_PULL, None, monkeypatch) + assert opened.get("success") is True + assert tool_ctx_kitchen_open.recipe_name == _RECIPE_FOR_PULL + + tool_ctx_kitchen_open.backend = BACKEND_REGISTRY["codex"]() + + # Force the load_recipe payload above the production 10,000-token + # bound so ``maybe_envelope_recipe_response`` actually builds an + # envelope (its recipe_pull + step_flow_skeleton + delivery_bound_spill + # trio is the artifact the test below pulls from). When the payload + # fits, ``maybe_envelope_recipe_response`` short-circuits to raw + # JSON and ``loaded["recipe_pull"]`` never exists — defeating the + # *envelope-pulls-from-its-own-artifact* contract. + _tight_backend = MagicMock() + _tight_backend.capabilities = BackendCapabilities( + effective_delivery_token_limit=1_000, + ) + _tight_backend.name = "codex" + tool_ctx_kitchen_open.backend = _tight_backend + + loaded = json.loads(await load_recipe_tool(name="implementation")) + pull = loaded["recipe_pull"] + assert pull["recipe_name"] == "implementation" + assert pull["producer_tool"] == "load_recipe" + assert tool_ctx_kitchen_open.recipe_name == _RECIPE_FOR_PULL + + # When the bound is tight enough to trigger delivery-bound spill, the + # envelope may drop ``step_flow_skeleton`` / ``artifact_path`` from + # ``recipe_pull`` to fit. Reconstruct the artifact path the same way + # the pull tool does (deterministic from temp_dir + producer_tool + + # recipe_name) and look up the post-prune step list from the + # persisted payload. + artifact_path_str = pull.get("artifact_path") or str( + _recipe_artifact_path( + tool_ctx_kitchen_open.temp_dir, pull["producer_tool"], pull["recipe_name"] + ) + ) + skeleton = loaded.get("step_flow_skeleton") + if skeleton is not None: + step_name = skeleton["steps"][0]["name"] + else: + persisted = json.loads(Path(artifact_path_str).read_text(encoding="utf-8")) + post_prune_raw = cast(list[object], persisted.get("post_prune_step_names") or []) + step_name = next(str(name) for name in post_prune_raw if isinstance(name, str)) + + response = json.loads( + await get_recipe_section( + section=step_name, + recipe_name=pull["recipe_name"], + producer_tool=pull["producer_tool"], + artifact_path=artifact_path_str, + artifact_sha256=pull.get("sha256"), + ) + ) + assert response.get("success") is True, response + assert response["content"] + + +@pytest.mark.anyio +@pytest.mark.medium +async def test_pull_tool_returns_bounded_step_content( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path +) -> None: + """Live ``get_recipe_section`` end-to-end against the persisted + artifact (REQ-B-T2): every post-prune step name returns the bounded + step body matching the canonical serializer, the response itself + fits the smallest backend bound, and an unknown section name + produces ``{"success": False, "error": "section_not_found"}``.""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + monkeypatch.chdir(tmp_path) + + ok_result = await _open_kitchen_patched( + _RECIPE_FOR_PULL, + None, + monkeypatch, + ) + assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" + + persisted = json.loads( + ( + artifact_path := _recipe_artifact_path( + tool_ctx_kitchen_open.temp_dir, "open_kitchen", _RECIPE_FOR_PULL + ) + ).read_text(encoding="utf-8") + ) + artifact_sha256 = _artifact_sha256(artifact_path) + persisted_yaml = persisted.get("content", "") or "" + parsed = load_yaml(persisted_yaml) + assert isinstance(parsed, dict), "persisted content must parse as a mapping" + parsed_steps = parsed.get("steps", {}) + assert isinstance(parsed_steps, dict) + + post_prune_raw = cast(list[object], persisted.get("post_prune_step_names") or []) + step_names = [str(n) for n in post_prune_raw if isinstance(n, str)] + assert step_names, "recipe must expose at least one post-prune step" + + bound_tokens = _smallest_bound_tokens() + bound_bytes_for_response = bound_tokens * 4 + missing_identity = json.loads(await get_recipe_section(section=step_names[0])) + assert missing_identity == { + "success": False, + "error": "recipe_artifact_identity_required", + } + for step_name in step_names: + response = json.loads( + await get_recipe_section( + section=step_name, + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert response.get("success") is True, ( + f"get_recipe_section({step_name!r}) failed: {response}" + ) + expected = fast_dumps({step_name: parsed_steps[step_name]}) + assert response["section"] == step_name + assert response["content"] == expected, ( + f"step {step_name!r}: pull content diverges from " + f"fast_dumps({{step: parsed['steps'][step_name]}})" + ) + serialized = json.dumps(response, ensure_ascii=False) + assert len(serialized.encode("utf-8")) <= bound_bytes_for_response, ( + f"step {step_name!r}: response exceeds the backend bound " + f"({bound_bytes_for_response} bytes); got {len(serialized.encode('utf-8'))}" + ) + + unknown = json.loads( + await get_recipe_section( + section="not_a_real_step", + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert unknown == { + "success": False, + "error": "section_not_found", + "section": "not_a_real_step", + } + + persisted["post_prune_step_names"] = [] + _, artifact_sha256 = persist_recipe_artifact( + tool_ctx_kitchen_open.temp_dir, + tool_name="open_kitchen", + recipe_name=_RECIPE_FOR_PULL, + payload=persisted, + ) + pruned = json.loads( + await get_recipe_section( + section=step_names[0], + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert pruned == { + "success": False, + "error": "section_not_found", + "section": step_names[0], + } + + +@pytest.mark.anyio +@pytest.mark.medium +async def test_artifact_recreation_from_parsed_recipe( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path +) -> None: + """When the persisted artifact is deleted, ``get_recipe_section`` + re-creates it via the same serve pipeline that built it originally + (REQ-B-T4). Digest binding rejects a recreation that does not match + the exact original artifact. When the recreation pipeline returns + an invalid recipe, the pull tool surfaces a structured + ``recipe_artifact_unavailable`` envelope (driven here via a + ``serve_recipe`` monkeypatch, since the literal "no active recipe" + condition is checked earlier and produces a different error).""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + monkeypatch.chdir(tmp_path) + + ok_result = await _open_kitchen_patched(_RECIPE_FOR_PULL, None, monkeypatch) + assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" + + artifact_path = _recipe_artifact_path( + tool_ctx_kitchen_open.temp_dir, "open_kitchen", _RECIPE_FOR_PULL + ) + artifact_sha256 = _artifact_sha256(artifact_path) + artifact_path.unlink() + assert not artifact_path.exists(), "precondition: artifact file deleted" + + post_prune_raw = cast(list[object], ok_result.get("post_prune_step_names") or []) + step_name = next( + (str(n) for n in post_prune_raw if isinstance(n, str)), + None, + ) + assert step_name is not None, "recipe must expose at least one post-prune step" + + response = json.loads( + await get_recipe_section( + section=step_name, + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert response == { + "success": False, + "error": "invalid_recipe_artifact_identity", + } + assert artifact_path.exists(), "artifact must be rewritten by the recreation path on miss" + + # Now drive the invalid-recreation branch via a monkeypatched + # ``serve_recipe`` — get_recipe_section imports it into its own + # module namespace, so we must override the attribute on the + # importing module (``tools_recipe``) for the patch to take + # effect. The pulled result must be a structured + # ``recipe_artifact_unavailable`` envelope rather than a generic + # error. + artifact_path.unlink() + import autoskillit.server.tools.tools_recipe as _tools_recipe_mod # noqa: PLC0415 + + def _fake_serve_recipe(*args, **kwargs) -> dict: + # Reference args/kwargs so linters see them as "used" — the patch + # accepts any signature, we just need to return invalid-recipe. + del args, kwargs + return {"valid": False, "content": "x"} + + monkeypatch.setattr(_tools_recipe_mod, "serve_recipe", _fake_serve_recipe) + failed = json.loads( + await get_recipe_section( + section=step_name, + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert failed.get("success") is False + assert failed["error"] == "recipe_artifact_unavailable" + assert failed["detail"] == "recreation returned invalid recipe" + + +@pytest.mark.anyio +@pytest.mark.medium +async def test_pull_tool_rejects_non_mapping_artifact( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path +) -> None: + """Valid JSON with the wrong top-level shape is artifact corruption.""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + monkeypatch.chdir(tmp_path) + ok_result = await _open_kitchen_patched(_RECIPE_FOR_PULL, None, monkeypatch) + assert ok_result.get("success") is True + + artifact_path = _recipe_artifact_path( + tool_ctx_kitchen_open.temp_dir, "open_kitchen", _RECIPE_FOR_PULL + ) + artifact_path.write_text('["not", "a", "mapping"]', encoding="utf-8") + response = json.loads( + await get_recipe_section( + section="content", + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=_artifact_sha256(artifact_path), + ) + ) + + assert response == { + "success": False, + "error": "recipe_artifact_unavailable", + "detail": "persisted recipe artifact is not a mapping", + } + + +@pytest.mark.anyio +@pytest.mark.medium +async def test_large_step_chunked_via_continuation( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path +) -> None: + """Persist a synthetic oversized artifact whose single ``giant_step`` + body exceeds the smallest backend's bound; driving + ``get_recipe_section`` against ``backend = None`` (which forces + the 10,000-token / 40,000-byte fallback) produces a chunked + response with ``has_more=True`` and a ``next_part`` cursor; + following that cursor concatenates the chunks back into the + canonical serializer's output (REQ-B-T5).""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + monkeypatch.chdir(tmp_path) + + # Open the kitchen first so the tool_ctx has recipes wired and + # ``recipe_name`` is set — the giant-step payload is written + # directly to the artifact path to bypass the size guard. + ok_result = await _open_kitchen_patched(_RECIPE_FOR_PULL, None, monkeypatch) + assert ok_result.get("success") is True + + artifact_path = _recipe_artifact_path( + tool_ctx_kitchen_open.temp_dir, "open_kitchen", _RECIPE_FOR_PULL + ) + oversized_field = "X" * 80_000 # ~80KB >> 40KB Codex bound + persisted_payload = { + "success": True, + "content": (f'version: "1"\nsteps:\n giant_step:\n note: {oversized_field}\n'), + "post_prune_step_names": ["giant_step"], + } + _, artifact_sha256 = persist_recipe_artifact( + tool_ctx_kitchen_open.temp_dir, + tool_name="open_kitchen", + recipe_name=_RECIPE_FOR_PULL, + payload=persisted_payload, + ) + assert artifact_path.exists() + + # Force the 40KB fallback bound (no backend → bound_tokens=10_000). + tool_ctx_kitchen_open.backend = None # type: ignore[assignment] + tool_ctx_kitchen_open.recipe_name = _RECIPE_FOR_PULL + + expected = fast_dumps( + {"giant_step": load_yaml(persisted_payload["content"])["steps"]["giant_step"]} + ) + + chunks: list[str] = [] + part = 0 + has_more = True + while has_more: + response = json.loads( + await get_recipe_section( + section="giant_step", + part=part, + recipe_name=_RECIPE_FOR_PULL, + producer_tool="open_kitchen", + artifact_sha256=artifact_sha256, + ) + ) + assert response.get("success") is True, f"chunk {part} failed: {response}" + assert response["section"] == "giant_step" + chunks.append(response["content"]) + has_more = bool(response.get("has_more")) + if has_more: + assert "next_part" in response, ( + f"chunk {part} reports has_more=True without next_part cursor" + ) + part = int(response["next_part"]) + else: + assert "next_part" not in response + + assert "".join(chunks) == expected, ( + "chunked pull content does not round-trip to the canonical serializer's output" + )