packages/cli/src/commands/run.test.ts (1)

67-69: 🧹 Nitpick | 🔵 Trivial

Initial mock missing timestampedSnapshotPath.

Overwritten in beforeEach, but inconsistent with SaveSnapshotResult type.

 const mockSaveExecutionSnapshot: MockedFunction<typeof saveExecutionSnapshot> = vi
   .fn()
-  .mockResolvedValue({ snapshotPath: '/mock/snapshot.snapshot.deepnote' })
+  .mockResolvedValue({
+    snapshotPath: '/mock/snapshot.snapshot.deepnote',
+    timestampedSnapshotPath: '/mock/snapshot-timestamped.snapshot.deepnote',
+  })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/run.test.ts` around lines 67 - 69, The mock for
saveExecutionSnapshot (mockSaveExecutionSnapshot) returns an object missing
timestampedSnapshotPath which violates the SaveSnapshotResult shape; update the
mockResolvedValue to include timestampedSnapshotPath (alongside snapshotPath) so
the initial mock matches the SaveSnapshotResult type used elsewhere (and remains
consistent with the value overwritten in beforeEach).

packages/mcp/src/tools/execution.ts (1)

248-298: ⚠️ Potential issue | 🟠 Major

Return type omits timestampedSnapshotPath.

Writes both snapshots but returns only snapshotPath. CLI version returns both. Inconsistent API.

-): Promise<{ snapshotPath: string }> {
+): Promise<{ snapshotPath: string; timestampedSnapshotPath: string }> {

And at line 298:

-  return { snapshotPath }
+  return { snapshotPath, timestampedSnapshotPath }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mcp/src/tools/execution.ts` around lines 248 - 298, The function
currently returns Promise<{ snapshotPath: string }> but writes both
timestampedSnapshotPath and snapshotPath; update the return type to include
timestampedSnapshotPath (e.g. Promise<{ snapshotPath: string;
timestampedSnapshotPath: string }>) and change the final return to return both {
snapshotPath, timestampedSnapshotPath } (symbols: snapshotPath,
timestampedSnapshotPath, serializeDeepnoteSnapshot, generateSnapshotFilename);
also search for and update any callers expecting the old single-field return to
handle the new two-field object.

packages/mcp/src/tools/execution.ts (2)

243-298: 🧹 Nitpick | 🔵 Trivial

Duplicated snapshot logic.

This function mirrors saveExecutionSnapshot in packages/cli/src/utils/output-persistence.ts. Consider importing or consolidating to avoid drift.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mcp/src/tools/execution.ts` around lines 243 - 298, The
saveExecutionSnapshot implementation is duplicated; refactor to reuse the
existing implementation in packages/cli/src/utils/output-persistence.ts by
extracting the shared snapshot logic into a single shared utility (or directly
importing the existing saveExecutionSnapshot/exported helpers) and update this
file to call that shared function; specifically consolidate behaviors around
building outputsByBlockId, merging outputs into DeepnoteFile, using
splitDeepnoteFile, getSnapshotDir, slugifyProjectName, generateSnapshotFilename
and serializeDeepnoteSnapshot so this module only calls the shared routine
instead of duplicating the logic.

---

243-298: ⚠️ Potential issue | 🟡 Minor

Return type inconsistent with output-persistence.ts.

timestampedSnapshotPath is computed but not returned. The sibling implementation in packages/cli/src/utils/output-persistence.ts returns both paths in SaveSnapshotResult.

Proposed fix

 async function saveExecutionSnapshot(
   sourcePath: string,
   file: DeepnoteFile,
   blockOutputs: Array<{ id: string; outputs: unknown[]; executionCount?: number | null }>,
   timing: { startedAt: string; finishedAt: string }
-): Promise<{ snapshotPath: string }> {
+): Promise<{ snapshotPath: string; timestampedSnapshotPath: string }> {
   // ... existing code ...

-  return { snapshotPath }
+  return { snapshotPath, timestampedSnapshotPath }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mcp/src/tools/execution.ts` around lines 243 - 298, The function
saveExecutionSnapshot currently computes timestampedSnapshotPath but only
returns snapshotPath, causing an inconsistent return type versus the sibling
SaveSnapshotResult; change the function's return type (Promise<{ snapshotPath:
string }>) to include timestampedSnapshotPath (e.g. Promise<{ snapshotPath:
string; timestampedSnapshotPath: string }>), and update the final return to
return both { snapshotPath, timestampedSnapshotPath }. Update any callers or
exported type aliases (e.g., SaveSnapshotResult) to accept the new shape so
usages of saveExecutionSnapshot receive both paths.