Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4: New lifecycle hooks #1817

Merged
merged 40 commits into from
Mar 25, 2025
Merged

v4: New lifecycle hooks #1817

merged 40 commits into from
Mar 25, 2025

Conversation

ericallam
Copy link
Member

@ericallam ericallam commented Mar 25, 2025

This PR revamps the lifecycle hooks system, including a new way of defining global hooks, better middleware with locals support, new hooks such as onWait, onResume, and onComplete, and a new init.ts file that is automatically loaded on every run.

Breaking changes

The params passed to the existing hooks have been consolidated into a single param object.

Before:

export const taskWithOnStart = task({
  id: "task-with-on-start",
  onStart: async (payload, { ctx }) => {
    //...
  },
  run: async (payload: any, { ctx }) => {
    //...
  },
});

After:

export const taskWithOnStart = task({
  id: "task-with-on-start",
  onStart: async ({ payload, ctx }) => {
    //...
  },
  run: async (payload: any, { ctx }) => {
    //...
  },
});

Deprecations

These are marked as deprecated but will continue to work:

  • init is deprecated, in favor of middleware and locals
  • cleanup is deprecated for the same reason
  • handleError is deprecated, use catchError instead

New global hooks support

Previously, you could only define a single global lifecycle hook in the trigger.config.ts file, but this meant that you'd have to import any dependencies you needed in the hooks into the config file, which could cause issues because it sometimes broke the config loading.

Now you can define one or more global hooks anywhere in your codebase, like so:

import { logger, tasks } from "@trigger.dev/sdk";

tasks.onSuccess(async ({ ctx, payload, output }) => {
  logger.info("Hello, world from the success", { ctx, payload });
});

Improved middleware and locals

Previously, if you wanted to initialize some resource (e.g. a database client) and provide it to your tasks, you'd have to use the init hook and return the client, which you could then access in your run function and other lifecycle hooks:

export const taskWithInitReturn = task({
  id: "task-with-init-return",
  init: async (payload, { ctx }) => {
    return { someData: "someValue" };
  },
  run: async (payload: any, { ctx, init }) => {
    console.log(init.someData); // "someValue"
  },
});

This works okay when passing the init option when creating a task, but the types don't really work if the init is defined globally. So, we've improved the middleware and added a type-safe "locals" system to be able to easily provide resources/clients, etc. to all your tasks and lifecycle hooks:

import { locals } from "@trigger.dev/sdk";
import { logger, tasks } from "@trigger.dev/sdk";

// Define the DbLocal key which is typed to the correct client type
const DbLocal = locals.create<{ connect: () => Promise<void>; disconnect: () => Promise<void> }>(
  "db"
);

// Use this getter to safely access the db client
export function getDb() {
  return locals.getOrThrow(DbLocal);
}

tasks.middleware("db", async ({ ctx, payload, next, task }) => {
  // this will run before ANY other lifecycle hooks and run function
  const db = locals.set(DbLocal, {
    connect: async () => {},
    disconnect: async () => {},
  });

  await db.connect();

  // This will call the next middleware
  await next();

  // Now you can cleanup your database connection here after everything else has finished
  await db.disconnect();
});

New hooks

We've introduced three new hooks

onWait

This hook will be called right before a wait is starting, such as wait.for({ seconds: 5 }), wait.forToken(token), or yourTask.triggerAndWait.

You can use this hook to cleanup resources should you need to, before starting a possibly long wait.

onResume

This hook is similar to onWait, but will be called after the wait has finished and before the rest of your code resumes running.

onComplete

This hook is called when a run has completed, either as a successful or failing run.

init.ts file

If you create an init.ts file in the root of your trigger directory, we'll automatically load this file before loading your task file when executing a task. This is handy as a place to put any global initialization code that you want to run before every task, without having to stick it in the trigger.config.ts file.

Summary by CodeRabbit

  • New Features
    • Introduced a new flag to streamline test command usage.
    • Expanded lifecycle hook and middleware support for enhanced task management.
    • Added optional initialization entry point detection for improved startup flows.
    • Extended visual feedback with additional icons for varied task statuses.
    • Added new task lifecycle hooks for improved task execution management.
  • Bug Fixes
    • Consolidated error reporting for clearer handling during task execution.
  • Tests
    • Implemented comprehensive tests to ensure robust task execution and lifecycle management.
  • Documentation
    • Updated testing examples to reflect the new command flag usage.

Copy link

changeset-bot bot commented Mar 25, 2025

🦋 Changeset detected

Latest commit: 6d5d10a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@trigger.dev/sdk Patch
trigger.dev Patch
@trigger.dev/core Patch
@trigger.dev/python Patch
@trigger.dev/build Patch
@trigger.dev/redis-worker Patch
@internal/redis Patch
@internal/run-engine Patch
@internal/testcontainers Patch
@internal/tracing Patch
@internal/zod-worker Patch
@trigger.dev/react-hooks Patch
@trigger.dev/rsc Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
references-nextjs-realtime Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

coderabbitai bot commented Mar 25, 2025

Walkthrough

This pull request introduces a wide range of updates across documentation, UI components, build processes, task execution, and core API functionality. Key modifications include updating command examples with a new flag, enhancing icon rendering logic, refining error handling, and extending lifecycle and local management APIs. Additionally, new lifecycle hook functions and middleware support have been integrated into worker and task executor modules, and example projects now include database middleware and task lifecycle handling.

Changes

File(s) Change Summary
.cursor/rules/.../executing-commands.mdc
apps/webapp/app/components/runs/v3/RunIcon.tsx
Updated documentation commands to include the --run flag; enhanced the RunIcon component with additional logging and new case conditions for extended task names.
internal-packages/run-engine/src/engine/errors.ts
packages/cli-v3/src/build/bundle.ts
Revised error handling by consolidating specific error codes and added an initEntryPoint property with helper functions for build result processing.
packages/cli-v3/src/entryPoints/dev-index-worker.ts
.../entryPoints/dev-run-worker.ts
.../entryPoints/managed-index-worker.ts
.../entryPoints/managed-run-worker.ts
Enhanced entry worker modules by appending initEntryPoint to messages and integrating new lifecycle hook functions; registered global managers for locals and lifecycle hooks.
packages/core/src/v3/config.ts
.../errors.ts
.../index.ts
.../lifecycle-hooks-api.ts
.../lifecycleHooks/index.ts
.../lifecycleHooks/manager.ts
.../lifecycleHooks/types.ts
.../locals-api.ts
.../locals/index.ts
.../locals/manager.ts
.../locals/types.ts
.../runtime/managedRuntimeManager.ts
.../schemas/build.ts
.../schemas/common.ts
.../semanticInternalAttributes.ts
.../tracer.ts
.../types/tasks.ts
.../utils/globals.ts
.../workers/index.ts
.../workers/taskExecutor.ts
Expanded the core APIs: integrated comprehensive lifecycle hook and local management support, updated task types with new hook interfaces, introduced an optional initEntryPoint in schema definitions, and refined tracing and global API structures.
packages/trigger-sdk/src/v3/hooks.ts
.../index.ts
.../locals.ts
.../shared.ts
.../tasks.ts
Introduced new lifecycle hook functions and re-exported their types; updated shared task creation and registration to integrate hook registration; exposed the locals API via the SDK module.
references/hello-world/src/db.ts
references/hello-world/src/trigger/example.ts
references/hello-world/src/trigger/init.ts
Added a local database management module with connection/disconnection middleware and lifecycle event handlers; defined a new task (hooksTask) demonstrating full hook usage and logging.

Sequence Diagram(s)

Loading
sequenceDiagram
    participant Client
    participant Executor as TaskExecutor
    participant Hooks as LifecycleHooksManager
    participant Middleware

    Client->>Executor: Initiate Task Execution
    Executor->>Hooks: Call onWait hook listeners (pre-execution)
    Executor->>Middleware: Execute middleware chain
    Middleware-->>Executor: Proceed with task logic
    alt Error Occurs
        Executor->>Hooks: Call onCatchError hook listeners
        Executor->>Executor: Process error handling result
    end
    Executor->>Hooks: Call onResume hook listeners (post-execution)
    Executor->>Hooks: Call onComplete hook listeners
    Executor-->>Client: Return task result

Possibly related PRs

Suggested reviewers

  • matt-aitken

Poem

I'm a rabbit hopping through the code,
With every hook and flag, my joy is showed.
Middleware and locals dance in the night,
Logging and lifecycle making everything bright.
I nibble on snippets with a twitch of delight,
Celebrating these changes with a hop and a byte!
🐇💻 Happy trails, code friends!

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (24)
apps/webapp/app/components/runs/v3/RunIcon.tsx (2)

31-32: Remove console.log statement

This appears to be debugging code that shouldn't be committed to production.

-  console.log("spanName", spanName, name);
-

81-102: Consider refactoring repetitive task hook icon definitions

All the new task hook cases return identical code. Consider refactoring to reduce repetition.

-    case "task-middleware":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-fn-run":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-init":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onStart":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onSuccess":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onFailure":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onComplete":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onWait":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-onResume":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-catchError":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
-    case "task-hook-cleanup":
-      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
+    case "task-middleware":
+    case "task-fn-run":
+    case "task-hook-init":
+    case "task-hook-onStart":
+    case "task-hook-onSuccess":
+    case "task-hook-onFailure":
+    case "task-hook-onComplete":
+    case "task-hook-onWait":
+    case "task-hook-onResume":
+    case "task-hook-catchError":
+    case "task-hook-cleanup":
+      return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
packages/core/src/v3/locals/index.ts (2)

9-20: Singleton pattern implementation for LocalsAPI

The LocalsAPI class follows the singleton pattern correctly, but there's a minor issue with static method implementation.

Replace this with explicit class name in static methods to improve clarity:

  public static getInstance(): LocalsAPI {
-    if (!this._instance) {
-      this._instance = new LocalsAPI();
+    if (!LocalsAPI._instance) {
+      LocalsAPI._instance = new LocalsAPI();
    }

-    return this._instance;
+    return LocalsAPI._instance;
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 16-16: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


30-40: Implementation of LocalsManager interface methods

The interface methods are properly implemented, but there's a minor issue with the return type of setLocal.

Remove the return statement in setLocal since it has a void return type:

  public setLocal<T>(key: LocalsKey<T>, value: T): void {
-    return this.#getManager().setLocal(key, value);
+    this.#getManager().setLocal(key, value);
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 39-39: The function should not return a value because its return type is void.

The function is here:

'void' signals the absence of value. The returned value is likely to be ignored by the caller.

(lint/correctness/noVoidTypeReturn)

packages/core/src/v3/lifecycleHooks/index.ts (2)

29-35: Use the class name instead of this in static context.

Static analysis indicates that referencing this within a static method can be confusing. You can avoid potential confusion (and lint issues) by referencing LifecycleHooksAPI._instance directly. Consider using:

- if (!this._instance) {
-   this._instance = new LifecycleHooksAPI();
- }
- return this._instance;
+ if (!LifecycleHooksAPI._instance) {
+   LifecycleHooksAPI._instance = new LifecycleHooksAPI();
+ }
+ return LifecycleHooksAPI._instance;

Additionally, if multiple threads or asynchronous calls invoke getInstance() simultaneously, you may encounter race conditions. You might want to add locking or ensure this initialization code runs prior to concurrent access.

🧰 Tools
🪛 Biome (1.9.4)

[error] 30-30: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 34-34: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


37-44: Consider concurrency checks when registering/unregistering global state.

setGlobalLifecycleHooksManager() and disable() manage a global reference. If these operations can be triggered concurrently, consider adding synchronization to avoid subtle race conditions.

packages/core/src/v3/locals/manager.ts (1)

37-37: Remove stray character

There's a stray "0;" at the end of the file that should be removed.

-0;
packages/core/src/v3/runtime/managedRuntimeManager.ts (3)

56-59: Audit error handling before resuming.

Similar to the wait process, the resume process uses await Promise.all(...) for listeners. Consider whether a single listener rejection should fail the entire waitForTask flow or if you’d like to isolate errors.


92-96: Double-check resilience during batch resume.

Just like the wait logic, confirm whether a single hook's rejection in callOnResumeHookListeners should block resuming the batch. Consider capturing errors in a logging mechanism to avoid unexplained failures in one hook blocking the entire flow.

Would you like me to propose a small code snippet to handle partial failures during batch resume?


131-141: Consider uniform error handling for resume logic.

The conditional usage of callOnResumeHookListeners parallels the wait logic, but the same caution about Promise.all rejections applies here. You might consider a try/catch approach or capturing errors individually if graceful degradation is required.

packages/core/src/v3/lifecycleHooks/manager.ts (2)

65-300: Extensive duplication of register/get methods.

Each hook type (start, init, failure, success, complete, etc.) uses near-identical logic. Consider extracting common logic into helper functions or using a single map with a composite key for the hook type and task ID to reduce duplication.


601-603: Potential collisions in generateHookId.

Using hook.fn.toString() might cause large string IDs and unforeseen collisions for complex or transcompiled functions. Consider generating a short hash of the function instead to optimize memory usage and reduce risk of collisions.

packages/core/src/v3/lifecycle-hooks-api.ts (1)

1-5: Centralized lifecycle API instance.

Exporting a singleton (lifecycleHooks) is a convenient global entry point. However, if testability or multi-tenant usage is needed in the future, you may consider a factory-based or dependency-injection approach instead.

packages/cli-v3/src/build/bundle.ts (1)

239-249: Enhance flexibility for isInitEntryPoint checks
This helper method cleanly ensures init.ts is located at the root of configured dirs. As a future improvement, consider allowing a user-defined init file name if needed or handling subdirectories.

packages/core/src/v3/workers/taskExecutor.ts (5)

23-24: Minor clarity nitpick:
Consider grouping these imports together with the rest of the lifecycle-related imports for easier discovery.


49-52: Configuration for retry logic is well defined.
This new optional retries property in TaskExecutorOptions clarifies retry behavior. Ensure documentation is updated to reflect the enabledInDev property usage.


138-198: Design feedback on local function executeTask
Grouping run initialization, error handling, and final result creation in a single async function simplifies the readability. However, it's quite large—consider extracting repetitive code blocks (e.g., calls to #callOnCompleteFunctions) into smaller helper functions, if needed, for maintainability.


355-392: #callRun function
The usage of an abort signal via Promise.race elegantly stops long-running tasks. Ensure side effects (like partial DB writes) are compensated for when aborted.


789-851: #callOnStartFunctions
Great approach to measure hook performance with runTimelineMetrics. Keep these metrics to help diagnose slow start hooks.

packages/trigger-sdk/src/v3/shared.ts (3)

1-1: Removed or replaced imports
Using SpanKind from @opentelemetry/api might be helpful if instrumentation is needed. Confirm it's still required.


20-24: Export blocks
Explicit re-export of types from @trigger.dev/core/v3 clarifies the public API surface. Keep these well-documented.


354-358: Unified hooks registration
Another registerTaskLifecycleHooks call. Maintaining parallel calls in both createTask and createSchemaTask is consistent, though watch for any duplication.

packages/trigger-sdk/src/v3/hooks.ts (1)

16-28: Type re-exports
Re-exporting these hook types fosters easier consumption for end users. Keep an eye on version-sync to avoid mismatches.

packages/core/src/v3/lifecycleHooks/types.ts (1)

4-4: Refine 'void' in union return types
Static analysis flags the usage of undefined | void as potentially confusing. Consider omitting void to reduce ambiguity, for example merging it into a single undefined type.

- ) => TInitOutput | undefined | void | Promise<TInitOutput | undefined | void>;
+ ) => TInitOutput | undefined | Promise<TInitOutput | undefined>;

Also applies to: 15-15, 32-32, 69-69, 87-87, 105-105, 128-128, 165-165, 229-229

🧰 Tools
🪛 Biome (1.9.4)

[error] 4-4: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef60ce and 1c36b9c.

📒 Files selected for processing (37)
  • .cursor/rules/executing-commands.mdc (1 hunks)
  • apps/webapp/app/components/runs/v3/RunIcon.tsx (3 hunks)
  • internal-packages/run-engine/src/engine/errors.ts (1 hunks)
  • packages/cli-v3/src/build/bundle.ts (5 hunks)
  • packages/cli-v3/src/entryPoints/dev-index-worker.ts (1 hunks)
  • packages/cli-v3/src/entryPoints/dev-run-worker.ts (8 hunks)
  • packages/cli-v3/src/entryPoints/managed-index-worker.ts (1 hunks)
  • packages/cli-v3/src/entryPoints/managed-run-worker.ts (7 hunks)
  • packages/core/src/v3/config.ts (2 hunks)
  • packages/core/src/v3/errors.ts (1 hunks)
  • packages/core/src/v3/index.ts (1 hunks)
  • packages/core/src/v3/lifecycle-hooks-api.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/index.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/manager.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/types.ts (1 hunks)
  • packages/core/src/v3/locals-api.ts (1 hunks)
  • packages/core/src/v3/locals/index.ts (1 hunks)
  • packages/core/src/v3/locals/manager.ts (1 hunks)
  • packages/core/src/v3/locals/types.ts (1 hunks)
  • packages/core/src/v3/runtime/managedRuntimeManager.ts (4 hunks)
  • packages/core/src/v3/schemas/build.ts (2 hunks)
  • packages/core/src/v3/schemas/common.ts (1 hunks)
  • packages/core/src/v3/semanticInternalAttributes.ts (1 hunks)
  • packages/core/src/v3/tracer.ts (1 hunks)
  • packages/core/src/v3/types/tasks.ts (6 hunks)
  • packages/core/src/v3/utils/globals.ts (2 hunks)
  • packages/core/src/v3/workers/index.ts (1 hunks)
  • packages/core/src/v3/workers/taskExecutor.ts (11 hunks)
  • packages/core/test/taskExecutor.test.ts (1 hunks)
  • packages/trigger-sdk/src/v3/hooks.ts (1 hunks)
  • packages/trigger-sdk/src/v3/index.ts (1 hunks)
  • packages/trigger-sdk/src/v3/locals.ts (1 hunks)
  • packages/trigger-sdk/src/v3/shared.ts (6 hunks)
  • packages/trigger-sdk/src/v3/tasks.ts (3 hunks)
  • references/hello-world/src/db.ts (1 hunks)
  • references/hello-world/src/trigger/example.ts (2 hunks)
  • references/hello-world/src/trigger/init.ts (1 hunks)
🧰 Additional context used
🧬 Code Definitions (12)
packages/core/src/v3/lifecycleHooks/index.ts (2)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
  • NoopLifecycleHooksManager (399-599)
packages/core/src/v3/lifecycleHooks/types.ts (14)
  • LifecycleHooksManager (233-310)
  • RegisterHookFunctionParams (169-172)
  • AnyOnInitHookFunction (17-17)
  • RegisteredHookFunction (174-178)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
  • AnyOnCleanupHookFunction (231-231)
  • TaskWait (36-53)
packages/core/src/v3/locals/index.ts (3)
packages/core/src/v3/locals/manager.ts (1)
  • NoopLocalsManager (3-16)
packages/core/src/v3/locals/types.ts (2)
  • LocalsManager (10-14)
  • LocalsKey (5-8)
packages/core/src/v3/utils/globals.ts (3)
  • registerGlobal (19-35)
  • unregisterGlobal (43-49)
  • getGlobal (37-41)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (14)
  • LifecycleHooksManager (233-310)
  • RegisteredHookFunction (174-178)
  • AnyOnInitHookFunction (17-17)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
  • AnyOnCleanupHookFunction (231-231)
  • TaskWait (36-53)
  • RegisterHookFunctionParams (169-172)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
packages/core/src/v3/lifecycleHooks/index.ts (1)
  • LifecycleHooksAPI (24-266)
packages/core/test/taskExecutor.test.ts (5)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
  • lifecycleHooks (5-5)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
  • StandardLifecycleHooksManager (18-397)
packages/core/src/v3/workers/taskExecutor.ts (12)
  • payload (302-353)
  • payload (355-393)
  • payload (535-629)
  • payload (631-702)
  • payload (704-775)
  • payload (777-787)
  • payload (789-851)
  • payload (853-861)
  • payload (863-931)
  • payload (1119-1190)
  • execution (952-1067)
  • execution (1192-1208)
packages/core/src/v3/types/tasks.ts (2)
  • RunFnParams (86-93)
  • TaskMetadataWithFunctions (871-887)
packages/core/src/v3/schemas/common.ts (4)
  • TaskRunErrorCodes (195-195)
  • TaskRunErrorCodes (196-196)
  • TaskRunExecution (302-316)
  • TaskRunExecution (318-318)
packages/trigger-sdk/src/v3/tasks.ts (1)
packages/trigger-sdk/src/v3/hooks.ts (2)
  • onHandleError (104-109)
  • onCatchError (113-121)
packages/core/src/v3/config.ts (3)
packages/core/src/v3/lifecycle-hooks-api.ts (4)
  • AnyOnInitHookFunction (9-9)
  • AnyOnSuccessHookFunction (18-18)
  • AnyOnFailureHookFunction (16-16)
  • AnyOnStartHookFunction (14-14)
packages/core/src/v3/lifecycleHooks/types.ts (4)
  • AnyOnInitHookFunction (17-17)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnStartHookFunction (34-34)
packages/trigger-sdk/src/v3/hooks.ts (3)
  • AnyOnSuccessHookFunction (21-21)
  • AnyOnFailureHookFunction (20-20)
  • AnyOnStartHookFunction (17-17)
packages/core/src/v3/workers/taskExecutor.ts (4)
packages/core/src/v3/schemas/schemas.ts (2)
  • RetryOptions (99-126)
  • RetryOptions (128-128)
packages/core/src/v3/semanticInternalAttributes.ts (1)
  • SemanticInternalAttributes (1-60)
packages/core/src/v3/errors.ts (1)
  • InternalError (18-44)
packages/core/src/v3/types/tasks.ts (1)
  • HandleErrorModificationOptions (128-134)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycleHooks/types.ts (8)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
packages/core/src/v3/lifecycle-hooks-api.ts (9)
  • AnyOnStartHookFunction (14-14)
  • lifecycleHooks (5-5)
  • AnyOnFailureHookFunction (16-16)
  • AnyOnSuccessHookFunction (18-18)
  • AnyOnCompleteHookFunction (20-20)
  • AnyOnWaitHookFunction (22-22)
  • AnyOnResumeHookFunction (24-24)
  • AnyOnCatchErrorHookFunction (26-26)
  • AnyOnMiddlewareHookFunction (29-29)
packages/cli-v3/src/entryPoints/dev-run-worker.ts (6)
packages/core/src/v3/workers/index.ts (3)
  • StandardLocalsManager (24-24)
  • StandardRunTimelineMetricsManager (21-21)
  • StandardLifecycleHooksManager (23-23)
packages/core/src/v3/locals/manager.ts (1)
  • StandardLocalsManager (18-36)
packages/core/src/v3/runTimelineMetrics/runTimelineMetricsManager.ts (1)
  • StandardRunTimelineMetricsManager (5-47)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
  • StandardLifecycleHooksManager (18-397)
packages/core/src/v3/lifecycle-hooks-api.ts (6)
  • lifecycleHooks (5-5)
  • AnyOnInitHookFunction (9-9)
  • AnyOnStartHookFunction (14-14)
  • AnyOnSuccessHookFunction (18-18)
  • AnyOnFailureHookFunction (16-16)
  • AnyOnCatchErrorHookFunction (26-26)
packages/core/src/v3/lifecycleHooks/types.ts (5)
  • AnyOnInitHookFunction (17-17)
  • AnyOnStartHookFunction (34-34)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnCatchErrorHookFunction (200-200)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (10)
  • OnInitHookFunction (13-15)
  • OnCleanupHookFunction (227-229)
  • OnCatchErrorHookFunction (195-198)
  • OnResumeHookFunction (85-87)
  • OnWaitHookFunction (67-69)
  • OnCompleteHookFunction (159-165)
  • OnMiddlewareHookFunction (210-212)
  • OnStartHookFunction (30-32)
  • OnSuccessHookFunction (122-128)
  • OnFailureHookFunction (103-105)
packages/core/src/v3/lifecycleHooks/types.ts (4)
packages/core/src/v3/lifecycle-hooks-api.ts (27)
  • TaskInitHookParams (11-11)
  • OnInitHookFunction (8-8)
  • AnyOnInitHookFunction (9-9)
  • TaskStartHookParams (12-12)
  • OnStartHookFunction (13-13)
  • AnyOnStartHookFunction (14-14)
  • TaskWait (34-34)
  • TaskWaitHookParams (21-21)
  • AnyOnWaitHookFunction (22-22)
  • TaskResumeHookParams (23-23)
  • AnyOnResumeHookFunction (24-24)
  • TaskFailureHookParams (15-15)
  • AnyOnFailureHookFunction (16-16)
  • TaskSuccessHookParams (17-17)
  • AnyOnSuccessHookFunction (18-18)
  • TaskCompleteResult (27-27)
  • TaskCompleteHookParams (19-19)
  • AnyOnCompleteHookFunction (20-20)
  • RegisteredHookFunction (10-10)
  • TaskCatchErrorHookParams (25-25)
  • AnyOnCatchErrorHookFunction (26-26)
  • TaskMiddlewareHookParams (28-28)
  • OnMiddlewareHookFunction (30-30)
  • AnyOnMiddlewareHookFunction (29-29)
  • TaskCleanupHookParams (33-33)
  • OnCleanupHookFunction (31-31)
  • AnyOnCleanupHookFunction (32-32)
packages/core/src/v3/types/tasks.ts (1)
  • HandleErrorResult (136-140)
packages/core/src/v3/lifecycleHooks/index.ts (1)
  • LifecycleHooksManager (263-265)
packages/core/src/v3/workers/taskExecutor.ts (2)
  • wait (395-463)
  • wait (465-533)
🪛 Biome (1.9.4)
packages/core/src/v3/lifecycleHooks/index.ts

[error] 30-30: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 34-34: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

packages/core/src/v3/locals/index.ts

[error] 15-15: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 16-16: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 39-39: The function should not return a value because its return type is void.

The function is here:

'void' signals the absence of value. The returned value is likely to be ignored by the caller.

(lint/correctness/noVoidTypeReturn)

packages/core/test/taskExecutor.test.ts

[error] 914-915: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

packages/core/src/v3/lifecycleHooks/types.ts

[error] 4-4: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 15-15: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 32-32: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 69-69: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 87-87: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 105-105: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 128-128: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 165-165: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 229-229: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / 🧪 Unit Tests
  • GitHub Check: typecheck / typecheck
🔇 Additional comments (148)
.cursor/rules/executing-commands.mdc (2)

16-16: Documentation update looks good!

The addition of the --run flag to the test command aligns with the current best practices for executing tests in the monorepo.


23-23: Command update is consistent!

The --run flag has been correctly added here as well, maintaining consistency across all test command examples.

apps/webapp/app/components/runs/v3/RunIcon.tsx (1)

49-51: Good enhancement for icon flexibility

Adding support for direct rendering of icons from the tablerIcons set improves the component's flexibility.

packages/core/src/v3/semanticInternalAttributes.ts (1)

36-36: New attribute addition looks good

The COLLAPSED attribute follows the established naming convention and integrates well with the existing attributes.

packages/trigger-sdk/src/v3/locals.ts (1)

1-5: Clean re-export implementation

This file effectively re-exports the local variable management API from the core package, making it accessible through the SDK. Good practice for API design.

packages/trigger-sdk/src/v3/index.ts (1)

15-15: LGTM: Local variables support added through export

The addition of this export statement appropriately exposes the functionality from the locals module, enabling access to local variable management features through the SDK.

packages/core/src/v3/index.ts (1)

18-19: LGTM: New lifecycle hooks and locals API exports

Good addition of exports for the lifecycle hooks and locals APIs. This makes these new features accessible to consumers of the core package.

packages/core/src/v3/errors.ts (1)

308-308: LGTM: Added middleware error to retry logic

The addition of "TASK_MIDDLEWARE_ERROR" as a retryable error code is appropriate. This ensures middleware-related errors can be properly handled through the retry mechanism.

packages/core/src/v3/workers/index.ts (1)

23-24: LGTM: Exported lifecycle and locals managers

These exports make the StandardLifecycleHooksManager and StandardLocalsManager available from the workers package, allowing for consistent management of lifecycle hooks and local variables across the worker context.

packages/core/src/v3/schemas/common.ts (1)

168-168: New error code added for task middleware.

The new "TASK_MIDDLEWARE_ERROR" error code has been added to the TaskRunInternalError schema. This is consistent with the PR objective of introducing new lifecycle hooks and middleware support for tasks.

references/hello-world/src/trigger/init.ts (2)

1-7: Middleware implementation looks good.

The task middleware implementation follows a proper middleware pattern with the use of next() to continue execution flow. The logging statement is appropriate for demonstration purposes.


9-41: Commented out lifecycle hooks provide useful reference.

These commented-out lifecycle hook implementations provide good examples of the different hook types available in the new API. Having them commented out is appropriate for a reference implementation.

Note that if you intend to activate these hooks later, you might want to fix the copy-paste error in the logger messages - several hooks use the same message "Hello, world from the start" or "Hello, world from the success" despite being different hook types.

references/hello-world/src/trigger/example.ts (2)

7-8: Added logging provides better visibility into the task execution.

This logging statement helps to trace the task lifecycle and adds consistency with the new hooks pattern.


151-194: Well-structured implementation of lifecycle hooks.

The hooksTask implementation thoroughly demonstrates all available lifecycle hooks with clear logging statements. This serves as an excellent reference example for users implementing the new hooks API.

The various hooks (init, onWait, onResume, onStart, onSuccess, onFailure, onComplete, handleError, catchError, and cleanup) are implemented in a logical order that follows the task lifecycle.

internal-packages/run-engine/src/engine/errors.ts (1)

16-18: Error handling updated to support middleware errors.

The error handling logic now properly categorizes middleware errors along with input and output errors as "COMPLETED_WITH_ERRORS". This provides a consistent experience when middleware encounters issues.

packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)

144-144: Feature addition: initEntryPoint property added to manifest

The addition of initEntryPoint property to the manifest object ensures that initialization entry point information is properly passed to the worker. This is consistent with the architectural pattern used throughout the codebase for other entry points.

packages/core/src/v3/utils/globals.ts (2)

3-4: New lifecycle and locals management features added

The imports for the new lifecycle hooks and locals management functionality have been properly added.


67-68: Global API extended with lifecycle hooks and locals management

The TriggerDotDevGlobalAPI type has been appropriately extended with two new optional properties for lifecycle hooks and locals management. This follows the established pattern for global API registration.

packages/core/src/v3/locals/index.ts (2)

1-8: New LocalsAPI implementation with proper imports

The file correctly imports necessary dependencies and establishes a NOOP manager as a fallback.


42-44: Private helper method for manager retrieval

The private helper method correctly retrieves the registered manager or falls back to the NOOP manager.

packages/cli-v3/src/entryPoints/managed-index-worker.ts (1)

144-144: Feature addition: initEntryPoint property added to manifest

The addition of initEntryPoint property to the manifest object ensures that initialization entry point information is properly passed to the worker. This is consistent with the change made in the dev-index-worker.ts file and maintains parity between development and managed environments.

packages/core/src/v3/lifecycleHooks/index.ts (1)

247-266: Global wait/resume hook approach looks consistent.

The implementation of callOnWaitHookListeners, callOnResumeHookListeners, registerOnWaitHookListener, and registerOnResumeHookListener is well-structured and aligns with the rest of the lifecycle logic. No issues flagged here.

packages/core/src/v3/tracer.ts (1)

171-186: Confirm necessity of creating an additional partial span.

Lines 171–186 create a “partialSpan” and end it immediately. While this can be useful for more granular logging or instrumentation, it does incur overhead. If the extra span is not strictly needed, removing or making it conditional could optimize performance. Otherwise, this approach appears logically sound and correct relative to the updated parentContext.

Do you want to verify usage of this “partialSpan” across the codebase to ensure it’s fully utilized? I can generate a script to search for references and confirm its purpose.

packages/core/src/v3/schemas/build.ts (1)

41-41: Validate optional initEntryPoint usage.

Lines 41 and 89 add an optional initEntryPoint property to the schema. This is consistent with your existing optional entries. Ensure that any code referencing initEntryPoint handles its absence gracefully (e.g., fallback to default or skip initialization). Otherwise, this addition looks good.

Also applies to: 89-89

packages/core/src/v3/locals/types.ts (2)

1-8: Well-designed type-safe local storage mechanism

The implementation uses TypeScript's type branding pattern with a unique symbol to create a type-safe key system, preventing accidental access with incorrect types. The LocalsKey<T> type effectively combines the brand with useful metadata (id and type symbol).


10-14: Clean interface design for locals management

The LocalsManager interface provides a well-structured API with the necessary operations for managing local variables. The generic type parameters ensure type safety throughout the system.

packages/core/src/v3/locals/manager.ts (2)

3-16: Appropriate no-op implementation

The NoopLocalsManager provides a useful null object pattern implementation that satisfies the interface without storing any data. This is helpful for testing or environments where locals storage isn't needed.


18-36: Well-implemented locals storage

The StandardLocalsManager correctly implements the LocalsManager interface with appropriate storage. Using Symbol.for(id) ensures consistent symbol creation across multiple calls with the same id, which is important for reliable key lookup.

references/hello-world/src/db.ts (3)

4-14: Good pattern for local database management

The implementation creates a typed local storage key for database operations and provides helper functions that encapsulate the locals API. Using getOrThrow ensures you'll get proper error messages if the database isn't initialized.


16-34: Well-structured database middleware

The middleware properly manages the database lifecycle around task execution:

  1. Initializes the database connection
  2. Logs before and after task execution
  3. Properly cleans up by disconnecting

This is a good pattern for resource management in serverless environments.


36-48: Proper handling of task lifecycle events

The onWait and onResume handlers appropriately manage database connections during task pauses. Disconnecting during waiting periods can help reduce resource usage and costs, while reconnecting ensures the database is available when needed.

packages/trigger-sdk/src/v3/tasks.ts (3)

1-11: Comprehensive lifecycle hook imports

The imports bring in a complete set of lifecycle hooks that cover the entire task execution flow, from start to completion, including error handling and wait/resume states.


60-60: Proper type exports

Exporting all types from the hooks module makes them available to consumers of the tasks module, which improves developer experience by reducing the need to import from multiple modules.


92-101: Good lifecycle hook integration with clear deprecation

The lifecycle hooks are well-integrated into the tasks object, making them easily accessible. The deprecation notice for handleError is clear and directs users to the preferred catchError method.

From the context, I can see that onHandleError is an alias for onCatchError to maintain backward compatibility while encouraging users to migrate to the new API.

packages/core/src/v3/runtime/managedRuntimeManager.ts (3)

48-51: Consider handling rejections from wait-hook listeners.

Currently, if any listener rejects, the await statement will reject the entire function call. Confirm whether this behavior is desirable, and if not, consider adding individual error-handling logic or a fallback to ensure a single failed hook does not block the rest.

Do you want me to check other parts of this codebase to see if there's a universal error handling pattern for rejection from hook listeners?


84-88: Batch wait logic is well structured.

The call to callOnWaitHookListeners for the batch scenario looks consistent with the single-task approach. However, verify that any potential errors from one hook in a batch do not block the entire batch processing unless it’s your intended behavior.


117-127: Conditional wait logic clarity.

Splitting wait logic based on finishDate for 'duration' vs. 'token' is clear and maintainable. Ensure you have adequate tests for each branch to confirm correct behavior when the hook listeners reject or throw exceptions.

packages/core/src/v3/lifecycleHooks/manager.ts (3)

1-16: Imported types and interfaces are consistent.

The import statements properly reference hook function types from “./types.js.” This organization is transparent for future maintainability and helps with tree-shaking. No apparent issues.


18-64: Parallel execution of wait/resume hook listeners.

Here, callOnWaitHookListeners and callOnResumeHookListeners effectively run all hooks in parallel. Ensure that you want a single rejected promise to short-circuit the rest. If partial or tolerant execution is desired, handle each promise individually.


399-599: Noop lifecycle manager is straightforward.

All methods do nothing, making it a suitable fallback for scenarios that do not require lifecycle management. Confirm that calling these methods inadvertently will not mislead the code flow by silently discarding callbacks.

packages/core/src/v3/lifecycle-hooks-api.ts (1)

7-35: Clean type re-exports.

Re-exporting all hook-related types from a single file is a best practice for discoverability and reduces import clutter. This approach is well structured, and no issues are found here.

packages/cli-v3/src/build/bundle.ts (5)

50-50: Add initEntryPoint property for clarity
Introducing initEntryPoint in the BundleResult helps track the initialization file distinctly.


233-233: Initialization of initEntryPoint variable
Declaring initEntryPoint provides a convenient placeholder that will be set when an init.ts is discovered.


299-299: Include initEntryPoint in the bundle result
Returning initEntryPoint in the build result is consistent and ensures consumers have the path to the initialization file.


377-377: Extend build manifest with initEntryPoint
Propagating initEntryPoint into the build manifest ensures any deployment or runtime environment can locate and use the initialization file seamlessly.


271-272:

❓ Verification inconclusive

Verify multiple init.ts files scenario
When assigning initEntryPoint, make sure there are no collisions if multiple init.ts files exist. Confirm only one is expected or handle multiple gracefully.

Would you like to scan for additional init.ts matches to ensure you only pick up the first occurrence?


🏁 Script executed:

#!/bin/bash
rg -A2 $'init.ts' 

Length of output: 1257


Attention: Confirm Handling of Multiple init.ts Files

The search results reveal that init.ts references exist in several parts of the repository (e.g., in the core schemas and documentation). In packages/cli-v3/src/build/bundle.ts, the bundler checks if an entry point is an init.ts but immediately assigns that file to initEntryPoint without guarding against multiple occurrences.

  • Verify that the bundler’s current logic is intentional—i.e., that it picks the first (or last) encountered init.ts file—and that any collision is either impossible by design or correctly managed.
  • If only one init.ts is truly expected per trigger directory, consider adding a guard to warn or error when a second instance is found.
  • If multiple instances are valid, ensure the downstream logic can gracefully handle them without unintended side effects.

Please review and adjust the logic accordingly if necessary.

packages/core/test/taskExecutor.test.ts (19)

1-24: Test suite initialization
Imports and setup appear standard. Using Vitest is a good choice; the initial lifecycle hooks re-instantiation ensures consistent test isolation.


25-66: onComplete hook test
Thorough check ensures final output is correct, including the aggregated init data. Looks well-structured for success scenarios.


68-172: onSuccess hooks ordering
This test effectively confirms multiple success hooks (global + task) are called in registration order, capturing essential coverage.


174-268: onStart hooks ordering
Verifies that hooks run in sequence before the run function. Good approach ensuring the correct payload and initialization data.


270-375: onFailure hooks
Ensures the error is propagated to all hooks in correct order. This covers essential error handling flows.


377-484: onComplete sequence for successful run
Demonstrates final completion ordering with correct data. Confirms that completion hooks see accurate success results.


486-574: onComplete when task fails
Validates hooking into the post-run phase even when there’s an error. This ensures reliable cleanup logic.


576-674: catchError hooks in order
Tests that the error cascades until a handler returns a modification. Correctly stops after an effective catch.


675-730: skipRetrying logic
This test ensures that a catch error hook can skip further retries, adjusting the final reported error if necessary.


732-785: Custom retry timing
Checks for specific scheduling of re-execution. The test thoroughly validates retryAt usage.


787-901: Middleware ordering
Great test ensuring global and task-specific middlewares wrap the run step in the intended layered sequence.


959-1035: Handling errors from init hooks
Verifies that a failing init stops the run and triggers onFailure. Good coverage for early errors.


1037-1113: Handling errors from task init hooks
Ensures initial global hooks succeed, but failing task-specific hooks block the run function. Nicely tested.


1115-1204: Propagation of start hook errors
Correctly tests partial initialization success, with start hook failure preventing run.


1205-1328: Cleanup hooks after success
Verifies the order in which cleanup runs and ensures it occurs before middleware’s final exit.


1330-1416: Cleanup hooks on error
Checks that all cleanup hooks still execute even though the task fails. This ensures robust resource cleanup.


1418-1503: Handling max duration abort
Simulates manual abort, verifying partial steps and correct error injection. Good coverage of timeouts.


1505-1623: onWait and onResume hooks
Ensures pause/resume logic is validated. The hooking for wait states is precisely tested, including data re-check.


1626-1716: executeTask helper
Functionally orchestrates the entire run within tests. Clear to read and maintain.

packages/core/src/v3/locals-api.ts (1)

1-30: Comprehensive locals API
This new module provides a straightforward API for creating, retrieving, and setting local variables. The typed LocalsKey ensures type safety. Overall, it finely encapsulates local state management.

packages/core/src/v3/config.ts (3)

5-11: Imports are consistent with the new lifecycle hooks.
These imported types align seamlessly with the updated lifecycle architecture. Please ensure all relevant call sites are updated accordingly.


12-12: New import of LogLevel.
This appears to be a straightforward addition. No concerns.


219-222:

❓ Verification inconclusive

Deprecation of init in favor of tasks.init.
Good callout. Confirm that any references to init in your project are replaced or accounted for in tests.

Run the following script to locate potential references:


🏁 Script executed:

#!/bin/bash
ast-grep --pattern $'config.init'

Length of output: 2291


Action Required: Verify/Update Deprecated init Usage

The init property in packages/core/src/v3/config.ts is deprecated in favor of tasks.init. However, I located references to config.init in the following files:

  • packages/cli-v3/src/entryPoints/managed-run-worker.ts (lines 198 & 201)
  • packages/cli-v3/src/entryPoints/dev-run-worker.ts (lines 188 & 191)

Please review these occurrences to determine if they should be updated to use tasks.init or if they are being maintained for backward compatibility. In either case, ensure that these paths are covered by tests.

packages/cli-v3/src/entryPoints/managed-run-worker.ts (14)

4-8: Addition of lifecycle hook function imports.
These newly imported types reflect the expanded hook coverage. Looks good.


9-9: Refined import of apiClientManager.
No issues.


11-11: Import of ExecutorToWorkerMessageCatalog.
This aligns with the message passing schema. No concerns.


13-14: Imports for lifecycle and locals API.
Both references appear consistent with the new architecture.


18-20: Added references to runMetadata, runtime, and runTimelineMetrics.
These imports integrate the new runtime monitoring approach. Looks correct.


24-24: Import of TriggerConfig.
No issues.


26-27: Import of WorkerManifest and WorkerToExecutorMessageCatalog.
Helps parse worker configuration and handle inter-process messages.


105-106: Global locals manager registration.
This correctly sets up a shared, standard locals manager.


107-110: Global lifecycle hooks manager registration.
Centralizing hooks in standardLifecycleHooksManager is consistent with the new design.


198-203: Registering global init hook.
config.init is now hooked globally. Deprecation is clearly indicated in config but used conditionally here. Confirm usage is tested.


205-210: Registering global onStart hook.
Ensures consistent pre-run logic across all tasks. All good.


212-217: Registering global onSuccess hook.
Properly extends the success path with a global callback.


219-224: Registering global onFailure hook.
Ensures universal handling of run failures. No issues.


226-231: Registering global handleError as a catch error hook.
Allows for a single place to handle thrown errors. Implementation looks good.

packages/cli-v3/src/entryPoints/dev-run-worker.ts (12)

4-8: New lifecycle hook function imports.
These new imports align with the hook-based design introduced in core/v3.


13-14: Added lifecycle and locals API imports.
They’re essential for the updated hooks and local data registration.


17-17: Import of resourceCatalog.
Complements the new approach to track tasks. No issues.


20-20: Bringing in runTimelineMetrics.
Supports advanced timeline tracking for tasks.


24-24: Import of TriggerConfig.
No concerns.


26-27: Imports for worker manifest and message catalogs.
Ensures correct dev-run message handling.


39-40: Added StandardLifecycleHooksManager and StandardLocalsManager.
Further consolidates the new lifecycle and local usage approach in dev-mode.


43-43: Added StandardRunTimelineMetricsManager.
Allows dev-run worker to track run metrics consistently.


269-269: Extracted bootstrap results.
Unpacks the new lifecycle logic objects from bootstrap(). Implementation looks fine.


309-320: Optional loading of initEntryPoint.
Including this logic supports extra user-defined initialization. The try-catch with error logging is a good safeguard.


385-385: Configuring the retries option in TaskExecutor.
Using config.retries ensures the dev-run consistently applies global retry settings.


403-403: Direct call to executor.execute.
Cleaner approach for handling the maxDuration signal. Properly leverages the new design pattern.

packages/core/src/v3/workers/taskExecutor.ts (20)

5-19: Imports for error and hook management look appropriate.
They centralize error parsing and lifecycle handling, which aids in structured error reporting and consistent hook usage.


34-35: Dependency consolidation observation:
Using tryCatch from a central utility is a good approach to standardize error handling across the codebase.


59-65: Initialization of _retries property
Storing the retry configuration paves the way for adjustable retry logic. This is helpful for local vs. production runs.


73-73: Constructor assignment for _retries
Assigning this._retries in the constructor is consistent with other fields. Make sure it's tested for edge cases like undefined values.


112-129: Payload parse block
Using tryCatch around payload parsing is a solid approach. It properly captures and records exceptions and aligns with your structured error handling.


130-136: Registration of wait/resume hooks
Hooking into onWait and onResume here promotes better event-driven control. Good design choice to integrate these at the earliest stage of the task.


200-261: Output serialization and completion logic
The sequence of stringifying output, exporting it, and updating span attributes is consistent. Nicely ensures trace data captures final output details. If called frequently in short intervals, watch for any performance overhead from repeated exports.


303-353: #executeTaskWithMiddlewareHooks
This layered approach (reduceRight + next) is effective for stackable middleware. The race condition between the runner and executeError is handled well. Just ensure each hook guards against unexpected side effects in shared state.


395-463: #callOnWaitFunctions
Running each global hook in a separate try-catch with tryCatch is correct. If any hook fails, we abort the entire flow, which is consistent.


465-533: #callOnResumeFunctions
Same pattern as onWait. Ensuring symmetrical logic fosters clear reentry points for paused or scheduled tasks.


535-629: #callInitFunctions
Merging multiple global hook results into a single object is practical. Watch for conflicts where multiple hooks return the same property name.


631-702: #callOnSuccessFunctions
Sequentially running success hooks is standard. No concerns here except verifying it does not mask upstream errors.


704-775: #callOnFailureFunctions
Registers meaningful spans for each global and task-level failure hook, enabling in-depth debugging. Looks good.


777-787: #parsePayload
Raising TaskPayloadParsedError on parse failure is consistent with your error model. Clear separation of concerns.


853-861: #cleanupAndWaitUntil
Running cleanup hooks before forcing the wait is well ordered. This ensures leftover resources are released promptly.


863-931: #callCleanupFunctions
Implementation matches the onStart/onWait pattern. This is consistent with your overall design, no issues spotted.


952-1067: #handleError
Logic for skipping retries in certain error conditions is thorough. The fallback to default retry settings is clearly documented. Good separation of local vs. global hooks.


1069-1117: #processHandleErrorResult
Consolidating skip/retry/noop statuses is neatly handled. This makes it easy to maintain or add new statuses.


1119-1190: #callOnCompleteFunctions
Invoking completion hooks after success/failure ensures finalization logic runs consistently. Good design.


1192-1209: #internalErrorResult
Wrapping internal errors under a known shape helps unify error responses. This is crucial for debugging.

packages/trigger-sdk/src/v3/shared.ts (7)

11-16: Utility imports
flattenIdempotencyKey, getEnvVar, and lifecycleHooks usage shows a cohesive approach for user environment, keys, and hooking. No immediate issues.


49-52: Reintroduced or updated batch references
These lines appear to re-export batch-related types. Confirm they align with your new batch logic introduced in the codebase.


62-63: Extended types
BatchTriggerAndWaitOptions and BatchTriggerTaskV2RequestBody are reintroduced or extended. Double-check internal references for correctness.


131-135: Refined generics in createTask()
The typed usage of TIdentifier, TInput, TOutput, TInitOutput helps maintain strict type safety. Good improvement.


198-199: registerTaskLifecycleHooks usage
Calling registerTaskLifecycleHooks ensures consistent hooking for tasks. Great integration point.


325-326: Refined generics in createSchemaTask()
Similar to createTask(), providing typed usage for schema tasks helps ensure typed payload parsing.


1564-1637: New function registerTaskLifecycleHooks
This function systematically registers all possible lifecycle hooks (init, start, failure, etc.). It is well-structured and future-proof for additional hooks. Ensure that each branch has test coverage to avoid silent no-ops if a parameter is misconfigured.

packages/trigger-sdk/src/v3/hooks.ts (10)

1-14: Imports from @trigger.dev/core/v3
All relevant hook types are consolidated from core. This is a neat separation of concerns, ensuring the SDK reuses the core hooking logic.


30-40: onStart
Provides both named and unnamed overloads. This dual signature approach is helpful for hooking with or without IDs.


42-52: onFailure
Uses the same pattern as onStart. Consistency helps devs quickly grasp usage.


54-64: onSuccess
The same design pattern for lifecycle hooks. LGTM.


66-76: onComplete
Completes the standard set of global event hooks. The fallback naming logic (fn.name) is convenient.


78-85: onWait
Same approach as other hooks—straightforward. No concerns.


87-97: onResume
Parallel structure to onWait. Easy to follow.


99-110: onHandleError (deprecated)
Deprecation strategy is well handled here. Delegating to onCatchError is a clean approach that avoids code duplication.


111-121: onCatchError
Registration in lifecycleHooks.registerGlobalCatchErrorHook clarifies global error handling. Ensure usage is well documented for advanced error scenarios.


123-133: middleware
Provides layering for hooking into the execution pipeline. This is highly extensible.

packages/core/src/v3/types/tasks.ts (15)

3-15: New lifecycle hooks imports
All referenced imports appear consistent with usage in this file. This integration of hook types looks well structured and aligns with the broader lifecycle enhancements.


26-26: No issues with new import
This import statement for QueueOptions is straightforward and doesn't introduce any obvious conflicts.


29-29: Prettify import
Importing Prettify as a helper type is a neat approach to simplifying complex TypeScript type definitions.


110-110: Optional init property
Adding init?: InitOutput to StartFnParams seamlessly integrates the initialization output into the start parameters.


142-142: Revising 'init' to a structured type
Using Record<string, unknown> clarifies what shape the init data can take, improving maintainability and type safety.


275-275: Deprecation note for init
Documenting the move from init to newer mechanisms ensures developers know to favor the more modern lifecycle or middleware approach.


278-278: Switching to OnInitHookFunction
Replacing a generic function callback with OnInitHookFunction<TPayload, TInitOutput> enhances type clarity for the init phase.


282-285: Deprecation note for cleanup
By suggesting middleware for cleanup operations, you're helping isolate cross-cutting concerns. This is a simpler, more compositional approach.


289-293: Deprecating handleError
Recommending the catchError hook clarifies intent and standardizes error-handling terminology across the codebase.


297-297: New catchError property
A dedicated catchError property distinctly separates error handling from the normal run flow, improving code readability.


299-301: Introducing onResume, onWait, onComplete
These hooks are great additions for controlling lifecycle transitions and final states, providing fine-grained flexibility.


320-320: middleware property
Defining a separate middleware function is an excellent way to address cross-cutting concerns without cluttering the primary run logic.


325-325: onStart hook
Allowing custom logic to run at the earliest stage unlocks more sophisticated control over initialization or startup actions.


330-330: onSuccess hook
Provides a clean place to perform follow-up actions and ensures the main run code path stays focused on business logic.


335-335: onFailure hook
Centralizing final failure handling at the lifecycle level avoids scattering error logic and helps maintain consistent cleanup strategies.

packages/core/src/v3/lifecycleHooks/types.ts (1)

1-311: Comprehensive lifecycle hooks introduction
This file provides a robust, well-typed framework for managing every stage of a task's lifecycle, from initialization and middleware to wait/resume and final cleanup. Each hook type cleanly separates concerns, and references to TaskRunContext and typed parameters help ensure correctness. Overall, this design is both flexible and maintainable.

🧰 Tools
🪛 Biome (1.9.4)

[error] 4-4: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 15-15: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 32-32: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 69-69: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 87-87: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 105-105: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 128-128: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 165-165: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 229-229: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

@ericallam ericallam force-pushed the revamped-lifecycle-hooks branch from 1c36b9c to 268c9f6 Compare March 25, 2025 15:55
Comment on lines 31 to 32
console.log("spanName", spanName, name);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericallam 👀

// Check if the entry point is an init.ts file at the root of a trigger directory
function isInitEntryPoint(entryPoint: string): boolean {
const normalizedEntryPoint = entryPoint.replace(/\\/g, "/"); // Normalize path separators
const initFileName = "init.ts";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we support js jsx and tsx files?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/core/src/v3/lifecycleHooks/index.ts (1)

45-245: 🛠️ Refactor suggestion

Reduce repetitive hook registration code.

The implementation contains significant duplication with nearly identical patterns for different hook types. This could make maintenance challenging if behavior needs to change.

Consider introducing a generic helper function to reduce repetition:

private createHookMethods<T>(hookType: string): {
  registerGlobal: (hook: RegisterHookFunctionParams<T>) => void;
  registerTask: (taskId: string, hook: RegisterHookFunctionParams<T>) => void;
  getTask: (taskId: string) => T | undefined;
  getGlobal: () => RegisteredHookFunction<T>[];
} {
  return {
    registerGlobal: (hook: RegisterHookFunctionParams<T>) => {
      this.#getManager()[`registerGlobal${hookType}Hook`](hook);
    },
    registerTask: (taskId: string, hook: RegisterHookFunctionParams<T>) => {
      this.#getManager()[`registerTask${hookType}Hook`](taskId, hook);
    },
    getTask: (taskId: string) => {
      return this.#getManager()[`getTask${hookType}Hook`](taskId);
    },
    getGlobal: () => {
      return this.#getManager()[`getGlobal${hookType}Hooks`]();
    }
  };
}

This would allow you to set up all hook methods more concisely in the constructor.

packages/core/test/taskExecutor.test.ts (1)

913-915: ⚠️ Potential issue

Remove unreachable code after throw statement

The code at lines 914-915 is unreachable because execution will be halted by the throw statement at line 912.

        throw expectedError;
-        // Should never get here
-        await next();
-        executionOrder.push("middleware-after");
🧰 Tools
🪛 Biome (1.9.4)

[error] 914-915: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

🧹 Nitpick comments (22)
references/hello-world/src/trigger/example.ts (1)

151-194: New task successfully demonstrates lifecycle hooks functionality.

The hooksTask effectively showcases the new lifecycle hooks system with implementations of all available hooks. This serves as good documentation for developers learning how to use the new features.

Note that according to the PR objectives, init and cleanup hooks are marked as deprecated and should be replaced with middleware and locals. Consider adding a comment indicating these are deprecated to avoid misleading new users.

  init: async () => {
+   // @deprecated Use middleware instead
    return {
      foobar: "baz",
    };
  },
  ...
  cleanup: async ({ ctx, payload }) => {
+   // @deprecated Use middleware instead
    logger.info("Hello, world from the cleanup hook", { payload });
  },
packages/core/src/v3/locals/index.ts (1)

1-45: Well-structured locals management implementation.

The LocalsAPI class provides a clean singleton implementation for managing local variables across the application. The approach using the global registry pattern with fallback to a no-operation manager is robust.

However, there are a few issues flagged by static analysis:

  1. Using this in static methods can be confusing. Replace with the class name:
  public static getInstance(): LocalsAPI {
-   if (!this._instance) {
-     this._instance = new LocalsAPI();
-   }
-   return this._instance;
+   if (!LocalsAPI._instance) {
+     LocalsAPI._instance = new LocalsAPI();
+   }
+   return LocalsAPI._instance;
  }
  1. Void function returning a value:
  public setLocal<T>(key: LocalsKey<T>, value: T): void {
-   return this.#getManager().setLocal(key, value);
+   this.#getManager().setLocal(key, value);
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 16-16: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 39-39: The function should not return a value because its return type is void.

The function is here:

'void' signals the absence of value. The returned value is likely to be ignored by the caller.

(lint/correctness/noVoidTypeReturn)

packages/core/src/v3/lifecycleHooks/index.ts (1)

30-35: Fix static method reference.

Using this in static methods can be confusing. Consider replacing with the class name for clarity.

  public static getInstance(): LifecycleHooksAPI {
-   if (!this._instance) {
-     this._instance = new LifecycleHooksAPI();
-   }
-   return this._instance;
+   if (!LifecycleHooksAPI._instance) {
+     LifecycleHooksAPI._instance = new LifecycleHooksAPI();
+   }
+   return LifecycleHooksAPI._instance;
  }
🧰 Tools
🪛 Biome (1.9.4)

[error] 30-30: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 34-34: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

packages/core/src/v3/locals/types.ts (2)

1-2: Consider adding documentation for your type branding approach.
Declaring __local as a unique symbol is a clever way to create a brand type, but adding a brief comment explaining its purpose can improve clarity for future maintainers.


10-14: Consider supporting a method to remove or clear locals.
The LocalsManager interface currently allows creation, retrieval, and setting of local values. In some scenarios, it may be useful to remove or reset a local key.

packages/core/src/v3/locals/manager.ts (3)

3-16: Maintain consistent symbol usage.
While using plain Symbol() is fine for a no-op manager, consider using the same approach (Symbol.for) as in the standard manager to ensure consistent global registration or to clarify the reason behind using ephemeral symbols here.


18-36: Avoid potential collisions with Symbol.for(id).
Reuse of the same identifier in Symbol.for from different parts of the codebase could lead to unexpected overlaps. Adding a namespace prefix to id or a more descriptive approach could mitigate collisions.


37-37: Remove the extraneous literal.
The 0; line at the end of the file appears to be unused. Removing it can declutter the code.

-0;
packages/trigger-sdk/src/v3/hooks.ts (4)

30-40: Consider removing void from the return type.
The return type undefined | void | Promise<undefined | void> can be simplified, as using void inside a union type is flagged by static analysis.

Here’s a possible diff:

-export function onStart(name: string, fn: AnyOnStartHookFunction): void;
-export function onStart(fn: AnyOnStartHookFunction): void;
-export function onStart(
-  fnOrName: string | AnyOnStartHookFunction,
-  fn?: AnyOnStartHookFunction
-): void {
+export function onStart(name: string, fn: AnyOnStartHookFunction): undefined;
+export function onStart(fn: AnyOnStartHookFunction): undefined;
+export function onStart(
+  fnOrName: string | AnyOnStartHookFunction,
+  fn?: AnyOnStartHookFunction
+): undefined {
   lifecycleHooks.registerGlobalStartHook({
     id: typeof fnOrName === "string" ? fnOrName : fnOrName.name ? fnOrName.name : undefined,
     fn: typeof fnOrName === "function" ? fnOrName : fn!,
   });
 }

42-52: onFailure hook consistency.
Function overloads look consistent. Consider similarly removing void from the return type to align with the static analysis recommendation.


54-64: onSuccess hook pattern.
You’re correctly applying the same overload pattern as onStart and onFailure. Removing void in union types would further align with best practices.


66-133: Consolidate return type usage and handle ID collisions.

  • Return type: Several hooks (onComplete, onWait, onResume, onHandleError, onCatchError, middleware) also feature void in union types, which is discouraged by static analysis.
  • ID collisions: If multiple hooks have no explicit name and blank function names, the hook id can end up undefined, potentially overwriting or failing to register properly. Consider providing a fallback or requiring a non-empty string for the name in user-facing docs.
packages/core/src/v3/lifecycleHooks/types.ts (10)

4-5: TaskInitOutput usage of void in union types.
Static analysis flags void as confusing in union types. Switching to undefined alone is typically clearer.

-export type TaskInitOutput = Record<string, any> | void | undefined;
+export type TaskInitOutput = Record<string, any> | undefined;
🧰 Tools
🪛 Biome (1.9.4)

[error] 4-4: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


13-15: OnInitHookFunction return type.
The union type includes void. As above, consider dropping void from the union.

🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


19-35: TaskStartHookParams and OnStartHookFunction.
Similar pattern of union return with void. Consider removing it to match best practices.

🧰 Tools
🪛 Biome (1.9.4)

[error] 32-32: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


55-71: OnWaitHookFunction.
Again, returning undefined | void is flagged by the linter. Removing void clarifies your API.

🧰 Tools
🪛 Biome (1.9.4)

[error] 69-69: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


73-90: OnResumeHookFunction.
Identical pattern regarding union with void. Otherwise, the function definition is consistent.

🧰 Tools
🪛 Biome (1.9.4)

[error] 87-87: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


91-108: OnFailureHookFunction.
Logic for passing the error is appropriate. Consider the same improvement for the return type.

🧰 Tools
🪛 Biome (1.9.4)

[error] 105-105: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


109-131: OnSuccessHookFunction.
The design is consistent. Watch for void usage in union types.

🧰 Tools
🪛 Biome (1.9.4)

[error] 128-128: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


146-168: OnCompleteHookFunction return type.
Static analysis likewise flags void in unions.

🧰 Tools
🪛 Biome (1.9.4)

[error] 165-165: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


180-201: OnCatchErrorHookFunction definition.
This is a neat way to unify error modification and retry logic. All good, aside from the recurring “void” union.


216-232: OnCleanupHookFunction.
Deprecation or not, it is consistent with the rest. Watch for union usage of void.

🧰 Tools
🪛 Biome (1.9.4)

[error] 229-229: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c36b9c and 887c5ca.

📒 Files selected for processing (38)
  • .changeset/weak-jobs-hide.md (1 hunks)
  • .cursor/rules/executing-commands.mdc (1 hunks)
  • apps/webapp/app/components/runs/v3/RunIcon.tsx (3 hunks)
  • internal-packages/run-engine/src/engine/errors.ts (1 hunks)
  • packages/cli-v3/src/build/bundle.ts (5 hunks)
  • packages/cli-v3/src/entryPoints/dev-index-worker.ts (1 hunks)
  • packages/cli-v3/src/entryPoints/dev-run-worker.ts (8 hunks)
  • packages/cli-v3/src/entryPoints/managed-index-worker.ts (1 hunks)
  • packages/cli-v3/src/entryPoints/managed-run-worker.ts (7 hunks)
  • packages/core/src/v3/config.ts (2 hunks)
  • packages/core/src/v3/errors.ts (1 hunks)
  • packages/core/src/v3/index.ts (1 hunks)
  • packages/core/src/v3/lifecycle-hooks-api.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/index.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/manager.ts (1 hunks)
  • packages/core/src/v3/lifecycleHooks/types.ts (1 hunks)
  • packages/core/src/v3/locals-api.ts (1 hunks)
  • packages/core/src/v3/locals/index.ts (1 hunks)
  • packages/core/src/v3/locals/manager.ts (1 hunks)
  • packages/core/src/v3/locals/types.ts (1 hunks)
  • packages/core/src/v3/runtime/managedRuntimeManager.ts (4 hunks)
  • packages/core/src/v3/schemas/build.ts (2 hunks)
  • packages/core/src/v3/schemas/common.ts (1 hunks)
  • packages/core/src/v3/semanticInternalAttributes.ts (1 hunks)
  • packages/core/src/v3/tracer.ts (1 hunks)
  • packages/core/src/v3/types/tasks.ts (6 hunks)
  • packages/core/src/v3/utils/globals.ts (2 hunks)
  • packages/core/src/v3/workers/index.ts (1 hunks)
  • packages/core/src/v3/workers/taskExecutor.ts (11 hunks)
  • packages/core/test/taskExecutor.test.ts (1 hunks)
  • packages/trigger-sdk/src/v3/hooks.ts (1 hunks)
  • packages/trigger-sdk/src/v3/index.ts (1 hunks)
  • packages/trigger-sdk/src/v3/locals.ts (1 hunks)
  • packages/trigger-sdk/src/v3/shared.ts (6 hunks)
  • packages/trigger-sdk/src/v3/tasks.ts (3 hunks)
  • references/hello-world/src/db.ts (1 hunks)
  • references/hello-world/src/trigger/example.ts (2 hunks)
  • references/hello-world/src/trigger/init.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .changeset/weak-jobs-hide.md
🚧 Files skipped from review as they are similar to previous changes (23)
  • packages/core/src/v3/semanticInternalAttributes.ts
  • packages/cli-v3/src/entryPoints/managed-index-worker.ts
  • packages/core/src/v3/errors.ts
  • .cursor/rules/executing-commands.mdc
  • packages/trigger-sdk/src/v3/locals.ts
  • packages/core/src/v3/utils/globals.ts
  • packages/trigger-sdk/src/v3/index.ts
  • packages/core/src/v3/schemas/common.ts
  • packages/core/src/v3/index.ts
  • internal-packages/run-engine/src/engine/errors.ts
  • references/hello-world/src/trigger/init.ts
  • packages/core/src/v3/workers/index.ts
  • packages/core/src/v3/tracer.ts
  • apps/webapp/app/components/runs/v3/RunIcon.tsx
  • packages/core/src/v3/schemas/build.ts
  • packages/core/src/v3/lifecycleHooks/manager.ts
  • packages/cli-v3/src/entryPoints/dev-index-worker.ts
  • packages/core/src/v3/config.ts
  • packages/cli-v3/src/build/bundle.ts
  • references/hello-world/src/db.ts
  • packages/core/src/v3/lifecycle-hooks-api.ts
  • packages/cli-v3/src/entryPoints/dev-run-worker.ts
  • packages/trigger-sdk/src/v3/shared.ts
🧰 Additional context used
🧬 Code Definitions (10)
references/hello-world/src/trigger/example.ts (4)
packages/trigger-sdk/src/v3/index.ts (1)
  • logger (36-36)
packages/trigger-sdk/src/v3/tasks.ts (1)
  • task (80-80)
packages/core/src/v3/workers/taskExecutor.ts (12)
  • payload (302-353)
  • payload (355-393)
  • payload (535-629)
  • payload (631-702)
  • payload (704-775)
  • payload (777-787)
  • payload (789-851)
  • payload (853-861)
  • payload (863-931)
  • payload (1119-1190)
  • wait (395-463)
  • wait (465-533)
packages/trigger-sdk/src/v3/wait.ts (1)
  • wait (138-293)
packages/core/src/v3/locals/types.ts (3)
packages/core/src/v3/locals-api.ts (1)
  • LocalsKey (29-29)
packages/trigger-sdk/src/v3/locals.ts (1)
  • LocalsKey (3-3)
packages/core/src/v3/locals/index.ts (1)
  • LocalsManager (42-44)
packages/trigger-sdk/src/v3/tasks.ts (1)
packages/trigger-sdk/src/v3/hooks.ts (2)
  • onHandleError (104-109)
  • onCatchError (113-121)
packages/core/src/v3/workers/taskExecutor.ts (4)
packages/core/src/v3/schemas/schemas.ts (2)
  • RetryOptions (99-126)
  • RetryOptions (128-128)
packages/core/src/v3/lifecycleHooks/types.ts (3)
  • RegisteredHookFunction (174-178)
  • AnyOnMiddlewareHookFunction (214-214)
  • TaskWait (36-53)
packages/core/src/v3/errors.ts (1)
  • InternalError (18-44)
packages/core/src/v3/types/tasks.ts (1)
  • HandleErrorModificationOptions (128-134)
packages/trigger-sdk/src/v3/hooks.ts (2)
packages/core/src/v3/lifecycleHooks/types.ts (8)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
packages/core/src/v3/lifecycle-hooks-api.ts (9)
  • AnyOnStartHookFunction (14-14)
  • lifecycleHooks (5-5)
  • AnyOnFailureHookFunction (16-16)
  • AnyOnSuccessHookFunction (18-18)
  • AnyOnCompleteHookFunction (20-20)
  • AnyOnWaitHookFunction (22-22)
  • AnyOnResumeHookFunction (24-24)
  • AnyOnCatchErrorHookFunction (26-26)
  • AnyOnMiddlewareHookFunction (29-29)
packages/core/src/v3/types/tasks.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (10)
  • OnInitHookFunction (13-15)
  • OnCleanupHookFunction (227-229)
  • OnCatchErrorHookFunction (195-198)
  • OnResumeHookFunction (85-87)
  • OnWaitHookFunction (67-69)
  • OnCompleteHookFunction (159-165)
  • OnMiddlewareHookFunction (210-212)
  • OnStartHookFunction (30-32)
  • OnSuccessHookFunction (122-128)
  • OnFailureHookFunction (103-105)
packages/core/src/v3/lifecycleHooks/types.ts (4)
packages/core/src/v3/lifecycle-hooks-api.ts (4)
  • TaskInitHookParams (11-11)
  • OnInitHookFunction (8-8)
  • AnyOnInitHookFunction (9-9)
  • TaskWait (34-34)
packages/core/src/v3/types/tasks.ts (1)
  • HandleErrorResult (136-140)
packages/core/src/v3/lifecycleHooks/index.ts (1)
  • LifecycleHooksManager (263-265)
packages/core/src/v3/workers/taskExecutor.ts (2)
  • wait (395-463)
  • wait (465-533)
packages/core/src/v3/lifecycleHooks/index.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (14)
  • LifecycleHooksManager (233-310)
  • RegisterHookFunctionParams (169-172)
  • AnyOnInitHookFunction (17-17)
  • RegisteredHookFunction (174-178)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
  • AnyOnCleanupHookFunction (231-231)
  • TaskWait (36-53)
packages/core/src/v3/locals/index.ts (3)
packages/core/src/v3/locals/manager.ts (1)
  • NoopLocalsManager (3-16)
packages/core/src/v3/locals/types.ts (2)
  • LocalsManager (10-14)
  • LocalsKey (5-8)
packages/core/src/v3/utils/globals.ts (3)
  • registerGlobal (19-35)
  • unregisterGlobal (43-49)
  • getGlobal (37-41)
packages/core/test/taskExecutor.test.ts (5)
packages/core/src/v3/lifecycle-hooks-api.ts (1)
  • lifecycleHooks (5-5)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
  • StandardLifecycleHooksManager (18-397)
packages/core/src/v3/workers/taskExecutor.ts (13)
  • payload (302-353)
  • payload (355-393)
  • payload (535-629)
  • payload (631-702)
  • payload (704-775)
  • payload (777-787)
  • payload (789-851)
  • payload (853-861)
  • payload (863-931)
  • payload (1119-1190)
  • result (1070-1117)
  • execution (952-1067)
  • execution (1192-1208)
packages/core/src/v3/types/tasks.ts (2)
  • RunFnParams (86-93)
  • TaskMetadataWithFunctions (871-887)
packages/core/src/v3/schemas/common.ts (4)
  • TaskRunErrorCodes (195-195)
  • TaskRunErrorCodes (196-196)
  • TaskRunExecution (302-316)
  • TaskRunExecution (318-318)
🪛 Biome (1.9.4)
packages/core/src/v3/lifecycleHooks/types.ts

[error] 4-4: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 15-15: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 32-32: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 69-69: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 87-87: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 105-105: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 128-128: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 165-165: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)


[error] 229-229: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

packages/core/src/v3/lifecycleHooks/index.ts

[error] 30-30: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 31-31: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 34-34: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

packages/core/src/v3/locals/index.ts

[error] 15-15: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 16-16: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 19-19: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)


[error] 39-39: The function should not return a value because its return type is void.

The function is here:

'void' signals the absence of value. The returned value is likely to be ignored by the caller.

(lint/correctness/noVoidTypeReturn)

packages/core/test/taskExecutor.test.ts

[error] 914-915: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: units / 🧪 Unit Tests
🔇 Additional comments (42)
references/hello-world/src/trigger/example.ts (1)

7-8: Appropriate logging added.

Additional logging at the beginning of the task execution is helpful for debugging. This provides visibility into the context and payload being received.

packages/core/src/v3/lifecycleHooks/index.ts (1)

247-261: Consistent hook listener management.

The wait and resume hook listener methods are well-implemented and provide a clean way to register callback functions for these events.

packages/trigger-sdk/src/v3/tasks.ts (3)

1-11: New hooks imported correctly.

The addition of the new hook imports aligns with the PR objectives to enhance the lifecycle hooks system.


60-60: Type export enhances API discoverability.

Re-exporting all types from the hooks module is a good practice that improves API discoverability and makes it easier for developers to access the necessary types.


92-101: Updated tasks object with clear deprecation notice.

The tasks object has been properly updated with the new hook methods. The deprecation notice for handleError is well-documented, making it clear that developers should use catchError instead, which aligns with the PR objectives.

packages/core/src/v3/runtime/managedRuntimeManager.ts (5)

56-59: Same error handling concern applies here.


84-88: Same error handling concern applies here.


92-96: Same error handling concern applies here.


117-127: Same error handling concern applies here.


131-141: Same error handling concern applies here.

packages/core/test/taskExecutor.test.ts (2)

16-43: Comprehensive test suite for TaskExecutor lifecycle hooks

This test file provides excellent coverage of the new lifecycle hooks system, testing all the hook types in various scenarios. The setup with StandardLifecycleHooksManager ensures tests run in isolation.


1626-1716: Well-structured test helper function

The executeTask helper function encapsulates all the necessary setup for testing, making the individual test cases easier to read and maintain. It properly initializes the tracing, console interception, and execution context.

packages/core/src/v3/locals-api.ts (1)

1-29: Well-designed locals API with tree-shaking support

This new API provides a clean, type-safe interface for managing local variables with complete CRUD operations. The separation of the API instance and the interface allows for better tree-shaking as mentioned in the comments.

The getOrThrow method provides a convenient way to retrieve values with proper error handling, including the variable ID in the error message for easier debugging.

packages/cli-v3/src/entryPoints/managed-run-worker.ts (4)

4-14: Well-structured imports for enhanced lifecycle hooks system

The addition of these new hook types aligns with the PR objectives to enhance the lifecycle hooks system. These imports enable type-safe registration of various hook functions.


105-109: Good initialization of the new managers

Initializing StandardLocalsManager and setting it as the global manager ensures consistent state management across the application. This is a key part of the new locals system described in the PR objectives.


197-231: Clear implementation of global hook registration

This section properly registers global hooks from the configuration, maintaining backward compatibility while implementing the new hook system. The type assertions ensure type safety when registering hooks.


401-401: Simplified task execution with direct executor call

The execution logic has been streamlined by directly calling executor.execute with the necessary parameters, which aligns with the changes to the TaskExecutor class.

packages/core/src/v3/workers/taskExecutor.ts (7)

45-53: Enhanced TaskExecutorOptions with configurable retries

Adding the retries configuration to TaskExecutorOptions allows for more flexible retry behavior, including enabling retries in development mode. This is a valuable addition that improves task execution reliability.


112-126: Improved error handling with tryCatch utility

The use of the tryCatch utility creates a more structured approach to error handling. It simplifies error management by capturing errors in a standardized way, making the code more readable and maintainable.


130-136: Support for onWait and onResume hooks

The addition of registering listeners for onWait and onResume events enhances the task execution flow by allowing for better resource management during wait periods, as specified in the PR objectives.


138-261: Well-structured executeTask function

This refactoring encapsulates the task execution logic in a dedicated function with clear error handling paths. The structured approach to handling various error scenarios and success cases improves code maintainability.


302-353: Excellent middleware implementation with reducer pattern

The use of the reducer pattern to compose middleware functions is elegant and effective. It ensures middleware executes in the correct order, with proper nesting and error handling.


1119-1190: Comprehensive onComplete hook implementation

The implementation of the onComplete hooks follows the established pattern for lifecycle hooks, ensuring consistency across the codebase. This new hook provides a reliable way to execute code after task completion regardless of success or failure.


1070-1117: Clear error result processing

The processHandleErrorResult method centralizes the logic for interpreting error handler results, making the code more maintainable. It handles various retry options consistently, improving the reliability of error recovery.

packages/trigger-sdk/src/v3/hooks.ts (2)

1-14: Imports and re-exports look good.
No issues with the import statements or exported types.


16-28: Re-exporting types.
These lines concisely re-export the needed lifecycle hook types. No concerns.

packages/core/src/v3/types/tasks.ts (9)

3-15: Newly added lifecycle hooks in imports.
The introduction of lifecycle hook types aligns well with the new v4 hooking system. No immediate concerns.


26-29: Additional imports.
Importing QueueOptions and Prettify appears routine. No issues.


110-110: Optional init property.
Allowing init?: InitOutput in StartFnParams is consistent with the new approach. Confirm that all call sites handle the possibility of undefined.


275-277: init marked as deprecated.
The deprecation note is clear. This change is consistent with the shift toward locals and middleware.


285-285: cleanup is also deprecated.
As indicated, shifting to middleware ensures more flexible resource management.


289-293: handleError property is deprecated.
Replacing handleError with catchError is straightforward. Good job keeping the legacy property for backward compatibility.


294-297: New catchError property.
Provides a clear, consistent error-handling hook. Well aligned with the new lifecycle approach.


299-301: onResume, onWait, and onComplete hooks.
These additions give more granular control over tasks. Looks clear and type-safe.


320-335: middleware, onStart, onSuccess, onFailure hooks.
The introduction of these hook properties in CommonTaskOptions is consistent with the rest of the architecture.

packages/core/src/v3/lifecycleHooks/types.ts (7)

1-3: Import statements.
All relevant types are correctly imported from sibling modules.


6-11: TaskInitHookParams signature looks good.
Including payload, ctx, and a signal is aligned with typical initialization hooks.


36-54: TaskWait union.
The typed “wait” definitions are clear. This block is a good approach allowing multiple wait mechanisms.


132-145: TaskCompleteResult.
Returning either success or error is well-defined. No functional issues.


169-179: RegisterHookFunctionParams and RegisteredHookFunction.
Straightforward approach for storing metadata and function references.


202-215: TaskMiddlewareHookParams.
Including a next() function is standard practice for layered middleware. Good approach.


233-310: LifecycleHooksManager interface.
Comprehensive coverage for all hook registration and retrieval methods. The organization is clear and modular.

Comment on lines +48 to +51
await lifecycleHooks.callOnWaitHookListeners({
type: "task",
runId: params.id,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling around hook calls.
If callOnWaitHookListeners throws an error, it could block task processing. Consider wrapping these asynchronous hook calls in a try/catch block and decide whether to proceed, retry, or fail gracefully.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)

43-43: ⚠️ Potential issue

Fix unreachable code in spanNameIcon handling

There's a JSX element on line 43 that's created but never returned, making it unreachable code. This isn't related to your current changes but should be fixed.

- <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
+ return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
🧹 Nitpick comments (6)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)

79-100: Consider refactoring repetitive task hook cases

All these new hook cases return the same InformationCircleIcon with identical styling. Consider refactoring this repetitive pattern to improve maintainability.

case "task-middleware":
  return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
case "task-fn-run":
  return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-init":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onStart":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onSuccess":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onFailure":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onComplete":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onWait":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-onResume":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-catchError":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-cleanup":
-   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
+ case "task-hook-init":
+ case "task-hook-onStart":
+ case "task-hook-onSuccess":
+ case "task-hook-onFailure":
+ case "task-hook-onComplete":
+ case "task-hook-onWait":
+ case "task-hook-onResume":
+ case "task-hook-catchError":
+ case "task-hook-cleanup":
+   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;

Alternatively, consider a more scalable approach using a pattern matching technique:

+ // Handle all task hook cases
+ if (name?.startsWith("task-hook-")) {
+   return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
+ }

switch (name) {
  case "task":
    return <TaskIcon className={cn(className, "text-blue-500")} />;
  // ... other cases
  case "task-middleware":
    return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
  case "task-fn-run":
    return <InformationCircleIcon className={cn(className, "text-text-dimmed")} />;
- case "task-hook-init":
- case "task-hook-onStart":
- // ... other task-hook cases removed
packages/core/src/v3/lifecycleHooks/manager.ts (5)

69-79: Consider adding error handling for hook listeners.

The callOnWaitHookListeners and callOnResumeHookListeners methods use Promise.allSettled which prevents failures from propagating, but there's no logging or handling of rejected promises.

  async callOnWaitHookListeners(wait: TaskWait): Promise<void> {
-   await Promise.allSettled(this.onWaitHookListeners.map((listener) => listener(wait)));
+   const results = await Promise.allSettled(this.onWaitHookListeners.map((listener) => listener(wait)));
+   const rejections = results.filter((result): result is PromiseRejectedResult => result.status === 'rejected');
+   if (rejections.length > 0) {
+     console.warn(`${rejections.length} onWait hook listeners failed:`, rejections.map(r => r.reason));
+   }
  }

  async callOnResumeHookListeners(wait: TaskWait): Promise<void> {
-   await Promise.allSettled(this.onResumeHookListeners.map((listener) => listener(wait)));
+   const results = await Promise.allSettled(this.onResumeHookListeners.map((listener) => listener(wait)));
+   const rejections = results.filter((result): result is PromiseRejectedResult => result.status === 'rejected');
+   if (rejections.length > 0) {
+     console.warn(`${rejections.length} onResume hook listeners failed:`, rejections.map(r => r.reason));
+   }
  }

112-123: Consider documenting hook replacement behavior.

When registering hooks with the same ID multiple times, the newer hook will silently replace the older one due to the Map behavior. This might cause confusion if not documented.

  registerGlobalInitHook(hook: RegisterHookFunctionParams<AnyOnInitHookFunction>): void {
    // if there is no id, lets generate one based on the contents of the function
    const id = generateHookId(hook);

+   // If a hook with this ID already exists, it will be replaced
    const registeredHook = {
      id,
      name: hook.id,
      fn: hook.fn,
    };

    this.globalInitHooks.set(id, registeredHook);
  }

125-136: Inconsistent naming between registerTaskInitHook and other methods.

In registerTaskInitHook, the registered hook uses the taskId as the name property, while other hook registration methods use hook.id as the name. This inconsistency might lead to confusion.

  registerTaskInitHook(
    taskId: string,
    hook: RegisterHookFunctionParams<AnyOnInitHookFunction>
  ): void {
    const registeredHook = {
      id: generateHookId(hook),
-     name: taskId,
+     name: hook.id,
      fn: hook.fn,
    };

    this.taskInitHooks.set(taskId, registeredHook);
  }

18-397: Comprehensive lifecycle hooks management implementation.

The StandardLifecycleHooksManager class is well-structured and provides a complete implementation of all the lifecycle hooks required by the system. The consistent pattern used for each hook type enhances maintainability.

Consider adding a clear comment documenting the purpose of this class and how it fits into the overall architecture.

+/**
+ * StandardLifecycleHooksManager implements the LifecycleHooksManager interface
+ * providing a comprehensive solution for managing task lifecycle hooks.
+ * 
+ * It supports both global hooks (applied to all tasks) and task-specific hooks,
+ * along with special listener patterns for wait and resume events.
+ */
export class StandardLifecycleHooksManager implements LifecycleHooksManager {
  // existing implementation...
}

399-599: NoopLifecycleHooksManager provides a good fallback implementation.

The no-op implementation is useful for testing or as a fallback, but has inconsistent method signatures compared to the main implementation.

In addition to fixing the parameter types mentioned in previous comments, consider adding a class-level comment explaining when this implementation should be used.

+/**
+ * NoopLifecycleHooksManager provides a no-operation implementation of the
+ * LifecycleHooksManager interface. This can be used as a fallback when no
+ * lifecycle hook management is needed or for testing purposes.
+ */
export class NoopLifecycleHooksManager implements LifecycleHooksManager {
  // existing implementation with fixed parameter types...
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 887c5ca and 6d5d10a.

📒 Files selected for processing (3)
  • apps/webapp/app/components/runs/v3/RunIcon.tsx (2 hunks)
  • packages/cli-v3/src/build/bundle.ts (5 hunks)
  • packages/core/src/v3/lifecycleHooks/manager.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/cli-v3/src/build/bundle.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/core/src/v3/lifecycleHooks/manager.ts (1)
packages/core/src/v3/lifecycleHooks/types.ts (12)
  • AnyOnInitHookFunction (17-17)
  • AnyOnStartHookFunction (34-34)
  • AnyOnFailureHookFunction (107-107)
  • AnyOnSuccessHookFunction (130-130)
  • AnyOnCompleteHookFunction (167-167)
  • AnyOnWaitHookFunction (71-71)
  • AnyOnResumeHookFunction (89-89)
  • AnyOnCatchErrorHookFunction (200-200)
  • AnyOnMiddlewareHookFunction (214-214)
  • AnyOnCleanupHookFunction (231-231)
  • TaskWait (36-53)
  • RegisterHookFunctionParams (169-172)
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / 🧪 Unit Tests
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/runs/v3/RunIcon.tsx (1)

47-49: Great enhancement for dynamic icon rendering!

Adding support for dynamic icon lookup in the tablerIcons set makes the component more flexible and future-proof. This allows for rendering any icon that exists in the icon set without modifying the switch statement.

packages/core/src/v3/lifecycleHooks/manager.ts (3)

1-16: Well-structured imports with descriptive type names.

The imports are comprehensive and clearly named, providing good type safety for the lifecycle hooks implementation.


18-64: Excellent design with clear separation of concerns.

The StandardLifecycleHooksManager class implements a robust lifecycle hooks system with:

  • Separate maps for global and task-specific hooks
  • Dedicated storage for each hook type
  • Well-designed listeners for wait and resume events

This architecture allows for flexible hook registration and efficient lookup.


601-603: Simple and effective utility function for generating hook IDs.

The generateHookId function provides a convenient way to generate unique IDs for hooks, either using the provided ID or falling back to the function's string representation.

Comment on lines +565 to +579
registerGlobalMiddlewareHook(): void {
// Noop
}

registerTaskMiddlewareHook(): void {
// Noop
}

getTaskMiddlewareHook(): undefined {
return undefined;
}

getGlobalMiddlewareHooks(): [] {
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing parameter types in middleware hook methods.

Similar to the previous issue, the middleware hook methods also lack proper parameter types.

-  registerGlobalMiddlewareHook(): void {
+  registerGlobalMiddlewareHook(hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>): void {
    // Noop
  }

-  registerTaskMiddlewareHook(): void {
+  registerTaskMiddlewareHook(
+    taskId: string,
+    hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>
+  ): void {
    // Noop
  }

-  getTaskMiddlewareHook(): undefined {
+  getTaskMiddlewareHook(taskId: string): AnyOnMiddlewareHookFunction | undefined {
    return undefined;
  }

-  getGlobalMiddlewareHooks(): [] {
+  getGlobalMiddlewareHooks(): RegisteredHookFunction<AnyOnMiddlewareHookFunction>[] {
    return [];
  }
📝 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.

Suggested change
registerGlobalMiddlewareHook(): void {
// Noop
}
registerTaskMiddlewareHook(): void {
// Noop
}
getTaskMiddlewareHook(): undefined {
return undefined;
}
getGlobalMiddlewareHooks(): [] {
return [];
}
registerGlobalMiddlewareHook(hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>): void {
// Noop
}
registerTaskMiddlewareHook(
taskId: string,
hook: RegisterHookFunctionParams<AnyOnMiddlewareHookFunction>
): void {
// Noop
}
getTaskMiddlewareHook(taskId: string): AnyOnMiddlewareHookFunction | undefined {
return undefined;
}
getGlobalMiddlewareHooks(): RegisteredHookFunction<AnyOnMiddlewareHookFunction>[] {
return [];
}

Comment on lines +549 to +563
registerGlobalCatchErrorHook(): void {
// Noop
}

registerTaskCatchErrorHook(): void {
// Noop
}

getTaskCatchErrorHook(): undefined {
return undefined;
}

getGlobalCatchErrorHooks(): [] {
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing parameter types in NoopLifecycleHooksManager methods.

Several methods in the NoopLifecycleHooksManager class don't include proper parameter types, which could lead to type safety issues.

-  registerGlobalCatchErrorHook(): void {
+  registerGlobalCatchErrorHook(hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>): void {
    // Noop
  }

-  registerTaskCatchErrorHook(): void {
+  registerTaskCatchErrorHook(
+    taskId: string,
+    hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>
+  ): void {
    // Noop
  }

-  getTaskCatchErrorHook(): undefined {
+  getTaskCatchErrorHook(taskId: string): AnyOnCatchErrorHookFunction | undefined {
    return undefined;
  }

-  getGlobalCatchErrorHooks(): [] {
+  getGlobalCatchErrorHooks(): RegisteredHookFunction<AnyOnCatchErrorHookFunction>[] {
    return [];
  }
📝 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.

Suggested change
registerGlobalCatchErrorHook(): void {
// Noop
}
registerTaskCatchErrorHook(): void {
// Noop
}
getTaskCatchErrorHook(): undefined {
return undefined;
}
getGlobalCatchErrorHooks(): [] {
return [];
}
registerGlobalCatchErrorHook(hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>): void {
// Noop
}
registerTaskCatchErrorHook(
taskId: string,
hook: RegisterHookFunctionParams<AnyOnCatchErrorHookFunction>
): void {
// Noop
}
getTaskCatchErrorHook(taskId: string): AnyOnCatchErrorHookFunction | undefined {
return undefined;
}
getGlobalCatchErrorHooks(): RegisteredHookFunction<AnyOnCatchErrorHookFunction>[] {
return [];
}

@ericallam ericallam merged commit 82251e2 into main Mar 25, 2025
12 checks passed
@ericallam ericallam deleted the revamped-lifecycle-hooks branch March 25, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants