Skip to content

Commit eff8654

Browse files
author
Ovtcharov
committed
fix(agent-gaia): end a turn parked on a confirmation when stdin closes
Closing the host while a permission prompt was on screen left the agent process running forever, holding the model slot — only the TUI's two-second kill grace ended it, so any host that closed stdin and waited politely got an immortal child. Turning unattended approval on, or launching with it on, also left no trace anywhere in a normal session: nobody could tell afterwards whether a gated tool had been approved or had just run. - stdin EOF now cancels the in-flight turn before queueing the run-loop sentinel, which sits behind the running turn and could never reach it. The turn still ends through its normal path, with its one terminal event. - Permission-state changes and denied or dropped decisions log through a channel pinned above the user/dev level split, and only to the log file — stdout stays the wire. - A dead stdout ends the run loop instead of raising a second time out of the handler that exists to keep the process alive. - Smaller: the decision handoff is atomic, the stdin pump always queues its sentinel, a Lemonade client that fails to build says why instead of reading as a dead server, and the turn hands the console back when it ends. Three "unit" tests were opening live sockets to Lemonade, and the wire-purity test asserted json.dumps output was JSON rather than exercising the logging lockdown that actually protects the wire. The stdin pump, the argv contract the Go side pins, the post-terminal write clamp, and the error paths of _terminal_error and _memory_dump_event had no coverage at all.
1 parent 3ec80f1 commit eff8654

4 files changed

Lines changed: 818 additions & 62 deletions

File tree

hub/agents/gaia/python/gaia_agent/memory_dump.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
2+
# SPDX-License-Identifier: MIT
13
"""Builds the read-only snapshot the TUI's ``/memory`` view renders.
24
35
The agent process already holds an open ``MemoryStore`` (via ``MemoryMixin``),

hub/agents/gaia/python/gaia_agent/stdio.py

Lines changed: 128 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# Copyright(C) 2025-2026 Advanced Micro Devices, Inc. All rights reserved.
2+
# SPDX-License-Identifier: MIT
13
"""Run the flagship agent over stdin/stdout as newline-delimited JSON.
24
35
The collapsed transport: the TUI spawns this process once and keeps it, writing
@@ -60,6 +62,7 @@
6062
from __future__ import annotations
6163

6264
import json
65+
import logging
6366
import os
6467
import queue
6568
import sys
@@ -76,6 +79,20 @@
7679

7780
logger = get_logger(__name__)
7881

82+
#: Level the permission audit trail is pinned at, independent of --dev.
83+
AUDIT_LEVEL = logging.INFO
84+
85+
#: Logger carrying permission-state history: bypass toggles and every
86+
#: decision that was denied or dropped.
87+
#:
88+
#: It needs a channel of its own because user mode logs ERROR only and the
89+
#: control channel writes nothing to stdout by design (see ``apply_control``)
90+
#: — so without this, turning unattended approval ON leaves no record
91+
#: anywhere. ``_configure_logging`` pins it to the log FILE; it must never
92+
#: reach stdout, which is the wire.
93+
AUDIT_LOGGER_NAME = "gaia_agent.stdio.audit"
94+
audit = get_logger(AUDIT_LOGGER_NAME)
95+
7996
AGENT_ID = "gaia"
8097

8198
#: Key that marks a stdin line as a control message rather than a query.
@@ -123,6 +140,10 @@ def __init__(self, bypass: bool = False) -> None:
123140
self._bypass = bypass
124141
self._grants: set = set()
125142
self._handler: Any = None
143+
if bypass:
144+
# Starting unattended is the same security event as toggling it on
145+
# mid-session, and it never went through set_bypass.
146+
audit.warning("Bypass permissions ENABLED at launch")
126147

127148
@property
128149
def bypass(self) -> bool:
@@ -139,7 +160,7 @@ def set_bypass(self, enabled: bool) -> None:
139160
self._bypass = enabled
140161
if self._handler is not None:
141162
self._handler.auto_approve_gated_tools = enabled
142-
logger.warning("Bypass permissions %s", "ENABLED" if enabled else "disabled")
163+
audit.warning("Bypass permissions %s", "ENABLED" if enabled else "disabled")
143164

144165
def attach(self, handler: Any) -> None:
145166
"""Hand a turn's handler the session's accumulated permission state."""
@@ -159,17 +180,42 @@ def detach(self, handler: Any) -> None:
159180
self._handler = None
160181

161182
def resolve(self, decision: str, confirm_id: Optional[str]) -> None:
162-
"""Answer the confirmation the agent thread is parked on."""
183+
"""Answer the confirmation the agent thread is parked on.
184+
185+
The lock is held across the resolve: dropping it first lets the turn
186+
thread detach and the next turn attach a different handler in between,
187+
and a decision carrying no ``confirm_id`` would then be accepted by a
188+
handler nobody is waiting on while the live prompt keeps waiting.
189+
"""
163190
with self._lock:
164191
handler = self._handler
165-
if handler is None:
166-
logger.warning("Dropped a '%s' tool decision: no turn is running", decision)
167-
return
168-
handler.resolve_tool_confirmation(
169-
approved=decision in (DECISION_ALLOW, DECISION_ALWAYS),
170-
always=decision == DECISION_ALWAYS,
171-
confirm_id=confirm_id,
172-
)
192+
if handler is None:
193+
audit.warning(
194+
"Dropped a '%s' tool decision: no turn is running", decision
195+
)
196+
return
197+
handler.resolve_tool_confirmation(
198+
approved=decision in (DECISION_ALLOW, DECISION_ALWAYS),
199+
always=decision == DECISION_ALWAYS,
200+
confirm_id=confirm_id,
201+
)
202+
203+
def cancel_active(self) -> bool:
204+
"""Cancel the turn currently running, if any. True if one was cancelled.
205+
206+
stdin closing means the host is gone, but the sentinel that ends the run
207+
loop sits BEHIND the running turn in the query queue — so a turn parked
208+
on a confirmation nobody can answer would keep the process alive forever,
209+
holding the model slot. Cancelling unblocks the wait, which lets the turn
210+
finish through its normal path and emit its one terminal event.
211+
"""
212+
with self._lock:
213+
handler = self._handler
214+
if handler is None:
215+
return False
216+
handler.cancelled.set()
217+
audit.warning("stdin closed mid-turn — cancelled the in-flight turn")
218+
return True
173219

174220

175221
def parse_control(line: str) -> Optional[Dict[str, Any]]:
@@ -221,7 +267,7 @@ def apply_control(message: Dict[str, Any], state: PermissionState) -> None:
221267
decision = str(message.get("decision") or DECISION_DENY)
222268
if decision not in (DECISION_ALLOW, DECISION_DENY, DECISION_ALWAYS):
223269
# Fail closed: an unreadable decision is not consent.
224-
logger.warning("Unknown tool decision %r — denying", decision)
270+
audit.warning("Unknown tool decision %r — denying", decision)
225271
decision = DECISION_DENY
226272
confirm_id = message.get("confirm_id")
227273
state.resolve(decision, str(confirm_id) if confirm_id else None)
@@ -303,8 +349,13 @@ def _lemonade_health(base_url: Optional[str]) -> Dict[str, Any]:
303349
"""
304350
try:
305351
client = LemonadeClient(base_url=base_url, verbose=False)
306-
except Exception: # pylint: disable=broad-exception-caught
307-
return {"lemonade_reachable": False}
352+
except Exception as exc: # pylint: disable=broad-exception-caught
353+
# A malformed base_url reads to the user as "Lemonade isn't running",
354+
# so name it rather than reporting a bare unreachable.
355+
logger.warning(
356+
"[lemonade] client construction failed for %r: %s", base_url, exc
357+
)
358+
return {"lemonade_base_url": base_url, "lemonade_reachable": False}
308359
state: Dict[str, Any] = {"lemonade_base_url": client.base_url}
309360
try:
310361
health = client.health_check() or {}
@@ -409,9 +460,7 @@ def _apply_switch(
409460
Snapshots everything first and restores it on ANY exception —
410461
``rebuild_system_prompt()`` runs inside this same guarded block, so a bug
411462
in prompt composition rolls back the client swap too, instead of leaving
412-
the session on a working new client with a half-composed prompt (the
413-
original version of this function called rebuild AFTER mutating, which
414-
could not roll back — caught in review).
463+
the session on a working new client with a half-composed prompt.
415464
"""
416465
snapshot = _snapshot_switch_state(agent)
417466
chat = agent.chat
@@ -599,23 +648,32 @@ def _pump_stdin(queries: "queue.Queue", state: PermissionState) -> None:
599648
itself, so while a turn ran nothing was reading — which is exactly when a
600649
confirmation decision needs to arrive. Control messages are handled here,
601650
inline, while the agent thread is still parked on the prompt.
651+
652+
The teardown in ``finally`` is the process's only exit signal, so it has to
653+
run even if iterating stdin itself raises: without it ``main`` waits on a
654+
queue nothing will ever fill again.
602655
"""
603-
for raw in sys.stdin:
604-
line = raw.strip()
605-
if not line:
606-
continue
607-
control = parse_control(line)
608-
if control is None:
609-
queries.put(parse_query(line))
610-
continue
611-
try:
612-
apply_control(control, state)
613-
except Exception: # pylint: disable=broad-exception-caught
614-
# A malformed control line must never take the pump down: losing
615-
# this thread means every later confirmation hangs with nothing
616-
# able to answer it.
617-
logger.exception("control message failed: %s", line)
618-
queries.put(None) # stdin closed
656+
try:
657+
for raw in sys.stdin:
658+
line = raw.strip()
659+
if not line:
660+
continue
661+
control = parse_control(line)
662+
if control is None:
663+
queries.put(parse_query(line))
664+
continue
665+
try:
666+
apply_control(control, state)
667+
except Exception: # pylint: disable=broad-exception-caught
668+
# A malformed control line must never take the pump down:
669+
# losing this thread means every later confirmation hangs with
670+
# nothing able to answer it.
671+
logger.exception("control message failed: %s", line)
672+
finally:
673+
# Cancel BEFORE the sentinel: the sentinel is queued behind the running
674+
# turn, so a turn parked on a confirmation would never reach it.
675+
state.cancel_active()
676+
queries.put(None) # stdin closed
619677

620678

621679
def _write(event: Dict[str, Any], out) -> None:
@@ -646,14 +704,9 @@ def _record_turn(agent: Any, query: str, answer: str) -> None:
646704
647705
Without this the flagship is amnesiac over stdio. ``Agent`` composes each
648706
request as ``[system, *conversation_history, user]`` (see
649-
``_build_messages``), and nothing in the base class ever appends to
650-
``conversation_history`` — the HTTP surface fills it in per request
651-
(``gaia/ui/agent_loop.py``), and this transport did not. So every TUI turn
652-
reached the model as exactly two messages, system + the current question.
653-
654-
The user-visible cost was not subtle: asked "print issue 2975" one turn
655-
after a triage of amd/gaia, the agent replied "I need to know which
656-
repository it belongs to". It was not being told.
707+
``_build_messages``) and nothing in the base class ever appends to
708+
``conversation_history``, so a turn this transport does not record reaches
709+
the model as system + the current question and nothing else.
657710
658711
Only the question and the final answer are kept. Tool calls and their
659712
results belong to the turn that made them and the agent already threads
@@ -689,7 +742,6 @@ def log_path() -> "Path":
689742
against yours. Every line also carries its pid (see ``_configure_logging``)
690743
so the shared default stays attributable when no override is set.
691744
"""
692-
import os
693745
from pathlib import Path
694746

