diff --git a/docs/manual/07-customization.md b/docs/manual/07-customization.md index e5f1a1a..02a9559 100644 --- a/docs/manual/07-customization.md +++ b/docs/manual/07-customization.md @@ -170,7 +170,9 @@ full sub-category breakdown of every category. When set to 1, agents-vault also pushes on wrapper launch. Defaults to off: the vault commits locally on every launch and pushes from the - Claude Code SessionEnd hook or an explicit agents-vault --push. + Claude Code SessionEnd hook or an explicit agents-vault --push. That + push is synchronous, so with autopush on the pull and the push are + each capped at 20 seconds; an explicit --push is left uncapped. NOTE: With autopush off and no SessionEnd hook installed, backups accumulate diff --git a/functions/agents-vault.fish b/functions/agents-vault.fish index 393d8ae..9193c65 100644 --- a/functions/agents-vault.fish +++ b/functions/agents-vault.fish @@ -38,7 +38,9 @@ # allowlist runs all the way down, not just at the top: inside agy's # knowledge store only *.md and *.json files are copied, so a credential # file or a conversation database appearing there is left behind by the -# same rule rather than by being known about in advance. +# same rule rather than by being known about in advance. Symlinks found +# inside the store are neither followed nor copied, so the allowlist +# bounds whose files it collects and not merely what kind. # # Global state that belongs to no project is tracked as well. Claude's # global memory directory (~/.claude/memory) is symlinked into the vault @@ -131,9 +133,13 @@ # # NOTES # Set __fish_agent_vault_dir to relocate the vault. Set -# __fish_agent_vault_autopush to 1 to also push on wrapper launch; -# it defaults to off so a backgrounded push can never hang or prompt -# invisibly underneath a starting agent. +# __fish_agent_vault_autopush to 1 to also push on wrapper launch; it +# defaults to off because that push is synchronous and so delays every +# launch. With it on, the pull and the push are each capped at 20 +# seconds, since git has no connect timeout of its own and an +# unreachable remote otherwise blocks for minutes. An explicit --push +# is left uncapped: it is watched, and it must report what a real +# transfer really did. # # --adopt rebinds an entry; it does not pin its name. The slug is # re-derived from the project on every run, so the next ordinary run @@ -329,10 +335,12 @@ function agents-vault --description 'track curated agent memory in a host-scoped '*.db-wal' \ '*.db-shm' \ '' \ - '# --adopt stashes the entry it is about to overwrite inside .git/,' \ - '# out of reach of `git add -A`. This covers the fallback location' \ - '# it uses when .git is not a directory.' \ - '/.adopt-stash' >"$vault/.gitignore" + '# --adopt and the slug migration each stash the entry they are' \ + '# about to overwrite inside .git/, out of reach of `git add -A`.' \ + '# These cover the fallback locations they use when .git is not a' \ + '# directory.' \ + '/.adopt-stash' \ + '/.migrate-stash' >"$vault/.gitignore" set changed 1 end @@ -627,7 +635,39 @@ function agents-vault --description 'track curated agent memory in a host-scoped # Fish wildcards skip dot-led names at every path component, so # dotfiles and hidden subdirectories are already out; the extension # allowlist is what keeps them out on purpose rather than by luck. - set -l kfiles $knowledge/**.md $knowledge/**.json + # + # The tree is walked a level at a time rather than globbed with **, + # because a symlink has to stop the walk and ** has no way to say + # so. A recursive ** descends through a symlinked directory and cp + # follows a symlinked file, which between them undo the whole point + # of the allowlist twice over. A `ln -s ~ knowledge/x` puts every + # qualifying .md and .json in the home directory -- settings.json, + # CLAUDE.md, cache and status files -- into the vault and into a + # commit: the extension rule still bounds what *kind* of file goes + # in, but the store boundary that decides *whose* files they are is + # gone. Worse, `ln -s / knowledge/x` makes the walk itself + # unbounded, and this runs synchronously in front of every agent + # launch. Fish does stop a true self-referential cycle; a symlink + # to a merely enormous tree is not a cycle. + # + # So nothing that is a symlink is ever followed or copied, whether + # it names a file or a directory. The knowledge store's own root + # may still be a link -- that one is the configured location of the + # store rather than something found inside it. + set -l kfiles + set -l kdirs "$knowledge" + while set -q kdirs[1] + set -l dir $kdirs[1] + set -e kdirs[1] + for e in $dir/* + test -L "$e"; and continue + if test -d "$e" + set -a kdirs "$e" + else if string match -qr '\.(md|json)$' -- "$e" + set -a kfiles "$e" + end + end + end set -l kfailed 0 for f in $kfiles test -f "$f"; or continue @@ -836,7 +876,18 @@ function agents-vault --description 'track curated agent memory in a host-scoped end printf 'renamed: %s → %s (%s)\n' "$prev_slug" "$slug" (date -I) \ >>"$entry/origin" - rm -f "$live" + # Only a link is dropped here, and only so the relink below has + # somewhere to put the new one. Usually $live is exactly that: a + # symlink at the old entry, now dangling. But the migration is + # also reachable from the path-derived fallback candidate, and + # there $live can be a real, populated directory -- someone's + # actual memory. rm -f cannot delete it, which is the right + # outcome, but it says so on stderr in rm's own voice, so a + # --silent run that succeeded printed what reads as an error. + # The directory case needs no removal anyway: + # _agents_repo_ensure_symlink copies a populated live directory + # into the vault without clobbering before it replaces it. + test -L "$live"; and rm -f "$live" set changed 1 test $verbose -eq 1; and echo "$c_ok→ Migrated vault entry $prev_slug → $slug$c_reset" end @@ -930,10 +981,41 @@ function agents-vault --description 'track curated agent memory in a host-scoped # helper, which git consults before it ever falls back to # prompting; they only close off the interactive last resort, # which under a starting agent is indistinguishable from a hang. + # + # They close the prompt, not the socket, and git has no knob + # that closes the socket either: there is no HTTP connect + # timeout in its configuration at all, and http.lowSpeedLimit / + # http.lowSpeedTime -- the usual suggestion -- only start + # counting once bytes are moving, so they expire never against + # an address that simply blackholes the SYN. Measured against + # 192.0.2.1 with both set: no return inside 30s; unset: 135s. + # + # ssh is the one transport that can time itself out, so it is + # told to, unless the user has already said how to run ssh. + # Everything else is bounded from outside with timeout(1). + set -l gitenv GIT_TERMINAL_PROMPT=0 GIT_ASKPASS=true + set -q GIT_SSH_COMMAND + or set -a gitenv 'GIT_SSH_COMMAND=ssh -o ConnectTimeout=10' + set -l gitnet env $gitenv + + # Only autopush is wrapped. An explicit --push is a thing the + # user asked for and is watching, and it must report the real + # exit status of a real transfer, so it is allowed to take as + # long as the transfer honestly takes. Autopush runs + # synchronously in front of every agent launch, which is the + # block this whole design exists to remove; there an + # unreachable remote costs a bounded 20s per operation instead + # of an open-ended wait. timeout's own 124 is a non-zero exit + # like any other, so a bounded push still reports as failed and + # the commit it could not send has already landed locally. A + # push too slow for the bound can always be run by hand. + if not set -q _flag_push; and type -q timeout + set gitnet timeout 20 $gitnet + end set -l reached 1 if git -C "$vault" rev-parse --abbrev-ref --symbolic-full-name '@{u}' >/dev/null 2>&1 - if not GIT_TERMINAL_PROMPT=0 GIT_ASKPASS=true \ - git -C "$vault" pull --rebase --autostash -q >/dev/null 2>/dev/null + if not $gitnet git -C "$vault" pull --rebase --autostash -q \ + >/dev/null 2>/dev/null # Two unrelated failures land here and reporting them as # one sends the user hunting for a conflict that never # existed. A rebase that genuinely started and stopped @@ -956,7 +1038,7 @@ function agents-vault --description 'track curated agent memory in a host-scoped # push would only fail a second time, more confusingly, and the # pull has already said exactly what went wrong. if test $reached -eq 1 - if GIT_TERMINAL_PROMPT=0 GIT_ASKPASS=true git -C "$vault" push -q origin HEAD + if $gitnet git -C "$vault" push -q origin HEAD test $verbose -eq 1; and echo "$c_ok→ Pushed the vault to origin$c_reset" else # The commit above did happen, so the memory is safe diff --git a/tests/test-agents-vault.fish b/tests/test-agents-vault.fish index eee5415..e3aa4aa 100644 --- a/tests/test-agents-vault.fish +++ b/tests/test-agents-vault.fish @@ -563,6 +563,54 @@ set -l dirty_new_slug git.rootiest.dev-rootiest-dirty check "fallback finds sanitized local entry" dirty-precious (cat $vroot2/agent-vault/projects/$dirty_new_slug/claude/memory/keep.md) check "fallback old entry removed" false (test -d $vroot2/agent-vault/projects/$dirty_local_slug; and echo true; or echo false) +# Same fallback path, but the live memory directory is a real populated +# directory rather than a symlink -- the shape a machine ends up in when +# an agent wrote memory while the link was missing. The migration then +# tries to clear the live path out of the relink's way, and clearing a +# directory is not something it may do: that is somebody's memory, and +# _agents_repo_ensure_symlink already folds it into the vault without +# clobbering. Refusing is therefore correct, but refusing in rm's voice +# is not: a --silent run that did the right thing and returned 0 still +# printed "rm: cannot remove ...: Is a directory", which is the only +# thing the user sees and reads as a failure. +set -l real_root (mktemp -d); set -ga TMPDIRS $real_root +set -l rp "$real_root/proj" +mkdir -p "$rp" +git -C "$rp" init -q +git -C "$rp" config user.email t@t +git -C "$rp" config user.name t +git -C "$rp" config commit.gpgsign false +git -C "$rp" config core.hooksPath /dev/null + +set -l rmangled (string replace -a '/' '-' -- $rp | string replace -a '.' '-') +mkdir -p $croot2/$rmangled/memory +echo "banked" >$croot2/$rmangled/memory/old.md + +pushd $rp >/dev/null +agents-vault --silent +set -l real_local_slug (_agents_repo_slug $rp) +popd >/dev/null + +# Replace the link with a real directory holding memory the vault has +# never seen, then change the slug so the migration runs. +rm -f $croot2/$rmangled/memory +mkdir -p $croot2/$rmangled/memory +echo "written-live" >$croot2/$rmangled/memory/fresh.md +git -C $rp remote add origin https://git.rootiest.dev/rootiest/realdir.git + +set -l rerr (mktemp); set -ga TMPDIRS $rerr +pushd $rp >/dev/null +set -l real_rc (agents-vault --silent 2>$rerr; echo $status) +popd >/dev/null +set -l real_err (cat $rerr) +set -l real_new_slug git.rootiest.dev-rootiest-realdir +check "real-directory migration succeeds" 0 "$real_rc" +check "real-directory migration stays silent" "" "$real_err" +check "real-directory migration keeps banked memory" banked (cat $vroot2/agent-vault/projects/$real_new_slug/claude/memory/old.md 2>/dev/null) +check "real-directory migration keeps live memory" written-live (cat $vroot2/agent-vault/projects/$real_new_slug/claude/memory/fresh.md 2>/dev/null) +check "real-directory migration relinks" true (test -L $croot2/$rmangled/memory; and echo true; or echo false) +check "real-directory old entry removed" false (test -d $vroot2/agent-vault/projects/$real_local_slug; and echo true; or echo false) + set -e __fish_agent_vault_dir set -e __fish_agent_vault_claude_root @@ -699,6 +747,30 @@ printf 'transcript\n' >$agy5/knowledge/session.jsonl : >$agy5/knowledge/conversations.db-wal : >$agy5/knowledge/conversations.db-shm +# Symlinks inside the store. The extension allowlist bounds what *kind* of +# file is collected; it says nothing about whose. A store-relative walk +# that dereferenced links would pull qualifying .md and .json out of +# whatever the link names -- a real home directory is full of them -- so +# the store boundary has to hold on its own. $outside5 stands in for that +# home: a directory the store has no business reaching into, planted with +# exactly the shapes that qualify. +set -l outside5 (mktemp -d); set -ga TMPDIRS $outside5 +mkdir -p $outside5/nested +echo SECRET-OUTSIDE-KNOWLEDGE >$outside5/leaked.json +echo SECRET-OUTSIDE-KNOWLEDGE >$outside5/target.md +echo SECRET-OUTSIDE-KNOWLEDGE >$outside5/nested/deep.md +ln -s $outside5 $agy5/knowledge/linked +ln -s $outside5/target.md $agy5/knowledge/alias.md +# Not a cycle, so a recursive glob's cycle guard does not catch it: a link +# to the filesystem root simply makes the walk enormous. This one is here +# for the clock as much as for the contents -- the copy runs synchronously +# in front of every agent launch, and a `**` glob over this fixture did +# not return within 20s. +ln -s / $agy5/knowledge/root +# A genuine cycle too, since the walk must not depend on the glob's guard. +mkdir -p $agy5/knowledge/cyc +ln -s $agy5/knowledge $agy5/knowledge/cyc/loop + # A global (non-per-project) Claude memory directory with a sentinel file. # __fish_agent_vault_claude_home is what keeps this off the real ~/.claude: # if agents-vault ignored the override, these checks would fail here *and* @@ -707,9 +779,11 @@ mkdir -p $chome5/memory echo global-memory >$chome5/memory/g.md set -l gp (new_repo https://git.rootiest.dev/rootiest/globals.git) +set -l t5_start (date +%s) pushd $gp >/dev/null agents-vault --silent popd >/dev/null +set -l t5_elapsed (math (date +%s) - $t5_start) check "agy knowledge copied" learned (cat $vroot5/agent-vault/global/agy/knowledge/fact.md) check "agy settings copied" '{"model":"x"}' (cat $vroot5/agent-vault/global/agy/settings.json) @@ -726,9 +800,27 @@ end # Not merely absent from the worktree: absent from the history, which is # what actually leaves the machine on a push. check "knowledge: no secret reached a commit" false (git -C $vroot5/agent-vault grep -q SECRET-INSIDE-KNOWLEDGE HEAD -- global 2>/dev/null; and echo true; or echo false) + +# Symlinks: nothing the links name may appear, under any name. The linked +# directory must not exist in the vault at all (following it would recreate +# its tree wholesale), and the aliased file must not exist either, even +# though its own name qualifies -- a dereferencing copy writes a real file +# at the link's name and the extension rule waves it through. +for escapee in linked linked/leaked.json linked/nested/deep.md alias.md root cyc/loop + check "knowledge: symlinked $escapee stayed out" false (test -e $vroot5/agent-vault/global/agy/knowledge/$escapee; and echo true; or echo false) +end +check "knowledge: nothing outside the store reached a commit" false (git -C $vroot5/agent-vault grep -q SECRET-OUTSIDE-KNOWLEDGE HEAD 2>/dev/null; and echo true; or echo false) +# The clock, not the contents: a walk that descends a link to / does not +# finish, and this is the every-launch path. Generous enough that a loaded +# machine cannot fail it by being slow. +check "knowledge: a link to / does not stall the launch path" true (test $t5_elapsed -lt 20; and echo true; or echo false) # A torn database with its completing write-ahead log deliberately excluded # is worse than no database at all, so the scaffold ignores all three. check "scaffolded .gitignore excludes *.db" true (grep -qxF '*.db' $vroot5/agent-vault/.gitignore; and echo true; or echo false) +# Both stash fallbacks, or a crash mid-rename leaves a shadow copy of an +# entry sitting at the vault root for the next `git add -A` to commit. +check "scaffolded .gitignore excludes /.adopt-stash" true (grep -qxF '/.adopt-stash' $vroot5/agent-vault/.gitignore; and echo true; or echo false) +check "scaffolded .gitignore excludes /.migrate-stash" true (grep -qxF '/.migrate-stash' $vroot5/agent-vault/.gitignore; and echo true; or echo false) check "global claude memory in the vault" global-memory (cat $vroot5/agent-vault/global/claude/memory/g.md) check "global claude memory is now a link" true (test -L $chome5/memory; and echo true; or echo false) @@ -1369,6 +1461,33 @@ popd >/dev/null set -e __fish_agent_vault_autopush check "failing autopush returns non-zero" 1 "$fp2rc" +# Autopush runs synchronously in front of every agent launch, so it must be +# bounded. Nothing in git bounds it: there is no HTTP connect timeout in +# its configuration, http.lowSpeedLimit/http.lowSpeedTime only start +# counting once bytes move, and GIT_TERMINAL_PROMPT/GIT_ASKPASS close the +# credential prompt rather than the socket. Unwrapped, this remote took +# 135s to give up. 192.0.2.1 is TEST-NET-1: reserved, unrouted, and +# therefore a blackhole rather than a fast refusal. A network that does +# refuse it quickly makes this pass without proving much, which is the +# right way round for a test that must never fail spuriously. +# +# This test costs its own bound in wall time. That is the price of +# measuring a timeout, and this defect -- a network call on the launch +# path -- has now been introduced twice. +set -l blackhole https://192.0.2.1/vault.git +agents-vault --remote=$blackhole --silent +echo pushed-into-the-void >$croot11/$pmang/memory/p6.md +set -g __fish_agent_vault_autopush 1 +set -l bh_start (date +%s) +pushd $pp >/dev/null +set -l bhrc (agents-vault --silent 2>/dev/null; echo $status) +popd >/dev/null +set -l bh_elapsed (math (date +%s) - $bh_start) +set -e __fish_agent_vault_autopush +check "autopush to a blackholed remote returns non-zero" 1 "$bhrc" +check "autopush to a blackholed remote is bounded" true (test $bh_elapsed -lt 60; and echo true; or echo false) +check "a bounded autopush still committed locally" true (git -C $vroot11/agent-vault ls-files --error-unmatch projects/$pslug/claude/memory/p6.md >/dev/null 2>&1; 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