fix: expand ${env:VAR} placeholders in settings#72
Conversation
…d settings
Settings like beads.userId were used verbatim without expanding VS Code-style
${env:VAR} placeholders. Adds a resolveEnvVariables utility that matches
VS Code's variable syntax and applies it when reading string settings.
Resolves: #60
Related: vsbeads-owi
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements environment variable placeholder expansion for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
jest.config.js (1)
1-9: Consider adding ESLint environment directive for CommonJS.The static analysis flagged
'module' is not definedbecause this CommonJS config file runs in a context where ESLint expects ESM. This doesn't affect functionality but could be silenced.🔧 Optional fix to silence ESLint
/** `@type` {import('jest').Config} */ +/* eslint-env node */ module.exports = {Alternatively, you could rename to
jest.config.cjsto make the CommonJS format explicit, butjest.config.jsis the conventional name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jest.config.js` around lines 1 - 9, The ESLint warning is because the CommonJS usage (module.exports) in jest.config.js is being linted as ESM; add an ESLint environment directive for CommonJS at the top of the file (e.g., add a line declaring the commonjs env) so ESLint recognizes module.exports, or alternatively rename the file to jest.config.cjs to make the CommonJS format explicit; ensure the change targets the module.exports export in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@jest.config.js`:
- Around line 1-9: The ESLint warning is because the CommonJS usage
(module.exports) in jest.config.js is being linted as ESM; add an ESLint
environment directive for CommonJS at the top of the file (e.g., add a line
declaring the commonjs env) so ESLint recognizes module.exports, or
alternatively rename the file to jest.config.cjs to make the CommonJS format
explicit; ensure the change targets the module.exports export in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88d5cd05-ca0b-4131-80a1-c00180b1b575
📒 Files selected for processing (7)
CHANGELOG.mdjest.config.jspackage.jsonsrc/backend/BeadsProjectManager.tssrc/providers/BaseViewProvider.tssrc/utils/__tests__/resolve-env-variables.test.tssrc/utils/resolve-env-variables.ts
Summary
resolveEnvVariables()utility that expands${env:VAR}placeholders usingprocess.env, matching VS Code's variable syntax from launch.json/tasks.jsonbeads.userIdandbeads.pathToBdsettings so users can use${env:USER}or custom env varsjest.config.jswith ts-jest) and 8 tests covering all edge casespackage.jsonto document${env:VAR}supportFixes #60
Test plan
beads.userIdto${env:USER}— verify "Assign to me" uses actual usernamebeads.userIdto${env:NONEXISTENT}— verify fallback to$USERbeads.pathToBdto${env:HOME}/bin/bd— verify CLI resolves correctlybeads.userIdto a plain string — verify no regressionbun run test— all 8 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
beads.userIdandbeads.pathToBdconfiguration settings now support environment variable expansion using${env:VAR}placeholder syntax. Missing environment variables are substituted with empty strings.Documentation