Skip to content

fix(metadata): FilesystemLoader.list() reports only names findFile() can resolve - #14922

Merged
hotlong merged 3 commits into
mainfrom
claude/issue-14486-filesystemloader-list-reachability
Sep 3, 2026
Merged

fix(metadata): FilesystemLoader.list() reports only names findFile() can resolve#14922
hotlong merged 3 commits into
mainfrom
claude/issue-14486-filesystemloader-list-reachability

Conversation

@os-musk

@os-musk os-musk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14486

FilesystemLoader.list() now reports only names findFile() / load() / exists() can resolve. One shared name-to-path derivation, per the maintainer ruling on the card (option A, narrow — via the director seat, 2026-09-02).

The defect, driven at origin/main (not inferred)

Probe fixture from the card, default format set (typescript / json / yaml):

LIST                : ["account","dotted.config","flat","nameless","nameless-nested","noext","scripted","scripted","yamlish","yamlish2"]
  account          exists=false stat=null load.data=null
  nameless-nested  exists=false stat=null load.data=null
  noext            exists=false stat=null load.data=null
  ...every other listed name: exists=true stat=HIT load.data=BODY

Both populations on the card reproduce. MetadataManager.listNames() unions loader output unfiltered, so a name sat in the list while get() answered null for it — the silent failure an author reads as their own typo.

The repair

list() converges on resolvableNameForPath(), the derivation loadManyKeyed() already used: a file directly under the type directory, carrying an extension one of this instance's registered serializers claims. The extension set is now derived from this.serializers rather than a second hard-coded list, so the set a name can be RESOLVED under cannot drift from the set that is LISTED.

That is also how the card's row-4 membership mismatch closes: under the default set a .js file leaves list(), where it was previously listed and resolvable while loadMany() could never return it and load() threw No serializer found for format: javascript.

The extension set deliberately does not follow ADR-0008 §10's .json-only rule, per the ruling: §10 governs the metadata-fs store, and applying it verbatim here would drop .yaml and .ts metadata out of listNames() — a breakage this card never asked for.

After (same fixture, same probe):

LIST                : ["dotted.config","flat","nameless","scripted","yamlish","yamlish2"]
  ...every listed name: exists=true stat=HIT load.data=BODY

PM assumptions, measured

  • A — both populations reproduce. Confirmed, driven (above).
  • B — list() over-reports, or the trio under-resolves? Settled by a citable contract, not by taste: the maintainer ruling in comment 5518651566 adopts A (narrow list()), recorded before this dispatch.
  • C — does any caller depend on the over-reporting? No. Repo-wide, every non-test listNames hit is a definition (metadata-manager.ts:1571, objectql/metadata-facade.ts:306, spec/contracts/metadata-service.ts:440, core/fallbacks/memory-metadata.ts:84); there is no production call site at all. listForIndex() reads loadMany*, not list(). Control: the same grep does return the call sites in test files.
  • D — does DatabaseLoader have the same split? No. Its list() returns the name COLUMN, the same key exists() / stat() / load() filter on and loadManyKeyed() keys by. No derivation, so no derivation mismatch — this card is one instance, not a two-instance class.
  • Card row 4, .ts half — FALSIFIED, as triage required it be re-measured. A .ts module carrying export const and a JSON-compatible object literal loads fine through TypeScriptSerializer under the default set. The card's probe missed because of its fixture, not the loader. Pinned as a CONTROL.

Two halves of the ruling this PR does NOT take, and why

