Commit Graph
7 Commits
Author SHA1 Message Date
Brennan BensonandMerge Sim fab78c7669 fix(native-chat): show one live-turn indicator, and make Thinking mean reasoning (#19977)
* native-chat: render one indicator row for the live desktop turn

The turn-timing row and the spinner+activity line were two rows saying
"Working" at once. A settled turn keeps its own row; the live turn now has
only the spinner row, labelled provider activity -> Thinking -> Working for N
through the shared resolver. Reasoning is the turn's content, so it no longer
becomes the activity label, and "Thinking" now means the turn is reasoning
right now rather than that it has produced no output yet.

* mobile: give the live turn row a spinner and the shared indicator label

Mobile's per-turn row is already the only live indicator on the structured
lane, but it pulsed a bare word and never showed what the provider said it was
doing. It now renders a spinner beside the same resolved label desktop uses,
and reads reasoning from the journal instead of inferring it from missing
output. The bridge lane's four prompt/interrupt write seams move to one module
so the controller stays under its line cap.

* codex: mark streamed reasoning as reasoning too, and pin the provider markers

The settled reasoning item carried the marker but the streaming one did not,
so a live Codex turn - the only time the indicator is on screen - never read
as reasoning. Both paths now stamp it; a plan document keeps its own
presentation and must never read as reasoning.

* fix(native-chat): tighten live turn reasoning state

---------

Co-authored-by: Merge Sim <sim@local>
2026-09-11 00:19:57 -07:00
Brennan BensonandMerge Sim 4b4acf26a4 fix(mobile): enable patch-free iOS text selection in native chat (#19769)
* fix(mobile): make every native-chat text node selectable

Long-press selection worked on some chat text and not others. Markdown
paragraphs — the default block for agent prose — were the one block type
left out when headings, quotes, code, lists and table cells gained
`selectable`, and tool result output, diff rows, the unloadable-image
placeholder, permission/question bodies and the send-error banner never
had it at all.

Selection is now set on every content Text in the chat surface, on the
outermost block Text so nested inline spans inherit it. Labels inside a
Pressable (option rows, tool-line headers, buttons) are deliberately left
alone: selection there would swallow the tap they exist for.

Extracting MobileNativeChatEmptyState keeps the view under its max-lines
cap and matches desktop, where NativeChatEmptyState is already its own
component.

Tests render each surface and assert selection on the block that carries
the prose; both files were ablated against the unfixed source (4/10 and
3/5 red) so they pin the defect rather than the current behavior.

* fix(mobile): support native text range selection on iOS

* fix(mobile): remove persistent assistant message controls

* fix(mobile): scope patch-free text selection to chat

Use the stock react-native-uitextview dependency behind an iOS adapter and opt assistant Markdown into range selection only in native chat. Preserve the existing React Native Text behavior elsewhere and remove the persistent assistant controls.

---------

Co-authored-by: Merge Sim <sim@local>
2026-09-09 21:50:31 -07:00
Brennan BensonandMerge Sim b0c67eaf88 feat(mobile): port the restructured native-chat turn status and live tool progress (#18761)
* feat(mobile): port the restructured native-chat turn status and live tool progress

Mobile chat had a single static "Agent is working" row and no live tool
activity, while the desktop restructure (#17597, #18705) replaced that with a
per-turn status row and a running-tool label. This brings mobile to parity and
puts the derivation in one place instead of two.

Shared (new, pure, RN-safe — desktop uses them as i18n fallbacks, mobile
directly, matching the native-chat-empty-state pattern):
- `native-chat-turn-status.ts`: duration formatting, label selection, the
  turn-timing state machine, and the active/settled split.
- `native-chat-tool-activity.ts`: command-tool classification, the running-tool
  label descriptor, and running-call selection.

Desktop now consumes both; `NativeChatWorkingStatus`, `NativeChatToolRun` and
`use-native-chat-turn-status` keep their existing behavior and strings.

Mobile gains the "Thinking" / "Working for 12s" / "Worked for 3m 4s" row with a
caret that discloses the turn's tool activity, the pulsing "Running npm test"
row with terminal-vs-wrench glyphs, and desktop's rule that a completed turn's
tool run hides behind the turn caret. The bridge lane is untouched and keeps its
three-dot indicator. Headings, quotes, code, lists and table cells are now
selectable.

Files at their max-lines cap were split rather than bumped: the tool-run subtree,
the prompt card, the session-lane wiring, and the turn-disclosure state each move
to their own module.

* perf(mobile): stop the turn-status rows from re-rendering the whole transcript

A streaming turn re-renders the chat list many times a second. The disclosure
wiring handed every row a fresh status object and a fresh toggle closure on each
of those renders, so `MobileNativeChatMessage`'s memo never held and every
visible row re-rendered per tick — including settled turns that had not changed.

Memoize the status selection on the timing map, and keep one stable toggle
handler per turn (pruned when a turn leaves the transcript) attached only to the
settled rows that can actually disclose anything. Now only the live turn's row
changes identity while the agent works.

* fix(mobile): keep the turn clock running when the optimistic echo is replaced

An accepted send renders as `pending-N` until the transcript echo lands under
its real message id. That flips the active turn key mid-turn, and the timing
reducer treated the new key as a new turn — so a turn that had reached
"Working for 8s" visibly restarted at "Working for 0s".

The reducer now carries the start over when the previous key names a turn that
has since left the transcript, which is exactly the echo-replacement case. A
genuinely new turn (the previous key still in the transcript) and a turn that had
already settled both keep their own clock; both are pinned by tests. Desktop does
not pass the new key and is unaffected.

* fix(mobile): keep the Tools toggle working on settled turns

Hiding a settled turn's tool run behind the turn caret (desktop parity) also
made the composer's global Tools control a no-op on every completed turn: the
run it wanted to expand was not rendered at all. Let that toggle override the
hiding, so it still reveals every run at once the way it did before.

* fix(mobile): re-key the turn timing instead of only carrying its start

The previous fix carried the start forward only while the turn was still
working. When the transcript echo landed after the turn had already settled,
the new key inherited nothing, the settled timing was pruned with the old key,
and the turn's "Worked for N" row disappeared entirely.

Move the timing onto the new key instead, which covers both orderings: an
in-flight turn keeps counting from its original start (and later settles against
it), and an already-settled turn keeps its duration. Both orderings are pinned.

* test(mobile): pin the structured turn-status wiring at the view level

Emulator QA could not reach the structured lane (mobile's Create Tab -> Codex
falls back to a terminal tab when agentSession.createSupport says unsupported),
so the view's own lane wiring had no coverage — the one seam between the shared
turn-timing reducer and the rendered rows.

Assert what the view hands each row: the live user turn gets a status object and
the three-dot indicator is gone on the structured lane; the bridge lane keeps the
indicator and gets no status; a finished turn settles to a numeric duration with
a toggle; and an assistant row never carries a status row of its own.

* fix(mobile): isolate structured chat turn state

* fix(mobile): let the capability RPC actually store what a phone advertises

`runtime.clientCapabilities.update` records the advertised set by assigning
`authenticatedSocket.clientCapabilities`, but the socket handed to the dispatcher
defined that property with a getter only. In strict mode the assignment throws
`TypeError: Cannot set property clientCapabilities ... which has only a getter`,
so the RPC answered `runtime_error` and the set was never stored.

The consequence is not subtle: `supportsStructuredAgentSessions` requires the
capability, so `projectSessionTabAgentStatus` removed every `agent-session` tab
from a phone that had advertised it correctly. A paired phone saw ZERO tabs on a
worktree whose only tab was a structured Codex chat — structured native chat was
unreachable on mobile over this transport, not just missing its new turn UI.

Give the socket a setter that writes through to the channel, which already owns
the set for the connection's lifetime, so later requests on the same socket see
it. Found while trying to capture emulator screenshots of the turn-status port:
two full QA runs reported the new UI "missing" because the phone could only ever
get a bridge/PTY tab.

* fix(mobile): carry the turn key instead of caching a handler in a ref

Builds on the scope-isolation fix: that kept (and extended) a ref that is
written during render — once to memoize a per-turn handler, once to prune dead
turns, once to reset on a scope change. React Doctor's "Ref mutated during
render" is what CI's `check:react-doctor:changed` was failing on (x2), and on
mobile it is a real hazard rather than a style note: react-freeze discards
renders, and a discarded render would leave the cache mutated.

Pass the settled turn's key down the row instead and let it call one stable
handler with it. That preserves both properties the cache was bought for — per
scope isolation, and identity stability so a streaming transcript does not
defeat the row's memo — with no ref writes and no pruning to get wrong. The
scope-keyed expanded set and the 128-turn cap are untouched; their tests move to
the new contract and one now pins handler identity across a re-render.

Note for future changes here: `check:code-quality:changed` does NOT cover this.
CI additionally runs the standalone react-doctor CLI, which has rules the oxlint
plugin config does not enable.

* fix: ship native chat status translations

* test(native-chat): pin the shared copy against the English catalog

The shared constants are desktop's i18n fallback and mobile's actually-rendered
string. If one changes without the other, desktop keeps rendering en.json while
mobile renders the constant — and nothing fails, because a fallback is only used
when the key is missing. That silent divergence is the exact thing the shared
module exists to prevent, and it is now reachable precisely because these strings
are runtime-required rather than statically extracted.

Assert every key in both shared copy objects matches en.json byte for byte, plus
the interpolation placeholders the catalog interpolates on.

---------

Co-authored-by: Merge Sim <sim@local>
2026-09-04 23:31:39 -07:00
Neil 6c765d5ad3 refactor(mobile): remove native chat re-export facades (#13436) 2026-08-09 19:25:28 -07:00
Neil c56ff58dd7 test(mobile): centralize renderer setup (#13408) 2026-08-09 18:31:00 -07:00
Brennan Benson c511e51442 fix(mobile): label native-chat tool rows with a clean, expandable input summary (STA-3333) (#12498)
* fix(mobile): label tool rows with a clean summary, expand full input (STA-3333)

Mobile tool rows showed the raw input JSON (`{"file_path":…}`) as the row
label, and the expanded detail just repeated that same truncated string.

- `describeToolInput` labels a row with the target file path, else the
  primary argument (command/cmd/query/pattern/url/description), else the
  bounded JSON preview.
- Codex delivers tool arguments as a JSON string; normalize those into the
  object shape the helpers already understand, so labels, file links,
  run summaries and the expanded detail all work for Codex calls too.
- The expanded detail now renders the fully formatted input, capped at
  MAX_TOOL_RESULT_CHARS like desktop's tool detail (and like the result
  body), and a structured input makes the row expandable.

* fix(mobile): name search rows by their term and keep the filename in path labels (STA-3333)

Review follow-ups to the tool-row summary, all in the shared helper:

- A Grep/Glob row labelled itself with the directory it scanned and dropped
  the pattern entirely, because `toolFilePath` treats `path` as a file target.
  That path is a scan root, so it also rendered a tap-to-open link that asked
  the app to open a folder. `toolFilePath` now ignores the generic `path` key
  for search-shaped input, which lets the pattern win the label and drops the
  bogus link; an explicit `file_path` still wins.

- An overlong path was truncated from the head, cutting off the basename —
  the one part that tells two rows apart. Trim from the front instead, so
  the label reads `…/session/MobileNativeChatMessage.tsx`.

- The primary-argument chain used `??`, so a present-but-blank key selected
  itself and swallowed the keys ranked after it, dropping the label all the
  way back to raw JSON. Take the first key that actually yields a label.

Refs STA-3333.

* fix(mobile): don't offer an expander whose detail repeats the row (STA-3333)

An empty tool input formats back to the row label verbatim, so `{}` and `[]`
advertised an expander and then re-showed the label — the same repeat-the-JSON
problem this change set out to remove. Gate `isStructuredToolInput` on the
collection actually having contents; the lazy detail path is untouched.

Also pins the overlong-path test to the path itself: asserting only length<=80
plus a `…` passed just as well with path labelling deleted.

* fix(mobile): gate the tool detail panel on having detail (STA-3333)

The Tools toggle opens every row at once, bypassing the row's tap guard,
so a row with nothing to expand rendered its own label again underneath
itself — and the tap that would dismiss it is a no-op. Matches desktop.

* fix(mobile): keep a blank tool argument out of the run header (STA-3333)

Skipping a present-but-blank primary key let `briefToolArg` fall through
to the raw JSON preview, so a run header read `Bash {"command":""}` where
it used to read `Bash`. Also state the search-path trade-off honestly:
suppressing the link costs a file-scoped search its tap target.

* fix(mobile): only treat a blank primary key as a missing argument (STA-3333)

The previous guard tested key presence, so a populated but non-string
argument — a mixed argv like ['kill','-9',pid], or a structured query —
dropped out of the run header instead of falling back to the preview.

* test(mobile): pin the tool-row chevron to the detail panel (STA-3333)

The panel gate was covered but the chevron beside it was not: swapping
`showDetail` back to `expanded` on the icon alone left all 909 mobile
tests green, so the affordance lie this branch fixes could return
unnoticed — a down-chevron over no panel, on a row whose tap is guarded
off.

Asserts both icon counts on the fixture that test already renders. The
two halves now die for distinct reasons: the panel gate on the duplicate
label text, the chevron on the icon count.

* test(shared): pin the blank-search-key guard in the tool label (STA-3333)

Dropping `.trim()` from summarizePrimaryToolArg left all 32 tests green,
yet it leaks through isSearchToolInput: a whitespace-only `query` starts
counting as a search term, which suppresses `path`. One character takes
the row's label, its tap-to-open link and its run-header argument at
once, and puts the raw JSON label back — the bug this branch removes.

Asserts all three outputs on that shape. Kills only that mutant; the
isSearchToolInput mutant still dies on the existing search test.

* fix(native-chat): share tool input display semantics (STA-3333)

Build the tool row label, file target, detail eligibility and bounded detail from one normalized input model. Mobile no longer reparses JSON-string input across independent helpers or repeats an already-complete plain label, and desktop now uses the same clean row summary instead of retaining raw JSON.\n\nKeep full detail formatting lazy for collapsed rows and share the 4000-character detail cap across both renderers. Tests pin desktop adoption, mobile disclosure parity, one-pass JSON parsing and the shared bound.
2026-08-04 19:34:59 -07:00
NeilandJinjing 9500ca7a65 fix(mobile): show attached images in native (rich) chat (#10135)
* fix(mobile): show attached images in native (rich) chat

Attaching an image in the mobile native chat did nothing visible — it reused
the terminal attach flow, which pastes a bracketed host path into the hidden
terminal, so there was no composer preview and nothing in the transcript.

Give native chat the desktop model instead:
- pick + upload shows a removable thumbnail chip in the composer (no early paste)
- on submit, images ride along: Ctrl+U clear -> bracketed paste(s) -> settle ->
  text + Enter (idempotent on retry)
- the optimistic echo carries the local preview URIs and the message renderer
  draws image-ref blocks as real thumbnails when the URI is loadable, so the
  sent photo appears in the conversation immediately
- image-only echoes reconcile by ordinal against user turns after their tail
  (ignores agent replies / paginated history / the 'unknown' ack-loss path)

Terminal chat attach is unchanged (both flows consolidated behind
useMobileSessionImageAttachments). Adds unit coverage for pick+upload, the
ride-along byte order, chip render/remove, and echo reconciliation.

* test(mobile): interactive native-chat image proof (real hooks, click-driven)

Replace the hand-fed component render with an interactive harness that mounts
the real MobileNativeChatComposer/Message + useMobileNativeChatImageAttachments +
drafts under react-native-web and drives the actual flow via clicks. Only the two
OS boundaries are faked: the photo picker and the paired-host RPC socket.

Screenshots (mobile/docs/native-chat-image-attachment/) are produced by real
clicks, not props:
- attach -> real upload pipeline -> chip appears, nothing pasted yet
- send -> real ride-along emits Ctrl+U clear, bracketed image paste, text+Enter
  (shown in the live byte trace) and the sent bubble renders the photo thumbnail

* fix(mobile): scope native-chat image attachments by active tab

Images are now scoped to the tab that initiated the pick, so switching tabs
during upload cannot ride an image into another terminal. Chips stay with
their original tab, and only the active scope's images send with text.
Improved error handling with user-facing toast messages for disconnection
and send failures.

* test(mobile): add image attachment tab-scoping and error tests

Add comprehensive test coverage for tab-scoped attachment behavior,
error handling when transport fails or lease is gated, and edge cases
like attaching images during an in-flight send. Extract baseArgs and
update helpers to reduce boilerplate across test cases.

* fix(mobile): show attached images in native rich chat

Images attached in the mobile native (rich) chat now display as:
- Removable composer chips while composing
- Thumbnails in the sent user bubble after sending (desktop parity)

Implements proper image echo reconciliation by distinguishing
image-source marker turns from text echoes, so an image send isn't
cleared by an unrelated text echo. Adds scope isolation to prevent
chips and drafts from leaking between tabs, and detects tab switches
during the image-paste settle window to abort the send.

Fixes Android tap-target positioning for the image removal badge and
clears stale terminal input after failed pastes to avoid gluing
fragments onto the next message.

* rm stubs

---------

Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
2026-07-23 13:25:50 -07:00