feat(cli): validate public vars cannot be secrets #525
Conversation
- Add validation to prevent public var prefixes (VITE_, AGENTUITY_PUBLIC_, PUBLIC_) from being set as secrets - Block reserved AGENTUITY_* keys (except AGENTUITY_PUBLIC_*) from being set by users - Allow AGENTUITY_PUBLIC_* keys as env vars CLI command changes: - secret set: Validates reserved keys + public vars as secrets - secret push: Uses splitEnvAndSecrets, warns about public vars - secret import: Uses splitEnvAndSecrets, warns about public vars - secret pull: Allows AGENTUITY_PUBLIC_* in .env file - env set: Allows AGENTUITY_PUBLIC_* keys - env push: Uses updated filterAgentuitySdkKeys - env import: Allows AGENTUITY_PUBLIC_* in .env file - env pull: Allows AGENTUITY_PUBLIC_* in .env file New utility functions in env-util.ts: - isPublicVarKey(): Check if key has public var prefix - isReservedAgentuityKey(): Check if key is reserved AGENTUITY_* (not PUBLIC_) - validateNoPublicSecrets(): Validate secrets don't contain public vars - getPublicSecretKeys(): Get public var keys from secrets object Updated functions: - filterAgentuitySdkKeys(): Now allows AGENTUITY_PUBLIC_* keys - splitEnvAndSecrets(): Routes public vars to env, not secrets Tests: - Added 48 unit tests for new validation functions - Added 9 integration tests for CLI command validation
📝 WalkthroughWalkthroughConsolidates secret management into the Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant CLI as agentuity CLI (cloud env)
participant Util as env-util
participant Cloud as Cloud API
participant FS as Local FS (.env)
User->>CLI: run "cloud env set KEY value [--secret]"
CLI->>Util: isReservedAgentuityKey / isPublicVarKey / looksLikeSecret
alt looksLikeSecret && no --secret
CLI->>User: prompt "store as secret? (Y/n)"
User-->>CLI: confirm
end
CLI->>Cloud: projectEnvUpdate({ env: {...}, secrets: {...} })
Cloud-->>CLI: success
CLI->>FS: read .env
CLI->>Util: filterAgentuitySdkKeys (preserve AGENTUITY_PUBLIC_*)
CLI->>FS: write .env
CLI-->>User: success (includes secret: boolean)
sequenceDiagram
participant User as CLI User
participant CLI as agentuity CLI (env import/push)
participant Util as env-util
participant Cloud as Cloud API
participant FS as Local FS
User->>CLI: run "cloud env import|push file"
CLI->>FS: read file -> rawVars
CLI->>Util: splitEnvAndSecrets(rawVars)
alt public keys found in secrets
CLI->>User: warn and auto-move public keys to env
end
CLI->>Cloud: projectEnvUpdate({ env: {...}, secrets: {...} })
Cloud-->>CLI: success (envCount, secretCount)
CLI->>FS: merge with local .env, skip reserved AGENTUITY_ except AGENTUITY_PUBLIC_*
CLI-->>User: summary (envCount / secretCount / total)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
📦 Canary Packages Publishedversion: PackagesInstallAdd to your {
"dependencies": {
"@agentuity/frontend": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-frontend-0.1.7-60c628a.tgz",
"@agentuity/schema": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-schema-0.1.7-60c628a.tgz",
"@agentuity/evals": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-evals-0.1.7-60c628a.tgz",
"@agentuity/cli": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-cli-0.1.7-60c628a.tgz",
"@agentuity/workbench": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-workbench-0.1.7-60c628a.tgz",
"@agentuity/react": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-react-0.1.7-60c628a.tgz",
"@agentuity/core": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-core-0.1.7-60c628a.tgz",
"@agentuity/auth": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-auth-0.1.7-60c628a.tgz",
"@agentuity/runtime": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-runtime-0.1.7-60c628a.tgz",
"@agentuity/server": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-server-0.1.7-60c628a.tgz"
}
}Or install directly: bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-frontend-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-schema-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-evals-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-cli-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-workbench-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-react-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-core-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-auth-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-runtime-0.1.7-60c628a.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.7-60c628a/agentuity-server-0.1.7-60c628a.tgzCLI Executables
Run Canary CLIagentuity canary 0.1.7-60c628a [command] [...args] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @apps/testing/integration-suite/src/test/cli-env-secrets.ts:
- Around line 106-129: Replace hardcoded env keys with uniqueId()-generated
names to prevent cross-test interference: import the uniqueId helper at the top
of the test file and for the 'cli-env-secrets' test named
'env-set-allows-agentuity-public-prefix' replace "AGENTUITY_PUBLIC_TEST_VAR"
with a variable like const key = `AGENTUITY_PUBLIC_${uniqueId()}` and use that
key in the cliAgent.run command and subsequent assertions (also update the other
two passing tests that reference VITE_TEST_VAR and PUBLIC_TEST_VAR in the same
file to use uniqueId() similarly). Ensure the generated key strings keep the
required prefixes (AGENTUITY_PUBLIC_, VITE_, PUBLIC_) when constructing the
unique keys and use those variables everywhere in the test logic/assertions.
- Around line 170-194: The test 'cli-env-secrets' /
'secret-set-allows-valid-key' uses a hardcoded key MY_TEST_SECRET_KEY; replace
it with a uniqueId-generated key (e.g., const key =
uniqueId('MY_TEST_SECRET_KEY') and use that variable in the cliAgent.run
command) to ensure test isolation, and add/import the uniqueId helper if not
already imported in this file; keep the rest of the assertions unchanged so they
validate against the generated key.
In @packages/cli/package.json:
- Around line 17-24: The package export for "./env-util" currently points to the
TypeScript source ("./src/env-util.ts"); update the package.json exports entry
to reference the compiled dist output instead (mirror how "./vite-plugin" uses
"./dist/..."), e.g., change both "import" and "types" for "./env-util" to point
at the corresponding files under "./dist" (ensure the compiled JS and .d.ts
exist in dist after build), so Node ESM consumers load the compiled module
rather than the TS source.
In @packages/cli/src/cmd/cloud/secret/push.ts:
- Around line 49-62: The warning message is misleading because publicSecretKeys
returned by validateNoPublicSecrets are reclassified into env and included in
the payload; update the message in the block that handles publicSecretKeys
(references: splitEnvAndSecrets, validateNoPublicSecrets, publicSecretKeys,
secrets, env, tui.warning) to state that these keys are being moved/reclassified
to env (e.g., "Reclassifying public variables to env: ...") instead of "Skipping
public variables", and keep the subsequent deletion from secrets and assignment
to env unchanged.
🧹 Nitpick comments (5)
packages/cli/src/cmd/cloud/secret/pull.ts (1)
58-63:pulled/success count may be wrong now that some keys are skipped from the file.You’re skipping reserved
AGENTUITY_keys (good), butcountstill reportsObject.keys(cloudSecrets).length, which can include keys that never made it into.env.One way to align the reported count with what’s written
await writeEnvFile(targetEnvPath, mergedEnv, { - skipKeys: Object.keys(mergedEnv).filter( + skipKeys: Object.keys(mergedEnv).filter( (k) => k.startsWith('AGENTUITY_') && !k.startsWith('AGENTUITY_PUBLIC_') ), }); @@ - const count = Object.keys(cloudSecrets).length; + const skipped = new Set( + Object.keys(mergedEnv).filter( + (k) => k.startsWith('AGENTUITY_') && !k.startsWith('AGENTUITY_PUBLIC_') + ) + ); + const count = Object.keys(mergedEnv).filter((k) => !skipped.has(k)).length; tui.success(`Pulled ${count} secret${count !== 1 ? 's' : ''} to ${targetEnvPath}`);Also applies to: 84-86
packages/cli/src/cmd/cloud/env/set.ts (1)
5-12: Error message should clarify the AGENTUITY_PUBLIC_ exception.The
isReservedAgentuityKey()check correctly allowsAGENTUITY_PUBLIC_prefixed variables, but the fatal error message "Cannot set AGENTUITY_ prefixed variables..." does not document this exception, which may confuse users. Update the message to indicate thatAGENTUITY_PUBLIC_*variables are permitted:Suggested change
- tui.fatal('Cannot set AGENTUITY_ prefixed variables. These are reserved for system use.'); + tui.fatal( + 'Cannot set AGENTUITY_ prefixed variables (except AGENTUITY_PUBLIC_*). These are reserved for system use.' + );Also applies to:
packages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/cmd/cloud/env/pull.ts (1)
58-62: UseisReservedAgentuityKey()(orfilterAgentuitySdkKeys()) instead of case-sensitivestartsWithchecks.Current
skipKeyswon’t catch lowercase/mixed-case keys and duplicates logic already centralized inenv-util.ts.Proposed change
-import { findExistingEnvFile, readEnvFile, writeEnvFile, mergeEnvVars } from '../../../env-util'; +import { + findExistingEnvFile, + readEnvFile, + writeEnvFile, + mergeEnvVars, + isReservedAgentuityKey, +} from '../../../env-util'; await writeEnvFile(targetEnvPath, mergedEnv, { - skipKeys: Object.keys(mergedEnv).filter( - (k) => k.startsWith('AGENTUITY_') && !k.startsWith('AGENTUITY_PUBLIC_') - ), + skipKeys: Object.keys(mergedEnv).filter(isReservedAgentuityKey), });packages/cli/src/cmd/cloud/env/import.ts (1)
139-142: Avoid duplicating reserved-key logic when writing the merged .env (case-sensitivity + drift risk).Since you already depend on
filterAgentuitySdkKeys(), you can just filter the merged object before writing (or useisReservedAgentuityKeylike elsewhere).Proposed change (simplify using existing helper)
- await writeEnvFile(localEnvPath, mergedEnv, { - skipKeys: Object.keys(mergedEnv).filter( - (k) => k.startsWith('AGENTUITY_') && !k.startsWith('AGENTUITY_PUBLIC_') - ), - }); + await writeEnvFile(localEnvPath, filterAgentuitySdkKeys(mergedEnv));packages/cli/src/cmd/cloud/secret/set.ts (1)
49-59: Validation logic matches the objective (reserved AGENTUITY_ blocked; public prefixes blocked as secrets).*Consider using
getCommand('env set ...')to construct the command suggestion for consistency with how this codebase generates command help text in examples.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/testing/integration-suite/app.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/package.jsonpackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/pull.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/secret/import.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/cmd/cloud/secret/push.tspackages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/env-util.tspackages/cli/test/config/env-util.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
packages/cli/test/config/env-util.test.tsapps/testing/integration-suite/app.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/env-util.tspackages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/cmd/cloud/secret/push.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/pull.tspackages/cli/src/cmd/cloud/secret/import.ts
packages/*/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/*/test/**/*.{ts,tsx}: Place test files intest/folder (never insrc/or__tests__/)
Import from../src/in test files to reference source code
Use@agentuity/test-utilsfor mocks and test helpers
Files:
packages/cli/test/config/env-util.test.ts
apps/testing/integration-suite/**/app.ts
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Test files must be imported in app.ts to register tests with the test suite
Files:
apps/testing/integration-suite/app.ts
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Usetui.*helpers for formatted output instead of raw console logs
Usectx.loggerfor logging; calllogger.fatal()to log and exit with code 1
Files:
packages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/cmd/cloud/secret/push.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/pull.tspackages/cli/src/cmd/cloud/secret/import.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Use
Bun.file(f).exists()instead ofexistsSync(f)for file existence checks
Files:
packages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/env-util.tspackages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/cmd/cloud/secret/push.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/pull.tspackages/cli/src/cmd/cloud/secret/import.ts
apps/testing/integration-suite/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Use path aliases for imports: @agents/* and @test/* instead of relative paths
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
apps/testing/integration-suite/src/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
apps/testing/integration-suite/src/test/**/*.{ts,tsx}: Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation
Test files must follow the pattern: setup section with uniqueId(), execute section calling agent.run(), and assert section with assertion helpers
Import test registration function and assertion helpers from @test/suite and @test/helpers
Use decodeKVValue() helper from @test/helpers/kv to decode stringified Uint8Arrays when reading KV values
Tests must be registered by calling test() function from @test/suite, not just defined
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/cli/test/config/env-util.test.tsapps/testing/integration-suite/app.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/env-util.tspackages/cli/src/cmd/cloud/secret/set.tspackages/cli/src/cmd/cloud/secret/push.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/pull.tspackages/cli/src/cmd/cloud/secret/import.ts
🧬 Code graph analysis (9)
packages/cli/test/config/env-util.test.ts (1)
packages/cli/src/env-util.ts (5)
isPublicVarKey(19-22)isReservedAgentuityKey(27-30)validateNoPublicSecrets(43-45)filterAgentuitySdkKeys(205-216)splitEnvAndSecrets(229-261)
packages/cli/src/cmd/cloud/env/set.ts (1)
packages/cli/src/env-util.ts (1)
isReservedAgentuityKey(27-30)
packages/cli/src/cmd/cloud/secret/pull.ts (1)
packages/cli/src/env-util.ts (1)
writeEnvFile(136-171)
packages/cli/src/cmd/cloud/secret/set.ts (1)
packages/cli/src/env-util.ts (3)
isReservedAgentuityKey(27-30)isPublicVarKey(19-22)PUBLIC_VAR_PREFIXES(14-14)
packages/cli/src/cmd/cloud/secret/push.ts (2)
packages/cli/src/env-util.ts (3)
filterAgentuitySdkKeys(205-216)splitEnvAndSecrets(229-261)validateNoPublicSecrets(43-45)packages/server/src/api/project/env-update.ts (1)
projectEnvUpdate(33-55)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (2)
apps/testing/integration-suite/src/test/suite.ts (1)
test(218-220)apps/testing/integration-suite/src/test/helpers/index.ts (2)
assertEqual(31-34)assert(22-26)
packages/cli/src/cmd/cloud/env/import.ts (1)
packages/cli/src/env-util.ts (1)
filterAgentuitySdkKeys(205-216)
packages/cli/src/cmd/cloud/env/pull.ts (1)
packages/cli/src/env-util.ts (1)
writeEnvFile(136-171)
packages/cli/src/cmd/cloud/secret/import.ts (2)
packages/cli/src/env-util.ts (7)
filterAgentuitySdkKeys(205-216)splitEnvAndSecrets(229-261)validateNoPublicSecrets(43-45)findExistingEnvFile(50-52)readEnvFile(111-130)mergeEnvVars(178-198)writeEnvFile(136-171)packages/server/src/api/project/env-update.ts (1)
projectEnvUpdate(33-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build
- GitHub Check: Pack & Upload
- GitHub Check: Sandbox CLI Tests
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Cloud Deployment Tests
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Template Integration Tests
🔇 Additional comments (20)
apps/testing/integration-suite/app.ts (1)
10-30: Good: new integration tests are registered inapp.ts.This follows the integration-suite rule that test modules must be imported in
app.tsto register with the suite.packages/cli/src/cmd/cloud/env/push.ts (1)
40-42: LGTM: push filters reservedAGENTUITY_while keepingAGENTUITY_PUBLIC_eligible.packages/cli/test/config/env-util.test.ts (1)
2-9: Test coverage for public/reserved key classification and env-vs-secret splitting looks solid.The new cases cover the key prefix rules, case-insensitivity, reserved key exclusion, and
splitEnvAndSecretsbehavior in a way that matches the CLI contract.Also applies to: 203-347
packages/cli/src/cmd/cloud/env/import.ts (1)
71-76: Updated messaging (“reserved AGENTUITY_ prefixed”) matches the new reserved/public split.packages/cli/src/cmd/cloud/secret/push.ts (1)
22-22: Update command description and spinner text to accurately reflect that both env and secrets are pushed.Line 22 and 74 say "secrets" but the implementation pushes both
envandsecrets(see lines 77–78). The backend correctly merges these with existing values (per the comment inenv-update.ts), so there's no risk of unintended overwrites, but the copy should match the actual behavior to avoid user confusion.packages/cli/src/cmd/cloud/secret/import.ts (7)
11-12: LGTM! Validation imports are correctly added.The new imports for
validateNoPublicSecretsandsplitEnvAndSecretsare correctly sourced from the env-util module and align with the PR objectives.
68-80: LGTM! Filtering logic correctly updated.The variable rename from
filteredSecretstofilteredVarsaccurately reflects the new semantics where the filtered set can contain both env vars and secrets. The updated early-return message correctly identifies that reservedAGENTUITY_prefixed keys are filtered out (exceptAGENTUITY_PUBLIC_).
82-95: LGTM! Public variable validation and correction logic is sound.The implementation correctly:
- Splits filtered variables into env and secrets using the new utility
- Validates that no public variables ended up in secrets
- Moves any public variables from secrets to env with an appropriate warning
- Provides clear messaging about frontend exposure
101-102: LGTM! Cloud push payload correctly structured.The
projectEnvUpdatecall now correctly sends bothenvandsecretsas separate fields, which aligns with the API contract shown in the relevant code snippets and supports the new env/secret split logic.
109-109: LGTM! Local merge uses correct variable.Using
filteredVars(which contains both env and secrets before the split) for the local .env merge is correct, as the local file should contain all imported variables.
112-114: LGTM! Skip keys logic correctly refined.The
skipKeysfilter now correctly excludes reservedAGENTUITY_keys while allowingAGENTUITY_PUBLIC_keys to be written to the local .env file. This aligns with the PR objectives.
117-127: LGTM! Count calculation and reporting are accurate.The updated logic correctly:
- Calculates separate counts for env and secrets
- Reports total imported variables with breakdown
- Adjusts skipped count to account for the total (env + secrets) rather than just secrets
The success message provides clear visibility into how variables were categorized.
apps/testing/integration-suite/src/test/cli-env-secrets.ts (3)
10-13: LGTM! Imports follow path alias conventions.The imports correctly use
@test/suite,@test/helpers, and@agents/cli/agentpath aliases as specified in the coding guidelines for integration tests.
16-31: Missing uniqueId() for test isolation.The tests directly use hardcoded key names like
AGENTUITY_SDK_KEY,VITE_API_KEY,PUBLIC_URL, andAGENTUITY_PUBLIC_KEY. Per coding guidelines, "Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation."While these specific keys are meant to test validation rules and won't be persisted to the cloud (since they're rejected), the guideline applies to all identifiers. Consider whether test isolation is necessary here.
If these keys are purely for validation testing and never reach the cloud/storage layer (i.e., they fail early validation), test isolation may not be strictly necessary. However, if there's any chance of cross-test interference or if the guideline is absolute, these should use uniqueId().
Can you confirm whether validation-only tests that never persist data still require uniqueId() for identifiers?
Also applies to: 34-49, 52-67, 70-85
16-194: Test structure follows recommended pattern.All tests follow the expected pattern:
- Setup section with authentication check
- Execute section calling
cliAgent.run()- Assert section with appropriate assertion helpers (
assertEqual,assert)The tests are properly registered using the
test()function from@test/suite, and assertions use helpers from@test/helpersas required by the coding guidelines.packages/cli/src/env-util.ts (5)
11-22: LGTM! Public variable detection is correctly implemented.The
PUBLIC_VAR_PREFIXESconstant andisPublicVarKeyfunction correctly identify public/frontend-exposed variables. The implementation:
- Uses
as constfor type safety- Converts keys to uppercase for case-insensitive comparison
- Covers all three public prefixes mentioned in the PR objectives (VITE_, AGENTUITY_PUBLIC_, PUBLIC_)
24-30: LGTM! Reserved key detection correctly excludes AGENTUITY_PUBLIC_.The
isReservedAgentuityKeyfunction correctly identifies reservedAGENTUITY_prefixed keys while explicitly allowingAGENTUITY_PUBLIC_keys. The case-insensitive comparison ensures consistent behavior.
32-45: LGTM! Validation functions are correctly structured.Both
getPublicSecretKeysandvalidateNoPublicSecretsare correctly implemented:
getPublicSecretKeysidentifies public variables that were incorrectly placed in secretsvalidateNoPublicSecretsprovides a semantic wrapper with clear naming for validation use cases- The implementation correctly leverages
isPublicVarKeyfor filtering
200-216: LGTM! filterAgentuitySdkKeys correctly updated.The function now:
- Safely handles
undefinedinput by returning an empty object- Uses the centralized
isReservedAgentuityKeyfunction for consistent filtering- Correctly allows
AGENTUITY_PUBLIC_keys through (since they're not reserved)- Maintains the original filtering semantics while improving consistency
The updated JSDoc comment accurately reflects the new behavior.
227-261: LGTM! splitEnvAndSecrets correctly handles new rules.The updated function correctly implements the env/secret split logic:
- Skips reserved
AGENTUITY_keys (exceptAGENTUITY_PUBLIC_) early in the loop (lines 239-242)- Routes public variables to env, never secrets (lines 244-248)
- Maintains the original suffix and exact-match logic for secret detection
- Updated JSDoc comment accurately documents the new behavior
The implementation prevents public variables from becoming secrets while preserving backward-compatible secret detection for non-public keys.
| "./vite-plugin": { | ||
| "import": "./dist/cmd/build/vite/index.js", | ||
| "types": "./dist/cmd/build/vite/index.d.ts" | ||
| }, | ||
| "./env-util": { | ||
| "import": "./src/env-util.ts", | ||
| "types": "./src/env-util.ts" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where env-util compiles to, and whether dist artifacts exist.
set -euo pipefail
echo "== Find env-util source =="
fd -a 'env-util\.ts$' packages/cli/src || echo "No matches"
echo "== Check for built artifacts (common locations) =="
fd -a 'env-util\.(js|d\.ts)$' packages/cli/dist packages/cli || echo "No matches"
echo "== Inspect package exports block =="
python3 <<'PY'
import json
p='packages/cli/package.json'
with open(p,'r',encoding='utf-8') as f:
data=json.load(f)
print(json.dumps(data.get("exports",{}).get("./env-util",{}), indent=2))
PY
echo "== Check build script in package.json =="
python3 <<'PY'
import json
p='packages/cli/package.json'
with open(p,'r',encoding='utf-8') as f:
data=json.load(f)
scripts = data.get("scripts", {})
for key in ["build", "build:*"]:
if key in scripts:
print(f"{key}: {scripts[key]}")
for key in scripts:
if "build" in key:
print(f"{key}: {scripts[key]}")
PY
echo "== List contents of packages/cli/src =="
ls -la packages/cli/src/ 2>/dev/null | head -20 || echo "Directory not found"
echo "== List contents of packages/cli/dist (if exists) =="
ls -la packages/cli/dist/ 2>/dev/null | head -20 || echo "Directory not found or empty"Repository: agentuity/sdk
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check env-util source existence =="
find packages/cli/src -name "*env-util*" -type f 2>/dev/null | head -20
echo "== Extract exports.env-util from package.json =="
cat packages/cli/package.json | grep -A 5 '"./env-util"' || echo "Pattern not found"
echo "== Check if dist directory exists =="
test -d packages/cli/dist && echo "dist exists" || echo "dist does not exist"Repository: agentuity/sdk
Length of output: 333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check build scripts =="
grep -A 3 '"build"' packages/cli/package.json | head -10
echo "== Check files field =="
grep -A 10 '"files"' packages/cli/package.json
echo "== Check tsconfig existence =="
ls -la packages/cli/tsconfig* 2>/dev/null | head -5 || echo "No tsconfig found"
echo "== Check all exports =="
python3 -c "
import json
with open('packages/cli/package.json', 'r') as f:
exports = json.load(f).get('exports', {})
for key in sorted(exports.keys()):
print(f'{key}:')
print(f' {exports[key]}')
" 2>/dev/null || echo "Failed to parse"Repository: agentuity/sdk
Length of output: 1077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check tsconfig.json output configuration =="
cat packages/cli/tsconfig.json
echo "== Check if env-util.ts would compile to a separate dist file or as part of index =="
grep -r "env-util" packages/cli/src --include="*.ts" | head -5Repository: agentuity/sdk
Length of output: 933
Export ./env-util from compiled dist output, not TypeScript source.
The export currently points to ./src/env-util.ts (line 21-23), which will break Node ESM consumers without a TypeScript loader. The tsconfig compiles all src/**/* to dist/ (outDir + rootDir), and dist is included in published files, but the export ignores the compiled output. This is inconsistent with ./vite-plugin and the main export, both of which correctly reference ./dist/*.
Required change
"./env-util": {
- "import": "./src/env-util.ts",
- "types": "./src/env-util.ts"
+ "import": "./dist/env-util.js",
+ "types": "./dist/env-util.d.ts"
}📝 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.
| "./vite-plugin": { | |
| "import": "./dist/cmd/build/vite/index.js", | |
| "types": "./dist/cmd/build/vite/index.d.ts" | |
| }, | |
| "./env-util": { | |
| "import": "./src/env-util.ts", | |
| "types": "./src/env-util.ts" | |
| } | |
| "./vite-plugin": { | |
| "import": "./dist/cmd/build/vite/index.js", | |
| "types": "./dist/cmd/build/vite/index.d.ts" | |
| }, | |
| "./env-util": { | |
| "import": "./dist/env-util.js", | |
| "types": "./dist/env-util.d.ts" | |
| } |
🤖 Prompt for AI Agents
In @packages/cli/package.json around lines 17 - 24, The package export for
"./env-util" currently points to the TypeScript source ("./src/env-util.ts");
update the package.json exports entry to reference the compiled dist output
instead (mirror how "./vite-plugin" uses "./dist/..."), e.g., change both
"import" and "types" for "./env-util" to point at the corresponding files under
"./dist" (ensure the compiled JS and .d.ts exist in dist after build), so Node
ESM consumers load the compiled module rather than the TS source.
| // Split into env and secrets (public vars will be in env, not secrets) | ||
| const { env, secrets } = splitEnvAndSecrets(filteredEnv); | ||
|
|
||
| // Check for any public vars that would have been treated as secrets | ||
| const publicSecretKeys = validateNoPublicSecrets(secrets); | ||
| if (publicSecretKeys.length > 0) { | ||
| tui.warning( | ||
| `Skipping public variables: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)` | ||
| ); | ||
| for (const key of publicSecretKeys) { | ||
| delete secrets[key]; | ||
| env[key] = filteredEnv[key]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Warning text is misleading: you’re not “skipping” public vars, you’re reclassifying them to env.
This currently says “Skipping public variables …” but then moves them into env and includes them in the update payload.
Proposed change
- tui.warning(
- `Skipping public variables: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)`
- );
+ tui.warning(
+ `Public variables cannot be secrets. Treating as env: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)`
+ );📝 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.
| // Split into env and secrets (public vars will be in env, not secrets) | |
| const { env, secrets } = splitEnvAndSecrets(filteredEnv); | |
| // Check for any public vars that would have been treated as secrets | |
| const publicSecretKeys = validateNoPublicSecrets(secrets); | |
| if (publicSecretKeys.length > 0) { | |
| tui.warning( | |
| `Skipping public variables: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)` | |
| ); | |
| for (const key of publicSecretKeys) { | |
| delete secrets[key]; | |
| env[key] = filteredEnv[key]; | |
| } | |
| } | |
| // Split into env and secrets (public vars will be in env, not secrets) | |
| const { env, secrets } = splitEnvAndSecrets(filteredEnv); | |
| // Check for any public vars that would have been treated as secrets | |
| const publicSecretKeys = validateNoPublicSecrets(secrets); | |
| if (publicSecretKeys.length > 0) { | |
| tui.warning( | |
| `Public variables cannot be secrets. Treating as env: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)` | |
| ); | |
| for (const key of publicSecretKeys) { | |
| delete secrets[key]; | |
| env[key] = filteredEnv[key]; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @packages/cli/src/cmd/cloud/secret/push.ts around lines 49 - 62, The warning
message is misleading because publicSecretKeys returned by
validateNoPublicSecrets are reclassified into env and included in the payload;
update the message in the block that handles publicSecretKeys (references:
splitEnvAndSecrets, validateNoPublicSecrets, publicSecretKeys, secrets, env,
tui.warning) to state that these keys are being moved/reclassified to env (e.g.,
"Reclassifying public variables to env: ...") instead of "Skipping public
variables", and keep the subsequent deletion from secrets and assignment to env
unchanged.
…lic-var-secret-validation
…m/agentuity/sdk into feat/public-var-secret-validation
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/cli/src/cmd/cloud/env/get.ts:
- Around line 31-32: Remove the schema-level default for mask (drop
.default(!!process.stdout.isTTY)) and compute the effective mask inside the
handler: determine maskDefault = isSecret ? true : !!process.stdout.isTTY, then
set shouldMask = (opts?.mask !== undefined) ? opts.mask : maskDefault; replace
any existing shouldMask logic that uses opts?.mask !== false / === true and
reference the symbols isSecret, opts.mask, and shouldMask (in the handler that
formats env output) so secrets are masked by default even in non-TTY unless the
user explicitly passes --no-mask.
In @packages/cli/src/cmd/cloud/env/push.ts:
- Around line 65-75: The warning message is misleading: validateNoPublicSecrets
returns keys that are moved from secrets into env (not skipped). Update the
tui.warning call that references publicSecretKeys to say these keys are being
moved to env/exposed to the frontend (e.g., "Moving public variables to env:
...") so it matches the subsequent logic that deletes from secrets and assigns
env[key] = filteredEnv[key]; keep the same variables (publicSecretKeys, secrets,
env, filteredEnv) and location around that block so the message accurately
describes the action.
🧹 Nitpick comments (2)
apps/testing/integration-suite/src/test/cli-deployment.ts (1)
226-231: Clarify scope of this placeholder test.The
deploy-syncs-env-varstest covers functionality that is not tested elsewhere incli-env-secrets.ts(which contains only env-set and secret validation tests, no deployment-related tests). The placeholder addresses a real gap in deployment env sync coverage.Decide whether this test should be implemented now or deferred as a placeholder. If implemented, follow the required test pattern: setup section with
uniqueId(), execute section callingagent.run(), and assert section with assertion helpers from@test/helpers.packages/cli/src/cmd/cloud/env/list.ts (1)
33-34: Consider adding mutual exclusivity validation for--secretsand--env-onlyflags.Both flags can technically be set to
truesimultaneously (e.g.,env list --secrets --env-only), which would result in showing all variables—the same as the default behavior. While not incorrect, this might confuse users.💡 Optional: Add validation or clarify behavior
You could either:
- Make the flags mutually exclusive with a validation check
- Document that setting both is equivalent to the default behavior
+ // If both filters are set, it's equivalent to showing all + if (opts?.secrets && opts?.['env-only']) { + tui.warning('Both --secrets and --env-only specified; showing all variables'); + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/testing/integration-suite/src/test/cli-deployment.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/README.mdpackages/cli/docs/secret-detection.mdpackages/cli/src/cmd/ai/capabilities/show.tspackages/cli/src/cmd/ai/prompt/llm.tspackages/cli/src/cmd/cloud/env/delete.tspackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/index.tspackages/cli/src/cmd/cloud/env/list.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/index.tspackages/cli/src/cmd/cloud/secret/delete.tspackages/cli/src/cmd/cloud/secret/get.tspackages/cli/src/cmd/cloud/secret/import.tspackages/cli/src/cmd/cloud/secret/index.tspackages/cli/src/cmd/cloud/secret/list.tspackages/cli/src/cmd/cloud/secret/pull.tspackages/cli/src/cmd/cloud/secret/push.tspackages/cli/src/cmd/cloud/secret/set.ts
💤 Files with no reviewable changes (9)
- packages/cli/src/cmd/cloud/index.ts
- packages/cli/src/cmd/cloud/secret/list.ts
- packages/cli/src/cmd/cloud/secret/pull.ts
- packages/cli/src/cmd/cloud/secret/get.ts
- packages/cli/src/cmd/cloud/secret/import.ts
- packages/cli/src/cmd/cloud/secret/index.ts
- packages/cli/src/cmd/cloud/secret/delete.ts
- packages/cli/src/cmd/cloud/secret/set.ts
- packages/cli/src/cmd/cloud/secret/push.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧰 Additional context used
📓 Path-based instructions (5)
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Usetui.*helpers for formatted output instead of raw console logs
Usectx.loggerfor logging; calllogger.fatal()to log and exit with code 1
Files:
packages/cli/src/cmd/ai/capabilities/show.tspackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/ai/prompt/llm.tspackages/cli/src/cmd/cloud/env/index.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/delete.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/list.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Use
Bun.file(f).exists()instead ofexistsSync(f)for file existence checks
Files:
packages/cli/src/cmd/ai/capabilities/show.tspackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/ai/prompt/llm.tspackages/cli/src/cmd/cloud/env/index.tspackages/cli/src/cmd/cloud/env/set.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/delete.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/list.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
packages/cli/src/cmd/ai/capabilities/show.tspackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/ai/prompt/llm.tspackages/cli/src/cmd/cloud/env/index.tspackages/cli/src/cmd/cloud/env/set.tsapps/testing/integration-suite/src/test/cli-deployment.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/delete.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/list.ts
apps/testing/integration-suite/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Use path aliases for imports: @agents/* and @test/* instead of relative paths
Files:
apps/testing/integration-suite/src/test/cli-deployment.ts
apps/testing/integration-suite/src/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
apps/testing/integration-suite/src/test/**/*.{ts,tsx}: Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation
Test files must follow the pattern: setup section with uniqueId(), execute section calling agent.run(), and assert section with assertion helpers
Import test registration function and assertion helpers from @test/suite and @test/helpers
Use decodeKVValue() helper from @test/helpers/kv to decode stringified Uint8Arrays when reading KV values
Tests must be registered by calling test() function from @test/suite, not just defined
Files:
apps/testing/integration-suite/src/test/cli-deployment.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/cli/src/cmd/ai/capabilities/show.tspackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/ai/prompt/llm.tspackages/cli/src/cmd/cloud/env/index.tspackages/cli/src/cmd/cloud/env/set.tsapps/testing/integration-suite/src/test/cli-deployment.tspackages/cli/src/cmd/cloud/env/import.tspackages/cli/src/cmd/cloud/env/delete.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/list.ts
🧬 Code graph analysis (4)
packages/cli/src/cmd/cloud/env/get.ts (1)
packages/server/src/api/project/get.ts (1)
projectGet(30-48)
apps/testing/integration-suite/src/test/cli-deployment.ts (2)
apps/testing/integration-suite/src/test/suite.ts (1)
test(218-220)apps/testing/integration-suite/src/test/helpers/index.ts (1)
assert(22-26)
packages/cli/src/cmd/cloud/env/delete.ts (2)
packages/server/src/api/project/get.ts (1)
projectGet(30-48)packages/server/src/api/project/env-delete.ts (1)
projectEnvDelete(20-41)
packages/cli/src/cmd/cloud/env/push.ts (2)
packages/cli/src/env-util.ts (5)
findExistingEnvFile(50-52)readEnvFile(111-130)filterAgentuitySdkKeys(205-216)splitEnvAndSecrets(229-261)validateNoPublicSecrets(43-45)packages/server/src/api/project/env-update.ts (1)
projectEnvUpdate(33-55)
🔇 Additional comments (11)
packages/cli/src/cmd/cloud/env/index.ts (1)
13-24: LGTM! Documentation updates align with new secret handling capabilities.The updated description and examples clearly communicate the unified env/secrets management approach. The new
--secretexample is helpful for discoverability.packages/cli/src/cmd/cloud/env/delete.ts (2)
47-52: Null safety check is correctly handled via optional chaining.The use of
projectData.secrets?.[args.key]andprojectData.env?.[args.key]correctly handles cases wheresecretsorenvmight be undefined. The logic correctly identifies whether the key exists as a secret or environment variable.
54-81: LGTM! Clean implementation of conditional deletion.The handler correctly:
- Determines the key type by checking cloud data
- Uses the appropriate field (
secretsvsenv) in the deletion payload- Provides clear user feedback distinguishing between secret and env var deletions
- Returns the
secretflag for programmatic consumerspackages/cli/src/cmd/cloud/env/list.ts (1)
89-101: LGTM! Well-implemented masking and display logic.The masking correctly applies only to secrets when the mask option is enabled. The output clearly distinguishes between env vars and secrets with the
[secret]indicator, and the dynamic count header provides useful context.packages/cli/src/cmd/cloud/env/set.ts (3)
59-69: LGTM! Validation logic correctly enforces the PR requirements.The validation correctly:
- First checks for reserved
AGENTUITY_*prefixed keys (exceptAGENTUITY_PUBLIC_*)- Then validates that public var prefixes cannot be stored as secrets
The error messages are clear and include the list of public prefixes for user clarity.
83-110: LGTM! Clean implementation of conditional secret/env storage.The handler correctly constructs the appropriate payload based on the
isSecretflag and provides clear feedback to the user. The response schema properly includes thesecretboolean for programmatic consumers.
71-80: Thetui.confirmprompt handles non-interactive environments safely and will not hang.The
confirm()function at line 75 already gracefully handles non-TTY environments (CI/non-interactive pipelines) by returning thedefaultValue(true) without prompting. However, this means the auto-detection will silently store the variable as a secret in CI without user awareness, which may not be the desired behavior for automated pipelines. Consider adding a TTY check before triggering the auto-detection prompt (e.g.,if (process.stdin.isTTY && !isSecret && looksLikeSecret(...))) to skip the prompt in non-interactive environments.Likely an incorrect or invalid review comment.
packages/cli/src/cmd/cloud/env/push.ts (1)
86-100: LGTM! Response includes detailed breakdown of pushed items.The success message and return value provide useful information about the number of env vars vs secrets pushed.
packages/cli/src/cmd/cloud/env/import.ts (3)
97-104: Good: Import correctly handles public variables with clear messaging.Unlike push.ts, the warning message here accurately states "Moving public variables to env" which matches the actual behavior. This is the correct approach.
136-142: LGTM! Skipped count correctly reflects filtered reserved keys.The
skippedcalculation (Object.keys(importedVars).length - totalCount) correctly represents the number of reservedAGENTUITY_*keys that were filtered out, sincetotalCountequals all keys that made it through the filter (regardless of whether they ended up as env or secrets).
115-124: Verify: Secrets are written to local .env file.The merge logic writes all
filteredVars(including secrets) to the local.envfile. This may be intentional for local development convenience, but it could also pose a security concern if users expect secrets to remain only in the cloud.Is this the intended behavior? If so, consider adding a comment clarifying that imported secrets are also written locally for development purposes. If not, the merge should exclude secret keys:
- const mergedEnv = mergeEnvVars(localEnv, filteredVars); + // Merge only env vars (not secrets) to local .env + const mergedEnv = mergeEnvVars(localEnv, env);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/testing/integration-suite/src/test/cli-env-secrets.ts:
- Around line 117-120: The asserts pass values that can be undefined (e.g.,
setResult.stdout?.includes(...), setResult.success) but assert requires a
boolean; update each failing assertion (examples include the `assert` using
`setResult.success || setResult.stdout?.includes('set successfully')` and other
occurrences listed) to coerce the condition to a strict boolean by applying `!!`
or `?? false` (for example `!!setResult.success ||
!!setResult.stdout?.includes(...)` or `(setResult.success ||
setResult.stdout?.includes(...)) ?? false`) so every `assert(condition,
message)` call receives a boolean; apply this pattern to all affected assertions
on lines noted (118, 129, 153, 164, 190, 206, 232, 242, 266, 276, 299, 319, 357,
398, 442, 500, 506, 550, 573, 584) referencing the same `assert` usages.
🧹 Nitpick comments (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
110-111: UseuniqueId()helper instead ofDate.now()for test keys.The coding guidelines require using
uniqueId()for all keys and identifiers in tests to ensure test isolation. Currently,Date.now()is used which may cause collisions if tests run within the same millisecond.♻️ Proposed fix
Import and use
uniqueId():import { test } from '@test/suite'; import { assert, assertEqual } from '@test/helpers'; import cliAgent from '@agents/cli/agent'; import { isAuthenticated } from '@test/helpers/cli'; +import { uniqueId } from '@test/helpers'; // ... - const testKey = `MASK_TEST_${Date.now()}`; + const testKey = `MASK_TEST_${uniqueId()}`;Apply this pattern to all test key declarations throughout the file.
As per coding guidelines, tests must use
uniqueId()helper for all keys to ensure test isolation.Also applies to: 145-146, 179-180, 222-223, 256-257, 290-291, 333-334, 371-372, 412-413, 484-485, 528-529, 565-566
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/src/cmd/cloud/env/list.ts
🧰 Additional context used
📓 Path-based instructions (5)
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Usetui.*helpers for formatted output instead of raw console logs
Usectx.loggerfor logging; calllogger.fatal()to log and exit with code 1
Files:
packages/cli/src/cmd/cloud/env/list.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Use
Bun.file(f).exists()instead ofexistsSync(f)for file existence checks
Files:
packages/cli/src/cmd/cloud/env/list.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
packages/cli/src/cmd/cloud/env/list.tsapps/testing/integration-suite/src/test/cli-env-secrets.ts
apps/testing/integration-suite/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Use path aliases for imports: @agents/* and @test/* instead of relative paths
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
apps/testing/integration-suite/src/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
apps/testing/integration-suite/src/test/**/*.{ts,tsx}: Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation
Test files must follow the pattern: setup section with uniqueId(), execute section calling agent.run(), and assert section with assertion helpers
Import test registration function and assertion helpers from @test/suite and @test/helpers
Use decodeKVValue() helper from @test/helpers/kv to decode stringified Uint8Arrays when reading KV values
Tests must be registered by calling test() function from @test/suite, not just defined
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
packages/cli/src/cmd/cloud/env/list.tsapps/testing/integration-suite/src/test/cli-env-secrets.ts
🧬 Code graph analysis (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (3)
apps/testing/integration-suite/src/test/suite.ts (1)
test(218-220)apps/testing/integration-suite/src/test/helpers/index.ts (2)
assertEqual(31-34)assert(22-26)packages/cli/src/tui.ts (1)
output(430-432)
🪛 GitHub Actions: Build Packages & Test
apps/testing/integration-suite/src/test/cli-env-secrets.ts
[error] 118-118: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 129-129: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'boolean'.
[error] 153-153: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 164-164: TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'boolean'.
[error] 190-190: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
206-206: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 232-232: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 242-242: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 266-266: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 276-276: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 299-299: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 319-319: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 357-357: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 398-398: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 442-442: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 500-500: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 506-506: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 550-550: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 573-573: TS2345: Argument of type 'boolean | undefined' is not assignable to parameter of type 'boolean'.
[error] 584-584: TS2345: Argument of type 'string | boolean | undefined' is not assignable to parameter of type 'boolean'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Sandbox CLI Tests
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Cloud Deployment Tests
- GitHub Check: Template Integration Tests
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: Pack & Upload
🔇 Additional comments (4)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
1-14: LGTM on overall test structure and coverage.The test module correctly:
- Documents the validation rules in the header comment
- Uses path aliases (
@test/suite,@test/helpers,@agents/cli/agent) as required- Follows the setup → execute → assert pattern
- Registers tests using the
test()function from@test/suite- Covers the key scenarios: reserved key blocking, public prefix blocking for secrets, masking behavior, CRUD operations, and auto-detection
packages/cli/src/cmd/cloud/env/list.ts (3)
7-12: LGTM on schema definitions.The
EnvItemSchemaandEnvListResponseSchemaare well-structured. Thez.record(z.string(), EnvItemSchema)pattern correctly maps string keys to the item schema in Zod 4.
40-68: LGTM on handler fetch and filtering logic.The implementation correctly:
- Fetches unmasked values to enable proper masking at display time
- Builds a combined result object with type information
- Filters based on
--secretsand--env-onlyflags with sensible defaults (show both when neither flag is set)
89-101: LGTM on masking and display logic.The masking logic correctly applies only to secrets when
maskis true (the default), and the type indicator[secret]provides clear differentiation. Usingtui.bold(),tui.muted(), andtui.maskSecret()aligns with the coding guidelines for CLI output formatting.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/testing/integration-suite/src/test/cli-env-secrets.ts:
- Line 110: Replace Date.now() usages used to generate test keys with the
uniqueId() helper to avoid collisions; specifically update the constant
assignment for testKey (`MASK_TEST_${Date.now()}`) to `MASK_TEST_${uniqueId()}`
and apply the same replacement for all other test key templates mentioned
(NOMASK_TEST_, AGENTUITY_PUBLIC_TEST_, VITE_TEST_, PUBLIC_TEST_, SECRET_KEY_,
MY_API_KEY_, CONFIG_VAL_, NORMAL_VAR_, CRUD_TEST_, OVERWRITE_TEST_,
CONVERT_TEST_) so each becomes e.g. `NOMASK_TEST_${uniqueId()}`; ensure
uniqueId() is imported/available in the test file before use.
- Around line 10-13: Add the missing uniqueId() import to the existing helper
imports so tests use uniqueId for keys/identifiers: update the import line that
currently imports assert and assertEqual from '@test/helpers' to also import
uniqueId (e.g., import { assert, assertEqual, uniqueId } from '@test/helpers')
and then replace any hardcoded test keys in the CLI env secrets test with calls
to uniqueId() where required for isolation.
🧹 Nitpick comments (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
17-18: Consider extracting authentication guard into a helper.The authentication check pattern
const authenticated = await isAuthenticated(); if (!authenticated) return;is repeated in every test. Consider creating a test wrapper or setup hook to reduce duplication.♻️ Example helper pattern
async function authenticatedTest(fn: () => Promise<void>): Promise<void> { const authenticated = await isAuthenticated(); if (!authenticated) return; await fn(); }Usage:
test('cli-env-secrets', 'env-set-secret-blocks-reserved-agentuity-key', async () => { await authenticatedTest(async () => { const result = await cliAgent.run({ command: 'cloud env set AGENTUITY_SDK_KEY test_value --secret', }); // ... assertions }); });Also applies to: 35-36, 53-54, 71-72, 89-90, 107-108, 142-143, 176-177, 221-222, 257-258, 293-294, 338-339, 376-377, 417-418, 461-462, 475-476, 489-490, 533-534, 570-571
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/testing/integration-suite/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Use path aliases for imports: @agents/* and @test/* instead of relative paths
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
apps/testing/integration-suite/src/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
apps/testing/integration-suite/src/test/**/*.{ts,tsx}: Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation
Test files must follow the pattern: setup section with uniqueId(), execute section calling agent.run(), and assert section with assertion helpers
Import test registration function and assertion helpers from @test/suite and @test/helpers
Use decodeKVValue() helper from @test/helpers/kv to decode stringified Uint8Arrays when reading KV values
Tests must be registered by calling test() function from @test/suite, not just defined
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
apps/testing/integration-suite/src/test/cli-env-secrets.ts
🧬 Code graph analysis (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (2)
apps/testing/integration-suite/src/test/suite.ts (1)
test(218-220)apps/testing/integration-suite/src/test/helpers/index.ts (2)
assertEqual(31-34)assert(22-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Pack & Upload
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Build
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Cloud Deployment Tests
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Sandbox CLI Tests
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: Template Integration Tests
🔇 Additional comments (1)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
16-617: Comprehensive test coverage with good assertion patterns.The test suite provides excellent coverage of:
- Reserved key validation (AGENTUITY_* blocking)
- Public var prefix validation (VITE_, PUBLIC_, AGENTUITY_PUBLIC_*)
- Secret masking behavior (default masking, --no-mask flag)
- Auto-detection of secret-like keys/values
- Full CRUD operations with cleanup
- Edge cases (not found, overwrite, secret-to-env conversion)
The defensive assertion patterns (checking both
result.successandstdoutcontent) handle different success/failure reporting methods well.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/cli/src/cmd/cloud/env/get.ts:
- Around line 59-61: The handler uses opts?.mask but the CLI schema defines the
option as maskSecret, causing a TS property error; update the code around
maskDefault/shouldMask to read opts?.maskSecret (and treat it as boolean)
instead of opts?.mask so shouldMask = opts?.maskSecret !== undefined ?
opts.maskSecret : maskDefault and keep the existing logic using isSecret and
process.stdout.isTTY unchanged.
🧹 Nitpick comments (4)
packages/cli/src/cmd/cloud/env/push.ts (2)
48-49: Clarify the comment to reflect thatAGENTUITY_PUBLIC_*keys are allowed.The comment states "Filter out reserved AGENTUITY_ prefixed keys," but according to the PR objectives,
filterAgentuitySdkKeys()now allowsAGENTUITY_PUBLIC_*keys through. Consider updating the comment to be more precise.📝 Suggested comment clarification
- // Filter out reserved AGENTUITY_ prefixed keys + // Filter out reserved AGENTUITY_ prefixed keys (except AGENTUITY_PUBLIC_*) const filteredEnv = filterAgentuitySdkKeys(localEnv);
65-75: Consider removing redundant public secret validation.The
splitEnvAndSecretsfunction already routes public variables (identified byisPublicVarKey) to theenvobject rather thansecrets(see lines 239-242 in env-util.ts). This meansvalidateNoPublicSecrets(secrets)should always return an empty array, making the validation and relocation logic at lines 66-75 redundant.If this is intentional defensive programming, consider adding a comment explaining why. Otherwise, this code can be simplified.
♻️ Proposed simplification
// Split into env and secrets based on key naming conventions const { env, secrets } = splitEnvAndSecrets(filteredEnv); - - // Check for any public vars that would have been treated as secrets - const publicSecretKeys = validateNoPublicSecrets(secrets); - if (publicSecretKeys.length > 0) { - tui.warning( - `Moving public variables to env: ${publicSecretKeys.join(', ')} (these are exposed to the frontend)` - ); - for (const key of publicSecretKeys) { - delete secrets[key]; - env[key] = filteredEnv[key]; - } - }Note: Only apply this change if the defensive check is confirmed to be unnecessary. If there are edge cases where public vars might end up in secrets despite
splitEnvAndSecretslogic, keep the validation.apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
133-138: Consider using the command string format consistently for delete.Other tests pass options like
--secretdirectly in the command string (e.g., line 115:`cloud env set ${testKey} ${testValue} --secret`), but the cleanup here uses theargsarray format. While both work, consistency would improve readability.♻️ Suggested change for consistency
// Cleanup await cliAgent.run({ - command: 'cloud env delete', - args: [testKey], + command: `cloud env delete ${testKey}`, });apps/testing/integration-suite/scripts/ci-test.sh (1)
166-169: Remove or use the unusedEVENTvariable.The static analysis hint correctly identifies that
EVENTis assigned but never used. Either remove the capture or use it for logging/debugging if needed.♻️ Option 1: Remove unused variable
if [[ "$line" =~ ^event:\ (.*)$ ]]; then - EVENT="${BASH_REMATCH[1]}" continue fi♻️ Option 2: Keep for debugging (suppress warning)
if [[ "$line" =~ ^event:\ (.*)$ ]]; then - EVENT="${BASH_REMATCH[1]}" + # shellcheck disable=SC2034 + EVENT="${BASH_REMATCH[1]}" # Captured for potential debugging continue fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/testing/integration-suite/scripts/ci-test.shapps/testing/integration-suite/src/test/api-route.tsapps/testing/integration-suite/src/test/cli-env-secrets.tsapps/testing/integration-suite/src/test/helpers/index.tsapps/testing/integration-suite/src/web/index.htmlpackages/cli/src/cmd/cloud/env/get.tspackages/cli/src/cmd/cloud/env/push.ts
🧰 Additional context used
📓 Path-based instructions (6)
apps/testing/integration-suite/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
Use path aliases for imports: @agents/* and @test/* instead of relative paths
Files:
apps/testing/integration-suite/src/test/helpers/index.tsapps/testing/integration-suite/src/test/api-route.tsapps/testing/integration-suite/src/test/cli-env-secrets.ts
apps/testing/integration-suite/src/test/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/testing/integration-suite/AGENTS.md)
apps/testing/integration-suite/src/test/**/*.{ts,tsx}: Use uniqueId() helper for all keys, namespaces, and identifiers in tests to ensure test isolation
Test files must follow the pattern: setup section with uniqueId(), execute section calling agent.run(), and assert section with assertion helpers
Import test registration function and assertion helpers from @test/suite and @test/helpers
Use decodeKVValue() helper from @test/helpers/kv to decode stringified Uint8Arrays when reading KV values
Tests must be registered by calling test() function from @test/suite, not just defined
Files:
apps/testing/integration-suite/src/test/helpers/index.tsapps/testing/integration-suite/src/test/api-route.tsapps/testing/integration-suite/src/test/cli-env-secrets.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use Prettier formatter with tabs (width 3), single quotes, and semicolons for TypeScript files
Use TypeScript strict mode with ESNext target and bundler moduleResolution
UseStructuredErrorfrom@agentuity/corefor error handling
Files:
apps/testing/integration-suite/src/test/helpers/index.tsapps/testing/integration-suite/src/test/api-route.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/get.ts
apps/testing/integration-suite/src/web/**/index.html
📄 CodeRabbit inference engine (apps/testing/integration-suite/src/web/AGENTS.md)
apps/testing/integration-suite/src/web/**/index.html: index.html must have a<div id="root"></div>element
Use a module script tag:<script type="module" src="/web/frontend.tsx"></script>in index.html
Files:
apps/testing/integration-suite/src/web/index.html
packages/cli/src/cmd/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/src/cmd/**/*.ts: Usetui.*helpers for formatted output instead of raw console logs
Usectx.loggerfor logging; calllogger.fatal()to log and exit with code 1
Files:
packages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/get.ts
packages/cli/src/**/*.ts
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Use
Bun.file(f).exists()instead ofexistsSync(f)for file existence checks
Files:
packages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/get.ts
🧠 Learnings (1)
📚 Learning: 2025-12-21T00:31:41.858Z
Learnt from: jhaynie
Repo: agentuity/sdk PR: 274
File: packages/cli/src/cmd/build/vite/server-bundler.ts:12-41
Timestamp: 2025-12-21T00:31:41.858Z
Learning: In Bun runtime, BuildMessage and ResolveMessage are global types and are not exported from the bun module. Do not import { BuildMessage } from 'bun' or similar; these types are available globally and should be used without import. This applies to all TypeScript files that target the Bun runtime within the repository.
Applied to files:
apps/testing/integration-suite/src/test/helpers/index.tsapps/testing/integration-suite/src/test/api-route.tsapps/testing/integration-suite/src/test/cli-env-secrets.tspackages/cli/src/cmd/cloud/env/push.tspackages/cli/src/cmd/cloud/env/get.ts
🧬 Code graph analysis (4)
apps/testing/integration-suite/src/test/api-route.ts (1)
apps/testing/integration-suite/src/test/suite.ts (2)
testSuite(213-213)test(218-220)
apps/testing/integration-suite/src/test/cli-env-secrets.ts (3)
apps/testing/integration-suite/src/test/suite.ts (1)
test(218-220)apps/testing/integration-suite/src/test/helpers/index.ts (3)
assertEqual(32-35)assert(23-27)uniqueId(12-18)packages/cli/src/tui.ts (1)
output(430-432)
packages/cli/src/cmd/cloud/env/push.ts (2)
packages/cli/src/env-util.ts (5)
findExistingEnvFile(50-52)readEnvFile(111-130)filterAgentuitySdkKeys(205-216)splitEnvAndSecrets(229-261)validateNoPublicSecrets(43-45)packages/server/src/api/project/env-update.ts (1)
projectEnvUpdate(33-55)
packages/cli/src/cmd/cloud/env/get.ts (3)
packages/cli/src/cmd/cloud/db/get.ts (1)
getSubcommand(20-164)packages/cli/src/cmd/cloud/storage/get.ts (1)
getSubcommand(17-107)packages/server/src/api/project/get.ts (1)
projectGet(30-48)
🪛 GitHub Actions: Build Packages & Test
packages/cli/src/cmd/cloud/env/get.ts
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
🪛 GitHub Actions: NPM Pack & Upload
packages/cli/src/cmd/cloud/env/get.ts
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
🪛 Shellcheck (0.11.0)
apps/testing/integration-suite/scripts/ci-test.sh
[warning] 167-167: EVENT appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Playwright E2E Smoke Test
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
🔇 Additional comments (7)
packages/cli/src/cmd/cloud/env/push.ts (2)
5-18: LGTM!The new imports from
env-utiland the extended response schema with granular counts align well with the PR objectives.
77-100: LGTM!The push operation correctly sends
envandsecretsseparately toprojectEnvUpdate, calculates counts accurately, and provides clear user feedback with proper pluralization.apps/testing/integration-suite/src/web/index.html (1)
1-22: LGTM!The file meets coding guidelines: has
<div id="root"></div>and uses a module script tag. The relative path./frontend.tsxis appropriate for the file's location within the web directory.apps/testing/integration-suite/src/test/helpers/index.ts (1)
12-18: LGTM!The change from hyphens to underscores ensures generated IDs are valid environment variable names (POSIX shell compatibility). The comment clearly documents the rationale.
apps/testing/integration-suite/src/test/api-route.ts (1)
45-54: LGTM!The
excludeparameter implementation is clean: parses comma-separated suite names, trims whitespace, and filters accordingly. The approach correctly preserves compatibility with existingsuiteandtestparameters.apps/testing/integration-suite/src/test/cli-env-secrets.ts (1)
1-14: LGTM!Comprehensive integration test suite covering validation rules, secret handling, masking, auto-detection, CRUD operations, and edge cases. Tests properly use path aliases (
@test/*,@agents/*) anduniqueId()for test isolation as per coding guidelines.apps/testing/integration-suite/scripts/ci-test.sh (1)
229-235: LGTM - well-designed two-phase execution.The approach of running most tests at high concurrency while isolating
cli-env-secretstests atconcurrency=1is appropriate given that those tests interact with cloud APIs that can't handle concurrent modifications to the same resources.
| // For secrets, mask by default even in non-TTY; for env vars, mask only in TTY by default | ||
| const maskDefault = isSecret ? true : !!process.stdout.isTTY; | ||
| const shouldMask = opts?.mask !== undefined ? opts.mask : maskDefault; |
There was a problem hiding this comment.
Fix property name mismatch causing TypeScript error.
The schema defines the option as maskSecret (line 29), but the handler accesses opts?.mask. This causes the pipeline failure TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
🐛 Proposed fix
// For secrets, mask by default even in non-TTY; for env vars, mask only in TTY by default
const maskDefault = isSecret ? true : !!process.stdout.isTTY;
- const shouldMask = opts?.mask !== undefined ? opts.mask : maskDefault;
+ const shouldMask = opts?.maskSecret !== undefined ? opts.maskSecret : maskDefault;📝 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.
| // For secrets, mask by default even in non-TTY; for env vars, mask only in TTY by default | |
| const maskDefault = isSecret ? true : !!process.stdout.isTTY; | |
| const shouldMask = opts?.mask !== undefined ? opts.mask : maskDefault; | |
| // For secrets, mask by default even in non-TTY; for env vars, mask only in TTY by default | |
| const maskDefault = isSecret ? true : !!process.stdout.isTTY; | |
| const shouldMask = opts?.maskSecret !== undefined ? opts.maskSecret : maskDefault; |
🧰 Tools
🪛 GitHub Actions: Build Packages & Test
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
🪛 GitHub Actions: NPM Pack & Upload
[error] 61-61: TypeScript error TS2339: Property 'mask' does not exist on type '{ maskSecret?: boolean | undefined; }'.
🤖 Prompt for AI Agents
In @packages/cli/src/cmd/cloud/env/get.ts around lines 59 - 61, The handler uses
opts?.mask but the CLI schema defines the option as maskSecret, causing a TS
property error; update the code around maskDefault/shouldMask to read
opts?.maskSecret (and treat it as boolean) instead of opts?.mask so shouldMask =
opts?.maskSecret !== undefined ? opts.maskSecret : maskDefault and keep the
existing logic using isSecret and process.stdout.isTTY unchanged.
Summary
Adds validation to prevent public variable prefixes from being added as secrets across the CLI commands.
Key Validation Rules
VITE_,AGENTUITY_PUBLIC_,PUBLIC_) cannot be marked as secrets - they are exposed to the frontendAGENTUITY_*keys (exceptAGENTUITY_PUBLIC_*) are blocked from being set by users - they are reserved for system useAGENTUITY_PUBLIC_*keys ARE allowed as env vars (not secrets)CLI Command Changes
secret setsecret pushsplitEnvAndSecrets, routes public vars to envsecret importsplitEnvAndSecrets, routes public vars to envsecret pullAGENTUITY_PUBLIC_*in .env fileenv setAGENTUITY_PUBLIC_*keys, blocks reservedenv pushfilterAgentuitySdkKeysenv importAGENTUITY_PUBLIC_*in .env fileenv pullAGENTUITY_PUBLIC_*in .env fileNew Utility Functions (env-util.ts)
isPublicVarKey(): Check if key has public var prefixisReservedAgentuityKey(): Check if key is reservedAGENTUITY_*(notPUBLIC_)validateNoPublicSecrets(): Validate secrets don't contain public varsgetPublicSecretKeys(): Get public var keys from secrets objectUpdated Functions
filterAgentuitySdkKeys(): Now allowsAGENTUITY_PUBLIC_*keyssplitEnvAndSecrets(): Routes public vars to env, not secretsTests
packages/cli/test/config/env-util.test.ts)apps/testing/integration-suite/src/test/cli-env-secrets.ts)Related
Companion PR in app repo: #969 (already merged)
Summary by CodeRabbit
New Features
Bug Fixes
Removed
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.