-
Notifications
You must be signed in to change notification settings - Fork 616
feat(persistence): add draft store and tab state management #8519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
christian-byrne
merged 5 commits into
main
from
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
Feb 20, 2026
Merged
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e07f341
feat(persistence): add V2 store and tab state composable
christian-byrne 024cfe7
fix: address CodeRabbit review - cross-browser quota, validation, mov…
christian-byrne db4fa12
fix: handle persistIndex failure in quota-eviction path
christian-byrne 8bd86da
fix: skip target draft in eviction loop, guard legacy storage reads
christian-byrne 5bbe90a
fix: persist index before deleting old payload in moveDraft
christian-byrne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
src/platform/workflow/persistence/composables/useWorkflowTabState.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
|
|
||
| vi.mock('@/scripts/api', () => ({ | ||
| api: { | ||
| clientId: 'test-client-id', | ||
| initialClientId: 'test-client-id' | ||
| } | ||
| })) | ||
|
|
||
| describe('useWorkflowTabState', () => { | ||
| beforeEach(() => { | ||
| vi.resetModules() | ||
| sessionStorage.clear() | ||
| }) | ||
|
|
||
| describe('activePath', () => { | ||
| it('returns null when no pointer exists', async () => { | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { getActivePath } = useWorkflowTabState() | ||
|
|
||
| expect(getActivePath()).toBeNull() | ||
| }) | ||
|
|
||
| it('saves and retrieves active path', async () => { | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { getActivePath, setActivePath } = useWorkflowTabState() | ||
|
|
||
| setActivePath('workflows/test.json') | ||
| expect(getActivePath()).toBe('workflows/test.json') | ||
| }) | ||
|
|
||
| it('ignores pointer from different workspace', async () => { | ||
| sessionStorage.setItem( | ||
| 'Comfy.Workspace.Current', | ||
| JSON.stringify({ type: 'team', id: 'ws-1' }) | ||
| ) | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { setActivePath } = useWorkflowTabState() | ||
| setActivePath('workflows/test.json') | ||
|
|
||
| vi.resetModules() | ||
| sessionStorage.setItem( | ||
| 'Comfy.Workspace.Current', | ||
| JSON.stringify({ type: 'team', id: 'ws-2' }) | ||
| ) | ||
|
|
||
| const { useWorkflowTabState: useWorkflowTabState2 } = | ||
| await import('./useWorkflowTabState') | ||
| const { getActivePath } = useWorkflowTabState2() | ||
|
|
||
| expect(getActivePath()).toBeNull() | ||
| }) | ||
| }) | ||
|
|
||
| describe('openPaths', () => { | ||
| it('returns null when no pointer exists', async () => { | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { getOpenPaths } = useWorkflowTabState() | ||
|
|
||
| expect(getOpenPaths()).toBeNull() | ||
| }) | ||
|
|
||
| it('saves and retrieves open paths', async () => { | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { getOpenPaths, setOpenPaths } = useWorkflowTabState() | ||
|
|
||
| const paths = ['workflows/a.json', 'workflows/b.json'] | ||
| setOpenPaths(paths, 1) | ||
|
|
||
| const result = getOpenPaths() | ||
| expect(result).not.toBeNull() | ||
| expect(result!.paths).toEqual(paths) | ||
| expect(result!.activeIndex).toBe(1) | ||
| }) | ||
|
|
||
| it('ignores pointer from different workspace', async () => { | ||
| sessionStorage.setItem( | ||
| 'Comfy.Workspace.Current', | ||
| JSON.stringify({ type: 'team', id: 'ws-1' }) | ||
| ) | ||
| const { useWorkflowTabState } = await import('./useWorkflowTabState') | ||
| const { setOpenPaths } = useWorkflowTabState() | ||
| setOpenPaths(['workflows/test.json'], 0) | ||
|
|
||
| vi.resetModules() | ||
| sessionStorage.setItem( | ||
| 'Comfy.Workspace.Current', | ||
| JSON.stringify({ type: 'team', id: 'ws-2' }) | ||
| ) | ||
|
|
||
| const { useWorkflowTabState: useWorkflowTabState2 } = | ||
| await import('./useWorkflowTabState') | ||
| const { getOpenPaths } = useWorkflowTabState2() | ||
|
|
||
| expect(getOpenPaths()).toBeNull() | ||
| }) | ||
| }) | ||
| }) |
101 changes: 101 additions & 0 deletions
101
src/platform/workflow/persistence/composables/useWorkflowTabState.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| /** | ||
| * Tab State Management - Per-tab workflow pointers in sessionStorage. | ||
| * | ||
| * Uses api.clientId to scope pointers per browser tab. | ||
| * Includes workspaceId for validation to prevent cross-workspace contamination. | ||
| */ | ||
|
|
||
| import type { ActivePathPointer, OpenPathsPointer } from '../base/draftTypes' | ||
| import { getWorkspaceId } from '../base/storageKeys' | ||
| import { | ||
| readActivePath, | ||
| readOpenPaths, | ||
| writeActivePath, | ||
| writeOpenPaths | ||
| } from '../base/storageIO' | ||
| import { api } from '@/scripts/api' | ||
|
|
||
| /** | ||
| * Gets the current client ID for browser tab identification. | ||
| * Falls back to initialClientId if clientId is not yet set. | ||
| */ | ||
| function getClientId(): string | null { | ||
| return api.clientId ?? api.initialClientId ?? null | ||
| } | ||
|
|
||
| /** | ||
| * Composable for managing per-tab workflow state in sessionStorage. | ||
| */ | ||
| export function useWorkflowTabState() { | ||
| const currentWorkspaceId = getWorkspaceId() | ||
|
|
||
| /** | ||
| * Gets the active workflow path for the current tab. | ||
| * Returns null if no pointer exists or workspaceId doesn't match. | ||
| */ | ||
| function getActivePath(): string | null { | ||
| const clientId = getClientId() | ||
| if (!clientId) return null | ||
|
|
||
| const pointer = readActivePath(clientId) | ||
| if (!pointer) return null | ||
|
|
||
| // Validate workspace - ignore stale pointers from different workspace | ||
| if (pointer.workspaceId !== currentWorkspaceId) return null | ||
|
|
||
| return pointer.path | ||
| } | ||
|
|
||
| /** | ||
| * Sets the active workflow path for the current tab. | ||
| */ | ||
| function setActivePath(path: string): void { | ||
| const clientId = getClientId() | ||
| if (!clientId) return | ||
|
|
||
| const pointer: ActivePathPointer = { | ||
| workspaceId: currentWorkspaceId, | ||
| path | ||
| } | ||
| writeActivePath(clientId, pointer) | ||
| } | ||
|
|
||
| /** | ||
| * Gets the open workflow paths for the current tab. | ||
| * Returns null if no pointer exists or workspaceId doesn't match. | ||
| */ | ||
| function getOpenPaths(): { paths: string[]; activeIndex: number } | null { | ||
| const clientId = getClientId() | ||
| if (!clientId) return null | ||
|
|
||
| const pointer = readOpenPaths(clientId) | ||
| if (!pointer) return null | ||
|
|
||
| // Validate workspace | ||
| if (pointer.workspaceId !== currentWorkspaceId) return null | ||
|
|
||
| return { paths: pointer.paths, activeIndex: pointer.activeIndex } | ||
| } | ||
|
|
||
| /** | ||
| * Sets the open workflow paths for the current tab. | ||
| */ | ||
| function setOpenPaths(paths: string[], activeIndex: number): void { | ||
| const clientId = getClientId() | ||
| if (!clientId) return | ||
|
|
||
| const pointer: OpenPathsPointer = { | ||
| workspaceId: currentWorkspaceId, | ||
| paths, | ||
| activeIndex | ||
| } | ||
| writeOpenPaths(clientId, pointer) | ||
| } | ||
|
|
||
| return { | ||
| getActivePath, | ||
| setActivePath, | ||
| getOpenPaths, | ||
| setOpenPaths | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n src/platform/workflow/persistence/base/storageIO.ts | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3156
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3905
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2740
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1661
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1126
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1001
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4448
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1481
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3805
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1166
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3667
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1825
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 168
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1255
Validate parsed storage data before returning.
localStorage/sessionStorage values are user-modifiable; returning parsed JSON without shape validation risks propagating corrupt data. Add type guards or Zod schemas for all four functions:
readIndex: Only checksv === 2, lacks validation forupdatedAt(number),order(string[]), andentries(object with DraftEntryMeta values)readPayload: No validation; directly casts parsed JSON as DraftPayloadV2readActivePathandreadOpenPaths: No validation in readers (though call sites in useWorkflowTabState.ts validateworkspaceId, the data shape itself is unchecked)Zod and zod-validation-error are available in the project. Use type guards or Zod schemas to ensure shape correctness before returning.
Applies to lines 31–46 (readIndex), 69–84 (readPayload), 175–185 (readActivePath), and 205–215 (readOpenPaths).
Per coding guidelines: "Validate trusted sources before processing".
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed — added
isValidIndextype guard that validatesv === 2,updatedAtis a number,orderis an array, andentriesis a non-null object.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 6193
@christian-byrne, thanks for adding theisValidIndextype guard—readIndexis now properly validated! 👍However, the original review flagged four functions that need validation. The remaining three still cast parsed JSON without validation:
readPayload(line 98): Should validate thatdatais a string andupdatedAtis a number before returning asDraftPayloadV2readActivePath(line 197): Should validate thatworkspaceIdandpathare strings before returning asActivePathPointerreadOpenPaths(line 227): Should validate thatworkspaceIdis a string,pathsis a string array, andactiveIndexis a number before returning asOpenPathsPointerYou can follow the same pattern as
isValidIndex—create type guards likeisValidPayload,isValidActivePath, andisValidOpenPathsto ensure malformed storage data doesn't propagate through the system.🧬 Code Graph Analysis Results
🧠 Learnings used