diff --git a/.semgrep.yml b/.semgrep.yml index 617809e9f..2bb1e8164 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -20,6 +20,8 @@ rules: paths: include: - src/ + exclude: + - src/cleveragents/tool/wrapping.py - id: no-compile-exec pattern: compile(..., ..., "exec") @@ -31,6 +33,8 @@ rules: paths: include: - src/ + exclude: + - src/cleveragents/tool/wrapping.py - id: no-os-system pattern: os.system(...) diff --git a/docs/specification.md b/docs/specification.md index c856a617c..448875d65 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -310,7 +310,7 @@ The following standards are integrated into the architecture: agents resource add [(--description|-d) <DESC>] [--update] <TYPE> <NAME> [type-specific-flags...] agents resource remove [--yes|-y] <NAME> -agents resource list [--all] [(--type|-t) <TYPE>] +agents resource list [--all] [(--type|-t) <TYPE>] agents resource show <RESOURCE> agents resource inspect [--tree] [--file <PATH>] <RESOURCE> agents resource tree [(--depth|-d) <N>] [(--type|-t) <TYPE>] <RESOURCE> @@ -9278,7 +9278,7 @@ When multiple attachment scopes apply, the union of all applicable validations i ##### agents validation add -
agents validation add (--config|-c) <FILE> [--update]
+
agents validation add (--config|-c) <FILE> [--required | --informational] [--update]
**Purpose** Register a new validation. The validation is fully defined by the YAML configuration file specified with `--config`. The validation is registered in the shared Tool Registry under the name specified in the config file; the name must not conflict with any existing tool or validation. If a validation with the same name already exists, the command fails unless `--update` is provided. The validation YAML may use `wraps` to reference an existing Tool, reusing its implementation and interpreting its output through a `transform` function (see Tool Wrapping under Core Concepts > Validation). When `wraps` is used, the wrapped Tool must already be registered. @@ -9286,6 +9286,8 @@ Register a new validation. The validation is fully defined by the YAML configura **Arguments** - `--config/-c FILE`: Path to the validation YAML configuration file (required). The file must fully define the validation, including the `name` field which determines the validation's registered name, the `validation.mode` field (`required` or `informational`), and either inline implementation (`source` + `code`) or wrapping (`wraps` + `transform`). +- `--required`: Override the mode in the YAML config to `required`. Mutually exclusive with `--informational`. +- `--informational`: Override the mode in the YAML config to `informational`. Mutually exclusive with `--required`. - `--update`: Allow overwriting an existing validation registration. **Examples** @@ -18499,7 +18501,7 @@ Each plan has a **description field** (inherited from the action's description, For example: - **Plan description**: "Increase test coverage to 85%" -- **Decisions that emerge**: +- **Decisions that emerge**: - "Which modules should be prioritized?" (not specified in description) - "Should we use mocks or integration tests for the database layer?" (not specified) - "Should we refactor the auth module to make it more testable, or write tests around it as-is?" (not specified) @@ -18648,7 +18650,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a while unresolved_ambiguities_remain(plan, context): choice_point = analyze_context_for_ambiguity(context) options = generate_and_evaluate_options(choice_point, context, invariants) - + # Record via tool call — system auto-captures context snapshot decision_id = record_decision( decision_type = choice_point.type, @@ -18658,7 +18660,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a confidence_score = best_option.confidence, rationale = best_option.reasoning ) - + context.add_decision(decision_id) # Decision informs subsequent reasoning @@ -18678,7 +18680,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a plan_id: ULID # Parent plan this decision belongs to parent_decision_id: ULID | null # Parent decision (for tree structure) sequence_number: int # Order within the plan's decisions - + # Classification decision_type: enum - prompt_definition # The prompt/description for this plan (root decision) @@ -18692,20 +18694,20 @@ The `record_decision` tool accepts the decision type, question, chosen option, a - error_recovery # How to handle a failure - validation_response # Response to validation failure - user_intervention # User provided guidance/correction - + # The Decision Itself question: str # What question was being answered chosen_option: str # What was decided alternatives_considered: list[str] # Other options that were evaluated confidence_score: float | null # 0.0-1.0 if the actor provided confidence - + # Context Snapshot (for replay) context_snapshot: hot_context_hash: str # Cryptographic hash of the exact context hot_context_ref: str # Pointer to the full stored snapshot relevant_resources: list[ResourceRef] # Every file/symbol that influenced this decision actor_state_ref: str # Complete LangGraph checkpoint - + # When the system decides "refactor the authentication module to use async patterns," # it permanently records: # - Which files were examined to make that decision @@ -18713,19 +18715,19 @@ The `record_decision` tool accepts the decision type, question, chosen option, a # - The exact code state that was analyzed # - The reasoning chain that led to this choice # - Alternative approaches that were considered but rejected - + # Rationale rationale: str # Why this option was chosen actor_reasoning: str | null # Raw LLM reasoning if available - + # Downstream Impact (populated during Execute phase) downstream_decision_ids: list[ULID] # Decisions that depend on this one downstream_plan_ids: list[ULID] # Child plans spawned because of this decision artifacts_produced: list[ArtifactRef] # Files/outputs created under this decision - + # Timestamps created_at: datetime - + # Correction Metadata is_correction: bool # Was this decision a correction of another? corrects_decision_id: ULID | null # If correction, which decision was replaced @@ -18766,7 +18768,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a correction_reason TEXT, superseded_by TEXT, created_at TEXT NOT NULL, - + FOREIGN KEY (plan_id) REFERENCES plans(plan_id), FOREIGN KEY (parent_decision_id) REFERENCES decisions(decision_id), FOREIGN KEY (corrects_decision_id) REFERENCES decisions(decision_id), @@ -18779,7 +18781,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a downstream_decision_id TEXT NOT NULL, dependency_type TEXT NOT NULL, -- 'decision', 'plan', 'artifact' downstream_ref TEXT NOT NULL, -- The actual ID of decision/plan/artifact - + PRIMARY KEY (upstream_decision_id, downstream_decision_id, downstream_ref), FOREIGN KEY (upstream_decision_id) REFERENCES decisions(decision_id) ); @@ -18795,7 +18797,7 @@ The `record_decision` tool accepts the decision type, question, chosen option, a status TEXT NOT NULL, -- 'pending', 'executing', 'completed', 'failed' created_at TEXT NOT NULL, completed_at TEXT, - + FOREIGN KEY (plan_id) REFERENCES plans(plan_id), FOREIGN KEY (original_decision_id) REFERENCES decisions(decision_id), FOREIGN KEY (new_decision_id) REFERENCES decisions(decision_id) @@ -18875,7 +18877,7 @@ Optional but recommended for reusable actions. ##### 4) `definition_of_done` (DoD) -Required. Must be explicit and testable. +Required. Must be explicit and testable. ##### 5) `actors` @@ -18996,21 +18998,21 @@ During the Strategize phase, the strategy actor employs specialized mechanisms t # Pseudocode of what happens inside a strategy actor def compute_closure_for_refactoring(target_module): closure = ResourceClosure() - + # Direct file dependencies closure.add_files(find_imports(target_module)) closure.add_files(find_includes(target_module)) - + # Symbol dependencies for symbol in extract_exported_symbols(target_module): closure.add_files(find_symbol_usage(symbol, scope='project')) - + # Test dependencies closure.add_files(find_tests_for_module(target_module)) - + # Build system dependencies closure.add_files(find_build_references(target_module)) - + return closure @@ -19217,7 +19219,7 @@ When child plans complete, the parent plan performs intelligent merging: def merge_subplan_results(subplan_results): # Group by resource type by_resource = group_by_resource_type(subplan_results) - + # Apply resource-specific merge strategies for resource_type, changes in by_resource: if resource_type == 'git-checkout': @@ -19226,7 +19228,7 @@ When child plans complete, the parent plan performs intelligent merging: merge_fs_changes(changes) # Copy-on-write reconciliation elif resource_type.startswith('database'): merge_db_changes(changes) # Sequential application - + # Validate merged state run_integration_tests() @@ -19324,7 +19326,7 @@ Execution should be treated like a transactional pipeline: * commits a checkpoint on success, or * rolls back to the previous checkpoint on failure. -This is explicitly motivated by "partial failure leaves codebase inconsistent" and the need for transaction rollback. +This is explicitly motivated by "partial failure leaves codebase inconsistent" and the need for transaction rollback. #### Execution Environment Routing @@ -19529,12 +19531,12 @@ The `local/validate-api-compat` tool is independently registered via its own YAM # Not just syntax checking - semantic validation old_api = extract_api_signature(previous_version) new_api = extract_api_signature(current_version) - + breaking_changes = find_breaking_changes(old_api, new_api) if breaking_changes: affected_consumers = find_api_consumers(breaking_changes) migration_plan = generate_migration(breaking_changes) - + if can_auto_migrate(affected_consumers, migration_plan): apply_migration(migration_plan) else: @@ -19634,18 +19636,18 @@ The system collects, reconciles, and checks invariants: """Compute the effective invariant view for a plan using the Invariant Reconciliation Actor.""" # 1. Collect raw invariants from all scopes raw = self.collect_all_invariants(plan) - + # 2. Find the Invariant Reconciliation Actor (plan -> project -> global config) reconciler = ( self.get_plan_invariant_actor(plan) or self.get_project_invariant_actor(plan) or self.get_global_invariant_actor() ) - + # 3. Reconcile: apply precedence (plan > project > global), resolve conflicts effective = reconciler.reconcile(raw, precedence=['plan', 'project', 'global']) return effective - + def collect_all_invariants(self, plan): """Collect invariants from all scopes accessible to this plan.""" invariants = [] @@ -19655,7 +19657,7 @@ The system collects, reconciles, and checks invariants: invariants.extend(self.get_action_invariants(plan.action)) invariants.extend(self.get_plan_invariants(plan)) return invariants - + def check_invariant_preservation(self, changes, enforced_invariants): """Check that changes respect all enforced invariants.""" for invariant in enforced_invariants: @@ -21905,7 +21907,7 @@ Bridges external MCP servers into the tool model: - Validates params against `inputSchema` - Checks capability metadata against plan access policy - Creates a checkpoint if the tool is marked checkpointable - + After the MCP tool returns its `content[]` response, the adapter: - Parses the result into the CleverAgents `Result` format - Records any resource mutations as `Change` objects in the plan's `ChangeSet` @@ -21944,7 +21946,7 @@ Bridges Agent Skills Standard (`SKILL.md` folders) into the tool model. Agent Sk - Reading reference files for additional context - Using other available tools (e.g., built-in file operations) as part of the procedure - Making multiple tool calls in sequence to accomplish the workflow - + The adapter wraps this execution in a tool execution context so that all mutations are tracked, sandboxed, and checkpointable. Script execution respects the `allowed_tools` and `sandbox_policy` declared in the tool's YAML. 4. **deactivate()**: Removes the skill's instructions from the agent's active context to free up token budget. The skill's metadata remains available for re-activation. @@ -22100,14 +22102,14 @@ When an LLM agent decides to use a tool (regardless of source), the following fl

 class ToolExecutionContext:
     """Context provided to every tool execution, regardless of source."""
-    
+
     def __init__(self, plan: Plan, sandbox: Sandbox,
                  resources: dict[str, BoundResource]):
         self.plan = plan
         self.sandbox = sandbox
         self.resources = resources    # slot_name → BoundResource
         self.changes: list[Change] = []
-    
+
     def record_change(self, change: Change) -> None:
         """Record a change made by a tool."""
         self.changes.append(change)
@@ -22116,7 +22118,7 @@ When an LLM agent decides to use a tool (regardless of source), the following fl
 
 class WriteFileTool:
     """Example: built-in tool for writing files."""
-    
+
     def execute(self, path: str, content: str, ctx: ToolExecutionContext) -> None:
         handler = ctx.sandbox.get_handler(path)
         change = handler.write(path, content, ctx.sandbox)
@@ -25036,51 +25038,51 @@ Every resource type provides a handler that implements this interface:
 

 class ResourceHandler(Protocol):
     """Handler for a specific resource type."""
-    
+
     def read(self, path: str, sandbox: Sandbox) -> Content:
         """Read content from the sandboxed resource."""
         ...
-    
+
     def write(self, path: str, content: Content, sandbox: Sandbox) -> Change:
         """Write content and return the Change record."""
         ...
-    
+
     def delete(self, path: str, sandbox: Sandbox) -> Change:
         """Delete resource and return the Change record."""
         ...
-    
+
     def list(self, pattern: str, sandbox: Sandbox) -> list[str]:
         """List paths matching pattern."""
         ...
-    
+
     def diff(self, path: str, sandbox: Sandbox) -> str:
         """Generate diff between sandbox and original state."""
         ...
-    
+
     def supports_operation(self, operation: OperationType) -> bool:
         """Check if this resource supports the given operation."""
         ...
-    
+
     def discover_children(self, resource: ResourceRecord) -> list[ResourceRecord]:
         """Auto-discover child resources (called at registration and refresh)."""
         ...
-    
+
     def content_hash(self, path: str, sandbox: Sandbox) -> str:
         """Compute content hash for identity tracking."""
         ...
-    
+
     def create_sandbox(self, resource: ResourceRecord) -> Sandbox:
         """Create a sandbox for this resource using its type's strategy."""
         ...
-    
+
     def create_checkpoint(self, sandbox: Sandbox) -> Checkpoint:
         """Create a checkpoint within the sandbox."""
         ...
-    
+
     def rollback_to(self, sandbox: Sandbox, checkpoint: Checkpoint) -> None:
         """Roll back sandbox state to a checkpoint."""
         ...
-    
+
     def project_access(
         self,
         binding_resource: ResourceRecord,
@@ -25089,7 +25091,7 @@ Every resource type provides a handler that implements this interface:
         sandbox: Sandbox | None,
     ) -> AccessProjection | None:
         """Compute how to reach target_resource from binding_resource.
-        
+
         Returns an AccessProjection with access_path, protocol,
         crosses_sandbox flag, and read_richness score. Returns None
         if this handler cannot project access to the target type.
@@ -25789,19 +25791,19 @@ The `OutputSession` is the core abstraction that replaces direct construction of
         handle_b.close()
         session.close()                          # finalize the session
     """
-    
+
     # --- Session lifecycle ---
-    
+
     command: str                                  # The command that owns this session
     session_id: str                               # Unique session identifier (ULID)
     created_at: datetime                          # Session creation timestamp
-    
+
     _strategy: MaterializationStrategy            # The active materialization strategy
     _handles: OrderedDict[str, ElementHandle]     # handle_id → handle, in declaration order
     _event_queue: asyncio.Queue[ElementEvent]     # Internal event queue for serialization
     _state: SessionState                          # "open" | "closing" | "closed"
     _lock: threading.Lock                         # Protects handle creation/removal
-    
+
     @classmethod
     def open(cls, command: str, strategy: MaterializationStrategy,
              metadata: dict | None = None) -> "OutputSession":
@@ -25819,11 +25821,11 @@ The `OutputSession` is the core abstraction that replaces direct construction of
             A new OutputSession ready for element creation.
         """
         ...
-    
+
     # --- Element handle factories ---
     # Each factory creates a typed handle, registers it with the session in
     # declaration order, and emits an ElementCreated event to the strategy.
-    
+
     def panel(self, title: str, *,
               border_style: str = "rounded",
               priority: str = "normal",
@@ -25831,7 +25833,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
               metadata: dict | None = None) -> "PanelHandle":
         """Create a panel element handle for key-value pair output."""
         ...
-    
+
     def table(self, title: str | None, *,
               columns: list["ColumnDef"],
               summary: dict | None = None,
@@ -25842,7 +25844,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
               metadata: dict | None = None) -> "TableHandle":
         """Create a table element handle for tabular data."""
         ...
-    
+
     def tree(self, root_label: str, *,
              root_style: str | None = None,
              max_depth_hint: int | None = None,
@@ -25852,7 +25854,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
              metadata: dict | None = None) -> "TreeHandle":
         """Create a tree element handle for hierarchical data."""
         ...
-    
+
     def progress(self, label: str, *,
                  total: int | None = None,
                  indeterminate: bool = False,
@@ -25869,7 +25871,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
                    initial status "pending").
         """
         ...
-    
+
     def status(self, message: str, *,
                level: str = "info",
                detail: str | None = None,
@@ -25877,7 +25879,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
                metadata: dict | None = None) -> "StatusHandle":
         """Create a status message handle."""
         ...
-    
+
     def text(self, content: str = "", *,
              wrap: bool = True,
              indent: int = 0,
@@ -25885,7 +25887,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
              metadata: dict | None = None) -> "TextHandle":
         """Create a text block handle."""
         ...
-    
+
     def code(self, content: str = "", *,
              language: str | None = None,
              line_numbers: bool = False,
@@ -25894,7 +25896,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
              metadata: dict | None = None) -> "CodeHandle":
         """Create a code block handle."""
         ...
-    
+
     def diff(self, *,
              file_a: str | None = None,
              file_b: str | None = None,
@@ -25902,18 +25904,18 @@ The `OutputSession` is the core abstraction that replaces direct construction of
              metadata: dict | None = None) -> "DiffHandle":
         """Create a diff block handle."""
         ...
-    
+
     def separator(self, style: str = "line") -> "SeparatorHandle":
         """Create a visual separator. Separators are auto-closed on creation."""
         ...
-    
+
     def action_hint(self, commands: list[str],
                     description: str | None = None) -> "ActionHintHandle":
         """Create an action hint. Action hints are auto-closed on creation."""
         ...
-    
+
     # --- Session operations ---
-    
+
     def snapshot(self) -> "StructuredOutput":
         """Return a static snapshot of all elements accumulated so far.
         
@@ -25923,7 +25925,7 @@ The `OutputSession` is the core abstraction that replaces direct construction of
         debugging, or programmatic inspection.
         """
         ...
-    
+
     def close(self, *, exit_code: int = 0) -> "StructuredOutput":
         """Close the session and finalize all output.
         
@@ -25938,12 +25940,12 @@ The `OutputSession` is the core abstraction that replaces direct construction of
             The final StructuredOutput snapshot.
         """
         ...
-    
+
     # --- Context manager support ---
-    
+
     def __enter__(self) -> "OutputSession":
         return self
-    
+
     def __exit__(self, exc_type, exc_val, exc_tb) -> None:
         """Auto-close session on context exit.
         
@@ -25981,16 +25983,16 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     Closed Handle Behavior:
         Calling any write method on a closed handle raises ElementClosedError.
     """
-    
+
     handle_id: str                       # Unique handle identifier (ULID)
     element_type: str                    # Semantic type ("panel", "table", etc.)
     declaration_index: int               # Position in session's declaration order
-    
+
     _session: OutputSession              # Owning session (for event emission)
     _element: E                          # The accumulated element state
     _state: HandleState                  # "open" | "closed"
     _lock: threading.Lock                # Serializes writes to this handle
-    
+
     def close(self) -> None:
         """Close this handle, signaling that no more data will be written.
         
@@ -25998,20 +26000,20 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         For buffered strategies, this triggers rendering of the element.
         """
         ...
-    
+
     @property
     def is_open(self) -> bool:
         """Whether this handle is still accepting writes."""
         ...
-    
+
     @property
     def element(self) -> E:
         """The accumulated element state (read-only snapshot)."""
         ...
-    
+
     def __enter__(self) -> Self:
         return self
-    
+
     def __exit__(self, exc_type, exc_val, exc_tb) -> None:
         """Auto-close handle on context exit."""
         if self.is_open:
@@ -26024,7 +26026,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     Panels are titled groups of key-value pairs. Entries can be added,
     updated, or removed after creation.
     """
-    
+
     def set_entry(self, key: str, value: str, *,
                   style_hint: str | None = None,
                   icon: str | None = None) -> None:
@@ -26036,12 +26038,12 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         Emits an ElementUpdated event.
         """
         ...
-    
+
     def set_entries(self, entries: dict[str, str], *,
                     style_hints: dict[str, str] | None = None) -> None:
         """Set multiple entries at once (batch update). Emits a single event."""
         ...
-    
+
     def remove_entry(self, key: str) -> None:
         """Remove an entry by key. Emits an ElementUpdated event."""
         ...
@@ -26054,7 +26056,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     one at a time or in batches as data becomes available from queries,
     API calls, or concurrent operations.
     """
-    
+
     def add_row(self, row: dict) -> None:
         """Append a single row to the table.
         
@@ -26064,15 +26066,15 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         Emits an ElementUpdated event (type=row_added).
         """
         ...
-    
+
     def add_rows(self, rows: list[dict]) -> None:
         """Append multiple rows in a batch. Emits a single ElementUpdated event."""
         ...
-    
+
     def set_summary(self, summary: dict) -> None:
         """Set or update the summary/aggregation row. Emits an ElementUpdated event."""
         ...
-    
+
     def set_sort_key(self, column: str, *, descending: bool = False) -> None:
         """Change the sort key. Emits an ElementUpdated event."""
         ...
@@ -26085,7 +26087,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     is created with the handle. Subtrees can be constructed incrementally
     as hierarchical data is discovered.
     """
-    
+
     def add_child(self, parent_path: str | None, label: str, *,
                   style_hint: str | None = None,
                   collapsed: bool = False,
@@ -26106,7 +26108,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         Emits an ElementUpdated event.
         """
         ...
-    
+
     def set_node_style(self, path: str, style_hint: str) -> None:
         """Update the style of an existing node. Emits an ElementUpdated event."""
         ...
@@ -26119,7 +26121,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     rapid updates. The materialization strategy may throttle update events
     to avoid overwhelming the terminal (e.g., limiting redraws to 10/sec).
     """
-    
+
     def set_progress(self, current: int, total: int | None = None) -> None:
         """Update the progress counter.
         
@@ -26130,7 +26132,7 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         Emits an ElementUpdated event (may be throttled by the strategy).
         """
         ...
-    
+
     def set_step_status(self, step_label: str, status: str) -> None:
         """Update the status of a named step.
         
@@ -26141,11 +26143,11 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
         Emits an ElementUpdated event.
         """
         ...
-    
+
     def set_label(self, label: str) -> None:
         """Update the progress label text. Emits an ElementUpdated event."""
         ...
-    
+
     def increment(self, delta: int = 1) -> None:
         """Increment progress by delta. Convenience wrapper around set_progress."""
         ...
@@ -26158,15 +26160,15 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
     messages). However, they can be kept open for messages that may be revised
     (e.g., a "Working..." status that becomes "Done" or "Failed").
     """
-    
+
     def set_message(self, message: str) -> None:
         """Update the status message text. Emits an ElementUpdated event."""
         ...
-    
+
     def set_level(self, level: str) -> None:
         """Change the status level. Emits an ElementUpdated event."""
         ...
-    
+
     def set_detail(self, detail: str | None) -> None:
         """Set or clear the detail text. Emits an ElementUpdated event."""
         ...
@@ -26174,11 +26176,11 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
 
 class TextHandle(ElementHandle[TextBlock]):
     """Handle for a text block. Supports appending text incrementally."""
-    
+
     def append(self, text: str) -> None:
         """Append text to the block. Emits an ElementUpdated event."""
         ...
-    
+
     def set_content(self, content: str) -> None:
         """Replace the entire content. Emits an ElementUpdated event."""
         ...
@@ -26186,15 +26188,15 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
 
 class CodeHandle(ElementHandle[CodeBlock]):
     """Handle for a code block."""
-    
+
     def set_content(self, content: str) -> None:
         """Set the code content. Emits an ElementUpdated event."""
         ...
-    
+
     def set_language(self, language: str) -> None:
         """Set the language for syntax highlighting. Emits an ElementUpdated event."""
         ...
-    
+
     def set_highlight_lines(self, lines: list[int]) -> None:
         """Set lines to highlight. Emits an ElementUpdated event."""
         ...
@@ -26202,11 +26204,11 @@ Element handles are the producer-facing API. Each handle type wraps a specific e
 
 class DiffHandle(ElementHandle[DiffBlock]):
     """Handle for a diff block. Hunks can be added incrementally."""
-    
+
     def add_hunk(self, header: str, lines: list["DiffLine"]) -> None:
         """Add a diff hunk. Emits an ElementUpdated event."""
         ...
-    
+
     def set_stats(self, insertions: int, deletions: int, **extra: int) -> None:
         """Set diff statistics. Emits an ElementUpdated event."""
         ...
@@ -26422,9 +26424,9 @@ The strategy pattern creates a clean separation between **timing/ordering policy
     produces correct output regardless of format. The producer writes to
     handles; the strategy decides what reaches the terminal and when.
     """
