fix(agents-vault): keep the user's ssh command and drop the quadratic walk
The connect bound was delivered by injecting GIT_SSH_COMMAND, and an environment variable outranks git's core.sshCommand -- so the guard, which read only the environment, did not merely miss a configured ssh command, it overruled one. A vault remote reachable only as `ssh -i ~/.ssh/vault_key` failed to authenticate on every push, autopush and --push alike, for the sake of a ten-second timeout. Both spellings now count, and `set -qx` rather than `set -q` on the environment side so an unexported fish variable -- which git never sees -- does not leave the push with neither the user's ssh command nor a bound. The agy knowledge walk appended each find with `set -a`, which rewrites the whole variable every time; 500 files cost 21ms but 20,000 cost 58s, on a path that runs in front of every agent launch. The walk now prints NUL-separated and the list is built once, which is flat: the same 20,000 files take 707ms. NUL rather than newline because a filename may legally contain one. What the walk collects, and its symlink and dot-led semantics, are byte-for-byte unchanged. Autopush is bounded by timeout(1) alone, so without it the launch path was quietly back to an open-ended network call. It now says so and skips the push instead; --push was never wrapped and is unaffected.
This commit is contained in:
@@ -139,7 +139,17 @@
|
||||
# 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.
|
||||
# transfer really did. The cap is timeout(1); on a system that somehow
|
||||
# lacks it, autopush says so on stderr and does not push at all, since
|
||||
# an unbounded network call in front of a launch is the one outcome the
|
||||
# cap exists to prevent. --push still works there.
|
||||
#
|
||||
# Over ssh the cap is delivered by setting GIT_SSH_COMMAND, which would
|
||||
# silently outrank the user's own configuration -- so it is not set at
|
||||
# all when GIT_SSH_COMMAND is already exported or git's core.sshCommand
|
||||
# is configured. A vault remote reachable only through a particular
|
||||
# identity file or ssh wrapper therefore keeps it, uncapped, rather than
|
||||
# failing to authenticate for the sake of a timeout.
|
||||
#
|
||||
# --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
|
||||
@@ -654,7 +664,22 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
# 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
|
||||
#
|
||||
# The walk prints its finds and the list is built once from that
|
||||
# output, rather than appending each path to a list as it goes.
|
||||
# `set -a` rewrites the whole variable every time, so appending n
|
||||
# paths one at a time costs O(n^2) copying -- 500 files took 21ms
|
||||
# and 20,000 took 58s on this machine, on a path that runs in
|
||||
# front of every agent launch. Printing is flat: the same 20,000
|
||||
# files take 756ms, and 500 take 13ms. Batching per directory does
|
||||
# not help, because a knowledge store is mostly one flat directory
|
||||
# and that is exactly where the growing list lives.
|
||||
#
|
||||
# NUL separators and `string split0`, not newlines: a filename may
|
||||
# legally contain a newline, and splitting on one would saw such a
|
||||
# path into two entries and copy neither. NUL is the one byte a
|
||||
# path cannot hold, so the round trip is lossless.
|
||||
set -l kfiles (begin
|
||||
set -l kdirs "$knowledge"
|
||||
while set -q kdirs[1]
|
||||
set -l dir $kdirs[1]
|
||||
@@ -664,10 +689,11 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
if test -d "$e"
|
||||
set -a kdirs "$e"
|
||||
else if string match -qr '\.(md|json)$' -- "$e"
|
||||
set -a kfiles "$e"
|
||||
printf '%s\0' "$e"
|
||||
end
|
||||
end
|
||||
end
|
||||
end | string split0)
|
||||
set -l kfailed 0
|
||||
for f in $kfiles
|
||||
test -f "$f"; or continue
|
||||
@@ -887,6 +913,16 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
# 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.
|
||||
#
|
||||
# A stray *regular* file at $live is left alone too, and that is
|
||||
# deliberate rather than an oversight in the test. Nothing this
|
||||
# function created is a plain file there, so whatever it is came
|
||||
# from the user or from something else writing to the same path,
|
||||
# and deleting it unasked would destroy data to make room for a
|
||||
# symlink. _agents_repo_ensure_symlink refuses the path and says
|
||||
# why, and the launch stops until someone looks -- the same
|
||||
# answer the global-memory block gives for the same shape of
|
||||
# surprise.
|
||||
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"
|
||||
@@ -965,6 +1001,20 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
if set -q __fish_agent_vault_autopush; and test "$__fish_agent_vault_autopush" = 1
|
||||
set do_push 1
|
||||
end
|
||||
# timeout(1) is the only thing bounding autopush, and an unbounded
|
||||
# network call in front of an agent launch is the exact block this
|
||||
# design exists to remove -- so without it, autopush does not happen
|
||||
# at all rather than happening open-endedly. Nothing is lost that was
|
||||
# not already local: the commit has landed, and `agents-vault --push`
|
||||
# still sends it by hand, deliberately unbounded because the user is
|
||||
# watching that one. It says so rather than skipping quietly, because
|
||||
# a vault that stopped leaving the machine must never look like one
|
||||
# that did not. timeout ships with coreutils, so this is a guard
|
||||
# against the impossible-until-it-happens, not a real dependency.
|
||||
if test $do_push -eq 1; and not set -q _flag_push; and not type -q timeout
|
||||
echo "$c_warn""agents-vault: timeout is unavailable, so autopush cannot be bounded; the vault is committed locally but not pushed$c_reset" >&2
|
||||
set do_push 0
|
||||
end
|
||||
if test $do_push -eq 1
|
||||
if git -C "$vault" remote get-url origin >/dev/null 2>&1
|
||||
# The pull belongs here and nowhere earlier. Fetching is only
|
||||
@@ -993,9 +1043,28 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
# 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).
|
||||
#
|
||||
# "Already said" has two spellings and both have to count.
|
||||
# GIT_SSH_COMMAND is the obvious one; core.sshCommand is the
|
||||
# documented place to name an identity file or an ssh wrapper,
|
||||
# and it is the one a vault on a private host is most likely to
|
||||
# need. An injected environment variable outranks the config,
|
||||
# so consulting only the environment does not merely miss the
|
||||
# user's setting -- it overrides it, and a remote reachable
|
||||
# only as `ssh -i ~/.ssh/vault_key` then fails to authenticate
|
||||
# on every push. A ten-second connect bound is not worth that.
|
||||
#
|
||||
# -qx rather than -q on the environment side: a fish variable
|
||||
# that was never exported satisfies -q but is not in the
|
||||
# environment git runs in, so treating it as the user's answer
|
||||
# would leave the push with neither their ssh command nor a
|
||||
# connect timeout. Only what git can actually see counts, and
|
||||
# exporting it is the user's call to make, not this function's.
|
||||
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 user_ssh (git -C "$vault" config --get core.sshCommand 2>/dev/null)
|
||||
if not set -qx GIT_SSH_COMMAND; and test -z "$user_ssh"
|
||||
set -a gitenv 'GIT_SSH_COMMAND=ssh -o ConnectTimeout=10'
|
||||
end
|
||||
set -l gitnet env $gitenv
|
||||
|
||||
# Only autopush is wrapped. An explicit --push is a thing the
|
||||
@@ -1008,8 +1077,10 @@ function agents-vault --description 'track curated agent memory in a host-scoped
|
||||
# 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
|
||||
# push too slow for the bound can always be run by hand. An
|
||||
# autopush with no timeout(1) to wrap it never reaches here;
|
||||
# it was turned off above.
|
||||
if not set -q _flag_push
|
||||
set gitnet timeout 20 $gitnet
|
||||
end
|
||||
set -l reached 1
|
||||
|
||||
@@ -1493,6 +1493,184 @@ set -e __fish_agent_vault_claude_root
|
||||
set -g __fish_agent_vault_claude_home $HERMETIC_HOME/claude
|
||||
set -g __fish_agent_vault_agy_root $HERMETIC_HOME/agy
|
||||
|
||||
# ──────────────── the bound never overrides the user's ssh ─────────────
|
||||
# The connect bound is delivered by injecting GIT_SSH_COMMAND, and an
|
||||
# environment variable outranks core.sshCommand -- so a guard that reads
|
||||
# only the environment does not merely fail to notice the config, it
|
||||
# overrules it. A vault reachable only as `ssh -i ~/.ssh/vault_key` then
|
||||
# fails to authenticate on every push, autopush and --push alike, for the
|
||||
# sake of a ten-second timeout. Counting ssh invocations is the honest
|
||||
# measurement here: it asks what git actually ran, not what this function
|
||||
# meant to arrange.
|
||||
echo ""
|
||||
echo "== agents-vault (the user's ssh command wins) =="
|
||||
|
||||
set -l vroot14 (mktemp -d); set -ga TMPDIRS $vroot14
|
||||
set -l croot14 (mktemp -d); set -ga TMPDIRS $croot14
|
||||
set -l chome14 (mktemp -d); set -ga TMPDIRS $chome14
|
||||
set -l agy14 (mktemp -d); set -ga TMPDIRS $agy14
|
||||
set -g __fish_agent_vault_dir $vroot14/agent-vault
|
||||
set -g __fish_agent_vault_claude_root $croot14
|
||||
set -g __fish_agent_vault_claude_home $chome14
|
||||
set -g __fish_agent_vault_agy_root $agy14
|
||||
|
||||
# Two fake ssh binaries that record their arguments: one found on PATH as
|
||||
# plain `ssh` (what the injected default resolves to) and one named
|
||||
# explicitly by the user. Both refuse the connection, so nothing leaves
|
||||
# the machine and no test waits on a network.
|
||||
set -l sbin (mktemp -d); set -ga TMPDIRS $sbin
|
||||
set -l pathssh_log $sbin/path-ssh.log
|
||||
set -l usessh $sbin/user-ssh
|
||||
set -l usessh_log $sbin/user-ssh.log
|
||||
printf '#!/bin/sh\nprintf "%%s\\n" "$*" >>%s\nexit 255\n' $pathssh_log >$sbin/ssh
|
||||
printf '#!/bin/sh\nprintf "%%s\\n" "$*" >>%s\nexit 255\n' $usessh_log >$usessh
|
||||
chmod +x $sbin/ssh $usessh
|
||||
|
||||
set -l sp14 (new_repo https://git.rootiest.dev/rootiest/sshcmd.git)
|
||||
set -l smang14 (string replace -a '/' '-' -- $sp14 | string replace -a '.' '-')
|
||||
mkdir -p $croot14/$smang14/memory
|
||||
echo s1 >$croot14/$smang14/memory/s1.md
|
||||
pushd $sp14 >/dev/null
|
||||
agents-vault --remote='ssh://git@example.invalid/vault.git' --silent >/dev/null 2>&1
|
||||
popd >/dev/null
|
||||
|
||||
set -l realpath14 $PATH
|
||||
set -g PATH $sbin $PATH
|
||||
|
||||
# Runs one autopush (or an explicit --push) with a fresh memory file, so
|
||||
# there is always a commit worth pushing, and reports nothing itself.
|
||||
function ssh_run --argument-names memroot proj name mode
|
||||
rm -f $argv[5..]
|
||||
echo $name >$memroot/memory/$name.md
|
||||
test "$mode" = auto; and set -g __fish_agent_vault_autopush 1
|
||||
pushd $proj >/dev/null
|
||||
if test "$mode" = auto
|
||||
agents-vault --silent >/dev/null 2>&1
|
||||
else
|
||||
agents-vault --push --silent >/dev/null 2>&1
|
||||
end
|
||||
popd >/dev/null
|
||||
set -e __fish_agent_vault_autopush
|
||||
end
|
||||
|
||||
git -C $vroot14/agent-vault config core.sshCommand $usessh
|
||||
ssh_run $croot14/$smang14 $sp14 s2 auto $usessh_log $pathssh_log
|
||||
check "autopush runs the user's core.sshCommand" true (test -s $usessh_log; and echo true; or echo false)
|
||||
ssh_run $croot14/$smang14 $sp14 s3 push $usessh_log $pathssh_log
|
||||
check "--push runs the user's core.sshCommand" true (test -s $usessh_log; and echo true; or echo false)
|
||||
|
||||
# With nothing configured either way the bound is still applied -- the
|
||||
# fix must not have simply removed it.
|
||||
git -C $vroot14/agent-vault config --unset core.sshCommand
|
||||
ssh_run $croot14/$smang14 $sp14 s4 auto $usessh_log $pathssh_log
|
||||
check "an unconfigured ssh still gets a connect bound" true (string match -q '*ConnectTimeout=10*' -- (cat $pathssh_log 2>/dev/null); and echo true; or echo false)
|
||||
|
||||
# A fish variable that was never exported satisfies `set -q` but is not in
|
||||
# git's environment, so treating it as the user's answer would leave the
|
||||
# push with neither their ssh command nor a bound.
|
||||
set -g GIT_SSH_COMMAND $usessh
|
||||
ssh_run $croot14/$smang14 $sp14 s5 auto $usessh_log $pathssh_log
|
||||
set -e GIT_SSH_COMMAND
|
||||
check "an unexported GIT_SSH_COMMAND does not suppress the bound" true (string match -q '*ConnectTimeout=10*' -- (cat $pathssh_log 2>/dev/null); and echo true; or echo false)
|
||||
|
||||
# An exported one is git's already and is passed through untouched.
|
||||
set -gx GIT_SSH_COMMAND "$usessh -o Marker=yes"
|
||||
ssh_run $croot14/$smang14 $sp14 s6 auto $usessh_log $pathssh_log
|
||||
set -e GIT_SSH_COMMAND
|
||||
check "an exported GIT_SSH_COMMAND is used verbatim" true (string match -q '*Marker=yes*' -- (cat $usessh_log 2>/dev/null); and echo true; or echo false)
|
||||
check "an exported GIT_SSH_COMMAND is not overridden" false (string match -q '*ConnectTimeout*' -- (cat $usessh_log 2>/dev/null); and echo true; or echo false)
|
||||
|
||||
set -g PATH $realpath14
|
||||
functions -e ssh_run
|
||||
set -e __fish_agent_vault_dir
|
||||
set -e __fish_agent_vault_claude_root
|
||||
set -g __fish_agent_vault_claude_home $HERMETIC_HOME/claude
|
||||
set -g __fish_agent_vault_agy_root $HERMETIC_HOME/agy
|
||||
|
||||
# ─────────────── autopush without timeout(1) does not run ──────────────
|
||||
# timeout(1) is the only thing bounding autopush, so without it the launch
|
||||
# path would be back to an open-ended network call -- the exact block this
|
||||
# design exists to remove. The push is skipped and says so instead. An
|
||||
# explicit --push is deliberately unaffected: it was never wrapped.
|
||||
#
|
||||
# The PATH here is the real one with every directory that provides a
|
||||
# timeout replaced by a symlink farm of itself missing that one entry, so
|
||||
# everything else agents-vault shells out to still resolves normally.
|
||||
echo ""
|
||||
echo "== agents-vault (autopush without timeout) =="
|
||||
|
||||
set -l vroot15 (mktemp -d); set -ga TMPDIRS $vroot15
|
||||
set -l croot15 (mktemp -d); set -ga TMPDIRS $croot15
|
||||
set -l chome15 (mktemp -d); set -ga TMPDIRS $chome15
|
||||
set -l agy15 (mktemp -d); set -ga TMPDIRS $agy15
|
||||
set -l bare15 (mktemp -d); set -ga TMPDIRS $bare15
|
||||
git init -q --bare $bare15
|
||||
set -g __fish_agent_vault_dir $vroot15/agent-vault
|
||||
set -g __fish_agent_vault_claude_root $croot15
|
||||
set -g __fish_agent_vault_claude_home $chome15
|
||||
set -g __fish_agent_vault_agy_root $agy15
|
||||
|
||||
set -l tp15 (new_repo https://git.rootiest.dev/rootiest/notimeout.git)
|
||||
set -l tslug15 git.rootiest.dev-rootiest-notimeout
|
||||
set -l tmang15 (string replace -a '/' '-' -- $tp15 | string replace -a '.' '-')
|
||||
mkdir -p $croot15/$tmang15/memory
|
||||
echo t1 >$croot15/$tmang15/memory/t1.md
|
||||
pushd $tp15 >/dev/null
|
||||
agents-vault --remote=$bare15 --silent >/dev/null 2>&1
|
||||
popd >/dev/null
|
||||
# symbolic-ref, not rev-parse: the vault branch is still unborn here and
|
||||
# rev-parse would report the literal string HEAD with a fatal on stderr.
|
||||
set -l vb15 (git -C $vroot15/agent-vault symbolic-ref --short HEAD)
|
||||
|
||||
set -l shimroot (mktemp -d); set -ga TMPDIRS $shimroot
|
||||
set -l nopath
|
||||
set -l shimn 0
|
||||
for d in $PATH
|
||||
test -d "$d"; or continue
|
||||
if test -x "$d/timeout"
|
||||
set shimn (math $shimn + 1)
|
||||
mkdir -p $shimroot/$shimn
|
||||
command cp -rs "$d/." $shimroot/$shimn/ 2>/dev/null
|
||||
rm -f $shimroot/$shimn/timeout
|
||||
set -a nopath $shimroot/$shimn
|
||||
else
|
||||
set -a nopath $d
|
||||
end
|
||||
end
|
||||
set -l realpath15 $PATH
|
||||
|
||||
echo t2 >$croot15/$tmang15/memory/t2.md
|
||||
set -l terr (mktemp); set -ga TMPDIRS $terr
|
||||
set -g __fish_agent_vault_autopush 1
|
||||
set -g PATH $nopath
|
||||
check "the shimmed PATH really has no timeout" false (type -q timeout; and echo true; or echo false)
|
||||
pushd $tp15 >/dev/null
|
||||
set -l trc (agents-vault --silent 2>$terr; echo $status)
|
||||
popd >/dev/null
|
||||
set -g PATH $realpath15
|
||||
set -e __fish_agent_vault_autopush
|
||||
check "an unboundable autopush is skipped, not run" false (git -C $bare15 cat-file -e $vb15:projects/$tslug15/claude/memory/t2.md 2>/dev/null; and echo true; or echo false)
|
||||
check "a skipped autopush says why" true (string match -q '*timeout is unavailable*' -- (cat $terr); and echo true; or echo false)
|
||||
check "a skipped autopush still committed locally" true (git -C $vroot15/agent-vault ls-files --error-unmatch projects/$tslug15/claude/memory/t2.md >/dev/null 2>&1; and echo true; or echo false)
|
||||
# Not a failure: nothing was attempted and failed, and the one thing the
|
||||
# user did not ask for -- a hang in front of an agent launch -- did not
|
||||
# happen. --push is where a caller demands a real transfer answer.
|
||||
check "a skipped autopush is not a failure" 0 "$trc"
|
||||
|
||||
echo t3 >$croot15/$tmang15/memory/t3.md
|
||||
set -g PATH $nopath
|
||||
pushd $tp15 >/dev/null
|
||||
set -l t3rc (agents-vault --push --silent 2>/dev/null; echo $status)
|
||||
popd >/dev/null
|
||||
set -g PATH $realpath15
|
||||
check "--push still pushes without timeout" 0 "$t3rc"
|
||||
check "--push landed in the remote without timeout" t3 (git -C $bare15 show $vb15:projects/$tslug15/claude/memory/t3.md 2>/dev/null)
|
||||
|
||||
set -e __fish_agent_vault_dir
|
||||
set -e __fish_agent_vault_claude_root
|
||||
set -g __fish_agent_vault_claude_home $HERMETIC_HOME/claude
|
||||
set -g __fish_agent_vault_agy_root $HERMETIC_HOME/agy
|
||||
|
||||
# ────────────── a failed vault commit is fatal, not a note ─────────────
|
||||
# _agents_repo_sync's own rejection path is covered further up, but
|
||||
# agents-vault has to *propagate* it. The function otherwise ends on a
|
||||
|
||||
Reference in New Issue
Block a user