-
Notifications
You must be signed in to change notification settings - Fork 1
fix: preserve local AGENTUITY_SDK_KEY in cloud env pull #689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
When running 'cloud env pull', the command was deleting the local AGENTUITY_SDK_KEY from the .env file. If the cloud project didn't have an api_key set, the local key would be permanently lost. The issue was that writeEnvFile skips reserved keys (including AGENTUITY_SDK_KEY), which effectively deletes them when overwriting the file. The restore logic only ran if projectData.api_key existed, so local keys were lost when the cloud value was missing. Fix: - Preserve localSdkKey before the first write operation - Always restore the SDK key after writing merged env vars - Use local value if it exists, otherwise fall back to cloud value Fixes #[ISSUE_NUMBER]
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (2)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the 📝 WalkthroughWalkthroughModifies env pull to handle a cloud API key separately for org vs project scope, removes prior first-write behavior, preserves local AGENTUITY_SDK_KEY before merging, writes merged .env while skipping reserved keys, then restores AGENTUITY_SDK_KEY (prefer local; project may use cloud) and logs writes when newly created. Changes
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…d-env-pull-preserve-sdk-key # Conflicts: # packages/cli/src/cmd/cloud/env/pull.ts
📦 Canary Packages Publishedversion: PackagesInstallAdd to your {
"dependencies": {
"@agentuity/frontend": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-frontend-0.1.24-371a166.tgz",
"@agentuity/core": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-core-0.1.24-371a166.tgz",
"@agentuity/schema": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-schema-0.1.24-371a166.tgz",
"@agentuity/react": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-react-0.1.24-371a166.tgz",
"@agentuity/runtime": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-runtime-0.1.24-371a166.tgz",
"@agentuity/workbench": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-workbench-0.1.24-371a166.tgz",
"@agentuity/evals": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-evals-0.1.24-371a166.tgz",
"@agentuity/cli": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-cli-0.1.24-371a166.tgz",
"@agentuity/opencode": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-opencode-0.1.24-371a166.tgz",
"@agentuity/server": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-server-0.1.24-371a166.tgz",
"@agentuity/auth": "https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-auth-0.1.24-371a166.tgz"
}
}Or install directly: bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-frontend-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-core-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-schema-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-react-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-runtime-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-workbench-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-evals-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-cli-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-opencode-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-server-0.1.24-371a166.tgz
bun add https://agentuity-sdk-objects.t3.storage.dev/npm/0.1.24-371a166/agentuity-auth-0.1.24-371a166.tgzCLI Executables
Run Canary CLIagentuity canary 0.1.24-371a166 [command] [...args] |
Add comprehensive integration tests for the env pull command to verify: - Local AGENTUITY_SDK_KEY is preserved when pulling - Local key is preserved when cloud has no api_key (bug fix) - Cloud api_key is used when local doesn't exist - Works correctly with --org flag - --force flag preserves SDK key while overwriting other vars Tests follow the same pattern as cli-env-secrets.ts and include proper cleanup and .env file backup/restore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/testing/integration-suite/src/test/cli-env-pull.ts`:
- Around line 391-409: The test currently relies on alphabetical ordering by
naming it "zzz-cleanup-all-env-vars"; replace this fragile approach by moving
the cleanup logic into a proper teardown hook: use test.afterAll (or the
framework's equivalent) instead of the test named 'zzz-cleanup-all-env-vars',
and in that afterAll callback await isAuthenticated(), iterate over
createdEnvVars and call cliAgent.run(...) to delete each key (including the
org-scoped delete), then clear createdEnvVars.length = 0 so cleanup always runs
regardless of test ordering.
- Line 17: The import fails because PROJECT_DIR is not exported from the cli
helper; open the module that defines PROJECT_DIR (the file that exports
isAuthenticated) and add an export for the constant PROJECT_DIR (e.g., export
const PROJECT_DIR = ...), ensuring the symbol name PROJECT_DIR matches the
import in cli-env-pull.ts and keeping existing isAuthenticated export intact so
both can be imported from '@test/helpers/cli'.
🧹 Nitpick comments (1)
apps/testing/integration-suite/src/test/cli-env-pull.ts (1)
55-62: Consider quoting values with special characters.Values containing spaces, quotes, or special characters won't be properly preserved when written without quoting. Since this is for test fixtures with controlled values, it's likely fine, but worth noting.
♻️ Optional: Add quoting for values with special characters
function writeEnvFile(filePath: string, env: Record<string, string>): void { const lines: string[] = []; for (const [key, value] of Object.entries(env).sort()) { - lines.push(`${key}=${value}`); + const needsQuotes = /[\s"'#]/.test(value) || value.includes('='); + const quotedValue = needsQuotes ? `"${value.replace(/"/g, '\\"')}"` : value; + lines.push(`${key}=${quotedValue}`); } writeFileSync(filePath, lines.join('\n') + '\n'); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/testing/integration-suite/src/test/cli-env-pull.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-pull.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/suiteand@test/helpers
Use decodeKVValue() helper from@test/helpers/kvto 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-pull.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-pull.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-pull.ts
🪛 GitHub Actions: Build Packages & Test
apps/testing/integration-suite/src/test/cli-env-pull.ts
[error] 17-17: bun run --filter='./apps/testing/integration-suite' typecheck failed. TS2459: Module '"@test/helpers/cli"' declares 'PROJECT_DIR' locally, but it is not exported.
⏰ 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: Playwright E2E Smoke Test
- GitHub Check: Framework Integration Tests (TanStack & Next.js)
- GitHub Check: SDK Integration Test Suite
- GitHub Check: Queue CLI Tests
- GitHub Check: Sandbox CLI Tests
- GitHub Check: Package Installation & Usage Test
- GitHub Check: Queue SDK Tests
- GitHub Check: Template Integration Tests
- GitHub Check: Cloud Deployment Tests
🔇 Additional comments (9)
apps/testing/integration-suite/src/test/cli-env-pull.ts (9)
30-53: LGTM!The
.envparser correctly handles comments, empty lines, quoted values, and values containing=characters. Sufficient for test purposes.
64-82: LGTM!The backup/restore pattern with empty string sentinel for non-existent files is clean and correctly handles all cases.
84-135: LGTM!Test follows the correct pattern with setup, execute, and assert sections. Proper cleanup with try/finally and cloud env var deletion.
137-187: LGTM!This test directly validates the fix for the reported bug. The comment on line 146 documents the environmental assumption about the project's api_key state.
189-236: LGTM!The test verifies the command doesn't crash and env vars are pulled correctly. The comment appropriately documents that SDK key verification depends on the project's api_key configuration.
238-288: LGTM!Good coverage for the org-scoped pull scenario. Since orgs don't have api_key, this effectively tests the fix for that code path.
290-342: LGTM!Important edge case coverage verifying that
--forceoverwrites env vars but still preserves the local SDK key.
344-389: LGTM!Good edge case coverage for
.envfile creation when the file doesn't exist.
21-28: LGTM!The module-level tracking works for sequential test execution. Each test also has its own cleanup in
finally, so this is a defensive backup mechanism.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Final cleanup test - runs last to clean up any leftover env vars | ||
| test('cli-env-pull', 'zzz-cleanup-all-env-vars', async () => { | ||
| const authenticated = await isAuthenticated(); | ||
| if (!authenticated) return; | ||
|
|
||
| // Delete all tracked env vars | ||
| for (const key of createdEnvVars) { | ||
| await cliAgent.run({ | ||
| command: `cloud env delete ${key}`, | ||
| }); | ||
| // Also try org scope in case some were created there | ||
| await cliAgent.run({ | ||
| command: `cloud env delete ${key} --org`, | ||
| }); | ||
| } | ||
|
|
||
| // Clear the tracking array | ||
| createdEnvVars.length = 0; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relying on alphabetical test ordering may be fragile.
The zzz- prefix assumes the test runner executes tests alphabetically, which isn't guaranteed by all test frameworks. However, since each test already performs its own cleanup in the finally block, this serves as a safety net and the risk is low.
🤖 Prompt for AI Agents
In `@apps/testing/integration-suite/src/test/cli-env-pull.ts` around lines 391 -
409, The test currently relies on alphabetical ordering by naming it
"zzz-cleanup-all-env-vars"; replace this fragile approach by moving the cleanup
logic into a proper teardown hook: use test.afterAll (or the framework's
equivalent) instead of the test named 'zzz-cleanup-all-env-vars', and in that
afterAll callback await isAuthenticated(), iterate over createdEnvVars and call
cliAgent.run(...) to delete each key (including the org-scoped delete), then
clear createdEnvVars.length = 0 so cleanup always runs regardless of test
ordering.
PROJECT_DIR was used internally but not exported, causing type errors when importing it in cli-env-pull.ts tests.
|
what is the case where you want the local sdk key (or where the cloud project doesn't have the sdk key stored) instead of the one from the cloud which will always be correct? |
The issue is what I described in Slack. I have an existing project, I have my |
|
|
||
| // Use local SDK key if it exists, otherwise use cloud value (project scope only) | ||
| // For org scope, only restore if local key exists (orgs don't have api_key) | ||
| const sdkKeyToWrite = localSdkKey || cloudApiKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we need to always prefer the cloudApiKey and error if for some reason that comes back empty/undefined
Update env pull to prioritize cloud api_key over local AGENTUITY_SDK_KEY. Cloud is now the source of truth - if cloud has api_key, it overwrites local. Local key is only used as fallback when cloud has no api_key. This fixes the bug where local AGENTUITY_SDK_KEY was deleted when cloud had no api_key, while ensuring cloud values take precedence when available. Updated tests to reflect this behavior.
Problem
When running
cloud env pull, the command was deleting the localAGENTUITY_SDK_KEYfrom the.envfile. If the cloud project didn't have anapi_keyset, the local key would be permanently lost and never restored.Root Cause
writeEnvFileis called withskipKeysthat includesAGENTUITY_SDK_KEY(since it's a reserved key)writeEnvFilecompletely overwrites the file, so skipping the key effectively deletes itprojectData.api_keyexisted:if (projectData.api_key) { ... }projectData.api_keywas undefined/missing, the entire restore block was skippedSolution
localSdkKeybefore the first write operationChanges
packages/cli/src/cmd/cloud/env/pull.ts:Testing
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.