fix tests

Author: MKartaviciute

e2e/cesium-viewer-tools-live.spec.ts

@@ -29,6 +29,7 @@ const INPUT_SELECTOR = '[data-testid="chat-input-wrapper"] input';
 interface ViewerSnapshot {
   entities: number;
   imageryLayers: number;
+  dataSources: number;
   cameraPosition: { x: number; y: number; z: number };
   enableLighting: boolean;
   clockMultiplier: number;
@@ -48,6 +49,7 @@ async function getViewerSnapshot(page: Page): Promise<ViewerSnapshot> {
     return {
       entities: viewer.entities.values.length,
       imageryLayers: viewer.imageryLayers.length,
+      dataSources: viewer.dataSources.length,
       cameraPosition: { x: camera.position.x, y: camera.position.y, z: camera.position.z },
       enableLighting: viewer.scene.globe.enableLighting,
       clockMultiplier: viewer.clock.multiplier,
@@ -685,6 +687,62 @@ test.describe("Cesium viewer tools — end-to-end against the live backend", ()
       .toBe(0);
   });
 
+  // ---- geoJson tools ---------------------------------------------------------------------------
+
+  const GEO_JSON_ADD_PROMPT =
+    "Using the geoJsonAdd tool, render this GeoJSON polygon on the globe (pass it directly, do " +
+    'not use any other tool first): {"type":"Feature","properties":{},"geometry":{"type":' +
+    '"Polygon","coordinates":[[[-0.5,51.3],[0.2,51.3],[0.2,51.7],[-0.5,51.7],[-0.5,51.3]]]}}. ' +
+    'Name it "geojson-test-zone".';
+
+  test("geoJsonAdd", async ({ page }) => {
+    test.setTimeout(3 * 60_000);
+
+    const before = await getViewerSnapshot(page);
+
+    const result = await runToolStep(page, { prompt: GEO_JSON_ADD_PROMPT, toolName: "geoJsonAdd" });
+
+    expect(result.name).toBe("geojson-test-zone");
+    expect(typeof result.entityCount, "expected result.entityCount to be a number").toBe("number");
+    expect(result.entityCount as number).toBeGreaterThan(0);
+
+    // The tool result carries no view-state confirmation beyond entityCount — cross-check against
+    // the live Viewer's own dataSources/entities collections, not just the reported success.
+    await expect
+      .poll(async () => (await getViewerSnapshot(page)).dataSources, {
+        message: "expected a new GeoJSON data source to be added",
+        timeout: 10_000,
+      })
+      .toBeGreaterThan(before.dataSources);
+  });
+
+  test("geoJsonRemove", async ({ page }) => {
+    test.setTimeout(5 * 60_000);
+
+    const before = await getViewerSnapshot(page);
+
+    await runToolStep(page, { prompt: GEO_JSON_ADD_PROMPT, toolName: "geoJsonAdd" });
+
+    await expect
+      .poll(async () => (await getViewerSnapshot(page)).dataSources, {
+        message: "expected the GeoJSON data source to be added",
+        timeout: 10_000,
+      })
+      .toBeGreaterThan(before.dataSources);
+
+    await runToolStep(page, {
+      prompt: 'Using geoJsonRemove, remove the GeoJSON data source named "geojson-test-zone".',
+      toolName: "geoJsonRemove",
+    });
+
+    await expect
+      .poll(async () => (await getViewerSnapshot(page)).dataSources, {
+        message: "expected the GeoJSON data source to be removed",
+        timeout: 10_000,
+      })
+      .toBe(before.dataSources);
+  });
+
   // ---- flyTo ----------------------------------------------------------------------------------
 
   /**

e2e/turf-tools-live.spec.ts

@@ -188,3 +188,113 @@ test.describe("Turf.js tools — end-to-end against the live backend", () => {
     expect(geojson.properties?.name).toBe("test-point");
   });
 });
+
+/**
+ * Regression coverage for a real bug: `geoJsonAdd` (a client-side `@cesium-ai/tools-schemas` tool,
+ * not a Turf tool) crashed with an opaque "Cannot read properties of undefined (reading 'length')"
+ * when the model passed it a structurally-loose GeoJSON object missing `features`/`geometry` —
+ * exactly the shape a model can end up constructing when relaying a Turf tool's dataset back out
+ * (see `packages/tools-schemas/src/tools/geoJsonAdd/geoJsonAdd.schema.ts`'s fix and this repo's
+ * `resolve-geojson.ts` for the identical, earlier-fixed bug class in Turf tools themselves).
+ *
+ * These tests exercise the full real-world chain this app is meant to support end-to-end against
+ * the live backend AND the live CesiumJS `Viewer` (unlike the tests above, which only assert on
+ * tool-result JSON): a Turf tool produces/stores a dataset, `turf_get_dataset` fetches its full
+ * GeoJSON back into the conversation, and `geoJsonAdd` renders that GeoJSON on the globe. Viewer
+ * state is read via the dev-only `window.__cesiumViewerForE2E` seam, same convention as
+ * `cesium-viewer-tools-live.spec.ts`.
+ */
+test.describe("Turf.js output rendered on the globe via geoJsonAdd", () => {
+  test.beforeEach(async ({ page }) => {
+    await page.goto("/");
+    await page.waitForSelector(INPUT_SELECTOR, { timeout: 30_000 });
+  });
+
+  async function getDataSourceCount(page: Page): Promise<number> {
+    return page.evaluate(() => {
+      const viewer = (window as unknown as { __cesiumViewerForE2E?: any }).__cesiumViewerForE2E;
+      if (!viewer) {
+        throw new Error(
+          "window.__cesiumViewerForE2E is undefined — is the app running in dev mode " +
+            "(`npm run dev:frontend`), and has CesiumGlobe finished mounting?",
+        );
+      }
+      return viewer.dataSources.length as number;
+    });
+  }
+
+  test("turf_buffer dataset -> turf_get_dataset -> geoJsonAdd renders a new data source", async ({
+    page,
+  }) => {
+    test.setTimeout(3 * 60_000);
+
+    const before = await getDataSourceCount(page);
+
+    const buffered = await runToolStep(page, {
+      prompt:
+        "Using the turf_buffer tool, buffer this GeoJSON point by 500 meters (do not register it " +
+        'first, pass it directly): {"type":"Feature","properties":{},"geometry":{"type":"Point",' +
+        '"coordinates":[-0.1278,51.5074]}}',
+      toolName: "turf_buffer",
+    });
+    const datasetId = buffered.dataset_id as string;
+    expect(typeof datasetId).toBe("string");
+
+    await runToolStep(page, {
+      prompt: `Using the turf_get_dataset tool, fetch the full GeoJSON for dataset_id "${datasetId}".`,
+      toolName: "turf_get_dataset",
+    });
+
+    const rendered = await runToolStep(page, {
+      prompt:
+        "Now using the geoJsonAdd tool, render the exact GeoJSON returned by the previous " +
+        'turf_get_dataset call on the globe. Name it "turf-buffer-zone" and use a red stroke color.',
+      toolName: "geoJsonAdd",
+    });
+
+    expect(rendered.success).toBe(true);
+    expect(rendered.name).toBe("turf-buffer-zone");
+    expect(typeof rendered.entityCount).toBe("number");
+    expect(rendered.entityCount as number).toBeGreaterThan(0);
+
+    const after = await getDataSourceCount(page);
+    expect(after).toBe(before + 1);
+  });
+
+  test("turf_hex_grid dataset -> turf_get_dataset -> geoJsonAdd renders a new data source", async ({
+    page,
+  }) => {
+    test.setTimeout(3 * 60_000);
+
+    const before = await getDataSourceCount(page);
+
+    const hexGrid = await runToolStep(page, {
+      prompt:
+        "Using the turf_hex_grid tool, generate a hexagonal grid (no point aggregation) over the " +
+        "bounding box west -0.5, south 51.3, east 0.2, north 51.7, with a cell side of 5 kilometers.",
+      toolName: "turf_hex_grid",
+    });
+    const datasetId = hexGrid.dataset_id as string;
+    expect(typeof datasetId).toBe("string");
+
+    await runToolStep(page, {
+      prompt: `Using the turf_get_dataset tool, fetch the full GeoJSON for dataset_id "${datasetId}".`,
+      toolName: "turf_get_dataset",
+    });
+
+    const rendered = await runToolStep(page, {
+      prompt:
+        "Now using the geoJsonAdd tool, render the exact GeoJSON returned by the previous " +
+        'turf_get_dataset call on the globe. Name it "turf-hex-grid".',
+      toolName: "geoJsonAdd",
+    });
+
+    expect(rendered.success).toBe(true);
+    expect(rendered.name).toBe("turf-hex-grid");
+    expect(typeof rendered.entityCount).toBe("number");
+    expect(rendered.entityCount as number).toBeGreaterThan(0);
+
+    const after = await getDataSourceCount(page);
+    expect(after).toBe(before + 1);
+  });
+});

packages/chat-element/src/chat-client/chat-client.ts

@@ -189,15 +189,40 @@ export class ChatClient {
       }
     }
 
+    // A tool call the server already fully resolved within THIS stream (its
+    // `tool-output-available`/`tool-output-error` chunk arrived right after
+    // `tool-input-available`, before the stream finished) is still recorded in
+    // `pendingToolCalls` by `handleStreamLine`, but its `state` has already been
+    // flipped to `"result"` by the time we get here — resolving it below is a
+    // no-op. Whether THAT still warrants a follow-up request depends on whether
+    // the model already produced its reply to it in this same stream (e.g. a
+    // non-`stopAfterTools` tool like `turf_area` that the model calls and then
+    // immediately answers about in one turn): if so, a follow-up would just
+    // re-send the same transcript — now including the model's own just-given
+    // answer — and get back a second, redundant reply. A tool call still in
+    // `"call"` state (needs real client-side execution) always needs a
+    // follow-up regardless, since the model hasn't seen any result for it yet.
+    const hadUnresolvedClientToolCalls = pendingToolCalls.some((inv) => inv.state === "call");
+    const hadUnresolvedApprovals = pendingApprovals.some(
+      (inv) => inv.state === "approval-requested",
+    );
+    const hasTextReply = ((assistantMsg as Message | null)?.content.length ?? 0) > 0;
+    const hadServerResolvedToolCallsAwaitingReply =
+      pendingToolCalls.length > 0 && !hadUnresolvedClientToolCalls && !hasTextReply;
+
     await this.resolveClientToolCalls(pendingToolCalls);
     await this.resolveApprovals(pendingApprovals);
     const continueForServerResults = await this.resolveServerToolOutcomes(pendingServerResults);
 
-    // If there were tool calls or approval decisions to send back, or a host
-    // reaction to a server-resolved result asked to continue, make another
-    // request so the model can react (the resolve* calls above guarantee
-    // every entry here is now resolved).
-    if (pendingToolCalls.length > 0 || pendingApprovals.length > 0 || continueForServerResults) {
+    // If there were tool calls or approval decisions that actually needed
+    // resolving, or a host reaction to a server-resolved result asked to
+    // continue, make another request so the model can react.
+    if (
+      hadUnresolvedClientToolCalls ||
+      hadUnresolvedApprovals ||
+      hadServerResolvedToolCallsAwaitingReply ||
+      continueForServerResults
+    ) {
       this.toolCallRound++;
       if (this.toolCallRound > this.maxToolCallRounds) {
         this.emitError(

packages/chat-element/src/components/MessageItem.tsx

@@ -41,6 +41,19 @@ export function MessageItem({
       >
         {isError ? "Error" : isUser ? "You" : "Assistant"}
       </Typography>
+      {message.toolInvocations?.map((inv) => (
+        <ToolCard
+          key={inv.toolCallId}
+          invocation={inv}
+          isPendingApproval={approval?.pendingApprovalToolCallId === inv.toolCallId}
+          onApprove={approval?.onApprove}
+          onReject={approval?.onReject}
+          structuredResult={structuredResultByToolName?.get(inv.toolName)}
+          mcpApp={mcpAppByToolName?.get(inv.toolName)}
+          mcpAppApiBase={mcpAppApiBase}
+          mcpAppSandboxUrl={mcpAppSandboxUrl}
+        />
+      ))}
       {message.content && (
         <Typography
           render={<div />}
@@ -58,19 +71,6 @@ export function MessageItem({
           )}
         </Typography>
       )}
-      {message.toolInvocations?.map((inv) => (
-        <ToolCard
-          key={inv.toolCallId}
-          invocation={inv}
-          isPendingApproval={approval?.pendingApprovalToolCallId === inv.toolCallId}
-          onApprove={approval?.onApprove}
-          onReject={approval?.onReject}
-          structuredResult={structuredResultByToolName?.get(inv.toolName)}
-          mcpApp={mcpAppByToolName?.get(inv.toolName)}
-          mcpAppApiBase={mcpAppApiBase}
-          mcpAppSandboxUrl={mcpAppSandboxUrl}
-        />
-      ))}
     </div>
   );
 }

packages/tools-schemas/src/tools/geoJsonAdd/geoJsonAdd.schema.ts

@@ -6,17 +6,54 @@ import { z } from "zod";
  * client-side executor. Carries no model-facing description text (see
  * `geoJsonAdd.ts` and `flyTo.schema.ts` for the convention this follows).
  *
- * `geojson` is deliberately typed loosely (`z.record`, refined to require a
- * GeoJSON `type`) rather than a fully-typed Feature/FeatureCollection schema —
- * this tool only needs to hand the object to `Cesium.GeoJsonDataSource.load`
- * unchanged, and a strict schema would reject legitimate GeoJSON this tool
- * has no reason to constrain (arbitrary per-feature `properties`, any
- * geometry type, etc.).
+ * `geojson` is a discriminated-by-`type` union rather than a single
+ * `.catchall(z.unknown())` object: a bare `{ type: "..." } & catchall(unknown)`
+ * shape serializes to JSON schema with only `type` listed under `properties`
+ * (everything else — including `features`/`geometry`/`geometries` — collapses
+ * into an opaque `additionalProperties: {}`), which models frequently ignore.
+ * That previously let a model pass e.g. `{"type":"FeatureCollection"}` with no
+ * `features` array at all, which `Cesium.GeoJsonDataSource.load` then crashes
+ * on with an opaque `Cannot read properties of undefined (reading 'length')`
+ * instead of a clear tool `{ error }`. Naming `features`/`geometry`/
+ * `geometries` as real (still-catchall) properties fixes both the model-facing
+ * schema and Zod's own validation, same fix already applied to
+ * `@cesium-ai/turf-tools`' `resolve-geojson.ts`.
  */
+const featureShape = z
+  .object({
+    type: z.literal("Feature"),
+    geometry: z
+      .object({ type: z.string() })
+      .catchall(z.unknown())
+      .describe(
+        "REQUIRED: the feature's GeoJSON geometry, e.g. { type: 'Point', coordinates: [...] }.",
+      ),
+  })
+  .catchall(z.unknown())
+  .describe("A GeoJSON Feature — geometry is required.");
+
+const featureCollectionShape = z
+  .object({
+    type: z.literal("FeatureCollection"),
+    features: z
+      .array(z.unknown())
+      .describe("REQUIRED: the array of GeoJSON Feature objects (may be empty)."),
+  })
+  .catchall(z.unknown())
+  .describe("A GeoJSON FeatureCollection — features is required.");
+
+const geometryCollectionShape = z
+  .object({
+    type: z.literal("GeometryCollection"),
+    geometries: z
+      .array(z.unknown())
+      .describe("REQUIRED: the array of GeoJSON geometry objects (may be empty)."),
+  })
+  .catchall(z.unknown())
+  .describe("A GeoJSON GeometryCollection — geometries is required.");
+
 export const geoJsonAddInputShape = z.object({
-  geojson: z
-    .object({ type: z.enum(["Feature", "FeatureCollection", "GeometryCollection"]) })
-    .catchall(z.unknown()),
+  geojson: z.union([featureShape, featureCollectionShape, geometryCollectionShape]),
   name: z.string().optional(),
   stroke: z.string().optional(),
   fill: z.string().optional(),

packages/tools/README.md

@@ -28,14 +28,12 @@ const result = await executors.flyTo(viewer, rawArgsFromTheModel);
 This package has no logging of its own by default — every executor just resolves a plain `{ success, error? }` result, so a caller that never reads `error` never finds out a tool call failed. Pass a `logger` as `createCesiumToolExecutors`'s second argument to have every executor's outcome (success, a resolved `{ error }`, or a thrown rejection) reported through it:
 
 ```ts
-import { createCesiumToolExecutors } from "@cesium-ai/tools";
-import { createConsoleLogger } from "@cesium-ai/observability";
+import { createCesiumToolExecutors, createConsoleToolsLogger } from "@cesium-ai/tools";
 
-const logger = createConsoleLogger({ scope: "cesium-tools", level: "warn" });
-const executors = createCesiumToolExecutors({}, logger);
+const executors = createCesiumToolExecutors({}, createConsoleToolsLogger("warn"));
 ```
 
-Pass any `Logger` from `@cesium-ai/observability` (e.g. one backed by your app's OTEL telemetry) to route logging through your own provider.
+Implement your own `ToolsLogger` (e.g. backed by an OTEL-wired app logger — see this repo's `frontend/src/tools/cesium-tool-executors.ts` for the worked example) to route this package's logging through your own telemetry instead of `console`.
 
 ## Customizing a tool: two mechanisms
 
@@ -167,6 +165,7 @@ Executors are grouped by domain rather than one file per tool (unlike `@cesium-a
 - [`src/utils/cesium-values.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/utils/cesium-values.ts) — small conversions from schema-shaped plain data (a `{longitude, latitude, height?}` position, a CSS color string) into real Cesium types (`Cartesian3`, `Color`, ...).
 - [`src/utils/animation-registry.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/utils/animation-registry.ts), [`src/utils/imagery-registry.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/utils/imagery-registry.ts) — per-`Viewer` `WeakMap`-based bookkeeping the animation and imagery tools need (which entity ids/imagery layers this package itself created), so `animationListActive`/`imageryList`/etc. only ever report on state they created.
 - [`src/utils/create-entity-add-executor.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/utils/create-entity-add-executor.ts) — `createEntityAddExecutor`, the generic validate/build/add/error-handling plumbing every `entityAdd*` tool's own `createXExecutor` (in `entities.ts`) is built from.
+- [`src/logger.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/logger.ts) — `ToolsLogger`, `noopToolsLogger`, `createConsoleToolsLogger` — see "Logging" above.
 - [`src/index.ts`](https://github.com/CesiumGS/cesiumjs-ai-starter-app/blob/main/packages/tools/src/index.ts) — `DEFAULT_CESIUM_TOOL_EXECUTORS`, `createCesiumToolExecutors`.
 
 ## Exports
@@ -190,3 +189,7 @@ Executors are grouped by domain rather than one file per tool (unlike `@cesium-a
 | `createEntityAddExecutor`                                               | The lower-level generic every `createEntityAddXExecutor` above is built from — only needed if you're building a brand-new `entityAdd*`-shaped tool from scratch.                       |
 | `EntityAddExecutorConfig`                                               | Type: an `entityAdd*` factory's config (`shape`, `extendEntityOptions`).                                                                                                               |
 | `flyTo`, `cameraSetView`, ...                                           | Every individual default executor, exported by name (one per tool).                                                                                                                    |
+| `ToolsLogger`                                                           | Type: the console-shaped logger interface (`debug`/`info`/`warn`/`error`) `createCesiumToolExecutors`'s `logger` argument accepts. See "Logging" above.                                |
+| `ToolsLogLevel`                                                         | Type: `"debug" \| "info" \| "warn" \| "error" \| "silent"`, accepted by `createConsoleToolsLogger`.                                                                                    |
+| `createConsoleToolsLogger`                                              | Builds a `console`-backed `ToolsLogger`, prefixed `[cesium-tools]`, filtered by level (default `"warn"`).                                                                              |
+| `noopToolsLogger`                                                       | A `ToolsLogger` whose methods are all no-ops — pass explicitly if you want `createCesiumToolExecutors`'s wrapping without any actual output.                                           |