feat: ENG-694 - Daemon Architecture for Background Task Execution (Consolidated) - #770
Merged
Merged
Conversation
added 3 commits
March 22, 2026 16:49
From PR #626 by david-mamani (David Abdiel Mamani Chuquimamani): - packages/agent-core/src/common/types/daemon.ts: daemon protocol types - packages/agent-core/src/daemon/server.ts: DaemonServer JSON-RPC dispatcher - packages/agent-core/src/daemon/client.ts: DaemonClient typed RPC caller - packages/agent-core/src/daemon/transport.ts: in-process transport pair - packages/agent-core/src/daemon/ipc-transport.ts: child_process IPC transport - packages/agent-core/src/daemon/scheduler.ts: cron-based task scheduler - packages/agent-core/src/daemon/index.ts: public exports - apps/desktop/src/main/daemon-bootstrap.ts: bootstrap daemon lifecycle - apps/desktop/src/main/daemon/cli-bridge.ts: CLI JSON-RPC bridge - apps/desktop/src/main/daemon/entry.ts: daemon entry point - apps/desktop/src/main/daemon/service-manager.ts: auto-start service - apps/desktop/src/main/tray.ts: system tray with task count - apps/desktop/e2e/specs/daemon.spec.ts: E2E tests Closes #402 Ref: ENG-694
…694) From PR #613 by SaaiAravindhRaja / ChaiAndCode: - apps/desktop/src/main/daemon/server.ts: Unix socket / named pipe server with JSON-RPC 2.0, supports daemon.ping, task.start, task.schedule, etc. - apps/desktop/src/main/ipc/task-callbacks.ts: add DaemonTaskCallbacksOptions and createDaemonTaskCallbacks() for background task execution with system notifications when window is hidden - apps/desktop/__tests__/unit/main/daemon/scheduler.unit.test.ts: scheduler tests - apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts: server unit tests - apps/web/src/client/components/settings/DaemonPanel.tsx: Settings UI panel with background mode toggle and socket path display - apps/web/src/client/components/ui/switch.tsx: accessible Switch component - apps/web/locales/en|zh-CN/settings.json: add 'daemon' tab i18n key - apps/web/src/client/components/layout/SettingsDialog.tsx: add Daemon tab - apps/web/src/client/lib/accomplish.ts: add daemon API interface methods - apps/desktop/src/preload/index.ts: expose daemon IPC methods to renderer - apps/desktop/src/main/index.ts: integrate daemon bootstrap + tray init - apps/desktop/src/main/ipc/handlers/settings-handlers.ts: IPC handlers for daemon:get-run-in-background, daemon:set-run-in-background, daemon:get-socket-path - packages/agent-core/src/storage/migrations/v013-daemon.ts: DB migration to add run_in_background column to app_settings - packages/agent-core/src/storage/repositories/appSettings.ts: add getRunInBackground / setRunInBackground CRUD - packages/agent-core/src/types/storage.ts: extend AppSettings + AppSettingsAPI Ref: ENG-694
…ces (ENG-694) From PR #598 by aryan877 (Aryan): - apps/daemon/package.json: @accomplish/daemon standalone app config - apps/daemon/src/index.ts: daemon entry point with HTTP server - apps/daemon/src/cli.ts: CLI argument parsing and startup - apps/daemon/src/health.ts: health check endpoint (/health) - apps/daemon/src/http-server-factory.ts: HTTP server with auth token validation (timing-safe), request size limits, route dispatch - apps/daemon/src/rate-limiter.ts: sliding window rate limiter per IP - apps/daemon/src/permission-service.ts: permission request management - apps/daemon/src/storage-service.ts: persistent storage service - apps/daemon/src/task-service.ts: task creation and management - apps/daemon/src/thought-stream-service.ts: thought stream events - apps/daemon/tsconfig.json: TypeScript config for daemon app - apps/daemon/tsup.config.ts: tsup build config - apps/daemon/vitest.config.ts: vitest test config - apps/daemon/__tests__/unit/health.test.ts: health endpoint tests - apps/daemon/__tests__/unit/http-server-factory.test.ts: server factory tests - apps/daemon/__tests__/unit/rate-limiter.test.ts: rate limiter tests - docs/architecture.md: daemon architecture documentation Key features: Bearer token auth, 1MB request body limit, 429 rate limiting, per-IP sliding window with automatic cleanup. Ref: ENG-694
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds a daemon subsystem and desktop integration: JSON‑RPC protocol, daemon server/client transports (in‑process and IPC), PID locking, scheduler, HTTP/WebSocket endpoints, task/permission/thought services, storage migration/settings for background run, desktop bootstrap/spawn/fallback, tray/CLI/UI settings, tests, and build/test configs. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Electron UI
participant Client as DaemonClient
participant Daemon as Daemon Process
participant Storage as Storage
participant TaskMgr as Task Manager
UI->>Client: JSON‑RPC request (task.start)
Client->>Daemon: deliver request
Daemon->>Storage: persist placeholder task
Daemon->>TaskMgr: start task
TaskMgr-->>Daemon: progress / messages
Daemon->>Storage: persist progress/status
Daemon-->>Client: notify UI (task.progress)
TaskMgr-->>Daemon: complete
Daemon->>Storage: mark complete
Daemon-->>Client: notify UI (task.complete)
sequenceDiagram
participant Desktop as Electron Main
participant Child as Forked Child
participant IPC as Node IPC
participant InProc as In‑Process Transport
Desktop->>Child: fork daemon entry (daemon:init)
Child->>Desktop: IPC "daemon:ready"
alt Child ready within timeout
Desktop->>IPC: create IPC transport → DaemonClient
else fork fails or times out
Desktop->>InProc: create in‑process transport pair and bootstrap daemon in‑process
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (19)
packages/agent-core/src/daemon/scheduler.ts-112-112 (1)
112-112: 🛠️ Refactor suggestion | 🟠 MajorReplace
console.logwith the application's logger.Production code should use the app's existing logger instead of
console.log. This applies to all logging statements in this file (lines 112, 134, 155, 166, 173, 186, 194). As per coding guidelines: "Noconsole.login production code — use the app's existing logger."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/scheduler.ts` at line 112, Replace all console.log calls in this file with the application's logger: import or reuse the existing logger instance (e.g., logger or processLogger) and replace console.log(...) with logger.info(...) for normal events and logger.error(...) for errors, preserving the original message text and interpolated values (e.g., id, cron, prompt, err). Update every logging site in this module (the places that currently log schedule additions, removals, execution starts/completions, and errors—e.g., inside functions handling addSchedule, removeSchedule, runSchedule/runDueSchedules/stopSchedule) so they use the app logger and remove any remaining console.* usages. Ensure the messages retain the same context and values so behavior and searchable logs remain consistent.apps/web/src/client/components/ui/switch.tsx-1-40 (1)
1-40:⚠️ Potential issue | 🟠 MajorRefactor to use
@radix-ui/react-switchwith CVA variants to match the project's UI component pattern.The custom Switch implementation should follow the established pattern used by other UI components in the codebase (label, avatar, button, etc.), which wrap Radix UI primitives with class-variance-authority for variants. Add
@radix-ui/react-switchas a dependency and refactor this component to import from it, similar to howlabel.tsxuses@radix-ui/react-label. This ensures consistency across the component library and leverages Radix UI's accessibility features.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/client/components/ui/switch.tsx` around lines 1 - 40, Replace the custom button-based Switch with a Radix-based wrapper following the project's CVA variant pattern: add `@radix-ui/react-switch` dependency, create a Radix Switch root and thumb wrapper that map the current props (checked, onChange, disabled, ariaLabel, className) and export the same Switch component interface (SwitchProps) so consuming code is unchanged; use class-variance-authority to define variants for the root and thumb (matching existing tokens like bg-primary/bg-muted, translate-x classes, focus/disabled styles) and ensure event handlers use Radix's onCheckedChange to call the provided onChange; update imports/exports so the component exports the same Switch function name and prop types.apps/daemon/vitest.config.ts-7-7 (1)
7-7:⚠️ Potential issue | 🟠 MajorReplace
__dirnamewith ESM-safe path resolution.The daemon package is configured as ESM (
"type": "module"), so__dirnameis undefined and will cause a ReferenceError at runtime, breaking Vitest startup.🔧 Proposed fix
-import { defineConfig } from 'vitest/config'; -import path from 'path'; +import { defineConfig } from 'vitest/config'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); export default defineConfig({ resolve: { alias: { - '@accomplish_ai/agent-core': path.resolve(__dirname, '../../packages/agent-core/src'), + '@accomplish_ai/agent-core': resolve(__dirname, '../../packages/agent-core/src'), }, }, test: { globals: true, root: __dirname,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/vitest.config.ts` at line 7, The config uses path.resolve(__dirname, '../../packages/agent-core/src') which fails in ESM where __dirname is undefined; replace the __dirname usage with an ESM-safe resolution using import.meta.url (e.g. derive a dir with fileURLToPath(import.meta.url) + path.dirname, or use new URL('../../packages/agent-core/src', import.meta.url) and convert to a filesystem path) so the mapping for '@accomplish_ai/agent-core' is resolved correctly in vitest.config.ts; update the expression that calls path.resolve (and import or use fileURLToPath/import.meta.url) accordingly.apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts-1-241 (1)
1-241: 🛠️ Refactor suggestion | 🟠 MajorSplit this spec to stay under the new-file size limit.
At 241 lines, this new file already exceeds the repo cap. Extracting
sendJsonRpc()or moving lifecycle/error-path cases into a second spec would get it back under the limit and keep future additions manageable. As per coding guidelines!(packages/agent-core/src/storage/migrations)/**/*.{js,ts,tsx}: New files must be < 200 lines — split into logical modules if needed (exceptions: generated files, migrations).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts` around lines 1 - 241, The test file exceeds the new-file size limit; split it by extracting the reusable sendJsonRpc helper and/or some lifecycle/error-path tests into a second spec file so each new test file is under 200 lines. Concretely, move the sendJsonRpc(...) function into a test utility module (e.g., test/utils/sendJsonRpc) and import it in this spec, and/or relocate a subset of tests (for example the parse-error, notification, and handler-error cases or the idempotency/lifecycle tests) into a new spec file that imports registerMethod/startDaemonServer/stopDaemonServer/getSocketPath; ensure the new files keep the same describe blocks and vi.mock setup so tests run unchanged.apps/daemon/src/storage-service.ts-21-30 (1)
21-30:⚠️ Potential issue | 🟠 MajorOnly publish the storage handle after
initialize()succeeds.If
storage.initialize()throws here,this.storagestays non-null and latergetStorage()/close()calls will operate on a partially opened instance. That makes recovery after a failed open or migration unreliable.💡 Safer initialization pattern
- this.storage = createStorage({ + const storage = createStorage({ databasePath, runMigrations: true, userDataPath: dir, secureStorageFileName: dataDir ? 'secure-storage.json' : 'secure-storage-dev.json', }); - this.storage.initialize(); + storage.initialize(); + this.storage = storage; console.log(`[StorageService] Database initialized at ${databasePath}`); - return this.storage; + return storage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/storage-service.ts` around lines 21 - 30, The current code assigns this.storage from createStorage(...) before calling this.storage.initialize(), which can leave a partially-initialized handle if initialize() throws; change to create a local variable (e.g., const storage = createStorage(...)), call await storage.initialize() (or handle the returned promise) and only after initialize() succeeds assign this.storage = storage; also ensure getStorage() and close() continue to guard against null/undefined storage references (they already exist but verify behavior remains correct).apps/desktop/src/main/index.ts-342-349 (1)
342-349:⚠️ Potential issue | 🟠 MajorGate tray persistence on the actual background-mode setting.
This path now always converts window close into
hide(), andwindow-all-closednever quits. That makes the new “Run in Background” setting effectively cosmetic: users can disable it in settings and still end up with the app resident in the tray.Also applies to: 365-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 342 - 349, The close handler currently always prevents close and hides the window, ignoring the user's "Run in Background" setting; update the logic in the mainWindow 'close' listener to check the real background-mode flag (e.g., a settings getter like getRunInBackground() or appSettings.runInBackground) before calling event.preventDefault() and mainWindow.hide(), and similarly adjust the 'window-all-closed' handler to only call app.quit() when the background-mode is disabled; reference the existing symbols mainWindow, isQuitting, and the window-all-closed handler so the change gates hiding/quitting on the actual setting.apps/desktop/src/main/tray.ts-103-105 (1)
103-105:⚠️ Potential issue | 🟠 Major
updateTray()drops the window reference.Rebuilding the menu with
buildContextMenu(null)means the "Show Accomplish" entry stops being able to reopen/focus the app after the first refresh. Keep the activeBrowserWindowin module state and reuse it when updating the menu.💡 Preserve the live window when refreshing the tray menu
let tray: Tray | null = null; +let trayWindow: BrowserWindow | null = null; let activeTaskCount = 0; @@ export function createTray(mainWindow: BrowserWindow | null): Tray { + trayWindow = mainWindow; const iconPath = getIconPath(); @@ export function updateTray(): void { if (tray && !tray.isDestroyed()) { - tray.setContextMenu(buildContextMenu(null)); + tray.setContextMenu(buildContextMenu(trayWindow)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/tray.ts` around lines 103 - 105, updateTray currently calls buildContextMenu(null) which drops the live BrowserWindow reference so the "Show Accomplish" menu item can't refocus the app; instead keep a module-level BrowserWindow variable (e.g., mainWindow or activeWindow) that is set when the window is created and cleared on close, and change updateTray to call buildContextMenu(activeWindow) (and ensure buildContextMenu accepts a BrowserWindow | null). Also update any window-creation/close handlers to maintain this module state so tray.setContextMenu always receives the live window reference.packages/agent-core/src/daemon/ipc-transport.ts-52-56 (1)
52-56:⚠️ Potential issue | 🟠 MajorDon't ignore IPC send failure/backpressure.
Both
child.send()(line 55) andprocess.send()(line 93) return a boolean indicating whether the message was successfully queued. This transport ignores the return value, treating every call as delivered. When the IPC message queue fills up (especially during daemon shutdown or restart),send()returnsfalse, silently dropping the JSON-RPC request or response and leaving the peer stranded until a higher-level timeout fires.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/ipc-transport.ts` around lines 52 - 56, The transport currently ignores the boolean return of child.send/process.send, so when IPC backpressure occurs messages are silently dropped; modify the send(message: JsonRpcMessage) and the analogous parent-send path to check the returned boolean and, if false, push the DaemonIpcEnvelope onto an internal outgoing queue and attach a one-time 'drain' handler on the corresponding IPC object (child or process) to flush the queue in order; also guard against disconnected peers by dropping queued messages with an error or invoking a callback if the socket closes. Use the existing send method, DaemonIpcEnvelope, child.send and process.send identifiers to locate places to add the queue, push-on-false logic, and the 'drain' flush implementation.apps/daemon/src/task-service.ts-295-318 (1)
295-318:⚠️ Potential issue | 🟠 MajorCarry the configured OpenAI base URL into daemon task env.
This environment builder forwards Bedrock and Ollama settings but omits
storage.getOpenAiBaseUrl(). Tasks running through the daemon path will ignore a user-configured OpenAI-compatible endpoint and fall back to the default API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/task-service.ts` around lines 295 - 318, In buildEnvironment add the configured OpenAI base URL into the environment config so daemon tasks use a user-specified OpenAI-compatible endpoint: call storage.getOpenAiBaseUrl() inside buildEnvironment and include its value (e.g. openAiBaseUrl or openaiBaseUrl) on the EnvironmentConfig object passed to buildOpenCodeEnvironment; update the EnvironmentConfig shape if needed and ensure buildOpenCodeEnvironment consumes that new field so tasks honor the configured OpenAI base URL (references: buildEnvironment, storage.getOpenAiBaseUrl, EnvironmentConfig, buildOpenCodeEnvironment).apps/daemon/src/task-service.ts-189-191 (1)
189-191:⚠️ Potential issue | 🟠 MajorDo not stamp
completedAton non-terminal status changes.Both the resume path and the generic
onStatusChangecallback passnew Date().toISOString()regardless of the new status. That makes queued/running tasks look completed and corrupts completion-based sorting and duration logic.Also applies to: 271-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/task-service.ts` around lines 189 - 191, The current call to this.storage.updateTaskStatus(existingTaskId, task.status, new Date().toISOString()) stamps completedAt for every status change; change it so completedAt is only set when task.status is a terminal state (e.g., "completed", "failed", "canceled")—compute completedAt = terminalStatuses.includes(task.status) ? new Date().toISOString() : null/undefined and pass that value to this.storage.updateTaskStatus, and apply the same conditional logic in the resume path and the onStatusChange callback locations referenced (the other updateTaskStatus call around lines 271-274).apps/desktop/src/main/daemon/server.ts-155-163 (1)
155-163:⚠️ Potential issue | 🟠 MajorOnly delete
daemon.sockafter proving it is stale.Unconditionally unlinking an existing socket lets a second daemon instance take over the path even if the first daemon is still alive, leaving the original process running but unreachable. This cleanup should only happen after a failed connect/health check proves the file is orphaned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/server.ts` around lines 155 - 163, Replace the unconditional removal of socketPath with a probe: attempt a short TCP/IPC connection to socketPath (use net.createConnection / net.connect) and set a short timeout; if the probe connects (on 'connect'), immediately close the connection and do not unlink the file; if the probe fails with a connection-refused/ECONNREFUSED/ENOENT/ENOENT-like error or times out, then treat the socket as stale and call fs.unlinkSync(socketPath); handle other errors by logging and not unlinking blindly. Update the block that currently uses fs.existsSync/fs.unlinkSync to use this probe logic and reference socketPath and the probe connection's event handlers to decide when to unlink.apps/daemon/src/task-service.ts-117-128 (1)
117-128:⚠️ Potential issue | 🟠 MajorSave the task before handing control to the task manager.
startTask()registers callbacks and awaitstaskManager.startTask()before the task is persisted. If the manager emits status/messages synchronously or very quickly, those callbacks can hit storage before the task row exists and the earliest state gets lost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/task-service.ts` around lines 117 - 128, The task is saved after calling taskManager.startTask which can cause race conditions; instead construct the Task object (set task.messages with the initialUserMessage), call this.storage.saveTask(task) before invoking this.taskManager.startTask, and then pass the saved task (or its id) into startTask; keep createCallbacks(taskId) as-is but ensure callbacks reference the existing persisted task row so synchronous callbacks won't run against a non-existent task.apps/desktop/src/main/daemon/server.ts-89-116 (1)
89-116:⚠️ Potential issue | 🟠 MajorValidate the JSON-RPC envelope before dispatch.
Right now any non-array object is cast to
JsonRpcRequest. Payloads with a missing/invalidjsonrpc, a non-stringmethod, or a badidcan be silently dropped as “notifications” or downgraded toMethod not foundinstead of returning-32600 Invalid Request, which will make client bugs very hard to diagnose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/server.ts` around lines 89 - 116, The request envelope isn't being fully validated before casting to JsonRpcRequest; add explicit checks after parsing to ensure parsed.jsonrpc === '2.0', parsed.method is a string, and parsed.id (if present) is either string, number, or null (allowing absence for notifications). If any of these fail, send a JSON-RPC -32600 Invalid Request response (id null) rather than treating it as a notification or falling through to methodHandlers; keep using isNotification(request) and methodHandlers.get(method) after the envelope passes validation. This validation should reference the same variables/types used now (parsed, JsonRpcRequest, isNotification, methodHandlers, id, method, params).apps/desktop/src/main/daemon-bootstrap.ts-84-117 (1)
84-117:⚠️ Potential issue | 🟠 MajorFail fast when the child exits before
daemon:ready.The
exithandler only logs and clearsdaemonProcess, so an early crash keeps the bootstrap promise pending until the 10s timeout fires. Rejecting there immediately will let startup fall back without the extra delay and with a more accurate error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon-bootstrap.ts` around lines 84 - 117, The exit handler for daemonProcess currently only logs and clears daemonProcess, leaving the bootstrap Promise pending; update the daemonProcess.on('exit', ...) callback to clear the readiness timeout and reject the Promise immediately (e.g., call clearTimeout(timer); reject(new Error(`Daemon exited before ready (code ${code})`)); set daemonProcess = null) so the bootstrap fails fast instead of waiting for DAEMON_READY_TIMEOUT_MS; ensure you only reject when the Promise is still pending (introduce a local "settled" boolean toggled when resolve/reject are called, or check for timer existence) to avoid unhandled rejections.apps/daemon/src/mcp-bridges.ts-54-58 (1)
54-58:⚠️ Potential issue | 🟠 MajorDo not expose these loopback control endpoints with
Access-Control-Allow-Origin: *.These servers have no auth and they drive permission/question flows for the active task. With permissive CORS, any website opened in the user's browser can
fetch()localhost and trigger prompts or read back question responses. Remove CORS for this internal bridge or require an auth token/origin allowlist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/mcp-bridges.ts` around lines 54 - 58, The setCors function currently sets Access-Control-Allow-Origin: * which exposes internal loopback control endpoints; change setCors to stop sending a wildcard origin and instead enforce a safe policy: either remove CORS headers entirely for these internal bridges or implement an origin allowlist check (compare req.headers.origin against a configured whitelist) and only set Access-Control-Allow-Origin to that trusted origin, and/or require a server-side auth token (e.g., check an Authorization header) before responding; update the setCors usage (and any handlers that call setCors) to perform this allowlist/auth check so unauthorized browser origins cannot access these endpoints.apps/daemon/src/daemon-options.ts-31-39 (1)
31-39:⚠️ Potential issue | 🟠 MajorThe CLI fallback is not valid on Windows.
When
resolveCliPath()misses, this code falls back to.../bin/opencodeor bareopencode. The Windows task runner in this repo now requires a real.exepath, so this branch guarantees daemon task launches fail there instead of degrading gracefully.Based on learnings: In
packages/agent-core/src/internal/classes/OpenCodeAdapter.ts, Windows now requires CLI commands to resolve to a real.exepath and fails fast for non-.execommands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/daemon-options.ts` around lines 31 - 39, getCliCommand currently falls back to non-executable names which breaks Windows; update getCliCommand to detect Windows (process.platform === 'win32') and prefer real executable paths: check for the Windows variants (e.g., path.join(DAEMON_ROOT, 'node_modules', 'opencode-ai', 'bin', 'opencode.exe') and 'opencode.cmd') and also check node_modules/.bin/opencode.cmd or .exe using fs.existsSync, returning the first real .exe/.cmd path found; if none found on Windows, return a platform-appropriate default (e.g., 'opencode.exe' or 'opencode.cmd') instead of the bare 'opencode', while retaining the existing behavior on non-Windows platforms.packages/agent-core/src/daemon/types.ts-120-184 (1)
120-184:⚠️ Potential issue | 🟠 MajorKeep one canonical daemon protocol definition.
This file diverges from
packages/agent-core/src/common/types/daemon.tson method names/results (task.stopvstask.cancel,health.checkvsdaemon.ping, different notification keys). Leaving both public maps around lets callers type-check against a protocol the runtime does not implement.Based on learnings: Place shared types in
packages/agent-core/src/common/types/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/types.ts` around lines 120 - 184, The RpcMethodMap, RpcNotificationMap and related types in this file diverge from the canonical daemon protocol in common/types/daemon.ts; remove the duplicate local protocol and instead import and re-export the single shared definitions (e.g. RpcMethodMap, RpcMethod, RpcNotificationMap, RpcNotificationType) from packages/agent-core/src/common/types/daemon.ts, and update any differing keys/names to match the canonical names (e.g. rename 'task.stop' to the canonical 'task.cancel', change 'health.check' to 'daemon.ping', and align notification keys like 'task.statusChange' etc. so callers type-check against the runtime protocol). Ensure all references in this file use the imported symbols rather than redeclaring them.apps/daemon/src/permission-service.ts-33-39 (1)
33-39:⚠️ Potential issue | 🟠 MajorA single global
getActiveTaskId()is not enough for concurrent tasks.Both endpoints stamp requests with whatever task ID this getter returns at that moment. Since the daemon is configured elsewhere in this PR for multiple concurrent tasks, a permission/question emitted by task A can be attributed to task B. The request payload or transport context needs a task-scoped identifier.
Also applies to: 80-88, 135-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/permission-service.ts` around lines 33 - 39, The current init registers a single global getActiveTaskId and onPermissionRequest causing cross-task attribution; change the APIs so each emitted permission/question carries an explicit task-scoped id: update init to accept a task-scoped getter or make permissionRequestHandler signature require a taskId parameter (e.g., permissionRequestHandler(request: unknown, taskId: string)), and modify all emit points that call getActiveTaskId/onPermissionRequest (including the code paths referenced by getActiveTaskId and onPermissionRequest) to pass through the originating taskId from the request or transport context instead of relying on a global getter; ensure the request payload or transport envelope is extended to include this taskId and the permission emission/stamping logic uses that field.apps/daemon/src/mcp-bridges.ts-65-74 (1)
65-74:⚠️ Potential issue | 🟠 MajorCatch rejected async route handlers inside
createMcpServer.
createMcpServer()invokeshandler(req, res)at line 70 without awaiting or catching rejections, but all three call sites (lines 92, 114, 135) passasynchandlers. The type signature declareshandler: (...) => void, but async functions returnPromise<void>. Any error thrown in the handler—fromparseBody(line 58:JSON.parsecan throw on malformed JSON),await promisewaits, or any async operation—becomes an unhandled rejection. The client receives no response and hangs.Wrap the
handler(req, res)call in a.catch()or top-level try/catch to send an error response:handler(req, res).catch(err => { console.error(`[${label}] Handler error:`, err); if (!res.headersSent) { res.writeHead(500); res.end(JSON.stringify({ error: 'Internal server error' })); } });Or update the type signature to
handler: (...) => Promise<void>and await it, then add error handling in the HTTP server wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/mcp-bridges.ts` around lines 65 - 74, createMcpServer currently calls handler(req, res) without handling Promise rejections (handler is async at the call sites), so update the signature to handler: (req: http.IncomingMessage, res: http.ServerResponse) => Promise<void> and in the http.createServer wrapper await the call and catch errors (or call handler(...).catch(...)); on error log with the label (use `[${label}]`) and if !res.headersSent send a 500 JSON error and end the response so the client doesn't hang; reference createMcpServer and the handler invocation when making the change.
🟡 Minor comments (5)
apps/daemon/src/pid.ts-35-35 (1)
35-35:⚠️ Potential issue | 🟡 MinorReplace
console.logwith the app's logger.The coding guidelines specify no
console.login production code — use the app's existing logger instead. This file usesconsole.logfor PID file operations logging.As per coding guidelines: "No
console.login production code — use the app's existing logger"♻️ Proposed approach
Import the daemon's logger (or create one) and replace the console.log calls:
+import { logger } from './logger.js'; // or appropriate logger import - console.log(`[Daemon] Removing stale PID file (pid ${pid} not running)`); + logger.info(`Removing stale PID file (pid ${pid} not running)`); - console.log(`[Daemon] PID file written: ${PID_FILE} (pid ${process.pid})`); + logger.info(`PID file written: ${PID_FILE} (pid ${process.pid})`); - console.log('[Daemon] PID file removed'); + logger.info('PID file removed');Also applies to: 49-49, 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/pid.ts` at line 35, Replace the console.log calls in this PID management module with the application's logger: import the app logger (use the actual exported name from your logging module, e.g., logger or daemonLogger) at the top of the file and change the console.log invocations (the "[Daemon] Removing stale PID file..." and the other console.log occurrences handling PID file operations) to logger.info or logger.debug as appropriate, preserving the original messages but routing them through the app logger API.apps/desktop/src/main/daemon/entry.ts-78-82 (1)
78-82:⚠️ Potential issue | 🟡 MinorMissing validation for
params.taskbefore callingstorage.saveTask.The handler checks that
paramsexists but doesn't verifyparams.taskis defined. If a caller sends{ }as params,storage.saveTask(undefined)would be called.🛡️ Suggested defensive check
server.registerMethod('storage.saveTask', (params) => { - if (params) { + if (params?.task) { storage.saveTask(params.task); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/entry.ts` around lines 78 - 82, The handler registered with server.registerMethod('storage.saveTask') currently only checks params but not params.task; update the handler to verify params.task exists and is valid before calling storage.saveTask. Specifically, add a guard that returns early (or responds with a clear error) if params is falsy or params.task is undefined/null/invalid, and only call storage.saveTask(params.task) when the check passes; refer to the server.registerMethod callback and the storage.saveTask call to locate where to add this defensive validation.apps/desktop/src/main/daemon/entry.ts-84-100 (1)
84-100:⚠️ Potential issue | 🟡 MinorSimilar validation gaps for other storage methods.
The handlers for
updateTaskStatus,updateTaskSummary, andaddTaskMessagealso check only thatparamsexists, but don't validate the required nested properties (taskId,status,summary,message). This could lead to passingundefinedvalues to storage methods.🛡️ Suggested validation pattern
server.registerMethod('storage.updateTaskStatus', (params) => { - if (params) { + if (params?.taskId && params.status !== undefined) { storage.updateTaskStatus(params.taskId, params.status, params.completedAt); } }); server.registerMethod('storage.updateTaskSummary', (params) => { - if (params) { + if (params?.taskId && params.summary !== undefined) { storage.updateTaskSummary(params.taskId, params.summary); } }); server.registerMethod('storage.addTaskMessage', (params) => { - if (params) { + if (params?.taskId && params.message) { storage.addTaskMessage(params.taskId, params.message); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/entry.ts` around lines 84 - 100, The handlers registered via server.registerMethod for 'storage.updateTaskStatus', 'storage.updateTaskSummary', and 'storage.addTaskMessage' only check that params is truthy but don't validate required nested fields (taskId, status, summary, message), which can pass undefined into storage.updateTaskStatus, storage.updateTaskSummary, and storage.addTaskMessage; update each handler to explicitly validate the required properties (e.g., ensure params.taskId is present and params.status/params.summary/params.message as applicable), and on missing/invalid values either log an error and return early or throw a clear error so the storage methods are only called with valid inputs.apps/desktop/src/main/daemon/service-manager.ts-69-72 (1)
69-72:⚠️ Potential issue | 🟡 MinorInvalid fallback path
'~'won't expand.The
'~'string literal won't be expanded by Node.js path operations. IfHOMEis undefined, the service file path will be incorrect.🐛 Proposed fix
function getSystemdServiceDir(): string { - const configDir = process.env.XDG_CONFIG_HOME || path.join(process.env.HOME || '~', '.config'); + const home = process.env.HOME || require('os').homedir(); + const configDir = process.env.XDG_CONFIG_HOME || path.join(home, '.config'); return path.join(configDir, 'systemd', 'user'); }Note: Since
osisn't imported, you could also useapp.getPath('home')which is already available from Electron.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/service-manager.ts` around lines 69 - 72, The fallback in getSystemdServiceDir uses the literal '~' which isn't expanded; replace the fallback with a real home directory resolver (e.g., use os.homedir() after importing os, or use Electron's app.getPath('home') since Electron is available) and update the configDir computation to use that value instead of '~' so path.join produces a valid path; ensure you add the corresponding import (os or Electron app) and update any references to process.env.HOME accordingly in getSystemdServiceDir.packages/agent-core/src/common/types/daemon.ts-33-38 (1)
33-38:⚠️ Potential issue | 🟡 MinorLet protocol error responses use
id: null.
JsonRpcResponsenarrowsidtostring | number, but this same module defines parse/invalid-request errors that must usenullwhen a request cannot be correlated. The current type makes spec-compliant error envelopes impossible without casts.Also applies to: 59-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/common/types/daemon.ts` around lines 33 - 38, The JsonRpc response types currently restrict id to string | number which prevents creating spec-compliant error envelopes that require id: null; update the JsonRpcResponse interface to allow id: string | number | null and likewise adjust any related response/error interfaces defined later (the block around the other response/error types at lines 59-71) so their id fields accept null as well, ensuring parse/invalid-request error objects can be constructed without casts.
🧹 Nitpick comments (10)
packages/agent-core/src/daemon/scheduler.ts (1)
160-167: Timer does not align to minute boundaries on start.The timer starts immediately with a 60-second interval, but if
addScheduledTaskis called at 12:00:30, the first tick occurs at 12:01:30, potentially missing a task scheduled for0 * * * *(every hour at minute 0) if the hour changes. Consider aligning the first tick to the next minute boundary.♻️ Suggested alignment approach
function startTimer(): void { - // Check every 60 seconds (aligned to minute boundaries) - timerId = setInterval(() => { - tick(); - }, 60_000); + // Align to next minute boundary, then check every 60 seconds + const now = new Date(); + const msUntilNextMinute = (60 - now.getSeconds()) * 1000 - now.getMilliseconds(); + + setTimeout(() => { + tick(); + timerId = setInterval(() => { + tick(); + }, 60_000); + }, msUntilNextMinute); console.log('[Scheduler] Timer started'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/scheduler.ts` around lines 160 - 167, startTimer currently calls setInterval immediately so the first tick can be offset from minute boundaries; change startTimer to compute the milliseconds until the next minute boundary, use setTimeout to call tick once at that aligned time, then start the recurring timerId = setInterval(tick, 60_000) from within that timeout so all subsequent ticks are aligned to minute boundaries; update any references to timerId/tick accordingly and clear existing timers as before to avoid duplicates.apps/daemon/tsconfig.json (1)
20-23: Consider using TypeScript project references instead of including agent-core source directly.Including
../../packages/agent-core/src/**/*directly in compilation scope may cause duplicate compilation and slower builds. Consider using TypeScript project references or referencing the built package instead.However, if this is intentional for development (e.g.,
noEmit: truesuggests type-checking only), this approach works.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/tsconfig.json` around lines 20 - 23, The tsconfig.json currently includes the agent-core source via the "include" entry ("../../packages/agent-core/src/**/*"), which can cause duplicate compilation and slow builds; update the tsconfig for apps/daemon to use TypeScript project references (add a "references" array pointing to the built agent-core tsconfig) or reference the built package instead, and remove the direct source include, while preserving the existing noEmit/type-checking intent (ensure "noEmit" remains if you still only want type checks); change the include back to only "src/**/*" and add a proper "references": [{ "path": "../../packages/agent-core" }] so the compiler uses project references rather than compiling agent-core source inline.apps/daemon/src/pid.ts (1)
13-16: Add braces for consistency with coding guidelines.The
try/catchblocks use single-line bodies without explicit braces structure. While technically functional, the coding guidelines require braces for all control flow statements.♻️ Proposed fix
function isProcessRunning(pid: number): boolean { - try { process.kill(pid, 0); return true; } - catch (e) { return (e as NodeJS.ErrnoException).code === 'EPERM'; } + try { + process.kill(pid, 0); + return true; + } catch (e) { + return (e as NodeJS.ErrnoException).code === 'EPERM'; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/pid.ts` around lines 13 - 16, The isProcessRunning function uses single-line try/catch bodies without braces; update the function (isProcessRunning) to use explicit block braces for both the try and catch clauses so it conforms to the coding guidelines—keep the same logic (call process.kill(pid, 0) and return true on success; in the catch, cast the error to NodeJS.ErrnoException and return e.code === 'EPERM') and preserve return values and types.apps/daemon/src/cli.ts (1)
5-10: Consider edge case:--socket-pathor--data-dirfollowed by another flag.If the user passes
--socket-path --version, the current logic treats--versionas the socket path value. This is becauseargv[i + 1]only checks for truthiness, not whether it's another flag.♻️ Proposed fix to detect flag-like values
- if (argv[i] === '--socket-path' && argv[i + 1]) { + if (argv[i] === '--socket-path' && argv[i + 1] && !argv[i + 1].startsWith('--')) { result.socketPath = argv[i + 1]; i++; - } else if (argv[i] === '--data-dir' && argv[i + 1]) { + } else if (argv[i] === '--data-dir' && argv[i + 1] && !argv[i + 1].startsWith('--')) { result.dataDir = argv[i + 1]; i++;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/cli.ts` around lines 5 - 10, The argument parsing treats the next token as a value even if it's another flag; update the logic around argv, result.socketPath and result.dataDir so you only accept argv[i+1] as a value when it exists and does not start with '-' (or '--'); if argv[i+1] is missing or looks like a flag, do not assign it (optionally emit an error or keep the current value) and only increment i when you actually consume a value. Ensure these checks are applied in the same parsing block that sets result.socketPath and result.dataDir so you don't accidentally treat flags like '--version' as values.apps/daemon/__tests__/unit/health.test.ts (1)
67-81: Fake timer setup may not affect the service's start time.The
serviceis created in the outerbeforeEach(line 11-13) before fake timers are enabled in the nestedbeforeEach(line 68-70). This means the service captures its start time using realDate.now(), not the fake timer. Whenvi.advanceTimersByTime(5000)runs, it advances the mockedDate.now()but the service's recorded start time remains the real one, which may cause inconsistent behavior.Consider creating a fresh
HealthServiceinstance inside theuptimedescribe block after enabling fake timers:🛠️ Suggested fix
describe('uptime', () => { + let uptimeService: HealthService; + beforeEach(() => { vi.useFakeTimers(); + uptimeService = new HealthService(); }); afterEach(() => { vi.useRealTimers(); }); it('should increase over time', () => { - const status1 = service.getStatus(); + const status1 = uptimeService.getStatus(); vi.advanceTimersByTime(5000); - const status2 = service.getStatus(); + const status2 = uptimeService.getStatus(); expect(status2.uptime).toBeGreaterThan(status1.uptime); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/__tests__/unit/health.test.ts` around lines 67 - 81, The test uses fake timers after the outer service instance is created, so HealthService captured a real start time; recreate the service after enabling fake timers so uptime uses mocked time — inside the 'uptime' describe block replace or add a beforeEach that calls vi.useFakeTimers() and then instantiates a new HealthService (the same constructor used originally, referenced as service or HealthService) so subsequent vi.advanceTimersByTime(5000) affects service.getStatus().uptime; also restore timers in afterEach with vi.useRealTimers().apps/desktop/e2e/specs/daemon.spec.ts (1)
48-49: Hardcoded delay may cause flakiness.The 2-second fixed delay for background processing might be insufficient on slower CI machines or excessive on fast ones. Consider using a polling approach or extending
ExecutionPagewith a method that waits for task progress indicators.That said, for simulating "UI hidden while processing," a brief fixed wait may be acceptable if the mock processing time is deterministic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/e2e/specs/daemon.spec.ts` around lines 48 - 49, Replace the hardcoded 2s sleep in daemon.spec.ts with a deterministic wait: add or use a helper on ExecutionPage (e.g., waitForProcessingIndicator, waitForDaemonIdle or waitForTaskCompletion) that polls the DOM or uses page.waitForSelector/page.waitForFunction to detect when the processing UI is hidden or the daemon task is complete; update the test to call that method instead of new Promise/setTimeout so the test waits reliably across CI speeds and avoids flakiness.apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts (1)
69-70: Avoid fixed startup sleeps in the socket tests.These hard-coded
200mswaits make the suite timing-sensitive; slower CI runners can still hit the connect before the server finishes binding. Prefer a readiness signal or a short retry loop that waits until the socket actually accepts connections.Also applies to: 90-90, 112-112, 152-152, 189-189, 212-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts` around lines 69 - 70, Replace the fixed 200ms startup sleeps (the statements like await new Promise((resolve) => setTimeout(resolve, 200));) with a deterministic readiness check: either have the server instance emit/resolve a "ready" promise when it finishes binding (add and await server.once('listening') or a custom ready promise in the server creation helper), or implement a short retry loop that attempts to connect to the socket until successful (with a small delay and a timeout). Update each test that uses the fixed sleep (the occurrences of the await new Promise(...) sleep) to await the server readiness signal or the connection-retry loop instead so tests only proceed once the socket actually accepts connections.apps/desktop/src/main/daemon/service-manager.ts (1)
110-117: Consider adding a timeout toexecSynccalls.If
systemctlhangs (e.g., waiting for D-Bus), the Electron main process will block indefinitely. A timeout provides a safety net.♻️ Proposed fix
try { - execSync('systemctl --user daemon-reload', { stdio: 'pipe' }); - execSync(`systemctl --user enable ${SYSTEMD_SERVICE_NAME}`, { stdio: 'pipe' }); + execSync('systemctl --user daemon-reload', { stdio: 'pipe', timeout: 10_000 }); + execSync(`systemctl --user enable ${SYSTEMD_SERVICE_NAME}`, { stdio: 'pipe', timeout: 10_000 }); console.log('[ServiceManager] systemd user service enabled'); } catch (err) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/service-manager.ts` around lines 110 - 117, The execSync calls in ServiceManager that run 'systemctl --user daemon-reload' and 'systemctl --user enable ${SYSTEMD_SERVICE_NAME}' can hang and block the Electron main process; add a timeout option to these execSync invocations (e.g., { stdio: 'pipe', timeout: <ms> }) so the calls will throw after a bounded period, and handle the timeout error in the existing catch block (preserve logging via console.error and rethrow). Locate the execSync usage in service-manager.ts around the ServiceManager systemd enable logic and update both calls to include a sensible timeout value and ensure error handling still reports the error.apps/daemon/src/index.ts (1)
55-329: Consider splitting the largemain()function into smaller modules.At ~275 lines, this function handles PID locking, storage init, service creation, RPC registration, event wiring, server startup, and shutdown. Extracting logical sections (e.g.,
initializeServices(),registerRpcMethods(),setupShutdown()) would improve maintainability.As per coding guidelines: "New files must be < 200 lines — split into logical modules if needed"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/index.ts` around lines 55 - 329, The main() function is too large; split it into smaller helper functions to keep files <200 lines and improve readability: extract service creation into initializeServices() (handle StorageService, TaskService, HealthService, PermissionService, ThoughtStreamService, authToken generation and related paths), move all rpc.registerMethod(...) calls into registerRpcMethods(rpc, taskService, storage, permissionService, thoughtStreamService, healthService), pull event wiring into wireTaskEvents(taskService, rpc, thoughtStreamService, healthService), factor server startup and env setup into startServers(rpc, permissionService, thoughtStreamService, storageService, taskService) that returns ports/auth, and encapsulate shutdown logic into setupShutdown(taskService, thoughtStreamService, permissionService, rpc, storageService, pidLock). Then refactor main() to call these helpers in order (acquirePidLock remains in main before initializeServices) so behavior is unchanged but code is modular.packages/agent-core/src/daemon/server.ts (1)
99-99: Replaceconsole.errorwith the app's logger.Production code should use the application's logging infrastructure rather than direct console calls.
As per coding guidelines: "No
console.login production code — use the app's existing logger"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/server.ts` at line 99, Replace the direct console.error call in the DaemonServer request handler with the application's logger: call the module's logger.error (e.g., logger.error or daemonLogger.error) instead of console.error and pass the same message and the error object (keep `[DaemonServer] Handler error for ${request.method}` and the errorMessage as arguments). If no logger is imported in server.ts, import the shared app logger used elsewhere (e.g., processLogger/logger) and use it here so production logging goes through the app's logging infrastructure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02434466-e45e-4f03-8364-0f5b4f98e134
📒 Files selected for processing (57)
apps/daemon/__tests__/unit/health.test.tsapps/daemon/__tests__/unit/http-server-factory.test.tsapps/daemon/__tests__/unit/rate-limiter.test.tsapps/daemon/package.jsonapps/daemon/src/cli.tsapps/daemon/src/daemon-options.tsapps/daemon/src/health.tsapps/daemon/src/http-server-factory.tsapps/daemon/src/index.tsapps/daemon/src/mcp-bridges.tsapps/daemon/src/permission-service.tsapps/daemon/src/pid.tsapps/daemon/src/rate-limiter.tsapps/daemon/src/storage-service.tsapps/daemon/src/task-service.tsapps/daemon/src/thought-stream-service.tsapps/daemon/src/websocket.tsapps/daemon/tsconfig.jsonapps/daemon/tsup.config.tsapps/daemon/vitest.config.tsapps/desktop/__tests__/unit/main/daemon/scheduler.unit.test.tsapps/desktop/__tests__/unit/main/daemon/server.unit.test.tsapps/desktop/e2e/specs/daemon.spec.tsapps/desktop/src/main/daemon-bootstrap.tsapps/desktop/src/main/daemon/cli-bridge.tsapps/desktop/src/main/daemon/entry.tsapps/desktop/src/main/daemon/server.tsapps/desktop/src/main/daemon/service-manager.tsapps/desktop/src/main/index.tsapps/desktop/src/main/ipc/handlers/settings-handlers.tsapps/desktop/src/main/ipc/task-callbacks.tsapps/desktop/src/main/tray.tsapps/desktop/src/preload/index.tsapps/web/locales/en/settings.jsonapps/web/locales/zh-CN/settings.jsonapps/web/src/client/components/layout/SettingsDialog.tsxapps/web/src/client/components/settings/DaemonPanel.tsxapps/web/src/client/components/ui/switch.tsxapps/web/src/client/lib/accomplish.tsdocs/architecture.mdpackages/agent-core/src/common/types/daemon.tspackages/agent-core/src/daemon/client.tspackages/agent-core/src/daemon/index.tspackages/agent-core/src/daemon/ipc-transport.tspackages/agent-core/src/daemon/pid-lock.tspackages/agent-core/src/daemon/scheduler.tspackages/agent-core/src/daemon/server.tspackages/agent-core/src/daemon/socket-path.tspackages/agent-core/src/daemon/transport.tspackages/agent-core/src/daemon/types.tspackages/agent-core/src/factories/storage.tspackages/agent-core/src/index.tspackages/agent-core/src/storage/migrations/index.tspackages/agent-core/src/storage/migrations/v013-daemon.tspackages/agent-core/src/storage/repositories/appSettings.tspackages/agent-core/src/storage/repositories/index.tspackages/agent-core/src/types/storage.ts
- websocket.ts: wrap message handler in try/catch with shape validation - daemon-bootstrap.ts: register missing task.start/session.resume/permission.respond RPCs - pnpm-lock.yaml: regenerate after adding daemon package Addresses: 2971335381, 2971335383, 2971335385
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/desktop/src/main/daemon-bootstrap.ts (2)
356-369:⚠️ Potential issue | 🟠 Major
requestId-based permission replies are still dropped in fallback mode.This branch logs and returns instead of resolving the outstanding permission request. Any permission flow keyed by
requestIdwill stay blocked whenever the child daemon is unavailable and the app falls back in-process.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon-bootstrap.ts` around lines 356 - 369, The current permission.respond handler returns early when a response contains requestId, which drops requestId-keyed flows in fallback mode; instead, when requestId is present (inside the srv.registerMethod('permission.respond') handler), look up and resolve the outstanding permission request keyed by requestId (use your existing pending-request store/resolver — e.g., the map or function that tracks in-process permission requests) with the provided response, logging if no pending entry exists; retain the existing taskManager.hasActiveTask check and only fall back to the original warning+return if there truly is no pending request to resolve.
297-302:⚠️ Potential issue | 🟠 MajorKeep storage in sync when cancelling queued tasks.
task.cancelQueuedremoves the task from the in-memory queue and returns, but unliketask.cancelit never marks the persisted task ascancelled. After this RPC succeeds, history can still show the task as queued.🐛 Suggested fix
srv.registerMethod('task.cancelQueued', (params) => { if (!params) { return false; } - return taskManager.cancelQueuedTask(params.taskId); + const cancelled = taskManager.cancelQueuedTask(params.taskId); + if (cancelled) { + storage.updateTaskStatus(params.taskId, 'cancelled', new Date().toISOString()); + } + return cancelled; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon-bootstrap.ts` around lines 297 - 302, The handler for 'task.cancelQueued' only removes the task from the in-memory queue (taskManager.cancelQueuedTask) but does not update the persisted task record; change the handler so that after taskManager.cancelQueuedTask(params.taskId) succeeds you also update the persisted task state to "cancelled" (e.g. call the same persistence code used by task.cancel—such as taskManager.markTaskCancelled(params.taskId) or taskStore.updateTaskStatus(params.taskId, 'cancelled')), handle and log any persistence errors, and return a combined success/failure result so history no longer shows the task as queued.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/daemon-bootstrap.ts`:
- Around line 330-337: The handlers call taskManager.startTask(...) before
persisting, which can leave orphaned running tasks if storage.saveTask(...)
later fails; change the flow to create a task record and persist it first, then
start the background work. Specifically, for the 'task.start' and the other
handler (the one at lines 340-351), construct the taskId (using createTaskId()
if needed), build any necessary callbacks (buildInProcessCallbacks(taskId, srv,
storage)) only if they don't require a started task, create the task
metadata/object and call storage.saveTask(task) first, then call
taskManager.startTask(taskId, config, callbacks) and update/save any runtime
fields after start; ensure error handling rolls back or cleans up if startTask
fails. Use the existing symbols taskManager.startTask, storage.saveTask,
createTaskId, buildInProcessCallbacks and srv.registerMethod to locate and
update the handlers.
- Line 58: Replace direct console.log calls that print lifecycle messages (e.g.,
"['DaemonBootstrap'] Running in child-process mode" and the other instances at
the noted positions) with the app's desktop logger so logs go through the
standard formatting/sinks; locate the console.log usages in daemon-bootstrap
(search for the exact message strings) and swap them to use the existing logger
instance (e.g., logger.info(...) or desktopLogger.info(...)), preserving the
original messages and log level, and ensure the logger is imported or referenced
consistently where those console.log calls currently appear.
- Around line 1-444: The file is too large and mixes unrelated responsibilities;
split bootstrapDaemon into smaller modules: extract child-process logic
(spawnDaemonProcess and getDaemonEntryPath + related constants and daemonProcess
state and stdout/stderr forwarding) into a daemon-spawn module, extract
in-process wiring (bootstrapInProcess, registerInProcessHandlers,
buildInProcessCallbacks and any references to DaemonServer/DaemonClient
creation) into an in-process module, and extract lifecycle helpers
(shutdownDaemon, getDaemonClient/getDaemonServer/getDaemonMode and shared state
like client/server/mode) into a small lifecycle module; update the top-level
daemon-bootstrap to orchestrate these modules (importing spawn, in-process
bootstrap, and lifecycle functions) so each new file stays under 200 lines and
preserve function names spawnDaemonProcess, getDaemonEntryPath,
bootstrapInProcess, registerInProcessHandlers, buildInProcessCallbacks,
shutdownDaemon, getDaemonClient, getDaemonServer, and getDaemonMode for
compatibility.
- Around line 95-101: The startup promise must reject immediately if the child
process exits before sending the 'daemon:ready' event; update the exit/close
handler that currently only clears daemonProcess (around where daemonProcess,
timer, and DAEMON_READY_TIMEOUT_MS are used) to call reject(new Error(...))
right away when not yet ready, clear the readiness timeout
(clearTimeout(timer)), and ensure daemonProcess is killed and nulled so the
promise does not hang until DAEMON_READY_TIMEOUT_MS; keep the existing
resolution path on 'daemon:ready' unchanged.
---
Duplicate comments:
In `@apps/desktop/src/main/daemon-bootstrap.ts`:
- Around line 356-369: The current permission.respond handler returns early when
a response contains requestId, which drops requestId-keyed flows in fallback
mode; instead, when requestId is present (inside the
srv.registerMethod('permission.respond') handler), look up and resolve the
outstanding permission request keyed by requestId (use your existing
pending-request store/resolver — e.g., the map or function that tracks
in-process permission requests) with the provided response, logging if no
pending entry exists; retain the existing taskManager.hasActiveTask check and
only fall back to the original warning+return if there truly is no pending
request to resolve.
- Around line 297-302: The handler for 'task.cancelQueued' only removes the task
from the in-memory queue (taskManager.cancelQueuedTask) but does not update the
persisted task record; change the handler so that after
taskManager.cancelQueuedTask(params.taskId) succeeds you also update the
persisted task state to "cancelled" (e.g. call the same persistence code used by
task.cancel—such as taskManager.markTaskCancelled(params.taskId) or
taskStore.updateTaskStatus(params.taskId, 'cancelled')), handle and log any
persistence errors, and return a combined success/failure result so history no
longer shows the task as queued.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87a07586-44d0-420f-ad8d-8d6b5fd73fea
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/daemon/src/websocket.tsapps/desktop/src/main/daemon-bootstrap.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/daemon/src/websocket.ts
added 8 commits
March 22, 2026 18:50
- daemon-bootstrap.ts: replace console.log/warn/error with getLogCollector().logEnv() - daemon-bootstrap.ts: reject promise immediately on pre-ready child process exit - daemon-bootstrap.ts: persist task to storage before calling startTask (task.start + session.resume) Addresses: 2971423605, 2971423607, 2971423609 Skipped : 2971423602
…ashHandlers from agent-core
…ck handlers, ESM fix, error handling - scheduler.ts: replace console.log with structured logger - switch.tsx: rewrite using @radix-ui/react-switch with CVA variants - vitest.config.ts: fix __dirname in ESM context using fileURLToPath - daemon-bootstrap.ts: task.cancelQueued now persists cancelled status to storage - daemon-bootstrap.ts: permission.respond now resolves requestId-based permissions via in-process permission handler (resolvePermission/resolveQuestion) - daemon/types: add task.stop, task.status, health.check to DaemonMethodMap - daemon/types: add task.thought, task.checkpoint, task.error to DaemonNotificationMap - daemon/types: add HealthCheckResult interface - config-generator.ts: add authToken field to ConfigGeneratorOptions - package.json: add ws and @types/ws dependencies to daemon
…cancelScheduledTask return type
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-core/src/index.ts (1)
1-3:⚠️ Potential issue | 🔴 CriticalRemove leftover debug statement.
Line 2 contains a
console.logthat appears to be a sync/test artifact. This violates the coding guideline prohibitingconsole.login production code.🐛 Proposed fix
// ============================================================================= -console.log('[agent-core] u2d sync test'); // `@accomplish/core` - Public API (v0.4.0)As per coding guidelines: "No
console.login production code — use the app's existing logger".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/index.ts` around lines 1 - 3, Remove the leftover debug statement console.log('[agent-core] u2d sync test') from the top-level module; either delete the line or replace it with the project's logger (e.g., call the existing logger.info/debug method used across the app) so no raw console.log remains in packages/agent-core/src/index.ts and ensure any import needed for the logger (the app logger or processLogger) is added if you choose to log instead of removing.
🧹 Nitpick comments (5)
apps/web/src/client/components/settings/DaemonPanel.tsx (1)
78-87: Use the shared UIButtoninstead of a raw<button>.This keeps behavior/theming consistent with the rest of settings UI and avoids duplicating button styling.
As per coding guidelines
apps/web/src/client/components/**/*.{ts,tsx}: "Reuse UI components — checkapps/web/src/client/components/ui/before creating new ones".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/client/components/settings/DaemonPanel.tsx` around lines 78 - 87, Replace the raw <button> in DaemonPanel with the shared UI Button component: import and use Button (the shared component named Button) instead of the HTML button, move the onClick handler (void navigator.clipboard.writeText(socketPath).catch(() => {})) and title prop onto Button, and remove the duplicated className styling so theming/behavior come from the shared Button; ensure the Button usage preserves the same text ("Copy") and accessibility attributes.packages/agent-core/src/daemon/rpc-server.ts (1)
205-212: Consider using a static import fornode:fs/promises.The dynamic import is functional but unnecessary since this module always needs
unlink. A static import at the top of the file would be cleaner and more consistent.♻️ Suggested refactor
import { createServer, type Server, type Socket } from 'node:net'; import { randomUUID } from 'node:crypto'; +import { unlink } from 'node:fs/promises'; import type { JsonRpcMessage, JsonRpcRequest, JsonRpcResponse } from '../common/types/daemon.js';Then simplify the method:
private async removeStaleSocket(): Promise<void> { - const { unlink } = await import('node:fs/promises'); try { await unlink(this.socketPath); } catch { // File doesn't exist — that's fine } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/daemon/rpc-server.ts` around lines 205 - 212, The method removeStaleSocket currently does a dynamic import of 'node:fs/promises' for unlink; replace that with a static import (import { unlink } from 'node:fs/promises') at the top of the file and remove the dynamic import inside removeStaleSocket so the method simply calls await unlink(this.socketPath) inside the existing try/catch (catching and ignoring ENOENT/non‑existent file cases) — update references to unlink and keep socketPath usage intact.apps/daemon/src/index.ts (2)
70-90: Consider using the daemon logger instead of direct console calls.The daemon subsystem includes a logger at
packages/agent-core/src/daemon/logger.ts. Using it here would provide consistent formatting and the ability to control debug output viaDEBUGenvironment variable.♻️ Suggested approach
import { DaemonRpcServer, getSocketPath, acquirePidLock, installCrashHandlers, + logger, type PidLockHandle, ... } from '@accomplish_ai/agent-core'; - console.log('[Daemon] Starting...'); + logger.info('Starting...'); - console.log(`[Daemon] PID lock acquired: ${pidLock.pidPath} (pid=${process.pid})`); + logger.info(`PID lock acquired: ${pidLock.pidPath} (pid=${process.pid})`); - console.warn(`[Daemon] Crash recovery: marking stale task ${task.id} as failed`); + logger.warn(`Crash recovery: marking stale task ${task.id} as failed`);Apply similar changes throughout the file.
As per coding guidelines: "No
console.login production code — use the app's existing logger"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/index.ts` around lines 70 - 90, Replace direct console calls in this startup sequence with the daemon logger: import and use the logger exported by the daemon logger module and call its methods instead of console.log/console.warn (e.g., use logger.info(`[Daemon] Starting...`), logger.info when reporting PID lock from acquirePidLock() and logger.debug or info for authToken generation, and logger.warn for crash-recovery messages). Update the block that uses pidLock/acquirePidLock, the authToken generation, and the StorageService flow (storage.initialize, storage.getTasks, storage.updateTaskStatus) to log via the daemon logger consistently and remove console.* usage throughout the file.
1-373: Consider splitting this file for maintainability.At 373 lines, this file exceeds the 200-line guideline significantly. While it's a cohesive entrypoint, consider extracting:
- RPC method registration (lines 129-242) into a separate
rpc-methods.tsmodule- Event forwarding setup (lines 244-273) into an
event-handlers.tsmoduleThis would make the main function easier to follow and test individual components.
As per coding guidelines: "New files must be < 200 lines — split into logical modules if needed"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/index.ts` around lines 1 - 373, This file is too large; extract the RPC registration block around rpc.registerMethod(...) (uses safeHandler, taskStartSchema, taskIdSchema, permissionResponseSchema, resumeSessionSchema, validate, taskService, storage, permissionService, thoughtStreamService, healthService) into a new module (e.g., rpc-methods.ts) that exports a function like registerRpcMethods(rpc, taskService, storage, permissionService, thoughtStreamService, healthService) which wires up all rpc.registerMethod calls and returns any teardown if needed; likewise extract the event forwarding block that calls taskService.on(...) and thoughtStreamService.unregisterTask/registerTask into a new module (e.g., event-handlers.ts) that exports setupEventHandlers(taskService, thoughtStreamService, rpc, healthService) and attaches those listeners. Update main() to call the two new functions and ensure imports for safeHandler and schemas are moved into rpc-methods.ts so the top-level file stays under 200 lines.apps/daemon/src/thought-stream-service.ts (1)
15-31: Consider if empty strings should be allowed forcontentandsummary.The
taskIdfield requires.min(1)butcontent(line 17) andsummary(line 26) accept empty strings. If empty content/summary is invalid, add.min(1)for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/daemon/src/thought-stream-service.ts` around lines 15 - 31, The schemas allow empty strings for content and summary while taskId enforces non-empty; update thoughtEventSchema and checkpointEventSchema to require non-empty values by adding .min(1) to the content property in thoughtEventSchema and to the summary property in checkpointEventSchema (i.e., change content: z.string() -> z.string().min(1) and summary: z.string() -> z.string().min(1)) so they consistently reject empty strings; ensure no other logic relies on empty content/summary before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/daemon/server.ts`:
- Around line 1-231: The file is over the repo's 200-line limit for new TS
files; split it into smaller modules by extracting logical pieces such as socket
path and utilities (getSocketPath, isNotification), JSON-RPC request/response
types and handleLine (and MethodHandler map), and server lifecycle
(startDaemonServer, stopDaemonServer) into separate files; update
imports/exports so startDaemonServer and stopDaemonServer remain public from the
original module, move method registration (registerMethod and methodHandlers)
alongside handleLine into a handlers module, and keep net/fs/electron usage
localized to a daemon-server module that composes the pieces and performs
listen/cleanup.
- Line 203: Replace the direct console.log calls with the app's structured
logger: import or use the existing logger instance (e.g., logger or appLogger)
and call logger.info(...) instead of console.log for the socket server messages
— specifically replace console.log(`[Daemon] Socket server listening at
${socketPath}`) and the similar occurrence around line 231 with
logger.info(`[Daemon] Socket server listening at ${socketPath}`) (or an
equivalent structured log call), ensuring no console.log calls remain in this
file.
- Around line 167-179: The newline-framing buffer in the socket data handler can
grow unbounded; add a maximum allowed buffer length (e.g., MAX_BUFFER_SIZE) and
enforce it inside the socket.on('data') callback that accumulates into buffer:
after appending data.toString(), check buffer.length and if it exceeds
MAX_BUFFER_SIZE either drop/ destroy the socket (socket.destroy()/socket.end())
and log an error or truncate the buffer and continue, then proceed to split on
'\n' and call handleLine(trimmed, socket) as before; add the constant
(MAX_BUFFER_SIZE) near the top of this module and ensure any early termination
is graceful and logged.
- Around line 89-102: The parsed object check is too loose and malformed
JSON-RPC objects (e.g., missing jsonrpc, non-string method, invalid id types)
fall through to "Method not found"; update the validation after parsing to
assert request.jsonrpc === '2.0', typeof request.method === 'string' (unless
it's a notification), and that request.id (when present) is a string, number, or
null; reuse the existing JsonRpcResponse shape and socket.write to return the
same Invalid Request error when these validations fail, and keep using
isNotification(request) to decide whether method may be omitted for
notifications.
In `@apps/web/src/client/components/settings/DaemonPanel.tsx`:
- Around line 11-20: The current useEffect in DaemonPanel swallows errors from
accomplish.getRunInBackground() and accomplish.getDaemonSocketPath() by using
.catch(() => {}); update it to handle errors: replace the empty catches with
calls that log the error (using the app logger used elsewhere in the app) and
optionally set a local error state to show a small hint in the panel; target the
useEffect block and the promises from accomplish.getRunInBackground and
accomplish.getDaemonSocketPath and ensure you still call
setRunInBackground/setSocketPath on success but on failure log the error
(including the error object) and set an error flag/state that DaemonPanel can
render.
- Around line 22-31: The handler handleToggle currently uses try/finally without
catching errors, so if accomplish.setRunInBackground(next) rejects the promise
bubbles and the user gets no feedback; update handleToggle to catch errors from
accomplish.setRunInBackground(next) (e.g., add a catch block or use try/catch)
and on failure revert any optimistic UI changes (do not call
setRunInBackground(next) or reset it), setSaving(false) in finally, and surface
an error to the user (via existing error state, a toast, or an onError
callback). Reference: handleToggle, accomplish.setRunInBackground,
setRunInBackground, setSaving.
In `@apps/web/src/client/components/ui/switch.tsx`:
- Around line 37-55: The custom onChange prop in SwitchProps and the Switch
component drops Radix's boolean payload and conflicts with Radix's
onCheckedChange; remove the onChange?: () => void declaration from SwitchProps,
stop destructuring onChange in the Switch component, and instead accept/use
Radix's onCheckedChange: (checked: boolean) => void (or simply rely on the
inherited prop) and pass it directly to SwitchPrimitive.Root as
onCheckedChange={onCheckedChange}; keep existing ariaLabel mapping and other
props unchanged and ensure the forwarded ref and switchVariants usage remain the
same.
In `@packages/agent-core/src/daemon/types.ts`:
- Around line 120-133: Remove the unused and divergent RPC type definitions by
deleting the RpcMethodMap, RpcMethod, RpcNotificationMap, and
RpcNotificationType declarations from the daemon/types.ts module; ensure any
references in that file to those symbols are removed or replaced with the
canonical DaemonMethodMap/DaemonNotification types (from common/types/daemon.ts)
and export nothing else that reintroduces the conflicting types so the codebase
uses DaemonMethodMap for RPC method typing consistently.
---
Outside diff comments:
In `@packages/agent-core/src/index.ts`:
- Around line 1-3: Remove the leftover debug statement console.log('[agent-core]
u2d sync test') from the top-level module; either delete the line or replace it
with the project's logger (e.g., call the existing logger.info/debug method used
across the app) so no raw console.log remains in
packages/agent-core/src/index.ts and ensure any import needed for the logger
(the app logger or processLogger) is added if you choose to log instead of
removing.
---
Nitpick comments:
In `@apps/daemon/src/index.ts`:
- Around line 70-90: Replace direct console calls in this startup sequence with
the daemon logger: import and use the logger exported by the daemon logger
module and call its methods instead of console.log/console.warn (e.g., use
logger.info(`[Daemon] Starting...`), logger.info when reporting PID lock from
acquirePidLock() and logger.debug or info for authToken generation, and
logger.warn for crash-recovery messages). Update the block that uses
pidLock/acquirePidLock, the authToken generation, and the StorageService flow
(storage.initialize, storage.getTasks, storage.updateTaskStatus) to log via the
daemon logger consistently and remove console.* usage throughout the file.
- Around line 1-373: This file is too large; extract the RPC registration block
around rpc.registerMethod(...) (uses safeHandler, taskStartSchema, taskIdSchema,
permissionResponseSchema, resumeSessionSchema, validate, taskService, storage,
permissionService, thoughtStreamService, healthService) into a new module (e.g.,
rpc-methods.ts) that exports a function like registerRpcMethods(rpc,
taskService, storage, permissionService, thoughtStreamService, healthService)
which wires up all rpc.registerMethod calls and returns any teardown if needed;
likewise extract the event forwarding block that calls taskService.on(...) and
thoughtStreamService.unregisterTask/registerTask into a new module (e.g.,
event-handlers.ts) that exports setupEventHandlers(taskService,
thoughtStreamService, rpc, healthService) and attaches those listeners. Update
main() to call the two new functions and ensure imports for safeHandler and
schemas are moved into rpc-methods.ts so the top-level file stays under 200
lines.
In `@apps/daemon/src/thought-stream-service.ts`:
- Around line 15-31: The schemas allow empty strings for content and summary
while taskId enforces non-empty; update thoughtEventSchema and
checkpointEventSchema to require non-empty values by adding .min(1) to the
content property in thoughtEventSchema and to the summary property in
checkpointEventSchema (i.e., change content: z.string() -> z.string().min(1) and
summary: z.string() -> z.string().min(1)) so they consistently reject empty
strings; ensure no other logic relies on empty content/summary before
committing.
In `@apps/web/src/client/components/settings/DaemonPanel.tsx`:
- Around line 78-87: Replace the raw <button> in DaemonPanel with the shared UI
Button component: import and use Button (the shared component named Button)
instead of the HTML button, move the onClick handler (void
navigator.clipboard.writeText(socketPath).catch(() => {})) and title prop onto
Button, and remove the duplicated className styling so theming/behavior come
from the shared Button; ensure the Button usage preserves the same text ("Copy")
and accessibility attributes.
In `@packages/agent-core/src/daemon/rpc-server.ts`:
- Around line 205-212: The method removeStaleSocket currently does a dynamic
import of 'node:fs/promises' for unlink; replace that with a static import
(import { unlink } from 'node:fs/promises') at the top of the file and remove
the dynamic import inside removeStaleSocket so the method simply calls await
unlink(this.socketPath) inside the existing try/catch (catching and ignoring
ENOENT/non‑existent file cases) — update references to unlink and keep
socketPath usage intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: edb48a4f-1b2a-4da6-9d9c-1c8cd54772ad
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
apps/daemon/__tests__/unit/health.test.tsapps/daemon/__tests__/unit/http-server-factory.test.tsapps/daemon/__tests__/unit/rate-limiter.test.tsapps/daemon/package.jsonapps/daemon/src/cli.tsapps/daemon/src/daemon-options.tsapps/daemon/src/health.tsapps/daemon/src/http-server-factory.tsapps/daemon/src/index.tsapps/daemon/src/mcp-bridges.tsapps/daemon/src/permission-service.tsapps/daemon/src/pid.tsapps/daemon/src/rate-limiter.tsapps/daemon/src/storage-service.tsapps/daemon/src/task-service.tsapps/daemon/src/thought-stream-service.tsapps/daemon/tsconfig.jsonapps/daemon/tsup.config.tsapps/daemon/vitest.config.tsapps/desktop/__tests__/unit/main/daemon/scheduler.unit.test.tsapps/desktop/__tests__/unit/main/daemon/server.unit.test.tsapps/desktop/e2e/specs/daemon.spec.tsapps/desktop/src/main/daemon-bootstrap.tsapps/desktop/src/main/daemon/cli-bridge.tsapps/desktop/src/main/daemon/entry.tsapps/desktop/src/main/daemon/server.tsapps/desktop/src/main/daemon/service-manager.tsapps/desktop/src/preload/index.tsapps/web/package.jsonapps/web/src/client/components/settings/DaemonPanel.tsxapps/web/src/client/components/ui/switch.tsxdocs/architecture.mdpackages/agent-core/src/common/types/daemon.tspackages/agent-core/src/daemon/client.tspackages/agent-core/src/daemon/crash-handlers.tspackages/agent-core/src/daemon/index.tspackages/agent-core/src/daemon/ipc-transport.tspackages/agent-core/src/daemon/logger.tspackages/agent-core/src/daemon/pid-lock.tspackages/agent-core/src/daemon/rpc-server.tspackages/agent-core/src/daemon/scheduler.tspackages/agent-core/src/daemon/server.tspackages/agent-core/src/daemon/socket-path.tspackages/agent-core/src/daemon/transport.tspackages/agent-core/src/daemon/types.tspackages/agent-core/src/index.tspackages/agent-core/src/opencode/config-generator.tspackages/agent-core/src/storage/migrations/v013-daemon.ts
✅ Files skipped from review due to trivial changes (17)
- apps/web/package.json
- packages/agent-core/src/opencode/config-generator.ts
- apps/daemon/vitest.config.ts
- apps/daemon/tsup.config.ts
- apps/daemon/src/health.ts
- apps/daemon/tsconfig.json
- apps/daemon/tests/unit/health.test.ts
- apps/daemon/package.json
- apps/daemon/tests/unit/http-server-factory.test.ts
- apps/daemon/src/rate-limiter.ts
- apps/desktop/tests/unit/main/daemon/server.unit.test.ts
- apps/desktop/tests/unit/main/daemon/scheduler.unit.test.ts
- packages/agent-core/src/daemon/server.ts
- packages/agent-core/src/daemon/pid-lock.ts
- packages/agent-core/src/common/types/daemon.ts
- apps/daemon/src/task-service.ts
- apps/daemon/src/http-server-factory.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/desktop/src/preload/index.ts
- packages/agent-core/src/storage/migrations/v013-daemon.ts
- apps/daemon/tests/unit/rate-limiter.test.ts
- apps/desktop/src/main/daemon/entry.ts
- packages/agent-core/src/daemon/socket-path.ts
- packages/agent-core/src/daemon/transport.ts
- apps/daemon/src/pid.ts
- apps/daemon/src/storage-service.ts
- apps/desktop/e2e/specs/daemon.spec.ts
- packages/agent-core/src/daemon/ipc-transport.ts
- docs/architecture.md
- apps/desktop/src/main/daemon/service-manager.ts
- apps/daemon/src/cli.ts
- apps/desktop/src/main/daemon/cli-bridge.ts
- apps/daemon/src/daemon-options.ts
- apps/daemon/src/mcp-bridges.ts
- apps/desktop/src/main/daemon-bootstrap.ts
- packages/agent-core/src/daemon/index.ts
- packages/agent-core/src/daemon/scheduler.ts
added 5 commits
March 23, 2026 10:17
- Extract RPC dispatch logic from server.ts to rpc-dispatcher.ts to keep server.ts under 200 lines and focused on socket lifecycle (fix #2972626290) - Add isValidJsonRpcRequest type guard returning -32600 Invalid Request for malformed JSON-RPC envelopes (fix #2972626292) - Add MAX_SOCKET_BUFFER_BYTES (1MB) guard with socket.destroy on overflow (fix #2972626296) - Replace all console.log/warn/error with getLogCollector() app logger in server.ts and rpc-dispatcher.ts (fix #2972626299) - Log instead of silently swallow daemon init errors in DaemonPanel (fix #2972626303) - Add explicit catch block in handleToggle to log save failures and keep local state unchanged on error (fix #2972626307) - Rename Switch onChange prop to onCheckedChange to align with Radix Switch.Root API, update DaemonPanel caller (fix #2972626318) - Consolidate RpcMethodMap/DaemonMethodMap by aliasing RpcMethodMap to DaemonMethodMap to prevent type divergence (fix #2972626325)
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
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)
packages/agent-core/src/index.ts (1)
2-2:⚠️ Potential issue | 🟡 MinorRemove debug console.log from production code.
Line 2 contains a debug statement that should not be in production code.
-console.log('[agent-core] u2d sync test');As per coding guidelines
**/{apps,packages}/**/*.{js,ts,tsx}: Noconsole.login production code — use the app's existing logger.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-core/src/index.ts` at line 2, Remove the debug console.log('[agent-core] u2d sync test') from packages/agent-core/src/index.ts and replace it with the project's structured logger: import and use the existing logger instance (e.g., logger.debug or processLogger.debug) instead of console.log, or simply delete the statement if no logging is required; ensure you add the appropriate import for the logger and no console.log calls remain in this module.
🤖 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 `@packages/agent-core/src/index.ts`:
- Line 2: Remove the debug console.log('[agent-core] u2d sync test') from
packages/agent-core/src/index.ts and replace it with the project's structured
logger: import and use the existing logger instance (e.g., logger.debug or
processLogger.debug) instead of console.log, or simply delete the statement if
no logging is required; ensure you add the appropriate import for the logger and
no console.log calls remain in this module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2dbe7e71-f5ca-458b-aec5-e229ecc2539f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/desktop/__tests__/unit/main/daemon/server.unit.test.tsapps/desktop/e2e/docker/Dockerfileapps/desktop/src/main/daemon/rpc-dispatcher.tsapps/desktop/src/main/daemon/server.tsapps/desktop/src/main/index.tsapps/desktop/src/main/ipc/task-callbacks.tsapps/desktop/src/preload/index.tsapps/web/locales/en/settings.jsonapps/web/locales/zh-CN/settings.jsonapps/web/package.jsonapps/web/src/client/components/settings/DaemonPanel.tsxapps/web/src/client/components/ui/switch.tsxapps/web/src/client/lib/accomplish.tspackages/agent-core/src/daemon/types.tspackages/agent-core/src/index.ts
✅ Files skipped from review due to trivial changes (5)
- apps/web/locales/en/settings.json
- apps/desktop/e2e/docker/Dockerfile
- apps/web/package.json
- apps/web/src/client/components/settings/DaemonPanel.tsx
- apps/web/locales/zh-CN/settings.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/src/client/lib/accomplish.ts
- apps/web/src/client/components/ui/switch.tsx
- packages/agent-core/src/daemon/types.ts
- apps/desktop/src/main/ipc/task-callbacks.ts
…tialized LogCollector in tests
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/daemon/rpc-dispatcher.ts`:
- Around line 132-142: The RPC error path logs the full exception via safeLog
but then includes the raw error string in the JsonRpcResponse (constructed as
JsonRpcResponse with jsonrpc/id/error) which can leak internal details; change
the response to return a sanitized generic message (e.g., "Internal error") and
optionally a non-sensitive metadata field such as errorType or a correlation id
instead of err.message, while keeping the full err message only in safeLog;
update the code around safeLog(...) and the JsonRpcResponse construction in
rpc-dispatcher.ts (look for variables method, id, err and the JsonRpcResponse
object) to remove err.message/String(err) from the response payload and replace
it with safe metadata.
- Around line 13-26: The safeLog function currently falls back to console.* and
uses braceless single-line branches; remove any direct console.* usage in
production by replacing the console fallback with the app logger (use
getLogCollector() return value or a no-op logger) so all logs always go through
logger.log (or are dropped), and add braces to every if/else block in this file
(including the branches inside safeLog and the other braceless blocks around
lines referenced 55–60) to satisfy the curly rule; refer to the safeLog function
and any other braceless conditional blocks in rpc-dispatcher.ts when making the
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66e0c953-ba68-46d9-8fa7-a22e2e20513c
📒 Files selected for processing (2)
apps/desktop/src/main/daemon/rpc-dispatcher.tsapps/desktop/src/main/daemon/server.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/main/daemon/server.ts
…fs, separate start/persist failures
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/desktop/src/main/daemon/daemon-inprocess-handlers.ts (2)
128-139:⚠️ Potential issue | 🟠 MajorDon’t mark resumed tasks as failed on post-start persistence errors.
session.resumestill wrapsstartTaskand storage writes in the sametry/catch, so a storage failure after successful start can incorrectly set status tofailed.Suggested fix
- try { - const task = await taskManager.startTask(taskId, { prompt, sessionId }, callbacks); - if (existingTaskId) { - storage.updateTaskStatus(existingTaskId, task.status, new Date().toISOString()); - } else { - storage.saveTask(task); - } - return task; - } catch (err) { - storage.updateTaskStatus(taskId, 'failed', new Date().toISOString()); - throw err; - } + let task; + try { + task = await taskManager.startTask(taskId, { prompt, sessionId }, callbacks); + } catch (err) { + storage.updateTaskStatus(taskId, 'failed', new Date().toISOString()); + throw err; + } + + try { + if (existingTaskId) { + storage.updateTaskStatus(existingTaskId, task.status, new Date().toISOString()); + } else { + storage.saveTask(task); + } + } catch { + // post-start persistence failure — task is running, don't mark as failed + } + return task;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/daemon-inprocess-handlers.ts` around lines 128 - 139, The current try/catch wraps taskManager.startTask and the subsequent storage writes so a storage write error after a successful start can mark a resumed task as 'failed'; change the flow in daemon-inprocess-handlers.ts so you first await taskManager.startTask(taskId, { prompt, sessionId }, callbacks) inside its own try/catch and only set status to 'failed' there if startTask throws, then perform storage.saveTask(task) or storage.updateTaskStatus(existingTaskId, ...) in a separate try/catch that handles/publishes persistence errors without mutating the task status for resumed tasks (i.e., do not call storage.updateTaskStatus(taskId, 'failed', ...) on storage write failures); refer to taskManager.startTask, storage.saveTask, storage.updateTaskStatus, and existingTaskId/session.resume to locate and implement this change.
33-35:⚠️ Potential issue | 🟠 MajorWrap the remaining single-line conditionals in braces.
There are still multiple braceless
ifstatements in this file, which will keep failing the enforcedcurlylint rule.As per coding guidelines "
**/*.{ts,tsx,js,jsx}: Always use braces forif/else/for/whilestatements — no single-line braceless statements (enforced bycurlyESLint rule)".Also applies to: 56-59, 75-93, 150-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/daemon/daemon-inprocess-handlers.ts` around lines 33 - 35, Several single-line braceless conditionals remain (e.g., in the srv.registerMethod('task.delete', (params) => { if (params) storage.deleteTask(params.taskId); }); handler) and must be changed to use braces to satisfy the `curly` lint rule; update each braceless conditional (including similar patterns around the other registerMethod handlers referenced) to a block form like: if (params) { storage.deleteTask(params.taskId); } (apply the same change to all other single-line if/else statements in this file such as the handlers covering the other mentioned ranges).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/__tests__/unit/main/daemon/helpers/server.helpers.ts`:
- Around line 15-32: Update sendJsonRpc and sendRawLine to robustly manage the
socket lifecycle and guard JSON parsing: add handlers for the socket 'close'
event (in addition to 'error' and timeout) to reject the promise if the
connection ends before a response, introduce a boolean "settled" flag to prevent
multiple resolve/reject calls, wrap JSON.parse(line) in try/catch and reject the
promise on parse errors instead of throwing, and replace any require('net')
usage in sendRawLine with the imported net variable; ensure each handler (data,
error, close, timeout) checks/sets the settled flag and properly destroys/ends
the socket when finishing.
- Line 40: The code uses require('net').createConnection at the socket
connection site; change that to use the already-imported ES module variable net
(i.e., replace require('net').createConnection(...) with
net.createConnection(...)) so the call in the connection callback uses the
existing net identifier and keeps import style consistent (look for the
createConnection call around the client variable initialization).
In `@apps/desktop/src/main/daemon/daemon-inprocess-handlers.ts`:
- Around line 97-99: The RPC handlers registered for 'task.start' and
'session.resume' destructure params directly (e.g., const { taskId:
providedTaskId, config } = params) which will throw on null/malformed JSON-RPC
params; update both handlers in srv.registerMethod to first validate that params
is a non-null object (and has expected keys/types) before destructuring, and
return or throw a controlled RPC error (with a clear message) when validation
fails so malformed input doesn't cause an uncaught exception; keep references to
the existing helpers like createTaskId() and the same parameter names when
applying the guarded destructuring.
---
Duplicate comments:
In `@apps/desktop/src/main/daemon/daemon-inprocess-handlers.ts`:
- Around line 128-139: The current try/catch wraps taskManager.startTask and the
subsequent storage writes so a storage write error after a successful start can
mark a resumed task as 'failed'; change the flow in daemon-inprocess-handlers.ts
so you first await taskManager.startTask(taskId, { prompt, sessionId },
callbacks) inside its own try/catch and only set status to 'failed' there if
startTask throws, then perform storage.saveTask(task) or
storage.updateTaskStatus(existingTaskId, ...) in a separate try/catch that
handles/publishes persistence errors without mutating the task status for
resumed tasks (i.e., do not call storage.updateTaskStatus(taskId, 'failed', ...)
on storage write failures); refer to taskManager.startTask, storage.saveTask,
storage.updateTaskStatus, and existingTaskId/session.resume to locate and
implement this change.
- Around line 33-35: Several single-line braceless conditionals remain (e.g., in
the srv.registerMethod('task.delete', (params) => { if (params)
storage.deleteTask(params.taskId); }); handler) and must be changed to use
braces to satisfy the `curly` lint rule; update each braceless conditional
(including similar patterns around the other registerMethod handlers referenced)
to a block form like: if (params) { storage.deleteTask(params.taskId); } (apply
the same change to all other single-line if/else statements in this file such as
the handlers covering the other mentioned ranges).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77ca9b09-4ab5-4eed-9f18-60b9b6d74a55
📒 Files selected for processing (7)
apps/desktop/__tests__/unit/main/daemon/helpers/server.helpers.tsapps/desktop/__tests__/unit/main/daemon/server.unit.test.tsapps/desktop/src/main/daemon/daemon-inprocess-handlers.tsapps/desktop/src/main/ipc/task-callbacks.tsapps/desktop/src/preload/index.tsapps/web/src/client/lib/accomplish.tspackages/agent-core/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/client/lib/accomplish.ts
- apps/desktop/tests/unit/main/daemon/server.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/preload/index.ts
- apps/desktop/src/main/ipc/task-callbacks.ts
…}, customText preserved, scoped cleanup in daemon-spawn, fix import in task-callbacks, update lockfile
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/daemon/rpc-dispatcher.ts`:
- Around line 77-150: The async response writes in handleLine (where writeFn is
invoked in the Promise .then() and .catch() branches) must be hardened: create
or use a safeWrite wrapper that first checks the underlying stream/socket
writable state (e.g., socket.writable) before attempting to write, wraps the
writeFn call in try/catch to handle synchronous throws (ERR_STREAM_DESTROYED),
handles a false return by waiting for a 'drain' or failing gracefully, and
listens for asynchronous 'error' events so writes don't crash the process;
replace direct calls to writeFn(JSON.stringify(...)+ '\n') in handleLine (and in
the handler-not-found/error branches) with calls to this safeWrite helper and
ensure notifications still quietly drop if the socket is not writable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f94c6b2f-b82d-44f9-a997-f39892ca8a21
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/desktop/src/main/daemon/daemon-inprocess-permissions.tsapps/desktop/src/main/daemon/daemon-spawn.tsapps/desktop/src/main/daemon/rpc-dispatcher.tsapps/desktop/src/main/ipc/task-callbacks.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/main/ipc/task-callbacks.ts
- apps/desktop/src/main/daemon/daemon-inprocess-permissions.ts
…e, safeWrite in rpc-dispatcher, socket.writable check in server
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This was referenced Mar 23, 2026
11 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Mar 23, 2026
…nsolidated) (#770) * feat(daemon): core JSON-RPC daemon infrastructure (ENG-694) From PR #626 by david-mamani (David Abdiel Mamani Chuquimamani): - packages/agent-core/src/common/types/daemon.ts: daemon protocol types - packages/agent-core/src/daemon/server.ts: DaemonServer JSON-RPC dispatcher - packages/agent-core/src/daemon/client.ts: DaemonClient typed RPC caller - packages/agent-core/src/daemon/transport.ts: in-process transport pair - packages/agent-core/src/daemon/ipc-transport.ts: child_process IPC transport - packages/agent-core/src/daemon/scheduler.ts: cron-based task scheduler - packages/agent-core/src/daemon/index.ts: public exports - apps/desktop/src/main/daemon-bootstrap.ts: bootstrap daemon lifecycle - apps/desktop/src/main/daemon/cli-bridge.ts: CLI JSON-RPC bridge - apps/desktop/src/main/daemon/entry.ts: daemon entry point - apps/desktop/src/main/daemon/service-manager.ts: auto-start service - apps/desktop/src/main/tray.ts: system tray with task count - apps/desktop/e2e/specs/daemon.spec.ts: E2E tests Closes #402 Ref: ENG-694 * feat(daemon): UI settings panel, socket server, background mode (ENG-694) From PR #613 by SaaiAravindhRaja / ChaiAndCode: - apps/desktop/src/main/daemon/server.ts: Unix socket / named pipe server with JSON-RPC 2.0, supports daemon.ping, task.start, task.schedule, etc. - apps/desktop/src/main/ipc/task-callbacks.ts: add DaemonTaskCallbacksOptions and createDaemonTaskCallbacks() for background task execution with system notifications when window is hidden - apps/desktop/__tests__/unit/main/daemon/scheduler.unit.test.ts: scheduler tests - apps/desktop/__tests__/unit/main/daemon/server.unit.test.ts: server unit tests - apps/web/src/client/components/settings/DaemonPanel.tsx: Settings UI panel with background mode toggle and socket path display - apps/web/src/client/components/ui/switch.tsx: accessible Switch component - apps/web/locales/en|zh-CN/settings.json: add 'daemon' tab i18n key - apps/web/src/client/components/layout/SettingsDialog.tsx: add Daemon tab - apps/web/src/client/lib/accomplish.ts: add daemon API interface methods - apps/desktop/src/preload/index.ts: expose daemon IPC methods to renderer - apps/desktop/src/main/index.ts: integrate daemon bootstrap + tray init - apps/desktop/src/main/ipc/handlers/settings-handlers.ts: IPC handlers for daemon:get-run-in-background, daemon:set-run-in-background, daemon:get-socket-path - packages/agent-core/src/storage/migrations/v013-daemon.ts: DB migration to add run_in_background column to app_settings - packages/agent-core/src/storage/repositories/appSettings.ts: add getRunInBackground / setRunInBackground CRUD - packages/agent-core/src/types/storage.ts: extend AppSettings + AppSettingsAPI Ref: ENG-694 * feat(daemon): standalone HTTP daemon app with rate limiting and services (ENG-694) From PR #598 by aryan877 (Aryan): - apps/daemon/package.json: @accomplish/daemon standalone app config - apps/daemon/src/index.ts: daemon entry point with HTTP server - apps/daemon/src/cli.ts: CLI argument parsing and startup - apps/daemon/src/health.ts: health check endpoint (/health) - apps/daemon/src/http-server-factory.ts: HTTP server with auth token validation (timing-safe), request size limits, route dispatch - apps/daemon/src/rate-limiter.ts: sliding window rate limiter per IP - apps/daemon/src/permission-service.ts: permission request management - apps/daemon/src/storage-service.ts: persistent storage service - apps/daemon/src/task-service.ts: task creation and management - apps/daemon/src/thought-stream-service.ts: thought stream events - apps/daemon/tsconfig.json: TypeScript config for daemon app - apps/daemon/tsup.config.ts: tsup build config - apps/daemon/vitest.config.ts: vitest test config - apps/daemon/__tests__/unit/health.test.ts: health endpoint tests - apps/daemon/__tests__/unit/http-server-factory.test.ts: server factory tests - apps/daemon/__tests__/unit/rate-limiter.test.ts: rate limiter tests - docs/architecture.md: daemon architecture documentation Key features: Bearer token auth, 1MB request body limit, 429 rate limiting, per-IP sliding window with automatic cleanup. Ref: ENG-694 * fix: address CodeRabbit comments on PR #770 - websocket.ts: wrap message handler in try/catch with shape validation - daemon-bootstrap.ts: register missing task.start/session.resume/permission.respond RPCs - pnpm-lock.yaml: regenerate after adding daemon package Addresses: 2971335381, 2971335383, 2971335385 * fix: address CodeRabbit comments on PR #770 (round 2) - daemon-bootstrap.ts: replace console.log/warn/error with getLogCollector().logEnv() - daemon-bootstrap.ts: reject promise immediately on pre-ready child process exit - daemon-bootstrap.ts: persist task to storage before calling startTask (task.start + session.resume) Addresses: 2971423605, 2971423607, 2971423609 Skipped : 2971423602 * fix: export DaemonRpcServer, getSocketPath, acquirePidLock, installCrashHandlers from agent-core * fix: address CodeRabbit review comments for PR #770 - exports, fallback handlers, ESM fix, error handling - scheduler.ts: replace console.log with structured logger - switch.tsx: rewrite using @radix-ui/react-switch with CVA variants - vitest.config.ts: fix __dirname in ESM context using fileURLToPath - daemon-bootstrap.ts: task.cancelQueued now persists cancelled status to storage - daemon-bootstrap.ts: permission.respond now resolves requestId-based permissions via in-process permission handler (resolvePermission/resolveQuestion) - daemon/types: add task.stop, task.status, health.check to DaemonMethodMap - daemon/types: add task.thought, task.checkpoint, task.error to DaemonNotificationMap - daemon/types: add HealthCheckResult interface - config-generator.ts: add authToken field to ConfigGeneratorOptions - package.json: add ws and @types/ws dependencies to daemon * fix: use double cast for placeholder task in daemon-bootstrap to satisfy TypeScript * style: apply prettier formatting to pre-existing daemon files * fix: export parseCronField and matchesCron, add cron validation, fix cancelScheduledTask return type * fix: implement cron OR semantics for dom/dow in matchesCron * fix: add apps/daemon/package.json to E2E Docker build context * fix: make daemon server unit tests platform-aware (Windows named pipe timing) * fix: update pnpm-lock.yaml after merge * fix: address CodeRabbit review comments on daemon architecture - Extract RPC dispatch logic from server.ts to rpc-dispatcher.ts to keep server.ts under 200 lines and focused on socket lifecycle (fix #2972626290) - Add isValidJsonRpcRequest type guard returning -32600 Invalid Request for malformed JSON-RPC envelopes (fix #2972626292) - Add MAX_SOCKET_BUFFER_BYTES (1MB) guard with socket.destroy on overflow (fix #2972626296) - Replace all console.log/warn/error with getLogCollector() app logger in server.ts and rpc-dispatcher.ts (fix #2972626299) - Log instead of silently swallow daemon init errors in DaemonPanel (fix #2972626303) - Add explicit catch block in handleToggle to log save failures and keep local state unchanged on error (fix #2972626307) - Rename Switch onChange prop to onCheckedChange to align with Radix Switch.Root API, update DaemonPanel caller (fix #2972626318) - Consolidate RpcMethodMap/DaemonMethodMap by aliasing RpcMethodMap to DaemonMethodMap to prevent type divergence (fix #2972626325) * fix: add 'daemon' to LogSource union type * fix: add 'daemon' to LOG_SOURCE_PATTERNS * fix: safe logger fallback in daemon server/dispatcher — handles uninitialized LogCollector in tests * fix: skip socket-based daemon tests on Windows (named pipe timing) * fix(ENG-1002): split daemon-bootstrap.ts into daemon-spawn, daemon-inprocess, daemon-lifecycle (#784) Co-authored-by: Avishay Maor <avishaym@tikalk.com> * fix: remove console fallback in safeLog, sanitize RPC error response data * fix: update test expectation to match sanitized RPC error response * fix: split server.unit.test.ts helper, fix TOCTOU cancel, braceless ifs, separate start/persist failures * fix: braceless ifs in isValidJsonRpcRequest, don't coerce params to {}, customText preserved, scoped cleanup in daemon-spawn, fix import in task-callbacks, update lockfile * fix: socket lifecycle in test helpers, guard params before destructure, safeWrite in rpc-dispatcher, socket.writable check in server * fix: restore net import in server.unit.test.ts (still used for notification test) --------- Co-authored-by: mavishay <mavishay@github.com> Co-authored-by: Avishay Mashiach <avishaym@tikalk.com>
This was referenced Mar 23, 2026
This was referenced Mar 30, 2026
yanaifranchi-tech
pushed a commit
that referenced
this pull request
Apr 16, 2026
Deletes the entire `report-thought` / `report-checkpoint` pipeline, which has been dead since 2026-01-26 (commit a48afa5 by Daniel Scharfstein — "refactor: remove dev-browser MCP, all skills, and system prompts"). That commit removed the MCP-server registration from the desktop's config-generator; since then, neither tool has been registered in opencode's per-task config, so the LLM has had no way to invoke either of them. During the later daemon migration (#770, #847) the infrastructure was partially rebuilt on the daemon side — new ThoughtStreamService on :9228, new agent-core handler classes, RPC forwarders, `registerTask`/`unregisterTask` plumbing, desktop IPC forwarders — but the renderer-side consumer was never restored. Net result: an HTTP server receiving zero real traffic, forwarders firing never, and no subscriber at the end of the pipe even if they did. Deleted files (11): packages/agent-core/mcp-tools/report-thought/ (full dir) packages/agent-core/mcp-tools/report-checkpoint/ (full dir) apps/daemon/src/thought-stream-service.ts packages/agent-core/src/internal/classes/ThoughtStreamHandler.ts packages/agent-core/src/services/thought-stream-handler.ts (duplicate of the above — byte-identical) packages/agent-core/src/factories/thought-stream.ts packages/agent-core/src/types/thought-stream.ts packages/agent-core/src/common/types/thought-stream.ts (second duplicate) apps/daemon/src/websocket.ts (see "Also removed" below) Stripped from the agent-core public API surface: - createThoughtStreamHandler factory - ThoughtStreamAPI / ThoughtStreamOptions / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus types - ThoughtStreamEvent / ThoughtStreamCheckpointEvent aliases - THOUGHT_STREAM_PORT constant Stripped from daemon wiring: - ThoughtStreamService construction, start, shutdown - setEventHandlers RPC forwarder - task-event-forwarding register/unregister per-task calls - env var propagation (ACCOMPLISH_THOUGHT_STREAM_PORT, THOUGHT_STREAM_PORT for child MCP processes) - thoughtStreamService field on RouteServices Stripped from desktop wiring: - client.onNotification('task.thought' | 'task.checkpoint', …) forwarders in daemon-bootstrap - outdated comment in app-startup referencing the deleted events Stripped from shared type maps: - 'task.thought' / 'task.checkpoint' / 'thought.event' / 'checkpoint.event' entries on RpcNotificationMap (two copies, one in common/types/daemon.ts and one in daemon/types.ts) Stripped from tool classification + UI: - 'report_thought' / 'report_checkpoint' from NON_TASK_CONTINUATION_TOOLS (the tools don't exist as invocable, so they can't trigger continuation classification regardless) - report_thought / report_checkpoint entries in tool-mappings.ts (unused label/icon map — Lightbulb and Flag icon imports removed too since they have no remaining referents) Also removed: unused daemon WebSocket helper During cleanup, discovered `apps/daemon/src/websocket.ts` had zero runtime consumers (verified by static search: no imports of setupWebSocket/broadcast/onClientMessage/DaemonEvent, not a build entrypoint, not referenced by tests). The DaemonEvent union still advertised 'task:thought' / 'task:checkpoint' variants for the deleted pipeline, so fixing those variants meant touching the file anyway. Deleted the whole module — the daemon's active transport is JSON-RPC over a Unix socket / named pipe, not WebSocket. A WebSocket-based daemon interface can be re-added later if it becomes necessary; until then, leaving the stub around is just a stale reference surface. Docs updated: - docs/functional-viewpoint.md (9 stale refs in Overview, Detailed Map, Channel Map, and Component Responsibility Matrix, removed; also fixed a typo where I wrote ":9229 WhatsApp send" — actual constant is WHATSAPP_API_PORT = 9230) - docs/qa-suites/task-execution-tests.md — removed EXEC-SDK-10 (regression check for the now-deleted flow) - docs/development-viewpoint.md — removed report-thought / report-checkpoint from the mcp-tools listing and the non-existent `useThoughtStream` hook from the client-dir map - docs/dev-processes-diagram.md — removed `HTTP :9228 (thought stream)` line from the daemon-process node - packages/agent-core/README.md — removed createThoughtStreamHandler API section Docs NOT updated in this PR (out of scope): docs/daemon-final-architecture.md, docs/information-viewpoint.md, docs/concurrency-viewpoint.md, docs/daemon-code-audit.md all carry pre-SDK-cutover HISTORICAL banners — their thought-stream mentions are part of their historical record of what existed pre-cutover, not current-state claims. Belongs to a separate historical-docs-sweep PR. Side benefit — latent port collision fixed: THOUGHT_STREAM_PORT was hardcoded to 9228, same as AZURE_FOUNDRY_PROXY_PORT (also hardcoded in azure-foundry-proxy.ts:15). If a user activated Azure Foundry while the daemon was up, the two would collide on bind. Removing THOUGHT_STREAM_PORT (and the service that bound it) eliminates the collision entirely. Net diff: −4208 / +14 on first commit, plus this amendment. Validation run: pnpm typecheck — clean (4 workspaces) pnpm lint:eslint — 0 errors pnpm format:check — clean pnpm -F @accomplish_ai/agent-core test — 619/619 + 1 skipped pnpm -F @accomplish/daemon test — 110/110 pnpm -F @accomplish/web test:unit — 358/358 pnpm -F @accomplish/desktop test:unit — 352/352 pnpm build — success Zero-grep across apps/ packages/ (src only) + the current-state docs for thought-stream / ThoughtStream / THOUGHT_STREAM / report-thought / report-checkpoint / report_thought / report_checkpoint / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus / createThoughtStreamHandler / useThoughtStream returns only a single intentional documentation comment in apps/daemon/src/index.ts and the "What's gone vs. PTY-era" bullet in functional-viewpoint.md that explicitly documents the removal — no orphan references remain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10 tasks
yanaifranchi-tech
added a commit
that referenced
this pull request
Apr 16, 2026
Deletes the entire `report-thought` / `report-checkpoint` pipeline, which has been dead since 2026-01-26 (commit a48afa5 by Daniel Scharfstein — "refactor: remove dev-browser MCP, all skills, and system prompts"). That commit removed the MCP-server registration from the desktop's config-generator; since then, neither tool has been registered in opencode's per-task config, so the LLM has had no way to invoke either of them. During the later daemon migration (#770, #847) the infrastructure was partially rebuilt on the daemon side — new ThoughtStreamService on :9228, new agent-core handler classes, RPC forwarders, `registerTask`/`unregisterTask` plumbing, desktop IPC forwarders — but the renderer-side consumer was never restored. Net result: an HTTP server receiving zero real traffic, forwarders firing never, and no subscriber at the end of the pipe even if they did. Deleted files (11): packages/agent-core/mcp-tools/report-thought/ (full dir) packages/agent-core/mcp-tools/report-checkpoint/ (full dir) apps/daemon/src/thought-stream-service.ts packages/agent-core/src/internal/classes/ThoughtStreamHandler.ts packages/agent-core/src/services/thought-stream-handler.ts (duplicate of the above — byte-identical) packages/agent-core/src/factories/thought-stream.ts packages/agent-core/src/types/thought-stream.ts packages/agent-core/src/common/types/thought-stream.ts (second duplicate) apps/daemon/src/websocket.ts (see "Also removed" below) Stripped from the agent-core public API surface: - createThoughtStreamHandler factory - ThoughtStreamAPI / ThoughtStreamOptions / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus types - ThoughtStreamEvent / ThoughtStreamCheckpointEvent aliases - THOUGHT_STREAM_PORT constant Stripped from daemon wiring: - ThoughtStreamService construction, start, shutdown - setEventHandlers RPC forwarder - task-event-forwarding register/unregister per-task calls - env var propagation (ACCOMPLISH_THOUGHT_STREAM_PORT, THOUGHT_STREAM_PORT for child MCP processes) - thoughtStreamService field on RouteServices Stripped from desktop wiring: - client.onNotification('task.thought' | 'task.checkpoint', …) forwarders in daemon-bootstrap - outdated comment in app-startup referencing the deleted events Stripped from shared type maps: - 'task.thought' / 'task.checkpoint' / 'thought.event' / 'checkpoint.event' entries on RpcNotificationMap (two copies, one in common/types/daemon.ts and one in daemon/types.ts) Stripped from tool classification + UI: - 'report_thought' / 'report_checkpoint' from NON_TASK_CONTINUATION_TOOLS (the tools don't exist as invocable, so they can't trigger continuation classification regardless) - report_thought / report_checkpoint entries in tool-mappings.ts (unused label/icon map — Lightbulb and Flag icon imports removed too since they have no remaining referents) Also removed: unused daemon WebSocket helper During cleanup, discovered `apps/daemon/src/websocket.ts` had zero runtime consumers (verified by static search: no imports of setupWebSocket/broadcast/onClientMessage/DaemonEvent, not a build entrypoint, not referenced by tests). The DaemonEvent union still advertised 'task:thought' / 'task:checkpoint' variants for the deleted pipeline, so fixing those variants meant touching the file anyway. Deleted the whole module — the daemon's active transport is JSON-RPC over a Unix socket / named pipe, not WebSocket. A WebSocket-based daemon interface can be re-added later if it becomes necessary; until then, leaving the stub around is just a stale reference surface. Docs updated: - docs/functional-viewpoint.md (9 stale refs in Overview, Detailed Map, Channel Map, and Component Responsibility Matrix, removed; also fixed a typo where I wrote ":9229 WhatsApp send" — actual constant is WHATSAPP_API_PORT = 9230) - docs/qa-suites/task-execution-tests.md — removed EXEC-SDK-10 (regression check for the now-deleted flow) - docs/development-viewpoint.md — removed report-thought / report-checkpoint from the mcp-tools listing and the non-existent `useThoughtStream` hook from the client-dir map - docs/dev-processes-diagram.md — removed `HTTP :9228 (thought stream)` line from the daemon-process node - packages/agent-core/README.md — removed createThoughtStreamHandler API section Docs NOT updated in this PR (out of scope): docs/daemon-final-architecture.md, docs/information-viewpoint.md, docs/concurrency-viewpoint.md, docs/daemon-code-audit.md all carry pre-SDK-cutover HISTORICAL banners — their thought-stream mentions are part of their historical record of what existed pre-cutover, not current-state claims. Belongs to a separate historical-docs-sweep PR. Side benefit — latent port collision fixed: THOUGHT_STREAM_PORT was hardcoded to 9228, same as AZURE_FOUNDRY_PROXY_PORT (also hardcoded in azure-foundry-proxy.ts:15). If a user activated Azure Foundry while the daemon was up, the two would collide on bind. Removing THOUGHT_STREAM_PORT (and the service that bound it) eliminates the collision entirely. Net diff: −4208 / +14 on first commit, plus this amendment. Validation run: pnpm typecheck — clean (4 workspaces) pnpm lint:eslint — 0 errors pnpm format:check — clean pnpm -F @accomplish_ai/agent-core test — 619/619 + 1 skipped pnpm -F @accomplish/daemon test — 110/110 pnpm -F @accomplish/web test:unit — 358/358 pnpm -F @accomplish/desktop test:unit — 352/352 pnpm build — success Zero-grep across apps/ packages/ (src only) + the current-state docs for thought-stream / ThoughtStream / THOUGHT_STREAM / report-thought / report-checkpoint / report_thought / report_checkpoint / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus / createThoughtStreamHandler / useThoughtStream returns only a single intentional documentation comment in apps/daemon/src/index.ts and the "What's gone vs. PTY-era" bullet in functional-viewpoint.md that explicitly documents the removal — no orphan references remain. Co-authored-by: Yanai Franchi <yanai@tikalk.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 16, 2026
Deletes the entire `report-thought` / `report-checkpoint` pipeline, which has been dead since 2026-01-26 (commit a48afa5 by Daniel Scharfstein — "refactor: remove dev-browser MCP, all skills, and system prompts"). That commit removed the MCP-server registration from the desktop's config-generator; since then, neither tool has been registered in opencode's per-task config, so the LLM has had no way to invoke either of them. During the later daemon migration (#770, #847) the infrastructure was partially rebuilt on the daemon side — new ThoughtStreamService on :9228, new agent-core handler classes, RPC forwarders, `registerTask`/`unregisterTask` plumbing, desktop IPC forwarders — but the renderer-side consumer was never restored. Net result: an HTTP server receiving zero real traffic, forwarders firing never, and no subscriber at the end of the pipe even if they did. Deleted files (11): packages/agent-core/mcp-tools/report-thought/ (full dir) packages/agent-core/mcp-tools/report-checkpoint/ (full dir) apps/daemon/src/thought-stream-service.ts packages/agent-core/src/internal/classes/ThoughtStreamHandler.ts packages/agent-core/src/services/thought-stream-handler.ts (duplicate of the above — byte-identical) packages/agent-core/src/factories/thought-stream.ts packages/agent-core/src/types/thought-stream.ts packages/agent-core/src/common/types/thought-stream.ts (second duplicate) apps/daemon/src/websocket.ts (see "Also removed" below) Stripped from the agent-core public API surface: - createThoughtStreamHandler factory - ThoughtStreamAPI / ThoughtStreamOptions / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus types - ThoughtStreamEvent / ThoughtStreamCheckpointEvent aliases - THOUGHT_STREAM_PORT constant Stripped from daemon wiring: - ThoughtStreamService construction, start, shutdown - setEventHandlers RPC forwarder - task-event-forwarding register/unregister per-task calls - env var propagation (ACCOMPLISH_THOUGHT_STREAM_PORT, THOUGHT_STREAM_PORT for child MCP processes) - thoughtStreamService field on RouteServices Stripped from desktop wiring: - client.onNotification('task.thought' | 'task.checkpoint', …) forwarders in daemon-bootstrap - outdated comment in app-startup referencing the deleted events Stripped from shared type maps: - 'task.thought' / 'task.checkpoint' / 'thought.event' / 'checkpoint.event' entries on RpcNotificationMap (two copies, one in common/types/daemon.ts and one in daemon/types.ts) Stripped from tool classification + UI: - 'report_thought' / 'report_checkpoint' from NON_TASK_CONTINUATION_TOOLS (the tools don't exist as invocable, so they can't trigger continuation classification regardless) - report_thought / report_checkpoint entries in tool-mappings.ts (unused label/icon map — Lightbulb and Flag icon imports removed too since they have no remaining referents) Also removed: unused daemon WebSocket helper During cleanup, discovered `apps/daemon/src/websocket.ts` had zero runtime consumers (verified by static search: no imports of setupWebSocket/broadcast/onClientMessage/DaemonEvent, not a build entrypoint, not referenced by tests). The DaemonEvent union still advertised 'task:thought' / 'task:checkpoint' variants for the deleted pipeline, so fixing those variants meant touching the file anyway. Deleted the whole module — the daemon's active transport is JSON-RPC over a Unix socket / named pipe, not WebSocket. A WebSocket-based daemon interface can be re-added later if it becomes necessary; until then, leaving the stub around is just a stale reference surface. Docs updated: - docs/functional-viewpoint.md (9 stale refs in Overview, Detailed Map, Channel Map, and Component Responsibility Matrix, removed; also fixed a typo where I wrote ":9229 WhatsApp send" — actual constant is WHATSAPP_API_PORT = 9230) - docs/qa-suites/task-execution-tests.md — removed EXEC-SDK-10 (regression check for the now-deleted flow) - docs/development-viewpoint.md — removed report-thought / report-checkpoint from the mcp-tools listing and the non-existent `useThoughtStream` hook from the client-dir map - docs/dev-processes-diagram.md — removed `HTTP :9228 (thought stream)` line from the daemon-process node - packages/agent-core/README.md — removed createThoughtStreamHandler API section Docs NOT updated in this PR (out of scope): docs/daemon-final-architecture.md, docs/information-viewpoint.md, docs/concurrency-viewpoint.md, docs/daemon-code-audit.md all carry pre-SDK-cutover HISTORICAL banners — their thought-stream mentions are part of their historical record of what existed pre-cutover, not current-state claims. Belongs to a separate historical-docs-sweep PR. Side benefit — latent port collision fixed: THOUGHT_STREAM_PORT was hardcoded to 9228, same as AZURE_FOUNDRY_PROXY_PORT (also hardcoded in azure-foundry-proxy.ts:15). If a user activated Azure Foundry while the daemon was up, the two would collide on bind. Removing THOUGHT_STREAM_PORT (and the service that bound it) eliminates the collision entirely. Net diff: −4208 / +14 on first commit, plus this amendment. Validation run: pnpm typecheck — clean (4 workspaces) pnpm lint:eslint — 0 errors pnpm format:check — clean pnpm -F @accomplish_ai/agent-core test — 619/619 + 1 skipped pnpm -F @accomplish/daemon test — 110/110 pnpm -F @accomplish/web test:unit — 358/358 pnpm -F @accomplish/desktop test:unit — 352/352 pnpm build — success Zero-grep across apps/ packages/ (src only) + the current-state docs for thought-stream / ThoughtStream / THOUGHT_STREAM / report-thought / report-checkpoint / report_thought / report_checkpoint / ThoughtEvent / CheckpointEvent / ThoughtCategory / CheckpointStatus / createThoughtStreamHandler / useThoughtStream returns only a single intentional documentation comment in apps/daemon/src/index.ts and the "What's gone vs. PTY-era" bullet in functional-viewpoint.md that explicitly documents the removal — no orphan references remain. Co-authored-by: Yanai Franchi <yanai@tikalk.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ENG-694 — Daemon Architecture for Background Task Execution
This consolidated PR merges the best unique contributions from 4 community PRs implementing daemon architecture for background task execution.
Closes
Closes #402
Ref: ENG-694
Original PRs Consolidated
What Was Taken From Each Contributor
🏆 PR #626 — david-mamani — Core JSON-RPC Daemon Infrastructure
The foundational RPC layer that everything else builds on:
packages/agent-core/src/common/types/daemon.ts— Complete daemon protocol types (JsonRpcRequest/Response/Notification, DaemonMethodMap, DaemonTransport, ScheduledTask)packages/agent-core/src/daemon/server.ts— DaemonServer: JSON-RPC dispatcher with method registration and notification pushpackages/agent-core/src/daemon/client.ts— DaemonClient: typed async RPC caller with connection statepackages/agent-core/src/daemon/transport.ts— In-process transport pair (Step 2: same-process messaging)packages/agent-core/src/daemon/ipc-transport.ts— Child process IPC transport (Step 3: cross-process via node IPC)packages/agent-core/src/daemon/scheduler.ts— Cron-based task scheduler with 5-field cron parsingapps/desktop/src/main/daemon-bootstrap.ts— Bootstrap lifecycle: tries child-process mode, falls back to in-processapps/desktop/src/main/daemon/cli-bridge.ts— CLI JSON-RPC bridge (nc/pipe interface)apps/desktop/src/main/daemon/entry.ts— Daemon child process entry pointapps/desktop/src/main/daemon/service-manager.ts— Auto-start on login service managerapps/desktop/src/main/tray.ts— System tray with active task count, context menu, show/hide windowapps/desktop/e2e/specs/daemon.spec.ts— E2E integration tests for daemonpackages/agent-core/src/index.ts🎨 PR #613 — SaaiAravindhRaja / ChaiAndCode — UI Panel, Socket Server, Background Mode
User-facing features and database persistence:
apps/desktop/src/main/daemon/server.ts— Unix socket / named pipe JSON-RPC server for external clients (nc, CLI tools)apps/desktop/src/main/ipc/task-callbacks.ts—createDaemonTaskCallbacks(): background task execution with desktop notifications when window is hiddenapps/desktop/__tests__/unit/main/daemon/scheduler.unit.test.ts— Scheduler unit testsapps/desktop/__tests__/unit/main/daemon/server.unit.test.ts— Socket server unit testsapps/web/src/client/components/settings/DaemonPanel.tsx— Settings UI: background mode toggle + socket path display with copy button and CLI usage examplesapps/web/src/client/components/ui/switch.tsx— Accessible Switch UI componentapps/web/locales/en/settings.json+zh-CN/settings.json— i18n: 'Daemon' tab labelapps/web/src/client/components/layout/SettingsDialog.tsx— Add Daemon tab to settingsapps/web/src/client/lib/accomplish.ts— Add daemon API interface methodsapps/desktop/src/preload/index.ts— ExposegetRunInBackground,setRunInBackground,getDaemonSocketPathto rendererapps/desktop/src/main/ipc/handlers/settings-handlers.ts— IPC handlers for daemon settingspackages/agent-core/src/storage/migrations/v013-daemon.ts— DB migration: addrun_in_backgroundcolumnrunInBackground🔧 PR #598 — aryan877 — HTTP Daemon App with Rate Limiting & Services
Standalone daemon app with security and observability:
apps/daemon/— New standalone@accomplish/daemonpackageapps/daemon/src/http-server-factory.ts— HTTP server with timing-safe Bearer token auth, 1MB body limit, route dispatchapps/daemon/src/rate-limiter.ts— Sliding window rate limiter per IP with automatic cleanup (unique security feature)apps/daemon/src/health.ts— Health check endpointapps/daemon/src/permission-service.ts— Permission request managementapps/daemon/src/storage-service.ts— Persistent storage serviceapps/daemon/src/task-service.ts— Task lifecycle managementapps/daemon/src/thought-stream-service.ts— Thought stream event broadcastingdocs/architecture.md— Architecture documentation🔌 PR #509 — Eshaan-byte — WebSocket Support & PID Management
Real-time event streaming and process lifecycle:
apps/daemon/src/websocket.ts— WebSocket server on/wspath for real-time daemon event streaming (task updates, permission requests, auth errors)apps/daemon/src/pid.ts— PID file management with stale lock detection and automatic cleanupapps/daemon/src/mcp-bridges.ts— MCP tool bridges for daemon communicationapps/daemon/src/daemon-options.ts— Daemon configuration optionsArchitecture Overview
The daemon architecture has two layers:
In-Electron Layer (
packages/agent-core/src/daemon/+apps/desktop/src/main/daemon/):Standalone Daemon (
apps/daemon/):Key Design Decisions
Summary by CodeRabbit
New Features
Enhancements
Tests
Documentation