diff --git a/lib/harness-register.ts b/lib/harness-register.ts index 8f9b8809c..03c91684b 100644 --- a/lib/harness-register.ts +++ b/lib/harness-register.ts @@ -643,7 +643,7 @@ export function register(opts: RegisterOptions): RegisterResult { 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, { 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, ...(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 } : {}) } : {}) }, files: recorded, dirs: [...dirs].sort((a, b) => b.length - a.length), blocks, }); record([...want, ...(prev?.files ?? []).filter(f => !files.has(f.path))]); diff --git a/lib/harness-start-hook.ts b/lib/harness-start-hook.ts index 28e6a6505..a106052b7 100644 --- a/lib/harness-start-hook.ts +++ b/lib/harness-start-hook.ts @@ -83,7 +83,10 @@ const SPECS = { } as const; export const isHookTarget = (t: string): t is HookTarget => t === 'openclaw' || t === 'hermes'; -export const hookProfile = (t: HookTarget, env: Env, home: string) => env[SPECS[t].profileVar] || path.join(home, SPECS[t].home); +export function hookProfile(t: HookTarget, env: Env, home: string): string { + const v = env[SPECS[t].profileVar]; + return v && path.isAbsolute(v) ? path.resolve(v) : v || path.join(home, SPECS[t].home); +} export const hookRoot = (t: HookTarget, profile: string) => path.join(profile, SPECS[t].sub, HOOK_ID); /** The skills folder a registration must use for the hook to go into that profile. */ export const profileSkills = (t: HookTarget, profile: string) => path.join(profile, ...SPECS[t].skills); @@ -154,13 +157,18 @@ export function inspectHook(t: HookTarget, profile: string, env: Env, home: stri return { state: 'written', detail: why, fix }; } +/** True when a plugin folder holds nothing but the .edited- copies gstack kept when it turned the hook off. */ +const onlyKeptEdits = (root: string) => fs.readdirSync(root, { recursive: true, withFileTypes: true }) + .every(e => e.isDirectory() || (e.isFile() && /\.edited-[0-9T-]+Z$/.test(e.name))); + /** Checks the plugin folder before anything is written: under the profile, no symlink from the profile down, and not someone else's. */ export function assertHookRoot(t: HookTarget, profile: string, root: string, recordedRoot: string | undefined, fail: (code: 'HARNESS_UNSAFE_DESTINATION' | 'HARNESS_HOOK_OWNED_ELSEWHERE', msg: string) => never): void { if (!path.isAbsolute(profile) || /[\t\n\r]/.test(profile)) fail('HARNESS_UNSAFE_DESTINATION', 'the ' + t + ' profile ' + profile + ' is not a plain absolute path'); - for (let d = root; d !== profile && d !== path.dirname(d); d = path.dirname(d)) { + const top = path.resolve(profile); + for (let d = root; d !== top && d !== path.dirname(d); d = path.dirname(d)) { let st: fs.Stats; try { st = fs.lstatSync(d); } catch { continue; } if (st.isSymbolicLink()) fail('HARNESS_UNSAFE_DESTINATION', d + ' is a symlink'); } - if (fs.existsSync(root) && recordedRoot !== root) fail('HARNESS_HOOK_OWNED_ELSEWHERE', 'a ' + HOOK_ID + ' plugin already exists at ' + root + ' and this registration did not write it'); + if (fs.existsSync(root) && recordedRoot !== root && !onlyKeptEdits(root)) fail('HARNESS_HOOK_OWNED_ELSEWHERE', 'a ' + HOOK_ID + ' plugin already exists at ' + root + ' and this registration did not write it'); } diff --git a/test/harness-start-hook.test.ts b/test/harness-start-hook.test.ts index e8c76abfc..180587290 100644 --- a/test/harness-start-hook.test.ts +++ b/test/harness-start-hook.test.ts @@ -23,6 +23,7 @@ const FAKE_OPENCLAW = String.raw`#!/bin/sh 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; } 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 @@ -167,6 +168,16 @@ describe('start hook: who decides', () => { expect(fs.readdirSync(path.join(s.d, 'elsewhere'))).toEqual([]); expect(r.calls().concat(s.calls())).toEqual([]); }); + + test('a profile path with a trailing slash under a symlinked parent still takes the hook and is saved without the slash', () => { + const r = rig('hermes'); + const link = path.join(tmp(), 'ln'); + fs.symlinkSync(r.d, link); + const res = register({ ...r.o({ startHook: true }), env: { ...r.env, HERMES_HOME: path.join(link, 'he') + '/' } }); + expect(res.hook?.state).toBe('active'); + expect(readLedger(res.ledger)!.meta.hook_profile).toBe(path.join(link, 'he')); + expect(fs.existsSync(path.join(r.root, 'plugin.yaml'))).toBe(true); + }, 120_000); }); describe('start hook: OpenClaw lifecycle (fake CLI)', () => { @@ -232,6 +243,33 @@ describe('start hook: OpenClaw lifecycle (fake CLI)', () => { expect(fs.existsSync(path.join(s.profile, 'skills', 'gstack', 'SKILL.md'))).toBe(true); }, 300_000); + test('a run killed during activation leaves the hook marked not active, so the next plain register activates it', () => { + const r = rig('openclaw'); + 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'], + { env: { ...r.env, GSTACK_HARNESS_START_HOOK: '1', 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'); + const lfile = fs.readdirSync(dir).find(f => f.endsWith('.ledger')); + expect(readLedger(path.join(dir, lfile!))!.meta).toMatchObject({ start_hook: 'on', hook_active: 'no' }); + const next = register(r.o()); + expect(next.hook).toMatchObject({ state: 'active', retried: true }); + expect(readLedger(next.ledger)!.meta.hook_active).toBe('yes'); + }, 300_000); + + test('off with an edited plugin file keeps the edit, and the same registration can turn the hook back on', () => { + const r = rig('openclaw'); + expect(register(r.o({ startHook: true })).hook?.state).toBe('active'); + fs.writeFileSync(path.join(r.root, 'note.txt'), 'my note\n'); + const off = register(r.o({ startHook: false })); + expect(off.kept).toHaveLength(1); + expect(off.kept[0]).toMatch(/note\.txt\.edited-/); + const on = register(r.o({ startHook: true })); + expect(on.hook?.state).toBe('active'); + expect(fs.readFileSync(path.join(r.root, 'note.txt'), 'utf8')).toBe(START_NOTE + '\n'); + expect(fs.readFileSync(off.kept[0], 'utf8')).toBe('my note\n'); + }, 300_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 }));