fix: agent command and history items from #34

src/app/projectSections.ts

@@ -75,6 +75,8 @@ const sections: ProjectSection[] = []
 
 /** Own a part of the project file — see ProjectSection. */
 export function registerProjectSection<J>(section: ProjectSection<J>): () => void {
+  // A key the app writes itself would be overwritten, or overwrite it.
+  if (CORE_KEYS.has(section.key)) throw new Error(`The project key "${section.key}" is the app's own.`)
   if (sections.some((s) => s.key === section.key)) throw new Error(`Two project sections share the key "${section.key}".`)
   sections.push(section as ProjectSection)
   return () => {

src/commands/bridge.ts

@@ -161,7 +161,11 @@ export function startBridge({ port, token, socket = (url) => new WebSocket(url),
     const answered = onThisComputer() || (await reach(`http://127.0.0.1:${port}/scanruler-mcp`))
     if (stopped) return
     if (!answered) {
-      if (typeof navigator !== 'undefined' && navigator.permissions && (await loopbackDenied())) {
+      const denied = typeof navigator !== 'undefined' && !!navigator.permissions && (await loopbackDenied())
+      // The link may have been switched off while the browser answered; a
+      // stopped link stays off rather than showing a refusal or retrying.
+      if (stopped) return
+      if (denied) {
         useAgentLink.setState({
           status: 'refused',
           detail: 'The browser keeps this site from reaching apps on this computer — allow local network access for it in the site settings (the icon left of the address), then reload.',

src/commands/core/session.ts

@@ -13,7 +13,7 @@ import { RECENT_KEPT, RECENT_RESULT_CHARS, recentOutcomes } from '../registry'
 import { enumOf, int, obj, str } from '../schema'
 import { internalWorkspace, publicWorkspace, sessionAfterStep, sessionState, workspaceNames } from '../state'
 import { CommandError, type Command } from '../types'
-import { discardSchema, requireDiscardable, waitFor } from './common'
+import { discardSchema, lastError, requireDiscardable, waitFor } from './common'
 
 const state: Command = {
   name: 'state',
@@ -44,6 +44,9 @@ const reset: Command<{ discard?: boolean }> = {
       flat?.closeImage()
       useStore.getState().setProjectName(null)
     })
+    // runImport reports a failure on screen rather than throwing it; the
+    // agent must hear of it too, not read a session it thinks is empty.
+    if (useStore.getState().errorText) throw new CommandError('failed', lastError('The session could not be started over.'))
     return await sessionState()
   },
 }

src/commands/registry.ts

@@ -90,6 +90,8 @@ export function runsHeadless(name: string): boolean {
 export const COMMAND_TIMEOUT_MS = 10 * 60_000
 
 let queue: Promise<unknown> = Promise.resolve()
+/** The change last given up on past its time, while the queue waits for it. */
+let lingering: Promise<void> | null = null
 
 /** The viewport's own queue: the commands that set it or picture it
  *  (Command.scene), one at a time, whatever else runs beside them. */
@@ -120,10 +122,12 @@ const asCommandError = (e: unknown): CommandError =>
  *  (ctx.signal) and let go — whatever it does after is nobody's answer —
  *  and the caller is answered `timeout`, so that what waits on it, the
  *  history step and the queue, moves on. */
-function limited<T>(name: string, ms: number, abort: AbortController, run: () => Promise<T>): Promise<T> {
+function limited<T>(name: string, ms: number, abort: AbortController, run: () => Promise<T>, onLate?: (ran: Promise<unknown>) => void): Promise<T> {
   let timer: ReturnType<typeof setTimeout> | undefined
+  let ran: Promise<T>
   const late = new Promise<never>((_, reject) => {
     timer = setTimeout(() => {
+      onLate?.(ran)
       const e = new CommandError(
         'timeout',
         `${name} did not finish in ${ms / 1000} s and was given up on. It may have changed the session in part — read session.state (history.undo says the last step) before running it again.`,
@@ -132,7 +136,6 @@ function limited<T>(name: string, ms: number, abort: AbortController, run: () =>
       reject(e)
     }, ms)
   })
-  let ran: Promise<T>
   try {
     ran = run()
   } catch (e) {
@@ -181,7 +184,14 @@ function start(name: string, input: unknown, onProgress: ((text: string) => void
   }
   if (command.interrupts) return interrupt(name, command, args, onProgress)
   const job = queue.then(() => inScene(command, () => execute(name, command, args, onProgress, deferred)))
-  queue = job.catch(() => {})
+  // A change given up on past its time may still be changing the session:
+  // the next change waits for it to finish, or for one more of its time if
+  // it never does, so their changes do not land in each other's steps.
+  queue = job.catch(() => {}).then(() => {
+    const waiting = lingering
+    lingering = null
+    return waiting
+  })
   return job
 }
 
@@ -288,7 +298,11 @@ async function guarded(
   }
   // Raced inside the history step, so that the step ends — and the history
   // takes changes again — when the command is given up on.
-  const run = () => limited(name, command.timeoutMs ?? COMMAND_TIMEOUT_MS, abort, () => command.run(input, ctx))
+  const ms = command.timeoutMs ?? COMMAND_TIMEOUT_MS
+  const linger = (ran: Promise<unknown>) => {
+    lingering = Promise.race([ran.then(() => {}, () => {}), new Promise<void>((resolve) => setTimeout(resolve, ms))])
+  }
+  const run = () => limited(name, ms, abort, () => command.run(input, ctx), linger)
   useCommandActivity.setState({ running: { name, title: command.title, since: Date.now() }, progress: null })
   current = { name, deferred }
   try {

src/plugins/registry.ts

@@ -75,17 +75,32 @@ let installed: readonly ScanRulerPlugin[] | null = null
  *  plugin registers never depends on the order the bundler found it in. */
 export function installPlugins(): readonly ScanRulerPlugin[] {
   if (installed) return installed
-  const all = Object.values(found)
-    .map((m) => m.default)
-    .sort((a, b) => a.id.localeCompare(b.id))
+  installed = installEnabled(Object.values(found).map((m) => m.default))
+  return installed
+}
+
+/** Install those of `all` that are enabled, by id, and return the ones that
+ *  installed. Only enabled plugins are checked for a shared id, so
+ *  `?plugins=none` still opens an app whose build has a clash. A plugin
+ *  whose install() throws is left out of the list, so nothing — the
+ *  headless engine included — takes it for one in use. */
+export function installEnabled(all: readonly ScanRulerPlugin[]): readonly ScanRulerPlugin[] {
+  const enabled = all.filter(pluginEnabled).sort((a, b) => a.id.localeCompare(b.id))
   const ids = new Set<string>()
-  for (const p of all) {
+  for (const p of enabled) {
     if (ids.has(p.id)) throw new Error(`Two plugins share the id "${p.id}".`)
     ids.add(p.id)
   }
-  installed = all.filter(pluginEnabled)
-  for (const p of installed) p.install?.()
-  return installed
+  const done: ScanRulerPlugin[] = []
+  for (const p of enabled) {
+    try {
+      p.install?.()
+      done.push(p)
+    } catch (error) {
+      console.error(`The plugin "${p.id}" could not be installed.`, error)
+    }
+  }
+  return done
 }
 
 /** The plugins in use — none until installPlugins() has run. */

src/state/historyStore.ts

@@ -150,16 +150,16 @@ export function clearHistory(): void {
 const sameValue = (a: unknown, b: unknown) => a === b ||
   (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.every((value, index) => value === b[index]))
 const same = (a: object, b: object) => Object.keys(a).every((k) => sameValue((a as Record<string, unknown>)[k], (b as Record<string, unknown>)[k]))
-function record(before: HistorySnapshot, after: HistorySnapshot, label: string, workspace: Workspace, generation: number) {
+function record(before: HistorySnapshot, after: HistorySnapshot, label: string, workspace: Workspace, generation: number, stepGroup = group) {
   if (generation !== epoch) return // A source replacement starts another document.
   const keys = Object.keys(before).filter((key) =>
     comparedByIdentity(key) ? before[key] !== after[key] : !same(before[key] as object, after[key] as object))
   if (!keys.length) return
   const previous = pick(before, keys), next = pick(after, keys)
   useHistory.setState((state) => {
     const last = state.past.at(-1)
-    const entry = { before: previous, after: next, label, workspace, group }
-    if (group && !state.future.length && last?.group === group && last.workspace === workspace) {
+    const entry = { before: previous, after: next, label, workspace, group: stepGroup }
+    if (stepGroup && !state.future.length && last?.group === stepGroup && last.workspace === workspace) {
       return { past: [...state.past.slice(0, -1), { ...last, before: { ...previous, ...last.before }, after: { ...last.after, ...next } }], future: [] }
     }
     return { past: [...state.past.slice(-(LIMIT - 1)), entry], future: [] }
@@ -181,7 +181,9 @@ export async function historyAsync(label: string, job: () => Promise<void>): Pro
   const before = captureHistory(), workspace = useShell.getState().workspace, generation = epoch
   paused++
   try { await job() }
-  finally { paused--; record(before, captureHistory(), label, workspace, generation) }
+  // An async step is a command of its own: it never joins the edit group a
+  // focused field or a held pointer happens to have open while it runs.
+  finally { paused--; record(before, captureHistory(), label, workspace, generation, 0) }
 }
 
 /** A dimension box with something in it — a slot filled, or an edit. The
@@ -238,7 +240,18 @@ async function travel(redo: boolean): Promise<void> {
   try {
     await hooks.runRestore(async () => {
       await hooks.beforeRestore(patch)
-      for (const p of participants) await p.beforeRestore?.(patch)
+      const done: HistoryParticipant[] = []
+      try {
+        for (const p of participants) { await p.beforeRestore?.(patch); done.push(p) }
+      } catch (error) {
+        // The app's part has already swapped the scan or moved it: a
+        // participant that fails puts it back, and those before it, so the
+        // scan stays on the same side of the step as the stores.
+        const back = pick(previous, Object.keys(patch))
+        await hooks.beforeRestore(back)
+        for (const p of done) await p.beforeRestore?.(back)
+        throw error
+      }
       apply(patch)
       useShell.getState().setWorkspace(entry.workspace)
       useHistory.setState((s) => redo
@@ -265,6 +278,13 @@ let wrapActions: ((a: HistoryActions) => () => void) | null = null
 /** Take part in the history — see HistoryParticipant. The returned function
  *  ends it. */
 export function registerHistory(participant: HistoryParticipant): () => void {
+  // A section shares the snapshot with the app's parts and every other
+  // participant's: a key taken twice would silently overwrite one of them.
+  const taken = new Set([...Object.keys(captureCore()), ...participants.flatMap((p) => (p.sections ?? []).map((s) => s.key))])
+  for (const s of participant.sections ?? []) {
+    if (taken.has(s.key)) throw new Error(`Two history sections share the key "${s.key}".`)
+    taken.add(s.key)
+  }
   participants.push(participant)
   const unwrap = wrapActions ? (participant.actions ?? []).map((a) => wrapActions!(a)) : []
   return () => {

tests/bridgeStop.test.ts

@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+// Switching the agent link off while the browser's local-network permission
+// query is still pending must leave the link off, not "refused".
+import { afterEach, expect, it, vi } from 'vitest'
+import { startBridge } from '../src/commands/bridge'
+import { useAgentLink } from '../src/commands/agentLink'
+
+const wait = (ms: number) => new Promise((r) => setTimeout(r, ms))
+
+afterEach(() => vi.unstubAllGlobals())
+
+it('a link stopped during the permission query stays off', async () => {
+  let answer: ((v: { state: string }) => void) | undefined
+  vi.stubGlobal('navigator', { permissions: { query: () => new Promise((r) => { answer = r }) } })
+  let sockets = 0
+  const stop = startBridge({ port: 1, token: 'tokentoken', socket: () => { sockets++; return {} as WebSocket }, reach: async () => false })
+  for (let i = 0; i < 300 && !answer; i++) await wait(10)
+  expect(answer).toBeDefined()
+  stop()
+  const after = useAgentLink.getState().status
+  answer!({ state: 'denied' })
+  await wait(20)
+  expect(useAgentLink.getState().status).toBe(after)
+  expect(useAgentLink.getState().status).not.toBe('refused')
+  expect(sockets).toBe(0)
+})

tests/commandTimeout.test.ts

@@ -115,6 +115,41 @@ describe('a command that never finishes', () => {
     expect(useHistory.getState().past.map((p) => p.label)).toEqual(['Agent: hang', 'Agent: bump'])
   })
 
+  it('lets the next change wait for one given up on that finishes late', async () => {
+    const late: Command = {
+      name: 'late',
+      title: 'Late',
+      description: 'Changes the session after its time.',
+      input: obj({}),
+      timeoutMs: 20,
+      run: async () => {
+        await new Promise((r) => setTimeout(r, 50))
+        useStore.setState((s) => ({ nextDimensionId: s.nextDimensionId + 1 }))
+      },
+    }
+    const slow: Command = {
+      name: 'slow',
+      title: 'Slow',
+      description: 'Adds one, a little later.',
+      input: obj({}),
+      run: async () => {
+        await new Promise((r) => setTimeout(r, 60))
+        useStore.setState((s) => ({ nextNumber: s.nextNumber + 1 }))
+      },
+    }
+    const offLate = registerCommands('later', [late, slow])
+    try {
+      const gaveUp = runCommand('later.late').catch((e: CommandError) => e)
+      const next = runCommand('later.slow')
+      expect(((await gaveUp) as CommandError).code).toBe('timeout')
+      await next
+      const step = useHistory.getState().past.find((p) => p.label === 'Agent: slow')!
+      // The late change landed before the next began, not inside its step.
+      expect(step.after.measure!.nextDimensionId).toBe(step.before.measure!.nextDimensionId)
+      expect(step.after.measure!.nextNumber).toBe(step.before.measure!.nextNumber + 1)
+    } finally { offLate() }
+  })
+
   it('answers a read that never finishes with timeout too', async () => {
     const e = await runCommand('test.look').catch((x: CommandError) => x)
     expect((e as CommandError).code).toBe('timeout')

tests/commandsMeasure.test.ts

@@ -618,4 +618,16 @@ describe('session, report, files and history', () => {
     expect(useStore.getState().busy).toBe(false)
     expect((await refused('element.fit', { kind: 'plane', at: { vertex: 0 } })).code).toBe('no_scan')
   })
+
+  it('session.reset says so when starting over fails', async () => {
+    await openBox()
+    const client = headless.session.clientRef.current!
+    const commitImport = client.commitImport
+    client.commitImport = async () => { throw new Error('worker gone') }
+    try {
+      const error = await refused('session.reset', { discard: true })
+      expect(error.code).toBe('failed')
+      expect(error.message).toMatch(/worker gone/)
+    } finally { client.commitImport = commitImport }
+  })
 })

tests/history.test.ts

@@ -5,7 +5,7 @@ import { useFlat } from '../src/state/flatStore'
 import { useDeviation } from '../src/state/deviationStore'
 import { useThickness } from '../src/state/thicknessStore'
 import { useShell } from '../src/state/shellStore'
-import { beginHistoryGroup, endHistoryGroup, clearHistory, configureHistory, historyAction, historyAsync, installHistory, undo, redo, useHistory } from '../src/state/historyStore'
+import { beginHistoryGroup, endHistoryGroup, clearHistory, configureHistory, historyAction, historyAsync, installHistory, registerHistory, undo, redo, useHistory } from '../src/state/historyStore'
 import { frameKey, type SectionFrame } from '../src/core/section/frame'
 
 const m = () => useStore.getState(), f = () => useFlat.getState(), d = () => useDeviation.getState()
@@ -229,3 +229,49 @@ describe('project history', () => {
   })
 })
 
+describe('async steps and edit groups', () => {
+  it('keeps an async step out of the edit group open while it runs', async () => {
+    const s0 = m()
+    beginHistoryGroup()
+    historyAction('3D Measure: user edit', () => useStore.setState({ nextNumber: s0.nextNumber + 1 }))
+    await historyAsync('Agent: fit element', async () => {
+      await new Promise((r) => setTimeout(r, 5))
+      useStore.setState({ nextDimensionId: s0.nextDimensionId + 7 })
+    })
+    endHistoryGroup()
+    expect(h().past.map((e) => e.label)).toEqual(['3D Measure: user edit', 'Agent: fit element'])
+  })
+})
+
+describe('history section keys', () => {
+  it('refuses a key the app or another participant already holds', () => {
+    const section = (key: string) => ({ key, capture: () => null, apply: () => {} })
+    expect(() => registerHistory({ sections: [section('measure')] })).toThrow(/share the key "measure"/)
+    const stop = registerHistory({ sections: [section('mine')] })
+    try {
+      expect(() => registerHistory({ sections: [section('mine')] })).toThrow(/share the key "mine"/)
+    } finally { stop() }
+  })
+})
+
+describe('a failing participant', () => {
+  it('puts the scan back when a participant fails before a step', async () => {
+    const older = { name: 'older' }, newer = { name: 'newer' }
+    let scan: object = older
+    unconfigure()
+    unconfigure = configureHistory({
+      getScan: () => scan,
+      beforeRestore: async (patch) => { if ('scan' in patch) scan = patch.scan as object },
+    })
+    const stop = registerHistory({ beforeRestore: async () => { throw new Error('worker gone') } })
+    try {
+      historyAction('Mesh: edit', () => { scan = newer; useStore.setState({ nextNumber: m().nextNumber + 1 }) })
+      const number = m().nextNumber
+      await undo()
+      expect(scan).toBe(newer)
+      expect(m().nextNumber).toBe(number)
+      expect(h().past).toHaveLength(1)
+    } finally { stop() }
+  })
+})
+

tests/pluginRegistry.test.ts

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: AGPL-3.0-only
+// Which plugins installPlugins puts in use: only the enabled ones are checked
+// for a shared id, and one whose install() throws is left out of the list.
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import type { ScanRulerPlugin } from '../src/plugins/api'
+import { installEnabled } from '../src/plugins/registry'
+
+const plugin = (id: string, install?: () => void) => ({ id, name: id, install }) as unknown as ScanRulerPlugin
+
+afterEach(() => vi.unstubAllGlobals())
+
+describe('installing plugins', () => {
+  it('lets ?plugins=none open a build whose plugins share an id', () => {
+    vi.stubGlobal('location', { search: '?plugins=none' })
+    expect(installEnabled([plugin('twin'), plugin('twin')])).toEqual([])
+  })
+
+  it('leaves a plugin whose install throws out of the list', () => {
+    vi.spyOn(console, 'error').mockImplementation(() => {})
+    const good = plugin('good', () => {})
+    const bad = plugin('bad', () => { throw new Error('broken') })
+    expect(installEnabled([good, bad])).toEqual([good])
+  })
+})

tests/projectSections.test.ts

@@ -112,3 +112,9 @@ describe('a project’s name', () => {
     expect(useStore.getState().projectName).toBeNull()
   })
 })
+
+describe('a section key', () => {
+  it('may not be one the app writes itself', () => {
+    expect(() => registerProjectSection({ key: 'scan' } as Parameters<typeof registerProjectSection>[0])).toThrow(/app's own/)
+  })
+})