feat(sdk): replace maxDuration with maxComputeSeconds (user-facing rename) - #4436
feat(sdk): replace maxDuration with maxComputeSeconds (user-facing rename)#4436deepshekhardas wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 80cb649 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
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 |
|
Hi @deepshekhardas, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // Resolve maxComputeSeconds → maxDuration so plain-object exports that bypass | ||
| // defineConfig() still work. This mirrors the resolution defineConfig() applies | ||
| // at the SDK boundary; downstream CLI/runtime code only reads `maxDuration`. | ||
| const resolvedMaxDuration = config.maxComputeSeconds ?? config.maxDuration; | ||
| if (!resolvedMaxDuration) { | ||
| throw new Error( | ||
| `The "maxDuration" trigger.config option is now required, and must be at least 5 seconds.` | ||
| `The "maxComputeSeconds" trigger.config option is now required, and must be at least 5 seconds.` | ||
| ); | ||
| } | ||
| config.maxDuration = resolvedMaxDuration; |
There was a problem hiding this comment.
🟡 Project-wide compute limit is silently ignored for config files that don't use the helper wrapper
The project-wide compute limit written under the new name is only converted to the old name inside the command-line tool's copy of the settings (config.maxDuration = resolvedMaxDuration at packages/cli-v3/src/config.ts:371), not in the separate copy the task-indexing step loads, so tasks get no limit at all.
Impact: Users who export their settings as a plain object with only the new option get runs with no project-level compute cap, even though the tool reported the setting as valid.
Why the CLI-side mutation does not reach the indexed task manifests
validateConfig runs only inside the CLI process (packages/cli-v3/src/config.ts:196). The index workers re-import the user's built config module directly (importConfig at packages/cli-v3/src/entryPoints/dev-index-worker.ts:75-86, same in packages/cli-v3/src/entryPoints/managed-index-worker.ts) and then apply the project default with if (typeof config.maxDuration === "number") (packages/cli-v3/src/entryPoints/dev-index-worker.ts:147-152). For a config exported through defineConfig() the resolution happens at import time in the SDK, so it works; for a plain-object export — exactly the case the new comment at packages/cli-v3/src/config.ts:362-364 claims to support — the worker sees only maxComputeSeconds, maxDuration is undefined, and no per-task default is applied.
Prompt for agents
The CLI's validateConfig now resolves maxComputeSeconds into maxDuration by mutating its in-process copy of the config (packages/cli-v3/src/config.ts). However the dev and managed index workers (packages/cli-v3/src/entryPoints/dev-index-worker.ts and managed-index-worker.ts) import the user's built trigger.config module directly via importConfig() and read config.maxDuration to apply the project-level default to every task manifest. For configs exported as plain objects (bypassing defineConfig, which is the case the new comment says it supports), the worker's copy has only maxComputeSeconds, so the project-level limit is silently dropped. Consider resolving maxComputeSeconds -> maxDuration in the index workers too (or in a shared helper both the CLI and the entrypoints use) so the fallback works for every code path that reads the config.
Was this helpful? React with 👍 or 👎 to provide feedback.
| maxAttempts: options?.maxAttempts, | ||
| metadata: options?.metadata, | ||
| maxDuration: options?.maxDuration, | ||
| maxDuration: options ? resolveMaxComputeSeconds(options) : undefined, |
There was a problem hiding this comment.
🔍 Chat/agent session trigger config still only reads maxDuration
packages/trigger-sdk/src/v3/ai.ts:10455-10460 and packages/trigger-sdk/src/v3/chat-server.ts:549-551 build a SessionTriggerConfig that forwards only maxDuration. These use their own option type (not TriggerOptions), so maxComputeSeconds isn't accepted there and the rename is incomplete for chat/agent session triggering. Not flagged as a bug because the type doesn't advertise the new field, but it's an inconsistency users may hit when the new name becomes the documented one.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
WalkthroughThe change adds ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Rebased version of #3533. Adds maxComputeSeconds as the user-facing replacement for maxDuration on defineConfig, task definitions, and trigger options. maxDuration stays JSDoc-deprecated and accepted; if both are set maxComputeSeconds wins. Updated init templates and added unit tests. Closes #3533.