fix(cost): close three review gaps in the fan-out cost knobs

Three issues raised in review on #1217, all real:

- The STRIX_MAX_AGENTS check read agent_count() and then spawned, so two
  parents racing for the last slot both passed before either child
  registered and the graph overshot the cap. The coordinator now hands out
  slots atomically (try_reserve_agent_slot / release_agent_slot), counting
  outstanding reservations alongside live agents under the same lock. The
  slot is released once the spawner has registered the child or failed.
  Depth stays a plain check — a parent's depth cannot change mid-spawn.

- _trim_parent_history kept the newest item whole when that item alone
  exceeded the budget ("and kept"), so the cap bounded nothing on the
  child's first request and could overflow the provider context window. An
  oversized newest item is now rendered as a truncated text message. No
  tool-call pairing can break: nothing else survives that trim.

- --reasoning-effort exported STRIX_REASONING_EFFORT, which persist_current()
  then wrote into cli-config.json, turning a documented per-run flag into
  the default for every later run. Env vars can now be marked run-scoped and
  are skipped when persisting.

Tests cover the concurrent reservation, slot release, the oversized-item
trim, and the persist exemption.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/usage/cli.mdx

@@ -52,7 +52,8 @@ strix (--target <target> | --target-list <path>) [options]
   Model reasoning effort for this run: `none`, `minimal`, `low`, `medium`,
   `high`, `xhigh`, or `max`. Higher = more thinking tokens = higher cost and
   (usually) deeper analysis. Overrides `STRIX_REASONING_EFFORT` and the config
-  file for this run. Defaults to the configured value (`high`).
+  file for this run. Defaults to the configured value (`high`). Not written back
+  to the config file, so it never changes what later runs do.
 </ParamField>
 
 <ParamField path="--scope-mode" type="string" default="auto">

strix/config/__init__.py

@@ -9,11 +9,13 @@ Public surface:
 - :func:`load_settings` — memoized resolve (env > JSON file > defaults).
 - :func:`apply_config_override` — switch the JSON source to a custom path.
 - :func:`persist_current` — write currently-set env vars to the active file.
+- :func:`mark_run_scoped` — keep a per-run env var out of that file.
 """
 
 from strix.config.loader import (
     apply_config_override,
     load_settings,
+    mark_run_scoped,
     persist_current,
 )
 from strix.config.settings import (
@@ -37,5 +39,6 @@ __all__ = [
     "TelemetrySettings",
     "apply_config_override",
     "load_settings",
+    "mark_run_scoped",
     "persist_current",
 ]

strix/config/loader.py

@@ -26,6 +26,10 @@ logger = logging.getLogger(__name__)
 _DEFAULT_PATH: Path = Path.home() / ".strix" / "cli-config.json"
 _override: Path | None = None
 _cached: Settings | None = None
+# Env vars set for this process only (e.g. from a per-run CLI flag). They still
+# win over the config file while the run lasts, but persist_current() must not
+# write them back, or a one-off flag would silently become the new default.
+_run_scoped: set[str] = set()
 
 # Model, API key, and API base describe one provider connection. When the shell
 # changes any of them, the stored values of the others no longer belong together
@@ -60,6 +64,16 @@ def apply_config_override(path: Path) -> None:
     logger.info("config override applied: %s", path)
 
 
+def mark_run_scoped(*env_names: str) -> None:
+    """Exempt ``env_names`` from :func:`persist_current`.
+
+    For values that apply to the current run only. Without this, exporting a
+    per-run CLI flag into the environment makes it indistinguishable from a
+    setting the user chose to keep, and it lands in the config file.
+    """
+    _run_scoped.update(name.upper() for name in env_names)
+
+
 def persist_current() -> None:
     """Merge currently-set env vars into the active config file (0o600).
 
@@ -67,6 +81,9 @@ def persist_current() -> None:
     run that gets its settings from the file does not erase them. An env
     var set to the empty string clears the field from the file. A change to
     any linked LLM connection var drops the whole stored connection first.
+
+    Run-scoped vars (see :func:`mark_run_scoped`) are left out entirely: a
+    per-run override must neither be written nor clear what the file holds.
     """
     s = load_settings()
     target = _override or _DEFAULT_PATH
@@ -80,7 +97,9 @@ def persist_current() -> None:
         for finfo in type(sub_model).model_fields.values():
             aliases = [alias.upper() for alias in _aliases_for(finfo)]
             active = next((alias for alias in aliases if alias in os.environ), None)
-            if active is None:
+            # A run-scoped value belongs to this run only, so it neither
+            # overwrites nor clears the field the file already stores.
+            if active is None or active in _run_scoped:
                 continue
             for alias in aliases:
                 env_block.pop(alias, None)

strix/core/agents.py

@@ -63,6 +63,9 @@ class AgentCoordinator:
         self.runtimes: dict[str, AgentRuntime] = {}
         self._parent_notified: set[str] = set()
         self._lock = asyncio.Lock()
+        # Slots claimed by a spawn that has not registered its child yet. Counted
+        # alongside live agents so a concurrent fan-out cannot overshoot the cap.
+        self._reserved_slots = 0
         self._snapshot_path: Path | None = None
         self.is_shutting_down = False
         self._budget_stopped = False
@@ -486,6 +489,28 @@ class AgentCoordinator:
                 current = self.parent_of.get(current)
             return max(depth, 1)
 
+    async def try_reserve_agent_slot(self, max_agents: int) -> bool:
+        """Atomically claim one slot under ``max_agents``; ``0`` means unlimited.
+
+        Reading :meth:`agent_count` and then spawning is check-then-act: two
+        parents asking for the last slot concurrently both see room before
+        either child registers, and the graph overshoots the cap. Counting
+        outstanding reservations with the live agents under the same lock
+        closes that window. Release the slot with
+        :meth:`release_agent_slot` once the child is registered or the spawn
+        has failed.
+        """
+        async with self._lock:
+            if max_agents and len(self.parent_of) + self._reserved_slots >= max_agents:
+                return False
+            self._reserved_slots += 1
+            return True
+
+    async def release_agent_slot(self) -> None:
+        """Give back a slot claimed by :meth:`try_reserve_agent_slot`."""
+        async with self._lock:
+            self._reserved_slots = max(self._reserved_slots - 1, 0)
+
     async def graph_snapshot(
         self,
     ) -> tuple[dict[str, str | None], dict[str, Status], dict[str, str], dict[str, str]]:

strix/core/inputs.py

@@ -32,6 +32,28 @@ _HISTORY_TRUNCATED_MARKER = {
     "role": "user",
     "content": "[... older inherited context dropped to bound token cost ...]",
 }
+_OVERSIZED_ITEM_NOTE = "[... newest inherited item truncated to bound token cost ...]\n"
+
+
+def _fit_oversized_item(item: Any, char_budget: int) -> dict[str, str]:
+    """Render ``item`` as a text message cut down to ``char_budget``.
+
+    Used only when the single newest item is larger than the whole budget:
+    keeping it whole would blow the cap on the child's very first request and
+    can overflow the provider's context window. Nothing else survives the trim
+    in that case, so rendering it as one text message cannot orphan a tool call
+    from its output.
+    """
+    body = json.dumps(item, ensure_ascii=False, default=str)
+    while body:
+        summary = {"role": "user", "content": f"{_OVERSIZED_ITEM_NOTE}{body}"}
+        overshoot = len(json.dumps(summary, ensure_ascii=False)) - char_budget
+        if overshoot <= 0:
+            return summary
+        body = body[: max(len(body) - overshoot, 0)]
+    # Budget too small to hold even a snippet; the marker alone is the most that
+    # can be said about the dropped item.
+    return dict(_HISTORY_TRUNCATED_MARKER)
 
 
 def _trim_parent_history(parent_history: list[Any]) -> list[Any]:
@@ -50,8 +72,13 @@ def _trim_parent_history(parent_history: list[Any]) -> list[Any]:
     used = 0
     for item in reversed(parent_history):
         size = len(json.dumps(item, ensure_ascii=False, default=str))
-        if used + size > char_budget and kept:
-            kept.append(_HISTORY_TRUNCATED_MARKER)
+        if used + size > char_budget:
+            # ``kept`` is empty only on the newest item, i.e. that one item is
+            # over budget all by itself. Truncate it rather than keeping it
+            # whole — otherwise the cap silently fails to bound anything.
+            kept.append(
+                _HISTORY_TRUNCATED_MARKER if kept else _fit_oversized_item(item, char_budget)
+            )
             break
         kept.append(item)
         used += size

strix/interface/cli_args.py

@@ -7,7 +7,7 @@ import os
 import sys
 from pathlib import Path
 
-from strix.config import apply_config_override
+from strix.config import apply_config_override, mark_run_scoped
 from strix.config.settings import DEFAULT_MAX_TURNS
 from strix.core.paths import run_dir_for, runtime_state_dir
 from strix.interface.scan_setup import attach_workspace_mount, build_targets_info
@@ -328,9 +328,13 @@ Strix Cloud:
         os.environ["STRIX_MCP_EXCLUDE"] = ",".join(args.mcp_exclude)
 
     # Settings read STRIX_REASONING_EFFORT from the environment (env wins over the
-    # config file), so exporting it here makes the flag win for this run.
+    # config file), so exporting it here makes the flag win for this run. Mark it
+    # run-scoped: persist_current() writes every settings env var it finds into
+    # cli-config.json, which would turn this one-run flag into the new default
+    # for every later scan.
     if args.reasoning_effort:
         os.environ["STRIX_REASONING_EFFORT"] = args.reasoning_effort
+        mark_run_scoped("STRIX_REASONING_EFFORT")
 
     if args.update:
         sys.exit(0 if self_update() else 1)

strix/tools/agents_graph/tools.py

@@ -26,33 +26,36 @@ _ACTIVE_STATUSES: frozenset[str] = frozenset({"running", "waiting"})
 logger = logging.getLogger(__name__)
 
 
-async def _fan_out_limit_error(coordinator: AgentCoordinator, parent_id: str) -> str | None:
-    """Return a model-facing error if spawning a child would breach a fan-out cap.
+def _agent_limit_error(max_agents: int) -> str:
+    """Model-facing refusal for a spawn that would breach ``STRIX_MAX_AGENTS``."""
+    return (
+        f"Agent limit reached ({max_agents} agents). Cannot spawn another. "
+        "Do this work yourself, reuse an existing agent via send_message_to_agent, "
+        "or wait_for_agents to let running ones finish. The operator can raise "
+        "STRIX_MAX_AGENTS if a larger fan-out is intended."
+    )
+
+
+async def _depth_limit_error(coordinator: AgentCoordinator, parent_id: str) -> str | None:
+    """Return a model-facing error if a child of ``parent_id`` would sit too deep.
 
     Bounds token spend: every extra agent re-pays the full system prompt on each
     of its turns, so an unbounded graph is the biggest single-target cost driver.
-    Both caps are configurable (``STRIX_MAX_AGENTS`` / ``STRIX_MAX_AGENT_DEPTH``);
-    ``0`` disables that check.
+    ``STRIX_MAX_AGENT_DEPTH`` caps the tree height; ``0`` disables the check. The
+    parent's own depth is fixed for the life of the agent, so unlike the total
+    agent cap this needs no reservation to stay correct under concurrent spawns.
     """
-    graph = load_settings().agent_graph
+    max_depth = load_settings().agent_graph.max_agent_depth
+    if not max_depth:
+        return None
 
-    if graph.max_agents and await coordinator.agent_count() >= graph.max_agents:
+    child_depth = await coordinator.depth_of(parent_id) + 1
+    if child_depth > max_depth:
         return (
-            f"Agent limit reached ({graph.max_agents} agents). Cannot spawn another. "
-            "Do this work yourself, reuse an existing agent via send_message_to_agent, "
-            "or wait_for_agents to let running ones finish. The operator can raise "
-            "STRIX_MAX_AGENTS if a larger fan-out is intended."
+            f"Agent depth limit reached (max {max_depth}). This agent is "
+            "too deep in the tree to spawn a child. Run the subtask yourself or hand it "
+            "back to a shallower agent. The operator can raise STRIX_MAX_AGENT_DEPTH."
         )
-
-    if graph.max_agent_depth:
-        child_depth = await coordinator.depth_of(parent_id) + 1
-        if child_depth > graph.max_agent_depth:
-            return (
-                f"Agent depth limit reached (max {graph.max_agent_depth}). This agent is "
-                "too deep in the tree to spawn a child. Run the subtask yourself or hand it "
-                "back to a shallower agent. The operator can raise STRIX_MAX_AGENT_DEPTH."
-            )
-
     return None
 
 
@@ -599,19 +602,24 @@ async def create_agent(
             default=str,
         )
 
-    limit_error = await _fan_out_limit_error(coordinator, parent_id)
-    if limit_error:
+    skill_list = list(skills or [])
+    spawn_error = validate_requested_skills(skill_list) or await _depth_limit_error(
+        coordinator, parent_id
+    )
+    if spawn_error:
         return json.dumps(
-            {"success": False, "error": limit_error, "agent_id": None},
+            {"success": False, "error": spawn_error, "agent_id": None},
             ensure_ascii=False,
             default=str,
         )
 
-    skill_list = list(skills or [])
-    skill_error = validate_requested_skills(skill_list)
-    if skill_error:
+    # Claim the slot before spawning, not just before checking: the child only
+    # enters the graph once the spawner registers it, and two parents racing for
+    # the last slot would otherwise both be waved through.
+    max_agents = load_settings().agent_graph.max_agents
+    if not await coordinator.try_reserve_agent_slot(max_agents):
         return json.dumps(
-            {"success": False, "error": skill_error, "agent_id": None},
+            {"success": False, "error": _agent_limit_error(max_agents), "agent_id": None},
             ensure_ascii=False,
             default=str,
         )
@@ -632,6 +640,11 @@ async def create_agent(
             ensure_ascii=False,
             default=str,
         )
