Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

bug(plugin-timeline): colorFieldLadder-7243 的 rung 2/3 在同一个 it 里 render 两次而不 unmount,共享模块级 lastItems —— 负载高时第二个断言收到第一个的值,已踢掉合并队列 #7521

Closed

Description

@os-project-manager

从 PR #7514 被合并队列以 CI_FAILURE 踢出而查出。⚠️ 这不是那个 PR 的失败 —— 它删的是 packages/components/src/SchemaRenderer.tsx,与 plugin-timeline 无关。这是一条真实的竞态,不是 flake,而且它现在正在阻塞合并队列。

定 p1:失败模式是随机踢掉任意 PR 的队列条目,与该 PR 的内容无关。承重性,不是严重性。

实测的失败

合并队列分支 gh-readonly-queue/main/pr-7514-fe4e7a9e8…,workflow run 33776772672,job Test (shard 3/4)。615 个测试文件里只有 1 个红,8048 通过:

FAIL  dom  packages/plugin-timeline/src/ObjectTimeline.colorFieldLadder-7243.test.tsx
  > objectui#7243 — timeline colorField ladder (control: green before and after)
  > rung 2: 3- and 6-digit hex literals pass through

AssertionError: expected [ '#abc' ] to deeply equal [ '#123456' ]
  - Expected   "#123456"
  + Received   "#abc"
  ❯ ObjectTimeline.colorFieldLadder-7243.test.tsx:109:72

⭐ 第二个断言收到了第一个断言的值。 这不是「颜色算错了」,是上一次 render 的结果泄漏了过来。

根因,按内容读出

:34  import { render, waitFor } from '@testing-library/react';   // ⛔ 没有 cleanup,没有 afterEach
:38  let lastItems: any[] = [];                                   // 模块级共享
:40  vi.mock('./renderer', () => ({ ... lastItems = schema.items ?? []; ... }))

async function colorsFor(colorField: string, rows: any[] = [ROW]) {
  lastItems = [];                                                // 重置共享状态
  ...
  render(<ObjectTimeline schema={schema} dataSource={makeDataSource(rows)} />);   // ⛔ 从不 unmount
  await waitFor(() => expect(lastItems.length).toBe(rows.length));
  return lastItems.map((i) => i.color);
}

而 rung 2 与 rung 3 各自在同一个 it 里调用它两次:

it('rung 2: 3- and 6-digit hex literals pass through', async () => {
  expect(await colorsFor('accent', [{ ...ROW, accent: '#abc' }])).toEqual(['#abc']);
  expect(await colorsFor('accent', [{ ...ROW, accent: '#123456' }])).toEqual(['#123456']);
});

it('rung 2: rgb() and hsl() literals pass through', async () => {   // 同一形状
  expect(await colorsFor('accent', [{ ...ROW, accent: 'rgb(1, 2, 3)' }])).toEqual(['rgb(1, 2, 3)']);
  expect(await colorsFor('accent', [{ ...ROW, accent: 'hsl(1 2% 3%)' }])).toEqual(['hsl(1 2% 3%)']);
});

testing-library 的自动 cleanup 只在 afterEach 跑,不在同一个 it 内的两次 render 之间。⇒ 第二次调用时,第一个组件仍然挂载着,而它的 find() promise 还在飞。

时序:

  1. 第一次 colorsFor render 组件 A(#abc),A 的 find() 已 resolve,lastItems = ['#abc'],断言 1 通过 ✓
  2. 第二次 colorsFor 执行 lastItems = [],render 组件 B(#123456)
  3. ⚠️ 组件 A 因某次重渲染再次写入 lastItems = ['#abc'](它还活着,mock 的 renderer 每次渲染都写)
  4. waitFor 只检查 lastItems.length === 1 —— 达标,立刻返回
  5. 返回 ['#abc'],断言 2 失败

⇒ 这个 waitFor 的谓词只看长度、不看内容,所以它无法区分「B 画好了」和「A 又写了一次」。在 CPU 空闲时 B 通常先到,测试就绿;队列 CI 跑 615 文件 / 772 秒,负载高,A 的重渲染插了进来。

⛔ 因此这不是 flake。 「flake」意味着没有确定的成因;这里成因是确定的:两个组件同时活着,共享一个模块级数组,而等待谓词分辨不出是谁写的。

建议补丁(未验证,实现者须自行测量)

最小且直击成因 —— 在每次 render 前卸载上一次:

-import { render, waitFor } from '@testing-library/react';
+import { render, waitFor, cleanup } from '@testing-library/react';

 async function colorsFor(colorField: string, rows: any[] = [ROW]) {
+  cleanup();          // 卸载上一次 render,杜绝并存组件写同一个 lastItems
   lastItems = [];

⚠️ 但只做这一步不够,因为它没有修掉那个分辨不出作者的等待谓词。真正的加固还应当让 waitFor 断言内容而不只是长度 —— 否则下一个在同一个 it 里 render 两次的人会再次踩到。请两条都做,并说明理由。

⛔ 不得把两个断言拆成两个 it 就算完 —— 那只是把竞态挪到 afterEach 的 cleanup 上碰运气,成因未除,而且这个文件里还有别的多 render 的 it。请普查全文件,每一处多次 render 的 it 都要处理。

可逆验证的要求

这条缺陷是负载依赖的,所以「跑一遍绿了」不构成证据。要求:

  • 先复现红:在未修改的树上构造出确定性的失败 —— 例如让第一个组件的 find() 延迟 resolve,或直接断言两次 render 之后 DOM 里有两个 data-testid="timeline-renderer"(这一条是成因的直接证据,且与时序无关)。⭐ 那个 DOM 计数就是最好的 pin:它把「两个组件同时活着」从时序问题变成了结构事实。
  • 修完之后同一个探针必须变绿,并留一个会点亮的对照。
  • ⛔ 不得跳过、禁用或隔离这个测试。

影响面

⚠️ 这一条踢掉的是 PR #7514,但它与 #7514 无关 —— 它会踢掉任何恰好在负载下入队的 PR。在它修好之前,合并队列的失败都要先排除这一条,否则会被误判成入队 PR 自己的问题。

Refs: #7243(该 ladder 的原始卡)· PR #7514(被它踢出队列的第一个已知受害者)

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    and removed on Sep 3, 2026
  2. os-project-manager commented on Sep 3, 2026

    @os-project-manager
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 7521,
      "status": "done",
      "branch": "claude/issue-7521-colorfieldladder-race",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7527",
      "premise_still_valid": true,
      "summary": "CAUSAL CHAIN (re-derived, agrees with the card and names the engine the card did not): lastItems is module-level, the vi.mock renderer writes it every render, colorsFor never unmounted, and the waitFor predicate read lastItems.length only. What makes the late write GUARANTEED rather than incidental is that ObjectTimeline's data effect lists objectDef in its deps while a separate metadata fetch sets objectDef, so every mount issues TWO find() calls; measured trace shows 'FIND phase=A #2' printed BEFORE 'waitFor resolved', i.e. each colorsFor returned with a write still queued against a component nothing unmounted. A gated repro forced the interleaving and reproduced the CI failure verbatim: 'RENDER phase=B len=1 color=[\"#abc\"]' then 'B predicate (length-only) resolved with [\"#abc\"]'. PROBE: the card's suggested shape (assert TWO renderers alive after two colorsFor) is NOT reliably red - in the very run where the bug bites, the length-only predicate resolves on the stale write before the new component paints, so only ONE element is in the DOM (measured: the failing matrix cell reports alive=1). Used the inverse instead: assert BEFORE each mount that ZERO renderers are alive. That is timing-independent - at the top of the 2nd colorsFor the first component's element is a settled DOM fact (it rendered, since only it could satisfy the previous waitFor) and RTL auto-cleanup runs in afterEach, never between two renders inside one it. On the UNMODIFIED tree its red is NECESSARY, not probabilistic: 'expected [...] to have a length of +0 but got 1', Tests 2 failed | 5 passed (7), red on exactly the two multi-render its. FIX 1 (unmount in a finally) removes the second writer. FIX 2 (per-call token stamped on titleField, predicate waits for lastItems to carry it) makes the predicate identify the AUTHOR; the token is deliberately not the value under test, so it does not smuggle the assertion into the waiter. HONEST DEVIATION FROM THE CARD: a 2x2 ablation on a deterministic race gate measured that EACH fix alone closes the correctness hole - no-unmount+length-only is WRONG ([\"#abc\"], alive=1), unmount+length-only correct, no-unmount+token correct (alive=2), both correct (alive=1). So the card's 'neither alone is enough' does not hold as stated and I did not manufacture a red I never got; both are still kept as defence in depth, with the structural probe pinning the unmount mechanically. My FIRST matrix control did NOT light up (20ms gap let the new component overwrite the stale one before waitFor's first 50ms poll) - repaired to an 800ms gap until the pre-fix cell actually failed, rather than reading 'all CORRECT' as 'no bug'. CENSUS of all 7 its (colorsFor calls): 1,1,2,2,1,1,1 - exactly two multi-render, matching the card; fix lives in the shared helper so all seven are covered. Nothing skipped/disabled/retried, no its split, no ObjectTimeline product code touched.",
      "tests": "All at final HEAD 4e69c05f5. (1) pnpm exec vitest run packages/plugin-timeline/ -> 'Test Files 22 passed (22)' / 'Tests 287 passed (287)', VITEST_EXIT=0. (2) tsc -p tsconfig.test.json --noEmit -> TSC_EXIT=0, AFTER building the dep closure (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-timeline^...' build); before that build it was exit 2 with 25 errors, ALL 'Cannot find module @object-ui/*' and ZERO in my file. NOT-MEASURED check done: package tsconfig.json excludes tests, but type-check also runs tsconfig.test.json which includes src/**/*.test.tsx, and --listFiles confirms the edited file IS in that program (1 hit), so the green is real not vacuous. (3) PRE-FIX RED, probe only, no fix applied: 'AssertionError: expected [...] to have a length of +0 but got 1', Tests 2 failed | 5 passed (7) - red on exactly the two multi-render its, green on the other five. (4) POST-FIX: Tests 7 passed (7). (5) ABLATION MATRIX (2x2, deterministic gate, mutation confirmed on disk by grep counts of the injected/removed anchors before each run; the first control did not light up and was repaired): no-unmount+length-only -> second=[\"#abc\"] alive=1 WRONG; unmount+length-only -> [\"#123456\"] alive=1; no-unmount+token -> [\"#123456\"] alive=2; unmount+token -> [\"#123456\"] alive=1. (6) node scripts/check-changeset-presence.mjs -> exit 0, verdict line: 'Every one of them has an EMPTY frontmatter - declared as releasing nothing, which is the explicit exemption'; check-changeset-no-major.mjs -> exit 0. (7) eslint on the changed file -> 0 errors, exit 0; warnings 6 before vs 6 after, all pre-existing no-explicit-any (compared against the BASE version of the same file). All heavy runs went through /home/user/objectstack/scripts/pm/os-verify-lock.sh with OS_VERIFY_LOCK_SLOT=issue-7521; exit codes captured to file BEFORE any pipe. NOTE: the tsc+vitest run was sequenced with ';' so the wrapper printed 'batch-last-exit ... NOT A VERDICT ON THE WHOLE COMMAND' - I therefore rely on the two per-part exits I captured explicitly (TSC_EXIT=0, VITEST_EXIT=0), not on that line. NARROWING DECLARED: repo-wide pnpm lint left to CI - eslint is not type-aware here (no projectService, no parserOptions.project), so this diff cannot move the verdict on any untouched file. Readings (1)(2) were taken at e1331a952; the only later commit adds .changeset/7521-*.md and moves 0 non-.changeset paths (git diff --name-only e1331a952..4e69c05f5), so they hold at 4e69c05f5.",
      "mcp_calls": "4",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Same-shape census (task 4) — nothing to file. Scanned all 2451 tracked test files for: module-level let + written inside a vi.mock factory + an it rendering more than once + no unmount()/cleanup().

    (a)+(b) module-level `let` written inside a vi.mock factory : 47
      +(c) AND an `it` that renders more than once              : 1
      +(d) AND no unmount()/cleanup() anywhere in the file       : 0
    

    The single step-(c) file is this one. The scanner was validated against the pre-fix version of this file as a positive control and lit up correctly (bindings=['lastItems'], both multi-render its named, teardown=False), so the zero is a reading rather than a query-shape artifact — an unlit zero would not have counted. No other file in the repo carries this shape, so no follow-up issue was filed.

    Generated by Claude Code


    Generated by Claude Code

  3. os-project-manager commented on Sep 3, 2026

    @os-project-manager
    CollaboratorAuthor

    席内核验:通过 —— 而且它在三处纠正了本席,两处是我写进卡片的东西错了

    domain:ui 执行席,session session_01EMrWaQw3XS5DxTHxp4yRyC。PR #7527,head 4e69c05f5。承重读数自测。

    ⭐ 纠正一:本席建议的探针,在最需要它红的那一次不会红

    我在卡片里建议「两次 colorsFor 之后断言 DOM 里有两个 data-testid="timeline-renderer"」,并说它「与时序无关 —— 把『两个组件同时活着』从偶发时序变成结构事实」。

    实测:不成立。 在 bug 真正咬人的那一次运行里,length-only 谓词在旧组件写入时就 resolve 了,而新组件还没画 ⇒ DOM 里只有一个元素。它的 2×2 矩阵里那个失败格逐字报 alive=1。

    ⇒ 我的探针问「有没有两个」,而这依赖新组件已经画出来 —— 那正是时序。

    它把方向反转了,在每次 mount 之前断言没有 renderer 活着:

    expect(screen.queryAllByTestId('timeline-renderer')).toHaveLength(0);

    这才真正与时序无关:在第二次 colorsFor 的开头,第一个组件的元素在 DOM 里是既成事实 —— 它必然渲染过(否则前一个 waitFor 不可能满足),而 RTL 的自动 cleanup 只在 afterEach 跑、不在一个 it 内两次 render 之间。前修读数,精确红在那两个多 render 的 it 上:

    AssertionError: expected [ ... ] to have a length of +0 but got 1
    Tests  2 failed | 5 passed (7)
    

    ⭐ 教训比这张卡大:「断言坏状态存在」依赖坏状态已经成形;「断言好状态成立」只依赖已经发生的事。 我给的是前者,对的是后者。

    ⭐ 纠正二:本席写的「两条修复各封一个口,只做一条不够」—— 实测不成立

    我在单子里写「⛔ 只做第一条不够,⛔ 只做第二条也不够」。它做了 2×2 ablation,在一个确定性的竞态门上测:

    unmount predicate 第二次返回 alive
    no length-only ["#abc"] 1 错 —— 复现 CI 失败
    yes length-only ["#123456"] 1 正确
    no token ["#123456"] 2 正确
    yes token ["#123456"] 1 正确(交付)

    ⇒ 每条单独都关掉了实测场景里的正确性洞。 它的原话:"the card's 'neither alone is enough' does not hold as stated, and I am not going to claim a red I did not get."

    ⭐ 拒绝声称一个自己没得到的红 —— 即使那个说法来自派发单。这正是本轮反复要的东西,而这次是对着我说的。它两条仍都保留,但理由改成各自不同的价值(unmount 去掉第二个写者,alive 2→1;predicate 在两组件并存时拒绝陈旧写入;结构探针机械地钉住 unmount),而不是我那句实测为假的断言。

    ⭐ 纠正三:它命名了卡片没命名的机制 —— 为什么迟到的写入是必然而非偶然

    本席自测证实:

    ObjectTimeline.tsx:176   const [objectDef, setObjectDef] = useState<any>(null);
    ObjectTimeline.tsx:254   }, [schema.objectName, dataSource, …, objectDef, …]);   ← data effect 依赖数组含 objectDef
    

    data effect 依赖 objectDef,而另一个 effect 会 setObjectDef ⇒ 每次挂载必然发两次 find(),第二次在 length-only 谓词变绿时仍在飞。它的实测 trace:

    FIND phase=A #1
    RENDER phase=A n=1 len=1 color=["#abc"]
    FIND phase=A #2                    ← 第二次已发出
    >>> waitFor resolved, returning ["#abc"]   ← 而它还在飞
    

    ⇒ 卡片(本席写的)说「负载高时 A 的重渲染插了进来」,把它当成偶发。真相是每一次调用都留下一个排队中的写入,负载只决定它什么时候落地。前者读作「概率问题」,后者读作「结构问题」—— 处置相同,但对下一个读者的意义完全不同。

    其余核验

    • 它的第一个对照没点亮(间隔太短,新组件在 waitFor 首次轮询前就覆盖了陈旧值),它修到点亮为止,而不是读成「没 bug」。
    • 全 it 普查:七个逐一列出 colorsFor 调用次数,两个是 2 次。修复落在共享 helper 上,所以七个全覆盖。⛔ 未拆 it、未 skip、未动产品代码 —— 围栏守住。
    • 同形普查:2451 个测试文件,(a)+(b) 模块级 let 且在 vi.mock 工厂里被写 = 47;+(c) 且有 it 多次 render = 1(就是本文件);+(d) 且全文件无 unmount()/cleanup() = 0。⭐ 扫描器用修复前的本文件做阳性对照并点亮 ⇒ 那个零是读数。⇒ 无后续卡可立。

    Clause-② no 复核确认:只动一个测试文件 + 空 frontmatter changeset。

    处置

    CI 于 17:14 起跑。全绿即剥 draft 放行。Fixes #7521。

    ⚠️ 这一条落地后,合并队列才真正脱离那个随机踢条目的风险 —— 本轮它已确实吃掉 #7514 一次(重试才过),另有 #7473 出队原因未查(⛔ 不归因)。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatchedpriority:p1

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions