Skip to content

feat(mobile): add manual prompt composer - #444

Merged
Ark0N merged 7 commits into
Ark0N:masterfrom
DodgyBadger:feat/mobile-prompt-composer
Sep 21, 2026
Merged

Ark0N merged 7 commits into
Ark0N:masterfrom
DodgyBadger:feat/mobile-prompt-composer

Conversation

@DodgyBadger

Copy link
Copy Markdown
Contributor

Summary

  • replace the agent keyboard bar's Paste action with a native multiline Compose dialog while retaining direct Paste for shell sessions
  • preserve exact per-session in-memory drafts, adopt locally buffered terminal input, and deliver prompts through guarded bracketed paste plus delayed Enter
  • support image attachments without prematurely writing paths to the PTY, including pending/concurrent upload handling
  • add mobile accessibility, focus, dynamic-viewport, and session-cleanup behavior

This is the manual-action first slice proposed in #359. The auto-open setting and terminal tap routing remain a separate follow-up.

Verification

  • npm test — 386 files passed, 1 skipped; 7,292 tests passed, 12 skipped
  • npm run typecheck
  • npm run lint
  • npm run check:frontend-syntax
  • Prettier and git diff --check
  • manual mobile verification at a 360px viewport
  • three independent peer-review passes: delivery/state, mobile UX/accessibility, and regression coverage

@Ark0N

Ark0N commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for following the two-slice split from #359 so closely: this is the manual Compose action with the multiline editor, Send, image attach, in-memory drafts and the escape hatch, with the setting and the tap routing left for the follow-up, which is exactly what I asked for. All four of the load-bearing points from that thread are handled, and putting the regression coverage in the CI-visible test/ root instead of test/mobile/** is the right call. Every check passes here too: typecheck, lint, format, frontend syntax, and the full gate at 386 files and 7,292 tests.

One blocker, then some small things.

Blocker: Send can only ever fire once per session.

src/web/public/keyboard-accessory.js:1176 gates delivery on app.terminal.modes.bracketedPasteMode. That flag is xterm's mirror of DECSET 2004 in the browser, and it is false for every session after any tab switch or page reload, permanently. Three things combine:

  1. _resetTerminalForReplay() (app.js:5748) calls terminal.reset(), which re-clones xterm's decPrivateModes from defaults, where bracketed paste is off. Every session select goes through that reset.
  2. The replay cannot put it back: GET /api/sessions/:id/terminal returns a tmux capture-pane, which carries no DEC private modes. I checked a live claude session: ?full=1 and ?tail= both contain zero ESC [ ? 2004 h.
  3. Nothing re-emits it. tmux does not forward the pane's DECSET 2004 to its client (I ran printf '\033[?2004h' in a live pane with a browser watching and the byte never arrived), it emits one at attach and not again on resize, and 120 seconds of the live SSE stream across about 20 claude sessions contained zero.

So the only ?2004h a browser ever sees is the one tmux writes at attach, which reaches it only if the page was already open when the session was created. Driving your own composePrompt() in a real browser against a real server:

Send on a session created with the page open:
  paste called with "first line\nsecond line", overlay closes, no toast
Send after one tab switch away and back:
  nothing pasted, overlay stays open,
  toast "Prompt composer is waiting for the agent input to become ready"

The tests miss it because the JSDOM harness mocks modes: { bracketedPasteMode: true } (test/mobile-prompt-composer.test.ts:38), and the one case that sets it false treats the refusal as correct.

The guard is not the problem: with the mode off, terminal.paste('a\nb') really does emit "a\rb", which is the mangling we were trying to avoid. The problem is where the truth comes from. Two ways out, either is fine by me:

  • Record the last bracketed-paste DECSET the session sees on the PTY stream, the same shape _recordStrippedMouseMode() and cliMouseTracking already use in src/session.ts, and re-assert it on the client after the replay. The server does see tmux's attach-time ?2004h even though the capture cannot carry it. Keep it a zero-width append, since nothing that can delete a line may run over a full capture.
  • Or stop consulting the mirror and emit the markers yourself on the durable path: _sendInputAsync(sessionId, '\x1b[200~' + text + '\x1b[201~'), then the delayed \r. Byte-identical to what xterm would have produced, comfortably inside the 64 KiB input limit, and it is what the CLI actually reads.

If neither fits in this PR, collapsing to one line and going through the existing sendCommand() path when the mode is unknown would at least leave a composer that sends.

Worth fixing in the same round

  • keyboard-accessory.js:1138 and :1328: opening Compose clears the local-echo overlay and backspaces the flushed prefix out of the CLI composer, which is right while the modal is up. But "Use terminal keyboard" then focuses xterm and the user's prompt is gone from the screen, living only in the in-memory draft. Your own test shows this (typing ; then continue into the terminal and expecting keep this; then continue back). Cancel, backdrop and Escape are the same. Re-injecting the draft into the terminal composer on any close other than Send would fix it, or mark the Compose key while a draft is parked.
  • keyboard-accessory.js:1205: after any arrow key from the bar, sendNavKey() puts the session in app._echoPassthroughSessions, where typed text goes straight to the CLI composer untracked. _takePendingLocalEcho() neither adopts nor erases it, so composing after an up-arrow recall sends the concatenation. The part I would definitely fix is the leak: passthrough is cleared only by \r or \x03 arriving through xterm's onData, and the composer's Enter goes out through _sendInputAsync, so after a composed Send the session stays in passthrough and local echo stays off for it. One app._echoPassthroughSessions?.delete(sessionId) in send() covers it.
  • keyboard-accessory.js:1149: '\x7f'.repeat(flushedCount) uses a UTF-16 code-unit count, so one emoji in the flushed prefix sends two backspaces for one composer character. Array.from(flushedText).length matches what the CLI deletes.

Small things, I can take these at merge time

  • styles.css:13524: the padding shorthand's bottom value never applies. The fold rule at styles.css:18273 is a later padding-bottom longhand at the same specificity, so the composer overlay measures padding-bottom: 0px, not calc(12px + safe-area). Nothing looks wrong because the dialog's max-height reserves that space anyway, but it is the exact cascade trap the folding devices section of CLAUDE.md is about.
  • docs/wiki/Mobile-Guide.md:63 still says the agent bar has "a clipboard key". A short CLAUDE.md entry would help too: delivery via terminal.paste() plus a separate delayed Enter and why, and drafts being in memory only and why.
  • Send is a silent no-op on an empty draft or a missing terminal, and refreshForActiveSession() reads app without the typeof app === 'undefined' guard the rest of the file uses (not reachable in practice).

Push the bracketed-paste fix and the composer-emptying one and I will merge. The rest I will tidy on the way in.

@Ark0N

Ark0N commented Sep 18, 2026

Copy link
Copy Markdown
Owner

One housekeeping note: your CI had never actually run. As a first-time contributor the workflow sat at action_required waiting for a maintainer to approve it, which is why this PR showed no checks at all rather than failing ones. I approved it and both jobs pass on 92889682, so the blocker above is a runtime defect rather than anything CI was going to catch for you.

@Ark0N

Ark0N commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for turning the bracketed-paste blocker around so quickly. The Compose key, the multiline editor with autocorrect, the per-session in-memory drafts with the marker on the key, the image attach that never touches the PTY, and the escape hatch are all exactly the first slice from #359. Every check passes here: typecheck, lint, format, frontend syntax, public assets, and the full gate at 386 files and 7,292 tests.

Two things to fix, then I will merge.

1. src/web/public/keyboard-accessory.js:1196: drop { useMux: true } from the bracketed-paste write.

The new delivery call is right, but the options object changes what happens when the terminal WebSocket is not open for that session, which on a phone is routine (reconnect backoff, a tab the OS suspended, the window between page load and socket attach). On that fallback the POST goes through session.writeViaMux() into TmuxManager.sendInput() (src/tmux-manager.ts:3187), which computes:

const hasCarriageReturn = input.includes('\r');
const textPart = input.replace(/\r/g, '').replace(/\n/g, '').trimEnd();

Your payload maps newlines to CR one line earlier, so both fire:

in:       "\x1b[200~first line\rsecond line\x1b[201~"
textPart: "\x1b[200~first linesecond line\x1b[201~"

tmux sends that, waits 50ms, and sends its own Enter. So the prompt arrives with the lines welded together, gets submitted, and your delayed \r fires a second Enter 120ms later. It also trips on a single-line prompt that ends with a blank line. That is the CLAUDE.md gotcha verbatim ("Embedded newlines are stripped, not rejected") and it breaks the acceptance criterion in #359 that Send preserves intended line breaks.

Without the options object the fallback takes session.write() and is byte-identical to the WebSocket path, which is what the snippet in my earlier comment did. Keep { useMux: true } on the trailing \r: that one is correct. test/mobile-prompt-composer.test.ts:220 currently asserts the option as part of the expected call, so that assertion needs to move with it, and a case covering the tmux transform would stop this coming back.

2. src/web/public/keyboard-accessory.js:1276: release echo passthrough in send().

This is the leak from my first round and it is still open. An arrow key from the bar adds the session to app._echoPassthroughSessions (:1116), and that set is cleared in exactly one place: terminal-ui.js:1182, on a \r or \x03 arriving through xterm's onData. The composer's Enter goes out through _sendInputAsync, so after a composed Send the flag survives and zero-lag local echo stays off for that session until the user presses Enter or Ctrl+C on the terminal keyboard. I confirmed it by driving your module in JSDOM. One line next to the draft cleanup covers it:

app._echoPassthroughSessions?.delete(sessionId);

Also worth taking while you are in there

keyboard-accessory.js:1165: '\x7f'.repeat(flushedCount) is a UTF-16 code-unit count (getFlushed().count comes from echoText.length), so one emoji in the flushed prefix sends two backspaces for one composer character and eats its neighbour. Array.from(flushedText).length is what the CLI deletes. clearTerminalInput() in terminal-ui.js:3951 has the same bug, so move both or leave a note saying why only one moved.

I will take these at merge time

  • styles.css:13532: the padding shorthand's bottom value never lands, because the fold rule at styles.css:18281 is a later padding-bottom longhand at the same specificity. Nothing looks wrong (the dialog's max-height reserves the space anyway), and test/foldable-layout.test.ts cannot see it because your overlay inherits the centring declarations rather than declaring them.
  • docs/wiki/Mobile-Guide.md:63 still says the agent bar has "a clipboard key", plus a short CLAUDE.md entry for the composer.
  • Send is a silent no-op on an unreachable branch, and refreshForActiveSession() reads app without the typeof guard the rest of the file uses.

One note on the mobile suite: test/mobile/keyboard.test.ts has five failures here, all tap-intent and layout-metric assertions, and they reproduce identically on the merge base, so they are nothing to do with your change. The accessory-bar action list you updated passes.

Push those two and it goes in.

@DodgyBadger

Copy link
Copy Markdown
Contributor Author

Apologies, my agent got a bit ahead of me and pushed before we were done. I'll comment again when this is ready for your final merge review.

@Ark0N

Ark0N commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for turning both items from the last round around quickly: the bracketed-paste write no longer carries { useMux: true }, and send() now releases the echo passthrough. The Compose key, the multiline editor with autocorrect, the per-session in-memory drafts with the marker on the key, the image attach that never touches the PTY and the escape hatch are exactly the first slice from #359, and putting the coverage in test/mobile-prompt-composer.test.ts rather than test/mobile/** means it actually runs in the gate. Every check passes here: typecheck, lint, format, frontend syntax, public assets, and the full gate at 405 files and 7,746 tests.

I am holding off on merging, and your own note about the agent pushing early is why.

1. The branch that is up now is not the branch I reviewed.

Your three commits (92889682, 2f88dbd7, fee6098a) touch only keyboard-accessory.js, image-input.js, styles.css, one line of app.js and the tests. Everything else in the diff came in through the merge commit fcb22429: about 180 new lines in app.js's reliable-delivery layer plus src/remote-wake.ts, src/session.ts, src/types/session.ts, src/web/schemas.ts, src/web/routes/session-routes.ts and src/web/routes/ws-routes.ts, roughly 900 lines in total. git log --oneline shows none of it, so nobody reading the commit list would know it is there.

A lot of it is defensible: forceHttp so the bracketed frame cannot take the WebSocket path, a durably queued delayed Enter instead of a bare setTimeout, and per-chunk delivery metadata so a composed prompt survives a wake-on-LAN buffer. But it is a second change inside a PR I asked to keep to one, and it reaches well outside the composer. Tell me which way you want to go:

(a) take the backend edits back out, land the composer on the one-line { useMux: true } removal I asked for, and open the delivery rework as its own PR, or
(b) keep it here and fix points 2 to 5 below.

Either way, please rebase onto master rather than merging master in again, so the next read is per commit.

2. src/web/routes/session-routes.ts:1770 and :1875: the input route answers 503 where it documented 200.

A chunk that will not fit the wake buffer now returns 503 OPERATION_FAILED instead of 200 {"buffered":true,"dropped":true}, and a tagged (clientId + seq) non-wait request whose write was refused now returns 503 instead of 200. The same block makes the tagged non-wait path await writeViaMux, which used to be fire-and-forget.

I think you are right on the merits: ws-routes.ts:196 already withholds the ACK on a failed write, so HTTP was the asymmetric one, and a 200 there was a durable client's ACK for input that never landed. But it contradicts docs/architecture-invariants.md:83 ("the non-wait path keeps its fire-and-forget shape byte for byte"), docs/api-reference.md:329-331, and the route's own comment at :1869 ("NOT an error response, deliberately"), which now sits directly above the block that contradicts it. docs/api-reference.md:77 also pins OPERATION_FAILED to 422, so 503 is a new undocumented override. If this stays, all three need updating and the stale comment needs to go. Worth noting it reaches agent callers too: the documented curl example at docs/api-reference.md:317 and the skill's own trust-dialog helper both tag their input.

3. src/remote-wake.ts:79: the wake buffer cap and its eviction policy both changed.

4 KiB becomes 256 KiB per sleeping session, and _enqueue now rejects the incoming chunk instead of dropping the oldest. The reasoning is good (a composed prompt can be 64 KiB, and evicting an already-ACKed chunk loses input the client believes it delivered), and pairing the reject with forgetInputSeq is the right call. It contradicts CLAUDE.md:220 and docs/remote-sessions.md:443 and :469, none of which moved. One dead corner too: the single-chunk-too-large branch at :796 is now unreachable, because the route already caps input at MAX_INPUT_LENGTH code units, whose worst case in UTF-8 is under the new cap.

4. Docs and dead code that go with 2 and 3.

docs/reliable-input-delivery.md is the design doc for the layer you reworked and none of it moved: the record shape at line 27, the first-attempt re-queue rule at line 56, and "a deduped duplicate returns 200 without writing" are all now wrong. appendBoundedPending (src/remote-wake.ts:132) has no callers left, and three comments still describe it as live (:121, :797, :837). And src/web/public/app.js:3349 quietly drops keepalive from the input POST, which is unrelated to the composer: recovery still works through the durable queue, but an in-flight POST no longer survives a page unload. Say what broke with it, or put it back.

5. src/web/public/keyboard-accessory.js:1168: the code-unit backspace count is still open.

'\x7f'.repeat(flushedCount) counts UTF-16 code units, so one emoji in the flushed prefix sends two backspaces for one composer character and eats its neighbour. Array.from(flushedText).length is what the CLI deletes. clearTerminalInput() in terminal-ui.js:3951 has the same bug, so move both or leave a note saying why only one moved.

Smaller things, not blocking

  • app.js:3264: the _postDraining early return moved to the top of _drainSession, so an in-flight POST is now a barrier for the WebSocket path too. That is the right idea, but fetch has no timeout and forceHttp means a POST is routinely in flight while the socket is open, so a black-holed request wedges all input for that session. An AbortSignal.timeout() around it would close that.
  • app.js:3488: for the same reason, the 2 s sweep can now read a slow composer POST as a half-open socket and close a perfectly healthy WebSocket.
  • app.js:3232: _markDeliveryAttempt adds a second synchronous localStorage write per keystroke on top of the one _reliableSend already does.
  • keyboard-accessory.js:1207: worth a comment saying the unconditional bracketed frame is deliberate, and why the client-side bracketedPasteMode mirror cannot be trusted.
  • The bar's own arrow keys recall a history entry into the CLI's composer, and Compose then appends to it, so Send submits the two joined. feat(mobile): add an autocorrect-aware prompt composer #359 puts that out of scope, so I am fine leaving it, but it is closer than "text already in the remote composer" suggests.

I will take the styles.css padding shorthand, the missing typeof app guard in refreshForActiveSession(), docs/wiki/Mobile-Guide.md:63 and a CLAUDE.md entry at merge time.

Tell me which way you want to go on point 1 and I will review from there. The composer itself is good work and I want it in.

@DodgyBadger
DodgyBadger force-pushed the feat/mobile-prompt-composer branch from fee6098 to 884713c Compare September 20, 2026 08:08
@DodgyBadger

Copy link
Copy Markdown
Contributor Author

Thank you for the detailed feedback and guidance, and apologies for letting the previous revision grow beyond the scope of this PR. There has been a bit of a learning curve for me on this repository, and I over-corrected by trying to solve broader delivery guarantees here.

I took option (a), rebuilt the branch directly on current master, and force-updated it with a linear history. The reliable-delivery, remote-wake, route, schema, and other backend changes have all been removed from this PR.

The branch now contains:

  • 773b4054 — initial mobile prompt composer
  • e2146914 — replay-safe explicit bracketed-paste delivery and visible saved drafts
  • c2eaba99 — release echo passthrough after a composed Send
  • 0761de3d — use the normal raw fallback for the bracketed frame while keeping the delayed Enter muxed
  • a220c28a — count Unicode code points correctly when clearing flushed input, in both affected locations
  • 884713cc — retain oversized prompts as drafts instead of attempting an over-limit frame

I left the CSS cascade, documentation, and defensive app guard items untouched as you offered to handle those at merge time.

Three focused peer reviews came back clean. Typecheck, lint, formatting, frontend syntax, public-assets checks, and the full CI gate pass locally: 405 test files and 7,729 tests.

Thanks again for helping me get this back to a focused and reviewable change.

@Ark0N

Ark0N commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Thanks for turning this around so cleanly, and for taking option (a): this is now the manual Compose slice on its own, with the multiline editor, Send, image attach, per-session in-memory drafts and the escape hatch, and nothing outside src/web/public/ plus tests.

I checked the shipped code rather than the commit list, and all five points from the last round are genuinely fixed. Two of them I verified against live processes rather than by reading:

  • The unconditional bracketed frame is safe for CLIs that never enable DECSET 2004. Writing ESC[200~hello CR world ESC[201~ into a real tmux pane's attach client, tmux stripped the markers for a pane running cat -v and forwarded them intact to a process that had emitted ESC[?2004h. So tmux is doing the gating the browser used to attempt with its unreliable mirror, and your frame degrades correctly everywhere.
  • The code-point backspace count is right. On a live Claude Code 2.1.278 pane, typing a + emoji + b and sending backspaces went a+emoji+b, then a+emoji, then a, then empty: exactly three backspaces for three composer characters. The old code-unit count would have sent four and eaten the neighbouring character.

I also confirmed the payload is byte-identical to what terminal.paste() would have produced (prepareTextForTerminal is replace(/\r?\n/g,'\r'), bracketTextForPaste is the same concatenation), and that the 65,536 guard sits exactly on the MAX_INPUT_LENGTH boundary in ws-routes.ts, where an oversized frame is dropped without an ACK and would wedge the durable queue.

Nothing blocking. Two small things I would like on top of what I already said I would take:

  1. src/web/public/keyboard-accessory.js:677: Compose as a text label pushes the simple agent bar past the viewport at 360px. Measured against the real stylesheets: scrollWidth 360 on master, 378 here, against a 360px client width, so the Dismiss key is clipped until the bar is swiped. 393px and 430px still fit. An icon with aria-label="Compose prompt", or a shorter label, puts it back.
  2. test/foldable-layout.test.ts: when I fix the .prompt-composer-overlay padding I will also add { name: '.paste-overlay.prompt-composer-overlay', classes: ['paste-overlay','prompt-composer-overlay'] } to that file's ELEMENTS list. The current guard misses the composer twice over (the derived overlay list keys on rules that declare position: fixed; inset: 0 themselves, and the cascade simulation runs ['paste-overlay'] alone), which is why the overlay computes padding-bottom: 0px with a green suite.

Follow-up material, not for this PR: the dialog's new copy (Compose prompt, Enter adds a new line, Use terminal keyboard, Write your prompt…) has no i18n.js entries while Cancel and Send already do, so zh-CN gets a half-translated dialog; keyboard-accessory.js:1286 treats a whitespace-only draft as content and submits blank lines, which textarea.value.trim() for the emptiness test would fix; and test/mobile-prompt-composer.test.ts:14 resolves its source paths from the process cwd rather than import.meta.dirname like its neighbours.

Everything passes here: typecheck, lint, format, frontend syntax, public assets, the full gate at 405 files and 7,729 tests, and the xterm-zerolag-input package suite. Send the Compose key width change and I will merge, taking the styles.css padding, the typeof app guard at keyboard-accessory.js:826, docs/wiki/Mobile-Guide.md:63 and a CLAUDE.md entry on the way in. Good work, and thank you for the patience through three rounds.

@DodgyBadger

Copy link
Copy Markdown
Contributor Author

Done in ac574d6c: the Compose action now uses a compact accessible icon in both agent layouts, keeping the 360px bar within its previous width budget. The focused composer suite passes 16/16, and the full gate passes at 405 files and 7,730 tests.

As discussed, I have left the shared/global surfaces for your merge-time pass: the composer overlay padding and corresponding foldable-layout ELEMENTS coverage, the defensive typeof app guard, the Mobile Guide update, and the CLAUDE.md entry. Thank you again.

@Ark0N
Ark0N merged commit aa13af1 into Ark0N:master Sep 21, 2026
2 checks passed
@Ark0N

Ark0N commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Merged into master for 1.32.0, with the merge-time list applied on the way in (d8e8528): the composer keeps its bottom gutter on folding devices and the foldable-layout guard now covers it, the typeof app guard, the Mobile Guide and a CLAUDE.md entry, plus the whitespace-only draft, the derived length guard and the zh-CN strings. Thank you for taking the scope back down to one slice and for verifying the delivery path against a live pane. Good first contribution.

@github-actions github-actions Bot mentioned this pull request Sep 21, 2026
opticon454 pushed a commit to opticon454/Codeman that referenced this pull request Sep 21, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
opticon454 pushed a commit to opticon454/Codeman that referenced this pull request Sep 21, 2026
- styles.css: restate the composer overlay's own bottom gutter after the fold rules
  (the generic .paste-overlay longhand erased it: 0px flat, hinge strip replacing it
  folded) and subtract the fold strip from the dialog's max-height
- test/foldable-layout.test.ts: simulate the cascade for
  .paste-overlay.prompt-composer-overlay (fails without the CSS fix); pin the palette
  anchor by name instead of ELEMENTS.at(-1)
- keyboard-accessory.js: guard the app global in refreshForActiveSession() like the
  rest of the file
- keyboard-accessory.js: a whitespace-only draft is empty (Send no longer submits
  blank lines); the text still goes out untrimmed
- keyboard-accessory.js: derive _composerMaxLength and the frame refusal from one
  64 KiB frame limit minus both bracketed-paste markers so they cannot drift
- keyboard-accessory.js: translate the textarea placeholder and label at build time,
  since the DOM translator skips <textarea> subtrees
- i18n.js: zh-CN entries for the composer dialog copy
- docs/wiki/Mobile-Guide.md: describe the Compose key instead of a clipboard key
- CLAUDE.md: a "Mobile prompt composer" paragraph after the accessory bar one
- test/mobile-prompt-composer.test.ts: pin the whitespace rule and the derived budget

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit f6725ba52da17b0bdbee8be3b5011e7cae514f69)
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