Improve mobile terminal reliability and session tooling - #173
Conversation
- Replace the split extended-keyboard option with responsive Esc, arrows, Enter, and Tab controls for keyboard-hidden and keyboard-open layouts. - Add swipe navigation, Up+Down Enter chords, best-effort volume-key input, and optional haptic/sound feedback. - Keep the accessory layout keyboard-safe, phone-width contained, and hidden behind application modals. Verified: 21 unit tests, 64 mobile browser tests, public asset/syntax checks, and npm run build.
- arbitrate shared PTY dimensions from explicit interaction and input - force one desktop redraw when the terminal or session tab retakes control - isolate mobile and viewport browser tests from host state
- distinguish editable prompt rows from TUI-owned readbacks and choices\n- keep content taps keyboard-free while preserving prompt focus and cursor positioning\n- cover readback collapse in unit and mobile browser tests
- add admin-only graceful shutdown with exact supervisor ownership checks - add a keyboard-safe header confirmation flow with phone browser coverage
- keep multiline clipboard text inside one bracketed-paste frame across native, CJK, dialog, image fallback, and voice paths - hold the first local draft until replay establishes the real prompt position
- keep bracketed multiline text on the direct PTY path when WebSocket delivery falls back to HTTP - recognize Android input-only paste events and Enter races in the CJK field
- confirm multiline paste from Android insertText and line-break event bursts - preserve ordinary Gboard word-space input and desktop autocomplete behavior - cover full textarea, segmented, and non-paste input paths in browser tests
- clear xterm helper text after handled touch input - turn Android delete mutations into one terminal Backspace each - forward deletes when editable text already lives in the PTY
- keep transient IME candidates separate from committed draft text - commit finalized composition atomically without duplicate words - guard newer compositions from stale cleanup timers
- let the prompt-aware overlay own composition rendering - keep xterm composition processing active but visually hidden - cover native-helper and local-overlay coexistence
Commit the last Android composition when xterm omits its final data event. Suppress one delayed matching payload so continued typing does not duplicate the word.
Read insertText mutations from xterm's helper textarea when Android omits InputEvent.data. Cover continued typing after the first visible character.
Release stale composition state when Android resumes with an explicitly non-composing text mutation. Keep later submissions visible without reviving the abandoned candidate.
Route mobile-control Enter through local echo so committed and composing text is sent before Enter. Keep the next draft clean without focusing xterm or opening the keyboard.
Ark0N
commented
Aug 5, 2026
Hi Lior, thank you for all of this, and I owe you a straight answer rather than more silence. I'm closing this PR and the sixteen others, and I want to explain properly, because the reason is not that the work is worthless. The volume is the core problem. There are seventeen open PRs from you, sixteen of them opened on 2026-07-29, totaling roughly 42,000 added lines. This one alone is +22,581 / -2,312 across 100 files. That is more change than the project took in over its previous several releases combined. It may help to know what reviewing a PR here actually costs me. Earlier today I merged three small ones (#203, #214, #215, together about 800 lines). For those I merged all three into a scratch worktree to check they did not conflict, ran the full 3,971-test suite on the combined result, copied each contributor's new tests onto unmodified master to confirm they genuinely failed there, and ran the merged build side by side with production against my real transcript tree to confirm the behavior change was the claimed one. That is the bar, and those three took most of a working session. At that standard, 42,000 lines is months of full-time review. I cannot give your PRs that, and merging them at a lower standard is worse than not merging them, because I am the one who carries whatever goes in. The split did not make them independent. I appreciate that you broke this up rather than leaving one giant PR, and that was the right instinct. But the slices land on the same hot files: nine of them touch Separately, none of the seventeen have CI results, so I have no signal that any of them typecheck, lint, or pass the suite in this repo's environment. Where they land matters. The bulk of this work concentrates on input delivery, session identity, terminal replay, and file serving. Those are the parts of Codeman with the most hard-won invariants, and they are documented at length in I want to be fair to you here: I looked at #183's path handling and it is not careless. It realpaths both sides, rejects null bytes and absolute paths, and escapes diff rows before rendering. That is better than I expected. But "not obviously wrong on the paths I spot-checked" is a long way from the confidence I need before adding another surface that reads arbitrary repository files, and getting to that confidence is exactly the review time I do not have at this volume. What I'm asking for going forward. Please keep no more than three open PRs at a time. Beyond that it stops being a queue and starts being a backlog that makes the whole project harder to navigate, for me and for anyone else looking at the issue tracker. What works well here:
#214 and #215, which I merged today, are good models. Both were small, both named the exact bug, both shipped tests that genuinely failed before the fix, and #215 in particular walked through a revision it had gone through after review. That earned a fast, confident merge. This is not a door closing on you. You clearly use Codeman heavily on mobile and you are hitting real friction that I would like fixed. If you pick the single mobile bug that annoys you most, open one focused PR for it, and let it get merged before opening the next, I will review it properly and promptly. I would rather land ten good small PRs from you over the next few months than leave seventeen sitting open that neither of us can move. Thanks again for the effort, and sorry it took me this long to come back to you on it. |
Ark0N
commented
Aug 5, 2026
One thing I want to make unmistakable, because rereading my message above I think it lands more like a full stop than I meant it to. Please do bring these back. Not all seventeen, but the ones that fix something real. I closed them because I could not review forty-two thousand lines at once, not because I decided the underlying work was wrong. Several of these are pointed at genuine friction, and I would rather have those fixes than not. The only thing I am asking you to change is the rate. Three open at a time, maximum. Reopen the one you think matters most, rebased on current master, standing on its own, and once it lands, open the next. If you would rather I help pick, say the word and I will look through the list and tell you which two or three I would take first, so you are not guessing at what I will accept. Sixteen at once meant none of them got looked at. Three at a time means all of them eventually do. |
Lint111
commented
Aug 7, 2026
Thanks for taking the time to write that out — the detail on what review actually costs you was the part that landed, and the rate limit is fair. I'd like to take you up on the offer to pick. I have zero PRs open right now, so there's room for three whenever you want to point me at them. Two things from your review are already fixed on my branch, in case they change what's worth reviving:
The keyboard-focus blocker you reduced in Of the closed ones, my instinct is that these three are the most self-contained, but I'd rather have your read than mine:
If you'd rather name a different two or three, I'll work from your list instead. Either way I'll send them one at a time, rebased on current master, each with a test that fails without the change, and wait for one to land before opening the next. |
Ark0N
commented
Aug 7, 2026
Thanks for coming back with this, and for the two fixes you already made unprompted. Moving the shutdown check ahead of I went through the closed list with current master open next to it, and your three are the same three I would have picked. What I can add is an order, because master has moved a lot under some of them and barely at all under others. #194 first. It only touches #190 second. I checked its premise before agreeing: master is on #186 last, folded together with the keyboard-focus fix. This is the one I want most, and also the one where master has changed the most underneath. Since you branched, wheel and touch scrolling on mobile now forward to the CLI transcript for claude/codex sessions (part of the #205 work), so your tap classification has to coexist with a touch path that did not exist in July. The One timing note: I have #227 open right now, round 2 of the scrollback fix, and it touches #182 can take a slot after these three. The 403-before- One at a time with each landing before the next opens works for me, and so does having all three open at once, since only #190 and #186 share a file. Your call. And thank you for the work so far. The closure was about review capacity, never about the list; these three are pointed at real friction, and I want them in. |
A mid-terminal tap on a claude-mode session left document.activeElement on <body>, so the on-screen keyboard could not be raised and there was no way to type — the blocker reduced upstream in Ark0N#173. _classifyMobileTerminalTap returns 'content' for any non-prompt row, and _handleMobileTerminalTap blurred on every 'content' tap while touchstart's preventDefault had already cancelled the compatibility click that would otherwise focus xterm. Both routes to focus were closed on the same gesture. Blur now applies only to rows that are actually TUI-owned. The distinguishing signal is the affordance a CLI prints on or beside the row ("ctrl+r to expand", "tap to collapse", "esc to interrupt"), not the row's title text — a readback's title row carries no hint of its own, so the adjacent row is consulted too. Keying on titles would recognise only the exact strings a fixture happens to use and would let a real readback keep the keyboard open. Measured with a real touchstart/touchend gesture, iPhone-class viewport, claude-mode session, tapping mid-transcript: before document.activeElement = body after document.activeElement = xterm-helper-textarea Note: upstream master already passes this assertion, so the added test is a regression guard for this branch, not a test that fails on master. test/mobile/keyboard.test.ts: 40 tests, 5 failed | 35 passed — the same 5 pre-existing failures as master (stale layout/accessory-bar expectations and a CJK timeout), unchanged by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
selectSession() ends with scrollToLastNonEmptyLine(), which parks the viewport one row ABOVE the bottom for any session whose buffer is taller than the screen and ends in blank rows, so that is the normal state after a tab switch. Nothing pinned that a tap there still leaves the keyboard reachable. The blocker reduced in #173 came back through exactly that gap in #244: a tap classifier that treats "viewport is scrolled up" as a reason to blur, paired with touchstart preventDefault cancelling the compatibility click, closes both routes to focus on the same gesture and strands document.activeElement on <body> with no way to type. The prompt row is no exception. Measured on a 390x844 viewport, claude-mode session, dispatched touch gesture: master leaves focus on textarea.xterm-helper-textarea, PR #244's terminal-ui.js leaves it on body. Green here, red against that branch. The test also pins the half that IS correct: SGR coordinates are meaningless off-bottom, so the tap must send no mouse report. It has to be a dispatched gesture. Calling the touchend handler directly bypasses touchstart's preventDefault, which is half of what closes the focus path, so a direct call reports the right intent and still misses the bug. test/mobile/keyboard.test.ts: 4 failed | 32 passed (36), against 4 failed | 31 passed (35) without it. Same four pre-existing failures either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A mid-terminal tap on a claude-mode session left document.activeElement on <body>, so the on-screen keyboard could not be raised and there was no way to type — the blocker reduced upstream in Ark0N#173. _classifyMobileTerminalTap returns 'content' for any non-prompt row, and _handleMobileTerminalTap blurred on every 'content' tap while touchstart's preventDefault had already cancelled the compatibility click that would otherwise focus xterm. Both routes to focus were closed on the same gesture. Blur now applies only to rows that are actually TUI-owned. The distinguishing signal is the affordance a CLI prints on or beside the row ("ctrl+r to expand", "tap to collapse", "esc to interrupt"), not the row's title text — a readback's title row carries no hint of its own, so the adjacent row is consulted too. Keying on titles would recognise only the exact strings a fixture happens to use and would let a real readback keep the keyboard open. Measured with a real touchstart/touchend gesture, iPhone-class viewport, claude-mode session, tapping mid-transcript: before document.activeElement = body after document.activeElement = xterm-helper-textarea Note: upstream master already passes this assertion, so the added test is a regression guard for this branch, not a test that fails on master. test/mobile/keyboard.test.ts: 40 tests, 5 failed | 35 passed — the same 5 pre-existing failures as master (stale layout/accessory-bar expectations and a CJK timeout), unchanged by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d up Addresses the review on Ark0N#244. BLOCKING (item 1). selectSession() ends with scrollToLastNonEmptyLine(), which parks the viewport above the bottom for any session taller than the screen, so after a tab switch every tap classified as 'history' — touchstart ran preventDefault() + blur, and touchend's early return skipped focus. Both routes to focus closed on one gesture, the same mechanism as Ark0N#173. Suppressing the mouse REPORT while scrolled up is right and is kept; suppressing FOCUS is not. touchstart now only preventDefaults 'content' taps (a scrolled-up viewport sends nothing, so there is no compatibility click worth cancelling), and the 'history' branch focuses instead of blurring. Verified against the maintainer's own test, which was already on master and red: `keeps the terminal input focusable after a tab switch parks the viewport off-bottom` fails without this change and passes with it. Item 2: dropped both `terminal-action-pending` guards. The class exists nowhere in the repo, so both branches were permanently false and the comment promised coverage that did not exist. Item 3: removed the `Working` literals. Live claude 2.1.226 prints "Cooked for 2m 6s" with a different bullet and a randomised verb, so they were dead code. The status row is matched by its affordance ("esc to interrupt") instead, which is what makes it actionable. The affordance regex is also tightened to require a key or gesture name, so prose like "click here to open the file" no longer dismisses the keyboard. Item 4: removed _shouldForwardTouchScrollToApp and its test. It was never called, and wiring it as written would have restricted forwarding to claude only, dropping gemini from the path Ark0N#205 established — a behaviour change this PR has no reason to make. Smaller items: the touchstart classification is cached and reused for the touchend of the same gesture (keyed on exact coordinates, so a moved finger re-classifies), removing two of the three full-viewport scans per gesture; the duplicated touchLastX assignment is gone; and the no-touch bail-out returns null rather than claiming 'history'. test/mobile/keyboard.test.ts: 51 tests, 5 failed | 46 passed — the same 5 pre-existing failures as master, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Merge notes
Merged
origin/masterat57b6be1. Conflicts inCLAUDE.mdandstyles.cssretain upstream structure/theme work and document the combined mobile/webview invariants.Validation