From 67e676070f292c2f7f359cb4a61bf57e921bdfa9 Mon Sep 17 00:00:00 2001 From: fujibee Date: Sat, 12 Sep 2026 22:07:16 -0700 Subject: [PATCH 1/2] fix(self-name): resolve project/type when the action hook's callers have none (#1137) send.sh/inbox.sh/history.sh call agmsg_self_name_on_action with only -- they never had a project or type to pass, not merely forgot to. The hook forwarded that straight through to the placement-record writer, so a record written by acting (the #1109 path, the one that exists for hand-started seats) carried two empty fields. arrange.sh requires both and refuses such a record outright; measured live, 12 of 36 placement records on one machine were stuck this way. Resolve project/type inside the hook itself, the same way whoami.sh already answers the identical question (agmsg_detect_cli_type, agmsg_resolve_project), instead of threading two new arguments through three callers that have no better source for them than this process's own cwd and type anyway. Only when the caller did not already supply them, and only right before the slow-half write -- the common fast-path short-circuit is untouched. Best-effort, matching every other lazy source in this function: a resolution failure leaves the field empty exactly as before, never blocks the naming this hook exists to do. tests/test_self_name.bats gains the failing-side control: a hand-started seat's action-hook record now carries non-empty project/type (verified red against a revert of this fix), and a caller that already supplies both is never second-guessed. --- scripts/lib/self-name.sh | 35 +++++++++++++++++++++++++++++++++++ tests/test_self_name.bats | 29 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/scripts/lib/self-name.sh b/scripts/lib/self-name.sh index 5d60d6d36..fc6ad0601 100644 --- a/scripts/lib/self-name.sh +++ b/scripts/lib/self-name.sh @@ -147,6 +147,41 @@ agmsg_self_name_on_action() { return 0 # named AND recorded at where I am fi + # #1137: none of send.sh/inbox.sh/history.sh pass project or type -- they + # never had them to pass, not merely forgot to -- so a record this hook + # writes had two empty fields, and arrange.sh (which requires both) refused + # any seat whose ONLY placement record came from acting rather than from + # spawn/actas/SessionStart (measured live: 12 of 36 records). Resolve them + # here, the same way whoami.sh does for the identical question, rather than + # threading two new arguments through three callers that have no better + # source for them than this process's own cwd and type anyway. Only when + # the caller did not supply them -- a caller that already knows better + # (every other path through the primitive) is never second-guessed. + if [ -z "$project" ] || [ -z "$type" ]; then + # Best-effort, matching every other lazy source in this function: a + # failure here must not block the naming this hook exists to do, so a + # record written with what could be resolved is better than none, and + # the record's own project/type stay only as good as this detection is. + if [ -z "$type" ]; then + # shellcheck disable=SC1091 + . "$SKILL_DIR/scripts/lib/type-registry.sh" 2>/dev/null || true + # shellcheck disable=SC1091 + . "$SKILL_DIR/scripts/lib/compat.sh" 2>/dev/null || true + # shellcheck disable=SC1091 + if . "$SKILL_DIR/scripts/lib/detect-cli-type.sh" 2>/dev/null \ + && declare -F agmsg_detect_cli_type >/dev/null 2>&1; then + type="$(agmsg_detect_cli_type 2>/dev/null || true)" + fi + fi + if [ -z "$project" ]; then + # shellcheck disable=SC1091 + if . "$SKILL_DIR/scripts/lib/resolve-project.sh" 2>/dev/null \ + && declare -F agmsg_resolve_project >/dev/null 2>&1; then + project="$(agmsg_resolve_project "$(pwd)" "$type" "$team" 2>/dev/null || true)" + fi + fi + fi + # Slow half, once: name the pane through the same primitive every other path # uses, and -- the #1109 fix -- record this pane as the seat's placement. The # `record` claim is legitimate here and only here among the label writers (see diff --git a/tests/test_self_name.bats b/tests/test_self_name.bats index f252fa68e..41d1ab268 100644 --- a/tests/test_self_name.bats +++ b/tests/test_self_name.bats @@ -313,6 +313,35 @@ _placement() { # -> ":" or empty [ "$(_placement team alice)" = 'tmux:/tmp/s:%3' ] } +@test "a hand-started seat's record carries project and type, not the two empty fields arrange.sh refuses (#1137)" { + _install_fake_tmux; _under_tmux /tmp/s 4242 %3 + # None of send.sh/inbox.sh/history.sh pass project or type -- this call + # shape (2 args) is exactly what they do. + [ -z "$(_placement team alice)" ] + agmsg_self_name_on_action team alice + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/actas-lock.sh" + local rec ref project type + rec="$(agmsg_spawn_path team alice)" + [ -f "$rec" ] + IFS=$'\t' read -r ref project type < "$rec" + [ "$ref" = 'tmux:/tmp/s:%3' ] + [ -n "$project" ] + [ -n "$type" ] +} + +@test "a caller that already supplies project and type is never second-guessed (#1137)" { + _install_fake_tmux; _under_tmux /tmp/s 4242 %3 + agmsg_self_name_on_action team alice /explicit/project explicit-type + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/actas-lock.sh" + local rec ref project type + rec="$(agmsg_spawn_path team alice)" + IFS=$'\t' read -r ref project type < "$rec" + [ "$project" = /explicit/project ] + [ "$type" = explicit-type ] +} + @test "named once but never recorded: the next action writes the missing record (#1109)" { _install_fake_tmux; _under_tmux /tmp/s 4242 %3 # A mark-only prior naming (watch.sh names with five args, no record): the mark From 985c2120c6d5099ad39de83afa626a84cfa3f27f Mon Sep 17 00:00:00 2001 From: fujibee Date: Sun, 13 Sep 2026 02:34:16 -0700 Subject: [PATCH 2/2] fix(self-name): guard the new SKILL_DIR reads, back to the enforced baseline The four new lazy-source lines added for #1137 read \$SKILL_DIR unguarded, tripping check-unguarded-env-reads.sh from 89 to 93 (the file's own \${SKILL_DIR:=...} default-assignment at sourcing time guarantees it in practice, but the checker's assignment detection does not recognize that form). Give each read the same default the checker itself suggests; SKILL_DIR is never actually empty here, so this changes nothing at runtime. Diff method per tonight's standard: baseline forced to 0 on both base (026d10f4) and this head, offender lists diffed -- exactly the four new lines, nothing else moved. --- scripts/lib/self-name.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/lib/self-name.sh b/scripts/lib/self-name.sh index fc6ad0601..dc5a1b0fa 100644 --- a/scripts/lib/self-name.sh +++ b/scripts/lib/self-name.sh @@ -164,18 +164,18 @@ agmsg_self_name_on_action() { # the record's own project/type stay only as good as this detection is. if [ -z "$type" ]; then # shellcheck disable=SC1091 - . "$SKILL_DIR/scripts/lib/type-registry.sh" 2>/dev/null || true + . "${SKILL_DIR:-}/scripts/lib/type-registry.sh" 2>/dev/null || true # shellcheck disable=SC1091 - . "$SKILL_DIR/scripts/lib/compat.sh" 2>/dev/null || true + . "${SKILL_DIR:-}/scripts/lib/compat.sh" 2>/dev/null || true # shellcheck disable=SC1091 - if . "$SKILL_DIR/scripts/lib/detect-cli-type.sh" 2>/dev/null \ + if . "${SKILL_DIR:-}/scripts/lib/detect-cli-type.sh" 2>/dev/null \ && declare -F agmsg_detect_cli_type >/dev/null 2>&1; then type="$(agmsg_detect_cli_type 2>/dev/null || true)" fi fi if [ -z "$project" ]; then # shellcheck disable=SC1091 - if . "$SKILL_DIR/scripts/lib/resolve-project.sh" 2>/dev/null \ + if . "${SKILL_DIR:-}/scripts/lib/resolve-project.sh" 2>/dev/null \ && declare -F agmsg_resolve_project >/dev/null 2>&1; then project="$(agmsg_resolve_project "$(pwd)" "$type" "$team" 2>/dev/null || true)" fi