fix(agents): preserve fd find failures by zhangguiping-xydt · Pull Request #99446 · openclaw/openclaw

zhangguiping-xydt · 2026-07-03T08:45:42Z

Related: problem-mode user report; no public issue number

What Problem This Solves

The find session tool could treat incomplete fd output as a complete search result. If fd wrote one or more stdout lines, then exited non-zero with a stderr failure, the tool returned the partial stdout as a successful result and dropped the failure reason.

Why This Change Was Made

Treat any non-zero fd exit as a failed search, even when stdout already contains partial results.
Preserve fd stderr or the fallback exit code in the thrown error.
Add a regression test that runs the real find tool path with a fake fd process producing stdout, stderr, and exit code 2.
Did not change custom FindOperations.glob behavior, glob pattern rewriting, result limiting, or output truncation.

User Impact

Users no longer receive incomplete file-search results as if the search finished normally. If fd fails while walking a subtree after emitting partial matches, the session tool now surfaces the failure instead of silently hiding it.

Evidence

Branch base checked during freshness gate: main@adafb56b2eb9207d93896ebf6c5faa9205875833. No final rebase was performed.
Before-fix proof on latest main: the new regression test failed because the promise resolved with partial.ts instead of rejecting after fake fd wrote stderr and exited 2.

Real behavior proof:

Behavior or issue addressed: fd partial stdout plus non-zero exit was incorrectly returned as a complete find result.
Real environment tested: macOS 26.5.1, Node v24.15.0, OpenClaw v2026.6.11, worktree SHA 16b4542.

Exact steps or command run after this patch:

node --import tsx <<'NODE'
import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";

const tempDir = mkdtempSync(join(tmpdir(), "openclaw-find-proof-"));
process.env.OPENCLAW_AGENT_DIR = join(tempDir, "agent");
process.env.OPENCLAW_OFFLINE = "1";
process.env.PATH = `${tempDir}:${process.env.PATH ?? ""}`;

try {
  const fdPath = join(tempDir, "fd");
  writeFileSync(fdPath, `#!/usr/bin/env node
if (process.argv.includes("--version")) {
  process.stdout.write("fd 10.0.0\\n");
  process.exit(0);
}
const searchRoot = process.argv[process.argv.length - 1];
process.stdout.write(searchRoot + "/partial.ts\\n");
process.stderr.write("fd failed while reading subtree\\n");
process.exit(2);
`);
  chmodSync(fdPath, 0o755);

  const { createFindToolDefinition } = await import("./src/agents/sessions/tools/find.ts");
  const tool = createFindToolDefinition(tempDir);

  console.log("=== find fd partial failure proof ===");
  console.log("fake fd behavior: stdout=partial.ts, stderr=fd failed while reading subtree, exit=2");
  try {
    const result = await tool.execute("proof-call", { pattern: "*.ts" }, undefined, undefined, {});
    console.log("unexpected success:", JSON.stringify(result));
    process.exitCode = 1;
  } catch (error) {
    console.log("observed failure:", error instanceof Error ? error.message : String(error));
    console.log("partial result returned: no");
  }
} finally {
  rmSync(tempDir, { recursive: true, force: true });
}
NODE

node scripts/run-vitest.mjs src/agents/sessions/tools/find.test.ts
OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false corepack pnpm check:changed -- src/agents/sessions/tools/find.ts src/agents/sessions/tools/find.test.ts
pnpm check:test-types

Evidence after fix:

=== find fd partial failure proof ===
fake fd behavior: stdout=partial.ts, stderr=fd failed while reading subtree, exit=2
observed failure: fd failed while reading subtree
partial result returned: no

Observed result after fix: the tool rejected with the fd stderr and did not return the partial stdout line as a successful result.
What was not tested: did not start a live gateway; this is a pure session-tool child-process failure path, and the proof calls the patched tool directly with a real child process.

Regression tests and local checks:

node scripts/run-vitest.mjs src/agents/sessions/tools/find.test.ts
Test Files  1 passed (1)
Tests  3 passed (3)

OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 PNPM_CONFIG_VERIFY_DEPS_BEFORE_RUN=false corepack pnpm check:changed -- src/agents/sessions/tools/find.ts src/agents/sessions/tools/find.test.ts
lanes=core, coreTests; no conflict markers; dependency pin guard passed; package patch guard passed; no new test temp-directory migration warnings; core and core-test typechecks passed; changed-file lint and guard checks passed.

pnpm check:test-types
tsgo core tests and extension tests completed successfully.

Regression Test Plan

Coverage level: Unit test
Target test file: src/agents/sessions/tools/find.test.ts
Scenario locked in: fake fd emits one stdout result, emits stderr, exits with code 2, and the find tool rejects with the stderr message.
Why this is the smallest reliable guardrail: it exercises the default fd child-process path without starting unrelated runtime services.

Root Cause

Root cause: find.ts computed the fd error message for non-zero exits but only rejected when stdout was empty.
Missing detection / guardrail: there was no regression test for partial stdout combined with a non-zero fd exit and stderr.

clawsweeper · 2026-07-03T08:46:45Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 6:54 AM ET / 10:54 UTC.

Summary
The branch changes the fd-backed session find tool to reject non-zero fd exits even after partial stdout and adds a dedicated regression test for that child-process failure path.

PR surface: Source -2, Tests +37. Total +35 across 2 files.

Reproducibility: yes. Source inspection of current main and the latest release shows the non-zero fd close path can resolve partial stdout, and the PR body includes terminal before/after proof for a fake fd emitting stdout, stderr, and exit code 2.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/tools/find.fd.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

[P2] No repair lane is needed because this pass found no actionable patch defect; the remaining action is ordinary maintainer review and merge gating.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes local child-process error handling and tests without new dependencies, workflows, secrets, or package metadata.

Review details

Best possible solution:

Merge the fd close-handler fix after normal maintainer and CI gates, keeping the behavior scoped to the fd-backed find path and the regression test isolated from custom-operation coverage.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection of current main and the latest release shows the non-zero fd close path can resolve partial stdout, and the PR body includes terminal before/after proof for a fake fd emitting stdout, stderr, and exit code 2.

Is this the best way to solve the issue?

Yes. Rejecting non-zero fd completions in the child-process close handler is the narrowest maintainable fix; the alternatives of filtering partial stdout later or changing custom FindOperations.glob would touch the wrong boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 05de72c4cd7d.

Label changes

Label changes:

add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof from a real Node child-process path showing the patched find tool rejects fd partial stdout plus non-zero exit instead of returning the partial result.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a focused agent session-tool correctness fix for silently incomplete file-search results, with limited blast radius.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof from a real Node child-process path showing the patched find tool rejects fd partial stdout plus non-zero exit instead of returning the partial result.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof from a real Node child-process path showing the patched find tool rejects fd partial stdout plus non-zero exit instead of returning the partial result.

Evidence reviewed

PR surface:

Source -2, Tests +37. Total +35 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	4	-2
Tests	1	37	0	+37
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	39	4	+35

What I checked:

Root policy read: Root review policy was read fully and applied; it required scoped policy, caller/callee, sibling, dependency-contract, proof, and best-fix checks before a verdict. (AGENTS.md:1, 05de72c4cd7d)
Scoped agents policy read: The scoped agent test guidance was read and applied; it favors narrow isolated tests for agent hot paths and no contrary rule affected this PR. (src/agents/AGENTS.md:1, 05de72c4cd7d)
Current main behavior: Current main computes an fd error for non-zero close but rejects only when collected stdout is empty, so partial stdout can still resolve as a successful find result. (src/agents/sessions/tools/find.ts:309, 05de72c4cd7d)
Latest release behavior: The latest release tag has the same stdout-empty guard on non-zero fd close, so the bug is not already shipped fixed. (src/agents/sessions/tools/find.ts:309, 08d1bbad1bd6)
PR runtime behavior: The PR head removes the stdout-empty guard and rejects immediately with fd stderr or an exit-code fallback for any non-zero close. (src/agents/sessions/tools/find.ts:309, e0719051f181)
Regression coverage: The added test keeps typed PassThrough stdout/stderr locals, emits a partial stdout line plus stderr and close code 2, and expects rejection with the stderr message. (src/agents/sessions/tools/find.fd.test.ts:16, e0719051f181)

Likely related people:

vincentkoc: Recent GitHub path history shows repeated changes to src/agents/sessions/tools/find.ts for session find rendering, result building, stderr bounds, and limit normalization. (role: recent area contributor; confidence: high; commits: 32494c7aced4, 336494c86373, 25b87b111d0d; files: src/agents/sessions/tools/find.ts, src/agents/sessions/tools/find.test.ts)
steipete: GitHub history shows session-tool docs/test contributions, and the PR discussion includes the exact-head maintainer proof update plus the latest fixture-isolation commit. (role: recent reviewer and adjacent contributor; confidence: medium; commits: 600a57e60f1e, d045deb79d28, e0719051f181; files: src/agents/sessions/tools/find.ts, src/agents/sessions/tools/find.test.ts, src/agents/sessions/tools/find.fd.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-03T10:16:20Z

Maintainer proof update for exact head e0719051f1813ac1641df5851c14c9710a1602e7:

Best-fix review: reject every non-zero fd completion at the child-process owner. Upstream fd defines all non-zero exits as errors, so returning accumulated stdout would misrepresent a partial traversal as complete.
Improvement: replaced the Unix-only executable fixture with a deterministic mocked child process in a dedicated test file. This avoids pipe-flush races, keeps the existing custom-operation test isolated, and restores the test-project routing inventory.
Focused proof: node scripts/run-vitest.mjs src/agents/sessions/tools/find.test.ts src/agents/sessions/tools/find.fd.test.ts — 3 passed; node scripts/run-vitest.mjs src/scripts/test-projects.test.ts — 83 passed; core test types passed with tsgo.
Fresh autoreview: clean, 0.98 confidence.
Duplicate search: no competing open issue or PR for partial fd stdout on failure.

Fresh hosted checks are running for this rebased exact head.

steipete · 2026-07-03T11:30:46Z

Merged via squash.

Prepared head SHA: e0719051f1813ac1641df5851c14c9710a1602e7
Landed commit: 749c0bf9581a2b196c972ce8ed263c7e0d855a0a

* fix(agents): preserve fd find failures * test(agents): use tracked temp dirs in find test * test(scripts): update temp-dir routing expectation * test(agents): make fd failure fixture deterministic * test(agents): isolate fd process failure fixture --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jul 3, 2026

steipete self-assigned this Jul 3, 2026

steipete requested a review from a team as a code owner July 3, 2026 10:15

openclaw-barnacle Bot added docs Improvements or additions to documentation app: ios App: ios gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu plugin: file-transfer extensions: ollama size: XL and removed size: XS labels Jul 3, 2026

steipete force-pushed the fix/find-fd-partial-failure branch from af32f55 to 840d109 Compare July 3, 2026 10:25

openclaw-barnacle Bot added size: S and removed docs Improvements or additions to documentation app: ios App: ios gateway Gateway runtime commands Command implementations channel: feishu Channel integration: feishu plugin: file-transfer extensions: ollama size: XL labels Jul 3, 2026

clawsweeper Bot removed the rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. label Jul 3, 2026

zhangguiping-xydt and others added 5 commits July 3, 2026 11:48

fix(agents): preserve fd find failures

2174b02

test(agents): use tracked temp dirs in find test

a10a85b

test(scripts): update temp-dir routing expectation

94bd83a

test(agents): make fd failure fixture deterministic

ec0eb5f

test(agents): isolate fd process failure fixture

e071905

steipete force-pushed the fix/find-fd-partial-failure branch from 840d109 to e071905 Compare July 3, 2026 10:48

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 3, 2026

steipete merged commit 749c0bf into openclaw:main Jul 3, 2026
122 of 130 checks passed

github-actions Bot mentioned this pull request Jul 3, 2026

📡 Upstream Digest — 2026-07-03 13:45 UTC curtismercier/openclaw-mods#1197

Open

clawsweeper Bot mentioned this pull request Jul 3, 2026

Recover stranded message-tool finals by default #99536

Merged

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(find): prevent recursive re-entry in child kill via sentinel guard (#98961) #99025

Closed

clawsweeper Bot mentioned this pull request Jul 16, 2026

fix(find): call stopChild in error handler for symmetric cleanup #98854

Closed

Navigation Menu

Uh oh!

fix(agents): preserve fd find failures - #99446

fix(agents): preserve fd find failures#99446
steipete merged 5 commits into
openclaw:mainfrom
zhangguiping-xydt:fix/find-fd-partial-failure

zhangguiping-xydt commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zhangguiping-xydt commented Jul 3, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Regression Test Plan

Root Cause

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

steipete commented Jul 3, 2026 •

edited

Loading