# Agent loop follow-up issues

## Issue 1: Fix dangling control tool calls across movement transitions

### Problem

`transition` and `complete` tool calls can be appended to the live conversation
without a matching tool result message. When the same `Conversation` is reused by
the next movement, strict OpenAI-compatible providers may reject the next request
because every assistant `tool_call` must have a corresponding `tool` message.

### Evidence

- `src/engine/agent-loop.ts`: assistant messages are recorded with all pending
  tool calls, including control calls.
- `src/engine/agent-loop/terminal-control.ts`: valid `transition` returns a
  `MovementResult` without a tool result message.
- `src/engine/agent-loop.ts`: valid `complete` returns immediately without a
  tool result message.
- `src/engine/context/conversation.ts`: `replayableTurns()` sanitizes persisted
  transcript replay, but live `conversation.messages` shared across movements is
  not sanitized.

### Expected behavior

After a movement exits via `transition` or `complete`, the next model request
must not contain unresolved control `tool_call`s.

### Suggested fix

Normalize live conversation state before entering the next movement, or avoid
recording terminal/control tool calls without corresponding tool messages.

### Acceptance criteria

- Add a regression test that uses a shared `Conversation` across a real
  `transition`.
- The second movement's model input contains no dangling `transition` or
  `complete` tool calls.
- Existing agent-loop tests continue to pass.

---

## Issue 2: Make context overflow terminal defaults consistently abort

### Problem

Context overflow handling has two different policies:

- `buildContextOverflowResult()` converts terminal defaults (`COMPLETE`, `ASK`)
  to `ABORT`.
- `applyContextManagerUpdate()` with `force_transition` uses
  `movement.defaultNext ?? 'ABORT'` directly.

This means context pressure can falsely complete or ask from a movement whose
default next step is terminal, even though the safer policy is to abort on
context loss.

### Evidence

- `src/engine/agent-loop/context-control.ts`: `buildContextOverflowResult()`
  normalizes `COMPLETE` and `ASK` to `ABORT`.
- `src/engine/agent-loop/context-control.ts`: `applyContextManagerUpdate()`
  force transition uses `movement.defaultNext` directly.

### Expected behavior

All context-overflow forced exits should use the same terminal-default policy.
When context is compromised, terminal defaults should not be treated as
successful completion.

### Suggested fix

Share one helper for context-overflow movement results, or apply the same
terminal-default normalization in `applyContextManagerUpdate()`.

### Acceptance criteria

- Add a regression test for `force_transition` with `defaultNext: "COMPLETE"`.
- The result is an abort-style movement result, not a successful completion.
- Existing context overflow tests continue to pass.

---

## Issue 3: Enforce `why_no_default` for `needs_user_input`

### Problem

The prompt and tool description require `why_no_default` when calling
`complete({ status: "needs_user_input" })`, but runtime validation only requires
`missing_info`.

This lets the agent ask the user without documenting why it could not choose a
reasonable default, which weakens the intended "avoid unnecessary user
questions" behavior.

### Evidence

- `src/engine/agent-loop/prompt.ts`: instructs the model to provide
  `why_no_default`.
- `src/engine/agent-loop/terminal-control.ts`: tool description says
  `why_no_default` is required.
- `src/engine/agent-loop/terminal-control.ts`: `validateCompleteArgs()` only
  checks `missing_info`.
- `src/engine/agent-loop/terminal-control.ts`: movement output ignores
  `why_no_default`.

### Expected behavior

`needs_user_input` should be rejected unless both `missing_info` and
`why_no_default` are non-empty strings.

### Suggested fix

Validate `why_no_default` in `validateCompleteArgs()` and include it in the
result/debug output if useful.

### Acceptance criteria

- Add a negative test for missing `why_no_default`.
- Add a positive test for `needs_user_input` with both fields.
- Existing `complete` behavior remains unchanged for other statuses.

---

## Issue 4: Validate interactive browse waiting-human session id

### Problem

`parseInteractiveBrowseWaitingHuman()` casts `sessionId` to `string` without
checking that it is present and actually a string.

If the tool returns malformed `waiting_human` output, the movement result can
carry `browserSessionId: undefined` despite the type expecting a string.

### Evidence

- `src/engine/agent-loop/tool-dispatcher.ts`: `sessionId` is read via
  `parsed["sessionId"] as string`.
- The function validates `action` and `waitReason`, but not `sessionId`.

### Expected behavior

Malformed waiting-human output should not produce a typed waiting-human movement
result with an undefined session id.

### Suggested fix

Require `typeof parsed["sessionId"] === "string"` and a non-empty value before
returning a waiting-human result.

### Acceptance criteria

- Add a test for malformed `waiting_human` output without `sessionId`.
- Add a test for valid `waiting_human` output.
- Existing interactive browse behavior remains unchanged for valid tool output.

---

## Issue 5: Split `tool-dispatcher.ts` before it becomes the next agent-loop

### Problem

`tool-dispatcher.ts` now owns several distinct responsibilities:

- cache routing
- cache hit/miss event logging
- tool execution
- memory checkpoint behavior
- waiting-human parsing
- batched result recording

This file is becoming the next high-coupling module after the agent-loop split.

### Expected behavior

Tool dispatch should be decomposed into focused modules so bugs in cache
routing, execution, and special tool outputs are easier to test independently.

### Suggested fix

Split along behavioral boundaries, for example:

- `tool-cache-routing.ts`
- `tool-execution.ts`
- `tool-result-recorder.ts`
- `interactive-browse-result.ts`

### Acceptance criteria

- No behavior change.
- Existing agent-loop and tool-loop tests pass.
- New modules expose narrow, testable functions.
