Add already-added notice for secret approval links by kentcdodds · Pull Request #109 · kentcdodds/kody

kentcdodds · 2026-03-30T23:05:43Z

Summary

show an "Already added" notice on secret detail pages when an allowed-host or capability from the landing URL is already present on the secret
hide the host approval card when the requested host is already in the secret's allowed hosts
fix the approval-card render branch so TypeScript can narrow the nullable approval state in CI
cover the stale approval-link case with a focused Playwright spec

Testing

npm run typecheck
npm run test:e2e:install
npm run test:e2e -- e2e/account-secrets.spec.ts
manual browser verification of the stale approval-link landing flow

Summary by CodeRabbit

New Features
- Approval flow now detects when a requested host or capability is already present and shows an "Already added" status instead of prompting for approval.
- Approval UI suppresses the approval prompt when the host is already present or on location refresh.
Tests
- Added an end-to-end test verifying the "Already added" status and that duplicate approval is not shown.
Chores
- Secret-saving helper now accepts optional host and capability lists when provided.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

coderabbitai · 2026-03-30T23:05:50Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fa9c62c1-d288-410a-8a23-318c3e539f5a

📥 Commits

Reviewing files that changed from the base of the PR and between 3e91c2f and e4db22c.

📒 Files selected for processing (1)

packages/worker/client/routes/account-secrets.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

packages/worker/client/routes/account-secrets.tsx

📝 Walkthrough

Walkthrough

Extends the E2E test helper to accept optional allowedHosts/allowedCapabilities and adds route logic to detect requested hosts/capabilities from approval links, rendering an "Already added" status when matches exist instead of showing the approval card.

Changes

Cohort / File(s)	Summary
E2E Test Helpers & Tests `e2e/account-secrets.spec.ts`	Extended local `saveSecret` helper to accept optional `allowedHosts` and `allowedCapabilities`. Added an E2E test verifying that an approval link with a host already present shows the "Already added" status and suppresses the approval UI.
Account Secrets Route Logic `packages/worker/client/routes/account-secrets.tsx`	Added URL parsing/normalization for `allowed-host`/capability, `getAlreadyAddedNotice` logic to detect host/capability matches against selected secret (and fallback approval state), and updated rendering to show an "Already added" status card when matches are found rather than the approval card.

Sequence Diagram

sequenceDiagram
    participant User as User
    participant Route as AccountSecretsRoute
    participant Data as SecretDataStore
    participant UI as UIRenderer

    User->>Route: Navigate to approval link (e.g. ?allowed-host=api.cloudflare.com&request=stale-token)
    Route->>Route: Parse query params & normalize requested host/capability
    Route->>Data: Read selected secret or approval state (allowedHosts / allowedCapabilities)
    Route->>Route: Compare requested vs existing -> compute alreadyAddedNotice

    alt Host or capability already present
        Route->>UI: Render AlreadyAdded status (with per-item messages)
        UI->>User: Display "Already added" status and hide approval controls
    else Not present
        Route->>UI: Render Approval card (pre-filled requested host/capability)
        UI->>User: Display approval controls
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Stabilize account secret detail refresh #75: Modifies the same saveSecret E2E helper and adds related Playwright tests for account secrets behavior.
feat: add secret-aware fetch gateway #61: Adds allowed-host metadata and approval token handling used by the route's detection logic.
Add batched secret approval errors with deep links #72: Changes account-secrets route parsing/normalization and approval UI control flow similar to this PR.

Poem

🐰 I hopped through queries, sniffed the host in sight,
Found it cozy in storage, already snug and tight.
No need to ask again — the job is done right,
I twirl my whiskers, approve a restful night. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add already-added notice for secret approval links' directly and clearly summarizes the main feature being added, matching the primary objective of showing an 'Already added' notice on secret detail pages.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch cursor/allowed-host-presence-notice-7dd9

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-03-30T23:47:44Z

🔎 Preview deployed: https://kody-pr-109.kentcdodds.workers.dev

Worker: kody-pr-109
D1: kody-pr-109-db
KV: kody-pr-109-oauth-kv

Mocks:

ai: https://kody-pr-109-mock-ai.kentcdodds.workers.dev/__mocks (kody-pr-109-mock-ai)
cloudflare: https://kody-pr-109-mock-cloudflare.kentcdodds.workers.dev/__mocks (kody-pr-109-mock-cloudflare)
cursor: https://kody-pr-109-mock-cursor.kentcdodds.workers.dev/__mocks (kody-pr-109-mock-cursor)
github: https://kody-pr-109-mock-github.kentcdodds.workers.dev/__mocks (kody-pr-109-mock-github)
resend: https://kody-pr-109-mock-resend.kentcdodds.workers.dev/__mocks (kody-pr-109-mock-resend)

kentcdodds · 2026-03-31T00:50:49Z

@cursoragent please address the failing typescript in CI

cursor · 2026-03-31T00:50:56Z

Summary

Fixed the CI TypeScript failure by narrowing the nullable approval state into a local approvalCard value before rendering the approval branch, preserving the already-added behavior while satisfying strict null checks.account-secrets.tsx:826-919

Testing

✅ npm run typecheck — this directly targets the failing CI job and now passes cleanly after the narrowing fix.terminal:1-12
✅ npm run test:e2e -- e2e/account-secrets.spec.ts — reran the focused secrets-route regression spec to confirm the UI behavior still works after the TypeScript-only render change.terminal:1-24

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.}

cursor · 2026-03-31T00:57:43Z

+							))}
+						</ul>
+					</section>
+				) : null}


Stale data notice shown during location change refresh

Low Severity

The alreadyAddedNotice section renders without checking isRefreshingForLocationChange, unlike approvalCard which is explicitly gated by it. During a navigation-triggered data refresh, selectedSecret and approval hold stale data from the previous page while currentHref already reflects the new URL. This means getAlreadyAddedNotice can match the new URL's allowed-host param against the old secret's allowed hosts, briefly flashing a misleading "Already added" notice for the wrong secret.

Additional Locations (1)

packages/worker/client/routes/account-secrets.tsx#L825-L836

Add already-added notice for secret approval links

3e91c2f

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

kentcdodds marked this pull request as ready for review March 30, 2026 23:45

Fix account secrets approval card type narrowing

e4db22c

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

kentcdodds merged commit 0b4e975 into main Mar 31, 2026
9 checks passed

kentcdodds deleted the cursor/allowed-host-presence-notice-7dd9 branch March 31, 2026 00:55

cursor Bot reviewed Mar 31, 2026

View reviewed changes

Navigation Menu

Repository navigation

Add already-added notice for secret approval links - #109

Add already-added notice for secret approval links#109
kentcdodds merged 2 commits into
mainfrom
cursor/allowed-host-presence-notice-7dd9

kentcdodds commented Mar 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 30, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

kentcdodds commented Mar 31, 2026

Uh oh!

cursor Bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kentcdodds commented Mar 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kentcdodds commented Mar 31, 2026

Uh oh!

cursor Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Mar 31, 2026

Choose a reason for hiding this comment

Stale data notice shown during location change refresh

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kentcdodds commented Mar 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 30, 2026 •

edited

Loading

github-actions Bot commented Mar 30, 2026 •

edited

Loading

cursor Bot commented Mar 31, 2026 •

edited

Loading