feat(kiloclaw): derive config endpoint secret status from catalog#1033
feat(kiloclaw): derive config endpoint secret status from catalog#1033
Conversation
| slackBot: !!config.channels?.slackBotToken, | ||
| slackApp: !!config.channels?.slackAppToken, | ||
| }, | ||
| configuredSecrets: buildConfiguredSecrets(config), |
There was a problem hiding this comment.
WARNING: Keep the legacy channels field for a rollout window
cloud and the KiloClaw worker deploy independently (KILOCLAW_API_URL points the web app at an external worker). Removing channels here means an older web build will see config.channels === undefined and temporarily render every channel as not configured until the frontend rollout completes. Returning both configuredSecrets and the legacy channels booleans for one release avoids that cross-service break.
There was a problem hiding this comment.
Acknowledged, this is documented in the Reviewer Notes. Both the worker and Next.js app change in this PR and deploy within minutes of each other. If briefly out of sync, the only effect is channels temporarily showing "Not configured" in the Settings tab. No errors, no data loss, tokens remain functional on the Fly machine.
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)None. Files Reviewed (7 files)
Reviewed by gpt-5.4-20260305 · 467,485 tokens |
96af637 to
4449a15
Compare
| slackBot: false, | ||
| slackApp: false, | ||
| }; | ||
| const configuredSecrets = config?.configuredSecrets ?? {}; |
There was a problem hiding this comment.
WARNING: Frontend-first rollout marks every channel as not configured
This component now reads only configuredSecrets. If the Next.js deploy lands before the worker change, older /api/kiloclaw/config responses only contain channels, so all entries fall back to false until the worker is rolled out. Keep the old config.channels mapping as a temporary fallback until both deployments are live.
There was a problem hiding this comment.
Prev acknowledged, this is documented in the Reviewer Notes. Both the worker and Next.js app change in this PR and deploy within minutes of each other. If briefly out of sync, the only effect is channels temporarily showing "Not configured" in the Settings tab. No errors, no data loss, tokens remain functional on the Fly machine.
Summary
Replaces the hardcoded per-field channel booleans in
GET /api/kiloclaw/configwith a catalog-drivenconfiguredSecrets: Record<string, boolean>map keyed by catalog entry ID. This is the P0 future item from the secret-catalog-registry plan.Before: The config endpoint returned
channels: { telegram: bool, discord: bool, slackBot: bool, slackApp: bool }and the UI needed a manualisEntryConfigured()switch to map these per-field booleans back to per-entry status. Adding a new secret type required updating both the endpoint and the switch.After: The endpoint iterates
SECRET_CATALOGentries and checks if all fields have values, returningconfiguredSecrets: { telegram: bool, discord: bool, slack: bool }. The UI readsconfiguredSecrets[entry.id]directly — no manual mapping needed. New catalog entries get status automatically.Verification
getFieldKeysByCategory)buildConfiguredSecretscovering:encryptedSecrets(env var key lookup)channelsfallback whenencryptedSecretsis nullencryptedSecretspreferred over legacychannelsVisual Changes
None — the UI renders identically.
SecretEntrySectionstill receives aconfigured: booleanprop; only the source of that boolean changed (from a hardcoded switch to a direct map lookup). The "Configured" / "Not configured" labels, placeholder text, and Remove button visibility are all unchanged.Reviewer Notes
UserConfigResponse.channelsis replaced byconfiguredSecrets. This is consumed only bySettingsTab.tsx(verified via grep), which is updated in this PR. Both the worker and Next.js app change in this PR and deploy within minutes of each other. If they're briefly out of sync, the only effect is channels temporarily showing "Not configured" in the Settings tab — no errors, no data loss, tokens remain functional on the Fly machine.encryptedSecretsstorage uses env var names as keys (e.g.TELEGRAM_BOT_TOKEN), not field keys. This was verified againstupdateSecrets()line 446 which remaps viaFIELD_KEY_TO_ENV_VARbefore persisting, and confirmed by existing DO test fixtures. Thefield.envVarlookup inbuildConfiguredSecretsis correct.channelsfield (keyed by field key, e.g.telegramBotToken).buildConfiguredSecretschecks both paths so these instances still report correct status.buildConfiguredSecretsis exported solely for unit testability.getFieldKeysByCategoryallocates per call — documented in JSDoc. Currently only called once at module level, so no perf concern.