feat: tag can impact return contract of task or resource#44
feat: tag can impact return contract of task or resource#44theodorDiaconu merged 9 commits intomainfrom
Conversation
…nse validation Updated the `HasContracts` and `EnsureResponseSatisfiesContracts` types to streamline contract enforcement logic. Introduced a new `ContractViolationError` type for clearer compile-time error reporting. Enhanced the `meta` structure in tests to validate the updated contract handling. This refactor improves type safety and clarity in contract enforcement across the codebase.
…proved type parameters Updated the `defineResource` function to include a new generic type parameter for metadata, enhancing type safety and flexibility. Adjusted the `IResourceDefinition` and `IResource` interfaces to accommodate the new metadata type, ensuring consistent contract validation. Improved the handling of response types in contract enforcement, leading to clearer error reporting and better type inference across the codebase.
…resource definitions Expanded the README to include a new section on contract tags, detailing how to enforce return types for tasks and resources. Updated type definitions in `define.ts` and `defs.ts` to improve type safety for resource values. Added tests to validate contract enforcement on resources, ensuring clearer error reporting for contract violations.
WalkthroughIntroduces tag-based contract types and metadata generics across tags, tasks, and resources. Enforces Promise-based resource values and adjusts types to unwrap values in run(). Adds a new defs.returnTag module with compile-time utilities, updates public type signatures, adapts tests, and expands documentation and quick reference. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Updated type definitions across multiple files to improve type safety for resource values, ensuring that resource initialization and execution handle promises correctly. Adjusted interfaces and function signatures in `defs.ts`, `run.ts`, `ResourceInitializer.ts`, and `Store.ts` to reflect these changes, enhancing the overall clarity and robustness of the codebase.
…and contract validation Enhanced type definitions in `ResourceInitializer.ts` and `defs.ts` to ensure better handling of resource initialization and contract validation. Updated interfaces and function signatures to reflect these changes, promoting clearer error reporting and improved type safety across the codebase.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
README.md (2)
290-297: Typo in example: useglobals.eventsinstead ofglobalEvents.The correct symbol exported by the library is
globals, notglobalEvents.-const taskLogger = task({ - id: "app.logging.taskLogger", - on: globalEvents.tasks.beforeRun, +const taskLogger = task({ + id: "app.logging.taskLogger", + on: globals.tasks.beforeRun, run(event) { console.log(`Running task: ${event.source} with input:`, event.data.input); }, });
1274-1303: Fix missing fenced code block (MD040) and specify language.The
rateLimitMiddlewaresnippet isn’t fenced, and the linter flags a missing language on fenced blocks. Add atypescriptfence around the snippet.-const rateLimitMiddleware = middleware({ - id: "app.middleware.rateLimit", - dependencies: { redis }, - run: async ({ task, next }, { redis }) => { - // Extraction can be done at task.definition level or at task.definition.meta.tags - const rateLimitCurrentTag = rateLimitTag.extract(task.definition); - // Alternative way - const tags = task.definition.meta?.tags; - const rateLimitCurrentTag = rateLimitTag.extract(tags); - if (rateLimitCurrentTag) { - const key = `rateLimit:${task.definition.id}`; - const current = await redis.incr(key); - if (current === 1) { - await redis.expire(key, 60); // 1 minute window - } - if (current > rateLimitCurrentTag.config.maxRequestsPerMinute) { - throw new Error("Rate limit exceeded"); - } - } - return next(task.input); - }, -}); +```typescript +const rateLimitMiddleware = middleware({ + id: "app.middleware.rateLimit", + dependencies: { redis }, + run: async ({ task, next }, { redis }) => { + // Extraction can be done at task.definition level or at task.definition.meta.tags + const rateLimitCurrentTag = rateLimitTag.extract(task.definition); + + // Alternative way + const tags = task.definition.meta?.tags; + const rateLimitTagFromArray = rateLimitTag.extract(tags); + + const currentTag = rateLimitCurrentTag ?? rateLimitTagFromArray; + if (currentTag) { + const key = `rateLimit:${String(task.definition.id)}`; + const current = await redis.incr(key); + if (current === 1) { + await redis.expire(key, 60); // 1 minute window + } + if (current > currentTag.config.maxRequestsPerMinute) { + throw new Error("Rate limit exceeded"); + } + } + return next(task.input); + }, +}); +```Note: Also avoid variable shadowing by not redeclaring
rateLimitCurrentTag. The patch introducesrateLimitTagFromArrayand unifies viacurrentTag.src/define.ts (1)
358-384: defineTag not fully updated for TEnforceContract; also missing [symbolFilePath]Issues:
- The return assertion uses ITag, dropping the TEnforceContract generic.
- with() returns ITagWithConfig instead of ITagWithConfig<TConfig, TEnforceContract>.
- extract() returns/casts to ITagWithConfig but the method signature returns ExtractedTagResult<TConfig, TEnforceContract>. Also contract generic is dropped.
- ITag requires [symbolFilePath]: string, but the returned object lacks it (unlike other factories).
Proposed fix:
export function defineTag<TConfig = void, TEnforceContract = void>( definition: ITagDefinition<TConfig, TEnforceContract> ): ITag<TConfig, TEnforceContract> { const id = definition.id; return { + [symbolFilePath]: getCallerFile(), id, with(tagConfig: TConfig) { return { id, tag: this, config: tagConfig as any, - } as ITagWithConfig<TConfig>; + } as ITagWithConfig<TConfig, TEnforceContract>; }, extract(target: TagType[] | ITaggable) { const tags = Array.isArray(target) ? target : target?.meta?.tags || []; for (const candidate of tags) { if (typeof candidate === "string") continue; - // Configured instance - if (candidate.id === id) { - return candidate as ITagWithConfig<TConfig>; - } + if ((candidate as any).id === id) { + // Return the minimal shape to match ExtractedTagResult<TConfig, TEnforceContract> + const c: any = candidate; + if ("config" in c) { + return { id: c.id, config: c.config } as ExtractedTagResult< + TConfig, + TEnforceContract + >; + } + // Bare tag (allowed only for void or optional config tags) + return { id: c.id } as ExtractedTagResult<TConfig, TEnforceContract>; + } } return null; }, - } as ITag<TConfig>; + } as ITag<TConfig, TEnforceContract>; }
🧹 Nitpick comments (12)
BLUELIBS_RUNNER_QUICKREAD.md (1)
118-126: Avoid shadowing helper names in examples (task vs const task).Using
const task = task({ ... })shadows the importedtaskhelper and can confuse readers or break copy/paste. Prefer a descriptive variable name.Apply this diff to the snippet:
-const task = task({ +const getUserIdTask = task({ middleware: [UserContext.require()], run: async () => { const user = UserContext.use(); // Available anywhere return user.userId; }, }); -// Provide context UserContext.provide({ userId: "123" }, async () => { - return await task(); + return await getUserIdTask(); });src/__tests__/typesafety.test.ts (5)
64-67: Duplicate event IDs can be misleading in examples.Both events use
id: "event". Even in type-only tests, prefer unique IDs to avoid confusion and accidental collisions if reused.-const event = defineEvent<{ message: string }>({ - id: "event", -}); - -const eventWithoutArguments = defineEvent({ - id: "event", -}); +const event = defineEvent<{ message: string }>({ + id: "types.events.message", +}); + +const eventWithoutArguments = defineEvent({ + id: "types.events.noArgs", +});
68-76: Task ID collision in tests (two tasks with id "task").
baseTaskandtaskboth useid: "task". While these aren’t registered together here, using unique, convention-aligned IDs improves clarity.-const baseTask = defineTask({ - id: "task", +const baseTask = defineTask({ + id: "types.tasks.base", run: async (input: InputTask) => "Task executed", }); -const task = defineTask({ - id: "task", +const task = defineTask({ + id: "types.tasks.main", dependencies: { baseTask, event },
192-205: Variable nametestResourceis a task; also duplicate task ID.Nit: rename for clarity and align IDs to the conventions.
-const testResource = defineTask({ - id: "test.resource", +const testListenerTask = defineTask({ + id: "types.tasks.testListener", dependencies: { task }, on: hookEvent, run: async (_, deps) => {And for the wildcard one:
-const testResource2 = defineTask({ - id: "test.resource", +const testWildcardListenerTask = defineTask({ + id: "types.tasks.testListenerWildcard",
329-381: Contract enforcement on tasks: good coverage; consider unique IDs.The positive and negative cases are well constructed and will surface the intended compile-time diagnostics. Minor nit: use distinct IDs per definition.
-const task = defineTask({ - id: "task", +const task = defineTask({ + id: "types.tasks.contract.ok", meta, run: async (input: { name: string }) => { return { age: 123, name: "123", }; }, }); const task2 = defineTask({ - id: "task", + id: "types.tasks.contract.bad1", meta, // @ts-expect-error run: async (input: { name: string }) => { return { age: "123", }; }, }); const task3 = defineTask({ - id: "task", + id: "types.tasks.contract.bad2",
383-430: Contract enforcement on resources: solid; minor naming nits.The valid and invalid shapes are clear. Consider aligning IDs to conventions.
-const resourceOk = defineResource({ - id: "resource.ok", +const resourceOk = defineResource({ + id: "types.resources.contract.ok",-const resourceBad1 = defineResource({ - id: "resource.bad1", +const resourceBad1 = defineResource({ + id: "types.resources.contract.bad1",-const resourceBad2 = defineResource({ - id: "resource.bad2", +const resourceBad2 = defineResource({ + id: "types.resources.contract.bad2",src/t1.ts (1)
35-43: Incorrect comment: InvalidResponse is notnever.
EnsureResponseSatisfiesContractsyields aContractViolationError<...>for violations, notnever. Fix the comment to prevent confusion.type InvalidResponse = EnsureResponseSatisfiesContracts< typeof meta, { name: string } ->; // never +>; // ContractViolationError<typeof meta, { name: string }>src/defs.returnTag.ts (3)
3-5: Minor import cleanup.Consolidate imports from the same module for brevity.
-import { ITag, ITagWithConfig } from "./defs"; -import { IMeta } from "./defs"; +import { ITag, ITagWithConfig, IMeta } from "./defs";
40-44: Simplify HasContracts to avoid array-shape edge cases.Leverage the existing
ContractsUnionutility to determine emptiness; this avoids tuple vs array nuances.-export type HasContracts<T extends IMeta> = - ExtractTagsWithNonVoidReturnTypeFromMeta<T> extends never[] ? false : true; // HasContracts and enforcement +export type HasContracts<T extends IMeta> = + [ContractsUnion<T>] extends [never] ? false : true;
40-41: Remove unused typeIsNeverTuple.This is unused and can be safely removed to reduce noise.
-type IsNeverTuple<T extends readonly unknown[]> = T extends [] ? true : false;src/define.ts (2)
104-119: Rename TPrivate to TContext for consistency with defs.tsYou’re passing the 4th generic to IResourceDefinition as the “context” type. Naming it TPrivate here can confuse readers since the corresponding type in defs.ts is TContext. This is a readability/consistency improvement; no runtime impact.
Suggested change:
-export function defineResource< - TConfig = void, - TValue extends Promise<any> = Promise<any>, - TDeps extends DependencyMapType = {}, - TPrivate = any, - TMeta extends IResourceMeta = any ->( - constConfig: IResourceDefinition< - TConfig, - TValue, - TDeps, - TPrivate, - any, - any, - TMeta - > -): IResource<TConfig, TValue, TDeps, TPrivate, TMeta> { +export function defineResource< + TConfig = void, + TValue extends Promise<any> = Promise<any>, + TDeps extends DependencyMapType = {}, + TContext = any, + TMeta extends IResourceMeta = any +>( + constConfig: IResourceDefinition< + TConfig, + TValue, + TDeps, + TContext, + any, + any, + TMeta + > +): IResource<TConfig, TValue, TDeps, TContext, TMeta> {
72-96: Use the same filePath for all task event brandsYou compute filePath at Line 59; however, in task events you call getCallerFile() again for each event brand. Use the same filePath for consistency with resources and to avoid accidental discrepancies.
Suggested change:
- [symbolFilePath]: getCallerFile(), + [symbolFilePath]: filePath, ... - [symbolFilePath]: getCallerFile(), + [symbolFilePath]: filePath, ... - [symbolFilePath]: getCallerFile(), + [symbolFilePath]: filePath,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
BLUELIBS_RUNNER_QUICKREAD.md(1 hunks)README.md(3 hunks)src/__tests__/typesafety.test.ts(2 hunks)src/define.ts(6 hunks)src/defs.returnTag.ts(1 hunks)src/defs.ts(14 hunks)src/t1.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: Use.with(config)to configure resources and middleware
Use theindex()helper to group dependencies
Use the context system for request-scoped data viacreateContext()
Use built-in caching middleware withglobals.middleware.cache
Use global events for lifecycle hooks (e.g.,globals.tasks.beforeRun)
Handle errors with suppression capability viaevent.data.suppress()
Use the override system for replacing components
Use built-in logging with levels (trace, debug, info, warn, error, critical)
Tasks should be named using the convention{domain}.tasks.{taskName}(e.g., "app.tasks.createUser")
Resources should be named using the convention{domain}.resources.{resourceName}(e.g., "app.resources.database")
Events should be named using the convention{domain}.events.{eventName}(e.g., "app.events.userRegistered")
Middleware should be named using the convention{domain}.middleware.{middlewareName}(e.g., "app.middleware.auth")
Files:
src/t1.tssrc/defs.returnTag.tssrc/__tests__/typesafety.test.tssrc/define.tssrc/defs.ts
src/define.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core definition functions (task, resource, event, middleware, index) in
src/define.ts
Files:
src/define.ts
src/defs.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place all TypeScript interfaces and type definitions in
src/defs.ts
Files:
src/defs.ts
🧠 Learnings (3)
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/define.ts : Place core definition functions (task, resource, event, middleware, index) in `src/define.ts`
Applied to files:
src/__tests__/typesafety.test.tssrc/define.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/globals/globalMiddleware.ts : Place built-in global middleware exports in `src/globals/globalMiddleware.ts`
Applied to files:
src/defs.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/globals/middleware/cache.middleware.ts : Place built-in caching middleware with LRU support in `src/globals/middleware/cache.middleware.ts`
Applied to files:
src/defs.ts
🧬 Code Graph Analysis (5)
src/t1.ts (3)
src/define.ts (1)
defineTag(358-384)src/defs.ts (1)
IMeta(133-137)src/defs.returnTag.ts (4)
ExtractContractsFromTags(28-31)ExtractTagsWithNonVoidReturnTypeFromMeta(33-38)HasContracts(42-43)EnsureResponseSatisfiesContracts(79-91)
src/defs.returnTag.ts (1)
src/defs.ts (3)
ITagWithConfig(71-77)ITag(83-97)IMeta(133-137)
src/__tests__/typesafety.test.ts (4)
src/define.ts (3)
defineTag(358-384)defineTask(44-100)defineResource(102-182)src/index.ts (3)
defineTag(30-30)defineTask(25-25)defineResource(26-26)src/defs.ts (1)
IMeta(133-137)src/defs.returnTag.ts (2)
HasContracts(42-43)EnsureResponseSatisfiesContracts(79-91)
src/define.ts (2)
src/defs.ts (11)
IEventDefinition(438-442)ITaskMeta(139-139)ITaskDefinition(220-262)ITask(294-315)DependencyMapType(149-152)IResourceMeta(140-140)IResourceDefinition(317-384)IResource(386-419)DependencyValuesType(196-198)ITagDefinition(64-66)ITag(83-97)src/index.ts (2)
defineResource(26-26)defineTag(30-30)
src/defs.ts (1)
src/defs.returnTag.ts (2)
HasContracts(42-43)EnsureResponseSatisfiesContracts(79-91)
🪛 markdownlint-cli2 (0.17.2)
README.md
1303-1303: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (25)
BLUELIBS_RUNNER_QUICKREAD.md (2)
137-145: Good, demonstrates cache.with() and custom keyBuilder clearly.This example aligns with the guidelines (using
.with(config)and showing a realistic key builder). No action needed.
217-229: Solid production monitoring pattern.Uses
globals.tasks.beforeRun, demonstrates conditional alerting, and reads as actionable. Nice.src/__tests__/typesafety.test.ts (3)
11-11: Importing IMeta to type meta is correct.This ensures
metaobjects are constrained to the tag structure under test. Good.
19-22: Imports for contract utilities are appropriate and minimal.These cover presence checks and enforcement; no unnecessary imports.
300-324: Adding run to the tag types test is fine.This keeps the example complete and compatible with current typings. Looks good.
README.md (1)
1207-1268: Contract Tags documentation is clear and accurate.Examples correctly show intersection of contract types and how violations surface a readable error type. Nice work.
src/defs.returnTag.ts (1)
79-91: Enforcement logic reads well and covers Promise/non-Promise cases.This is the right place to gate return types based on meta-derived contract intersections. Looks good.
src/define.ts (4)
36-38: Imports align with new meta typesImporting ITaskMeta and IResourceMeta is correct and matches the updated public types in defs.ts.
97-98: Sane default for task metaDefaulting meta to an empty object is OK and consistent with ITaskMeta.
179-181: Sane default for resource metaDefaulting meta to an empty object is OK and consistent with IResourceMeta.
197-198: defineIndex return type updated to Promise — correctThe Promise-based TValue matches the async init and the updated ExtractResourceValue semantics. This is consistent with defs.ts and improves type accuracy for consumers of index resources.
src/defs.ts (14)
18-21: Contract-enforcement imports wired correctlyImporting EnsureResponseSatisfiesContracts and HasContracts here is correct and they’re applied where needed for tasks/resources.
64-67: Tags now carry contract generic — goodITagDefinition<TConfig, TEnforceContract> addition is consistent with the PR objective to propagate contract metadata via tags.
71-77: Configured tag type expanded correctlyIncluding TEnforceContract in ITagWithConfig and in the tag field type preserves contract info through configured instances.
83-97: ITag updated with contract generic and typed extract — good
- with returns ITagWithConfig<TConfig, TEnforceContract>
- extract returns ExtractedTagResult<TConfig, TEnforceContract> | null
This matches the intended API and type-level contract flow.
104-109: TagType union updated appropriatelyRestricting bare tags to void/optional-config tags and requiring configured instances otherwise is a solid ergonomic constraint.
116-119: ExtractedTagResult generic carries contract typeEven if TEnforceContract isn’t used in the structure, keeping it in the type helps preserve the parameter for inference across extract chains.
154-161: Resource value extractor now unwraps Promise — correctThis brings DependencyValueType for resources in line with the new Promise-bound TValue and remove ambiguity for consumers.
188-195: DependencyValueType uses unwrapped resource value — correctUsing ExtractResourceValue ensures resource dependencies are the resolved values in task/resource bodies.
220-262: Task meta and contract-aware run return — correct
- TOn constraint and meta type align with the new model.
- Conditional run return enforces contract satisfaction when meta contains contract tags.
291-301: ITask extended with TOn and TMeta — correctExtends ITaskDefinition with the new generics; preserves backward compatibility via defaults.
317-384: IResourceDefinition Promise-based TValue and contract-aware init — correct
- TValue constrained to Promise.
- init return conditional integrates contract enforcement.
- meta?: TMeta added.
386-419: IResource updated generics and extension — correctExtends the updated IResourceDefinition with TContext and TMeta; signatures look consistent with defineResource.
421-433: IResourceWithConfig TValue now Promise-based — correctMatches the new Promise-based TValue across the API.
438-441: Event meta retainedNo issues; consistent with other meta types.
| TOn extends "*" | IEventDefinition | undefined = undefined, | ||
| TMeta extends ITaskMeta = any | ||
| >( | ||
| taskConfig: ITaskDefinition<Input, Output, Deps, TOn> | ||
| ): ITask<Input, Output, Deps, TOn> { | ||
| taskConfig: ITaskDefinition<Input, Output, Deps, TOn, TMeta> | ||
| ): ITask<Input, Output, Deps, TOn, TMeta> { | ||
| /** |
There was a problem hiding this comment.
Align TOn constraint with defs.ts (use IEventDefinition)
In defs.ts, TOn is constrained as "" | IEventDefinition | undefined. Here it’s "" | IEventDefinition | undefined (defaults to void), which is a narrower constraint and may hinder inference/compatibility.
Apply this change:
- TOn extends "*" | IEventDefinition | undefined = undefined,
+ TOn extends "*" | IEventDefinition<any> | undefined = undefined,🤖 Prompt for AI Agents
In src/define.ts around lines 48 to 53, change the generic constraint for TOn
from 'TOn extends "*" | IEventDefinition | undefined = undefined' to 'TOn
extends "*" | IEventDefinition<any> | undefined = undefined' so it matches
defs.ts; update the type parameter declaration accordingly to allow
IEventDefinition<any> and preserve existing default = undefined.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/models/Store.ts (1)
128-142: Align initializeStore and setupRootResource typings; guard dispose against uninitialized values
- Typing: initializeStore now uses IResource<any, any, any, any, any> but setupRootResource still accepts IResource. For consistency and stronger types, update setupRootResource to the widened signature as well.
- Safety: dispose currently passes resource.value without checking initialization. If initialization fails early, dispose might be called with undefined.
Within the changed method, no code change needed. Apply these outside changes:
// Change signature (outside this hunk) private setupRootResource( root: IResource<any, any, any, any, any>, config: any ) { // ... unchanged body ... } // Strengthen dispose safety (outside this hunk) public async dispose() { for (const resource of this.resources.values()) { if (!resource.isInitialized) continue; if (resource.resource.dispose) { await resource.resource.dispose( resource.value as any, // will be addressed by StoreTypes change to Awaited<V> resource.config, resource.computedDependencies as DependencyMapType, resource.context ); } } }src/models/StoreTypes.ts (1)
16-24: Make ResourceStoreElementType.value store the resolved value and allow uninitialized stateWith V constrained to Promise, value should be Awaited, not V, and it should allow undefined before init. This also aligns dispose(value) to receive the resolved value.
Apply this diff:
-export type ResourceStoreElementType< - C = any, - V extends Promise<any> = any, - D extends DependencyMapType = {}, - TContext = any -> = { - resource: IResource<C, V, D>; - computedDependencies?: DependencyValuesType<D>; - config: C; - value: V; - context: TContext; - isInitialized?: boolean; -}; +export type ResourceStoreElementType< + C = any, + V extends Promise<any> = Promise<any>, + D extends DependencyMapType = {}, + TContext = any +> = { + resource: IResource<C, V, D>; + computedDependencies?: DependencyValuesType<D>; + config: C; + // Store the resolved value; it is undefined until initialization completes + value: Awaited<V> | undefined; + context: TContext; + isInitialized?: boolean; +};This change prevents passing a Promise to disposers and fixes the current “value: undefined” assignment in Store.setupRootResource.
🧹 Nitpick comments (11)
AI.md (6)
18-21: Docs: event() usage is inconsistent (object vs string id)You use both event({ id }) and event("id"). Please unify to the actual API to avoid confusion.
Apply this diff to align the “At a Glance” snippet with the string-id style used later:
- - `event<T>`: - - Emits/handles async events: `const e = event<T>({ id })` then `await e(payload)`. + - `event<T>`: + - Emits/handles async events: `const e = event<T>("app.events.someEvent")` then `await e(payload)`.
31-33: Docs: tag() invocation style is also inconsistentYou show tag being invoked with an object in one place and a string in others. Standardize to one style (prefer string id for consistency with events).
Apply this diff to the contract example:
-const userContract = tag<void, { name: string }>({ id: "contract.user" }); +const userContract = tag<void, { name: string }>("contract.user");Also applies to: 52-53
91-95: Triple-quoted string is not valid TypeScriptUse backticks for multiline strings in the example.
- description: """ -Describe what it does -""", + description: `Describe what it does`,
102-103: Typo: “infered” → “inferred”- // Types are automatically infered + // Types are automatically inferred
6-6: Nit: “stand alone” → “standalone”-- Tasks can be stand alone tasks or listeners (that listen to events) +- Tasks can be standalone tasks or listeners (that listen to events)
286-287: Naming: resource id should follow the “{domain}.resources.{name}” conventionThe guideline suggests resources be named like app.resources.root.
- id: "app", + id: "app.resources.root",src/__tests__/recursion/c.resource.ts (1)
18-18: Prefer inference over “as” casting for IResource value typeCasting to IResource<void, Promise> can mask type issues. If defineResource already returns the correct type, drop the cast and let TS infer it. If inference doesn’t work due to unioned dependencies, consider tightening defineResource generics.
Would you like me to scan usages of defineResource to confirm its inferred return aligns with Promise-based values and remove redundant casts?
src/testing.ts (2)
30-33: Nit: resource id should follow the naming conventionUse {domain}.resources.{resourceName}, for example tests.resources.harness.{n}.
- id: `tests.createTestResource.${++testResourceCounter}`, + id: `tests.resources.harness.${++testResourceCounter}`,
51-66: Optional DX: add a helper to await resource valuesSince store.resources.get(id)?.value is now a Promise-resolved value in the store (post-initialization), exposing a convenience method that awaits it improves test ergonomics.
return { // Run a task within the fully initialized ecosystem runTask: <I, O extends Promise<any>, D extends DependencyMapType>( task: ITask<I, O, D>, input: I ): Promise<Awaited<O> | undefined> => deps.taskRunner.run(task, input) as any, // Access a resource value by id (string or symbol) getResource: (id: string | symbol) => deps.store.resources.get(id)?.value, + // Convenience: await the resource value + getResourceValue: async <T = unknown>(id: string | symbol): Promise<T | undefined> => { + const entry = deps.store.resources.get(id); + return entry?.value as unknown as T | undefined; + },src/models/ResourceInitializer.ts (1)
112-112: Type cast is necessary but consider documenting the safety assumption.The cast to
unknownthen toTValueis needed for the error suppression path. While this is technically safe since the value is only used when errors are suppressed, consider adding a comment explaining this safety assumption.Consider adding a comment to explain the type cast:
- return { value: undefined as unknown as TValue, context: {} as TContext }; + // Safe cast: undefined value is only used when error is suppressed + return { value: undefined as unknown as TValue, context: {} as TContext };src/defs.ts (1)
139-142: Consider documenting the purpose of separate meta interfaces.While extending
IMetawithout adding properties might seem redundant, this pattern provides flexibility for future extensions. Consider adding JSDoc comments to clarify this design decision.Add documentation to explain the extensibility pattern:
+/** + * Task-specific metadata interface. Currently identical to IMeta but + * provides a extension point for task-specific metadata in the future. + */ export interface ITaskMeta extends IMeta {} +/** + * Resource-specific metadata interface. Currently identical to IMeta but + * provides a extension point for resource-specific metadata in the future. + */ export interface IResourceMeta extends IMeta {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
AI.md(1 hunks)src/__tests__/recursion/c.resource.ts(1 hunks)src/__tests__/typesafety.test.ts(3 hunks)src/defs.ts(15 hunks)src/models/ResourceInitializer.ts(2 hunks)src/models/Store.ts(1 hunks)src/models/StoreTypes.ts(1 hunks)src/run.ts(2 hunks)src/testing.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: Use.with(config)to configure resources and middleware
Use theindex()helper to group dependencies
Use the context system for request-scoped data viacreateContext()
Use built-in caching middleware withglobals.middleware.cache
Use global events for lifecycle hooks (e.g.,globals.tasks.beforeRun)
Handle errors with suppression capability viaevent.data.suppress()
Use the override system for replacing components
Use built-in logging with levels (trace, debug, info, warn, error, critical)
Tasks should be named using the convention{domain}.tasks.{taskName}(e.g., "app.tasks.createUser")
Resources should be named using the convention{domain}.resources.{resourceName}(e.g., "app.resources.database")
Events should be named using the convention{domain}.events.{eventName}(e.g., "app.events.userRegistered")
Middleware should be named using the convention{domain}.middleware.{middlewareName}(e.g., "app.middleware.auth")
Files:
src/models/Store.tssrc/testing.tssrc/__tests__/recursion/c.resource.tssrc/models/StoreTypes.tssrc/run.tssrc/models/ResourceInitializer.tssrc/__tests__/typesafety.test.tssrc/defs.ts
src/models/Store.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models/Store.ts: Place the central registry for all tasks, resources, events, and middleware insrc/models/Store.ts
Place the store insrc/models/Store.ts
Files:
src/models/Store.ts
src/run.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place the main runner function that orchestrates execution in
src/run.ts
Files:
src/run.ts
src/models/ResourceInitializer.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/models/ResourceInitializer.ts: Place resource lifecycle management (init/dispose) insrc/models/ResourceInitializer.ts
Place the resource initializer insrc/models/ResourceInitializer.ts
Files:
src/models/ResourceInitializer.ts
src/defs.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place all TypeScript interfaces and type definitions in
src/defs.ts
Files:
src/defs.ts
🧠 Learnings (7)
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/models/Store.ts : Place the central registry for all tasks, resources, events, and middleware in `src/models/Store.ts`
Applied to files:
src/models/Store.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/models/Store.ts : Place the store in `src/models/Store.ts`
Applied to files:
src/models/Store.tssrc/models/StoreTypes.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/models/ResourceInitializer.ts : Place the resource initializer in `src/models/ResourceInitializer.ts`
Applied to files:
src/models/Store.tssrc/models/ResourceInitializer.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/models/ResourceInitializer.ts : Place resource lifecycle management (init/dispose) in `src/models/ResourceInitializer.ts`
Applied to files:
src/models/ResourceInitializer.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/define.ts : Place core definition functions (task, resource, event, middleware, index) in `src/define.ts`
Applied to files:
src/__tests__/typesafety.test.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/globals/globalMiddleware.ts : Place built-in global middleware exports in `src/globals/globalMiddleware.ts`
Applied to files:
src/defs.ts
📚 Learning: 2025-08-07T11:37:57.854Z
Learnt from: CR
PR: bluelibs/runner#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-07T11:37:57.854Z
Learning: Applies to src/globals/middleware/cache.middleware.ts : Place built-in caching middleware with LRU support in `src/globals/middleware/cache.middleware.ts`
Applied to files:
src/defs.ts
🧬 Code Graph Analysis (7)
src/models/Store.ts (1)
src/defs.ts (1)
IResource(386-419)
src/testing.ts (1)
src/defs.ts (1)
IResource(386-419)
src/__tests__/recursion/c.resource.ts (1)
src/defs.ts (1)
IResource(386-419)
src/run.ts (1)
src/defs.ts (1)
IResource(386-419)
src/models/ResourceInitializer.ts (1)
src/defs.ts (1)
DependencyMapType(149-152)
src/__tests__/typesafety.test.ts (4)
src/index.ts (4)
run(32-32)defineTag(30-30)defineTask(25-25)defineResource(26-26)src/define.ts (3)
defineTag(358-384)defineTask(44-100)defineResource(102-182)src/defs.ts (1)
IMeta(133-137)src/defs.returnTag.ts (2)
HasContracts(42-43)EnsureResponseSatisfiesContracts(79-91)
src/defs.ts (1)
src/defs.returnTag.ts (2)
HasContracts(42-43)EnsureResponseSatisfiesContracts(79-91)
🔇 Additional comments (15)
src/testing.ts (1)
28-28: LGTM: return type updated to Promise-based resource valueThe change aligns with the Promise-aware resource value semantics introduced across the codebase.
src/run.ts (2)
20-29: Type constraint onVlooks good!The addition of the
V extends Promise<any> = anyconstraint ensures that resource values are always Promise-based, which aligns well with the broader contract enforcement system. The default value maintains backward compatibility.
55-61: Excellent Promise unwrapping in return type!The updated return type correctly unwraps Promise values, ensuring that consumers receive the resolved value rather than a nested Promise. This is a significant improvement to the API's usability.
src/__tests__/typesafety.test.ts (4)
18-22: Import changes align with public API surface.The test correctly imports from the package root, which validates that
runis properly exported as part of the public API.
252-252: Clean transition to simplified run API!The direct use of
run(harness)without needing to call.value()improves the developer experience by removing an unnecessary step.
329-381: Comprehensive contract enforcement tests for tasks!The test cases thoroughly validate the contract enforcement mechanism for tasks with multiple scenarios including valid and invalid return types. The use of
@ts-expect-errorcomments appropriately validates compile-time type checking.
383-430: Well-structured contract enforcement tests for resources!The resource contract tests mirror the task tests appropriately, ensuring consistent behavior across both constructs. The type-level validation is comprehensive.
src/models/ResourceInitializer.ts (2)
24-29: Consistent Promise constraint applied!The type constraint matches the pattern used throughout the codebase, ensuring resources always work with Promise-based values.
116-121: Clean generic constraints on middleware helper!The updated generic constraints properly enforce Promise-based values throughout the middleware chain.
src/defs.ts (6)
18-21: Good modular design with the contract utilities!Separating contract enforcement utilities into
defs.returnTagmodule promotes maintainability and clear separation of concerns.
64-66: Tag definition properly supports contract enforcement!The addition of
TEnforceContractgeneric parameter enables tags to specify the contract type they enforce.
157-161: Correct Promise unwrapping in ExtractResourceValue!The helper type now properly unwraps Promise values, which is consistent with the overall design where resources return Promises but consumers work with the resolved values.
224-226: Excellent compile-time contract enforcement for tasks!The conditional return type using
HasContractsandEnsureResponseSatisfiesContractsprovides strong compile-time guarantees that task outputs satisfy the contracts defined by their tags.Also applies to: 259-261
345-347: Consistent contract enforcement for resources!The resource
initmethod applies the same contract enforcement pattern as tasks, ensuring consistency across the framework.
358-358: Proper value unwrapping in dispose method!The dispose method correctly receives the unwrapped value, which makes the API more intuitive since disposal logic typically works with the actual resource value rather than a Promise.
| id: "app.tasks.createUser",) | ||
| dependencies: { db: database }, |
There was a problem hiding this comment.
Syntax: stray closing parenthesis
There’s an extra ) after the id field.
- id: "app.tasks.createUser",)
+ id: "app.tasks.createUser",📝 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.
| id: "app.tasks.createUser",) | |
| dependencies: { db: database }, | |
| id: "app.tasks.createUser", | |
| dependencies: { db: database }, |
🤖 Prompt for AI Agents
In AI.md around lines 136 to 137, the object literal has a stray closing
parenthesis after the id field ("id: \"app.tasks.createUser\",)"); remove the
extra ')' so the line reads just the id entry with its trailing comma (or remove
the trailing comma if appropriate for the surrounding syntax) to restore valid
JavaScript/JSON-style object syntax.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests