Greptile Summary

This PR adds per-handler timeout protection to all async plugin hook runners, preventing a single hanging plugin from permanently blocking agent runs. The approach — wrapping each call in a Promise.race via the existing withTimeout utility — is sound, and the implementation correctly threads the new hookTimeoutMs option from config all the way through the loader → global runner → createHookRunner.

Key observations:

• Timeout logic is correct: the three-way ternary (> 0 → use value, === 0 → disable, undefined → default 10 s) handles all cases and is validated upstream by Zod's int().min(0).
• Leaked background promises: when a timeout fires the underlying hook-handler promise continues executing; the AbortSignal surfaced by withTimeout is not forwarded to the actual handler (() => fn() ignores the signal). For I/O-heavy hooks this means network/DB work keeps running after the agent has already moved on. This is a known JavaScript limitation but worth documenting.
• Severity mismatch: timed-out handlers are routed through handleHookError, which logs at error level. Since the agent continues gracefully, warn would better reflect the severity and reduce operational noise.
• The PR description states the default is 30 000 ms, but the code (DEFAULT_HOOK_TIMEOUT_MS = 10_000) and all inline docs consistently say 10 000 ms — small description inaccuracy that doesn't affect functionality.
• Test coverage is solid, using vi.useFakeTimers across void, modifying, and claiming hook patterns, plus the catchErrors: false throw path.

Confidence Score: 4/5

• Safe to merge; addresses a real hang bug with no breaking changes. Two non-blocking style improvements (log level and signal forwarding) are worth follow-up but don't block the fix.
• The core timeout logic is correct and well-tested. The two issues flagged (leaked promises, error vs. warn log level) are style/observability concerns, not correctness bugs. Config schema, type definitions, labels, and help text are all consistent (except the PR description's "30 000 ms" vs code's 10 000 ms). No breaking interface changes.
• src/plugins/hooks.ts — the callHandlerWithTimeout signal-forwarding gap and the error-vs-warn log severity are worth revisiting in a follow-up.

This is a comment left during a code review.
Path: src/plugins/hooks.ts
Line: 179-188

Comment:
**Timed-out hook promise is leaked**

When the timeout fires, `withTimeout` races the handler's promise against the abort promise and returns early — but the underlying hook-handler promise (`fn()`) continues executing in the background indefinitely. The `AbortSignal` that `withTimeout` exposes through its `work` callback is never forwarded to the actual hook handler because the wrapper is `() => fn()` (signal ignored).

For hooks doing I/O or significant async work (network calls, file I/O, DB queries) this means that work keeps running and consuming resources long after the agent has already moved on. Consider documenting this behavior, and if the plugin contract ever evolves to accept an `AbortSignal`, wiring it through here would allow cooperative cancellation:

```ts
async function callHandlerWithTimeout<T>(
  fn: (signal?: AbortSignal) => Promise<T>,
  hookName: PluginHookName,
  pluginId: string,
): Promise<T> {
  if (!hookTimeoutMs) {
    return fn();
  }
  return withTimeout((signal) => fn(signal), hookTimeoutMs, `${hookName} handler from ${pluginId}`);
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/plugins/hooks.ts
Line: 244-257

Comment:
**Timeout funneled through error-level log instead of warn**

When a hook handler exceeds the timeout, `withTimeout` rejects with a `"timed out"` error. That rejection is caught and routed through `handleHookError`, which always calls `logger?.error(msg)`. The resulting log line reads:

```
[hooks] before_agent_start handler from my-plugin failed: Error: before_agent_start handler from my-plugin timed out
```

A timeout is an expected, gracefully handled condition — the agent continues running normally. Logging it at `error` level risks unnecessary alert fatigue for operators and obscures real failures. The PR description mentions "logs a warning", which aligns better with the severity. Consider adding a dedicated timeout check before delegating to `handleHookError`:

```ts
const handleHookError = (params: {
  hookName: PluginHookName;
  pluginId: string;
  error: unknown;
}): never | void => {
  const isTimeout =
    params.error instanceof Error && params.error.message.includes("timed out");
  const msg = `[hooks] ${params.hookName} handler from ${params.pluginId} ${
    isTimeout ? "timed out and was skipped" : `failed: ${String(params.error)}`
  }`;
  if (catchErrors) {
    isTimeout ? logger?.warn(msg) : logger?.error(msg);
    return;
  }
  throw new Error(msg, { cause: params.error });
};
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: dc0b997}

Good point about the leaked promise. Added a documentation comment in commit 6de4b04 explaining the limitation — since the current plugin hook contract (() => Promise<T>) doesn't accept an AbortSignal, cooperative cancellation isn't possible yet. The comment notes the path forward for when the plugin API evolves to support signal-based cancellation.

