Skip to content

fix(daemon): prefer stderr over stale stdout in gateway restart diagnostics#93001

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
Alix-007:fix/daemon-diagnostics-stderr
Jun 15, 2026
Merged

fix(daemon): prefer stderr over stale stdout in gateway restart diagnostics#93001
vincentkoc merged 1 commit into
openclaw:mainfrom
Alix-007:fix/daemon-diagnostics-stderr

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

readLastGatewayErrorLine (src/daemon/diagnostics.ts) surfaces the most relevant gateway error line for daemon restart diagnostics (shown in status / doctor). On non-darwin platforms it reads both stderr and stdout, builds [...stderrRaw.split, ...stdoutRaw.split], then scans from the end. Because stdout is appended last, scanning backwards walks the entire stdout block before reaching any stderr line — so any (even stale) stdout match wins over a newer stderr match. This contradicts the function's own comment ("stderr [is] the strongest failure signal" on non-darwin) and its stderr-first fallback. The existing test only covered the darwin branch (stderr suppressed), so the linux/win32 two-log merge path was uncovered.

What changed

  • Place stderr last in the combined list ([...stdoutRaw.split, ...stderrRaw.split]) so the backward scan reaches the most recent stderr error first, with stdout as the fallback — matching the comment and the stderr-first fallback below. The darwin (stderr-suppressed) behavior is unchanged.

Real behavior proof

  • Behavior or issue addressed: on linux/win32, a stale matching line in gateway stdout was returned as the restart "error reason" instead of the newer real failure in stderr, pointing status/doctor users at an outdated cause.
  • Real environment tested: local OpenClaw from source on Linux. Seeded real gateway service logs at the paths resolveGatewayLogPaths(env) resolves to ($STATE/logs/gateway.log = stdout with a stale matching line; $STATE/logs/gateway.err.log = stderr with the newer real failure), then drove the real exported readLastGatewayErrorLine via node --import tsx — no mocks; the production function reads the on-disk logs. This returns the exact value status prints as Last gateway error: (status.gather.ts:709 → status.print.ts:440) and doctor prints via note("Last gateway error: …") (doctor-gateway-daemon-flow.ts:290).
  • Exact steps or command run after this patch: 
    # seed real gateway logs at the resolved paths
printf '...[error] refusing to bind gateway: stale stdout from a previous failed start\n' > "$STATE/logs/gateway.log"
printf '...[error] failed to bind gateway socket: address already in use (the real current failure)\n' > "$STATE/logs/gateway.err.log"
# drive the real production diagnostics function against the on-disk logs
OPENCLAW_STATE_DIR=$STATE node --import tsx -e \
  'import {readLastGatewayErrorLine} from "./src/daemon/diagnostics.ts"; console.log("Last gateway error: " + await readLastGatewayErrorLine(process.env,{platform:"linux"}))'
  • Evidence after fix (terminal capture): 
    Last gateway error: 2026-06-14T12:00:00Z [error] failed to bind gateway socket: address already in use (the real current failure)
    i.e. the stderr line (the real current failure) — exactly what status / doctor would print.
  • Observed result after fix: with both logs containing a matching error pattern, the surfaced reason is the newer stderr line, not the stale stdout line. The Vitest unit suite (src/daemon/diagnostics.test.ts) also asserts this for linux while keeping the darwin stderr-suppressed case.
  • What was not tested: spawning the full openclaw daemon status CLI process end-to-end — in this isolated sandbox a from-scratch CLI build is blocked by native postinstall binaries (e.g. @matrix-org/matrix-sdk-crypto-nodejs, node-llama-cpp) downloaded from GitHub Releases timing out. The proof instead drives the exact production function those commands call, against real on-disk logs (real runtime, not a test double).
  • Negative control: reverting diagnostics.ts to the pre-fix [...stderr, ...stdout] order (git checkout 357000c8^ -- src/daemon/diagnostics.ts) and re-running the same command prints the stale stdout line instead: 
    Last gateway error: 2026-06-10T00:00:01Z [error] refusing to bind gateway: stale stdout from a previous failed start
    confirming the fix is required and the proof is not a tautology. The Vitest suite likewise fails on the linux assertion when reverted.

Tests and validation

  • node --import tsx driving the real readLastGatewayErrorLine on seeded on-disk gateway logs → returns the stderr line (after fix) / the stale stdout line (pre-fix negative control).
  • node scripts/run-vitest.mjs src/daemon/diagnostics.test.ts → 2 passed (added a linux case asserting stderr wins over a stale stdout match; the existing darwin case still passes). The non-darwin two-log path previously had no coverage.
  • oxfmt --write clean on both changed files.

Risk checklist

  • User-visible behavior change? Yes (diagnostics only) — status/doctor now report the stderr failure reason on non-darwin instead of a possibly stale stdout line.
  • Config/env/migration change? No.
  • Security/auth/secrets/network/tool-execution change? No.
  • Highest-risk area: regressing the darwin (stderr-suppressed) path; covered by the retained darwin test.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 14, 2026, 10:07 PM ET / 02:07 UTC.

Summary
The PR changes daemon gateway diagnostics so non-Darwin restart error selection prefers stderr matches over stale stdout matches and adds a Linux regression test.

PR surface: Source +3, Tests +17. Total +20 across 2 files.

Reproducibility: yes. Source inspection gives a high-confidence reproduction path: seed matching stdout and stderr gateway logs on Linux and call readLastGatewayErrorLine; current main checks stdout first because it is appended after stderr and scanned from the end.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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.

Rank-up moves:

  • none.

Next step before merge

  • No ClawSweeper repair work is needed; the remaining path is normal maintainer review and merge handling for the contributor PR.

Security
Cleared: The diff only changes local diagnostics log ordering and a focused test; it does not touch secrets, auth, dependencies, workflows, downloads, or package execution surfaces.

Review details

Best possible solution:

Land the focused helper-order change after ordinary maintainer review, keeping readLastGatewayErrorLine as the single source for status, doctor, onboard, and status-all report diagnostics.

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

Yes. Source inspection gives a high-confidence reproduction path: seed matching stdout and stderr gateway logs on Linux and call readLastGatewayErrorLine; current main checks stdout first because it is appended after stderr and scanned from the end.

Is this the best way to solve the issue?

Yes. The helper owns the selection that all visible callers print, so reversing the combined log order is the narrow maintainable fix; timestamp parsing would be a larger contract change than the current stderr-priority behavior needs.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body/comment include terminal output from seeding resolved gateway log files and invoking the production helper via node --import tsx, plus a negative control; this sufficiently shows the after-fix stderr selection even though it is not a full CLI process run.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 updated PR body/comment include terminal output from seeding resolved gateway log files and invoking the production helper via node --import tsx, plus a negative control; this sufficiently shows the after-fix stderr selection even though it is not a full CLI process run.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a normal-priority daemon diagnostics bug with limited blast radius: it affects reported restart error text, not runtime availability or data integrity.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 updated PR body/comment include terminal output from seeding resolved gateway log files and invoking the production helper via node --import tsx, plus a negative control; this sufficiently shows the after-fix stderr selection even though it is not a full CLI process run.
  • proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body/comment include terminal output from seeding resolved gateway log files and invoking the production helper via node --import tsx, plus a negative control; this sufficiently shows the after-fix stderr selection even though it is not a full CLI process run.
Evidence reviewed

PR surface:

Source +3, Tests +17. Total +20 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 1 +3
Tests 1 17 0 +17
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 21 1 +20

