mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 16:02:42 +00:00
Harden Linear view persistence against the failures review surfaced
Five issues, each found by a reviewer and reproduced before fixing: - The filter dropdown's prune effect only ran when the user opened the popover, because the filter was always empty at startup. Restoration makes it run on mount, where `availableTeams` may still be the issue-scraped fallback rather than the real fetch. Metadata complete for a *partial* team set passes every R12 guard, so it pruned facets belonging to teams it simply hadn't seen — and the write persisted, deleting them permanently. Gated on `teamsSettled`. - `canonicalize` dedupes but enforces none of the transport bounds; only the throwing parser does. So `serialize` could emit a 101-label filter that the strict `ui.set` schema rejects, which drops the WHOLE taskResumeState — github, jira and linear query included — on every subsequent write, since the renderer resends the merged object each time. Added `boundLinearIssueAttributeFilter` and a round-trip test built from serializer output rather than a literal, which is the only kind that can catch renderer/schema drift. - `linearIssueView` now carries `.catch(undefined)`: value tolerance stops at the top level, so any future instance of the above is a cosmetic reset of the view instead of silent loss of every other resume field. - A workspace switch forced an uncached list read in both directions. The switch is a later observation, so the null-baseline fix didn't cover it; the cache is already workspace-keyed, making the force pure cost. - Recency for the 20-workspace cap came from object key order, which is wrong twice: re-filtering an existing workspace left it at the head (first evicted, though just used), and an array-index-like key enumerates first regardless of insertion, so a write could evict the very entry it added. Recency is now an explicit ordered key list. Also adds the nested parity assertion — the top-level one compares only TaskResumeState's own keys, so a field added to LinearIssueViewResumeState stayed invisible to it, which is exactly what `.strict()` rejects. The wiring test was blind: deleting the hydration guard outright left all four assertions green. The gate is now `shouldPersistLinearIssueView`, unit-tested directly, and the file is renamed to the repo's `*-boundary.test.ts` convention with an assertion that fails on that mutation.
This commit is contained in:
@@ -1,6 +1,13 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { getDefaultUIState } from '../../../../shared/constants'
|
||||
import { LINEAR_ISSUE_ATTRIBUTE_FILTER_ID_MAX_LENGTH } from '../../../../shared/linear-issue-attribute-filter'
|
||||
import {
|
||||
LINEAR_ISSUE_ATTRIBUTE_FILTER_ID_MAX_LENGTH,
|
||||
LINEAR_ISSUE_ATTRIBUTE_FILTER_MAX_LABEL_IDS
|
||||
} from '../../../../shared/linear-issue-attribute-filter'
|
||||
import {
|
||||
defaultLinearIssueViewResumeState,
|
||||
serializeLinearIssueViewResumeState
|
||||
} from '../../../../shared/linear-issue-view-resume-state'
|
||||
import type { OrcaRuntimeService } from '../../orca-runtime'
|
||||
import type { RpcRequest } from '../core'
|
||||
import { RpcDispatcher } from '../dispatcher'
|
||||
@@ -98,16 +105,58 @@ describe('ui.set Linear issue view resume state', () => {
|
||||
}
|
||||
}
|
||||
]
|
||||
])('refuses to persist a view with %s', async (_label, linearIssueView) => {
|
||||
])('drops only the view when it has %s', async (_label, linearIssueView) => {
|
||||
const { dispatcher, updateUIState } = makeDispatcher()
|
||||
|
||||
const response = await dispatcher.dispatch(
|
||||
makeRequest({ sidebarWidth: 280, taskResumeState: { linearIssueView } })
|
||||
makeRequest({
|
||||
sidebarWidth: 280,
|
||||
taskResumeState: { linearQuery: 'label:bug', jiraPreset: 'reported', linearIssueView }
|
||||
})
|
||||
)
|
||||
|
||||
// Why: taskResumeState rides the value-tolerant wrapper, so an invalid view is
|
||||
// dropped from the batch rather than written — the rest of the payload lands.
|
||||
// Why: value tolerance stops at the top level, so without the schema's own `.catch`
|
||||
// one bad view value would drop every other resume field too — and keep doing so on
|
||||
// every subsequent write, since the renderer resends the whole merged object.
|
||||
expect(response).toMatchObject({ ok: true })
|
||||
expect(updateUIState).toHaveBeenCalledWith({ sidebarWidth: 280 })
|
||||
expect(updateUIState).toHaveBeenCalledWith({
|
||||
sidebarWidth: 280,
|
||||
taskResumeState: {
|
||||
linearQuery: 'label:bug',
|
||||
jiraPreset: 'reported',
|
||||
linearIssueView: undefined
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
// Why: hand-written fixtures can't catch renderer/schema drift — only the renderer's
|
||||
// own output can. An over-limit filter here stops ALL resume state from persisting.
|
||||
it.each([
|
||||
[
|
||||
'a filter at the facet-count limit',
|
||||
Array.from(
|
||||
{ length: LINEAR_ISSUE_ATTRIBUTE_FILTER_MAX_LABEL_IDS + 1 },
|
||||
(_, index) => `label-${String(index).padStart(4, '0')}`
|
||||
)
|
||||
],
|
||||
[
|
||||
'a filter with an over-long facet id',
|
||||
['x'.repeat(LINEAR_ISSUE_ATTRIBUTE_FILTER_ID_MAX_LENGTH + 1)]
|
||||
]
|
||||
])('accepts serializer output for %s', async (_label, labelIds) => {
|
||||
const { dispatcher, updateUIState } = makeDispatcher()
|
||||
const linearIssueView = serializeLinearIssueViewResumeState({
|
||||
...defaultLinearIssueViewResumeState(),
|
||||
filtersByWorkspaceId: {
|
||||
'workspace-1': { stateIds: [], priorities: [], assignee: null, labelIds }
|
||||
}
|
||||
})
|
||||
|
||||
const response = await dispatcher.dispatch(
|
||||
makeRequest({ taskResumeState: { linearIssueView } })
|
||||
)
|
||||
|
||||
expect(response).toMatchObject({ ok: true })
|
||||
expect(updateUIState).toHaveBeenCalledWith({ taskResumeState: { linearIssueView } })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -53,7 +53,10 @@ export const TaskResumeState = z
|
||||
})
|
||||
.strict()
|
||||
.optional(),
|
||||
linearIssueView: LinearIssueViewResumeState.optional(),
|
||||
// Why: value tolerance stops at the top level, so without `.catch` one bad view
|
||||
// value drops the whole taskResumeState — github/jira/linear query included —
|
||||
// for paired clients on every subsequent write, not just this one.
|
||||
linearIssueView: LinearIssueViewResumeState.optional().catch(undefined),
|
||||
jiraPreset: z.enum(['assigned', 'reported', 'all', 'done']).optional(),
|
||||
jiraQuery: z.string().optional()
|
||||
})
|
||||
@@ -64,3 +67,11 @@ const _taskResumeStateParity: AssertNoMissingKeys<
|
||||
z.infer<typeof TaskResumeState>
|
||||
> = true
|
||||
void _taskResumeStateParity
|
||||
|
||||
// Why: the top-level assertion only compares TaskResumeState's own keys, so a field
|
||||
// added one level down stays invisible to it — and `.strict()` rejects exactly that.
|
||||
const _linearIssueViewParity: AssertNoMissingKeys<
|
||||
NonNullable<TaskResumeStateType['linearIssueView']>,
|
||||
z.infer<typeof LinearIssueViewResumeState>
|
||||
> = true
|
||||
void _linearIssueViewParity
|
||||
|
||||
@@ -231,7 +231,9 @@ import {
|
||||
isLinearIssueSearchActive,
|
||||
shouldClearTeamDerivedFacets,
|
||||
shouldForceLinearIssueListRead,
|
||||
shouldPersistLinearIssueView,
|
||||
teamDerivedFacetsForPrimaryTeamChange,
|
||||
type LinearIssueListFilterRead,
|
||||
type LinearPrimaryTeamObservation
|
||||
} from '@/components/task-page-linear-issue-request'
|
||||
import {
|
||||
@@ -4400,7 +4402,7 @@ export default function TaskPage(): React.JSX.Element {
|
||||
[linearAttributeFilterWorkspaceId, linearIssueFiltersByWorkspaceId]
|
||||
)
|
||||
// Why: null until the first list read, so a restored filter isn't mistaken for a change.
|
||||
const linearAttributeFilterSignatureRef = useRef<string | null>(null)
|
||||
const linearAttributeFilterReadRef = useRef<LinearIssueListFilterRead | null>(null)
|
||||
const linearPrimaryTeamRef = useRef<LinearPrimaryTeamObservation | null>(null)
|
||||
const [linearViewMode, setLinearViewMode] = useState<LinearViewMode>(DEFAULT_LINEAR_VIEW_MODE)
|
||||
const [linearGroupBy, setLinearGroupBy] = useState<LinearGroupBy>(DEFAULT_LINEAR_GROUP_BY)
|
||||
@@ -7413,12 +7415,11 @@ export default function TaskPage(): React.JSX.Element {
|
||||
// primary-team facet reset) persist too. The payload is workspace-independent,
|
||||
// so selecting a workspace mid-startup can't write a half-restored state.
|
||||
useEffect(() => {
|
||||
if (!taskResumeApplied) {
|
||||
return
|
||||
}
|
||||
if (!linearViewPersistReadyRef.current) {
|
||||
// Why: the first pass after hydration carries the restored values; echoing them back is a no-op at best and a default-state stomp at worst.
|
||||
const persistReady = linearViewPersistReadyRef.current
|
||||
if (taskResumeApplied) {
|
||||
linearViewPersistReadyRef.current = true
|
||||
}
|
||||
if (!shouldPersistLinearIssueView({ taskResumeApplied, persistReady })) {
|
||||
return
|
||||
}
|
||||
setTaskResumeState({
|
||||
@@ -7500,12 +7501,15 @@ export default function TaskPage(): React.JSX.Element {
|
||||
)
|
||||
}
|
||||
|
||||
const nextFilterSignature = linearIssueAttributeFilterSignature(linearAttributeFilter)
|
||||
const previousFilterSignature = linearAttributeFilterSignatureRef.current
|
||||
linearAttributeFilterSignatureRef.current = nextFilterSignature
|
||||
const nextFilterRead: LinearIssueListFilterRead = {
|
||||
workspaceId: linearAttributeFilterWorkspaceId,
|
||||
signature: linearIssueAttributeFilterSignature(linearAttributeFilter)
|
||||
}
|
||||
const previousFilterRead = linearAttributeFilterReadRef.current
|
||||
linearAttributeFilterReadRef.current = nextFilterRead
|
||||
const filterForce = shouldForceLinearIssueListRead({
|
||||
previousFilterSignature,
|
||||
nextFilterSignature,
|
||||
previousFilterRead,
|
||||
nextFilterRead,
|
||||
refreshForced: false
|
||||
})
|
||||
|
||||
@@ -9010,6 +9014,7 @@ export default function TaskPage(): React.JSX.Element {
|
||||
primaryTeam={linearAttributePrimaryTeam}
|
||||
selectedTeamIds={[...linearTeamSelection]}
|
||||
availableTeams={linearTeamOptions}
|
||||
teamsSettled={availableTeams.length > 0}
|
||||
settings={linearTaskSourceContext ?? settings}
|
||||
/>
|
||||
) : null}
|
||||
|
||||
@@ -99,6 +99,7 @@ describe('LinearIssueAttributeFilterDropdowns', () => {
|
||||
primaryTeam={team}
|
||||
selectedTeamIds={[]}
|
||||
availableTeams={[team]}
|
||||
teamsSettled
|
||||
/>
|
||||
)
|
||||
})
|
||||
@@ -130,6 +131,7 @@ describe('LinearIssueAttributeFilterDropdowns', () => {
|
||||
primaryTeam={team}
|
||||
selectedTeamIds={[]}
|
||||
availableTeams={[team]}
|
||||
teamsSettled
|
||||
/>
|
||||
)
|
||||
})
|
||||
@@ -163,6 +165,7 @@ describe('LinearIssueAttributeFilterDropdowns', () => {
|
||||
primaryTeam={team}
|
||||
selectedTeamIds={[]}
|
||||
availableTeams={[team]}
|
||||
teamsSettled
|
||||
/>
|
||||
)
|
||||
})
|
||||
|
||||
@@ -32,6 +32,8 @@ type Props = {
|
||||
/** Selected Linear team ids (All teams / multi-select). Empty → primary fallback. */
|
||||
selectedTeamIds: readonly string[]
|
||||
availableTeams: readonly LinearTeam[]
|
||||
/** False while `availableTeams` is still the issue-scraped fallback, not the real fetch. */
|
||||
teamsSettled: boolean
|
||||
settings?: RuntimeLinearSettings
|
||||
}
|
||||
|
||||
@@ -71,6 +73,7 @@ export default function LinearIssueAttributeFilterDropdowns({
|
||||
primaryTeam,
|
||||
selectedTeamIds,
|
||||
availableTeams,
|
||||
teamsSettled,
|
||||
settings
|
||||
}: Props): React.JSX.Element {
|
||||
const [popoverOpen, setPopoverOpen] = useState(false)
|
||||
@@ -111,6 +114,12 @@ export default function LinearIssueAttributeFilterDropdowns({
|
||||
if (activeTeamIds.length === 0 || !concreteWorkspaceId) {
|
||||
return
|
||||
}
|
||||
// Why: restored filters make this run at startup, when availableTeams may still be
|
||||
// the issue-scraped subset. Metadata complete for a partial team set looks valid,
|
||||
// so pruning there would permanently delete facets from another team (R12).
|
||||
if (!teamsSettled) {
|
||||
return
|
||||
}
|
||||
if (states.loading || labels.loading || members.loading) {
|
||||
return
|
||||
}
|
||||
@@ -143,6 +152,7 @@ export default function LinearIssueAttributeFilterDropdowns({
|
||||
}, [
|
||||
activeTeamIds,
|
||||
concreteWorkspaceId,
|
||||
teamsSettled,
|
||||
states.loading,
|
||||
states.error,
|
||||
states.data,
|
||||
|
||||
@@ -1,58 +0,0 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
// Why: TaskPage is too large to render in a unit test, but the persistence
|
||||
// contract lives in how its effects are wired, not in the pure module they call.
|
||||
const SOURCE = readFileSync(join(__dirname, 'TaskPage.tsx'), 'utf8')
|
||||
|
||||
const PERSIST_WRITE = 'linearIssueView: serializeLinearIssueViewResumeState('
|
||||
|
||||
function persistEffectSource(): string {
|
||||
const write = SOURCE.indexOf(PERSIST_WRITE)
|
||||
expect(write).toBeGreaterThan(0)
|
||||
const start = SOURCE.lastIndexOf('useEffect(() => {', write)
|
||||
const end = SOURCE.indexOf('])', write)
|
||||
expect(start).toBeGreaterThan(0)
|
||||
expect(end).toBeGreaterThan(write)
|
||||
return SOURCE.slice(start, end)
|
||||
}
|
||||
|
||||
function hydrationEffectSource(): string {
|
||||
const start = SOURCE.indexOf('if (taskResumeAppliedRef.current || !persistedUIReady')
|
||||
expect(start).toBeGreaterThan(0)
|
||||
const end = SOURCE.indexOf('taskResumeAppliedRef.current = true', start)
|
||||
expect(end).toBeGreaterThan(start)
|
||||
return SOURCE.slice(start, end)
|
||||
}
|
||||
|
||||
describe('Linear issue view resume wiring', () => {
|
||||
it('writes the persisted view from exactly one effect, gated until after hydration', () => {
|
||||
expect(SOURCE.match(/linearIssueView:/g)).toHaveLength(1)
|
||||
|
||||
const effect = persistEffectSource()
|
||||
expect(effect).toContain('if (!taskResumeApplied)')
|
||||
// Why: the first pass after hydration must be skipped or startup rewrites the restored state.
|
||||
expect(effect).toContain('linearViewPersistReadyRef.current = true')
|
||||
})
|
||||
|
||||
it('keeps the persisted payload independent of the selected workspace', () => {
|
||||
// Why: the payload carries every workspace's filter, so a workspace resolving
|
||||
// or switching mid-startup cannot make the write depend on effect ordering.
|
||||
expect(persistEffectSource()).not.toContain('selectedLinearWorkspaceId')
|
||||
})
|
||||
|
||||
it('restores every workspace filter during hydration', () => {
|
||||
expect(hydrationEffectSource()).toContain(
|
||||
'setLinearIssueFiltersByWorkspaceId(linearIssueView.filtersByWorkspaceId)'
|
||||
)
|
||||
})
|
||||
|
||||
it('derives the active filter from the selected workspace instead of resetting it', () => {
|
||||
expect(SOURCE).toContain('selectLinearWorkspaceIssueFilter(')
|
||||
// Why: a mutable previous-workspace ref plus a reset effect is exactly the
|
||||
// race this model replaces — a cold start would clear the restored filter.
|
||||
expect(SOURCE).not.toContain('previousLinearWorkspaceIdForFiltersRef')
|
||||
expect(SOURCE).not.toContain('setLinearAttributeFilter(')
|
||||
})
|
||||
})
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
isLinearIssueSearchActive,
|
||||
shouldClearTeamDerivedFacets,
|
||||
shouldForceLinearIssueListRead,
|
||||
shouldPersistLinearIssueView,
|
||||
teamDerivedFacetsForPrimaryTeamChange
|
||||
} from './task-page-linear-issue-request'
|
||||
import type { LinearIssueAttributeFilter } from '../../../shared/linear-issue-attribute-filter'
|
||||
@@ -50,18 +51,18 @@ describe('task-page-linear-issue-request', () => {
|
||||
expect(signature).toContain('"stateIds":["s1"]')
|
||||
})
|
||||
|
||||
it('forces a read when the filter signature changes', () => {
|
||||
it('forces a read when the filter signature changes within one workspace', () => {
|
||||
expect(
|
||||
shouldForceLinearIssueListRead({
|
||||
previousFilterSignature: 'a',
|
||||
nextFilterSignature: 'b',
|
||||
previousFilterRead: { workspaceId: 'ws-1', signature: 'a' },
|
||||
nextFilterRead: { workspaceId: 'ws-1', signature: 'b' },
|
||||
refreshForced: false
|
||||
})
|
||||
).toBe(true)
|
||||
expect(
|
||||
shouldForceLinearIssueListRead({
|
||||
previousFilterSignature: 'a',
|
||||
nextFilterSignature: 'a',
|
||||
previousFilterRead: { workspaceId: 'ws-1', signature: 'a' },
|
||||
nextFilterRead: { workspaceId: 'ws-1', signature: 'a' },
|
||||
refreshForced: false
|
||||
})
|
||||
).toBe(false)
|
||||
@@ -70,20 +71,46 @@ describe('task-page-linear-issue-request', () => {
|
||||
it('does not force the first read of a session, so a restored filter serves warm cache', () => {
|
||||
expect(
|
||||
shouldForceLinearIssueListRead({
|
||||
previousFilterSignature: null,
|
||||
nextFilterSignature: 'restored',
|
||||
previousFilterRead: null,
|
||||
nextFilterRead: { workspaceId: 'ws-1', signature: 'restored' },
|
||||
refreshForced: false
|
||||
})
|
||||
).toBe(false)
|
||||
expect(
|
||||
shouldForceLinearIssueListRead({
|
||||
previousFilterSignature: null,
|
||||
nextFilterSignature: 'restored',
|
||||
previousFilterRead: null,
|
||||
nextFilterRead: { workspaceId: 'ws-1', signature: 'restored' },
|
||||
refreshForced: true
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
// Why: the list cache is workspace-keyed, so B's warm entry is already correct —
|
||||
// forcing would mean a round trip behind a blocking spinner on every switch.
|
||||
it('does not force on a workspace switch, even when each workspace has its own filter', () => {
|
||||
expect(
|
||||
shouldForceLinearIssueListRead({
|
||||
previousFilterRead: { workspaceId: 'ws-1', signature: 'filter-a' },
|
||||
nextFilterRead: { workspaceId: 'ws-2', signature: 'filter-b' },
|
||||
refreshForced: false
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('persists the Linear view only after hydration has taken its first pass', () => {
|
||||
expect(shouldPersistLinearIssueView({ taskResumeApplied: false, persistReady: false })).toBe(
|
||||
false
|
||||
)
|
||||
// Why: the first pass after hydration carries the restored values, not a user change.
|
||||
expect(shouldPersistLinearIssueView({ taskResumeApplied: true, persistReady: false })).toBe(
|
||||
false
|
||||
)
|
||||
expect(shouldPersistLinearIssueView({ taskResumeApplied: true, persistReady: true })).toBe(true)
|
||||
expect(shouldPersistLinearIssueView({ taskResumeApplied: false, persistReady: true })).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('clears team-derived facets only when the primary team changes within one workspace', () => {
|
||||
expect(
|
||||
shouldClearTeamDerivedFacets({
|
||||
|
||||
@@ -54,10 +54,13 @@ export function buildLinearIssueListRequestSignature(options: {
|
||||
return `${sourceScope}::${workspace}::list::${options.filter ?? 'all'}::${options.limit}::${signature}`
|
||||
}
|
||||
|
||||
/** The last list read's filter, scoped to the workspace it was read for. */
|
||||
export type LinearIssueListFilterRead = { workspaceId: string | null; signature: string }
|
||||
|
||||
export function shouldForceLinearIssueListRead(options: {
|
||||
/** `null` until this session's first list read; a restored filter is a baseline, not a change. */
|
||||
previousFilterSignature: string | null
|
||||
nextFilterSignature: string
|
||||
previousFilterRead: LinearIssueListFilterRead | null
|
||||
nextFilterRead: LinearIssueListFilterRead
|
||||
refreshForced: boolean
|
||||
}): boolean {
|
||||
if (options.refreshForced) {
|
||||
@@ -65,12 +68,29 @@ export function shouldForceLinearIssueListRead(options: {
|
||||
}
|
||||
// Why: forcing the very first read would bypass warm cache and show the blocking
|
||||
// spinner on every cold start that restores a persisted filter.
|
||||
if (options.previousFilterSignature === null) {
|
||||
if (options.previousFilterRead === null) {
|
||||
return false
|
||||
}
|
||||
// Why: the list cache is keyed by workspace, so each workspace's filter is its own
|
||||
// baseline — switching restores a different filter, which is not a filter *change*.
|
||||
if (options.previousFilterRead.workspaceId !== options.nextFilterRead.workspaceId) {
|
||||
return false
|
||||
}
|
||||
// Why: filter signature changes always need a current server read, even when
|
||||
// returning to a previously cached signature that is still warm.
|
||||
return options.previousFilterSignature !== options.nextFilterSignature
|
||||
return options.previousFilterRead.signature !== options.nextFilterRead.signature
|
||||
}
|
||||
|
||||
/**
|
||||
* The persist effect must skip its first pass after hydration: that pass carries the
|
||||
* values just restored, so writing them back is a no-op at best and, before the
|
||||
* restore lands, a default-state stomp.
|
||||
*/
|
||||
export function shouldPersistLinearIssueView(options: {
|
||||
taskResumeApplied: boolean
|
||||
persistReady: boolean
|
||||
}): boolean {
|
||||
return options.taskResumeApplied && options.persistReady
|
||||
}
|
||||
|
||||
/** Where a primary team was observed; the team catalog is Linear-workspace scoped. */
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
// Why: TaskPage is too large to render in a unit test, so the decisions live in
|
||||
// task-page-linear-issue-request.ts where they are unit-tested directly. What is
|
||||
// left to pin here is that the effects delegate to them rather than re-deriving.
|
||||
const TASK_PAGE_SOURCE = readFileSync(join(__dirname, 'TaskPage.tsx'), 'utf8')
|
||||
|
||||
function sourceBetween(source: string, startPattern: string, endPattern: string): string {
|
||||
const start = source.indexOf(startPattern)
|
||||
expect(start).toBeGreaterThanOrEqual(0)
|
||||
const end = source.indexOf(endPattern, start + startPattern.length)
|
||||
expect(end).toBeGreaterThan(start)
|
||||
return source.slice(start, end)
|
||||
}
|
||||
|
||||
function persistEffectSource(): string {
|
||||
const write = TASK_PAGE_SOURCE.indexOf('linearIssueView: serializeLinearIssueViewResumeState(')
|
||||
expect(write).toBeGreaterThan(0)
|
||||
const start = TASK_PAGE_SOURCE.lastIndexOf('useEffect(() => {', write)
|
||||
expect(start).toBeGreaterThan(0)
|
||||
return TASK_PAGE_SOURCE.slice(start, TASK_PAGE_SOURCE.indexOf('])', write))
|
||||
}
|
||||
|
||||
describe('TaskPage Linear issue view persistence boundary', () => {
|
||||
it('writes the persisted view from exactly one effect, gated by the tested predicate', () => {
|
||||
expect(TASK_PAGE_SOURCE.match(/linearIssueView:/g)).toHaveLength(1)
|
||||
|
||||
const effect = persistEffectSource()
|
||||
// Why: the gate is `shouldPersistLinearIssueView` in task-page-linear-issue-request.ts,
|
||||
// which owns the hydration-first-pass skip; inlining it here would leave it untested.
|
||||
expect(effect).toContain('shouldPersistLinearIssueView({')
|
||||
expect(
|
||||
sourceBetween(effect, 'shouldPersistLinearIssueView({', 'setTaskResumeState(')
|
||||
).toContain('return')
|
||||
})
|
||||
|
||||
it('keeps the persisted payload independent of the selected workspace', () => {
|
||||
// Why: the payload carries every workspace's filter, so a workspace resolving
|
||||
// or switching mid-startup cannot make the write depend on effect ordering.
|
||||
expect(persistEffectSource()).not.toContain('selectedLinearWorkspaceId')
|
||||
})
|
||||
|
||||
it('restores every workspace filter during hydration', () => {
|
||||
const hydration = sourceBetween(
|
||||
TASK_PAGE_SOURCE,
|
||||
'if (taskResumeAppliedRef.current || !persistedUIReady',
|
||||
'taskResumeAppliedRef.current = true'
|
||||
)
|
||||
expect(hydration).toContain(
|
||||
'setLinearIssueFiltersByWorkspaceId(linearIssueView.filtersByWorkspaceId)'
|
||||
)
|
||||
})
|
||||
|
||||
it('derives the active filter from the selected workspace instead of resetting it', () => {
|
||||
expect(TASK_PAGE_SOURCE).toContain('selectLinearWorkspaceIssueFilter(')
|
||||
// Why: a mutable previous-workspace ref plus a reset effect is exactly the
|
||||
// race this model replaces — a cold start would clear the restored filter.
|
||||
expect(TASK_PAGE_SOURCE).not.toContain('previousLinearWorkspaceIdForFiltersRef')
|
||||
expect(TASK_PAGE_SOURCE).not.toContain('setLinearAttributeFilter(')
|
||||
})
|
||||
})
|
||||
@@ -98,6 +98,31 @@ export function canonicalizeLinearIssueAttributeFilter(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Enforces the transport bounds that `canonicalize` does not. Canonical form is
|
||||
* deduped but unbounded, so a filter built in memory can exceed limits that only
|
||||
* the throwing parser checks — and be rejected the moment it crosses a schema.
|
||||
*/
|
||||
export function boundLinearIssueAttributeFilter(
|
||||
filter: LinearIssueAttributeFilter
|
||||
): LinearIssueAttributeFilter {
|
||||
const withinLength = (id: string): boolean =>
|
||||
id.length > 0 && id.length <= LINEAR_ISSUE_ATTRIBUTE_FILTER_ID_MAX_LENGTH
|
||||
return {
|
||||
stateIds: filter.stateIds
|
||||
.filter(withinLength)
|
||||
.slice(0, LINEAR_ISSUE_ATTRIBUTE_FILTER_MAX_STATE_IDS),
|
||||
priorities: filter.priorities.slice(0, LINEAR_ISSUE_ATTRIBUTE_FILTER_MAX_PRIORITIES),
|
||||
assignee:
|
||||
filter.assignee?.kind === 'user' && !withinLength(filter.assignee.id)
|
||||
? null
|
||||
: filter.assignee,
|
||||
labelIds: filter.labelIds
|
||||
.filter(withinLength)
|
||||
.slice(0, LINEAR_ISSUE_ATTRIBUTE_FILTER_MAX_LABEL_IDS)
|
||||
}
|
||||
}
|
||||
|
||||
export function isEmptyLinearIssueAttributeFilter(
|
||||
filter: LinearIssueAttributeFilter | null | undefined
|
||||
): boolean {
|
||||
|
||||
@@ -133,6 +133,39 @@ describe('normalizeLinearIssueViewResumeState', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('isDefaultLinearIssueViewResumeState', () => {
|
||||
// Why: this is the sole reason normalize returns undefined, which is what actually
|
||||
// clears a persisted view when the user puts every setting back to its default.
|
||||
it('drops a serialized default view so reverting a setting clears what was stored', () => {
|
||||
expect(
|
||||
normalizeLinearIssueViewResumeState(
|
||||
serializeLinearIssueViewResumeState(defaultLinearIssueViewResumeState())
|
||||
)
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['viewMode', { viewMode: 'board' as const }],
|
||||
['groupBy', { groupBy: 'status' as const }],
|
||||
['orderBy', { orderBy: 'updated' as const }],
|
||||
['teamPropertyTouched', { teamPropertyTouched: true }],
|
||||
['displayProperties', { displayProperties: ['state' as const] }],
|
||||
[
|
||||
'filtersByWorkspaceId',
|
||||
{ filtersByWorkspaceId: { 'workspace-1': filter({ priorities: [1] }) } }
|
||||
]
|
||||
])('keeps a view that differs from the default in %s', (_label, overrides) => {
|
||||
expect(
|
||||
normalizeLinearIssueViewResumeState(
|
||||
serializeLinearIssueViewResumeState({
|
||||
...defaultLinearIssueViewResumeState(),
|
||||
...overrides
|
||||
})
|
||||
)
|
||||
).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('serializeLinearIssueViewResumeState', () => {
|
||||
it('round-trips a restored view unchanged', () => {
|
||||
const restored = resolveLinearIssueViewResumeState({
|
||||
@@ -217,6 +250,44 @@ describe('setLinearWorkspaceIssueFilter', () => {
|
||||
expect(selectLinearWorkspaceIssueFilter(filters, 'workspace-b')).toEqual(FILTER_B)
|
||||
})
|
||||
|
||||
// Why: re-filtering an existing workspace used to reassign the key in place, leaving
|
||||
// the workspace you just used at the head — first in line to be evicted.
|
||||
it('evicts the least recently used workspace, not the one just re-filtered', () => {
|
||||
let filters: Record<string, LinearIssueAttributeFilter> = {}
|
||||
for (let index = 0; index < LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES; index += 1) {
|
||||
filters = setLinearWorkspaceIssueFilter(
|
||||
filters,
|
||||
`workspace-${index}`,
|
||||
filter({ stateIds: [`state-${index}`] })
|
||||
)
|
||||
}
|
||||
filters = setLinearWorkspaceIssueFilter(filters, 'workspace-0', FILTER_A)
|
||||
filters = setLinearWorkspaceIssueFilter(filters, 'workspace-fresh', FILTER_B)
|
||||
|
||||
expect(Object.keys(filters)).toHaveLength(LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES)
|
||||
expect(selectLinearWorkspaceIssueFilter(filters, 'workspace-0')).toEqual(FILTER_A)
|
||||
expect(selectLinearWorkspaceIssueFilter(filters, 'workspace-1')).toEqual(
|
||||
emptyLinearIssueAttributeFilter()
|
||||
)
|
||||
})
|
||||
|
||||
// Why: array-index-like keys enumerate before string keys regardless of insertion
|
||||
// order, so key order alone would evict the entry this very write just added.
|
||||
it('keeps an integer-like workspace key that the write just added', () => {
|
||||
let filters: Record<string, LinearIssueAttributeFilter> = {}
|
||||
for (let index = 0; index < LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES; index += 1) {
|
||||
filters = setLinearWorkspaceIssueFilter(
|
||||
filters,
|
||||
`workspace-${index}`,
|
||||
filter({ stateIds: [`state-${index}`] })
|
||||
)
|
||||
}
|
||||
|
||||
filters = setLinearWorkspaceIssueFilter(filters, '42', FILTER_A)
|
||||
|
||||
expect(selectLinearWorkspaceIssueFilter(filters, '42')).toEqual(FILTER_A)
|
||||
})
|
||||
|
||||
it('returns the same record when the canonical filter is unchanged', () => {
|
||||
const filters = setLinearWorkspaceIssueFilter({}, 'workspace-a', FILTER_A)
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
// Persisted values are untrusted input: normalize before use, never assume shape.
|
||||
|
||||
import {
|
||||
boundLinearIssueAttributeFilter,
|
||||
canonicalizeLinearIssueAttributeFilter,
|
||||
emptyLinearIssueAttributeFilter,
|
||||
isEmptyLinearIssueAttributeFilter,
|
||||
@@ -96,6 +97,7 @@ function normalizeFiltersByWorkspaceId(value: unknown): Record<string, LinearIss
|
||||
return {}
|
||||
}
|
||||
const next: Record<string, LinearIssueAttributeFilter> = {}
|
||||
const order: string[] = []
|
||||
for (const [workspaceId, filter] of Object.entries(value)) {
|
||||
if (!isSafeWorkspaceKey(workspaceId)) {
|
||||
continue
|
||||
@@ -111,19 +113,31 @@ function normalizeFiltersByWorkspaceId(value: unknown): Record<string, LinearIss
|
||||
continue
|
||||
}
|
||||
next[workspaceId] = parsed
|
||||
order.push(workspaceId)
|
||||
}
|
||||
return trimToMostRecentWorkspaces(next)
|
||||
return trimToMostRecentWorkspaces(next, order)
|
||||
}
|
||||
|
||||
// Why: writes append newly filtered workspaces, so the tail is the most recent.
|
||||
/**
|
||||
* Keeps the newest workspaces, newest last. Recency is taken from `order` rather
|
||||
* than object key order: array-index-like keys enumerate first regardless of when
|
||||
* they were inserted, so `Object.keys` would evict the entry a write just added.
|
||||
*/
|
||||
function trimToMostRecentWorkspaces(
|
||||
filters: Record<string, LinearIssueAttributeFilter>
|
||||
filters: Record<string, LinearIssueAttributeFilter>,
|
||||
order: readonly string[]
|
||||
): Record<string, LinearIssueAttributeFilter> {
|
||||
const entries = Object.entries(filters)
|
||||
if (entries.length <= LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES) {
|
||||
const kept = order.filter((workspaceId) =>
|
||||
Object.prototype.hasOwnProperty.call(filters, workspaceId)
|
||||
)
|
||||
if (kept.length <= LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES) {
|
||||
return filters
|
||||
}
|
||||
return Object.fromEntries(entries.slice(-LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES))
|
||||
const next: Record<string, LinearIssueAttributeFilter> = {}
|
||||
for (const workspaceId of kept.slice(-LINEAR_ISSUE_VIEW_MAX_PERSISTED_WORKSPACES)) {
|
||||
next[workspaceId] = filters[workspaceId] as LinearIssueAttributeFilter
|
||||
}
|
||||
return next
|
||||
}
|
||||
|
||||
/** Validates untrusted persisted state; returns undefined when nothing worth restoring survives. */
|
||||
@@ -177,11 +191,18 @@ export function serializeLinearIssueViewResumeState(
|
||||
view: LinearIssueViewSelection
|
||||
): LinearIssueViewResumeState {
|
||||
const filtersByWorkspaceId: Record<string, LinearIssueAttributeFilter> = {}
|
||||
const order: string[] = []
|
||||
for (const [workspaceId, filter] of Object.entries(view.filtersByWorkspaceId)) {
|
||||
if (!isSafeWorkspaceKey(workspaceId) || isEmptyLinearIssueAttributeFilter(filter)) {
|
||||
continue
|
||||
}
|
||||
filtersByWorkspaceId[workspaceId] = canonicalizeLinearIssueAttributeFilter(filter)
|
||||
// Why: canonicalize dedupes but does not bound; only the throwing parser does, and
|
||||
// an over-length facet list here is rejected by `ui.set` and drops the WHOLE
|
||||
// taskResumeState for paired clients on every subsequent write.
|
||||
filtersByWorkspaceId[workspaceId] = boundLinearIssueAttributeFilter(
|
||||
canonicalizeLinearIssueAttributeFilter(filter)
|
||||
)
|
||||
order.push(workspaceId)
|
||||
}
|
||||
return {
|
||||
viewMode: view.viewMode,
|
||||
@@ -189,7 +210,7 @@ export function serializeLinearIssueViewResumeState(
|
||||
orderBy: view.orderBy,
|
||||
displayProperties: orderLinearDisplayProperties(view.displayProperties),
|
||||
teamPropertyTouched: view.teamPropertyTouched,
|
||||
filtersByWorkspaceId: trimToMostRecentWorkspaces(filtersByWorkspaceId)
|
||||
filtersByWorkspaceId: trimToMostRecentWorkspaces(filtersByWorkspaceId, order)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -228,10 +249,14 @@ export function setLinearWorkspaceIssueFilter(
|
||||
return filters
|
||||
}
|
||||
const next = { ...filters }
|
||||
delete next[workspaceId]
|
||||
if (isEmptyLinearIssueAttributeFilter(filter)) {
|
||||
delete next[workspaceId]
|
||||
return next
|
||||
}
|
||||
// Why: re-touching a workspace makes it the most recent, so it must be the last
|
||||
// to be evicted — not the first, which reassigning in place would leave it.
|
||||
next[workspaceId] = canonicalizeLinearIssueAttributeFilter(filter)
|
||||
return trimToMostRecentWorkspaces(next)
|
||||
const order = Object.keys(filters).filter((key) => key !== workspaceId)
|
||||
order.push(workspaceId)
|
||||
return trimToMostRecentWorkspaces(next, order)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user