mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
Remove deleted repos from GitHub views
Implements the filtering behavior described in docs/remove-repo-github-views.md so removed repositories are excluded from GitHub task and project views.
This commit is contained in:
@@ -0,0 +1,95 @@
|
||||
# Remove Repos From GitHub Views
|
||||
|
||||
## Problem
|
||||
|
||||
When a repo is removed from Orca, GitHub Tasks/Projects can still surface that repo’s data.
|
||||
|
||||
Current behavior is mixed:
|
||||
- Tasks repo selection is pruned when `repos` changes, so new fetches target only live repos.
|
||||
- `removeRepo` does **not** evict GitHub caches (`workItemsCache`, `prCache`, `issueCache`, `checksCache`, `commentsCache`) and does not clear in-flight work-item requests.
|
||||
- Project table rendering is cache-driven and currently unfiltered by “repos opened in Orca”, so removed-repo rows remain visible until a refetch happens to exclude them.
|
||||
- Project row actions already gate on slug→repo matching; unmatched rows fall back to slug-mode/open-in-GitHub behavior. Visibility is the gap, not action routing.
|
||||
|
||||
## Code Reality (must be reflected in design)
|
||||
|
||||
- `workItemsCacheKey(repoId, limit, query)` is repo-id keyed.
|
||||
- `selectTaskPageWorkItemsCacheEntries` still calls `workItemsCacheKey(repo.path, ...)` (wrong key).
|
||||
- `findTaskPageDialogWorkItem` scans whole `workItemsCache`, but requires both `id` and `repoId` match.
|
||||
- `repoScopedCacheKey` for PR/issue/check/comment caches is `${repoId ?? repoPath}::${suffix}`; legacy path-keyed entries can exist.
|
||||
- `ProjectViewWrapper` currently passes unfiltered `table` to `ProjectViewList`.
|
||||
- `useRepoSlugIndex` currently returns only a lookup function (no "ready/rebuilding" state), so callers cannot distinguish "index still rebuilding" from "no rows match".
|
||||
- `useRepoSlugIndex` already evicts cache entries for repos no longer in `state.repos`, but repo removal does not explicitly clear by repo id at removal time.
|
||||
|
||||
## Root Cause
|
||||
|
||||
Two independent correctness gaps:
|
||||
1. No repo-removal cache invalidation in GitHub slice.
|
||||
2. No project-row visibility filter against currently opened Orca repos.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- No GitHub-side deletion/mutation of issues, PRs, or project items.
|
||||
- No server-side “opened in Orca” project filter.
|
||||
- No Linear/GitLab behavior changes.
|
||||
- No auto-clone flow from project rows.
|
||||
|
||||
## Required Changes
|
||||
|
||||
1. Add GitHub-slice repo-eviction API
|
||||
- Add `evictGitHubRepoCaches(repoId: string, repoPath?: string)` in `github.ts`.
|
||||
- Evict all matching keys from:
|
||||
- `workItemsCache` (repo-id prefix and legacy path prefix)
|
||||
- `prCache`, `issueCache`, `checksCache`, `commentsCache` (id/path scoped)
|
||||
- Drop matching `inflightWorkItemsRequests` keys.
|
||||
- Bump `workItemsInvalidationNonce` when any `workItemsCache` entries were evicted.
|
||||
|
||||
2. Call eviction from repo removal
|
||||
- In `repos.ts removeRepo`, after backend remove succeeds and before final state prune, read repo path from current state and call:
|
||||
- `evictGitHubRepoCaches(repoId, repoPath)`
|
||||
- `clearRepoSlugCacheEntry(repoId)`
|
||||
- Keep this call in renderer state path (same window that executes removal).
|
||||
|
||||
3. Fix Tasks selector keying
|
||||
- `selectTaskPageWorkItemsCacheEntries` must use `repo.id` for `workItemsCacheKey`.
|
||||
- `repoPath` stays in `TaskPageRepoSourceState` only for UI identity/retry labels.
|
||||
|
||||
4. Filter project rows client-side
|
||||
- In `ProjectViewWrapper`, derive `visibleRows` from `table.rows` where `row.content.repository` resolves via slug index to at least one live Orca repo.
|
||||
- Exclude missing/invalid/unresolvable repository rows once slug index is resolved for current repo set.
|
||||
- Extend `useRepoSlugIndex` (or add a companion hook) to expose readiness for the current repo snapshot (e.g., `{ lookupSlug, ready }` or generation token).
|
||||
- While slug index is rebuilding for a new repo snapshot, keep last rendered filtered table to avoid flash-to-empty; apply strict filtering once ready.
|
||||
- Pass filtered table to `ProjectViewList`.
|
||||
- Keep row action guards; do not rely on filtering alone.
|
||||
|
||||
5. Close stale dialogs/modals on repo/slug changes
|
||||
- If `dialogRepoItem.repoId` is no longer in live repos, close dialog.
|
||||
- If a slug currently in `slugDialog`/`repoNotInOrca` now resolves to a live repo, close stale unknown-repo state.
|
||||
|
||||
## Consistency and Concurrency
|
||||
|
||||
- In-flight race: removal must clear in-flight work-item dedupe keys before nonce-driven re-fetch paths can reuse stale in-flight entries.
|
||||
- `projectViewCache` is keyed by project/view/query, not repo; repo removal cannot key-evict rows surgically. Row filtering is the correctness boundary.
|
||||
- Multi-window: cache invalidation is renderer-local unless each window executes equivalent invalidation on repo change events. Document as eventual consistency unless cross-window invalidation is explicitly wired.
|
||||
- External mutation (remote URL/repo transfer): slug cache can stale. Repo-removal invalidation is required; remote-change invalidation is separate follow-up.
|
||||
|
||||
## Feasibility Constraints
|
||||
|
||||
- There is no single GitHub API call that applies “repo is currently opened in Orca” filtering to a Project view.
|
||||
- Project query overrides are not a substitute for this requirement; they are user-entered search strings and do not provide safe/complete set filtering for dynamic Orca repo membership.
|
||||
- Slug resolution has real cost (IPC + git remote resolution, possibly runtime RPC). Keep async generation guards and avoid blocking render on a full rebuild.
|
||||
|
||||
## Edge Cases
|
||||
|
||||
- Removing the last repo: Tasks selection collapses to empty/all-eligible behavior; Project table should render zero visible rows after slug resolution settles.
|
||||
- Multiple Orca repos mapping to same slug: row remains visible while any matching repo exists.
|
||||
- Non-GitHub repos or unresolved slugs: filtered out from Project visibility.
|
||||
- Repo removed while dialog is open: dialog must close.
|
||||
- Legacy path-keyed cache entries: evict alongside repo-id keys.
|
||||
|
||||
## Rollout Order
|
||||
|
||||
1. Add `evictGitHubRepoCaches` with tests (id/path key eviction, inflight clearing, nonce bump conditions).
|
||||
2. Wire `removeRepo` to eviction + `clearRepoSlugCacheEntry`; add repo-removal tests.
|
||||
3. Fix `task-page-cache-selectors` keying + tests.
|
||||
4. Add `ProjectViewWrapper` filtered-table and stale-dialog cleanup + tests.
|
||||
5. Run typecheck, lint, targeted renderer/store tests.
|
||||
@@ -46,6 +46,7 @@ import type { GitHubWorkItem } from '../../../../shared/types'
|
||||
import ProjectPicker, { type ResolvedProjectSelection } from './ProjectPicker'
|
||||
import ProjectViewList from './ProjectViewList'
|
||||
import ProjectItemSlugDialog from './ProjectItemSlugDialog'
|
||||
import { filterProjectTableRowsByOpenRepos } from './project-row-filtering'
|
||||
|
||||
type Props = Record<string, never>
|
||||
|
||||
@@ -70,7 +71,8 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
const patchProjectIssueOrPr = useAppStore((s) => s.patchProjectIssueOrPr)
|
||||
const patchProjectRowIssueType = useAppStore((s) => s.patchProjectRowIssueType)
|
||||
const addRepoFromStore = useAppStore((s) => s.addRepo)
|
||||
const lookupSlug = useRepoSlugIndex()
|
||||
const repos = useAppStore((s) => s.repos)
|
||||
const { lookupSlug, ready: slugIndexReady } = useRepoSlugIndex()
|
||||
|
||||
const activeProject = settings?.githubProjects?.activeProject ?? null
|
||||
const lastViewByProject = useMemo(
|
||||
@@ -288,6 +290,31 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
const table: GitHubProjectTable | null = currentCacheKey
|
||||
? (projectViewCache[currentCacheKey]?.data ?? null)
|
||||
: null
|
||||
const filteredTable = useMemo(
|
||||
() => (table && slugIndexReady ? filterProjectTableRowsByOpenRepos(table, lookupSlug) : null),
|
||||
[table, slugIndexReady, lookupSlug]
|
||||
)
|
||||
const [lastFilteredTable, setLastFilteredTable] = useState<{
|
||||
cacheKey: string
|
||||
table: GitHubProjectTable
|
||||
} | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
if (!currentCacheKey || !table) {
|
||||
setLastFilteredTable(null)
|
||||
return
|
||||
}
|
||||
if (slugIndexReady && filteredTable) {
|
||||
setLastFilteredTable({ cacheKey: currentCacheKey, table: filteredTable })
|
||||
}
|
||||
}, [currentCacheKey, table, slugIndexReady, filteredTable])
|
||||
|
||||
const visibleTable =
|
||||
slugIndexReady || !currentCacheKey
|
||||
? filteredTable
|
||||
: lastFilteredTable?.cacheKey === currentCacheKey
|
||||
? lastFilteredTable.table
|
||||
: null
|
||||
|
||||
// Parent-dropped toast, once per table.
|
||||
useEffect(() => {
|
||||
@@ -332,6 +359,28 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
repo: string
|
||||
url: string | null
|
||||
} | null>(null)
|
||||
const liveRepoIds = useMemo(() => new Set(repos.map((repo) => repo.id)), [repos])
|
||||
|
||||
useEffect(() => {
|
||||
if (dialogRepoItem && !liveRepoIds.has(dialogRepoItem.repoId)) {
|
||||
setDialogRepoItem(null)
|
||||
}
|
||||
}, [dialogRepoItem, liveRepoIds])
|
||||
|
||||
useEffect(() => {
|
||||
if (!slugIndexReady) {
|
||||
return
|
||||
}
|
||||
if (
|
||||
slugDialog &&
|
||||
lookupSlug(`${slugDialog.origin.owner}/${slugDialog.origin.repo}`).length > 0
|
||||
) {
|
||||
setSlugDialog(null)
|
||||
}
|
||||
if (repoNotInOrca && lookupSlug(`${repoNotInOrca.owner}/${repoNotInOrca.repo}`).length > 0) {
|
||||
setRepoNotInOrca(null)
|
||||
}
|
||||
}, [slugIndexReady, lookupSlug, slugDialog, repoNotInOrca])
|
||||
|
||||
const buildWorkItem = useCallback(
|
||||
(row: GitHubProjectRow, repoId: string): GitHubWorkItem | null => {
|
||||
@@ -597,7 +646,7 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
{table ? (
|
||||
<>
|
||||
<span className="ml-auto rounded-full border border-border/50 bg-background px-2 py-0.5 text-[11px]">
|
||||
{table.totalCount}
|
||||
{visibleTable?.totalCount ?? table.totalCount}
|
||||
</span>
|
||||
{selectedViewUrl ? (
|
||||
<Button
|
||||
@@ -679,9 +728,9 @@ export default function ProjectViewWrapper(_props: Props = {} as Props): React.J
|
||||
}
|
||||
}}
|
||||
/>
|
||||
) : table ? (
|
||||
) : visibleTable ? (
|
||||
<ProjectViewList
|
||||
table={table}
|
||||
table={visibleTable}
|
||||
onOpenDialog={handleOpenDialog}
|
||||
onEditField={handleEditField}
|
||||
onEditAssignees={(row, add, remove) => void handleEditAssignees(row, add, remove)}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { GitHubProjectRow, GitHubProjectTable } from '../../../../shared/github-project-types'
|
||||
import type { Repo } from '../../../../shared/types'
|
||||
import { filterProjectTableRowsByOpenRepos } from './project-row-filtering'
|
||||
|
||||
function repo(id: string): Repo {
|
||||
return {
|
||||
id,
|
||||
path: `/repos/${id}`,
|
||||
displayName: id,
|
||||
badgeColor: '#000',
|
||||
addedAt: 1
|
||||
}
|
||||
}
|
||||
|
||||
function row(id: string, repository: string | null): GitHubProjectRow {
|
||||
return {
|
||||
id,
|
||||
itemType: 'ISSUE',
|
||||
content: {
|
||||
number: 1,
|
||||
title: id,
|
||||
body: null,
|
||||
url: null,
|
||||
state: 'OPEN',
|
||||
stateReason: null,
|
||||
isDraft: null,
|
||||
repository,
|
||||
assignees: [],
|
||||
labels: [],
|
||||
parentIssue: null,
|
||||
issueType: null
|
||||
},
|
||||
fieldValuesByFieldId: {},
|
||||
updatedAt: '2026-01-01T00:00:00Z',
|
||||
position: 0
|
||||
}
|
||||
}
|
||||
|
||||
function table(rows: GitHubProjectRow[]): GitHubProjectTable {
|
||||
return {
|
||||
project: {
|
||||
id: 'P',
|
||||
owner: 'acme',
|
||||
ownerType: 'organization',
|
||||
number: 1,
|
||||
title: 'Project',
|
||||
url: 'https://github.com/orgs/acme/projects/1'
|
||||
},
|
||||
selectedView: {
|
||||
id: 'V',
|
||||
number: 1,
|
||||
name: 'Default',
|
||||
layout: 'TABLE_LAYOUT',
|
||||
filter: '',
|
||||
fields: [],
|
||||
groupByFields: [],
|
||||
sortByFields: []
|
||||
},
|
||||
rows,
|
||||
totalCount: rows.length,
|
||||
parentFieldDropped: false
|
||||
}
|
||||
}
|
||||
|
||||
describe('filterProjectTableRowsByOpenRepos', () => {
|
||||
it('keeps rows whose repository slug resolves to at least one live repo', () => {
|
||||
const rows = [row('visible', 'acme/orca'), row('missing', 'acme/removed')]
|
||||
const filtered = filterProjectTableRowsByOpenRepos(table(rows), (slug) =>
|
||||
slug?.toLowerCase() === 'acme/orca' ? [repo('repo-1')] : []
|
||||
)
|
||||
|
||||
expect(filtered.rows.map((r) => r.id)).toEqual(['visible'])
|
||||
expect(filtered.totalCount).toBe(1)
|
||||
})
|
||||
|
||||
it('keeps rows while any of multiple Orca repos map to the slug', () => {
|
||||
const rows = [row('visible', 'acme/orca')]
|
||||
const filtered = filterProjectTableRowsByOpenRepos(table(rows), () => [
|
||||
repo('repo-1'),
|
||||
repo('repo-2')
|
||||
])
|
||||
|
||||
expect(filtered.rows.map((r) => r.id)).toEqual(['visible'])
|
||||
})
|
||||
|
||||
it('filters missing or unresolved repository slugs', () => {
|
||||
const rows = [row('missing-slug', null), row('unresolved', 'gitlab/orca')]
|
||||
const filtered = filterProjectTableRowsByOpenRepos(table(rows), () => [])
|
||||
|
||||
expect(filtered.rows).toEqual([])
|
||||
expect(filtered.totalCount).toBe(0)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,22 @@
|
||||
import type { GitHubProjectRow, GitHubProjectTable } from '../../../../shared/github-project-types'
|
||||
import type { Repo } from '../../../../shared/types'
|
||||
|
||||
export type ProjectRowSlugLookup = (slug: string | null | undefined) => readonly Repo[]
|
||||
|
||||
export function projectRowHasOpenRepo(
|
||||
row: GitHubProjectRow,
|
||||
lookupSlug: ProjectRowSlugLookup
|
||||
): boolean {
|
||||
return lookupSlug(row.content.repository).length > 0
|
||||
}
|
||||
|
||||
export function filterProjectTableRowsByOpenRepos(
|
||||
table: GitHubProjectTable,
|
||||
lookupSlug: ProjectRowSlugLookup
|
||||
): GitHubProjectTable {
|
||||
const rows = table.rows.filter((row) => projectRowHasOpenRepo(row, lookupSlug))
|
||||
if (rows.length === table.rows.length && table.totalCount === rows.length) {
|
||||
return table
|
||||
}
|
||||
return { ...table, rows, totalCount: rows.length }
|
||||
}
|
||||
@@ -27,11 +27,11 @@ describe('task page cache selectors', () => {
|
||||
const repo = { id: 'repo-1', path: '/repo/one' }
|
||||
const selectedEntry = entry<GitHubWorkItem[]>([workItem('issue-1', 'repo-1')])
|
||||
const firstCache = {
|
||||
[workItemsCacheKey(repo.path, 20, '')]: selectedEntry
|
||||
[workItemsCacheKey(repo.id, 20, '')]: selectedEntry
|
||||
}
|
||||
const secondCache = {
|
||||
...firstCache,
|
||||
[workItemsCacheKey('/repo/two', 20, '')]: entry<GitHubWorkItem[]>([
|
||||
[workItemsCacheKey('repo-2', 20, '')]: entry<GitHubWorkItem[]>([
|
||||
workItem('issue-2', 'repo-2')
|
||||
])
|
||||
}
|
||||
@@ -50,6 +50,18 @@ describe('task page cache selectors', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('selects work-item cache entries by repo id, not legacy path keys', () => {
|
||||
const repo = { id: 'repo-1', path: '/same/path' }
|
||||
const repoEntry = entry<GitHubWorkItem[]>([workItem('issue-1', 'repo-1')])
|
||||
const pathEntry = entry<GitHubWorkItem[]>([workItem('stale', 'legacy')])
|
||||
const cache = {
|
||||
[workItemsCacheKey(repo.id, 20, '')]: repoEntry,
|
||||
[workItemsCacheKey(repo.path, 20, '')]: pathEntry
|
||||
}
|
||||
|
||||
expect(selectTaskPageWorkItemsCacheEntries(cache, [repo], 20, '')).toEqual([repoEntry])
|
||||
})
|
||||
|
||||
it('returns null while the GitHub dialog is closed so cache writes do not re-render it', () => {
|
||||
const item = workItem('issue-1', 'repo-1')
|
||||
const cache = {
|
||||
|
||||
@@ -33,7 +33,7 @@ export function selectTaskPageWorkItemsCacheEntries(
|
||||
limit: number,
|
||||
query: string
|
||||
): (CacheEntry<GitHubWorkItem[]> | undefined)[] {
|
||||
return repos.map((repo) => workItemsCache[workItemsCacheKey(repo.path, limit, query)])
|
||||
return repos.map((repo) => workItemsCache[workItemsCacheKey(repo.id, limit, query)])
|
||||
}
|
||||
|
||||
export function buildTaskPageRepoSourceState(
|
||||
|
||||
@@ -115,35 +115,45 @@ async function buildIndex(
|
||||
return next
|
||||
}
|
||||
|
||||
/** Returns a lookup function `(slug) => Repo[]`. The lookup is stable
|
||||
* across renders until `state.repos` changes; callers in deep trees can
|
||||
* treat it as referentially equal inside a single render cycle. */
|
||||
export function useRepoSlugIndex(): (slug: string | null | undefined) => Repo[] {
|
||||
export type RepoSlugIndexState = {
|
||||
lookupSlug: (slug: string | null | undefined) => Repo[]
|
||||
ready: boolean
|
||||
}
|
||||
|
||||
/** Returns a slug lookup plus readiness for the current repo snapshot. The
|
||||
* lookup is stable across renders until `state.repos` changes; callers in
|
||||
* deep trees can treat it as referentially equal inside a single render cycle. */
|
||||
export function useRepoSlugIndex(): RepoSlugIndexState {
|
||||
const repos = useAppStore((s) => s.repos)
|
||||
const settings = useAppStore((s) => s.settings)
|
||||
const [index, setIndex] = useState<SlugIndex>(() => new Map())
|
||||
const [ready, setReady] = useState(false)
|
||||
// Why: track the current repos snapshot so the effect can ignore stale
|
||||
// resolutions when repos change mid-flight.
|
||||
const generationRef = useRef(0)
|
||||
|
||||
useEffect(() => {
|
||||
const gen = ++generationRef.current
|
||||
setReady(false)
|
||||
void buildIndex(repos, settings).then((next) => {
|
||||
if (gen !== generationRef.current) {
|
||||
return
|
||||
}
|
||||
setIndex(next)
|
||||
setReady(true)
|
||||
})
|
||||
}, [repos, settings])
|
||||
|
||||
return useMemo(
|
||||
() =>
|
||||
(slug: string | null | undefined): Repo[] => {
|
||||
() => ({
|
||||
lookupSlug: (slug: string | null | undefined): Repo[] => {
|
||||
if (!slug) {
|
||||
return []
|
||||
}
|
||||
return index.get(slug.toLowerCase()) ?? []
|
||||
},
|
||||
[index]
|
||||
ready
|
||||
}),
|
||||
[index, ready]
|
||||
)
|
||||
}
|
||||
|
||||
@@ -3,7 +3,7 @@ envelope, and IssueSourceIndicator suppression tests in one file keeps the
|
||||
GitHub slice's cross-cutting invariants verifiable in one place. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { create } from 'zustand'
|
||||
import { createGitHubSlice } from './github'
|
||||
import { createGitHubSlice, workItemsCacheKey } from './github'
|
||||
import type { AppState } from '../types'
|
||||
import type { PRInfo } from '../../../../shared/types'
|
||||
import {
|
||||
@@ -67,6 +67,101 @@ function makePR(overrides: Partial<PRInfo> = {}): PRInfo {
|
||||
}
|
||||
}
|
||||
|
||||
describe('createGitHubSlice.evictGitHubRepoCaches', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetRemoteRuntimeMocks()
|
||||
})
|
||||
|
||||
it('evicts repo-id and legacy path scoped cache entries', () => {
|
||||
const store = createTestStore()
|
||||
const repoId = 'repo-1'
|
||||
const repoPath = '/repo/one'
|
||||
store.setState({
|
||||
workItemsInvalidationNonce: 4,
|
||||
workItemsCache: {
|
||||
[workItemsCacheKey(repoId, 20, '')]: { data: [], fetchedAt: 1 },
|
||||
[workItemsCacheKey(repoPath, 20, '')]: { data: [], fetchedAt: 1 },
|
||||
[workItemsCacheKey('repo-2', 20, '')]: { data: [], fetchedAt: 1 }
|
||||
},
|
||||
prCache: {
|
||||
[`${repoId}::branch`]: { data: makePR(), fetchedAt: 1 },
|
||||
[`${repoPath}::branch`]: { data: makePR(), fetchedAt: 1 },
|
||||
'repo-2::branch': { data: makePR(), fetchedAt: 1 }
|
||||
},
|
||||
issueCache: {
|
||||
[`${repoId}::12`]: { data: {} as never, fetchedAt: 1 },
|
||||
[`${repoPath}::12`]: { data: {} as never, fetchedAt: 1 },
|
||||
'repo-2::12': { data: {} as never, fetchedAt: 1 }
|
||||
},
|
||||
checksCache: {
|
||||
[`${repoId}::pr-checks::12`]: { data: [], fetchedAt: 1 },
|
||||
[`${repoPath}::pr-checks::12`]: { data: [], fetchedAt: 1 },
|
||||
'repo-2::pr-checks::12': { data: [], fetchedAt: 1 }
|
||||
},
|
||||
commentsCache: {
|
||||
[`${repoId}::pr-comments::12`]: { data: [], fetchedAt: 1 },
|
||||
[`${repoPath}::pr-comments::12`]: { data: [], fetchedAt: 1 },
|
||||
'repo-2::pr-comments::12': { data: [], fetchedAt: 1 }
|
||||
}
|
||||
})
|
||||
|
||||
store.getState().evictGitHubRepoCaches(repoId, repoPath)
|
||||
const state = store.getState()
|
||||
|
||||
expect(Object.keys(state.workItemsCache)).toEqual([workItemsCacheKey('repo-2', 20, '')])
|
||||
expect(Object.keys(state.prCache)).toEqual(['repo-2::branch'])
|
||||
expect(Object.keys(state.issueCache)).toEqual(['repo-2::12'])
|
||||
expect(Object.keys(state.checksCache)).toEqual(['repo-2::pr-checks::12'])
|
||||
expect(Object.keys(state.commentsCache)).toEqual(['repo-2::pr-comments::12'])
|
||||
expect(state.workItemsInvalidationNonce).toBe(5)
|
||||
})
|
||||
|
||||
it('does not bump the work-item invalidation nonce when no work-item entries are evicted', () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
workItemsInvalidationNonce: 4,
|
||||
prCache: {
|
||||
'repo-1::branch': { data: makePR(), fetchedAt: 1 }
|
||||
}
|
||||
})
|
||||
|
||||
store.getState().evictGitHubRepoCaches('repo-1', '/repo/one')
|
||||
|
||||
expect(store.getState().prCache).toEqual({})
|
||||
expect(store.getState().workItemsInvalidationNonce).toBe(4)
|
||||
})
|
||||
|
||||
it('clears matching in-flight work-item dedupe keys before the next fetch', async () => {
|
||||
const store = createTestStore()
|
||||
type WorkItemsEnvelope = {
|
||||
items: []
|
||||
sources: { issues: null; prs: null; upstreamCandidate: null }
|
||||
}
|
||||
let resolveFirst: (value: WorkItemsEnvelope) => void = () => {}
|
||||
const firstRequest = new Promise<WorkItemsEnvelope>((resolve) => {
|
||||
resolveFirst = resolve
|
||||
})
|
||||
mockApi.gh.listWorkItems.mockReturnValueOnce(firstRequest).mockResolvedValueOnce({
|
||||
items: [],
|
||||
sources: { issues: null, prs: null, upstreamCandidate: null }
|
||||
})
|
||||
|
||||
const firstFetch = store.getState().fetchWorkItems('repo-1', '/repo/one', 20, '')
|
||||
await Promise.resolve()
|
||||
store.getState().evictGitHubRepoCaches('repo-1', '/repo/one')
|
||||
const secondFetch = store.getState().fetchWorkItems('repo-1', '/repo/one', 20, '')
|
||||
resolveFirst({
|
||||
items: [],
|
||||
sources: { issues: null, prs: null, upstreamCandidate: null }
|
||||
})
|
||||
await firstFetch
|
||||
await secondFetch
|
||||
|
||||
expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('createGitHubSlice.fetchPRChecks', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
@@ -355,6 +355,44 @@ function repoScopedCacheKey(repoPath: string, repoId: string | undefined, suffix
|
||||
return `${repoId ?? repoPath}::${suffix}`
|
||||
}
|
||||
|
||||
function repoCacheKeyPrefixes(repoId: string, repoPath?: string): string[] {
|
||||
const prefixes = [`${repoId}::`]
|
||||
if (repoPath && repoPath !== repoId) {
|
||||
prefixes.push(`${repoPath}::`)
|
||||
}
|
||||
return prefixes
|
||||
}
|
||||
|
||||
function matchesRepoCacheKey(key: string, prefixes: readonly string[]): boolean {
|
||||
return prefixes.some((prefix) => key.startsWith(prefix))
|
||||
}
|
||||
|
||||
function clearInflightWorkItemsForRepo(repoId: string, repoPath?: string): void {
|
||||
const prefixes = repoCacheKeyPrefixes(repoId, repoPath)
|
||||
for (const key of Array.from(inflightWorkItemsRequests.keys())) {
|
||||
if (matchesRepoCacheKey(key, prefixes)) {
|
||||
inflightWorkItemsRequests.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function evictRepoCacheEntries<T>(
|
||||
cache: Record<string, CacheEntry<T>>,
|
||||
prefixes: readonly string[]
|
||||
): { cache: Record<string, CacheEntry<T>>; evicted: boolean } {
|
||||
let next: Record<string, CacheEntry<T>> | null = null
|
||||
for (const key of Object.keys(cache)) {
|
||||
if (!matchesRepoCacheKey(key, prefixes)) {
|
||||
continue
|
||||
}
|
||||
if (!next) {
|
||||
next = { ...cache }
|
||||
}
|
||||
delete next[key]
|
||||
}
|
||||
return next ? { cache: next, evicted: true } : { cache, evicted: false }
|
||||
}
|
||||
|
||||
// Why: 500 entries is generous enough that active developers will never hit it
|
||||
// during normal use, but prevents the cache from growing without bound across
|
||||
// many repos and branches over a long-running session.
|
||||
@@ -553,6 +591,7 @@ export type GitHubSlice = {
|
||||
repoPath: string,
|
||||
preference: IssueSourcePreference
|
||||
) => Promise<void>
|
||||
evictGitHubRepoCaches: (repoId: string, repoPath?: string) => void
|
||||
// ── ProjectV2 view cache ─────────────────────────────────────────────
|
||||
projectViewCache: Record<string, ProjectViewCacheEntry<GitHubProjectTable>>
|
||||
fetchProjectViewTable: (
|
||||
@@ -1650,11 +1689,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
// dispatch could collapse onto it and skip the source swap. Clearing
|
||||
// first makes the "new fetch gets a fresh request" invariant impossible
|
||||
// to trip on later refactors that change zustand or React flush timing.
|
||||
for (const key of Array.from(inflightWorkItemsRequests.keys())) {
|
||||
if (key.startsWith(`${repoId}::`) || key.startsWith(`${repoPath}::`)) {
|
||||
inflightWorkItemsRequests.delete(key)
|
||||
}
|
||||
}
|
||||
clearInflightWorkItemsForRepo(repoId, repoPath)
|
||||
// Why: evict every cache entry keyed on this repo AFTER the IPC
|
||||
// resolves. If we evicted before awaiting, an overlapping fetch triggered
|
||||
// by a different subscriber would hit main with the pre-flip persisted
|
||||
@@ -1679,6 +1714,38 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
||||
})
|
||||
},
|
||||
|
||||
evictGitHubRepoCaches: (repoId, repoPath) => {
|
||||
clearInflightWorkItemsForRepo(repoId, repoPath)
|
||||
set((s) => {
|
||||
const prefixes = repoCacheKeyPrefixes(repoId, repoPath)
|
||||
const workItems = evictRepoCacheEntries(s.workItemsCache, prefixes)
|
||||
const prs = evictRepoCacheEntries(s.prCache, prefixes)
|
||||
const issues = evictRepoCacheEntries(s.issueCache, prefixes)
|
||||
const checks = evictRepoCacheEntries(s.checksCache, prefixes)
|
||||
const comments = evictRepoCacheEntries(s.commentsCache, prefixes)
|
||||
const updates: Partial<AppState> = {}
|
||||
|
||||
if (workItems.evicted) {
|
||||
updates.workItemsCache = workItems.cache
|
||||
updates.workItemsInvalidationNonce = s.workItemsInvalidationNonce + 1
|
||||
}
|
||||
if (prs.evicted) {
|
||||
updates.prCache = prs.cache
|
||||
}
|
||||
if (issues.evicted) {
|
||||
updates.issueCache = issues.cache
|
||||
}
|
||||
if (checks.evicted) {
|
||||
updates.checksCache = checks.cache
|
||||
}
|
||||
if (comments.evicted) {
|
||||
updates.commentsCache = comments.cache
|
||||
}
|
||||
|
||||
return updates
|
||||
})
|
||||
},
|
||||
|
||||
// Why: worktree switches previously force-refreshed GitHub data on every
|
||||
// click, bypassing the 5-min TTL. This variant only fetches when stale,
|
||||
// avoiding unnecessary API calls and latency during rapid switching.
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it, vi, beforeEach } from 'vitest'
|
||||
import { createTestStore, makeWorktree } from './store-test-helpers'
|
||||
import { workItemsCacheKey } from './github'
|
||||
import type { Repo } from '../../../../shared/types'
|
||||
import {
|
||||
createCompatibleRuntimeStatusResponseIfNeeded,
|
||||
@@ -194,6 +195,32 @@ describe('repo slice runtime routing', () => {
|
||||
expect(reposRemove).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('evicts GitHub caches for removed repos using repo id and legacy path keys', async () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
repos: [localRepo],
|
||||
workItemsInvalidationNonce: 2,
|
||||
workItemsCache: {
|
||||
[workItemsCacheKey(localRepo.id, 20, '')]: { data: [], fetchedAt: 1 },
|
||||
[workItemsCacheKey(localRepo.path, 20, '')]: { data: [], fetchedAt: 1 },
|
||||
[workItemsCacheKey('other-repo', 20, '')]: { data: [], fetchedAt: 1 }
|
||||
},
|
||||
prCache: {
|
||||
[`${localRepo.id}::branch`]: { data: {} as never, fetchedAt: 1 },
|
||||
[`${localRepo.path}::branch`]: { data: {} as never, fetchedAt: 1 },
|
||||
'other-repo::branch': { data: {} as never, fetchedAt: 1 }
|
||||
}
|
||||
})
|
||||
|
||||
await store.getState().removeRepo(localRepo.id)
|
||||
|
||||
expect(Object.keys(store.getState().workItemsCache)).toEqual([
|
||||
workItemsCacheKey('other-repo', 20, '')
|
||||
])
|
||||
expect(Object.keys(store.getState().prCache)).toEqual(['other-repo::branch'])
|
||||
expect(store.getState().workItemsInvalidationNonce).toBe(3)
|
||||
})
|
||||
|
||||
it('stops remote runtime terminals instead of killing remote ids through local pty IPC', async () => {
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-remote',
|
||||
|
||||
@@ -187,6 +187,10 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
|
||||
: callRuntimeRpc(target, 'repo.rm', { repo: repoId }, { timeoutMs: 15_000 }))
|
||||
|
||||
get().clearOrcaHookTrustForRepo(repoId)
|
||||
const repoPath = get().repos.find((repo) => repo.id === repoId)?.path
|
||||
get().evictGitHubRepoCaches(repoId, repoPath)
|
||||
const { clearRepoSlugCacheEntry } = await import('../../lib/repo-slug-index')
|
||||
clearRepoSlugCacheEntry(repoId)
|
||||
|
||||
// Kill PTYs for all worktrees belonging to this repo
|
||||
const worktreeIds = (get().worktreesByRepo[repoId] ?? []).map((w) => w.id)
|
||||
|
||||
Reference in New Issue
Block a user