close
Skip to content

refactor(cli): drop dead fallback retry and followup-takeover paths - #3650

Open
rbalachandar wants to merge 1 commit into
apache:mainfrom
rbalachandar:refactor/cli-dead-queue-fallbacks
Open

refactor(cli): drop dead fallback retry and followup-takeover paths#3650
rbalachandar wants to merge 1 commit into
apache:mainfrom
rbalachandar:refactor/cli-dead-queue-fallbacks

Conversation

@rbalachandar

Copy link
Copy Markdown
Contributor

Summary

Removes two CLI code paths whose last real consumer is gone now that the Runtime Host is the complete queue authority (per-entry queue ops, protocol epoch 40, #3544). Covers sections 1 and 2 of the audit; section 3 (runtime-kernel embedded queue API) is left for a separate PR pending the embedded-composition decision.

  • Section 1 — fallback retry machinery (~650 lines): the only production driver (RuntimeHostMakaSessionDriverImpl) returns fallback solely when no sessionId exists, while every fallback producer (steer during a running turn, alt+enter queue) requires a live turn and therefore a session. Removes the retry timer loop, deferred-fallback state, turn-boundary flush, and the pending-bar merge, plus their tests.
  • Section 2 — takePendingFollowup (~30 lines): the production stub is always null (the Host starts queued follow-ups atomically; returning text would double-submit), so the runner's re-queue/nextPrompt fold was unreachable outside test doubles. Removes the interface method, stub, consumer block, and doubles.

Kept as still needed (per the audit): trackEnqueue/settlePendingEnqueues (interrupt-path in-flight submit ordering), Host op queue.retract, sessions:steer/sessions:enqueue IPC, the session_busy fallback in sessions:send, and the QueueUpdateEvent steering/followup mirrors the pending bar renders.

Refs #3556

Verification

  • npm run build — pass
  • npm run lint (biome, 2652 files) — pass
  • npm run format:check — pass
  • npm run typecheck — pass
  • packages/cli workspace suite: 412/412 pass
  • Repo-wide grep: no remaining references to pendingFallback, takePendingFollowup, or the fallback retry helpers
  • Not run: knip workspaces (apps/desktop, packages/ui — untouched by this PR), full multi-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: Claude Code — reviewed the removal against the issue inventory, verified kept/removed boundaries, ran verification; human contributor of record reviewed and owns the change.

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

The Runtime Host is now the complete queue authority (per-entry queue ops,
protocol epoch 40, apache#3544), which leaves two CLI code paths with no
production consumer. Covers sections 1 and 2 of apache#3556; section 3
(runtime-kernel embedded queue API) is intentionally left for a separate
PR pending maintainer confirmation.

Section 1 — fallback retry machinery: the only production driver
(RuntimeHostMakaSessionDriverImpl) returns `fallback` solely when no
sessionId exists, while every fallback producer (steer during a running
turn, alt+enter queue) requires a live turn and therefore a session.
Removes the retry timer loop, deferred-fallback state, turn-boundary
flush, pending-bar merge, and their tests. trackEnqueue /
settlePendingEnqueues stay: the interrupt path still needs in-flight
submit ordering.

Section 2 — takePendingFollowup: the production stub is always null (the
Host starts queued follow-ups atomically; returning text would make the
TUI double-submit), so the runner's re-queue/nextPrompt fold was
unreachable outside test doubles. Removes the interface method, stub,
consumer block, and the doubles' implementations.

Verified as still needed and kept: Host op queue.retract (CLI interrupt /
alt+up), sessions:steer/enqueue IPC, session_busy fallback in
sessions:send, and the QueueUpdateEvent steering/followup mirrors the
pending bar renders.

@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.

Verdict: NO-GO at exact head ad2d5a69cbd79e450688d7c40fa46b22da77c329. Simplify audit: removing the production always-null takePendingFollowup stub and its consumer is demand-free and sound, but the fallback producer is still reachable during first-session admission. A production-shaped delayed-prepare regression probe failed with actual prompts ['start'] versus expected ['start', 'must survive']; the current focused exact-head suites pass 249/249 because the fallback-specific coverage was deleted. Hosted exact-head test is green but does not exercise this window. One P1 inline; no other P0–P3 findings.

return;
}
// Queued: the runtime's `queue_update` event refreshes the mirror.
.then(() => {

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.

[P1] Preserve first-session input until Session admission completes

runAgentTurn() sets turnRunning = true before preparePrompt() awaits #ensureSession(). On a new TUI Session, #createSession() has not yet assigned #sessionId, so Enter here (and Alt+Enter in the matching hunk below) calls the production driver's #enqueue(), gets { kind: 'fallback' }, and this callback now ignores that outcome after the editor was cleared. A production-shaped delayed-prepare probe observed this and retained only ['start'] instead of ['start', 'must survive']. Either expose queue availability only after Session identity exists or retain a durable fallback handoff, and keep a delayed session.create regression.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants