Skip to content

feat(ui): add Settings tab with API key management#46

Merged
roziscoding merged 1 commit into
feat/multiple-api-keysfrom
feat/settings-page
Jun 24, 2026
Merged

feat(ui): add Settings tab with API key management#46
roziscoding merged 1 commit into
feat/multiple-api-keysfrom
feat/settings-page

Conversation

@roziscoding

@roziscoding roziscoding commented Jun 24, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Related issue

Checklist

  • Commits follow Conventional Commits
  • Commits are signed (GitHub enforces this — unsigned commits are rejected)
  • mise run lint:fix was run and its output is committed
  • mise run test passes (and mise run test:e2e if transfer/request paths changed)
  • Docs updated if behavior or configuration changed

Greptile Summary

This PR adds a Settings page to the UI with full API key management — listing, creating, editing, and revoking keys — behind a useManagement request wrapper, along with a one-time key reveal modal after creation.

  • apps/ui/app/pages/settings.vue: New page with key list, create/edit form modal, revoke confirm modal, and a one-time reveal modal; copyKey does not catch clipboard API rejections, meaning a failed copy shows no feedback and the user may close the modal without realising the key wasn't captured.
  • apps/ui/app/components/ApiKeyForm.vue: Reusable form handling preset and custom expiration; nowLocal is a computed() with no reactive dependencies so the min constraint on the datetime-local input grows stale over long form sessions.
  • apps/ui/app/types/management.ts and apps/ui/app/layouts/default.vue: Clean additions of the new types and navigation entry.

Confidence Score: 3/5

The page is mostly well-structured, but the one-time key reveal modal has a gap: if the browser clipboard API rejects (permissions denied, non-HTTPS, Safari private mode), the copy silently fails and the user gets no indication — they may close the modal and permanently lose the only chance to see the plaintext key.

The clipboard failure path in copyKey is unhandled in a flow where the cost of failure is high: the key is shown exactly once, and a silent copy failure can leave the user locked out of their newly created credential. The rest of the implementation — list fetching, form validation, revoke confirmation — is clean and follows existing patterns.

apps/ui/app/pages/settings.vue — specifically the copyKey function and the reveal modal's dismissal flow.

Important Files Changed

Filename Overview
apps/ui/app/pages/settings.vue New settings page implementing full API key CRUD (list, create, edit, revoke) with a one-time key reveal modal; copyKey does not handle clipboard API rejection, which can silently fail and leave the user without a copy of their newly created key.
apps/ui/app/components/ApiKeyForm.vue New form component for creating/editing API keys with preset and custom expiration; nowLocal computed has no reactive dependencies and grows stale, causing the datetime-local min attribute to drift behind real time.
apps/ui/app/layouts/default.vue Adds a Settings navigation link to the default layout — minimal, correct change.
apps/ui/app/types/management.ts Adds ApiKey, CreatedApiKey, and ApiKeyInput type definitions — clean and consistent with existing patterns in the file.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant U as User
    participant S as settings.vue
    participant F as ApiKeyForm.vue
    participant API as /api/management/api-keys

    U->>S: Open Settings page
    S->>API: GET api-keys
    API-->>S: ApiKey[]

    U->>S: Click "Create key"
    S->>F: "Show form (initial=null)"
    U->>F: "Fill fields & submit"
    F->>S: emit submit(ApiKeyInput)
    S->>API: POST api-keys
    API-->>S: CreatedApiKey (key shown once)
    S->>S: "showForm=false, created=key"
    S->>API: GET api-keys (refresh)
    S-->>U: Show key reveal modal
    U->>S: Click "Copy" (navigator.clipboard)
    Note over S: ⚠ clipboard error not caught
    U->>S: Click "Done" → dismissReveal()

    U->>S: Click "Edit" on a key
    S->>F: "Show form (initial=ApiKey)"
    U->>F: "Edit & submit"
    F->>S: emit submit(ApiKeyInput)
    S->>API: PATCH api-keys/:id
    API-->>S: 200 OK
    S->>S: "showForm=false"
    S->>API: GET api-keys (refresh)

    U->>S: Click "Revoke"
    S->>S: "confirmTarget=key"
    U->>S: Confirm revoke
    S->>API: DELETE api-keys/:id
    API-->>S: 200 OK
    S->>API: GET api-keys (refresh)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant U as User
    participant S as settings.vue
    participant F as ApiKeyForm.vue
    participant API as /api/management/api-keys

    U->>S: Open Settings page
    S->>API: GET api-keys
    API-->>S: ApiKey[]

    U->>S: Click "Create key"
    S->>F: "Show form (initial=null)"
    U->>F: "Fill fields & submit"
    F->>S: emit submit(ApiKeyInput)
    S->>API: POST api-keys
    API-->>S: CreatedApiKey (key shown once)
    S->>S: "showForm=false, created=key"
    S->>API: GET api-keys (refresh)
    S-->>U: Show key reveal modal
    U->>S: Click "Copy" (navigator.clipboard)
    Note over S: ⚠ clipboard error not caught
    U->>S: Click "Done" → dismissReveal()

    U->>S: Click "Edit" on a key
    S->>F: "Show form (initial=ApiKey)"
    U->>F: "Edit & submit"
    F->>S: emit submit(ApiKeyInput)
    S->>API: PATCH api-keys/:id
    API-->>S: 200 OK
    S->>S: "showForm=false"
    S->>API: GET api-keys (refresh)

    U->>S: Click "Revoke"
    S->>S: "confirmTarget=key"
    U->>S: Confirm revoke
    S->>API: DELETE api-keys/:id
    API-->>S: 200 OK
    S->>API: GET api-keys (refresh)
Loading

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "feat(ui): add Settings tab with API key ..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

New /settings route (nav: Settings) housing API key CRUD against the
management API. Create flow hands off to a one-time reveal modal — the raw
jack_ key is shown once with copy-to-clipboard, since it's never retrievable
again. Keys are identified by name (with #id fallback for unnamed); expiration
is entered via presets (never/30d/90d/1y/custom, future-only) and rendered as
a tri-state signal (never / expires-soon / expired). Edit patches metadata;
the destructive action is 'Revoke' with a consequence-spelling confirm.
@roziscoding roziscoding marked this pull request as ready for review June 24, 2026 19:07
@roziscoding roziscoding merged commit a0e964f into feat/multiple-api-keys Jun 24, 2026
5 checks passed
@roziscoding roziscoding deleted the feat/settings-page branch June 24, 2026 19:07
Comment on lines +85 to +92
function dismissReveal() {
created.value = null
copied.value = false
}

function closeConfirm() {
confirmTarget.value = null
revokeError.value = null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Clipboard failure silently swallowed — user may lose the key

navigator.clipboard.writeText rejects when the Clipboard API is unavailable (non-HTTPS context, browser permissions denied, Safari private mode, etc.). The await is not wrapped in a try/catch, so the rejection propagates unhandled and copied.value is never set to true. The button keeps its "Copy" label and the user gets no error feedback. Because this is the only time the plaintext key is shown, a user who clicks "Copy", sees no change, and then clicks "Done" will lose the key permanently with no warning.

Fix in Claude Code Fix in Codex

return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}`
}

const nowLocal = computed(() => toDatetimeLocal(new Date().toISOString()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 nowLocal is a computed() with no reactive dependencies — new Date() is evaluated once at component mount and never again. The min attribute on the datetime-local input will lag behind real time, so a user who opens the form, waits several minutes, then picks a "custom" date could select a datetime that is technically in the past relative to current time yet still passes the HTML min constraint. The customInFuture guard at submission time catches this, but the stale min makes the browser's native date-picker mislead the user about what dates are selectable.

Suggested change
const nowLocal = computed(() => toDatetimeLocal(new Date().toISOString()))
// Refresh every minute so the min constraint stays current without a reactive
// dependency on a non-reactive value.
const nowLocal = ref(toDatetimeLocal(new Date().toISOString()))
let _nowTimer: ReturnType<typeof setInterval> | undefined
onMounted(() => { _nowTimer = setInterval(() => { nowLocal.value = toDatetimeLocal(new Date().toISOString()) }, 60_000) })
onUnmounted(() => clearInterval(_nowTimer))

Fix in Claude Code Fix in Codex

Comment on lines +162 to +164
<p class="text-xs font-medium" :class="toneClass[expiryInfo(key).tone]">
{{ expiryInfo(key).label }}
</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 expiryInfo(key) is called twice per key row in the template (once to read .tone, once to read .label). Each call recomputes Date.now() and new Date(key.expiresAt). Consider extracting to a single call per row.

Suggested change
<p class="text-xs font-medium" :class="toneClass[expiryInfo(key).tone]">
{{ expiryInfo(key).label }}
</p>
<template v-for="exp in [expiryInfo(key)]" :key="exp.label">
<p class="text-xs font-medium" :class="toneClass[exp.tone]">
{{ exp.label }}
</p>
</template>

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant