8ea65bb23830b56c4530897270edaa6e556de392
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  GodopuandClaude Fable 5.1
|
05b0ea4762 |
fix(issue-23-24): merge review - the environment form of the auto-trust switch reaches the handler; a rate-limited dialog does not hide the next one
Two probes from the merge-side codex review (its report was lost to an API outage; the probes and their results were recovered from its transcript): - create_session.sh and resume_session.sh set AUTO_TRUST=1 and then exported MAM_AUTO_TRUST from it, so the documented MAM_AUTO_TRUST=0 without a flag reached handle_startup_dialogs as 1: the operator had turned automatic trust off and it stayed on. Now AUTO_TRUST defaults to the environment value and an explicit --auto-trust/--no-auto-trust wins. - _mam_dialog_pick exited on the first dialog inside its resend cooldown instead of looking further down the screen, so a skipped (--no-auto-trust) or just-answered trust prompt hid the codex update notice below it, which the docs say is always answered. It continues to the next dialog now; S5 and S5b (one send per cooldown, one retry after it) still hold. Regressions: S6c, S5f (startup path), R3b and C3 (resume/create, env only, flag over env). Each fails without the fix. The create skill doc names the environment form and its precedence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff |
||
|
GodopuandClaude Fable 5.1
|
f4ba205c9f |
fix(issue-23): the remote audit record names the dispatch it vouches for; a record without identity fails closed
Review round 4 (planner-reviewer-claude-01 B-1, planner-reviewer-codex-01 P2, reached independently): dispatch_verified is a boolean. It says the subscriber's token check ran when the record was written, but names no dispatch, the token being masked by the audit redactor and attempt_id not written. events.ndjson is appended to across a job's attempts, so a started verified under attempt 1 satisfied every later attempt's wait - the very misattribution R1 forbids, alive on the remote path while the local path (data.dispatch_token in events.log) was already immune. codex reproduced it with the real signed producer, the real _Watcher, the real audit writer and the real operator re-dispatch; both waits answered 0 where 1 was due, even when attempt 2's subscriber had correctly dropped attempt 1's replayed packet. The subscriber now writes dispatch_fingerprint = SHA-256 of the token it verified, and attempt_id for readers. With a token demanded, the predicate accepts an audit record only if it is dispatch_verified AND its fingerprint equals SHA-256 of that token; a record with no fingerprint - written before the field existed - names nothing and is rejected on that path. Without a token, any received started counts, as before. Fingerprint over attempt_id as the identity, weighing the two proposals: the local path already compares the token, so hashing it keeps one identity axis for both sources; the token is a uuid4 nonce (registry.py:141), 128 bits, so its SHA-256 is not recoverable and it is a CAS fence rather than the HMAC secret in the first place; and attempt_id cannot tell apart a token re-minted within one attempt, where the token can. attempt_id is recorded because it is cheap and readable, not relied on. W7 pins codex's five cases end to end - current dispatch accepted, an old attempt's record rejected after a real re-dispatch, still rejected when the stale packet is replayed and dropped, the new dispatch's own started accepted, a bad HMAC never recorded - with create's wait and the wrapper's wait required to agree on each. W8 pins the fail-closed legacy record and the untokened compatibility path. W5 gains the no-fingerprint and other-dispatch cases; W6 checks the real _Watcher writes the fingerprint and never the token. S7e's status reset had no reason left and is gone. codex's own reproduction file, unmodified, passes 5/5 against this tree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
GodopuandClaude Fable 5.1
|
087eacaf21 |
fix(issue-23): both ack waits share one contract - the wrapper's status bypass is gone; the G9 guard removes only the docstring node
Review round 3 (planner-reviewer-codex-01, reviewer-opencode-01): R1 blocking, R2 and the "ONE builder" wording non-blocking. All three reproduced; none rebutted. R1. _wait_started still returned 0 as soon as the record left `pending`, ahead of the shared predicate, so `error`, `cancelled` and another dispatch's `running` all acknowledged an injection that create's delegate_wait_started rejects - the two contracts the ack-finding brief said must not diverge. Both reviewers reproduced it with the real producer and no record manipulation; my S7n had hidden it by resetting the record to `pending`. The bypass and the console grep are gone. The wrapper's evidence is now delegate_started_recorded alone, the same call create makes. For a remote agent - no shared disk, so no line in events.log - the predicate reads the subscriber's audit record of the `started` it received. That record is attributable by construction: the subscriber verifies the HMAC and drops any event whose data.dispatch_token differs from the job's expected token before it writes anything. The token itself cannot go into the record (the audit redactor masks token-named keys), so the subscriber now stamps a boolean dispatch_verified, and with a token demanded only a verified record counts; an untokened legacy record fails closed. The registry status is never evidence; a terminal job is preserved by the terminal logic, not turned into an acknowledgement. Tests: W1/W2/W3 are the reviewers' three negatives and the positive, real producer, record left exactly as it produced it; W4 runs create's wait and the wrapper's wait side by side on all four and requires the same answer; W5 pins the remote route (a verified record counts, an unverified or non-started one does not); W6 drives the real _Watcher and shows it stamps the flag on a matching started and records nothing for a mismatch; S7n no longer resets the record. test_issue14's "registry ack over unconfirmed inject" produced its ack with a bare status flip - the very bypass - and now produces it the way an agent does, publish_event with the dispatch token; its scenario and every other assertion are unchanged. test_issue18's remote e2e accepted the status messages and now expects the shared one. R2. _code_only blanked a docstring's whole line range, so code sharing that line - `"""doc"""; call()` - vanished from the scan. It now removes only the docstring node from the AST and unparses the rest (comments drop out with it, code strings stay); test_g9b pins the four same-line shapes. The builder comment claimed one builder for create and the wrapper; the wrapper keeps _pub_cmd and S7l pins the two equal argument-for-argument. The comment now says that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
GodopuandClaude Fable 5.1
|
1667fda8dd |
fix(issue-23): the onboarding ack is satisfied by what a real agent publishes; one command builder and one started predicate for create and the wrapper
Trial 2 of this branch: cross-server delivery was restored (
|
||
|
GodopuandClaude Fable 5.1
|
f7cee00dec |
fix(issue-23): the caller's server comes from HERDR_SOCKET_PATH, not only HERDR_SESSION
The live trial of this branch found the cross-server verdict inert on this host: _herdr_call_is_cross_server read the caller's server from HERDR_SESSION, and herdr 0.8.2 does not export that variable to a pane of the default server - the orchestrator's usual home, and exactly the pane the #23 leak comes from. Every call was judged "not crossing", the three ids were never dropped, and all four trial onboardings ended "no started within 180s". My own pane sits in a named server and does carry HERDR_SESSION, which is how the assumption got past me. HERDR_SOCKET_PATH is exported to every pane and its layout names the server: .../.config/herdr/herdr.sock is the default server and .../.config/herdr/sessions/<name>/herdr.sock a named one - the convention `herdr session list --json` reports and the shim already uses to find a named server's socket. _herdr_caller_server prefers HERDR_SESSION when a herdr does export it, otherwise reads the socket's shape (suffix-matched, not $HOME- anchored), and yields "" for anything unrecognised so an exported scope is respected rather than guessed away. The rest of the rule is unchanged from |
||
|
GodopuandClaude Fable 5.1
|
10a678c010 |
fix(issue-23-24): the dialog handler sees the ready banner on its own; only this dispatch's started is delivery evidence
Review round 1 (planner-reviewer-claude-01, reviewer-agy-01, planner-reviewer-codex-01): one blocking defect and two P2s, all confirmed. B-1. handle_startup_dialogs took its ready tokens only from the environment, which wait_for_tui_ready fills when it evaluates the adapter facts. Called on its own - as resume_session.sh calls it, with no readiness wait before it - it had no tokens, its early-return branch could never fire, and every resume of every agent idled through the full 20 s timeout (base: 0.03 s). rc was 0 throughout, so nothing failed; the resume tests just took ~30 s each. _mam_load_dialog_facts now also takes MAM_STRONG_READY_TOKENS / MAM_READY_TOKENS from the facts, but only when the caller has not set them, so a caller-set value (wait_for_tui_ready's own, or a test steering readiness) is never clobbered - the concern that made it take a single fact in the first place. S5d pins one-iteration return on a ready first frame for five adapters with no environment; S5e pins that caller tokens survive; R4 gets a wall-clock cap so a timeout-idle can no longer pass as success. The old _MAM_READY_TOKENS_CLAUDE fallback was left defined but unread - removed. R1 / N-2. delegate_wait_started treated any status past `pending` as the agent's acknowledgement. codex reproduced three false positives: `error` and `cancelled` records with no started line, and a `running` record from another dispatch. The status branch is gone; the evidence is a started line carrying this dispatch's token, which the local publisher writes before it syncs the status, so nothing that really started is missed (S7c re-pinned on the line with running/completed/error statuses; S7f/S7g pin the three negatives). N-4, taken with it: the events.log line is parsed as JSON rather than pattern- matched. publish_event nests dispatch_token under `data`, and the old regex only matched because of key order (S7h, real line shape and a reordered one). R2 / N-1. Two tests hardcoded <repo>/.venv/bin/python and failed with FileNotFoundError in every review worktree. sys.executable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
GodopuandClaude Opus 5
|
33b91fe0c8 |
fix(issue-23): an unacknowledged onboarding marks the job and keeps the session
Two defects in the started-based verdict, both found by the full suite.
The first was destructive. No `started` within T exited 1, which fired the
rollback trap and killed the session. That is worse than the bug being fixed:
create_session already has a rule for "the agent is not responding but the
process is alive" one step earlier - a TUI that never signalled ready is
preserved for diagnosis - and an onboarding that is merely slow is the same
situation. The job is marked `error` and warned about loudly; the session, its
row and the pane stay. MULTI_AGENT_RULES §6 has the GM gate on this job
anyway, so an unacknowledged onboarding still blocks real work.
It also broke 21 tests across 8 modules (create/sanity/tier2/tier3/tier4/
issue15/issue19/issue21) and added roughly an hour to the suite: no sandbox
has an agent to publish `started`, so every create_session test sat out the
full 180 s and then failed on the rollback. MAM_ONBOARD_ACK_S shortens that
wait, and the sandbox fixture sets 3 s. It is deliberately not the wrapper's
MAM_INJECT_ACK_GRACE_S - that is a different wait whose own tests assert on
the value they pass, and reusing it made issue18's acceptance2 read "within
3s" instead of "within 4s".
The second: the delivery failure published `error` and then the rollback
published `cancelled` on the same job, which the registry refuses
("conflicting cancelled after terminal error"). Terminal events are published
exactly once, so the trap now skips a job this run already closed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
GodopuandClaude Opus 5
|
123dca47bb |
fix(issue-23): onboarding delivery is judged by the agent's own started event, and a rollback verifies the kill
Plan commits 3 and 4 landed together: both rewrite the same region of create_session.sh and share one test file, and splitting them afterwards would have meant hunk-level surgery on already-verified code. Delivery verdict (item 3). create_session --onboard called a healthy agent a failure three seconds after registering the job: inject_instructions -> send_keys_safe returns rc 4 when `agent prompt --wait --until working` fails and the pane is still not working two seconds later, which is exactly where a TUI that is still starting up lands. It never got the ISSUE-18 confirmation the delegate-job wrapper has. rc 4 is a warning now and the agent's own `started` decides, within the job's own ack timeout; rc 1/2/3 stay hard failures, since they say the pane never quiesced, a dialog was blocking, or the text never appeared, and no later `started` repairs those. The self-published `started` had to go first. create_session published it on the agent's behalf right after injection, so the registry showed a started job whether or not the agent ever saw the prompt - with that line in place, no started-based verdict can mean anything. delegate_wait_started is lib.sh's counterpart to the wrapper's _wait_started (plan item D10-b). The wrapper's own `submit` was the first choice, but it blocks until the job reaches a terminal state, and create_session must return as soon as delivery is confirmed; the on-disk evidence it reads - registry status past `pending`, or a started line carrying this dispatch token - is written by publish_event before its wire publish, so no broker is involved. Rollback integrity (item 4). The trap ran `_herdr kill-session -t "$SESSION_NAME" 2>/dev/null || true`, so a kill that closed nothing still printed "rolling back" while the session kept running, and the row stayed `running` pointing at the job the same rollback had just failed (#23 symptom 5). With item 1 in place this check is finally trustworthy: the kill used to fail because the caller's workspace id hid the pane. The trap now reads the kill's rc, confirms with has-session, closes the onboarding job as `cancelled` (a terminal transition the registry knows, and the truthful one - the job was never dispatched), and marks the row. A session still alive exits 43 with the manual cleanup command, distinct from 42 (TUI never ready). The row keeps a schema-valid status rather than the plan's `error`, which atomic_yaml.py rejects: `terminated` once the session is really gone, and `running` when the kill failed, because it is still running. What makes that case legible is the note, the cleared delegate_job_id and exit 43 - not a status claiming the opposite of the truth. The row is never deleted; a deleted row takes the evidence with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
GodopuandClaude Opus 5
|
5ba04a27ca |
feat(issue-24): startup dialogs come from the agent adapter and are handled while waiting for the TUI
handle_startup_dialogs was a claude-only string ladder called only when AGENT=claude, and wait_for_tui_ready does not treat a dialog screen as ready. So agy, grok and codex sat on their directory-trust prompt for the full readiness timeout, the session was recorded as tui-ready-timeout, and onboarding injection was disabled (create_session.sh:313-314) - a human had to accept each dialog by hand. The signatures and accept keys move to the adapters as `startup_dialogs` (pattern, keys, kind), published through the existing `facts` eval contract as MAM_STARTUP_DIALOGS. claude's ladder moves over verbatim; agy takes Enter, grok y, codex Enter, and codex's update notice takes 2 (Skip), which blocks a start just as hard while changing nothing. kind is `trust` or `notice`: --no-auto-trust leaves trust dialogs to a human and still answers notices, since trusting a directory makes codex load its project-local config, hooks and exec policy. Re-sending is gated by time (MAM_DIALOG_RESEND_S, default 5s), not by frames. #24's comment reports that `--source visible` keeps showing an accepted dialog for a while, and _pane_capture hardcodes that source, so "still on screen" is not evidence the keys were missed; acceptance is judged by the ready tokens alone. Two existing tests asserted the old shape and are updated, not weakened: b19's "Escape branch stays" now looks for it in claude's adapter, and c1b's "Enter once per iteration" becomes "never twice for one frame", which the cooldown satisfies strictly more tightly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
GodopuandClaude Opus 5
|
3bcaa39ab0 |
fix(issue-23): a herdr call to another server drops the caller pane's workspace/pane/tab ids
A MAM script often runs inside one herdr server's pane (the orchestrator's) while targeting another (the isolated MAM server). The pane exports HERDR_WORKSPACE_ID, HERDR_PANE_ID and HERDR_TAB_ID, and those ids only mean something on the server that issued them. They reached the scope filters as a foreign workspace, whose fail-closed miss (ISSUE-3 / F-2b) reports a live session as absent: has-session, kill-session, agent prompt and the delegate-job preflight all answered "no such session". Reproduced on Linux and macOS in the #23 comments, worked around until now with `env -u` at every call site. mam_herdr drops the three ids for a call aimed at a named server other than the caller's own. Scope then falls back to .mam/herdr_workspace_id and finally to server-global, which is what _herdr_ws_scope already does. `default` is not a crossing: it is the sentinel for "pass no session flag", so herdr answers on the ambient server, which for a caller in a pane is that caller's own. A caller outside any pane keeps an exported scope, so H-18/H-21/H-23 keep their meaning. The fix lives in mam_herdr, not in the shim heredoc: that heredoc is materialised into $WORKSPACE_ROOT/.mam/shim/herdr, one file shared by every checkout bound to a control plane and rewritten on every herdr call, so editing it would put unreviewed code in the control root and let two checkouts overwrite each other. S11 fixes that invariant. lib_py/loop/watcher.py builds its own herdr argv and is the one path that does not go through mam_herdr, so it applies the same rule via scoped_env. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |