test(agents): add unit tests for exec output rendering helpers by dwc1997 · Pull Request #96448 · openclaw/openclaw

dwc1997 · 2026-06-24T13:39:24Z

What Problem This Solves

The bash-tools.exec-output.ts module provides rendering helpers for exec output and status updates. However, this module lacked unit tests to verify the rendering logic, which could lead to regressions when the function is modified.

Why This Change Was Made

Add unit tests for renderExecOutputText and renderExecUpdateText functions to verify output rendering behavior including placeholder handling and warning text placement. This improves test coverage and prevents regressions.

Focused tests cover placeholder for empty/undefined output, preserving newlines, warning text placement, multiple warnings handling, empty warnings array with tailText, and undefined tailText with warnings.

User Impact

Exec output rendering now has comprehensive test coverage, reducing the risk of regressions when the function is modified.

Evidence

Reproducibility: yes. Source inspection of current main shows the bash-tools.exec-output.ts module exists and provides rendering helpers for exec output, but lacks unit tests.

Direct behavior probe:

$ node --import tsx -e "import('./src/agents/bash-tools.exec-output.js').then(({ renderExecOutputText, renderExecUpdateText }) => console.log(JSON.stringify([{ desc: 'Placeholder for undefined', input: undefined, result: renderExecOutputText(undefined) }, { desc: 'Placeholder for empty', input: '', result: renderExecOutputText('') }, { desc: 'Preserving newlines', input: 'line1\nline2', result: renderExecOutputText('line1\nline2') }, { desc: 'Warning text placement', params: { tailText: 'hello', warnings: ['warning1'] }, result: renderExecUpdateText({ tailText: 'hello', warnings: ['warning1'] }) }, { desc: 'Multiple warnings', params: { tailText: 'hello', warnings: ['warning1', 'warning2'] }, result: renderExecUpdateText({ tailText: 'hello', warnings: ['warning1', 'warning2'] }) }], null, 2)))"
[
  {
    "desc": "Placeholder for undefined",
    "result": "(no output)"
  },
  {
    "desc": "Placeholder for empty",
    "input": "",
    "result": "(no output)"
  },
  {
    "desc": "Preserving newlines",
    "input": "line1\nline2",
    "result": "line1\nline2"
  },
  {
    "desc": "Warning text placement",
    "params": {
      "tailText": "hello",
      "warnings": [
        "warning1"
      ]
    },
    "result": "warning1\n\nhello"
  },
  {
    "desc": "Multiple warnings",
    "params": {
      "tailText": "hello",
      "warnings": [
        "warning1",
        "warning2"
      ]
    },
    "result": "warning1\nwarning2\n\nhello"
  }
]

Targeted test:

$ node scripts/run-vitest.mjs run src/agents/bash-tools.exec-output.test.ts

 RUN  v4.1.8 /home/0668001110/ZTEProject/openclaw-worktrees/pr-94335

 Test Files  1 passed (1)
      Tests  12 passed (12)
   Start at  14:48:45
   Duration  304ms (transform 18ms, setup 0ms, import 39ms, tests 12ms)

[test] passed 1 Vitest shard in 10.51s

Formatting:

$ npx oxfmt --check src/agents/bash-tools.exec-output.ts src/agents/bash-tools.exec-output.test.ts
Checking formatting...

All matched files use the correct format.
Finished in 73ms on 2 files using 8 threads.

Whitespace:

$ git diff --check

AI-assisted

Prepared with Codex. I reviewed the change, understand the touched code path, and kept the PR focused on the bug described above.

Add unit tests for renderExecOutputText and renderExecUpdateText functions in src/agents/bash-tools.exec-output.ts to verify output rendering behavior. Tests cover: - Placeholder for empty/undefined output - Preserving newlines in output - Warning text placement - Multiple warnings handling - Empty warnings array with tailText - Undefined tailText with warnings

clawsweeper · 2026-06-24T13:56:53Z

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 5:28 AM ET / 09:28 UTC.

Summary
The branch adds a colocated Vitest file covering exec output helper placeholders, newline preservation, and warning placement.

PR surface: Tests +59. Total +59 across 1 file.

Reproducibility: not applicable. This PR does not report a broken user behavior; the review checked the current helper source, callers, and adjacent tests instead of reproducing a failure.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Next step before merge

[P2] No repair lane is needed because no concrete PR defect was found; the remaining decision is ordinary maintainer review of low-priority test coverage.

Security
Cleared: Security review cleared: the diff adds only a local Vitest test file and does not touch dependencies, workflows, scripts, lockfiles, secrets, or runtime execution paths.

Review details

Best possible solution:

If maintainers want the coverage, land only this narrow helper test file and do not reintroduce the mixed runtime/docs changes from the earlier closed branch.

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

Not applicable. This PR does not report a broken user behavior; the review checked the current helper source, callers, and adjacent tests instead of reproducing a failure.

Is this the best way to solve the issue?

Yes. For a test-only cleanup, a colocated pure-helper Vitest file is a narrow maintainable way to cover renderExecOutputText directly without expanding runtime imports.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P3: This is low-risk test-only coverage with no linked product bug, runtime behavior change, config surface, or upgrade impact.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied terminal output for a direct helper behavior probe and the targeted new Vitest file passing, which is sufficient for this test-only helper coverage change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output for a direct helper behavior probe and the targeted new Vitest file passing, which is sufficient for this test-only helper coverage change.

Evidence reviewed

PR surface:

Tests +59. Total +59 across 1 file.

View PR surface stats

Area	Files	Added	Net
Source	0	0	0
Tests	1	59	+59
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	1	59	+59

What I checked:

Live PR scope: Live GitHub metadata and diff show one added test file at head SHA 32026d2. (src/agents/bash-tools.exec-output.test.ts:1, 32026d2c4527)
Current helper behavior: Current main defines renderExecOutputText and renderExecUpdateText as pure formatting helpers, so a direct colocated unit test is an appropriate narrow coverage target. (src/agents/bash-tools.exec-output.ts:9, 0757cad597ec)
Runtime caller checked: The polling update path renders command tail text and warnings through renderExecUpdateText, matching the invariant covered by the new tests. (src/agents/bash-tools.exec-runtime.ts:679, 0757cad597ec)
Foreground caller checked: Foreground exec completion renders aggregated output through renderExecOutputText, so direct placeholder coverage has a real caller. (src/agents/bash-tools.exec.ts:206, 0757cad597ec)
Existing adjacent coverage: Current main already covers some renderExecUpdateText behavior in the broader runtime test file, but not the new direct helper test file from this PR. (src/agents/bash-tools.exec-runtime.test.ts:365, 0757cad597ec)
Current main not already implemented: The dedicated src/agents/bash-tools.exec-output.test.ts file is absent from current main, so this PR is not already implemented on main. (src/agents/bash-tools.exec-output.test.ts, 0757cad597ec)

Likely related people:

Josh Lehman: git blame attributes the current exec-output helper and adjacent renderExecUpdateText tests to fab0048; the local history is shallow/grafted, so this is a routing signal rather than full ownership proof. (role: recent area contributor; confidence: medium; commits: fab0048d7beb; files: src/agents/bash-tools.exec-output.ts, src/agents/bash-tools.exec-runtime.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.

openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 24, 2026

vincentkoc self-assigned this Jun 29, 2026

vincentkoc merged commit bf83ea1 into openclaw:main Jun 29, 2026
201 of 223 checks passed

solodmd mentioned this pull request Jun 29, 2026

test(firecrawl): add unit tests for firecrawl client functions #97714

Merged

vincentkoc mentioned this pull request Jul 6, 2026

chore(jobs): prune resolved OpenClaw inbox jobs openclaw/clownfish#221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test(agents): add unit tests for exec output rendering helpers#96448

test(agents): add unit tests for exec output rendering helpers#96448
vincentkoc merged 1 commit into
openclaw:mainfrom
dwc1997:test/exec-output-only

dwc1997 commented Jun 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

AI-assisted

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dwc1997 commented Jun 24, 2026 •

edited

Loading

clawsweeper Bot commented Jun 24, 2026 •

edited

Loading