Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions strix/interface/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ def main() -> None: # noqa: PLR0912, PLR0915
finally:
tracer = get_global_tracer()
if tracer:
tracer.cleanup()
posthog.end(tracer, exit_reason=exit_reason)

results_path = Path("strix_runs") / args.run_name
Expand Down
4 changes: 3 additions & 1 deletion strix/llm/llm.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,9 @@ def _prepare_messages(self, conversation_history: list[dict[str, Any]]) -> list[
conversation_history.extend(compressed)
messages.extend(compressed)

if messages[-1].get("role") == "assistant" and not self.config.interactive:
if messages[-1].get("role") == "assistant" and (
not self.config.interactive or self._is_anthropic()
):
messages.append({"role": "user", "content": "<meta>Continue the task.</meta>"})

if self._is_anthropic() and self.config.enable_prompt_caching:
Expand Down
18 changes: 5 additions & 13 deletions strix/tools/finish/finish_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,11 @@ def finish_scan(
if active_agents_error:
return active_agents_error

validation_errors = []

if not executive_summary or not executive_summary.strip():
validation_errors.append("Executive summary cannot be empty")
if not methodology or not methodology.strip():
validation_errors.append("Methodology cannot be empty")
if not technical_analysis or not technical_analysis.strip():
validation_errors.append("Technical analysis cannot be empty")
if not recommendations or not recommendations.strip():
validation_errors.append("Recommendations cannot be empty")

if validation_errors:
return {"success": False, "message": "Validation failed", "errors": validation_errors}
_NOT_PROVIDED = "[Not provided by model]"
executive_summary = (executive_summary or "").strip() or _NOT_PROVIDED
methodology = (methodology or "").strip() or _NOT_PROVIDED
technical_analysis = (technical_analysis or "").strip() or _NOT_PROVIDED
recommendations = (recommendations or "").strip() or _NOT_PROVIDED
Comment on lines +102 to +116
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent placeholder substitution removes model self-correction opportunity

Previously, if the model provided empty strings for required report fields, the function returned a structured error ({"success": False, ...}), giving the model a chance to retry with actual content. After this change, empty fields are silently replaced with "[Not provided by model]" and the scan is marked success: True. The model never learns that its output was incomplete, so it cannot retry — the user receives a scan report with placeholder text and no in-session opportunity for the model to fill in the missing data.

If the intent is to prevent infinite retry loops, a middle path would be to return a soft warning in the success response, or to log/surface the placeholder substitution to the user at completion time.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/tools/finish/finish_actions.py
Line: 102-106

Comment:
**Silent placeholder substitution removes model self-correction opportunity**

Previously, if the model provided empty strings for required report fields, the function returned a structured error (`{"success": False, ...}`), giving the model a chance to retry with actual content. After this change, empty fields are silently replaced with `"[Not provided by model]"` and the scan is marked `success: True`. The model never learns that its output was incomplete, so it cannot retry — the user receives a scan report with placeholder text and no in-session opportunity for the model to fill in the missing data.

If the intent is to prevent infinite retry loops, a middle path would be to return a soft warning in the success response, or to log/surface the placeholder substitution to the user at completion time.

How can I resolve this? If you propose a fix, please make it concise.


try:
from strix.telemetry.tracer import get_global_tracer
Expand Down
50 changes: 50 additions & 0 deletions tests/llm/test_prepare_messages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""Tests for LLM._prepare_messages trailing-assistant-message handling."""
from strix.llm.config import LLMConfig
from strix.llm.llm import LLM


def _make_llm(monkeypatch, model_name: str, interactive: bool) -> LLM:
monkeypatch.setenv("STRIX_LLM", model_name)
config = LLMConfig(model_name=model_name, interactive=interactive, enable_prompt_caching=False)
return LLM(config, agent_name=None)


def _history_ending_with_assistant() -> list[dict]:
return [
{"role": "user", "content": "Scan this target."},
{"role": "assistant", "content": "I found a vulnerability."},
]


def test_non_interactive_anthropic_adds_user_message(monkeypatch) -> None:
"""Non-interactive mode always appends a user message when history ends with assistant."""
llm = _make_llm(monkeypatch, "claude-sonnet-4-6", interactive=False)
history = _history_ending_with_assistant()
messages = llm._prepare_messages(history)
assert messages[-1]["role"] == "user"
assert messages[-1]["content"] == "<meta>Continue the task.</meta>"


def test_interactive_anthropic_adds_user_message(monkeypatch) -> None:
"""Interactive mode with Anthropic model must also append a user message.

Anthropic API rejects messages where the last entry has role 'assistant'
(no assistant prefill support). This should hold regardless of interactive mode.
"""
llm = _make_llm(monkeypatch, "claude-sonnet-4-6", interactive=True)
history = _history_ending_with_assistant()
messages = llm._prepare_messages(history)
assert messages[-1]["role"] == "user"
assert messages[-1]["content"] == "<meta>Continue the task.</meta>"


def test_interactive_non_anthropic_does_not_add_user_message(monkeypatch) -> None:
"""Interactive mode with a non-Anthropic model keeps the trailing assistant message.

Non-Anthropic models may support assistant prefill; in interactive mode the
caller (TUI) is responsible for appending the next user message.
"""
llm = _make_llm(monkeypatch, "openai/gpt-5.4", interactive=True)
history = _history_ending_with_assistant()
messages = llm._prepare_messages(history)
assert messages[-1]["role"] == "assistant"