diff --git a/docs/glossary.md b/docs/glossary.md index 2967614002..d009c353c5 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -99,8 +99,20 @@ Synonym for the kitchen-tagged MCP tools. Two words, no hyphen. ### kitchen_id -The unique identifier assigned to a kitchen instance (typically a session -UUID). Recorded in pipeline telemetry. +The campaign lineage identifier: the supplied resume campaign ID, otherwise +`AUTOSKILLIT_CAMPAIGN_ID`, otherwise a UUID minted when the kitchen opens. +It stays stable across close and reopen within a campaign and is shared by +the campaign's food trucks. Captures, child environment, hooks, tracker, +session logs, plan-set binding, and response envelopes use this identifier. +Per-activation recipe state uses [kitchen_activation_id](#kitchen_activation_id). + +### kitchen_activation_id + +The identity minted fresh on each `CLOSED -> INFRASTRUCTURE_READY` transition, +never reused, and carried unchanged when an already-open kitchen receives new +arguments. It keys recipe artifacts and their tombstones, the recipe generation +store, and the page-plan cache. [kitchen_id](#kitchen_id) retains the campaign +lineage across these activations. ### L0 diff --git a/scripts/e2e/recipes/sandbox-smoke.yaml b/scripts/e2e/recipes/sandbox-smoke.yaml index eeaa735c73..8aab0903d3 100644 --- a/scripts/e2e/recipes/sandbox-smoke.yaml +++ b/scripts/e2e/recipes/sandbox-smoke.yaml @@ -87,6 +87,7 @@ steps: `smoke: add sandbox canary`. Do not run a test command or call tools that dispatch work to another worker. cwd: "${{ context.worktree_path }}" + output_dir: "${{ context.worktree_path }}" step_name: implement on_success: test on_failure: failed @@ -120,14 +121,14 @@ steps: tool: run_cmd with: cmd: >- - body_file="{{AUTOSKILLIT_TEMP}}/sandbox-smoke-pr-body.md" && mkdir -p "{{AUTOSKILLIT_TEMP}}" && - printf '%s\n' 'Headless sandbox smoke canary.' > "$body_file" && + printf '%s\n' 'Headless sandbox smoke canary.' > + "{{AUTOSKILLIT_TEMP}}/sandbox-smoke-pr-body.md" && gh pr create --repo "${{ inputs.repository }}" --head "${{ context.branch_name }}" --base "${{ inputs.base_branch }}" --title 'Automated smoke test (DO NOT MERGE)' - --body-file "$body_file" + --body-file "{{AUTOSKILLIT_TEMP}}/sandbox-smoke-pr-body.md" cwd: "${{ context.worktree_path }}" step_name: create_pull_request on_success: close_pull_request diff --git a/src/autoskillit/pipeline/__init__.py b/src/autoskillit/pipeline/__init__.py index 511f325729..8fc63423c5 100644 --- a/src/autoskillit/pipeline/__init__.py +++ b/src/autoskillit/pipeline/__init__.py @@ -60,6 +60,7 @@ from autoskillit.pipeline.kitchen_transition import ( KITCHEN_EFFECT_RECIPE_SERVING, KITCHEN_EFFECT_RESPONSE_ENFORCEMENT, + KitchenActivationId, KitchenEffectPhase, KitchenEffectRecord, KitchenIntentConflict, @@ -81,6 +82,7 @@ new_kitchen_open_state, prepare_kitchen_response, release_kitchen_request, + require_kitchen_activation_id, start_kitchen_effect, transition_abort, transition_ambiguous, @@ -174,6 +176,7 @@ # kitchen transition "KITCHEN_EFFECT_RECIPE_SERVING", "KITCHEN_EFFECT_RESPONSE_ENFORCEMENT", + "KitchenActivationId", "KitchenEffectPhase", "KitchenEffectRecord", "KitchenIntentConflict", @@ -197,6 +200,7 @@ "mark_kitchen_effect_ambiguous", "mark_kitchen_effect_degraded", "new_kitchen_open_state", + "require_kitchen_activation_id", "prepare_kitchen_response", "release_kitchen_request", "start_kitchen_effect", diff --git a/src/autoskillit/pipeline/context.py b/src/autoskillit/pipeline/context.py index ae6397a3c1..2c8ee835a2 100644 --- a/src/autoskillit/pipeline/context.py +++ b/src/autoskillit/pipeline/context.py @@ -83,6 +83,7 @@ ) from autoskillit.pipeline.background import DefaultBackgroundSupervisor from autoskillit.pipeline.kitchen_transition import ( + KitchenActivationId, KitchenOpenState, closed_kitchen_open_state, ) @@ -198,8 +199,10 @@ class ToolContext: context accounting and recovery service. workspace_outcome_ledger: WorkspaceOutcomeLedger — durable shared authority for terminal workspace outcomes across parent and child sessions. - kitchen_id: Read-only identity derived from kitchen_open_state; set only through - establish_kitchen_identity and empty while the state is CLOSED. + kitchen_id: Read-only campaign-lineage identity derived from kitchen_open_state; + set only through establish_kitchen_identity and empty while CLOSED. + kitchen_activation_id: Read-only per-activation identity derived from kitchen_open_state; + raises while the state is CLOSED. kitchen_open_state: Immutable process-local open-operation lifecycle and effect journal retained for the ToolContext lifetime. Fleet dispatch provenance instead uses request/task-scoped ContextVars; the different owners @@ -414,6 +417,14 @@ def kitchen_id(self) -> str: """Return the identity derived from the current lifecycle state.""" return self.kitchen_open_state.kitchen_id + @property + def kitchen_activation_id(self) -> KitchenActivationId: + """Return the live activation identity; raise when none is live.""" + activation_id = self.kitchen_open_state.activation_id + if activation_id is None: + raise RuntimeError("no kitchen activation is live") + return activation_id + @property def host_client_attestation(self) -> HostClientAttestation | None: """Return the launcher attestation snapshot owned by this context.""" diff --git a/src/autoskillit/pipeline/kitchen_transition.py b/src/autoskillit/pipeline/kitchen_transition.py index 6193ddb668..23cb8e0c4f 100644 --- a/src/autoskillit/pipeline/kitchen_transition.py +++ b/src/autoskillit/pipeline/kitchen_transition.py @@ -18,6 +18,7 @@ "KITCHEN_EFFECT_RESPONSE_ENFORCEMENT", "KitchenEffectPhase", "KitchenEffectRecord", + "KitchenActivationId", "KitchenIntentConflict", "KitchenOpenPhase", "KitchenOpenState", @@ -35,6 +36,7 @@ "mark_kitchen_effect_ambiguous", "mark_kitchen_effect_degraded", "new_kitchen_open_state", + "require_kitchen_activation_id", "prepare_kitchen_response", "release_kitchen_request", "start_kitchen_effect", @@ -48,6 +50,24 @@ KITCHEN_EFFECT_RESPONSE_ENFORCEMENT = "response_enforcement" +@dataclass(frozen=True, slots=True) +class KitchenActivationId: + """Identity of one CLOSED -> INFRASTRUCTURE_READY activation. Never reused.""" + + value: str + + def __post_init__(self) -> None: + if not isinstance(self.value, str) or not self.value.strip(): + raise ValueError("KitchenActivationId.value must contain non-whitespace characters") + + +def require_kitchen_activation_id(value: object) -> KitchenActivationId: + """Return ``value`` when it is an activation identity; reject anything else.""" + if not isinstance(value, KitchenActivationId): + raise TypeError("a KitchenActivationId is required") + return value + + class KitchenOpenPhase(StrEnum): """Authoritative lifecycle phases for one process-local open operation.""" @@ -107,6 +127,7 @@ class KitchenOpenState: phase: KitchenOpenPhase kitchen_id: str + activation_id: KitchenActivationId | None operation_id: str context_id: str intent_fingerprint: str | None = None @@ -156,6 +177,7 @@ def closed_kitchen_open_state(*, context_id: str | None = None) -> KitchenOpenSt return KitchenOpenState( phase=KitchenOpenPhase.CLOSED, kitchen_id="", + activation_id=None, operation_id="", context_id=(context_id if isinstance(context_id, str) and context_id else str(uuid4())), ) @@ -164,15 +186,18 @@ def closed_kitchen_open_state(*, context_id: str | None = None) -> KitchenOpenSt def new_kitchen_open_state( *, kitchen_id: str, + activation_id: KitchenActivationId, context_id: str, operation_id: str | None = None, ) -> KitchenOpenState: """Mint the infrastructure transition before request arguments are bound.""" if not kitchen_id: raise ValueError("kitchen_id must be non-empty") + activation_id = require_kitchen_activation_id(activation_id) return KitchenOpenState( phase=KitchenOpenPhase.INFRASTRUCTURE_READY, kitchen_id=kitchen_id, + activation_id=activation_id, operation_id=operation_id or str(uuid4()), context_id=context_id, ) diff --git a/src/autoskillit/server/lifecycle/AGENTS.md b/src/autoskillit/server/lifecycle/AGENTS.md index 230ac4f370..1e87314fbd 100644 --- a/src/autoskillit/server/lifecycle/AGENTS.md +++ b/src/autoskillit/server/lifecycle/AGENTS.md @@ -17,10 +17,12 @@ boot sequence — the async context manager wired via `lifespan=`, per-session-t auto-gate boots, and one-shot startup checks — lives in the `_lifespan/` sub-package. -`_kitchen_identity` is the sole kitchen-identity mint and activation authority. -This is application lifecycle state: `ToolContext.kitchen_id` is derived from -`kitchen_open_state`, while the remaining context fields stay mutable. It is -separate from FastMCP transport/session identity. +`_kitchen_identity` is the sole kitchen-identity mint, activation, and retirement +authority. `ToolContext.kitchen_id` (campaign lineage) and +`ToolContext.kitchen_activation_id` (per activation) are both derived from +`kitchen_open_state`. Per-activation recipe state keys on the activation ID, +never on the kitchen ID. This application lifecycle state is separate from +FastMCP transport/session identity. The pre-deletion editable-install scan that halts `perform_merge()` before a worktree is deleted lives at `server/_editable_guard.py`, one level up, not diff --git a/src/autoskillit/server/lifecycle/_kitchen_identity.py b/src/autoskillit/server/lifecycle/_kitchen_identity.py index 82b9e80ddc..98895c4756 100644 --- a/src/autoskillit/server/lifecycle/_kitchen_identity.py +++ b/src/autoskillit/server/lifecycle/_kitchen_identity.py @@ -1,11 +1,20 @@ -"""Sole owner of the kitchen-identity CLOSED -> INFRASTRUCTURE_READY edge.""" +"""Sole owner of the kitchen-identity lifecycle edges and activation retirement.""" -from autoskillit.core import resolve_kitchen_id +from pathlib import Path +from uuid import uuid4 + +from autoskillit.core import get_logger, resolve_kitchen_id from autoskillit.pipeline import ( + KitchenActivationId, KitchenOpenPhase, ToolContext, + closed_kitchen_open_state, new_kitchen_open_state, ) +from autoskillit.server.recipe import _recipe_artifact, _recipe_generation +from autoskillit.server.recipe.section import _lifecycle + +logger = get_logger(__name__) def establish_kitchen_identity(ctx: ToolContext, *, campaign_id: str | None = None) -> str: @@ -16,18 +25,46 @@ def establish_kitchen_identity(ctx: ToolContext, *, campaign_id: str | None = No with ctx.kitchen_transition_lock: state = ctx.kitchen_open_state if state.phase is KitchenOpenPhase.CLOSED: - from autoskillit.server.recipe._recipe_generation import ( # circular-break - activate_kitchen, - ) - kitchen_id = campaign_id or resolve_kitchen_id() if not kitchen_id.strip(): raise ValueError("kitchen_id must contain non-whitespace characters") - activate_kitchen(kitchen_id) + activation_id = KitchenActivationId(uuid4().hex) + _recipe_generation.activate_activation(activation_id) ctx.kitchen_process_identity = None state = new_kitchen_open_state( kitchen_id=kitchen_id, + activation_id=activation_id, context_id=state.context_id, ) ctx.kitchen_open_state = state return state.kitchen_id + + +def retire_kitchen_identity(ctx: ToolContext) -> None: + """Return the lifecycle to CLOSED, then permanently retire its activation.""" + with ctx.kitchen_transition_lock: + state = ctx.kitchen_open_state + ctx.kitchen_open_state = closed_kitchen_open_state(context_id=state.context_id) + activation_id = state.activation_id + if not isinstance(activation_id, KitchenActivationId): + return + if isinstance(ctx.temp_dir, Path): + try: + if _recipe_artifact.retire_recipe_artifacts(ctx.temp_dir, activation_id=activation_id): + return + except Exception: + try: + _lifecycle.notify_activation_retired(activation_id) + except Exception: + logger.warning( + "kitchen_activation_retirement_notification_failed", + activation_id=activation_id.value, + exc_info=True, + ) + raise + logger.warning( + "kitchen_activation_artifact_retirement_failed", + kitchen_id=state.kitchen_id, + activation_id=activation_id.value, + ) + _lifecycle.notify_activation_retired(activation_id) diff --git a/src/autoskillit/server/recipe/_recipe_artifact.py b/src/autoskillit/server/recipe/_recipe_artifact.py index cd4b9dc110..63ffe83d9a 100644 --- a/src/autoskillit/server/recipe/_recipe_artifact.py +++ b/src/autoskillit/server/recipe/_recipe_artifact.py @@ -34,13 +34,14 @@ load_yaml, validate_recipe_artifact_sections, ) +from autoskillit.pipeline import KitchenActivationId, require_kitchen_activation_id from autoskillit.server.recipe._recipe_generation import ( RecipeGenerationError, RecipeGenerationRecord, generation_json_primitive, get_recipe_generation_store, ) -from autoskillit.server.recipe.section._lifecycle import notify_kitchen_retired +from autoskillit.server.recipe.section._lifecycle import notify_activation_retired if TYPE_CHECKING: from autoskillit.pipeline import ToolContext @@ -307,7 +308,8 @@ def prepare_recipe_delivery_generation( flow_generation=flow_generation, ) store = get_recipe_generation_store() - existing = store.lookup_compile(tool_ctx.kitchen_id, normalized_compile_key) + activation_id = tool_ctx.kitchen_activation_id + existing = store.lookup_compile(activation_id, normalized_compile_key) if existing is not None: if ( existing.recipe_name != recipe_name @@ -366,7 +368,7 @@ def prepare_recipe_delivery_generation( ) admitted = store.put( RecipeGenerationRecord( - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, normalized_compile_key=normalized_compile_key, recipe_name=recipe_name, finalized_projection=finalized_projection, @@ -459,10 +461,14 @@ def _artifact_root(temp_dir: Path) -> Path: return temp_dir / "recipe-delivery" +def _namespace_component(activation_id: KitchenActivationId) -> str: + return _safe_component(activation_id.value) + + def _generation_dir( temp_dir: Path, *, - kitchen_id: str, + activation_id: KitchenActivationId, producer_tool: str, recipe_name: str, descriptor_version: int, @@ -471,7 +477,7 @@ def _generation_dir( ) -> Path: return ( _artifact_root(temp_dir) - / _safe_component(kitchen_id) + / _namespace_component(activation_id) / _safe_component(producer_tool) / _safe_component(recipe_name) / f"descriptor-{descriptor_version}-schema-{schema_version}" @@ -479,8 +485,8 @@ def _generation_dir( ) -def _retired_namespace_marker(temp_dir: Path, *, kitchen_id: str) -> Path: - return _artifact_root(temp_dir) / ".retired" / f"{_safe_component(kitchen_id)}.retired" +def _retired_namespace_marker(temp_dir: Path, *, activation_id: KitchenActivationId) -> Path: + return _artifact_root(temp_dir) / ".retired" / f"{_namespace_component(activation_id)}.retired" @contextmanager @@ -544,13 +550,14 @@ def _generation_from_payload( def persist_recipe_artifact( temp_dir: Path, *, - kitchen_id: str, + activation_id: KitchenActivationId, producer_tool: str, recipe_name: str, payload: dict[str, Any], flow_generation: RecipeFlowGeneration | None = None, ) -> RecipeArtifactGeneration: """Publish an immutable canonical payload and its generation descriptor.""" + require_kitchen_activation_id(activation_id) _validate_recipe_artifact_schema(payload) canonical_flow_generation = _flow_generation_from_payload(payload) if flow_generation is not None and canonical_flow_generation != flow_generation: @@ -568,7 +575,7 @@ def persist_recipe_artifact( ) directory = _generation_dir( temp_dir, - kitchen_id=kitchen_id, + activation_id=activation_id, producer_tool=producer_tool, recipe_name=recipe_name, descriptor_version=generation.descriptor_version, @@ -577,7 +584,7 @@ def persist_recipe_artifact( ) descriptor = _canonical_payload(generation.pull_identity()) with _generation_lock(temp_dir, exclusive=True): - if _retired_namespace_marker(temp_dir, kitchen_id=kitchen_id).exists(): + if _retired_namespace_marker(temp_dir, activation_id=activation_id).exists(): raise RecipeArtifactError("recipe artifact namespace is retired") directory.mkdir(parents=True, exist_ok=True) blob_path = directory / "payload.json" @@ -608,15 +615,16 @@ def persist_recipe_artifact( def load_recipe_artifact( temp_dir: Path, *, - kitchen_id: str, + activation_id: KitchenActivationId, identity: RecipeArtifactGeneration, ) -> dict[str, Any]: """Read and independently verify an exact immutable payload generation.""" + require_kitchen_activation_id(activation_id) if not identity.has_valid_read_bounds(): raise RecipeArtifactError("invalid recipe artifact identity bounds") directory = _generation_dir( temp_dir, - kitchen_id=kitchen_id, + activation_id=activation_id, producer_tool=identity.producer_tool, recipe_name=identity.recipe_name, descriptor_version=identity.descriptor_version, @@ -667,11 +675,12 @@ def load_recipe_artifact( return payload -def retire_recipe_artifacts(temp_dir: Path, *, kitchen_id: str) -> bool: - """Retire one kitchen namespace after all shared-lock readers finish.""" +def retire_recipe_artifacts(temp_dir: Path, *, activation_id: KitchenActivationId) -> bool: + """Retire one activation namespace after all shared-lock readers finish.""" + require_kitchen_activation_id(activation_id) try: - namespace = _artifact_root(temp_dir) / _safe_component(kitchen_id) - retired_marker = _retired_namespace_marker(temp_dir, kitchen_id=kitchen_id) + namespace = _artifact_root(temp_dir) / _namespace_component(activation_id) + retired_marker = _retired_namespace_marker(temp_dir, activation_id=activation_id) with _generation_lock(temp_dir, exclusive=True): retired_marker.parent.mkdir(parents=True, exist_ok=True) atomic_write(retired_marker, "retired\n") @@ -680,11 +689,11 @@ def retire_recipe_artifacts(temp_dir: Path, *, kitchen_id: str) -> bool: except (OSError, RecipeArtifactError, TypeError): return False try: - notify_kitchen_retired(kitchen_id) + notify_activation_retired(activation_id) except Exception: get_logger(__name__).warning( "recipe_section_cache_retirement_eviction_failed", - kitchen_id=kitchen_id, + activation_id=activation_id.value, exc_info=True, ) return True diff --git a/src/autoskillit/server/recipe/_recipe_delivery/__init__.py b/src/autoskillit/server/recipe/_recipe_delivery/__init__.py index eb1fd9aa9d..edcf583f13 100644 --- a/src/autoskillit/server/recipe/_recipe_delivery/__init__.py +++ b/src/autoskillit/server/recipe/_recipe_delivery/__init__.py @@ -32,7 +32,6 @@ prepare_recipe_delivery_generation, recipe_pull_producers, recipe_recreation_producers, - retire_recipe_artifacts, ) from autoskillit.server.recipe._recipe_delivery._completion import ( complete_finalized_recipe_response, @@ -76,7 +75,6 @@ "prepare_recipe_delivery_generation", "recipe_pull_producers", "recipe_recreation_producers", - "retire_recipe_artifacts", "validate_compiled_recipe_delivery_budget", "validate_recipe_exemption_fitness", ] diff --git a/src/autoskillit/server/recipe/_recipe_delivery/_finalize.py b/src/autoskillit/server/recipe/_recipe_delivery/_finalize.py index 40d2ae7d9f..c061f5c377 100644 --- a/src/autoskillit/server/recipe/_recipe_delivery/_finalize.py +++ b/src/autoskillit/server/recipe/_recipe_delivery/_finalize.py @@ -202,17 +202,18 @@ def finalize_recipe_delivery( surface_payload[generation_field] = candidate_payload[generation_field] initialization_id = uuid4().hex if surface_definition.initialization_activating else None active_requirements: tuple[RecipeInitializationRequirement, ...] | None = None + activation_id = tool_ctx.kitchen_activation_id try: generation = persist_recipe_artifact( tool_ctx.temp_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, producer_tool=surface_definition.producer_tool, recipe_name=recipe_name, payload=candidate_payload, flow_generation=flow_generation, ) get_recipe_generation_store().bind_surface( - tool_ctx.kitchen_id, + activation_id, normalized_compile_key, surface, generation, @@ -223,7 +224,15 @@ def finalize_recipe_delivery( RecipeGenerationError, TypeError, ValueError, - ): + ) as exc: + get_logger(__name__).warning( + "recipe_artifact_persistence_failed", + producer=surface_definition.producer_tool, + recipe_name=recipe_name, + surface=surface, + error_type=type(exc).__name__, + exc_info=True, + ) decision = _failure_decision( producer=surface_definition.producer_tool, reason="recipe_artifact_persistence_failed", diff --git a/src/autoskillit/server/recipe/_recipe_delivery_helpers.py b/src/autoskillit/server/recipe/_recipe_delivery_helpers.py index 76e2564875..93f8c4be65 100644 --- a/src/autoskillit/server/recipe/_recipe_delivery_helpers.py +++ b/src/autoskillit/server/recipe/_recipe_delivery_helpers.py @@ -97,8 +97,9 @@ def _entrypoint_content(step_name: str) -> str: raise RecipeArtifactSchemaError( f"required recipe initialization section is absent: {section}" ) + activation_id = tool_ctx.kitchen_activation_id page_plan = get_or_build_recipe_section_page_plan( - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, generation=generation, selected=selected, recipe_section_bound_bytes=bound_bytes, diff --git a/src/autoskillit/server/recipe/_recipe_generation.py b/src/autoskillit/server/recipe/_recipe_generation.py index e25fbb574f..44f5db1774 100644 --- a/src/autoskillit/server/recipe/_recipe_generation.py +++ b/src/autoskillit/server/recipe/_recipe_generation.py @@ -1,4 +1,4 @@ -"""Kitchen-scoped ownership of compiled and persisted recipe generations.""" +"""Ownership of compiled and persisted recipe generations by activation.""" from __future__ import annotations @@ -18,30 +18,34 @@ RecipeExecutionSnapshot, RecipeFlowGeneration, ) +from autoskillit.pipeline import ( + KitchenActivationId, + require_kitchen_activation_id, +) from autoskillit.server.recipe.section._lifecycle import ( - register_kitchen_retirement_callback, + register_activation_retirement_callback, ) RECIPE_GENERATION_STORE_MAX_ENTRIES = 8 RECIPE_GENERATION_STORE_MAX_BYTES = 32 * 1024 * 1024 -RECIPE_GENERATION_STORE_MAX_RETIRED_KITCHENS = 1_024 +RECIPE_GENERATION_STORE_MAX_RETIRED_ACTIVATIONS = 1_024 __all__ = [ "RECIPE_GENERATION_STORE_MAX_BYTES", "RECIPE_GENERATION_STORE_MAX_ENTRIES", - "RECIPE_GENERATION_STORE_MAX_RETIRED_KITCHENS", + "RECIPE_GENERATION_STORE_MAX_RETIRED_ACTIVATIONS", "RecipeGenerationCapacityError", "RecipeGenerationConflictError", "RecipeGenerationError", - "RecipeGenerationKitchenNotActiveError", + "RecipeGenerationActivationNotActiveError", "RecipeGenerationRecord", "RecipeGenerationRetiredError", "RecipeGenerationStore", - "activate_kitchen", + "activate_activation", "generation_json_primitive", "get_recipe_generation_store", "recipe_generation_weight_bytes", - "retire_kitchen", + "retire_activation", "thaw_recipe_generation_mapping", ] @@ -59,11 +63,11 @@ class RecipeGenerationCapacityError(RecipeGenerationError): class RecipeGenerationRetiredError(RecipeGenerationError): - """A write targeted a kitchen whose generation namespace is retired.""" + """A write targeted an activation whose generation namespace is retired.""" -class RecipeGenerationKitchenNotActiveError(RecipeGenerationError): - """A write targeted an inactive kitchen without retained retirement evidence.""" +class RecipeGenerationActivationNotActiveError(RecipeGenerationError): + """A write targeted an inactive activation without retained retirement evidence.""" def _freeze_primitive(value: object, *, path: str) -> object: @@ -139,7 +143,7 @@ def _canonical_json(value: object) -> str: class RecipeGenerationRecord: """One immutable compile generation and its exact persisted surface bindings.""" - kitchen_id: str + activation_id: KitchenActivationId normalized_compile_key: str recipe_name: str finalized_projection: FinalizedRecipeProjection @@ -151,7 +155,8 @@ class RecipeGenerationRecord: surface_bindings: Mapping[str, RecipeArtifactGeneration] = field(default_factory=dict) def __post_init__(self) -> None: - for field_name in ("kitchen_id", "normalized_compile_key", "recipe_name", "execution_id"): + require_kitchen_activation_id(self.activation_id) + for field_name in ("normalized_compile_key", "recipe_name", "execution_id"): value = getattr(self, field_name) if not isinstance(value, str) or not value: raise ValueError(f"RecipeGenerationRecord.{field_name} must be a non-empty string") @@ -232,7 +237,7 @@ def _same_compile_generation( right: RecipeGenerationRecord, ) -> bool: return ( - left.kitchen_id == right.kitchen_id + left.activation_id == right.activation_id and left.normalized_compile_key == right.normalized_compile_key and left.recipe_name == right.recipe_name and left.finalized_projection == right.finalized_projection @@ -244,18 +249,17 @@ def _same_compile_generation( ) -_CompileIndexKey = tuple[str, str] -_ArtifactIndexKey = tuple[str, RecipeArtifactGeneration] +_CompileIndexKey = tuple[KitchenActivationId, str] +_ArtifactIndexKey = tuple[KitchenActivationId, RecipeArtifactGeneration] def _validate_and_extract_compile_key( - kitchen_id: str, + activation_id: KitchenActivationId, normalized_compile_key: str, surface: str, generation: RecipeArtifactGeneration, -) -> tuple[str, str]: - if not isinstance(kitchen_id, str): - raise TypeError("kitchen_id must be a string") +) -> _CompileIndexKey: + require_kitchen_activation_id(activation_id) for name, value in ( ("normalized_compile_key", normalized_compile_key), ("surface", surface), @@ -264,7 +268,7 @@ def _validate_and_extract_compile_key( raise ValueError(f"{name} must be a non-empty string") if not isinstance(generation, RecipeArtifactGeneration): raise TypeError("generation must be a RecipeArtifactGeneration") - return (kitchen_id, normalized_compile_key) + return (activation_id, normalized_compile_key) class RecipeGenerationStore: @@ -275,22 +279,22 @@ def __init__( *, max_entries: int = RECIPE_GENERATION_STORE_MAX_ENTRIES, max_bytes: int = RECIPE_GENERATION_STORE_MAX_BYTES, - max_retired_kitchens: int = RECIPE_GENERATION_STORE_MAX_RETIRED_KITCHENS, + max_retired_activations: int = RECIPE_GENERATION_STORE_MAX_RETIRED_ACTIVATIONS, ) -> None: if max_entries < 0: raise ValueError("generation store max_entries must not be negative") if max_bytes < 0: raise ValueError("generation store max_bytes must not be negative") - if max_retired_kitchens < 0: - raise ValueError("generation store max_retired_kitchens must not be negative") + if max_retired_activations < 0: + raise ValueError("generation store max_retired_activations must not be negative") self._max_entries = max_entries self._max_bytes = max_bytes - self._max_retired_kitchens = max_retired_kitchens + self._max_retired_activations = max_retired_activations self._compile_index: OrderedDict[_CompileIndexKey, RecipeGenerationRecord] = OrderedDict() self._artifact_index: dict[_ArtifactIndexKey, _CompileIndexKey] = {} self._weight_bytes = 0 - self._retired_kitchens: OrderedDict[str, None] = OrderedDict() - self._active_kitchens: set[str] = set() + self._retired_activations: OrderedDict[KitchenActivationId, None] = OrderedDict() + self._active_activations: set[KitchenActivationId] = set() self._lifecycle_authoritative = False self._lock = RLock() @@ -305,28 +309,26 @@ def weight_bytes(self) -> int: return self._weight_bytes @property - def retired_kitchen_count(self) -> int: + def retired_activation_count(self) -> int: with self._lock: - return len(self._retired_kitchens) + return len(self._retired_activations) - def activate_kitchen(self, kitchen_id: str) -> None: - """Mark one current lifecycle identity active without retaining history.""" - if not isinstance(kitchen_id, str) or not kitchen_id: - raise ValueError("kitchen_id must be a non-empty string") + def activate_activation(self, activation_id: KitchenActivationId) -> None: + """Mark one new lifecycle activation active without clearing retirement history.""" + require_kitchen_activation_id(activation_id) with self._lock: self._lifecycle_authoritative = True - self._active_kitchens.add(kitchen_id) - self._retired_kitchens.pop(kitchen_id, None) + self._active_activations.add(activation_id) - def _require_active_locked(self, kitchen_id: str) -> None: - if kitchen_id in self._retired_kitchens: + def _require_active_locked(self, activation_id: KitchenActivationId) -> None: + if activation_id in self._retired_activations: raise RecipeGenerationRetiredError( - f"recipe generation kitchen is retired: {kitchen_id}" + f"recipe generation activation is retired: {activation_id.value}" ) - if self._lifecycle_authoritative and kitchen_id not in self._active_kitchens: - raise RecipeGenerationKitchenNotActiveError( - "recipe generation kitchen is not active in this process: " - f"{kitchen_id} ({len(self._active_kitchens)} active)" + if self._lifecycle_authoritative and activation_id not in self._active_activations: + raise RecipeGenerationActivationNotActiveError( + "recipe generation activation is not active in this process: " + f"{activation_id.value} ({len(self._active_activations)} active)" ) def _deindex_artifacts_locked( @@ -335,7 +337,7 @@ def _deindex_artifacts_locked( record: RecipeGenerationRecord, ) -> None: for generation in record.surface_bindings.values(): - artifact_key = (record.kitchen_id, generation) + artifact_key = (record.activation_id, generation) if self._artifact_index.get(artifact_key) == key: self._artifact_index.pop(artifact_key) @@ -350,7 +352,7 @@ def _validate_artifact_owners_locked( record: RecipeGenerationRecord, ) -> None: for generation in record.surface_bindings.values(): - owner = self._artifact_index.get((record.kitchen_id, generation)) + owner = self._artifact_index.get((record.activation_id, generation)) if owner is not None and owner != key: raise RecipeGenerationConflictError( "exact artifact generation is already bound to another compile generation" @@ -368,7 +370,7 @@ def _replace_locked( self._compile_index[key] = record self._weight_bytes += weight for generation in record.surface_bindings.values(): - self._artifact_index[(record.kitchen_id, generation)] = key + self._artifact_index[(record.activation_id, generation)] = key while len(self._compile_index) > self._max_entries or self._weight_bytes > self._max_bytes: oldest_key = next(iter(self._compile_index)) self._remove_locked(oldest_key) @@ -378,9 +380,9 @@ def put(self, record: RecipeGenerationRecord) -> RecipeGenerationRecord: if not isinstance(record, RecipeGenerationRecord): raise TypeError("record must be a RecipeGenerationRecord") candidate = _copy_record(record) - key = (candidate.kitchen_id, candidate.normalized_compile_key) + key = (candidate.activation_id, candidate.normalized_compile_key) with self._lock: - self._require_active_locked(candidate.kitchen_id) + self._require_active_locked(candidate.activation_id) existing = self._compile_index.get(key) if existing is not None: if not _same_compile_generation(existing, candidate): @@ -407,20 +409,20 @@ def put(self, record: RecipeGenerationRecord) -> RecipeGenerationRecord: def bind_surface( self, - kitchen_id: str, + activation_id: KitchenActivationId, normalized_compile_key: str, surface: str, generation: RecipeArtifactGeneration, ) -> RecipeGenerationRecord: """Atomically bind one surface to an exact persisted generation.""" key = _validate_and_extract_compile_key( - kitchen_id, normalized_compile_key, surface, generation + activation_id, normalized_compile_key, surface, generation ) with self._lock: - self._require_active_locked(kitchen_id) + self._require_active_locked(activation_id) existing = self._compile_index.get(key) if existing is None: - raise KeyError("normalized compile generation is not present for this kitchen") + raise KeyError("normalized compile generation is not present for this activation") if generation.recipe_name != existing.recipe_name: raise RecipeGenerationConflictError( "artifact generation recipe_name does not match compile generation" @@ -433,7 +435,7 @@ def bind_surface( ) self._compile_index.move_to_end(key) return _copy_record(existing) - owner = self._artifact_index.get((kitchen_id, generation)) + owner = self._artifact_index.get((activation_id, generation)) if owner is not None and owner != key: raise RecipeGenerationConflictError( "exact artifact generation is already bound to another compile generation" @@ -452,11 +454,12 @@ def bind_surface( def lookup_compile( self, - kitchen_id: str, + activation_id: KitchenActivationId, normalized_compile_key: str, ) -> RecipeGenerationRecord | None: """Return an immutable defensive copy for a normalized compile key.""" - key = (kitchen_id, normalized_compile_key) + require_kitchen_activation_id(activation_id) + key = (activation_id, normalized_compile_key) with self._lock: record = self._compile_index.get(key) if record is None: @@ -466,12 +469,13 @@ def lookup_compile( def lookup_artifact( self, - kitchen_id: str, + activation_id: KitchenActivationId, generation: RecipeArtifactGeneration, ) -> RecipeGenerationRecord | None: """Return the compile record owning one exact artifact descriptor.""" + require_kitchen_activation_id(activation_id) with self._lock: - key = self._artifact_index.get((kitchen_id, generation)) + key = self._artifact_index.get((activation_id, generation)) if key is None: return None record = self._compile_index.get(key) @@ -480,18 +484,17 @@ def lookup_artifact( self._compile_index.move_to_end(key) return _copy_record(record) - def retire_kitchen(self, kitchen_id: str) -> None: - """Permanently reject writes and remove all entries for one kitchen.""" - if not isinstance(kitchen_id, str) or not kitchen_id: - raise ValueError("kitchen_id must be a non-empty string") + def retire_activation(self, activation_id: KitchenActivationId) -> None: + """Remove entries and reject writes while retirement evidence is retained.""" + require_kitchen_activation_id(activation_id) with self._lock: - self._active_kitchens.discard(kitchen_id) - if self._max_retired_kitchens > 0: - self._retired_kitchens[kitchen_id] = None - self._retired_kitchens.move_to_end(kitchen_id) - while len(self._retired_kitchens) > self._max_retired_kitchens: - self._retired_kitchens.popitem(last=False) - keys = [key for key in self._compile_index if key[0] == kitchen_id] + self._active_activations.discard(activation_id) + if self._max_retired_activations > 0: + self._retired_activations[activation_id] = None + self._retired_activations.move_to_end(activation_id) + while len(self._retired_activations) > self._max_retired_activations: + self._retired_activations.popitem(last=False) + keys = [key for key in self._compile_index if key[0] == activation_id] for key in keys: self._remove_locked(key) @@ -501,8 +504,8 @@ def clear(self) -> None: self._compile_index.clear() self._artifact_index.clear() self._weight_bytes = 0 - self._retired_kitchens.clear() - self._active_kitchens.clear() + self._retired_activations.clear() + self._active_activations.clear() self._lifecycle_authoritative = False @@ -519,14 +522,14 @@ def get_recipe_generation_store() -> RecipeGenerationStore: return _RECIPE_GENERATION_STORE -def activate_kitchen(kitchen_id: str) -> None: - """Activate generation state for one current kitchen lifecycle.""" - get_recipe_generation_store().activate_kitchen(kitchen_id) +def activate_activation(activation_id: KitchenActivationId) -> None: + """Activate generation state for one current kitchen activation.""" + get_recipe_generation_store().activate_activation(activation_id) -def retire_kitchen(kitchen_id: str) -> None: - """Retire generation state when the owning kitchen artifact is retired.""" - get_recipe_generation_store().retire_kitchen(kitchen_id) +def retire_activation(activation_id: KitchenActivationId) -> None: + """Retire generation state when a kitchen activation is retired.""" + get_recipe_generation_store().retire_activation(activation_id) -register_kitchen_retirement_callback(retire_kitchen) +register_activation_retirement_callback(retire_activation) diff --git a/src/autoskillit/server/recipe/_recipe_section_pagination.py b/src/autoskillit/server/recipe/_recipe_section_pagination.py index 95e1bed850..0e721f40e8 100644 --- a/src/autoskillit/server/recipe/_recipe_section_pagination.py +++ b/src/autoskillit/server/recipe/_recipe_section_pagination.py @@ -21,6 +21,7 @@ recipe_section_plan_digest, resolve_recipe_section_response_bound, ) +from autoskillit.pipeline import KitchenActivationId from autoskillit.server.recipe._recipe_section_planning import ( _render_candidate, plan_pages, @@ -38,7 +39,7 @@ RecipeSectionRequestState, SelectedRecipeSection, ) -from autoskillit.server.recipe.section._lifecycle import register_kitchen_retirement_callback +from autoskillit.server.recipe.section._lifecycle import register_activation_retirement_callback from autoskillit.server.recipe.section._rendering import render_recipe_section_failure from autoskillit.server.recipe.section._verification import ( verify_finalized_recipe_section_plan, @@ -71,7 +72,7 @@ def _convergence_iteration_ceiling() -> int: "RecipeSectionRequestState", "SelectedRecipeSection", "build_recipe_section_page_plan", - "evict_kitchen", + "evict_activation", "get_or_build_recipe_section_page_plan", "render_recipe_section_failure", "render_recipe_section_page", @@ -84,7 +85,7 @@ def _convergence_iteration_ceiling() -> int: @dataclass(frozen=True, slots=True) class _PagePlanCacheKey: - kitchen_id: str + activation_id: KitchenActivationId generation: RecipeArtifactGeneration initialization_id: str | None section: str @@ -123,7 +124,7 @@ def __init__( self._entries: OrderedDict[_PagePlanCacheKey, RecipeSectionPagePlan] = OrderedDict() self._weight_bytes = 0 self._builds: dict[_PagePlanCacheKey, _PagePlanBuildState] = {} - self._retired_kitchens: set[str] = set() + self._retired_activations: set[KitchenActivationId] = set() self._lock = RLock() def get(self, key: _PagePlanCacheKey) -> RecipeSectionPagePlan | None: @@ -147,7 +148,7 @@ def _put_locked(self, key: _PagePlanCacheKey, plan: RecipeSectionPagePlan) -> No def put(self, key: _PagePlanCacheKey, plan: RecipeSectionPagePlan) -> None: with self._lock: - if key.kitchen_id not in self._retired_kitchens: + if key.activation_id not in self._retired_activations: self._put_locked(key, plan) def get_or_build( @@ -155,7 +156,7 @@ def get_or_build( key: _PagePlanCacheKey, builder: Callable[[], RecipeSectionPagePlan], ) -> RecipeSectionPagePlan: - """Share a key build and admit it only while its kitchen remains active.""" + """Share a key build and admit it only while its activation remains active.""" with self._lock: cached = self._entries.get(key) if cached is not None: @@ -177,7 +178,7 @@ def get_or_build( build_state.completed.set() raise with self._lock: - if key.kitchen_id not in self._retired_kitchens: + if key.activation_id not in self._retired_activations: self._put_locked(key, plan) build_state.plan = plan self._builds.pop(key, None) @@ -195,12 +196,12 @@ def clear(self) -> None: with self._lock: self._entries.clear() self._weight_bytes = 0 - self._retired_kitchens.clear() + self._retired_activations.clear() - def evict_kitchen(self, kitchen_id: str) -> None: + def evict_activation(self, activation_id: KitchenActivationId) -> None: with self._lock: - self._retired_kitchens.add(kitchen_id) - keys = [key for key in self._entries if key.kitchen_id == kitchen_id] + self._retired_activations.add(activation_id) + keys = [key for key in self._entries if key.activation_id == activation_id] for key in keys: self._weight_bytes -= self._entries.pop(key).cache_weight_bytes @@ -217,24 +218,26 @@ def _page_plan_cache() -> PagePlanCache: return _PAGE_PLAN_CACHE -def evict_kitchen(kitchen_id: str) -> None: - """Best-effort idempotent eviction for a retired kitchen namespace.""" +def evict_activation(activation_id: KitchenActivationId) -> None: + """Best-effort idempotent eviction for a retired activation namespace.""" try: with _PAGE_PLAN_CACHE_LOCK: cache = _PAGE_PLAN_CACHE if cache is not None: - cache.evict_kitchen(kitchen_id) + cache.evict_activation(activation_id) except Exception: logger.warning( - "recipe_section_cache_eviction_failed", kitchen_id=kitchen_id, exc_info=True + "recipe_section_cache_eviction_failed", + activation_id=activation_id.value, + exc_info=True, ) -def _evict_retired_kitchen(kitchen_id: str) -> None: - evict_kitchen(kitchen_id) +def _evict_retired_activation(activation_id: KitchenActivationId) -> None: + evict_activation(activation_id) -register_kitchen_retirement_callback(_evict_retired_kitchen) +register_activation_retirement_callback(_evict_retired_activation) def resolve_recipe_section_bound_bytes( @@ -314,14 +317,12 @@ def select_recipe_section( def build_recipe_section_page_plan( *, - kitchen_id: str, generation: RecipeArtifactGeneration, selected: SelectedRecipeSection, recipe_section_bound_bytes: int, char_ceiling: int | None = None, ) -> RecipeSectionPagePlan: """Build, finalize, and verify one deterministic immutable page plan.""" - del kitchen_id # Identity participates in the cache key, not the manifest. if recipe_section_bound_bytes <= 0: raise RecipeSectionBoundError("recipe section bound must be positive") section_sha256 = selected_section_sha256(selected) @@ -411,14 +412,14 @@ def build_recipe_section_page_plan( def _cache_key( *, - kitchen_id: str, + activation_id: KitchenActivationId, generation: RecipeArtifactGeneration, selected: SelectedRecipeSection, recipe_section_bound_bytes: int, char_ceiling: int | None = None, ) -> _PagePlanCacheKey: return _PagePlanCacheKey( - kitchen_id=kitchen_id, + activation_id=activation_id, generation=generation, initialization_id=selected.initialization_id, section=selected.section, @@ -433,7 +434,7 @@ def _cache_key( def get_or_build_recipe_section_page_plan( *, - kitchen_id: str, + activation_id: KitchenActivationId, generation: RecipeArtifactGeneration, selected: SelectedRecipeSection, recipe_section_bound_bytes: int, @@ -441,7 +442,7 @@ def get_or_build_recipe_section_page_plan( ) -> RecipeSectionPagePlan: """Return a verified cached plan or build and admit one.""" key = _cache_key( - kitchen_id=kitchen_id, + activation_id=activation_id, generation=generation, selected=selected, recipe_section_bound_bytes=recipe_section_bound_bytes, @@ -451,7 +452,6 @@ def get_or_build_recipe_section_page_plan( return cache.get_or_build( key, lambda: build_recipe_section_page_plan( - kitchen_id=kitchen_id, generation=generation, selected=selected, recipe_section_bound_bytes=recipe_section_bound_bytes, diff --git a/src/autoskillit/server/recipe/_recipe_segment_delivery.py b/src/autoskillit/server/recipe/_recipe_segment_delivery.py index 1acf02ab39..0dd4f797a3 100644 --- a/src/autoskillit/server/recipe/_recipe_segment_delivery.py +++ b/src/autoskillit/server/recipe/_recipe_segment_delivery.py @@ -500,9 +500,10 @@ def prepare_recipe_segment_delivery( ): return None artifact_dir = tool_ctx.temp_dir + activation_id = tool_ctx.kitchen_activation_id persisted = load_recipe_artifact( artifact_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, identity=state.artifact_generation, ) expected_projection = _finalized_projection_payload(state.finalized_projection) diff --git a/src/autoskillit/server/recipe/section/_lifecycle.py b/src/autoskillit/server/recipe/section/_lifecycle.py index b764c9ecc2..b1c045fe15 100644 --- a/src/autoskillit/server/recipe/section/_lifecycle.py +++ b/src/autoskillit/server/recipe/section/_lifecycle.py @@ -6,33 +6,34 @@ from threading import RLock from autoskillit.core import get_logger +from autoskillit.pipeline import KitchenActivationId logger = get_logger(__name__) -_KitchenRetirementCallback = Callable[[str], None] -_KITCHEN_RETIREMENT_CALLBACKS: set[_KitchenRetirementCallback] = set() -_KITCHEN_RETIREMENT_LOCK = RLock() +_ActivationRetirementCallback = Callable[[KitchenActivationId], None] +_ACTIVATION_RETIREMENT_CALLBACKS: set[_ActivationRetirementCallback] = set() +_ACTIVATION_RETIREMENT_LOCK = RLock() -def register_kitchen_retirement_callback(callback: _KitchenRetirementCallback) -> None: +def register_activation_retirement_callback(callback: _ActivationRetirementCallback) -> None: """Register an idempotent recipe-section cleanup callback.""" - with _KITCHEN_RETIREMENT_LOCK: - _KITCHEN_RETIREMENT_CALLBACKS.add(callback) + with _ACTIVATION_RETIREMENT_LOCK: + _ACTIVATION_RETIREMENT_CALLBACKS.add(callback) -def notify_kitchen_retired(kitchen_id: str) -> None: - """Notify registered supporting state after artifact retirement succeeds.""" - with _KITCHEN_RETIREMENT_LOCK: - callbacks = tuple(_KITCHEN_RETIREMENT_CALLBACKS) +def notify_activation_retired(activation_id: KitchenActivationId) -> None: + """Notify registered supporting state when a kitchen activation is retired.""" + with _ACTIVATION_RETIREMENT_LOCK: + callbacks = tuple(_ACTIVATION_RETIREMENT_CALLBACKS) for callback in callbacks: try: - callback(kitchen_id) + callback(activation_id) except Exception: callback_module = getattr(callback, "__module__", type(callback).__module__) callback_name = getattr(callback, "__qualname__", type(callback).__qualname__) logger.warning( "recipe_section_retirement_callback_failed", - kitchen_id=kitchen_id, + activation_id=activation_id.value, callback=f"{callback_module}.{callback_name}", exc_info=True, ) diff --git a/src/autoskillit/server/tools/_recipe_section_handler.py b/src/autoskillit/server/tools/_recipe_section_handler.py index 5e9f46aafe..df8ba3fcad 100644 --- a/src/autoskillit/server/tools/_recipe_section_handler.py +++ b/src/autoskillit/server/tools/_recipe_section_handler.py @@ -355,10 +355,11 @@ async def get_recipe_section( if part < 0: return _recipe_section_failure("invalid_recipe_section_part") + activation_id = tool_ctx.kitchen_activation_id try: persisted = load_recipe_artifact( artifact_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, identity=identity, ) except RecipeArtifactSchemaError as exc: @@ -372,7 +373,7 @@ async def get_recipe_section( if producer_tool not in recipe_recreation_producers(): return _recipe_section_failure("recipe_artifact_unavailable") generation_record = get_recipe_generation_store().lookup_artifact( - tool_ctx.kitchen_id, + activation_id, identity, ) if generation_record is None: @@ -383,7 +384,7 @@ async def get_recipe_section( ) recreated_generation = persist_recipe_artifact( artifact_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, producer_tool=producer_tool, recipe_name=requested_recipe_name, payload=recreated_payload, @@ -407,7 +408,7 @@ async def get_recipe_section( try: persisted = load_recipe_artifact( artifact_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, identity=identity, ) except RecipeArtifactSchemaError as exc: @@ -448,7 +449,7 @@ async def get_recipe_section( try: page_plan = get_or_build_recipe_section_page_plan( - kitchen_id=tool_ctx.kitchen_id, + activation_id=activation_id, generation=identity, selected=selected, recipe_section_bound_bytes=request_state.recipe_section_bound_bytes, diff --git a/src/autoskillit/server/tools/tools_kitchen/_close_kitchen.py b/src/autoskillit/server/tools/tools_kitchen/_close_kitchen.py index 32c7958254..81cf19f4d7 100644 --- a/src/autoskillit/server/tools/tools_kitchen/_close_kitchen.py +++ b/src/autoskillit/server/tools/tools_kitchen/_close_kitchen.py @@ -4,7 +4,6 @@ import json from copy import deepcopy -from pathlib import Path from typing import TYPE_CHECKING from fastmcp import Context @@ -15,7 +14,6 @@ fast_dumps, get_logger, ) -from autoskillit.pipeline import closed_kitchen_open_state from autoskillit.server import mcp from autoskillit.server._notify import track_response_size from autoskillit.server._tracker_authority import ( @@ -23,11 +21,8 @@ _release_kitchen_tracker_authority, try_retire_tracker, ) +from autoskillit.server.lifecycle._kitchen_identity import retire_kitchen_identity from autoskillit.server.lifecycle._session_scope import SCOPE_ORCHESTRATOR_EXACT, session_scoped -from autoskillit.server.recipe._recipe_delivery import retire_recipe_artifacts -from autoskillit.server.recipe._recipe_generation import ( - retire_kitchen as retire_recipe_generation, -) # Late-binding for monkeypatch reach: tests patch # "autoskillit.server.tools.tools_kitchen." (the package facade), so @@ -93,16 +88,8 @@ def _close_kitchen_handler() -> None: abandoned_targets = _drain_context_tracker_leases(ctx) for target in abandoned_targets: try_retire_tracker(target) - if isinstance(ctx.kitchen_id, str) and ctx.kitchen_id: - if isinstance(ctx.temp_dir, Path) and not retire_recipe_artifacts( - ctx.temp_dir, - kitchen_id=ctx.kitchen_id, - ): - logger.warning("close_kitchen_recipe_artifact_retirement_failed") - try: - retire_recipe_generation(ctx.kitchen_id) - except Exception: - logger.warning("close_kitchen_recipe_generation_retirement_failed", exc_info=True) + kitchen_id = ctx.kitchen_id + retire_kitchen_identity(ctx) ctx.active_recipe_packs = None ctx.active_recipe_features = None ctx.active_recipe_steps = None @@ -118,7 +105,7 @@ def _close_kitchen_handler() -> None: ctx.gate_infrastructure_ready = False logger.info("close_kitchen", gate_state="closed") if (log := ctx.github_api_log) is not None: - orphan_usage = log.drain(ctx.kitchen_id) + orphan_usage = log.drain(kitchen_id) if orphan_usage is not None: try: log_dir = _tk_pkg.resolve_log_dir(ctx.config.linux_tracing.log_dir) @@ -150,9 +137,6 @@ def _close_kitchen_handler() -> None: review_gate_path.unlink(missing_ok=True) except OSError: logger.warning("review_gate_state_remove_failed", path=str(review_gate_path)) - with ctx.kitchen_transition_lock: - context_id = ctx.kitchen_open_state.context_id - ctx.kitchen_open_state = closed_kitchen_open_state(context_id=context_id) @mcp.tool( diff --git a/src/autoskillit/server/tools/tools_kitchen/_open_kitchen_transition.py b/src/autoskillit/server/tools/tools_kitchen/_open_kitchen_transition.py index c7029fd663..31bef570e5 100644 --- a/src/autoskillit/server/tools/tools_kitchen/_open_kitchen_transition.py +++ b/src/autoskillit/server/tools/tools_kitchen/_open_kitchen_transition.py @@ -237,6 +237,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> str: ): tool_ctx.kitchen_open_state = new_kitchen_open_state( kitchen_id=active.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, context_id=active.context_id, ) try: diff --git a/src/autoskillit/workspace/clone/__init__.py b/src/autoskillit/workspace/clone/__init__.py index c1505cbcc7..2a03e8f723 100644 --- a/src/autoskillit/workspace/clone/__init__.py +++ b/src/autoskillit/workspace/clone/__init__.py @@ -47,6 +47,7 @@ from ._remote import ( CloneSourceResolution, _ensure_origin_isolated, + _matching_push_remote, _probe_clone_source_url, ) from ._worktree import ( @@ -431,6 +432,10 @@ def push_to_remote( is not accessed for URL lookup. This is the preferred calling convention when remote_url has been captured from clone_repo at pipeline start. + A configured remote is used only when all its push URLs match the resolved + destination, preserving branch tracking without changing remote configuration. + Otherwise, the resolved URL is passed to git push directly. + When remote_url is empty, falls back to reading the upstream URL from source_dir via 'git remote get-url origin' (read-only — no writes to source_dir). @@ -508,9 +513,10 @@ def push_to_remote( ), } - push_cmd = ["git", "push", "-u", "upstream", branch] + destination = _matching_push_remote(Path(clone_path), resolved_url) + push_cmd = ["git", "push", "-u", "--", destination, branch] if force: - push_cmd.append("--force-with-lease") + push_cmd.insert(3, "--force-with-lease") push_result = subprocess.run( push_cmd, cwd=clone_path, diff --git a/src/autoskillit/workspace/clone/_remote.py b/src/autoskillit/workspace/clone/_remote.py index 20b052f120..444bb43c3e 100644 --- a/src/autoskillit/workspace/clone/_remote.py +++ b/src/autoskillit/workspace/clone/_remote.py @@ -14,6 +14,26 @@ logger = get_logger(__name__) +def _matching_push_remote(checkout: Path, url: str) -> str: + """Use a named remote only when it pushes solely to the requested URL.""" + result = subprocess.run( + ["git", "remote", "-v"], + cwd=str(checkout), + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + return url + destinations: dict[str, set[str]] = {} + for line in result.stdout.splitlines(): + name, _, entry = line.partition("\t") + destination, _, direction = entry.rpartition(" ") + if direction == "(push)": + destinations.setdefault(name, set()).add(destination) + return next((name for name, urls in destinations.items() if urls == {url}), url) + + def _add_or_set_upstream(clone_path: Path, url: str) -> None: """Add or update the upstream remote in the clone. diff --git a/tests/_helpers.py b/tests/_helpers.py index 43e5e6f5c8..608650499d 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -12,7 +12,7 @@ from collections.abc import Callable from pathlib import Path from types import ModuleType -from typing import TypeVar +from typing import Any, TypeVar from unittest.mock import MagicMock import pytest @@ -39,6 +39,16 @@ INVALID_PATH_INPUTS: tuple[object, ...] = (None, 0, b"", object(), MagicMock()) +def kitchen_identity_fields(kitchen_id: str) -> dict[str, Any]: + """Build deterministic, distinct lineage and activation fields for test contexts.""" + from autoskillit.pipeline import KitchenActivationId + + return { + "kitchen_id": kitchen_id, + "kitchen_activation_id": KitchenActivationId(f"activation-{kitchen_id}"), + } + + def make_rollout_marker(*, backends: tuple[str, ...] = ("claude-code", "codex")) -> RolloutMarker: return { "change_id": "prompt-cost-study", diff --git a/tests/arch/test_git_ref_qualification.py b/tests/arch/test_git_ref_qualification.py index 1d6f2b849e..832065b95c 100644 --- a/tests/arch/test_git_ref_qualification.py +++ b/tests/arch/test_git_ref_qualification.py @@ -209,18 +209,18 @@ ( "workspace/clone/__init__.py", "_decontaminate_generated_files", - 169, + 170, "", "UNRESOLVABLE", ), ( "workspace/clone/__init__.py", "_decontaminate_generated_files", - 177, + 178, "", "UNRESOLVABLE", ), - ("workspace/clone/__init__.py", "push_to_remote", 511, "push", "BARE"), + ("workspace/clone/__init__.py", "push_to_remote", 517, "push", "BARE"), ) _DEFERRED_REF_SITES: dict[tuple[str, str, int, str, str], TrackedDeferral] = {} @@ -287,13 +287,25 @@ def _before_path_separator(arguments: tuple[ast.expr, ...]) -> tuple[ast.expr, . return arguments +def _remote_refspec_indices(arguments: tuple[ast.expr, ...]) -> tuple[int, ...]: + """Skip the remote operand while honoring the end of options.""" + positions: list[int] = [] + options_ended = False + for index, argument in enumerate(arguments): + value = _constant_string(argument) + if value == "--": + options_ended = True + elif options_ended or not (value or "").startswith("-"): + positions.append(index) + return tuple(positions[1:]) + + def _revision_indices(subcommand: str, arguments: tuple[ast.expr, ...]) -> tuple[int, ...] | None: before_separator = _before_path_separator(arguments) if subcommand in _ALL_NON_FLAG_REVISIONS or subcommand == "diff": return _non_flag_indices(before_separator) if subcommand in _REMOTE_REFSPEC_SUBCOMMANDS: - positions = _non_flag_indices(before_separator) - return positions[1:] + return _remote_refspec_indices(arguments) if subcommand in {"checkout", "switch"}: if any( _constant_string(argument) in {"-B", "-C", "-b", "-c"} for argument in before_separator @@ -717,6 +729,15 @@ def test_push_remote_name_is_not_a_revision() -> None: assert [site.classification for site in sites] == ["BARE"] +@pytest.mark.parametrize("subcommand", ["fetch", "ls-remote", "push"]) +def test_remote_ref_after_option_terminator_is_still_checked(subcommand: str) -> None: + sites = _scan_tree( + ast.parse(f'subprocess.run(["git", "{subcommand}", "--", remote, branch])'), + "synthetic.py", + ) + assert [(site.subcommand, site.classification) for site in sites] == [(subcommand, "BARE")] + + def test_checkout_dash_b_names_a_new_branch() -> None: sites = _scan_tree( ast.parse('subprocess.run(["git", "checkout", "-b", name])'), "synthetic.py" diff --git a/tests/arch/test_kitchen_identity_single_authority.py b/tests/arch/test_kitchen_identity_single_authority.py index 83f7da4df1..9c3bcb3f93 100644 --- a/tests/arch/test_kitchen_identity_single_authority.py +++ b/tests/arch/test_kitchen_identity_single_authority.py @@ -20,7 +20,15 @@ Location = tuple[str, str] _REFERENCE_TARGETS = frozenset({"resolve_kitchen_id"}) -_CALL_TARGETS = frozenset({"new_kitchen_open_state", "activate_kitchen", "KitchenOpenState"}) +_CALL_TARGETS = frozenset( + { + "new_kitchen_open_state", + "activate_activation", + "KitchenOpenState", + "KitchenActivationId", + "retire_recipe_artifacts", + } +) class _KitchenIdentityOccurrenceCollector(ast.NodeVisitor): @@ -32,6 +40,7 @@ def __init__(self, relative_path: str) -> None: self.references: defaultdict[str, Counter[Location]] = defaultdict(Counter) self.calls: defaultdict[str, Counter[Location]] = defaultdict(Counter) self.replace_kitchen_id_keywords: Counter[Location] = Counter() + self.replace_activation_id_keywords: Counter[Location] = Counter() def _location(self) -> Location: return ( @@ -77,15 +86,19 @@ def visit_Call(self, node: ast.Call) -> None: # Its public wrapper delegates to the store method; count callers of # the API, not that implementation detail as a second authority site. delegated_activation = ( - symbol == "activate_kitchen" + symbol == "activate_activation" and self._relative_path == "server/recipe/_recipe_generation.py" - and self._scope_stack == ["activate_kitchen"] + and self._scope_stack == ["activate_activation"] and isinstance(node.func, ast.Attribute) ) if symbol in _CALL_TARGETS and not delegated_activation: self._record(self.calls, symbol) if symbol == "replace" and any(keyword.arg == "kitchen_id" for keyword in node.keywords): self.replace_kitchen_id_keywords[self._location()] += 1 + if symbol == "replace" and any( + keyword.arg == "activation_id" for keyword in node.keywords + ): + self.replace_activation_id_keywords[self._location()] += 1 self.generic_visit(node) @@ -103,11 +116,13 @@ def _collect_occurrences( dict[str, Counter[Location]], dict[str, Counter[Location]], Counter[Location], + Counter[Location], ]: assert sources, "kitchen identity collector scanned no source files" references: defaultdict[str, Counter[Location]] = defaultdict(Counter) calls: defaultdict[str, Counter[Location]] = defaultdict(Counter) replace_kitchen_id_keywords: Counter[Location] = Counter() + replace_activation_id_keywords: Counter[Location] = Counter() for relative_path, source in sources: collector = _KitchenIdentityOccurrenceCollector(relative_path) @@ -117,14 +132,21 @@ def _collect_occurrences( for symbol, counts in collector.calls.items(): calls[symbol].update(counts) replace_kitchen_id_keywords.update(collector.replace_kitchen_id_keywords) + replace_activation_id_keywords.update(collector.replace_activation_id_keywords) - return dict(references), dict(calls), replace_kitchen_id_keywords + return ( + dict(references), + dict(calls), + replace_kitchen_id_keywords, + replace_activation_id_keywords, + ) def _collect_source_tree() -> tuple[ dict[str, Counter[Location]], dict[str, Counter[Location]], Counter[Location], + Counter[Location], ]: sources = [ (path.relative_to(SRC_ROOT).as_posix(), path.read_text(encoding="utf-8")) @@ -145,7 +167,9 @@ def _assert_exact_counter( def test_kitchen_identity_authority_matches_exact_occurrence_counts() -> None: - references, calls, replace_kitchen_id_keywords = _collect_source_tree() + references, calls, replace_kitchen_id_keywords, replace_activation_id_keywords = ( + _collect_source_tree() + ) _assert_exact_counter( references.get("resolve_kitchen_id", Counter()), @@ -172,9 +196,19 @@ def test_kitchen_identity_authority_matches_exact_occurrence_counts() -> None: "new_kitchen_open_state calls", ) _assert_exact_counter( - calls.get("activate_kitchen", Counter()), + calls.get("activate_activation", Counter()), + Counter({("server/lifecycle/_kitchen_identity.py", "establish_kitchen_identity"): 1}), + "activate_activation calls", + ) + _assert_exact_counter( + calls.get("KitchenActivationId", Counter()), Counter({("server/lifecycle/_kitchen_identity.py", "establish_kitchen_identity"): 1}), - "activate_kitchen calls", + "KitchenActivationId calls", + ) + _assert_exact_counter( + calls.get("retire_recipe_artifacts", Counter()), + Counter({("server/lifecycle/_kitchen_identity.py", "retire_kitchen_identity"): 1}), + "retire_recipe_artifacts calls", ) _assert_exact_counter( calls.get("KitchenOpenState", Counter()), @@ -189,6 +223,10 @@ def test_kitchen_identity_authority_matches_exact_occurrence_counts() -> None: assert not replace_kitchen_id_keywords, ( f"dataclasses.replace must not change kitchen identity: {replace_kitchen_id_keywords}" ) + assert not replace_activation_id_keywords, ( + "dataclasses.replace must not change kitchen activation identity: " + f"{replace_activation_id_keywords}" + ) def test_committed_reseed_passes_the_existing_kitchen_id_attribute() -> None: @@ -224,26 +262,37 @@ def test_committed_reseed_passes_the_existing_kitchen_id_attribute() -> None: def establish_kitchen_identity(): resolve_kitchen_id() + KitchenActivationId("first-activation") new_kitchen_open_state(kitchen_id="first", context_id="ctx") new_kitchen_open_state(kitchen_id="second", context_id="ctx") - activate_kitchen("first") + activate_activation("first") KitchenOpenState(phase=None, kitchen_id="first", operation_id="", context_id="ctx") replace(state, kitchen_id="forged") +def retire_kitchen_identity(): + retire_recipe_artifacts("temp", activation_id=activation_id) + + def unregistered_boot(): resolve_kitchen_id() + KitchenActivationId("rogue-activation") new_kitchen_open_state(kitchen_id="rogue", context_id="ctx") - activate_kitchen("rogue") + activate_activation("rogue") KitchenOpenState(phase=None, kitchen_id="rogue", operation_id="", context_id="ctx") + retire_recipe_artifacts("temp", activation_id=activation_id) replace(state, kitchen_id="forged") + replace(state, activation_id="forged") """ def test_forged_source_canary_trips_counting_guards() -> None: - references, calls, replace_kitchen_id_keywords = _collect_occurrences( - [("synthetic/boot.py", _FORGED_SOURCE)] - ) + ( + references, + calls, + replace_kitchen_id_keywords, + replace_activation_id_keywords, + ) = _collect_occurrences([("synthetic/boot.py", _FORGED_SOURCE)]) with pytest.raises(AssertionError, match="duplicate occurrences|occurrences changed"): _assert_exact_counter( @@ -262,7 +311,7 @@ def test_forged_source_canary_trips_counting_guards() -> None: assert ( calls["new_kitchen_open_state"][("synthetic/boot.py", "establish_kitchen_identity")] == 2 ) - for symbol in ("new_kitchen_open_state", "activate_kitchen", "KitchenOpenState"): + for symbol in ("new_kitchen_open_state", "activate_activation", "KitchenOpenState"): with pytest.raises(AssertionError, match="duplicate occurrences|occurrences changed"): _assert_exact_counter( calls[symbol], @@ -270,6 +319,33 @@ def test_forged_source_canary_trips_counting_guards() -> None: f"synthetic {symbol} calls", ) + activation_creation = calls.get("KitchenActivationId", Counter()) + assert activation_creation[("synthetic/boot.py", "establish_kitchen_identity")] == 1 + assert activation_creation[("synthetic/boot.py", "unregistered_boot")] == 1 + with pytest.raises(AssertionError, match="duplicate occurrences|occurrences changed"): + _assert_exact_counter( + activation_creation, + Counter({("synthetic/boot.py", "establish_kitchen_identity"): 1}), + "synthetic KitchenActivationId calls", + ) + + artifact_retirement = calls.get("retire_recipe_artifacts", Counter()) + assert artifact_retirement[("synthetic/boot.py", "retire_kitchen_identity")] == 1 + assert artifact_retirement[("synthetic/boot.py", "unregistered_boot")] == 1 + with pytest.raises(AssertionError, match="duplicate occurrences|occurrences changed"): + _assert_exact_counter( + artifact_retirement, + Counter({("synthetic/boot.py", "retire_kitchen_identity"): 1}), + "synthetic retire_recipe_artifacts calls", + ) + + assert replace_activation_id_keywords[("synthetic/boot.py", "unregistered_boot")] == 1 + with pytest.raises(AssertionError, match="replace must not change activation identity"): + assert not replace_activation_id_keywords, ( + "dataclasses.replace must not change activation identity: " + f"{replace_activation_id_keywords}" + ) + with pytest.raises(AssertionError, match="replace must not change kitchen identity"): assert not replace_kitchen_id_keywords, ( f"dataclasses.replace must not change kitchen identity: {replace_kitchen_id_keywords}" diff --git a/tests/arch/test_recipe_delivery_provenance.py b/tests/arch/test_recipe_delivery_provenance.py index b200e13fa1..b03f5f3c04 100644 --- a/tests/arch/test_recipe_delivery_provenance.py +++ b/tests/arch/test_recipe_delivery_provenance.py @@ -12,6 +12,7 @@ AUTOSKILLIT_ATTESTED_META_SUPPORT, CLAUDE_INJECTED_CLIENT_RESULT_TOKENS, ) +from tests._helpers import kitchen_identity_fields pytestmark = [pytest.mark.layer("arch"), pytest.mark.small] @@ -270,7 +271,7 @@ def test_implementation_recipe_segments_public_startup_but_persists_full_authori SimpleNamespace( backend=BACKEND_REGISTRY["claude-code"](), config=SimpleNamespace(output_budget=OutputBudgetConfig()), - kitchen_id="impl-envelope-proof", + **kitchen_identity_fields("impl-envelope-proof"), recipe_execution_lock=threading.RLock(), recipe_initialization_state=NoActiveRecipe(), temp_dir=tmp_path, @@ -307,7 +308,7 @@ def test_implementation_recipe_segments_public_startup_but_persists_full_authori assert finalized.artifact_generation is not None persisted = load_recipe_artifact( tmp_path, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, identity=finalized.artifact_generation, ) assert persisted["content"] == prepared.canonical_artifact_payload["content"] diff --git a/tests/conftest.py b/tests/conftest.py index 4c193a50fe..df1b7f9d99 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -49,7 +49,11 @@ SubprocessResult, TerminationReason, ) -from tests._helpers import _collect_structlog_proxies, _flush_structlog_proxy_caches +from tests._helpers import ( + _collect_structlog_proxies, + _flush_structlog_proxy_caches, + kitchen_identity_fields, +) from tests.arch._policy_gate_plumbing import TEST_BASE_KEY, BaseRefContext # Mirror the standalone hook process import mode: runtime sibling modules @@ -893,13 +897,24 @@ def tool_ctx_kitchen_open(tool_ctx): def bind_test_kitchen_identity(ctx, kitchen_id: str) -> None: - """Bind a test kitchen identity without activating the recipe store.""" - from autoskillit.pipeline import closed_kitchen_open_state, new_kitchen_open_state + """Bind deterministic, distinct lineage and activation identities. + + Tests of identity separation or activation freshness must use the real + ``establish_kitchen_identity`` authority. + """ + from autoskillit.pipeline import ( + closed_kitchen_open_state, + new_kitchen_open_state, + ) with ctx.kitchen_transition_lock: context_id = ctx.kitchen_open_state.context_id ctx.kitchen_open_state = ( - new_kitchen_open_state(kitchen_id=kitchen_id, context_id=context_id) + new_kitchen_open_state( + kitchen_id=kitchen_id, + activation_id=kitchen_identity_fields(kitchen_id)["kitchen_activation_id"], + context_id=context_id, + ) if kitchen_id else closed_kitchen_open_state(context_id=context_id) ) diff --git a/tests/contracts/fixtures/recipes.py b/tests/contracts/fixtures/recipes.py index adf4e92622..f4272b2354 100644 --- a/tests/contracts/fixtures/recipes.py +++ b/tests/contracts/fixtures/recipes.py @@ -26,6 +26,7 @@ ) from autoskillit.server.recipe._recipe_generation import RecipeGenerationStore from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload +from tests._helpers import kitchen_identity_fields from tests._tracked_recipes import tracked_recipe_paths BUNDLED_RECIPE_PATHS = tracked_recipe_paths( @@ -99,7 +100,7 @@ def compile_bounded_page_plan( SimpleNamespace( backend=BACKEND_REGISTRY[backend_name](), config=SimpleNamespace(output_budget=resolved_output_budget), - kitchen_id=f"contract-{surface}-{backend_name}-{recipe_path.stem}", + **kitchen_identity_fields(f"contract-{surface}-{backend_name}-{recipe_path.stem}"), recipe_execution_lock=threading.RLock(), recipe_initialization_state=NoActiveRecipe(), temp_dir=temp_dir, diff --git a/tests/contracts/test_completion_receipt_fitness.py b/tests/contracts/test_completion_receipt_fitness.py index d638f8f4de..8d44619cc2 100644 --- a/tests/contracts/test_completion_receipt_fitness.py +++ b/tests/contracts/test_completion_receipt_fitness.py @@ -28,6 +28,7 @@ _render_completion_receipt, recipe_initialization_receipt, ) +from tests._helpers import kitchen_identity_fields from tests.contracts.test_delivery_bound_fitness import ( _backend_capabilities, _delivery_recipe_names, @@ -106,7 +107,7 @@ def test_implementation_shape_supplies_unavailable_context( Any, SimpleNamespace( backend=None, - kitchen_id=f"implementation-{step_name}-shape", + **kitchen_identity_fields(f"implementation-{step_name}-shape"), temp_dir=tmp_path, ), ) @@ -162,7 +163,7 @@ def test_completion_receipt_fits_every_delivery_bound( Any, SimpleNamespace( backend=None, - kitchen_id=f"receipt-fitness-{recipe_name}", + **kitchen_identity_fields(f"receipt-fitness-{recipe_name}"), temp_dir=tmp_path, ), ) @@ -174,7 +175,7 @@ def test_completion_receipt_fits_every_delivery_bound( ) generation = persist_recipe_artifact( tmp_path, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, producer_tool="open_kitchen", recipe_name=recipe_name, payload=prepared.canonical_artifact_payload, diff --git a/tests/contracts/test_delivery_bound_fitness.py b/tests/contracts/test_delivery_bound_fitness.py index dd0a2e5ac8..3b5ec93520 100644 --- a/tests/contracts/test_delivery_bound_fitness.py +++ b/tests/contracts/test_delivery_bound_fitness.py @@ -50,6 +50,7 @@ from autoskillit.server.tools._serve_helpers import ( build_open_kitchen_recipe_payload, ) +from tests._helpers import kitchen_identity_fields from tests._tracked_recipes import tracked_recipe_load_result, tracked_recipe_names from tests.contracts._delivery_constants import ( CALIBRATED_PAGES_PER_SECTION, @@ -170,7 +171,7 @@ def test_bundled_recipe_open_kitchen_envelope_fits_per_backend( SimpleNamespace( backend=BACKEND_REGISTRY[backend_name](), config=SimpleNamespace(output_budget=OutputBudgetConfig()), - kitchen_id=f"fitness-{backend_name}-{recipe_name}", + **kitchen_identity_fields(f"fitness-{backend_name}-{recipe_name}"), temp_dir=tmp_path, ), ) diff --git a/tests/contracts/test_delivery_mode_ledger.py b/tests/contracts/test_delivery_mode_ledger.py index 6795b295cf..0bf1013de4 100644 --- a/tests/contracts/test_delivery_mode_ledger.py +++ b/tests/contracts/test_delivery_mode_ledger.py @@ -27,6 +27,7 @@ ) from autoskillit.server.recipe._recipe_generation import RecipeGenerationStore from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload +from tests._helpers import kitchen_identity_fields from tests.contracts.fixtures.recipes import BUNDLED_RECIPE_PATHS pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] @@ -103,7 +104,7 @@ def _resolve_delivery( SimpleNamespace( backend=BACKEND_REGISTRY[backend_name](), config=SimpleNamespace(output_budget=OutputBudgetConfig()), - kitchen_id=f"ledger-{backend_name}-{recipe_path.stem}", + **kitchen_identity_fields(f"ledger-{backend_name}-{recipe_path.stem}"), recipe_execution_lock=threading.RLock(), recipe_initialization_state=NoActiveRecipe(), temp_dir=tmp_path, diff --git a/tests/contracts/test_flat_array_encoding.py b/tests/contracts/test_flat_array_encoding.py index d1800efeda..daaba0b8f3 100644 --- a/tests/contracts/test_flat_array_encoding.py +++ b/tests/contracts/test_flat_array_encoding.py @@ -74,7 +74,6 @@ def test_array_section_content_is_structured_not_string(section: str) -> None: payload = _payload() selected = select_recipe_section(payload, section) plan = build_recipe_section_page_plan( - kitchen_id="contract-flat-array", generation=_generation(), selected=selected, recipe_section_bound_bytes=8_192, @@ -92,7 +91,6 @@ def test_non_array_section_content_remains_a_string(section: str) -> None: payload = _payload() selected = select_recipe_section(payload, section) plan = build_recipe_section_page_plan( - kitchen_id="contract-flat-array", generation=_generation(), selected=selected, recipe_section_bound_bytes=8_192, @@ -111,7 +109,6 @@ def test_complete_array_page_has_no_nested_json_strings(section: str) -> None: payload = _payload() selected = select_recipe_section(payload, section) plan = build_recipe_section_page_plan( - kitchen_id="contract-flat-array-nesting", generation=_generation(), selected=selected, recipe_section_bound_bytes=8_192, @@ -175,7 +172,6 @@ def test_fragment_pages_preserve_string_content() -> None: selected = select_recipe_section(dict(payload), "flow_records") plan = build_recipe_section_page_plan( - kitchen_id="contract-fragment", generation=_generation(), selected=selected, recipe_section_bound_bytes=2_048, # force fragmentation of the large element diff --git a/tests/contracts/test_flow_record_size_pin.py b/tests/contracts/test_flow_record_size_pin.py index 6ec578b73b..6c4da6aa7e 100644 --- a/tests/contracts/test_flow_record_size_pin.py +++ b/tests/contracts/test_flow_record_size_pin.py @@ -35,6 +35,7 @@ select_recipe_section, ) from autoskillit.server.tools._serve_helpers import build_open_kitchen_recipe_payload +from tests._helpers import kitchen_identity_fields from tests.contracts.fixtures.recipes import ( BUNDLED_RECIPE_PATHS, compile_bounded_page_plan, @@ -108,7 +109,7 @@ def test_implementation_flow_records_real_page_within_115k( SimpleNamespace( backend=backend, config=SimpleNamespace(output_budget=OutputBudgetConfig()), - kitchen_id="flow-pin-test", + **kitchen_identity_fields("flow-pin-test"), recipe_execution_lock=threading.RLock(), recipe_initialization_state=NoActiveRecipe(), temp_dir=tmp_path, @@ -125,7 +126,7 @@ def test_implementation_flow_records_real_page_within_115k( # Build the artifact so we have a RecipeArtifactGeneration generation = persist_recipe_artifact( tmp_path, - kitchen_id="flow-pin-test", + activation_id=tool_ctx.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="implementation", payload=prepared.canonical_artifact_payload, @@ -148,7 +149,6 @@ def test_implementation_flow_records_real_page_within_115k( ) page_plan = build_recipe_section_page_plan( - kitchen_id="flow-pin-test", generation=generation, selected=selected, recipe_section_bound_bytes=bound_bytes, diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 7c9c671567..0ecd87e78a 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -747,6 +747,7 @@ def test_fixture_stages_discovers_and_passes_the_production_validator(self, tmp_ assert "child agents" in discipline and "replay" in discipline assert recipe.steps["setup"].with_args["cwd"] == "${{ context.worktree_path }}" assert worker.with_args["cwd"] == "${{ context.worktree_path }}" + assert worker.with_args["output_dir"] == "${{ context.worktree_path }}" assert recipe.steps["test"].tool == "test_check" assert recipe.steps["test"].with_args["worktree_path"] == "${{ context.worktree_path }}" assert recipe.steps["push"].with_args["clone_path"] == "${{ context.worktree_path }}" diff --git a/tests/infra/test_git_ops_guard.py b/tests/infra/test_git_ops_guard.py index 86bc478130..b90bc99f6e 100644 --- a/tests/infra/test_git_ops_guard.py +++ b/tests/infra/test_git_ops_guard.py @@ -1133,6 +1133,39 @@ def test_mutation_git_after_failed_cd_is_denied( class TestCheckedOutRefAllows: + @pytest.mark.parametrize("write_to_owned_ref", [False, True], ids=["pr-body", "owned-ref"]) + def test_smoke_pr_creation_command_preserves_ref_protection( + self, linked_repo: dict[str, Path | str], write_to_owned_ref: bool + ) -> None: + from autoskillit.core import load_yaml + + linked = linked_repo["linked"] + assert isinstance(linked, Path) + recipe_path = ( + Path(__file__).resolve().parents[2] / "scripts/e2e/recipes/sandbox-smoke.yaml" + ) + command = load_yaml(recipe_path)["steps"]["create_pull_request"]["with"]["cmd"] + temp_dir = linked / ".autoskillit" / "temp" + replacements = { + "{{AUTOSKILLIT_TEMP}}": str(temp_dir), + "${{ inputs.repository }}": "example/sandbox", + "${{ context.branch_name }}": "review", + "${{ inputs.base_branch }}": "develop", + } + for source, value in replacements.items(): + command = command.replace(source, value) + assert "{{" not in command + if write_to_owned_ref: + command = command.replace( + str(temp_dir / "sandbox-smoke-pr-body.md"), + str(Path(str(linked_repo["common_git_dir"])) / "refs/heads/review"), + ) + + output = _run_guard(command, kitchen_open=True, tmpdir=linked, session_type="orchestrator") + + assert _is_denied(output) is write_to_owned_ref + _assert_ref_unchanged(linked, "refs/heads/review", str(linked_repo["old_sha"])) + @pytest.mark.parametrize( "command_template", [ diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 4411d10b42..1519b3ca04 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -33,6 +33,7 @@ _format_response, _strip_yaml_ingredients_block, ) +from autoskillit.pipeline import KitchenActivationId from autoskillit.recipe import load_and_validate from autoskillit.recipe.api import _api_cache from autoskillit.recipe.api._api_cache import LoadCache @@ -522,7 +523,7 @@ def test_attested_recipe_delivery_region_is_preserved_byte_for_byte( payload["recipe_flow"] = flow_generation.identity() generation = persist_recipe_artifact( tmp_path, - kitchen_id="formatter-contract", + activation_id=KitchenActivationId("formatter-contract"), producer_tool=tool_name, recipe_name="remediation", payload=payload, @@ -1189,7 +1190,7 @@ def _measure_persisted_envelope( payload["recipe_flow"] = flow_generation.identity() generation = persist_recipe_artifact( tmp_path, - kitchen_id="pretty-output", + activation_id=KitchenActivationId("pretty-output"), producer_tool="open_kitchen", recipe_name=recipe_name, payload=payload, diff --git a/tests/pipeline/test_context.py b/tests/pipeline/test_context.py index addf0bc291..03c4aa8958 100644 --- a/tests/pipeline/test_context.py +++ b/tests/pipeline/test_context.py @@ -359,6 +359,22 @@ def test_tool_context_kitchen_id_is_read_only_and_derived(tmp_path) -> None: assert ctx.kitchen_id == ctx.kitchen_open_state.kitchen_id == "test-kitchen" +def test_tool_context_kitchen_activation_id_is_read_only_and_derived(tmp_path) -> None: + from tests.conftest import bind_test_kitchen_identity + + ctx = _make_ctx(tmp_path) + + assert "kitchen_activation_id" not in {field.name for field in dataclasses.fields(ToolContext)} + with pytest.raises(RuntimeError, match="no kitchen activation is live"): + _ = ctx.kitchen_activation_id + with pytest.raises(AttributeError): + ctx.kitchen_activation_id = "forged" + + bind_test_kitchen_identity(ctx, "test-kitchen") + assert ctx.kitchen_activation_id == ctx.kitchen_open_state.activation_id + assert ctx.kitchen_activation_id.value != ctx.kitchen_id + + def test_toolcontext_github_client_annotated_with_protocol(): """github_client annotation must reference GitHubFetcher protocol.""" hints = get_type_hints(ToolContext) diff --git a/tests/pipeline/test_kitchen_transition.py b/tests/pipeline/test_kitchen_transition.py index bb7fabd089..043a96a3af 100644 --- a/tests/pipeline/test_kitchen_transition.py +++ b/tests/pipeline/test_kitchen_transition.py @@ -14,6 +14,7 @@ bind_kitchen_intent, canonical_kitchen_intent_fingerprint, claim_kitchen_request, + closed_kitchen_open_state, commit_kitchen_response, confirm_kitchen_effect, mark_kitchen_effect_ambiguous, @@ -26,8 +27,11 @@ def _state(): + from autoskillit.pipeline import KitchenActivationId + return new_kitchen_open_state( kitchen_id="kitchen-1", + activation_id=KitchenActivationId("kitchen-1"), operation_id="operation-1", context_id="context-1", ) @@ -167,3 +171,64 @@ def test_confirmed_progress_converges_to_cached_exact_response() -> None: def test_unknown_or_out_of_order_phase_is_rejected() -> None: with pytest.raises(ValueError, match="illegal kitchen transition"): advance_kitchen_phase(_state(), KitchenOpenPhase.COMMITTED) + + +@pytest.mark.parametrize("value", ["", " ", "\t\n"]) +def test_kitchen_activation_id_rejects_blank_value_and_untyped_identity(value: str) -> None: + from autoskillit.pipeline import KitchenActivationId, require_kitchen_activation_id + + with pytest.raises(ValueError): + KitchenActivationId(value) + with pytest.raises(TypeError): + require_kitchen_activation_id("k") + + +def test_new_kitchen_open_state_requires_typed_activation_id() -> None: + from autoskillit.pipeline import KitchenActivationId + + with pytest.raises(TypeError): + new_kitchen_open_state(kitchen_id="kitchen-1", context_id="context-1") + with pytest.raises(TypeError): + new_kitchen_open_state( + kitchen_id="kitchen-1", + activation_id="activation-1", + context_id="context-1", + ) + state = new_kitchen_open_state( + kitchen_id="kitchen-1", + activation_id=KitchenActivationId("activation-1"), + context_id="context-1", + ) + assert state.activation_id == KitchenActivationId("activation-1") + + +def test_closed_kitchen_state_has_no_activation_id() -> None: + assert closed_kitchen_open_state().activation_id is None + + +def test_lifecycle_transitions_preserve_activation_id() -> None: + from autoskillit.pipeline import KitchenActivationId + + activation_id = KitchenActivationId("activation-1") + state = new_kitchen_open_state( + kitchen_id="kitchen-1", + activation_id=activation_id, + context_id="context-1", + ) + bound = bind_kitchen_intent(state, fingerprint=_fingerprint()) + advanced = advance_kitchen_phase(state, KitchenOpenPhase.REQUEST_BOUND) + started = start_kitchen_effect(bound, "recipe_serving") + ambiguous = mark_kitchen_effect_ambiguous( + started, + "recipe_serving", + evidence="connection lost after dispatch", + ) + committed = commit_kitchen_response( + bound, + response='{"success":true}', + ) + + assert all( + item.activation_id == activation_id + for item in (bound, advanced, started, ambiguous, committed) + ) diff --git a/tests/server/_recipe_section_pagination_test_helpers.py b/tests/server/_recipe_section_pagination_test_helpers.py index e54d5a1987..930f2913cf 100644 --- a/tests/server/_recipe_section_pagination_test_helpers.py +++ b/tests/server/_recipe_section_pagination_test_helpers.py @@ -107,7 +107,6 @@ def _build( *, bound: int, generation: RecipeArtifactGeneration | None = None, - kitchen_id: str = "kitchen-test", dynamic_content: str | None = None, ) -> Any: """Select a section and build a page plan.""" @@ -117,7 +116,6 @@ def _build( dynamic_content=dynamic_content, ) return build_recipe_section_page_plan( - kitchen_id=kitchen_id, generation=generation or _generation(), selected=selected, recipe_section_bound_bytes=bound, diff --git a/tests/server/conftest.py b/tests/server/conftest.py index 75e36eeebf..be47a01559 100644 --- a/tests/server/conftest.py +++ b/tests/server/conftest.py @@ -143,12 +143,13 @@ def _set_mock_kitchen_transition(ctx: MagicMock, *, kitchen_id: str = "") -> Non """Give a mock context the typed transition state guaranteed by make_context().""" from threading import RLock - from autoskillit.pipeline import closed_kitchen_open_state + from autoskillit.pipeline import ToolContext, closed_kitchen_open_state ctx.kitchen_transition_lock = RLock() ctx.kitchen_open_state = closed_kitchen_open_state() bind_test_kitchen_identity(ctx, kitchen_id) type(ctx).kitchen_id = property(lambda self: self.kitchen_open_state.kitchen_id) + type(ctx).kitchen_activation_id = ToolContext.kitchen_activation_id def _make_mock_ctx(config=None) -> MagicMock: diff --git a/tests/server/test_attestation_delivery_reachability.py b/tests/server/test_attestation_delivery_reachability.py index d38c505052..79b2ccad65 100644 --- a/tests/server/test_attestation_delivery_reachability.py +++ b/tests/server/test_attestation_delivery_reachability.py @@ -41,6 +41,7 @@ build_embedded_completion_response, recipe_initialization_receipt, ) +from tests._helpers import kitchen_identity_fields from tests.conftest import _make_result from tests.contracts.test_delivery_bound_fitness import _full_open_kitchen_generation from tests.server._pipeline_test_helpers import _write_tracker @@ -611,7 +612,7 @@ async def test_delivery_modes_preserve_one_snapshot_skill_input_shapes( payload, projection = _full_open_kitchen_generation("implementation") tool_ctx = SimpleNamespace( backend=None, - kitchen_id="same-snapshot-delivery", + **kitchen_identity_fields("same-snapshot-delivery"), temp_dir=tmp_path, ) prepared = prepare_recipe_delivery_generation( @@ -622,7 +623,7 @@ async def test_delivery_modes_preserve_one_snapshot_skill_input_shapes( ) generation = persist_recipe_artifact( tmp_path, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="implementation", payload=prepared.canonical_artifact_payload, @@ -643,7 +644,7 @@ async def test_delivery_modes_preserve_one_snapshot_skill_input_shapes( ] spilled = load_recipe_artifact( tmp_path, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, identity=generation, )[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] embedded_completion = build_embedded_completion_response( diff --git a/tests/server/test_helpers_gate.py b/tests/server/test_helpers_gate.py index 385dbccb4f..d97784aadb 100644 --- a/tests/server/test_helpers_gate.py +++ b/tests/server/test_helpers_gate.py @@ -54,7 +54,6 @@ def _valid_recipe_section_pages(format_family: str) -> list[dict[str, object]]: flow_record_count=2, ) plan = build_recipe_section_page_plan( - kitchen_id="consumer-sequence", generation=generation, selected=select_recipe_section(payload, section), recipe_section_bound_bytes=2_000, diff --git a/tests/server/test_kitchen_identity_lifecycle.py b/tests/server/test_kitchen_identity_lifecycle.py index 7482157d78..38105b255a 100644 --- a/tests/server/test_kitchen_identity_lifecycle.py +++ b/tests/server/test_kitchen_identity_lifecycle.py @@ -125,14 +125,228 @@ async def test_boot_open_load_share_one_kitchen_identity( assert tool_ctx.kitchen_id == campaign_id +def _install_fresh_recipe_stores(monkeypatch): + from autoskillit.server.recipe import _recipe_generation, _recipe_section_pagination + + store = _recipe_generation.RecipeGenerationStore() + cache = _recipe_section_pagination.PagePlanCache() + monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", store) + monkeypatch.setattr(_recipe_section_pagination, "_PAGE_PLAN_CACHE", cache) + return store, cache, _recipe_section_pagination + + +def _build_page_plan_for_activation(payload, activation_id, generation, pagination): + from tests.server._recipe_section_pagination_test_helpers import _build + + selected = pagination.select_recipe_section(payload, "content") + plan = _build(payload, "content", bound=4096, generation=generation) + key = pagination._cache_key( + activation_id=activation_id, + generation=generation, + selected=selected, + recipe_section_bound_bytes=4096, + ) + return key, plan + + +@pytest.mark.parametrize("campaign_mode", [False, True], ids=["anonymous", "campaign"]) +def test_recipe_stores_agree_across_retire_and_reactivate(tool_ctx, monkeypatch, campaign_mode): + from autoskillit.core import CAMPAIGN_ID_ENV_VAR + from autoskillit.pipeline import KitchenOpenPhase + from autoskillit.server.lifecycle import _kitchen_identity + from autoskillit.server.recipe._recipe_artifact import persist_recipe_artifact + from tests.server.test_recipe_generation import _record + from tests.server.test_tools_recipe_pull import _payload + + store, cache, pagination = _install_fresh_recipe_stores(monkeypatch) + if campaign_mode: + monkeypatch.setenv(CAMPAIGN_ID_ENV_VAR, "campaign-store-agreement") + else: + monkeypatch.delenv(CAMPAIGN_ID_ENV_VAR, raising=False) + + first_kitchen_id = _kitchen_identity.establish_kitchen_identity(tool_ctx) + first_activation_id = tool_ctx.kitchen_activation_id + assert first_activation_id.value != first_kitchen_id + + payload = _payload() + generation = persist_recipe_artifact( + tool_ctx.temp_dir, + activation_id=first_activation_id, + producer_tool="load_recipe", + recipe_name="remediation", + payload=payload, + ) + first_record = _record( + activation_id=first_activation_id, + compile_key=f"compile-{first_activation_id.value}", + ) + assert store.put(first_record).activation_id == first_activation_id + first_key, first_plan = _build_page_plan_for_activation( + payload, + first_activation_id, + generation, + pagination, + ) + cache.put(first_key, first_plan) + assert cache.get(first_key) is first_plan + + _kitchen_identity.retire_kitchen_identity(tool_ctx) + + from autoskillit.server.recipe._recipe_artifact import RecipeArtifactError + from autoskillit.server.recipe._recipe_generation import RecipeGenerationRetiredError + + assert tool_ctx.kitchen_open_state.phase is KitchenOpenPhase.CLOSED + with pytest.raises(RecipeArtifactError, match="namespace is retired"): + persist_recipe_artifact( + tool_ctx.temp_dir, + activation_id=first_activation_id, + producer_tool="load_recipe", + recipe_name="remediation", + payload=payload, + ) + with pytest.raises(RecipeGenerationRetiredError): + store.put(first_record) + assert cache.get(first_key) is None + cache.put(first_key, first_plan) + assert cache.get(first_key) is None + + second_kitchen_id = _kitchen_identity.establish_kitchen_identity(tool_ctx) + second_activation_id = tool_ctx.kitchen_activation_id + assert second_activation_id != first_activation_id + assert second_activation_id.value != second_kitchen_id + if campaign_mode: + assert second_kitchen_id == first_kitchen_id + else: + assert second_kitchen_id != first_kitchen_id + + second_generation = persist_recipe_artifact( + tool_ctx.temp_dir, + activation_id=second_activation_id, + producer_tool="load_recipe", + recipe_name="remediation", + payload=payload, + ) + second_record = _record( + activation_id=second_activation_id, + compile_key=f"compile-{second_activation_id.value}", + ) + assert store.put(second_record).activation_id == second_activation_id + second_key, second_plan = _build_page_plan_for_activation( + payload, + second_activation_id, + second_generation, + pagination, + ) + cache.put(second_key, second_plan) + assert cache.get(second_key) is second_plan + + +@pytest.mark.parametrize("retirement_result", ["raise", "false"], ids=["exception", "false"]) +def test_retire_kitchen_identity_closes_and_retires_memory_stores_on_artifact_failure( + tool_ctx, monkeypatch, retirement_result +): + from structlog.testing import capture_logs + + from autoskillit.pipeline import KitchenOpenPhase + from autoskillit.server.lifecycle import _kitchen_identity + from autoskillit.server.recipe import _recipe_artifact + from autoskillit.server.recipe._recipe_artifact import persist_recipe_artifact + from autoskillit.server.recipe._recipe_generation import RecipeGenerationRetiredError + from tests.server.test_recipe_generation import _record + from tests.server.test_tools_recipe_pull import _payload + + store, cache, pagination = _install_fresh_recipe_stores(monkeypatch) + _kitchen_identity.establish_kitchen_identity(tool_ctx) + activation_id = tool_ctx.kitchen_activation_id + payload = _payload() + generation = persist_recipe_artifact( + tool_ctx.temp_dir, + activation_id=activation_id, + producer_tool="load_recipe", + recipe_name="remediation", + payload=payload, + ) + record = _record( + activation_id=activation_id, + compile_key=f"compile-{activation_id.value}", + ) + store.put(record) + key, plan = _build_page_plan_for_activation( + payload, + activation_id, + generation, + pagination, + ) + cache.put(key, plan) + assert cache.get(key) is plan + + retirement_error = RuntimeError("tombstone write failed") + if retirement_result == "raise": + monkeypatch.setattr( + _recipe_artifact, + "retire_recipe_artifacts", + Mock(side_effect=retirement_error), + ) + with pytest.raises(RuntimeError, match="tombstone write failed") as exc_info: + _kitchen_identity.retire_kitchen_identity(tool_ctx) + assert exc_info.value is retirement_error + else: + monkeypatch.setattr(_recipe_artifact, "retire_recipe_artifacts", Mock(return_value=False)) + with capture_logs() as logs: + _kitchen_identity.retire_kitchen_identity(tool_ctx) + assert any( + event.get("event") == "kitchen_activation_artifact_retirement_failed" for event in logs + ) + + assert tool_ctx.kitchen_open_state.phase is KitchenOpenPhase.CLOSED + assert store.lookup_compile(activation_id, record.normalized_compile_key) is None + with pytest.raises(RecipeGenerationRetiredError): + store.put(record) + assert cache.get(key) is None + cache.put(key, plan) + assert cache.get(key) is None + + +def test_retirement_preserves_artifact_error_when_notification_also_fails(tool_ctx, monkeypatch): + from structlog.testing import capture_logs + + from autoskillit.pipeline import KitchenOpenPhase + from autoskillit.server.lifecycle import _kitchen_identity + from autoskillit.server.recipe import _recipe_artifact + from autoskillit.server.recipe.section import _lifecycle + + _kitchen_identity.establish_kitchen_identity(tool_ctx) + activation_id = tool_ctx.kitchen_activation_id + artifact_error = RuntimeError("tombstone write failed") + notify = Mock(side_effect=RuntimeError("notification lock failed")) + monkeypatch.setattr( + _recipe_artifact, "retire_recipe_artifacts", Mock(side_effect=artifact_error) + ) + monkeypatch.setattr(_lifecycle, "notify_activation_retired", notify) + + with capture_logs() as logs, pytest.raises(RuntimeError) as exc_info: + _kitchen_identity.retire_kitchen_identity(tool_ctx) + + assert exc_info.value is artifact_error + assert tool_ctx.kitchen_open_state.phase is KitchenOpenPhase.CLOSED + notify.assert_called_once_with(activation_id) + assert any( + event.get("event") == "kitchen_activation_retirement_notification_failed" for event in logs + ) + + def test_establish_kitchen_identity_activates_and_resets_owner_cache(tool_ctx): - from autoskillit.pipeline import KitchenOpenPhase, get_kitchen_process_identity + from autoskillit.pipeline import ( + KitchenActivationId, + KitchenOpenPhase, + get_kitchen_process_identity, + ) from autoskillit.server.lifecycle._kitchen_identity import establish_kitchen_identity from autoskillit.server.recipe._recipe_generation import get_recipe_generation_store from tests.server.test_recipe_generation import _record store = get_recipe_generation_store() - store.activate_kitchen("other") + store.activate_activation(KitchenActivationId("other")) owner_identity = get_kitchen_process_identity(tool_ctx, "owner-before-establishment") assert owner_identity.kitchen_id == "owner-before-establishment" @@ -142,8 +356,14 @@ def test_establish_kitchen_identity_activates_and_resets_owner_cache(tool_ctx): assert tool_ctx.kitchen_open_state.phase is KitchenOpenPhase.INFRASTRUCTURE_READY assert tool_ctx.kitchen_open_state.kitchen_id == kitchen_id assert tool_ctx.kitchen_process_identity is None - stored = store.put(_record(kitchen_id=kitchen_id, compile_key=f"compile-{kitchen_id}")) - assert stored.kitchen_id == kitchen_id + activation_id = tool_ctx.kitchen_activation_id + stored = store.put( + _record( + activation_id=activation_id, + compile_key=f"compile-{activation_id.value}", + ) + ) + assert stored.activation_id == activation_id @pytest.mark.parametrize("source", ["campaign", "resolver"]) @@ -154,7 +374,7 @@ def test_establish_kitchen_identity_rejects_whitespace_before_activation( from autoskillit.server.recipe import _recipe_generation activate = Mock() - monkeypatch.setattr(_recipe_generation, "activate_kitchen", activate) + monkeypatch.setattr(_recipe_generation, "activate_activation", activate) monkeypatch.setattr(_kitchen_identity, "resolve_kitchen_id", lambda: " \t\n") state = tool_ctx.kitchen_open_state cached_identity = tool_ctx.kitchen_process_identity diff --git a/tests/server/test_kitchen_lifecycle.py b/tests/server/test_kitchen_lifecycle.py index 45f5b34288..58d0d48f90 100644 --- a/tests/server/test_kitchen_lifecycle.py +++ b/tests/server/test_kitchen_lifecycle.py @@ -2,6 +2,7 @@ import json import os from datetime import UTC, datetime, timedelta +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import psutil @@ -215,29 +216,41 @@ async def test_close_kitchen_preserves_peer_tracker_and_lease_sidecar(monkeypatc lease.close_preserving() -async def test_back_to_back_open_close_open_resets_infrastructure(monkeypatch, tmp_path): - """Close resets identity, and a real reopen establishes a new usable identity.""" - import json +@pytest.fixture +def campaign_id(request, monkeypatch) -> str | None: + value = getattr(request, "param", None) + if value is not None: + monkeypatch.setenv("AUTOSKILLIT_CAMPAIGN_ID", value) + return value - from autoskillit.pipeline import KitchenOpenPhase + +@pytest.fixture +def real_kitchen_lifecycle(monkeypatch, tmp_path, campaign_id): + """Build real contexts with the lifecycle effects isolated to this test.""" from autoskillit.server import _misc from autoskillit.server.lifecycle import _lifespan - from autoskillit.server.tools.tools_kitchen import open_kitchen + from autoskillit.server.recipe import _recipe_generation, _recipe_section_pagination + from autoskillit.server.recipe._recipe_generation import RecipeGenerationStore + from autoskillit.server.recipe._recipe_section_pagination import PagePlanCache from autoskillit.server.tools.tools_kitchen._open_kitchen import _gate as open_gate - from autoskillit.server.tools.tools_recipe import load_recipe from tests.server._helpers import _mock_fmcp_ctx from tests.server.conftest import _READY_RECIPE_OVERRIDES monkeypatch.chdir(tmp_path) monkeypatch.setenv("AUTOSKILLIT_HEADLESS", "0") monkeypatch.setenv("AUTOSKILLIT_SESSION_TYPE", "orchestrator") + monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", RecipeGenerationStore()) + monkeypatch.setattr(_recipe_section_pagination, "_PAGE_PLAN_CACHE", PagePlanCache()) + + def make_context_for_lifecycle(): + return make_context( + AutomationConfig(), + runner=None, + plugin_authority=FakePluginArtifactAuthority(tmp_path), + project_dir=tmp_path, + ) - ctx = make_context( - AutomationConfig(), - runner=None, - plugin_authority=FakePluginArtifactAuthority(tmp_path), - project_dir=tmp_path, - ) + ctx = make_context_for_lifecycle() monkeypatch.setattr(_state, "_ctx", ctx) monkeypatch.setattr(_state, "_startup_ready", None) overrides = { @@ -246,6 +259,9 @@ async def test_back_to_back_open_close_open_resets_infrastructure(monkeypatch, t } fmcp_ctx = _mock_fmcp_ctx() + def new_context(): + return make_context_for_lifecycle() + with ( patch.object(tools_kitchen, "_prime_quota_cache", new_callable=AsyncMock), patch.object(_misc, "_prime_quota_cache", new_callable=AsyncMock), @@ -260,40 +276,282 @@ async def test_back_to_back_open_close_open_resets_infrastructure(monkeypatch, t patch.object(_tracker_authority, "register_active_kitchen", return_value=True), patch.object(_tracker_authority, "unregister_active_kitchen", return_value=True), ): - result1 = json.loads( - await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx) + yield SimpleNamespace( + ctx=ctx, + overrides=overrides, + fmcp_ctx=fmcp_ctx, + new_context=new_context, ) - assert result1["success"] is True, result1 - assert ctx.kitchen_open_state.phase is KitchenOpenPhase.COMMITTED - assert ctx.gate_infrastructure_ready is True - first_id = ctx.kitchen_id - first_identity = ctx.kitchen_process_identity - assert first_identity is not None - assert first_identity.kitchen_id == first_id - _close_kitchen_handler() - assert ctx.gate_infrastructure_ready is False - assert ctx.kitchen_open_state.phase is KitchenOpenPhase.CLOSED - assert ctx.kitchen_id == "" - assert ctx.kitchen_process_identity is None - result2 = json.loads( - await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx) - ) - assert result2["success"] is True, result2 - assert ctx.kitchen_open_state.phase is KitchenOpenPhase.COMMITTED - assert ctx.gate_infrastructure_ready is True - second_id = ctx.kitchen_id +@pytest.mark.parametrize( + "campaign_id", + [None, "campaign-5270"], + indirect=True, + ids=["ephemeral", "campaign"], +) +async def test_back_to_back_open_close_open_resets_infrastructure( + campaign_id, + real_kitchen_lifecycle, +): + """Close resets identity, and a real reopen establishes a new usable identity.""" + from autoskillit.pipeline import KitchenOpenPhase + from autoskillit.server.tools.tools_kitchen import open_kitchen + from autoskillit.server.tools.tools_recipe import load_recipe + + ctx = real_kitchen_lifecycle.ctx + overrides = real_kitchen_lifecycle.overrides + fmcp_ctx = real_kitchen_lifecycle.fmcp_ctx + + result1 = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert result1["success"] is True, result1 + assert ctx.kitchen_open_state.phase is KitchenOpenPhase.COMMITTED + assert ctx.gate_infrastructure_ready is True + first_id = ctx.kitchen_id + first_activation_id = ctx.kitchen_activation_id + first_identity = ctx.kitchen_process_identity + assert first_identity is not None + assert first_identity.kitchen_id == first_id + + _close_kitchen_handler() + assert ctx.gate_infrastructure_ready is False + assert ctx.kitchen_open_state.phase is KitchenOpenPhase.CLOSED + assert ctx.kitchen_id == "" + assert ctx.kitchen_process_identity is None + + result2 = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert result2["success"] is True, result2 + assert ctx.kitchen_open_state.phase is KitchenOpenPhase.COMMITTED + assert ctx.gate_infrastructure_ready is True + second_id = ctx.kitchen_id + assert ctx.kitchen_activation_id != first_activation_id + if campaign_id is None: assert second_id != first_id - second_identity = ctx.kitchen_process_identity - assert second_identity is not None - assert second_identity.kitchen_id == second_id + else: + assert first_id == second_id == campaign_id + second_identity = ctx.kitchen_process_identity + assert second_identity is not None + assert second_identity.kitchen_id == second_id - loaded = json.loads(await load_recipe(name="research", overrides=overrides)) - assert loaded["success"] is True, loaded + loaded = json.loads(await load_recipe(name="research", overrides=overrides)) + assert loaded["success"] is True, loaded - _close_kitchen_handler() - await asyncio.sleep(0) + _close_kitchen_handler() + await asyncio.sleep(0) + + +@pytest.mark.parametrize("campaign_id", ["campaign-5270"], indirect=True) +async def test_same_campaign_reopen_issues_fresh_recipe_authority( + campaign_id, + real_kitchen_lifecycle, +): + import json + + from autoskillit.fleet import ( + DispatchRecord, + DispatchStatus, + read_all_campaign_captures, + write_captured_values, + write_initial_state, + ) + from autoskillit.pipeline import ReadyRecipe + from autoskillit.server.recipe._recipe_execution import ( + RecipeExecutionAdmissionError, + bind_attested_runtime_invocation, + ) + from autoskillit.server.tools.tools_kitchen import open_kitchen + from autoskillit.server.tools.tools_recipe import ( + complete_recipe_initialization, + get_recipe_section, + ) + from tests.server._helpers import _credit_initialization_sections, _resolve_recipe_section + + ctx = real_kitchen_lifecycle.ctx + overrides = real_kitchen_lifecycle.overrides + fmcp_ctx = real_kitchen_lifecycle.fmcp_ctx + + first = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert first["success"] is True, first + await _credit_initialization_sections(first) + first_state = ctx.recipe_initialization_state + assert isinstance(first_state, ReadyRecipe) + first_execution_id = first_state.installed_execution.snapshot.execution_id + + prior_state = ctx.temp_dir / "dispatches" / "prior.json" + write_initial_state( + prior_state, + campaign_id=ctx.kitchen_id, + campaign_name="c", + manifest_path="", + dispatches=[DispatchRecord(name="a", status=DispatchStatus.SUCCESS)], + ) + write_captured_values(prior_state, {"k": "v"}) + + _close_kitchen_handler() + recipe_root = ctx.temp_dir / "recipe-delivery" + retired_markers = list((recipe_root / ".retired").glob("*.retired")) + assert len(retired_markers) == 1 + retired_marker = retired_markers[0] + + second = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert second["success"] is True, second + assert ctx.kitchen_id == campaign_id + await _credit_initialization_sections(second) + second_state = ctx.recipe_initialization_state + assert isinstance(second_state, ReadyRecipe) + assert await _resolve_recipe_section(second, section="content") is not None + second_execution_id = second_state.installed_execution.snapshot.execution_id + assert second_execution_id != first_execution_id + assert second["initialization_id"] != first["initialization_id"] + + stale_identity = { + key: value for key, value in first["recipe_pull"].items() if key != "pull_tool" + } + stale_pull = json.loads(await get_recipe_section(section="content", **stale_identity)) + assert stale_pull == { + "success": False, + "error": "invalid_recipe_artifact_identity", + } + stale_completion = json.loads( + await complete_recipe_initialization(initialization_id=first["initialization_id"]) + ) + assert stale_completion == { + "success": False, + "error": "recipe_initialization_stale", + } + + installed = second_state.installed_execution + with pytest.raises(RecipeExecutionAdmissionError) as error: + bind_attested_runtime_invocation( + installed, + execution_id=first_execution_id, + step_name="scope", + template_digest="", + skill_command="", + skill_inputs=None, + actual_mcp_kwargs={}, + ) + assert error.value.code == "recipe_execution_id_mismatch" + + assert retired_marker.exists() + active_namespaces = [ + path for path in recipe_root.iterdir() if path.is_dir() and path.name != ".retired" + ] + assert len(active_namespaces) == 1 + assert active_namespaces[0].name != retired_marker.stem + assert read_all_campaign_captures(ctx.temp_dir / "dispatches", ctx.kitchen_id) == {"k": "v"} + _close_kitchen_handler() + + +@pytest.mark.parametrize("campaign_id", ["campaign-5270"], indirect=True) +async def test_same_campaign_anonymous_reopen_then_load_recipe_and_get_recipe( + campaign_id, + real_kitchen_lifecycle, +): + import json + + from autoskillit.server.tools.tools_kitchen import get_recipe, open_kitchen + from autoskillit.server.tools.tools_recipe import load_recipe + from tests.server._helpers import _resolve_recipe_section + + overrides = real_kitchen_lifecycle.overrides + fmcp_ctx = real_kitchen_lifecycle.fmcp_ctx + + named = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert named["success"] is True, named + _close_kitchen_handler() + + anonymous = json.loads(await open_kitchen(ctx=fmcp_ctx)) + assert anonymous["success"] is True, anonymous + assert real_kitchen_lifecycle.ctx.kitchen_id == campaign_id + loaded = json.loads(await load_recipe(name="research", overrides=overrides)) + assert loaded["success"] is True, loaded + resource = json.loads(get_recipe("research")) + assert "error" not in resource, resource + assert await _resolve_recipe_section(resource) is not None + _close_kitchen_handler() + + +@pytest.mark.parametrize( + "campaign_id", + [None, "campaign-5270"], + indirect=True, + ids=["ephemeral", "campaign"], +) +async def test_reopen_with_new_arguments_stays_in_one_activation( + campaign_id, + real_kitchen_lifecycle, +): + import json + + from autoskillit.server.tools.tools_kitchen import open_kitchen + from tests.server._helpers import _resolve_recipe_section + + ctx = real_kitchen_lifecycle.ctx + fmcp_ctx = real_kitchen_lifecycle.fmcp_ctx + overrides = real_kitchen_lifecycle.overrides + + first = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert first["success"] is True, first + first_id = ctx.kitchen_id + second_overrides = {**overrides, "task_description": "changed task"} + second = json.loads( + await open_kitchen(name="research", overrides=second_overrides, ctx=fmcp_ctx) + ) + assert second["success"] is True, second + assert ctx.kitchen_id == first_id + if campaign_id is not None: + assert ctx.kitchen_id == campaign_id + assert await _resolve_recipe_section(first, section="content") is not None + + _close_kitchen_handler() + recipe_root = ctx.temp_dir / "recipe-delivery" + namespaces = [ + path for path in recipe_root.iterdir() if path.is_dir() and path.name != ".retired" + ] + assert namespaces == [] + assert len(list((recipe_root / ".retired").glob("*.retired"))) == 1 + + +@pytest.mark.parametrize("campaign_id", ["campaign-5270"], indirect=True) +async def test_sibling_contexts_sharing_campaign_and_temp_dir_are_isolated( + monkeypatch, + campaign_id, + real_kitchen_lifecycle, +): + import json + + from autoskillit.server.tools.tools_kitchen import open_kitchen + from autoskillit.server.tools.tools_recipe import get_recipe_section + + ctx_a = real_kitchen_lifecycle.ctx + ctx_b = real_kitchen_lifecycle.new_context() + assert ctx_b.temp_dir == ctx_a.temp_dir + overrides = real_kitchen_lifecycle.overrides + fmcp_ctx = real_kitchen_lifecycle.fmcp_ctx + + first = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert first["success"] is True, first + monkeypatch.setattr(_state, "_ctx", ctx_b) + second = json.loads(await open_kitchen(name="research", overrides=overrides, ctx=fmcp_ctx)) + assert second["success"] is True, second + assert ctx_a.kitchen_id == ctx_b.kitchen_id == campaign_id + + monkeypatch.setattr(_state, "_ctx", ctx_a) + _close_kitchen_handler() + monkeypatch.setattr(_state, "_ctx", ctx_b) + b_identity = {key: value for key, value in second["recipe_pull"].items() if key != "pull_tool"} + pulled = json.loads(await get_recipe_section(section="content", **b_identity)) + assert pulled["success"] is True, pulled + + recipe_root = ctx_b.temp_dir / "recipe-delivery" + active_namespaces = [ + path for path in recipe_root.iterdir() if path.is_dir() and path.name != ".retired" + ] + assert len(active_namespaces) == 1 + retired_markers = list((recipe_root / ".retired").glob("*.retired")) + assert all(marker.stem != active_namespaces[0].name for marker in retired_markers) + _close_kitchen_handler() def _write_tracker(tracker_dir, kitchen_id, *, initialized_at=None): diff --git a/tests/server/test_recipe_generation.py b/tests/server/test_recipe_generation.py index fe246fc823..2ae43575c1 100644 --- a/tests/server/test_recipe_generation.py +++ b/tests/server/test_recipe_generation.py @@ -1,4 +1,4 @@ -"""Focused contracts for the kitchen-scoped recipe generation store.""" +"""Focused contracts for the recipe generation store scoped by activation.""" from __future__ import annotations @@ -27,6 +27,7 @@ RecipeFlowGeneration, compute_recipe_execution_snapshot_digest, ) +from autoskillit.pipeline import KitchenActivationId from autoskillit.server.recipe._recipe_generation import ( RecipeGenerationCapacityError, RecipeGenerationConflictError, @@ -35,7 +36,7 @@ RecipeGenerationStore, recipe_generation_weight_bytes, ) -from autoskillit.server.recipe.section._lifecycle import notify_kitchen_retired +from autoskillit.server.recipe.section._lifecycle import notify_activation_retired pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -100,14 +101,14 @@ def _flow_generation(*, entrypoint: str = "step") -> RecipeFlowGeneration: def _record( *, - kitchen_id: str = "kitchen-a", + activation_id: KitchenActivationId = KitchenActivationId("kitchen-a"), compile_key: str = "compile-a", recipe_name: str = "recipe", payload_marker: str = "a", ) -> RecipeGenerationRecord: execution_id = f"execution-{compile_key}" return RecipeGenerationRecord( - kitchen_id=kitchen_id, + activation_id=activation_id, normalized_compile_key=compile_key, recipe_name=recipe_name, finalized_projection=_projection(), @@ -146,9 +147,19 @@ def _generation( ) -def test_record_rejects_empty_kitchen_id() -> None: - with pytest.raises(ValueError, match="kitchen_id must be a non-empty string"): - _record(kitchen_id="") +def test_record_rejects_bare_activation_id() -> None: + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + _record(activation_id="k") # type: ignore[arg-type] + + +def test_generation_store_lookups_reject_bare_activation_strings() -> None: + store = RecipeGenerationStore() + artifact = _generation("wrong-activation-type") + + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + store.lookup_compile("k", "key") + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + store.lookup_artifact("k", artifact) def test_surface_binding_rejects_a_different_flow_generation() -> None: @@ -158,7 +169,7 @@ def test_surface_binding_rejects_a_different_flow_generation() -> None: with pytest.raises(ValueError, match="must match the record flow generation"): store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", _generation( @@ -182,7 +193,7 @@ def test_record_owns_canonical_immutable_primitive_copies() -> None: store = RecipeGenerationStore() stored = store.put(record) - fetched = store.lookup_compile("kitchen-a", "compile-a") + fetched = store.lookup_compile(KitchenActivationId("kitchen-a"), "compile-a") assert fetched is not None assert fetched is not stored @@ -214,13 +225,13 @@ def test_surface_binding_is_exact_idempotent_and_dually_indexed() -> None: artifact = _generation("one") bound = store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", artifact, ) replay = store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", artifact, @@ -228,10 +239,10 @@ def test_surface_binding_is_exact_idempotent_and_dually_indexed() -> None: assert replay == bound assert bound.surface_bindings == {"open_kitchen": artifact} - assert store.lookup_artifact(record.kitchen_id, artifact) == bound + assert store.lookup_artifact(record.activation_id, artifact) == bound with pytest.raises(RecipeGenerationConflictError): store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", _generation("different"), @@ -244,7 +255,7 @@ def test_artifact_generation_cannot_alias_another_compile_generation() -> None: second = store.put(_record(compile_key="second")) artifact = _generation("shared") store.bind_surface( - first.kitchen_id, + first.activation_id, first.normalized_compile_key, "open_kitchen", artifact, @@ -252,7 +263,7 @@ def test_artifact_generation_cannot_alias_another_compile_generation() -> None: with pytest.raises(RecipeGenerationConflictError): store.bind_surface( - second.kitchen_id, + second.activation_id, second.normalized_compile_key, "load_recipe", artifact, @@ -271,7 +282,7 @@ def test_entry_lru_eviction_removes_every_artifact_alias() -> None: strict=True, ): store.bind_surface( - first.kitchen_id, + first.activation_id, first.normalized_compile_key, surface, artifact, @@ -282,23 +293,23 @@ def test_entry_lru_eviction_removes_every_artifact_alias() -> None: strict=True, ): store.bind_surface( - second.kitchen_id, + second.activation_id, second.normalized_compile_key, surface, artifact, ) for artifact in first_artifacts: - assert store.lookup_artifact(first.kitchen_id, artifact) is not None + assert store.lookup_artifact(first.activation_id, artifact) is not None store.put(_record(compile_key="third")) - assert store.lookup_compile(second.kitchen_id, second.normalized_compile_key) is None + assert store.lookup_compile(second.activation_id, second.normalized_compile_key) is None for artifact in second_artifacts: - assert store.lookup_artifact(second.kitchen_id, artifact) is None - retained = store.lookup_compile(first.kitchen_id, first.normalized_compile_key) + assert store.lookup_artifact(second.activation_id, artifact) is None + retained = store.lookup_compile(first.activation_id, first.normalized_compile_key) assert retained is not None for artifact in first_artifacts: - assert store.lookup_artifact(first.kitchen_id, artifact) == retained + assert store.lookup_artifact(first.activation_id, artifact) == retained def test_canonical_weight_drives_byte_admission_and_lru_eviction() -> None: @@ -314,8 +325,8 @@ def test_canonical_weight_drives_byte_admission_and_lru_eviction() -> None: store.put(first) store.put(second) - assert store.lookup_compile(first.kitchen_id, first.normalized_compile_key) is None - assert store.lookup_compile(second.kitchen_id, second.normalized_compile_key) is not None + assert store.lookup_compile(first.activation_id, first.normalized_compile_key) is None + assert store.lookup_compile(second.activation_id, second.normalized_compile_key) is not None assert store.weight_bytes == second_weight too_small = RecipeGenerationStore( @@ -335,16 +346,16 @@ def test_retirement_callback_removes_kitchen_and_permanently_rejects_writes( record = store.put(_record()) artifact = _generation("retired") store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", artifact, ) - notify_kitchen_retired(record.kitchen_id) + notify_activation_retired(record.activation_id) - assert store.lookup_compile(record.kitchen_id, record.normalized_compile_key) is None - assert store.lookup_artifact(record.kitchen_id, artifact) is None + assert store.lookup_compile(record.activation_id, record.normalized_compile_key) is None + assert store.lookup_artifact(record.activation_id, artifact) is None assert store.entry_count == 0 assert store.weight_bytes == 0 with pytest.raises(RecipeGenerationRetiredError): @@ -356,20 +367,20 @@ def test_authoritative_store_rejects_never_activated_kitchen( write: str, ) -> None: from autoskillit.server.recipe._recipe_generation import ( - RecipeGenerationKitchenNotActiveError, + RecipeGenerationActivationNotActiveError, ) store = RecipeGenerationStore() - active = _record(kitchen_id="A") - store.activate_kitchen(active.kitchen_id) + active = _record(activation_id=KitchenActivationId("A")) + store.activate_activation(active.activation_id) store.put(active) - with pytest.raises(RecipeGenerationKitchenNotActiveError, match="B") as error: + with pytest.raises(RecipeGenerationActivationNotActiveError, match="B") as error: if write == "put": - store.put(_record(kitchen_id="B")) + store.put(_record(activation_id=KitchenActivationId("B"))) else: store.bind_surface( - "B", + KitchenActivationId("B"), active.normalized_compile_key, "open_kitchen", _generation("never-active"), @@ -381,30 +392,30 @@ def test_authoritative_store_rejects_never_activated_kitchen( def test_lifecycle_authority_bounds_retirement_history_without_stale_reactivation() -> None: - store = RecipeGenerationStore(max_retired_kitchens=1) - first = _record(kitchen_id="first-kitchen", compile_key="first") - second = _record(kitchen_id="second-kitchen", compile_key="second") + store = RecipeGenerationStore(max_retired_activations=1) + first = _record(activation_id=KitchenActivationId("first-kitchen"), compile_key="first") + second = _record(activation_id=KitchenActivationId("second-kitchen"), compile_key="second") - store.activate_kitchen(first.kitchen_id) + store.activate_activation(first.activation_id) store.put(first) - store.retire_kitchen(first.kitchen_id) + store.retire_activation(first.activation_id) with pytest.raises(RecipeGenerationRetiredError): store.put(first) - store.activate_kitchen(second.kitchen_id) + store.activate_activation(second.activation_id) store.put(second) - store.retire_kitchen(second.kitchen_id) + store.retire_activation(second.activation_id) - assert store.retired_kitchen_count == 1 + assert store.retired_activation_count == 1 from autoskillit.server.recipe._recipe_generation import ( - RecipeGenerationKitchenNotActiveError, + RecipeGenerationActivationNotActiveError, ) - with pytest.raises(RecipeGenerationKitchenNotActiveError, match="not active") as error: + with pytest.raises(RecipeGenerationActivationNotActiveError, match="not active") as error: store.put(first) assert "retired" not in str(error.value) - store.activate_kitchen(first.kitchen_id) - assert store.put(first).kitchen_id == first.kitchen_id + store.activate_activation(first.activation_id) + assert store.put(first).activation_id == first.activation_id def test_concurrent_conflicting_surface_bind_has_one_winner() -> None: @@ -417,7 +428,7 @@ def bind(artifact: RecipeArtifactGeneration) -> str: barrier.wait() try: store.bind_surface( - record.kitchen_id, + record.activation_id, record.normalized_compile_key, "open_kitchen", artifact, @@ -430,7 +441,7 @@ def bind(artifact: RecipeArtifactGeneration) -> str: outcomes = list(executor.map(bind, generations)) assert sorted(outcomes) == ["bound", "conflict"] - stored = store.lookup_compile(record.kitchen_id, record.normalized_compile_key) + stored = store.lookup_compile(record.activation_id, record.normalized_compile_key) assert stored is not None winner = stored.surface_bindings["open_kitchen"] - assert store.lookup_artifact(record.kitchen_id, winner) == stored + assert store.lookup_artifact(record.activation_id, winner) == stored diff --git a/tests/server/test_recipe_section_pagination_cache.py b/tests/server/test_recipe_section_pagination_cache.py index cc54bded1a..b5681b75c2 100644 --- a/tests/server/test_recipe_section_pagination_cache.py +++ b/tests/server/test_recipe_section_pagination_cache.py @@ -14,6 +14,7 @@ import pytest +from autoskillit.pipeline import KitchenActivationId from autoskillit.server.recipe import _recipe_section_pagination as pagination from autoskillit.server.recipe import _recipe_section_planning as planning from autoskillit.server.recipe._recipe_section_pagination import ( @@ -42,7 +43,7 @@ def test_cached_plans_are_reused_and_cache_clear_forces_a_rebuild( ) -> None: selected = select_recipe_section(_payload(content="cache me " * 500), "content") kwargs: dict[str, Any] = { - "kitchen_id": "kitchen-test", + "activation_id": KitchenActivationId("kitchen-test"), "generation": _generation(), "selected": selected, "recipe_section_bound_bytes": _PAGE_TEST_BOUND, @@ -74,7 +75,7 @@ def test_concurrent_same_key_requests_share_one_page_plan_build( ) -> None: selected = select_recipe_section(_payload(content="single flight"), "content") kwargs: dict[str, Any] = { - "kitchen_id": "kitchen-single-flight", + "activation_id": KitchenActivationId("kitchen-single-flight"), "generation": _generation(), "selected": selected, "recipe_section_bound_bytes": _PAGE_TEST_BOUND, @@ -113,7 +114,7 @@ def test_retirement_during_build_prevents_stale_cache_admission( selected = select_recipe_section(_payload(content="retirement race"), "content") generation = _generation() kwargs: dict[str, Any] = { - "kitchen_id": "kitchen-retirement-race", + "activation_id": KitchenActivationId("kitchen-retirement-race"), "generation": generation, "selected": selected, "recipe_section_bound_bytes": _PAGE_TEST_BOUND, @@ -130,15 +131,15 @@ def blocked_builder(**builder_kwargs: object) -> Any: assert release_build.wait(timeout=5) return real_builder(**builder_kwargs) # type: ignore[arg-type] - def retire_kitchen() -> None: + def retire_activation() -> None: retirement_started.set() - cache.evict_kitchen("kitchen-retirement-race") + cache.evict_activation(KitchenActivationId("kitchen-retirement-race")) monkeypatch.setattr(pagination, "build_recipe_section_page_plan", blocked_builder) with ThreadPoolExecutor(max_workers=2) as executor: build_future = executor.submit(get_or_build_recipe_section_page_plan, **kwargs) build_started.wait(timeout=5) - retirement_future = executor.submit(retire_kitchen) + retirement_future = executor.submit(retire_activation) assert retirement_started.wait(timeout=5) release_build.set() plan = build_future.result(timeout=5) @@ -168,7 +169,7 @@ def test_every_cache_key_dimension_prevents_aliasing( ) -> None: baseline_selected = select_recipe_section(_payload(content="same"), "content") kwargs: dict[str, Any] = { - "kitchen_id": "kitchen-a", + "activation_id": KitchenActivationId("kitchen-a"), "generation": _generation(), "selected": baseline_selected, "recipe_section_bound_bytes": _PAGE_TEST_BOUND, @@ -176,7 +177,7 @@ def test_every_cache_key_dimension_prevents_aliasing( baseline = get_or_build_recipe_section_page_plan(**kwargs) if dimension == "kitchen": - kwargs["kitchen_id"] = "kitchen-b" + kwargs["activation_id"] = KitchenActivationId("kitchen-b") elif dimension == "generation": kwargs["generation"] = replace(_generation(), body_size_bytes=2049) elif dimension == "section": @@ -234,21 +235,21 @@ def test_cache_entry_limit_evicts_oldest_plan() -> None: selected = select_recipe_section(_payload(content="entry eviction"), "content") generation = _generation() first = get_or_build_recipe_section_page_plan( - kitchen_id="kitchen-0", + activation_id=KitchenActivationId("kitchen-0"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, ) for index in range(1, pagination.PAGE_PLAN_CACHE_MAX_ENTRIES + 1): get_or_build_recipe_section_page_plan( - kitchen_id=f"kitchen-{index}", + activation_id=KitchenActivationId(f"kitchen-{index}"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, ) rebuilt = get_or_build_recipe_section_page_plan( - kitchen_id="kitchen-0", + activation_id=KitchenActivationId("kitchen-0"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -262,7 +263,7 @@ def test_cache_rejects_a_single_plan_over_its_byte_limit() -> None: generation = _generation() plan = _build(_payload(content="overweight"), "content", bound=_PAGE_TEST_BOUND) key = pagination._cache_key( - kitchen_id="kitchen-overweight", + activation_id=KitchenActivationId("kitchen-overweight"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -284,7 +285,7 @@ def test_cache_byte_limit_evicts_oldest_plan() -> None: ) keys = [ pagination._cache_key( - kitchen_id=f"kitchen-{index}", + activation_id=KitchenActivationId(f"kitchen-{index}"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -310,13 +311,13 @@ def test_cache_replacement_subtracts_the_previous_plan_weight() -> None: replacement = dataclasses.replace(base_plan, cache_weight_bytes=3) other = dataclasses.replace(base_plan, cache_weight_bytes=7) key = pagination._cache_key( - kitchen_id="kitchen-replacement", + activation_id=KitchenActivationId("kitchen-replacement"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, ) other_key = pagination._cache_key( - kitchen_id="kitchen-other", + activation_id=KitchenActivationId("kitchen-other"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -341,7 +342,7 @@ def test_cache_kitchen_eviction_subtracts_every_matching_plan_weight() -> None: ) retired_keys = [ pagination._cache_key( - kitchen_id="retired-kitchen", + activation_id=KitchenActivationId("retired-kitchen"), generation=generation, selected=selected, recipe_section_bound_bytes=bound, @@ -349,7 +350,7 @@ def test_cache_kitchen_eviction_subtracts_every_matching_plan_weight() -> None: for bound in (_PAGE_TEST_BOUND, _PAGE_TEST_BOUND + 1) ] retained_key = pagination._cache_key( - kitchen_id="retained-kitchen", + activation_id=KitchenActivationId("retained-kitchen"), generation=generation, selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -358,7 +359,7 @@ def test_cache_kitchen_eviction_subtracts_every_matching_plan_weight() -> None: for key in (*retired_keys, retained_key): cache.put(key, plan) - cache.evict_kitchen("retired-kitchen") + cache.evict_activation(KitchenActivationId("retired-kitchen")) assert all(cache.get(key) is None for key in retired_keys) assert cache.get(retained_key) is plan diff --git a/tests/server/test_recipe_section_pagination_invariants.py b/tests/server/test_recipe_section_pagination_invariants.py index e13f5ed673..5b8cfb3dfa 100644 --- a/tests/server/test_recipe_section_pagination_invariants.py +++ b/tests/server/test_recipe_section_pagination_invariants.py @@ -254,7 +254,6 @@ def test_cross_process_plan_and_rendering_are_deterministic() -> None: flow_record_count=2, ) plan = build_recipe_section_page_plan( - kitchen_id="kitchen-test", generation=generation, selected=select_recipe_section(payload, "warnings"), recipe_section_bound_bytes=2000, diff --git a/tests/server/test_recipe_section_pagination_planning.py b/tests/server/test_recipe_section_pagination_planning.py index 9f7328e179..03956f03b5 100644 --- a/tests/server/test_recipe_section_pagination_planning.py +++ b/tests/server/test_recipe_section_pagination_planning.py @@ -182,7 +182,6 @@ def test_terminal_initialization_page_carries_progress_and_completion_receipt() }, ) plan = build_recipe_section_page_plan( - kitchen_id="kitchen-test", generation=generation, selected=selected, recipe_section_bound_bytes=90_000, @@ -213,7 +212,6 @@ def test_char_ceiling_accepts_a_page_within_it() -> None: ) replan = build_recipe_section_page_plan( - kitchen_id="kitchen-test", generation=_generation(), selected=select_recipe_section( _payload(orchestration_rules="follow the graph exactly"), "orchestration_rules" @@ -243,7 +241,6 @@ def test_char_ceiling_rejects_a_page_that_exceeds_it() -> None: with pytest.raises(RecipeSectionBoundError, match="cannot fit progress"): build_recipe_section_page_plan( - kitchen_id="kitchen-test", generation=_generation(), selected=select_recipe_section(payload, "orchestration_rules"), recipe_section_bound_bytes=_PAGE_TEST_BOUND, @@ -324,7 +321,6 @@ def test_string_scalar_strategy_rejects_non_string_values() -> None: with pytest.raises((TypeError, ValueError), match="string"): build_recipe_section_page_plan( - kitchen_id="kitchen-test", generation=_generation(), selected=selected, recipe_section_bound_bytes=_PAGE_TEST_BOUND, diff --git a/tests/server/test_recipe_segment_delivery.py b/tests/server/test_recipe_segment_delivery.py index a7b335617c..e4460b4e4e 100644 --- a/tests/server/test_recipe_segment_delivery.py +++ b/tests/server/test_recipe_segment_delivery.py @@ -15,7 +15,7 @@ FinalizedRecipeStep, RecipeBindingProjection, ) -from autoskillit.pipeline import NoActiveRecipe, ReadyRecipe +from autoskillit.pipeline import KitchenActivationId, NoActiveRecipe, ReadyRecipe from autoskillit.server.recipe._recipe_artifact import ( _finalized_projection_payload, _normalized_recipe_compile_identity, @@ -127,7 +127,7 @@ def _install_segmented_ready(ready_context): ) persisted = load_recipe_artifact( tool_ctx.temp_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, identity=state.artifact_generation, ) persisted["finalized_recipe_projection"] = _finalized_projection_payload(projection) @@ -141,7 +141,7 @@ def _install_segmented_ready(ready_context): ) artifact = persist_recipe_artifact( tool_ctx.temp_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, producer_tool=state.artifact_generation.producer_tool, recipe_name=state.recipe_name, payload=persisted, @@ -223,7 +223,7 @@ async def test_persisted_generation_recomputes_original_compile_identity( assert isinstance(state, ReadyRecipe) persisted = load_recipe_artifact( tool_ctx.temp_dir, - kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, identity=state.artifact_generation, ) @@ -298,7 +298,7 @@ async def test_checkpoint_delivery_survives_ready_generation_lru_eviction( original_state = tool_ctx_ready_recipe.tool_ctx.recipe_initialization_state assert isinstance(original_state, ReadyRecipe) base_record = original_store.lookup_compile( - original_state.kitchen_id, + tool_ctx_ready_recipe.tool_ctx.kitchen_activation_id, original_state.generation_store_key, ) assert base_record is not None @@ -316,12 +316,18 @@ async def test_checkpoint_delivery_survives_ready_generation_lru_eviction( store.put( replace( ready_record, - kitchen_id=f"unrelated-kitchen-{index}", + activation_id=KitchenActivationId(f"unrelated-kitchen-{index}"), normalized_compile_key=f"unrelated-compile-{index}", surface_bindings={}, ) ) - assert store.lookup_compile(state.kitchen_id, state.generation_store_key) is None + assert ( + store.lookup_compile( + tool_ctx_ready_recipe.tool_ctx.kitchen_activation_id, + state.generation_store_key, + ) + is None + ) prepared = prepare_recipe_segment_delivery(tool_ctx_ready_recipe.tool_ctx, "scope") diff --git a/tests/server/test_serve_idempotence.py b/tests/server/test_serve_idempotence.py index 9d3bcb9355..07a5f71cc8 100644 --- a/tests/server/test_serve_idempotence.py +++ b/tests/server/test_serve_idempotence.py @@ -345,7 +345,7 @@ async def test_all_surfaces_share_canonical_flow_artifact_and_execution_identity pull_identity = dict(response["recipe_pull"]) pull_identity.pop("pull_tool") record = get_recipe_generation_store().lookup_artifact( - tool_ctx_kitchen_open.kitchen_id, + tool_ctx_kitchen_open.kitchen_activation_id, RecipeArtifactGeneration(**pull_identity), ) assert record is not None diff --git a/tests/server/test_tools_execution_write_prefix.py b/tests/server/test_tools_execution_write_prefix.py index 5bc532aa3e..01bfa5c80d 100644 --- a/tests/server/test_tools_execution_write_prefix.py +++ b/tests/server/test_tools_execution_write_prefix.py @@ -141,6 +141,47 @@ def test_non_worktree_skill_no_worktree_prefix(tmp_path: Path) -> None: assert worktree_parent_str not in all_prefixes +def test_smoke_recipe_grants_only_its_owned_worktree(tmp_path: Path) -> None: + from types import SimpleNamespace + + from autoskillit.core import SkillSource, load_yaml + from autoskillit.server.tools.tools_execution._run_skill_session import ( + _extend_closure_write_scope, + _prepare_owned_write_scope, + _resolve_dispatch_paths, + ) + from autoskillit.workspace.skills import _skill_info_from_frontmatter + + repo_root = Path(__file__).resolve().parents[2] + recipe = load_yaml(repo_root / "scripts/e2e/recipes/sandbox-smoke.yaml") + worker = recipe["steps"]["implement"]["with"] + clone_root, worktree = _make_worktree_layout(tmp_path) + info = _skill_info_from_frontmatter( + "smoke-task", + SkillSource.BUNDLED_EXTENDED, + repo_root / "src/autoskillit/skills_extended/smoke-task/SKILL.md", + ) + assert not info.invalidities + state = SimpleNamespace( + cwd=str(worktree), + output_dir=worker["output_dir"].replace("${{ context.worktree_path }}", str(worktree)), + skill_command="/autoskillit:smoke-task write the canary", + target_name="smoke-task", + closure_spec="", + invocation=SimpleNamespace(root=info, closure=(info,)), + is_read_only=False, + ) + + assert _resolve_dispatch_paths(state, base_cwd=worktree) is None + assert _extend_closure_write_scope(state) is None + assert _prepare_owned_write_scope(state) is None + assert scope_covers_cwd(state.allowed_write_prefixes, str(worktree / "sandbox/text.py")) + assert scope_covers_cwd( + state.allowed_write_prefixes, str(worktree / "tests/test_smoke_canary.py") + ) + assert not scope_covers_cwd(state.allowed_write_prefixes, str(clone_root)) + + # --------------------------------------------------------------------------- # AC2: dispatch preflight tests # --------------------------------------------------------------------------- diff --git a/tests/server/test_tools_recipe_pull.py b/tests/server/test_tools_recipe_pull.py index b536679250..e8fb86c1fd 100644 --- a/tests/server/test_tools_recipe_pull.py +++ b/tests/server/test_tools_recipe_pull.py @@ -46,6 +46,7 @@ from autoskillit.execution.backends import ClaudeCodeBackend, CodexBackend from autoskillit.pipeline import ( InitializingRecipe, + KitchenActivationId, KitchenEffectPhase, KitchenRetryDisposition, ReadyRecipe, @@ -62,6 +63,7 @@ _generation_dir, _generation_from_payload, _safe_component, + retire_recipe_artifacts, ) from autoskillit.server.recipe._recipe_delivery import ( RECIPE_BODY_END, @@ -78,7 +80,6 @@ prepare_recipe_delivery_generation, recipe_pull_producers, recipe_recreation_producers, - retire_recipe_artifacts, ) from autoskillit.server.recipe._recipe_section_pagination import ( PagePlanCache, @@ -286,11 +287,42 @@ def _persist_finalized_generation( return finalized.artifact_generation, finalized +def test_finalize_persistence_failure_logs_cause(tool_ctx_kitchen_open, monkeypatch) -> None: + from structlog.testing import capture_logs + + from autoskillit.server.recipe._recipe_delivery import _finalize + + monkeypatch.setattr( + _finalize, + "persist_recipe_artifact", + MagicMock(side_effect=RecipeArtifactError("boom")), + ) + with capture_logs() as logs: + finalized = _finalize_recipe_delivery( + _payload(), + surface="open_kitchen", + recipe_name="remediation", + tool_ctx=tool_ctx_kitchen_open, + finalized_projection=_test_projection(), + ) + + assert json.loads(finalized.rendered) == { + "success": False, + "error": "recipe_artifact_unavailable", + } + warnings = [log for log in logs if log["event"] == "recipe_artifact_persistence_failed"] + assert len(warnings) == 1 + assert warnings[0]["log_level"] == "warning" + assert warnings[0]["error_type"] == "RecipeArtifactError" + assert warnings[0]["exc_info"] is True + + def test_finalized_response_rejects_stale_kitchen_transition_owner(tool_ctx) -> None: bind_test_kitchen_identity(tool_ctx, kitchen_id="transition-owner") tool_ctx.kitchen_open_state = start_kitchen_effect( new_kitchen_open_state( kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, context_id="context-1", operation_id="operation-1", ), @@ -308,6 +340,7 @@ def test_finalized_response_rejects_stale_kitchen_transition_owner(tool_ctx) -> tool_ctx.kitchen_open_state = start_kitchen_effect( new_kitchen_open_state( kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, context_id="context-1", operation_id="operation-2", ), @@ -327,6 +360,7 @@ def test_finalized_response_confirms_owned_kitchen_serving_effect(tool_ctx) -> N tool_ctx.kitchen_open_state = start_kitchen_effect( new_kitchen_open_state( kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, context_id="context-1", operation_id="operation-1", ), @@ -361,6 +395,7 @@ def test_finalized_response_marks_changed_kitchen_serving_effect_ambiguous( tool_ctx.kitchen_open_state = start_kitchen_effect( new_kitchen_open_state( kitchen_id=tool_ctx.kitchen_id, + activation_id=tool_ctx.kitchen_activation_id, context_id="context-1", operation_id="operation-1", ), @@ -397,12 +432,12 @@ def _persist( payload: dict[str, object] | None = None, *, producer: str = "open_kitchen", - kitchen_id: str = "kitchen-test", + activation_id: KitchenActivationId = KitchenActivationId("kitchen-test"), ) -> RecipeArtifactGeneration: persisted_payload, flow_generation = _with_flow(dict(payload or _payload())) return persist_recipe_artifact( tmp_path, - kitchen_id=kitchen_id, + activation_id=activation_id, producer_tool=producer, recipe_name="remediation", payload=persisted_payload, @@ -414,7 +449,7 @@ def _write_malformed_generation( tmp_path: Path, payload: dict[str, object], *, - kitchen_id: str = "kitchen-test", + activation_id: KitchenActivationId = KitchenActivationId("kitchen-test"), producer: str = "open_kitchen", ) -> RecipeArtifactGeneration: """Write a digest-consistent artifact while bypassing producer validation.""" @@ -429,7 +464,7 @@ def _write_malformed_generation( ) directory = _generation_dir( tmp_path, - kitchen_id=kitchen_id, + activation_id=activation_id, producer_tool=producer, recipe_name="remediation", descriptor_version=generation.descriptor_version, @@ -504,6 +539,31 @@ def test_persistence_rejects_malformed_pullable_sections( _persist(tmp_path, payload) +def test_recipe_artifact_namespace_rejects_lineage_string_before_schema_checks( + tmp_path: Path, +) -> None: + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + persist_recipe_artifact( + tmp_path, + activation_id="kitchen-test", + producer_tool="open_kitchen", + recipe_name="remediation", + payload={}, + ) + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + retire_recipe_artifacts(tmp_path, activation_id="kitchen-test") + assert not (tmp_path / "recipe-delivery").exists() + + +def test_recipe_artifact_load_rejects_lineage_string_before_reading(tmp_path: Path) -> None: + identity = _persist(tmp_path, _payload()) + shutil.rmtree(tmp_path / "recipe-delivery") + + with pytest.raises(TypeError, match="a KitchenActivationId is required"): + load_recipe_artifact(tmp_path, activation_id="kitchen-test", identity=identity) + assert not (tmp_path / "recipe-delivery").exists() + + def test_persistence_rejects_missing_required_content(tmp_path: Path) -> None: payload = _payload() payload.pop("content") @@ -547,31 +607,34 @@ def test_load_rejects_digest_consistent_malformed_artifact(tmp_path: Path) -> No with pytest.raises(RecipeArtifactSchemaError): load_recipe_artifact( tmp_path, - kitchen_id="kitchen-test", + activation_id=KitchenActivationId("kitchen-test"), identity=generation, ) -def test_artifact_namespace_encodes_colliding_kitchen_ids_injectively(tmp_path: Path) -> None: +def test_artifact_namespace_encodes_colliding_activation_ids_injectively(tmp_path: Path) -> None: first = persist_recipe_artifact( tmp_path, - kitchen_id="a/b", + activation_id=KitchenActivationId("a/b"), producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), ) second = persist_recipe_artifact( tmp_path, - kitchen_id="a?b", + activation_id=KitchenActivationId("a?b"), producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), ) - assert retire_recipe_artifacts(tmp_path, kitchen_id="a/b") is True + assert retire_recipe_artifacts(tmp_path, activation_id=KitchenActivationId("a/b")) is True with pytest.raises(RecipeArtifactError): - load_recipe_artifact(tmp_path, kitchen_id="a/b", identity=first) - assert load_recipe_artifact(tmp_path, kitchen_id="a?b", identity=second) == _payload() + load_recipe_artifact(tmp_path, activation_id=KitchenActivationId("a/b"), identity=first) + assert ( + load_recipe_artifact(tmp_path, activation_id=KitchenActivationId("a?b"), identity=second) + == _payload() + ) def test_artifact_persistence_rejects_blob_above_read_ceiling(tmp_path: Path) -> None: @@ -600,8 +663,8 @@ def test_shared_producer_surfaces_require_identical_pull_policies( recipe_recreation_producers() -def _remove_persisted_namespace(temp_dir: Path, *, kitchen_id: str) -> None: - shutil.rmtree(temp_dir / "recipe-delivery" / kitchen_id) +def _remove_persisted_namespace(temp_dir: Path, *, activation_id: KitchenActivationId) -> None: + shutil.rmtree(temp_dir / "recipe-delivery" / _safe_component(activation_id.value)) def _build_envelope( @@ -657,10 +720,15 @@ def test_same_payload_is_idempotent_and_changed_payload_is_immutable(tmp_path: P assert same == first assert changed.payload_sha256 != first.payload_sha256 - assert load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=first) == _payload() - assert load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=changed) == _payload( - "name: remediation\nsteps: {}\n" + assert ( + load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("kitchen-test"), identity=first + ) + == _payload() ) + assert load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("kitchen-test"), identity=changed + ) == _payload("name: remediation\nsteps: {}\n") @pytest.mark.parametrize( @@ -684,8 +752,15 @@ def test_generation_path_includes_version_domain( assert second.payload_sha256 == first.payload_sha256 assert second.pull_identity() != first.pull_identity() with pytest.raises(RecipeArtifactError, match="invalid recipe artifact identity bounds"): - load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=first) - assert load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=second) == _payload() + load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("kitchen-test"), identity=first + ) + assert ( + load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("kitchen-test"), identity=second + ) + == _payload() + ) def test_concurrent_writers_publish_one_exact_generation(tmp_path: Path) -> None: @@ -693,7 +768,11 @@ def test_concurrent_writers_publish_one_exact_generation(tmp_path: Path) -> None generations = list(pool.map(lambda _: _persist(tmp_path), range(24))) assert generations == [generations[0]] * len(generations) assert ( - load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=generations[0]) + load_recipe_artifact( + tmp_path, + activation_id=KitchenActivationId("kitchen-test"), + identity=generations[0], + ) == _payload() ) @@ -743,7 +822,7 @@ def test_generation_identity_domains_are_independently_verified( with pytest.raises(RecipeArtifactError): load_recipe_artifact( tmp_path, - kitchen_id="kitchen-test", + activation_id=KitchenActivationId("kitchen-test"), identity=replace(generation, **{field: value}), ) @@ -754,7 +833,11 @@ def test_generation_descriptor_read_has_server_owned_ceiling(tmp_path: Path) -> descriptor_path.write_bytes(b"x" * 20_000) with pytest.raises(RecipeArtifactError, match="descriptor exceeds read limit"): - load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=generation) + load_recipe_artifact( + tmp_path, + activation_id=KitchenActivationId("kitchen-test"), + identity=generation, + ) def test_non_utf8_payload_is_normalized_to_recipe_artifact_error(tmp_path: Path) -> None: @@ -780,7 +863,7 @@ def test_non_utf8_payload_is_normalized_to_recipe_artifact_error(tmp_path: Path) ) directory = _generation_dir( tmp_path, - kitchen_id="kitchen-test", + activation_id=KitchenActivationId("kitchen-test"), producer_tool=generation.producer_tool, recipe_name=generation.recipe_name, descriptor_version=generation.descriptor_version, @@ -795,7 +878,11 @@ def test_non_utf8_payload_is_normalized_to_recipe_artifact_error(tmp_path: Path) ) with pytest.raises(RecipeArtifactError, match="not valid JSON"): - load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=generation) + load_recipe_artifact( + tmp_path, + activation_id=KitchenActivationId("kitchen-test"), + identity=generation, + ) def test_generation_descriptor_has_no_caller_selected_path(tmp_path: Path) -> None: @@ -825,41 +912,47 @@ def test_kitchen_retirement_removes_only_that_namespace(tmp_path: Path) -> None: first = _persist(tmp_path) second = persist_recipe_artifact( tmp_path, - kitchen_id="other-kitchen", + activation_id=KitchenActivationId("other-kitchen"), producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), ) - assert retire_recipe_artifacts(tmp_path, kitchen_id="kitchen-test") is True + assert ( + retire_recipe_artifacts(tmp_path, activation_id=KitchenActivationId("kitchen-test")) + is True + ) with pytest.raises(RecipeArtifactError): - load_recipe_artifact(tmp_path, kitchen_id="kitchen-test", identity=first) + load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("kitchen-test"), identity=first + ) with pytest.raises(RecipeArtifactError, match="namespace is retired"): _persist(tmp_path) - assert ( - load_recipe_artifact(tmp_path, kitchen_id="other-kitchen", identity=second) == _payload() + assert load_recipe_artifact( + tmp_path, activation_id=KitchenActivationId("other-kitchen"), identity=second ) def test_kitchen_retirement_notifies_callbacks_after_one_fails( monkeypatch: pytest.MonkeyPatch, ) -> None: - notified: list[str] = [] + activation_id = KitchenActivationId("kitchen-test") + notified: list[KitchenActivationId] = [] - def failing_callback(_kitchen_id: str) -> None: + def failing_callback(_activation_id: KitchenActivationId) -> None: raise RuntimeError("cleanup failed") - def succeeding_callback(kitchen_id: str) -> None: - notified.append(kitchen_id) + def succeeding_callback(retired_activation_id: KitchenActivationId) -> None: + notified.append(retired_activation_id) monkeypatch.setattr( recipe_section_lifecycle, - "_KITCHEN_RETIREMENT_CALLBACKS", + "_ACTIVATION_RETIREMENT_CALLBACKS", (failing_callback, succeeding_callback), ) - recipe_section_lifecycle.notify_kitchen_retired("kitchen-test") + recipe_section_lifecycle.notify_activation_retired(activation_id) - assert notified == ["kitchen-test"] + assert notified == [activation_id] @pytest.mark.parametrize("kwargs", [{"max_entries": -1}, {"max_bytes": -1}]) @@ -882,19 +975,30 @@ def test_kitchen_retirement_evicts_only_matching_page_plans( "recipe_section_bound_bytes": 10_000, } retired = get_or_build_recipe_section_page_plan( - kitchen_id="kitchen-test", + activation_id=KitchenActivationId("kitchen-test"), **common, ) retained = get_or_build_recipe_section_page_plan( - kitchen_id="other-kitchen", + activation_id=KitchenActivationId("other-kitchen"), **common, ) - assert retire_recipe_artifacts(tmp_path, kitchen_id="kitchen-test") is True + assert ( + retire_recipe_artifacts(tmp_path, activation_id=KitchenActivationId("kitchen-test")) + is True + ) - assert get_or_build_recipe_section_page_plan(kitchen_id="other-kitchen", **common) is retained assert ( - get_or_build_recipe_section_page_plan(kitchen_id="kitchen-test", **common) is not retired + get_or_build_recipe_section_page_plan( + activation_id=KitchenActivationId("other-kitchen"), **common + ) + is retained + ) + assert ( + get_or_build_recipe_section_page_plan( + activation_id=KitchenActivationId("kitchen-test"), **common + ) + is not retired ) @@ -904,7 +1008,10 @@ def test_cold_kitchen_retirement_does_not_create_page_plan_cache( ) -> None: monkeypatch.setattr(pagination, "_PAGE_PLAN_CACHE", None) - assert retire_recipe_artifacts(tmp_path, kitchen_id="cold-kitchen") is True + assert ( + retire_recipe_artifacts(tmp_path, activation_id=KitchenActivationId("cold-kitchen")) + is True + ) assert pagination._PAGE_PLAN_CACHE is None @@ -925,17 +1032,18 @@ def _tracked_lock(temp_dir: Path, *, exclusive: bool): finally: lock_active = False - evicted: list[str] = [] + activation_id = KitchenActivationId("lock-kitchen") + evicted: list[KitchenActivationId] = [] - def _evict(kitchen_id: str) -> None: + def _evict(retired_activation_id: KitchenActivationId) -> None: assert lock_active is False - evicted.append(kitchen_id) + evicted.append(retired_activation_id) monkeypatch.setattr(recipe_artifact, "_generation_lock", _tracked_lock) - monkeypatch.setattr(pagination, "evict_kitchen", _evict) + monkeypatch.setattr(pagination, "evict_activation", _evict) - assert retire_recipe_artifacts(tmp_path, kitchen_id="lock-kitchen") is True - assert evicted == ["lock-kitchen"] + assert retire_recipe_artifacts(tmp_path, activation_id=activation_id) is True + assert evicted == [activation_id] def test_generation_lock_translates_timeout( @@ -957,10 +1065,11 @@ def test_kitchen_retirement_eviction_is_success_only_and_nonthrowing( tmp_path: Path, ) -> None: evict = MagicMock(side_effect=RuntimeError("cache unavailable")) - monkeypatch.setattr(pagination, "evict_kitchen", evict) + monkeypatch.setattr(pagination, "evict_activation", evict) - assert retire_recipe_artifacts(tmp_path, kitchen_id="successful-kitchen") is True - evict.assert_called_once_with("successful-kitchen") + successful_activation = KitchenActivationId("successful-kitchen") + assert retire_recipe_artifacts(tmp_path, activation_id=successful_activation) is True + evict.assert_called_once_with(successful_activation) evict.reset_mock() monkeypatch.setattr( @@ -968,16 +1077,24 @@ def test_kitchen_retirement_eviction_is_success_only_and_nonthrowing( "atomic_write", MagicMock(side_effect=OSError("retirement failed")), ) - assert retire_recipe_artifacts(tmp_path, kitchen_id="failed-kitchen") is False + assert ( + retire_recipe_artifacts(tmp_path, activation_id=KitchenActivationId("failed-kitchen")) + is False + ) evict.assert_not_called() -@pytest.mark.parametrize("kitchen_id", [".", ".."]) -def test_kitchen_retirement_rejects_dot_path_components(tmp_path: Path, kitchen_id: str) -> None: +@pytest.mark.parametrize( + "activation_id", + [KitchenActivationId("."), KitchenActivationId("..")], +) +def test_kitchen_retirement_rejects_dot_path_components( + tmp_path: Path, activation_id: KitchenActivationId +) -> None: sentinel = tmp_path / "unrelated-temp-data" sentinel.write_text("preserve me", encoding="utf-8") - assert retire_recipe_artifacts(tmp_path, kitchen_id=kitchen_id) is False + assert retire_recipe_artifacts(tmp_path, activation_id=activation_id) is False assert sentinel.read_text(encoding="utf-8") == "preserve me" @@ -1692,7 +1809,7 @@ async def test_pull_tool_reads_exact_generation_and_reports_byte_offsets( expected_content = "héllo\n" * 12_000 generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(expected_content), @@ -1749,7 +1866,7 @@ async def test_pull_tool_rejects_missing_or_wrong_continuation( tool_ctx_kitchen_open.config.output_budget = OutputBudgetConfig(page_max_bytes=None) generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload("x" * 50_000), @@ -1809,7 +1926,7 @@ async def test_pull_tool_maps_planner_failures_to_exact_bounded_codes( ) generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), @@ -1866,7 +1983,7 @@ async def test_pull_tool_distinguishes_missing_none_and_present_empty_sections( payload[section] = [] generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=payload, @@ -1905,7 +2022,7 @@ async def test_initial_schema_failure_is_bounded_and_never_recreates( generation = _write_malformed_generation( tool_ctx_kitchen_open.temp_dir, payload, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, ) recreate = MagicMock(side_effect=AssertionError("schema failure must not recreate")) warning = MagicMock() @@ -1938,7 +2055,7 @@ async def test_recreation_persistence_schema_failure_precedes_artifact_error( generation, _finalized = _persist_finalized_generation(tool_ctx_kitchen_open) _remove_persisted_namespace( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, ) monkeypatch.setattr( recipe_section_handler, @@ -2051,7 +2168,10 @@ async def test_negative_part_is_rejected_before_artifact_load( tool_ctx_kitchen_open.backend = CodexBackend() bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="kitchen-test") - generation = _persist(tool_ctx_kitchen_open.temp_dir) + generation = _persist( + tool_ctx_kitchen_open.temp_dir, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, + ) artifact_load = MagicMock(side_effect=AssertionError("negative part reached artifact load")) monkeypatch.setattr(recipe_section_handler, "load_recipe_artifact", artifact_load) kwargs = generation.pull_identity() @@ -2069,7 +2189,10 @@ async def test_oversized_part_is_rejected_after_page_planning( ) -> None: tool_ctx_kitchen_open.backend = CodexBackend() bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="kitchen-test") - generation = _persist(tool_ctx_kitchen_open.temp_dir) + generation = _persist( + tool_ctx_kitchen_open.temp_dir, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, + ) kwargs = generation.pull_identity() kwargs.pop("pull_tool") @@ -2095,7 +2218,10 @@ async def test_request_specific_floor_returns_exact_bounded_failure( ), ), ) - generation = _persist(tool_ctx_kitchen_open.temp_dir) + generation = _persist( + tool_ctx_kitchen_open.temp_dir, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, + ) kwargs = generation.pull_identity() kwargs.pop("pull_tool") @@ -2129,7 +2255,7 @@ async def test_pull_tool_returns_named_step_and_rejects_unknown_section( bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-named-step") generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), @@ -2155,7 +2281,7 @@ async def test_pull_tool_reports_malformed_named_step_yaml(tool_ctx_kitchen_open malformed = _payload("steps: [") generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=malformed, @@ -2180,7 +2306,7 @@ async def test_oversized_named_step_round_trips_through_continuation( payload["post_prune_step_names"] = ["giant_step"] generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=payload, @@ -2222,7 +2348,7 @@ async def test_pull_tool_rejects_wrong_generation_identity(tool_ctx_kitchen_open bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-wrong") generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), @@ -2242,7 +2368,7 @@ async def test_pull_tool_rejects_forged_unbounded_identity_sizes( bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-unbounded-identity") generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool=producer_tool, recipe_name="remediation", payload=_payload(), @@ -2264,7 +2390,7 @@ async def test_pull_tool_recreates_missing_exact_generation( bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-recreate") generation, finalized = _persist_finalized_generation(tool_ctx_kitchen_open) _remove_persisted_namespace( - tool_ctx_kitchen_open.temp_dir, kitchen_id=tool_ctx_kitchen_open.kitchen_id + tool_ctx_kitchen_open.temp_dir, activation_id=tool_ctx_kitchen_open.kitchen_activation_id ) recompile = MagicMock(side_effect=AssertionError("recreation must not recompile")) monkeypatch.setattr( @@ -2281,7 +2407,7 @@ async def test_pull_tool_recreates_missing_exact_generation( assert response["content"] == _payload()["content"] recreated = load_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, identity=generation, ) assert recreated["content"] == _payload()["content"] @@ -2305,7 +2431,7 @@ async def test_recreation_reuses_original_snapshot_without_snapshot_factory( payload, ) _remove_persisted_namespace( - tool_ctx_kitchen_open.temp_dir, kitchen_id=tool_ctx_kitchen_open.kitchen_id + tool_ctx_kitchen_open.temp_dir, activation_id=tool_ctx_kitchen_open.kitchen_activation_id ) snapshot_factory = MagicMock( side_effect=AssertionError("recreation must reuse the original snapshot") @@ -2323,7 +2449,7 @@ async def test_recreation_reuses_original_snapshot_without_snapshot_factory( assert response["success"] is True recreated = load_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, identity=generation, ) assert recreated["recipe_execution"]["execution_id"] == ( @@ -2338,13 +2464,13 @@ async def test_pull_tool_reports_invalid_missing_generation_recreation( bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-recreate-invalid") generation = persist_recipe_artifact( tool_ctx_kitchen_open.temp_dir, - kitchen_id=tool_ctx_kitchen_open.kitchen_id, + activation_id=tool_ctx_kitchen_open.kitchen_activation_id, producer_tool="open_kitchen", recipe_name="remediation", payload=_payload(), ) _remove_persisted_namespace( - tool_ctx_kitchen_open.temp_dir, kitchen_id=tool_ctx_kitchen_open.kitchen_id + tool_ctx_kitchen_open.temp_dir, activation_id=tool_ctx_kitchen_open.kitchen_activation_id ) kwargs = generation.pull_identity() @@ -2360,7 +2486,7 @@ async def test_pull_tool_rejects_changed_recreated_generation( bind_test_kitchen_identity(tool_ctx_kitchen_open, kitchen_id="pull-recreate-changed") generation, _finalized = _persist_finalized_generation(tool_ctx_kitchen_open) _remove_persisted_namespace( - tool_ctx_kitchen_open.temp_dir, kitchen_id=tool_ctx_kitchen_open.kitchen_id + tool_ctx_kitchen_open.temp_dir, activation_id=tool_ctx_kitchen_open.kitchen_activation_id ) kwargs = generation.pull_identity() diff --git a/tests/server/test_track_response_size.py b/tests/server/test_track_response_size.py index 3f2ae8e535..57b6fd6591 100644 --- a/tests/server/test_track_response_size.py +++ b/tests/server/test_track_response_size.py @@ -349,6 +349,7 @@ async def test_attested_inline_open_kitchen_confirms_response_enforcement(self): from autoskillit.core import RecipeDeliveryMode from autoskillit.pipeline import ( + KitchenActivationId, KitchenEffectPhase, new_kitchen_open_state, start_kitchen_effect, @@ -374,6 +375,7 @@ async def test_attested_inline_open_kitchen_confirms_response_enforcement(self): ctx.kitchen_open_state = start_kitchen_effect( new_kitchen_open_state( kitchen_id="kitchen", + activation_id=KitchenActivationId("kitchen"), context_id="context", operation_id="operation", ), diff --git a/tests/workspace/test_clone_push.py b/tests/workspace/test_clone_push.py index 8f3a911e29..64f3989b3d 100644 --- a/tests/workspace/test_clone_push.py +++ b/tests/workspace/test_clone_push.py @@ -20,6 +20,120 @@ class TestPushToRemote: + @pytest.mark.parametrize( + ("conflicting_upstream", "multiple_push_urls"), + [(False, False), (True, False), (False, True)], + ids=["origin-only", "conflicting-upstream", "multiple-push-urls"], + ) + def test_push_uses_resolved_destination_from_worktree( + self, + tmp_path: Path, + local_with_remote: Path, + bare_remote: Path, + conflicting_upstream: bool, + multiple_push_urls: bool, + ) -> None: + worktree = tmp_path / "smoke-worktree" + branch = "e2e-smoke" + subprocess.run( + ["git", "-C", str(local_with_remote), "worktree", "add", "-b", branch, str(worktree)], + check=True, + capture_output=True, + ) + wrong_remote = tmp_path / "wrong.git" + if conflicting_upstream or multiple_push_urls: + subprocess.run( + ["git", "init", "--bare", str(wrong_remote)], check=True, capture_output=True + ) + if conflicting_upstream: + subprocess.run( + [ + "git", + "-C", + str(local_with_remote), + "remote", + "add", + "upstream", + str(wrong_remote), + ], + check=True, + capture_output=True, + ) + if multiple_push_urls: + for url in (bare_remote, wrong_remote): + subprocess.run( + [ + "git", + "-C", + str(local_with_remote), + "config", + "--add", + "remote.origin.pushurl", + str(url), + ], + check=True, + capture_output=True, + ) + before_remotes = subprocess.run( + ["git", "-C", str(worktree), "remote", "-v"], + check=True, + capture_output=True, + text=True, + ).stdout + + result = push_to_remote( + str(worktree), branch=branch, remote_url=str(bare_remote), protected_branches=[] + ) + + assert result["success"] is True, result + expected = subprocess.run( + ["git", "-C", str(worktree), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + published = subprocess.run( + ["git", "--git-dir", str(bare_remote), "rev-parse", f"refs/heads/{branch}"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + assert published == expected + assert ( + subprocess.run( + ["git", "-C", str(worktree), "remote", "-v"], + check=True, + capture_output=True, + text=True, + ).stdout + == before_remotes + ) + if not multiple_push_urls: + assert ( + subprocess.run( + ["git", "-C", str(worktree), "rev-parse", "@{upstream}"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + == expected + ) + if conflicting_upstream or multiple_push_urls: + assert ( + subprocess.run( + [ + "git", + "--git-dir", + str(wrong_remote), + "show-ref", + "--verify", + f"refs/heads/{branch}", + ], + capture_output=True, + ).returncode + != 0 + ) + def test_push_to_remote_propagates_to_upstream(self, tmp_path: Path) -> None: # 1. Create bare remote (simulates GitHub) remote = tmp_path / "remote.git" @@ -204,8 +318,15 @@ def test_push_to_remote_rejects_protected_branch(self, tmp_path: Path, branch: s class TestPushToRemoteMocked: + @pytest.fixture(autouse=True) + def _use_url_destination(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + "autoskillit.workspace.clone._matching_push_remote", + lambda checkout, url: url, + ) + def test_ds6_push_to_remote_calls_get_url_then_push(self) -> None: - """T_DS6: push_to_remote calls git remote get-url origin then git push -u upstream.""" + """T_DS6: push_to_remote resolves origin, then pushes directly to that URL.""" mock_url = MagicMock() mock_url.returncode = 0 mock_url.stdout = "git@github.com:org/repo.git\n" @@ -227,9 +348,16 @@ def test_ds6_push_to_remote_calls_get_url_then_push(self) -> None: first_call = mock_run.call_args_list[0] assert first_call[0][0] == ["git", "remote", "get-url", "origin"] assert first_call[1]["cwd"] == "/source" - # Second call: git push -u upstream from clone_path — no --force-with-lease + # Push from clone_path to the resolved destination without --force-with-lease. second_call = mock_run.call_args_list[1] - assert second_call[0][0] == ["git", "push", "-u", "upstream", "main"] + assert second_call[0][0] == [ + "git", + "push", + "-u", + "--", + "git@github.com:org/repo.git", + "main", + ] assert second_call[1]["cwd"] == "/clone" def test_ds7_push_to_remote_fails_when_no_origin(self) -> None: @@ -270,7 +398,7 @@ def test_push_to_remote_with_force_injects_force_with_lease(self) -> None: assert "--force-with-lease" in cmd assert "git" in cmd assert "push" in cmd - assert "upstream" in cmd + assert "git@github.com:org/repo.git" in cmd assert "main" in cmd assert second_call[1]["cwd"] == "/clone" @@ -295,7 +423,7 @@ def test_push_to_remote_default_force_false_does_not_inject_lease(self) -> None: assert result == {"success": True, "stderr": ""} second_call = mock_run.call_args_list[1] cmd = second_call[0][0] - assert cmd == ["git", "push", "-u", "upstream", "main"] + assert cmd == ["git", "push", "-u", "--", "git@github.com:org/repo.git", "main"] def test_default_clone_manager_push_to_remote_accepts_force_param(self) -> None: """T5: DefaultCloneManager.push_to_remote has force keyword param with default False."""