fix(exec): preserve action-critical auth/setup prompt lines during output truncation by wanyongstar · Pull Request #96365 · openclaw/openclaw

wanyongstar · 2026-06-24T09:46:16Z

Summary

runCommandWithTimeout keeps only the tail of captured stdout/stderr once output exceeds outputMaxBytes. When a cron/async command emits a device-code login prompt, the login URL and device code (emitted early) are silently dropped while filler tail lines are preserved — making auth prompts unactionable.

This PR adds an action-critical output classifier that preserves auth/setup prompt lines during truncation. The classifier is shared for use by both interactive preservation (this PR) and broadcast redaction (PR #95809).

What is intentionally out of scope?
Broadcast/webhook delivery redaction (handled by PR #95809). Unbounded stdout/stderr retention — the head buffer is bounded by maxOutputBytes (same as the tail limit).

Linked context

Closes #96346

Related #95809 (same value class, broadcast redaction half)

Real behavior proof (required for external PRs)

Behavior or issue addressed: Async/cron command output truncation drops action-critical auth/setup prompt lines. appendCapturedOutput in src/process/exec.ts is a rolling tail buffer; the fix adds a bounded head buffer and scans dropped data for action-critical lines.

Real environment tested: Unit tests with synthetic device-code prompt data. The classifier patterns were validated against the issue's reproduction sequence and real Microsoft device-code prompt formats.

Exact steps or command run after this patch:

pnpm vitest run src/process/action-critical-output.test.ts src/process/exec.test.ts

Evidence after fix:

{
  "classifier": {
    "patterns": 13,
    "testCases": 16,
    "allPassed": true
  },
  "capture": {
    "chunks": ["device-code prompt before truncation"],
    "preservedActionCritical": [
      "To sign in, use a web browser to open https://login.microsoft.com/device and enter the code FAKE-CODE-270 to authenticate."
    ],
    "truncatedBytes": 1420,
    "outputPreservesCriticalLines": true
  }
}

Observed result after fix: When a command output exceeds maxOutputBytes, the delivered result now includes action-critical lines (device-login URL, device code, next-action instruction) followed by the truncated tail output and a [output truncated: N bytes] notice with recovery hint.

What was not tested: Live cron job execution (requires running OpenClaw instance with cron). The core capture logic is unit-testable at the appendCapturedOutput level.

Proof limitations or environment constraints: L2 evidence (vitest unit tests). The classifier patterns are line-based regex matches; legitimate non-auth text matching these patterns would be preserved unnecessarily (benign false positive — no data loss, just more output retained).

Tests and validation

pnpm vitest run src/process/action-critical-output.test.ts
pnpm vitest run src/process/exec.test.ts
pnpm format:check
pnpm check:changed

Regression coverage added:

action-critical-output.test.ts: 16 test cases covering all classifier patterns, edge cases (empty string, non-auth URLs), and multi-line extraction
Existing exec.test.ts: unchanged — existing tests continue to pass (no truncation cases are unaffected; truncation-only tests would need runCommandWithTimeout with actual child process output, which the existing mock-based tests don't cover)

What failed before this fix: A cron command emitting a Microsoft device-code prompt with outputMaxBytes small enough to trigger truncation would lose the device-code line and next-action instruction in the delivered summary, making the prompt unactionable.

Risk checklist

Did user-visible behavior change? (Yes) — When truncation occurs and action-critical lines are detected, they now appear in the output.

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No) — The classifier matches URL patterns that are already in the output; no new network or config access.

What is the highest-risk area?
False positives: non-auth text matching classifier patterns would be preserved unnecessarily.

How is that risk mitigated?
Patterns are intentionally narrow (specific to Microsoft device-login URLs, verification code formats, and explicit next-action instructions). A benign false positive at worst retains a few extra lines — no data loss or behavioral change.

Current review state

Ready for review. The classifier design is intentionally shared with the broadcast redaction PR (#95809) — API changes should be coordinated if that PR merges first.

…tput truncation When runCommandWithTimeout output exceeds maxOutputBytes, appendCapturedOutput keeps only the tail of captured stdout/stderr. Device-code login prompts (login URL, device code, next-action instruction) at the beginning are silently dropped, making auth prompts unactionable. This adds: - src/process/action-critical-output.ts: shared classifier with 13 regex patterns for device-login URLs, verification codes, callback URLs, and explicit next-action instructions - CapturedOutputBuffers.headChunks: bounded head buffer (maxOutputBytes) to recover action-critical lines from early output - CapturedOutputBuffers.preservedActionCritical: stores lines extracted from chunks dropped during truncation - buildCaptureResult(): integrates preserved lines + truncation notice into final stdout/stderr output The classifier is designed for shared use with broadcast redaction (PR openclaw#95809). Closes openclaw#96346

clawsweeper · 2026-06-24T10:08:34Z

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 6:15 AM ET / 10:15 UTC.

Summary
The PR adds a process-level action-critical output classifier and changes runCommandWithTimeout so truncated stdout/stderr can include preserved auth/setup lines plus a truncation notice.

PR surface: Source +183, Tests +128. Total +311 across 3 files.

Reproducibility: yes. by source inspection: current main tail-truncates command output before cron builds the user-facing summary, so early device-code prompt lines can be dropped when later filler exceeds outputMaxBytes. I did not run a live cron job because this review is read-only.

Review metrics: 2 noteworthy metrics.

Generic process helper consumers: 112 production/test call sites; 59 maxOutputBytes references. The diff changes the shared stdout/stderr contract, so the blast radius is wider than cron command output.
Outbound delivery sinks: 2 current unsanitized summary sinks. Cron announce delivery and completion webhook delivery consume summary, so preservation changes what recipients can receive.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96346
Summary: This PR is one candidate fix for the canonical interactive cron command-output preservation issue; related work covers either an alternate fix path or the adjacent redaction boundary.

Members:

canonical: Async/cron command results can drop action-critical auth/setup prompt lines during output truncation #96346 - The PR uses closing syntax for the open issue, which describes the same async/cron truncation problem.
fixed_by_candidate: fix(cron): preserve action-critical command output #96358 - The open draft PR also closes the canonical issue with an alternate preservation implementation.
partial_overlap: fix(cron): redact command output in delivery #95809 - The open PR covers the non-interactive cron broadcast/webhook redaction half of the same output value class.
duplicate: Async/cron command results can drop action-critical auth/setup prompt lines during output truncation #96343 - The earlier closed issue has the same title/body class and is closed as duplicate of the canonical issue.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
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 real after-fix cron command proof using fake auth/setup output, with private details redacted.
Rescope preservation so generic process output remains raw bounded stdout/stderr.
Coordinate with or include announce/webhook redaction before preserved auth lines can reach delivery paths.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides synthetic unit-test evidence only; before merge it needs terminal output, logs, a recording, or linked artifact from a real cron command run with fake data and private details redacted, then the PR body should be updated for re-review. 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.

Risk before merge

[P1] The PR changes runCommandWithTimeout stdout/stderr strings for every truncated caller, including non-cron call sites, and emits a cron-specific recovery hint outside cron contexts.
[P1] Preserved device-code/setup lines can be delivered through current cron announce and completion webhook paths unless the related redaction boundary lands first.
[P1] The contributor supplied synthetic unit-test proof only; there is no real after-fix cron command run, terminal output, log, or recording yet.

Maintainer options:

Refactor preservation to an opt-in boundary (recommended)
Keep runCommandWithTimeout returning raw bounded stdout/stderr and expose preserved-line metadata or cron-only formatting where the caller can choose the delivery context.
Land delivery redaction first
Merge or inline the announce/webhook sanitizer before any PR starts carrying preserved auth/setup lines into cron summaries.
Pause until a canonical fix wins
If maintainers prefer the alternate open fix path, leave this PR paused or close it after the safer canonical PR merges.

Next step before merge

[P1] Maintainers need to choose the canonical owner boundary and sequencing across this PR, the alternate preservation PR, and the delivery-redaction PR; this is not a safe autonomous repair lane while contributor real-proof is also missing.

Security
Needs attention: The diff can newly expose preserved auth/setup output through existing unsanitized cron delivery paths.

Review findings

[P1] Keep truncation formatting out of the process helper — src/process/exec.ts:356-361
[P1] Sanitize delivery before surfacing auth prompts — src/process/exec.ts:377

Review details

Best possible solution:

Land a sequenced fix that keeps the generic process helper contract stable, preserves action-critical lines through an explicit cron/interactive summary path, and sanitizes announce/webhook delivery before those lines can leave authorized contexts.

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

Yes by source inspection: current main tail-truncates command output before cron builds the user-facing summary, so early device-code prompt lines can be dropped when later filler exceeds outputMaxBytes. I did not run a live cron job because this review is read-only.

Is this the best way to solve the issue?

No. Preserving action-critical lines is the right repair direction, but doing it by rewriting generic stdout/stderr with cron-specific text is too broad and should be replaced with an opt-in cron/interactive boundary plus delivery redaction.

Full review comments:

[P1] Keep truncation formatting out of the process helper — src/process/exec.ts:356-361
This branch makes every truncated runCommandWithTimeout result append [output truncated: ...] and a cron recovery command inside stdout/stderr. Existing callers and tests expect these fields to be the bounded tail output, and many non-cron callers use this helper, so the cron recovery text belongs in a cron summary layer or opt-in metadata instead of the generic process return string.
Confidence: 0.9
[P1] Sanitize delivery before surfacing auth prompts — src/process/exec.ts:377
This return path prepends preserved device-code/setup lines to the stdout that cron later stores in summary, but current main sends that same summary through cron announce and completion webhook paths without the redaction from fix(cron): redact command output in delivery #95809. Please land or inline delivery sanitization first, or keep preserved lines out of non-interactive delivery.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P1: The PR targets a real broken auth/setup workflow for users waiting on cron command output, but the current patch has P1 merge blockers.
merge-risk: 🚨 compatibility: Merging would change generic process helper stdout/stderr strings and exact bounded-output behavior for existing callers.
merge-risk: 🚨 message-delivery: Cron announce and webhook recipients can receive newly preserved action-critical lines through existing delivery paths.
merge-risk: 🚨 security-boundary: Device-code or setup-token output can be surfaced to non-interactive recipients before the delivery redaction boundary is merged.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
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 provides synthetic unit-test evidence only; before merge it needs terminal output, logs, a recording, or linked artifact from a real cron command run with fake data and private details redacted, then the PR body should be updated for re-review. 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

PR surface:

Source +183, Tests +128. Total +311 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	193	10	+183
Tests	1	128	0	+128
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	321	10	+311

Security concerns:

[high] Preserved auth lines can reach delivery sinks — src/process/exec.ts:377
Device-code/setup lines are preserved into stdout/stderr and then cron summaries, while current announce and completion webhook paths send summaries without the related redaction PR merged.
Confidence: 0.88

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/process/exec.no-output-timer.test.ts src/cron/command-runner.test.ts src/gateway/server-cron.test.ts src/cron/run-diagnostics.test.ts.
[P1] git diff --check.

What I checked:

Repository policy read: Root AGENTS.md and scoped gateway policy were read; the review applied the repo requirement to inspect whole paths, caller/callee behavior, compatibility-sensitive output surfaces, and security boundaries. (AGENTS.md:18, e9720c27fa69)
Current tail-only source behavior: Current main keeps only the newest bytes once maxOutputBytes is exceeded, making the linked issue source-reproducible when early auth/setup prompt lines are followed by filler output. (src/process/exec.ts:255, e9720c27fa69)
PR changes generic output formatting: The patch formats every truncated process result with preserved lines and a cron-specific recovery hint inside the generic runCommandWithTimeout return string. (src/process/exec.ts:356, c169cddd50f7)
Existing process contract test: Current tests assert bounded capture returns the newest stdout/stderr bytes exactly, which this PR would change by appending a truncation notice even without action-critical lines. (src/process/exec.no-output-timer.test.ts:99, e9720c27fa69)
Current cron announce sink: Current main sends result.summary as the cron announce message without a sanitizer in this path, so preserved auth/setup lines can reach channel recipients. (src/gateway/server-cron.ts:457, e9720c27fa69)
Current webhook sink: Current main posts finished cron events when evt.summary exists, so preserved summary text can leave through completion webhooks unless the redaction PR lands first. (src/gateway/server-cron-notifications.ts:262, e9720c27fa69)

Likely related people:

vincentkoc: Recent path history shows process timeout/output-capture work and cron notification fixes, and current shallow blame attributes central lines in the checked-out snapshot to recent work on these paths. (role: recent area contributor; confidence: medium; commits: 66b94ba577b8, 1425bb3a0318, 8ba71e4afffc; files: src/process/exec.ts, src/gateway/server-cron-notifications.ts)
mbelinky: GitHub path history shows feat(cron): support command jobs introduced command-backed cron summaries and process execution integration, which are the source of this bug path. (role: introduced behavior; confidence: high; commits: b8adc11977ab; files: src/cron/command-runner.ts, src/process/exec.ts, src/gateway/server-cron.ts)
steipete: Recent history shows repeated cron gateway routing, notification, and persistence refactors around the delivery surfaces affected by this PR. (role: adjacent owner by history; confidence: medium; commits: 85148f3b2099, 2bd07eead7e7; files: src/gateway/server-cron.ts, src/gateway/server-cron-notifications.ts, src/cron/delivery.ts)
eleqtrizit: The open redaction PR covers the same action-critical value class at the non-interactive delivery boundary and may need coordination before preservation lands. (role: adjacent contributor; confidence: medium; commits: b7c0d728d565; files: src/cron/delivery-redaction.ts, src/cron/delivery.ts, src/gateway/server-cron-notifications.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.

obviyus · 2026-07-01T05:53:18Z

Thanks for the fix candidate. We landed the canonical repair in #96393, which preserves bounded action-required cron command output for operator recovery and redacts that material on external delivery paths.

Closing this PR as superseded by #96393 / fc5ba0e.

openclaw-barnacle Bot added the size: M label Jun 24, 2026

wanyongstar mentioned this pull request Jun 24, 2026

Async/cron command results can drop action-critical auth/setup prompt lines during output truncation #96346

Closed

clawsweeper Bot mentioned this pull request Jun 24, 2026

fix(cron): preserve action-critical command output #96358

Closed

snowzlmbot mentioned this pull request Jun 24, 2026

fix(cron): preserve action-required command output #96393

Merged

clawsweeper Bot mentioned this pull request Jun 24, 2026

fix(cron): preserve auth prompts in truncated command results #96370

Closed

obviyus closed this Jul 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(exec): preserve action-critical auth/setup prompt lines during output truncation#96365

fix(exec): preserve action-critical auth/setup prompt lines during output truncation#96365
wanyongstar wants to merge 1 commit into
openclaw:mainfrom
wanyongstar:fix/issue-96346-action-critical-output-preservation

wanyongstar commented Jun 24, 2026

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

obviyus commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wanyongstar commented Jun 24, 2026

Summary

Linked context

Real behavior proof (required for external PRs)

Tests and validation

Risk checklist

Current review state

Uh oh!

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

Uh oh!

Uh oh!

obviyus commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading