feat(cli/compute): --env-file + partial env edits via --env-set/--env-unset#94
Merged
tonychang04 merged 2 commits intomainfrom Apr 30, 2026
Merged
feat(cli/compute): --env-file + partial env edits via --env-set/--env-unset#94tonychang04 merged 2 commits intomainfrom
tonychang04 merged 2 commits intomainfrom
Conversation
…text The CLI passes --cron through to the backend unchanged. Once the validator update at InsForge/InsForge#1159 lands, --cron also accepts pg_cron interval syntax ("30 seconds", "5 minutes") for sub-minute cadence. Update the help text on `schedules create --cron` and `schedules update --cron` so commander's --help output mentions both formats. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two ergonomics fixes for env vars on compute services. `compute deploy --env-file <path>`. Today --env requires inline JSON, so deploying with seven secrets means hand-rolling a JSON object on the command line. --env-file accepts a standard .env (KEY=VALUE per line, # comments, blank lines, quoted values, trailing inline comments). Mutually exclusive with --env. Pure CLI parser — backend already accepts the same envVars: Record<string,string> shape that --env emits. `compute update --env-set KEY=VALUE` (repeatable) and `--env-unset KEY` (repeatable). Today --env replaces the whole envVars object, meaning to rotate one secret you have to know the other six. The new flags target the OSS envVarsPatch endpoint that lands the same week — the server decrypts existing env, applies set/unset, and re-encrypts. --env and --env-set/--env-unset are mutually exclusive (the request has one of envVars or envVarsPatch, never both). CLI-side validation mirrors the OSS schema: env var keys must match [A-Z_][A-Z0-9_]* in both flag paths so a typo errors locally instead of round-tripping a 400 from the backend. Tests: 12 new for the dotenv parser (quoted values, # comments, CRLF, inline comments, malformed lines with line-number errors, missing file). 133/133 + 13 skipped existing tests still pass. Typecheck + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR introduces environment variable management enhancements for compute commands and improves cron format documentation for schedules. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
Review rate limit: 3/5 reviews remaining, refill in 23 minutes and 56 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/compute/deploy.ts (1)
79-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
--envkeys with the same regex used by--env-file.
--env-fileenforces OSS key format, but the--envJSON path currently only checks value types. That lets invalid keys bypass CLI validation and shifts failure to the API.Suggested fix
if (opts.env) { let parsed: unknown; try { parsed = JSON.parse(opts.env); @@ if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { throw new CLIError('--env must be a JSON object like {"KEY":"value"}'); } + const ENV_KEY_REGEX = /^[A-Z_][A-Z0-9_]*$/; for (const [k, v] of Object.entries(parsed)) { + if (!ENV_KEY_REGEX.test(k)) { + throw new CLIError( + `Invalid env var key "${k}": must match [A-Z_][A-Z0-9_]*` + ); + } if (typeof v !== 'string') { throw new CLIError( `--env values must be strings — got ${typeof v} for key "${k}"` ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/compute/deploy.ts` around lines 79 - 97, The --env JSON path only validates values but not keys, allowing invalid env variable names to bypass CLI checks; update the parsing block handling opts.env in the deploy command to validate each key against the same OSS key regex used for --env-file (reuse the same validation function or regex symbol used for env-file validation), and throw a CLIError with a clear message when a key fails validation (similar to the existing value error path), then assign envVars = parsed as Record<string,string> only after keys and values pass validation.
🤖 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/commands/compute/update.ts`:
- Around line 105-115: The env patch building in update.ts allows the same key
to appear in both envSetArgs and envUnsetArgs, causing ambiguous patches; before
constructing setMap and assigning body.envVarsPatch (in the block guarded by
hasPatch), validate and reject overlaps by computing the intersection of keys
produced by envSetArgs (use parseKeyValue to extract keys) and envUnsetArgs
(after assertValidKey), and if any duplicates exist throw a user-facing error
(e.g., exit or throw) explaining the conflicting keys so the CLI fails fast
rather than producing a patch where a key is both set and unset.
---
Outside diff comments:
In `@src/commands/compute/deploy.ts`:
- Around line 79-97: The --env JSON path only validates values but not keys,
allowing invalid env variable names to bypass CLI checks; update the parsing
block handling opts.env in the deploy command to validate each key against the
same OSS key regex used for --env-file (reuse the same validation function or
regex symbol used for env-file validation), and throw a CLIError with a clear
message when a key fails validation (similar to the existing value error path),
then assign envVars = parsed as Record<string,string> only after keys and values
pass validation.
🪄 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: e9f35530-4a84-4166-9051-3563a76f9cc3
📒 Files selected for processing (6)
src/commands/compute/deploy.tssrc/commands/compute/update.tssrc/commands/schedules/create.tssrc/commands/schedules/update.tssrc/lib/env-file.test.tssrc/lib/env-file.ts
Comment on lines
+105
to
+115
| if (hasPatch) { | ||
| const setMap: Record<string, string> = {}; | ||
| for (const arg of envSetArgs) { | ||
| const [k, v] = parseKeyValue(arg); | ||
| setMap[k] = v; | ||
| } | ||
| for (const k of envUnsetArgs) assertValidKey(k); | ||
| body.envVarsPatch = { | ||
| ...(envSetArgs.length > 0 && { set: setMap }), | ||
| ...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }), | ||
| }; |
There was a problem hiding this comment.
Reject conflicting env patch operations for the same key.
A key can currently be present in both set and unset, creating an ambiguous patch contract. Fail fast in CLI when overlap exists.
Suggested fix
if (hasPatch) {
const setMap: Record<string, string> = {};
for (const arg of envSetArgs) {
const [k, v] = parseKeyValue(arg);
setMap[k] = v;
}
for (const k of envUnsetArgs) assertValidKey(k);
+ const overlaps = envUnsetArgs.filter((k) => Object.hasOwn(setMap, k));
+ if (overlaps.length > 0) {
+ throw new CLIError(
+ `Conflicting env patch: key(s) present in both --env-set and --env-unset: ${overlaps.join(', ')}`
+ );
+ }
body.envVarsPatch = {
...(envSetArgs.length > 0 && { set: setMap }),
...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }),
};
}📝 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
| if (hasPatch) { | |
| const setMap: Record<string, string> = {}; | |
| for (const arg of envSetArgs) { | |
| const [k, v] = parseKeyValue(arg); | |
| setMap[k] = v; | |
| } | |
| for (const k of envUnsetArgs) assertValidKey(k); | |
| body.envVarsPatch = { | |
| ...(envSetArgs.length > 0 && { set: setMap }), | |
| ...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }), | |
| }; | |
| if (hasPatch) { | |
| const setMap: Record<string, string> = {}; | |
| for (const arg of envSetArgs) { | |
| const [k, v] = parseKeyValue(arg); | |
| setMap[k] = v; | |
| } | |
| for (const k of envUnsetArgs) assertValidKey(k); | |
| const overlaps = envUnsetArgs.filter((k) => Object.hasOwn(setMap, k)); | |
| if (overlaps.length > 0) { | |
| throw new CLIError( | |
| `Conflicting env patch: key(s) present in both --env-set and --env-unset: ${overlaps.join(', ')}` | |
| ); | |
| } | |
| body.envVarsPatch = { | |
| ...(envSetArgs.length > 0 && { set: setMap }), | |
| ...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/compute/update.ts` around lines 105 - 115, The env patch
building in update.ts allows the same key to appear in both envSetArgs and
envUnsetArgs, causing ambiguous patches; before constructing setMap and
assigning body.envVarsPatch (in the block guarded by hasPatch), validate and
reject overlaps by computing the intersection of keys produced by envSetArgs
(use parseKeyValue to extract keys) and envUnsetArgs (after assertValidKey), and
if any duplicates exist throw a user-facing error (e.g., exit or throw)
explaining the conflicting keys so the CLI fails fast rather than producing a
patch where a key is both set and unset.
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
2 participants
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.
Summary
compute deploy --env-file <path>— standard dotenv parsing (KEY=VALUE per line,#-comments, blank lines, quoted values, CRLF, inline comments). Mutually exclusive with--env. Preferred for >1 secret.compute update --env-set KEY=VALUE/--env-unset KEY— repeatable partial-merge flags that pair with the OSSenvVarsPatchwork. Lets users rotate one secret without restating the rest. Mutually exclusive with--env(wholesale replace).Pairs with InsForge/InsForge#TBD (OSS) and InsForge/insforge-skills#TBD (docs).
Test plan
parseEnvFile(quotes, comments, CRLF, edge cases)compute deploy --env-file ./.env.production --image ... --name ...deploys with varscompute update <id> --env-set FOO=bar --env-unset OLD_TOKENmerges correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--env-fileoption to deploy command for loading environment variables from.envfiles.--env-setand--env-unsetoptions to update command for managing environment variables.Tests