Skip to content

fix(agent): preserve successful non-replayable tool results - #12509

Open
jyaunches wants to merge 2 commits into
mainfrom
fix/agent-replay-result
Open

jyaunches wants to merge 2 commits into
mainfrom
fix/agent-replay-result

Conversation

@jyaunches

@jyaunches jyaunches commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Outcome

Successful OpenClaw tool turns retain their native exit code when replayInvalid=true. Previously, NemoClaw changed a successful exit to 1 and reported an incomplete turn.

Reason

OpenClaw can mark a completed turn unsafe to replay after tool side effects. Replay safety alone does not establish failure. This surfaced during Spark testing for #12502; the classifier also exists on main.

Changes

  • Require an abandoned, timeout, or incomplete-turn marker before adding replay safety to failure diagnostics.
  • Cover completed gateway and local response envelopes, output preservation, and native exit codes 0 and 7.
  • Replace the regression test that incorrectly treated replay safety alone as failure. Existing incomplete-turn and timeout tests remain passing.

Verification

  • Follow-up repair: all 117 focused classifier, passthrough, and E2E support tests pass. Removed the obsolete E2E helper workaround; timeout and incomplete-turn rejection remain covered.

  • Focused Vitest run for agent-json-provenance.test.ts and passthrough-json.test.ts: three regression cases failed before the fix; all 65 tests passed afterward.

  • npm run build:cli: passed on macOS and Sparky.

  • Normal commit and publication hooks: passed, including secret scanning. The diff contains no secrets, API keys, or credentials.

  • Manual Sparky retest at 462f7fda94579dc0fa5cba8b43c3975993867422: completed write, read, and exec; returned status=ok, replayInvalid=true, and CLI exit 0. Independently verified the created file and four-word count.

  • The live receipt identified local nvidia/Qwen3.6-35B-A3B-NVFP4, with rerouted=false. On the identical fresh response, the old classifier returned replayInvalid=true as a failure marker; the fixed classifier returned no incomplete-turn signal.

  • Searched production src consumers of replayInvalid; this classifier was the only occurrence.

Review notes

Local Advisor attempted on the repaired tree using the trusted base 41b9d9fa90b04281af8b98238aea4e7fc745aa8d in the Lima Docker controller. It failed before any specialist review because the sandbox image pull exhausted the VM disk. Task-owned resources were removed. Full-diff self-review and focused tests completed under the authorized alternative path; hosted checks and selected E2E remain pending. This is not independent Advisor clearance.

The Undici npm-audit failure is inherited: reproduced against current main lockfiles in Linux with Node 22.23.2 and npm 12.0.2. Existing dependency repair #12507 requires trusted-base prerequisite #12517. No dependency changes are mixed into this classifier PR.

The manual retest used the existing pr12502-64gb sandbox and local inference endpoint on Sparky, a 128 GB physical host running a constrained-memory test configuration. This verifies result classification, not physical 64 GB hardware qualification. An initial attempt against the retired port 12503 failed before any tool call; the successful run used active port 12513.


Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Completed agent turns now retain their JSON output and original exit code even when replay metadata is marked invalid.
    • Completed gateway and local turns are no longer treated as incomplete solely because of that metadata.
    • Completed turns with successful replies are accepted even without optional tool-summary information, while turns marked incomplete due to a provider timeout remain rejected.

OpenClaw marks completed tool turns replayInvalid when side effects make replay unsafe.
Treating that flag as failure changed native exit zero to one on Spark.

Require an incomplete, abandoned, or timeout marker before including replay safety in diagnostics.
Regression tests failed before the fix. All 65 focused tests now pass, including exit preservation.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches jyaunches self-assigned this Sep 30, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 30, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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: NVIDIA/NemoClaw/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 46588073-3960-4906-8e56-bd7a0e578e07

📥 Commits

Reviewing files that changed from the base of the PR and between 462f7fd and 36b8c5e.

📒 Files selected for processing (2)
  • test/e2e/fixtures/openclaw-agent-output.ts
  • test/e2e/support/openclaw-agent-output.test.ts

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


📝 Walkthrough

Walkthrough

The change updates incomplete-turn detection so replayInvalid=true does not signal an incomplete turn by itself. Tests cover completed turns, reply extraction, and passthrough behavior for upstream exit codes 0 and 7.

Changes

Turn handling

