diff --git a/backend/apps/agents/manager/predict_followups.py b/backend/apps/agents/manager/predict_followups.py index 9c4601be..208dd9b2 100644 --- a/backend/apps/agents/manager/predict_followups.py +++ b/backend/apps/agents/manager/predict_followups.py @@ -22,6 +22,11 @@ MIN_EXCHANGES = 2 # Enough tail to know where the conversation is, small enough to stay a sub-cent aux call. P_TAIL_MESSAGES = 12 P_PER_MESSAGE_CAP = 700 +# The model's own words go in gisted and unlabelled. This tail is sent to an aux model on the +# user's own lane (a Claude subscription for most people), and up to 12 turns of verbatim +# `User:/Assistant:` was the exact shape ENG-358 removed from the recap, spent here on suggestion +# chips. Whether the filter keys on it is unknown; the trade is not, so it costs a gist (ENG-396). +P_MODEL_TEXT_CAP = 200 @typechecked @@ -39,9 +44,13 @@ def conversation_tail(session: AgentSession) -> str: if getattr(m, "hidden", False) or m.role not in ("user", "assistant"): continue text = m.content if isinstance(m.content, str) else str(m.content) - if len(text) > P_PER_MESSAGE_CAP: - text = text[:P_PER_MESSAGE_CAP] + "..." - lines.append(f"{'User' if m.role == 'user' else 'Assistant'}: {text}") + if m.role == "user": + # The user's own words are not model output; they are what we are predicting from. + lines.append(f"They asked: {text[:P_PER_MESSAGE_CAP]}" + + ("..." if len(text) > P_PER_MESSAGE_CAP else "")) + else: + gist = text[:P_MODEL_TEXT_CAP] + ("..." if len(text) > P_MODEL_TEXT_CAP else "") + lines.append(f"They were answered, in gist: {gist}") return "\n".join(lines) diff --git a/backend/apps/agents/manager/session/history_compaction.py b/backend/apps/agents/manager/session/history_compaction.py index dc016dac..d9024761 100644 --- a/backend/apps/agents/manager/session/history_compaction.py +++ b/backend/apps/agents/manager/session/history_compaction.py @@ -132,6 +132,56 @@ def get_branch_messages(session) -> List: return result +@typechecked +def trail_lines(messages, cutoff_msg_id: Optional[str] = None) -> List[str]: + """The user's asks and the tool trail, and NEVER a line of model-authored prose. + + Extracted so every renderer bound for a model's context shares one definition of what is safe + to send. Two others were still emitting raw `USER:/ASSISTANT:` replays of another agent's chat + (ENG-396), which is the exact shape ENG-358 removed from the recap; a safety property with two + implementations is one drift away from being no safety property at all. + """ + from backend.apps.agents.manager.session.aged_recap_lines import age_tool_results + cutoff_idx = -1 + if cutoff_msg_id: + cutoff_idx = next((i for i, m in enumerate(messages) if m.id == cutoff_msg_id), -1) + visible = [(i, m) for i, m in enumerate(messages) if not getattr(m, "hidden", False)] + fates = age_tool_results([m for _, m in visible], cutoff_idx=next( + (v for v, (i, _) in enumerate(visible) if i == cutoff_idx), -1)) + lines: List[str] = [] + for v, (i, m) in enumerate(visible): + if m.role == "user": + text = m.content if isinstance(m.content, str) else str(m.content) + lines.append(f"The user asked: {strip_forged_sentinels(clamp_recap_text(text))}") + elif m.role == "tool_call": + lines.append(recap_tool_call_line(m.content)) + elif m.role == "tool_result": + body = fates.get(v) + if body is None: + lines.append(recap_tool_result_line(m.content)) + else: + tool_name = m.content.get("tool_name") if isinstance(m.content, dict) else None + label = f"Tool result ({tool_name})" if tool_name else "Tool result" + lines.append(f"{label}: {strip_forged_sentinels(body)}") + return lines + + +@typechecked +def render_agent_trail(messages, max_chars: int = 14_000) -> str: + """What ANOTHER agent's run did, for a model that has to reason about it. + + Same safe body as the recap, different framing: this is someone else's run, not your own past. + Tail-biased cap so the end, where a run succeeds or blows up, always survives. + """ + lines = trail_lines(messages) + if not lines: + return "" + out = "\n".join(lines) + if len(out) > max_chars: + out = "...(earlier steps trimmed)...\n" + out[-max_chars:] + return out + + @typechecked def build_history_prefix(messages, cutoff_msg_id: Optional[str] = None) -> str: """Format branch messages into a conversation summary for context injection. @@ -148,28 +198,7 @@ def build_history_prefix(messages, cutoff_msg_id: Optional[str] = None) -> str: # Aging replaced dropping (ENG-354, hermes lift): pre-cutoff history becomes re-runnable # one-line stubs instead of vanishing, duplicates collapse, and the newest tool results # survive verbatim inside a budget, so a context break costs detail, never the trail. - from backend.apps.agents.manager.session.aged_recap_lines import age_tool_results - cutoff_idx = -1 - if cutoff_msg_id: - cutoff_idx = next((i for i, m in enumerate(messages) if m.id == cutoff_msg_id), -1) - visible = [(i, m) for i, m in enumerate(messages) if not getattr(m, "hidden", False)] - fates = age_tool_results([m for _, m in visible], cutoff_idx=next( - (v for v, (i, _) in enumerate(visible) if i == cutoff_idx), -1)) - lines = [] - for v, (i, m) in enumerate(visible): - if m.role == "user": - text = m.content if isinstance(m.content, str) else str(m.content) - lines.append(f"The user asked: {strip_forged_sentinels(clamp_recap_text(text))}") - elif m.role == "tool_call": - lines.append(recap_tool_call_line(m.content)) - elif m.role == "tool_result": - body = fates.get(v) - if body is None: - lines.append(recap_tool_result_line(m.content)) - else: - tool_name = m.content.get("tool_name") if isinstance(m.content, dict) else None - label = f"Tool result ({tool_name})" if tool_name else "Tool result" - lines.append(f"{label}: {strip_forged_sentinels(body)}") + lines = trail_lines(messages, cutoff_msg_id) if not lines: return "" # Framing lifted from hermes-agent's compaction handoff (context_compressor.py, MIT): reference only, never active instructions, the message after it is the single source of truth, and an explicit end marker so a weak model cannot read the last line as fresh input. diff --git a/backend/apps/agents/schedule_mcp_server.py b/backend/apps/agents/schedule_mcp_server.py index e0cfa999..b51bba44 100644 --- a/backend/apps/agents/schedule_mcp_server.py +++ b/backend/apps/agents/schedule_mcp_server.py @@ -670,7 +670,10 @@ def handle_invoke_workflow(args: dict) -> dict: status = res.get("status") or "unknown" err_line = f"\nError: {res.get('error')}" if res.get("error") else "" transcript = res.get("transcript") or "(no transcript)" - return _ok(f"Workflow '{match.get('title')}' run {status}.{err_line}\n\n=== RUN TRANSCRIPT ===\n{transcript}\n=== END TRANSCRIPT ===") + # Labelled as a trail, not a TRANSCRIPT: the body no longer replays anyone's turns (ENG-396), + # and a header promising a transcript invites the model to treat it as one. + return _ok(f"Workflow '{match.get('title')}' run {status}.{err_line}\n\n" + f"=== WHAT THE RUN DID ===\n{transcript}\n=== END ===") HANDLERS = { diff --git a/backend/apps/workflows/workflows.py b/backend/apps/workflows/workflows.py index d3349fed..965231b2 100644 --- a/backend/apps/workflows/workflows.py +++ b/backend/apps/workflows/workflows.py @@ -583,45 +583,16 @@ def _enriched(wf: Workflow) -> dict: def p_render_test_transcript(messages: list, max_chars: int = 14000) -> str: - """Flatten a Test Agent's messages into a readable role-tagged transcript. + """What the Test Agent's run DID, for the Edit Agent to diagnose from. - Tail-biased cap so the end (where a run succeeds or blows up) always - survives, protecting the Edit Agent's context window. + This used to emit `USER: ... ASSISTANT: ...` verbatim, which is byte-for-byte the shape + Anthropic's filter blocks on the subscription lane and the exact thing ENG-358 removed from the + session recap; it survived because it is an MCP tool result rather than a recap (ENG-396). Now + it shares the recap's one definition of what is safe to send: the asks and the tool trail, with + the run's own status and error carried separately by the caller. """ - import json as json_mod - lines: list[str] = [] - for m in messages: - if getattr(m, "hidden", False): - continue - role = (getattr(m, "role", "") or "?").upper() - content = getattr(m, "content", "") - if isinstance(content, str): - text = content - elif isinstance(content, list): - parts: list[str] = [] - for b in content: - if not isinstance(b, dict): - parts.append(str(b)) - continue - kind = b.get("type") - if kind == "text": - parts.append(str(b.get("text") or "")) - elif kind == "tool_use": - parts.append(f"[tool {b.get('name')}] {json_mod.dumps(b.get('input') or {})[:300]}") - elif kind == "tool_result": - inner = b.get("content") - parts.append(f"[result] {inner if isinstance(inner, str) else json_mod.dumps(inner)[:300]}") - else: - parts.append(str(b)[:200]) - text = "\n".join(p for p in parts if p) - else: - text = "" - if text.strip(): - lines.append(f"{role}: {text.strip()}") - out = "\n\n".join(lines) - if len(out) > max_chars: - out = "...(earlier turns trimmed)...\n\n" + out[-max_chars:] - return out + from backend.apps.agents.manager.session.history_compaction import render_agent_trail + return render_agent_trail(messages, max_chars=max_chars) @workflows.router.get("/active") diff --git a/backend/tests/test_no_transcript_replay.py b/backend/tests/test_no_transcript_replay.py new file mode 100644 index 00000000..84d497d5 --- /dev/null +++ b/backend/tests/test_no_transcript_replay.py @@ -0,0 +1,100 @@ +"""Nothing bound for a model's context may replay another agent's turns. + +ENG-358 removed model-authored prose from the session recap because a `USER:/ASSISTANT:` replay is +the shape Anthropic's filter blocks on the subscription lane. Two renderers kept doing it anyway +(ENG-396) because they were MCP tool results rather than the recap: `ReadTestTranscript` and the +workflow-invoke result, both up to 14,000 chars of another agent's verbatim output. + +These pin the property at the shared chokepoint, and that the useful half survives. +""" + +import re + +from backend.apps.agents.manager.session.history_compaction import render_agent_trail, trail_lines +from backend.apps.workflows.workflows import p_render_test_transcript + + +class P_Msg: + def __init__(self, role, content, mid="m"): + self.role, self.content, self.id, self.hidden = role, content, mid, False + + +def p_run(): + return [ + P_Msg("user", "find out why the build fails"), + P_Msg("assistant", "Let me reason about this. I suspect the parser is at fault because..."), + P_Msg("tool_call", {"tool": "Bash", "input": {"command": "pytest -q"}}), + P_Msg("tool_result", {"tool_name": "Bash", "text": "3 failed, 12 passed\nFATAL: parser died"}), + P_Msg("assistant", "Based on my analysis the root cause is the parser's lookahead."), + ] + + +ROLE_REPLAY = re.compile(r"^\s*(USER|ASSISTANT|MODEL|AI)\s*:", re.MULTILINE | re.IGNORECASE) + + +def test_the_renderer_never_emits_a_role_tagged_replay(): + out = p_render_test_transcript(p_run()) + assert not ROLE_REPLAY.search(out), f"role-tagged replay leaked back in:\n{out}" + + +def test_no_model_authored_prose_survives(): + out = p_render_test_transcript(p_run()) + assert "root cause is the parser" not in out + assert "I suspect the parser" not in out + + +def test_the_useful_half_still_survives(): + # A control: the fix would be worthless if it also deleted what the Edit Agent diagnoses from. + out = p_render_test_transcript(p_run()) + assert "pytest -q" in out, "the command has to survive so the agent can re-run it" + assert "3 failed" in out and "FATAL" in out, "the verdict is the whole point" + assert "find out why the build fails" in out, "the user's own words are not model output" + + +def test_every_renderer_shares_one_definition_of_safe(): + # A safety property with two implementations is one drift away from being none. + src = open("backend/apps/workflows/workflows.py").read() + assert "render_agent_trail" in src + assert 'f"{role}: {text.strip()}"' not in src, "the replay formatter must not come back" + + +def test_the_invoke_result_no_longer_promises_a_transcript(): + src = open("backend/apps/agents/schedule_mcp_server.py").read() + assert "=== RUN TRANSCRIPT ===" not in src + assert "WHAT THE RUN DID" in src + + +def test_the_trail_is_capped_from_the_tail(): + big = [P_Msg("user", "go")] + [ + P_Msg("tool_call", {"tool": "Bash", "input": {"command": f"step-{i}"}}) for i in range(4000) + ] + [P_Msg("tool_call", {"tool": "Bash", "input": {"command": "LAST-STEP"}})] + out = render_agent_trail(big, max_chars=2_000) + assert len(out) < 2_400 + assert "LAST-STEP" in out, "a run's end is where it succeeds or blows up" + + +def test_an_empty_run_renders_empty_not_a_frame(): + assert render_agent_trail([]) == "" + assert trail_lines([]) == [] + + +def test_the_aux_conversation_tail_gists_model_text_and_keeps_the_user_verbatim(): + # Shared by predict_followups AND memory distillation, both aux calls on the user's own lane. + from backend.apps.agents.manager.predict_followups import conversation_tail, P_MODEL_TEXT_CAP + + class P_Sess: + pass + + import backend.apps.agents.manager.predict_followups as mod + sess = P_Sess() + msgs = [P_Msg("user", "how do I deploy this"), + P_Msg("assistant", "Here is my full reasoning. " + "z" * 900)] + orig = mod.get_branch_messages + mod.get_branch_messages = lambda s: msgs + try: + tail = conversation_tail(sess) + finally: + mod.get_branch_messages = orig + assert not ROLE_REPLAY.search(tail), f"role-tagged replay in the aux tail:\n{tail}" + assert "how do I deploy this" in tail, "the user's own words are what we predict from" + assert "z" * (P_MODEL_TEXT_CAP + 50) not in tail, "model prose must arrive gisted, not whole" diff --git a/backend/tests/test_predict_followups.py b/backend/tests/test_predict_followups.py index 246f81fd..c1376e5f 100644 --- a/backend/tests/test_predict_followups.py +++ b/backend/tests/test_predict_followups.py @@ -39,9 +39,12 @@ def test_tool_noise_does_not_count_as_exchanges(): def test_tail_contains_only_visible_user_assistant_text(): + # Asserts WHICH turns survive, not how they are labelled: the labels changed deliberately when + # role-tagged replay was removed from every renderer bound for a model (ENG-396). s = p_session("user", "tool_call", "assistant") tail = conversation_tail(s) - assert "User: m0" in tail and "Assistant: m2" in tail and "m1" not in tail + assert "m0" in tail and "m2" in tail, "both visible turns must survive" + assert "m1" not in tail, "tool noise is not part of the tail" def test_tail_caps_giant_messages():