fix(daemon): expose agent tools, fix scheduled tasks + MCP scope - #45
Conversation
- Wire orchestration + Pi built-in tools into daemon tools.listDefinitions route (previously only MCP tools were returned, leaving the Argos agent tools section empty) - Add piToolCatalog exposing read/bash/edit/write/grep/find/ls with real descriptions and parameter schemas - Redesign ArgosAgentsSettings Tools section: collapsible group cards with per-tool descriptions, gating for orchestration tools - Fix MCP scope panel reading from wrong store: AgentExtensionPolicyPanel now uses daemon mcp.getServers route instead of legacy desktop IPC presenter (servers added via MCP settings were invisible) - Fix scheduled task action type select reverting to notify: commitTask captured stale tasks closure, persisting the pre-update task; now passes the freshly-built task as override - Fix daily/weekly tasks firing in an infinite loop: computeNextFireAt ignored lastFiredAt for recurring triggers, so the 60s drift tolerance caused same-slot re-fire after each dispatch - Fix daemon prompt-draft crash: DaemonScheduledTasksWindowPresenter had a fake mainWindow that passed truthy checks but threw on sendToWindow; now null so draft falls through to notification - Add error logging to daemon route dispatch catch and all ScheduledTasksSettings catch blocks - Fix hooksNotifications config warning on undefined store value
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds Pi and orchestration tool discovery, updates agent tool controls, synchronizes Windows title-bar overlays with themes, centralizes scheduled-task logic, improves task persistence, adds ACP update notifications, and updates desktop shell interactions and styling. ChangesTool discovery and agent tool settings
Desktop window presentation
Scheduled task correctness
Desktop UI shell
ACP update notifications
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DaemonStartup
participant DaemonDispatcher
participant PiToolCatalog
participant OrchestrationRuntime
DaemonStartup->>DaemonDispatcher: pass orchestrationRuntime
DaemonDispatcher->>PiToolCatalog: getPiToolDefinitions()
DaemonDispatcher->>OrchestrationRuntime: definitions()
DaemonDispatcher-->>DaemonStartup: return combined tool definitions
sequenceDiagram
participant MainLayout
participant ACPNotifications
participant ACPConfig
participant AgentRegistry
participant Sonner
MainLayout->>ACPNotifications: start update checks
ACPNotifications->>ACPConfig: verify ACP is enabled
ACPNotifications->>AgentRegistry: compare installed agent versions
AgentRegistry-->>ACPNotifications: return changed versions
ACPNotifications->>Sonner: show update toast with settings action
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Confidence Score: 4/5The recurring-task fix should be completed in the desktop scheduler before merging because desktop daily and weekly tasks remain able to refire the same occurrence. The daemon scheduler now advances recurring tasks using lastFiredAt, but the desktop runtime imports a separate unchanged calculation while retaining the same drift-tolerant scheduling behavior, leaving the reported infinite-refire failure reachable on desktop. Files Needing Attention: packages/backend-core/src/scheduled/normalize.ts and apps/desktop/src/main/presenter/scheduledTasks/normalize.ts
|
| Filename | Overview |
|---|---|
| packages/backend-core/src/scheduled/normalize.ts | Adds a lastFiredAt floor for daemon recurrence calculations, but the equivalent active desktop implementation remains unchanged. |
| apps/desktop/src/main/presenter/scheduledTasks/normalize.ts | Important unchanged sibling implementation still permits the recurring-task refire behavior that this PR intends to fix. |
| packages/ui/settings/components/ScheduledTasksSettings.tsx | Passes freshly constructed tasks to synchronous persistence sites and improves operation error reporting. |
| apps/daemon/src/dispatch/daemonDispatcher.ts | Extends daemon tool-definition responses with Pi and orchestration catalogs. |
| apps/daemon/src/host/piToolCatalog.ts | Defines UI-facing metadata and schemas for Pi built-in tools. |
| packages/ui/settings/components/AgentExtensionPolicyPanel.tsx | Loads MCP servers from the daemon-backed client and preserves undefined as allow-all scope. |
| apps/desktop/src/main/presenter/windowPresenter/index.ts | Adds Windows native window-controls overlay creation and theme synchronization. |
Prompt To Fix All With AI
### Issue 1
packages/backend-core/src/scheduled/normalize.ts:170-173
**Desktop recurrence remains unfixed**
When a daily or weekly task fires in the desktop application, its scheduler still uses the separate unchanged `apps/desktop` recurrence calculation, so the 60-second drift window can select the consumed occurrence again and repeatedly create notifications or prompt sessions. Apply the `lastFiredAt` fix to the desktop implementation as well, or consolidate both runtimes onto one implementation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(daemon): expose agent tools, fix sch..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const effectiveAfter = task.lastFiredAt != null ? Math.max(after, task.lastFiredAt) : after; | ||
| let candidate = buildWallClockToday(effectiveAfter, trigger.hour, trigger.minute, 0); | ||
| if (candidate <= effectiveAfter) { | ||
| candidate = buildWallClockToday(effectiveAfter, trigger.hour, trigger.minute, 1); |
There was a problem hiding this comment.
When a daily or weekly task fires in the desktop application, its scheduler still uses the separate unchanged apps/desktop recurrence calculation, so the 60-second drift window can select the consumed occurrence again and repeatedly create notifications or prompt sessions. Apply the lastFiredAt fix to the desktop implementation as well, or consolidate both runtimes onto one implementation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/backend-core/src/scheduled/normalize.ts
Line: 170-173
Comment:
**Desktop recurrence remains unfixed**
When a daily or weekly task fires in the desktop application, its scheduler still uses the separate unchanged `apps/desktop` recurrence calculation, so the 60-second drift window can select the consumed occurrence again and repeatedly create notifications or prompt sessions. Apply the `lastFiredAt` fix to the desktop implementation as well, or consolidate both runtimes onto one implementation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple configuration and runtime issues spanning agent settings (tool visibility + MCP scope), scheduled tasks (UI persistence + next-fire computation), and Windows desktop window chrome (native titlebar overlay syncing). It aligns the UI with the daemon-backed stores that actually enforce agent/session behavior and improves observability via better error logging.
Changes:
- Expose first-party agent tool definitions (Pi + orchestration) via the daemon tools route and update the agent tools UI to show grouped, descriptive tool cards with orchestration gating.
- Fix MCP scope panel to load MCP servers from the daemon store and improve selection handling semantics.
- Fix scheduled task persistence (stale closure) and prevent daily/weekly triggers from refiring in a loop by respecting
lastFiredAt; plus Windows titlebar overlay behavior + logging/toast enhancements.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/routes/_main.tsx | Hooks ACP agent-update notifications into the main layout; aligns main surface styling with sidebar tone. |
| packages/ui/src/composables/useAcpAgentUpdateNotifications.ts | Adds daemon-backed ACP registry update checks with toast notifications and settings deep-link action. |
| packages/ui/src/components/WindowSideBarSessionItem.tsx | Reworks session item interactions to include context/dropdown menus for actions. |
| packages/ui/src/components/WindowSideBar.tsx | Updates sidebar styling and icon set; removes connection indicator from sidebar. |
| packages/ui/src/components/use-toast.ts | Extends toast wrapper to support Sonner actions (label + callback). |
| packages/ui/src/components/sidepanel/ChatSidePanel.tsx | Aligns side panel surface color with the sidebar theme token. |
| packages/ui/src/components/chat-input/McpIndicator.tsx | Adds new tool group ordering/labels for orchestration and Pi. |
| packages/ui/src/components/AppBar.tsx | Adds Windows detection and hides custom window buttons when native overlay is used; updates styling. |
| packages/ui/src/assets/style.css | Adds custom Sonner toast “dropdown-glass” styling. |
| packages/ui/settings/components/ScheduledTasksSettings.tsx | Fixes stale-closure persistence by using a ref + override-based commit; improves error logging/toasts. |
| packages/ui/settings/components/ArgosAgentsSettings.tsx | Adds orchestration/Pi tool groups and redesigns the Tools section into collapsible, described group cards with gating. |
| packages/ui/settings/components/AgentExtensionPolicyPanel.tsx | Switches MCP server fetching to daemon client and updates selection semantics/UX. |
| packages/shared/src/types/presenters/legacy.presenters.d.ts | Extends window presenter typing with syncWindowTitleBarAppearance(). |
| packages/backend-core/src/scheduled/normalize.ts | Prevents daily/weekly scheduled tasks from looping by flooring with lastFiredAt. |
| apps/desktop/src/main/presenter/windowPresenter/index.ts | Implements Windows Window Controls Overlay (WCO) setup and a resync method for theme changes. |
| apps/desktop/src/main/presenter/hooksNotifications/config.ts | Fixes undefined/null config normalization by returning defaults early. |
| apps/desktop/src/main/presenter/configPresenter/index.ts | Reapplies Windows titlebar overlay options when theme changes. |
| apps/daemon/test/daemonToolDefinitions.test.ts | Updates/extends tests to validate orchestration + Pi tool definitions are included. |
| apps/daemon/src/transport/http.ts | Adds route-name + error logging on daemon route dispatch failures. |
| apps/daemon/src/index.ts | Passes orchestration runtime into the daemon dispatcher wiring. |
| apps/daemon/src/host/piToolCatalog.ts | Introduces Pi tool catalog definitions for UI/tool discovery. |
| apps/daemon/src/host/daemonScheduledTasks.ts | Fixes headless scheduled-task presenter by making mainWindow null to avoid draft-path crashes. |
| apps/daemon/src/dispatch/daemonDispatcher.ts | Appends Pi + orchestration definitions to tool definition listing route output. |
Suppressed comments (1)
apps/daemon/src/host/piToolCatalog.ts:38
- The
edittool claimsedits[].oldText/edits[].newTextare required, but the JSON schema foredits.itemsis just{ type: "object" }with noproperties/required. This makes the tool definition internally inconsistent and prevents consumers from reliably constructing valid edit calls.
edits: {
type: "array",
items: { type: "object", description: "A single replacement: oldText to find and newText to replace it with." },
description: "The edits to apply.",
},
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type CatalogEntry = { | ||
| description: string; | ||
| properties: Record<string, { type: string; items?: { type: string; description?: string }; description: string }>; | ||
| required?: string[]; | ||
| }; |
| <p className="text-xs text-muted-foreground"> | ||
| Leave this unset to allow every configured MCP server. An empty list blocks MCP tools entirely. | ||
| Checked servers are available to this agent. Uncheck when everything is allowed to create an explicit | ||
| blocklist. | ||
| </p> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
apps/daemon/test/daemonToolDefinitions.test.ts (1)
59-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every Pi tool definition.
The test checks only the first Pi tool. A regression in
bash,edit,write,grep,find, orlswill pass. Assert the complete Pi tool-name set and the required schema fields for each tool.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/daemon/test/daemonToolDefinitions.test.ts` around lines 59 - 113, Expand the assertions in the toolsListDefinitionsRoute test around the piTool lookup to validate every expected Pi tool: read, bash, edit, write, grep, find, and ls. Assert each definition has source "agent" and includes the required function schema fields, including the tool name and its parameters definition, rather than checking only the read tool.packages/backend-core/src/scheduled/normalize.ts (1)
170-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for the recurrence boundary.
Cover
lastFiredAtvalues before, equal to, and after the daily or weekly occurrence. Also cover both earlier and lateraftervalues. Assert that each returned candidate is later than the effective boundary.As per coding guidelines, tests must use Vitest and filenames must end in
.test.ts,.test.tsx, or.spec.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend-core/src/scheduled/normalize.ts` around lines 170 - 185, Add Vitest regression tests in a .test.ts, .test.tsx, or .spec.ts file for the daily and weekly recurrence logic around the normalization function. Cover lastFiredAt before, equal to, and after the scheduled occurrence, combined with both earlier and later after values, and assert every returned candidate is strictly later than the effective boundary.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/presenter/windowPresenter/index.ts`:
- Around line 53-61: Update getTitleBarOverlayOptions() to preserve native
caption-button contrast when nativeTheme.inForcedColorsMode is true. In that
mode, omit symbolColor from the returned TitleBarOverlayOptions; retain the
existing explicit dark/light symbol colors for normal Windows themes.
- Around line 44-63: Update the Windows title-bar overlay layout associated with
getTitleBarOverlayOptions so the AppBar’s drag/content region uses the
titlebar-area-width CSS environment value instead of assuming full width;
preserve the existing overlay colors, height, and non-Windows undefined
behavior.
In `@packages/shared/src/types/presenters/legacy.presenters.d.ts`:
- Line 300: Update the IWindowPresenter partial mock in eventbus.test.ts to
include a syncWindowTitleBarAppearance() method matching the required shared
presenter contract, while preserving the existing mock behavior.
In `@packages/ui/settings/components/AgentExtensionPolicyPanel.tsx`:
- Around line 172-173: Update the explanatory text in AgentExtensionPolicyPanel
to describe enabledMcpServerIds as an explicit allowlist, replacing “explicit
blocklist” with “explicit allowlist” while preserving the existing unchecked and
newly added server behavior.
In `@packages/ui/settings/components/ArgosAgentsSettings.tsx`:
- Around line 1015-1017: Remove the redundant sr-only labels in
ArgosAgentsSettings.tsx: delete the hidden “Unnamed Agent” span near lines
1015-1017 and the hidden Vision model and Image generation model spans near
lines 1217-1223, while preserving the visible labels.
In `@packages/ui/settings/components/ScheduledTasksSettings.tsx`:
- Around line 148-153: The task mutation callback around persistTask currently
allows concurrent writes and out-of-order responses to overwrite newer settings.
Serialize mutations for each task before invoking persistTask, or add
revision/conditional-write protection so responses are applied only when
current; preserve the settingsRef lookup while ensuring every field-blur and
select-change snapshot is ordered.
In `@packages/ui/src/components/AppBar.tsx`:
- Line 21: Track whether platform detection has completed in the AppBar
component and gate the custom-controls rendering near lines 75-77 on that
readiness state. Keep controls hidden until getDeviceInfo() resolves, then
render the appropriate Windows/macOS controls without changing the detected
platform behavior.
In `@packages/ui/src/composables/useAcpAgentUpdateNotifications.ts`:
- Around line 22-24: Update checkForAgentUpdates so requests received while
checkInFlight is true set a pending-check flag instead of being discarded. In
the function’s finally path, clear the active state and run exactly one
follow-up check when that flag is set, preserving the existing behavior for
requests that arrive when no check is active.
---
Nitpick comments:
In `@apps/daemon/test/daemonToolDefinitions.test.ts`:
- Around line 59-113: Expand the assertions in the toolsListDefinitionsRoute
test around the piTool lookup to validate every expected Pi tool: read, bash,
edit, write, grep, find, and ls. Assert each definition has source "agent" and
includes the required function schema fields, including the tool name and its
parameters definition, rather than checking only the read tool.
In `@packages/backend-core/src/scheduled/normalize.ts`:
- Around line 170-185: Add Vitest regression tests in a .test.ts, .test.tsx, or
.spec.ts file for the daily and weekly recurrence logic around the normalization
function. Cover lastFiredAt before, equal to, and after the scheduled
occurrence, combined with both earlier and later after values, and assert every
returned candidate is strictly later than the effective boundary.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 98b36193-baba-4f26-af88-7f09dd88445b
📒 Files selected for processing (23)
apps/daemon/src/dispatch/daemonDispatcher.tsapps/daemon/src/host/daemonScheduledTasks.tsapps/daemon/src/host/piToolCatalog.tsapps/daemon/src/index.tsapps/daemon/src/transport/http.tsapps/daemon/test/daemonToolDefinitions.test.tsapps/desktop/src/main/presenter/configPresenter/index.tsapps/desktop/src/main/presenter/hooksNotifications/config.tsapps/desktop/src/main/presenter/windowPresenter/index.tspackages/backend-core/src/scheduled/normalize.tspackages/shared/src/types/presenters/legacy.presenters.d.tspackages/ui/settings/components/AgentExtensionPolicyPanel.tsxpackages/ui/settings/components/ArgosAgentsSettings.tsxpackages/ui/settings/components/ScheduledTasksSettings.tsxpackages/ui/src/assets/style.csspackages/ui/src/components/AppBar.tsxpackages/ui/src/components/WindowSideBar.tsxpackages/ui/src/components/WindowSideBarSessionItem.tsxpackages/ui/src/components/chat-input/McpIndicator.tsxpackages/ui/src/components/sidepanel/ChatSidePanel.tsxpackages/ui/src/components/use-toast.tspackages/ui/src/composables/useAcpAgentUpdateNotifications.tspackages/ui/src/routes/_main.tsx
| // Window Controls Overlay (WCO): on Windows the native caption buttons are drawn by | ||
| // Chromium into the top-right of the web contents. The overlay height matches the AppBar | ||
| // (h-9 = 36px); the overlay color is effectively transparent so the buttons float directly | ||
| // on the sidebar-toned AppBar surface. Symbol colors mirror the sidebar-foreground token. | ||
| const TITLEBAR_OVERLAY_HEIGHT = 36; | ||
| const TITLEBAR_OVERLAY_COLOR = "#01000000"; // "#00000000" renders black on some platforms | ||
| const TITLEBAR_OVERLAY_SYMBOL_LIGHT = "#43434c"; // sidebar foreground (light) oklch(0.38 0 0) | ||
| const TITLEBAR_OVERLAY_SYMBOL_DARK = "#b9b9c0"; // sidebar foreground (dark) oklch(0.78 0 0) | ||
|
|
||
| function getTitleBarOverlayOptions(): Electron.TitleBarOverlayOptions | undefined { | ||
| if (process.platform !== "win32") { | ||
| return undefined; | ||
| } | ||
| return { | ||
| color: TITLEBAR_OVERLAY_COLOR, | ||
| height: TITLEBAR_OVERLAY_HEIGHT, | ||
| symbolColor: nativeTheme.shouldUseDarkColors ? TITLEBAR_OVERLAY_SYMBOL_DARK : TITLEBAR_OVERLAY_SYMBOL_LIGHT, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 'titlebar-area-(x|y|width|height)|-webkit-app-region|app-region|AppBar' packages/ui apps/desktopRepository: dvaJi/argos
Length of output: 6559
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== AppBar outline =="
ast-grep outline packages/ui/src/components/AppBar.tsx --view expanded || true
echo "== AppBar excerpt =="
sed -n '1,260p' packages/ui/src/components/AppBar.tsx
echo "== _main AppBar layout excerpt =="
sed -n '480,535p' packages/ui/src/routes/_main.tsx
echo "== focus on titlebar-area / WCO env variables and custom caption button classes =="
rg -n -C 4 '--(titlebar-area-|-webkit-app-region|-apple|webkit-app|app-region)|titlebar-area|AppBar|window-drag-region|window-no-drag-region|no-drag' packages/ui apps/desktopRepository: dvaJi/argos
Length of output: 7593
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== AppBar styles/classes in packages/ui/src/assets/style.css =="
sed -n '880,955p' packages/ui/src/assets/style.css
echo "== AppBar styles/classes in package css files excluding asset =="
rg -n -C 3 'window-drag-region|window-no-drag-region|titlebar-area|app-region|webkit-app-region' packages/ui -g '*.css' -g '*.scss'
echo "== TypeScript definitions/usages for titlebar-area =="
rg -n -C 3 'titlebar-area-(x|y|width|height)' .Repository: dvaJi/argos
Length of output: 2141
🌐 Web query:
Electron titleBarOverlay titlebar-area-width titlebar-area-x CSS environment variables drag region default area
💡 Result:
In Electron, the titleBarOverlay feature enables the Window Controls Overlay API, which allows developers to customize the title bar area while maintaining native window control buttons [1][2]. When this feature is enabled, you must use CSS environment variables to ensure your custom content does not overlap with the native window controls [1][2][3]. CSS Environment Variables The following environment variables are available to define the dimensions and position of the title bar area: - env(titlebar-area-x, 0px): The horizontal offset of the available title bar area [1][3]. - env(titlebar-area-y, 0px): The vertical offset of the available title bar area [4][3]. - env(titlebar-area-width, 100%): The width of the available title bar area [1][3]. - env(titlebar-area-height, 30px): The height of the available title bar area [4][3]. These variables help you position your custom title bar container so it stays adjacent to the native window controls [3][5]. A typical implementation looks like this: #title-bar { position: fixed; left: env(titlebar-area-x, 0); top: env(titlebar-area-y, 0); height: env(titlebar-area-height, 50px); width: env(titlebar-area-width, 100%); } Drag Regions To make your custom title bar draggable, you apply the -webkit-app-region: drag CSS property [6][7]. When using titleBarOverlay, the native window controls (close, minimize, maximize) are automatically non-draggable [1][2]. You must ensure that any interactive elements within your custom title bar (such as buttons) are explicitly set to -webkit-app-region: no-drag [6]. Important Considerations: - Default Area: When titleBarOverlay is enabled, the area occupied by the native buttons is reserved by the OS [1][2]. Your CSS content should not be placed in this reserved region, which is why the environment variables are used to provide the safe "available" width and position [1][3]. - Compatibility: Ensure you are using a modern version of Electron (v18.0.0 or later recommended), as earlier versions had issues where titleBarOverlay could interfere with drag regions on Windows [8][7]. - Integration: This API is based on the web standard Window Controls Overlay API, meaning it behaves consistently with how it is implemented in Progressive Web Apps (PWAs) [1][9][4].
Citations:
- 1: https://electronjs.org/docs/latest/tutorial/custom-title-bar
- 2: https://github.com/electron/electron/blob/main/docs/tutorial/custom-title-bar.md
- 3: https://learn.microsoft.com/en-us/microsoft-edge/progressive-web-apps/how-to/window-controls-overlay
- 4: https://github.com/WICG/window-controls-overlay/blob/main/explainer.md
- 5: https://github.com/MicrosoftDocs/edge-developer/blob/main/microsoft-edge/progressive-web-apps/how-to/window-controls-overlay.md
- 6: https://electronjs.org/docs/latest/tutorial/custom-window-interactions
- 7: https://stackoverflow.com/questions/71614490/electron-window-dragging-incompatible-with-titlebaroverlay
- 8: [Bug]:
titleBarOverlayprevent dragging theBrowserWindowelectron/electron#32966 - 9: https://developer.mozilla.org/en-US/docs/Web/API/Window_Controls_Overlay_API
🌐 Web query:
WICG window-controls-overlay titlebar-area-x default CSS environment variables width titlebar-area-width titlebar-area-x 0px
💡 Result:
The CSS environment variables titlebar-area-x, titlebar-area-y, titlebar-area-width, and titlebar-area-height are part of the Window Controls Overlay API, which allows Progressive Web Apps (PWAs) to customize their title bar area [1][2][3]. These variables define the rectangular region that would typically be occupied by the browser's title bar and window control buttons (minimize, maximize, and close) [4][5]. Because these variables are only defined when the window-controls-overlay display mode is active, it is standard practice to provide fallback values within the CSS env() function for environments where the feature is unsupported or disabled [4][6][5]. Commonly used fallback values for these variables include: - titlebar-area-x: 0px [1][4] - titlebar-area-y: 0px [4][6] - titlebar-area-width: 100% [1][4] - titlebar-area-height: A static pixel value (e.g., 33px or 50px), representing the height of a standard title bar [4][6] Example usage in CSS: #title-bar { position: fixed; left: env(titlebar-area-x, 0px); top: env(titlebar-area-y, 0px); width: env(titlebar-area-width, 100%); height: env(titlebar-area-height, 50px); } Using these variables ensures that your custom content correctly aligns with the system's window controls when the overlay is active, while providing a reasonable layout fallback when it is not [4][5].
Citations:
- 1: https://github.com/WICG/window-controls-overlay/blob/main/explainer.md
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Window_Controls_Overlay_API
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Environment_variables/Using
- 4: https://learn.microsoft.com/en-us/microsoft-edge/progressive-web-apps/how-to/window-controls-overlay
- 5: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/env
- 6: https://web.dev/articles/window-controls-overlay
Avoid using AppBar width for Windows controls layout.
On Windows, titleBarOverlay reserves the native caption button region and exposes it via CSS environment variables. AppBar is rendered at 100% width with buttons in the top-right drag region, so its layout should use env(titlebar-area-width, ...), rather than the current full-width drag behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/main/presenter/windowPresenter/index.ts` around lines 44 -
63, Update the Windows title-bar overlay layout associated with
getTitleBarOverlayOptions so the AppBar’s drag/content region uses the
titlebar-area-width CSS environment value instead of assuming full width;
preserve the existing overlay colors, height, and non-Windows undefined
behavior.
Consolidate desktop scheduled-task normalize onto backend-core so the daily/weekly lastFiredAt floor applies on desktop too (was a separate, unfixed copy -> still infinite-looped). Omit WCO symbolColor in Windows forced-colors mode. Serialize per-task upserts in ScheduledTasksSettings. Queue ACP recheck when an event arrives mid-flight. Gate AppBar custom controls on platform detection to avoid a duplicate-control flash. Remove manual useMemo (React Compiler) and redundant sr-only labels, add a session-row keyboard handler, broaden piToolCatalog schema (edit.edits[] + recursive property type), fix MCP scope allowlist wording, and align the eventbus test mock with the presenter contract. Add recurrence-boundary + full Pi-tool regression tests.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/test/main/presenter/scheduledTasks.test.ts (1)
155-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact next-fire timestamp for the daily cases.
Both daily tests use inequality assertions only. An implementation that returns any far-future timestamp passes them. The weekly test at Line 199 already asserts an exact value. Apply the same precision to the daily cases so a regression in the day-advance arithmetic is caught.
♻️ Proposed exact assertions
const next = computeNextFireAt(task, reference.getTime()); - expect(next).not.toBeNull(); - expect(next!).toBeGreaterThan(reference.getTime()); - // Must not be the same-day slot that already fired. - expect(next).not.toBe(slotToday.getTime()); + // Must be the next-day slot, not the same-day slot that already fired. + const slotTomorrow = new Date(slotToday); + slotTomorrow.setDate(slotTomorrow.getDate() + 1); + expect(next).toBe(slotTomorrow.getTime());const next = computeNextFireAt(task, after.getTime()); - expect(next).not.toBeNull(); - expect(next!).toBeGreaterThan(lastFired.getTime()); + const slotTomorrow = new Date(); + slotTomorrow.setHours(9, 30, 0, 0); + slotTomorrow.setDate(slotTomorrow.getDate() + 1); + expect(next).toBe(slotTomorrow.getTime());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/test/main/presenter/scheduledTasks.test.ts` around lines 155 - 197, Update the two daily tests around computeNextFireAt to assert the exact expected next-fire timestamp, matching the precision of the weekly test. For the 09:30:05 same-day case and the 09:29 effectiveAfter case, construct the expected next-day 09:30 timestamp and compare next exactly against it, while retaining the non-null assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/settings/components/ScheduledTasksSettings.tsx`:
- Around line 134-152: Replace the single isSaving boolean tracking around the
task upsert flow with a pending-operation counter or task-ID set,
incrementing/adding before each mutation and decrementing/removing in every
completion path, including errors and finally cleanup. Keep the Saving indicator
active until all concurrent upserts have finished, and update the relevant state
logic around the task persistence handler.
- Around line 130-160: The current per-task persistChain does not serialize
complete settings-snapshot updates across different task operations. Update the
settings mutation flow around persistChainRef and the toggle, remove, and
fireNow handlers so every upsert or operation that applies response.settings
uses one shared ordered queue, ensuring older snapshots cannot overwrite newer
mutations.
---
Nitpick comments:
In `@apps/desktop/test/main/presenter/scheduledTasks.test.ts`:
- Around line 155-197: Update the two daily tests around computeNextFireAt to
assert the exact expected next-fire timestamp, matching the precision of the
weekly test. For the 09:30:05 same-day case and the 09:29 effectiveAfter case,
construct the expected next-day 09:30 timestamp and compare next exactly against
it, while retaining the non-null assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ff4a214-8eee-4f13-9569-1d7e59158eab
📒 Files selected for processing (13)
apps/daemon/src/host/piToolCatalog.tsapps/daemon/test/daemonScheduledTasks.test.tsapps/daemon/test/daemonToolDefinitions.test.tsapps/desktop/src/main/presenter/scheduledTasks/normalize.tsapps/desktop/src/main/presenter/windowPresenter/index.tsapps/desktop/test/main/eventbus/eventbus.test.tsapps/desktop/test/main/presenter/scheduledTasks.test.tspackages/ui/settings/components/AgentExtensionPolicyPanel.tsxpackages/ui/settings/components/ArgosAgentsSettings.tsxpackages/ui/settings/components/ScheduledTasksSettings.tsxpackages/ui/src/components/AppBar.tsxpackages/ui/src/components/WindowSideBarSessionItem.tsxpackages/ui/src/composables/useAcpAgentUpdateNotifications.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/daemon/src/host/piToolCatalog.ts
- packages/ui/src/composables/useAcpAgentUpdateNotifications.ts
- apps/daemon/test/daemonToolDefinitions.test.ts
- packages/ui/settings/components/AgentExtensionPolicyPanel.tsx
- packages/ui/settings/components/ArgosAgentsSettings.tsx
- packages/ui/src/components/WindowSideBarSessionItem.tsx
- apps/desktop/src/main/presenter/windowPresenter/index.ts
- packages/ui/src/components/AppBar.tsx
Replace the per-task upsert chain and isSaving boolean with a single ordered mutation queue that covers every settings-mutating operation (upsert, toggle, remove, fireNow) and a pending-operation counter for the Saving indicator. Out-of-order responses across different tasks or operations can no longer restore older fields, and concurrent saves no longer flip the indicator off early. Uses promise chaining instead of try/finally so React Compiler can still optimize the component.
Summary
Multiple fixes for Argos agent settings, scheduled tasks, and MCP scope — all discovered while configuring a new Argos agent.
Agent tools visibility
tools.listDefinitionsroute returned only MCP tools; the Argos agents settings page filterssource === "agent", so the tools list was always empty.piToolCatalog(read/bash/edit/write/grep/find/ls with real descriptions) into the daemon dispatcher route.orchestrationEnabledis off).MCP scope panel — servers invisible
AgentExtensionPolicyPanelfetched MCP servers via legacy desktop IPC presenter (Electron main store), but servers added through MCP settings live in the daemon store. Two different stores → empty list.createMcpClient().getMcpServers()— the same daemon store the agent sessions actually enforce against.Scheduled task action type reverting to Notify
commitTask(index)captured the staletasksclosure (or stale ref). Called synchronously aftersetSettings(next), it persisted the pre-update task, and the server response overwrote the local state.commitTasknow accepts an optional task override; all synchronous commit sites pass the freshly-builtnext.tasks[index].Daily/weekly tasks firing in an infinite loop
computeNextFireAtignoredlastFiredAtfor daily/weekly triggers. After firing, the 60s drift tolerance made it return the same-day slot → delay=0 → immediate re-fire → infinite loop, each iteration creating a new session.computeNextFireAtnow useslastFiredAtas a floor (effectiveAfter = max(after, lastFiredAt)), so fired slots are skipped.Daemon prompt-draft crash
DaemonScheduledTasksWindowPresenter.mainWindowwas a fake{ id: -1, isDestroyed: () => false }that passed the truthy check inrunPromptDraft, thensendToWindowthrew.mainWindow = nullso the draft path falls through to notification-only (correct headless behavior).Error logging
catchintransport/http.tsnowconsole.errors the route name + full error (previously silently converted to JSON response).ScheduledTasksSettingscatch blocks nowconsole.errorwith context and include the error message in the toast description.Misc
hooksNotificationsconfig warning on undefined store value (early return defaults before Zod parse).McpIndicator.Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Style