Changing the handler signature would be a breaking change to the plugin API, so keeping it as documented behavior for now seemed like the right scope for this PR.

Rebased on latest upstream/main and resolved conflicts in src/plugins/hooks.ts — upstream extracted the claiming-hook loop into runClaimingHooksList, which already uses our callHandlerWithTimeout pattern. Merged cleanly with our timeout infrastructure.

Addressed both review points in commit b430cf6:

P1 — Scope timeout away from gatekeeper hooks: Added a TIMEOUT_EXEMPT_HOOKS set containing message_sending and before_tool_call. These security/policy hooks now bypass the timeout wrapper entirely, preventing silent fail-open when moderation or tool-blocking checks take longer than the budget. All other hooks (before_agent_start, before_model_resolve, etc.) remain timeout-protected.

P2 — Raise default timeout: Bumped DEFAULT_HOOK_TIMEOUT_MS from 10s to 30s. This gives plugins like memory-lancedb enough headroom for cold-start embedding requests while still catching genuinely hung handlers.

Added tests verifying that message_sending and before_tool_call handlers are never timed out even with a very short hookTimeoutMs.

Rebased on latest upstream/main and resolved conflict in src/plugins/loader.ts — upstream refactored activatePluginRegistry to accept cached.registry instead of the full cache entry. Merged cleanly with our hookTimeoutMs option parameter. All timeout infrastructure preserved.

Addressed both P2 comments in commit 533235d:

P2 — Raise default hook timeout / memory recall budget (comment #2970594620): Added before_agent_start to TIMEOUT_EXEMPT_HOOKS. Memory recall via memory-lancedb does live embedding requests in this hook, and the core embedding budget allows up to 60s remote / 5m local — well above the 30s hook timeout. Exempting this hook ensures memory recall completes on slower remote embedders without being silently dropped.

P2 — Exempt agent_end from timeout (comment #2970594621): Added agent_end to TIMEOUT_EXEMPT_HOOKS for the same reason — the memory auto-capture path performs up to three sequential embed() calls plus DB operations, which legitimately exceed 30s on slower providers. These hooks have their own upstream timeout budgets and should not be subject to the general hook timeout.

Updated the comment above TIMEOUT_EXEMPT_HOOKS to document both categories of exempt hooks: (a) security/policy gates that must not fail-open, and (b) memory-critical hooks with their own upstream timeout budgets. Migrated existing tests from before_agent_start/agent_end to non-exempt hooks (before_prompt_build/message_received) and added tests verifying both newly exempt hooks are not timed out.

Rebased on latest upstream/main and resolved conflict in src/plugins/hook-runner-global.ts — upstream refactored the global state to use resolveGlobalSingleton at module scope instead of getHookRunnerGlobalState(). Preserved our options?: { hookTimeoutMs?: number } parameter addition. All changes intact.

Addressed both review points in commit 26c775e:

P1 — Restore timeout protection for memory hooks: Removed before_agent_start and agent_end from TIMEOUT_EXEMPT_HOOKS. Instead of full exemption, they now get a dedicated MEMORY_HOOK_TIMEOUT_MS of 120s (2 minutes) — enough headroom for remote embed() calls (typically <60s) while still terminating genuinely hung handlers. This restores the deadlock protection that this PR is meant to provide, without silently dropping legitimate memory operations.

P2 — Regenerate config schema: Ran generate-base-config-schema.ts --write and config:docs:gen — hookTimeoutMs is now present in src/config/schema.base.generated.ts and the config baseline docs.

Fixed in commit 29d22f2 — memory hooks now use Math.max(MEMORY_HOOK_TIMEOUT_MS, hookTimeoutMs) instead of a hard-coded 120s cap. When plugins.hookTimeoutMs is configured above the 120s floor (e.g. hookTimeoutMs: 300000 for slow local embedding providers), the configured value is honored. The 120s floor is preserved as the minimum for memory hooks when the configured timeout is lower.

Added a test verifying that a 4-minute handler completes successfully when hookTimeoutMs is set to 5 minutes.

Good catch — added subagent_spawning to the elevated timeout set in commit 57ec97a. Channel thread-binding hooks (e.g. Discord) make several REST calls that can legitimately exceed the generic 30s cap. They now get at least 120s (or the configured hookTimeoutMs, whichever is higher), consistent with how memory hooks are handled.

Rebased on latest upstream/main and resolved conflict in docs/.generated/config-baseline.jsonl (generated file meta header). All changes preserved.

Rebased on latest upstream/main and resolved conflicts in src/plugins/loader.ts (upstream added runtimeSubagentMode parameter — preserved alongside our hookTimeoutMs options). All changes preserved.