Skip to content

feat(chat): retry, compaction, sub-agents and permission rulesets for the chat turn - #359

Merged
Makisuo merged 3 commits into
mainfrom
feat/chat-agent-retry-compaction-subagents
Aug 6, 2026
Merged

feat(chat): retry, compaction, sub-agents and permission rulesets for the chat turn#359
Makisuo merged 3 commits into
mainfrom
feat/chat-agent-retry-compaction-subagents

Conversation

@Makisuo

@Makisuo Makisuo commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Ports four things opencode's agent runtime has that Maple's chat turn did not, adapted where Durable Objects force a different shape from a long-lived Node process.

lib/llm is untouched. It is a provider/protocol layer that deliberately owns no loop — LLM.generate is one provider turn, ToolRuntime.dispatch is documented as executing one tool call "without owning provider IO or continuation". Worth noting: the diff 32f278b4...dev is 70 commits upstream and the only change under packages/llm/ is a package.json version bump. There is nothing to gain by re-syncing, and the loop will always be ours.

Retry

A stream that died mid-body killed the turn. apps/api/src/chat/llm-retry.ts classifies what is worth another attempt; runStep retries with backoff, bounded by a whole-turn budget so it cannot outrun ChatSession's 15-minute TURN_STALE_MS watchdog.

The load-bearing detail, and the thing most worth a reviewer's attention: TransportReason.retryable and InvalidProviderOutputReason.retryable are both hardcoded false in the vendored package (lib/llm/src/schema/errors.ts:129, :143) — and those two are exactly how a mid-stream body failure surfaces, via route/client.ts's stream-level catch. A classifier that trusted LlmCallError.retryable would retry only what RequestExecutor already retries: it would look correct, pass a naive test, and do nothing. The classifier names them explicitly, and a test asserts Transport with retryable: false is retried.

Retries emit a new turn-retry event carrying both a retraction and the user-facing progress signal — either alone is useless. The retraction rests on an invariant of the loop: within one step, tool events are only emitted after the model stream completes, so a failed attempt emitted nothing but text and a character count is a complete undo.

Discovered while testing: Stream.groupedWithin discards its pending buffer on upstream failure rather than flushing it, so a provider that dies inside one 16ms batching window retracts zero. Recorded in a comment and pinned by a test that forces a real flush with 24 deltas.

MAX_STEPS no longer stops the turn dead on a wall of tool rows with no words — it spends one further tool-less step letting the model answer from what it found.

Compaction

toLlmMessages silently dropped the oldest 40 messages / 60k chars of a long investigation. It now replays a summary of the head instead.

Runs post-turn, not mid-turn as opencode does: mid-turn would interleave a second model call into a stream the browser renders keyed by messageId, add seconds to the visible answer, and need a second turn-start the client would have to learn. It runs inside the turn's own program before the runtime is disposed and before the slot is released, bounded by a 20s timeout and swallowed on failure — a failed compaction can never turn a delivered answer into a failed turn. The head-drop stays as the fallback and is tested byte-for-byte.

The compaction is an event, not a DO side table, because history() is the single source of truth and a side table would be invisible to a reconnecting client. It is inert for display: unlike opencode, where the transcript and the model input are the same list, a Maple user scrolling back must still see what they actually said.

Sub-agents

A task tool that hands a self-contained question to a read-only explore agent and gets back only its final text, never its tool payloads. That firewall is the whole point — delegation is how the model searches broadly without burying the thread in warehouse rows. A task that returned the child's transcript would be strictly worse than calling the tools inline.

Runs in process, not as a child Durable Object. A child DO would need a new addressing scheme surviving all four session-id parsers, a distributed abort fan-out that survives isolate eviction, and either cross-DO event mirroring or a second SSE stream per task — all to solve a CPU-headroom problem the budgets already bound. In process, abort propagates for free (the child gets the parent's isCurrent closure), there is one log and one stream, and toLlmMessages excludes child chatter for free because it lives nested inside a tool call.

Child events reach the log through an emit side channel rather than a Stream.merge, which would race the parent's terminal event against undrained child events. Depth is capped twice, as opencode does: structurally (every sub-agent's ruleset denies task, so the tool is never offered) and numerically.

Permission rulesets

MUTATING_TOOL_NAMES was a flat Set that could answer exactly one question. Rulesets add deny — the model never sees the tool, a stronger and cheaper guarantee than refusing the call — and per-agent scoping, which is what makes the read-only sub-agent possible.

MUTATING_TOOL_NAMES is unchanged and still the POST /api/chat/apply allowlist floor; it seeds DEFAULT_RULESET. The apps/slack-agent mirror needs no change. A test in mutating.test.ts asserts every registered tool resolves identically to the old Set, so day-one behaviour is locked and drift fails loudly.

Bugs fixed in passing

  • triage-agent.ts leaked internals to the model. It rendered a whole Effect Cause — stack frames and, inside a DatabaseError, connection details — into a ToolFailure message, which went into the model's context and into the durable transcript the browser reads back. This is the leak agent.ts's summarizeToolFailure was written to fix; the two had drifted. Both agents now share mcp/tools/llm-tools.ts.
  • A child's turn-end ended the parent's stream — in the web client's read loop and in the Durable Object's SSE pump. The first sub-agent to finish would have cut the connection, and the rest of the parent's answer would only have appeared on the next reconnect.
  • The closing step could loop forever if a provider emitted tool calls despite tools: [] and toolChoice: "none".

Reviewer notes

Two calls I'd like a second opinion on:

  1. @cf/moonshotai/kimi-k2.6's context window is a guess-avoidance, not a measurement. openai/gpt-5.6-luna is verified against OpenRouter's public catalogue (1,050,000 / 128,000). Cloudflare does not publish what its Workers AI deployment serves, so it sits at the conservative 128k default with a MAPLE_TRIAGE_MODEL_CONTEXT override. Upstream Moonshot is 262,144 if that turns out to be right.
  2. InvalidProviderOutput in the retry classifier is debatable. Genuinely malformed provider output will fail identically on every attempt and burn the budget. It is included because an interrupted body also lands on it (client.ts:220 finds no Fail reason and falls through). Narrowing to Transport alone is a one-line change if it proves noisy.

Also worth watching: sub-agents roughly triple the worst-case turn wall time, which brings the 15-minute TURN_STALE_MS watchdog within reach for the first time. If real turns approach it, lower TASK_BUDGET_PER_TURN — do not raise the watchdog.

Verification

  • 339 tests pass across apps/api, apps/web and packages/domain; all three typecheck.
  • bun run llm:sync --check confirms nothing Maple-specific leaked into the vendored package.
  • The sub-agent card renders, collapses and expands under jsdom.
  • Not verified in a live browser. The web bundle compiles and the app boots, but exercising the new paths end to end needs the full stack plus a live model that chooses to delegate — that was not stood up.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Makisuo added 2 commits August 6, 2026 14:22
… the chat turn

Ports four things opencode's agent runtime has that Maple's chat turn did not,
adapted where Durable Objects force a different shape than a long-lived process.
`lib/llm` is untouched — it is a provider/protocol layer and deliberately owns no
loop, and upstream has landed no source changes since the vendor pin.

**Retry.** A stream that died mid-body killed the turn. `llm-retry.ts` classifies
what is worth another attempt and `runStep` retries with backoff, bounded by a
whole-turn budget so it cannot outrun `TURN_STALE_MS`.

The load-bearing detail: `TransportReason.retryable` and
`InvalidProviderOutputReason.retryable` are both hardcoded `false` in the vendored
package, and those two are exactly how a mid-stream failure surfaces. A classifier
that trusted the flag would retry only what `RequestExecutor` already retries —
it would look right and do nothing. The classifier names them explicitly.

Retries emit a `turn-retry` event carrying both a retraction and the progress
signal, so text from an abandoned attempt is taken back rather than duplicated in
a durable log. Note `Stream.groupedWithin` discards its pending buffer on upstream
failure, so a provider that dies inside one batching window retracts zero.

**Compaction.** `toLlmMessages` silently dropped the oldest 40 messages / 60k
chars of a long investigation. It now replays a summary of the head instead.
Runs post-turn — mid-turn would interleave a second model call into a stream the
browser renders keyed by `messageId` — bounded by a timeout and swallowed on
failure, so it can never turn a delivered answer into a failed turn. The head-drop
stays as the fallback.

**Sub-agents.** A `task` tool that hands a self-contained question to a read-only
`explore` agent and returns only its final text, never its tool payloads. That
firewall is the point: it is how the model searches broadly without burying the
thread in warehouse rows.

Runs in process rather than as a child Durable Object. A child DO would need a new
addressing scheme surviving all four session-id parsers, a distributed abort
fan-out, and either cross-DO event mirroring or a second SSE stream per task — all
to solve a CPU-headroom problem the budgets already bound. In process, abort
propagates for free and there is one log and one stream.

**Permission rulesets.** `MUTATING_TOOL_NAMES` was a flat `Set` that could only
answer "does this need approval?". Rulesets add `deny` (the model never sees the
tool) and per-agent scoping. The set itself is unchanged and seeds
`DEFAULT_RULESET`; a test asserts every registered tool resolves identically, so
day-one behaviour is locked.

Fixed in passing: `triage-agent.ts` rendered a whole Effect `Cause` — stack frames
and, inside a `DatabaseError`, connection details — into the model's context and
into the durable transcript. Both agents now share `mcp/tools/llm-tools.ts`.

