Commit647805b6Recorded4 Aug 2026Repositorycourier
Identify a claim owner by (pid, starttime), not by process name
Message
MEASURED ON THE LIVE HOST, and it defeats the single-poller claim in production while passing every test I had.
claude-ops copies each courier to /tmp/mcp-bins/courier-<launcher-pid> and runs it from there, so /proc/<pid>/comm is "courier-21570" -- it differs PER INSTANCE by construction. The liveness check compared the claim owner's comm against our own and read a mismatch as a recycled pid, i.e. dead. So a second leader session judged the first one's LIVE claim stale, broke it, and started a second poller: the exact failure this module exists to prevent, arriving through the repair path.
The harness could not see it. It ran both instances from one binary path, so their comms matched. Production is the one shape it did not reproduce, and it is the only shape that matters here.
The fix removes the name comparison rather than teaching the harness to match production. (pid, starttime) is the kernel's own process identity: starttime is field 22 of /proc/<pid>/stat, fixed for the life of a process, and it does not care what the binary is called or where it lives. The claim target becomes "<pid>:<starttime>". Parsed after the last ")" because field 2 is the command name in parentheses and can itself contain spaces.
A claim written by an older build records only a pid and cannot be discriminated from a recycled one. Those read ALIVE, deliberately: reading them dead would mean the first instance running this build breaks the live holder's claim and starts a second poller -- the bug, triggered by the upgrade itself. The cost is that a legacy claim whose pid is recycled is not breakable; it self-heals on the holder's next clean shutdown.
Log pruning keeps using the weaker pid-only check, since a log filename carries no start time. A recycled pid there costs disk and nothing else, and it errs toward keeping a live instance's log, which is the point of it.
Also: a clean first acquisition used to log NOTHING, so "is the new build running?" was unanswerable from the log. During today's deploy both the leader and I read the absence of claim lines as evidence of an old binary and both were wrong -- an absence is not a signal. It now logs the claim path and owner at info, once per session.
Verification, all behavioural:
- repro/two_paths_probe.py reproduces the production shape with symlinks (a copied dev bundle cannot find its library dir; a symlink still resolves /proc/self/exe while yielding the symlink's basename as comm -- both verified). Against the merged build: 2 pollers, 3 conflicts. Against this build: 1 poller, 0 conflicts.
- Folded in as harness case 10, with a setup control asserting the two names really are distinct, or the case silently degenerates into a copy of case 1.
- Note the end-state poller count ALONE would have passed the broken build: the loser stands down within 10s, so only the mock's max_inflight and conflict counters show the overlap. Asserting on the mock is what caught it.
Suite 120/0. Harness 59/0 with the pre-fix control still red.
Changed
repro/single_poller_harness.py | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
repro/two_paths_probe.py | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/courier/claim.sgl | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
src/courier/main.sgl | 6 ++++++
src/courier/poller.sgl | 9 +++++++++
test/test-claim.sgl | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
6 files changed, 401 insertions(+), 72 deletions(-)Diff
repro/single_poller_harness.pymodified
@@ -6,7 +6,7 @@ Telegram: every courier instance is started with COURIER_TELEGRAM_API_URL
6
pointed at the local mock, which is the only endpoint the send AND poll paths 7
use once that variable is set. 8
−9
The cases are NUMBERED BY TOPIC, and run in the order 1, 3, 4, 7, 9, 8, 5, 6, 2 --+9
The cases are NUMBERED BY TOPIC, and run in the order 1, 3, 4, 7, 10, 9, 8, 5, 6, 2 -- 10
the ordering is dictated by shared state (cases 1, 3, 4 and 7 all operate on the 11
same pair of live instances, and the pre-fix control in case 2 runs last so its 12
409s can never be confused with the fixed binary's). What each proves:@@ -42,6 +42,10 @@ same pair of live instances, and the pre-fix control in case 2 runs last so its
42
8 LOG PRUNING instance logs are capped, a LIVE instance's log is 43
never pruned however old it looks, and nothing that 44
is not an instance log is touched.+45
10 PRODUCTION SHAPE two instances launched from DIFFERENT binary paths,+46
as claude-ops actually does, still yield one poller.+47
Every other case runs both from one path and so+48
cannot see a name-dependent identity bug. 49
9 FORCED TAKEOVER COURIER_TELEGRAM_POLL_FORCE takes the claim from a 50
LIVE holder; the displaced instance stops polling, 51
stays healthy, and does not delete the new owner's@@ -327,12 +331,21 @@ def claim_files(state_dir):
331
332
333
def claim_owner(state_dir):+334
"""Raw claim target: "<pid>:<starttime>" (or a bare pid on older builds).""" 335
files = claim_files(state_dir) 336
if not files: 337
return None 338
return os.readlink(os.path.join(state_dir, files[0])) 339
340
+341
def claim_owner_pid(state_dir):+342
"""Just the pid. The claim records pid:starttime -- start time is what+343
makes a recycled pid detectable without depending on the process NAME,+344
which differs per instance in production."""+345
o = claim_owner(state_dir)+346
return o.split(":")[0] if o else None+347
+348
349
def wait_for(predicate, timeout=30, interval=0.25): 350
deadline = time.time() + timeout 351
while time.time() < deadline:@@ -373,7 +386,8 @@ def case_one_poller(binp, run_dir, label="1 ONE POLLER"):
386
387
owner = claim_owner(state_dir) 388
check("claim file records the polling instance's pid",−376
owner == str(a.pid), f"claim owner={owner} A={a.pid} B={b.pid}")+389
claim_owner_pid(state_dir) == str(a.pid),+390
f"claim owner={owner} A={a.pid} B={b.pid}") 391
392
log = b.log_text() 393
skip_lines = [l for l in log.splitlines() if "poller claim" in l]@@ -492,7 +506,7 @@ def case_sigkill(binp, run_dir, a, b, state_dir):
506
time.sleep(1) 507
owner = claim_owner(state_dir) 508
check("claim SURVIVES the SIGKILL (nothing released it)",−495
owner == str(dead_pid),+509
claim_owner_pid(state_dir) == str(dead_pid), 510
f"owner={owner} killed={dead_pid}") 511
512
# (a) The instance that was ALREADY RUNNING and had been told to skip must@@ -505,7 +519,7 @@ def case_sigkill(binp, run_dir, a, b, state_dir):
519
took_over, f"B children={poller_children(b.pid)} " 520
f"claim owner={claim_owner(state_dir)}") 521
check("the claim now records the instance that took over",−508
claim_owner(state_dir) == str(b.pid),+522
claim_owner_pid(state_dir) == str(b.pid), 523
f"owner={claim_owner(state_dir)} B={b.pid}") 524
log = b.log_text() 525
check("the takeover is logged, once, at INFO",@@ -529,7 +543,7 @@ def case_sigkill(binp, run_dir, a, b, state_dir):
543
check("a fresh instance breaks the stale claim and polls", got, 544
f"children={poller_children(c.pid)}") 545
check("claim now records the new instance",−532
claim_owner(state_dir) == str(c.pid),+546
claim_owner_pid(state_dir) == str(c.pid), 547
f"owner={claim_owner(state_dir)} C={c.pid}") 548
log = c.log_text() 549
check("the break is logged",@@ -574,7 +588,7 @@ def case_forced_takeover(binp, run_dir):
588
check("the forcing instance takes over and polls", took, 589
f"B children={poller_children(b.pid)} owner={claim_owner(d)}") 590
check("the claim records the forcing instance",−577
claim_owner(d) == str(b.pid),+591
claim_owner_pid(d) == str(b.pid), 592
f"owner={claim_owner(d)} B={b.pid}") 593
594
stood_down = wait_for(lambda: len(poller_children(a.pid)) == 0, timeout=45)@@ -605,7 +619,7 @@ def case_forced_takeover(binp, run_dir):
619
620
# And the displaced instance must NOT have deleted the new owner's claim. 621
check("the displaced holder left the new owner's claim intact",−608
claim_owner(d) == str(b.pid), f"owner={claim_owner(d)}")+622
claim_owner_pid(d) == str(b.pid), f"owner={claim_owner(d)}") 623
624
for inst in (a, b): 625
inst.close_stdin()@@ -786,6 +800,66 @@ def case_log_hygiene(run_dir, pids):
800
801
# ------------------------------------------------------------------ main ---- 802
+803
def case_production_launch_shape(binp, run_dir):+804
"""The claim must survive claude-ops' actual launch shape.+805
+806
claude-ops copies each courier to /tmp/mcp-bins/courier-<launcher-pid>, so+807
every instance has a DIFFERENT /proc/<pid>/comm. Every other case in this+808
file runs both instances from ONE binary path, so their names match --+809
which is exactly why none of them could see the defect measured on the+810
live host on 2026-08-04, where a name-based identity check made each live+811
instance look like a recycled pid to the other.+812
+813
Distinct names come from symlinks: a copied dev bundle cannot find its own+814
library dir, while a symlink still resolves /proc/self/exe back to the real+815
bundle and still yields the symlink's basename as comm. Both verified.+816
"""+817
print("=== 10 PRODUCTION LAUNCH SHAPE (per-instance binary names) ===")+818
d = os.path.join(run_dir, "twopath")+819
os.makedirs(os.path.join(d, "relays"), exist_ok=True)+820
post("/_seed", {"updates": []})+821
post("/_dwell", {"seconds": DWELL})+822
+823
a_path = os.path.join(d, "courier-1111")+824
b_path = os.path.join(d, "courier-2222")+825
for p in (a_path, b_path):+826
if not os.path.exists(p):+827
os.symlink(os.path.abspath(binp), p)+828
+829
a = Courier(a_path, d, "twopath-a")+830
a.initialize()+831
wait_for(lambda: len(poller_children(a.pid)) == 1, timeout=40)+832
b = Courier(b_path, d, "twopath-b")+833
b.initialize()+834
time.sleep(OBSERVE_WINDOW)+835
+836
ca = open(f"/proc/{a.pid}/comm").read().strip()+837
cb = open(f"/proc/{b.pid}/comm").read().strip()+838
# Setup control: if the names are NOT distinct this case degenerates into+839
# a duplicate of case 1 and proves nothing about the production shape.+840
check("the two instances really do have different process names",+841
ca != cb, f"A={ca!r} B={cb!r}")+842
+843
pa, pb = poller_children(a.pid), poller_children(b.pid)+844
check("exactly one poller despite per-instance binary names",+845
len(pa) + len(pb) == 1, f"A={pa} B={pb}")+846
s = stats()+847
check("no 409s under the production launch shape", s["conflicts"] == 0,+848
f"conflicts={s['conflicts']} getupdates={s['getupdates']}")+849
# The end-state poller count alone would have PASSED the broken build --+850
# the loser stands down within 10s, so only the mock shows the overlap.+851
check("the mock never saw two concurrent pollers",+852
s["max_inflight"] <= 1, f"max_inflight={s['max_inflight']}")+853
check("the claim records a pid:starttime identity, not a bare pid",+854
(claim_owner(d) or "").count(":") == 1, f"owner={claim_owner(d)!r}")+855
+856
for inst in (a, b):+857
inst.close_stdin()+858
time.sleep(1)+859
for inst in (a, b):+860
inst.kill()+861
+862
863
def main(): 864
ap = argparse.ArgumentParser() 865
ap.add_argument("--bin", required=True)@@ -818,6 +892,7 @@ def main():
892
inst.kill() 893
instances = [] 894
+895
case_production_launch_shape(args.bin, run_dir) 896
case_forced_takeover(args.bin, run_dir) 897
case_log_pruning(args.bin, run_dir) 898
repro/two_paths_probe.pyadded
@@ -0,0 +1,169 @@
+1
#!/usr/bin/env python3+2
"""Does the single-poller claim survive the PRODUCTION launch shape?+3
+4
claude-ops copies each courier to /tmp/mcp-bins/courier-<launcher-pid> and runs+5
it from there, so every instance has a DIFFERENT /proc/<pid>/comm --+6
"courier-21570", not "courier". Any liveness check that compares the claim+7
owner's comm against our own therefore sees every other instance as a recycled+8
pid, judges its live claim stale, breaks it, and starts a second poller.+9
+10
repro/single_poller_harness.py cannot see this: it runs both instances from one+11
binary path, so their comms match. That is the whole point of this probe --+12
production is the one shape the harness does not reproduce.+13
+14
Distinct names come from SYMLINKS rather than copies: a copied dev bundle+15
cannot find its own library directory ("library not found: (courier main)"),+16
while a symlink still resolves /proc/self/exe back to the real bundle AND+17
still yields the symlink's basename as comm. Verified both.+18
+19
Usage: two_paths_probe.py --bin <courier>+20
Exit 0 = exactly one poller (claim holds). Exit 1 = defect present.+21
"""+22
import argparse+23
import json+24
import os+25
import shutil+26
import subprocess+27
import sys+28
import threading+29
import time+30
import urllib.request+31
+32
PORT = int(os.environ.get("PORT", "8643"))+33
BASE = f"http://127.0.0.1:{PORT}"+34
RUN = "/tmp/courier-two-paths"+35
+36
+37
def stats():+38
with urllib.request.urlopen(BASE + "/_stats", timeout=10) as r:+39
return json.load(r)+40
+41
+42
def post(path, obj):+43
data = json.dumps(obj).encode()+44
req = urllib.request.Request(BASE + path, data=data,+45
headers={"Content-Type": "application/json"})+46
with urllib.request.urlopen(req, timeout=10) as r:+47
return json.load(r)+48
+49
+50
def start(binp, name):+51
env = dict(os.environ)+52
env.update({+53
"COURIER_TELEGRAM_TOKEN": "TESTTOKEN-two-paths",+54
"COURIER_TELEGRAM_CHAT_ID": "1",+55
"COURIER_TELEGRAM_API_URL": BASE,+56
"COURIER_RELAY_DIR": os.path.join(RUN, "relays"),+57
})+58
env.pop("COURIER_DISABLE_TELEGRAM_SEND", None)+59
env.pop("COURIER_TELEGRAM_POLL_FORCE", None)+60
p = subprocess.Popen([binp, "serve"], stdin=subprocess.PIPE,+61
stdout=subprocess.PIPE,+62
stderr=open(os.path.join(RUN, name + ".err"), "wb"),+63
env=env, bufsize=0)+64
p.stdin.write((json.dumps({+65
"jsonrpc": "2.0", "id": 1, "method": "initialize",+66
"params": {"protocolVersion": "2024-11-05", "capabilities": {},+67
"clientInfo": {"name": "two-paths", "version": "0"}}}) + "\n").encode())+68
p.stdin.flush()+69
p.stdout.readline()+70
p.stdin.write((json.dumps({"jsonrpc": "2.0",+71
"method": "notifications/initialized",+72
"params": {}}) + "\n").encode())+73
p.stdin.flush()+74
threading.Thread(target=lambda: [None for _ in p.stdout], daemon=True).start()+75
return p+76
+77
+78
def poller_children(pid):+79
out = []+80
for e in os.listdir("/proc"):+81
if not e.isdigit():+82
continue+83
try:+84
ppid = None+85
for line in open(f"/proc/{e}/status"):+86
if line.startswith("PPid:"):+87
ppid = int(line.split()[1])+88
break+89
if ppid != pid:+90
continue+91
cmd = open(f"/proc/{e}/cmdline", "rb").read().decode("utf8", "replace")+92
except (FileNotFoundError, ProcessLookupError, PermissionError):+93
continue+94
if "--telegram-poller" in cmd:+95
out.append(int(e))+96
return out+97
+98
+99
def main():+100
ap = argparse.ArgumentParser()+101
ap.add_argument("--bin", required=True)+102
args = ap.parse_args()+103
binp = os.path.abspath(args.bin)+104
+105
shutil.rmtree(RUN, ignore_errors=True)+106
os.makedirs(os.path.join(RUN, "relays"), exist_ok=True)+107
a_path = os.path.join(RUN, "courier-1111")+108
b_path = os.path.join(RUN, "courier-2222")+109
os.symlink(binp, a_path)+110
os.symlink(binp, b_path)+111
+112
here = os.path.dirname(os.path.abspath(__file__))+113
mock = subprocess.Popen([sys.executable, os.path.join(here, "mock_telegram.py"),+114
str(PORT), os.path.join(RUN, "mocklog")],+115
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)+116
for _ in range(50):+117
try:+118
stats()+119
break+120
except Exception:+121
time.sleep(0.1)+122
post("/_seed", {"updates": []})+123
post("/_dwell", {"seconds": 2.5})+124
+125
a = b = None+126
try:+127
a = start(a_path, "a")+128
time.sleep(10)+129
b = start(b_path, "b")+130
time.sleep(25)+131
+132
ca = open(f"/proc/{a.pid}/comm").read().strip()+133
cb = open(f"/proc/{b.pid}/comm").read().strip()+134
pa, pb = poller_children(a.pid), poller_children(b.pid)+135
claims = [f for f in os.listdir(RUN) if f.endswith(".claim")]+136
owner = os.readlink(os.path.join(RUN, claims[0])) if claims else None+137
s = stats()+138
+139
print(f"comm A = {ca!r} comm B = {cb!r} (distinct: {ca != cb})")+140
print(f"poller children: A={pa} B={pb}")+141
print(f"claim owner = {owner!r} (A={a.pid}, B={b.pid})")+142
print(f"mock: conflicts={s['conflicts']} max_inflight={s['max_inflight']} "+143
f"getupdates={s['getupdates']}")+144
print()+145
+146
# The probe is only meaningful if the two names really did differ and+147
# polling really happened -- otherwise "one poller" passes for free.+148
ok_setup = (ca != cb) and s["getupdates"] >= 3+149
if not ok_setup:+150
print("PROBE INVALID: names not distinct, or no polling observed "+151
f"(comms differ={ca != cb}, getupdates={s['getupdates']})")+152
return 2+153
+154
total = len(pa) + len(pb)+155
if total == 1 and s["conflicts"] == 0 and s["max_inflight"] <= 1:+156
print("PASS: exactly one poller under the production launch shape")+157
return 0+158
print(f"FAIL: {total} pollers, {s['conflicts']} conflicts "+159
f"-- the claim does not survive per-instance binary names")+160
return 1+161
finally:+162
for p in (a, b):+163
if p:+164
p.kill()+165
mock.terminate()+166
+167
+168
if __name__ == "__main__":+169
sys.exit(main())src/courier/claim.sglmodified
@@ -73,8 +73,10 @@
73
claim-token-hash 74
read-claim-owner 75
pid-liveness−76
pid-liveness/comm−77
proc-comm)+76
claim-identity+77
identity-pid+78
identity-start+79
proc-start-time) 80
(begin 81
82
;; After breaking a stale claim, two instances can both have decided the@@ -111,19 +113,53 @@
113
;; Liveness 114
;; ============================================================ 115
−114
;;; First line of /proc/<pid>/comm, or #f if it cannot be read.+116
;;; Kernel start time of a process (field 22 of /proc/<pid>/stat) as a+117
;;; string, or #f if it cannot be read.+118
;;;+119
;;; Parsed from AFTER the last ")" because field 2 is the command name in+120
;;; parentheses and may itself contain spaces and brackets -- splitting the+121
;;; whole line on whitespace picks the wrong field for any such process. 122
;;; 123
;;; MEASURED: procfs files report st_size 0, so `read-file-string` (which 124
;;; reads stat's size) returns "" for every one of them -- a confident, 125
;;; wrong empty answer. A PORT read is size-independent and returns the 126
;;; real content.−120
(define (proc-comm pid-str)+127
(define (proc-start-time pid-str) 128
(: string? -> any?) 129
(guard (e (else #f)) 130
(let ((line (call-with-input-file−124
(string-append "/proc/" pid-str "/comm")+131
(string-append "/proc/" pid-str "/stat") 132
(lambda (port) (read-line port)))))−126
(and (string? line) (string-trim line)))))+133
(and (string? line)+134
(let ((close (string-index-right+135
line (lambda (c) (char=? c #\))))))+136
(and close+137
(let ((fields (string-split+138
(string-trim+139
(substring line (+ close 1)+140
(string-length line)))+141
" ")))+142
;; These fields start at overall field 3, so field 22+143
;; (starttime) is index 19.+144
(and (> (length fields) 19)+145
(list-ref fields 19)))))))))+146
+147
;;; This process's claim identity, "<pid>:<starttime>".+148
(define (claim-identity)+149
(: -> string?)+150
(let* ((pid (number->string (process-id)))+151
(start (proc-start-time pid)))+152
(if start (string-append pid ":" start) pid)))+153
+154
;; pid part of a recorded identity: "1234:567" -> "1234", "1234" -> "1234".+155
(define (identity-pid id)+156
(let ((i (string-index id (lambda (c) (char=? c #\:)))))+157
(if i (substring id 0 i) id)))+158
+159
;; starttime part, or #f for a pid-only claim written by an older build.+160
(define (identity-start id)+161
(let ((i (string-index id (lambda (c) (char=? c #\:)))))+162
(and i (substring id (+ i 1) (string-length id))))) 163
164
;;; Is the process holding a claim still running? 165
;;;@@ -131,21 +167,12 @@
167
;;; 'undetermined means the instrument could not answer, which is a 168
;;; different fact from "the process is gone" and must not be treated as 169
;;; permission to break the claim.−134
(define (pid-liveness pid-str)+170
;;; Liveness of a recorded claim identity ("<pid>:<starttime>", or a bare+171
;;; pid from an older build).+172
(define (pid-liveness id) 173
(: string? -> symbol?)−136
(pid-liveness/comm pid-str (proc-comm (number->string (process-id)))))−137
−138
;;; pid-liveness with the expected command name passed in explicitly.−139
;;;−140
;;; Split out so the recycled-pid discriminator can be tested from both−141
;;; sides: a live process whose comm MATCHES must read 'alive, and the−142
;;; same live process with a non-matching comm must read 'dead. Testing−143
;;; only the second would be satisfied by an implementation that always−144
;;; answers 'dead -- which is the pre-fix behaviour wearing a lock's−145
;;; clothes, and would break every claim it ever saw.−146
(define (pid-liveness/comm pid-str expected-comm)−147
(: string? any? -> symbol?)−148
(let ((self (number->string (process-id))))+174
(let ((pid-str (identity-pid id))+175
(self (number->string (process-id)))) 176
(cond 177
;; Tier 1: /proc. POSITIVE CONTROL -- our own pid must be visible 178
;; through the instrument before its answer about another pid means@@ -155,16 +182,19 @@
182
(if (not (file-exists? (string-append "/proc/" pid-str))) 183
'dead 184
;; The pid exists -- but pids are recycled, and a recycled pid−158
;; would make a dead courier's claim look alive forever. Compare−159
;; the command name against OUR OWN (passed in, read at runtime−160
;; rather than hardcoded, so it cannot drift if the binary is−161
;; renamed). If either name is unreadable we cannot rule out a−162
;; live courier, so we say 'alive and leave the claim alone --−163
;; the conservative direction.−164
(let ((theirs (proc-comm pid-str)))+185
;; would make a dead courier's claim look alive forever. The+186
;; kernel's start time settles it: same pid AND same starttime+187
;; is the same process, full stop. This deliberately does NOT+188
;; compare the command name -- see the header; comm differs per+189
;; instance in production and comparing it made every live peer+190
;; look dead.+191
(let ((recorded (identity-start id))+192
(actual (proc-start-time pid-str))) 193
(cond−166
((or (not expected-comm) (not theirs)) 'alive)−167
((string=? expected-comm theirs) 'alive)+194
;; Older claim with no starttime, or an unreadable one: we+195
;; cannot rule out a live courier, so leave the claim alone.+196
((or (not recorded) (not actual)) 'alive)+197
((string=? recorded actual) 'alive) 198
(else 'dead))))) 199
;; Tier 2: kill -0, for a host with no /proc. Same shape: prove the 200
;; instrument works on a pid known to be alive (ours) before@@ -215,7 +245,7 @@
245
(define (poller-claim-held? token) 246
(: string? -> boolean?) 247
(equal? (read-claim-owner (poller-claim-path token))−218
(number->string (process-id))))+248
(claim-identity))) 249
250
;;; Try to become the single poller for `token`. 251
;;;@@ -235,7 +265,7 @@
265
(define (acquire-poller-claim! token force?) 266
(: string? any? -> symbol?) 267
(let* ((path (poller-claim-path token))−238
(me (number->string (process-id))))+268
(me (claim-identity))) 269
(guard (e (else 270
(log-warn "Poller claim failed" error: (format "~a" e)) 271
'undetermined))@@ -294,7 +324,7 @@
324
reason: reason) 325
;; Unlink ONLY while the claim still names the dead pid we just judged. 326
;; An unconditional delete here is a TOCTOU big enough to drive two−297
;; pollers through: deciding `owner` is dead involves a proc-comm port+327
;; pollers through: deciding `owner` is dead involves a procfs port 328
;; read, which yields the scheduler inside the supervisor goroutine, so 329
;; a second instance that reached the same verdict can have broken the 330
;; claim AND taken it in the meantime. Deleting blind would then remove@@ -331,7 +361,6 @@
361
(when path 362
(set-car! *held-claim* #f) 363
(guard (e (else #f))−334
(when (equal? (read-claim-owner path)−335
(number->string (process-id)))+364
(when (equal? (read-claim-owner path) (claim-identity)) 365
(delete-file path) 366
(log-info "Released Telegram poller claim"))))))))src/courier/main.sglmodified
@@ -108,6 +108,12 @@
108
;; destroying exactly the long-lived evidence per-instance logs exist to 109
;; preserve. A name with no pid in it is not ours to judge, so it is 110
;; treated as live and left alone.+111
;; NOTE: a log filename carries only a pid, with no start time, so this+112
;; asks the weaker question "is that pid running at all". A recycled pid+113
;; therefore keeps a dead instance's log alive forever. That costs disk+114
;; and nothing else, and it errs in the safe direction -- the failure this+115
;; guards against is deleting a LIVE instance's log, which is the evidence+116
;; per-instance logs exist to preserve. 117
(define (instance-log-live? path) 118
(let ((pid (instance-log-pid path))) 119
(if (not pid)src/courier/poller.sglmodified
@@ -647,6 +647,15 @@
647
(poller-kill! st)))))))) 648
649
(define (poller-supervise-loop server token)+650
;; Say so explicitly, including WHICH claim and WHO holds it. A clean+651
;; first acquisition used to log nothing at all, which made "is the new+652
;; build actually running?" unanswerable from the log -- during the+653
;; 2026-08-04 deploy both the worker and the leader read the absence of+654
;; claim lines as evidence of an OLD binary, and both were wrong. An+655
;; absence is not a signal; this is. One line per session.+656
(log-info "Telegram poller claim held -- this instance is the poller"+657
claim: (poller-claim-path token)+658
owner: (claim-identity)) 659
(log-info "Telegram polling started (isolated poller process)") 660
(set-car! *claim-lost* #f) 661
(set-car! *polling* #t)test/test-claim.sglmodified
@@ -19,7 +19,11 @@
19
(sigil path) 20
(sigil process) 21
(courier dedup)−22
(courier claim))+22
(only (courier claim)+23
acquire-poller-claim! release-poller-claim! poller-claim-held?+24
abandon-poller-claim! poller-claim-path claim-token-hash+25
read-claim-owner pid-liveness claim-identity identity-pid+26
identity-start proc-start-time)) 27
28
;; Every existing test predates the force? argument and means "the normal, 29
;; unforced path", so bind that spelling once rather than threading #f through@@ -98,26 +102,43 @@
102
(test "reports garbage dead rather than raising" 103
(assert-equal 'dead (pid-liveness "not-a-pid"))) 104
−101
(test "can read our own comm (the recycled-pid discriminator works)"−102
(assert-true (string? (proc-comm (number->string (process-id)))))−103
(assert-true (> (string-length (proc-comm (number->string (process-id))))−104
0)))+105
(test "can read our own start time (the recycled-pid discriminator works)"+106
(assert-true (string? (proc-start-time (number->string (process-id)))))+107
(assert-true (> (string-length+108
(proc-start-time (number->string (process-id)))) 0))) 109
110
(test "reads nothing for a pid that does not exist"−107
(assert-equal #f (proc-comm "999999")))−108
−109
;; The recycled-pid discriminator, tested from BOTH sides against the same−110
;; live process (pid 1 always exists). Only testing the 'dead side would be−111
;; satisfied by an implementation that always answers 'dead, which would−112
;; break every claim it ever saw.−113
(test "a live pid with a MATCHING comm is alive"−114
(assert-equal 'alive (pid-liveness/comm "1" (proc-comm "1"))))−115
−116
(test "a live pid with a DIFFERENT comm is treated as dead (recycled pid)"−117
(assert-equal 'dead (pid-liveness/comm "1" "not-the-same-comm")))+111
(assert-equal #f (proc-start-time "999999")))+112
+113
;; The recycled-pid discriminator, tested from BOTH sides against the SAME+114
;; live pid. Only testing the 'dead side would be satisfied by an+115
;; implementation that always answers 'dead -- which is what would break+116
;; every claim it ever saw.+117
(test "a live pid with its REAL start time is alive"+118
(assert-equal 'alive+119
(pid-liveness (string-append "1:" (proc-start-time "1")))))+120
+121
(test "a live pid with a DIFFERENT start time is dead (recycled pid)"+122
(assert-equal 'dead (pid-liveness "1:99999999")))+123
+124
;; This is the production case the comm-based version got wrong: two live+125
;; couriers run from /tmp/mcp-bins/courier-<launcher-pid>, so their command+126
;; NAMES differ. Identity must not care.+127
(test "liveness does not depend on the process name matching ours"+128
(assert-equal 'alive+129
(pid-liveness (string-append "1:" (proc-start-time "1"))))+130
(assert-true (not (string=? (proc-start-time "1")+131
(proc-start-time+132
(number->string (process-id)))))))+133
+134
(test "a legacy pid-only claim reads alive, never dead, while the pid exists"+135
(assert-equal 'alive (pid-liveness "1"))+136
(assert-equal 'dead (pid-liveness "999999"))) 137
−119
(test "an unreadable comm falls back to 'alive, never to 'dead"−120
(assert-equal 'alive (pid-liveness/comm "1" #f))))+138
(test "our own identity round-trips"+139
(assert-equal (number->string (process-id))+140
(identity-pid (claim-identity)))+141
(assert-equal 'alive (pid-liveness (claim-identity))))) 142
143
;; ============================================================ 144
;; Acquire / release@@ -132,7 +153,7 @@
153
(test "records OUR pid in the claim" 154
(fresh-dir) 155
(acquire-poller-claim/1 "tok-b")−135
(assert-equal (number->string (process-id))+156
(assert-equal (claim-identity) 157
(read-claim-owner (poller-claim-path "tok-b"))) 158
(release-poller-claim!)) 159
@@ -157,23 +178,43 @@
178
(make-symlink "999999" path) 179
(assert-equal "999999" (read-claim-owner path)) 180
(assert-equal 'acquired (acquire-poller-claim/1 "tok-stale"))−160
(assert-equal (number->string (process-id)) (read-claim-owner path))+181
(assert-equal (claim-identity) (read-claim-owner path)) 182
(release-poller-claim!)))) 183
−163
;; A claim whose owner pid is ALIVE but is not a courier can only be a−164
;; recycled pid, so it is stale and must be broken -- otherwise one−165
;; unlucky pid reuse would silence Telegram until that unrelated process−166
;; exited. (The complementary case, a claim held by a genuinely live−167
;; courier being left strictly alone, needs a second courier process and−168
;; is proved end-to-end by repro/repro-single-poller.sh case 1, where−169
;; instance B neither steals nor rewrites instance A's claim.)−170
(test "treats a live-but-unrelated owner pid as a recycled, stale claim"+184
;; A recycled pid is now detected by START TIME, not by name, so a claim+185
;; recording (pid, starttime) whose pid was reused reads dead and is broken.+186
(test "breaks a claim whose pid was recycled (start time differs)" 187
(let ((d (fresh-dir))) 188
(let ((path (poller-claim-path "tok-recycled"))) 189
(ensure-directory (path-dirname path))−174
(make-symlink "1" path) ;; pid 1 is alive and is definitely not us+190
(make-symlink "1:99999999" path) ;; pid 1 is alive, but not THAT one 191
(assert-equal 'acquired (acquire-poller-claim/1 "tok-recycled"))−176
(assert-equal (number->string (process-id)) (read-claim-owner path))+192
(assert-equal (claim-identity) (read-claim-owner path))+193
(release-poller-claim!))))+194
+195
;; DELIBERATE SEMANTIC CHOICE, and the direction matters. A claim written by+196
;; an older build records only a pid, so it cannot be discriminated from a+197
;; recycled one. We treat it as ALIVE and leave it alone. Treating it as+198
;; dead would mean that the first instance running the new build breaks the+199
;; live holder's claim and starts a second poller -- which is precisely the+200
;; bug, triggered by the upgrade itself. The cost of this choice is that a+201
;; legacy claim whose pid gets recycled is not breakable; it self-heals on+202
;; the holder's next clean shutdown, which deletes the claim.+203
(test "leaves a legacy pid-only claim alone while that pid is alive"+204
(let ((d (fresh-dir)))+205
(let ((path (poller-claim-path "tok-legacy")))+206
(ensure-directory (path-dirname path))+207
(make-symlink "1" path)+208
(assert-equal 'held-by-other (acquire-poller-claim/1 "tok-legacy"))+209
(assert-equal "1" (read-claim-owner path)))))+210
+211
(test "still breaks a legacy pid-only claim once that pid is gone"+212
(let ((d (fresh-dir)))+213
(let ((path (poller-claim-path "tok-legacy-dead")))+214
(ensure-directory (path-dirname path))+215
(make-symlink "999999" path)+216
(assert-equal 'acquired (acquire-poller-claim/1 "tok-legacy-dead"))+217
(assert-equal (claim-identity) (read-claim-owner path)) 218
(release-poller-claim!)))) 219
220
(test "does not release a claim that now belongs to someone else"@@ -227,7 +268,7 @@
268
(ensure-directory (path-dirname path)) 269
(make-symlink "999999" path) 270
(assert-equal 'acquired (acquire-poller-claim! "tok-forced-stale" #t))−230
(assert-equal (number->string (process-id)) (read-claim-owner path))+271
(assert-equal (claim-identity) (read-claim-owner path)) 272
(release-poller-claim!)))) 273
274
(test "force? does not change an uncontended acquire"