+    finally:
+        # The spawner registers the child before it returns, so by now the slot
+        # is accounted for by the graph itself (and on failure there is nothing
+        # to account for).
+        await coordinator.release_agent_slot()
 
     logger.info(
         "create_agent: spawned %s (%s) parent=%s skills=%d task_len=%d",

tests/test_agent_fanout_limits.py

@@ -2,11 +2,12 @@
 
 from __future__ import annotations
 
+import asyncio
 from typing import TYPE_CHECKING
 
 from strix.config import loader
 from strix.core.agents import AgentCoordinator
-from strix.tools.agents_graph.tools import _fan_out_limit_error
+from strix.tools.agents_graph.tools import _depth_limit_error
 
 
 if TYPE_CHECKING:
@@ -21,31 +22,51 @@ async def _graph(*edges: tuple[str, str | None]) -> AgentCoordinator:
     return coordinator
 
 
-async def test_max_agents_blocks_when_reached(monkeypatch: pytest.MonkeyPatch) -> None:
-    monkeypatch.setenv("STRIX_MAX_AGENTS", "2")
-    monkeypatch.setenv("STRIX_MAX_AGENT_DEPTH", "0")
-    loader._cached = None
-    try:
-        coordinator = await _graph(("root", None), ("child", "root"))
-        error = await _fan_out_limit_error(coordinator, "root")
-    finally:
-        loader._cached = None
+async def test_reserve_blocks_when_limit_reached() -> None:
+    coordinator = await _graph(("root", None), ("child", "root"))
 
-    assert error is not None
-    assert "Agent limit reached" in error
+    assert await coordinator.try_reserve_agent_slot(2) is False
 
 
-async def test_max_agents_allows_below_limit(monkeypatch: pytest.MonkeyPatch) -> None:
-    monkeypatch.setenv("STRIX_MAX_AGENTS", "4")
-    monkeypatch.setenv("STRIX_MAX_AGENT_DEPTH", "0")
-    loader._cached = None
-    try:
-        coordinator = await _graph(("root", None), ("child", "root"))
-        error = await _fan_out_limit_error(coordinator, "root")
-    finally:
-        loader._cached = None
+async def test_reserve_allows_below_limit() -> None:
+    coordinator = await _graph(("root", None), ("child", "root"))
 
-    assert error is None
+    assert await coordinator.try_reserve_agent_slot(4) is True
+
+
+async def test_reserve_unlimited_when_zero() -> None:
+    coordinator = await _graph(("root", None), ("a", "root"), ("b", "a"), ("c", "b"))
+
+    assert await coordinator.try_reserve_agent_slot(0) is True
+
+
+async def test_concurrent_reservations_cannot_overshoot_cap() -> None:
+    # Two parents race for the last slot. Counting only registered agents would
+    # wave both through, because neither child registers before the other checks.
+    coordinator = await _graph(("root", None), ("a", "root"))
+
+    granted = await asyncio.gather(*(coordinator.try_reserve_agent_slot(3) for _ in range(2)))
+
+    assert sorted(granted) == [False, True]
+
+
+async def test_released_slot_is_reusable() -> None:
+    coordinator = await _graph(("root", None), ("a", "root"))
+
+    assert await coordinator.try_reserve_agent_slot(3) is True
+    assert await coordinator.try_reserve_agent_slot(3) is False
+    await coordinator.release_agent_slot()
+    assert await coordinator.try_reserve_agent_slot(3) is True
+
+
+async def test_release_does_not_go_negative() -> None:
+    coordinator = await _graph(("root", None))
+
+    await coordinator.release_agent_slot()
+    await coordinator.release_agent_slot()
+
+    # A stray release must not hand out a free slot beyond the cap.
+    assert await coordinator.try_reserve_agent_slot(1) is False
 
 
 async def test_max_depth_blocks_grandchild(monkeypatch: pytest.MonkeyPatch) -> None:
@@ -55,9 +76,9 @@ async def test_max_depth_blocks_grandchild(monkeypatch: pytest.MonkeyPatch) -> N
     try:
         coordinator = await _graph(("root", None), ("child", "root"))
         # Spawning from the child would create a depth-3 grandchild.
-        child_error = await _fan_out_limit_error(coordinator, "child")
+        child_error = await _depth_limit_error(coordinator, "child")
         # Spawning from the root creates a depth-2 child — allowed.
-        root_error = await _fan_out_limit_error(coordinator, "root")
+        root_error = await _depth_limit_error(coordinator, "root")
     finally:
         loader._cached = None
 
@@ -66,7 +87,7 @@ async def test_max_depth_blocks_grandchild(monkeypatch: pytest.MonkeyPatch) -> N
     assert root_error is None
 
 
-async def test_limits_disabled_when_zero(monkeypatch: pytest.MonkeyPatch) -> None:
+async def test_depth_limit_disabled_when_zero(monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setenv("STRIX_MAX_AGENTS", "0")
     monkeypatch.setenv("STRIX_MAX_AGENT_DEPTH", "0")
     loader._cached = None
@@ -74,7 +95,7 @@ async def test_limits_disabled_when_zero(monkeypatch: pytest.MonkeyPatch) -> Non
         coordinator = await _graph(
             ("root", None), ("a", "root"), ("b", "a"), ("c", "b"), ("d", "c")
         )
-        error = await _fan_out_limit_error(coordinator, "d")
+        error = await _depth_limit_error(coordinator, "d")
     finally:
         loader._cached = None
 

tests/test_config_loader.py

@@ -406,6 +406,47 @@ def test_persist_current_replaces_corrupt_file(
     assert json.loads(target.read_text(encoding="utf-8")) == {"env": {"STRIX_LLM": "env-model"}}
 
 
+def test_persist_current_skips_run_scoped_env(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # A per-run flag exports its env var so it wins for this run; persisting it
+    # would silently make it the default for every later run.
+    monkeypatch.setenv("STRIX_LLM", "persisted-model")
+    monkeypatch.setenv("STRIX_REASONING_EFFORT", "max")
+    monkeypatch.setattr(loader, "_run_scoped", set())
+    loader.mark_run_scoped("STRIX_REASONING_EFFORT")
+    target = tmp_path / "cli-config.json"
+    loader.apply_config_override(target)
+
+    loader.persist_current()
+
+    assert json.loads(target.read_text(encoding="utf-8")) == {
+        "env": {"STRIX_LLM": "persisted-model"}
+    }
+
+
+def test_run_scoped_env_does_not_clear_stored_value(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # The per-run override must leave the user's stored preference alone, not
+    # just avoid overwriting it with the run's value.
+    target = tmp_path / "cli-config.json"
+    target.write_text(
+        json.dumps({"env": {"STRIX_REASONING_EFFORT": "low"}}),
+        encoding="utf-8",
+    )
+    loader.apply_config_override(target)
+    monkeypatch.setenv("STRIX_REASONING_EFFORT", "max")
+    monkeypatch.setattr(loader, "_run_scoped", set())
+    loader.mark_run_scoped("STRIX_REASONING_EFFORT")
+
+    loader.persist_current()
+
+    assert json.loads(target.read_text(encoding="utf-8")) == {
+        "env": {"STRIX_REASONING_EFFORT": "low"}
+    }
+
+
 def test_persist_current_sets_0600_mode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setenv("STRIX_LLM", "persisted-model")
     target = tmp_path / "cli-config.json"

tests/test_inputs.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import json
 from itertools import pairwise
 from typing import Any
 
@@ -10,6 +11,7 @@ import pytest
 
 from strix.config import loader
 from strix.core.inputs import (
+    _trim_parent_history,
     build_root_task,
     build_scan_targets,
     build_scope_context,
@@ -64,8 +66,8 @@ def test_child_initial_input_no_consecutive_same_role(parent_history: list[Any])
 
 
 def test_child_initial_input_trims_inherited_history(monkeypatch: pytest.MonkeyPatch) -> None:
-    # Tiny cap so all but the most recent item is dropped.
-    monkeypatch.setenv("STRIX_INHERIT_CONTEXT_MAX_TOKENS", "1")
+    # Cap (20 tokens ~= 80 chars) fits the newest item but not both.
+    monkeypatch.setenv("STRIX_INHERIT_CONTEXT_MAX_TOKENS", "20")
     loader._cached = None
     try:
         history = [
@@ -82,6 +84,35 @@ def test_child_initial_input_trims_inherited_history(monkeypatch: pytest.MonkeyP
     assert "older inherited context dropped" in content
 
 
+def test_trim_truncates_oversized_newest_item(monkeypatch: pytest.MonkeyPatch) -> None:
+    # One item, larger than the whole budget: keeping it whole would mean the cap
+    # bounds nothing at all on the child's first request.
+    monkeypatch.setenv("STRIX_INHERIT_CONTEXT_MAX_TOKENS", "40")
+    loader._cached = None
+    try:
+        history = [{"role": "assistant", "content": "x" * 5000}]
+        trimmed = _trim_parent_history(history)
+    finally:
+        loader._cached = None
+
+    assert len(trimmed) == 1
+    assert len(json.dumps(trimmed[0], ensure_ascii=False)) <= 40 * 4
+    assert "truncated to bound token cost" in trimmed[0]["content"]
+
+
+def test_trim_falls_back_to_marker_when_budget_tiny(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setenv("STRIX_INHERIT_CONTEXT_MAX_TOKENS", "1")
+    loader._cached = None
+    try:
+        history = [{"role": "assistant", "content": "x" * 5000}]
+        trimmed = _trim_parent_history(history)
+    finally:
+        loader._cached = None
+
+    assert len(trimmed) == 1
+    assert "x" * 100 not in trimmed[0]["content"]
+
+
 def test_child_initial_input_keeps_full_history_when_cap_disabled(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None: