Skip to content

[Bug]: P0 Auditor review loop can exhaust tokens and prevent run termination #435

Description

@GTC2333

Bug Description

A host-bound Auditor turn can call edit_trace_review repeatedly for the same target. This can create an unbounded provider/tool loop, exceed the configured token budget by orders of magnitude, and leave an outer benchmark runner waiting after the session has already become inactive.

Observed impact in one interrupted benchmark run:

  • about 43.43M tokens consumed against an 800K budget (~54x)
  • node reviews repeatedly succeeded and overwrote the same audit
  • parent-edge reviews repeatedly failed after their fingerprint became stale
  • the final RUN_FINISHED lacked a run ID, so a runner that only matched terminal events by run ID did not clear its active set

This is a runtime safety issue, independent of the benchmark model-quality result.

Root Cause

  1. currentTraceAuditTarget remains valid until the entire agent.prompt() returns; a successful tool call does not consume it.
  2. edit_trace_review has no per-target/per-run idempotency guard or call limit.
  3. Trace.review() accepts repeated transitions and appends a new change record each time.
  4. A node audit fingerprint excludes reviewConclusion and reviewReason, so repeated node reviews keep passing the same expected fingerprint.
  5. A parent-edge fingerprint includes the mutable relation; after the first review it becomes stale, turning the same behavior into an error/retry loop.
  6. There is no enforced total token hard limit.
  7. MasAgent.runPrompt() emits terminal events using mutable this.currentRunId. A cleanup/overlap race can therefore produce a terminal event without the original ID; event construction uses a type assertion rather than runtime validation.

Relevant code:

  • packages/runtime/src/tools/system-tools.ts:createEditTraceReviewTool
  • packages/runtime/src/session-manager.ts:runDeliveryLoop
  • packages/runtime/src/trace.ts:review, auditFingerprint, auditNodeEvidence
  • packages/runtime/src/mas-agent.ts:runPrompt
  • packages/runtime/src/events.ts:envelope

Steps to Reproduce

  1. Create an unreviewed Trace node so the host queues an Auditor target.
  2. Use an Auditor model response that calls edit_trace_review more than once before ending the same response.
  3. For a node target, observe every call succeed and append another node_reviewed change.
  4. For a parent-edge target, observe the first call mutate the relation and later calls fail as stale; a model attempting to satisfy “call exactly once” may keep retrying.
  5. Interrupt the session during the loop and inspect terminal events and aggregate session state.

Expected Behavior

  • One target can be finalized at most once per bound Auditor run.
  • An identical duplicate is an idempotent no-op; a conflicting duplicate is rejected without encouraging retries.
  • The target is consumed immediately after the first accepted submission.
  • Every started run emits exactly one terminal event with the same non-empty run ID.
  • The configured token budget is a hard upper bound apart from a bounded single-request overshoot.
  • Consumers can settle from authoritative session_state.runState.active=false if an event stream is malformed or incomplete.

Actual Behavior

The target remains callable for the full model response. Node reviews can succeed indefinitely; parent-edge reviews can enter an indefinite failure/retry loop. Token usage is recorded but not enforced as a stop condition. A missing terminal run ID can leave an outer runner active after the runtime is idle.

Minimal Fix

  1. Add an atomic host-owned audit claim keyed by (sessionId, auditorRunId, nodeId, parentNodeId, evidenceFingerprint).
  2. Claim before mutation; after the first accepted call, mark the target consumed immediately.
  3. Change Trace.review() to an explicit state transition:
    • node: unreviewed -> approved|rejected|uncertain
    • edge: candidate -> confirmed|rejected|uncertain
    • identical final submission: return already_recorded
    • conflicting final submission: return already_final
  4. Return a non-error idempotent result for identical duplicates, e.g. review already recorded; end this turn, while enforcing a one-review-call limit for the bound turn.
  5. Capture const runId = newRunId() at run start; use it for all run events/stats and only clear instance state when it still equals that run ID. Validate RUN_* events before publishing.
  6. Enforce the session token budget before each provider request and abort/pause delivery when exhausted.
  7. Benchmark/CLI consumers should also settle when the authoritative session aggregate transitions to inactive, while logging missing/mismatched run IDs as protocol violations.

Do not rely only on adding the conclusion to the fingerprint: that converts the successful loop into a stale-error loop rather than making the operation idempotent.

Acceptance Tests

  • Two sequential identical node-review calls create exactly one change record.
  • Two parallel review calls create exactly one change record.
  • A conflicting second conclusion cannot overwrite the first.
  • Parent-edge duplicate submission does not enter stale-error retry behavior.
  • A stale evidence fingerprint is re-queued at most once.
  • Every RUN_STARTED has exactly one matching RUN_FINISHED or RUN_ERROR with the same run ID, including interrupt and overlapping-delivery tests.
  • An 800K token budget stops further provider requests and settles the session/runner.

Deployment Method

From source (npm run bp)

Version

Commit a2c09e4

Environment

  • Node.js v22.23.1
  • Linux 5.15 x86_64

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions