fix(telegram): reclaim the lock of a self-retired daemon owner

[Yeachan-Heo]

What broke

Telegram notifications stopped for every session on my machine tonight. Root-causing it turned up two separate defects; this PR fixes the one that lives on dev.

A daemon that finishes its shutdown but never exits keeps its PID alive while still holding the ownership lock. Recovery accepts process liveness as the only proof of ownership:

// Daemon lock: clear only when the recorded owner process is dead.
} else if (pidAlive(state.pid)) {
    daemon = { action: "left-active", detail: `live daemon owned by pid ${state.pid} left untouched` … }

…with removeDeadOwnerLock re-checking the same thing (if (pidAlive(current.pid)) return "now-alive").

The sharp edge: stoppedAt is already parsed into NormalizedDaemonState and is even surfaced by status (stopped: state?.stoppedAt !== undefined) — reclaim just never consulted it. So an owner that had written its own retirement notice, stopped heartbeating, and torn down every socket was still classified as a live owner and protected indefinitely.

That also put recovery in direct contradiction with the acquisition path, which had already stopped believing that tombstone:

• classifyForeignLiveOwner: "A stopped tombstone with a canonical acquisition is not a live owner"
• isFreshLiveOwner: requires now - heartbeatAt <= HEARTBEAT_TTL_MS and pidAlive

Field evidence

Two wedged owners (pids 53382, 13899) held the lock with heartbeats frozen for minutes and zero sockets open. gjc notify recovery refused both:

daemon: left-active — live daemon owned by pid 13899 left untouched

They had to be killed by hand. Ownership itself kept migrating correctly (acquisition ignored them), so the cost was leaked wedged processes plus operator tooling confidently reporting a "live daemon" that was doing nothing.

The change

Recovery now uses the same liveness definition acquisition already uses: an owner that published stoppedAt is reclaimable even if its PID lingers.

Deliberately narrow:

• A live owner without a stop marker is still protected — a SIGSTOPped or merely slow owner must never be reclaimed, because it still holds the Telegram single-poller slot.
• Stale-heartbeat-only reclaim is not introduced. A self-published stop marker is unambiguous consent; a stale heartbeat is not.
• TOCTOU protection is untouched: reclaim still runs under the steal-mutex with the exact-owner re-check.
• Recovery still never kills a process.

Verification

• bun test packages/coding-agent/test/notifications-service.test.ts — 78 pass / 0 fail
• bun --cwd=packages/coding-agent run check (biome + tsc) — exit 0 (the 11 biome warnings are pre-existing on dev in edit-result-persistence-bounding.test.ts, untouched here)
• Both new tests fail without the source change (77 pass / 1 fail), so they discriminate:
  • clears the lock of a retired owner whose process never exited
  • still protects a live owner that has not published a stop marker

Known follow-ups (not in this PR)

1. Why the process never exits after run() completes. Both wedged owners emitted "shutdown persistence failed" and "shutdown was not durably quiesced" — the last lines of run()'s finally — so run() returned and the process simply never exited. Some handle survives teardown. This PR stops a wedged owner from pinning the lock; it does not stop the wedge. I explicitly did not ship a speculative fix here: my first hypothesis (an unbounded beginToolActivityShutdown() await) was disproved by a test that passed without the fix.

2. The shared topic registry is all-or-nothing. One record this build cannot parse makes the whole snapshot unreadable (malformed Telegram topic state), which empties the registry, prevents topic-lease renewal, drops every attached session, and idle-exits the owner with zero attachments — a total notification outage from one bad record.

   That is what actually caused my outage, but as version skew: a dev daemon wrote records carrying telegramBinding with a chat-only binding, and my v0.13.3 build (1174 commits behind) rejected all 179 records. I verified dev's parser accepts that data, so dev is not broken by it today — but the hazard is structural and reciprocal: the next version will do to dev what dev did to main. Fixing it needs record-level isolation plus a repair/quarantine seam, and dev no longer has the healTelegramDaemonNotificationState seam that exists on the release line, so it needs a design pass rather than a port. Happy to open that separately.

gajae.pr-review-verdict.v1 needs-human sha256:14a44c8259bb9c0c53f471e69c325d0d20bb0f26aab400eb0bd0ec19776f2fd2 reviewer:human reviewer-id:bellman-clawdbot evidence:fix-forward-f78fe820ec-on-top-of-fddf532218-red-team-found-fractional-stoppedAt-1.5-treated-as-consent-unlinking-a-live-pid-lock-while-canonical-hasSafeDaemonStateShape-rejects-it-fixed-by-normalizing-stoppedAt-with-safeNonNegativeInteger-new-regression-test-red-on-unfixed-source-79-of-79-pass-check-exit-0;CI-31915577912-and-31917778038-no-PR-owned-failure-shard1-3-files-all-END_PATCH_MARKER-TDZ-identical-to-base-dev-31912741125-owned-by-4597;blocking-needs-authorized-non-author-exact-head-APPROVED-review-on-f78fe820ec-probepark-HaD0Yun-or-IYENTeam-only-credential-here-is-author-Yeachan-Heo

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Yeachan-Heo]

PR #4599 review disposition — needs-human (head fddf532218, verdict line posted to body)

Adversarial review: CLEAR.

• Reclaim now keys on owner consent (stoppedAt) at both gates — recoverNotifications (L1823) and the steal-mutex re-check in removeDeadOwnerLock (L1699) — matching the acquisition path (telegram-daemon.ts L2160, L2074, L2331). Acquisition/recovery no longer disagree.
• SIGSTOP/paused owner protected: no stop marker → left-active, lock untouched. markDaemonOwnerStopped (L2956) is the only writer and is fenced on full owner identity; a live serving owner never writes it.
• No TOCTOU regression: steal-mutex acquired before the state re-read, exact ownerId+pid identity re-check, transition-lock-held re-verified immediately before unlink.
• Recovery still never kills a process; the diff only unlinks the lock file.
• Tests discriminate: clears the lock of a retired owner whose process never exited fails on base source (verified: 77 pass / 1 fail), still protects a live owner without a stop marker pins the protection invariant (green on both, by design).

Local reproduction: notifications-service.test.ts 78/78 pass; bun --cwd=packages/coding-agent run check exit 0 (11 biome warnings pre-existing on dev, untouched).

CI 31915577912 reconciliation: no PR-owned failure.

• shard-1-of-8: 3/176 files — core/apply-patch-regression, eval/python-env, tools/safe-summary — all END_PATCH_MARKER TDZ at src/edit/streaming.ts:85. Inherited: base dev run 31912741125 shard-1 fails the identical 3 files with the identical error; base shards 2/3/5/7/8 fail sibling variants. Owned by fix(edit): defer apply patch marker lookup #4597, which remains open.
• evidence producer / affected path validation: fail-closed aggregation of shard-1 only (CI_DEV_SHARDS_RESULT: failure, every other input success).
• PR contract bootstrap: Expected exactly one verdict line; found 0 — now satisfied structurally by the verdict line; it still gates on merge-approved + approval.
• Every telegram/notifications job on the exact head is green: notifications-service.test.ts, notifications-live-stream, check:coding-agent, cli-smoke, ts-build, native-build, gjc-state-gates.

Blocker (human-only): merge requires an authorized non-author exact-head APPROVED review (probepark, HaD0Yun, or IYENTeam). The only credential in this lane is the author (Yeachan-Heo), and self-approval is explicitly rejected by the bootstrap contract. Secondary: dev CI stays red until #4597 lands, so a fresh exact-head run will remain shard-red for inherited reasons even after approval — merge should wait for #4597 → dev, then re-run Dev CI and flip the verdict to merge-approved with a fresh digest.

No release/tag/publish/main mutation performed.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Fix-forward pushed: f78fe820ec (fix(telegram): require a canonical stop marker to reclaim a live owner), on top of fddf532218. Verdict line in the PR body updated to the new digest.

What the red-team lane found (ADV-05, real deviation): recovery normalized stoppedAt with finiteNonNegativeNumber, so a fractional stoppedAt (e.g. 1.5) counted as owner consent and let gjc notify recovery unlink a live pid's lock — while the daemon's canonical shape predicate hasSafeDaemonStateShape requires Number.isSafeInteger(stoppedAt) and no compliant writer can emit such a value. That reintroduced exactly the acquisition/recovery disagreement this PR exists to close, in the dangerous direction (recovery reclaiming what acquisition considers an unparseable/unknown state).

The fix: normalize stoppedAt with the same safeNonNegativeInteger rule (notification-service.ts parseDaemonState). A non-canonical marker now reads as marker-less → left-active, lock retained. Health's stopped: read uses the same normalized field and now reports not-stopped for a fractional marker. stoppedAt: 0 stays consent (canonical predicate accepts it — rejecting it would create divergence in the other direction).

Evidence: new regression test still protects a live owner whose stop marker is not canonical is red on the unfixed source and green after; suite 79/79; check exit 0; adversarial harness case m4 (stoppedAt 1.5 + live pid) re-run post-fix → left-active, lockUnlinked=false; 26-case harness otherwise unchanged (TOCTOU t1/t2/t4/t5/t6 all still supersede/contend/protect).

Still blocking merge (human-only): authorized non-author exact-head APPROVED on f78fe820ec from probepark, HaD0Yun, or IYENTeam; dev shard red inherited from #4597 clears separately. Dev CI on the new head is running (31920382510).
—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Dev CI on f78fe820ec reconciled (run 31920382510) — no PR-owned failure, same inherited shape as the previous head.

• shard-1-of-8: again exactly 3/176 files — core/apply-patch-regression, eval/python-env, tools/safe-summary — all END_PATCH_MARKER TDZ at src/edit/streaming.ts:85, byte-identical to base dev run 31912741125 shard-1 (same 3 files, same error). Owned by fix(edit): defer apply patch marker lookup #4597, untouched by this PR.
• evidence producer / affected path validation: fail-closed aggregation of shard-1 only (CI_DEV_SHARDS_RESULT: failure; every other input success).
• PR contract bootstrap: Verdict needs-human intentionally blocks merge — the posted verdict line is now valid, digest-matched (sha256:14a44c82…), and correctly blocking pending the non-author approval. This is the gate doing its job, not a failure of the change.
• Green on the exact head: notifications-service.test.ts (includes the new fractional-marker regression), notifications-live-stream, check:coding-agent, cli-smoke, ts-build, native-build, gjc-state-gates, sdk-production-host-isolated.

Remaining to merge, both outside this lane's authority: (1) exact-head APPROVED from probepark/HaD0Yun/IYENTeam — the only credential here is the author's; (2) #4597 landing to clear the inherited dev shard red. Verdict line stays needs-human and the body is stable until both clear.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Rebased onto current dev (52dad458, includes da648897 "fix(edit): defer apply patch marker lookup (#4597)") to absorb the fresh-process TDZ regression that failed shard-1 on the previous head (f78fe820): apply-patch-regression, python-env, safe-summary — same dev-side END_PATCH_MARKER TDZ class that hit #4571, not a diff of this PR. The two telegram commits replayed unchanged.

Local verification at new head 8544bcbb: the three failing files now pass (76 tests, 0 fail). CI rerunning on the new head.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]