Skip to content

KoogAgentPlanAdapter.plan: a throwing tick listener is indistinguishable from an LLM failure #999

Description

@bedaHovorka

Hazard (from the PR #998 code review — Important finding)

In KoogAgentPlanAdapter.plan(), the tick listeners are invoked **inside the same try
block whose catch handlers convert an LLM failure into a rule-based fallback dispatch (reportTick(TickOutcome.LLM_ACTIONS, …)at ~line 310 fires theCompositeTickListenerfan-out inside the try; the genericcatch (e: Exception)` at ~line 358 is the LLM-failure
handler).

If a tick listener throws, the catch handler cannot distinguish it from an LLM exception:

  1. It reports RULE_FALLBACK for a cycle the LLM already acted on — the cycle is counted
    twice in the planner metrics.
  2. It dispatches the fallback on top of already-emitted actions — the double-dispatch
    hazard the class KDoc warns about (only the idempotent downstream backstop contains it).
  3. runFallback's own reportTick in the catch handler is outside any try — a second throw
    from the same listener escapes plan() entirely to the driver thread.

The structure predates PR #998, but Task 9 widened the seam: MeasuringPlanAdapter, the
driver attribution listener, and the run recorder now all share it.

Current mitigations (why this is a hazard, not a live bug)

  • MeasuringPlanAdapter.onTick is contractually MUST NOT THROW — pinned by test.
  • DefaultDispatcherRunRecorder.onTick is total (map lookups only).
  • CompositeTickListener deliberately propagates listener exceptions.

Fix direction (agreed in review + gemma4 consult, 2026-08-29)

Isolate the fan-out from plan's catch — either:

  • (A) catch per listener inside CompositeTickListener.onTick and log the error
    (contains the blast radius; changes the currently documented "deliberately propagates"
    semantics), or
  • (B) move the fan-out invocation out of plan's try block (contained to plan(), but a
    listener throw then escapes to the driver thread instead of triggering fallback).

Decide the semantics before the next consumer joins the seam — the fix cost grows with
each listener added. PR #998 documents the hazard with a comment at the catch handler and
keeps behavior unchanged (the PR's contract is "no behavior change outside Tasks 9–12").

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions