Skip to content

fix(custody): reclaim a hold whose holder PROCESS IS DEAD, without waiting out the TTL (model_performance-oy4) #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Brian Krabach (bkrabach) merged 2 commits into from
Sep 3, 2026
Merged

fix(custody): reclaim a hold whose holder PROCESS IS DEAD, without waiting out the TTL (model_performance-oy4) #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d83e811
fix(custody): reclaim a hold whose holder PROCESS IS DEAD, without wa…
microsoft-amplifier Sep 3, 2026
f8b971b
test+docs(oy4): unit pins for the reaper's list-page and per-item iso…
microsoft-amplifier Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ Nothing above the seam should ever need to change for a Beads upgrade.
## `doctor` is the gate, not a suggestion

Run `amplifier-work-tracker doctor` after any `bd` upgrade and before
trusting parallel agents against a queue. It must report **37/37
trusting parallel agents against a queue. It must report **38/38
assumptions hold**; anything less means Beads' behavior moved out from
under an assumption we depend on (or, for `sweeps.alive`, that the
reap/notify sweep loops have stopped completing sweeps, or, for
`sweeps.reclaiming`, that the reap loop is still turning but its last
sweep FAILED on one or more projects -- alive and not reclaiming are
different states, and `sweeps.alive` alone cannot tell them apart, or, for
`project.removal`, that `remove`/`new` no longer honestly handle a
database that outlives its project directory, or, for
`service.restart_policy`, that the installed unit's Restart= line has
Expand All @@ -50,7 +53,10 @@ not 33+2 arithmetic, which would have said 35. The two
destructive-reopen defect, `model_performance-2nx` -- then take it to
**36**, again MEASURED from `doctor`, not computed. `read.unavailable_not_absent`
(model_performance-8zv) makes it **37** -- measured on the rebased branch, not
computed from 36+1.)
computed from 36+1. `sweeps.reclaiming` (model_performance-oy4) makes it
**38** -- again MEASURED by running `doctor` on this branch, not computed;
the run is committed at
`docs/lanes/oy4-dead-holder-reclaim/evidence/doctor-measured.txt`.)

## Test scope

Expand Down Expand Up @@ -136,7 +142,7 @@ runs itself is how you lose data you meant to keep.

## What "done" looks like

Full suite green, `doctor` 37/37, `ruff check` / `ruff format --check` /
Full suite green, `doctor` 38/38, `ruff check` / `ruff format --check` /
`pyright` clean. For any change to the bundle's zero-state install path
(service bootstrap, `work_tracker_install`, prereqs), the acceptance gate is
a fresh Digital Twin Universe run from a genuinely empty machine (no `bd`,
Expand Down
287 changes: 287 additions & 0 deletions docs/lanes/oy4-dead-holder-reclaim/DONE-NOTE.md
View file
Open in desktop

Large diffs are not rendered by default.

67 changes: 67 additions & 0 deletions docs/lanes/oy4-dead-holder-reclaim/evidence/cli-tier-jyg-failure.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
F [100%]
=================================== FAILURES ===================================
___________ test_doctor_quick_succeeds_against_the_real_installed_bd ___________

run_cli = <function run_cli.<locals>._run at 0xeb8d1098ec00>

def test_doctor_quick_succeeds_against_the_real_installed_bd(run_cli):
result = run_cli(["doctor", "--quick"])
> assert result.returncode == 0, result.stdout + result.stderr
E AssertionError: [PASS] version bd 1.1.2
E [PASS] capabilities all required bd commands present
E [PASS] read.unavailable_not_absent an infrastructure read failure raises BeadsUnavailableError with its cause intact on read/claim and reports UNAVAILABLE (not ERROR) per project, while genuine absence on a healthy database still reports plain 'not found'
E [PASS] resolve.fenced stale holder refused, as required
E [PASS] resolve.divergent_text_refused resolving a closed item with different text refuses and writes nothing
E [PASS] resolve.identical_text_idempotent re-sending identical resolution text is an idempotent success
E [PASS] reopen.reopens a resolved item reopens unassigned and is directly claimable again
E [PASS] reopen.clears_closed_at reopen clears closed_at (the documented, surfaced accounting cost)
E [PASS] reopen.close_reason_disposition reopen clears close_reason (measured), and the wrapper's archive comment preserves the previous resolution regardless
E [PASS] reopen.emits_event bd records a `reopened` events row, attributed
E [PASS] defer.refuses_resolved defer on a resolved item refuses, writes nothing, and names `reopen`
E [PASS] block.refuses_resolved block on a resolved item refuses, writes nothing, and names `reopen`
E [PASS] release.reopens_unresolved release reopens a held item with no resolution, and it is re-claimable
E [PASS] claim.subcommand --claim present, rejects --assignee as expected
E [PASS] claim.atomic skipped (--quick); run full doctor before trusting parallel agents
E [PASS] claim.directed_atomic skipped (--quick); run full doctor before trusting parallel agents
E [PASS] link.nonblocking discovered-from is non-blocking
E [PASS] list.includes_closed all-flag required and working
E [PASS] list.status_filter_includes_closed an explicit --status filter shows closed items without --all
E [PASS] show.dependents reverse link visible (1 links)
E [PASS] read.no_mutation repeated reads (including not-found/wrong-project misses) leave status, holder, and metadata unchanged
E [PASS] resolution.readable resolution text round-trips
E [PASS] timestamps.readable created_at/updated_at/closed_at all round-trip as real datetimes
E [PASS] metadata.roundtrip arbitrary JSON metadata round-trips
E [PASS] project.name_rules dotted names appear usable now; validator may be relaxed
E [PASS] custody.fresh_survives a fresh renewal survives regardless of total hold duration
E [PASS] custody.stale_reclaimed stale custody is reclaimed: custody stale -- last seen 3600s ago (ttl 900s)
E [PASS] custody.idle_not_exempt awaiting_human with stale custody is still reclaimed: custody stale -- last seen 3600s ago (ttl 900s)
E [PASS] custody.fenced old holder's renew and resolve are both refused after takeover
E [PASS] project.removal remove() drops both the directory and database; re-create afterward is genuinely empty
E [PASS] project.create_atomic an abandoned creation lock (dead pid) is healed automatically and create() completes fresh in the same call; path=/tmp/awtcontract_zf0anue7/projects/contract178844191296atomic
E [PASS] project.creation_state_reporting creation_state distinguishes none/creating/abandoned correctly
E [PASS] service.installed installed and active (unit: /home/bkrabach/.config/systemd/user/amplifier-work-tracker.service)
E [PASS] systemd.user_bus_reachable systemctl --user show-environment succeeded
E [PASS] dolt.reachable dolt sql-server responds on 127.0.0.1:35411
E [FAIL] sweeps.alive no heartbeat ever recorded for the reap sweep loop -- it may never have started, or the heartbeat file was removed; restart the service (`amplifier-work-tracker service restart`); no heartbeat ever recorded for the notify sweep loop -- it may never have started, or the heartbeat file was removed; restart the service (`amplifier-work-tracker service restart`)
E [PASS] sweeps.reclaiming skipped -- no reap heartbeat recorded yet (see the reap sweep liveness check, which reports that directly)
E [PASS] service.restart_policy installed unit has Restart=always -- survives a clean/unintended exit
E
E 1 assumption(s) VIOLATED. Beads has changed underneath us.
E Fix scope: amplifier_work_tracker/adapter.py only -- nothing above the seam encodes Beads behaviour.
E project 'contract178844191296atomic': healing an abandoned creation attempt (lock /tmp/awtcontract_zf0anue7/projects/contract178844191296atomic/.create.lock named a dead pid) before retrying
E
E assert 1 == 0
E + where 1 = CompletedProcess(args=['/home/bkrabach/dev/hw-model-performance/lanes/oy4-dead-holder-reclaim/amplifier-work-tracker/.... (lock /tmp/awtcontract_zf0anue7/projects/contract178844191296atomic/.create.lock named a dead pid) before retrying\n").returncode

tests/cli/test_cli_surface.py:831: AssertionError
---------------------------- Captured stdout setup -----------------------------
Starting server with Config HP="127.0.0.1:35411"|T="28800000"|R="false"|L="info"
---------------------------- Captured stderr setup -----------------------------
----------------------------- Captured stdout call -----------------------------


----------------------------- Captured stderr call -----------------------------
--------------------------- Captured stderr teardown ---------------------------
=========================== short test summary info ============================
FAILED tests/cli/test_cli_surface.py::test_doctor_quick_succeeds_against_the_real_installed_bd
1 failed in 46.41s
41 changes: 41 additions & 0 deletions docs/lanes/oy4-dead-holder-reclaim/evidence/doctor-measured.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
project 'contract1788440121343atomic': healing an abandoned creation attempt (lock /tmp/awtcontract_0m323x7t/projects/contract1788440121343atomic/.create.lock named a dead pid) before retrying
[PASS] version bd 1.1.2
[PASS] capabilities all required bd commands present
[PASS] read.unavailable_not_absent an infrastructure read failure raises BeadsUnavailableError with its cause intact on read/claim and reports UNAVAILABLE (not ERROR) per project, while genuine absence on a healthy database still reports plain 'not found'
[PASS] resolve.fenced stale holder refused, as required
[PASS] resolve.divergent_text_refused resolving a closed item with different text refuses and writes nothing
[PASS] resolve.identical_text_idempotent re-sending identical resolution text is an idempotent success
[PASS] reopen.reopens a resolved item reopens unassigned and is directly claimable again
[PASS] reopen.clears_closed_at reopen clears closed_at (the documented, surfaced accounting cost)
[PASS] reopen.close_reason_disposition reopen clears close_reason (measured), and the wrapper's archive comment preserves the previous resolution regardless
[PASS] reopen.emits_event bd records a `reopened` events row, attributed
[PASS] defer.refuses_resolved defer on a resolved item refuses, writes nothing, and names `reopen`
[PASS] block.refuses_resolved block on a resolved item refuses, writes nothing, and names `reopen`
[PASS] release.reopens_unresolved release reopens a held item with no resolution, and it is re-claimable
[PASS] claim.subcommand --claim present, rejects --assignee as expected
[PASS] claim.atomic 5 trials x 12 concurrent claimers, no double-claims
[PASS] claim.directed_atomic 5 trials x 12 concurrent directed claimers on the SAME item, exactly one winner each time
[PASS] link.nonblocking discovered-from is non-blocking
[PASS] list.includes_closed all-flag required and working
[PASS] list.status_filter_includes_closed an explicit --status filter shows closed items without --all
[PASS] show.dependents reverse link visible (1 links)
[PASS] read.no_mutation repeated reads (including not-found/wrong-project misses) leave status, holder, and metadata unchanged
[PASS] resolution.readable resolution text round-trips
[PASS] timestamps.readable created_at/updated_at/closed_at all round-trip as real datetimes
[PASS] metadata.roundtrip arbitrary JSON metadata round-trips
[PASS] project.name_rules dotted names appear usable now; validator may be relaxed
[PASS] custody.fresh_survives a fresh renewal survives regardless of total hold duration
[PASS] custody.stale_reclaimed stale custody is reclaimed: custody stale -- last seen 3600s ago (ttl 900s)
[PASS] custody.idle_not_exempt awaiting_human with stale custody is still reclaimed: custody stale -- last seen 3600s ago (ttl 900s)
[PASS] custody.fenced old holder's renew and resolve are both refused after takeover
[PASS] project.removal remove() drops both the directory and database; re-create afterward is genuinely empty
[PASS] project.create_atomic an abandoned creation lock (dead pid) is healed automatically and create() completes fresh in the same call; path=/tmp/awtcontract_0m323x7t/projects/contract1788440121343atomic
[PASS] project.creation_state_reporting creation_state distinguishes none/creating/abandoned correctly
[PASS] service.installed installed and active (unit: /home/bkrabach/.config/systemd/user/amplifier-work-tracker.service)
[PASS] systemd.user_bus_reachable systemctl --user show-environment succeeded
[PASS] dolt.reachable dolt sql-server responds on 127.0.0.1:3308
[PASS] sweeps.alive reap sweep completed 159s ago (threshold 900s); notify sweep completed 45s ago (threshold 900s)
[PASS] sweeps.reclaiming unknown -- the running supervisor predates per-sweep outcome reporting, so its reap heartbeat cannot say whether any project failed; restart the service (`amplifier-work-tracker service restart`) to start recording it
[PASS] service.restart_policy installed unit has Restart=always -- survives a clean/unintended exit

All 38 assumptions hold. Safe to run parallel agents.
48 changes: 48 additions & 0 deletions docs/lanes/oy4-dead-holder-reclaim/evidence/fail-before-pass-after.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
FAIL-BEFORE / PASS-AFTER for model_performance-oy4
==================================================

Probe: `failbefore_probe.py` (in this directory), run as a pytest integration
test against this repo's isolated dolt server + real `bd`. IDENTICAL script on
both trees -- the only difference is whether `src/` carries the fix (captured
by `git stash push -- src/` and `git stash pop`), so what changes below is
BEHAVIOUR, not an API that did not exist yet.

Scenario, reproducing the measured `model_performance-h6v` shape exactly:
an item claimed and given custody by an agent whose process is then genuinely
dead (a real subprocess, started and reaped, so `os.kill(pid, 0)` really does
report it gone), whose custody was last renewed 400 seconds ago -- i.e. WELL
INSIDE the documented 900s `CUSTODY_TTL_SECONDS`. Reaped with the default TTL:
exactly the call the background service makes every 300s.


-------------------------- FAIL-BEFORE (pre-fix tree) --------------------------

holder pid 3821490 running? False
custody last_seen 400s ago; CUSTODY_TTL_SECONDS=900
work_stats view: held=1 held_stale=0 held_stale_oldest_age_seconds=None
reap_project(default ttl): reclaimed_count=0 reasons=[]
after sweep: status=held holder='dead-agent'
successor work_claim: REFUSED -- claim failbefore26743d9e270d-57i as
'successor-agent' failed: Error claiming failbefore26743d9e270d-57i:
issue already claimed by dead-agent

Note `held_stale=0` with `held_stale_oldest_age_seconds=None` beside it, and a
refusal naming a dead agent id. That is the incident's signature verbatim --
the successor session measured exactly those three values on the live queue at
2026-09-03T07:52Z.


---------------------------- PASS-AFTER (with fix) ----------------------------

holder pid 3824385 running? False
custody last_seen 400s ago; CUSTODY_TTL_SECONDS=900
work_stats view: held=1 held_stale=1 held_stale_oldest_age_seconds=401.256
reap_project(default ttl): reclaimed_count=1 reasons=["holder process is dead
-- pid 3824385 on host 'spark-1' is not running, and custody has been silent
401s (corroboration window 240s); ttl 900s not yet reached, but the holder
is gone"]
after sweep: status=open holder=None
successor work_claim: SUCCESS

The reclaim reason states plainly that the TTL is NOT what fired, so a later
reader cannot mistake this for ordinary staleness arriving early.
64 changes: 64 additions & 0 deletions docs/lanes/oy4-dead-holder-reclaim/evidence/failbefore_probe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""FAIL-BEFORE / PASS-AFTER probe for model_performance-oy4.

Reproduces the measured h6v shape with the SAME API on both trees, so the
before/after difference is behaviour, not a missing function signature:
a hold whose holder process is genuinely dead, last renewed 400s ago,
against the documented 900s CUSTODY_TTL_SECONDS.
"""

from __future__ import annotations

import json
import os
import socket
import subprocess
import sys
import time

import pytest

from amplifier_work_tracker import adapter as A
from amplifier_work_tracker import custody as C
from amplifier_work_tracker import supervisor as SV

pytestmark = pytest.mark.integration
SILENCE = 400


def _dead_pid() -> int:
p = subprocess.Popen([sys.executable, "-c", "pass"]) # noqa: S603
p.wait()
return p.pid


def test_probe(workspace, project_factory):
name, bd = project_factory("failbefore")
item_id = bd.create("dead holder probe", priority=1)
bd.claim_item(item_id, actor="dead-agent")
pid = _dead_pid()
bd.take_custody(item_id, holder="dead-agent", pid=pid, host=socket.gethostname())
rec = dict(bd.get(item_id).meta[C.CUSTODY_KEY])
rec["last_seen"] = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(time.time() - SILENCE))
bd._run(["update", item_id, "--metadata", json.dumps({C.CUSTODY_KEY: rec})], actor="dead-agent")

out = []
out.append(f"holder pid {pid} running? {os.path.exists(f'/proc/{pid}')}")
out.append(f"custody last_seen {SILENCE}s ago; CUSTODY_TTL_SECONDS={C.CUSTODY_TTL_SECONDS}")
s = A.project_summary(workspace, name)
out.append(
f"work_stats view: held={s.held} held_stale={s.held_stale} "
f"held_stale_oldest_age_seconds={s.held_stale_oldest_age_seconds}"
)
r = SV.reap_project(bd) # default TTL, exactly what the service runs
out.append(
f"reap_project(default ttl): reclaimed_count={r['reclaimed_count']} "
f"reasons={[x['reason'] for x in r['reclaimed']]}"
)
after = bd.get(item_id)
out.append(f"after sweep: status={after.status} holder={after.holder!r}")
try:
bd.claim_item(item_id, actor="successor-agent")
out.append("successor work_claim: SUCCESS")
except Exception as e:
out.append(f"successor work_claim: REFUSED -- {str(e)[:150]}")
print("\n".join(" " + line for line in out))
Loading
Loading