apps/desktop/main/runtime/manifests.ts (1)

239-258: Async retry delay uses setTimeout instead of shell sleep – good improvement over sync version.

The async extraction correctly uses setTimeout for the retry delay (Line 256), which is an improvement over the deprecated sync version that shells out to sleep (Line 293). However, Line 243 and Line 246 still invoke rm and tar via PATH lookup.

While these are standard POSIX utilities (not npm/pnpm/npx), the coding guideline states to "never use any shell command that relies on the user's PATH." Consider whether these utilities could be unavailable or aliased in edge cases on macOS.

💡 Alternative: Use Node.js fs APIs for rm

-      if (existsSync(extractedSidecarRoot)) {
-        await execFileAsync("rm", ["-rf", extractedSidecarRoot]);
-      }
+      if (existsSync(extractedSidecarRoot)) {
+        const { rm } = await import("node:fs/promises");
+        await rm(extractedSidecarRoot, { recursive: true, force: true });
+      }

For tar, there's no pure-Node equivalent without adding a dependency, so continuing to use the system tar is reasonable.

As per coding guidelines: "Desktop packaged app: never use npx, npm, pnpm, or any shell command that relies on the user's PATH."

🤖 Prompt for AI Agents

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

In `@apps/desktop/main/runtime/manifests.ts` around lines 239 - 258, Replace the
PATH-dependent "rm" exec with the Node fs API: instead of execFileAsync("rm",
["-rf", extractedSidecarRoot]) in the retry loop, use fs.rm or fs.promises.rm
(or fs.rmSync) with { recursive: true, force: true } to remove
extractedSidecarRoot; keep using execFileAsync("tar", ...) for archive
extraction since there's no pure-Node tar in scope. Update imports/usages
accordingly and preserve the surrounding logic that checks
existsSync(extractedSidecarRoot), mkdirSync(extractedSidecarRoot, { recursive:
true }), writeFileSync(stampPath, archiveStamp), and the MAX_RETRIES retry
behavior.

apps/desktop/main/index.ts (1)

30-33: Synchronous extraction in launchd-bootstrap.ts should be tracked for future migration.

launchd-bootstrap.ts at line 885 still uses ensurePackagedOpenclawSidecar, the synchronous extraction method. While this is intentional for backward compatibility, consider adding a TODO comment or opening a tracking issue to migrate this to the async pattern (extractOpenclawSidecarAsync) in a future PR.

🤖 Prompt for AI Agents

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

In `@apps/desktop/main/index.ts` around lines 30 - 33, The launchd-bootstrap usage
of the synchronous extraction function ensurePackagedOpenclawSidecar should be
annotated for future migration to the async API extractOpenclawSidecarAsync: add
a clear TODO comment next to the call to ensurePackagedOpenclawSidecar
indicating this is intentionally synchronous for backward compatibility and
reference extractOpenclawSidecarAsync as the target migration, and optionally
create/open a tracking issue ID in the comment so maintainers can follow up in a
later PR.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".