Skip to content

fix: .tool-versions regex - #144

Open
ronnnnn wants to merge 6 commits into
oven-sh:mainfrom
ronnnnn:fix-tool-versions-match
Open

fix: .tool-versions regex#144
ronnnnn wants to merge 6 commits into
oven-sh:mainfrom
ronnnnn:fix-tool-versions-match

Conversation

@ronnnnn

@ronnnnn ronnnnn commented Oct 15, 2025

Copy link
Copy Markdown
Contributor

Overview

I had a mistake on #94.
We can't find bun's version correctly when .tool-versions have other versions begin "bun".

Reproduce steps

In .tool-versions:

bundler 1.0.0
bun     1.0.0

Regex check

https://regex101.com/r/2aCusB/4

@coderabbitai

coderabbitai Bot commented Oct 15, 2025

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bde08e08-1904-4bb7-bf54-e8b98f039909

📥 Commits

Reviewing files that changed from the base of the PR and between 46fef78 and 7e2fb2d.

⛔ Files ignored due to path filters (1)
  • dist/setup/index.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • src/utils.ts

Walkthrough

Updated the .tool-versions parser in src/utils.ts: the bun version regex now disallows letters immediately after bun, matches optional whitespace after bun, and requires the captured version to be a non-whitespace token (\S+).

Changes

Cohort / File(s) Summary
Version parsing regex update
src/utils.ts
Adjusted bun entry regex in FILE_VERSION_READERS: added negative lookahead (?![a-zA-Z]) to prevent matching bun followed by letters, allowed optional whitespace after bun, and changed version capture from .*? to \S+ to require a non-empty, non-space token.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: .tool-versions regex' directly and clearly describes the main change in the pull request.
Description check ✅ Passed The description provides relevant context about the bug fix, including reproduction steps and a reference to the regex being used.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6356405 and e5ed3e1.

⛔ Files ignored due to path filters (1)
  • dist/setup/index.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • src/utils.ts (1 hunks)

Comment thread src/utils.ts Outdated

@xhyrom xhyrom 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.

LGTM

@ronnnnn
ronnnnn requested a review from xhyrom October 31, 2025 00:29
@ronnnnn

ronnnnn commented Oct 31, 2025

Copy link
Copy Markdown
Contributor Author

@xhyrom
Thank you for your review. I fixed the CI failure.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e5ed3e1 and b08cdef.

⛔ Files ignored due to path filters (1)
  • dist/setup/index.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • src/utils.ts (1 hunks)

Comment thread src/utils.ts Outdated
@ronnnnn
ronnnnn force-pushed the fix-tool-versions-match branch from b08cdef to a961131 Compare October 31, 2025 00:40
file: .tool-versions
run: echo "bun 1.1.0" > .tool-versions

- name: .tool-versions (bun1.1.0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this format is not supported on asdf

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.

we should continue supporting this, even though it’s invalid, until the next major version so we don’t break semver

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I've updated the regex to use negative lookahead (?![a-zA-Z]) which:

  • Excludes bundler and similar patterns (the original bug)
  • Maintains backward compatibility for bun1.1.0 format (no space)
  • Keeps support for the standard bun 1.1.0 format

This approach preserves semver compatibility as @xhyrom suggested.

file: .tool-versions
run: echo "bun 1.1.0" > .tool-versions

- name: .tool-versions (bun1.1.0)

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.

we should continue supporting this, even though it’s invalid, until the next major version so we don’t break semver

… support

- Update regex to /^bun(?![a-zA-Z])\s*(?<version>\S+)$/m
- Restore test case for .tool-versions (bun1.1.0)
@ronnnnn
ronnnnn requested a review from xhyrom January 7, 2026 01:05
@xhyrom

xhyrom commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

thanks!

@xhyrom

xhyrom commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

@ronnnnn can you resolve conflicts please?

Resolved conflict in dist/setup/index.js by rebuilding the bundle
from src/utils.ts which retains the negative lookahead regex
(`/^bun(?![a-zA-Z])\\s*(?<version>\\S+)$/m`) to exclude
"bundler" entries while still parsing "bun" versions correctly.
Copilot AI review requested due to automatic review settings April 25, 2026 06:10
@ronnnnn

ronnnnn commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

@xhyrom
I'm sorry for late response. I resolved conflicts.

Copilot AI 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.

Pull request overview

This PR aims to fix .tool-versions parsing so the action correctly detects Bun’s version even when other tool entries begin with bun (e.g. bundler).

Changes:

  • Update the .tool-versions regex used to extract the Bun version.
  • Update the compiled dist/setup/index.js bundle to reflect the source change.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/utils.ts Adjusts the .tool-versions version-extraction regex used by readVersionFromFile.
dist/setup/index.js Updates the bundled output so runtime behavior matches src/ changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils.ts
},
".tool-versions": (content: string) =>
content.match(/^bun\s*(?<version>.*?)$/m)?.groups?.version,
content.match(/^bun(?![a-zA-Z])\s*(?<version>\S+)$/m)?.groups?.version,
Comment thread src/utils.ts
},
".tool-versions": (content: string) =>
content.match(/^bun\s*(?<version>.*?)$/m)?.groups?.version,
content.match(/^bun(?![a-zA-Z])\s*(?<version>\S+)$/m)?.groups?.version,
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.

4 participants