fix(cli): set output mode before loadLocalEnv, normalise URLs - #8
fix(cli): set output mode before loadLocalEnv, normalise URLs#8DuncanAForbes wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes implement runtime-configurable API and dashboard base URLs, replacing compile-time constants with dynamic getters. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (bin/bagdock.ts)
participant Config as Config Module
participant Env as .env.local
participant API as API Endpoint
User->>CLI: Execute command with --ngrok flag
CLI->>CLI: Check opts.ngrok in preAction hook
alt ngrok flag set
CLI->>Config: Call loadLocalEnv()
Config->>Env: Read .env.local from cwd
Env-->>Config: BAGDOCK_API_URL, BAGDOCK_DASHBOARD_URL
Config->>Config: normalizeUrl() validation & normalisation
Config->>Config: Update _apiBase, _dashboardBase variables
Config-->>CLI: Configuration loaded (print message)
end
CLI->>CLI: Apply per-invocation overrides (--api-key, --profile, --env)
CLI->>API: Make API request
API->>Config: getApiBase() called at request time
Config-->>API: Return current _apiBase value
API-->>CLI: Response
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/config.ts`:
- Around line 38-69: Change loadLocalEnv to accept an optional options object
(e.g., loadLocalEnv({ silent?: boolean } = {})) and use that silent flag to
suppress the final informational console.log; leave error console.error calls
intact so failures still surface. Update the call site (where loadLocalEnv is
invoked, e.g., in bin/bagdock.ts) to pass { silent: Boolean(opts.json ||
opts.quiet) } when running in JSON/quiet mode so the dim "Using local env → ..."
line does not emit during --json/--quiet.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9bcca0c5-81f1-4351-9f26-9a2eff73d41e
📒 Files selected for processing (8)
bin/bagdock.tspackage.jsonsrc/api.tssrc/auth.tssrc/config.tssrc/deploy.tssrc/open.tssrc/submit.ts
| export function loadLocalEnv() { | ||
| const envPath = join(process.cwd(), '.env.local') | ||
| if (!existsSync(envPath)) { | ||
| console.error(chalk.red('No .env.local found in'), chalk.bold(process.cwd())) | ||
| console.error(chalk.dim('Create one with BAGDOCK_API_URL=https://your-ngrok-url')) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| const vars: Record<string, string> = {} | ||
| const lines = readFileSync(envPath, 'utf-8').split('\n') | ||
| for (const line of lines) { | ||
| const trimmed = line.trim() | ||
| if (!trimmed || trimmed.startsWith('#')) continue | ||
| const eq = trimmed.indexOf('=') | ||
| if (eq === -1) continue | ||
| vars[trimmed.slice(0, eq).trim()] = trimmed.slice(eq + 1).trim() | ||
| } | ||
|
|
||
| const apiUrl = vars['BAGDOCK_API_URL'] | ||
| if (!apiUrl) { | ||
| console.error(chalk.red('BAGDOCK_API_URL not found in .env.local')) | ||
| console.error(chalk.dim('Add BAGDOCK_API_URL=https://your-ngrok-url to .env.local')) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| _apiBase = normalizeUrl(apiUrl) | ||
| if (vars['BAGDOCK_DASHBOARD_URL']) { | ||
| _dashboardBase = normalizeUrl(vars['BAGDOCK_DASHBOARD_URL']) | ||
| } | ||
|
|
||
| console.log(chalk.dim(`Using local env → ${_apiBase}`)) | ||
| } |
There was a problem hiding this comment.
loadLocalEnv() still emits stdout in JSON/quiet mode.
Line 68 prints unconditionally, so commands using --ngrok --json can still emit non-JSON text before structured output.
Proposed fix
-export function loadLocalEnv() {
+export function loadLocalEnv(opts: { silent?: boolean } = {}) {
const envPath = join(process.cwd(), '.env.local')
@@
- console.log(chalk.dim(`Using local env → ${_apiBase}`))
+ if (!opts.silent) {
+ console.log(chalk.dim(`Using local env → ${_apiBase}`))
+ }
}// bin/bagdock.ts (call site)
if (opts.ngrok) loadLocalEnv({ silent: Boolean(opts.json || opts.quiet) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config.ts` around lines 38 - 69, Change loadLocalEnv to accept an
optional options object (e.g., loadLocalEnv({ silent?: boolean } = {})) and use
that silent flag to suppress the final informational console.log; leave error
console.error calls intact so failures still surface. Update the call site
(where loadLocalEnv is invoked, e.g., in bin/bagdock.ts) to pass { silent:
Boolean(opts.json || opts.quiet) } when running in JSON/quiet mode so the dim
"Using local env → ..." line does not emit during --json/--quiet.
- Reorder preAction hook so --json/--quiet flags are active before loadLocalEnv prints any messages - Add normalizeUrl() to strip surrounding quotes, trailing slashes, and validate http(s) scheme on .env.local URL values
|
Replaced with a fresh PR after rebasing develop onto main to fix SHA divergence from squash merge. |
Summary
Addresses CodeRabbit feedback from PR #7:
setOutputModenow runs beforeloadLocalEnv()in the preAction hook, so--json/--quietflags are active before any loader messages printnormalizeUrl()helper that strips surrounding quotes, removes trailing slashes, and validateshttp(s)://scheme on.env.localURL valuesTest plan
bagdock login --ngrok --jsonproduces no text before JSON output.env.localwithBAGDOCK_API_URL="https://foo.ngrok.app/"resolves tohttps://foo.ngrok.app(no quotes, no trailing slash).env.localwithBAGDOCK_API_URL=not-a-urlexits with a clear errorSummary by CodeRabbit
New Features
--ngrokglobal CLI flag to load API and dashboard URLs from.env.localfile at runtime.Improvements
Version