refactor: standardize legacy token naming aliases#113
refactor: standardize legacy token naming aliases#113chitcommit wants to merge 1 commit intomainfrom
Conversation
Consolidate scattered token/env aliases to canonical names across 19 files: - CHITTY_ID_SERVICE_TOKEN / CHITTY_SERVICE_TOKEN → CHITTY_ID_TOKEN - CHITTY_API_KEY → removed (dead code) - CLOUDFLARE_ACCOUNT_ID / CF_ACCOUNT_ID → CHITTYOS_ACCOUNT_ID (internal only) External-facing contract names (GitHub Actions secrets, CLI scripts) preserved. Tests, mocks, migrations, deploy scripts, secret catalog, and wrangler manifest all updated to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyconnect | d6031c9 | Mar 26 2026, 04:20 AM |
📝 WalkthroughWalkthroughThis pull request systematically renames authentication secrets and environment variables across the codebase: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors environment/secret naming to standardize on canonical token/ID variables (notably CHITTY_ID_TOKEN and CHITTYOS_ACCOUNT_ID), removing legacy aliases and tightening header injection behavior.
Changes:
- Consolidates ChittyID auth token usage to
CHITTY_ID_TOKENacross runtime code and tests/mocks, and updates connection seed data accordingly. - Removes Cloudflare account ID fallbacks (
CLOUDFLARE_ACCOUNT_ID/CF_ACCOUNT_ID) in runtime paths in favor ofCHITTYOS_ACCOUNT_ID. - Updates secret/deploy documentation & tooling and adds a safety guard to only send Ollama CF Access headers when both values are present.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wrangler.jsonc | Documents canonical secrets/vars and marks legacy aliases as retired. |
| tests/lib/cloudflare-api-helper.test.js | Updates Cloudflare credential tests for CHITTYOS_ACCOUNT_ID. |
| tests/intelligence/experience-anchor.test.js | Updates mock env to use CHITTY_ID_TOKEN. |
| tests/helpers/mocks.js | Updates shared test env mock to use CHITTY_ID_TOKEN. |
| src/services/credential-provisioner.js | Removes CLOUDFLARE_ACCOUNT_ID fallback; uses CHITTYOS_ACCOUNT_ID only. |
| src/mcp/tool-dispatcher.js | Removes CLOUDFLARE_ACCOUNT_ID fallback for Evidence AI tools. |
| src/lib/cloudflare-api-helper.js | Uses CHITTYOS_ACCOUNT_ID as the env fallback key for account ID retrieval. |
| src/lib/chittyid-client.js | Removes legacy token/api-key env aliases; uses CHITTY_ID_TOKEN. |
| src/intelligence/experience-anchor.js | Switches Authorization header to CHITTY_ID_TOKEN. |
| src/intelligence/context-intelligence.js | Switches mint auth header to CHITTY_ID_TOKEN. |
| src/intelligence/tests/experience-anchor.test.js | Updates test env token name to CHITTY_ID_TOKEN. |
| src/intelligence/tests/context-intelligence.test.js | Updates test env token name to CHITTY_ID_TOKEN. |
| src/api/routes/thirdparty.js | Only injects CF Access headers when both client id + secret are present. |
| src/api/routes/credentials.js | Health output now checks only CHITTYOS_ACCOUNT_ID. |
| scripts/deploy-missing-secrets.sh | Drops legacy ChittyID token secret; adds a few additional secrets; fixes arithmetic with set -e. |
| migrations/013_connections.sql | Updates ChittyID connection seed token pattern to CHITTY_ID_TOKEN. |
| CLAUDE.md | Updates setup docs to use wrangler secret put CHITTY_ID_TOKEN. |
| .github/workflows/sync-secrets.yml | Renames env var from CF_ACCOUNT_ID to CHITTYOS_ACCOUNT_ID. |
| .github/secret-catalog.json | Renames secret entry from CHITTY_ID_SERVICE_TOKEN to CHITTY_ID_TOKEN. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const env = { CHITTYOS_ACCOUNT_ID: "acct-456" }; | ||
|
|
||
| const { accountId } = await getCloudflareApiCredentials(env); | ||
| expect(accountId).toBe("acct-456"); |
There was a problem hiding this comment.
This test name and setup don’t match the current implementation: getCloudflareApiCredentials returns env.CHITTYOS_ACCOUNT_ID directly when present, so it won’t call getCredential for the account ID. As written, the second .mockResolvedValueOnce("acct-456") is unused and the test isn’t exercising the broker/env-fallback path. Consider either (a) renaming the test to reflect “accountId from env”, or (b) setting env without CHITTYOS_ACCOUNT_ID and asserting the accountId comes from the second getCredential call (and optionally asserting call count/args).
| const env = { CHITTYOS_ACCOUNT_ID: "acct-456" }; | |
| const { accountId } = await getCloudflareApiCredentials(env); | |
| expect(accountId).toBe("acct-456"); | |
| const env = {}; | |
| const { accountId } = await getCloudflareApiCredentials(env); | |
| expect(accountId).toBe("acct-456"); | |
| expect(getCredential).toHaveBeenCalledTimes(2); |
| // │ RETIRED ALIASES (removed from code — DO NOT re-add) │ | ||
| // ├─────────────────────────────────────────────────────────────────┤ | ||
| // │ CF_ACCOUNT_ID → CHITTYOS_ACCOUNT_ID (var) │ | ||
| // │ CHITTY_SERVICE_TOKEN → CHITTY_ID_SERVICE_TOKEN (secret) │ | ||
| // │ CHITTY_API_KEY → API_KEYS KV lookup (binding) │ | ||
| // │ CF_ACCOUNT_ID → removed, use CHITTYOS_ACCOUNT_ID │ | ||
| // │ CLOUDFLARE_ACCOUNT_ID → removed, use CHITTYOS_ACCOUNT_ID │ | ||
| // │ CHITTY_SERVICE_TOKEN → removed, use CHITTY_ID_TOKEN │ | ||
| // │ CHITTY_ID_SERVICE_TOKEN → removed, use CHITTY_ID_TOKEN │ | ||
| // │ CHITTY_API_KEY → removed (dead code) │ |
There was a problem hiding this comment.
The header says these aliases are “removed from code — DO NOT re-add”, but there are still active references to CLOUDFLARE_ACCOUNT_ID in the repo (e.g. GitHub workflows and scripts). Either update the remaining references as part of this refactor, or clarify the comment scope (e.g. “removed from Worker runtime code” / “still used as external GitHub secret contract”).
| # Credential Provisioning & Emergency | ||
| # Note: CHITTYOS_ACCOUNT_ID is a wrangler var, not a secret — no deployment needed | ||
| "ENCRYPTION_KEY|$VAULT_EMERGENCY|encryption|key" | ||
| "INTERNAL_WEBHOOK_SECRET|$VAULT_EMERGENCY|internal-webhook|secret" | ||
| "OP_EVENTS_API_TOKEN|$VAULT_EMERGENCY|1password-events|api_token" | ||
| "EMERGENCY_REVOKE_TOKEN|$VAULT_EMERGENCY|emergency-revoke|token" | ||
|
|
||
| # Google Integration | ||
| "GOOGLE_ACCESS_TOKEN|$VAULT_INTEGRATIONS|google-drive|access_token" |
There was a problem hiding this comment.
deploy-missing-secrets.sh says this list is “in the manifest AND used in code”, but EMERGENCY_REVOKE_TOKEN doesn’t appear to be referenced anywhere in src/ (searching the repo only finds it in docs/manifest comments). If this token is intentionally pre-provisioned for future use, consider moving it into a separate “not yet used” section or updating the comment so the script output isn’t interpreted as an actionable runtime requirement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/013_connections.sql`:
- Line 52: The migration currently uses an INSERT OR IGNORE for the
conn-chittyid row which will not change existing records; add an UPDATE after
the INSERT to set the new service_token_pattern ('CHITTY_ID_TOKEN') and any
other changed columns for rows where id = 'conn-chittyid' so existing
deployments are migrated; target the same table and the same unique id
'conn-chittyid' (the row inserted in migrations/013_connections.sql) and ensure
the UPDATE runs unconditionally after the INSERT OR IGNORE to overwrite the old
CHITTY_ID_SERVICE_TOKEN pattern.
In `@tests/lib/cloudflare-api-helper.test.js`:
- Around line 73-78: The test currently sets env.CHITTYOS_ACCOUNT_ID which
prevents exercising the getCredential fallback; update the test so
CHITTYOS_ACCOUNT_ID is not present (e.g., set env = {} or delete
CHITTYOS_ACCOUNT_ID) and keep the getCredential mocks
(.mockResolvedValueOnce("op-api-token") then .mockResolvedValueOnce("acct-456"))
so the code path in the function that reads CHITTYOS_ACCOUNT_ID falls back to
getCredential is actually executed and validated.
In `@wrangler.jsonc`:
- Line 313: Remove the duplicate secret entry for CHITTY_ID_TOKEN: locate the
second occurrence of "CHITTY_ID_TOKEN" (the entry with description "ChittyID
service auth token") and delete that entire line so only the original entry
(description "ChittyID service auth") remains in the secrets manifest; ensure no
other duplicate CHITTY_ID_TOKEN entries remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2d2d920-681e-4674-9dd9-d70a8173f7bb
📒 Files selected for processing (19)
.github/secret-catalog.json.github/workflows/sync-secrets.ymlCLAUDE.mdmigrations/013_connections.sqlscripts/deploy-missing-secrets.shsrc/api/routes/credentials.jssrc/api/routes/thirdparty.jssrc/intelligence/__tests__/context-intelligence.test.jssrc/intelligence/__tests__/experience-anchor.test.jssrc/intelligence/context-intelligence.jssrc/intelligence/experience-anchor.jssrc/lib/chittyid-client.jssrc/lib/cloudflare-api-helper.jssrc/mcp/tool-dispatcher.jssrc/services/credential-provisioner.jstests/helpers/mocks.jstests/intelligence/experience-anchor.test.jstests/lib/cloudflare-api-helper.test.jswrangler.jsonc
| -- Seed: ChittyOS Services (Tier 0-5) | ||
| INSERT OR IGNORE INTO connections (id, name, slug, category, provider, base_url, health_endpoint, tier, credential_source, service_token_pattern, status, description, icon, depends_on) VALUES | ||
| ('conn-chittyid', 'ChittyID', 'chittyid', 'chittyos_service', 'chittyos', 'https://id.chitty.cc', '/health', 0, 'env', 'CHITTY_ID_SERVICE_TOKEN', 'active', 'Identity minting and validation', 'ID', '[]'), | ||
| ('conn-chittyid', 'ChittyID', 'chittyid', 'chittyos_service', 'chittyos', 'https://id.chitty.cc', '/health', 0, 'env', 'CHITTY_ID_TOKEN', 'active', 'Identity minting and validation', 'ID', '[]'), |
There was a problem hiding this comment.
Migration will not update existing database rows.
The INSERT OR IGNORE statement only inserts new rows—it won't update the service_token_pattern for the conn-chittyid row if it already exists in production. Since src/api/routes/connections.js dynamically looks up credentials via c.env[conn.service_token_pattern] (line 752), existing deployments will continue using the old CHITTY_ID_SERVICE_TOKEN pattern and fail silently when that env var no longer exists.
Add an UPDATE statement to fix existing rows:
Proposed fix: Add UPDATE for existing rows
INSERT OR IGNORE INTO connections (id, name, slug, category, provider, base_url, health_endpoint, tier, credential_source, service_token_pattern, status, description, icon, depends_on) VALUES
('conn-chittyid', 'ChittyID', 'chittyid', 'chittyos_service', 'chittyos', 'https://id.chitty.cc', '/health', 0, 'env', 'CHITTY_ID_TOKEN', 'active', 'Identity minting and validation', 'ID', '[]'),
...
+-- Update existing row if it has the old token pattern
+UPDATE connections SET service_token_pattern = 'CHITTY_ID_TOKEN' WHERE id = 'conn-chittyid' AND service_token_pattern = 'CHITTY_ID_SERVICE_TOKEN';Based on learnings: "All database changes must be coordinated with other ChittyOS services."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@migrations/013_connections.sql` at line 52, The migration currently uses an
INSERT OR IGNORE for the conn-chittyid row which will not change existing
records; add an UPDATE after the INSERT to set the new service_token_pattern
('CHITTY_ID_TOKEN') and any other changed columns for rows where id =
'conn-chittyid' so existing deployments are migrated; target the same table and
the same unique id 'conn-chittyid' (the row inserted in
migrations/013_connections.sql) and ensure the UPDATE runs unconditionally after
the INSERT OR IGNORE to overwrite the old CHITTY_ID_SERVICE_TOKEN pattern.
| it("falls back to CHITTYOS_ACCOUNT_ID via getCredential", async () => { | ||
| getCredential | ||
| .mockResolvedValueOnce("op-api-token") // apiToken | ||
| .mockResolvedValueOnce("acct-456"); // accountId via env fallback | ||
| const env = { CLOUDFLARE_ACCOUNT_ID: "acct-456" }; | ||
| .mockResolvedValueOnce("acct-456"); // accountId via getCredential | ||
| const env = { CHITTYOS_ACCOUNT_ID: "acct-456" }; | ||
|
|
There was a problem hiding this comment.
Fix the fallback test so it actually exercises credential fallback.
The test title says fallback is via getCredential, but env.CHITTYOS_ACCOUNT_ID is pre-populated, so it validates env resolution instead. This can hide fallback regressions.
✅ Suggested test adjustment
it("falls back to CHITTYOS_ACCOUNT_ID via getCredential", async () => {
getCredential
.mockResolvedValueOnce("op-api-token") // apiToken
.mockResolvedValueOnce("acct-456"); // accountId via getCredential
- const env = { CHITTYOS_ACCOUNT_ID: "acct-456" };
+ const env = {};
const { accountId } = await getCloudflareApiCredentials(env);
expect(accountId).toBe("acct-456");
+ expect(getCredential).toHaveBeenCalledTimes(2);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/lib/cloudflare-api-helper.test.js` around lines 73 - 78, The test
currently sets env.CHITTYOS_ACCOUNT_ID which prevents exercising the
getCredential fallback; update the test so CHITTYOS_ACCOUNT_ID is not present
(e.g., set env = {} or delete CHITTYOS_ACCOUNT_ID) and keep the getCredential
mocks (.mockResolvedValueOnce("op-api-token") then
.mockResolvedValueOnce("acct-456")) so the code path in the function that reads
CHITTYOS_ACCOUNT_ID falls back to getCredential is actually executed and
validated.
| // │ CHITTY_DISPUTES_TOKEN ChittyDisputes service auth │ | ||
| // │ CHITTY_LEDGER_TOKEN ChittyLedger service auth │ | ||
| // │ CHITTY_ID_SERVICE_TOKEN ChittyID generic service token │ | ||
| // │ CHITTY_ID_TOKEN ChittyID service auth token │ |
There was a problem hiding this comment.
Duplicate entry in secrets manifest.
CHITTY_ID_TOKEN is already listed at line 294 with description "ChittyID service auth". Line 313 adds a second entry "ChittyID service auth token". Remove the duplicate to keep the manifest clean.
Proposed fix
Remove line 313 entirely since line 294 already documents this secret:
// │ CHITTY_LEDGER_TOKEN ChittyLedger service auth │
-// │ CHITTY_ID_TOKEN ChittyID service auth token │
// │ CHITTYCONNECT_SERVICE_TOKEN ChittyConnect self-service token │📝 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.
| // │ CHITTY_ID_TOKEN ChittyID service auth token │ | |
| // │ CHITTY_LEDGER_TOKEN ChittyLedger service auth │ | |
| // │ CHITTYCONNECT_SERVICE_TOKEN ChittyConnect self-service token │ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wrangler.jsonc` at line 313, Remove the duplicate secret entry for
CHITTY_ID_TOKEN: locate the second occurrence of "CHITTY_ID_TOKEN" (the entry
with description "ChittyID service auth token") and delete that entire line so
only the original entry (description "ChittyID service auth") remains in the
secrets manifest; ensure no other duplicate CHITTY_ID_TOKEN entries remain.
chitcommit
added
security-approved
Summary
CHITTY_ID_SERVICE_TOKEN/CHITTY_SERVICE_TOKEN→CHITTY_ID_TOKENacross all source, tests, mocks, migrations, deploy scripts, secret catalog, and CI configCHITTY_API_KEYreferencesCLOUDFLARE_ACCOUNT_ID/CF_ACCOUNT_IDfallbacks with directCHITTYOS_ACCOUNT_IDusage19 files changed, 45 insertions, 48 deletions
Test plan
npm testpasses (test mocks updated to use canonical names)cloudflare-api-helper.test.jsno longer tests removed fallbackCHITTY_ID_SERVICE_TOKENorCLOUDFLARE_ACCOUNT_IDas secretssecrets.CLOUDFLARE_ACCOUNT_ID(external contract unchanged)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores