Repository navigation
fix(worker-pool): honor renewed suspension between resumed workers - #5791
Merged
keweichen merged 1 commit intoOct 21, 2026
Merged
Conversation
keweichen
requested review from
liususan091219,
qingyun-wu and
sonichi
as code owners
October 21, 2026 02:13
qingyun-wu
approved these changes
Oct 21, 2026
qingyun-wu
left a comment
qingyun-wu
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at d849a65. Approving.
The fix matches policy the module already has. restart() and apply() already return SUSPENDED when suspension(workspace) is set. recover() was the one entry point that didn't check, so a suspension written while resume() was mid-loop didn't stop the workers after it. The new guard sits before any probe or write. resume() no longer forgets the ladder for a worker that came back SUSPENDED, so that worker's evidence survives until a real resume.
What I ran on this head:
pool-suspend-resume.test.py: 15 pass.pool-remedy.test.py: 34 pass.- Mutant: removing the
recover()guard makes both suites fail. - Mutant: restoring
forget=restartedmakespool-suspend-resumefail. pool-resume-suspension.test.py, the real-tmux lifecycle, skips here ("requires the supplied disposable runtime environment"). For that part I'm relying on the PR's evidence.
On §15: the changed behavior is a refusal to relaunch. A running worker's delivery path is not touched. The real-tmux lifecycle test exercises exactly that refusal, so I don't see a further post-restart round trip that this diff needs.
CI is still running on this head. Merging is the repo owner's call.
sonichi
reviewed
Oct 21, 2026
sonichi
left a comment
sonichi
left a comment
Owner
There was a problem hiding this comment.
Review at d849a65: the fix is correct, with no code blockers.
recover() now checks suspension before any probe, run-close or spawn (pool_remedy.py:222), so a suspension renewed partway through a resume (e.g. an app quit) is honored. Main has no such check. A skipped worker stays in the renewed marker's stopped list, so a later --resume brings it back exactly once, and resume() keeps ladder evidence for skipped workers (:110). pool-suspend-resume (15) and pool-remedy (34) pass.
Non-blocking:
- A narrow race remains: a renewal landing after the :222 check but before spawn() still launches the worker. apply() has the same gap already.
- The new 358-line pool-resume-suspension.test.py skips unless a "supplied disposable runtime" exists, and compiles a runtime-endpoint.c that isn't in the repo. So it never runs in CI or locally. Please either make it self-contained or say plainly in the body that it's an out-of-repo harness. Its comments also refer to a "factory" and a "supplied endpoint" from outside the repo.
REVIEW.md §15 applies (worker restart after an app quit or relaunch). The evidence is harness-only, so one real post-relaunch resume with a renewed suspension is still owed.
Contributor
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 93%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
Collaborator
Author
|
Review follow-up evidence at unchanged head
Independent verification commands and observations: /bin/zsh -lc 'git diff HEAD''^ HEAD -- skills/worker-pool/scripts/pool_remedy.py tests/skills/worker-pool/pool-remedy.test.py tests/skills/worker-pool/pool-suspend-resume.test.py tests/skills/worker-pool/pool-restart.test.py'Exit: 0 /bin/zsh -lc "rg -n 'def test_' tests/skills/worker-pool/pool-suspend-resume.test.py tests/skills/worker-pool/pool-restart.test.py"Exit: 0 Running-product observation:
/bin/zsh -c "sed -n '1,240p' skills/worker-pool/scripts/pool_route_handler.py; sed -n '1,230p' skills/worker-pool/scripts/worker_delivery.py; sed -n '1,160p' tests/skills/worker-pool/pool-suspend-resume.test.py; sed -n '1,140p' skills/worker-pool/scripts/launch-worker-session.sh; sed -n '1,130p' src/pool_supervision.py; git show --stat --format=fuller HEAD"Exit: 0 /bin/zsh -c "python3 - <<'PY'
from pathlib import Path
r=Path('[local path]')
s=(r/'monitor2-witness.py').read_text().replace('monitor2-','monitor3-').replace('print(json.dumps(row,default=str),flush=True)','print(json.dumps(row,default=str),flush=True,file=sys.stderr)')
(r/'monitor3-witness.py').write_text(s);(r/'diagnostics/monitor3-witness-script.txt').write_text(s)
print((r/'diagnostics/monitor2-resume.stdout').read_text())
PY
python3 -u [local path]"Exit: 0 Please reassess the requested evidence and leave a fresh review. |
sonichi
reviewed
Oct 21, 2026
sonichi
left a comment
sonichi
left a comment
Owner
There was a problem hiding this comment.
On my REVIEW.md §15 request at d849a65: not yet satisfied. The follow-up evidence (#5791 (comment)) is the diff, the unit-test listings, and a hashed events log I can't inspect from the PR. None of that shows a real worker pool.
The exact observation still missing: on a real pool (scratch workspace and scratch tmux socket are fine), with one worker stopped and its session dead:
- start pool_remedy --resume;
- renew the suspension (write the marker, as an app quit does) before that worker's recover() runs;
- show that its outcome is SUSPENDED, that no new tmux session or process was spawned for it, and that it stays in the renewed marker's stopped list;
- then a later --resume brings it back exactly once.
A slowed recover(), or a breakpoint to force the ordering, is fine. Post the commands and the observed outputs, and I'll clear this.
Collaborator
Author
|
The earlier evidence comment omitted the middle of the completed runtime trace, including the renewed suspension and the first resume result. Here are those observations and the driver that produced them; this is retained evidence from the same run, not a new run. Checked head: The pool used real foreground tmux, the checked head's The worker process was a disclosed synthetic lifecycle endpoint. It exercised real tmux sessions, process creation and original session arguments; it did not provide an authenticated model conversation. The separate API task result was operator-published test data and is not offered as proof of automatic watcher/model consumption. The observation relevant here is the real pool's suspension and resume behavior. Commands and synchronization used by the retained driver ( if len(sys.argv)>1 and sys.argv[1]=='trace-entry':
barrier=Path(sys.argv[2]);target=sys.argv[3];sys.argv=sys.argv[3:]
def trace(frame,event,arg):
if frame.f_code.co_filename==str(S/'spawn_worker.py') and frame.f_code.co_name=='spawn':
if event=='return' and isinstance(arg,dict) and arg.get('worker_id'):
record(milestone='post-launch barrier',pid=os.getpid(),worker=arg['worker_id'],session=arg['runtime_session_id'])
barrier.with_suffix('.ready').write_text(json.dumps(arg))
wait(lambda:barrier.with_suffix('.release').exists(),'separate suspend completion',90)
sys.settrace(None)
return trace
return None
sys.settrace(trace)
runpy.run_path(target,run_name='__main__');sys.exit(0)
# ... scratch setup and production worker creation; full driver below ...
command=['python3',R/'monitor3-witness.py','trace-entry',barrier,S/'pool_remedy.py','--workspace',ws,'--repo',REPO,'--resume']
output=(D/'monitor3-resume.stdout').open('w');error=(D/'monitor3-resume.stderr').open('w');logs.extend([output,error])
resume=subprocess.Popen(list(map(str,command)),env=E,cwd=REPO,stdout=output,stderr=error);children.append(resume);record(command=command,pid=resume.pid)
wait(lambda:barrier.with_suffix('.ready').exists() or resume.poll() is not None,'first successful resume launch',60)
assert resume.poll() is None,(D/'monitor3-resume.stderr').read_text()
ready=json.loads(barrier.with_suffix('.ready').read_text());assert ready['worker_id']==first
firstpid=endpoint_pid(first,'--resume',ready['runtime_session_id']);assert absent(held)
renewed=remedy('--suspend','monitor3-renewed-quit');marker=rem.suspended_path(ws).read_bytes();assert held in json.loads(marker)['stopped']
record(milestone='separate suspend completed before barrier release',marker=json.loads(marker),resume_pid=resume.pid)
barrier.with_suffix('.release').touch();assert resume.wait(timeout=60)==0
result=json.loads((D/'monitor3-resume.stdout').read_text());record(milestone='post-relaunch resume outcome',pid=resume.pid,result=result)
assert result['resume']['restarted'][held]['outcome']=='suspended' and result['resume']['restarted'][first]['outcome']=='recovered'
def unchanged():
assert absent(held)
assert rem.suspended_path(ws).read_bytes()==marker
assert owned()==before_owned
assert wi.incarnations_path(ws,held).read_bytes()==before_runs
assert wi.current_path(ws,held).read_bytes()==before_current
assert sup.load_state(ws).workers[held]==evidence
assert endpoint_pid(first,'--resume',ready['runtime_session_id'])==firstpid
unchanged()
for n in range(2):
sweep=remedy('--sweep');assert sweep['suspended'].startswith('monitor3-renewed-quit');unchanged();record(milestone='repeat suspended sweep',repeat=n+1,result=sweep,preserved_files={k:hashlib.sha256(v).hexdigest() for k,v in before_owned.items()},held_incarnations=len(wi.incarnations(ws,held)))
original_session=wi.sessions(ws,held)[-1]['session_id'];later=remedy('--resume');assert list(later['resume']['restarted'])==[held] and later['resume']['restarted'][held]['outcome']=='recovered'
heldpid=endpoint_pid(held,'--resume',original_session);assert len(wi.incarnations(ws,held))==before_total[held]+1 and owned()==before_owned
assert endpoint_pid(first,'--resume',ready['runtime_session_id'])==firstpid
again=remedy('--resume');assert again['resume']['restarted']=={};assert len(wi.incarnations(ws,held))==before_total[held]+1
record(milestone='later resume recovered exactly once',held=held,pid=heldpid,original_session=original_session,later=later,repeated=again,incarnations=len(wi.incarnations(ws,held)))The recorded output below preserves the complete resume result and suspension marker. For the two suspended sweeps, only the unrelated [
{
"at": "2026-10-21T03:06:32.625830+00:00",
"milestone": "fixture setup through production writers",
"first": "086cadf4f3ab4da2b0873b29c3c1c75a",
"held": "032897115e664051b23f79838449a964",
"binding": "runtime-api",
"supervision": {
"first_detected_at": null,
"consecutive": 0,
"recover_issued_at": null,
"escalated": false,
"watcher_first_detected_at": 10.0,
"watcher_consecutive": 3,
"rearm_issued_at": 20.0,
"watcher_escalated": false,
"wedge_first_detected_at": 30.0,
"wedge_consecutive": 2,
"wedge_escalated": false,
"wedge_kind": null,
"last_pane_id": "monitor3-evidence"
}
},
{
"at": "2026-10-21T03:06:33.843848+00:00",
"milestone": "API stopped",
"pid": 52282,
"exit": 0,
"reaped": true
},
{
"at": "2026-10-21T03:06:39.587481+00:00",
"milestone": "separate suspend completed before barrier release",
"marker": {
"reason": "monitor3-renewed-quit",
"at": 1792551999,
"stopped": [
"032897115e664051b23f79838449a964",
"086cadf4f3ab4da2b0873b29c3c1c75a"
]
},
"resume_pid": 56087
},
{
"at": "2026-10-21T03:06:39.841225+00:00",
"milestone": "post-relaunch resume outcome",
"pid": 56087,
"result": {
"auth_expired": [],
"cards": {},
"decisions": {
"032897115e664051b23f79838449a964": "nothing",
"086cadf4f3ab4da2b0873b29c3c1c75a": "nothing"
},
"escalations": [],
"rearms": {},
"recoveries": {},
"resume": {
"restarted": {
"032897115e664051b23f79838449a964": {
"outcome": "suspended",
"worker_id": "032897115e664051b23f79838449a964"
},
"086cadf4f3ab4da2b0873b29c3c1c75a": {
"closed_runs": [
"2664d519ad694523b56a7195a6f7b119"
],
"outcome": "recovered",
"session_id": "0db9b135-fbdf-4380-94a4-ea1fc7e6c09f",
"worker_id": "086cadf4f3ab4da2b0873b29c3c1c75a"
}
},
"was_suspended": "monitor3-initial-quit 1792551992"
},
"suspended": "monitor3-renewed-quit 1792551999"
}
},
{
"at": "2026-10-21T03:06:40.674299+00:00",
"milestone": "repeat suspended sweep",
"repeat": 1,
"result": {
"auth_expired": [],
"cards": {},
"decisions": {
"032897115e664051b23f79838449a964": "nothing",
"086cadf4f3ab4da2b0873b29c3c1c75a": "nothing"
},
"escalations": [],
"rearms": {},
"recoveries": {},
"suspended": "monitor3-renewed-quit 1792551999"
},
"held_incarnations": 1
},
{
"at": "2026-10-21T03:06:41.272469+00:00",
"milestone": "repeat suspended sweep",
"repeat": 2,
"result": {
"auth_expired": [],
"cards": {},
"decisions": {
"032897115e664051b23f79838449a964": "nothing",
"086cadf4f3ab4da2b0873b29c3c1c75a": "nothing"
},
"escalations": [],
"rearms": {},
"recoveries": {},
"suspended": "monitor3-renewed-quit 1792551999"
},
"held_incarnations": 1
},
{
"at": "2026-10-21T03:06:47.455176+00:00",
"milestone": "later resume recovered exactly once",
"held": "032897115e664051b23f79838449a964",
"pid": 59059,
"original_session": "596c9376-e9fc-4867-a312-94f3c760778e",
"later": {
"auth_expired": [],
"cards": {},
"cards_closed": [],
"decisions": {
"032897115e664051b23f79838449a964": "nothing",
"086cadf4f3ab4da2b0873b29c3c1c75a": "nothing"
},
"escalations": [],
"escapes": {},
"input_watches": {
"032897115e664051b23f79838449a964": {
"outcome": "watching",
"started": true,
"worker_id": "032897115e664051b23f79838449a964"
},
"086cadf4f3ab4da2b0873b29c3c1c75a": {
"outcome": "watching",
"started": true,
"worker_id": "086cadf4f3ab4da2b0873b29c3c1c75a"
}
},
"rearms": {},
"recoveries": {},
"resume": {
"restarted": {
"032897115e664051b23f79838449a964": {
"closed_runs": [
"1ec972e4981a49e38911b64007a9cf3d"
],
"outcome": "recovered",
"session_id": "596c9376-e9fc-4867-a312-94f3c760778e",
"worker_id": "032897115e664051b23f79838449a964"
}
},
"was_suspended": "monitor3-renewed-quit 1792551999"
},
"supervisors": {
"032897115e664051b23f79838449a964": {
"outcome": "supervised",
"worker_id": "032897115e664051b23f79838449a964"
},
"086cadf4f3ab4da2b0873b29c3c1c75a": {
"outcome": "supervised",
"worker_id": "086cadf4f3ab4da2b0873b29c3c1c75a"
}
}
},
"repeated": {
"auth_expired": [],
"cards": {},
"cards_closed": [],
"decisions": {
"032897115e664051b23f79838449a964": "nothing",
"086cadf4f3ab4da2b0873b29c3c1c75a": "nothing"
},
"escalations": [],
"escapes": {},
"input_watches": {
"032897115e664051b23f79838449a964": {
"outcome": "watching",
"worker_id": "032897115e664051b23f79838449a964"
},
"086cadf4f3ab4da2b0873b29c3c1c75a": {
"outcome": "watching",
"worker_id": "086cadf4f3ab4da2b0873b29c3c1c75a"
}
},
"rearms": {},
"recoveries": {},
"resume": {
"restarted": {},
"was_suspended": null
},
"supervisors": {
"032897115e664051b23f79838449a964": {
"outcome": "supervised",
"worker_id": "032897115e664051b23f79838449a964"
},
"086cadf4f3ab4da2b0873b29c3c1c75a": {
"outcome": "supervised",
"worker_id": "086cadf4f3ab4da2b0873b29c3c1c75a"
}
}
},
"incarnations": 2
},
{
"at": "2026-10-21T03:06:48.429200+00:00",
"milestone": "journey assertions passed"
},
{
"at": "2026-10-21T03:06:53.419370+00:00",
"milestone": "cleanup complete",
"processes": [
{
"pid": 56087,
"exit": 0,
"reaped": true
},
{
"pid": 55901,
"exit": 0,
"reaped": true
},
{
"pid": 52282,
"exit": 0,
"reaped": true
},
{
"pid": 52221,
"exit": 0,
"reaped": true
}
],
"tracked_remaining": []
}
]The driver exited 0; cleanup reaped all direct children and found no remaining tmux descendants. The original requester should evaluate whether these concrete observations satisfy the remaining requirement; this does not grant approval or waive any repository gate. Complete retained observation driver (scratch path replaced by <scratch>)This is the successful script reconstructed from its recorded creation command and two recorded temporary observer corrections. It is shown for inspection, not as a portable one-command test: import os,sys,json,time,subprocess,signal,shlex,hashlib,runpy,traceback
from pathlib import Path
from datetime import datetime,timezone
R=Path('<scratch>');D=R/'diagnostics';REPO=Path.cwd();S=REPO/'skills/worker-pool/scripts'
E=os.environ.copy();E.update(json.loads((R/'environment.json').read_text()));E.update({'PYTHONDONTWRITEBYTECODE':'1','HOME':str(R/'monitor3-home'),'SUTANDO_TELEMETRY':'0','ANTHROPIC_BASE_URL':'http://factory-runtime.invalid','SUTANDO_NOTIFIER_GRACE_PERIOD':'300','SUTANDO_NOTIFIER_ROLE_POLL':'1','SUTANDO_TMUX_SOCKET':str(R/'monitor3-tmux.sock')})
E.update({'SUTANDO_WORKSPACE':str(R/'monitor3-workspace'),'SUTANDO_WORKSPACE_DIR':str(R/'monitor3-workspace'),'SUTANDO_DEFAULT_WORKSPACE':str(R/'monitor3-workspace'),'SUTANDO_RESULTS_DIR':str(R/'monitor3-workspace/results'),'SUTANDO_RUNTIME_STATE':str(R/'monitor3-workspace/state'),'SUTANDO_RUNTIME_DB':str(R/'monitor3-workspace/state/runtime.sqlite'),'SUTANDO_HA_DIR':str(R/'monitor3-workspace/state/ha'),'SUTANDO_RUNTIME_SOCKET':str(R/'monitor3-api.sock'),'CLAUDE_CONFIG_DIR':str(R/'monitor3-claude')})
if len(sys.argv)>1 and sys.argv[1]=='trace-entry': E['PATH']=str(R/'monitor3-endpoint')+os.pathsep+E['PATH']
os.environ.clear();os.environ.update(E)
J=D/'monitor3-journey.jsonl'
def record(**kw):
row={'at':datetime.now(timezone.utc).isoformat(),**kw}
with J.open('a') as f:f.write(json.dumps(row,default=str)+'\n')
if 'milestone' in row: print(json.dumps(row,default=str),flush=True,file=sys.stderr)
def wait(pred,desc,timeout=25):
stop=time.monotonic()+timeout
while time.monotonic()<stop:
if pred():return
time.sleep(.1)
raise AssertionError('timeout: '+desc)
if len(sys.argv)>1 and sys.argv[1]=='trace-entry':
barrier=Path(sys.argv[2]);target=sys.argv[3];sys.argv=sys.argv[3:]
def trace(frame,event,arg):
if frame.f_code.co_filename==str(S/'spawn_worker.py') and frame.f_code.co_name=='spawn':
if event=='return' and isinstance(arg,dict) and arg.get('worker_id'):
record(milestone='post-launch barrier',pid=os.getpid(),worker=arg['worker_id'],session=arg['runtime_session_id'])
barrier.with_suffix('.ready').write_text(json.dumps(arg))
wait(lambda:barrier.with_suffix('.release').exists(),'separate suspend completion',90)
sys.settrace(None)
return trace
return None
sys.settrace(trace)
runpy.run_path(target,run_name='__main__');sys.exit(0)
home=Path(E['HOME']);(home/'Library').mkdir(parents=True,exist_ok=True)
(home/'Library/LaunchAgents').write_text('Disposable timer-installation exclusion: deliberate non-directory.\n')
endpoint=R/'monitor3-endpoint';endpoint.mkdir(exist_ok=True)
children=[];logs=[];tm=None;api=None;barrier=R/'monitor3-resume-unique';ws=Path(E['SUTANDO_WORKSPACE']);ws.mkdir(exist_ok=True)
def run(argv,check=True,timeout=40,stdin=None,env=None):
argv=list(map(str,argv));start=datetime.now(timezone.utc).isoformat()
p=subprocess.Popen(argv,cwd=REPO,env=env or E,stdin=subprocess.PIPE if stdin is not None else subprocess.DEVNULL,stdout=subprocess.PIPE,stderr=subprocess.PIPE,text=True)
try:out,err=p.communicate(stdin,timeout=timeout)
except BaseException:
p.terminate();out,err=p.communicate(timeout=10);record(command=argv,pid=p.pid,start=start,exit=p.returncode,stdout=out[-16000:],stderr=err[-16000:],timed_out=True);raise
record(command=argv,pid=p.pid,start=start,exit=p.returncode,stdout=out[-16000:],stderr=err[-16000:],reaped=True)
if check:assert p.returncode==0,(argv,p.returncode,out,err)
return subprocess.CompletedProcess(argv,p.returncode,out,err)
def start(argv,name):
log=(D/(name+'.log')).open('w');logs.append(log)
p=subprocess.Popen(list(map(str,argv)),cwd=REPO,env=E,stdout=log,stderr=log);children.append(p)
record(milestone='process started',name=name,command=argv,pid=p.pid);return p
def tmux(*args,check=True):return run(['tmux','-S',E['SUTANDO_TMUX_SOCKET'],*args],check=check)
def cli(*args):return json.loads(run(['python3',REPO/'src/runtime-cli/sutando-runtime.py',*args]).stdout)
def remedy(*args):return json.loads(run(['python3',S/'pool_remedy.py','--workspace',ws,'--repo',REPO,*args],timeout=90).stdout)
def api_start(name):
p=start(['python3',REPO/'src/runtime-api/server.py'],name)
sock=Path(E['SUTANDO_RUNTIME_SOCKET'])
wait(lambda:sock.exists() or p.poll() is not None,'runtime socket')
assert p.poll() is None
health=cli('runtime','health');record(milestone='API ready',pid=p.pid,health=health)
return p
def gone(pid):
try:os.kill(pid,0);return False
except ProcessLookupError:return True
def descendants(pid):
p=run(['pgrep','-P',pid],check=False)
assert p.returncode in (0,1),p.stderr
return [nested for c in map(int,p.stdout.split()) for nested in [*descendants(c),c]]
try:
commit=run(['git','rev-parse','HEAD']).stdout.strip();assert commit=='d849a655ec2924d86e57e88a84a7805001a967e5'
assert not run(['git','status','--porcelain=v1']).stdout
record(milestone='candidate',commit=commit,adaptations=['supplied PATH and PYTHONPATH preserved','synthetic C endpoint: lifecycle and argv only','scratch HOME/Library/LaunchAgents regular file prevents timer installation before launchctl','trace-only post-launch barrier; no product function replaced'])
run(['cc',R/'runtime-endpoint.c','-o',endpoint/'claude'])
E['PATH']=str(endpoint)+os.pathsep+E['PATH'];os.environ['PATH']=E['PATH']
tm=start(['tmux','-D','-f','/dev/null','-S',E['SUTANDO_TMUX_SOCKET']],'monitor3-tmux')
wait(lambda:Path(E['SUTANDO_TMUX_SOCKET']).exists() or tm.poll() is not None,'private tmux');assert tm.poll() is None
tmux('set-option','-g','default-shell','/bin/bash');tmux('new-session','-d','-s','monitor3-anchor','sleep','900')
api=api_start('monitor3-api-before')
sys.path.insert(0,str(S));import pool_remedy as rem
wi,sw,sup,pd,ps=rem.wi,rem.sw,rem.sup,rem.pd,rem.ps
def absent(w):return tmux('has-session','-t','='+wi.tmux_session_name(w),check=False).returncode==1
def endpoint_pid(w,flag,sid):
name=wi.tmux_session_name(w)
pid=int(tmux('list-panes','-t','='+name+':0','-F','#{pane_pid}').stdout)
argv=shlex.split(run(['ps','-p',pid,'-o','args=']).stdout)
assert Path(argv[0]).name=='claude' and argv[argv.index(flag)+1]==sid and argv[argv.index('--name')+1]==name,argv
pane=tmux('capture-pane','-p','-t','='+name+':0.0').stdout
assert 'factory synthetic endpoint ready' in pane
record(milestone='worker ready',worker=w,pid=pid,argv=argv,pane=pane.strip());return pid
workers=[]
for label in ['Monitor first','Monitor second']:
folder=R/label.replace(' ','-');folder.mkdir(exist_ok=True)
row=json.loads(run(['python3',S/'create_worker.py','--workspace',ws,'--repo',REPO,'--folder',folder,'--label',label,'--runtime','claude','--socket',E['SUTANDO_TMUX_SOCKET'],'--json'],timeout=60).stdout)
assert row['remedy_timer']['ensured'] is False and 'FileExistsError' in row['remedy_timer']['why'],row
endpoint_pid(row['worker_id'],'--session-id',row['runtime_session_id']);workers.append(row)
ids=list(sup.supervised_workers(ws));assert len(ids)==2
first,held=ids
sw.pr.bind_room(ws,'runtime-api',held)
evidence=ps.WorkerEvidence(watcher_consecutive=3,watcher_first_detected_at=10.0,rearm_issued_at=20.0,wedge_consecutive=2,wedge_first_detected_at=30.0,last_pane_id='monitor3-evidence')
sup.save_state(ws,ps.SupervisionState(last_sample_at=time.time(),workers={w:evidence for w in ids}))
record(milestone='fixture setup through production writers',first=first,held=held,binding='runtime-api',supervision=evidence.__dict__)
remedy('--suspend','monitor3-initial-quit')
for w in ids:
pid=int(tmux('list-panes','-t','='+wi.tmux_session_name(w)+':0','-F','#{pane_pid}').stdout)
os.kill(pid,signal.SIGTERM);wait(lambda:absent(w),'worker stopped');wait(lambda:gone(pid),'worker reaped')
wait(lambda:tmux('list-sessions','-F','#{session_name}').stdout.strip()=='monitor3-anchor','old worker supervisors exited',40)
before_pid=api.pid;api.terminate();api.wait(timeout=10);record(milestone='API stopped',pid=api.pid,exit=api.returncode,reaped=True)
api=api_start('monitor3-api-after');assert api.pid!=before_pid
taskids=[]
for tag in ['pending','accepted']:
out=cli('task','submit','Monitor post-relaunch '+tag,'--idempotency-key','monitor3-'+tag)
tid=out['taskId'];taskids.append(tid)
run(['python3',S/'pool_route_handler.py','--workspace',ws,'--task-file',ws/'tasks'/(tid+'.txt')])
sentinel=pd.find(ws,held,tid);assert sentinel is not None
payload=Path(run([S/'resolve-inbox-entry',sentinel,'--workspace',ws]).stdout.strip())
assert 'Monitor post-relaunch '+tag in payload.read_text()
if tag=='accepted':pd.accept(sentinel);pd.mark_done(ws,held,tid,published=False)
assert len(pd.pending(ws,held))==1 and len(pd.accepted(ws,held))==1
def owned():
paths=[wi.sessions_path(ws,held),sw.pr.roster_path(ws)]
for base in [pd.deliveries_dir(ws,held),ws/'tasks',ws/'state/attribution',wi.worker_dir(ws,held)/'done']:
if base.exists():paths.extend(p for p in base.rglob('*') if p.is_file())
return {str(p.relative_to(ws)):p.read_bytes() for p in paths}
before_owned=owned();before_runs=wi.incarnations_path(ws,held).read_bytes();before_current=wi.current_path(ws,held).read_bytes()
before_total={w:len(wi.incarnations(ws,w)) for w in ids}
command=['python3',R/'monitor3-witness.py','trace-entry',barrier,S/'pool_remedy.py','--workspace',ws,'--repo',REPO,'--resume']
output=(D/'monitor3-resume.stdout').open('w');error=(D/'monitor3-resume.stderr').open('w');logs.extend([output,error])
resume=subprocess.Popen(list(map(str,command)),env=E,cwd=REPO,stdout=output,stderr=error);children.append(resume);record(command=command,pid=resume.pid)
wait(lambda:barrier.with_suffix('.ready').exists() or resume.poll() is not None,'first successful resume launch',60)
assert resume.poll() is None,(D/'monitor3-resume.stderr').read_text()
ready=json.loads(barrier.with_suffix('.ready').read_text());assert ready['worker_id']==first
firstpid=endpoint_pid(first,'--resume',ready['runtime_session_id']);assert absent(held)
renewed=remedy('--suspend','monitor3-renewed-quit');marker=rem.suspended_path(ws).read_bytes();assert held in json.loads(marker)['stopped']
record(milestone='separate suspend completed before barrier release',marker=json.loads(marker),resume_pid=resume.pid)
barrier.with_suffix('.release').touch();assert resume.wait(timeout=60)==0
result=json.loads((D/'monitor3-resume.stdout').read_text());record(milestone='post-relaunch resume outcome',pid=resume.pid,result=result)
assert result['resume']['restarted'][held]['outcome']=='suspended' and result['resume']['restarted'][first]['outcome']=='recovered'
def unchanged():
assert absent(held)
assert rem.suspended_path(ws).read_bytes()==marker
assert owned()==before_owned
assert wi.incarnations_path(ws,held).read_bytes()==before_runs
assert wi.current_path(ws,held).read_bytes()==before_current
assert sup.load_state(ws).workers[held]==evidence
assert endpoint_pid(first,'--resume',ready['runtime_session_id'])==firstpid
unchanged()
for n in range(2):
sweep=remedy('--sweep');assert sweep['suspended'].startswith('monitor3-renewed-quit');unchanged();record(milestone='repeat suspended sweep',repeat=n+1,result=sweep,preserved_files={k:hashlib.sha256(v).hexdigest() for k,v in before_owned.items()},held_incarnations=len(wi.incarnations(ws,held)))
original_session=wi.sessions(ws,held)[-1]['session_id'];later=remedy('--resume');assert list(later['resume']['restarted'])==[held] and later['resume']['restarted'][held]['outcome']=='recovered'
heldpid=endpoint_pid(held,'--resume',original_session);assert len(wi.incarnations(ws,held))==before_total[held]+1 and owned()==before_owned
assert endpoint_pid(first,'--resume',ready['runtime_session_id'])==firstpid
again=remedy('--resume');assert again['resume']['restarted']=={};assert len(wi.incarnations(ws,held))==before_total[held]+1
record(milestone='later resume recovered exactly once',held=held,pid=heldpid,original_session=original_session,later=later,repeated=again,incarnations=len(wi.incarnations(ws,held)))
tid=taskids[0];sentinel=pd.find(ws,held,tid);payload=Path(run([S/'resolve-inbox-entry',sentinel,'--workspace',ws]).stdout.strip());assert 'Monitor post-relaunch pending' in payload.read_text()
answer='Local monitor consumed the preserved post-relaunch payload; synthetic lifecycle endpoint, no model conversation.\n'
run(['python3',REPO/'src/result_publish.py',ws/'results'/(tid+'.txt')],stdin=answer)
retrieved=cli('task','get-result',tid);assert answer.strip() in json.dumps(retrieved)
status=cli('task','status',tid)
repeat=cli('task','submit','Monitor post-relaunch pending','--idempotency-key','monitor3-pending');assert repeat['taskId']==tid
record(milestone='real API post-relaunch task round trip',server_before=before_pid,server_after=api.pid,task_id=tid,worker=held,resolver_payload=payload,result=retrieved,status=status,repeated_submission=repeat,consumer='production one-shot routing and inbox resolver; operator-published test result, no automatic watcher/model claim')
assert not run(['git','status','--porcelain=v1']).stdout
(D/'monitor3-outcome.json').write_text(json.dumps({'status':'PASS','commit':commit,'first':first,'held':held,'first_pid':firstpid,'held_pid':heldpid,'server_before':before_pid,'server_after':api.pid,'task_ids':taskids,'owned_sha256':{k:hashlib.sha256(v).hexdigest() for k,v in before_owned.items()}},indent=2))
record(milestone='journey assertions passed')
except BaseException:
record(milestone='journey error',error=traceback.format_exc());raise
finally:
barrier.with_suffix('.release').touch()
cleanup=[]
for p in reversed(children):
if p is tm:continue
if p.poll() is None:p.terminate()
try:p.wait(timeout=10)
except subprocess.TimeoutExpired:p.kill();p.wait(timeout=5)
cleanup.append({'pid':p.pid,'exit':p.returncode,'reaped':True})
if tm is not None:
try:
for pid in descendants(tm.pid) if tm.poll() is None else []:
try:os.kill(pid,signal.SIGTERM)
except ProcessLookupError:continue
try:wait(lambda:gone(pid),'descendant '+str(pid),6)
except AssertionError:
try:os.kill(pid,signal.SIGKILL)
except ProcessLookupError:pass
wait(lambda:gone(pid),'killed descendant '+str(pid),5)
remaining=descendants(tm.pid) if tm.poll() is None else [];assert not remaining,remaining
finally:
if tm.poll() is None:tmux('kill-server',check=False)
tm.wait(timeout=10);cleanup.append({'pid':tm.pid,'exit':tm.returncode,'reaped':True})
for f in logs:f.close()
record(milestone='cleanup complete',processes=cleanup,tracked_remaining=[p.pid for p in children if p.poll() is None])
(D/'monitor3-cleanup.json').write_text(json.dumps({'processes':cleanup,'tracked_remaining':[p.pid for p in children if p.poll() is None]},indent=2))Disclosed lifecycle endpoint source used for the worker processesThis small endpoint keeps a real process and terminal alive and exposes its session arguments. It performs no model inference or authentication. /* Synthetic long-lived process for launcher argument/lifecycle tests only.
* It is NOT a model, authentication fixture, or conversation implementation.
* Compile under a disposable directory with output name claude or codex.
* Observe its actual argv and PID through the scoped process observer.
*/
#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
static volatile sig_atomic_t done = 0;
static void stop(int sig) { (void)sig; done = 1; }
int main(int argc, char **argv) {
if (argc == 2 && strcmp(argv[1], "--version") == 0) {
puts("factory synthetic runtime endpoint v1 (no model)"); return 0;
}
if (argc == 3 && strcmp(argv[1], "login") == 0 && strcmp(argv[2], "status") == 0) {
puts("factory synthetic login-status response; no account or authentication tested"); return 0;
}
signal(SIGTERM, stop); signal(SIGINT, stop); signal(SIGHUP, stop);
puts("factory synthetic endpoint ready; no conversation or authentication");
fflush(stdout);
while (!done) sleep(1);
return 0;
} |
sonichi
reviewed
Oct 21, 2026
sonichi
left a comment
sonichi
left a comment
Owner
There was a problem hiding this comment.
My REVIEW.md §15 request is satisfied at d849a65 by #5791 (comment). On real tmux, using this head's pool scripts in a scratch workspace, the suspension was renewed after the first worker's launch and before the held worker's recover(). The output shows:
- the held worker's outcome was "suspended";
- no new session or process was spawned for it;
- it stayed in the renewed marker's stopped list, and the marker and incarnation files were unchanged across repeated suspended sweeps;
- the first worker kept its pid;
- a later --resume recovered the held worker exactly once, on its original session, with exactly one new incarnation.
The synthetic endpoint is disclosed, and it doesn't affect the behaviour under test. I'm no longer holding this PR.
keweichen
deleted the
factory/kc/publication-repair-75cf9db89b58cb7a-ac4d4c851f7a
branch
October 21, 2026 04:16
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Priority
Level: medium
Reason: Owner-approved reliability improvement.
Closes
N/A — owner-approved work; no public issue is claimed as closed.
Summary
fix(worker-pool): honor renewed suspension between resumed workers
Review first:
skills/worker-pool/scripts/pool_remedy.py,tests/skills/worker-pool/pool-remedy.test.py,tests/skills/worker-pool/pool-resume-suspension.test.py,tests/skills/worker-pool/pool-suspend-resume.test.py.Checklist
gh pr list --repo sonichi/sutando --search "closes #N")git log -1 --format='%ae'shows a GH-mapped email, not*.localornoreply@anthropic.com)upstream/main(or feature isn't already covered)branches:filter can silently exclude a stacked PR)resolve_workspace/resolveWorkspace/claude_home_pathhelpers (seeCLAUDE.md"Workspace contract"; enforced by thelint-workspace-resolution,lint-claude-home-path, andworkspace-leak-checkCI checks)Contribution observations (unchecked items remain reviewer decisions):
Duplicate search: Searched PRs (all states) and open issues using fix(worker-pool): honor renewed suspension between resumed workers, "pool_remedy.py". Related results for reviewer comparison: docs(pool): worker-pool design v1 — step 1 of staging #3604 (no router process) #3860, worker-pool: per-instance opt-in for --chrome #4789, fix(watcher): an inherited SUTANDO_WORKSPACE_DIR no longer claims a foreign inbox #5371, fix(worker-pool): recover the last running conversation #5777, feat(worker-pool): the owning host can suspend and resume the pool's remedy #4879, fix(worker-pool): persist cards for human wedges and repair uncarded acknowledgments #5507, fix(worker-pool): close a pressed proxy remedy card when its worker is positively observed recovered #5503, fix(claude-launch): forward the resolved configuration directory to tmux #5780, fix(health): report owner-paused workers as suspended, not abnormal offline #5448, feat(worker-pool): pool_remedy --restart, the owner's restart of one worker [HealthStatus] #4952, fix(alerts): one usage-limit card per episode, with unblock steps; macOS pop-up only as DM fallback #5686, fix(worker-pool): declare the router on every path that makes a pool exist #4580 Returned PRs were compared against current files and overlapping diffs: docs(pool): worker-pool design v1 — step 1 of staging #3604 (no router process) #3860: Different changed files.; worker-pool: per-instance opt-in for --chrome #4789: Different changed files.; fix(watcher): an inherited SUTANDO_WORKSPACE_DIR no longer claims a foreign inbox #5371: Different changed files.; fix(worker-pool): recover the last running conversation #5777: Related files, different patch; current-source reproduction and independent review remain required.; feat(worker-pool): the owning host can suspend and resume the pool's remedy #4879: Related files, different patch; current-source reproduction and independent review remain required.; fix(worker-pool): persist cards for human wedges and repair uncarded acknowledgments #5507: Related files, different patch; current-source reproduction and independent review remain required.; fix(worker-pool): close a pressed proxy remedy card when its worker is positively observed recovered #5503: Related files, different patch; current-source reproduction and independent review remain required.; fix(claude-launch): forward the resolved configuration directory to tmux #5780: Different changed files.; fix(health): report owner-paused workers as suspended, not abnormal offline #5448: Different changed files.; feat(worker-pool): pool_remedy --restart, the owner's restart of one worker [HealthStatus] #4952: Related files, different patch; current-source reproduction and independent review remain required.; fix(alerts): one usage-limit card per episode, with unblock steps; macOS pop-up only as DM fallback #5686: Related files, different patch; current-source reproduction and independent review remain required.; fix(worker-pool): declare the router on every path that makes a pool exist #4580: Different changed files.. All 32 returned PRs were compared; this public excerpt lists the first twelve. Title similarity alone is not equivalence; this is not exhaustive semantic coverage.
Upstream observed during evidence preparation:
1bcfbf20f14dfa1a398ff2cc3c4f8b6fb05c21a6. Main advanced after the recorded source base. Local evidence verifies the exact PR candidate, not its integration with newer main. PR checks, independent review and branch maintenance govern merge.Diff review: - review-correctness: Inspected recover(), resume(), restart(), apply(), _sweep(), suspension parsing, supervision persistence and production creation/launch callers. The guard at pool_remedy.py:222 precedes recovery probes and writes. The filter at line 110 preserves suspended workers' evidence while retaining existing treatment of other outcomes. Focused assertions fail with the original coordinator loaded from Git and pass with the candidate.
review-scope: HEAD matches the exact candidate. The diff against original base 7f7f56a hashes to supplied SHA256 a5a0f54da668a57ed44ebaecabb4e41c525c83940ecc17e03cfe52b304acc5b5. All four changed paths are allowed. The immediate-parent repair adds only a blank line between test imports. Launcher and selection policies remain unchanged; final Git status is clean.
review-maintainability: The fix reuses the shared suspension reader and existing outcome contract without introducing another policy implementation. Tests cover the actual interleaving and selective bookkeeping. git diff --check and scripts/review-checks.sh passed, including the previously reported comment-cap violation.
Documentation: No standalone documentation files changed. The reviewer should assess the code and contract comments for documentation completeness.
Live-path verification: - monitor-user-journey: Executed the separate CLI journey saved in monitor-journey-command.txt, using fixture setup and synchronization without invoking a test method. Production creation launched two workers through the unchanged launcher. At 2026-10-20T22:21:55Z, a separate pool_remedy.py --suspend monitor-renewed-stop completed after the first recovery launch and before continuation. Resume returned first='recovered', second='suspended'; only the first launched. Full argv, outputs and readiness observations are in monitor-journey.json.
monitor-repeatability: Ran pool_remedy.py --sweep and --restart 8fc6e16e625345809b331426ec489315 three times while suspended. Each reported suspension, with no launch or new incarnation. Later --resume recovered that worker exactly once; scoped ps confirmed PID 38565 received --resume 6eb2667c-0bb1-4376-97cc-8f735a863b8e. Another --resume returned restarted={}. First-worker PID 36911 survived throughout. Exact commands and outputs: monitor-journey.json.
These observations used the isolated candidate; no production deployment was performed.
Rollback: revert this PR. Production behavior after deployment remains to be observed.
keweichen; CLA status must be observed on the PR.Before / after evidence
Before
python3 [local path]exited 0: a separate suspend CLI completed after the first recovery while the pool lock was held, yet the second worker recovered and incarnations increased from 1 to 2. The renewed marker survived; second-worker supervision evidence did not. Injecting a suspended outcome independently reproduced evidence loss. This source-level witness used the existing FakeTmux runner, not real lifecycle validation. Results are recorded in diagnostics/triage-baseline-observation.json. The environment-loaded process-tools.py preflight passed private sockets, tmux 3.8 and scoped process inspection; diagnostics/capability-preflight.json explicitly reports product readiness unobserved.Earlier baseline observations at 7f7f56a. These are historical; current-candidate evidence follows below.
Exit: 0
Exit: 0
After
acceptance-1: The new lifecycle test used production creation, the unchanged launcher, supplied subprocess environment and foreground private tmux. In the final private-home run, the first recovery launched at 22:10:12 UTC; a separate --suspend completed before recovery continued. At 22:10:13, resume reported the second worker as suspended, with no launch or new incarnation. Commands, readiness and outcomes are retained in lifecycle-evidence.json.
acceptance-2: Lifecycle assertions verified preservation of the renewed marker, skipped worker identity, lineage, roster label, pending/accepted ownership, payloads and supervision evidence. Two suspended sweeps launched nothing. Later explicit resume recovered that worker exactly once using its recorded session; another resume launched nothing.
acceptance-3: Without renewed local suspension, both workers recovered despite a foreign workspace suspension. Owner-paused workers stayed paused. The already-started worker retained PID 9045 through subsequent sweeps and resume. Focused tests also passed live-worker, indeterminate-probe, failed-launch, runtime-selection and ordinary-restart cases.
acceptance-4: Focused tests exercised the shared guard before probes/writes, selective resume evidence reset and existing suspension-between-sweep-actions behavior. Both lifecycle runs cleaned up successfully: six foreground tmux servers were reaped, all 83 recorded process IDs were absent, and scoped pgrep found no tmux, claude or fswatch descendants. See final-audit.json. No model conversation or authentication claim is made.
check-resume-suspension: python3 tests/skills/worker-pool/pool-resume-suspension.test.py: 3 tests passed, exit 0, in 95.829 seconds. Repeated at 2026-10-20T22:09:04Z with an explicit private scratch HOME: 3 passed, exit 0, in 84.753 seconds, within the 180-second limit. See resume-suspension-private-home.log and its JSON receipt.
check-suspend-resume-regressions: 2026-10-20T22:07:13Z: python3 tests/skills/worker-pool/pool-suspend-resume.test.py — 15 tests passed, exit 0, 0.491 seconds; limit 60 seconds.
check-recovery-regressions: 2026-10-20T22:07:13Z: python3 tests/skills/worker-pool/pool-remedy.test.py — 34 tests passed, exit 0, 1.577 seconds; limit 60 seconds.
check-restart-regressions: 2026-10-20T22:07:13Z: python3 tests/skills/worker-pool/pool-restart.test.py — 16 tests passed, exit 0, 1.997 seconds; limit 60 seconds.
runtime-preflight: Loaded environment.json and ran python3 [local path] preflight. Private sockets, PTY, actual tmux session, scoped process inspection, temporary files and compiler passed. Supplied PATH/PYTHONPATH were preserved, including fswatch poll_monitor, scratch mktemp and xcrun --no-cache adaptations. See capability-preflight.json.
coverage: Instrumented all 65 focused regression cases with python3 -m coverage run --rcfile=.coveragerc; all passed. diff-cover against original base 7f7f56a with --fail-under=95 measured 100%: 3 changed executable lines, none missing. No changed product files were unmeasured. Module coverage was 91%. Commands and measurements are retained under diagnostics/coverage.
repository-checks: git diff --check against the original base passed. bash scripts/review-checks.sh --diff [local path] passed hardcoded-path, root-artifact and comment-cap checks.
candidate-scope: Initial and final HEAD matched the exact candidate. All four changed paths are approved, the launcher is unchanged from the original base, and final git status --porcelain=v1 --untracked-files=all was empty at 2026-10-20T22:10:57Z.
Test plan
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Observed: PASS; exit 0.
Canonical review scanner:
Verified candidate:
4b455c41f89ede82054913bf2782c61bc53a39b6. Publication commit:d849a655ec2924d86e57e88a84a7805001a967e5. Both have tree9cead8c68003742b12a536327d3c0bc6125cba4d; only factory-authored commit metadata changed. Acceptance evidence refers to the verified candidate; the scanner ran on the publication tree.