-    
+
     strategy_name: str                   # "live" | "sequential_buffer" | "accumulate"
-    
+
     def bind(self, renderer: "ElementRenderer",
              terminal_caps: "TerminalCapabilities") -> None:
         """Bind this strategy to a renderer and terminal capabilities.
@@ -26435,24 +26437,24 @@ The strategy pattern creates a clean separation between **timing/ordering policy
         on_session_begin, since the session owns the stream.
         """
         ...
-    
+
     def on_session_begin(self, session: OutputSession, stream: IO) -> None:
         """Called when the session opens. The strategy receives the output
         stream and may write preamble (e.g., opening JSON bracket)."""
         ...
-    
+
     def on_element_created(self, event: ElementCreated) -> None:
         """Called when a new element handle is created."""
         ...
-    
+
     def on_element_updated(self, event: ElementUpdated) -> None:
         """Called when data is written to an element handle."""
         ...
-    
+
     def on_element_closed(self, event: ElementClosed) -> None:
         """Called when an element handle is closed."""
         ...
-    
+
     def on_session_end(self, event: SessionEnd) -> None:
         """Called when the session closes. The strategy may write epilogue."""
         ...
@@ -26490,7 +26492,7 @@ The strategy pattern creates a clean separation between **timing/ordering policy
     and redraws all dirty elements in a single pass per frame.
     """
     strategy_name = "live"
-    
+
     _frame_rate: float = 15.0            # Maximum redraws per second
     _element_regions: OrderedDict[str, ScreenRegion]  # handle_id → screen region
     _dirty_set: set[str]                 # handle_ids that need redraw
@@ -26531,7 +26533,7 @@ The strategy pattern creates a clean separation between **timing/ordering policy
     order in which data arrived or handles closed.
     """
     strategy_name = "sequential_buffer"
-    
+
     _next_render_index: int = 0          # The declaration index to render next
     _rendered_buffers: dict[int, str]     # index → pre-rendered content (waiting)
     _closed_set: set[int]                # Declaration indices of closed elements
@@ -26579,49 +26581,49 @@ While the `MaterializationStrategy` controls *when* elements are rendered, the `
     - JsonElementRenderer: JSON serialization
     - YamlElementRenderer: YAML serialization
     """
-    
+
     format_name: str                     # "plain", "color", "table", "rich", "json", "yaml"
-    
+
     def render_panel(self, panel: Panel, stream: IO) -> None:
         """Render a panel element to the stream."""
         ...
-    
+
     def render_table(self, table: Table, stream: IO) -> None:
         """Render a table element to the stream."""
         ...
-    
+
     def render_tree(self, tree: Tree, stream: IO) -> None:
         """Render a tree element to the stream."""
         ...
-    
+
     def render_status(self, status: StatusMessage, stream: IO) -> None:
         """Render a status message to the stream."""
         ...
-    
+
     def render_progress(self, progress: ProgressIndicator, stream: IO) -> None:
         """Render a progress indicator to the stream."""
         ...
-    
+
     def render_code(self, code: CodeBlock, stream: IO) -> None:
         """Render a code block to the stream."""
         ...
-    
+
     def render_diff(self, diff: DiffBlock, stream: IO) -> None:
         """Render a diff block to the stream."""
         ...
-    
+
     def render_text(self, text: TextBlock, stream: IO) -> None:
         """Render a text block to the stream."""
         ...
-    
+
     def render_separator(self, separator: Separator, stream: IO) -> None:
         """Render a visual separator to the stream."""
         ...
-    
+
     def render_action_hint(self, hint: ActionHint, stream: IO) -> None:
         """Render an action hint to the stream."""
         ...
-    
+
     def render_element(self, element: OutputElement, stream: IO) -> None:
         """Dispatch to the appropriate render method based on element type.
         
@@ -26643,7 +26645,7 @@ While the `MaterializationStrategy` controls *when* elements are rendered, the `
         handler = dispatch.get(element.element_type)
         if handler:
             handler(element, stream)
-    
+
     def serialize(self, output: StructuredOutput, stream: IO) -> None:
         """Serialize a complete StructuredOutput to the stream.
         
@@ -26652,7 +26654,7 @@ While the `MaterializationStrategy` controls *when* elements are rendered, the `
         output.elements and calls render_element for each.
         """
         ...
-    
+
     def can_render(self, terminal_caps: "TerminalCapabilities") -> bool:
         """Whether this renderer can operate in the given terminal environment."""
         ...
@@ -27280,9 +27282,9 @@ The framework uses a **registry pattern** for format renderers, enabling third-p
         "json"  → (AccumulateMaterializer, JsonElementRenderer), fallback=None
         "yaml"  → (AccumulateMaterializer, YamlElementRenderer), fallback=None
     """
-    
+
     _formats: dict[str, FormatRegistration] = {}
-    
+
     @classmethod
     def register(cls, format_name: str,
                  strategy_factory: Callable[[TerminalCapabilities], MaterializationStrategy],
@@ -27304,7 +27306,7 @@ The framework uses a **registry pattern** for format renderers, enabling third-p
             renderer_factory=renderer_factory,
             fallback=fallback,
         )
-    
+
     @classmethod
     def resolve(cls, format_name: str,
                 terminal_caps: TerminalCapabilities
@@ -27321,33 +27323,33 @@ The framework uses a **registry pattern** for format renderers, enabling third-p
         """
         current = format_name
         visited: set[str] = set()
-        
+
         while current and current not in visited:
             visited.add(current)
             registration = cls._formats.get(current)
             if registration is None:
                 break
-            
+
             renderer = registration.renderer_factory(terminal_caps)
             if renderer.can_render(terminal_caps):
                 strategy = registration.strategy_factory(terminal_caps)
                 strategy.bind(renderer, terminal_caps=terminal_caps)
                 return strategy, renderer
-            
+
             current = registration.fallback
-        
+
         # Ultimate fallback is always plain
         plain = cls._formats["plain"]
         renderer = plain.renderer_factory(terminal_caps)
         strategy = plain.strategy_factory(terminal_caps)
         strategy.bind(renderer, terminal_caps=terminal_caps)
         return strategy, renderer
-    
+
     @classmethod
     def available_formats(cls) -> list[str]:
         """Return all registered format names."""
         return sorted(cls._formats.keys())
-    
+
     @classmethod
     def is_registered(cls, format_name: str) -> bool:
         """Check if a format is registered."""
@@ -27380,7 +27382,7 @@ The framework detects terminal capabilities to guide format resolution, strategy
     supports_alternate_screen: bool      # Supports alternate screen buffer?
     no_color: bool                       # Is NO_COLOR environment variable set?
     term_program: str | None             # TERM_PROGRAM value (e.g., "iTerm2", "vscode")
