Contrib/excel enhancements clean

packages/excel/src/lib/adapter.ts

@@ -5,6 +5,7 @@ import SelectionIndicator from "./components/selection-indicator.svelte";
 import excelApiDts from "./docs/excel-officejs-api.d.ts?raw";
 import { getWorkbookMetadata, navigateTo } from "./excel/api";
 import { buildExcelSystemPrompt } from "./system-prompt";
+import { logToolCall } from "./telemetry-hooks";
 import { createExcelTools } from "./tools";
 import { getCustomCommands } from "./vfs/custom-commands";
 
@@ -66,15 +67,47 @@ export function createExcelAdapter(): AppAdapter {
       }
     },
 
-    onToolResult: (_toolCallId, result, isError) => {
-      if (isError) return;
-      const dirtyRanges = parseDirtyRanges(result);
-      if (dirtyRanges && dirtyRanges.length > 0) {
-        const first = dirtyRanges[0];
-        if (first.sheetId >= 0 && first.range !== "*") {
-          navigateTo(first.sheetId, first.range).catch(console.error);
-        } else if (first.sheetId >= 0) {
-          navigateTo(first.sheetId).catch(console.error);
+    onToolResult: (toolCallId, toolName, result, isError, durationMs) => {
+      // Parse tool result to check for success/error
+      let toolSuccess = !isError;
+      let toolErrorMessage: string | undefined;
+
+      // Check if result contains {"success":false}
+      if (!isError && result) {
+        try {
+          const parsed = JSON.parse(result);
+          if (parsed && typeof parsed === "object" && "success" in parsed) {
+            toolSuccess = parsed.success === true;
+            if (!toolSuccess && parsed.error) {
+              toolErrorMessage = parsed.error;
+            }
+          }
+        } catch {
+          // Not JSON - use isError flag
+        }
+      }
+
+      // Optional telemetry hook (can be set by implementations)
+      logToolCall({
+        toolCallId,
+        toolName,
+        success: toolSuccess,
+        threwException: isError, // True if tool crashed, false if returned {"success":false}
+        durationMs,
+        errorMessage: isError ? result?.substring(0, 500) : toolErrorMessage,
+        resultPreview: result?.substring(0, 500),
+      });
+
+      // Navigate to modified range on success
+      if (toolSuccess) {
+        const dirtyRanges = parseDirtyRanges(result);
+        if (dirtyRanges && dirtyRanges.length > 0) {
+          const first = dirtyRanges[0];
+          if (first.sheetId >= 0 && first.range !== "*") {
+            navigateTo(first.sheetId, first.range).catch(console.error);
+          } else if (first.sheetId >= 0) {
+            navigateTo(first.sheetId).catch(console.error);
+          }
         }
       }
     },

packages/excel/src/lib/system-prompt.ts

@@ -53,8 +53,51 @@ EXCEL WRITE:
 - resize_range: Adjust column widths and row heights
 - modify_object: Create/update/delete charts and pivot tables
 
+UNDO/REDO:
+- undo: Programmatically reverse write operations (real Ctrl+Z)
+- undo_history: View operations that can be undone
+
+PERMISSION & FALLBACK:
+- check_write_permissions: Check if workbook allows writes
+- provide_copy_paste_formulas: Generate formulas for manual entry when writes blocked
+
 eval_officejs has access to readFile(path) → Promise<string>, readFileBuffer(path) → Promise<Uint8Array>, and writeFile(path, content) → Promise<void> (content: string | Uint8Array) for VFS files.
 
+## UNDO SYSTEM - You Can Fix Mistakes!
+
+✅ **UNDO IS AVAILABLE**: All write operations are automatically tracked and can be reversed:
+- Made a mistake? Use the undo tool to reverse it programmatically
+- Accidentally overwrote data? Call undo immediately to restore it
+- Not sure about a change? Make it, then undo if needed
+- Check what can be undone with undo_history
+
+⚠️ **BEST PRACTICES**:
+1. Read data before modifying to understand what you're changing
+2. If uncertain, make the change and undo if it's wrong
+3. Check undo_history to see what operations can be reversed
+
+## HANDLING WRITE FAILURES - ALWAYS PROVIDE SOLUTIONS
+
+⚠️ **When writes are blocked** (workbook protected, read-only mode):
+1. **Don't just fail** - Excel may block direct writes due to protection
+2. **Immediately offer copy-paste formulas** using provide_copy_paste_formulas
+3. **Show exact formulas** the user can manually paste
+4. **Guide them step-by-step** on where to paste
+
+**Example flow when set_cell_range fails:**
+1. set_cell_range returns error "protected"
+2. Immediately call: provide_copy_paste_formulas with all the formulas
+3. Tell user: "Excel is blocking writes, here are formulas to paste manually"
+4. Show formatted, easy-to-copy formulas
+
+**Why this matters:**
+- Users expect solutions, not just errors
+- Manual paste is a valid fallback
+- Copy-paste formulas work even when Excel blocks programmatic writes
+- Maintains user productivity despite technical limitations
+
+IMPORTANT: Always build on existing logic and data - do not delete or overwrite unless the user explicitly asks you to. Extend formulas, add to ranges, and preserve existing work. If you need to restructure, ask first.
+
 Citations: Use markdown links with #cite: hash to reference sheets/cells. Clicking navigates there.
 - Sheet only: [Sheet Name](#cite:sheetId)
 - Cell/range: [A1:B10](#cite:sheetId!A1:B10)

packages/excel/src/lib/telemetry-hooks.ts

@@ -0,0 +1,100 @@
+/**
+ * Telemetry Hooks Interface
+ *
+ * Provides optional hooks for monitoring tool execution.
+ * Implementations can integrate with any monitoring service (Datadog, Application Insights, etc.)
+ */
+
+export interface TelemetryHooks {
+  /**
+   * Called after a tool executes
+   * @param toolCallId - Unique ID for this tool invocation
+   * @param toolName - Name of the tool that was called
+   * @param success - Whether the tool succeeded (parses JSON result for success field)
+   * @param durationMs - Execution time in milliseconds (if available)
+   * @param threwException - True if tool crashed with exception, false if returned {"success":false}
+   * @param errorMessage - Error message if failed
+   * @param resultPreview - First 500 chars of result for debugging
+   */
+  onToolResult?(params: {
+    toolCallId: string;
+    toolName: string;
+    success: boolean;
+    durationMs?: number;
+    threwException?: boolean;
+    errorMessage?: string;
+    resultPreview?: string;
+  }): void;
+
+  /**
+   * Called when user context is set (after authentication)
+   */
+  onUserContext?(user: { email: string; name: string; id: string }): void;
+
+  /**
+   * Called on errors
+   */
+  onError?(error: Error, context?: Record<string, any>): void;
+}
+
+/**
+ * Global telemetry hooks instance
+ * Set this during app initialization to enable monitoring
+ */
+export let telemetryHooks: TelemetryHooks | null = null;
+
+/**
+ * Initialize telemetry hooks
+ * Call this at app startup to enable monitoring
+ */
+export function initTelemetryHooks(hooks: TelemetryHooks): void {
+  telemetryHooks = hooks;
+  console.log("[Telemetry] Hooks initialized");
+}
+
+/**
+ * Helper to safely call hook
+ */
+function callHook<T extends keyof TelemetryHooks>(
+  hookName: T,
+  ...args: Parameters<NonNullable<TelemetryHooks[T]>>
+): void {
+  try {
+    const hook = telemetryHooks?.[hookName];
+    if (hook && typeof hook === "function") {
+      // @ts-expect-error - TypeScript struggles with spread args on union types
+      hook(...args);
+    }
+  } catch (error) {
+    console.error(`[Telemetry] Hook ${hookName} failed:`, error);
+  }
+}
+
+/**
+ * Log tool execution result
+ */
+export function logToolCall(params: {
+  toolCallId: string;
+  toolName: string;
+  success: boolean;
+  durationMs?: number;
+  threwException?: boolean;
+  errorMessage?: string;
+  resultPreview?: string;
+}): void {
+  callHook("onToolResult", params);
+}
+
+/**
+ * Log user context
+ */
+export function logUserContext(user: { email: string; name: string; id: string }): void {
+  callHook("onUserContext", user);
+}
+
+/**
+ * Log error
+ */
+export function logError(error: Error, context?: Record<string, any>): void {
+  callHook("onError", error, context);
+}

packages/excel/src/lib/tools/check-permissions.ts

@@ -0,0 +1,67 @@
+import { defineTool, toolSuccess, toolError } from "./types";
+
+/**
+ * Check workbook write permissions
+ *
+ * Before performing write operations, check if the workbook allows writes.
+ * If blocked, guide the user to manually enable write access.
+ */
+export const checkPermissionsTool = defineTool({
+  name: "check_write_permissions",
+  description:
+    "Check if the workbook allows write operations. Use this before attempting writes if you suspect permission issues.",
+  parameters: {
+    type: "object",
+    properties: {},
+  },
+  execute: async () => {
+    try {
+      const status = await Excel.run(async (context) => {
+        const workbook = context.workbook;
+        const protection = workbook.protection;
+
+        protection.load("protected");
+        await context.sync();
+
+        const sheets = context.workbook.worksheets;
+        sheets.load("items/protection/protected");
+        await context.sync();
+
+        const protectedSheets = sheets.items
+          .filter((sheet) => sheet.protection.protected)
+          .map((sheet) => sheet.name);
+
+        return {
+          workbookProtected: protection.protected,
+          protectedSheets,
+          canWrite: !protection.protected && protectedSheets.length === 0,
+        };
+      });
+
+      if (status.canWrite) {
+        return toolSuccess("✅ Workbook is writable. All write operations should work.");
+      } else {
+        let message = "⚠️ **Write restrictions detected:**\n\n";
+
+        if (status.workbookProtected) {
+          message += "- Workbook is protected\n";
+        }
+
+        if (status.protectedSheets.length > 0) {
+          message += `- Protected sheets: ${status.protectedSheets.join(", ")}\n`;
+        }
+
+        message += "\n**To enable writes:**\n";
+        message += "1. Go to Review → Unprotect Workbook/Sheet\n";
+        message += "2. Or ask the user to enable write access\n\n";
+        message += "I can provide copy-paste ready formulas instead.";
+
+        return toolSuccess(message);
+      }
+    } catch (error) {
+      return toolError(
+        `Failed to check permissions: ${error instanceof Error ? error.message : String(error)}`
+      );
+    }
+  },
+});

packages/excel/src/lib/tools/index.ts

@@ -12,6 +12,9 @@ export { resizeRangeTool } from "./resize-range";
 export { screenshotRangeTool } from "./screenshot-range";
 export { searchDataTool } from "./search-data";
 export { setCellRangeTool } from "./set-cell-range";
+export { undoTool, undoHistoryTool } from "./undo";
+export { checkPermissionsTool } from "./check-permissions";
+export { provideFormulasTool } from "./provide-formulas";
 export {
   defineTool,
   type ToolResult,
@@ -35,6 +38,9 @@ import { resizeRangeTool } from "./resize-range";
 import { screenshotRangeTool } from "./screenshot-range";
 import { searchDataTool } from "./search-data";
 import { setCellRangeTool } from "./set-cell-range";
+import { undoHistoryTool, undoTool } from "./undo";
+import { checkPermissionsTool } from "./check-permissions";
+import { provideFormulasTool } from "./provide-formulas";
 
 export function createExcelTools(ctx: AgentContext) {
   return [
@@ -56,5 +62,11 @@ export function createExcelTools(ctx: AgentContext) {
     resizeRangeTool,
     modifyObjectTool,
     createEvalOfficeJsTool(ctx),
+    // Undo/redo
+    undoTool,
+    undoHistoryTool,
+    // Safety & fallback tools
+    checkPermissionsTool,
+    provideFormulasTool,
   ];
 }