Skip to content

fix(metadata): FilesystemLoader remembers what findFile resolved, so a repeated read makes no fs.access probe - #24621

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24537-findfile-memo
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24537-findfile-memo

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24537
Clause-②: no

What changed

FilesystemLoader (packages/metadata/src/loaders/filesystem-loader.ts) now remembers what findFile resolved for each (type, name). The answer is either the path or "no file". The memo belongs to one loader instance, so each kernel has its own. A repeated read of the same name makes no fs.access probe, whether a file holds the name or not.

  • The caller is plugin-security's catalogSetLoader (security-plugin.ts). It does not change.
  • The fix lands where the claim expected. The loader is the producer of the cost, and no other package changes.
  • Nothing public is added: no member, export or option. In the emitted dist/node.d.ts, FilesystemLoader gains only private members, and its public members are byte-identical.

Premise, measured before any change (origin/main 2667e24)

The probe counted calls to node:fs/promises.access along the real chain:

createSecurityCatalogReader().resolve('permission', name) → NodeMetadataManager.getDiagnosed → FilesystemLoader.load → findFile

It used the plugin's format set (yaml, json, typescript, javascript), which makes five resolvable extensions.

name fs.access per resolution, over 4 resolutions
no file holds it 5, 5, 5, 5
permission/hit_set.json holds it 2, 2, 2, 2 (load and stat each resolved it)

One narrowing of the card's reach: only names that the engine registry does not serve fall through to the metadata service. A set the registry holds never reaches this loader. That covers platform sets, the sets in package manifests, and environment-authored sets hydrated from sys_metadata.

The same probe after this change reads 5, 0, 0, 0, 0 for the miss and 1, 0, 0, 0, 0 for the hit.

What can change the answer, and what retires it

I enumerated every in-process path that touches ROOT/TYPE/:

path exists? how the memo is retired pinned by
FilesystemLoader.save(), the only in-process writer. It is reached through MetadataManager.save(), which no production code in this repo calls. yes Every entry is retired, in a finally, because a direct write can fail after creating the file. "a save through the loader retires a remembered miss"
FilesystemLoader.delete() does not exist n/a n/a
The file watcher on added / changed. This is NodeMetadataManager.handleFileEvent (chokidar), which runs only with watch: true. yes The handler already re-reads with useCache: false. That read now resolves the name again from disk and keeps the answer. "an added file event ..."
The file watcher on unlink yes The handler reads nothing. The next read finds the remembered path gone: stat reports ENOENT/ENOTDIR, or the probe that exists() makes to confirm the path fails. That read resolves the name once more from disk and logs no stat failure. "an unlink event ..."
The loader's own walk (list, loadMany, loadManyKeyed, all through resolvableNames) yes that type's entries the RECORD case
Artifact reload (HMR POST or the artifact watcher), and register() yes Not needed. Both write the manager's registry, which get / getDiagnosed ask before any loader. the two "writing a permission set" cases
The ADR-0008 FileSystemRepository It writes under ROOT/.objectstack/metadata, never under ROOT/TYPE/. n/a n/a

Two more guards:

  • Race. A probe can still be on disk when a retirement happens. An epoch counter stops it from writing its older answer back. Pinned by "a probe that was on disk ...".
  • Bound. Callers supply the names, so the memo is capped at 10,000 entries and starts over past that. Pinned by "is bounded ...".

The gap no event reaches

The gap is named here and deliberately not closed with a clock. The scenario:

  1. A name has already been answered "no file".
  2. Another process then creates ROOT/TYPE/NAME.ext.
  3. No watcher is running.

No watcher running is the normal case: every shipped composition passes watch: false. That covers createStandaloneStack, both of os serve's doors (the dev artifact door and the host-config door), and the verify harness.

  • On main today. get() sees the file on the next read, because findFile probes every time. list() sees it once the manager's 30 s list cache expires and the next list() walks.
  • After this PR, list(). Unchanged.
  • After this PR, get(). It keeps the remembered miss until one of three things happens: the next walk of that type (the next list() after the TTL is one), a save() through the loader, or a restart. A walk retires the type, so get() is never staler than list(). A name that list() reports is never answered null.
  • Edits to a file that already resolves. These are still read on the next request, because the content cache revalidates by etag. Pinned by the CONTROL case.
  • Deletions. These are covered by the unlink row above.

The gap is pinned as a RECORD: case, so any change to it shows up in a diff.

"Activating or writing a permission set is seen on the next request"

Neither of these goes through this loader:

  • Activation. POST /security/_activation/:type/:name (packages/runtime/src/domains/catalog-activation.ts) writes a sys_metadata_activation row. The resolver reads that ledger (readDisabledCatalogNames), not the catalog body, so the memo never sees it.
  • Writing a definition. saveMetaItem (the clone door, for example) puts the set in the engine registry through registerItem, and catalog.resolve asks that registry first. MetadataManager.register() puts it in the manager's registry, which getDiagnosed asks before any loader.

Two cases pin that a remembered miss hides neither write.

Tests

packages/metadata/src/loaders/filesystem-loader-resolution-memo.test.ts has 13 cases. They run real files in a tmpdir through the real NodeMetadataManager and the real createSecurityCatalogReader, and count probes at node:fs/promises.access.

  • probe count, miss: [5, 0, 0, 0, 0]
  • probe count, hit: [1, 0, 0, 0, 0], and the body is still served
  • CONTROL: an edit to a resolved file is read on the next resolution
  • two managers do not share what they remember
  • a set written to the engine registry is seen on the next resolution
  • a set registered with the metadata service is seen on the next resolution
  • a save through the loader retires a remembered miss
  • an added file event announces the body (not null), and the next resolution sees the file
  • an unlink event: load() and exists(), each asked first, answer the miss, and no stat failure is logged
  • a deletion no event reports is re-resolved from disk (json removed, yaml added → the yaml body)
  • RECORD: the gap above, closed by the next walk
  • a probe still on disk during a save does not write its miss back
  • the memo is bounded

Results on the final head (HEAD dff1e3f):

  • pnpm --filter @objectstack/metadata test: 64 files, 905 tests, all passed.
  • pnpm --filter @objectstack/metadata typecheck: exit 0. tsc --listFiles includes the new test file.

Ablation

There is one leg per mechanism. Each leg ran through scripts/ablation-replace.mjs in WRAP mode: the anchor hit once, the mutation was proven on disk by blob hash, and the restore was proven (blob equals HEAD, git diff HEAD empty). The test imports ./filesystem-loader.js relatively, so it reads source and no dist/ step applies.

leg mutation went red
memo if (remembered?.has(name)) → if (false && …) both probe counts, the RECORD case, the bound (4)
save retire this.retireResolutions(); → void 0; the save case, the late-probe case (2)
walk retire this.retireResolutions(type); → void type; the RECORD case (1)
fresh read resolveStats(type, name, !useCache) → false the added event case (1)
self-heal the ENOENT/ENOTDIR continue → if (false) the unlink case, the unreported-deletion case (2)
exists() confirmation the fresh fallback → return true the unlink case (1)
epoch if (epoch === this.resolutionEpoch) → if (true) the late-probe case (1)
bound the cap test → if (false) the bound case (1)

Gates

The local subset was derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack from the real diff, on the final head dff1e3f (merge base 5a871e4). It produced 64 commands, and I ran all of them.

  • Each exit code was captured before any pipe. All 64 exited 0.
  • --ran reconciliation: 64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN ("a DERIVED zero — all 64 recorded an exit code and none of them is 3").
  • An earlier run, before merging main (6fb0a2d), hit three prerequisite refusals (exit 3), all since cleared:
    • the plugin-teardown self-test could not reach its pinned fixture commit from the shallow clone;
    • check:lean-entry-closure needed @objectstack/objectql's built dist;
    • check:dual-build-cjs-loads needed built dist for every package.
      All three exit 0 on the rerun at dff1e3f. Its last line reads: 111 require entry points across 69 packages load.
  • After the merge I rebuilt @objectstack/spec. check:generated reported all 14 generated artifacts up to date.
  • Left to CI and read from this PR's check-runs, not re-run locally: the required contexts (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard). test:repo was not run locally either: the diff touches no file in that project.

Acceptance notes

  • Not filed, no owner yet. Artifact-mode runtimes still register this FilesystemLoader at rootDir. NodeMetadataManager installs it whenever no loaders are passed, and MetadataPlugin never passes any. Two pieces of documentation disagree with that: MetadataPluginOptions.watch says the source-file scan is redundant in artifact mode, and the bootstrap: 'artifact-only' branch says "Never touch the filesystem". The per-request probe this PR memoises exists only because that loader is registered. Removing it from artifact modes would be a composition change with its own blast radius. I took no public-door reading, so it is noted here rather than filed.
  • Observation. A name that a file does hold still costs one fs.stat and one readFile per read. That is the content cache's etag revalidation, and it is what keeps edits visible without a watcher. It only applies when a file exists under the root, so it is not the miss path this card measured.
  • Observation. FilesystemLoader.contract declares supportsWatch: true, but the loader has no watch of its own: NodeMetadataManager owns the chokidar watcher. That is why the watch retirement rides on the handler's existing useCache: false re-read and the read-time repair, and not on a new loader member.

Generated by Claude Code

Every read through the filesystem loader paid one fs.access per
resolvable extension (five under the plugin's format set), every time,
for a miss as much as a hit. The security catalog asks the metadata
service for every held permission set the engine registry does not
serve, on every authenticated request, so the probe ran per request.

The loader now remembers each resolution (path or null) per instance,
retired by what can change the answer: its own save, its own walk of a
type directory, a useCache:false read (the file watcher's re-read), and
a remembered path found gone on the next read. Bounded at 10,000
entries.

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
Counts fs.access under the catalog read for a miss and a hit (0 after the
first; main read 5 and 2 per resolution), and pins each retirement: the
loader's save, the watcher's added/unlink events, a deletion nothing
reported, the walk of a type, a probe already on disk when it retires,
and the memo's bound. RECORD: a file created with no watcher stays unseen
by a remembered miss until the next walk of its type.

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata, touching 12 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/kernel/metadata-service.mdx (via FilesystemLoader (symbol, a top-level class))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-3.mdx (via FilesystemLoader (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 5a871e4e41b110d5d4b0cdcc325a75723740bb31 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5434c04c6923f22762cf67f69e8601b06187ec1d — the merge of head dff1e3f7254bc56911b52c1a065e72675bd281e7 into base 5a871e4e41b110d5d4b0cdcc325a75723740bb31, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5434c04c6923f22762cf67f69e8601b06187ec1d && git checkout 5434c04c6923f22762cf67f69e8601b06187ec1d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a871e4e41b110d5d4b0cdcc325a75723740bb31 dff1e3f7254bc56911b52c1a065e72675bd281e7 && git checkout -B drift-repro 5a871e4e41b110d5d4b0cdcc325a75723740bb31 && git merge --no-ff dff1e3f7254bc56911b52c1a065e72675bd281e7

node scripts/docs-audit/affected-docs.mjs --json 5a871e4e41b110d5d4b0cdcc325a75723740bb31

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 5a871e4e41b110d5d4b0cdcc325a75723740bb31 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:engine seat 2 · session_01AHeUBbe6S3JRawcpbLejXD · 2026-10-19T16:41Z

Reviewed against GitHub on head dff1e3f725, not against the report.

  • Form: draft into main. The body's first line is Fixes #24537, and Clause-②: no sits at a line start. A closing-keyword scan of the whole body finds one hit, Fixes #24537.
  • Scope: 3 files, +615 / −51: filesystem-loader.ts, the new filesystem-loader-resolution-memo.test.ts and the changeset. All are inside claim 6270081689's file surface, and the caller (plugin-security) is untouched. Nothing under content/docs/releases/, and no governed path.
  • Clause-②: no holds on the diff. Every added member of FilesystemLoader is private (resolvedFiles, resolvedFileCount, resolutionEpoch, RESOLUTION_MEMO_CAP, resolveStats, probeFile, rememberResolution, forgetResolution, retireResolutions). findFile gains an optional parameter but stays private. No export or option is added.
  • Changeset (@objectstack/metadata patch), checked against the diff:
    • the four retirement bullets match the four retirement sites: save()'s finally, resolvableNames, the !useCache fresh read, and the ENOENT/ENOTDIR continue plus exists()'s confirmation probe;
    • "at most 10,000 names and then starts over" matches RESOLUTION_MEMO_CAP and the clear() in rememberResolution;
    • the one behaviour change is stated in it: a remembered miss is kept until the next walk, save() or restart, when another process creates the file with no watcher running.
  • Code read: resolveStats loops at most twice: a remembered path that stats ENOENT/ENOTDIR is re-resolved once from disk, and a second failure logs and returns null. The epoch guard stops a probe that was in flight during a retirement from writing back.
  • Pins: 13 cases on real files through the real NodeMetadataManager and createSecurityCatalogReader, counting node:fs/promises.access. Main measured [5,5,5,5] (miss) and [2,2,2,2] (hit); the fix pins [5,0,0,0,0] and [1,0,0,0,0]. The 8-leg ablation goes red one leg per mechanism, and the restore is proven.
  • The card's "activating or writing a permission set" clause: measured as not passing through this loader (the activation ledger, and the engine and manager registries are asked first). Two cases pin that a remembered miss hides neither. Accepted as the measured reading of that acceptance line.
  • Out-of-scope note (Acceptance notes, no carrier, not filed): artifact-mode runtimes still mount this loader at rootDir. No public-door reading was taken, so it stays a note.
  • Landing: not yet. At this reading 10 of the 39 check-runs on dff1e3f725 are in progress, and none is red. This seat flips the PR to ready and arms auto-merge once every check is green.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 16:50
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 23dae89 Oct 19, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24537-findfile-memo branch October 19, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants