specs/designs/launchd-process-architecture.md (5)

537-544: Consider internationalization for dialog text.

The exit dialog contains hardcoded Chinese text (lines 539-543). If the Nexu Desktop application supports multiple languages, this should use i18n string resources instead of hardcoded strings.

🌐 Proposed refactor for i18n support

   const { response } = await dialog.showMessageBox({
     type: "question",
-    buttons: ["完全退出", "后台运行", "取消"],
+    buttons: [
+      t("dialog.quit.fully_exit"),
+      t("dialog.quit.background_mode"),
+      t("dialog.quit.cancel")
+    ],
     defaultId: 0,
-    title: "退出 Nexu",
-    message: "选择退出方式",
-    detail: "后台运行将保持服务运行,Bot 继续工作",
+    title: t("dialog.quit.title"),
+    message: t("dialog.quit.message"),
+    detail: t("dialog.quit.detail"),
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 537 - 544, The
dialog.showMessageBox call uses hardcoded Chinese strings for buttons, title,
message, and detail; replace those literals with i18n resource lookups (e.g.,
fetch localized strings via your i18n.get / t function) and pass the returned
values into buttons, title, message, and detail so the exit dialog uses
localized text; update the call site where dialog.showMessageBox is invoked and
ensure keys like "exit.confirm.title", "exit.confirm.message",
"exit.button.quit", "exit.button.background", "exit.button.cancel", and
"exit.confirm.detail" are used to retrieve the translations.

---

565-598: Add cleanup trap for development script.

The script starts Electron in the background (line 594) but doesn't capture the PID or set up a trap to clean up processes when the script exits (e.g., via Ctrl+C). This could leave orphaned Electron and tail processes.

🧹 Proposed enhancement to add cleanup handling

  #!/bin/bash
  # scripts/dev-launchd.sh (pnpm dev 调用)
  
  set -e
+ 
+ # Trap to cleanup on exit
+ cleanup() {
+   echo "Cleaning up..."
+   if [ -n "$ELECTRON_PID" ]; then
+     kill "$ELECTRON_PID" 2>/dev/null || true
+   fi
+   if [ -n "$TAIL_PID" ]; then
+     kill "$TAIL_PID" 2>/dev/null || true
+   fi
+ }
+ trap cleanup EXIT INT TERM
  
  REPO_ROOT=$(pwd)
  LOG_DIR="$REPO_ROOT/.tmp/logs"
  PLIST_DIR="$REPO_ROOT/.tmp/launchd"
  
  mkdir -p "$LOG_DIR" "$PLIST_DIR"
  
  # ... rest of script ...
  
  # 4. 启动 Electron
- pnpm exec electron apps/desktop &
+ pnpm exec electron apps/desktop &
+ ELECTRON_PID=$!
  
  # 5. tail 日志
- exec tail -f "$LOG_DIR"/*.log
+ tail -f "$LOG_DIR"/*.log &
+ TAIL_PID=$!
+ 
+ # Wait for Electron to exit
+ wait $ELECTRON_PID

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 565 - 598, The
dev-launchd.sh script launches Electron in the background and tails logs but
doesn't clean up on exit; update the script to capture the background PIDs (the
Electron process started by "pnpm exec electron apps/desktop &" and the tail
process from "exec tail -f \"$LOG_DIR\"/*.log"), register a trap (e.g., trap
'...' EXIT INT TERM) that kills those PIDs and performs any necessary cleanup
(stop or unload the launchctl jobs if desired), and ensure the trap waits/cleans
up before exiting so no orphaned Electron or tail processes remain.

---

445-452: Avoid empty catch blocks that hide errors.

Lines 447 and 451 use empty catch blocks with /* ignore */ comments, silently suppressing all errors during service uninstallation. This could hide legitimate issues like permission errors or invalid service names.

📋 Proposed fix to log errors while allowing graceful degradation

   async uninstallService(label: string): Promise<void> {
     try {
       await execFileAsync("launchctl", ["bootout", `${this.domain}/${label}`]);
-    } catch { /* ignore */ }
+    } catch (error) {
+      // Service may not be running, which is acceptable during uninstall
+      console.warn(`Failed to bootout ${label}:`, error instanceof Error ? error.message : error);
+    }
     try {
       await fs.unlink(path.join(this.plistDir, `${label}.plist`));
-    } catch { /* ignore */ }
+    } catch (error) {
+      // Plist may not exist, which is acceptable
+      console.warn(`Failed to remove plist for ${label}:`, error instanceof Error ? error.message : error);
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 445 - 452, The
uninstallService method currently swallows all errors in two empty catch blocks
(around the execFileAsync("launchctl", ["bootout", `${this.domain}/${label}`])
call and fs.unlink(path.join(this.plistDir, `${label}.plist`))) which hides
permission/invalid-name failures; update uninstallService to catch the errors
into variables and log them (e.g., via this.logger or a passed-in logger) with
contextual messages including label and this.plistDir, and only rethrow or
return based on error type if necessary so failures are visible while still
allowing graceful degradation.

---

12-18: Add language specifiers to fenced code blocks.

Multiple fenced code blocks in the document lack language specifiers, which reduces readability and prevents proper syntax highlighting. Static analysis flagged code blocks at lines 12, 65, 149, 626, 726, and 746.

📝 Proposed fixes for language specifiers

Line 12-18 (process flow diagram):

-```
+```text
 配置变化 → OpenClaw 检测到变化

Line 65-88 (architecture diagram):

-```
+```text
 ┌─────────────────────────────────────────────────────────────────┐

Line 149-158 (directory structure):

-```
+```text
 {NEXU_LOG_DIR}/

Line 626-639 (directory structure):

-```
+```text
 {repo}/

Line 726-743 (directory structure):

-```
+```text
 ~/Library/

Line 746-761 (directory structure):

-```
+```text
 {repo}/

Also applies to: 65-88, 149-158, 626-639, 726-743, 746-761

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 12 - 18, The
fenced code blocks in the document (e.g., the block containing "配置变化 → OpenClaw
检测到变化" and other blocks that render diagrams or directory paths) are missing
language specifiers; update each triple-backtick fence to include an appropriate
language tag such as "text" (for flow/diagrams) or "bash" (for directory
listings) to enable proper syntax highlighting—specifically modify the blocks
shown in the diff at the snippet starting with "配置变化 → OpenClaw 检测到变化" and the
other diagram/directory blocks referenced in the comment so each opening ```
becomes ```text or ```bash as appropriate.

---

479-486: isServiceInstalled only checks file existence, not launchd registration.

The method only verifies that the plist file exists on disk, but doesn't check if the service is actually registered with launchd. A service could have a plist file but not be bootstrapped, or vice versa (registered but plist deleted).

Consider renaming to hasPlistFile() for clarity, or enhance the method to also check launchd registration:

♻️ Proposed enhancement to check both file and registration

   async isServiceInstalled(label: string): Promise<boolean> {
-    try {
-      await fs.access(path.join(this.plistDir, `${label}.plist`));
-      return true;
-    } catch {
-      return false;
-    }
+    const hasPlist = await this.hasPlistFile(label);
+    const isBootstrapped = await this.isBootstrapped(label);
+    return hasPlist && isBootstrapped;
+  }
+
+  private async hasPlistFile(label: string): Promise<boolean> {
+    try {
+      await fs.access(path.join(this.plistDir, `${label}.plist`));
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  private async isBootstrapped(label: string): Promise<boolean> {
+    try {
+      await execFileAsync("launchctl", ["print", `${this.domain}/${label}`]);
+      return true;
+    } catch {
+      return false;
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 479 - 486, The
isServiceInstalled(label: string) method currently only checks for a plist file
via fs.access(this.plistDir, `${label}.plist`); update it to reflect true
installation by also querying launchd registration: call launchctl list (or use
appropriate API) to see if the label is registered and return true only if
either the plist exists and/or the label appears in launchctl; alternatively, if
you prefer the original single-check behavior, rename isServiceInstalled to
hasPlistFile() and keep the existing fs.access logic in that method and add a
new isServiceInstalled that performs the launchctl check (or composes both
checks) so callers get a clear semantic.

specs/designs/launchd-process-architecture.md (3)

367-370: ⚠️ Potential issue | 🟡 Minor

Proxy route set is missing /openapi.json.

Line 368 only forwards /api and /v1; this diverges from common controller proxy expectations and can break schema consumers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 367 - 370, The
proxy condition that routes requests to proxyToController (using controllerPort)
currently only checks url.pathname.startsWith("/api") and "/v1", missing the
controller's OpenAPI schema endpoint; update the conditional that guards
proxyToController to also match the OpenAPI path (e.g., include a check for
url.pathname === "/openapi.json" or equivalent) so requests for /openapi.json
are forwarded to the controller.

---

586-589: ⚠️ Potential issue | 🟠 Major

Do not suppress bootstrap failures with 2>/dev/null || true.

Lines 587-588 hide real launchctl/bootstrap errors and proceed to kickstart regardless, which makes startup failures opaque and harder to recover from.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 586 - 589, The
bootstrap commands for launchctl (launchctl bootstrap gui/$UID
"$PLIST_DIR/com.nexu.controller.dev.plist" and
"$PLIST_DIR/com.nexu.openclaw.dev.plist") are suppressing errors with
`2>/dev/null || true`; remove the redirection and the `|| true`, capture/inspect
exit status, and fail fast or log the actual stderr when bootstrap returns
non-zero so we do not proceed to `launchctl kickstart -k
gui/$UID/com.nexu.controller.dev` on a failed bootstrap; ensure any logging
includes the plist path and the launchctl error output to aid debugging.

---

431-434: ⚠️ Potential issue | 🟠 Major

Avoid hardcoded UID fallback (501).

Line 433 hardcodes a macOS-specific assumption that can point to the wrong launchd domain on non-default environments.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.md` around lines 431 - 434, The
constructor currently hardcodes a fallback UID of 501 when process.getuid is
unavailable; replace that with a robust lookup and platform guard: use
process.getuid()?.() when available, otherwise try os.userInfo().uid or
Number(process.env.SUDO_UID) (and only build the launchd domain on darwin), and
avoid defaulting to 501; update the uid and domain assignments in the
constructor (referencing the constructor, plistDir, uid, domain symbols) so uid
is derived from os.userInfo() or env when getuid is missing and domain is set
only on macOS.

specs/designs/launchd-process-architecture.en.md (1)

587-588: Don’t suppress all bootstrap errors with || true.

This hides real failures (permissions, malformed plist, bad domain), making startup diagnostics unreliable. Handle “already bootstrapped” explicitly and fail on other errors.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture.en.md` around lines 587 - 588,
Replace the blanket "|| true" on the two launchctl bootstrap calls so failures
are not silently ignored: run launchctl bootstrap for
"com.nexu.controller.dev.plist" and "com.nexu.openclaw.dev.plist", capture the
exit status and stderr, explicitly detect and ignore the known “already
bootstrapped”/“service already running” message, but for any other error
(permissions, malformed plist, bad domain) log the captured stderr with context
and exit non‑zero so startup fails; update the script around the launchctl
bootstrap invocations to implement this conditional handling.

specs/designs/launchd-process-architecture/en.md (1)

627-650: ⚠️ Potential issue | 🔴 Critical

Exit handler references undefined labels variable.

Lines 645-646 reference labels.openclaw and labels.controller, but the labels object is not defined in this code snippet's scope. This will cause a runtime error when the exit handler executes.

The labels object should be defined at module level or passed into the handler, similar to how it's defined in the bootstrap flow (lines 591-594).

🐛 Proposed fix to define labels in scope

+// Define labels at module level or pass via closure
+const labels = {
+  controller: isDev ? "com.nexu.controller.dev" : "com.nexu.controller",
+  openclaw: isDev ? "com.nexu.openclaw.dev" : "com.nexu.openclaw",
+};
+
 app.on("before-quit", async (event) => {
   event.preventDefault();
 
   const { response } = await dialog.showMessageBox({
     type: "question",
     buttons: ["Quit Completely", "Run in Background", "Cancel"],
     defaultId: 0,
     title: "Quit Nexu",
     message: "Choose exit mode",
     detail: "Running in background keeps services running, bots continue working",
   });
 
   if (response === 2) return; // Cancel
 
   if (response === 0) {
     // Quit completely: gracefully stop all services
     const launchd = new LaunchdManager();
     await launchd.stopServiceGracefully(labels.openclaw);
     await launchd.stopServiceGracefully(labels.controller);
   }
 
   app.exit(0);
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 627 - 650, The
before-quit handler references labels.openclaw and labels.controller but labels
is not defined in scope; fix by ensuring the same labels object used in the
bootstrap flow is available to the handler—either define the labels constant at
module level (so app.on("before-quit") can access it) or pass the labels into
the handler/closure where LaunchdManager is used; update the code that
creates/uses LaunchdManager and the before-quit listener to reference the
module-level or injected labels so labels.openclaw and labels.controller are
defined at runtime.

specs/designs/launchd-process-architecture/en.md (3)

693-702: Consider distinguishing bootstrap error types.

The current approach shows all bootstrap failures as "may already be bootstrapped", but other errors (like invalid plist, permission issues) could also cause bootstrap to fail. Consider checking error messages to provide more specific feedback.

♻️ Proposed improvement

 # Bootstrap (show error if fails)
-if ! launchctl bootstrap gui/$UID_VAL "$PLIST_DIR/com.nexu.controller.dev.plist" 2>&1; then
-  echo "Note: Controller service may already be bootstrapped"
-fi
-if ! launchctl bootstrap gui/$UID_VAL "$PLIST_DIR/com.nexu.openclaw.dev.plist" 2>&1; then
-  echo "Note: OpenClaw service may already be bootstrapped"
-fi
+bootstrap_output=$(launchctl bootstrap gui/$UID_VAL "$PLIST_DIR/com.nexu.controller.dev.plist" 2>&1) || {
+  if echo "$bootstrap_output" | grep -q "Already loaded"; then
+    echo "Controller service already bootstrapped"
+  else
+    echo "Failed to bootstrap Controller: $bootstrap_output"
+  fi
+}
+
+bootstrap_output=$(launchctl bootstrap gui/$UID_VAL "$PLIST_DIR/com.nexu.openclaw.dev.plist" 2>&1) || {
+  if echo "$bootstrap_output" | grep -q "Already loaded"; then
+    echo "OpenClaw service already bootstrapped"
+  else
+    echo "Failed to bootstrap OpenClaw: $bootstrap_output"
+  fi
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 693 - 702, The
bootstrap error handling around launchctl bootstrap (using UID_VAL and PLIST_DIR
for com.nexu.controller.dev.plist and com.nexu.openclaw.dev.plist) should
capture the stderr output into a variable, inspect it for known cases (e.g.,
"service already bootstrapped"/"already bootstrapped", plist parse errors like
"XML property list" or "could not parse", and permission errors like "Permission
denied" or "Operation not permitted"), and then print a specific message for
each case instead of always saying "may already be bootstrapped"; for unexpected
errors print the full captured error and exit non‑zero so failures aren’t
silently ignored.

---

388-395: Consider logging file serving errors.

The catch block silently swallows errors, which can make debugging difficult. Consider logging the error before returning a 404 response.

♻️ Proposed improvement

       try {
         const stat = statSync(filePath);
         res.writeHead(200, { "Content-Type": contentType, "Content-Length": stat.size });
         createReadStream(filePath).pipe(res);
-      } catch {
+      } catch (err) {
+        console.error(`Failed to serve ${filePath}:`, err);
         res.writeHead(404);
         res.end("Not Found");
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 388 - 395, The
catch block that handles errors from statSync/createReadStream is silently
swallowing exceptions; update the catch to capture the error (e.g., catch (err))
and log it before sending the 404 response so you can diagnose file-serving
failures — locate the block around statSync(filePath),
createReadStream(filePath).pipe(res) and add a call to your logger (or
console.error) that includes the error and the filePath, then continue to call
res.writeHead(404) and res.end("Not Found").

---

589-622: Consider adding error handling for service startup failures.

The bootstrap flow doesn't explicitly handle cases where service startup fails. Consider adding try-catch blocks or timeout handling for service readiness checks to provide better error messages.

♻️ Proposed improvement

 export async function bootstrapWithLaunchd(env: DesktopEnv): Promise<void> {
   const launchd = new LaunchdManager({ plistDir: env.plistDir });
   const labels = {
     controller: env.isDev ? "com.nexu.controller.dev" : "com.nexu.controller",
     openclaw: env.isDev ? "com.nexu.openclaw.dev" : "com.nexu.openclaw",
   };
 
   // 1. Ensure plist installed
   for (const [service, label] of Object.entries(labels)) {
     if (!(await launchd.isServiceInstalled(label))) {
       const plist = generatePlist(service as "controller" | "openclaw", env);
       await launchd.installService(label, plist);
     }
   }
 
   // 2. Start services not running
   const controllerStatus = await launchd.getServiceStatus(labels.controller);
   if (controllerStatus.status !== "running") {
-    await launchd.startService(labels.controller);
-    await waitForControllerReadiness(env.controllerPort);
+    try {
+      await launchd.startService(labels.controller);
+      await waitForControllerReadiness(env.controllerPort);
+    } catch (err) {
+      throw new Error(`Failed to start Controller: ${err instanceof Error ? err.message : err}`);
+    }
   }
 
   const openclawStatus = await launchd.getServiceStatus(labels.openclaw);
   if (openclawStatus.status !== "running") {
-    await launchd.startService(labels.openclaw);
+    try {
+      await launchd.startService(labels.openclaw);
+    } catch (err) {
+      console.error(`Failed to start OpenClaw: ${err instanceof Error ? err.message : err}`);
+      // OpenClaw failure might not be critical, continue
+    }
   }
 
   // 3. Start embedded Web Server
   await startEmbeddedWebServer({
     port: env.webPort,
     webRoot: env.webRoot,
     controllerPort: env.controllerPort,
   });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 589 - 622, Add
explicit error handling and timeouts around service installation/startup in
bootstrapWithLaunchd: wrap the loop that calls LaunchdManager.isServiceInstalled
and installService in try-catch and log/throw a descriptive error if
installation fails; wrap getServiceStatus/startService calls for both labels in
try-catch and add a bounded wait (timeout) to waitForControllerReadiness so it
rejects if the controller never becomes ready, then surface a clear error
message that includes the label and the underlying error; ensure failures
prevent proceeding to startEmbeddedWebServer (propagate or rethrow) and consider
adding a retry or cleanup path if startService fails for controller or openclaw.

specs/designs/launchd-process-architecture/en.md (2)

490-500: Recommended: Add explicit error logging for bootstrap command.

While execFileAsync will throw on failure (including stderr in the error), the previous review suggested explicitly capturing and logging the command output to aid debugging. The development script (lines 715-720) shows this pattern, but the TypeScript implementation could benefit from similar visibility.

💡 Suggested improvement for visibility

  async installService(label: string, plistContent: string): Promise<void> {
    const plistPath = path.join(this.plistDir, `${label}.plist`);
    await fs.mkdir(this.plistDir, { recursive: true });
    await fs.writeFile(plistPath, plistContent, "utf8");

    // Check if service is already registered to avoid duplicate bootstrap
    const isRegistered = await this.isServiceRegistered(label);
    if (!isRegistered) {
-      await execFileAsync("launchctl", ["bootstrap", this.domain, plistPath]);
+      try {
+        const { stdout, stderr } = await execFileAsync("launchctl", ["bootstrap", this.domain, plistPath]);
+        if (stdout) console.log(`Bootstrap ${label}:`, stdout);
+        if (stderr) console.warn(`Bootstrap ${label} stderr:`, stderr);
+      } catch (err) {
+        console.error(`Failed to bootstrap ${label}:`, err instanceof Error ? err.message : err);
+        throw err;
+      }
    }
  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 490 - 500, The
installService method currently calls execFileAsync("launchctl", ["bootstrap",
this.domain, plistPath]) without explicit logging; wrap that call in a try/catch
inside installService so you capture stdout/stderr on success and include them
in a debug/info log, and on failure catch the thrown error, extract its
stderr/stdout (or message) and log an explicit error that includes the command,
this.domain, plistPath and the captured output before rethrowing; reference the
existing symbols installService, execFileAsync, plistPath, plistDir, domain and
isServiceRegistered when making this change.

---

602-635: Consider adding error handling for robustness.

The bootstrapWithLaunchd function orchestrates critical startup steps but lacks error handling. If any step fails (service installation, startup, or controller readiness check), the entire bootstrap process will crash without graceful degradation.

For production implementation, consider wrapping critical sections in try-catch blocks to provide user-friendly error messages or fallback behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@specs/designs/launchd-process-architecture/en.md` around lines 602 - 635, The
bootstrapWithLaunchd flow has no error handling; wrap the main orchestration
(the install loop using LaunchdManager.isServiceInstalled/installService, the
service start and getServiceStatus/startService calls, the
waitForControllerReadiness call, and the final startEmbeddedWebServer
invocation) in a try/catch so failures produce a clear, actionable error or
fallback instead of crashing; inside the catch, log the error with context
(include label/service and the thrown error), perform any necessary cleanup or
rollback (e.g., stop or uninstall partially installed services via
LaunchdManager), and either rethrow a wrapped error or return a controlled
failure result; consider adding short retry logic around startService and
waitForControllerReadiness to handle transient failures.