Also fixed, both found while testing sub-agents: a child's `turn-end` ended the
parent's SSE read loop, in the web client and in the Durable Object's pump; and a
provider emitting tool calls on the tool-less closing step would recurse forever.
`chat/` had become a flat pile: the control flow, its policies, the agent
registry, the permission rulesets, the prompt text and the Durable Object all
sat side by side, and reading "how does a turn progress?" meant knowing which of
nine files to ignore.

The loop and its policies move to `chat/loop/`, one question per file:

  turn.ts      the control flow — the only file that decides anything
  types.ts     the vocabulary: what goes in, what comes out, one step's state
  budgets.ts   every ceiling, together
  retry.ts     which failures earn another attempt
  context.ts   keeping the request inside the model's window
  delegate.ts  handing a sub-question to a sub-agent, which re-enters turn.ts
  index.ts     the barrel callers import, so the split stays free to move

What stays outside is the world the loop runs in, not part of how it progresses:
`ChatSession.ts` (log, ordering, turn slot), `turn-runner.ts` (bootstrap),
`agents.ts` and `permissions.ts` (config), `prompts.ts`, and a new `tools.ts`
holding `buildChatTools` and `submit_diagnosis` — the loop decides *when* to call
a tool, not which tools exist.

The one non-mechanical change is `budgets.ts`. Those constants were spread across
three modules, where each looked independently reasonable and their product did
not: MAX_STEPS (10) x TASK_BUDGET_PER_TURN (4) x SUBAGENT_MAX_STEPS (6) is 240
model calls, which has to stay under TURN_STALE_MS or the DO declares a running
turn abandoned. Together, that relationship is visible and TURN_STEP_BUDGET reads
as the backstop it is.

`tagged`/`turnEnd` now take the task ref structurally rather than the whole turn
input, which is what breaks the types <-> events import cycle the split would
otherwise have created.

Also fixes a dead comparison in delegate.test.ts that only `typecheck:test`
caught: `ChatTurnEvent` excludes `user-message`, so that branch was unreachable
and the assertion was weaker than it read.
@Makisuo

Makisuo commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed a follow-up commit reorganising apps/api/src/chat/: the turn loop and its policies now live in chat/loop/, one question per file, separate from the world the loop runs in (the Durable Object, the agent registry, the rulesets, the prompts, the tool set).

chat/
  ChatSession.ts    the log, ordering, the turn slot
  turn-runner.ts    DO -> loop bootstrap, replay projection, post-turn compaction
  agents.ts         which agent a turn runs as
  permissions.ts    rulesets
  prompts.ts        prompt text
  tools.ts          what the loop may call
  loop/
    turn.ts         the control flow -- the only file that decides anything
    types.ts        the vocabulary: input, events, one step's state
    budgets.ts      every ceiling, together
    retry.ts        which failures earn another attempt
    context.ts      keeping the request inside the model's window
    delegate.ts     handing a sub-question to a sub-agent, which re-enters turn.ts
    index.ts        the barrel callers import

Mostly mechanical — git tracked the moves as renames, so the diff is readable. Two things that aren't:

budgets.ts is the part worth a look. Those constants were spread across three modules, where each looked independently reasonable and their product did not: MAX_STEPS (10) x TASK_BUDGET_PER_TURN (4) x SUBAGENT_MAX_STEPS (6) = 240 model calls, which has to stay under TURN_STALE_MS or the Durable Object declares a still-running turn abandoned. Together, that relationship is visible and TURN_STEP_BUDGET reads as the backstop it is rather than an arbitrary 30.

A dead assertion in delegate.test.ts, which only typecheck:test caught. ChatTurnEvent excludes user-message, so event.type === "user-message" ? undefined : ... was an unreachable branch and the assertion was weaker than it read. Now narrows on compaction instead, which is the member that genuinely has no task ref.

No behaviour change: 268 apps/api tests still pass, and typecheck is clean across all three packages.

The closest thing to provider-contract verification available without a live
call: every other test in the loop stubs the model, so nothing would notice if
`Schema.Literals` over a runtime-built agent-name list produced something a
provider rejects.

Also records a wart rather than fixing it. Effect renders the literal union as a
single-member `anyOf` wrapping the enum instead of a bare enum — harmless on
OpenRouter and Workers AI, but exactly the shape opencode's `tool/json-schema.ts`
normalizes away because some providers reject a degenerate `anyOf`.
@Makisuo
Makisuo merged commit 29fe783 into main Aug 6, 2026
17 checks passed
@Makisuo
Makisuo deleted the feat/chat-agent-retry-compaction-subagents branch August 6, 2026 12:56
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🍁 Maple PR preview

Note

Preview resources were removed when this pull request closed.

Final commit a6b5ea1 · View workflow run

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant