feat: Smart Remote Control (Approvals & Action Timeline)#10
feat: Smart Remote Control (Approvals & Action Timeline)#10alexchaomander merged 10 commits intomainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Smart Remote Control' feature, which includes a heuristic engine to detect agent activity and transform it into a task timeline, along with interactive approval modals for terminal prompts. The implementation adds a HeuristicsEngine on the backend to parse PTY data and corresponding frontend components to display the timeline and handle prompts. Feedback focuses on improving the robustness of the heuristic detection, specifically addressing issues where tool labels changing mid-execution could cause orphaned timeline entries, ensuring multi-byte characters are handled correctly in the terminal buffer, and making shell command detection more flexible for varied prompt environments.
backend/src/terminal/heuristics.ts
Outdated
| if (!this.activeAction || this.activeAction.label !== label || this.activeAction.status !== 'running') { | ||
| this.activeAction = { | ||
| id: nanoid(8), | ||
| type, | ||
| label, | ||
| status: 'running', | ||
| startTime: new Date().toISOString() | ||
| }; | ||
| return this.activeAction; | ||
| } |
There was a problem hiding this comment.
This logic creates a new action with a new ID whenever the label changes (e.g., when the 'Working...' placeholder is replaced by the actual command in a subsequent chunk). This results in orphaned 'running' entries in the UI timeline that never complete. Instead, the existing activeAction should be updated if it is already running.
if (this.activeAction && this.activeAction.status === 'running') {
if (this.activeAction.label !== label) {
this.activeAction.label = label;
this.activeAction.type = type;
return this.activeAction;
}
return null;
}
this.activeAction = {
id: nanoid(8),
type,
label,
status: 'running',
startTime: new Date().toISOString()
};
return this.activeAction;
backend/src/terminal/heuristics.ts
Outdated
| const chunk = Buffer.from(chunkBase64, 'base64').toString('utf8'); | ||
| this.term.write(chunk); |
There was a problem hiding this comment.
Converting raw PTY data to a UTF-8 string before writing to xterm can lead to corrupted output if a multi-byte character is split across chunks. Additionally, this.term should be checked for nullity to avoid potential crashes if data arrives after the engine has been disposed (e.g., during WebSocket closure).
| const chunk = Buffer.from(chunkBase64, 'base64').toString('utf8'); | |
| this.term.write(chunk); | |
| if (!this.term) return {}; | |
| const chunk = Buffer.from(chunkBase64, 'base64'); | |
| this.term.write(chunk); |
backend/src/terminal/heuristics.ts
Outdated
| const lastLine = lines[lines.length - 1] || ''; | ||
| // Looks for └──────────┘ or ╰──────────╯ | ||
| if (lastLine.match(/[└╰]─+┘/)) { | ||
| this.activeAction.status = 'completed'; | ||
| this.activeAction.endTime = new Date().toISOString(); | ||
| const completedAction = { ...this.activeAction }; | ||
| this.activeAction = null; | ||
| return completedAction; | ||
| } |
There was a problem hiding this comment.
Checking only the very last line for the completion marker is fragile. If the PTY sends the border followed by a prompt or newline in the same chunk, the marker will be on a previous line and the action will stay in the 'running' state. Scanning the last few lines is more robust.
| const lastLine = lines[lines.length - 1] || ''; | |
| // Looks for └──────────┘ or ╰──────────╯ | |
| if (lastLine.match(/[└╰]─+┘/)) { | |
| this.activeAction.status = 'completed'; | |
| this.activeAction.endTime = new Date().toISOString(); | |
| const completedAction = { ...this.activeAction }; | |
| this.activeAction = null; | |
| return completedAction; | |
| } | |
| for (let i = lines.length - 1; i >= Math.max(0, lines.length - 3); i--) { | |
| if (lines[i].match(/[└╰]─+┘/)) { | |
| this.activeAction.status = 'completed'; | |
| this.activeAction.endTime = new Date().toISOString(); | |
| const completedAction = { ...this.activeAction }; | |
| this.activeAction = null; | |
| return completedAction; | |
| } | |
| } |
backend/src/terminal/heuristics.ts
Outdated
| if (!this.activeAction) { | ||
| for (let i = lines.length - 1; i >= Math.max(0, lines.length - 3); i--) { | ||
| const line = lines[i]; | ||
| const bashMatch = line.match(/^[\$]\s+([a-zA-Z0-9].+)$/); |
There was a problem hiding this comment.
The regex ^[$]\s+ is too restrictive as it only matches lines starting exactly with a dollar sign. Most shell prompts include user, host, or path information before the prompt character. Consider allowing leading characters to make this fallback more effective across different environments.
| const bashMatch = line.match(/^[\$]\s+([a-zA-Z0-9].+)$/); | |
| const bashMatch = line.match(/[$#]\s+([a-zA-Z0-9].+)$/); |
- Fix UTF-8 byte splitting: add decodeUtf8WithCarryover() to handle incomplete multi-byte sequences at chunk boundaries - Fix fragile completion detection: scan all lines instead of just the last line to detect tool completion markers - Fix orphaned running actions: update existing action label instead of creating duplicate when label changes during execution - Add null check for disposed terminal in process() - Expand shell prompt regex to match $, #, >, ❯, λ prompts - Add stale action timeout: mark actions as 'error' after 5 minutes to prevent stuck 'running' states
- Fix lastCompletionCheckIndex bug: track absolute buffer line positions instead of local array indices so completion detection is correct across successive process() calls as the cursor advances - Make HeuristicsEngine.process() async so xterm write() completes before buffer is scanned; update routes to await the result - Remove any casts in useTerminal: extend WebSocket message type union with promptState and action fields - Add dismissPrompt() to useTerminal so approval buttons optimistically clear the overlay immediately on tap without waiting for the backend - Replace smoke-only tests with behavioral assertions: prompt detection (yesno, enter), tool-use box start/completion, shell command fallback, deduplication, and unique action IDs
alexchaomander
force-pushed
the
feat/smart-remote-control
branch
from
7e2ab35 to
7d3a0db
Compare
Review fixes appliedAll issues from the code review have been addressed: 1. Commit message violation (hard block) — fixed 2. 3. Behavioral tests — replaced 4. Prompt overlay self-dismiss — fixed 5. |
1 participant
Summary
This PR implements the Smart Remote Control layer for CloudCode, transforming it from a passive terminal mirror into an active agent management platform. Inspired by specialized tools like claude-watch, these features are built to be agent-agnostic, working with any CLI tool that uses standard terminal conventions.
Key Features
Technical Details
Test Plan