Skip to content

fix(runtime-host): project queue changes that land while no root Turn is live - #5521

Merged
Astro-Han merged 6 commits into
apache:mainfrom
ggbdpq:fix/queue-drain-projection
Sep 27, 2026
Merged

Astro-Han merged 6 commits into
apache:mainfrom
ggbdpq:fix/queue-drain-projection

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5520

A queue that drains while no root Turn is live was never projected: the session projector gated queue_update on root && queueChanged(...), and seedActive returned nothing for a rootless snapshot. The Desktop's queue mirror is driven only by queue_update, so the renderer kept a phantom queued card whose 删除/retract failed with not_found forever — and switching sessions back and forth did not clear it, because the reseed hit the same gate. The Host broadcast the projection frames all along (#publishCanonical fires on any canonical change); the drop was purely in the projector adapter.

The fix stays at that shared root:

  • session-projector.ts now pushes queue_update whenever the queue changed, root or not, attributed to root?.turnId ?? previousRoot?.turnId ?? ''. The steering synthesis stays root-gated (it attributes messages to the live Turn).
  • seedActive seeds the queue mirror for a rootless session too — the empty queue is exactly the signal that lets an observer clear a stale card on (re)subscribe.

No consumer change is needed: the Desktop already deletes messageQueueBySession[sessionId] when a queue_update carries empty arrays, and the TUI mirrors the arrays verbatim.

Verification

Check Result
New regression tests (drain without root / seed without root) red before the fix, green after
session-projector.test.ts (full file, rebuilt dist) 29/29
message-coordinator.test.ts 76/76
execution-host-queue.test.ts 16/16 — one "did not become ready" timeout on the first full-file run; passed single and on full-file rerun (spawn-heavy flake, not code)
session-subscription-client.test.ts 13 failures reproduce identically on a pristine upstream/main worktree (Windows transport read side ended) — pre-existing, untouched by this PR
biome format/lint on touched files clean
npm run check:asf-headers pass

Not run locally: the full runtime-host matrix (~40 min; CI covers it) and the issue's Desktop manual repro (reported on macOS; the fixed root cause and its regression tests are platform-independent).

AI use

Analysis, patch, and tests done with GLM-5.3-Flash (ZCode) under human review.

Checklist

  • The fix lives at the shared root (projector), not in an individual caller
  • Regression tests fail before the fix and pass after
  • No protocol/event contract change — only the emission gating of the existing queue_update
  • ASF headers intact; biome clean on touched files

… is live

The session projector pushed `queue_update` only when a root Turn existed
(`root && queueChanged(...)`), and `seedActive` returned no events at all
for a snapshot without a root Turn. A queue that drained inside that
window - the queued follow-up consumed as the Turn went terminal, or a
drain between subscriptions - was therefore never projected: the renderer
kept the queued card, and every retract of it failed with `not_found`
(`queue.entry.retract`) while the failure path deliberately left the
projection unchanged. Switching sessions back and forth did not clear it
either, because the reseed hit the same gate.

Project the authoritative queue whenever it changes, independent of a
live root Turn (attributing the event to the previous root when the drain
lands in the same frame the Turn disappears), and seed the queue mirror
on subscribe even for an idle session - the empty queue is exactly what
tells an observer to drop a stale card. The steering synthesis stays
root-gated: it attributes messages to the live Turn.

Fixes apache#5520

Generated-by: GLM-5.3-Flash (ZCode)
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 20, 2026
The Desktop session observer pins an empty seed event list for a snapshot
without a root Turn ("projects root lifecycle without fabricating content
events"), and the projector snapshots it seeds from can carry no queue at
all - the unconditional rootless seed both fabricated a queue_update the
contract forbids and crashed on the missing field.

The connected path is where the apache#5520 phantom actually lives: the drain
lands while the client is subscribed (session events route per session,
not per visible view), so projecting it there clears the card. A client
that never observed the session has no stale card for a seed to clear,
which is why seeding stays silent.

Fixes the regression introduced in 7b4a974.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

The CI red was a real regression in my first push, thank you for catching it — diagnosed and fixed at 5dac0705a.

What failed (3 tests, all Desktop-side consumers):

  • runtime-host-desktop-candidate × 2 — TypeError: Cannot read properties of undefined (reading 'hostEpoch'): the observer constructs projectors from snapshots that can carry no queue at all, and my unconditional rootless seed fed that straight into projectQueueUpdate.
  • runtime-host-session-observer "projects root lifecycle without fabricating content events" — this test pins the contract that a rootless seed emits []; my seed hunk fabricated a queue_update against it.

The fix: I reverted the seed hunk entirely (seedActive is back to returning [] for rootless snapshots) and kept only the accept-path change — which is where the #5520 phantom actually lives. The drain lands while the client is subscribed (session events route per session, not per visible view), so projecting it there clears the card whether or not that session is on screen; a client that never observed the session has no stale card for a seed to clear, which is exactly why the observer contract is what it is. The surviving regression test now documents that reasoning.

Verified locally after the fix: session-projector 28/28, runtime-host-session-observer 55/55, runtime-host-desktop-candidate 25/25 — the last two are the suites CI caught, which my local matrix had missed because the projector's consumers live in the Desktop workspace, not runtime-host. That gap is on me; the affected-workspace set is now part of my run book for this area.

@github-actions github-actions Bot added effort/S Under 100 readable lines and removed effort/M Under 500 readable lines labels Sep 21, 2026
…jection

# Conflicts:
#	packages/runtime-host/src/adapter/session-projector.ts

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding

[P1][Normal user path] Reopening an idle Session still cannot clear a queue entry that drained while the Session was inactive.

The accept-path change correctly emits an empty queue_update when the drain arrives while the client remains subscribed. However, Desktop subscribes only to the active Session and unsubscribes when activeId changes (apps/desktop/src/renderer/app-shell-effects.ts). If the queued follow-up is consumed while that Session is inactive, there is no projector instance present to observe the drain.

When the user returns, the new projector is created from the current rootless snapshot, but RuntimeHostSessionProjector.seedActive() still returns [] as soon as rootTurn is null (packages/runtime-host/src/adapter/session-projector.ts). The renderer retains messageQueueBySession[sessionId] across navigation, and it removes that stale entry only after receiving an empty queue_update. Therefore the card remains visible and retracting it still returns not_found, which is one of the issue's documented normal reproduction paths. A direct probe against this exact head also returned an empty seed for a rootless snapshot with an authoritative empty queue.

Please make the rootless observation seed carry the authoritative queue state (or add an equivalent out-of-band queue snapshot) without violating the observer's no-content-event contract, and add a Desktop-level regression that switches away, drains the queue, then switches back. The PR description currently says that seedActive performs this rootless reseed, but that change was reverted from the final code.

Review summary

The continuously subscribed drain path and its focused projector regression are correct; the projector suite passed 27/27. Biome, ASF headers, diff checks, the hosted exact-head test check, and a clean synthetic merge onto current main all passed. Because the off-subscription production path still reproduces the reported phantom card, this exact head a5f8cb028957d8302f870beff409afb44e0d1ff3 is blocked. I did not approve or merge.


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

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding

[P1] The normal switch-away / switch-back path still leaves a phantom queued card. The change in packages/runtime-host/src/adapter/session-projector.ts:406-418 emits the authoritative empty queue when a subscriber stays connected through a rootless drain. Desktop instead unsubscribes from the old Session on navigation (apps/desktop/src/renderer/app-shell-effects.ts:400-458). If the queue drains while that Session is inactive, the new projector returns no events for its rootless snapshot (packages/runtime-host/src/adapter/session-projector.ts:161-168), while the renderer retains messageQueueBySession[sessionId] until it receives an empty queue_update (apps/desktop/src/renderer/app-shell-session-events.ts:303-323). The stale card therefore remains visible, and retract can return not_found. Please make resubscription convey the authoritative empty queue and cover switching away, draining, then switching back in a Desktop regression. I independently corroborate the same current-head issue already reported by jackwener, so I am not duplicating an inline comment.

Scope and readiness

This PR changes the projector drain emission and adds a focused continuously subscribed regression (2 files, +44/-2). I reviewed the projector, Desktop subscription lifecycle, queue state reducer, and the new test. I found no other substantiated P0–P3 issue in that scope. The current-head test check passed, and the diff and synthetic merge onto fetched main are clean; the remaining normal navigation path still blocks merging. I did not run the suite locally or exercise a real Desktop session switch. No schema or migration files changed.

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.

A Desktop that navigates away unsubscribes, so the projector fix that
projects a rootless drain never reaches it: the drain lands while no
subscriber is connected, and on navigating back the fresh projector's
seedActive returned no events for a rootless snapshot. The client's
stale queued card survived — retracting it fails with not_found — until
an update that will never come.

The rootless seed now emits the authoritative queue once, empty or not;
the Desktop queue projection keys by Session and retires the card on
the empty update.

Carries the apache#5520 review finding; covered by a projector seed test and
a Desktop queue-projection regression for switch away, drain, switch
back.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Fixed at 65cb72209: seedActive no longer returns an empty seed for a rootless snapshot — it emits the authoritative queue once (empty or not, through the same unplacedQueue filter the update path uses), which the Desktop queue projection keys by Session and uses to retire the stale card.

Covered at both layers: a projector seed test (rootless seed → one empty queue_update) and a Desktop queue-projection regression for switch away → drain → switch back. The full navigation test through app-shell-effects would need the e2e harness; the two-layer coverage pins both halves of the contract.

Desktop test fixtures build rootless continuity snapshots without a
queue field, and the seed's new authoritative-queue emission read
straight into unplacedQueue. Project an empty queue when the snapshot
carries none, matching the update path's tolerance; the observer seed
expectation now reflects the queue_update the rootless seed carries.

Generated-by: GLM-5.3-Flash (ZCode)

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head eba044200976a9e72ca15b76033ea7a369a3cc98. I found no substantiated remaining P0–P3 issue in the changed queue-projection path.

The projector now emits queue_update when the queue changes even without a live root Turn, and seedActive() also sends an authoritative queue for a rootless snapshot (packages/runtime-host/src/adapter/session-projector.ts:161-175,416-429). This addresses the previous resubscription gap: when Desktop navigates away while the queue drains and then returns, the rootless seed reaches its existing empty-queue reducer, which deletes the stale card (apps/desktop/src/renderer/app-shell-session-events.ts:303-321). Added projector and Desktop tests cover the rootless drain, rootless seed, and stale-card clearing. No schema/migration files change.

At review time the current-head test check succeeds and the PR merges cleanly with freshly fetched main at the Git merge-tree level; GitHub still reports a blocked merge state, so this is not a merge-readiness claim. Diff-check is clean. I did not run tests locally or a real Desktop navigate-away/re-subscribe scenario. Human validation and merge decision remain outstanding.

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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at @Astro-Han's explicit request: a small, focused fix with no blocking findings in the automated review of this head and green CI.

@Astro-Han
Astro-Han merged commit cc6dcca into apache:main Sep 27, 2026
1 check passed
Shouly pushed a commit to Shouly/maka that referenced this pull request Sep 27, 2026
A queue change that landed after the root Turn was gone projected nothing,
and a rootless seed projected nothing either, so a queued card a client still
held stayed on screen and its retract failed with not_found. The projector now
emits the authoritative `queue_update` for every queue change and once on a
rootless seed. The Desktop candidate harness's default snapshot gains the
queue our protocol requires; upstream instead defaults a missing queue in the
projector.

Lead: apache#5521 (cc6dcca).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shouly pushed a commit to Shouly/maka that referenced this pull request Sep 27, 2026
Watermark bfb315a. Done: apache#5573/apache#5600/apache#5601, apache#5521, apache#4875, apache#5723, apache#5738,
apache#5742. Not applicable: apache#5737, apache#5593. Deferred: apache#5730. Consider: apache#5599,
apache#5120, apache#5693. Diverged: apache#5740. Skipped: ACP, WorkHub, upstream renderer and
packages/ui, one refactor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

4 participants