From b1511ef5a95ffb4a03bae6cd8d28744e026c2e83 Mon Sep 17 00:00:00 2001 From: snowingfox <1503401882@qq.com> Date: Tue, 11 Aug 2026 13:20:17 +0000 Subject: [PATCH 1/2] fix(agent-core-v2): rebuild hook index on config change so late [[hooks]] fire Fixes #2779 --- .changeset/hooks-reload-on-config-change.md | 5 ++++ .../externalHooksRunnerService.ts | 14 +++++++++++ .../test/agent/externalHooks/runner-stub.ts | 3 +++ .../externalHooksRunner.test.ts | 24 +++++++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 .changeset/hooks-reload-on-config-change.md diff --git a/.changeset/hooks-reload-on-config-change.md b/.changeset/hooks-reload-on-config-change.md new file mode 100644 index 0000000000..043d3ef490 --- /dev/null +++ b/.changeset/hooks-reload-on-config-change.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Rebuild the external-hook index when the config changes so `[[hooks]]` that arrive after engine construction (e.g. the interactive TUI loading config.toml) fire instead of silently never running. diff --git a/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts b/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts index 6bacea0fde..e41fe41e09 100644 --- a/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts +++ b/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts @@ -56,6 +56,20 @@ export class ExternalHooksRunnerService extends Disposable implements IExternalH void this.reloadSafe(); }), ); + // Rebuild the hook index when the config changes, not just on plugin + // reload. The user config file (`config.toml`) can be (re)loaded into the + // layered config service after this runner's initial `loadSafe()` — in the + // interactive TUI in particular, `[[hooks]]` may arrive late. Without this + // subscription the index would stay empty forever and every hook would + // silently never fire (#2779). The member check keeps the subscription a + // no-op against partial `IConfigService` stubs in unit tests. + if (this.config.onDidChangeConfiguration !== undefined) { + this._register( + this.config.onDidChangeConfiguration(() => { + void this.reloadSafe(); + }), + ); + } } get summary(): Record { diff --git a/packages/agent-core-v2/test/agent/externalHooks/runner-stub.ts b/packages/agent-core-v2/test/agent/externalHooks/runner-stub.ts index f2052b52b0..d86c063338 100644 --- a/packages/agent-core-v2/test/agent/externalHooks/runner-stub.ts +++ b/packages/agent-core-v2/test/agent/externalHooks/runner-stub.ts @@ -31,6 +31,8 @@ export function makeHookRunner( reason: string | undefined, durationMs: number, ) => void; + /** Emit when the backing config gains/loses hook sections (mirrors `IConfigService.onDidChangeConfiguration`). */ + onDidChangeConfiguration?: Event; } = {}, ): ExternalHooksRunnerService { return new ExternalHooksRunnerService( @@ -38,6 +40,7 @@ export function makeHookRunner( _serviceBrand: undefined, ready: Promise.resolve(), get: (section: string) => (section === HOOKS_SECTION ? hooks : undefined), + onDidChangeConfiguration: options.onDidChangeConfiguration ?? Event.None, } as unknown as IConfigService, { _serviceBrand: undefined, diff --git a/packages/agent-core-v2/test/app/externalHooksRunner/externalHooksRunner.test.ts b/packages/agent-core-v2/test/app/externalHooksRunner/externalHooksRunner.test.ts index 86809fa587..b4d53e7d7f 100644 --- a/packages/agent-core-v2/test/app/externalHooksRunner/externalHooksRunner.test.ts +++ b/packages/agent-core-v2/test/app/externalHooksRunner/externalHooksRunner.test.ts @@ -1,6 +1,8 @@ import { realpathSync } from 'node:fs'; import { tmpdir } from 'node:os'; +import { Emitter } from '#/_base/event'; +import type { HookDef } from '#/agent/externalHooks/types'; import type { ContentPart } from '#/kosong/contract/message'; import { describe, expect, it, vi } from 'vitest'; @@ -341,4 +343,26 @@ describe('ExternalHooksRunnerService', () => { expect(runner.hasHooksFor('SessionHeartbeat')).toBe(true); expect(runner.hasHooksFor('Stop')).toBe(false); }); + + it('rebuilds the hook index when hooks arrive through a config change', async () => { + const hooks: HookDef[] = []; + const emitter = new Emitter(); + const runner = makeHookRunner(hooks, { onDidChangeConfiguration: emitter.event }); + + await runner.ready; + expect(runner.hasHooksFor('SessionStart')).toBe(false); + + // The config gains a [[hooks]] section after the runner was constructed + // (the interactive TUI can load config.toml after app-scope construction). + // The runner must re-read the config instead of keeping the empty index + // forever, or the hook would silently never fire. + hooks.push({ event: 'SessionStart', command: nodeCommand('process.exit(0);'), timeout: 5 }); + emitter.fire(); + + await vi.waitFor(() => { + expect(runner.hasHooksFor('SessionStart')).toBe(true); + }); + const results = await runner.trigger('SessionStart'); + expect(results).toHaveLength(1); + }); }); From 9c29837316cf781d08a090cb1d6ef17134932f3c Mon Sep 17 00:00:00 2001 From: snowingfox <1503401882@qq.com> Date: Tue, 11 Aug 2026 22:52:54 +0800 Subject: [PATCH 2/2] fix(agent-core-v2): serialize hook-index rebuilds so late config hooks are not missed Address codex P1 review comments on #2822: fold the config-change rationale into the file header (AGENTS.md header-only comment convention) and serialize reloads into a promise chain that triggerInner awaits, so a hook trigger that lands right after a config change reads the rebuilt index instead of the stale empty one. --- .../externalHooksRunnerService.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts b/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts index e41fe41e09..71e7fce63e 100644 --- a/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts +++ b/packages/agent-core-v2/src/app/externalHooksRunner/externalHooksRunnerService.ts @@ -3,7 +3,10 @@ * * Owns the configured-hook lifecycle: builds the event→hooks index from * `IConfigService` (`[[hooks]]`) + `IPluginService.enabledHooks()`, reloads it - * on `plugin.onDidReload`, and dispatches each trigger through the pure + * on plugin reload and on config change (the interactive TUI can (re)load + * `config.toml` into the layered config after app-scope construction, so the + * index must be rebuilt or late `[[hooks]]` would silently never fire, #2779), + * and dispatches each trigger through the pure * `runMatchedHooks`. The App-scope `IHostProcessService` is injected here and * threaded down to `runHook`, so hook commands spawn through the shared host * process service (cross-platform kill, hidden console on Windows) rather than @@ -38,6 +41,9 @@ export class ExternalHooksRunnerService extends Disposable implements IExternalH private byEvent = new Map(); readonly ready: Promise; + // Serializes index rebuilds so a trigger that lands right after a config + // change awaits the newest index instead of reading the stale one. + private reloadChain: Promise; private readonly _onDidReload = this._register(new Emitter()); readonly onDidReload: Event = this._onDidReload.event; @@ -51,22 +57,21 @@ export class ExternalHooksRunnerService extends Disposable implements IExternalH ) { super(); this.ready = this.loadSafe(); + this.reloadChain = Promise.resolve(); this._register( this.plugins.onDidReload(() => { - void this.reloadSafe(); + this.queueReload(); }), ); // Rebuild the hook index when the config changes, not just on plugin - // reload. The user config file (`config.toml`) can be (re)loaded into the - // layered config service after this runner's initial `loadSafe()` — in the - // interactive TUI in particular, `[[hooks]]` may arrive late. Without this - // subscription the index would stay empty forever and every hook would - // silently never fire (#2779). The member check keeps the subscription a - // no-op against partial `IConfigService` stubs in unit tests. + // reload: the user config file (`config.toml`) can be (re)loaded into the + // layered config service after this runner's initial `loadSafe()`, and in + // the interactive TUI `[[hooks]]` may arrive late. The member check keeps + // the subscription a no-op against partial `IConfigService` stubs. if (this.config.onDidChangeConfiguration !== undefined) { this._register( this.config.onDidChangeConfiguration(() => { - void this.reloadSafe(); + this.queueReload(); }), ); } @@ -115,6 +120,7 @@ export class ExternalHooksRunnerService extends Disposable implements IExternalH args: ExternalHooksRunnerTriggerArgs, ): Promise { await this.ready; + await this.reloadChain; return runMatchedHooks( this.hostProcess, this.byEvent, @@ -137,10 +143,8 @@ export class ExternalHooksRunnerService extends Disposable implements IExternalH } catch {} } - private async reloadSafe(): Promise { - try { - await this.load(); - } catch {} + private queueReload(): void { + this.reloadChain = this.reloadChain.then(() => this.loadSafe()); } private async load(): Promise {