fix: stabilize mobile remote session ui#564
Open
jeffscottward wants to merge 6 commits intoRunMaestro:mainfrom
Open
fix: stabilize mobile remote session ui#564jeffscottward wants to merge 6 commits intoRunMaestro:mainfrom
jeffscottward wants to merge 6 commits intoRunMaestro:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds PTY-backed local command execution with interactive shell arg handling, implements per-session and per-mode command drafts plus mobile input/layout changes, serves index.html fresh per request, adds desktop dogfood automation artifacts, and expands test coverage across these areas. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as LocalCommandRunner
participant PTY as PTY Process
participant Shell as Shell
participant Handler as DataHandler
Runner->>PTY: spawn(shellPath, buildInteractiveShellArgs(...))
PTY->>Shell: execute command (interactive flags)
Shell->>PTY: stdout/stderr (may contain control sequences)
PTY->>Handler: onData(data)
Handler->>Handler: stripControlSequences(data)
Handler->>Runner: emit 'data' (filtered)
Shell->>PTY: exit(code)
PTY->>Runner: emit 'command-exit' + resolve(exitCode)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
Greptile SummaryThis PR stabilises the mobile remote session UI through three targeted fixes: stopping Key changes:
One critical gap: the new PTY code path in Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
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 })
Last reviewed commit: bc32666 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+95
to
+138
| if (!isWindows()) { | ||
| const ptyArgs = buildInteractiveShellArgs(command, shellName); | ||
|
|
||
| logger.debug('[ProcessManager] runCommand spawning PTY', 'ProcessManager', { | ||
| shell: shellToUse, | ||
| shellPath, | ||
| ptyArgs, | ||
| cwd, | ||
| PATH: env.PATH?.substring(0, 100), | ||
| }); | ||
|
|
||
| const ptyProcess = pty.spawn(shellPath, ptyArgs, { | ||
| name: 'xterm-256color', | ||
| cols: 120, | ||
| rows: 40, | ||
| cwd, | ||
| env: env as Record<string, string>, | ||
| }); | ||
|
|
||
| ptyProcess.onData((data) => { | ||
| const output = stripControlSequences(data, command, true); | ||
| logger.debug('[ProcessManager] runCommand PTY stdout FILTERED', 'ProcessManager', { | ||
| sessionId, | ||
| filteredLength: output.length, | ||
| filteredPreview: output.substring(0, 200), | ||
| trimmedEmpty: !output.trim(), | ||
| }); | ||
|
|
||
| if (output.trim()) { | ||
| this.emitter.emit('data', sessionId, output); | ||
| } | ||
| }); | ||
|
|
||
| ptyProcess.onExit(({ exitCode }) => { | ||
| logger.debug('[ProcessManager] runCommand PTY exit', 'ProcessManager', { | ||
| sessionId, | ||
| exitCode, | ||
| }); | ||
| this.emitter.emit('command-exit', sessionId, exitCode); | ||
| resolve({ exitCode }); | ||
| }); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
PTY spawn has no error handling
pty.spawn() is a synchronous native call that throws if the shell binary doesn't exist, cwd is not a valid directory, or the OS refuses to allocate a PTY. When it throws, the Promise constructor catches it and rejects the promise — but the caller receives no command-exit event, no stderr event, and resolve is never called. This leaves the session permanently hanging and produces an unhandled promise rejection.
The Windows path underneath has a proper childProcess.on('error', ...) handler that calls resolve({ exitCode: 1 }) and emits stderr / command-exit. The Unix PTY path should do the same:
if (!isWindows()) {
const ptyArgs = buildInteractiveShellArgs(command, shellName);
let ptyProcess: pty.IPty;
try {
ptyProcess = pty.spawn(shellPath, ptyArgs, {
name: 'xterm-256color',
cols: 120,
rows: 40,
cwd,
env: env as Record<string, string>,
});
} catch (error: unknown) {
const message = error instanceof Error ? error.message : String(error);
logger.error('[ProcessManager] runCommand PTY spawn failed', 'ProcessManager', {
sessionId,
error: message,
});
this.emitter.emit('stderr', sessionId, `Error: ${message}`);
this.emitter.emit('command-exit', sessionId, 1);
resolve({ exitCode: 1 });
return;
}
ptyProcess.onData(...);
ptyProcess.onExit(...);
return;
}
| : activeSession?.inputMode === 'ai' | ||
| ? isSmallScreen | ||
| ? 'Query AI...' | ||
| ? (typeof window !== 'undefined' ? window.innerWidth <= 480 : isSmallScreen) |
There was a problem hiding this comment.
Non-reactive window.innerWidth read in render
This line reads window.innerWidth directly during render instead of using the already-computed, reactive isSmallScreen from useMobileViewState. window.innerWidth is a snapshot value: it won't update the placeholder when the window is resized until an unrelated re-render happens. isSmallScreen already tracks window width reactively via a resize listener, so there's no reason to bypass it here.
Suggested change
| ? (typeof window !== 'undefined' ? window.innerWidth <= 480 : isSmallScreen) | |
| ? isSmallScreen |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
-cwithout double-escaping.Consider adding tests for
fishshell (which uses-i -cwithout-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, butcheckMobile()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
⛔ Files ignored due to path filters (72)
dogfood-output/remote-session-mobile-20260312/screenshots/desktop-live-overlay.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/issue-001-phone-composer-overflow.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-ai-response.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-cli-switch-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-picker-switch-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-back-to-ai-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-current-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-draft-restored-after-picker-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-offline-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-reconnected-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-search-menu-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-switch-session-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-focus-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-then-fill-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-back-to-ai-after-cli.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-cli-pwd.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-session-back-to-ai.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-output-settled.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-pwd-output.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-after-wait.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-ai-factoid-response.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-back-to-ai-with-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-cli-pwd-output.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored-via-picker.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-390.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-device.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-touch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-no-leak.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-via-picker.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-terminal-after-switch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-verification-start.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-history-open.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-offline-banner.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-reconnected.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-switch-other-session-with-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-terminal-after-switch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-blur-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-short-draft-refresh-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-wait-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-ai-factoid-response-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-back-to-ai-restored-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-js-toggle-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-switch-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-empty-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-pwd-output-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-via-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-collapsed-after-blur-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-expanded-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-initial-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix-2.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-short-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-short-draft-before-cli-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-terminal-mode-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-current-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-verify-long-draft-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current-2.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-verify-long-draft-final.pngis excluded by!**/*.png
📒 Files selected for processing (13)
dogfood-output/remote-session-mobile-20260312/.gitignoredogfood-output/remote-session-mobile-20260312/desktop-setup.mjsdogfood-output/remote-session-mobile-20260312/report.mdsrc/__tests__/main/process-manager/utils/pathResolver.test.tssrc/__tests__/main/utils/terminalFilter.test.tssrc/__tests__/main/web-server/routes/staticRoutes.test.tssrc/__tests__/web/mobile/App.test.tsxsrc/__tests__/web/mobile/CommandInputBar.test.tsxsrc/main/process-manager/runners/LocalCommandRunner.tssrc/main/process-manager/utils/pathResolver.tssrc/main/web-server/routes/staticRoutes.tssrc/web/mobile/App.tsxsrc/web/mobile/CommandInputBar.tsx
jeffscottward
force-pushed
the
fix/mobile-remote-session-ux
branch
from
08ff929 to
291d30e
Compare
There was a problem hiding this comment.
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 | 🟠 MajorReplay queued commands with the original
inputMode.The immediate send path now includes
inputMode, but the offline branch still replays throughsendRef.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). ThreadcurrentModethrough thesendRef/useOfflineQueue.sendCommandpath 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 | 🟠 MajorAdd cache-busting headers to the HTML response.
Reading
index.htmlfresh 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
⛔ Files ignored due to path filters (72)
dogfood-output/remote-session-mobile-20260312/screenshots/desktop-live-overlay.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/issue-001-phone-composer-overflow.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-ai-response.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-cli-switch-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-after-picker-switch-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-back-to-ai-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-current-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-draft-restored-after-picker-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-offline-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-reconnected-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-search-menu-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-switch-session-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-focus-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-audit-tap-then-fill-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-back-to-ai-after-cli.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-cli-pwd.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-clean-session-back-to-ai.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-output-settled.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-cli-pwd-output.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-after-wait.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-ai-factoid-response.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-back-to-ai-with-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-cli-pwd-output.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored-via-picker.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-draft-restored.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-390.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-device.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft-touch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-no-leak.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-switch-session-via-picker.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-terminal-after-switch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-fixed-verification-start.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-history-open.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-offline-banner.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-reconnected.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-switch-other-session-with-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-terminal-after-switch.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-blur-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-short-draft-refresh-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-after-wait-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-ai-factoid-response-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-back-to-ai-restored-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-js-toggle-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-switch-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-after-toggle-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-empty-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-pwd-output-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-cli-via-eval-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-collapsed-after-blur-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-expanded-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-initial-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix-2.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-long-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-search-open-short-draft-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-short-draft-before-cli-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/phone-verify-terminal-mode-postfix.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-current-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-initial.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/tablet-verify-long-draft-final.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current-2.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-audit-long-draft-current.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-long-draft.pngis excluded by!**/*.pngdogfood-output/remote-session-mobile-20260312/screenshots/wide-verify-long-draft-final.pngis excluded by!**/*.png
📒 Files selected for processing (15)
docs/releases.mddogfood-output/remote-session-mobile-20260312/.gitignoredogfood-output/remote-session-mobile-20260312/desktop-setup.mjsdogfood-output/remote-session-mobile-20260312/report.mdsrc/__tests__/main/process-manager/runners/LocalCommandRunner.test.tssrc/__tests__/main/process-manager/utils/pathResolver.test.tssrc/__tests__/main/utils/terminalFilter.test.tssrc/__tests__/main/web-server/routes/staticRoutes.test.tssrc/__tests__/web/mobile/App.test.tsxsrc/__tests__/web/mobile/CommandInputBar.test.tsxsrc/main/process-manager/runners/LocalCommandRunner.tssrc/main/process-manager/utils/pathResolver.tssrc/main/web-server/routes/staticRoutes.tssrc/web/mobile/App.tsxsrc/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
Comment on lines
+107
to
+126
| try { | ||
| ptyProcess = pty.spawn(shellPath, ptyArgs, { | ||
| name: 'xterm-256color', | ||
| cols: 120, | ||
| rows: 40, | ||
| cwd, | ||
| env: env as Record<string, string>, | ||
| }); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| logger.error('[ProcessManager] runCommand PTY spawn error', 'ProcessManager', { | ||
| sessionId, | ||
| error: message, | ||
| shell: shellToUse, | ||
| shellPath, | ||
| }); | ||
| this.emitter.emit('stderr', sessionId, `Error: ${message}`); | ||
| this.emitter.emit('command-exit', sessionId, 1); | ||
| resolve({ exitCode: 1 }); | ||
| return; |
There was a problem hiding this comment.
Only treat expected PTY setup failures as recoverable.
This catch normalizes every thrown exception into exit code 1. That makes unexpected runner/runtime bugs look like ordinary shell errors and keeps them out of Sentry. Handle known spawn failures here; capture and rethrow anything else.
As per coding guidelines, "For expected/recoverable errors, catch them explicitly, check the error code, and handle gracefully" and "Do not silently swallow errors with try-catch blocks that only log. Let exceptions bubble up to Sentry for error tracking in production."
🤖 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 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.
There was a problem hiding this comment.
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 | 🟠 MajorOnly clear/log the draft after the command is accepted.
Line 848 clears the input even when
queueCommand()orsend()returnsfalse. 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 | 🟡 MinorManual 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''inaiByTab, socommandInputkeeps preferring the blank local draft over a lateractiveAiTab.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
pwddraft 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
📒 Files selected for processing (4)
docs/releases.mddogfood-output/remote-session-mobile-20260312/report.mdsrc/__tests__/web/mobile/App.test.tsxsrc/web/mobile/App.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- dogfood-output/remote-session-mobile-20260312/report.md
| - **Context warning sash:** Dark text colors in light mode for readability | ||
| - **Session name dimming:** Use `textMain` color to prevent visual dimming | ||
| - **Session name pill:** Allow shrinking so date doesn't collide with type pill | ||
| # Major 0.15.x Additions |
There was a problem hiding this comment.
Demote these nested release-note headings.
The page already has an H1 on Line 6, so these new # headings render as peer page titles instead of subsections and make the outline harder to scan. Line 107 also ships 014.x as the version label. Keep these under their enclosing release blocks instead.
Representative fix
-# Major 0.15.x Additions
+### Major 0.15.x Additions
-# Smaller Changes in 014.x
+### Smaller Changes in 0.14.x
-# Other Changes
+### Other ChangesApply the same demotion to the later # sections in the v0.7.x block as well.
Also applies to: 107-107, 203-203, 295-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/releases.md` at 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
index.htmlso rebuilt asset hashes are served immediatelylsandlrender correctlyTesting
pwd,ls, andlagainst the PM2-hosted remote sessionSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation