- loop SKILL.md: --review-isolation and --remote-review in the option table,
a paragraph on review identity (snapshot, snapshot diff, stale re-review,
pre-promotion re-check, report-<jid>.identity.json with evidence, nested
repositories refused), and the remote-reviewer rule in the Issue #18
remote-session section.
- design doc: the §2-C reviewer sentence describes the implementation; §5
marks item 5 as loop B and says plainly that item 7's acceptance-case tests
are not written yet; the §7 loop row states the remote-reviewer limit and
that snapshot sharing is out of scope; a compatibility note for the
--worktree/--base + remote-reviewer combination.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
The remote runner subscribes to every job's retained dispatch topic on the
public broker and serves the first dispatch signed for its session. With the
fixed session name `sim-worker` and a fixed secret, a stale retained dispatch
left by an earlier run verified fine and was served instead of this run's
job, which then timed out: in the observed failure the runner logged
`published started/completed ... job=5eb1abcf` and `DONE rc=0` while the
orchestrator waited on job 5ee3fbcf with nothing received. Each run now uses
a random session name and secret, so stale dispatches cannot match.
Also, per the plan: the no-event window is 10 x 2 s instead of 3 x 2 s, and a
failure skips only when the runner provably published `started` for the
orchestrator's own job, the orchestrator subscribed to that job's topic, and
received and dropped nothing for it (broker non-delivery). Every other failure
still fails. The runner's own `started --heartbeat` already covers the window.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Active with --worktree/--base only; without them prompts, flow and promotion
are unchanged (a loop_id is recorded in the checkpoint either way).
- Every review round is pinned (B3): the source's HEAD and its working state
as a tree built through a temporary index (the real index/status/refs are
untouched); a dirty state becomes a ref-less snapshot commit on HEAD
(committed as mam-loop, not as the user). An untracked nested repository
cannot be pinned and is refused. identity = (loop_id, base, head, tree).
- Review isolation (B4): a detached review worktree of the snapshot under
TMPDIR (default), or `--review-isolation env` (the source root with env and
temp-dir instructions). Failing to open the tree refuses the round - the
isolation is never lowered silently. Trees are removed after the round and
on any exit (run() wraps the body; release_guard too).
- The reviewers' diff comes from the snapshot (B13):
diff_collect.sh takes an optional head commit for a commit-to-commit diff;
its two-argument form is byte-identical.
- Stale rounds (B7/B8): after the round the source is measured again; if it
changed, the round's approvals are discarded and the next iteration
re-reviews without a correction job (last iteration: rc 1). Only the last
non-stale round's PASSes are promoted.
- Records (B9/B15): review_rounds in the checkpoint with per-reviewer
transport/evidence, and report-<jid>.identity.json next to each promoted
report: evidence review-tree | source-tree | diff-only.
- Pre-promotion re-check (B10): a source changed after approval is not
promoted (rc 1).
- Remote reviewers (B14, Creator challenge 91c81095): they cannot reach this
host's review tree. Checked before anything is delegated: refused by
default; `--remote-review diff-only` admits them with the snapshot SHAs and
diff as their only evidence (no local path in their prompt), and refuses a
round whose diff is truncated or unavailable.
- run_loop.sh accepts both options (usage identical to the runner's); the
legacy shell loop refuses them before the lock.
Tests tests/test_issue21_loop_b.py L1-L16 (22).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
T2-6 set both variables in os.environ directly and restored them in a
finally block; a failing assertion between the two could still leak them to
later tests in the process. They are now set with monkeypatch.setenv (the
wt_repo fixture already delenvs both). The assertions are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Both unpushed checks in lib_py/worktree.py - `submodule foreach` over the
initialised checkouts and the direct `--git-dir` walk over the repositories
under the worktree's modules/ - used `HEAD --branches --not --remotes`. A
commit made on a submodule's detached HEAD and kept only by a tag is reachable
from neither, the top-level status stays clean, and worktree_remove.sh then
deleted the repository with its single --force (exit 0).
Both checks now share one rev list, UNPUSHED_REVS = HEAD --exclude=refs/stash
--all --not --remotes: HEAD and every local ref (branches, tags, notes, ...)
minus remote-tracking refs. refs/stash stays out of it because the stash check
(stash list / reflog) owns it, so a stash is reported once, as 'stashes'. As
intended, a local-only note is now protected too. The stash, error, partial
repository and dirty checks are unchanged.
Tests W6l (tag-only commit; initialised submodule and deinit'd repository) and
W6m (annotated tag). The helper header, the stop SKILL.md and the design doc
state the wider rule.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
- N1 (planner-reviewer-claude-01) worktree_remove.sh: when `git status` fails
in the worktree (e.g. its directory was deleted), judge_tree returned before
METHOD was set and the summary line died under set -u with exit 1. METHOD
now has its default before the function; the refusal is exit 3 with
"nothing was changed", plus a `git worktree prune` hint when the directory
is gone. Same for --dry-run.
- R4 (planner-reviewer-codex-01) worktree_prepare.sh: a submodule path that is
not initialised yet but already holds files made the clone fail, and the
rollback's `deinit -f` then deleted those files. --submodules now refuses
such a path up front (new exit 3, nothing changed), which also makes the
rollback's premise true: after a failed attempt, everything under a
not-yet-initialised path was created by that attempt.
- R5 (planner-reviewer-codex-01) lib_py/worktree.py: `submodule foreach`
visits initialised checkouts only, but a deinit'd submodule keeps its
repository - stash and unpushed commits included - under the worktree's
modules/, which the forced removal deletes. Every repository under modules/
(nested ones included) is now checked directly (`--git-dir`): commits on no
remote (HEAD included) and refs/stash with its reflog. A directory that looks
like a partial repository cannot be checked and refuses.
- Also: the rollback loops are C-style (`seq 0 -1` counts down on BSD), and
prepare warns that --base is ignored for an existing branch.
Tests W6i (deleted worktree directory, with and without --dry-run), W6j and
W6j2 (stash / unpushed commit in a deinit'd submodule's repository) and W6k
(existing files in an uninitialised submodule path). Skill docs and the design
doc describe the new refusal and the wider repository check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Review ffb5055e (planner-reviewer-codex-01):
- R1 [P1] worktree_remove.sh: the clean-tree and submodule checks ran only
before --stop, so work a stopping session wrote (or committed, or stashed)
was deleted by the forced removal. The checks are one function now, run
before any change and again right before the removal; any finding then
refuses with exit 3 (sessions already stopped, worktree kept), and the
removal method follows that latest look.
- R2 [P1] lib_py/worktree.py: a submodule stash is reachable from no branch
and leaves the status clean, and the forced removal deletes the worktree's
own submodule repositories. `git stash list` in every submodule is a
separate finding (`stashes`), refused like dirty/unpushed; errors fail
closed as before.
- R3 [P2] worktree_prepare.sh: when one submodule cloned and a later one
failed, the rollback deleted modules/ while the cloned checkout's .git file
still pointed into it, and `git status` failed in the worktree. The rollback
now undoes only this attempt: each submodule whose git dir did not exist
before is deinitialised before its git dir is deleted; submodules
initialised earlier are kept; shared submodule.* config values are restored
exactly (new keys removed, pre-existing values put back - init and deinit
both write .git/config). A final `git status` either confirms a usable
checkout or prints what to inspect.
Tests W6e (a write during stop keeps the worktree and the file), W6f (a
submodule stash refuses), W6g (partial failure on a fresh worktree: usable,
no modules/, no new config, then prepare and a plain removal work) and W6h (a
failing retry keeps the earlier `sm` init and its config value). The skill
docs and the design doc describe the recheck, the stash rule and the scoped
rollback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
- design doc §3: the hand-assembled worktree/.venv/create recipe is replaced by
worktree_prepare.sh --creator-agent (its BASE= line is the base to record);
cleanup is worktree_remove.sh --stop; resume needs the original path.
- §4: what the helpers do - no .mam.env copy, the shared info/exclude, opt-in
submodules (any init leaves modules/, which blocks a plain remove), removal
of submodule worktrees with one --force after the checks, and never by hand.
- §5: item 4 is loop A on this branch; phase-1 items are done in v4.6.0.
- §7: create (control-root reconcile/watchdog, loud binding failure), resume
(original cwd only, binding persisted when already running), stop (the
removal helper's checks), monitor (register via create/prepare only).
- create / stop / resume SKILL.md: usage and exit codes of worktree_prepare.sh
and worktree_remove.sh, the original-cwd rule and its exit code (1 -> 2).
Frontmatter versions are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
- worktree_prepare.sh (create skill): idempotent `git worktree add` next to the
control root (a path inside it is refused), .venv linked to the control
root's, /.venv and .pytest_cache/ in the shared info/exclude, submodules only
with --submodules - a failed init removes its own traces (modules/, new
submodule.* keys in the shared config) and exits 1, since git refuses a plain
`worktree remove` once modules/ exists - and an optional creator registered
in the control root's .mam with MAM_NO_SUBSCRIBER_DAEMON=1. Prints BASE= for
`run_loop.sh --worktree ... --base ... --review-only`.
- worktree_remove.sh (stop skill): every check first, nothing changed on a
refusal (exit 3): linked and not locked, running sessions only with --stop,
no open job of those sessions except the delegate jobs stop cancels, clean
tree including submodules, and inside submodules no modification and no
commit on no remote (HEAD included - submodules sit on a detached HEAD).
Then stop, re-check jobs, and remove: plain, or a single --force when the
worktree holds submodule state.
- lib_py/worktree.py: the parsing behind both (stdlib only).
- A8: create's immediate reconcile and watchdog use the control root's code
and WORKSPACE_ROOT, not the worktree --workspace's copy.
Tests W1-W6d (local submodules allowed through GIT_CONFIG_* env only).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
- A11: resume refuses (exit 2) a --workspace other than the row's pane.cwd, and
a pane.cwd that no longer exists (checked first: mam_abs_workspace returns a
missing path unchanged). Before, claude/grok failed with a misleading "No
saved session" and class A agents started a new conversation in the wrong
directory. Rows without pane.cwd are not checked.
- A12 (recommendation 1): create and resume no longer swallow a failed binding
computation. A worktree (--workspace != control root) is refused with exit 2
and nothing spawned or recorded; the control root itself warns and records
the row without control_plane.
- A13 (recommendation 2): resume computes the binding before the herdr-alive
check, so the already-running path records it too (update_yaml_resumed.sh
fills a missing field only). Pane forwarding and the whitespace refusal stay
on the spawn path.
Tests W7-W9.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Both installers wrote `/.venv/`, which only matches a directory: a .venv
symlink (the worktree recipe links the control root's) or file showed up as
untracked in installed repositories. The managed block now has `/.venv`.
Existing installations get it on the next install/update, which rewrites the
block between its markers as a whole; the integrity values are unaffected
(.gitignore is in neither asset_hashes.txt nor the install manifest).
Tests W10 (both installers; .venv as link, file and directory all ignored) and
W11 (a pre-#27 block is replaced, user lines outside it are kept, .gitignore
stays out of the ledgers).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
d34b532 committed `.venv` as a symlink to
/Users/godopu16/PuKi/laa/canary_projects/multi-agent-mux/.venv, the merge
worktree's preparation link. `.gitignore` had `.venv/`, which matches
directories only, so the link was not ignored. Updating a checkout to main
then silently replaces its existing (ignored) .venv directory with that
link: a broken path on other hosts and a self-referencing link in the
original checkout. Untrack it and ignore `.venv` in both forms, which also
covers the worktree preparation link of the #21 operating model.
Also correct the release date at the top of VERSIONS.md to 2026-09-14 to
match the v4.6.0 entry. No code change; the version stays 4.6.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PfoijjCS2U6TQPFuxAE5Pn
Reviewed with [VERDICT: PASS] from claude, agy, grok (author-side loop) and codex; re-reviewed merge-side by claude, agy and codex at 4a81d15. Full suite at 4a81d15: 940 passed.
Issue #21 phase-1 prerequisites (PR #22) add loop runner options and the
control-plane module: MINOR. VERSIONS.md gains the v4.6.0 entry, the
archived design reviews lose their markdown trailing whitespace so
`git diff --check` is clean, and the three merge-side PASS reports
(claude, agy, codex @ 4a81d15) are archived under .agents/reports/.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
run_loop.sh sources lib.sh before it refuses --worktree/--base/--review-only
under MAM_LOOP_PY=0, and sourcing lib.sh runs _init_herdr_isolation, which
rewrites $WORKSPACE_ROOT/.mam/shim/herdr. T2-8 copied the caller's
environment as is, so that root was the caller's control root (normally the
real checkout). The test now requests mam_sandbox, pins WORKSPACE_ROOT to it
explicitly, runs from it, and asserts the shim was written there; the four
parametrized refusal cases and their three assertions are unchanged. No
product code changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Review N1 (planner-reviewer-claude-01 50ae9fb0, planner-reviewer-codex-01
8779f4b4): af8a54c (R1) stopped reusing an idle pane when env flags are
present, because the old path put `env K=V ...` after `herdr agent start ...
--`, where herdr passes it to the agent executable as arguments. h25 still
required that old behaviour (idle pane reused, env in the agent command) and
failed. It now asserts the new contract instead: a pane split / workspace
create carries MY_TEST_FLAG=active and HERDR_WORKSPACE_ID as --env, the agent
does not land on the idle pane, and nothing env-like follows `--` in agent
start. Finding A's intent (env must not be silently lost) is kept; reuse
without env flags stays pinned by h24. No product code changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Review 9a1adb46 (planner-reviewer-codex-01):
- R1 [P1] lib.sh new-session: a forwarded binding reached the agent as
arguments. With env flags the idle-pane branch prefixed `env K=V ...` to
the command that `herdr agent start ... --kind <agent> -- <args>` passes to
the agent executable. An idle pane is now reused only when no environment
has to be set; otherwise the pane is created by pane split / workspace
create, which take --env. The dead `env` prefix code is removed (its
send-text fallback would have typed the same line).
- R2 [P2] runner: a relative MAM_JOBS_DIR is resolved once against the
control root, the way the wrapper (cwd=workspace_root) reads it, and that
absolute dir is used for delegation, wait, report lookup and cleanup.
- R3 [P2] stop: the row's control_plane.delegate_job_logs_dir is restored
(exported unless the caller set DELEGATE_JOB_LOGS_DIR), so the
cancellation history lands in the row's audit-log root.
Tests: T3-1 and T3-3 now assert that the binding arrives as --env on the
pane-creation calls and that no env/K=V token follows `--` in agent start
(fresh and reused pane); T3-4c (audit root on stop) and T2-15 (relative
MAM_JOBS_DIR) are new. The design doc's compatibility table and a create
comment no longer describe the removed idle-pane env path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
Fixes the sentences the four design reviews flagged and records what phase 1
actually implements: status line, monitor-topic scope, option A's isolation
and capacity wording, creator/reviewer ownership (tests write; promotion is
the control root's), the control-plane binding persisted and forwarded, two
loop recipes ((a) the loop implements, (b) the base recorded before an
out-of-loop implementation, then --base --review-only; no base in --task),
the loop lock as a refusal, "phase 1 = parallel implementation, one review
loop", the eight Issue #21 items with their status, and a new section 7 with
root definitions and a per-skill compatibility table. The review files are
unchanged. The loop SKILL.md lock note names the lock branch instead of line
numbers that moved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
A session row now records the control plane it belongs to, and the pane
gets it explicitly instead of relying on env inheritance.
- lib_py/control_plane.py (+ thin lib.sh wrappers mam_control_plane_json /
mam_control_plane_prefix): the binding is WORKSPACE_ROOT,
AGENT_SESSIONS_YAML, MAM_JOBS_DIR, DELEGATE_JOB_LOGS_DIR as absolute paths.
- create: the binding is always persisted on the row (`control_plane`) and
forwarded to the pane only when it differs from what --workspace alone
implies, as leading K=V tokens that lib.sh's new-session already turns into
--env (fresh pane) or `env K=V` (reused idle pane). pane.cmd_full and
start_command stay unprefixed. A binding with whitespace that would have to
be forwarded is refused with exit 2 (lib.sh splits the flags unquoted); the
~/.local/bin wrapper path records the binding but warns it is not forwarded.
- resume: the row's binding wins over the caller's env (a difference is
reported on stderr) and is forwarded again; a legacy row without one uses
the caller's binding and update_yaml_resumed.sh records it.
- stop: registry order MAM_JOBS_DIR > the row's mam_jobs_dir >
$WORKSPACE_ROOT/.mam/jobs > .mam/jobs, exported for delegate_publish_event.
- status: both get_job_status copies look in MAM_JOBS_DIR, the row's
registry and the matching audit logs before the old candidates.
- reconcile: MAM_JOBS_DIR wins in the subscribe heredoc and is passed in.
Tests T3-0..T3-8 (mock herdr: forwarding is verified at the herdr-call level
only; real pane env propagation is left to item 7). T3-7 is a static guard.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
- --worktree <path> (CLI over MAM_LOOP_WORKTREE): the source root. Only the
diff, base, head and the empty-set check use it; the wrapper, registry,
checkpoint, loop guard, promotion and cleanup stay on the control root. The
collector is the control root's diff_collect.sh (never the worktree's
unreviewed copy), its arguments shlex-quoted. Validation: an existing
directory, the top of its working tree, the same git common dir as the
control root (".git" is resolved against the directory it came from).
- --base <ref> (CLI only, no env - a stale sha would silently widen the next
range): resolved to a full sha, must be an ancestor of the source HEAD.
- --review-only: requires --base, refuses --plan; Phase 2 is skipped and
logged; correction and comply jobs still go to the creator.
- With --worktree or --base every delegated prompt starts with a context
block (path, branch, base and its origin, head, base..head commit count,
uncommitted changes); without them prompts are byte-identical. All call
sites in run() go through one _dispatch helper.
- Fail-closed guard (only with --worktree/--base): before each review round,
self-review included, an empty change set or one git cannot determine
(exit codes, never output parsing) ends the loop with rc 1 instead of
asking for reviews. This closes the fail-open where a loop started after
an out-of-loop commit sent reviewers "(no changes since base commit)".
- The runner honours MAM_JOBS_DIR and passes --registry-dir to the wrapper.
- run_loop.sh accepts the three options; the legacy shell loop
(MAM_LOOP_PY=0) refuses them before the freeze and the loop lock.
Tests T2-1..T2-14.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
The injected prompt, the start/success/error commands and the brief's Output
Report Path used cwd-relative `.mam/jobs/...`, and the wrapper ignored
MAM_JOBS_DIR. A creator whose cwd is a git worktree could not run them as
written, and its audit log fell back to <cwd>/.mam/delegate_job_logs.
- REGISTRY_DIR defaults to ${MAM_JOBS_DIR:-.mam/jobs} and is made absolute
right after `cd "$WORKDIR"` (a relative value still resolves against WORKDIR)
- the publish command is shell-quoted (printf %q) and prefixed with
DELEGATE_JOB_LOGS_DIR=<abs>, resolved by the same get_logs_dir() the
wrapper's subscriber uses; the heartbeat timer inherits it
- direct and loop jobs share one _instructions helper; brief and report paths
are $REGISTRY_DIR/$JOB_ID/... (absolute)
- create --onboard: absolute publish_event.py, --registry-dir and log root
Tests T1-1..T1-5 (a space in the control root, commands run from another cwd
with no workspace env).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017R3Eg7MEpSWnm1oB9ZPP6G
claude, agy, grok: PASS with amendments; codex: NOT PASS (recipe overstates
current env-override support; review identity and lifecycle ownership
underspecified). Direction (creator per worktree, shared reviewer pool)
agreed by all four.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Design proposal for review (no code change): why the checkout-path
workspace assumption blocks parallel issues and caused the #18 checkout
switch, three candidate models, the recommended hybrid (writers isolated
per worktree, readers shared, one .mam control plane via env overrides),
how it maps onto today's scripts, what has to change, and the questions
put to the reviewers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Review round 5 (planner-reviewer-codex-01, NOT PASS at 2b1d590):
- F1 bounded lock cycle: the stopper holds the per-job lifecycle lock while
it waits for the timer to exit, and the timer's exit cleanup waited for
that same lock, so every stop ran out the drain and ended in SIGKILL with
a false "did not drain" warning. The exiting timer now tries the lock
without blocking (lifecycle_lock(blocking=False)); when it is busy the
ownership operation holding it removes the pidfile, and when it is free
the own-pid comparison is kept.
- F2 untracked owner: when writing the new timer's pid fails after the
spawn, _spawn_timer_locked kills and reaps that exact process while the
lock is still held and drops only a partial pidfile of that attempt,
then re-raises.
- tests: an idle timer stops well before a 10 s drain without SIGKILL;
a pidfile failure after spawn leaves no survivor, no pidfile and a free
lock (both fail on 2b1d590); a spawn failure leaves nothing behind and
the next start works.
- docs: MESSAGING 2.5 and VERSIONS item 1 describe the lifecycle lock;
VERSIONS test count 31.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
Review round 4 (codex R3): two concurrent `start_timer` calls for the same
attempt both passed the "no timer" check, both spawned, and the pidfile
tracked only one - the untracked timer could still publish a retained
`running` after `completed`. A per-job flock (`<job>.heartbeat.lock`,
separate from the registry/seq lock so in-flight beats can still take
their seq) now makes observe/drain/spawn/publish-pid one ownership
operation; the terminal stop takes the same lock; a start that finds the
attempt ended (terminal record or raised barrier) does not arm; the timer
removes its own pidfile only under the lock so a replacement's pid is never
deleted. Regressions: concurrent starts leave exactly one tracked owner that
the terminal stop drains; a start after the barrier arms nothing.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
Review round 3 (planner-reviewer-codex-01, NOT PASS R3 at 7e1d3f0):
start_timer stopped a running timer with a bare TERM and overwrote the only
pidfile. A beat the old timer had already admitted - seq taken, recorded on
disk, held at the wire - stayed alive but undiscoverable, so a later
terminal publish drained only the replacement and the old beat could still
put a retained `running` on the broker after `completed`.
- heartbeat.start_timer: stop_timer(drain=MAM_HEARTBEAT_DRAIN_S). The old
timer is drained or killed, and confirmed gone, before the pidfile names
the new one. A terminal publish racing the replacement either drains the
same old pid or meets a new timer whose beats the barrier refuses.
- test: an old timer held at the wire after admission is replaced, then
the job completes; the old timer is gone before the handover and nothing
it held follows the terminal (fails on 7e1d3f0).
- docs: MESSAGING 4.3 no longer says any state resets the idle clock;
MESSAGING 2.5 and VERSIONS state the replacement rule; test count 26.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
The subscriber is started before the injection, and the injection itself
(idle handshake, quiescence, delivery grace) can take tens of seconds, so
a first-event budget of T + K x N measured from the subscription could
expire before `started` was even possible. The wrapper now passes
--ack-timeout <wall clock>: it judges delivery with _wait_started (T from
the injection) and reaps the subscriber on failure; K x N applies from the
first event on. A standalone subscriber keeps T + K x N. Documented in
MESSAGING.md 4.3.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
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
Second-round reports of the fix/issue-12-8-7 review loop on ea911d7:
planner-reviewer-claude-01 (b368210a), reviewer-agy-01 (3f5d1cd4) and
reviewer-grok-01 (cc059645), all [VERDICT: PASS]. The codex review is
deferred until its usage limit resets; the branch merges only after it.
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
PATCH release for the fix/issue-12-8-7 branch (Issue #12 broker password
kept out of job records, #8 fresh install stamps kept on update, #7
same-cwd workspaces packed). No code logic changes.
- lib.sh MAM_VERSION, lib_py/loop/eager.py FRAMEWORK_VERSION and the eight
SKILL.md frontmatters (create, stop, resume, status, monitor, delegate-job,
loop, orc-onboard) move from 4.4.3 to 4.4.4
- VERSIONS.md: current release v4.4.4 (2026-09-13 KST), the eight-row skill
matrix, and a v4.4.4 changelog section in the v4.4.3 format covering the
password-free registry block restored from MQTT_PASSWORD at connect time,
the scrub on every record write, `registry.py scrub-secrets [--dry-run]`,
why auth_token/dispatch_token stay, the update.sh stamp and install_state
merge rules, the sibling-workspace walk in new-session, and the 8 new tests
- VERSIONS.md v4.4.3 ledger correction: its new-test count predated fae70c1,
which added two purge tests; now 26 (19 + 7) with both listed
- README, README.ko, BOOTSTRAP and BOOTSTRAP.ko badges and the MESSAGING.md
"Applies to" header follow the current version; historical 4.4.3 mentions
(v4.4.3 changelog, reviewer reports) stay as history
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
GodopuandClaude Opus 5