fix(tui): redo timezone loop + restore session browser selection#1518
Merged
Aaronontheweb merged 5 commits intoJun 29, 2026
Merged
Conversation
The IdentityStepView is a singleton on IdentityRedoViewModel. Each layout rebuild creates new TextInputNode instances with new subscriptions added to _stepSubs, but old subscriptions were never cleared. After the user submitted the timezone field, stale callbacks from the previous render would re-fire and re-trigger GoNext(), creating an infinite loop back to the timezone input screen. Clear _stepSubs inside the DynamicLayoutNode callback before creating new subscriptions to prevent this accumulation.
…coverage The PR's SessionsPageTests referenced a non-existent SessionsViewModel.ResumeSessionId (resume state lives on ChatNavigationState), which broke the build on every Test-* job. Rewrite it to observe the shared ChatNavigationState, register a stub /chat landing route so the Enter/N resume paths don't throw 'No route matches', use the real SessionId (strips the 'session-' prefix), and enqueue Ctrl+Q in the render-only cases so they stop burning the 10s cancellation timeout. The corrected tests pass against current dev: arrow keys reach the ViewModel, SelectedIndex tracks, and Enter resumes the highlighted session — the two-way selection binding netclaw-dev#1363 preserved is intact, so the described regression does not reproduce in dev. Add IdentityRedoPageTests: first page-level coverage of the identity-redo flow through Termina's real render + input pipeline, asserting the timezone submit reaches the saved screen (the user-visible symptom of the redo loop).
Adds init-redo-identity.tape (+ sibling assertion) to the light smoke set: the existing-install menu had a redo path with no native coverage. The tape walks the menu -> 'Redo identity setup' -> all four identity sub-steps to the saved screen, and the assertion proves the redo wrote SOUL.md/TOOLING.md without clobbering netclaw.json (redo never calls WriteConfig). This is forward-progress coverage: a regression that blocks advancing past any redo sub-step (the symptom of the timezone loop) hangs the Wait+Screen anchor and fails the tape. It deliberately does NOT claim to reproduce the original loop, whose trigger is a non-step-transition re-render (cursor-blink/resize) that deterministic VHS input does not generate — verified by reverting the fix and watching the tape still pass. Runs sequentially within the already-parallel smoke-linux/smoke-macos jobs; ~40s added on the critical path (~3-4% of the ~20m smoke job).
Aaronontheweb
enabled auto-merge (squash)
June 29, 2026 19:05
The session list is rendered as a focusable SelectionListNode (FocusPriority 10, added by netclaw-dev#1363). On a populated list, ReactivePage.ApplyFocusPolicy hands it keyboard focus, and Termina's input dispatch routes keys to the focus manager (SelectionListNode.HandleInput consumes UpArrow/DownArrow/Enter/Escape, returning true) BEFORE they ever reach the ViewModel. SessionsPage had no HandlePageInput override and relied on a ViewModel.Input subscription, so on a real terminal the focused list swallowed every navigation/selection key: the highlight could not be driven, SelectedIndex stayed 0, and Enter resumed nothing — 'you can't select anything in netclaw sessions'. Fix: claim the keys in SessionsPage.HandlePageInput (dispatched before the focus manager) and forward them to a new public SessionsViewModel.HandleKey. The list stays a pure renderer driven one-way by .WithHighlightedIndex(SelectedIndex). This matches what HandlePageInput is documented for ('page-level input before focused components') and the established pattern in the other list pages. Why no test caught it: the empty-state tape (sessions-tui) renders a TextNode, not a focusable list, so it never hit the path; and the headless xUnit harness does not hand the list focus during input processing, so keys fell through to the ViewModel and the prior tests were false positives. They now drive the real HandlePageInput path — removing the override fails them — but faithful focus-eating coverage still needs native/manual validation.
Aaronontheweb
changed the title
Aaronontheweb
force-pushed
the
fix/identity-redo-timezone-loop
branch
from
June 29, 2026 19:48
e471da9 to
65365af
Compare
Open
4 tasks
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two TUI bug fixes, rebased onto current
dev. Supersedes #1515, whose branch wasbased on stale
dev, bundled in the already-merged GitHub Enterprise Copilot auth feature(#1509), and shipped a
SessionsPageTeststhat referenced a non-existentSessionsViewModel.ResumeSessionId(the compile error that failed everyTest-*job).1. Identity redo timezone loop
IdentityStepViewis a singleton onIdentityRedoViewModel. Every content rebuild —including Termina's cursor-blink-timer re-renders (the class of bug behind #792) — creates
fresh
TextInputNodes and adds newSubmittedsubscriptions to_stepSubs, which wereonly cleared on step transitions, not on rebuilds. Stale callbacks accumulated and re-fired
on the timezone submit, bouncing the user back to the timezone field.
Fix: clear
_stepSubsat the top of the contentDynamicLayoutNoderebuild — thedocumented
StepViewCallbacks.Subscriptionscontract ("Cleared when the step content isrebuilt") that the sibling
InitWizardPagealready honours.2. Session browser selection —
netclaw sessionsReal regression, root-caused from the Termina source. #1363 ("make all TUI list views
scrollable") rendered the session list as a focusable
SelectionListNode(FocusPriority10). On a populated list,
ReactivePage.ApplyFocusPolicyhands it keyboard focus, andTermina's input dispatch routes keys to the focus manager before the ViewModel —
SelectionListNode.HandleInputconsumesUpArrow/DownArrow/Enter/Escape(returnstrue).
SessionsPagehad noHandlePageInputoverride and relied on aViewModel.Inputsubscription, so on a real terminal the focused list swallowed every navigation/selection
key: the highlight couldn't be driven,
SelectedIndexstayed 0, and Enter resumednothing — "you can't select anything."
Fix: claim the keys in
SessionsPage.HandlePageInput(dispatched before the focusmanager) and forward them to a new public
SessionsViewModel.HandleKey; the list stays apure renderer driven one-way by
.WithHighlightedIndex(SelectedIndex). This is exactlywhat
HandlePageInputis documented for ("page-level input before focused components").Tests
SessionsPageTests(rewritten): observes the sharedChatNavigationState, registersa stub
/chatroute, uses the realSessionId. Now drives input through the realHandlePageInputpath — removing that override fails them — so they guard the page→VMwiring (the previous tests were false positives). They do not reproduce the focus-eating
itself; that needs native/manual coverage (documented in the header).
IdentityRedoPageTests(new): first page-level coverage of the redo flow to the savedscreen.
init-redo-identitynative smoke tape + assertion: forward-progress coverage for theredo flow (menu → all four sub-steps → saved; identity files written, config preserved).
Notes
github-code-qualityadvisory on fix(tui): prevent identity redo loop + fix session browser selection bug #1515 (HttpResponseMessagenot disposed) isresolved by matching the suite's existing mock-handler convention rather than a
[SuppressMessage](which would be inconsistent and risk a Slopwatch flag).