feat: add an input to run at a subdirectory of the repo root#232
feat: add an input to run at a subdirectory of the repo root#232bookchris wants to merge 2 commits intoun-ts:mainfrom
Conversation
- Allows for situations where the root of the npm workspace is not at the top level.
🦋 Changeset detectedLatest commit: 67170aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR threads a configurable working directory (cwd) through the codebase, allowing changeset and package path resolution relative to a repo subdirectory rather than the repository root. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/comment.ts`:
- Around line 261-266: Normalize the raw INPUT_CWD before constructing prefixes
and paths: compute a normalizedCwd by stripping a leading "./" and any trailing
slashes from cwdInput (turning "." or "./" into an empty string), then use
normalizedCwd when building changesetPrefix and any diff/path prefixes (e.g.,
replace uses of cwdInput in the changesetPrefix logic and other prefix
constructions around the existing cwdInput references and in the diff matching
code at the regions corresponding to lines ~291-299 and ~321-324); keep
absoluteCwd as path.resolve(process.cwd(), cwdInput ?? '.') but ensure all
string comparisons and prefix concatenations use normalizedCwd so GitLab path
forms like "subdir/file" will match correctly.
In `@src/main.ts`:
- Line 54: The code currently allows getOptionalInput('cwd') to resolve outside
the repo via path.resolve(process.cwd(), ...); change to explicitly resolve the
candidate relative to the repository root and reject or clamp traversals:
capture repoRoot = process.cwd(), compute candidate = getOptionalInput('cwd') ??
'.', then let resolved = path.resolve(repoRoot, candidate); normalize and check
that resolved === repoRoot || resolved.startsWith(repoRoot + path.sep) (use
path.normalize and path.sep), and if the check fails throw an error (or fallback
to repoRoot) before assigning to cwd; update the code around the cwd variable to
use this guarded resolved path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d10c4eec-7e2f-4999-9ea4-d4426c3c69aa
📒 Files selected for processing (4)
README.mdsrc/comment.tssrc/get-changed-packages.tssrc/main.ts
| const cwdInput = getOptionalInput('cwd') | ||
| const changesetPrefix = cwdInput | ||
| ? `${cwdInput.replace(/\/$/, '')}/.changeset` | ||
| : '.changeset' | ||
| const absoluteCwd = path.resolve(process.cwd(), cwdInput ?? '.') | ||
|
|
There was a problem hiding this comment.
Normalize INPUT_CWD before building prefixes used for diff matching.
Lines [261]-[266] and [291]-[299] use raw input text, so values like . or ./subdir won’t match GitLab paths. That can incorrectly report no changeset and break package file remapping.
🔧 Suggested normalization
- const cwdInput = getOptionalInput('cwd')
- const changesetPrefix = cwdInput
- ? `${cwdInput.replace(/\/$/, '')}/.changeset`
- : '.changeset'
- const absoluteCwd = path.resolve(process.cwd(), cwdInput ?? '.')
+ const repoRoot = process.cwd()
+ const cwdInput = getOptionalInput('cwd')
+ const absoluteCwd = path.resolve(repoRoot, cwdInput ?? '.')
+ const relativeCwd = path
+ .relative(repoRoot, absoluteCwd)
+ .split(path.sep)
+ .join('/')
+ const normalizedCwd = relativeCwd === '' ? '.' : relativeCwd
+ const subdirPrefix = normalizedCwd === '.' ? '' : `${normalizedCwd}/`
+ const changesetPrefix = `${subdirPrefix}.changeset`
@@
- const subdirPrefix = cwdInput ? cwdInput.replace(/\/$/, '') + '/' : ''
const packageChangedFiles = changedFilesPromise.then(changedFiles =>
changedFiles.map(({ new_path }) =>
subdirPrefix && new_path.startsWith(subdirPrefix)
? new_path.slice(subdirPrefix.length)
: new_path,
),
)Also applies to: 291-299, 321-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/comment.ts` around lines 261 - 266, Normalize the raw INPUT_CWD before
constructing prefixes and paths: compute a normalizedCwd by stripping a leading
"./" and any trailing slashes from cwdInput (turning "." or "./" into an empty
string), then use normalizedCwd when building changesetPrefix and any diff/path
prefixes (e.g., replace uses of cwdInput in the changesetPrefix logic and other
prefix constructions around the existing cwdInput references and in the diff
matching code at the regions corresponding to lines ~291-299 and ~321-324); keep
absoluteCwd as path.resolve(process.cwd(), cwdInput ?? '.') but ensure all
string comparisons and prefix concatenations use normalizedCwd so GitLab path
forms like "subdir/file" will match correctly.
| } | ||
|
|
||
| const { changesets } = await readChangesetState() | ||
| const cwd = path.resolve(process.cwd(), getOptionalInput('cwd') ?? '.') |
There was a problem hiding this comment.
Enforce INPUT_CWD to stay within the repository root.
Line [54] resolves any path, including absolute or ../ traversal targets. That breaks the “relative to repo root” contract and can run version/publish logic in unintended directories.
🔧 Suggested guard
- const cwd = path.resolve(process.cwd(), getOptionalInput('cwd') ?? '.')
+ const repoRoot = process.cwd()
+ const cwdInput = getOptionalInput('cwd')
+ const cwd = path.resolve(repoRoot, cwdInput ?? '.')
+ const relativeCwd = path.relative(repoRoot, cwd)
+ if (cwdInput && (path.isAbsolute(cwdInput) || relativeCwd.startsWith('..'))) {
+ throw new Error('INPUT_CWD must be a relative path inside the repository root')
+ }📝 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.
| const cwd = path.resolve(process.cwd(), getOptionalInput('cwd') ?? '.') | |
| const repoRoot = process.cwd() | |
| const cwdInput = getOptionalInput('cwd') | |
| const cwd = path.resolve(repoRoot, cwdInput ?? '.') | |
| const relativeCwd = path.relative(repoRoot, cwd) | |
| if (cwdInput && (path.isAbsolute(cwdInput) || relativeCwd.startsWith('..'))) { | |
| throw new Error('INPUT_CWD must be a relative path inside the repository root') | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.ts` at line 54, The code currently allows getOptionalInput('cwd') to
resolve outside the repo via path.resolve(process.cwd(), ...); change to
explicitly resolve the candidate relative to the repository root and reject or
clamp traversals: capture repoRoot = process.cwd(), compute candidate =
getOptionalInput('cwd') ?? '.', then let resolved = path.resolve(repoRoot,
candidate); normalize and check that resolved === repoRoot ||
resolved.startsWith(repoRoot + path.sep) (use path.normalize and path.sep), and
if the check fails throw an error (or fallback to repoRoot) before assigning to
cwd; update the code around the cwd variable to use this guarded resolved path.
1 participant
Allows for situations where the root of the npm workspace is not at the top level.
Fixes #231
Summary by CodeRabbit
New Features
Documentation