Repository navigation
Rework package token editor like secrets - #563
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughDocs, handler and repository logic, shared account-management components, and the account package invocation token and secrets routes were updated. Token updates can now optionally replace the stored token hash when a new raw token is provided. ChangesAccount management token rotation and shells
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
🔎 Preview deployed: https://kody-pr-563.kentcdodds.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/worker/src/app/handlers/account-package-invocation-tokens.ts (1)
345-358: 📐 Maintainability & Code Quality | 🔵 TrivialUse
getUniqueConstraintField()here instead of matching error text. This file uses the samemessage.includes('UNIQUE') || message.includes('token_hash')check in both the create and update paths, whileauth.tsandaccount-profile.tsalready rely on the shared parser.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-package-invocation-tokens.ts` around lines 345 - 358, Replace the ad hoc error-text matching in the token create/update catch blocks with the shared unique-constraint parser used elsewhere. In the relevant handler logic in account-package-invocation-tokens.ts, call getUniqueConstraintField() on the caught error and use that result to detect token_hash uniqueness conflicts, matching the approach already used in auth.ts and account-profile.ts, then keep returning the same 409 response for that specific case and rethrow everything else.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/client/routes/account-management-components.tsx`:
- Around line 99-104: AccountManagementMessage currently hardcodes role="alert"
for every message, which is too aggressive for non-error states. Update the
component to choose the live-region role based on handle.props.tone, keeping
alert for error tones and using a polite status-style role for success/info
messages. Adjust the render logic in AccountManagementMessage so routine
messages like saved confirmations do not interrupt screen-reader output.
In `@packages/worker/client/routes/account-secrets.tsx`:
- Around line 1085-1093: The New secret and secret-selection handlers in
account-secrets.tsx stay clickable while a save is in progress, which lets
editorState change before saveSecretChanges() finishes. Gate both the New secret
button and the secret row selection logic on isMutating so navigation/selection
is disabled during an in-flight mutation, and update saveSecretChanges() to
capture the submitted editor state before the first await so later success
messaging and follow-up navigation use the original state.
---
Nitpick comments:
In `@packages/worker/src/app/handlers/account-package-invocation-tokens.ts`:
- Around line 345-358: Replace the ad hoc error-text matching in the token
create/update catch blocks with the shared unique-constraint parser used
elsewhere. In the relevant handler logic in
account-package-invocation-tokens.ts, call getUniqueConstraintField() on the
caught error and use that result to detect token_hash uniqueness conflicts,
matching the approach already used in auth.ts and account-profile.ts, then keep
returning the same 409 response for that specific case and rethrow everything
else.
🪄 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 Plus
Run ID: 95a8a80e-d8e8-4f3c-a7bf-b85d6fb6ee9e
📒 Files selected for processing (8)
docs/contributing/package-invocation-api.mddocs/guides/account-package-invocation-token-setup.mdpackages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/account-package-invocation-tokens.tsxpackages/worker/client/routes/account-secrets.tsxpackages/worker/src/app/handlers/account-package-invocation-tokens.node.test.tspackages/worker/src/app/handlers/account-package-invocation-tokens.tspackages/worker/src/package-invocations/repo.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/client/routes/account-secrets.tsx (1)
1080-1088: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the Remote connectors link during mutations.
New secretand list selection now respectisMutating, but this header link can still navigate away while a save/delete/approval request is in flight, potentially aborting the write or hiding its result. Prevent the click whenisMutatingis true and mark the link disabled.Suggested patch
<a href="/account/remote-connectors" + aria-disabled={isMutating} mix={css({ ...secondaryButtonCss, textDecoration: 'none', + ...(isMutating + ? { pointerEvents: 'none', opacity: 0.6 } + : null), })} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/account-secrets.tsx` around lines 1080 - 1088, Guard the Remote connectors header link in the account-secrets route so it cannot be used while a mutation is in flight. Update the existing anchor element near the Remote connectors label to respect isMutating the same way the New secret action and list selection already do: prevent navigation when isMutating is true, and apply a disabled state/style so the user can see it is unavailable during save/delete/approval requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/worker/client/routes/account-secrets.tsx`:
- Around line 1080-1088: Guard the Remote connectors header link in the
account-secrets route so it cannot be used while a mutation is in flight. Update
the existing anchor element near the Remote connectors label to respect
isMutating the same way the New secret action and list selection already do:
prevent navigation when isMutating is true, and apply a disabled state/style so
the user can see it is unavailable during save/delete/approval requests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e6c4bb0c-58aa-4377-bf29-ad8375ac0d43
📒 Files selected for processing (2)
packages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/account-secrets.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/client/routes/account-management-components.tsx
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 768ed26. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

Summary
Walkthrough
Testing
npx vitest run packages/worker/src/app/handlers/account-package-invocation-tokens.node.test.ts packages/worker/src/mcp/capabilities/coding/kody-official-guide.node.test.tsnpx vitest run packages/worker/src/app/handlers/account-package-invocation-tokens.node.test.tsnpm run typechecknpm run format:checknpm run lint(existing unrelated warnings only)npm run validateSummary by CodeRabbit