fix(cron): summarize shell failures directly by ashishpatel26 · Pull Request #92603 · openclaw/openclaw

ashishpatel26 · 2026-06-13T05:17:07Z

Summary

Problem

Cron announce failures with no final assistant message could deliver a synthesized (agent) failed summary that paraphrased raw bash into a misleading step list, such as shell keywords being shown as separate run then, run [ and run jq steps.

Root Cause

Exec/bash tool failures reused the generic tool display summary. That summary is useful for timelines, but it tokenizes shell stages into human-readable action fragments, which becomes confusing when cron delivery promotes the error payload as the user-visible failure notification.

Solution

Capture a bounded, redacted raw command excerpt for exec/bash tool calls.
Carry that excerpt into the last tool error summary.
For cron-triggered exec/bash failures, format the failure directly from exit/timeout/error context and the command excerpt.
Preserve the existing generic summaries for non-shell tools and non-cron warning paths.

Testing Performed

cmd.exe /C node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/payloads.errors.test.ts src/cron/isolated-agent.helpers.test.ts src/cron/run-diagnostics.test.ts src/agents/tool-display.test.ts src/agents/embedded-agent-subscribe.handlers.tools.test.ts src/agents/embedded-agent-runner/run/payloads.test.ts src/auto-reply/status.test.ts src/agents/command/session-store.test.ts src/gateway/session-utils.fs.test.ts
- Passed 5 Vitest shards in 236.27s.
git diff --check
- Passed.

Real behavior proof

Behavior addressed: Cron shell failures now produce direct failure copy like Bash exited with code 1. Last command: ... instead of promoting generic shell step-list text.

Real environment tested: Local source checkout on current upstream/main, using the repo Vitest wrapper through Windows cmd.exe.

Exact steps or command run after this patch: The focused multi-file node scripts/run-vitest.mjs ... command listed above, plus git diff --check.

Evidence after fix: The parent validation passed all 5 Vitest shards, including payloads.errors.test.ts, isolated-agent.helpers.test.ts, run-diagnostics.test.ts, tool-display.test.ts, and embedded-agent-subscribe.handlers.tools.test.ts.

Observed result after fix: Regression tests assert cron bash failures deliver direct shell failure text and command excerpts, while existing non-shell payload behavior remains covered by surrounding tests.

What was not tested: A live cron-to-Discord delivery run was not executed in this environment.

clawsweeper · 2026-06-13T05:18:42Z

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 7:48 PM ET / 23:48 UTC.

Summary
The PR adds redacted shell command excerpts to exec/bash tool error state and uses them to render cron shell failures as direct exit/timeout messages instead of generic tool-display step lists.

Reproducibility: yes. source-level: current main still stores generic exec/bash summary text and formats it through the existing warning path before cron can promote fatal payload text. I did not run a live cron-to-channel reproduction in this read-only review.

Review metrics: 2 noteworthy metrics.

Channel-visible command excerpt path: 1 added. The PR introduces a new path that can show bounded redacted shell command text in cron delivery notifications.
Pre-commit automation surface: 1 hook changed, 1 helper changed. The branch changes contributor automation outside the cron bugfix, and that path is part of the current merge risk.

Stored data model
Persistent data-model change detected: unknown-truncated-pull-files, vector/embedding metadata: extensions/memory-wiki/src/okf.ts, vector/embedding metadata: src/secrets/target-registry-data.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #87426
Summary: This PR is the current candidate fix for the canonical cron shell-summary issue; the earlier heuristic PR is closed unmerged and the later exec-display PR is adjacent rather than a replacement.

Members:

canonical: Cron delivery: synthesized "(agent) failed" summary paraphrases bash into garbled "step list" #87426 - The PR body uses closing syntax for this open issue and changes the implicated cron payload and exec/bash summary paths.
superseded: fix(agents): fall back to raw command when most exec stages are generic #87431 - The earlier PR proposed a broader exec-summary heuristic for the same user report, but it is closed unmerged and is not a viable current landing path.
adjacent_distinct: fix(agents): remove misleading run prefix from exec display for interpreter commands (fixes #97319) #97326 - That closed PR addressed a misleading exec display prefix for interpreter commands, which overlaps the tool-display area but does not solve the cron direct shell-failure notification path.
adjacent_distinct: WORKING: All Microsoft Issues and PRs (refresh) #74163 - The tracker mentions this PR in a broad Microsoft/Windows checklist, but it does not own the root cause or replacement implementation path.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

[P1] Add redacted real cron-to-channel output, terminal output, runtime logs, screenshot, recording, or linked artifact showing the delivered after-fix notification.
Resolve conflicts and make the current head pass lint/quality checks.
Get maintainer acceptance for command excerpts or narrow the emitted text.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body lists focused Vitest and diff-check validation and explicitly says no live cron-to-Discord delivery run was executed, so contributor real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Proof path suggestion
A visible real-channel proof would materially help because the changed behavior is the delivered cron failure notification text. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

[P1] Contributor proof is still test-only; there is no redacted live cron-to-channel output, runtime log, terminal output, screenshot, recording, or linked artifact showing the delivered after-fix notification.
[P1] Merging would intentionally make bounded redacted shell command excerpts channel-visible in cron failure notifications, so maintainers need to accept or narrow that disclosure boundary.
[P1] The branch is currently conflicting and carries broad formatting churn plus a pre-commit automation change outside the cron bugfix.
[P1] The pre-commit helper changes currently have lint defects, and live checks also report failing Critical Quality lanes that need resolution before merge.

Maintainer options:

Clean, narrow, and prove delivery (recommended)
Resolve conflicts, fix the script lint defects, trim unrelated churn where practical, add redacted real cron-to-channel or runtime-log proof, and document maintainer acceptance or narrowing of command excerpts.
Split the pre-commit ARG_MAX work
If the stdin/xargs hook change is wanted, move it to a separate tooling PR with focused script validation so this PR stays scoped to cron failure copy.
Accept the command-excerpt boundary
Maintainers may intentionally accept bounded redacted command excerpts in scheduled failure notifications, but that should be explicit because CI cannot prove every private command string is safe to show.

Next step before merge

[P1] Manual review is needed because the remaining blockers include contributor real behavior proof, merge conflicts/check failures, broad branch cleanup, and a maintainer security-boundary choice.

Security
Needs attention: The diff adds a channel-visible shell command excerpt path, which needs maintainer acceptance or tighter emission rules despite bounded redaction.

Review findings

[P2] Remove the unused pre-commit import — scripts/pre-commit/filter-staged-files.mjs:2
[P2] Fix the stdin reader lint failures — scripts/pre-commit/filter-staged-files.mjs:59-68

Review details

Best possible solution:

Land a narrowed, conflict-free branch that keeps the cron shell-failure fix, fixes or splits the pre-commit tooling change, adds real redacted delivery proof, and has maintainer acceptance or tightening of command excerpts.

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

Yes, source-level: current main still stores generic exec/bash summary text and formats it through the existing warning path before cron can promote fatal payload text. I did not run a live cron-to-channel reproduction in this read-only review.

Is this the best way to solve the issue?

Yes with caveats: direct cron shell-failure formatting is narrower than changing all exec display summaries, but the submitted branch is not the best merge shape until proof, conflicts, lint/quality failures, and command-excerpt acceptance are handled.

Full review comments:

[P2] Remove the unused pre-commit import — scripts/pre-commit/filter-staged-files.mjs:2
createReadStream is imported but never used in the modified helper, which keeps the submitted automation change lint-failing. Remove the import before merge.
Confidence: 0.95
[P2] Fix the stdin reader lint failures — scripts/pre-commit/filter-staged-files.mjs:59-68
The new stdin path uses unbraced single-line if statements and appends the data chunk directly, matching the existing lint failure pattern for the helper. Brace the branches and coerce or type the chunk so the pre-commit helper passes lint.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5e572dcf781a.

Label changes

Label justifications:

P2: The PR targets misleading scheduled-job failure notifications with limited blast radius, not an urgent outage, data loss, or core availability failure.
merge-risk: 🚨 security-boundary: The diff can expose bounded redacted shell command excerpts in user-visible cron delivery channels.
merge-risk: 🚨 automation: The diff changes the pre-commit hook/helper and current checks show lint/quality failures in the submitted branch.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists focused Vitest and diff-check validation and explicitly says no live cron-to-Discord delivery run was executed, so contributor real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

Security concerns:

[medium] Validate channel-visible command excerpts — src/agents/embedded-agent-runner/run/payloads.ts:219
Last command snippets can include private endpoints, local paths, account names, or custom secret shapes that generic redaction may miss, so this disclosure boundary should be explicitly accepted or narrowed before merge.
Confidence: 0.75

What I checked:

Root policy read and applied: Root AGENTS.md was read fully from the checkout; its PR review guidance on source proof, scoped policy, security-boundary review, automation risk, and real behavior proof affected this review. (AGENTS.md:1, 5e572dcf781a)
Scoped policy read: Relevant scoped AGENTS.md files for agents, embedded runner, scripts, outbound/plugin/extension/UI/test paths were read; the central guidance favored focused helper tests, wrapper rules, and avoiding unrelated automation churn. (scripts/AGENTS.md:1, 5e572dcf781a)
Current main still formats exec failures through generic warning text: Current main builds exec-like warning text from formatToolAggregate/formatExecLikeFailureSubject rather than a cron-specific shell failure summary, so a summarized shell meta can still be promoted into user-visible cron output. (src/agents/embedded-agent-runner/run/payloads.ts:214, 5e572dcf781a)
Current main still joins shell stages into arrow summaries: Current main splits shell commands into stages, summarizes each, and joins multi-stage output with arrows; mixed known/generic shell scripts can still produce the reported step-list style text. (src/agents/tool-display-exec.ts:358, 5e572dcf781a)
Current main stores only generic meta on lastToolError: Current main records toolName, meta, error, timeout, middleware, and mutation fields in lastToolError, but no raw command excerpt exists for the cron payload layer to prefer. (src/agents/embedded-agent-subscribe.handlers.tools.ts:1193, 5e572dcf781a)
Cron delivery promotes fatal error payload text: resolveCronPayloadOutcome uses the fatal structured error payload text as summary/output/delivery when no later success or final assistant text recovers the run, matching the linked issue's delivery path. (src/cron/isolated-agent/helpers.ts:339, 5e572dcf781a)

Likely related people:

Peter Steinberger: Git history shows the exec display parsing split and shared tool-display helper refactor on the central shell summary path, and shortlog shows the most central-file touches in this area. (role: feature-history owner; confidence: high; commits: ca0159569954, 2b5d8ac95120; files: src/agents/tool-display-exec.ts, src/agents/tool-display-common.ts, src/agents/tool-display-record.ts)
Vincent Koc: Git shortlog/history shows recent touches across the central payload and tool-display files included in this review surface. (role: recent area contributor; confidence: medium; commits: e085fa1a3ffd; files: src/agents/tool-display-exec.ts, src/agents/embedded-agent-runner/run/payloads.ts, src/agents/embedded-agent-subscribe.handlers.tools.ts)
welfo-beo: History on the cron isolated-agent helper includes prior final announce delivery work in the same delivery boundary that promotes error payloads. (role: adjacent cron delivery contributor; confidence: medium; commits: 81c7304a18b8; files: src/cron/isolated-agent/helpers.ts, src/cron/isolated-agent/run.ts, src/cron/isolated-agent.helpers.test.ts)
Masato Hoshino: The current main line blame and history on the affected agent files include a recent merged agent/tool-display change, making this person useful routing context for nearby behavior. (role: recent area contributor; confidence: medium; commits: a75431c586ce; files: src/agents/tool-display-exec.ts, src/agents/embedded-agent-runner/run/payloads.ts, src/cron/isolated-agent/helpers.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.

Review history (1 earlier review cycle)

reviewed 2026-06-21T18:59:29.339Z sha 2433595 :: needs real behavior proof before merge. :: [P2] Remove the unused pre-commit import | [P2] Fix the stdin reader lint failures

…mmary Resolve conflict in embedded-agent-subscribe.handlers.tools.ts: preserve upstream replay-safety guard (attemptedMutatingAction) for mutatingAction/actionFingerprint/fileTarget while retaining the commandExcerpt field from this branch. Fix pre-commit hook ARG_MAX failure on Windows when staging large file counts (e.g. upstream merge). Pipe file lists via NUL-delimited stdin instead of argv, and use xargs -0 for git add to chunk large lists. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

ashishpatel26 · 2026-06-19T09:32:10Z

@clawsweeper re-review

Branch merged with upstream/main. Conflict in embedded-agent-subscribe.handlers.tools.ts resolved: preserved upstream replay-safety guard (attemptedMutatingAction) for mutatingAction/actionFingerprint/fileTarget while retaining the commandExcerpt field from this branch. Also fixed pre-commit hook ARG_MAX issue for large merges.

clawsweeper · 2026-06-19T09:32:14Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

openclaw-barnacle · 2026-07-21T04:36:07Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

fix(cron): summarize shell failures directly

529739c

openclaw-barnacle Bot added agents Agent runtime and tooling size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 13, 2026

clawsweeper Bot mentioned this pull request Jun 15, 2026

Cron delivery: synthesized "(agent) failed" summary paraphrases bash into garbled "step list" #87426

Open

ashishpatel26 requested a review from a team as a code owner June 19, 2026 09:31

openclaw-barnacle Bot added extensions: deepinfra extensions: amazon-bedrock extensions: diffs extensions: copilot size: XL and removed size: M labels Jun 19, 2026

BradGroux mentioned this pull request Jun 19, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

clawsweeper Bot mentioned this pull request Jun 27, 2026

fix(agents): remove misleading run prefix from exec display for interpreter commands (fixes #97319) #97326

Closed

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): summarize shell failures directly#92603

fix(cron): summarize shell failures directly#92603
ashishpatel26 wants to merge 2 commits into
openclaw:mainfrom
ashishpatel26:fix/87426-cron-bash-failure-summary

ashishpatel26 commented Jun 13, 2026

Uh oh!

clawsweeper Bot commented Jun 13, 2026 •

edited

Loading

Uh oh!

ashishpatel26 commented Jun 19, 2026

Uh oh!

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

ashishpatel26 commented Jun 13, 2026

Summary

Problem

Root Cause

Solution

Testing Performed

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

ashishpatel26 commented Jun 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jun 13, 2026 •

edited

Loading

clawsweeper Bot commented Jun 19, 2026 •

edited

Loading