fix: gracefully handle deleted GitHub repos in vault detail page - #6
Conversation
When a GitHub repo backing a vault is deleted, the dashboard was completely blocked with "GitHub App Not Installed" error. This fix shows a degraded read-only view with a warning banner and delete option. - Backend: catch specific ForbiddenError in getVaultByRepo, add ownership check so only vault owners see inaccessible vaults - Backend: add requireAdminOrOwnerAccess middleware for DELETE route, falling back to DB ownership when GitHub API is unavailable - Frontend: show warning banner with delete button, hide permission badge and syncs, disable write operations for inaccessible repos Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR enables vault owners to delete their vaults even when the GitHub App isn't installed by introducing a fallback authorization path. When GitHub App access fails, the system checks vault ownership directly in the database. A new "repo_inaccessible" warning marks vaults where repo access couldn't be verified, with corresponding UI handling to restrict operations and provide deletion capability. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client
participant Route Handler
participant Auth Middleware
participant Vault Service
participant GitHub App
participant Database
User->>Client: DELETE /vaults/:owner/:repo
Client->>Route Handler: Send delete request
Route Handler->>Auth Middleware: Check authorization
Auth Middleware->>Vault Service: Get user role with GitHub App
Vault Service->>GitHub App: Query user role
alt GitHub App installed
GitHub App-->>Vault Service: Return user role
Vault Service-->>Auth Middleware: Return role
alt User is admin
Auth Middleware-->>Route Handler: Authorization granted
Route Handler->>Database: Delete vault
Database-->>Route Handler: Success
Route Handler-->>Client: Vault deleted
else User not admin
Auth Middleware-->>Route Handler: ForbiddenError
Route Handler-->>Client: Access denied
end
else GitHub App not installed
GitHub App-->>Vault Service: ForbiddenError (not installed)
Vault Service->>Database: Query vault owner
Database-->>Vault Service: Return vault details
alt Authenticated user is vault owner
Vault Service-->>Auth Middleware: Authorization granted
Auth Middleware-->>Route Handler: Authorization granted
Route Handler->>Database: Delete vault
Database-->>Route Handler: Success
Route Handler-->>Client: Vault deleted
else User not vault owner
Vault Service-->>Auth Middleware: ForbiddenError
Auth Middleware-->>Route Handler: ForbiddenError
Route Handler-->>Client: Access denied
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/dashboard/app/(dashboard)/vaults/[owner]/[repo]/page.tsx (1)
478-487: Consider adding loading state during vault deletion.The
handleDeleteVaultfunction doesn't set any loading state, which could allow users to click "Delete vault" multiple times. Consider reusingisSubmittingor adding a dedicated state.Suggested improvement
const handleDeleteVault = async () => { + setIsSubmitting(true) try { await api.deleteVault(owner, repo) setShowDeleteModal(false) queryClient.invalidateQueries({ queryKey: ['vaults'] }) router.push('/') } catch (err) { toast.error(err instanceof Error ? err.message : 'Failed to delete vault') + } finally { + setIsSubmitting(false) } }Then pass
isSubmittingtoDeleteVaultModalto disable the confirm button during the operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard/app/`(dashboard)/vaults/[owner]/[repo]/page.tsx around lines 478 - 487, handleDeleteVault lacks a loading state so users can click delete multiple times; add a boolean state (e.g. const [isDeleting, setIsDeleting] = useState(false)) or reuse existing isSubmitting, set setIsDeleting(true) before the await and setIsDeleting(false) in a finally block, keep the existing api.deleteVault, queryClient.invalidateQueries and router.push calls inside try, and pass isDeleting to the DeleteVaultModal as a prop to disable the confirm button while the deletion is in progress.packages/dashboard/lib/api/vaults.ts (1)
44-44: Minor inconsistency in warning field mapping.
getVaultByRepo(line 102) uses conditional spread to only includewarningwhen truthy, butgetVaults(line 44) always assignswarningvia type assertion. Consider aligning both for consistency:Suggested fix for getVaults
permission: v.permission as Vault['permission'], - warning: v.warning as Vault['warning'], + ...(v.warning && { warning: v.warning as Vault['warning'] }), is_private: v.isPrivate,Also applies to: 102-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard/lib/api/vaults.ts` at line 44, getVaults currently assigns the warning field unconditionally using "warning: v.warning as Vault['warning']", which is inconsistent with getVaultByRepo that only includes warning when truthy; update getVaults to mirror getVaultByRepo by conditionally spreading the warning field (e.g., include ...(v.warning ? { warning: v.warning } : {}) ) or otherwise only set warning when v.warning is truthy so both functions map the Vault['warning'] consistently; modify the mapping where "v" is transformed into a Vault object in getVaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/middleware/auth.ts`:
- Around line 361-399: Tests are failing because the test mocks for the auth
middleware do not export the newly added function requireAdminOrOwnerAccess;
update the mock for the "../../src/middleware/auth" module to include
requireAdminOrOwnerAccess (e.g., extend the mocked export with
requireAdminOrOwnerAccess: vi.fn().mockImplementation(async () => {})) so tests
see and can stub this new async function; ensure the mock returns the original
module spread (...actual) plus the new mocked symbol requireAdminOrOwnerAccess.
In `@packages/dashboard/app/`(dashboard)/vaults/[owner]/[repo]/page.tsx:
- Line 470: The test mocks are missing an export for useRouter from
next/navigation; update the mock (where you call vi.mock for "next/navigation")
to import the real module via the async importOriginal pattern and return an
object that spreads the actual exports and adds useRouter as a vi.fn that
returns an object with push, replace, and back mock functions so components
using useRouter (e.g., the useRouter() call) have the expected methods during
tests.
---
Nitpick comments:
In `@packages/dashboard/app/`(dashboard)/vaults/[owner]/[repo]/page.tsx:
- Around line 478-487: handleDeleteVault lacks a loading state so users can
click delete multiple times; add a boolean state (e.g. const [isDeleting,
setIsDeleting] = useState(false)) or reuse existing isSubmitting, set
setIsDeleting(true) before the await and setIsDeleting(false) in a finally
block, keep the existing api.deleteVault, queryClient.invalidateQueries and
router.push calls inside try, and pass isDeleting to the DeleteVaultModal as a
prop to disable the confirm button while the deletion is in progress.
In `@packages/dashboard/lib/api/vaults.ts`:
- Line 44: getVaults currently assigns the warning field unconditionally using
"warning: v.warning as Vault['warning']", which is inconsistent with
getVaultByRepo that only includes warning when truthy; update getVaults to
mirror getVaultByRepo by conditionally spreading the warning field (e.g.,
include ...(v.warning ? { warning: v.warning } : {}) ) or otherwise only set
warning when v.warning is truthy so both functions map the Vault['warning']
consistently; modify the mapping where "v" is transformed into a Vault object in
getVaults.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d2d2ffa-09db-4c9a-b770-40a6d8cb077f
📒 Files selected for processing (5)
packages/backend/src/api/v1/routes/vaults.routes.tspackages/backend/src/middleware/auth.tspackages/backend/src/services/vault.service.tspackages/dashboard/app/(dashboard)/vaults/[owner]/[repo]/page.tsxpackages/dashboard/lib/api/vaults.ts
| } | ||
| } | ||
|
|
||
| const router = useRouter() |
There was a problem hiding this comment.
Pipeline failure: Test mocks need to export useRouter.
The CI is failing because the test mock for next/navigation doesn't include useRouter. Update the test mocks to include this export.
Example fix for test mocks
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
useRouter: vi.fn(() => ({
push: vi.fn(),
replace: vi.fn(),
back: vi.fn(),
})),
}
})🧰 Tools
🪛 GitHub Actions: CI: Dashboard
[error] 470-470: Vitest failed due to missing mock export: "No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?"
🪛 GitHub Check: build
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Secrets List > should render all secrets
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Environment Filters > should filter secrets when environment is clicked
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Environment Filters > should show All filter by default
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Environment Filters > should render all environments as filter buttons
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Vault Header > should show Collaborators button for admin
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Vault Header > should show permission badge
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Vault Header > should show Add Secrets dropdown for admin users
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Vault Header > should render vault avatar
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Vault Header > should render vault name and stats
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
[failure] 470-470: tests/pages/VaultDetailPage.test.tsx > VaultDetailPage > Loading State > should show loading skeletons when data is loading
Error: [vitest] No "useRouter" export is defined on the "next/navigation" mock. Did you forget to return it from "vi.mock"?
If you need to partially mock a module, you can use "importOriginal" helper inside:
vi.mock(import("next/navigation"), async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
// your mocked methods
}
})
❯ VaultDetailPage app/(dashboard)/vaults/[owner]/[repo]/page.tsx:470:18
❯ renderWithHooks ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:15486:18
❯ mountIndeterminateComponent ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:20103:13
❯ beginWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:21626:16
❯ beginWork$1 ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:27465:14
❯ performUnitOfWork ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26599:12
❯ workLoopSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26505:5
❯ renderRootSync ../../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.development.js:26473:7
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard/app/`(dashboard)/vaults/[owner]/[repo]/page.tsx at line
470, The test mocks are missing an export for useRouter from next/navigation;
update the mock (where you call vi.mock for "next/navigation") to import the
real module via the async importOriginal pattern and return an object that
spreads the actual exports and adds useRouter as a vi.fn that returns an object
with push, replace, and back mock functions so components using useRouter (e.g.,
the useRouter() call) have the expected methods during tests.
- Backend: add requireAdminOrOwnerAccess mock in environments.test.ts - Dashboard: add useRouter mock in VaultDetailPage.test.tsx - Dashboard: add DeleteVaultModal mock in VaultDetailPage.test.tsx Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds getOrgMembership to the github mock + a test asserting the installation-token role overrides the user-token role (fix #6 was only exercised via the catch-fallback before). Also documents why ensureOrganizationExists is intentionally insert-only (no role rewrite). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
getVaultByReponow catches specificForbiddenError(not all errors) and returns awarning: "repo_inaccessible"field, with ownership check so only vault owners see the degraded viewrequireAdminOrOwnerAccessmiddleware (scoped to DELETE route only) falls back to DB ownership check when GitHub API is unavailableTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes