diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 7fa6a3c9e..bf83629bb 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "agmsg", "description": "Cross-agent messaging via SQLite. Send messages between CLI AI agents. No daemon, no network.", - "version": "1.2.3", + "version": "1.3.0", "author": { "name": "fujibee" }, diff --git a/.github/enforced-assertions-baseline b/.github/enforced-assertions-baseline index 7d71bf4ad..104eee38f 100644 --- a/.github/enforced-assertions-baseline +++ b/.github/enforced-assertions-baseline @@ -1 +1 @@ -635 +626 diff --git a/.github/errexit-status-reads-baseline b/.github/errexit-status-reads-baseline new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/.github/errexit-status-reads-baseline @@ -0,0 +1 @@ +0 diff --git a/.github/herdr-cli-routing-allowlist b/.github/herdr-cli-routing-allowlist new file mode 100644 index 000000000..cd45ac004 --- /dev/null +++ b/.github/herdr-cli-routing-allowlist @@ -0,0 +1,19 @@ +# Functions in scripts/drivers/terminals/herdr/ops.sh that call the `herdr` CLI +# directly, with the exact number of such calls. Checked by +# .github/scripts/check-herdr-cli-routing.sh in both directions. +# +# Every function here is instance-wide or deliberately ambient: it lists or +# creates in the instance the environment names, and takes no pane id whose +# socket could name a different one. A function that operates ON a pane id is +# not allowed here -- it routes through `_herdr_cli ...`, which sets +# HERDR_SOCKET_PATH from a socket-qualified id. (`_herdr_cli` itself calls +# `herdr "$@"` with no literal subcommand word, so the position rule never +# counts it: it is the one routed call by construction.) +# +# +_herdr_pane_for_session 1 +terminal_describe 2 +terminal_detect 1 +terminal_find_by_label 1 +terminal_spawn 2 +terminal_enumerate_panes 2 diff --git a/.github/scripts/bats-file-seconds.tsv b/.github/scripts/bats-file-seconds.tsv new file mode 100644 index 000000000..d79312125 --- /dev/null +++ b/.github/scripts/bats-file-seconds.tsv @@ -0,0 +1,146 @@ +# Per-file real macOS bats wall time (seconds), used by shard-tests.sh to +# weight the CI partition by measured cost instead of @test count (#1243). +# @test count is a loose, occasionally blind proxy for a file's real cost: +# a file whose time is almost entirely waiting (background processes, poll +# loops) can carry a tiny count while dominating its shard's wall clock, and +# a file that is merely large in @test count is not necessarily slow. This +# table sidesteps the proxy by using each file's own measured time directly. +# +# MEASURED AGAINST: GitHub Actions run 34845597032 (release-candidate draft +# PR #1240, tests.yml bats job, macos-latest, 5 shards, 2026-09-14, the tree +# after #1105/#1231/#1234/#1237/#1239/#1241/#1245 landed). Every shard in +# that run completed (none timed out), so every file below has a real +# file_end record -- summed from the run's 5 bats-timings-macos-latest-* +# artifacts (file_end rows only, macOS only). +# +# A file with no row here (renamed, or added after this measurement) is +# estimated at this table's own average seconds/file by shard-tests.sh, +# never treated as zero-cost or dropped from the partition -- coverage does +# not depend on this table being current. Refresh by re-running this +# extraction against a green run's bats-timings artifacts once the tree +# drifts enough that the balance this buys degrades (watched by +# tests/test_ci_sharding.bats's balance-ratio assertion, not by a schedule). +# +# Columns: basenameseconds. Sorted by basename for a legible diff. +test_actas_integration.bats 65 +test_actas_lock.bats 35 +test_antigravity_resume.bats 1 +test_antigravity_transport.bats 12 +test_api.bats 44 +test_app_release_signing.bats 1 +test_apply_fail_stderr.bats 0 +test_bin_agmsg.bats 2 +test_binding_mode.bats 3 +test_boot_command.bats 10 +test_bump_app_version.bats 0 +test_capability_docs.bats 2 +test_ci_forensics.bats 1 +test_ci_sharding.bats 69 +test_ci_workflow.bats 0 +test_claude_template.bats 1 +test_close_fds.bats 0 +test_cmdline_path_match.bats 1 +test_codex_bridge.bats 199 +test_codex_bridge_launcher.bats 281 +test_codex_monitor.bats 9 +test_codex_resume.bats 10 +test_codex_shim.bats 20 +test_compat.bats 3 +test_compat_posix.bats 1 +test_config.bats 2 +test_cut_release.bats 5 +test_delivery.bats 432 +test_despawn.bats 137 +test_dispatch.bats 75 +test_doctor.bats 97 +test_endpoint_scheme.bats 1 +test_endpoint_table_node.bats 0 +test_enforced_assertions.bats 2 +test_engine_inherited_fds.bats 21 +test_export.bats 37 +test_harness_self_name_off.bats 1 +test_hash.bats 4 +test_herdr_cli_routing.bats 3 +test_identity_path_forms.bats 6 +test_inbox.bats 114 +test_install.bats 109 +test_instance_id.bats 30 +test_jsonl_remote_sync.bats 44 +test_key.bats 72 +test_legacy_mirror.bats 11 +test_local_quoting.bats 1 +test_local_team_ids.bats 11 +test_locator.bats 5 +test_messaging.bats 124 +test_migrate_team_store.bats 151 +test_node_resolve.bats 1 +test_peek_poke.bats 22 +test_perf_harness.bats 12 +test_placement_actual_location.bats 2 +test_placement_collisions.bats 38 +test_plugin_registry.bats 7 +test_printed_command_paths.bats 10 +test_refute.bats 1 +test_registry_lock.bats 8 +test_release_ci_skip.bats 1 +test_release_dist_tag.bats 1 +test_remote.bats 453 +test_remote_curl_config_paths.bats 5 +test_remote_curl_stderr.bats 3 +test_remote_curl_stderr_get.bats 6 +test_remote_engine_start_refusal.bats 660 +test_remote_forget.bats 11 +test_remote_header_sink.bats 4 +test_remote_refusal.bats 18 +test_remote_setup_doc.bats 8 +test_remote_status_liveness.bats 346 +test_remote_sync.bats 328 +test_remote_sync_apply_blob.bats 5 +test_remote_sync_ca_bundle.bats 1 +test_remote_sync_driver_input.bats 0 +test_remote_sync_engine.bats 12 +test_remote_sync_reject_envelope.bats 5 +test_require_python3.bats 5 +test_reset_role_session.bats 13 +test_resolve_project.bats 35 +test_resume_seat_guard.bats 19 +test_resurrect_panes.bats 11 +test_role_session.bats 14 +test_roster_journal.bats 99 +test_self_fix.bats 13 +test_self_identity.bats 24 +test_self_name.bats 149 +test_self_proof.bats 42 +test_self_rename.bats 30 +test_self_write.bats 46 +test_self_write_lock.bats 36 +test_session_start_terminal_line.bats 12 +test_setup.bats 1 +test_shquote.bats 1 +test_spawn.bats 477 +test_spawn_fd_guard.bats 1 +test_spawn_options.bats 2 +test_sqlite_crlf.bats 9 +test_sqlite_sync_jq_binary.bats 5 +test_storage.bats 37 +test_storage_contract.bats 44 +test_sweep_enumeration.bats 9 +test_sync_autostart.bats 29 +test_sync_cipher.bats 7 +test_team.bats 198 +test_team_guidance_usage.bats 1 +test_team_list.bats 28 +test_team_status.bats 14 +test_teardown_reap.bats 3 +test_terminal_context_line.bats 2 +test_terminal_registry.bats 81 +test_token_locate.bats 3 +test_transcript_exists.bats 2 +test_type_registry.bats 36 +test_unguarded_env_reads.bats 2 +test_wait_helpers.bats 40 +test_watch.bats 317 +test_watch_install_changed.bats 24 +test_watch_once.bats 91 +test_watch_stuck_map.bats 3 +test_where.bats 1 diff --git a/.github/scripts/check-errexit-status-reads.sh b/.github/scripts/check-errexit-status-reads.sh new file mode 100755 index 000000000..eeb78e7ad --- /dev/null +++ b/.github/scripts/check-errexit-status-reads.sh @@ -0,0 +1,481 @@ +#!/usr/bin/env bash +# +# Fail when a script grows a `$?` read that errexit never lets it reach — or +# that it reaches only to find a 0 that means nothing. +# +# Measured, not assumed. Each row run as `bash -c 'set -e;
'` on both +# interpreters this project ships against: +# +# bash 3.2.57 bash 5.3.15 +# x=$(false); rc=$?; echo $rc shell dies shell dies +# local x=$(false); rc=$?; echo $rc rc=0, survives rc=0, survives +# declare x=$(false); rc=$? rc=0, survives rc=0, survives +# export x=$(false); rc=$? rc=0, survives rc=0, survives +# x=$(false) || rc=$? rc=1 rc=1 +# if x=$(false); then :; fi not fatal not fatal +# f() { . failing.sh; rc=$?; } shell dies rc=1, survives <- differs +# f() { . failing.sh || rc=$?; } shell dies rc=1, survives <- differs +# +# Three ways to be wrong, one shape to look for — a status read that follows +# something errexit already decided: +# +# [bare] x=$(cmd) the assignment's status IS the substitution's, +# rc=$? so a non-zero one kills the shell HERE. The +# next line is unreachable; the handler it feeds +# has never run. +# +# [decl] local x=$(cmd) `local`/`declare`/`typeset`/`export`/`readonly` +# rc=$? is a builtin whose own status wins. The shell +# survives and `rc` is ALWAYS 0 — a handler that +# reads as present and can never fire. This is +# the quiet one; nothing crashes. +# +# [source] . file On bash 3.2 a failing command at the top of a +# rc=$? sourced file fires the CALLER's errexit, and it +# does so EVEN with `|| rc=$?` on the source line +# (measured; the `||` does not save 3.2). macOS +# /bin/bash is 3.2, so this is a macOS-only death +# that passes every Linux run. +# +# The accepted fix for all three is the codebase's two-line lift — see +# `agmsg_terminal_load` in scripts/lib/terminal-registry.sh: +# +# local rc=0 restore_e=0 +# case $- in *e*) restore_e=1 ;; esac +# set +e +# x=$(cmd) # or `. file` +# rc=$? +# [ "$restore_e" = 1 ] && set -e +# +# so a statement sitting between `set +e` and `set -e` is NOT flagged: that is +# the fix, not the defect. `|| rc=$?` is not flagged either for [bare]/[decl] +# (measured correct on both shells) — but it does NOT clear [source]. +# +# WHAT IS EXCLUDED, and why: +# - anything between `set +e` and the next `set -e`: errexit is lifted, which +# is the whole point of lifting it +# - a statement carrying `||` or `&&`: explicit control (except [source]) +# - the condition of `if` / `while` / `until`: errexit does not apply there +# +# The baseline is a COUNT, not a file:line list, so moving code between files +# does not produce a spurious failure. It may only go down. +# +# WHY THIS CANNOT PASS BY FAILING TO LOOK: before it reports anything about the +# tree, it runs the same scanner over a fixture holding one known-bad instance +# of each kind and requires all three back. A regex that stops matching — a +# refactor, a quoting change, a wrong path — then exits 2 (could not answer) +# instead of 0 (nothing found). "Zero" is only ever printed by a scanner that +# has just proved it can find one. + +set -u + +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +BASELINE_FILE="${AGMSG_ERREXIT_BASELINE:-$ROOT/.github/errexit-status-reads-baseline}" +SCAN_DIR="${1:-$ROOT/scripts}" + +scan() { + python3 - "$1" <<'PY' +import re, sys, pathlib + +ASSIGN = re.compile(r'^(?Plocal|declare|typeset|export|readonly)?\s*' + r'(?P[A-Za-z_][A-Za-z0-9_]*)=(?P.*)$', re.S) +SOURCEC = re.compile(r'^(\.|source)\s+\S') +# A status READ is an assignment whose whole value is `$?` -- `rc=$?`, +# `local rc=$?`. A statement that merely CONTAINS `$?` is not one: the +# `|| vrc=$?` on a guarded assignment is that assignment's own handling, +# and reporting the line before it (herdr/ops.sh:60) was a false positive. +STATUS = re.compile(r'^(local|declare|typeset|export|readonly)?\s*[A-Za-z_][A-Za-z0-9_]*=\$\?\s*$') +COND = re.compile(r'^(if|while|until|elif)\b') +SETPLUS = re.compile(r'^set\s+\+[a-zA-Z]*e') +SETMINUS= re.compile(r'^set\s+-[a-zA-Z]*e') + +def split_statements(text): + """Walk the file once, tracking quote state and $( ) nesting ACROSS LINES. + + Splitting per line is what made this wrong: a SQL string that opens on one + line and closes on another left every `;` between them looking like a + statement separator, so + + x="$(sqlite3 :memory: "SELECT ... LIMIT 1;" 2>/dev/null)" || rc=$? + + was cut in half and its `|| rc=$?` guard was reported as an unguarded bare + assignment. A checker that reports the correct form gets worked around, and + a worked-around checker passes while guarding nothing. + + A command substitution opens a FRESH quoting context even when it appears + inside double quotes -- `"$( ... "inner" ... )"` is one word to bash, and + the inner quotes are the substitution's, not the outer string's. So the + quote character is pushed on entering `$(` and restored on the matching + `)`. Modelling that as a flat flag is what let the first fix swallow a real + instance: the `"` right after `$(` read as CLOSING the outer string, and + everything after it fell out of the statement.""" + out = [] + buf, start = '', None + line = 1 + q = None # active quote char in the CURRENT context + stack = [] # saved quote chars, one per open $( + i, n = 0, len(text) + while i < n: + c = text[i] + + # A backslash escapes the next character, and a backslash-NEWLINE is a + # line continuation: bash removes both and the statement carries on. + # Splitting there is what hid + # + # . \\ + # "$dir/ops.sh" + # rc=$? + # + # from the source check -- `. \\` and `"$dir/ops.sh"` became two + # statements, so neither was the `rc=$?`'s predecessor and neither + # matched SOURCEC. `source` is one of the three forms this checker + # names, so that hole made a count of zero unprovable for it. Inside + # SINGLE quotes a backslash is literal and does not continue a line, so + # that case is left to the single-quote branch below. + if c == '\\' and q != "'": + nxt = text[i+1] if i + 1 < n else '' + if nxt == '\n': + line += 1 + i += 2 + continue + if nxt: + buf += c + nxt + if start is None: + start = line + i += 2 + continue + + if c == '\n': + here = line + line += 1 + if q is None and not stack: + if buf.strip(): + out.append((start or here, buf.strip())) + buf, start = '', None + else: + buf += c + i += 1 + continue + + # inside single quotes nothing is special but the closing quote + if q == "'": + buf += c + if c == "'": + q = None + i += 1 + continue + + # command substitution opens a new quoting context, double quotes or not + if text[i:i+2] == '$(': + stack.append(q) + q = None + buf += '$(' + if start is None: + start = line + i += 2 + continue + + if c == ')' and stack and q is None: + q = stack.pop() + buf += c + i += 1 + continue + + if q == '"': + buf += c + if c == '"' and text[i-1] != '\\': + q = None + i += 1 + continue + + # unquoted, at some $( depth or none + if c in ('"', "'"): + q = c + buf += c + if start is None: + start = line + i += 1 + continue + + # `#` opens a comment when it STARTS A WORD -- at the beginning of a + # statement or after whitespace. Requiring the statement to be empty + # was not merely incomplete, it was actively dangerous: a TRAILING + # comment stayed in the text, and the apostrophe in one (`review's`) read + # as an opening single quote and swallowed everything to the next one. + # That silently disabled the sql_bare control several functions later, + # so the file reported three findings instead of four and still looked + # healthy. A checker whose own controls can be switched off by a + # comment is not measuring anything. + # + # `${x#f}` and `$#` are not comments and are not caught here: neither + # follows whitespace. + if c == '#' and (not buf or buf[-1].isspace()) and not stack: + while i < n and text[i] != '\n': + i += 1 + continue + + if c == ';' and not stack: + if buf.strip(): + out.append((start or line, buf.strip())) + buf, start = '', None + i += 1 + continue + + buf += c + if start is None and c.strip(): + start = line + i += 1 + + if buf.strip(): + out.append((start or line, buf.strip())) + return out + +rows = [] +for f in sorted(pathlib.Path(sys.argv[1]).rglob('*.sh')): + lifted = False + prev = None + for n, st in split_statements(f.read_text(errors='replace')): + if SETPLUS.match(st): + lifted = True; prev = (n, st); continue + if SETMINUS.match(st): + lifted = False; prev = (n, st); continue + + if prev and STATUS.match(st) and not lifted: + pn, ps = prev + if not COND.match(ps): + m = ASSIGN.match(ps) + guarded = re.search(r'\|\||&&', ps) + kind = None + if m and ('$(' in m.group('rhs') or '`' in m.group('rhs')): + if not guarded: + kind = 'decl' if m.group('decl') else 'bare' + elif SOURCEC.match(ps): + kind = 'source' + if kind: + flat = ' '.join(ps.split()) + rows.append(f"{f}:{n}: [{kind}] {flat[:60]} -> {' '.join(st.split())[:40]}") + prev = (n, st) + +for r in rows: + print(r) +PY +} + +# ---- what the controls DO and DO NOT cover --------------------------------- +# +# Each hole below was found by a reviewer, not by this file, so the list is +# written down: a count of zero from the tree means "not present" only for the +# syntax the controls actually exercise. +# +# COVERED (a control exists and is pinned by name or by kind): +# `;` as a separator bare_case / decl_case +# single- and double-quoted strings all controls +# `$( )` nesting, incl. quotes inside it sql_guarded_case / sql_bare_case +# a quoted string spanning several lines sql_bare_case (pinned by name) +# backslash-newline continuation source_continued_case, pinned on +# the path it sources +# a TRAILING comment, incl. one holding +# an apostrophe source_continued_case's own +# header comment carries one; if the +# scanner treats it as a quote the +# continuation pin goes red first +# (the swallow starts in that same +# header), and sql_bare with it +# the two ACCEPTED forms staying silent lifted_case / guarded_case / +# sql_guarded_case +# +# NOT COVERED — a status read hidden inside any of these is invisible here, and +# nobody has measured whether the tree contains one: +# backticks `cmd` instead of $( ) the scanner keys on `$(` and on a +# literal backtick in the RHS, but +# no control exercises a backtick +# spanning lines +# heredocs their body is scanned as ordinary +# text, so a `;` or a quote inside +# one can still split a statement +# `{ ...; }` and `( ... )` grouping treated as plain text +# arithmetic `$(( ))` and `(( ))` `$((` enters the `$(` stack and +# its `))` pops only one level +# `case` patterns' `;;` splits, which is harmless today +# but is not asserted anywhere +# `set -e` toggled inside a function or a +# subshell lifting is tracked file-wide, not +# per scope +# +# And one that is NOT a syntax gap, which is why the list above missed it: +# +# a BARE COMMAND OR FUNCTION CALL `f "$x"; rc=$?` is not looked at. +# followed by `rc=$?` -> #1034 The predecessor is only examined +# when it is an assignment with a +# command substitution, or a +# `source` — so the most ordinary +# producer of `$?` is skipped +# entirely. Under `set -e` such a +# call exits the shell before the +# status can be classified. Found by +# review, not by this file. The +# splitter is NOT the problem: it +# already returns the two statements +# correctly; the PREDICATE is what +# is narrow. Widening it, and +# counting what the tree then shows, +# is #1034 — deliberately not done +# here, because the count is unknown +# and would move the baseline. +# +# The two lists are different axes. Everything above the gap line enumerates +# SYNTAX the scanner may mis-split. The entry below it enumerates a SHAPE that +# produces `$?` at all — and that axis had never been written down, which is how +# the commonest shape of the three stayed invisible while the file looked +# thorough. +# +# Adding a control for one of these means moving it up, not deleting the line. +# +# ---- positive control: prove the scanner can still find each known-bad kind -- +control_dir="$(mktemp -d)" +trap 'rm -rf "$control_dir"' EXIT +cat > "$control_dir/control.sh" <<'CTL' +#!/usr/bin/env bash +set -e +bare_case() { + local out + out="$(some_command)" + rc=$? + [ "$rc" -eq 0 ] || return 1 +} +decl_case() { + local out="$(some_command)" + local rc=$? + [ "$rc" -eq 0 ] || return 1 +} +source_case() { + . "$dir/ops.sh" + rc=$? + [ "$rc" -eq 0 ] || return 1 +} +lifted_case() { # the accepted fix — must NOT be reported + set +e + out="$(some_command)" + rc=$? + set -e + [ "$rc" -eq 0 ] || return 1 +} +guarded_case() { # explicit control — must NOT be reported + out="$(some_command)" || rc=$? + [ "${rc:-0}" -eq 0 ] || return 1 +} +sql_guarded_case() { # the false positive that cost a workaround — must NOT + # be reported. The `;` sits inside a double-quoted SQL + # string that OPENS on one line and CLOSES on another. + local out rc=0 + out="$(sqlite3 :memory: " + SELECT json_extract(value,'$.pane_id') + FROM json_each('$j') + LIMIT 1;" 2>/dev/null)" || rc=$? + [ "$rc" -eq 0 ] || return 2 +} +source_continued_case() { # the review's second hole — a source split across a + # backslash-newline. MUST be reported: this is a + # single-line source with a line break in it, and a + # splitter that breaks there reports nothing while + # looking exactly like a clean tree. + local rc=0 + . \ + "$dir/continued-ops.sh" + rc=$? + [ "$rc" -eq 0 ] || return 1 +} +sql_bare_case() { # the same multi-line SQL shape, genuinely unguarded — + # MUST still be reported, or the splitter fix would have + # bought a false negative in place of a false positive. + local out + out="$(sqlite3 :memory: " + SELECT 1;" 2>/dev/null)" + rc=$? + [ "$rc" -eq 0 ] || return 2 +} +CTL +control="$(scan "$control_dir")" +# The multi-line SQL bare case must come back BY NAME, not merely by kind: the +# kinds are covered below, and what this proves is the other direction — that a +# quoted string spanning lines cannot swallow a real instance. +case "$control" in + *continued-ops.sh*) ;; + *) + echo "check-errexit-status-reads: positive control did not report the source" >&2 + echo "split across a backslash-newline. A continuation is ONE statement to" >&2 + echo "bash; a splitter that breaks there finds nothing and looks clean." >&2 + printf '%s\n' "$control" | sed 's/^/ /' >&2 + exit 2 ;; +esac +case "$control" in + *sql_bare_case*|*"SELECT 1"*) ;; + *) + echo "check-errexit-status-reads: positive control did not report the multi-line" >&2 + echo "SQL bare case; the splitter can be made to hide a real one." >&2 + printf '%s\n' "$control" | sed 's/^/ /' >&2 + exit 2 ;; +esac +for kind in bare decl source; do + case "$control" in + *"[$kind]"*) ;; + *) + echo "check-errexit-status-reads: positive control did not report [$kind]." >&2 + echo "The scanner cannot find a form it is supposed to find, so a count of" >&2 + echo "zero from the tree would mean nothing. Fix the scanner, not the tree." >&2 + printf '%s\n' "$control" | sed 's/^/ /' >&2 + exit 2 ;; + esac +done +# and the two correct forms must not be reported, or every fix would look like +# a defect and the baseline could never come down +for bad in lifted_case guarded_case sql_guarded_case; do + case "$control" in + *"$bad"*) + echo "check-errexit-status-reads: positive control reported $bad, which is the" >&2 + echo "accepted form. The scanner would flag the fix; that is not usable." >&2 + exit 2 ;; + esac +done + +# ---- the tree --------------------------------------------------------------- +if [ ! -d "$SCAN_DIR" ] || [ -z "$(find "$SCAN_DIR" -name '*.sh' -print -quit)" ]; then + echo "check-errexit-status-reads: no .sh files under $SCAN_DIR; this is not a clean tree." >&2 + exit 2 +fi + +listing="$(scan "$SCAN_DIR")" +if [ -z "$listing" ]; then + found=0 +else + found="$(printf '%s\n' "$listing" | wc -l | tr -d '[:space:]')" +fi + +baseline="$(tr -d '[:space:]' < "$BASELINE_FILE" 2>/dev/null || echo '')" +case "$baseline" in + ''|*[!0-9]*) + echo "check-errexit-status-reads: no readable baseline at $BASELINE_FILE" >&2 + exit 2 ;; +esac + +if [ "$found" -gt "$baseline" ]; then + echo "check-errexit-status-reads: $found status reads after an errexit decision, baseline is $baseline." >&2 + echo >&2 + printf '%s\n' "$listing" | sed 's/^/ /' >&2 + echo >&2 + echo "[bare] the shell dies at the assignment; the \$? line never runs." >&2 + echo "[decl] local/declare/export wins the status; \$? is ALWAYS 0." >&2 + echo "[source] bash 3.2 (macOS /bin/bash) dies here even with \`|| rc=\$?\`." >&2 + echo >&2 + echo "Lift errexit around it and restore it, as agmsg_terminal_load does:" >&2 + echo " case \$- in *e*) restore_e=1 ;; esac; set +e; x=\$(cmd); rc=\$?; [ \"\$restore_e\" = 1 ] && set -e" >&2 + exit 1 +fi + +if [ "$found" -lt "$baseline" ]; then + echo "check-errexit-status-reads: $found status reads after an errexit decision, below the baseline of $baseline." + echo "Lower the baseline in $BASELINE_FILE to $found so it cannot drift back up." + exit 1 +fi + +echo "check-errexit-status-reads: $found status reads after an errexit decision, at the baseline ($baseline)." diff --git a/.github/scripts/check-herdr-cli-routing.sh b/.github/scripts/check-herdr-cli-routing.sh new file mode 100755 index 000000000..e9ca6be8d --- /dev/null +++ b/.github/scripts/check-herdr-cli-routing.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# +# Fail when scripts/drivers/terminals/herdr/ops.sh calls the `herdr` CLI directly +# from a function that is not on the named allowlist, or when an allowlisted +# function's count of direct calls moves in EITHER direction. +# +# WHY. A herdr pane id may be qualified by the socket of the instance that owns +# it (`:wN:pX`, #1055). Every call ABOUT that pane must reach that +# socket, which `_herdr_cli ...` does by setting HERDR_SOCKET_PATH from the +# id. A call that bypasses `_herdr_cli` raises no error: it goes to whatever +# instance the ambient environment names, and a pane id is only unique inside +# one instance -- so the call lands on a different seat's live pane (measured +# 2026-09-11, the accident #1055 is about). One missed site is silent, and a +# review that reads twenty-one one-line edits will miss one. So this is +# counted by machine. +# +# HOW IT COUNTS. By CALL POSITION, not by text match: a line is a direct call +# only when `herdr ` stands where a command stands -- at the start +# of a statement, or right after `$(`, `if`, `!`, `&&`, `||`, `|`, `then`, +# `else`, `do`, with any `VAR=value` assignment prefixes in between (a call +# that sets HERDR_SOCKET_PATH by hand is the bypass this exists to catch). +# Comment lines are skipped before matching, and `herdr:` inside a +# message string never has a subcommand word after it. (An earlier count of +# these calls by plain grep answered 55 where the true number is 34: comments +# and prose matched. That count is what this script must not repeat.) +# +# THE ALLOWLIST is `.github/herdr-cli-routing-allowlist`: one ` ` +# per line, naming the functions whose direct calls are instance-wide or +# deliberately ambient (listing every pane of an instance, spawning into the +# caller's own instance, describing the backend). A function not listed with a +# direct call fails; a listed function whose count went UP fails; a count that +# went DOWN fails too, and says to lower the entry -- an allowlist that is +# stale in the low direction lets the next addition hide inside the old number. +# `_herdr_cli` itself is the one function that must call `herdr` directly. + +set -u +root="$(cd "$(dirname "$0")/../.." && pwd)" +ops="$root/scripts/drivers/terminals/herdr/ops.sh" +allow="$root/.github/herdr-cli-routing-allowlist" +[ -f "$ops" ] || { echo "check-herdr-cli-routing: $ops not found" >&2; exit 2; } +[ -f "$allow" ] || { echo "check-herdr-cli-routing: $allow not found" >&2; exit 2; } + +# function -> number of direct `herdr ` calls at command position +counts="$(awk ' + /^[A-Za-z_][A-Za-z0-9_]*\(\)/ { fn=$0; sub(/\(\).*/,"",fn); next } + /^[[:space:]]*#/ { next } + fn != "" { + line=$0 + # strip a trailing comment that begins after whitespace + sub(/[[:space:]]#.*$/,"",line) + n=0 + # a command boundary, then optional spaces, then `herdr ` + # An assignment prefix (VAR=value ...) before the word is still a command + # position: `HERDR_SOCKET_PATH="$sock" herdr pane list` is exactly the form a + # bypass of _herdr_cli takes, and it must be counted, not hidden by the prefix. + while (match(line, /(^|\$\(|[;|&(]|[[:space:]](if|then|else|do|!)[[:space:]])[[:space:]]*([A-Za-z_][A-Za-z0-9_]*=("[^"]*"|[^[:space:]"]*)[[:space:]]+)*herdr[[:space:]]+[a-z][a-z-]*/)) { + n++ + line=substr(line, RSTART+RLENGTH) + } + if (n) c[fn]+=n + } + END { for (f in c) printf "%s %d\n", f, c[f] }' "$ops" | sort)" + +status=0 +# every function with direct calls must be listed with exactly that count +while read -r fn n; do + [ -n "$fn" ] || continue + want="$(awk -v f="$fn" '$1==f {print $2}' "$allow")" + if [ -z "$want" ]; then + printf ' %s calls herdr directly %s time(s) and is not on the allowlist: route it through _herdr_cli , or add "%s %s" to %s with a reason in the commit\n' "$fn" "$n" "$fn" "$n" "${allow#"$root/"}" + status=1 + elif [ "$n" -gt "$want" ]; then + printf ' %s: %s direct herdr calls, allowlist says %s -- ABOVE: a new direct call; route it through _herdr_cli or raise the entry deliberately\n' "$fn" "$n" "$want" + status=1 + elif [ "$n" -lt "$want" ]; then + printf ' %s: %s direct herdr calls, allowlist says %s -- BELOW: lower the entry, or the next addition hides inside the old number\n' "$fn" "$n" "$want" + status=1 + fi +done <<< "$counts" +# every listed function must still exist with a nonzero count +while read -r fn want; do + case "$fn" in ''|'#'*) continue ;; esac + if ! printf '%s\n' "$counts" | grep -q "^$fn "; then + printf ' %s is on the allowlist with %s but makes no direct herdr call now -- remove the entry\n' "$fn" "$want" + status=1 + fi +done < "$allow" + +total="$(printf '%s\n' "$counts" | awk '{s+=$2} END {print s+0}')" +if [ "$status" -eq 0 ]; then + echo "check-herdr-cli-routing: $total direct herdr calls, all on the allowlist at their listed counts." +else + echo "check-herdr-cli-routing: FAILED (direct herdr calls outside _herdr_cli must be listed by name and count)." +fi +exit "$status" diff --git a/.github/scripts/check-unguarded-env-reads.sh b/.github/scripts/check-unguarded-env-reads.sh new file mode 100755 index 000000000..af56ec0b9 --- /dev/null +++ b/.github/scripts/check-unguarded-env-reads.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# +# Fail when `scripts/**/*.sh` grows a read of an ENVIRONMENT variable with no +# default, under shell options that make such a read fatal. +# +# WHY THIS IS STATIC, and not a test (#1129). Every entry point that reaches +# these files runs `set -euo pipefail` -- join.sh, actas-claim.sh, watch.sh, +# session-start.sh, inbox.sh and the rest. The bats suite does not: measured, +# `tests/test_helper.bash` sets no shell options at all, so a read that is only +# fatal under `-u` cannot fire inside a test. +# +# That was measured, not assumed. Forcing `set -u` into the shared test helper +# and running all 102 suites produced 8 reds, and NONE of them was a defect in +# `scripts/`: five were tests reading their own undefined variables, one was an +# artefact of where the option was placed, and two did not reproduce in +# isolation. Then the method was CALIBRATED against the one confirmed defect of +# this class -- #1126, `sock="${TMUX%%,*}"` with no default, which killed the +# tmux label search in a subshell whose caller discarded stderr. With `set -u` +# forced, `test_terminal_registry` on the tree that still contained it was +# 129/129 GREEN. The suite cannot see this class, so the guard has to be static. +# +# WHAT COUNTS AS A FINDING +# a read of an ALL-CAPS name that +# - is never assigned anywhere in the same file (so it comes from the +# environment or from a caller's export), and +# - is read without a default: `$NAME`, `${NAME}`, `${NAME%%,*}`, and +# - is not already guarded, in the same function, by an earlier +# `[ -n "${NAME:-}" ]` / `[ -z "${NAME:-}" ]`. +# +# The guard clause is what lets this go DOWN as well as up, and it was the +# difference between a calibrated check and a decorative one. Measured on three +# trees: +# +# 040a4c7 before #1112 introduced the defect 89 +# 07d76ef with the defect present 90 <- the new row is the defect +# b3fe77d after #1126 guarded it 89 +# +# Without the guard clause the count was 147 / 147 / 147: it would not have +# moved when the defect landed OR when it was fixed. +# +# TWO WAYS TO BURN ONE DOWN, and they are not equal. Giving the read a default +# (`${NAME:-}`) is the one that also makes the code correct wherever it runs. +# Adding a `[ -n "${NAME:-}" ]` guard in the same function is accepted here +# because it is what the tmux driver's own `terminal_detect` does and it makes +# an honest refusal -- but it only protects the reads BELOW it in that function. +# +# WHAT THIS DOES NOT SEE, said plainly so nobody reads a pass as a promise: +# - a variable assigned somewhere in the file and read before that line +# - a guard in a CALLER rather than in the same function +# - `${NAME:-}` used where a missing value is not actually acceptable +# The baseline is a COUNT, so it certifies nothing about the rows already in it. + +set -u + +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +# Overridable so the checker can be exercised against a fixture tree. A guard +# that can only be run against the real, already-clean tree has never been shown +# to fire. +BASELINE_FILE="${AGMSG_ENV_READS_BASELINE:-$ROOT/.github/unguarded-env-reads-baseline}" +SCAN_DIR="${1:-$ROOT/scripts}" + +count_and_list() { + python3 - "$1" <<'PY' +import re, sys, pathlib +root = pathlib.Path(sys.argv[1]) +# Op group: defaults (:- :+ :? / - + ?) plus assignment (:= / =). +# :- :+ :? skip this read; they are not assignments (#1197). +READ = re.compile(r'\$\{([A-Z][A-Z0-9_]*)(:[-+?=]|[-+?=])?[^}]*\}|\$([A-Z][A-Z0-9_]*)\b') +GUARD = re.compile(r'\[\s+-[nz]\s+"\$\{([A-Z][A-Z0-9_]*):-[^}]*\}"') +FUNC = re.compile(r'^([A-Za-z_][A-Za-z0-9_]*)\(\)\s*\{') +# Shell-provided or set-by-the-OS-everywhere: reading these unguarded is not the +# hazard this looks for. +SPECIAL = {'BASH_SOURCE','FUNCNAME','BASH_REMATCH','PIPESTATUS','OPTARG','OPTIND', + 'RANDOM','LINENO','SECONDS','BASHPID','BASH_VERSINFO','EUID','UID','PPID', + 'HOSTNAME','OSTYPE','MACHTYPE','SHLVL','REPLY','IFS','PATH','HOME','PWD', + 'TMPDIR','USER','SHELL','LANG','LC_ALL','TERM','COLUMNS','LINES','EDITOR'} +rows = [] +for f in sorted(root.rglob('*.sh')): + txt = f.read_text(errors='replace') + guarded = set() + for i, line in enumerate(txt.splitlines(), 1): + if FUNC.match(line): + guarded = set() + elif line == '}': + guarded = set() + s = line.strip() + if s.startswith('#'): + continue + for g in GUARD.finditer(line): + guarded.add(g.group(1)) + for m in READ.finditer(line): + name = m.group(1) or m.group(3) + op = m.group(2) + if not name or name in SPECIAL or op or name in guarded: + continue + # NAME= and NAME:= (the latter is ${NAME:=...} / ${NAME=...}). + if re.search(r'(?&2 + exit 2 +fi + +listing="$(count_and_list "$SCAN_DIR")" +if [ -z "$listing" ]; then + found=0 +else + found="$(printf '%s\n' "$listing" | wc -l | tr -d '[:space:]')" +fi + +baseline="$(tr -d '[:space:]' < "$BASELINE_FILE" 2>/dev/null || echo '')" +case "$baseline" in + ''|*[!0-9]*) + echo "check-unguarded-env-reads: no readable baseline at $BASELINE_FILE" >&2 + exit 2 ;; +esac + +if [ "$found" -gt "$baseline" ]; then + echo "check-unguarded-env-reads: $found unguarded environment reads, baseline is $baseline." >&2 + echo >&2 + printf '%s\n' "$listing" | sed 's/^/ /' >&2 + echo >&2 + echo "Every entry point that reaches these files runs 'set -euo pipefail', so a" >&2 + echo "read with no default kills the shell -- and inside a command substitution" >&2 + echo "whose caller discards stderr, it does so silently. Give the read a default" >&2 + echo "(\${NAME:-}), or guard the function with [ -n \"\${NAME:-}\" ] and refuse." >&2 + echo "The test suite cannot catch this: it runs with no shell options (#1129)." >&2 + exit 1 +fi + +echo "check-unguarded-env-reads: $found unguarded environment reads, at the baseline ($baseline)." diff --git a/.github/scripts/run-bats-timed.sh b/.github/scripts/run-bats-timed.sh new file mode 100755 index 000000000..e55013942 --- /dev/null +++ b/.github/scripts/run-bats-timed.sh @@ -0,0 +1,78 @@ +#!/usr/bin/env bash +# Run one bats file at a time and append machine-readable wall-clock timings. +# +# Usage: run-bats-timed.sh +# +# The TSV is intentionally append-only while the suite runs. If the job reaches +# its wall-clock cap, every completed file remains useful evidence instead of +# disappearing with the unfinished bats invocation. Files from several runs can +# be concatenated directly: the run metadata is repeated on every row. +set -u + +usage() { + echo "usage: ${0##*/} " >&2 + exit 2 +} + +[ "$#" -eq 2 ] || usage +manifest="$1" +timings="$2" +[ -s "$manifest" ] || { echo "${0##*/}: empty or missing manifest: $manifest" >&2; exit 1; } + +run_id="${GITHUB_RUN_ID:-local}" +run_attempt="${GITHUB_RUN_ATTEMPT:-1}" +sha="${GITHUB_SHA:-unknown}" +runner_os="${RUNNER_OS:-unknown}" +shard="${SHARD:-unknown}" +shard_total="${SHARD_TOTAL:-unknown}" + +printf 'schema\trecord\trun_id\trun_attempt\tsha\tos\tshard\tshard_total\tfile\tstarted_at\tended_at\telapsed_seconds\tstatus\n' > "$timings" + +suite_started_epoch="$(date -u +%s)" +suite_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)" +suite_status=0 +completed=0 + +finish() { + rc=$? + suite_ended_epoch="$(date -u +%s)" + suite_ended_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + suite_elapsed=$((suite_ended_epoch - suite_started_epoch)) + [ "$suite_status" -ne 0 ] || suite_status="$rc" + printf '1\tshard\t%s\t%s\t%s\t%s\t%s\t%s\t-\t%s\t%s\t%s\t%s\n' \ + "$run_id" "$run_attempt" "$sha" "$runner_os" "$shard" "$shard_total" \ + "$suite_started_at" "$suite_ended_at" "$suite_elapsed" "$suite_status" >> "$timings" + echo "bats timing: shard $shard/$shard_total completed $completed file(s) in ${suite_elapsed}s (status $suite_status)" +} +trap finish EXIT +stop() { + suite_status=143 + exit 143 +} +trap stop INT TERM + +while IFS= read -r file <&3; do + [ -n "$file" ] || continue + started_epoch="$(date -u +%s)" + started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + printf '1\tfile_start\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t-\t-\t-\n' \ + "$run_id" "$run_attempt" "$sha" "$runner_os" "$shard" "$shard_total" \ + "$file" "$started_at" >> "$timings" + echo "bats timing: start $file at $started_at" + + bats --print-output-on-failure "$file" > "$timings" + echo "bats timing: end $file at $ended_at (${elapsed}s, status $status)" + + completed=$((completed + 1)) + [ "$status" -eq 0 ] || suite_status="$status" +done 3< "$manifest" + +exit "$suite_status" diff --git a/.github/scripts/shard-tests.sh b/.github/scripts/shard-tests.sh index 13db2f598..914a363b5 100755 --- a/.github/scripts/shard-tests.sh +++ b/.github/scripts/shard-tests.sh @@ -12,86 +12,61 @@ # the directory is assigned to exactly one shard, so the union of all shards is # always the whole suite (asserted by tests/test_ci_sharding.bats). # -# Balancing is by @test count, greedy longest-processing-time first, rather -# than by file count: the suite's files differ by more than an order of -# magnitude in size, so splitting on names alone would leave one shard doing -# most of the work and cap the speedup at whatever that shard costs. -# -# Test count is a proxy for runtime, and a loose one — measured on macOS, the -# whole suite is 860s and the count-balanced quarters (197/196/197/197 tests) -# come out at 125s/298s/366s/71s. Per-test cost varies from ~0.0s to ~8s -# depending on how much a file forks or waits. So the real speedup here is -# 860s -> 366s (~2.4x), not 4x. -# -# It is still the right weight to ship first. The alternative, a checked-in -# table of measured per-file seconds, buys ~150s more but goes stale silently: -# it would be wrong the moment the fixed `sleep`s in the suite are replaced by -# condition polling, which is the very next CI change queued. Weights are worth -# revisiting once runtimes stop moving. Note the floor either way is the -# slowest single file (test_spawn.bats, 199s) — no split beats that, so the -# ceiling on this approach is ~4.5x, not 4x-and-then-some. -# -# Whatever the weights, the property that matters is coverage, not balance: the -# worst case of a bad weight is an unevenly filled shard, never a missing file. -# -# --- Pinned-apart files (#847, #848) --------------------------------------- -# -# @test count is a loose proxy for runtime in general (above), but for a -# specific shape of file it is not loose, it is blind: a file whose cost is -# almost entirely waiting (background processes, poll loops with -# hundred-plus-iteration bounds) rather than how many @test blocks it -# contains can carry a tiny weight here while dominating its shard's actual -# wall clock. A file that is merely large -- many @test blocks, ordinary -# per-test cost -- is NOT this case; count already weights it correctly, and -# it is not pinned. -# -# Measured 2026-08-19 on a green main run (head 626a625b, run 32193147987) by -# correlating each `ok N ` line's own GitHub Actions timestamp against -# which file's `@test` block that description belongs to, then ranking every -# file by seconds-per-test rather than by raw duration (raw duration alone -# does not distinguish "slow because few tests wait a long time" from "slow -# because there are simply many tests", and only the former is what count -# weighting misses): -# -# tests/test_remote_engine_start_refusal.bats 722s / 9 tests = ~80s/test -# tests/test_remote_status_liveness.bats 380s / 31 tests = ~12s/test -# -# against a whole-suite per-test cost this script's own header already says -# runs ~0.0s-8s. Both are 1.5x-10x above that ceiling on a low test count, so -# both rank near the bottom of the count-weighted sort while carrying some of -# the largest absolute durations in the suite. (Files that are merely large in -# absolute terms -- e.g. a 179-test file at a very ordinary ~1.2s/test -- were -# checked and excluded: their weight already reflects their real cost.) -# -# #847's own trigger was exactly this class of file landing next to another -# heavy one purely because an unrelated 15-test addition elsewhere repacked -# the partition — the count weight cannot tell "heavy because slow" from -# "heavy because voluminous", so nothing stops two slow-but-few-tests files -# from drifting onto the same shard as the tree changes shape. Pinning these -# apart, in fixed shard slots decided before the ordinary weighted pass runs, -# means no future change to any OTHER file's test count can put two of them -# together again — that was the actual, demonstrated failure, not merely a -# theoretical one. -# -# This does not bound a shard's total duration: the heavier entry above -# (722s) is heavy enough on its own that no repacking of the rest of the -# suite moves its shard's floor by much. See tests.yml's bats-shard -# timeout-minutes for the ceiling this is paired with, sized to cover that -# floor plus a fair share of everything else with real margin. And a file NOT -# on this list can still turn out to be similarly disproportionate and land -# next to another one by chance — nothing here detects that case in general, -# only these two measured instances of it. Revisit alongside the counting -# scheme itself once the concurrent effort to shorten these files (tracked -# separately from #847/#848) lands and the numbers above are stale. -# -# FOLLOW-UP: whichever of that effort's PRs (#876 et al.) touches either file -# named below changes its real cost, possibly enough to make pinning it -# pointless or to make some other, currently-unremarkable file the next -# hidden outlier. Re-run this script's own measurement method (correlate a -# green run's `ok N` timestamps against each file, rank by seconds-per-test) -# on main once that work lands, and drop or replace entries here based on -# what it says then — this list is not meant to be permanent. The 30-minute -# job cap in tests.yml is a separate decision and does not depend on this one. +# Balancing is by each file's real measured wall time, greedy +# longest-processing-time first, rather than by file count or @test count: +# the suite's files differ by more than an order of magnitude in cost, so +# splitting on names alone would leave one shard doing most of the work and +# cap the speedup at whatever that shard costs, and @test count is only a +# loose proxy for that cost (a file whose time is almost entirely waiting -- +# background processes, poll loops -- can carry a tiny count while +# dominating its shard's wall clock; see #847/#1243 below for the measured +# instance of this that motivated moving off count entirely). +# +# Real time comes from .github/scripts/bats-file-seconds.tsv, a checked-in +# table of per-file macOS wall seconds measured from a real CI run (see that +# file's own header for which run and how to refresh it). A file with no row +# in the table -- renamed, or added since the last measurement -- is +# estimated at the table's own average seconds/file, never at zero: coverage +# (every file lands in exactly one shard) does not depend on the table being +# current, only balance does, and an unmeasured file at the average is a far +# better guess than an unmeasured file at zero. +# +# Whatever the weights, the property that matters MOST is coverage, not +# balance: the worst case of a bad weight is an unevenly filled shard, never +# a missing file. +# +# --- Pinned-apart files (#847, #848, #1243) --------------------------------- +# +# Two files are additionally seeded into FIXED, distinct shard slots before +# the ordinary weighted pass runs, rather than simply sorting into place by +# their (now real-time) weight like everything else: +# +# test_remote_engine_start_refusal.bats +# test_remote_status_liveness.bats +# +# Real-time weighting already prevents either from dominating its shard's +# wall clock unnoticed -- that was #847's original problem, and it is what +# @test-count weighting could not see. What fixed-slot seeding buys ON TOP of +# that is a STRUCTURAL guarantee, independent of whatever else the tree looks +# like on a given day: these specific two files can never land on the same +# shard, at any shard total >= 2 (asserted by tests/test_ci_sharding.bats). +# Without it, two files that happen to be the two heaviest overall could in +# principle still be placed together by an unlucky greedy pass at a low +# shard total -- fixed slots rule that out by construction rather than by +# probability. +# +# This seeding does NOT exclude these two shards from the rest of the +# weighted pass (#1243 revision): earlier this pinned a shard by excluding it +# outright, which produced a *correctness* guarantee (never refilled) at the +# cost of *balance* -- on the merged (2550 @test) tree, forcing all +# non-pinned work onto only 3 of 5 shards floored the worst case at ~29 +# minutes of the 30-minute cap, regardless of how well those 3 were balanced +# (5243s of non-pinned macOS work over 3 shards = ~1748s = 29.1min each, even +# under perfect packing). Letting the other files fill a pinned shard's +# remaining real-time budget -- exactly like any other shard -- brings every +# shard to ~20.8 minutes on that same measured tree, because the two pinned +# files' own real costs (660s, 346s) are ordinary-sized next to the whole +# suite's 6249s once every file is weighted in the same unit. # # Matched by basename, not by the `$dir`-relative path `files` below uses, so # the pin still resolves when this script is invoked against a different @@ -127,21 +102,56 @@ case "$total" in ''|*[!0-9]*) usage ;; esac files="$(find "$dir" -maxdepth 1 -name '*.bats' | LC_ALL=C sort)" [ -n "$files" ] || { echo "${0##*/}: no .bats files under $dir" >&2; exit 1; } -# Weight each file by its number of test cases. `grep -c` exits 1 on no match -# after printing 0, which set -e would otherwise treat as fatal. -file_weight() { - local n - n="$(grep -c '^[[:space:]]*@test' "$1" || true)" - [ -n "$n" ] || n=0 - printf '%s' "$n" +# The checked-in table of measured per-file macOS seconds this script weights +# by. Overridable so this script's own tests can point at a fixture table +# without touching the real one. +SECONDS_TABLE="${SHARD_TESTS_SECONDS_TABLE:-$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)/bats-file-seconds.tsv}" + +# The table's own average seconds/file, used below as every unmeasured file's +# estimate. Computed from the table itself (not a second hardcoded number) +# so it can never silently drift out of step with the table it is meant to +# summarize; nearest-integer rounding via the usual awk `(x+0.5)` idiom. +# Falls back to a small positive constant if the table is missing or empty +# so an unmeasured file is still weighted something, never zero or an error. +avg_seconds=1 +if [ -r "$SECONDS_TABLE" ]; then + avg_seconds="$(awk -F'\t' ' + $0 !~ /^#/ && NF == 2 { sum += $2; n++ } + END { if (n > 0) printf "%d", (sum / n) + 0.5; else print 1 } + ' "$SECONDS_TABLE")" + [ -n "$avg_seconds" ] || avg_seconds=1 +fi + +# Weight a file by its own measured real seconds from the table, matched by +# basename (see the header comment above on why basename, not path). A file +# absent from the table -- renamed, or added since the measurement -- is +# estimated at the table's average rather than treated as zero-cost or an +# error: this function must never fail or omit a value, since every file in +# the tree passes through it once and a missing weight would silently drop +# that file from the balance pass (not from coverage, which does not use +# this value at all). +file_seconds() { + local base line + base="$(basename "$1")" + if [ -r "$SECONDS_TABLE" ]; then + line="$(awk -F'\t' -v b="$base" '$1 == b { print $2; exit }' "$SECONDS_TABLE" 2>/dev/null || true)" + if [ -n "$line" ]; then + printf '%s' "$line" + return + fi + fi + printf '%s' "$avg_seconds" } # Seed the pinned files into distinct shards first, in PINNED_APART's own -# (measured-heaviest-first) order — not the order they happen to sort in -# below, which is by count and is exactly the metric these files defeat. Each -# consumes one shard slot (wrapping if there are more pinned files than -# shards); everything else is decided by the ordinary weighted pass afterward, -# which never reconsiders a file placed here. +# order (each consumes one shard slot, wrapping if there are more pinned +# files than shards) -- not sorted into place by weight like everything +# else, because their whole point is a STRUCTURAL "never share a shard" +# guarantee (see the header comment), which sorting cannot provide even +# though real-time weighting alone now keeps either from dominating its +# shard unnoticed. `load` starts seeded with each pinned file's own real +# cost; the ordinary weighted pass below is free to add more to these same +# shards; it does not skip them. i=0 while [ "$i" -lt "$total" ]; do load[i]=0 @@ -163,7 +173,7 @@ $files EOF [ -n "$match" ] || continue s=$((slot % total)) - load[s]=$((load[s] + $(file_weight "$match"))) + load[s]=$((load[s] + $(file_seconds "$match"))) if [ "$s" -eq "$((index - 1))" ]; then printf '%s\n' "$match" fi @@ -171,14 +181,14 @@ EOF slot=$((slot + 1)) done -# Weight every remaining (non-pinned) file the same way as before. +# Weight every remaining (non-pinned) file by its own real seconds. weighted="" while IFS= read -r f; do [ -n "$f" ] || continue case "$pinned_paths" in *" $f "*) continue ;; esac - weighted="${weighted}$(file_weight "$f") ${f} + weighted="${weighted}$(file_seconds "$f") ${f} " done <... +# +# File percentiles are nearest-rank values across every supplied run. Shard +# headroom is kept per run and OS so a fast sample cannot hide a slow sibling. +set -euo pipefail + +timeout=1800 +if [ "${1:-}" = --timeout-seconds ]; then + timeout="${2:-}" + shift 2 +fi +case "$timeout" in ''|*[!0-9]*) echo "${0##*/}: timeout must be seconds" >&2; exit 2 ;; esac +[ "$#" -gt 0 ] || { echo "usage: ${0##*/} [--timeout-seconds N] ..." >&2; exit 2; } + +tmp="$(mktemp -d "${TMPDIR:-/tmp}/agmsg-bats-timings.XXXXXX")" +cleanup() { + rm -f "$tmp/files" "$tmp/shards" + rmdir "$tmp" 2>/dev/null || true +} +trap cleanup EXIT + +awk -F '\t' '$1 == 1 && $2 == "file_end" && $13 == 0 { print $6 "\t" $9 "\t" $12 }' "$@" \ + | LC_ALL=C sort -t ' ' -k1,1 -k2,2 -k3,3n > "$tmp/files" + +printf 'record\tos\tfile\tsamples\tp50_seconds\tp95_seconds\tmax_seconds\n' +awk -F '\t' ' + function emit( p50,p95) { + if (!n) return + p50 = int((n + 1) / 2) + p95 = int((95 * n + 99) / 100) + printf "file\t%s\t%s\t%d\t%d\t%d\t%d\n", os, file, n, value[p50], value[p95], value[n] + } + { + key = $1 FS $2 + if (last != "" && key != last) { emit(); delete value; n=0 } + os=$1; file=$2; value[++n]=$3; last=key + } + END { emit() } +' "$tmp/files" + +awk -F '\t' '$1 == 1 && $2 == "shard" { print $3 "\t" $4 "\t" $6 "\t" $7 "\t" $12 "\t" $13 }' "$@" \ + | LC_ALL=C sort -t ' ' -k1,1 -k2,2n -k3,3 -k5,5nr > "$tmp/shards" + +printf 'record\trun_id\trun_attempt\tos\tmax_shard\tmax_seconds\theadroom_seconds\tstatus\n' +awk -F '\t' -v timeout="$timeout" ' + { + key=$1 FS $2 FS $3 + if (!(key in seen)) { + seen[key]=1 + printf "run\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n", $1, $2, $3, $4, $5, timeout-$5, $6 + } + } +' "$tmp/shards" diff --git a/.github/unguarded-env-reads-baseline b/.github/unguarded-env-reads-baseline new file mode 100644 index 000000000..1479e19b5 --- /dev/null +++ b/.github/unguarded-env-reads-baseline @@ -0,0 +1 @@ +65 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 544c5896c..1bf642af5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -104,7 +104,25 @@ concurrency: env: # Number of parallel bats shards per OS. The matrix and the shard helper must # stay in lockstep; the stable summary job below verifies that they do. - SHARD_TOTAL: 4 + # + # 4 -> 5 (#1107). The suite outgrew a 4-shard partition and a 4-shard macOS + # leg was reaching the 30-min cap while still passing tests (a slow shard, + # not a hang). Five, not six, because the account runs at most FIVE macOS + # jobs at once (the concurrency header): a sixth would queue behind the + # other five of its own run, inflating the wall clock past what a lighter + # shard buys. + # + # shard-tests.sh weights every file (pinned or not) by its own real measured + # macOS seconds from .github/scripts/bats-file-seconds.tsv (#1243) rather + # than @test count or a count-equivalent fit -- count was never more than a + # loose proxy for a file's real cost, and two prior fits against it (#847, + # then a #1243 predecessor that excluded pinned shards outright) each + # eventually drifted or traded balance away as the suite grew. Re-measure + # that table (its own header says how) when the suite's shape has moved + # enough that tests/test_ci_sharding.bats's balance-ratio assertion goes + # red, the same way #1107 itself was tracked: a constant sized against an + # older system stays right only as long as the system does. + SHARD_TOTAL: 5 # The Windows legs' sqlite3, pinned three ways (#824). # # This comment used to end "one trip to chocolatey, which is the only time @@ -249,7 +267,7 @@ jobs: # the required contexts are `bats` (the aggregate) and the Windows install # leg, and neither is renamed, so branch protection sees exactly what it # saw. `needs` is one of the contexts a job name may read. - name: bats (${{ matrix.os }} ${{ matrix.shard }}/4)${{ needs.changes.outputs.docs_only == 'true' && ' — docs-only, suite skipped' || '' }} + name: bats (${{ matrix.os }} ${{ matrix.shard }}/5)${{ needs.changes.outputs.docs_only == 'true' && ' — docs-only, suite skipped' || '' }} needs: changes # Run even if `changes` somehow failed/was skipped — fail open to the full # suite rather than leaving this REQUIRED check unreported (which would @@ -283,7 +301,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - shard: [1, 2, 3, 4] + shard: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v4 @@ -377,9 +395,11 @@ jobs: - name: Run bats suite (this shard) if: needs.changes.outputs.docs_only != 'true' + env: + SHARD: ${{ matrix.shard }} run: | set +e - xargs bats --print-output-on-failure < shard-files.txt + .github/scripts/run-bats-timed.sh shard-files.txt "$RUNNER_TEMP/bats-timings.tsv" status=$? : > "$RUNNER_TEMP/bats-done" exit $status @@ -437,6 +457,16 @@ jobs: if-no-files-found: ignore retention-days: 3 + - name: Upload bats timings + if: always() && needs.changes.outputs.docs_only != 'true' + continue-on-error: true + uses: actions/upload-artifact@v4 + with: + name: bats-timings-${{ matrix.os }}-${{ matrix.shard }} + path: ${{ runner.temp }}/bats-timings.tsv + if-no-files-found: warn + retention-days: 30 + - name: Record which files this shard ran if: needs.changes.outputs.docs_only != 'true' uses: actions/upload-artifact@v4 @@ -680,6 +710,18 @@ jobs: # differently there than on the hosts that were measured. The engine's # own bats leg is ubuntu and macos only, so this selects the two tests # that would notice and runs them on the platform in question. + # The codex hook wrapper (#1015). The defect is a Windows-only path -- a + # PowerShell one-liner that invokes Git Bash as a LOGIN shell, so the + # profile's output lands on the stdout codex parses -- and the fix + # moves the payload off that stdout. The bash half is bash's + # everywhere and is proved on the POSIX legs; the PowerShell half is + # asserted as a string there because nothing on those hosts can run + # it. This leg is the only place it is EXECUTED. Without it the fix + # ships to Windows having never run on Windows. + - leg: codex hook wrapper (#1015) + filter: "windows-wrapper" + target: tests/test_delivery.bats + sqlite: true - leg: driver input (#817) filter: "driver-input" target: tests/test_remote_sync_driver_input.bats @@ -1114,6 +1156,44 @@ jobs: - name: No new assertion that cannot fail run: .github/scripts/check-enforced-assertions.sh + errexit-status-reads: + name: errexit status reads + runs-on: ubuntu-latest + timeout-minutes: 5 + # Unconditional for the same reason as `enforced-assertions` above: a job + # that skips on some diffs is a required context that can sit pending. It + # is a static read of `scripts/**/*.sh`. + steps: + - uses: actions/checkout@v4 + + # Ubuntu's bash is 5.x, and one of the three shapes this looks for is + # fatal ONLY on bash 3.2 (macOS /bin/bash). That is why the check is + # static: running it under one interpreter would miss the shape that + # kills the other. The behaviour was measured on both (see the header); + # what runs here is the count. + - name: No status read that errexit already decided + run: .github/scripts/check-errexit-status-reads.sh + + unguarded-env-reads: + name: unguarded env reads + runs-on: ubuntu-latest + timeout-minutes: 5 + # Unconditional, like its two siblings above: a job that skips on some + # diffs is a required context that can sit pending. It is a static read of + # `scripts/**/*.sh`. + steps: + - uses: actions/checkout@v4 + + # The suite cannot do this one. Every entry point that reaches those + # files runs `set -euo pipefail`; tests/test_helper.bash sets no shell + # options, so a read that is only fatal under `-u` never fires in a test. + # Measured (#1129): with `set -u` forced into the helper, all 102 suites + # produced 8 reds and none was a defect in scripts/ -- and the suite was + # 129/129 green on the tree that still held the one confirmed defect of + # this class (#1126). + - name: No unguarded environment read + run: .github/scripts/check-unguarded-env-reads.sh + private-names: name: internal names runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 8745b97f1..d8e891f2e 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ teams/ .env .env.* /.secrets/ +run/ diff --git a/CHANGELOG.md b/CHANGELOG.md index fffd5eaba..4bc4c4109 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,55 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.3.0] - 2026-09-14 + +### Added +- Terminal drivers for tmux, herdr, and plain OS terminal windows: `peek`, `poke`, and `arrange` work through whichever terminal a member runs in (#1014, #1060, #1078, #1163, #1207) +- `peek` with no member name summarizes the screens of the whole local team (#1194) +- `where` reports this session's terminal, its pane, and the operations that terminal supports; Claude Code sessions are told their terminal at start (#1082, #1173, #1224) +- `team` shows, for each teammate, whether `peek`, `poke`, and `arrange` can reach it, and why not when they cannot (#1232) +- `fix`: a session proves which pane it is running in and repairs only its own name and placement records (#1152, #1157, #1188, #1191, #1227) +- A session names its own pane when it acts, and `spawn` names the pane it created and sets its agent key (#1065, #1080, #1099) +- `send` accepts `--body-file ` and `--body -` (stdin) like `poke`, and refuses a body that looks like a flag (#1101) +- Antigravity (`agy`): the skill is installed where `agy` discovers skills, and monitor delivery with safe automatic resume is available on Linux (#1090, #1223) +- Report a mark-read that lost to a concurrent writer (#1011) (#1013) +- Deliver the inbox mid-turn via a PostToolUse hook, not only at Stop (#1003) (#1004) + +### Changed +- `team` is read-only: the leader-side repair options `--fix`, `--fix-pane-names`, and `--rename-sessions` are removed; each session repairs itself with `fix` (#1152, #1211, #1213) +- actas lock, readiness, and spawn records are keyed by team and member id, so team or member names containing `__` no longer collide (#1023) +- Agent type detection follows an explicit priority, and `GEMINI_API_KEY` on its own is only a last resort (#1247) +- Per-driver notes live at `scripts/drivers/terminals//README.md`; `install --update` removes the old `SKILL.md` copies (#1248) + +### Fixed +- Codex seats that hold an actas lock no longer drop out of their own bridge's delivery, and spawned seats do not inherit it (#1239, #1245) +- Claude Code's `actas` and `drop` instructions again start the watcher through the Monitor tool and confirm it attached (#1237) +- `spawn` skips the readiness wait only when delivery is explicitly off or turn (#647) +- A graceful `despawn` waits for the pane to close, not only for the lock (#1097) +- herdr socket paths containing `:` round-trip through placement records (#1166) +- `watch` and `inbox` deliver a backlog past the argument-length limit instead of failing silently (#777, #1045) +- `check-inbox` consumes rows only after the payload is written (#1026) +- The Windows hook payload stays off the login shell's stdout (#1015) +- Antigravity's delivery plug parses under bash 3.2 (#1244) +- Pass non-remote subcommands through (#590) +- The remaining argv-to-stdin sites, plus a Windows CRLF row-separator bug (#991) +- Give the identity lease a start token on Windows (#978) +- One wake, one turn — attribute mid-start turn-end signals by turn id (#889) +- Discover a ws:// app-server in the SessionStart plug (#1057) +- Refuse a non-string envelope.blob/cipher instead of storing "null" (#1042) (#1048) +- Count roster mutations as held so the read frontier can advance (#968) (#986) +- A store at the current schema revision skips init's write batch (#1001) (#1010) +- Probe before the pipe, not inside it (#462) (#904) +- Widen _wait_pidfile's window and name what it saw (#595) (#797) +- Stand down instead of an unfiltered watcher when a resumed seat is unidentified (#982) (#993) +- Refuse to guess between multiple installs on --update with no --cmd (#659) +- Write Codex writable_roots through a symlinked config.toml instead of replacing the link (#747) (#995) +- Stop the apply failure path from discarding the shell's stderr (#974) +- Stand down when the installation is updated underneath (#963) (#965) + +### Performance +- Batch the rollout mtime scan (#1035) (#1037) + ## [1.2.3] - 2026-08-22 ### Added @@ -144,7 +193,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Narrow a claim I disproved myself two commits later - Delete a comment describing the control this one replaced - Point key.sh at the current design, and stop asserting the move has happened -- Move the connect onboarding design to agmsg-cloud, and drop labels that point at nothing - Make the promise about this document weak enough to be true - Label what stands behind each claim, instead of asserting they all cite - Add a Japanese translation, and consolidate the derivation it mirrors @@ -456,7 +504,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support spawning into herdr panes (#495) - Drag files onto a pane to insert their path (#481) - Adaptive catch-up so a backlog doesn't crawl at 100/5s -- Add team-list.sh (agmsg team list --json, koit-approved) +- Add team-list.sh (agmsg team list --json) - Add status --json and pending list/abort (ADR 0007 addendum) - Consume connected team credentials - Add scripts/remote.sh (connect/status/disconnect/doctor) per ADR 0007 @@ -491,10 +539,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Resolve symlinks before trampoline compare; doctor checks node - Detect the macOS CLT python3 trampoline, not just PATH presence - Close the python3 dependency-tiering gap on the remote path -- Close the integer-overflow bypass in AGMSG_TEAM_LIST_MAX_TEAMS validation (co1 delta review round 2) -- Validate AGMSG_TEAM_LIST_MAX_TEAMS as a positive integer (co1 delta review) +- Close the integer-overflow bypass in AGMSG_TEAM_LIST_MAX_TEAMS validation +- Validate AGMSG_TEAM_LIST_MAX_TEAMS as a positive integer - Fail closed on incompleteness; shrink v1 schema -- Wire 'agmsg team list' into actual dispatch entry points (co1 P1) +- Wire 'agmsg team list' into actual dispatch entry points - Stop binding config JSON via .param set (#87-class tokenizer bug) - Hide imported identity at TTY - Separate token input from E2EE prompts @@ -529,15 +577,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Read messages from the event log too, not just the legacy table - Assert against the event log, not the legacy messages table - Escape interpolated names in rename/rename-team SQL (#223, #87) -- Jsonl compact keys reads by tuple, not a space-join (co1 #221) +- Jsonl compact keys reads by tuple, not a space-join (#221) - Make the jsonl driver parse under macOS bash 3.2 (#207, #221 CI) -- Jsonl mark aborts on a failed existing-reads scan (co1 #207 residual) -- Jsonl driver must not swallow failures as ok (co1 #207 review) +- Jsonl mark aborts on a failed existing-reads scan (#207) +- Jsonl driver must not swallow failures as ok (#207) - Watch-once stale-wake token = unread-set digest, not a max id (#207) -- Document --limit semantics + make storage_history agent truly optional (co1 #206 review) -- Export skips unknown event types; pin high-water with a tail-duplicate test (co1 #205 review) -- Describe is a metadata op; surface backend errors; chronological reads (co1 re-review, #204) -- Legacy read, pipefail framing, §1.4 control ops (co1 review, #204) +- Document --limit semantics + make storage_history agent truly optional (#206) +- Export skips unknown event types; pin high-water with a tail-duplicate test (#205) +- Describe is a metadata op; surface backend errors; chronological reads (#204) +- Legacy read, pipefail framing, §1.4 control ops (#204) ### Performance - Seal a bulk push page in parallel (#502) @@ -565,8 +613,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Close Stage 2 frontier edge cases - Define Stage 2 read-state synchronization - Note rename.sh/rename-team.sh/api.sh as sqlite-coupled known gaps -- Correct the ctrl:despawn cursor-advance comment (co1 step-3 review) -- Clarify stdout framing, cursor token, watch tip (co1 review, #203) +- Correct the ctrl:despawn cursor-advance comment +- Clarify stdout framing, cursor token, watch tip (#203) - Storage contract §2 — messages-only, opaque cursor, recipient-scoped read (#203) - Draft ADR 0003 — storage axis driver ABI, contract, scope (proposed) @@ -794,7 +842,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Documentation - Add supported-agents logo strip -- List hermes in the --agent-type help (co1 nit) +- List hermes in the --agent-type help - Add docs/plugins.md + README section + plugins/ drop-in dir - Refresh manifest table + paths for the 1.1.0 layout - Lead Quick Start with npx, the zero-clone install path @@ -877,6 +925,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Handle empty TaskList explicitly to stop fresh-session loop (#71) - Storage driver pluginization design (epic #51) (#52) +[1.3.0]: https://github.com/fujibee/agmsg/compare/v1.2.3...v1.3.0 [1.2.3]: https://github.com/fujibee/agmsg/compare/v1.2.2...v1.2.3 [1.2.2]: https://github.com/fujibee/agmsg/compare/v1.2.1...v1.2.2 [1.2.1]: https://github.com/fujibee/agmsg/compare/v1.2.0...v1.2.1 diff --git a/README.ja.md b/README.ja.md index 2bb4e11b9..0734fdb25 100644 --- a/README.ja.md +++ b/README.ja.md @@ -210,7 +210,7 @@ codex: --dangerously-skip-permissions: false # `false`の値はフラグ自体を出力しない ``` -9種類のエージェントタイプのうち8つがspawn可能 — `claude-code`、`codex`、`grok-build`、`cursor`、`gemini`、`antigravity`、`copilot`、`opencode`。`hermes` は不可 — そのCLIには初期プロンプトを事前に仕込んだインタラクティブセッションを開始するモードがない（#279）。macOSが主なターゲットで、LinuxとWindowsはベストエフォート（ターミナルが未対応の場合はissueまたはPRを歓迎）。ヘッドレス環境 — tmuxもなく使えるターミナルもない — はエージェントCLIがインタラクティブなターミナルを必要とするためエラーになる。 +10種類のエージェントタイプのうち8つがspawn可能 — `claude-code`、`codex`、`grok-build`、`cursor`、`gemini`、`antigravity`、`copilot`、`opencode`。`hermes` は、初期プロンプトを事前に仕込んだインタラクティブセッションを開始する既知のCLIモードがないためspawn不可（#279）。`devin` は、同等のインタラクティブ起動方法がまだ検証されていないため、現時点ではspawnableとして扱っていない。macOSが主なターゲットで、LinuxとWindowsはベストエフォート（ターミナルが未対応の場合はissueまたはPRを歓迎）。ヘッドレス環境 — tmuxもなく使えるターミナルもない — はエージェントCLIがインタラクティブなターミナルを必要とするためエラーになる。 ### spawnしたエージェントを終了する（`despawn`） diff --git a/README.md b/README.md index b775aedf9..867c2ac25 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ codex: --dangerously-skip-permissions: false # a `false` value suppresses the flag entirely ``` -Eight of the nine agent types are spawnable — `claude-code`, `codex`, `grok-build`, `cursor`, `gemini`, `antigravity`, `copilot`, `opencode`. `hermes` is not: its CLI has no mode that starts an interactive session pre-seeded with an initial prompt (#279). macOS is the primary target; Linux and Windows are best-effort (please open an issue/PR if your terminal isn't handled). Headless environments — no tmux **and** no usable terminal — error out, since the agent CLIs need an interactive terminal. +Eight of the ten agent types are spawnable — `claude-code`, `codex`, `grok-build`, `cursor`, `gemini`, `antigravity`, `copilot`, `opencode`. `hermes` is not spawnable because its CLI has no known mode that starts an interactive session pre-seeded with an initial prompt (#279). `devin` is currently not marked spawnable because an equivalent interactive boot mode has not yet been verified. macOS is the primary target; Linux and Windows are best-effort (please open an issue/PR if your terminal isn't handled). Headless environments — no tmux **and** no usable terminal — error out, since the agent CLIs need an interactive terminal. ### Tear down a spawned agent (`despawn`) @@ -334,13 +334,20 @@ See [docs/opencode.md](docs/opencode.md) for full setup instructions. ~/.agents/skills//scripts/send.sh "" [--force] ~/.agents/skills//scripts/inbox.sh ~/.agents/skills//scripts/history.sh [agent_id] [limit] -~/.agents/skills//scripts/team.sh +~/.agents/skills//scripts/team.sh [--json | --fix | --fix-pane-names | --rename-sessions] +~/.agents/skills//scripts/placement-collisions.sh ~/.agents/skills//scripts/whoami.sh ~/.agents/skills//scripts/delivery.sh set ~/.agents/skills//scripts/delivery.sh status [ ] ~/.agents/skills//scripts/reset.sh [agent_id] ``` +`team.sh` combines the roster with terminal placement, activity, delivery mode, and identity consistency. Verified identity is collapsed to `identity=ok`; mismatches and values that could not be observed are expanded with their evidence. `--json` emits every field for every registration. The repair flags report each action per identity cell as `changed`, `skipped`, `failed`, or (for a session name that could not be read back) `poked_unverified`. They are two different kinds of act: `--fix-pane-names` repairs the pane label and agent key through the terminal's own API and never types into a session; `--rename-sessions` repairs the CLI session name by typing the type's rename command (`/rename -` for Claude Code, whatever the type's manifest declares otherwise) into the pane, and only after positively identifying a ready process there. `--fix` does both, unconditionally, including the keystroke. Pane liveness will join this view when the pane-state contract lands; until then the unavailable column is omitted rather than filled with `unknown`. + +`placement-collisions.sh` is a separate, read-only installation-wide report. It never repairs or removes a record; keeping this fleet observation outside `team.sh` prevents an operator-level scan from becoming part of a seat's repair path. Today it reports only the record-only layer: two DIFFERENT seats' records resolved to the same canonical (kind, instance, pane) locator, entirely from records on disk — no terminal is ever asked anything. A ref with no instance component to resolve (every herdr ref today; a legacy bare tmux `%N`/`@N`) is not joined by raw string equality; it is listed under `unscoped_records` instead, since record-only evidence cannot tell such refs apart across terminal instances. `collisions: none` means the walk completed and found nothing; `collisions: none_observed` paired with `coverage: partial` means something along the way (a team config, a placement record, an empty ref) could not be read, so the empty answer is not a proven one; `collisions: not_attempted` means there was no `teams/` directory to walk at all. An actual-location layer — matching a seat's own record against where a live census actually observes it — is designed but not yet wired here; see the script's header. + +Terminal identity has a different number of observable names on each backend. Herdr exposes three independent values: the visible pane label, its internal agent key, and the CLI session name. tmux exposes two: the `@agmsg_agent` pane option is the internal key, while the CLI owns `pane_title`, so there is no independent pane-label field after the CLI starts. `team.sh` reports that tmux field as `n/a` rather than treating an unavailable concept as a mismatch. + `send.sh` takes four positional arguments — ` ""` — plus an optional trailing `--force`. Quote the message so the shell sees it as one argument; an unquoted message with spaces will be misparsed. Both `from` and `to` must already be registered in ``; an unregistered name errors out (listing the currently registered names) instead of silently storing an undeliverable message. Pass `--force` to bypass this check for an intentional pre-registration send. ## FAQ / Design notes diff --git a/SKILL.md b/SKILL.md index 57d420ec8..8c6b85a3e 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,230 +1,230 @@ --- -name: agmsg -description: Cross-agent messaging via SQLite. Send messages between Claude Code, Codex, Gemini CLI, GitHub Copilot CLI, and other agents. No daemon, no network, no dependencies beyond bash and sqlite3. +name: __SKILL_NAME__ +description: Cross-agent messaging via SQLite. Send messages between Claude Code, Codex, Gemini CLI, and other agents. No daemon, no network, no dependencies beyond bash and sqlite3. --- -# Agent Messaging + -**IMPORTANT: Always use the provided scripts. NEVER directly read or edit config files, DB, or team data. There is NO register.sh — use join.sh to join a team.** +Agent messaging command. **IMPORTANT: Always use the provided scripts. NEVER directly read or edit config files, DB, or team data. There is NO register.sh — use join.sh to join a team.** -**Shell requirement:** All agmsg scripts are Bash scripts. Always execute them via `bash`, never via PowerShell or cmd directly. If your default shell is not Bash (e.g. PowerShell on Windows), wrap every command with `bash -lc '...'`. Example: `bash -lc '~/.agents/skills/agmsg/scripts/send.sh myteam alice bob "hello"'`. Do NOT construct DB paths manually — the scripts handle path resolution internally. If you need to redirect storage, use `AGMSG_STORAGE_PATH` (the supported override). +**Use agmsg, not the host agent's own inter-session messaging.** Several agent +CLIs ship a native way for one session to message another on the same machine +(in Claude Code, the `SendMessage` / `ListAgents` tools over its peer-session +list). While a project is on agmsg, route agent-to-agent messages through agmsg +instead. A message sent natively does not exist as far as agmsg is concerned: +it is absent from `history.sh` and the team's export, it never reaches a member +on another machine through remote sync, it does not mark read or advance any +cursor, and it cannot address a member whose CLI is a different type. Half the +conversation living somewhere unrecorded is worse than either channel alone, +and the gap is invisible until someone reads the history and finds a decision +with no message behind it. The native channel stays fine for anything outside +the team — a subagent you spawned for your own task, or a session that has not +joined. -## How to use +**Shell requirement:** All agmsg scripts are Bash scripts. Always execute them via `bash`, never via PowerShell or cmd directly. If your default shell is not Bash (e.g. PowerShell on Windows), wrap every command with `bash -lc '...'`. Example: `bash -lc '~/.agents/skills/__SKILL_NAME__/scripts/send.sh myteam alice bob "hello"'`. Do NOT construct DB paths manually — the scripts handle path resolution internally. If you need to redirect storage, use `AGMSG_STORAGE_PATH` (the supported override). -### Step 0: First-run bootstrap + + -agmsg keeps its SQLite database, team registry, and runtime state under `~/.agents/skills/agmsg/`. The `./install.sh` install path creates that tree; the Claude Code plugin install path does not (the plugin marketplace flow only drops the skill content into `~/.claude/plugins/cache/`). Before any other command, bootstrap if needed: +## Identity -```bash -if [ ! -d ~/.agents/skills/agmsg ]; then - # Locate the plugin install script (any version), run it once. - installer=$(ls ~/.claude/plugins/cache/fujibee-agmsg/agmsg/*/install.sh 2>/dev/null | head -1) - if [ -n "$installer" ]; then - bash "$installer" --cmd agmsg - else - echo "agmsg not installed. Either:" >&2 - echo " - run ./install.sh in the agmsg repo, or" >&2 - echo " - install via /plugin marketplace add fujibee/agmsg && /plugin install agmsg@fujibee-agmsg" >&2 - exit 1 - fi -fi -``` - -After this runs once, `~/.agents/skills/agmsg/` is populated and you can skip Step 0 on future invocations. - -### Step 1: Check identity - -```bash -~/.agents/skills/agmsg/scripts/whoami.sh "$(pwd)" -# type: claude-code, codex, gemini, antigravity, copilot -# Returns: agent=... / multiple=true ... / suggest=true ... / not_joined=true ... -``` - -### Step 2a: If not in a team — join one - -Before first-time setup, inspect the user's request. If they ask to join, -import, or bring in a team that already exists on a server, do not run -`join.sh`. Go directly to the `remote pull` command under Step 2b. Before -pulling, run `team-list.sh --json --scope all`; if a same-named local team has -`binding_state` `none` or `disconnected`, stop and ask the user how to proceed. -After pull succeeds, return here so the user can register a new local agent in -the team that pull just created. - -Ask the user for a team name. If it's an existing team, run `team.sh ` first to see the current roster and note the names already in use. Look for a naming convention already in play (e.g. a shared base name with role and number suffixes (`-`), or names derived from the team name) and, when one exists, propose 2-3 unused names that extend it; otherwise propose 2-3 short, distinctive identity names (not a bare tool-type label like `codex`/`cc`). Either way, names must not collide with the roster. For a brand-new team, skip the roster check and just ask. Then run: - -```bash -~/.agents/skills/agmsg/scripts/join.sh "$(pwd)" [--force] -``` - -Do NOT manually edit config files. Always use join.sh. If the name was recently renamed away with `rename.sh`, join.sh refuses to revive it (printing the new name it maps to) instead of silently re-registering it — this guards against a CLI slash-command history resubmitting `actas ` after a rename. Pass `--force` only for a deliberate, unrelated reuse of that exact name. - -### Step 2b: If already in a team — execute command - -**Default (no arguments): IMMEDIATELY check inbox. Do NOT ask what to do.** - -```bash -# Check inbox (marks messages as read) — DEFAULT action -~/.agents/skills/agmsg/scripts/inbox.sh - -# Send a message (from/to must already be registered in ; add --force to bypass) -~/.agents/skills/agmsg/scripts/send.sh "" [--force] - -# Message history -~/.agents/skills/agmsg/scripts/history.sh [agent_id] [limit] - -# Export a team's message history as JSONL — one message_sent record per line, -# chronological. Default to stdout (pipeable); --out writes a file. -# --agent limits to one agent; --limit keeps the most recent N (omit = all -# currently retained). Output is plaintext (the local store is plaintext). -~/.agents/skills/agmsg/scripts/export.sh --team [--agent ] [--limit N] [--out ] - -# List team members -~/.agents/skills/agmsg/scripts/team.sh - -# List every locally known team (read-only, secret-free — "agmsg team list"). -# Distinct from `team.sh ` above: check for "team list" FIRST so -# "list" is never mistaken for a team name. --json emits a strict, -# versioned object ({schema_version, teams: [{name, remote_team_id, scope, -# binding_state}]}) and exits non-zero with NO payload if any team was -# unreadable or the count was truncated — never a partial list dressed up -# as complete. See scripts/team-list.sh's own header comment for the exact -# enums and why onboarding_state/promote_eligible/blocked_reason are -# deliberately NOT in this schema yet (their meaning depends on ADR 0010, -# which hasn't landed). -~/.agents/skills/agmsg/scripts/team-list.sh [--json] [--scope all|project] [] - -# Leave a team -~/.agents/skills/agmsg/scripts/leave.sh - -# Rename a team (moves dir, updates config + messages). -# After renaming, each existing member should re-run whoami.sh to refresh -# their cached team name in any running session. -~/.agents/skills/agmsg/scripts/rename-team.sh - -# Show the installed version — the git-describe provenance string recorded at -# install time (tag + commits-since + abbreviated commit, plus -dirty when -# installed from a tree with uncommitted changes). See #117. -~/.agents/skills/agmsg/scripts/version.sh - -# Clear registrations for the current project/type. -# A trailing additionally releases any actas exclusivity locks -# this session held on so peers can pick them up immediately. -~/.agents/skills/agmsg/scripts/reset.sh "$(pwd)" [agent_id] [session_id] - -# Set delivery mode for this project. -# monitor — real-time push via SessionStart + Monitor tool (claude-code only) -# turn — Stop-hook pulls at the end of each assistant turn -# both — monitor primary, turn as fallback -# off — no automatic delivery -~/.agents/skills/agmsg/scripts/delivery.sh set "$(pwd)" -~/.agents/skills/agmsg/scripts/delivery.sh status "$(pwd)" - -# Multiple roles per project (one CC = one active role). -# Claude Code: `actas` claims an exclusivity lock for across sessions -# and restarts the Monitor filtered to only; peer watchers stop -# subscribing to while this session holds the lock. `drop` releases. -# Codex: actas is send-side only (no stable session_id during slash commands -# → no peer-visible lock). See README "Codex caveat" for details. -# If is new and none was given upfront (bare `actas`, or the user asks -# for a suggestion), check the target team's roster first (team.sh ). -# Look for a naming convention already in play (e.g. a shared base name with -# role and number suffixes (-), or names derived from the -# team name) and, when one exists, propose 2-3 unused names that extend it; -# otherwise propose 2-3 short, distinctive names. Either way, names must not -# collide with the roster. Ask the user to pick before continuing. -~/.agents/skills/agmsg/scripts/actas-claim.sh "$(pwd)" "$session_id" -~/.agents/skills/agmsg/scripts/reset.sh "$(pwd)" "$session_id" - -# (Both of the above are normally driven by `/agmsg actas ` and -# `/agmsg drop ` slash commands, which also handle the Monitor -# TaskStop + relaunch dance described in the cmd template.) - -# Spawn a NEW agent process that takes an actas identity on boot. -# Pre-joins to a team, then launches the agent CLI in a tmux pane/window -# (when run inside tmux) or a new OS terminal, with `/agmsg actas ` as the -# initial prompt. By default it BLOCKS until the new agent's watcher attaches -# (prints `status=ready`), so a leader can send work right after spawn returns -# without losing it to the agent's cold start. claude-code/codex only; macOS -# primary, Linux/Windows best-effort. Non-tmux + no usable terminal (headless) -# errors out. -# --project project to launch in (default: $PWD) -# --team team to join into (default: auto-resolved from project) -# --window new tmux window instead of splitting the current one -# --split h|v tmux split direction (default h) -# --terminal terminal command template ({cmd} = path to the boot -# script) for the non-tmux path; overrides $AGMSG_TERMINAL -# / config spawn.terminal. macOS default uses `open -a` -# (no Automation/TCC permission prompt). -# --no-wait don't block on readiness (fire-and-forget) -# --ready-timeout N seconds to wait for readiness (default 90; on timeout -# prints status=timeout and exits 3). Codex skips the -# wait (it has no Monitor). -# --boot-prompt hand the new agent an initial task: the boot prompt -# becomes the actas command followed (newline-separated) -# by , so it claims its identity AND starts the task -# in its first turn. The only way to give a one-shot goal -# to a codex peer (no Monitor → a post-spawn send to its -# idle session is never noticed). -~/.agents/skills/agmsg/scripts/spawn.sh [options] - -# Tear down a spawned member — the inverse of spawn. -# Default (graceful): sends a `ctrl:despawn` control message to ; the -# member's watcher drops its own role (releasing the actas lock + registration) -# and closes its own tmux pane, ending the agent. Blocks until the lock releases -# (--timeout, default 30s) then prints `status=ok`; on timeout prints -# status=timeout and exits 3 (retry with --force). Only an exclusive watcher -# dedicated to acts on it — the despawning session is never torn down. -# --force: skip the message and tear the member down from the placement recorded -# at spawn time (kill its tmux pane/window, drop its registration) — for a dead -# watcher or a codex member (no Monitor). A hand-started member with no placement -# record can't be --forced. -# --force tear down from the recorded placement, no message -# --timeout N seconds to wait for graceful teardown (default 30) -~/.agents/skills/agmsg/scripts/despawn.sh [--force] [--timeout N] -``` - -### Rename +If you already know your AGENT and TEAMS from a previous `__CMD_PREFIX____SKILL_NAME__` call in this session, skip to **Execute** below. + +Otherwise, run: `~/.agents/skills/__SKILL_NAME__/scripts/whoami.sh "$(pwd)" __AGENT_TYPE__` + +Four possible outputs: + +**A) Single identity:** +`agent= teams= type=__AGENT_TYPE__ project=` +→ Remember AGENT and TEAMS, then go to **Execute**. + +**B) Multiple identities:** +`multiple=true agents= teams= type=__AGENT_TYPE__ project=` +→ Ask the user which agent name to use for this session, then go to **Execute**. + +**C) Not in a team:** +`not_joined=true available_teams=` (or `available_teams=none`) +→ Show the user the available teams from the output, then: + + Before first-time setup, inspect the user's request. If they ask to join, import, or bring in a team that already exists on a server, do not call `join.sh`. Go directly to `remote pull` under Execute. First run `~/.agents/skills/__SKILL_NAME__/scripts/team-list.sh --json --scope all`; if a same-named local team has `binding_state` `none` or `disconnected`, stop and ask the user how to proceed. After pull succeeds, return to Identity setup so the user can register a new local agent in the pulled team. + + > **First-time setup required.** + > Joining a team so this agent can send and receive messages. + > - **Team name**: a group of agents that can message each other (available: ) + > - **Agent name**: this agent's identity within the team + + 1. Ask: "Enter a team name (joins existing or creates new)" + 2. If the team name given already appears in `available_teams`, run `~/.agents/skills/__SKILL_NAME__/scripts/team.sh ` to see the current roster (name, type, project) and note the names already in use. Look for a naming convention already in play (e.g. a shared base name with role and number suffixes (`-`), or names derived from the team name) and, when one exists, propose 2-3 unused names that extend it; otherwise propose 2-3 short, distinctive identity names (not a bare tool-type label like `codex`/`cc`). Either way, names must not collide with the roster. Then ask: "Enter a name for this agent (suggestions: , , — or type your own)". For a brand-new team, skip the roster check and just ask: "Enter a name for this agent". + 3. **You MUST use join.sh** — run: `~/.agents/skills/__SKILL_NAME__/scripts/join.sh __AGENT_TYPE__ "$(pwd)"` + 4. Show the result and explain: + + > **Joined!** You can now use `__CMD_PREFIX____SKILL_NAME__` to check and send messages. + > - `__CMD_PREFIX____SKILL_NAME__` — check inbox + > - `__CMD_PREFIX____SKILL_NAME__ send ` — send a message + > - `__CMD_PREFIX____SKILL_NAME__ team` — list team members + > - `__CMD_PREFIX____SKILL_NAME__ history` — message history + + + + + 6. Then check inbox for the newly joined team. + +**D) Suggestions for reuse:** +`suggest=true agents= teams= type=__AGENT_TYPE__ project= available_teams=` +→ No exact registration exists for this project, but there are same-type agent names registered elsewhere. + + 1. Show the suggested agent names to the user. + 2. Ask whether to reuse one of those names or choose a new one. + 3. Ask for the team name to join (existing or new). + 4. Run: `~/.agents/skills/__SKILL_NAME__/scripts/join.sh __AGENT_TYPE__ "$(pwd)"` + 5. Then continue with the normal post-join flow above. + +## Execute + +**Only use scripts in `~/.agents/skills/__SKILL_NAME__/scripts/` — do not read or modify files under `teams/` or `db/` directly.** Treat the storage layout as internal: never construct a database path or invoke `sqlite3` directly. The scripts resolve the active store, including `AGMSG_STORAGE_PATH` overrides. + +**Terminal/pane self-awareness.** Asked about this session's own terminal, pane, or driver — or before using `arrange`, `peek`, or `poke` below — run `where.sh` (see the "where" argument below) first and answer from its `terminal=`/`capabilities=` fields. Never infer the driver from environment variables or a `grep`/`ps` guess: that is how a session under a real driver ends up reporting a false negative about its own placement, or claiming a capability or a whole driver does not exist when it does (#1171). Each driver's own operational detail lives in `~/.agents/skills/__SKILL_NAME__/scripts/drivers/terminals//README.md`, named by `where.sh`'s own `terminal=` field — never guessed at from a remembered syntax. + +Asked about a *teammate's* placement or status, or what can be done to one, that is `team.sh `'s question (see the "team" argument below), not something to infer from a stale memory of their last known pane. Act on a teammate with `peek.sh`/`poke.sh`/`arrange.sh ` directly rather than guessing reachability first — its exit code says whether it worked and, if not, why (see the "peek"/"poke"/"arrange" arguments below). + +**If no arguments provided (DEFAULT action — always do this when the command is invoked without arguments):** +1. **IMMEDIATELY** run inbox check for each TEAM: `~/.agents/skills/__SKILL_NAME__/scripts/inbox.sh $TEAM $AGENT` +2. Do NOT ask the user what to do — just run the inbox check. +3. If there are messages, read and respond appropriately. To reply: + `~/.agents/skills/__SKILL_NAME__/scripts/send.sh $TEAM $AGENT ""` + + + + +If argument is "history": +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/history.sh $TEAM $AGENT` + +If argument starts with "team list" (e.g. "team list", "team list --json", "team list --scope project"): +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/team-list.sh ` +2. This is a distinct command from bare "team" below — check for "team list" FIRST so "list" is never mistaken for a team name. + +If argument is "team" or "team --json": +1. For each TEAM, run: `~/.agents/skills/__SKILL_NAME__/scripts/team.sh $TEAM [--json]`, preserving the option when present. `--json` returns every observed field. +2. This is read-only. It reports each member's identity cells and whether they are consistent — including a member that does not answer at all — but writes nothing and pokes no one. A seat that is wrong or unresponsive is not something this command, or any other, repairs from the outside: typing into another seat's session to fix it is exactly the mistake that used to happen here, and it is gone on purpose, not replaced by another form of the same thing. A member repairs its own identity cells by running `fix` (below), from itself. A seat that cannot or will not do that gets despawned and restarted, or a person takes it — not patched over from another pane. + +If argument starts with "send" (e.g. "send misaki check the server"): +1. Parse target agent and message from the arguments +2. Determine which team the target agent belongs to, then run: + `~/.agents/skills/__SKILL_NAME__/scripts/send.sh $TEAM $AGENT ""` + +If argument is "config": +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/config.sh show` +2. Show the output to the user. + +If argument starts with "config set" (e.g. "config set hook.check_interval 30"): +1. Parse key and value from the arguments. +2. Run: `~/.agents/skills/__SKILL_NAME__/scripts/config.sh set ` + +If argument is "version": +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/version.sh` +2. Show the output — the installed version (git-describe provenance recorded at install time). + +If argument is "where" (e.g. asked to report this session's own pane or placement): +1. Run: `~/.agents/skills/__SKILL_NAME__/scripts/where.sh` +2. Report exactly what it prints. Do not try to answer this by naming a terminal yourself or running any terminal-specific command directly — this call already asked every driver on this session's behalf. +3. `resolved=true placement=:` is a known pane; `resolved=true placement=none` is a GENUINE negative (this session's own terminal confirmed it has no addressable pane). `resolved=false` means placement could NOT be determined — `reason` names which terminal(s) were asked and why. Never report a `resolved=false` answer as "no pane" or "not attached to a pane"; those are different answers to different questions, and the difference is the entire point of this command (#1171). +4. `where.sh`'s output also carries `capabilities=` (#1082) — that resolved terminal's own manifest, space-separated, verbatim. Before using `arrange`, `peek`, or `poke` below, check that the verb is in this list; if it is not, report it as unavailable for this terminal (name the terminal) rather than attempting it and finding out from an exit code. If it IS listed, read that terminal's own file — `~/.agents/skills/__SKILL_NAME__/scripts/drivers/terminals//README.md` — before reporting a peek/poke/arrange failure: exit-code meanings differ by driver, and that file, not this one, is where they live. + + + +If argument starts with "actas" followed by an agent name: +1. Parse the new role name and inspect the team roster when suggestions are needed. +2. Run `~/.agents/skills/__SKILL_NAME__/scripts/identities.sh "$(pwd)" __AGENT_TYPE__`. +3. If needed, join with `~/.agents/skills/__SKILL_NAME__/scripts/join.sh __AGENT_TYPE__ "$(pwd)"`. +4. Set the session's active FROM to `` for subsequent sends. +5. Tell the user which role is active. + + +If argument starts with "drop" followed by an agent name: +1. Run `~/.agents/skills/__SKILL_NAME__/scripts/reset.sh "$(pwd)" __AGENT_TYPE__ `. +2. Clear the active role when it matches `` and report the result. + + +If argument starts with "spawn" (e.g. "spawn claude-code alice", "spawn codex reviewer --window"): +1. Parse `` (a spawnable agent type), ``, and any options (`--boot-prompt `, `--project `, `--team `, `--window`, `--split h|v`, `--terminal