mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-21 08:02:26 +00:00
fix(sessions): scope fork session Edits to session-edited items only (#9989)
* fix(sessions): scope fork session Edits to session-edited items only A session chat with an undefined modified-items mask fell back to showing every draft in its (possibly forked) workspace, so the Edits bar/diff drawer listed all fork drafts instead of just what the session edited. Always track session chats: seed an empty mask for legacy chats in loadPastChat and guard the not-yet-persisted-chat case in initRuntime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: clarify session chats always persist their modified-items mask Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -367,10 +367,11 @@ export class AIChatManager {
|
||||
|
||||
// Workspace items the CURRENT chat modified via AI tool calls, as
|
||||
// `${UserDraftItemKind}:${storagePath}` keys (see modifiedItemsMask.ts).
|
||||
// undefined = untracked: the global side-panel chat (never initialised) and
|
||||
// loaded legacy chats with no stored mask, both of which fall back to the
|
||||
// show-all bar. A SvelteSet (even empty) = tracked. Reactive so the session
|
||||
// bar updates as tools record mid-turn.
|
||||
// undefined = untracked: only the global side-panel chat (never initialised),
|
||||
// which falls back to the show-all bar. Session chats are always tracked (a
|
||||
// SvelteSet, even empty) — see loadPastChat/initRuntime — so their Edits
|
||||
// surface never claims drafts the session didn't touch. Reactive so the
|
||||
// session bar updates as tools record mid-turn.
|
||||
modifiedItems = $state<SvelteSet<string> | undefined>(undefined)
|
||||
|
||||
// Start tracking for a brand-new session chat (empty = "tracked, nothing yet").
|
||||
@@ -2159,13 +2160,14 @@ export class AIChatManager {
|
||||
this.displayMessages = chat.displayMessages
|
||||
this.messages = chat.actualMessages
|
||||
this.contextUsage = normalizeContextUsage(chat.contextUsage)
|
||||
// Seed the modified-items mask from the stored chat. A stored array
|
||||
// (even empty) → tracked; a legacy chat with no field stays untracked
|
||||
// (undefined) so the session bar falls back to showing all drafts. The
|
||||
// global side-panel chat never tracks, so leave it untouched there.
|
||||
// Seed the modified-items mask from the stored chat. A session's Edits
|
||||
// surface is scoped strictly to what this session edited, so it must never
|
||||
// fall back to showing every draft in the (possibly forked) workspace: a
|
||||
// legacy chat with no stored mask seeds an empty tracked set, not undefined.
|
||||
// The global side-panel chat never tracks, so leave it untouched there.
|
||||
if (this.isSessionChat) {
|
||||
const stored = this.historyManager.getModifiedItems(id)
|
||||
this.modifiedItems = stored !== undefined ? new SvelteSet(stored) : undefined
|
||||
this.modifiedItems = new SvelteSet(stored ?? [])
|
||||
}
|
||||
this.#automaticScroll = true
|
||||
this.onChatRotated?.(id)
|
||||
|
||||
@@ -634,6 +634,47 @@ describe('AIChatManager queued messages', () => {
|
||||
await session.saveAndClear()
|
||||
expect(session.attachedFiles.count).toBe(1)
|
||||
})
|
||||
|
||||
it('tracks (empty mask) a session chat loaded with no stored modified-items', async () => {
|
||||
// A legacy session chat has no persisted mask. It must NOT stay untracked
|
||||
// (undefined) — that makes the Edits surface fall back to showing every
|
||||
// draft in the (possibly forked) workspace. Seed an empty tracked set so the
|
||||
// session only ever surfaces what it actually edited.
|
||||
const manager = createManager(createInputMock())
|
||||
manager.isSessionChat = true
|
||||
vi.spyOn(manager.historyManager, 'loadPastChat').mockReturnValue({
|
||||
id: 'legacy-session-chat',
|
||||
title: 'Legacy',
|
||||
displayMessages: [],
|
||||
actualMessages: [],
|
||||
lastModified: 0
|
||||
} as unknown as ReturnType<typeof manager.historyManager.loadPastChat>)
|
||||
vi.spyOn(manager.historyManager, 'getModifiedItems').mockReturnValue(undefined)
|
||||
|
||||
await manager.loadPastChat('legacy-session-chat')
|
||||
|
||||
expect(manager.modifiedItems).toBeInstanceOf(Set)
|
||||
expect(manager.modifiedItems?.size).toBe(0)
|
||||
})
|
||||
|
||||
it('seeds a session chat mask from its stored modified-items', async () => {
|
||||
const manager = createManager(createInputMock())
|
||||
manager.isSessionChat = true
|
||||
vi.spyOn(manager.historyManager, 'loadPastChat').mockReturnValue({
|
||||
id: 'tracked-session-chat',
|
||||
title: 'Tracked',
|
||||
displayMessages: [],
|
||||
actualMessages: [],
|
||||
lastModified: 0
|
||||
} as unknown as ReturnType<typeof manager.historyManager.loadPastChat>)
|
||||
vi.spyOn(manager.historyManager, 'getModifiedItems').mockReturnValue([
|
||||
'script:u/admin/hello_world'
|
||||
])
|
||||
|
||||
await manager.loadPastChat('tracked-session-chat')
|
||||
|
||||
expect([...(manager.modifiedItems ?? [])]).toEqual(['script:u/admin/hello_world'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('AIChatManager context compaction', () => {
|
||||
|
||||
@@ -239,12 +239,13 @@ export default class HistoryManager {
|
||||
lastModified: Date.now(),
|
||||
...(this.sessionId ? { sessionId: this.sessionId } : {}),
|
||||
...(contextUsage !== undefined ? { contextUsage } : {}),
|
||||
// Only persist when the caller passes a defined array. Loaded legacy
|
||||
// chats keep their accumulator undefined, so we never retroactively
|
||||
// stamp them with [] (which would flip them to the filtered view).
|
||||
// But since `put` replaces the whole record, a caller that omits the
|
||||
// argument must not ERASE a tracked chat's stored mask — fall back to
|
||||
// the previously saved field.
|
||||
// Only persist when the caller passes a defined array — an untracked
|
||||
// chat (the global side-panel chat, mask still undefined) must not be
|
||||
// stamped with [], which would flip it to the filtered view. Session
|
||||
// chats are always tracked (see AIChatManager.loadPastChat), so they do
|
||||
// pass a defined array and persist it. Since `put` replaces the whole
|
||||
// record, a caller that omits the argument must not ERASE a tracked
|
||||
// chat's stored mask — fall back to the previously saved field.
|
||||
...(modifiedItems !== undefined
|
||||
? { modifiedItems }
|
||||
: this.savedChats[this.currentChatId]?.modifiedItems !== undefined
|
||||
|
||||
@@ -713,6 +713,11 @@ async function initRuntime(runtime: SessionRuntime, session: Session) {
|
||||
manager.historyManager.setCurrentChatId(session.chatId)
|
||||
await manager.historyManager.tagChatWithSession(session.chatId, session.id)
|
||||
await manager.loadPastChat(session.chatId)
|
||||
// loadPastChat only seeds the mask when the chat exists in history; a chatId
|
||||
// pointing at a chat not yet persisted (no turn saved) would leave it
|
||||
// undefined, and the Edits surface would then show every workspace draft.
|
||||
// Start tracking so this session is scoped to its own edits from the outset.
|
||||
if (manager.modifiedItems === undefined) manager.initModifiedItemsTracking()
|
||||
} else {
|
||||
// Brand-new session chat: start tracking modified items now (empty mask)
|
||||
// so the session bar filters to this chat's changes from the first turn.
|
||||
|
||||
Reference in New Issue
Block a user