Repository navigation
Warn when SF_HOST overrides the configured host - #283
michieldegezelle wants to merge 1 commit into
Conversation
The deprecated SF_HOST environment variable takes precedence over the host stored with `config --set-host`, but nothing said so: --set-host reported success while later commands kept using the SF_HOST host, and no host line was printed when it was the live default. Switching to a staging host could therefore silently write to live. - Warn on every command when the host comes from SF_HOST - Warn in --set-host when SF_HOST overrides it; show the source in --get-host - Name the target host in the overwrite confirmation prompt Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Note Some coding-guideline sources were skipped; the reason is listed next to each one. Other guidelines were applied as usual. On a public repository, sources from your configuration are listed without their names. Skipped guideline sources (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📚 Code guidelines (1)📓 Path-based instructions (3)`lib/api/` must not touch the filesystem.📄 CodeRabbit inference engine (CLAUDE.md) Files:
`bin/cli.js` must not call the API directly📄 CodeRabbit inference engine (CLAUDE.md) Files:
Run `npm run lint && npm test` after code changes.📄 CodeRabbit inference engine (CLAUDE.md) Files:
WalkthroughThe CLI now reports when ChangesHost configuration
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The CLI identifies when SF_HOST supplies the active host, and overwrite confirmations name the current host. No material merge-blocking risk is evident in the reviewed changes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Related: #281, #282
Description
The deprecated
SF_HOSTenvironment variable takes precedence over the host stored withsilverfin config --set-host, but the CLI never said so:--set-hostprinted "Host set to: …" while every following command kept using theSF_HOSThost.SF_HOSTgave no visible clue at all.This happened in practice: a colleague with
export SF_HOST=https://live.getsilverfin.comin~/.zshrcset the host to staging and ranadd-shared-part -p 1 …. Both links were created on live partner 1, with no error.This PR makes the host in use visible. Behaviour is unchanged (
SF_HOSTstill wins); removing it is #281.SF_HOST, including when it's live.config --set-host: warns whenSF_HOSToverrides the host just set.config --get-host: says when the host comes fromSF_HOST.firmCredentials.isHostFromEnv().Version bumped to 1.60.1, with a CHANGELOG entry and
tests/TESTS.mdrows.Testing Instructions
Steps:
export H=$(mktemp -d)HOME=$H SF_HOST=https://live.getsilverfin.com node bin/cli.js config --set-host https://bso-staging-beta.staging.getsilverfin.com→ a warning thatSF_HOSToverrides it, plus the "Current host … from the deprecated SF_HOST" warning.HOME=$H SF_HOST=https://live.getsilverfin.com node bin/cli.js config --get-host→Current host: https://live.getsilverfin.com (from the SF_HOST environment variable).HOME=$H node bin/cli.js config --get-host→ the staging host, no warning.npx jest→ 779/779 pass. The newlogCurrentHosttest fails if theisHostFromEnv()branch is disabled (mutation-checked).Author Checklist
Reviewer Checklist
🤖 Generated with Claude Code