From 356bda10d177f070ec603a72e3e1fb4274fe3a12 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:36:49 -0700 Subject: [PATCH] fix(github): repo cache eviction clears every host's entries, keeping a live twin's (#31312) * fix(github): repo cache eviction drops every host scope's entries, keeping a twin's own Eviction and issue-source changes matched only unscoped key prefixes, so SSH-, runtime- and source-scoped entries survived repo removal. Keys are now composed by one builder and read back by its inverse, so eviction classifies each key by host and keeps only a surviving same-id twin's bucket. Hosted review (GitLab MR) entries are evicted too. Co-Authored-By: Claude * fix(github): read task-source scopes with empty parts back whole Co-Authored-By: Claude * fix(github): an issue-source change evicts only the changed twin's buckets Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../src/store/github/cache-identity.ts | 66 +++++--- .../src/store/github/request-coordination.ts | 9 +- src/renderer/src/store/github/slice-types.ts | 3 +- .../github/work-item-mutation-actions.ts | 52 +++--- src/renderer/src/store/repos/repo-removal.ts | 2 +- .../github-cache-eviction-and-bounds.test.ts | 16 +- .../github-cache-eviction-host-scopes.test.ts | 160 ++++++++++++++++++ .../slices/hosted-review-cache-identity.ts | 3 +- src/shared/github/github-cache-key.ts | 49 +++++- src/shared/task-source-context.ts | 23 ++- 10 files changed, 311 insertions(+), 72 deletions(-) create mode 100644 src/renderer/src/store/slices/github-cache-eviction-host-scopes.test.ts diff --git a/src/renderer/src/store/github/cache-identity.ts b/src/renderer/src/store/github/cache-identity.ts index cdf53ef07f3..b874366988b 100644 --- a/src/renderer/src/store/github/cache-identity.ts +++ b/src/renderer/src/store/github/cache-identity.ts @@ -8,12 +8,17 @@ import type { TaskSourceContext } from '../../../../shared/task-source-context' import { getTaskSourceCacheScope } from '../../../../shared/task-source-context' import { LOCAL_EXECUTION_HOST_ID, - normalizeExecutionHostId + getRepoExecutionHostId, + normalizeExecutionHostId, + type ExecutionHostId } from '../../../../shared/execution-host' import { + composeRepoCacheKey, getGitHubPRCacheKey, - getGitHubRepoCacheKey + getGitHubRepoCacheKey, + repoCacheKeyHostId } from '../../../../shared/github/github-cache-key' +import type { Repo } from '../../../../shared/repo-types' import type { AppState } from '../types' import type { RuntimeClientTarget } from '../../../../shared/runtime-client-target' import type { CacheEntry } from './cache-model' @@ -70,11 +75,11 @@ export function workItemsCacheKey( ): string { const scope = executionHostId?.trim() ?? '' const hostId = normalizeExecutionHostId(scope) - const owner = `${repoId}::${limit}::${query}` - if (hostId) { - return hostId !== LOCAL_EXECUTION_HOST_ID ? `${hostId}::${owner}` : owner - } - return scope ? `${scope}::${owner}` : owner + return composeRepoCacheKey( + hostId ? (hostId !== LOCAL_EXECUTION_HOST_ID ? hostId : null) : scope, + repoId, + `${limit}::${query}` + ) } export function workItemsInflightRequestKey( @@ -137,7 +142,7 @@ export function sourceScopedRepoCacheKey( hasRepoOwner = false ): string { if (sourceContext?.provider === 'github') { - return `${getTaskSourceCacheScope(sourceContext)}::${repoId ?? repoPath}::${suffix}` + return composeRepoCacheKey(getTaskSourceCacheScope(sourceContext), repoId ?? repoPath, suffix) } return runtimeScopedRepoCacheKey( repoPath, @@ -170,25 +175,44 @@ export function prCacheKey( ) } -export function repoCacheKeyPrefixes(repoId: string, repoPath?: string): string[] { - const prefixes = [`${repoId}::`] - if (repoPath && repoPath !== repoId) { - prefixes.push(`${repoPath}::`) +/** A repo's cache entries to drop: every host's bucket except a surviving same-id twin's. */ +export type RepoCacheEviction = { + owners: readonly string[] + keptHostIds: ReadonlySet +} + +export function repoCacheEviction( + repos: readonly Pick[], + repoId: string, + repoPath: string | undefined, + evictedHostId: ExecutionHostId | null +): RepoCacheEviction { + const owners = repoPath && repoPath !== repoId ? [repoId, repoPath] : [repoId] + const keptHostIds = new Set() + // Why: with no known host to evict, every bucket for the id may be the stale one. + if (evictedHostId) { + for (const repo of repos) { + const hostId = getRepoExecutionHostId(repo) + if (repo.id === repoId && hostId !== evictedHostId) { + keptHostIds.add(hostId) + } + } } - return prefixes + return { owners, keptHostIds } } -export function matchesRepoCacheKey(key: string, prefixes: readonly string[]): boolean { - return prefixes.some((prefix) => key.startsWith(prefix)) +export function matchesRepoCacheEviction(key: string, eviction: RepoCacheEviction): boolean { + const hostId = repoCacheKeyHostId(key, eviction.owners) + return hostId !== null && !eviction.keptHostIds.has(hostId) } -export function evictRepoCacheEntries( - cache: Record>, - prefixes: readonly string[] -): { cache: Record>; evicted: boolean } { - let next: Record> | null = null +export function evictRepoCacheEntries( + cache: Record, + eviction: RepoCacheEviction +): { cache: Record; evicted: boolean } { + let next: Record | null = null for (const key of Object.keys(cache)) { - if (!matchesRepoCacheKey(key, prefixes)) { + if (!matchesRepoCacheEviction(key, eviction)) { continue } next ??= { ...cache } diff --git a/src/renderer/src/store/github/request-coordination.ts b/src/renderer/src/store/github/request-coordination.ts index 7aecfcd2b44..93e572a3b1b 100644 --- a/src/renderer/src/store/github/request-coordination.ts +++ b/src/renderer/src/store/github/request-coordination.ts @@ -4,6 +4,7 @@ import type { IssueInfo, PRInfo } from '../../../../shared/github/pull-request-t import type { GitHubWorkItem } from '../../../../shared/github/work-item-types' import type { GetProjectViewTableResult } from '../../../../shared/github/project-result-types' import type { AppState } from '../types' +import { matchesRepoCacheEviction, type RepoCacheEviction } from './cache-identity' export type InflightPR = { promise: Promise @@ -96,13 +97,9 @@ export function releaseProviderRequestSlot(): void { providerRequestsInFlight -= 1 } -export function clearInflightWorkItemsForRepo(repoId: string, repoPath?: string): void { - const prefixes = [`${repoId}::`] - if (repoPath && repoPath !== repoId) { - prefixes.push(`${repoPath}::`) - } +export function clearInflightWorkItemsForRepo(eviction: RepoCacheEviction): void { for (const key of Array.from(inflightWorkItemsRequests.keys())) { - if (prefixes.some((prefix) => key.startsWith(prefix))) { + if (matchesRepoCacheEviction(key, eviction)) { inflightWorkItemsRequests.delete(key) } } diff --git a/src/renderer/src/store/github/slice-types.ts b/src/renderer/src/store/github/slice-types.ts index 709a911a0ab..e8159c0f24a 100644 --- a/src/renderer/src/store/github/slice-types.ts +++ b/src/renderer/src/store/github/slice-types.ts @@ -254,7 +254,8 @@ export type GitHubSlice = { /** Required: a twin's rows share the id, so only the caller's row host names which one changed. */ hostId: ExecutionHostId ) => Promise - evictGitHubRepoCaches: (repoId: string, repoPath?: string) => void + /** Drops the row's entries on every host scope; `hostId` names the row so a same-id twin on another host keeps its own. */ + evictGitHubRepoCaches: (repoId: string, repoPath?: string, hostId?: ExecutionHostId) => void // ── ProjectV2 view cache ───────────────────────────────────────────── projectViewCache: Record> /** `sourceTarget` names the board's row-less source host; the focused server is never read. */ diff --git a/src/renderer/src/store/github/work-item-mutation-actions.ts b/src/renderer/src/store/github/work-item-mutation-actions.ts index b238bc6e613..e6d9c531803 100644 --- a/src/renderer/src/store/github/work-item-mutation-actions.ts +++ b/src/renderer/src/store/github/work-item-mutation-actions.ts @@ -2,16 +2,28 @@ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import type { GitHubSlice } from './slice-types' import { toast } from 'sonner' -import type { GitHubWorkItem } from '../../../../shared/github/work-item-types' import { parseGitHubIssueOrPRLink } from '../../../../shared/github/links' import { githubRepoIdentityKey } from '../../../../shared/github/repository-identity-key' import { getTaskSourceCacheScope } from '../../../../shared/task-source-context' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { translate } from '@/i18n/i18n' import { repoMatchesHostIdentity } from '../slices/repo-host-identity' -import type { CacheEntry } from './cache-model' -import { evictRepoCacheEntries, repoCacheKeyPrefixes } from './cache-identity' +import { evictRepoCacheEntries, repoCacheEviction, type RepoCacheEviction } from './cache-identity' import { ERROR_TOAST_DURATION } from './cache-policy' import { clearInflightWorkItemsForRepo } from './request-coordination' +import { findRepoForGitHubOwner } from './repository-routing' + +// Why: the row's own host is the bucket to drop; a same-id twin on another host keeps its own. +function repoCacheEvictionForRow( + state: AppState, + repoId: string, + repoPath: string | undefined, + hostId?: ExecutionHostId +): RepoCacheEviction { + const row = hostId ? undefined : findRepoForGitHubOwner(state, repoId, repoPath ?? '') + const evictedHostId = hostId ?? (row ? getRepoExecutionHostId(row) : null) + return repoCacheEviction(state.repos, repoId, repoPath, evictedHostId) +} export const createWorkItemMutationActions = ( set: Parameters>[0], @@ -82,32 +94,26 @@ export const createWorkItemMutationActions = ( // Why: the optimistic patch may now disagree with disk; resync rather than leave a lie on screen. void get().fetchReposForAllHosts() } + const eviction = repoCacheEvictionForRow(get(), repoId, repoPath, hostId) // Why: clear inflight dedupe BEFORE bumping the nonce so the re-triggered fetch can't collapse onto a pre-flip in-flight entry. - clearInflightWorkItemsForRepo(repoId, repoPath) + clearInflightWorkItemsForRepo(eviction) // Why: evict AFTER the await so an overlapping fetch can't repopulate with pre-flip data; also drops legacy path-scoped keys. - set((s) => { - const prefix = `${repoId}::` - const legacyPrefix = `${repoPath}::` - const next: Record> = {} - for (const [key, entry] of Object.entries(s.workItemsCache)) { - if (!key.startsWith(prefix) && !key.startsWith(legacyPrefix)) { - next[key] = entry - } - } + set((s) => ({ + workItemsCache: evictRepoCacheEntries(s.workItemsCache, eviction).cache, // Why: the Tasks fetch effect keys on the nonce, not the cache, so bump it to re-run and re-populate the evicted entries. - return { workItemsCache: next, workItemsInvalidationNonce: s.workItemsInvalidationNonce + 1 } - }) + workItemsInvalidationNonce: s.workItemsInvalidationNonce + 1 + })) }, - evictGitHubRepoCaches: (repoId, repoPath) => { - clearInflightWorkItemsForRepo(repoId, repoPath) + evictGitHubRepoCaches: (repoId, repoPath, hostId) => { + const eviction = repoCacheEvictionForRow(get(), repoId, repoPath, hostId) + clearInflightWorkItemsForRepo(eviction) 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 workItems = evictRepoCacheEntries(s.workItemsCache, eviction) + const prs = evictRepoCacheEntries(s.prCache, eviction) + const issues = evictRepoCacheEntries(s.issueCache, eviction) + const checks = evictRepoCacheEntries(s.checksCache, eviction) + const comments = evictRepoCacheEntries(s.commentsCache, eviction) const updates: Partial = {} if (workItems.evicted) { diff --git a/src/renderer/src/store/repos/repo-removal.ts b/src/renderer/src/store/repos/repo-removal.ts index 05e3d1c208c..7cf590c744f 100644 --- a/src/renderer/src/store/repos/repo-removal.ts +++ b/src/renderer/src/store/repos/repo-removal.ts @@ -179,7 +179,7 @@ export function createRepoRemovalActions( const repoPath = get().repos.find((repo) => repoMatchesHostIdentity(repo, projectId, ownerHostId) )?.path - get().evictGitHubRepoCaches(projectId, repoPath) + get().evictGitHubRepoCaches(projectId, repoPath, ownerHostId) const { clearRepoSlugCacheEntry } = await import('../../lib/repo-slug-index') clearRepoSlugCacheEntry(projectId) diff --git a/src/renderer/src/store/slices/github-cache-eviction-and-bounds.test.ts b/src/renderer/src/store/slices/github-cache-eviction-and-bounds.test.ts index 863ca4f6ec3..9c0e0bc91bb 100644 --- a/src/renderer/src/store/slices/github-cache-eviction-and-bounds.test.ts +++ b/src/renderer/src/store/slices/github-cache-eviction-and-bounds.test.ts @@ -62,7 +62,7 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { expect(state.workItemsInvalidationNonce).toBe(5) }) - it('preserves runtime and explicit-source-prefixed entries during repo eviction', () => { + it('evicts runtime and explicit-source-prefixed entries of a repo no row still owns', () => { const store = createTestStore() const sourceContext = githubSourceContext('runtime:source-env', 'source-repo') const runtimeKey = workItemsCacheKey('repo-1', 20, '', 'runtime:owner-env') @@ -77,14 +77,11 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { store.getState().evictGitHubRepoCaches('repo-1', '/repo/one') - expect(store.getState().workItemsCache).toEqual({ - [runtimeKey]: { data: [], fetchedAt: 1 }, - [sourceKey]: { data: [], fetchedAt: 1 } - }) - expect(store.getState().workItemsInvalidationNonce).toBe(4) + expect(store.getState().workItemsCache).toEqual({}) + expect(store.getState().workItemsInvalidationNonce).toBe(5) }) - it('preference invalidation removes local keys but preserves prefixed host/source keys', async () => { + it('preference invalidation removes every host bucket of a repo with one row', async () => { const store = createTestStore() const updateRepo = vi.fn().mockResolvedValue(true) const sourceContext = githubSourceContext('runtime:source-env', 'source-repo') @@ -105,10 +102,7 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { }) await store.getState().setIssueSourcePreference('repo-1', '/repo/one', 'origin', 'local') - expect(store.getState().workItemsCache).toEqual({ - [runtimeKey]: { data: [], fetchedAt: 1 }, - [sourceKey]: { data: [], fetchedAt: 1 } - }) + expect(store.getState().workItemsCache).toEqual({}) expect(store.getState().workItemsInvalidationNonce).toBe(5) expect(updateRepo).toHaveBeenCalledWith( 'repo-1', diff --git a/src/renderer/src/store/slices/github-cache-eviction-host-scopes.test.ts b/src/renderer/src/store/slices/github-cache-eviction-host-scopes.test.ts new file mode 100644 index 00000000000..187c5ffb747 --- /dev/null +++ b/src/renderer/src/store/slices/github-cache-eviction-host-scopes.test.ts @@ -0,0 +1,160 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + issueCacheKey, + prCacheKey, + sourceScopedRepoCacheKey, + workItemsCacheKey +} from '../github/cache-identity' +import { + createTestStore, + githubSourceContext, + makePR, + resetRemoteRuntimeMocks +} from './github-slice-test-harness' +import { toSshExecutionHostId } from '../../../../shared/execution-host' +import { getTaskSourceCacheScope } from '../../../../shared/task-source-context' +import type { Repo } from '../../../../shared/repo-types' + +const SSH_HOST = toSshExecutionHostId('builder') +const RUNTIME_HOST = 'runtime:paired' + +const localRow: Repo = { + id: 'twin', + path: '/laptop/twin', + displayName: 'Twin', + badgeColor: '#000', + addedAt: 1 +} +// Why no executionHostId: older SSH rows carry only connectionId. +const sshRow: Repo = { ...localRow, path: '/remote/twin', addedAt: 2, connectionId: 'builder' } + +function sshSourceScopes(repoId: string): string[] { + const full = githubSourceContext(SSH_HOST, repoId) + // Why: empty parts put `::` inside the scope, which a first-`::` split misreads. + const sparse = { ...full, projectHostSetupId: null, providerIdentity: null } + return [getTaskSourceCacheScope(full), getTaskSourceCacheScope(sparse)] +} + +/** Every repo-owned cache seeded with one entry per host scope, keyed by the real builders. */ +function seedEveryHostScope(repoId: string, repoPath: string) { + const entry = { data: [], fetchedAt: 1 } + const hosts = { local: null, ssh: SSH_HOST, runtime: RUNTIME_HOST } as const + const keys = Object.fromEntries( + Object.entries(hosts).map(([name, hostId]) => [ + name, + { + workItems: workItemsCacheKey(repoId, 20, '', hostId), + pr: prCacheKey(repoPath, repoId, 'main', null, null, hostId, true), + issue: issueCacheKey(repoPath, repoId, 7, null, null, hostId, true), + checks: sourceScopedRepoCacheKey(repoPath, repoId, 'pr-checks::7', null, null, hostId) + } + ]) + ) + const sourceKeys = sshSourceScopes(repoId).map((scope) => + workItemsCacheKey(repoId, 20, '', scope) + ) + const all = Object.values(keys) + return { + keys, + sourceKeys, + state: { + workItemsCache: Object.fromEntries( + [...all.map((k) => k.workItems), ...sourceKeys].map((key) => [key, entry]) + ), + prCache: Object.fromEntries(all.map((k) => [k.pr, { data: makePR(), fetchedAt: 1 }])), + issueCache: Object.fromEntries(all.map((k) => [k.issue, { data: null, fetchedAt: 1 }])), + checksCache: Object.fromEntries(all.map((k) => [k.checks, entry])) + } + } +} + +function cachedKeys(store: ReturnType): string[] { + const s = store.getState() + return [s.workItemsCache, s.prCache, s.issueCache, s.checksCache].flatMap((cache) => + Object.keys(cache) + ) +} + +describe('repo cache eviction across host scopes', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeMocks() + }) + + it('evicting a repo clears its local, SSH-scoped and runtime-scoped entries, and no other repo’s', () => { + const store = createTestStore() + const removed = seedEveryHostScope('repo-1', '/repo/one') + const other = seedEveryHostScope('repo-2', '/repo/two') + store.setState({ + repos: [{ ...localRow, id: 'repo-1', path: '/repo/one' }], + workItemsCache: { ...removed.state.workItemsCache, ...other.state.workItemsCache }, + prCache: { ...removed.state.prCache, ...other.state.prCache }, + issueCache: { ...removed.state.issueCache, ...other.state.issueCache }, + checksCache: { ...removed.state.checksCache, ...other.state.checksCache } + }) + + store.getState().evictGitHubRepoCaches('repo-1', '/repo/one', 'local') + + expect(cachedKeys(store).sort()).toEqual( + [ + ...Object.keys(other.state.workItemsCache), + ...Object.keys(other.state.prCache), + ...Object.keys(other.state.issueCache), + ...Object.keys(other.state.checksCache) + ].sort() + ) + }) + + it('keeps another repo’s entry under a source scope that names this repo', () => { + const store = createTestStore() + const [, sparseScope] = sshSourceScopes('repo-1') + const otherKey = workItemsCacheKey('repo-2', 20, '', sparseScope) + store.setState({ workItemsCache: { [otherKey]: { data: [], fetchedAt: 1 } } }) + + store.getState().evictGitHubRepoCaches('repo-1', '/repo/one') + + expect(Object.keys(store.getState().workItemsCache)).toEqual([otherKey]) + }) + + it('removing the SSH twin keeps the local twin’s entries, and the reverse', () => { + const seeded = seedEveryHostScope('twin', localRow.path) + const local = Object.values(seeded.keys.local) + const ssh = [...Object.values(seeded.keys.ssh), ...seeded.sourceKeys] + + const store = createTestStore() + store.setState({ repos: [localRow, sshRow], ...seeded.state }) + store.getState().evictGitHubRepoCaches('twin', sshRow.path, SSH_HOST) + expect(cachedKeys(store)).toEqual(expect.arrayContaining(local)) + expect(cachedKeys(store).filter((key) => ssh.includes(key))).toEqual([]) + + const reverse = createTestStore() + reverse.setState({ repos: [localRow, sshRow], ...seeded.state }) + reverse.getState().evictGitHubRepoCaches('twin', localRow.path, 'local') + expect(cachedKeys(reverse)).toEqual(expect.arrayContaining(ssh)) + expect(cachedKeys(reverse).filter((key) => local.includes(key))).toEqual([]) + }) + + it('without a host, evicts the bucket of the row at the request path', () => { + const seeded = seedEveryHostScope('twin', localRow.path) + const store = createTestStore() + store.setState({ repos: [localRow, sshRow], ...seeded.state }) + + store.getState().evictGitHubRepoCaches('twin', sshRow.path) + + expect(Object.keys(store.getState().workItemsCache)).toEqual([seeded.keys.local.workItems]) + }) + + it('an issue-source change on the SSH twin drops its work items and keeps the local twin’s', async () => { + const seeded = seedEveryHostScope('twin', localRow.path) + const store = createTestStore() + store.setState({ + repos: [localRow, sshRow], + updateRepo: vi.fn().mockResolvedValue(true), + ...seeded.state + }) + + await store.getState().setIssueSourcePreference('twin', sshRow.path, 'origin', SSH_HOST) + + expect(Object.keys(store.getState().workItemsCache)).toEqual([seeded.keys.local.workItems]) + }) +}) diff --git a/src/renderer/src/store/slices/hosted-review-cache-identity.ts b/src/renderer/src/store/slices/hosted-review-cache-identity.ts index c53f91a82f4..e7949359f91 100644 --- a/src/renderer/src/store/slices/hosted-review-cache-identity.ts +++ b/src/renderer/src/store/slices/hosted-review-cache-identity.ts @@ -1,4 +1,5 @@ import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { composeRepoCacheKey } from '../../../../shared/github/github-cache-key' import { getSettingsFocusedExecutionHostId, normalizeExecutionHostId, @@ -24,7 +25,7 @@ export function getHostedReviewCacheKey( hasRepoOwner = false ): string { const scope = getHostedReviewCacheHostScope(settings, connectionId, executionHostId, hasRepoOwner) - return `${scope}::${repoId ?? repoPath}::${branch}` + return composeRepoCacheKey(scope, repoId ?? repoPath, branch) } function getHostedReviewCacheHostScope( diff --git a/src/shared/github/github-cache-key.ts b/src/shared/github/github-cache-key.ts index 4bc08dd1bac..939057b7911 100644 --- a/src/shared/github/github-cache-key.ts +++ b/src/shared/github/github-cache-key.ts @@ -2,8 +2,10 @@ import type { GlobalSettings } from '../global-settings-types' import { LOCAL_EXECUTION_HOST_ID, normalizeExecutionHostId, - toSshExecutionHostId + toSshExecutionHostId, + type ExecutionHostId } from '../execution-host' +import { taskSourceCacheScopeHostId } from '../task-source-context' type RuntimeFocusSettings = Pick | null | undefined @@ -16,14 +18,47 @@ export function getGitHubRepoCacheKey( executionHostId?: string | null, hasRepoOwner = false ): string { - const owner = repoId ?? repoPath - const scope = getGitHubCacheHostScope(settings, connectionId, executionHostId, hasRepoOwner) // Why: runtime/SSH lookups can observe different remotes than the local repo // path, so cache keys include the repo's owning execution boundary. - if (scope) { - return `${scope}::${owner}::${suffix}` + return composeRepoCacheKey( + getGitHubCacheHostScope(settings, connectionId, executionHostId, hasRepoOwner), + repoId ?? repoPath, + suffix + ) +} + +/** + * The one shape of a repo-owned cache key: `[scope::]owner::suffix`, where scope is an execution + * host id or a task-source scope and no scope means this computer. Read back by repoCacheKeyHostId. + */ +export function composeRepoCacheKey( + scope: string | null | undefined, + owner: string, + suffix: string +): string { + return scope ? `${scope}::${owner}::${suffix}` : `${owner}::${suffix}` +} + +/** Which host's bucket `key` is when one of `owners` (repo id or path) owns it; null when none does. */ +export function repoCacheKeyHostId(key: string, owners: readonly string[]): ExecutionHostId | null { + if (owners.some((owner) => key.startsWith(`${owner}::`))) { + return LOCAL_EXECUTION_HOST_ID } - return `${owner}::${suffix}` + // Why not the first `::`: a task-source scope with empty parts contains `::` itself. + for (const owner of owners) { + const marker = `::${owner}::` + for (let at = key.indexOf(marker); at !== -1; at = key.indexOf(marker, at + 1)) { + const scope = key.slice(0, at) + // Why the `::` check: a composed host id is encoded, so a candidate spanning `::` is owner + suffix. + const hostId = + (scope.includes('::') ? null : normalizeExecutionHostId(scope)) ?? + taskSourceCacheScopeHostId(scope) + if (hostId) { + return hostId + } + } + } + return null } function getGitHubCacheHostScope( @@ -57,7 +92,7 @@ export function getLegacyGitHubRepoCacheKey( repoId: string | undefined, suffix: string ): string { - return `${repoId ?? repoPath}::${suffix}` + return composeRepoCacheKey(null, repoId ?? repoPath, suffix) } export function getGitHubPRCacheKey( diff --git a/src/shared/task-source-context.ts b/src/shared/task-source-context.ts index cc54a689a12..beaedb2de30 100644 --- a/src/shared/task-source-context.ts +++ b/src/shared/task-source-context.ts @@ -13,7 +13,7 @@ import { taskProviderIdentityCachePart, type TaskProviderIdentity } from './task-provider-identity' -import type { TaskProvider } from './task-providers' +import { isTaskProvider, type TaskProvider } from './task-providers' import type { GlobalSettings } from './global-settings-types' import type { Repo } from './repo-types' @@ -186,6 +186,27 @@ export function getTaskSourceCacheScope( .join(':') } +// provider, hostId, projectId, projectHostSetupId, repoId, identity; accountLabel is optional. +const TASK_SOURCE_CACHE_SCOPE_FIXED_PARTS = 6 + +/** The host a {@link getTaskSourceCacheScope} scope was built for, or null when `scope` isn't a whole one. */ +export function taskSourceCacheScopeHostId(scope: string): ExecutionHostId | null { + // Why count parts: empty parts put `::` inside a scope, so a truncated prefix must not parse as one. + const parts = scope.split(':') + const whole = + parts.length === TASK_SOURCE_CACHE_SCOPE_FIXED_PARTS || + (parts.length === TASK_SOURCE_CACHE_SCOPE_FIXED_PARTS + 1 && parts.at(-1) !== '') + const [provider, encodedHostId] = parts + if (!whole || !isTaskProvider(provider) || !encodedHostId) { + return null + } + try { + return normalizeExecutionHostId(decodeURIComponent(encodedHostId)) + } catch { + return null + } +} + export function buildWorkspaceRunContext(args: { projectId: string hostId: string | null | undefined