Skip to content

fix: quiet missing daily memory reads#82929

Merged
galiniliev merged 2 commits into
openclaw:mainfrom
galiniliev:bug-010-missing-daily-memory-read
May 25, 2026
Merged

fix: quiet missing daily memory reads#82929
galiniliev merged 2 commits into
openclaw:mainfrom
galiniliev:bug-010-missing-daily-memory-read

Conversation

@galiniliev

@galiniliev galiniliev commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: optional date-only daily memory reads can fail with ENOENT before the file exists and get logged as scary tool failures.
  • Why it matters: the missing file is harmless first-run state, but the raw tool error pollutes logs and can confuse agent/user flow.
  • What changed: the OpenClaw read wrapper now converts missing memory/YYYY-MM-DD.md reads into structured optional not-found context.
  • What did NOT change (scope boundary): ordinary missing read paths still throw, and non-ENOENT daily memory failures still propagate.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Real behavior proof (required for external PRs)

  • Behavior addressed: Missing date-only daily memory reads now return optional not-found context instead of throwing through the generic read tool failure path.
  • Real environment tested: Windows worktree source checkout; local Vitest/Testbox/AWS proof was attempted but blocked by environment/tooling listed below.
  • Exact steps or command run after this patch: git diff --check from branch bug-010-missing-daily-memory-read after the patch.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Terminal capture:
> git diff --check
<no output; exit 0>

Before evidence from the redacted bug log:

[tools] read failed: ENOENT: no such file or directory, access '[redacted workspace path]/memory/2026-05-15.md' raw_params={"path":"memory/2026-05-15.md"}
  • Observed result after fix: The patch changes executeReadPage so a not-found error for canonical memory/YYYY-MM-DD.md returns details: { status: "not_found", optional: true } with a non-error text result; the added regression test asserts that ordinary missing files still throw.
  • What was not tested: Vitest did not complete locally because node scripts/run-vitest.mjs src/agents/pi-tools.workspace-only-false.test.ts failed with Cannot find module 'vitest/package.json' before install; pnpm install failed twice in esbuild postinstall with spawnSync C:\Program Files\nodejs\node.exe EPERM; direct node node_modules\vitest\vitest.mjs run src/agents/pi-tools.workspace-only-false.test.ts --reporter=verbose and the one-test retry timed out; Testbox-through-Crabbox was blocked by missing blacksmith CLI; direct AWS Crabbox was blocked by missing AWS/broker credentials.
  • Before evidence (optional but encouraged): Included above.

Root Cause (if applicable)

  • Root cause: createOpenClawReadTool delegated all missing read failures to the upstream read tool, so optional daily memory misses propagated as generic tool errors and were logged by toToolDefinitions.
  • Missing detection / guardrail: There was no special case for first-run canonical daily memory files, and no regression coverage ensuring ordinary missing reads still fail.
  • Contributing context (if known): Daily memory paths are expected to be created lazily, so reading today’s file can race ahead of the first write.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/agents/pi-tools.workspace-only-false.test.ts
  • Scenario the test should lock in: Missing memory/YYYY-MM-DD.md returns structured optional not-found context, while notes/missing.md still throws.
  • Why this is the smallest reliable guardrail: It exercises the OpenClaw read wrapper seam where the upstream read error is converted before the generic tool adapter logs failures.
  • Existing test that already covers this (if any): None found.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

Reading a missing canonical daily memory file now reports that the file does not exist yet instead of surfacing a generic read failure.

Diagram (if applicable)

Before:
read(memory/YYYY-MM-DD.md) -> ENOENT throw -> [tools] read failed log

After:
read(memory/YYYY-MM-DD.md) -> optional not_found result -> no generic tool failure log

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: Windows worktree for source changes; original redacted evidence came from a gateway-dev log.
  • Runtime/container: Node local tooling was present but local test execution was blocked as described above.
  • Model/provider: NOT_ENOUGH_INFO
  • Integration/channel (if any): Agent tool execution / daily memory read path.
  • Relevant config (redacted): Read path shaped as memory/2026-05-15.md.

Steps

  1. Invoke the read tool for a missing canonical date-only daily memory file.
  2. Observe pre-fix generic read failed: ENOENT logging.
  3. Apply this patch and invoke the same read path.

Expected

  • Missing daily memory returns optional not-found context without generic tool failure logging.