What I checked:

  • PR diff: The patch changes readLastGatewayErrorLine to combine stdout before stderr so the reverse scan reaches stderr matches first, and adds a Linux test for stale stdout losing to stderr. (src/daemon/diagnostics.ts:43, 357000c80525)
  • Current main behavior: Current main says stderr is the strongest non-Darwin failure signal, but builds [...stderr, ...stdout] and scans backward, so a matching stdout line is checked before any stderr line. (src/daemon/diagnostics.ts:35, 44e6caff5401)
  • Status caller: daemon status stores readLastGatewayErrorLine as lastError, and the print path displays it as Last gateway error, so the helper order directly affects user-visible diagnostics. (src/cli/daemon-cli/status.gather.ts:709, 44e6caff5401)
  • Doctor and onboard callers: Doctor and non-interactive onboard both use the same helper before printing Last gateway error, so fixing the shared helper is the right boundary instead of patching callers separately. (src/commands/doctor-gateway-daemon-flow.ts:290, 44e6caff5401)
  • Sibling diagnostics surface: status-all separately tails stderr and stdout, while report data also calls readLastGatewayErrorLine; the PR does not need a sibling change for the one-line selection invariant. (src/commands/status-all/diagnosis.ts:401, 44e6caff5401)
  • Regression coverage: The existing test covered the Darwin stderr-suppressed path; the PR adds the missing Linux two-log case where stderr must beat stale stdout. (src/daemon/diagnostics.test.ts:23, 44e6caff5401)

Likely related people:

  • vincentkoc: Current-main blame for readLastGatewayErrorLine and the daemon status/doctor call sites points to recent daemon diagnostics import-boundary work. (role: recent area contributor; confidence: medium; commits: fc6d448138fc; files: src/daemon/diagnostics.ts, src/cli/daemon-cli/status.gather.ts, src/commands/doctor-gateway-daemon-flow.ts)
  • steipete: Git history shows earlier restart-log convention, gateway command-env status, and scheduled restart diagnostics work around the same user-visible daemon diagnostic surfaces. (role: adjacent diagnostics contributor; confidence: medium; commits: 28be124cc108, 9747da868240, b31836317a47; files: src/daemon/restart-logs.ts, src/cli/daemon-cli/status.gather.ts, src/commands/doctor-gateway-daemon-flow.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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 14, 2026
@Alix-007

Alix-007 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Strengthened the proof to a real-runtime run (beyond the Vitest unit case): seeded real gateway logs at the resolveGatewayLogPaths paths and drove the production readLastGatewayErrorLine via node --import tsx — the exact value status (status.gather.ts:709→status.print.ts:440) and doctor print as Last gateway error:.

After fix it returns the stderr line (failed to bind gateway socket: address already in use); negative control (revert to the pre-fix [...stderr, ...stdout] order) returns the stale stdout line (refusing to bind gateway: stale stdout). Full daemon status CLI build is blocked in this isolated sandbox by native postinstall binaries (@matrix-org/matrix-sdk-crypto-nodejs, node-llama-cpp) timing out from GitHub Releases, so the proof drives the exact production function those commands call against real on-disk logs.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 15, 2026
@vincentkoc vincentkoc self-assigned this Jun 15, 2026
@vincentkoc
vincentkoc merged commit f6a3ac7 into openclaw:main Jun 15, 2026
238 of 252 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 16, 2026
@Alix-007
fix(daemon): prefer stderr over stale stdout in gateway restart diagn…
99f0fc4 
…ostics

Refs openclaw#93001.
crh-code pushed a commit to crh-code/openclaw that referenced this pull request Jun 18, 2026
@Alix-007 @crh-code
fix(daemon): prefer stderr over stale stdout in gateway restart diagn…
fde19b8 
…ostics

Refs openclaw#93001.
@clawsweeper clawsweeper Bot mentioned this pull request Jun 23, 2026
Merged
badgerbees pushed a commit to badgerbees/openclaw that referenced this pull request Jul 8, 2026
@Alix-007 @badgerbees
fix(daemon): prefer stderr over stale stdout in gateway restart diagn…
7d8ef1e 
…ostics

Refs openclaw#93001.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alix-007 @vincentkoc