695747
override = os.environ.get(LOG_PATH_ENV, "").strip()
@@ -710,9 +762,9 @@ def _configure_logging(real_stdout, *, dev: bool) -> "Path":
710762
User mode logs errors only — a healthy run should leave a boring file.
711763
``--dev`` turns on DEBUG for the whole tree, because the questions a
712764
developer asks (which tool, how long, why that step) are answered by the
713-
records user mode drops.
765+
records user mode drops. The permission audit trail is deliberately outside
766+
that split — see ``AUDIT_LOGGER_NAME``.
714767
"""
715-
import logging
716768
from pathlib import Path
717769

718770
sys.stdout = sys.stderr
@@ -752,6 +804,21 @@ def _configure_logging(real_stdout, *, dev: bool) -> "Path":
752804
# process-wide, so every debug call builds its LogRecord just for
753805
# the handler to drop it.
754806
lg.setLevel(logging.NOTSET)
807+
808+
# Configured last, so the NOTSET sweep above cannot clear it. Its own
809+
# handler at AUDIT_LEVEL is what keeps a bypass toggle on the record in
810+
# user mode, where the shared handler drops everything below ERROR.
811+
audit_handler = logging.FileHandler(path, encoding="utf-8")
812+
audit_handler.setLevel(AUDIT_LEVEL)
813+
audit_handler.setFormatter(
814+
logging.Formatter(
815+
"%(asctime)s | pid:%(process)d | AUDIT | %(name)s | %(message)s"
816+
)
817+
)
818+
audit_log = logging.getLogger(AUDIT_LOGGER_NAME)
819+
audit_log.handlers = [audit_handler]
820+
audit_log.setLevel(AUDIT_LEVEL)
821+
audit_log.propagate = False
755822
return Path(path)
756823

757824

@@ -832,6 +899,7 @@ def run_turn(
832899
from gaia.ui.sse_handler import SSEOutputHandler
833900

834901
handler = SSEOutputHandler()
902+
previous_console = getattr(agent, "console", None)
835903
agent.console = handler
836904
if state is not None:
837905
state.attach(handler)
@@ -936,6 +1004,10 @@ def _run() -> None:
9361004
# is no longer listening, and the prompt after it would hang.
9371005
if state is not None:
9381006
state.detach(handler)
1007+
# Base-agent threads outlive their turn (``_call_tool_bounded`` leaves a
1008+
# timed-out worker running), so a handler left attached between turns
1009+
# accumulates events on a queue nobody drains.
1010+
agent.console = previous_console
9391011

9401012

9411013
def dispatch_query(
@@ -1059,16 +1131,27 @@ def main(argv: Optional[list] = None) -> int:
10591131
break
10601132
try:
10611133
dispatch_query(agent, query, out, dev=args.dev, state=state)
1134+
except BrokenPipeError:
1135+
# The wire is the parent. It being gone is not a turn to report.
1136+
logger.warning("stdout closed mid-turn — ending the run loop")
1137+
break
10621138
except Exception as exc: # never let one bad turn kill the process
10631139
logger.exception("stdio turn crashed outside the run loop")
1064-
_write(_terminal_error(exc), out)
1140+
try:
1141+
_write(_terminal_error(exc), out)
1142+
except OSError:
1143+
logger.warning("stdout closed while reporting — ending the run loop")
1144+
break
10651145

10661146
# stdin closed: the parent is done with us. The agent leaves non-daemon
10671147
# threads behind (memory extraction, the filesystem watcher), so a plain
10681148
# return would hang the interpreter at shutdown waiting on them — a one-shot
10691149
# `run --query` sat for 400s until its caller killed it. Nothing here owns
10701150
# unflushed state: events are flushed per line and the DBs commit per write.
1071-
out.flush()
1151+
try:
1152+
out.flush()
1153+
except OSError as exc:
1154+
logger.warning("stdout was already gone at exit: %s", exc)
10721155
sys.stderr.flush()
10731156
os._exit(0)
10741157

0 commit comments

Comments
 (0)