From acceadbdf5687df5f5a006aa2691f2338a527eee Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 11 Sep 2026 09:55:38 -0700 Subject: [PATCH 01/12] feat(self-write): the seat's own identity writer, its fence, and a seat-local lock (#1152) One path writes a seat's own cells -- placement record, pane label, agent key, session name -- for the calling seat only, from a pane handed in by the channel. The record gains a fourth field, fence=:, read once per generation through the new optional driver op terminal_fence and re-read before every later mutation; a mismatch refuses that mutation visibly. The session rename is typed once, unconditionally, when the pane is ready: no pre-read skip, mark or flag. Policy is decided in one place from the record cell alone. The generation runs under a seat-local single-flight lock built on the actas lock's core, which now takes a lock path and owner. Extracting it surfaced and fixes, for both locks: an ownerless reclaim mutex that outlived a crashed reclaimer forever, a tombstone restore that deleted the only inode on a failed link, and mutex helpers whose nonzero status killed a set -e caller before any verdict was printed. Closes #1137. --- scripts/drivers/terminals/herdr/ops.sh | 28 ++ scripts/drivers/terminals/plain/ops.sh | 3 + scripts/drivers/terminals/tmux/ops.sh | 17 + scripts/lib/actas-lock.sh | 211 ++++++++-- scripts/lib/self-write-lock.sh | 89 ++++ scripts/lib/self-write.sh | 268 ++++++++++++ scripts/lib/terminal-registry.sh | 2 +- tests/test_actas_lock.bats | 9 +- tests/test_self_write.bats | 236 +++++++++++ tests/test_self_write_lock.bats | 537 +++++++++++++++++++++++++ 10 files changed, 1365 insertions(+), 35 deletions(-) create mode 100644 scripts/lib/self-write-lock.sh create mode 100644 scripts/lib/self-write.sh create mode 100644 tests/test_self_write.bats create mode 100644 tests/test_self_write_lock.bats diff --git a/scripts/drivers/terminals/herdr/ops.sh b/scripts/drivers/terminals/herdr/ops.sh index 49162999..3e5b355f 100644 --- a/scripts/drivers/terminals/herdr/ops.sh +++ b/scripts/drivers/terminals/herdr/ops.sh @@ -1330,5 +1330,33 @@ _herdr_panes_of() { # [ -n "$pane" ] || continue printf '%s\t%s\n' "$sock" "$pane" done +} + +# Fence for a self-write (#1152). Prints "\t" for one pane: +# the herdr SESSION this driver is talking to (pane ids repeat across sessions -- +# w1:p2 exists in both `jugemu` and `oma`, measured 2026-09-11) and the pane's +# server-side terminal_id (unique across sessions, 52 panes / 0 crossings; CHANGES +# across a herdr restart, so a stored fence expires with the server and a later +# write refuses instead of landing in whatever now sits at that pane id). +# Each half is either a value or a namespaced reason; the caller compares both +# against the stored pair right before each mutation. This is a PREFLIGHT check, +# not an atomic fence: the read and the keystroke are separate calls, so a pane +# closed and reused between them is not caught (a real fence would need herdr +# to compare-and-type). What it removes is the day's actual accident -- a write +# resolved in one session landing in another's live pane. +terminal_fence() { # + local id="$1" instance pane_json esc tid + instance="${HERDR_SESSION:-}" + [ -n "$instance" ] || instance="unknown:no_session_in_env" + command -v herdr >/dev/null 2>&1 || { printf '%s\tunknown:terminal_unreachable\n' "$instance"; return 2; } + _herdr_pane_id_ok "$id" || { printf '%s\tunknown:invalid_pane_id\n' "$instance"; return 2; } + pane_json="$(herdr pane get "$id" 2>/dev/null)" || { printf '%s\tunknown:pane_query_failed\n' "$instance"; return 2; } + esc="$(printf '%s' "$pane_json" | sed "s/'/''/g")" + tid="$(sqlite3 :memory: "SELECT CASE WHEN json_type('$esc','\$.result.pane.terminal_id')='text' THEN json_extract('$esc','\$.result.pane.terminal_id') ELSE '' END" 2>/dev/null)" \ + || { printf '%s\tunknown:pane_response_invalid\n' "$instance"; return 2; } + [ -n "$tid" ] || { printf '%s\tunknown:terminal_id_missing\n' "$instance"; return 2; } + case "$tid" in *[[:cntrl:]]*|*[[:space:]]*) printf '%s\tunknown:terminal_id_malformed\n' "$instance"; return 2 ;; esac + printf '%s\t%s\n' "$instance" "$tid" + case "$instance" in unknown:*) return 2 ;; esac return 0 } diff --git a/scripts/drivers/terminals/plain/ops.sh b/scripts/drivers/terminals/plain/ops.sh index 7cc486c2..d09f1419 100644 --- a/scripts/drivers/terminals/plain/ops.sh +++ b/scripts/drivers/terminals/plain/ops.sh @@ -249,3 +249,6 @@ terminal_name() { _plain_unsupported "name"; } # has no answer -- rather than as a failure to retry. A stub that returned # "nothing found" would be indistinguishable from a pane whose processes we could # not read, and the two must not land in the same bucket (#1152). +# No pane, so nothing to fence a write against (#1152): the self-write path +# reads this as "unsupported here" and writes nothing. +terminal_fence() { printf 'n/a:unsupported\tn/a:unsupported\n'; return 3; } diff --git a/scripts/drivers/terminals/tmux/ops.sh b/scripts/drivers/terminals/tmux/ops.sh index ca0d4236..79d7e723 100644 --- a/scripts/drivers/terminals/tmux/ops.sh +++ b/scripts/drivers/terminals/tmux/ops.sh @@ -759,5 +759,22 @@ terminal_enumerate_panes() { done done [ -n "$err" ] && rm -f "$err" +} + +# Fence for a self-write (#1152): "\t". The instance is +# the socket the id names (tmux pane ids repeat across servers, one server per +# socket -- the #1051 shape); the terminal_id is the pane's shell pid, which a +# pane that was closed and recreated does not keep. Same contract and the same +# limit as the herdr op: a preflight check right before a mutation, not an atomic +# fence. +terminal_fence() { # + local id="$1" sock bare pid + terminal_id_ok "$id" || { printf 'unknown:invalid_pane_id\tunknown:invalid_pane_id\n'; return 2; } + sock="$(_tmux_sock_of "$id")"; bare="$(_tmux_bare_of "$id")" + [ -n "$sock" ] || sock="default" + pid="$(_tmux_do "$id" display-message -p -t "$bare" '#{pane_pid}' 2>/dev/null)" \ + || { printf '%s\tunknown:pane_query_failed\n' "$sock"; return 2; } + case "$pid" in ''|*[!0-9]*) printf '%s\tunknown:pane_pid_missing\n' "$sock"; return 2 ;; esac + printf '%s\tpane_pid=%s\n' "$sock" "$pid" return 0 } diff --git a/scripts/lib/actas-lock.sh b/scripts/lib/actas-lock.sh index 0d6cc52c..e38bdf3c 100644 --- a/scripts/lib/actas-lock.sh +++ b/scripts/lib/actas-lock.sh @@ -203,9 +203,24 @@ _actas_lock_verdict() { # # so this producer and actas_lock_observe cannot disagree about one file. _actas_lock_try_claim() { local team="$1" agent="$2" sid="$3" - local lock dir tmp _r _v _w verdict existing - lock="$(actas_lock_path "$team" "$agent")" - dir="$(_actas_lock_dir)" + _agmsg_lock_try_claim_at "$(actas_lock_path "$team" "$agent")" "$sid" +} + +# The same claim, addressed by LOCK PATH and OWNER TOKEN instead of by role. +# +# The actas lock is not the only exclusion in this tree that must survive a +# claimant dying at any instruction: a seat's own single-flight (self-write-lock.sh) +# needs the identical write-then-readback-then-link publish, the identical +# three-valued verdict, and the identical positive-dead-only reclaim. Copying the +# body would make two producers of the same three values that drift apart one +# review at a time (the shape _actas_lock_verdict exists to prevent). So the body +# lives here, once, keyed on a path; the role-keyed functions above and below +# compute their path and call in. The owner token is whatever agmsg_instance_alive +# can judge: a session id, or a composite . instance token. +_agmsg_lock_try_claim_at() { # + local lock="$1" sid="$2" + local dir tmp _r _v _w verdict existing + dir="${lock%/*}" mkdir -p "$dir" 2>/dev/null || true tmp="$(mktemp "$dir/.actas-claim.XXXXXX" 2>/dev/null)" || return 1 @@ -273,11 +288,17 @@ _actas_lock_try_claim() { # instead of inferring one from silence. (#983, review) actas_lock_claim() { local team="$1" agent="$2" sid="$3" - local attempts=0 result lock_path reclaim_dir _r _owner _alive_rc - lock_path="$(actas_lock_path "$team" "$agent")" - reclaim_dir="${lock_path}.reclaim.d" + agmsg_lock_claim_at "$(actas_lock_path "$team" "$agent")" "$sid" +} + +# The claim loop by LOCK PATH and OWNER TOKEN (see _agmsg_lock_try_claim_at for +# why the body is shared). Same output contract and exit codes as actas_lock_claim. +agmsg_lock_claim_at() { # + local lock_path="$1" sid="$2" + local attempts=0 result mutex mres _r _owner _alive_rc + mutex="$(_agmsg_lock_mutex_path "$lock_path")" while [ "$attempts" -lt 3 ]; do - if ! result="$(_actas_lock_try_claim "$team" "$agent" "$sid")"; then + if ! result="$(_agmsg_lock_try_claim_at "$lock_path" "$sid")"; then # mktemp failed, or the lock directory could not be made. Nothing was # claimed and nothing was learned about the holder. echo "unknown:claim_failed" @@ -298,30 +319,46 @@ actas_lock_claim() { # A's fresh lock -- the original blocker from #65 review finding 1, # and the same hazard the mv-only variant inherited. # - # Per-lock mutex via `mkdir` (atomic on POSIX). Re-check inside it: - # only remove the lock if its current owner is still dead. If a peer - # snuck a live owner in between our stale decision and the mutex, - # leave it -- the next try_claim observes it as held. - if mkdir "$reclaim_dir" 2>/dev/null; then - # Reclaim DELETES, so it needs three facts, not one: the read - # SUCCEEDED, an owner is actually there, and that owner is POSITIVELY - # dead. "could not read it" and "could not tell" are neither. (#983) - _r="$(_actas_lock_read_path "$lock_path")" - if [ "${_r%%$'\t'*}" = "ok" ]; then - _owner="${_r#*$'\t'}" - if [ -n "$_owner" ]; then - _alive_rc=0 - actas_lock_sid_alive "$_owner" || _alive_rc=$? - if [ "$_alive_rc" -eq 1 ]; then - rm -f "$lock_path" + # The mutex is a LOCK OF THE SAME KIND as the one it protects (an + # owner-bearing file published by _agmsg_lock_try_claim_at), not a bare + # `mkdir`. A directory has no owner: a reclaimer dying between mkdir and + # rmdir left it behind with nothing to say whose it was, and with no + # time-based reclaim every later claim spun three rounds into + # unknown:reclaim_contended -- the protected lock was crash-safe and + # the thing protecting it was not (review, 2026-09-11). With an owner + # in the mutex, a reclaimer that died mid-reclaim is found the same way + # a dead lock owner is: read, three-valued liveness, positive dead only. + mres="$(_agmsg_lock_mutex_take "$mutex" "$sid")" + case "$mres" in + ok) + # Reclaim DELETES, so it needs three facts, not one: the read + # SUCCEEDED, an owner is actually there, and that owner is POSITIVELY + # dead. "could not read it" and "could not tell" are neither. (#983) + _r="$(_actas_lock_read_path "$lock_path")" + if [ "${_r%%$'\t'*}" = "ok" ]; then + _owner="${_r#*$'\t'}" + if [ -n "$_owner" ]; then + _alive_rc=0 + agmsg_instance_alive "$_owner" || _alive_rc=$? + if [ "$_alive_rc" -eq 1 ]; then + rm -f "$lock_path" + fi fi fi - fi - rmdir "$reclaim_dir" 2>/dev/null - fi - # If mkdir failed, another caller is mid-reclaim. Loop without - # touching anything; the next try_claim sees whichever state they - # end up in (live -> held, or empty -> we ln-claim). + agmsg_lock_release_at "$mutex" "$sid" + ;; + held:*) + # Another reclaimer is alive and mid-reclaim, or a dead one was + # just cleared. Touch nothing; the next round sees the result. + ;; + unknown:*) + # The mutex's own state could not be established (unreadable, + # empty, liveness undecidable). Spinning would only repeat the + # read; say so and stop, so the caller shows it. + printf 'unknown:reclaim_mutex:%s\n' "${mres#unknown:}" + return 1 + ;; + esac attempts=$((attempts + 1)) continue ;; @@ -341,11 +378,123 @@ actas_lock_claim() { return 1 } +# Where the reclaim mutex for a lock lives: beside it, one file. +_agmsg_lock_mutex_path() { # + printf '%s.reclaim' "$1" +} + +# Take the reclaim mutex for . Prints exactly one of: +# ok held by us now; caller must agmsg_lock_release_at it +# held: not ours this round (a live reclaimer, a vanished or +# just-cleared mutex); caller loops, touching nothing +# unknown: the mutex's state could not be established +# The exit status is 0 on EVERY verdict: the line decides. A step here that +# printed its verdict and also returned 1 killed a `set -e` caller of the claim +# loop inside `mres=$(...)`, before any verdict reached stdout -- silence in the +# shape of a refusal (measured 2026-09-11: child rc 1, empty stdout). +# +# A dead reclaimer's mutex is cleared here, and that clearing is the one place +# a lock of this kind is removed without holding a mutex over IT. Regress has to +# stop somewhere; it stops with an atomic rename to a name only this claimant +# uses. `mv` of the mutex to `.dead.` succeeds for exactly one +# caller (the second finds no source), and the winner then owns the moved file +# exclusively: it is SETTLED there (_agmsg_lock_tomb_settle) -- deleted only if +# its owner is still positively dead, linked back otherwise. A caller dying +# between the rename and the settle leaves `.dead.` behind, and that +# is why every take starts by settling whatever tombstones exist: a tombstone is +# a mutex in transit, not garbage, and a claim that ignored it would link into +# the gap it left. +_agmsg_lock_mutex_take() { # + local mutex="$1" sid="$2" r tomb s t + # Tombstones first. Any of them is a displaced mutex whose fate was not yet + # decided; deciding it is the same routine as below. One that cannot be + # settled stops this claim with a named unknown instead of a gap. + for t in "$mutex".dead.*; do + [ -e "$t" ] || continue + s="$(_agmsg_lock_tomb_settle "$t" "$mutex")" + case "$s" in + settled:*) ;; + *) printf 'unknown:tombstone_%s\n' "${s#unsettled:}"; return 0 ;; + esac + done + r="$(_agmsg_lock_try_claim_at "$mutex" "$sid")" || { echo "unknown:mutex_claim_failed"; return 0; } + case "$r" in + ok) echo ok; return 0 ;; + held:*) printf '%s\n' "$r"; return 0 ;; + vanished) echo "held:vanished"; return 0 ;; + unknown:*) printf '%s\n' "$r"; return 0 ;; + stale) ;; + *) echo "unknown:mutex_unclassified"; return 0 ;; + esac + tomb="${mutex}.dead.$(_actas_lock_encode "$sid")" + if mv "$mutex" "$tomb" 2>/dev/null; then + s="$(_agmsg_lock_tomb_settle "$tomb" "$mutex")" + case "$s" in + settled:*) ;; + *) printf 'unknown:tombstone_%s\n' "${s#unsettled:}"; return 0 ;; + esac + fi + echo "held:reclaiming" + return 0 +} + +# Decide the fate of one tombstone (a mutex displaced by rename). Prints: +# settled:removed its owner is positively dead -> it is gone +# settled:restored linked back to -> the mutex is as it was +# settled:superseded a fresh mutex already sits at , read and +# confirmed there -> the tombstone is dropped +# unsettled: it is KEPT, and the caller must not treat the mutex slot +# as free: restore_failed (ln failed and the destination is +# absent -- ENOSPC, EIO, a permission), destination_unreadable +# (something is there and cannot be read) +# Exit status 0 on every verdict, for the same reason as _agmsg_lock_mutex_take. +# +# The restore is `ln`, never `mv`: a mutex somebody published into the gap must +# not be overwritten. And a failed `ln` is NOT read as "somebody did": that +# folds the benign failure (destination exists) with the destructive ones +# (nothing there, and the link could not be made), and the delete that followed +# removed the only inode of a mutex just judged undeletable. The destination is +# re-read instead, and only a mutex actually READ there licenses dropping the +# tombstone. (Review, 2026-09-11 -- the third "two failure kinds folded into +# one" of the day.) +_agmsg_lock_tomb_settle() { # + local tomb="$1" mutex="$2" _r _owner _alive_rc _d + _r="$(_actas_lock_read_path "$tomb")" + _owner="${_r#*$'\t'}" + _alive_rc=2 + if [ "${_r%%$'\t'*}" = "ok" ] && [ -n "$_owner" ]; then + _alive_rc=0 + agmsg_instance_alive "$_owner" || _alive_rc=$? + fi + if [ "$_alive_rc" -eq 1 ]; then + rm -f "$tomb" + echo "settled:removed" + return 0 + fi + if ln "$tomb" "$mutex" 2>/dev/null; then + rm -f "$tomb" + echo "settled:restored" + return 0 + fi + _d="$(_actas_lock_read_path "$mutex")" + case "${_d%%$'\t'*}" in + ok) rm -f "$tomb"; echo "settled:superseded"; return 0 ;; + unreadable) echo "unsettled:destination_unreadable"; return 0 ;; + *) echo "unsettled:restore_failed"; return 0 ;; + esac +} + # Release a lock if we own it. Idempotent. actas_lock_release() { local team="$1" agent="$2" sid="$3" - local lock _r - lock="$(actas_lock_path "$team" "$agent")" + agmsg_lock_release_at "$(actas_lock_path "$team" "$agent")" "$sid" +} + +# Release by LOCK PATH and OWNER TOKEN. Only an exact owner match deletes; a lock +# that is unreadable, empty, or someone else's is left exactly as found. +agmsg_lock_release_at() { # + local lock="$1" sid="$2" + local _r # This DELETES, so it needs a read that worked AND an owner that is positively # us. `[ -f ] || return 0` followed by comparing a possibly-empty owner landed # on the same behaviour by accident (an unreadable lock compares unequal to any diff --git a/scripts/lib/self-write-lock.sh b/scripts/lib/self-write-lock.sh new file mode 100644 index 00000000..1bb2ca6e --- /dev/null +++ b/scripts/lib/self-write-lock.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# self-write-lock.sh -- a seat's own single-flight lock around its self-writes. +# +# WHAT IT EXCLUDES. One seat, several processes that may all try to write the +# seat's identity cells (placement record, pane label, agent key, session name) +# at the same moment: a watcher that restarted while its predecessor was still +# mid-write, two watchers on one seat (measured), a request re-issued before the +# first one finished. Two writers interleaving on one seat queue two CLI commands +# into one pane and leave the cells half from each; the once-only property of a +# session rename (#1081) does not vanish when its stored mark goes, it moves to +# whoever writes -- and that is this lock. +# +# WHAT IT IS NOT. It is NOT the leader/seat exchange lock (run/self-fix.*). That +# one guards a protocol between two parties and may disappear with the protocol; +# this one guards one party against itself and stays. They share no name, no +# path and no API on purpose: deleting one must not delete the other, and a +# design that finds "a lock already exists" must not borrow this one for the +# other job. +# +# Lock file: $SKILL_DIR/run/self-write.__.lock +# Content : one line -- the owner token, the writer's composite instance id +# (.) as agmsg_instance_alive judges it. +# +# WHAT IT REUSES. The publish and reclaim are actas-lock.sh's, addressed by path +# (_agmsg_lock_try_claim_at / agmsg_lock_claim_at / agmsg_lock_release_at): the +# owner is written to a temp file, read back, and hard-linked into place, so a +# claimant that dies before the link leaves an orphan temp and NO lock -- the +# next claim links into the gap without any reclaim. A lock whose owner is +# POSITIVELY dead is reclaimed under a per-lock mutex after re-reading the owner; +# "could not read" and "could not tell" are never reclaimed and never treated as +# free. There is no time-based reclaim: a lock is held until released or until +# its owner is proven dead. +# +# WHAT A CALLER SEES. Three answers, never silence: +# agmsg_self_write_lock_acquire -> rc 0 "ok" +# rc 1 "busy:" someone alive holds it +# rc 2 "unknown:" could not establish who +# "busy" is a fact the caller must surface (the self-fix header's `none:busy`), +# because a dropped attempt and an attempt in progress look identical from the +# outside -- the shape that cost the most on 2026-09-11. + +# shellcheck disable=SC1091 +. "${SKILL_DIR:?SKILL_DIR must be set}/scripts/lib/actas-lock.sh" + +agmsg_self_write_lock_path() { # + local team="$1" agent="$2" + local t a; t="$(_actas_lock_encode "$team")"; a="$(_actas_lock_encode "$agent")" + printf '%s/self-write.%s__%s.lock' "$(_actas_lock_dir)" "$t" "$a" +} + +# Acquire for . Prints exactly one verdict line (see header). Re-acquiring +# a lock this owner already holds is "ok" (the shared core answers `mine`). +agmsg_self_write_lock_acquire() { # + local team="$1" agent="$2" owner="$3" result + [ -n "$owner" ] || { echo "unknown:owner_empty"; return 2; } + # The verdict LINE decides, not the exit status: the shared core prints one on + # every path (a silent failure there once read as success, #983), so the + # status is deliberately not consulted here. + result="$(agmsg_lock_claim_at "$(agmsg_self_write_lock_path "$team" "$agent")" "$owner" || true)" + case "$result" in + ok) echo ok; return 0 ;; + held:*) printf 'busy:%s\n' "${result#held:}"; return 1 ;; + unknown:*) printf '%s\n' "$result"; return 2 ;; + *) printf 'unknown:unclassified:%s\n' "$result"; return 2 ;; + esac +} + +# Release if, and only if, is the recorded owner. Idempotent; never +# touches a lock that is unreadable, empty, or held by another owner. +agmsg_self_write_lock_release() { # + agmsg_lock_release_at "$(agmsg_self_write_lock_path "$1" "$2")" "$3" +} + +# Is the recorded owner right now? rc 0 yes; rc 1 no (free, someone +# else, or empty); rc 2 could not read. Prints the verdict from the shared +# three-valued reader so a caller can show WHY, not only whether. +agmsg_self_write_lock_held_by() { # + local lock _r _v verdict + lock="$(agmsg_self_write_lock_path "$1" "$2")" + _r="$(_actas_lock_read_path "$lock")" + _v="$(_actas_lock_verdict "$3" "${_r%%$'\t'*}" "${_r#*$'\t'}")" + verdict="${_v%%$'\t'*}" + printf '%s\n' "$verdict" + case "$verdict" in + mine) return 0 ;; + unknown:*) return 2 ;; + *) return 1 ;; + esac +} diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh new file mode 100644 index 00000000..844a0e2c --- /dev/null +++ b/scripts/lib/self-write.sh @@ -0,0 +1,268 @@ +#!/usr/bin/env bash +# self-write.sh -- the ONE path by which a seat writes its own identity cells. +# +# #1152 inverted who writes a seat's identity. Before, three writers (spawn, +# `team --fix`, the seat's own hook) each wrote whichever seat they resolved, +# and every fix was an arbitration between them. Now one writer, the seat +# itself, writes ONLY its own cells, and nothing here reads another seat's state +# to decide a write. There is no "who is right" left to decide. +# +# WHERE THE PANE COMES FROM. Not from here. The seat does not derive or verify +# its pane: a leader sweeps the workspace and types `fix --pane P` into pane P, +# and the seat that receives it is in P by construction -- typing into P is the +# only way to reach P (koit, 2026-09-11). So arrives as an argument and is +# a LOCATION carried by the channel, never an identity: team and agent come from +# the seat's own actas, and no argument may name another seat. +# +# THE FENCE. Pane ids repeat across terminal instances (two herdr sessions both +# have a w1:p2; tmux has one id space per socket), so a pane id alone can name +# a live pane in another instance. Each write generation therefore takes a +# fence -- the driver's for the pane -- once, stores it +# in the placement record, and re-reads it right before EVERY later mutation in +# the generation; a difference in either half refuses the mutation, visibly. +# This is best-effort safety: a preflight check that minimises the window, not +# an atomic fence. The read and the keystroke are separate calls, so a pane +# closed and reused between them is not caught; a full fence needs the terminal +# to compare-and-type. A herdr restart changes every terminal_id, so a stored +# fence expires with the server: the record then refuses instead of writing into +# whatever now sits at that id, and the next sweep re-delivers. +# +# CELLS, in order, each independent: record (REQUIRED -- the only cell a seat's +# reachability runs through) / label / key / session (decorations: their failure +# is visible, and does not stop the record). The session cell types +# ` -` UNCONDITIONALLY, at most once per generation, +# when the pane's input is ready: there is no pre-read skip, no stored mark and +# no process flag, because every one of those was a stale value keyed to skip a +# needed rename (#1130's shape). The title before and after is observation only. +# +# EXCLUSION. The whole generation runs under the seat-local single-flight lock +# (self-write-lock.sh). A second writer on the same seat sees `none:busy` -- +# never a silent drop, because "did it, not fixed" and "doing it now" look the +# same from outside. +# +# OUTPUT. One line per fact, never silence: +# seat=/ sid= pane= or +# seat=... none:|bad_ref|no_driver|lock_unknown:|fence_unreadable:> +# seat=... unsupported: (plain: no pane exists) +# fence=: +# record attempt=> readback=|unavailable:|not_attempted> +# label attempt=|skipped:> readback=<...> +# key attempt= readback=<...> +# session attempt=|skipped:> readback=|mismatch:|unavailable:|not_attempted> +# policy= +# The same lines are written, last and atomically, to run/self-write-done.__. +# policy is decided HERE and only here: accepted = record verified; +# accepted_unverified = record written, readback unavailable; anything else in +# the record = repair_incomplete. Decorations never change it. + +: "${SKILL_DIR:?self-write.sh requires SKILL_DIR}" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/actas-lock.sh" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/self-write-lock.sh" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/registry-lock.sh" # agmsg_write_atomic +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/terminal-registry.sh" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/type-registry.sh" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/role-session.sh" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/team-status.sh" # agmsg_cli_session_observed + +agmsg_self_write_done_path() { # + local t a; t="$(_actas_lock_encode "$1")"; a="$(_actas_lock_encode "$2")" + printf '%s/self-write-done.%s__%s' "$(_actas_lock_dir)" "$t" "$a" +} + +# Accumulated report lines for one generation (printed as they happen, and +# written as the done file at the end). +_SW_LINES="" +_sw_say() { printf '%s\n' "$1"; _SW_LINES="${_SW_LINES}${1}"$'\n'; } + +# Read the fence for the pane. Sets _SW_F_INSTANCE / _SW_F_TID; returns the +# driver's rc (0 value, 2 unreadable, 3 unsupported), or 4 when the driver has no +# fence op at all. +_sw_fence_read() { # + local out rc=0 + _SW_F_INSTANCE=""; _SW_F_TID="" + declare -F terminal_fence >/dev/null 2>&1 || return 4 + out="$(terminal_fence "$1")" || rc=$? + _SW_F_INSTANCE="${out%%$'\t'*}"; _SW_F_TID="${out#*$'\t'}" + return "$rc" +} + +# Re-read the fence and compare with the stored pair. Prints the reason on a +# mismatch ("instance" / "terminal_id" / "unreadable:"), nothing when equal. +_sw_fence_check() { # + local rc=0 + _sw_fence_read "$1" || rc=$? + if [ "$rc" -ne 0 ]; then printf 'unreadable:%s\n' "${_SW_F_TID#unknown:}"; return 1; fi + [ "$_SW_F_INSTANCE" = "$2" ] || { echo instance; return 1; } + [ "$_SW_F_TID" = "$3" ] || { echo terminal_id; return 1; } + return 0 +} + +# Write the record cell. Prints "attempt=... readback=...". +_sw_cell_record() { # + local rec content back + rec="$(agmsg_spawn_path "$1" "$2")" + if [ -z "$4" ] || [ -z "$5" ]; then + printf 'attempt=failed:missing_fields readback=not_attempted\n'; return 0 + fi + content="$(printf '%s\t%s\t%s\tfence=%s' "$3" "$4" "$5" "$6")" + mkdir -p "${rec%/*}" 2>/dev/null || true + if ! agmsg_write_atomic "$rec" "$content" 2>/dev/null; then + printf 'attempt=failed:write readback=not_attempted\n'; return 0 + fi + if back="$(head -1 "$rec" 2>/dev/null)"; then + if [ "$back" = "$content" ]; then printf 'attempt=ok readback=verified\n' + else printf 'attempt=ok readback=mismatch:%s\n' "${back%%$'\t'*}"; fi + else + printf 'attempt=ok readback=unavailable:record_unreadable\n' + fi + return 0 +} + +# The label and key cells: one terminal_name call, two separate readbacks. +# Prints two lines: "label attempt=... readback=..." and "key ...". +_sw_cell_label_key() { # + local id="$1" team="$2" agent="$3" rc=0 attempt obs lab key exp_label exp_key + terminal_name "$id" "$team" "$agent" >/dev/null 2>&1 || rc=$? + if [ "$rc" -eq 0 ]; then attempt=ok; else attempt="failed:$rc"; fi + if declare -F _herdr_label >/dev/null 2>&1; then exp_label="$(_herdr_label "$team" "$agent")"; else exp_label="$team:$agent"; fi + if declare -F _herdr_internal_key >/dev/null 2>&1; then exp_key="$(_herdr_internal_key "$team" "$agent" 2>/dev/null || true)"; else exp_key=""; fi + if declare -F terminal_team_observe >/dev/null 2>&1 && obs="$(terminal_team_observe "$id" 2>/dev/null)"; then + lab="$(printf '%s' "$obs" | awk -F '\t' 'NR==1{print $2}')" + key="$(printf '%s' "$obs" | awk -F '\t' 'NR==1{print $3}')" + printf 'label attempt=%s readback=%s\n' "$attempt" "$(_sw_judge "$lab" "$exp_label")" + if [ -n "$exp_key" ]; then printf 'key attempt=%s readback=%s\n' "$attempt" "$(_sw_judge "$key" "$exp_key")" + else printf 'key attempt=%s readback=unavailable:no_expected_key\n' "$attempt"; fi + else + printf 'label attempt=%s readback=unavailable:observe_failed\n' "$attempt" + printf 'key attempt=%s readback=unavailable:observe_failed\n' "$attempt" + fi + return 0 +} + +# One observed value against one expected value -> a readback verdict. +_sw_judge() { # + case "$1" in + "$2") echo verified ;; + n/a:*|unknown:*) printf 'unavailable:%s\n' "$1" ;; + absent:*) printf 'mismatch:%s\n' "$1" ;; + *) printf 'mismatch:%s\n' "$1" ;; + esac +} + +_sw_title_now() { # -> observed session name or unknown:/n/a: + local obs title + if declare -F terminal_team_observe >/dev/null 2>&1 && obs="$(terminal_team_observe "$1" 2>/dev/null)"; then + title="$(printf '%s' "$obs" | awk -F '\t' 'NR==1{print $4}')" + agmsg_cli_session_observed "$2" "$title" "$1" 2>/dev/null + else + echo "unknown:observe_failed" + fi +} + +# The session cell. Prints "attempt=... readback=...". +_sw_cell_session() { # + local id="$1" team="$2" agent="$3" type="$4" rename_cmd cli ready rc=0 expected before after + rename_cmd="$(agmsg_type_get "$type" rename_cmd 2>/dev/null || true)" + [ -n "$rename_cmd" ] || { printf 'attempt=skipped:no_rename_cmd readback=not_attempted\n'; return 0; } + cli="$(agmsg_type_get "$type" cli 2>/dev/null || true)" + declare -F terminal_team_input_ready >/dev/null 2>&1 || { printf 'attempt=skipped:no_readiness_op readback=not_attempted\n'; return 0; } + ready="$(terminal_team_input_ready "$id" "$cli" 2>/dev/null)" || rc=$? + case "$rc" in + 0) ;; + 1) printf 'attempt=skipped:not_ready:%s readback=not_attempted\n' "${ready#not_ready:}"; return 0 ;; + *) printf 'attempt=skipped:readiness_unknown:%s readback=not_attempted\n' "${ready#unknown:}"; return 0 ;; + esac + expected="$team-$agent" + before="$(_sw_title_now "$id" "$type")" # a BASELINE for the delta, never a reason to skip + rc=0 + terminal_poke "$id" "$rename_cmd $expected" >/dev/null 2>&1 || rc=$? + if [ "$rc" -ne 0 ]; then printf 'attempt=failed:%s readback=not_attempted\n' "$rc"; return 0; fi + after="$(_sw_title_now "$id" "$type")" + case "$after" in + "$expected") + if [ "$before" = "$expected" ]; then printf 'attempt=ok readback=matched_no_delta\n' + else printf 'attempt=ok readback=verified\n'; fi ;; + n/a:*|unknown:*) printf 'attempt=ok readback=unavailable:%s\n' "$after" ;; + "$before") printf 'attempt=ok readback=unchanged:%s\n' "$after" ;; + *) printf 'attempt=ok readback=mismatch:%s\n' "$after" ;; + esac + return 0 +} + +# The entry. is the writer's instance token (the watcher's composite id). +# Exit: 0 a generation ran (policy line printed, done file written); 1 busy; +# 2 refused before any write (bad ref, no driver, lock unknown, fence unreadable); +# 3 unsupported here (plain). +agmsg_self_write() { # + local team="$1" agent="$2" ref="$3" owner="$4" + local term id head lockv fence_rc fence project type rec_line lk_lines sess_line policy + _SW_LINES="" + head="seat=$team/$agent sid=$owner pane=$ref" + [ -n "$team" ] && [ -n "$agent" ] && [ -n "$owner" ] || { _sw_say "seat=$team/$agent sid=$owner none:bad_identity"; return 2; } + if ! _agmsg_placement_split "$ref"; then _sw_say "$head none:bad_ref"; return 2; fi + term="$_AGMSG_PS_TERM"; id="$_AGMSG_PS_ID" + _agmsg_terminal_id_ok "$term" "$id" || { _sw_say "$head none:bad_ref"; return 2; } + agmsg_terminal_load "$term" 2>/dev/null || { _sw_say "$head none:no_driver:$term"; return 2; } + + lockv="$(agmsg_self_write_lock_acquire "$team" "$agent" "$owner")" + case "$lockv" in + ok) ;; + busy:*) _sw_say "$head none:$lockv"; return 1 ;; + *) _sw_say "$head none:lock_$lockv"; return 2 ;; + esac + + fence_rc=0 + _sw_fence_read "$id" || fence_rc=$? + case "$fence_rc" in + 0) ;; + 3) _sw_say "$head unsupported:${_SW_F_TID#n/a:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 3 ;; + 4) _sw_say "$head none:fence_unreadable:no_fence_op"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; + *) _sw_say "$head none:fence_unreadable:${_SW_F_TID#unknown:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; + esac + fence="$_SW_F_INSTANCE:$_SW_F_TID" + _sw_say "$head" + _sw_say "fence=$fence" + + project="$(agmsg_role_session_get "$team" "$agent" project 2>/dev/null || true)" + type="$(agmsg_role_session_get "$team" "$agent" type 2>/dev/null || true)" + + # record -- the required cell. Written on the fence just read; nothing between. + rec_line="$(_sw_cell_record "$team" "$agent" "$ref" "$project" "$type" "$fence")" + _sw_say "record $rec_line" + + # label + key -- fence first. + local why + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + lk_lines="$(_sw_cell_label_key "$id" "$team" "$agent")" + _sw_say "$(printf '%s' "$lk_lines" | sed -n 1p)" + _sw_say "$(printf '%s' "$lk_lines" | sed -n 2p)" + else + _sw_say "label attempt=skipped:fence_mismatch:$why readback=not_attempted" + _sw_say "key attempt=skipped:fence_mismatch:$why readback=not_attempted" + fi + + # session -- fence again: this one types into the pane. + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + sess_line="$(_sw_cell_session "$id" "$team" "$agent" "$type")" + _sw_say "session $sess_line" + else + _sw_say "session attempt=skipped:fence_mismatch:$why readback=not_attempted" + fi + + case "$rec_line" in + "attempt=ok readback=verified") policy=accepted ;; + "attempt=ok readback=unavailable:"*) policy=accepted_unverified ;; + *) policy=repair_incomplete ;; + esac + _sw_say "policy=$policy" + agmsg_write_atomic "$(agmsg_self_write_done_path "$team" "$agent")" "${_SW_LINES%$'\n'}" 2>/dev/null || true + agmsg_self_write_lock_release "$team" "$agent" "$owner" + return 0 +} diff --git a/scripts/lib/terminal-registry.sh b/scripts/lib/terminal-registry.sh index 66926dde..ef4059a9 100644 --- a/scripts/lib/terminal-registry.sh +++ b/scripts/lib/terminal-registry.sh @@ -200,7 +200,7 @@ EOF # what makes a missing op FAIL rather than silently borrow the previously loaded # driver's same-named function. _AGMSG_TERMINAL_REQUIRED="terminal_check terminal_describe terminal_detect terminal_spawn terminal_despawn terminal_pane_state terminal_peek terminal_poke terminal_where terminal_arrange terminal_name" -_AGMSG_TERMINAL_OPTIONAL="terminal_capability terminal_team_observe terminal_team_input_ready terminal_find_by_label terminal_label_of terminal_id_ok terminal_pane_process_observe terminal_enumerate_panes" +_AGMSG_TERMINAL_OPTIONAL="terminal_capability terminal_team_observe terminal_team_input_ready terminal_find_by_label terminal_label_of terminal_id_ok terminal_pane_process_observe terminal_enumerate_panes terminal_fence" # Resolve one capability for the terminal instance addressed by . # terminal.conf is the implementation ceiling: a runtime hook may narrow that diff --git a/tests/test_actas_lock.bats b/tests/test_actas_lock.bats index e917d2aa..e4640171 100644 --- a/tests/test_actas_lock.bats +++ b/tests/test_actas_lock.bats @@ -130,11 +130,14 @@ live_pid() { echo "$$"; } # slot is stale and is about to enter the cleanup. With the fix the # reclaim path now re-checks ownership *inside* this mutex, so even # if a peer made it through, sid-A's live lock would be respected. - local rd="$(actas_lock_path "T" "alice").reclaim.d" - mkdir "$rd" + # The mutex is an owner-bearing lock file (a bare directory had no owner + # and outlived a crashed reclaimer forever); the peer holding it is live. + # (One marker per pid in this harness, so the live peer is sid-A itself.) + local rd="$(actas_lock_path "T" "alice").reclaim" + echo "sid-A" > "$rd" run actas_lock_claim "T" "alice" "sid-B" - rmdir "$rd" + rm -f "$rd" [ "$status" -eq 1 ] [[ "$output" == "held:sid-A" ]] diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats new file mode 100644 index 00000000..be9afb53 --- /dev/null +++ b/tests/test_self_write.bats @@ -0,0 +1,236 @@ +#!/usr/bin/env bats +# The one path by which a seat writes its own identity cells (scripts/lib/self-write.sh). +# +# The pane arrives as an argument (the channel carries the location); the fence +# is read once, stored in the record, and re-read before each later mutation. +# Every test drives the REAL herdr driver against a fake `herdr` binary that +# answers pane get / agent list / agent get / agent rename / agent prompt from a +# fixture file the test can change mid-run, and logs every argv. + +load test_helper + +setup() { + setup_test_env + export SKILL_DIR="$TEST_SKILL_DIR" + export RUN_DIR="$SKILL_DIR/run"; mkdir -p "$RUN_DIR" + export FAKEBIN="$SKILL_DIR/fakebin"; mkdir -p "$FAKEBIN" + export ARGV_LOG="$SKILL_DIR/argv.log"; : > "$ARGV_LOG" + export FIX="$SKILL_DIR/fixture" + export PATH="$FAKEBIN:$PATH" + export HERDR_ENV=1 HERDR_SESSION=jugemu HERDR_PANE_ID=w1:pB + unset TMUX TMUX_PANE + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/self-write.sh" + # the seat's own registration: type and project come from here, never from args + agmsg_role_session_record T alice sid-me /proj/alice claude-code + ME="sid-me.$$"; printf '%s\n' "$ME" > "$RUN_DIR/cc-instance.$$" + _fixture terminal_id term_AAA title "◐ T-alice" label "" key "" status idle kind claude + _fake_herdr +} +teardown() { teardown_test_env; } + +# fixture: key value pairs -> one file the fake reads on every call +_fixture() { : > "$FIX"; while [ $# -ge 2 ]; do printf '%s=%s\n' "$1" "$2" >> "$FIX"; shift 2; done; } +_fx() { sed -n "s/^$1=//p" "$FIX" | head -1; } + +_fake_herdr() { + cat > "$FAKEBIN/herdr" <<'FAKE' +#!/usr/bin/env bash +{ printf 'herdr'; for a in "$@"; do printf ' [%s]' "$a"; done; printf '\n'; } >> "$ARGV_LOG" +fx() { sed -n "s/^$1=//p" "$FIX" | head -1; } +if [ "$1" = pane ] && [ "$2" = get ]; then + [ "$3" = "$(fx pane)" ] || [ -z "$(fx pane)" ] || { echo '{"error":"pane_not_found"}'; exit 1; } + printf '{"result":{"pane":{"agent_status":"%s","label":"%s","terminal_title":"%s","terminal_id":"%s"}}}\n' \ + "$(fx status)" "$(fx label)" "$(fx title)" "$(fx terminal_id)" +elif [ "$1" = agent ] && [ "$2" = list ]; then + if [ -n "$(fx key)" ]; then + printf '{"id":"1","result":{"type":"list","agents":[{"pane_id":"w1:pB","name":"%s"}]}}\n' "$(fx key)" + else + printf '{"id":"1","result":{"type":"list","agents":[]}}\n' + fi +elif [ "$1" = agent ] && [ "$2" = get ]; then + printf '{"result":{"agent":{"agent":"%s","agent_status":"%s"}}}\n' "$(fx kind)" "$(fx status)" +elif [ "$1" = pane ] && [ "$2" = rename ]; then + # the label lands: the fixture now shows it + sed -i '' -e "s/^label=.*/label=$4/" "$FIX" 2>/dev/null || sed -i "s/^label=.*/label=$4/" "$FIX" + exit 0 +elif [ "$1" = agent ] && [ "$2" = rename ]; then + sed -i '' -e "s/^key=.*/key=$4/" "$FIX" 2>/dev/null || sed -i "s/^key=.*/key=$4/" "$FIX" + exit 0 +elif [ "$1" = agent ] && [ "$2" = prompt ]; then + # a /rename lands in the title on the next read (glyph kept) + case "$4" in "/rename "*) sed -i '' -e "s/^title=.*/title=✳ ${4#/rename }/" "$FIX" 2>/dev/null || sed -i "s/^title=.*/title=✳ ${4#/rename }/" "$FIX" ;; esac + exit 0 +fi +exit 0 +FAKE + chmod +x "$FAKEBIN/herdr" +} + +_line() { printf '%s\n' "$output" | grep -E "^$1( |=)" | head -1; } +_rec() { cat "$(agmsg_spawn_path T alice)"; } + +# --- the accepted path ----------------------------------------------------------- + +@test "accepted: a fresh seat writes its record with the fence, names its pane, renames its session, and policy=accepted" { + # born under another name, so the rename is a visible DELTA (the already-named + # case is the next test) + _fixture terminal_id term_AAA title "◐ claude" label "" key "" status idle kind claude + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line seat)" = "seat=T/alice sid=$ME pane=herdr:w1:pB" ] + [ "$(_line fence)" = "fence=jugemu:term_AAA" ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + [ "$(_line label)" = "label attempt=ok readback=verified" ] + [ "$(_line key)" = "key attempt=ok readback=verified" ] + [ "$(_line session)" = "session attempt=ok readback=verified" ] + [ "$(_line policy)" = "policy=accepted" ] + # the record: ref, project, type, fence -- four TAB fields, nothing derived + [ "$(_rec)" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=jugemu:term_AAA')" ] + # exactly one keystroke, into our own pane, the rename command + [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 1 ] + grep -q 'herdr \[agent\] \[prompt\] \[w1:pB\] \[/rename T-alice\]' "$ARGV_LOG" + # the done file carries the same lines + diff <(printf '%s\n' "$output") "$(agmsg_self_write_done_path T alice)" + # the lock is released + [ ! -e "$(agmsg_self_write_lock_path T alice)" ] +} + +@test "accepted: a session already named gets ONE unconditional rename and reads matched_no_delta (no pre-read skip)" { + _fixture terminal_id term_AAA title "◐ T-alice" label "" key "" status idle kind claude + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line session)" = "session attempt=ok readback=matched_no_delta" ] + [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 1 ] +} + +# --- the record is the only required cell ----------------------------------------- + +@test "policy: label/key/session failures leave policy=accepted (decorations), visibly reported" { + _fixture terminal_id term_AAA title "◐ other" label "" key "" status busy kind claude + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + [ "$(_line session)" = "session attempt=skipped:not_ready:agent_status_busy readback=not_attempted" ] + [ "$(_line policy)" = "policy=accepted" ] + [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 0 ] +} + +@test "policy: a record whose fields are missing is repair_incomplete and writes no record (#1137)" { + rm -f "$(_agmsg_role_session_path T alice 2>/dev/null || echo /nonexistent)" + agmsg_role_session_record T alice sid-me "" "" + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line record)" = "record attempt=failed:missing_fields readback=not_attempted" ] + [ "$(_line policy)" = "policy=repair_incomplete" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +@test "policy: a record that was written but cannot be read back is accepted_unverified, never accepted and never repair_incomplete" { + # The readback is the only thing that fails: the write lands (the file is + # there with the right content), but the read of it errors. `head` is what + # the readback uses, and only for this file. + local rec; rec="$(agmsg_spawn_path T alice)" + head() { if [ "$2" = "$rec" ]; then return 1; fi; command head "$@"; } + run agmsg_self_write T alice herdr:w1:pB "$ME" + unset -f head + [ "$status" -eq 0 ] + [ "$(_line record)" = "record attempt=ok readback=unavailable:record_unreadable" ] + [ "$(_line policy)" = "policy=accepted_unverified" ] + [ "$(cat "$rec")" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=jugemu:term_AAA')" ] +} + +# --- the fence --------------------------------------------------------------------- + +@test "fence: a terminal_id that changes after the record refuses label/key and session, visibly, and the record keeps the fence it was written on" { + # the pane get answering the LABEL fence re-read sees a different terminal_id: + # model it by rewriting the fixture right after the record is written, i.e. at + # the first `agent list` call (which only the label/key readback makes) -- too + # late. Instead: make the fake flip terminal_id on the SECOND pane get. + cat >> "$FAKEBIN/herdr" <<'FAKE' +FAKE + # simplest faithful model: count pane gets in the argv log inside the fake + sed -i '' -e 's|^fx() { sed -n "s/^$1=//p" "$FIX" \| head -1; }$|fx() { if [ "$1" = terminal_id ] \&\& [ "$(grep -c "\\[pane\\] \\[get\\]" "$ARGV_LOG")" -gt 1 ]; then echo term_BBB; return; fi; sed -n "s/^$1=//p" "$FIX" \| head -1; }|' "$FAKEBIN/herdr" 2>/dev/null \ + || sed -i 's|^fx() { sed -n "s/^$1=//p" "$FIX" \| head -1; }$|fx() { if [ "$1" = terminal_id ] \&\& [ "$(grep -c "\\[pane\\] \\[get\\]" "$ARGV_LOG")" -gt 1 ]; then echo term_BBB; return; fi; sed -n "s/^$1=//p" "$FIX" \| head -1; }|' "$FAKEBIN/herdr" + grep -q 'term_BBB' "$FAKEBIN/herdr" + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line fence)" = "fence=jugemu:term_AAA" ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + [ "$(_line label)" = "label attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] + [ "$(_line key)" = "key attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] + [ "$(_line session)" = "session attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] + [ "$(_line policy)" = "policy=accepted" ] + [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 0 ] + [ "$(grep -c 'herdr \[pane\] \[rename\]' "$ARGV_LOG")" -eq 0 ] + case "$(_rec)" in *"fence=jugemu:term_AAA") : ;; *) false ;; esac +} + +@test "fence: an unreadable fence before any write refuses the whole generation and writes nothing" { + _fixture terminal_id "" title "x" label "" key "" status idle kind claude + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=$ME pane=herdr:w1:pB none:fence_unreadable:terminal_id_missing" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] + [ ! -e "$(agmsg_self_write_done_path T alice)" ] + [ ! -e "$(agmsg_self_write_lock_path T alice)" ] +} + +@test "fence: no session in the environment is an unreadable instance half, refused before any write" { + unset HERDR_SESSION + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 2 ] + case "$output" in *"none:fence_unreadable:"*) : ;; *) false ;; esac + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +# --- the entry refuses what is not a location of ours ------------------------------ + +@test "refuse: a ref outside the driver grammar writes nothing" { + run agmsg_self_write T alice "herdr:../../etc" "$ME" + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=$ME pane=herdr:../../etc none:bad_ref" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] + [ ! -s "$ARGV_LOG" ] +} + +@test "refuse: a plain ref is unsupported, not a failure, and writes nothing" { + run agmsg_self_write T alice "plain:-" "$ME" + [ "$status" -eq 3 ] + [ "$output" = "seat=T/alice sid=$ME pane=plain:- unsupported:unsupported" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +@test "refuse: an empty owner is refused" { + run agmsg_self_write T alice herdr:w1:pB "" + [ "$status" -eq 2 ] + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +# --- exclusion ------------------------------------------------------------------- + +@test "busy: a live writer on the same seat makes the second one say none:busy, and it writes nothing" { + local bpid other + sleep 30 & bpid=$! + other="sid-other.$bpid"; printf '%s\n' "$other" > "$RUN_DIR/cc-instance.$bpid" + agmsg_self_write_lock_acquire T alice "$other" >/dev/null + run agmsg_self_write T alice herdr:w1:pB "$ME" + kill "$bpid" 2>/dev/null; wait "$bpid" 2>/dev/null || true + [ "$status" -eq 1 ] + [ "$output" = "seat=T/alice sid=$ME pane=herdr:w1:pB none:busy:$other" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] + [ ! -s "$ARGV_LOG" ] +} + +# --- what this file must never do ------------------------------------------------ + +@test "never: the writer touches no pane other than the one it was handed" { + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + refute grep -E '\[(w[0-9]+:p[^B]|w[0-9]+:pB[^]])' "$ARGV_LOG" + refute grep -E 'herdr \[pane\] \[list\]' "$ARGV_LOG" +} + +@test "never: the library holds no pane derivation, no search and no other-seat resolution, by name" { + refute grep -E 'terminal_find_by_label|terminal_detect|_agmsg_placement_claimed_by|_agmsg_terminal_resolve_by_label|HERDR_PANE_ID|TMUX_PANE' "$SKILL_DIR/scripts/lib/self-write.sh" +} diff --git a/tests/test_self_write_lock.bats b/tests/test_self_write_lock.bats new file mode 100644 index 00000000..c4362190 --- /dev/null +++ b/tests/test_self_write_lock.bats @@ -0,0 +1,537 @@ +#!/usr/bin/env bats +# The seat-local single-flight lock (scripts/lib/self-write-lock.sh). +# +# Every test names the failure it exists to catch; each is meant to go red on +# exactly one mutation of the code (listed at the bottom of the file). + +load test_helper + +setup() { + setup_test_env + export SKILL_DIR="$TEST_SKILL_DIR" + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/self-write-lock.sh" + export RUN_DIR="$SKILL_DIR/run" + mkdir -p "$RUN_DIR" +} + +teardown() { teardown_test_env; } + +# A composite owner token whose pid is this test process: alive for the whole +# test, and its marker names the token, so agmsg_instance_alive answers 0. +me_token() { printf 'sid-me.%s' "$$"; } +mark_alive() { printf '%s\n' "$1" > "$RUN_DIR/cc-instance.${1##*.}"; } + +# A composite token whose pid is positively dead: a child that has already +# been reaped. Its marker names the token, so only the pid decides. +dead_token() { + local pid + ( : ) & pid=$! + wait "$pid" 2>/dev/null || true + printf 'sid-gone.%s' "$pid" +} + +# --- path: its own name and place ------------------------------------------ + +@test "path: lives under run/ as self-write.__.lock, encoded" { + local p; p="$(agmsg_self_write_lock_path "te am" "al/ice")" + [ "$p" = "$RUN_DIR/self-write.te%20am__al%2Fice.lock" ] +} + +@test "path: never the actas lock and never the leader lock for the same seat" { + local p a; p="$(agmsg_self_write_lock_path T alice)"; a="$(actas_lock_path T alice)" + [ "$p" != "$a" ] + case "$p" in *self-fix.*) false ;; esac + case "$p" in *self-write.T__alice.lock) : ;; *) false ;; esac +} + +# --- acquire: the plain cases ------------------------------------------------ + +@test "acquire: a free lock is taken and records the owner" { + local tok; tok="$(me_token)"; mark_alive "$tok" + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] +} + +@test "acquire: the holder re-acquiring its own lock is ok, not busy" { + local tok; tok="$(me_token)"; mark_alive "$tok" + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] +} + +@test "acquire: an empty owner is refused as unknown, and nothing is written" { + run agmsg_self_write_lock_acquire T alice "" + [ "$status" -eq 2 ] + [ "$output" = unknown:owner_empty ] + [ ! -e "$(agmsg_self_write_lock_path T alice)" ] +} + +# --- control 1: another live process holds it -> busy, visible ---------------- + +@test "control other-process: a live holder makes the second claimant busy, naming the holder" { + local tok; tok="$(me_token)"; mark_alive "$tok" + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null + run agmsg_self_write_lock_acquire T alice "sid-other.$$" + [ "$status" -eq 1 ] + [ "$output" = "busy:$tok" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] +} + +@test "control other-process: busy clears once the holder releases (sequential race)" { + # Two claimants need two live pids: one marker per pid is the whole point of + # the composite token (a second token on the same pid IS a pid reuse). + local a b bpid + a="$(me_token)"; mark_alive "$a" + sleep 30 & bpid=$! + b="sid-b.$bpid"; mark_alive "$b" + agmsg_self_write_lock_acquire T alice "$a" >/dev/null + run agmsg_self_write_lock_acquire T alice "$b" + [ "$status" -eq 1 ] + [ "$output" = "busy:$a" ] + agmsg_self_write_lock_release T alice "$a" + run agmsg_self_write_lock_acquire T alice "$b" + kill "$bpid" 2>/dev/null; wait "$bpid" 2>/dev/null || true + [ "$status" -eq 0 ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$b" ] +} + +# --- control 2: PID reuse -> the marker disagrees -> dead -> reclaimed --------- + +@test "control pid-reuse: a live pid whose marker names another instance is dead, and the lock is reclaimed" { + local stale="sid-old.$$" + # The pid is alive (it is us) but the marker says this pid is now someone else. + printf 'sid-new.%s\n' "$$" > "$RUN_DIR/cc-instance.$$" + printf '%s\n' "$stale" > "$(agmsg_self_write_lock_path T alice)" + run agmsg_self_write_lock_acquire T alice "sid-new.$$" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "sid-new.$$" ] +} + +# --- control 3: a crash BEFORE publish leaves an orphan temp, not a lock ------- + +@test "control orphan-temp: a claimant that died before linking leaves no lock; the next claim succeeds without reclaim and the orphan is left alone" { + local tok orphan; tok="$(me_token)"; mark_alive "$tok" + orphan="$RUN_DIR/.actas-claim.orphan" + printf 'sid-crashed.%s\n' 999999 > "$orphan" + [ ! -e "$(agmsg_self_write_lock_path T alice)" ] + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ -e "$orphan" ] # temp GC is a separate fact + [ "$(cat "$orphan")" = "sid-crashed.999999" ] +} + +# --- control 4: an EMPTY lock on disk is a torn write, never free -------------- + +@test "control write-empty: an empty lock file is unknown, is not taken, and is not deleted" { + local tok; tok="$(me_token)"; mark_alive "$tok" + : > "$(agmsg_self_write_lock_path T alice)" + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:owner_empty ] + [ -e "$(agmsg_self_write_lock_path T alice)" ] + [ ! -s "$(agmsg_self_write_lock_path T alice)" ] +} + +# --- control 5: a crash MID-WRITE leaves a lock whose owner is dead ------------ + +@test "control crash-mid-cell: a lock held by a positively dead owner is reclaimed by the next claimant" { + local gone tok; gone="$(dead_token)"; mark_alive "$gone" + tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] +} + +@test "control crash-mid-cell: an owner whose liveness cannot be decided is NOT reclaimed" { + local tok; tok="$(me_token)"; mark_alive "$tok" + printf 'sid-x.%s\n' 424242 > "$(agmsg_self_write_lock_path T alice)" + agmsg_instance_alive() { return 2; } # cannot tell + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:liveness_undecidable ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "sid-x.424242" ] +} + +@test "control crash-mid-cell: a dead verdict that turns undecidable under the reclaim mutex does NOT delete the lock" { + # The reclaim re-checks the owner INSIDE the mutex, and that re-check is only + # reached after the first read already said dead. A test in which liveness is + # undecidable from the start never arrives there (the claim refuses one step + # earlier), so the re-check's own guard can only be seen shut by a liveness + # that FLIPS: dead on the first read, undecidable on the second. Mutating the + # re-check to reclaim on anything-but-alive reddens exactly this test. + local tok; tok="$(me_token)"; mark_alive "$tok" + printf 'sid-x.%s\n' 424242 > "$(agmsg_self_write_lock_path T alice)" + : > "$RUN_DIR/alive-calls" + agmsg_instance_alive() { + printf 'x' >> "$RUN_DIR/alive-calls" + [ "$(wc -c < "$RUN_DIR/alive-calls")" -le 1 ] && return 1 # first read: dead + return 2 # re-check: cannot tell + } + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:liveness_undecidable ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "sid-x.424242" ] + [ "$(wc -c < "$RUN_DIR/alive-calls")" -ge 2 ] # the re-check RAN +} + +# --- control 7: the reclaim MUTEX is as crash-safe as the lock it protects ------ +# +# Every case below starts from a main lock whose owner is positively dead (so the +# claim enters the reclaim path) and varies only the state of the mutex file +# beside it. Before this, the mutex was a bare directory: a reclaimer dying +# between mkdir and rmdir left it forever and every later claim spun into +# unknown:reclaim_contended -- with no time-based reclaim, a permanent stop. + +mutex_of() { printf '%s.reclaim' "$(agmsg_self_write_lock_path "$1" "$2")"; } + +@test "mutex crash-live: a reclaimer that is alive keeps its mutex; the claimant is contended, and the dead main lock is untouched" { + local gone tok bpid other + gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + sleep 30 & bpid=$! + other="sid-reclaimer.$bpid"; mark_alive "$other" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf '%s\n' "$other" > "$(mutex_of T alice)" + run agmsg_self_write_lock_acquire T alice "$tok" + kill "$bpid" 2>/dev/null; wait "$bpid" 2>/dev/null || true + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_contended ] + [ "$(cat "$(mutex_of T alice)")" = "$other" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] +} + +@test "mutex crash-dead: a reclaimer that died mid-reclaim leaves a mutex that is cleared, and the claim then succeeds" { + local gone deadmx tok + gone="$(dead_token)"; mark_alive "$gone" + deadmx="$(dead_token)"; mark_alive "$deadmx" + tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf '%s\n' "$deadmx" > "$(mutex_of T alice)" + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ ! -e "$(mutex_of T alice)" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] + # nothing else was left beside the lock (no tombstone survives a completed clear) + [ -z "$(ls "$RUN_DIR" | grep -F "$(basename "$(mutex_of T alice)").dead." || true)" ] +} + +@test "mutex crash-between-take-and-release: a reclaimer killed while HOLDING the mutex (published by the real path) does not stop later claims" { + # The path tl named: die after taking the mutex and before releasing it. The + # mutex here is produced by the code under test, not written by hand: a child + # process takes it through _agmsg_lock_mutex_take, reports, and is then + # killed mid-hold. Its token dies with it, so the next claimant must find a + # dead-owned mutex, clear it, reclaim the dead main lock, and succeed. + local gone tok mx child ctok + gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + mx="$(mutex_of T alice)" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + ( ctok="sid-child.$BASHPID"; printf '%s\n' "$ctok" > "$RUN_DIR/cc-instance.$BASHPID" + _agmsg_lock_mutex_take "$mx" "$ctok" > "$RUN_DIR/child-take" + sleep 30 ) & + child=$! + local i=0; while [ ! -s "$RUN_DIR/child-take" ] && [ "$i" -lt 100 ]; do sleep 0.05; i=$((i+1)); done + [ "$(cat "$RUN_DIR/child-take")" = ok ] + kill -9 "$child" 2>/dev/null; wait "$child" 2>/dev/null || true + pkill -P "$child" 2>/dev/null || true + [ -e "$mx" ] # died holding it + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ ! -e "$mx" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] +} + +@test "mutex empty: an empty mutex file is a torn write -> unknown, kept, main lock kept" { + local gone tok; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + : > "$(mutex_of T alice)" + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:owner_empty ] + [ -e "$(mutex_of T alice)" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] +} + +@test "mutex unreadable: an unreadable mutex is unknown, kept, and the main lock is kept" { + [ "$(id -u)" -eq 0 ] && skip "chmod 000 is ineffective as root" + local gone tok mx; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + mx="$(mutex_of T alice)"; printf 'sid-someone.%s\n' 424242 > "$mx"; chmod 000 "$mx" + run agmsg_self_write_lock_acquire T alice "$tok" + local kept=0; [ -e "$mx" ] && kept=1 + chmod 644 "$mx" 2>/dev/null || true + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:lock_unreadable ] + [ "$kept" -eq 1 ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] +} + +@test "mutex undecidable: a mutex whose owner's liveness cannot be judged is unknown and kept" { + local gone tok; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$(mutex_of T alice)" + _real_alive="$(declare -f agmsg_instance_alive)" + agmsg_instance_alive() { case "$1" in sid-mx.*) return 2 ;; esac; eval "${_real_alive/agmsg_instance_alive/_orig_alive}"; _orig_alive "$1"; } + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:liveness_undecidable ] + [ "$(cat "$(mutex_of T alice)")" = "sid-mx.424242" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] +} + +@test "mutex pid-reuse: a mutex whose owner pid is alive but whose marker names another instance is dead -> cleared" { + local gone tok; gone="$(dead_token)"; mark_alive "$gone" + tok="sid-new.$$"; printf '%s\n' "$tok" > "$RUN_DIR/cc-instance.$$" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-old.%s\n' "$$" > "$(mutex_of T alice)" # same pid, different instance + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ ! -e "$(mutex_of T alice)" ] +} + +@test "mutex flip: a mutex owner judged dead on the first read but undecidable on the tombstone re-read is restored, not deleted" { + # The one deletion that holds no mutex over itself is the clearing of a dead + # reclaimer's mutex. Its guard is the re-read of the exclusively renamed file: + # if the owner is no longer POSITIVELY dead there, the file goes back by ln. + # The flip lands on "cannot tell", the answer a guard written as "anything but + # alive" would wrongly treat as dead (the same mutation as control 5's). + local gone tok; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$(mutex_of T alice)" + : > "$RUN_DIR/mx-calls" + _real_alive="$(declare -f agmsg_instance_alive)" + eval "${_real_alive/agmsg_instance_alive/_orig_alive}" + agmsg_instance_alive() { + case "$1" in + sid-mx.*) printf 'x' >> "$RUN_DIR/mx-calls" + [ "$(wc -c < "$RUN_DIR/mx-calls")" -le 1 ] && return 1 # first: dead + return 2 ;; # re-read: cannot tell + esac + _orig_alive "$1" + } + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$(cat "$(mutex_of T alice)")" = "sid-mx.424242" ] # restored + [ "$(wc -c < "$RUN_DIR/mx-calls")" -ge 2 ] # the re-read RAN + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] # main lock untouched + [ "$status" -ne 0 ] +} + +# --- control 8: a tombstone (a mutex in transit) is never lost -------------------- +# +# The restore of a displaced mutex is `ln tomb mutex`. Its failure has two kinds +# that must not be folded: benign (a fresh mutex already sits there) and +# destructive (nothing is there and the link could not be made). Each case below +# injects one failure into `ln` and asserts which files survive. The starting +# state is always: dead main lock, and a mutex whose owner reads dead on the +# first check and NOT positively dead on the tombstone re-read, so the code +# reaches the restore. + +tomb_of() { printf '%s.dead.%s' "$(mutex_of "$1" "$2")" "$(_actas_lock_encode "$3")"; } + +_arm_flip() { # first liveness read of sid-mx.* = dead, later ones = cannot tell + : > "$RUN_DIR/mx-calls" + _real_alive="$(declare -f agmsg_instance_alive)" + eval "${_real_alive/agmsg_instance_alive/_orig_alive}" + agmsg_instance_alive() { + case "$1" in + sid-mx.*) printf 'x' >> "$RUN_DIR/mx-calls" + [ "$(wc -c < "$RUN_DIR/mx-calls")" -le 1 ] && return 1 + return 2 ;; + esac + _orig_alive "$1" + } +} + +@test "tombstone (a) restore ln fails with the destination ABSENT: the tombstone is kept, the claim is unknown, and later claims stay unknown (no gap)" { + local gone tok mx; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + mx="$(mutex_of T alice)" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$mx" + _arm_flip + # The fault is injected on the RESTORE link only (source = a tombstone); the + # mutex's own publish link must keep working or the injection tests the + # wrong ln (it did, first time round). + ln() { case "$1" in *.dead.*) return 1 ;; esac; command ln "$@"; } # ENOSPC/EIO stand-in + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:tombstone_restore_failed ] + [ -e "$(tomb_of T alice "$tok")" ] + [ "$(cat "$(tomb_of T alice "$tok")")" = "sid-mx.424242" ] + [ ! -e "$mx" ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$gone" ] # main lock untouched + # a later claimant, ln still broken, must not read the empty mutex slot as free + run agmsg_self_write_lock_acquire T alice "sid-later.$$" + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:tombstone_restore_failed ] + [ -e "$(tomb_of T alice "$tok")" ] +} + +@test "tombstone (a2) once ln works again, the kept tombstone is restored by the next claim and the mutex is back as it was" { + local gone tok mx; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + mx="$(mutex_of T alice)" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$mx" + _arm_flip + ln() { case "$1" in *.dead.*) return 1 ;; esac; command ln "$@"; } + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null || true + [ -e "$(tomb_of T alice "$tok")" ] + unset -f ln + run agmsg_self_write_lock_acquire T alice "sid-later.$$" + [ ! -e "$(tomb_of T alice "$tok")" ] + [ "$(cat "$mx")" = "sid-mx.424242" ] # restored, not deleted + [ "$status" -ne 0 ] # its owner is still undecidable +} + +@test "tombstone (b) restore ln fails because a FRESH mutex already sits there: the fresh one is kept and the tombstone dropped" { + local gone tok mx bpid fresh; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + mx="$(mutex_of T alice)" + sleep 30 & bpid=$! + fresh="sid-fresh.$bpid"; mark_alive "$fresh" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$mx" + _arm_flip + # the gap is filled by a peer between our rename and our ln: model it inside ln + ln() { case "$1" in *.dead.*) printf '%s\n' "$fresh" > "$mx"; return 1 ;; esac; command ln "$@"; } + run agmsg_self_write_lock_acquire T alice "$tok" + kill "$bpid" 2>/dev/null; wait "$bpid" 2>/dev/null || true + [ ! -e "$(tomb_of T alice "$tok")" ] + [ "$(cat "$mx")" = "$fresh" ] + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_contended ] # the fresh holder is live +} + +@test "tombstone (c) restore ln fails and the destination is UNREADABLE: neither file is deleted, the claim is unknown" { + [ "$(id -u)" -eq 0 ] && skip "chmod 000 is ineffective as root" + local gone tok mx; gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + mx="$(mutex_of T alice)" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf 'sid-mx.%s\n' 424242 > "$mx" + _arm_flip + ln() { case "$1" in *.dead.*) printf 'x\n' > "$mx"; chmod 000 "$mx"; return 1 ;; esac; command ln "$@"; } + run agmsg_self_write_lock_acquire T alice "$tok" + local tomb_kept=0 mx_kept=0 + [ -e "$(tomb_of T alice "$tok")" ] && tomb_kept=1 + [ -e "$mx" ] && mx_kept=1 + chmod 644 "$mx" 2>/dev/null || true + [ "$status" -eq 2 ] + [ "$output" = unknown:reclaim_mutex:tombstone_destination_unreadable ] + [ "$tomb_kept" -eq 1 ] + [ "$mx_kept" -eq 1 ] +} + +@test "tombstone (d) a claimant that died between the rename and the settle leaves a tombstone; if its owner is dead it is removed by the next claim, which then succeeds" { + local gone deadmx tok; gone="$(dead_token)"; mark_alive "$gone" + deadmx="$(dead_token)"; mark_alive "$deadmx"; tok="$(me_token)"; mark_alive "$tok" + printf '%s\n' "$gone" > "$(agmsg_self_write_lock_path T alice)" + printf '%s\n' "$deadmx" > "$(tomb_of T alice "sid-crashed.999999")" # left by a dead displacer + run agmsg_self_write_lock_acquire T alice "$tok" + [ "$status" -eq 0 ] + [ "$output" = ok ] + [ ! -e "$(tomb_of T alice "sid-crashed.999999")" ] + [ ! -e "$(mutex_of T alice)" ] +} + +# --- control 6: release only on an EXACT owner match -------------------------- + +@test "control release-exact: releasing with another owner token leaves the lock as found" { + local tok; tok="$(me_token)"; mark_alive "$tok" + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null + agmsg_self_write_lock_release T alice "sid-other.$$" + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] + agmsg_self_write_lock_release T alice "$tok" + [ ! -e "$(agmsg_self_write_lock_path T alice)" ] +} + +@test "control release-exact: an unreadable lock is not deleted by release" { + [ "$(id -u)" -eq 0 ] && skip "chmod 000 is ineffective as root" + local tok lock; tok="$(me_token)"; mark_alive "$tok" + lock="$(agmsg_self_write_lock_path T alice)" + printf '%s\n' "$tok" > "$lock" + chmod 000 "$lock" + agmsg_self_write_lock_release T alice "$tok" + local kept=0; [ -e "$lock" ] && kept=1 + chmod 644 "$lock" 2>/dev/null || true + [ "$kept" -eq 1 ] +} + +@test "control release-exact: release of an empty lock leaves it (a torn write is not ours to erase)" { + local tok; tok="$(me_token)" + : > "$(agmsg_self_write_lock_path T alice)" + agmsg_self_write_lock_release T alice "$tok" + [ -e "$(agmsg_self_write_lock_path T alice)" ] +} + +# --- held_by: the three answers ----------------------------------------------- + +@test "held_by: mine / free / other / unreadable are four distinct, named answers" { + [ "$(id -u)" -eq 0 ] && skip "chmod 000 is ineffective as root" + local tok lock; tok="$(me_token)"; mark_alive "$tok" + lock="$(agmsg_self_write_lock_path T alice)" + run agmsg_self_write_lock_held_by T alice "$tok" + [ "$status" -eq 1 ]; [ "$output" = free ] + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null + run agmsg_self_write_lock_held_by T alice "$tok" + [ "$status" -eq 0 ]; [ "$output" = mine ] + run agmsg_self_write_lock_held_by T alice "sid-other.$$" + [ "$status" -eq 1 ]; [ "$output" = "other:$tok" ] + chmod 000 "$lock" + run agmsg_self_write_lock_held_by T alice "$tok" + chmod 644 "$lock" 2>/dev/null || true + [ "$status" -eq 2 ]; [ "$output" = unknown:lock_unreadable ] +} + +# --- the shared core: the actas lock still lands in its own file -------------- + +@test "sharing: the actas lock and the self-write lock for one seat are two files, each with its own owner" { + local tok; tok="$(me_token)"; mark_alive "$tok" + printf 'sid-me\n' > "$RUN_DIR/cc-instance.$$" # bare sid for the actas side + actas_lock_claim T alice sid-me >/dev/null + mark_alive "$tok" + agmsg_self_write_lock_acquire T alice "$tok" >/dev/null + [ "$(cat "$(actas_lock_path T alice)")" = sid-me ] + [ "$(cat "$(agmsg_self_write_lock_path T alice)")" = "$tok" ] +} + +# Mutations, one test each (run by hand, never committed): +# M1 in agmsg_self_write_lock_acquire map held:* to ok -> control other-process red +# M2 in agmsg_lock_claim_at reclaim on _alive_rc != 0 -> liveness-undecided red +# M3 in agmsg_lock_release_at drop the owner comparison -> release-exact (other owner) red +# M4 in _actas_lock_verdict answer free for an empty owner -> control write-empty red +# M5 in agmsg_self_write_lock_path drop the "self-write." prefix -> path tests red +# M6 in _agmsg_lock_try_claim_at skip the temp readback -> actas axis-6 test red (shared core) + +# --- errexit: the verdict LINE must reach stdout under a set -e caller ------------ +# +# The shared loop's internal steps print a verdict and used to also return 1 +# (held / unknown). A `set -e` caller doing `v=$(agmsg_lock_claim_at ...)` as a +# bare statement then died INSIDE the loop, before any verdict was printed -- +# stdout empty, exit 1: silence wearing the shape of a refusal (control measured +# 2026-09-11: child_rc=1, stdout=[]). `run` suspends errexit, so this cannot be +# seen through `run`: the call is made as a bare statement in a real `bash -e` +# child, and the signal is whether the verdict reached stdout. +@test "errexit: a bare set -e call of the shared claim loop with a live-held mutex still prints its verdict" { + local gone tok bpid other lockp + gone="$(dead_token)"; mark_alive "$gone"; tok="$(me_token)"; mark_alive "$tok" + sleep 30 & bpid=$! + other="sid-reclaimer.$bpid"; mark_alive "$other" + lockp="$(agmsg_self_write_lock_path T alice)" + printf '%s\n' "$gone" > "$lockp" + printf '%s\n' "$other" > "$(mutex_of T alice)" + # A BARE statement, not `v=$(...)`: the loop's own exit status is 1 for + # "not claimed" by contract, so a set -e child dies right after it either + # way. What distinguishes the two worlds is whether the verdict line was + # printed BEFORE that death -- so the loop's stdout is the child's stdout. + bash -e -c '. "$1/scripts/lib/actas-lock.sh"; agmsg_lock_claim_at "$2" "$3"; echo unreachable' \ + _ "$SKILL_DIR" "$lockp" "$tok" > "$RUN_DIR/errexit-out" 2>/dev/null || true + kill "$bpid" 2>/dev/null; wait "$bpid" 2>/dev/null || true + [ "$(cat "$RUN_DIR/errexit-out")" = "unknown:reclaim_contended" ] +} From d3d344d2c7a62ffdb402f758317494e71401e7ad Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 11 Sep 2026 10:02:13 -0700 Subject: [PATCH 02/12] self-write: guard every SKILL_DIR read inline, as the static checker requires The library opened with : "${SKILL_DIR:?}" and then read $SKILL_DIR bare seven times. The unguarded-env-reads checker does not treat that opening line as a guard, so the PR grew the baseline by seven. Each read now carries the guard itself, the form the sibling lock library already used. --- scripts/lib/self-write.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index 844a0e2c..119f0f67 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -57,19 +57,19 @@ : "${SKILL_DIR:?self-write.sh requires SKILL_DIR}" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/actas-lock.sh" +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/actas-lock.sh" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/self-write-lock.sh" +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/self-write-lock.sh" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/registry-lock.sh" # agmsg_write_atomic +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/registry-lock.sh" # agmsg_write_atomic # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/terminal-registry.sh" +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/terminal-registry.sh" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/type-registry.sh" +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/type-registry.sh" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/role-session.sh" +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/role-session.sh" # shellcheck disable=SC1091 -. "$SKILL_DIR/scripts/lib/team-status.sh" # agmsg_cli_session_observed +. "${SKILL_DIR:?self-write.sh requires SKILL_DIR}/scripts/lib/team-status.sh" # agmsg_cli_session_observed agmsg_self_write_done_path() { # local t a; t="$(_actas_lock_encode "$1")"; a="$(_actas_lock_encode "$2")" From f1cf26206192009839cd1589344f09e966716f3e Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 11 Sep 2026 18:20:17 -0700 Subject: [PATCH 03/12] self-write: name the herdr fence instance by socket path, as the sweep enumeration does The fence's instance half was the herdr session NAME from HERDR_SESSION. The sweep that will hand a seat its pane enumerates instances by SOCKET PATH (one running session per socket, reached with HERDR_SOCKET_PATH=), so a location the sweep delivers and the fence the seat stores would not compare as equal strings. The herdr driver now reports HERDR_SOCKET_PATH as the instance, refusing an empty or malformed one visibly, and the stored fence is split on its LAST colon so a path instance is safe. Tests drive a socket-path instance. --- scripts/drivers/terminals/herdr/ops.sh | 14 +++++++++----- scripts/lib/self-write.sh | 6 ++++-- tests/test_self_write.bats | 17 +++++++++-------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/scripts/drivers/terminals/herdr/ops.sh b/scripts/drivers/terminals/herdr/ops.sh index 3e5b355f..27edea9b 100644 --- a/scripts/drivers/terminals/herdr/ops.sh +++ b/scripts/drivers/terminals/herdr/ops.sh @@ -1333,9 +1333,12 @@ _herdr_panes_of() { # } # Fence for a self-write (#1152). Prints "\t" for one pane: -# the herdr SESSION this driver is talking to (pane ids repeat across sessions -- -# w1:p2 exists in both `jugemu` and `oma`, measured 2026-09-11) and the pane's -# server-side terminal_id (unique across sessions, 52 panes / 0 crossings; CHANGES +# the herdr instance this driver is talking to, named by its SOCKET PATH -- the +# same string the sweep's enumeration (terminal_enumerate_panes) uses for an +# instance, so a location the sweep hands a seat and the fence the seat stores +# compare as equal strings (pane ids repeat across instances -- w1:p2 exists in +# both `jugemu` and `oma`, measured 2026-09-11) -- and the pane's server-side +# terminal_id (unique across sessions, 52 panes / 0 crossings; CHANGES # across a herdr restart, so a stored fence expires with the server and a later # write refuses instead of landing in whatever now sits at that pane id). # Each half is either a value or a namespaced reason; the caller compares both @@ -1346,8 +1349,9 @@ _herdr_panes_of() { # # resolved in one session landing in another's live pane. terminal_fence() { # local id="$1" instance pane_json esc tid - instance="${HERDR_SESSION:-}" - [ -n "$instance" ] || instance="unknown:no_session_in_env" + instance="${HERDR_SOCKET_PATH:-}" + [ -n "$instance" ] || instance="unknown:no_socket_in_env" + case "$instance" in *:*|*[[:cntrl:]]*|*[[:space:]]*) instance="unknown:socket_path_malformed" ;; esac command -v herdr >/dev/null 2>&1 || { printf '%s\tunknown:terminal_unreachable\n' "$instance"; return 2; } _herdr_pane_id_ok "$id" || { printf '%s\tunknown:invalid_pane_id\n' "$instance"; return 2; } pane_json="$(herdr pane get "$id" 2>/dev/null)" || { printf '%s\tunknown:pane_query_failed\n' "$instance"; return 2; } diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index 119f0f67..f8f4d525 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -226,6 +226,8 @@ agmsg_self_write() { # 4) _sw_say "$head none:fence_unreadable:no_fence_op"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; *) _sw_say "$head none:fence_unreadable:${_SW_F_TID#unknown:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; esac + # instance:terminal_id -- the instance may be a socket PATH; the terminal_id + # never contains ':', so readers split on the LAST colon. fence="$_SW_F_INSTANCE:$_SW_F_TID" _sw_say "$head" _sw_say "fence=$fence" @@ -239,7 +241,7 @@ agmsg_self_write() { # # label + key -- fence first. local why - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence##*:}")"; then lk_lines="$(_sw_cell_label_key "$id" "$team" "$agent")" _sw_say "$(printf '%s' "$lk_lines" | sed -n 1p)" _sw_say "$(printf '%s' "$lk_lines" | sed -n 2p)" @@ -249,7 +251,7 @@ agmsg_self_write() { # fi # session -- fence again: this one types into the pane. - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence##*:}")"; then sess_line="$(_sw_cell_session "$id" "$team" "$agent" "$type")" _sw_say "session $sess_line" else diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats index be9afb53..bc6bb9fe 100644 --- a/tests/test_self_write.bats +++ b/tests/test_self_write.bats @@ -17,7 +17,8 @@ setup() { export ARGV_LOG="$SKILL_DIR/argv.log"; : > "$ARGV_LOG" export FIX="$SKILL_DIR/fixture" export PATH="$FAKEBIN:$PATH" - export HERDR_ENV=1 HERDR_SESSION=jugemu HERDR_PANE_ID=w1:pB + export HERDR_ENV=1 HERDR_SOCKET_PATH=/tmp/herdr/sessions/jugemu/herdr.sock HERDR_PANE_ID=w1:pB + unset HERDR_SESSION unset TMUX TMUX_PANE # shellcheck disable=SC1090 source "$SKILL_DIR/scripts/lib/self-write.sh" @@ -79,14 +80,14 @@ _rec() { cat "$(agmsg_spawn_path T alice)"; } run agmsg_self_write T alice herdr:w1:pB "$ME" [ "$status" -eq 0 ] [ "$(_line seat)" = "seat=T/alice sid=$ME pane=herdr:w1:pB" ] - [ "$(_line fence)" = "fence=jugemu:term_AAA" ] + [ "$(_line fence)" = "fence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA" ] [ "$(_line record)" = "record attempt=ok readback=verified" ] [ "$(_line label)" = "label attempt=ok readback=verified" ] [ "$(_line key)" = "key attempt=ok readback=verified" ] [ "$(_line session)" = "session attempt=ok readback=verified" ] [ "$(_line policy)" = "policy=accepted" ] # the record: ref, project, type, fence -- four TAB fields, nothing derived - [ "$(_rec)" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=jugemu:term_AAA')" ] + [ "$(_rec)" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA')" ] # exactly one keystroke, into our own pane, the rename command [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 1 ] grep -q 'herdr \[agent\] \[prompt\] \[w1:pB\] \[/rename T-alice\]' "$ARGV_LOG" @@ -137,7 +138,7 @@ _rec() { cat "$(agmsg_spawn_path T alice)"; } [ "$status" -eq 0 ] [ "$(_line record)" = "record attempt=ok readback=unavailable:record_unreadable" ] [ "$(_line policy)" = "policy=accepted_unverified" ] - [ "$(cat "$rec")" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=jugemu:term_AAA')" ] + [ "$(cat "$rec")" = "$(printf 'herdr:w1:pB\t/proj/alice\tclaude-code\tfence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA')" ] } # --- the fence --------------------------------------------------------------------- @@ -155,7 +156,7 @@ FAKE grep -q 'term_BBB' "$FAKEBIN/herdr" run agmsg_self_write T alice herdr:w1:pB "$ME" [ "$status" -eq 0 ] - [ "$(_line fence)" = "fence=jugemu:term_AAA" ] + [ "$(_line fence)" = "fence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA" ] [ "$(_line record)" = "record attempt=ok readback=verified" ] [ "$(_line label)" = "label attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] [ "$(_line key)" = "key attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] @@ -163,7 +164,7 @@ FAKE [ "$(_line policy)" = "policy=accepted" ] [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 0 ] [ "$(grep -c 'herdr \[pane\] \[rename\]' "$ARGV_LOG")" -eq 0 ] - case "$(_rec)" in *"fence=jugemu:term_AAA") : ;; *) false ;; esac + case "$(_rec)" in *"fence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA") : ;; *) false ;; esac } @test "fence: an unreadable fence before any write refuses the whole generation and writes nothing" { @@ -176,8 +177,8 @@ FAKE [ ! -e "$(agmsg_self_write_lock_path T alice)" ] } -@test "fence: no session in the environment is an unreadable instance half, refused before any write" { - unset HERDR_SESSION +@test "fence: no socket path in the environment is an unreadable instance half, refused before any write" { + unset HERDR_SOCKET_PATH run agmsg_self_write T alice herdr:w1:pB "$ME" [ "$status" -eq 2 ] case "$output" in *"none:fence_unreadable:"*) : ;; *) false ;; esac From 91696ee8faa990fc1bea95d55eb4f9727bd3ba1b Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 11 Sep 2026 18:26:13 -0700 Subject: [PATCH 04/12] self-write: split the stored fence on its first colon, the side the driver guarantees The stored fence is instance:terminal_id. The herdr driver refuses an instance containing a colon (unknown:socket_path_malformed), so the instance is colon-free by construction; the terminal_id is a server-issued string whose alphabet is not ours, and a failed read already spells it unknown:. The reader split on the LAST colon, which assumed the opposite guarantee: a tid holding a colon was truncated when stored and every re-read compared unequal to it, refusing every later cell for a pane that had not moved. Split on the first colon, and say which guarantee the code actually holds. Two controls: a tid with colons that does not change lets the later cells proceed (red before this change), and two tids differing before their last colon are still told apart. --- scripts/lib/self-write.sh | 13 +++++++++---- tests/test_self_write.bats | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index f8f4d525..69f3d1e7 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -226,8 +226,13 @@ agmsg_self_write() { # 4) _sw_say "$head none:fence_unreadable:no_fence_op"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; *) _sw_say "$head none:fence_unreadable:${_SW_F_TID#unknown:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; esac - # instance:terminal_id -- the instance may be a socket PATH; the terminal_id - # never contains ':', so readers split on the LAST colon. + # instance:terminal_id. The guarantee runs ONE way: the driver refuses an + # instance containing ':' (unknown:socket_path_malformed), so the instance is + # colon-free; the terminal_id is a server-issued string whose alphabet is not + # ours to decide (and a failed read is spelled unknown:, a colon already). + # So readers split on the FIRST colon. An earlier revision split on the last + # one, which truncated a tid holding a colon and made every re-read compare + # unequal to it: a false refusal of every later cell (review, 2026-09-12). fence="$_SW_F_INSTANCE:$_SW_F_TID" _sw_say "$head" _sw_say "fence=$fence" @@ -241,7 +246,7 @@ agmsg_self_write() { # # label + key -- fence first. local why - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence##*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then lk_lines="$(_sw_cell_label_key "$id" "$team" "$agent")" _sw_say "$(printf '%s' "$lk_lines" | sed -n 1p)" _sw_say "$(printf '%s' "$lk_lines" | sed -n 2p)" @@ -251,7 +256,7 @@ agmsg_self_write() { # fi # session -- fence again: this one types into the pane. - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence##*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then sess_line="$(_sw_cell_session "$id" "$team" "$agent" "$type")" _sw_say "session $sess_line" else diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats index bc6bb9fe..6e20e844 100644 --- a/tests/test_self_write.bats +++ b/tests/test_self_write.bats @@ -185,6 +185,30 @@ FAKE [ ! -e "$(agmsg_spawn_path T alice)" ] } +@test "fence: a terminal_id that CONTAINS a colon and does not change lets the later cells proceed (stored and re-read tids compare whole)" { + # The instance half is guaranteed colon-free by the driver; the terminal_id + # half is not (a failed read is even spelled unknown:). A stored fence + # split on the LAST colon truncates such a tid and every re-read compares + # unequal to it -- a false refusal on every later cell. + _fixture terminal_id "term:0x5:9" title "◐ claude" label "" key "" status idle kind claude + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line fence)" = "fence=/tmp/herdr/sessions/jugemu/herdr.sock:term:0x5:9" ] + [ "$(_line label)" = "label attempt=ok readback=verified" ] + [ "$(_line session)" = "session attempt=ok readback=verified" ] +} + +@test "fence: two terminal_ids that differ only BEFORE their last colon are still told apart" { + _fixture terminal_id "term:a:9" title "◐ claude" label "" key "" status idle kind claude + sed -i '' -e 's|^fx() { sed -n "s/^$1=//p" "$FIX" \| head -1; }$|fx() { if [ "$1" = terminal_id ] \&\& [ "$(grep -c "\\[pane\\] \\[get\\]" "$ARGV_LOG")" -gt 1 ]; then echo term:c:9; return; fi; sed -n "s/^$1=//p" "$FIX" \| head -1; }|' "$FAKEBIN/herdr" 2>/dev/null \ + || sed -i 's|^fx() { sed -n "s/^$1=//p" "$FIX" \| head -1; }$|fx() { if [ "$1" = terminal_id ] \&\& [ "$(grep -c "\\[pane\\] \\[get\\]" "$ARGV_LOG")" -gt 1 ]; then echo term:c:9; return; fi; sed -n "s/^$1=//p" "$FIX" \| head -1; }|' "$FAKEBIN/herdr" + grep -q 'term:c:9' "$FAKEBIN/herdr" + run agmsg_self_write T alice herdr:w1:pB "$ME" + [ "$status" -eq 0 ] + [ "$(_line label)" = "label attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] + [ "$(_line session)" = "session attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] +} + # --- the entry refuses what is not a location of ours ------------------------------ @test "refuse: a ref outside the driver grammar writes nothing" { From 2bce1ca4cf45d7a0d1a1dbf30c7399f25333d60b Mon Sep 17 00:00:00 2001 From: fujibee Date: Sat, 12 Sep 2026 19:08:13 -0700 Subject: [PATCH 05/12] test(self-proof): when no single shell state is red, print the combined run's own output The whole-suite-under-shell-states test reruns the file with every state at once and, on red, narrows by rerunning each state alone. When none of the single states is red -- an interaction, or something outside the states such as an inner run that did not finish -- the narrowing printed five green lines and nothing else, which is what CI showed on 2026-09-13: a red with no test named. The combined run's not-ok block, or its tail when it never reached a verdict, is now printed in that case. --- tests/test_self_proof.bats | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_self_proof.bats b/tests/test_self_proof.bats index fe86e61d..59159b0a 100644 --- a/tests/test_self_proof.bats +++ b/tests/test_self_proof.bats @@ -751,17 +751,32 @@ RSEOF return 0 fi echo "the suite is not green with [$all]; narrowing:" - local st one + local st one any_red=0 for st in $all; do one="$BATS_TEST_TMPDIR/state.$st.out" if AGMSG_CALLER_SHELL_NEST=1 AGMSG_CALLER_SHELL_STATE="$st" \ bats "$BATS_TEST_FILENAME" > "$one" 2>&1; then echo " [$st] green" else + any_red=1 echo " [$st] RED:" grep -A3 '^not ok' "$one" | head -12 | sed 's/^/ /' fi done + # When no single state is red, the combined run failed for a reason the + # narrowing cannot name -- an interaction, or something outside the states + # entirely (a timeout, a killed helper). Say what the combined run said, + # instead of leaving an empty narrowing as the only evidence (measured + # 2026-09-13: CI red here with five green lines and nothing else). + if [ "$any_red" -eq 0 ]; then + echo " no single state is red; the combined run's own output:" + if grep -q '^not ok' "$out"; then + grep -A6 '^not ok' "$out" | head -30 | sed 's/^/ /' + else + echo " (no 'not ok' line -- the inner bats did not finish; tail follows)" + tail -15 "$out" | sed 's/^/ /' + fi + fi return 1 } From 4294dcfc8545510618c25e31579f7d5e2425e410 Mon Sep 17 00:00:00 2001 From: fujibee Date: Sat, 12 Sep 2026 19:20:34 -0700 Subject: [PATCH 06/12] self-write: every placement-record reader takes the fence field, so it never lands in the type The self-write record carries a fourth TAB field, fence=:. Eight readers split the record with `read -r ref proj type`, and read puts everything after the third TAB into the last variable: the type became "claude-codefence=...", and despawn --force handed that to reset, which then found nothing registered under it and left the registration behind. Each reader now takes a fourth variable (which also absorbs any later field). Controls: despawn --force on a record with the fence field still drops the registration (red on the three-variable reader), and a static test requires every placement-record read in scripts/ to take four variables -- it is what found the eighth reader, in watch.sh. --- scripts/arrange.sh | 2 +- scripts/despawn.sh | 10 +++++----- scripts/peek.sh | 4 ++-- scripts/placement-collisions.sh | 2 +- scripts/poke.sh | 2 +- scripts/watch.sh | 2 +- tests/test_despawn.bats | 19 +++++++++++++++++++ tests/test_self_write.bats | 13 +++++++++++++ 8 files changed, 43 insertions(+), 11 deletions(-) diff --git a/scripts/arrange.sh b/scripts/arrange.sh index 37eadffa..a21fa6a0 100755 --- a/scripts/arrange.sh +++ b/scripts/arrange.sh @@ -26,7 +26,7 @@ _placement() { local agent="$1" rec ref terminal id project type rec="$(agmsg_spawn_path "$TEAM" "$agent")" [ -f "$rec" ] || { echo "arrange: no placement record for '$TEAM/$agent'" >&2; return 1; } - IFS=$'\t' read -r ref project type < "$rec" || true + IFS=$'\t' read -r ref project type _fence < "$rec" || true terminal="$(agmsg_terminal_ref_terminal "$ref" 2>/dev/null)" || terminal="" id="$(agmsg_terminal_ref_id "$ref" 2>/dev/null)" || id="" [ -n "$terminal" ] && [ -n "$id" ] \ diff --git a/scripts/despawn.sh b/scripts/despawn.sh index 75508c76..9ea2a703 100755 --- a/scripts/despawn.sh +++ b/scripts/despawn.sh @@ -78,8 +78,8 @@ SPAWN_REC="$(agmsg_spawn_path "$TEAM" "$NAME")" # Asking with it would make a graceful teardown that WORKED report needs-force. recorded_pane_state() { [ -f "$SPAWN_REC" ] || { printf 'no-record'; return 0; } - local id _proj _type _term _bare _out _rc=0 - IFS=$'\t' read -r id _proj _type < "$SPAWN_REC" + local id _proj _type _fence _term _bare _out _rc=0 + IFS=$'\t' read -r id _proj _type _fence < "$SPAWN_REC" [ -n "$id" ] || { printf 'unknown'; return 0; } _term="$(agmsg_terminal_ref_terminal "$id")" || { printf 'unknown'; return 0; } _bare="$(agmsg_terminal_ref_id "$id")" || { printf 'unknown'; return 0; } @@ -99,8 +99,8 @@ recorded_pane_state() { kill_recorded_placement() { [ -f "$SPAWN_REC" ] || return 1 - local id _proj _type _term _bare - IFS=$'\t' read -r id _proj _type < "$SPAWN_REC" + local id _proj _type _fence _term _bare + IFS=$'\t' read -r id _proj _type _fence < "$SPAWN_REC" [ -n "$id" ] || return 1 _term="$(agmsg_terminal_ref_terminal "$id")" || return 1 # unknown/corrupt ref _bare="$(agmsg_terminal_ref_id "$id")" @@ -111,7 +111,7 @@ kill_recorded_placement() { if [ "$FORCE" = "1" ]; then [ -f "$SPAWN_REC" ] || die "no placement record for '$TEAM/$NAME' — nothing to force (was it launched via 'spawn'? graceful despawn does not need this)" - IFS=$'\t' read -r _id _proj _type < "$SPAWN_REC" + IFS=$'\t' read -r _id _proj _type _fence < "$SPAWN_REC" if ! kill_recorded_placement; then # Teardown NOT confirmed. Keep the record (the only retry authority), the # registration and the lock, and say so — never claim a forced teardown that did diff --git a/scripts/peek.sh b/scripts/peek.sh index d746696d..eee53d91 100755 --- a/scripts/peek.sh +++ b/scripts/peek.sh @@ -44,7 +44,7 @@ _peek_one() { # [lines] echo "peek: no placement record for '$team/$name' — nothing here knows which pane is theirs (spawn writes it at launch; a hand-joined member gets one when a terminal-aware session names its pane)" >&2 return 1 } - IFS=$'\t' read -r ref _proj _type < "$rec" || true + IFS=$'\t' read -r ref _proj _type _fence < "$rec" || true [ -n "$ref" ] || { echo "peek: placement record for '$team/$name' has no pane id — a record with no id is not a placement (a bug in whatever wrote it)" >&2 return 1 @@ -121,7 +121,7 @@ _peek_team() { _sweep_row "$name" - no_record no_placement_record continue fi - IFS=$'\t' read -r ref _rec_project _rec_type < "$rec" || true + IFS=$'\t' read -r ref _rec_project _rec_type _rec_fence < "$rec" || true pane="$(agmsg_terminal_ref_id "${ref:-}" 2>/dev/null)" || pane="?" screen=""; rc=0 screen="$(_peek_one "$team" "$name" 2>&1)" || rc=$? diff --git a/scripts/placement-collisions.sh b/scripts/placement-collisions.sh index 0fbf58ea..d33fdab0 100755 --- a/scripts/placement-collisions.sh +++ b/scripts/placement-collisions.sh @@ -39,7 +39,7 @@ _placement_collision_rows() { [ -n "$agent" ] || continue rec="$(agmsg_spawn_path "$team" "$agent" 2>/dev/null)" || continue [ -f "$rec" ] || continue - IFS="$tab" read -r ref _project type < "$rec" 2>/dev/null || continue + IFS="$tab" read -r ref _project type _fence < "$rec" 2>/dev/null || continue [ -n "$ref" ] || continue printf '%s\t%s\t%s\t%s\n' "$ref" "$agent" "$team" "$type" done < <(sqlite3 -noheader :memory: \ diff --git a/scripts/poke.sh b/scripts/poke.sh index 39a3f4f3..2e0cd0f9 100755 --- a/scripts/poke.sh +++ b/scripts/poke.sh @@ -69,7 +69,7 @@ esac REC="$(agmsg_spawn_path "$TEAM" "$NAME")" [ -f "$REC" ] || die "no placement record for '$TEAM/$NAME' — nothing here knows which pane is theirs (spawn writes it at launch; a hand-joined member gets one when a terminal-aware session names its pane)" -IFS=$'\t' read -r REF _PROJ _TYPE < "$REC" || true +IFS=$'\t' read -r REF _PROJ _TYPE _FENCE < "$REC" || true [ -n "$REF" ] || die "placement record for '$TEAM/$NAME' has no pane id — a record with no id is not a placement (a bug in whatever wrote it)" # The ref parser fails CLOSED (non-zero) on a corrupt/unknown-scheme ref. Under diff --git a/scripts/watch.sh b/scripts/watch.sh index fbea37ed..103230a6 100755 --- a/scripts/watch.sh +++ b/scripts/watch.sh @@ -351,7 +351,7 @@ close_own_placement() { watch_log "despawned '$name' (role dropped); no placement record for '$team/$name', so there is no pane to close from here — if a window remains it was not placed by agmsg; close it directly" return 2 fi - IFS=$'\t' read -r ref _ _ < "$rec" + IFS=$'\t' read -r ref _ _ _ < "$rec" if [ -z "$ref" ]; then watch_report "despawned '$name' (role dropped); the placement record at $rec is empty, so the pane cannot be identified — close this window manually" return 1 diff --git a/tests/test_despawn.bats b/tests/test_despawn.bats index eab39725..28bd5ffa 100644 --- a/tests/test_despawn.bats +++ b/tests/test_despawn.bats @@ -127,6 +127,25 @@ _stub_tmux_exit() { [[ "$output" != *alice* ]] # registration dropped } +@test "despawn --force: a record carrying the self-write fence field still hands the exact type to reset (#1152)" { + # The self-write path appends a fourth TAB field, fence=:. + # A reader splitting with `read -r ref proj type` puts everything after the + # third TAB into `type` -- "claude-codefence=..." -- and --force then + # calls reset with a type nothing is registered under, so the registration + # survives. The reader takes a fourth variable; this is the control. + bash "$SCRIPTS/join.sh" team alice claude-code "$PROJ" >/dev/null + printf '%s\t%s\t%s\t%s\n' '%99' "$PROJ" claude-code 'fence=/run/herdr-a.sock:term_X' > "$RUN/spawn.team__alice" + printf 'somesid\n' > "$RUN/actas.team__alice.session" + _stub_tmux_exit 0 + + run bash "$SCRIPTS/despawn.sh" team leader alice --force + [ "$status" -eq 0 ] + [ ! -f "$RUN/spawn.team__alice" ] + [ ! -f "$RUN/actas.team__alice.session" ] + run bash "$SCRIPTS/identities.sh" "$PROJ" claude-code + [ "$(printf '%s\n' "$output" | grep -c alice)" -eq 0 ] # dropped: the type reached reset intact +} + @test "despawn --force: an UNCONFIRMED teardown keeps the record and reports error (#625, --force side)" { # If the terminal driver does not confirm the pane closed (here: kill-pane exits # non-zero), the pane may still be alive. --force must NOT delete the record (the diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats index 6e20e844..c8c5fc4c 100644 --- a/tests/test_self_write.bats +++ b/tests/test_self_write.bats @@ -259,3 +259,16 @@ FAKE @test "never: the library holds no pane derivation, no search and no other-seat resolution, by name" { refute grep -E 'terminal_find_by_label|terminal_detect|_agmsg_placement_claimed_by|_agmsg_terminal_resolve_by_label|HERDR_PANE_ID|TMUX_PANE' "$SKILL_DIR/scripts/lib/self-write.sh" } + +# --- the record's fourth field must not land in anyone's `type` ------------------- + +@test "readers: every script that splits a placement record with read takes a fourth variable for the fence field" { + # `read -r ref proj type` puts everything after the third TAB into `type`. The + # self-write record has a fourth field, so every such reader takes a fourth + # variable (which absorbs any later fields too). Counted at the read sites, + # not by a guess: a reader with three variables is the defect this catches. + local bad; bad="$(grep -nE "IFS=(\\\$'\\\\t'|\"\\\$tab\") read -r [A-Za-z_]+ [A-Za-z_]+ [A-Za-z_]+ < \"?\\\$?(SPAWN_REC|REC|rec)\"?" "$SKILL_DIR"/scripts/*.sh || true)" + [ -z "$bad" ] || { echo "placement-record readers with only three variables:" >&2; printf '%s\n' "$bad" >&2; return 1; } + # and the readers do exist -- the pattern is not vacuous + [ "$(grep -cE "read -r [A-Za-z_]+ [A-Za-z_]+ [A-Za-z_]+ [A-Za-z_]+ < \"?\\\$?(SPAWN_REC|REC|rec)\"?" "$SKILL_DIR"/scripts/despawn.sh "$SKILL_DIR"/scripts/peek.sh "$SKILL_DIR"/scripts/poke.sh "$SKILL_DIR"/scripts/arrange.sh "$SKILL_DIR"/scripts/placement-collisions.sh | awk -F: '{s+=$2} END {print s+0}')" -ge 5 ] +} From 62dc48b36dea813d2a4f41451b2e009133f7da88 Mon Sep 17 00:00:00 2001 From: fujibee Date: Sat, 12 Sep 2026 19:33:59 -0700 Subject: [PATCH 07/12] self-write: plain is record-only, fenced on the seat's own tty observed through its process A plain seat can write the placement record for the locator it was handed and nothing else; the driver's capability hook says why in its own words, so "no adapter in this implementation" is never reported as "the emulator cannot". Before writing, the plain fence observes the tty of the seat's own CLI process (the pid in the owner token, via ps -o tty=), never the environment; it refuses by name when the process has no controlling tty, sits on a different tty than the locator names, or no pid was given, and the writer then writes nothing. The anchor is the tty plus the owning pid and its start time, so a /dev/ttysNNN recycled to a new session cannot pass as an old match. The emulator half is carried as delivered and is not evidence. The fence is re-read right after the record lands and BEFORE the record line is printed: a witness that moved between the two reads is named on the record (readback=mismatch:fence_changed:) and the generation is not accepted, and stdout and the done file say the same thing. Controls: the plain happy path, tty unobservable, tty mismatch, no pid, a recycled tty; the legacy plain sentinel names no place and is refused by name. --- scripts/drivers/terminals/plain/ops.sh | 40 ++++++++++- scripts/lib/self-write.sh | 54 +++++++++++--- tests/test_self_write.bats | 99 ++++++++++++++++++++++++-- 3 files changed, 173 insertions(+), 20 deletions(-) diff --git a/scripts/drivers/terminals/plain/ops.sh b/scripts/drivers/terminals/plain/ops.sh index d09f1419..5f121773 100644 --- a/scripts/drivers/terminals/plain/ops.sh +++ b/scripts/drivers/terminals/plain/ops.sh @@ -241,6 +241,43 @@ terminal_find_by_label() { _plain_unsupported "find_by_label"; } terminal_label_of() { _plain_unsupported "label_of"; } terminal_name() { _plain_unsupported "name"; } +# Fence for a self-write (#1152, #1149). A plain seat is record-only: it can +# write the placement record for the locator it was handed, and nothing else +# (no label, key or session op exists in this implementation -- see the +# capability hook, which says so in #1163's words). What it CAN verify before +# writing is that the locator's tty is the tty of the seat's own CLI process, +# observed through that process, never through the environment (launcher- +# inherited session ids were measured colliding across seats, so the emulator +# half of the locator is carried as delivered and treated as NO evidence). +# +# Prints "\t" where the anchor is the tty plus something that +# changes when the tty is reused -- /dev/ttysNNN is handed to the next session +# when this one ends -- namely the owning pid and its start time: +# iterm\ttty=/dev/ttys040,pid=12345,start=Sat_Sep_13_02:10:11_2026 +# Every failure is a named unknown in the anchor half, and the writer writes +# nothing on any of them: +# unknown:no_seat_pid the caller gave no pid to observe +# unknown:tty_unobservable the process has no controlling tty (a daemon, +# a background job), or ps could not answer +# unknown:tty_mismatch: the process sits on a different tty than +# the locator names -- the leader typed into one +# window and this seat lives in another +# unknown:invalid_id the id is not : +terminal_fence() { # [] + local id="$1" pid="${2:-}" observed start + _plain_parse_id "$id" || { printf 'unknown:invalid_id\tunknown:invalid_id\n'; return 2; } + local emulator="$_PLAIN_EMULATOR" tty="$_PLAIN_TTY" + case "$pid" in ''|*[!0-9]*) printf '%s\tunknown:no_seat_pid\n' "$emulator"; return 2 ;; esac + observed="$(ps -o tty= -p "$pid" 2>/dev/null | tr -d ' ')" + case "$observed" in ''|'??'|'-'|'?') printf '%s\tunknown:tty_unobservable\n' "$emulator"; return 2 ;; esac + case "$observed" in /dev/*) ;; *) observed="/dev/$observed" ;; esac + [ "$observed" = "$tty" ] || { printf '%s\tunknown:tty_mismatch:%s\n' "$emulator" "$observed"; return 2; } + start="$(ps -o lstart= -p "$pid" 2>/dev/null | sed 's/^ *//; s/ *$//' | tr ' ' '_')" + [ -n "$start" ] || { printf '%s\tunknown:tty_unobservable\n' "$emulator"; return 2; } + printf '%s\ttty=%s,pid=%s,start=%s\n' "$emulator" "$tty" "$pid" "$start" + return 0 +} + # NO terminal_pane_process_observe HERE, deliberately. # # The plain driver has no pane and no process to bind to, so there is nothing for @@ -249,6 +286,3 @@ terminal_name() { _plain_unsupported "name"; } # has no answer -- rather than as a failure to retry. A stub that returned # "nothing found" would be indistinguishable from a pane whose processes we could # not read, and the two must not land in the same bucket (#1152). -# No pane, so nothing to fence a write against (#1152): the self-write path -# reads this as "unsupported here" and writes nothing. -terminal_fence() { printf 'n/a:unsupported\tn/a:unsupported\n'; return 3; } diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index 69f3d1e7..0f413daf 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -84,20 +84,20 @@ _sw_say() { printf '%s\n' "$1"; _SW_LINES="${_SW_LINES}${1}"$'\n'; } # Read the fence for the pane. Sets _SW_F_INSTANCE / _SW_F_TID; returns the # driver's rc (0 value, 2 unreadable, 3 unsupported), or 4 when the driver has no # fence op at all. -_sw_fence_read() { # +_sw_fence_read() { # [] local out rc=0 _SW_F_INSTANCE=""; _SW_F_TID="" declare -F terminal_fence >/dev/null 2>&1 || return 4 - out="$(terminal_fence "$1")" || rc=$? + out="$(terminal_fence "$1" "${2:-}")" || rc=$? _SW_F_INSTANCE="${out%%$'\t'*}"; _SW_F_TID="${out#*$'\t'}" return "$rc" } # Re-read the fence and compare with the stored pair. Prints the reason on a # mismatch ("instance" / "terminal_id" / "unreadable:"), nothing when equal. -_sw_fence_check() { # +_sw_fence_check() { # [] local rc=0 - _sw_fence_read "$1" || rc=$? + _sw_fence_read "$1" "${4:-}" || rc=$? if [ "$rc" -ne 0 ]; then printf 'unreadable:%s\n' "${_SW_F_TID#unknown:}"; return 1; fi [ "$_SW_F_INSTANCE" = "$2" ] || { echo instance; return 1; } [ "$_SW_F_TID" = "$3" ] || { echo terminal_id; return 1; } @@ -125,10 +125,28 @@ _sw_cell_record() { # return 0 } +# Ask the driver's capability hook (#1163) whether a cell is implemented here. +# rc 0 = go; rc 1 = unsupported in THIS implementation (the hook's own sentence +# is the reason, so "no adapter yet" is never reported as "the emulator cannot"). +# Prints the reason on rc 1; a driver without the hook answers "go". +_sw_capability_reason() { # + local why + declare -F terminal_capability >/dev/null 2>&1 || return 0 + if why="$(terminal_capability "$1" "$2" 2>&1 >/dev/null)"; then return 0; fi + why="${why#unsupported: }"; why="${why%%$'\n'*}" + printf '%s\n' "${why:-not_implemented_here}" + return 1 +} + # The label and key cells: one terminal_name call, two separate readbacks. # Prints two lines: "label attempt=... readback=..." and "key ...". _sw_cell_label_key() { # - local id="$1" team="$2" agent="$3" rc=0 attempt obs lab key exp_label exp_key + local id="$1" team="$2" agent="$3" rc=0 attempt obs lab key exp_label exp_key why + if ! why="$(_sw_capability_reason name "$id")"; then + printf 'label attempt=skipped:unsupported:%s readback=not_attempted\n' "$why" + printf 'key attempt=skipped:unsupported:%s readback=not_attempted\n' "$why" + return 0 + fi terminal_name "$id" "$team" "$agent" >/dev/null 2>&1 || rc=$? if [ "$rc" -eq 0 ]; then attempt=ok; else attempt="failed:$rc"; fi if declare -F _herdr_label >/dev/null 2>&1; then exp_label="$(_herdr_label "$team" "$agent")"; else exp_label="$team:$agent"; fi @@ -168,7 +186,10 @@ _sw_title_now() { # -> observed session name or unknown:/n/a: # The session cell. Prints "attempt=... readback=...". _sw_cell_session() { # - local id="$1" team="$2" agent="$3" type="$4" rename_cmd cli ready rc=0 expected before after + local id="$1" team="$2" agent="$3" type="$4" rename_cmd cli ready rc=0 expected before after why + if ! why="$(_sw_capability_reason poke "$id")"; then + printf 'attempt=skipped:unsupported:%s readback=not_attempted\n' "$why"; return 0 + fi rename_cmd="$(agmsg_type_get "$type" rename_cmd 2>/dev/null || true)" [ -n "$rename_cmd" ] || { printf 'attempt=skipped:no_rename_cmd readback=not_attempted\n'; return 0; } cli="$(agmsg_type_get "$type" cli 2>/dev/null || true)" @@ -202,8 +223,11 @@ _sw_cell_session() { # # 3 unsupported here (plain). agmsg_self_write() { # local team="$1" agent="$2" ref="$3" owner="$4" - local term id head lockv fence_rc fence project type rec_line lk_lines sess_line policy + local term id head lockv fence_rc fence project type rec_line lk_lines sess_line policy seat_pid="" _SW_LINES="" + # The seat's own CLI process, when the owner token is composite .: + # the plain fence observes the tty THROUGH this process, never through env. + case "$owner" in *.*) seat_pid="${owner##*.}"; case "$seat_pid" in *[!0-9]*) seat_pid="" ;; esac ;; esac head="seat=$team/$agent sid=$owner pane=$ref" [ -n "$team" ] && [ -n "$agent" ] && [ -n "$owner" ] || { _sw_say "seat=$team/$agent sid=$owner none:bad_identity"; return 2; } if ! _agmsg_placement_split "$ref"; then _sw_say "$head none:bad_ref"; return 2; fi @@ -219,7 +243,7 @@ agmsg_self_write() { # esac fence_rc=0 - _sw_fence_read "$id" || fence_rc=$? + _sw_fence_read "$id" "$seat_pid" || fence_rc=$? case "$fence_rc" in 0) ;; 3) _sw_say "$head unsupported:${_SW_F_TID#n/a:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 3 ;; @@ -242,11 +266,19 @@ agmsg_self_write() { # # record -- the required cell. Written on the fence just read; nothing between. rec_line="$(_sw_cell_record "$team" "$agent" "$ref" "$project" "$type" "$fence")" + + # The witness must still be the same right after the record landed: a tty + # or pane handed to a new owner between the two reads is named here, and the + # record is not left standing as accepted. Judged BEFORE the record line is + # printed, so what the caller sees and what the done file says are one thing. + local why + if ! why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then + case "$rec_line" in "attempt=ok readback=verified") rec_line="attempt=ok readback=mismatch:fence_changed:$why" ;; esac + fi _sw_say "record $rec_line" # label + key -- fence first. - local why - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then lk_lines="$(_sw_cell_label_key "$id" "$team" "$agent")" _sw_say "$(printf '%s' "$lk_lines" | sed -n 1p)" _sw_say "$(printf '%s' "$lk_lines" | sed -n 2p)" @@ -256,7 +288,7 @@ agmsg_self_write() { # fi # session -- fence again: this one types into the pane. - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then sess_line="$(_sw_cell_session "$id" "$team" "$agent" "$type")" _sw_say "session $sess_line" else diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats index c8c5fc4c..fb2d21b3 100644 --- a/tests/test_self_write.bats +++ b/tests/test_self_write.bats @@ -143,7 +143,7 @@ _rec() { cat "$(agmsg_spawn_path T alice)"; } # --- the fence --------------------------------------------------------------------- -@test "fence: a terminal_id that changes after the record refuses label/key and session, visibly, and the record keeps the fence it was written on" { +@test "fence: a terminal_id that changes after the record refuses label/key and session, names the moved witness on the record, and is not accepted" { # the pane get answering the LABEL fence re-read sees a different terminal_id: # model it by rewriting the fixture right after the record is written, i.e. at # the first `agent list` call (which only the label/key readback makes) -- too @@ -157,11 +157,13 @@ FAKE run agmsg_self_write T alice herdr:w1:pB "$ME" [ "$status" -eq 0 ] [ "$(_line fence)" = "fence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA" ] - [ "$(_line record)" = "record attempt=ok readback=verified" ] + # the witness moved right after the record landed: the record is written but + # not accepted -- what it names is no longer what was observed + [ "$(_line record)" = "record attempt=ok readback=mismatch:fence_changed:terminal_id" ] [ "$(_line label)" = "label attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] [ "$(_line key)" = "key attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] [ "$(_line session)" = "session attempt=skipped:fence_mismatch:terminal_id readback=not_attempted" ] - [ "$(_line policy)" = "policy=accepted" ] + [ "$(_line policy)" = "policy=repair_incomplete" ] [ "$(grep -c 'herdr \[agent\] \[prompt\]' "$ARGV_LOG")" -eq 0 ] [ "$(grep -c 'herdr \[pane\] \[rename\]' "$ARGV_LOG")" -eq 0 ] case "$(_rec)" in *"fence=/tmp/herdr/sessions/jugemu/herdr.sock:term_AAA") : ;; *) false ;; esac @@ -219,10 +221,10 @@ FAKE [ ! -s "$ARGV_LOG" ] } -@test "refuse: a plain ref is unsupported, not a failure, and writes nothing" { +@test "refuse: the legacy plain sentinel names no place -> nothing written, named" { run agmsg_self_write T alice "plain:-" "$ME" - [ "$status" -eq 3 ] - [ "$output" = "seat=T/alice sid=$ME pane=plain:- unsupported:unsupported" ] + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=$ME pane=plain:- none:fence_unreadable:invalid_id" ] [ ! -e "$(agmsg_spawn_path T alice)" ] } @@ -272,3 +274,88 @@ FAKE # and the readers do exist -- the pattern is not vacuous [ "$(grep -cE "read -r [A-Za-z_]+ [A-Za-z_]+ [A-Za-z_]+ [A-Za-z_]+ < \"?\\\$?(SPAWN_REC|REC|rec)\"?" "$SKILL_DIR"/scripts/despawn.sh "$SKILL_DIR"/scripts/peek.sh "$SKILL_DIR"/scripts/poke.sh "$SKILL_DIR"/scripts/arrange.sh "$SKILL_DIR"/scripts/placement-collisions.sh | awk -F: '{s+=$2} END {print s+0}')" -ge 5 ] } + +# --- plain: record-only, fenced on the seat's own tty -------------------------------- +# +# The plain fence observes the seat's tty THROUGH the seat's CLI process (the +# pid in the owner token), never through the environment. `ps` is faked: it +# answers `-o tty=` and `-o lstart=` for the pid the fixture names. + +_fake_ps() { # + cat > "$FAKEBIN/ps" <> "$ARGV_LOG" +pid=""; fmt="" +while [ \$# -gt 0 ]; do case "\$1" in -o) fmt="\$2"; shift 2 ;; -p) pid="\$2"; shift 2 ;; *) shift ;; esac; done +[ "\$pid" = "$1" ] || exit 1 +case "\$fmt" in tty=) printf '%s\\n' "$2" ;; lstart=) printf '%s\\n' "$3" ;; *) exit 1 ;; esac +FAKE + chmod +x "$FAKEBIN/ps" +} + +_plain_seat() { # register alice as a plain-hosted claude-code seat; ME carries the test pid + agmsg_role_session_record T alice sid-me /proj/alice claude-code + unset HERDR_ENV HERDR_SOCKET_PATH HERDR_PANE_ID +} + +@test "plain: a locator whose tty is the seat's own tty is recorded with a pid+start anchor; label/key/session are unsupported in the driver's words; policy=accepted" { + _plain_seat; _fake_ps "$$" ttys040 "Sat Sep 13 02:10:11 2026" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 0 ] + [ "$(_line fence)" = "fence=iterm:tty=/dev/ttys040,pid=$$,start=Sat_Sep_13_02:10:11_2026" ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + case "$(_line label)" in "label attempt=skipped:unsupported:"*) : ;; *) echo "$(_line label)" >&2; return 1 ;; esac + case "$(_line session)" in "session attempt=skipped:unsupported:"*) : ;; *) echo "$(_line session)" >&2; return 1 ;; esac + [ "$(_line policy)" = "policy=accepted" ] + [ "$(_rec)" = "$(printf 'plain:iterm:/dev/ttys040\t/proj/alice\tclaude-code\tfence=iterm:tty=/dev/ttys040,pid=%s,start=Sat_Sep_13_02:10:11_2026' "$$")" ] + refute grep -q 'herdr' "$ARGV_LOG" # nothing was typed or renamed anywhere +} + +@test "plain: a seat whose process has NO controlling tty writes nothing, and says tty_unobservable" { + _plain_seat; _fake_ps "$$" '??' "Sat Sep 13 02:10:11 2026" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=$ME pane=plain:iterm:/dev/ttys040 none:fence_unreadable:tty_unobservable" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] + [ ! -e "$(agmsg_self_write_done_path T alice)" ] +} + +@test "plain: a locator naming a DIFFERENT tty than the seat sits on writes nothing, and names both" { + _plain_seat; _fake_ps "$$" ttys041 "Sat Sep 13 02:10:11 2026" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=$ME pane=plain:iterm:/dev/ttys040 none:fence_unreadable:tty_mismatch:/dev/ttys041" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +@test "plain: an owner token without a pid cannot observe a tty -> nothing written, named" { + _plain_seat; _fake_ps "$$" ttys040 "Sat Sep 13 02:10:11 2026" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "sid-bare" + [ "$status" -eq 2 ] + [ "$output" = "seat=T/alice sid=sid-bare pane=plain:iterm:/dev/ttys040 none:fence_unreadable:no_seat_pid" ] + [ ! -e "$(agmsg_spawn_path T alice)" ] +} + +@test "plain: a tty reused by a new owner between the record and the re-read is named, and the record is not accepted" { + # The same /dev/ttys040 answers, but the process start time is different on + # the second observation: the tty was recycled under us. The record stays + # (a later sweep re-delivers) but is not left standing as accepted. + _plain_seat + cat > "$FAKEBIN/ps" < "$SKILL_DIR/ps-calls.marker.\$\$" +n=\$(ls "$SKILL_DIR"/ps-calls.marker.* 2>/dev/null | wc -l | tr -d ' ') +pid=""; fmt="" +while [ \$# -gt 0 ]; do case "\$1" in -o) fmt="\$2"; shift 2 ;; -p) pid="\$2"; shift 2 ;; *) shift ;; esac; done +case "\$fmt" in + tty=) printf 'ttys040\\n' ;; + lstart=) if [ "\$n" -le 2 ]; then printf 'Sat Sep 13 02:10:11 2026\\n'; else printf 'Sat Sep 13 02:44:00 2026\\n'; fi ;; +esac +FAKE + chmod +x "$FAKEBIN/ps" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 0 ] + case "$(_line record)" in "record attempt=ok readback=mismatch:fence_changed:"*) : ;; *) echo "$(_line record)" >&2; return 1 ;; esac + [ "$(_line policy)" = "policy=repair_incomplete" ] + grep -q 'start=Sat_Sep_13_02:10:11_2026' "$(agmsg_spawn_path T alice)" # the record carries the FIRST anchor +} From 17f241cd7b19e8e9fbfde39501391e555a16896d Mon Sep 17 00:00:00 2001 From: fujibee Date: Sat, 12 Sep 2026 22:06:35 -0700 Subject: [PATCH 08/12] self-write: plain is record-only by ruling, and the spawn-time boot witness is carried when it names the same tty Two review findings. First, plain seats were record-only only when the emulator adapter said it could not name or type; an adapter that can poke would have typed a session rename. The rule is by kind, not by capability: the emulator's identity is not evidence, so no decoration is written on its strength, whatever the adapter can do. Second, the record spawn writes for a plain window carries the boot shell's pid and start time, and teardown needs them after the CLI's pid is gone. The first self-write generation now carries exactly that pair into its own fence when the existing record names the same emulator and tty and both keys are complete; unknown keys are never copied. The re-reads compare against the driver's own anchor, not the carried one. --- scripts/lib/self-write.sh | 55 +++++++++++++++++++++++++++++++++++--- tests/test_self_write.bats | 35 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index 0f413daf..8244d5eb 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -104,6 +104,30 @@ _sw_fence_check() { # [] return 0 } +# The boot pair to carry from the seat's own existing record, or nothing. +# Conditions, all required: the record exists and reads; its ref equals the new +# locator exactly (same kind, emulator and tty); its fence anchor holds BOTH +# boot= and boot_start=. Prints "boot=,boot_start=" or nothing. +_sw_boot_carry() { # + local rec line ref anchor kv boot="" boot_start="" + case "$3" in plain:*) ;; *) return 0 ;; esac + rec="$(agmsg_spawn_path "$1" "$2")" + line="$(head -1 "$rec" 2>/dev/null)" || return 0 + ref="${line%%$'\t'*}" + [ "$ref" = "$3" ] || return 0 + case "$line" in *$'\t'fence=*) anchor="${line##*$'\t'fence=}"; anchor="${anchor#*:}" ;; *) return 0 ;; esac + local IFS=, + for kv in $anchor; do + case "$kv" in + boot=*) [ -z "$boot" ] || return 0; boot="${kv#boot=}" ;; + boot_start=*) [ -z "$boot_start" ] || return 0; boot_start="${kv#boot_start=}" ;; + esac + done + [ -n "$boot" ] && [ -n "$boot_start" ] || return 0 + case "$boot" in ''|*[!0-9]*) return 0 ;; esac + printf 'boot=%s,boot_start=%s' "$boot" "$boot_start" +} + # Write the record cell. Prints "attempt=... readback=...". _sw_cell_record() { # local rec content back @@ -142,6 +166,14 @@ _sw_capability_reason() { # # Prints two lines: "label attempt=... readback=..." and "key ...". _sw_cell_label_key() { # local id="$1" team="$2" agent="$3" rc=0 attempt obs lab key exp_label exp_key why + if [ "${_SW_KIND:-}" = plain ]; then + # By ruling, not by capability: a plain seat writes its record and nothing + # else, even where an emulator adapter could name or type. The emulator's + # identity is not evidence, so no decoration is written on its strength. + printf 'label attempt=skipped:unsupported:plain_record_only readback=not_attempted\n' + printf 'key attempt=skipped:unsupported:plain_record_only readback=not_attempted\n' + return 0 + fi if ! why="$(_sw_capability_reason name "$id")"; then printf 'label attempt=skipped:unsupported:%s readback=not_attempted\n' "$why" printf 'key attempt=skipped:unsupported:%s readback=not_attempted\n' "$why" @@ -187,6 +219,9 @@ _sw_title_now() { # -> observed session name or unknown:/n/a: # The session cell. Prints "attempt=... readback=...". _sw_cell_session() { # local id="$1" team="$2" agent="$3" type="$4" rename_cmd cli ready rc=0 expected before after why + if [ "${_SW_KIND:-}" = plain ]; then + printf 'attempt=skipped:unsupported:plain_record_only readback=not_attempted\n'; return 0 + fi if ! why="$(_sw_capability_reason poke "$id")"; then printf 'attempt=skipped:unsupported:%s readback=not_attempted\n' "$why"; return 0 fi @@ -231,7 +266,7 @@ agmsg_self_write() { # head="seat=$team/$agent sid=$owner pane=$ref" [ -n "$team" ] && [ -n "$agent" ] && [ -n "$owner" ] || { _sw_say "seat=$team/$agent sid=$owner none:bad_identity"; return 2; } if ! _agmsg_placement_split "$ref"; then _sw_say "$head none:bad_ref"; return 2; fi - term="$_AGMSG_PS_TERM"; id="$_AGMSG_PS_ID" + term="$_AGMSG_PS_TERM"; id="$_AGMSG_PS_ID"; _SW_KIND="$term" _agmsg_terminal_id_ok "$term" "$id" || { _sw_say "$head none:bad_ref"; return 2; } agmsg_terminal_load "$term" 2>/dev/null || { _sw_say "$head none:no_driver:$term"; return 2; } @@ -250,6 +285,18 @@ agmsg_self_write() { # 4) _sw_say "$head none:fence_unreadable:no_fence_op"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; *) _sw_say "$head none:fence_unreadable:${_SW_F_TID#unknown:}"; agmsg_self_write_lock_release "$team" "$agent" "$owner"; return 2 ;; esac + # Carry the spawn-time boot witness forward (review ruling on #1186): when the + # seat's OWN existing record names the same plain emulator and tty as the + # locator just delivered, and carries a complete boot pair, that pair rides + # into the new fence -- the boot shell outlives a CLI whose pid has gone, and + # teardown needs it. Nothing else is copied: an unknown key is not evidence, + # and this reads the seat's own record only, never another seat's. + # The re-reads compare against what the DRIVER reports (the base anchor); + # the carried pair is stored but never expected back from a fresh read. + local _carry="" _base_tid="$_SW_F_TID" + _carry="$(_sw_boot_carry "$team" "$agent" "$ref")" + [ -z "$_carry" ] || _SW_F_TID="$_SW_F_TID,$_carry" + # instance:terminal_id. The guarantee runs ONE way: the driver refuses an # instance containing ':' (unknown:socket_path_malformed), so the instance is # colon-free; the terminal_id is a server-issued string whose alphabet is not @@ -272,13 +319,13 @@ agmsg_self_write() { # # record is not left standing as accepted. Judged BEFORE the record line is # printed, so what the caller sees and what the done file says are one thing. local why - if ! why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then + if ! why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "$_base_tid" "$seat_pid")"; then case "$rec_line" in "attempt=ok readback=verified") rec_line="attempt=ok readback=mismatch:fence_changed:$why" ;; esac fi _sw_say "record $rec_line" # label + key -- fence first. - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "$_base_tid" "$seat_pid")"; then lk_lines="$(_sw_cell_label_key "$id" "$team" "$agent")" _sw_say "$(printf '%s' "$lk_lines" | sed -n 1p)" _sw_say "$(printf '%s' "$lk_lines" | sed -n 2p)" @@ -288,7 +335,7 @@ agmsg_self_write() { # fi # session -- fence again: this one types into the pane. - if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "${fence#*:}" "$seat_pid")"; then + if why="$(_sw_fence_check "$id" "$_SW_F_INSTANCE" "$_base_tid" "$seat_pid")"; then sess_line="$(_sw_cell_session "$id" "$team" "$agent" "$type")" _sw_say "session $sess_line" else diff --git a/tests/test_self_write.bats b/tests/test_self_write.bats index fb2d21b3..5e64a03b 100644 --- a/tests/test_self_write.bats +++ b/tests/test_self_write.bats @@ -359,3 +359,38 @@ FAKE [ "$(_line policy)" = "policy=repair_incomplete" ] grep -q 'start=Sat_Sep_13_02:10:11_2026' "$(agmsg_spawn_path T alice)" # the record carries the FIRST anchor } + +@test "plain: an emulator adapter that CAN poke still gets no session rename -- plain is record-only by ruling, not by capability" { + _plain_seat; _fake_ps "$$" ttys040 "Sat Sep 13 02:10:11 2026" + # the capability hook says poke and name are supported for this emulator + terminal_capability() { return 0; } + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 0 ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + [ "$(_line label)" = "label attempt=skipped:unsupported:plain_record_only readback=not_attempted" ] + [ "$(_line session)" = "session attempt=skipped:unsupported:plain_record_only readback=not_attempted" ] + [ "$(_line policy)" = "policy=accepted" ] + refute grep -q 'rename' "$ARGV_LOG" +} + +@test "plain: a spawn-written record with a complete boot pair for the SAME emulator+tty carries that pair into the new fence; an unknown key does not" { + _plain_seat; _fake_ps "$$" ttys040 "Sat Sep 13 02:10:11 2026" + mkdir -p "$(dirname "$(agmsg_spawn_path T alice)")" + printf 'plain:iterm:/dev/ttys040\t/proj/alice\tclaude-code\tfence=iterm:tty=/dev/ttys040,boot=4242,boot_start=Sat_Sep_13_02:00:00_2026,mystery=1\n' > "$(agmsg_spawn_path T alice)" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$status" -eq 0 ] + [ "$(_line fence)" = "fence=iterm:tty=/dev/ttys040,pid=$$,start=Sat_Sep_13_02:10:11_2026,boot=4242,boot_start=Sat_Sep_13_02:00:00_2026" ] + [ "$(_line record)" = "record attempt=ok readback=verified" ] + refute grep -q 'mystery' "$(agmsg_spawn_path T alice)" +} + +@test "plain: a boot pair is NOT carried when the existing record names another tty, or the pair is incomplete" { + _plain_seat; _fake_ps "$$" ttys040 "Sat Sep 13 02:10:11 2026" + mkdir -p "$(dirname "$(agmsg_spawn_path T alice)")" + printf 'plain:iterm:/dev/ttys041\t/proj/alice\tclaude-code\tfence=iterm:tty=/dev/ttys041,boot=4242,boot_start=X\n' > "$(agmsg_spawn_path T alice)" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$(_line fence)" = "fence=iterm:tty=/dev/ttys040,pid=$$,start=Sat_Sep_13_02:10:11_2026" ] + printf 'plain:iterm:/dev/ttys040\t/proj/alice\tclaude-code\tfence=iterm:tty=/dev/ttys040,boot=4242\n' > "$(agmsg_spawn_path T alice)" + run agmsg_self_write T alice "plain:iterm:/dev/ttys040" "$ME" + [ "$(_line fence)" = "fence=iterm:tty=/dev/ttys040,pid=$$,start=Sat_Sep_13_02:10:11_2026" ] +} From e4763c3c0c50d814723cecd86fe50582eb23ecb0 Mon Sep 17 00:00:00 2001 From: fujibee Date: Sun, 13 Sep 2026 02:12:28 -0700 Subject: [PATCH 09/12] fix: the seat-side entry takes no arguments and writes only where it proved itself to be `fix` (scripts/fix.sh -> scripts/lib/self-fix.sh) is the sweep command a seat runs on itself. It takes no location, from anyone: identity is the actas locks this session owns; the pane the environment names is only a CANDIDATE handed to agmsg_self_proof (process ancestry); when the proof does not say proved and the emit-and-observe fallback agmsg_token_locate_self (#1188) is present it runs under the same four-state contract; the writer (agmsg_self_write) is reached only on proved, with the pane qualified by the instance the observation went through. Anything else is reported by name with nothing written. Any argument is refused by name: a location passed from outside was measured live to resolve a seat into another seat's pane. Tests spy on the proof, the fallback and the writer and pin what reaches the writer and what never does; each guard was mutated and goes red in the test that names it. SKILL.md gains the `fix` dispatch; the self-write header no longer describes a leader typing a pane in. --- SKILL.md | 4 ++ scripts/fix.sh | 16 +++++ scripts/lib/self-fix.sh | 131 ++++++++++++++++++++++++++++++++++ scripts/lib/self-write.sh | 15 ++-- tests/test_self_fix.bats | 143 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 303 insertions(+), 6 deletions(-) create mode 100755 scripts/fix.sh create mode 100644 scripts/lib/self-fix.sh create mode 100644 tests/test_self_fix.bats diff --git a/SKILL.md b/SKILL.md index 582ee666..5bf1e35a 100644 --- a/SKILL.md +++ b/SKILL.md @@ -189,6 +189,10 @@ If argument starts with "poke" (e.g. "poke reviewer status?"): +If argument is "fix" (no further words): +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/fix.sh` — with NO arguments. `fix` repairs THIS session's own seat marks (placement record, pane label, agent key, session name) at the pane the seat PROVES it is in. It takes no location: the seat establishes where it is from its own process ancestry (and, when that cannot decide, by writing a token to its own screen and finding it), and when it cannot establish that, it writes nothing and says why. Whoever invokes it — a `poke` from another member, a person at the keyboard, or this skill — gets the same answer. Passing a pane, a `--pane`, or any word is refused by name: a location handed in from outside is exactly the mistake this exists to remove. +2. Show the output. Exit 0: every seat this session holds was written. Exit 2: at least one seat was left unwritten, with `state=` and `reason=` on its line. Exit 1: refused (an argument, no session id, or no seat held by this session). + If argument is "reset": 1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/reset.sh "$(pwd)" __AGENT_TYPE__` 2. Tell the user the result. diff --git a/scripts/fix.sh b/scripts/fix.sh new file mode 100755 index 00000000..1b3d8ec8 --- /dev/null +++ b/scripts/fix.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# fix -- a seat establishes where it is and repairs its own identity cells there. +# No arguments (see scripts/lib/self-fix.sh for why). Run it from the seat. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +export SKILL_DIR="${SKILL_DIR:-$(cd "$SCRIPT_DIR/.." && pwd)}" +# shellcheck disable=SC1091 +. "$SKILL_DIR/scripts/lib/self-fix.sh" +# The session this shell belongs to: what the CLI exports for its own session. +# It identifies the seat's LOCKS (which roles this session holds); it says +# nothing about where the seat is -- that is the proof's job. +export AGMSG_SESSION_ID="${AGMSG_SESSION_ID:-${CLAUDE_CODE_SESSION_ID:-${CODEX_THREAD_ID:-}}}" +[ -n "$AGMSG_SESSION_ID" ] || { echo "fix none:no_session_id (no AGMSG_SESSION_ID, CLAUDE_CODE_SESSION_ID or CODEX_THREAD_ID in this shell)" >&2; exit 1; } +rc=0 +agmsg_fix_run "$@" || rc=$? +exit "$rc" diff --git a/scripts/lib/self-fix.sh b/scripts/lib/self-fix.sh new file mode 100644 index 00000000..0bca8207 --- /dev/null +++ b/scripts/lib/self-fix.sh @@ -0,0 +1,131 @@ +#!/usr/bin/env bash +# self-fix.sh -- `fix`: a seat establishes where it is, and repairs itself there. +# +# NO ARGUMENTS. This is the whole design (#1152, ruling of 2026-09-13). A location +# handed in from outside is the accident this exists to remove: measured live, +# a seat whose label had been broken resolved itself through an inherited +# environment into ANOTHER seat's pane and wrote that pane into its own mark -- +# it stopped short of typing there only because that pane's name was not +# readable. So nothing tells the seat where it is. The seat proves it: +# +# 1. identity -- which seats this session holds, from the actas locks it OWNS +# (identity is not location; a lock names a role, not a pane) +# 2. candidate -- the pane the environment names, as a CANDIDATE only; the +# environment is a generator, never an authority +# 3. proof -- agmsg_self_proof (#1154): the pane's process is in the +# owner's complete ancestry, or it is not, or we cannot tell +# 4. fallback -- when the proof does not say proved and #1188's emit-and- +# observe is present (agmsg_token_locate_self), that runs, in +# the same four-state contract +# 5. write -- ONLY on proved: the record and decorations through +# agmsg_self_write, under the seat-local lock. Anything else +# is reported by name and NOTHING is written. +# +# Whoever runs it -- a poke from another seat, a person at the keyboard, a skill +# on a loop -- gets the same answer, because none of them carries a location. +# +# OUTPUT. One line per seat this session holds, then the writer's lines: +# fix seat=/ state=proved locator= via= +# fix seat=/ state= reason= via=<...> (written nothing) +# fix none: +# Exit: 0 when every held seat was written; 2 when at least one was not; 1 on refusal. + +: "${SKILL_DIR:?self-fix.sh requires SKILL_DIR}" +# shellcheck disable=SC1091 +. "${SKILL_DIR:?}/scripts/lib/actas-lock.sh" +# shellcheck disable=SC1091 +. "${SKILL_DIR:?}/scripts/lib/terminal-registry.sh" +# shellcheck disable=SC1091 +. "${SKILL_DIR:?}/scripts/lib/self-proof.sh" +# shellcheck disable=SC1091 +. "${SKILL_DIR:?}/scripts/lib/self-write.sh" + +# The seats whose actas lock this session OWNS: "\t\t" per line. +# is the caller's session id; a lock is ours when its owner's bare sid +# equals it. Unreadable locks are skipped, not guessed. +_fix_seats_of() { # + local sid="$1" f name team agent rd kind owner + for f in "$(_actas_lock_dir)"/actas.*.session; do + [ -e "$f" ] || continue + name="${f##*/actas.}"; name="${name%.session}" + team="${name%%__*}"; agent="${name#*__}" + [ "$team" != "$name" ] || continue + rd="$(_actas_lock_read_path "$f")"; kind="${rd%%$'\t'*}"; owner="${rd#*$'\t'}" + [ "$kind" = ok ] && [ -n "$owner" ] || continue + [ "$(agmsg_instance_bare_sid "$owner")" = "$sid" ] || continue + printf '%s\t%s\t%s\n' "$team" "$agent" "$owner" + done +} + +# The locator a proof established. The proof's canonical ref names the kind +# and the pane; the INSTANCE is the one the proof's observation went through -- +# the socket the driver was talking to while it observed the pane's processes. +# That is the name of the observation's path, not the environment as an +# authority: if the proof did not say proved, this is never consulted. +_fix_locator_of_proof() { # + local ref="$1" kind pane inst="" + kind="${ref%%:*}"; pane="${ref#*:}" + case "$kind" in + herdr) inst="${HERDR_SOCKET_PATH:-}" ;; + tmux) case "$pane" in *:*) inst="${pane%:*}"; pane="${pane##*:}" ;; *) inst="${TMUX%%,*}" ;; esac ;; + plain) case "$pane" in *:*) inst="${pane%%:*}"; pane="${pane#*:}" ;; esac ;; + esac + [ -n "$inst" ] || { printf '%s\n' "$ref"; return 0; } # bare: ambient instance + agmsg_locator_compose "$kind" "$inst" "$pane" 2>/dev/null || printf '%s\n' "$ref" +} + +# Prove one seat's location. Prints "\t\t"; rc as the proof's. +_fix_locate() { # + local team="$1" agent="$2" env cand out rc=0 st + env="$(agmsg_terminal_self_env 2>/dev/null)" + if [ -n "$env" ]; then + cand="$(printf '%s' "$env" | cut -f2)" + out="$(agmsg_self_proof "$team" "$agent" "$cand")" || rc=$? + st="${out%%$'\t'*}" + if [ "$rc" -eq 0 ] && [ "$st" = proved ]; then + printf 'proved\t%s\tproof\n' "$(_fix_locator_of_proof "${out#*$'\t'}")"; return 0 + fi + else + out="undetermined"$'\t'"no_candidate_in_env"; rc=2 + fi + # not proved: the emit-and-observe fallback (#1188), when it is present + if declare -F agmsg_token_locate_self >/dev/null 2>&1; then + local fo frc=0 + fo="$(agmsg_token_locate_self "$team" "$agent")" || frc=$? + case "$frc:${fo%%$'\t'*}" in + 0:proved) printf 'proved\t%s\temit_observe\n' "${fo#*$'\t'}"; return 0 ;; + *) printf '%s\t%s\temit_observe\n' "${fo%%$'\t'*}" "${fo#*$'\t'}"; return "${frc:-2}" ;; + esac + fi + printf '%s\t%s\tproof\n' "${out%%$'\t'*}" "${out#*$'\t'}" + return "$rc" +} + +# The entry. Refuses any argument by name. +agmsg_fix_run() { + if [ "$#" -ne 0 ]; then + echo "fix none:arguments_refused (fix takes no arguments: a location handed from outside is the accident this exists to remove)" >&2 + return 1 + fi + local sid seats line team agent owner loc st payload via rc=0 any=0 failed=0 + sid="$(agmsg_instance_bare_sid "${AGMSG_SESSION_ID:-}" 2>/dev/null)" + [ -n "$sid" ] || { echo "fix none:no_session_id" >&2; return 1; } + seats="$(_fix_seats_of "$sid")" + [ -n "$seats" ] || { echo "fix none:no_seat_for_this_session" >&2; return 1; } + while IFS=$'\t' read -r team agent owner; do + [ -n "$team" ] || continue + any=1 + loc="$(_fix_locate "$team" "$agent")" || true + st="${loc%%$'\t'*}"; payload="${loc#*$'\t'}"; via="${payload##*$'\t'}"; payload="${payload%$'\t'*}" + if [ "$st" = proved ]; then + printf 'fix seat=%s/%s state=proved locator=%s via=%s\n' "$team" "$agent" "$payload" "$via" + agmsg_self_write "$team" "$agent" "$payload" "$owner" || failed=1 + else + printf 'fix seat=%s/%s state=%s reason=%s via=%s (written nothing)\n' "$team" "$agent" "$st" "$payload" "$via" + failed=1 + fi + done <<< "$seats" + [ "$any" -eq 1 ] || return 1 + [ "$failed" -eq 0 ] || return 2 + return 0 +} diff --git a/scripts/lib/self-write.sh b/scripts/lib/self-write.sh index 8244d5eb..e85256c0 100644 --- a/scripts/lib/self-write.sh +++ b/scripts/lib/self-write.sh @@ -7,12 +7,15 @@ # itself, writes ONLY its own cells, and nothing here reads another seat's state # to decide a write. There is no "who is right" left to decide. # -# WHERE THE PANE COMES FROM. Not from here. The seat does not derive or verify -# its pane: a leader sweeps the workspace and types `fix --pane P` into pane P, -# and the seat that receives it is in P by construction -- typing into P is the -# only way to reach P (koit, 2026-09-11). So arrives as an argument and is -# a LOCATION carried by the channel, never an identity: team and agent come from -# the seat's own actas, and no argument may name another seat. +# WHERE THE PANE COMES FROM. Not from here, and not from outside. The seat +# PROVES it (scripts/lib/self-fix.sh, ruling of 2026-09-13): its own process +# ancestry against the pane's processes (self-proof.sh), or failing that a token +# it writes to its own screen and finds (token-locate.sh); if neither decides, +# nothing is written. A location handed in from outside was measured to be the +# accident itself: a seat with a broken label resolved itself through an +# inherited environment into ANOTHER seat's pane. So arrives here already +# proved, and it is a LOCATION, never an identity: team and agent come from the +# seat's own actas, and no argument may name another seat. # # THE FENCE. Pane ids repeat across terminal instances (two herdr sessions both # have a w1:p2; tmux has one id space per socket), so a pane id alone can name diff --git a/tests/test_self_fix.bats b/tests/test_self_fix.bats new file mode 100644 index 00000000..cd6214a4 --- /dev/null +++ b/tests/test_self_fix.bats @@ -0,0 +1,143 @@ +#!/usr/bin/env bats +# `fix` (scripts/lib/self-fix.sh): no arguments; identity from the locks this +# session owns; the environment only proposes a candidate; the proof decides; +# the emit-and-observe fallback (#1188) runs when present; nothing is written +# unless a proof said proved. The proof, the fallback and the writer are spies +# here: what this file pins is the ORCHESTRATION -- what reaches the writer, +# and what never does. + +load test_helper + +setup() { + setup_test_env + export SKILL_DIR="$TEST_SKILL_DIR" + export RUN_DIR="$SKILL_DIR/run"; mkdir -p "$RUN_DIR" + export SPY="$SKILL_DIR/spy.log"; : > "$SPY" + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/self-fix.sh" + ME="sid-me.$$"; export AGMSG_SESSION_ID="$ME" + printf '%s\n' "$ME" > "$RUN_DIR/cc-instance.$$" + export HERDR_ENV=1 HERDR_PANE_ID=w1:pB HERDR_SOCKET_PATH=/tmp/herdr/sessions/a/herdr.sock + unset TMUX TMUX_PANE + # the writer is a spy: it records its arguments and writes nothing + agmsg_self_write() { printf 'write %s %s %s %s\n' "$1" "$2" "$3" "$4" >> "$SPY"; echo "seat=$1/$2 sid=$4 pane=$3"; echo "policy=accepted"; return 0; } +} +teardown() { teardown_test_env; } + +_own_seat() { printf '%s\n' "$2" > "$(actas_lock_path T "$1")"; } +_proof_says() { # + local rc="$1" st="$2" pl="$3" + eval "agmsg_self_proof() { printf 'proof %s %s %s\\n' \"\$1\" \"\$2\" \"\$3\" >> \"\$SPY\"; printf '%s\\t%s\\n' '$st' '$pl'; return $rc; }" +} + +@test "fix: any argument is refused by name, and nothing runs" { + _own_seat alice "$ME"; _proof_says 0 proved herdr:w1:pB + run agmsg_fix_run herdr:w1:pB + [ "$status" -eq 1 ] + [ "$output" = "fix none:arguments_refused (fix takes no arguments: a location handed from outside is the accident this exists to remove)" ] + [ ! -s "$SPY" ] +} + +@test "fix: a session that owns no seat writes nothing, and says so" { + _proof_says 0 proved herdr:w1:pB + run agmsg_fix_run + [ "$status" -eq 1 ] + [ "$output" = "fix none:no_seat_for_this_session" ] + [ ! -s "$SPY" ] +} + +@test "fix: proved -> the writer gets the seat, the proof's pane qualified by the observation's socket, and the LOCK's owner token" { + _own_seat alice "$ME"; _proof_says 0 proved herdr:w1:pB + run agmsg_fix_run + [ "$status" -eq 0 ] + [ "$(printf '%s\n' "$output" | head -1)" = "fix seat=T/alice state=proved locator=herdr:/tmp/herdr/sessions/a/herdr.sock:w1:pB via=proof" ] + grep -Fqx "proof T alice w1:pB" "$SPY" # env pane reached the PROOF as a candidate + grep -Fqx "write T alice herdr:/tmp/herdr/sessions/a/herdr.sock:w1:pB $ME" "$SPY" +} + +@test "fix: disproved -> nothing written; the env candidate never reaches the writer" { + _own_seat alice "$ME"; _proof_says 1 disproved pane_process_not_ancestor + run agmsg_fix_run + [ "$status" -eq 2 ] + [ "$output" = "fix seat=T/alice state=disproved reason=pane_process_not_ancestor via=proof (written nothing)" ] + refute grep -q '^write' "$SPY" +} + +@test "fix: undetermined with NO fallback present -> nothing written, the proof's reason named" { + _own_seat alice "$ME"; _proof_says 2 undetermined owner_marker_absent + unset -f agmsg_token_locate_self 2>/dev/null || true + run agmsg_fix_run + [ "$status" -eq 2 ] + [ "$output" = "fix seat=T/alice state=undetermined reason=owner_marker_absent via=proof (written nothing)" ] + refute grep -q '^write' "$SPY" +} + +@test "fix: undetermined, then the emit-and-observe fallback says proved -> written with ITS locator, via=emit_observe" { + _own_seat alice "$ME"; _proof_says 2 undetermined invocation_not_bound_to_owner + agmsg_token_locate_self() { printf 'fallback %s %s\n' "$1" "$2" >> "$SPY"; printf 'proved\therdr:/tmp/herdr/sessions/a/herdr.sock:w1:p7\n'; return 0; } + run agmsg_fix_run + [ "$status" -eq 0 ] + [ "$(printf '%s\n' "$output" | head -1)" = "fix seat=T/alice state=proved locator=herdr:/tmp/herdr/sessions/a/herdr.sock:w1:p7 via=emit_observe" ] + grep -Fqx "fallback T alice" "$SPY" + grep -Fqx "write T alice herdr:/tmp/herdr/sessions/a/herdr.sock:w1:p7 $ME" "$SPY" + [ "$(grep -c '^write' "$SPY")" -eq 1 ] +} + +@test "fix: the fallback's own undetermined (ambiguous) -> nothing written, named, via=emit_observe" { + _own_seat alice "$ME"; _proof_says 2 undetermined owner_marker_absent + agmsg_token_locate_self() { printf 'undetermined\tambiguous\n'; return 2; } + run agmsg_fix_run + [ "$status" -eq 2 ] + [ "$output" = "fix seat=T/alice state=undetermined reason=ambiguous via=emit_observe (written nothing)" ] + refute grep -q '^write' "$SPY" +} + +@test "fix: no candidate in the environment -> the proof is not even asked; fallback if present, else no_candidate_in_env" { + _own_seat alice "$ME"; _proof_says 0 proved herdr:w1:pB + unset HERDR_ENV HERDR_PANE_ID + unset -f agmsg_token_locate_self 2>/dev/null || true + run agmsg_fix_run + [ "$status" -eq 2 ] + [ "$output" = "fix seat=T/alice state=undetermined reason=no_candidate_in_env via=proof (written nothing)" ] + refute grep -q '^proof' "$SPY" + refute grep -q '^write' "$SPY" +} + +@test "fix: a session holding two seats proves and writes each; a lock owned by another session is not ours" { + _own_seat alice "$ME"; _own_seat bob "$ME"; _own_seat carol "sid-other.424242" + _proof_says 0 proved herdr:w1:pB + run agmsg_fix_run + [ "$status" -eq 0 ] + [ "$(grep -c '^write T alice ' "$SPY")" -eq 1 ] + [ "$(grep -c '^write T bob ' "$SPY")" -eq 1 ] + refute grep -q 'carol' "$SPY" +} + +@test "fix: the entry script refuses without a session id in the shell, and takes no arguments" { + run env -u AGMSG_SESSION_ID -u CLAUDE_CODE_SESSION_ID -u CODEX_THREAD_ID bash "$SKILL_DIR/scripts/fix.sh" + [ "$status" -eq 1 ] + case "$output" in "fix none:no_session_id"*) : ;; *) echo "$output" >&2; return 1 ;; esac + run bash "$SKILL_DIR/scripts/fix.sh" herdr:w1:p2 + [ "$status" -eq 1 ] + case "$output" in "fix none:arguments_refused"*) : ;; *) echo "$output" >&2; return 1 ;; esac +} + +@test "fix: herdr with no socket in the environment -> the proof's bare ref is written as-is (ambient instance), not a malformed 'herdr::pane'" { + _own_seat alice "$ME"; _proof_says 0 proved herdr:w1:pB + unset HERDR_SOCKET_PATH + run agmsg_fix_run + [ "$status" -eq 0 ] + grep -Fqx "write T alice herdr:w1:pB $ME" "$SPY" + refute grep -q 'herdr::' "$SPY" +} + +@test "fix: tmux -> the proof's pane is qualified by the socket the observation went through" { + # tmux's self-env candidate is socket-qualified, and so is the proof's ref (#1051) + _own_seat alice "$ME"; _proof_says 0 proved 'tmux:/tmp/tmux-501/default:%5' + unset HERDR_ENV HERDR_PANE_ID HERDR_SOCKET_PATH + export TMUX="/tmp/tmux-501/default,123,0" TMUX_PANE='%5' + run agmsg_fix_run + [ "$status" -eq 0 ] + grep -Fqx "proof T alice /tmp/tmux-501/default:%5" "$SPY" + grep -Fqx "write T alice tmux:/tmp/tmux-501/default:%5 $ME" "$SPY" +} From 82551154323c8b4bbe40eda4e8e91d6fbee05cfd Mon Sep 17 00:00:00 2001 From: fujibee Date: Sun, 13 Sep 2026 02:21:03 -0700 Subject: [PATCH 10/12] fix: guard the TMUX read in the tmux locator step, and prove the missing-TMUX path under set -u The tmux branch of _fix_locator_of_proof read TMUX unguarded. Under the entry's set -u, a seat with no TMUX in its environment would have killed the command substitution the locator runs in, and the seat line would have come out with an EMPTY state -- neither written nor refused by name. The read now takes a default before the split. The control runs the entry in a real bash -u shell with spies, and was red before the change: the seat line lost its state. The unguarded-env checker is back at its baseline (89). --- scripts/lib/self-fix.sh | 2 +- tests/test_self_fix.bats | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/scripts/lib/self-fix.sh b/scripts/lib/self-fix.sh index 0bca8207..ef5a481b 100644 --- a/scripts/lib/self-fix.sh +++ b/scripts/lib/self-fix.sh @@ -67,7 +67,7 @@ _fix_locator_of_proof() { # kind="${ref%%:*}"; pane="${ref#*:}" case "$kind" in herdr) inst="${HERDR_SOCKET_PATH:-}" ;; - tmux) case "$pane" in *:*) inst="${pane%:*}"; pane="${pane##*:}" ;; *) inst="${TMUX%%,*}" ;; esac ;; + tmux) case "$pane" in *:*) inst="${pane%:*}"; pane="${pane##*:}" ;; *) inst="${TMUX:-}"; inst="${inst%%,*}" ;; esac ;; plain) case "$pane" in *:*) inst="${pane%%:*}"; pane="${pane#*:}" ;; esac ;; esac [ -n "$inst" ] || { printf '%s\n' "$ref"; return 0; } # bare: ambient instance diff --git a/tests/test_self_fix.bats b/tests/test_self_fix.bats index cd6214a4..70f9ebb5 100644 --- a/tests/test_self_fix.bats +++ b/tests/test_self_fix.bats @@ -141,3 +141,26 @@ _proof_says() { # grep -Fqx "proof T alice /tmp/tmux-501/default:%5" "$SPY" grep -Fqx "write T alice tmux:/tmp/tmux-501/default:%5 $ME" "$SPY" } + +@test "fix: tmux proved with NO TMUX in the environment, under set -u -> the bare ref is written and the failure path is spoken, never an aborted substitution" { + # The entry runs under set -euo pipefail. An unguarded read of TMUX inside the + # locator step would kill the $( ) it runs in, and the seat line would come out + # with an EMPTY state -- neither written nor refused by name. So this runs in + # a real shell with -u, and the signal is the seat line's content. + _own_seat alice "$ME" + cat > "$SKILL_DIR/probe.sh" <> "\$SPY"; } +agmsg_fix_run +PROBE + run bash "$SKILL_DIR/probe.sh" + [ "$status" -eq 0 ] || { echo "$output" >&2; return 1; } + [ "$(printf '%s\n' "$output" | head -1)" = "fix seat=T/alice state=proved locator=tmux:%5 via=proof" ] + grep -Fqx "write T alice tmux:%5 $ME" "$SPY" + refute grep -q 'state= ' <<< "$output" +} From 75285fa18bf62c73a7b3d76ee8af4b9bb2a4c4aa Mon Sep 17 00:00:00 2001 From: fujibee Date: Sun, 13 Sep 2026 02:34:49 -0700 Subject: [PATCH 11/12] ci: re-trigger checks for the guarded-TMUX head; the push during the GitHub outage produced no run From c1668ea803e9d0854d791ffeb1a9a4d8fdf5e2e5 Mon Sep 17 00:00:00 2001 From: fujibee Date: Sun, 13 Sep 2026 03:37:21 -0700 Subject: [PATCH 12/12] fix(herdr): route self-write fences to their instance --- scripts/drivers/terminals/herdr/ops.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/drivers/terminals/herdr/ops.sh b/scripts/drivers/terminals/herdr/ops.sh index 27edea9b..cab272e2 100644 --- a/scripts/drivers/terminals/herdr/ops.sh +++ b/scripts/drivers/terminals/herdr/ops.sh @@ -1349,12 +1349,13 @@ _herdr_panes_of() { # # resolved in one session landing in another's live pane. terminal_fence() { # local id="$1" instance pane_json esc tid - instance="${HERDR_SOCKET_PATH:-}" + instance="$(_herdr_sock_of "$id")" + [ -n "$instance" ] || instance="${HERDR_SOCKET_PATH:-}" [ -n "$instance" ] || instance="unknown:no_socket_in_env" case "$instance" in *:*|*[[:cntrl:]]*|*[[:space:]]*) instance="unknown:socket_path_malformed" ;; esac command -v herdr >/dev/null 2>&1 || { printf '%s\tunknown:terminal_unreachable\n' "$instance"; return 2; } _herdr_pane_id_ok "$id" || { printf '%s\tunknown:invalid_pane_id\n' "$instance"; return 2; } - pane_json="$(herdr pane get "$id" 2>/dev/null)" || { printf '%s\tunknown:pane_query_failed\n' "$instance"; return 2; } + pane_json="$(_herdr_cli "$id" pane get "$(_herdr_bare_of "$id")" 2>/dev/null)" || { printf '%s\tunknown:pane_query_failed\n' "$instance"; return 2; } esc="$(printf '%s' "$pane_json" | sed "s/'/''/g")" tid="$(sqlite3 :memory: "SELECT CASE WHEN json_type('$esc','\$.result.pane.terminal_id')='text' THEN json_extract('$esc','\$.result.pane.terminal_id') ELSE '' END" 2>/dev/null)" \ || { printf '%s\tunknown:pane_response_invalid\n' "$instance"; return 2; }