Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-detail): record:activity never widens on an unusable types filter (#5841) - #5890
Merged
Merged
Conversation
…s` filter (#5841) `normalizeFeedTypes` returned `undefined` for three different authored inputs — no `types` key, `types: []`, and a list whose every member was unrecognised — and `applyFeedConfig` reads `undefined` as "apply no filter". So a page that named the wrong feed kind was served EVERY activity on the record, with no diagnostic anywhere at runtime. Measured in a real app: a lead page authored `types: ['crm_task']` (an object name where a feed kind belongs) and its Activity tab rendered the audit stream for as long as it shipped. A sanitiser may narrow an author's request or refuse it; it must never silently widen it. Widening turns a typo into "show the user everything" — the one outcome no author asked for — and it hides behind a PLAUSIBLE result, since a populated timeline reads as working while an empty one gets investigated. `undefined` is now reserved for one meaning: no `types` key was authored. An authored filter that keeps nothing returns `[]`, and the call site tests `!== undefined` rather than truthiness, so `[]` filters to nothing. A non-array `types` is refused for the same reason rather than ignored. Unrecognised entries are named once, through the same warn-once plumbing #5886 added for unmapped `sys_activity.type` values, now factored into a shared `warnOnce` with a bucket per channel — the two vocabularies overlap, so one channel having spoken must not silence the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
August 23, 2026 18:43
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5841
record:activitysanitised its owntypesprop and turned an unrecognised or empty kind list into no filter, so an author who named the wrong kind was served every activity on the record with no diagnostic anywhere at runtime.Re-measured at the live source
The card measured a shipped bundle (
@objectstack/console17.1.0,dist/assets/plugins-views-BaBStVok.js). Re-measured here at the repo source,packages/plugin-detail/src/renderers/recordActivityFeed.tsateddc1dd97— the premise holds exactly, minifier names aside:and the call site in
applyFeedConfig:The source even documented the collapse as intended — "an all-unknown list is treated as 'not configured' rather than 'show nothing', so a typo cannot empty the feed silently" — and a test pinned it under the title "a
typeslist of nothing but typos does not silently empty the feed". Both are replaced here.The principle
A sanitiser may narrow an author's request or refuse it, but it must never silently widen it. Widening turns a typo into "show the user everything", which is the one outcome no author asked for — and it hides behind a plausible result. A populated timeline reads as working; an empty one gets investigated. That is why this shipped: a lead page authored
types: ['crm_task'](an object name where a feed kind belongs) and its Activity tab rendered the audit stream for as long as it shipped.undefinedis now reserved for one meaning — notypeskey was authored. An authored filter that keeps nothing returns[], and the call site tests!== undefinedrather than truthiness.Ruled behaviour, with controls
typesomitted['c1','f1','s1']types: ['comment','system']['c1','s1'], no diagnostictypes: [][]types: ['crm_task'](all unrecognised)[], 1 warntypes: ['comment','crm_task'](mixed)['c1'], 1 warnThe recognised vocabulary is derived from the spec's own
FeedItemTypeat runtime and in the tests (SpecFeedItemType.options— 13 values), never hand-typed. A guard asserts that the value the suite calls unrecognised really is outside that vocabulary, so the suite cannot quietly start testing nothing ifcrm_taskever became a declared kind.The diagnostic
Follows the convention #5886 landed in this same file: deduped so one bad kind warns once however many times the feed re-renders, with a test seam to reset (
resetUnrecognisedFeedTypeWarnings). NoNODE_ENVguard — the package carries 8console.warnsites and zero such guards, and the failure being fixed is invisibility.Rather than a second mechanism, #5886's warn plumbing is factored into a shared
warnOnce(bucket, keys, build)used by both sites, with a bucket per channel. The two vocabularies overlap —crm_taskis a plausible unmappedsys_activity.typeand a plausible unrecognisedtypesentry — so a shared dedupe bucket would let whichever fired first swallow the other. A test pins that they stay apart.types: []warns nothing: the request was carried out exactly, and warning about a request that was honoured teaches authors to ignore the channel (the same posture the unknown-activity-type warning already takes).Bounded extension beyond the four ruled rows — declared
A
typesthat is not an array at all (types: 'comment'— brackets dropped) is refused rather than ignored, returning[]with its own diagnostic. This is the same defect class in the same function: the kind is spelled correctly, so vocabulary alone cannot catch it, and ignoring it rendered the whole audit stream — the exact failure this card is about. Leaving it would have left one silently-widening branch inside the function whose contract now says it never widens. Flagged for the PM to rule back if the fence is meant to be literal; it is a one-line change to revert.Reverse verification
Three ablations. Each one's mutation was confirmed on disk by grepping the injected and removed text (never an editor exit code), each ran under a
trap … EXIT INT TERMrestore, and each restored to an emptygit diff HEAD --stat. No rebuild is involved and none is needed: the tests import../recordActivityFeed— a relative in-package specifier that resolves tosrc/, sodist/is not on the path. The A1 leg going red with no rebuild is itself the proof of that.return kept;→return kept.length > 0 ? kept : undefined;types: [], all-unrecognised, the replaced typo pin, and thenormalizeFeedTypesdistinctionif (types !== undefined)→if (types)warnOnce(…, unrecognised, …)→warnOnce(…, [], …)A2 is reported as a null result rather than a guard.
[]is truthy in JS, soif (types)andif (types !== undefined)behave identically given the new sanitiser: the call-site change is intent-clarifying, and every behavioural guard lives innormalizeFeedTypes. Non-discriminating legs elsewhere: both controls and the mixed row stay green under A1 (the mixed row returns a non-empty array, so the ablated branch never runs), and every filtering leg stays green under A3.Verification
All at
582348918, exit codes captured before any pipe, each verdict line quoted from the gate itself. Heavy legs ran through the shared verify lock.npx vitest run …/recordActivityFeed.test.ts …/record-activity.test.tsx --maxWorkers=2→Test Files 2 passed (2)·Tests 61 passed (61)pnpm --filter '@object-ui/plugin-detail^...' build→ lock verdictcommand-exit 0(run first — a fresh worktree has nodist/)pnpm --filter @object-ui/plugin-detail type-check→command-exit 0, script echoed as> tsc --noEmit && tsc -p tsconfig.test.json(not a zero-match no-op)node scripts/check-changeset-presence.mjs→EXIT=0· "✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)"node scripts/check-changeset-no-major.mjs→EXIT=0· "✅ No changeset declares amajorbump."node scripts/check-changeset-fixed.mjs→EXIT=0· "✅ All workspace packages are in the changeset fixed group."node scripts/check-control-bytes.mjs→EXIT=0· "✅ check-control-bytes: OK (scanned 4901 tracked text file(s); skipped 85 binary)." Plus a per-file scan of the diff withgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]': all four clean.node scripts/check-doc-component-types.mjs→EXIT=0· "✅ Every documented component type is registered."node scripts/check-doc-links.mjs→EXIT=0· "Links are valid across 13 scan roots."node scripts/check-doc-snippet-types.mjs→EXIT=0· "Semantic phase: 101 of 101 block(s) judged, 0 failed." First run reported[unbuilt-package]and said "The snippet program was NOT run" — a precondition, not a verdict. Rather than call that environmental, the gate's own--build-filterwas fed toturbo run build … --concurrency=2exactly asdoc-snippet-types.ymldoes (32 successful, 32 total,@object-ui/plugin-detailamong them) and the gate re-run for a real judgement.Lint narrowing, declared and measured. Repo-wide
pnpm lintis CI's run; the narrowing here is a measurement, not a skip:eslint.config.jsscopes every block tofiles: ['**/*.{ts,tsx}']. ESLint itself reports the changeset and the.mdxas "File ignored because no matching configuration was supplied."--format json— 4 paths returned, 2 actually judged (the.tspair), 0 errors / 0 warnings on both.projectService/parserOptions.project/project:→ 0 hits ineslint.config.js), so this diff cannot move the verdict of any file it does not touch.Beyond the narrowing, the affected package's own CI gate ran whole:
pnpm --filter @object-ui/plugin-detail lint(eslint .) →command-exit 0. Its warnings are pre-existing and in files this PR does not touch.Scope
Sanitiser + its call site in
recordActivityFeed.ts, its tests, the changeset, and therecord:activitydocs row that documented the oldtypesbehaviour.ACTIVITY_TYPE_TO_FEED_TYPEis untouched — #5886 settled it. Nothing here touches the storage path;PageComponentSchema.propertiesremains an open bag, which the card itself names as the platform-side half.Behaviour change, stated in the changeset: a page authoring
types: []or an all-unrecognised list now renders an empty timeline where it previously rendered everything. That is the fix, not a regression.Generated by Claude Code