feat(v5): Setup wizard#1212
Open
brendan-kellam wants to merge 7 commits into
Open
Conversation
Contributor
WalkthroughThe PR introduces a new interactive CLI setup wizard package ( ChangesInteractive Setup Wizard & Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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 docstrings
🧪 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 |
Contributor
|
@brendan-kellam your pull request is missing a changelog! |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
packages/setupWizard/src/models.ts (1)
15-25: 💤 Low valueGoogle Vertex environment variables will be categorized incorrectly in
.env.
GOOGLE_VERTEX_PROJECT,GOOGLE_VERTEX_REGION, andGOOGLE_APPLICATION_CREDENTIALSare collected for AI providers but not listed inPROVIDER_ENV_KEYS. Inindex.ts, these get filtered into the "Code host credentials" section instead of "AI provider credentials".Consider adding these to
PROVIDER_ENV_KEYSor using a separate set for the filtering logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/setupWizard/src/models.ts` around lines 15 - 25, PROVIDER_ENV_KEYS is missing Google Vertex-specific keys so GOOGLE_VERTEX_PROJECT, GOOGLE_VERTEX_REGION, and GOOGLE_APPLICATION_CREDENTIALS are being miscategorized; update PROVIDER_ENV_KEYS to include those three keys (referencing the constant PROVIDER_ENV_KEYS in packages/setupWizard/src/models.ts) or alternatively add a distinct set used by the filtering logic in index.ts so the index.ts filtering routine treats these keys as AI provider credentials rather than code host credentials; modify the constant or the filter to include the three Google Vertex env var names and ensure the index.ts code that computes AI provider vs code host sections uses that updated set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/setupWizard/src/azuredevops.ts`:
- Around line 24-37: The URL input currently allows plain http and doesn't
persist a trimmed value; update the input validation and assignment around the
prompt (the call to input(...) that sets config.url) to require HTTPS by default
by changing the validate function to reject non-https schemes, trim the value
before validation/assignment, and only allow http if the user explicitly
confirms an insecure choice (e.g., an extra confirmation prompt when v
startsWith 'http://'); finally assign the trimmed URL to config.url instead of
the raw input.
In `@packages/setupWizard/src/bitbucket.ts`:
- Around line 161-174: The URL input validation currently allows http and stores
the raw value; update the validate callback passed to input(...) to require
HTTPS only (reject if it doesn't start with https://) and normalize the saved
value by trimming whitespace before assigning to config.url (use the trimmed url
variable). If you want to support insecure HTTP as an opt-in, add an explicit
flag (e.g., allowInsecure) or a separate prompt rather than silently accepting
http in the validate function; locate the validate lambda and the assignment to
config.url to make these changes.
In `@packages/setupWizard/src/genericGit.ts`:
- Around line 6-22: The prompt validates trimmed input but assigns the raw
untrimmed value to config.url, so normalize the URL by trimming before storing;
update the code around the input(...) call and the
GenericGitHostConnectionConfig creation (the url assignment in config) to use a
trimmed value (e.g., const url = (await input(...)).trim() or const trimmed =
url.trim(); config.url = trimmed) so whitespace is removed before writing the
config.
In `@packages/setupWizard/src/gerrit.ts`:
- Around line 7-23: The Gerrit URL input is validated using trim() but the code
stores the raw url, allowing leading/trailing whitespace to persist; update the
input handling so that the captured variable (url from input()) is normalized
(call .trim()) before constructing the GerritConnectionConfig object (the config
variable of type GerritConnectionConfig) so the saved url is clean and
connection resolution won’t break; ensure any further uses of url reference the
normalized value rather than the original raw input.
In `@packages/setupWizard/src/gitea.ts`:
- Around line 10-25: The Gitea URL prompt currently accepts http:// and doesn't
trim input; update the input validation and post-processing so only https:// is
allowed by default: in the input(...) validator (the call that returns `url`)
require /^https:\/\// or, if you must allow http, add an explicit additional
confirmation step before accepting http URLs, and always use the trimmed value
(e.g., v.trim()) when assigning to config.url; finally assign the trimmed URL to
config.url (replace the current config.url = url conditional) so stored URLs are
normalized and HTTPS is enforced unless the user explicitly confirms insecure
HTTP.
In `@packages/setupWizard/src/github.ts`:
- Around line 43-44: Wrap the network call and JSON parsing in a try-catch
around the fetch(url, { headers }) and await res.json() calls so network errors
or malformed JSON are handled; on catch, log the error and return an empty list
or signal failure so the wizard falls back to literal entry instead of crashing;
update the code that uses the parsed `data` (the `data` variable and any
handling of `data.items`) to handle the fallback/empty case safely.
In `@packages/setupWizard/src/gitlab.ts`:
- Line 41: Wrap the network call that currently does "const res = await
fetch(url, { headers })" in a try-catch inside the same function so
network-level failures (DNS, timeouts, connection refused) are caught before the
subsequent "!res.ok" logic; in the catch block return or throw a controlled
error (or call the existing error handling/logging path) including context (URL
and token scope) so the wizard doesn't crash, and keep the existing res.ok
handling for HTTP-level errors.
In `@packages/setupWizard/src/index.ts`:
- Around line 25-27: The DOCKER_COMPOSE_BRANCH constant is pointed at a feature
branch; update DOCKER_COMPOSE_BRANCH to 'main' so DOCKER_COMPOSE_URL fetches the
canonical docker-compose.yml from the main branch (ensure the symbol
DOCKER_COMPOSE_BRANCH is changed and that DOCKER_COMPOSE_URL continues to build
from it).
- Around line 211-224: The current override generation mounts all
localRepoHostPaths to the same container path (/repos), causing shadowing;
change the mount creation in the block that builds overrideYaml (where
uniquePaths is used) to produce indexed container paths like /repos/0, /repos/1,
etc., and then update collectLocalReposConfig() to accept or build a mapping
from each host path to its assigned indexed container path so connection URIs
are generated as file:///repos/0/... (or file:///repos/{index}/{rel}) instead of
the current hardcoded file:///repos/*; ensure the same deduplicated order is
used for both the overrideYaml generation and the mapping passed into
collectLocalReposConfig() so paths and indices align.
In `@packages/setupWizard/src/localRepos.ts`:
- Around line 84-96: The path validator (the validate arrow function calling
expandHostPath, existsSync and statSync) can throw from statSync (or existsSync)
and must be guarded; wrap the filesystem checks in a try/catch so any thrown
errors return a validation message rather than crashing the wizard—use
expandHostPath(v) to get resolved, then in a try block check
existsSync(resolved) and statSync(resolved).isDirectory(), and in the catch
return a descriptive string like `Unable to validate path: <error message>` (or
a generic "Unable to access path") to surface permission/race errors to the
user.
- Around line 111-140: The relative-path handling is Windows-unsafe: change uses
of relative(hostPath, p)/rel that assume POSIX separators; compute a normalized
POSIX-rel path (e.g., call relative(hostPath, repoPath) then replace backslashes
with forward slashes or use path.posix operations) and use that normalized value
when computing allAtDepthOne (check !normalized.includes('/')) and when building
the file URL in connections (use the normalized rel inside
file:///repos/${...}); update references in this block (repos, hostPath,
relative, allAtDepthOne, connections, rel, GenericGitHostConnectionConfig) so
all path checks and generated URLs use the forward-slash normalized path.
In `@packages/setupWizard/src/utils.ts`:
- Around line 27-29: toEnvKey currently only replaces hyphens and can produce
invalid env var names for inputs with spaces, dots, slashes or leading digits;
update the toEnvKey function to fully sanitize the connectionName by replacing
all non-alphanumeric characters with underscores, converting to upper case,
collapsing consecutive underscores, trimming leading/trailing underscores, and
if the resulting key starts with a digit prefix it with an underscore, then
append the uppercased suffix separated by a single underscore; reference the
toEnvKey(connectionName: string, suffix: string) function and apply the
described normalization steps so generated env keys are valid POSIX-style
environment variable names.
In `@packages/setupWizard/tsconfig.json`:
- Line 12: Update the TypeScript configuration so builds fail to emit when there
are type errors by changing the tsconfig.json setting "noEmitOnError" from false
to true; locate the "noEmitOnError" key in tsconfig.json and set it to true to
prevent publishing or running a dist build that contains type-check failures.
---
Nitpick comments:
In `@packages/setupWizard/src/models.ts`:
- Around line 15-25: PROVIDER_ENV_KEYS is missing Google Vertex-specific keys so
GOOGLE_VERTEX_PROJECT, GOOGLE_VERTEX_REGION, and GOOGLE_APPLICATION_CREDENTIALS
are being miscategorized; update PROVIDER_ENV_KEYS to include those three keys
(referencing the constant PROVIDER_ENV_KEYS in
packages/setupWizard/src/models.ts) or alternatively add a distinct set used by
the filtering logic in index.ts so the index.ts filtering routine treats these
keys as AI provider credentials rather than code host credentials; modify the
constant or the filter to include the three Google Vertex env var names and
ensure the index.ts code that computes AI provider vs code host sections uses
that updated set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66b2f077-3fff-49a1-b40d-891710bc4c95
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
docker-compose.ymlpackages/setupWizard/package.jsonpackages/setupWizard/src/azuredevops.tspackages/setupWizard/src/bitbucket.tspackages/setupWizard/src/genericGit.tspackages/setupWizard/src/gerrit.tspackages/setupWizard/src/gitea.tspackages/setupWizard/src/github.tspackages/setupWizard/src/gitlab.tspackages/setupWizard/src/index.tspackages/setupWizard/src/localRepos.tspackages/setupWizard/src/models.tspackages/setupWizard/src/utils.tspackages/setupWizard/tsconfig.json
Comment on lines
+24
to
+37
| const url = await input({ | ||
| message: 'Azure DevOps Server URL (e.g. https://ado.example.com)', | ||
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'URL is required'; | ||
| } | ||
| if (!/^https?:\/\//.test(v)) { | ||
| return 'Must start with http:// or https://'; | ||
| } | ||
| return true; | ||
| }, | ||
| }); | ||
| config.url = url; | ||
|
|
Contributor
There was a problem hiding this comment.
Harden server URL handling for token auth (http:// currently allowed).
Line 30 accepts plain HTTP, which can expose PATs in transit. Require HTTPS by default (or explicitly gate HTTP behind an insecure-confirmation step), and persist the trimmed URL.
🔐 Suggested patch
- const url = await input({
+ const url = await input({
message: 'Azure DevOps Server URL (e.g. https://ado.example.com)',
- validate: (v) => {
- if (!v?.trim()) {
+ validate: (raw) => {
+ const v = raw?.trim();
+ if (!v) {
return 'URL is required';
}
- if (!/^https?:\/\//.test(v)) {
- return 'Must start with http:// or https://';
+ if (!/^https:\/\//.test(v)) {
+ return 'Must start with https://';
}
return true;
},
});
- config.url = url;
+ config.url = url.trim();📝 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.
Suggested change
| const url = await input({ | |
| message: 'Azure DevOps Server URL (e.g. https://ado.example.com)', | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https?:\/\//.test(v)) { | |
| return 'Must start with http:// or https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| config.url = url; | |
| const url = await input({ | |
| message: 'Azure DevOps Server URL (e.g. https://ado.example.com)', | |
| validate: (raw) => { | |
| const v = raw?.trim(); | |
| if (!v) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https:\/\//.test(v)) { | |
| return 'Must start with https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| config.url = url.trim(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/azuredevops.ts` around lines 24 - 37, The URL input
currently allows plain http and doesn't persist a trimmed value; update the
input validation and assignment around the prompt (the call to input(...) that
sets config.url) to require HTTPS by default by changing the validate function
to reject non-https schemes, trim the value before validation/assignment, and
only allow http if the user explicitly confirms an insecure choice (e.g., an
extra confirmation prompt when v startsWith 'http://'); finally assign the
trimmed URL to config.url instead of the raw input.
Comment on lines
+161
to
+174
| const url = await input({ | ||
| message: 'Bitbucket Data Center URL (e.g. https://bitbucket.example.com)', | ||
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'URL is required'; | ||
| } | ||
| if (!/^https?:\/\//.test(v)) { | ||
| return 'Must start with http:// or https://'; | ||
| } | ||
| return true; | ||
| }, | ||
| }); | ||
| config.url = url; | ||
|
|
Contributor
There was a problem hiding this comment.
Bitbucket Server URL validation should not allow insecure HTTP by default.
Line 167 currently permits http://. Since this flow collects/stores access tokens, enforce HTTPS (or add explicit insecure opt-in) and normalize with trim() before storing.
🔐 Suggested patch
- const url = await input({
+ const url = await input({
message: 'Bitbucket Data Center URL (e.g. https://bitbucket.example.com)',
- validate: (v) => {
- if (!v?.trim()) {
+ validate: (raw) => {
+ const v = raw?.trim();
+ if (!v) {
return 'URL is required';
}
- if (!/^https?:\/\//.test(v)) {
- return 'Must start with http:// or https://';
+ if (!/^https:\/\//.test(v)) {
+ return 'Must start with https://';
}
return true;
},
});
- config.url = url;
+ config.url = url.trim();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/bitbucket.ts` around lines 161 - 174, The URL input
validation currently allows http and stores the raw value; update the validate
callback passed to input(...) to require HTTPS only (reject if it doesn't start
with https://) and normalize the saved value by trimming whitespace before
assigning to config.url (use the trimmed url variable). If you want to support
insecure HTTP as an opt-in, add an explicit flag (e.g., allowInsecure) or a
separate prompt rather than silently accepting http in the validate function;
locate the validate lambda and the assignment to config.url to make these
changes.
Comment on lines
+6
to
+22
| const url = await input({ | ||
| message: 'Git clone URL (e.g. https://github.com/sourcebot-dev/sourcebot)', | ||
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'URL is required'; | ||
| } | ||
| if (!/^https?:\/\//.test(v)) { | ||
| return 'Must start with http:// or https://'; | ||
| } | ||
| return true; | ||
| }, | ||
| }); | ||
|
|
||
| const config: GenericGitHostConnectionConfig = { | ||
| type: 'git', | ||
| url, | ||
| }; |
Contributor
There was a problem hiding this comment.
Trim the clone URL before writing config.
The prompt validates trimmed input, but config.url stores the raw string. Normalize it once to avoid whitespace-related failures.
Suggested fix
export async function collectGenericGitConfig(): Promise<CollectResult> {
- const url = await input({
+ const rawUrl = await input({
message: 'Git clone URL (e.g. https://github.com/sourcebot-dev/sourcebot)',
validate: (v) => {
if (!v?.trim()) {
return 'URL is required';
}
if (!/^https?:\/\//.test(v)) {
return 'Must start with http:// or https://';
}
return true;
},
});
+ const url = rawUrl.trim();📝 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.
Suggested change
| const url = await input({ | |
| message: 'Git clone URL (e.g. https://github.com/sourcebot-dev/sourcebot)', | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https?:\/\//.test(v)) { | |
| return 'Must start with http:// or https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| const config: GenericGitHostConnectionConfig = { | |
| type: 'git', | |
| url, | |
| }; | |
| const rawUrl = await input({ | |
| message: 'Git clone URL (e.g. https://github.com/sourcebot-dev/sourcebot)', | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https?:\/\//.test(v)) { | |
| return 'Must start with http:// or https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| const url = rawUrl.trim(); | |
| const config: GenericGitHostConnectionConfig = { | |
| type: 'git', | |
| url, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/genericGit.ts` around lines 6 - 22, The prompt
validates trimmed input but assigns the raw untrimmed value to config.url, so
normalize the URL by trimming before storing; update the code around the
input(...) call and the GenericGitHostConnectionConfig creation (the url
assignment in config) to use a trimmed value (e.g., const url = (await
input(...)).trim() or const trimmed = url.trim(); config.url = trimmed) so
whitespace is removed before writing the config.
Comment on lines
+7
to
+23
| const url = await input({ | ||
| message: 'Gerrit URL (e.g. https://gerrit.example.com)', | ||
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'URL is required'; | ||
| } | ||
| if (!/^https?:\/\//.test(v)) { | ||
| return 'Must start with http:// or https://'; | ||
| } | ||
| return true; | ||
| }, | ||
| }); | ||
|
|
||
| const config: GerritConnectionConfig = { | ||
| type: 'gerrit', | ||
| url, | ||
| }; |
Contributor
There was a problem hiding this comment.
Normalize URL before saving the Gerrit connection.
Validation checks trim(), but the stored url is raw input. Leading/trailing spaces can still leak into config and break connection resolution.
Suggested fix
export async function collectGerritConfig(): Promise<CollectResult> {
- const url = await input({
+ const rawUrl = await input({
message: 'Gerrit URL (e.g. https://gerrit.example.com)',
validate: (v) => {
if (!v?.trim()) {
return 'URL is required';
}
if (!/^https?:\/\//.test(v)) {
return 'Must start with http:// or https://';
}
return true;
},
});
+ const url = rawUrl.trim();📝 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.
Suggested change
| const url = await input({ | |
| message: 'Gerrit URL (e.g. https://gerrit.example.com)', | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https?:\/\//.test(v)) { | |
| return 'Must start with http:// or https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| const config: GerritConnectionConfig = { | |
| type: 'gerrit', | |
| url, | |
| }; | |
| const rawUrl = await input({ | |
| message: 'Gerrit URL (e.g. https://gerrit.example.com)', | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'URL is required'; | |
| } | |
| if (!/^https?:\/\//.test(v)) { | |
| return 'Must start with http:// or https://'; | |
| } | |
| return true; | |
| }, | |
| }); | |
| const url = rawUrl.trim(); | |
| const config: GerritConnectionConfig = { | |
| type: 'gerrit', | |
| url, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/gerrit.ts` around lines 7 - 23, The Gerrit URL input
is validated using trim() but the code stores the raw url, allowing
leading/trailing whitespace to persist; update the input handling so that the
captured variable (url from input()) is normalized (call .trim()) before
constructing the GerritConnectionConfig object (the config variable of type
GerritConnectionConfig) so the saved url is clean and connection resolution
won’t break; ensure any further uses of url reference the normalized value
rather than the original raw input.
Comment on lines
+10
to
+25
| const url = await input({ | ||
| message: 'Gitea URL', | ||
| default: 'https://gitea.com', | ||
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'URL is required'; | ||
| } | ||
| if (!/^https?:\/\//.test(v)) { | ||
| return 'Must start with http:// or https://'; | ||
| } | ||
| return true; | ||
| }, | ||
| }); | ||
| if (url !== 'https://gitea.com') { | ||
| config.url = url; | ||
| } |
Contributor
There was a problem hiding this comment.
Gitea URL prompt should enforce HTTPS for safer credential transport.
Line 17 accepts http://; this weakens credential safety when a token is configured. Enforce HTTPS by default (or require explicit insecure confirmation) and save a trimmed URL.
🔐 Suggested patch
- const url = await input({
+ const url = await input({
message: 'Gitea URL',
default: 'https://gitea.com',
- validate: (v) => {
- if (!v?.trim()) {
+ validate: (raw) => {
+ const v = raw?.trim();
+ if (!v) {
return 'URL is required';
}
- if (!/^https?:\/\//.test(v)) {
- return 'Must start with http:// or https://';
+ if (!/^https:\/\//.test(v)) {
+ return 'Must start with https://';
}
return true;
},
});
- if (url !== 'https://gitea.com') {
- config.url = url;
+ const normalizedUrl = url.trim();
+ if (normalizedUrl !== 'https://gitea.com') {
+ config.url = normalizedUrl;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/gitea.ts` around lines 10 - 25, The Gitea URL prompt
currently accepts http:// and doesn't trim input; update the input validation
and post-processing so only https:// is allowed by default: in the input(...)
validator (the call that returns `url`) require /^https:\/\// or, if you must
allow http, add an explicit additional confirmation step before accepting http
URLs, and always use the trimmed value (e.g., v.trim()) when assigning to
config.url; finally assign the trimmed URL to config.url (replace the current
config.url = url conditional) so stored URLs are normalized and HTTPS is
enforced unless the user explicitly confirms insecure HTTP.
Comment on lines
+211
to
+224
| if (localRepoHostPaths.length > 0) { | ||
| const uniquePaths = [...new Set(localRepoHostPaths)]; | ||
| const overrideYaml = [ | ||
| '# Generated by setup-sourcebot', | ||
| '# Merged with docker-compose.yml at `docker compose up` time.', | ||
| 'services:', | ||
| ' sourcebot:', | ||
| ' volumes:', | ||
| ...uniquePaths.map((p) => ` - ${p}:/repos:ro`), | ||
| '', | ||
| ].join('\n'); | ||
| writeFileSync('docker-compose.override.yml', overrideYaml); | ||
| writtenFiles.push('docker-compose.override.yml'); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how localRepos config references the repo path to ensure consistency
rg -n -A5 'localRepos|localRepoHostPath' packages/setupWizard/src/Repository: sourcebot-dev/sourcebot
Length of output: 3055
🏁 Script executed:
#!/bin/bash
# Get the full context of lines 211-224 in index.ts
echo "=== Full index.ts lines 205-230 ==="
sed -n '205,230p' packages/setupWizard/src/index.ts
# Get the localRepos.ts file to see how connections are created and used
echo "=== Full localRepos.ts file (to understand connections) ==="
wc -l packages/setupWizard/src/localRepos.tsRepository: sourcebot-dev/sourcebot
Length of output: 1074
🏁 Script executed:
#!/bin/bash
# Check the full localRepos.ts to understand how connections reference paths
cat -n packages/setupWizard/src/localRepos.tsRepository: sourcebot-dev/sourcebot
Length of output: 5470
🏁 Script executed:
#!/bin/bash
# Check how connections are stored in config.json and whether they reference the repo paths
echo "=== Check full index.ts around config generation (lines 170-210) ==="
sed -n '170,210p' packages/setupWizard/src/index.ts
# Also check if there's any mapping between localRepoHostPaths and connections
echo -e "\n=== Search for how connections are used with localRepoHostPaths ==="
rg -n -B5 -A5 'connections' packages/setupWizard/src/index.ts | head -80Repository: sourcebot-dev/sourcebot
Length of output: 3553
🏁 Script executed:
#!/bin/bash
# Check the complete flow - see how many times collectLocalReposConfig can be called
# and what happens with multiple local repo paths
echo "=== Check index.ts lines 54-125 to see full setup loop ==="
sed -n '54,125p' packages/setupWizard/src/index.ts
echo -e "\n=== Check how localRepoHostPaths length is used ==="
rg -n 'localRepoHostPaths' packages/setupWizard/src/index.tsRepository: sourcebot-dev/sourcebot
Length of output: 3404
🏁 Script executed:
#!/bin/bash
# Key question: When there are multiple localRepoHostPaths, are the connections
# updated to reference different mount points? Or do they all reference /repos?
echo "=== Full context around connections usage with repos ==="
sed -n '100,120p' packages/setupWizard/src/localRepos.ts
echo -e "\n=== See how connections reference /repos ==="
grep -n "file:///" packages/setupWizard/src/localRepos.tsRepository: sourcebot-dev/sourcebot
Length of output: 956
Multiple local repo paths mount to the same container path, causing volume conflicts.
When multiple local repo directories are configured, they all mount to /repos:ro in the docker-compose override. This creates volume shadowing where only the last mounted path is accessible, and earlier ones are lost.
The proposed fix to mount to indexed paths (/repos/0, /repos/1, etc.) is a necessary step, but it is incomplete. The connections generated by collectLocalReposConfig() hardcode references to file:///repos/* or file:///repos/${rel}, and these connection configs are created before the mount indices are known. After applying the proposed mount fix, the connection configurations would reference the wrong container paths.
A complete solution requires either:
- Preventing multiple local repo paths from being added as separate configuration entries (UI constraint), or
- Modifying connection generation to be aware of and reference the correct indexed mount points.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/index.ts` around lines 211 - 224, The current
override generation mounts all localRepoHostPaths to the same container path
(/repos), causing shadowing; change the mount creation in the block that builds
overrideYaml (where uniquePaths is used) to produce indexed container paths like
/repos/0, /repos/1, etc., and then update collectLocalReposConfig() to accept or
build a mapping from each host path to its assigned indexed container path so
connection URIs are generated as file:///repos/0/... (or
file:///repos/{index}/{rel}) instead of the current hardcoded file:///repos/*;
ensure the same deduplicated order is used for both the overrideYaml generation
and the mapping passed into collectLocalReposConfig() so paths and indices
align.
Comment on lines
+84
to
+96
| validate: (v) => { | ||
| if (!v?.trim()) { | ||
| return 'Path is required'; | ||
| } | ||
| const resolved = expandHostPath(v); | ||
| if (!existsSync(resolved)) { | ||
| return `Path does not exist: ${resolved}`; | ||
| } | ||
| if (!statSync(resolved).isDirectory()) { | ||
| return `Not a directory: ${resolved}`; | ||
| } | ||
| return true; | ||
| }, |
Contributor
There was a problem hiding this comment.
Guard path validation against filesystem exceptions.
statSync can throw (permission errors, races), which would abort the wizard instead of returning a validation message.
Suggested fix
const rawPath = await input({
message: 'Path to your repos directory (e.g. ~/code)',
validate: (v) => {
if (!v?.trim()) {
return 'Path is required';
}
const resolved = expandHostPath(v);
- if (!existsSync(resolved)) {
- return `Path does not exist: ${resolved}`;
- }
- if (!statSync(resolved).isDirectory()) {
- return `Not a directory: ${resolved}`;
- }
+ try {
+ if (!existsSync(resolved)) {
+ return `Path does not exist: ${resolved}`;
+ }
+ if (!statSync(resolved).isDirectory()) {
+ return `Not a directory: ${resolved}`;
+ }
+ } catch (error) {
+ return `Unable to access path: ${resolved}`;
+ }
return true;
},
});📝 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.
Suggested change
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'Path is required'; | |
| } | |
| const resolved = expandHostPath(v); | |
| if (!existsSync(resolved)) { | |
| return `Path does not exist: ${resolved}`; | |
| } | |
| if (!statSync(resolved).isDirectory()) { | |
| return `Not a directory: ${resolved}`; | |
| } | |
| return true; | |
| }, | |
| validate: (v) => { | |
| if (!v?.trim()) { | |
| return 'Path is required'; | |
| } | |
| const resolved = expandHostPath(v); | |
| try { | |
| if (!existsSync(resolved)) { | |
| return `Path does not exist: ${resolved}`; | |
| } | |
| if (!statSync(resolved).isDirectory()) { | |
| return `Not a directory: ${resolved}`; | |
| } | |
| } catch (error) { | |
| return `Unable to access path: ${resolved}`; | |
| } | |
| return true; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/localRepos.ts` around lines 84 - 96, The path
validator (the validate arrow function calling expandHostPath, existsSync and
statSync) can throw from statSync (or existsSync) and must be guarded; wrap the
filesystem checks in a try/catch so any thrown errors return a validation
message rather than crashing the wizard—use expandHostPath(v) to get resolved,
then in a try block check existsSync(resolved) and
statSync(resolved).isDirectory(), and in the catch return a descriptive string
like `Unable to validate path: <error message>` (or a generic "Unable to access
path") to surface permission/race errors to the user.
Comment on lines
+111
to
+140
| const choices = repos.map((repoPath) => ({ | ||
| name: relative(hostPath, repoPath), | ||
| value: repoPath, | ||
| checked: true, | ||
| })); | ||
|
|
||
| const selected = await checkbox<string>({ | ||
| message: 'Which repositories should be indexed?', | ||
| choices, | ||
| required: true, | ||
| pageSize: 15, | ||
| loop: false, | ||
| }); | ||
|
|
||
| const allSelected = selected.length === repos.length; | ||
| const allAtDepthOne = repos.every((p) => !relative(hostPath, p).includes('/')); | ||
|
|
||
| const connections = allSelected && allAtDepthOne | ||
| ? [{ | ||
| config: { | ||
| type: 'git', | ||
| url: 'file:///repos/*', | ||
| } satisfies GenericGitHostConnectionConfig, | ||
| }] | ||
| : selected.map((repoPath) => { | ||
| const rel = relative(hostPath, repoPath); | ||
| const config: GenericGitHostConnectionConfig = { | ||
| type: 'git', | ||
| url: `file:///repos/${rel}`, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js, does path.relative()return platform-specific separators (like` on Windows), and what is the recommended way to normalize those paths when constructing file:// URLs?`
💡 Result:
Yes. In Node.js, path.relative returns a relative path using the platform’s path separator style. On Windows, examples show it returning backslashes (e.g., '..\..\impl\bbb'), while on POSIX it uses forward slashes (e.g., '../../impl/bbb') [1]. Recommended normalization for file:// URLs: don’t manually swap separators from path.relative. Instead, construct file URLs using Node’s URL helpers (preferred): 1) Convert paths to URLs with url.pathToFileURL(path), which takes a local path and produces a properly formatted file: URL [2]. 2) If you already have a file URL string and need the OS path, use url.fileURLToPath(url), which converts back to a cross-platform absolute path [2]. If you absolutely must normalize yourself before building a file:// URL, normalize separators to forward slashes for the URL and resolve/absolutize the path first; e.g., one common approach is path.resolve(...).replace(/\/g,'/'), then prefix with file:// (handling Windows drive letters) [3]. However, the Node-provided url.pathToFileURL(...) is the safer, standard recommendation.
Citations:
- 1: https://nodejs.org/api/path.html
- 2: https://nodejs.org/api/url.html
- 3: https://stackoverflow.com/questions/20619488/how-to-convert-local-file-path-to-a-file-url-safely-in-node-js
🏁 Script executed:
# Find and inspect the file
fd -t f "localRepos.ts" packages/setupWizard/src/Repository: sourcebot-dev/sourcebot
Length of output: 106
🏁 Script executed:
# Read the specific file and lines mentioned
cat -n packages/setupWizard/src/localRepos.ts | sed -n '100,150p'Repository: sourcebot-dev/sourcebot
Length of output: 1882
🏁 Script executed:
# Check the full imports at the top of the file
head -20 packages/setupWizard/src/localRepos.tsRepository: sourcebot-dev/sourcebot
Length of output: 645
Fix cross-platform relative-path handling before generating file URLs.
Using includes('/') on relative(...) and interpolating rel directly into file:///repos/${rel} breaks on Windows, where path.relative() returns backslash separators. This causes allAtDepthOne to be incorrectly calculated and generates malformed file URLs with backslashes.
Suggested fix
-import { basename, join, relative, resolve } from 'path';
+import { basename, join, relative, resolve, sep } from 'path';
@@
- const choices = repos.map((repoPath) => ({
- name: relative(hostPath, repoPath),
+ const choices = repos.map((repoPath) => ({
+ name: relative(hostPath, repoPath) || '.',
value: repoPath,
checked: true,
}));
@@
- const allSelected = selected.length === repos.length;
- const allAtDepthOne = repos.every((p) => !relative(hostPath, p).includes('/'));
+ const allSelected = selected.length === repos.length;
+ const segmentCount = (repoPath: string) => relative(hostPath, repoPath).split(sep).filter(Boolean).length;
+ const allAtDepthOne = repos.every((repoPath) => segmentCount(repoPath) === 1);
@@
: selected.map((repoPath) => {
const rel = relative(hostPath, repoPath);
+ const relPosix = rel.split(sep).join('/');
const config: GenericGitHostConnectionConfig = {
type: 'git',
- url: `file:///repos/${rel}`,
+ url: relPosix ? `file:///repos/${relPosix}` : 'file:///repos',
};
return { name: basename(repoPath), config };
});📝 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.
Suggested change
| const choices = repos.map((repoPath) => ({ | |
| name: relative(hostPath, repoPath), | |
| value: repoPath, | |
| checked: true, | |
| })); | |
| const selected = await checkbox<string>({ | |
| message: 'Which repositories should be indexed?', | |
| choices, | |
| required: true, | |
| pageSize: 15, | |
| loop: false, | |
| }); | |
| const allSelected = selected.length === repos.length; | |
| const allAtDepthOne = repos.every((p) => !relative(hostPath, p).includes('/')); | |
| const connections = allSelected && allAtDepthOne | |
| ? [{ | |
| config: { | |
| type: 'git', | |
| url: 'file:///repos/*', | |
| } satisfies GenericGitHostConnectionConfig, | |
| }] | |
| : selected.map((repoPath) => { | |
| const rel = relative(hostPath, repoPath); | |
| const config: GenericGitHostConnectionConfig = { | |
| type: 'git', | |
| url: `file:///repos/${rel}`, | |
| }; | |
| import { basename, join, relative, resolve, sep } from 'path'; | |
| const choices = repos.map((repoPath) => ({ | |
| name: relative(hostPath, repoPath) || '.', | |
| value: repoPath, | |
| checked: true, | |
| })); | |
| const selected = await checkbox<string>({ | |
| message: 'Which repositories should be indexed?', | |
| choices, | |
| required: true, | |
| pageSize: 15, | |
| loop: false, | |
| }); | |
| const allSelected = selected.length === repos.length; | |
| const segmentCount = (repoPath: string) => relative(hostPath, repoPath).split(sep).filter(Boolean).length; | |
| const allAtDepthOne = repos.every((repoPath) => segmentCount(repoPath) === 1); | |
| const connections = allSelected && allAtDepthOne | |
| ? [{ | |
| config: { | |
| type: 'git', | |
| url: 'file:///repos/*', | |
| } satisfies GenericGitHostConnectionConfig, | |
| }] | |
| : selected.map((repoPath) => { | |
| const rel = relative(hostPath, repoPath); | |
| const relPosix = rel.split(sep).join('/'); | |
| const config: GenericGitHostConnectionConfig = { | |
| type: 'git', | |
| url: relPosix ? `file:///repos/${relPosix}` : 'file:///repos', | |
| }; | |
| return { name: basename(repoPath), config }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/localRepos.ts` around lines 111 - 140, The
relative-path handling is Windows-unsafe: change uses of relative(hostPath,
p)/rel that assume POSIX separators; compute a normalized POSIX-rel path (e.g.,
call relative(hostPath, repoPath) then replace backslashes with forward slashes
or use path.posix operations) and use that normalized value when computing
allAtDepthOne (check !normalized.includes('/')) and when building the file URL
in connections (use the normalized rel inside file:///repos/${...}); update
references in this block (repos, hostPath, relative, allAtDepthOne, connections,
rel, GenericGitHostConnectionConfig) so all path checks and generated URLs use
the forward-slash normalized path.
Comment on lines
+27
to
+29
| export function toEnvKey(connectionName: string, suffix: string): string { | ||
| return `${connectionName.toUpperCase().replace(/-/g, '_')}_${suffix}`; | ||
| } |
Contributor
There was a problem hiding this comment.
Harden env var key normalization for non-alphanumeric connection names.
Current normalization only handles -. Names containing spaces, /, ., or leading digits can produce invalid env keys and break generated .env consumption.
Suggested fix
export function toEnvKey(connectionName: string, suffix: string): string {
- return `${connectionName.toUpperCase().replace(/-/g, '_')}_${suffix}`;
+ const normalized = connectionName
+ .toUpperCase()
+ .replace(/[^A-Z0-9_]/g, '_')
+ .replace(/_+/g, '_')
+ .replace(/^_+|_+$/g, '');
+ const safePrefix = /^[A-Z_]/.test(normalized) ? normalized : `C_${normalized}`;
+ return `${safePrefix}_${suffix}`;
}📝 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.
Suggested change
| export function toEnvKey(connectionName: string, suffix: string): string { | |
| return `${connectionName.toUpperCase().replace(/-/g, '_')}_${suffix}`; | |
| } | |
| export function toEnvKey(connectionName: string, suffix: string): string { | |
| const normalized = connectionName | |
| .toUpperCase() | |
| .replace(/[^A-Z0-9_]/g, '_') | |
| .replace(/_+/g, '_') | |
| .replace(/^_+|_+$/g, ''); | |
| const safePrefix = /^[A-Z_]/.test(normalized) ? normalized : `C_${normalized}`; | |
| return `${safePrefix}_${suffix}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/src/utils.ts` around lines 27 - 29, toEnvKey currently
only replaces hyphens and can produce invalid env var names for inputs with
spaces, dots, slashes or leading digits; update the toEnvKey function to fully
sanitize the connectionName by replacing all non-alphanumeric characters with
underscores, converting to upper case, collapsing consecutive underscores,
trimming leading/trailing underscores, and if the resulting key starts with a
digit prefix it with an underscore, then append the uppercased suffix separated
by a single underscore; reference the toEnvKey(connectionName: string, suffix:
string) function and apply the described normalization steps so generated env
keys are valid POSIX-style environment variable names.
| "module": "Node16", | ||
| "moduleResolution": "Node16", | ||
| "target": "ES2022", | ||
| "noEmitOnError": false, |
Contributor
There was a problem hiding this comment.
Do not emit build artifacts when TypeScript has errors.
"noEmitOnError": false allows publishing/running a dist build that failed type-checking. For a CLI package, this can produce runtime breakage in released artifacts.
Suggested fix
- "noEmitOnError": false,
+ "noEmitOnError": true,📝 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.
Suggested change
| "noEmitOnError": false, | |
| "noEmitOnError": true, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/setupWizard/tsconfig.json` at line 12, Update the TypeScript
configuration so builds fail to emit when there are type errors by changing the
tsconfig.json setting "noEmitOnError" from false to true; locate the
"noEmitOnError" key in tsconfig.json and set it to true to prevent publishing or
running a dist build that contains type-check failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR adds a setup wizard that streamlines the onboarding process when using docker-compose
npx setup-sourcebotSummary by CodeRabbit
New Features
Chores