Skip to content

fix: prevent dashboard crash when vault repo is deleted - #2

Merged
NicolasRitouet merged 2 commits into
mainfrom
fix/dashboard-crash-deleted-repo
Feb 15, 2026
Merged

fix: prevent dashboard crash when vault repo is deleted#2
NicolasRitouet merged 2 commits into
mainfrom
fix/dashboard-crash-deleted-repo

Conversation

@NicolasRitouet

@NicolasRitouet NicolasRitouet commented Feb 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Backend: Wrap getUserRoleWithApp() in try-catch inside getVaultsForUser() so a deleted/inaccessible repo no longer crashes the entire vault listing via Promise.all(). Failed vaults get permission: null and warning: "repo_inaccessible".
  • Dashboard: Show a dismissable amber banner when inaccessible vaults are detected. Inaccessible vault cards are dimmed (opacity-60), non-clickable, and show an always-visible delete button so users can clean up.
  • Vault detail page: Guard against nullable permission with effectivePermission fallback to 'read'.

Test plan

  • Delete a GitHub repo that has a Keyway vault, reload dashboard — other vaults display normally
  • Inaccessible vault shows dimmed with "Repo not found" subtitle and visible delete menu
  • Amber banner appears with correct count and is dismissable via X button
  • Clicking an inaccessible vault card does not navigate anywhere
  • Deleting an inaccessible vault works and removes it from the list
  • pnpm --filter keyway-api type-check passes
  • pnpm --filter keyway-dashboard build passes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added repository accessibility warnings to alert users when vault repositories are inaccessible.
    • Dashboard shows a dismissible banner with the count of inaccessible vaults.
    • Inaccessible vault cards display a "Repo not found" warning and are non-navigable.
  • Bug Fixes

    • Graceful handling when repository access checks fail to avoid errors and surface a warning instead.

Catch per-vault GitHub API errors in getVaultsForUser so a deleted repo
no longer kills the entire vault listing via Promise.all. Inaccessible
vaults are returned with warning: "repo_inaccessible" and permission: null,
and the dashboard renders them dimmed with a dismissable banner and
an always-visible delete button.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Feb 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
keyway-dashboard Ready Ready Preview, Comment Feb 15, 2026 4:48pm

Request Review

@coderabbitai

coderabbitai Bot commented Feb 15, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Backend now marks vaults with a repo_inaccessible warning when permission checks fail (logged instead of thrown). The API surfaces nullable permission and optional warning. Frontend shows a dismissible inaccessible-repo banner and renders inaccessible vault cards as non-navigable, allowing deletion.

Changes

Cohort / File(s) Summary
Backend service
packages/backend/src/services/vault.service.ts
Wraps permission fetch in try/catch; logs failures and sets warning: "repo_inaccessible" on VaultListItem instead of throwing.
API & types
packages/dashboard/lib/api/vaults.ts, packages/dashboard/lib/types.ts
Treats permission as nullable (`string
Dashboard main
packages/dashboard/app/(dashboard)/page.tsx
Adds inaccessibleVaultCount and a dismissible "Inaccessible repo" banner shown when vaults have warning === 'repo_inaccessible'.
Vault detail page
packages/dashboard/app/(dashboard)/vaults/[owner]/[repo]/page.tsx
Introduces effectivePermission defaulting to 'read' when permission is null and uses it for permission-driven UI and admin checks.
Vault card component
packages/dashboard/app/components/dashboard/VaultCard.tsx
Adds isInaccessible handling: displays inaccessible cards with reduced opacity, disables navigation (non-Link wrapper), and allows deletion for inaccessible vaults.
Tests & mocks
packages/dashboard/tests/*
Updates: BulkImportModal tests change existingSecretNames to Set<string>; adds next/image mock; adds React Query provider helper for ActivityPage tests.

Sequence Diagram

sequenceDiagram
    participant User as User
    participant Dashboard as Dashboard Page
    participant API as VaultsApiClient
    participant Service as Backend Service
    participant Repo as Repository

    User->>Dashboard: Load dashboard
    Dashboard->>API: getVaults()
    API->>Service: request vaults for user
    Service->>Repo: check user permission
    Repo-->>Service: permission check fails / inaccessible
    Service->>Service: catch error, log warning
    Service->>Service: set warning = "repo_inaccessible"
    Service-->>API: return vaults with warning
    API-->>Dashboard: vault list (permission nullable, warning included)
    Dashboard->>Dashboard: count vaults with warning
    alt inaccessibleVaultCount > 0
        Dashboard->>User: render dismissible "Inaccessible repo" banner
    end
    Dashboard->>Dashboard: render VaultCards
    alt vault.warning == 'repo_inaccessible'
        Dashboard->>User: render non-navigable card, enable delete action
    else
        Dashboard->>User: render normal navigable card
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I sniffed a vault behind a door,
A tiny flag that said "no more" —
I logged the trouble, raised a sign,
A banner waves, the cards decline,
Now rabbits know which burrows bore.

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing dashboard crashes when vault repositories are deleted, which is the core objective of this pull request.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/dashboard-crash-deleted-repo

No actionable comments were generated in the recent review. 🎉


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.

- BulkImportModal: pass Set<string> instead of string[] for existingSecretNames
- SecretRow: add missing next/image mock
- ActivityPage: wrap render in QueryClientProvider

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@NicolasRitouet
NicolasRitouet merged commit d2b589f into main Feb 15, 2026
5 checks passed
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