feat: add user secrets management page#25371
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Docs preview📖 View docs preview for |
afe1534 to
d6ed353
Compare
6ca38b7 to
8f346c5
Compare
2ed690a to
3515ca7
Compare
8f346c5 to
57e2ed5
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
Well-structured secrets management page with thorough Storybook coverage (15 stories, 14 with play functions). The component reuse is good: Dialog, Table, Badge, SettingsHeader, ErrorAlert, etc. The return-focus management for dialogs is genuine accessibility work. The FeatureStageBadge refactor is safe for existing consumers (verified all beta xs paths are unchanged).
1 P2, 8 P3, 7 Nit.
The P2 is a security misrepresentation: the dialog says "Secrets are encrypted" but this is only true with the enterprise dbcrypt layer. AGPL deployments store values in plaintext. A user reading this claim might skip using an external vault.
The most convergent P3 (7/19 reviewers flagged independently): the three new Promise wrappers around mutate() in SecretsPage.tsx. Every sibling page in UserSettingsPage/ uses mutateAsync with try/catch. The manual wrappers produce correct behavior but add ~45 lines of indirection that mutateAsync eliminates. This refactor also subsumes the unnecessary useCallback wrappers and the onMutationError helper.
A related P3: the create/update error path fires both a toast AND inline form errors for the same failure. The mutateAsync refactor is the right time to decide which channel owns error display.
"If the user presses Escape while the form is submitting, the dialog closes and the Radix portal unmounts the form. The mutation continues in the background. On error, the toast fires but the form-level error display is lost because the component is unmounted." (Mafuuu)
Process note: commit 939dc440 subject "address user secrets review feedback" tells a reader nothing about what changed. Good subjects elsewhere in the branch ("preserve secret API field errors", "use backend secret validation").
🤖 This review was automatically generated with Coder Agents.
Use mutateAsync for secret mutations, keep create and update errors inline, and keep delete failures toasted. Prevent closing the secret dialog while submitting, tighten warning copy, and expand Storybook coverage for dialog edge cases. Align feature badge and secret type labels without changing layout.
Use mutateAsync for secret mutations, keep create and update errors inline, and keep delete failures toasted. Prevent closing the secret dialog while submitting, tighten warning copy, and expand Storybook coverage for dialog edge cases. Align feature badge and secret type labels without changing layout.
| autoComplete="off" | ||
| className="placeholder:text-content-disabled" | ||
| data-lpignore="true" | ||
| data-1p-ignore="true" |
There was a problem hiding this comment.
what's the thinking behind telling password mangers to ignore this field? I use a password manager that can fill in things like this when configured.
There was a problem hiding this comment.
This field is file path which seems less likely to be filled in by password managers, value (which is below) won't be ignored by password managers. Tracy and I were both having the experience of password managers being overly eager to fill in all the fields in this pop-out, which was a little annoying, so I added password manager ignore to a few fields to help
zedkipp
left a comment
There was a problem hiding this comment.
I don't think I can give a meaningful deep review of the frontend code, so you'll want some frontend folks eyes on this. I did some manual testing and it looks good functionality wise.
Documentation CheckUpdates Needed
Automated review via Coder Agents |
Mention dashboard management in user secrets docs and wait for the Clear button to become visible in the Storybook interaction to avoid dialog animation races.
jakehwll
left a comment
There was a problem hiding this comment.
Big PR so I'm going to do another pass later 🙂
Nothing too scary, but this is a few pointers to get this closer
| }; | ||
|
|
||
| const infoText = "Secret values cannot be retrieved once saved."; | ||
| const savedSecretValueDisplay = "••••••••••••••••••••"; |
There was a problem hiding this comment.
Whynot just render the value as a type="password" ?
There was a problem hiding this comment.
Yeah I did this so since type="password" was exacerbating the over eager password manager suggestions that we were seeing
| useEffect(() => { | ||
| if (open) { | ||
| setClearValueRequested(false); | ||
| } | ||
| }, [open]); |
There was a problem hiding this comment.
I don't love this use of useEffect to keep another value in sync..
There was a problem hiding this comment.
Good call, I removed the mirrored state/effect and made isShowingSavedValue derived from the current props plus a small piece of interaction state. The field remounts when the edit dialog opens, so the hidden-saved-value state resets without needing an effect to keep it in sync.

Adds the account settings UI for managing user secrets, including the table, add/edit/delete dialog, Storybook coverage, and route/sidebar entry.
Also updates the shared
FeatureStageBadgeearly-access variant with dedicated Early Access styling, sizing, and label casing for the Secrets page.Stacked on #25370.
This PR was generated by Coder Agents.