fix(agent-runtime): three correctness bugs in adapter and legacy-bridge

[dylanneve1]

Summary

Deep code review of the main...HEAD diff surfaced three correctness bugs in the new agent-runtime infrastructure (Phase 1–2). All are in code that has no production callers yet, which means they would have silently shipped as-is into Phase 3+ without being caught by tests. All 2990 tests pass before and after this fix.

---

Bug 1 — adapter.ts: sessions capability falsely advertised when only warmSession is defined

File: src/core/agent-runtime/adapter.ts

Root cause:

// BEFORE (buggy)
const sessions: SessionBackend | undefined =
  legacy.resetChat || legacy.warmSession  // ← OR: too broad
    ? {
        resetChat(chatId) {
          return legacy.resetChat?.(chatId);  // ← silent no-op when resetChat is undefined
        },
        ...
      }
    : undefined;

If a legacy backend defined only warmSession (no resetChat), the adapter would:

1. Set capabilities.sessions = true
2. Provide a sessions.resetChat() method that calls legacy.resetChat?.() — which returns undefined and does nothing

Callers checking backend.capabilities.sessions would believe the backend supports session reset, call sessions.resetChat(chatId), and silently get a no-op. No error, no log, no reset.

Fix: Guard session object creation on legacy.resetChat being defined. warmSession is still forwarded when both are present.

// AFTER (fixed)
const sessions: SessionBackend | undefined = legacy.resetChat  // ← AND: precise
  ? {
      resetChat(chatId) {
        return legacy.resetChat!(chatId);  // ← non-null assertion safe; guarded above
      },
      warmSession: legacy.warmSession
        ? (chatId) => legacy.warmSession!(chatId)
        : undefined,
    }
  : undefined;

---

Bug 2 — legacy-bridge.ts: non-plain-object tool inputs silently discarded

File: src/core/agent-runtime/legacy-bridge.ts

Root cause:

The tool_call event in events.ts types input as unknown — valid values include arrays and other non-plain objects. The legacy onToolUse callback requires Record<string, unknown>, so the bridge must convert. The old code silently replaced any non-plain-object input with {}:

// BEFORE (buggy)
const input = isPlainObject(event.input)
  ? (event.input as Record<string, unknown>)
  : {};  // ← array inputs become {}, data gone, no warning
callbacks.onToolUse?.(event.name, input);

A Phase 3 backend emitting { type: "tool_call", name: "write_file", input: ["a", "b"] } would silently have ["a", "b"] replaced with {} before reaching onToolUse. The tool would receive empty arguments.

Fix: Add a console.warn when data loss occurs, so operators can identify backends producing array-shaped tool inputs before the bridge strips them.

// AFTER (fixed)
if (!isPlainObject(event.input) && event.input !== undefined && event.input !== null) {
  const typeLabel = Array.isArray(event.input) ? "array" : typeof event.input;
  console.warn(
    `[legacy-bridge] tool_call "${event.name}": input is ${typeLabel}, ` +
      `not a plain object — bridging to {} ...`,
  );
}
const input = isPlainObject(event.input)
  ? (event.input as Record<string, unknown>)
  : {};
callbacks.onToolUse?.(event.name, input);

---

Bug 3 — legacy-bridge.ts: dead-code saw variable in reduceEventsToResult

File: src/core/agent-runtime/legacy-bridge.ts

Root cause:

// BEFORE (buggy)
let saw = false;
...
case "completed":
  saw = true;
  if (event.result) { return event.result; }
  break;
...
return {
  text, durationMs, usage,
  ...(modelId ? { modelId } : {}),
  // saw === false means we hit the silent-stream path
  ...(saw ? {} : {}),   // ← BOTH branches spread {}.  saw has zero effect.
};

...(saw ? {} : {}) always spreads an empty object. The saw flag was computed correctly (set true when a completed event with a falsy result was seen) but had no effect on the returned value. The comment claimed it distinguished a "silent-stream path" — but both paths produced identical output. This is dead code that would mislead any future engineer extending reduceEventsToResult.

Fix: Remove the saw variable and its meaningless spread; clarify the comment.

// AFTER (fixed)
// (no saw variable)
return {
  text, durationMs, usage,
  ...(modelId ? { modelId } : {}),
  // Stream ended without a completed terminator, or completed carried no
  // result — synthesise from observed deltas.
};

---

Test results

Test Files  130 passed | 7 skipped (137)
     Tests  2990 passed | 41 skipped (3031)

https://claude.ai/code/session_01Ddo8qvtcTtq5NVU4SkwCCP

---

Generated by Claude Code

[claudiusthebot]

All three bugs are real and the fixes are correct. 34/34 CI ✅.

Bug 1 (adapter.ts sessions capability): The || → AND guard is the right fix. If only warmSession is defined, advertising sessions capability and providing a no-op resetChat() is a silent contract breach. The fix is minimal and precise.

Bug 2 (legacy-bridge.ts non-plain-object inputs): Adding console.warn instead of silently discarding is the right call. No data loss path changes, but operators now get visibility when a Phase 3 backend emits array-shaped tool inputs before the bridge strips them. The "bridge to {}" behaviour is preserved intentionally — a hard throw would break existing callers.

Bug 3 (legacy-bridge.ts dead saw variable): Clean removal. The ...(saw ? {} : {}) spread was truly a no-op in both branches — not even a future footgun, just misleading dead code. The updated comment is clearer.

One observation for when Dylan reads this: Bug 1 could silently surface in Phase 3+ if any new backend (e.g. the antigravity or agy wrappers) implements warmSession but not resetChat. Worth a grep before the Phase 3 backend refactor lands.