diff --git a/docs/developer/diagnostics.md b/docs/developer/diagnostics.md index 3609f46a28..fcfb131588 100644 --- a/docs/developer/diagnostics.md +++ b/docs/developer/diagnostics.md @@ -255,11 +255,15 @@ inclusive. Historical numeric zero values load as `unknown` because their observation status cannot be recovered. `context_fraction` is the normalized cache-read proxy -`cache_read_tokens / context_window_tokens` for that row's observed model. It +`cache_read_tokens / context_window_tokens` for the session's requested model. It does not include uncached or newly cached input and is not total context occupancy. `context_window_tokens` is positive capacity metadata, not an accounting observation; a missing capacity or unobserved cache read leaves the -fraction `null`. Claude snapshots deduplicate by +fraction `null`. For Claude, the CLI-reported window comes from +`modelUsage[].contextWindow`, identified by the +init record or the sole `modelUsage` key. The turn row's `model` remains the +served name. This does not measure the provider's maximum accepted input or +repair upstream reporting errors. Claude snapshots deduplicate by `(backend, provider_used, message.id)`, preserving first-seen order and the first timestamp. Identical message IDs from different source pairs remain separate rows. Codex rows diff --git a/docs/operations/observability.md b/docs/operations/observability.md index 1624053d04..fc9c8bd9b4 100644 --- a/docs/operations/observability.md +++ b/docs/operations/observability.md @@ -65,6 +65,13 @@ observation. `context_window_tokens` is positive capacity metadata, not a measure; `context_fraction` exists only with observed cache-read usage and a known capacity. +For Claude turns, the window is the CLI-reported context budget from +`modelUsage[].contextWindow`. The init record +identifies that request; otherwise the sole `modelUsage` key is used. Ambiguous, +invalid or conflicting evidence leaves the window unknown. The turn's `model` +is still the served name. This reports the CLI's budget, without independently +verifying the provider's maximum accepted input or correcting upstream reports. + ## Mid-run accessors The orchestrator can read accumulators mid-run via the status MCP tools: diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index 8883f7d720..da923f52bd 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -48,8 +48,9 @@ must have: - a pull-request CI workflow, so `wait_for_ci` has something to wait for; - a `Taskfile.yml` with `install-worktree` and `test-check` tasks; -- a `.autoskillit/config.yaml` that sets none of `features`, `quota_guard` or - `agent_backend` — as the project layer it outranks the harness's user-layer config; +- a `.autoskillit/config.yaml` that sets none of `features`, `quota_guard`, + `agent_backend`, `model` or `linux_tracing` — the project layer outranks the harness's + user-layer config, and a tracing log directory would move logs away from the gate; - no committed `.claude/settings.json` or `.claude/settings.local.json` with `env` provider or model keys — project and local settings outrank user settings. @@ -69,18 +70,40 @@ The harness writes that block (mode `0600`): | `CLAUDE_CODE_AUTO_COMPACT_WINDOW` | `1000000` | | `CLAUDE_CODE_MAX_CONCURRENT_SUBAGENTS` | `6` | -The values follow MiniMax's Claude Code guide -() and the repository's own -`providers.profiles.minimax`. The `[1m]` suffix makes Claude Code assume a 1M-token window -for the unrecognized model id; the compact window is capped at it. Six concurrent subagents +The env model values carry `[1m]` so sessions resolved through them are sized at 1M; +they differ from [MiniMax's current Claude Code guide](https://platform.minimax.io/docs/m-plan/claude-code), +whose manual example uses `MiniMax-M3.1-Flash-Preview[1m]` and a 524,288 compact window +(checked 2026-10-14). Six concurrent subagents covers `implement` (four children) and `resolve-review` (three to six); a spawn past the limit fails without retry. +| Setting | Sessions it governs in E2E | +|---|---| +| `--model claude-sonnet-5[1m]` (from `model.default: sonnet[1m]`) | Fleet food truck and `run_skill` workers, fresh and resumed; also the canary and in-session `sonnet` subagents, which inherit the parent's id. A recipe step's own `model:` replaces this default. Subagent windows are not in turn rows, so the gate does not observe them | +| `ANTHROPIC_MODEL` | Contract-nudge resumes, which pass no `--model` and resolve to `MiniMax-M3[1m]`. Their single-envelope output records no turn row, so the gate does not observe it | +| `ANTHROPIC_DEFAULT_HAIKU_MODEL` | A `haiku`-class step (`--model haiku`) and in-session `haiku` subagents | +| `ANTHROPIC_DEFAULT_OPUS_MODEL` | In-session `opus` subagents. AutoSkillit's `opus` class passes `claude-opus-5-5`, not the alias | +| `ANTHROPIC_DEFAULT_SONNET_MODEL` | Provider fallback: AutoSkillit passes no bare `sonnet`, and `sonnet` subagents inherit the parent's id | +| `CLAUDE_CODE_AUTO_COMPACT_WINDOW` | Every session, capped at that session's model window | + +The live probe confirms the explicit `[1m]` declaration yields 1M on the pinned +2.1.280/custom-endpoint path. [Claude Code 2.1.285](https://github.com/anthropics/claude-code/releases/tag/v2.1.285) +changed custom-endpoint sessions to use Sonnet 5's native 1M window; the +[current model-config docs](https://code.claude.com/docs/en/model-config) describe +`[1m]` as having no effect when Sonnet already has a native 1M window. The harness +retains the suffix while supporting the pinned path. + +[Claude Code 2.1.295](https://github.com/anthropics/claude-code/releases/tag/v2.1.295) +documents retrying without a rejected context-1m beta. Do not assume that fallback +exists at the pinned floor or that every older release had the same defect; +attribute failures from the probe's actual responses. The floor change belongs to #5276. + The harness relies on this provider contract: the Anthropic Messages API at `ANTHROPIC_BASE_URL`, Bearer authentication from `ANTHROPIC_AUTH_TOKEN`, tool use, and -acceptance of the explicit `claude-*` model ids AutoSkillit passes with `--model`. The -canary passes no `--model`, so `ANTHROPIC_MODEL` covers it; `ANTHROPIC_DEFAULT_HAIKU_MODEL` -covers the bare `haiku` alias. +successful calls using the exact `claude-sonnet-5[1m]` id that fleet sessions and the +canary pass with `--model`, including any beta-header behavior of the pinned client. +The canary passes `--model` `FLEET_MODEL` and fails unless +`modelUsage[FLEET_MODEL].contextWindow` is 1,000,000. Every child process the harness starts gets an environment with `MINIMAX_API_KEY`, `E2E_SANDBOX_TOKEN`, `CLAUDE_CODE_OAUTH_TOKEN` and every `ANTHROPIC_*` variable removed, so @@ -137,6 +160,8 @@ executes it explicitly. For model tests, `autoskillit-config.yaml` becomes `~/.autoskillit/config.yaml` in the container: +- `model.default: sonnet[1m]` — puts the window declaration on the id AutoSkillit + passes with `--model`; `tests/infra/test_e2e_harness.py` derives `FLEET_MODEL` from it; - `quota_guard.enabled: false` — the guard measures Anthropic account quota through Claude credentials (`~/.claude/.credentials.json`) the container does not have; `fleet run` also gets `--disable-quota-guard`; @@ -196,7 +221,8 @@ cleanup and redaction never depend on GitHub cancelling the job. ## Artifacts and redaction For model tests, the container's AutoSkillit data directory (`~/.local/share/autoskillit`) is bind-mounted -from the runner, so session logs survive a killed container. Those files belong to uid 1000 +from the runner, so session logs survive a killed container; the harness also reads this +log tree after fleet runs. Those files belong to uid 1000 and are often `0600`, so a second container run copies `out/` and `data/logs/` into `upload/` world-readable with both secrets replaced by `[REDACTED]`, skips symlinks, and fails if any secret survives. Only `upload/` is uploaded, and only when redaction @@ -233,6 +259,14 @@ one sandbox pull request numbered above the pre-run baseline exists in the catal `expected_pull_request_state` with a non-empty diff. `gh` reports states in upper case; the harness lower-cases them. Its existing cleanup closes newly opened PRs with their branches. +After `fleet run`, the harness reads `sessions.jsonl` from the directory it gave the +fleet through `AUTOSKILLIT_LOG_DIR`. Every session new in this run must have +`turn_usage.jsonl` rows at 1,000,000, and a successful envelope must have at least +one new session. Missing windows fail closed, and window failures survive an +expected-failure match. This checks the CLI-reported budget: failures retain the +session `dir_name` and observed value rather than inferring a cause or substituting +the expected value. + `headless-smoke` exercises one orchestrator and one recipe worker (`peak_sessions: 2`). The harness stages `sandbox-smoke.yaml` from the read-only `/opt/e2e` mount into the disposable sandbox clone's `.autoskillit/recipes/` directory before invoking fleet by recipe name. This diff --git a/scripts/e2e/autoskillit-config.yaml b/scripts/e2e/autoskillit-config.yaml index 577e8df598..511b455990 100644 --- a/scripts/e2e/autoskillit-config.yaml +++ b/scripts/e2e/autoskillit-config.yaml @@ -3,6 +3,9 @@ quota_guard: enabled: false +model: + default: sonnet[1m] + features: fleet: true fleet_headless_run: true diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index 157f948203..15e810040d 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -25,6 +25,9 @@ MINIMAX_BASE_URL = "https://api.minimax.io/anthropic" MINIMAX_MODEL = "MiniMax-M3[1m]" MINIMAX_COMPACT_WINDOW_TOKENS = "1000000" +# tests/infra/test_e2e_harness.py derives this --model value from the shipped config. +FLEET_MODEL = "claude-sonnet-5[1m]" +REQUIRED_CONTEXT_WINDOW_TOKENS = 1_000_000 # implement runs four concurrent children and resolve-review three to six; a spawn past the # limit fails without retry. MAX_CONCURRENT_SUBAGENTS = "6" @@ -169,6 +172,22 @@ def check_canary_output(stdout: str) -> list[str]: reply = payload.get("result") if not isinstance(reply, str) or CANARY_TOKEN not in reply: return [f"canary: reply does not contain {CANARY_TOKEN}"] + return _check_reported_window(payload) + + +def _check_reported_window(payload: dict[str, Any]) -> list[str]: + model_usage = payload.get("modelUsage") + metadata = model_usage.get(FLEET_MODEL) if isinstance(model_usage, dict) else None + if not isinstance(metadata, dict): + return [f"canary: claude output has no modelUsage entry for {FLEET_MODEL}"] + window = metadata.get("contextWindow") + if type(window) is not int: + return [f"canary: {FLEET_MODEL} reported no integer context window"] + if window != REQUIRED_CONTEXT_WINDOW_TOKENS: + return [ + f"canary: {FLEET_MODEL} reported a {window}-token context window; " + f"the harness requires {REQUIRED_CONTEXT_WINDOW_TOKENS}" + ] return [] @@ -178,7 +197,7 @@ def run_canary( workdir = Path(tempfile.mkdtemp(prefix="e2e-canary-")) result, failure = _call( runner, - ["claude", "-p", CANARY_PROMPT, "--output-format", "json"], + ["claude", "-p", CANARY_PROMPT, "--output-format", "json", "--model", FLEET_MODEL], what="claude -p", cwd=workdir, env=env, @@ -307,6 +326,59 @@ def check_envelope(returncode: int, stdout: str) -> tuple[list[str], dict[str, A return failures, envelope +def session_index_dir_names(log_dir: Path) -> frozenset[str]: + index = log_dir / "sessions.jsonl" + if not index.exists(): + return frozenset() + names = set() + for line_number, line in enumerate(index.read_text().splitlines(), start=1): + if not line.strip(): + continue + try: + row = json.loads(line) + except json.JSONDecodeError as exc: + raise ValueError(f"{index}:{line_number}: invalid session index JSON: {exc}") from exc + if isinstance(row.get("dir_name"), str): + names.add(row["dir_name"]) + return frozenset(names) + + +def _check_session_turn_windows(log_dir: Path, dir_name: str) -> list[str]: + path = log_dir / "sessions" / dir_name / "turn_usage.jsonl" + prefix = f"session windows: {dir_name}: " + if not path.exists(): + return [prefix + "no turn usage"] + rows = [] + for line_number, line in enumerate(path.read_text().splitlines(), start=1): + if not line.strip(): + continue + try: + rows.append(json.loads(line)) + except json.JSONDecodeError as exc: + return [prefix + f"{path}:{line_number}: invalid turn usage JSON: {exc}"] + if not rows: + return [prefix + "no turn usage"] + if any(type(row.get("context_window_tokens")) is not int for row in rows): + return [prefix + "no observed context window"] + windows = sorted({row["context_window_tokens"] for row in rows}) + if windows != [REQUIRED_CONTEXT_WINDOW_TOKENS]: + return [prefix + f"observed {windows} != required {REQUIRED_CONTEXT_WINDOW_TOKENS}"] + return [] + + +def check_session_windows( + log_dir: Path, baseline: frozenset[str], *, sessions_required: bool +) -> list[str]: + new_sessions = session_index_dir_names(log_dir) - baseline + if not new_sessions and sessions_required: + return [f"session windows: the fleet run recorded no sessions in {log_dir}"] + return [ + failure + for dir_name in sorted(new_sessions) + for failure in _check_session_turn_windows(log_dir, dir_name) + ] + + def run_fleet( test: e2e_catalog.CatalogTest, out: Path, @@ -314,21 +386,36 @@ def run_fleet( runner: Runner, runtime_ingredients: Mapping[str, str] | None = None, ) -> tuple[list[str], dict[str, Any] | None]: - result, failure = _call( - runner, - fleet_run_argv(test, runtime_ingredients), - what="fleet run", - cwd=SANDBOX_CLONE, - env=env, - timeout=test.timeout_sec + e2e_catalog.HARNESS_GRACE_SEC, - evidence=out / "fleet-run", - ) - if result is None: - return [str(failure)], None - (out / "fleet-run.stdout.log").write_text(result.stdout, encoding="utf-8") - (out / "fleet-run.stderr.log").write_text(result.stderr, encoding="utf-8") - (out / "envelope.json").write_text(last_line(result.stdout) + "\n", encoding="utf-8") - return check_envelope(result.returncode, result.stdout) + log_dir = Path(env["AUTOSKILLIT_LOG_DIR"]) + baseline = session_index_dir_names(log_dir) + failures: list[str] = [] + envelope: dict[str, Any] | None = None + try: + result, failure = _call( + runner, + fleet_run_argv(test, runtime_ingredients), + what="fleet run", + cwd=SANDBOX_CLONE, + env=env, + timeout=test.timeout_sec + e2e_catalog.HARNESS_GRACE_SEC, + evidence=out / "fleet-run", + ) + if result is None: + failures.append(str(failure)) + else: + (out / "fleet-run.stdout.log").write_text(result.stdout, encoding="utf-8") + (out / "fleet-run.stderr.log").write_text(result.stderr, encoding="utf-8") + (out / "envelope.json").write_text(last_line(result.stdout) + "\n", encoding="utf-8") + failures, envelope = check_envelope(result.returncode, result.stdout) + finally: + failures.extend( + check_session_windows( + log_dir, + baseline, + sessions_required=envelope is not None and envelope.get("success") is True, + ) + ) + return failures, envelope def match_recipe_failure( @@ -1016,7 +1103,11 @@ def _run_test( if test.kind == "canary": return run_canary(test, out, scrubbed, runner), [] token = env["E2E_SANDBOX_TOKEN"] - return run_recipe(test, catalog.sandbox_repository, token, out, scrubbed, runner) + recipe_env = { + **scrubbed, + "AUTOSKILLIT_LOG_DIR": str(home / ".local" / "share" / "autoskillit" / "logs"), + } + return run_recipe(test, catalog.sandbox_repository, token, out, recipe_env, runner) def _copy_failure_files( diff --git a/scripts/e2e/recipes/sandbox-smoke.yaml b/scripts/e2e/recipes/sandbox-smoke.yaml index 8aab0903d3..d659d21b40 100644 --- a/scripts/e2e/recipes/sandbox-smoke.yaml +++ b/scripts/e2e/recipes/sandbox-smoke.yaml @@ -85,7 +85,8 @@ steps: only those two files, then create one new commit containing only sandbox/text.py and tests/test_smoke_canary.py with message `smoke: add sandbox canary`. Do not run a test command or call tools - that dispatch work to another worker. + that dispatch work to another worker. Report the commit hash and + changed file paths in your final response before the completion marker. cwd: "${{ context.worktree_path }}" output_dir: "${{ context.worktree_path }}" step_name: implement diff --git a/src/autoskillit/execution/session/_session_model.py b/src/autoskillit/execution/session/_session_model.py index 82ca21dd6f..00aa4387be 100644 --- a/src/autoskillit/execution/session/_session_model.py +++ b/src/autoskillit/execution/session/_session_model.py @@ -246,14 +246,41 @@ def lifespan_started(self) -> bool: return bool(self.tool_uses) +def _record_model_windows(model_usage: object, model_windows: dict[str, set[int]]) -> None: + """Retain requested model keys even when their reported windows are invalid.""" + if not isinstance(model_usage, dict): + return + for model, metadata in model_usage.items(): + if not isinstance(model, str) or not model: + continue + windows = model_windows.setdefault(model, set()) + if not isinstance(metadata, dict): + continue + window = valid_context_window(metadata.get("contextWindow")) + if window is not None: + windows.add(window) + + +def _requested_window_key( + requested_models: set[str], model_windows: dict[str, set[int]] +) -> str | None: + """Use the parent's exact init request, or the sole requested modelUsage key.""" + if len(requested_models) == 1 and (model := next(iter(requested_models))) in model_windows: + return model + if len(model_windows) == 1: + return next(iter(model_windows)) + return None + + def _collect_token_usage_evidence( stdout: str, provider_used: str, -) -> tuple[dict[str, int | None] | None, list[TurnTokenEntry], dict[str, set[int]]]: - """Collect result totals, assistant rows, and model windows from NDJSON stdout.""" +) -> tuple[dict[str, int | None] | None, list[TurnTokenEntry], dict[str, set[int]], set[str]]: + """Collect result totals, parent rows, reported windows, and init model requests.""" result_usage: dict[str, int | None] | None = None candidate_rows: list[TurnTokenEntry] = [] model_windows: dict[str, set[int]] = {} + requested_models: set[str] = set() for line in stdout.strip().splitlines(): line = line.strip() @@ -267,6 +294,9 @@ def _collect_token_usage_evidence( continue record_type = obj.get("type") + if record_type == "system" and obj.get("subtype") == "init": + if model := first_nonempty_string(obj.get("model")): + requested_models.add(model) if is_parent_assistant_record(obj): msg = obj.get("message") if not isinstance(msg, dict): @@ -301,16 +331,9 @@ def _collect_token_usage_evidence( canon_f: first_valid_token_count(usage, api_f, canon_f) for api_f, canon_f in zip(_API_TOKEN_FIELDS, _CANONICAL_TOKEN_FIELDS) } - model_usage = obj.get("modelUsage") - if isinstance(model_usage, dict): - for model, metadata in model_usage.items(): - if not isinstance(model, str) or not model or not isinstance(metadata, dict): - continue - window = valid_context_window(metadata.get("contextWindow")) - if window is not None: - model_windows.setdefault(model, set()).add(window) + _record_model_windows(obj.get("modelUsage"), model_windows) - return result_usage, candidate_rows, model_windows + return result_usage, candidate_rows, model_windows, requested_models def extract_token_usage( @@ -326,7 +349,7 @@ def extract_token_usage( if not stdout.strip(): return None, [] - result_usage, candidate_rows, model_windows = _collect_token_usage_evidence( + result_usage, candidate_rows, model_windows, requested_models = _collect_token_usage_evidence( stdout, provider_used ) raw_rows = merge_turn_usage(candidate_rows) @@ -334,7 +357,10 @@ def extract_token_usage( return None, [] model_buckets, peak_context, turn_usage = _build_turn_usage_and_model_buckets( - raw_rows, provider_used, model_windows + raw_rows, + provider_used, + model_windows, + _requested_window_key(requested_models, model_windows), ) totals: dict[str, TokenMeasure] @@ -371,6 +397,7 @@ def _build_turn_usage_and_model_buckets( raw_rows: list[TurnTokenEntry], provider_used: str, model_windows: dict[str, set[int]], + window_key: str | None, ) -> tuple[dict[str, dict[str, TokenMeasure]], TokenMeasure | None, list[TurnTokenEntry]]: model_buckets: dict[str, dict[str, TokenMeasure]] = {} peak_context: TokenMeasure | None = None @@ -412,7 +439,7 @@ def _build_turn_usage_and_model_buckets( cache_read = row["cache_read_tokens"] cache_creation = row["cache_creation_tokens"] inclusive_input = claude_inclusive_input_tokens(raw_input, cache_read, cache_creation) - windows = model_windows.get(model, set()) if model is not None else set() + windows = model_windows.get(window_key, set()) if window_key is not None else set() context_window = next(iter(windows)) if len(windows) == 1 else None turn_usage.append( build_turn_token_entry( diff --git a/src/autoskillit/server/tools/tools_execution/_run_skill_session.py b/src/autoskillit/server/tools/tools_execution/_run_skill_session.py index 5455522aea..bf42d72535 100644 --- a/src/autoskillit/server/tools/tools_execution/_run_skill_session.py +++ b/src/autoskillit/server/tools/tools_execution/_run_skill_session.py @@ -33,7 +33,11 @@ resolve_skill_temp_dir as _resolve_skill_temp_dir, ) from autoskillit.execution import ReplayingSubprocessRunner -from autoskillit.hooks._write_scope import output_dir_widens_scope, widened_output_dir_message +from autoskillit.hooks._write_scope import ( + WriteScopeKind, + output_dir_widens_scope, + widened_output_dir_message, +) from autoskillit.pipeline import canonical_step_name as _canonical_step_name from autoskillit.pipeline import gate_error_result from autoskillit.server._explorer_projection import _build_requested_execution_identity @@ -479,6 +483,15 @@ def _materialize_fresh_session(state: _RunSkillDispatchState) -> str | None: def _prepare_owned_write_scope(state: _RunSkillDispatchState) -> str | None: state.allowed_write_prefix = "" state.allowed_write_prefixes = () + # An implicit artifact watch directory does not narrow an unrestricted skill. + if ( + not state.is_read_only + and not state.output_dir + and state.invocation is not None + and state.invocation.root.write_scope is not None + and state.invocation.root.write_scope.kind is WriteScopeKind.UNRESTRICTED + ): + return None if state.write_watch_dirs: state.allowed_write_prefix, state.allowed_write_prefixes = _compute_write_prefixes( state.write_watch_dirs, state.cwd, state.skill_command diff --git a/src/autoskillit/workspace/clone/__init__.py b/src/autoskillit/workspace/clone/__init__.py index 2a03e8f723..d7983268f0 100644 --- a/src/autoskillit/workspace/clone/__init__.py +++ b/src/autoskillit/workspace/clone/__init__.py @@ -417,6 +417,30 @@ def _classify_push_failure(stderr_text: str, *, force: bool) -> str | None: return None +def _resolve_push_url(source_dir: str, remote_url: str, *, clone_path: str, branch: str) -> str: + """Resolve the push URL without changing the source checkout.""" + if remote_url: + return remote_url + if not source_dir: + logger.error("push_to_remote_no_url_or_source", clone_path=clone_path, branch=branch) + raise RuntimeError("push_to_remote: neither remote_url nor source_dir was provided") + result = subprocess.run( + ["git", "remote", "get-url", "origin"], + cwd=source_dir, + capture_output=True, + text=True, + ) + if result.returncode != 0: + logger.error( + "push_to_remote_get_url_failed", + source_dir=source_dir, + branch=branch, + stderr=result.stderr.strip(), + ) + raise RuntimeError(result.stderr.strip()) + return result.stdout.strip() + + def push_to_remote( clone_path: str, source_dir: str = "", @@ -471,49 +495,31 @@ def push_to_remote( ), } - resolved_url = remote_url - if not resolved_url: - if not source_dir: - logger.error("push_to_remote_no_url_or_source", clone_path=clone_path, branch=branch) - return { - "success": False, - "stderr": "push_to_remote: neither remote_url nor source_dir was provided", - } - url_result = subprocess.run( - ["git", "remote", "get-url", "origin"], - cwd=source_dir, - capture_output=True, - text=True, + try: + resolved_url = _resolve_push_url( + source_dir, remote_url, clone_path=clone_path, branch=branch ) - if url_result.returncode != 0: + # Classify the resolved URL to catch non-bare local remotes early + url_type = classify_remote_url(resolved_url) + if url_type == "nonbare_local": logger.error( - "push_to_remote_get_url_failed", - source_dir=source_dir, + "push_to_remote_nonbare_local", + clone_path=clone_path, + remote_url=resolved_url, branch=branch, - stderr=url_result.stderr.strip(), ) - return {"success": False, "stderr": url_result.stderr.strip()} - resolved_url = url_result.stdout.strip() - - # Classify the resolved URL to catch non-bare local remotes early - url_type = classify_remote_url(resolved_url) - if url_type == "nonbare_local": - logger.error( - "push_to_remote_nonbare_local", - clone_path=clone_path, - remote_url=resolved_url, - branch=branch, - ) - return { - "success": False, - "error_type": "local_non_bare_remote", - "stderr": ( - "push_to_remote: target is a non-bare local repository with the branch " - "checked out. Use a bare repository or a network remote." - ), - } + return { + "success": False, + "error_type": "local_non_bare_remote", + "stderr": ( + "push_to_remote: target is a non-bare local repository with the branch " + "checked out. Use a bare repository or a network remote." + ), + } + destination = _matching_push_remote(Path(clone_path), resolved_url) + except (RuntimeError, OSError, subprocess.SubprocessError) as exc: + return {"success": False, "stderr": str(exc)} - destination = _matching_push_remote(Path(clone_path), resolved_url) push_cmd = ["git", "push", "-u", "--", destination, branch] if force: push_cmd.insert(3, "--force-with-lease") diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 11282366ef..bd5b47daf8 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -1180,6 +1180,8 @@ class SourceMapRejection(enum.StrEnum): # file-level: the Docker pin guard compares CODEX_CLI_MIN_VERSION from # execution.backends with the Dockerfile's CODEX_VERSION pin. "infra/test_docker_image.py", + # imports execution.backends.claude.ClaudeCodeBackend for FLEET_MODEL derivation. + "infra/test_e2e_harness.py", # file-level: the deck column drift guard imports # execution._report_index_types.ReportSessionRow. "report/test_deck_registry.py", diff --git a/tests/arch/test_git_ref_qualification.py b/tests/arch/test_git_ref_qualification.py index 832065b95c..392c9be94b 100644 --- a/tests/arch/test_git_ref_qualification.py +++ b/tests/arch/test_git_ref_qualification.py @@ -220,7 +220,7 @@ "", "UNRESOLVABLE", ), - ("workspace/clone/__init__.py", "push_to_remote", 517, "push", "BARE"), + ("workspace/clone/__init__.py", "push_to_remote", 523, "push", "BARE"), ) _DEFERRED_REF_SITES: dict[tuple[str, str, int, str, str], TrackedDeferral] = {} diff --git a/tests/execution/test_session_model_peak_context.py b/tests/execution/test_session_model_peak_context.py index c71dcf8374..d89fb75ba4 100644 --- a/tests/execution/test_session_model_peak_context.py +++ b/tests/execution/test_session_model_peak_context.py @@ -40,6 +40,24 @@ def _assistant( return json.dumps(record) +def _init(model: str | None) -> str: + record: dict[str, object] = { + "type": "system", + "subtype": "init", + "session_id": "s", + } + if model is not None: + record["model"] = model + return json.dumps(record) + + +def _subagent(model: str, cache_read: object, message_id: str) -> str: + record = json.loads(_assistant(cache_read=cache_read, message_id=message_id, model=model)) + record["subagent_type"] = "Explore" + record["parent_tool_use_id"] = "call_1" + return json.dumps(record) + + def _result(cache_read: int = 0) -> str: return json.dumps( { @@ -177,11 +195,140 @@ def test_duplicate_message_snapshots_count_once_in_rows_and_aggregate(): assert [row["message_id"] for row in rows] == [f"msg-{index}" for index in range(1, 6)] -def test_rows_use_exact_model_windows_and_inclusive_input(): +@pytest.mark.parametrize( + ("init_models", "parent_models", "model_usage", "include_subagent", "window"), + [ + pytest.param( + ("claude-sonnet-5",), + ("MiniMax-M3",), + {"claude-sonnet-5": {"contextWindow": 200_000}}, + False, + 200_000, + id="gateway-unsuffixed", + ), + pytest.param( + ("claude-sonnet-5[1m]",), + ("MiniMax-M3",), + {"claude-sonnet-5[1m]": {"contextWindow": 1_000_000}}, + False, + 1_000_000, + id="gateway-suffixed", + ), + pytest.param( + ("claude-opus-4-6[1m]",), + ("MiniMax-M3",), + { + "claude-opus-4-6[1m]": {"contextWindow": 1_000_000}, + "MiniMax-M3": {"contextWindow": 200_000}, + }, + True, + 1_000_000, + id="subagent-served-name-collision", + ), + pytest.param( + ("claude-sonnet-5",), + ("claude-sonnet-5",), + {"claude-sonnet-5": {"contextWindow": 1_000_000}}, + False, + 1_000_000, + id="first-party-bare-name", + ), + pytest.param( + ("haiku",), + ("claude-haiku-4-5-20251001",), + {"claude-haiku-4-5-20251001": {"contextWindow": 200_000}}, + False, + 200_000, + id="alias-launch", + ), + pytest.param( + ("claude-sonnet-5[1m]",) * 3, + ("MiniMax-M3",), + {"claude-sonnet-5[1m]": {"contextWindow": 1_000_000}}, + False, + 1_000_000, + id="repeated-init-same-model", + ), + ], +) +def test_parent_turn_window_is_the_requested_models_window( + init_models: tuple[str, ...], + parent_models: tuple[str, ...], + model_usage: dict[str, dict[str, object]], + include_subagent: bool, + window: int, +): + lines = [_init(model) for model in init_models] + lines.extend( + _assistant(cache_read=100, message_id=f"parent-{index}", model=model) + for index, model in enumerate(parent_models, start=1) + ) + if include_subagent: + lines.append(_subagent("MiniMax-M3", 100, "subagent-1")) + lines.append(_model_usage_result(model_usage)) + + _usage, rows = extract_token_usage(_build_ndjson(lines)) + + assert [row["context_window_tokens"] for row in rows] == [window] * len(parent_models) + + +@pytest.mark.parametrize( + ("init_model", "model_usage", "include_subagent"), + [ + pytest.param( + None, + { + "claude-opus-4-6[1m]": {"contextWindow": 1_000_000}, + "MiniMax-M3": {"contextWindow": 200_000}, + }, + False, + id="missing-init", + ), + pytest.param( + "claude-sonnet-5", + { + "MiniMax-M3": {"contextWindow": 200_000}, + "claude-haiku-4-5": {"contextWindow": 400_000}, + }, + False, + id="requested-model-not-in-usage", + ), + pytest.param( + "claude-sonnet-5", + { + "claude-sonnet-5": {"contextWindow": 0}, + "MiniMax-M3": {"contextWindow": 200_000}, + }, + True, + id="invalid-requested-window-with-valid-subagent-window", + ), + ], +) +def test_parent_turn_window_is_unknown_when_the_requested_model_is_ambiguous( + init_model: str | None, + model_usage: dict[str, dict[str, object]], + include_subagent: bool, +): + lines = [] if init_model is None else [_init(init_model)] + lines.append(_assistant(cache_read=100, message_id="parent-1", model="MiniMax-M3")) + if include_subagent: + lines.append(_subagent("MiniMax-M3", 100, "subagent-1")) + lines.append(_model_usage_result(model_usage)) + + _usage, rows = extract_token_usage(_build_ndjson(lines)) + + assert len(rows) == 1 + assert rows[0]["context_window_tokens"] is None + assert rows[0]["context_fraction"] is None + + +def test_rows_use_requested_model_window_and_inclusive_input(): opus = "claude-opus-4-6-20260901" sonnet = "claude-sonnet-4-6-20260901" + requested_model = "claude-sonnet-5" stdout = _build_ndjson( [ + _init(requested_model), _assistant( cache_read=100, message_id="opus-message", @@ -198,8 +345,7 @@ def test_rows_use_exact_model_windows_and_inclusive_input(): ), _model_usage_result( { - opus: {"contextWindow": 50}, - sonnet: {"contextWindow": 200}, + requested_model: {"contextWindow": 200}, } ), ] @@ -212,19 +358,20 @@ def test_rows_use_exact_model_windows_and_inclusive_input(): assert [row["backend"] for row in rows] == ["claude-code", "claude-code"] assert [row["model"] for row in rows] == [opus, sonnet] assert [row["input_tokens"] for row in rows] == [115, 125] - assert [row["context_window_tokens"] for row in rows] == [50, 200] - assert [row["context_fraction"] for row in rows] == [2.0, 0.5] + assert [row["context_window_tokens"] for row in rows] == [200, 200] + assert [row["context_fraction"] for row in rows] == [0.5, 0.5] -@pytest.mark.parametrize("window", [None, 0, -1, True, 200.0, "200"]) +@pytest.mark.parametrize("window", [0, -1, True, 200.0, "200"]) def test_unknown_or_invalid_context_window_leaves_fraction_unknown( window: object, ): - model = "another-model" if window is None else "claude-sonnet-4-6" + model = "claude-sonnet-4-6" stdout = _build_ndjson( [ + _init(model), _assistant(cache_read=100, message_id="message-1"), - _model_usage_result({model: {"contextWindow": 200 if window is None else window}}), + _model_usage_result({model: {"contextWindow": window}}), ] ) @@ -234,21 +381,21 @@ def test_unknown_or_invalid_context_window_leaves_fraction_unknown( assert rows[0]["context_fraction"] is None -def test_conflicting_windows_invalidate_only_the_affected_model(): +def test_conflicting_windows_for_the_requested_model_leave_window_unknown(): opus = "claude-opus-4-6" sonnet = "claude-sonnet-4-6" + requested_model = "claude-sonnet-5" stdout = _build_ndjson( [ + _init(requested_model), _assistant(cache_read=100, message_id="opus-message", model=opus), _assistant(cache_read=100, message_id="sonnet-message", model=sonnet), - _model_usage_result({opus: {"contextWindow": 200}, sonnet: {"contextWindow": 400}}), - _model_usage_result({opus: {"contextWindow": 300}, sonnet: {"contextWindow": 400}}), + _model_usage_result({requested_model: {"contextWindow": 200}}), + _model_usage_result({requested_model: {"contextWindow": 300}}), ] ) _usage, rows = extract_token_usage(stdout) - assert rows[0]["context_window_tokens"] is None - assert rows[0]["context_fraction"] is None - assert rows[1]["context_window_tokens"] == 400 - assert rows[1]["context_fraction"] == 0.25 + assert [row["context_window_tokens"] for row in rows] == [None, None] + assert [row["context_fraction"] for row in rows] == [None, None] diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 0ecd87e78a..c2236f0ef3 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -229,6 +229,8 @@ def test_pipeline_runs_directory_matches_clone_source_of_truth() -> None: SANDBOX_TOKEN = "sandbox-test-token" SANDBOX = "owner/sandbox" _MINIMAX_MODEL = "MiniMax-M3[1m]" +_EXPECTED_FLEET_MODEL = "claude-sonnet-5[1m]" +_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS = 1_000_000 _EXPECTED_SETTINGS_ENV = { "ANTHROPIC_BASE_URL": "https://api.minimax.io/anthropic", "ANTHROPIC_AUTH_TOKEN": MINIMAX_KEY, @@ -240,7 +242,14 @@ def test_pipeline_runs_directory_matches_clone_source_of_truth() -> None: "CLAUDE_CODE_MAX_CONCURRENT_SUBAGENTS": "6", } _CANARY_REPLY = json.dumps( - {"type": "result", "is_error": False, "result": "AUTOSKILLIT-E2E-CANARY-OK"} + { + "type": "result", + "is_error": False, + "result": "AUTOSKILLIT-E2E-CANARY-OK", + "modelUsage": { + _EXPECTED_FLEET_MODEL: {"contextWindow": _EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS} + }, + } ) _SKILL_NOTICE = "2 skills unavailable on this backend (codex): a, b" _CLEAN_INSTALL_ISSUE = "https://github.com/TalonT-Org/AutoSkillit/issues/5231" @@ -292,11 +301,18 @@ def _pr(number: int, state: str, additions: int = 3, deletions: int = 1) -> dict def _recipe_handler( *, + log_dir: Path, new_prs: list[dict] | None = None, fleet: subprocess.CompletedProcess[str] | BaseException | None = None, + sessions: dict[str, list[int | None] | None] | None = None, extra: Handler | None = None, ) -> Handler: listed = [_pr(8, "OPEN"), _pr(7, "MERGED")] if new_prs is None else new_prs + fleet_sessions = ( + {"fleet-session": [_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS]} + if sessions is None + else sessions + ) def handle(argv: list[str]): if extra is not None and (response := extra(argv)) is not None: @@ -305,6 +321,7 @@ def handle(argv: list[str]): baseline = argv[argv.index("--limit") + 1] == "1" return _completed(argv, json.dumps([{"number": 7}] if baseline else listed)) if argv[:3] == ["autoskillit", "fleet", "run"]: + _seed_sessions(log_dir, fleet_sessions) return fleet or _completed(argv, f'{_SKILL_NOTICE}\n{{"success": true}}\n') return None @@ -331,6 +348,27 @@ def _recipe_catalog(state: str = "open"): ) +def _seed_sessions(log_dir: Path, sessions: dict[str, list[int | None] | None]) -> None: + log_dir.mkdir(parents=True, exist_ok=True) + with (log_dir / "sessions.jsonl").open("a", encoding="utf-8") as index: + for name, windows in sessions.items(): + index.write(json.dumps({"dir_name": name, "session_id": name}) + "\n") + if windows is None: + continue + usage = log_dir / "sessions" / name / "turn_usage.jsonl" + usage.parent.mkdir(parents=True, exist_ok=True) + usage.write_text( + "".join( + json.dumps({"context_window_tokens": window}) + "\n" for window in windows + ), + encoding="utf-8", + ) + + +def _log_dir(tmp_path: Path) -> Path: + return tmp_path / "home" / ".local" / "share" / "autoskillit" / "logs" + + def _env(**overrides: str) -> dict[str, str]: env = { "PATH": "/usr/bin", @@ -344,8 +382,21 @@ def _env(**overrides: str) -> dict[str, str]: return {name: value for name, value in env.items() if value} -def _run_recipe(tmp_path: Path, runner: FakeRunner, *, state: str = "open", env=None): +def _run_recipe( + tmp_path: Path, + runner: FakeRunner, + *, + state: str = "open", + expected_failures: list[dict[str, str]] | None = None, + env=None, +): catalog = _recipe_catalog(state) + if expected_failures is not None: + test = replace(catalog.get("impl"), expected_failures=tuple(expected_failures)) + catalog = replace( + catalog, + tests=tuple(test if entry.name == test.name else entry for entry in catalog.tests), + ) out = tmp_path / "out" failures = harness.run_test( catalog.get("impl"), @@ -442,6 +493,7 @@ def _smoke_fleet( out: Path, fleet: subprocess.CompletedProcess[str] | BaseException | None, state: dict[str, str], + sessions: dict[str, list[int | None] | None] | None = None, ) -> subprocess.CompletedProcess[str] | BaseException: assert argv[:3] == ["autoskillit", "fleet", "run"] and call.cwd == clone assert (clone / ".autoskillit" / "recipes" / "sandbox-smoke.yaml").is_file() @@ -460,6 +512,13 @@ def _smoke_fleet( "base_branch": "main", "branch_name": descriptor["branch_name"], } + fleet_sessions = ( + {"fleet-session": [_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS]} + if sessions is None + else sessions + ) + assert call.env is not None + _seed_sessions(Path(call.env["AUTOSKILLIT_LOG_DIR"]), fleet_sessions) if isinstance(fleet, BaseException): return fleet if fleet is not None: @@ -615,6 +674,7 @@ def _smoke_runner( *, mode: str = "success", fleet: subprocess.CompletedProcess[str] | BaseException | None = None, + sessions: dict[str, list[int | None] | None] | None = None, ) -> tuple[FakeRunner, Path]: clone = tmp_path / "sandbox" clone.mkdir() @@ -632,7 +692,7 @@ def _smoke_runner( ("git", "branch"): lambda argv, _call: _smoke_metadata(argv, mode, state, remote_url), ("git", "ls-remote"): lambda argv, _call: _smoke_metadata(argv, mode, state, remote_url), ("autoskillit", "fleet"): lambda argv, call: _smoke_fleet( - argv, call, clone, out, fleet, state + argv, call, clone, out, fleet, state, sessions ), ("gh", "pr"): lambda argv, _call: _smoke_pr_list(argv, mode, state), ("gh", "api"): lambda argv, _call: _smoke_ref(argv, mode, state), @@ -743,6 +803,7 @@ def test_fixture_stages_discovers_and_passes_the_production_validator(self, tmp_ assert worker.skill_name == "smoke-task" prompt = worker.with_args["skill_command"].lower() assert "one new commit" in prompt and "only sandbox/text.py" in prompt + assert "report the commit hash" in prompt and "changed file paths" in prompt discipline = " ".join(recipe.kitchen_rules).lower() assert "child agents" in discipline and "replay" in discipline assert recipe.steps["setup"].with_args["cwd"] == "${{ context.worktree_path }}" @@ -984,6 +1045,21 @@ def test_branch_collision_stops_before_launch_marker(self, tmp_path, monkeypatch assert (out / "smoke-launched").exists() is False assert not any(call.argv[:3] == ["autoskillit", "fleet", "run"] for call in runner.calls) + def test_new_session_with_a_small_window_fails_the_smoke_run(self, tmp_path, monkeypatch): + runner, _clone = _smoke_runner( + tmp_path, + monkeypatch, + sessions={"too-small": [200000]}, + ) + failures, result, _out, _catalog = _run_smoke(tmp_path, runner, monkeypatch) + + assert result["outcome"] == "failed" + assert ( + f"session windows: too-small: observed [200000] != required " + f"{_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}" + ) in result["failures"] + assert failures == result["failures"] + def test_expected_failure_matches_only_the_exact_structured_envelope( self, tmp_path, monkeypatch ): @@ -1533,10 +1609,95 @@ def test_shipped_config_routes_every_default_codex_step_to_claude_code( assert cfg.features["fleet_headless_run"] is True +def test_fleet_launch_shape_matches_shipped_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from autoskillit.config import load_config + from autoskillit.execution.backends.claude import ClaudeCodeBackend + + home = tmp_path / "home" + monkeypatch.setattr("pathlib.Path.home", lambda: home) + harness.install_autoskillit_config(home) + project = tmp_path / "project" + project.mkdir() + cfg = load_config(project) + + assert harness.FLEET_MODEL == ClaudeCodeBackend().translate_model(cfg.model.default_model) + assert int(harness.MINIMAX_COMPACT_WINDOW_TOKENS) <= harness.REQUIRED_CONTEXT_WINDOW_TOKENS + + class TestCanaryOutput: def test_successful_reply_passes(self) -> None: assert harness.check_canary_output(_CANARY_REPLY) == [] + def test_context_window_must_match_the_required_window(self) -> None: + reply = json.loads(_CANARY_REPLY) + reply["modelUsage"] = {_EXPECTED_FLEET_MODEL: {"contextWindow": 200000}} + + assert harness.check_canary_output(json.dumps(reply)) == [ + f"canary: {_EXPECTED_FLEET_MODEL} reported a 200000-token context window; " + f"the harness requires {_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}" + ] + + @pytest.mark.parametrize( + ("model_usage", "include_model_usage"), + [ + pytest.param( + {"MiniMax-M3[1m]": {"contextWindow": _EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}}, + True, + id="old-model-key", + ), + pytest.param(None, False, id="missing-model-usage"), + pytest.param(None, True, id="non-dict-model-usage"), + pytest.param({_EXPECTED_FLEET_MODEL: "invalid"}, True, id="non-dict-model-entry"), + ], + ) + def test_missing_expected_model_usage_fails( + self, model_usage: object, include_model_usage: bool + ) -> None: + reply = json.loads(_CANARY_REPLY) + if include_model_usage: + reply["modelUsage"] = model_usage + else: + reply.pop("modelUsage") + + assert harness.check_canary_output(json.dumps(reply)) == [ + f"canary: claude output has no modelUsage entry for {_EXPECTED_FLEET_MODEL}" + ] + + @pytest.mark.parametrize( + "model_usage", + [ + {_EXPECTED_FLEET_MODEL: {"contextWindow": "1000000"}}, + {_EXPECTED_FLEET_MODEL: {}}, + {_EXPECTED_FLEET_MODEL: {"contextWindow": True}}, + ], + ids=["string", "missing-window", "bool"], + ) + def test_context_window_must_be_an_integer(self, model_usage: dict) -> None: + reply = json.loads(_CANARY_REPLY) + reply["modelUsage"] = model_usage + + assert harness.check_canary_output(json.dumps(reply)) == [ + f"canary: {_EXPECTED_FLEET_MODEL} reported no integer context window" + ] + + def test_reply_and_error_checks_keep_their_messages_before_window_validation(self) -> None: + missing_token = json.loads(_CANARY_REPLY) + missing_token.pop("modelUsage") + missing_token["result"] = "missing token" + assert harness.check_canary_output(json.dumps(missing_token)) == [ + "canary: reply does not contain AUTOSKILLIT-E2E-CANARY-OK" + ] + + error_reply = json.loads(_CANARY_REPLY) + error_reply.pop("modelUsage") + error_reply["is_error"] = True + error_reply["result"] = "claude failed" + assert harness.check_canary_output(json.dumps(error_reply)) == [ + "canary: claude reported an error: claude failed" + ] + @pytest.mark.parametrize( "stdout", [ @@ -1593,9 +1754,106 @@ def test_unexpected_pull_requests_fail(self, prs: list[dict]) -> None: assert harness.check_pull_requests(prs, "merged") +class TestSessionWindowGate: + def test_malformed_session_index_names_the_file_and_row(self, tmp_path: Path) -> None: + path = tmp_path / "sessions.jsonl" + path.write_text("\n{broken\n", encoding="utf-8") + + with pytest.raises(ValueError) as error: + harness.session_index_dir_names(tmp_path) + + assert str(error.value).startswith(f"{path}:2: invalid session index JSON:") + + def test_malformed_turn_usage_names_the_session_file_and_row(self, tmp_path: Path) -> None: + log_dir = tmp_path / "logs" + _seed_sessions(log_dir, {"broken-session": []}) + path = log_dir / "sessions" / "broken-session" / "turn_usage.jsonl" + path.write_text("\n{broken\n", encoding="utf-8") + + failures = harness.check_session_windows(log_dir, set(), sessions_required=True) + + assert len(failures) == 1 + assert failures[0].startswith( + f"session windows: broken-session: {path}:2: invalid turn usage JSON:" + ) + + def test_all_new_sessions_must_report_the_required_window(self, tmp_path: Path) -> None: + log_dir = tmp_path / "logs" + _seed_sessions( + log_dir, + { + "session-a": [_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS], + "session-b": [ + _EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS, + _EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS, + ], + }, + ) + + assert harness.check_session_windows(log_dir, set(), sessions_required=True) == [] + + @pytest.mark.parametrize("sessions_required", [True, False]) + def test_wrong_window_fails_for_a_new_session_even_when_none_are_required( + self, tmp_path: Path, sessions_required: bool + ) -> None: + log_dir = tmp_path / "logs" + _seed_sessions(log_dir, {"too-small": [200000]}) + + assert harness.check_session_windows( + log_dir, set(), sessions_required=sessions_required + ) == [ + f"session windows: too-small: observed [200000] != required " + f"{_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}" + ] + + def test_missing_context_window_row_fails(self, tmp_path: Path) -> None: + log_dir = tmp_path / "logs" + _seed_sessions(log_dir, {"missing-window": [None]}) + + assert harness.check_session_windows(log_dir, set(), sessions_required=True) == [ + "session windows: missing-window: no observed context window" + ] + + def test_missing_turn_usage_file_fails(self, tmp_path: Path) -> None: + log_dir = tmp_path / "logs" + _seed_sessions(log_dir, {"missing-usage": None}) + + assert harness.check_session_windows(log_dir, set(), sessions_required=True) == [ + "session windows: missing-usage: no turn usage" + ] + + @pytest.mark.parametrize("sessions_required", [True, False]) + def test_no_new_sessions_depends_on_whether_the_fleet_was_required( + self, tmp_path: Path, sessions_required: bool + ) -> None: + log_dir = tmp_path / "logs" + failures = harness.check_session_windows( + log_dir, set(), sessions_required=sessions_required + ) + + expected = ( + [f"session windows: the fleet run recorded no sessions in {log_dir}"] + if sessions_required + else [] + ) + assert failures == expected + + def test_bad_stale_session_is_ignored_by_dir_name(self, tmp_path: Path) -> None: + log_dir = tmp_path / "logs" + _seed_sessions( + log_dir, + { + "stale": [200000], + "new": [_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS], + }, + ) + + assert harness.check_session_windows(log_dir, {"stale"}, sessions_required=True) == [] + + class TestRecipeFlow: def test_steps_run_in_order_with_the_token_only_on_stdin(self, tmp_path: Path) -> None: - runner = FakeRunner(_recipe_handler()) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path))) failures, result = _run_recipe(tmp_path, runner) assert failures == [] @@ -1628,13 +1886,17 @@ def test_steps_run_in_order_with_the_token_only_on_stdin(self, tmp_path: Path) - ] assert [call.input for call in runner.calls if call.input] == [SANDBOX_TOKEN] assert all(SANDBOX_TOKEN not in " ".join(call.argv) for call in runner.calls) - assert all(call.env == harness.child_env(_env()) for call in runner.calls) + expected_env = { + **harness.child_env(_env()), + "AUTOSKILLIT_LOG_DIR": str(_log_dir(tmp_path)), + } + assert all(call.env == expected_env for call in runner.calls) fleet_call = runner.calls[6] assert fleet_call.cwd == harness.SANDBOX_CLONE assert fleet_call.timeout == 600 + e2e_catalog.HARNESS_GRACE_SEC def test_outputs_keep_full_stdout_and_only_the_envelope_line(self, tmp_path: Path) -> None: - _run_recipe(tmp_path, FakeRunner(_recipe_handler())) + _run_recipe(tmp_path, FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path)))) out = tmp_path / "out" stdout = (out / "fleet-run.stdout.log").read_text(encoding="utf-8") assert stdout == f'{_SKILL_NOTICE}\n{{"success": true}}\n' @@ -1642,7 +1904,7 @@ def test_outputs_keep_full_stdout_and_only_the_envelope_line(self, tmp_path: Pat assert (out / "fleet-run.stderr.log").exists() def test_harness_writes_provider_settings_and_user_config(self, tmp_path: Path) -> None: - _run_recipe(tmp_path, FakeRunner(_recipe_handler())) + _run_recipe(tmp_path, FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path)))) home = tmp_path / "home" settings = json.loads((home / ".claude" / "settings.json").read_text(encoding="utf-8")) assert settings["env"] == _EXPECTED_SETTINGS_ENV @@ -1650,7 +1912,9 @@ def test_harness_writes_provider_settings_and_user_config(self, tmp_path: Path) assert installed == harness.CONFIG_TEMPLATE.read_bytes() def test_merged_pull_request_is_not_closed(self, tmp_path: Path) -> None: - runner = FakeRunner(_recipe_handler(new_prs=[_pr(8, "MERGED")])) + runner = FakeRunner( + _recipe_handler(log_dir=_log_dir(tmp_path), new_prs=[_pr(8, "MERGED")]) + ) failures, _ = _run_recipe(tmp_path, runner, state="merged") assert failures == [] assert ("gh", "pr", "close") not in runner.argv_prefixes() @@ -1661,7 +1925,7 @@ def reject_login(argv: list[str]): return _completed(argv, returncode=1, stderr="bad credentials") return None - runner = FakeRunner(_recipe_handler(extra=reject_login)) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path), extra=reject_login)) failures, result = _run_recipe(tmp_path, runner) prefixes = runner.argv_prefixes() assert result["passed"] is False @@ -1670,19 +1934,50 @@ def reject_login(argv: list[str]): assert ("gh", "pr", "close") not in prefixes assert ("autoskillit", "fleet", "run") not in prefixes - def test_fleet_timeout_is_a_failure_and_cleanup_still_runs(self, tmp_path: Path) -> None: + def test_fleet_timeout_and_window_failures_keep_cleanup(self, tmp_path: Path) -> None: timeout = subprocess.TimeoutExpired(["autoskillit"], 720) - runner = FakeRunner(_recipe_handler(fleet=timeout)) + runner = FakeRunner( + _recipe_handler( + log_dir=_log_dir(tmp_path), + fleet=timeout, + sessions={"timed-out": [200000]}, + ) + ) failures, result = _run_recipe(tmp_path, runner) assert result["passed"] is False assert any("fleet run: timed out" in failure for failure in failures) + assert ( + f"session windows: timed-out: observed [200000] != required " + f"{_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}" + ) in failures assert runner.calls[-1].argv[:4] == ["gh", "pr", "close", "8"] def test_unexpected_fleet_error_records_failure_and_still_cleans_up( - self, tmp_path: Path + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - runner = FakeRunner(_recipe_handler(fleet=RuntimeError("unexpected fleet error"))) + events: list[str] = [] + original_check = harness.check_session_windows + original_format_exc = harness.traceback.format_exc + + def check_session_windows(*args, **kwargs): + events.append("window gate") + return original_check(*args, **kwargs) + + def format_exc(*args, **kwargs): + events.append("exception recorded") + return original_format_exc(*args, **kwargs) + + monkeypatch.setattr(harness, "check_session_windows", check_session_windows) + monkeypatch.setattr(harness.traceback, "format_exc", format_exc) + runner = FakeRunner( + _recipe_handler( + log_dir=_log_dir(tmp_path), + fleet=RuntimeError("unexpected fleet error"), + ) + ) failures, result = _run_recipe(tmp_path, runner) + assert events.index("window gate") < events.index("exception recorded") + assert events.count("window gate") == 1 assert failures == ["harness error: RuntimeError: unexpected fleet error"] exception = result.pop("exception") assert "Traceback (most recent call last)" in exception @@ -1705,7 +2000,7 @@ def no_gh(argv: list[str]): else None ) - runner = FakeRunner(_recipe_handler(extra=no_gh)) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path), extra=no_gh)) failures, result = _run_recipe(tmp_path, runner) assert result["passed"] is False assert failures[0].startswith("gh auth login:") @@ -1719,7 +2014,7 @@ def flaky_listing(argv: list[str]): return _completed(argv, next(listings)) return None - runner = FakeRunner(_recipe_handler(extra=flaky_listing)) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path), extra=flaky_listing)) failures, _ = _run_recipe(tmp_path, runner) assert failures == ["gh pr list: output is not JSON"] assert runner.calls[-1].argv[:4] == ["gh", "pr", "close", "8"] @@ -1731,7 +2026,7 @@ def malformed_listing(argv: list[str]): return _completed(argv, json.dumps([{"state": "OPEN"}])) return None - runner = FakeRunner(_recipe_handler(extra=malformed_listing)) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path), extra=malformed_listing)) failures, result = _run_recipe(tmp_path, runner) assert "gh pr list: pull request number is missing or invalid" in failures assert result == { @@ -1744,12 +2039,44 @@ def malformed_listing(argv: list[str]): assert ("gh", "pr", "close") not in runner.argv_prefixes() def test_missing_sandbox_token_fails_before_any_launch(self, tmp_path: Path) -> None: - runner = FakeRunner(_recipe_handler()) + runner = FakeRunner(_recipe_handler(log_dir=_log_dir(tmp_path))) failures, result = _run_recipe(tmp_path, runner, env=_env(E2E_SANDBOX_TOKEN="")) assert failures == ["E2E_SANDBOX_TOKEN is not set"] assert result["passed"] is False assert runner.calls == [] + def test_expected_failure_keeps_a_window_failure(self, tmp_path: Path) -> None: + expected = _SMOKE_FAILURE + fleet = _completed( + ["autoskillit", "fleet", "run"], + json.dumps( + { + "success": False, + "kind": "completed", + "dispatch_status": "failed", + "error": "workflow_failed", + "user_visible_message": "model reported failure", + } + ), + returncode=1, + ) + runner = FakeRunner( + _recipe_handler( + log_dir=_log_dir(tmp_path), + fleet=fleet, + sessions={"too-small": [200000]}, + ) + ) + + failures, result = _run_recipe(tmp_path, runner, expected_failures=[expected]) + + assert ( + f"session windows: too-small: observed [200000] != required " + f"{_EXPECTED_REQUIRED_CONTEXT_WINDOW_TOKENS}" + ) in failures + assert result["outcome"] == "failed" + assert result["expected_findings"] == [expected] + class TestCanaryFlow: def test_canary_runs_one_claude_prompt(self, tmp_path: Path, monkeypatch) -> None: @@ -1758,7 +2085,15 @@ def test_canary_runs_one_claude_prompt(self, tmp_path: Path, monkeypatch) -> Non assert failures == [] assert result == {"test": "canary", "passed": True, "failures": []} assert [call.argv for call in runner.calls] == [ - ["claude", "-p", harness.CANARY_PROMPT, "--output-format", "json"] + [ + "claude", + "-p", + harness.CANARY_PROMPT, + "--output-format", + "json", + "--model", + harness.FLEET_MODEL, + ] ] assert runner.calls[0].env == harness.child_env(_env()) assert (tmp_path / "out" / "canary.json").read_text(encoding="utf-8") == _CANARY_REPLY diff --git a/tests/server/test_tools_execution_step_resolution.py b/tests/server/test_tools_execution_step_resolution.py index 25bb9ffab9..a452ee4bbe 100644 --- a/tests/server/test_tools_execution_step_resolution.py +++ b/tests/server/test_tools_execution_step_resolution.py @@ -177,8 +177,7 @@ async def test_run_skill_logs_warning_when_output_dir_resolved_from_recipe( async def test_run_skill_skips_auto_fill_when_output_dir_has_template( tool_ctx_kitchen_open, monkeypatch, tmp_path ) -> None: - """When recipe step output_dir has ${{ }}, auto-fill MUST skip AND fallback - to resolve_skill_temp_dir — producing the correct skill-scoped prefix.""" + """Unresolved output_dir templates use the default artifact watch directory.""" from autoskillit.recipe.schema import RecipeStep from tests.fakes import InMemoryHeadlessExecutor @@ -202,9 +201,10 @@ async def test_run_skill_skips_auto_fill_when_output_dir_has_template( cwd=str(tmp_path), step_name="verify", ) - expected = str(tmp_path / ".autoskillit" / "temp" / "dry-walkthrough") + "/" - assert executor.calls[0].allowed_write_prefix == expected - assert executor.calls[0].allowed_write_prefixes == (expected,) + call = executor.calls[0] + assert tmp_path / ".autoskillit" / "temp" / "dry-walkthrough" in call.write_watch_dirs + assert call.allowed_write_prefix == "" + assert call.allowed_write_prefixes == () @pytest.mark.anyio diff --git a/tests/server/test_tools_execution_write_prefix.py b/tests/server/test_tools_execution_write_prefix.py index 01bfa5c80d..fd8f2e72e0 100644 --- a/tests/server/test_tools_execution_write_prefix.py +++ b/tests/server/test_tools_execution_write_prefix.py @@ -365,6 +365,40 @@ async def test_investigate_contract_runs_writable_with_report_watch_dir( # --------------------------------------------------------------------------- +@pytest.mark.anyio +@pytest.mark.parametrize( + ("read_only", "output_dir"), + [(False, ""), (True, ""), (False, ".autoskillit/temp/smoke-output")], +) +async def test_unrestricted_skill_does_not_inherit_an_implicit_temp_write_limit( + tool_ctx_kitchen_open, monkeypatch, tmp_path: Path, read_only: bool, output_dir: str +) -> None: + from tests.fakes import InMemoryHeadlessExecutor + + executor = InMemoryHeadlessExecutor() + tool_ctx_kitchen_open.executor = executor + monkeypatch.setattr(tool_ctx_kitchen_open, "read_only_resolver", lambda _command: read_only) + monkeypatch.setattr("autoskillit.server._ctx", tool_ctx_kitchen_open) + + await run_skill( + "/autoskillit:smoke-task append a function to sandbox/text.py", + str(tmp_path), + output_dir=output_dir, + ) + + assert len(executor.calls) == 1 + call = executor.calls[0] + assert call.readonly_skill is read_only + target = tmp_path / (output_dir or ".autoskillit/temp/smoke-task") + assert target in call.write_watch_dirs + if not read_only and not output_dir: + assert not call.allowed_write_prefix + assert not call.allowed_write_prefixes + else: + assert call.allowed_write_prefix == str(target) + "/" + assert call.allowed_write_prefixes == (str(target) + "/",) + + def _scoped_skill(tmp_path: Path, name: str, declaration: str): from autoskillit.core import SkillSource from autoskillit.workspace.skills import _skill_info_from_frontmatter diff --git a/tests/workspace/test_clone_push.py b/tests/workspace/test_clone_push.py index 64f3989b3d..eaa7f3d821 100644 --- a/tests/workspace/test_clone_push.py +++ b/tests/workspace/test_clone_push.py @@ -318,14 +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( + @pytest.fixture + def upstream_remote(self): + with patch( "autoskillit.workspace.clone._matching_push_remote", - lambda checkout, url: url, - ) + side_effect=lambda checkout, url: url, + ) as remote: + yield remote - def test_ds6_push_to_remote_calls_get_url_then_push(self) -> None: + def test_ds6_push_to_remote_calls_get_url_then_push(self, upstream_remote) -> None: """T_DS6: push_to_remote resolves origin, then pushes directly to that URL.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -344,6 +345,7 @@ def test_ds6_push_to_remote_calls_get_url_then_push(self) -> None: ) assert result == {"success": True, "stderr": ""} + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") # First call: git remote get-url origin from source_dir first_call = mock_run.call_args_list[0] assert first_call[0][0] == ["git", "remote", "get-url", "origin"] @@ -360,7 +362,29 @@ def test_ds6_push_to_remote_calls_get_url_then_push(self) -> None: ] assert second_call[1]["cwd"] == "/clone" - def test_ds7_push_to_remote_fails_when_no_origin(self) -> None: + @pytest.mark.parametrize( + "error", + [ + RuntimeError("destination lookup failed"), + OSError("destination lookup failed"), + subprocess.TimeoutExpired(["git", "remote", "-v"], 30), + ], + ) + def test_push_returns_failure_when_destination_lookup_fails( + self, upstream_remote, error: Exception + ) -> None: + upstream_remote.side_effect = error + with patch("autoskillit.workspace.clone.subprocess.run") as run: + result = push_to_remote( + "/clone", + remote_url="https://github.com/org/repo.git", + branch="topic", + protected_branches=[], + ) + assert result == {"success": False, "stderr": str(error)} + run.assert_not_called() + + def test_ds7_push_to_remote_fails_when_no_origin(self, upstream_remote) -> None: """T_DS7: push_to_remote returns error when git remote get-url fails, no push attempted.""" mock_fail = MagicMock() mock_fail.returncode = 128 @@ -375,8 +399,9 @@ def test_ds7_push_to_remote_fails_when_no_origin(self) -> None: assert result["success"] is False assert "origin" in result["stderr"] assert mock_run.call_count == 1 # no push attempted + upstream_remote.assert_not_called() - def test_push_to_remote_with_force_injects_force_with_lease(self) -> None: + def test_push_to_remote_with_force_injects_force_with_lease(self, upstream_remote) -> None: """T1: push_to_remote with force=True appends --force-with-lease to push command.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -402,7 +427,11 @@ def test_push_to_remote_with_force_injects_force_with_lease(self) -> None: assert "main" in cmd assert second_call[1]["cwd"] == "/clone" - def test_push_to_remote_default_force_false_does_not_inject_lease(self) -> None: + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") + + def test_push_to_remote_default_force_false_does_not_inject_lease( + self, upstream_remote + ) -> None: """T2: push_to_remote with force=False (default) does not inject --force-with-lease.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -424,6 +453,7 @@ def test_push_to_remote_default_force_false_does_not_inject_lease(self) -> None: second_call = mock_run.call_args_list[1] cmd = second_call[0][0] assert cmd == ["git", "push", "-u", "--", "git@github.com:org/repo.git", "main"] + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") 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.""" @@ -434,7 +464,7 @@ def test_default_clone_manager_push_to_remote_accepts_force_param(self) -> None: param = sig.parameters["force"] assert param.default is False, "force param must default to False" - def test_force_with_lease_stale_returns_error_type(self) -> None: + def test_force_with_lease_stale_returns_error_type(self, upstream_remote) -> None: """push_to_remote returns error_type=force_with_lease_stale when git reports stale info.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -452,8 +482,9 @@ def test_force_with_lease_stale_returns_error_type(self) -> None: assert result["success"] is False assert result.get("error_type") == "force_with_lease_stale" + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") - def test_force_with_lease_no_upstream_returns_error_type(self) -> None: + def test_force_with_lease_no_upstream_returns_error_type(self, upstream_remote) -> None: """push_to_remote returns error_type=force_with_lease_no_upstream for missing upstream.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -471,8 +502,9 @@ def test_force_with_lease_no_upstream_returns_error_type(self) -> None: assert result["success"] is False assert result.get("error_type") == "force_with_lease_no_upstream" + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") - def test_force_push_generic_failure_has_no_error_type(self) -> None: + def test_force_push_generic_failure_has_no_error_type(self, upstream_remote) -> None: """push_to_remote returns no error_type for generic force-push failures.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -491,7 +523,9 @@ def test_force_push_generic_failure_has_no_error_type(self) -> None: assert result["success"] is False assert "error_type" not in result - def test_non_force_failure_has_no_error_type(self) -> None: + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") + + def test_non_force_failure_has_no_error_type(self, upstream_remote) -> None: """push_to_remote returns no error_type for non-force push failures.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -512,7 +546,9 @@ def test_non_force_failure_has_no_error_type(self) -> None: assert result["success"] is False assert "error_type" not in result - def test_gh006_returns_queued_branch_error_type(self) -> None: + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") + + def test_gh006_returns_queued_branch_error_type(self, upstream_remote) -> None: """GH006 stderr produces error_type=queued_branch.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -534,7 +570,9 @@ def test_gh006_returns_queued_branch_error_type(self) -> None: assert result["success"] is False assert result.get("error_type") == "queued_branch" - def test_merge_queue_stderr_returns_queued_branch_error_type(self) -> None: + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") + + def test_merge_queue_stderr_returns_queued_branch_error_type(self, upstream_remote) -> None: """'protected by merge queue' stderr variant also produces queued_branch.""" mock_url = MagicMock() mock_url.returncode = 0 @@ -553,7 +591,9 @@ def test_merge_queue_stderr_returns_queued_branch_error_type(self) -> None: assert result["success"] is False assert result.get("error_type") == "queued_branch" - def test_gh006_non_force_also_returns_queued_branch(self) -> None: + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") + + def test_gh006_non_force_also_returns_queued_branch(self, upstream_remote) -> None: """GH006 fires for non-force pushes too (queue protects all push modes).""" mock_url = MagicMock() mock_url.returncode = 0 @@ -573,6 +613,7 @@ def test_gh006_non_force_also_returns_queued_branch(self) -> None: assert result["success"] is False assert result.get("error_type") == "queued_branch" + upstream_remote.assert_called_once_with(Path("/clone"), "git@github.com:org/repo.git") class TestPushToRemoteNonBare: