GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix(runs): do not apply default 7d period when explicit runIds are supplied (#5110) by almagas · Pull Request #5112 · triggerdotdev/trigger.dev

Conversation

almagas commented Nov 11, 2026

Copy link

Copy Markdown

Resolves #5110
/claim #5110

Problem Summary

In convertRunListInputOptionsToFilterRunsOptions (apps/webapp/app/services/runsRepository/runsRepository.server.ts), when explicit runId values are passed (e.g. during bulk cancellation runs.bulk.cancel({ runIds: [...] })), timeFilters was called unconditionally. When no explicit period, from, or to was provided, timeFilters applied the default 7-day time window (period: "7d"). Consequently, targeted runs older than 7 days were silently excluded from bulk operations and list queries.

Solution

In convertRunListInputOptionsToFilterRunsOptions, guarded timeFilters: when explicit runIds are supplied without an explicit time filter (period, from, or to), period remains undefined so all explicitly specified runs are included.
Filter-only queries (or when runId is empty) retain the default 7-day period.
Added unit tests in apps/webapp/test/runsRepositoryConvert.runIds.test.ts covering explicit runIds, filter-only queries, empty runId arrays, explicit periods, and explicit from/to ranges.

Verification

pnpm --filter webapp exec vitest run test/runsRepositoryConvert.runIds.test.ts -> 6/6 tests passing (Exit Code 0).
Lefthook format, knip, and lint hooks passing with 0 warnings and 0 errors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

granbellllc-png added 2 commits

November 11, 2026 05:20


          fix(runs): do not apply default 7d period when explicit runIds are su…

819ed75

…pplied (triggerdotdev#5110)

When explicit runIds are provided to bulk actions or run list queries without an explicit time filter (period, from, or to), convertRunListInputOptionsToFilterRunsOptions previously called timeFilters unconditionally, reintroducing the default 7d time bound. As a result, explicit runIds older than 7 days were silently excluded from bulk actions and list queries.

This patch guards the call to timeFilters: when explicit runIds are supplied and no explicit time filter is requested, period remains undefined so all explicitly targeted runs are included. Filter-only queries retain the 7d default.


          test(webapp): add unit tests for runsRepository explicit runIds conve…

118a301

…rsion (triggerdotdev#5110)

changeset-bot Bot commented Nov 11, 2026

Copy link

Copy Markdown

⚠️ No Changeset found

Latest commit: 118a301

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

granbellllc-png mentioned this pull request

Nov 11, 2026

Explicit runIds bulk cancel silently excludes runs older than seven days #5110

Open

coderabbitai Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Caution

Review failed

The pull request is closed.

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot commented Nov 11, 2026

Copy link

Copy Markdown

Contributor

Hi @almagas, 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot closed this

Nov 11, 2026

devin-ai-integration Bot reviewed

Nov 11, 2026

View reviewed changes

devin-ai-integration Bot left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

apps/webapp/test/runsRepositoryConvert.runIds.test.ts

Comment on lines +4 to +6

		vi.mock("~/db.server", () => ({}));
		vi.mock("~/v3/runStore.server", () => ({ runStore: {} }));
		vi.mock("~/env.server", () => ({ env: {} }));

devin-ai-integration Bot Nov 11, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟡 New run tests replace real dependencies

When these tests run, vi.mock replaces three production modules with empty objects. Repository test rules prohibit mocks and fakes, so these tests cannot exercise their real dependencies.

Learn more

The repository's testing guidance requires tests without mocks, stubs, or fakes. These module replacements prevent import-time singletons from constructing, while the suite also passes empty fake Prisma and store objects. The converter does not use those objects for the tested inputs, but the test still relies on forbidden substitutes instead of a testable production dependency boundary.

Example: The explicit-ID test succeeds with runStore: {} even though no real store is available; a future path calling the store would fail inside the test rather than validate the intended behavior.

Recommended fix: Extract the pure time-filter conversion behind a dependency-free module and test it directly without vi.mock or fake clients. For integration coverage, use real testcontainers and injected clients.

Was this helpful? React with 👍 or 👎 to provide feedback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

apps/webapp/app/services/runsRepository/runsRepository.server.ts

Comment on lines +372 to +378

		if (!hasExplicitRunIds \|\| hasExplicitTimeFilter) {
		const time = timeFilters({
		period: options.period,
		from: options.from,
		to: options.to,
		});
		convertedOptions.period = time.period ? (parseDuration(time.period) ?? undefined) : undefined;

devin-ai-integration Bot Nov 11, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟡 Bulk action fix lacks release note

When runId selects an older run, the new conversion includes it in bulk actions. This user-visible server fix has no required release note, so customers receive no announcement.

Learn more

Repository guidance requires a .server-changes/ entry for user-visible fixes made only in server components. This change affects which runs bulk actions and run lists can include, and the PR changes only the webapp and its tests. Without an entry, the release summary omits the fix.

Example: A customer whose bulk cancellation previously skipped an eight-day-old run will not see that selected older runs now work in the server release notes.

Recommended fix: Add a .server-changes/ markdown file with area: webapp, type: fix, and one user-facing sentence describing the inclusion of older explicitly selected runs.

Was this helpful? React with 👍 or 👎 to provide feedback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

apps/webapp/test/runsRepositoryConvert.runIds.test.ts

Comment on lines +26 to +27

		// Period must remain undefined so runs older than 7 days are not silently excluded (#5110)
		expect(result.period).toBeUndefined();

devin-ai-integration Bot Nov 11, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟡 Regression test omits required bug label

When the new unit suite checks result.period, its comment does not use the required // Catches: <bug> form. The regression therefore lacks the repository-mandated test annotation.

Suggested change

	// Period must remain undefined so runs older than 7 days are not silently excluded (#5110)
	expect(result.period).toBeUndefined();
	// Catches: explicit run IDs older than seven days were silently excluded from bulk actions.
	expect(result.period).toBeUndefined();

Was this helpful? React with 👍 or 👎 to provide feedback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Reviewers

devin-ai-integration[bot]

devin-ai-integration[bot] left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Explicit runIds bulk cancel silently excludes runs older than seven days

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

almagas commented Nov 11, 2026

Problem Summary

Solution

Verification

Uh oh!

changeset-bot Bot commented Nov 11, 2026

⚠️ No Changeset found

Uh oh!

coderabbitai Bot commented Nov 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Uh oh!

github-actions Bot commented Nov 11, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.