From 2ec30464ff58c1b5db2f39da5a3494ebcfcac595 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 06:18:22 +0000 Subject: [PATCH] fix(harness): an env or config start-hook opt-in with an unsafe plugin folder turns the hook off instead of failing refresh (GST-718) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-harness | 4 ++-- lib/harness-register.ts | 7 ++++++- test/harness-start-hook.test.ts | 7 ++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/bin/gstack-harness b/bin/gstack-harness index c8f4699bd..a80024b9f 100755 --- a/bin/gstack-harness +++ b/bin/gstack-harness @@ -134,7 +134,7 @@ async function main(): Promise { } case 'refresh': { let code = 0; - for (const r of refresh(base)) { if (r.status !== 'unchanged' || r.hook?.required || r.hook?.retried) receipt(r); code ||= hookFailed(r); } + for (const r of refresh(base)) { if (r.status !== 'unchanged' || r.hook?.required || r.hook?.retried || r.hook?.detail) receipt(r); code ||= hookFailed(r); } return code; } case 'auto': { @@ -145,7 +145,7 @@ async function main(): Promise { try { const r = register({ ...base, target: TARGETS[n], registry: true }); done.add(r.id); receipt(r); hookFailed(r); } catch (e) { if (e instanceof HarnessError) { console.error(e.report()); failed = 1; } else throw e; } } - try { for (const r of refresh(base, done)) { if (r.status !== 'unchanged' || r.hook?.required || r.hook?.retried) receipt(r); hookFailed(r); } } + try { for (const r of refresh(base, done)) { if (r.status !== 'unchanged' || r.hook?.required || r.hook?.retried || r.hook?.detail) receipt(r); hookFailed(r); } } catch (e) { if (e instanceof HarnessError) { console.error(e.report()); failed = 1; } else throw e; } for (const n of d.present.filter(n => !d.inside.includes(n) && TARGETS[n].mechanism === 'skill-folder' && !listLedgers(stateRoot).some(l => l.ledger.meta.target === n))) { console.log('gstack-harness: ' + TARGETS[n].displayName + ' is installed here; to give it gstack skills: ' + self + ' register ' + n); diff --git a/lib/harness-register.ts b/lib/harness-register.ts index 67976bed7..8f9b8809c 100644 --- a/lib/harness-register.ts +++ b/lib/harness-register.ts @@ -547,7 +547,12 @@ function hookPlan(opts: RegisterOptions, dest: string, prev: Ledger | null): Hoo 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)' }; } - assertHookRoot(t.name, profile, root, prevOn ? prev!.meta.hook_root : undefined, (code, msg) => { throw new HarnessError(code, msg); }); + 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 { ...plan, files: hookFiles(t.name, root) }; } diff --git a/test/harness-start-hook.test.ts b/test/harness-start-hook.test.ts index 9bf500a64..e8c76abfc 100644 --- a/test/harness-start-hook.test.ts +++ b/test/harness-start-hook.test.ts @@ -149,7 +149,7 @@ describe('start hook: who decides', () => { expect(fs.existsSync(path.join(s.d, 'custom'))).toBe(false); }); - test('a plugin folder gstack did not write, or a symlink on the way to it, stops the hook untouched', () => { + test('a plugin folder gstack did not write, or a symlink on the way to it, stops an explicit --start-hook with nothing written, and turns an env or config opt-in into hook off', () => { const r = rig('openclaw'); fs.mkdirSync(r.root, { recursive: true }); fs.writeFileSync(path.join(r.root, 'index.js'), 'mine'); @@ -160,6 +160,11 @@ describe('start hook: who decides', () => { fs.symlinkSync(path.join(s.d, 'elsewhere'), path.join(s.profile, 'plugins')); expect(() => register(s.o({ startHook: true }))).toThrow(expect.objectContaining({ code: 'HARNESS_UNSAFE_DESTINATION' })); expect(fs.readdirSync(path.join(s.d, 'elsewhere'))).toEqual([]); + const viaEnv = register({ ...s.o(), env: { ...s.env, GSTACK_HARNESS_START_HOOK: '1' } }); + expect(viaEnv.hook?.state).toBe('off'); + expect(viaEnv.hook?.detail).toContain('is a symlink (env asked for the hook'); + expect(fs.existsSync(path.join(s.profile, 'skills', 'gstack'))).toBe(true); + expect(fs.readdirSync(path.join(s.d, 'elsewhere'))).toEqual([]); expect(r.calls().concat(s.calls())).toEqual([]); }); });