Skip to content

Bound plugin lifecycle hooks with cancellable deadlines - #85

Merged
saeedvaziry merged 1 commit into
mainfrom
fix/bounded-hook-execution
Nov 7, 2026
Merged

saeedvaziry merged 1 commit into
mainfrom
fix/bounded-hook-execution

Conversation

@saeedvaziry

@saeedvaziry saeedvaziry commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Summary

Bound plugin lifecycle hooks so an uncooperative asynchronous handler cannot keep a coding turn pending forever.

  • Add a 60-second per-invocation deadline and optional validated Hook.timeoutMs (integer 1–600,000 ms).
  • Make prompt, pre-tool, completion-gate and turn-end waits cancellable through the existing abort helper. Ignore late results, redact unexpected late failures, and clean up timers/signals.
  • Preserve after-tool processing for interrupted executions, but bound it by the same deadline. A timeout reports that the tool may already have changed state; it does not imply rollback or safe retry.
  • Document the hook API, fail-closed semantics and in-process limitations; add critical hook and real-file regression tests.

Why this improvement

Static inspection of the locally installed Claude Code 2.1.319 found timeout selection, an abortable callback wait, and cleanup in its packaged function-hook dispatcher. Claude was not launched or prompted. Xal already bounds provider stalls and completion-gate continuations, but its shared hook dispatcher directly awaited arbitrary promises, including after interruption.

REA 6.8.0 native identity Evidence: ev_e4ff052dbdf922a9536c239d8019be93afec1dd1bb04e7d2b6384c81dbe8cc4e; installed artifact SHA-256: 1f53587e92dee8e089fa349188a4cda64fa34ace6e7b0ca2ee278ddaa3cc2ebd. Focused static function Evidence: ev_3287a78de67e12b0d93011aa97eb7d1ad8fd9c3e2b22af0087c2540357cae1fe, derived from exact binary bytes [273132358, 273133308). Broad bundle analysis was partial; focused syntax parsed completely, with dynamic callees unresolved. These are static observations, not runtime claims or a complete Claude Code audit. No proprietary source is included in this PR. Xal's budgets and fail-closed policy are independent design decisions.

Deferred follow-ups: foreground session single-writer ownership and fail-closed transcript-write barriers before subsequent side effects. Neither is mixed into this change.

Verification

  • bun test --isolate apps/cli/src/hooks/registry.test.ts apps/cli/src/agent/session/hook-deadlines.test.ts apps/cli/src/agent/session/completion-hooks.test.ts — 31 passed.

  • bun checks:fix — passed (format/lint, Rust checks/tests/clippy, native benchmark, typecheck, all tests, website build, compiled release smoke checks).

  • CLI suite: 930 tests passed; no failures.

  • GitHub checks and all eight macOS/Linux/Windows release-build targets passed for e4d9acab23f79f99c09cf64a4932dd98a526572c.

  • Parent review and independent sub-agent $code-review: Standards PASS, Spec PASS, zero blockers; 6/6 files and 11/11 hunks reviewed (+343/-6).

  • CodeRabbit reported its free OSS review limit and skipped reviewing. No actionable bot findings were posted; its neutral check is not an approval.

Scope: six source/test/documentation files; no dependencies, unrelated changes, or instruction-file changes.

@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 70a82b32-67b0-42a5-8d91-212efb38a0a0
Commits

Reviewing files that changed from the base of the PR and between a579689 and e4d9aca.

Files selected for processing: 6

  • apps/cli/src/agent/session/completion-hooks.test.ts
  • apps/cli/src/agent/session/hook-deadlines.test.ts
  • apps/cli/src/hooks/registry.test.ts
  • apps/cli/src/hooks/registry.ts
  • apps/cli/src/hooks/types.ts
  • docs/plugins.md
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@saeedvaziry

Copy link
Copy Markdown
Member Author

Final review and verification

Reviewed immutable diff a579689da3e0e054f08474328dab60866f7671a0..e4d9acab23f79f99c09cf64a4932dd98a526572c using the repository's code-review skill, both by the implementing agent and an independent read-only sub-agent.

Standards

PASS — 0 blockers. Typed registration validation, reuse of the shared abort helper, no added dependencies or plugin coupling, timer/signal cleanup, redacted late errors, and updated public docs were reviewed.

Spec

PASS — 0 blockers. Reviewed fail-closed deadlines, interruption without handler cooperation, late results/errors, preservation of post-tool cleanup semantics, and file-side-effect warnings. Synchronous event-loop blocking and uncooperative plugin side effects remain explicitly outside the waiting guarantee.

Both reviews covered 6/6 files, 11/11 hunks, all 343 added and 6 removed lines, with no unchecked hunks or binaries. The independent reviewer additionally ran the registry tests (18 passed) and six in-memory cooperative-cancellation race probes; all retained named timeout failures and exactly one terminal event.

Verification:

  • Focused hook/session tests: 31 passed.
  • bun checks:fix: passed — 930 CLI, 58 Rust, 17 website, and 3 script tests, plus lint, formatting, typechecks, native benchmark, website build, and compiled release smoke check.
  • GitHub CI: main checks and all eight release targets passed.
  • CodeRabbit unavailable: the bot reported its free OSS review limit and skipped the review. No code findings were posted. Its neutral check is not treated as approval.

TL;DR

Axis Parent Independent reviewer Blockers
Standards PASS PASS 0
Spec PASS PASS 0

Recommendation: PROCEED. User explicitly authorized merging once verification and blocking reviews pass.

@saeedvaziry
saeedvaziry merged commit 083f1f6 into main Nov 7, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant