Review round 2 (planner-reviewer-codex-01, NOT PASS at d09c162):
- R1 remote.mirror_job: admission under the registry lock. The same
dispatch_token is idempotent (a finished attempt never runs again), a
lower attempt or the same attempt under another token is refused before
any write, and only a strictly newer attempt replaces the record
(register_job(supersede_older_attempt=True); events.log kept, seqs stay
monotonic).
- R2 remote.handle_dispatch: the --exec command runs in its own session;
at timeout_sec, or when the runner is interrupted (SIGTERM now unwinds
it), the whole process group is terminated (TERM, then KILL) before the
error is published.
- R3 terminal barrier: a terminal publish raises an attempt-scoped barrier
under the lock that allocates seqs, stops the heartbeat timer, gives it
MAM_HEARTBEAT_DRAIN_S (default 1 s) to exit and then kills it, and only
then takes its own seq. Beats are admitted under that lock
(next_seq(admit=...)); a beat still on the wire is cancelled with the
timer rather than holding the terminal back; and a late non-terminal
event never publishes a running state over a terminal record.
- R4 job_subscriber: a retained state renews the idle clock only when its
seq is newer than everything already accepted; re-delivered or older
snapshots still recover a terminal status.
- tests: five regressions, all failing on 77f2829; T1-14's wall clock
15 -> 30 s against public-broker latency.
- docs: MESSAGING 2.5 (barrier, state renewal, detection vs final close),
heartbeat docstring, loop SKILL.md (remote rows need the Python runner),
VERSIONS v4.5.0 items 1/4/5 and the test count (25).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
In the v4.5.0 review round grok read the brief and published `started`
70 s after the injection; with T = 60 the wrapper had already closed the
job as agent_blocked and refused its later PASS. T bounds delivery, not
work, so a generous default costs nothing when the agent answers fast.
Code, docs and the CLI snapshot updated together.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Both tests ran against the public broker with second-scale budgets: the
error publish after the close can take 10-20 s, and a worker's `started`
must land before a 12 s wall clock. Assert the close time from the record's
updated_at and give the loop scenario a 30 s wall clock.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Review round 1 (claude NOT PASS) corrections:
- B2: the timer now watches a process - `--watch-pid`, or by default the
nearest agent-CLI ancestor of the publishing command - and stops within
one period once it is gone, so a crashed agent stops heartbeating and is
closed after K x N; a remote runner watches itself. Documented as process
liveness (an alive-but-stuck agent keeps heartbeating).
- R1: `_wait_started` accepts an events.log `started` only when it carries
the current dispatch token (loop jobs share one events.log).
- R2: the remote runner ignores a re-sent dispatch of an attempt that
already reached its terminal.
- R3: registry.md EOF blank line (git diff --check clean).
- R4: the ack error states the effective budget when the wall clock cut it.
- P3: stop_timer verifies the pidfile pid is this job's timer before
signalling; a retained state message keeps its own seq watermark so it
can never make the monotonic guard drop its own event.
- MULTI_AGENT_RULES.md timeout section updated for the contract.
- tests: agent-process death stops the timer, recycled-pid safety,
attempt-scoped started, re-sent dispatch; acceptance 3 now kills the
agent process instead of the timer. Help captures refreshed
(--watch-pid). The wrapper can be sourced with
MAM_DELEGATE_JOB_NO_MAIN=1 for helper-level tests.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
T1-14 and T1-20 model an agent that goes `working` and completes late. Under
the liveness contract an agent that never publishes `started` is a delivery
failure, so the watcher threads now acknowledge (non-blocking: the record
lands in events.log before the wire publish) and the late completion is
honoured inside the K x N grace as before. help-job_subscriber.txt records
the intended --ack-timeout option.
Full suite at 66f23c7: 888 passed, 3 failed (these), 0 leaked daemons.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Contract (no new event types): heartbeat every N s (automatic timer),
unresponsive after K x N s without an event, `started` within T s of the
injection, retained per-job state, and a remote transport.
- registry: heartbeat_sec / heartbeat_misses / ack_timeout_sec / transport
fields; idle_timeout_sec derives K x N unless explicit (defaults 30/4/60
keep the historical 120 s idle window).
- heartbeat.py + publish_event --heartbeat N: detached fixed-cadence timer
publishing signed `progress` heartbeats (threaded wire publish, local
record first); stopped by the terminal event via pidfile, by re-dispatch
and by lifetime. Terminal events refuse --heartbeat.
- publish_event: retained `<prefix>/state` (status, last_event, seq, HMAC)
sent on the same connection after every event (--no-state to skip);
publisher.publish_raw takes extra messages.
- job_subscriber: subscribes the state topic, recovers terminal status from
retained state, resets idle on any accepted message, records seq_gap and
reconnected in the audit log, idle budget before the first event is
T + K x N (--ack-timeout), --remote mirrors started/terminal events into
the registry through the transition boundary.
- lib.sh send_keys_safe: an accepted-but-unconfirmed submission is confirmed
by agent_status working or the job's own `started` (_sks_job_started);
pane change and busy token are no longer delivery signals.
- wrapper: --heartbeat/--heartbeat-misses/--ack-timeout (MAM_* env),
injected start command arms the timer, brief states the contract,
_wait_started (registry / events.log / subscriber log, bounded by T and
the wall clock) replaces the 10 s inject grace, R1 grace uses only the
on-disk event age (herdr/pane logged once as a diagnostic),
remote:<name> sessions skip preflight/idle/pane and dispatch over the
broker (remote_dispatch).
- remote.py (+ scripts shim): signed retained dispatch (MAM_DISPATCH_SECRET)
and a runner that mirrors the job into its own registry with the
orchestrator's identity, publishes started --heartbeat, runs --exec and
publishes the terminal event.
- loop runner: passes MAM_HEARTBEAT_SEC / MAM_HEARTBEAT_MISSES /
MAM_ACK_TIMEOUT_S through; YAML rows with transport: remote delegate as
remote:<name>.
- docs: MESSAGING.md 2.5 contract + remote transport, delegate-job SKILL.md,
registry.md, loop SKILL.md.
- tests: tests/test_issue18_liveness_contract.py (16); issue 14 pane-signal
tests flipped to started-based; issue 6 working-agent tests acknowledge
with started; CLI snapshot/help captures refreshed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Review round 3 (reviewer-grok-01, NOT PASS N1; non-blocking B1):
- reconcile.sh (N1): the python subscriber installs its SIGTERM/SIGINT
handlers before connect/loop_start. A signal during the broker handshake,
the CONNACK wait, or right after the paho thread logs "subscribed to" now
takes the clean path (the signal line, loop_stop/disconnect, rc 0) instead
of Python's default action; the CONNACK wait ends on it, and a signalled
subscriber never falls back to polling (rc 3).
- reconcile.sh (B1): `_sub_stop` reads `$!` with nounset off. On bash 3.2
`${!:-}` and `${!+x}` are both unbound before the first `&`, so the trap
died with rc 1 instead of 143.
- tests: SIGTERM during a CONNACK wait against a silent local listener (no
broker needed), and the verbatim `_sub_stop` run without and with a
child; both fail on 8f9745b.
- docs: monitor SKILL.md and VERSIONS.md state the handler order; test
count 16.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Review round 2 (planner-reviewer-codex-01, NOT PASS P2):
- conftest `_is_subscriber_daemon_argv`: `ps -o args=` joins argv unquoted,
so the script path is taken from the shell word up to the first
`/reconcile.sh --subscribe` instead of as one whitespace token. A sandbox
or --basetemp path with a space no longer hides its daemon from the
fixture reaper and the session leak guard. A candidate with whitespace
counts only when it names an existing file, so a bash wrapper under the
root whose later argv carries the words is still excluded.
- tests: a whitespace-path selector regression and an owned live-process
cleanup check (both fail on c096cb4); the R-2 test also pins the bash
wrapper exclusion.
- docs: the polling fallback's signal handling is stated precisely in
monitor SKILL.md and VERSIONS.md (a running --once pass finishes first,
the interval's sleep child ends on its own); create SKILL.md says an
already-running daemon's PID is still reported under the opt-out;
VERSIONS selector wording and test count (14).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Review round 1 (claude PASS, agy PASS) recommendations:
- reconcile.sh: `_sub_stop` falls back to `$!` for the window before
`_sub_pid=$!` (R-1); lock identity compared as (st_dev, st_ino).
- lib.sh legacy start_watchdog prints nothing when opted out and no daemon
runs, byte-identical to the python body.
- conftest/test: `_is_subscriber_daemon_argv` selects only
`bash|sh <root>/.../reconcile.sh --subscribe`, never a brief-carrying
wrapper that quotes the words or a prefix-colliding sibling root (R-2);
one new unit test for the selector.
- monitor SKILL.md / VERSIONS.md: the polling fallback checks the lock
file's presence only and does not exclude a second daemon; measurement
wording uses the argv-shape criterion instead of raw pgrep.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
The reaper executes in fixture teardown, before monkeypatch restores a
test's patches; G5 forbids subprocess.run/Popen for the test's duration and
subprocess.run resolves Popen through the module at call time, so the
teardown tripped the spy. Bind the Popen class at import instead.
Full suite at 38d8ccb: 870 passed, 1 error (this teardown), 0 leaked daemons.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
create_session.sh detaches `reconcile.sh --subscribe --idle-timeout 0` per
workspace; test sandboxes never reaped it and the daemon neither noticed its
workspace vanish nor honoured SIGTERM (bash deferred the signal behind the
foreground python child). 202 orphans were found holding public-broker
wildcard subscriptions for deleted pytest tmpdirs.
- daemon.py / lib.sh: MAM_NO_SUBSCRIBER_DAEMON=1 (literal) skips the spawn but
still reports a running daemon; cli prints a stderr note, stdout stays empty.
- reconcile.sh: python subscriber runs as a waited background child with a
TERM/INT trap (exit 143); the MQTT loop exits when the workspace dir is gone
or .mam/monitor.lock is no longer the held inode; SIGTERM/SIGINT handled via
the stop event so loop_stop/disconnect run; the polling fallback checks the
same and sleeps in a waited child.
- conftest: sandbox default MAM_NO_SUBSCRIBER_DAEMON=1, per-sandbox reaper
finalizer, session-scope leak guard over the pytest basetemp.
- tests: 11 lifecycle tests (tests/test_issue19_subscriber_daemon_lifecycle.py);
phase-1 watchdog/daemon characterization tests opt back in.
- docs: monitor SKILL.md lifecycle contract + pitfalls, create SKILL.md note.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
The test blocked on each publish_event heartbeat, and against the public
broker a publish (with retries) takes 2-6 s, so consecutive events.log
records landed further apart than the 6 s idle window and the wrapper
closed the job before the next heartbeat - a broker-latency flake that
failed the full suite once and passed on re-run.
Heartbeats are now fired without waiting for the wire publish (the disk
record is written before the broker is touched, ebe58f6) and keep coming
for 20 s to outlast a slow connect; the wall clock is widened to match.
3/3 green in a row afterwards.
Refs #14
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Round-1 review of 4176d75 (planner-reviewer-claude-01, non-blocking):
- update.sh: merge install_state by exact key match (awk, no regex
interpretation of the key), skip lines without '=', and terminate a fresh
install_state that lacks a final newline before appending
- tests (#7): an agent-less pane left behind in a later same-cwd workspace
is reused instead of splitting or creating; MAM_WS_LABEL renames only the
workspace that was chosen
Refs #8#7
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Review of 4176d75 (planner-reviewer-claude-01, N1): _cmd_scrub_secrets chose
records by the presence of broker.password, not by its value. The old
to_registry_block wrote "password": null even for an unauthenticated broker,
so on this repo's registry (901 of 903 records carry the null key, none a
real password) --dry-run reported "would scrub 901 record(s)" and a real run
would rewrite every one of them while holding the registry lock.
- registry.py: select on broker.get("password"); an empty key is no secret,
and _atomic_write_record drops it on the record's next write anyway
- tests: the migration test gains a legacy "password": null record that must
be neither listed nor rewritten (fails on 4176d75 with "would scrub 2")
- VERSIONS.md v4.4.4 item 2 and registry.md now describe the stored-value
rule
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Issue #12 - MQTT_PASSWORD was copied into every job record, which any
sub-agent can read. BrokerConfig.to_registry_block no longer persists it;
broker_config_from_job layers the password back from the environment
(.mam.env) at connect time, so publish_event still connects from the record.
_atomic_write_record scrubs broker.password from any record it writes, and
`registry.py scrub-secrets [--dry-run]` migrates an existing registry.
auth_token/dispatch_token stay: the agent's own publish_event needs them.
Issue #8 - update.sh restored the pre-update asset_hashes.txt and version.txt
over the installer's fresh ones, so version.txt named the previous commit and
remove.sh backed up every pristine updated file as "locally modified". The
success path now keeps the installer's stamps and only merges install_state
keys the re-install did not write (gitignore_created); the failure path
still restores everything verbatim.
Issue #7 - the shim's new-session took the first same-cwd pane, always found
the full first workspace, judged it overflow and opened another workspace per
agent (4 + 1 + 1). It now walks every same-cwd workspace in pane order and
takes the first with an idle pane or room in its 2xK grid; a workspace is
created only when all are full (4 + 2).
Tests: test_issue7_workspace_packing.py (3; the mock herdr gains an opt-in
per-workspace 2x2 geometry), test_issue8_update_state.py (1, real
install+update cycle), test_issue12_registry_secrets.py (4); registry CLI
snapshot and help capture refreshed for the intentional scrub-secrets
subcommand; registry.md documents the broker block.
Refs #12#8#7
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Round-3 review of f5a6575 (planner-reviewer-codex-01, R1/R2).
R1 - the adapter's in-use check handed only the conversation DB to lsof, so a
holder that keeps the presence lock (or a sidecar) open while naming the uuid
on no command line and in no session variable was invisible, and the held
lock was unlinked from under it. AgyAgentAdapter.conversation_in_use now
passes every path of the conversation (DB, -wal/-shm/-journal, brain,
annotation, presence lock).
R2 - both resolver stages required the DB, so once an interrupted or pre-#10
purge had removed it, the WAL/annotation/lock left behind were "no artifact"
(exit 5) forever. BaseAgentAdapter.has_artifacts (default: a uuid-bearing
artifact path that exists) replaces the resolver's path fallback, and agy's
override accepts any of its pieces.
Tests: a flock holder that names the uuid nowhere is refused and nothing is
removed; orphaned leftovers resolve to agy and are purged. VERSIONS.md
v4.4.3 items 1-2 and orc-onboard SKILL.md updated.
Refs #10#13
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Round-2 review of 7610258 (planner-reviewer-claude-01, reviewer-grok-01,
N2): the purge tests used two fixed UUIDs, while conversation_in_use scans
every process on the host. A review job's delegation wrapper carries the
whole brief - including this file's diff and so both constants - on its
command line, so five tests read "in use" for as long as that wrapper
lived. Reproduced here: this job's own wrapper named both constants and
the same five tests failed.
- tests: `conv` / `other` fixtures hand every test a fresh uuid4 and
assert that no live process names it yet; the helpers already took the
id as an argument. With a bystander process naming the old constants,
the file passes 21/21 and the brief's required set 248/248
- orc-onboard SKILL.md: any live process whose command line contains the
UUID blocks the purge, not only a resumed session (review R-2), and how
to find it
No product code changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Review of 0282312 (planner-reviewer-claude-01, N1): conversation_in_use
excluded the purge's whole ancestor lineage, and that lineage includes the
agent CLI that launched the skill. An orchestrator purging its own UUID
from its own session was therefore never refused - the only process naming
the UUID was excluded, and claude keeps no transcript open for lsof - so
its live conversation was deleted, contrary to what orc-onboard SKILL.md
promises.
- base.conversation_in_use: in use when one of the caller's session-id
variables (CLAUDE_CODE_SESSION_ID, ANTIGRAVITY_CONVERSATION_ID,
HERMES_SESSION_ID, GROK_SESSION_ID, OPENCODE_SESSION_ID) equals the uuid;
the command-line check now excludes only the purge's own helper chain and
stops at the nearest agent CLI ancestor (basename claude/agy/hermes/grok/
opencode/codex, read like detect_nearest_agent), which counts as a holder
when it names the uuid; the lsof check is unchanged
- tests: the non-agent helper chain is excluded but an agent ancestor is
counted; the purge CLI is refused under an agent CLI naming the uuid and
with each of the five session variables set to it; orc_onboard --remove
--purge-conversation --yes is refused from the orchestrator's own session
(exit 5, registration kept). The eight new cases fail on 0282312
- job_subscriber --registry-dir now defaults to default_registry_dir() like
the other entry points (review R-d)
- orc-onboard SKILL.md spells out how the calling session is recognised;
VERSIONS.md v4.4.3 item 2 and the new-test count (24)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Issue #10 - agy purge_artifacts left resumable pieces behind: it now also
removes the SQLite sidecars (-wal/-shm/-journal), the title annotation and
the presence lock (the lock only once no live process references the
conversation), and residual_artifacts() reports anything under
~/.gemini/antigravity-cli still carrying the uuid.
Issue #13 - the orchestrator has no herdr_sessions row, so its conversation
could not be purged by any skill. `lib_py.agents purge <agent|auto> <uuid>`
resolves the adapter from what is on disk (orchestrator_uuids stores bare
uuids), refuses while a live process names the uuid (conversation_in_use:
process list minus the caller's own lineage, plus lsof), and prints what it
purged. `orc_onboard.sh --remove <uuid> --purge-conversation --yes [--agent]`
runs it before the registry update so a refusal changes nothing (exit 4
without --yes, 5 while in use).
Issue #11 - "Successfully removeed": conjugate add/remove explicitly.
Issue #15 - the job registry and audit log followed the caller's cwd, so the
pytest suite (create_session.sh with its default onboarding job, run from the
repo root) had left 593 test records in the real .mam/jobs. Defaults now
resolve MAM_JOBS_DIR / DELEGATE_JOB_LOGS_DIR, then $WORKSPACE_ROOT/.mam/...,
then the cwd (mqtt_common.default_registry_dir / get_logs_dir; registry.py,
publish_event.py, service.py, create_session.sh, stop_session.sh); the
sandbox fixture pins both. The tier suites that used to leak now leave the
real registry untouched (881 records before and after).
Tests: test_issue10_13_purge.py (13), test_issue15_registry_isolation.py (7);
T-O13 pins the corrected message; the R1 helper test and B-9 logs-dir test
now assert the workspace-anchored contract; publish_event snapshot updated.
Refs #10#11#13#15
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Round-2 review of 952657e (reviewer-grok-01): R1's disk-heartbeat
evidence comes from events.log, but publish_event appended to it only
after with_retry(_publish_once) returned - after the broker ACK or all of
its retries. With the public broker at ~5 s per publish, an agent's
heartbeat reached the disk after the grace window and the live job was
closed as deadline_timeout (test_wrapper_idle_timeout_grace_accepts_disk_heartbeat
failed 2/2 on 952657e here, as it did for the reviewer).
- publish_event: new record_local_event appends the prepared payload to
events.log right after prepare_event (validation and seq allocation
first) and before the wire publish; record_outcome takes
local_event_recorded=True so the line is not written twice, while the
audit "published" entry and the status sync stay after the publish
- DelegationService.publish_event: the same local-first order
- wrapper comment and VERSIONS.md v4.4.2 item 6 now say the heartbeat is
on disk before the publish; new-test count 21
- tests: the heartbeat is already in events.log when the publish starts
(and is recorded once), and a publish failing on every retry leaves one
local record with published=False in the audit; both fail on 952657e.
The wrapper disk-heartbeat test keeps the real publish_event path and
now passes 3/3 at the same broker latency
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Round-2 review of d5a191c (planner-reviewer-claude-01, planner-reviewer-codex-01).
- delegate wrapper: the R1 idle grace and the broker_unavailable re-check
counted only their sleeps against TIMEOUT, so python/herdr probe time plus
fresh disk heartbeats let a job run far past --timeout (55 s for a 20 s
budget in the new test). Both loops now end at START_TIME + TIMEOUT read
off the clock on every pass; heartbeats extend only the idle window, and
the herdr probe is cut off at the deadline (_within_deadline)
- document the window policy d5a191c actually implements: an agent with no
evidence is closed ~2 x IDLE_TIMEOUT after its last wire event (Issue #6
promised IDLE_TIMEOUT + 5 s) and heartbeats may be ~2 x IDLE_TIMEOUT apart;
correct the claim that publish_event records a heartbeat on disk before
the wire publish - it records right after the publish attempt
- shim ls: every failed `agent list` other than a structured
server_not_running now exits nonzero with a "state unknown" diagnostic
instead of reaching the list parser, where an error document on stdout
read as an empty list and let drift A terminate live rows
- tests: stdout variants for down and unrelated errors, reconcile keeping
herdr_confirmed False on an unrelated failure, and a wall-clock upper bound
under continuous heartbeats and slow probes; the three new checks fail on
d5a191c
- VERSIONS.md v4.4.2: items 3 and 6 and the test count (19)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
The R1 grace after a subscriber idle timeout only continued while herdr
reported `working` or the pane showed a busy token, and gave up the moment
that evidence vanished. Two live jobs in the v4.4.2 review loop were closed
as deadline_timeout that way: a grok review whose heartbeat gap was 147 s,
and the creator running the full suite in a background shell (idle to herdr,
no busy token) while publishing progress every 45 s. Both later published
`completed`, which the terminal record refused.
- `_recent_disk_heartbeat`: publish_event appends every event to the job's
events.log before the wire publish, so a heartbeat the subscriber never saw
is still on disk; treat one at most IDLE_TIMEOUT old as liveness
- R1 loop: any evidence (herdr working, pane busy token, disk heartbeat)
opens a new IDLE_TIMEOUT window; a job with no evidence for a whole window
is closed as before, still inside the wall clock
- test: a human-agent job whose subscriber idles out stays open across five
disk heartbeats and picks up the out-of-band completion (rc 0)
- VERSIONS.md v4.4.2 item 6 and test count updated
Refs #14
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
The ls translation searched stderr and the whole stdout for the substring
server_not_running, so a successful `agent list` that merely carried the text
(an agent named server_not_running-codex, or metadata) made the shim report
"no server running". reconcile.sh reads that as a confirmed-empty server, and
drift A would then mark every live row on it terminated (codex review of
0918917).
- keep the RPC exit code and translate only a failed call whose error
envelope (the whole stream or any line of it) has
error.code == "server_not_running"; every other failure keeps the unknown
path (nonzero exit, herdr_confirmed False) as before
- tests: the real herdr 0.9 envelope, a successful list carrying the
substring in a name and in metadata, other failures staying unknown, and
reconcile keeping both live rows with no drift A; the new cases fail on
0918917
- delegate wrapper: log a non-pending status seen during the inject ack
grace as "registry advanced" instead of attributing it to the agent
- VERSIONS.md v4.4.2: describe the structured check and count 16 new tests
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
The R1 grace after a subscriber idle timeout only asked herdr whether the
agent was still `working`. herdr never reports that for codex, so a codex
review longer than the idle timeout was closed as deadline_timeout mid-turn
and its eventual `completed` refused - the same status blindness behind the
agent_blocked misjudgement. `_agent_still_working` now also accepts the pane's
own busy token, the evidence send_keys_safe already uses to confirm delivery.
Adds a wrapper test where a codex-like agent (idle per herdr, busy per pane)
idles out the subscriber and completes out of band afterwards.
Refs #14
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
4d444ce changed the agy adapter's exit_key to /exit for a clean graceful
shutdown but left test_cli_bridge_subcommands_and_quote_safety expecting the
old 'Exit', so the full suite has failed on that one case since.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
- Drop per-session agent runtime markers (CLAUDECODE, CLAUDE_CODE_CHILD_SESSION,
CLAUDE_CODE_SESSION_ID, Claude Code messaging/bridge/pid markers, and the
agy/hermes/grok/opencode session ids) from the environment lib.sh's shim hands
to a bootstrapped `herdr --session <name> server`; user configuration such as
CLAUDE_PROJECT_DIR still inherits
- Add MAM_NO_AUTOSTART=1 so query-only callers never bootstrap a server as a side
effect; status.sh and reconcile.sh export it, and the shim's `ls` reports a down
server as "no server running" so reconcile keeps herdr_confirmed instead of
freezing drift detection for every server
- send_keys_safe: treat `timeout` / `agent_prompt_stalled` from `agent prompt
--wait` as an accepted submission and confirm delivery for SKS_DELIVERY_GRACE_S
(default 15 s) via agent_status working, a busy token the pane did not show
before, or the pane moving on from its pre-submit snapshot; no signal at all
still fails closed (rc 4). Codex under herdr 0.9.0 never reports `working`, so
every delivered prompt was closed as agent_blocked and its later
started/completed events refused
- delegate wrapper: before closing an unconfirmed inject as agent_blocked, honour
the registry's own acknowledgement for MAM_INJECT_ACK_GRACE_S (default 10 s)
- Switch T1-7's failure trigger to transport_error (timeout now means accepted);
add test_issue9_server_env.py (5 cases) and test_issue14_delivery_ack.py (7)
- Verified live: a freshly bootstrapped multi-agent-mux server carries no markers
and the claude panes save transcripts again; codex job d04ebcb5 had its
started/completed accepted with no agent_blocked
Closes#9Closes#14
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Issue #5 (Python core migration, CAS state transitions, shadow watcher,
auto-completion gate, eager-import immutable runtime, B-13 freeze removal)
is complete and merged to main. This stamps the release.
SemVer: MINOR, not MAJOR. The change is large but breaks nothing. Across the
whole Issue #5 range there are zero file deletions and zero renames, the event
wire schema stays at SCHEMA_VERSION 1, all four literal script paths survive as
compatibility shims, the 968-line legacy shell loop is still in run_loop.sh,
and every default change reverts through a single environment variable
(MAM_DELEGATION_PY / MAM_ENGINE_CAS / MAM_LOOP_PY / MAM_LOOP_FREEZE). The
riskiest new feature, auto-completion, ships default-off. No operator has
anything to migrate, which is the decisive reason not to call it 5.0.0. The
reasoning is recorded in the VERSIONS.md release notes, not just here.
Close a gap in the lockstep guard. lib.sh exports MAM_VERSION and eager_load()
refuses to start the loop when it disagrees with FRAMEWORK_VERSION, so eager.py
is a load-bearing fourth leg — but test_version_consistency.py only checked
three, and the sole check on eager.py was a hardcoded literal in the phase 4
test that a release bump has to edit anyway. The lockstep test now covers
eager.py, a new test pins the fail-closed gate itself, and the phase 4 test no
longer carries a version literal.
Fix a latent test-isolation defect the bump exposed. The first full run failed
four tests with "MAM_VERSION mismatch: expected 4.4.0, got 4.3.0 in
environment" — 4.3.0 was not in the tree, it was inherited from the shell that
launched pytest and had sourced the pre-bump lib.sh. Those tests were measuring
the runner's shell rather than the tree under test, and only passed until now
because the two values always happened to agree. A session-scoped fixture drops
the ambient value. Production is unaffected: run_loop.sh:17 sources lib.sh
before anything else and lib.sh re-exports MAM_VERSION unconditionally, so a
stale inherited value is always overwritten before Python starts.
Full suite: 750 passed in 912.50s, 0 failed, 0 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
The subscriber marked a notification intent as seen before the monotonic
sequence guard ran. A late packet was then dropped by that guard with its
intent already burned, so the genuine retry of a notification this
subscriber had never delivered was mistaken for a replay and dropped too.
Identity comparison stays ahead of the sequence guard -- a duplicate must
not consume a sequence number -- but the bookkeeping write moves behind
it, so "seen" means "accepted" rather than "arrived".
An intent that cannot yield a stable identity (not an object, or holding
an unhashable value) is now refused instead of raising out of the
callback: a paho callback that raises takes the network loop down for
every job that subscriber is watching.
Six regressions; three of them fail on the unfixed tree, and one drives
a real notifier retry through the real consumer with no hand-built
payload. Full suite: 732 passed, no skips.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Two findings from round 2, both reproduced before being accepted.
R2 -- attribution was compared before the terminal write and nothing tied the
two together. A job could complete, be re-dispatched and claimed again in that
window, and evidence from the first attempt then satisfied a status check and
finished the second one. Reproduced with real transitions: committed, with the
registry already on iteration 2.
This corrects my own earlier reasoning, and I want to be plain about it. Last
round I argued that the attempt guard could live entirely in the evidence layer
because that is where attempt identity lives. That holds for comparing evidence
to a dispatch, and it does not hold for atomicity: a precondition can only be
atomic with the commit if the transaction can read it under its own lock. So the
identity now lives in the record.
It is an opaque dispatch token, not the iteration counter. That distinction is
the part of the earlier rebuttal that still stands: iteration already means the
loop round, and comparing a dispatch attempt against it produces a guard that is
vacuous when they agree and wrong when they diverge. A token has no competing
meaning. It is minted when an attempt begins -- claim and redispatch -- and the
gate passes the token it decided against, so any intervening re-dispatch refuses
the commit.
R4 -- the intent identity was being emitted but nothing consumed it. A replayed
notification is re-published with a fresh wire sequence, so the monotonic guard
cannot recognise it; to that guard a retry looks new. The subscriber now
deduplicates on the stable intent identity, leaving the sequence guard as the
rule for ordinary events. Across a restart the disk fallback resolves terminal
status and a duplicate terminal is refused as an idempotent ack, so a replay
after restart cannot double-apply either.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Seven blocking items across three reviews, every one reproduced here before
being accepted. Two themes run through them: a gate whose strictness depended on
what the caller remembered to pass, and a boundary that trusted its only current
caller instead of enforcing its own rule.
Independent verification is no longer optional. Omitting the report directory
skipped the disk check entirely and handed the decision back to the collector's
own flags -- the self-certification this design exists to prevent. It is now a
refusal, as is an empty directory or a job with no recorded dispatch.
Evidence is now bound to its target. The policy compares a scenario against the
dispatch context carried inside that same scenario, which is self-consistent by
construction, so one job's evidence could complete another. The gate now checks
the job, attempt, session and pane generation against the recorded dispatch.
A reviewer with no verdict token is no longer completed. Phase 0 made this a
safety contract and run_loop reads a missing token as NOT PASS, so inferring
completion first would start a correction round against a review that was never
finished. The requirement applies to reviewers only.
Bookkeeping may write one field. A status-free request was an unrestricted merge
on a terminal record: it could rewrite the prompt and the session. Settling is
also conditional now on the exact intent that was sent, because publishing
happens outside the lock and a newer intent can legitimately land mid-flight;
clearing unconditionally dropped a notification nobody had sent. The intent's
own identity travels in the payload, since the transport sequence cannot
identify a retry of the same intent.
Side-effect evidence must be evidence. A kind with no reference names something
without evidencing it, and the CLI parser could not be the only check because
the API and hand-written manifests bypass it.
Also honours MAM_ENGINE_CAS=0, which the operations note already promised
disables auto-completion, matches backend keys exactly rather than by prefix so
a future name cannot inherit evidence never collected for it, records an
absolute report path, and imports the notifier at module scope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Two failures the full suite caught, both mine.
finalize.py started in lib_py/loop, which made publish_event import a sibling
package. That is the rule phase 1 wrote down and phase 2a already made me move
the engine for: under a frozen loop, delegation is bound to one tree while the
parent may be another, so reaching sideways mixes them. The manifest is written
into the job's own report directory and the CLI that declares it is a delegation
entry point, so finalize belongs there. Second time on the same rule, and the
guard caught it both times.
The publish_event --help golden no longer matched, because phase 2b
intentionally added the finalization options. The diff is exactly those three
options and nothing else, so the capture is refreshed rather than the assertion
weakened, and the test now states when a refresh is legitimate: an intended
extension, never drift nobody meant.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Three scenarios join the contract set, so the fifteen vectors now cover an
unproven side effect, an undecidable attribution, and an agent whose state goes
unreadable in the window between finalization and notification. All fifteen
match their expected decision.
The side-effect case needed a decision about layering. Authoritative enforcement
belongs in manifest verification, which has the job record and therefore knows
what was declared; the policy honours an explicit negative from the collector so
the case is expressible as a vector. Absence is not read as proof there, because
the manifest check is the layer that fails closed on missing evidence.
The operations note records why the agent declares and the reader re-derives,
the three gates and why the backend allowlist can only be narrowed, the
notification transaction, and the rollback.
It also records a deviation from my own plan. §D2 asked for reviewer sign-off
before promoting an attempt id into the record, since that changes a contract
phase 1 froze. Measurement during implementation showed the change was not
needed: the attempt guard already lives in the evidence layer, where attempt
identity actually is, so auto-completion passes the engine a status precondition
instead. The schema is untouched, the frozen contract holds, and a
characterization test fails the moment a record starts carrying an attempt id,
which is when to revisit it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
The gate does not re-decide anything. Phase 2a already decides, fail-closed and
against contract-pinned vectors; this supplies real evidence to that decision and
commits only if three independent gates agree: the global switch is on, the
backend has observed evidence that it reaches a quiescent state, and the job
itself opted in. Default is off.
The backend allowlist is evidence-based rather than optimistic. Inferring
"finished" from stillness requires stillness to have been observed, and only
three of six backends have that. An operator may narrow the list; a name that
was never observed is dropped rather than trusted, so it cannot be widened by
configuration.
Everything else holds. An unreadable manifest, a report rewritten after
finalization, missing evidence for a declared side effect, a policy that raises:
all refuse. "Could not decide" is never "done". A late explicit terminal wins,
because the boundary refuses the inferred one -- that is the boundary working,
not an error in the gate.
Notification intent is committed in the same atomic write as the state, so a
crash between committing and publishing leaves a record that still says the
world has not been told. Recovery re-sends; duplicates are already dropped by
sequence. A failed send deliberately leaves the intent pending, because an
unsent notification that looks sent is worse than one that is retried.
Two fixes this surfaced. Settling used a request that terminal records refuse,
which made an owed notification impossible to ever settle; settling is now a
status-free bookkeeping request, valid in any state. And the phase 2a guard
against shell references was matching hashlib.sha256 as a substring, so it now
requires a word boundary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
The agent declares that its work is finished; the reader re-derives the hash
from the file rather than trusting the number in the declaration. A manifest the
watcher wrote itself would attest only that the watcher saw a stable file, which
is the self-certifying evidence shape rejected earlier in this issue.
Finalizing promotes a staged report by atomic rename before writing the
manifest, so a reader cannot observe a half-written report as final. Rewriting
the report afterwards invalidates the declaration immediately, because the
recomputed hash no longer matches -- a new body needs a new finalization.
A job that declared side effects must show evidence for every one of them.
Partial evidence is treated as none: the work is not known to be complete.
publish_event gains --finalize-report, --side-effect and --attempt-id, and does
the finalization before publishing. A `completed` event whose report is still
being written would invite exactly the partial adoption the contract forbids, so
a failure there fails the publish.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Pins what the system refuses to do today, so nothing added later can weaken it
quietly. X-01 records the property the whole phase leans on: a job with no
completion manifest is already held, which is why an agent that declares nothing
stays on the explicit path without a single new branch.
X-03 and X-04 record the layering decision and its evidence. The engine compares
an expected attempt against a record field that does not exist, falling back to
the loop-iteration counter, which means something else. So auto-completion does
not use that guard; the attempt check stays where attempt identity actually
lives, comparing observed evidence against the dispatch record. X-03 will fail
if a record ever starts carrying an attempt id, which is the moment to revisit
the decision.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Both reviewers sustained the round 1 fixes and found four more. Each was
reproduced here before being accepted; none were rebutted.
F-C (claude, blocking) -- my own round 1 fix caused this. Making refused writes
exit non-zero also classified an idempotent ACK as a refusal, and the wrapper
finalises an approved job with `status --set completed` on a record that is
already completed. It runs under `set -e`, so the *success* path aborted with
exit 4 before its validation hook and its final stdout line. Reproduced at the
shell. An idempotent result is now distinguishable from a refusal and exits 0; a
genuine conflict still exits 4.
F-D (claude) -- routing the operator write around update_status dropped the
status vocabulary check. Under the rollback flag an invalid status was written
to the record with exit 0, where before it was refused with exit 1 and the record
left alone. The check now runs first, in both modes.
R1 (codex, blocking) -- the previous fix compared identifiers only when they
happened to be present, so evidence carrying none at all was still approved on
the dispatch record's own flags. That is the self-assertion the check exists to
replace. Every identifier must now be supplied and agree, and a report manifest
must associate with the attempt being judged. Identifiers are read from a
collector envelope beside the payload rather than from inside it: a native Herdr
payload may not carry MAM fields, so one appearing there is either fabricated or
a contract violation, and either way it is not evidence.
R4 (codex) -- a sequence number orders nothing unless the compare and the write
are atomic. Two flushes could both read the old value, the newer land first, and
the slower overwrite it. Reproduced deterministically. The mirror now serialises
read-compare-write under a per-job audit lock, taken outside the registry
transaction so a slow mirror still cannot lengthen it, and each write uses its
own temp file instead of a shared name.
All eight new regressions fail against the pre-fix code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Two reviewers raised five defects. Every one was reproduced here before being
accepted; none were rebutted.
F-A (claude, blocking) -- the CAS table had no row for two transitions the
wrapper legitimately makes, so `submit --type loop|discuss` silently stopped
working by default: the per-iteration re-dispatch was refused and its metadata
discarded, the exhausted-budget finalisation was refused, and the CLI returned 0
both times. Reproduced against the real modules. Fixed by naming those
transitions rather than carving an exception: `redispatch` and `finalize` are
requests gated on operator authority, so the wrapper can move a finished record
while an agent still cannot touch its own terminal. A re-dispatch must advance
the iteration, so a replay cannot resurrect a job. The operator CLI now exits
non-zero with a reason when a write is refused -- the silence is what let this
hide.
F-B (claude) -- moving the audit mirror out of the lock made status.json
last-writer-wins, so a slow flush could leave it behind the registry. It now
carries transition_seq and skips a write that would go backwards, restoring the
sequential consistency the lock used to provide for free.
R1 (codex, blocking) -- the policy called attribution with only the dispatch
record, so it proved the record was well-formed and never confronted the
identifiers the evidence carried. A finalized scenario with a different session,
a different attempt and a manifest from another attempt was still approved.
Observed identifiers are now collected from the timeline and every one must
agree; a manifest belonging to another attempt holds. Collecting every
occurrence rather than the first matters, because taking the first would drop
exactly the contradicting value this is meant to catch.
R2 (codex, blocking) -- a supplied expected_attempt was ignored when the record
had no attempt field, so a compare-and-swap with nothing to compare committed.
It now fails closed, which matters because 2b plans to gate inferred completion
on precisely that precondition.
R3 (codex) -- a status could ride in as an ordinary field, so `progress` could
commit a terminal from inside the boundary that exists to prevent it. Status is
now settable only by the request that owns it, unknown requests are refused, and
pending has an owning request instead of being an untyped merge.
Each of the eleven new regressions was checked against the pre-fix code first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
policy.decide is a pure function so the phase-0 scenario fixtures can be fed to
it verbatim. Those seven decisions survived three-reviewer scrutiny, so they are
the contract rather than a fresh set of rules, and the test asserts against
expected_policy.decision directly.
Order inside the policy matters. Transport doubt and staleness invalidate the
evidence itself, so they are answered before anything is inferred from it; a
blocked agent is waiting for a human and can never be completed; attribution
including the execution anchor comes before quiescence, because idle is not
evidence of finishing when a session is reused; and a report must be finalized,
never merely paused, since a debounce is a candidate filter and not an authority.
The shadow runner is never handed the engine, so it has no route to a write.
That is the structural half of P2A-03. The measured half hashes every file under
the registry directory around a full run and asserts it is unchanged, which is a
stronger claim than checking a status field.
Adds the five scenarios P2A-04 requires: a raced double dispatch, a reused
attempt with no anchor, a mid-write report, an unknown status, and a pane
restarted mid-flight. They pass phase 0's own fixture validator, which is what
keeps them honest. The phase-0 test that pinned exactly seven scenarios now
requires the seven to still exist rather than forbidding growth.
Also pins the two phase 1 handoffs as dormant: nothing here spawns a daemon, so
preexec_fn stays unreachable, and nothing calls a frozen shell helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
watcher wraps one `herdr agent wait` barrier and observes only; it never
concludes. The live contract is encoded rather than described: the timeout
parameter is named timeout_ms because herdr counts milliseconds, --until is
always passed explicitly since omitting it silently drops `working`, and a
non-zero exit is read from stderr because stdout is empty by contract.
A timeout counts as healthy. The socket answered and the state simply did not
change; treating that as a failure would make an idle agent look unreachable.
Only a matched snapshot becomes evidence, so a timeout can never be mistaken for
an observation of agent state.
Backoff doubles from 1s to a 16s ceiling and drops the cached snapshot on
failure. Reasoning from state captured before a gap is exactly what would make a
disconnect look like a completion, which is why the contract answer there is
hold_reconnect.
store keeps dispatch and attribution evidence outside the registry. The job
record carries none of attempt_id, agent_session_uuid or
pane_process_generation, phase 1 froze its schema, and this phase may not write
the registry at all. Keeping the evidence here lets shadow mode carry full
attribution while the registry stays untouched; promoting these fields is a 2b
decision. The execution anchor starts false because acknowledging a dispatch is
not evidence that the attempt ran.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
engine.transition is now the only writer of job status. It implements the target
table in state-transitions.md §3: claim, non-terminal metadata, explicit
terminals, idempotent re-receipt, refusal of conflicting or late signals with an
audit entry, stale-attempt rejection, and deadline timeout.
Two write paths become one. update_job_status merged fields unconditionally;
pick_pending hand-rolled a correct pending->running swap plus its own audit
mirror. Neither was wrong alone, but a rule added to one silently missed the
other, which is the shape that produced the phase 1 transport bugs. Both keep
their signatures and their preserved legacy bodies behind MAM_ENGINE_CAS=0.
A merged `status` value is routed to the request that owns it, so a raw field
write can no longer slip past the table. Getting that wrong first was caught by
the characterization tests: a metadata merge was overwriting the caller's status
with the pre-merge value.
Lock discipline is now enforced, not described. The lock covers a decision and
one record write; audit mirroring is queued and flushed after release, carrying a
monotonic transition_seq so ordering survives the move. A test watches for
subprocess, socket and sleep calls while the lock is held and asserts none.
Structural correction to my own plan, caught by a phase 1 guard: the plan put the
engine in lib_py/loop, but delegation would then have had to import a sibling,
which is exactly what mixes trees under a frozen loop. The engine belongs with
the registry writers, so it lives in lib_py/delegation and loop consumes it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Fixes what the job-status writers do today, so routing them through a single
transition boundary has an oracle instead of a promise. Same ordering that paid
off in phase 1.
X-02 states the claim path's real contract rather than the one the plan first
assumed: pick_pending filters on pending under the lock, so it is already a
correct swap. It duplicates the chokepoint, it does not bypass its safety. The
test pins the properties that must survive routing -- only pending is claimed,
only one session wins, and a session never sees another's work.
Every test is parametrized over MAM_ENGINE_CAS so the legacy body and the engine
body must agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Three reviewers examined fc0bd96..8def235. Codex and Claude each reproduced a
defect; all three findings were re-verified here before being accepted, and none
were rebutted.
R1 (codex) — the shell helpers ran `python -m lib_py.delegation.cli`. For -m,
sys.path[0] is the working directory, which therefore beats PYTHONPATH: with cwd
inside a tree containing lib_py, the live package won even with the frozen path
listed first. The documented B-13 guarantee for this path did not hold.
Reproduced, then replaced with an explicit bootstrap that inserts SKILL_DIR
itself. cwd is left alone, because the registry and audit-log paths are relative
to it. The file-based shim path was never affected.
R2 (codex) — the persistent connection was reused on host and port alone. Two
jobs sharing an endpoint but carrying different credentials or TLS settings
silently published over the first connection. Reproduced (the client factory ran
once, under the first account, with TLS off). The guard now compares an
immutable snapshot of the whole connection identity and names only the differing
field, never its value.
F-1 (claude) — paho queues a publish issued while disconnected and raises
RuntimeError from wait_for_publish. RuntimeError is deliberately outside the
retry tuple, so the one state a persistent connection exists to survive, the gap
between a drop and paho's reconnect, failed on the first attempt. Confirmed
against paho 2.1.0. That single condition is now translated into a retryable
error; the tuple is unchanged, so genuine programming errors are still not
retried.
Two fixtures were hiding these: FakeInfo had no rc and never raised. It now
mirrors paho's contract, which is what makes the new tests meaningful.
Also folds in the duplication both Claude (O-1) and I had flagged: the CLI path
kept its own copy of connect-publish-disconnect plus its own timeout constants.
_publish_once now delegates to OneShotPublisher.publish_raw, so a transport fix
lands once. Having two copies is exactly why these bugs existed in one of them.
Documentation corrected rather than defended: the benchmark's rationale for
keeping three forks was imprecise (the || true already guarantees rc 0; the
lookup buys quietness and legacy parity), SKILL.md no longer claims PYTHONPATH
decides the frozen path, and the daemon's directory creation is recorded as a
deliberate difference. Trailing whitespace inherited by the relocated file is
stripped, so the commit range is now clean too.
Verified: each regression fails against the pre-fix code and passes after
(R1 1, R2 4, F-1 1). 66 phase-1 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Found while self-reviewing phase 1. PersistentPublisher held the connect
handshake result on the instance, so a refused attempt left its reason code
behind for the next one. On retry the wait loop saw a non-None value straight
away and never waited for the new connection's CONNACK, which is wrong in both
directions:
* a healthy second connection was rejected using the first attempt's rc, and
* once a late callback had written a success rc, a subsequent connection that
never acknowledged could be accepted on that stale value, letting a publish
proceed on a socket that was not ready.
Both need an asynchronous CONNACK to reproduce, which is exactly how paho
delivers it; the earlier fake called the callback inline and hid it. The
handshake state is now a local the callback closes over, so it can only ever
describe the connection that produced it, and a client that fails the handshake
is torn down instead of leaked.
No production path is affected today: the CLI and shell helpers publish once
per process and use OneShotPublisher. The defect mattered because the phase 2a
engine is meant to build on this prototype, which is where a transient refusal
would have become a stuck publisher.
Also tightens PV-1 to check the bound tree against pytest's real basetemp
rather than sniffing for "pytest-" in the path.
Verified: the three new regressions fail against the previous implementation
(2 of 3, the third covers the healthy path) and pass against this one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Splits publish_event.main into three pieces with no behaviour change:
prepare_event load the job, consume the seq, build the signed payload,
decide retain. The seq is burned here, before any transport
runs, so a failed publish can never leave a number a replayed
packet could reuse.
record_outcome audit record, registry event log and status sync -- all three
best effort, all three regardless of the publish result. That
is exactly the contract G-1/G-2/G-4 pin down, now in one place.
main parse, prepare, publish with retry, record.
publisher.py holds the two transports behind one interface. OneShotPublisher is
the current behaviour. PersistentPublisher keeps a single connection and lets
paho reconnect underneath, raising on an unacknowledged publish so the caller's
retry policy stays the only thing deciding attempt counts. It refuses an event
for a different broker rather than publishing to the wrong place.
Nothing in production uses the persistent path: the CLI and shell helpers exit
after one publish. The consumer is the phase 2a engine, which pays the CONNACK
cost on every event today -- this job's own started event took ~12.5s because
of one CONNACK timeout plus reconnect.
bench.py measures both arms. The fake transport is deterministic and is what
the tests assert on; the real-broker arm is informational, since the PoC
default is a public broker whose latency is not ours to control.
Verified: N publishes cost N connections one-shot and 1 persistent; the two
transports leave field-for-field identical registry and audit records including
the HMAC signature and retain flag; an unacknowledged publish retries on the
same connection and records published=false; a second broker is refused. No
test opens a socket.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
Adds the python side of the delegation helpers and switches lib.sh over to it.
daemon.py port of start_watchdog. Same monitor path, same pgrep pattern, so
a daemon started by either implementation is found by the other;
same stderr wording and exit 1 when reconcile.sh is missing; same
"print the PID" contract, multiple pgrep matches included, since
the shell echoed them all and trimming would be a silent change.
start_new_session detaches it, and SIGHUP is ignored across the
exec so it outlives its parent exactly as nohup did.
cli.py thin router. submit/publish forward to the existing main() of the
module that already owned the behaviour, so exit codes, stdout
shape and side effects cannot drift between the shell path and
the script path. Only ensure-daemon carries logic of its own.
service.py DelegationService facade, with an optional persistent connection
for the phase 2a engine. CLI and shell always use one-shot.
lib.sh keeps the three legacy bodies verbatim as _*_legacy behind
MAM_DELEGATION_PY=0, which is the instant rollback switch. PYTHONPATH is passed
explicitly so `-m` resolves lib_py from the tree this lib.sh belongs to, which
is what makes a frozen loop run frozen delegation code. start_watchdog survives
as an alias for ensure_subscriber_daemon.
The wrapper and run_loop.sh are untouched; reconcile.sh gains only the
ensure_env_loaded line from the previous commit.
Verified: C-01..C-09 pass against both the legacy and the python bodies, the
daemon dedups and survives its parent, the cli reproduces the legacy failure
line exactly, and publish failure semantics (status synced, seq consumed,
audit records published=false) are identical through the shim and the cli.
tier1+tier2 109 passed; bash -n clean on all six shell files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
The four delegation scripts become a real package. Their old paths stay as
shims because those paths are a wire contract: agent instructions, the bash
wrapper, SKILL.md, BOOTSTRAP.md, deploy/install.sh and three test modules all
call them literally.
The shim binds the lib_py.delegation *package* to the tree beside it, once per
process, and never touches sys.path. Two cheaper designs were rejected with
evidence (the context matrix now lives in the test suite):
* inserting the sibling skills dir at sys.path[0] makes the first sandbox
copy imported under pytest shadow lib_py.agents/layout/state for the whole
session, and the identity assertion cannot see it -- `is` proves same
object, not same tree.
* loading only the named module by file path leaves a module that imports
siblings pulling them from whichever tree is on the path, so a frozen
publish_event ran against live dependencies and lost half of B-13.
Binding the package fixes both: one tree for all delegation modules, the parent
lib_py untouched, and a sys.path append only when no lib_py exists anywhere
(true standalone). Bind-once rather than nearest-wins, because rebinding would
split existing references and break monkeypatching.
Also moves the env-file load off import (_load_dotenv -> ensure_env_loaded,
called by each process entry point including reconcile.sh's embedded
subscriber), so importing the library no longer mutates os.environ.
Verified: PV-1 identity+provenance, PV-4's ten import scenarios plus K1/K3/K6
in fresh interpreters, PV-5 shim byte-identity, an AST check that the package
imports no sibling lib_py module, and --help output diffed against the
pre-move capture.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDNJkHmm18uu7n7JwV5Qd
GodopuandClaude Opus 5