starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-21 00:03:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(mobile): reset every client-scoped GitHub cache on host change

Browse Source 
Final review round plus CodeRabbit.

- The render-time client reset cleared items and the slug cache but left
  githubRepoSources, githubSourceErrors and githubSourceFallbacks, so
  switching hosts could still show the previous host's repo names and
  error banners. All of them reset together now, via a shared
  resetGitHubItemsState that also replaces four duplicated copies of the
  same five setters.
- The route source tests sliced with an unvalidated end index, and
  slice(start, -1) trims one byte instead of failing, so several
  assertions could pass with the marker missing (CodeRabbit). They now go
  through one extractor that asserts both boundaries.
- Added the missing test for the render gate itself: readiness is only
  useful if the renderer consults it, and removing that condition left
  the suite green.
This commit is contained in:
Jinwoo-H
2026-08-08 11:11:12 -04:00
parent bb733bf889
commit a32ab06f4b
2 changed files with 46 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mobile/app/h/[hostId]/tasks.tsx
+16 -13
View File
@@ -2637,6 +2637,15 @@ export default function MobileTasksScreen() {
// Why: task-loading effects use this as a stale-client guard, so the ref
// must be current before those passive effects can run after commit.
const resetGitHubItemsState = useCallback(() => {
setGithubRepoSources({})
setGithubPages([])
setGithubCurrentPage(0)
setGithubTotalCount(null)
setGithubSourceErrors([])
setGithubSourceFallbacks([])
}, [])
// Why: Expo reuses this screen for the next host, so an effect reset runs a
// render too late and the previous host's rows show under the new one. The
// repo list resets itself; these are the other client-scoped caches.
@@ -2645,6 +2654,7 @@ export default function MobileTasksScreen() {
setBoundClient(client)
setItems([])
setGithubRepoSlugCache({})
resetGitHubItemsState()
}
useLayoutEffect(() => {
@@ -2906,7 +2916,7 @@ export default function MobileTasksScreen() {
// pair but must not receive the newer task-specific method calls.
setTasksSupportState({ kind: 'unsupported', client })
setItems([])
setGithubPages([])
resetGitHubItemsState()
setGithubProjectTable(null)
setShowLinearWorkspacePicker(false)
setShowLinearTeamPicker(false)
@@ -3274,11 +3284,7 @@ export default function MobileTasksScreen() {
}
try {
if (provider !== 'github' || githubMode !== 'items') {
setGithubPages([])
setGithubCurrentPage(0)
setGithubTotalCount(null)
setGithubSourceErrors([])
setGithubSourceFallbacks([])
resetGitHubItemsState()
}
if (provider === 'linear' && !linearConnected) {
setItems([])
@@ -3309,11 +3315,7 @@ export default function MobileTasksScreen() {
return
}
setItems([])
setGithubPages([])
setGithubCurrentPage(0)
setGithubTotalCount(null)
setGithubSourceErrors([])
setGithubSourceFallbacks([])
resetGitHubItemsState()
return
}
if (provider === 'github') {
@@ -3448,8 +3450,7 @@ export default function MobileTasksScreen() {
return
}
setItems([])
setGithubSourceErrors([])
setGithubSourceFallbacks([])
resetGitHubItemsState()
setError(err instanceof Error ? err.message : 'Failed to load tasks')
} finally {
if (isCurrent()) {
@@ -3470,6 +3471,8 @@ export default function MobileTasksScreen() {
linearConnected,
linearFilter,
linearOrderBy,
// resetGitHubItemsState is useCallback([]), so its identity never changes
// and listing it here would only cost a line against the max-lines budget.
repoListEnsureLoaded,
provider,
selectedLinearTeamIds,
mobile/src/tasks/github-project-repo-list-load.test.ts
+30 -21
View File
@@ -8,14 +8,20 @@ import { describe, expect, it } from 'vitest'
* these pin the consumption points the original bug lived in. */
const source = readFileSync(new URL('../../app/h/[hostId]/tasks.tsx', import.meta.url), 'utf8')
function loadTasksBody(): string {
const start = source.indexOf('const loadTasks = useCallback(')
expect(start, 'loadTasks must exist in the tasks route').toBeGreaterThan(-1)
const end = source.indexOf('const connectLinearAccount = useCallback(', start)
expect(end, 'loadTasks must be followed by connectLinearAccount').toBeGreaterThan(start)
/** Why: `slice(start, -1)` silently trims one byte instead of failing, so a
* missing end marker would let every assertion below pass vacuously. */
function block(startMarker: string, endMarker: string): string {
const start = source.indexOf(startMarker)
expect(start, `tasks.tsx must contain ${startMarker}`).toBeGreaterThan(-1)
const end = source.indexOf(endMarker, start)
expect(end, `${startMarker} must be followed by ${endMarker}`).toBeGreaterThan(start)
return source.slice(start, end)
}
function loadTasksBody(): string {
return block('const loadTasks = useCallback(', 'const connectLinearAccount = useCallback(')
}
describe('mobile GitHub Project repo list loading', () => {
// Regression (#12966): project mode returned before the repo list was ever
// loaded, so hostedRepos stayed empty and every board row was filtered out.
@@ -55,21 +61,28 @@ describe('mobile GitHub Project readiness and refresh', () => {
// `every` is vacuously true on an empty list, so readiness must ask the
// resource whether that list is real yet rather than infer it.
it('derives slug readiness from the resource status', () => {
const start = source.indexOf('const githubProjectRepoSlugReady = useMemo(')
expect(start, 'slug readiness memo must exist').toBeGreaterThan(-1)
const body = source.slice(start, source.indexOf(' )', start))
const body = block('const githubProjectRepoSlugReady = useMemo(', ' )')
expect(body).toContain('hasSettledHostRepoList(repoList.state)')
expect(body).toContain('[githubRepoSlugCache, hostedRepos, repoList.state]')
})
it('re-reads the host and retries failed slug lookups on refresh', () => {
const start = source.indexOf('const refreshTasks = useCallback(')
expect(start, 'refresh must be a single shared callback').toBeGreaterThan(-1)
expect(source.slice(start, source.indexOf('}, [', start))).toContain('repoListReload()')
// Regression: readiness is only useful if the renderer consults it. Without
// this the board renders "No project items" before the repo list arrives, and
// the memo assertion above stays green.
it('gates the empty state on readiness in the renderer', () => {
const gate = block(
"githubMode === 'project' ? (",
'<Text style={styles.emptyText}>No project items</Text>'
)
expect(gate, 'the empty state must sit behind the readiness spinner').toContain(
'githubProjectTable && !githubProjectRepoSlugReady ? ('
)
})
const project = source.indexOf('const refreshGitHubProject = useCallback(')
expect(project, 'project refresh must be a single shared callback').toBeGreaterThan(-1)
const projectBody = source.slice(project, source.indexOf('}, [', project))
it('re-reads the host and retries failed slug lookups on refresh', () => {
expect(block('const refreshTasks = useCallback(', '}, [')).toContain('repoListReload()')
const projectBody = block('const refreshGitHubProject = useCallback(', '}, [')
expect(projectBody).toContain('dropFailedGitHubRepoSlugEntries')
expect(projectBody).toContain('refreshTasks()')
expect(projectBody).toContain('loadGitHubProjectTable(')
@@ -90,9 +103,7 @@ describe('mobile GitHub Project readiness and refresh', () => {
// Regression: Expo reuses this screen for the next host, so an effect-based
// reset runs a render too late and the previous host's rows show through.
it('clears the other client-scoped caches during render', () => {
const start = source.indexOf('if (boundClient !== client) {')
expect(start, 'the client-scoped reset must run during render').toBeGreaterThan(-1)
const body = source.slice(start, source.indexOf('\n }', start))
const body = block('if (boundClient !== client) {', '\n }')
expect(body).toContain('setItems([])')
expect(body).toContain('setGithubRepoSlugCache({})')
expect(body, 'a ref write here would leak from an abandoned render').not.toContain('.current =')
@@ -100,9 +111,7 @@ describe('mobile GitHub Project readiness and refresh', () => {
// ...and the ref half belongs in the commit phase, for the same reason.
it('resets the selection-hydration ref in the commit phase', () => {
const start = source.indexOf('clientRef.current = client')
expect(start, 'the client ref effect must exist').toBeGreaterThan(-1)
expect(source.slice(start, source.indexOf('}, [client])', start))).toContain(
expect(block('clientRef.current = client', '}, [client])')).toContain(
'repoSelectionHydratedRef.current = false'
)
})