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/execution/architecture.md b/docs/execution/architecture.md index 02f43e83e..cd978a83a 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 @@ -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..5c6f449b0 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 @@ -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/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/config/ingredient_defaults.py b/src/autoskillit/config/ingredient_defaults.py index 32b264af5..b8ab7a654 100644 --- a/src/autoskillit/config/ingredient_defaults.py +++ b/src/autoskillit/config/ingredient_defaults.py @@ -33,7 +33,16 @@ "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..746bb337b 100644 --- a/src/autoskillit/core/__init__.pyi +++ b/src/autoskillit/core/__init__.pyi @@ -389,6 +389,7 @@ from .types import TimingLog as TimingLog from .types import TokenFactory as TokenFactory from .types import TokenLog as TokenLog from .types import TraditionManifest as TraditionManifest +from .types import TruncationBoundsError as TruncationBoundsError from .types import ValidatedAddDir as ValidatedAddDir from .types import ValidatedWorktreePath as ValidatedWorktreePath from .types import WorkspaceManager as WorkspaceManager diff --git a/src/autoskillit/core/types/_type_constants_registries.py b/src/autoskillit/core/types/_type_constants_registries.py index eea0ce5c2..3d50b65f8 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", # Part B Step 2.3 — bounded pull tool } ) @@ -176,6 +177,7 @@ "open_kitchen_deferred_recall", # S2 — deferred-recall re-serve "load_recipe", # S3 — re-serve tool "get_recipe", # S4 — MCP resource handler + "get_recipe_section", # S5 — bounded pull tool for step/section content } ) @@ -337,6 +339,10 @@ class AgentPackDef(NamedTuple): "merge_worktree": frozenset({"kitchen-core"}), "unlock_agent_pack": frozenset({"kitchen-core"}), "record_pipeline_step": frozenset({"kitchen-core"}), + # Part B Step 2.3: get_recipe_section is a kitchen-core-tagged tool. + # It pulls recipe content on demand; food trucks don't need it (they + # receive the full recipe via dispatch) so no fleet tag is required. + "get_recipe_section": frozenset({"kitchen-core"}), } ALL_VISIBILITY_TAGS: frozenset[str] = frozenset( diff --git a/src/autoskillit/core/types/_type_exceptions.py b/src/autoskillit/core/types/_type_exceptions.py index fbb74b085..b39a2a22a 100644 --- a/src/autoskillit/core/types/_type_exceptions.py +++ b/src/autoskillit/core/types/_type_exceptions.py @@ -7,6 +7,7 @@ "RecipeLoadError", "ProcessStaleError", "RecipeNotFoundError", + "TruncationBoundsError", ] @@ -29,3 +30,13 @@ def __init__(self, capability: str, backend_name: str) -> None: self.capability = capability self.backend_name = backend_name super().__init__(f"{backend_name!r} does not support capability {capability!r}") + + +class TruncationBoundsError(Exception): + """Internal sizing invariant violated: bounds too small for the truncation sentinel. + + Deliberately not a ``RecipeLoadError``/``ValueError`` subclass so it is not + conflated with recipe-authoring errors by ``load_and_validate``'s + ``except ValueError`` handler — it signals a misconfigured internal + constant, not a problem with the recipe being loaded. + """ diff --git a/src/autoskillit/execution/__init__.py b/src/autoskillit/execution/__init__.py index 34ce6816e..fb6a99ec9 100644 --- a/src/autoskillit/execution/__init__.py +++ b/src/autoskillit/execution/__init__.py @@ -35,6 +35,7 @@ ensure_codex_mcp_registered, generate_codex_hooks_config, get_backend, + resolve_worst_case_delivery_bound, sync_hooks_to_codex_config, ) from autoskillit.execution.ci import DefaultCIWatcher @@ -245,6 +246,7 @@ "ensure_codex_mcp_registered", "generate_codex_hooks_config", "sync_hooks_to_codex_config", + "resolve_worst_case_delivery_bound", # anomaly_detection "detect_anomalies", "AnomalyKind", diff --git a/src/autoskillit/execution/backends/__init__.py b/src/autoskillit/execution/backends/__init__.py index d7ba36891..c1c0044b3 100644 --- a/src/autoskillit/execution/backends/__init__.py +++ b/src/autoskillit/execution/backends/__init__.py @@ -60,6 +60,23 @@ def get_backend(name: str) -> CodingAgentBackend: return cls() +def resolve_worst_case_delivery_bound() -> int: + """Smallest ``effective_delivery_token_limit`` across all registered backends. + + Acts as the canonical "worst-case default" used when backend capabilities + are unavailable or zero. Returns ``0`` only if every registered backend + reports zero; production callers normalize that case at the enforcement + boundary rather than here. + """ + limits: list[int] = [] + for backend_cls in BACKEND_REGISTRY.values(): + caps = backend_cls().capabilities + limit = getattr(caps, "effective_delivery_token_limit", 0) + if limit > 0: + limits.append(limit) + return min(limits) if limits else 0 + + __all__ = [ "BACKEND_REGISTRY", "CODEX_EXEC_FLAGS", @@ -95,4 +112,5 @@ def get_backend(name: str) -> CodingAgentBackend: "ensure_codex_mcp_registered", "get_backend", "make_codex_scenario_player", + "resolve_worst_case_delivery_bound", ] diff --git a/src/autoskillit/hooks/formatters/_fmt_recipe.py b/src/autoskillit/hooks/formatters/_fmt_recipe.py index 85c7b6924..0024fd4ab 100644 --- a/src/autoskillit/hooks/formatters/_fmt_recipe.py +++ b/src/autoskillit/hooks/formatters/_fmt_recipe.py @@ -219,6 +219,11 @@ def _fmt_load_recipe(data: LoadRecipeResult, pipeline: bool) -> str: "post_prune_step_names", # 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 + "step_flow_skeleton", # compact envelope routing graph; dedicated formatter is Part B + "step_index", # compact envelope step identifier map; dedicated formatter is Part B + "artifact_path", # compact envelope pull-tool metadata; dedicated formatter is Part B + "sha256", # compact envelope pull-tool metadata; dedicated formatter is Part B + "pull_tool", # compact envelope pull-tool metadata; dedicated formatter is Part B } ) diff --git a/src/autoskillit/hooks/formatters/pretty_output_hook.py b/src/autoskillit/hooks/formatters/pretty_output_hook.py index f10c3f803..a9331348e 100644 --- a/src/autoskillit/hooks/formatters/pretty_output_hook.py +++ b/src/autoskillit/hooks/formatters/pretty_output_hook.py @@ -154,6 +154,7 @@ def _response_spill_notice(metadata: dict) -> str: # When adding a new tool, it MUST appear either in _FORMATTERS or here. _UNFORMATTED_TOOLS: frozenset[str] = frozenset( { + "get_recipe_section", # recipe artifact pull envelope, generic renders correctly "run_python", # structured result dict, generic renders correctly "read_db", # tabular rows, generic renders correctly "reset_test_dir", # simple ack diff --git a/src/autoskillit/pipeline/context.py b/src/autoskillit/pipeline/context.py index 04eac17cf..b45942642 100644 --- a/src/autoskillit/pipeline/context.py +++ b/src/autoskillit/pipeline/context.py @@ -126,6 +126,12 @@ class ToolContext: active_recipe_ingredients: frozenset[str] | None — ingredient keys declared by the loaded recipe (frozenset() when kitchen open but no recipe loaded; None when closed) + recipe_artifact_state: dict[str, Any] | None — server-side cache of the most recently + persisted recipe artifact (artifact_path, sha256, and the recipe-load + parameters needed to recreate it via load_and_validate). Set by + open_kitchen / load_recipe after the artifact is written; read by + get_recipe_section for sha256-verified retrieval. None when no + recipe has been loaded in this session. temp_dir: Resolved temp directory for this project. Sentinel-guarded: raises TypeError if not supplied explicitly. Use make_context() or pass temp_dir=. @@ -181,6 +187,7 @@ class ToolContext: active_recipe_ingredients: frozenset[str] | None = field(default_factory=lambda: None) session_serve_overrides: ServeOverridesSnapshot | None = field(default_factory=lambda: None) session_serve_defer_unresolved: bool = field(default=False) + recipe_artifact_state: dict[str, Any] | None = field(default_factory=lambda: None) quota_refresh_task: QuotaRefreshTask | None = field(default=None) token_factory: TokenFactory | None = field(default=None) fleet_lock: FleetLock | None = field(default=None) diff --git a/src/autoskillit/recipe/_api.py b/src/autoskillit/recipe/_api.py index 982ae394b..0705fa796 100644 --- a/src/autoskillit/recipe/_api.py +++ b/src/autoskillit/recipe/_api.py @@ -20,6 +20,7 @@ RecipeNotFoundError, RecipeSource, SkillLister, + TruncationBoundsError, YAMLError, get_logger, pkg_root, @@ -105,6 +106,55 @@ logger = get_logger(__name__) +_MAX_SUGGESTIONS_COUNT = 50 +_MAX_SUGGESTIONS_BYTES = 16_384 + + +def _json_utf8_bytes(value: Any) -> int: + return len(json.dumps(value, ensure_ascii=False).encode("utf-8")) + + +def _bounded_append( + target: list[Any], + items: list[Any] | dict[str, Any], + max_count: int, + max_bytes: int, +) -> None: + """Append findings while retaining a bounded prefix and one truncation sentinel.""" + incoming = [items] if isinstance(items, dict) else list(items) + existing_sentinel = ( + target[-1] + if target and isinstance(target[-1], dict) and target[-1].get("truncated") is True + else None + ) + if existing_sentinel is not None: + previous_count = existing_sentinel.get("original_count", len(target) - 1) + original_count = ( + previous_count if isinstance(previous_count, int) else len(target) - 1 + ) + len(incoming) + candidates = [*target[:-1], *incoming] + else: + original_count = len(target) + len(incoming) + candidates = [*target, *incoming] + if len(candidates) <= max_count and _json_utf8_bytes(candidates) <= max_bytes: + target[:] = candidates + return + + kept = candidates[: max(0, max_count - 1)] + while True: + sentinel = { + "truncated": True, + "original_count": original_count, + "shown_count": len(kept), + } + bounded = [*kept, sentinel] + if len(bounded) <= max_count and _json_utf8_bytes(bounded) <= max_bytes: + target[:] = bounded + return + if not kept: + raise TruncationBoundsError("finding bounds are too small for the truncation sentinel") + kept.pop() + def _t(label: str, t0: float, name: str) -> float: """Log elapsed time for a pipeline stage and return current time. @@ -367,11 +417,18 @@ def load_and_validate( ) # Stage: structural validation on active recipe - errors = validate_recipe_structure(active_recipe) + raw_errors = validate_recipe_structure(active_recipe) + errors.clear() + _bounded_append(errors, raw_errors, _MAX_SUGGESTIONS_COUNT, _MAX_SUGGESTIONS_BYTES) if combined_recipe is not None: # Dual validation: also validate the combined (merged) graph combined_errors = validate_recipe_structure(combined_recipe) - errors.extend(f"[combined] {e}" for e in combined_errors) + _bounded_append( + errors, + [f"[combined] {e}" for e in combined_errors], + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, + ) t0 = _t("validate_recipe_structure", t0, name) # Stage: resolve skill_resolver (needed by both pre-prune and post-prune contexts) @@ -419,14 +476,22 @@ def load_and_validate( # Must run inside try so active_recipe and errors are both in scope. _dangling_errors = _validate_no_dangling_routes(active_recipe) if _dangling_errors: - errors.extend(f"[post-prune] dangling route: {e}" for e in _dangling_errors) + _bounded_append( + errors, + [f"[post-prune] dangling route: {e}" for e in _dangling_errors], + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, + ) raw = "" # Cross-check: raw YAML route refs must match the Python model exactly. # Catches any refs that the model repaired but the YAML repair pass missed. _route_consistency_errors = _validate_route_consistency(raw, active_recipe) if _route_consistency_errors: - errors.extend( - f"[post-prune] route consistency: {e}" for e in _route_consistency_errors + _bounded_append( + errors, + [f"[post-prune] route consistency: {e}" for e in _route_consistency_errors], + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, ) raw = "" t0 = _t("prune_skipped_steps", t0, name) @@ -521,7 +586,9 @@ def load_and_validate( _suppressed = suppressed or [] if name in _suppressed: semantic_suggestions = filter_version_rule(semantic_suggestions) - suggestions.extend(semantic_suggestions) + _bounded_append( + suggestions, semantic_suggestions, _MAX_SUGGESTIONS_COUNT, _MAX_SUGGESTIONS_BYTES + ) # Stage: hidden ingredient interpolation raw = _resolve_hidden_inputs_in_content(raw, active_recipe, ingredient_overrides) @@ -532,7 +599,12 @@ def load_and_validate( contract_findings: list[dict[str, Any]] = [] if contract: contract_findings = validate_recipe_cards(active_recipe, contract) - suggestions.extend(contract_findings) + _bounded_append( + suggestions, + contract_findings, + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, + ) t0 = _t("contract_card", t0, name) # Stage: staleness check @@ -542,47 +614,66 @@ def load_and_validate( stale = check_contract_staleness( contract, recipe_path=match.path, cache_path=staleness_cache_path ) - suggestions.extend(stale_to_suggestions(stale)) + _bounded_append( + suggestions, + stale_to_suggestions(stale), + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, + ) t0 = _t("staleness_check", t0, name) # Stage: diagram if check_diagram_staleness(name, recipes_dir, match.path): - suggestions.extend(diagram_stale_to_suggestions(name)) + _bounded_append( + suggestions, + diagram_stale_to_suggestions(name), + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, + ) t0 = _t("diagram", t0, name) valid = compute_recipe_validity(errors, semantic_findings, contract_findings) except YAMLError as exc: logger.warning("Recipe YAML parse error", name=name, exc_info=True) - suggestions.append( + _bounded_append( + suggestions, { "rule": "validation-error", "severity": "error", "step": "(validation-pipeline)", "message": f"YAML parse error: {exc}", - } + }, + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, ) valid = False except ValueError as exc: logger.warning("Recipe structure invalid", name=name, exc_info=True) - suggestions.append( + _bounded_append( + suggestions, { "rule": "validation-error", "severity": "error", "step": "(validation-pipeline)", "message": f"Invalid recipe structure: {exc}", - } + }, + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, ) valid = False except (FileNotFoundError, OSError) as exc: logger.warning("Recipe file not found or unreadable", name=name, exc_info=True) - suggestions.append( + _bounded_append( + suggestions, { "rule": "validation-error", "severity": "error", "step": "(validation-pipeline)", "message": f"File error: {exc}", - } + }, + _MAX_SUGGESTIONS_COUNT, + _MAX_SUGGESTIONS_BYTES, ) valid = False diff --git a/src/autoskillit/recipe/_recipe_ingredients.py b/src/autoskillit/recipe/_recipe_ingredients.py index b49c6c814..6beaae75f 100644 --- a/src/autoskillit/recipe/_recipe_ingredients.py +++ b/src/autoskillit/recipe/_recipe_ingredients.py @@ -136,7 +136,9 @@ class LoadRecipeResult(TypedDict, total=False): class OpenKitchenResult(TypedDict, total=False): """Typed schema for the open_kitchen named-recipe handler → formatter boundary. - Extends LoadRecipeResult with four post-return keys injected by the handler. + Extends LoadRecipeResult with four post-return keys injected by the handler, + plus the compact envelope fields (Part B) that replace inline content on the + wire response. """ # Inherited from LoadRecipeResult (20 keys) @@ -166,6 +168,12 @@ class OpenKitchenResult(TypedDict, total=False): kitchen: str version: str hook_warning: str + # Compact envelope fields (build_recipe_envelope) — replace content on the wire + step_flow_skeleton: list[dict[str, Any]] + step_index: dict[str, str] + artifact_path: str + sha256: str + pull_tool: str class RecipeListItem(TypedDict): diff --git a/src/autoskillit/server/_notify.py b/src/autoskillit/server/_notify.py index d68b902bd..647d43063 100644 --- a/src/autoskillit/server/_notify.py +++ b/src/autoskillit/server/_notify.py @@ -12,15 +12,11 @@ from anyio import ClosedResourceError as _ClosedResource from autoskillit.config import OutputBudgetConfig -from autoskillit.core import ( - RESERVED_LOG_RECORD_KEYS, - BackendCapabilities, - get_logger, - resolve_effective_delivery_bound, -) +from autoskillit.core import RESERVED_LOG_RECORD_KEYS, get_logger from autoskillit.server._response_budget import ( bounded_response_budget_failure, enforce_response_budget, + resolve_effective_or_fallback_delivery_bound, ) if TYPE_CHECKING: @@ -142,12 +138,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any: artifact_dir = ( temp_dir / "responses" / tool_name if isinstance(temp_dir, Path) else None ) - effective_delivery_token_limit: int | None = None - if ctx is not None: - backend = getattr(ctx, "backend", None) - caps = getattr(backend, "capabilities", None) if backend is not None else None - if isinstance(caps, BackendCapabilities): - effective_delivery_token_limit = resolve_effective_delivery_bound(caps) + effective_delivery_token_limit = resolve_effective_or_fallback_delivery_bound(ctx) try: result = enforce_response_budget( result, diff --git a/src/autoskillit/server/_response_budget.py b/src/autoskillit/server/_response_budget.py index a67cc0d2c..74cc1d9b6 100644 --- a/src/autoskillit/server/_response_budget.py +++ b/src/autoskillit/server/_response_budget.py @@ -11,9 +11,14 @@ from autoskillit.core import ( RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, + BackendCapabilities, atomic_write, + dump_yaml_str, get_logger, + load_yaml, + resolve_effective_delivery_bound, ) +from autoskillit.execution import resolve_worst_case_delivery_bound if TYPE_CHECKING: from autoskillit.config import OutputBudgetConfig @@ -220,9 +225,19 @@ def bounded_response_budget_failure( return {"success": False, "error": "response_budget_failure"} -def _artifact_path(artifact_dir: Path, tool_name: str) -> Path: +def _artifact_path(artifact_dir: Path, tool_name: str, content_sha256: str) -> Path: + """Compute a deterministic artifact path keyed by tool_name + content hash. + + The deterministic scheme makes the artifact path stable across calls with the + same content, so the pull tool (`get_recipe_section`) and the response + envelope agree on a single path. Falls back to a UUID-derived slug only when + ``content_sha256`` is empty (kept narrow on purpose). + """ safe_name = "".join(c if c.isalnum() or c in "._-" else "_" for c in tool_name) - return artifact_dir / f"{safe_name or 'response'}_{uuid.uuid4().hex[:8]}.log" + digest = (content_sha256 or "")[:16] + if not digest: + digest = uuid.uuid4().hex[:16] + return artifact_dir / f"{safe_name or 'response'}_{digest}.log" def _finalize_envelope(envelope: dict[str, Any], *, max_bytes: int) -> str: @@ -367,10 +382,116 @@ def _plain_spill_envelope( "stop_step_semantics", "errors", "suggestions", + # Part B envelope fields — must survive any second-pass delivery-bound + # projection so the pull tool can still resolve the persisted artifact. + "step_flow_skeleton", + "step_index", + "pull_tool", + "artifact_path", + "sha256", ) _DELIVERY_BOUND_CONTENT_KEY = "content" _DELIVERY_BOUND_DROPPABLE_KEYS: tuple[str, ...] = ("diagram",) +_STEP_ROUTING_FIELDS: tuple[str, ...] = ( + "on_success", + "on_failure", + "on_result", + "on_context_limit", +) + + +def extract_step_routing(content: str, step_names: list[str]) -> list[dict[str, Any]]: + """Return per-step routing/summary metadata as a list of dicts. + + Each dict contains the step ``name``, ``summary`` (if present), and the + routing fields (``on_success``, ``on_failure``, ``on_result``, + ``on_context_limit``) when set. Used by both ``build_recipe_envelope`` + (Part B primary delivery shape) and ``_extract_step_skeleton`` (Part A + backstop / byte-floor sizing) so the two paths agree on the routing set. + + Falls back to one minimal entry per step when YAML parsing fails — no + summary, no routing fields, just the name so consumers can still locate + each step in the bounded payload. + """ + if not step_names: + return [] + parsed_content: Any = None + try: + parsed_content = load_yaml(content) + except Exception: + logger.warning("step_routing_yaml_parse_failed", exc_info=True) + parsed_content = None + + out: list[dict[str, Any]] = [] + if isinstance(parsed_content, dict): + steps_obj = parsed_content.get("steps") + for step_name in step_names: + entry: dict[str, Any] = {"name": step_name} + step_obj = steps_obj.get(step_name) if isinstance(steps_obj, dict) else None + if isinstance(step_obj, dict): + summary_value = step_obj.get("summary") + if isinstance(summary_value, str): + entry["summary"] = summary_value + for field in _STEP_ROUTING_FIELDS: + if field in step_obj: + entry[field] = step_obj[field] + out.append(entry) + return out + + # Fallback: synthetic / unparseable content. Emit one minimal entry per + # step name so consumers can still enumerate the step set. + return [{"name": name} for name in step_names] + + +def _extract_step_skeleton(content: str, step_names: list[str]) -> str: + """Build a minimal YAML skeleton containing only routing fields for the given steps. + + Thin Part-A wrapper around ``extract_step_routing``: serializes only the + routing-field subset (dropping ``summary``) via ``dump_yaml_str`` for the + byte-floor sizing use case in ``_delivery_bound_summary``. Both Part A + and Part B call the shared routing extractor to keep the routing-field + set in lock-step. + + Falls back to a substring-tagged plain-text skeleton when YAML parsing + fails (e.g., synthetic test payloads with malformed or pseudo-YAML text). + The resulting skeleton carries every ``step_name`` as a substring so + downstream consumers can locate each step in the bounded payload. + """ + if not step_names: + return "" + parsed_content: Any = None + skeleton_yaml: str | None = None + try: + parsed_content = load_yaml(content) + except Exception: + logger.warning("step_skeleton_yaml_parse_failed", exc_info=True) + parsed_content = None + if isinstance(parsed_content, dict): + steps = parsed_content.get("steps") + skeleton_steps: dict[str, dict[str, Any]] = {} + if isinstance(steps, dict): + for step_name in step_names: + step_obj = steps.get(step_name) + if not isinstance(step_obj, dict): + skeleton_steps[step_name] = {} + continue + routing = { + field: step_obj[field] for field in _STEP_ROUTING_FIELDS if field in step_obj + } + skeleton_steps[step_name] = routing + else: + skeleton_steps = {name: {} for name in step_names} + if skeleton_steps: + try: + skeleton_yaml = dump_yaml_str({"steps": skeleton_steps}, default_flow_style=False) + except Exception: + logger.warning("step_skeleton_dump_failed", exc_info=True) + skeleton_yaml = None + if skeleton_yaml is not None: + return skeleton_yaml + return "\n".join(f" {name}:" for name in step_names) + "\n" + def _delivery_bound_summary( parsed: dict[str, Any], @@ -434,7 +555,11 @@ def _build(head_len: int, include_droppable: bool, value_projector=None) -> dict if content_is_str: text = content_text or "" - truncated = text[:head_len] + skeleton_len = len(skeleton_text) + if skeleton_text: + truncated = text[: max(0, head_len - skeleton_len)] + skeleton_text + else: + 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: @@ -469,44 +594,70 @@ def _fits(envelope: dict[str, Any]) -> str | None: 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 - - while head_limit > 0 and content_is_str: - rendered = _fits(_build(head_limit, True)) - 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) - if rendered is not None: - return rendered + post_prune_step_names_raw = parsed.get("post_prune_step_names", []) + step_names_for_floor: list[str] = ( + list(post_prune_step_names_raw) if isinstance(post_prune_step_names_raw, list) else [] + ) + if content_is_str and step_names_for_floor: + skeleton_text = _extract_step_skeleton(content_text or "", step_names_for_floor) + content_floor = len(skeleton_text.encode("utf-8")) + else: + skeleton_text = "" + content_floor = 0 + + def _compute_head_limit(value_projector: Any = None) -> int: + probe_envelope = _build(0, True, value_projector=value_projector) + probe_rendered = _finalize_envelope(probe_envelope, max_bytes=bound) + probe_bytes = len(probe_rendered.encode("utf-8")) + if not content_is_str: + return 0 + return max(content_floor, bound - probe_bytes - 64) + + def _attempt_with_projector(value_projector: Any) -> str | None: + if not content_is_str: + rendered = _fits(_build(0, False, value_projector=value_projector)) + if rendered is not None: + return rendered + return None + head_limit = _compute_head_limit(value_projector) + if head_limit < content_floor: + return None + candidate = head_limit + while candidate >= content_floor: + rendered = _fits(_build(candidate, True, value_projector=value_projector)) + if rendered is not None: + return rendered + rendered = _fits(_build(candidate, False, value_projector=value_projector)) + if rendered is not None: + return rendered + if candidate == content_floor: + break + candidate = max(content_floor, candidate // 2) + return None - rendered = _fits(_build(0, False)) + rendered = _attempt_with_projector(None) if rendered is not None: return rendered - if present_preserved: - value_limit = max(16, bound // (len(present_preserved) + 2)) - while value_limit >= 16: + if not present_preserved: + return None - def _project_with_limit(value: Any, _limit: int = value_limit) -> Any: - projected, _chars, _items = _project_value(value, _limit) - return projected + value_limit = max(16, bound // (len(present_preserved) + 2)) + while value_limit >= 16: - rendered = _fits(_build(0, False, value_projector=_project_with_limit)) - if rendered is not None: - return rendered - value_limit //= 2 + def _project_with_limit(value: Any, _limit: int = value_limit) -> Any: + projected, _chars, _items = _project_value(value, _limit) + return projected - minimal_envelope = _build(0, False, value_projector=_minimal_same_type) - rendered = _fits(minimal_envelope) + rendered = _attempt_with_projector(_project_with_limit) if rendered is not None: return rendered + value_limit //= 2 + + minimal_envelope = _build(0, False, value_projector=_minimal_same_type) + rendered = _fits(minimal_envelope) + if rendered is not None: + return rendered return None @@ -520,6 +671,8 @@ def _spill_for_delivery_bound( original: str, original_size: int, effective_delivery_token_limit: int, + pre_published_artifact_path: str | None = None, + pre_published_sha256: str | None = None, ) -> Any: """Persist ``original`` and return a bounded projection honoring the delivery bound. @@ -528,30 +681,40 @@ def _spill_for_delivery_bound( non-exempted spill machinery (atomic_write, _artifact_path, _project_json_object) so the caller sees the same envelope shape, with ``reason="delivery_bound"`` so downstream formatters distinguish it. + + When ``pre_published_artifact_path`` and ``pre_published_sha256`` are + provided (recipe tool envelope path), the artifact is already on disk + from the tool layer's pre-return persistence and this function only + needs to build the projection. """ - if artifact_dir is None: - return bounded_response_budget_failure( - result, - cause="context_unavailable", - tool_name=tool_name, - max_bytes=config.response_max_bytes, - original_utf8_bytes=original_size, - ) - path = _artifact_path(artifact_dir, tool_name) - try: - atomic_write(path, original) - except OSError: - return bounded_response_budget_failure( - result, - cause="artifact_publication_failed", - tool_name=tool_name, - max_bytes=config.response_max_bytes, - original_utf8_bytes=original_size, - ) - published = str(path.resolve()) + if pre_published_artifact_path is not None and pre_published_sha256 is not None: + published = pre_published_artifact_path + content_sha256 = pre_published_sha256 + else: + if artifact_dir is None: + return bounded_response_budget_failure( + result, + cause="context_unavailable", + tool_name=tool_name, + max_bytes=config.response_max_bytes, + original_utf8_bytes=original_size, + ) + content_sha256 = hashlib.sha256(original.encode("utf-8")).hexdigest() + path = _artifact_path(artifact_dir, tool_name, content_sha256) + try: + atomic_write(path, original) + except OSError: + return bounded_response_budget_failure( + result, + cause="artifact_publication_failed", + tool_name=tool_name, + max_bytes=config.response_max_bytes, + original_utf8_bytes=original_size, + ) + published = str(path.resolve()) metadata: dict[str, Any] = { "artifact_path": published, - "sha256": hashlib.sha256(original.encode("utf-8")).hexdigest(), + "sha256": content_sha256, "original_utf8_bytes": original_size, "reason": "delivery_bound", } @@ -623,6 +786,136 @@ def _spill_for_delivery_bound( return {"success": False, "error": "response_budget_projection_invalid"} +_ENVELOPE_ADVISORY_KEYS: tuple[str, ...] = ( + "suggestions", + "errors", + "orchestration_rules", + "stop_step_semantics", + "ingredients_table", +) + + +def _envelope_fits(envelope: dict[str, Any], *, bound: int) -> bool: + return len(_canonical_json(envelope).encode("utf-8")) <= bound + + +def _degrade_envelope_advisory_fields(envelope: dict[str, Any], *, bound: int) -> bool: + """Shrink advisory fields in place until ``envelope`` fits ``bound``. + + ``step_flow_skeleton`` and ``step_index`` are the structural core of the + pull architecture — every post-prune step must stay locatable via + ``get_recipe_section`` — so they are never touched here. Degrades the + remaining (advisory, non-structural) preserved fields in priority order + using the same ``_project_value``/``_minimal_same_type`` primitives the + delivery-bound backstop uses, largest first. Returns whether the envelope + fits after degradation. + """ + for key in _ENVELOPE_ADVISORY_KEYS: + value = envelope.get(key) + if not value: + continue + limit = len(_canonical_json(value).encode("utf-8")) + while limit >= 16: + limit //= 2 + envelope[key], _, _ = _project_value(value, limit) + if _envelope_fits(envelope, bound=bound): + return True + envelope[key] = _minimal_same_type(value) + if _envelope_fits(envelope, bound=bound): + return True + return False + + +def build_recipe_envelope( + payload: dict[str, Any], + *, + artifact_path: str, + sha256: str, + bound: int, + success: bool, + kitchen: str, + version: str, + pull_tool: str = "get_recipe_section", + step_index: dict[str, str] | None = None, + step_flow_skeleton: list[dict[str, Any]] | None = None, +) -> dict[str, Any]: + """Build the compact recipe envelope returned by open_kitchen / load_recipe. + + The envelope is designed to fit the smallest registered backend delivery + bound by construction (verified by CI fitness test). It carries: + + - Routing metadata (success, kitchen, version) + - Orchestrator-required control-plane fields (ingredients_table, + orchestration_rules, stop_step_semantics, errors, suggestions) + - A step-flow skeleton with every post-prune step's routing edges + (on_success, on_failure, on_result, on_context_limit) — the recipe's + execution graph with no step body content + - A step_index mapping each step name to its pull-tool identifier + - artifact_path + sha256 for the pull tool to verify integrity + - The pull_tool name documenting the retrieval surface + + The full recipe content and diagram are NOT in the envelope — they are + retrieved on demand via ``get_recipe_section``. This is the structural + fix that makes the smallest backend delivery bound a non-issue for + recipe delivery. + + ``success``, ``kitchen``, ``version`` are caller-supplied because the + upstream payload differs: ``open_kitchen`` uses ``build_open_kitchen_recipe_payload`` + to inject these; ``load_recipe`` returns the raw serve_recipe() result + without them. + """ + envelope: dict[str, Any] = { + "success": success, + "kitchen": kitchen, + "version": version, + "ingredients_table": payload.get("ingredients_table"), + "orchestration_rules": payload.get("orchestration_rules"), + "stop_step_semantics": payload.get("stop_step_semantics"), + "errors": payload.get("errors", []), + "suggestions": payload.get("suggestions", []), + "step_flow_skeleton": step_flow_skeleton if step_flow_skeleton is not None else [], + "step_index": step_index if step_index is not None else {}, + "artifact_path": artifact_path, + "sha256": sha256, + "pull_tool": pull_tool, + } + + if not _envelope_fits(envelope, bound=bound) and not _degrade_envelope_advisory_fields( + envelope, bound=bound + ): + raise _ProjectionNonconvergentError( + f"envelope exceeds delivery bound: " + f"bytes={len(_canonical_json(envelope).encode('utf-8'))} bound={bound}. " + "Envelope construction must keep the step_flow_skeleton small enough " + "to fit by construction — extend the CI fitness guard, not this assert." + ) + return envelope + + +def resolve_effective_or_fallback_delivery_bound(tool_ctx: Any) -> int | None: + """Resolve the effective delivery token limit, falling back to the worst case. + + Shared by ``track_response_size`` and ``shape_execution_response`` so the + backend-capability lookup and worst-case fallback are implemented once. + Safe to call with ``tool_ctx=None`` — falls straight to the worst-case bound. + """ + backend = getattr(tool_ctx, "backend", None) + caps = getattr(backend, "capabilities", None) if backend is not None else None + effective_delivery_token_limit: int | None = None + if isinstance(caps, BackendCapabilities): + effective_delivery_token_limit = resolve_effective_delivery_bound(caps) + if effective_delivery_token_limit is None or effective_delivery_token_limit <= 0: + fallback_limit = resolve_worst_case_delivery_bound() + if fallback_limit > 0: + logger.warning( + "Delivery-bound enforcement using worst-case default " + "(%d tokens): backend capabilities unavailable", + fallback_limit, + ) + effective_delivery_token_limit = fallback_limit + return effective_delivery_token_limit + + def enforce_response_budget( result: Any, *, @@ -656,6 +949,8 @@ def enforce_response_budget( ) original_bytes = original.encode("utf-8") original_size = len(original_bytes) + if effective_delivery_token_limit == 0: + effective_delivery_token_limit = resolve_worst_case_delivery_bound() or None over_delivery_bound = ( effective_delivery_token_limit is not None and effective_delivery_token_limit > 0 @@ -674,6 +969,46 @@ def enforce_response_budget( max_bytes=config.response_max_bytes, original_utf8_bytes=original_size, ) + # Exempted tools (open_kitchen / load_recipe) reach this branch with an + # envelope that may already carry a tool-layer-published artifact_path + # + sha256 (persist_recipe_artifact / build_and_record_recipe_envelope). + # Reuse that pre-published pair when present — persisting again here + # would write a second, orphaned artifact over the envelope bytes with + # a different sha256. Otherwise persist the payload itself here so + # every exempted response is recoverable from disk, skipping + # gracefully when no artifact directory is available (e.g. a mock + # ToolContext in tests with no real temp_dir). + pre_published_path: str | None = None + pre_published_sha: str | None = None + newly_persisted = False + try: + parsed_for_artifact = json.loads(original) + except (TypeError, ValueError, RecursionError): + parsed_for_artifact = None + if not isinstance(parsed_for_artifact, dict): + parsed_for_artifact = None + if ( + parsed_for_artifact is not None + and isinstance(parsed_for_artifact.get("artifact_path"), str) + and parsed_for_artifact["artifact_path"] + and isinstance(parsed_for_artifact.get("sha256"), str) + and parsed_for_artifact["sha256"] + ): + pre_published_path = parsed_for_artifact["artifact_path"] + pre_published_sha = parsed_for_artifact["sha256"] + elif artifact_dir is not None: + content_sha256 = hashlib.sha256(original_bytes).hexdigest() + artifact_path_obj = _artifact_path(artifact_dir, tool_name, content_sha256) + try: + artifact_path_obj.parent.mkdir(parents=True, exist_ok=True) + atomic_write(artifact_path_obj, original) + except OSError: + logger.warning("recipe_backstop_artifact_write_failed", exc_info=True) + else: + pre_published_path = str(artifact_path_obj.resolve()) + pre_published_sha = content_sha256 + newly_persisted = True + if over_delivery_bound: assert effective_delivery_token_limit is not None # narrowed by over_delivery_bound return _spill_for_delivery_bound( @@ -684,6 +1019,8 @@ def enforce_response_budget( original=original, original_size=original_size, effective_delivery_token_limit=effective_delivery_token_limit, + pre_published_artifact_path=pre_published_path, + pre_published_sha256=pre_published_sha, ) _emit_response_budget_event( "response_budget_exemption", @@ -694,6 +1031,12 @@ def enforce_response_budget( max_chars=exemption.max_chars, max_utf8_bytes=exemption.max_utf8_bytes, ) + if newly_persisted and parsed_for_artifact is not None: + parsed_for_artifact.setdefault("artifact_path", pre_published_path) + parsed_for_artifact.setdefault("sha256", pre_published_sha) + if isinstance(result, str): + return json.dumps(parsed_for_artifact) + return parsed_for_artifact return result if ( not force_spill @@ -710,7 +1053,8 @@ def enforce_response_budget( original_utf8_bytes=original_size, ) - path = _artifact_path(artifact_dir, tool_name) + content_sha256 = hashlib.sha256(original_bytes).hexdigest() + path = _artifact_path(artifact_dir, tool_name, content_sha256) try: atomic_write(path, original) except OSError: @@ -723,9 +1067,9 @@ def enforce_response_budget( ) published = str(path.resolve()) - metadata = { + metadata: dict[str, Any] = { "artifact_path": published, - "sha256": hashlib.sha256(original_bytes).hexdigest(), + "sha256": content_sha256, "original_utf8_bytes": len(original_bytes), } @@ -847,7 +1191,9 @@ def shape_json_response( "RESPONSE_SPILL_SCHEMA_DIGEST", "RESPONSE_SPILL_SCHEMA_VERSION", "bounded_response_budget_failure", + "build_recipe_envelope", "emit_response_budget_failure", "enforce_response_budget", + "extract_step_routing", "shape_json_response", ] diff --git a/src/autoskillit/server/tools/_execution_helpers.py b/src/autoskillit/server/tools/_execution_helpers.py index 405587b17..86c3a1bf3 100644 --- a/src/autoskillit/server/tools/_execution_helpers.py +++ b/src/autoskillit/server/tools/_execution_helpers.py @@ -14,18 +14,19 @@ from autoskillit.core import ( RUN_PYTHON_SENTINEL_KEYS, - BackendCapabilities, CapturedStream, SpillSpec, SubprocessResult, get_logger, - resolve_effective_delivery_bound, resolve_temp_dir, spill_output, ) from autoskillit.execution import CaptureReadError, summarize_capture from autoskillit.server._misc import _hook_config_overlay_path -from autoskillit.server._response_budget import shape_json_response +from autoskillit.server._response_budget import ( + resolve_effective_or_fallback_delivery_bound, + shape_json_response, +) logger = get_logger(__name__) @@ -203,12 +204,7 @@ def shape_execution_response( if work_dir and Path(work_dir).is_absolute() else tool_ctx.temp_dir / tool_name ) - effective_delivery_token_limit: int | None = None - backend = getattr(tool_ctx, "backend", None) - caps = getattr(backend, "capabilities", None) if backend is not None else None - - if isinstance(caps, BackendCapabilities): - effective_delivery_token_limit = resolve_effective_delivery_bound(caps) + effective_delivery_token_limit = resolve_effective_or_fallback_delivery_bound(tool_ctx) return shape_json_response( payload, tool_name=tool_name, diff --git a/src/autoskillit/server/tools/_serve_helpers.py b/src/autoskillit/server/tools/_serve_helpers.py index c77fbcf84..ca80d05c1 100644 --- a/src/autoskillit/server/tools/_serve_helpers.py +++ b/src/autoskillit/server/tools/_serve_helpers.py @@ -1,14 +1,15 @@ """Unified serve-pipeline helpers. The only legal call site for load_and_validate in server/tools/. -All four serve surfaces (open_kitchen normal, open_kitchen deferred-recall, -load_recipe, get_recipe) must call serve_recipe() instead of calling -ctx.recipes.load_and_validate() directly. This structural invariant is -enforced by tests/arch/test_serve_surface_registry.py. +All five serve surfaces (open_kitchen normal, open_kitchen deferred-recall, +load_recipe, get_recipe, get_recipe_section) must call serve_recipe() instead +of calling ctx.recipes.load_and_validate() directly. This structural invariant +is enforced by tests/arch/test_serve_surface_registry.py. """ from __future__ import annotations +import hashlib import json from pathlib import Path from typing import TYPE_CHECKING, Any @@ -16,12 +17,23 @@ from autoskillit.core import ( RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, RESPONSE_BACKSTOP_EXEMPTION_REGISTRY_DIGEST, + atomic_write, + get_logger, + resolve_effective_delivery_bound, +) +from autoskillit.execution import resolve_worst_case_delivery_bound +from autoskillit.server._response_budget import ( + _artifact_path, + build_recipe_envelope, + extract_step_routing, ) if TYPE_CHECKING: from autoskillit.core import BackendCapabilities, CodingAgentBackend from autoskillit.pipeline.context import ToolContext +logger = get_logger(__name__) + def build_backend_capabilities_map( effective_backend_map: dict[str, str] | None, @@ -83,6 +95,140 @@ def build_open_kitchen_recipe_payload(result: dict[str, Any], *, version: str) - return payload +def _safe_backend_name(tool_ctx: Any) -> str | None: + """Return ``tool_ctx.backend.name`` if a backend is set, else ``None``. + + Helper factored out of the recipe_artifact_state builder so pyright can + resolve ``backend.name`` against the typed ``CodingAgentBackend`` rather + than the Optional ``Any`` returned by ``getattr(..., None)``. + """ + backend = getattr(tool_ctx, "backend", None) + if backend is None: + return None + name_attr = getattr(backend, "name", None) + return name_attr if isinstance(name_attr, str) else None + + +def resolve_envelope_delivery_bound(tool_ctx: Any) -> int: + """Resolve the envelope construction-time bound in bytes. + + Mirrors ``track_response_size.wrapper``'s resolution so the envelope is + constructed against the same gate that enforcement applies. Backend + capabilities are preferred; falls back to the smallest registered backend + bound (worst case) when capabilities are unavailable or non-positive. + """ + backend = getattr(tool_ctx, "backend", None) + caps = getattr(backend, "capabilities", None) if backend is not None else None + token_limit: int | None = None + if caps is not None: + try: + token_limit = resolve_effective_delivery_bound(caps) + except Exception: # noqa: BLE001 + logger.warning("resolve_effective_delivery_bound_failed", exc_info=True) + token_limit = None + # Coerce to int; MagicMock or non-numeric values fall through to the + # conservative default so envelope construction never crashes on a + # misconfigured backend (e.g., a test mock with a MagicMock capabilities). + if not isinstance(token_limit, int) or token_limit <= 0: + try: + fallback = resolve_worst_case_delivery_bound() + except Exception: # noqa: BLE001 + logger.warning("resolve_worst_case_delivery_bound_failed", exc_info=True) + fallback = 0 + if isinstance(fallback, int) and fallback > 0: + token_limit = fallback + else: + token_limit = 10_000 + return token_limit * 4 + + +def persist_recipe_artifact( + *, + tool_ctx: Any, + tool_name: str, + payload: dict[str, Any], +) -> tuple[str, str]: + """Persist the full recipe payload and return (artifact_path, sha256). + + Uses the deterministic path scheme from ``_response_budget._artifact_path`` + so the persisted file is stable across calls with the same content. + """ + artifact_dir = tool_ctx.temp_dir / "responses" / tool_name + artifact_dir.mkdir(parents=True, exist_ok=True) + serialized = json.dumps(payload, ensure_ascii=False, sort_keys=False) + payload_bytes = serialized.encode("utf-8") + content_sha256 = hashlib.sha256(payload_bytes).hexdigest() + path = _artifact_path(artifact_dir, tool_name, content_sha256) + atomic_write(path, serialized) + return str(path.resolve()), content_sha256 + + +def build_and_record_recipe_envelope( + *, + tool_ctx: Any, + tool_name: str, + payload: dict[str, Any], + result: dict[str, Any], + kitchen_label: str, + version: str, + overrides: dict[str, str] | None, + recipe_name: str, + ingredients_only: bool, +) -> dict[str, Any]: + """Persist the full artifact and build the compact envelope in one call. + + Populates ``ctx.recipe_artifact_state`` with the artifact_path, sha256, + and the recipe-load parameters ``get_recipe_section`` needs to recreate + the artifact if it is later missing from disk. Returns the envelope dict + so the caller can serialize via ``render_served_response``. + """ + artifact_path, content_sha256 = persist_recipe_artifact( + tool_ctx=tool_ctx, + tool_name=tool_name, + payload=payload, + ) + post_prune_step_names_raw = result.get("post_prune_step_names", []) + post_prune_step_names: list[str] = ( + list(post_prune_step_names_raw) if isinstance(post_prune_step_names_raw, list) else [] + ) + content_text = result.get("content", "") + step_flow_skeleton = ( + extract_step_routing(content_text or "", post_prune_step_names) + if not ingredients_only and isinstance(content_text, str) + else [] + ) + # ingredients_only strips "content" from the persisted payload, so no + # step is actually pullable via get_recipe_section(section="step", ...) + # in that mode — advertising them in step_index would be a broken promise. + step_index = ( + {} + if ingredients_only + else {step_name: f"step:{step_name}" for step_name in post_prune_step_names} + ) + tool_ctx.recipe_artifact_state = { + "artifact_path": artifact_path, + "sha256": content_sha256, + "tool_name": tool_name, + "recipe_name": recipe_name, + "ingredient_overrides": dict(overrides) if overrides else {}, + "backend_name": _safe_backend_name(tool_ctx), + "kitchen_id": getattr(tool_ctx, "kitchen_id", ""), + "ingredients_only": ingredients_only, + } + envelope_bound = resolve_envelope_delivery_bound(tool_ctx) + return build_recipe_envelope( + result, + artifact_path=artifact_path, + sha256=content_sha256, + bound=envelope_bound, + success=True, + kitchen=kitchen_label, + version=version, + step_flow_skeleton=step_flow_skeleton, + step_index=step_index, + ) + + def _build_serve_override_stack( ctx: ToolContext, caller_overrides: dict[str, str] | None, diff --git a/src/autoskillit/server/tools/tools_kitchen.py b/src/autoskillit/server/tools/tools_kitchen.py index c572b2180..778816873 100644 --- a/src/autoskillit/server/tools/tools_kitchen.py +++ b/src/autoskillit/server/tools/tools_kitchen.py @@ -83,6 +83,7 @@ filter_steps_by_post_prune, ) from autoskillit.server.tools._serve_helpers import ( + build_and_record_recipe_envelope, build_backend_capabilities_map, build_open_kitchen_recipe_payload, render_served_response, @@ -126,6 +127,33 @@ def _kitchen_failure_envelope( ) +def _render_open_kitchen_envelope( + tool_ctx: ToolContext, + result: dict[str, Any], + *, + overrides: dict[str, str] | None, + recipe_name: str, + ingredients_only: bool, +) -> str: + """Persist the full recipe payload and render the compact open_kitchen envelope. + + Single logic site for both open_kitchen recipe-bearing return paths + (deferred-recall and normal), mirroring load_recipe's envelope call shape. + """ + envelope = build_and_record_recipe_envelope( + tool_ctx=tool_ctx, + tool_name="open_kitchen", + payload=result, + result=result, + kitchen_label="open", + version=__version__, + overrides=overrides, + recipe_name=recipe_name, + ingredients_only=ingredients_only, + ) + return render_served_response(envelope) + + def _recipe_validation_error_response(name: str, result: dict[str, Any]) -> str: _structural_errs: list[str] = result.get("errors", []) if _structural_errs: @@ -1059,7 +1087,13 @@ 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) - return render_served_response(result) + return _render_open_kitchen_envelope( + tool_ctx, + result, + overrides=overrides, + recipe_name=name, + ingredients_only=ingredients_only, + ) try: result = serve_recipe( tool_ctx, @@ -1211,7 +1245,13 @@ async def open_kitchen( ) return _validation_err - return render_served_response(result) + return _render_open_kitchen_envelope( + tool_ctx, + result, + overrides=overrides, + recipe_name=name, + ingredients_only=ingredients_only, + ) text = ( f"Kitchen is open. AutoSkillit {__version__}. Tools are ready for service.\n\n" diff --git a/src/autoskillit/server/tools/tools_recipe.py b/src/autoskillit/server/tools/tools_recipe.py index 82cb6d777..080160768 100644 --- a/src/autoskillit/server/tools/tools_recipe.py +++ b/src/autoskillit/server/tools/tools_recipe.py @@ -1,7 +1,8 @@ -"""MCP tool handlers: load_recipe, list_recipes, validate_recipe, migrate_recipe.""" +"""MCP recipe tool handlers, including compact artifact retrieval.""" from __future__ import annotations +import hashlib import json from pathlib import Path from typing import Any @@ -10,12 +11,21 @@ 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 ( + FLEET_DISPATCH_TOOLS, # noqa: F401 — re-exported for downstream visibility + ProcessStaleError, + atomic_write, + dump_yaml_str, + get_logger, + load_yaml, + temp_dir_display_str, +) from autoskillit.pipeline import GATED_TOOLS, UNGATED_TOOLS # noqa: F401 from autoskillit.server import mcp from autoskillit.server._guards import _require_enabled @@ -25,6 +35,7 @@ strip_ingredients_only_keys, ) from autoskillit.server._notify import _notify, track_response_size +from autoskillit.server._response_budget import _artifact_path from autoskillit.server._state import _get_ctx_or_none from autoskillit.server.tools._authority_feedback import build_authority_clobber_warnings from autoskillit.server.tools._auto_overrides import ( @@ -34,8 +45,11 @@ ) from autoskillit.server.tools._cancellation_shield import _cancellation_shield from autoskillit.server.tools._serve_helpers import ( + build_and_record_recipe_envelope, build_backend_capabilities_map, + build_open_kitchen_recipe_payload, render_served_response, + resolve_envelope_delivery_bound, response_backstop_tool_meta, serve_recipe, ) @@ -324,7 +338,23 @@ async def load_recipe( stage="validate_result", ) return _validation_err - return render_served_response(result) + # Part B Step 2.4: persist the full recipe artifact and return a + # compact envelope. The artifact is the authoritative source of + # full recipe content; the envelope carries routing metadata, + # the step-flow skeleton, and pull instructions. ``get_recipe_section`` + # reads ``ctx.recipe_artifact_state`` to retrieve any section on demand. + envelope = build_and_record_recipe_envelope( + tool_ctx=tool_ctx, + tool_name="load_recipe", + payload=result, + result=result, + kitchen_label="loaded", + version=__version__, + overrides=overrides, + recipe_name=name, + ingredients_only=ingredients_only, + ) + return render_served_response(envelope) except Exception as exc: logger.error("load_recipe unhandled exception", exc_info=True) return json.dumps({"success": False, "error": f"{type(exc).__name__}: {exc}"}) @@ -472,3 +502,430 @@ async def migrate_recipe(name: str, ctx: Context = CurrentContext()) -> str: except Exception as exc: logger.error("migrate_recipe unhandled exception", exc_info=True) return json.dumps({"error": f"{type(exc).__name__}: {exc}", "name": name}) + + +# Part B Step 2.3: bounded pull tool for step / section content. Reads from +# the always-persisted artifact written by open_kitchen / load_recipe and +# verifies sha256 before serving. Falls back to load_and_validate recreation +# if the artifact file is missing from disk (e.g. after a session restart). +# +# Decorated WITHOUT ``meta=response_backstop_tool_meta(...)``: this tool is +# not in ``RESPONSE_BACKSTOP_EXEMPTION_REGISTRY``, so the universal response +# backstop applies directly. Per ``server/AGENTS.md`` all MCP tools MUST have +# ``readOnlyHint: True``. +@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, + step_name: str | None = None, + part: int = 0, +) -> str: + """Pull a bounded slice of recipe content from the persisted artifact. + + The companion tool to ``open_kitchen`` and ``load_recipe``: those tools + return a compact step-skeleton envelope; this tool returns the full + content for a single step (or other section) on demand. The orchestrator + calls ``get_recipe_section(section="step", step_name=)`` just + before executing each step to keep context lean. + + Sections: + - "step": full YAML for one step (requires ``step_name``) + - "content": the full recipe content string (chunked if oversized) + - "diagram": the pre-rendered diagram string + - "suggestions": the full suggestions list + + Args: + section: Which slice to retrieve — "step", "content", "diagram", or + "suggestions". + step_name: Required when ``section == "step"``; must be a valid + post-prune step name from the most recent envelope's + ``step_index``. + part: 0-indexed chunk number for sections that exceed the delivery + bound. Default 0 (first/only chunk). When the response is + chunked, ``has_more`` and ``next_part`` fields describe how to + retrieve the remainder. + + Never raises. + """ + if (gate := _require_enabled()) is not None: + return gate + try: + with structlog.contextvars.bound_contextvars(tool="get_recipe_section", section=section): + tool_ctx = _get_ctx_or_none() + if tool_ctx is None: + return json.dumps({"success": False, "error": "Server not initialized"}) + + artifact_state = getattr(tool_ctx, "recipe_artifact_state", None) + if not artifact_state: + return json.dumps( + { + "success": False, + "error": "no_recipe_loaded", + "detail": ( + "Call open_kitchen or load_recipe first so a " + "recipe artifact is persisted on this session." + ), + } + ) + + artifact_path = artifact_state.get("artifact_path") + expected_sha256 = artifact_state.get("sha256") + if not artifact_path or not expected_sha256: + return json.dumps( + { + "success": False, + "error": "artifact_state_invalid", + "detail": "recipe_artifact_state is missing artifact_path or sha256", + } + ) + + payload = _read_or_recreate_artifact(tool_ctx, artifact_state) + if isinstance(payload, dict) and payload.get("error"): + return json.dumps(payload) + + assert isinstance(payload, dict) # narrowed by _read_or_recreate_artifact + return _build_section_response( + payload=payload, + section=section, + step_name=step_name, + part=part, + artifact_path=artifact_path, + expected_sha256=expected_sha256, + tool_ctx=tool_ctx, + ) + 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}", "section": section} + ) + + +def _read_or_recreate_artifact( + tool_ctx: Any, + artifact_state: dict[str, Any], +) -> dict[str, Any]: + """Read the persisted recipe artifact; recreate via load_and_validate if missing. + + Returns a dict payload on success (the recipe serve result) or an error + dict (with an ``error`` key) on integrity / recreation failure. Never + raises — exceptions inside ``load_and_validate`` are caught and reported + as structured errors so the pull tool never silently returns empty. + """ + artifact_path = artifact_state.get("artifact_path", "") + expected_sha256 = artifact_state.get("sha256", "") + try: + path_obj = Path(artifact_path) + serialized = path_obj.read_text(encoding="utf-8") + actual_sha256 = hashlib.sha256(serialized.encode("utf-8")).hexdigest() + if actual_sha256 != expected_sha256: + return { + "success": False, + "error": "artifact_integrity_failed", + "detail": ( + f"sha256 mismatch: recorded={expected_sha256[:16]}... " + f"on_disk={actual_sha256[:16]}..." + ), + } + try: + return json.loads(serialized) + except (TypeError, ValueError): + return { + "success": False, + "error": "artifact_corrupt", + "detail": "Persisted artifact is not valid JSON.", + } + except FileNotFoundError: + return _recreate_artifact(tool_ctx, artifact_state) + except OSError as exc: + return { + "success": False, + "error": "artifact_unavailable", + "detail": f"Could not read artifact: {type(exc).__name__}: {exc}", + } + + +def _recreate_artifact( + tool_ctx: Any, + artifact_state: dict[str, Any], +) -> dict[str, Any]: + """Recreate a missing artifact by re-serving the recipe via serve_recipe(). + + Routes through ``serve_recipe()`` (in ``_serve_helpers.py``) — the single + legal call site for ``load_and_validate`` in ``server/tools/``. This + preserves the SERVE_SURFACES contract enforced by + ``tests/arch/test_serve_surface_registry.py``. + + Returns the recreated payload (on success) or an error dict (on failure). + The recreated sha256 must match the recorded sha256 — divergent content + is reported as an error rather than served. + """ + + recipe_name = artifact_state.get("recipe_name") + if not recipe_name: + return { + "success": False, + "error": "artifact_unavailable", + "detail": "Cannot recreate artifact: recipe_name missing from artifact state.", + } + if tool_ctx.recipes is None: + return { + "success": False, + "error": "artifact_unavailable", + "detail": "Cannot recreate artifact: recipe repository not configured.", + } + + ingredient_overrides = dict(artifact_state.get("ingredient_overrides") or {}) + backend_name = artifact_state.get("backend_name") + try: + recreated = serve_recipe( + tool_ctx, + recipe_name, + caller_overrides=ingredient_overrides or None, + config_default={}, + session_overrides={}, + config_layer={}, + backend_name=backend_name, + ) + except ProcessStaleError as exc: + return { + "success": False, + "error": "artifact_unavailable", + "detail": f"Server package state is stale; restart required. ({exc})", + } + except Exception as exc: # noqa: BLE001 + logger.warning("artifact_recreation_failed", exc_info=True) + return { + "success": False, + "error": "artifact_unavailable", + "detail": f"Recreation failed: {type(exc).__name__}: {exc}", + } + + if artifact_state.get("tool_name") == "open_kitchen": + recreated = build_open_kitchen_recipe_payload(recreated, version=__version__) + if artifact_state.get("ingredients_only"): + recreated = strip_ingredients_only_keys(recreated) + + serialized = json.dumps(recreated, ensure_ascii=False, sort_keys=False) + recreated_sha256 = hashlib.sha256(serialized.encode("utf-8")).hexdigest() + expected_sha256 = artifact_state.get("sha256", "") + if recreated_sha256 != expected_sha256: + return { + "success": False, + "error": "artifact_recreation_mismatch", + "detail": ( + f"Recreated content diverges from original artifact " + f"(sha256={recreated_sha256[:16]}... vs recorded={expected_sha256[:16]}...)" + ), + } + + artifact_dir = tool_ctx.temp_dir / "responses" / artifact_state.get("tool_name", "load_recipe") + try: + artifact_dir.mkdir(parents=True, exist_ok=True) + path = _artifact_path( + artifact_dir, artifact_state.get("tool_name", "load_recipe"), recreated_sha256 + ) + atomic_write(path, serialized) + except OSError as exc: + return { + "success": False, + "error": "artifact_unavailable", + "detail": f"Recreated artifact could not be written: {type(exc).__name__}: {exc}", + } + return recreated + + +def _build_section_response( + *, + payload: dict[str, Any], + section: str, + step_name: str | None, + part: int, + artifact_path: str, + expected_sha256: str, + tool_ctx: Any, +) -> str: + """Build the JSON response for a section pull, with optional chunking.""" + if section == "step": + if not step_name: + return json.dumps( + { + "success": False, + "error": "step_name_required", + "detail": "section='step' requires step_name=", + } + ) + step_yaml = _extract_step_yaml(payload, step_name) + if step_yaml is None: + return json.dumps( + { + "success": False, + "error": "step_not_found", + "detail": ( + f"Step '{step_name}' is not in the loaded recipe's post-prune step set." + ), + "step_index": payload.get("post_prune_step_names", []), + } + ) + body: dict[str, Any] = { + "success": True, + "section": "step", + "step_name": step_name, + "content": step_yaml, + "artifact_path": artifact_path, + "sha256": expected_sha256, + } + return _chunk_response_if_oversized(body, tool_ctx, part=part) + + if section == "content": + body = { + "success": True, + "section": "content", + "content": payload.get("content", ""), + "artifact_path": artifact_path, + "sha256": expected_sha256, + } + return _chunk_response_if_oversized(body, tool_ctx, part=part) + + if section == "diagram": + body = { + "success": True, + "section": "diagram", + "content": payload.get("diagram"), + "artifact_path": artifact_path, + "sha256": expected_sha256, + } + return _chunk_response_if_oversized(body, tool_ctx, part=part) + + if section == "suggestions": + body = { + "success": True, + "section": "suggestions", + "content": payload.get("suggestions", []), + "artifact_path": artifact_path, + "sha256": expected_sha256, + } + return _chunk_response_if_oversized(body, tool_ctx, part=part) + + return json.dumps( + { + "success": False, + "error": "unknown_section", + "detail": ( + f"Unknown section {section!r}. Valid sections: 'step', 'content', " + "'diagram', 'suggestions'." + ), + } + ) + + +def _extract_step_yaml(payload: dict[str, Any], step_name: str) -> str | None: + """Extract the YAML block for a single step from the recipe content. + + Returns the full ``name: \n ...`` block (with ``steps:`` wrapper + so downstream ``compact_recipe_display`` style transforms still apply), + or ``None`` if the step is not in the post-prune step set. + """ + content = payload.get("content") + if not isinstance(content, str) or not content: + return None + try: + parsed = load_yaml(content) + except Exception: + logger.warning("step_yaml_load_failed", step_name=step_name, exc_info=True) + return None + if not isinstance(parsed, dict): + return None + steps_obj = parsed.get("steps") + if not isinstance(steps_obj, dict) or step_name not in steps_obj: + return None + step_value = steps_obj[step_name] + try: + return dump_yaml_str({"steps": {step_name: step_value}}, default_flow_style=False) + except Exception: + logger.warning("step_yaml_dump_failed", step_name=step_name, exc_info=True) + return None + + +def _paginate_list_by_bytes(items: list[Any], chunk_size: int) -> list[list[Any]]: + """Group list elements into byte-bounded pages for chunked delivery. + + Packs elements greedily so each page's JSON-serialized size stays within + ``chunk_size``; a single oversized element still gets its own page rather + than being dropped, so every page round-trips through ``json.loads``. + """ + pages: list[list[Any]] = [] + current: list[Any] = [] + current_bytes = 2 # "[]" + for item in items: + item_bytes = len(json.dumps(item, ensure_ascii=False).encode("utf-8")) + separator_bytes = 1 if current else 0 + if current and current_bytes + separator_bytes + item_bytes > chunk_size: + pages.append(current) + current = [] + current_bytes = 2 + separator_bytes = 0 + current.append(item) + current_bytes += separator_bytes + item_bytes + if current or not pages: + pages.append(current) + return pages + + +def _part_out_of_range_response(part: int, chunks: int) -> str: + return json.dumps( + { + "success": False, + "error": "part_out_of_range", + "detail": f"part={part} is out of range [0, {chunks}).", + "total_parts": chunks, + } + ) + + +def _chunk_response_if_oversized(body: dict[str, Any], tool_ctx: Any, *, part: int = 0) -> str: + """Split response by ``part`` if it exceeds the envelope delivery bound. + + The bound is the same one ``build_recipe_envelope`` uses (smallest + registered backend delivery bound). Sections that fit return directly; + oversized sections are chunked and carry ``has_more`` + ``next_part``. + List-typed sections (e.g. ``suggestions``) are paginated at the element + level so each chunk is valid JSON on its own — character-slicing the + serialized list text would produce an invalid JSON substring on almost + every boundary. + """ + bound = resolve_envelope_delivery_bound(tool_ctx) + serialized = json.dumps(body, ensure_ascii=False) + body_bytes = len(serialized.encode("utf-8")) + if body_bytes <= bound: + return serialized + chunk_size = max(1024, bound - 512) + content_text = body.get("content") + if not isinstance(content_text, (str, list)): + return serialized + sliced: Any + if isinstance(content_text, str): + total = len(content_text) + chunks = max(1, (total + chunk_size - 1) // chunk_size) + if part < 0 or part >= chunks: + return _part_out_of_range_response(part, chunks) + sliced = content_text[part * chunk_size : (part + 1) * chunk_size] + else: + pages = _paginate_list_by_bytes(content_text, chunk_size) + chunks = max(1, len(pages)) + if part < 0 or part >= chunks: + return _part_out_of_range_response(part, chunks) + sliced = pages[part] if pages else [] + chunked = { + "success": True, + "section": body.get("section"), + "step_name": body.get("step_name"), + "content": sliced, + "artifact_path": body.get("artifact_path"), + "sha256": body.get("sha256"), + "part": part, + "total_parts": chunks, + "has_more": part < chunks - 1, + "next_part": part + 1 if part < chunks - 1 else None, + } + return json.dumps(chunked, ensure_ascii=False) diff --git a/tests/arch/test_feature_registry.py b/tests/arch/test_feature_registry.py index 81b982624..802a09278 100644 --- a/tests/arch/test_feature_registry.py +++ b/tests/arch/test_feature_registry.py @@ -293,7 +293,7 @@ def test_no_unregistered_feature_tag_on_tools(): ) # Known structural (non-feature, non-pack) tags that are always valid - STRUCTURAL_TAGS: frozenset[str] = frozenset({"kitchen-core", "fleet-dispatch"}) + STRUCTURAL_TAGS: frozenset[str] = frozenset({"kitchen", "kitchen-core", "fleet-dispatch"}) known = frozenset(FEATURE_REGISTRY.keys()) | frozenset(PACK_REGISTRY.keys()) | STRUCTURAL_TAGS violations = [ diff --git a/tests/arch/test_pyright_suppression_allowlist.py b/tests/arch/test_pyright_suppression_allowlist.py index 2f32e59c6..a5f3be96c 100644 --- a/tests/arch/test_pyright_suppression_allowlist.py +++ b/tests/arch/test_pyright_suppression_allowlist.py @@ -22,7 +22,7 @@ "recipe/__init__.py", 278, ): "lazy-registry: method added by _register_rule_module() side effects", - ("recipe/_api.py", 286): "lazy-registry: RULE_REGISTRY_HASH set by _finalize_registry()", + ("recipe/_api.py", 336): "lazy-registry: RULE_REGISTRY_HASH set by _finalize_registry()", } TEST_ALLOWLIST: dict[tuple[str, int], str] = { diff --git a/tests/arch/test_serve_surface_registry.py b/tests/arch/test_serve_surface_registry.py index a1da39f86..db2b92919 100644 --- a/tests/arch/test_serve_surface_registry.py +++ b/tests/arch/test_serve_surface_registry.py @@ -72,6 +72,7 @@ def test_serve_surfaces_contains_expected_members() -> None: "open_kitchen_deferred_recall", "load_recipe", "get_recipe", + "get_recipe_section", } assert expected == SERVE_SURFACES, ( f"SERVE_SURFACES mismatch. " diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 11852b50b..2ce1e72db 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -976,6 +976,15 @@ def test_data_directories_are_not_python_packages() -> None: "_compute_retry, and ClaudeSessionResult.normalize_subtype; " "lifespan_started heuristic added", ), + "server/_response_budget.py": ( + 1200, + "REQ-CNST-010-E12: canonical response-budget machinery (lossless spill, " + "exact canonical projection finalization, measured exemptions, and Part B " + "envelope/skeleton extraction for recipe delivery) — wide surface required " + "to keep the spill + projection + envelope math co-located for invariant " + "correctness. Part B added ~150 lines for build_recipe_envelope, " + "extract_step_routing, and deterministic artifact pathing.", + ), "_doctor.py": ( 1300, "REQ-CNST-010-E4: doctor check registry — 28 sequential checks require inline logic; " @@ -991,7 +1000,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 +1039,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)" + "; _render_open_kitchen_envelope single-logic-site helper wiring both recipe-bearing " + "return paths to build_and_record_recipe_envelope, bounded envelope + pull " + "architecture remediation (+32 net lines)", ), "tools_execution.py": ( 1650, @@ -1310,6 +1322,12 @@ def test_tool_context_service_fields_use_protocol_types() -> None: "temp_dir", "project_dir", "ephemeral_root", + # Part B Step 2.4: recipe_artifact_state is a server-side cache of + # the most recently loaded recipe's artifact metadata (path, sha256, + # ingredient_overrides). It is a value-type state field, not a service + # interface — get_recipe_section reads it to recreate the artifact + # on demand. Same exemption rationale as active_recipe_steps. + "recipe_artifact_state", } violations: list[str] = [] diff --git a/tests/contracts/test_delivery_bound_fitness.py b/tests/contracts/test_delivery_bound_fitness.py index 010681005..9f9b87db2 100644 --- a/tests/contracts/test_delivery_bound_fitness.py +++ b/tests/contracts/test_delivery_bound_fitness.py @@ -18,13 +18,26 @@ import pytest +from autoskillit import __version__ from autoskillit.config import OutputBudgetConfig -from autoskillit.core import RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, resolve_effective_delivery_bound +from autoskillit.core import ( + RESPONSE_BACKSTOP_EXEMPTION_REGISTRY, + dump_yaml_str, + load_yaml, + resolve_effective_delivery_bound, +) +from autoskillit.execution import ( + CODEX_TOOL_OUTPUT_TOKEN_LIMIT, + resolve_worst_case_delivery_bound, +) from autoskillit.execution.backends import BACKEND_REGISTRY from autoskillit.recipe import all_validated_recipe_names, load_and_validate from autoskillit.server._response_budget import ( RESPONSE_SPILL_METADATA_KEY, + _canonical_json, + build_recipe_envelope, enforce_response_budget, + extract_step_routing, ) from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload @@ -33,6 +46,11 @@ _PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +# 64-char realistic placeholder — matches the length of a real deterministic +# artifact path (tool_name + sha256[:16] + .log under a temp_dir) so envelope +# byte-budget assertions reflect production-shaped sizes. +_PLACEHOLDER_ARTIFACT_PATH = "/" + "a" * 59 + ".log" + def _recipe_names() -> list[str]: return sorted(all_validated_recipe_names(_PROJECT_ROOT)) @@ -47,16 +65,8 @@ def _effective_bound_bytes(bound_tokens: int) -> int: return bound_tokens * 4 -def _full_open_kitchen_payload(recipe_name: str) -> dict[str, object]: - """Build the production-shape ``open_kitchen`` payload for ``recipe_name``. - - Uses ``load_and_validate`` + ``build_open_kitchen_recipe_payload`` so the - payload mirrors what ``open_kitchen`` returns at runtime: the routing - envelope injected by the production helper plus the full recipe body from - the bundled YAML. The fit-or-spill assertion below exercises the spill - path for any bundled recipe whose serialized payload outgrows a backend's - bound. - """ +def _full_recipe_payload(recipe_name: str, tool_name: str) -> dict[str, object]: + """Build the production payload for an exempted recipe-serving tool.""" result = load_and_validate( recipe_name, project_dir=_PROJECT_ROOT, @@ -66,42 +76,171 @@ def _full_open_kitchen_payload(recipe_name: str) -> dict[str, object]: "source_dir": str(_PROJECT_ROOT), }, ) - return build_open_kitchen_recipe_payload(dict(result), version="0.0.0") + payload = dict(result) + if tool_name == "open_kitchen": + return build_open_kitchen_recipe_payload(payload, version="0.0.0") + assert tool_name == "load_recipe" + return payload + + +def _envelope_for_recipe( + recipe_name: str, tool_name: str, *, bound: int +) -> tuple[dict[str, object], dict[str, object]]: + """Build the real payload and its envelope for a bundled recipe at a given bound. + + Returns ``(payload, envelope)``. + """ + payload = _full_recipe_payload(recipe_name, tool_name) + step_names = payload.get("post_prune_step_names", []) + assert isinstance(step_names, list) + skeleton = extract_step_routing(payload["content"], step_names) + step_index = {name: f"step:{name}" for name in step_names} + kitchen_label = "open" if tool_name == "open_kitchen" else "loaded" + envelope = build_recipe_envelope( + payload, + artifact_path=_PLACEHOLDER_ARTIFACT_PATH, + sha256="0" * 64, + bound=bound, + success=True, + kitchen=kitchen_label, + version=__version__, + step_index=step_index, + step_flow_skeleton=skeleton, + ) + return payload, envelope + + +# Test A6 +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_open_kitchen_envelope_fits_smallest_backend_by_construction(recipe_name: str) -> None: + """Every bundled recipe's open_kitchen envelope fits the worst-case backend bound + by construction, and every post-prune step's routing edges survive into the + step-flow skeleton.""" + bound = resolve_worst_case_delivery_bound() * 4 + payload, envelope = _envelope_for_recipe(recipe_name, "open_kitchen", bound=bound) + rendered = _canonical_json(envelope) + assert len(rendered.encode("utf-8")) <= bound, ( + f"{recipe_name}: envelope is {len(rendered.encode('utf-8'))} bytes, " + f"exceeds worst-case bound {bound}" + ) + step_names = payload.get("post_prune_step_names", []) + assert isinstance(step_names, list) + skeleton_by_name = {entry["name"]: entry for entry in envelope["step_flow_skeleton"]} + parsed_content = load_yaml(payload["content"]) + steps_obj = parsed_content.get("steps", {}) if isinstance(parsed_content, dict) else {} + routing_fields = ("on_success", "on_failure", "on_result", "on_context_limit") + for step_name in step_names: + assert step_name in skeleton_by_name, ( + f"{recipe_name}: step {step_name!r} missing from step_flow_skeleton" + ) + assert step_name in envelope["step_index"], ( + f"{recipe_name}: step {step_name!r} missing from step_index" + ) + step_obj = steps_obj.get(step_name) + if not isinstance(step_obj, dict): + continue + entry = skeleton_by_name[step_name] + for field in routing_fields: + if field in step_obj: + assert entry.get(field) == step_obj[field], ( + f"{recipe_name}/{step_name}: routing edge {field!r} " + "missing or mismatched in skeleton" + ) + +# Test A7 @pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) -def test_bundled_recipe_open_kitchen_fits_or_spills_per_backend( - recipe_name: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +def test_envelope_step_index_covers_all_steps(recipe_name: str) -> None: + """step_index covers exactly the post-prune step set via the step:{name} scheme.""" + bound = resolve_worst_case_delivery_bound() * 4 + payload, envelope = _envelope_for_recipe(recipe_name, "open_kitchen", bound=bound) + step_names = payload.get("post_prune_step_names", []) + assert isinstance(step_names, list) + + assert set(envelope["step_index"]) == set(step_names) + for name, identifier in envelope["step_index"].items(): + assert identifier == f"step:{name}" + + parsed_content = load_yaml(payload["content"]) + steps_obj = parsed_content.get("steps", {}) if isinstance(parsed_content, dict) else {} + for name in step_names: + assert steps_obj.get(name), f"{recipe_name}: step {name!r} has an empty YAML block" + + +# Test A8 +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_largest_step_fits_pull_bound(recipe_name: str) -> None: + """Every bundled recipe's largest single-step YAML block fits the pull-tool bound, + or is losslessly reconstructible via chunked retrieval when it doesn't.""" + payload = _full_recipe_payload(recipe_name, "open_kitchen") + parsed_content = load_yaml(payload["content"]) + steps_obj = parsed_content.get("steps", {}) if isinstance(parsed_content, dict) else {} + bound = resolve_worst_case_delivery_bound() * 4 + + serialized_steps: dict[str, str] = {} + sizes: dict[str, int] = {} + for name, step_obj in steps_obj.items(): + serialized = dump_yaml_str({"steps": {name: step_obj}}, default_flow_style=False) + serialized_steps[name] = serialized + sizes[name] = len(serialized.encode("utf-8")) + + if not sizes: + return + + largest_name = max(sizes, key=lambda name: sizes[name]) + largest_size = sizes[largest_name] + assert largest_size <= bound, ( + f"{recipe_name}: step {largest_name!r} is {largest_size} bytes, exceeds pull bound {bound}" + ) + + if largest_size > bound: + # Defensive: only exercised if a bundled recipe's step ever exceeds the + # worst-case pull bound. Mirrors get_recipe_section's chunking scheme + # (tools_recipe._chunk_response_if_oversized) and asserts lossless + # reconstruction via concatenation. + content_text = serialized_steps[largest_name] + chunk_size = max(1024, bound - 512) + total = len(content_text) + chunks = max(1, (total + chunk_size - 1) // chunk_size) + reconstructed = "".join( + content_text[part * chunk_size : (part + 1) * chunk_size] for part in range(chunks) + ) + assert reconstructed == content_text + + +# Test A9 +@pytest.mark.parametrize("tool_name", ["open_kitchen", "load_recipe"]) +@pytest.mark.parametrize("recipe_name", _recipe_names(), ids=lambda n: n) +def test_bundled_recipe_envelope_fits_per_backend( + tool_name: str, + recipe_name: str, ) -> None: - """For each backend, the ``open_kitchen`` payload either fits the - effective delivery bound or spills to a projection that fits.""" - monkeypatch.chdir(tmp_path) - payload = _full_open_kitchen_payload(recipe_name) - serialized = json.dumps(payload) - serialized_bytes = len(serialized.encode("utf-8")) + """Every recipe's envelope fits every registered backend's positive delivery bound, + with every post-prune step name covered by the step-flow skeleton and step_index.""" 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: + if bound_tokens <= 0: 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)" + bound_bytes = _effective_bound_bytes(bound_tokens) + payload, envelope = _envelope_for_recipe(recipe_name, tool_name, bound=bound_bytes) + rendered = _canonical_json(envelope) + assert len(rendered.encode("utf-8")) <= bound_bytes, ( + f"{backend_name}: envelope for {tool_name}/{recipe_name} exceeds {bound_bytes} bytes" ) - data = json.loads(result) - assert data.get("delivery_bound_spill") is True - metadata = data[RESPONSE_SPILL_METADATA_KEY] - assert metadata["reason"] == "delivery_bound" + + step_names = payload.get("post_prune_step_names", []) + assert isinstance(step_names, list) + skeleton_names = {entry["name"] for entry in envelope["step_flow_skeleton"]} + for step_name in step_names: + assert step_name in skeleton_names, ( + f"{tool_name}/{backend_name}/{recipe_name}: " + f"post-prune step {step_name!r} missing from step_flow_skeleton" + ) + assert step_name in envelope["step_index"], ( + f"{tool_name}/{backend_name}/{recipe_name}: " + f"post-prune step {step_name!r} missing from step_index" + ) def test_non_exempted_oversized_payload_spills_within_delivery_bound(tmp_path) -> None: @@ -141,3 +280,30 @@ 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" + + +def test_cross_threshold_relationships() -> None: + worst_case = resolve_worst_case_delivery_bound() + assert worst_case > 0 + for backend_name, caps in _backend_capabilities().items(): + enforcement_bound = resolve_effective_delivery_bound(caps) + if enforcement_bound > 0: + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT >= enforcement_bound, ( + f"{backend_name}: configured tool-output capacity is below " + f"the runtime enforcement bound" + ) + + max_exemption_bytes = max( + definition.max_utf8_bytes for definition in RESPONSE_BACKSTOP_EXEMPTION_REGISTRY.values() + ) + assert CODEX_TOOL_OUTPUT_TOKEN_LIMIT == ((max_exemption_bytes + 3) // 4) + 8_000 + + +@pytest.mark.parametrize("tool_name", sorted(RESPONSE_BACKSTOP_EXEMPTION_REGISTRY)) +def test_exemption_ceiling_covers_all_bundled_recipe_payloads(tool_name: str) -> None: + exemption = RESPONSE_BACKSTOP_EXEMPTION_REGISTRY[tool_name] + measured = [ + json.dumps(_full_recipe_payload(recipe_name, tool_name)) for recipe_name in _recipe_names() + ] + assert max(len(payload) for payload in measured) <= exemption.max_chars + assert max(len(payload.encode("utf-8")) for payload in measured) <= exemption.max_utf8_bytes diff --git a/tests/docs/test_doc_counts.py b/tests/docs/test_doc_counts.py index 1c3e8b311..4a55819d7 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( diff --git a/tests/execution/backends/test_backend_registry.py b/tests/execution/backends/test_backend_registry.py index 39d7459c2..e756b252c 100644 --- a/tests/execution/backends/test_backend_registry.py +++ b/tests/execution/backends/test_backend_registry.py @@ -7,6 +7,7 @@ ClaudeCodeBackend, CodexBackend, get_backend, + resolve_worst_case_delivery_bound, ) pytestmark = [pytest.mark.layer("execution"), pytest.mark.small] @@ -35,6 +36,13 @@ def test_get_backend_codex(self) -> None: result = get_backend("codex") assert isinstance(result, CodexBackend) + def test_worst_case_delivery_bound_uses_smallest_registered_limit(self) -> None: + assert resolve_worst_case_delivery_bound() == min( + backend_cls().capabilities.effective_delivery_token_limit + for backend_cls in BACKEND_REGISTRY.values() + if backend_cls().capabilities.effective_delivery_token_limit > 0 + ) + def test_all_exports_complete(self) -> None: from autoskillit.execution.backends import __all__ as all_exports @@ -72,6 +80,7 @@ def test_all_exports_complete(self) -> None: "generate_codex_hooks_config", "get_backend", "make_codex_scenario_player", + "resolve_worst_case_delivery_bound", "sync_hooks_to_codex_config", } assert set(all_exports) == expected diff --git a/tests/fleet/test_pack_enforcement.py b/tests/fleet/test_pack_enforcement.py index 4f558814d..78e709c71 100644 --- a/tests/fleet/test_pack_enforcement.py +++ b/tests/fleet/test_pack_enforcement.py @@ -322,7 +322,7 @@ def test_kitchen_core_and_packs_partition_kitchen_gated_tools() -> None: for tools in TOOLS_BY_PACK.values(): union_of_packs |= tools - orphaned = all_tools_in_subsets - union_of_packs + orphaned = all_tools_in_subsets - union_of_packs - {"get_recipe_section"} assert not orphaned, f"Tools not in any pack: {orphaned}" extra = union_of_packs - all_tools_in_subsets assert not extra, f"Tools in packs but not in TOOL_SUBSET_TAGS: {extra}" diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index b544178ec..51cb815f3 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1052,8 +1052,8 @@ 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": (142_044, "remediation", "all_truthy"), + "open_kitchen": (142_103, "remediation", "all_truthy"), } diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index a2bc62ce1..8d28b16a2 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", 304), + ("src/autoskillit/server/tools/tools_kitchen.py", 323), + ("src/autoskillit/server/tools/tools_kitchen.py", 357), + ("src/autoskillit/server/tools/tools_kitchen.py", 1431), # 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..c3cdca6e6 100644 --- a/tests/pipeline/test_gate.py +++ b/tests/pipeline/test_gate.py @@ -48,6 +48,7 @@ def test_gated_tools_contains_expected_names(): "kitchen_status", "list_recipes", "load_recipe", + "get_recipe_section", "validate_recipe", "register_clone_status", "batch_cleanup_clones", diff --git a/tests/recipe/test_api.py b/tests/recipe/test_api.py index 5f1ff7b9b..ac7d5dd47 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,57 @@ 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)" +def test_suggestions_and_errors_bounded_at_source(tmp_path, monkeypatch): + import autoskillit.recipe._api as api_mod + import autoskillit.recipe._api_cache as cache_mod + + monkeypatch.setattr(cache_mod, "_LOAD_CACHE", cache_mod.LoadCache()) + bounded_findings_yaml = _RECIPE_NO_RULES.replace( + "name: test-recipe-no-rules", "name: bounded-findings" + ) + _setup_project_recipe(tmp_path, "bounded-findings", bounded_findings_yaml) + + structural_errors = [f"structural-error-{index}: " + "e" * 900 for index in range(80)] + semantic_suggestions = [ + { + "rule": f"semantic-rule-{index}", + "severity": "warning", + "step": f"step-{index}", + "message": "s" * 900, + } + for index in range(80) + ] + monkeypatch.setattr( + api_mod, + "validate_recipe_structure", + lambda _recipe: structural_errors, + ) + monkeypatch.setattr( + api_mod, + "findings_to_dicts", + lambda _findings: semantic_suggestions, + ) + monkeypatch.setattr( + api_mod, + "check_diagram_staleness", + lambda _name, _dir, _path: False, + ) + + result = api_mod.load_and_validate("bounded-findings", project_dir=tmp_path) + + for key in ("suggestions", "errors"): + values = result[key] + assert len(values) <= api_mod._MAX_SUGGESTIONS_COUNT + assert len(json.dumps(values, ensure_ascii=False).encode("utf-8")) <= ( + api_mod._MAX_SUGGESTIONS_BYTES + ) + sentinel = next( + item for item in values if isinstance(item, dict) and item.get("truncated") is True + ) + assert sentinel["original_count"] == 80 + assert sentinel["shown_count"] == len(values) - 1 + + # --------------------------------------------------------------------------- # Minimal recipe fixture for cache tests # --------------------------------------------------------------------------- diff --git a/tests/server/conftest.py b/tests/server/conftest.py index e241940f0..951eb54ec 100644 --- a/tests/server/conftest.py +++ b/tests/server/conftest.py @@ -129,6 +129,27 @@ def _make_mock_ctx() -> MagicMock: return ctx +def _make_track_response_ready_mock_ctx(*, delivery_token_limit: int = 1_000_000) -> MagicMock: + """Return a mock_ctx with backend.capabilities configured so track_response_size + resolves an effective delivery bound instead of falling back to the worst case. + + Tests that route open_kitchen (or any @track_response_size-wrapped tool) through + track_response_size's decorator stack without a real kitchen context must patch + ``autoskillit.server._notify._get_ctx_or_none`` to return this ctx. This avoids + the bounded_response_budget_failure("context_unavailable") envelope that the Part A + remediation now produces for any ctx where caps is not a real BackendCapabilities. + """ + from autoskillit.core import BackendCapabilities + + ctx = _make_mock_ctx() + ctx.backend = MagicMock() + ctx.backend.capabilities = BackendCapabilities( + effective_delivery_token_limit=delivery_token_limit + ) + ctx.temp_dir = Path("/tmp") + return ctx + + _SUCCESS_JSON = ( '{"type": "result", "subtype": "success", "is_error": false,' ' "result": "done", "session_id": "s1"}' diff --git a/tests/server/test_capability_admission_e2e.py b/tests/server/test_capability_admission_e2e.py index fde1fde1c..9a13cf7a1 100644 --- a/tests/server/test_capability_admission_e2e.py +++ b/tests/server/test_capability_admission_e2e.py @@ -108,7 +108,7 @@ def test_capability_override_parity() -> None: @pytest.mark.anyio -async def test_open_kitchen_refuses_doa_codex_pipeline() -> None: +async def test_open_kitchen_refuses_doa_codex_pipeline(tmp_path: Path) -> None: """open_kitchen must return success=False with kitchen='dispatch_infeasible' when load_and_validate reports dispatch_feasible=False.""" import json @@ -122,6 +122,8 @@ async def test_open_kitchen_refuses_doa_codex_pipeline() -> None: tool_ctx.gate_infrastructure_ready = True tool_ctx.recipe_name = "implementation" tool_ctx.kitchen_id = "test-kitchen" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + tool_ctx.temp_dir = tmp_path tool_ctx.backend.name = "codex" tool_ctx.recipes.load_and_validate.return_value = { "content": "name: implementation\nsteps:\n build:\n cmd: task build\n", @@ -355,7 +357,7 @@ def test_provider_aware_capability_override_claude_backend_no_op() -> None: @pytest.mark.anyio -async def test_open_kitchen_codex_with_provider_overrides_feasible() -> None: +async def test_open_kitchen_codex_with_provider_overrides_feasible(tmp_path: Path) -> None: """open_kitchen must return success=True with kitchen='open' when Codex backend has provider-overridden guarded steps (provider-aware path).""" import json @@ -369,6 +371,8 @@ async def test_open_kitchen_codex_with_provider_overrides_feasible() -> None: tool_ctx.gate_infrastructure_ready = True tool_ctx.recipe_name = "implementation" tool_ctx.kitchen_id = "test-kitchen" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + tool_ctx.temp_dir = tmp_path _setup_provider_override_ctx(tool_ctx) tool_ctx.recipes.load_and_validate.return_value = _make_feasible_load_result() @@ -386,11 +390,13 @@ async def test_open_kitchen_codex_with_provider_overrides_feasible() -> None: parsed = json.loads(result) assert parsed["success"] is True assert parsed["kitchen"] == "open" - assert "implement" in parsed.get("post_prune_step_names", []) + assert "implement" in parsed.get("step_index", {}) @pytest.mark.anyio -async def test_load_recipe_codex_with_provider_overrides_no_infeasible() -> None: +async def test_load_recipe_codex_with_provider_overrides_no_infeasible( + tmp_path: Path, +) -> None: """load_recipe must NOT return dispatch_infeasible when Codex backend has provider-overridden guarded steps.""" import json @@ -402,6 +408,10 @@ async def test_load_recipe_codex_with_provider_overrides_no_infeasible() -> None tool_ctx = _make_mock_ctx() tool_ctx.gate.enabled = True tool_ctx.kitchen_id = "test-kitchen" + # New envelope: load_recipe persists the full recipe payload to + # tool_ctx.temp_dir/responses/load_recipe/. Provide a real temp_dir + # so artifact_dir.mkdir succeeds under the mock. + tool_ctx.temp_dir = tmp_path _setup_provider_override_ctx(tool_ctx) tool_ctx.recipes.load_and_validate.return_value = _make_feasible_load_result() @@ -625,7 +635,9 @@ def _per_step_resolve(_step_name, _recipe_name, _config_providers, step_provider @pytest.mark.anyio -async def test_open_kitchen_no_override_infeasible_includes_provider_guidance() -> None: +async def test_open_kitchen_no_override_infeasible_includes_provider_guidance( + tmp_path: Path, +) -> None: """Test 1B: open_kitchen with Codex + NO provider overrides (none_pass) produces dispatch_infeasible envelope with missing_provider_steps and escape_hatch.""" import json @@ -639,6 +651,8 @@ async def test_open_kitchen_no_override_infeasible_includes_provider_guidance() tool_ctx.gate_infrastructure_ready = True tool_ctx.recipe_name = "implementation" tool_ctx.kitchen_id = "test-kitchen" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + tool_ctx.temp_dir = tmp_path tool_ctx.backend.name = "codex" from types import SimpleNamespace @@ -1149,6 +1163,7 @@ def _make_git_write_resolver() -> MagicMock: @pytest.mark.anyio async def test_open_kitchen_capability_route_reaches_admission_with_true_override( monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, ) -> None: """Step 5d sibling of Test 1B: open_kitchen with Codex + NO provider overrides but capability route active (git_metadata_write skills + binary present) must @@ -1170,6 +1185,8 @@ async def test_open_kitchen_capability_route_reaches_admission_with_true_overrid tool_ctx.gate_infrastructure_ready = True tool_ctx.recipe_name = "implementation" tool_ctx.kitchen_id = "test-kitchen" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + tool_ctx.temp_dir = tmp_path from types import SimpleNamespace diff --git a/tests/server/test_lock_ingredients.py b/tests/server/test_lock_ingredients.py index 0462cc062..b4364d80d 100644 --- a/tests/server/test_lock_ingredients.py +++ b/tests/server/test_lock_ingredients.py @@ -393,6 +393,8 @@ async def test_every_authority_key_emits_feedback_on_both_surfaces( mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -417,7 +419,12 @@ async def test_every_authority_key_emits_feedback_on_both_surfaces( ctx=mock_ctx, ) parsed = json.loads(result_str) - warnings = parsed.get("warnings") or [] + # warnings is not a compact-envelope field — it + # survives on the persisted artifact payload. + artifact = json.loads( + Path(parsed["artifact_path"]).read_text(encoding="utf-8") + ) + warnings = artifact.get("warnings") or [] matching = [w for w in warnings if key in w] assert matching, ( f"open_kitchen must emit a warning mentioning {key!r} " diff --git a/tests/server/test_mcp_overrides.py b/tests/server/test_mcp_overrides.py index dd63a3f77..2a2a0d5bd 100644 --- a/tests/server/test_mcp_overrides.py +++ b/tests/server/test_mcp_overrides.py @@ -38,6 +38,9 @@ async def test_load_recipe_tool_accepts_overrides_param(tmp_path: Path) -> None: } ) mock_tool_ctx = _make_mock_ctx(mock_recipes) + # New envelope: load_recipe persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_tool_ctx.temp_dir = tmp_path with ( patch("autoskillit.server.tools.tools_recipe._require_enabled", return_value=None), @@ -61,7 +64,11 @@ async def test_load_recipe_tool_accepts_overrides_param(tmp_path: Path) -> None: ) result = json.loads(result_str) assert "error" not in result - assert result.get("valid") is True + # New envelope shape: success=True confirms recipe load succeeded; + # artifact_path carries the full recipe payload for on-demand pull. + assert result.get("success") is True + assert result.get("kitchen") == "loaded" + assert "artifact_path" in result # Verify user-supplied overrides were passed through to load_and_validate # (merged with auto-injected kitchen_id and pipeline_health) @@ -84,6 +91,9 @@ async def test_open_kitchen_accepts_overrides_param(tmp_path: Path) -> None: } ) mock_tool_ctx = _make_mock_ctx(mock_recipes) + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_tool_ctx.temp_dir = tmp_path mock_mcp_ctx = AsyncMock() mock_mcp_ctx.enable_components = AsyncMock() @@ -117,8 +127,11 @@ async def test_open_kitchen_accepts_overrides_param(tmp_path: Path) -> None: ctx=mock_mcp_ctx, ) result = json.loads(result_str) + # New envelope shape: success=True + kitchen="open" confirm the recipe + # loaded and the gate opened; "valid" is not carried into the compact + # envelope (it's implied by success and surfaced via the artifact). + assert result.get("success") is True assert result.get("kitchen") == "open" - assert result.get("valid") is True # Verify user-supplied overrides were passed through to load_and_validate # (merged with auto-injected kitchen_id and pipeline_health) @@ -157,6 +170,9 @@ async def test_unknown_override_key_warned(tmp_path: Path) -> None: mock_recipes.load.return_value = mock_recipe mock_tool_ctx = _make_mock_ctx(mock_recipes) + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_tool_ctx.temp_dir = tmp_path mock_mcp_ctx = AsyncMock() with ( @@ -192,8 +208,11 @@ async def test_unknown_override_key_warned(tmp_path: Path) -> None: ) result = json.loads(result_str) assert result.get("kitchen") == "open" - warnings = result.get("warnings", []) - assert warnings, f"Expected warnings for unknown override key, got result: {result}" + # New envelope shape: warnings are not carried into the compact envelope + # directly — read them from the persisted artifact via artifact_path. + artifact = json.loads(Path(result["artifact_path"]).read_text()) + warnings = artifact.get("warnings", []) + assert warnings, f"Expected warnings for unknown override key, got artifact: {artifact}" assert any("audit_impl" in w for w in warnings) @@ -223,6 +242,9 @@ async def test_valid_override_key_no_warning(tmp_path: Path) -> None: mock_recipes.load.return_value = mock_recipe mock_tool_ctx = _make_mock_ctx(mock_recipes) + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_tool_ctx.temp_dir = tmp_path mock_mcp_ctx = AsyncMock() with ( @@ -258,7 +280,10 @@ async def test_valid_override_key_no_warning(tmp_path: Path) -> None: ) result = json.loads(result_str) assert result.get("kitchen") == "open" - assert "warnings" not in result, f"Expected no warnings, got: {result.get('warnings')}" + # New envelope shape never carries "warnings" directly; verify via the + # persisted artifact that no override warnings were recorded either. + artifact = json.loads(Path(result["artifact_path"]).read_text()) + assert "warnings" not in artifact, f"Expected no warnings, got: {artifact.get('warnings')}" async def test_unknown_override_key_warned_deferred_recall(tmp_path: Path) -> None: @@ -287,6 +312,9 @@ async def test_unknown_override_key_warned_deferred_recall(tmp_path: Path) -> No mock_recipes.load.return_value = mock_recipe mock_tool_ctx = _make_mock_ctx(mock_recipes) + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_tool_ctx.temp_dir = tmp_path # Simulate kitchen already open with recipe "test" loaded → deferred-recall path mock_tool_ctx.gate.enabled = True mock_tool_ctx.recipe_name = "test" @@ -314,6 +342,9 @@ async def test_unknown_override_key_warned_deferred_recall(tmp_path: Path) -> No ) result = json.loads(result_str) assert result.get("kitchen") == "open" - warnings = result.get("warnings", []) - assert warnings, f"Expected warnings on deferred-recall path, got result: {result}" + # New envelope shape: warnings are not carried into the compact envelope + # directly — read them from the persisted artifact via artifact_path. + artifact = json.loads(Path(result["artifact_path"]).read_text()) + warnings = artifact.get("warnings", []) + assert warnings, f"Expected warnings on deferred-recall path, got artifact: {artifact}" assert any("audit_impl" in w for w in warnings) diff --git a/tests/server/test_open_kitchen_deferred_recall.py b/tests/server/test_open_kitchen_deferred_recall.py index 2602806b3..f402b0256 100644 --- a/tests/server/test_open_kitchen_deferred_recall.py +++ b/tests/server/test_open_kitchen_deferred_recall.py @@ -23,11 +23,14 @@ def _make_deferred_recall_ctx(name: str) -> MagicMock: @pytest.mark.anyio -async def test_deferred_recall_sets_active_recipe_steps_from_recipe(): +async def test_deferred_recall_sets_active_recipe_steps_from_recipe(tmp_path): """Deferred-recall path populates active_recipe_steps from the freshly loaded recipe.""" from autoskillit.server.tools.tools_kitchen import open_kitchen mock_ctx = _make_deferred_recall_ctx("test-recipe") + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path mock_ctx.recipes.load_and_validate.return_value = { "content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n", "valid": True, @@ -60,11 +63,14 @@ async def test_deferred_recall_sets_active_recipe_steps_from_recipe(): @pytest.mark.anyio -async def test_deferred_recall_sets_active_recipe_steps_none_when_find_raises(): +async def test_deferred_recall_sets_active_recipe_steps_none_when_find_raises(tmp_path): """When recipes.find raises, active_recipe_steps is set to None and the call still succeeds.""" from autoskillit.server.tools.tools_kitchen import open_kitchen mock_ctx = _make_deferred_recall_ctx("test-recipe") + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path mock_ctx.recipes.load_and_validate.return_value = { "content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n", "valid": True, @@ -87,11 +93,14 @@ async def test_deferred_recall_sets_active_recipe_steps_none_when_find_raises(): @pytest.mark.anyio -async def test_deferred_recall_sets_active_recipe_steps_none_when_find_returns_none(): +async def test_deferred_recall_sets_active_recipe_steps_none_when_find_returns_none(tmp_path): """When recipes.find returns None (recipe not on disk), active_recipe_steps is None.""" from autoskillit.server.tools.tools_kitchen import open_kitchen mock_ctx = _make_deferred_recall_ctx("test-recipe") + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path mock_ctx.recipes.load_and_validate.return_value = { "content": "name: test-recipe\nsteps:\n build:\n cmd: task build\n", "valid": True, @@ -221,12 +230,15 @@ def _make_pre_revealed_ctx(name: str) -> MagicMock: @pytest.mark.anyio -async def test_pre_reveal_then_open_does_not_re_execute_handler(): +async def test_pre_reveal_then_open_does_not_re_execute_handler(tmp_path): """Pre-revealed state (gate enabled, recipe_name empty, infrastructure ready) must skip _open_kitchen_handler and still load the recipe.""" from autoskillit.server.tools import tools_kitchen mock_ctx = _make_pre_revealed_ctx("test-recipe") + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path mock_recipe_info = MagicMock() mock_recipe_info.path = Path("/fake/.autoskillit/recipes/test-recipe.yaml") mock_ctx.recipes.find.return_value = mock_recipe_info @@ -406,6 +418,9 @@ async def test_deferred_recall_preserves_active_locks(tmp_path): ctx = _make_deferred_recall_ctx("test-recipe") ctx.project_dir = tmp_path + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + ctx.temp_dir = tmp_path ctx.active_recipe_steps = {"investigate": MagicMock(skip_when_false="inputs.investigate")} ctx.active_recipe_ingredients = frozenset(["investigate"]) ctx.gate.enabled = True diff --git a/tests/server/test_pipeline_tracker.py b/tests/server/test_pipeline_tracker.py index d1fa42cfe..19260122d 100644 --- a/tests/server/test_pipeline_tracker.py +++ b/tests/server/test_pipeline_tracker.py @@ -172,6 +172,7 @@ def _configure_open_kitchen_mock(ctx, steps, tmp_path): from unittest.mock import MagicMock ctx.project_dir = tmp_path + ctx.temp_dir = tmp_path / ".autoskillit" / "temp" ctx.recipes.load_and_validate.return_value = { "content": "name: remediation\nsteps: {}\n", "valid": True, diff --git a/tests/server/test_response_backstop.py b/tests/server/test_response_backstop.py index 44e2f1911..be2fe70eb 100644 --- a/tests/server/test_response_backstop.py +++ b/tests/server/test_response_backstop.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import json from pathlib import Path @@ -10,6 +11,7 @@ from autoskillit.config import OutputBudgetConfig from autoskillit.core import RESPONSE_BACKSTOP_EXEMPTION_REGISTRY +from autoskillit.execution import resolve_worst_case_delivery_bound from autoskillit.server._response_budget import ( RESPONSE_SPILL_METADATA_KEY, RESPONSE_SPILL_METADATA_KEYS, @@ -118,7 +120,7 @@ def test_projected_utf8_bytes_is_exact_with_multibyte_digit_boundaries( monkeypatch.setattr( _response_budget, "_artifact_path", - lambda _artifact_dir, _tool_name: artifact, + lambda _artifact_dir, _tool_name, _content_hash=None: artifact, ) original = ( json.dumps({"success": True, "result": "界" * 10_000}, ensure_ascii=False) @@ -158,7 +160,7 @@ def test_minimal_projection_has_exact_bytes_and_omission_aggregates(tmp_path, mo monkeypatch.setattr( _response_budget, "_artifact_path", - lambda _artifact_dir, _tool_name: tmp_path / "fixed.log", + lambda _artifact_dir, _tool_name, _content_hash=None: tmp_path / "fixed.log", ) original_data = {f"route_{index}": "界" * 120 for index in range(8)} original = json.dumps(original_data, ensure_ascii=False) @@ -308,6 +310,73 @@ def _stack_exhausted(*_args, **_kwargs): assert open(data["artifact_path"], encoding="utf-8").read() == original +def test_recipe_artifact_always_persisted(tmp_path): + payload = {"success": True, "kitchen": "open", "content": "name: demo\n"} + original = json.dumps(payload) + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=1_000_000, + ) + artifacts = list(tmp_path.iterdir()) + assert len(artifacts) == 1 + assert artifacts[0].read_text(encoding="utf-8") == original + data = json.loads(result) + assert data["artifact_path"] == str(artifacts[0].resolve()) + assert data["sha256"] == hashlib.sha256(original.encode("utf-8")).hexdigest() + + +def test_recipe_artifact_deterministic_path(tmp_path): + payload = {"success": True, "kitchen": "open", "content": "name: demo\n"} + original = json.dumps(payload) + first = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=1_000_000, + ) + second = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=1_000_000, + ) + first_data = json.loads(first) + second_data = json.loads(second) + assert first_data["artifact_path"] == second_data["artifact_path"] + expected_sha256 = hashlib.sha256(original.encode("utf-8")).hexdigest() + assert Path(first_data["artifact_path"]).name == f"open_kitchen_{expected_sha256[:16]}.log" + assert len(list(tmp_path.iterdir())) == 1 + + +def test_recipe_artifact_pre_published_not_rewritten(tmp_path): + sentinel_path = tmp_path / "sentinel.log" + sentinel_path.write_text("pre-published content", encoding="utf-8") + payload = { + "success": True, + "kitchen": "open", + "artifact_path": str(sentinel_path), + "sha256": "f" * 64, + } + original = json.dumps(payload) + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=1_000_000, + ) + data = json.loads(result) + assert data["artifact_path"] == str(sentinel_path) + assert data["sha256"] == "f" * 64 + assert [p.name for p in tmp_path.iterdir()] == ["sentinel.log"] + assert sentinel_path.read_text(encoding="utf-8") == "pre-published content" + + def test_nonpositive_response_max_bytes_is_rejected(): with pytest.raises(ValueError, match="response_max_bytes"): OutputBudgetConfig(response_max_bytes=0) @@ -495,11 +564,127 @@ def test_exempted_payload_spills_when_over_delivery_bound(tmp_path): assert metadata["reason"] == "delivery_bound" +def _delivery_starvation_payload( + *, + content_chars: int, + suggestion_chars: int, + post_prune_step_names: list[str] | None = None, +) -> dict: + suggestions = [ + { + "rule": f"realistic-rule-{index}", + "severity": "warning", + "step": f"step_{index}", + "message": "s" * suggestion_chars, + } + for index in range(60) + ] + step_lines = "\n".join( + f" step_{index}:\n action: confirm\n message: {'x' * 80}" for index in range(200) + ) + content = "name: realistic-large-recipe\nsteps:\n" + step_lines + "\n" + content += "# filler\n" * max(0, content_chars - len(content)) + payload = { + "success": True, + "kitchen": "open", + "version": "1.2.3", + "ingredients_table": "| Name | Description | Default |", + "orchestration_rules": "Execute every routed step.", + "stop_step_semantics": "Stop steps are terminal.", + "errors": [], + "suggestions": suggestions, + "content": content[:content_chars], + } + if post_prune_step_names is not None: + payload["post_prune_step_names"] = list(post_prune_step_names) + return payload + + +_STARVATION_STEP_NAMES = [f"step_{index}" for index in range(200)] + + +@pytest.mark.parametrize( + "step_names", + [None, _STARVATION_STEP_NAMES], + ids=["without_post_prune_step_names", "with_post_prune_step_names"], +) +def test_delivery_bound_summary_content_starvation_by_suggestions(tmp_path, step_names): + payload = _delivery_starvation_payload( + content_chars=100_000, suggestion_chars=750, post_prune_step_names=step_names + ) + original = json.dumps(payload) + + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=10_000, + ) + + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= 40_000 + data = json.loads(result) + assert len(data["content"]) > 0 + if step_names is not None: + for step_name in step_names: + assert step_name in data["content"] + + +@pytest.mark.parametrize( + "step_names", + [None, _STARVATION_STEP_NAMES], + ids=["without_post_prune_step_names", "with_post_prune_step_names"], +) +def test_delivery_bound_summary_budget_reallocation_after_projection(tmp_path, step_names): + payload = _delivery_starvation_payload( + content_chars=30_000, suggestion_chars=850, post_prune_step_names=step_names + ) + original = json.dumps(payload) + + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=10_000, + ) + + assert isinstance(result, str) + data = json.loads(result) + assert len(data["content"]) > 0 + assert len(result.encode("utf-8")) <= 40_000 + if step_names is not None: + for step_name in step_names: + assert step_name in data["content"] + + +def test_zero_delivery_limit_uses_worst_case_bound(tmp_path): + payload = {"success": True, "content": "x" * 80_000} + original = json.dumps(payload) + + result = enforce_response_budget( + original, + tool_name="open_kitchen", + artifact_dir=tmp_path, + config=OutputBudgetConfig(), + effective_delivery_token_limit=0, + ) + + assert isinstance(result, str) + bound = resolve_worst_case_delivery_bound() * 4 + assert len(result.encode("utf-8")) <= bound + data = json.loads(result) + assert data["delivery_bound_spill"] is True + assert data[RESPONSE_SPILL_METADATA_KEY]["reason"] == "delivery_bound" + + def test_delivery_bound_summary_preserves_operational_fields(tmp_path): """Bounded summary must preserve success/kitchen/version/ingredients_table/ - orchestration_rules/stop_step_semantics/errors/suggestions verbatim, - truncate content to fit, and nest spill metadata with reason='delivery_bound' - and top-level delivery_bound_spill=True.""" + orchestration_rules/stop_step_semantics/errors/suggestions and the Part B + envelope keys (step_flow_skeleton/step_index/pull_tool/artifact_path/sha256) + verbatim, truncate content to fit, and nest spill metadata with + reason='delivery_bound' and top-level delivery_bound_spill=True.""" payload = { "success": True, "kitchen": "open", @@ -509,10 +694,38 @@ def test_delivery_bound_summary_preserves_operational_fields(tmp_path): "stop_step_semantics": {"on_success": "stop"}, "errors": [], "suggestions": [{"rule": "x"}], + "step_flow_skeleton": [ + { + "name": "step-one", + "summary": "first", + "on_success": "step-two", + "on_failure": None, + "on_result": None, + "on_context_limit": None, + }, + { + "name": "step-two", + "summary": "second", + "on_success": None, + "on_failure": "step-one", + "on_result": None, + "on_context_limit": None, + }, + ], + "step_index": {"step-one": "step:step-one", "step-two": "step:step-two"}, + "pull_tool": "get_recipe_section", + "artifact_path": str(tmp_path / "envelope-sentinel.log"), + "sha256": "deadbeef" * 8, "diagram": "graph TD; A-->B", "content": "x" * 150_000, } original = json.dumps(payload) + # tool_name="open_kitchen" is an exempted tool, so enforce_response_budget + # treats a payload-supplied artifact_path/sha256 pair as already published + # by the tool layer (test_recipe_artifact_pre_published_not_rewritten) and + # reuses it verbatim instead of persisting `original` fresh. The sentinel + # file must therefore actually exist with the expected content. + Path(payload["artifact_path"]).write_text(original, encoding="utf-8") result = enforce_response_budget( original, tool_name="open_kitchen", @@ -533,6 +746,11 @@ def test_delivery_bound_summary_preserves_operational_fields(tmp_path): assert data["stop_step_semantics"] == payload["stop_step_semantics"] assert data["errors"] == payload["errors"] assert data["suggestions"] == payload["suggestions"] + assert data["step_flow_skeleton"] == payload["step_flow_skeleton"] + assert data["step_index"] == payload["step_index"] + assert data["pull_tool"] == payload["pull_tool"] + assert data["artifact_path"] == payload["artifact_path"] + assert data["sha256"] == payload["sha256"] assert data["diagram"] == payload["diagram"] assert data["content"].startswith("x") assert payload["content"].startswith(data["content"]) diff --git a/tests/server/test_serve_idempotence.py b/tests/server/test_serve_idempotence.py index 88218e6ba..62bd14fdd 100644 --- a/tests/server/test_serve_idempotence.py +++ b/tests/server/test_serve_idempotence.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -26,12 +27,47 @@ _RECIPE = "remediation" +def _load_recipe_content(envelope: dict[str, object]) -> str: + """Read the full recipe payload referenced by a compact load_recipe envelope.""" + assert "content" not in envelope + assert envelope["pull_tool"] == "get_recipe_section" + assert { + "step_flow_skeleton", + "step_index", + "artifact_path", + "sha256", + "pull_tool", + } <= envelope.keys() + payload = json.loads(Path(str(envelope["artifact_path"])).read_text(encoding="utf-8")) + return str(payload["content"]) + + +def _open_kitchen_content(envelope: dict[str, object]) -> str: + """Read the full recipe payload referenced by a compact open_kitchen envelope.""" + assert "content" not in envelope + assert envelope["pull_tool"] == "get_recipe_section" + assert { + "step_flow_skeleton", + "step_index", + "artifact_path", + "sha256", + "pull_tool", + } <= envelope.keys() + payload = json.loads(Path(str(envelope["artifact_path"])).read_text(encoding="utf-8")) + return str(payload["content"]) + + def test_re_serve_surfaces_in_sync_with_serve_surfaces() -> None: - """_RE_SERVE_SURFACES must exactly match SERVE_SURFACES - {"open_kitchen"}.""" - assert set(_RE_SERVE_SURFACES) == SERVE_SURFACES - {"open_kitchen"}, ( + """Content-serving surfaces exclude the section-oriented artifact pull tool.""" + assert set(_RE_SERVE_SURFACES) == SERVE_SURFACES - { + "open_kitchen", + "get_recipe_section", + }, ( f"_RE_SERVE_SURFACES out of sync with SERVE_SURFACES. " - f"Missing: {(SERVE_SURFACES - {'open_kitchen'}) - set(_RE_SERVE_SURFACES)}. " - f"Extra: {set(_RE_SERVE_SURFACES) - (SERVE_SURFACES - {'open_kitchen'})}." + f"Missing: " + f"{(SERVE_SURFACES - {'open_kitchen', 'get_recipe_section'}) - set(_RE_SERVE_SURFACES)}. " + f"Extra: " + f"{set(_RE_SERVE_SURFACES) - (SERVE_SURFACES - {'open_kitchen', 'get_recipe_section'})}." ) @@ -83,11 +119,10 @@ async def test_load_recipe_after_open_kitchen_with_overrides_serves_identical_co monkeypatch, ) assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" - ok_content = ok_result["content"] + ok_content = _open_kitchen_content(ok_result) lr_result = json.loads(await load_recipe(name=_RECIPE)) - assert "content" in lr_result, f"load_recipe missing content: {lr_result}" - lr_content = lr_result["content"] + lr_content = _load_recipe_content(lr_result) assert ok_content == lr_content, ( "load_recipe content diverges from open_kitchen content — " @@ -114,7 +149,7 @@ async def test_load_recipe_after_open_kitchen_without_overrides_serves_identical monkeypatch, ) assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" - ok_content = ok_result["content"] + ok_content = _open_kitchen_content(ok_result) assert tool_ctx_kitchen_open.session_serve_overrides == {}, ( "session_serve_overrides must be empty dict (not None) when no overrides passed" @@ -124,8 +159,7 @@ async def test_load_recipe_after_open_kitchen_without_overrides_serves_identical ) lr_result = json.loads(await load_recipe(name=_RECIPE)) - assert "content" in lr_result, f"load_recipe missing content: {lr_result}" - lr_content = lr_result["content"] + lr_content = _load_recipe_content(lr_result) assert ok_content == lr_content, ( "load_recipe content diverges from open_kitchen content (no-override path) — " @@ -152,7 +186,7 @@ async def test_deferred_recall_open_kitchen_serves_identical_to_first_serving( monkeypatch, ) assert first_result.get("success") is True, f"first open_kitchen failed: {first_result}" - first_content = first_result["content"] + first_content = _open_kitchen_content(first_result) deferred_result = await _open_kitchen_patched( _RECIPE, @@ -162,12 +196,17 @@ async def test_deferred_recall_open_kitchen_serves_identical_to_first_serving( assert deferred_result.get("success") is True, ( f"deferred-recall open_kitchen failed: {deferred_result}" ) - deferred_content = deferred_result["content"] + deferred_content = _open_kitchen_content(deferred_result) assert first_content == deferred_content, ( "Deferred-recall open_kitchen content diverges from first serving — " "session_serve_overrides not injected into deferred-recall _merged_overrides" ) + # NOTE: artifact_path/sha256 are NOT asserted equal here — the first call + # passes explicit overrides (producing an authority-clobber `warnings` key) + # while the deferred-recall call passes none, so the persisted payloads + # legitimately differ outside of `content`. Determinism for identical + # inputs is covered by test_recipe_artifact_deterministic_path. async def test_session_serve_overrides_cleared_on_close_kitchen( @@ -234,11 +273,11 @@ async def test_explicit_load_recipe_overrides_layer_on_top_of_session_baseline( lr_result = json.loads( await load_recipe(name=_RECIPE, overrides={"extra_ingredient": "extra_value"}) ) - assert "content" in lr_result, f"load_recipe missing content: {lr_result}" + lr_content = _load_recipe_content(lr_result) from autoskillit.core.io import load_yaml - parsed = load_yaml(lr_result["content"]) + parsed = load_yaml(lr_content) assert parsed["steps"]["clone"]["on_success"] == "claim_and_resolve", ( "issue_url session baseline must still be active when load_recipe passes extra_ingredient" ) @@ -270,7 +309,7 @@ async def test_get_recipe_content_matches_open_kitchen_with_overrides( monkeypatch, ) assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" - ok_content = ok_result["content"] + ok_content = _open_kitchen_content(ok_result) gr_content = get_recipe(_RECIPE) try: @@ -308,8 +347,7 @@ async def _call_re_serve_surface( from autoskillit.server.tools.tools_recipe import load_recipe result = json.loads(await load_recipe(name=recipe_name)) - assert "content" in result, f"load_recipe returned no content: {result}" - return result["content"] + return _load_recipe_content(result) elif surface == "get_recipe": from autoskillit.server.tools.tools_kitchen import get_recipe @@ -323,7 +361,7 @@ async def _call_re_serve_surface( elif surface == "open_kitchen_deferred_recall": result = await _open_kitchen_patched(recipe_name, None, monkeypatch) assert result.get("success") is True, f"deferred-recall failed: {result}" - return result["content"] + return _open_kitchen_content(result) else: raise ValueError(f"Unknown surface: {surface!r}") @@ -353,7 +391,7 @@ async def test_serve_surfaces_parametric_content_identity( monkeypatch, ) assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" - ok_content = ok_result["content"] + ok_content = _open_kitchen_content(ok_result) re_served_content = await _call_re_serve_surface(surface, _RECIPE, monkeypatch) @@ -383,7 +421,7 @@ async def test_get_recipe_snapshot_lifecycle( monkeypatch, ) assert ok_result.get("success") is True, f"open_kitchen failed: {ok_result}" - ok_content = ok_result["content"] + ok_content = _open_kitchen_content(ok_result) gr_content = get_recipe(_RECIPE) try: @@ -464,10 +502,10 @@ async def test_load_recipe_routing_matches_open_kitchen_for_arbitrary_overrides( assume(False) # discard: open_kitchen failed (e.g. invalid combos) lr_result = json.loads(await load_recipe(name=_RECIPE)) - if "content" not in lr_result: + if not lr_result.get("success"): assume(False) # discard: load_recipe failed - assert lr_result["content"] == ok_result["content"], ( + assert _load_recipe_content(lr_result) == _open_kitchen_content(ok_result), ( f"Routing divergence for overrides={overrides!r}: " "load_recipe content must match open_kitchen content" ) diff --git a/tests/server/test_server_tool_registration.py b/tests/server/test_server_tool_registration.py index 54bb66a8f..c9b76c8f2 100644 --- a/tests/server/test_server_tool_registration.py +++ b/tests/server/test_server_tool_registration.py @@ -61,6 +61,7 @@ async def test_all_tools_exist(self, kitchen_enabled): "read_db", "list_recipes", "load_recipe", + "get_recipe_section", "migrate_recipe", "kitchen_status", "validate_recipe", diff --git a/tests/server/test_tools_kitchen_cache_poison.py b/tests/server/test_tools_kitchen_cache_poison.py index 3fa5e2009..117bdcca2 100644 --- a/tests/server/test_tools_kitchen_cache_poison.py +++ b/tests/server/test_tools_kitchen_cache_poison.py @@ -5,6 +5,7 @@ from __future__ import annotations import json +from pathlib import Path from unittest.mock import AsyncMock, patch import pytest @@ -45,9 +46,15 @@ async def test_open_kitchen_ingredients_only_does_not_poison_load_recipe( ) lr_result = json.loads(await load_recipe(name="implementation")) - assert "content" in lr_result, ( - "load_recipe must return 'content' after open_kitchen(ingredients_only=True); " - "cache was poisoned by the ingredients_only pop" - ) - assert isinstance(lr_result["content"], str) - assert len(lr_result["content"]) > 0 + assert "content" not in lr_result + assert lr_result["pull_tool"] == "get_recipe_section" + assert { + "step_flow_skeleton", + "step_index", + "artifact_path", + "sha256", + "pull_tool", + } <= lr_result.keys() + payload = json.loads(Path(lr_result["artifact_path"]).read_text(encoding="utf-8")) + assert isinstance(payload["content"], str) + assert payload["content"] diff --git a/tests/server/test_tools_kitchen_envelope.py b/tests/server/test_tools_kitchen_envelope.py index 60e29d685..87ee8d987 100644 --- a/tests/server/test_tools_kitchen_envelope.py +++ b/tests/server/test_tools_kitchen_envelope.py @@ -9,7 +9,7 @@ import pytest from tests.server._helpers import _PATCHED_DEFAULTS, _SERVER_ONLY_KEYS -from tests.server.conftest import _make_mock_ctx +from tests.server.conftest import _make_mock_ctx, _make_track_response_ready_mock_ctx pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -43,10 +43,13 @@ async def test_open_kitchen_warns_on_orphaned_hooks(tmp_path, monkeypatch): lambda _: [], ) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -86,10 +89,13 @@ async def test_open_kitchen_warns_on_missing_hook_scripts(tmp_path, monkeypatch) lambda _: HookDriftResult(missing=0, orphaned=0), ) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -198,10 +204,13 @@ async def raise_type_error(*a, **kw): async def test_open_kitchen_no_name_returns_json_envelope_with_success_true(tmp_path, monkeypatch): """No-recipe open_kitchen returns JSON envelope with success=True.""" monkeypatch.chdir(tmp_path) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -236,6 +245,9 @@ async def test_open_kitchen_recipe_found_returns_envelope_with_content_and_ingre } mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -272,6 +284,9 @@ async def test_open_kitchen_injects_hidden_ingredient_overrides(tmp_path, monkey mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -324,6 +339,9 @@ async def test_config_layer_keys_match_server_authoritative_ingredients(tmp_path mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -361,7 +379,7 @@ async def test_config_layer_keys_match_server_authoritative_ingredients(tmp_path @pytest.mark.anyio -async def test_open_kitchen_smoke_test_renders_resolved_base_branch(monkeypatch): +async def test_open_kitchen_smoke_test_renders_resolved_base_branch(tmp_path, monkeypatch): """T7: open_kitchen smoke-test renders the config-resolved base_branch value.""" monkeypatch.delenv("AUTOSKILLIT_HEADLESS", raising=False) import autoskillit.recipe._api_cache as cache_mod @@ -383,6 +401,9 @@ async def test_open_kitchen_smoke_test_renders_resolved_base_branch(monkeypatch) mock_ctx.backend = None mock_ctx.recipes = DefaultRecipeRepository() mock_ctx.config.migration.suppressed = [] + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -429,6 +450,9 @@ async def test_open_kitchen_config_authority_overrides_caller(tmp_path, monkeypa mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -486,6 +510,9 @@ async def test_open_kitchen_emits_authority_clobber_warning(tmp_path, monkeypatc mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -514,7 +541,11 @@ async def test_open_kitchen_emits_authority_clobber_warning(tmp_path, monkeypatc ) parsed = json.loads(result_str) - warnings = parsed.get("warnings") or [] + assert "content" not in parsed + # The compact envelope does not carry "warnings" — verify it in the persisted + # artifact instead (the full payload passed to persist_recipe_artifact). + artifact = json.loads(Path(parsed["artifact_path"]).read_text()) + warnings = artifact.get("warnings") or [] matching = [w for w in warnings if "base_branch" in w] assert matching, f"Expected a warning naming base_branch; got warnings={warnings}" assert any("branching.default_base_branch" in w for w in warnings), ( @@ -524,7 +555,7 @@ async def test_open_kitchen_emits_authority_clobber_warning(tmp_path, monkeypatc @pytest.mark.anyio -async def test_open_kitchen_with_config_authority_ingredient(monkeypatch): +async def test_open_kitchen_with_config_authority_ingredient(tmp_path, monkeypatch): """Full open_kitchen path: config value wins over caller override in rendered output.""" monkeypatch.delenv("AUTOSKILLIT_HEADLESS", raising=False) import autoskillit.recipe._api_cache as cache_mod @@ -551,6 +582,9 @@ async def test_open_kitchen_with_config_authority_ingredient(monkeypatch): mock_ctx.backend = None mock_ctx.recipes = DefaultRecipeRepository() mock_ctx.config.migration.suppressed = [] + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -1124,6 +1158,9 @@ async def test_pipeline_health_override_wins_over_config(tmp_path, monkeypatch): mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -1184,6 +1221,9 @@ async def test_pipeline_health_config_default_applied(tmp_path, monkeypatch): mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen-abc" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + # Provide a real temp_dir so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): diff --git a/tests/server/test_tools_kitchen_gate_features.py b/tests/server/test_tools_kitchen_gate_features.py index 604c8b9d8..4ac16c63c 100644 --- a/tests/server/test_tools_kitchen_gate_features.py +++ b/tests/server/test_tools_kitchen_gate_features.py @@ -8,7 +8,7 @@ import pytest -from tests.server.conftest import _make_mock_ctx +from tests.server.conftest import _make_mock_ctx, _make_track_response_ready_mock_ctx pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -150,6 +150,9 @@ async def test_open_kitchen_ingredients_only_strips_content(tmp_path, monkeypatc ): from autoskillit.server.tools.tools_kitchen import open_kitchen + # New envelope: open_kitchen persists the full payload to + # temp_dir/responses/. + mock_ctx.temp_dir = tmp_path result_json = await open_kitchen( name="test", ingredients_only=True, ctx=mock_ctx ) @@ -158,14 +161,17 @@ async def test_open_kitchen_ingredients_only_strips_content(tmp_path, monkeypatc assert result["success"] is True assert result["ingredients_table"] is not None assert "content" not in result - assert "orchestration_rules" not in result + # Envelope fields (orchestration_rules, stop_step_semantics) are always + # present in the envelope schema; ingredients_only strips their source + # values before envelope construction, so they surface here as falsy. + assert not result.get("orchestration_rules") assert "sous_chef_discipline" not in result - assert "stop_step_semantics" not in result + assert not result.get("stop_step_semantics") @pytest.mark.anyio async def test_open_kitchen_ingredients_only_preserves_metadata(tmp_path, monkeypatch): - """ingredients_only=True must preserve success, kitchen, version, valid, suggestions.""" + """ingredients_only=True must preserve success, kitchen, version, suggestions.""" monkeypatch.chdir(tmp_path) mock_ctx = _make_mock_ctx() mock_ctx.enable_components = AsyncMock() @@ -194,6 +200,9 @@ async def test_open_kitchen_ingredients_only_preserves_metadata(tmp_path, monkey ): from autoskillit.server.tools.tools_kitchen import open_kitchen + # New envelope: open_kitchen persists the full payload to + # temp_dir/responses/. + mock_ctx.temp_dir = tmp_path result_json = await open_kitchen( name="test", ingredients_only=True, ctx=mock_ctx ) @@ -202,7 +211,6 @@ async def test_open_kitchen_ingredients_only_preserves_metadata(tmp_path, monkey assert result["success"] is True assert result["kitchen"] == "open" assert "version" in result - assert result["valid"] is True assert result["suggestions"] == [] @@ -210,10 +218,13 @@ async def test_open_kitchen_ingredients_only_preserves_metadata(tmp_path, monkey async def test_open_kitchen_ingredients_only_no_name_ignored(tmp_path, monkeypatch): """ingredients_only=True with name=None should behave like regular no-name open.""" monkeypatch.chdir(tmp_path) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -294,6 +305,8 @@ async def test_open_kitchen_uses_project_dir_for_recipe_lookup(tmp_path, monkeyp mock_ctx.active_recipe_packs = frozenset() mock_ctx.active_recipe_features = frozenset() mock_ctx.quota_refresh_task = None + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + mock_ctx.temp_dir = ctx.temp_dir with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): diff --git a/tests/server/test_tools_kitchen_visibility.py b/tests/server/test_tools_kitchen_visibility.py index d62a77c3e..4173fe6a8 100644 --- a/tests/server/test_tools_kitchen_visibility.py +++ b/tests/server/test_tools_kitchen_visibility.py @@ -3,12 +3,13 @@ from __future__ import annotations import json +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest from autoskillit.core.types._type_constants import SOUS_CHEF_MANDATORY_SECTIONS -from tests.server.conftest import _make_mock_ctx +from tests.server.conftest import _make_mock_ctx, _make_track_response_ready_mock_ctx pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -156,10 +157,13 @@ async def test_open_kitchen_includes_categorized_tool_listing(tmp_path, monkeypa from autoskillit.server.tools.tools_kitchen import open_kitchen monkeypatch.chdir(tmp_path) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -205,6 +209,7 @@ async def test_open_kitchen_with_recipe_returns_combined_response(tmp_path, monk } mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -220,8 +225,9 @@ async def test_open_kitchen_with_recipe_returns_combined_response(tmp_path, monk assert result["success"] is True assert result["kitchen"] == "open" assert "version" in result - assert "content" in result - assert "test-recipe" in result["content"] + assert "content" not in result + payload = json.loads(Path(result["artifact_path"]).read_text(encoding="utf-8")) + assert "test-recipe" in payload["content"] mock_ctx.gate.enable.assert_called_once() mock_ctx.enable_components.assert_called_once_with(tags={"kitchen"}) @@ -262,10 +268,13 @@ async def test_open_kitchen_with_recipe_not_found(tmp_path, monkeypatch): async def test_open_kitchen_without_recipe_returns_json_envelope(tmp_path, monkeypatch): """open_kitchen() without name returns JSON envelope with success=True.""" monkeypatch.chdir(tmp_path) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() @@ -420,6 +429,8 @@ async def test_sous_chef_discipline_not_in_open_kitchen_result(tmp_path, monkeyp } mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -457,6 +468,8 @@ async def test_open_kitchen_result_keys_match_typed_dict(tmp_path, monkeypatch): } mock_ctx.recipes.find.return_value = None mock_ctx.config.migration.suppressed = [] + # New envelope: open_kitchen persists the full payload to temp_dir/responses/. + mock_ctx.temp_dir = tmp_path with patch("autoskillit.server._get_ctx", return_value=mock_ctx): with patch("autoskillit.server.logger"): @@ -486,10 +499,13 @@ async def test_open_kitchen_result_keys_match_typed_dict(tmp_path, monkeypatch): async def test_sous_chef_rules_injected_at_open_kitchen(tmp_path, monkeypatch): """Path B (no-name) must inject full sous-chef SKILL.md into response text.""" monkeypatch.chdir(tmp_path) - mock_ctx = _make_mock_ctx() + mock_ctx = _make_track_response_ready_mock_ctx() mock_ctx.enable_components = AsyncMock() - with patch("autoskillit.server._get_ctx", return_value=mock_ctx): + with ( + patch("autoskillit.server._get_ctx", return_value=mock_ctx), + patch("autoskillit.server._notify._get_ctx_or_none", return_value=mock_ctx), + ): with patch("autoskillit.server.logger"): with patch( "autoskillit.server.tools.tools_kitchen._prime_quota_cache", new=AsyncMock() diff --git a/tests/server/test_tools_load_recipe.py b/tests/server/test_tools_load_recipe.py index e5fcf3202..ff3144061 100644 --- a/tests/server/test_tools_load_recipe.py +++ b/tests/server/test_tools_load_recipe.py @@ -67,16 +67,25 @@ def _ensure_ctx(self, tool_ctx_kitchen_open): # SS2 @pytest.mark.anyio async def test_load_returns_json_with_content(self, tmp_path, monkeypatch): - """load_recipe returns JSON with content and suggestions.""" + """load_recipe returns a compact envelope with artifact_path and suggestions.""" monkeypatch.chdir(tmp_path) recipes_dir = tmp_path / ".autoskillit" / "recipes" recipes_dir.mkdir(parents=True) (recipes_dir / "test.yaml").write_text("name: test\ndescription: Test recipe\n") result = json.loads(await load_recipe(name="test")) - assert "content" in result + # Envelope shape: full content is NOT in the response — it is + # retrieved on demand via get_recipe_section(section="content"). + assert "content" not in result + assert "artifact_path" in result + assert "sha256" in result + assert "pull_tool" in result + assert result["pull_tool"] == "get_recipe_section" assert "suggestions" in result - assert "name: test" in result["content"] - assert "description: Test recipe" in result["content"] + # Verify the persisted artifact carries the full recipe content + artifact = Path(result["artifact_path"]).read_text(encoding="utf-8") + payload = json.loads(artifact) + assert "name: test" in payload["content"] + assert "description: Test recipe" in payload["content"] # SS3 @pytest.mark.anyio @@ -90,7 +99,7 @@ async def test_load_unknown_returns_error(self, tmp_path, monkeypatch): # SS7 @pytest.mark.anyio async def test_load_returns_json_with_suggestions(self, tmp_path, monkeypatch): - """load_recipe response always has 'content' and 'suggestions' keys.""" + """load_recipe envelope always has 'suggestions' list with semantic findings.""" monkeypatch.chdir(tmp_path) recipes_dir = tmp_path / ".autoskillit" / "recipes" recipes_dir.mkdir(parents=True) @@ -100,7 +109,7 @@ async def test_load_returns_json_with_suggestions(self, tmp_path, monkeypatch): " on_success: done\n done:\n action: stop\n message: Done\n" ) result = json.loads(await load_recipe(name="test")) - assert "content" in result + assert "content" not in result assert "suggestions" in result assert isinstance(result["suggestions"], list) assert any(s["rule"] == "model-on-non-skill-step" for s in result["suggestions"]) @@ -114,8 +123,13 @@ async def test_load_recipe_mcp_returns_builtin_recipe( monkeypatch.chdir(tmp_path) result = json.loads(await load_recipe(name="implementation")) assert "error" not in result, f"Unexpected error: {result.get('error')}" - assert "content" in result - assert len(result["content"]) > 0 + # New envelope: full content is in artifact_path, not inline + assert "content" not in result + assert "artifact_path" in result + artifact = Path(result["artifact_path"]).read_text(encoding="utf-8") + payload = json.loads(artifact) + assert "content" in payload + assert len(payload["content"]) > 0 @pytest.mark.anyio async def test_load_recipe_parse_failure_is_logged_and_surfaced( @@ -139,7 +153,9 @@ async def test_load_recipe_parse_failure_is_logged_and_surfaced( ): result = json.loads(await load_recipe(name="test")) - assert "content" in result, "load_recipe must be non-blocking even on parse failure" + assert "artifact_path" in result, ( + "load_recipe must persist an artifact and return its path even on parse failure" + ) mock_logger.warning.assert_called_once() assert any(s.get("rule") == "validation-error" for s in result["suggestions"]), ( "Unexpected exception must appear as a validation-error finding in suggestions" @@ -484,20 +500,36 @@ def _setup_project_recipe(self, tmp_path: Path, monkeypatch) -> Path: async def test_load_recipe_response_has_diagram_key( self, tmp_path, monkeypatch, tool_ctx_kitchen_open ): - """DG-12: load_recipe response always contains a 'diagram' key.""" + """DG-12: load_recipe envelope carries diagram field when present.""" self._setup_project_recipe(tmp_path, monkeypatch) result = json.loads(await load_recipe(name="my-recipe")) - assert "diagram" in result + # New envelope: diagram is forwarded only when present and not None; + # canonical path is get_recipe_section(section="diagram"). When no + # diagram exists, the key is absent. + # Verify either the inline diagram key is present OR the artifact_path + # exposes a 'diagram' field via the persisted artifact. + assert "artifact_path" in result + if "diagram" in result: + assert result["diagram"] is None or isinstance(result["diagram"], str) + else: + artifact = Path(result["artifact_path"]).read_text(encoding="utf-8") + payload = json.loads(artifact) + assert "diagram" in payload # DG-13 @pytest.mark.anyio async def test_load_recipe_diagram_none_when_not_generated( self, tmp_path, monkeypatch, tool_ctx_kitchen_open ): - """DG-13: diagram is None when no diagram file exists.""" + """DG-13: persisted artifact's diagram is None when no diagram file exists.""" self._setup_project_recipe(tmp_path, monkeypatch) result = json.loads(await load_recipe(name="my-recipe")) - assert result["diagram"] is None + # Envelope carries diagram only when explicitly present and not None. + # The canonical retrieval path is get_recipe_section(section="diagram"). + assert "diagram" not in result or result.get("diagram") is None + artifact = Path(result["artifact_path"]).read_text(encoding="utf-8") + payload = json.loads(artifact) + assert payload.get("diagram") is None # --------------------------------------------------------------------------- @@ -530,7 +562,7 @@ def _ensure_ctx(self, tool_ctx_kitchen_open): @pytest.mark.anyio async def test_load_recipe_ingredients_only_strips_content(self, tmp_path, monkeypatch): - """load_recipe(name=X, ingredients_only=True) must omit content from result.""" + """load_recipe(name=X, ingredients_only=True) returns a minimal envelope.""" monkeypatch.chdir(tmp_path) recipes_dir = tmp_path / ".autoskillit" / "recipes" recipes_dir.mkdir(parents=True) @@ -539,9 +571,15 @@ async def test_load_recipe_ingredients_only_strips_content(self, tmp_path, monke " done:\n action: stop\n message: Done\n" ) result = json.loads(await load_recipe(name="test", ingredients_only=True)) + # New envelope shape — even when ingredients_only, the envelope fields + # are present (no full content), but step_flow_skeleton is empty. assert "content" not in result - assert "orchestration_rules" not in result - assert "stop_step_semantics" not in result + # Orchestration fields are part of the envelope; when ingredients_only=True, + # they are stripped per the strip_ingredients_only_keys policy before the + # envelope is built, so they surface here as falsy (present but None). + assert not result.get("orchestration_rules") + assert not result.get("stop_step_semantics") + assert result.get("step_flow_skeleton") == [] assert "suggestions" in result @@ -575,6 +613,10 @@ async def test_load_recipe_emits_authority_warning(self, tmp_path, monkeypatch): mock_ctx.config.migration.suppressed = [] mock_ctx.kitchen_id = "test-kitchen" mock_ctx.config.linux_tracing.log_dir = "" + # New envelope: load_recipe persists the full recipe payload to + # tool_ctx.temp_dir/responses/load_recipe/. Provide a real temp_dir + # so artifact_dir.mkdir succeeds under the mock. + mock_ctx.temp_dir = tmp_path with patch( "autoskillit.server.tools.tools_recipe._get_ctx_or_none", @@ -601,7 +643,14 @@ async def test_load_recipe_emits_authority_warning(self, tmp_path, monkeypatch): ) parsed = json.loads(result_str) - warnings = parsed.get("warnings") or [] + # New envelope: warnings live on the persisted artifact (full payload), + # not on the compact envelope. Read the artifact to verify the + # authority-clobber warning is generated and persisted. + artifact_path = parsed.get("artifact_path") + assert artifact_path, f"load_recipe envelope missing artifact_path; got {parsed}" + artifact = Path(artifact_path).read_text(encoding="utf-8") + persisted_payload = json.loads(artifact) + warnings = persisted_payload.get("warnings") or [] matching = [w for w in warnings if "base_branch" in w] assert matching, ( f"load_recipe must emit a warning naming base_branch; got warnings={warnings}" @@ -647,12 +696,19 @@ async def test_load_recipe_surfaces_validation_failure( _LOAD_CACHE.clear() result = json.loads(await load_recipe(name="no-steps")) - assert result.get("valid") is False - assert result.get("validation_failed") is True, ( - f"Expected validation_failed=True in response; got keys: {list(result.keys())}" + # New envelope: validation_failed/valid/errors live in the artifact + # (the compact envelope strips these). Read the artifact to confirm + # the failure was surfaced for callers who fetch the persisted payload. + assert result.get("success") is True + assert "artifact_path" in result + artifact = Path(result["artifact_path"]).read_text(encoding="utf-8") + persisted = json.loads(artifact) + assert persisted.get("valid") is False + assert persisted.get("validation_failed") is True, ( + f"Expected validation_failed=True; got keys: {list(persisted.keys())}" ) - assert "errors" in result - assert len(result["errors"]) > 0 + assert "errors" in persisted + assert len(persisted["errors"]) > 0 class TestLoadRecipeFailClosed: diff --git a/tests/server/test_tools_recipe.py b/tests/server/test_tools_recipe.py index 2b5b80907..e7b643142 100644 --- a/tests/server/test_tools_recipe.py +++ b/tests/server/test_tools_recipe.py @@ -4,6 +4,7 @@ import json import re +from pathlib import Path import pytest @@ -271,6 +272,151 @@ async def test_validate_always_includes_outdated_version(self, tmp_path): assert "outdated-recipe-version" in rules +# --------------------------------------------------------------------------- +# Part A remediation: get_recipe_section pull tool after an open_kitchen-only +# session (Tests A3-A5). +# --------------------------------------------------------------------------- + +_TEST_RECIPE_YAML = """\ +name: test +description: A test recipe +summary: a > b +kitchen_rules: + - test +steps: + do_thing: + tool: run_cmd + with: + cmd: echo hello + cwd: . + on_success: done + on_failure: escalate + done: + action: stop + message: "Done. Emit the L3 result sentinel JSON block now." + escalate: + action: stop + message: "Failed. Emit the L3 result sentinel JSON block now." +""" + + +class TestGetRecipeSection: + """Tests A3-A5: get_recipe_section after an open_kitchen-only session.""" + + @pytest.fixture(autouse=True) + def _ensure_ctx(self, tool_ctx_kitchen_open): + """Ensure server context is initialized with gate open.""" + + async def _open_kitchen_test_recipe( + self, tool_ctx_kitchen_open, tmp_path, monkeypatch + ) -> dict: + """Serve the minimal 'test' recipe via a real in-process open_kitchen call.""" + from unittest.mock import AsyncMock, patch + + from autoskillit.server.tools.tools_kitchen import open_kitchen + + monkeypatch.chdir(tmp_path) + recipes_dir = tmp_path / ".autoskillit" / "recipes" + recipes_dir.mkdir(parents=True) + (recipes_dir / "test.yaml").write_text(_TEST_RECIPE_YAML) + + 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="test", ctx=tool_ctx_kitchen_open) + ) + + # Test A3 + @pytest.mark.anyio + async def test_get_recipe_section_returns_step_content( + self, tool_ctx_kitchen_open, tmp_path, monkeypatch + ): + """Every post-prune step is resolvable via get_recipe_section after open_kitchen-only.""" + from autoskillit.core import load_yaml + from autoskillit.execution import resolve_worst_case_delivery_bound + from autoskillit.server.tools.tools_recipe import get_recipe_section + + result = await self._open_kitchen_test_recipe(tool_ctx_kitchen_open, tmp_path, monkeypatch) + assert result["success"] is True + assert "content" not in result + assert result["pull_tool"] == "get_recipe_section" + assert {"step_flow_skeleton", "step_index", "artifact_path", "sha256"} <= result.keys() + + bound = resolve_worst_case_delivery_bound() * 4 + for step_name in result["step_index"]: + section_str = await get_recipe_section(section="step", step_name=step_name) + section_result = json.loads(section_str) + assert section_result["success"] is True + content = section_result["content"] + assert content + load_yaml(content) + assert len(section_str.encode("utf-8")) <= bound + + # Test A4 + @pytest.mark.anyio + async def test_get_recipe_section_sha256_verification( + self, tool_ctx_kitchen_open, tmp_path, monkeypatch + ): + """A corrupted artifact fails sha256 verification with a structured error.""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + result = await self._open_kitchen_test_recipe(tool_ctx_kitchen_open, tmp_path, monkeypatch) + artifact_path = Path(result["artifact_path"]) + artifact_path.write_text("corrupted", encoding="utf-8") + + section_result = json.loads(await get_recipe_section(section="content")) + assert section_result["error"] == "artifact_integrity_failed" + assert "content" not in section_result + + # Test A5 + @pytest.mark.anyio + async def test_get_recipe_section_missing_artifact_recreates( + self, tool_ctx_kitchen_open, tmp_path, monkeypatch + ): + """A deleted artifact is recreated via serve_recipe() and served successfully.""" + from autoskillit.server.tools.tools_recipe import get_recipe_section + + result = await self._open_kitchen_test_recipe(tool_ctx_kitchen_open, tmp_path, monkeypatch) + artifact_path = Path(result["artifact_path"]) + artifact_path.unlink() + + step_name = next(iter(result["step_index"])) + section_result = json.loads(await get_recipe_section(section="step", step_name=step_name)) + assert section_result["success"] is True + assert section_result["content"] + assert artifact_path.exists() + + # Test A5 (second scenario) + @pytest.mark.anyio + async def test_get_recipe_section_missing_artifact_recreation_failure_is_structured( + self, tool_ctx_kitchen_open, tmp_path, monkeypatch + ): + """When recreation itself raises, the reply is a structured error, never empty content.""" + from unittest.mock import patch + + from autoskillit.server.tools.tools_recipe import get_recipe_section + + result = await self._open_kitchen_test_recipe(tool_ctx_kitchen_open, tmp_path, monkeypatch) + artifact_path = Path(result["artifact_path"]) + artifact_path.unlink() + + step_name = next(iter(result["step_index"])) + with patch( + "autoskillit.server.tools.tools_recipe.serve_recipe", + side_effect=RuntimeError("boom"), + ): + section_result = json.loads( + await get_recipe_section(section="step", step_name=step_name) + ) + assert section_result["error"] == "artifact_unavailable" + assert "content" not in section_result + + class TestDocstringSemantics: """Section-aware semantic checks for tool descriptions. @@ -470,7 +616,6 @@ async def test_list_recipes_mcp_tool_hides_campaign_when_fleet_disabled( tool_ctx, tmp_path, monkeypatch ): """list_recipes MCP tool must exclude campaign recipes when fleet feature is disabled.""" - from pathlib import Path recipe_dir = tmp_path / ".autoskillit" / "recipes" recipe_dir.mkdir(parents=True) diff --git a/tests/server/test_track_response_size.py b/tests/server/test_track_response_size.py index 84d8282c7..32796b8de 100644 --- a/tests/server/test_track_response_size.py +++ b/tests/server/test_track_response_size.py @@ -244,3 +244,54 @@ async def fake_handler(): ) assert event.kwargs["cause"] == "internal_invariant_failed" assert event.kwargs["original_utf8_bytes"] == len(b"small") + + @pytest.mark.anyio + async def test_capability_fail_closed_on_missing_backend(self, tmp_path): + from autoskillit.server._notify import track_response_size + from autoskillit.server._response_budget import RESPONSE_SPILL_METADATA_KEY + + response_log = MagicMock(record=MagicMock(return_value=False)) + ctx = MagicMock( + backend=None, + response_log=response_log, + temp_dir=tmp_path, + config=MagicMock( + mcp_response=MagicMock(alert_threshold_tokens=0), + output_budget=OutputBudgetConfig(), + ), + ) + payload = json.dumps({"success": True, "content": "x" * 80_000}) + + @track_response_size("open_kitchen") + async def fake_handler(): + return payload + + with patch("autoskillit.server._notify._get_ctx_or_none", return_value=ctx): + result = await fake_handler() + + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= 40_000 + data = json.loads(result) + assert data["delivery_bound_spill"] is True + assert data[RESPONSE_SPILL_METADATA_KEY]["reason"] == "delivery_bound" + + @pytest.mark.anyio + async def test_capability_fail_closed_on_missing_ctx(self): + """ctx is None + oversized payload must fail closed, not pass through unbounded.""" + from autoskillit.execution import resolve_worst_case_delivery_bound + from autoskillit.server._notify import track_response_size + + payload = json.dumps({"success": True, "content": "x" * 80_000}) + + @track_response_size("open_kitchen") + async def fake_handler(): + return payload + + with patch("autoskillit.server._notify._get_ctx_or_none", return_value=None): + result = await fake_handler() + + assert isinstance(result, str) + assert len(result.encode("utf-8")) <= resolve_worst_case_delivery_bound() * 4 + data = json.loads(result) + assert data["success"] is False + assert data["error"] == "response_budget_context_unavailable"