Skip to content

feat(config): configurable agent-loop retry limits with pause-and-ask (#897) - #904

Open
shoryabansalgithub wants to merge 19 commits into
Nano-Collective:mainfrom
shoryabansalgithub:feat/configurable-retry-limits
Open

feat(config): configurable agent-loop retry limits with pause-and-ask (#897)#904
shoryabansalgithub wants to merge 19 commits into
Nano-Collective:mainfrom
shoryabansalgithub:feat/configurable-retry-limits

Conversation

@shoryabansalgithub

Copy link
Copy Markdown

Description

Implements #897: configurable retry limits so a stuck agent cannot silently drain API tokens by retrying a failing operation forever.

Adds a new nanocoder.retries section to agents.config.json exposing three previously hardcoded agent-loop caps:

  • maxRepeatedToolCalls (default 3, minimum 2) - consecutive identical tool calls before intervention
  • maxEmptyTurns (default 2, minimum 0) - consecutive empty assistant turns before erroring
  • maxMalformedRetries (default 2, minimum 0) - malformed tool-call self-correction attempts before erroring

When the repeated-tool-call limit is hit in an interactive session, the agent pauses and asks the user whether to continue or stop instead of hard-stopping. Choosing continue grants another window and re-prompts when it is spent, reporting the cumulative repeat total. Any non-exact answer (including Esc) fails safe to stop. Non-interactive and headless runs keep a hard stop, and the same caps are enforced in the --plain runtime loop so unattended CI runs are protected too.

The issue proposed the name maxRetries, but that is already a public per-provider setting meaning network request retries (default 2). Reusing it would silently change meaning for existing configs, so the agent-loop caps live under a distinct nanocoder.retries section instead. Defaults preserve current behavior. MAX_TOOL_STEPS and MAX_COMPACT_RETRIES are deliberately left hardcoded (per-request step cap and a sub-branch of the empty-turn path, not user-facing retry drains).

Supersedes #902, reopened from a dedicated feature branch per maintainer request.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully)
  • Tests cover both success and error scenarios

New ava tests cover: config loader defaults, custom values, partial config, clamping, and invalid types; stop at the default limit; continue grants a new window then re-prompts; one-under-limit boundary; custom limits; non-interactive never prompts; custom malformed and empty-turn limits; plain-runtime cap enforcement.

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

End-to-end proof was captured against a local mock OpenAI-compatible server deliberately repeating the same failing tool call: interactive pause-and-ask flow (pause, continue, resume, cumulative re-prompt, stop), Esc fail-safe, and the non-interactive hard stop, with measured API request counts confirming the limits (default stops at exactly 3 calls, limit 5 at exactly 5, limit 1 clamps to the minimum 2). The built CLI was smoke-tested including real agents.config.json resolution. No live paid provider was used.

Checklist

  • If this was for an open issue, I was assigned to it ([Feature] Configurable Retry Limits to Prevent Token Drain #897 is unassigned; happy to be assigned)
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (docs/configuration/index.md and related pages)
  • No breaking changes (defaults preserve existing behavior)
  • Appropriate logging added using structured logging (no new log points required; existing error paths unchanged)

… loops

Adds a nanocoder.retries section to agents.config.json exposing the
previously hardcoded conversation-loop caps: maxRepeatedToolCalls
(default 3), maxEmptyTurns (default 2), maxMalformedRetries (default 2).
Defaults preserve existing behaviour exactly.

When the repeated-tool-call limit is hit in an interactive session the
loop now pauses and asks the user whether to continue (granting another
window of attempts) or stop, instead of always hard-stopping. Headless
and non-interactive runs keep the hard stop since there is nobody to ask.

Named 'retries' rather than the issue's proposed 'maxRetries' because
maxRetries is already a public per-provider setting that caps network
request retries.

Closes Nano-Collective#897
Review follow-up: keep currentRepeatedCount const (it names the detected
streak used in user-facing messages) and carry the post-prompt streak in
repeatedCountForNextTurn so the recursion site shows where a reset can
come from.
The plain runtime (auto-selected for 'nanocoder run' in CI and non-TTY
environments) previously had no repeated-tool-call, empty-turn, or
malformed-retry protection at all - it was bounded only by
headless.maxTurns (default 200), which is exactly the unattended
token-drain scenario issue Nano-Collective#897 targets.

The loop now honors the same nanocoder.retries limits as the interactive
runtime: consecutive identical tool calls, consecutive empty turns
(nudged up to the cap), and malformed tool-call self-correction retries
each hard-stop with a clear error naming the setting once their cap is
hit - there is no user to ask in a plain run. Empty assistant messages
are no longer appended to history (providers reject them), matching the
interactive loop. Docs and the changeset now describe both runtimes.
feat(chat-handler): configurable agent-loop retry limits with pause-and-ask on tool loops
feat(config): configurable agent-loop retry limits with pause-and-ask
…nd cap subagent loops

Apply the four captain-decided fixes from review round 2 of Nano-Collective#897:

- Unknown-tool turns now count toward the maxRepeatedToolCalls streak in
  both runtimes, so a model stuck calling a nonexistent tool trips the cap
  (interactive: pause-and-ask; plain: hard stop) instead of looping until
  the turn ceiling. The streak/prompt logic is shared between the valid and
  unknown-tool branches.
- The subagent executor loop, previously uncapped, now applies
  maxRepeatedToolCalls and stops with an error naming the setting.
- Docs: loud CI warning that legitimate polling patterns in --plain runs
  hard-stop at the cap and need maxRepeatedToolCalls raised; fixed the
  off-by-one in the retry-limits table (a limit of 3 executes the repeated
  call twice and pauses on the third emission) and reworded the continue
  option to 'check again after N more' to match the implementation.
- Tests for the unknown-tool path in both loops and for all subagent cap
  paths (trip, one-under, unknown-tool, custom limit).

Also fixes a misplaced biome-ignore in chat-panel-harness.ts that left two
standing lint warnings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds configurable agent-loop retry limits (nanocoder.retries) so the interactive and --plain conversation loops can’t silently spin forever (and drain tokens) when a model repeats tool calls, emits empty turns, or produces malformed tool-call text.

Changes:

  • Introduces RetryLimitsConfig + config loader support (getRetryLimits) with defaults matching the prior hardcoded caps.
  • Updates both the interactive loop and the --plain runtime to enforce the configured caps (including pause-and-ask behavior for repeated tool calls in interactive sessions).
  • Adds/updates AVA tests and docs describing the new configuration and behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
source/types/config.ts Adds RetryLimitsConfig and AppConfig.retries typing.
source/config/index.ts Loads nanocoder.retries with clamping/defaults; exports getRetryLimits().
source/constants.ts Documents constants as defaults for the new config surface.
source/hooks/chat-handler/conversation/conversation-loop.tsx Uses configurable limits; adds interactive pause-and-ask flow for repeated tool calls.
source/plain/conversation.ts Enforces the same limits in the --plain loop (repeats/empty/malformed).
source/config/index.spec.ts Adds unit coverage for retry-limits config loading, defaults, clamping, invalid types.
source/hooks/chat-handler/conversation/conversation-loop.spec.ts Adds/extends tests for the interactive pause-and-ask repeated-tool-call behavior and limit overrides.
source/plain/conversation.spec.ts Updates tests to cover empty-turn nudging and retry-limit behavior in --plain.
docs/configuration/index.md Documents the new nanocoder.retries section and semantics.
docs/configuration/providers/index.md Clarifies provider maxRetries vs agent-loop retry limits.
docs/features/development-modes.md Notes repeated-tool-call safeguard remains even in yolo mode.
docs/features/commands.md Documents hard-stop behavior for retry limits in non-interactive run/--plain.
CLAUDE.md Updates tool-call parsing description to mention malformed retry limit.
.changeset/configurable-retry-limits.md Adds changeset entry for the feature.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +346 to +363
// Skip appending a fully-empty assistant message (no content, no tool
// calls): providers reject them, and the empty-turn nudge below re-asks
// without one — same rule the interactive loop applies.
const hasAssistantPayload =
cleanedContent.trim() ||
validToolCalls.length > 0 ||
errorResults.length > 0;
if (hasAssistantPayload) {
messages = [
...messages,
{
role: 'assistant',
content: cleanedContent,
tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined,
reasoning: streamedReasoning || undefined,
},
];
}
chat-panel.js reads globalThis.NanocoderMentionUtils at boot since the
@-mention autocomplete landed, and the real webview loads
mention-utils.js first (see chat-panel.html). The VM harness only
evaluated chat-panel.js, so every chat-panel spec failed with an
undefined-global TypeError.
…the non-interactive approval exit

The non-interactive approval exit saved history where the assistant
message announced confirmTools' tool_calls but none of them received a
tool result. A later session resume replays that history, and strict
OpenAI-compatible providers reject announced tool_calls without
results. Pair each unconfirmed tool with a cancellation result, the
same pattern the interactive decline path already uses.
@shoryabansalgithub

Copy link
Copy Markdown
Author

Pushed a hardening round on top of the initial implementation: repeated invalid (unknown) tool calls now count toward the retry cap in all runtimes, assistant/tool message pairing is preserved on every error and abort path (prevents provider API errors on strict OpenAI-compatible backends), subagent loops get the same caps with partial output preserved, and docs/changeset wording was tightened. Full suite: 6,534 tests passing.

@akramcodez akramcodez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @shoryabansalgithub, I went through the latest changes and did a deep review of the PR. Everything looks really solid now.

The implementation is well scoped, the retry-limit behavior is properly covered, the tool-call/result pairing issues are handled, and the test coverage is comprehensive. Typecheck, lint, and the relevant test suite are all passing as well.

I don't see any blocking issues from my side. The remaining points around ACP scope and the --plain behavior are already documented and look like reasonable design decisions.

Great work on this one.

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.

3 participants