Skip to content

fix(lint): lintLivenessProperties skips malformed collection items instead of throwing - #11734

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-11385-liveness-lint-null-guard
Aug 24, 2026
Merged

fix(lint): lintLivenessProperties skips malformed collection items instead of throwing#11734
os-warren merged 1 commit into
mainfrom
claude/issue-11385-liveness-lint-null-guard

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#11385

The contradiction

lintLivenessProperties's docblock states its contract plainly: "Advisory
only — returns findings, never throws." Three of its four walks did not hold
it — each read item.name (or item.object) straight off every collection
element with no check that the element is actually a record:

  • the flat TYPE_COLLECTIONS loop (covers every OTHER governed type: flow,
    action, agent, tool, skill, dataset, permission, hook, page, view, webhook,
    datasource, app, book, job, email_template, mapping, dashboard)
  • the object walk
  • the field walk nested under it

A null element in any of those collections threw TypeError: Cannot read properties of null (reading 'name') instead of being skipped — reachable via
the exported stack: AnyRec signature on an unparsed or hand-built stack,
which the docblock's "never throws" promise invites a caller to hand this
function without validating first. The translation bundle walk already
guards its two levels this way (isRecord(bundle) / isRecord(data),
#11383); these three did not.

The fix

One if (!isRecord(item)) continue; guard in each of the three walks,
mirroring the existing idiom from the translation bundle walk and using the
isRecord helper already defined in the file. No new helper, no behavior
change for well-formed input — a malformed element is now skipped and the
walk continues over the rest of the collection, exactly like the translation
walk already does.

Measurement

The card's own reachability note asked only about the flat loop; it flagged
the object/field walk above it as "the same one-line question" without
measuring it. Measured on origin/main before this fix, all three throw:

lintLivenessProperties({ agents: [null] }) -> throws TypeError
lintLivenessProperties({ objects: [null] }) -> throws TypeError
lintLivenessProperties({ objects: [{ name: 'w', fields: [null] }] }) -> throws TypeError

So the object/field walk needed the same guard as the flat loop — this PR
guards all three, not just the one the card's suggested shape named.

Tests

Three new regression tests in lint-liveness-properties.test.ts, one per
guarded walk, each pairing the malformed element with a real still-warning
ledger row from the shipped ledgers (not a synthetic one) so the assertion
proves two things at once: no throw, and the walk kept going past the bad
element instead of aborting silently —

  • flat loop: agents: [null, { name: 'ag1', memory: { kind: 'buffer' } }]
    still warns on memory (real experimental row)
  • object walk: objects: [null, { name: 'widget', externalSharingModel: 'read' }]
    → still warns on externalSharingModel (real planned row)
  • field walk: a null field entry alongside a real field.relatedListFilter
    (real planned row)

Reverse verification: ran these three tests against the unguarded
origin/main source first (before writing the guards) — all three failed
with the TypeError above, not the assertion they were written for. Added the
three guards, reran — all three pass.

Verification

  • pnpm --filter '@objectstack/lint^...' build (dependency closure), then
    pnpm --filter @objectstack/lint build — clean.
  • pnpm --filter @objectstack/lint test -- --maxWorkers=22274/2274
    passed
    (81 files), including the 3 new regression tests.
  • pnpm --filter @objectstack/lint typecheck — clean.
  • Scoped ESLint (eslint packages/lint/src/lint-liveness-properties.ts packages/lint/src/lint-liveness-properties.test.ts --no-inline-config --format json) — 2 files linted (matches this diff exactly), 0
    errors/warnings. Repo-wide pnpm lint is CI's to run; the collapse to
    these 2 files is sound because this repo's single eslint.config.mjs
    never enables type-aware linting for any file (no parserOptions.project, no typed @typescript-eslint rules — the file's own header, eslint.config.mjs:326-328),
    so no untouched file's lint verdict can move from this diff.
  • Local gate list derived via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from this worktree — every path-derived and
    convention-triggered local gate it named ran green: check:changeset-gate- self-tests, check:cross-package-test-inputs, check:objectui-changeset,
    check:published-files, check:slot-lookup, check:test-source-alias,
    check:type-source-resolution, check-adr-0087-registration,
    check-changeset-no-major, check-empty-changeset,
    check-plugin-teardown-shape, check-affected-docs,
    check:query-options-erasure, check:engine-double-contract,
    check:where-matcher, release-rehearsal-clone --self-test. (Two path-
    derived entries — check:cross-package-test-inputs and
    check-cross-package-test-inputs.mjs — are the same script; ran once.)
    check:type-check-debt's --re-measure ratchet needs the full
    ./packages/* + ./packages/*/* closure built; not run locally given the
    shared-container resource discipline (this diff adds no cross-package
    import and no new type surface outside the touched file, so it is very
    unlikely to move that ratchet) — flagged for CI to confirm.
  • Changeset added: .changeset/liveness-lint-null-collection-item-guard.md
    (@objectstack/lint, patch).

Scope

Touches only packages/lint/src/lint-liveness-properties.ts, its test file,
and the changeset. No other file in the package or repo is touched.


Generated by Claude Code

…stead of throwing
The docblock's own contract — "Advisory only — returns findings, never
throws" — was not held by three walks: the flat TYPE_COLLECTIONS loop, the
object walk, and the field walk nested under it. Each read `item.name` (or
`item.object`) off every collection element with no record guard, so a null
element threw TypeError instead of being skipped. The translation bundle
walk already guarded its two levels this way (#11383); this adds the same
`isRecord()` guard to the three that did not.
Measured before the fix: all three walks throw on a null element, not just
the flat loop the card's suggested shape named. Three new regression tests
(one per guarded walk) reverse-verified red against the unguarded source.
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 4 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 5f124a219c038dbcb941273df3ff7e85f2672bf4packageMentionDocs.

Which tree this was computed on

This run read content/docs from 49ebf4f524631dfbf32fcc9218ee387fe4ec4839 — the merge of head 98479e2f5d815fae2acd3026df0a1a7910b46e69 into base 5f124a219c038dbcb941273df3ff7e85f2672bf4, 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 49ebf4f524631dfbf32fcc9218ee387fe4ec4839 && git checkout 49ebf4f524631dfbf32fcc9218ee387fe4ec4839
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5f124a219c038dbcb941273df3ff7e85f2672bf4 98479e2f5d815fae2acd3026df0a1a7910b46e69 && git checkout -B drift-repro 5f124a219c038dbcb941273df3ff7e85f2672bf4 && git merge --no-ff 98479e2f5d815fae2acd3026df0a1a7910b46e69
node scripts/docs-audit/affected-docs.mjs --json 5f124a219c038dbcb941273df3ff7e85f2672bf4

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-warren
os-warren marked this pull request as ready for review August 24, 2026 15:14
@os-warren
os-warren added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit 20a452eAug 24, 2026
32 checks passed
@os-warren
os-warren deleted the claude/issue-11385-liveness-lint-null-guard branch August 24, 2026 15:42
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: lintLivenessProperties throws on a null collection item, against its own "returns findings, never throws" contract

2 participants

@os-warren@claude