starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-13 08:02:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: read the cell at the resolved path, and clean up by it after a deploy

Browse Source 
Review round 6, all three at the seam the reshape moved:

- A draft that exists only as a local cell — a second session tab, or an
  editor with autosave off — was invisible by its chosen name: resolution
  found it, then the callers looked for a cell at the literal path and fell
  through to the deployed item. The write merge then rebuilt the draft from
  that deployed item. Both reads now look at the resolved path.
- Deploying an app by its chosen name creates the app at that name, which then
  wins the name back, so the cleanup deleted nothing and the draft stayed
  listed. The cleanup uses the path resolved before the deploy.
- Discarding a draft addressed by its chosen name cleared the modified-items
  entry under the name while writes record it under the storage path, leaving
  the item marked as changed. The discard resolves once and uses that.
- A listing failure no longer fails a read: only a write, which is the call
  that would create a second draft by trusting the path it was given.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
2026-09-23 14:21:04 +02:00
co-authored by Claude Opus 5
parent 80f23238d9
commit 60fc0f4f9c
3 changed files with 84 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/components/copilot/chat/global/core.test.ts
+52
View File
@@ -2538,6 +2538,58 @@ describe('global AI tools', () => {
}
})
// A draft can exist only as a local cell — a second session tab, or an editor with
// autosave off — with no backend row to fall back to.
it('reads a cell-only draft by its chosen name', async () => {
UserDraft.save(
'raw_app',
'u/admin/draft_cell_only',
{ summary: 'Cell only', draft_path: 'f/sales/cell_app', files: {}, runnables: {} },
{ workspace: WORKSPACE }
)
const read = JSON.parse(
await callGlobalTool('read_workspace_item', { type: 'app', path: 'f/sales/cell_app' })
)
expect(read).toMatchObject({ path: 'u/admin/draft_cell_only', summary: 'Cell only' })
})
// Deploying by the chosen name creates the app at that name, which would then win the
// name back and leave the draft the deploy came from listed forever.
it('removes the source draft of an app deployed by its chosen name', async () => {
const storageKey = 'u/admin/draft_deployed_by_name'
seedBackendDraft(
'raw_app',
storageKey,
{
summary: 'By name',
draft_path: 'f/sales/by_name',
files: { '/App.tsx': 'export default () => null' },
runnables: {},
data: { tables: [] }
},
{ workspace: WORKSPACE }
)
const deployed = new Set<string>()
vi.mocked(AppService.existsApp).mockImplementation(async ({ path }) => deployed.has(path))
vi.mocked(AppService.createAppRaw).mockImplementation(async ({ formData }: any) => {
deployed.add(formData.app.path)
return 1 as any
})
vi.mocked(AppService.getAppByPath).mockResolvedValueOnce({
draft: { draft_path: 'f/sales/by_name' }
} as any)
try {
await callGlobalTool('deploy_workspace_item', { type: 'app', path: 'f/sales/by_name' })
expect(deployed.has('f/sales/by_name')).toBe(true)
expect(getBackendDraft('raw_app', storageKey, { workspace: WORKSPACE })).toBeUndefined()
} finally {
vi.mocked(AppService.existsApp).mockImplementation(async () => false)
vi.mocked(AppService.createAppRaw).mockImplementation(async () => 1 as any)
}
})
it('refuses a draft_path that two drafts are staged under', async () => {
for (const storage of ['u/admin/draft_a', 'u/admin/draft_b']) {
seedBackendDraft(
frontend/src/lib/components/copilot/chat/global/core.ts
+10 -6
View File
@@ -6834,16 +6834,16 @@ async function discardLocalDraft(
throw new Error(`No draft found for ${type} "${path}".`)
}
await deleteGlobalDraft(workspace, type, path, triggerKind)
// Resolved once: the mask records a draft under the path it is stored at, so clearing
// it by the name the call used would leave the entry behind.
const storagePath = await resolveGlobalDraftStoragePath(workspace, type, path, triggerKind)
await deleteGlobalDraft(workspace, type, storagePath, triggerKind)
// The chat's touch on the item is undone — drop it from the mask so a
// pre-existing deployed item doesn't keep reading as this chat's edit.
const discardedKind = itemKindFor(type, triggerKind)
if (discardedKind) {
toolCallbacks.onItemDiscarded?.(
discardedKind,
liveGlobalDraftStoragePath(workspace, type, path, triggerKind)
)
toolCallbacks.onItemDiscarded?.(discardedKind, storagePath)
}
toolCallbacks.setToolStatus(toolId, {
@@ -8274,7 +8274,11 @@ async function deployDraft(
// fork comparisons before the fallible draft cleanup below.
invalidateWorkspaceComparison(workspace)
await deleteGlobalDraft(workspace, type, path, triggerKind, { preserveLiveDraft: true })
// By the path resolved before the deploy: the app now deployed at the draft's chosen
// name would otherwise win the name back, and the draft would be left behind.
await deleteGlobalDraft(workspace, type, draftStoragePath, triggerKind, {
preserveLiveDraft: true
})
// Move the chat's mask entry to the deployed path: a draft-only item's
// synthetic storage key never exists deployed, so the entry would otherwise
frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts
+22 -10
View File
@@ -336,7 +336,10 @@ type ResolvedDraft = { storagePath: string; value?: unknown }
async function resolveDraft(
workspace: string,
itemKind: UserDraftItemKind,
path: string
path: string,
/** A write must know which draft a name belongs to; a read can fall back to the path
* itself, which is what it addressed before names were resolved at all. */
opts: { forWrite?: boolean } = {}
): Promise<ResolvedDraft> {
const live = liveEditorStoragePath(workspace, itemKind, path)
if (live !== path) return { storagePath: live }
@@ -345,13 +348,14 @@ async function resolveDraft(
if (UserDraft.get(itemKind, path, { workspace }) !== undefined) return { storagePath: path }
const own = await fetchBackendDraftValue(workspace, itemKind, path)
if (own !== undefined) return { storagePath: path, value: own }
return { storagePath: await storagePathForChosenName(workspace, itemKind, path) }
return { storagePath: await storagePathForChosenName(workspace, itemKind, path, opts) }
}
async function storagePathForChosenName(
workspace: string,
itemKind: UserDraftItemKind,
path: string
path: string,
opts: { forWrite?: boolean }
): Promise<string> {
const staged: StagedDraft[] = []
const add = (storagePath: string, summary: string | undefined) => {
@@ -365,7 +369,9 @@ async function storagePathForChosenName(
rows = await DraftService.listDrafts({ workspace })
} catch (e) {
// Nothing is stored at this path, so it may be a name. Without the listing that
// cannot be told, and guessing the path itself would write a second draft.
// cannot be told, and a write that guessed the path itself would create a second
// draft; a read of whatever is deployed there is harmless.
if (!opts.forWrite) return path
throw new Error(
`Could not load this workspace's drafts, so "${path}" cannot be matched to the draft ` +
`it may name: ${e instanceof Error ? e.message : String(e)}. Try again.`
@@ -494,11 +500,11 @@ export async function readGlobalDraftValue<V>(
): Promise<V | undefined> {
const itemKind = itemKindFor(type, triggerKind)
if (!itemKind) return undefined
const cell = UserDraft.get<V>(itemKind, liveEditorStoragePath(workspace, itemKind, path), {
workspace
})
if (cell !== undefined) return cell
const resolved = await resolveDraft(workspace, itemKind, path)
// The cell at the RESOLVED path: it is the freshest state when a save is parked, failed
// or conflicted, and a draft that exists only as a cell has no backend row behind it.
const cell = UserDraft.get<V>(itemKind, resolved.storagePath, { workspace })
if (cell !== undefined) return cell
if (resolved.value !== undefined) return resolved.value as V
return (await fetchBackendDraftValue(workspace, itemKind, resolved.storagePath)) as V | undefined
}
@@ -538,7 +544,8 @@ export async function persistGlobalDraft(
if (!itemKind) throw new Error(`Unsupported draft type "${type}".`)
// Resolved rather than taken as given: a write under a chosen name that did not resolve
// would create a second draft beside the one it meant to edit.
const storagePath = (await resolveDraft(workspace, itemKind, path)).storagePath
const storagePath = (await resolveDraft(workspace, itemKind, path, { forWrite: true }))
.storagePath
UserDraft.seed(itemKind, storagePath, value, { workspace })
await UserDraftDbSyncer.save({
workspace,
@@ -596,7 +603,12 @@ export async function getGlobalDraft(
if (!itemKind) return undefined
const resolved = await resolveDraft(workspace, itemKind, path)
const storagePath = resolved.storagePath
const value = resolved.value ?? (await fetchBackendDraftValue(workspace, itemKind, storagePath))
// A draft can live only as a local cell — a second session tab, or an editor with
// autosave off — with no backend row behind it.
const value =
UserDraft.get(itemKind, storagePath, { workspace }) ??
resolved.value ??
(await fetchBackendDraftValue(workspace, itemKind, storagePath))
if (value === undefined || value === null) return undefined
const { displayPath, isLiveDraft } = liveDisplayPath(workspace, itemKind, storagePath)
return userDraftEntryToWorkspaceItem(