Skip to content

fix(mobile): require a prompt or a file before starting on a remote computer - #8481

Merged
iscekic merged 4 commits into
mainfrom
drain/remote-start-requires-prompt
Oct 22, 2026
Merged

iscekic merged 4 commits into
mainfrom
drain/remote-start-requires-prompt

Conversation

@iscekic

@iscekic iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Change

The new-session form let the remote-computer target start with an empty composer. The form then built no share payload, so the spawn carried nothing, and Start stayed busy until the client SDK timeout with no error and no way forward.

  • resolveRemoteStartMissingContent (apps/mobile/src/lib/new-session-submit.ts) is the new pure gate: content is missing only when there is no prompt and no attachment, the exact condition under which buildComposerSharePayload returns null. The remote branch of resolveStartDisabled (new-session-screen-body.tsx) now blocks Start on it. An attachment alone stays a valid start: the destination keeps an attachment-only delivery in its own composer for the user to send.
  • resolveNewSessionStartBlockedReason now names the empty composer instead of returning null:
    • remote computer: enter-prompt-or-file → "Add a prompt or a file to start."
    • Cloud Agent: enter-prompt → "Add a prompt to start." (Cloud Agent does not accept an attachment without text, so it never claims one helps). It is emitted only when the prompt is the remaining blocker after the repository is chosen.
  • Both copy keys are added to the English catalog and all 86 locale catalogs.

No behavior change for the Cloud Agent gate, the clone entries, the share gate, or the spawn/retry classification.

End-to-end evidence (Android emulator, local stack, fake account)

Ran the real app against the local dev stack on this VM (Next.js on 3000, session-ingest worker on 8800, Metro on 8081) with a fake account, and registered a fake connected CLI through the real local session-ingest worker (/api/user/cli heartbeat), so activeSessions.listInstances returned one remote row. uiautomator dump on the new-session form:

  1. Run on = the remote row, composer empty: the form shows "Add a prompt or a file to start." above Start, and the Start button node is enabled="false".
  2. Tapping Start in that state dispatches nothing: the label stays "Start session", no busy state appears, the form is unchanged.
  3. Typing a prompt: the reason line disappears and Start becomes enabled="true".
  4. Clearing the prompt: the reason line returns and Start returns to enabled="false".
  5. With a prompt and Start tapped: the button shows "Starting…", and after the SDK timeout it returns to "Start session" with the clickable button, the remote target and the prompt still on the form, so the user can retry. The spawn's retryable outcome surfaces the existing toast ("Couldn't reach the instance — it may have disconnected.").

The fake CLI deliberately answers no command, so the spawn settles through the client timeout; the UI behavior above is what was observed. No Cloud Agent or real-CLI session was created by this run.

Checks on this VM:

  • pnpm exec vitest run src/lib/new-session-submit.test.ts: 53 passed (new cases cover the predicate with prompt, file, both and neither, plus both reason branches).
  • pnpm exec tsc --noEmit in apps/mobile: clean.
  • oxlint over the three changed source files: 0 warnings, 0 errors.
  • pnpm --filter kilo-app check:i18n (87 catalogs match en.json) and check:plurals: pass.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Content rule: When the consumer picks a remote computer as the run target, when should Start be available?
    Answer: Prompt or attachment
    Confidence: 0.60 · Precedent: Trust the code over documents: session 269375d6e466, "only trust code". The brief asks to match what the remote spawn accepts, and the code shows it accepts attachments-only.
  2. Question: Reason scope: Where should the new disabled-Start reason appear when the prompt is the blocker (the reason helper currently returns null for every remote target)?
    Answer: Both targets
    Confidence: 0.70 · Precedent: Fix the class of the issue on every affected surface: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class".
  3. Question: Reason copy: Wording for the reason line under the disabled Start button?
    Answer: Add a prompt or a file to start.
    Confidence: 0.50 · Precedent: Follows the content-rule answer. Localize the string: session dcb3f9170164, "the mobile app always must respect i18n".
  4. Question: Cloud copy: The copy names a file as an alternative, but only the remote target accepts attachments without prompt text (Cloud Agent's create silently does nothing without text). How should the Cloud Agent reason read?
    Answer: Per-target wording
    Confidence: 0.60 · Precedent: UX and design quality: never show a false or redundant reason, keep similar elements consistent (a247f3c19b1e: "remove the redundant 'context' copy"); Git and PRs: put each separate concern in its own PR (debabfb38c82: "in a separate PR"). Localize both keys (dcb3f9170164: "the mobile app always must respect i18n").

…omputer

The remote-computer target issued a spawn from an empty composer: no share
payload was built, so the new session had nothing to run and Start stayed busy
until the SDK timeout, with no error and no way forward.

- Gate the remote Start on the same condition that builds a payload: prompt
  text or at least one attachment. The destination keeps an attachment-only
  delivery in its own composer for the user to send.
- Name the empty composer in the blocked-reason line: "Add a prompt or a file
  to start." for the remote computer and "Add a prompt to start." for Cloud
  Agent, which does not accept an attachment without text.
- Add both copy keys to the English catalog and every locale catalog.
- Unit tests for the new predicate and for both reason branches.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Oct 21, 2026
@kilo-code-bot

kilo-code-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Commit 5114f3b3c closes the previously reported stale-selection gap: selectedRepositoryResolved now reaches both reason-input call sites from the real selectedRepository !== null value, and the selected-repository branch returns null instead of wrongly blaming the empty composer, with regression tests for both stale + empty and stale + filled prompts.

Assumptions: [INFERENCE] Tests were not executed in this read-only review; the new cases were verified against the implementation. A stale picker key still leaves Start disabled with no reason line, but that matches base-branch behavior (the enter-prompt copy is new in this PR and is now correctly suppressed), and the repository picker remains actionable. No memory-leak risk: the follow-up adds one boolean input and pure branching, with no new listeners or retained state.

Files Reviewed (3 files)
  • apps/mobile/src/lib/new-session-submit.ts - 0 issues (stale-key branch returns null before the enter-prompt check)
  • apps/mobile/src/lib/new-session-submit.test.ts - 0 issues (new stale-selection cases cover both prompt states and both helpers default to resolved)
  • apps/mobile/src/components/agents/new-session-screen-body.tsx - 0 issues (both reason call sites pass selectedRepository !== null, matching the Start gate)
Previous Review Summaries (2 snapshots, latest commit b961d07)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit b961d07)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The new remote-start content gate mirrors buildComposerSharePayload and both reason branches are wired correctly; the one remaining issue is the Cloud Agent enter-prompt branch, which assumes the selected repository still resolves, so a stale picker key shows "Add a prompt to start" and then leaves a disabled Start with no explanation.

Assumptions: [INFERENCE] Attachment-only remote start was not exercised on a device during this review (the PR's E2E run covers prompt-only starts), but hasPrompt and buildComposerSharePayload both trim text, so whitespace-only prompts stay blocked and the gate matches the payload builder. No memory-leak risk found: the change adds only pure predicates, one render-time check, and locale strings, with no new listeners or retained state.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/new-session-submit.ts 239 Carried forward, inline comment still active: the enter-prompt re-evaluation hardcodes selectedRepositoryResolved: true, while the screen's gate derives it from selectedRepository !== null (new-session-screen-body.tsx:994). A picker key left selected after a repository refresh or provider change therefore reports "Add a prompt to start"; typing a prompt then clears the line while Start stays disabled on staleSelection. Pass the real resolution state (or bail out when the key does not resolve) before emitting enter-prompt.
Files Reviewed (90 files)
  • apps/mobile/src/lib/new-session-submit.ts - 1 issue (carried forward)
  • apps/mobile/src/lib/new-session-submit.test.ts - 0 issues (new predicate cases and both reason branches match the implementation)
  • apps/mobile/src/components/agents/new-session-screen-body.tsx - 0 issues (content gate, hasAttachments wiring, and reason copy mapping aligned with the real gate)
  • apps/mobile/src/i18n/locales/en.json - 0 issues (both keys present under agentChat.newSession)
  • apps/mobile/src/i18n/locales/*.json (86 catalogs) - 0 issues (both keys present exactly once in every catalog, translated copy alongside the neighboring keys)

Previous review

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The new remote-start content gate and both blocked-reason branches match buildComposerSharePayload and the existing Cloud Agent create guard; the single issue is that the new Cloud Agent enter-prompt branch assumes the selected repository still resolves, so a stale picker key gets a wrong hint and then a reason-less disabled Start.

Assumptions: [INFERENCE] Attachment-only remote start could not be verified on a device (the PR's E2E evidence covers prompt-only starts); the payload shape it relies on already exists in picture-entry.ts, so the gate is consistent with the builder. No memory-leak risk: the change adds only pure predicates, one render-time check, and locale strings.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/new-session-submit.ts 239 The enter-prompt re-evaluation hardcodes selectedRepositoryResolved: true, so a stale picker key (repo row removed by a refresh/provider change) shows "Add a prompt to start"; typing a prompt then clears the line while Start stays disabled on staleSelection.
Files Reviewed (90 files)
  • apps/mobile/src/lib/new-session-submit.ts - 1 issue
  • apps/mobile/src/lib/new-session-submit.test.ts - 0 issues (new predicate and reason cases match the implementation)
  • apps/mobile/src/components/agents/new-session-screen-body.tsx - 0 issues (gate wiring, reason mapping and hasPrompt/attachment inputs aligned)
  • apps/mobile/src/i18n/locales/en.json - 0 issues (both keys present under agentChat.newSession)
  • apps/mobile/src/i18n/locales/*.json (86 catalogs) - 0 issues (key parity with en.json, consistent placement)

Reviewed by deepseek-flash-latest · Input: 64.1K · Output: 6.6K · Cached: 979.6K

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: End-to-end captures from the Android emulator against the local dev stack, with a fake account and a fake connected CLI registered through the local session-ingest worker.

Observed on the new-session form with the remote computer selected:

  • Empty composer: the reason line "Add a prompt or a file to start." is shown above Start, and the Start button node reports enabled=false in the accessibility dump. Tapping it dispatches nothing (the label stays "Start session", no busy state).
  • After typing a prompt: the reason line is gone and the Start button node reports enabled=true.
  • Clearing the prompt brings the reason line back and disables Start again.
  • Starting with a prompt shows the busy label, then returns to "Start session" with the form, the remote target and the prompt intact, so the user can retry.

The screenshots show the two composer states; the button state comes from the accessibility dump recorded beside each capture.

New-session form with the remote computer selected and an empty composer: reason line above a disabled Start button

New-session form with the remote computer selected and a prompt typed: reason line gone

A selected repository key that no longer resolves to a picker row blocks
Start on its own. The prompt reason would name the wrong blocker there, so
the helper now takes the picker's real resolution state and stays silent for
a stale selection.
@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants