From 6dfb1e7e3a19004b233abe00702b57f80fd94753 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 27 Jul 2026 18:02:12 -0700 Subject: [PATCH 1/2] feat: add reasoning controls and harden retries and artifact capture --- ...0017-provider-native-reasoning-controls.md | 383 ++++++++ docs/getting-started/provider-setup.md | 28 + docs/guides/node-modes.md | 37 +- docs/guides/workflow-authoring.md | 6 + docs/reference/artifacts.md | 20 + docs/reference/configuration.md | 4 +- docs/reference/workflow-syntax.md | 14 +- src/crewplane/adapters/invokers/cli.py | 32 +- .../invokers/cli_invoker/capabilities.py | 45 +- .../invokers/cli_invoker/reasoning.py | 365 +++++++ .../architecture/contracts/invocation.py | 3 +- src/crewplane/architecture/ports/artifacts.py | 2 +- .../artifacts/generated_files/catalog.py | 244 +++-- .../generated_files/snapshot_policy.py | 171 ++++ src/crewplane/artifacts/manager.py | 2 +- .../artifacts/results/aggregation.py | 33 +- .../artifacts/results/stage_outputs.py | 1 + src/crewplane/artifacts/results/writer.py | 7 +- src/crewplane/cli/dry_run.py | 7 +- src/crewplane/cli/run/preflight.py | 42 +- src/crewplane/core/config.py | 2 + .../core/preflight/execution_nodes.py | 1 + src/crewplane/core/preflight/models.py | 1 + .../core/preflight/runtime_config/__init__.py | 2 + src/crewplane/core/workflow/models.py | 22 + src/crewplane/example_templates/config.yml | 10 + .../observability/events/execution_event.py | 2 + .../observability/log_presentation/tail.py | 5 + src/crewplane/observability/tmux/log_tail.py | 5 + .../runtime/agent/failures/__init__.py | 15 +- .../runtime/agent/failures/patterns.py | 5 +- src/crewplane/runtime/agent/failures/types.py | 1 + .../runtime/agent/invocation/loop.py | 35 +- .../runtime/agent/invocation/retry.py | 35 +- src/crewplane/runtime/agent/invoker.py | 22 +- .../runtime/execution/activity/events.py | 4 + .../provider_call/artifact_capture.py | 61 ++ .../runtime/execution/provider_call/events.py | 20 + .../provider_call/generated_files.py | 1 + .../execution/provider_call/lifecycle.py | 6 +- .../execution/workspace_files/generated.py | 15 +- .../integration/adapters/test_invoker_cli.py | 643 ++++++++++++- .../integration/architecture/static_checks.py | 319 +++++++ .../architecture/test_docs_drift.py | 55 ++ .../architecture/test_import_boundaries.py | 63 ++ .../test_private_api_boundaries.py | 41 + .../test_public_api_boundaries.py | 208 ++++ .../architecture/test_refactor_boundaries.py | 892 ------------------ .../test_runtime_ownership_boundaries.py | 121 +++ .../test_source_size_and_hygiene.py | 54 ++ tests/integration/cli/test_run_preflight.py | 105 +++ tests/integration/cli/test_workflow_runner.py | 49 + .../runtime/test_compact_runtime_log_tail.py | 20 + .../test_invocation_failure_reporting.py | 28 + .../agent/invoker/test_quota_parsing.py | 12 + .../runtime/agent/test_invocation_loop.py | 169 ++++ .../runtime/agent/test_invocation_retry.py | 98 ++ .../runtime/agent/test_invoker_facade.py | 45 + .../architecture/test_invocation_contracts.py | 10 + tests/unit/artifacts/test_output.py | 306 +++++- .../core/test_role_and_workspace_enums.py | 17 + .../log_presentation/test_formatters.py | 29 + .../test_generated_file_change_detection.py | 95 +- .../test_service_provider_invocation.py | 235 +++++ 64 files changed, 4268 insertions(+), 1062 deletions(-) create mode 100644 docs/architecture/adr/0017-provider-native-reasoning-controls.md create mode 100644 src/crewplane/adapters/invokers/cli_invoker/reasoning.py create mode 100644 src/crewplane/artifacts/generated_files/snapshot_policy.py create mode 100644 src/crewplane/runtime/execution/provider_call/artifact_capture.py create mode 100644 tests/integration/architecture/static_checks.py create mode 100644 tests/integration/architecture/test_docs_drift.py create mode 100644 tests/integration/architecture/test_import_boundaries.py create mode 100644 tests/integration/architecture/test_private_api_boundaries.py create mode 100644 tests/integration/architecture/test_public_api_boundaries.py delete mode 100644 tests/integration/architecture/test_refactor_boundaries.py create mode 100644 tests/integration/architecture/test_runtime_ownership_boundaries.py create mode 100644 tests/integration/architecture/test_source_size_and_hygiene.py diff --git a/docs/architecture/adr/0017-provider-native-reasoning-controls.md b/docs/architecture/adr/0017-provider-native-reasoning-controls.md new file mode 100644 index 0000000..1072785 --- /dev/null +++ b/docs/architecture/adr/0017-provider-native-reasoning-controls.md @@ -0,0 +1,383 @@ +# ADR 0017: Provider-Native Reasoning Controls + +## Status +Accepted and implemented + +## Date +2026-04-30 + +## Implementation Update +2026-07-22 + +## Decision +Introduce an optional `reasoning` field on workflow provider objects: + +```yaml +providers: + - provider: codex + role: executor + - provider: codex + model: gpt-5.6-sol + reasoning: ultra + role: reviewer +``` + +The field accepts a provider-native value: Crewplane passes it through unchanged +and the selected provider defines its meaning. Crewplane does not translate the +value into a shared effort scale. + +Crewplane translates `reasoning` into native command-line arguments only for +the built-in `cli` invoker when `provider_kind` is `codex` or `claude`. + +Model and reasoning resolution remain independent. A workflow may request +either, both, or neither. A reasoning request does not require an explicit +model, and Crewplane does not define an agent-level reasoning default. + +When `reasoning` is omitted or set to `null`, Crewplane emits no reasoning +argument. Provider configuration, raw CLI arguments, session state, and +provider defaults continue to determine behavior. This preserves the existing +invocation path and execution identity for workflows that do not opt in. + +## Context +Before this change, workflows could select a model but could not directly state +the desired reasoning effort. Users had to rely on ambient provider settings or +manually add provider-specific CLI arguments. As a result, an +execution-affecting request was not clearly represented in the workflow, +dry-run plan, run records, or duplicate-run identity. + +Providers do not share a common reasoning model. Values with similar names may +have different meanings, model compatibility, fallback behavior, or additional +provider-owned effects. Some CLIs expose a per-run option; others use stateful +configuration, model variants, numeric budgets, or no equivalent control. + +Crewplane already assigns provider-specific command construction to invoker +adapters and compiles runtime semantics during preflight. This decision extends +those existing boundaries instead of introducing a portable reasoning +abstraction or a separate profile system. + +## Goals + +- Allow workflows to request provider-native reasoning explicitly. +- Keep provider-specific arguments inside the built-in CLI adapter. +- Detect structural errors and direct configuration conflicts before launch. +- Include an explicit request in plans, run records, and execution identity. +- Preserve existing behavior when `reasoning` is absent. +- Record the exact reasoning value Crewplane sends to each provider. + +## Non-Goals + +This decision does not introduce: + +- named model or reasoning profiles +- a portable `low`/`medium`/`high` reasoning scale +- an agent-level default reasoning value +- first-class reasoning support for every provider kind or invoker +- a generic reasoning compiler or command-line template language +- CLI version probes or a model-and-effort compatibility catalog +- reasoning summaries, raw reasoning, or thinking-block visibility controls +- response verbosity, prompt keywords, or numeric thinking-token budgets +- control over nested provider-owned agents, routing, fallback, clamping, or + organization policy + +## Configuration and Semantic Contract + +`ProviderSpec` and its serialized workflow payload carry: + +```python +reasoning: str | None = None +``` + +A non-null value must be an ASCII token between 1 and 64 characters and match +`^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$`. Crewplane preserves the value exactly as +written. It does not normalize, translate, clamp, or replace it. + +The compact string form remains valid, but cannot include a reasoning request: + +```yaml +providers: [codex] +``` + +The selected invoker and configured `provider_kind` determine support. An alias +or executable name does not imply a capability. The current support matrix and +failure behavior are defined in +[Provider Support and Native Transport](#provider-support-and-native-transport). +Crewplane performs reasoning-specific eligibility checks only when the field is +present. + +### Omission Delegates Reasoning to the Provider + +When `reasoning` is omitted or set to `null`, Crewplane sends no reasoning +override. **The provider** then determines the reasoning behavior through its +normal resolution path, including raw CLI arguments, user configuration, +session state, defaults, and provider policy. + +In other words, ***omission delegates the choice to the provider***. It does not +request `none`, `off`, or any particular reasoning level. + +### Crewplane Records the Supplied Value + +Crewplane records the exact value it sends as `requested_reasoning` in plans, +events, and artifacts. This gives each run an auditable record of the requested +reasoning value without requiring provider-specific output parsing. + +## Architecture Boundaries + +The feature follows the existing preflight and ports-and-adapters boundaries. + +### Core + +Core is responsible for: + +- parsing and validating the optional workflow field +- carrying `requested_reasoning` through compiled provider records and runtime + metadata +- including an explicit request in execution identity +- rendering the request in dry-run output and execution records + +### Built-in CLI Adapter + +The built-in CLI adapter is responsible for: + +- declaring support through explicit `provider_kind` capabilities +- validating eligibility and direct conflicts without side effects +- compiling provider-native command arguments +- rejecting duplicate or ambiguous selectors in `cli_cmd`, `extra_args`, or + directly conflicting environment state +- preserving the existing command when no request is present + +### Runtime + +Runtime is responsible for: + +- passing the compiled request to the selected invoker without interpreting it +- executing the adapter-built argument vector without shell interpolation +- reporting provider failures without lowering, removing, or replacing the + requested value + +## Provider Support and Native Transport + +This snapshot records provider behavior verified on 2026-07-16. It explains why +Crewplane initially supports `reasoning` for **Codex CLI** and **Claude Code** only. +Provider products and accepted values may change after that date, so this table +is design context rather than a Crewplane-wide allowlist. + +| Provider | First-class `reasoning` support | Known native values or controls | Provider-native transport | +| --- | --- | --- | --- | +| Claude Code | Supported | Model-dependent `low`, `medium`, `high`, `xhigh`, and `max`; composite modes retain their provider-defined meaning | `--effort ` | +| Codex CLI | Supported | Documented `minimal` through `xhigh`; model-dependent `max`; `ultra` retains its provider-defined composite meaning | `--config model_reasoning_effort=...` | +| GitHub Copilot CLI | Not supported | `low`, `medium`, `high`, `xhigh`, `max`, and model-specific values | `--effort=` or `--reasoning-effort=` | +| Gemini CLI | Not supported | Model-specific `thinkingLevel` or `thinkingBudget` in stateful model configuration | No direct per-run thinking flag | +| Kilo CLI | Not supported | Provider- and model-specific variants | `--variant=` | +| Generic CLI | Not supported | Integration-specific raw configuration | Existing `cli_cmd` and `extra_args` | + +### Supported CLIs: Claude Code & Codex CLI + +Crewplane supports the workflow `reasoning` field when both of these conditions +are true: + +- the selected invoker is the built-in `cli` invoker +- `provider_kind` is `codex` or `claude` + +These are the only combinations that receive adapter-owned validation and the +native transport described below. + +#### Claude Code + +The adapter emits: + +```text +--effort + +``` + +When a workflow sets `reasoning`, the adapter rejects any competing Claude effort +configuration. Conflicting settings include `--effort` in `cli_cmd` or `extra_args`, +and a non-empty `CLAUDE_CODE_EFFORT_LEVEL` inherited from the environment or supplied +through `--settings`. + +`CLAUDE_CODE_EFFORT_LEVEL` is defined by Claude Code rather than Crewplane. Users do +not need to set it; this check applies only when the variable is already set. + +#### Codex CLI + +The adapter emits: + +```text +--config +model_reasoning_effort="" +``` + +When a workflow sets `reasoning`, the adapter rejects an existing +`model_reasoning_effort` setting in `cli_cmd` or `extra_args`. + +`model_reasoning_effort` is defined by Codex CLI rather than Crewplane. Users do not +need to set it; Crewplane generates it automatically from the workflow +`reasoning` value. + +Crewplane validates the request's structural safety and direct configuration +conflicts. The provider validates whether the native value is recognized and +compatible with the selected model. + +### CLIs Not Yet Supported + +Crewplane does not currently provide first-class `reasoning` support for +`copilot`, `gemini`, `kilo`, `generic`, or third-party invokers. Setting +`reasoning` for one of these combinations fails during validation or preflight. + +The unsupported built-in CLI provider kinds can still use `cli_cmd` and +`extra_args` for their native options. Crewplane passes those options through +as raw adapter configuration without treating them as a workflow `reasoning` +request or recording them as `requested_reasoning`. Third-party invokers remain +responsible for their own configuration contract. + +## Persistence + +An explicit `requested_reasoning` value appears in: + +- the compiled preflight execution plan +- `crewplane run --dry-run` +- invocation-start metadata and provider log headers +- the execution identity used by resume and duplicate-run detection + +When the field is absent, Crewplane omits it from persisted and rendered data +where possible. Existing workflows therefore retain their previous signatures +and record shapes. + +## Failure Behavior + +Invalid values, unsupported integrations, duplicate native selectors, and +direct Claude environment conflicts fail before the provider starts. Unknown +values and model incompatibilities may fail after launch because Crewplane does +not maintain a compatibility catalog. + +Existing retry policy remains authoritative. Every attempt uses the same +reasoning arguments. Crewplane never recovers by changing or removing the +request, selecting another model, or falling back to an unmanaged default. + +## Tradeoffs + +### Native fidelity over portability + +Passing values through unchanged preserves each provider's vocabulary and +allows providers to add values without requiring a Crewplane schema release. +The cost is that a workflow value is not portable across providers and may +carry semantics beyond a simple effort level. + +### Stable boundary validation over exhaustive compatibility checks + +Crewplane can deterministically validate value shape, adapter eligibility, and +direct configuration conflicts. It leaves model-and-value compatibility to the +provider. This avoids a fast-moving compatibility catalog, but moves some +failures from preflight to provider invocation. + +### Explicit workflow intent over centralized defaults + +Keeping reasoning on each workflow provider makes the request visible and keeps +precedence rules simple. Workflows may repeat the same value across several +provider entries, and ambient provider configuration still lacks the +provenance of an explicit request. + +### Narrow first-class support over speculative abstraction + +Supporting two concrete CLI transports keeps provider behavior behind the +adapter boundary without inventing an unproven extension model. Other CLIs must +continue to use raw configuration and receive neither first-class validation +nor `requested_reasoning` provenance. + +### Auditable inputs over provider-state introspection + +Crewplane records the exact value supplied to the provider instead of parsing +provider output to reconstruct internal state. This keeps records deterministic +and avoids coupling core orchestration to provider-specific output formats. + +### Backward-compatible omission over fully controlled execution + +Emitting no argument when the field is absent preserves existing commands, +signatures, and defaults. Such executions may still depend on ambient provider +configuration, so omission is less reproducible than an explicit request. + +## Consequences + +### Positive + +- Codex and Claude workflows gain one small, auditable configuration field. +- Provider-specific flags remain behind the built-in invoker boundary. +- Plans and run records state what Crewplane requested. +- Present reasoning requests participate in resume and duplicate-run identity. +- New provider-native tokens do not require a Crewplane schema change. +- Workflows that omit reasoning retain existing behavior. + +### Negative + +- Reasoning values are intentionally not portable. +- Invalid or model-incompatible values may fail only after provider launch. +- First-class support and provenance are limited to selected built-in CLI + provider kinds. +- Adapter conflict detection must evolve if native CLI transports change. + +## Rejected Alternatives + +### Named model and reasoning profiles + +A single optional field does not justify a profile catalog, another set of +precedence rules, or profile-specific execution identity. Workflows can pair +`model` and `reasoning` directly when they need both. + +### Portable Crewplane reasoning levels + +Providers and models do not assign consistent meaning or support to similarly +named levels. A Crewplane-owned mapping would create false portability and +would require Crewplane to define fallback or clamping behavior. + +### Agent-level default reasoning + +An agent-level default would add another precedence layer among workflows, +agent configuration, raw arguments, and provider configuration. Leaving the +field unset preserves existing agent and provider behavior. + +### First-class support for every CLI + +Crewplane has no dependable transport or semantic contract that spans every +provider. Other integrations can continue to use `cli_cmd`, `model_arg`, and +`extra_args` for provider-specific options. + +### Capability catalogs and CLI version probes + +Maintaining model-and-value compatibility across CLI versions would add a large, +fast-changing policy surface. Providers remain the authority on which values +their models support. + +### Treat omission as `none` or as a known provider default + +Raw arguments, persisted settings, aliases, session state, and organization +policy may still select reasoning behavior. Omission can describe only what +Crewplane adds to the invocation. + +### Infer reasoning state from provider output + +Provider output formats are not stable cross-provider contracts. Parsing them +to reconstruct internal reasoning state would couple core orchestration to +provider-specific presentation, while the workflow and run record already +capture the exact value Crewplane supplied. + +### Add a generic reasoning compiler or invoker extension contract + +Two concrete CLI transports do not establish a stable cross-invoker +abstraction. A generic contract should follow a demonstrated third-party +integration need. + +### Infer support from aliases, executable names, or arguments + +Names and command text are user-configurable, so they are not reliable +capability declarations. Support follows the selected invoker and the +adapter-owned `provider_kind`. + +## Related Decisions + +- [ADR 0001: Ports + Adapters Runtime Integrations](0001-ports-adapters-runtime-integrations.md) +- [ADR 0012: Preflight-Compiled Runtime Execution Plan](0012-preflight-compiled-runtime-execution-plan.md) + +## User Documentation + +- [Provider setup](../../getting-started/provider-setup.md) +- [Workflow syntax](../../reference/workflow-syntax.md) diff --git a/docs/getting-started/provider-setup.md b/docs/getting-started/provider-setup.md index 3b5aece..bafef77 100644 --- a/docs/getting-started/provider-setup.md +++ b/docs/getting-started/provider-setup.md @@ -197,6 +197,34 @@ provider CLI. Built-in provider kinds choose their own model flag. For `--model`. Set `model_arg: null` if your generic CLI should not receive a model flag. +## Choose Reasoning + +Provider objects can request a provider-native reasoning value when the +built-in `cli` invoker uses `provider_kind: codex` or `provider_kind: claude`: + +```yaml +providers: + - provider: codex + model: gpt-5.6-sol + reasoning: xhigh +``` + +Crewplane passes Codex requests through +`--config model_reasoning_effort="..."` and Claude requests through +`--effort ...`. The value is provider-native and may be model-dependent; +Crewplane records the request but does not claim that it was applied +unchanged. Omit `reasoning` to leave the provider's current defaults and user +configuration unmanaged. + +Do not configure a second reasoning authority in `cli_cmd` or `extra_args`. +For Claude, a non-empty inherited `CLAUDE_CODE_EFFORT_LEVEL` also conflicts. +Explicit `--settings` JSON or files may contain unrelated settings, but +`effortLevel` or `env.CLAUDE_CODE_EFFORT_LEVEL` conflicts with the workflow +field. When reasoning is requested, Crewplane must be able to read and parse +each explicit Claude settings source so `crewplane validate` can report +conflicts before launch. An `env` wrapper cannot use `--chdir` or `-C` with a +workflow reasoning request because it would change relative settings resolution. + ## Choose Prompt Transport Crewplane can send the rendered prompt to a provider CLI in two ways: diff --git a/docs/guides/node-modes.md b/docs/guides/node-modes.md index d000892..428ece8 100644 --- a/docs/guides/node-modes.md +++ b/docs/guides/node-modes.md @@ -5,8 +5,8 @@ nodes wait for upstream artifacts. This guide zooms in on **one node**. Inside a node, `mode` chooses the invocation shape, `providers` selects the configured agents, `model` can override an agent's default model for that -invocation, and `role` decides whether a provider executes the work or reviews -a candidate. +invocation, `reasoning` can request provider-native reasoning effort, and +`role` decides whether a provider executes the work or reviews a candidate. ## Choosing A Mode @@ -23,27 +23,52 @@ Independent DAG nodes can also run concurrently when their dependencies are satisfied. That is separate from `mode: parallel`, which is provider fanout inside one node. -## Provider Entries, Models, And Roles +## Provider Entries, Models, Reasoning, And Roles Every non-input node lists one or more configured agents in `providers`. A -string is shorthand for that agent's default model and executor role: +string uses that agent's default model, adds no Crewplane reasoning override, +and assigns the executor role: ```yaml providers: [codex] ``` -Use object form when one node invocation needs a model override or explicit -role: +### Use Provider Objects + +Use object form when one node invocation needs a model or reasoning override, +or an explicit role: ```yaml providers: - provider: codex model: gpt-5.5 + reasoning: high role: executor - provider: claude role: reviewer ``` +### Choose Reasoning Effort + +In the example above, `reasoning: high` asks Codex to use high reasoning effort +for that invocation. Claude has no workflow override and uses its normal +configuration. + +The value is provider-native: Codex and Claude define which values their models +support. Crewplane currently supports this field for the built-in `cli` invoker +when `provider_kind` is `codex` or `claude`. + +When `reasoning` is omitted or set to `null`, Crewplane sends no reasoning +override and the provider uses its normal configuration. Do not also configure +the provider's native reasoning option in `cli_cmd` or `extra_args`. + +See [Provider setup](../getting-started/provider-setup.md#choose-reasoning) for +the native Codex and Claude transports, and the +[workflow syntax reference](../reference/workflow-syntax.md#provider-objects) +for the complete validation contract. + +### Assign Provider Roles + The `role` field describes what a provider does inside this node: - `executor`: runs the node prompt and produces candidate output. This is the diff --git a/docs/guides/workflow-authoring.md b/docs/guides/workflow-authoring.md index babd891..f438b0b 100644 --- a/docs/guides/workflow-authoring.md +++ b/docs/guides/workflow-authoring.md @@ -137,9 +137,15 @@ providers: - claude - provider: codex model: gpt-5.5 + reasoning: high role: reviewer ``` +Provider objects can request a provider-native `reasoning` value for supported +CLIs. See +[Provider entries, models, reasoning, and roles](node-modes.md#provider-entries-models-reasoning-and-roles) +for usage guidance. + **Roles** are `executor` and `reviewer`. Parallel nodes only allow executor roles. Sequential review loops use executor providers followed by reviewer providers. Reviewers approve with `NO_FINDINGS` or `NITS_ONLY`; all reviewers must approve diff --git a/docs/reference/artifacts.md b/docs/reference/artifacts.md index adfb1de..b88346b 100644 --- a/docs/reference/artifacts.md +++ b/docs/reference/artifacts.md @@ -97,6 +97,26 @@ files are present for nodes that declare `findings: true`. Generated-file artifacts are present when Crewplane detects provider-created files that should be copied into the result tree. +Generated-file ownership depends on the workspace boundary: + +- In an isolated managed workspace, Crewplane owns the workspace for the + invocation and may capture filesystem changes. +- In the shared project root, Crewplane captures only files explicitly listed + by the provider under a `## Generated Files` section and verified by Git as + changed since the invocation began. Without a usable Git baseline, + generated-file capture fails closed. + +Snapshot capture limits and snapshot copy failures do not invalidate an +otherwise successful provider result. Crewplane preserves the result, captures +accepted files, emits an `artifact_capture_partial` or +`artifact_capture_failed` runtime-log event, and records rejected-file metadata +in the generated-file snapshot metadata. Rejection metadata keeps the exact +`rejected_file_count` while bounding detailed `rejected_files` records to 100; +`rejected_files_truncated` reports whether details were omitted. If later +publication into the consolidated result tree fails, Crewplane omits the failed +generated-file link, preserves the consolidated result, and emits a +`stage_finalize_warning` event. + Consolidated result and findings Markdown uses human-readable section headings. Stable provider task IDs remain in stage artifact filenames, logs, manifests, and review-loop state. diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 96bc1c2..8007b8e 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -108,7 +108,7 @@ settings use dotted paths in the left column. | `prompt_transport` | `stdin` or `argv`. Defaults to `stdin`. | | `prompt_transport_arg` | Required for `argv`; optional stdin sentinel for `stdin`. | | `extra_args` | Additional argv tokens appended to the provider command. | -| `max_retries` | Maximum retry attempts. Defaults to `0`. | +| `max_retries` | Maximum ordinary non-quota retry attempts. Defaults to `0`. | | `retry_delay_seconds` | Delay between generic retries. Defaults to `300.0`. | | `retry_on_exit_codes` | Exit codes that trigger retry. | | `retry_on_stderr_contains` | Stderr substrings that trigger retry. | @@ -116,6 +116,8 @@ settings use dotted paths in the left column. | `quota_reached_on_contains` | Output substrings treated as quota exhaustion. | | `quota_reached_retry_delay_seconds` | Delay after quota detection. Defaults to `300.0`. | | `quota_reset_sleep_floor_seconds` | Minimum sleep when a quota reset time is parsed. Defaults to `5.0`. | +| `quota_retry_max_wait_seconds` | Optional cumulative quota-wait ceiling for one invocation. Quota retries stop before the next sleep would reach this value. | +| `quota_retry_max_attempts` | Optional quota-retry attempt ceiling for one invocation. Quota retries are independent of `max_retries`. | | `invocation_timeout_seconds` | Optional wall-clock timeout. Defaults to `null`. | | `invocation_idle_timeout_seconds` | Optional idle-output timeout. Defaults to `1800.0`. | | `pricing` | Optional token pricing buckets. | diff --git a/docs/reference/workflow-syntax.md b/docs/reference/workflow-syntax.md index c97294a..505bb83 100644 --- a/docs/reference/workflow-syntax.md +++ b/docs/reference/workflow-syntax.md @@ -231,6 +231,7 @@ candidate; the round-0 review does not consume depth. providers: - provider: codex model: gpt-5.5 + reasoning: xhigh role: executor ``` @@ -240,6 +241,7 @@ Provider object fields: | --- | --- | | `provider` | Agent name from `.crewplane/config.yml`. Required. | | `model` | Optional model override for this provider use. | +| `reasoning` | Optional provider-native reasoning request for the built-in CLI invoker with `provider_kind: codex` or `claude`. | | `role` | `executor` or `reviewer`. Defaults to `executor`. | Roles are `executor` and `reviewer`. Parallel nodes do not allow reviewers. @@ -250,7 +252,17 @@ end with a contiguous reviewer segment. Use `review_starts_with`, not provider reordering, when reviewers should run before the first executor candidate. Provider shorthand strings are executor providers. Use provider objects when a -provider needs a `model` override or `role: reviewer`. +provider needs a `model` or `reasoning` override, or `role: reviewer`. + +`reasoning` is an opaque provider-native token, not a portable Crewplane +effort scale. Crewplane records it as `requested_reasoning` and includes it in +execution identity. When omitted or set to `null`, Crewplane supplies no +reasoning argument and the provider's existing configuration remains in +control. Direct reasoning selectors in `cli_cmd`, `extra_args`, or +`CLAUDE_CODE_EFFORT_LEVEL` conflict with a first-class request and fail +preflight. For Claude, that includes `effortLevel` and +`env.CLAUDE_CODE_EFFORT_LEVEL` in explicit `--settings` JSON or files. +Unrelated Claude settings remain valid. ## Input Nodes diff --git a/src/crewplane/adapters/invokers/cli.py b/src/crewplane/adapters/invokers/cli.py index 68fa71c..169b4b2 100644 --- a/src/crewplane/adapters/invokers/cli.py +++ b/src/crewplane/adapters/invokers/cli.py @@ -2,7 +2,7 @@ import os import shutil -from collections.abc import Callable +from collections.abc import Callable, Mapping from pathlib import Path from crewplane.architecture.contracts import ( @@ -17,6 +17,7 @@ from crewplane.runtime.agent.invoker import PlannedAgentInvoker from .cli_invoker import build_cli_invocation_plan, build_cli_log_presentation +from .cli_invoker.reasoning import validate_reasoning_request def collect_cli_availability_errors( @@ -49,6 +50,35 @@ def collect_cli_availability_errors( return _format_missing_cli_errors(config, missing_cli_locations) +def collect_cli_reasoning_errors( + workflow: WorkflowPlan, + config: Config, + environment: Mapping[str, str] | None = None, + working_directory: Path | None = None, +) -> list[str]: + errors: list[str] = [] + for node in workflow.nodes: + for provider in node.providers: + if provider.reasoning is None: + continue + agent_config = config.agents.get(provider.provider) + if agent_config is None: + continue + try: + validate_reasoning_request( + agent_config, + provider.reasoning, + environment, + working_directory, + ) + except ValueError as exc: + errors.append( + f"workflow '{workflow.name}' -> node '{node.id}' -> provider " + f"'{provider.provider}': {exc}" + ) + return errors + + def _format_missing_cli_errors( config: Config, missing_cli_locations: dict[str, list[str]], diff --git a/src/crewplane/adapters/invokers/cli_invoker/capabilities.py b/src/crewplane/adapters/invokers/cli_invoker/capabilities.py index 64948da..cef8673 100644 --- a/src/crewplane/adapters/invokers/cli_invoker/capabilities.py +++ b/src/crewplane/adapters/invokers/cli_invoker/capabilities.py @@ -8,6 +8,7 @@ from pathlib import Path from crewplane.architecture.contracts import ( + InvocationContext, InvocationPlan, LogPresentationDescriptor, LogPresentationFormat, @@ -19,6 +20,8 @@ ) from crewplane.core.config import AgentConfig +from .reasoning import build_reasoning_args + @dataclass(frozen=True) class CliProviderCapability: @@ -101,10 +104,25 @@ def build_cli_invocation_plan( model: str | None, prompt: str, output_file: Path, + invocation_context: InvocationContext | None = None, + working_directory: Path | None = None, ) -> InvocationPlan: capability = get_cli_provider_capability(config.provider_kind) + requested_reasoning = ( + invocation_context.requested_reasoning + if invocation_context is not None + else None + ) structured_output_file = _structured_output_file(capability) - cmd = _build_argv(config, capability, model, prompt, structured_output_file) + cmd = _build_argv( + config, + capability, + model, + prompt, + structured_output_file, + requested_reasoning, + working_directory, + ) stdin_data = prompt.encode("utf-8") if config.prompt_transport == "stdin" else None return InvocationPlan( cmd=cmd, @@ -120,6 +138,7 @@ def build_cli_invocation_plan( cli_executable=cmd[0], model=model, output_file=output_file, + requested_reasoning=requested_reasoning, ), ) @@ -138,14 +157,10 @@ def _build_argv( model: str | None, prompt: str, structured_output_file: Path | None, + requested_reasoning: str | None = None, + working_directory: Path | None = None, ) -> list[str]: - """Build the provider CLI argv and resolve the executable when available. - - The first argument identifies the process to launch, so it is resolved - before appending provider flags. This keeps log headers tied to the actual - executable when PATH resolution succeeds while leaving user-supplied - arguments untouched. - """ + """Build the provider CLI argv and resolve the executable when available.""" cmd = config.get_command() cmd[0] = _resolved_cli_executable(cmd[0]) model_arg = ( @@ -155,6 +170,13 @@ def _build_argv( ) if model_arg is not None and model is not None: cmd.extend([model_arg, model]) + cmd.extend( + build_reasoning_args( + config, + requested_reasoning, + working_directory, + ) + ) cmd.extend(config.extra_args) cmd.extend(_structured_output_args(capability, structured_output_file)) if config.prompt_transport == "stdin": @@ -232,13 +254,20 @@ def _build_log_header( cli_executable: str, model: str | None, output_file: Path, + requested_reasoning: str | None = None, ) -> bytes: started_at = datetime.now(UTC).isoformat() model_label = model if model is not None else "provider default" + reasoning_line = ( + f"requested_reasoning: {requested_reasoning}\n" + if requested_reasoning is not None + else "" + ) header = ( f"started_at: {started_at}\n" f"cli_executable: {cli_executable}\n" f"model: {model_label}\n" + f"{reasoning_line}" f"output_file: {output_file}\n" "---\n" ) diff --git a/src/crewplane/adapters/invokers/cli_invoker/reasoning.py b/src/crewplane/adapters/invokers/cli_invoker/reasoning.py new file mode 100644 index 0000000..5608e11 --- /dev/null +++ b/src/crewplane/adapters/invokers/cli_invoker/reasoning.py @@ -0,0 +1,365 @@ +from __future__ import annotations + +import json +import os +from collections.abc import Mapping, Sequence +from pathlib import Path + +from crewplane.architecture.contracts import ProviderKind +from crewplane.core.config import AgentConfig + +CODEX_REASONING_KEY = "model_reasoning_effort" +CLAUDE_REASONING_ENV = "CLAUDE_CODE_EFFORT_LEVEL" +SUPPORTED_REASONING_PROVIDER_KINDS = frozenset( + {ProviderKind.CODEX, ProviderKind.CLAUDE} +) + + +def validate_reasoning_request( + config: AgentConfig, + requested_reasoning: str | None, + environment: Mapping[str, str] | None = None, + working_directory: Path | None = None, +) -> None: + if requested_reasoning is None: + return + provider_kind = ProviderKind(config.provider_kind) + if provider_kind not in SUPPORTED_REASONING_PROVIDER_KINDS: + raise ValueError( + "First-class reasoning requires the built-in CLI invoker with " + "provider_kind 'codex' or 'claude'." + ) + effective_environment = os.environ if environment is None else environment + cli_arguments, effective_reasoning_environment = _cli_reasoning_context( + config.cli_cmd, + effective_environment.get(CLAUDE_REASONING_ENV, ""), + ) + _reject_cli_command_terminator(cli_arguments) + if provider_kind == ProviderKind.CODEX: + _reject_codex_reasoning_conflict(cli_arguments) + _reject_codex_reasoning_conflict(config.extra_args) + return + _reject_claude_reasoning_conflict(cli_arguments, working_directory) + _reject_claude_reasoning_conflict(config.extra_args, working_directory) + if effective_reasoning_environment.strip(): + raise ValueError( + f"{CLAUDE_REASONING_ENV} conflicts with the workflow reasoning request." + ) + + +def build_reasoning_args( + config: AgentConfig, + requested_reasoning: str | None, + working_directory: Path | None = None, +) -> tuple[str, ...]: + validate_reasoning_request( + config, + requested_reasoning, + working_directory=working_directory, + ) + if requested_reasoning is None: + return () + provider_kind = ProviderKind(config.provider_kind) + if provider_kind == ProviderKind.CODEX: + return ("--config", f'{CODEX_REASONING_KEY}="{requested_reasoning}"') + return ("--effort", requested_reasoning) + + +def _reject_cli_command_terminator(tokens: Sequence[str]) -> None: + if "--" in tokens: + raise ValueError( + "A first-class reasoning request cannot be appended after the " + "cli_cmd option terminator." + ) + + +def _reject_codex_reasoning_conflict(tokens: Sequence[str]) -> None: + for assignment in _codex_config_assignments(tokens): + key = assignment.partition("=")[0].strip() + if key == CODEX_REASONING_KEY: + raise ValueError( + f"{CODEX_REASONING_KEY} conflicts with the workflow reasoning request." + ) + + +def _codex_config_assignments(tokens: Sequence[str]) -> tuple[str, ...]: + assignments: list[str] = [] + index = 0 + while index < len(tokens): + token = tokens[index] + if token == "--": + break + assignment, consumed = _codex_config_assignment(tokens, index) + if assignment is not None: + assignments.append(assignment) + index += consumed + return tuple(assignments) + + +def _codex_config_assignment( + tokens: Sequence[str], + index: int, +) -> tuple[str | None, int]: + token = tokens[index] + if token in {"--config", "-c"}: + if index + 1 >= len(tokens) or tokens[index + 1] == "--": + raise ValueError(f"{token} requires a TOML assignment.") + return _require_toml_assignment(tokens[index + 1], token), 2 + if token.startswith("--config="): + return _require_toml_assignment(token.removeprefix("--config="), "--config"), 1 + if token.startswith("-c="): + return _require_toml_assignment(token.removeprefix("-c="), "-c"), 1 + if token.startswith("-c") and token != "-c": + return _require_toml_assignment(token[2:], "-c"), 1 + return None, 1 + + +def _require_toml_assignment(value: str, option: str) -> str: + if "=" not in value or not value.partition("=")[0].strip(): + raise ValueError(f"{option} requires a TOML key=value assignment.") + return value + + +def _reject_claude_reasoning_conflict( + tokens: Sequence[str], + working_directory: Path | None, +) -> None: + index = 0 + while index < len(tokens): + token = tokens[index] + if token == "--": + return + if token == "--effort": + if index + 1 >= len(tokens) or tokens[index + 1] == "--": + raise ValueError("--effort requires a value.") + raise ValueError("--effort conflicts with the workflow reasoning request.") + if token.startswith("--effort="): + raise ValueError("--effort conflicts with the workflow reasoning request.") + if token == "--settings": + if index + 1 >= len(tokens) or tokens[index + 1] == "--": + raise ValueError("--settings requires a JSON object or file path.") + _reject_claude_settings_reasoning_conflict( + tokens[index + 1], + working_directory, + ) + index += 2 + continue + if token.startswith("--settings="): + settings_value = token.removeprefix("--settings=") + if not settings_value: + raise ValueError("--settings requires a JSON object or file path.") + _reject_claude_settings_reasoning_conflict( + settings_value, + working_directory, + ) + index += 1 + + +def _reject_claude_settings_reasoning_conflict( + settings_value: str, + working_directory: Path | None, +) -> None: + settings = _load_claude_settings(settings_value, working_directory) + if _is_nonblank_settings_value(settings.get("effortLevel")): + raise ValueError( + "--settings effortLevel conflicts with the workflow reasoning request." + ) + settings_environment = settings.get("env") + if not isinstance(settings_environment, Mapping): + return + if _is_nonblank_settings_value(settings_environment.get(CLAUDE_REASONING_ENV)): + raise ValueError( + f"--settings {CLAUDE_REASONING_ENV} conflicts with the workflow " + "reasoning request." + ) + + +def _load_claude_settings( + settings_value: str, + working_directory: Path | None, +) -> Mapping[str, object]: + stripped_value = settings_value.lstrip() + if stripped_value.startswith("{"): + raw_settings = settings_value + else: + settings_path = Path(settings_value).expanduser() + if not settings_path.is_absolute(): + base_directory = ( + Path.cwd() if working_directory is None else working_directory + ) + settings_path = base_directory / settings_path + try: + raw_settings = settings_path.read_text(encoding="utf-8") + except (OSError, UnicodeError, ValueError) as exc: + raise ValueError( + "Cannot validate the Claude --settings file against the workflow " + "reasoning request." + ) from exc + try: + settings: object = json.loads(raw_settings) + except json.JSONDecodeError as exc: + raise ValueError( + "Claude --settings must contain a valid JSON object when workflow " + "reasoning is requested." + ) from exc + if not isinstance(settings, dict): + raise ValueError( + "Claude --settings must contain a JSON object when workflow reasoning " + "is requested." + ) + return settings + + +def _is_nonblank_settings_value(value: object) -> bool: + if value is None: + return False + if isinstance(value, str): + return bool(value.strip()) + return True + + +def _cli_reasoning_context( + tokens: Sequence[str], + inherited_value: str, +) -> tuple[tuple[str, ...], str]: + if not tokens: + return (), inherited_value + if Path(tokens[0]).name != "env": + return tuple(tokens[1:]), inherited_value + + effective_value = inherited_value + index = 1 + while index < len(tokens): + token = tokens[index] + if token == "--": + index += 1 + break + if token == "-": + effective_value = "" + index += 1 + continue + if token.startswith("--"): + index, effective_value = _consume_env_long_option( + tokens, + index, + effective_value, + ) + continue + if token.startswith("-"): + index, effective_value = _consume_env_short_options( + tokens, + index, + effective_value, + ) + continue + break + + while index < len(tokens): + key, separator, value = tokens[index].partition("=") + if not separator: + break + if key == CLAUDE_REASONING_ENV: + effective_value = value + index += 1 + command_arguments = tuple(tokens[index + 1 :]) if index < len(tokens) else () + return command_arguments, effective_value + + +def _consume_env_long_option( + tokens: Sequence[str], + index: int, + effective_value: str, +) -> tuple[int, str]: + token = tokens[index] + option, separator, inline_value = token.partition("=") + if option == "--ignore-environment" and not separator: + return index + 1, "" + if option == "--unset": + value, next_index = _env_option_value( + tokens, + index, + inline_value if separator else None, + option, + ) + if value == CLAUDE_REASONING_ENV: + effective_value = "" + return next_index, effective_value + if option == "--chdir": + raise ValueError( + "--chdir cannot be combined with a workflow reasoning request." + ) + if option == "--argv0": + _, next_index = _env_option_value( + tokens, + index, + inline_value if separator else None, + option, + ) + return next_index, effective_value + if option == "--split-string": + raise ValueError( + "--split-string cannot be combined with a workflow reasoning request." + ) + if option in { + "--block-signal", + "--debug", + "--default-signal", + "--ignore-signal", + "--list-signal-handling", + "--null", + }: + return index + 1, effective_value + raise ValueError( + f"Cannot validate env option {token!r} with a workflow reasoning request." + ) + + +def _consume_env_short_options( + tokens: Sequence[str], + index: int, + effective_value: str, +) -> tuple[int, str]: + cluster = tokens[index][1:] + option_index = 0 + while option_index < len(cluster): + option = cluster[option_index] + if option == "i": + effective_value = "" + option_index += 1 + continue + if option in {"0", "v"}: + option_index += 1 + continue + if option not in {"C", "P", "S", "a", "u"}: + raise ValueError( + f"Cannot validate env option '-{option}' " + "with a workflow reasoning request." + ) + inline_value = cluster[option_index + 1 :] or None + value, next_index = _env_option_value( + tokens, + index, + inline_value, + f"-{option}", + ) + if option in {"C", "S"}: + raise ValueError( + f"-{option} cannot be combined with a workflow reasoning request." + ) + if option == "u" and value == CLAUDE_REASONING_ENV: + effective_value = "" + return next_index, effective_value + return index + 1, effective_value + + +def _env_option_value( + tokens: Sequence[str], + index: int, + inline_value: str | None, + option: str, +) -> tuple[str, int]: + if inline_value is not None: + return inline_value, index + 1 + if index + 1 >= len(tokens): + raise ValueError(f"{option} requires a value.") + return tokens[index + 1], index + 2 diff --git a/src/crewplane/architecture/contracts/invocation.py b/src/crewplane/architecture/contracts/invocation.py index 0a5d4d6..cd6c40a 100644 --- a/src/crewplane/architecture/contracts/invocation.py +++ b/src/crewplane/architecture/contracts/invocation.py @@ -3,7 +3,7 @@ import re from collections.abc import Callable, Iterator, Mapping from contextlib import suppress -from dataclasses import dataclass +from dataclasses import KW_ONLY, dataclass from enum import StrEnum from pathlib import Path from types import MappingProxyType @@ -284,6 +284,7 @@ class InvocationContext: None ) workspace: InvocationWorkspaceContext | None = None + requested_reasoning: str | None = None @dataclass(frozen=True) diff --git a/src/crewplane/architecture/ports/artifacts.py b/src/crewplane/architecture/ports/artifacts.py index cc5ed25..217db30 100644 --- a/src/crewplane/architecture/ports/artifacts.py +++ b/src/crewplane/architecture/ports/artifacts.py @@ -57,7 +57,7 @@ def finalize_stage( findings_enabled: bool = False, task_specs: tuple[StageTaskSpec, ...] = (), generated_file_detection_enabled: bool = True, - generated_file_workspace_roots: dict[Path, Path] | None = None, + generated_file_workspace_roots: dict[Path, Path | None] | None = None, ) -> StageFinalizeResult: """Consolidate a stage's run artifacts into result artifacts.""" diff --git a/src/crewplane/artifacts/generated_files/catalog.py b/src/crewplane/artifacts/generated_files/catalog.py index c0ffabb..41c9bd2 100644 --- a/src/crewplane/artifacts/generated_files/catalog.py +++ b/src/crewplane/artifacts/generated_files/catalog.py @@ -4,7 +4,9 @@ import os import shutil import stat +import tempfile from collections.abc import Sequence +from contextlib import suppress from dataclasses import dataclass from hashlib import sha256 from pathlib import Path @@ -18,19 +20,35 @@ GeneratedFileReferenceDetector, is_reserved_workspace_path, ) +from .snapshot_policy import ( + GeneratedFileRejectionLog, + GeneratedFileSnapshotCandidate, + GeneratedFileSnapshotPolicy, + generated_file_rejection_metadata, + generated_file_snapshot_candidate_metadata, + select_generated_file_snapshot_candidates, +) MAX_GENERATED_FILE_SNAPSHOT_FILES = 100 MAX_GENERATED_FILE_SNAPSHOT_BYTES = 50 * 1024 * 1024 MAX_GENERATED_FILE_SNAPSHOT_TOTAL_BYTES = 200 * 1024 * 1024 +MAX_GENERATED_FILE_SNAPSHOT_REJECTION_DETAILS = 100 + + +@dataclass(frozen=True) +class GeneratedFileLinkResult: + links: tuple[GeneratedFileLink, ...] + warnings: tuple[str, ...] = () @dataclass(frozen=True) -class _GeneratedFileSnapshotCandidate: - source_path: Path - relative_path: Path - relative_label: str - size_bytes: int - changed: bool | None +class GeneratedFileSnapshotRejectionSummary: + total_count: int = 0 + recorded_files: tuple[dict[str, object], ...] = () + + @property + def truncated(self) -> bool: + return self.total_count > len(self.recorded_files) def build_generated_files_section( @@ -75,7 +93,7 @@ def generated_file_links_for_content( materialize: bool = False, copy_namespace: str | None = None, candidate_files: Sequence[Path] | None = None, -) -> tuple[GeneratedFileLink, ...]: +) -> GeneratedFileLinkResult: resolved_candidate_files = candidate_files if resolved_candidate_files is None: resolved_candidate_files = _generated_file_snapshot_candidate_files( @@ -86,6 +104,7 @@ def generated_file_links_for_content( source_root=_generated_file_snapshot_source_root(workspace_root), ) links: list[GeneratedFileLink] = [] + warnings: list[str] = [] for generated_file in _ordered_generated_files_for_content( content, detector, @@ -96,17 +115,24 @@ def generated_file_links_for_content( label = relative_path target_path = generated_file if materialize: - target_path = _copy_workspace_generated_file( - generated_file, - relative_path, - result_file, - stage_name, - copy_namespace, - ) if copy_namespace is not None: label = f"{copy_namespace}/{relative_path}" + try: + target_path = _copy_workspace_generated_file( + generated_file, + relative_path, + result_file, + stage_name, + copy_namespace, + ) + except OSError as exc: + warnings.append(f"Generated-file copy failed for {label!r}: {exc}") + continue links.append(GeneratedFileLink(label=label, target_path=target_path)) - return tuple(links) + return GeneratedFileLinkResult( + links=tuple(links), + warnings=tuple(warnings), + ) def snapshot_generated_file_workspace( @@ -114,35 +140,62 @@ def snapshot_generated_file_workspace( workspace_root: Path, changed_paths: set[str] | None = None, candidate_files: Sequence[Path] | None = None, + explicit_claims_only: bool = False, ) -> Path: content = output_file.read_text(encoding="utf-8") if output_file.is_file() else "" snapshot_root = generated_file_source_root(output_file) resolved_workspace_root = workspace_root.resolve(strict=True) detector = GeneratedFileReferenceDetector(resolved_workspace_root) + explicit_files = detector.detect_explicit_section(content) + explicit_labels = { + path.relative_to(resolved_workspace_root).as_posix() for path in explicit_files + } generated_files = _ordered_generated_files_for_content( content, detector, resolved_workspace_root, candidate_files, + explicit_claims_only, ) - candidates = _generated_file_snapshot_candidates( + selection = select_generated_file_snapshot_candidates( generated_files, - resolved_workspace_root, - changed_paths, + GeneratedFileSnapshotPolicy( + resolved_workspace_root=resolved_workspace_root, + changed_paths=changed_paths, + explicit_labels=explicit_labels, + baseline_supplied=candidate_files is not None, + file_count_limit=MAX_GENERATED_FILE_SNAPSHOT_FILES, + per_file_size_limit=MAX_GENERATED_FILE_SNAPSHOT_BYTES, + total_size_limit=MAX_GENERATED_FILE_SNAPSHOT_TOTAL_BYTES, + rejection_detail_limit=MAX_GENERATED_FILE_SNAPSHOT_REJECTION_DETAILS, + ), ) _replace_generated_file_source_root(snapshot_root) _write_generated_file_source_metadata(snapshot_root, resolved_workspace_root) - for candidate in candidates: + copied_candidates: list[GeneratedFileSnapshotCandidate] = [] + for candidate in selection.candidates: target = snapshot_root.joinpath(*candidate.relative_path.parts) _ensure_contained_directory(snapshot_root, candidate.relative_path.parent) - _copy_generated_file_snapshot_candidate(candidate, target) + try: + _copy_generated_file_snapshot_candidate(candidate, target) + except (OSError, RuntimeError) as exc: + selection.rejections.record( + generated_file_rejection_metadata( + candidate, + reason="copy_failed", + error=str(exc), + ) + ) + continue + copied_candidates.append(candidate) _write_generated_file_snapshot_metadata( snapshot_root, [ - _generated_file_snapshot_candidate_metadata(candidate) - for candidate in candidates + generated_file_snapshot_candidate_metadata(candidate) + for candidate in copied_candidates ], + selection.rejections, ) return snapshot_root @@ -152,17 +205,18 @@ def _ordered_generated_files_for_content( detector: GeneratedFileReferenceDetector, workspace_root: Path, candidate_files: Sequence[Path] | None, + explicit_claims_only: bool = False, ) -> tuple[Path, ...]: + explicit_paths = detector.detect_explicit_section(content) if candidate_files is None: - return detector.detect(content) + return () if explicit_claims_only else detector.detect(content) resolved_workspace_root = workspace_root.resolve(strict=True) candidates = _safe_unique_candidate_files(candidate_files, resolved_workspace_root) if not candidates: return () - explicit_paths = detector.detect_explicit_section(content) if not explicit_paths: - return tuple(candidates.values()) + return () if explicit_claims_only else tuple(candidates.values()) ordered: list[Path] = [] seen_labels: set[str] = set() @@ -173,9 +227,10 @@ def _ordered_generated_files_for_content( continue ordered.append(candidate) seen_labels.add(label) - ordered.extend( - path for label, path in candidates.items() if label not in seen_labels - ) + if not explicit_claims_only: + ordered.extend( + path for label, path in candidates.items() if label not in seen_labels + ) return tuple(ordered) @@ -206,67 +261,21 @@ def _safe_unique_candidate_files( return dict(sorted(candidates.items())) -def _generated_file_snapshot_candidates( - generated_files: Sequence[Path], - resolved_workspace_root: Path, - changed_paths: set[str] | None, -) -> tuple[_GeneratedFileSnapshotCandidate, ...]: - total_bytes = 0 - candidates: list[_GeneratedFileSnapshotCandidate] = [] - for generated_file in generated_files: - relative_path = generated_file.relative_to(resolved_workspace_root) - relative_label = relative_path.as_posix() - if changed_paths is not None and relative_label not in changed_paths: - continue - if len(candidates) >= MAX_GENERATED_FILE_SNAPSHOT_FILES: - raise RuntimeError("Generated-file snapshot rejected too many files.") - size_bytes = generated_file.stat().st_size - if size_bytes > MAX_GENERATED_FILE_SNAPSHOT_BYTES: - raise RuntimeError( - f"Generated-file snapshot rejected oversized file: {relative_label}" - ) - next_total_bytes = total_bytes + size_bytes - if next_total_bytes > MAX_GENERATED_FILE_SNAPSHOT_TOTAL_BYTES: - raise RuntimeError("Generated-file snapshot rejected oversized total.") - total_bytes = next_total_bytes - candidates.append( - _GeneratedFileSnapshotCandidate( - source_path=generated_file, - relative_path=relative_path, - relative_label=relative_label, - size_bytes=size_bytes, - changed=( - relative_label in changed_paths - if changed_paths is not None - else None - ), - ) - ) - return tuple(candidates) - - -def _generated_file_snapshot_candidate_metadata( - candidate: _GeneratedFileSnapshotCandidate, -) -> dict[str, object]: - return { - "path": candidate.relative_label, - "changed": candidate.changed, - "size_bytes": candidate.size_bytes, - } - - def _copy_generated_file_snapshot_candidate( - candidate: _GeneratedFileSnapshotCandidate, + candidate: GeneratedFileSnapshotCandidate, target: Path, ) -> None: - shutil.copyfile(candidate.source_path, target) - if target.stat().st_size == candidate.size_bytes: - return - target.unlink(missing_ok=True) - raise RuntimeError( - "Generated-file snapshot source changed while copying: " - f"{candidate.relative_label}" - ) + try: + shutil.copyfile(candidate.source_path, target) + if target.stat().st_size == candidate.size_bytes: + return + raise RuntimeError( + "Generated-file snapshot source changed while copying: " + f"{candidate.relative_label}" + ) + except (OSError, RuntimeError): + target.unlink(missing_ok=True) + raise def generated_file_source_root(output_file: Path) -> Path: @@ -323,6 +332,39 @@ def _generated_file_snapshot_candidate_files( return tuple(candidates) +def generated_file_snapshot_rejection_summary( + snapshot_root: Path, +) -> GeneratedFileSnapshotRejectionSummary: + metadata_file = contained_regular_file( + snapshot_root, + GENERATED_FILE_SNAPSHOT_METADATA_NAME, + ) + if metadata_file is None: + return GeneratedFileSnapshotRejectionSummary() + try: + payload = json.loads(metadata_file.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return GeneratedFileSnapshotRejectionSummary() + if not isinstance(payload, dict): + return GeneratedFileSnapshotRejectionSummary() + raw_rejections = payload.get("rejected_files") + if not isinstance(raw_rejections, list): + raw_rejections = [] + recorded_files = tuple(item for item in raw_rejections if isinstance(item, dict)) + raw_total_count = payload.get("rejected_file_count") + total_count = ( + raw_total_count + if isinstance(raw_total_count, int) + and not isinstance(raw_total_count, bool) + and raw_total_count >= len(recorded_files) + else len(recorded_files) + ) + return GeneratedFileSnapshotRejectionSummary( + total_count=total_count, + recorded_files=recorded_files, + ) + + def _write_generated_file_source_metadata( snapshot_root: Path, source_root: Path, @@ -337,10 +379,20 @@ def _write_generated_file_source_metadata( def _write_generated_file_snapshot_metadata( snapshot_root: Path, copied_files: Sequence[dict[str, object]], + rejections: GeneratedFileRejectionLog, ) -> None: + payload: dict[str, object] = {"files": list(copied_files)} + if rejections.rejected_file_count: + payload.update( + { + "rejected_file_count": rejections.rejected_file_count, + "rejected_files": list(rejections.rejected_files), + "rejected_files_truncated": rejections.truncated, + } + ) metadata_file = snapshot_root / GENERATED_FILE_SNAPSHOT_METADATA_NAME metadata_file.write_text( - json.dumps({"files": list(copied_files)}, sort_keys=True) + "\n", + json.dumps(payload, sort_keys=True) + "\n", encoding="utf-8", ) @@ -362,7 +414,23 @@ def _copy_workspace_generated_file( for part in Path(relative_path).parts: target = target / part target.parent.mkdir(parents=True, exist_ok=True) - shutil.copyfile(generated_file, target) + temporary_path: Path | None = None + try: + with tempfile.NamedTemporaryFile( + dir=target.parent, + prefix=".generated-file-", + suffix=".tmp", + delete=False, + ) as handle: + temporary_path = Path(handle.name) + shutil.copyfile(generated_file, temporary_path) + shutil.copymode(generated_file, temporary_path) + temporary_path.replace(target) + except OSError: + if temporary_path is not None: + with suppress(OSError): + temporary_path.unlink() + raise return target diff --git a/src/crewplane/artifacts/generated_files/snapshot_policy.py b/src/crewplane/artifacts/generated_files/snapshot_policy.py new file mode 100644 index 0000000..7cf07d0 --- /dev/null +++ b/src/crewplane/artifacts/generated_files/snapshot_policy.py @@ -0,0 +1,171 @@ +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass, field +from pathlib import Path + + +@dataclass(frozen=True) +class GeneratedFileSnapshotCandidate: + source_path: Path + relative_path: Path + relative_label: str + size_bytes: int + changed: bool | None + discovery_source: str + explicit: bool + + +@dataclass(frozen=True) +class GeneratedFileSnapshotPolicy: + resolved_workspace_root: Path + changed_paths: set[str] | None + explicit_labels: set[str] + baseline_supplied: bool + file_count_limit: int + per_file_size_limit: int + total_size_limit: int + rejection_detail_limit: int + + +@dataclass +class GeneratedFileRejectionLog: + detail_limit: int + rejected_files: list[dict[str, object]] = field(default_factory=list) + rejected_file_count: int = 0 + + def record(self, metadata: dict[str, object] | None = None) -> None: + self.rejected_file_count += 1 + if metadata is not None and len(self.rejected_files) < self.detail_limit: + self.rejected_files.append(metadata) + + @property + def details_full(self) -> bool: + return len(self.rejected_files) >= self.detail_limit + + @property + def truncated(self) -> bool: + return self.rejected_file_count > len(self.rejected_files) + + +@dataclass(frozen=True) +class GeneratedFileSnapshotSelection: + candidates: tuple[GeneratedFileSnapshotCandidate, ...] + rejections: GeneratedFileRejectionLog + + +def select_generated_file_snapshot_candidates( + generated_files: Sequence[Path], + policy: GeneratedFileSnapshotPolicy, +) -> GeneratedFileSnapshotSelection: + total_bytes = 0 + candidates: list[GeneratedFileSnapshotCandidate] = [] + rejections = GeneratedFileRejectionLog(policy.rejection_detail_limit) + for generated_file in generated_files: + relative_path = generated_file.relative_to(policy.resolved_workspace_root) + relative_label = relative_path.as_posix() + if ( + policy.changed_paths is not None + and relative_label not in policy.changed_paths + ): + continue + if len(candidates) >= policy.file_count_limit and rejections.details_full: + rejections.record() + continue + candidate = GeneratedFileSnapshotCandidate( + source_path=generated_file, + relative_path=relative_path, + relative_label=relative_label, + size_bytes=generated_file.stat().st_size, + changed=( + relative_label in policy.changed_paths + if policy.changed_paths is not None + else None + ), + discovery_source=generated_file_discovery_source( + relative_label, + policy.explicit_labels, + policy.baseline_supplied, + ), + explicit=relative_label in policy.explicit_labels, + ) + if len(candidates) >= policy.file_count_limit: + rejections.record( + generated_file_rejection_metadata( + candidate, + reason="file_count_limit", + configured_limit_count=policy.file_count_limit, + ) + ) + continue + if candidate.size_bytes > policy.per_file_size_limit: + rejections.record( + generated_file_rejection_metadata( + candidate, + reason="per_file_size_limit", + configured_limit_bytes=policy.per_file_size_limit, + ) + ) + continue + next_total_bytes = total_bytes + candidate.size_bytes + if next_total_bytes > policy.total_size_limit: + rejections.record( + generated_file_rejection_metadata( + candidate, + reason="total_size_limit", + configured_limit_bytes=policy.total_size_limit, + ) + ) + continue + total_bytes = next_total_bytes + candidates.append(candidate) + return GeneratedFileSnapshotSelection( + candidates=tuple(candidates), + rejections=rejections, + ) + + +def generated_file_snapshot_candidate_metadata( + candidate: GeneratedFileSnapshotCandidate, +) -> dict[str, object]: + return { + "path": candidate.relative_label, + "changed": candidate.changed, + "size_bytes": candidate.size_bytes, + } + + +def generated_file_rejection_metadata( + candidate: GeneratedFileSnapshotCandidate, + reason: str, + configured_limit_bytes: int | None = None, + configured_limit_count: int | None = None, + error: str | None = None, +) -> dict[str, object]: + metadata: dict[str, object] = { + "path": candidate.relative_label, + "size_bytes": candidate.size_bytes, + "discovery_source": candidate.discovery_source, + "explicit": candidate.explicit, + "disposition": "rejected", + "reason": reason, + } + if configured_limit_bytes is not None: + metadata["configured_limit_bytes"] = configured_limit_bytes + if configured_limit_count is not None: + metadata["configured_limit_count"] = configured_limit_count + if error: + metadata["error"] = error + return metadata + + +def generated_file_discovery_source( + relative_label: str, + explicit_labels: set[str], + baseline_supplied: bool, +) -> str: + if relative_label in explicit_labels: + return "provider_explicit_section" + if baseline_supplied: + return "workspace_change_baseline" + return "provider_claim" diff --git a/src/crewplane/artifacts/manager.py b/src/crewplane/artifacts/manager.py index 8964edb..52374ad 100644 --- a/src/crewplane/artifacts/manager.py +++ b/src/crewplane/artifacts/manager.py @@ -92,7 +92,7 @@ def finalize_stage( findings_enabled: bool = False, task_specs: tuple[StageTaskSpec, ...] = (), generated_file_detection_enabled: bool = True, - generated_file_workspace_roots: dict[Path, Path] | None = None, + generated_file_workspace_roots: dict[Path, Path | None] | None = None, ) -> StageFinalizeResult: return self._result_writer.finalize_stage( stage_name, diff --git a/src/crewplane/artifacts/results/aggregation.py b/src/crewplane/artifacts/results/aggregation.py index 928a81e..3ff5217 100644 --- a/src/crewplane/artifacts/results/aggregation.py +++ b/src/crewplane/artifacts/results/aggregation.py @@ -24,7 +24,7 @@ def aggregate_stage_outputs( findings_enabled: bool, result_file: Path, stage_name: str, - generated_file_workspace_roots: dict[Path, Path], + generated_file_workspace_roots: dict[Path, Path | None], generated_file_detection_enabled: bool, ) -> StageOutputAggregation: aggregation = StageOutputAggregation() @@ -57,7 +57,7 @@ def add_output_to_aggregation( findings_selection: FindingsSelection, result_file: Path, stage_name: str, - generated_file_workspace_roots: dict[Path, Path], + generated_file_workspace_roots: dict[Path, Path | None], generated_file_detection_enabled: bool, ) -> None: raw_output = output_file.read_text(encoding="utf-8") @@ -113,24 +113,25 @@ def record_generated_file_links( output_file: Path, result_file: Path, stage_name: str, - generated_file_workspace_roots: dict[Path, Path], + generated_file_workspace_roots: dict[Path, Path | None], detection_enabled: bool, ) -> None: if not detection_enabled: return - workspace_root = generated_file_workspace_roots.get( - output_file.resolve(strict=False) - ) - if workspace_root is None: + resolved_output_file = output_file.resolve(strict=False) + if resolved_output_file not in generated_file_workspace_roots: aggregation.generated_file_reference_content.append(display_content) return - aggregation.generated_file_links.extend( - generated_file_links_for_content( - display_content, - workspace_root, - result_file, - stage_name, - materialize=True, - copy_namespace=task_id, - ) + workspace_root = generated_file_workspace_roots[resolved_output_file] + if workspace_root is None: + return + link_result = generated_file_links_for_content( + display_content, + workspace_root, + result_file, + stage_name, + materialize=True, + copy_namespace=task_id, ) + aggregation.generated_file_links.extend(link_result.links) + aggregation.warnings.extend(link_result.warnings) diff --git a/src/crewplane/artifacts/results/stage_outputs.py b/src/crewplane/artifacts/results/stage_outputs.py index 2c7dac9..3c6f165 100644 --- a/src/crewplane/artifacts/results/stage_outputs.py +++ b/src/crewplane/artifacts/results/stage_outputs.py @@ -14,3 +14,4 @@ class StageOutputAggregation: findings_sections: list[tuple[str, str]] = field(default_factory=list) generated_file_reference_content: list[str] = field(default_factory=list) generated_file_links: list[GeneratedFileLink] = field(default_factory=list) + warnings: list[str] = field(default_factory=list) diff --git a/src/crewplane/artifacts/results/writer.py b/src/crewplane/artifacts/results/writer.py index 091ff34..005a524 100644 --- a/src/crewplane/artifacts/results/writer.py +++ b/src/crewplane/artifacts/results/writer.py @@ -48,7 +48,7 @@ def finalize_stage( findings_enabled: bool = False, task_specs: tuple[StageTaskSpec, ...] = (), generated_file_detection_enabled: bool = True, - generated_file_workspace_roots: dict[Path, Path] | None = None, + generated_file_workspace_roots: dict[Path, Path | None] | None = None, ) -> StageFinalizeResult: result_file = self._result_file_resolver(stage_name) findings_file = ( @@ -96,7 +96,10 @@ def finalize_stage( findings_file=resolved_findings_file, included_outputs=tuple(aggregation.included_outputs), skipped_empty_outputs=tuple(aggregation.skipped_empty_outputs), - warnings=tuple(self._warnings_for_skipped_outputs(aggregation)), + warnings=( + *self._warnings_for_skipped_outputs(aggregation), + *aggregation.warnings, + ), generated_files=tuple( dict.fromkeys( link.target_path for link in aggregation.generated_file_links diff --git a/src/crewplane/cli/dry_run.py b/src/crewplane/cli/dry_run.py index 4c8206a..536c064 100644 --- a/src/crewplane/cli/dry_run.py +++ b/src/crewplane/cli/dry_run.py @@ -68,7 +68,12 @@ def preview_topological_waves( def _format_provider_dry_run_line(provider: ProviderRecord) -> str: model = provider.model or "provider default" role_suffix = f" [{provider.role}]" if provider.role else "" - return f" - {provider.provider}{role_suffix} ({model})" + reasoning_suffix = ( + f", requested reasoning: {provider.requested_reasoning}" + if provider.requested_reasoning is not None + else "" + ) + return f" - {provider.provider}{role_suffix} ({model}{reasoning_suffix})" def _print_workspace_summary( diff --git a/src/crewplane/cli/run/preflight.py b/src/crewplane/cli/run/preflight.py index 02a0179..14c18cb 100644 --- a/src/crewplane/cli/run/preflight.py +++ b/src/crewplane/cli/run/preflight.py @@ -7,7 +7,10 @@ import typer from rich.console import Console -from crewplane.adapters.invokers.cli import collect_cli_availability_errors +from crewplane.adapters.invokers.cli import ( + collect_cli_availability_errors, + collect_cli_reasoning_errors, +) from crewplane.architecture.errors import IntegrationResolutionError from crewplane.architecture.loader import ( instantiate_adapter, @@ -256,7 +259,14 @@ def compile_preview( state_dir=context.state_dir, allowed_template_paths=allowed_template_paths(snapshot_result), fingerprint_key_policy=fingerprint_key_policy, - additional_validation_errors=additional_validation_errors, + additional_validation_errors=( + run_reasoning_control_errors( + context.source.workflow, + context.config, + context.project_root, + ) + + additional_validation_errors + ), additional_diagnostics=workspace_preflight_diagnostics(workspace_check), workspace_source_snapshot=workspace_check.source_snapshot, ), @@ -281,6 +291,34 @@ def run_cli_availability_errors( ) +def run_reasoning_control_errors( + workflow: WorkflowPlan, + config: Config, + working_directory: Path | None = None, +) -> tuple[str, ...]: + requested_locations = [ + f"workflow '{workflow.name}' -> node '{node.id}' -> provider " + f"'{provider.provider}'" + for node in workflow.nodes + for provider in node.providers + if provider.reasoning is not None + ] + if not requested_locations: + return () + if not uses_cli_invoker(config): + return tuple( + f"{location}: first-class reasoning requires the built-in CLI invoker." + for location in requested_locations + ) + return tuple( + collect_cli_reasoning_errors( + workflow, + config, + working_directory=working_directory, + ) + ) + + def write_preflight_diagnostics( output: ArtifactStorePort, diagnostics: list[PreflightDiagnostic], diff --git a/src/crewplane/core/config.py b/src/crewplane/core/config.py index 1b96795..b535e82 100644 --- a/src/crewplane/core/config.py +++ b/src/crewplane/core/config.py @@ -125,6 +125,8 @@ class AgentConfig(BaseModel): quota_reached_on_contains: list[str] = Field(default_factory=list) quota_reached_retry_delay_seconds: float = Field(default=300.0, ge=0) quota_reset_sleep_floor_seconds: float = Field(default=5.0, ge=0) + quota_retry_max_wait_seconds: float | None = Field(default=None, gt=0) + quota_retry_max_attempts: int | None = Field(default=None, ge=1) # A finite wall-clock timeout cancels and reaps the provider process. invocation_timeout_seconds: float | None = DEFAULT_INVOCATION_TIMEOUT_SECONDS invocation_idle_timeout_seconds: float | None = ( diff --git a/src/crewplane/core/preflight/execution_nodes.py b/src/crewplane/core/preflight/execution_nodes.py index 63e2779..27417ed 100644 --- a/src/crewplane/core/preflight/execution_nodes.py +++ b/src/crewplane/core/preflight/execution_nodes.py @@ -128,6 +128,7 @@ def provider_records( role=provider.role, model=provider.model or (agent_config.default_model if agent_config is not None else None), + requested_reasoning=provider.reasoning, task_id=artifact_task_id(provider, role_index), agent_config_key=provider.provider, invoker_alias=runtime_snapshot.invoker.implementation, diff --git a/src/crewplane/core/preflight/models.py b/src/crewplane/core/preflight/models.py index b809e73..a73cf06 100644 --- a/src/crewplane/core/preflight/models.py +++ b/src/crewplane/core/preflight/models.py @@ -232,6 +232,7 @@ class ProviderRecord(BaseModel): provider: str role: ProviderRole model: str | None = None + requested_reasoning: str | None = None task_id: str agent_config_key: str invoker_alias: str diff --git a/src/crewplane/core/preflight/runtime_config/__init__.py b/src/crewplane/core/preflight/runtime_config/__init__.py index ace21e0..69fb9bd 100644 --- a/src/crewplane/core/preflight/runtime_config/__init__.py +++ b/src/crewplane/core/preflight/runtime_config/__init__.py @@ -90,6 +90,8 @@ class RuntimeAgentConfigSnapshot(BaseModel): quota_reached_on_contains: list[str] = Field(default_factory=list) quota_reached_retry_delay_seconds: float = 300.0 quota_reset_sleep_floor_seconds: float = 5.0 + quota_retry_max_wait_seconds: float | None = Field(default=None, gt=0) + quota_retry_max_attempts: int | None = Field(default=None, ge=1) invocation_timeout_seconds: float | None = None invocation_idle_timeout_seconds: float | None = 1800.0 pricing: TokenPricing = Field(default_factory=TokenPricing) diff --git a/src/crewplane/core/workflow/models.py b/src/crewplane/core/workflow/models.py index c1b1c96..e2d89f4 100644 --- a/src/crewplane/core/workflow/models.py +++ b/src/crewplane/core/workflow/models.py @@ -1,5 +1,6 @@ from __future__ import annotations +import re from collections.abc import Callable from dataclasses import dataclass from typing import NotRequired, TypedDict @@ -41,12 +42,15 @@ ) from .syntax import INPUT_SOURCE_PATTERN +REASONING_TOKEN_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$") + class ProviderSpec(BaseModel): model_config = ConfigDict(extra="forbid") provider: str model: str | None = None + reasoning: str | None = None role: ProviderRole = ProviderRole.EXECUTOR @field_validator("provider", mode="before") @@ -54,6 +58,21 @@ class ProviderSpec(BaseModel): def _validate_provider(cls, value: object) -> object: return normalize_provider_name(value, "provider") + @field_validator("reasoning", mode="before") + @classmethod + def _validate_reasoning(cls, value: object) -> object: + if value is None: + return value + if ( + not isinstance(value, str) + or REASONING_TOKEN_PATTERN.fullmatch(value) is None + ): + raise ValueError( + "reasoning must be a 1-64 character ASCII token containing only " + "letters, digits, underscores, or hyphens" + ) + return value + @field_validator("role", mode="before") @classmethod def _validate_role(cls, value: object) -> object: @@ -68,6 +87,7 @@ def _validate_role(cls, value: object) -> object: class WorkflowProviderPayload(TypedDict): provider: str model: NotRequired[str] + reasoning: NotRequired[str] role: NotRequired[ProviderRole] @@ -207,6 +227,8 @@ def workflow_provider_payload_dict(provider: ProviderSpec) -> WorkflowProviderPa provider_payload: WorkflowProviderPayload = {"provider": provider.provider} if provider.model is not None: provider_payload["model"] = provider.model + if provider.reasoning is not None: + provider_payload["reasoning"] = provider.reasoning if provider.role != ProviderRole.EXECUTOR: provider_payload["role"] = provider.role return provider_payload diff --git a/src/crewplane/example_templates/config.yml b/src/crewplane/example_templates/config.yml index 1968fcb..fbadd1c 100644 --- a/src/crewplane/example_templates/config.yml +++ b/src/crewplane/example_templates/config.yml @@ -45,6 +45,8 @@ agents: # - "quota reached" # - "too many requests" # quota_reached_retry_delay_seconds: 300 + # quota_retry_max_wait_seconds: null + # quota_retry_max_attempts: null # # codex: # cli_cmd: ["codex", "exec"] @@ -70,6 +72,8 @@ agents: # - "rate limit reached" # - "too many requests" # quota_reached_retry_delay_seconds: 300 + # quota_retry_max_wait_seconds: null + # quota_retry_max_attempts: null # # gemini: # cli_cmd: ["gemini"] @@ -92,6 +96,8 @@ agents: # - "resource exhausted" # - "quota will reset after" # quota_reached_retry_delay_seconds: 300 + # quota_retry_max_wait_seconds: null + # quota_retry_max_attempts: null # # copilot: # cli_cmd: ["copilot"] @@ -118,6 +124,8 @@ agents: # - "quota reached" # - "too many requests" # quota_reached_retry_delay_seconds: 300 + # quota_retry_max_wait_seconds: null + # quota_retry_max_attempts: null # # kilo: # cli_cmd: ["kilo"] @@ -136,6 +144,8 @@ agents: # - "quota reached" # - "too many requests" # quota_reached_retry_delay_seconds: 300 + # quota_retry_max_wait_seconds: null + # quota_retry_max_attempts: null settings: # Experimental workspace isolation feature is disabled by default. With enabled: diff --git a/src/crewplane/observability/events/execution_event.py b/src/crewplane/observability/events/execution_event.py index 6e6636d..627a895 100644 --- a/src/crewplane/observability/events/execution_event.py +++ b/src/crewplane/observability/events/execution_event.py @@ -22,6 +22,7 @@ class ExecutionEventContext: provider: str | None = None role: str | None = None model: str | None = None + requested_reasoning: str | None = None task_id: str | None = None audit_round_num: int | None = None round_num: int | None = None @@ -36,6 +37,7 @@ def as_event_fields(self) -> dict[str, object]: "provider": self.provider, "role": self.role, "model": self.model, + "requested_reasoning": self.requested_reasoning, "task_id": self.task_id, "audit_round_num": self.audit_round_num, "round_num": self.round_num, diff --git a/src/crewplane/observability/log_presentation/tail.py b/src/crewplane/observability/log_presentation/tail.py index 1d031ef..9d884c0 100644 --- a/src/crewplane/observability/log_presentation/tail.py +++ b/src/crewplane/observability/log_presentation/tail.py @@ -13,6 +13,7 @@ "model:", "output_file:", ) +_OPTIONAL_REASONING_PREFIX = "requested_reasoning:" _RETRY_MARKER = b"\n---\nretry_attempt:" @@ -136,6 +137,10 @@ def find_initial_body_start( else: return 0 + if len(header_lines) == len(_INITIAL_HEADER_PREFIXES) + 1 and header_lines[ + 3 + ].startswith(_OPTIONAL_REASONING_PREFIX): + header_lines.pop(3) if len(header_lines) != len(_INITIAL_HEADER_PREFIXES): return 0 for line, prefix in zip(header_lines, _INITIAL_HEADER_PREFIXES, strict=True): diff --git a/src/crewplane/observability/tmux/log_tail.py b/src/crewplane/observability/tmux/log_tail.py index d7fa3cf..16e421e 100644 --- a/src/crewplane/observability/tmux/log_tail.py +++ b/src/crewplane/observability/tmux/log_tail.py @@ -10,6 +10,7 @@ "model:", "output_file:", ) +_OPTIONAL_REASONING_PREFIX = "requested_reasoning:" @dataclass(frozen=True) @@ -155,6 +156,10 @@ def _find_log_body_start(log_path: Path, file_size: int) -> int: else: return 0 + if len(header_lines) == len(_LOG_HEADER_PREFIXES) + 1 and header_lines[ + 3 + ].startswith(_OPTIONAL_REASONING_PREFIX): + header_lines.pop(3) if len(header_lines) != len(_LOG_HEADER_PREFIXES): return 0 if any( diff --git a/src/crewplane/runtime/agent/failures/__init__.py b/src/crewplane/runtime/agent/failures/__init__.py index 7afe261..db3e535 100644 --- a/src/crewplane/runtime/agent/failures/__init__.py +++ b/src/crewplane/runtime/agent/failures/__init__.py @@ -66,6 +66,7 @@ def build_quota_failure_error( provider_kind: ProviderKind, result: CommandResult, log_file: Path | None, + last_non_quota_failure: InvocationFailureSummary | None = None, ) -> InvocationFailureError: summary = classify_invocation_failure(provider_kind, result) if summary.kind != "quota_or_rate_limit": @@ -77,4 +78,16 @@ def build_quota_failure_error( advice=ADVICE_BY_KIND["quota_or_rate_limit"], condensed=summary.condensed, ) - return InvocationFailureError(prefix, summary, log_file) + if last_non_quota_failure is not None: + prefix = ( + f"{prefix}; last distinct non-quota failure: " + f"{last_non_quota_failure.message}" + ) + error = InvocationFailureError(prefix, summary, log_file) + error.last_non_quota_failure = last_non_quota_failure + if last_non_quota_failure is not None: + error.add_note( + "Last distinct non-quota failure before quota retries: " + f"{last_non_quota_failure.message}" + ) + return error diff --git a/src/crewplane/runtime/agent/failures/patterns.py b/src/crewplane/runtime/agent/failures/patterns.py index 8aa2f91..beb8ac4 100644 --- a/src/crewplane/runtime/agent/failures/patterns.py +++ b/src/crewplane/runtime/agent/failures/patterns.py @@ -63,8 +63,8 @@ "answer, or use a model/profile with a larger output budget." ), "quota_or_rate_limit": ( - "The provider reported quota or rate limiting. Check retry/reset details " - "and provider account limits." + "The provider reported quota or rate limiting. Quota retries are independent " + "of max_retries; check retry/reset details and provider account limits." ), "auth_or_permission": ( "The provider CLI is unauthenticated or lacks required tool, file, or " @@ -108,6 +108,7 @@ "unknown option", "unrecognized option", "invalid argument", + "reasoning.effort", ) INITIAL_REQUEST_TOO_LARGE_PATTERNS: tuple[str, ...] = ( "prompt is too long", diff --git a/src/crewplane/runtime/agent/failures/types.py b/src/crewplane/runtime/agent/failures/types.py index d715f96..338bc38 100644 --- a/src/crewplane/runtime/agent/failures/types.py +++ b/src/crewplane/runtime/agent/failures/types.py @@ -63,6 +63,7 @@ def __init__( self.source = summary.source self.advice = summary.advice self.log_file = log_file + self.last_non_quota_failure: InvocationFailureSummary | None = None super().__init__(f"{prefix}: {summary.format_for_error(log_file)}") diff --git a/src/crewplane/runtime/agent/invocation/loop.py b/src/crewplane/runtime/agent/invocation/loop.py index d1c26fa..d4093e9 100644 --- a/src/crewplane/runtime/agent/invocation/loop.py +++ b/src/crewplane/runtime/agent/invocation/loop.py @@ -17,7 +17,9 @@ build_invocation_failure_error, build_output_extraction_failure_error, build_quota_failure_error, + classify_invocation_failure, ) +from ..failures.types import InvocationFailureSummary from ..usage import InvocationUsageAccumulator from .command import ( build_invocation_runtime, @@ -86,9 +88,11 @@ async def run_invocation_loop( quota_retry_count=0, quota_retry_started_at=None, ) + quota_retry_wait_seconds = 0.0 usage_state = InvocationUsageState( accumulator=InvocationUsageAccumulator(plan.log_provider_kind, prompt) ) + last_non_quota_failure: InvocationFailureSummary | None = None try: while True: @@ -115,7 +119,15 @@ async def run_invocation_loop( runtime=runtime, attempt_result=attempt_result, cursor=cursor, + quota_retry_wait_seconds=quota_retry_wait_seconds, ) + if _is_non_quota_retry(transition): + failure_summary = classify_invocation_failure( + runtime.failure_profile, + attempt_result.result, + ) + if failure_summary.kind != "quota_or_rate_limit": + last_non_quota_failure = failure_summary continuation = await _execute_transition_action( transition=transition, runtime=runtime, @@ -126,12 +138,19 @@ async def run_invocation_loop( config=config, usage_state=usage_state, attempt=attempt, + last_non_quota_failure=last_non_quota_failure, ) finally: result.cleanup_stream_files() if continuation is None: return - attempt, cursor = continuation + next_attempt, next_cursor = continuation + if ( + isinstance(transition, SleepAndRetryAttemptTransition) + and next_cursor.quota_retry_count > cursor.quota_retry_count + ): + quota_retry_wait_seconds += transition.retry_delay_seconds + attempt, cursor = next_attempt, next_cursor except asyncio.CancelledError: raise except Exception: @@ -141,11 +160,20 @@ async def run_invocation_loop( cleanup_structured_output_file(runtime.structured_output_file) +def _is_non_quota_retry(transition: InvocationAttemptTransition) -> bool: + return ( + isinstance(transition, SleepAndRetryAttemptTransition) + and transition.notice is not None + and transition.notice.operation != "quota_retry_scheduled" + ) + + def _select_attempt_transition( config: AgentConfig, runtime: InvocationCommandRuntime, attempt_result: InvocationAttemptResult, cursor: InvocationRetryCursor, + quota_retry_wait_seconds: float, ) -> InvocationAttemptTransition: structured_retry_decision = _evaluate_structured_retry( config, @@ -169,6 +197,7 @@ def _select_attempt_transition( result=attempt_result.result, quota_retry_started_at=cursor.quota_retry_started_at, quota_retry_count=cursor.quota_retry_count, + quota_retry_wait_seconds=quota_retry_wait_seconds, ) transition = transition_from_quota_retry( attempt_result=attempt_result, @@ -243,6 +272,7 @@ async def _execute_transition_action( config: AgentConfig, usage_state: InvocationUsageState, attempt: int, + last_non_quota_failure: InvocationFailureSummary | None = None, ) -> tuple[int, InvocationRetryCursor] | None: try: match transition: @@ -288,6 +318,7 @@ async def _execute_transition_action( runtime=runtime, result=attempt_result.result, message=message, + last_non_quota_failure=last_non_quota_failure, ) case RaiseOutputExtractionFailureAttemptTransition( extracted_output=extracted_output @@ -386,10 +417,12 @@ def _raise_quota_failure( runtime: InvocationCommandRuntime, result: CommandResult, message: str, + last_non_quota_failure: InvocationFailureSummary | None = None, ) -> None: raise build_quota_failure_error( message, runtime.failure_profile, result, None, + last_non_quota_failure, ) diff --git a/src/crewplane/runtime/agent/invocation/retry.py b/src/crewplane/runtime/agent/invocation/retry.py index 32fc382..ff1f807 100644 --- a/src/crewplane/runtime/agent/invocation/retry.py +++ b/src/crewplane/runtime/agent/invocation/retry.py @@ -107,6 +107,7 @@ def evaluate_quota_retry( result: CommandResult, quota_retry_started_at: float | None, quota_retry_count: int, + quota_retry_wait_seconds: float = 0.0, ) -> QuotaRetryDecision: quota = classify_quota(config, result, quota_parser) if not quota.is_quota: @@ -127,6 +128,19 @@ def evaluate_quota_retry( ), ) + if ( + config.quota_retry_max_attempts is not None + and quota_retry_count >= config.quota_retry_max_attempts + ): + return QuotaRetryFailure( + quota_retry_started_at=quota_retry_started_at, + quota_retry_count=quota_retry_count, + message=( + f"Quota retry configured attempt ceiling of " + f"{config.quota_retry_max_attempts} reached for {cmd[0]}" + ), + ) + if ( quota.reset_after_seconds is not None and quota.reset_after_seconds > QUOTA_RETRY_GUARD_SECONDS @@ -143,6 +157,22 @@ def evaluate_quota_retry( ) wait_seconds = compute_quota_wait_seconds(config, quota) + if ( + config.quota_retry_max_wait_seconds is not None + and quota_retry_wait_seconds + wait_seconds + >= config.quota_retry_max_wait_seconds + ): + return QuotaRetryFailure( + quota_retry_started_at=quota_retry_started_at, + quota_retry_count=quota_retry_count, + message=( + "Quota retry configured wait ceiling of " + f"{format_wait_duration(config.quota_retry_max_wait_seconds)} " + f"would be reached for {cmd[0]}; cumulative quota wait is " + f"{format_wait_duration(quota_retry_wait_seconds)} and next wait is " + f"{format_wait_duration(wait_seconds)}" + ), + ) if quota_retry_guard_will_exhaust(quota_retry_started_at, wait_seconds): elapsed_seconds = quota_retry_elapsed_seconds(quota_retry_started_at) return QuotaRetryFailure( @@ -173,17 +203,18 @@ def evaluate_quota_retry( level="warning", message=( f"Quota reached for {cmd[0]}; retrying in {wait_detail} " - f"(quota attempt {quota_retry_count})" + f"(quota attempt {quota_retry_count}; independent of max_retries)" ), operation="quota_retry_scheduled", attributes={ "quota_attempt": quota_retry_count, "wait_seconds": round(wait_seconds, 3), + "ordinary_max_retries": config.max_retries, }, console_message=( "[yellow]WARN[/] Quota reached for " f"{cmd[0]}; retrying in {wait_detail} " - f"(quota attempt {quota_retry_count})" + f"(quota attempt {quota_retry_count}; independent of max_retries)" ), ), ) diff --git a/src/crewplane/runtime/agent/invoker.py b/src/crewplane/runtime/agent/invoker.py index a287048..f967a3a 100644 --- a/src/crewplane/runtime/agent/invoker.py +++ b/src/crewplane/runtime/agent/invoker.py @@ -24,6 +24,8 @@ def __call__( model: str | None, prompt: str, output_file: Path, + invocation_context: InvocationContext | None = None, + working_directory: Path | None = None, ) -> InvocationPlan: ... @@ -119,6 +121,24 @@ async def invoke( log_file: Path | None = None, invocation_context: InvocationContext | None = None, ) -> None: + def build_plan( + plan_config: AgentConfig, + plan_model: str | None, + plan_prompt: str, + plan_output_file: Path, + _plan_invocation_context: InvocationContext | None = None, + _plan_working_directory: Path | None = None, + ) -> InvocationPlan: + del _plan_invocation_context, _plan_working_directory + return self._plan_builder( + plan_config, + plan_model, + plan_prompt, + plan_output_file, + invocation_context, + cwd, + ) + return await invoke_agent( config, model, @@ -127,5 +147,5 @@ async def invoke( cwd, log_file, invocation_context=invocation_context, - plan_builder=self._plan_builder, + plan_builder=build_plan, ) diff --git a/src/crewplane/runtime/execution/activity/events.py b/src/crewplane/runtime/execution/activity/events.py index b26c9cc..553a9c9 100644 --- a/src/crewplane/runtime/execution/activity/events.py +++ b/src/crewplane/runtime/execution/activity/events.py @@ -38,6 +38,7 @@ class RuntimeEventContext: provider: str | None = None role: ProviderRole | None = None model: str | None = None + requested_reasoning: str | None = None task_id: str | None = None audit_round_num: int | None = None round_num: int | None = None @@ -58,6 +59,7 @@ def as_execution_event_context( provider=self.provider, role=self.role, model=self.model, + requested_reasoning=self.requested_reasoning, task_id=self.task_id, audit_round_num=self.audit_round_num, round_num=self.round_num, @@ -139,6 +141,7 @@ class InvocationMetadata: round_num: int output_file: Path log_file: Path | None + requested_reasoning: str | None = None findings_enabled: bool = False log_presentation_format: str | None = None log_presentation_profile: str | None = None @@ -150,6 +153,7 @@ def event_context(self) -> RuntimeEventContext: provider=self.provider, role=self.role, model=self.model, + requested_reasoning=self.requested_reasoning, task_id=self.task_id, audit_round_num=self.audit_round_num, round_num=self.round_num, diff --git a/src/crewplane/runtime/execution/provider_call/artifact_capture.py b/src/crewplane/runtime/execution/provider_call/artifact_capture.py new file mode 100644 index 0000000..3a30923 --- /dev/null +++ b/src/crewplane/runtime/execution/provider_call/artifact_capture.py @@ -0,0 +1,61 @@ +from __future__ import annotations + +from pathlib import Path + +from crewplane.artifacts.generated_files.catalog import ( + generated_file_snapshot_rejection_summary, +) +from crewplane.runtime.workspace import PreparedWorkspace + +from ..activity.events import InvocationMetadata +from .events import emit_artifact_capture_event +from .generated_file_changes import GeneratedFileChangeBaseline +from .generated_files import snapshot_invocation_generated_files_async +from .types import ProviderCallRequest + + +async def capture_invocation_generated_files( + request: ProviderCallRequest, + prepared_workspace: PreparedWorkspace, + baseline: GeneratedFileChangeBaseline | None, + metadata: InvocationMetadata, +) -> Path | None: + try: + workspace = await snapshot_invocation_generated_files_async( + request, + prepared_workspace, + baseline, + ) + except Exception as exc: + request.runtime_context.generated_file_workspaces.record_capture_failure( + request.node_id, + request.output_file, + ) + emit_artifact_capture_event( + request.telemetry, + metadata, + "artifact_capture_failed", + f"Provider output succeeded, but generated-file capture failed: {exc}", + {"output_file": request.output_file.as_posix()}, + ) + return None + + rejection_summary = ( + generated_file_snapshot_rejection_summary(workspace) + if workspace is not None + else None + ) + if rejection_summary is not None and rejection_summary.total_count: + emit_artifact_capture_event( + request.telemetry, + metadata, + "artifact_capture_partial", + "Provider output succeeded with a partial generated-file capture.", + { + "output_file": request.output_file.as_posix(), + "rejected_file_count": rejection_summary.total_count, + "rejected_file_details_recorded": len(rejection_summary.recorded_files), + "rejected_files_truncated": rejection_summary.truncated, + }, + ) + return workspace diff --git a/src/crewplane/runtime/execution/provider_call/events.py b/src/crewplane/runtime/execution/provider_call/events.py index 3399313..596d6d4 100644 --- a/src/crewplane/runtime/execution/provider_call/events.py +++ b/src/crewplane/runtime/execution/provider_call/events.py @@ -59,6 +59,7 @@ def diagnostics_sink(diagnostic: InvocationDiagnostic) -> None: task_id=metadata.task_id, provider=metadata.provider, role=metadata.role, + requested_reasoning=metadata.requested_reasoning, audit_round_num=metadata.audit_round_num, round_num=metadata.round_num, findings_enabled=metadata.findings_enabled, @@ -103,6 +104,25 @@ def emit_provider_invocation_failure_event( exc.add_note(f"invocation failure telemetry failed: {telemetry_exc}") +def emit_artifact_capture_event( + telemetry: ExecutionTelemetry | None, + invocation_metadata: InvocationMetadata | None, + operation: str, + message: str, + attributes: dict[str, object] | None = None, +) -> None: + if telemetry is None or invocation_metadata is None: + return + emit_runtime_log( + telemetry, + "warning", + message, + operation, + context=invocation_metadata.event_context(), + attributes=attributes, + ) + + def resolve_invocation_usage( capture: InvocationEventCapture, agent_config: AgentConfig, diff --git a/src/crewplane/runtime/execution/provider_call/generated_files.py b/src/crewplane/runtime/execution/provider_call/generated_files.py index bd1fbb0..b0faba2 100644 --- a/src/crewplane/runtime/execution/provider_call/generated_files.py +++ b/src/crewplane/runtime/execution/provider_call/generated_files.py @@ -316,6 +316,7 @@ def snapshot_invocation_generated_files( request.output_file, workspace_root, candidate_files=candidate_files, + explicit_claims_only=prepared_workspace.workspace_kind == "project_root", ) diff --git a/src/crewplane/runtime/execution/provider_call/lifecycle.py b/src/crewplane/runtime/execution/provider_call/lifecycle.py index ee40161..a590fe8 100644 --- a/src/crewplane/runtime/execution/provider_call/lifecycle.py +++ b/src/crewplane/runtime/execution/provider_call/lifecycle.py @@ -22,6 +22,7 @@ ) from ..log_presentation import resolve_log_presentation_descriptor from ..runtime_context import CompiledRuntimeContext +from .artifact_capture import capture_invocation_generated_files from .display import ( ProviderCallDisplay, invoke_with_display, @@ -38,7 +39,6 @@ capture_generated_file_change_baseline, finalize_successful_workspace, rendered_workspace_file_descriptors, - snapshot_invocation_generated_files_async, ) from .types import ProviderCallRequest, ProviderCallResult, ProviderOutputPolicy from .workspace import ( @@ -163,6 +163,7 @@ def _initial_invocation_metadata( provider=request.provider.provider, role=request.role_label, model=model, + requested_reasoning=request.provider.requested_reasoning, task_id=request.task_id, audit_round_num=request.audit_round_num, round_num=request.round_num, @@ -268,10 +269,11 @@ async def _invoke_provider_and_finalize_workspace( _validate_provider_output_file(request) if state.child_environment_applied: state.invocation_metadata = metadata.with_workspace_child_environment_applied() - generated_file_workspace = await snapshot_invocation_generated_files_async( + generated_file_workspace = await capture_invocation_generated_files( request, prepared_workspace, state.generated_file_change_baseline, + state.invocation_metadata, ) state.workspace_success_finalization_started = True await finalize_successful_workspace( diff --git a/src/crewplane/runtime/execution/workspace_files/generated.py b/src/crewplane/runtime/execution/workspace_files/generated.py index b54b03c..fc7f34e 100644 --- a/src/crewplane/runtime/execution/workspace_files/generated.py +++ b/src/crewplane/runtime/execution/workspace_files/generated.py @@ -14,7 +14,7 @@ class GeneratedFileWorkspaceCleanupResult: @dataclass class GeneratedFileWorkspaceRegistry: - roots_by_node: dict[str, dict[Path, Path]] = field(default_factory=dict) + roots_by_node: dict[str, dict[Path, Path | None]] = field(default_factory=dict) cleanup_by_node: dict[str, list[Callable[[], None]]] = field(default_factory=dict) def record( @@ -31,7 +31,12 @@ def record( if cleanup is not None: self.cleanup_by_node.setdefault(node_id, []).append(cleanup) - def roots_for_node(self, node_id: str) -> dict[Path, Path]: + def record_capture_failure(self, node_id: str, output_file: Path) -> None: + self.roots_by_node.setdefault(node_id, {})[ + output_file.resolve(strict=False) + ] = None + + def roots_for_node(self, node_id: str) -> dict[Path, Path | None]: return dict(self.roots_by_node.get(node_id, {})) def alias_output_file( @@ -43,10 +48,10 @@ def alias_output_file( roots = self.roots_by_node.get(node_id) if roots is None: return - workspace_root = roots.get(source_output_file.resolve(strict=False)) - if workspace_root is None: + source_output = source_output_file.resolve(strict=False) + if source_output not in roots: return - roots[alias_output_file.resolve(strict=False)] = workspace_root + roots[alias_output_file.resolve(strict=False)] = roots[source_output] def cleanup_node(self, node_id: str) -> None: errors = self.cleanup_node_best_effort(node_id) diff --git a/tests/integration/adapters/test_invoker_cli.py b/tests/integration/adapters/test_invoker_cli.py index 3742094..d306dd0 100644 --- a/tests/integration/adapters/test_invoker_cli.py +++ b/tests/integration/adapters/test_invoker_cli.py @@ -11,7 +11,7 @@ build_cli_log_presentation, ) from crewplane.adapters.invokers.cli_invoker.capabilities import CAPABILITIES -from crewplane.architecture.contracts import SUPPORTED_PROVIDER_KINDS +from crewplane.architecture.contracts import SUPPORTED_PROVIDER_KINDS, InvocationContext from crewplane.core.config import AgentConfig, Config from crewplane.version import SCHEMA_VERSION @@ -122,3 +122,644 @@ def test_invocation_plan_preserves_relative_path_cli_executable( ) self.assertEqual(plan.cmd[0], relative_executable) + + def test_codex_reasoning_request_builds_native_config_before_extra_args( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="codex", + role="executor", + requested_reasoning="xhigh", + ) + plan = build_cli_invocation_plan( + AgentConfig( + cli_cmd=["codex", "exec"], + provider_kind="codex", + extra_args=["--ephemeral"], + ), + model="gpt-5.5", + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + self.assertEqual( + plan.cmd[1:8], + [ + "exec", + "--model", + "gpt-5.5", + "--config", + 'model_reasoning_effort="xhigh"', + "--ephemeral", + "--json", + ], + ) + self.assertIn("requested_reasoning: xhigh\n", plan.log_header.decode()) + + def test_claude_reasoning_request_builds_native_effort(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="reviewer", + requested_reasoning="high", + ) + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + plan = build_cli_invocation_plan( + AgentConfig(cli_cmd=["claude"], provider_kind="claude"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + self.assertEqual(plan.cmd[1:3], ["--effort", "high"]) + + def test_reasoning_request_rejects_claude_settings_effort(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + configs = ( + AgentConfig( + cli_cmd=[ + "claude", + "--settings", + '{"effortLevel": "low"}', + ], + provider_kind="claude", + ), + AgentConfig( + cli_cmd=["claude"], + extra_args=['--settings={"effortLevel": "low"}'], + provider_kind="claude", + ), + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + for config in configs: + with ( + self.subTest(config=config), + self.assertRaisesRegex(ValueError, "--settings effortLevel"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_claude_settings_environment(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=[ + "claude", + "--settings", + '{"env": {"CLAUDE_CODE_EFFORT_LEVEL": "low"}}', + ], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "CLAUDE_CODE_EFFORT_LEVEL"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_allows_unrelated_claude_settings(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=[ + "claude", + "--settings", + ( + '{"permissions": {"allow": ["Read"]}, ' + '"effortLevel": "", ' + '"env": {"CLAUDE_CODE_EFFORT_LEVEL": ""}}' + ), + ], + provider_kind="claude", + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + plan = build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + self.assertIn("--effort", plan.cmd) + + def test_reasoning_request_checks_relative_claude_settings_file(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + with tempfile.TemporaryDirectory() as tmp_dir: + working_directory = Path(tmp_dir) + (working_directory / "claude-settings.json").write_text( + '{"effortLevel": "low"}', + encoding="utf-8", + ) + config = AgentConfig( + cli_cmd=["claude", "--settings", "claude-settings.json"], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "--settings effortLevel"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=working_directory / "output.md", + invocation_context=context, + working_directory=working_directory, + ) + + def test_reasoning_request_fails_closed_for_unreadable_claude_settings( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=["claude", "--settings", "missing-settings.json"], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "Cannot validate"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_direct_codex_config_conflict(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="codex", + role="executor", + requested_reasoning="high", + ) + conflict_forms = ( + ["--config", 'model_reasoning_effort="low"'], + ['--config=model_reasoning_effort="low"'], + ["-c", 'model_reasoning_effort = "low"'], + ['-c=model_reasoning_effort="low"'], + ['-cmodel_reasoning_effort="low"'], + ) + + for extra_args in conflict_forms: + with ( + self.subTest(extra_args=extra_args), + self.assertRaisesRegex(ValueError, "model_reasoning_effort"), + ): + build_cli_invocation_plan( + AgentConfig( + cli_cmd=["codex", "exec"], + provider_kind="codex", + extra_args=extra_args, + ), + model="gpt-5.5", + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_codex_reasoning_conflict_scan_respects_extra_args_terminator( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="codex", + role="executor", + requested_reasoning="high", + ) + + plan = build_cli_invocation_plan( + AgentConfig( + cli_cmd=["codex", "exec"], + provider_kind="codex", + extra_args=["--", 'model_reasoning_effort="low"'], + ), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + self.assertEqual( + plan.cmd[2:6], + [ + "--config", + 'model_reasoning_effort="high"', + "--", + 'model_reasoning_effort="low"', + ], + ) + + def test_codex_reasoning_allows_env_prefix_provider_like_tokens(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="codex", + role="executor", + requested_reasoning="high", + ) + + plan = build_cli_invocation_plan( + AgentConfig( + cli_cmd=["env", "-u", "--config", "codex"], + provider_kind="codex", + ), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + command_index = plan.cmd.index("codex") + self.assertEqual( + plan.cmd[command_index + 1 : command_index + 3], + ["--config", 'model_reasoning_effort="high"'], + ) + + def test_reasoning_request_rejects_cli_command_option_terminator(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="codex", + role="executor", + requested_reasoning="high", + ) + + with self.assertRaisesRegex(ValueError, "option terminator"): + build_cli_invocation_plan( + AgentConfig( + cli_cmd=["codex", "exec", "--"], + provider_kind="codex", + ), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_unsupported_provider_kind(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="generic", + role="executor", + requested_reasoning="high", + ) + + with self.assertRaisesRegex(ValueError, "provider_kind 'codex' or 'claude'"): + build_cli_invocation_plan( + AgentConfig(cli_cmd=["provider"], provider_kind="generic"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_claude_environment_conflict(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": "low"}), + self.assertRaisesRegex(ValueError, "CLAUDE_CODE_EFFORT_LEVEL"), + ): + build_cli_invocation_plan( + AgentConfig(cli_cmd=["claude"], provider_kind="claude"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_claude_cli_environment_assignment( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=["env", "CLAUDE_CODE_EFFORT_LEVEL=low", "claude"], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "CLAUDE_CODE_EFFORT_LEVEL"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_allows_blank_claude_cli_environment_assignment( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=["env", "CLAUDE_CODE_EFFORT_LEVEL=", "claude"], + provider_kind="claude", + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": "low"}): + plan = build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + self.assertIn("--effort", plan.cmd) + + def test_reasoning_request_rejects_path_qualified_env_assignment(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=[ + "/usr/bin/env", + "-i", + "CLAUDE_CODE_EFFORT_LEVEL=low", + "claude", + ], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "CLAUDE_CODE_EFFORT_LEVEL"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_allows_assignment_tokens_after_env_command( + self, + ) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + assignment = "CLAUDE_CODE_EFFORT_LEVEL=low" + configs = ( + AgentConfig( + cli_cmd=["env", "claude", assignment], + provider_kind="claude", + ), + AgentConfig( + cli_cmd=["claude"], + extra_args=[assignment], + provider_kind="claude", + ), + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + for config in configs: + with self.subTest(config=config): + plan = build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + self.assertIn(assignment, plan.cmd) + self.assertIn("--effort", plan.cmd) + + def test_reasoning_request_allows_env_to_remove_inherited_effort(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + configs = ( + AgentConfig( + cli_cmd=["env", "-i", "claude"], + provider_kind="claude", + ), + AgentConfig( + cli_cmd=["env", "-u", "CLAUDE_CODE_EFFORT_LEVEL", "claude"], + provider_kind="claude", + ), + AgentConfig( + cli_cmd=[ + "env", + "--unset=CLAUDE_CODE_EFFORT_LEVEL", + "claude", + ], + provider_kind="claude", + ), + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": "low"}): + for config in configs: + with self.subTest(config=config): + plan = build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + self.assertIn("--effort", plan.cmd) + + def test_reasoning_request_rejects_clustered_env_assignment(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + config = AgentConfig( + cli_cmd=[ + "env", + "-iv", + "CLAUDE_CODE_EFFORT_LEVEL=low", + "claude", + ], + provider_kind="claude", + ) + + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "CLAUDE_CODE_EFFORT_LEVEL"), + ): + build_cli_invocation_plan( + config, + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_env_split_string(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + split_value = "CLAUDE_CODE_EFFORT_LEVEL=low claude" + cli_commands = ( + ["env", "-S", split_value], + ["env", f"-S{split_value}"], + ["env", "--split-string", split_value], + ["env", f"--split-string={split_value}"], + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + for cli_cmd in cli_commands: + with ( + self.subTest(cli_cmd=cli_cmd), + self.assertRaisesRegex(ValueError, "cannot be combined"), + ): + build_cli_invocation_plan( + AgentConfig(cli_cmd=cli_cmd, provider_kind="claude"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_rejects_env_working_directory_change(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + cli_commands = ( + ["env", "--chdir", "subdir", "claude"], + ["env", "--chdir=subdir", "claude"], + ["env", "-C", "subdir", "claude"], + ["env", "-Csubdir", "claude"], + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + for cli_cmd in cli_commands: + with ( + self.subTest(cli_cmd=cli_cmd), + self.assertRaisesRegex(ValueError, "cannot be combined"), + ): + build_cli_invocation_plan( + AgentConfig(cli_cmd=cli_cmd, provider_kind="claude"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + + def test_reasoning_request_allows_env_prefix_provider_like_tokens(self) -> None: + context = InvocationContext( + node_id="node", + task_id="task", + provider="claude", + role="executor", + requested_reasoning="high", + ) + cli_commands = ( + ["env", "--", "claude"], + ["env", "-u", "--effort", "claude"], + ["env", "--unset", "--effort", "claude"], + ) + + with patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}): + for cli_cmd in cli_commands: + with self.subTest(cli_cmd=cli_cmd): + plan = build_cli_invocation_plan( + AgentConfig(cli_cmd=cli_cmd, provider_kind="claude"), + model=None, + prompt="prompt", + output_file=Path("output.md"), + invocation_context=context, + ) + command_index = plan.cmd.index("claude") + self.assertEqual( + plan.cmd[command_index + 1 : command_index + 3], + ["--effort", "high"], + ) diff --git a/tests/integration/architecture/static_checks.py b/tests/integration/architecture/static_checks.py new file mode 100644 index 0000000..4e9d0ac --- /dev/null +++ b/tests/integration/architecture/static_checks.py @@ -0,0 +1,319 @@ +from __future__ import annotations + +import ast +from collections import deque +from collections.abc import Iterator +from dataclasses import dataclass +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +SRC_ROOT = REPO_ROOT / "src" +TESTS_ROOT = REPO_ROOT / "tests" + + +@dataclass(frozen=True) +class ForbiddenImportRule: + name: str + roots: tuple[Path, ...] + forbidden_prefixes: tuple[str, ...] + + +@dataclass(frozen=True) +class PrivateApiRule: + name: str + roots: tuple[Path, ...] + allowed_references: frozenset[str] = frozenset() + + +@dataclass(frozen=True) +class AllowedTextReference: + path: Path + term: str + reason: str + + +@dataclass(frozen=True) +class ForbiddenTextRule: + name: str + paths: tuple[Path, ...] + forbidden_terms: frozenset[str] + allowed_references: tuple[AllowedTextReference, ...] = () + + +def python_files(root: Path) -> tuple[Path, ...]: + if root.is_file(): + return (root,) if root.suffix == ".py" else () + return tuple(path for path in root.rglob("*.py") if "__pycache__" not in path.parts) + + +def physical_line_count(path: Path) -> int: + return len(path.read_text(encoding="utf-8").splitlines()) + + +def parse_python(path: Path) -> ast.Module: + return ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + + +def walk_ast(node: ast.AST) -> Iterator[ast.AST]: + todo: deque[ast.AST] = deque([node]) + while todo: + current = todo.popleft() + yield current + for field in current._fields: + value = getattr(current, field, None) + if isinstance(value, ast.AST): + todo.append(value) + elif isinstance(value, list): + todo.extend(child for child in value if isinstance(child, ast.AST)) + + +def call_name(node: ast.AST) -> str | None: + if isinstance(node, ast.Name): + return node.id + if isinstance(node, ast.Attribute): + return node.attr + return None + + +def expression_chain(node: ast.AST) -> tuple[str, ...]: + if isinstance(node, ast.Name): + return (node.id,) + if isinstance(node, ast.Attribute): + return (*expression_chain(node.value), node.attr) + return () + + +def import_from_module_name(path: Path, node: ast.ImportFrom) -> str: + if node.level == 0: + return node.module or "" + + module_parts = module_name_for_path(path).split(".") + package_parts = module_parts[:-1] + keep_count = max(0, len(package_parts) - node.level + 1) + base_parts = package_parts[:keep_count] + if node.module: + base_parts.extend(node.module.split(".")) + return ".".join(base_parts) + + +def find_forbidden_imports(rule: ForbiddenImportRule) -> list[str]: + offenders: list[str] = [] + for root in rule.roots: + for path in python_files(root): + for node in walk_ast(parse_python(path)): + for imported_module in imported_modules(path, node): + if any( + imported_module == prefix + or imported_module.startswith(f"{prefix}.") + for prefix in rule.forbidden_prefixes + ): + offenders.append(offender(path, node.lineno, imported_module)) + return offenders + + +def find_private_imports(rule: PrivateApiRule) -> list[str]: + offenders: list[str] = [] + for root in rule.roots: + for path in python_files(root): + for node in walk_ast(parse_python(path)): + references = private_import_references(path, node) + offenders.extend( + offender(path, node.lineno, reference) + for reference in references + if reference not in rule.allowed_references + ) + return offenders + + +def find_private_attribute_access(rule: PrivateApiRule) -> list[str]: + offenders: list[str] = [] + for root in rule.roots: + for path in python_files(root): + module = parse_python(path) + imported_aliases = imported_repo_aliases(path, module) + for node in walk_ast(module): + if not isinstance(node, ast.Attribute): + continue + reference = ".".join(expression_chain(node)) + if ( + is_single_underscore_name(node.attr) + and is_repo_import_expression(node.value, imported_aliases) + and reference not in rule.allowed_references + ): + offenders.append(offender(path, node.lineno, reference)) + return offenders + + +def find_private_patch_targets(rule: PrivateApiRule) -> list[str]: + offenders: list[str] = [] + for root in rule.roots: + for path in python_files(root): + module = parse_python(path) + imported_aliases = imported_repo_aliases(path, module) + for node in walk_ast(module): + if not isinstance(node, ast.Call): + continue + target = private_patch_target(node, imported_aliases) + if target is not None and target not in rule.allowed_references: + offenders.append(offender(path, node.lineno, target)) + return offenders + + +def find_forbidden_text(rule: ForbiddenTextRule) -> list[str]: + allowed = { + (reference.path.resolve(), reference.term): reference.reason + for reference in rule.allowed_references + } + if any(not reason.strip() for reason in allowed.values()): + raise ValueError(f"Text allowlist reasons must be non-empty for {rule.name}.") + + offenders: list[str] = [] + for path in text_rule_files(rule.paths): + for line_number, line in enumerate( + path.read_text(encoding="utf-8").splitlines(), + start=1, + ): + offenders.extend( + offender(path, line_number, term) + for term in rule.forbidden_terms + if term in line and (path.resolve(), term) not in allowed + ) + return offenders + + +def offender(path: Path, line: int, detail: str | None = None) -> str: + location = f"{path.relative_to(REPO_ROOT)}:{line}" + return f"{location}: {detail}" if detail else location + + +def imported_modules(path: Path, node: ast.AST) -> tuple[str, ...]: + if isinstance(node, ast.Import): + return tuple(alias.name for alias in node.names) + if isinstance(node, ast.ImportFrom): + return (import_from_module_name(path, node),) + return () + + +def text_rule_files(paths: tuple[Path, ...]) -> tuple[Path, ...]: + files: list[Path] = [] + for path in paths: + if not path.exists(): + raise FileNotFoundError(f"Text rule path does not exist: {path}") + if path.is_file(): + files.append(path) + elif path.is_dir(): + files.extend( + candidate for candidate in path.rglob("*") if candidate.is_file() + ) + return tuple(files) + + +def module_name_for_path(path: Path) -> str: + if path.is_relative_to(SRC_ROOT): + return ".".join(path.relative_to(SRC_ROOT).with_suffix("").parts) + if path.is_relative_to(TESTS_ROOT): + relative = path.relative_to(TESTS_ROOT).with_suffix("") + return ".".join(("tests", *relative.parts)) + raise ValueError(f"Unsupported Python path: {path}") + + +def is_single_underscore_name(name: str) -> bool: + return name.startswith("_") and not name.startswith("__") + + +def is_repo_module_name(module_name: str) -> bool: + return ( + module_name == "crewplane" + or module_name.startswith("crewplane.") + or module_name == "tests" + or module_name.startswith("tests.") + ) + + +def imported_repo_aliases(path: Path, module: ast.Module) -> set[str]: + aliases: set[str] = set() + for node in walk_ast(module): + if isinstance(node, ast.Import): + aliases.update( + alias.asname or alias.name.split(".")[0] + for alias in node.names + if is_repo_module_name(alias.name) + ) + elif isinstance(node, ast.ImportFrom): + module_name = import_from_module_name(path, node) + if is_repo_module_name(module_name): + aliases.update( + alias.asname or alias.name + for alias in node.names + if alias.name != "*" and (alias.asname or alias.name)[:1].islower() + ) + return aliases + + +def is_repo_import_expression(node: ast.AST, imported_aliases: set[str]) -> bool: + chain = expression_chain(node) + return bool(chain) and ( + chain[0] in imported_aliases or chain[0] in {"crewplane", "tests"} + ) + + +def private_import_references(path: Path, node: ast.AST) -> tuple[str, ...]: + if isinstance(node, ast.Import): + return tuple( + alias.name + for alias in node.names + if any(is_single_underscore_name(part) for part in alias.name.split(".")) + ) + if not isinstance(node, ast.ImportFrom): + return () + module_name = import_from_module_name(path, node) + references = [ + module_name + for part in module_name.split(".") + if is_single_underscore_name(part) + ] + references.extend( + f"{module_name}.{alias.name}" + for alias in node.names + if is_single_underscore_name(alias.name) + ) + return tuple(references) + + +def private_patch_target( + node: ast.Call, + imported_aliases: set[str], +) -> str | None: + call_chain = expression_chain(node.func) + if call_chain[-1:] == ("patch",) and node.args: + target = string_value(node.args[0]) + if target is not None and has_private_dotted_part(target): + return target + if ( + call_chain[-1:] == ("setattr",) + and len(node.args) >= 2 + and is_repo_import_expression(node.args[0], imported_aliases) + ): + return private_name_argument(node.args[1]) + if ( + call_chain[-2:] == ("patch", "object") + and len(node.args) >= 2 + and is_repo_import_expression(node.args[0], imported_aliases) + ): + return private_name_argument(node.args[1]) + return None + + +def private_name_argument(node: ast.AST) -> str | None: + name = string_value(node) + return name if name is not None and is_single_underscore_name(name) else None + + +def string_value(node: ast.AST) -> str | None: + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return node.value + return None + + +def has_private_dotted_part(value: str) -> bool: + return any(is_single_underscore_name(part) for part in value.split(".")) diff --git a/tests/integration/architecture/test_docs_drift.py b/tests/integration/architecture/test_docs_drift.py new file mode 100644 index 0000000..3745c88 --- /dev/null +++ b/tests/integration/architecture/test_docs_drift.py @@ -0,0 +1,55 @@ +from __future__ import annotations + +from tests.integration.architecture.static_checks import ( + REPO_ROOT, + SRC_ROOT, + ForbiddenTextRule, + find_forbidden_text, + python_files, +) + +LEGACY_PROMPT_CONFIG_RULE = ForbiddenTextRule( + name="public docs and templates omit legacy prompt config", + paths=( + REPO_ROOT / "README.md", + REPO_ROOT / "docs" / "architecture" / "modular-orchestration-architecture.md", + SRC_ROOT / "crewplane" / "example_templates" / "config.yml", + ), + forbidden_terms=frozenset( + { + "prompt_arg", + "quota_parser", + "stdin_prompt_arg", + "use_stdin", + } + ), +) + +STALE_VERSION_IMPORT_RULE = ForbiddenTextRule( + name="production source uses the canonical version catalog", + paths=python_files(SRC_ROOT / "crewplane"), + forbidden_terms=frozenset( + { + "crewplane.architecture.api_version", + "crewplane.core.versions", + "crewplane.versions", + } + ), +) + + +def test_docs_and_templates_do_not_reference_legacy_prompt_config_fields() -> None: + assert find_forbidden_text(LEGACY_PROMPT_CONFIG_RULE) == [] + + +def test_version_catalog_has_single_public_python_source() -> None: + stale_paths = [ + SRC_ROOT / "crewplane" / "versions.py", + SRC_ROOT / "crewplane" / "core" / "versions.py", + SRC_ROOT / "crewplane" / "architecture" / "api_version.py", + ] + assert [ + path.relative_to(REPO_ROOT).as_posix() for path in stale_paths if path.exists() + ] == [] + + assert find_forbidden_text(STALE_VERSION_IMPORT_RULE) == [] diff --git a/tests/integration/architecture/test_import_boundaries.py b/tests/integration/architecture/test_import_boundaries.py new file mode 100644 index 0000000..691bd64 --- /dev/null +++ b/tests/integration/architecture/test_import_boundaries.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import pytest + +from tests.integration.architecture.static_checks import ( + REPO_ROOT, + SRC_ROOT, + ForbiddenImportRule, + find_forbidden_imports, +) + +IMPORT_RULES = ( + ForbiddenImportRule( + name="adapters stay independent from runtime execution", + roots=(SRC_ROOT / "crewplane" / "adapters",), + forbidden_prefixes=("crewplane.runtime.execution",), + ), + ForbiddenImportRule( + name="architecture ports stay runtime neutral", + roots=(SRC_ROOT / "crewplane" / "architecture" / "ports",), + forbidden_prefixes=( + "crewplane.core.preflight.runtime_config", + "crewplane.runtime", + "crewplane.observability", + ), + ), + ForbiddenImportRule( + name="review contract stays core neutral", + roots=(SRC_ROOT / "crewplane" / "core" / "review_contract.py",), + forbidden_prefixes=( + "crewplane.runtime", + "crewplane.adapters", + "crewplane.artifacts", + "crewplane.observability", + ), + ), + ForbiddenImportRule( + name="log presentation stays independent from runtime parsing", + roots=(SRC_ROOT / "crewplane" / "observability" / "log_presentation",), + forbidden_prefixes=( + "crewplane.runtime.agent.invocation.output", + "crewplane.runtime.agent.invocation.claude_json", + "crewplane.runtime.agent.usage_parsing", + "crewplane.runtime.agent.quota", + "crewplane.runtime.agent.failures", + ), + ), +) + + +@pytest.mark.parametrize("rule", IMPORT_RULES, ids=lambda rule: rule.name) +def test_forbidden_import_rule(rule: ForbiddenImportRule) -> None: + assert find_forbidden_imports(rule) == [] + + +def test_import_rule_roots_exist() -> None: + missing = [ + str(root.relative_to(REPO_ROOT)) + for rule in IMPORT_RULES + for root in rule.roots + if not root.exists() + ] + assert missing == [] diff --git a/tests/integration/architecture/test_private_api_boundaries.py b/tests/integration/architecture/test_private_api_boundaries.py new file mode 100644 index 0000000..813fb58 --- /dev/null +++ b/tests/integration/architecture/test_private_api_boundaries.py @@ -0,0 +1,41 @@ +from __future__ import annotations + +import pytest + +from tests.integration.architecture.static_checks import ( + SRC_ROOT, + TESTS_ROOT, + PrivateApiRule, + find_private_attribute_access, + find_private_imports, + find_private_patch_targets, +) + +REPO_PRIVATE_API_RULE = PrivateApiRule( + name="repo modules expose cross-module collaborators publicly", + roots=(SRC_ROOT, TESTS_ROOT), +) + + +@pytest.mark.parametrize( + ("check", "rule"), + ( + pytest.param( + find_private_imports, + REPO_PRIVATE_API_RULE, + id="private-imports", + ), + pytest.param( + find_private_attribute_access, + REPO_PRIVATE_API_RULE, + id="private-attribute-access", + ), + pytest.param( + find_private_patch_targets, + REPO_PRIVATE_API_RULE, + id="private-patch-targets", + ), + ), +) +def test_private_api_rule(check, rule: PrivateApiRule) -> None: + assert check(rule) == [] diff --git a/tests/integration/architecture/test_public_api_boundaries.py b/tests/integration/architecture/test_public_api_boundaries.py new file mode 100644 index 0000000..8b52f68 --- /dev/null +++ b/tests/integration/architecture/test_public_api_boundaries.py @@ -0,0 +1,208 @@ +from __future__ import annotations + +import ast +from dataclasses import fields + +from crewplane.artifacts.results.review_loop_status import ReviewLoopStatusError +from crewplane.runtime.execution.provider_call import ProviderCallRequest +from tests.integration.architecture.static_checks import ( + REPO_ROOT, + SRC_ROOT, + TESTS_ROOT, + call_name, + offender, + parse_python, + python_files, + walk_ast, +) + +LEGACY_EVENT_FIELDS = { + "attempt_count", + "attributes", + "audit_round_num", + "cli_captured", + "configured_cost_usd", + "duration_ms", + "error", + "failure_advice", + "failure_kind", + "failure_phase", + "failure_source", + "invocation_cost_confidence", + "level", + "log_file", + "log_presentation_format", + "log_presentation_profile", + "message", + "model", + "node_id", + "operation", + "output_extraction_status", + "output_file", + "provider", + "provider_tokens", + "provider_usage_status", + "role", + "round_num", + "task_id", + "usage_parse_error", + "visible_estimate_is_lower_bound", + "visible_estimate_method", + "visible_estimate_tokens", +} + + +def test_pep561_marker_is_packaged() -> None: + marker = SRC_ROOT / "crewplane" / "py.typed" + pyproject = (REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8") + assert marker.is_file() + assert '"src/crewplane/py.typed"' in pyproject + + +def test_execution_events_do_not_use_legacy_flat_fields() -> None: + offenders: list[str] = [] + for root in (SRC_ROOT, TESTS_ROOT): + for path in python_files(root): + for node in walk_ast(parse_python(path)): + if ( + not isinstance(node, ast.Call) + or call_name(node.func) != "ExecutionEvent" + ): + continue + legacy_keywords = sorted( + keyword.arg + for keyword in node.keywords + if keyword.arg in LEGACY_EVENT_FIELDS + ) + if legacy_keywords: + offenders.append(offender(path, node.lineno, str(legacy_keywords))) + assert offenders == [] + + +def test_execution_event_has_no_legacy_flat_accessors() -> None: + path = SRC_ROOT / "crewplane" / "observability" / "events" / "execution_event.py" + offenders: list[str] = [] + for node in walk_ast(parse_python(path)): + if ( + isinstance(node, ast.FunctionDef) + and node.name in LEGACY_EVENT_FIELDS + and any( + call_name(decorator) == "property" for decorator in node.decorator_list + ) + ): + offenders.append(offender(path, node.lineno, node.name)) + assert offenders == [] + + +def test_public_package_exports_are_narrow() -> None: + import crewplane.core as core_package + import crewplane.runtime as runtime_package + + assert core_package.__all__ == ["SCHEMA_VERSION"] + assert runtime_package.__all__ == [] + assert "__getattr__" not in vars(runtime_package) + + +def test_output_manager_directory_fields_are_read_only_properties() -> None: + path = SRC_ROOT / "crewplane" / "artifacts" / "manager.py" + managed_fields = { + "base_dir", + "log_cli_output", + "logs_dir", + "results_dir", + "run_id", + "stages_dir", + "task_name", + } + assigned_fields: list[str] = [] + property_fields: set[str] = set() + for node in parse_python(path).body: + if not isinstance(node, ast.ClassDef) or node.name != "OutputManager": + continue + for member in node.body: + if ( + isinstance(member, ast.FunctionDef) + and member.name in managed_fields + and any( + call_name(decorator) == "property" + for decorator in member.decorator_list + ) + ): + property_fields.add(member.name) + if not isinstance(member, ast.FunctionDef) or member.name != "__init__": + continue + assigned_fields.extend( + target.attr + for child in walk_ast(member) + for target in assignment_targets(child) + if ( + isinstance(target, ast.Attribute) + and isinstance(target.value, ast.Name) + and target.value.id == "self" + and target.attr in managed_fields + ) + ) + assert assigned_fields == [] + assert property_fields == managed_fields + + +def test_boundary_option_contracts_use_json_object() -> None: + checked_paths = ( + SRC_ROOT / "crewplane" / "architecture", + SRC_ROOT / "crewplane" / "bootstrap", + SRC_ROOT / "crewplane" / "adapters", + ) + forbidden_types = {"dict[str, Any]", "dict[str, object]"} + offenders: list[str] = [] + for root in checked_paths: + for path in python_files(root): + for node in walk_ast(parse_python(path)): + if isinstance(node, ast.Subscript): + rendered = ast.unparse(node) + if rendered in forbidden_types: + offenders.append(offender(path, node.lineno, rendered)) + assert offenders == [] + + +def test_preflight_any_maps_are_limited_to_redaction_traversal() -> None: + allowed_path = ( + SRC_ROOT + / "crewplane" + / "core" + / "preflight" + / "runtime_config" + / "redaction.py" + ) + offenders: list[str] = [] + for path in python_files(SRC_ROOT / "crewplane" / "core" / "preflight"): + module = parse_python(path) + for node in walk_ast(module): + if not isinstance(node, ast.Subscript): + continue + if ast.unparse(node) != "dict[str, Any]": + continue + if path != allowed_path: + offenders.append(offender(path, node.lineno, "dict[str, Any]")) + if path == allowed_path: + docstring = ast.get_docstring(module) or "" + if "arbitrary JSON-compatible config snapshots" not in docstring: + offenders.append(offender(path, 1, "missing rationale")) + assert offenders == [] + + +def test_review_loop_status_error_is_public() -> None: + assert ReviewLoopStatusError.__name__ == "ReviewLoopStatusError" + + +def test_provider_call_request_does_not_carry_display_state() -> None: + request_fields = {field.name for field in fields(ProviderCallRequest)} + assert "progress_description" not in request_fields + assert "show_console_summary" not in request_fields + + +def assignment_targets(node: ast.AST) -> tuple[ast.expr, ...]: + if isinstance(node, ast.Assign): + return tuple(node.targets) + if isinstance(node, ast.AnnAssign): + return (node.target,) + return () diff --git a/tests/integration/architecture/test_refactor_boundaries.py b/tests/integration/architecture/test_refactor_boundaries.py deleted file mode 100644 index d96691a..0000000 --- a/tests/integration/architecture/test_refactor_boundaries.py +++ /dev/null @@ -1,892 +0,0 @@ -from __future__ import annotations - -import ast -from collections import deque -from collections.abc import Iterator -from pathlib import Path - -REPO_ROOT = Path(__file__).resolve().parents[3] -SRC_ROOT = REPO_ROOT / "src" -TESTS_ROOT = REPO_ROOT / "tests" -PRODUCTION_LINE_LIMIT = 500 - -ORIGINAL_OVERSIZED_FILES = ( - "src/crewplane/core/workflow_markdown.py", - "src/crewplane/core/workflow_composition.py", - "src/crewplane/cli/workflow_runner.py", - "src/crewplane/observability/events.py", - "src/crewplane/observability/persistent.py", - "src/crewplane/observability/render.py", - "src/crewplane/artifacts/results/writer.py", - "src/crewplane/runtime/agent/quota.py", - "src/crewplane/runtime/agent/failures.py", - "src/crewplane/runtime/execution/consensus.py", - "src/crewplane/runtime/execution/common.py", - "src/crewplane/adapters/invokers/mock.py", -) - -SPLIT_MODULES = ( - "src/crewplane/core/review_contract.py", - "src/crewplane/core/workflow/markdown/models.py", - "src/crewplane/core/workflow/markdown/frontmatter.py", - "src/crewplane/core/workflow/markdown/sections.py", - "src/crewplane/core/workflow/markdown/markers.py", - "src/crewplane/core/workflow/markdown/payloads.py", - "src/crewplane/core/workflow/composition/models.py", - "src/crewplane/core/workflow/composition/parsing.py", - "src/crewplane/core/workflow/composition/imports.py", - "src/crewplane/core/workflow/composition/nodes.py", - "src/crewplane/core/workflow/composition/rewrites.py", - "src/crewplane/core/workflow/composition/traversal.py", - "src/crewplane/cli/run/context.py", - "src/crewplane/cli/run/preflight.py", - "src/crewplane/cli/run/manifest.py", - "src/crewplane/cli/run/components.py", - "src/crewplane/cli/run/observability.py", - "src/crewplane/cli/run/execution.py", - "src/crewplane/cli/run/topology.py", - "src/crewplane/observability/events/types.py", - "src/crewplane/observability/events/payloads.py", - "src/crewplane/observability/events/execution_event.py", - "src/crewplane/observability/events/builders.py", - "src/crewplane/observability/events/dashboard_state.py", - "src/crewplane/observability/events/reducer.py", - "src/crewplane/observability/events/log.py", - "src/crewplane/observability/run_summary/logger.py", - "src/crewplane/observability/run_summary/models.py", - "src/crewplane/observability/run_summary/builder.py", - "src/crewplane/observability/run_summary/issues.py", - "src/crewplane/observability/run_summary/spend.py", - "src/crewplane/observability/run_summary/markdown.py", - "src/crewplane/observability/run_summary/terminal.py", - "src/crewplane/observability/run_summary/formatting.py", - "src/crewplane/observability/render/viewport.py", - "src/crewplane/observability/render/header.py", - "src/crewplane/observability/render/timeline.py", - "src/crewplane/observability/render/cells.py", - "src/crewplane/observability/render/text.py", - "src/crewplane/observability/text_layout.py", - "src/crewplane/artifacts/results/selection.py", - "src/crewplane/artifacts/results/review_loop_status.py", - "src/crewplane/artifacts/results/stage_document.py", - "src/crewplane/artifacts/results/findings.py", - "src/crewplane/artifacts/results/stage_outputs.py", - "src/crewplane/runtime/agent/quota/lexicons.py", - "src/crewplane/runtime/agent/quota/parser_resolution.py", - "src/crewplane/runtime/agent/quota/evidence.py", - "src/crewplane/runtime/agent/quota/waits.py", - "src/crewplane/runtime/agent/quota/classifier.py", - "src/crewplane/runtime/agent/failures/types.py", - "src/crewplane/runtime/agent/failures/patterns.py", - "src/crewplane/runtime/agent/failures/evidence.py", - "src/crewplane/runtime/agent/failures/classifier.py", - "src/crewplane/runtime/agent/failures/formatting.py", - "src/crewplane/runtime/agent/process/diagnostics.py", - "src/crewplane/runtime/agent/process/runner.py", - "src/crewplane/runtime/agent/process/signals.py", - "src/crewplane/runtime/agent/process/streams.py", - "src/crewplane/runtime/agent/invocation/command.py", - "src/crewplane/runtime/agent/invocation/loop.py", - "src/crewplane/runtime/agent/invocation/output.py", - "src/crewplane/runtime/agent/invocation/retry.py", - "src/crewplane/runtime/agent/invocation/state.py", - "src/crewplane/runtime/agent/invocation/telemetry.py", - "src/crewplane/runtime/agent/invocation/transitions.py", - "src/crewplane/runtime/execution/reviews/structured.py", - "src/crewplane/runtime/execution/reviews/fingerprints.py", - "src/crewplane/runtime/execution/reviews/plain_language.py", - "src/crewplane/runtime/execution/reviews/consensus.py", - "src/crewplane/runtime/execution/reviews/types.py", - "src/crewplane/runtime/execution/runtime_context.py", - "src/crewplane/runtime/execution/activity/telemetry.py", - "src/crewplane/runtime/execution/activity/console.py", - "src/crewplane/runtime/execution/activity/events.py", - "src/crewplane/runtime/execution/fragment_assembler.py", - "src/crewplane/runtime/execution/prompt_budgeting.py", - "src/crewplane/runtime/execution/stage_tasks.py", - "src/crewplane/runtime/execution/provider_call/__init__.py", - "src/crewplane/runtime/execution/provider_call/display.py", - "src/crewplane/runtime/execution/review_loop/audit_round.py", - "src/crewplane/runtime/execution/review_loop/drift.py", - "src/crewplane/runtime/execution/review_loop/drift_detection.py", - "src/crewplane/runtime/execution/review_loop/drift_events.py", - "src/crewplane/runtime/execution/review_loop/executor_round.py", - "src/crewplane/runtime/execution/review_loop/policy.py", - "src/crewplane/runtime/execution/review_loop/prompts.py", - "src/crewplane/runtime/execution/review_loop/reviewer_round.py", - "src/crewplane/runtime/execution/review_loop/rounds.py", - "src/crewplane/runtime/execution/review_loop/state.py", - "src/crewplane/runtime/execution/review_loop/types.py", - "src/crewplane/runtime/execution/review_loop/validation.py", - "src/crewplane/runtime/execution/stage_finalize_events.py", - "src/crewplane/adapters/invokers/mock_invoker/context.py", - "src/crewplane/adapters/invokers/mock_invoker/fixtures.py", - "src/crewplane/adapters/invokers/mock_invoker/invoker.py", - "src/crewplane/adapters/invokers/mock_invoker/logging.py", - "src/crewplane/adapters/invokers/mock_invoker/mutations.py", - "src/crewplane/adapters/invokers/mock_invoker/options.py", - "src/crewplane/adapters/invokers/mock_invoker/outputs.py", - "src/crewplane/adapters/invokers/mock_invoker/selectors.py", -) - -LEGACY_EVENT_FIELDS = { - "node_id", - "provider", - "role", - "model", - "task_id", - "audit_round_num", - "round_num", - "output_file", - "log_file", - "log_presentation_format", - "log_presentation_profile", - "duration_ms", - "error", - "attempt_count", - "cli_captured", - "output_extraction_status", - "provider_usage_status", - "provider_tokens", - "visible_estimate_tokens", - "visible_estimate_method", - "visible_estimate_is_lower_bound", - "configured_cost_usd", - "invocation_cost_confidence", - "usage_parse_error", - "failure_kind", - "failure_phase", - "failure_source", - "failure_advice", - "level", - "message", - "operation", - "attributes", -} - - -def python_files(root: Path) -> tuple[Path, ...]: - return tuple(path for path in root.rglob("*.py") if "__pycache__" not in path.parts) - - -def physical_line_count(path: Path) -> int: - return len(path.read_text(encoding="utf-8").splitlines()) - - -def parse_python(path: Path) -> ast.Module: - return ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) - - -def walk_ast(node: ast.AST) -> Iterator[ast.AST]: - todo: deque[ast.AST] = deque([node]) - while todo: - current = todo.popleft() - yield current - for field in current._fields: - value = getattr(current, field, None) - if isinstance(value, ast.AST): - todo.append(value) - elif isinstance(value, list): - todo.extend(child for child in value if isinstance(child, ast.AST)) - - -def is_single_underscore_name(name: str) -> bool: - return name.startswith("_") and not name.startswith("__") - - -def call_name(node: ast.AST) -> str | None: - if isinstance(node, ast.Name): - return node.id - if isinstance(node, ast.Attribute): - return node.attr - return None - - -def expression_chain(node: ast.AST) -> tuple[str, ...]: - if isinstance(node, ast.Name): - return (node.id,) - if isinstance(node, ast.Attribute): - return (*expression_chain(node.value), node.attr) - return () - - -def module_name_for_path(path: Path) -> str: - if path.is_relative_to(SRC_ROOT): - return ".".join(path.relative_to(SRC_ROOT).with_suffix("").parts) - if path.is_relative_to(TESTS_ROOT): - return ".".join(("tests", *path.relative_to(TESTS_ROOT).with_suffix("").parts)) - raise ValueError(f"Unsupported Python path: {path}") - - -def import_from_module_name(path: Path, node: ast.ImportFrom) -> str: - if node.level == 0: - return node.module or "" - - module_parts = module_name_for_path(path).split(".") - package_parts = module_parts[:-1] - keep_count = max(0, len(package_parts) - node.level + 1) - base_parts = package_parts[:keep_count] - if node.module: - base_parts.extend(node.module.split(".")) - return ".".join(base_parts) - - -def is_repo_module_name(module_name: str) -> bool: - return ( - module_name == "crewplane" - or module_name.startswith("crewplane.") - or module_name == "tests" - or module_name.startswith("tests.") - ) - - -def imported_repo_aliases(path: Path, module: ast.Module) -> set[str]: - aliases: set[str] = set() - for node in walk_ast(module): - if isinstance(node, ast.Import): - for alias in node.names: - if is_repo_module_name(alias.name): - aliases.add(alias.asname or alias.name.split(".")[0]) - elif isinstance(node, ast.ImportFrom): - module_name = import_from_module_name(path, node) - if not is_repo_module_name(module_name): - continue - for alias in node.names: - if alias.name == "*": - continue - imported_name = alias.asname or alias.name - if imported_name[:1].islower(): - aliases.add(imported_name) - return aliases - - -def is_repo_import_expression(node: ast.AST, imported_aliases: set[str]) -> bool: - chain = expression_chain(node) - if not chain: - return False - return chain[0] in imported_aliases or chain[0] in {"crewplane", "tests"} - - -def string_value(node: ast.AST) -> str | None: - if isinstance(node, ast.Constant) and isinstance(node.value, str): - return node.value - return None - - -def has_private_dotted_part(value: str) -> bool: - return any(is_single_underscore_name(part) for part in value.split(".")) - - -def test_ast_walker_does_not_depend_on_mutable_stdlib_walk_helpers( - monkeypatch, -) -> None: - monkeypatch.setattr(ast, "iter_child_nodes", deque()) - module = ast.parse("value = helper(1)\n") - - assert any(isinstance(node, ast.Call) for node in walk_ast(module)) - - -def test_original_oversized_files_are_small_or_deleted() -> None: - offenders = [ - f"{relative_path}: {physical_line_count(REPO_ROOT / relative_path)}" - for relative_path in ORIGINAL_OVERSIZED_FILES - if (REPO_ROOT / relative_path).exists() - and physical_line_count(REPO_ROOT / relative_path) > 200 - ] - assert offenders == [] - - -def test_split_modules_stay_under_line_limit() -> None: - offenders = [ - f"{relative_path}: {physical_line_count(REPO_ROOT / relative_path)}" - for relative_path in SPLIT_MODULES - if (REPO_ROOT / relative_path).exists() - and physical_line_count(REPO_ROOT / relative_path) > 400 - ] - assert offenders == [] - - -def test_all_production_modules_stay_under_line_limit() -> None: - offenders = [ - f"{path.relative_to(REPO_ROOT)}: {physical_line_count(path)}" - for path in python_files(SRC_ROOT / "crewplane") - if physical_line_count(path) > PRODUCTION_LINE_LIMIT - ] - assert offenders == [] - - -def test_adapters_do_not_import_runtime_execution_modules() -> None: - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "adapters"): - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.Import): - for alias in node.names: - if alias.name.startswith("crewplane.runtime.execution"): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - elif ( - isinstance(node, ast.ImportFrom) - and node.module - and node.module.startswith("crewplane.runtime.execution") - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_architecture_ports_do_not_import_runtime_or_observability() -> None: - forbidden_prefixes = ( - "crewplane.core.preflight.runtime_config", - "crewplane.runtime", - "crewplane.observability", - ) - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "architecture" / "ports"): - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.Import): - for alias in node.names: - if alias.name.startswith(forbidden_prefixes): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - elif ( - isinstance(node, ast.ImportFrom) - and node.module - and node.module.startswith(forbidden_prefixes) - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_runtime_modules_do_not_own_module_level_console_singletons() -> None: - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "runtime"): - module = parse_python(path) - for node in module.body: - targets: list[ast.expr] = [] - if isinstance(node, ast.Assign): - targets = list(node.targets) - value = node.value - elif isinstance(node, ast.AnnAssign): - targets = [node.target] - value = node.value - else: - continue - if value is None or not isinstance(value, ast.Call): - continue - if call_name(value.func) != "Console": - continue - offenders.extend( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}" for _ in targets - ) - assert offenders == [] - - -def test_cli_modules_do_not_own_module_level_console_singletons() -> None: - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "cli"): - module = parse_python(path) - for node in module.body: - value: ast.expr | None - if isinstance(node, (ast.Assign, ast.AnnAssign)): - value = node.value - else: - continue - if isinstance(value, ast.Call) and call_name(value.func) == "Console": - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_no_cross_module_single_underscore_imports() -> None: - offenders: list[str] = [] - for root in (SRC_ROOT, TESTS_ROOT): - for path in python_files(root): - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.Import): - for alias in node.names: - parts = alias.name.split(".") - if any(is_single_underscore_name(part) for part in parts): - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}" - ) - elif isinstance(node, ast.ImportFrom): - if node.module and any( - is_single_underscore_name(part) - for part in node.module.split(".") - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - for alias in node.names: - if is_single_underscore_name(alias.name): - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}" - ) - assert offenders == [] - - -def test_no_cross_module_single_underscore_attribute_access() -> None: - offenders: list[str] = [] - for root in (SRC_ROOT, TESTS_ROOT): - for path in python_files(root): - module = parse_python(path) - imported_aliases = imported_repo_aliases(path, module) - for node in walk_ast(module): - if not isinstance(node, ast.Attribute): - continue - if not is_single_underscore_name(node.attr): - continue - if not is_repo_import_expression(node.value, imported_aliases): - continue - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}: {'.'.join(expression_chain(node))}" - ) - assert offenders == [] - - -def test_source_does_not_suppress_unused_arguments() -> None: - offenders = [ - f"{path.relative_to(REPO_ROOT)}" - for path in python_files(SRC_ROOT / "crewplane") - if "noqa: ARG002" in path.read_text(encoding="utf-8") - ] - assert offenders == [] - - -def test_no_private_patch_targets() -> None: - offenders: list[str] = [] - for root in (SRC_ROOT, TESTS_ROOT): - for path in python_files(root): - module = parse_python(path) - imported_aliases = imported_repo_aliases(path, module) - for node in walk_ast(module): - if not isinstance(node, ast.Call): - continue - call_chain = expression_chain(node.func) - if call_chain and call_chain[-1] == "patch" and node.args: - target = string_value(node.args[0]) - if target is not None and has_private_dotted_part(target): - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}: {target}" - ) - if ( - call_chain - and call_chain[-1] == "setattr" - and len(node.args) >= 2 - and is_repo_import_expression(node.args[0], imported_aliases) - ): - target_name = string_value(node.args[1]) - if target_name is not None and is_single_underscore_name( - target_name - ): - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}: {target_name}" - ) - if ( - len(call_chain) >= 2 - and call_chain[-2:] == ("patch", "object") - and len(node.args) >= 2 - and is_repo_import_expression(node.args[0], imported_aliases) - ): - target_name = string_value(node.args[1]) - if target_name is not None and is_single_underscore_name( - target_name - ): - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}: {target_name}" - ) - assert offenders == [] - - -def test_pep561_marker_is_packaged() -> None: - marker = SRC_ROOT / "crewplane" / "py.typed" - pyproject = (REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8") - assert marker.is_file() - assert '"src/crewplane/py.typed"' in pyproject - - -def test_review_contract_has_neutral_imports() -> None: - path = SRC_ROOT / "crewplane" / "core" / "review_contract.py" - forbidden_prefixes = ( - "crewplane.runtime", - "crewplane.adapters", - "crewplane.artifacts", - "crewplane.observability", - ) - offenders: list[str] = [] - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.Import): - for alias in node.names: - if alias.name.startswith(forbidden_prefixes): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - elif ( - isinstance(node, ast.ImportFrom) - and node.module - and node.module.startswith(forbidden_prefixes) - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_execution_events_do_not_use_legacy_flat_fields() -> None: - offenders: list[str] = [] - for root in (SRC_ROOT, TESTS_ROOT): - for path in python_files(root): - module = parse_python(path) - for node in walk_ast(module): - if ( - not isinstance(node, ast.Call) - or call_name(node.func) != "ExecutionEvent" - ): - continue - legacy_keywords = { - keyword.arg - for keyword in node.keywords - if keyword.arg in LEGACY_EVENT_FIELDS - } - if legacy_keywords: - offenders.append( - f"{path.relative_to(REPO_ROOT)}:{node.lineno}: {sorted(legacy_keywords)}" - ) - assert offenders == [] - - -def test_execution_event_has_no_legacy_flat_accessors() -> None: - path = SRC_ROOT / "crewplane" / "observability" / "events" / "execution_event.py" - module = parse_python(path) - legacy_properties: list[str] = [] - for node in walk_ast(module): - if ( - not isinstance(node, ast.FunctionDef) - or node.name not in LEGACY_EVENT_FIELDS - ): - continue - if any(call_name(decorator) == "property" for decorator in node.decorator_list): - legacy_properties.append(node.name) - assert legacy_properties == [] - - -def test_runtime_code_uses_event_builders_instead_of_direct_event_construction() -> ( - None -): - offenders: list[str] = [] - runtime_root = SRC_ROOT / "crewplane" / "runtime" - for path in python_files(runtime_root): - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.Call) and call_name(node.func) == "ExecutionEvent": - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_runtime_does_not_infer_provider_behavior_from_executable_names() -> None: - forbidden_names = { - "AUTO_QUOTA_PARSER_PROVIDER_BY_EXECUTABLE", - "parser_resolution", - } - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "runtime" / "agent"): - source = path.read_text(encoding="utf-8") - for forbidden_name in forbidden_names: - if forbidden_name in source: - offenders.append(f"{path.relative_to(REPO_ROOT)}: {forbidden_name}") - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.ImportFrom) and node.module == "os.path": - imported_names = {alias.name for alias in node.names} - if "basename" in imported_names: - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - if ( - isinstance(node, ast.Attribute) - and node.attr == "basename" - and expression_chain(node.value) in {("os", "path"), ("posixpath",)} - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_log_presentation_does_not_import_runtime_output_extractors() -> None: - forbidden_modules = { - "crewplane.runtime.agent.invocation.output", - "crewplane.runtime.agent.invocation.claude_json", - "crewplane.runtime.agent.usage_parsing", - "crewplane.runtime.agent.quota", - "crewplane.runtime.agent.failures", - } - offenders: list[str] = [] - presentation_root = SRC_ROOT / "crewplane" / "observability" / "log_presentation" - for path in python_files(presentation_root): - module = parse_python(path) - for node in walk_ast(module): - if isinstance(node, ast.ImportFrom): - module_name = import_from_module_name(path, node) - if any( - module_name == forbidden or module_name.startswith(f"{forbidden}.") - for forbidden in forbidden_modules - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - if isinstance(node, ast.Import): - for alias in node.names: - if any( - alias.name == forbidden - or alias.name.startswith(f"{forbidden}.") - for forbidden in forbidden_modules - ): - offenders.append(f"{path.relative_to(REPO_ROOT)}:{node.lineno}") - assert offenders == [] - - -def test_runtime_and_tmux_do_not_infer_log_presentation_from_provider_names() -> None: - provider_literals = {"claude", "codex", "copilot", "gemini", "kilo"} - checked_roots = [ - SRC_ROOT / "crewplane" / "runtime" / "execution", - SRC_ROOT / "crewplane" / "observability" / "tmux", - ] - offenders: list[str] = [] - for root in checked_roots: - for path in python_files(root): - source = path.read_text(encoding="utf-8") - for provider_literal in provider_literals: - if ( - f'"{provider_literal}"' in source - or f"'{provider_literal}'" in source - ): - offenders.append( - f"{path.relative_to(REPO_ROOT)}: {provider_literal}" - ) - assert offenders == [] - - -def test_process_stream_capture_uses_persisted_files_and_bounded_tails() -> None: - streams_path = ( - SRC_ROOT / "crewplane" / "runtime" / "agent" / "process" / "streams.py" - ) - capture_path = ( - SRC_ROOT / "crewplane" / "runtime" / "agent" / "process" / "stream_capture.py" - ) - streams_source = streams_path.read_text(encoding="utf-8") - capture_source = capture_path.read_text(encoding="utf-8") - assert "asyncio.Queue(maxsize=LOG_QUEUE_MAX_ITEMS)" in streams_source - assert "stdout_chunks" not in streams_source - assert "stderr_chunks" not in streams_source - assert "tempfile.mkstemp" in capture_source - assert "MAX_CAPTURE_MEMORY_BYTES" in capture_source - assert "tail_bytes" in capture_source - assert "SpooledTemporaryFile" not in streams_source - assert "SpooledTemporaryFile" not in capture_source - - -def test_provider_invocation_fallback_usage_does_not_materialize_artifact() -> None: - path = ( - SRC_ROOT - / "crewplane" - / "runtime" - / "execution" - / "provider_call" - / "__init__.py" - ) - events_path = ( - SRC_ROOT / "crewplane" / "runtime" / "execution" / "provider_call" / "events.py" - ) - source = path.read_text(encoding="utf-8") - events_source = events_path.read_text(encoding="utf-8") - assert ".read_text(" not in source - assert ".read_text(" not in events_source - assert "build_fallback_usage_from_output_file" in source + events_source - - -def test_persistent_run_logger_retains_bounded_event_window() -> None: - path = SRC_ROOT / "crewplane" / "observability" / "run_summary" / "logger.py" - source = path.read_text(encoding="utf-8") - assert "MAX_RETAINED_SUMMARY_EVENTS" in source - assert "deque(maxlen=MAX_RETAINED_SUMMARY_EVENTS)" in source - assert "list[ExecutionEvent]" not in source - - -def test_run_summary_retains_bounded_invocation_usage_details() -> None: - accumulator_path = ( - SRC_ROOT / "crewplane" / "observability" / "run_summary" / "accumulator.py" - ) - markdown_path = ( - SRC_ROOT / "crewplane" / "observability" / "run_summary" / "markdown.py" - ) - accumulator_source = accumulator_path.read_text(encoding="utf-8") - markdown_source = markdown_path.read_text(encoding="utf-8") - assert "MAX_RETAINED_INVOCATION_USAGE_DETAILS" in accumulator_source - assert "deque[InvocationUsageSummary]" in accumulator_source - assert "maxlen=MAX_RETAINED_INVOCATION_USAGE_DETAILS" in accumulator_source - assert "UsageRollupAccumulator" in accumulator_source - assert "list[InvocationUsageSummary]" not in accumulator_source - assert "omitted_invocation_usage_count" in markdown_source - assert "Full invocation events remain" in markdown_source - - -def test_docs_and_templates_do_not_reference_legacy_prompt_config_fields() -> None: - legacy_terms = { - "prompt_arg", - "quota_parser", - "stdin_prompt_arg", - "use_stdin", - } - checked_paths = [ - REPO_ROOT / "README.md", - REPO_ROOT / "docs" / "architecture" / "modular-orchestration-architecture.md", - SRC_ROOT / "crewplane" / "example_templates" / "config.yml", - ] - offenders: list[str] = [] - for path in checked_paths: - source = path.read_text(encoding="utf-8") - for term in legacy_terms: - if term in source: - offenders.append(f"{path.relative_to(REPO_ROOT)}: {term}") - assert offenders == [] - - -def test_version_catalog_has_single_public_python_source() -> None: - stale_paths = [ - SRC_ROOT / "crewplane" / "versions.py", - SRC_ROOT / "crewplane" / "core" / "versions.py", - SRC_ROOT / "crewplane" / "architecture" / "api_version.py", - ] - assert [ - path.relative_to(REPO_ROOT).as_posix() for path in stale_paths if path.exists() - ] == [] - - offenders: list[str] = [] - stale_imports = { - "crewplane.architecture.api_version", - "crewplane.core.versions", - "crewplane.versions", - } - for path in python_files(SRC_ROOT / "crewplane"): - source = path.read_text(encoding="utf-8") - for stale_import in stale_imports: - if stale_import in source: - offenders.append(f"{path.relative_to(REPO_ROOT)}: {stale_import}") - assert offenders == [] - - -def test_public_package_exports_are_narrow() -> None: - import crewplane.core as core_package - import crewplane.runtime as runtime_package - - assert core_package.__all__ == ["SCHEMA_VERSION"] - assert runtime_package.__all__ == [] - assert "__getattr__" not in vars(runtime_package) - - -def test_output_manager_directory_fields_are_read_only_properties() -> None: - path = SRC_ROOT / "crewplane" / "artifacts" / "manager.py" - module = parse_python(path) - managed_fields = { - "base_dir", - "log_cli_output", - "logs_dir", - "results_dir", - "run_id", - "stages_dir", - "task_name", - } - assigned_fields: list[str] = [] - property_fields: set[str] = set() - for node in module.body: - if not isinstance(node, ast.ClassDef) or node.name != "OutputManager": - continue - for member in node.body: - if ( - isinstance(member, ast.FunctionDef) - and member.name in managed_fields - and any( - call_name(decorator) == "property" - for decorator in member.decorator_list - ) - ): - property_fields.add(member.name) - if not isinstance(member, ast.FunctionDef) or member.name != "__init__": - continue - for child in walk_ast(member): - targets: list[ast.expr] = [] - if isinstance(child, ast.Assign): - targets = list(child.targets) - elif isinstance(child, ast.AnnAssign): - targets = [child.target] - for target in targets: - if ( - isinstance(target, ast.Attribute) - and isinstance(target.value, ast.Name) - and target.value.id == "self" - and target.attr in managed_fields - ): - assigned_fields.append(target.attr) - assert assigned_fields == [] - assert property_fields == managed_fields - - -def test_preflight_fingerprint_key_state_is_scoped() -> None: - secrets_path = SRC_ROOT / "crewplane" / "core" / "preflight" / "secrets.py" - compile_state_path = ( - SRC_ROOT / "crewplane" / "core" / "preflight" / "compile_state.py" - ) - secrets_source = secrets_path.read_text(encoding="utf-8") - compile_state_source = compile_state_path.read_text(encoding="utf-8") - assert "_EPHEMERAL_KEYS" not in secrets_source - assert "class FingerprintKeyCache" in secrets_source - assert "fingerprint_key_cache: FingerprintKeyCache" in compile_state_source - - -def test_boundary_option_contracts_use_json_object() -> None: - checked_paths = [ - SRC_ROOT / "crewplane" / "architecture", - SRC_ROOT / "crewplane" / "bootstrap", - SRC_ROOT / "crewplane" / "adapters", - ] - offenders = [ - f"{path.relative_to(REPO_ROOT)}" - for root in checked_paths - for path in python_files(root) - if "dict[str, Any]" in path.read_text(encoding="utf-8") - or "dict[str, object]" in path.read_text(encoding="utf-8") - ] - assert offenders == [] - - -def test_preflight_any_maps_are_limited_to_redaction_traversal() -> None: - allowed_path = ( - SRC_ROOT - / "crewplane" - / "core" - / "preflight" - / "runtime_config" - / "redaction.py" - ) - offenders: list[str] = [] - for path in python_files(SRC_ROOT / "crewplane" / "core" / "preflight"): - source = path.read_text(encoding="utf-8") - if "dict[str, Any]" not in source: - continue - if path != allowed_path: - offenders.append(f"{path.relative_to(REPO_ROOT)}") - continue - module = parse_python(path) - docstring = ast.get_docstring(module) or "" - if "arbitrary JSON-compatible config snapshots" not in docstring: - offenders.append(f"{path.relative_to(REPO_ROOT)}: missing rationale") - assert offenders == [] - - -def test_review_loop_status_error_is_public() -> None: - from crewplane.artifacts.results.review_loop_status import ( - ReviewLoopStatusError, - ) - - assert ReviewLoopStatusError.__name__ == "ReviewLoopStatusError" - - -def test_provider_call_request_does_not_carry_display_state() -> None: - from dataclasses import fields - - from crewplane.runtime.execution.provider_call import ( - ProviderCallRequest, - ) - - request_fields = {field.name for field in fields(ProviderCallRequest)} - assert "progress_description" not in request_fields - assert "show_console_summary" not in request_fields diff --git a/tests/integration/architecture/test_runtime_ownership_boundaries.py b/tests/integration/architecture/test_runtime_ownership_boundaries.py new file mode 100644 index 0000000..6deb893 --- /dev/null +++ b/tests/integration/architecture/test_runtime_ownership_boundaries.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +import ast + +from tests.integration.architecture.static_checks import ( + SRC_ROOT, + call_name, + expression_chain, + offender, + parse_python, + python_files, + walk_ast, +) + + +def test_runtime_and_cli_do_not_own_module_level_console_singletons() -> None: + offenders: list[str] = [] + checked_roots = ( + SRC_ROOT / "crewplane" / "runtime", + SRC_ROOT / "crewplane" / "cli", + ) + for root in checked_roots: + for path in python_files(root): + for node in parse_python(path).body: + value: ast.expr | None + if isinstance(node, (ast.Assign, ast.AnnAssign)): + value = node.value + else: + continue + if isinstance(value, ast.Call) and call_name(value.func) == "Console": + offenders.append(offender(path, node.lineno)) + assert offenders == [] + + +def test_runtime_code_uses_event_builders_instead_of_direct_construction() -> None: + offenders: list[str] = [] + for path in python_files(SRC_ROOT / "crewplane" / "runtime"): + for node in walk_ast(parse_python(path)): + if isinstance(node, ast.Call) and call_name(node.func) == "ExecutionEvent": + offenders.append(offender(path, node.lineno)) + assert offenders == [] + + +def test_runtime_does_not_infer_provider_behavior_from_executable_names() -> None: + forbidden_names = { + "AUTO_QUOTA_PARSER_PROVIDER_BY_EXECUTABLE", + "parser_resolution", + } + offenders: list[str] = [] + for path in python_files(SRC_ROOT / "crewplane" / "runtime" / "agent"): + source = path.read_text(encoding="utf-8") + for line_number, line in enumerate(source.splitlines(), start=1): + offenders.extend( + offender(path, line_number, forbidden_name) + for forbidden_name in forbidden_names + if forbidden_name in line + ) + module = parse_python(path) + for node in walk_ast(module): + if isinstance(node, ast.ImportFrom) and node.module == "os.path": + for alias in node.names: + if alias.name == "basename": + offenders.append( + offender(path, node.lineno, "os.path.basename") + ) + if ( + isinstance(node, ast.Attribute) + and node.attr == "basename" + and expression_chain(node.value) in {("os", "path"), ("posixpath",)} + ): + offenders.append(offender(path, node.lineno, "basename")) + assert offenders == [] + + +def test_runtime_and_tmux_do_not_infer_presentation_from_provider_names() -> None: + provider_literals = {"claude", "codex", "copilot", "gemini", "kilo"} + checked_roots = ( + SRC_ROOT / "crewplane" / "runtime" / "execution", + SRC_ROOT / "crewplane" / "observability" / "tmux", + ) + offenders: list[str] = [] + for root in checked_roots: + for path in python_files(root): + for node in walk_ast(parse_python(path)): + if ( + isinstance(node, ast.Constant) + and isinstance(node.value, str) + and node.value in provider_literals + ): + offenders.append(offender(path, node.lineno, node.value)) + assert offenders == [] + + +def test_provider_usage_fallback_does_not_materialize_output_artifacts() -> None: + checked_paths = ( + SRC_ROOT + / "crewplane" + / "runtime" + / "execution" + / "provider_call" + / "__init__.py", + SRC_ROOT + / "crewplane" + / "runtime" + / "execution" + / "provider_call" + / "events.py", + ) + fallback_calls = 0 + offenders: list[str] = [] + for path in checked_paths: + for node in walk_ast(parse_python(path)): + if not isinstance(node, ast.Call): + continue + name = call_name(node.func) + if name == "read_text": + offenders.append(offender(path, node.lineno, "read_text")) + if name == "build_fallback_usage_from_output_file": + fallback_calls += 1 + assert offenders == [] + assert fallback_calls > 0 diff --git a/tests/integration/architecture/test_source_size_and_hygiene.py b/tests/integration/architecture/test_source_size_and_hygiene.py new file mode 100644 index 0000000..1236329 --- /dev/null +++ b/tests/integration/architecture/test_source_size_and_hygiene.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +import ast +from collections import deque +from pathlib import Path + +import pytest + +from tests.integration.architecture.static_checks import ( + REPO_ROOT, + SRC_ROOT, + ForbiddenTextRule, + find_forbidden_text, + physical_line_count, + python_files, + text_rule_files, + walk_ast, +) + +PRODUCTION_LINE_LIMIT = 500 + + +def test_ast_walker_does_not_depend_on_mutable_stdlib_walk_helpers( + monkeypatch, +) -> None: + monkeypatch.setattr(ast, "iter_child_nodes", deque()) + module = ast.parse("value = helper(1)\n") + + assert any(isinstance(node, ast.Call) for node in walk_ast(module)) + + +def test_text_rule_files_rejects_missing_paths(tmp_path: Path) -> None: + missing_path = tmp_path / "required.md" + + with pytest.raises(FileNotFoundError, match="Text rule path does not exist"): + text_rule_files((missing_path,)) + + +def test_all_production_modules_stay_under_line_limit() -> None: + offenders = [ + f"{path.relative_to(REPO_ROOT)}: {physical_line_count(path)}" + for path in python_files(SRC_ROOT / "crewplane") + if physical_line_count(path) > PRODUCTION_LINE_LIMIT + ] + assert offenders == [] + + +def test_source_does_not_suppress_unused_arguments() -> None: + rule = ForbiddenTextRule( + name="production source does not suppress unused arguments", + paths=python_files(SRC_ROOT / "crewplane"), + forbidden_terms=frozenset({"noqa: ARG002"}), + ) + assert find_forbidden_text(rule) == [] diff --git a/tests/integration/cli/test_run_preflight.py b/tests/integration/cli/test_run_preflight.py index 231093d..257710d 100644 --- a/tests/integration/cli/test_run_preflight.py +++ b/tests/integration/cli/test_run_preflight.py @@ -3,6 +3,7 @@ from rich.console import Console from crewplane.bootstrap import build_runtime_config_snapshot +from crewplane.cli.run.preflight import run_reasoning_control_errors from crewplane.core.config import ( AgentConfig, Config, @@ -288,6 +289,110 @@ def test_successful_preflight_preview_builds_execution_contract( assert preview.nodes[0].provider_records[0].agent_config_key == "alpha" +def test_preflight_records_reasoning_and_changes_execution_identity( + tmp_path: Path, +) -> None: + base_node = WorkflowNode( + id="build", + mode="sequential", + providers=[ProviderSpec(provider="alpha")], + prompt_segments=[PromptSegment(role=PromptSegmentRole.SHARED, content="Build")], + ) + default_preview = _preview( + WorkflowPlan(name="demo", nodes=[base_node]), + tmp_path, + ) + reasoning_preview = _preview( + WorkflowPlan( + name="demo", + nodes=[ + base_node.model_copy( + update={ + "providers": [ProviderSpec(provider="alpha", reasoning="high")] + } + ) + ], + ), + tmp_path, + ) + + provider = reasoning_preview.nodes[0].provider_records[0] + assert provider.requested_reasoning == "high" + assert default_preview.nodes[0].provider_records[0].requested_reasoning is None + assert reasoning_preview.workflow_signature != default_preview.workflow_signature + + +def test_reasoning_control_rejects_non_cli_invoker() -> None: + workflow = WorkflowPlan( + name="demo", + nodes=[ + WorkflowNode( + id="build", + mode="sequential", + providers=[ProviderSpec(provider="alpha", reasoning="high")], + prompt_segments=[ + PromptSegment(role=PromptSegmentRole.SHARED, content="Build") + ], + ) + ], + ) + + errors = run_reasoning_control_errors(workflow, _config()) + + assert errors == ( + "workflow 'demo' -> node 'build' -> provider 'alpha': first-class " + "reasoning requires the built-in CLI invoker.", + ) + + +def test_reasoning_control_checks_relative_claude_settings_file( + tmp_path: Path, +) -> None: + (tmp_path / "claude-settings.json").write_text( + '{"effortLevel": "low"}', + encoding="utf-8", + ) + workflow = WorkflowPlan( + name="demo", + nodes=[ + WorkflowNode( + id="build", + mode="sequential", + providers=[ProviderSpec(provider="alpha", reasoning="high")], + prompt_segments=[ + PromptSegment(role=PromptSegmentRole.SHARED, content="Build") + ], + ) + ], + ) + config = Config( + version=SCHEMA_VERSION, + agents={ + "alpha": AgentConfig( + cli_cmd=["claude", "--settings", "claude-settings.json"], + provider_kind="claude", + ) + }, + settings=Settings( + integrations=IntegrationsConfig( + invoker=IntegrationSpec(implementation="cli", options={}), + artifacts=IntegrationSpec( + implementation="filesystem", + options={"allowed_template_paths": [], "log_cli_output": True}, + ), + ui=IntegrationSpec(implementation="none", options={}), + ) + ), + ) + + errors = run_reasoning_control_errors(workflow, config, tmp_path) + + assert errors == ( + "workflow 'demo' -> node 'build' -> provider 'alpha': --settings " + "effortLevel conflicts with the workflow reasoning request.", + ) + + def test_preflight_preview_warns_on_argv_prompt_transport( tmp_path: Path, ) -> None: diff --git a/tests/integration/cli/test_workflow_runner.py b/tests/integration/cli/test_workflow_runner.py index d53b6f3..ac9ee7e 100644 --- a/tests/integration/cli/test_workflow_runner.py +++ b/tests/integration/cli/test_workflow_runner.py @@ -10,6 +10,7 @@ from collections.abc import Callable, Mapping from pathlib import Path from typing import Any +from unittest.mock import AsyncMock, patch import typer from rich.console import Console @@ -379,6 +380,54 @@ async def test_non_filesystem_artifact_real_run_fails_before_run_allocation( self.assertEqual(_result_dirs(root), []) self.assertFalse((root / ".crewplane" / "locks").exists()) + async def test_reasoning_validation_stops_real_run_before_execution_setup( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + root = Path(tmp_dir) + stream = io.StringIO() + console = Console(file=stream, force_terminal=False, color_system=None) + workflow = _workflow() + workflow.nodes[0] = workflow.nodes[0].model_copy( + update={"providers": [ProviderSpec(provider="alpha", reasoning="high")]} + ) + execute_workflow_mock = AsyncMock() + original_cwd = Path.cwd() + os.chdir(root) + try: + with ( + patch( + "crewplane.cli.run.execution.acquire_same_context_lock", + side_effect=AssertionError( + "reasoning validation reached locking" + ), + ) as acquire_lock, + patch( + "crewplane.cli.run.execution.allocate_run_output", + side_effect=AssertionError( + "reasoning validation reached allocation" + ), + ) as allocate_output, + self.assertRaises(typer.Exit) as raised, + ): + await _run_workflow( + workflow, + _mock_config(), + console, + execute_workflow_impl=execute_workflow_mock, + ) + finally: + os.chdir(original_cwd) + + self.assertEqual(raised.exception.exit_code, 1) + self.assertIn( + "first-class reasoning requires the built-in CLI invoker", + stream.getvalue(), + ) + acquire_lock.assert_not_called() + allocate_output.assert_not_called() + execute_workflow_mock.assert_not_called() + async def test_force_ignores_duplicate_signature(self) -> None: with tempfile.TemporaryDirectory() as tmp_dir: root = Path(tmp_dir) diff --git a/tests/integration/observability/runtime/test_compact_runtime_log_tail.py b/tests/integration/observability/runtime/test_compact_runtime_log_tail.py index 68a363e..c79e281 100644 --- a/tests/integration/observability/runtime/test_compact_runtime_log_tail.py +++ b/tests/integration/observability/runtime/test_compact_runtime_log_tail.py @@ -302,3 +302,23 @@ def test_read_log_tail_bounded_returns_last_body_lines_only(self) -> None: tail_lines = read_log_tail(log_path, 10005) self.assertEqual(tail_lines[0], "body-line-0") self.assertEqual(len(tail_lines), 10000) + + def test_read_log_tail_strips_optional_reasoning_header(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + log_path = Path(tmp_dir) / "reasoning.log" + log_path.write_text( + "\n".join( + [ + "started_at: 2026-04-10T12:00:00", + "cli_executable: /usr/bin/echo", + "model: mock-model", + "requested_reasoning: xhigh", + "output_file: output.md", + "---", + "body-line", + ] + ), + encoding="utf-8", + ) + + self.assertEqual(read_log_tail(log_path, 10), ["body-line"]) diff --git a/tests/integration/runtime/agent/invoker/test_invocation_failure_reporting.py b/tests/integration/runtime/agent/invoker/test_invocation_failure_reporting.py index 19d05b8..6b7a870 100644 --- a/tests/integration/runtime/agent/invoker/test_invocation_failure_reporting.py +++ b/tests/integration/runtime/agent/invoker/test_invocation_failure_reporting.py @@ -250,6 +250,34 @@ def test_classifies_provider_failure_categories(self) -> None: "provider_session", "stderr_text", ), + ( + "codex", + CommandResult( + returncode=1, + stdout_text="", + stderr_text=( + '{"type":"error","error":{"type":"invalid_request_error",' + '"message":"Invalid value for reasoning.effort"}}' + ), + ), + "model_or_config_error", + "provider_config", + "stderr_json", + ), + ( + "codex", + CommandResult( + returncode=1, + stdout_text="", + stderr_text=( + "invalid_request_error: Tool call failed: " + "invalid value for path" + ), + ), + "provider_tool_error", + "provider_tool", + "stderr_text", + ), ( "gemini", CommandResult( diff --git a/tests/integration/runtime/agent/invoker/test_quota_parsing.py b/tests/integration/runtime/agent/invoker/test_quota_parsing.py index bdf3861..cdc8bb9 100644 --- a/tests/integration/runtime/agent/invoker/test_quota_parsing.py +++ b/tests/integration/runtime/agent/invoker/test_quota_parsing.py @@ -185,6 +185,18 @@ def test_parses_claude_reset_at_local_time_with_timezone(self) -> None: self.assertTrue(waits) self.assertAlmostEqual(max(waits), 3600.0, delta=1.0) + def test_does_not_treat_timezone_qualified_reset_as_host_local(self) -> None: + now_utc = datetime(2026, 7, 10, 17, 0, 0, tzinfo=UTC) + lines = ( + "Usage limit reached; try again at 2 PM UTC.", + "Usage limit reached; try again at 2 PM EDT.", + "Usage limit reached; try again at 2 PM, (America/New_York).", + ) + + for line in lines: + with self.subTest(line=line): + self.assertEqual(extract_wait_candidates_from_line(line, now_utc), []) + def test_parses_epoch_pipe_reset_hint(self) -> None: now_utc = datetime(2026, 4, 10, 0, 0, 0, tzinfo=UTC) line = "quota reached |1893456000" diff --git a/tests/integration/runtime/agent/test_invocation_loop.py b/tests/integration/runtime/agent/test_invocation_loop.py index 6cb1ae9..2682915 100644 --- a/tests/integration/runtime/agent/test_invocation_loop.py +++ b/tests/integration/runtime/agent/test_invocation_loop.py @@ -748,6 +748,50 @@ async def runner( self.assertEqual(len(usages), 1) + async def test_quota_wait_ceiling_counts_only_completed_quota_sleeps( + self, + ) -> None: + quota_result = CommandResult( + returncode=1, + stdout_text="You have exhausted your capacity on this model.", + stderr_text="", + ) + runner = AsyncMock(side_effect=[quota_result, quota_result, quota_result]) + sleep = AsyncMock() + + with ( + tempfile.TemporaryDirectory() as tmp_dir, + patch("crewplane.runtime.agent.invocation.loop.asyncio.sleep", sleep), + patch( + "crewplane.runtime.agent.invocation.retry.quota_retry_elapsed_seconds", + return_value=20, + ), + self.assertRaises(InvocationFailureError) as caught, + ): + await invoke_agent_with_runner( + config=AgentConfig( + cli_cmd=["gemini"], + provider_kind="gemini", + default_model="test", + model_arg=None, + quota_reached_retry_delay_seconds=10, + quota_retry_max_wait_seconds=25, + ), + model="test", + prompt="prompt", + output_file=Path(tmp_dir) / "output.txt", + cwd=Path(tmp_dir), + log_file=None, + invocation_context=None, + command_runner=runner, + plan_builder=build_cli_invocation_plan, + ) + + self.assertEqual(runner.await_count, 3) + self.assertEqual(sleep.await_count, 2) + self.assertEqual([item.args[0] for item in sleep.await_args_list], [10, 10]) + self.assertIn("cumulative quota wait is 20s", str(caught.exception)) + async def test_quota_failure_records_usage_once_before_reraising(self) -> None: quota_message = ( "You have exhausted your capacity on this model. " @@ -808,3 +852,128 @@ async def runner( + estimate_token_count(len(quota_message)), ) self.assertFalse(output_file.exists()) + + async def test_quota_failure_preserves_last_non_quota_failure(self) -> None: + results = [ + CommandResult(returncode=1, stdout_text="", stderr_text="fatal transport"), + CommandResult( + returncode=0, + stdout_text="partial success", + stderr_text="HTTP 429 Too Many Requests", + ), + CommandResult( + returncode=1, + stdout_text="Quota reached. Your quota will reset after 6h.", + stderr_text="", + ), + ] + + async def runner( + cmd: list[str], # noqa: ARG001 + stdin_data: bytes | None, # noqa: ARG001 + log_file: Path | None, # noqa: ARG001 + append_log: bool, # noqa: ARG001 + log_header: bytes | None, # noqa: ARG001 + cwd: Path, # noqa: ARG001 + invocation_context: InvocationContext | None, # noqa: ARG001 + idle_timeout_seconds: float | None, # noqa: ARG001 + child_environment: ChildProcessEnvironment | None = None, # noqa: ARG001 + ) -> CommandResult: + return results.pop(0) + + with ( + tempfile.TemporaryDirectory() as tmp_dir, + self.assertRaises(InvocationFailureError) as caught, + ): + await invoke_agent_with_runner( + config=AgentConfig( + cli_cmd=["provider"], + max_retries=2, + retry_delay_seconds=0, + retry_on_exit_codes=[1], + retry_on_stderr_contains=["HTTP 429"], + quota_reached_retry_delay_seconds=0, + ), + model=None, + prompt="prompt", + output_file=Path(tmp_dir) / "output.txt", + cwd=Path(tmp_dir), + log_file=None, + invocation_context=None, + command_runner=runner, + plan_builder=build_cli_invocation_plan, + ) + + last_failure = caught.exception.last_non_quota_failure + self.assertIsNotNone(last_failure) + assert last_failure is not None + self.assertIn("fatal transport", last_failure.message) + self.assertIn( + "last distinct non-quota failure: fatal transport", str(caught.exception) + ) + self.assertTrue( + any( + "Last distinct non-quota failure" in note + for note in caught.exception.__notes__ + ) + ) + + async def test_quota_classified_retry_is_not_saved_as_non_quota_failure( + self, + ) -> None: + results = [ + CommandResult( + returncode=0, + stdout_text="partial success", + stderr_text="HTTP 429 Too Many Requests", + ), + CommandResult( + returncode=1, + stdout_text="Quota reached. Your quota will reset after 6h.", + stderr_text="", + ), + ] + + async def runner( + cmd: list[str], # noqa: ARG001 + stdin_data: bytes | None, # noqa: ARG001 + log_file: Path | None, # noqa: ARG001 + append_log: bool, # noqa: ARG001 + log_header: bytes | None, # noqa: ARG001 + cwd: Path, # noqa: ARG001 + invocation_context: InvocationContext | None, # noqa: ARG001 + idle_timeout_seconds: float | None, # noqa: ARG001 + child_environment: ChildProcessEnvironment | None = None, # noqa: ARG001 + ) -> CommandResult: + return results.pop(0) + + with ( + tempfile.TemporaryDirectory() as tmp_dir, + self.assertRaises(InvocationFailureError) as caught, + ): + await invoke_agent_with_runner( + config=AgentConfig( + cli_cmd=["provider"], + max_retries=1, + retry_delay_seconds=0, + retry_on_stderr_contains=["HTTP 429"], + quota_reached_retry_delay_seconds=0, + ), + model=None, + prompt="prompt", + output_file=Path(tmp_dir) / "output.txt", + cwd=Path(tmp_dir), + log_file=None, + invocation_context=None, + command_runner=runner, + plan_builder=build_cli_invocation_plan, + ) + + self.assertIsNone(caught.exception.last_non_quota_failure) + self.assertNotIn("last distinct non-quota failure", str(caught.exception)) + self.assertFalse( + any( + "Last distinct non-quota failure" in note + for note in getattr(caught.exception, "__notes__", ()) + ) + ) diff --git a/tests/integration/runtime/agent/test_invocation_retry.py b/tests/integration/runtime/agent/test_invocation_retry.py index 9478ea8..928ed06 100644 --- a/tests/integration/runtime/agent/test_invocation_retry.py +++ b/tests/integration/runtime/agent/test_invocation_retry.py @@ -137,6 +137,104 @@ def test_evaluate_quota_retry_returns_failure_when_reset_exceeds_guard() -> None assert "exceeds 5 hours" in decision.message +def test_evaluate_quota_retry_enforces_configured_attempt_ceiling() -> None: + decision = evaluate_quota_retry( + config=AgentConfig( + cli_cmd=["gemini"], + provider_kind="gemini", + quota_retry_max_attempts=2, + ), + cmd=["gemini"], + quota_parser="gemini", + result=CommandResult( + returncode=1, + stdout_text="You have exhausted your capacity on this model.", + stderr_text="", + ), + quota_retry_started_at=None, + quota_retry_count=2, + ) + + assert isinstance(decision, QuotaRetryFailure) + assert "configured attempt ceiling of 2" in decision.message + + +def test_evaluate_quota_retry_enforces_configured_wait_ceiling() -> None: + decision = evaluate_quota_retry( + config=AgentConfig( + cli_cmd=["gemini"], + provider_kind="gemini", + quota_reached_retry_delay_seconds=120, + quota_retry_max_wait_seconds=60, + ), + cmd=["gemini"], + quota_parser="gemini", + result=CommandResult( + returncode=1, + stdout_text="You have exhausted your capacity on this model.", + stderr_text="", + ), + quota_retry_started_at=None, + quota_retry_count=0, + ) + + assert isinstance(decision, QuotaRetryFailure) + assert "configured wait ceiling of 1m" in decision.message + + +def test_quota_wait_ceiling_excludes_provider_execution_time() -> None: + with patch( + "crewplane.runtime.agent.invocation.retry.time.monotonic", + return_value=20, + ): + decision = evaluate_quota_retry( + config=AgentConfig( + cli_cmd=["gemini"], + provider_kind="gemini", + quota_reached_retry_delay_seconds=10, + quota_retry_max_wait_seconds=25, + ), + cmd=["gemini"], + quota_parser="gemini", + result=CommandResult( + returncode=1, + stdout_text="You have exhausted your capacity on this model.", + stderr_text="", + ), + quota_retry_started_at=0, + quota_retry_count=1, + quota_retry_wait_seconds=10, + ) + + assert isinstance(decision, ScheduleQuotaRetry) + + +def test_bare_local_reset_uses_configured_delay_in_utc_timezone() -> None: + with local_timezone("UTC"): + decision = evaluate_quota_retry( + config=AgentConfig( + cli_cmd=["gemini"], + provider_kind="gemini", + quota_reached_retry_delay_seconds=73, + ), + cmd=["gemini"], + quota_parser="gemini", + result=CommandResult( + returncode=1, + stdout_text=( + "You have exhausted your capacity on this model. Try again at 2 PM." + ), + stderr_text="", + ), + quota_retry_started_at=None, + quota_retry_count=0, + ) + + assert isinstance(decision, ScheduleQuotaRetry) + assert decision.wait_seconds == 73 + assert "configured fixed delay" in decision.notice.message + + def test_evaluate_failure_retry_reads_retried_output_from_persisted_stream() -> None: with tempfile.TemporaryDirectory() as tmp_dir: path = Path(tmp_dir) / "stream.txt" diff --git a/tests/integration/runtime/agent/test_invoker_facade.py b/tests/integration/runtime/agent/test_invoker_facade.py index d6b7551..10f25a2 100644 --- a/tests/integration/runtime/agent/test_invoker_facade.py +++ b/tests/integration/runtime/agent/test_invoker_facade.py @@ -1,3 +1,4 @@ +import os import tempfile import unittest from pathlib import Path @@ -97,3 +98,47 @@ async def test_planned_agent_invoker_delegates_to_invoke_agent(self) -> None: delegated.assert_awaited_once() assert delegated.await_args.args[4] == Path(tmp_dir) assert delegated.await_args.kwargs["invocation_context"] is context + + async def test_planned_invoker_validates_settings_from_runtime_cwd(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + working_directory = Path(tmp_dir) + output_file = working_directory / "output.txt" + (working_directory / "claude-settings.json").write_text( + '{"effortLevel": "low"}', + encoding="utf-8", + ) + config = AgentConfig( + cli_cmd=["claude", "--settings", "claude-settings.json"], + provider_kind="claude", + ) + context = InvocationContext( + node_id="node.a", + task_id="claude_executor_0", + provider="claude", + role=ProviderRole.EXECUTOR, + requested_reasoning="high", + ) + delegated = AsyncMock() + + with patch( + "crewplane.runtime.agent.invoker.invoke_agent", + delegated, + ): + await PlannedAgentInvoker( + plan_builder=build_cli_invocation_plan, + log_presentation_builder=build_cli_log_presentation, + ).invoke( + config=config, + model=None, + prompt="prompt", + output_file=output_file, + cwd=working_directory, + invocation_context=context, + ) + + plan_builder = delegated.await_args.kwargs["plan_builder"] + with ( + patch.dict(os.environ, {"CLAUDE_CODE_EFFORT_LEVEL": ""}), + self.assertRaisesRegex(ValueError, "--settings effortLevel"), + ): + plan_builder(config, None, "prompt", output_file) diff --git a/tests/unit/architecture/test_invocation_contracts.py b/tests/unit/architecture/test_invocation_contracts.py index 7866645..316cead 100644 --- a/tests/unit/architecture/test_invocation_contracts.py +++ b/tests/unit/architecture/test_invocation_contracts.py @@ -4,6 +4,7 @@ from crewplane.architecture.contracts import ( SUPPORTED_PROVIDER_KIND_VALUES, + InvocationContext, InvocationSourceContext, InvocationWorkspaceContext, InvocationWorktreeContract, @@ -129,3 +130,12 @@ def test_invocation_workspace_context_records_source_identity(tmp_path) -> None: assert workspace.cwd == tmp_path assert workspace.invocation_source.source_kind == "project" assert workspace.worktree_contract.mode == "blob_exact" + + +def test_invocation_context_preserves_existing_positional_contract() -> None: + context = InvocationContext("node", "task", "codex", "executor", 2, 3, True) + + assert context.audit_round_num == 2 + assert context.round_num == 3 + assert context.findings_enabled is True + assert context.requested_reasoning is None diff --git a/tests/unit/artifacts/test_output.py b/tests/unit/artifacts/test_output.py index 9775e5b..e41e03c 100644 --- a/tests/unit/artifacts/test_output.py +++ b/tests/unit/artifacts/test_output.py @@ -3,6 +3,7 @@ import json import os import shutil +import stat import tempfile import unittest from datetime import datetime @@ -11,7 +12,7 @@ from crewplane.artifacts import OutputManager from crewplane.artifacts.generated_files.catalog import ( - generated_file_source_root, + generated_file_snapshot_rejection_summary, snapshot_generated_file_workspace, ) from crewplane.core.execution_state import ( @@ -165,6 +166,7 @@ def test_finalize_stage_namespaces_workspace_generated_files_by_task(self) -> No "alpha content", encoding="utf-8", ) + (alpha_workspace / "src" / "app.txt").chmod(0o640) (beta_workspace / "src" / "app.txt").write_text( "beta content", encoding="utf-8", @@ -202,6 +204,111 @@ def test_finalize_stage_namespaces_workspace_generated_files_by_task(self) -> No ), "beta content", ) + self.assertEqual( + stat.S_IMODE( + (generated_dir / "alpha" / "src" / "app.txt").stat().st_mode + ), + 0o640, + ) + + def test_finalize_stage_preserves_result_when_generated_file_copy_fails( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + output = OutputManager("Workflow", base_dir=base_dir) + workspace = base_dir / "workspace" + (workspace / "blocked").mkdir(parents=True) + (workspace / "blocked" / "app.txt").write_text( + "blocked content", + encoding="utf-8", + ) + (workspace / "good.txt").write_text("good content", encoding="utf-8") + stage_dir = output.create_stage_dir("build.node") + provider_output = stage_dir / "alpha_round1.md" + provider_output.write_text( + "## Generated Files\n\n- `blocked/app.txt`\n- `good.txt`\n", + encoding="utf-8", + ) + snapshot = snapshot_generated_file_workspace(provider_output, workspace) + blocking_path = ( + output.results_dir + / "generated-files" + / "build.node" + / "alpha" + / "blocked" + ) + blocking_path.parent.mkdir(parents=True, exist_ok=True) + blocking_path.write_text("preserve me", encoding="utf-8") + + result = output.finalize_stage( + "build.node", + generated_file_workspace_roots={ + provider_output.resolve(strict=False): snapshot, + }, + ) + + result_text = result.result_file.read_text(encoding="utf-8") + generated_file = ( + output.results_dir + / "generated-files" + / "build.node" + / "alpha" + / "good.txt" + ) + self.assertTrue(provider_output.is_file()) + self.assertIn("[alpha/good.txt]", result_text) + self.assertNotIn("[alpha/blocked/app.txt]", result_text) + self.assertEqual(result.generated_files, (generated_file,)) + self.assertEqual( + generated_file.read_text(encoding="utf-8"), + "good content", + ) + self.assertEqual(blocking_path.read_text(encoding="utf-8"), "preserve me") + self.assertEqual(len(result.warnings), 1) + self.assertIn("alpha/blocked/app.txt", result.warnings[0]) + self.assertIn("copy failed", result.warnings[0].lower()) + + def test_finalize_stage_removes_partial_generated_file_copy(self) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + output = OutputManager("Workflow", base_dir=base_dir) + workspace = base_dir / "workspace" + workspace.mkdir() + (workspace / "app.txt").write_text("complete", encoding="utf-8") + stage_dir = output.create_stage_dir("build.node") + provider_output = stage_dir / "alpha_round1.md" + provider_output.write_text( + "## Generated Files\n\n- `app.txt`\n", + encoding="utf-8", + ) + snapshot = snapshot_generated_file_workspace(provider_output, workspace) + + def fail_after_partial_copy(source: Path, target: Path) -> None: # noqa: ARG001 + Path(target).write_text("partial", encoding="utf-8") + raise OSError("disk full") + + with patch( + "crewplane.artifacts.generated_files.catalog.shutil.copyfile", + side_effect=fail_after_partial_copy, + ): + result = output.finalize_stage( + "build.node", + generated_file_workspace_roots={ + provider_output.resolve(strict=False): snapshot, + }, + ) + + result_text = result.result_file.read_text(encoding="utf-8") + generated_file_dir = ( + output.results_dir / "generated-files" / "build.node" / "alpha" + ) + self.assertNotIn("[alpha/app.txt]", result_text) + self.assertEqual(result.generated_files, ()) + self.assertEqual(len(result.warnings), 1) + self.assertIn("disk full", result.warnings[0]) + self.assertFalse((generated_file_dir / "app.txt").exists()) + self.assertEqual(list(generated_file_dir.glob(".generated-file-*.tmp")), []) def test_workspace_generated_files_hash_truncated_stage_directories(self) -> None: with tempfile.TemporaryDirectory() as tmp_dir: @@ -456,7 +563,7 @@ def test_workspace_generated_file_snapshot_accepts_missing_output_file_with_cand "created", ) - def test_workspace_generated_file_snapshot_rejects_too_many_before_copying( + def test_workspace_generated_file_snapshot_records_files_over_count_limit( self, ) -> None: with tempfile.TemporaryDirectory() as tmp_dir: @@ -479,7 +586,110 @@ def test_workspace_generated_file_snapshot_rejects_too_many_before_copying( ), encoding="utf-8", ) - snapshot_root = generated_file_source_root(alpha_output) + + with patch( + "crewplane.artifacts.generated_files.catalog." + "MAX_GENERATED_FILE_SNAPSHOT_FILES", + 1, + ): + snapshot_root = snapshot_generated_file_workspace( + alpha_output, workspace + ) + + metadata = json.loads( + (snapshot_root / ".crewplane-generated-file-snapshot.json").read_text( + encoding="utf-8" + ) + ) + self.assertEqual(len(metadata["files"]), 1) + self.assertEqual(metadata["rejected_file_count"], 1) + self.assertFalse(metadata["rejected_files_truncated"]) + self.assertEqual( + metadata["rejected_files"], + [ + { + "configured_limit_count": 1, + "discovery_source": "provider_explicit_section", + "disposition": "rejected", + "explicit": True, + "path": "src/two.txt", + "reason": "file_count_limit", + "size_bytes": 1, + } + ], + ) + self.assertTrue((snapshot_root / "src" / "one.txt").is_file()) + self.assertFalse((snapshot_root / "src" / "two.txt").exists()) + + def test_workspace_generated_file_snapshot_records_oversized_explicit_file( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + output = OutputManager("Workflow", base_dir=base_dir) + workspace = base_dir / "workspace" + (workspace / "src").mkdir(parents=True) + oversized = workspace / "src" / "large.bin" + oversized.write_bytes(b"xx") + stage_dir = output.create_stage_dir("build.node") + alpha_output = stage_dir / "alpha_round1.md" + alpha_output.write_text( + "## Generated Files\n\n- `src/large.bin`\n", + encoding="utf-8", + ) + + with patch( + "crewplane.artifacts.generated_files.catalog." + "MAX_GENERATED_FILE_SNAPSHOT_BYTES", + 1, + ): + snapshot_root = snapshot_generated_file_workspace( + alpha_output, + workspace, + candidate_files=(oversized,), + explicit_claims_only=True, + ) + + metadata = json.loads( + (snapshot_root / ".crewplane-generated-file-snapshot.json").read_text( + encoding="utf-8" + ) + ) + self.assertEqual(metadata["files"], []) + self.assertEqual(metadata["rejected_file_count"], 1) + self.assertFalse(metadata["rejected_files_truncated"]) + self.assertEqual( + metadata["rejected_files"], + [ + { + "configured_limit_bytes": 1, + "discovery_source": "provider_explicit_section", + "disposition": "rejected", + "explicit": True, + "path": "src/large.bin", + "reason": "per_file_size_limit", + "size_bytes": 2, + } + ], + ) + self.assertFalse((snapshot_root / "src" / "large.bin").exists()) + + def test_workspace_generated_file_snapshot_bounds_rejection_details( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + output = OutputManager("Workflow", base_dir=base_dir) + workspace = base_dir / "workspace" + source_dir = workspace / "src" + source_dir.mkdir(parents=True) + generated_files = [] + for index in range(6): + generated_file = source_dir / f"{index}.txt" + generated_file.write_text(str(index), encoding="utf-8") + generated_files.append(generated_file) + stage_dir = output.create_stage_dir("build.node") + alpha_output = stage_dir / "alpha_round1.md" with ( patch( @@ -487,13 +697,34 @@ def test_workspace_generated_file_snapshot_rejects_too_many_before_copying( "MAX_GENERATED_FILE_SNAPSHOT_FILES", 1, ), - self.assertRaisesRegex(RuntimeError, "too many files"), + patch( + "crewplane.artifacts.generated_files.catalog." + "MAX_GENERATED_FILE_SNAPSHOT_REJECTION_DETAILS", + 2, + ), ): - snapshot_generated_file_workspace(alpha_output, workspace) + snapshot_root = snapshot_generated_file_workspace( + alpha_output, + workspace, + candidate_files=generated_files, + ) - self.assertFalse(snapshot_root.exists()) + metadata = json.loads( + (snapshot_root / ".crewplane-generated-file-snapshot.json").read_text( + encoding="utf-8" + ) + ) + summary = generated_file_snapshot_rejection_summary(snapshot_root) - def test_workspace_generated_file_snapshot_rejects_size_change_during_copy( + self.assertEqual(len(metadata["files"]), 1) + self.assertEqual(metadata["rejected_file_count"], 5) + self.assertEqual(len(metadata["rejected_files"]), 2) + self.assertTrue(metadata["rejected_files_truncated"]) + self.assertEqual(summary.total_count, 5) + self.assertEqual(len(summary.recorded_files), 2) + self.assertTrue(summary.truncated) + + def test_workspace_generated_file_snapshot_records_size_change_during_copy( self, ) -> None: with tempfile.TemporaryDirectory() as tmp_dir: @@ -505,25 +736,66 @@ def test_workspace_generated_file_snapshot_rejects_size_change_during_copy( stage_dir = output.create_stage_dir("build.node") alpha_output = stage_dir / "alpha_round1.md" alpha_output.write_text("Updated `src/app.txt`.\n", encoding="utf-8") - snapshot_root = generated_file_source_root(alpha_output) def write_expanded_copy(source: Path, target: Path) -> Path: target.write_bytes(source.read_bytes() + b"expanded") return target - with ( - patch( - "crewplane.artifacts.generated_files.catalog.shutil.copyfile", - side_effect=write_expanded_copy, - ), - self.assertRaisesRegex(RuntimeError, "changed while copying"), + with patch( + "crewplane.artifacts.generated_files.catalog.shutil.copyfile", + side_effect=write_expanded_copy, ): - snapshot_generated_file_workspace(alpha_output, workspace) + snapshot_root = snapshot_generated_file_workspace( + alpha_output, workspace + ) + metadata = json.loads( + (snapshot_root / ".crewplane-generated-file-snapshot.json").read_text( + encoding="utf-8" + ) + ) + self.assertEqual(metadata["files"], []) + self.assertEqual(metadata["rejected_files"][0]["reason"], "copy_failed") + self.assertEqual(metadata["rejected_files"][0]["path"], "src/app.txt") self.assertFalse((snapshot_root / "src" / "app.txt").exists()) - self.assertFalse( - (snapshot_root / ".crewplane-generated-file-snapshot.json").exists() + + def test_workspace_generated_file_snapshot_removes_partial_failed_copy( + self, + ) -> None: + with tempfile.TemporaryDirectory() as tmp_dir: + base_dir = Path(tmp_dir) + output = OutputManager("Workflow", base_dir=base_dir) + workspace = base_dir / "workspace" + (workspace / "src").mkdir(parents=True) + (workspace / "src" / "app.txt").write_text( + "complete", + encoding="utf-8", ) + stage_dir = output.create_stage_dir("build.node") + alpha_output = stage_dir / "alpha_round1.md" + alpha_output.write_text("Updated `src/app.txt`.\n", encoding="utf-8") + + def write_partial_copy_then_fail(source: Path, target: Path) -> None: + target.write_bytes(source.read_bytes()[:3]) + raise OSError("copy failed") + + with patch( + "crewplane.artifacts.generated_files.catalog.shutil.copyfile", + side_effect=write_partial_copy_then_fail, + ): + snapshot_root = snapshot_generated_file_workspace( + alpha_output, + workspace, + ) + + metadata = json.loads( + (snapshot_root / ".crewplane-generated-file-snapshot.json").read_text( + encoding="utf-8" + ) + ) + self.assertEqual(metadata["files"], []) + self.assertEqual(metadata["rejected_files"][0]["reason"], "copy_failed") + self.assertFalse((snapshot_root / "src" / "app.txt").exists()) def test_workspace_generated_file_snapshot_ignores_hardlinked_files(self) -> None: with tempfile.TemporaryDirectory() as tmp_dir: diff --git a/tests/unit/core/test_role_and_workspace_enums.py b/tests/unit/core/test_role_and_workspace_enums.py index 333ef1f..ca6d2fd 100644 --- a/tests/unit/core/test_role_and_workspace_enums.py +++ b/tests/unit/core/test_role_and_workspace_enums.py @@ -25,6 +25,23 @@ def test_provider_role_validates_and_serializes_lowercase_values() -> None: assert provider.model_dump(mode="json")["role"] == "reviewer" +@pytest.mark.parametrize( + "reasoning", + ["", "has space", "élevé", "bad/value", "bad\\value", "bad=value", "x" * 65], +) +def test_provider_reasoning_rejects_unsafe_native_tokens(reasoning: str) -> None: + with pytest.raises(ValidationError, match="reasoning must be"): + ProviderSpec(provider="alpha", reasoning=reasoning) + + +def test_provider_reasoning_null_matches_omission() -> None: + omitted = ProviderSpec(provider="alpha") + explicit_null = ProviderSpec(provider="alpha", reasoning=None) + + assert omitted == explicit_null + assert omitted.model_dump(mode="json") == explicit_null.model_dump(mode="json") + + @pytest.mark.parametrize("role", ["Reviewer", " reviewer ", "author"]) def test_provider_role_rejects_invalid_or_mixed_case_values(role: str) -> None: with pytest.raises(ValidationError): diff --git a/tests/unit/observability/log_presentation/test_formatters.py b/tests/unit/observability/log_presentation/test_formatters.py index b38358e..2972f35 100644 --- a/tests/unit/observability/log_presentation/test_formatters.py +++ b/tests/unit/observability/log_presentation/test_formatters.py @@ -634,6 +634,35 @@ def test_plain_formatter_strips_recognized_header(tmp_path: Path) -> None: assert snapshot.lines == ("visible",) +def test_plain_formatter_strips_header_with_requested_reasoning( + tmp_path: Path, +) -> None: + log_path = tmp_path / "plain-reasoning.log" + log_path.write_text( + "\n".join( + [ + "started_at: 2026-06-10T00:00:00+00:00", + "cli_executable: provider", + "model: test", + "requested_reasoning: xhigh", + f"output_file: {tmp_path / 'out.md'}", + "---", + "visible", + ] + ), + encoding="utf-8", + ) + + snapshot = format_log_file( + log_path, + LogPresentationDescriptor(format="plain", profile="generic"), + line_budget=5, + wall_time_now=0.0, + ) + + assert snapshot.lines == ("visible",) + + def _nested_json_object(depth: int) -> str: return '{"message":' * depth + '"leaf"' + "}" * depth + "\n" diff --git a/tests/unit/runtime/execution/test_generated_file_change_detection.py b/tests/unit/runtime/execution/test_generated_file_change_detection.py index b3ad0b2..3bfb539 100644 --- a/tests/unit/runtime/execution/test_generated_file_change_detection.py +++ b/tests/unit/runtime/execution/test_generated_file_change_detection.py @@ -15,13 +15,14 @@ from crewplane.core.workflow.keywords import ProviderRole from crewplane.runtime.execution.provider_call.generated_files import ( GeneratedFileChangeBaseline, + capture_generated_file_change_baseline, snapshot_invocation_generated_files, ) from crewplane.runtime.execution.provider_call.types import ProviderOutputPolicy from crewplane.runtime.workspace import PreparedWorkspace -def test_git_project_root_generated_files_do_not_require_provider_claim( +def test_shared_project_root_does_not_capture_unclaimed_changes( tmp_path: Path, ) -> None: repo = _clean_repo(tmp_path) @@ -36,11 +37,77 @@ def test_git_project_root_generated_files_do_not_require_provider_claim( repo, baseline, "No generated files were mentioned.\n", + explicit_claims_only=True, + ) + + assert "## Generated Files" not in result_text + assert generated_files == () + + +def test_shared_project_root_captures_explicit_generated_file_claim( + tmp_path: Path, +) -> None: + repo = _clean_repo(tmp_path) + baseline = GeneratedFileChangeBaseline.capture( + repo, + filesystem_fallback_enabled=False, + ) + generated_file = repo / "src" / "created.txt" + generated_file.write_text("created\n", encoding="utf-8") + unrelated_file = repo / "ambient.tmp" + unrelated_file.write_text("ambient\n", encoding="utf-8") + + result_text, generated_files = _finalize_with_snapshot( + repo, + baseline, + "## Generated Files\n\n- `src/created.txt`\n", + explicit_claims_only=True, ) - assert "[alpha/src/app.txt]" in result_text or "[src/app.txt]" in result_text assert "[alpha/src/created.txt]" in result_text - assert {path.name for path in generated_files} == {"app.txt", "created.txt"} + assert {path.name for path in generated_files} == {"created.txt"} + assert all(path.name != unrelated_file.name for path in generated_files) + + +def test_shared_project_root_skips_unchanged_explicit_generated_file_claim( + tmp_path: Path, +) -> None: + repo = _clean_repo(tmp_path) + baseline = GeneratedFileChangeBaseline.capture( + repo, + filesystem_fallback_enabled=False, + ) + + result_text, generated_files = _finalize_with_snapshot( + repo, + baseline, + "## Generated Files\n\n- `src/app.txt`\n", + explicit_claims_only=True, + ) + + assert "[alpha/src/app.txt]" not in result_text + assert generated_files == () + + +def test_shared_project_root_uses_git_change_baseline(tmp_path: Path) -> None: + repo = _clean_repo(tmp_path) + prepared_workspace = PreparedWorkspace( + cwd=repo, + invocation_context=InvocationContext( + node_id="build", + task_id="task", + provider="provider", + role=ProviderRole.EXECUTOR, + ), + ) + + baseline = capture_generated_file_change_baseline(prepared_workspace) + + assert baseline is not None + assert baseline.candidate_files() == () + changed_file = repo / "src" / "app.txt" + changed_file.write_text("changed\n", encoding="utf-8") + assert baseline.candidate_files() == (changed_file,) def test_git_managed_workspace_generated_files_do_not_require_provider_claim( @@ -223,15 +290,16 @@ def test_non_git_project_root_prose_fallback_still_links_files(tmp_path: Path) - assert "[alpha/src/app.txt]" in result_text or "[src/app.txt]" in result_text -def test_non_git_project_root_fallback_uses_workspace_cwd_for_snapshot( - tmp_path: Path, -) -> None: +def test_non_git_project_root_explicit_claim_fails_closed(tmp_path: Path) -> None: output = OutputManager("Workflow", base_dir=tmp_path) (tmp_path / "src").mkdir() (tmp_path / "src" / "app.txt").write_text("content", encoding="utf-8") stage_dir = output.create_stage_dir("build.node") output_file = stage_dir / "alpha_round1.md" - output_file.write_text("Updated `src/app.txt`.\n", encoding="utf-8") + output_file.write_text( + "## Generated Files\n\n- `src/app.txt`\n", + encoding="utf-8", + ) request = SimpleNamespace(output_file=output_file) prepared_workspace = PreparedWorkspace( @@ -243,18 +311,25 @@ def test_non_git_project_root_fallback_uses_workspace_cwd_for_snapshot( role=ProviderRole.EXECUTOR, ), ) + baseline = capture_generated_file_change_baseline(prepared_workspace) + assert baseline is not None + assert baseline.candidate_files() is None + snapshot = snapshot_invocation_generated_files( request, prepared_workspace, + baseline, ) assert snapshot is not None - output.finalize_stage( + result = output.finalize_stage( "build.node", generated_file_workspace_roots={output_file.resolve(strict=False): snapshot}, ) result_text = output.get_stage_output_path("build.node").read_text(encoding="utf-8") - assert "[alpha/src/app.txt]" in result_text or "[src/app.txt]" in result_text + assert "[alpha/src/app.txt]" not in result_text + assert "[src/app.txt]" not in result_text + assert result.generated_files == () def test_snapshot_invocation_generated_files_skips_missing_workspace_cwd( @@ -323,6 +398,7 @@ def _finalize_with_snapshot( repo: Path, baseline: GeneratedFileChangeBaseline, provider_output: str, + explicit_claims_only: bool = False, ) -> tuple[str, tuple[Path, ...]]: output = OutputManager("Workflow", base_dir=repo) stage_dir = output.create_stage_dir("build.node") @@ -332,6 +408,7 @@ def _finalize_with_snapshot( alpha_output, baseline.invocation_root, candidate_files=baseline.candidate_files(), + explicit_claims_only=explicit_claims_only, ) result = output.finalize_stage( diff --git a/tests/unit/runtime/workspace/test_service_provider_invocation.py b/tests/unit/runtime/workspace/test_service_provider_invocation.py index 4a2e621..4186f3e 100644 --- a/tests/unit/runtime/workspace/test_service_provider_invocation.py +++ b/tests/unit/runtime/workspace/test_service_provider_invocation.py @@ -10,6 +10,7 @@ import pytest +import crewplane.runtime.execution.provider_call.artifact_capture as provider_invocation_artifact_capture_module import crewplane.runtime.execution.provider_call.generated_files as provider_invocation_generated_files_module import crewplane.runtime.execution.provider_call.lifecycle as provider_invocation_lifecycle_module import crewplane.runtime.execution.provider_call.workspace as provider_invocation_workspace_module @@ -33,6 +34,7 @@ mark_workspace_succeeded, record_generated_file_workspace, run_provider_call, + run_provider_invocation, ) from crewplane.runtime.execution.provider_call.workspace import ( prepare_workspace_with_cancellation, @@ -59,6 +61,42 @@ def test_provider_invocation_uses_snapshot_workspace_cwd( asyncio.run(_run_provider_invocation_uses_snapshot_workspace_cwd(tmp_path)) +def test_artifact_capture_wrapper_failure_fails_provider_invocation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + asyncio.run( + _run_artifact_capture_wrapper_failure_fails_provider_invocation( + tmp_path, + monkeypatch, + ) + ) + + +def test_workspace_finalization_failure_fails_provider_invocation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + asyncio.run( + _run_workspace_finalization_failure_fails_provider_invocation( + tmp_path, + monkeypatch, + ) + ) + + +def test_generated_file_capture_failure_disables_project_root_fallback( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + asyncio.run( + _run_generated_file_capture_failure_disables_project_root_fallback( + tmp_path, + monkeypatch, + ) + ) + + def test_provider_invocation_cancellation_marks_workspace_state( tmp_path: Path, ) -> None: @@ -422,6 +460,203 @@ def record_event(event) -> None: assert workspace_payload.workspace_child_environment_required is False +async def _run_artifact_capture_wrapper_failure_fails_provider_invocation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + plan = disabled_workspace_plan(repo) + output = workspace_output_manager(tmp_path, repo) + node_dir = output.create_stage_dir("implement") + runtime_context = CompiledRuntimeContext( + plan=plan, + secret_context=SecretContext(), + ) + events = [] + telemetry = ExecutionTelemetry( + workflow_name=plan.workflow_name, + run_id=plan.run_id, + event_sink=events.append, + suppress_console_output=True, + ) + + async def fail_artifact_capture( + request: ProviderCallRequest, + prepared_workspace: PreparedWorkspace, + baseline: object, + metadata: object, + ) -> None: + del request, prepared_workspace, baseline, metadata + raise RuntimeError("artifact capture wrapper failed") + + monkeypatch.setattr( + provider_invocation_lifecycle_module, + "capture_invocation_generated_files", + fail_artifact_capture, + ) + output_file = node_dir / "alpha_round1.md" + + result = await run_provider_invocation( + ProviderCallRequest( + runtime_context=runtime_context, + output=output, + node_id="implement", + provider=plan.nodes[0].provider_records[0], + task_id="alpha", + audit_round_num=None, + round_num=1, + prompt="done", + output_file=output_file, + role_label=ProviderRole.EXECUTOR, + invoker=SuccessfulRuntimeInvoker(), + telemetry=telemetry, + ), + capture_exception=True, + display=ProviderCallDisplay(telemetry=telemetry), + ) + + assert output_file.read_text(encoding="utf-8") == "done\n" + assert isinstance(result.error, RuntimeError) + assert str(result.error) == "artifact capture wrapper failed" + assert not any(event.event_type == "invocation_finished" for event in events) + assert any(event.event_type == "invocation_failed" for event in events) + + +async def _run_workspace_finalization_failure_fails_provider_invocation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + plan = disabled_workspace_plan(repo) + output = workspace_output_manager(tmp_path, repo) + node_dir = output.create_stage_dir("implement") + runtime_context = CompiledRuntimeContext( + plan=plan, + secret_context=SecretContext(), + ) + events = [] + telemetry = ExecutionTelemetry( + workflow_name=plan.workflow_name, + run_id=plan.run_id, + event_sink=events.append, + suppress_console_output=True, + ) + + async def fail_workspace_finalization( + request: ProviderCallRequest, + prepared_workspace: PreparedWorkspace, + child_environment_applied: bool | None, + generated_file_workspace: Path | None, + ) -> None: + del request, prepared_workspace, child_environment_applied + del generated_file_workspace + raise RuntimeError("workspace finalization failed") + + monkeypatch.setattr( + provider_invocation_lifecycle_module, + "finalize_successful_workspace", + fail_workspace_finalization, + ) + output_file = node_dir / "alpha_round1.md" + + result = await run_provider_invocation( + ProviderCallRequest( + runtime_context=runtime_context, + output=output, + node_id="implement", + provider=plan.nodes[0].provider_records[0], + task_id="alpha", + audit_round_num=None, + round_num=1, + prompt="done", + output_file=output_file, + role_label=ProviderRole.EXECUTOR, + invoker=SuccessfulRuntimeInvoker(), + telemetry=telemetry, + ), + capture_exception=True, + display=ProviderCallDisplay(telemetry=telemetry), + ) + + assert isinstance(result.error, RuntimeError) + assert str(result.error) == "workspace finalization failed" + assert not any(event.event_type == "invocation_finished" for event in events) + assert any(event.event_type == "invocation_failed" for event in events) + + +async def _run_generated_file_capture_failure_disables_project_root_fallback( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo = tmp_path / "repo" + generated_file = repo / "src" / "app.txt" + generated_file.parent.mkdir(parents=True) + generated_file.write_text("live project content\n", encoding="utf-8") + plan = disabled_workspace_plan(repo) + output = workspace_output_manager(tmp_path, repo) + node_dir = output.create_stage_dir("implement") + runtime_context = CompiledRuntimeContext( + plan=plan, + secret_context=SecretContext(), + ) + telemetry = ExecutionTelemetry( + workflow_name=plan.workflow_name, + run_id=plan.run_id, + suppress_console_output=True, + ) + + async def fail_generated_file_capture( + request: ProviderCallRequest, + prepared_workspace: PreparedWorkspace, + baseline: object, + ) -> None: + del request, prepared_workspace, baseline + raise RuntimeError("generated-file capture failed") + + monkeypatch.setattr( + provider_invocation_artifact_capture_module, + "snapshot_invocation_generated_files_async", + fail_generated_file_capture, + ) + output_file = node_dir / "alpha_round1.md" + await run_provider_call( + ProviderCallRequest( + runtime_context=runtime_context, + output=output, + node_id="implement", + provider=plan.nodes[0].provider_records[0], + task_id="alpha", + audit_round_num=None, + round_num=1, + prompt="done", + output_file=output_file, + role_label=ProviderRole.EXECUTOR, + invoker=SuccessfulRuntimeInvoker(), + telemetry=telemetry, + ), + display=ProviderCallDisplay(telemetry=telemetry), + ) + output_file.write_text( + "Updated `src/app.txt`.\n\n## Generated Files\n\n- `src/app.txt`\n", + encoding="utf-8", + ) + + workspace_roots = runtime_context.generated_file_workspaces.roots_for_node( + "implement" + ) + assert workspace_roots[output_file.resolve(strict=False)] is None + result = output.finalize_stage( + "implement", + generated_file_workspace_roots=workspace_roots, + ) + result_text = result.result_file.read_text(encoding="utf-8") + + assert result.generated_files == () + assert "(../../src/app.txt)" not in result_text + + async def _run_provider_invocation_generated_file_snapshot_does_not_block_event_loop( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From c87b4ea2db7b1e75a89c883da2574e6588f1d129 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 27 Jul 2026 18:04:08 -0700 Subject: [PATCH 2/2] Fix lint --- src/crewplane/architecture/contracts/invocation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crewplane/architecture/contracts/invocation.py b/src/crewplane/architecture/contracts/invocation.py index cd6c40a..3a7b6d0 100644 --- a/src/crewplane/architecture/contracts/invocation.py +++ b/src/crewplane/architecture/contracts/invocation.py @@ -3,7 +3,7 @@ import re from collections.abc import Callable, Iterator, Mapping from contextlib import suppress -from dataclasses import KW_ONLY, dataclass +from dataclasses import dataclass from enum import StrEnum from pathlib import Path from types import MappingProxyType