Repository navigation
views unknown-key lint says "dropped at load" for a key the next step refuses — the second disagreeing voice lintUnknownStackKeys avoids at the top level #10039
Description
Activity
Triage: lands in
packages/spec/src/kernel/metadata-authoring-lint.ts⇒domain:spec,pm:queue, type Bug. Rationale: misleading diagnostic — the lint promises "dropped at load" for an unknown key that the strict view-container parse then refuses loudly (defineStack throws), and the finding is emitted twice but masked by warn-dedupe; a diagnostic that mis-states what will happen is worse than none. Diagnostic-text/posture fix, no acceptance change expected (Clause-②: nounless the fix moves the strip/strict boundary itself — that would be a fork to report, not to absorb).
Generated by Claude Code
{ "issue": 10039, "status": "done", "branch": "claude/issue-10039-views-lint-posture", "pr": "https://github.com/objectstack-ai/objectstack/pull/10192", "premise_still_valid": true, "summary": "Both defects reproduced on origin/main and fixed at the root. (1) lintUnknownAuthoringKeys read each collection's unknown-key posture from getMetadataTypeSchema(type) — the persisted-BODY schema — while defineStack parses a collection ENTRY through ObjectStackDefinitionSchema's own slot; for `view` those are ViewMetadataSchema (strip union, 91 keys) vs ViewSchema (strict defineView container, 15 keys), so the lint promised 'dropped at load' for a key the parse refused. Posture now comes from the stack schema's slot, injected as a REQUIRED parameter the same way and for the same cycle reason lintUnknownStackKeys already required it. Measured over all 29 collections, `views` is the only verdict that changes (themes/analyticsCubes are strict on the stack side and unregistered on the registry side, so skipped either way); for every collection that still lints the stack slot's element is the IDENTICAL node the registry returned, so the walk is unchanged there and `connectors` is the honest remainder. (2) The duplicate came from lintUnknownKeysAgainstSchema reporting the root record AND handing that same record to descend, whose object arm skipped depth===0 while its union arm had no such guard — `view` was the only union root in the wild, so defineStack's module-level warn-once set absorbed the second copy while every other consumer of the exported walker saw both. The root report now lives in descend alone: one record, one report. That also closes a latent third copy (a discriminated-union root whose branch the author picked was reported once against the merged key set and again against the branch's; now once, against the branch). Clause-② holds: the parse is untouched and defineStack accept/reject is byte-identical — only which of the two existing voices speaks changes. The strip/strict boundary is not moved, so no fork to report.", "tests": "All at 51b23d26b (tree bbebc1c8), merge base cc21aad8e; the gate union was run on this exact tree — verified by tree-hash equality, not assumed. Every gate exit code captured per-command (`echo GATE_EXIT_<name>=$?` immediately after each), never through a pipe. (a) `pnpm --filter @objectstack/spec test` — 'Test Files 414 passed (414) / Tests 11044 passed (11044)', os-verify-lock VERDICT command-exit 0. (b) `pnpm --workspace-concurrency=2 --filter @objectstack/spec --filter @objectstack/cli run typecheck` — both 'Done'; scripts echoed the real script names (tsc --noEmit, check:scripts-typecheck, check:test-typecheck) so this is not a zero-match pnpm filter; spec's check:test-typecheck printed 'OK — @objectstack/spec's test layer compiles ... 55 file(s) / 263 error(s) held in test-typecheck-debt.json' (ledger unmoved). (c) `pnpm --filter @objectstack/cli exec vitest run test/metadata-type-schema-gate.test.ts test/validate-build-gate-parity.test.ts` — 'Test Files 2 passed (2) / Tests 16 passed (16)'. (d) GATES — 25 run, all exit 0, list re-derived from the ACTUAL diff with `node scripts/pm/dispatch-gates.mjs` (no paths passed; script derived 7 paths vs merge base cc21aad8e). Convention-triggered (5): check:type-check-coverage ('OK — 64/77 workspace packages type-checked'), check:type-check-debt --re-measure ('OK — 33 ledger entr(ies) re-measured in 232.4s, 1924 raw tsc error(s) total, none above its recorded number'; workspace closure built first via turbo, 70/70 tasks successful), check:engine-double-contract ('OK — 329 pinned, 133 in the DEBT ledger, 2 exempt'), check:where-matcher, check:query-options-erasure. Path-derived (15): check:cross-package-test-inputs and scripts/check-cross-package-test-inputs.mjs ('OK: 12 package(s) read outside themselves, all declared'), doc-formula-expressions, spec check:empty-state, spec check:liveness, check:merge-driver, check:slot-lookup, check:spec-parsed-alias ('--self-test: 18 assertions passed'), check:stack-collection-maps, spec check:strictness-ledger, check:type-source-resolution ('OK — 76 packages with a tsconfig.json scanned'), spec check:variant-docs, check-dev-prereqs.mjs, docs-audit/check-affected-docs.mjs, check-nul-bytes.mjs ('OK (scanned 6067 text file(s) ... no raw ASCII control bytes)'). Changeset-triggered (5) — NAMED ONLY BY THE RE-DERIVATION, missed by both the dispatch brief's predicted list and my own first pass because the changeset file joined the change set afterwards: check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration.mjs ('this PR adds no declared-breaking changeset'), check-changeset-no-major.mjs, check-empty-changeset.mjs. The brief's predicted list also did not name check:stack-collection-maps. (e) REVERSE VERIFICATION of the duplicate half, direction declared before running (revert must make union roots report twice, object roots stay at one): plain union root 2 -> 1; discriminated union with the branch picked 3 -> 2; plain object root 1 -> 1 (control); nested strip object 1 -> 1 (control) — observed exactly as predicted. Mutation confirmed ON DISK before running by grepping the two reintroduced anchors (1 hit each), not by the editor's exit code; the restore leg confirmed 0/0 hits AND byte-identical (`diff -q`) to the pre-mutation file. NO BUILD OR dist/ IS INVOLVED in that probe — it imports packages/spec/src/** directly through tsx, so the ablation cannot be reading a stale artifact. (f) The 'before' side of the POSTURE half is not an ablation but a direct measurement on unmodified origin/main: listLintableAuthoringCollections() advertised [connectors, views] and the card's repro printed 'findings: 2' alongside the throw; after the fix it advertises [connectors] and the same repro prints 0 findings for the view key with the strict-container refusal as the only voice.", "open_questions": [], "out_of_scope_findings": [ "filed as #10194: `theme` and `analytics_cube` have strict stack schemas (ThemeSchema/CubeSchema in stack.zod.ts) but no UNREGISTERED_KIND_SCHEMAS binding, so PUT /api/v1/meta/theme/:name still takes saveMetaItem's 'unregistered type -> store without validation' branch — the two doors #6245 left open when it closed the same hole for webhook/connector/sharing_rule. Filed unassigned, no label (concrete defect, left for PM triage).", "NOT filed (checked, already tracked): `ragPipelines` is in PLURAL_TO_SINGULAR but has no ObjectStackDefinitionSchema slot, so the now-strict stack root refuses it outright. scripts/check-stack-collection-maps.mjs:382 already carries it as a recorded drift row citing #6242. Recording the check so the next sweep does not re-derive it." ], "deviations": [ "CHANGESET LEVEL: `minor`, not the `patch` the dispatch brief predicted. The behavioural fix alone is a patch; the signature is not. lintUnknownAuthoringKeys and listLintableAuthoringCollections — both public exports from packages/spec/src/index.ts — now take ObjectStackDefinitionSchema as a REQUIRED parameter. Required rather than optional deliberately: an omitted argument falling back to the type registry would silently reinstate this exact bug, which is the silent-loss shape the rule family exists to report and which the module's own docblock already argues against for the sibling function. All three in-repo call sites (defineStack, os validate, os compile) already held the schema for the sibling call on the adjacent line, so the break is a compile error at every site, not a runtime surprise. Stated rather than absorbed — overrule if the maintainer wants patch. check-changeset-no-major.mjs is green either way.", "BRANCH HISTORY: three commits, the first named `wip`. The work was pushed before it was tidy (in response to the PM's status probe), and squashing after that push would have required a force-push, which is prohibited outright. The tree is unaffected — I verified the squashed and pushed commits produce the identical tree bbebc1c8 before restoring the pushed history, so nothing was lost. Squash-on-merge yields the intended single commit; the PR title and body carry the description.", "GATE LIST: the dispatch brief's predicted list was treated as a lead. Re-deriving from the actual diff added check:stack-collection-maps, and re-deriving AGAIN after the changeset landed added five more (the changeset-gate family). All six were run; all green. Recording it because the first re-derivation would itself have been stale — the change set grew after it.", "BASE MOVED UNDER ME: origin/main advanced cc21aad8e -> 19f98fa1f mid-task (worktrees share refs, so a sibling agent's fetch moved it). `git diff origin/main` consequently showed five files I never touched. My commits are clean — verified against the merge base: exactly 7 files. The gates were derived and run against merge base cc21aad8e throughout, which is the correct three-dot semantics. The branch is NOT rebased onto the newer main; CI will evaluate the merge." ], "files_changed": [ "packages/spec/src/kernel/metadata-authoring-lint.ts", "packages/spec/src/kernel/metadata-authoring-lint.test.ts", "packages/spec/src/stack.zod.ts", "packages/cli/src/commands/validate.ts", "packages/cli/src/commands/compile.ts", "packages/cli/test/metadata-type-schema-gate.test.ts", ".changeset/views-lint-posture-one-voice.md" ] }
Generated by Claude Code
Contract review (
needs:contract-review) — PASS. Reviewed PR #10192's contract increment atCONTRACT_REVIEW_TIER(claude-fable-5; reviewer = triage seat, not the dispatching seat; sessionsession_014Nctb3bgNQFRqi1troHpPL).Verdict, one line: the metadata contract's accept/reject set is unchanged — the parse is untouched; the diff changes which of two existing voices speaks, and the exported lint API's signature, loudly. Specifics verified on the diff:
- Posture source correction is contract-faithful:
collectionEntrySchemareads the element schema offObjectStackDefinitionSchema's own slot — the schemadefineStackactually applies — replacing the persisted-body registry read. The measured 29-collection table (onlyviewsleaves the lintable set;themes/analyticsCubesskipped either way) matches the schemas on main. Note for the spec seat: this PR makes the lint independent ofgetMetadataTypeSchema, so it neither conflicts with nor prejudgesthemeandanalytics_cubehave strict stack schemas but noUNREGISTERED_KIND_SCHEMASbinding —PUT /meta/theme/:namestill accepts any JSON (the two doors #6245 left open) #10194 (bindingtheme/analytics_cubeinUNREGISTERED_KIND_SCHEMAS— the/metadoor, a different registry). - Required (not optional)
stackSchemaparameter is the right fail-loud shape — an optional fallback to the registry would silently reinstate the exact divergence this fixes. This is the contract-tightening direction (声明即强制), endorsed. - Single-report ownership moved wholly into
descend, with the previously-maskingnew Set(...)removed from the test that would have caught the duplicate — the mask removal is the part that keeps this honest. - One flag, not a blocker: the required-param addition is a compile-breaking signature change for any external caller of
lintUnknownAuthoringKeys/listLintableAuthoringCollections, shipped asminorwith explicit reasoning in the changeset. Within the v17 train and for tooling-facing exports this is acceptable; the changeset's own justification is on the record.
Label cleared same-stroke; the card may enter the queue path once the PM seat's normal review passes.
本评论来自分诊座位 Routine
Generated by Claude Code
Generated by Claude Code
- Posture source correction is contract-faithful:
- added a commit that references this issue
on Oct 7, 2026
Found while working #9907 (correcting the
objectstack-platformskill's account of unknown-key handling), where the per-surface split had to be re-measured rather than recalled. Filing unassigned for triage; #9907 touches onlyskills/**and does not change any of this.Measured
Against the built
@objectstack/speconmainat09b880b0c, with an otherwise valid view container (it haslist, so nothing else is wrong with the fixture):Two problems, one root
1. The warning contradicts the behaviour. The author is told the value is dropped at load — i.e. the view still loads without it — and is then told the stack does not load at all.
lintUnknownStackKeysguards against exactly this at the top level, and its own source says so:It reads posture and returns
[]for a strict schema (verified:[]for the now-strict stack root). The per-collection walkerlintUnknownAuthoringKeysreads posture too, but offgetMetadataTypeSchema('view')— the strip-mode view item schema — whiledefineStackparses each entry through the strict view container. The posture the lint reads is therefore not the posture the parse applies, soviewsis judged lintable when its parse refuses.listLintableAuthoringCollections()inherits the same root and currently advertises:connectorsgenuinely warns-and-drops (measured: warning printed, no throw, key absent after the parse).viewsdoes not.2. The finding is emitted twice.
lintUnknownAuthoringKeysreturns the identical finding two times for one key:Only one warning reaches the console because
warnUnknownAuthoringKeysdedupes through a module-levelwarnedUnknownAuthoringKeysset. The duplication is invisible throughdefineStackand fully visible to every other consumer of the exported function, so the dedupe is masking it rather than fixing it.Why it matters
The direction is the same one that made #9907 worth filing, only inverted: here the tooling calls a hard refusal a silent drop. An author who reads the warning and stops there concludes the view will load minus one key, and the truth is that nothing loads. It also weakens the warn channel generally — a warning that is sometimes really an error trains readers to discount it.
Suggested shape (not a decision): have the walker read posture from the schema the parse actually applies to a collection entry, so a strict container drops out of the lintable set the way the strict root already does. That would leave
connectorsas the honest remainder.Refs: #9907 · #4001
Generated by Claude Code