feat: implement EventManager with parallel event execution #79
Conversation
… types, and tests.
WalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Emitter as Emitter
participant Manager as EventManager
participant Cycle as CycleContext
participant Registry as ListenerRegistry
participant Executor as EmissionExecutor
Emitter->>Manager: emit(event, { source, payload })
Manager->>Cycle: runEmission(frame, source, processEmission)
Cycle->>Manager: (invokes) processEmission()
Manager->>Registry: getListenersForEmit(event)
Registry-->>Manager: ordered listeners
Manager->>Manager: group listeners into batches by order
alt event.parallel == true
Manager->>Executor: executeInParallel(batch0, event)
Executor->>Executor: run listeners in batch concurrently (Promise.allSettled)
Executor-->>Manager: batch0 results / aggregated errors
opt no fatal error
Manager->>Executor: executeInParallel(nextBatch, event)
end
else
Manager->>Executor: executeSequentially(allListeners, event)
Executor-->>Manager: sequential results/errors (stops on propagation)
end
Manager-->>Cycle: emission finished
Cycle-->>Emitter: resolve / reject (AggregateError if multiple)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/fastify-mikroorm/readmes/runner-AI.md (1)
644-666: Section ordering appears incorrect.The "## Validation" header at line 644 is immediately followed by "## Event Cycle Safety" at line 646, but the validation content (interface definition at lines 660-666 and examples at lines 668-697) appears after the Event Cycle Safety section. This makes the document structure confusing.
Consider reordering so that the Event Cycle Safety section is placed before or after the complete Validation section, or make Event Cycle Safety a subsection (
###) if it's meant to be nested under Validation.
🧹 Nitpick comments (1)
src/models/EventManager.ts (1)
555-598: Consider extracting shared listener execution logic.The source-skip check (lines 565-567) and filter application (lines 569-571) are duplicated between
executeSequentiallyandexecuteInParallel. Consider extracting a helper method likeshouldExecuteListener(listener, event)to reduce duplication and ensure consistent behavior if the logic evolves.+ private shouldExecuteListener(listener: IListenerStorage, event: IEventEmission<any>): boolean { + // Skip handlers that identify themselves as the source of this event + if (listener.id && listener.id === event.source) { + return false; + } + return !listener.filter || listener.filter(event); + }Then use
if (this.shouldExecuteListener(listener, event))in both executors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/fastify-mikroorm/readmes/runner-AI.md(4 hunks)src/__tests__/models/EventManagerParallel.test.ts(1 hunks)src/definers/defineEvent.ts(1 hunks)src/models/EventManager.ts(3 hunks)src/types/event.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.tssrc/types/event.tssrc/definers/defineEvent.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.tssrc/types/event.tssrc/definers/defineEvent.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.tssrc/types/event.tssrc/definers/defineEvent.ts
🧠 Learnings (2)
📚 Learning: 2025-09-26T12:31:39.948Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-26T12:31:39.948Z
Learning: Applies to {AI.md,README.md} : Update AI.md and README.md when introducing features/enhancements/changes
Applied to files:
examples/fastify-mikroorm/readmes/runner-AI.md
📚 Learning: 2025-09-01T12:21:34.022Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-01T12:21:34.022Z
Learning: Applies to {README.md,AI.md} : Update README.md and AI.md after any feature or change
Applied to files:
examples/fastify-mikroorm/readmes/runner-AI.md
🧬 Code graph analysis (2)
src/__tests__/models/EventManagerParallel.test.ts (3)
src/models/EventManager.ts (1)
EventManager(65-599)src/types/event.ts (1)
IEvent(48-58)src/definers/defineEvent.ts (1)
defineEvent(11-30)
src/models/EventManager.ts (1)
src/types/event.ts (1)
IEventEmission(63-97)
🔇 Additional comments (10)
src/types/event.ts (1)
37-41: LGTM!The new
parallelproperty is well-documented and correctly typed as an optional boolean. The JSDoc clearly communicates the behavioral implication regarding execution order.src/definers/defineEvent.ts (1)
22-22: LGTM!The
parallelproperty is correctly propagated from the event configuration to the returned event object, maintaining consistency with how other properties likeidandtagsare explicitly assigned.src/models/EventManager.ts (2)
183-191: LGTM!The branching logic cleanly delegates to the appropriate executor based on the event's parallel configuration. The propagation closure is correctly passed to
executeSequentiallyto maintain the existing stop-propagation behavior.
528-547: LGTM!The
executeSequentiallymethod correctly implements sequential execution with proper propagation checking, source listener exclusion, and filter application.src/__tests__/models/EventManagerParallel.test.ts (6)
10-17: LGTM!Good test setup with
runtimeCycleDetectionenabled and a parallel event definition. ThebeforeEachensures clean state for each test.
19-44: LGTM!This test correctly validates that within a single batch (same
order), faster listeners complete before slower ones, demonstrating true parallel execution.
46-81: LGTM!Good test coverage for batch sequencing - validates that batches with different orders execute sequentially while listeners within the same batch execute in parallel.
83-136: LGTM!Excellent coverage of propagation semantics in parallel mode. The tests correctly validate that
stopPropagation()blocks subsequent batches (lines 83-108) but does not affect other listeners in the same parallel batch (lines 110-136).
138-158: LGTM!Good error propagation test. Validates that errors thrown within parallel listeners correctly propagate to the caller via
Promise.all.
159-212: LGTM!Good coverage for source listener exclusion and filter functionality in parallel execution mode. Both tests correctly validate that these features work consistently with the parallel execution path.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/__tests__/models/EventManagerParallel.test.ts (1)
14-60: Optional: factor out thedelayhelper to reduce repetitionThe
delayhelper is redefined in multiple tests; you could move it to a small shared utility within this file (or a common test helper) to avoid duplication and keep each test focused purely on behavior setup and assertions.Also applies to: 41-76
src/models/EventManager.ts (1)
526-540: Centralized listener filtering/source-exclusion is a good improvement; consider tightening event typingMoving source-id exclusion and filter evaluation into
shouldExecuteListenerand using it from both sequential and parallel executors reduces duplication and keeps behavior consistent. One possible improvement is to makeexecuteSequentially,executeInParallel, andshouldExecuteListenergeneric over the payload type (e.g.,IEventEmission<TPayload>instead ofIEventEmission<any>) to better respect the strict typing guideline without changing runtime behavior.Also applies to: 548-566, 598-607
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/models/EventManagerParallel.test.ts(1 hunks)src/models/EventManager.ts(2 hunks)src/types/event.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/event.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/__tests__/models/EventManagerParallel.test.tssrc/models/EventManager.ts
🧬 Code graph analysis (2)
src/__tests__/models/EventManagerParallel.test.ts (2)
src/models/EventManager.ts (1)
EventManager(65-608)src/definers/defineEvent.ts (1)
defineEvent(11-30)
src/models/EventManager.ts (1)
src/types/event.ts (1)
IEventEmission(63-97)
🔇 Additional comments (3)
src/__tests__/models/EventManagerParallel.test.ts (1)
5-12: Comprehensive coverage of parallel execution semantics looks solidThe suite nicely exercises parallel vs sequential batches, propagation stop across/within batches, interceptor interaction, error propagation, source-id skipping, and per-listener filters for parallel events. The expectations align with the new
EventManager.executeInParallelandshouldExecuteListenerbehavior, so from a behavior standpoint this file looks good.Also applies to: 14-236
src/models/EventManager.ts (2)
150-176: Emit branching oneventDefinition.parallelcleanly separates sequential vs parallel pathsThe new
parallel-based branch reuses the same listener selection and interceptor chain while delegating execution strategy toexecuteSequentiallyvsexecuteInParallel. The earlyallListeners.length === 0check also guardsexecuteInParallelfrom empty arrays, so this integration looks correct and non-breaking for non-parallel events.Also applies to: 183-191
519-588: Parallel batch execution and propagation semantics match the intended behavior
executeInParallelcorrectly:
- Groups listeners by
order, running each group as a batch.- Executes each batch concurrently, waiting for completion before moving to the next batch.
- Re-checks
event.isPropagationStopped()between batches so a stopper in one batch prevents subsequent batches, while not cancelling peers in the same batch.This aligns with the new tests (same-order parallelism, sequential batches, stopPropagation behavior) and should behave well with interceptors that stop propagation before reaching the executor.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/__tests__/models/EventManagerParallel.test.ts (1)
17-43: Same-order parallelism test is clear; consider extractingdelayhelperThis test cleanly distinguishes true parallel execution from sequential execution by asserting
["fast", "slow"]even though the slow listener is registered first; it will reliably fail if same‑order listeners are run sequentially. To avoid repeating thedelayhelper and keep tests DRY, you could lift it to thedescribescope and reuse it here and in the next test.describe("EventManager Parallel Execution", () => { let eventManager: EventManager; let parallelEvent: IEvent<string>; + + const delay = (ms: number) => + new Promise((resolve) => setTimeout(resolve, ms)); @@ it("should execute listeners with the same order in parallel", async () => { const results: string[] = []; - const delay = (ms: number) => - new Promise((resolve) => setTimeout(resolve, ms));(And similarly drop the local
delayin the batch test below.) Optionally, you could also reduce the delay to a smaller value (e.g., 5–10 ms) to keep the suite fast as it grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/__tests__/models/EventManagerParallel.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/__tests__/models/EventManagerParallel.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/__tests__/models/EventManagerParallel.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/__tests__/models/EventManagerParallel.test.ts
🧬 Code graph analysis (1)
src/__tests__/models/EventManagerParallel.test.ts (3)
src/models/EventManager.ts (1)
EventManager(65-608)src/types/event.ts (1)
IEvent(48-58)src/definers/defineEvent.ts (1)
defineEvent(11-30)
🔇 Additional comments (7)
src/__tests__/models/EventManagerParallel.test.ts (7)
1-15: Setup correctly exercises parallel event definitions under cycle detectionThe imports are purely relative, and the shared
beforeEachusesdefineEventwithparallel: trueplusEventManager({ runtimeCycleDetection: true }), which is a good baseline for all the following parallel semantics tests. No changes needed here.As per coding guidelines, this adheres to the relative‑import and type‑strictness expectations.
45-81: Batch sequencing test accurately validates inter-batch orderingThis test nicely complements the previous one: within batch 1 (order 0) you still assert
["batch1-fast", "batch1-slow"], and then you verify that batch 2 (order 1) only runs after batch 1 completes, yielding["batch1-fast", "batch1-slow", "batch2"]. This will catch regressions where batches are not awaited sequentially or where intra-batch execution becomes inadvertently sequential.
83-136: Propagation tests clearly distinguish between inter-batch and intra-batch semanticsThe pair of tests here do a good job of locking in the contract:
- Stopping propagation in a batch with order 0 prevents later batches (order 1) from running at all.
- Stopping propagation within a batch does not prevent other listeners in that same batch from executing.
Together they ensure
executeInParallelperforms propagation checks between batches rather than per-listener, which matches the described EventManager semantics.
138-165: Interceptor propagation test correctly asserts short-circuiting at entryBy having an interceptor call
event.stopPropagation()and still invokingawait next(event), this test ensures that EventManager checksisPropagationStopped()before scheduling any listeners, even when interceptors choose to forward the call. The expectation ofresultsstaying empty captures that contract well. No changes needed.
167-187: Parallel error propagation test is minimal but sufficientThis test verifies that an error thrown by one listener in a parallel batch rejects
emitwith that error, which is the critical behaviour to lock in for callers. Given other tests already cover ordering semantics, this focused assertion is reasonable as-is; you can always extend it later to also assert that successful listeners still get executed if you need stronger guarantees.
188-211: Source self-invocation skipping is well coveredUsing
id: sourceIdon one listener and passingsourceIdas the third argument toemitis a clear way to validate thatshouldExecuteListenerskips the source listener while allowing others in the same batch to run. The expected["should-run"]result precisely targets that behaviour.
213-241: Filter semantics test cleanly validates conditional execution in parallel modeThe two listeners with opposing
filterpredicates ensure that parallel execution respects listener-level filters: thefalsefilter never fires, while thetruefilter does, resulting in exactly["filtered-in"]. This is a succinct and robust assertion of filter handling in the parallel path.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/chatmodes/code-reviewer.chatmode.md (1)
55-58: Minor style improvement: Vary sentence structure in constraints.The static analysis tool flagged three successive sentences beginning with "Do not". Consider varying the phrasing for better readability.
Mode-Specific Constraints: - Do not provide feedback on code that is not provided by the user. -- Do not engage in discussions unrelated to code review. -- Do not provide feedback on code that is not in a programming language that you are familiar with. -- Do not provide feedback on code that is not in a format that you can understand. +- Avoid engaging in discussions unrelated to code review. +- Only provide feedback on code in programming languages you are familiar with. +- Ensure the code is in a format you can understand before providing feedback.README.md (1)
476-491: Consider adding a payload schema to the example for clarity.The example references
event.dataindoWork(event.data)(Line 483), but the event has nopayloadSchema, makingevent.datatypevoid. Adding a schema would make the example more realistic.-const parallelEvent = r.event("app.events.parallel").parallel(true).build(); +const parallelEvent = r + .event("app.events.parallel") + .payloadSchema<{ id: string }>({ parse: (v) => v }) + .parallel(true) + .build(); r.hook("app.hooks.first") .on(parallelEvent) .order(0) .run(async (event) => { - await doWork(event.data); + await doWork(event.data); // { id: string } }) .build();src/__tests__/models/EventManagerParallel.test.ts (2)
17-43: Timing-based test may be flaky under load.The test relies on a 50ms delay to demonstrate parallel execution. While reasonable, this could occasionally fail in slow CI environments.
Consider using a deterministic approach with shared state or increasing the delay margin if flakiness is observed.
214-225: Jest'sfail()function may not be available in all configurations.The
fail()function is not part of Jest's core API in newer versions. Consider using an alternative pattern:- try { - await eventManager.emit(parallelEvent, "data", "test"); - fail("Should have thrown"); - } catch (err: any) { + let caughtError: any; + try { + await eventManager.emit(parallelEvent, "data", "test"); + expect.fail("Should have thrown"); + } catch (err) { + caughtError = err; + } + expect(caughtError).toBeDefined(); + expect(caughtError.name).toBe("AggregateError");Alternatively, use
expect.assertions(n)at the start and let the test fail if assertions aren't hit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/chatmodes/code-reviewer.chatmode.md(1 hunks)AI.md(2 hunks)README.md(1 hunks)prettier.config.js(1 hunks)src/__tests__/models/EventManagerParallel.test.ts(1 hunks)src/definers/builders/event.ts(5 hunks)src/models/EventManager.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/models/EventManager.tssrc/definers/builders/event.tssrc/__tests__/models/EventManagerParallel.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/models/EventManager.tssrc/definers/builders/event.tssrc/__tests__/models/EventManagerParallel.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/models/EventManager.tsprettier.config.jssrc/definers/builders/event.tssrc/__tests__/models/EventManagerParallel.test.ts
{README.md,AI.md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update README.md and AI.md after any feature or change
Files:
README.mdAI.md
{AI.md,README.md}
📄 CodeRabbit inference engine (AGENTS.md)
Update AI.md and README.md when introducing features/enhancements/changes
Files:
README.mdAI.md
🧠 Learnings (1)
📚 Learning: 2025-09-26T12:31:39.948Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-26T12:31:39.948Z
Learning: Applies to {AI.md,README.md} : Update AI.md and README.md when introducing features/enhancements/changes
Applied to files:
README.md
🧬 Code graph analysis (3)
src/models/EventManager.ts (1)
src/types/event.ts (1)
IEventEmission(63-97)
src/definers/builders/event.ts (2)
src/types/event.ts (1)
IEventDefinition(28-42)src/types/tag.ts (1)
TagType(83-86)
src/__tests__/models/EventManagerParallel.test.ts (4)
src/models/EventManager.ts (1)
EventManager(65-640)src/types/event.ts (1)
IEvent(48-58)src/definers/defineEvent.ts (1)
defineEvent(11-30)src/definers/builders/event.ts (1)
event(100-100)
🪛 LanguageTool
README.md
[style] ~470-~470: As an alternative to the over-used intensifier ‘really’, consider replacing this phrase.
Context: ... }) .build(); ``` > runtime: "'A really good office messenger.' That's me in rollerb...
(EN_WEAK_ADJECTIVE)
.github/chatmodes/code-reviewer.chatmode.md
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...discussions unrelated to code review. - Do not provide feedback on code that is no...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... language that you are familiar with. - Do not provide feedback on code that is no...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (16)
src/definers/builders/event.ts (3)
39-46: LGTM! Well-documented parallel API.The JSDoc clearly explains the behavior: listeners with the same
orderrun concurrently within a batch, and batches execute sequentially. The defaulttrueforenabledprovides ergonomic usage.
72-75: LGTM! Clean implementation following builder pattern.The
parallel()method correctly clones state with the new flag and returns a new builder instance, consistent with the existing fluent API methods.
95-95: LGTM! Correct default value.Using
undefinedas the initial value allows distinguishing between "parallel not configured" vs "parallel explicitly disabled", which is appropriate for optional configuration.README.md (2)
472-498: LGTM! Clear documentation of parallel execution semantics.The section effectively explains the parallel execution model with a practical example. The execution semantics bullet points cover important edge cases like
AggregateErrorandstopPropagationbehavior between batches.
500-505: LGTM! Well-documented execution semantics.The bullet points clearly explain the parallel execution guarantees, error aggregation behavior, and the important caveat that
stopPropagation()only works between batches—not within a parallel batch.AI.md (2)
18-19: LGTM! TOC additions for HTTP client documentation.
156-161: LGTM! Concise parallel execution guidance.The bullet points effectively summarize the parallel execution semantics in a token-friendly format, covering batch behavior, error aggregation, propagation rules, and failure handling.
src/models/EventManager.ts (4)
183-191: LGTM! Clean dispatch to parallel/sequential executors.The branching logic correctly delegates based on
eventDefinition.parallel, with the sequential path properly passing the propagation-stopped callback. The separation of concerns between execution strategies improves maintainability.
526-540: LGTM! Sequential execution is clean and correct.The implementation properly checks propagation status before each listener and delegates filtering to
shouldExecuteListener. The earlybreakon propagation stop is the expected behavior for sequential execution.
630-639: LGTM! Clean extraction of listener filtering logic.The method correctly handles both source-exclusion (preventing self-invocation) and optional filter conditions. Good refactoring to centralize this logic for use by both sequential and parallel executors.
590-595: The current implementation correctly uses manualAggregateErrorconstruction because the project targets ES2020 (pertsconfig.json). NativeAggregateErrorwas introduced in ES2021, which is not available in the project's target environment. The manual construction with theas anytype cast is necessary and appropriate for ES2020 compatibility.Likely an incorrect or invalid review comment.
src/__tests__/models/EventManagerParallel.test.ts (5)
1-15: LGTM! Well-structured test setup.The test file properly imports dependencies and sets up a fresh
EventManagerwithruntimeCycleDetection: truebefore each test. The parallel event is correctly defined withparallel: true.
45-136: LGTM! Comprehensive batch and propagation tests.These tests correctly validate:
- Sequential execution across batches with parallel execution within batches
- Propagation stopping prevents subsequent batches from running
- Propagation stop within a batch doesn't affect other listeners in the same batch
Good coverage of the core parallel execution semantics.
227-280: LGTM! Good coverage of listener filtering scenarios.The tests for source exclusion and filter functions correctly validate the
shouldExecuteListenerlogic. Both scenarios are well-structured and verify the expected behavior.
282-342: LGTM! Edge cases are well covered.Good test coverage for:
- Empty listeners array (graceful no-op)
- Single listener (no batching overhead)
- All different orders (effectively sequential execution)
These ensure the parallel execution handles boundary conditions correctly.
344-370: LGTM! Error propagation correctly stops subsequent batches.This test validates that when a batch throws an error, subsequent batches do not execute. The assertion correctly verifies both the error propagation and that batch 1 never ran.
| trailingComma: "all", | ||
| tabWidth: 2, | ||
| singleQuote: false, | ||
| ignoreList: [".github/**/*.md"], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Prettier ignoreList configuration option prettier.config.js documentation 2024 2025
💡 Result:
Short answer: Prettier does not support an ignoreList (or similar) option inside prettier.config.js as of 2024–2025. Use .prettierignore or the CLI/--ignore-path behavior instead.
Sources:
- Prettier docs — Ignoring files (.prettierignore and ignore behavior). [1]
- Prettier docs — Configuration and CLI options (mentions --ignore-path and config file types). [2]
- Prettier GitHub issue requesting an “ignore paths” config option (proposal, not implemented). [3]
References:
[1] https://prettier.io/docs/next/ignore/
[2] https://prettier.io/docs/configuration/ and https://prettier.io/docs/next/cli/
[3] prettier/prettier#4708
ignoreList is not a valid Prettier configuration option.
Prettier does not support ignoreList in prettier.config.js. Use a .prettierignore file in the project root to ignore files instead.
Create a .prettierignore file with:
.github/**/*.md
Or remove the invalid option from prettier.config.js:
module.exports = {
trailingComma: "all",
tabWidth: 2,
singleQuote: false,
- ignoreList: [".github/**/*.md"],
overrides: [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ignoreList: [".github/**/*.md"], | |
| module.exports = { | |
| trailingComma: "all", | |
| tabWidth: 2, | |
| singleQuote: false, | |
| overrides: [ |
🤖 Prompt for AI Agents
In prettier.config.js around line 5, the configuration uses an invalid option
`ignoreList`; Prettier does not support this key. Remove the `ignoreList:
[".github/**/*.md"],` entry from prettier.config.js, and instead create a
.prettierignore file at the project root containing `.github/**/*.md` (or add
that pattern to an existing .prettierignore) to achieve the intended ignore
behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/models/EventManager.ts (2)
542-647: Consider using native AggregateError if available.The parallel execution logic is well-implemented with proper batching, error handling, and metadata annotation. However, consider using the native
AggregateErrorconstructor (available in modern Node.js and browsers) instead of manually creating an error object and setting its name.Apply this diff to use native AggregateError when available:
if (errors.length > 0) { if (errors.length === 1) { throw errors[0]; } - // Create a composite error that contains all failures and their listener ids on each error object - const aggregateError = new Error( - `${errors.length} listeners failed in parallel batch`, - ); - (aggregateError as any).errors = errors; - aggregateError.name = "AggregateError"; - throw aggregateError; + // Use native AggregateError if available, otherwise polyfill + if (typeof AggregateError !== "undefined") { + throw new AggregateError( + errors, + `${errors.length} listeners failed in parallel batch`, + ); + } else { + // Fallback for environments without native AggregateError + const aggregateError = new Error( + `${errors.length} listeners failed in parallel batch`, + ) as any; + aggregateError.errors = errors; + aggregateError.name = "AggregateError"; + throw aggregateError; + } }This eliminates the type cast and uses the standard error type when available.
574-590: Improve type safety in error annotation.The error annotation logic uses type casting that could be more type-safe. Consider defining a proper interface for annotated errors.
Add a type definition near the top of the file and update the annotation function:
// Near other type definitions (after imports) interface AnnotatedError extends Error { listenerId?: string; listenerOrder?: number; }Then update the annotateError function:
const annotateError = ( reason: unknown, listener: IListenerStorage, - ) => { + ): AnnotatedError => { const base = reason && typeof reason === "object" - ? (reason as any) - : (new Error(String(reason)) as any); + ? (reason as AnnotatedError) + : (new Error(String(reason)) as AnnotatedError); if (base.listenerId === undefined) { base.listenerId = listener.id; } if (base.listenerOrder === undefined) { base.listenerOrder = listener.order; } - return base as Error & { listenerId?: string; listenerOrder?: number }; + return base; };This improves type safety and aligns with the coding guideline to avoid unnecessary type casting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/definers/event.builder.test.ts(1 hunks)src/__tests__/models/EventManager.test.ts(1 hunks)src/models/EventManager.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/__tests__/models/EventManager.test.tssrc/__tests__/definers/event.builder.test.tssrc/models/EventManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/__tests__/models/EventManager.test.tssrc/__tests__/definers/event.builder.test.tssrc/models/EventManager.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/__tests__/models/EventManager.test.tssrc/__tests__/definers/event.builder.test.tssrc/models/EventManager.ts
🧬 Code graph analysis (2)
src/__tests__/models/EventManager.test.ts (2)
src/index.ts (1)
defineEvent(48-48)src/define.ts (1)
defineEvent(5-5)
src/__tests__/definers/event.builder.test.ts (1)
src/index.ts (1)
r(64-76)
🔇 Additional comments (8)
src/__tests__/models/EventManager.test.ts (3)
268-327: LGTM! Comprehensive test for parallel error aggregation.This test thoroughly validates the parallel execution error handling, including:
- Multiple concurrent failures aggregated into AggregateError
- Listener metadata (listenerId, listenerOrder) properly attached to errors
- Propagation stop after batch failure (order: 1 listener doesn't run)
- Mixed success/failure scenarios within a batch
329-350: LGTM! Single error annotation validated correctly.This test confirms that when only one listener fails in a parallel batch, the error is thrown directly (not wrapped in AggregateError) with proper listener metadata attached.
352-373: LGTM! Primitive error handling covered.Good coverage of the edge case where a listener throws a non-Error value (primitive string). The test validates that the error is properly wrapped and annotated with listener metadata.
src/__tests__/definers/event.builder.test.ts (2)
4-28: LGTM! Builder API for parallel execution well-tested.The test validates:
- Parallel flag propagation through the builder chain
- Default behavior when
parallel()is called without arguments (defaults to true)- Explicit
parallel(false)setting- Proper integration with other builder methods (payloadSchema, meta)
- Internal symbolFilePath assignment
30-58: LGTM! Tag behavior thoroughly tested.Comprehensive coverage of tag management:
- Append behavior (default)
- Override behavior with explicit
{ override: true }- Default options handling with empty object
{}src/models/EventManager.ts (3)
183-191: LGTM! Clean delegation to execution strategies.The branching logic properly delegates to the appropriate executor based on the
parallelflag, passing the necessary context for propagation control in the sequential case.
519-540: LGTM! Sequential execution correctly implemented.The method properly:
- Checks propagation before each listener
- Uses the extracted
shouldExecuteListenerhelper for source/filter checks- Maintains backward compatibility with existing behavior
649-666: LGTM! Helper method properly encapsulates listener execution checks.The
shouldExecuteListenermethod cleanly extracts the source exclusion and filter logic, making it reusable across both sequential and parallel execution paths.
…d modularity and maintainability - Moved event listener management to ListenerRegistry for better separation of concerns. - Introduced CycleContext for handling cycle detection in event emissions. - Simplified event emission execution with dedicated functions for sequential and parallel execution. - Enhanced interceptor handling with a utility function to compose interceptors. - Updated tests to cover new structures and functionalities in ListenerRegistry and CycleContext. - Removed deprecated code and interfaces to streamline the EventManager class.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/__tests__/models/event/CycleContext.test.ts (1)
3-12: Consider expanding test coverage for CycleContext.The test correctly verifies the disabled path, but CycleContext has significant logic that remains untested:
runEmissionwith cycle detection- Enabled mode when
AsyncLocalStorageis available- The
safeReEmitBySameHookbypass logic- Error throwing via
eventCycleErrorsrc/__tests__/models/event/ListenerRegistry.test.ts (1)
5-5: Avoidas anytype casting per coding guidelines.Multiple uses of
as anyfor event objects. Consider defining a minimal test interface that satisfies the registry's requirements.+interface TestEvent { + id: string; + exclude: boolean; +} + describe("ListenerRegistry", () => { it("getListenersForEmit respects excludeFromGlobal hook", () => { - const registry = new ListenerRegistry((event: any) => event.exclude === true); + const registry = new ListenerRegistry((event: TestEvent) => event.exclude === true); // ... - const includeEvent = { id: eventId, exclude: false } as any; - const excludeEvent = { id: eventId, exclude: true } as any; + const includeEvent: TestEvent = { id: eventId, exclude: false }; + const excludeEvent: TestEvent = { id: eventId, exclude: true };Also applies to: 14-15, 30-31
src/models/event/EmissionExecutor.ts (1)
69-79: Consider using nativeAggregateErrorif targeting ES2021+.The manual construction works but native
AggregateErrorprovides better stack traces and is recognized by error handling tools.if (errors.length > 0) { if (errors.length === 1) { throw errors[0]; } - const aggregateError = new Error( + throw new AggregateError( + errors, `${errors.length} listeners failed in parallel batch`, ); - (aggregateError as any).errors = errors; - aggregateError.name = "AggregateError"; - throw aggregateError; }If ES2021 isn't guaranteed, the current approach is acceptable.
src/models/event/CycleContext.ts (1)
10-22: Minor: CachegetPlatform()result to avoid repeated calls.
getPlatform()is called three times in the constructor. While likely cheap, caching improves clarity.constructor(runtimeCycleDetection: boolean) { - if (getPlatform().hasAsyncLocalStorage() && runtimeCycleDetection) { + const platform = getPlatform(); + if (platform.hasAsyncLocalStorage() && runtimeCycleDetection) { this.emissionStack = - getPlatform().createAsyncLocalStorage<IEmissionFrame[]>(); + platform.createAsyncLocalStorage<IEmissionFrame[]>(); this.currentHookIdContext = - getPlatform().createAsyncLocalStorage<string>(); + platform.createAsyncLocalStorage<string>(); this.isEnabled = true;src/models/EventManager.ts (3)
31-34: Consider refactoring tests to avoid exposing internal registry references.The direct exposure of
listeners,globalListeners, andcachedMergedListenersfrom the registry creates tight coupling. While the comments acknowledge this is for backward compatibility, consider incrementally updating tests to use public methods likehasListeners()or adding test-specific public accessors instead of direct field references.Also applies to: 54-57
105-111: Potential code duplication with registry method.The logic here manually checks
excludeFromGlobaland selects listeners, which duplicates the logic inregistry.getListenersForEmit(). Consider using:- const excludeFromGlobal = - globalTags.excludeFromGlobalHooks.exists(eventDefinition); - - // Choose listeners: if globals are excluded, only use event-specific listeners - const allListeners = excludeFromGlobal - ? this.listeners.get(eventDefinition.id) || [] - : this.getCachedMergedListeners(eventDefinition.id); + const allListeners = this.registry.getListenersForEmit(eventDefinition);This centralizes the listener selection logic and improves maintainability.
282-288: Consider simplifying the cycle context conditional.The conditional logic works correctly but could be more concise:
- if (this.cycleContext.isEnabled) { - return await this.cycleContext.runHook(hook.id, () => - executeWithInterceptors(hook, event), - ); - } - - return await executeWithInterceptors(hook, event); + return this.cycleContext.isEnabled + ? await this.cycleContext.runHook(hook.id, () => executeWithInterceptors(hook, event)) + : await executeWithInterceptors(hook, event);Or keep the
ifbut remove the early return:+ let result: any; if (this.cycleContext.isEnabled) { - return await this.cycleContext.runHook(hook.id, () => + result = await this.cycleContext.runHook(hook.id, () => executeWithInterceptors(hook, event), ); + } else { + result = await executeWithInterceptors(hook, event); } - - return await executeWithInterceptors(hook, event); + return result;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
src/__tests__/models/event/CycleContext.test.ts(1 hunks)src/__tests__/models/event/ListenerRegistry.test.ts(1 hunks)src/models/EventManager.ts(8 hunks)src/models/event/CycleContext.ts(1 hunks)src/models/event/EmissionExecutor.ts(1 hunks)src/models/event/InterceptorPipeline.ts(1 hunks)src/models/event/ListenerRegistry.ts(1 hunks)src/models/event/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/models/event/InterceptorPipeline.tssrc/models/event/CycleContext.tssrc/__tests__/models/event/ListenerRegistry.test.tssrc/models/event/types.tssrc/__tests__/models/event/CycleContext.test.tssrc/models/event/ListenerRegistry.tssrc/models/EventManager.tssrc/models/event/EmissionExecutor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/models/event/InterceptorPipeline.tssrc/models/event/CycleContext.tssrc/__tests__/models/event/ListenerRegistry.test.tssrc/models/event/types.tssrc/__tests__/models/event/CycleContext.test.tssrc/models/event/ListenerRegistry.tssrc/models/EventManager.tssrc/models/event/EmissionExecutor.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/models/event/InterceptorPipeline.tssrc/models/event/CycleContext.tssrc/__tests__/models/event/ListenerRegistry.test.tssrc/models/event/types.tssrc/__tests__/models/event/CycleContext.test.tssrc/models/event/ListenerRegistry.tssrc/models/EventManager.tssrc/models/event/EmissionExecutor.ts
🧬 Code graph analysis (5)
src/models/event/CycleContext.ts (3)
src/models/event/types.ts (1)
IEmissionFrame(68-71)src/platform/index.ts (1)
getPlatform(18-24)src/errors.ts (1)
eventCycleError(146-153)
src/__tests__/models/event/ListenerRegistry.test.ts (1)
src/models/event/ListenerRegistry.ts (2)
ListenerRegistry(14-142)createListener(144-152)
src/__tests__/models/event/CycleContext.test.ts (1)
src/models/event/CycleContext.ts (1)
CycleContext(5-58)
src/models/EventManager.ts (5)
src/models/event/types.ts (3)
IListenerStorage(17-25)EventEmissionInterceptor(42-45)HookExecutionInterceptor(50-54)src/models/event/ListenerRegistry.ts (2)
ListenerRegistry(14-142)createListener(144-152)src/models/event/CycleContext.ts (1)
CycleContext(5-58)src/models/event/EmissionExecutor.ts (2)
executeInParallel(26-98)executeSequentially(10-24)src/models/event/InterceptorPipeline.ts (1)
composeInterceptors(5-18)
src/models/event/EmissionExecutor.ts (2)
src/models/event/types.ts (1)
IListenerStorage(17-25)src/types/event.ts (1)
IEventEmission(63-97)
🔇 Additional comments (17)
src/__tests__/models/event/ListenerRegistry.test.ts (1)
36-47: LGTM!Good test for cache invalidation lifecycle - verifies the getter starts true, becomes false after adding a global listener, and returns to true after cache rebuild.
src/models/event/InterceptorPipeline.ts (1)
1-18: LGTM!Clean implementation of the middleware composition pattern. The
slice()beforereverse()correctly avoids mutating the input array, and the LIFO ordering is clearly documented.src/models/event/EmissionExecutor.ts (4)
52-66: Error augmentation logic is sound.The
as anycasting here is justified to dynamically addlistenerIdandlistenerOrderproperties for debugging purposes. The conditional checks prevent overwriting existing properties.
10-24: LGTM!Sequential execution correctly handles propagation stopping and listener filtering.
100-108: LGTM!Clean implementation that prevents self-invocation loops and respects optional filters.
82-97: No verification needed—listeners are guaranteed pre-sorted.The ListenerRegistry maintains a sorting invariant using binary insertion (see
insertListenermethod). Both event-specific and global listeners are kept in sorted order, andgetCachedMergedListenersmerges them while preserving order. The batching logic correctly relies on this guarantee.src/models/event/CycleContext.ts (2)
24-50: LGTM!The cycle detection logic is well-designed:
- Correctly identifies cycles by matching
frame.idin the emission stack- The
safeReEmitBySameHookcheck appropriately allows a hook to re-emit its own event without triggering a false cycle error- Stack immutability via spread operator prevents accidental mutations
52-57: LGTM!Clean implementation that correctly handles the disabled path and wraps hook execution with context tracking when enabled.
src/models/EventManager.ts (4)
1-23: LGTM: Clean modular imports.The imports are well-organized, use relative paths as required, and properly separate the new modular components (types, registry, executors, interceptors, cycle context) from existing dependencies.
136-144: LGTM: Parallel execution correctly integrated.The branching logic properly delegates to
executeInParallelfor parallel events andexecuteSequentiallyfor sequential events. The different parameter signatures are appropriate: parallel execution checksevent.isPropagationStopped()internally between order-based batches, while sequential execution receives the callback for per-listener checks.
167-209: LGTM: Proper listener registration delegation.Both
addListenerandaddGlobalListenercorrectly delegate to the registry, use thecreateListenerfactory for consistent listener creation, and perform lock checks before modifications.
310-313: LGTM: Proper type re-exports for API compatibility.Re-exporting the types from the event module maintains backward compatibility for consumers who import these types from EventManager.
src/models/event/types.ts (1)
1-71: LGTM: Well-structured type definitions.The type definitions are clean, well-documented, and properly organized. The separation into a dedicated types module improves maintainability and allows for easy reuse across the event system components.
Key strengths:
- Clear JSDoc comments for each type
- Consistent naming conventions
- Proper separation between internal (
IListenerStorage) and public (IEventHandlerOptions) types- Standard middleware pattern for interceptor types
src/models/event/ListenerRegistry.ts (4)
14-30: LGTM: Well-designed registry structure.The registry properly encapsulates listener storage with appropriate visibility modifiers. The constructor's optional
isExcludedFromGlobalparameter with a sensible default allows for testability while maintaining production behavior.
32-45: LGTM: Proper listener addition with cache invalidation.Both methods correctly handle listener insertion using binary insertion to maintain order, and properly invalidate caches. The distinction between event-specific cache invalidation (line 39) and global cache invalidation (line 44) is correct.
47-94: LGTM: Efficient listener retrieval with proper caching.The retrieval methods are well-optimized:
getListenersForEmitrespects global exclusion ruleshasListenersshort-circuits efficientlygetCachedMergedListenershandles edge cases (empty lists) before merging and properly manages cache invalidationThe caching strategy balances performance with correctness.
104-141: LGTM: Efficient sorting and insertion algorithms.The private helper methods implement optimal algorithms:
mergeSortedListeners: O(n) merge of two sorted arraysinsertListener: O(log n) binary search insertionBoth maintain the order invariant correctly and are well-implemented for performance.
| export function createListener(newListener: Partial<IListenerStorage>): IListenerStorage { | ||
| return { | ||
| handler: newListener.handler!, | ||
| order: newListener.order ?? HandlerOptionsDefaults.order, | ||
| filter: newListener.filter, | ||
| id: newListener.id, | ||
| isGlobal: newListener.isGlobal ?? false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Handler non-null assertion may be unsafe.
Line 146 uses a non-null assertion (newListener.handler!) but the type signature allows handler to be undefined. Consider making the handler required in the parameter type:
-export function createListener(newListener: Partial<IListenerStorage>): IListenerStorage {
+export function createListener(
+ newListener: Partial<IListenerStorage> & Pick<IListenerStorage, 'handler'>
+): IListenerStorage {
return {
- handler: newListener.handler!,
+ handler: newListener.handler,
order: newListener.order ?? HandlerOptionsDefaults.order,
filter: newListener.filter,
id: newListener.id,
isGlobal: newListener.isGlobal ?? false,
};
}This eliminates the need for the non-null assertion and makes the requirement explicit in the type system.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function createListener(newListener: Partial<IListenerStorage>): IListenerStorage { | |
| return { | |
| handler: newListener.handler!, | |
| order: newListener.order ?? HandlerOptionsDefaults.order, | |
| filter: newListener.filter, | |
| id: newListener.id, | |
| isGlobal: newListener.isGlobal ?? false, | |
| }; | |
| } | |
| export function createListener( | |
| newListener: Partial<IListenerStorage> & Pick<IListenerStorage, 'handler'> | |
| ): IListenerStorage { | |
| return { | |
| handler: newListener.handler, | |
| order: newListener.order ?? HandlerOptionsDefaults.order, | |
| filter: newListener.filter, | |
| id: newListener.id, | |
| isGlobal: newListener.isGlobal ?? false, | |
| }; | |
| } |
…d improve output formatting
- Updated `check-large-files.sh` to improve portability of awk commands. - Added backward compatibility test for `getCachedMergedListeners` in `EventManager`. - Introduced cycle detection tests in `CycleContext` to ensure proper handling of emission cycles. - Enhanced `ListenerRegistry` tests to utilize a more structured event creation approach. - Refactored `EventManager` to streamline listener retrieval logic. - Improved `CycleContext` constructor for better readability and efficiency. - Expanded `nodeExposure` tests to cover more edge cases and ensure proper error handling. - Modified `allowList` logic to return 403 for non-exposed tasks/events instead of 404. - Updated HTTP client interfaces to include error registry for better error handling. - Added comprehensive tests for error handling in HTTP smart and mixed clients.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/node/__tests__/exposure.resource.more-coverage.test.ts (1)
482-564: Wrap cleanup in try/finally to prevent test pollution.The
http.createServerrestoration at line 562 is not protected by a try/finally block. Ifrr.dispose()or earlier assertions throw, the mock won't be restored, potentially affecting subsequent tests.const rr = await run(app); - const handlers = await rr.getResourceValue(exposure.resource); - - expect(typeof capturedHandler).toBe("function"); - const req = createBaseReq(); - // ... rest of test ... - await rr.dispose(); - httpWithMutableCreate.createServer = realCreate; - expect(handlers.server).toBe(server); + try { + const handlers = await rr.getResourceValue(exposure.resource); + + expect(typeof capturedHandler).toBe("function"); + const req = createBaseReq(); + // ... rest of test assertions ... + await rr.dispose(); + expect(handlers.server).toBe(server); + } finally { + httpWithMutableCreate.createServer = realCreate; + }
🧹 Nitpick comments (11)
src/node/__tests__/http-smart-client.errors.test.ts (1)
16-29: Consider improving mock robustness.The
makeSinkmock stubsonandsetTimeoutto returnsinkfor chaining, but the stub foronignores all event types. If future code relies on specific event listeners (e.g.,'error'), tests may silently pass when they shouldn't.A more targeted mock could selectively handle events:
-function makeSink(): any { - const sink = new Writable({ - write(_c, _e, n) { - n(); - }, - final(n) { - n(); - }, - }) as any; - sink.on = (_: any, __: any) => sink; - sink.setTimeout = () => sink; - sink.destroy = () => undefined; - return sink; -} +function makeSink(): any { + const sink = new Writable({ + write(_c, _e, n) { + n(); + }, + final(n) { + n(); + }, + }) as any; + const originalOn = sink.on.bind(sink); + sink.on = (event: string, handler: any) => { + originalOn(event, handler); + return sink; + }; + sink.setTimeout = () => sink; + sink.destroy = () => undefined; + return sink; +}src/node/__tests__/exposure.resource.auto-discovery.test.ts (1)
15-43: Well-structured mock request factory.The helper correctly constructs a mock
IncomingMessagewith all required properties. One minor consideration:Using
new Socket()(line 29) creates a real socket instance. Consider using a minimal mock object instead to avoid any potential resource overhead in tests:- socket: new Socket(), + socket: {} as any,This is a nit since the socket isn't actually used in these tests, but it avoids creating real system resources.
src/node/__tests__/exposure.resource.more-coverage.test.ts (2)
12-18: Consider simplifying the MockRes type.The union type with intersection is somewhat convoluted:
type MockRes = ( | ServerResponse | (ServerResponse & { body?: Buffer | null }) ) & { body?: Buffer | null; };This simplifies to just adding the optional
bodyproperty toServerResponse:- type MockRes = ( - | ServerResponse - | (ServerResponse & { body?: Buffer | null }) - ) & { - body?: Buffer | null; - }; + type MockRes = ServerResponse & { body?: Buffer | null };
20-37: Potential for shared test utility.This
createBaseReqhelper is nearly identical tomakeReqinexposure.resource.auto-discovery.test.ts. Consider extracting to a shared test utility if more test files need similar mocks.This is a minor nit for test maintainability - acceptable to leave as-is if the tests are expected to evolve independently.
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.ts (2)
66-84: Consider extracting common mock setup into a helper function.The mock construction for
store,taskRunner,eventManager, andloggeris duplicated across all three tests. Extracting these into a helper function likecreateMockDeps()would reduce duplication and improve maintainability.For example:
function createBasicMocks() { return { store: { tasks: new Map(), events: new Map(), asyncContexts: new Map(), resources: new Map(), } as unknown as NodeExposureDeps["store"], taskRunner: { async run() { return 1; }, } as unknown as NodeExposureDeps["taskRunner"], eventManager: { emit: async () => {}, } as unknown as NodeExposureDeps["eventManager"], logger: { info: async () => {}, warn: async () => {}, error: async () => {}, } as unknown as NodeExposureDeps["logger"], }; }Also applies to: 115-133, 158-190
222-228: Consider avoiding router mutation for clearer test structure.Mutating
deps.router.extractafter creating the handlers (line 225) works but makes the test logic harder to follow. A cleaner approach would be to create a fresh set of deps for the event test or use a router that can handle both task and event paths.For example:
// Event request blocked by allow-list const eventReq = makeJsonReq("/api/event/blocked.event"); const eventRes = makeRes(); const eventDeps = { ...deps, router: makeRouter((_p: string) => ({ kind: "event", id: "blocked.event", })), }; const { handleEvent: handleEventForBlocked } = createRequestHandlers(eventDeps); await handleEventForBlocked(eventReq, eventRes);src/models/event/CycleContext.ts (1)
23-49: Clarify thesourceinvariant and consider simplifying the APIThe core cycle detection logic and stack handling look correct: you detect prior frames by
id, build a path from the first occurrence, and throw a structuredeventCycleErrorwith the full chain. ThesafeReEmitBySameHookguard is a subtle but reasonable exception, but it currently relies on an important invariant: thesourceargument passed intorunEmissionmust match thehookIdstored viarunHookwhenever the emission originates from that hook.If any future call sites pass a
framewhosesourcediffers from the separatesourceparameter, this guard could behave unexpectedly. You can make this safer and easier to reason about by deriving the comparison purely fromframe:- runEmission( - frame: IEmissionFrame, - source: string, - processEmission: () => Promise<void>, - ): Promise<void> { + runEmission( + frame: IEmissionFrame, + processEmission: () => Promise<void>, + ): Promise<void> { @@ - const safeReEmitBySameHook = - top.id === frame.id && currentHookId && currentHookId === source; + const safeReEmitBySameHook = + top.id === frame.id && + currentHookId && + currentHookId === frame.source; @@ - const nextStack = currentStack ? [...currentStack, frame] : [frame]; - return this.emissionStack.run(nextStack, processEmission); + const nextStack = currentStack ? [...currentStack, frame] : [frame]; + return this.emissionStack.run(nextStack, processEmission); }and updating the single caller in
EventManager.emitaccordingly:- const frame = { id: eventDefinition.id, source }; + const frame = { id: eventDefinition.id, source }; @@ - await this.cycleContext.runEmission(frame, source, processEmission); + await this.cycleContext.runEmission(frame, processEmission);This removes an extra parameter, encodes the invariant in a single place (
frame.source), and reduces the risk of accidental mismatch.src/models/EventManager.ts (4)
30-41: Registry and cycle-context wiring look correct; aliasing assumption should be kept stableInitializing
ListenerRegistryandCycleContextin the constructor and then aliasinglisteners,globalListeners, andcachedMergedListenersto the registry’s collections is a good way to preserve test back-compat while moving logic into the registry.This does rely on
ListenerRegistrykeeping those collections as long-lived instances (mutating in place rather than reassigning the fields). As long as that holds, tests that reach intoEventManager.listeners/globalListenerswill continue to see up‑to‑date data; it’s worth documenting this contract inListenerRegistryto avoid accidental refactors that break the aliasing.Also applies to: 48-57
102-149: Emit flow refactor is sound; document propagation semantics for parallel eventsThe new emission path looks good overall:
- Listener resolution is delegated to
this.registry.getListenersForEmit(eventDefinition).- The
IEventEmissionobject is built once and passed throughcomposeInterceptors.- Sequential vs parallel listener execution is chosen via
eventDefinition.parallel, usingexecuteSequentiallyvsexecuteInParallel.One behavioral nuance worth calling out: in the
parallelbranch you no longer passisPropagationStoppedinto the executor, soevent.stopPropagation()cannot prevent other listeners from running once dispatch has begun. That’s likely the only sensible behavior for fully parallel dispatch, but it’s a change from purely sequential semantics and is easy to miss by users.I recommend making this explicit in event/parallel documentation and tests (e.g., asserting that
stopPropagation()is a no‑op whenparallel: true) so the behavior is clearly intentional and guarded against regressions.
160-180: Listener registration via ListenerRegistry is correct; minor clean-up possible for array caseCreating listeners through
createListenerand delegating registration tothis.registry.addListeneris the right direction and keeps listener shape internal to the event subsystem.One minor tidy-up: when
eventis an array, you computenewListenerbut don’t use it in the outer call, since you delegate tothis.addListenerfor each element. You can avoid that extracreateListenercall by early‑returning on the array branch:addListener<T>( event: IEvent<T> | Array<IEvent<T>>, handler: EventHandlerType<T>, options: IEventHandlerOptions<T> = HandlerOptionsDefaults, ): void { this.checkLock(); - const newListener = createListener({ - handler, - order: options.order, - filter: options.filter, - id: options.id, - isGlobal: false, - }); - - if (Array.isArray(event)) { - event.forEach((id) => this.addListener(id, handler, options)); - } else { - const eventId = event.id; - this.registry.addListener(eventId, newListener); - } + if (Array.isArray(event)) { + event.forEach((single) => this.addListener(single, handler, options)); + return; + } + + const newListener = createListener({ + handler, + order: options.order, + filter: options.filter, + id: options.id, + isGlobal: false, + }); + + this.registry.addListener(event.id, newListener); }Purely a small efficiency/readability win; behavior stays the same.
269-279: Hook execution pipeline is correct; outerisEnabledcheck is redundantComposing hook interceptors with
composeInterceptorsand then optionally executing withincycleContext.runHookis the right place to integrate hook context with cycle detection.Given that
CycleContext.runHookalready checksisEnabledand falls back to executing the callback directly, the ternary here is effectively duplicating that check. You can simplify without changing behavior:- return this.cycleContext.isEnabled - ? await this.cycleContext.runHook(hook.id, () => - executeWithInterceptors(hook, event), - ) - : await executeWithInterceptors(hook, event); + return this.cycleContext.runHook(hook.id, () => + executeWithInterceptors(hook, event), + );This centralizes the enable/disable logic inside
CycleContextand slightly reduces branching here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
scripts/check-large-files.sh(2 hunks)src/__tests__/models/EventManager.test.ts(2 hunks)src/__tests__/models/event/CycleContext.test.ts(1 hunks)src/__tests__/models/event/ListenerRegistry.test.ts(1 hunks)src/models/EventManager.ts(9 hunks)src/models/event/CycleContext.ts(1 hunks)src/node/__tests__/exposure.resource.auto-discovery.test.ts(2 hunks)src/node/__tests__/exposure.resource.more-coverage.test.ts(16 hunks)src/node/__tests__/http-smart-client.errors.test.ts(1 hunks)src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.ts(2 hunks)src/node/exposure/allowList.ts(2 hunks)src/node/http-mixed-client.ts(3 hunks)src/node/http-smart-client.model.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/tests/models/event/ListenerRegistry.test.ts
- src/tests/models/event/CycleContext.test.ts
- src/tests/models/EventManager.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Format code with Prettier for all TypeScript source files (npx prettier --write src)
Check code formatting with Prettier for all TypeScript source files (npx prettier --check src)
Files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/models/event/CycleContext.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/http-mixed-client.tssrc/node/exposure/allowList.tssrc/models/EventManager.tssrc/node/http-smart-client.model.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Type-strictness is a must; avoid type casting unless absolutely necessary
Files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/models/event/CycleContext.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/http-mixed-client.tssrc/node/exposure/allowList.tssrc/models/EventManager.tssrc/node/http-smart-client.model.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,mjs,cjs}: Use only relative imports
Place imports at the top of the file by default; inline imports only for specific optimizations
Files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/models/event/CycleContext.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/http-mixed-client.tssrc/node/exposure/allowList.tssrc/models/EventManager.tssrc/node/http-smart-client.model.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
src/node/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Node-specific code under ./src/node/
Files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/http-mixed-client.tssrc/node/exposure/allowList.tssrc/node/http-smart-client.model.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
🧠 Learnings (5)
📚 Learning: 2025-09-12T05:57:37.217Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: examples/fastify-mikroorm/AGENTS.md:0-0
Timestamp: 2025-09-12T05:57:37.217Z
Learning: Applies to examples/fastify-mikroorm/src/**/*.test.ts : Ensure tests cover validation errors (400), auth paths (401/403), happy paths (200), and HTTPError mapping
Applied to files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
📚 Learning: 2025-09-12T05:57:37.217Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: examples/fastify-mikroorm/AGENTS.md:0-0
Timestamp: 2025-09-12T05:57:37.217Z
Learning: Applies to examples/fastify-mikroorm/src/**/*.test.ts : In HTTP tests, register httpRoute, fastify, and fastifyRouter with buildTestRunner
Applied to files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
📚 Learning: 2025-09-12T05:57:37.217Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: examples/fastify-mikroorm/AGENTS.md:0-0
Timestamp: 2025-09-12T05:57:37.217Z
Learning: Applies to examples/fastify-mikroorm/src/**/tasks/*.task.ts : Tag all HTTP-exposed tasks with httpRoute and configure method, path, auth, and inputFrom as needed
Applied to files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
📚 Learning: 2025-09-12T05:57:37.217Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: examples/fastify-mikroorm/AGENTS.md:0-0
Timestamp: 2025-09-12T05:57:37.217Z
Learning: Applies to examples/fastify-mikroorm/src/**/tasks/*.task.ts : Set httpRoute auth to public | optional | required appropriately for each route
Applied to files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.ts
📚 Learning: 2025-09-12T05:57:37.217Z
Learnt from: CR
Repo: bluelibs/runner PR: 0
File: examples/fastify-mikroorm/AGENTS.md:0-0
Timestamp: 2025-09-12T05:57:37.217Z
Learning: Applies to examples/fastify-mikroorm/src/**/tasks/*.task.ts : Throw HTTPError for expected HTTP failures (e.g., new HTTPError(status, message)) from tasks
Applied to files:
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.tssrc/node/__tests__/exposure.resource.auto-discovery.test.tssrc/node/__tests__/http-smart-client.errors.test.tssrc/node/__tests__/exposure.resource.more-coverage.test.ts
🧬 Code graph analysis (4)
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.ts (3)
src/globals/resources/tunnel/serializer.ts (1)
getDefaultSerializer(27-29)src/globals/globalTags.ts (1)
globalTags(6-30)src/node/exposure/allowList.ts (1)
createAllowListGuard(8-40)
src/models/event/CycleContext.ts (3)
src/models/event/types.ts (1)
IEmissionFrame(68-71)src/platform/index.ts (1)
getPlatform(18-24)src/errors.ts (1)
eventCycleError(146-153)
src/node/__tests__/http-smart-client.errors.test.ts (4)
src/node/http-mixed-client.ts (2)
Readable(109-109)createHttpMixedClient(63-107)src/node/http-smart-client.model.ts (2)
Readable(425-425)createHttpSmartClient(358-423)src/globals/globalResources.ts (1)
serializer(25-33)src/globals/resources/tunnel/serializer.ts (1)
getDefaultSerializer(27-29)
src/node/exposure/allowList.ts (2)
src/node/exposure/httpResponse.ts (1)
jsonErrorResponse(26-42)src/definers/defineError.ts (1)
id(26-28)
🔇 Additional comments (26)
scripts/check-large-files.sh (2)
45-52: ✓ Token estimation and relative path storage implemented correctly.The token approximation formula
(chars + 3) / 4is a standard GPT-style heuristic, and the arithmetic is sound (integer division naturally rounds up). The transition to storing relative paths and restructuring results as"count chars tokens rel_path"is clean and maintains compatibility with the null-delimited find pipeline. Parameter expansion via${file#"$ROOT_DIR/"}is the idiomatic bash approach.
61-83: ✓ Terminal styling and output formatting well-implemented.The ANSI escape codes for bold (
\033[1m) and reset (\033[0m) are standard and portable. Passing them to awk via-vis the correct approach for cross-platform compatibility. The awk formatting logic correctly:
- Reconstructs the path from fields 4+ to preserve spaces in file paths
- Uses aligned printf format specifiers for readable columnar output
- Generates a bold, dashed header
One minor note: ensure printf format widths (
%-16s,%-12s) remain adequate as the codebase evolves (e.g., if files exceed 9,999 lines or 999,999 characters). If needed, these can be made dynamic via awk variables, but current defaults appear reasonable for typical codebases.src/node/http-mixed-client.ts (1)
6-6: LGTM! Error registry correctly propagated to both client paths.The
errorRegistryis properly typed usingIErrorHelper<any>and consistently passed to bothfetchClientandsmartClient, ensuring typed error handling works across all HTTP client paths.Also applies to: 26-26, 87-87
src/node/http-smart-client.model.ts (4)
347-356: Well-structured error transformation helper.The
rethrowTypedfunction correctly uses return typeneverand provides a clean pattern for registry-based error transformation. The conditional check forregistry && error && id && dataensures safe access before attempting typed rethrow.
370-374: Verify: Streaming duplex path lacks typed error handling.The octet-stream path bypasses
rethrowTyped, unlike the multipart and JSON paths. If this is intentional (raw streams don't return JSON error envelopes), consider adding a brief comment. Otherwise, wrap in try/catch for consistency.
382-410: LGTM! Error handling correctly integrated into multipart and JSON paths.Both paths now properly wrap network calls in try/catch and route errors through
rethrowTyped, enabling typed error propagation when the error envelope contains a matchingidin the registry.
415-420: LGTM! Event path error handling is consistent.The event path mirrors the task paths with proper try/catch wrapping and typed error rethrow.
src/node/__tests__/http-smart-client.errors.test.ts (4)
31-45: Good test setup with proper cleanup.The shared
helpermock correctly implements theIErrorHelperinterface pattern, andafterEachensures mocks are restored between tests, preventing test pollution.
47-76: LGTM! JSON path error handling test is comprehensive.The test correctly mocks
http.requestto return an error envelope and verifies that the client rethrows typed errors via the error registry. Using regex/typed:7/for assertion is appropriate.
78-112: LGTM! Smart path test validates multipart error handling.The test correctly uses
createNodeFileto trigger the multipart code path via the mixed client, verifying that typed errors flow through even for file uploads.
114-143: LGTM! Event path error handling test completes the coverage.The test verifies that event calls also benefit from the typed error registry, ensuring consistent error handling across all client operations.
src/node/exposure/allowList.ts (1)
24-27: LGTM! Correct semantic choice for access control.Changing from 404 to 403 is the right HTTP semantics here. A 403 FORBIDDEN correctly indicates that the resource exists but access is denied, whereas 404 would imply the task/event doesn't exist at all. This prevents information leakage about non-allowlisted resources while being semantically accurate.
Also applies to: 34-37
src/node/__tests__/exposure.resource.auto-discovery.test.ts (2)
45-59: LGTM!Minimal and sufficient mock for these test scenarios. The chaining support via return values is correctly implemented.
98-134: LGTM! Clean test structure with correct assertions.The removal of type casting on
exposure.resource(line 98) and the updated 403 expectation (line 121) correctly align with the improved typing and access control semantics. Good coverage of allowed/not-allowed scenarios for both tasks and events.src/node/__tests__/exposure.resource.more-coverage.test.ts (2)
340-365: LGTM! Well-structured server mock.Good use of Jest spy with proper
finallyblock cleanup. The typed listener map and minimal Server interface are appropriate for the test scenario.
437-448: LGTM! Assertions correctly updated.The 403 expectations and descriptive comments correctly align with the access control semantics change in
allowList.ts. Good coverage for both tasks and events in the not-allowlisted scenarios.Also applies to: 463-477
src/node/exposure/__tests__/requestHandlers.auth-and-missing.unit.test.ts (3)
1-12: LGTM!The imports are well-organized and follow the coding guidelines. The type definitions for
MockReqandMockResclearly document the mock structure.
14-62: LGTM!The mock helper functions effectively standardize test infrastructure. The implementations provide sufficient mock behavior for the request handlers under test.
157-235: LGTM! Good coverage of allow-list blocking scenarios.The test correctly verifies that both tasks and events return 403 FORBIDDEN when blocked by the allow-list. The assertions check both status codes and error codes, providing comprehensive coverage. Based on learnings, HTTP tests should cover auth paths including 403, which this test does.
src/models/event/CycleContext.ts (2)
5-21: Platform-aware enabling/disablement looks solidThe constructor correctly gates cycle detection on both
runtimeCycleDetectionandplatform.hasAsyncLocalStorage(), and falls back cleanly to a disabled state with null storages andisEnabled = false. This keeps the rest of the API simple (runEmission/runHookcan just checkisEnabled) and avoids partial initialization states.
51-56: Hook context scoping is minimal and correct
runHook’s contract is clear and small: it either executes the callback directly when disabled or wraps it incurrentHookIdContext.runwhen enabled. This matches howEventManageruses it and keeps hook-related context management well encapsulated.src/models/EventManager.ts (5)
7-22: Imports and modularization align with guidelinesThe new imports cleanly separate concerns into
./event/*modules and maintain relative paths only, matching the repo’s guidelines. Pulling shared types and executors out ofEventManageralso makes the file significantly easier to navigate.
189-202: Global listener registration is consistent with per-event listenersUsing
createListenerwithisGlobal: trueand delegating tothis.registry.addGlobalListeneris consistent with the per-event path and keeps all ordering/filter logic centralized in the registry. No issues here.
210-212: Listener existence check is correctly delegated
hasListenersnow simply forwards tothis.registry.hasListeners(eventDefinition), which keeps the querying logic in one place alongside the storage. This is a straightforward and correct delegation.
295-299: Cached merged listeners compatibility shim is appropriateKeeping
getCachedMergedListenersas a thin passthrough tothis.registry.getCachedMergedListenersis a good way to preserve existing test hooks while migrating the actual caching logic into the registry.
302-304: Type re-exports preserve public API surfaceRe-exporting
IEventHandlerOptions,EventEmissionInterceptor, andHookExecutionInterceptorfromEventManagermaintains compatibility for external consumers while allowing their definitions to live in./event/types. This is a clean way to evolve internals without forcing breaking changes.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Behavior
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.