feat(deepnote-file): Migration to single notebook deepnote file - #364

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

Inline comments:
In `@packages/cli/src/commands/split.test.ts`:
- Around line 37-157: Add a test to split.test.ts that uses
createMultiNotebookFile/serializeDeepnoteFile to write a multi-notebook project
and also writes an accompanying snapshot file next to the source (e.g.,
`${inputPath}.snapshot`), then run the split via createSplitAction(program) and
assert that per-notebook snapshot files were created in the output directory
(one snapshot per generated .deepnote file) and that their contents deserialize
via deserializeDeepnoteFile and correspond to each notebook; use the existing
tempDir, fs helpers, and cleanup pattern consistent with the other tests and
mirror the checks used for generated .deepnote files.

In `@packages/cli/src/completions.ts`:
- Around line 155-162: The completion logic for the output option doesn't treat
`split` as a command, so when completing the `-o/--output` value Bash uses
generic file completion instead of directory completion; update the case that
handles `-o --output` (the branch that inspects `prev` for output flags) to
include `split)` alongside other command labels so the output-option handler
triggers directory-only completion for the `split` subcommand as well (look for
the `split)` case and the `COMPREPLY=( $(compgen -W "-o --output --force" --
"${cur}") )` logic to make the change).

In `@packages/cli/src/utils/analysis.ts`:
- Around line 212-218: The warning message uses nonInitNotebooks.length but
calls it "notebooks" which is misleading when file.project.initNotebookId
exists; update the issues.push message in the block that creates the
'multi-notebook' warning (the place using nonInitNotebooks and
file.project.initNotebookId) to either (a) report the actual total number of
notebooks using file.project.notebooks.length if you mean total notebooks, or
(b) explicitly say "non-init notebooks" and keep nonInitNotebooks.length if you
mean to count only non-init ones—make the message text consistent with the count
you choose.

In `@packages/cli/src/utils/output-persistence.test.ts`:
- Around line 133-137: Add a test case in
packages/cli/src/utils/output-persistence.test.ts that covers the multi-notebook
branch by constructing a fixture where file.project.notebooks.length > 1 and
asserting the produced path matches the legacy format
"{slug}_{projectId}_latest.snapshot.deepnote" (e.g., resolve(..., 'snapshots',
'test-project_test-project-id-1234-5678-90ab_latest.snapshot.deepnote'););
locate the existing single-notebook assertions around the resolve(...) lines and
duplicate the pattern for the multi-notebook scenario, ensuring the code path
exercised calls the same helper (the function under test used by those
assertions) so the legacy filename format remains covered during migration;
apply the same addition for the other mentioned blocks (around lines 149-153 and
165-169) to fully cover all branches.

In `@packages/convert/src/snapshot/split.ts`:
- Around line 149-152: The single-notebook branch returns the original file
object which breaks immutability consistency with the multi-notebook path that
returns new objects; update the single-notebook return to return a shallow copy
of file (e.g., spread properties) so callers that mutate results won't alter the
input — change the branch that uses nb = file.project.notebooks[0] and return [{
notebook: { id: nb.id, name: nb.name }, file }] to return an object where file
is a copied object instead of the original reference.

In `@packages/mcp/src/tools/snapshots.ts`:
- Around line 227-229: handleSnapshotSplit currently writes legacy filenames
like {slug}_{projectId}_... while reads (findSnapshotsForProject) are scoped by
notebookId; update handleSnapshotSplit (and the other snapshot-write sites
referenced around lines ~318-320 and ~490-491) to include the notebookId in the
snapshot filenames (or use a single filename-construction helper) so that split
single-notebook snapshots are saved as notebook-scoped names (e.g., include
notebookId in the "latest" and timestamped filename patterns), and ensure any
helper/function used by findSnapshotsForProject matches the new naming
convention.

---

Outside diff comments:
In `@packages/cli/src/commands/lint.test.ts`:
- Around line 309-318: Add behavioral tests that assert the "multi-notebook"
warning is actually emitted and suppressed: in
packages/cli/src/commands/lint.test.ts (near the validCodes array and existing
lint command tests) add two Vitest cases — one that runs the lint command
against a synthetic multi-notebook project fixture and expects a warning with
code "multi-notebook" to appear in the output/diagnostics, and a second that
runs the same fixture but passes the CLI flag "--notebook" (or uses the helper
that simulates that flag) and asserts that no "multi-notebook" warning is
emitted; ensure you reference the existing validCodes array and the lint
invocation helper (e.g., runLintCommand / invokeCli or whatever test helper is
used) so the new tests integrate with the current suite.

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

Inline comments:
In `@packages/cli/src/commands/split.ts`:
- Around line 69-112: The final success line always reports "Split N snapshot(s)
into ..." using existingSnapshots.length even if some failed, which can mislead
users; update the code in the snapshot handling block (around
findSnapshotsForProject, loadSnapshotFile, splitSnapshotByNotebooks,
serializeDeepnoteSnapshot and the snapshotFailures array) to compute and report
the actual number of successfully split snapshots (e.g., successCount =
existingSnapshots.length - snapshotFailures.length) or include both counts
(e.g., "Split X of N snapshot(s); Y failed") and use that value in the output
call instead of existingSnapshots.length so the summary accurately reflects
failures.

In `@packages/cli/src/utils/output-persistence.ts`:
- Around line 107-113: The generated snapshot filenames pass notebookId verbatim
to generateSnapshotFilename, but parseSnapshotFilename only recognizes strict
UUID/hex notebook IDs; to fix, apply the same validation guard used by
parseSnapshotFilename to the notebookId before calling generateSnapshotFilename
(i.e., if notebookId does not match the UUID/hex regex, set it to undefined) —
update both occurrences where notebookId is computed (the notebookId assignment
used with generateSnapshotFilename around the current block and the similar
block at the later occurrence referenced in the comment) so filenames and lookup
parsing remain consistent.

---

Duplicate comments:
In `@packages/cli/src/commands/split.test.ts`:
- Around line 203-227: Add a new test in packages/cli/src/commands/split.test.ts
that mirrors the existing "should print a warning..." case but for the success
path: use createMultiNotebookFile and serializeDeepnoteFile to write the input
deepnote file, create a valid snapshot file (named like
my-project_<projectId>_latest.snapshot.deepnote) containing a properly
serialized per-notebook snapshot payload, call createSplitAction(program) and
await action(inputPath, {}), then assert that per-notebook snapshot files (e.g.,
project-dashboard.snapshot.deepnote and project-data.snapshot.deepnote) exist in
tempDir (use fs.access or similar) and that consoleSpy output does not contain
the failure warning; reference createMultiNotebookFile, createSplitAction,
serializeDeepnoteFile and consoleSpy when locating where to add the test.

In `@packages/cli/src/utils/output-persistence.test.ts`:
- Around line 125-168: Add a test for the multi-notebook fallback branch of
getSnapshotPath: use loadTestFile() to get a file, mutate file.project.notebooks
to have length > 1 (e.g., push another notebook or set an array with two
notebook entries), choose a sourcePath like '/path/to/project.deepnote', call
getSnapshotPath(sourcePath, file) and assert the returned path uses the legacy
multi-notebook filename format "{slug}_{projectId}_latest.snapshot.deepnote"
(i.e., no notebook id in the filename) while still being resolved under
resolve('/path/to', 'snapshots', ...); reference getSnapshotPath and
file.project.notebooks to locate the code path to exercise.

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

Inline comments:
In `@skills/deepnote/SKILL.md`:
- Around line 116-124: The "Location" example in SKILL.md must show the
single-notebook filename pattern so readers don't think {notebookId} is
optional; update the example block to include both patterns by adding an example
using `{slug}_{projectId}_{notebookId}_{timestamp}.snapshot.deepnote` alongside
the existing legacy `{slug}_{projectId}_{timestamp}.snapshot.deepnote`, and
ensure the explanatory text references the `{notebookId}` token explicitly so
users see when to use each format.

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

Inline comments:
In `@packages/cli/src/commands/split.test.ts`:
- Around line 236-261: Add a new test that mirrors the existing failure case but
uses a valid snapshot so we can assert successful per-notebook snapshot files
are created: create a test (e.g., "should split a valid snapshot into
per-notebook snapshots") that uses createMultiNotebookFile and writes the
project.deepnote via serializeDeepnoteFile to inputPath, create snapshotsDir and
write a valid snapshot file named using the same pattern
(`my-project_${projectId}_latest.snapshot.deepnote`) containing a properly
serialized snapshot (use the test helper you have for snapshots, analogous to
serializeDeepnoteFile), invoke the split action via createSplitAction(program)
and then assert that per-notebook snapshot files (e.g., names derived from the
notebook slugs like project-dashboard_<projectId>_latest.snapshot.deepnote and
project-data_...snapshot.deepnote) exist in snapshotsDir and contain the
expected snapshot content; reference createSplitAction, createMultiNotebookFile,
serializeDeepnoteFile, tempDir, snapshotsDir and projectId to locate where to
change/add the test.

In `@packages/cli/src/commands/split.ts`:
- Around line 93-101: The code directly reads initSnapshot.project.notebooks[0]
and mainSnapshot.project.notebooks[0] which can be undefined if either notebooks
array is empty; update the logic in the split handler to defensively check that
initSnapshot.project.notebooks and mainSnapshot.project.notebooks are non-empty
before assigning initNb and mainNb (e.g., validate array length or presence),
and if either is empty, handle it explicitly (throw a clear error or fallback)
before constructing nbSnapshot so nbSnapshot.project.notebooks never contains
undefined entries.

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

Inline comments:
In `@packages/mcp/src/tools/execution.ts`:
- Around line 268-275: Existing code duplicates notebook-aware snapshot filename
logic; extract a shared helper that centralizes slug/notebook/timestamp filename
generation and path resolution so other modules can reuse it. Create a function
(e.g., buildSnapshotPaths or makeSnapshotFilenames) that takes the inputs used
here (file, slug, snapshotDir, timing/finishedAt) and internally calls
resolveSnapshotNotebookId and generateSnapshotFilename to produce both the
timestamped and latest filenames and their resolved paths
(timestampedSnapshotPath, snapshotPath), keeping the timestamp formatting logic
identical (new Date(...).toISOString().replace(/[:.]/g, '-').slice(0, 19)); then
replace the duplicated block in this file to call the new helper and update
other places (where the same logic exists) to use it too so future
snapshot-format changes are made in one spot.

---

Duplicate comments:
In `@packages/cli/src/utils/output-persistence.test.ts`:
- Around line 270-333: The test suite is missing a case for the legacy
multi-notebook path when resolveSnapshotNotebookId(...) returns undefined; add a
new test (in packages/cli/src/utils/output-persistence.test.ts) that constructs
a DeepnoteFile whose project either has 3+ notebooks or has 2 notebooks but no
initNotebookId, call saveExecutionSnapshot(sourcePath, file, outputs, timing)
and assert the returned result.snapshotPath and result.timestampedSnapshotPath
follow the legacy pattern (contain `_{projectId}_latest.snapshot.deepnote` or
the `{slug}_{projectId}_latest.snapshot.deepnote` shape). Use the same
sourcePath/outputs/timing variables pattern as the existing test so the new test
verifies the fallback branch in saveExecutionSnapshot and the behavior of
resolveSnapshotNotebookId.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 205-213: The preflight plumbing is using initNotebookName to key
the init notebook scope which can collide when names aren't unique; update all
places that reference or store initNotebookName to use initNotebookId instead so
the init scope is keyed by the unique identifier. Search for uses of
initNotebookName (including where scope/inputs/dry-run/requirement checks are
built) and replace them with initNotebookId, ensuring variables, function
parameters and any maps/lookup keys (e.g., where the init notebook is added to
scope, listed in --list-inputs, included in dry-run plans, and in requirement
checks) are switched to use initNotebookId and that initNotebookName is only
retained for diagnostics where uniqueness is not required. Verify
functions/methods that accept the notebook identifier (references to
initNotebookId) are passed the id and update any validation or filter logic that
previously compared names to compare ids instead.

In `@packages/cli/src/commands/split.ts`:
- Around line 125-126: The snapshot writes (creating snapshotPath via join and
calling fs.writeFile with serializeDeepnoteSnapshot(nbSnapshot)) always use the
default write mode and ignore the CLI --force option; update the fs.writeFile
calls in split.ts (the ones that write snapshotPath and the other write at lines
~141-142) to respect the force flag by passing write options: use
exclusive-write mode when force is false (e.g., flag 'wx') and normal overwrite
when force is true (flag 'w'), so the writes will error if a snapshot exists
unless --force was provided.

In `@packages/convert/src/load-runnable-file.ts`:
- Around line 78-85: The .deepnote branch in loadRunnableFile currently catches
fs.readFile errors and throws a new LoadRunnableFileError, which breaks the
contract that filesystem errors are propagated; modify the catch in the ext ===
'.deepnote' block (around rawBytes/absolutePath) to rethrow the original
readError (or remove the try/catch) instead of wrapping it in
LoadRunnableFileError so callers receive the original errno/code details.

In `@packages/convert/src/snapshot/resolve-init.ts`:
- Around line 345-350: The normalization in areSettingsEqual currently treats
undefined/null and {} differently; update the norm function so that undefined,
null, and an empty plain object ({}) all normalize to the same sentinel (e.g.,
'null') before JSON.stringify with sortedKeysReplacer. Concretely, inside
areSettingsEqual's norm, check if value is undefined or null or (typeof value
=== 'object' && value !== null && Object.keys(value).length === 0) and return
'null' in those cases, otherwise return JSON.stringify(value,
sortedKeysReplacer).

In `@packages/mcp/src/tools/execution.ts`:
- Around line 322-333: The code currently maps a notebookFilter match back to
engineNotebookName (string) which loses the exact-ID selection; instead, when
notebookFilter matched by id (i.e., found.id === notebookFilter or original
filter looked like an id), do not collapse to name—leave notebookName undefined
and include the exact id in preludeNotebookIds (use targetNotebookId / found.id)
so engine.runProject receives the precise notebook id via preludeNotebookIds;
alternatively, if you must use notebookName, detect duplicate notebook names in
file.project.notebooks and throw/return an error when duplicates exist to avoid
ambiguous selection. Ensure changes touch the variables engineNotebookName,
targetNotebookId, and the call to engine.runProject (preludeNotebookIds /
notebookName) so selection is unambiguous.

In `@skills/deepnote/SKILL.md`:
- Around line 274-287: The Markdown table has a long unwrapped cell (the
"deepnote split <path>" row) causing Prettier/formatting failures; fix by
reflowing that long description into shorter lines or run Prettier to auto-wrap
the table so the row fits within line-length limits, ensuring the table syntax
remains valid and all pipes/columns (e.g., the "deepnote split <path>" row)
align with the other rows.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/convert/src/snapshot/split.ts`:
- Around line 54-61: The filename builder generateSnapshotFilename currently
sanitizes slug, projectId and notebookId but interpolates timestamp raw; update
generateSnapshotFilename to pass timestamp through sanitizeFilenameComponent
(e.g. const safeTimestamp = sanitizeFilenameComponent(timestamp)) and use
safeTimestamp in both the notebookId and non-notebookId return branches so the
produced basename cannot contain traversal or unsafe characters while preserving
existing sanitization of slug/projectId/notebookId.

---

Duplicate comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 535-550: The preflight block validation uses
getNotebooksForExecutionScope with additionalNotebookNames and can pick a
same-named non-init notebook; instead thread the init notebook identity
(initNotebookId) into the scope resolution so the preflight scope matches the
engine's execution scope instead of relying on initNotebookName. Update the
call-site around options.block && initBlockIds to pass initNotebookId to
getNotebooksForExecutionScope (and any downstream helpers it calls) and keep
initNotebookName only for logging/diagnostics; ensure
assertExecutableBlockExists is invoked with the notebook list resolved using
initNotebookId so the dry-run/preflight and engine use the same notebook-keyed
scope.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/cli/src/commands/run.test.ts`:
- Around line 1910-2383: Add a new test case to cover the regression where a
non-init notebook shares the same name as the init notebook: create files via
makeInitFile()/makeMainFile() plus an extra sibling notebook file whose
notebook.name equals the init notebook's name, call action(mainPath, { notebook:
'Main' }) (or the appropriate invocation used in similar tests), then assert
mockRunProject was called once and that the runOptions.preludeNotebookIds is a
Set containing only INIT_NB_ID (and that the project's notebooks order remains
[INIT_NB_ID, MAIN_NB_ID]); place this alongside the other "sibling init
resolution" tests so it exercises the resolver and preflight logic (referencing
action, mockRunProject, and runOptions.preludeNotebookIds).

In `@packages/cli/src/commands/split.test.ts`:
- Around line 323-467: Add two tests around createSplitAction: one asserting
that when a snapshot file already exists the split action does not overwrite it
by default (createSplitAction(program); call action(inputPath, {}) and then
verify the original snapshot file content remains unchanged and no new snapshot
replace occurs), and a second asserting that when invoked with force: true (call
action(inputPath, { force: true }) or the equivalent option) the existing
snapshot is overwritten (write an identifiable original snapshot to snapshotsDir
before running, run action, then assert the snapshot file content has changed to
the new split output). Reference createSplitAction, action(inputPath, {}),
action(inputPath, { force: true }), snapshotsDir, and the snapshot filenames to
locate where to add these assertions.

In `@packages/mcp/src/tools/execution.ts`:
- Around line 524-533: The current composition always expands initBlockIds into
effectiveBlockIds even when the requested targetBlock sits in the same init
notebook, which causes all executable init blocks to run; update the logic
around effectiveBlockIds (and the engine.runProject preludeNotebookIds and the
dry-run branch) to only compose and include initBlockIds when initNotebookId !==
targetNotebook.id (i.e., gate the prelude on targetNotebook.id !==
initNotebookId) so that if the target block is inside the init notebook we pass
only that blockId, not the whole init notebook's executable blocks; apply the
same conditional in the dry-run branch to mirror the behavior.

---

Duplicate comments:
In `@packages/cli/src/commands/run.ts`:
- Around line 346-349: The code calls collectRequiredIntegrationIds(file,
options.notebook, { additionalNotebookNames: initNotebookName ?
[initNotebookName] : [] }) which passes the init/prelude notebook by name
(initNotebookName) causing ambiguous scope when multiple notebooks share a name;
replace name-based usage with ID-based propagation: compute the init/prelude
notebook ID (use preludeNotebookIds or resolve initNotebookName to its ID
earlier) and pass it via additionalNotebookIds to collectRequiredIntegrationIds
and all related flows (scope selection, input listing, validation, integration
collection), keep notebook names only for display; update the other occurrences
that pass additionalNotebookNames (the blocks referenced around the other
ranges) to use additionalNotebookIds consistently.

In `@packages/cli/src/commands/split.ts`:
- Line 149: The summary message uses existingSnapshots.length but does not
account for failures; change the split logic to count only successful splits
(e.g., introduce a successfulSplits counter updated inside the block that
performs each split) and then replace existingSnapshots.length with that counter
in the output call (the line calling output(`\n  ${c.dim(`Split
${existingSnapshots.length} snapshot(s) into ${snapshotDir}`)}`)). Ensure the
counter is initialized before processing, incremented only on confirmed success
(not on warnings/errors), and used in the final output so the message reflects
partial failures accurately.
- Line 117: The snapshot writes currently call fs.writeFile(snapshotPath,
serializeDeepnoteSnapshot(...), 'utf-8') and always clobber files; update both
occurrences (the writes using snapshotPath and serializeDeepnoteSnapshot at
lines noted) to honor the CLI --force flag (likely exposed as a boolean like
force or flags.force). Use write options: if force is true use 'w' (or default)
else use { encoding: 'utf-8', flag: 'wx' } so the write will fail if the file
exists, and catch EEXIST to surface a friendly error/exit; ensure both write
sites are changed consistently and any error handling uses the same message.

In `@packages/convert/src/load-runnable-file.ts`:
- Line 43: The .deepnote read currently wraps read failures in
LoadRunnableFileError, breaking the stated contract that filesystem errors
should be propagated unwrapped; in the loadRunnableFile implementation, update
the .deepnote file-read path (the block that currently catches errors at lines
handling ".deepnote") to stop catching and rewrapping raw fs errors—only convert
non-fs/content-validation errors to LoadRunnableFileError (or rethrow after
inspecting error type), and let native fs errors bubble up unchanged so
.deepnote behaves consistently with .ipynb/.py/.qmd; reference
LoadRunnableFileError and the .deepnote read block when making this change.

In `@packages/convert/src/snapshot/split.ts`:
- Around line 33-50: The timestamp parameter is not sanitized causing
path-traversal when interpolated into the filename; update
generateSnapshotFilename to pass timestamp through sanitizeFilenameComponent
(use sanitizeFilenameComponent(timestamp || 'latest')) before building the
basename so the returned string always uses safeSlug, safeProjectId, sanitized
notebookId and sanitized timestamp (preserving the .snapshot.deepnote suffix) to
prevent directory escape; reference generateSnapshotFilename and
sanitizeFilenameComponent when making the change.

In `@packages/mcp/src/tools/execution.ts`:
- Around line 237-250: The selection logic for notebookFilter uses find(...) and
then later relies on engine.runProject filtering by name, which causes ambiguity
when multiple notebooks share the same name; update the block that computes
notebooks (variables: notebookFilter, found, initNotebook,
file.project.notebooks) to detect when notebookFilter matched by name but more
than one notebook has that name and fail fast with an error asking for the
notebook ID (or require using an ID match), instead of proceeding; implement the
same uniqueness guard in the analogous selection code later (the other notebooks
filtering block) so we never pass an ambiguous name to engine.runProject.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli/src/commands/analyze.ts`:
- Around line 367-370: The exit-code mapping in analyze.ts should treat
ParseError the same as FileResolutionError and InitNotebookResolutionError,
since loadAndResolveDeepnoteFile can fail with ParseError on malformed .deepnote
files. Update the error instanceof check around the exitCode assignment in
analyze.ts to include ParseError so these failures return ExitCode.InvalidUsage,
and make the same adjustment in dag.ts to keep both command handlers consistent.