From 0aefea7ce3845c2a43cae6c9bf3f42da7e8fe6b6 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Mon, 7 Sep 2026 18:59:33 +0200 Subject: [PATCH] fix(frontend): make the archived route independent of write permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three ways the archived-only placeholder failed to deliver what it promised: Reading archived items is not a write, but the notice offering them sat behind the create-permission gate — so an operator, or a workspace whose direct-deploy protection cleared `showEditButtons`, got "no items found" over items it could see and a toolbar now inert. The gate governs the create actions alone; the notice is shown to whoever the probe found something for. The probe answered once per workspace and was never invalidated, so archiving the last item left a cached "nothing archived" claiming the workspace was empty until a page load. `reloadItemsAndCounts` clears it. And it omitted `includeWithoutMain`, which the backend reads as excluding library scripts — a workspace holding only archived ones answered "empty". Always true here: hiding library scripts puts a filter in `activeFilters`, which `workspaceEmpty` requires to be empty. `whenIdle()` gains the two tests its contract deserves, since the reload correctness three rounds argued over rests on it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012fRjnaHLwjpHN84gNNxah9 --- .../src/lib/components/home/ItemsList.svelte | 46 ++++++--- .../home/WorkspaceEmptyState.svelte | 94 ++++++++++--------- frontend/src/lib/importWizard/abandon.test.ts | 24 +++++ 3 files changed, 110 insertions(+), 54 deletions(-) diff --git a/frontend/src/lib/components/home/ItemsList.svelte b/frontend/src/lib/components/home/ItemsList.svelte index c2ed62ff26..25a30b2bd8 100644 --- a/frontend/src/lib/components/home/ItemsList.svelte +++ b/frontend/src/lib/components/home/ItemsList.svelte @@ -700,6 +700,10 @@ // runnables an owner holds. A scope change (sort/archive/kind/…) doesn't go // through here: the counts resource keys on those itself. async function reloadItemsAndCounts(): Promise { + // The answer can change with the rows: archiving the last item leaves the listing empty + // with something archived behind it, and a cached "nothing archived" would then call + // the workspace empty and hide the way to it until a page load. + archivedProbe = undefined // A mutated row can be gone, or sit at a new path, afterwards: snapshot what // was on screen so the selection can drop what this reload removes instead of // keeping a dead path. `tick` lets the reloaded rows re-register first. @@ -1023,7 +1027,16 @@ }) async function probeArchived(workspace: string) { try { - const res = await ScriptService.listRunnables({ workspace, showArchived: true, perPage: 1 }) + // `includeWithoutMain` to match the listing: the backend drops `auto_kind = 'lib'` + // without it, so a workspace holding only archived library scripts would answer + // "nothing archived". Always true here — hiding library scripts puts a filter in + // `activeFilters`, which `workspaceEmpty` requires to be empty. + const res = await ScriptService.listRunnables({ + workspace, + showArchived: true, + includeWithoutMain: true, + perPage: 1 + }) archivedProbe = { workspace, hasArchived: (res.items?.length ?? 0) > 0 } } catch (error) { console.error('Could not check for archived items:', error) @@ -1031,6 +1044,13 @@ } } let emptyStateAnswered = $derived(archivedProbe?.workspace === $workspaceStore) + /** + * Whether this user may be offered the create actions. The empty state's template import + * and create menu do no permission check of their own, so an operator — or a workspace + * whose direct-deploy protection cleared `showEditButtons` — must not be shown them. + * Reading archived items is not a write, so it is not gated on this. + */ + let canCreateHere = $derived(!$userStore?.operator && showEditButtons) // The workspace itself holds nothing — no filter is narrowing the list away. It stays // false until the first load resolves: a skeleton already means "loading", and the @@ -1894,20 +1914,24 @@ - - {#if workspaceEmpty && !$userStore?.operator && showEditButtons} + {#if workspaceEmpty} {#if emptyStateAnswered} - (hubPick = project)} - onShowArchived={() => (filterValues.val = { ...filterValues.val, archived: true })} - /> + {#if archivedProbe?.hasArchived || canCreateHere} + + (hubPick = project)} + onShowArchived={() => (filterValues.val = { ...filterValues.val, archived: true })} + /> + {:else} + + {/if} {/if} {:else} diff --git a/frontend/src/lib/components/home/WorkspaceEmptyState.svelte b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte index 7513c659d3..50a29a7714 100644 --- a/frontend/src/lib/components/home/WorkspaceEmptyState.svelte +++ b/frontend/src/lib/components/home/WorkspaceEmptyState.svelte @@ -17,9 +17,15 @@ archivedOnly?: boolean /** Switches the list to the archived view. */ onShowArchived: () => void + /** + * Whether to offer the template import and the create menu. Neither checks permissions + * itself, so an operator gets the state described without the two actions it may not + * take — the archived link stays, since reading archived items is not a write. + */ + canCreate?: boolean } - let { onPick, archivedOnly = false, onShowArchived }: Props = $props() + let { onPick, archivedOnly = false, onShowArchived, canCreate = true }: Props = $props() // Row opacities: the list fading out of existence. Static on purpose — motion is what // makes a skeleton mean "loading", and this state means "empty". @@ -81,56 +87,58 @@ {:else} Your scripts, flows and apps will show up here. {/if} - - {#if !$disableHubStore} - - - e.detail && logFeatureUsage('home', 'template_picker_open', { key: 'empty_state' })} - > - {#snippet trigger()}Start from a template{/snippet} - {#snippet content({ close })} - { - close() - onPick(project) - }} - /> - {/snippet} - - or - {/if} - - {#snippet trigger()} - - . - {/snippet} - + . + {/snippet} + + {/if} diff --git a/frontend/src/lib/importWizard/abandon.test.ts b/frontend/src/lib/importWizard/abandon.test.ts index abf0ff61b9..d240c8147d 100644 --- a/frontend/src/lib/importWizard/abandon.test.ts +++ b/frontend/src/lib/importWizard/abandon.test.ts @@ -150,6 +150,30 @@ describe('abandoning mid-import', () => { expect(run.itemResults.length).toBe(3) }) + // What a caller acting on the run's leftovers depends on: `abandon()` only stops the next + // phase, so a reload issued when it is called reads the workspace while the request already + // sent is still landing. `whenIdle()` is the difference between reloading then and after. + it('whenIdle resolves only once the abandoned run has stopped writing', async () => { + const run = new ImportExecution(PLAN, deps) + let idleResolved = false + hooks.afterFirstItem = () => { + run.abandon() + void run.whenIdle().then(() => (idleResolved = true)) + // Still inside the run: the promise must not have resolved yet. + expect(run.running).toBe(true) + expect(idleResolved).toBe(false) + } + await run.run() + await run.whenIdle() + expect(run.running).toBe(false) + expect(idleResolved).toBe(true) + }) + + it('whenIdle resolves immediately when no run is in flight', async () => { + const run = new ImportExecution(PLAN, deps) + await expect(run.whenIdle()).resolves.toBeUndefined() + }) + it('stops the migrate row spinning when it is abandoned mid-migration', async () => { const run = new ImportExecution(PLAN, depsWithMigration) // After `onMigrationsStart`, which is where the row is actually set to running —