Layer / File(s) Summary
Incomplete-turn marker logic
src/lib/openclaw/agent-json-provenance.ts, src/lib/openclaw/agent-json-provenance.test.ts, src/lib/actions/sandbox/agent/passthrough-json.test.ts
turnMetaMarkers adds replayInvalid=true only when another marker exists. Tests cover completed gateway and local turns, and passthrough behavior for exit codes 0 and 7.
Incomplete-turn text extraction
test/e2e/fixtures/openclaw-agent-output.ts, test/e2e/support/openclaw-agent-output.test.ts
openClawAgentTextParts rejects inputs with an incomplete-turn signal. Tests cover accepted non-replayable replies and rejected incomplete turns.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: cv

Merge Risk: ⚪ Minimal · up to 36b8c

Completed non-replayable turns retain their output and native exit status, while timeout and incomplete turns remain rejected. No actionable merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. 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: preserving successful non-replayable tool results.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@github-code-quality

github-code-quality Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 36b8c5e in the fix/agent-replay-res... branch is 97%. The line coverage in commit 63002cd in the main branch is 96%.

Show a line coverage summary of the most impacted files.
File main 63002cd fix/agent-replay-res... 36b8c5e +/-
nemoclaw/src/onboard/config.ts 98% 96% -2%
nemoclaw/src/index.ts 94% 93% -1%
nemoclaw/src/bl...t-management.ts 100% 100% 0%
nemoclaw/src/co.../config-show.ts 100% 100% 0%
nemoclaw/src/commands/slash.ts 100% 100% 0%
nemoclaw/src/on...native-route.ts 0% 100% +100%

TypeScript / code-coverage/cli

The overall line coverage in commit 36b8c5e in the fix/agent-replay-res... branch is 85%. The line coverage in commit 63002cd in the main branch is 84%.

Show a line coverage summary of the most impacted files.
File main 63002cd fix/agent-replay-res... 36b8c5e +/-
src/lib/actions.../status-text.ts 84% 46% -38%
src/lib/onboard...al-inference.ts 84% 90% +6%
src/lib/inferen...file/cleanup.ts 73% 80% +7%
src/lib/state/p...l-retirement.ts 79% 89% +10%
src/lib/readine...y-production.ts 76% 90% +14%
src/lib/onboard.../application.ts 55% 72% +17%
src/lib/onboard...mage/catalog.ts 69% 90% +21%
src/lib/securit...zer-boundary.ts 0% 85% +85%
src/lib/onboard...ternal-image.ts 0% 94% +94%
src/lib/securit...ig-structure.ts 0% 94% +94%

Updated September 30, 2026 12:39 UTC

@jyaunches
jyaunches marked this pull request as ready for review September 30, 2026 12:11
@jyaunches
jyaunches requested a review from ericksoa September 30, 2026 12:12
Two E2E support tests still classified non-replayable turns as incomplete.
Remove their obsolete success exception and rely on the shared turn classifier.
Keep explicit timeout and incomplete-turn rejection, including after tool success.

Reproduced both failures before repair. All 117 focused classifier, passthrough,
and E2E support tests pass after repair.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches

Copy link
Copy Markdown
Contributor Author

The follow-up head is 36b8c5e121aaff08ca8342c2a8069de82f5d4e65 (Verified). All 117 focused tests and normal publication hooks pass. Selected E2E is running at https://github.com/NVIDIA/NemoClaw/actions/runs/36714597299 (cloud-onboard,full-e2e,security-posture).

The new managed-image failures are also inherited. The Pi and staging-QA builders fail resolving libssl-dev=3.5.7-1~deb13u2 because APT selects libssl3t64=3.5.7-1~deb13u3. I reproduced the same solver failure with main's exact pinned node:24.18.1-trixie-slim@sha256:ac39e4b5fcb2b1b34b20364fd58b2e898f3bb80731ee6f62a7536f9df3d6aadc image and the unchanged package list, using an isolated Linux container and apt-get install --simulate. Those Dockerfiles are unchanged by this PR.

Existing #12517 addresses the OpenSSL pins and trusted audit inputs; #12507 supplies the Undici remediation. This PR remains dependent on that existing chain. No audit or build protection has been bypassed, and local Advisor clearance is not claimed.

@github-actions github-actions Bot added v0.0.131 Release target and removed v0.0.130 labels Oct 1, 2026

This branch has not been deployed

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

Labels

v0.0.131 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant