fix(desktop): a check that runs beside a change no longer hides it from the view

apps/desktop/src/main/solidcraft/headless.ts

@@ -32,6 +32,7 @@ export interface HeadlessOptions {
   /** Design file to open, or none for a new design. */
   design?: string | undefined;
   env: NodeJS.ProcessEnv;
+  log?: (message: string) => void;
 }
 
 interface BatchBody {
@@ -61,9 +62,16 @@ export class HeadlessEngine {
   private readonly changeListeners = new Set<(revision: Revision) => void>();
   private readonly closeListeners = new Set<(reason: string) => void>();
   private last: Revision | null = null;
+  /** Tool calls of the assistant that have not been checked yet. */
+  private calls = 0;
+  /** A change the listeners have not been told of, although its revision is recorded already. */
+  private untold = false;
   private closed = false;
 
-  private constructor(private readonly client: Client) {}
+  private constructor(
+    private readonly client: Client,
+    private readonly log: (message: string) => void = () => undefined,
+  ) {}
 
   /** Starts the engine and waits for the MCP handshake; rejects when the process exits first. */
   static async start(options: HeadlessOptions): Promise<HeadlessEngine> {
@@ -82,7 +90,7 @@ export class HeadlessEngine {
     });
     const client = new Client({ name: 'partbuddy', version: '0.1.0' });
     await client.connect(transport);
-    const engine = new HeadlessEngine(client);
+    const engine = new HeadlessEngine(client, options.log);
     client.onclose = () => {
       engine.shutdown('SolidCraft (headless) ended');
     };
@@ -117,15 +125,48 @@ export class HeadlessEngine {
   }
 
   /**
-   * Reads the revision after a call that may have changed the design, and tells the listeners
-   * when it moved. `quiet` records it without telling (PartBuddy's own autosave).
+   * After a change PartBuddy made itself: the listeners are told, also when a quiet check that ran
+   * beside the change recorded its revision first (a trial of the print check ending, an autosave).
    */
-  async checkForChange(quiet = false): Promise<void> {
-    if (this.closed) return;
-    const next = await this.revision();
+  async changed(): Promise<void> {
+    this.untold = true;
+    await this.check(false);
+  }
+
+  /**
+   * After work that moves the revision and leaves the design as it was (a trial, PartBuddy's own
+   * save): the revision is recorded and nobody is told, so the next read of the assistant mirrors
+   * nothing.
+   */
+  async unchanged(): Promise<void> {
+    await this.check(true);
+  }
+
+  /**
+   * Reads the revision and tells the listeners when it moved or a change is still untold. A quiet
+   * check tells nobody; what it records while a tool call of the assistant is under way may be
+   * that call's change, so it leaves it for the call's own check to tell.
+   */
+  private async check(quiet: boolean, endsCall = false): Promise<void> {
+    let next: Revision | null = null;
+    try {
+      if (!this.closed) next = await this.revision();
+    } finally {
+      // In one go with the check itself: a quiet check that comes right after it finds no call left.
+      if (endsCall) this.calls -= 1;
+    }
+    if (!next) return;
     const moved = this.last?.documentId !== next.documentId || this.last.revision !== next.revision;
     this.last = next;
-    if (moved && !quiet) for (const listener of this.changeListeners) listener(next);
+    if (quiet) {
+      if (moved && this.calls > 0) this.untold = true;
+      return;
+    }
+    if (!moved && !this.untold) return;
+    // The race this guards against, or a change that left the revision where it was.
+    if (!moved) this.log(`revision ${String(next.revision)} was recorded before its change was told; telling it now`);
+    this.untold = false;
+    for (const listener of this.changeListeners) listener(next);
   }
 
   /** The mirror's view of the engine: the same two requests the control channel answers. */
@@ -150,10 +191,11 @@ export class HeadlessEngine {
     instructions: () => this.client.getInstructions(),
     listTools: async (): Promise<UpstreamTool[]> => (await this.client.listTools()).tools,
     callTool: async (name, args) => {
+      this.calls += 1;
       try {
         return await this.client.callTool({ name, arguments: args }, { timeout: LONG_TIMEOUT_MS });
       } finally {
-        await this.checkForChange().catch(() => undefined);
+        await this.check(false, true).catch(() => undefined);
       }
     },
   };
@@ -164,7 +206,7 @@ export class HeadlessEngine {
     const result = (await this.client.callTool({ name: 'save', arguments: { path } }, { timeout: LONG_TIMEOUT_MS })) as ToolResult;
     if (result.isError) throw new Error(textOf(result).slice(0, 500) || 'save failed');
     // Saving moves the revision; nothing in the design changed, so nobody needs to mirror.
-    await this.checkForChange(true);
+    await this.unchanged();
   }
 
   async close(): Promise<void> {

apps/desktop/src/main/solidcraft/supervisor.ts

@@ -259,7 +259,7 @@ export class SolidCraftSupervisor {
         10 * 60 * 1000,
       );
       if (batch.failed) throw new Error(batch.failed.error);
-      await this.engine?.checkForChange();
+      await this.engine?.changed();
       const created = (batch.results[0] as { created_bodies?: unknown } | undefined)?.created_bodies;
       return Array.isArray(created) ? created.filter((name): name is string => typeof name === 'string') : [];
     } finally {
@@ -531,7 +531,7 @@ export class SolidCraftSupervisor {
       const naming = await requester.request<BatchResult>('engine.batch', { commands: steps, rollback: false }, 60_000);
       if (naming.failed) this.options.log(`the standard part was not named: ${naming.failed.error}`);
     }
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
     return body;
   }
 
@@ -550,7 +550,7 @@ export class SolidCraftSupervisor {
       try {
         return await this.dimensions.probe(target, current, value);
       } finally {
-        await this.engine?.checkForChange(true).catch(() => undefined);
+        await this.engine?.unchanged().catch(() => undefined);
       }
     });
     this.probing = run.catch(() => undefined);
@@ -561,7 +561,7 @@ export class SolidCraftSupervisor {
   async applyDimension(parameter: string, value: number, factor: 1 | 2, keepFitMark = false): Promise<void> {
     await this.probing;
     await this.dimensions.apply(parameter, value, factor, keepFitMark);
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   /** The user parameters of the design for the parameter panel; reading changes nothing. */
@@ -581,7 +581,7 @@ export class SolidCraftSupervisor {
       try {
         return await work();
       } finally {
-        await this.engine?.checkForChange(true).catch(() => undefined);
+        await this.engine?.unchanged().catch(() => undefined);
         clearTimeout(slow);
         const took = performance.now() - started;
         if (took > 20_000) this.options.log(`a trial (${what}) took ${String(Math.round(took / 1000))} s`);
@@ -598,7 +598,7 @@ export class SolidCraftSupervisor {
   async applyAction(action: DesignAction): Promise<void> {
     await this.probing;
     await this.actions.apply(action);
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   /**
@@ -676,7 +676,7 @@ export class SolidCraftSupervisor {
       const coloured = await this.actions.colourSplitPieces(layers);
       if (coloured > 0) this.options.log(`coloured the pieces of ${String(coloured)} split logo bodies`);
     }
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   /** The placed logo at a picked point, for the logo controls of the selection dock; reading changes nothing. */
@@ -720,7 +720,7 @@ export class SolidCraftSupervisor {
     // Already where it should be (centred twice): nothing to do.
     if (planned.steps.length === 0) return;
     await this.actions.applySteps(planned.steps);
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   measure(items: MeasureItem[]): Promise<MeasureResult> {
@@ -737,7 +737,7 @@ export class SolidCraftSupervisor {
       () => undefined,
     );
     await run;
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   /** Where parts stop each joint value on its way to a limit (ADR 0027, `joint.sweep`); the design stays as it is. */
@@ -1041,7 +1041,7 @@ export class SolidCraftSupervisor {
     const commands = Array.from({ length: count }, () => ({ command, params: {} }));
     const batch = await this.requester().request<{ failed?: { error: string } }>('engine.batch', { commands, rollback: false }, 10 * 60 * 1000);
     if (batch.failed) throw new Error(`${command} failed: ${batch.failed.error}`);
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   /** One engine command of the user, after any running trial; the change is mirrored as usual. */
@@ -1050,7 +1050,7 @@ export class SolidCraftSupervisor {
     const batch = await this.requester().request<{ failed?: { error: string } }>('engine.batch', { commands: [step], rollback: false }, 10 * 60 * 1000);
     if (batch.failed) throw new Error(`${step.command} failed: ${batch.failed.error}`);
     // The window reports the change with design.changed; the headless engine is asked.
-    await this.engine?.checkForChange();
+    await this.engine?.changed();
   }
 
   private publish(): void {
@@ -1091,7 +1091,7 @@ export class SolidCraftSupervisor {
     if (!this.launchedBefore && this.options.openLastOnStart === false) this.designs?.forget();
     this.launchedBefore = true;
     const design = this.designs?.last();
-    const start = (file: string | undefined) => HeadlessEngine.start({ cli, allowPaths, design: file, env: this.options.environment.env });
+    const start = (file: string | undefined) => HeadlessEngine.start({ cli, allowPaths, design: file, env: this.options.environment.env, log: (message) => this.options.log(message) });
     let engine: HeadlessEngine;
     try {
       engine = await start(design);

apps/desktop/test/headless.test.ts

@@ -0,0 +1,107 @@
+import { describe, expect, it } from 'vitest';
+
+import { HeadlessEngine, type Revision } from '../src/main/solidcraft/headless';
+
+/**
+ * The engine's change checks against a stand-in for SolidCraft whose revision the test moves by
+ * hand. Tool calls other than `batch` stay open until the test ends them, so a check can be put
+ * between a call and its own check.
+ */
+function setup() {
+  let revision = 1;
+  const open: (() => void)[] = [];
+  const logged: string[] = [];
+  const client = {
+    getInstructions: () => undefined,
+    listTools: () => Promise.resolve({ tools: [] }),
+    callTool: ({ name }: { name: string }) => {
+      if (name === 'batch') {
+        return Promise.resolve({ content: [{ type: 'text', text: JSON.stringify({ results: [{ document_id: 'doc', revision }] }) }] });
+      }
+      return new Promise((resolve) => {
+        open.push(() => {
+          resolve({ content: [{ type: 'text', text: '{}' }] });
+        });
+      });
+    },
+  };
+  const engine = Reflect.construct(HeadlessEngine, [client, (message: string) => logged.push(message)]) as HeadlessEngine;
+  const told: Revision[] = [];
+  engine.onChange((next) => told.push(next));
+  return {
+    engine,
+    told,
+    logged,
+    /** The design changes: SolidCraft counts the revision up. */
+    move: () => {
+      revision += 1;
+    },
+    /** Ends the oldest open tool call. */
+    answer: () => {
+      open.shift()?.();
+    },
+  };
+}
+
+describe('headless engine: telling a change', () => {
+  it('tells a change of PartBuddy whose revision a trial recorded first', async () => {
+    const { engine, told, logged, move } = setup();
+    await engine.unchanged();
+    // The change is built; a trial that ran beside it ends before the change's own check.
+    move();
+    await engine.unchanged();
+    await engine.changed();
+    expect(told).toEqual([{ documentId: 'doc', revision: 2 }]);
+    expect(logged).toHaveLength(1);
+  });
+
+  it('tells a change of the assistant whose revision a trial recorded while the call ran', async () => {
+    const { engine, told, move, answer } = setup();
+    await engine.unchanged();
+    const call = engine.tools.callTool('extrude', {});
+    move();
+    await engine.unchanged();
+    answer();
+    await call;
+    expect(told).toEqual([{ documentId: 'doc', revision: 2 }]);
+  });
+
+  it('tells nothing after a trial alone, nor after the assistant reads when the trial is over', async () => {
+    const { engine, told, move, answer } = setup();
+    await engine.unchanged();
+    move();
+    await engine.unchanged();
+    expect(told).toEqual([]);
+    const read = engine.tools.callTool('inspect', {});
+    answer();
+    await read;
+    expect(told).toEqual([]);
+  });
+
+  it('tells each of two changes that follow one another', async () => {
+    const { engine, told, move, answer } = setup();
+    await engine.unchanged();
+    move();
+    await engine.changed();
+    const call = engine.tools.callTool('extrude', {});
+    move();
+    answer();
+    await call;
+    expect(told.map((next) => next.revision)).toEqual([2, 3]);
+  });
+
+  it('counts a call as over with its check, so a later trial is not taken for its change', async () => {
+    const { engine, told, move, answer } = setup();
+    await engine.unchanged();
+    const first = engine.tools.callTool('inspect', {});
+    answer();
+    await first;
+    // A trial after the call moves the revision; the next read must not mirror it.
+    move();
+    await engine.unchanged();
+    const second = engine.tools.callTool('inspect', {});
+    answer();
+    await second;
+    expect(told).toEqual([]);
+  });
+});