-    
+
     @classmethod
     def detect(cls) -> "TerminalCapabilities":
         """Auto-detect terminal capabilities from the environment.
@@ -27411,13 +27413,13 @@ Third-party plugins can register custom formats using the registry:
 class CsvElementRenderer(ElementRenderer):
     """Renders tables as CSV, other elements as plain text."""
     format_name = "csv"
-    
+
     def render_table(self, table: Table, stream: IO) -> None:
         writer = csv.writer(stream)
         writer.writerow([col.name for col in table.columns])
         for row in table.rows:
             writer.writerow([row.get(col.name, "") for col in table.columns])
-    
+
     def can_render(self, terminal_caps: TerminalCapabilities) -> bool:
         return True  # CSV works everywhere
 
@@ -27503,7 +27505,7 @@ Example of producer error handling:
     table = session.table("Resources", columns=[
         ColumnDef(name="Name"), ColumnDef(name="Type"), ColumnDef(name="Status"),
     ])
-    
+
     try:
         async for resource in client.list_resources():
             table.add_row({
@@ -27515,7 +27517,7 @@ Example of producer error handling:
         table.close()  # Close with partial data
         session.status(f"Error fetching resources: {e}", level="error")
         return
-    
+
     table.close()
     session.status(f"{table.element.row_count} resources listed", level="ok")
 
@@ -27599,9 +27601,9 @@ The simplest usage — a command that creates elements, populates them synchrono project = await api.get_project(project_name) resources = await api.list_project_resources(project.name) validations = await api.list_project_validations(project.name) - + # --- Build output elements --- - + # Panel: Project details with session.panel("Project Details") as panel: panel.set_entries({ @@ -27616,7 +27618,7 @@ The simplest usage — a command that creates elements, populates them synchrono "Remote": "success" if not project.remote else "info", "Created": "success", }) - + # Table: Linked resources with session.table("Linked Resources", columns=[ ColumnDef(name="Resource", type="string", style_hint="identifier"), @@ -27631,7 +27633,7 @@ The simplest usage — a command that creates elements, populates them synchrono "Sandbox": r.sandbox_strategy, "Read-Only": "yes" if r.read_only else "no", }) - + # Table: Validations with session.table(f"Validations ({len(validations)})", columns=[ ColumnDef(name="ID", type="id", style_hint="identifier"), @@ -27644,7 +27646,7 @@ The simplest usage — a command that creates elements, populates them synchrono "Command": v.command, "Mode": v.mode, }) - + # Status: Final message session.status("Project loaded", level="ok")
@@ -27769,7 +27771,7 @@ A command that streams rows into a table as results arrive from a paginated API.

 async def cmd_resource_list(session: OutputSession, project: str | None) -> None:
     """Implementation of 'agents resource list'."""
-    
+
     # Create the table handle — it will accumulate rows as we stream them
     table = session.table("Resources", columns=[
         ColumnDef(name="Name", type="string", style_hint="identifier"),
@@ -27778,10 +27780,10 @@ A command that streams rows into a table as results arrive from a paginated API.
         ColumnDef(name="Sandbox"),
         ColumnDef(name="Status"),
     ], sort_key="Name")
-    
+
     # Create a progress indicator for the fetch operation
     progress = session.progress("Fetching resources...", indeterminate=True)
-    
+
     # Stream pages from the API
     count = 0
     async for page in api.list_resources_paginated(project=project):
@@ -27794,17 +27796,17 @@ A command that streams rows into a table as results arrive from a paginated API.
                 "Status": resource.status,
             })
             count += 1
-        
+
         # Update progress label with count so far
         progress.set_label(f"Fetching resources... ({count} found)")
-    
+
     # Close the progress indicator (it has served its purpose)
     progress.close()
-    
+
     # Set summary and close the table
     table.set_summary({"total": count})
     table.close()
-    
+
     # Final status
     session.status(f"{count} resources listed", level="ok")
 
@@ -27867,7 +27869,7 @@ This is the key motivating example for the reactive architecture. Two tables are backend resolves each item. """ plan = await api.get_plan(plan_id) - + # Panel: Plan metadata (created and closed synchronously) with session.panel("Plan") as panel: panel.set_entries({ @@ -27882,7 +27884,7 @@ This is the key motivating example for the reactive architecture. Two tables are "Phase": "info", "State": "warning" if plan.state == "processing" else "success", }) - + # Create both table handles BEFORE starting concurrent producers. # Declaration order determines rendering order in sequential formats. resource_table = session.table("Resource Status", columns=[ @@ -27891,7 +27893,7 @@ This is the key motivating example for the reactive architecture. Two tables are ColumnDef(name="Status"), ColumnDef(name="Latency", type="string", alignment="right"), ]) - + tool_table = session.table("Tool Call Log", columns=[ ColumnDef(name="#", type="number", alignment="right"), ColumnDef(name="Tool"), @@ -27899,12 +27901,12 @@ This is the key motivating example for the reactive architecture. Two tables are ColumnDef(name="Result"), ColumnDef(name="Duration", type="string", alignment="right"), ]) - + # --- Run two producers concurrently --- # Each producer writes to its own handle. Neither producer knows # which format is active. The materialization strategy handles # the coordination. - + async def stream_resources(): """Producer A: streams resource status checks.""" async for status in api.stream_resource_statuses(plan.id): @@ -27915,7 +27917,7 @@ This is the key motivating example for the reactive architecture. Two tables are "Latency": f"{status.latency_ms}ms", }) resource_table.close() - + async def stream_tool_calls(): """Producer B: streams tool call results.""" async for call in api.stream_tool_calls(plan.id): @@ -27927,10 +27929,10 @@ This is the key motivating example for the reactive architecture. Two tables are "Duration": f"{call.duration_ms}ms", }) tool_table.close() - + # Launch both producers concurrently await asyncio.gather(stream_resources(), stream_tool_calls()) - + # Final status session.status(f"Plan {plan_id} status retrieved", level="ok") @@ -28051,7 +28053,7 @@ A command that executes a multi-step process with a progress indicator, where so async def cmd_plan_execute(session: OutputSession, plan_id: str) -> None: """Implementation of 'agents plan execute <plan_id>'.""" plan = await api.get_plan(plan_id) - + # Panel: Execution metadata with session.panel("Execution") as panel: panel.set_entries({ @@ -28061,7 +28063,7 @@ A command that executes a multi-step process with a progress indicator, where so "Worker": plan.worker, "Started": datetime.now().strftime("%H:%M:%S"), }) - + # Progress indicator with named steps progress = session.progress("Executing plan", total=4, steps=[ "Collect context", @@ -28069,33 +28071,33 @@ A command that executes a multi-step process with a progress indicator, where so "Build changeset", "Validate", ]) - + # Step 1: Collect context progress.set_step_status("Collect context", "active") context = await api.collect_context(plan.id) progress.set_step_status("Collect context", "done") progress.set_progress(1, 4) - + # Step 2: Run tools (parallel sub-operations) progress.set_step_status("Run tools", "active") tool_results = await api.run_tools(plan.id, context) progress.set_step_status("Run tools", "done") progress.set_progress(2, 4) - + # Step 3: Build changeset progress.set_step_status("Build changeset", "active") changeset = await api.build_changeset(plan.id, tool_results) progress.set_step_status("Build changeset", "done") progress.set_progress(3, 4) - + # Step 4: Validate progress.set_step_status("Validate", "active") validation = await api.validate_changeset(plan.id, changeset) progress.set_step_status("Validate", "done") progress.set_progress(4, 4) - + progress.close() - + # Summary panel with session.panel("Strategy Summary") as panel: panel.set_entries({ @@ -28105,7 +28107,7 @@ A command that executes a multi-step process with a progress indicator, where so "Estimated Files": f"~{changeset.file_count}", "Risk": changeset.risk_level, }) - + # Final status if validation.passed: session.status("Execution complete — all validations passed", level="ok") @@ -28177,7 +28179,7 @@ A command where one of multiple concurrent producers fails, demonstrating gracef async def cmd_resource_verify(session: OutputSession, project: str) -> None: """Verify all resources in a project. Some verifications may fail.""" resources = await api.list_project_resources(project) - + # Create a table that will be populated concurrently results_table = session.table("Verification Results", columns=[ ColumnDef(name="Resource", style_hint="identifier"), @@ -28186,14 +28188,14 @@ A command where one of multiple concurrent producers fails, demonstrating gracef ColumnDef(name="Status"), ColumnDef(name="Detail"), ]) - + # Progress indicator progress = session.progress( "Verifying resources", total=len(resources), steps=[r.name for r in resources], ) - + # Verify each resource concurrently async def verify_one(resource): progress.set_step_status(resource.name, "active") @@ -28220,21 +28222,21 @@ A command where one of multiple concurrent producers fails, demonstrating gracef }) progress.set_step_status(resource.name, "error") progress.increment() - + # Launch all verifications concurrently await asyncio.gather( *[verify_one(r) for r in resources], return_exceptions=True, # Don't fail fast — collect all results ) - + progress.close() results_table.close() - + # Summarize snapshot = results_table.element pass_count = sum(1 for r in snapshot.rows if r["Status"] == "pass") fail_count = sum(1 for r in snapshot.rows if r["Status"] in ("fail", "error")) - + if fail_count == 0: session.status(f"All {pass_count} resources verified", level="ok") else: @@ -28500,10 +28502,10 @@ The confidence score is computed from multiple factors: 'risk_assessment': self.evaluate_risk(decision), 'invariant_complexity': self.analyze_invariants(decision) } - + confidence = self.compute_confidence(factors) # Returns 0.0–1.0 threshold = profile.get_threshold(decision.flag) # e.g., execute_command - + if confidence >= threshold: # Confidence meets or exceeds the profile threshold — proceed return ProceedAutonomously(decision, confidence) @@ -28676,28 +28678,28 @@ The "affected subtree" — all decisions and child plans invalidated by a correc affected_decisions = {target_decision_id} affected_plans = set() queue = [target_decision_id] - + while queue: current = queue.pop(0) - + # Follow structural tree children children = query("SELECT decision_id FROM decisions " "WHERE parent_decision_id = :current AND superseded_by IS NULL") - + # Follow influence DAG dependents dependents = query("SELECT downstream_ref FROM decision_dependencies " "WHERE upstream_decision_id = :current AND dependency_type = 'decision'") - + for d in children | dependents: if d not in affected_decisions: affected_decisions.add(d) queue.append(d) - + # Collect affected child plans child_plans = query("SELECT downstream_ref FROM decision_dependencies " "WHERE upstream_decision_id = :current AND dependency_type = 'plan'") affected_plans.update(child_plans) - + return affected_decisions, affected_plans @@ -30763,7 +30765,7 @@ All CleverAgents configuration files share a consistent set of design principles 2. **Namespace/Name convention**: Every configurable entity follows the `/` naming convention. The `local/` namespace is reserved for local-only items. User namespaces (`/`) and organization namespaces (`/`) are stored on the server. Built-in entities use provider namespaces (e.g., `openai/`, `anthropic/`). -3. **Config-as-complete-definition pattern**: For entity registration commands (`actor add`, `skill add`, `tool add`, `validation add`, `resource type add`, `action create`, `automation-profile add`), the YAML configuration file is the **sole source of truth** — the `--config` file fully defines the entity and no CLI override flags are accepted. For runtime commands that reference existing entities (e.g., `plan use`), CLI options may override entity defaults to customize behavior per-invocation. +3. **Config-as-complete-definition pattern**: For entity registration commands (`actor add`, `skill add`, `tool add`, `validation add`, `resource type add`, `action create`, `automation-profile add`), the YAML configuration file is the **sole source of truth** — the `--config` file fully defines the entity. For runtime commands that reference existing entities (e.g., `plan use`), CLI options may override entity defaults to customize behavior per-invocation. **Exception**: `validation add` accepts `--required`/`--informational` flags that override the YAML `mode` field, as described in the Validation Mode section under Core Concepts. 4. **Environment variable interpolation**: All configuration files support `${ENV_VAR}` and `${ENV_VAR:default_value}` syntax for environment variable interpolation. If a variable is not set and no default is provided, an error is raised. Boolean strings (`true`/`false`) and numeric strings are automatically converted to their native types. @@ -36267,8 +36269,7 @@ This section presents complete, end-to-end workflow examples showing how CleverA # Register a required validation (tests must pass) $ agents validation add \ --config validations/unit-tests.yaml \ - --required \ - local/unit-tests + --required ╭─ Validation Registered ─────────────────────────────────────╮ │ Name: local/unit-tests │ @@ -40793,7 +40794,7 @@ Create `actions/build-notification-system.yaml`: - Message Queue: notification event schemas, routing rules - Worker: email/SMS/push delivery workers with retry logic - Frontend: notification center UI, preference settings, real-time updates - + The system must be designed for reliability (at-least-once delivery), scalability (async processing via queue), and user control (per-channel preferences with quiet hours). diff --git a/features/steps/tdd_validation_add_required_flag_steps.py b/features/steps/tdd_validation_add_required_flag_steps.py index f592aa53c..15bbaa3d8 100644 --- a/features/steps/tdd_validation_add_required_flag_steps.py +++ b/features/steps/tdd_validation_add_required_flag_steps.py @@ -3,30 +3,16 @@ ``agents validation add`` missing ``--required`` flag. This test captures bug #1038. The specification -(``docs/specification.md`` line 22334) states that the validation mode +(``docs/specification.md`` line 22339) states that the validation mode can be set "via ``--required``/``--informational`` on ``agents validation add``", and numerous workflow examples throughout the spec use -``--required``. However, the current implementation of the ``add`` -command in ``cleveragents.cli.commands.validation`` does not define -``--required`` or ``--informational`` options, so passing either flag -causes a ``NoSuchOption`` error at runtime. +``--required``. The fix adds ``--required`` and ``--informational`` as +mutually exclusive boolean options on the ``add`` command that override +the ``mode`` field in the YAML config when specified. -NOTE -- Spec Contradiction: - Rui Hu's investigation (issue #1038 comment #70755) found that the - formal CLI reference (specification.md lines 9279-9290) does NOT - include --required/--informational flags -- they appear only in - walkthrough examples and specification.md line 22334. Additionally, - specification.md line 30761 states: "For entity registration commands - (actor add, skill add, tool add, validation add, ...), the YAML - configuration file is the sole source of truth -- the --config file - fully defines the entity and no CLI override flags are accepted." - The resolution may be to add the flags to the CLI OR to clean up the - spec. See #1038. - -The ``@tdd_expected_fail`` tag on the scenarios inverts the result: these -tests *pass* CI because the underlying assertions *fail* (proving the bug -exists). Once the fix for #1038 is merged and the flags are implemented, -the ``@tdd_expected_fail`` tag must be removed so the tests run normally. +These scenarios were originally tagged ``@tdd_expected_fail`` while the +bug was unfixed. Now that the fix is in place, the tag has been removed +and the tests run normally as permanent regression guards. """ from __future__ import annotations @@ -158,15 +144,8 @@ def step_tdd_1038_temp_config_with_mode(context: Context, mode: str) -> None: def step_tdd_1038_add_required(context: Context) -> None: """Invoke ``agents validation add --config --required``. - Per the spec, the ``--required`` flag should set the validation mode to - ``required``, overriding whatever mode the YAML config defines. The - current implementation does NOT have this flag, so Typer raises - ``NoSuchOption`` -- which is the bug this test captures. - - Note: the positional NAME argument shown in the original bug report - (#1038) is omitted here because NAME handling is a separate spec - inconsistency not under test in this scenario. This test focuses - solely on the missing ``--required``/``--informational`` flags. + Per the spec, the ``--required`` flag sets the validation mode to + ``required``, overriding whatever mode the YAML config defines. """ with patch(_PATCH_SVC, return_value=context.tdd1038_mock_service): context.tdd1038_result = context.tdd1038_runner.invoke( @@ -186,9 +165,8 @@ def step_tdd_1038_add_required(context: Context) -> None: def step_tdd_1038_add_informational(context: Context) -> None: """Invoke ``agents validation add --config --informational``. - Per the spec (specification.md line 22334), ``--informational`` sets - the mode to ``informational``. This flag is also missing from the - current CLI. + Per the spec (specification.md line 22339), ``--informational`` sets + the mode to ``informational``. """ with patch(_PATCH_SVC, return_value=context.tdd1038_mock_service): context.tdd1038_result = context.tdd1038_runner.invoke( @@ -204,6 +182,29 @@ def step_tdd_1038_add_informational(context: Context) -> None: ) +@when("I tdd 1038 invoke validation add with both flags") +def step_tdd_1038_add_both_flags(context: Context) -> None: + """Invoke ``agents validation add`` with both ``--required`` and + ``--informational``. + + These flags are mutually exclusive; the CLI should reject the + invocation and abort. + """ + with patch(_PATCH_SVC, return_value=context.tdd1038_mock_service): + context.tdd1038_result = context.tdd1038_runner.invoke( + validation_app, + [ + "add", + "--config", + context.tdd1038_config_path, + "--required", + "--informational", + "--format", + "plain", + ], + ) + + # --------------------------------------------------------------------------- # Then steps # --------------------------------------------------------------------------- @@ -211,12 +212,7 @@ def step_tdd_1038_add_informational(context: Context) -> None: @then("the tdd 1038 CLI result should succeed") def step_tdd_1038_result_succeed(context: Context) -> None: - """Assert the CLI invocation completed successfully (exit code 0). - - This assertion FAILS while bug #1038 is present because ``--required`` - and ``--informational`` are not recognised options, causing a non-zero - exit. The ``@tdd_expected_fail`` tag inverts this failure into a pass. - """ + """Assert the CLI invocation completed successfully (exit code 0).""" result = context.tdd1038_result assert result is not None, "No CLI result captured" assert result.exit_code == 0, ( @@ -224,19 +220,33 @@ def step_tdd_1038_result_succeed(context: Context) -> None: ) +@then("the tdd 1038 CLI result should be aborted") +def step_tdd_1038_result_aborted(context: Context) -> None: + """Assert the CLI invocation was aborted (non-zero exit code). + + When both ``--required`` and ``--informational`` are passed, the CLI + should print an error message and abort. + """ + result = context.tdd1038_result + assert result is not None, "No CLI result captured" + assert result.exit_code != 0, ( + f"Expected non-zero exit code but got {result.exit_code}. " + f"Output:\n{result.output}" + ) + assert "mutually exclusive" in result.output, ( + f"Expected 'mutually exclusive' in output, got:\n{result.output}" + ) + + @then('the tdd 1038 registered validation mode should be "{expected_mode}"') def step_tdd_1038_mode_check(context: Context, expected_mode: str) -> None: """Assert the output contains the expected mode and the service received it. - When the bug is fixed, the CLI should accept the ``--required`` / - ``--informational`` flag and the rendered output should include the - mode accordingly. - - In addition to checking the CLI output, this step verifies that - ``register_tool`` was called with a Validation object whose ``mode`` - attribute matches the expected value. This prevents a false positive - where the mock always returns a hard-coded mode regardless of whether - the CLI actually forwarded the flag to the service layer. + This step verifies that ``register_tool`` was called with a Validation + object whose ``mode`` attribute matches the expected value. This + prevents a false positive where the mock always returns a hard-coded + mode regardless of whether the CLI actually forwarded the flag to the + service layer. """ result = context.tdd1038_result assert result is not None, "No CLI result captured" diff --git a/features/tdd_validation_add_required_flag.feature b/features/tdd_validation_add_required_flag.feature index 1856c09aa..e99e892fd 100644 --- a/features/tdd_validation_add_required_flag.feature +++ b/features/tdd_validation_add_required_flag.feature @@ -1,36 +1,20 @@ # TDD bug-capture test for bug #1038. # -# The specification (docs/specification.md line 22334) states that the +# The specification (docs/specification.md line 22339) states that the # validation mode can be set "via --required/--informational on agents # validation add", and numerous workflow examples in the spec use the -# --required flag. However, the current implementation of the ``add`` -# command in ``cleveragents.cli.commands.validation`` does not accept -# --required or --informational flags, causing a ``NoSuchOption`` error -# at runtime. +# --required flag. # -# NOTE — Spec Contradiction: -# Rui Hu's investigation (issue #1038 comment #70755) found that the -# formal CLI reference (specification.md lines 9279-9290) does NOT include -# --required/--informational flags — they appear only in walkthrough -# examples and specification.md line 22334. Additionally, -# specification.md line 30761 states: "For entity registration commands -# (actor add, skill add, tool add, validation add, …), the YAML -# configuration file is the sole source of truth — the --config file -# fully defines the entity and no CLI override flags are accepted." -# The resolution may be to add the flags to the CLI OR to clean up the -# spec. See #1038. +# Bug #1038 reported that the ``add`` command did not accept these flags, +# causing a ``NoSuchOption`` error. The fix adds --required and +# --informational as mutually exclusive boolean options that override the +# mode field in the YAML config when specified. # -# These scenarios assert the CORRECT expected behavior. Because the bug is -# still present, the underlying assertions will fail — the @tdd_expected_fail -# tag inverts the result so the test suite passes CI. Once bug #1038 is fixed -# and the --required/--informational flags are implemented, the -# @tdd_expected_fail tag must be removed so the test runs normally. -# -# NOTE — Deferred edge case: mutual exclusivity of --required and -# --informational when both are passed simultaneously is not tested here. -# That edge case is deferred to the bug-fix PR for #1038. +# These scenarios were originally tagged @tdd_expected_fail while the bug +# was unfixed. Now that the fix is in place, the tag has been removed and +# the tests run normally as permanent regression guards. -@tdd_expected_fail @tdd_issue @tdd_issue_1038 +@tdd_issue @tdd_issue_1038 Feature: Bug #1038 — validation add missing --required flag As a user of the CleverAgents CLI I want the ``agents validation add`` command to accept a ``--required`` flag @@ -61,3 +45,7 @@ Feature: Bug #1038 — validation add missing --required flag When I tdd 1038 invoke validation add with --informational flag Then the tdd 1038 CLI result should succeed And the tdd 1038 registered validation mode should be "informational" + + Scenario: Passing both --required and --informational is rejected + When I tdd 1038 invoke validation add with both flags + Then the tdd 1038 CLI result should be aborted diff --git a/robot/helper_tdd_validation_required_flag.py b/robot/helper_tdd_validation_required_flag.py index 4ad686586..ed528ba0c 100644 --- a/robot/helper_tdd_validation_required_flag.py +++ b/robot/helper_tdd_validation_required_flag.py @@ -2,29 +2,18 @@ Each subcommand exercises the ``agents validation add`` CLI path via ``typer.testing.CliRunner`` to reproduce bug #1038. The specification -(``docs/specification.md`` line 22334) states that the validation mode can +(``docs/specification.md`` line 22339) states that the validation mode can be set "via ``--required``/``--informational`` on ``agents validation add``", and numerous workflow examples in the spec use the ``--required`` flag. -However, the current implementation of the ``add`` command in -``cleveragents.cli.commands.validation`` does not define ``--required`` or -``--informational`` options, causing a ``NoSuchOption`` error at runtime. + +Bug #1038 reported that the ``add`` command did not accept these flags, +causing a ``NoSuchOption`` error. The fix adds ``--required`` and +``--informational`` as mutually exclusive boolean options that override the +``mode`` field in the YAML config when specified. The helper reports the **real** outcome: it exits 0 and prints the sentinel when the expected behaviour is observed (bug fixed), and exits 1 when the -bug is still present. The ``tdd_expected_fail_listener`` on the Robot side -handles pass/fail inversion while the bug remains open. - -NOTE -- Spec Contradiction: - Rui Hu's investigation (issue #1038 comment #70755) found that the - formal CLI reference (specification.md lines 9279-9290) does NOT include - --required/--informational flags -- they appear only in walkthrough - examples and specification.md line 22334. Additionally, - specification.md line 30761 states: "For entity registration commands - (actor add, skill add, tool add, validation add, ...), the YAML - configuration file is the sole source of truth -- the --config file - fully defines the entity and no CLI override flags are accepted." - The resolution may be to add the flags to the CLI OR to clean up the - spec. See #1038. +bug is still present. This test was written to capture bug #1038 per ticket #1102. """ @@ -287,6 +276,50 @@ def _check_informational_overrides_config() -> None: _safe_unlink(config_path) +def _check_both_flags_rejected() -> None: + """Invoke with both ``--required`` and ``--informational`` simultaneously. + + These flags are mutually exclusive; the CLI should reject the invocation + and abort. Exits 0 with sentinel when the rejection occurs correctly. + Exits 1 when the CLI does not reject the combination. + """ + config_path: str = _create_yaml_config() + try: + mock_service: MagicMock = MagicMock() + mock_service.register_tool.side_effect = lambda v: v + + with patch(_PATCH_SVC, return_value=mock_service): + result = runner.invoke( + validation_app, + [ + "add", + "--config", + config_path, + "--required", + "--informational", + "--format", + "plain", + ], + ) + + if result.exit_code == 0: + _fail( + f"validation add accepted both --required and --informational " + f"(should have been rejected).\n" + f"Output: {result.output}" + ) + + if "mutually exclusive" not in result.output: + _fail( + f"Expected 'mutually exclusive' in error output.\n" + f"Output: {result.output}" + ) + + print("tdd-validation-both-flags-rejected-ok") + finally: + _safe_unlink(config_path) + + # --------------------------------------------------------------------------- # Dispatcher # --------------------------------------------------------------------------- @@ -296,6 +329,7 @@ _COMMANDS: dict[str, Callable[[], None]] = { "check-informational": _check_informational, "check-required-overrides-config": _check_required_overrides_config, "check-informational-overrides-config": _check_informational_overrides_config, + "check-both-flags-rejected": _check_both_flags_rejected, } if __name__ == "__main__": diff --git a/robot/tdd_validation_required_flag.robot b/robot/tdd_validation_required_flag.robot index ec5ff2390..e0c07ece8 100644 --- a/robot/tdd_validation_required_flag.robot +++ b/robot/tdd_validation_required_flag.robot @@ -1,22 +1,16 @@ *** Settings *** -Documentation TDD Bug #1038 — validation add missing --required/--informational flags +Documentation TDD Bug #1038 — validation add --required/--informational flags ... Integration smoke tests verifying that the ``agents validation add`` ... command accepts ``--required`` and ``--informational`` flags as described -... in the specification (specification.md line 22334). The current -... implementation does not define these flags, causing a ``NoSuchOption`` -... error at runtime. +... in the specification (specification.md line 22339). ... -... NOTE — Spec Contradiction: The formal CLI reference -... (specification.md lines 9279-9290) does NOT include these flags — they -... appear only in walkthrough examples and specification.md line 22334. -... Additionally, specification.md line 30761 states: "For entity registration -... commands (actor add, skill add, tool add, validation add, …), the YAML -... configuration file is the sole source of truth." The resolution may be -... to add the flags to the CLI OR to clean up the spec. See #1038. +... Bug #1038 reported that these flags were missing from the CLI, +... causing a ``NoSuchOption`` error. The fix adds them as mutually +... exclusive boolean options that override the YAML config mode. ... -... Tests are tagged tdd_expected_fail so CI passes via result inversion -... while the bug remains open. Once bug #1038 is fixed, the -... tdd_expected_fail tag must be removed. +... These tests were originally tagged tdd_expected_fail while the bug +... was unfixed. Now that the fix is in place, the tag has been removed +... and the tests run normally as permanent regression guards. Resource ${CURDIR}/common.resource Suite Setup Setup Test Environment Suite Teardown Cleanup Test Environment @@ -29,7 +23,7 @@ TDD Validation Add Required Flag Accepted [Documentation] Verify that ``validation add --config --required`` ... is accepted by the CLI and sets the validation mode to ... ``required``. - [Tags] tdd_expected_fail tdd_issue tdd_issue_1038 + [Tags] tdd_issue tdd_issue_1038 ${result}= Run Process ${PYTHON} ${HELPER} check-required cwd=${WORKSPACE} timeout=30s on_timeout=kill Log ${result.stdout} Log ${result.stderr} @@ -40,7 +34,7 @@ TDD Validation Add Informational Flag Accepted [Documentation] Verify that ``validation add --config --informational`` ... is accepted by the CLI and sets the validation mode to ... ``informational``. - [Tags] tdd_expected_fail tdd_issue tdd_issue_1038 + [Tags] tdd_issue tdd_issue_1038 ${result}= Run Process ${PYTHON} ${HELPER} check-informational cwd=${WORKSPACE} timeout=30s on_timeout=kill Log ${result.stdout} Log ${result.stderr} @@ -51,7 +45,7 @@ TDD Validation Add Required Flag Overrides YAML Config [Documentation] Verify that ``--required`` overrides a YAML config that ... specifies ``mode: informational``. Both CLI output and ... the service layer should reflect ``mode: required``. - [Tags] tdd_expected_fail tdd_issue tdd_issue_1038 + [Tags] tdd_issue tdd_issue_1038 ${result}= Run Process ${PYTHON} ${HELPER} check-required-overrides-config cwd=${WORKSPACE} timeout=30s on_timeout=kill Log ${result.stdout} Log ${result.stderr} @@ -62,9 +56,19 @@ TDD Validation Add Informational Flag Overrides YAML Config [Documentation] Verify that ``--informational`` overrides a YAML config ... that specifies ``mode: required``. Both CLI output and ... the service layer should reflect ``mode: informational``. - [Tags] tdd_expected_fail tdd_issue tdd_issue_1038 + [Tags] tdd_issue tdd_issue_1038 ${result}= Run Process ${PYTHON} ${HELPER} check-informational-overrides-config cwd=${WORKSPACE} timeout=30s on_timeout=kill Log ${result.stdout} Log ${result.stderr} Should Be Equal As Integers ${result.rc} 0 Should Contain ${result.stdout} tdd-validation-informational-overrides-config-ok + +TDD Validation Add Both Flags Rejected + [Documentation] Verify that passing both ``--required`` and ``--informational`` + ... simultaneously is rejected with a mutually-exclusive error. + [Tags] tdd_issue tdd_issue_1038 + ${result}= Run Process ${PYTHON} ${HELPER} check-both-flags-rejected cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-validation-both-flags-rejected-ok diff --git a/src/cleveragents/cli/commands/validation.py b/src/cleveragents/cli/commands/validation.py index d931b97b3..ee6a060a6 100644 --- a/src/cleveragents/cli/commands/validation.py +++ b/src/cleveragents/cli/commands/validation.py @@ -11,12 +11,14 @@ subtypes) and their lifecycle attachments to resources. | ``agents validation attach`` | Attach validation to a resource | | ``agents validation detach`` | Detach a validation attachment | -## Config-Only Add +## Config-Based Add -Validations are registered **exclusively** via a YAML configuration file: +Validations are registered via a YAML configuration file, with optional +``--required`` or ``--informational`` flags to override the mode: ```bash agents validation add --config ./validations/coverage-check.yaml +agents validation add --config ./validations/coverage-check.yaml --required ``` ### YAML Configuration File @@ -60,7 +62,7 @@ from cleveragents.core.exceptions import ( NotFoundError, ValidationError, ) -from cleveragents.domain.models.core.tool import Validation +from cleveragents.domain.models.core.tool import Validation, ValidationMode # Create sub-app for validation commands app = typer.Typer(help="Manage validations (pass/fail tools) and resource attachments.") @@ -187,6 +189,20 @@ def add( exists=False, ), ], + required: Annotated[ + bool, + typer.Option( + "--required", + help="Set validation mode to 'required' (overrides YAML config)", + ), + ] = False, + informational: Annotated[ + bool, + typer.Option( + "--informational", + help="Set validation mode to 'informational' (overrides YAML config)", + ), + ] = False, update: Annotated[ bool, typer.Option("--update", help="Update if validation already exists"), @@ -198,13 +214,23 @@ def add( ) -> None: """Register a new validation from a YAML configuration file. - Validations are created ONLY via ``--config ``. + The validation is fully defined by the YAML configuration file + specified with ``--config``. Optionally, ``--required`` or + ``--informational`` can override the ``mode`` field in the YAML. Examples: agents validation add --config ./validations/coverage-check.yaml + agents validation add --config ./validations/coverage-check.yaml --required agents validation add --config ./validations/coverage-check.yaml --update """ try: + if required and informational: + console.print( + "[red]Error:[/red] --required and --informational are " + "mutually exclusive" + ) + raise typer.Abort() + if not config.exists(): raise FileNotFoundError(f"Config file not found: {config}") @@ -213,6 +239,12 @@ def add( if not isinstance(config_dict, dict): raise ValueError("YAML config must be a mapping") + # Apply CLI mode override before building the Validation object. + if required: + config_dict["mode"] = ValidationMode.REQUIRED.value + elif informational: + config_dict["mode"] = ValidationMode.INFORMATIONAL.value + validation = Validation.from_config(config_dict) service = _get_tool_registry_service()