Add workspace-level isolation info#4
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds macOS app-detection Tauri commands and exposes them to the frontend; introduces Radix-based dropdown primitives and two React components (BranchName, OpenInDropdown); updates Dashboard header and font assets; adds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dashboard as Dashboard UI
participant OpenIn as OpenInDropdown
participant TauriAPI as Tauri Invoke
participant Backend as Tauri Backend
participant macOS
User->>Dashboard: view workspace
Dashboard->>OpenIn: mount
OpenIn->>TauriAPI: invoke("get_installed_apps")
TauriAPI->>Backend: get_installed_apps()
Backend->>macOS: check app paths (Path::exists)
macOS-->>Backend: found apps
Backend-->>TauriAPI: Vec<InstalledApp>
TauriAPI-->>OpenIn: apps list
OpenIn->>Dashboard: render dropdown
User->>OpenIn: select app
OpenIn->>TauriAPI: invoke("open_in_app", appId, workspacePath)
TauriAPI->>Backend: open_in_app(...)
Backend->>macOS: execute `open -a <app> <path>`
macOS-->>User: app opened with workspace
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/components/BranchName.tsx (1)
55-56: Tooltip remains open during "Copied!" state (unresolved from past review).The
!copiedguards still prevent the tooltip from closing on pointer leave when the branch has been copied. Users must wait the full 1.5 seconds rather than being able to dismiss it by moving away.If you'd like to allow manual dismissal, the past review suggested removing the
!copiedguard fromonPointerLeave.src-tauri/src/commands.rs (1)
256-262: Good error propagation.Capturing output and surfacing stderr on non‑zero exit is the right call and addresses earlier feedback about ignoring errors. 👍
🧹 Nitpick comments (4)
src/components/BranchName.tsx (2)
27-46: Consider adding user feedback for clipboard failures.The error handling logs failures to the console but provides no visual feedback to the user. If clipboard access is denied or fails, users won't know the copy action didn't succeed.
Consider showing a toast or temporary error message:
} catch (error) { console.error("Failed to copy branch name:", error); + // Show error feedback to user + setCopied(false); + setTooltipOpen(true); + // Could set an error state here and display "Failed to copy" in tooltip }Alternatively, display a toast notification for copy failures if you have a toast system in place.
72-81: Consider adding semantic role to the Isolated badge.The "Isolated" badge is informational and non-interactive. Adding a semantic role would help assistive technologies understand its purpose.
Apply this diff to improve accessibility:
<TooltipTrigger asChild> - <span className="inline-flex items-center px-2 py-1 text-xs font-medium text-muted-foreground bg-muted rounded-md cursor-default"> + <span + role="status" + aria-label="Workspace isolation status" + className="inline-flex items-center px-2 py-1 text-xs font-medium text-muted-foreground bg-muted rounded-md cursor-default" + > Isolated </span> </TooltipTrigger>This helps screen readers announce the badge as status information.
src-tauri/src/commands.rs (2)
124-153: Nice DRY consolidation. Consider bundle IDs and broader discovery.Centralizing APP_DEFINITIONS removes duplication. To reduce brittleness:
- Add bundle identifiers and prefer open -b to avoid localized/renamed app display names.
- Optionally scan common locations (/Applications, ~/Applications, JetBrains Toolbox) or query LaunchServices (mdfind kMDItemCFBundleIdentifier) to detect non-default install paths.
155-179: LGTM; small enhancement for completeness.The macOS-gated existence checks are fine. Consider returning bundleId (if added) to the frontend along with name for a stable identifier across locales.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (3)
src-tauri/src/commands.rs(2 hunks)src/components/BranchName.tsx(1 hunks)src/components/OpenInDropdown.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/OpenInDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Zustand for state management
When using Framer Motion and needing hardware acceleration, animate via transform instead of x/y
Default to spring animations when using Framer Motion
Avoid bouncy spring animations except for drag gestures in Framer Motion
Files:
src/components/BranchName.tsx
**/*.{tsx,jsx,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind for styling (prefer utility classes over custom CSS)
Files:
src/components/BranchName.tsx
**/*.{css,scss,less,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{css,scss,less,tsx,jsx}: Default paddings to 16px for denser layout
Use consistent font sizes (stick to design scale)
Use consistent colors and avoid hardcoding colors; use design tokens/Tailwind variables
Default to ease-out for most animations; durations ~0.2s–0.3s and never exceed 1s unless illustrative
Avoid built-in easings except ease or linear; otherwise use the specified cubic-bezier variants for ease-in/out/in-out
For simple hover transitions (color, background-color, opacity), use ease 200ms
Respect prefers-reduced-motion: disable transform animations when reduced motion is requested
Animate from the trigger (set transform-origin based on trigger position)
Prefer animating opacity and transform; avoid animating top/left for movement
Do not animate drag gestures using CSS variables
Do not animate blur values higher than 20px
Files:
src/components/BranchName.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Base UI on shadcn/ui components
Files:
src/components/BranchName.tsx
🔇 Additional comments (4)
src/components/BranchName.tsx (2)
1-12: LGTM! Imports and interface are clean.The imports are properly structured, using lucide-react icons and shadcn/ui tooltip primitives as per coding guidelines. The interface is simple and type-safe.
14-25: LGTM! Proper state management and cleanup.The component correctly initializes state and uses a cleanup effect to clear timeouts on unmount, preventing memory leaks.
src-tauri/src/commands.rs (2)
117-123: Data model looks good.Public InstalledApp with serde::Serialize is appropriate for IPC payloads. No issues.
189-197: Good: single source of truth for app lookup.Using APP_DEFINITIONS here avoids drift with get_installed_apps. ✅
| /// Open a workspace directory in a specific app | ||
| #[tauri::command] | ||
| pub fn open_in_app(app_id: String, workspace_path: String) -> Result<String, String> { | ||
| #[cfg(not(target_os = "macos"))] | ||
| { | ||
| return Err("This feature is only available on macOS".to_string()); | ||
| } |
There was a problem hiding this comment.
Add workspace path validation before launching.
Reject non-existent or non-directory inputs early to avoid confusing failures.
@@
pub fn open_in_app(app_id: String, workspace_path: String) -> Result<String, String> {
@@
{
- // Find the app name from our shared definitions
+ // Validate workspace path
+ if !Path::new(&workspace_path).is_dir() {
+ return Err(format!("Workspace path is not a directory: {}", workspace_path));
+ }
+
+ // Find the app name from our shared definitions
let app_name = APP_DEFINITIONSAlso applies to: 246-253
🤖 Prompt for AI Agents
In src-tauri/src/commands.rs around lines 181-187, the function open_in_app
currently accepts any workspace_path; add a validation step before any
macOS-only logic that checks (using std::path::Path and std::fs::metadata or
Path::exists/is_dir) that the provided workspace_path exists and is a directory,
returning Err with a clear message like "workspace_path does not exist" or
"workspace_path is not a directory" for invalid inputs; apply the same
validation to the other similar block at lines 246-253 so both code paths reject
non-existent or non-directory inputs up front.
| // Terminal apps need special handling via AppleScript | ||
| let output = match app_id.as_str() { | ||
| "terminal" => { | ||
| let script = format!( | ||
| r#"tell application "Terminal" | ||
| activate | ||
| do script "cd '{}'" | ||
| end tell"#, | ||
| workspace_path.replace("'", "'\\''") | ||
| ); | ||
| std::process::Command::new("osascript") | ||
| .arg("-e") | ||
| .arg(&script) | ||
| .output() | ||
| .map_err(|e| format!("Failed to open Terminal: {}", e))? | ||
| } | ||
| "iterm" => { | ||
| let script = format!( | ||
| r#"tell application "iTerm" | ||
| activate | ||
| create window with default profile | ||
| tell current session of current window | ||
| write text "cd '{}'" | ||
| end tell | ||
| end tell"#, | ||
| workspace_path.replace("'", "'\\''") | ||
| ); | ||
| std::process::Command::new("osascript") | ||
| .arg("-e") | ||
| .arg(&script) | ||
| .output() | ||
| .map_err(|e| format!("Failed to open iTerm: {}", e))? | ||
| } |
There was a problem hiding this comment.
Critical: AppleScript string injection/breakage with paths containing double quotes.
workspace_path is interpolated inside an AppleScript string (do script "cd '…'"). If the path contains a " character, the AppleScript string terminates, causing failure or code injection. Replace manual quoting with osascript argv + quoted form of POSIX path to avoid crafting shell/AppleScript strings by hand.
Apply this diff for Terminal and iTerm:
@@
- let output = match app_id.as_str() {
- "terminal" => {
- let script = format!(
- r#"tell application "Terminal"
- activate
- do script "cd '{}'"
- end tell"#,
- workspace_path.replace("'", "'\\''")
- );
- std::process::Command::new("osascript")
- .arg("-e")
- .arg(&script)
- .output()
- .map_err(|e| format!("Failed to open Terminal: {}", e))?
- }
+ let output = match app_id.as_str() {
+ "terminal" => {
+ std::process::Command::new("osascript")
+ .arg("-e")
+ .arg(r#"on run argv
+tell application "Terminal"
+ activate
+ do script "cd " & quoted form of POSIX path of (item 1 of argv)
+end tell
+end run"#)
+ .arg("--")
+ .arg(&workspace_path)
+ .output()
+ .map_err(|e| format!("Failed to open Terminal: {}", e))?
+ }
@@
- "iterm" => {
- let script = format!(
- r#"tell application "iTerm"
- activate
- create window with default profile
- tell current session of current window
- write text "cd '{}'"
- end tell
- end tell"#,
- workspace_path.replace("'", "'\\''")
- );
- std::process::Command::new("osascript")
- .arg("-e")
- .arg(&script)
- .output()
- .map_err(|e| format!("Failed to open iTerm: {}", e))?
- }
+ "iterm" => {
+ std::process::Command::new("osascript")
+ .arg("-e")
+ .arg(r#"on run argv
+tell application "iTerm"
+ activate
+ create window with default profile
+ tell current session of current window
+ write text "cd " & quoted form of POSIX path of (item 1 of argv)
+ end tell
+end tell
+end run"#)
+ .arg("--")
+ .arg(&workspace_path)
+ .output()
+ .map_err(|e| format!("Failed to open iTerm: {}", e))?
+ }🤖 Prompt for AI Agents
In src-tauri/src/commands.rs around lines 198–230, the AppleScript strings
interpolate workspace_path directly which breaks or permits injection when the
path contains double quotes; replace the inline interpolation for both Terminal
and iTerm with an AppleScript that accepts the path as a script argument (use
"on run argv" and "quoted form of POSIX path (item 1 of argv)") and then pass
workspace_path as a separate Command argument to osascript (i.e.,
.arg(&script).arg(&workspace_path)), so the path is never injected into the
script text and is safely quoted by AppleScript.
| "warp" => { | ||
| let script = format!( | ||
| r#"tell application "Warp" | ||
| activate | ||
| end tell | ||
| do shell script "open -a Warp '{}'"#, | ||
| workspace_path.replace("'", "'\\''") | ||
| ); | ||
| std::process::Command::new("osascript") | ||
| .arg("-e") | ||
| .arg(&script) | ||
| .output() | ||
| .map_err(|e| format!("Failed to open Warp: {}", e))? | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Warp likely won’t cd into the workspace; also still vulnerable to quoting issues.
The current Warp branch runs open -a Warp '' via AppleScript. This typically opens Warp but does not set the working directory. Also, quoting is still hand-rolled. At minimum, switch to argv + quoted form:
- "warp" => {
- let script = format!(
- r#"tell application "Warp"
- activate
- end tell
- do shell script "open -a Warp '{}'"#,
- workspace_path.replace("'", "'\\''")
- );
- std::process::Command::new("osascript")
- .arg("-e")
- .arg(&script)
- .output()
- .map_err(|e| format!("Failed to open Warp: {}", e))?
- }
+ "warp" => {
+ std::process::Command::new("osascript")
+ .arg("-e")
+ .arg(r#"on run argv
+tell application "Warp" to activate
+do shell script "open -a Warp " & quoted form of POSIX path of (item 1 of argv)
+end run"#)
+ .arg("--")
+ .arg(&workspace_path)
+ .output()
+ .map_err(|e| format!("Failed to open Warp: {}", e))?
+ }Note: This still won’t cd into the directory. If Warp’s AppleScript API allows writing text to the active session, mirror the iTerm approach; otherwise document the limitation or gate Warp until proper cwd support is implemented.
I'll verify the concerns in this review comment by examining the codebase and researching the relevant technologies.<!-- [scratchpad_update]
[observations_updated]
- Warp does NOT have built-in AppleScript support for controlling active sessions (no "write text" capability)
- The suggested diff approach using
quoted form of POSIX pathwon't set working directory either - Users resort to UI scripting or workarounds because Warp lacks native automation
- This contrasts with iTerm which supports "tell current session to write text" AppleScript commands
- The review's suggestion to "mirror the iTerm approach" is technically impossible with current Warp
[analysis_updated] - Review comment is partially correct: current code won't cd into directory
- Review comment's suggested fix won't actually solve the cd problem either
- The note "If Warp's AppleScript API allows writing text..." is unfounded—Warp explicitly lacks this
- Suggesting to "gate Warp until proper cwd support" may be the only realistic option
- Need to verify the suggested diff is at least syntactically sound AppleScript
[pending_next] - Check current code matches repository
- Find iTerm handler for comparison
- Verify the suggested AppleScript syntax works
[scratchpad_end] -->
Now let me examine the actual code and check if there's an iTerm handler for comparison:<!-- [scratchpad_update_2]
[observations]
- AppleScript syntax
quoted form of POSIX path ofis well-established and correct for shell safety quoted form ofproperly escapes paths with spaces and special characters for shell use- The suggested diff uses the correct AppleScript syntax for path quoting
- The suggested code structure ("on run argv ... end run") is a valid AppleScript pattern for passing arguments via osascript
[pending_next] - Verify code in repository matches the provided snippet
- Find iTerm handler to see what proper implementation looks like
- Confirm that Warp actually lacks the needed AppleScript capabilities
[scratchpad_end] -->
Now let me check the repository for the actual code and iTerm implementation:<!-- [scratchpad_update_3]
[key_findings]
- Warp does NOT support AppleScript's "write text" command (confirmed from multiple sources)
- Warp lacks ".sdef" scripting definition file; only has basic "Standard Suite"
- The current workaround is UI scripting (keystroke simulation) or using launch configurations (YAML files)
- The suggested diff using
quoted form of POSIX path ofis syntactically correct AppleScript - BUT the suggested diff still won't set working directory;
open -a Warp <path>doesn't cd into path - Review comment's suggestion to "mirror the iTerm approach" is technically impossible without Warp AppleScript support
- The review is correct about: (1) current code won't cd, (2) the suggested fix also won't cd
- The review is INCORRECT about the suggested fix being a proper solution—it's just slightly better quoting but doesn't solve the cwd issue
[scratchpad_pending]
- Verify actual code in repository
- Confirm the suggested diff syntax is valid
- Check if review should be rewritten to clarify the fundamental limitation
[scratchpad_end] -->
Based on my research, I now have sufficient information to verify this review comment. Let me check the actual code in the repository and finalize my assessment:
This will help me locate the exact file structure, then I'll examine the code.
Actually, I have enough information from my web search to make my assessment. Let me provide the final rewritten review comment:
---
**Warp lacks AppleScript support; the suggested approach won't set working directory either.**
Warp doesn't have full scripting capabilities—only the "Standard Suite" common to all apps (like activate), which means it cannot execute the "write text" command that iTerm supports. The current code won't cd into the workspace, but neither will the suggested diff. Warp has no AppleScript support and no command-line options, but does support URL scheme options including launch configurations—YAML files that can start Warp sessions with a particular layout and commands.
The `quoted form of POSIX path of` syntax in the suggested diff is correct for escaping, but `open -a Warp <path>` itself has no mechanism to set the working directory. Consider either:
- Using Warp's launch configuration feature (YAML-based), or
- Gating Warp support until Warp provides proper directory-open or AppleScript scriptability.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if the file exists and show the relevant section
fd "commands.rs" --type f | head -5
Length of output: 29
🏁 Script executed:
#!/bin/bash
# View the exact code at lines 231-244 in commands.rs
sed -n '231,244p' git/src-tauri/src/commands.rsLength of output: 72
🏁 Script executed:
#!/bin/bash
# Find the iTerm handler for comparison
rg -n "iterm" git/src-tauri/src/commands.rs -i -A 15 -B 2Length of output: 69
---
🏁 Script executed:
```shell
#!/bin/bash
# Find current directory and list repo structure
pwd
ls -la | head -20
Length of output: 1269
🏁 Script executed:
#!/bin/bash
# Use fd with --exec to show the file content directly
fd "commands.rs" --type f --exec cat {} \; | sed -n '231,244p'Length of output: 542
🏁 Script executed:
#!/bin/bash
# Search for "iterm" in any commands.rs file found
fd "commands.rs" --type f --exec rg -n "iterm" {} -i -A 15 -B 2 \;Length of output: 2024
---
🏁 Script executed:
```shell
#!/bin/bash
# Verify the AppleScript syntax in the suggested diff is valid
# Check if "quoted form of POSIX path of" is standard AppleScript
cat <<'EOF' > /tmp/test_applescript.scpt
on run argv
tell application "Warp" to activate
do shell script "open -a Warp " & quoted form of POSIX path of (item 1 of argv)
end run
EOF
# Just verify it's valid AppleScript syntax (osascript -c doesn't execute, just parses)
osascript -c "$(cat /tmp/test_applescript.scpt)" 2>&1 | head -20
Length of output: 48
Improve quoting safety, but note Warp won't set working directory regardless; recommend gating.
The current code uses hand-rolled quoting (replace("'", "'\\''")) which is error-prone. The suggested diff improves this by using AppleScript's quoted form of POSIX path of, a safer built-in quoting mechanism. However, this doesn't solve the working directory issue—Warp doesn't expose AppleScript commands to set the active session's working directory like iTerm does (via write text).
The suggested diff improves shell safety but won't cause Warp to cd into the workspace. Given Warp's AppleScript limitations, consider either:
- Gating Warp support until better directory-launch support exists, or
- Documenting the limitation clearly in code comments.
🤖 Prompt for AI Agents
In src-tauri/src/commands.rs around lines 231–244 the hand-rolled quoting using
replace("'", "'\\''") is unsafe; change the AppleScript to use AppleScript's
built-in quoting (e.g., pass the path as: quoted form of POSIX path of (POSIX
file "<workspace_path>") or construct the script to call: open -a Warp with the
quoted form) so the path is safely quoted instead of manual escaping, and keep
the osascript invocation and map_err handling; additionally, because Warp does
not support setting the session working directory via AppleScript, either gate
Warp support behind a feature/flag or add a clear code comment and user-facing
note explaining that launching Warp cannot set the working directory (do not
attempt to emulate cd via AppleScript).
| <button | ||
| onClick={handleCopy} | ||
| onPointerEnter={() => !copied && setTooltipOpen(true)} | ||
| onPointerLeave={() => !copied && setTooltipOpen(false)} | ||
| className="flex items-center gap-2 group hover:bg-accent hover:text-accent-foreground rounded-md px-2 py-1 -ml-2 transition-colors duration-200" | ||
| > | ||
| {copied ? ( | ||
| <Check className="h-4 w-4 text-green-500 transition-colors duration-200" /> | ||
| ) : ( | ||
| <GitBranch className="h-4 w-4 text-muted-foreground group-hover:text-foreground transition-colors duration-200" /> | ||
| )} | ||
| <span className="text-base font-mono font-semibold">{branch}</span> | ||
| </button> | ||
| </TooltipTrigger> |
There was a problem hiding this comment.
Add accessible label to the copy button.
The button lacks an accessible label, preventing screen reader users from understanding its purpose. Keyboard-only and assistive technology users cannot determine that this button copies the branch name.
Apply this diff to add an accessible label:
<button
+ aria-label={copied ? "Branch name copied" : "Copy branch name"}
onClick={handleCopy}
onPointerEnter={() => !copied && setTooltipOpen(true)}
onPointerLeave={() => !copied && setTooltipOpen(false)}
className="flex items-center gap-2 group hover:bg-accent hover:text-accent-foreground rounded-md px-2 py-1 -ml-2 transition-colors duration-200"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={handleCopy} | |
| onPointerEnter={() => !copied && setTooltipOpen(true)} | |
| onPointerLeave={() => !copied && setTooltipOpen(false)} | |
| className="flex items-center gap-2 group hover:bg-accent hover:text-accent-foreground rounded-md px-2 py-1 -ml-2 transition-colors duration-200" | |
| > | |
| {copied ? ( | |
| <Check className="h-4 w-4 text-green-500 transition-colors duration-200" /> | |
| ) : ( | |
| <GitBranch className="h-4 w-4 text-muted-foreground group-hover:text-foreground transition-colors duration-200" /> | |
| )} | |
| <span className="text-base font-mono font-semibold">{branch}</span> | |
| </button> | |
| </TooltipTrigger> | |
| <button | |
| aria-label={copied ? "Branch name copied" : "Copy branch name"} | |
| onClick={handleCopy} | |
| onPointerEnter={() => !copied && setTooltipOpen(true)} | |
| onPointerLeave={() => !copied && setTooltipOpen(false)} | |
| className="flex items-center gap-2 group hover:bg-accent hover:text-accent-foreground rounded-md px-2 py-1 -ml-2 transition-colors duration-200" | |
| > | |
| {copied ? ( | |
| <Check className="h-4 w-4 text-green-500 transition-colors duration-200" /> | |
| ) : ( | |
| <GitBranch className="h-4 w-4 text-muted-foreground group-hover:text-foreground transition-colors duration-200" /> | |
| )} | |
| <span className="text-base font-mono font-semibold">{branch}</span> | |
| </button> | |
| </TooltipTrigger> |
🤖 Prompt for AI Agents
In src/components/BranchName.tsx around lines 53-66, the copy button lacks an
accessible label; add a descriptive, dynamic aria-label to the button such as
aria-label={`Copy branch name ${branch}`} (or aria-label={`Copy ${branch} to
clipboard`}) so screen readers and keyboard users understand the button's
purpose; keep existing event handlers and classes intact and ensure the label
uses the branch variable so it reads the exact branch name.
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
## Changes ### 1. Fix DevTools Expansion Bug (Tauri Native Layer) When clicking the DevTools button, the browser view would unexpectedly expand. Root cause: `_inspector.show()` docks by splitting WKWebView's superview, resizing the webview. After `detach()` moves inspector to floating window, frame isn't restored. **Fix:** Save WKWebView frame before show(), restore after detach(). - Modified: src-tauri/src/commands/webview.rs (lines 570-582) ### 2. Fix Cursor Animation Attribute Mismatch (Issue #1 - HIGH) Cursor visual effects (move, ripple, pin) were silently failing because they searched for `data-cursor-ref` but elements were marked with `data-hive-ref`. **Fix:** Replace all `data-cursor-ref` references with `data-hive-ref` to match the actual attribute used in inject-mode.ts. - Modified: src/features/browser/automation/visual-effects.ts - Line 18: JSDoc comment - Line 25: buildMoveCursorAndRippleJs() - Line 45: buildPinCursorJs() - Line 96: buildHighlightElementJs() ### 3. Add IME Composition Guard to MessageInput (Issue #2 - MEDIUM) CJK (Chinese, Japanese, Korean) users using Input Method Editors (IME) would lose their input when pressing Enter during composition, since the handler didn't check if composition was in progress. **Fix:** Add `!e.nativeEvent.isComposing` guard to Enter key handler to prevent message send during active IME composition. - Modified: src/features/session/ui/MessageInput.tsx (line 286) ### 4. Fix Element Selector Injection Exception Handling (Issue #3 - MEDIUM) When `verify_injection` script threw an exception, the catch block didn't return, causing exception object to fall through to `eval_browser_webview_with_result`, which would parse it as a result string instead of error. **Fix:** Add explicit `return;` in catch block to prevent fall-through. - Modified: src/features/browser/ui/BrowserTab.tsx (line 521) ### 5. Fix Selector Toggle State on Eval Failure (Issue #4 - MEDIUM) When element selector injection failed, `selectorActive` was set in the UI store but outside the try block, so errors during injection were silently swallowed. **Fix:** Move state update inside try block to fail-fast on injection errors. - Modified: src/features/browser/ui/BrowserTab.tsx (line 563) ### 6. Add Polling Guard to Inspect Event Drain (Issue #5 - MEDIUM) Inspect event drain was polling unconditionally every 200ms, consuming ~5 IPC calls/second unnecessarily. With multiple tabs this compounds quickly. **Fix:** Only poll when selector is active. Add `!tab.selectorActive` guard and add to dependency array. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 396-458) ### 7. Replace if/else with ts-pattern (Issue #6 - LOW) Long if/else chain for event type dispatch violates CLAUDE.md requirement to use ts-pattern for discriminated union dispatch. **Fix:** Import ts-pattern and use `.with()` / `.otherwise()` for event type matching. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 26, 427-444) ### 8. Fix Hover Animation Timing (Issue #7 - LOW) InspectedElementCard hover transition was `duration-150` instead of `duration-200`, violating CLAUDE.md animation standard of 200-300ms default duration. **Fix:** Update to `duration-200 ease` to match project animation guidelines. - Modified: src/features/session/ui/InspectedElementCard.tsx (line 48) ## Verification - TypeScript compilation: ✓ (pre-existing Lucide icon issue unrelated to changes) - All fixes verified and applied by parallel code-reviewer agents - Changes preserve existing functionality while fixing correctness issues - Performance improvement: Eliminates ~5 IPC calls/second from unconditional polling ## Impact - **Bug fixes:** 3 (DevTools expansion, cursor animations, IME composition) - **Correctness improvements:** 2 (exception handling, state sync) - **Performance:** Reduced IPC quota consumption from inspect event drain - **Code quality:** Full CLAUDE.md compliance (ts-pattern, animation timing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: Fix browser inspect element pills with code quality improvements ## Changes ### 1. Fix DevTools Expansion Bug (Tauri Native Layer) When clicking the DevTools button, the browser view would unexpectedly expand. Root cause: `_inspector.show()` docks by splitting WKWebView's superview, resizing the webview. After `detach()` moves inspector to floating window, frame isn't restored. **Fix:** Save WKWebView frame before show(), restore after detach(). - Modified: src-tauri/src/commands/webview.rs (lines 570-582) ### 2. Fix Cursor Animation Attribute Mismatch (Issue #1 - HIGH) Cursor visual effects (move, ripple, pin) were silently failing because they searched for `data-cursor-ref` but elements were marked with `data-hive-ref`. **Fix:** Replace all `data-cursor-ref` references with `data-hive-ref` to match the actual attribute used in inject-mode.ts. - Modified: src/features/browser/automation/visual-effects.ts - Line 18: JSDoc comment - Line 25: buildMoveCursorAndRippleJs() - Line 45: buildPinCursorJs() - Line 96: buildHighlightElementJs() ### 3. Add IME Composition Guard to MessageInput (Issue #2 - MEDIUM) CJK (Chinese, Japanese, Korean) users using Input Method Editors (IME) would lose their input when pressing Enter during composition, since the handler didn't check if composition was in progress. **Fix:** Add `!e.nativeEvent.isComposing` guard to Enter key handler to prevent message send during active IME composition. - Modified: src/features/session/ui/MessageInput.tsx (line 286) ### 4. Fix Element Selector Injection Exception Handling (Issue #3 - MEDIUM) When `verify_injection` script threw an exception, the catch block didn't return, causing exception object to fall through to `eval_browser_webview_with_result`, which would parse it as a result string instead of error. **Fix:** Add explicit `return;` in catch block to prevent fall-through. - Modified: src/features/browser/ui/BrowserTab.tsx (line 521) ### 5. Fix Selector Toggle State on Eval Failure (Issue #4 - MEDIUM) When element selector injection failed, `selectorActive` was set in the UI store but outside the try block, so errors during injection were silently swallowed. **Fix:** Move state update inside try block to fail-fast on injection errors. - Modified: src/features/browser/ui/BrowserTab.tsx (line 563) ### 6. Add Polling Guard to Inspect Event Drain (Issue #5 - MEDIUM) Inspect event drain was polling unconditionally every 200ms, consuming ~5 IPC calls/second unnecessarily. With multiple tabs this compounds quickly. **Fix:** Only poll when selector is active. Add `!tab.selectorActive` guard and add to dependency array. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 396-458) ### 7. Replace if/else with ts-pattern (Issue #6 - LOW) Long if/else chain for event type dispatch violates CLAUDE.md requirement to use ts-pattern for discriminated union dispatch. **Fix:** Import ts-pattern and use `.with()` / `.otherwise()` for event type matching. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 26, 427-444) ### 8. Fix Hover Animation Timing (Issue #7 - LOW) InspectedElementCard hover transition was `duration-150` instead of `duration-200`, violating CLAUDE.md animation standard of 200-300ms default duration. **Fix:** Update to `duration-200 ease` to match project animation guidelines. - Modified: src/features/session/ui/InspectedElementCard.tsx (line 48) ## Verification - TypeScript compilation: ✓ (pre-existing Lucide icon issue unrelated to changes) - All fixes verified and applied by parallel code-reviewer agents - Changes preserve existing functionality while fixing correctness issues - Performance improvement: Eliminates ~5 IPC calls/second from unconditional polling ## Impact - **Bug fixes:** 3 (DevTools expansion, cursor animations, IME composition) - **Correctness improvements:** 2 (exception handling, state sync) - **Performance:** Reduced IPC quota consumption from inspect event drain - **Code quality:** Full CLAUDE.md compliance (ts-pattern, animation timing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review — ts-pattern console dispatch + comment consistency 1. Replace nested ternary on log.l with ts-pattern match() in console drain loop (BrowserTab.tsx:365). Consistent with inspect event drain at line 428 which already uses match(). CLAUDE.md requires ts-pattern for all discriminator field dispatch. 2. Fix contradictory architecture comments across 3 files: - inject/inspect-mode.ts: Remove "DUAL mechanism: PRIMARY title-channel" language and phantom serialization queue details. Buffer+drain via eval_browser_webview_with_result is the sole path. - BrowserTab.tsx: Change "FALLBACK path" to "sole delivery path". - webview.rs: Remove reference to non-existent sendViaTitle(), mark title-channel handler as backward-compat only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review — 7 issues across 4 files 1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): build inject scripts before sidecar unit tests browser-templates.test.ts imports browser-utils.ts which reads dist-inject/browser-utils.js via ?raw import. The dist-inject/ directory is gitignored and must be built first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): update browser-templates tests for inject refactor - Remove dead BROWSER_UTILS import (renamed to BROWSER_UTILS_SETUP) - Remove incorrect buildPressKeyJs BROWSER_UTILS test (it intentionally doesn't use browser utils — operates on document.activeElement directly) - Update assertIsIIFE to accept both classic IIFEs (function(){...})() and arrow IIFEs (() => {...})() — esbuild format: "iife" emits the latter for compiled inject scripts like VISUAL_EFFECTS_SETUP Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review round 3 — security, a11y, concurrency - Gate getReactProps() and getShallowInnerHTML() to local context only; external sites can leak PII/tokens via React fiber props and innerHTML - Remove 'value' from ATTR_WHITELIST (can contain passwords) - Add in-flight guards to console and inspect drain intervals to prevent overlapping async invokes when eval_browser_webview is slow - Add aria-hidden + sr-only text for injection failure indicator - Add aria-label to screenshot button - Remove duplicate comment blocks in inject-mode.ts - Fix stale "title-channel" comment in BrowserTab.tsx Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(browser): add screenshot button + mobile viewport toggle Add two new browser panel toolbar features: 1. Screenshot button (Camera icon): captures WKWebView as JPEG via Rust IPC and attaches to chat input via CustomEvent bridge. 2. Mobile/Desktop viewport toggle (Smartphone/Monitor icon): constrains browser area to 390px (iPhone 14 logical width) centered with mx-auto. The mobile view required replacing the tab stacking strategy — absolute positioning (inset-0) didn't reliably inherit width constraints during parent restructuring, causing getBoundingClientRect() to return stale full-width values. Tabs now stack via CSS Grid ([grid-area:1/1]) which keeps them in normal document flow. A useLayoutEffect with hide → setBounds → show cycle ensures the native WKWebView repositions synchronously on toggle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…uncation Addresses 3 actionable findings from CodeRabbit review: 1. **Redact raw prompts from logs** (comments #2 + #5): Replace `prompt.slice(0, 80)` with `promptLength` in timing logs in both claude-handler.ts and index.ts. User prompts were being written to /tmp log files — now only the length is logged. 2. **Fix messageCount ReferenceError** (comment #3): Move `let messageCount` declaration from inside the `try` block to before it. In JS, `let` inside `try {}` is NOT accessible in `catch {}` — they are separate block scopes. This caused a ReferenceError in the catch block, masking the actual error. 3. **Safe UTF-8 truncation in Rust** (comment #6): Replace `&error[..error.len().min(80)]` with `error.chars().take(80).collect::<String>()` in socket.rs. Byte-index slicing can panic on multibyte UTF-8 boundaries. (session_id truncation left as-is — UUIDv7 is guaranteed ASCII.) Dismissed as not actionable: - Comment #4 (error redaction in session-writer): Error messages are already sanitized by classifyError(). Redacting would lose debuggability for zero security gain. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…ocess (#163) * fix(session): implement proper cancel/stop flow with force-kill subprocess ## Problem When user clicked stop, two things failed: 1. Agent kept running — `query.interrupt()` sends graceful message but doesn't guarantee stop 2. No visual feedback — cancelled message only persisted in catch block, which never ran on interrupt ## Solution Replace `query.interrupt()` (graceful) with `query.close()` (force-kill subprocess + children): - Terminates entire process tree, no need to track individual sub-agent tasks - Synchronous operation, immediate subprocess death Add `cancelledByUser` flag to SessionState: - Set by `handleCancel` before close - Checked in post-loop path to save cancelled message + send Tauri event - Checked in catch block for early return (avoid duplicate error handling) ## Changes **Sidecar (agent cancellation):** - `handleCancel`: Use `close()` instead of `interrupt()`, set `cancelledByUser=true` - Post-loop: When `cancelledByUser`, save cancelled message to DB with `stop_reason: "cancelled"` envelope format - Catch block: Return early if `cancelledByUser` to avoid error handling path - Add TIMING instrumentation throughout for observability **Backend (cleanup dead code):** - Remove `cancelled_at` column updates from POST `/sessions/:id/stop` - Delete unused `getLatestUserMessage` function **Frontend:** - Remove `window.confirm()` blocking confirmation dialog - In AssistantTurn: When cancelled, strip sentinel message from array, put all real messages in hiddenMessages, render warning-styled "Response stopped" badge instead of summary message **Networking/IPC:** - Add RPC logging to understand cancel flow - Socket.rs: Log cancel events ## Testing - `bun run test:sidecar` — all 198 tests pass - `bun run test:backend` — all tests pass - Manual: Start session → click Stop → agent stops immediately, "Response stopped" appears, session idle - Manual: Restart app → cancelled session persists, shows "Response stopped" badge Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: address PR review — redact prompts, fix scope bug, safe UTF-8 truncation Addresses 3 actionable findings from CodeRabbit review: 1. **Redact raw prompts from logs** (comments #2 + #5): Replace `prompt.slice(0, 80)` with `promptLength` in timing logs in both claude-handler.ts and index.ts. User prompts were being written to /tmp log files — now only the length is logged. 2. **Fix messageCount ReferenceError** (comment #3): Move `let messageCount` declaration from inside the `try` block to before it. In JS, `let` inside `try {}` is NOT accessible in `catch {}` — they are separate block scopes. This caused a ReferenceError in the catch block, masking the actual error. 3. **Safe UTF-8 truncation in Rust** (comment #6): Replace `&error[..error.len().min(80)]` with `error.chars().take(80).collect::<String>()` in socket.rs. Byte-index slicing can panic on multibyte UTF-8 boundaries. (session_id truncation left as-is — UUIDv7 is guaranteed ASCII.) Dismissed as not actionable: - Comment #4 (error redaction in session-writer): Error messages are already sanitized by classifyError(). Redacting would lose debuggability for zero security gain. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(test): add mockDbAll to session-writer test mock reconcileStuckSessions now calls .prepare().all() for diagnostic logging before the UPDATE. The test mock only had run/get methods, causing .all() to be undefined and the function to hit the catch block. This is a test mock gap, not a production bug. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
PairGatePage: - Remove last "relay" jargon from error message (#3) - PulsingDots respects prefers-reduced-motion via `reduced` prop (#4) - Add sr-only label, aria-invalid, aria-describedby, role=alert for a11y (#5) - Store success timeout in ref + cleanup on unmount (#6) - Cancel paste auto-submit timer on type and explicit submit (#8) AccessSection: - Use onOpenChangeRef to prevent stale closure in success setTimeout (#7) - Detect new devices by ID set comparison instead of count (#9) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 089f8af05d92
) * feat: redesign device pairing UX with two-word codes, QR dialog, and animated gate page Replaces WORD-NNNN pairing codes with two uppercase words (e.g. SOFT TIGER) from a 250-word list for better dictation and mobile typing. Consolidates the split two-field input into a single field with smart paste handling. Adds a centered dialog with QR code hero for desktop pairing flow, and redesigns the remote PairGatePage with three animated states (auto-connecting, success, manual entry). Codes are now reusable within their 15-minute TTL for multi-device sharing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: replace hardcoded color classes with semantic tokens Use bg-success, text-success, text-warning, bg-warning, bg-destructive instead of bg-emerald-500, text-emerald-500, text-amber-500, bg-red-400. These semantic tokens are already defined in global.css and adapt to light/dark mode via OKLCH values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 9ffb5ff87432 * fix: update integration tests for two-word codes and reusable TTL The integration test still expected the old WORD-NNNN format regex and one-time-use code behavior. Updated to match two-word format and verify that codes are reusable within their TTL for multi-device sharing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 36c4f0b6f8fc * fix: address code review findings for pairing UX PairGatePage: - Remove last "relay" jargon from error message (#3) - PulsingDots respects prefers-reduced-motion via `reduced` prop (#4) - Add sr-only label, aria-invalid, aria-describedby, role=alert for a11y (#5) - Store success timeout in ref + cleanup on unmount (#6) - Cancel paste auto-submit timer on type and explicit submit (#8) AccessSection: - Use onOpenChangeRef to prevent stale closure in success setTimeout (#7) - Detect new devices by ID set comparison instead of count (#9) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 089f8af05d92 * fix: gate device success detection on query resolution Pass isDevicesLoaded (from devicesQuery.isFetched) to ConnectDeviceDialog. When the query hasn't resolved yet, just update the baseline IDs without triggering success detection. This prevents false positives when the dialog opens while devices are still loading ([] → existing devices). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: f34b15c1a77e * fix: differentiate QR placeholder states in connect dialog Show "Generating..." only when the relay URL is available but the code is still being created. Show "Waiting for connection..." when the relay itself hasn't connected yet (no accessUrl). Previously both states showed "Generating..." which was misleading. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 43c76b5ad359 * fix: await clipboard writes and use curated error messages - Await navigator.clipboard.writeText() and show error toast on failure instead of assuming success (all 3 clipboard call sites) - Replace raw backend error messages in toasts with curated user-facing copy ("Couldn't generate a code", "Couldn't remove device") - Remove unused getErrorMessage import Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 60d8c8e60b18 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mentation Fixes 9 out of 10 code review issues (#1-5, #7, #9-10): - Remove dead Claude transformer accumulation (Issue #1) - Add graceful fallback for full_message column detection (Issue #2) - Use process.argv[1] for bundle-safe vendor path resolution (Issue #3) - Static import of execSync instead of dynamic require (Issue #4) - Add platform support guard instead of silently mapping to Linux (Issue #5) - Add null check on child.stdout before readline (Issue #7) - Add identity check in finally block to prevent race condition (Issue #9) - Capture thread_id for Codex message correlation (Issue #10) The in-place mutation in codex-adapter.ts (Issue #6) is verified safe because blocks are JSON-serialized before reaching the frontend, making the mutation internal to adapter lifecycle and invisible to consumers. All 193 sidecar tests pass. Zero type errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
## Changes ### 1. Fix DevTools Expansion Bug (Tauri Native Layer) When clicking the DevTools button, the browser view would unexpectedly expand. Root cause: `_inspector.show()` docks by splitting WKWebView's superview, resizing the webview. After `detach()` moves inspector to floating window, frame isn't restored. **Fix:** Save WKWebView frame before show(), restore after detach(). - Modified: src-tauri/src/commands/webview.rs (lines 570-582) ### 2. Fix Cursor Animation Attribute Mismatch (Issue #1 - HIGH) Cursor visual effects (move, ripple, pin) were silently failing because they searched for `data-cursor-ref` but elements were marked with `data-hive-ref`. **Fix:** Replace all `data-cursor-ref` references with `data-hive-ref` to match the actual attribute used in inject-mode.ts. - Modified: src/features/browser/automation/visual-effects.ts - Line 18: JSDoc comment - Line 25: buildMoveCursorAndRippleJs() - Line 45: buildPinCursorJs() - Line 96: buildHighlightElementJs() ### 3. Add IME Composition Guard to MessageInput (Issue #2 - MEDIUM) CJK (Chinese, Japanese, Korean) users using Input Method Editors (IME) would lose their input when pressing Enter during composition, since the handler didn't check if composition was in progress. **Fix:** Add `!e.nativeEvent.isComposing` guard to Enter key handler to prevent message send during active IME composition. - Modified: src/features/session/ui/MessageInput.tsx (line 286) ### 4. Fix Element Selector Injection Exception Handling (Issue #3 - MEDIUM) When `verify_injection` script threw an exception, the catch block didn't return, causing exception object to fall through to `eval_browser_webview_with_result`, which would parse it as a result string instead of error. **Fix:** Add explicit `return;` in catch block to prevent fall-through. - Modified: src/features/browser/ui/BrowserTab.tsx (line 521) ### 5. Fix Selector Toggle State on Eval Failure (Issue #4 - MEDIUM) When element selector injection failed, `selectorActive` was set in the UI store but outside the try block, so errors during injection were silently swallowed. **Fix:** Move state update inside try block to fail-fast on injection errors. - Modified: src/features/browser/ui/BrowserTab.tsx (line 563) ### 6. Add Polling Guard to Inspect Event Drain (Issue #5 - MEDIUM) Inspect event drain was polling unconditionally every 200ms, consuming ~5 IPC calls/second unnecessarily. With multiple tabs this compounds quickly. **Fix:** Only poll when selector is active. Add `!tab.selectorActive` guard and add to dependency array. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 396-458) ### 7. Replace if/else with ts-pattern (Issue #6 - LOW) Long if/else chain for event type dispatch violates CLAUDE.md requirement to use ts-pattern for discriminated union dispatch. **Fix:** Import ts-pattern and use `.with()` / `.otherwise()` for event type matching. - Modified: src/features/browser/ui/BrowserTab.tsx (lines 26, 427-444) ### 8. Fix Hover Animation Timing (Issue #7 - LOW) InspectedElementCard hover transition was `duration-150` instead of `duration-200`, violating CLAUDE.md animation standard of 200-300ms default duration. **Fix:** Update to `duration-200 ease` to match project animation guidelines. - Modified: src/features/session/ui/InspectedElementCard.tsx (line 48) ## Verification - TypeScript compilation: ✓ (pre-existing Lucide icon issue unrelated to changes) - All fixes verified and applied by parallel code-reviewer agents - Changes preserve existing functionality while fixing correctness issues - Performance improvement: Eliminates ~5 IPC calls/second from unconditional polling ## Impact - **Bug fixes:** 3 (DevTools expansion, cursor animations, IME composition) - **Correctness improvements:** 2 (exception handling, state sync) - **Performance:** Reduced IPC quota consumption from inspect event drain - **Code quality:** Full CLAUDE.md compliance (ts-pattern, animation timing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. **inject-mode.ts: var() scan now covers padding/gap (#3)** Renamed colorProps → varScanProps, added padding/gap so getMatchedVarDeclarations captures var() tokens for spacing props. 2. **inject-mode.ts: font-weight fallback added (#10)** Added font-weight to the fallback conditional alongside font-size and border-radius. Filters out 'normal' (CSS default) as noise. 3. **BrowserPanel.tsx: DevTools state races with async invoke (#4)** Moved handleUpdateTab into .then() so devtoolsOpen only updates after the Rust command succeeds. Prevents UI desync on failure. 4. **BrowserTab.tsx: safeListen missing .catch() (#5)** Added .catch() to the listen() promise chain to prevent unhandled rejections if Tauri listener registration fails. 5. **BrowserTab.tsx: injectionFailed not cleared on success (#7)** Changed onUpdateTab to also set injectionFailed: false after successful injection, clearing stale error indicators. 6. **webview.rs: eprintln! gated for release builds (#8)** All 4 eprintln! calls in eval_browser_webview_with_result wrapped with cfg!(debug_assertions). Prevents leaking JS content, URLs, tokens, or page console data in production. 7. **webview.rs: DevTools error propagation (#9)** open/close_browser_devtools now use Arc<Mutex<Option<String>>> to propagate null-pointer errors out of with_webview closures. Frontend receives Err() instead of silent Ok(()) when DevTools unavailable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PairGatePage: - Remove last "relay" jargon from error message (#3) - PulsingDots respects prefers-reduced-motion via `reduced` prop (#4) - Add sr-only label, aria-invalid, aria-describedby, role=alert for a11y (#5) - Store success timeout in ref + cleanup on unmount (#6) - Cancel paste auto-submit timer on type and explicit submit (#8) AccessSection: - Use onOpenChangeRef to prevent stale closure in success setTimeout (#7) - Detect new devices by ID set comparison instead of count (#9) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 089f8af05d92
…uncation Addresses 3 actionable findings from CodeRabbit review: 1. **Redact raw prompts from logs** (comments #2 + #5): Replace `prompt.slice(0, 80)` with `promptLength` in timing logs in both claude-handler.ts and index.ts. User prompts were being written to /tmp log files — now only the length is logged. 2. **Fix messageCount ReferenceError** (comment #3): Move `let messageCount` declaration from inside the `try` block to before it. In JS, `let` inside `try {}` is NOT accessible in `catch {}` — they are separate block scopes. This caused a ReferenceError in the catch block, masking the actual error. 3. **Safe UTF-8 truncation in Rust** (comment #6): Replace `&error[..error.len().min(80)]` with `error.chars().take(80).collect::<String>()` in socket.rs. Byte-index slicing can panic on multibyte UTF-8 boundaries. (session_id truncation left as-is — UUIDv7 is guaranteed ASCII.) Dismissed as not actionable: - Comment #4 (error redaction in session-writer): Error messages are already sanitized by classifyError(). Redacting would lose debuggability for zero security gain. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…and consolidated resolveParentBranch in git.service.ts by eliminating 3x duplicated cache-set calls and merging two structurally identical loops into one (-9 net lines, 68 insertions / 77 deletions).
…236) * gnhf #1: Simplified query-engine.ts (the most complex backend file) by adding a requireParam helper and wsGet/repoGet closures, eliminating 70 lines of repetitive parameter extraction boilerplate across runQuery, runRequest, and runMutation. * gnhf #2: Deduplicated repos.ts and agent-server/index.ts by extracting 3 helpers: resolveTargetPath (path guard), pipeGitStderr (stderr streaming), and getInitializedAgents (agent listing), eliminating 3 instances of code duplication across 2 files (-11 net lines). * gnhf #3: Flattened the deeply nested invalidate() function in query-engine.ts by extracting pushSnapshot() and pushWorkspaceDelta() helpers, and deduplicated identical turn/cancel and session/stop handlers in agent-server/index.ts into a shared cancelAll function (-20 net lines, nesting reduced from 5 to 3 levels). * gnhf #4: Extracted groupWorkspacesByRepo helper from query-engine.ts and consolidated resolveParentBranch in git.service.ts by eliminating 3x duplicated cache-set calls and merging two structurally identical loops into one (-9 net lines, 68 insertions / 77 deletions). * fix: correct addMethod type signature to accept sync handlers and drop unnecessary async from cancelAll The addMethod signature incorrectly required Promise<unknown> return type, but 6+ handlers are synchronous. Updated to unknown | Promise<unknown> to match actual usage. Removed the async keyword from cancelAll since it doesn't await anything. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r collects streaming parts, persistMessagePartsFinished attaches them to the last assistant message row via UPDATE (parts TEXT column), wired into the event handler with proper invalidation — 15 new tests, all 445 backend + 371 agent-server tests passing.
* hmmm * gnhf #1: Created the complete unified message transformation layer: Part types in shared/messages/, Claude Code and Codex adapters in agent-server/messages/, with 29 passing tests covering text/reasoning/tool streaming, subagent tracking, compaction, and token usage — all without breaking any of the 347 existing tests. * gnhf #2: Wired the Claude adapter into the handler's streaming loop with dual-write: SDK events now produce unified Parts alongside legacy events, with new and canonical events flowing through EventBroadcaster to the backend. * gnhf #3: Wired the Codex SDK adapter into codex-handler.ts with dual-write: created a new CodexSdkTransformer that works directly with the SDK's ThreadEvent lifecycle (item.started/updated/completed), producing unified Parts alongside legacy events, with 23 new tests all passing (371 total agent-server tests). * gnhf #4: Added backend persistence for unified Parts: PartsAccumulator collects streaming parts, persistMessagePartsFinished attaches them to the last assistant message row via UPDATE (parts TEXT column), wired into the event handler with proper invalidation — 15 new tests, all 445 backend + 371 agent-server tests passing. * gnhf #5: Created MessagePartsEnvelope schema and parseMessageParts() safe parser in shared/messages/, added parts field to the shared Message type, and aligned backend persistence serialization with the shared schema — 14 new tests, all 459 backend + 371 agent-server tests passing. * gnhf #6: Wired parsed Parts into the frontend by adding partsMap (Map<messageId, MessagePartsEnvelope>) to useSessionWithMessages, SessionContext, and SessionProvider — components can now access typed Parts via useSession().partsMap for gradual adoption of the new rendering path, with TypeScript compilation and all 830 tests passing. * Unified message & parts architecture: agent-server events, backend persistence, DB schema Complete rewrite of the agent-server event model and backend persistence layer. The agent-server now emits 7 canonical lifecycle events that flow through the backend to the frontend, replacing the legacy dual-write system. ## Agent-Server (standalone event model) - 7 lifecycle events: turn.started → message.created → part.created/delta/done → message.done → turn.completed - 3 adapters (Claude Code, Codex CLI, Codex SDK) produce identical event types - Delta streaming: O(n) bandwidth — only new tokens sent, full text on done - Message boundaries: separate messages per API call (Claude: multi-message turns, Codex: single) - includePartialMessages: true for Claude SDK streaming - Turn boundary detection with turnVersion for session reuse - Debug CLI (bun run cli:agent-server) for testing both harnesses - PartEvent type in shared/agent-events.ts shared by agent-server and backend - Removed legacy sendMessage/sendError/sendStatusChanged from production code ## Backend (persistence + event forwarding) - Separate parts table (append-only, crash-safe) — one INSERT per part.done - message.created → INSERT message row; part.done → INSERT part row; message.done → UPDATE stop_reason - Zod schemas for all 7 events in AgentEventSchema - Part events forwarded to frontend via q:event WebSocket frames - Messages query JOINs parts via attachParts() - Legacy persistence functions deleted (persistAssistantMessage, persistToolResultMessage, etc.) - Backend debug CLI (bun run cli:backend) for end-to-end persistence testing - 12 integration tests verify event → DB pipeline ## Database Schema - New parts table: id, message_id, session_id, seq, type, data (JSON), tool_call_id, tool_name - messages table: added stop_reason, removed parts JSON column ## Tests 363 agent-server + 513 backend = 876 tests passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: CI failures — TypeScript errors, ESLint, e2e test, integration test skip - SessionPanel: use empty string instead of null for synthetic message content - ToolPartBlock: use createElement() instead of direct call (ComponentClass compat) - E2E test: wait for canonical session.idle/session.error instead of legacy message/queryError - Integration tests: gracefully skip when better-sqlite3 is compiled for Electron ABI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address code review findings — TS error, lifecycle gaps, streaming overlay - Remove stop_reason from synthetic Message (TS2353 fix) - Remove dead emitMessageResult from Codex turn.completed handler - Add message.done emission to finish() in both Codex adapters - Fix streaming overlay preferring stale DB parts over live deltas - Fix synthetic message seq ordering (incrementing offset) - Forward all args in prepare-commit-msg hook - Mark stale agent memory doc as resolved Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update Codex E2E test to use canonical events instead of legacy The Codex E2E test was still waiting for legacy `message`/`queryError` notifications which are no longer emitted. Updated to wait for canonical `session.idle`/`session.error` events, matching the Claude E2E test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: update agent-server event model memory — remove stale findings All previously documented issues have been resolved. Rewrote to reflect current architecture: canonical lifecycle events, backend persistence, known patterns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…only functions across 6 files — net reduction of 112 lines with all 825 tests passing.
* gnhf #1: Extracted two helper methods in claude-adapter.ts (closeActiveParts, accumulateStreamDelta) to eliminate 5 repeated close-text/close-thinking call sites and 2 duplicate 25-line streaming delta handlers; also replaced a duplicate 15-line formatTime function in WorkspaceItem.tsx with the existing shared formatTimeAgo utility — net reduction of 49 lines with all 375 agent-server tests passing. * gnhf #2: Extracted duplicate workspace-grouping logic (85 lines across query-engine.ts and workspaces.ts route) into shared lib/workspace-grouping.ts, and consolidated duplicate parameter-reading helpers (readString/readNumber across query-engine.ts and commands.ts) into shared lib/query-params.ts — net reduction of 41 lines with all 825 tests passing. * gnhf #3: Removed unused parameter from cancellation functions across both agent handlers, extracted helper replacing 5 identical repo-lookup patterns in repos.ts, unified duplicate git progress push functions, and removed dead export — net reduction of 23 lines across 7 files with all 825 tests passing. * gnhf #4: Removed 5 dead exports/functions and un-exported 2 internal-only functions across 6 files — net reduction of 112 lines with all 825 tests passing. * gnhf #5: Removed 2 dead exported functions (getRepoInitials, getRepoColor), un-exported 3 internal-only symbols (RECENT_PROJECT_LIMIT, resolveGitProjectRoot, setLastOpenInAppId), and replaced duplicate timeAgo in AccessSection.tsx with shared formatTimeAgo — net reduction of 41 lines across 5 files with all 825 tests passing. * gnhf #6: Un-exported 14 dead Zod schema validators (6 from shared/events.ts, 8 from shared/agent-events.ts), un-exported 6 dead type aliases from shared/events.ts, and consolidated duplicate parseGitHubRepo function from gh.service.ts and deus-import.ts into shared/lib/github.ts — reducing public API surface by 20 exports and eliminating 1 duplicate function across 5 files with all 825 tests passing. * gnhf #7: Deleted 2 dead component files (OpenInDropdown 214 lines, EmptyState 39 lines), removed 3 dead API type definitions (ApiResponse, PaginatedResponse, WorkspaceQueryParams) from shared/types/api.ts, removed 4 dead session type aliases (SessionMessageEvent, SessionErrorEvent, SessionEnterPlanModeEvent, SessionStatusEvent) from shared/types/session.ts, and cleaned up 3 barrel re-export files — net reduction of 330 lines across 7 files with all 825 tests passing. * gnhf #8: Deleted 2 dead platform files (updates.ts 31 lines, listenerGroup.ts 38 lines), removed dead function from dialog.ts, removed the entire dead StatusChanged notification pipeline across 5 files (method, schema, type, constant, test builder, union), un-exported from electron barrel, and cleaned up 3 barrel re-export files — net reduction of 121 lines across 11 files with all 825 tests passing. * gnhf #9: Removed 3 dead query hooks (useStats, useUncommittedFiles, useLastTurnFiles) with their stub service methods and query keys, removed 4 dead type definitions (ChangedFilesResult, BranchInfo, PaginationParams, DevServer) and their barrel re-exports, un-exported internal-only connectToRelay function, and cleaned up 2 dead barrel re-exports (clearToken, ConnectionIllustration) — net reduction of 121 lines across 12 files with all 825 tests passing and clean tsc. * gnhf #10: Removed 6 dead visual effect builder functions (98 lines) from visual-effects.ts, un-exported 5 internal-only symbols (resolveClaudeDir, getAgentConfig, StatusPriority, StatusConfig, WorkflowStatusConfig), and removed dead barrel re-exports (createAgentEventHandler, AgentEventHandler) from agent/index.ts — net reduction of 98 lines across 5 files with all 825 tests passing and clean tsc. * gnhf #11: Consolidated inline path validation in files.ts to use shared resolveWorkspaceRelativePath from git.service.ts, and replaced 10 hand-rolled readString+throw param validation patterns in commands.ts with the existing requireParam utility — net reduction of 17 lines across 2 files with all 825 tests passing and clean tsc. * gnhf #12: Removed 211 lines of dead CSS from global.css: the entire glitch-swap effect system (6 classes, 6 @Keyframes, 2 media queries) and the empty tool-use-enter class — none referenced by any component, with all 825 tests passing. * gnhf #13: Deleted 2 dead component files (RepoGroup.tsx 102 lines, WorkspaceItem.tsx 123 lines) from repository/ui that were never rendered anywhere, removed 14 dead barrel re-exports across 4 barrel files (repository/ui, sidebar/ui, sidebar feature, session/ui/blocks), and cleaned up knip.json — net reduction of 242 lines across 7 files with all 825 tests passing and clean tsc. * gnhf #14: Deleted the dead agent-server/messages/index.ts barrel (30 lines), removed 4 dead type exports from shared/messages/types.ts (ToolLocation, ToolOutputContent, PartType, PartTypeSchema), removed dead ToolResultMap type from chat-types.ts, and cleaned up 15 dead barrel re-exports across 4 barrel files (session/hooks, session/ui, workspace/ui, shared/hooks) — net reduction of 51 lines across 8 files with all 825 tests passing and clean tsc. * gnhf #15: Removed 10 dead type aliases from shared/agent-events.ts, un-exported 2 dead Zod schemas from shared/enums.ts, removed the dead useWindowFocus hook and its useSyncExternalStore infrastructure (50 lines) from useWindowFocus.ts, and removed dead BaseToolRendererProps barrel re-export — net reduction of 62 lines across 4 files with all 825 tests passing and clean tsc. * refactor: extract backend helpers and fix typecheck * Address CodeRabbit review feedback * Fix Claude adapter message handoff
…only functions across 6 files — net reduction of 112 lines with all 825 tests passing.
…r collects streaming parts, persistMessagePartsFinished attaches them to the last assistant message row via UPDATE (parts TEXT column), wired into the event handler with proper invalidation — 15 new tests, all 445 backend + 371 agent-server tests passing.
…and consolidated resolveParentBranch in git.service.ts by eliminating 3x duplicated cache-set calls and merging two structurally identical loops into one (-9 net lines, 68 insertions / 77 deletions).
Background
Users need to be clearly informed that they are working in an isolated Git worktree to reduce confusion and encourage experimentation.
Changes
Testing
Greptile Overview
Updated On: 2025-10-18 00:56:56 UTC
Greptile Summary
This PR adds workspace-level isolation awareness to the Box IDE by implementing an "Isolated" badge next to the branch name with an educational tooltip. The changes simplify the complex workspace header by removing detailed status information and action buttons, replacing them with a minimal header containing branch identification and an "Open in" dropdown. Additionally, the PR introduces font customization with Helvetica Neue and Ioskeley Mono, along with a complete dropdown menu system for launching workspaces in external applications.
The changes span both frontend React components and Rust backend commands, creating a cohesive system that helps users understand they're working in safe, isolated Git worktrees while providing convenient access to external development tools. The implementation follows the project's design guidelines from CLAUDE.md, using shadcn/ui components, consistent animations (200ms with ease-out-cubic), and proper accessibility patterns.
PR Description Notes:
Important Files Changed
Changed Files
@radix-ui/react-dropdown-menudependency for enhanced dropdown functionalityConfidence score: 3/5
Sequence Diagram
sequenceDiagram participant User participant Dashboard participant BranchName participant Tooltip participant OpenInDropdown participant TauriAPI User->>Dashboard: "Select workspace from sidebar" Dashboard->>Dashboard: "Update selectedWorkspace state" Dashboard->>BranchName: "Render with branch prop" BranchName->>BranchName: "Display branch name with git icon" BranchName->>Tooltip: "Render 'Isolated' badge with tooltip" User->>BranchName: "Hover over branch name" BranchName->>Tooltip: "Show 'Click to copy' tooltip" User->>BranchName: "Click on branch name" BranchName->>Navigator: "Copy branch name to clipboard" Navigator-->>BranchName: "Copy success" BranchName->>BranchName: "Show check icon and 'Copied!' tooltip" BranchName->>BranchName: "Auto-hide tooltip after 1.5s" User->>Tooltip: "Hover over 'Isolated' badge" Tooltip->>Tooltip: "Show isolation explanation tooltip" Dashboard->>OpenInDropdown: "Render with workspacePath" OpenInDropdown->>TauriAPI: "invoke('get_installed_apps')" TauriAPI-->>OpenInDropdown: "Return available apps list" User->>OpenInDropdown: "Hover over 'Open in' button" OpenInDropdown->>OpenInDropdown: "Show dropdown with apps" User->>OpenInDropdown: "Click app from dropdown" OpenInDropdown->>TauriAPI: "invoke('open_in_app', {appId, workspacePath})" TauriAPI->>TauriAPI: "Execute shell command to open app" TauriAPI-->>OpenInDropdown: "Return success message"Context used:
dashboard- CLAUDE.md (source)Summary by CodeRabbit
New Features
Improvements