fix(desktop,runtime): interrupt and bound empty assistant loops

[Rangsh]

Interrupt the active desktop turn before sending a plain Enter follow-up, and clean up transient messages retracted by the stop operation.

Bound repeated textless tool steps when no explicit maxSteps is configured, while preserving normal multi-step workflows and explicit step limits.

Fixes #4083

Summary

Desktop mid-turn plain Enter in the main conversation used to queue a follow-up, so a runaway empty-reply loop never stopped and the typed message could not take effect. Plain Enter now interrupts the active turn first, then starts a new root send. Cmd/Ctrl+Enter still steers; Shift+Enter inserts a line break (unchanged).

Scoped product decision: this interrupt-on-Enter change applies to the main chat Composer only. Side chat and WorkHub keep Enter-to-queue because those surfaces expose a managed message queue. Keyboard help and Composer comments document both behaviors.

In the runtime agent loop, when maxSteps is unset, consecutive identical textless tool-only steps (and short alternating empty cycles) are capped so the turn ends instead of flooding blank assistant replies, without changing normal multi-step work or explicit step budgets.

Fixes #4083

Verification

• node --test --test-name-pattern="stops an unbounded loop after consecutive identical empty" packages/runtime/dist/__tests__/ai-sdk-backend.test.js — pass
• Desktop follow-up submit routing tests — pass
• Rebased onto current apache/maka main and resolved conflict with fix(runtime): preserve Plan final responses #3886 in ai-sdk-backend.test.ts
• Did not run full-repo lint / format:check / typecheck / full workspace npm test

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope:

Cursor (Composer) helped locate the empty-loop / mid-turn send paths, implement the runtime empty-step bound and Desktop interrupt-on-send behavior, add/adjust tests, update Enter help copy for review feedback, and rebase through the upstream conflict. Human owns the final review and submission.

Affected commits should retain: Generated-by: Cursor

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Thanks for addressing the output-free tool loop in the shared Runtime boundary. The cap is a useful safety net and keeps explicit maxSteps authoritative. I found one normal Responses-protocol path that still bypasses the new counter.

This is a suggestion from an outside review, so please push back if the empty reasoning carrier has a different production invariant than the adapter currently expresses.

中文摘要

感谢把无输出 tool loop 的保护放在共享 Runtime 边界。当前仍有 1 个 P1：OpenAI Responses 的空 reasoning carrier 会把 stepSawThinking 置真，使完全无可见进展的重复 tool step 永远不进入新计数器。若我遗漏了 provider invariant，欢迎直接 push back。

AI-assisted review disclosure: Codex coordinated an independent reviewer lane; Astro-Han independently checked the exact head, adapter composition, reachability, and severity, and owns this review.

[Astro-Han]

Thanks for adding the shared loop cap. I think this unconditional flag leaves a [P1] category ① production bypass for OpenAI Responses: the model adapter emits a thinking event with text: "" at reasoning-end whenever Responses provider metadata is present. A normal textless tool step can therefore set stepSawThinking=true even though it has no visible reasoning or text, so emptyStepSignature stays undefined and identical tool-only steps never reach the new cap. The added test uses a mock with no such carrier, so it does not exercise the real Responses composition. My suggestion is to count only non-empty visible thinking here (while preserving provider metadata separately) and add a Responses reasoning-end → repeated tool-call regression. Please push back if an empty carrier is intentionally considered user-visible progress.

[Rangsh]

Thanks — that was a real bypass. An empty Responses reasoning-end carrier is not user-visible progress.

stepSawThinking now only flips on non-empty thinking text. Provider metadata still persists separately via sawStepThinking, so the encrypted carrier still round-trips.

Added a regression that streams Responses reasoning-end with empty text + identical tool-calls and asserts the loop cap still fires (step_limit after 3 steps).

[Rangsh]

Thanks for addressing the output-free tool loop in the shared Runtime boundary. The cap is a useful safety net and keeps explicit maxSteps authoritative. I found one normal Responses-protocol path that still bypasses the new counter.

This is a suggestion from an outside review, so please push back if the empty reasoning carrier has a different production invariant than the adapter currently expresses.

中文摘要
感谢把无输出 tool loop 的保护放在共享 Runtime 边界。当前仍有 1 个 P1：OpenAI Responses 的空 reasoning carrier 会把 stepSawThinking 置真，使完全无可见进展的重复 tool step 永远不进入新计数器。若我遗漏了 provider invariant，欢迎直接 push back。

AI-assisted review disclosure: Codex coordinated an independent reviewer lane; Astro-Han independently checked the exact head, adapter composition, reachability, and severity, and owns this review.

Thanks — that was a real bypass. An empty Responses reasoning-end carrier is not user-visible progress.

stepSawThinking now only flips on non-empty thinking text. Provider metadata still persists separately via sawStepThinking, so the encrypted carrier still round-trips.

Added a regression that streams Responses reasoning-end with empty text + identical tool-calls and asserts the loop cap still fires (step_limit after 3 steps).

[Astro-Han]

Thanks for bounding repeated empty assistant steps and for adding the Responses empty-reasoning regression. One normal provider carrier still bypasses the new cap. This is a suggestion from an outside review, so please do push back if a signature-only event is intentionally treated as user-visible progress.

AI-assisted review disclosure: Codex ran an independent runtime/retry analysis lane; Astro-Han is the contributor of record for this review.

[Astro-Han]

[P1] (category ① — normal provider stream path)

Thanks for preserving the signature for continuation/replay. Setting stepSawThinking for a signature-only carrier also prevents emptyStepSignature from being computed. Anthropic/Responses can emit omitted or redacted reasoning as a standalone signature with no text (the adapter and current tests explicitly preserve that path), so a model that repeats the same textless tool call plus signature can still loop without ever reaching the three-step cap. Could the signature remain persisted without counting as visible progress, and add a signature-only + repeated-identical-tool-call regression? Please feel free to push back if the provider contract guarantees every such signature corresponds to substantive progress that should reset the bound.

[Rangsh]

Thanks for bounding repeated empty assistant steps and for adding the Responses empty-reasoning regression. One normal provider carrier still bypasses the new cap. This is a suggestion from an outside review, so please do push back if a signature-only event is intentionally treated as user-visible progress.

AI-assisted review disclosure: Codex ran an independent runtime/retry analysis lane; Astro-Han is the contributor of record for this review.

Agreed — a standalone signature is omitted/redacted reasoning, not user-visible progress.

thinking-signature still persists on stepSignature for replay, but it no longer sets stepSawThinking. The empty-step cap now treats signature-only + identical tool calls as textless.

Added a regression: Anthropic signature-only delta (no thinking text) + the same tool-call repeats, then the turn ends at step_limit after 3 steps, and the signature is still on thinking_complete.

[Astro-Han]

Thanks for taking #4083 on — the loop cap is more carefully built than most attempts at this problem. Before anything else, though:

CI has never run on this branch. The head commit 7a26babf has zero check runs, so none of the verification is independently confirmed. Please rebase onto current main and push; that should get the workflows going. main has moved a long way since 08-31, and this touches ai-sdk-backend.ts, which has changed in that window.

What holds up, and it's the hard part: the empty-step signature is conservative in exactly the right places. Requiring no visible text, no thinking, at least one tool call, and an identical toolName + input means an ordinary multi-step workflow can't trip it, and gating the whole thing on maxSteps === undefined leaves every explicit budget authoritative. Distinguishing stepSawThinking (non-empty text only) from sawStepThinking (any carrier) is the detail that makes it actually work — the OpenAI Responses empty carrier at reasoning-end would otherwise reset the streak on every step and the cap would never fire. Same for treating a standalone thinking-signature as non-progress. Resetting the streak when a steer is redirected is right too.

Two things I'd like you to look at.

P2 — alternating signatures walk straight past the cap. The streak only survives while consecutive signatures are equal, so A B A B A B … — two different textless tool steps taking turns — keeps resetting to 1 and never reaches 3. That is still a runaway empty-reply loop, and it's the same user-visible symptom #4083 reports. I don't think the fix is to drop the equality requirement (consecutive different textless tool steps are ordinary agent work), but a small window would cover it: track the last N signatures and stop when the window has no distinct progress, rather than comparing only against the immediately previous one.

P2 — resolveFollowUpModeAtSubmit is now an identity function with a dead parameter.

export function resolveFollowUpModeAtSubmit(input: {
  requestedMode?: FollowUpMode;
  /** Retained so call sites keep compiling. */
  hasActiveTurn?: boolean;
}): FollowUpMode | undefined {
  if (input.requestedMode) return input.requestedMode;
  return undefined;
}

That is input.requestedMode with extra steps. The comment is honest about why the parameter is still there, which is the problem: a parameter kept alive to avoid touching call sites is the kind of thing that reads as meaningful to the next person. Since the function no longer decides anything, please delete it and let the call site use metadata?.followUpMode directly. hasActiveTurnAtSubmit still earns its keep — the new interrupt branch uses it — so that one stays.

One question rather than a finding. After sessions.stop(...) resolves as interrupted, the code falls through to a new root send. Does the Host guarantee the new send is admitted, or can it come back session_busy while the interrupted turn is still settling? The per-Session admission gate is FIFO with no priority lane (this is the same ground as #3713), so I want to know whether the ordering here is guaranteed or just usually fast enough. If it's the latter, it's worth a test that holds the stop's settlement open.

[Rangsh]

@Astro-Han Thanks for the careful re-review — addressed all three points and rebased onto current main so CI can run.

P2 alternating signatures. Agreed that consecutive-equality alone lets A B A B … walk past the cap. Kept the 3-step identical streak (ordinary multi-step work still won’t trip it), and added a sliding window of the last 6 empty-step signatures: once the window is full and unique < length, we stop as step_limit. Added a regression that alternates two textless Read inputs and asserts the turn ends after 6 steps.

P2 resolveFollowUpModeAtSubmit. Deleted. Call site now uses !slashCommand ? metadata?.followUpMode : undefined directly; hasActiveTurnAtSubmit stays for the interrupt branch.

Stop → send / session_busy. Host-side turn.interrupt awaits the cancelled turn’s terminal fact (claim.terminal) before resolving success. Desktop only falls through to the new root send after sessions.stop returns interrupted, so admission is ordered by that await — not “usually fast enough” relative to settlement. Documented that invariant next to the interrupt-then-send path. Happy to add a Host test that holds settlement open if you’d still like an explicit regression for a non-awaiting caller; this Desktop path doesn’t need one.

Verification: node --test --test-name-pattern="stops an unbounded loop" packages/runtime/dist/__tests__/ai-sdk-backend.test.js — pass (4 cases, including the new alternating-window one).

[me2seeks]

Automated review (Command Code) — not an approval

The change does bound the reported loop, and the desktop interrupt is correctly ordered (it awaits the host's terminal settlement before a new root starts). But the new heuristic is broader than the problem, and it is reported as something it is not.

P2 (Should-Fix) — the empty-step window stops turns that are silent but making progress.

// packages/runtime/src/ai-sdk-turn.ts:2396-2408
const windowHasNoDistinctProgress =
  recentEmptyStepSignatures.length >= EMPTY_STEP_SIGNATURE_WINDOW &&
  new Set(recentEmptyStepSignatures).size < recentEmptyStepSignatures.length;
if (
  consecutiveIdenticalEmptySteps >= MAX_CONSECUTIVE_IDENTICAL_EMPTY_STEPS ||
  windowHasNoDistinctProgress
) {
  this.loopStopReason = 'step_limit';
  this.loopStopRequested = true;
}

The signature is derived from the request only, and the tool results — the actual progress — are not part of it. So any duplicate among six consecutive textless tool steps ends the turn. A normal "search, then read each hit" pattern (the same search signature recurring while every result is new) trips it, as does five distinct steps plus one benign repeat. Because the desktop passes no step budget, the cap is also the effective bound with no way to tune or disable it.

This is not hypothetical: the PR had to edit an existing fixture to work around it —

// packages/runtime/src/__tests__/overflow-reactive-recovery.test.ts
+  /** Give each scripted `tool` step a distinct Read path. Needed when a test
+   *  chains several textless tool steps: the Runtime empty-step cap (#4083)
+   *  stops consecutive identical tool signatures ... */
+  distinctToolPaths?: boolean;

i.e. a previously legitimate fixture now trips the new heuristic and was changed to avoid it. Smallest sound fix: make progress evidence result-aware (a step only counts as empty when neither the request nor the result changed), and/or expose the thresholds.

P2 (Should-Fix) — the heuristic stop is persisted and rendered as a configured step limit, and marks the invocation failed.

loopStopReason = 'step_limit' (:2407) maps to the tool_step_cap_reached failure class and a transcript note whose copy reads "Reached the configured step limit…" — but no step limit is configured on this path. The user and the telemetry cannot distinguish this heuristic from a real configured cap. A distinct stop reason (or gating the note copy on whether a budget was actually configured) would keep both accurate. It is at least not recorded as a user cancel.

P2 (Should-Fix) — the desktop interrupts before the send is known to be admissible.

The new ordering runs the stop before the revision and eligibility checks, so a mid-turn send that then fails those checks (unchanged revision text, or a failed revision preparation) has already killed the active turn while sending nothing. Move the interrupt after the eligibility checks, immediately before the actual send.

P3 (Nice-to-have) — the queue follow-up lane is now unreachable from the main shell. The only producer of the queue mode is deleted and the composer only ever passes steer, so the host rejects a busy send instead of queueing it, while the queue UI (promote/reorder/delete) remains. If that is intentional, the UI and the product copy should say so.

P3 (Nice-to-have) — one clear is redundant. The clearEmptyStepProgress() call on the tool-free steer continuation is reached only when no tool calls were returned, in which case the signature is already undefined and the existing else branch has cleared it.

Review-relevant risks. Changing retry/termination policy and user-visible failure classification warrants independent human review under CONTRIBUTING.md. No security or licensing effect found.

Required conclusion.

1. Optimal for the actual problem? Partially. The desktop interrupt is well placed; the runtime adds a second hidden step budget alongside the existing one, where a single default budget (or a result-aware progress check with its own stop reason) would be smaller and observably clean.
2. Production code that can be deleted? The redundant clear noted above; and if the queue lane is truly gone, the unreachable next-turn branch.
3. Low-quality tests to delete or replace? None to delete. Add the missing must-survive coverage: six or more distinct textless steps must continue, a text/thinking step must reset the window, and a benign repeat whose result changed must not trip it — no current test pins the survive side, so a mis-sized window within the range would pass.
4. Deeper refactor required? Recommended, not blocking: one progress/budget source of truth carrying its own terminal classification.
5. Ready to merge? Not as-is — the P2s above should be resolved and the must-survive test added.
6. Residual risks / verification gaps: I did not run the suite. The branch is based on an older main in the touched renderer file, so a rebase may reveal further drift.

Approval boundary. This is automated review; it is not an approval. Per CONTRIBUTING.md, the merge decision requires an independent human review. No approve was submitted.

[Rangsh]

@me2seeks Thanks for the careful review — addressed the P2s (and the redundant clear) on 2ae395c.

P2 result-aware empty-step progress. Agreed the request-only window was too broad. A textless tool step now signatures (toolName, input, result), so a repeated search/read whose result changed counts as progress. Also tightened the sliding window: once full, it stops only when distinct * 2 <= length (short cycles like A/B), not on a single benign repeat among otherwise new steps.

P2 distinct stop reason. Heuristic stops now use empty_step_loop → failure class empty_assistant_loop, with its own system note / UI / CLI copy. Configured maxSteps still ends as step_limit / tool_step_cap_reached.

P2 interrupt after eligibility. Plain-Enter interrupt moved to immediately before the root send, after revision / slash / compact eligibility. An inadmissible submit no longer kills the active turn. Host turn.interrupt still awaits terminal settlement before the new root send is admitted.

P3 redundant clear. Removed the steer-path clearEmptyStepProgress() — with no tool calls the else branch already cleared.

P3 queue lane. Left as intentional for this PR: main-shell plain Enter interrupts+roots instead of queueing; Shift+Enter still steers. Happy to follow up on product copy / UI if maintainers want the queue surface retired or re-documented.

Must-survive tests added. Six+ distinct textless steps continue; visible text resets the window; same request with changing results does not trip. Existing trip cases now assert empty_step_loop.

Verification: node --test --test-name-pattern="stops an unbounded loop|continues through six|resets the empty-step|does not trip the empty-step|empty_step_loop|empty_assistant_loop|mapCompleteStopReason" packages/runtime/dist/__tests__/ai-sdk-backend.test.js packages/runtime/dist/__tests__/session-event-runtime-mapper.test.js packages/runtime/dist/__tests__/runtime-event-read-model.test.js — pass (10).

[me2seeks]

Follow-up (Command Code) — verified resolved

I re-checked the head after your push.

The P2 is fixed. The empty-step signature now includes the tool result alongside the name and input, so a step that repeats a request while producing new output is no longer treated as no-progress — which was my concern (a search-then-read pattern tripping the bound, and a fixture that had to be rewritten to avoid it). Watching the (toolName, input, result) triple is the result-aware shape I asked for, and the window handling was tightened as well.

Thanks. (Automated review; not an approval.)

[hqhq1025]

This head changes plain Enter during a live turn to interrupt before a root send and adds a bound for repeated textless tool-only steps without an explicit maxSteps. I found two remaining race/correctness paths (inline). The existing current-head review verifies that including tool results in the empty-step signature resolves its separate progress-detection concern; these findings do not repeat that point.

There are no visible CI checks on this head. It also currently conflicts with main in the Desktop composer/routing files, runtime turn implementation, and related ledgers/tests. I checked the diff and control flow but did not run a complete build or a real Electron/Host timing reproduction. No schema migration is involved.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Clear the empty-step streak when this top-of-loop drain injects a new steer. The only progress reset here occurs on a visible/thinking/different tool step, not when drainSteeringInto adds a user message. After two identical textless tool steps, a Shift+Enter steer arriving between the post-step drain and this drain can be injected before the next model request, yet an identical tool result then becomes the third step and immediately triggers empty_step_loop. Use the injected-message count to reset the streak and add that timing regression.

[hqhq1025]

[P2] Preserve the submitting Session across the awaited stop. This captures sessionId before stopping, but the following send() reads activeIdRef.current again in app-shell-chat-actions.ts. If the user submits A's draft and navigates to B while sessions.stop(A) is pending, the root send targets B. Guard against a changed active Session after the await (or pass the captured owner to send), and test navigation during a deferred stop.

[Rangsh]

@hqhq1025 Thanks for the careful re-review — addressed both P2s on 4743ecb9d, then merged current main on 9e21e19fe so CI can run.

P2 empty-step streak vs mid-loop steer. Agreed that a Shift+Enter steer injected at the top-of-loop drain is user progress and must not be charged as the next identical empty step. The agent loop now records injectedSteeringMessages.length before drainSteeringInto and calls clearEmptyStepProgress() when anything was injected. Added a regression that lands a steer after two identical textless tool steps and asserts the turn continues (end_turn) instead of tripping empty_step_loop on the third.

P2 submitting Session across the awaited stop. Plain-Enter interrupt now passes the captured sessionId into stop(...), so a navigation during settlement cannot retarget the stop. After the await, shouldContinueRootSendAfterInterrupt refuses the root send if activeIdRef no longer matches the submitting Session — otherwise send() would re-read the active id and deliver the draft elsewhere. Added coverage for stop-with-captured-owner during a deferred await, and for the post-interrupt routing guard.

Verification: node --test --test-name-pattern="stops an unbounded loop|continues through six|resets the empty-step|does not trip the empty-step|mid-loop steer" packages/runtime/dist/__tests__/ai-sdk-backend.test.js — pass (8). Desktop app-shell-stop-action / follow-up-submit-routing cases for the Session pin — pass.

[hqhq1025]

Reviewed the current head f4e0fbd1. The runtime loop bound and empty-loop terminal classification are wired through Runtime, Core, CLI and UI; the previously reported mid-loop steer case now clears the empty-step streak at ai-sdk-turn.ts:1472-1476, with a targeted regression at ai-sdk-backend.test.ts:6156-6251. The Desktop path now passes the captured Session ID to stop and checks selection after awaiting it (follow-up-submit-routing.ts:49-65), addressing the previously reported cross-Session send path in code.

One P1 blocker remains: this PR's current-head test check fails in Desktop renderer typecheck with TS2322 at app-shell.tsx:1614 (run 36308629917). The caller passes a LiveTurnBuffer (an array), while the new interrupt helper expects one turn. This is a mismatch in the PR branch, not an unrelated CI failure. It also means the helper's single-turn active/terminal check does not model the actual buffer. Please handle the buffer explicitly and cover both active and terminal/multiple-turn cases before retrying CI.

The PR changes 26 files (+1103/-213). I inspected the production paths, focused regression tests, and current-head CI; git diff --check is clean. I did not run the local test suite (dependencies are not installed), Electron smoke, or a post-merge build. The branch currently conflicts with fresh main in apps/desktop/src/renderer/app-shell.tsx and apps/desktop/renderer-architecture.json; those conflicts also need resolution and current-head checks must pass before merge. No schema or migration change is present.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P1] Pass a turn-buffer-aware value here. liveTurnBySessionRef.current[sessionId] is LiveTurnBuffer | undefined (readonly LiveTurnProjection[]), but interruptBeforeRootSend accepts one { turnId, terminal? }; current-head CI fails with TS2322 at this line. A cast would only hide the mismatch: hasActiveTurnAtSubmit must inspect the relevant turns in the buffer, including retained terminal turns.

[Rangsh]

@hqhq1025 Agreed — casting would only hide the mismatch. interruptBeforeRootSend / hasActiveTurnAtSubmit now take the Session's liveTurns buffer (LiveTurnBuffer) explicitly:

• any non-terminal projection means an active turn (interrupt)
• retained terminal turns are inspected so their Host runningTurnIds alone do not re-trigger interrupt
• a running turn id outside the retained terminal set still counts as active

Call site passes liveTurnBySessionRef.current[sessionId] as liveTurns. Added regressions for active+retained-terminal, multiple terminal-only, and an extra running id. Included in 8d19b2792 with the main merge.

[Rangsh]

@hqhq1025 Thanks for the careful re-review — addressed the P1 and the main conflicts on 8d19b2792.

P1 LiveTurnBuffer vs single-turn interrupt helper. Agreed a cast would only hide the mismatch. hasActiveTurnAtSubmit / interruptBeforeRootSend now take liveTurns (the Session buffer) explicitly: any non-terminal entry is active; retained terminal turns are inspected so their Host runningTurnIds alone do not re-arm interrupt; a running id outside the retained terminal set still counts as active. AppShell passes liveTurnBySessionRef.current[sessionId] as liveTurns. Added coverage for active+retained-terminal, multiple terminal-only, and an extra running id.

Merge conflicts with main. Rebased/merged current apache/maka main: kept main's queueSurface ownership in AppShell, preserved the interrupt-before-send path, and refreshed renderer-architecture.json so the debt ratchet stays green against upstream-main.

Verification: npm run build:main -w @maka/desktop — pass (no TS2322). node --test apps/desktop/dist/main/__tests__/follow-up-submit-routing.test.js — pass (8). npm run check:renderer-architecture -- --base upstream-main — pass.

[Rangsh]

Thanks @hqhq1025 and @Astro-Han for the thorough follow-up reviews — much appreciated.

P2 (Enter help / scope) — addressed

• Updated keyboard help in all three locales and the composer.tsx comment so they no longer claim plain Enter queues in the main chat.
• Scoped product decision (documented in help + PR summary): main conversation Composer interrupts on Enter; Side chat / WorkHub keep Enter-to-queue because they expose a managed message queue.
• Corrected the PR description: Cmd/Ctrl+Enter steers; Shift+Enter inserts a line break.

CI — addressed

• Rewrote the failing streaming-remount E2E (ordinary Enter interrupts…) to expect interrupt-then-root-send instead of queue.

P3 (partial)

• Responses empty-carrier test now uses an OpenAI connection so the empty thinking carrier path is actually hit.
• Left the remaining P3 items (signature hashing, stop any/expectedTurnId polish, unrelated script splits) for follow-ups so this stays focused on Bug: maka loop leads to repeated empty AI replies and cannot be interrupted by message #4083.

Also rebased/synced onto current main. CI is re-running on the new head — please take another look when you have a moment. Thanks again!

[hqhq1025]

Reviewed head 0bd9c710da286dd2a5bc5238d05051e9f5cbb500. The latest four-file change aligns Enter help in all three locales and the Composer comment with the main-chat interrupt behavior, changes the streaming-remount E2E to assert a root send rather than a queued follow-up, and runs the Responses empty-carrier regression with an OpenAI connection. I found no new blocking issue in this delta. Earlier non-blocking review follow-ups are not addressed by this commit.

The current-head test CI passed, including the Desktop E2E tier (34 tests). Locally, Node 24 npm ci, build:test, focused routing/stop tests (11), the Responses regression (1), and renderer architecture checks (112) passed. A static merge against current main (71bc045c) and git diff --check were clean. I did not run a separate packaged Electron/manual UI pass. GitHub still reports the PR as blocked, so these checks are not a merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Incremental re-review at 0bd9c710 (Claude lineage). Apart from a merge of main, the only new commit is 0bd9c710d (4 files, +37/−28).

Our earlier P2 was that plain Enter now interrupts the running main-chat turn instead of queueing the message, a user-visible behavior change that the Enter help did not describe. The author kept the behavior and made it explicit instead:

• The Enter help in all three locales (shell-copy.ts:1151,1678,2217-2223) now says Enter interrupts the current turn in the main chat, while Side chat and WorkHub still queue.
• The composer comment is corrected.
• streaming-remount.spec.ts:59-135 now asserts the interrupt-then-root-send path, with no queued entry.

The documentation part of the P2 is resolved, and no new P0–P2 were found in the delta. Whether interrupt-on-Enter is the desired product behavior is a maintainer call rather than a code defect, so we flag it for the merge decision instead of blocking on it. The earlier non-blocking P3s are not all addressed in this commit.

Automated review (Claude lineage, posted from the Astro-Han account). No approval implied.

[me2seeks]

Automated review notice: This comment was posted by an automated review agent operated by me2seeks make. It is not an independent human review and does not replace one.

Summary

Interrupts and bounds empty assistant loops: the stop action now accepts an optional sessionId override and returns a success boolean, follow-up submit routing and session-status presentation gain the loop-interrupt states with locale copy, core/events.ts + session.ts carry the bound, and overflow-reactive-recovery tests cover the runtime side. The renderer-architecture ledger is updated.

Findings

1. [P1] test fails on this head — the required merge gate is red. For a loop-interrupt feature touching stop actions, routing, and recovery, the failing suite is very likely the new stop-action or routing assertions; pull the log, fix, and show green before merge.
2. [P2] apps/desktop/src/renderer/app-shell-stop-action.ts — the change LOOSENS types to any: the previous typed ToastApi interface (error(title, description?, diagnosticDetails?, diagnosticTarget?)) and explicit return type () => Promise<void> become toastApi: { error(...args: any): void } and }): any {. On a user-facing stop/interrupt path this erases the very contract the renderer-architecture ledger (updated in the same PR) exists to enforce — a miscalled error(...) no longer fails typecheck. Restore the typed interface with the new optional-parameter signature.

Verdict

needs-changes — red test gate, plus a type-loosening to any on the stop path that should be re-tightened.

[Rangsh]

@me2seeks @Astro-Han @hqhq1025 Thanks for the re-review — addressed the remaining stop-path findings on b72a5c4a5.

me2seeks P2 type loosening. Restored a typed ShellErrorToastApi (error(title, description?, diagnosticDetails?, diagnosticTarget?)) and an explicit (sessionId?, expectedTurnId?) => Promise<boolean | undefined> return type on createAppShellStopAction. The toast contract lives in locales/shell-copy.ts so the renderer-architecture token ratchet stays green against main.

Astro-Han P3 stop cleanup.

• Interrupt now pins expectedTurnId into sessions.stop, so a queued turn that starts during settlement is not cancelled in place.
• If the active Session changes during the awaited stop, plain-Enter root send is refused and the user gets a localized toast (interruptSendAbandoned*) instead of a silent failure.

me2seeks P1 / CI. Current-head test was already green on the prior head; this commit keeps the focused routing/stop coverage updated (13 cases).

Earlier non-blocking follow-ups called out as product/consider (signature hashing, polling cut-off policy, unrelated script splits) are intentionally left for maintainer direction / follow-up.

Verification: npm run build:main -w @maka/desktop — pass. node --test on app-shell-stop-action + follow-up-submit-routing — pass (13). npm run check:architecture -w @maka/desktop -- --base apache/main — pass.

[hqhq1025]

Reviewed head b72a5c4a51089efd083c1daba01a899ea8d502a2. This update pins plain-Enter interruption to the turn visible at submit time and adds a session-switch toast, but I found one P2 race in the stop result handling (inline). The new routing tests and stop-action tests pass locally (13 focused cases); fresh main merge-tree and git diff --check are clean, and hosted test is green. Local build:test did not complete: unchanged UI component-contract type errors (settledText, menuAnchorRef, trailingAction) stop the build. I did not independently run Electron E2E or packaged Desktop. This head should not merge until the race is addressed and retested.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Treat a no-op stop as a failed interruption before root send. When Enter captures turn A, A can settle and turn B can become the Host root before this IPC runs. createRuntimeHostSessionStop returns undefined if expectedTurnId no longer matches (runtime-host-session-execution-ipc-main.ts:952-970), but this wrapper returns true for that result. interruptBeforeRootSend then permits the root send while B is still running, defeating the intended interrupt-before-send guarantee. A direct probe with sessions.stop returning undefined yielded rootSendAllowed: true. Please distinguish an actual interrupted result from a stale/no-op result for the Enter path, and add a settlement-race regression.

[Astro-Han]

Review of head b1f8095. I checked the earlier review findings and reviewed the full PR.

Status: GitHub reports this PR as CONFLICTING with main. apps/desktop/src/renderer/app-shell.tsx has two hunks that conflict with #5892 and #5893, and renderer-architecture.json also conflicts. Because of the conflict, no CI checks ran on this head. The hosted test and Desktop E2E results from earlier heads (b72a5c4 and before) do not cover the stop no-op change. The streaming-remount E2E in particular now depends on the stop returning interrupted.

Earlier findings

• hqhq1025 P2 (b72a5c4), a stop that does nothing reported as a successful interrupt: fixed. app-shell-stop-action.ts now returns result?.kind === 'interrupted'. I changed it back to return true as a check, and the new regression tests fail (2 failures), so the fix is covered.
• me2seeks P2, stop-path types loosened to any: fixed. There is now a typed ShellErrorToastApi and an explicit return type.
• Astro-Han P2, Enter help and scope: fixed at 0bd9c71. Whether Enter should interrupt is still a product decision for the maintainers.
• Astro-Han P3s: expectedTurnId pinning, the toast on session switch, and the OpenAI-backed Responses test are fixed. Three are still open:
  • hashing the empty-step signature (see the inline comment);
  • the policy that cuts off legitimate tool-only polling;
  • the unrelated edits to scripts/protocol-epoch-check.mjs and scripts/biome-staged-check.mjs.

New findings

• P2: a new system_note kind is added without a compatibility-epoch bump (see the inline comment on packages/core/src/session.ts).
• P3: when plain Enter's stop does nothing, the send is dropped silently (see the inline comment).
• P3: about 40 unrelated comment rewrites from // to /* */ in apps/desktop/src/renderer/app-shell.tsx. They have no functional effect and are the direct cause of the first merge conflict, at the main-process-interruption block that moved in #5892. Please revert them while rebasing.

Runtime bound: the logic looks correct. It only applies when maxSteps is unset. A step has a signature only when it has no visible text, no non-empty thinking and at least one tool call, and the signature includes the settled results. A steer resets the counter before the next step is evaluated. The stop is recorded as empty_step_loop, which maps to failed, gets its own notice, and is also handled in the CLI.

Verification (local): I ran npm ci and built core, storage, runtime, runtime-host and Desktop build:main. ui reports tsc errors in files this PR does not touch (settledText, menuAnchorRef, trailingAction); hqhq1025 saw the same errors. The runtime suites ai-sdk-backend, overflow-reactive-recovery, runtime-event-read-model and session-event-runtime-mapper pass 445/445. The Desktop tests follow-up-submit-routing, app-shell-stop-action and session-status-presentation pass 24/24. I did not run E2E or Electron.

Verdict: needs a rebase onto current main, an epoch decision for the new note kind, and a green CI run at the rebased head.

---

This is an automated review by Claude (Anthropic), run on behalf of the maintainer.

[Astro-Han]

P2: empty_step_loop is a new system_note kind, and both decoders check note kinds against a closed allowlist: decodeStoredMessage (session.ts:1727-1734, via isSystemNoteKind) and the RuntimeEvent system_note content check (runtime-event.ts:1125-1129). The Host projects this note into transcripts (runtime-event-read-model.ts:1537, read through runtime-host session-transcript-reader). RUNTIME_HOST_COMPATIBILITY_EPOCH stays at 202, so an older Client that passes the handshake will throw Invalid stored message schema on any Session where the bound fired. For example, the CLI decodes with decodeStoredMessage in runtime-host-session-driver.ts:119.

Epoch 106 was bumped for exactly this reason (see the comment at protocol/index.ts:287-295: "Session transcripts gain five system_note kinds"). protocol-epoch-check only watches the protocol directory, so CI will not flag this.

Fix: either bump the epoch with a comment (203 is already claimed by #5902, #5709, #5495 and #5753, and 204 by #5826, so coordinate), or render the notice on the client from failureClass === 'empty_assistant_loop' without adding a new stored note kind.

[Astro-Han]

P3: the no-op fix is correct, but now every case where stop returns something other than true drops the send silently. That covers three cases:

• the pinned turn finished on its own between the Enter snapshot and the IPC, which is a common race at the end of a turn;
• stopPending was already claimed (the user pressed Stop and then Enter);
• the stop threw, and the toast only appears if the session is still active.

The draft is kept (composer.tsx:1442), but pressing Enter visibly does nothing. Suggestion: when stop returns false, read liveTurns and runningTurnIds again. If nothing is active any more, go ahead with the root send. If a different turn is now running, show a toast (or retry once against the new turn).

[Astro-Han]

P3 (carried over from the earlier review, still open): this does a full JSON.stringify of every tool input and settled result on every textless tool step, including screenshots and large file reads. Hashing it (for example sha256 over a bounded or canonical serialization), or skipping the bound when a result is large or binary, would avoid the cost and avoid holding a second copy of the batch in memory.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 92a5d376. The previous reviewed head was b1f80956. The increment is a merge of main, which moved the merge-base from 57964425 to 7c90bac2, plus 92a5d376 ("drop exports orphaned by the composer-submission merge"). To leave out main's own changes, I compared each head's diff against its own merge-base. The PR is now 30 files, +1324/-79.

Status: MERGEABLE. The earlier conflict with main is resolved, and app-shell.tsx and renderer-architecture.json are no longer in the diff. CI: test is still pending on this head, so there is no result yet. Merge is BLOCKED because review is required.

Status of the previous findings

• P2, empty_step_loop system_note added without an epoch bump: fixed. RUNTIME_HOST_COMPATIBILITY_EPOCH goes from 204 to 205, with a comment explaining it: old Clients reject the unknown note kind at decode (protocol/index.ts:107-110). protocol.test.ts asserts that the epoch is above 204, and the report-sanitize.mjs allowlist gains the new kind.
• P3, Enter silently did nothing when stop was a no-op: fixed. After a stop that is not interrupted, the helper re-reads the live turns and running turn IDs (interrupt-before-root-send.ts:112-117). If nothing is active any more, the send goes ahead; otherwise it shows a localized "send blocked" toast (:118-129).
• P3, unrelated // → /* */ comment rewrites in app-shell.tsx: fixed. The file is no longer touched.
• P3, unrelated scripts (protocol-epoch-check.mjs, biome-staged-check.mjs): fixed. They are no longer in the diff.
• P3, carried over: full JSON.stringify of tool inputs and results on every textless step: partly addressed, with a new gap. See below.

New findings (no P0, P1 or P2)

• P3: epoch 205 collides with #5709. #5709 ("filter usage queries by model call kind", open) also changes the epoch from 204 to 205. Whichever PR merges second has to rebase to 206 and update its comment. Otherwise two incompatible wire changes share one epoch, and a 205 peer from one branch would be accepted by a 205 peer from the other. Please coordinate with #5709 before merging.
• P3: the oversized-signature guard defeats the loop bound, and the serialization cost is still paid. hashEmptyStepSignature serializes the whole batch before comparing it with the 64 KiB cap (ai-sdk-turn.ts:606-609). So large results, such as screenshots or big file reads, are still fully stringified on every textless step, which the cap was meant to avoid. Then, because the result is undefined, the else branch calls clearEmptyStepProgress() (:2359). A model looping on an identical large tool result therefore never trips the bound. That is the case most likely to flood the transcript. No test covers a payload over the cap. Suggested fix: hash the batch incrementally, or with a size-bounded digest such as a per-result hash or a length plus prefix/suffix. At minimum, treat "too large to hash" as "no progress evidence" rather than as progress.
• P3: duplicate or premature toasts on the Enter interrupt path.
  • If services.stop throws, stop-action.ts already shows the stop-failed toast and returns undefined (stop-action.ts:66-75). interruptBeforeRootSend then re-reads, still sees the turn active, and shows a second "send blocked" toast (interrupt-before-root-send.ts:123).
  • If a Stop click is already in progress, stopPending.claim fails and stop returns undefined immediately (stop-action.ts:53). Enter then shows "send blocked" even though the stop the user already requested may land a moment later.
  • Suggested fix: have stop return a three-way result (interrupted, no-op, failed or busy), so the helper does not toast after a failure that was already toasted, and waits for or reports an in-flight stop.

Validation: I read the code at the new head and checked each claim against it. I did not rerun tests locally this round. The renderer and runtime changes since b1f80956 are mostly the merge with main and the epoch bump.

Verdict: the earlier P2 and most P3s are resolved. What remains is the epoch coordination with #5709 and two minor P3s. CI still needs to finish.

[Astro-Han]

P3, coordination: #5709 (open) also bumps 204 → 205, for usage-query call-kind filtering. Whichever of the two merges second needs to move to 206 and update its comment. Otherwise two different wire breaks share one epoch.

[Astro-Han]

P3: this serializes the whole batch before comparing it with the cap, so the cost the cap is meant to avoid (large results such as screenshots) is still paid on every textless step. Also, undefined for an oversized step leads to clearEmptyStepProgress() (:2359), so a loop on an identical large result never trips the bound, and no test covers a payload over the cap. Consider a size-bounded digest (for example a hash per result) and treating "too large to hash" as "no progress evidence" rather than as a reset.

[Astro-Han]

P3: when stop throws, createStopAction already shows the stop-failed toast and returns undefined (stop-action.ts:66-75). This then shows a second "send blocked" toast. When a Stop click is in flight, stopPending.claim fails (stop-action.ts:53), so Enter shows "send blocked" even though that stop may land a moment later. A three-way result from stop (interrupted, no-op, failed or busy) would let this helper skip or adjust the toast.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 0c0381eb. The previous reviewed head was 92a5d376. The merge-base is unchanged (7c90bac2), so the increment is exactly two commits: 8a0ed2575 ("hash empty-step signatures incrementally") and 0c0381eb2 ("report stop outcomes so Enter interrupt toasts once"). That is +329/-47 across 9 files; the PR is now +1608/-81.

Status of the previous findings

• P3, epoch 205 collides with #5709: still open (coordination only). This PR still sets RUNTIME_HOST_COMPATIBILITY_EPOCH = 205 (protocol/index.ts:107). Main is at 204. #5709 is still open at 2aed8926, and it also sets 205. Whichever PR merges second needs to move to 206 and add its own comment line.
• P3, loop-bound gap (stringify before the size check; an oversized step reset the counter; no test): fixed. hashEmptyStepSignature now feeds the value to SHA-256 piece by piece (ai-sdk-turn.ts:603-663): length-prefixed strings, typed arrays hashed as bytes, toJSON honoured, undefined properties dropped, and a guard against cycles. Nothing is serialized up front, and there is no cap, so it always returns a digest. At the call site (:2368-2412) an oversized identical step therefore counts toward the bound instead of calling clearEmptyStepProgress(). New tests: a 256 KiB identical result stops with empty_step_loop after 3 calls, and a 256 KiB result that changes only at its end runs to end_turn (7 calls), so there is no false trip.
• P3, duplicate or premature toasts on Enter-interrupt: fixed. stop now returns 'interrupted' | 'not_running' | 'failed' | 'busy' (stop-action.ts:39). A stop that is already running for the Session is shared through inFlight (a map held in useState), so Enter waits for the user's Stop click and gets its real outcome instead of toasting "send blocked" early. failed returns with no second toast, because the stop action already toasted it. busy (the claim is held by a stop this action cannot await) shows the new "stopping" description. not_running re-reads the active turns, as before. Tests cover each branch.

New findings: none at P0-P3.

CI and merge state: test passes on this head. The PR is MERGEABLE; merge is BLOCKED because review is required. I reviewed the code only this round and did not run tests locally.

Verdict: both behavioural P3s are resolved. The only remaining item is the epoch-205 coordination with #5709 at merge time.

[Astro-Han]

P3, coordination (still open): #5709 (open at 2aed8926) also sets this to 205, and main is at 204. Whichever of the two merges second should move to 206 and add its own comment line.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 2d8d5317. The previous reviewed head was 0c0381eb. The merge-base is unchanged (7c90bac2), so the increment is exactly one commit, 2d8d5317c ("take epoch 206 to avoid colliding with #5709"). It changes 2 files, +7/-4. The PR is now +1611/-81 across 30 files.

Status of the previous finding

• P3, epoch 205 collides with #5709: resolved. RUNTIME_HOST_COMPATIBILITY_EPOCH is now 206 (packages/runtime-host/src/protocol/index.ts:107), with the // 206: ledger line. The guard test moves to > 205 (packages/runtime-host/src/__tests__/protocol.test.ts:255). #5709 is still open at 2aed8926 and still sets 205, with its own // 205: line. The two values are now textually different, so whichever PR merges second gets a real git conflict instead of a silent same-number merge (#3313). Merge order is also safe both ways. If #4138 lands first, a later #5709 at 205 is rejected by scripts/protocol-epoch-check.mjs:104 ("went backward"), which forces it to 207. If #5709 lands first, 206 > 205 passes. If #5709 is abandoned, 205 is simply unused; the check only requires the head to exceed the base.

New findings

• P3, comment hygiene (optional). The ledger comment (protocol/index.ts:110-111) and the test comment (protocol.test.ts:253-254) say "open #5709 already claims 205". That is true today but goes stale whichever way #5709 resolves: it may merge as 205 or 207, or close. Something durable would age better, for example "205 is reserved by #5709 (UsageQuery.callKinds)" in the ledger, and simply "Epoch 204 peers reject ..." in the test. The > 205 assertion itself is fine.

CI and merge state: test fails on this head (run 37183922896, attempt 1), but the failure does not look attributable to this change. The only failing step is Desktop e2e: apps/desktop/e2e/workhub-layout.spec.ts:156, expect(addPanel).not.toHaveAttribute('aria-expanded', 'true'), with Protocol error (Runtime.callFunctionOn): Internal server error, session closed. That is the native Linux workbar Menu.closePopup race that the spec's own comment (:145-149) says can crash the main process. This PR touches no workbar or menu code. The increment is a constant plus comments. The same job passed on the previous head 0c0381eb (run 37178079857). A re-run should clear it, but test has to be green before merge. The PR is MERGEABLE; BLOCKED.

Verdict: the epoch coordination item is resolved. There is one optional comment-wording nit, and CI needs a re-run of the flaky e2e job.

[Astro-Han]

P3 (optional, wording): "open #5709 already claims 205" goes stale as soon as #5709 merges, as 205 or 207, or closes. A durable form such as "205 is reserved by #5709 (UsageQuery.callKinds)" would age better. The same applies to the test comment at protocol.test.ts:253-254. The 206 value and the > 205 guard are fine: the different number now forces a real merge conflict with #5709, and protocol-epoch-check.mjs rejects a backward 205 if this PR lands first.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 47e2a74a. The previous reviewed head was 2d8d5317. The merge-base is unchanged (7c90bac2), so the increment is exactly one commit, 47e2a74af ("word the epoch 205 reservation durably"). It changes 3 files, +7/-4. The PR is now +1614/-81 across 31 files.

Status of the previous finding

• P3, the "open #5709 claims 205" comments would go stale: resolved. The ledger line now reads "205 is reserved by #5709 (UsageQuery.callKinds)" (packages/runtime-host/src/protocol/index.ts:110-111). The test drops the PR-state sentence and keeps the > 205 assertion with only the "Epoch 204 peers reject ..." rationale (protocol.test.ts:250-254). The epoch value (206) and the coordination with #5709 are unchanged. #5709 is still open at 2aed8926 with 205.

New finding

• P3 (optional): the new protocol-compatible-changes/epoch-206-ledger-wording.json is redundant for the merge gate and will read oddly once it lands. I assume it was added because the staged pre-commit check compares against the branch's previous commit, which is already at 206, so a comment edit in index.ts needed either a declaration or a bump. The merge-result check, however, compares against the base. That moves 204 → 206, and evaluateEpochCheck ignores declarations whenever the epoch moved (scripts/protocol-epoch-check.mjs, the headEpoch === baseEpoch branch), so the file grants nothing. It has two small costs. (1) Its reason says RUNTIME_HOST_COMPATIBILITY_EPOCH and every shape are unchanged. That is true for this commit but not for the PR against main, where index.ts publishes a wire-incompatible epoch. After merge it sits next to the other declarations and looks as if 206's index.ts change had been declared compatible. (2) parseDeclaration requires every declaration added against the base to match the head epoch. So if this PR ever has to re-bump (for example, another PR takes 206 first), this file must be re-pinned too, or the check fails. Options: drop it (for example, fold the wording change into the earlier epoch commit so the hook sees 204 → 206), or scope the reason to "comment-only follow-up within the 206 bump of this PR". Either is fine, and so is leaving it.

CI and merge state: test passes on this exact head (run 37187679136, attempt 1). The e2e flake from the previous head did not recur. The PR is MERGEABLE; merge is BLOCKED because review is required.

Verdict: the previous nit is resolved. There is one optional nit about the redundant declaration file. Nothing blocks merge from this review's side.

[Astro-Han]

P3 (optional): this declaration is needed only by the staged pre-commit check, whose HEAD is already at 206. Against main the PR moves the epoch 204 -> 206, and evaluateEpochCheck ignores declarations when the epoch moved, so the file grants nothing at merge. Once landed, the reason ("RUNTIME_HOST_COMPATIBILITY_EPOCH ... unchanged") reads as if this PR's index.ts change were wire-compatible, which it is not. It also has to be re-pinned if the PR ever re-bumps, because parseDeclaration requires epoch === headEpoch for added declarations. Consider dropping it (fold the wording into the epoch commit) or scoping the reason to a comment-only follow-up within this PR's 206 bump.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head aa1b4b45. The previous reviewed head was 47e2a74a. The merge-base is unchanged (7c90bac2), so the increment is exactly one commit, aa1b4b450 ("drop redundant epoch 206 wording declaration"). It deletes one file, -5 lines. The PR is now +1609/-81 across 30 files. Comparing the old head's diff against the new head's diff (each against the same merge-base), the only difference is that packages/runtime-host/protocol-compatible-changes/epoch-206-ledger-wording.json no longer appears. Every other hunk is byte-identical.

Status of the previous finding

• P3 (optional), redundant epoch-206-ledger-wording.json declaration: resolved. The file is removed. The merge gate still passes without it. main is at epoch 204 and this head is at 206 (packages/runtime-host/src/protocol/index.ts:107), so evaluateEpochCheck takes the epoch-moved path and needs no declaration (scripts/protocol-epoch-check.mjs, the headEpoch === baseEpoch branch). The ledger wording "205 is reserved by #5709" (index.ts:110-111) and the > 205 test assertion (protocol.test.ts:250-254) are unchanged. #5709 is still open at 2aed8926 with 205.

New findings: none.

CI and merge state: test passes on this exact head (run 37200250335, attempt 1, completed 12:15Z). It is the only check run. The PR is MERGEABLE; merge is BLOCKED because review is required.

Verdict: the previous optional nit is resolved and the increment introduces nothing new. Nothing blocks merge from this review's side.