Skip to content

fix(metadata): RemoteLoader.list() no longer reports a nameless body as a literal undefined - #16107

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-15037-remote-loader-list-nameless
Sep 6, 2026
Merged

fix(metadata): RemoteLoader.list() no longer reports a nameless body as a literal undefined#16107
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-15037-remote-loader-list-nameless

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #15037

RemoteLoader.list() declares Promise<string[]> and read the collection as loadMany<{ name: string }>(type) before mapping items.map(i => i.name). That type argument is an assertion about bodies that arrived over HTTP, and nothing checked it: a body with no top-level name yielded undefined, which went into an array the signature declares as string[]. MetadataManager.listNames() unions loader list() output unfiltered, so the violation reached consumers — a runtime violation of a declared type, not an untidy entry.

The repair is one file away, and the direction was already decided

RemoteLoader was the only one of the four loaders in packages/metadata/src/loaders/ with no guard at all. Anchors re-derived on the merged tree:

loader what list() does today can it yield undefined?
DatabaseLoader :1062 rows.map(row => row.name as string) followed by .filter(name => typeof name === 'string') :1076-1078 no — same cast, guard behind it
MemoryLoader :97 Array.from(typeStore.keys()) no — returns the store key
FilesystemLoader :366 narrowed by #14486 — only names findFile() resolves no — list and door aligned
RemoteLoader :113 items.map(i => i.name) yes — the only unguarded one

So this copies DatabaseLoader's guard shape: same directory, same method name, same cast-then-map spelling, one typeof filter behind it.

"Refuse loudly" was not taken, and that is a landed decision rather than a preference. DatabaseLoader's guard is a silent .filter(), and FilesystemLoader's narrowing carries a maintainer ruling (director seat, #14486, 2026-09-02, direction A: narrow the list, with B explicitly refused). That ruling's own docblock reasoning points straight here:

A name in the list that get() answers null for is the silent failure an author (human or AI) reads as their own typo, so they retry the same word: the list and the door now agree instead.

An undefined in listNames() is the extreme form of a name the door can never answer.

Two things deliberately NOT copied. MemoryLoader's structural fix (return the store key) and #14205's keying rule ("identity is the key the store holds an item under, not body.name") — RemoteLoader reads over HTTP and holds no store key, so body.name is the only identity it has and that rule cannot be satisfied here. loadMany() is deliberately untouched: it keys nothing, so a nameless body is still served there.

Not a duplicate of the closed neighbours

This table is preserved from the card because it is what stops this being closed as a dupe.

card what it fixed why this survives it
#14341 the item being dropped opposite direction — that card is about a nameless item going missing; this is about it appearing, as undefined. The code repaired here is what was on main after that card closed.
#14205 the same drop-direction defect, one layer up same reason
#14486 a different loader, and names that exist but are unreachable this one is a name that is not a string at all

The shape of the guard

The predicate is spelled as a type guard and the mapped element type left unknown, so tsc proves the declared string[] instead of a cast asserting it. Spelled with the original { name: string } cast, the compiler reads the filter as always-true and a later reader deletes it as dead.

Verification

Everything below was measured on the merged tree at 20cf207c4 (this branch merged with origin/main 1157e7b72; merge, never rebase).

Ablation — the test genuinely reddens. RemoteLoader.list() reverted to the pre-fix shape, mutation proven on disk before measuring (anchored grep counts flipped 1→0 and 0→1; git hash-object 279ddc56 differs from the HEAD blob 2a7b5dce), then restored and the restore proven (blob equals HEAD blob, git diff HEAD empty, whole-tree git status empty). No rebuild leg applies and that is a measurement, not an assumption: the test imports the subject relatively and in-package (./remote-loader.js, ../metadata-manager.js), so vitest resolves src/ directly — the package's exports map and dist/ are not on the resolution path, and this package's vitest aliases cover only @objectstack/core, /spec and /types.

Mutated:   6 failed | 6 passed (12)
Repaired:  12 passed (12)

The 6 that stayed green under mutation are the CONTROL: and RECORD: cases — they pin what must not move, so a guard that dropped everything would fail them. The failures reproduce the defect literally:

AssertionError: expected [ 'account', undefined, 42 ] to not include undefined
AssertionError: expected 'undefined' to be 'string'
AssertionError: expected undefined to be defined     (listNames/get disagreement)

'account' is the in-run control: a well-formed body stays listed and stays loadable throughout.

Tests. pnpm --filter @objectstack/metadata test — 49 files / 757 tests passed. pnpm --filter @objectstack/metadata typecheck — clean. The typecheck is a real reading about the new file, not a vacuous one: tsc --noEmit --listFiles contains remote-loader-list-nameless.test.ts (count 1), this package's tsconfig.json includes src/**/* and excludes only node_modules and dist, and it carries no test-typecheck-debt.json.

Pin shape. Triage named the model and it is reused on the RemoteLoader face: filesystem-loader-list-reachability.test.ts :210/:218 is already "listNames() and get() give the same answer for the same name". That sibling pin was run alongside and stays green.

Gate union — 53/53 families, re-derived after the final commit. scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (provenance line checked; harvested with --commands, never by grepping the prose). 51 exit 0. Two returned exit 3 = PREREQUISITE NOT MET, which their own verdict lines state is neither a pass nor a finding:

  • check:dual-build-cjs-loads — "this gate reads built output, and some package has no dist/ … This is NOT a pass: nothing was measured." 57 packages unbuilt.
  • check:type-check-debt — "--re-measure cannot run: 20 workspace dependencies … have no built type entry point on disk … This is NOT a pass and NOT a finding."

Both need the whole-workspace build CI performs before those steps, so they are recorded NOT MEASURED and left to CI rather than run half-built — the debt gate states outright that a number measured without the closure "would silently measure a DIFFERENT WORLD". Both self-tests passed. Neither is plausibly moved by this diff: it adds no entry point, export or import, and @objectstack/metadata is not in the DEBT or TEST_DEBT ledgers.

Lint. The full repo sweep pnpm lint (eslint . --no-inline-config) ran to completion: exit 0. No narrowing was used, so none needs justifying.

Docs. scripts/docs-audit/check-affected-docs.mjs exit 0. Measured rather than assumed: the only content/docs/** mention of RemoteLoader is a one-line capability row in content/docs/protocol/kernel/metadata-service.mdx:67 ("Fetches metadata from a remote HTTP API"), which does not document list()'s nameless-body behaviour — nothing published is falsified, so no doc repair is owed.

Clause-② — both limbs re-derived: no

Measured with the corrected instrument: build at head, swap remote-loader.ts back to 1157e7b72, rebuild, and diff every declaration file the package publishes resolved from its exports map and files[] (dist/**, 10 files) — not the root barrel alone.

Limb 1 — does any exported symbol or signature move? No. Two of the ten declaration files differ in bytes (dist/index.d.ts, dist/index.d.cts), and reading the bytes alone would have produced a false "widening". The textual diff is the added JSDoc block and nothing else; the declaration itself is byte-identical before and after:

list(type: string): Promise<string[]>;

The other eight published declaration files are byte-identical. The restore leg was rebuilt too, and dist/index.d.ts hashes back to the head snapshot exactly (6c459008), so no mutant output was left behind for later runs to measure.

Limb 2 — is any request newly accepted or rejected? No. Nothing validly returned before stops being returned; the only entries that disappear are the ones the declared type already ruled out. A caller that previously received [undefined] now receives [] — that is a declared-type violation being removed, not an accept set moving. The changeset is therefore patch.

Grade

priority:p3 is unchanged — this round measured neither of the two things triage said decide severity, and had no means to: population (how many deployments use RemoteLoader, how many remote bodies lack a top-level name) and a demonstrated downstream break. The promotion condition stands as triage wrote it: any single such measurement re-grades to p2.

Recovery-round note

The previous round was killed mid-flight by a container restart, leaving one pushed commit and no PR and no report — so no verification record was inherited, and the pushed diff was re-read adversarially rather than trusted. Found already on the branch: the guard, the test file and the changeset. Added this round: the origin/main merge, the re-derived anchors, and the entire verification record above — ablation with on-disk mutation and restore proofs, the 53-family gate union, the full lint sweep, the docs verdict, the --listFiles coverage proof and the Clause-② declaration-file instrument.

Authored by Claude Code in session session_01ARYe3yQTQCUFm5qPYNgKaJ.


Generated by Claude Code

…as a literal `undefined`

`list()` declares `Promise<string[]>` and read the collection as
`loadMany<{ name: string }>(type)` before `items.map(i => i.name)`. That type
argument is an assertion about bodies that arrived over HTTP; nothing checked
it. A body with no top-level `name` yielded `undefined`, pushed into an array
the signature declares as `string[]`, and `MetadataManager.listNames()` unions
loader `list()` output unfiltered — so the violation reached consumers. On this
fixture `listNames()` answered `[ 'account', undefined, 42 ]`.

The guard is `DatabaseLoader.list()`'s, one file away: same cast-then-map
spelling, `typeof name === 'string'` behind it. `RemoteLoader` was the only one
of the four loaders in the directory with no guard at all. The predicate is
spelled as a type guard with the mapped element left `unknown`, so `tsc` proves
the declared `string[]` instead of a cast asserting it.

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

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata, touching 1 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 RemoteLoader (symbol, a top-level class))
What this run could not see
  • 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 — 13 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 64011dd3fd2fc4352951f26c71da5c824abc6050packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2adff617054e94bf6eef7bbc7c4dd967a5938fcd — the merge of head 20cf207c406b97e9dc533a9e25f8079812c8aa6e into base 64011dd3fd2fc4352951f26c71da5c824abc6050, 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 2adff617054e94bf6eef7bbc7c4dd967a5938fcd && git checkout 2adff617054e94bf6eef7bbc7c4dd967a5938fcd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 64011dd3fd2fc4352951f26c71da5c824abc6050 20cf207c406b97e9dc533a9e25f8079812c8aa6e && git checkout -B drift-repro 64011dd3fd2fc4352951f26c71da5c824abc6050 && git merge --no-ff 20cf207c406b97e9dc533a9e25f8079812c8aa6e

node scripts/docs-audit/affected-docs.mjs --json 64011dd3fd2fc4352951f26c71da5c824abc6050

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 6, 2026
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 6, 2026 01:59
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit efc5447 Sep 6, 2026
35 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15037-remote-loader-list-nameless branch September 6, 2026 02:45
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

2 participants