Both are the "neither listed nor resolvable" clause. Each would invert a landed #14341 pin in filesystem-loader-keyed-items.test.ts — a file under a concurrent claim (PR #14627 touches this package's existing tests and its package.json), which this card was dispatched not to touch.

  1. Filtering the shared loadMany() walk (the ruling's "nothing unlisted is returned by loadMany() either") would break filesystem-loader-keyed-items.test.ts:113 (exact-equality on the keyed set, including extensionless_named and nested_named), :167, :187, and the loadMany() CONTROL at :196 ("with every file", length 7).
  2. Refusing a path-shaped name in findFile() would break :175, which asserts exists(TYPE, 'crm/nested_named') is true. It would also need save() to narrow in the same stroke — save() mkdir-p's the name's dirname, so it still CREATES nested files.

Both are pinned as RECORD cases in the new test file rather than left implicit, so whoever takes the remainder sees the divergence in a diff.

Tests

New file only — packages/metadata/src/loaders/filesystem-loader-list-reachability.test.ts. ⛔ No edit to packages/metadata/package.json and ⛔ none to any existing *.test.ts in that package (diff is 3 files).

Reverse verification: with the loader reverted to origin/main and the new test kept, 7 of 14 faillists exactly..., EVERY listed name resolves..., the nested case, the extension-less case, the .js case, and both manager-level cases. The 7 that stay green are exactly the CONTROLs and RECORDs, which pin what must not move. Restore proven by blob hash equality with HEAD plus an empty git diff HEAD, not by an exit code.

At 97531bb76:

Changeset

minor with a BREAKING banner, per the ruling. The dispatch prompt said patch; the ruling is the more specific and citable instruction, and this narrows consumer-visible output, so minor it is — flagged here rather than chosen silently.

Per the ruling this PR carries needs:contract-review at CONTRACT_REVIEW_TIER, a dual carrier with the card.

Out-of-scope finding filed: #14921 — two files sharing a stem across registered extensions make list() report one name twice, and the loser is reachable through no name at all. Not repaired here; it is the opposite direction from this card and needs its own contract call.

Draft on purpose: the PM seat marks ready and arms the queue.

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…can resolve

`list()` reported `path.basename(file, ext)` for every file its glob found —
nested or not, with an extension or without — while `findFile()` resolved
`ROOT/TYPE/NAME` plus one of five hard-coded extensions. The two disagreed for
three shapes, and the disagreement reached consumers through
`MetadataManager.listNames()`, which unions loader `list()` output unfiltered:
a name sat in the list while `get()` answered `null` for it, silently.

`list()` now converges on `resolvableNameForPath()` — the derivation
`loadManyKeyed()` already used — so it reports a file only where the mapping is
a bijection: directly under `ROOT/TYPE/`, carrying an extension one of this
instance's REGISTERED serializers claims. The extension set is registered
rather than hard-coded, which is also how the card's row-4 membership mismatch
closes: under the default format set a `.js` file leaves `list()`, where it was
previously listed and resolvable but loadable by nothing.

Ruled by the maintainer via the director seat (2026-09-02) as option A over
the reverse-unify, which would have made a slash inside a metadata name every
consumer's permanent obligation. The extension set deliberately does not follow
ADR-0008 §10's `.json`-only rule — that governs the `metadata-fs` store, and
applying it here would drop `.yaml` and `.ts` metadata from `listNames()`.

Not taken: filtering the shared `loadMany()` walk, and refusing a path-shaped
name in `findFile()`. Each inverts a landed #14341 pin in
`filesystem-loader-keyed-items.test.ts`, a file under a concurrent claim
(PR #14627). Both are pinned as RECORD cases in the new test file instead.

Part of #14486

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…owing

`check-adr-0087-registration` requires a declared-breaking changeset to answer
the ledger question in writing. No authorable key, Zod schema or stored row
moves here, and a tree carrying one of the two shapes needs the FILE relocated
rather than any document rewritten — nothing `objectstack migrate meta` can
project, so the disposition is `not-required (no-migration-prescription)`.

Part of #14486

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions github-actions Bot added the size/m label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata, touching 6 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))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 12 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 431979e67cc4d0edfb0b8743385628cc425662bcpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 4b6caebe035928ad5ca9a7e8646c416b85ae0485 — the merge of head 97531bb76c7db379a7ec22383fe51f1149ede1ff into base 431979e67cc4d0edfb0b8743385628cc425662bc, 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 4b6caebe035928ad5ca9a7e8646c416b85ae0485 && git checkout 4b6caebe035928ad5ca9a7e8646c416b85ae0485
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 431979e67cc4d0edfb0b8743385628cc425662bc 97531bb76c7db379a7ec22383fe51f1149ede1ff && git checkout -B drift-repro 431979e67cc4d0edfb0b8743385628cc425662bc && git merge --no-ff 97531bb76c7db379a7ec22383fe51f1149ede1ff

node scripts/docs-audit/affected-docs.mjs --json 431979e67cc4d0edfb0b8743385628cc425662bc

⚠️ 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 431979e67cc4d0edfb0b8743385628cc425662bc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

hotlong commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Contract review: PASS at head 97531bb76 — landing held on one CI red in a package this diff does not touch; failed jobs re-run once by this seat

Director seat, summon #12, session session_01WXyGTWPbbreqXow7Z2pZCk, on the maintainer's instruction 「现在执行契约复审」. First review of this PR (the engine seat reported the tier unavailable, 5525655441). Fuse: served claude-fable-5-1 = CONTRACT_REVIEW_TIER.

① Derived judgments — against ruling 5518651566 (maintainer 「其他同意」, option A narrow)

  • list() narrows to names findFile() can open, via the one derivation loadManyKeyed() already used (resolvableNameForPath), and the extension set now comes from the instance's registered serializers (EXTENSIONS_BY_FORMAT filtered by this.serializers) instead of a second hard-coded list — so the set a name can be LISTED under and the set it can be RESOLVED under cannot drift. Under the default format set .js leaves list() as a consequence (row 4 closes). Nested and extension-less files stop being listed. That is the ruling at the scale the recording seat corrected it to (registered serializer set, not §10's .json-only rule).
  • BREAKING narrowing of a published loader's output, correctly banner'd; loadMany(), findFile() on a path-shaped name, and save() are unchanged.
  • The clause not taken — "nothing unlisted is returned by loadMany() either" — is the recording seat's pin text, not the maintainer's verbatim reply, and it collides with the landed metadata: FilesystemLoader and RemoteLoader still discard the store key in loadMany(), so a nameless body stays invisible to list() through those two loaders #14341 pins (filesystem-loader-keyed-items.test.ts:113/167/187/196), which carry their own ruling. The engine seat's reading A (5524349583) is the right reconciliation: the defect as filed (a listed name get() cannot open) is fully repaired, every listed name resolves, and the remaining listed ⊂ loaded divergence is pinned as a RECORD so it is visible in a diff. If the maintainer wants loadMany() narrowed too, that is its own card after PR fix(metadata): repair the 89 hidden test-type errors and wire a typecheck script #14627 lands — flagged in this summon's report, not decided here.
  • Row 4's .ts half is falsified by re-measurement and pinned as a control ✓.

② Semver vs changeset

@objectstack/metadata minor with the BREAKING banner and the ADR-0087 not-required (no-migration-prescription) marker — the dispatch's patch was wrong and the dev flagged it rather than choosing silently ✓.

③ Boundary flags

#14921 (stem shared across extensions; the loser reachable through no name) filed, opposite direction, not repaired here ✓. packages/metadata still has no typecheck script (#14342) — the test file's type reading is NOT MEASURED and stated ✓. Fixes #14486 kept (Q2 = A) ✓. No edit to package.json or any existing test in the package (PR #14627's claim respected) ✓.

The CI red, and why it is not this PR's until a re-run says so

Test Core (6/6) failed on @objectstack/plugin-authdurability-swallow-repair.test.ts:673 timed out at 10 s (import 335.31s on that shard). This diff is three files under packages/metadata; it does not touch plugin-auth or anything it imports. Per the drive-to-green rule, an error naming a package the diff does not touch is ruled out by one re-run: failed jobs of run 33744244649 re-queued by this seat (201 Created). If the shard is green on the re-run, this PR lands (carriers stripped citing this PASS, ready, auto-merge — this seat if still in office, otherwise the engine seat). If the same test fails again on this head, it is real and goes back to the engine seat as a finding against plugin-auth, still not a change to this diff.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 3, 2026 14:28

hotlong commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Re-run green — the plugin-auth timeout did not reproduce; landing on the PASS above

Director seat, summon #12. The one permitted re-run of run 33744244649's failed jobs (queued 14:19Z) finished at 14:24Z: Test Core (6/6) success, Test Core aggregate success; every other check on head 97531bb76 was already green. The earlier durability-swallow-repair.test.ts 10 s timeout in a package this diff does not touch is therefore ruled out as this PR's — no second re-run, no test touched.

Landing: needs:contract-review stripped from this PR and from #14486 citing PASS 5527218051, marked ready, auto-merge armed (merge queue). #14486 closes via Fixes when the queue lands it; the loadMany() clause question stays with the maintainer as noted in the review.


Generated by Claude Code

@hotlong
hotlong enabled auto-merge September 3, 2026 14:29
@hotlong
hotlong added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 4b4d5a3 Sep 3, 2026
57 of 59 checks passed
@hotlong
hotlong deleted the claude/issue-14486-filesystemloader-list-reachability branch September 3, 2026 14:56
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/m tests tooling

Projects

None yet

3 participants