Skip to content

fix(restart): skip unavailable lsof in initial scan#76364

Merged
steipete merged 1 commit into
openclaw:mainfrom
neilofneils404:fix-lsof-initial-scan-unavailable
Jul 10, 2026
Merged

fix(restart): skip unavailable lsof in initial scan#76364
steipete merged 1 commit into
openclaw:mainfrom
neilofneils404:fix-lsof-initial-scan-unavailable

Conversation

@neilofneils404

@neilofneils404 neilofneils404 commented May 3, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Service-mode Gateway startup runs a stale-process scan before binding its port. On minimal Debian, Ubuntu, and Alpine-style hosts where lsof is not installed, Node reports ENOENT; OpenClaw treated that expected absence as a warning even though startup continued normally.

Fixes #76352.

Why This Change Was Made

The initial scan now skips only the exact ENOENT missing-command case. Permission failures (EACCES/EPERM), timeouts, unknown spawn failures, and unexpected nonzero lsof exits remain warnings because they mean an installed command or runtime path failed unexpectedly.

This intentionally stays separate from post-kill port polling, where broader permanent-error classification controls retry behavior rather than operator diagnostics.

User Impact

Minimal Linux hosts can start the Gateway without a misleading missing-lsof warning. Real permission, timeout, and command failures remain visible for diagnosis.

Evidence

  • Negative control on Testbox: the new regression test against pre-fix production code failed exactly on the unexpected ENOENT warning; the other 56 stale-process tests passed.
  • Exact-head focused Testbox proof: 60/60 passed (restart-stale-pids 57, ports-lsof 3).
  • Exact-head changed gate passed for core and coreTests in 3m01s.
  • Exact-head full build passed in 114.6s.
  • Live Gateway proof on digest-pinned node:24-bookworm-slim: asserted lsof absent and service mode enabled, reached /healthz in about 3.1s, and emitted no missing-lsof stale-scan warning.
  • Testbox: tbx_01kx5742mgqja5kjc423m7qyq2 (golden-barnacle), stopped after proof.
  • Direct Node 24 process probe confirmed missing command → ENOENT, non-executable command → EACCES, and timeout → ETIMEDOUT, all with null exit status. This matches the official Node child-process contract.
  • Fresh structured autoreview on the rewritten two-file diff: clean, confidence 0.84.
  • Exact-head CI passed with 43 successful jobs and no failures: https://github.com/openclaw/openclaw/actions/runs/29070246531.

Co-authored-by: Neil [email protected]

@clawsweeper

clawsweeper Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open: current main still warns on the initial missing-lsof stale-PID scan, and this PR is the linked candidate fix, but it is not merge-ready because proof is missing, the branch is conflicting, and the patch broadens silent handling to permission failures without enough justification.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #76352
Summary: The open issue is the canonical tracker for the missing-lsof initial stale-PID warning; this PR is the open candidate fix, while other lsof/gateway items overlap only by broader restart or lsof surfaces.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main can fall back to bare lsof, Node returns ENOENT/status null for a missing command, and the initial scan warns for any spawn error. I did not run a full minimal-container gateway startup in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. Suppressing ENOENT is the right direction, but sharing the silent path with EACCES/EPERM in the initial scan may hide permission diagnostics; a narrower fix or explicit maintainer approval is safer.

Security review:

Security review cleared: The diff only changes restart error classification and tests; it does not touch secrets, dependencies, workflows, lockfiles, permissions, or supply-chain surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-05-03T01:37:18Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • Takhoffman: Commit 1be39d4 added the stale-PID cleanup tests and materially changed the gateway startup/restart robustness path this PR edits. (role: introduced adjacent restart lifecycle behavior; confidence: high; commits: 1be39d4250e3; files: src/infra/restart-stale-pids.ts, src/infra/restart-stale-pids.test.ts, src/cli/gateway-cli/run.ts)
  • steipete: Commit 403904e added the shared lsof resolver used by this path, and file history shows repeated adjacent infra runtime work. (role: lsof helper contributor and adjacent owner; confidence: high; commits: 403904ecd107, 0d109750aec7; files: src/infra/ports-lsof.ts, src/infra/ports-inspect.ts)
  • vincentkoc: Commit 7c6ad23 recently changed the same stale cleanup files and the gateway startup caller around inherited gateway PID protection. (role: recent area contributor; confidence: medium; commits: 7c6ad2327c64; files: src/infra/restart-stale-pids.ts, src/infra/restart-stale-pids.test.ts, src/cli/gateway-cli/run.ts)
  • openperf: Commit 8aadca4 changed stale gateway cleanup and tests around ancestor exclusion while retaining the initial lsof warning path. (role: adjacent stale cleanup contributor; confidence: medium; commits: 8aadca4c3e47; files: src/infra/restart-stale-pids.ts, src/infra/restart-stale-pids.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 68e06b9ea148.

@RomneyDa

Copy link
Copy Markdown
Member

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 17, 2026
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 2, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 2, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 2, 2026
@steipete steipete self-assigned this Jul 10, 2026
@steipete
steipete force-pushed the fix-lsof-initial-scan-unavailable branch from 4b1dfc6 to 6514273 Compare July 10, 2026 04:57
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 10, 2026
@steipete

Copy link
Copy Markdown
Contributor

Land-ready proof for 651427363b7d2f1470d1db491ca2e4106311c683:

  • Rewrote the stale branch on current main. The initial Unix scan now suppresses only Node's exact missing-command ENOENT at src/infra/restart-stale-pids.ts:367; EACCES, EPERM, ETIMEDOUT, unknown spawn errors, and unexpected nonzero exits still warn.
  • Kept the post-kill polling classifier separate because that branch owns retry termination, not startup diagnostics.
  • Added focused regression coverage at src/infra/restart-stale-pids.test.ts:261 for silent ENOENT and warning-preserving EACCES/ETIMEDOUT.

Validation:

  • Direct Node 24 probe: nonexistent command → ENOENT, non-executable command → EACCES, timeout → ETIMEDOUT; each returned null status. This matches the official child-process contract.
  • Testbox negative control: new tests against pre-fix production code failed exactly the silent-ENOENT assertion; 56 sibling tests passed.
  • pnpm test src/infra/restart-stale-pids.test.ts src/infra/ports-lsof.test.ts → 60/60 passed.
  • OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 pnpm check:changed → passed (core, coreTests) in 3m01s.
  • pnpm build → passed in 114.6s.
  • Digest-pinned node:24-bookworm-slim live proof: asserted lsof absent and OPENCLAW_SERVICE_MARKER present, started the real Gateway, reached /healthz in about 3.1s, and observed no lsof failed during initial stale-pid scan warning.
  • Testbox tbx_01kx5742mgqja5kjc423m7qyq2 (golden-barnacle) was stopped after proof.
  • Fresh structured autoreview: clean, confidence 0.84.
  • Exact-head CI: run 29070246531, 43 successful jobs, no failures.
  • Required real-behavior proof check passed after the body refresh: run 29070646102.

Known proof gap: no separate Alpine Gateway build was run. The live proof covers the reporter's Debian/Ubuntu environment and the same OS-independent Node ENOENT spawn boundary used on Alpine.

No blocking findings. This is the smallest correct owner-boundary fix, and contributor credit is preserved in the commit trailer.

@steipete
steipete merged commit dc783bb into openclaw:main Jul 10, 2026
111 of 112 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(restart): silently skip lsof stale-pid scan when lsof not installed

3 participants