Conversation
- Add Tauri 2 desktop app wrapper (desktop/) - Hide OpenCode top bar, sidebar, session title, and main border in embedded mode - Strip [opencode] prefix from worker task display - Collapse worker list cards to single-line task text - Replace interactive badge with OpenCode badge for opencode workers - Remove badge hover effects and color variants in worker list - Add subtle OpenCode/Transcript toggle in worker detail header - Neutralize accent colors in worker list (gray filter pills, selection state) - Add desktop/.gitignore for node_modules and target
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds macOS Tauri desktop support (build scripts, Swift bindings, Info.plist), introduces a TopBar system and Sidebar simplification in the frontend, applies WKWebView scaling for Tauri, and adds a shared live_worker_transcripts cache with persistence on worker cancellation; also updates message shorthand parsing and related tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| const TopBarContext = createContext<TopBarStore | null>(null); | ||
|
|
||
| export function TopBarProvider({ children }: { children: ReactNode }) { | ||
| const storeRef = useRef<TopBarStore>(null); |
There was a problem hiding this comment.
useRef init is null, so the ref type should allow it.
| const storeRef = useRef<TopBarStore>(null); | |
| const storeRef = useRef<TopBarStore | null>(null); |
| * The component that calls this hook "owns" the topbar content for its lifetime. | ||
| * Uses a ref + effect to avoid re-render loops. | ||
| */ | ||
| export function useSetTopBar(node: ReactNode) { |
There was a problem hiding this comment.
Minor: useSetTopBar is doing a side-effect during render via store.setContent(node). Worth moving this into useLayoutEffect/useEffect to avoid any weirdness with StrictMode / future concurrent rendering.
| const target = e.target as HTMLElement; | ||
| if (target.closest("a, button, input, select, textarea, [role=button]")) return; | ||
| e.preventDefault(); | ||
| (window as any).__TAURI_INTERNALS__.invoke("plugin:window|start_dragging"); |
There was a problem hiding this comment.
invoke() returns a promise here; it's easy to end up with an unhandled rejection if start-dragging fails.
| (window as any).__TAURI_INTERNALS__.invoke("plugin:window|start_dragging"); | |
| void (window as any).__TAURI_INTERNALS__.invoke("plugin:window|start_dragging").catch(() => {}); |
| } | ||
| ], | ||
| "security": { | ||
| "csp": null |
There was a problem hiding this comment.
Worth avoiding csp: null in prod if possible (especially with tauri-plugin-shell enabled) — even a minimal CSP helps limit blast radius if the webview ever navigates somewhere unexpected.
Also: frontendDist being a URL looks suspicious for packaged builds; is the intent for the desktop app to always point at an external server rather than bundling static assets?
| "permissions": [ | ||
| "core:default", | ||
| "core:window:allow-start-dragging", | ||
| "shell:allow-open" |
There was a problem hiding this comment.
shell:allow-open is pretty broad. If we only need this for the OpenCode direct link, consider scoping it down to specific schemes/hosts (e.g. http(s) to localhost/127.0.0.1) to reduce the impact of any accidental/untrusted navigation in the webview.
|
|
||
| // Show window after setup | ||
| if let Some(window) = app.get_webview_window("main") { | ||
| let _ = window.show(); |
There was a problem hiding this comment.
Ignoring the show() result here makes it harder to debug startup issues.
| let _ = window.show(); | |
| if let Err(error) = window.show() { | |
| tracing::warn!(%error, "failed to show main window"); | |
| } |
| @@ -152,10 +162,70 @@ impl ChannelState { | |||
| return Err(format!("Worker {worker_id} not found")); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Small ordering nit: I'd abort the worker handle before draining live_worker_transcripts so we don't drop any last-moment ToolStarted/ToolCompleted events between the drain and the abort.
| if let Some(handle) = handle { | |
| handle.abort(); | |
| } | |
| // Drain the live transcript after aborting so we can persist it. | |
| // The abort kills the worker future, so persist_transcript() inside the | |
| // worker's run() method will never execute. We compensate here. | |
| let live_steps = self | |
| .live_worker_transcripts | |
| .write() | |
| .await | |
| .remove(&worker_id.to_string()); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
interface/src/components/Sidebar.tsx (1)
29-31: Consider removing the unusedliveStatesprop.The
liveStatesprop is declared inSidebarPropsbut is immediately renamed to_liveStatesand never used in the component body. If activity display has been intentionally removed from the sidebar, consider removing this prop from the interface entirely to keep the API clean.♻️ Proposed cleanup
-interface SidebarProps { - liveStates: Record<string, ChannelLiveState>; -} +interface SidebarProps {}And update the component signature:
-export function Sidebar({ liveStates: _liveStates }: SidebarProps) { +export function Sidebar({}: SidebarProps) {Note: This would also require updating the call site in
interface/src/router.tsx.Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/Sidebar.tsx` around lines 29 - 31, SidebarProps declares liveStates which the Sidebar component receives as _liveStates but never uses; remove the unused prop to clean the API. Update the SidebarProps interface to drop the liveStates field and change the Sidebar component signature to remove the _liveStates parameter (reference Sidebar and SidebarProps in interface/src/components/Sidebar.tsx), then update any call sites (e.g., where Sidebar is instantiated in interface/src/router.tsx) to stop passing liveStates. Run TypeScript to catch any remaining references and remove or refactor them accordingly.interface/src/main.tsx (1)
10-13: Consider documenting the zoom factor rationale.The
1.1zoom factor is a magic number. Adding a brief note about why specifically 10% zoom corrects WKWebView's rendering scale would help future maintainers understand the choice.Also note that
document.body.style.zoomis non-standard CSS, but this should be fine since it's specifically targeting WKWebView in Tauri.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/main.tsx` around lines 10 - 13, The magic zoom value 1.1 used in the WKWebView detection block should be documented and made explicit: add a short comment above the if ((window as any).__TAURI_INTERNALS__) check explaining that WKWebView renders at ~10% smaller effective scale and therefore we apply a 1.1 zoom correction, and note that document.body.style.zoom is non-standard but acceptable here because the adjustment is scoped to Tauri/WKWebView; optionally centralize the value as a named constant (e.g., WK_WEBVIEW_ZOOM = 1.1) referenced in the document.body.style.zoom assignment and mention why 1.1 was chosen (empirical/visual correction) so future maintainers understand the rationale.interface/src/components/TopBar.tsx (1)
57-64: Avoid mutating the top-bar store during render.
store.setContent(node)publishes shared state before the route commit finishes. Under interrupted/concurrent renders that can surface header content for a page that never committed, and there is no cleanup path when the owner unmounts. Move the set/clear logic into a layout effect keyed bynode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/TopBar.tsx` around lines 57 - 64, useSetTopBar currently calls store.setContent(node) during render which can publish state prematurely; change it to call store.setContent(node) inside a useLayoutEffect keyed on node, and perform cleanup in the effect return to clear the top bar when the component unmounts or node changes (e.g., call store.setContent(null) or only clear if the store still holds the same node to avoid stomping other updates). In short: move the mutation out of the render path in useSetTopBar, invoke store.setContent(node) in useLayoutEffect([node]) and clear it in the cleanup, referencing the existing useSetTopBar hook and store.setContent method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/src-tauri/build.rs`:
- Around line 14-16: The build script currently only calls
println!("cargo:rerun-if-changed={}", icon_source) inside the existence check,
so Cargo won't watch Spacebot.icon if it didn't exist initially; move the
println! that emits the rerun-if-changed for the icon path (the one using
icon_source) out of the if Path::new(&icon_source).exists() block so the path is
always printed, and keep the warning/error logging about missing file
conditional; apply the same change for the second occurrence referenced around
the other Path::new(&icon_source) check (lines ~54-55).
In `@desktop/src-tauri/crates/macos/Package.resolved`:
- Around line 7-11: The SwiftRs dependency is pinned to the stale "specta"
branch and should be switched to a stable tag; update the Package.resolved entry
that currently shows "branch": "specta" and the associated "revision" value so
that it references the released semantic version (e.g., set "version": "1.0.7")
instead of a branch, and remove or clear the "branch" and old "revision" fields
accordingly so the package now resolves to the tagged release (ensure changes
correspond to the SwiftRs dependency block so tooling will pick up the 1.0.7
tag).
In `@desktop/src-tauri/src/main.rs`:
- Line 35: The call to window.show() in setup() is currently discarding its
Result with `let _ = window.show()`; change this to handle the Result by either
propagating the error from setup() (returning a Result and using the ? operator
on window.show()) or logging the failure (e.g., use process_logger or the app
logger to record the error) so the app won't silently boot without a visible
window; update the function signature of setup() if you choose propagation and
replace the `let _ = window.show()` with either `window.show()?` or an explicit
match/if let Err(e) => log_error!("window.show failed: {:?}", e)` referencing
the window.show() call and setup() function.
In `@interface/src/components/TopBar.tsx`:
- Around line 99-109: The Link in TopBar.tsx that renders the home button
currently contains only a decorative img (alt=""), so add an accessible name to
the Link (the Link element itself) by adding an aria-label (e.g.,
aria-label="Home") or include visually-hidden text inside the Link; keep the
image alt empty if it remains decorative and ensure the attribute is added on
the Link component used in the TopBar to provide a proper accessible name for
screen readers.
- Around line 78-86: Replace the private Tauri invoke call in handleMouseDown
with the public API: import getCurrentWindow from "@tauri-apps/api/window" and
call await getCurrentWindow().startDragging() instead of (window as
any).__TAURI_INTERNALS__.invoke(...); keep the existing guards (IS_TAURI,
e.buttons check, interactive element check) but wrap the await call in try/catch
to handle and log errors (don’t swallow the promise) so startDragging errors are
surfaced.
In `@interface/src/routes/AgentWorkers.tsx`:
- Around line 334-336: The Badge was hardcoded to variant="outline", leaving the
unused helper statusBadgeVariant() and causing TS6133; either restore usage of
statusBadgeVariant(...) as the Badge's variant prop (e.g.,
variant={statusBadgeVariant(status)}) so the helper is referenced, or delete the
unused statusBadgeVariant function and any related imports; update the Badge
declaration in AgentWorkers component (the Badge JSX where variant is currently
"outline") and ensure tests/compilation pass after removing or wiring the
helper.
- Around line 320-333: Replace the literal worker.worker_type === "opencode"
check with the shared OpenCode detection helper so rows that had only the legacy
"[opencode]" prefix still show the badge; import and use the isOpenCodeWorker
helper (from useChannelLiveState) or the hook-provided result when rendering the
Badge in AgentWorkers.tsx (the conditional rendering around Badge should use
isOpenCodeWorker(worker) or the hook value instead of worker.worker_type ===
"opencode") so the badge visibility matches the canonical logic.
- Around line 487-510: The segmented control for OpenCode/Transcript (rendered
when hasOpenCodeEmbed) currently toggles via setActiveTab and uses activeTab for
styling but lacks accessibility state; update the two buttons (the ones that
call setActiveTab("opencode") and setActiveTab("transcript")) to expose the
selected state by adding appropriate ARIA attributes (e.g., aria-pressed or
role="tab" with aria-selected) tied to activeTab (e.g., aria-pressed={activeTab
=== "opencode"} for the OpenCode button and similarly for Transcript), and
optionally add aria-controls pointing to the corresponding panel IDs so
assistive tech can determine which view is active.
In `@src/agent/channel.rs`:
- Around line 165-172: Draining and removing the entry from
live_worker_transcripts before aborting the worker can drop late-arriving
ToolStarted/ToolCompleted events; instead synchronize with the worker or use a
worker-owned transcript source: have the abort path request the worker to
flush/persist its transcript (call persist_transcript from the worker's run() or
expose a flush_transcript method) and only remove the live_worker_transcripts
entry after confirmation of that flush, or acquire a stronger synchronization
point (e.g., hold a per-worker lock or await a flush-complete notification)
before calling remove(&worker_id.to_string()); reference
live_worker_transcripts, persist_transcript, run, ToolStarted/ToolCompleted, and
worker_id to locate where to add the flush/notification and move the removal to
after confirmation.
In `@src/main.rs`:
- Around line 1770-1773: The ChannelControlHandle captured a clone of state
inside Channel::new before you replaced channel.state.live_worker_transcripts,
so control-plane cancels still reference the old map; modify Channel::new to
accept the shared Arc for live_worker_transcripts as an input (or accept a
pre-built State that already contains that Arc) and use that when constructing
the Channel and its ChannelControlHandle (instead of cloning and mutating after
construction), then update call sites that create Channel (the other location
noted) to pass api_state.live_worker_transcripts so both the Channel.state and
the ChannelControlHandle reference the same Arc-backed live_worker_transcripts.
---
Nitpick comments:
In `@interface/src/components/Sidebar.tsx`:
- Around line 29-31: SidebarProps declares liveStates which the Sidebar
component receives as _liveStates but never uses; remove the unused prop to
clean the API. Update the SidebarProps interface to drop the liveStates field
and change the Sidebar component signature to remove the _liveStates parameter
(reference Sidebar and SidebarProps in interface/src/components/Sidebar.tsx),
then update any call sites (e.g., where Sidebar is instantiated in
interface/src/router.tsx) to stop passing liveStates. Run TypeScript to catch
any remaining references and remove or refactor them accordingly.
In `@interface/src/components/TopBar.tsx`:
- Around line 57-64: useSetTopBar currently calls store.setContent(node) during
render which can publish state prematurely; change it to call
store.setContent(node) inside a useLayoutEffect keyed on node, and perform
cleanup in the effect return to clear the top bar when the component unmounts or
node changes (e.g., call store.setContent(null) or only clear if the store still
holds the same node to avoid stomping other updates). In short: move the
mutation out of the render path in useSetTopBar, invoke store.setContent(node)
in useLayoutEffect([node]) and clear it in the cleanup, referencing the existing
useSetTopBar hook and store.setContent method.
In `@interface/src/main.tsx`:
- Around line 10-13: The magic zoom value 1.1 used in the WKWebView detection
block should be documented and made explicit: add a short comment above the if
((window as any).__TAURI_INTERNALS__) check explaining that WKWebView renders at
~10% smaller effective scale and therefore we apply a 1.1 zoom correction, and
note that document.body.style.zoom is non-standard but acceptable here because
the adjustment is scoped to Tauri/WKWebView; optionally centralize the value as
a named constant (e.g., WK_WEBVIEW_ZOOM = 1.1) referenced in the
document.body.style.zoom assignment and mention why 1.1 was chosen
(empirical/visual correction) so future maintainers understand the rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29bcdd84-9b88-4aed-a7c3-aeadd5e8a049
⛔ Files ignored due to path filters (24)
desktop/assets/Spacebot.icon/Assets/sd 2.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/Spacebot.icon/Assets/sd.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/Spacebot.icon/icon.jsonis excluded by!**/*.jsondesktop/assets/exports/Spacebot-iOS-ClearDark-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/exports/Spacebot-iOS-ClearLight-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/exports/Spacebot-iOS-Dark-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/exports/Spacebot-iOS-Default-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/exports/Spacebot-iOS-TintedDark-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/assets/exports/Spacebot-iOS-TintedLight-1024x1024@1x.pngis excluded by!**/*.png,!**/*.pngdesktop/bun.lockis excluded by!**/*.lock,!**/*.lockdesktop/package.jsonis excluded by!**/*.jsondesktop/src-tauri/Cargo.lockis excluded by!**/*.lock,!**/*.lockdesktop/src-tauri/Cargo.tomlis excluded by!**/*.tomldesktop/src-tauri/capabilities/default.jsonis excluded by!**/*.jsondesktop/src-tauri/crates/macos/Cargo.lockis excluded by!**/*.lock,!**/*.lockdesktop/src-tauri/crates/macos/Cargo.tomlis excluded by!**/*.tomldesktop/src-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**,!**/*.json,!**/gen/**desktop/src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**,!**/*.json,!**/gen/**desktop/src-tauri/gen/schemas/desktop-schema.jsonis excluded by!**/gen/**,!**/*.json,!**/gen/**desktop/src-tauri/gen/schemas/macOS-schema.jsonis excluded by!**/gen/**,!**/*.json,!**/gen/**desktop/src-tauri/icons/128x128.pngis excluded by!**/*.png,!**/*.pngdesktop/src-tauri/icons/128x128@2x.pngis excluded by!**/*.png,!**/*.pngdesktop/src-tauri/icons/32x32.pngis excluded by!**/*.png,!**/*.pngdesktop/src-tauri/tauri.conf.jsonis excluded by!**/*.json
📒 Files selected for processing (23)
desktop/.gitignoredesktop/src-tauri/.gitignoredesktop/src-tauri/Info.plistdesktop/src-tauri/build.rsdesktop/src-tauri/crates/macos/Package.resolveddesktop/src-tauri/crates/macos/Package.swiftdesktop/src-tauri/crates/macos/build.rsdesktop/src-tauri/crates/macos/src-swift/window.swiftdesktop/src-tauri/crates/macos/src/lib.rsdesktop/src-tauri/icons/icon.icnsdesktop/src-tauri/src/main.rsinterface/src/api/client.tsinterface/src/components/Sidebar.tsxinterface/src/components/TopBar.tsxinterface/src/main.tsxinterface/src/router.tsxinterface/src/routes/AgentWorkers.tsxinterface/src/routes/Overview.tsxinterface/src/routes/Settings.tsxinterface/src/ui/style/style.scsssrc/agent/channel.rssrc/main.rstests/context_dump.rs
| if std::path::Path::new(&icon_source).exists() { | ||
| println!("cargo:rerun-if-changed={}", icon_source); | ||
|
|
There was a problem hiding this comment.
Always emit cargo:rerun-if-changed for the icon path.
If Spacebot.icon is absent on the first build, this script only logs a warning and Cargo never watches that path. Adding the file later will not re-run the build script until some unrelated change forces a rebuild.
Suggested fix
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
let icon_source = format!("{}/../assets/Spacebot.icon", manifest_dir);
let gen_dir = format!("{}/gen/icon", manifest_dir);
+ println!("cargo:rerun-if-changed={}", icon_source);
std::fs::create_dir_all(&gen_dir).expect("Failed to create gen/icon directory");
if std::path::Path::new(&icon_source).exists() {
- println!("cargo:rerun-if-changed={}", icon_source);
-
let output = Command::new("xcrun")Also applies to: 54-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src-tauri/build.rs` around lines 14 - 16, The build script currently
only calls println!("cargo:rerun-if-changed={}", icon_source) inside the
existence check, so Cargo won't watch Spacebot.icon if it didn't exist
initially; move the println! that emits the rerun-if-changed for the icon path
(the one using icon_source) out of the if Path::new(&icon_source).exists() block
so the path is always printed, and keep the warning/error logging about missing
file conditional; apply the same change for the second occurrence referenced
around the other Path::new(&icon_source) check (lines ~54-55).
| "state": { | ||
| "branch": "specta", | ||
| "revision": "e0b4a5f444a4204efa8e8270468318bc7836fcce", | ||
| "version": null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
swift-rs latest version and specta branch status
💡 Result:
- swift-rs latest published crate version: 1.0.7 (docs.rs “latest” points to 1.0.7). (docs.rs)
spectabranch status (Brendonovich/swift-rs):- The repo has a
spectabranch, last updated May 8, 2023. (github.com) - It appears stale vs
main:spectashows 126 commits, whilemainshows 154 commits (sospectais ~28 commits behind, assuming mostly linear history). (github.com)
- The repo has a
Citations:
- 1: https://docs.rs/crate/swift-rs/latest?utm_source=openai
- 2: https://github.com/Brendonovich/swift-rs/branches/all
- 3: https://github.com/Brendonovich/swift-rs/tree/specta
Switch from the stale specta branch to a stable, tagged release.
The SwiftRs package is pinned to the specta branch, which has been unmaintained since May 2023 and is now 28+ commits behind the main branch. A stable released version (1.0.7) is available and should be used instead. While the current revision pinning provides reproducibility, relying on an abandoned branch diverges from best practices and creates long-term maintenance risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src-tauri/crates/macos/Package.resolved` around lines 7 - 11, The
SwiftRs dependency is pinned to the stale "specta" branch and should be switched
to a stable tag; update the Package.resolved entry that currently shows
"branch": "specta" and the associated "revision" value so that it references the
released semantic version (e.g., set "version": "1.0.7") instead of a branch,
and remove or clear the "branch" and old "revision" fields accordingly so the
package now resolves to the tagged release (ensure changes correspond to the
SwiftRs dependency block so tooling will pick up the 1.0.7 tag).
|
|
||
| // Show window after setup | ||
| if let Some(window) = app.get_webview_window("main") { | ||
| let _ = window.show(); |
There was a problem hiding this comment.
Don't drop window.show() errors.
If show() fails here, the app can boot with no visible window and no clue why. Please log or propagate the failure from setup(). As per coding guidelines, "Don't silently discard errors. No let _ = on Results. Handle them, log them, or propagate them. The only exception is .ok() on channel sends where the receiver may be dropped"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src-tauri/src/main.rs` at line 35, The call to window.show() in
setup() is currently discarding its Result with `let _ = window.show()`; change
this to handle the Result by either propagating the error from setup()
(returning a Result and using the ? operator on window.show()) or logging the
failure (e.g., use process_logger or the app logger to record the error) so the
app won't silently boot without a visible window; update the function signature
of setup() if you choose propagation and replace the `let _ = window.show()`
with either `window.show()?` or an explicit match/if let Err(e) =>
log_error!("window.show failed: {:?}", e)` referencing the window.show() call
and setup() function.
| const handleMouseDown = useCallback((e: MouseEvent) => { | ||
| if (!IS_TAURI) return; | ||
| // Only drag on primary button, and not when clicking interactive elements | ||
| if (e.buttons !== 1) return; | ||
| const target = e.target as HTMLElement; | ||
| if (target.closest("a, button, input, select, textarea, [role=button]")) return; | ||
| e.preventDefault(); | ||
| (window as any).__TAURI_INTERNALS__.invoke("plugin:window|start_dragging"); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure to understand what we're working with
git ls-files | head -20Repository: spacedriveapp/spacebot
Length of output: 603
🏁 Script executed:
# Check if there's a Tauri configuration file
fd -name "tauri.conf.json" -o -name "Cargo.toml" -o -name "package.json" | head -10Repository: spacedriveapp/spacebot
Length of output: 296
🏁 Script executed:
# Examine the TopBar.tsx file in context
cat -n interface/src/components/TopBar.tsx | sed -n '70,95p'Repository: spacedriveapp/spacebot
Length of output: 1123
🏁 Script executed:
# Search for other uses of __TAURI_INTERNALS__ in the codebase to understand scope
rg "__TAURI_INTERNALS__" -nRepository: spacedriveapp/spacebot
Length of output: 353
🏁 Script executed:
# Search for Tauri window API usage patterns
rg "window\.__TAURI__" -n -A 2Repository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
# Check for any comments about this decision or error handling
rg "start_dragging\|dragging\|window.*drag" -n -B 2 -A 2Repository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
# Check package.json for Tauri version
fd "package.json" -path "*/interface/*" | head -5Repository: spacedriveapp/spacebot
Length of output: 234
🏁 Script executed:
# Look at the Tauri dependencies
cat interface/package.json 2>/dev/null | grep -A 10 -B 2 "@tauri"Repository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
# Check for any imports from Tauri APIs in the file
cat interface/src/components/TopBar.tsx | grep -i "import.*tauri\|from.*tauri"Repository: spacedriveapp/spacebot
Length of output: 117
🏁 Script executed:
# Check the main.tsx to see how Tauri is initialized
cat interface/src/main.tsx | head -50Repository: spacedriveapp/spacebot
Length of output: 642
🏁 Script executed:
# Find package.json in the interface directory
find interface -name "package.json" -type fRepository: spacedriveapp/spacebot
Length of output: 88
🏁 Script executed:
# Read the package.json
cat interface/package.json 2>/dev/null | grep -E "\"@tauri-apps|version" | head -20Repository: spacedriveapp/spacebot
Length of output: 86
🏁 Script executed:
# If package.json not found in interface, search the whole repo
find . -maxdepth 3 -name "package.json" -type f | xargs grep "@tauri-apps" 2>/dev/null | head -10Repository: spacedriveapp/spacebot
Length of output: 116
🏁 Script executed:
# Read the full interface/package.json to see all dependencies
cat interface/package.jsonRepository: spacedriveapp/spacebot
Length of output: 2398
🏁 Script executed:
# Read desktop/package.json to see Tauri setup
cat desktop/package.json | head -50Repository: spacedriveapp/spacebot
Length of output: 278
🏁 Script executed:
# Check if `@tauri-apps/api` is imported anywhere in the codebase
rg "@tauri-apps/api" -nRepository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
# Look for tauri window references more broadly
rg "tauri.*window\|window.*tauri" -iRepository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
# Check the full TopBar.tsx context around the drag handler
cat interface/src/components/TopBar.tsxRepository: spacedriveapp/spacebot
Length of output: 3514
🌐 Web query:
Tauri v2 window dragging API public documentation
💡 Result:
- JavaScript API (Tauri v2):
getCurrentWindow().startDragging()is documented in thewindownamespace reference (method startDragging(): Promise). [1] - Guide (custom titlebar / drag regions): Shows using
data-tauri-drag-region, and a manual alternative callingappWindow.startDragging()onmousedown. [2] - Rust API:
tauri::window::Window::start_dragging(&self) -> Result<()>(desktop only). [3] - Tauri v2 permissions / capabilities: permission string
core:window:allow-start-dragging(enables thestart_draggingcommand). [4]
Sources: [1] (v2.tauri.app) [2] (v2.tauri.app) [3] (docs.rs) [4] (v2.tauri.org.cn)
Citations:
- 1: https://v2.tauri.app/ko/reference/javascript/api/namespacewindow/
- 2: https://v2.tauri.app/ko/learn/window-customization/
- 3: https://docs.rs/tauri/latest/tauri/window/struct.Window.html?utm_source=openai
- 4: https://v2.tauri.org.cn/reference/acl/core-permissions/
🌐 Web query:
@tauri-apps/api window module start_dragging
💡 Result:
In Tauri, the Rust API is window.start_dragging(), but the JavaScript/TypeScript API in @tauri-apps/api/window is startDragging() (camelCase).
Tauri v2 (recommended)
import { getCurrentWindow } from '@tauri-apps/api/window'
await getCurrentWindow().startDragging()This calls the window “startDragging()” API. [1]
You also need the permission in your capability (ACL), e.g.:
{
"permissions": ["core:window:default", "core:window:allow-start-dragging"]
}[2]
Tauri v1
import { appWindow } from '@tauri-apps/api/window'
await appWindow.startDragging()[3]
And enable it in tauri.conf.json allowlist (v1):
allowlist.window.startDragging: true[3]
Platform notes (v2 docs): Linux is listed as unsupported for startDragging(). [1]
Sources:
[1] Tauri v2 JS window API (startDragging) (v2.tauri.app)
[2] Tauri v2 window customization / permissions (core:window:allow-start-dragging) (v2.tauri.app)
[3] Tauri v1 JS window API (appWindow.startDragging) (v1.tauri.app)
Citations:
- 1: https://v2.tauri.app/ko/reference/javascript/api/namespacewindow/?utm_source=openai
- 2: https://v2.tauri.app/ja/learn/window-customization/?utm_source=openai
- 3: https://v1.tauri.app/v1/api/js/window?utm_source=openai
🌐 Web query:
Tauri window manager start_dragging public API alternative
💡 Result:
If you’re currently calling an internal WindowManager::start_dragging, the public alternatives are:
- JS (frontend):
appWindow.startDragging()(useful for custom titlebars / drag handles). (v1.tauri.app) - HTML (no JS needed): add
data-tauri-drag-regionto the element(s) you want to behave as a draggable region. (v2.tauri.app) - Rust (backend): call the window API
Window::start_dragging(&self) -> Result<()>. (docs.rs)
Also make sure the feature/permission for startDragging is enabled in your Tauri config/permissions (Tauri v2 docs show it under “Permissions” for window customization). (v2.tauri.app)
Citations:
- 1: https://v1.tauri.app/v1/api/js/window
- 2: https://v2.tauri.app/ko/learn/window-customization/
- 3: https://docs.rs/tauri/latest/tauri/window/struct.Window.html?utm_source=openai
- 4: https://v2.tauri.app/ko/learn/window-customization/
Replace with Tauri's public getCurrentWindow().startDragging() API.
The current code uses window.__TAURI_INTERNALS__.invoke("plugin:window|start_dragging"), which is a private, untyped API. Tauri v2 provides a public alternative: getCurrentWindow().startDragging() (requires importing from @tauri-apps/api/window). Additionally, the current invoke is not awaited, so errors are silently ignored—the promise should be awaited and errors handled appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/components/TopBar.tsx` around lines 78 - 86, Replace the
private Tauri invoke call in handleMouseDown with the public API: import
getCurrentWindow from "@tauri-apps/api/window" and call await
getCurrentWindow().startDragging() instead of (window as
any).__TAURI_INTERNALS__.invoke(...); keep the existing guards (IS_TAURI,
e.buttons check, interactive element check) but wrap the await call in try/catch
to handle and log errors (don’t swallow the promise) so startDragging errors are
surfaced.
| <Link | ||
| to="/" | ||
| className="flex w-14 shrink-0 items-center justify-center border-r border-sidebar-line bg-sidebar" | ||
| > | ||
| <img | ||
| src={`${BASE_PATH}/ball.png`} | ||
| alt="" | ||
| className="h-6 w-6 transition-transform duration-150 ease-out hover:scale-110 active:scale-95" | ||
| draggable={false} | ||
| /> | ||
| </Link> |
There was a problem hiding this comment.
Give the home link an accessible name.
The link only contains an image with alt="", so assistive tech lands on an unlabeled interactive control. Add an aria-label on the Link or visible text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/components/TopBar.tsx` around lines 99 - 109, The Link in
TopBar.tsx that renders the home button currently contains only a decorative img
(alt=""), so add an accessible name to the Link (the Link element itself) by
adding an aria-label (e.g., aria-label="Home") or include visually-hidden text
inside the Link; keep the image alt empty if it remains decorative and ensure
the attribute is added on the Link component used in the TopBar to provide a
proper accessible name for screen readers.
| <div className="flex items-center justify-between gap-2"> | ||
| <p className={cx("min-w-0 flex-1 truncate text-xs font-medium", selected ? "text-ink" : "text-ink-dull")}> | ||
| {worker.task.replace(/^\[opencode]\s*/, "")} | ||
| </p> | ||
| <div className="flex items-center gap-1.5"> | ||
| {isInteractive && ( | ||
| <div className="flex shrink-0 items-center gap-1.5 pointer-events-none"> | ||
| {worker.worker_type === "opencode" ? ( | ||
| <Badge variant="outline" size="sm"> | ||
| OpenCode | ||
| </Badge> | ||
| ) : isInteractive ? ( | ||
| <Badge variant="outline" size="sm"> | ||
| interactive | ||
| </Badge> | ||
| )} | ||
| ) : null} |
There was a problem hiding this comment.
Reuse the existing OpenCode fallback here.
After Line 322 strips the [opencode] prefix, rows that only have the legacy task prefix lose the only visible OpenCode signal because this badge now keys solely off worker.worker_type === "opencode". interface/src/hooks/useChannelLiveState.ts:21-39 already has isOpenCodeWorker() for this case; reusing it here will also keep the detail surface aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/AgentWorkers.tsx` around lines 320 - 333, Replace the
literal worker.worker_type === "opencode" check with the shared OpenCode
detection helper so rows that had only the legacy "[opencode]" prefix still show
the badge; import and use the isOpenCodeWorker helper (from useChannelLiveState)
or the hook-provided result when rendering the Badge in AgentWorkers.tsx (the
conditional rendering around Badge should use isOpenCodeWorker(worker) or the
hook value instead of worker.worker_type === "opencode") so the badge visibility
matches the canonical logic.
| {hasOpenCodeEmbed && ( | ||
| <div className="flex items-center gap-1 rounded-full border border-app-line/50 p-0.5"> | ||
| <button | ||
| onClick={() => setActiveTab("opencode")} | ||
| className={cx( | ||
| "rounded-full px-2 py-0.5 text-tiny font-medium transition-colors", | ||
| activeTab === "opencode" | ||
| ? "bg-app-hover/50 text-ink" | ||
| : "text-ink-faint hover:text-ink-dull", | ||
| )} | ||
| > | ||
| OpenCode | ||
| </button> | ||
| <button | ||
| onClick={() => setActiveTab("transcript")} | ||
| className={cx( | ||
| "rounded-full px-2 py-0.5 text-tiny font-medium transition-colors", | ||
| activeTab === "transcript" | ||
| ? "bg-app-hover/50 text-ink" | ||
| : "text-ink-faint hover:text-ink-dull", | ||
| )} | ||
| > | ||
| Transcript | ||
| </button> |
There was a problem hiding this comment.
Expose the selected state on this OpenCode/Transcript toggle.
These buttons behave like a segmented control, but there is no aria-pressed or aria-selected signal for the active view. Keyboard users can still activate them, but assistive tech cannot tell which panel is currently shown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/AgentWorkers.tsx` around lines 487 - 510, The segmented
control for OpenCode/Transcript (rendered when hasOpenCodeEmbed) currently
toggles via setActiveTab and uses activeTab for styling but lacks accessibility
state; update the two buttons (the ones that call setActiveTab("opencode") and
setActiveTab("transcript")) to expose the selected state by adding appropriate
ARIA attributes (e.g., aria-pressed or role="tab" with aria-selected) tied to
activeTab (e.g., aria-pressed={activeTab === "opencode"} for the OpenCode button
and similarly for Transcript), and optionally add aria-controls pointing to the
corresponding panel IDs so assistive tech can determine which view is active.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/agent/channel.rs (1)
165-172:⚠️ Potential issue | 🟠 MajorAbort/drain ordering can still lose the tail of a cancelled transcript.
Line 165 removes the cache entry before Line 175 aborts the worker. Any
ToolStarted/ToolCompletedevent already emitted by the worker but not yet applied tolive_worker_transcriptsgets dropped, so the persisted snapshot can be truncated. This needs a real flush/ack from the transcript producer or a worker-owned transcript handoff instead of an optimisticremove()here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 165 - 172, Currently you remove the entry from live_worker_transcripts via remove(&worker_id.to_string()) before aborting the worker, which can drop in-flight ToolStarted/ToolCompleted events; instead implement a coordinated transcript handoff/flush: have the abort path request a flush/ack from the worker (or call a worker-owned method to transfer its transcript) and only remove the live entry after receiving that acknowledgement, or atomically swap the transcript ownership to the aborting controller (e.g., via a oneshot/handshake) and then call the abort; update the code around live_worker_transcripts.remove(...) and the worker abort call so the removal happens after a confirmed handoff/flush rather than optimistically before aborting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/agent/channel.rs`:
- Around line 165-172: Currently you remove the entry from
live_worker_transcripts via remove(&worker_id.to_string()) before aborting the
worker, which can drop in-flight ToolStarted/ToolCompleted events; instead
implement a coordinated transcript handoff/flush: have the abort path request a
flush/ack from the worker (or call a worker-owned method to transfer its
transcript) and only remove the live entry after receiving that acknowledgement,
or atomically swap the transcript ownership to the aborting controller (e.g.,
via a oneshot/handshake) and then call the abort; update the code around
live_worker_transcripts.remove(...) and the worker abort call so the removal
happens after a confirmed handoff/flush rather than optimistically before
aborting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6aa0b33-76d9-4a72-89ab-9013096b5708
📒 Files selected for processing (3)
src/agent/channel.rssrc/main.rstests/context_dump.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/send_message_to_another_channel.rs (1)
187-212:⚠️ Potential issue | 🟠 MajorReturn a structured validation error for malformed implicit Signal shorthands.
parse_implicit_signal_shorthandinsrc/messaging/target.rs:333-372returnsOption, so invalid Signal-like inputs and “not a Signal shorthand” both fall through to the generic channel lookup error here. That makes the tool reportno channel foundinstead of something recoverable likeinvalid Signal target format. Please distinguish those cases in this branch so the model can correct the target. As per coding guidelines, Tool errors are returned as structured results, not panics. The LLM sees the error and can recover (error-as-result for tools pattern).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/send_message_to_another_channel.rs` around lines 187 - 212, Change parse_implicit_signal_shorthand so callers can tell "not a Signal shorthand" from "malformed Signal shorthand": make parse_implicit_signal_shorthand return Result<Option<SignalTarget>, ParseSignalError> (or equivalent) instead of Option, update this branch to match on the Result—on Ok(Some(target)) proceed with messaging_manager.broadcast and return SendMessageOutput as before; on Ok(None) fall through to the generic channel lookup logic; on Err(parse_err) return a structured tool error via SendMessageError containing a clear validation message like "invalid Signal target format" along with parse_err details so the LLM can recover. Ensure references: parse_implicit_signal_shorthand, current_adapter, messaging_manager.broadcast, SendMessageError, and SendMessageOutput are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/tools/send_message_to_another_channel.rs`:
- Around line 187-212: Change parse_implicit_signal_shorthand so callers can
tell "not a Signal shorthand" from "malformed Signal shorthand": make
parse_implicit_signal_shorthand return Result<Option<SignalTarget>,
ParseSignalError> (or equivalent) instead of Option, update this branch to match
on the Result—on Ok(Some(target)) proceed with messaging_manager.broadcast and
return SendMessageOutput as before; on Ok(None) fall through to the generic
channel lookup logic; on Err(parse_err) return a structured tool error via
SendMessageError containing a clear validation message like "invalid Signal
target format" along with parse_err details so the LLM can recover. Ensure
references: parse_implicit_signal_shorthand, current_adapter,
messaging_manager.broadcast, SendMessageError, and SendMessageOutput are updated
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4005c3eb-b841-4be2-aeb9-3846409ae36c
📒 Files selected for processing (2)
interface/src/routes/AgentWorkers.tsxsrc/tools/send_message_to_another_channel.rs
… handle, signal validation - Swap abort/drain order in cancel_worker_with_reason so the worker is stopped before draining live_worker_transcripts, preventing late events from being lost. - Pass shared live_worker_transcripts Arc into Channel::new so the ChannelControlHandle (cloned during construction) references the same backing map as ApiState. Previously the control handle captured the standalone empty map, causing cortex-driven cancellation to miss transcript data. - Change parse_implicit_signal_shorthand to return Result<Option<…>, String> so malformed shorthands (bad UUID, short phone, empty group) surface structured validation errors instead of falling through to a generic 'no channel found' message.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/agent/channel.rs (1)
165-179:⚠️ Potential issue | 🟠 MajorAbort-before-drain still doesn't synchronize with the transcript writer.
src/api/state.rsappendsToolStarted/ToolCompletedinto this map from a separate async event-loop task. Aborting the worker stops future emissions, but it does not guarantee that already-broadcast events have been applied beforeremove(&worker_id.to_string())runs, so cancelled workers can still persist truncated transcripts intermittently. This still needs a flush/barrier with the event consumer, or a worker-owned transcript source, before the entry is removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 165 - 179, Aborting the worker then immediately calling self.live_worker_transcripts.write().await.remove(&worker_id.to_string()) can race with the async event-loop that appends ToolStarted/ToolCompleted; add a synchronization barrier so removal only happens after the event consumer has applied all pending events for that worker. Concretely: introduce and await a per-worker flush/confirmation (e.g., a Notify/oneshot or a new async method like wait_for_transcript_flush(worker_id) on the state/event-consumer) before calling remove; alternatively move transcript ownership so the worker calls persist_transcript() (the existing persist_transcript()/run() path) or publish a completion confirmation from the state append task and await it here prior to remove.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/agent/channel.rs`:
- Around line 165-179: Aborting the worker then immediately calling
self.live_worker_transcripts.write().await.remove(&worker_id.to_string()) can
race with the async event-loop that appends ToolStarted/ToolCompleted; add a
synchronization barrier so removal only happens after the event consumer has
applied all pending events for that worker. Concretely: introduce and await a
per-worker flush/confirmation (e.g., a Notify/oneshot or a new async method like
wait_for_transcript_flush(worker_id) on the state/event-consumer) before calling
remove; alternatively move transcript ownership so the worker calls
persist_transcript() (the existing persist_transcript()/run() path) or publish a
completion confirmation from the state append task and await it here prior to
remove.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1671b4c5-526f-41d8-b308-66be3c655f81
📒 Files selected for processing (4)
src/agent/channel.rssrc/cron/scheduler.rssrc/main.rssrc/tools/send_message_to_another_channel.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.rs
# Conflicts: # src/tools/send_message_to_another_channel.rs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/agent/channel.rs (1)
165-179:⚠️ Potential issue | 🟠 MajorAbort does not flush the API-side transcript cache.
Aborting the worker stops new work, but it does not guarantee every already-emitted
ToolStarted/ToolCompletedevent has been applied tolive_worker_transcriptsbefore Line 175 removes the entry. A queued late event can still land after this drain, leaving the persisted snapshot incomplete and the cache repopulated with orphaned steps. This needs a real flush/ack barrier or a worker-owned transcript source beforeremove().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 165 - 179, The current abort+remove sequence can drop in-flight ToolStarted/ToolCompleted events because handle.abort() does not guarantee the worker has flushed its transcript to live_worker_transcripts before remove(&worker_id.to_string()) runs; change this to use an explicit flush/ack protocol: have the worker expose a flush_transcript() or send_final_transcript() message (from run()/persist_transcript() path) and await that ACK on the controller side before calling remove(), or alternatively make the worker own its transcript and send a final snapshot over a oneshot/mpsc channel which the controller awaits; update the abort path to send the flush request, await the worker ACK (or channel response) and only then remove the entry from live_worker_transcripts to prevent orphaned steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agent/channel.rs`:
- Around line 213-229: The update inside the tokio::spawn uses sqlx::query to
update worker_runs but only checks for SQL errors; after .execute(&pool).await
capture the successful result and inspect result.rows_affected() and handle the
zero-row case explicitly (e.g., log a warning/error with worker_id_str,
transcript_blob summary and tool_calls, or perform an alternate insert/backup)
so cancelled transcripts aren't silently dropped; update the block around the
sqlx::query/.bind(&transcript_blob).bind(tool_calls).bind(&worker_id_str).execute(&pool)
to check rows_affected() and act accordingly.
---
Duplicate comments:
In `@src/agent/channel.rs`:
- Around line 165-179: The current abort+remove sequence can drop in-flight
ToolStarted/ToolCompleted events because handle.abort() does not guarantee the
worker has flushed its transcript to live_worker_transcripts before
remove(&worker_id.to_string()) runs; change this to use an explicit flush/ack
protocol: have the worker expose a flush_transcript() or send_final_transcript()
message (from run()/persist_transcript() path) and await that ACK on the
controller side before calling remove(), or alternatively make the worker own
its transcript and send a final snapshot over a oneshot/mpsc channel which the
controller awaits; update the abort path to send the flush request, await the
worker ACK (or channel response) and only then remove the entry from
live_worker_transcripts to prevent orphaned steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef0f0f59-b7b3-4261-84bf-5b77a16acaae
📒 Files selected for processing (6)
interface/src/api/client.tsinterface/src/routes/Settings.tsxsrc/agent/channel.rssrc/cron/scheduler.rssrc/main.rstests/context_dump.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- interface/src/routes/Settings.tsx
- src/cron/scheduler.rs
- src/main.rs
| tokio::spawn(async move { | ||
| if let Err(error) = sqlx::query( | ||
| "UPDATE worker_runs SET transcript = ?, tool_calls = ? WHERE id = ? AND transcript IS NULL", | ||
| ) | ||
| .bind(&transcript_blob) | ||
| .bind(tool_calls) | ||
| .bind(&worker_id_str) | ||
| .execute(&pool) | ||
| .await | ||
| { | ||
| tracing::warn!( | ||
| %error, | ||
| worker_id = %worker_id_str, | ||
| "failed to persist cancelled worker transcript" | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Handle zero-row transcript updates explicitly.
This only warns on SQL errors. If cancellation wins the race with the WorkerStarted bookkeeping, worker_runs may not exist yet, execute() succeeds with rows_affected() == 0, and the cancelled transcript/tool-call snapshot is dropped silently.
Possible fix
tokio::spawn(async move {
- if let Err(error) = sqlx::query(
- "UPDATE worker_runs SET transcript = ?, tool_calls = ? WHERE id = ? AND transcript IS NULL",
- )
- .bind(&transcript_blob)
- .bind(tool_calls)
- .bind(&worker_id_str)
- .execute(&pool)
- .await
- {
- tracing::warn!(
- %error,
- worker_id = %worker_id_str,
- "failed to persist cancelled worker transcript"
- );
- }
+ match sqlx::query(
+ "UPDATE worker_runs SET transcript = ?, tool_calls = ? WHERE id = ? AND transcript IS NULL",
+ )
+ .bind(&transcript_blob)
+ .bind(tool_calls)
+ .bind(&worker_id_str)
+ .execute(&pool)
+ .await
+ {
+ Ok(result) if result.rows_affected() == 0 => {
+ tracing::warn!(
+ worker_id = %worker_id_str,
+ "cancelled worker transcript was not persisted because no worker_runs row was updated"
+ );
+ }
+ Ok(_) => {}
+ Err(error) => {
+ tracing::warn!(
+ %error,
+ worker_id = %worker_id_str,
+ "failed to persist cancelled worker transcript"
+ );
+ }
+ }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agent/channel.rs` around lines 213 - 229, The update inside the
tokio::spawn uses sqlx::query to update worker_runs but only checks for SQL
errors; after .execute(&pool).await capture the successful result and inspect
result.rows_affected() and handle the zero-row case explicitly (e.g., log a
warning/error with worker_id_str, transcript_blob summary and tool_calls, or
perform an alternate insert/backup) so cancelled transcripts aren't silently
dropped; update the block around the
sqlx::query/.bind(&transcript_blob).bind(tool_calls).bind(&worker_id_str).execute(&pool)
to check rows_affected() and act accordingly.
1 participant
Summary
desktop/) with macOS native window chrome[opencode]prefix, OpenCode badge, neutral gray styling throughoutdesktop/.gitignorefornode_modules/andtarget/Note
This PR introduces a complete Tauri 2 desktop wrapper with macOS native window integration and significant UI polish for OpenCode embedding. The
desktop/directory contains the full Tauri configuration (47 new files including assets, Swift-based macOS window chrome, and Tauri source code). Key interface changes modernize the worker list display and seamlessly integrate the embedded OpenCode experience by removing visual chrome and moving controls into the header. Updates across interface routing and styling ensure smooth integration with the desktop wrapper.Generated for commit 5509418.