Repository navigation
fix(metadata): FilesystemLoader remembers what findFile resolved, so a repeated read makes no fs.access probe - #24621
Conversation
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>
Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
…ngs as measured Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
席位核对 · ACCEPT ·
|
Fixes #24537
Clause-②: no
What changed
FilesystemLoader(packages/metadata/src/loaders/filesystem-loader.ts) now remembers whatfindFileresolved 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 nofs.accessprobe, whether a file holds the name or not.plugin-security'scatalogSetLoader(security-plugin.ts). It does not change.dist/node.d.ts,FilesystemLoadergains onlyprivatemembers, and its public members are byte-identical.Premise, measured before any change (
origin/main2667e24)The probe counted calls to
node:fs/promises.accessalong the real chain:createSecurityCatalogReader().resolve('permission', name)→NodeMetadataManager.getDiagnosed→FilesystemLoader.load→findFileIt used the plugin's format set (yaml, json, typescript, javascript), which makes five resolvable extensions.
fs.accessper resolution, over 4 resolutionspermission/hit_set.jsonholds itloadandstateach 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, 0for the miss and1, 0, 0, 0, 0for the hit.What can change the answer, and what retires it
I enumerated every in-process path that touches
ROOT/TYPE/:FilesystemLoader.save(), the only in-process writer. It is reached throughMetadataManager.save(), which no production code in this repo calls.finally, because a direct write can fail after creating the file.FilesystemLoader.delete()added/changed. This isNodeMetadataManager.handleFileEvent(chokidar), which runs only withwatch: true.useCache: false. That read now resolves the name again from disk and keeps the answer.addedfile event ..."unlinkstatreports ENOENT/ENOTDIR, or the probe thatexists()makes to confirm the path fails. That read resolves the name once more from disk and logs no stat failure.unlinkevent ..."list,loadMany,loadManyKeyed, all throughresolvableNames)register()get/getDiagnosedask before any loader.FileSystemRepositoryROOT/.objectstack/metadata, never underROOT/TYPE/.Two more guards:
The gap no event reaches
The gap is named here and deliberately not closed with a clock. The scenario:
ROOT/TYPE/NAME.ext.No watcher running is the normal case: every shipped composition passes
watch: false. That coverscreateStandaloneStack, both ofos serve's doors (the dev artifact door and the host-config door), and the verify harness.get()sees the file on the next read, becausefindFileprobes every time.list()sees it once the manager's 30 s list cache expires and the nextlist()walks.list(). Unchanged.get(). It keeps the remembered miss until one of three things happens: the next walk of that type (the nextlist()after the TTL is one), asave()through the loader, or a restart. A walk retires the type, soget()is never staler thanlist(). A name thatlist()reports is never answerednull.unlinkrow 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:
POST /security/_activation/:type/:name(packages/runtime/src/domains/catalog-activation.ts) writes asys_metadata_activationrow. The resolver reads that ledger (readDisabledCatalogNames), not the catalog body, so the memo never sees it.saveMetaItem(the clone door, for example) puts the set in the engine registry throughregisterItem, andcatalog.resolveasks that registry first.MetadataManager.register()puts it in the manager's registry, whichgetDiagnosedasks before any loader.Two cases pin that a remembered miss hides neither write.
Tests
packages/metadata/src/loaders/filesystem-loader-resolution-memo.test.tshas 13 cases. They run real files in a tmpdir through the realNodeMetadataManagerand the realcreateSecurityCatalogReader, and count probes atnode:fs/promises.access.[5, 0, 0, 0, 0][1, 0, 0, 0, 0], and the body is still servedaddedfile event announces the body (notnull), and the next resolution sees the fileunlinkevent:load()andexists(), each asked first, answer the miss, and no stat failure is loggedResults 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 --listFilesincludes the new test file.Ablation
There is one leg per mechanism. Each leg ran through
scripts/ablation-replace.mjsin 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 HEADempty). The test imports./filesystem-loader.jsrelatively, so it reads source and nodist/step applies.if (remembered?.has(name))→if (false && …)this.retireResolutions();→void 0;this.retireResolutions(type);→void type;resolveStats(type, name, !useCache)→falseaddedevent case (1)continue→if (false)unlinkcase, the unreported-deletion case (2)exists()confirmationreturn trueunlinkcase (1)if (epoch === this.resolutionEpoch)→if (true)if (false)Gates
The local subset was derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackfrom the real diff, on the final head dff1e3f (merge base 5a871e4). It produced 64 commands, and I ran all of them.--ranreconciliation:64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN("a DERIVED zero — all 64 recorded an exit code and none of them is 3").check:lean-entry-closureneeded@objectstack/objectql's built dist;check:dual-build-cjs-loadsneeded 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.
@objectstack/spec.check:generatedreported all 14 generated artifacts up to date.Lint & Repo Gates,TypeScript Type Check,Test Core,Build Core,Governed Surface Queue Guard).test:repowas not run locally either: the diff touches no file in that project.Acceptance notes
FilesystemLoaderatrootDir.NodeMetadataManagerinstalls it whenever no loaders are passed, andMetadataPluginnever passes any. Two pieces of documentation disagree with that:MetadataPluginOptions.watchsays the source-file scan is redundant in artifact mode, and thebootstrap: '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.fs.statand onereadFileper 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.FilesystemLoader.contractdeclaressupportsWatch: true, but the loader has no watch of its own:NodeMetadataManagerowns the chokidar watcher. That is why the watch retirement rides on the handler's existinguseCache: falsere-read and the read-time repair, and not on a new loader member.Generated by Claude Code