Actual

  • Pre-fix evidence showed generic read failed: ENOENT log noise.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: inspected upstream @earendil-works/pi-coding-agent@0.74.1 read implementation; it calls ops.access(absolutePath) and rejects access/read errors, which are then logged by OpenClaw’s generic tool adapter. Added focused regression coverage at the OpenClaw wrapper seam.
  • Edge cases checked: non-ENOENT failures still throw; non-daily-memory missing paths still throw; offset-beyond-EOF behavior remains unchanged.
  • What you did not verify: local/remote Vitest execution and live gateway rerun, due to the environment/tooling blockers listed in the Real behavior proof section.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: accidentally hiding real read failures.
    • Mitigation: the conversion only applies to canonical memory/YYYY-MM-DD.md paths and only for not-found errors; ordinary missing files and other failures still throw.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels May 17, 2026
@clawsweeper

clawsweeper Bot commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 10:18 AM ET / 14:18 UTC.

Summary
The PR adds a daily-memory missing-file special case in the OpenClaw read wrapper, two regression tests, and a changelog entry.

PR surface: Source +39, Tests +34, Docs +1. Total +74 across 3 files.

Reproducibility: yes. at source level: the pinned upstream read tool rejects missing files during access, and OpenClaw's tool-definition adapter logs thrown tool errors. I did not run a live current-main reproduction in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Add redacted after-fix proof showing read on a missing memory/YYYY-MM-DD.md returns optional not-found context instead of a generic failure.
  • Include focused test or Testbox output for src/agents/pi-tools.workspace-only-false.test.ts when available.

Proof guidance:
Needs stronger real behavior proof before merge: The PR body has a before log and git diff --check, but needs redacted after-fix runtime output, terminal output, logs, a recording, or a linked artifact showing the changed behavior; updating the PR body should trigger a fresh ClawSweeper review, or a maintainer can request @clawsweeper 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

  • The remaining merge gate is proof: the PR body still lacks after-fix runtime, test, or redacted log output showing the missing daily-memory read returns optional not-found context instead of the generic failure path.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused wrapper and regression-test change after maintainer review and after-fix behavior proof confirms the daily-memory miss path and the ordinary missing-file failure path.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
Manual handling is appropriate because the PR has a protected maintainer label and needs contributor or maintainer after-fix behavior proof rather than an automated code repair.

Security
Cleared: The diff only changes read-tool error normalization, tests, and changelog text; it adds no dependency, network, permission, credential, or code-execution surface.

Review details

Best possible solution:

Land the focused wrapper and regression-test change after maintainer review and after-fix behavior proof confirms the daily-memory miss path and the ordinary missing-file failure path.

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

Yes, at source level: the pinned upstream read tool rejects missing files during access, and OpenClaw's tool-definition adapter logs thrown tool errors. I did not run a live current-main reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed fix targets the OpenClaw read-wrapper seam and preserves ordinary missing-read failures. The missing piece is after-fix behavior proof, not an obvious code-design blocker.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against a3ae5c83821c.

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body has a before log and git diff --check, but needs redacted after-fix runtime output, terminal output, logs, a recording, or a linked artifact showing the changed behavior; updating the PR body should trigger a fresh ClawSweeper review, or a maintainer can request @clawsweeper 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.

Label justifications:

  • P3: This is a low-severity log-noise bug fix for optional first-run daily memory reads with limited blast radius.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body has a before log and git diff --check, but needs redacted after-fix runtime output, terminal output, logs, a recording, or a linked artifact showing the changed behavior; updating the PR body should trigger a fresh ClawSweeper review, or a maintainer can request @clawsweeper 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 +39, Tests +34, Docs +1. Total +74 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 39 0 +39
Tests 1 34 0 +34
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 74 0 +74

Acceptance criteria:

  • node scripts/run-vitest.mjs src/agents/pi-tools.workspace-only-false.test.ts
  • Redacted runtime or Testbox proof invoking read for a missing memory/YYYY-MM-DD.md and an ordinary missing file

What I checked:

  • Current main read behavior: Current main's executeReadPage only converts offset-beyond-EOF errors to an empty read result and otherwise rethrows read errors, so missing daily memory files still reach the generic failure path without this PR. (src/agents/pi-tools.read.ts:227, a3ae5c83821c)
  • Generic tool-failure logging path: toToolDefinitions catches thrown tool errors, logs [tools] <name> failed with raw/effective params, and returns an error result, matching the linked issue's redacted ENOENT log shape. (src/agents/pi-tool-definition-adapter.ts:370, a3ae5c83821c)
  • Dependency contract: The repo pins @earendil-works/pi-coding-agent 0.75.4, and the upstream read tool calls ops.access(absolutePath) before reading and rejects access/read errors. (package.json:1808, a3ae5c83821c)
  • PR diff scope: The patch adds a canonical memory/YYYY-MM-DD.md matcher, returns { status: "not_found", optional: true } only for not-found errors on that path, and adds coverage proving ordinary missing reads still throw. (src/agents/pi-tools.read.ts:60, e592f2b0d3d5)
  • Proof gate: The PR body currently provides a before log and git diff --check, but no after-fix runtime output, focused test output, recording, or redacted log showing the changed read behavior. (e592f2b0d3d5)
  • History provenance: Recent history ties this read-wrapper/test surface to Peter Steinberger's current-main file work, Tyler Yust's read-tool hardening, and Frank Yang's adjacent memory-flush tool work. (src/agents/pi-tools.read.ts:221, 087dca8fa9f5)

Likely related people:

  • steipete: Current-main history and blame show Peter Steinberger recently carried the read-wrapper and adjacent workspace-only test files through broad TypeScript/test-matrix work. (role: recent area contributor; confidence: medium; commits: bbc1772f4d29, a374c3a5bfd5; files: src/agents/pi-tools.read.ts, src/agents/pi-tools.workspace-only-false.test.ts)
  • tyler6204: The read-wrapper's adaptive paging and overflow handling history is tied to the read-tool hardening commit in this same file. (role: read-wrapper feature contributor; confidence: medium; commits: 087dca8fa9f5; files: src/agents/pi-tools.read.ts)
  • frankekn: Adjacent memory-flush read/write behavior and tests in this file were added in the memory flush protection commit. (role: adjacent memory tool contributor; confidence: medium; commits: 96e4975922de; files: src/agents/pi-tools.read.ts, src/agents/pi-tools.workspace-only-false.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.

@clawsweeper clawsweeper Bot added the P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. label May 17, 2026
@clawsweeper clawsweeper Bot mentioned this pull request May 17, 2026
Closed
@galiniliev galiniliev self-assigned this May 25, 2026
@galiniliev galiniliev force-pushed the bug-010-missing-daily-memory-read branch 2 times, most recently from 6a8909e to deb9f79 Compare May 25, 2026 14:08
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@galiniliev galiniliev force-pushed the bug-010-missing-daily-memory-read branch from e592f2b to 937eac4 Compare May 25, 2026 14:32
@galiniliev

galiniliev commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 25, 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:

@galiniliev

galiniliev commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Pre-merge verification for head 937eac4:

Exact local commands run after this patch:

pnpm test src/agents/model-catalog-visibility.test.ts src/agents/pi-tools.workspace-only-false.test.ts -- --reporter=verbose
git diff --check upstream/main...HEAD

Local result:

Test Files  2 passed (2)
Tests       14 passed (14)
[test] passed 1 Vitest shard in 22.86s
git diff --check upstream/main...HEAD: no output, exit 0

CI proof:

CI run: https://github.com/openclaw/openclaw/actions/runs/26405705091
Real behavior proof: https://github.com/openclaw/openclaw/actions/runs/26405704131/job/77728407856
Critical Quality network-runtime-boundary: https://github.com/openclaw/openclaw/actions/runs/26405705045/job/77728428942
CodeQL Security High: https://github.com/openclaw/openclaw/actions/runs/26405705089
OpenGrep PR diff: https://github.com/openclaw/openclaw/actions/runs/26405705087/job/77728410560

Observed result after fix:

The PR regression test covers read(memory/YYYY-MM-DD.md) returning optional not_found context and verifies ordinary missing paths still throw. The previous CI failure in checks-node-agentic-agents is green on this head.

What was not tested:

No live Gateway session was run for this landing; validation is focused local Vitest proof plus the PR real-behavior proof workflow and full selected CI matrix.

@galiniliev galiniliev merged commit 277d8fe into openclaw:main May 25, 2026
98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@galiniliev galiniliev

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Missing daily memory file read logs ENOENT noise

1 participant

@galiniliev