Skip to content

fix(agents): recognize streamed tool arguments as watchdog progress - #1566

Merged
Alan-TheGentleman merged 1 commit into
mainfrom
fix/1562-tool-argument-watchdog
Sep 29, 2026
Merged

Alan-TheGentleman merged 1 commit into
mainfrom
fix/1562-tool-argument-watchdog

Conversation

@Alan-TheGentleman

@Alan-TheGentleman Alan-TheGentleman commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Linked Issue

Refs #1562. The issue has status:approved; this PR does not claim the historical test-tokens incidents are conclusively reproduced and does not close it automatically.

PR Type

  • Bug fix

Summary

  • Recognize validated nonempty tool-argument deltas as idle-watchdog progress independently of display events.
  • Show a privacy-safe generation step without storing arguments or incrementing execution/usage counters early.
  • Preserve renewable idle/execution budgets and cancellation; no absolute run ceiling is added, as explicitly selected by the maintainer.

Changes

File Change
lib/agents-protocol.ts Active-generation validation and bounded hash-only duplicate rejection.
lib/agents-runner.ts Independent argument liveness and existing status projection.
tests/agents-protocol.test.ts Current/legacy shapes, malformed updates, duplicates and bounds.
tests/agents-runner.test.ts Streaming, silence, noise, execution, cancellation and finalized usage.
docs/gentle-agents-activity.md Renewable bounds and replay limitations.

Test Plan

  • RED: two intended regressions failed before implementation; 83 passed.
  • GREEN and independent rerun: node --experimental-strip-types --test tests/agents-protocol.test.ts tests/agents-runner.test.ts — 87 passed.
  • pnpm typecheck — passed ratchet, 187 existing diagnostics, no regressions.
  • pnpm run check:runtime-modules — passed, eight generated modules match.
  • git diff --check — passed.
  • Initial pnpm test exceeded 1200 seconds without final summary and printed two prompt-route failures. Their test/handler blobs are identical at base; inherited subagent gating suppresses the primary prompt. No base execution is claimed. Offline-isolated complete rerun passed in 62.3 seconds: 4,040 passed, 41 skipped, zero failures; unit tests, provider contract and runtime harness all passed. Command: env -u GENTLE_PI_AGENTS_CHILD -u GENTLE_PI_AGENTS_PARENT_PERMISSION_FD DO_NOT_TRACK=1 GENTLE_AI_TELEMETRY=0 pnpm test. Isolation affected the test subprocess only; tracked candidate bytes remained unchanged. The earlier timeout cause remains unproven.
  • Native review review-871f8bc5392af676 approved and acknowledged. One informational/nonblocking advisory (R3-closed-unannounced-block); no correction required.
  • node scripts/verify-package-files.mjs — passed: 155 files, 69 byte-pinned artifacts.
  • No live-provider reproduction. Shellcheck and changed-skill loading are inapplicable (no scripts or skills changed).

Limitations

Sequence-less RPC cannot distinguish identical chunks from replay; duplicate hashes conservatively do not renew liveness. Tracking is capped at 4096 fingerprints per assistant message. Continuous valid progress may extend a run indefinitely; budgets constrain silence, not total duration.

Contributor Checklist

  • Approved issue linked.
  • Exactly one type:* label: type:bug.
  • Focused regression checks and independent review performed.
  • Behavior documentation updated.
  • Conventional commit; no Co-Authored-By trailers.
  • Complete local suite passed with test-subprocess isolation.
  • Repository CI green on 6e4f0b5716144fa62559b09117fed5b093e1b840: CI verify/macOS/Windows and Windows Hidden Internal Processes Ubuntu/Windows all passed. Packed-installation checks passed in CI. Optional CodeRabbit was not awaited.

243 authored changed lines. Maintainer authorized merge after mandatory checks without waiting for optional CodeRabbit review.

Summary by CodeRabbit

  • Bug Fixes

    • Long-running tool-argument generation can continue without triggering an idle timeout while valid argument updates are arriving. Empty, repeated, or invalid updates do not extend the timeout.
    • Partial tool arguments are not displayed or counted as tool executions. Usage totals continue to reflect finalized message data.
  • Documentation

    • Added guidance on progress tracking, timeout behavior, and the handling of tool-argument updates.

@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 61e56968-1a89-4d1a-8320-4a18edb1e7c7

📥 Commits

Reviewing files that changed from the base of the PR and between f122642 and 6e4f0b5.

📒 Files selected for processing (5)
  • docs/gentle-agents-activity.md
  • lib/agents-protocol.ts
  • lib/agents-runner.ts
  • tests/agents-protocol.test.ts
  • tests/agents-runner.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Valid tool-argument deltas during an active assistant message now update task activity and renew the idle watchdog. The runner does not expose partial arguments or count them as tool executions or finalized usage.

Changes

Tool-argument progress

Layer / File(s) Summary
Track valid tool-argument progress
lib/agents-protocol.ts, tests/agents-protocol.test.ts
ToolArgumentProgress accepts nonempty deltas for active, announced tool-call blocks. It rejects duplicate fingerprints and stops accepting new fingerprints at 4096. Tests cover current and legacy start shapes, block closure, stale generations, and invalid inputs.
Apply progress to task activity
lib/agents-runner.ts, tests/agents-runner.test.ts, docs/gentle-agents-activity.md
The runner updates task activity and renews the idle watchdog when the tracker reports progress. Tests cover timeout renewal, rejected events, message completion, tool execution, and cancellation. The documentation describes the tracking and watchdog behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AgentRunner
  participant ToolArgumentProgress
  participant StallWatchdog
  AgentRunner->>ToolArgumentProgress: Observe RPC event
  ToolArgumentProgress-->>AgentRunner: Report accepted argument progress
  AgentRunner->>StallWatchdog: Re-arm idle timeout
Loading

Suggested reviewers: decode2

Merge Risk: ⚪ Minimal · up to 6e4f0

Validated tool-argument progress now renews the idle watchdog without exposing partial arguments or counting execution early. No concrete merge-blocking issue is established; merge readiness remains subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6e4f0

The change lets active argument generation continue longer without granting additional tool authority or recording partial arguments. Cancellation and process cleanup remain independent of progress. Broader deployment exposure was not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly affected exposure is the originating task’s runtime and occupied runner capacity. Multiple children sustaining progress could prolong aggregate capacity use, but this path adds no tool privileges, cross-task mutation, or new credential access. Tenant and deployment scope were not established.

Security Findings and Attack Paths

  • inferred — A child controlling its RPC output can sustain liveness with accepted distinct argument deltas. This is an intentional expansion of renewable progress, not evidence of genuine work or authenticated freshness. The base already renewed the watchdog on RPC responses and normalized events, and the changed path adds no execution authority; no introduced material bypass was established within the examined scope.

Trust Boundaries and Controls

  • observed — Untrusted child output crosses into parent-owned liveness state through validation, not authorization. New assistant-message tracking requires an increasing safe timestamp while inactive; only announced open blocks accept deltas. Lifecycle-end messages clear active tracking, while the runner’s live-task and terminal guards isolate stopped tasks.

Resilience and Maintainability Implications

  • observed — RPC deltas lack sequence numbers, so identical chunks within a block are conservatively rejected as indistinguishable from replay. Fingerprint exhaustion also stops argument renewal until a newer generation. These fail-closed decisions can reduce liveness recognition, while later silence still times out and cancellation remains available.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: recognizing streamed tool arguments as watchdog progress.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Alan-TheGentleman
Alan-TheGentleman merged commit a3bbd07 into main Sep 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant