fix: don't call a hookOnce callback twice for overlapping callHook calls

[deepanshupal]

resolves #171

• The hookOnce wrapper now no-ops when invoked again after its first call. A concurrent callHook can hold a stale snapshot of the hook list (taken before the first call unregistered the wrapper, e.g. while queued behind an async hook), and previously the second invocation fell through to calling the user's callback again.
• Also simplifies the unregister check to _unreg?.(), which keeps the minified new Hookable() bundle smaller than base (2988 bytes, budget 3000) even with the added guard.

Tests (test/hookable.test.ts):

• new regression test: an async hook ahead of a hookOnce callback plus two overlapping callHook promises fires the callback exactly once (fails pre-fix with x = 2).

Verification against base 6815e4f (main, v6.1.2): new test fails pre-fix and passes post-fix; full vitest run --coverage 45/45 across all 4 files including bundle-size budgets; tsc --noEmit, oxlint, and oxfmt --check all clean.

Summary by CodeRabbit

• Bug Fixes
  • One-time callbacks now run only once, even when multiple concurrent events have already captured the same callback. This prevents duplicate execution in that situation and makes one-time event handling more consistent.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3faac1c1-b647-48e1-81c4-c0c200f2153c

📥 Commits

Reviewing files that changed from the base of the PR and between 6815e4f and f9f1242.

📒 Files selected for processing (2)

• src/hookable.ts
• test/hookable.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

hookOnce now skips its callback when its wrapper has already been cleared. A regression test checks this behavior when two callHook calls overlap behind an async hook.

Changes

hookOnce concurrency

Layer / File(s)	Summary
Guard one-shot callback invocation src/hookable.ts, test/hookable.test.ts	hookOnce skips the callback if its wrapper has already been cleared and invokes the unregister function only when present. The regression test expects the callback to run once across two overlapping calls.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: danielroe

Merge Risk: ⚪ Minimal · up to f9f12

The change prevents duplicate one-shot callbacks during overlapping calls while preserving sequential behavior. No actionable merge risk remains in the reviewed change.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to f9f12

The change prevents duplicate callback execution without expanding caller permissions. The reviewed callback lifecycle introduces no material security risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly affected execution scope is callbacks registered through hookOnce on a Hookable instance. The compared change restricts repeated callback execution rather than adding callers or callback authority; downstream application-specific callback effects are not represented in the supplied evidence.

Trust Boundaries and Controls

• inferred — The changed control is wrapper-local execution eligibility. It does not introduce a new identity transition or bypass registration ownership in the inspected path.

Resilience and Maintainability Implications

• observed — Manual unregister removes a wrapper from future snapshots but does not invalidate a snapshot already captured before removal. This cancellation limitation exists in both base and head and is not an introduced concern.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preventing a hookOnce callback from running twice during overlapping callHook calls.
Linked Issues check	✅ Passed	Issue #171 requires the hookOnce callback to run at most once when overlapping callHook calls retain a stale hook-list snapshot. The reported hookOnce wrapper now returns after its unregister fu…
Out of Scope Changes check	✅ Passed	The reported source change prevents the duplicate invocation described in #171. The regression test verifies that behavior. The optional unregister invocation supports the same change and the stated b…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}