fix(daemon): reject pid <= 0 in parseLaunchctlPrint by mvanhorn · Pull Request #39281 · openclaw/openclaw

mvanhorn · 2026-03-08T00:14:51Z

Summary

parseLaunchctlPrint() only checks Number.isFinite(pid) before storing the PID
The equivalent systemd parser at src/daemon/systemd.ts:122 correctly checks Number.isFinite(pid) && pid > 0
When launchctl print reports pid = 0 (no running process), the launchd parser stores pid: 0, which is semantically incorrect (PID 0 is the kernel scheduler)

Change

src/daemon/launchd.ts:131

- if (Number.isFinite(pid)) {
+ if (Number.isFinite(pid) && pid > 0) {
    info.pid = pid;
  }

Aligns with the systemd parser's guard at src/daemon/systemd.ts:122.

Test plan

parseLaunchctlPrint with pid = 0 should not set info.pid
parseLaunchctlPrint with pid = 1234 should still set info.pid = 1234
Negative PIDs should also be rejected

Fixes #39188

Co-Authored-By: Claude Opus 4.6 [email protected]

When launchctl print reports pid = 0 (no running process), the parser stores pid: 0, which is semantically incorrect (PID 0 is the kernel scheduler). The systemd parser already checks pid > 0; align launchd parser for consistency. Fixes openclaw#39188 Co-Authored-By: Claude Opus 4.6 <[email protected]>

greptile-apps · 2026-03-08T00:22:23Z

Greptile Summary

This PR fixes a semantic bug in parseLaunchctlPrint where a pid = 0 value reported by launchctl print (indicating no running process) was incorrectly stored as info.pid = 0, aligning the launchd parser with the existing guard in the systemd parser (systemd.ts:122).

Changes:

Added && pid > 0 to the PID validation check in src/daemon/launchd.ts:131, ensuring PID 0 and negative PIDs are rejected

Safety:
The downstream call sites (readLaunchAgentRuntime, restartLaunchAgent, waitForPidExit) are all resilient to this change — waitForPidExit already independently guards with pid <= 1, and readLaunchAgentRuntime treats parsed.pid as optional.

Coverage gap:
The PR's test plan lists three edge cases as unchecked, and no new tests were added in this diff. Consider adding these test cases to prevent silent regression of the new guard.

Confidence Score: 4/5

The single-line fix is semantically correct and low-risk, with no adverse impact on downstream consumers.
This PR is safe to merge. The change is minimal (one condition added), aligns with the already-correct systemd parser, and all downstream consumers either treat pid as optional or already guard against pid <= 1. The only gap is the missing test cases for pid = 0 and negative PIDs, which leaves the new guard without dedicated regression tests.
The test file src/daemon/launchd.test.ts would benefit from the three test cases outlined in the PR description's test plan to cover the new guard comprehensively.

_{Last reviewed commit: 304b45e}

greptile-apps · 2026-03-08T00:22:26Z

src/daemon/launchd.ts

  if (pidValue) {
    const pid = Number.parseInt(pidValue, 10);
-    if (Number.isFinite(pid)) {
+    if (Number.isFinite(pid) && pid > 0) {


Missing test coverage for the new guard.

The PR description's test plan lists three cases as unchecked [ ]:

parseLaunchctlPrint with pid = 0 should not set info.pid

parseLaunchctlPrint with pid = 1234 should still set info.pid = 1234

Negative PIDs should also be rejected

The existing test suite in launchd.test.ts only covers the positive-PID happy path (lines 91–104). Without dedicated tests for pid = 0 and negative PIDs, this guard could silently regress. Consider adding the three cases from the test plan to the "launchd runtime parsing" describe block before merging.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/daemon/launchd.ts Line: 131 Comment: Missing test coverage for the new guard. The PR description's test plan lists three cases as unchecked `[ ]`: - `parseLaunchctlPrint` with `pid = 0` should not set `info.pid` - `parseLaunchctlPrint` with `pid = 1234` should still set `info.pid = 1234` - Negative PIDs should also be rejected The existing test suite in `launchd.test.ts` only covers the positive-PID happy path (lines 91–104). Without dedicated tests for `pid = 0` and negative PIDs, this guard could silently regress. Consider adding the three cases from the test plan to the `"launchd runtime parsing"` describe block before merging. How can I resolve this? If you propose a fix, please make it concise.

Fair point - adding test coverage now for all three cases: pid=0 (should not set), pid=1234 (should set), and negative PIDs (should not set).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

@mvanhorn

Landed from contributor PR #39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

steipete · 2026-03-08T02:44:19Z

Landed on main after rebasing onto the latest origin/main.

What shipped:

reject pid <= 0 in parseLaunchctlPrint so launchd runtime parsing no longer records sentinel/non-running values as real PIDs
add regression coverage for pid = 0, positive pid, and negative pid cases
updated CHANGELOG.md
ran pnpm lint, pnpm build, and pnpm test

Landed commit:

7f44bc5

Source PR commits:

Thanks @mvanhorn.

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

@mvanhorn

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]> (cherry picked from commit 7f44bc5)

@widingmarcus-cyber

* refactor(discord): extract route resolution helpers (cherry picked from commit c1d07b0) * refactor(discord): extract native command context builder (cherry picked from commit 9d10697) * refactor(discord): extract native command session targets (cherry picked from commit 8f719e5) * fix(discord): default missing native command args (cherry picked from commit eb9e78d) * refactor(discord): extract inbound context helpers (cherry picked from commit 547436b) * refactor(discord): compose native command routes (cherry picked from commit 6016e22) * fix(telegram): honor commands.allowFrom in native command auth (openclaw#39310) * telegram: honor commands.allowFrom in native auth * test(telegram): cover native commands.allowFrom precedence * changelog: note telegram native commands allowFrom fix * Update CHANGELOG.md * telegram: preserve group policy in native command auth * test(telegram): keep commands.allowFrom under group gating (cherry picked from commit c22a445) * Discord: fix native command context test args (cherry picked from commit ad80ecd) * refactor(routing): centralize inbound last-route policy (cherry picked from commit 6a8081a) * refactor(telegram): centralize text parsing helpers (cherry picked from commit e705627) * refactor(telegram): split bot message context helpers (cherry picked from commit c2e1ae6) * fix: isolate TUI /new sessions per client Landed from contributor PR openclaw#39238 by @widingmarcus-cyber. Co-authored-by: Marcus Widing <[email protected]> (cherry picked from commit 4600817) * TUI: type setSession test mocks (cherry picked from commit 6cb889d) * fix(telegram): restore DM draft streaming (cherry picked from commit e45fcc5) * fix(ci): pin multi-arch docker base digests (cherry picked from commit 5759b93) * fix: reject launchd pid sentinel values Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]> (cherry picked from commit 7f44bc5) * refactor: register gateway service adapters (cherry picked from commit bd41326) * fix: resolve cherry-pick type errors (rebrand, gutted modules, lastRoutePolicy) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: resolve cherry-pick test failures (gutted Dockerfiles, rebrand INSTALL_BROWSER, preflight stub) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: Vincent Koc <[email protected]> Co-authored-by: Marcus Widing <[email protected]> Co-authored-by: Ayaan Zaidi <[email protected]> Co-authored-by: Matt Van Horn <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

openclaw-barnacle bot added gateway Gateway runtime size: XS labels Mar 8, 2026

mvanhorn mentioned this pull request Mar 8, 2026

bug: parseLaunchctlPrint accepts PID 0 (inconsistent with systemd parser) #39188

Open

greptile-apps bot reviewed Mar 8, 2026

View reviewed changes

test: add coverage for pid <= 0 rejection in parseLaunchctlPrint

f069134

Co-Authored-By: Claude Opus 4.6 <[email protected]>

steipete added a commit that referenced this pull request Mar 8, 2026

fix: reject launchd pid sentinel values

7f44bc5

Landed from contributor PR #39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

steipete closed this Mar 8, 2026

mcaxtr pushed a commit to mcaxtr/openclaw that referenced this pull request Mar 8, 2026

fix: reject launchd pid sentinel values

a3b796c

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

Saitop pushed a commit to NomiciAI/openclaw that referenced this pull request Mar 8, 2026

fix: reject launchd pid sentinel values

22e220e

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

scoootscooob mentioned this pull request Mar 8, 2026

fix(daemon): reject pid 0 in launchctl print parser #39193

Closed

2 tasks

GordonSH-oss pushed a commit to GordonSH-oss/openclaw that referenced this pull request Mar 9, 2026

fix: reject launchd pid sentinel values

c0b01ea

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

jenawant pushed a commit to jenawant/openclaw that referenced this pull request Mar 10, 2026

fix: reject launchd pid sentinel values

f182281

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

dhoman pushed a commit to dhoman/chrono-claw that referenced this pull request Mar 11, 2026

fix: reject launchd pid sentinel values

d02478d

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

senw-developers pushed a commit to senw-developers/va-openclaw that referenced this pull request Mar 17, 2026

fix: reject launchd pid sentinel values

1ccfa0f

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

V-Gutierrez pushed a commit to V-Gutierrez/openclaw-vendor that referenced this pull request Mar 17, 2026

fix: reject launchd pid sentinel values

aed4180

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]>

alexey-pelykh pushed a commit to remoteclaw/remoteclaw that referenced this pull request Mar 22, 2026

fix: reject launchd pid sentinel values

941babc

Landed from contributor PR openclaw#39281 by @mvanhorn. Co-authored-by: Matt Van Horn <[email protected]> (cherry picked from commit 7f44bc5)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(daemon): reject pid <= 0 in parseLaunchctlPrint#39281

fix(daemon): reject pid <= 0 in parseLaunchctlPrint#39281
mvanhorn wants to merge 2 commits intoopenclaw:mainfrom
mvanhorn:fix/launchd-parser-reject-pid-zero

mvanhorn commented Mar 8, 2026

Uh oh!

greptile-apps bot commented Mar 8, 2026

Uh oh!

greptile-apps bot Mar 8, 2026

Uh oh!

mvanhorn Mar 8, 2026

Uh oh!

steipete commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mvanhorn commented Mar 8, 2026

Summary

Change

Test plan

Uh oh!

greptile-apps bot commented Mar 8, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

mvanhorn Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants