mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
fix(harness): teardown keeps kept copies and refuses symlinked plugin paths; a failed opt-out can rerun; a saved hook survives OPENCLAW_CONFIG_PATH
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
e265acabcd
commit
2b0b503c60
4 files changed
+81
-16
No files matched your search
@@ -31,6 +31,14 @@ _gstack_harness_sha() {
|
||||
else shasum -a 256 ${1:+"$1"} | cut -d' ' -f1; fi
|
||||
}
|
||||
|
||||
# True when $1 is below $2 and nothing between them is a symlink ($2 itself may be one).
|
||||
_gstack_harness_below() {
|
||||
local p="$1" top="$2"
|
||||
[ -n "$top" ] || return 1
|
||||
case "$p" in "$top"/*) ;; *) return 1 ;; esac
|
||||
while [ "$p" != "$top" ]; do [ -L "$p" ] && return 1; p="$(dirname "$p")"; done
|
||||
}
|
||||
|
||||
# True when $1 is below $2 and no path from $2 down to $1 is a symlink.
|
||||
_gstack_harness_owned() {
|
||||
local p="$1" dest="$2"
|
||||
@@ -63,20 +71,25 @@ _gstack_harness_cut_block() {
|
||||
}
|
||||
|
||||
# Turns a registration's start hook off in its harness, from the ledger: OpenClaw drops
|
||||
# plugins.entries.gstack-start; Hermes removes the plugin and its config entries when every
|
||||
# plugin file is still the one gstack wrote, else only disables it so an edited file
|
||||
# survives. The harness runs with PATH, HOME and the profile only. Without the harness on
|
||||
# plugins.entries.gstack-start; Hermes removes the plugin and its config entries when its
|
||||
# folder holds exactly the files gstack wrote, unedited, and is reached without a symlink
|
||||
# below the profile; otherwise it only disables it so an edit or a kept copy survives. The harness runs with PATH, HOME and the profile only. Without the harness on
|
||||
# PATH it prints the command to run instead.
|
||||
gstack_harness_hook_release() {
|
||||
local ledger="$1" kind a b target="" on="" profile="" root="" edited=0 to="" sub cmd
|
||||
local ledger="$1" kind a b target="" on="" profile="" root="" edited=0 to="" sub cmd n=0 safe=1 total
|
||||
[ -f "$ledger" ] || return 0
|
||||
while IFS=$'\t' read -r kind a b; do
|
||||
case "$kind" in
|
||||
meta) case "$a" in target) target="$b" ;; start_hook) on="$b" ;; hook_profile) profile="$b" ;; hook_root) root="$b" ;; esac ;;
|
||||
file) if [ -n "$root" ]; then case "$b" in "$root"/*) [ -f "$b" ] && [ "$(_gstack_harness_sha "$b")" != "$a" ] && edited=1 ;; esac; fi ;;
|
||||
file) if [ -n "$root" ]; then case "$b" in "$root"/*) if [ -f "$b" ]; then n=$((n + 1)); [ "$(_gstack_harness_sha "$b")" != "$a" ] && edited=1; fi ;; esac; fi ;;
|
||||
esac
|
||||
done < "$ledger"
|
||||
{ [ "$on" = on ] && [ -n "$profile" ] && [ -n "$root" ]; } || return 0
|
||||
if [ -d "$root" ]; then
|
||||
_gstack_harness_below "$root" "$profile" || { safe=0; edited=1; }
|
||||
total="$(find "$root" -type f ! -path "$root/__pycache__/*" 2>/dev/null | wc -l | tr -d ' ')"
|
||||
[ "${total:-0}" -gt "$n" ] && edited=1
|
||||
fi
|
||||
command -v timeout >/dev/null 2>&1 && to="timeout 60"
|
||||
case "$target" in
|
||||
openclaw) cmd="config unset plugins.entries.gstack-start"
|
||||
@@ -86,7 +99,7 @@ gstack_harness_hook_release() {
|
||||
if command -v hermes >/dev/null 2>&1 && env -i PATH="$PATH" HOME="$HOME" HERMES_HOME="$profile" $to hermes plugins "$sub" gstack-start </dev/null >/dev/null 2>&1; then :
|
||||
else echo " start hook: turn it off in Hermes yourself: HERMES_HOME=\"$profile\" hermes plugins disable gstack-start"; fi ;;
|
||||
esac
|
||||
if [ -d "$root/__pycache__" ] && _gstack_harness_owned "$root/__pycache__" "$root"; then
|
||||
if [ "$safe" = 1 ] && [ -d "$root/__pycache__" ] && _gstack_harness_owned "$root/__pycache__" "$root"; then
|
||||
rm -f "$root"/__pycache__/*.pyc
|
||||
rmdir "$root/__pycache__" 2>/dev/null
|
||||
fi
|
||||
@@ -94,16 +107,16 @@ gstack_harness_hook_release() {
|
||||
}
|
||||
|
||||
gstack_harness_unregister_ledger() {
|
||||
local ledger="$1" stamp kind a b target="" dest="" pack="" pad="" padpath="" hroot=""
|
||||
local ledger="$1" stamp kind a b target="" dest="" pack="" pad="" padpath="" hroot="" hprof=""
|
||||
: "${GSTACK_STATE_ROOT:?gstack-harness-ledger: GSTACK_STATE_ROOT is not set}"
|
||||
[ -f "$ledger" ] || return 0
|
||||
stamp="$(date -u +%Y-%m-%dT%H-%M-%SZ)"
|
||||
gstack_harness_hook_release "$ledger"
|
||||
while IFS=$'\t' read -r kind a b; do
|
||||
case "$kind" in
|
||||
meta) case "$a" in target) target="$b" ;; dest) dest="$b" ;; qm_pack) pack="$b" ;; hook_root) hroot="$b" ;; esac ;;
|
||||
meta) case "$a" in target) target="$b" ;; dest) dest="$b" ;; qm_pack) pack="$b" ;; hook_root) hroot="$b" ;; hook_profile) hprof="$b" ;; esac ;;
|
||||
file)
|
||||
{ [ -f "$b" ] && { _gstack_harness_owned "$b" "$dest" || _gstack_harness_owned "$b" "$hroot"; }; } || continue
|
||||
{ [ -f "$b" ] && { _gstack_harness_owned "$b" "$dest" || { _gstack_harness_owned "$b" "$hroot" && _gstack_harness_below "$hroot" "$hprof"; }; }; } || continue
|
||||
if [ "$(_gstack_harness_sha "$b")" = "$a" ]; then rm -f "$b"
|
||||
else mv "$b" "$b.edited-$stamp" && echo " kept your edit: $b.edited-$stamp"; fi ;;
|
||||
blockpad) pad="$a"; padpath="$b" ;;
|
||||
|
||||
+2
-2
@@ -83,11 +83,11 @@ It is off unless you turn it on, because turning it on changes the harness's own
|
||||
| Settings it changes, through the harness's own CLI | `openclaw config set plugins.entries.gstack-start.hooks.allowConversationAccess true` and `...enabled true` in `openclaw.json`. OpenClaw runs `before_prompt_build` from a plugin it did not bundle only with that grant, which also lets the plugin's hooks see conversation content (the plugin reads only its own note.txt) | `hermes plugins enable gstack-start` adds it to `plugins.enabled` in `config.yaml` (and creates `config.yaml` when there is none) |
|
||||
| Check | `openclaw plugins inspect gstack-start --runtime --json` lists `before_prompt_build` under `typedHooks` | `hermes plugins list` shows `gstack-start` enabled |
|
||||
|
||||
**Turn it on.** `gstack-harness register openclaw --start-hook` (or `hermes`), or `gstack-config set harness_start_hook on` followed by `./setup`, or `GSTACK_HARNESS_START_HOOK=1 ./setup --host openclaw`. The first of these that is set decides: the `--start-hook`/`--no-start-hook` flag, then `GSTACK_HARNESS_START_HOOK` (`1`/`on` or `0`/`off`), then `harness_start_hook`, then the choice saved in the registration's ledger. Whatever decides, the choice is saved, so refresh and upgrades keep it. The hook goes only into the harness profile's own plugin folder; a registration with a custom `--skills-dir`, or OpenClaw run with `OPENCLAW_CONFIG_PATH` pointing at another config, gets no hook (`HARNESS_HOOK_UNSUPPORTED` when you ask for one).
|
||||
**Turn it on.** `gstack-harness register openclaw --start-hook` (or `hermes`), or `gstack-config set harness_start_hook on` followed by `./setup`, or `GSTACK_HARNESS_START_HOOK=1 ./setup --host openclaw`. The first of these that is set decides: the `--start-hook`/`--no-start-hook` flag, then `GSTACK_HARNESS_START_HOOK` (`1`/`on` or `0`/`off`), then `harness_start_hook`, then the choice saved in the registration's ledger. Whatever decides, the choice is saved, so refresh and upgrades keep it. The hook goes only into the harness profile's own plugin folder; a registration with a custom `--skills-dir`, or OpenClaw run with `OPENCLAW_CONFIG_PATH` pointing at another config, gets no hook (`HARNESS_HOOK_UNSUPPORTED` when you ask for one). A hook you already turned on stays on when a later run happens to have `OPENCLAW_CONFIG_PATH` set.
|
||||
|
||||
**Check it.** The receipt and `gstack-harness status` show `start hook: active` (configured for new sessions; a running OpenClaw gateway or Hermes gateway picks it up when it reloads or restarts), `written` (files in place but not active, with the reason and the command that fixes it), `disabled` (you turned it off in the harness) or `off`. When the run that turns the hook on (or `--start-hook`) leaves it inactive, `gstack-harness register` and `./setup --host` exit 4 after registering the skills (`HARNESS_HOOK_INACTIVE`). Later refreshes and upgrades try once more each time, without failing, until the hook is active or you disable the plugin in the harness.
|
||||
|
||||
**Turn it off.** `gstack-harness register openclaw --no-start-hook` (or `hermes`), or `gstack-config set harness_start_hook off` followed by `./setup`. Either turns the plugin off through the harness's CLI and removes its folder. You can also switch it off in the harness (`openclaw config set plugins.entries.gstack-start.enabled false`, `hermes plugins disable gstack-start`); gstack then leaves it off and never turns it back on unless you pass `--start-hook` again. `gstack-harness unregister` and `gstack-uninstall` remove it with the rest, and print the harness command to run when the harness CLI is not on `PATH`.
|
||||
**Turn it off.** `gstack-harness register openclaw --no-start-hook` (or `hermes`), or `gstack-config set harness_start_hook off` followed by `./setup`. Either turns the plugin off through the harness's CLI and removes its folder. You can also switch it off in the harness (`openclaw config set plugins.entries.gstack-start.enabled false`, `hermes plugins disable gstack-start`); gstack then leaves it off and never turns it back on unless you pass `--start-hook` again. `gstack-harness unregister` and `gstack-uninstall` remove it with the rest (Hermes only disables it when its folder holds an edited file or a kept `.edited-*` copy, so your edit survives), and print the harness command to run when the harness CLI is not on `PATH`.
|
||||
|
||||
**What it costs.** The note is about 50 tokens and lands on every user turn, and each copy stays in the session's history, so a long session pays more (mostly cache reads). HARN-18's panel measures the added cost per trial. OpenClaw also logs that it cannot verify where the plugin came from, because gstack writes it instead of installing it from npm or ClawHub.
|
||||
|
||||
|
||||
@@ -540,10 +540,11 @@ function hookPlan(opts: RegisterOptions, dest: string, prev: Ledger | null): Hoo
|
||||
const profile = prevOn ? prev!.meta.hook_profile : hookProfile(t.name, opts.env, opts.home);
|
||||
const root = hookRoot(t.name, profile);
|
||||
const plan = { on: choice.on, prevOn, explicit: choice.source === 'flag', retry: prevOn && prev!.meta.hook_active === 'no', profile, root, files: new Map<string, Buffer>() };
|
||||
if (prevOn && !choice.on) assertHookRoot(t.name, profile, root, root, (code, msg) => { throw new HarnessError(code, msg); });
|
||||
if (!choice.on) return plan;
|
||||
const why = dest !== profileSkills(t.name, profile) ? 'this registration uses another skills folder (' + dest + ')'
|
||||
: t.name === 'openclaw' && opts.env.OPENCLAW_CONFIG_PATH ? 'OPENCLAW_CONFIG_PATH points OpenClaw at a config outside that profile' : '';
|
||||
if (why) {
|
||||
if (why && (plan.explicit || !prevOn)) {
|
||||
if (plan.explicit) throw new HarnessError('HARNESS_HOOK_UNSUPPORTED', 'the start hook goes into the ' + t.displayName + ' profile ' + profile + ', and ' + why);
|
||||
return { ...plan, on: false, note: 'off: ' + why + ' (' + choice.source + ' asked for the hook)' };
|
||||
}
|
||||
@@ -641,12 +642,12 @@ export function register(opts: RegisterOptions): RegisterResult {
|
||||
if (!same) {
|
||||
const dirs = new Set(prev?.dirs ?? []);
|
||||
for (const p of files.keys()) for (let d = path.dirname(p); !fs.existsSync(d) && path.dirname(d) !== d; d = path.dirname(d)) dirs.add(d);
|
||||
const record = (recorded: typeof want) => writeLedger(lfile, {
|
||||
const record = (recorded: typeof want, final: boolean) => writeLedger(lfile, {
|
||||
meta: { id, target: t.name, dest, mode, checkout: opts.checkout, version: opts.version, names: visible.join(','), ...(router ? { router } : {}), ...(context ? { context } : {}),
|
||||
...(hook ? { start_hook: hook.on ? 'on' : 'off' } : {}), ...(hook?.on ? { hook_profile: hook.profile, hook_root: hook.root, ...(!hook.prevOn || hook.explicit ? { hook_active: 'no' } : prev?.meta.hook_active ? { hook_active: prev.meta.hook_active } : {}) } : {}) },
|
||||
...(hook ? { start_hook: hook.on ? 'on' : 'off' } : {}), ...(hook?.on ? { hook_profile: hook.profile, hook_root: hook.root, ...(!hook.prevOn || hook.explicit ? { hook_active: 'no' } : prev?.meta.hook_active ? { hook_active: prev.meta.hook_active } : {}) } : !final && prev?.meta.hook_root ? { hook_profile: prev.meta.hook_profile, hook_root: prev.meta.hook_root } : {}) },
|
||||
files: recorded, dirs: [...dirs].sort((a, b) => b.length - a.length), blocks,
|
||||
});
|
||||
record([...want, ...(prev?.files ?? []).filter(f => !files.has(f.path))]);
|
||||
record([...want, ...(prev?.files ?? []).filter(f => !files.has(f.path))], false);
|
||||
for (const [p, sha] of prevFiles) {
|
||||
if (files.has(p)) continue;
|
||||
const disk = onDiskSha(p);
|
||||
@@ -665,7 +666,7 @@ export function register(opts: RegisterOptions): RegisterResult {
|
||||
fs.renameSync(p + '.gstack-new', p);
|
||||
written++;
|
||||
}
|
||||
record(want);
|
||||
record(want, true);
|
||||
}
|
||||
for (const [file, text] of blockWrites) {
|
||||
if (text === (fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : null)) continue;
|
||||
|
||||
@@ -386,6 +386,57 @@ describe('start hook: Hermes lifecycle and the saved choice (fake CLI)', () => {
|
||||
}, 300_000);
|
||||
});
|
||||
|
||||
describe('start hook: teardown keeps what gstack did not write (fake CLI)', () => {
|
||||
test('a kept copy of an edited Hermes plugin file survives unregister: Hermes only disables the plugin', () => {
|
||||
const r = rig('hermes');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
fs.writeFileSync(path.join(r.root, 'note.txt'), 'my note\n');
|
||||
const again = register(r.o());
|
||||
expect(again.kept).toHaveLength(1);
|
||||
unregister(r.o().stateRoot, ROOT, 'hermes', r.env);
|
||||
expect(r.calls().at(-1)!.split('|')[0]).toBe('plugins disable gstack-start');
|
||||
expect(fs.readFileSync(again.kept[0], 'utf8')).toBe('my note\n');
|
||||
}, 300_000);
|
||||
|
||||
test('a plugins folder swapped for a symlink after registration: off refuses, unregister deletes nothing through it', () => {
|
||||
const r = rig('hermes');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
const elsewhere = path.join(r.d, 'elsewhere');
|
||||
fs.renameSync(path.join(r.profile, 'plugins'), elsewhere);
|
||||
fs.symlinkSync(elsewhere, path.join(r.profile, 'plugins'));
|
||||
fs.writeFileSync(path.join(elsewhere, HOOK_ID, 'important.txt'), 'keep');
|
||||
expect(() => register(r.o({ startHook: false }))).toThrow(expect.objectContaining({ code: 'HARNESS_UNSAFE_DESTINATION' }));
|
||||
unregister(r.o().stateRoot, ROOT, 'hermes', r.env);
|
||||
expect(r.calls().some(l => l.startsWith('plugins remove'))).toBe(false);
|
||||
expect(fs.readdirSync(path.join(elsewhere, HOOK_ID)).sort()).toEqual(['__init__.py', 'important.txt', 'note.txt', 'plugin.yaml']);
|
||||
}, 300_000);
|
||||
|
||||
test.skipIf(process.platform === 'win32' || process.getuid?.() === 0)('an opt-out that fails to delete the plugin files can be run again', () => {
|
||||
const r = rig('openclaw');
|
||||
const on = register(r.o({ startHook: true }));
|
||||
expect(on.hook?.state).toBe('active');
|
||||
fs.chmodSync(r.root, 0o555);
|
||||
try {
|
||||
expect(() => register(r.o({ startHook: false }))).toThrow(/EACCES/);
|
||||
expect(readLedger(on.ledger)!.meta.hook_root).toBe(r.root);
|
||||
} finally { fs.chmodSync(r.root, 0o755); }
|
||||
const off = register(r.o({ startHook: false }));
|
||||
expect(off.hook?.state).toBe('off');
|
||||
expect(fs.existsSync(path.join(r.profile, 'extensions'))).toBe(false);
|
||||
expect(readLedger(off.ledger)!.meta.hook_root).toBeUndefined();
|
||||
}, 300_000);
|
||||
|
||||
test('a saved opt-in survives one run from a shell that sets OPENCLAW_CONFIG_PATH', () => {
|
||||
const r = rig('openclaw');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
const res = register({ ...r.o(), env: { ...r.env, OPENCLAW_CONFIG_PATH: '/elsewhere/openclaw.json' } });
|
||||
expect(res.hook?.state).not.toBe('off');
|
||||
expect(r.calls().some(l => l.startsWith('config unset'))).toBe(false);
|
||||
expect(fs.existsSync(path.join(r.root, 'index.js'))).toBe(true);
|
||||
expect(readLedger(res.ledger)!.meta.start_hook).toBe('on');
|
||||
}, 300_000);
|
||||
});
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('start hook through ./setup --host', () => {
|
||||
test('exits 4 with the skills registered when the hook does not turn on, and the next run retries it', () => {
|
||||
const f = makeFixture();
|
||||
|
||||
Reference in new issue
Block a user