mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-06-08 15:29:52 +00:00
8206022982
*PR Created by the Glary-Bot Agent*
---
## Summary
Fix FE-559: browser forward/back to a deleted subgraph used to leave the
canvas on stale state (and sometimes triggered unrelated tab navigation)
because the subgraph id in the URL hash was looked up with no validation
or fallback.
## Changes
- **What**:
- Added `src/schemas/subgraphIdSchema.ts` — `zSubgraphId =
z.string().uuid()` + `isValidSubgraphId(value)` type guard, matching how
subgraph ids are persisted in `workflowSchema.ts` and generated by
`createUuidv4()`.
- `subgraphNavigationStore.navigateToHash()` now (a) validates the hash
with `isValidSubgraphId` before any lookup, (b) redirects to the root
graph (`router.replace('#' + root.id)` + `canvas.setGraph(root)`) when
the locator is malformed, missing from `root.subgraphs`, or still
unresolved after a workflow-load attempt.
- Replaced the `console.error('subgraph poofed after load?')` dead-end
with the same redirect helper.
- Re-ordered the "already on this graph" short-circuit so a stale canvas
reference to a now-deleted subgraph doesn't suppress the redirect.
## Review Focus
- TDD: 6 new tests in `subgraphNavigationStore.navigateToHash.test.ts`
cover valid navigation, deleted-subgraph hash, malformed (non-UUID)
hash, no-op when target equals current, empty-hash root case, and
stale-canvas recovery. 15 new tests in `subgraphIdSchema.test.ts` lock
down the validator.
- `redirectToRoot()` toggles `blockHashUpdate` while calling
`router.replace`, so the new redirect doesn't re-trigger `updateHash()`
and clobber the canvas state.
- Generalized validation: the new schema lives in `src/schemas/` and can
be reused anywhere a subgraph id crosses an untrusted boundary (URL,
IPC, etc.).
## Manual Verification
Ran ComfyUI backend (`--cpu --port 8188`) + frontend dev server, then
drove Playwright through three scenarios:
| Input hash | Result | Console |
|---|---|---|
| `#11111111-2222-4333-8444-555555555555` (UUID-shaped, non-existent) |
URL replaced with `#<root-id>` | `[subgraphNavigation] subgraph not
found: 11111111-…; redirecting to root graph` |
| `#not-a-valid-uuid` (malformed) | URL replaced with `#<root-id>` |
`[subgraphNavigation] invalid subgraph id in hash: not-a-valid-uuid;
redirecting to root graph` |
| `#aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee` (UUID-shaped, non-existent) |
URL replaced with `#<root-id>` | (same redirect message) |
Screenshot below shows the redirected viewport.
Fixes FE-559
## Screenshots
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12169-fix-subgraph-validate-URL-hash-and-redirect-to-root-when-subgraph-missing-35e6d73d3650819f840af1475b9f44d4)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
88 lines
2.5 KiB
TypeScript
88 lines
2.5 KiB
TypeScript
import type { Page } from '@playwright/test'
|
|
import { expect } from '@playwright/test'
|
|
|
|
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
|
|
|
|
async function waitForRootCanvasReady(page: Page) {
|
|
await expect
|
|
.poll(async () => {
|
|
const state = await page.evaluate(() => ({
|
|
rootId: window.app?.rootGraph?.id ?? '',
|
|
canvasGraphId: window.app?.canvas?.graph?.id ?? ''
|
|
}))
|
|
return state.rootId !== '' && state.canvasGraphId === state.rootId
|
|
})
|
|
.toBe(true)
|
|
}
|
|
|
|
async function expectCanvasOnRootGraph(page: Page) {
|
|
await expect
|
|
.poll(async () =>
|
|
page.evaluate(() => ({
|
|
rootId: window.app!.rootGraph.id,
|
|
canvasGraphId: window.app!.canvas.graph?.id,
|
|
hash: window.location.hash
|
|
}))
|
|
)
|
|
.toEqual({
|
|
rootId: expect.any(String),
|
|
canvasGraphId: expect.stringMatching(/.+/),
|
|
hash: expect.stringMatching(/^#.+/)
|
|
})
|
|
const state = await page.evaluate(() => ({
|
|
rootId: window.app!.rootGraph.id,
|
|
canvasGraphId: window.app!.canvas.graph?.id,
|
|
hash: window.location.hash
|
|
}))
|
|
expect(state.canvasGraphId).toBe(state.rootId)
|
|
expect(state.hash).toBe(`#${state.rootId}`)
|
|
}
|
|
|
|
test.describe(
|
|
'Subgraph hash validation (FE-559)',
|
|
{ tag: ['@subgraph'] },
|
|
() => {
|
|
test('redirects URL and canvas to root for a non-existent subgraph hash', async ({
|
|
comfyPage
|
|
}) => {
|
|
await waitForRootCanvasReady(comfyPage.page)
|
|
const rootId = await comfyPage.page.evaluate(
|
|
() => window.app!.rootGraph.id
|
|
)
|
|
const phantomId = '11111111-1111-4111-8111-111111111111'
|
|
expect(phantomId).not.toBe(rootId)
|
|
|
|
await comfyPage.page.evaluate((hash) => {
|
|
window.location.hash = hash
|
|
}, `#${phantomId}`)
|
|
|
|
await expect
|
|
.poll(() => comfyPage.page.evaluate(() => window.location.hash), {
|
|
timeout: 5000
|
|
})
|
|
.toBe(`#${rootId}`)
|
|
await expectCanvasOnRootGraph(comfyPage.page)
|
|
})
|
|
|
|
test('redirects URL and canvas to root when hash is malformed', async ({
|
|
comfyPage
|
|
}) => {
|
|
await waitForRootCanvasReady(comfyPage.page)
|
|
const rootId = await comfyPage.page.evaluate(
|
|
() => window.app!.rootGraph.id
|
|
)
|
|
|
|
await comfyPage.page.evaluate(() => {
|
|
window.location.hash = '#not-a-valid-uuid'
|
|
})
|
|
|
|
await expect
|
|
.poll(() => comfyPage.page.evaluate(() => window.location.hash), {
|
|
timeout: 5000
|
|
})
|
|
.toBe(`#${rootId}`)
|
|
await expectCanvasOnRootGraph(comfyPage.page)
|
|
})
|
|
}
|
|
)
|