Skip to content

fix: stabilize mobile remote session ui#564

Open
jeffscottward wants to merge 9 commits intoRunMaestro:mainfrom
jeffscottward:fix/mobile-remote-session-ux
Open

fix: stabilize mobile remote session ui#564
jeffscottward wants to merge 9 commits intoRunMaestro:mainfrom
jeffscottward:fix/mobile-remote-session-ux

Conversation

@jeffscottward
Copy link
Contributor

@jeffscottward jeffscottward commented Mar 12, 2026

Summary

  • stop caching mobile remote index.html so rebuilt asset hashes are served immediately
  • run Unix mobile CLI commands through a transient PTY so alias and TTY-sensitive commands like ls and l render correctly
  • strip transient PTY control bytes and add regression tests for the web server and shell arg handling

Testing

  • npm test -- src/tests/main/web-server/routes/staticRoutes.test.ts src/tests/main/process-manager/utils/pathResolver.test.ts src/tests/main/utils/terminalFilter.test.ts
  • npm run build:main
  • verified live mobile UI output for pwd, ls, and l against the PM2-hosted remote session

Summary by CodeRabbit

  • New Features

    • Per-session, per-mode command drafts.
    • PTY-based shell execution on Unix-like systems.
    • Enhanced mobile command input with responsive, collapsible layouts.
    • Desktop remote-session orchestration and runtime report generation.
  • Bug Fixes

    • Serve index.html fresh each request to avoid stale assets.
    • VCS ignore updated to exclude runtime artifacts.
  • Tests

    • Expanded coverage for shell args, output filtering, asset serving, runner error handling, and mobile drafts/layouts.
  • Documentation

    • Updated release notes and added session report.

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds PTY-backed local command execution and interactive-shell arg handling; implements per-session and per-mode command drafts and mobile input/layout adjustments; serves index.html fresh per request; introduces desktop dogfood automation artifacts and expands unit/integration tests across these areas.

Changes

Cohort / File(s) Summary
PTY & Shell Integration
src/main/process-manager/runners/LocalCommandRunner.ts, src/main/process-manager/utils/pathResolver.ts, src/__tests__/main/process-manager/runners/LocalCommandRunner.test.ts, src/__tests__/main/process-manager/utils/pathResolver.test.ts
Adds node-pty-based PTY spawn path for non-Windows, exports buildInteractiveShellArgs, applies control-sequence stripping to PTY/non-PTY output, adds recoverable PTY-spawn error handling, and includes tests for shell-arg construction and PTY spawn failure behavior.
Terminal Output Filtering Tests
src/__tests__/main/utils/terminalFilter.test.ts
Adds test to ensure transient PTY wrapper control bytes are removed from output before visible content is emitted.
Mobile: Per-session & Per-mode Drafts
src/web/mobile/App.tsx, src/__tests__/web/mobile/App.test.tsx
Replaces single input state with per-session/mode-scoped draft store, adds draft update/clear/prune logic, updates submit/change flows to use drafts, and adds tests validating per-session and per-mode draft behavior.
Mobile: Command Input UI & Tests
src/web/mobile/CommandInputBar.tsx, src/__tests__/web/mobile/CommandInputBar.test.tsx
Refactors mobile composer layout (collapsed/expanded heights, stacking behavior), simplifies mobile detection to width-based, alters AI vs Terminal submit semantics, adjusts focus/blur/accessibility handling, and expands responsive/layout tests.
Static Index Freshness
src/main/web-server/routes/staticRoutes.ts, src/__tests__/main/web-server/routes/staticRoutes.test.ts
Stops using cached index.html; reads index.html from disk per request and adds an integration test asserting that on-disk changes are served immediately.
Desktop dogfood tooling & artifacts
dogfood-output/remote-session-mobile-20260312/desktop-setup.mjs, dogfood-output/remote-session-mobile-20260312/.gitignore, dogfood-output/remote-session-mobile-20260312/report.md
Adds an Electron orchestration script for automated sessions, ignores runtime/artifact files, and adds a dogfood session report document.
Docs: Release Notes Formatting
docs/releases.md
Reorganizes release notes header structure and presentation without changing feature content.
Tests: Static & Runner Coverage
src/__tests__/main/web-server/routes/staticRoutes.test.ts, src/__tests__/main/process-manager/runners/LocalCommandRunner.test.ts
Adds integration test for index freshness and unit test for PTY spawn failure behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Caller
    participant Runner as LocalCommandRunner
    participant PTY as PTY Process
    participant Handler as DataHandler

    Client->>Runner: run(sessionId, command, cwd)
    Runner->>PTY: spawn(shellPath, buildInteractiveShellArgs(command, shell))
    PTY->>Handler: onData(rawData)
    Handler->>Handler: stripControlSequences(rawData)
    Handler->>Runner: emit 'data' (filtered)
    PTY->>Runner: emit 'command-exit' (exitCode)
    Runner->>Client: resolve(exitCode)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main objective: stabilizing the mobile remote session UI through fixes to PTY handling, asset caching, and control byte stripping.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR stabilises the mobile remote session UI through three targeted fixes: stopping index.html from being cached so rebuilt asset hashes are served immediately, routing Unix CLI commands through a transient PTY so shell aliases (e.g. lseza) resolve correctly, and scoping CommandInputBar drafts per-session and per-AI-tab so they no longer bleed across sessions or modes.

Key changes:

  • staticRoutes.ts: index.html is now read fresh from disk on every request via readFileSync; manifest.json and sw.js retain their cache.
  • LocalCommandRunner.ts: Unix commands now spawn a node-pty process with -l -i -c <cmd> args instead of using child_process.spawn; Windows path is unchanged.
  • pathResolver.ts: New buildInteractiveShellArgs helper encapsulates the PTY arg construction per shell type.
  • App.tsx: commandInput state replaced with a CommandDraftStore (Record<sessionId, { aiByTab, terminal }>), with a cleanup effect that prunes orphaned session/tab keys.
  • CommandInputBar.tsx: useIsMobilePhone now detects narrow screens by width alone (≤480px); a new shouldStackPhoneComposer flag switches the form to flex-direction: column when a long AI draft grows beyond 3 lines.
  • New regression tests cover: PTY control-byte stripping, cache-bypass freshness for index.html, and buildInteractiveShellArgs arg construction.

One critical gap: the new PTY code path in LocalCommandRunner.ts has no try-catch around pty.spawn(). Since node-pty is a native module that throws synchronously on spawn failures (invalid cwd, missing binary, etc.), such an error propagates out of the Promise constructor as an unhandled rejection — resolve is never called, no command-exit event fires, and the session hangs. The Windows path immediately below it has a proper childProcess.on('error', ...) handler; the same safeguard is needed here.

Confidence Score: 3/5

  • Safe to merge after adding a try-catch around pty.spawn() in LocalCommandRunner.ts; all other changes are well-scoped and tested.
  • The three core fixes (cache bypass, draft scoping, PTY aliases) are correct and backed by new tests. The score is lowered from 5 because the PTY spawn path lacks error handling: a synchronous throw from node-pty (e.g., invalid cwd) produces an unhandled promise rejection and permanently hangs the session with no user-visible error, unlike the Windows path which handles this gracefully.
  • src/main/process-manager/runners/LocalCommandRunner.ts — PTY spawn path needs a try-catch with proper resolve({ exitCode: 1 }) + event emission on failure.

Important Files Changed

Filename Overview
src/main/process-manager/runners/LocalCommandRunner.ts Replaces direct child_process.spawn with node-pty on Unix for interactive alias support. The PTY code path is missing a try-catch around pty.spawn(), which can throw synchronously on invalid cwd/shell, leaving the session hung and producing an unhandled promise rejection. The Windows path beneath it retains proper error handling.
src/main/process-manager/utils/pathResolver.ts Adds buildInteractiveShellArgs which returns [-l, -i, -c, command] for zsh/bash and [-i, -c, command] for fish on Unix; returns [command] on Windows. Clean, well-tested utility with no issues.
src/main/web-server/routes/staticRoutes.ts Stops caching index.html by replacing getCachedFile() with a fresh readFileSync() on every request. Intentional trade-off to serve rebuilt asset hashes immediately; other static assets (manifest, sw.js) still use the cache.
src/web/mobile/App.tsx Replaces the flat commandInput state with a CommandDraftStore that scopes drafts per-session and per-AI-tab. Logic is sound; cleanup effect prunes orphaned session/tab keys. Minor: placeholder text reads window.innerWidth non-reactively instead of the existing reactive isSmallScreen prop.
src/web/mobile/CommandInputBar.tsx Adds shouldStackPhoneComposer logic for a column layout on narrow screens when the AI draft grows tall. useIsMobilePhone simplified to width-only check (≤480px). No issues found.
src/tests/main/utils/terminalFilter.test.ts Adds regression test confirming that transient PTY control bytes (\x04\x08\x08) before visible output are stripped by stripControlSequences. Solid coverage.
src/tests/main/web-server/routes/staticRoutes.test.ts New integration test writes two versions of index.html to a temp directory and asserts the handler serves fresh content each time. Properly cleans up via finally block.
src/tests/main/process-manager/utils/pathResolver.test.ts New unit tests for buildInteractiveShellArgs covering zsh, bash, and a command with embedded single quotes. All cases are correct.

Sequence Diagram

sequenceDiagram
    participant MobileUI as Mobile UI
    participant WS as WebSocket
    participant LCR as LocalCommandRunner
    participant PTY as node-pty (Unix)
    participant Shell as Shell Process
    participant TF as terminalFilter

    MobileUI->>WS: send_command { command, inputMode }
    WS->>LCR: run(sessionId, command, cwd, shell)
    Note over LCR: isWindows() == false
    LCR->>LCR: buildInteractiveShellArgs(cmd, shellName)<br/>→ ["-l", "-i", "-c", cmd]
    LCR->>PTY: pty.spawn(shellPath, args, { cwd, env })
    PTY->>Shell: fork + exec with PTY
    Shell-->>PTY: stdout+stderr (merged)
    PTY-->>LCR: onData(rawChunk)
    LCR->>TF: stripControlSequences(rawChunk, cmd, true)
    TF-->>LCR: cleaned output
    LCR-->>WS: emit("data", sessionId, output)
    WS-->>MobileUI: terminal output
    Shell-->>PTY: process exit
    PTY-->>LCR: onExit({ exitCode })
    LCR-->>WS: emit("command-exit", sessionId, exitCode)
    LCR->>LCR: resolve({ exitCode })
Loading

Last reviewed commit: bc32666

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/__tests__/main/process-manager/utils/pathResolver.test.ts (1)

1-23: LGTM - Good test coverage for interactive shell args.

The tests validate the core behavior for zsh and bash shells. The test at lines 10-17 is particularly valuable as it confirms that quoted commands are passed directly to -c without double-escaping.

Consider adding tests for fish shell (which uses -i -c without -l) and the default fallback path for completeness, but the current coverage addresses the primary use cases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/main/process-manager/utils/pathResolver.test.ts` around lines 1
- 23, Add tests to cover the fish-shell behavior and the default fallback in
buildInteractiveShellArgs: add a case asserting
buildInteractiveShellArgs('ls','fish') returns ['-i','-c','ls'] (fish uses -i -c
without -l) and a case for an unknown/undefined shell (e.g.,
buildInteractiveShellArgs('ls', undefined or 'unknown')) asserting it returns
the expected default flags used by buildInteractiveShellArgs; include these new
it() blocks alongside the existing zsh/bash tests to ensure fish and fallback
behavior are validated.
src/web/mobile/CommandInputBar.tsx (1)

79-93: Consider initializing state to avoid layout flash.

The initial state is false, but checkMobile() is called immediately in the effect. On narrow viewports, this causes a brief render with desktop layout before switching to mobile. Consider initializing with a width check:

💡 Proposed improvement
 function useIsMobilePhone(): boolean {
-	const [isMobile, setIsMobile] = useState(false);
+	const [isMobile, setIsMobile] = useState(
+		typeof window !== 'undefined' && window.innerWidth <= MOBILE_MAX_WIDTH
+	);

 	useEffect(() => {
 		const checkMobile = () => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/CommandInputBar.tsx` around lines 79 - 93, The hook
useIsMobilePhone currently initializes isMobile to false which can cause a
layout flash; change the initialization to compute the initial value from the
current viewport (e.g., useState(() => typeof window !== 'undefined' ?
window.innerWidth <= MOBILE_MAX_WIDTH : false)) so the first render matches the
actual width, while keeping the existing resize effect (checkMobile,
add/removeEventListener) unchanged; reference the useIsMobilePhone function and
MOBILE_MAX_WIDTH constant when making this change and ensure the window check
guards server-side rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dogfood-output/remote-session-mobile-20260312/report.md`:
- Around line 48-50: The three bullet lines use "AI to CLI" and "CLI to AI" as
compound modifiers; update those phrases to hyphenated forms ("AI-to-CLI" and
"CLI-to-AI") in the report lines that mention the switch/toggle and the restore
so the modifiers are clear (e.g., change "AI to CLI switch" to "AI-to-CLI
switch" and "CLI to AI restores" to "CLI-to-AI restores"); ensure corresponding
screenshot captions/evidence lines keep the same hyphenated wording for
consistency.

In `@src/main/process-manager/runners/LocalCommandRunner.ts`:
- Around line 106-137: The PTY branch in LocalCommandRunner (inside the
runCommand flow) lacks error handling for pty.spawn which can throw and leave
the promise unresolved; wrap the pty.spawn call in a try-catch, and in the catch
log the error with logger.error including sessionId and error details, emit
this.emitter.emit('command-exit', sessionId, <non-zero code>) to match the
Windows childProcess error behavior, and resolve the surrounding promise with an
appropriate exitCode (e.g., 1 or -1) so the caller is not left waiting.

---

Nitpick comments:
In `@src/__tests__/main/process-manager/utils/pathResolver.test.ts`:
- Around line 1-23: Add tests to cover the fish-shell behavior and the default
fallback in buildInteractiveShellArgs: add a case asserting
buildInteractiveShellArgs('ls','fish') returns ['-i','-c','ls'] (fish uses -i -c
without -l) and a case for an unknown/undefined shell (e.g.,
buildInteractiveShellArgs('ls', undefined or 'unknown')) asserting it returns
the expected default flags used by buildInteractiveShellArgs; include these new
it() blocks alongside the existing zsh/bash tests to ensure fish and fallback
behavior are validated.

In `@src/web/mobile/CommandInputBar.tsx`:
- Around line 79-93: The hook useIsMobilePhone currently initializes isMobile to
false which can cause a layout flash; change the initialization to compute the
initial value from the current viewport (e.g., useState(() => typeof window !==
'undefined' ? window.innerWidth <= MOBILE_MAX_WIDTH : false)) so the first
render matches the actual width, while keeping the existing resize effect
(checkMobile, add/removeEventListener) unchanged; reference the useIsMobilePhone
function and MOBILE_MAX_WIDTH constant when making this change and ensure the
window check guards server-side rendering.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7890f4b6-8d97-4b48-91c5-a5f184ffd351

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff6839 and bc32666.

⛔ Files ignored due to path filters (72)
  • dogfood-output/remote-session-mobile-20260312/screenshots/desktop-live-overlay.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/issue-001-phone-composer-overflow.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-ai-response.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-cli-switch-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-picker-switch-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-back-to-ai-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-current-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-draft-restored-after-picker-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-offline-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-reconnected-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-search-menu-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-switch-session-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-focus-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-then-fill-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-back-to-ai-after-cli.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-cli-pwd.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-session-back-to-ai.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-output-settled.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-pwd-output.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-after-wait.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-ai-factoid-response.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-back-to-ai-with-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-cli-pwd-output.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored-via-picker.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-390.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-device.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-touch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-no-leak.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-via-picker.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-terminal-after-switch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-verification-start.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-history-open.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-offline-banner.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-reconnected.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-switch-other-session-with-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-terminal-after-switch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-blur-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-short-draft-refresh-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-wait-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-ai-factoid-response-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-back-to-ai-restored-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-js-toggle-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-switch-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-empty-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-pwd-output-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-via-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-collapsed-after-blur-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-expanded-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-initial-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix-2.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-short-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-short-draft-before-cli-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-terminal-mode-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-current-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-verify-long-draft-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current-2.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-verify-long-draft-final.png is excluded by !**/*.png
📒 Files selected for processing (13)
  • dogfood-output/remote-session-mobile-20260312/.gitignore
  • dogfood-output/remote-session-mobile-20260312/desktop-setup.mjs
  • dogfood-output/remote-session-mobile-20260312/report.md
  • src/__tests__/main/process-manager/utils/pathResolver.test.ts
  • src/__tests__/main/utils/terminalFilter.test.ts
  • src/__tests__/main/web-server/routes/staticRoutes.test.ts
  • src/__tests__/web/mobile/App.test.tsx
  • src/__tests__/web/mobile/CommandInputBar.test.tsx
  • src/main/process-manager/runners/LocalCommandRunner.ts
  • src/main/process-manager/utils/pathResolver.ts
  • src/main/web-server/routes/staticRoutes.ts
  • src/web/mobile/App.tsx
  • src/web/mobile/CommandInputBar.tsx

@jeffscottward jeffscottward force-pushed the fix/mobile-remote-session-ux branch from 08ff929 to 291d30e Compare March 13, 2026 20:00
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/web/mobile/App.tsx (1)

776-827: ⚠️ Potential issue | 🟠 Major

Replay queued commands with the original inputMode.

The immediate send path now includes inputMode, but the offline branch still replays through sendRef.current(sessionId, command). If a user queues an AI prompt while offline and the session mode flips before reconnect, that prompt can still be replayed as terminal input (or vice versa). Thread currentMode through the sendRef / useOfflineQueue.sendCommand path as well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/App.tsx` around lines 776 - 827, The offline-queue path in
handleCommandSubmit doesn't preserve inputMode: when queuing you call
queueCommand(activeSessionId, command, currentMode) but the replay path still
uses sendRef.current(sessionId, command) (and useOfflineQueue.sendCommand) which
loses currentMode; update the offline queue API and replay logic to accept and
store inputMode and pass it through to send/sendRef.current (e.g., change
queueCommand/sendCommand signatures to include inputMode, ensure the stored
queued item contains inputMode, and update the replay handler that calls
sendRef.current(sessionId, command) to call sendRef.current({ type:
'send_command', sessionId, command, inputMode }) or otherwise pass inputMode
through) so replayed commands use the original inputMode; keep
clearCommandDraft/addUserLogEntry behavior unchanged.
src/main/web-server/routes/staticRoutes.ts (1)

125-152: ⚠️ Potential issue | 🟠 Major

Add cache-busting headers to the HTML response.

Reading index.html fresh fixes the in-process cache, but clients can still reuse a stale document because this response has no explicit cache policy. After a rebuild, a browser or intermediary can keep serving the old HTML and miss the new asset hashes until a hard refresh.

Suggested change
-			reply.type('text/html').send(html);
+			reply
+				.header('Cache-Control', 'no-store, no-cache, must-revalidate')
+				.header('Pragma', 'no-cache')
+				.type('text/html')
+				.send(html);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/web-server/routes/staticRoutes.ts` around lines 125 - 152, The HTML
response served after reading indexPath lacks cache headers—update the route
handling where you build html (the block that calls readFileSync(indexPath,
'utf-8') and ultimately uses reply.type('text/html').send(html)) to set explicit
cache-busting headers before sending: add a Cache-Control header like "no-cache,
no-store, must-revalidate" (and optionally "Pragma: no-cache" and "Expires: 0")
so browsers and intermediaries will revalidate/avoid serving stale index.html
tied to this.securityToken; keep the existing sanitizeId and script injection
logic unchanged.
🧹 Nitpick comments (1)
docs/releases.md (1)

22-22: Consider hyphenating compound adjectives for grammatical correctness.

When compound adjectives precede the noun they modify, they should be hyphenated:

  • Line 22: "open-source" (not "open source")
  • Line 90: "cross-context" (not "cross context")
  • Line 176: "built-in" (not "built in")

Also applies to: 90-90, 176-176

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 22, Update the compound adjectives to use hyphens:
change "open source" in the phrase "Contribute to open source with AI
assistance!" to "open-source", change "cross context" occurrences to
"cross-context", and change "built in" to "built-in" (refer to the phrases
"Contribute to open source with AI assistance!", "cross context", and "built in"
to locate the edits).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 201: The release notes use the incorrect capitalization "Github
Worktree"; update the text to "GitHub Worktree" (replace the string "Github
Worktree" with "GitHub Worktree") so the official brand name is correct in the
releases.md content.

In `@dogfood-output/remote-session-mobile-20260312/report.md`:
- Around line 3-8: The report currently embeds live Cloudflare tunnel URLs (the
"App URL" table cell and multiple issue blocks) that expose the path token;
locate the "App URL" field and any occurrences of URLs matching the
trycloudflare.com pattern (e.g., https://*.trycloudflare.com/<token>) and
replace them with a sanitized placeholder such as [REDACTED_TUNNEL_URL] or a
non-sensitive example (e.g., https://example.trycloudflare.com/REDACTED)
everywhere they appear in the document so no tokenized tunnel URL remains.

In `@src/main/process-manager/runners/LocalCommandRunner.ts`:
- Around line 107-126: In LocalCommandRunner.runCommand where pty.spawn is
wrapped in a try/catch, only treat known PTY spawn failures as recoverable:
inspect the thrown error (e.g., error.code or error.message) and if it matches
expected spawn failures (missing shell binary like ENOENT or validation errors)
perform the existing logger.error, emitter.emit('stderr'...) and resolve({
exitCode: 1 }); otherwise rethrow the error after logging so it bubbles to
Sentry; keep references to pty.spawn, LocalCommandRunner.runCommand, logger,
this.emitter and sessionId when locating and changing the catch block.

In `@src/web/mobile/App.tsx`:
- Around line 636-699: The bug is that clearCommandDraft writes an empty string
into aiByTab which permanently overrides activeAiTab.inputValue; change the
clear behavior so clearing removes the key from aiByTab instead of setting ''.
Update updateCommandDraft (used by clearCommandDraft) so when mode !==
'terminal' and nextValue is '' (or explicitly indicate "clear"), produce a new
state where aiByTab is a shallow copy with the activeAiDraftKey deleted (not set
to ''), while keeping the existing terminal behavior unchanged; reference
commandInput, updateCommandDraft, clearCommandDraft, commandDrafts, aiByTab, and
activeAiDraftKey when making this change.

---

Outside diff comments:
In `@src/main/web-server/routes/staticRoutes.ts`:
- Around line 125-152: The HTML response served after reading indexPath lacks
cache headers—update the route handling where you build html (the block that
calls readFileSync(indexPath, 'utf-8') and ultimately uses
reply.type('text/html').send(html)) to set explicit cache-busting headers before
sending: add a Cache-Control header like "no-cache, no-store, must-revalidate"
(and optionally "Pragma: no-cache" and "Expires: 0") so browsers and
intermediaries will revalidate/avoid serving stale index.html tied to
this.securityToken; keep the existing sanitizeId and script injection logic
unchanged.

In `@src/web/mobile/App.tsx`:
- Around line 776-827: The offline-queue path in handleCommandSubmit doesn't
preserve inputMode: when queuing you call queueCommand(activeSessionId, command,
currentMode) but the replay path still uses sendRef.current(sessionId, command)
(and useOfflineQueue.sendCommand) which loses currentMode; update the offline
queue API and replay logic to accept and store inputMode and pass it through to
send/sendRef.current (e.g., change queueCommand/sendCommand signatures to
include inputMode, ensure the stored queued item contains inputMode, and update
the replay handler that calls sendRef.current(sessionId, command) to call
sendRef.current({ type: 'send_command', sessionId, command, inputMode }) or
otherwise pass inputMode through) so replayed commands use the original
inputMode; keep clearCommandDraft/addUserLogEntry behavior unchanged.

---

Nitpick comments:
In `@docs/releases.md`:
- Line 22: Update the compound adjectives to use hyphens: change "open source"
in the phrase "Contribute to open source with AI assistance!" to "open-source",
change "cross context" occurrences to "cross-context", and change "built in" to
"built-in" (refer to the phrases "Contribute to open source with AI
assistance!", "cross context", and "built in" to locate the edits).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7421c3f9-d4ae-4177-8b36-8d69fe582ae7

📥 Commits

Reviewing files that changed from the base of the PR and between bc32666 and 291d30e.

⛔ Files ignored due to path filters (72)
  • dogfood-output/remote-session-mobile-20260312/screenshots/desktop-live-overlay.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/issue-001-phone-composer-overflow.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-ai-response.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-cli-switch-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-picker-switch-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-back-to-ai-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-current-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-draft-restored-after-picker-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-offline-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-reconnected-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-search-menu-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-switch-session-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-focus-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-then-fill-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-back-to-ai-after-cli.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-cli-pwd.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-session-back-to-ai.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-output-settled.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-pwd-output.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-after-wait.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-ai-factoid-response.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-back-to-ai-with-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-cli-pwd-output.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored-via-picker.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-390.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-device.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-touch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-no-leak.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-via-picker.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-terminal-after-switch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-verification-start.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-history-open.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-offline-banner.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-reconnected.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-switch-other-session-with-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-terminal-after-switch.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-blur-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-short-draft-refresh-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-wait-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-ai-factoid-response-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-back-to-ai-restored-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-js-toggle-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-switch-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-empty-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-pwd-output-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-via-eval-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-collapsed-after-blur-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-expanded-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-initial-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix-2.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-short-draft-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-short-draft-before-cli-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-terminal-mode-postfix.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-current-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-initial.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/tablet-verify-long-draft-final.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current-2.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-long-draft.png is excluded by !**/*.png
  • dogfood-output/remote-session-mobile-20260312/screenshots/wide-verify-long-draft-final.png is excluded by !**/*.png
📒 Files selected for processing (15)
  • docs/releases.md
  • dogfood-output/remote-session-mobile-20260312/.gitignore
  • dogfood-output/remote-session-mobile-20260312/desktop-setup.mjs
  • dogfood-output/remote-session-mobile-20260312/report.md
  • src/__tests__/main/process-manager/runners/LocalCommandRunner.test.ts
  • src/__tests__/main/process-manager/utils/pathResolver.test.ts
  • src/__tests__/main/utils/terminalFilter.test.ts
  • src/__tests__/main/web-server/routes/staticRoutes.test.ts
  • src/__tests__/web/mobile/App.test.tsx
  • src/__tests__/web/mobile/CommandInputBar.test.tsx
  • src/main/process-manager/runners/LocalCommandRunner.ts
  • src/main/process-manager/utils/pathResolver.ts
  • src/main/web-server/routes/staticRoutes.ts
  • src/web/mobile/App.tsx
  • src/web/mobile/CommandInputBar.tsx
✅ Files skipped from review due to trivial changes (1)
  • dogfood-output/remote-session-mobile-20260312/.gitignore
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/tests/main/web-server/routes/staticRoutes.test.ts
  • src/tests/main/utils/terminalFilter.test.ts
  • src/tests/web/mobile/CommandInputBar.test.tsx
  • dogfood-output/remote-session-mobile-20260312/desktop-setup.mjs
  • src/main/process-manager/utils/pathResolver.ts

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/web/mobile/App.tsx (1)

809-859: ⚠️ Potential issue | 🟠 Major

Only clear/log the draft after the command is accepted.

Line 848 clears the input even when queueCommand() or send() returns false. On a transient socket miss or a full offline queue, the user loses the unsent command and still gets an optimistic log entry from Line 820. Gate both actions on a successful queue/send result.

Suggested change
-			// Add user message to session logs immediately for display
-			addUserLogEntry(command, currentMode);
+			let accepted = false;

			// If offline or not connected, queue the command for later
			if (isOffline || !isActuallyConnected) {
				const queued = queueCommand(activeSessionId, command, currentMode);
				if (queued) {
+					accepted = true;
					webLogger.debug(`Command queued for later: ${command.substring(0, 50)}`, 'Mobile');
					// Provide different haptic feedback for queued commands
					triggerHaptic(HAPTIC_PATTERNS.tap);
				} else {
					webLogger.warn('Failed to queue command - queue may be full', 'Mobile');
				}
			} else {
				// Send the command to the active session immediately
				// Include inputMode so the server uses the web's intended mode (not stale server state)
				const sendResult = send({
					type: 'send_command',
					sessionId: activeSessionId,
					command,
					inputMode: currentMode,
				});
+				accepted = sendResult;
				webLogger.info(
					`[Web->Server] Command send result: ${sendResult}, command="${command.substring(0, 50)}" mode=${currentMode} session=${activeSessionId}`,
					'Mobile'
				);
			}

-			// Clear the input
-			clearCommandDraft(currentMode);
+			if (accepted) {
+				addUserLogEntry(command, currentMode);
+				clearCommandDraft(currentMode);
+			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/App.tsx` around lines 809 - 859, handleCommandSubmit currently
adds an optimistic user log entry (addUserLogEntry) and clears the draft
(clearCommandDraft) unconditionally, causing loss of input when queueCommand or
send fails; change the flow so you only call addUserLogEntry, triggerHaptic for
send, and clearCommandDraft after a successful queueCommand (returns true) or a
successful send (send indicates success). Specifically, in handleCommandSubmit,
move the addUserLogEntry and clearCommandDraft calls behind the branches that
check queueCommand(...) and send(...), use the return value of queueCommand and
send to decide success, and on failure keep the draft, log a warning via
webLogger, and trigger the queued/tap haptic only for queued success and the
send haptic only for send success.
♻️ Duplicate comments (1)
src/web/mobile/App.tsx (1)

655-690: ⚠️ Potential issue | 🟡 Minor

Manual AI clears still preserve the blank override.

This only fixes the explicit clearCommandDraft() path. When the user deletes the textarea down to '', updateCommandDraft() still stores '' in aiByTab, so commandInput keeps preferring the blank local draft over a later activeAiTab.inputValue.

Suggested change
-				if (currentDrafts.aiByTab[activeAiDraftKey] === nextValue) {
-					return prev;
-				}
+				if (nextValue === '') {
+					if (!(activeAiDraftKey in currentDrafts.aiByTab)) {
+						return prev;
+					}
+
+					const nextAiByTab = { ...currentDrafts.aiByTab };
+					delete nextAiByTab[activeAiDraftKey];
+
+					return {
+						...prev,
+						[activeSessionId]: {
+							...currentDrafts,
+							aiByTab: nextAiByTab,
+						},
+					};
+				}
+
+				if (currentDrafts.aiByTab[activeAiDraftKey] === nextValue) {
+					return prev;
+				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/web/mobile/App.tsx` around lines 655 - 690, updateCommandDraft currently
writes an explicit empty string into aiByTab when the user clears the textarea,
which preserves a blank override; change the non-terminal branch in
updateCommandDraft so that if nextValue === '' it deletes/removes the
activeAiDraftKey from currentDrafts.aiByTab (or sets it to undefined) instead of
storing '', returning the updated prev with that key omitted; keep the existing
equality check (compare against currentDrafts.aiByTab[activeAiDraftKey]) but
treat '' as a signal to remove the entry so commandInput falls back to
activeAiTab.inputValue; reference updateCommandDraft, currentDrafts, aiByTab,
activeAiDraftKey, and getEmptyDrafts.
🧹 Nitpick comments (1)
src/__tests__/web/mobile/App.test.tsx (1)

1120-1147: Assert the terminal draft comes back too.

This proves the AI draft survives the round-trip toggle, but it never checks that the pwd draft is restored on the next switch back to terminal. A regression that drops the terminal bucket after leaving AI would still pass here.

Tighten the assertion
 			fireEvent.click(screen.getByTestId('mode-toggle'));

 			expect(screen.getByTestId('input-mode')).toHaveTextContent('ai');
 			expect(screen.getByTestId('command-input')).toHaveValue('Explain the repo status');
+
+			fireEvent.click(screen.getByTestId('mode-toggle'));
+			expect(screen.getByTestId('input-mode')).toHaveTextContent('terminal');
+			expect(screen.getByTestId('command-input')).toHaveValue('pwd');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/web/mobile/App.test.tsx` around lines 1120 - 1147, The test
"keeps separate drafts for AI and terminal mode" currently verifies the AI draft
is restored after toggling back, but doesn't assert the terminal draft is
restored; update the test to also assert that when you toggle back to terminal
mode (after returning to AI and then clicking mode-toggle again)
screen.getByTestId('input-mode') shows 'terminal' and
screen.getByTestId('command-input') has the value 'pwd' so both drafts are
preserved; locate the test in the MobileApp suite (the it block named 'keeps
separate drafts for AI and terminal mode') and add that final assertion using
the same test ids ('mode-toggle', 'input-mode', 'command-input').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 20: Change the top-level nested release headings (e.g., the heading
"Major 0.15.x Additions" and the other lone "#" headings in the v0.14.x/v0.7.x
blocks) to demoted headings so they appear as subsections under the page H1:
replace the single leading "#" with "##" (or "###" if you prefer one more level)
for those headings at lines referencing "Major 0.15.x Additions", the "014.x"
version label occurrence, and the other lone "#" headings in the v0.7.x block
(the blocks around the ranges noted) so they render as subsections rather than
peer page titles.

---

Outside diff comments:
In `@src/web/mobile/App.tsx`:
- Around line 809-859: handleCommandSubmit currently adds an optimistic user log
entry (addUserLogEntry) and clears the draft (clearCommandDraft)
unconditionally, causing loss of input when queueCommand or send fails; change
the flow so you only call addUserLogEntry, triggerHaptic for send, and
clearCommandDraft after a successful queueCommand (returns true) or a successful
send (send indicates success). Specifically, in handleCommandSubmit, move the
addUserLogEntry and clearCommandDraft calls behind the branches that check
queueCommand(...) and send(...), use the return value of queueCommand and send
to decide success, and on failure keep the draft, log a warning via webLogger,
and trigger the queued/tap haptic only for queued success and the send haptic
only for send success.

---

Duplicate comments:
In `@src/web/mobile/App.tsx`:
- Around line 655-690: updateCommandDraft currently writes an explicit empty
string into aiByTab when the user clears the textarea, which preserves a blank
override; change the non-terminal branch in updateCommandDraft so that if
nextValue === '' it deletes/removes the activeAiDraftKey from
currentDrafts.aiByTab (or sets it to undefined) instead of storing '', returning
the updated prev with that key omitted; keep the existing equality check
(compare against currentDrafts.aiByTab[activeAiDraftKey]) but treat '' as a
signal to remove the entry so commandInput falls back to activeAiTab.inputValue;
reference updateCommandDraft, currentDrafts, aiByTab, activeAiDraftKey, and
getEmptyDrafts.

---

Nitpick comments:
In `@src/__tests__/web/mobile/App.test.tsx`:
- Around line 1120-1147: The test "keeps separate drafts for AI and terminal
mode" currently verifies the AI draft is restored after toggling back, but
doesn't assert the terminal draft is restored; update the test to also assert
that when you toggle back to terminal mode (after returning to AI and then
clicking mode-toggle again) screen.getByTestId('input-mode') shows 'terminal'
and screen.getByTestId('command-input') has the value 'pwd' so both drafts are
preserved; locate the test in the MobileApp suite (the it block named 'keeps
separate drafts for AI and terminal mode') and add that final assertion using
the same test ids ('mode-toggle', 'input-mode', 'command-input').

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a6b2947c-440a-4ce3-b266-55a2efa09de5

📥 Commits

Reviewing files that changed from the base of the PR and between 291d30e and 17a94bf.

📒 Files selected for processing (4)
  • docs/releases.md
  • dogfood-output/remote-session-mobile-20260312/report.md
  • src/__tests__/web/mobile/App.test.tsx
  • src/web/mobile/App.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • dogfood-output/remote-session-mobile-20260312/report.md

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
docs/releases.md (1)

170-182: ⚠️ Potential issue | 🟡 Minor

Demote nested v0.12.x subsection headings to keep hierarchy consistent.

These headings are currently peers of the release section header instead of children. Please change them to ### so the outline stays structured.

Suggested diff
-## Show Thinking
+### Show Thinking

-## GitHub Spec-Kit Integration
+### GitHub Spec-Kit Integration

-## Context Management Tools
+### Context Management Tools

-## Changes Specific to v0.12.3:
+### Changes Specific to v0.12.3:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` around lines 170 - 182, The nested v0.12.x subsection
headings under the release section are using the same level as the parent
header; update those nested headings (e.g., the "Changes Specific to v0.12.3:"
and any other v0.12.x subsection titles) to use "###" so they become children of
the release section and the document outline remains consistent; search for the
v0.12.x heading texts in docs/releases.md and convert their leading hashes to
"###" accordingly.
🧹 Nitpick comments (1)
src/main/process-manager/runners/LocalCommandRunner.ts (1)

205-210: Remove redundant shell-sequence pre-filtering before stripControlSequences().

The manual regex removals overlap with stripControlSequences, so this does extra work on every stdout chunk. Keeping one filter path will simplify maintenance and reduce hot-path overhead.

Suggested diff
-				// Filter out shell integration sequences
-				output = output.replace(/\x1b?\]1337;[^\x07\x1b\n]*(\x07|\x1b\\)?/g, '');
-				output = output.replace(/\x1b?\]133;[^\x07\x1b\n]*(\x07|\x1b\\)?/g, '');
-				output = output.replace(/\x1b?\]7;[^\x07\x1b\n]*(\x07|\x1b\\)?/g, '');
-				output = output.replace(/\x1b?\][0-9];[^\x07\x1b\n]*(\x07|\x1b\\)?/g, '');
 				output = stripControlSequences(output, command, true);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/process-manager/runners/LocalCommandRunner.ts` around lines 205 -
210, The four manual regex pre-filters that mutate output before calling
stripControlSequences (the lines using .replace(/\x1b?\]1337.../,
/\x1b?\]133.../, /\x1b?\]7.../, /\x1b?\][0-9];.../) are redundant and should be
removed from LocalCommandRunner so the code only calls
stripControlSequences(output, command, true) on each stdout chunk; remove those
specific .replace(...) lines, keep the single stripControlSequences call, and
run/adjust any tests that relied on the duplicated filtering to ensure behavior
is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 101: Multiple user-facing typos were introduced in docs/releases.md
(e.g., "FIle", "built in", "Wortrees", "You will received"); update the affected
strings to correct English: change "FIle" to "File", "built in" to "built-in",
"Wortrees" to "Worktrees" (or the intended term), and "You will received" to
"You will receive" in the sentence starting with "🗄️ Document Graphs. Launch
from file preview..." and likewise fix the same typos in the other affected
block (around the section referenced 176-180); scan the file for similar
regressions and apply the same corrections to ensure consistent, polished
wording.

---

Duplicate comments:
In `@docs/releases.md`:
- Around line 170-182: The nested v0.12.x subsection headings under the release
section are using the same level as the parent header; update those nested
headings (e.g., the "Changes Specific to v0.12.3:" and any other v0.12.x
subsection titles) to use "###" so they become children of the release section
and the document outline remains consistent; search for the v0.12.x heading
texts in docs/releases.md and convert their leading hashes to "###" accordingly.

---

Nitpick comments:
In `@src/main/process-manager/runners/LocalCommandRunner.ts`:
- Around line 205-210: The four manual regex pre-filters that mutate output
before calling stripControlSequences (the lines using .replace(/\x1b?\]1337.../,
/\x1b?\]133.../, /\x1b?\]7.../, /\x1b?\][0-9];.../) are redundant and should be
removed from LocalCommandRunner so the code only calls
stripControlSequences(output, command, true) on each stdout chunk; remove those
specific .replace(...) lines, keep the single stripControlSequences call, and
run/adjust any tests that relied on the duplicated filtering to ensure behavior
is unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8440e1c8-b047-4d38-8617-90ccd077f890

📥 Commits

Reviewing files that changed from the base of the PR and between 17a94bf and 401bda3.

📒 Files selected for processing (3)
  • docs/releases.md
  • src/main/process-manager/runners/LocalCommandRunner.ts
  • src/web/mobile/App.tsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
docs/releases.md (2)

170-182: ⚠️ Potential issue | 🟡 Minor

Demote nested headings under the v0.12.x section.

Lines 170, 174, 178, and 182 are ## headings inside an existing ## v0.12.x block, which breaks heading hierarchy.

Suggested fix
-## Show Thinking
+### Show Thinking
...
-## GitHub Spec-Kit Integration
+### GitHub Spec-Kit Integration
...
-## Context Management Tools
+### Context Management Tools
...
-## Changes Specific to v0.12.3:
+### Changes Specific to v0.12.3:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` around lines 170 - 182, The headings "Show Thinking",
"GitHub Spec-Kit Integration", "Context Management Tools", and "Changes Specific
to v0.12.3:" are currently top-level `##` headers inside the existing `##
v0.12.x` section; demote these nested headers to one level deeper (e.g., change
each `##` to `###`) so they are proper subsections under the `v0.12.x` section
while preserving their exact titles and content.

101-101: ⚠️ Potential issue | 🟡 Minor

Fix typo in release notes text ("FIle" → "File").

Line 101 still has a user-facing typo: FIle tree panel.

Suggested fix
-🗄️ Document Graphs. Launch from file preview or from the FIle tree panel. Explore relationships between Markdown documents that contain links between documents and to URLs.
+🗄️ Document Graphs. Launch from file preview or from the File tree panel. Explore relationships between Markdown documents that contain links between documents and to URLs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 101, In the "🗄️ Document Graphs." release note
sentence that currently reads 'Launch from file preview or from the FIle tree
panel', correct the typo by replacing "FIle" with "File" so the user-facing text
reads 'Launch from file preview or from the File tree panel'; locate the
sentence under the Document Graphs heading and update that exact string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 266: The doc uses both "prerelease" and "pre-release" inconsistently;
update the sentence "Enhanced update checker to filter prerelease tags like -rc,
-beta 🚀" to use the chosen consistent spelling (e.g., change "prerelease" to
"pre-release" or vice versa) so it matches other occurrences in the document
(search for the string "pre-release" / "prerelease" and make all instances
identical).

---

Duplicate comments:
In `@docs/releases.md`:
- Around line 170-182: The headings "Show Thinking", "GitHub Spec-Kit
Integration", "Context Management Tools", and "Changes Specific to v0.12.3:" are
currently top-level `##` headers inside the existing `## v0.12.x` section;
demote these nested headers to one level deeper (e.g., change each `##` to
`###`) so they are proper subsections under the `v0.12.x` section while
preserving their exact titles and content.
- Line 101: In the "🗄️ Document Graphs." release note sentence that currently
reads 'Launch from file preview or from the FIle tree panel', correct the typo
by replacing "FIle" with "File" so the user-facing text reads 'Launch from file
preview or from the File tree panel'; locate the sentence under the Document
Graphs heading and update that exact string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4cb2dd8-8985-40f5-8b9a-7dd518ac09fd

📥 Commits

Reviewing files that changed from the base of the PR and between 401bda3 and dab4476.

📒 Files selected for processing (1)
  • docs/releases.md

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.

1 participant