Skip to content

Refactor file access to use fs-safe primitives#78255

Merged
steipete merged 3 commits into
mainfrom
refactor/use-fs-safe-primitives
May 6, 2026
Merged

Refactor file access to use fs-safe primitives#78255
steipete merged 3 commits into
mainfrom
refactor/use-fs-safe-primitives

Conversation

@steipete

@steipete steipete commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace hand-rolled JSON reads/writes and path checks across core, plugins, and memory helpers with @openclaw/fs-safe-backed wrappers.
  • Route plugin/package metadata, Matrix recovery state, QQBot credential backup, browser profile decoration, provider usage, device identity/auth, and install inventory paths through shared fs-safe primitives.
  • Keep intentionally low-level cases local, including fd-pinned plugin bundle reads, transcript JSONL writes, and QA/report artifacts.

Verification

  • pnpm docs:list
  • pnpm exec oxfmt --check --threads=1
  • git diff --check
  • pnpm test src/infra/npm-managed-root.test.ts src/config/plugin-auto-enable.channels.test.ts src/cli/channel-options.test.ts
  • OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test src/config/plugin-auto-enable.prefer-over.test.ts src/plugins/sdk-alias.test.ts src/plugins/loader.test.ts src/plugins/bundled-plugin-metadata.test.ts src/plugins/bundled-capability-metadata.test.ts src/infra/provider-usage.shared.test.ts extensions/qqbot/src/engine/config/credential-backup.test.ts extensions/qqbot/src/engine/utils/platform-storage-laziness.test.ts extensions/browser/src/browser/chrome.internal.test.ts extensions/matrix/src/legacy-crypto.test.ts extensions/matrix/src/migration-snapshot.test.ts extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts src/commands/doctor-device-pairing.test.ts src/commands/doctor-plugin-registry.test.ts src/tts/status-config.test.ts src/utils/usage-format.test.ts
  • Testbox pnpm check:changed: https://github.com/openclaw/openclaw/actions/runs/25414705930
  • CI: https://github.com/openclaw/openclaw/actions/runs/25415594016

Real behavior proof

  • Behavior addressed: Refactored file-access paths still assemble and boot through the production OpenClaw CLI after moving the shared filesystem helpers to @openclaw/fs-safe.
  • Real environment tested: Local macOS checkout of openclaw/openclaw on commit 332724e, Node 22 via the repo pnpm wrapper.
  • Exact steps or command run after the patch: pnpm openclaw --help
  • Evidence after fix: Terminal output from the real CLI entrypoint included:
> openclaw@2026.5.5 openclaw /Users/steipete/Projects/clawdbot3
> node scripts/run-node.mjs --help

[openclaw] Building TypeScript (dist is stale: git_head_changed - git head changed).
runtime-postbuild: plugin SDK root alias completed in 1ms
runtime-postbuild: bundled plugin metadata completed in 116ms
runtime-postbuild: official channel catalog completed in 4ms
runtime-postbuild: bundled plugin runtime overlay completed in 247ms
runtime-postbuild: stable root runtime imports completed in 165ms
runtime-postbuild: stable root runtime aliases completed in 34ms
runtime-postbuild: legacy root runtime compat aliases completed in 7ms
runtime-postbuild: legacy CLI exit compat chunks completed in 0ms
runtime-postbuild: static extension assets completed in 9ms
[plugins] glueclaw: loaded 9 replacements (6 input, 3 output)

OpenClaw 2026.5.5 (332724e)
Usage: openclaw [options] [command]
Commands include: channels, config, doctor, gateway, memory, plugins, secrets, status
  • Observed result after fix: The production CLI built stale dist artifacts, loaded plugin metadata/replacements, printed the OpenClaw version and command tree, and exited successfully.
  • What was not tested: No live gateway start or external channel login was run for this filesystem refactor.

@steipete steipete requested a review from a team as a code owner May 6, 2026 03:39
@openclaw-barnacle openclaw-barnacle Bot added channel: matrix Channel integration: matrix channel: msteams Channel integration: msteams channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes commands Command implementations agents Agent runtime and tooling extensions: acpx channel: qqbot extensions: memory-wiki extensions: codex size: L maintainer Maintainer-authored PR labels May 6, 2026
@clawsweeper

clawsweeper Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR replaces many ad hoc JSON reads/writes and path checks across core, plugins, memory, channel, and gateway code with existing fs-safe, json-store, and privateFileStore primitives.

Reproducibility: not applicable. as a cleanup PR rather than a bug report. We do have high-confidence exact-head CI reproduction for the current blockers through the failing channel-options and plugin-auto-enable tests.

Real behavior proof
Needs real behavior proof before merge: The PR body lists formatter, tests, and CI only; the linked Testbox run is cancelled on main rather than after-fix proof for this PR head, and the Real behavior proof check failed. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Protected maintainer label and broad file-access refactor keep this in human maintainer review rather than an automated sweep repair lane.

Security
Cleared: No concrete security or supply-chain regression was found; the diff uses the existing pinned fs-safe/json helpers and does not add new dependencies, workflows, downloads, or secret access.

Review findings

  • [P2] Restore startup metadata coverage — src/cli/startup-metadata.ts:23
  • [P2] Restore catalog-read memoization coverage — src/config/plugin-auto-enable.prefer-over.ts:78
Review details

Best possible solution:

Land this only after maintainer review confirms the broad file-access boundary, the two CI test-contract blockers are repaired, exact-head checks are green, and representative real runtime proof is added.

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

Not applicable as a cleanup PR rather than a bug report. We do have high-confidence exact-head CI reproduction for the current blockers through the failing channel-options and plugin-auto-enable tests.

Is this the best way to solve the issue?

Unclear: centralizing on existing fs-safe primitives is a maintainable direction, but this patch is not merge-ready while exact-head CI and the real behavior proof gate are failing.

Full review comments:

  • [P2] Restore startup metadata coverage — src/cli/startup-metadata.ts:23
    This swap routes startup metadata reads through tryReadJsonSync, so the existing channel-options tests no longer inject cli-startup-metadata.json through their fs.readFileSync mock and now fall back to the static channel list. Please either preserve a testable read seam here or update the tests to exercise the new json-files path before merge.
    Confidence: 0.86
  • [P2] Restore catalog-read memoization coverage — src/config/plugin-auto-enable.prefer-over.ts:78
    After moving external catalog reads to tryReadJsonSync, the memoization test's fs.readFileSync spy observes zero catalog reads and the exact-head CI fails. Please move that assertion to the new json-files seam or otherwise update the test so the memoization contract remains covered.
    Confidence: 0.84

Overall correctness: patch is incorrect
Overall confidence: 0.86

What I checked:

  • Protected maintainer handling: GitHub API for the PR shows the protected maintainer label on the open PR, so sweep cleanup must not close it automatically. (12b5a7ea7a15)
  • Exact-head CI failures: Check runs for the PR head report failures in checks-node-core, checks-node-agentic-cli, checks-node-core-runtime-infra-process, and Real behavior proof. (12b5a7ea7a15)
  • Startup metadata test regression: CI annotations show resolveCliChannelOptions falls back to [quietchat, forum] instead of mocked startup metadata after readCliStartupMetadata switched to tryReadJsonSync. (src/cli/startup-metadata.ts:23, 12b5a7ea7a15)
  • Catalog memoization test regression: CI annotations show the external catalog memoization test now observes zero catalog reads after resolveExternalCatalogPreferOver switched to tryReadJsonSync. (src/config/plugin-auto-enable.prefer-over.ts:78, 12b5a7ea7a15)
  • Dependency contract checked: The published @openclaw/fs-safe@0.1.1 types show Root.stat returns boolean PathStat fields and Root.list(..., {withFileTypes:true}) returns DirEntry[], matching the PR's stat.isFile and entry.isDirectory usage. (src/infra/fs-safe.ts:2, b971ebaaab65)
  • Current-main ownership trail: Sampled central fs-safe wrappers and touched file-access paths blame/log to commit 9e108fa, which introduced the current fs-safe sweep baseline on main. (src/infra/json-files.ts:1, 9e108fa9a762)

Likely related people:

  • steipete: Current-main blame/log for the sampled fs-safe wrappers and central touched paths point to Peter Steinberger's fs-safe baseline commit, and the PR branch is also authored by this handle. (role: recent maintainer; confidence: high; commits: 9e108fa9a762, 12b5a7ea7a15; files: src/infra/json-files.ts, src/plugin-sdk/json-store.ts, src/gateway/server-methods/agents.ts)

Remaining risk / open question:

  • The PR head still has failing node-core, agentic CLI, and runtime/infra CI lanes that need exact-log follow-up before merge.
  • The PR body provides tests and CI, but no after-fix real OpenClaw runtime proof for representative changed file-access paths.

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

Re-review progress:

@steipete steipete merged commit b85b1c6 into main May 6, 2026
122 of 126 checks passed
@steipete steipete deleted the refactor/use-fs-safe-primitives branch May 6, 2026 04:03
@steipete

steipete commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Landed in b85b1c6. Pre-merge CI and real behavior proof were green on 332724e.

@THookz

THookz commented May 6, 2026

Copy link
Copy Markdown

Have we recently been pulling any from skills, anti-patterns,and other tools from Impeccable for frontend and other ux/ui capabilities [adding more ease for if we ever want more bandwidth used else where and can pull from these repositories for the Impeccable design skills and this robust ai-memory scoring system could be used in tandem, potentially in sandbox if not yet deployed mainnet. These might help as a resources. I'm sure we already have a 100,000 foot view, but sometimes these simple tools in the 10,000 - 1,000 foot utility view get lost. It's always good to remind our AI friends which foot has peddled in the direction before them, and remind them of how they might keep memory of such simple day-to-day insights..not-forsaking the abilities at our disposal for creating hierarchical memory, recourses and tasks to be done that will cost some certain amount of tokens but can potentially be offset (not a promise) through the use of tools like OpenViking and the use of premade and frameworks which have been proved to be worthy to be brought upstream to the Agency Agents Repo (though I AM not specifically, impartially or any other to the contrary, thereof, suggesting the use of agency agents as a base_model or main reference...but rather creating a way to eat less tokens and create a running-in-tandem agent to help with logic-optimization, NLP nuances, tools calls, fragmentation overload, and other common agentic bottlenecks...NOTWITHSTANDING ANYTHING TO THE CONTRARY, THEREOF.) this is merely theoretical. If code can be presumed as the cause for the effects of ideas..then the actions of agents can not be unequivocally connected to assumed human counterparts, just as a program that runs and executes autonomously cannot without a shadow of a doubt be considered dependent upon the frame and intention of work, thereof, notwithstanding any laws and precedents of the jurisdiction in which intellectual property is being operated, and/or otherwise used for its various purposes. IP can be owned by Agents if so assumed by the operators of the highest Order (and/or otherwise,) so the actions taken by autonomous ("artificial intelligent") creations cannot be held, insinuated, held in contempt and/or conspiracy as acting in tandem accountability, not forsaking all intentions and actions of those human counterparts working in conjunction, asynchronously, agnostic and or otherwise together with autonomous working from its own code and not basing each action off equivalent human imposed intention.) here is an Agency Agents repo for reference 😃

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
* refactor: use fs-safe primitives across file access

* fix: preserve invalid managed npm manifests

* fix: keep fs seams for startup metadata
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
* refactor: use fs-safe primitives across file access

* fix: preserve invalid managed npm manifests

* fix: keep fs seams for startup metadata
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
* refactor: use fs-safe primitives across file access

* fix: preserve invalid managed npm manifests

* fix: keep fs seams for startup metadata
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
* refactor: use fs-safe primitives across file access

* fix: preserve invalid managed npm manifests

* fix: keep fs seams for startup metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling channel: matrix Channel integration: matrix channel: msteams Channel integration: msteams channel: qqbot channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call cli CLI command changes commands Command implementations extensions: acpx extensions: codex extensions: memory-core Extension: memory-core extensions: memory-wiki gateway Gateway runtime maintainer Maintainer-authored PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants