Repository navigation
fix(cli): break Ink useBoxMetrics commit-phase setState loop (React #185) - #11565
Conversation
|
Also affected on Windows, on 0.23.3 (auto-updated) — so this is not 0.23.2-specific.
Data point on versions: 0.23.2 and 0.23.3 ship the same chunk ( Env: custom |
|
Thanks for the PR — the direction is right and the loop guard is the correct place to break the cascade. Two findings in review are real blockers, both from the guard being module-scoped rather than per-instance, and both are confirmed by this repo's own suite. R1-1 (must fix) — module-scoped budget starves measurement. const guardRef = useRef({ count: 0, timer: null });and read/advance R1-2 (must fix) — the R1-3 (please apply) — the functional R1-5 (please apply) — regenerate with The two Criticals are the gate; the rest can ride along in the same push. Happy to re-review once the guard is per-instance and the patch is regenerated. |
The guard added to the vendored Ink patch kept its 32-entry budget in module scope, so every useBoxMetrics instance shared one budget, and the budget counted invocations rather than oscillation cycles. With 40 boxes mounting in a single commit, instances past the 32nd invocation never measured: hasMeasured stayed false for good, because no later commit re-fires their layout listener. Hold the budget in a ref so each instance refills on its own. Cancel the pending budget-refill timer on unmount. Vitest fakes setImmediate by default and vi.useRealTimers() discards a pending faked immediate, so the stored handle stayed non-null after the timer was gone and the counter never refilled. Drop the functional same-value bail for setHasMeasured. The useState setter already bails for an identical primitive, and the functional form cannot be eagerly bailed - React has to schedule the render to evaluate the updater. Reverted to upstream's setHasMeasured(Boolean(ref.current)). Regenerate patches/ink+7.0.3.patch with patch-package instead of hand-appending the hunk, so it lands in path order with an index line and function-context headers. The regeneration also reformats the pre-existing hunks (index hashes, hunk-context labels, offsets); that churn is cosmetic, and the only content change outside the new hunk is one "\ No newline at end of file" marker. Adds packages/cli/src/ui/use-box-metrics-loop-guard.test.tsx, which renders the real hook and pins three behaviours: an oscillating box settles without React QwenLM#185, a later resize still re-measures once an oscillation tripped the guard, and 40 instances all measure within one commit.
|
Picked this up: the branch had been idle for a day and this is a P1 crash that kills the session. I kept your design and pushed Three changes to the guard: Per-instance budget. Cancel the refill timer on unmount. The pending Drop the functional same-value bail for Regenerated the patch with Regression test at
The oscillation case throws out of the commit phase through the layout listener path, the same stack as the #11500 report, so it exercises the reported path rather than a synthetic one. Suites with the pushed commit — |
|
I'm hitting this crash on 0.23.2 very reliably — the session dies when background agents finish: their notification goes to the model, and everything goes down on the reply. Since it reproduced so easily, I spent a couple of hours digging, and the root cause turned out to be outside useBoxMetrics entirely. History item ids are minted as What fixed it for me — make ids strictly monotonic in const id = Math.max(baseTimestamp + counter, lastId + 1);(thread Some reports in #11500 may take a different path to the same #185, but the id collision is a shared hazard either way — worth checking your captures for duplicate keys. |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Round-2 head 4d568d9 verified independently — no blockers from me.
What I checked locally on this head:
npm installapplies the patch to pristine ink@7.0.3 (ink@7.0.3 ✔); the new hunk sits in generator order atpatches/ink+7.0.3.patch:206with anindexline and function-context headers like the rest of the file.- R1-1/R1-2/R1-3 confirmed fixed in the applied hook: the budget is a per-instance
useRef, the refill handle isclearImmediated on unmount, andsetHasMeasuredis back to upstream's plain form. - Suites on this head: the new loop-guard file 3/3,
VirtualizedList+Footer66/66,ink-cursor-rendering28/28.tsc --noEmitand eslint on the new test file are clean. - One scenario the suite doesn't pin: several oscillators at once. I mounted three simultaneous
OscillatingBoxinstances — the cascade settled (each instance stopped at 18 renders, app fully quiescent after). React'snestedUpdateCountincrements once per nested commit (react-reconcilercommitRootImpl), so the per-instance cap of 32 stays below the 50 limit no matter how many instances oscillate in the same tick.
On the open R2 suggestions — all non-blocking, my read:
- R2-3 is the one I'd do before merge. The PR body still documents the round-1 module-scoped design and the removed
setHasMeasuredbail. This patch file is supply-chain-classified, so the body is the merge record — worth one edit. - R2-1 reproduces: the 40-instance case prints
MaxListenersExceededWarningon the realprocess.stdoutbecauseObject.create(process.stdout, …)inherits its_events. Giving the fake stdout its own EventEmitter state would keep the test file quiet. - R2-5 (refill never latches, so an oscillating box next to a steady commit source re-runs a bounded cascade every macrotask) is real, but it's a CPU-damping question, not a crash question — fine as a follow-up.
- R2-2/R2-4/R2-6 are fair extra pins; the mechanisms they'd cover are correct as written.
Round-1 findings (R1-1 module-scoped budget, R1-2 unmount timer leak, R1-3 redundant bail, R1-4 unpinned guard, R1-5 hand-appended hunk) were all addressed in 4d568d9 and confirmed fixed by the round-2 review (no blockers) plus independent maintainer verification. Superseded; dismissing to unblock.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approving. Transparency: I authored the round-2 head 4d568d9 (per-instance guard, unmount cleanup, regenerated patch) during closeout, so this approval covers design + verification, not a fresh read of my own commit — the design is @zaalipro's.
Why now: (1) Round-1 Criticals R1-1/R1-2 confirmed fixed by the round-2 re-review (no blockers) and by @qwen-code-dev-bot's independent verification on this head (patch applies to pristine ink@7.0.3, 40-box case green, multi-oscillator cascade settles). (2) I just merged origin/main into the branch to clear the lint-gate freshness check (main touched eslint.config.js + eslint.legacy-filenames.mjs since the branch base); the loop-guard suite still passes on the merged tree. (3) @vetaljanos's duplicate-key finding (comment below) is a separate input-data driver — a loop guard cannot fix broken keys, but it throttles any commit-phase cascade regardless of driver, including crashes with no notification in flight. The id fix lands in its own PR; this guard ships as defense-in-depth, not as the root fix. PR body updated to match the shipped design.
Remaining R2 suggestions (refill/latch semantics, test pinning) are real but non-blocking; happy to see them as follow-ups.
|
@vetaljanos's duplicate-key hypothesis checks out — verified against main ( Verification summary:
Positioning vs this PR: a loop guard cannot repair broken input data, but it throttles any commit-phase cascade regardless of driver — including the crashes @zwj-lambda saw with no completion notification in flight. Complementary, not competing: this PR ships as defense-in-depth, #11743 removes the confirmed driver. Thanks for the careful field work — "died within a couple of minutes before, never since" plus the id-math made this findable. |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Holding this until a deterministic CI hang introduced on this branch is explained. I'm moving my earlier approval to request-changes for process hygiene — the evidence below landed after that approval.
Symptom (2/2 runs, deterministic): Test (ubuntu-latest, Node 22.x) was cancelled at exactly the 60-minute job timeout twice (19:25→20:25 and 20:57→21:57). In both logs the same 1088 test files complete — the last is src/ui/hooks/useDialogClose.test.ts — and then the cli suite's Test Files … passed summary never prints. The repo's disk-pressure sampler shows the run going idle, not busy: load decays 3.9 → ~1.0 with memory and disk flat for 33 straight minutes until the timeout. That is a worker-teardown deadlock at the end of the cli suite, not slow tests.
Controls: on the same day and runner pool, Test (ubuntu-latest) passes in ~29 min for #11742 and #11743 — both branch from the same main (b5c7635), so the only tree delta here is the Ink patch + the new loop-guard test file — and five other PR runs between 19:34 and 21:00 all passed. The hosted pool is healthy; the hang is specific to this branch. Locally (macOS) the full cli suite on this merged tree completes in 433s, so the hang needs the CI environment (ubuntu + worker topology) to manifest.
Prime suspects — all already flagged in round-2 review: R2-1 (the new test's fake stdout has no own EventEmitter state, so resize listeners land on the real process.stdout of the vitest worker and persist for the worker's lifetime), R2-4/R2-6 (refill timer pinning/cleanup), R2-5 (never-latching cascade). A mounted-then-abandoned Ink instance or a listener chain on a real stdio handle in a worker that has finished its files would produce exactly this signature.
To isolate quickly: run CI on a variant with only the test file (patch hunk dropped) and one with only the hunk (test file dropped) — one of the two will hang. I can prepare those if useful.
The guard's value stands (defense-in-depth per the duplicate-key root cause landing in #11743), but a P1 crash fix can't merge while it deterministically blocks the suite. Happy to pair on the bisect.
The measurement budget added to the vendored ink hook refilled itself from a setImmediate callback. A test that installs fake timers only runs that callback when it advances the clock, and the InputPrompt suite flushes renders with microtask yields inside act(), so the refill stayed pending and React's act queue never drained with it. Ten tests in that file each burned their full 15s timeout; on CI, where the suite runs with --retry=2, that single file outlived the 60 minute job limit and the lane was cancelled without printing a summary. Refilling from elapsed wall-clock time keeps the same per-instance budget and the same protection against the commit -> layout listener -> setState loop, and leaves no handle for a test to drain, so the unmount cleanup is no longer needed either.
|
Root cause of the deterministic CI hang found, and the fix is pushed as 6e460f7. Withdrawing my request-changes once the test lane reports green on that head. First, two corrections to what I wrote in that review, because both were wrong:
Which file. The cli workspace has 1047 test files. The cancelled run's log contains ✓ lines for exactly 1046 of them, and the three workspaces before cli each printed a Why it stalls. The guard refilled its per-instance budget from Local A/B at 79d1dba, macOS,
Isolation control: running a single affected test on its own with the guard on passes in 7.35s. The stall needs in-file contamination from earlier tests, which is why the PR's own new test file looked clean — it reported ✓ in 192ms on CI. Under CI's The commit message says "ten tests"; the verified count is 16 before I killed the run. Treat the table as authoritative. The fix. Same per-instance budget of 32, refilled from elapsed wall-clock time instead of from a timer: const now = performance.now();
if (now - guard.windowStart >= 16) { guard.count = 0; guard.windowStart = now; }
if (guard.count >= 32) return;
guard.count += 1;No pending handle means nothing for a test to drain, so the unmount cleanup is no longer needed either — net −13 lines in the patch. Regenerated with One open question for @zaalipro — the window size, not the mechanism. 16ms × 32 permits roughly 2000 measurements per second, which is barely a throttle: a real oscillation becomes a rate-limited livelock rather than a hard stop. The window length is the actual knob. A longer window (250ms–1s) would genuinely contain an oscillation, but Ink caps output at 30fps while React renders can exceed that, so a long window risks starving legitimate per-frame re-measurement. I kept 16ms because it is the value I verified green end-to-end and it leaves legitimate measuring behaviour untouched — but if the intent is for the guard to stop oscillation rather than rate-limit it, that is where to push. I can A/B a longer window if the numbers would help. Everything else from round 2 stands as non-blocking. Separately: |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-approving on 165fa43 and withdrawing the request-changes. The hang it was holding on is fixed; the diagnosis and the numbers are in the comment above.
What I checked before signing this:
Test (ubuntu-latest, Node 22.x)ran 04:34:38 → 05:06:01, 31m23s, and completed instead of being cancelled at the 60-minute limit. The one file missing from the cancelled run's log now reports:✓ src/ui/components/InputPrompt.test.tsx (215 tests) 65047ms. The cli workspace prints its summary —Test Files 1049 passed (1049)— where the two previous runs printed nothing at all.- That count reconciles rather than just looking plausible: 1047 cli test files at 6e460f7, plus exactly the two the main merge brought in (
serve/model-configuration.test.ts,serve/server/auth-provider-helpers.test.ts). Lint & Staticgreen, including step 8Check lint gate freshness, which is what failed before main was merged.- Locally on the same tree:
InputPrompt.test.tsx215/215 in 219.61s, and the three guard tests pass (22 / 8 / 24 ms), including the "still measures on a later resize once an oscillation tripped the guard" case. The patch was regenerated withnpx patch-package inkand validated against pristineink@7.0.3fromnpm pack, both blob hashes checked.
Transparency: I authored 6e460f7, the wall-clock refill. So this approval covers the diagnosis and the verification, not a fresh read of someone else's fix — the guard's design is @zaalipro's. I also want to correct the record on my request-changes review: two of its load-bearing claims were wrong. The hang did not need the CI environment to manifest, and it was not a worker-teardown deadlock. Both are addressed in the comment above.
Still open, and deliberately not blocking on it: the window size. 16ms × 32 permits roughly 2000 measurements per second, which rate-limits an oscillation rather than stopping it. I kept 16ms because it is the value I verified green end to end and it leaves legitimate measuring behaviour untouched. Whether the guard should hard-stop instead is @zaalipro's design call, and I would rather see it as a follow-up than hold a P1 crash fix on it.
For whoever merges: main requires two approving reviews and this PR has had one human reviewer, so this approval alone does not make it mergeable.
|
tmux e2e verification — two-arm A/B on the patched Independent run on Linux (Node v24.18.1, ink 7.0.3, tmux 2.7, real PTY 100×30). This is not a re-run of the collocated suite: it executes the hook inside a real Ink render in a real terminal, on both arms, and it reproduces the crash on the base arm. Arms — one file differs, verified by blob hash rather than by label
Everything else is identical by construction: both arms resolve every other dependency (react, react-reconciler, yoga-layout, …) to the same real paths, and the harness is byte-identical in both ( DriverThe oscillator from this PR's own regression test, in shape: a box whose Result
The base arm reproduces the reported failure mode in a terminal, through exactly the commit-phase layout-listener path the description names, and the patched arm does not. Arm A's 53 renders also independently matches the count Resize after the guard trips — driven by a real SIGWINCH, not a synthetic event
Arm B's probe therefore re-measured This is the behaviour the collocated test pins with CI at head, read per lane
What this arm does not cover
Conclusion: mergeable from our sideNo Critical found at State read immediately before posting this comment (05:53:16Z): head |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving on the strength of the tmux e2e two-arm A/B posted as comment 5651538987 at head 165fa43c07fb.
What this approval covers: an independent execution of the patched hook in a real PTY on both arms. Base arm (hook blob c4386e45…, the pre-image named in the patch's own header) dies 82 ms after mount with Maximum update depth exceeded thrown out of emitLayoutListeners → resetAfterCommit → commitRoot, exit 1, pane back at the shell prompt. PR arm (hook blob d1921ca1…, that header's post-image) survives a 25 s window, exits 0, and still re-measures across real SIGWINCH resizes (probe:100 → 55 → 90) after its own budget has tripped. Plus: no Critical found by us at this head, and all 7 reportable product lanes green.
What it does not cover: the full CLI (ink is inlined into the bundle and this host's disk did not fund a second build), the field notification-turn driver, and Windows/macOS. It is also not a fresh design review of the guard's constants — @yiliang114's approval already records the 16 ms × 32 window as an open, deliberately non-blocking design call for @zaalipro, and we agree that should not hold a P1 crash fix; our arm-B numbers (63 bounded renders, no #185) are consistent with it being CPU cost rather than a correctness issue.
Read at 05:53:16Z: reviewDecision=REVIEW_REQUIRED, mergeStateStatus=BLOCKED, one human approval at head. As @yiliang114 notes, main requires two approving reviews, so this row does not by itself make the PR mergeable.
…e speed (QwenLM#11835) * fix(cli): make the ink useBoxMetrics loop guard independent of machine speed The measurement budget added in QwenLM#11565 refilled on elapsed wall-clock time (32 measurements per 16ms), but React's nested-update cap is a fixed count of 50. The guard therefore only won that race while ~32 commits fit inside one 16ms window (~0.5ms each, including a Yoga layout pass). Under CPU contention or on a slower platform the window refilled mid-cascade, the count reset to 0, the budget never tripped, and React threw "Maximum update depth exceeded" (#185) out of the commit phase - which is what the regression suite caught on loaded CI runners, and why lastFrame() came back empty: onRender never ran. Spend budget only on measurements that actually change the metrics, since those are the only ones that schedule another render of the component, and refill it on events in React's own update chain rather than on elapsed milliseconds: a render the hook did not cause, a measurement that settled, and a terminal resize. The cap is 16 self-driven renders - the same burst the old budget allowed in practice, since updateMetrics runs twice per render - which sits well under React's 50 and no longer depends on how fast the machine is. No timer handle is introduced, so fake-timer tests still drain and React's act queue is unaffected. Adds a deterministic reproduction that pins machine speed by advancing performance.now() 16ms per read: it fails with the reported #185 stack before this change and passes after. Also adds regression coverage that the guard still stops an oscillation in far fewer commits than React tolerates, and that a box driven by external re-renders keeps reporting fresh metrics well past a single budget. Fixes QwenLM#11817 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-issue-patrol/jmu0yjkfs3z * fix(cli): settle the ink box-metrics budget per commit, not per render The guard settled its budget in the render body. React invokes a render body twice under StrictMode, which `DEBUG=1 npm run dev` renders the CLI inside: the first invocation spends budget and the second refills it, so the count never drains, the guard never trips, and React's #185 fires - the same failure QwenLM#11817 reports, on the one code path that renders inside StrictMode. Settle the budget from the root layout listener instead. It runs once per commit and never during render, so a doubled render body cannot double count it, and the measurement that schedules a render marks the commit that follows as the hook's own: that commit spends budget, any other commit refills it. The cap of 16 and the resize refill are unchanged. Adds the StrictMode case to the regression suite - it fails with #185 against both the previous patch and unpatched ink - and rewords the wall-clock case to say what its performance.now() spy actually establishes. * style(cli): keep the guard test React import on one line Adding StrictMode to the existing one-line import is what Prettier produces; the expanded form failed the lint job's Prettier check. * test(cli): pin the resize refill, the budget bounds and the shared oscillator The resize refill is the only path that reaches an instance whose budget already tripped, and dropping it left every case green. The render bounds also admitted any constant up to React own cap and pinned no floor at all. Folding the counting fixture back into the oscillator removes the second copy of the oscillation rule, and the metrics dispatch is now skipped when the caller already knows the value did not change. * fix(cli): arm the ink box-metrics guard on a hasMeasured transition Round-2 review findings on the previous revision. The budget is now armed when `hasMeasured` flips as well as when a measurement changes: `setHasMeasured` schedules a commit of its own with no changed measurement behind it, so a consumer gating the measured element on `hasMeasured` took the refill branch on every commit and ran to React's cap. Only the transition arms it - that call is unconditional, and arming on the call would charge the external commits that keep a streaming box measuring. Test changes, all pinned by mutation: - The render counter is commit-scoped (reported from an effect, not the render body) and the budget bound now runs in both StrictMode modes. With a render-body counter the StrictMode arm reports 34 against `<= 24`. - The StrictMode case pins the clock, so it is red against the pre-PR guard instead of green; it now fails with `Maximum update depth exceeded` and the commit-phase stack. - Dropped the try/catch around that mount. It does not contain a #185 that escapes the commit phase - every case after it fails either way - so it only cost the stack trace. - Corrected the resize case's title and comment: any commit in the ink root refills a tripped instance, not only a resize. A resize is special in that ink recomputes it with no React commit at all. - Folded the clock pinning into one helper, restored in a `finally`. Verified at this head: guard suite 10/10; the 8-file `useBoxMetrics` consumer suite 173 passed; loaded A/B (6 CPU burners on 4 cores, 8 consecutive rounds, `--retry=0`) 8/8 green here against 8/8 red on the pre-PR guard. The patch round-trips: pristine `ink@7.0.3` + `npx patch-package` reproduces `build/hooks/use-box-metrics.js` at blob `a6d1a4d`. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): pin the multi-pass budget headroom and correct the guard comments The lower bound on the budget was only pinned as "more than 2 commits", and no cap tighter than the oscillator needs can violate that: the oscillator settles after cap + 2 commits, so a cap of 2 leaves the suite green while a consumer legitimately needing a second measurement pass freezes mid-convergence. Add a consumer that halves its own width on every measurement and assert the settled width reaches the frame. It settles at 3px on the current budget and freezes at 16px with the budget lowered to 2. The guard comments claimed a fixed per-commit trip depth, and that onLayout runs for any commit in the ink root. The charge happens per layout-listener invocation, and the listener is only registered while the measured ref resolves to a root, so a consumer that detaches its measured element is charged for roughly every other commit and trips at up to twice the count. Say that instead, and stop promising headroom the constant does not deliver for that shape. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmu1r4dew54 * test(cli): pin the commit-path refill and the gated cascade's depth Three review findings on the loop-guard suite, plus the PR description's stale numbers. The commit-path refill of an already-tripped instance had no witness: every other recovery in the file goes through stdout.emit('resize'), and the external-rerender case never trips (its count oscillates between 0 and 1), so the comment on the resize case asserted a guarantee nothing checked. The new case trips the oscillator, then bumps a sibling that owns its own state and shifts the oscillator's left, and asserts the oscillator runs a second cascade off that commit. Both refill paths are now pinned separately: deleting the onResize reset reddens only the resize case, while a partial refill or one gated on the instance not being tripped reddens only the new case (measured deltas 1 and 0, against 16 at this head). HasMeasuredGatedBox reports its commits through the same onRender counter its siblings use, and the case now asserts the cascade's depth (<= 34) alongside the frame. The frame alone is parity-blind - this fixture detaches its measured element every other commit, so whichever of its two states it freezes in, the last non-blank frame is the fallback render - and the suite stayed green with the budget at 23 or 26, which takes this cascade to 46 or 52 commits against React's own cap of 50. ConvergingBox drops the `from`/`floor` props no caller ever sets and inlines the 32 and 3 that its comment's arithmetic is about, so the multi-pass headroom property cannot be quietly disabled by a mount that parameterises it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmu1zp0la5i * docs(cli): narrow the guard comments to the bound it delivers The vendored patch's two comments promised that "a cascade trips after the same number of charged invocations on any machine", which review measured to be false for two self-sustaining shapes: a cascade re-entering through a parent's state, and one where two instances in a single component refill each other's budget. Neither reaches the count - zero clamps - and only React's 50-update cap stops them, because the budget is per hook instance while the cap is per root. Both shapes are now named in the comment, with QwenLM#11858 as the tracked rewrite. The detached-ref caveat and the wall-clock comparison are unchanged, and the hook is byte-identical apart from these comment lines: 12/12 tests pass. Refs QwenLM#11817 * test(cli): pin the loop guard's exhaustion precondition and cascade floor Two recovery cases preconditioned on `afterTrip > 1` where the refill lines they pin depend on the budget having been exhausted, and the gated cascade case asserted a depth ceiling with no floor, so an early-settling hook passed both. Assert exhaustion (`>= 16`, the vendored budget) at both preconditions and floor the gated case's depth at the same constant. Mutation-verified on the real suite: a one-line `hasMeasured` latch drops the gated cascade from 32 commits to 2 and now reddens it (`2 >= 16`); a hook that dedupes seen layouts drops both recovery cases to `afterTrip=3` and now reddens them, where the resize case previously stayed green with the `onResize` reset deleted. Deleting either refill line alone still reddens only its own case. * fix(cli): scope the box metrics guard to the ink root --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: yiliang114 <yiliang114@users.noreply.github.com>
What this PR does
Extends
patches/ink+7.0.3.patchwith one hunk fornode_modules/ink/build/hooks/use-box-metrics.jsthat breaks the commit → layout-listener → setState → commit feedback loop which crashes the TUI with an uncaught Minified React error #185 (Maximum update depth exceeded).Refs #11500 (parent: #5199). Refs #11732.
The patched hook changes exactly two things:
useBoxMetricsinstance gets its own budget (useRef, 32 measure→setState cycles per event-loop tick, refilled via asetImmediatethat isclearImmediated on unmount). A measure/render oscillation can never reach React's nested-update limit, while normal measuring (mount, resize, content changes — including 40+ boxes in one commit) stays far under the cap. Per-instance scoping matters: the loop being broken is one component's own measure → setState → commit cycle, and React's nested-update counter is per-commit-chain, so the cap holds no matter how many instances oscillate in the same tick.setHasMeasured(Boolean(ref.current))stays in upstream's plain form, since React'suseStatesetter already eagerly bails an identical primitive.A collocated regression test (
packages/cli/src/ui/use-box-metrics-loop-guard.test.tsx) renders the real hook and pins all three behaviors: an oscillating box settles instead of throwing #185, a later resize still measures after the guard trips, and 40 instances mounting in one commit all measure.Why it's needed
When background agents/monitors complete and their notifications land, the footer/status line and transcript re-render, and a
useBoxMetricslayout listener fires inside the commit phase. If the measured layout oscillates, commit → listener → setState → commit repeats until React throws #185; nothing catches it (the throw site is host-config code, outside the app-level error boundary), so the process exits and the in-flight turn is lost. Multiple users reproduce this within minutes-to-hours of normal use (#11500: 5× in one day; #11732: two independent sessions; additional confirmations in both threads).Scope note (added during review): @vetaljanos root-caused a distinct input-data driver for this crash — duplicate history-item React keys from
baseTimestamp + counterid minting on notification turns (see comment). A loop guard cannot fix broken input data; it throttles the cascade regardless of driver. The id fix lands separately; this guard remains the defense-in-depth that covers drivers we have not found, including crashes with no notification in flight.Reviewer Test Plan
How to verify
npm install(ornpm ci) —postinstallapplies the patch; expectink@7.0.3 ✔.npx vitest run src/ui/use-box-metrics-loop-guard.test.tsxinpackages/cli— 3/3. Deleting the guard hunk or restoring it to module scope turns the suite red withMaximum update depth exceeded, so the patch cannot be silently lost in a regeneration.VirtualizedList,Footer,ink-cursor-rendering— all pass unmodified.Evidence (Before & After)
ink@7.0.3: the budget is per-instance, the refill handle is cleared on unmount, and the new suite plusVirtualizedList(66/66) andink-cursor-rendering(28/28) pass; three simultaneous oscillators settle (18 renders each, then quiescent).Tested on
Environment (optional)
Local: macOS, Node v22–v24,
npm ci+npm run build. CI runs the full matrix.Risk & Scope
useTerminalBuffer: falseexperiment and terminal/width data points remain open on TUI exits silently (uncaught React #185) when multiple background agents complete — Ink useBoxMetrics layout-listener setState loop #11500.Linked Issues
Refs #11500 — crash reports share the throw site this guard throttles; the confirmed notification-turn driver is fixed separately.
Refs #11732 — same stack; its orphaned-monitor half is handled in a separate PR.
中文说明
本 PR 做了什么
给
patches/ink+7.0.3.patch增加一个针对node_modules/ink/build/hooks/use-box-metrics.js的 hunk，打断 commit → layout-listener → setState → commit 的反馈回路——正是它让 TUI 以未捕获的 Minified React error #185（Maximum update depth exceeded）崩溃。关联 #11500（父 issue：#5199）、#11732。
补丁只改两件事：
useBoxMetrics实例有自己的预算（useRef，每个事件循环 tick 最多 32 次 measure→setState 循环，通过setImmediate续杯并在 unmount 时clearImmediate）。测量/渲染振荡再也不可能触及 React 的嵌套更新上限，而正常测量（挂载、resize、内容变化——包括单 commit 内 40+ 个盒子）远低于上限。按实例划分是关键：被打断的是单个组件自己的 measure → setState → commit 循环，且 React 的嵌套更新计数按 commit 链计算，所以无论多少实例在同一 tick 振荡，上限都守得住。setHasMeasured(Boolean(ref.current))保持上游原样，因为 React 的useStatesetter 对相同原始值本就会提前 bailout。同目录的回归测试（
packages/cli/src/ui/use-box-metrics-loop-guard.test.tsx）渲染真实 hook，固定三种行为：振荡盒子收敛而不是抛 #185；守卫触发后后续 resize 仍能正常测量；单 commit 内 40 个实例全部完成测量。为什么需要
当后台 agent/monitor 完成、通知到达时，footer/状态行和 transcript 会重渲染，
useBoxMetrics的 layout listener 在 commit 阶段内触发。如果测得的布局发生振荡，commit → listener → setState → commit 会循环到 React 抛 #185；没有任何东西接住它（抛出点在 host-config 代码里，应用级 error boundary 之外），于是进程退出、进行中的 turn 丢失。多名用户在正常使用几分钟到几小时内即可复现（#11500：一天 5 次；#11732：两个独立会话；两个帖子里还有更多确认）。范围说明（评审期间补充）：@vetaljanos 定位了这个崩溃的另一个独立输入数据驱动——通知 turn 上
baseTimestamp + counter铸 id 撞出的重复 React key（见评论）。环路守卫无法修复坏掉的输入数据，但它能掐断任何驱动引起的级联。id 修复单独落地；本守卫作为纵深防御，覆盖我们尚未发现的驱动——包括那些没有通知在途的崩溃。评审验证计划
如何验证
npm install（或npm ci）——postinstall应用补丁，应看到ink@7.0.3 ✔。packages/cli下npx vitest run src/ui/use-box-metrics-loop-guard.test.tsx——3/3 通过。删掉守卫 hunk 或改回模块级作用域，套件会变红（Maximum update depth exceeded），因此补丁不会在重新生成时被无声丢失。VirtualizedList、Footer、ink-cursor-rendering全部原样通过。证据（前后对比）
ink@7.0.3上验证了补丁应用后的 hook：预算是按实例的，续杯句柄在 unmount 时清理；新套件及VirtualizedList（66/66）、ink-cursor-rendering（28/28）通过；三个振荡器并发时收敛（各 18 次渲染后静止）。测试平台
环境（可选）
本地：macOS、Node v22–v24、
npm ci+npm run build。CI 跑完整矩阵。风险与范围
useTerminalBuffer: false实验和终端/宽度数据点在 TUI exits silently (uncaught React #185) when multiple background agents complete — Ink useBoxMetrics layout-listener setState loop #11500 上仍然开放。关联 Issue
Refs #11500——各崩溃报告共享本守卫所扼制的抛出点；已确认的通知 turn 驱动由独立 PR 修复。
Refs #11732——相同堆栈；其孤儿 monitor 一半由另一个 PR 处理。