Skip to content

fix(cli): avoid O(n²) spinner truncation in CI#3943

Closed
ADITYA-CODE-SOURCE wants to merge 1 commit into
triggerdotdev:mainfrom
ADITYA-CODE-SOURCE:fix/cli-truncate-message-ci-hang
Closed

fix(cli): avoid O(n²) spinner truncation in CI#3943
ADITYA-CODE-SOURCE wants to merge 1 commit into
triggerdotdev:mainfrom
ADITYA-CODE-SOURCE:fix/cli-truncate-message-ci-hang

Conversation

@ADITYA-CODE-SOURCE

Copy link
Copy Markdown

Summary

  • return the original spinner message when stdout is not a TTY or has no column width so CI does not pay truncation costs
  • replace the repeated rescan loop in truncateMessage() with a single-pass ANSI-aware truncation path
  • add CLI utility tests for non-TTY behavior plus ANSI and terminal hyperlink truncation

Testing

  • corepack pnpm --filter trigger.dev test src/utilities/windows.test.ts

@changeset-bot

changeset-bot Bot commented Jun 14, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 26277a8

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

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0842268f-2be1-462c-8e72-bb082057818b

📥 Commits

Reviewing files that changed from the base of the PR and between e092919 and 26277a8.

📒 Files selected for processing (2)
  • packages/cli-v3/src/utilities/windows.test.ts
  • packages/cli-v3/src/utilities/windows.ts

Walkthrough

windows.ts replaces its previous iterative slice-and-remeasure truncation with a stateful, single-pass character-walking implementation. New regex patterns and an ANSI-state tracker are introduced to compute visible string length while ignoring ANSI escape sequences and terminal hyperlink sequences. The truncation loop accumulates encountered escape sequences, tracks whether SGR color/style or terminal hyperlink sequences remain open at the truncation boundary, and conditionally appends the appropriate closing hyperlink escape and ANSI reset before the trailing ellipsis. windows.test.ts is added as a new file containing a Vitest suite that mocks process.stdout.isTTY and process.stdout.columns, restores both after each test, and covers all truncation paths including non-TTY passthrough, missing column width, short strings, plain-string truncation, ANSI-colored strings, and hyperlink preservation.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @ADITYA-CODE-SOURCE, 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.

@github-actions github-actions Bot closed this Jun 14, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

const rawCodes = sequence.slice(2, -1);
const codes = rawCodes === "" ? [0] : rawCodes.split(";").map(Number);

return codes.some((code) => !ansiResetCodes.has(code));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 updateAnsiState ignores previous SGR state on partial resets, causing terminal attribute leaks

updateAnsiState accepts a hasActiveSgr parameter but never uses it when the sequence ends with m (line 24). It only checks whether the current sequence's codes are non-reset codes. When a partial reset is encountered (e.g. \x1b[39m to reset foreground), the function returns false even if other attributes (e.g. bold from a prior \x1b[1;31m) are still active.

Example trace showing the bug

For input \x1b[1;31mhe\x1b[39mllo world with truncation at 4 visible chars:

  1. \x1b[1;31m → codes=[1,31], 1 not in resetCodes → hasActiveSgr = true
  2. 'h','e' → visible chars
  3. \x1b[39m → codes=[39], 39 IS in resetCodes → codes.some(code => !ansiResetCodes.has(code)) returns falsehasActiveSgr = false
  4. 'l','l' → 4 visible chars reached, truncate
  5. hasActiveSgr is false → no \x1b[0m appended

Result: \x1b[1;31mhe\x1b[39mll... — bold (code 1) leaks into subsequent terminal output.

The fix should consult hasActiveSgr when all codes in the current sequence are reset codes but none is a full reset (code 0). Only a full reset (\x1b[0m) or explicit reset of every active attribute should clear the state.

Suggested change
return codes.some((code) => !ansiResetCodes.has(code));
if (codes.some((code) => !ansiResetCodes.has(code))) return true;
if (codes.includes(0)) return false;
return hasActiveSgr;
Open in Devin Review

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

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.

1 participant