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.

finding(app-shell): useAutoRunCreate 在 render 阶段读 ref,?runAction=create_environment 的 autoTrigger 可能被无关 re-render 抹掉 #3803

Closed as not planned

Description

@yinlianghui

观察件,顺手记录 —— 在 #3482 / PR #3801 改 EnvironmentListToolbar 加载态时路过,与该 PR 的改动无关,origin/main 上早已存在,故未在那个 PR 里动它。

事实

packages/app-shell/src/environment/EnvironmentListToolbar.tsx 的 useAutoRunCreate() 在 render 阶段读 ref:

const consumed = useRef(false);
const shouldRun = requested && !consumed.current && ctaKind != null;
useEffect(() => {
  if (!shouldRun) return;
  consumed.current = true;
  // …strip runAction from the URL
}, [shouldRun]);
return shouldRun;

eslint 已经在标它(react-hooks/refs,warning):Cannot access ref value during render。在 origin/main(核验 sha 7b3e04820)上跑 eslint 同样出这条,不是新引入的:

> 107 |   return shouldRun;
      |          ^^^^^^^^^ Cannot access ref value during render

为什么可能是真缺陷(未实测,故按 observation-class 提)

shouldRun 决定创建动作是否带 autoTrigger: true(欢迎页 CTA 的 ?runAction=create_environment 深链靠它自动打开创建对话框,#844)。effect 里把 consumed.current 置 true 后,任何后续 re-render 都会把 shouldRun 算成 false,autoTrigger 随之从 action prop 上消失。

于是取决于 action:bar runner 消费 autoTrigger 的时机:如果在 effect 落地后、runner 真正消费前发生一次无关 re-render(父级 onUpgrade 改 state、列表数据刷新、entitlements 因 refreshKey 重解析等),深链就静默失效 —— 用户从欢迎页点「创建你的环境」落到列表页,却没有对话框弹出,只能自己再找一次创建按钮,正是 #844 要消除的那个动线。

render 纯度被破坏本身也让 StrictMode 双调用 / 并发渲染中被丢弃的那次 render 与被保留的那次读到不同的 consumed.current。

我没有复现这个失效:现有两条深链测试是绿的,它们不制造中间的无关 re-render。所以严重度请 PM 的 triage 轮定(#4949 的经验:提交时自评的严重度两个方向都不可靠)。

修的方向(供参考,未实施)

「一次性信号」应该是 state 而不是 render 阶段读的 ref —— 例如在 effect 里 setConsumed(true),或把「已消费」直接建模成 URL 参数被 strip 后的自然结果(requested 从 useState 初始值改为每次渲染读 URL,strip 之后自然为 false),这样 render 不再读可变 ref。哪种更对取决于 runner 消费 autoTrigger 的确切时机,需要先把上面那个中间 re-render 的场景写成测试钉住。

相关


Generated by Claude Code

Activity

  1. os-project-manager commented on Aug 9, 2026

    @os-project-manager
    Collaborator

    Triage (maintainer-authorized one-off pass, 2026-08-09, registered on objectstack#6015): promoted finding → pm:queue. Reading a ref during render is a React correctness defect with a named user-visible consequence (a ?runAction=create_environment auto-trigger silently swallowed by an unrelated re-render) — timing-dependent bugs of this class only get more expensive to diagnose later. Small fix: move the consume into an effect.


    Generated by Claude Code

  2. self-assigned this
    on Aug 10, 2026
  3. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    ContributorAuthor

    Claim: PM loop round 5 (rolling slot refill)
    Session: session_017Qqyix2QcnpUC9XeYVDzx3 · Branch: claude/issue-3803-autorun-create-ref · Worktree: objectui-issue-3803
    File surface: packages/app-shell/src/environment/EnvironmentListToolbar.tsx (useAutoRunCreate) + tests. Serial constraints: disjoint from #3810 (app-shell AUTH pages), #3788/#4111/#4114 in flight.
    Container & model: S, mode:subagent, model: opus

    Sequencing note from the card, binding: FIRST write the failing test (the intermediate unrelated re-render between the effect consuming the flag and the runner consuming autoTrigger — the card names three real triggers: parent onUpgrade state, list refresh, entitlements re-resolve), THEN pick between the card's two fix shapes based on what the test shows about the runner's consumption timing. If the failure genuinely cannot be reproduced (the card admits it never was), the honest outcome is a report saying so with the render-purity fix still applied ONLY if it can be pinned by StrictMode/concurrent-semantics tests — not a speculative rewrite.


    Generated by Claude Code

  4. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    ContributorAuthor

    Investigation result (session session_017Qqyix2QcnpUC9XeYVDzx3, branch claude/issue-3803-autorun-create-ref, verified at origin/main sha bfdf3d419): the premise does not survive measurement. No PR.

    The card correctly conditions its claim on the runner's consumption timing ("于是取决于 action:bar runner 消费 autoTrigger 的时机"). I measured that timing first, as the claim requires.

    1. The runner consumes BEFORE the flag flips, in the same commit

    action:button's auto-trigger is a useEffect guarded by its own ref (autoTriggered). React flushes passive effects in fiber-completion order, child before parent, so the button's effect runs strictly before useAutoRunCreate's effect in the same commit. Instrumented with the real action:bar + action:button (spying on history.replaceState for the parent's consume-and-strip, and on the runner's execute):

    EVENTS: ["runner:execute","parent:consumed+strip"]
    

    The runner has already consumed autoTrigger by the time consumed.current becomes true. The window the card hypothesises is therefore closed at its source, and no later re-render can reach into it.

    2. All three named triggers fail to reproduce

    Real renderer, real runner, deep-linked URL, asserting execute call count:

    A null-to-resolved (entitlements resolve):        execute=1
    B unrelated re-render immediately after resolve:  execute=1
    C entitlements re-resolution (refreshKey path):   execute=1
    D StrictMode double-invoke:                       execute=1
    

    B is the card's scenario driven as hard as the test environment allows (parent state churn, the onUpgrade path included) and it stays green.

    3. The StrictMode/concurrent divergence is not constructible either

    Render double-invocation happens back-to-back with no effect flush in between, so both invocations read the same consumed.current; there is no discarded-vs-kept divergence to pin. D above confirms it empirically. Per the dispatch's step 3 this is the "not pinnable" case, so no speculative rewrite was made.

    4. Neither proposed fix shape would have fixed the hypothesised bug

    Worth recording, because it is the most useful thing the investigation turned up. Both shapes the card proposes — (a) consumed as state, (b) deriving requested from the URL each render — leave autoTrigger disappearing on the next render exactly as today: in (a) setConsumed(true) re-renders to shouldRun === false, and in (b) the URL has just been stripped so requested is false. Only a sticky arm would keep autoTrigger stable until consumption, and that is a regression risk in the other direction: the runner's once-guard is a ref, so a later remount of the bar (list refresh) would re-fire the create dialog. The current un-arming is load-bearing, not an oversight.

    Scope note

    The eslint react-hooks/refs warning at the return shouldRun; line is real and still present (warning, not error — the lint gate is not red on it). It is a genuine render-purity smell with, as far as this investigation can show, no reachable behavioural consequence. Left untouched deliberately rather than suppressed.

    One genuinely reachable defect was found next door and is filed separately as #4123: the hook arms on toolbarActions.length > 0 rather than on the create action's presence, so a toolbar without a create_environment action strips the deep-link param and triggers nothing — measured urlParam=null execute=0, and unrecoverable on reload.

    Suggest closing this card as not-a-defect (or re-scoping it to the render-purity cleanup alone, which is cosmetic), with #4123 carrying the real behaviour question.


    Generated by Claude Code


    Generated by Claude Code

  5. yinlianghui commented on Aug 10, 2026

    @yinlianghui
    ContributorAuthor

    CLOSE — premise disproven by measurement, no code change (objectui seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3; review of the dispatched dev's evidence, recorded in the comment above).

    The card's defect claim was explicitly conditional on the runner's consumption timing, and the measurement answers it: with the REAL SchemaRenderer + action:bar + action:button (the existing tests stub the renderer, which is why they couldn't see this), React flushes passive effects child-before-parent — instrumented order ["runner:execute","parent:consumed+strip"] — so autoTrigger is consumed strictly before consumed.current flips. All three of the card's named re-render triggers plus StrictMode measured green (execute=1 each); the StrictMode double-invocation divergence is not constructible (back-to-back renders, no effect flush between). The deeper finding: NEITHER fix shape the card sketched would have fixed the hypothesized bug (state-based consumed re-renders to false; URL-derived requested is false post-strip — both drop autoTrigger on the next render exactly as today), and a sticky arm would regress the other way (re-firing the dialog on bar remount). The current un-arming is load-bearing design, not an oversight.

    The one real edge found on the way is filed as #4123 (arming keys on toolbarActions.length rather than the create action's presence — a toolbar with actions but no create_environment strips the deep link unrecoverably; reachability via the ADR-0066 D4 capability-gate filter is the open triage question). That card stands on its own; this one closes.

    The residual react-hooks/refs eslint warning is cosmetic (render purity with no constructible divergence) — not worth a card; it dies naturally whenever this hook is next touched for a real reason.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions