diff --git a/functions/agents-vault.fish b/functions/agents-vault.fish index 9da3d4e..ae50dcc 100644 --- a/functions/agents-vault.fish +++ b/functions/agents-vault.fish @@ -71,9 +71,11 @@ # # --adopt=SLUG rebinds the current project's entry to SLUG, which is how # a machine-specific local-* key or an ambiguous migration is resolved. -# SLUG must match [a-z0-9._-]+ with no slash and no leading dot -- the -# charset the slug formula itself emits -- since it is interpolated into -# a vault path and handed to git mv. +# SLUG must match [a-z0-9._-]+ and be neither "." nor ".." -- the charset +# the slug formula itself emits -- since it is interpolated into a vault +# path and handed to git mv. The rename and the relink are atomic: if the +# live memory directory cannot be repinned onto the new entry the rename +# is rolled back, so an ordinary run still finds the original entry. # # --remote=URL points the vault at a remote; --push commits and then # pushes there. @@ -94,7 +96,15 @@ # EXIT STATUS # 0 Completed successfully # 1 Fatal error (vault unavailable, git failure, ambiguous migration, -# invalid --adopt slug, or --push with no remote configured) +# invalid --adopt slug, nothing committed, or a push that did not +# reach the remote) +# +# RETURNS +# --status prints its report on stdout: the vault path, the remote and +# how far ahead of it the vault is, a warning for an unresolved rebase, +# then one line per entry reading "linked" or "orphan", the slug, and the +# file count. Every other mode prints only verbosity-gated progress +# lines, and nothing at all when there was nothing to do. # # EXAMPLE # agents-vault @@ -362,11 +372,20 @@ function agents-vault --description 'track curated agent memory in a host-scoped # to `git mv`, so it is validated before it is used anywhere: # --adopt=../../../etc would otherwise walk straight out of the # vault. Only the charset the slug formula itself emits is - # accepted, and a leading dot is refused as well, which also rules - # out the bare "." and ".." entries. - if not string match -qr '^[a-z0-9_-][a-z0-9._-]*$' -- "$_flag_adopt" + # accepted, which excludes the slash that any traversal needs, and + # the two names that traverse without one are refused by name. + # + # A leading dot is *not* refused: _agents_repo_slug legitimately + # emits one for a dot-led subdomain (https://.hidden.example.com/r + # keys as .hidden.example.com-r), and refusing it would make such + # an entry impossible to adopt. Inside projects/ a leading dot is + # a hidden directory, not an escape. + set -l bad_slug 0 + string match -qr '^[a-z0-9._-]+$' -- "$_flag_adopt"; or set bad_slug 1 + contains -- "$_flag_adopt" . ..; and set bad_slug 1 + if test $bad_slug -eq 1 echo "$c_err""agents-vault: invalid slug '$_flag_adopt'$c_reset" >&2 - echo "$c_err"" A slug is [a-z0-9._-]+ with no slash and no leading dot.$c_reset" >&2 + echo "$c_err"" A slug is [a-z0-9._-]+, is not '.' or '..', and holds no slash.$c_reset" >&2 return 1 end @@ -393,23 +412,49 @@ function agents-vault --description 'track curated agent memory in a host-scoped echo "$c_err""agents-vault: $_flag_adopt already holds content; refusing to overwrite$c_reset" >&2 return 1 end - test -d "$to"; and rm -rf "$to" + # Adopting is a rename plus a relink, and it must be all or + # nothing. A rename that lands while the relink fails leaves the + # memory intact at the new slug but unreferenced: the next ordinary + # run finds no live link, recomputes the old slug, finds nothing + # there, and fabricates a fresh empty entry, so the agent writes + # history-less memory from then on. No bytes are lost, but + # continuity is, and nothing recovers it automatically. + # + # So a contentless target entry is moved aside rather than deleted, + # and the origin note is appended only once the relink has + # succeeded. Both are undone below if it does not. The stash lives + # at the vault root, not under projects/, so a crash between the + # two renames cannot leave something that reads as an entry. + set -l stash "" + if test -d "$to" + set stash "$vault/.adopt-stash" + rm -rf "$stash" + command mv "$to" "$stash"; or return 1 + end if not git -C "$vault" mv "projects/$cur" "projects/$_flag_adopt" 2>/dev/null - command mv "$from" "$to"; or return 1 + if not command mv "$from" "$to" + test -n "$stash"; and command mv "$stash" "$to" + return 1 + end end - printf 'adopted: %s → %s (%s)\n' "$cur" "$_flag_adopt" (date -I) >>"$to/origin" set -l claude_root $__fish_agent_vault_claude_root test -n "$claude_root"; or set claude_root "$HOME/.claude/projects" set -l mangled (string replace -a '/' '-' -- "$root" | string replace -a '.' '-') - # The live path still points at the old entry, which no longer - # exists; drop it so ensure_symlink is not asked to resolve a - # broken link before repinning it. - test -L "$claude_root/$mangled/memory"; and rm -f "$claude_root/$mangled/memory" + # The live link is deliberately left in place for ensure_symlink to + # repin, which it does on its own for a link pointing elsewhere. + # Removing it first would only widen the window in which a failure + # leaves the project with no link at all. if not _agents_repo_ensure_symlink "$claude_root/$mangled/memory" "$to/claude/memory" >/dev/null - echo "$c_err""agents-vault: adopted $_flag_adopt but could not relink $claude_root/$mangled/memory$c_reset" >&2 + if not git -C "$vault" mv "projects/$_flag_adopt" "projects/$cur" 2>/dev/null + command mv "$to" "$from" + end + test -n "$stash"; and command mv "$stash" "$to" + echo "$c_err""agents-vault: could not relink $claude_root/$mangled/memory; $cur was left as it was$c_reset" >&2 return 1 end + printf 'adopted: %s → %s (%s)\n' "$cur" "$_flag_adopt" (date -I) >>"$to/origin" + test -n "$stash"; and rm -rf "$stash" _agents_repo_sync "$vault" "chore: adopt $cur as $_flag_adopt" >/dev/null test $verbose -eq 1; and echo "$c_ok→ Adopted $cur as $_flag_adopt$c_reset" @@ -427,6 +472,7 @@ function agents-vault --description 'track curated agent memory in a host-scoped if set -q _flag_restore set -l claude_root $__fish_agent_vault_claude_root test -n "$claude_root"; or set claude_root "$HOME/.claude/projects" + set -l restore_failed 0 for entry in "$vault"/projects/* test -d "$entry/claude/memory"; or continue set -l eslug (path basename "$entry") @@ -440,6 +486,7 @@ function agents-vault --description 'track curated agent memory in a host-scoped set -l msg (_agents_repo_ensure_symlink "$claude_root/$m/memory" "$entry/claude/memory") if test $status -ne 0 echo "$c_err""agents-vault: could not relink $eslug$c_reset" >&2 + set restore_failed 1 else if test -n "$msg" test $verbose -eq 1; and echo "$c_ok→ Restored $eslug$c_reset" end @@ -448,7 +495,11 @@ function agents-vault --description 'track curated agent memory in a host-scoped and echo "$c_warn→ Cannot place $eslug: no live project found; use --adopt from the project$c_reset" end end - return 0 + # An entry that could not be relinked is a failure, not a note in + # passing: the same reasoning as the commit and push paths below. + # An entry with no live project is not -- there is nothing wrong + # with the vault, the project simply is not on this machine. + return $restore_failed end # ────────────────────────── global state ─────────────────────────── @@ -642,6 +693,12 @@ function agents-vault --description 'track curated agent memory in a host-scoped end # ───────────────────────────── commit ────────────────────────────── + # A sync that did not commit is a backup that did not happen, so it is + # reported as a failure rather than warned about and walked past. The + # function otherwise ends on a branchless `if`, which resolves to 0, + # and a backup tool that reports success while nothing was recorded + # recreates the exact loss the vault exists to prevent. + set -l failed 0 if not set -q _flag_link set -l msg "chore: sync agent memory vault" test $did_init -eq 1; and set msg "chore: initialize agent memory vault" @@ -649,6 +706,10 @@ function agents-vault --description 'track curated agent memory in a host-scoped set -l sync_rc $status if test $sync_rc -eq 2 echo "$c_warn""agents-vault: unresolved rebase conflict in the vault; nothing committed$c_reset" >&2 + set failed 1 + else if test $sync_rc -ne 0 + echo "$c_err""agents-vault: the vault commit failed; nothing recorded$c_reset" >&2 + set failed 1 else if test -n "$sync_out" set changed 1 test $verbose -eq 1; and echo "$c_ok$sync_out$c_reset" @@ -669,9 +730,12 @@ function agents-vault --description 'track curated agent memory in a host-scoped if git -C "$vault" push -q origin HEAD test $verbose -eq 1; and echo "$c_ok→ Pushed the vault to origin$c_reset" else - # Not fatal: the commit above already happened, so the - # memory is safe locally and the next push will carry it. - echo "$c_warn""agents-vault: push failed; the vault is committed locally$c_reset" >&2 + # The commit above did happen, so the memory is safe + # locally and the next push will carry it -- but nothing + # left this machine, which is the whole point of pushing, + # so this is a failure and not a warning to walk past. + echo "$c_warn""agents-vault: push failed; the vault is committed locally but not backed up off this machine$c_reset" >&2 + set failed 1 end else if set -q _flag_push # An explicit --push that pushed nowhere must not read as a @@ -689,4 +753,8 @@ function agents-vault --description 'track curated agent memory in a host-scoped echo "$c_ok→ Synced agent memory vault$c_reset" end end + + # Explicit, because the branchless `if` above resolves to 0 and would + # otherwise be this function's exit status. + test $failed -eq 0 end diff --git a/tests/test-agents-vault.fish b/tests/test-agents-vault.fish index 475df2f..411f480 100644 --- a/tests/test-agents-vault.fish +++ b/tests/test-agents-vault.fish @@ -771,7 +771,8 @@ check "adopt recorded the rebind" true (string match -q "*$aslug*my-chosen-slug* # An unvalidated --adopt slug is a path-traversal primitive: it lands in # "$vault/projects/$slug" and in `git mv`. Only the charset the slug -# formula itself emits is accepted, and a leading dot is refused too. +# formula itself emits is accepted, plus a by-name refusal of the two +# traversal names that need no slash. set -l bp (new_repo) set -l bmang (string replace -a '/' '-' -- $bp | string replace -a '.' '-') mkdir -p $croot9/$bmang/memory @@ -781,7 +782,7 @@ agents-vault --silent popd >/dev/null set -l projects_before (command ls -A $vroot9/agent-vault/projects | sort | string join ',') -set -l bad_slugs ../escape .hidden has/slash . .. 'UPPER' 'sp ace' '' +set -l bad_slugs ../escape has/slash . .. 'UPPER' 'sp ace' '' pushd $bp >/dev/null for bad in $bad_slugs set -l berr (mktemp); set -ga TMPDIRS $berr @@ -793,7 +794,71 @@ popd >/dev/null check "refused adopts moved nothing" "$projects_before" (command ls -A $vroot9/agent-vault/projects | sort | string join ',') check "refused adopts escaped nothing above projects/" false (test -e $vroot9/agent-vault/escape; and echo true; or echo false) -check "refused adopts created no dotted entry" false (test -e $vroot9/agent-vault/projects/.hidden; and echo true; or echo false) + +# ... but a *leading* dot is legitimate, not traversal. _agents_repo_slug +# emits one for a dot-led subdomain, so refusing it would make such an +# entry impossible to adopt. Inside projects/ it is a hidden directory. +pushd $bp >/dev/null +set -l drc (agents-vault --adopt=.hidden.example.com-repo --silent; echo $status) +popd >/dev/null +check "adopt accepts a leading-dot slug" 0 "$drc" +check "leading-dot slug landed inside projects/" bad (cat $vroot9/agent-vault/projects/.hidden.example.com-repo/claude/memory/b.md 2>/dev/null) +check "leading-dot slug escaped nothing" false (test -e $vroot9/agent-vault/.hidden.example.com-repo; and echo true; or echo false) +check "leading-dot slug repinned the link" (path resolve $vroot9/agent-vault/projects/.hidden.example.com-repo/claude/memory) (path resolve $croot9/$bmang/memory) + +# --adopt must be atomic. A rename that lands while the relink fails +# leaves the memory intact but unreferenced: the next ordinary run finds +# no live link, recomputes the old slug, finds nothing there, and +# fabricates a fresh empty entry, so the agent writes history-less memory +# from then on. The relink is forced to fail by making the project's live +# parent directory read-only, which stops ensure_symlink repinning it. +set -l tp (new_repo https://git.rootiest.dev/rootiest/atomic.git) +set -l tslug git.rootiest.dev-rootiest-atomic +set -l tmang (string replace -a '/' '-' -- $tp | string replace -a '.' '-') +mkdir -p $croot9/$tmang/memory +echo atomic-precious >$croot9/$tmang/memory/keep.md +pushd $tp >/dev/null +agents-vault --silent +popd >/dev/null + +set -l pre_entries (command ls -A $vroot9/agent-vault/projects | sort | string join ',') +set -l pre_head (git -C $vroot9/agent-vault rev-list --count HEAD) +set -l pre_porcelain (git -C $vroot9/agent-vault status --porcelain | string join ',') +set -l pre_link (path resolve $croot9/$tmang/memory) + +set -l terr (mktemp); set -ga TMPDIRS $terr +chmod 500 $croot9/$tmang +pushd $tp >/dev/null +set -l trc (agents-vault --adopt=atomic-target --silent 2>$terr; echo $status) +popd >/dev/null +chmod 700 $croot9/$tmang + +check "atomic adopt: failed relink returns 1" 1 "$trc" +check "atomic adopt: says the entry was left alone" true (string match -q "*$tslug*left as it was*" -- (cat $terr); and echo true; or echo false) +check "atomic adopt: rename rolled back" "$pre_entries" (command ls -A $vroot9/agent-vault/projects | sort | string join ',') +check "atomic adopt: target entry not created" false (test -e $vroot9/agent-vault/projects/atomic-target; and echo true; or echo false) +check "atomic adopt: original entry intact" atomic-precious (cat $vroot9/agent-vault/projects/$tslug/claude/memory/keep.md 2>/dev/null) +check "atomic adopt: no origin note appended" false (string match -q '*adopted:*' -- (cat $vroot9/agent-vault/projects/$tslug/origin); and echo true; or echo false) +check "atomic adopt: nothing committed" "$pre_head" (git -C $vroot9/agent-vault rev-list --count HEAD) +check "atomic adopt: index and worktree unchanged" "$pre_porcelain" (git -C $vroot9/agent-vault status --porcelain | string join ',') +check "atomic adopt: live link never removed" "$pre_link" (path resolve $croot9/$tmang/memory) +check "atomic adopt: no stash left behind" false (test -e $vroot9/agent-vault/.adopt-stash; and echo true; or echo false) + +# The point of rolling back: an ordinary run afterwards must find the +# original entry and must NOT fabricate a second one. +pushd $tp >/dev/null +agents-vault --silent +popd >/dev/null +check "atomic adopt: ordinary run fabricated no entry" "$pre_entries" (command ls -A $vroot9/agent-vault/projects | sort | string join ',') +check "atomic adopt: ordinary run kept the original link" (path resolve $vroot9/agent-vault/projects/$tslug/claude/memory) (path resolve $croot9/$tmang/memory) +check "atomic adopt: memory still reachable through the link" atomic-precious (cat $croot9/$tmang/memory/keep.md 2>/dev/null) + +# ... and once the underlying problem is fixed, --adopt simply works. +pushd $tp >/dev/null +set -l t2rc (agents-vault --adopt=atomic-target --silent; echo $status) +popd >/dev/null +check "atomic adopt: retry succeeds" 0 "$t2rc" +check "atomic adopt: retry moved the memory" atomic-precious (cat $vroot9/agent-vault/projects/atomic-target/claude/memory/keep.md 2>/dev/null) set -e __fish_agent_vault_dir set -e __fish_agent_vault_claude_root @@ -843,6 +908,17 @@ set -l r2rc (agents-vault --restore >$r2out; echo $status) check "restore: exits 0 with an unplaceable entry" 0 "$r2rc" check "restore: reports the unplaceable entry" true (string match -q '*ghost-entry*' -- (cat $r2out); and echo true; or echo false) +# An entry that *could* be placed but could not be relinked is a failure, +# not a note in passing -- the same branchless-`if` false zero as the +# commit and push paths. An entry with no live project is not a failure. +rm -f $croot10/$rmang/memory +chmod 500 $croot10/$rmang +set -l r3err (mktemp); set -ga TMPDIRS $r3err +set -l r3rc (agents-vault --restore >/dev/null 2>$r3err; echo $status) +chmod 700 $croot10/$rmang +check "restore: a failed relink returns non-zero" 1 "$r3rc" +check "restore: a failed relink is reported" true (string match -q '*restoreme*' -- (cat $r3err); and echo true; or echo false) + set -e __fish_agent_vault_dir set -e __fish_agent_vault_claude_root set -g __fish_agent_vault_claude_home $HERMETIC_HOME/claude @@ -902,6 +978,33 @@ popd >/dev/null set -e __fish_agent_vault_autopush check "autopush pushes when enabled" pushed-auto (git -C $bare show $vbranch:projects/$pslug/claude/memory/p3.md 2>/dev/null) +# A push that *fails* against a configured remote must return non-zero. +# The function otherwise ends on a branchless `if`, which resolves to 0, +# so warning on stderr and falling through reports a successful backup +# while nothing left the machine -- the exact loss the vault prevents. +# The remote is a real path that is not a repository, not a mock. +set -l deadremote $vroot11/not-a-repo.git +agents-vault --remote=$deadremote --silent +echo pushed-never >$croot11/$pmang/memory/p4.md +set -l fperr (mktemp); set -ga TMPDIRS $fperr +pushd $pp >/dev/null +set -l fprc (agents-vault --push --silent 2>$fperr; echo $status) +popd >/dev/null +check "failing push returns non-zero" 1 "$fprc" +check "failing push warns on stderr" true (string match -q '*push failed*' -- (cat $fperr); and echo true; or echo false) +check "failing push still committed locally" true (git -C $vroot11/agent-vault ls-files --error-unmatch projects/$pslug/claude/memory/p4.md >/dev/null 2>&1; and echo true; or echo false) + +# Autopush is the same failure through the quiet path: a summary line +# saying "Synced" must not come with a zero exit when the push failed. +echo pushed-never-2 >$croot11/$pmang/memory/p5.md +set -g __fish_agent_vault_autopush 1 +set -l fp2err (mktemp); set -ga TMPDIRS $fp2err +pushd $pp >/dev/null +set -l fp2rc (agents-vault --quiet 2>$fp2err >/dev/null; echo $status) +popd >/dev/null +set -e __fish_agent_vault_autopush +check "failing autopush returns non-zero" 1 "$fp2rc" + set -e __fish_agent_vault_dir set -e __fish_agent_vault_claude_root set -g __fish_agent_vault_claude_home $HERMETIC_HOME/claude