fix(host-kit): keep Linux process identity stable across clock steps - #4438

Thanks for the fix. I reviewed 1347a16 and found one problem that must be fixed before merge: on Linux, upgrading from a pre-fix version can orphan the old daemon. CI is green (14 checks, 0 failing on this head) and there are no conflicts.

On Linux the start time now comes from a linux:<boot_id>:<ticks> token instead of ps lstart (host-process.ts#L355), but recorded start times are still compared as opaque strings (28 startTime ===/!== sites in packages/ and src/). A record written before the upgrade (daemon.json processStartTime, the registration lock owner, app-log markers, device claims) now meets a value in a different format. provesPidReused (owner-identity.ts:186), judgeProcessIdentity (host-process.ts:770) and waitForDaemonExit (daemon-process.ts:86) each read that as proof the process changed hands, when it is only a format change. On every Linux upgrade, a new client sends quiesce and awaitDrainedDaemon (daemon-client-ensure.ts:154) sees exited=true on the first poll while the old daemon is still draining. The client then runs recoverAbandonedDaemonRegistration and starts the successor next to the live predecessor. The registration lock also reads owner-process-reused, so isRegistrationAvailable (daemon-client-startup.ts:455) lets the new daemon take it. If the old daemon hangs, stopDaemonProcess returns exited/already-exited without a signal (daemon-process.ts:142), and no later client can prove its identity again, so it stays orphaned until someone kills it by hand. Persisted app-log markers become ownership-lost, which leaves their log processes running. The PR body's advice to restart the daemon depends on the same comparison.

The rule the code must satisfy: a recorded start time proves a different process only when it was read from the same source as the current observation. A source or format mismatch is unknown evidence, never proof. Could host-kit own one comparison, for example compareProcessStartTime(recorded, observed) returning same | different | unknown, where a non-linux: record against a linux: observation (or the reverse) is unknown? provesPidReused, judgeProcessIdentity, waitForDaemonExit and isAgentDeviceDaemonProcess would route through it. waitForDaemonExit then waits for kill(pid, 0) to release the pid, which keeps the drain guarantee. Keeping exact legacy matching, for example by carrying both the lstart and the token, would be a compatibility path and needs explicit maintainer approval under AGENTS.md.

Could the Linux identity row come from one source instead? In readHostProcessIdentityRowsSync, readHostProcessIdentityRowsAsync and readProcessIdentityRows, read /proc/<pid>/stat directly: field 3 for state, field 22 plus a memoized boot_id for the start time, and a missing or unreadable file means an unanswered pid. That skips ps for identity rows, gives one snapshot, removes the lstart fallback and removes spawnSync from the sync path. The first decision needed is how to treat legacy records: a format mismatch is unknown (fail-closed, my recommendation, no compatibility path), or a dual-value legacy match (needs approval).

Not blocking, take or leave: when procfs fails, host-process.ts#L233 falls back to lstart, so one host can produce two formats and an unreadable value (fail-closed before) becomes proof against a recorded linux: token, so Linux should return unknown instead; parseHostProcessIdentityRows now does blocking file reads per row, so state comes from the ps snapshot and the start time from a later read, which contradicts the one-snapshot comments at host-process.ts:271 and owner-identity.ts:100-103 (move the read into the row readers or use the single-source approach above); readLinuxProcessStartToken checks process.platform instead of hostPlatform() and reads boot_id for every pid, so use hostPlatform() and memoize boot_id; and the tests in host-process-liveness.test.ts#L271 only use a hand-written stat line through a mocked fs.readFileSync, so one unmocked Linux test would help (readProcessStartTime(process.pid) matches /^linux:[0-9a-f-]+:\d+$/ and equals readHostProcessIdentityObservations([process.pid]).get(process.pid)?.startTime).

About the open review thread: the one on two sync procfs reads per pid (discussion) does not apply, so you can resolve it. Procfs reads are kernel-generated with no disk I/O, and owner and claim batches are a few pids. The only cheap win is reading boot_id once per process, which is covered above.

I had no Linux or WSL2 host, so I did not see a real clock step or read a real /proc/<pid>/stat, and I did not run the new tests or the upgrade route with an old-version daemon. The upgrade path above is traced from code, and I did not trace all 28 compare sites, only the proof sites. Once a format mismatch reads as unknown evidence, this should be ready to merge.

Published 8e977aa469: rebased onto current main, fixed mixed-format death/reuse proofs in the shared owner, daemon exit wait and browser records, and added an unmocked Linux identity test. Independent review found no remaining code blocker; both documentation requests are addressed. Ubuntu must execute the real /proc test.

The PR is a draft while validation remains incomplete. The exact prior affected run below failed; its cause remains unproven. Selected failing files pass unchanged on both base and head; this is not a reproduction of the full suite. A full-unit base comparison is now running. No timeout was widened.

{
  "status": "failed",
  "exactCommit": true,
  "checkoutChanged": false,
  "refs": {
    "base": "origin/main",
    "head": "HEAD",
    "resolvedBase": "80075b6412dfa94fd765a36407270e21adce8981",
    "resolvedHead": "8e977aa4696a474550b74ea95592d59c3c8b62e3",
    "mergeBase": "80075b6412dfa94fd765a36407270e21adce8981"
  },
  "results": [
    {
      "id": "format",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "format:check"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "lint",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "lint"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "typecheck",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "typecheck"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "layering",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "check:layering"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "fallow",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "check:fallow",
        "--base",
        "80075b6412dfa94fd765a36407270e21adce8981"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "build",
      "status": "passed",
      "command": [
        "pnpm",
        "run",
        "build"
      ],
      "exitCode": 0,
      "tests": []
    },
    {
      "id": "vitest-related",
      "status": "passed",
      "command": [
        "pnpm",
        "exec",
        "vitest",
        "related",
        "--run",
        "--passWithNoTests",
        "--project=!unit-core",
        "--project=!apple-runner",
        "--project=!fuzz-worker",
        "packages/daemon-contracts/src/daemon-process.ts",
        "packages/host-kit/src/internal/host-process-liveness.test.ts",
        "packages/host-kit/src/internal/host-process.ts",
        "packages/host-kit/src/internal/owner-identity-liveness.test.ts",
        "packages/host-kit/src/internal/owner-identity.test.ts",
        "packages/host-kit/src/internal/owner-identity.ts",
        "packages/host-kit/src/process.ts",
        "packages/platform-web/src/__tests__/agent-browser-process-record.test.ts",
        "packages/platform-web/src/agent-browser-process-record.ts",
        "src/__tests__/daemon-exit-wait.test.ts"
      ],
      "exitCode": 0,
      "tests": [
        {
          "passed": 406,
          "failed": 0,
          "skipped": 0
        }
      ]
    },
    {
      "id": "unit",
      "status": "failed",
      "command": [
        "pnpm",
        "run",
        "check:unit"
      ],
      "exitCode": 1,
      "tests": [
        {
          "passed": 19755,
          "failed": 2,
          "skipped": 8
        }
      ],
      "failures": [
        {
          "file": "scripts/__tests__/eager-closure-budgets.test.ts",
          "test": "rename detection survives a git mv with an edit and a hostile renameLimit",
          "error": "Test timed out in 5000ms"
        },
        {
          "file": "scripts/__tests__/release-publish.test.ts",
          "test": "reports signal-terminated publish as unknown with its signal",
          "error": "Test timed out in 5000ms"
        }
      ]
    },
    {
      "id": "coverage",
      "status": "skipped",
      "reason": "GitHub-authoritative (jobs: Coverage)",
      "tests": []
    },
    {
      "id": "provider-integration",
      "status": "not-run",
      "tests": []
    },
    {
      "id": "web-smoke",
      "status": "skipped",
      "reason": "GitHub-authoritative (jobs: Apple / Smoke Tests (${{ matrix.platform }}), Integration Tests, Lint, Typecheck & Package, Perf Nightly / Android Command Perf, Perf Nightly / iOS Command Perf)",
      "tests": []
    },
    {
      "id": "replay-ios",
      "status": "skipped",
      "reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)",
      "tests": []
    },
    {
      "id": "replay-ios-device",
      "status": "skipped",
      "reason": "parked, workflow_dispatch only (Replay Manual / iOS Replay Suite)",
      "tests": []
    },
    {
      "id": "replay-macos",
      "status": "skipped",
      "reason": "GitHub-authoritative (jobs: Apple / Smoke Tests (${{ matrix.platform }}))",
      "tests": []
    },
    {
      "id": "replay-linux",
      "status": "skipped",
      "reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)",
      "tests": []
    },
    {
      "id": "linux-command-evidence",
      "status": "skipped",
      "reason": "GitHub-authoritative (jobs: Linux / Smoke Tests)",
      "tests": []
    },
    {
      "id": "replay-android",
      "status": "skipped",
      "reason": "parked, workflow_dispatch only (Replay Manual / Android Full Emulator Suite)",
      "tests": []
    }
  ]
}

Coverage failed at 8e977aa. The job annotations identify four identity-related failures: the process-lock guard reuse fixture, daemon acquisition of a reused owner, replaced-daemon status, and registration-owner replacement. They supply legacy-format stale timestamps while Linux now observes boot/tick identities, so the new comparison conservatively retains them. I am updating the reuse fixtures to express a comparable different lifetime while retaining separate mixed-format safety tests. A fifth failure reports an unexpected second daemon fixture launch during version/signature replacement; its cause is still under investigation. No CI rerun or readiness label is warranted yet.

Pushed 60ecdaa with the four same-format reuse fixtures corrected. Their assertions and timeouts are unchanged, and the existing mixed-format retention tests remain. Focused macOS run: 131 passed, one Linux-only skip. Exact affected validation is running.

The failed Ubuntu Coverage job remains a failed result at 8e977aa. The new run must prove that those four failures and the unexpected second launch are gone. The second launch may have followed the timed-out stale-lock request continuing into the next case; that is a hypothesis until Linux validation confirms it. The PR remains a draft.

At 60ecdaa, Ubuntu Coverage passed, including the full unit-ci lane that contains the unmocked Linux process-identity test. All five failures from 8e977aa are gone after correcting the four reuse fixtures. The unexpected second launch did not recur; no production change or retry was used to address it.

Six checks succeeded and six draft entries skipped. Marked ready for review so the remaining lanes can run. The exact local affected gate still failed on the iOS recording provider-test timeout; its cause remains unproven. That local failure and the successful Ubuntu run are separate results. The coordinated Linux-upgrade requirement remains in the PR description.

Apex Sentinel · ✅ Approve

1 finding(s): 1 minor.

🟡 Minor (1)

2 findings below minor hidden (user_settings.min_severity).

Reviewed with this repository's own Sentinel configuration (cc3b99b38530).

This PR is ready at 50182d6. The Linux process identity now stays stable across clock steps, and the earlier findings from the 1347a16 review (#4438 (comment)) are fixed. All 14 checks pass at this head, and there are no conflicts.

Not blocking: the "reused lifetime" token literal is hand-copied in four test files (daemon-client-lifecycle.test.ts:387, daemon-registration-status.test.ts:121, process-lock-guard.test.ts:80, daemon-registration-owner.test.ts:238), while owner-identity.test.ts:34 derives it from readProcessStartTime, so one shared host-kit test fixture could own it. Take it or leave it.

On the other review threads: the state-and-start-token thread (#4438 (comment)) is fixed at this head. Zero lower-priority threads still apply. These can be resolved: the async snapshot thread, because a /proc stat read is a bounded in-memory kernel read and replaces a per-pid ps spawn (#4438 (comment)); the two test.each title threads, now fixed (#4438 (comment), #4438 (comment)); the Rollout paragraph thread, now fixed (#4438 (comment)); and the host-process.ts:368 comment thread, now fixed (#4438 (comment)).

I read the code and relied on the green CI. The Linux-only tests (real /proc, clock-step probes) were skipped on my macOS host. I did not run a mutation to prove the new tests fail on the old code, and I did not run a live old-format daemon against a new CLI. I traced the quiesce, drain, stop and retire routes by reading them. No merge step remains once you are happy with the optional fixture note.

Re-checked b85f3ea. The only change since the clean review at 50182d6 (#4438 (comment)) is a regression test in host-process-liveness.test.ts: it feeds contradictory ps and /proc states in both directions and asserts that state, start token and the zombie fact all come from the one /proc read. Reverting the parser to take state from the ps row fails both cases. An independent review of this head found nothing blocking. No code changed, and there are no conflicts.