Restore constrained Codex tool transport with reversible admission

docs/reference/tools.md

@@ -150,7 +150,7 @@ Source: [`src/tools/defs/media_scheduling.py`](https://github.com/Calmingstorm/O
 | <code>cron</code> | string | No | Cron expression for recurring tasks (e.g. &#x27;0 9 &#42; &#42; &#42;&#x27; = daily 9am). Omit for one-time. |
 | <code>cron&#95;timezone</code> | string | No | IANA timezone for the cron expression (e.g. &#x27;America/New&#95;York&#x27;). The task fires on that timezone&#x27;s wall clock across DST. Defaults to UTC. |
 | <code>run&#95;at</code> | string | No | Offset-aware ISO datetime for one-time tasks (e.g. &#x27;2026-03-20T09:00:00Z&#x27;). Use parse&#95;time to convert natural language. Omit for recurring. |
-| <code>trigger</code> | object | No | Webhook trigger (AND logic). E.g. &#123;&quot;source&quot;: &quot;github&quot;, &quot;event&quot;: &quot;push&quot;, &quot;repo&quot;: &quot;myproject&quot;&#125;. |
+| <code>trigger</code> | object | No | Webhook trigger (AND logic). E.g. &#123;&quot;source&quot;: &quot;github&quot;, &quot;event&quot;: &quot;push&quot;, &quot;repo&quot;: &quot;myproject&quot;&#125;.<br>Constraints: <code>&#123;&quot;additionalProperties&quot;:false&#125;</code> |
 | <code>trigger.source</code> | string | No | Webhook source to match<br>Constraints: <code>&#123;&quot;enum&quot;:&#91;&quot;gitea&quot;,&quot;grafana&quot;,&quot;generic&quot;,&quot;github&quot;,&quot;gitlab&quot;&#93;&#125;</code> |
 | <code>trigger.event</code> | string | No | Event type (e.g. &#x27;push&#x27;, &#x27;pull&#95;request&#x27;, &#x27;alert&#x27;) |
 | <code>trigger.repo</code> | string | No | Repository name substring (case-insensitive) |
@@ -194,7 +194,11 @@ No input properties.
 | <code>cron</code> | string | No | New cron expression (replaces previous timing) |
 | <code>cron&#95;timezone</code> | string | No | IANA timezone for the cron expression (e.g. &#x27;America/New&#95;York&#x27;). Defaults to UTC. |
 | <code>run&#95;at</code> | string | No | New offset-aware ISO datetime for one-time (replaces previous timing) |
-| <code>trigger</code> | object | No | New webhook trigger (replaces previous timing) |
+| <code>trigger</code> | object | No | New webhook trigger (replaces previous timing)<br>Constraints: <code>&#123;&quot;additionalProperties&quot;:false&#125;</code> |
+| <code>trigger.source</code> | string | No | <br>Constraints: <code>&#123;&quot;enum&quot;:&#91;&quot;gitea&quot;,&quot;grafana&quot;,&quot;generic&quot;,&quot;github&quot;,&quot;gitlab&quot;&#93;&#125;</code> |
+| <code>trigger.event</code> | string | No | — |
+| <code>trigger.repo</code> | string | No | — |
+| <code>trigger.alert&#95;name</code> | string | No | — |
 | <code>message</code> | string | No | New message (for reminder actions) |
 | <code>tool&#95;name</code> | string | No | New tool name (for check actions) |
 | <code>tool&#95;input</code> | object | No | New tool input parameters |
@@ -899,7 +903,9 @@ Source: [`src/tools/defs/integrations_email.py`](https://github.com/Calmingstorm
 | <code>url</code> | string | Yes | URL to probe (http or https) |
 | <code>host</code> | string | No | Host alias to run curl from (omit to run locally) |
 | <code>method</code> | string | No | HTTP method (default GET)<br>Constraints: <code>&#123;&quot;enum&quot;:&#91;&quot;GET&quot;,&quot;POST&quot;,&quot;PUT&quot;,&quot;DELETE&quot;,&quot;PATCH&quot;,&quot;HEAD&quot;,&quot;OPTIONS&quot;&#93;&#125;</code> |
-| <code>headers</code> | object | No | Request headers as key-value pairs (e.g. &#123;&quot;Authorization&quot;: &quot;Bearer tok&quot;&#125;) |
+| <code>headers</code> | array&lt;object&gt; | No | Request headers as name/value entries. Case-insensitive duplicate names are rejected. |
+| <code>headers&#91;&#93;.name</code> | string | Yes | — |
+| <code>headers&#91;&#93;.value</code> | string | Yes | — |
 | <code>body</code> | string | No | Request body string (for POST/PUT/PATCH). Max 50KB. |
 | <code>timeout</code> | integer | No | Request timeout in seconds (default 30, max 120) |
 | <code>follow&#95;redirects</code> | boolean | No | Follow HTTP redirects (default true) |
@@ -949,7 +955,7 @@ Severity &#x27;critical&#x27; (default), &#x27;warn&#x27;, or &#x27;info&#x27;.
 | <code>checks</code> | array&lt;object&gt; | Yes | List of validation checks (max 25). |
 | <code>checks&#91;&#93;.type</code> | string | Yes | http&#124;port&#124;service&#124;process&#124;log&#95;absent&#124;log&#95;present&#124;command |
 | <code>checks&#91;&#93;.target</code> | string | Yes | — |
-| <code>checks&#91;&#93;.expected</code> | any | No | Type-specific expectation (int, string, list) |
+| <code>checks&#91;&#93;.expected</code> | anyOf(integer, string, array&lt;integer&gt;, array&lt;string&gt;) | No | Type-specific expectation: integer, string, integer list, or string list<br>Constraints: <code>&#123;&quot;anyOf&quot;:&#91;&#123;&quot;type&quot;:&quot;integer&quot;&#125;,&#123;&quot;type&quot;:&quot;string&quot;&#125;,&#123;&quot;type&quot;:&quot;array&quot;,&quot;items&quot;:&#123;&quot;type&quot;:&quot;integer&quot;&#125;&#125;,&#123;&quot;type&quot;:&quot;array&quot;,&quot;items&quot;:&#123;&quot;type&quot;:&quot;string&quot;&#125;&#125;&#93;&#125;</code> |
 | <code>checks&#91;&#93;.severity</code> | string | No | critical (default) &#124; warn &#124; info |
 | <code>checks&#91;&#93;.host</code> | string | No | — |
 | <code>checks&#91;&#93;.compare</code> | string | No | — |

docs/strict-wire-lowerings.md

@@ -0,0 +1,85 @@
+# Strict wire lowerings
+
+This document inventories built-in wire shapes that differ from their existing
+runtime interfaces. Lowerings happen before effectful dispatch and must be
+tested at both the wire and runtime boundaries.
+
+## `http_probe.headers`
+
+- Canonical path: `http_probe.headers`, historically an arbitrary-key object.
+- Wire form: array of closed `{name: string, value: string}` objects.
+- Runtime: `build_http_probe_command` converts records to the existing header
+  dictionary before curl command construction; dictionary callers remain
+  supported. Case-folded duplicate names, malformed records, and non-string
+  names/values are rejected before request execution. Existing curl header
+  validation and output redaction remain in place.
+- Fixtures: `tests/test_strict_wire_defs.py` covers schema, list and legacy
+  dict positive cases, case-insensitive duplicate names and malformed records.
+
+## Schedule triggers
+
+- Canonical paths: `schedule_task.trigger` and `update_schedule.trigger`.
+- Wire form: shared closed object with `source`, `event`, `repo`, and
+  `alert_name`; source retains the five supported values.
+- Semantics remain AND across supplied conditions; partial conditions without
+  `source` remain valid. Existing scheduler runtime rejects an empty trigger.
+- Fixtures: `tests/test_strict_wire_defs.py` asserts both definitions use the
+  same closed four-field shape.
+
+## `validate_action.checks[].expected`
+
+- Canonical path: `validate_action.checks[].expected`.
+- Wire form: integer, string, integer array, or string array, with typed items.
+- Runtime: `parse_checks` rejects unsupported types and incompatible typed
+  expectations before any validation command runs. HTTP expectations are
+  status integers or digit strings; service status lists contain strings.
+  Existing scalar stringification paths for command checks are preserved.
+- Omitted expectations, explicit expectations, and check-type defaults remain
+  distinct: only an omitted `expected` selects the existing built-in default.
+- Fixtures: `tests/test_strict_wire_defs.py` covers integer lists, invalid
+  mixed HTTP values, invalid service list values, and command compatibility.
+
+## Nested tool payloads
+
+- Canonical paths: `schedule_task.tool_input`, `schedule_task.steps[].tool_input`,
+  `update_schedule.tool_input`, `update_schedule.steps[].tool_input`,
+  `delegate_task.steps[].tool_input`, and `invoke_skill.input`.
+- Wire form: JSON-encoded object in a string field. The request-local acceptance
+  boundary decodes once, rejects malformed JSON, duplicate keys, and non-object
+  roots, then validates the selected tool's canonical schema and authorization.
+  Canonical objects, including meaningful nested nulls, reach persistence and
+  dispatch. Workflow templates are decoded before substitution; concrete fields
+  are checked at admission, unresolved placeholders after substitution. Existing
+  stored jobs do not acquire new retroactive validation.
+- Fixtures: `tests/test_strict_nested_payload.py` and
+  `tests/test_strict_tool_adapter.py` cover round trips, malformed data,
+  target validation, and unresolved placeholders.
+
+## `computer_*` explicit lowerings
+
+The canonical computer contracts remain in `src/tools/defs/computer.py`.
+The request-local adapter owns the wrapper lowering: a closed outer object
+contains `payload`, whose schema is a nested per-operation union with const
+operation selectors. It must not use a root `anyOf`, and must reject ambiguous
+matches rather than choosing a first branch. Canonical schemas stay unchanged.
+
+The following unsupported wire constraints require explicit computer-only
+lowering. Each wire relaxation must be enforced by the canonical JSON Schema
+tripwire before dispatch; these are not generic recursive keyword deletions.
+
+| Canonical schema constraint | Wire representation | Pre-effect validator |
+| --- | --- | --- |
+| `computer_act.properties.modifiers.uniqueItems`, `computer_act.properties.steps.items.properties.modifiers.uniqueItems`, `computer_act.properties.strokes.items.properties.modifiers.uniqueItems` | Omitted from wire array schemas | Canonical computer payload validator rejects duplicate modifiers |
+| `computer_observe.properties.task_context.minProperties` | Omitted from wire object schema | Canonical validator rejects an empty task context |
+| `computer_act.properties.key.allOf[*].not`, and the matching nested `computer_act.properties.steps.items.properties.key.allOf[*].not` | Omitted from wire key schema | Canonical validator rejects repeated key modifiers |
+| `computer_act.properties.key.allOf`, and the matching nested `computer_act.properties.steps.items.properties.key.allOf` | Omitted from wire key schema | Canonical validator applies every key-chord restriction |
+| `computer_act.properties.key.pattern` and nested `computer_act.properties.steps.items.properties.key.pattern` (Python lookaround) | Key string type retained; incompatible lookaround omitted on wire | Canonical computer payload validator applies the exact original regex before dispatch; controller key parsing independently rejects malformed chords |
+| `computer_act.oneOf`, `computer_act.properties.steps.items.oneOf` and their coordinate-versus-region sub-unions | Nested operation branches with const selectors; coordinate and region remain distinct branches | Canonical validator and adapter ambiguity check reject conflicting and unmatched branches |
+| `false` property schemas under `computer_act.oneOf[*].properties` and `computer_act.properties.steps.items.oneOf[*].properties`, including coordinate-versus-region sub-unions | Forbidden property removed from each wire branch | Canonical validator rejects forbidden fields, even if supplied as null |
+
+`tests/test_strict_tool_adapter.py` exercises positive focus, click-coordinate,
+click-region, sequence and stroke forms; negative focus expectation,
+coordinate/region conflicts, forbidden operation fields, duplicate modifiers,
+repeated key modifiers, empty task context, malformed nested sequence and
+duplicate modifiers in nested strokes. These are local schema fixtures only:
+no desktop input is performed.

src/discord/background_task.py

@@ -102,6 +102,7 @@ class BackgroundTask:
     channel: discord.abc.Messageable
     requester: str
     requester_id: str = ""
+    nested_payload_validated: bool = False
     created_at: str = field(default_factory=lambda: datetime.now().isoformat())
     status: str = "running"  # running, completed, failed, cancelled
     results: list[StepResult] = field(default_factory=list)
@@ -188,6 +189,42 @@ async def run_background_task(
 
         # Variable substitution in tool_input string values
         tool_input = _substitute_vars(tool_input, variables, prev_output)
+        if task.nested_payload_validated:
+            try:
+                from ..tools.nested_payload import validate_nested_payload
+
+                catalog = getattr(executor, "_tool_catalog", None)
+                definitions = catalog.merged_definitions() if catalog is not None else []
+                validate_nested_payload(
+                    "delegate_task",
+                    {"steps": [{**step, "tool_input": tool_input}]},
+                    definitions,
+                    allow_placeholders=False,
+                )
+                denial = executor.check_permission(tool_name, task.requester_id)
+                if denial:
+                    raise ValueError(denial)
+                if tool_name == "invoke_skill":
+                    target = tool_input.get("name")
+                    if not isinstance(target, str) or not target:
+                        raise ValueError("invoke_skill requires a selected skill name")
+                    denial = executor.check_permission(target, task.requester_id)
+                    if denial:
+                        raise ValueError(denial)
+            except ValueError as exc:
+                task.results.append(
+                    StepResult(
+                        index=i,
+                        tool_name=tool_name,
+                        description=step_desc,
+                        status="error",
+                        output=f"Invalid concrete step payload: {exc}",
+                    )
+                )
+                if on_failure == "abort":
+                    task.status = "failed"
+                    break
+                continue
 
         # Evaluate condition
         if condition and prev_output:
@@ -448,10 +485,22 @@ async def _execute_tool(
 
     with execution_delivery_scope(requester_id):
         result = await _execute_tool_captured(
-            tool_name, tool_input, executor, skill_manager, knowledge_store,
-            embedder, requester, step_desc, mcp_manager, requester_id)
+            tool_name,
+            tool_input,
+            executor,
+            skill_manager,
+            knowledge_store,
+            embedder,
+            requester,
+            step_desc,
+            mcp_manager,
+            requester_id,
+        )
         return deliver_runtime_result(
-            executor, result, tool_name=tool_name, tool_input=tool_input,
+            executor,
+            result,
+            tool_name=tool_name,
+            tool_input=tool_input,
             user_id=requester_id,
         )
 
@@ -557,8 +606,7 @@ async def _execute_tool_captured(
         return RankedOutput(
             formatted,
             matches=tuple(
-                f"[{r['source']}] (score: {r.get('score', r.get('rrf_score', 0))}): "
-                f"{r['content']}"
+                f"[{r['source']}] (score: {r.get('score', r.get('rrf_score', 0))}): {r['content']}"
                 for r in results
             ),
             recovery_required=any(len(r["content"]) > 200 for r in results),
@@ -603,9 +651,7 @@ async def _execute_tool_captured(
             target_name, skill_input, requester_id=requester_id or None
         )
     if skill_manager.has_skill(tool_name):
-        return await skill_manager.execute(
-            tool_name, tool_input, requester_id=requester_id or None
-        )
+        return await skill_manager.execute(tool_name, tool_input, requester_id=requester_id or None)
 
     # MCP tools (namespaced as mcp_<server>_<tool>)
     if mcp_manager is not None and mcp_manager.has_tool(tool_name):

src/discord/native_tools/agents_tasks.py

@@ -27,6 +27,7 @@
 from ...llm.tool_history import normalize_tool_calls
 from ...odin_log import get_logger
 from ...tools.defs.agents import SPAWN_NEUTRAL_REASONING_OPTIONS
+from ...tools.nested_payload import ValidatedNestedPayload
 from ...tools.result_validator import ToolResult
 from ..background_task import (
     MAX_STEPS,
@@ -689,10 +690,14 @@ def _agent_iteration_cap(agents_cfg, *, provider: str, scheduled: bool) -> int:
     if provider != "codex":
         return hard_max
     configured = (
-        getattr(agents_cfg, "scheduled_max_iterations", 180)
-        if scheduled
-        else getattr(agents_cfg, "max_iterations", 120)
-    ) if agents_cfg else (180 if scheduled else 120)
+        (
+            getattr(agents_cfg, "scheduled_max_iterations", 180)
+            if scheduled
+            else getattr(agents_cfg, "max_iterations", 120)
+        )
+        if agents_cfg
+        else (180 if scheduled else 120)
+    )
     return min(configured, hard_max)
 
 
@@ -752,6 +757,10 @@ def __init__(self, deps: AgentTaskDeps) -> None:
 
     async def _handle_delegate_task(self, message: discord.Message, inp: dict) -> str:
         """Create and start a background task."""
+        # RequestToolAdapter validates Codex payloads before dispatch. Legacy
+        # providers supply canonical objects directly and retain their existing
+        # permissive delegation contract, including deferred execution checks.
+        nested_validated = isinstance(inp, ValidatedNestedPayload)
         description = inp.get("description", "Background task")
         steps = inp.get("steps", [])
 
@@ -780,13 +789,29 @@ async def _handle_delegate_task(self, message: discord.Message, inp: dict) -> st
                         f"Rebuild the steps with proper tool_input and retry."
                     )
 
+        if nested_validated:
+            for i, step in enumerate(steps, 1):
+                denied = self._tool_executor.check_permission(
+                    step["tool_name"], str(message.author.id)
+                )
+                if isinstance(denied, str) and denied:
+                    return f"Step {i}: {denied}"
+                if step["tool_name"] == "invoke_skill":
+                    target = (step.get("tool_input") or {}).get("name")
+                    if not isinstance(target, str) or not target:
+                        return f"Step {i}: invoke_skill requires a selected skill name"
+                    denied = self._tool_executor.check_permission(target, str(message.author.id))
+                    if isinstance(denied, str) and denied:
+                        return f"Step {i}: {denied}"
+
         task = BackgroundTask(
             task_id=create_task_id(),
             description=description,
             steps=steps,
             channel=message.channel,
             requester=str(message.author),
             requester_id=str(message.author.id),
+            nested_payload_validated=nested_validated,
         )
 
         # Prune old completed tasks
@@ -1141,9 +1166,7 @@ async def _handle_spawn_agent(self, message: object, inp: dict) -> str:
         # list so the ``model_reasoning_dialect`` consumer below sees a
         # clean ``str`` rather than ``Any | None``.
         if not native_choices and _model_mode != "auto":
-            native_choices = [
-                configured_agent_model(self._get_config()) or DEFAULT_AGENT_MODEL
-            ]
+            native_choices = [configured_agent_model(self._get_config()) or DEFAULT_AGENT_MODEL]
         if native_choices and all(
             model_reasoning_dialect(self._get_config(), item) == "effort" for item in native_choices
         ):