From a98bf7ad508dcea16c03465bf5a049ef3d8bffbc Mon Sep 17 00:00:00 2001 From: PurpleCHOIms Date: Fri, 3 Jul 2026 14:19:14 +0900 Subject: [PATCH 1/2] test(filesystem): fix sandbox-rebind mocks to accept the config arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #749 made the filesystem middleware re-resolve the per-run sandbox via build_sandbox_backend(config) — i.e. it now calls build_sandbox_backend with a positional `config` argument. The rebind unit tests still patched it with a zero-arg `lambda: per_run`, so every call raised TypeError: () takes 0 positional arguments but 1 was given turning three tests red on main (test_rebind_reresolves_bare_http_sandbox_per_run, test_rebind_swaps_composite_default_preserving_routes, test_get_backend_reresolves_sandbox_transport_per_run) — a stale mock, not a production bug. Mirror the real signature build_sandbox_backend(config=None) in the four mocks (the "must not be called" throw-mock included for consistency). Target tests: 5 passed. ruff format + check clean. Co-Authored-By: Claude Fable 5 --- .../decepticon/tests/unit/middleware/test_filesystem.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/decepticon/tests/unit/middleware/test_filesystem.py b/packages/decepticon/tests/unit/middleware/test_filesystem.py index 6b5e0450..72eb7d17 100644 --- a/packages/decepticon/tests/unit/middleware/test_filesystem.py +++ b/packages/decepticon/tests/unit/middleware/test_filesystem.py @@ -634,7 +634,7 @@ def test_rebind_reresolves_bare_http_sandbox_per_run(monkeypatch) -> None: by the per-run resolution of ``build_sandbox_backend()`` — which consults ``configurable.sandbox_url`` first, env second.""" per_run = HTTPSandbox("http://per-run-vm:9999", token="t") - monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda: per_run) + monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda config=None: per_run) env = HTTPSandbox("http://shared-sidecar:9999") assert _rebind_sandbox_per_run(env) is per_run @@ -647,7 +647,7 @@ def test_rebind_swaps_composite_default_preserving_routes(monkeypatch) -> None: ``/skills/`` route and ``artifacts_root`` are preserved, and the cached construction-time composite is left untouched.""" per_run = HTTPSandbox("http://per-run-vm:9999", token="t") - monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda: per_run) + monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda config=None: per_run) skills = RecordingBackend() env = HTTPSandbox("http://shared-sidecar:9999") @@ -667,7 +667,7 @@ def test_rebind_leaves_composite_with_non_sandbox_default_untouched(monkeypatch) returned unchanged — we never inject a sandbox where there wasn't one.""" monkeypatch.setattr( "decepticon.backends.build_sandbox_backend", - lambda: (_ for _ in ()).throw(AssertionError("must not be called")), + lambda config=None: (_ for _ in ()).throw(AssertionError("must not be called")), ) state_like = RecordingBackend() composite = CompositeBackend(default=state_like, routes={"/skills/": RecordingBackend()}) @@ -680,7 +680,7 @@ def test_get_backend_reresolves_sandbox_transport_per_run(monkeypatch) -> None: the env sidecar) re-resolves each run's filesystem transport to that run's own sandbox before scoping it to the engagement workspace.""" per_run = HTTPSandbox("http://per-run-vm:9999", token="t") - monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda: per_run) + monkeypatch.setattr("decepticon.backends.build_sandbox_backend", lambda config=None: per_run) env = HTTPSandbox("http://shared-sidecar:9999") composite = CompositeBackend(default=env, routes={"/skills/": RecordingBackend()}) From 77778f1d27d9db6f71245fa3759ff6ccbaf842e0 Mon Sep 17 00:00:00 2001 From: PurpleCHOIms Date: Sun, 5 Jul 2026 04:45:46 +0900 Subject: [PATCH 2/2] fix(write_file): per-model max_tokens + actionable "already exists" error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two chronic write_file failures: 1. Output was capped at a flat 16384 tokens (DEFAULT_LLM_MAX_TOKENS), ~8x below modern Claude limits. A large single write_file call (full finding report / recon model) truncated mid-argument, the tool-call JSON never closed, `content` was dropped, and the write failed. Now each model gets its OWN max-output ceiling: opus*/sonnet* -> 128000, haiku* -> 64000, unknown -> safe 64000 default. It is a ceiling (short replies cost nothing extra); values >64k use the streaming path the SDK already takes. DECEPTICON_LLM_MAX_TOKENS still wins as an explicit override. 2. write_file on an existing path errored, and the agent had to guess to switch to edit_file. The refusal is raised in the deepagents backends (external dep, no-overwrite by design — an overwrite would force the agent to re-emit the whole file for a one-line change, wasteful and it re-triggers the max_tokens truncation). Left as no-overwrite; instead EngagementFilesystemBackend.write (OSS-controlled choke point) now normalizes the varying backend messages into one actionable instruction pointing at edit_file, with the virtual (not real) path. Strengthened the bash tool prompt so the agent uses edit_file for existing files and ls/read first when unsure, covering retries that re-write a file the first attempt already created. Co-Authored-By: Claude Opus 4.8 --- packages/decepticon/decepticon/llm/factory.py | 53 ++++++++++++++----- .../decepticon/middleware/filesystem.py | 17 +++++- .../decepticon/tools/bash/prompt.py | 9 ++++ .../decepticon/tests/unit/llm/test_factory.py | 31 +++++++++++ .../tests/unit/middleware/test_filesystem.py | 20 +++++++ 5 files changed, 116 insertions(+), 14 deletions(-) diff --git a/packages/decepticon/decepticon/llm/factory.py b/packages/decepticon/decepticon/llm/factory.py index 6f7cf832..58ddbe2e 100644 --- a/packages/decepticon/decepticon/llm/factory.py +++ b/packages/decepticon/decepticon/llm/factory.py @@ -52,17 +52,20 @@ log = get_logger("llm.factory") -# Default output-token cap for every model created through this factory. +# Output-token cap for every model created through this factory. # We must set one explicitly: the LiteLLM proxy applies the Anthropic default # of 4096 when the client sends no ``max_tokens``, and a single large # ``write_file`` tool call (e.g. a full finding report or a recon target model) -# then truncates mid-argument at 4096 output tokens — the tool-call JSON never -# closes, so the ``content`` arg is dropped and the write fails. Modern Claude -# models (opus-4-8 / sonnet-4-6 / haiku-4-5) support far larger outputs; 16384 -# is a generous cap (it is a ceiling, not a forced value — short replies cost -# nothing extra) that lets large deliverables complete in one call. -# Override with ``DECEPTICON_LLM_MAX_TOKENS``. -DEFAULT_LLM_MAX_TOKENS = 16384 +# then truncates mid-argument — the tool-call JSON never closes, so the +# ``content`` arg is dropped and the write fails. The cap is per-model: we hand +# each model its OWN max-output ceiling so a big deliverable completes in one +# call. It is a ceiling, not a forced value — short replies cost nothing extra. +# Values above 64k require the streaming API path (the SDK already streams). +# Authoritative caps (see ``_model_max_output_tokens``): Claude Opus 4.x and +# Sonnet (4.6/5) = 128000; Claude Haiku 4.5 = 64000. Unknown models fall back +# to the safe 64k default. Override with ``DECEPTICON_LLM_MAX_TOKENS`` (an +# explicit value wins over the per-model resolution). +DEFAULT_LLM_MAX_TOKENS = 64000 LLM_MAX_TOKENS_ENV = "DECEPTICON_LLM_MAX_TOKENS" DEFAULT_LLM_REQUEST_TIMEOUT_SECONDS = 600 @@ -78,8 +81,32 @@ LLM_TIMEOUT_ENV_ALIAS = "DECEPTICON_LLM__TIMEOUT" -def _resolve_max_tokens() -> int: - """Output-token cap for factory-created models (env override, safe parse).""" +def _model_max_output_tokens(model: str) -> int: + """Return ``model``'s own max-output-token ceiling. + + Match on the model slug suffix (last path segment) so every namespace we + route through resolves the same — ``anthropic/claude-opus-4-8``, + ``auth/claude-opus-4-8``, ``openrouter/anthropic/claude-sonnet-4-6``. + Opus 4.x and Sonnet (4.6/5) support 128000 output tokens; Haiku 4.5 + supports 64000. Unknown / non-Claude models fall back to the safe 64k + default rather than an over-large value the upstream might reject. + """ + slug = model.rsplit("/", 1)[-1].lower() + if "opus" in slug or "sonnet" in slug: + return 128000 + if "haiku" in slug: + return 64000 + return DEFAULT_LLM_MAX_TOKENS + + +def _resolve_max_tokens(model: str) -> int: + """Output-token cap for a factory-created model. + + ``DECEPTICON_LLM_MAX_TOKENS`` (explicit override) wins when set to a + positive int; otherwise the model's own max-output ceiling is used so a + large single-call deliverable (finding report, recon model) never truncates + at the proxy's 4096 default. + """ raw = os.environ.get(LLM_MAX_TOKENS_ENV) if raw: try: @@ -88,7 +115,7 @@ def _resolve_max_tokens() -> int: value = 0 if value > 0: return value - return DEFAULT_LLM_MAX_TOKENS + return _model_max_output_tokens(model) class LLMTimeoutError(RuntimeError): @@ -1608,9 +1635,9 @@ def _create_chat_model(self, model: str, temperature: float) -> BaseChatModel: "api_key": SecretStr(self._proxy.api_key), "timeout": self._proxy.timeout, "max_retries": self._proxy.max_retries, - # Explicit cap so large single-call deliverables (finding reports, + # Per-model cap so large single-call deliverables (finding reports, # recon target models) don't truncate at the proxy's 4096 default. - "max_tokens": _resolve_max_tokens(), + "max_tokens": _resolve_max_tokens(model), } if _model_drops_temperature(model): kwargs["disabled_params"] = {"temperature": None} diff --git a/packages/decepticon/decepticon/middleware/filesystem.py b/packages/decepticon/decepticon/middleware/filesystem.py index fee58f4c..4064388f 100644 --- a/packages/decepticon/decepticon/middleware/filesystem.py +++ b/packages/decepticon/decepticon/middleware/filesystem.py @@ -176,7 +176,22 @@ def write(self, file_path: str, content: str) -> WriteResult: return WriteResult(error=str(e)) result = self._backend.write(real_path, content) if result.error: - return replace(result, error=self._mask(result.error, real_path)) + masked = self._mask(result.error, real_path) + if masked and "already exists" in masked.lower(): + # deepagents backends refuse to overwrite by design (an + # overwrite would force the agent to re-emit the whole file for + # a one-line change — wasteful, and it re-triggers max_tokens + # truncation on large files). The raw backend messages vary + # ("File already exists" from the sandbox backend, a longer one + # from state/filesystem); normalize to a single actionable + # instruction so the agent switches to edit_file instead of + # guessing. + virtual = self._virtual(real_path) or file_path + masked = ( + f"{virtual} already exists — use edit_file to modify it, " + "or delete it first if you intend to replace it." + ) + return replace(result, error=masked) path = self._virtual(result.path or "") if result.path else None return replace(result, path=path) if path else result diff --git a/packages/decepticon/decepticon/tools/bash/prompt.py b/packages/decepticon/decepticon/tools/bash/prompt.py index 9bb516e1..f0e0cf5c 100644 --- a/packages/decepticon/decepticon/tools/bash/prompt.py +++ b/packages/decepticon/decepticon/tools/bash/prompt.py @@ -190,6 +190,15 @@ report) write it in sections — one `write_file` then `edit_file` to append the rest — rather than one oversized call. An oversized single `content` is the case a model most often drops or truncates, leaving the call invalid. +- `write_file` CREATES a new file — it will NOT overwrite. Calling it on a path + that already exists fails with "already exists". To change a file that exists, + use `edit_file` (targeted string replace), never a second `write_file`. This + matters on retries too: if an earlier attempt already wrote the file, the + retry must `edit_file` it, not re-`write_file` it. +- When unsure whether a file exists (e.g. resuming work, or after a retry), + `ls` the directory (or `read_file` the path) FIRST, then `write_file` if it is + absent or `edit_file` if it is present. Don't `write_file` blind and rely on + the error to tell you. - Before `read_file`, confirm the path EXISTS and is a file: `ls` the directory and read only what it returns. Never read a path before the bash command or `write_file` that creates it has SUCCEEDED, and never invent an diff --git a/packages/decepticon/tests/unit/llm/test_factory.py b/packages/decepticon/tests/unit/llm/test_factory.py index 495e771b..e797ce48 100644 --- a/packages/decepticon/tests/unit/llm/test_factory.py +++ b/packages/decepticon/tests/unit/llm/test_factory.py @@ -6,13 +6,16 @@ import pytest from decepticon.llm.factory import ( + LLM_MAX_TOKENS_ENV, LLMFactory, _is_real_key, _llamacpp_local_configured, _method_is_configured, + _model_max_output_tokens, _oauth_credentials_present, _oauth_env_credentials_present, _resolve_credentials, + _resolve_max_tokens, ) from decepticon_core.types.llm import ( AuthMethod, @@ -163,6 +166,34 @@ def test_codex_has_no_env_token_override(self, monkeypatch) -> None: assert _oauth_env_credentials_present(AuthMethod.OPENAI_OAUTH) is False +class TestMaxTokensResolution: + """Per-model output-token ceiling (fixes write_file truncation).""" + + def test_opus_and_sonnet_get_128k(self) -> None: + assert _model_max_output_tokens("anthropic/claude-opus-4-8") == 128000 + assert _model_max_output_tokens("auth/claude-opus-4-8") == 128000 + assert _model_max_output_tokens("openrouter/anthropic/claude-sonnet-4-6") == 128000 + + def test_haiku_gets_64k(self) -> None: + assert _model_max_output_tokens("anthropic/claude-haiku-4-5") == 64000 + + def test_unknown_model_falls_back_to_safe_default(self) -> None: + assert _model_max_output_tokens("openai/gpt-5-nano") == 64000 + + def test_env_override_wins(self, monkeypatch) -> None: + monkeypatch.setenv(LLM_MAX_TOKENS_ENV, "8192") + # Override wins even for a model whose own ceiling is higher. + assert _resolve_max_tokens("anthropic/claude-opus-4-8") == 8192 + + def test_invalid_env_falls_back_to_per_model(self, monkeypatch) -> None: + monkeypatch.setenv(LLM_MAX_TOKENS_ENV, "not-a-number") + assert _resolve_max_tokens("anthropic/claude-opus-4-8") == 128000 + + def test_no_env_uses_per_model(self, monkeypatch) -> None: + monkeypatch.delenv(LLM_MAX_TOKENS_ENV, raising=False) + assert _resolve_max_tokens("anthropic/claude-haiku-4-5") == 64000 + + class TestLLMFactory: def setup_method(self): self.proxy = ProxyConfig(url="http://localhost:4000", api_key="test-key") diff --git a/packages/decepticon/tests/unit/middleware/test_filesystem.py b/packages/decepticon/tests/unit/middleware/test_filesystem.py index 72eb7d17..a0d25f87 100644 --- a/packages/decepticon/tests/unit/middleware/test_filesystem.py +++ b/packages/decepticon/tests/unit/middleware/test_filesystem.py @@ -124,6 +124,26 @@ def test_returns_virtual_paths_to_agent() -> None: assert write_result.path == "/workspace/findings/FIND-001.md" +def test_write_exists_error_is_actionable_and_masks_real_path() -> None: + """A no-overwrite backend error is rewritten to point the agent at + edit_file, using the virtual (not the real engagement) path.""" + + class ExistsBackend(RecordingBackend): + def write(self, file_path: str, content: str) -> WriteResult: + # Mirror the deepagents sandbox backend's terse message shape, + # which does not itself mention edit_file. + return WriteResult(error=f"Error: File already exists: '{file_path}'") + + scoped = EngagementFilesystemBackend(ExistsBackend(), "/workspace/test") + result = scoped.write("/workspace/findings/FIND-001.md", "x") + + assert result.error is not None + assert "/workspace/findings/FIND-001.md already exists" in result.error + assert "edit_file" in result.error + # Real engagement path must not leak into the message. + assert "/workspace/test" not in result.error + + def test_scopes_glob_and_grep_without_exposing_real_engagement_path() -> None: backend = RecordingBackend() scoped = EngagementFilesystemBackend(backend, "/workspace/test")