mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(chat): scope troubleshoot handoff to its workspace (#5344)
* fix(chat): scope troubleshoot handoff to its workspace MothershipHandoffStorage now records the target workspaceId and only the matching workspace consumes (and clears) it; a different workspace leaves it untouched for its owner. Prevents a workspace-A handoff from firing in workspace B, where A's executionId can't resolve and the run context is dropped. * test(chat): cover legacy no-workspaceId handoff tombstoning
This commit is contained in:
@@ -343,9 +343,9 @@ export function Home({ chatId, userName, userId }: HomeProps) {
|
||||
*/
|
||||
useEffect(() => {
|
||||
if (chatId) return
|
||||
const handoff = MothershipHandoffStorage.consume()
|
||||
const handoff = MothershipHandoffStorage.consume(workspaceId)
|
||||
if (handoff) sendMessage(handoff.message, undefined, handoff.contexts)
|
||||
}, [chatId, sendMessage])
|
||||
}, [chatId, workspaceId, sendMessage])
|
||||
|
||||
function resolveResourceFromContext(
|
||||
context: ChatContext
|
||||
|
||||
@@ -418,7 +418,7 @@ export function LogDetailsContent({ log, onActiveTabChange }: LogDetailsContentP
|
||||
? `The "${workflowName}" workflow run failed. Investigate the error in this run and help me fix it.`
|
||||
: 'This workflow run failed. Investigate the error in this run and help me fix it.'
|
||||
if (sendMothershipMessage(message, [context])) return
|
||||
if (MothershipHandoffStorage.store({ message, contexts: [context] })) {
|
||||
if (MothershipHandoffStorage.store({ message, contexts: [context] }, workspaceId)) {
|
||||
router.push(`/workspace/${workspaceId}/home`)
|
||||
}
|
||||
}, [log.executionId, log.workflow?.name, workspaceId, router])
|
||||
|
||||
@@ -5,6 +5,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { MothershipHandoffStorage, STORAGE_KEYS } from '@/lib/core/utils/browser-storage'
|
||||
import type { ChatContext } from '@/stores/panel'
|
||||
|
||||
const WS = 'ws-1'
|
||||
|
||||
describe('MothershipHandoffStorage', () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear()
|
||||
@@ -12,39 +14,72 @@ describe('MothershipHandoffStorage', () => {
|
||||
|
||||
it('round-trips a handoff and trims the message, preserving contexts', () => {
|
||||
const contexts: ChatContext[] = [{ kind: 'logs', executionId: 'run-1', label: 'My Flow' }]
|
||||
expect(MothershipHandoffStorage.store({ message: ' fix it ', contexts })).toBe(true)
|
||||
expect(MothershipHandoffStorage.store({ message: ' fix it ', contexts }, WS)).toBe(true)
|
||||
|
||||
expect(MothershipHandoffStorage.consume()).toEqual({ message: 'fix it', contexts })
|
||||
expect(MothershipHandoffStorage.consume(WS)).toEqual({ message: 'fix it', contexts })
|
||||
})
|
||||
|
||||
it('is one-shot — a second consume returns null', () => {
|
||||
MothershipHandoffStorage.store({ message: 'fix it' })
|
||||
MothershipHandoffStorage.store({ message: 'fix it' }, WS)
|
||||
|
||||
expect(MothershipHandoffStorage.consume()).not.toBeNull()
|
||||
expect(MothershipHandoffStorage.consume()).toBeNull()
|
||||
expect(MothershipHandoffStorage.consume(WS)).not.toBeNull()
|
||||
expect(MothershipHandoffStorage.consume(WS)).toBeNull()
|
||||
})
|
||||
|
||||
it('refuses to store an empty message', () => {
|
||||
expect(MothershipHandoffStorage.store({ message: ' ' })).toBe(false)
|
||||
expect(MothershipHandoffStorage.consume()).toBeNull()
|
||||
it('refuses to store without a message or workspace', () => {
|
||||
expect(MothershipHandoffStorage.store({ message: ' ' }, WS)).toBe(false)
|
||||
expect(MothershipHandoffStorage.store({ message: 'fix it' }, '')).toBe(false)
|
||||
expect(MothershipHandoffStorage.consume(WS)).toBeNull()
|
||||
})
|
||||
|
||||
it('leaves a handoff owned by another workspace untouched for its owner', () => {
|
||||
MothershipHandoffStorage.store({ message: 'fix it' }, WS)
|
||||
|
||||
// A different workspace must not claim it, and must not clear it.
|
||||
expect(MothershipHandoffStorage.consume('ws-other')).toBeNull()
|
||||
expect(localStorage.getItem(STORAGE_KEYS.MOTHERSHIP_HANDOFF)).not.toBeNull()
|
||||
|
||||
// The owning workspace still consumes it.
|
||||
expect(MothershipHandoffStorage.consume(WS)).toEqual({ message: 'fix it', contexts: undefined })
|
||||
})
|
||||
|
||||
it('tombstones a corrupted entry (missing timestamp) instead of leaving it forever', () => {
|
||||
localStorage.setItem(STORAGE_KEYS.MOTHERSHIP_HANDOFF, JSON.stringify({ message: 'fix it' }))
|
||||
localStorage.setItem(
|
||||
STORAGE_KEYS.MOTHERSHIP_HANDOFF,
|
||||
JSON.stringify({ message: 'fix it', workspaceId: WS })
|
||||
)
|
||||
|
||||
expect(MothershipHandoffStorage.consume()).toBeNull()
|
||||
expect(MothershipHandoffStorage.consume(WS)).toBeNull()
|
||||
expect(localStorage.getItem(STORAGE_KEYS.MOTHERSHIP_HANDOFF)).toBeNull()
|
||||
})
|
||||
|
||||
it('tombstones a legacy entry (message + timestamp, no workspaceId) rather than firing it', () => {
|
||||
// The old pre-scoping format could be sitting in storage across a deploy —
|
||||
// it must be discarded, not attributed to the current workspace.
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
vi.setSystemTime(new Date('2026-01-01T00:00:00Z'))
|
||||
localStorage.setItem(
|
||||
STORAGE_KEYS.MOTHERSHIP_HANDOFF,
|
||||
JSON.stringify({ message: 'fix it', timestamp: Date.now() })
|
||||
)
|
||||
|
||||
expect(MothershipHandoffStorage.consume(WS)).toBeNull()
|
||||
expect(localStorage.getItem(STORAGE_KEYS.MOTHERSHIP_HANDOFF)).toBeNull()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('drops and clears a handoff older than maxAge', () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
vi.setSystemTime(new Date('2026-01-01T00:00:00Z'))
|
||||
MothershipHandoffStorage.store({ message: 'fix it' })
|
||||
MothershipHandoffStorage.store({ message: 'fix it' }, WS)
|
||||
|
||||
vi.advanceTimersByTime(61 * 1000)
|
||||
|
||||
expect(MothershipHandoffStorage.consume()).toBeNull()
|
||||
expect(MothershipHandoffStorage.consume(WS)).toBeNull()
|
||||
expect(localStorage.getItem(STORAGE_KEYS.MOTHERSHIP_HANDOFF)).toBeNull()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
|
||||
@@ -320,33 +320,37 @@ export class MothershipHandoffStorage {
|
||||
private static readonly KEY = STORAGE_KEYS.MOTHERSHIP_HANDOFF
|
||||
|
||||
/**
|
||||
* Store a handoff to be auto-sent on the next home-surface mount.
|
||||
* @returns True if stored, false when the message is empty.
|
||||
* Store a handoff to be auto-sent on the next home-surface mount, scoped to
|
||||
* the workspace it targets so a different workspace never claims it.
|
||||
* @returns True if stored, false when the message or workspace is empty.
|
||||
*/
|
||||
static store(handoff: MothershipHandoff): boolean {
|
||||
static store(handoff: MothershipHandoff, workspaceId: string): boolean {
|
||||
const message = handoff.message.trim()
|
||||
if (!message) {
|
||||
if (!message || !workspaceId) {
|
||||
return false
|
||||
}
|
||||
|
||||
return BrowserStorage.setItem(MothershipHandoffStorage.KEY, {
|
||||
message,
|
||||
contexts: handoff.contexts,
|
||||
workspaceId,
|
||||
timestamp: Date.now(),
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieve and consume the stored handoff. Clears the entry as soon as one
|
||||
* exists — before the validity and expiry checks — so a malformed or expired
|
||||
* handoff is tombstoned rather than lingering across future mounts, and a
|
||||
* valid handoff fires exactly once.
|
||||
* Retrieve and consume the stored handoff for `workspaceId`. A handoff owned
|
||||
* by a different workspace is left untouched for its owner — its tagged run
|
||||
* only resolves in its own workspace, so misfiring it elsewhere would drop the
|
||||
* context. The owner (and any legacy/corrupt entry) is tombstoned via `clear`
|
||||
* before the validity/expiry checks so it fires at most once and never lingers.
|
||||
* @param maxAge - Maximum age in milliseconds (default: 60 seconds)
|
||||
*/
|
||||
static consume(maxAge: number = 60 * 1000): MothershipHandoff | null {
|
||||
static consume(workspaceId: string, maxAge: number = 60 * 1000): MothershipHandoff | null {
|
||||
const data = BrowserStorage.getItem<{
|
||||
message?: string
|
||||
contexts?: ChatContext[]
|
||||
workspaceId?: string
|
||||
timestamp?: number
|
||||
} | null>(MothershipHandoffStorage.KEY, null)
|
||||
|
||||
@@ -354,9 +358,18 @@ export class MothershipHandoffStorage {
|
||||
return null
|
||||
}
|
||||
|
||||
if (data.workspaceId && data.workspaceId !== workspaceId) {
|
||||
return null
|
||||
}
|
||||
|
||||
MothershipHandoffStorage.clear()
|
||||
|
||||
if (!data.message || !data.timestamp || Date.now() - data.timestamp > maxAge) {
|
||||
if (
|
||||
!data.workspaceId ||
|
||||
!data.message ||
|
||||
!data.timestamp ||
|
||||
Date.now() - data.timestamp > maxAge
|
||||
) {
|
||||
return null
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user