fix(core): adapter/engine correctness — editor ambiguity, pytest SKIP, kilocode process handling, denylist gaps (#955) (#1088)

* fix(core): reject ambiguous fuzzy edits, skip unspawnable gates, bound child output (#955)

Three of the five #955 defects, each with the regression test its acceptance
criterion asks for.

editor: _match_fuzzy hardcoded match_count=1, so the ambiguity rejection every
other match level feeds (editor.py's match_count > 1 guard) was bypassed at the
one level where near-misses are expected. Two blocks scoring identically meant
the agent silently edited whichever came first. Windows that overlap are one
region seen at several offsets, so they still count once; separate regions that
tie are genuinely ambiguous and now say so.

gates: _run_pytest gated on shutil.which("uv"), which proves uv exists, not that
the target project has pytest. 'uv run pytest' then exits non-zero with 'Failed
to spawn' and the gate read FAILED -- 'CodeFRAME says my project is broken' when
the honest answer is 'unverifiable'. run_lint_on_file already drew this
distinction; the detection is now shared as _tool_is_missing.

subprocess_adapter: stdin=None *inherits* the parent's stdin, so a TTY-probing
CLI waits for input that never comes and burns the whole timeout having done
nothing -- DEVNULL answers the probe immediately. stdout/stderr were also
retained without bound, putting the driver's memory under the child's control;
both are capped now, keeping the tail (where an agent's conclusion is) and
saying when earlier output was dropped. stderr keeps draining past the cap, or
the child would deadlock on a full pipe -- the very thing the drain thread
exists to prevent.

Refs #955

* fix(kilocode): send the prompt over stdin, never argv (#955)

argv is world-readable. `ps` shows every element to every user on the machine,
so a prompt carrying the task description and whatever file excerpts were
assembled into it was on display for the duration of the run. It is bounded too:
Linux caps a single argv entry at 128 KiB, under CodeFrame's ~100K-token budget.

#1015 already routed *oversized* prompts to stdin and verified against 7.4.17
that `kilo run` with no positional reads the message from there. Size was never
the whole problem, so that path is now the only modern path -- which also
retires _prompt_exceeds_argv and the size branch it fed.

Legacy 0.22.0 is untouched: it has no stdin path, so the prompt stays positional
and an oversized one keeps failing loudly rather than silently doing nothing.

Refs #955

* fix(dangerous_commands): cover $HOME expansion and non-sh interpreters (#955)

Two patterns described more than they matched.

`[/~]` reads as "root or home", but home has another spelling. Nothing in this
module expands variables -- shlex.split leaves $HOME as a literal token and the
shell expands it afterwards -- so `rm -rf $HOME` was being checked against a
pattern that could never see it. `\b` after HOME keeps $HOMEDIR and
$HOMEBREW_PREFIX out, which is tested.

`(ba)?sh` reads as "a shell" and matches exactly two of them; `curl ... | zsh`
and `curl ... | python` were not a piped download as far as the denylist was
concerned. The interpreter list now covers the sh family, fish, python, perl,
ruby, node and php, with an optional sudo between the pipe and the interpreter.

The trailing \b also fixes a false positive in the other direction: `(ba)?sh`
with no boundary matched `curl ... | shasum -a 256`, blocking a checksum.

Refs #955

* fix(core): remove dead surface and correct messages that stopped being true (#955)

Each of these told a reader something false.

AgentResultStatus had no production caller and defined TIMEOUT, which
AgentResult.status (a Literal of completed/failed/blocked) will not accept --
so the enum was not just unused but divergent. Removed with its export and its
tests. engine_stats._update_aggregate_stats likewise: no callers, and a
docstring advertising external ones that do not exist.

IsolationLevel.CLOUD still said 'reserved for the future E2B agent adapter
phase' after E2B shipped. Cloud execution is an *engine*, not an isolation
level, so the error now names --engine cloud and --isolation worktree instead of
a phase that already happened.

Builtin requirements() hardcoded ANTHROPIC_API_KEY, so 'cf engines check react'
called the engine unready on an OpenAI or Ollama workspace -- and ready on a
machine that merely had an Anthropic key exported while configured for something
else. It now resolves through the standard provider chain and reuses the
REQUIRED_KEY_ENV map that already existed in llm_resolution; local providers
report nothing to satisfy. check_requirements takes an optional repo_path so the
config.yaml tier applies, and the two CLI call sites pass cwd.

engine_registry built OpenCodeAdapter() with no arguments while every sibling
forwarded **kwargs, silently ignoring the caller's timeout_s and auto_approve.

_truncate_history could return an empty list -- one turn over the budget trimmed
to nothing -- and the caller sends that straight to the provider, where an empty
messages array is an API error. The request failed outright instead of the
history being shortened. It now keeps the last turn and lets the provider's own
limit judge it.

Refs #955

* test: make mocked stderr streams signal EOF (#955)

The bounded stderr drain reads in 65 KiB chunks and stops on a falsy chunk. A
MagicMock configured with `read.return_value = "Fatal error"` returns that same
string on every call, so the loop never reached EOF: the suite hung on the first
test with non-empty mocked stderr and pytest-timeout took the whole session down
with an INTERNALERROR. (Empty-string mocks were unaffected, which is why it
surfaced 197 tests in rather than immediately.)

A fake stream that never signals EOF is a broken fake -- the production loop is
correct for a real pipe -- so the fakes now do what a stream does. Also lifts an
`import os` in builtin.llm_key_requirement to module scope.

Refs #955

* fix(engines): resolve the workspace root, and stop inferring legacy kilo by elimination (#955)

Two defects found by the codex review of this branch.

cf engines list/check passed Path.cwd() straight through. Provider resolution
reads .codeframe/config.yaml in exactly the directory it is handed, so running
either command from repo/src/ missed a workspace configured for OpenAI and
reported ANTHROPIC_API_KEY. Same class of bug as #926, and find_workspace_root
-- added by that issue -- is the fix.

_detect_surface picked legacy by *elimination*: any --help output without the
modern marker counted as evidence for legacy, including output that was not help
text at all. codex's own sandbox demonstrated it -- kilo's log directory was
read-only, so --help printed a Bun EROFS stack trace and exited 1, and the
adapter answered by emitting '--auto --workspace' at a 7.x CLI, where --auto is
the permission bypass #916 established must stay off. The docstring already
promised that a failing --help falls back to modern; the code did not keep it.

Both surfaces are now selected on a marker they actually contain (--workspace
for legacy, 'kilo run' for modern), verified against the captured help of both
real CLIs, so text that is not help matches neither and falls back as
documented.

Refs #955

* fix(dangerous_commands): let sudo carry its own flags; disclose truncated stderr (#955)

Both findings from the PR review (claude-review and the GLM precision review
agreed on the first).

The sudo branch matched only a bare `sudo` directly before the interpreter, so
`curl … | sudo -E bash`, `sudo -i bash` and `sudo -u root bash` all returned
(False, "") while `sudo bash` was caught. Virtually every real sudo-piped
install carries one of those flags, so the download-to-*rooted*-shell case --
the worst one -- was precisely the one getting through, and the added coverage
was narrower than it looked. Verified before fixing: 3 of the 4 spellings
bypassed. `sudo tee` and `sudo -u nobody jq` stay allowed, and are tested.

stdout announced dropped lines; stderr was capped in silence, so a truncated
error message read as the whole story to whoever was debugging a failed run.
It now carries the same kind of marker.

Refs #955

* test(kilocode): skip the installed-CLI smoke tests when --help cannot run (#955)

Second codex review pass, and it is right that the two halves disagreed. This
branch makes an unreadable `kilo --help` a *supported* state -- detection falls
back to modern, asserted directly in test_kilocode_prompt_955 -- but
test_the_installed_cli_matches_one_of_the_two_known_surfaces still read the same
EROFS crash log as evidence of an unknown third surface and failed. Any sandbox
or hardened CI image with kilo installed and an unwritable log dir stayed red on
a case the adapter handles by design.

When the binary cannot produce help, the installed surface is unknowable, so
these two tests have nothing to measure and skip with the reason. Their real job
-- catching the next CLI rewrite -- is unaffected: a kilo whose --help works and
matches neither surface still fails.

Refs #955

* fix(subprocess_adapter): set the stderr-truncation flag on the crossing chunk (#955)

The GLM precision review caught a boundary hole in 86d059f, the commit that
added the marker. Reproduced before fixing.

`retained` counts the whole chunk while only `chunk[:room]` is appended, so the
chunk that crosses the cap takes the retaining branch and leaves the flag alone
-- it was only ever set by a *later* read. When the crossing chunk is the last
one before EOF (total stderr between the cap and one 64 KiB read above it), the
loop exits with the flag still false and the dropped tail reads as a complete
error message. That is precisely the case the marker was added for, so the fix
was defeated in exactly its own boundary window.

The existing test writes 200 KB against a 1000-char cap, so several non-empty
chunks always followed the crossing one and the path never ran. The new test
writes 1500 chars, which arrive in a single read.

Refs #955

Author: frankbria

codeframe/cli/engines_commands.py

@@ -30,6 +30,23 @@
 )
 
 
+def _config_root() -> Optional[Path]:
+    """The enclosing workspace root, for the builtin engines' provider lookup.
+
+    ``Path.cwd()`` alone is wrong from a subdirectory: the provider resolution
+    reads ``.codeframe/config.yaml`` in exactly the directory it is handed, so
+    ``cf engines check react`` run from ``repo/src/`` misses a workspace
+    configured for OpenAI and reports ANTHROPIC_API_KEY instead. Same class of
+    bug as #926, and the same walk-up helper fixes it.
+
+    None means "not inside a workspace", which correctly leaves only the
+    environment tier to answer.
+    """
+    from codeframe.core.workspace import find_workspace_root
+
+    return find_workspace_root(Path.cwd())
+
+
 @engines_app.command("list")
 def engines_list() -> None:
     """List all available execution engines and their requirement status."""
@@ -46,7 +63,9 @@ def engines_list() -> None:
             engine_type = "alias → react"
 
         try:
-            reqs = check_requirements(engine)
+            # The workspace root, so a builtin engine's requirement reflects the
+            # provider in this workspace's .codeframe/config.yaml (#955).
+            reqs = check_requirements(engine, _config_root())
         except ValueError:
             reqs = {}
 
@@ -72,7 +91,7 @@ def engines_check(
     from codeframe.core.engine_registry import check_requirements
 
     try:
-        reqs = check_requirements(name)
+        reqs = check_requirements(name, _config_root())
     except ValueError as e:
         console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(1)

codeframe/core/adapters/__init__.py

@@ -10,7 +10,6 @@
     AgentContext,
     AgentEvent,
     AgentResult,
-    AgentResultStatus,
 )
 from codeframe.core.adapters.builtin import (
     BuiltinPlanAdapter,
@@ -32,7 +31,6 @@
     "AgentContext",
     "AgentEvent",
     "AgentResult",
-    "AgentResultStatus",
     "BuiltinPlanAdapter",
     "BuiltinReactAdapter",
     "ClaudeCodeAdapter",

codeframe/core/adapters/agent_adapter.py

@@ -9,20 +9,10 @@
 
 from dataclasses import dataclass, field
 from datetime import datetime, timezone
-from enum import Enum
 from pathlib import Path
 from typing import Callable, Literal, Protocol, runtime_checkable
 
 
-class AgentResultStatus(str, Enum):
-    """Terminal status from an agent execution."""
-
-    COMPLETED = "completed"
-    FAILED = "failed"
-    BLOCKED = "blocked"
-    TIMEOUT = "timeout"
-
-
 @dataclass
 class AdapterTokenUsage:
     """Lightweight token usage for adapter results."""

codeframe/core/adapters/builtin.py

@@ -7,6 +7,7 @@
 from __future__ import annotations
 
 import logging
+import os
 from pathlib import Path
 from typing import TYPE_CHECKING, Callable, Optional
 
@@ -28,6 +29,32 @@
 _MAX_STALL_RETRIES = 1
 
 
+def llm_key_requirement(repo_path: Optional[Path] = None) -> dict[str, str]:
+    """The API key the *configured* LLM provider needs, if it needs one.
+
+    Builtin engines call the LLM directly, so a hardcoded ``ANTHROPIC_API_KEY``
+    made ``cf engines check react`` report the engine unready on an OpenAI or
+    Ollama workspace — and ready on a machine that merely happens to have an
+    Anthropic key exported while being configured for something else (#955).
+
+    The provider comes from the standard chain (``CODEFRAME_LLM_PROVIDER`` →
+    ``.codeframe/config.yaml`` → anthropic); ``repo_path`` is what enables the
+    config tier. Local providers (ollama, vllm, compatible) need no key, so they
+    report nothing to satisfy — which is what "Ready" should mean for them.
+    """
+    from codeframe.core.llm_resolution import REQUIRED_KEY_ENV, resolve_llm_settings
+
+    try:
+        provider = resolve_llm_settings(repo_path=repo_path).provider_type
+    except Exception:  # noqa: BLE001 - see below
+        # A requirements *check* must not be the thing that fails on a broken or
+        # untrusted workspace config; the run itself reports that properly.
+        provider = os.getenv("CODEFRAME_LLM_PROVIDER") or "anthropic"
+
+    key = REQUIRED_KEY_ENV.get(provider)
+    return {key: f"API key for the configured LLM provider ({provider})"} if key else {}
+
+
 def _adapter_token_usage(agent: object) -> Optional[AdapterTokenUsage]:
     """Build AdapterTokenUsage from a builtin agent's accumulated records.
 
@@ -93,9 +120,9 @@ def name(self) -> str:
         return "react"
 
     @classmethod
-    def requirements(cls) -> dict[str, str]:
-        """Return requirement names and descriptions."""
-        return {"ANTHROPIC_API_KEY": "Anthropic API key for LLM calls"}
+    def requirements(cls, repo_path: Optional[Path] = None) -> dict[str, str]:
+        """Return requirement names and descriptions (#955: provider-aware)."""
+        return llm_key_requirement(repo_path)
 
     def run(
         self,
@@ -211,9 +238,9 @@ def name(self) -> str:
         return "plan"
 
     @classmethod
-    def requirements(cls) -> dict[str, str]:
-        """Return requirement names and descriptions."""
-        return {"ANTHROPIC_API_KEY": "Anthropic API key for LLM calls"}
+    def requirements(cls, repo_path: Optional[Path] = None) -> dict[str, str]:
+        """Return requirement names and descriptions (#955: provider-aware)."""
+        return llm_key_requirement(repo_path)
 
     def run(
         self,

codeframe/core/adapters/kilocode.py

@@ -29,18 +29,16 @@
 #: #1012). Help text is what the binary actually offers.
 _RUN_SUBCOMMAND_MARKER = "kilo run"
 
-#: Linux caps a *single* argv entry at MAX_ARG_STRLEN — 128 KiB — independently
-#: of the much larger total ARG_MAX. Same constraint opencode hit in #913.
-_MAX_ARG_BYTES = 128 * 1024
+#: The legacy workspace flag, which 7.x renamed to ``--dir``. Legacy is chosen on
+#: this *positive* marker rather than by elimination, because "not modern" is not
+#: the same claim as "legacy" — see ``_detect_surface``. Verified against both
+#: captured help fixtures: each marker appears in exactly one of them.
+_LEGACY_WORKSPACE_MARKER = "--workspace"
 
 #: Detection runs a subprocess, so it is cached per binary path for the process.
 _SURFACE_CACHE: dict[str, str] = {}
 
 
-def _prompt_exceeds_argv(prompt: str) -> bool:
-    return len(prompt.encode("utf-8")) >= _MAX_ARG_BYTES
-
-
 def _detect_surface(binary_path: str) -> str:
     """Which kilo CLI is installed, according to its own ``--help``.
 
@@ -61,6 +59,15 @@ def _detect_surface(binary_path: str) -> str:
     An unreadable or failing ``--help`` falls back to modern — the version any
     new install gets, and the one whose ``run`` subcommand fails loudly rather
     than opening a TUI that hangs until the timeout (#1012).
+
+    That fallback used to be a promise the code did not keep. Legacy was chosen
+    by *elimination* — anything without the modern marker — so a ``--help`` that
+    ran and printed an error still counted as evidence for legacy. A sandboxed
+    kilo whose log directory is read-only prints a Bun stack trace and exits 1;
+    the adapter concluded "legacy" and emitted ``--auto --workspace`` at a CLI
+    that has neither. Both surfaces are now chosen on a marker they actually
+    contain, so output that is not help text matches neither and falls back as
+    documented. (#955)
     """
     if binary_path in _SURFACE_CACHE:
         return _SURFACE_CACHE[binary_path]
@@ -83,7 +90,11 @@ def _detect_surface(binary_path: str) -> str:
         )
         help_text = ""
 
-    surface = _MODERN if (not help_text or _RUN_SUBCOMMAND_MARKER in help_text) else _LEGACY
+    is_legacy = (
+        _LEGACY_WORKSPACE_MARKER in help_text
+        and _RUN_SUBCOMMAND_MARKER not in help_text
+    )
+    surface = _LEGACY if is_legacy else _MODERN
     _SURFACE_CACHE[binary_path] = surface
     return surface
 
@@ -109,12 +120,15 @@ class KilocodeAdapter(SubprocessAdapter):
       got it swallowed as the prompt, opening the TUI to hang until the timeout
       having written nothing (#1012).
 
-    Prompt length: the prompt is a single argv entry, and Linux caps one entry
-    at 128 KiB (macOS at 256 KB) — well under CodeFrame's ~100K-token budget. On
-    7.x an oversized prompt goes to stdin instead, which ``kilo run`` accepts
-    when given no positional (verified: ``echo "say ok" | kilo run --dir /tmp``
-    reaches the model call). 0.22.0 has no stdin path, so there it still goes
-    positionally and fails loudly.
+    Prompt delivery: on 7.x the prompt always goes over **stdin**, never argv
+    (#955). argv is world-readable — the whole prompt, including whatever task
+    context it carries, shows up in ``ps`` for every user on the box — and Linux
+    caps a single argv entry at 128 KiB (macOS 256 KB), under CodeFrame's ~100K
+    token budget, so a large task raised ``OSError(E2BIG)`` before kilo started.
+    ``kilo run`` with no positional reads the message from stdin (verified
+    against 7.4.17: ``echo "say ok" | kilo run --dir /tmp`` reaches the model
+    call). 0.22.0 has no stdin path at all, so there the prompt stays positional
+    and an oversized one fails loudly rather than silently doing nothing.
 
     Exit codes:
         0   — success
@@ -197,9 +211,9 @@ def build_command(self, prompt: str, workspace_path: Path) -> list[str]:
             # all permissions", the 0.22 `--yolo` that #916 established must
             # stay off. Renaming --workspace to --dir while keeping --auto
             # would have silently upgraded the adapter into a permission bypass.
+            # No positional: the prompt goes over stdin (see get_stdin), keeping
+            # it out of `ps` and off the 128 KiB argv-entry ceiling (#955).
             cmd = [self._binary_path, "run", "--dir", str(workspace_path)]
-            if not _prompt_exceeds_argv(prompt):
-                cmd.append(prompt)
         else:
             # 0.22.0: bare positional prompt, --auto is merely non-interactive
             # and --yolo (never passed) is the bypass. Verified in #1012/#916.
@@ -222,21 +236,23 @@ def build_command(self, prompt: str, workspace_path: Path) -> list[str]:
         return cmd
 
     def get_stdin(self, prompt: str) -> str | None:
-        """The prompt, only when it is too large to survive as an argv entry.
-
-        Normally None — both eras take the prompt positionally. But Linux caps a
-        *single* argv entry at 128 KiB while CodeFrame budgets ~100K tokens of
-        prompt, so a large task would raise OSError(E2BIG) before kilo starts.
+        """The prompt on modern kilo; None on legacy, which has no stdin path.
 
         ``kilo run`` with no positional reads the message from stdin: verified
         against 7.4.17, where `echo "say ok" | kilo run --dir /tmp` gets past
-        message validation to the model call. The legacy CLI has no such path,
-        so an oversized prompt still goes positionally there and fails loudly
-        rather than silently doing nothing.
+        message validation to the model call.
+
+        This used to apply only to prompts over 128 KiB, the Linux cap on a
+        single argv entry. Size was never the whole problem: argv is readable by
+        every user on the machine via ``ps``, so a normal-sized prompt — task
+        description, file excerpts, whatever context was assembled — was on
+        display for the duration of the run (#955). Sending it over stdin fixes
+        both, and it is the same code path the oversized case already used.
+
+        0.22.0 takes the prompt positionally and has no stdin path, so there an
+        oversized prompt still fails loudly rather than silently doing nothing.
         """
-        if self._surface() == _MODERN and _prompt_exceeds_argv(prompt):
-            return prompt
-        return None
+        return prompt if self._surface() == _MODERN else None
 
     def _map_result(
         self,

codeframe/core/adapters/streaming_chat.py

@@ -320,9 +320,20 @@ def _count(msgs: list[dict]) -> int:
 
         while messages and _count(messages) > _MAX_HISTORY_TOKENS:
             # Drop in pairs so we don't strand an assistant message at index 0
-            messages = messages[2:] if len(messages) >= 2 else messages[1:]
-
-        # First message must have role "user"
+            trimmed = messages[2:] if len(messages) >= 2 else messages[1:]
+            if not trimmed:
+                # A single turn that busts the budget on its own would otherwise
+                # trim the history to nothing, and the caller sends this list
+                # straight to the provider — an empty `messages` is an API error,
+                # so the request fails instead of the history being shortened.
+                # Keep the last turn and let the provider's own limit judge it.
+                # (#955)
+                break
+            messages = trimmed
+
+        # First message must have role "user". An empty result here means the
+        # history holds no user turn at all — unusable, and the caller's own
+        # guard, not something truncation invented.
         while messages and messages[0].get("role") != "user":
             messages = messages[1:]