Skip to content

fix(daemon): avoid loading full gateway logs during diagnostics#99407

Merged
steipete merged 3 commits into
openclaw:mainfrom
sunlit-deng:sunlit/fix/daemon-diagnostics-tail-read
Jul 3, 2026
Merged

fix(daemon): avoid loading full gateway logs during diagnostics#99407
steipete merged 3 commits into
openclaw:mainfrom
sunlit-deng:sunlit/fix/daemon-diagnostics-tail-read

Conversation

@sunlit-deng

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where gateway diagnostics could load the full stdout/stderr log files just to surface one recent error line. On hosts with large gateway logs, status, doctor, or onboarding failure diagnostics could allocate far more memory than needed while still reporting stale errors from the top of an old log.

Related: #79422

Why This Change Was Made

Gateway diagnostics now read only the last 256 KiB of each resolved log file before scanning for known error patterns or the last non-empty line. This is a read-side bound only: it does not change launchd/systemd logging, log retention, or the error-pattern set from other diagnostics PRs.

User Impact

Operators still get the most recent actionable gateway diagnostic line, but diagnostic commands no longer need to buffer an entire accumulated gateway log.

Evidence

  • node scripts/test-projects.mjs src/daemon/diagnostics.test.ts:
Test Files  1 passed (1)
     Tests  3 passed (3)
[test] passed 1 Vitest shard in 2.63s
  • pnpm exec oxlint src/daemon/diagnostics.ts src/daemon/diagnostics.test.ts: exit 0.
  • Local runtime proof using the production readLastGatewayErrorLine helper with a 440066-byte gateway log whose stale matching error is at the start and current line is in the tail:
{
  "size": 440066,
  "result": "gateway stdout current"
}

Overlap checked before opening: #79892 handles launchd write-side log retention, #90828 changes macOS stderr routing, and #95902 adds ENOSPC error matching. Those PRs do not bound the diagnostics read path.

AI-assisted: built with Codex

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 5:09 AM ET / 09:09 UTC.

Summary
The PR bounds gateway diagnostic log reads to a recent tail window and reuses that bounded reader for status-all gateway log summaries.

PR surface: Source +52, Tests +97. Total +149 across 4 files.

Reproducibility: yes. at source level: current main and v2026.6.11 fully read resolved gateway logs before selecting one diagnostic line, so a large log can force unnecessary buffering. I did not run tests because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Diagnostic tail limit: 1 limit added: 256 KiB per resolved log file. This is the maintainer-visible semantic change because diagnostics stop considering older log content outside the recent tail.

Stored data model
Persistent data-model change detected: serialized state: src/commands/status-all/gateway.ts, serialized state: src/daemon/diagnostics.ts. Confirm migration or upgrade compatibility proof before merge.

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

  • No repair lane is needed because there are no blocking review findings; normal maintainer review and CI can gate merge.

Security
Cleared: No concrete security or supply-chain issue found; the diff only changes bounded reads from existing local log files and adds tests.

Review details

Best possible solution:

Land the bounded shared diagnostics reader after ordinary maintainer review if the 256 KiB recent-tail window is the desired operator diagnostic behavior.

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

Yes at source level: current main and v2026.6.11 fully read resolved gateway logs before selecting one diagnostic line, so a large log can force unnecessary buffering. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes. Putting the bound in the shared daemon diagnostics helper is narrower than per-caller guards, and reusing it from status-all avoids a second full-read implementation.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR fixes a real gateway diagnostics memory and stale-error problem with limited daemon/status blast radius.
  • 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 after-fix terminal output from the production helper against a large seeded gateway log, plus targeted test and lint output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from the production helper against a large seeded gateway log, plus targeted test and lint output.
Evidence reviewed

PR surface:

Source +52, Tests +97. Total +149 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 73 21 +52
Tests 2 101 4 +97
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 174 25 +149

What I checked:

  • Repository policy read: Root AGENTS.md was found, read fully, and applied; no narrower AGENTS.md owns src/daemon or src/commands/status-all. (AGENTS.md:1, 1fef99962edf)
  • Current main fully buffers diagnostics logs: Current main reads the full stdout/stderr files with fs.readFile before selecting one diagnostic line. (src/daemon/diagnostics.ts:16, 1fef99962edf)
  • Latest release has the same behavior: The v2026.6.11 release tag still uses full-file fs.readFile in the gateway diagnostics helper. (src/daemon/diagnostics.ts:16, e085fa1a3ffd)
  • PR bounds the shared diagnostic read: The branch adds a 256 KiB tail window reader, drops partial first lines when the window begins mid-line, and replaces full stdout/stderr diagnostic reads with bounded line arrays. (src/daemon/diagnostics.ts:14, 4c1bd52c48f7)
  • Status-all reuses the bounded reader: The status-all gateway helper now delegates log-tail reads to readGatewayLogTailLines instead of reading the whole file itself. (src/commands/status-all/gateway.ts:10, 4c1bd52c48f7)
  • Regression coverage: The added tests cover stale errors outside the bounded tail, partial lines at the tail boundary, short positional reads, and status-all recent-line behavior. (src/daemon/diagnostics.test.ts:66, 4c1bd52c48f7)

Likely related people:

  • steipete: History shows Peter Steinberger introduced readLastGatewayErrorLine, status-all log-tail behavior, restart log conventions, and onboarding diagnostics around this gateway diagnostic surface; he also authored the latest maintainer follow-up commits on this branch. (role: feature-history owner and recent branch maintainer; confidence: high; commits: b367ed75bfb8, 1eb50ffac476, 28be124cc108; files: src/daemon/diagnostics.ts, src/daemon/restart-logs.ts, src/commands/status-all/gateway.ts)
  • vincentkoc: Recent release snapshot history touched the diagnostics and status files involved in this PR, so Vincent Koc is a secondary routing candidate for release-current behavior context. (role: recent release-area contributor; confidence: medium; commits: e085fa1a3ffd; files: src/daemon/diagnostics.ts, src/commands/status-all/gateway.ts, src/cli/daemon-cli/status.gather.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.

@sunlit-deng

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

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.

@steipete steipete self-assigned this Jul 3, 2026
@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. P2 Normal backlog priority with limited blast radius. labels Jul 3, 2026
@steipete
steipete force-pushed the sunlit/fix/daemon-diagnostics-tail-read branch from 9636319 to f17709c Compare July 3, 2026 08:54
@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S and removed size: XS labels Jul 3, 2026
@steipete
steipete force-pushed the sunlit/fix/daemon-diagnostics-tail-read branch from 4c1bd52 to 5f93789 Compare July 3, 2026 09:12
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Land-ready verification for exact head 5f93789740dcd4c2111d28a8f5598610a20225ea:

  • Gateway diagnostics and status --all now share one 256 KiB complete-line tail reader; the original PR’s remaining full-file status --all path is removed.
  • The reader drops a partial UTF-8/line prefix, fills legal short positional reads, retries once after concurrent size change, and preserves stderr priority.
  • Focused daemon/status tests — 21 passed, including forced three-byte reads.
  • Real source-built CLI proof against a 67,108,930-byte gateway log: before, status --all emitted a 67,113,083-byte report containing stale/filler content; after, it emitted 4,172 bytes containing only gateway stdout current in both diagnostic sections.
  • Two accepted autoreview findings were fixed; final local and full-branch structured autoreviews are clean.

No screenshot attached: the exact before/after artifact is terminal report size/content. Write-side log rotation remains separate. Thanks @sunlit-deng for the fix.

@steipete
steipete merged commit a9051b4 into openclaw:main Jul 3, 2026
96 checks passed
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
…claw#99407)

* fix(daemon): bound gateway diagnostic log reads

* fix(daemon): share bounded gateway log tails

* fix(daemon): complete bounded tail reads

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants