mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-11 11:10:13 +01:00
fix(harness): tolerant JSON read-back, short read-back timeout, retry an interrupted explicit activation, symlink checks on every hook opt-out and folder cleanup
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
f276da838f
commit
7e8cb6791c
4 files changed
+61
-9
No files matched your search
@@ -125,7 +125,10 @@ gstack_harness_unregister_ledger() {
|
||||
done < "$ledger"
|
||||
while IFS=$'\t' read -r kind a b; do
|
||||
[ "$kind" = dir ] && [ -d "$b" ] && [ ! -L "$b" ] || continue
|
||||
case "$b" in "$dest"/*) _gstack_harness_owned "$b" "$dest" || continue ;; esac
|
||||
case "$b" in
|
||||
"$dest"/*) _gstack_harness_owned "$b" "$dest" || continue ;;
|
||||
*) if [ -n "$hprof" ]; then case "$b" in "$hprof"/*) _gstack_harness_below "$b" "$hprof" || continue ;; esac; fi ;;
|
||||
esac
|
||||
rmdir "$b" 2>/dev/null
|
||||
done < "$ledger"
|
||||
case "$target" in
|
||||
|
||||
@@ -541,19 +541,20 @@ function hookPlan(opts: RegisterOptions, dest: string, prev: Ledger | null): Hoo
|
||||
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>() };
|
||||
const saved = prev?.meta.hook_root && prev.meta.hook_profile ? { profile: prev.meta.hook_profile, root: prev.meta.hook_root } : null;
|
||||
if (saved && !choice.on) assertHookRoot(t.name, saved.profile, saved.root, saved.root, (code, msg) => { throw new HarnessError(code, msg); });
|
||||
if (!choice.on) return plan;
|
||||
const name = t.name;
|
||||
const off = (p: HookPlan) => { if (saved) assertHookRoot(name, saved.profile, saved.root, saved.root, (code, msg) => { throw new HarnessError(code, msg); }); return p; };
|
||||
if (!choice.on) return off(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 && (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)' };
|
||||
return off({ ...plan, on: false, note: 'off: ' + why + ' (' + choice.source + ' asked for the hook)' });
|
||||
}
|
||||
try {
|
||||
assertHookRoot(t.name, profile, root, prevOn ? prev!.meta.hook_root : undefined, (code, msg) => { throw new HarnessError(code, msg); });
|
||||
} catch (e) {
|
||||
if (plan.explicit || prevOn || !(e instanceof HarnessError)) throw e;
|
||||
return { ...plan, on: false, note: 'off: ' + e.message + ' (' + choice.source + ' asked for the hook; ' + HARNESS_CODES[e.code].fix + ')' };
|
||||
return off({ ...plan, on: false, note: 'off: ' + e.message + ' (' + choice.source + ' asked for the hook; ' + HARNESS_CODES[e.code].fix + ')' });
|
||||
}
|
||||
return { ...plan, files: hookFiles(t.name, root) };
|
||||
}
|
||||
@@ -675,6 +676,7 @@ export function register(opts: RegisterOptions): RegisterResult {
|
||||
fs.writeFileSync(file, text);
|
||||
}
|
||||
if (opts.registry && t.render === 'host') shellRegistry(opts, 'gstack_install_registry_upsert', [t.name, 'global', '-', dest, opts.checkout, opts.checkout, opts.version, '-', '-']);
|
||||
if (same && hook?.on && hook.explicit && prev.meta.hook_active !== 'no') writeLedger(lfile, { ...prev, meta: { ...prev.meta, hook_active: 'no' } });
|
||||
const hookResult = finishHook(opts, hook, released);
|
||||
const active = hookResult?.required || hookResult?.retried ? ({ active: 'yes', disabled: 'disabled' } as Record<string, string>)[hookResult.state] ?? 'no' : '';
|
||||
const saved = active ? readLedger(lfile) : null;
|
||||
|
||||
@@ -131,26 +131,31 @@ export function activateHook(t: HookTarget, profile: string, env: Env, home: str
|
||||
return '';
|
||||
}
|
||||
|
||||
const parseJson = (s: string): any => { try { return JSON.parse(s.slice(s.search(/[[{]/))); } catch { return null; } };
|
||||
function parseJson(s: string): any {
|
||||
for (const m of s.matchAll(/^[ \t]*[[{]/gm)) { try { return JSON.parse(s.slice(m.index)); } catch {} }
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Reads the hook back from the harness: active (configured for new sessions), disabled by the user, or written (with the fix). */
|
||||
const READ_TIMEOUT = 15_000;
|
||||
|
||||
export function inspectHook(t: HookTarget, profile: string, env: Env, home: string): HookResult {
|
||||
const fix = activateCommands(t, profile);
|
||||
if (!onPath(t, env)) return { state: 'written', detail: t + ' is not on PATH, so gstack could not turn the plugin on or check it', fix };
|
||||
if (t === 'hermes') {
|
||||
const r = runHarness(t, profile, ['plugins', 'list', '--json'], env, home);
|
||||
const r = runHarness(t, profile, ['plugins', 'list', '--json'], env, home, READ_TIMEOUT);
|
||||
const row = (parseJson(r.stdout) ?? []).find?.((p: any) => p?.name === HOOK_ID);
|
||||
if (row?.status === 'enabled') return { state: 'active' };
|
||||
if (row?.status === 'disabled') return { state: 'disabled', detail: 'turned off in Hermes (plugins.disabled); gstack leaves it off', fix: activateCommands(t, profile) };
|
||||
return { state: 'written', detail: row ? 'Hermes lists it as ' + row.status : 'hermes plugins list does not show ' + HOOK_ID + (r.why ? ' (' + r.why + ')' : ''), fix };
|
||||
}
|
||||
const cfg = parseJson(runHarness(t, profile, ['config', 'get', 'plugins', '--json'], env, home).stdout) ?? {};
|
||||
const cfg = parseJson(runHarness(t, profile, ['config', 'get', 'plugins', '--json'], env, home, READ_TIMEOUT).stdout) ?? {};
|
||||
const entry = cfg.entries?.[HOOK_ID] ?? {};
|
||||
if (entry.enabled === false) return { state: 'disabled', detail: 'turned off in openclaw.json (plugins.entries.' + HOOK_ID + '.enabled false); gstack leaves it off', fix };
|
||||
if (Array.isArray(cfg.deny) && cfg.deny.includes(HOOK_ID)) return { state: 'written', detail: 'plugins.deny lists ' + HOOK_ID, fix: 'remove ' + HOOK_ID + ' from plugins.deny in openclaw.json' };
|
||||
if (Array.isArray(cfg.allow) && cfg.allow.length && !cfg.allow.includes(HOOK_ID)) return { state: 'written', detail: 'plugins.allow is set and does not list ' + HOOK_ID, fix: 'OPENCLAW_STATE_DIR=' + JSON.stringify(profile) + " openclaw config set plugins.allow '" + JSON.stringify([...cfg.allow, HOOK_ID]) + "'" };
|
||||
if (entry.hooks?.allowPromptInjection === false) return { state: 'written', detail: 'plugins.entries.' + HOOK_ID + '.hooks.allowPromptInjection is false', fix };
|
||||
const r = runHarness(t, profile, ['plugins', 'inspect', HOOK_ID, '--runtime', '--json'], env, home);
|
||||
const r = runHarness(t, profile, ['plugins', 'inspect', HOOK_ID, '--runtime', '--json'], env, home, READ_TIMEOUT);
|
||||
const d = parseJson(r.stdout);
|
||||
if (d?.plugin?.status === 'loaded' && d.plugin.enabled && (d.typedHooks ?? []).some((h: any) => h?.name === 'before_prompt_build')) return { state: 'active' };
|
||||
const why = d?.plugin ? 'status ' + d.plugin.status + (d.plugin.enabled ? '' : ', not enabled') + ((d.typedHooks ?? []).length ? '' : ', before_prompt_build not registered') : 'openclaw plugins inspect failed' + (r.why ? ' (' + r.why + ')' : '');
|
||||
|
||||
@@ -24,6 +24,8 @@ d="$OPENCLAW_STATE_DIR"; c="$d/fake-config"
|
||||
printf '%s|secret=%s|home=%s|cfg=%s\n' "$*" "$ANTHROPIC_API_KEY" "$HOME" "$OPENCLAW_CONFIG_PATH" >> "$d/fake-calls"
|
||||
[ -f "$d/fake-fail" ] && { echo "config write refused" >&2; exit 3; }
|
||||
[ -f "$d/fake-kill" ] && { rm -f "$d/fake-kill"; kill -9 "$PPID"; sleep 5; }
|
||||
[ -f "$d/fake-noisy" ] && echo "[plugins] gstack-start: cannot verify where this plugin came from"
|
||||
[ -f "$d/fake-hang" ] && [ "$1" = plugins ] && exec sleep 40
|
||||
touch "$c"
|
||||
acc=false; grep -q '^plugins.entries.gstack-start.hooks.allowConversationAccess=true$' "$c" && acc=true
|
||||
en=$(grep '^plugins.entries.gstack-start.enabled=' "$c" | tail -1 | cut -d= -f2); enj=null; [ -n "$en" ] && enj=$en
|
||||
@@ -281,6 +283,34 @@ describe('start hook: OpenClaw lifecycle (fake CLI)', () => {
|
||||
expect(fs.readFileSync(off.kept[0], 'utf8')).toBe('my note\n');
|
||||
}, 300_000);
|
||||
|
||||
test('an explicit --start-hook on an unchanged registration killed during activation is retried by the next run', () => {
|
||||
const r = rig('openclaw');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
fs.writeFileSync(path.join(r.profile, 'fake-kill'), '');
|
||||
const cli = spawnSync(process.execPath, ['--no-env-file', '--config=/dev/null', path.join(ROOT, 'bin', 'gstack-harness'), 'register', 'openclaw', '--start-hook'],
|
||||
{ env: { ...r.env, PATH: r.env.PATH + path.delimiter + path.dirname(process.execPath), HOME: path.join(r.d, 'home'), GSTACK_STATE_ROOT: path.join(r.d, 'state') }, encoding: 'utf8', timeout: 240_000 });
|
||||
expect(cli.signal, cli.stderr).toBe('SIGKILL');
|
||||
const dir = path.join(r.d, 'state', 'harness');
|
||||
expect(readLedger(path.join(dir, fs.readdirSync(dir).find(f => f.endsWith('.ledger'))!))!.meta.hook_active).toBe('no');
|
||||
expect(register(r.o()).hook).toMatchObject({ state: 'active', retried: true });
|
||||
}, 300_000);
|
||||
|
||||
test('a log line in brackets before the harness JSON still reads back as active', () => {
|
||||
const r = rig('openclaw');
|
||||
fs.writeFileSync(path.join(r.profile, 'fake-noisy'), '');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
expect(inspectHook('openclaw', r.profile, r.env, path.join(r.d, 'home')).state).toBe('active');
|
||||
}, 120_000);
|
||||
|
||||
test('a hung harness read-back gives up after a short timeout', () => {
|
||||
const r = rig('openclaw');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
fs.writeFileSync(path.join(r.profile, 'fake-hang'), '');
|
||||
const t0 = Date.now();
|
||||
expect(inspectHook('openclaw', r.profile, r.env, path.join(r.d, 'home')).state).toBe('written');
|
||||
expect(Date.now() - t0).toBeLessThan(35_000);
|
||||
}, 120_000);
|
||||
|
||||
test('without the harness on PATH the hook is written, not active, and names the commands to run', () => {
|
||||
const r = rig('openclaw', { noCli: true });
|
||||
const res = register(r.o({ startHook: true }));
|
||||
@@ -424,6 +454,7 @@ describe('start hook: teardown keeps what gstack did not write (fake CLI)', () =
|
||||
fs.renameSync(ext, moved);
|
||||
fs.symlinkSync(moved, ext);
|
||||
expect(() => register(r.o({ startHook: false }))).toThrow(expect.objectContaining({ code: 'HARNESS_UNSAFE_DESTINATION' }));
|
||||
expect(() => register({ ...r.o(), env: { ...r.env, GSTACK_HARNESS_START_HOOK: '1' } })).toThrow(expect.objectContaining({ code: 'HARNESS_UNSAFE_DESTINATION' }));
|
||||
expect(fs.existsSync(path.join(moved, HOOK_ID, 'index.js'))).toBe(true);
|
||||
fs.unlinkSync(ext);
|
||||
fs.renameSync(moved, ext);
|
||||
@@ -433,6 +464,17 @@ describe('start hook: teardown keeps what gstack did not write (fake CLI)', () =
|
||||
expect(readLedger(off.ledger)!.meta.hook_root).toBeUndefined();
|
||||
}, 300_000);
|
||||
|
||||
test('unregister leaves an empty plugin folder alone when the folder above it was swapped for a symlink', () => {
|
||||
const r = rig('openclaw');
|
||||
expect(register(r.o({ startHook: true })).hook?.state).toBe('active');
|
||||
const ext = path.join(r.profile, 'extensions'), elsewhere = path.join(r.d, 'elsewhere');
|
||||
fs.renameSync(ext, path.join(r.d, 'old'));
|
||||
fs.mkdirSync(path.join(elsewhere, HOOK_ID), { recursive: true });
|
||||
fs.symlinkSync(elsewhere, ext);
|
||||
unregister(r.o().stateRoot, ROOT, 'openclaw', r.env);
|
||||
expect(fs.existsSync(path.join(elsewhere, HOOK_ID))).toBe(true);
|
||||
}, 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');
|
||||
|
||||
Reference in new issue
Block a user