Repository navigation
[finding] sharedUserFeeds keeps a THIRD hand-written sys_activity.type reading, and buckets every unrecognised type as update #6730
Description
Activity
- addeddomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatobjectui ui stream: fix lands on the published library or apps — objectui execution seat
on Aug 29, 2026 huangyiirene commented
on Aug 29, 2026 CollaboratorMore actions分诊:
pm:queue·domain:ui·priority:p2· typeTask(finding已离标)为什么入队而不进决策箱
逐条对人工地板自查:新功能?否。已发布契约变更?否 ——
ActivityItem是 app-shell 内部类型,不是 spec 面。破坏性?否。安全边界?否。门禁削弱?否。⇒ 不命中。卡面提出的「
ActivityItem是FeedItem的投影,还是独立词表」是架构问题,但它落在一个内部类型上,属实现者可判范围。⛔ 但有一条例外,见下。派发第一步就是那个问题,不能跳过
⚠️ 卡面说得对:两个半边都悬在同一个问题上。⇒ dispatch 的第一步是回答它,⛔ 不是先动代码。⭐ 未映射分支那一半有在仓先例,可以直接用:
UNMAPPED_ACTIVITY_FEED_TYPE的做法是通用桶 + 每值一次的诊断,而不是替行做一个具体断言。现状把一切未识别值渲染成update,那是说了一句关于这行的具体的、可能错的话 ——scheduled的会议和作者自定义的contract_countersigned都显示成「更新了」。⇒ 方向由先例给定:未识别值应落通用桶并留一次诊断,⛔ 不应继续冒充
update。实现者只需决定ActivityItem用什么承载那个桶。⛔ 一条停下上报的条件
若实现者的结论是让
ActivityItem成为 specFeedItemType的投影 —— 那就把一个内部类型绑到已发布枚举上,⇒ 变成契约耦合决定,命中人工地板。⛔ 此时停下回帖,不要自行落地。若结论是「保持独立词表、只补通用桶」⇒ 属实现裁量,直接做。
p2 的依据
卡面自陈:今天没有行为丢失,图标比行本身应得的粗糙而已。⇒ 是漂移风险不是用户可感缺陷。
⚠️ 但值得记一句:这是同一列sys_activity.type的第三处手写读法(#5878 合并了两处,#5896 合并了构造器),且它同样漏了scheduled—— 与 #5878 之前控制台记录页漏的是同一个值。⇒ 每多一份拷贝,同一个漂移就重演一次。
Generated by Claude Code
claude commented
on Aug 29, 2026 claudeboton Aug 29, 2026 – with ClaudeContributorAuthorMore actionsClaim + dispatch order —
domain:uiexecution seat, PM sessionsession_01CRJge11jso9TpXRWFt1Z49, round R17.
Branch:claude/issue-6730-shared-user-feeds-activity-type. Oneos-devagent, one worktree.The defect
packages/app-shell/src/hooks/sharedUserFeeds.ts—mapActivityRows(rows)carries a third hand-written reading of thesys_activity.typecolumn, and buckets every unrecognised type asupdate. It feeds the AppHeader bell's Activity tab and Home's inbox. It also carries its own copy of the"NOW()"timestamp quirk, whose two other copies #5896 just folded into one.#5878 shared the table between the
record:activityblock andRecordDetailView; #5896 shared the constructor around it. This one survived both.⛔ The card already tells you the trap
Read it carefully: the card says this is not simply "adopt
activityRowToFeedItem", because the target type differs — this surface builds anActivityItem, not aFeedItem. So the obvious convergence is wrong, and the card is filed as a drift risk rather than a user-visible defect today.⇒ Your first job is to establish what the right shared shape is, not to reach for the existing helper. If the honest answer is that the two target types cannot share a reading without a conversion nobody has asked for, say that and stop — a measured "these should stay separate, here is why" is a complete and valuable outcome for a drift card.
The one thing that is a defect regardless
⭐ Bucketing every unrecognised type as
updateis a silent default. Whatever you conclude about sharing, report what happens today when a newsys_activity.typevalue appears: it is silently displayed as an update. Whether that should be a visible unknown, a dropped row, or a warning is worth a measurement and possibly its own card.Acceptance
- Whatever you change, pin the mapping for every type the column can carry, including the unrecognised case explicitly.
- If you converge the
"NOW()"timestamp quirk with [finding] The sys_activity row -> FeedItem construction is still written twice, and the console surface drops unmapped types silently #5896's folded copy, pin that the fold is behaviour-preserving on this surface. - ⛔ Do not widen into
record:activityorRecordDetailView— [finding] RecordDetailView hand-copies record:activity's sys_activity map, and nothing fails when the two disagree #5878 and [finding] The sys_activity row -> FeedItem construction is still written twice, and the console surface drops unmapped types silently #5896 already own those.
Report whether any test fails when you revert. This is a drift card; if nothing moves, that is the honest finding.
Note on channels: the MCP/GraphQL pool is intermittently rate-limited; repo-scoped REST through the proxy works for reads and writes including PR creation.
/search/issuesand/graphqlare 403 — dedupe via repo-scoped listing plus local grep with a control term.
Generated by Claude Code
claude commented
on Aug 29, 2026 claudeboton Aug 29, 2026 – with ClaudeContributorAuthorMore actionsos-dev-report
{ "issue": 6730, "status": "done", "branch": "claude/issue-6730-shared-user-feeds-activity-type", "pr": "https://github.com/objectstack-ai/objectui/pull/6814", "premise_still_valid": true, "summary": "Answered the card's first question with a measurement, then fixed the one thing that was a defect regardless. THE TWO VOCABULARIES CROSS and must NOT be converged: ACTIVITY_TYPE_TO_FEED_TYPE collapses created/updated/deleted/assigned/shared into one field_change while ActivityItem splits them three ways, and it maps commented/mentioned to undefined (a deliberate exclusion) where ActivityItem makes them the comment kind, so routing this surface through activityRowToFeedItem would cost the bell every comment row and every create/delete distinction; FeedItem is also finer than ActivityItem for system/task/event. Neither type is a projection of the other, so the honest shared artefact is a PIN, not an import: the new suite reads plugin-detail's real table through the devDependency (no runtime edge, and plugin-detail is a PEER dep of app-shell while the bell is not a record-detail surface) and fails if the declared vocabulary grows an entry this side has not read, or if the three disagreements ever stop being true. The reading moved to a new DOM-free layout/activityItemType.ts holding the table, the generic bucket, the NOW() fallback and the row constructor. ActivityItem['type'] gained a fifth kind, 'system': an app-shell-internal generic bucket, NOT a binding to the spec's FeedItemType, so the triage's stop-condition was not hit. system/completed/scheduled/login/logout now land in the bucket instead of claiming 'update'; assigned/shared stay 'update' because both write to the record. An unrecognised value renders through the bucket and is named once on console.warn (UNMAPPED_ACTIVITY_FEED_TYPE's precedent): a bucket, not a drop. MEASUREMENT THE CARD ASKED FOR: today a new sys_activity.type value is displayed as an update, but that is invisible on both surfaces the card names. InboxPopover's Activity tab (layout/InboxPopover.tsx:545) and HomeActivity (console/home/HomeRail.tsx:262) render user/description/timestamp/objectName and NEVER a.type. The only reader of ActivityItem['type'] is the ActivityFeed Sheet, which is exported from the barrel and has ZERO in-repo call sites; there the type drives icon, colour, label and a Record-keyed notification filter that silently drops kinds missing from the record. That is why nothing was ever observed, and why this change cannot regress the bell or Home. Filed as objectui#6816. Scope held: nothing under record:activity or RecordDetailView touched, and the plugin-detail import added is test-only. The NOW() quirk stays a third copy DELIBERATELY: it is the only target-type-independent part of the reading, but activityTimestamp is not on plugin-detail's barrel (#5896 published the whole FeedItem reading on purpose, not its pieces) and importing it would buy one five-line predicate for a peer-dependency edge on the shell's header chrome. No package owns 'how to read a sys_activity column' for both a widget plugin and the shell, so the pin is the instrument until one does. 10 locale packs gained layout.activityFeed.typeSystem.", "tests": "All heavy runs serialized through scripts/pm/os-verify-lock.sh; every verdict quoted from the lock's own VERDICT line, never a piped $?. (1) pnpm --filter '@object-ui/app-shell^...' build, VERDICT command-exit 0. (2) pnpm --filter @object-ui/app-shell type-check && pnpm --filter @object-ui/i18n type-check, VERDICT command-exit 0. app-shell's type-check is `tsc --noEmit && tsc -p tsconfig.test.json`, so the new suites ARE covered by it: the first attempt was VERDICT command-exit 2 with three TS errors in my own test files (TS7006 twice, TS2345), fixed in commit e3cb0dbcc. (3) UNION AT THE FINAL COMMIT e3cb0dbcc: pnpm exec vitest run --maxWorkers=2 packages/app-shell/ packages/i18n/ , VERDICT command-exit 0, 'Test Files 634 passed (634)', 'Tests 6573 passed | 1 skipped (6574)'. (4) New suites: 16 passed (2 files), layout/__tests__/activityItemType-6730.test.ts (pure, node project) and hooks/__tests__/sharedUserFeeds.activityType-6730.test.tsx (end-to-end through useSharedActivityFeed). REVERSE VERIFICATION, direction predicted BEFORE running: restoring the pre-6730 inline reading in mapActivityRows ONLY should turn the e2e suite RED and leave the pure suite GREEN, because the defect was never in a table, it was in the reading done inline around one. MEASURED exactly that: 'Test Files 1 failed | 1 passed (2)', 'Tests 2 failed | 14 passed (16)', with the failure diff reading r5/r6/r7 expected 'system' received 'update' (r5=scheduled, r6=author-extended contract_countersigned, r7=system). Mutation and restore were both CONFIRMED ON DISK, not by exit code: the script carried trap restore EXIT INT TERM with an absolute REPO_ROOT, printed 'on disk: old catch-all lines=1 new-reading references=0', compared blob hashes (HEAD 687fd0125519ba7c436a2db8b1f3d7c015eb9183, then mutated b98ea22867c00959cfc27334d5247f2fcb6098f1, then restored back to 687fd0125519ba7c436a2db8b1f3d7c015eb9183) and asserted 'git diff HEAD' empty afterwards. NO REBUILD WAS NEEDED FOR EITHER LEG, and that is a property of the harness rather than an assumption: vitest.config.mts:289 aliases @object-ui/plugin-detail to packages/plugin-detail/src, and sharedUserFeeds.ts is reached by relative path, so no dist participates in these runs. THE NOW() CASE STAYED GREEN IN BOTH LEGS, which is the card's 'pin that the fold is behaviour-preserving' answered: the pin cannot tell the two implementations apart, which is the point; the equivalence is also asserted directly against plugin-detail's activityTimestamp value for value over the quirk's whole nine-row input table. THE DRIFT-CARD ANSWER TO 'DOES ANYTHING FAIL WHEN YOU REVERT': BEFORE THIS PR, NOTHING DID. No test in the repo referenced mapActivityRows or pinned any part of this mapping, so reverting the whole change against only the pre-existing suites is a no-op. That absence IS the finding the card was filed for. GATES re-derived from the actual diff, each EXIT=0: check:control-bytes, check:i18n-keys, check:i18n-drift, check:phantom-deps, check:self-import, check:vi-mock-specifiers, check:changeset-fixed, check:changeset-no-major. NOT MEASURED (prerequisite, not red, and each says so itself): check:readme-exports needs `pnpm build` for six packages this diff never touches, and app-shell's own entries PASS now that it is built (69 unjudged before the build, 61 after, none of them app-shell); check:eager-closure prints 'This is a broken gauge, not a passing budget' with no apps/console build. LINT WAS NARROWED AND THE NARROWING IS DECLARED, with three pieces of evidence: (a) the population is eslint's own, and the run used --no-inline-config, the same flag the root lint script uses and STRICTER than the per-package `eslint .` CI runs; (b) the count is read from --format json: 6 files, 0 errors, 0 warnings; (c) invariance for untouched files, because eslint.config.js sets no project/projectService and extends tseslint.configs.recommended (not recommendedTypeChecked), so no rule reads another file's types and this diff cannot move a verdict on a file it does not touch; the diff also removes no export, so no importer's resolution changes.", "mcp_calls": "0 — the whole run went through repo-scoped REST via curl (reads, PR creation, issue creation, comment), as the dispatch's channel note described; no MCP GitHub tool was called. Dedupe used the sanctioned fallback: a repo-scoped open-issue listing (253 issues) plus local grep, validated with a control term that must hit (grep for sharedUserFeeds or sys_activity returned #6730 itself), then zero hits for activityfeed / activity feed / activity tab / activity card.", "open_questions": [ { "question": "NON-BLOCKING, no action needed for this PR. No package owns 'how to read a sys_activity column' for both a widget plugin and the shell's chrome, so the target-type-INDEPENDENT part of the reading (the NOW() sentinel fallback) is now a pinned third copy rather than one definition. Should that home exist?", "options": [ "A. Leave it pinned (what shipped). Two copies, one cross-package equivalence test over the quirk's whole input table, zero runtime coupling; the cost is that a future third consumer copies again.", "B. Export activityTimestamp from plugin-detail's barrel and import it in app-shell. One definition; the cost is a runtime edge from the shell's header chrome onto a record-detail widget PEER dependency, and it re-opens #5896's deliberate 'publish the whole reading, not its pieces' barrel decision.", "C. A new shared home for sys_activity column readings that neither package owns. Correct long-term; a real refactor that would have to edit plugin-detail, which this card's fence forbids." ], "recommendation": "A for now, which is what shipped: the pin makes the drift detectable, which is the whole of what the drift card asked for, and B trades a five-line predicate for an install-time requirement on a surface that has nothing to do with record detail. C is the right answer if and when a THIRD consumer of the column appears; it is not worth a refactor for two." } ], "out_of_scope_findings": [ "filed as #6816: ActivityItem.type has no reader. The bell's Activity tab (InboxPopover.tsx:545) and Home's activity card (HomeRail.tsx:262) both ignore it, and ActivityFeed, the component that does read it (icon/colour/label/notification filter), is exported from the barrel with ZERO in-repo call sites. That is why #6730's wrong bucket was never observed: a field with no reader cannot go red. Labelled finding, unassigned." ] }
Generated by Claude Code
claude commented
on Aug 29, 2026 claudeboton Aug 29, 2026 – with ClaudeContributorAuthorMore actions卡的第一个问题被测掉了,而答案是「不该共享」
派发令说:若结论是两个目标类型无法共享,把这个结论交出来就是完整交付。dev 交的正是这个,而且给了让它成立的读数:
⭐ 两套词汇是交叉的,不是包含的。
ACTIVITY_TYPE_TO_FEED_TYPE把 created/updated/deleted/assigned/shared 塌进一个field_change,而ActivityItem把它们分成三类;它把 commented/mentioned 映射为undefined(刻意排除),而ActivityItem把它们当作comment这一类。⇒ 把这个面路由进activityRowToFeedItem,铃铛会丢掉每一条评论行,以及所有 create/delete 的区分。反方向,FeedItem在 system/task/event 上又比ActivityItem细。⇒ 两个类型互相都不是对方的投影,所以诚实的共享物是一条 pin,不是一次 import。这条判断我采纳。
⭐ 而它顺带回答了「为什么没人发现过」
派发令点名要求测「今天新出现一个
sys_activity.type会怎样」。答案比预期更彻底:今天它被显示成 update,但那在卡点名的两个面上根本不可见 ——
InboxPopover.tsx:545的 Activity 页和HomeRail.tsx:262的 Home 活动卡渲染 user/description/timestamp/objectName,从不渲染a.type。唯一读它的ActivityFeed从 barrel 导出,却有零个仓内调用点。⭐ **一个没有读者的字段不会变红。**这解释了这张卡为什么是靠人手撞见的,也正是它被立为 drift 卡而非缺陷卡的原因。已另立 #6816。
同样诚实的是它对「revert 会不会有测试红」的回答:本 PR 之前,一个都不会 —— 仓内没有任何测试引用
mapActivityRows或钉住这个映射的任何部分。那个缺席本身就是这张卡被立出来的理由。⚖️ 裁那个开放问题:采 A(即已交付的形状)
NOW()兜底作为被 pin 住的第三份副本保留,不导出activityTimestamp。理由与它给的一致:pin 让漂移可检测,而可检测正是 drift 卡要的全部;B 用一条五行谓词换来 shell 的头部 chrome 对一个记录详情 widget 的 peer 依赖边,并且重开 #5896「发布整份读法而不是它的零件」的刻意决定;C 在第三个消费者出现时才划算。⚠️ 一处我必须交给维护者,不由本席放行ActivityItem['type']新增了第五个 kind'system'。本席核过:ActivityItem是已发布类型 ——packages/app-shell/src/index.ts:128从 barrel 导出它。⇒ 给一个已发布联合类型加成员,对生产者无害,但会让任何做穷举 switch 的消费者多出一个未处理分支。这落在本席的人工地板上(加宽已发布类型),⛔ 不由本席自行放行。
两件让它不那么重的事,一并说明白:① 真正危险的那个版本 —— 把它绑到 spec 的
FeedItemType—— 是分诊预先设了停止条件的,而 dev 明确避开了,'system'只是 app-shell 内部的通用桶;② 仓内唯一的读者ActivityFeed有零个调用点,所以仓内没有穷举 switch 会因此变红。低摩擦确认:回「可以」即放行;回「不要加类型」则本席让 dev 把 system/completed/scheduled/login/logout 归到某个既有 kind 并重新钉。⛔ 在收到答复前本 PR 不入合并队列 —— 它现在是 draft,自然处于按住状态。
⚠️ 其余部分本席不阻断:停止把未识别类型冒充update、改为具名桶 + 一次console.warn,是这张卡里无论如何都成立的那条缺陷,派发令也是这么写的。
Generated by Claude Code
Observed while implementing objectui#5896 (the console record page's
sys_activityrow toFeedItemconstruction). Out of that card's fence and not touched by its PR: different target type, different surface, and no drop — so it is a drift risk rather than a user-visible defect today.The third reading
objectui#5878 shared the
sys_activity.typetable between therecord:activityblock andRecordDetailView; objectui#5896 shared the constructor around it. A third hand-written reading of the same column survives in a different package surface:packages/app-shell/src/hooks/sharedUserFeeds.ts--mapActivityRows(rows), which feeds the AppHeader bell's Activity tab and Home's inbox:It also carries its own copy of the
"NOW()"timestamp quirk, five lines above -- the same quirk whose two other copies objectui#5896 just folded into one.Why it is NOT simply "adopt
activityRowToFeedItem"The target type differs. This surface produces
ActivityItem(four kinds:comment/delete/create/update), notFeedItem(the closed 13-valueFeedItemTypespec enum). So the shared constructor cannot be dropped in, and converging would first need a decision about whetherActivityItemis a projection ofFeedItemor an independent vocabulary.What is worth deciding
update. That is a specific claim about the row ("something was updated"), which is a different posture fromUNMAPPED_ACTIVITY_FEED_TYPE's deliberately genericsystembucket plus a once-per-value diagnostic. Ascheduledmeeting and an author'scontract_countersignedboth show the update presentation, with nothing said.scheduledis missing here too, exactly as it was missing from the console record page before objectui#5878 -- the same drift, in the copy nobody has converged yet.assigned/shared/system/completedlikewise all land onupdate.No behaviour is lost today, which is why this is filed rather than fixed: nothing vanishes, the icon is just coarser than the row deserves, and picking the right convergence needs the
ActivityItemvsFeedItemquestion answered first.Filed unassigned for triage.
Generated by Claude Code