Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/notebook-create-action.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": minor
---

Let the notebook tools create a new empty notebook.
72 changes: 59 additions & 13 deletions packages/kilo-vscode/src/services/notebook/adapter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from "node:path"
import * as vscode from "vscode"
import { normalizeOutputs, normalizeSource } from "./output"
import { NotebookError, resolveNotebookPath, type NotebookPathDeps } from "./path"
import { NotebookError, resolveNotebookCreatePath, resolveNotebookPath, type NotebookPathDeps } from "./path"
import { cellFingerprint, fingerprint, notebookState, sameCell, type NotebookState } from "./revision"
import {
NOTEBOOK_LIMITS,
Expand All @@ -21,6 +21,18 @@ const RETAINED_REVISIONS = 1_000
const revisions = new Map<string, NotebookState>()
const locks = new Map<string, Promise<void>>()

// Minimal valid empty Jupyter notebook accepted by the built-in ipynb serializer.
const EMPTY_IPYNB = JSON.stringify(
{
cells: [],
metadata: {},
nbformat: 4,
nbformat_minor: 5,
},
null,
1,
)

function revisionKey(target: string, revision: string): string {
return `${target}\0${revision}`
}
Expand Down Expand Up @@ -60,6 +72,7 @@ function defaults(): NotebookAdapterDeps {
return {
documents: () => vscode.workspace.notebookDocuments,
open: (uri) => Promise.resolve(vscode.workspace.openNotebookDocument(uri)),
write: (uri, content) => Promise.resolve(vscode.workspace.fs.writeFile(uri, content)),
apply: (edit) => Promise.resolve(vscode.workspace.applyEdit(edit)),
execute: (command, ...args) => Promise.resolve(vscode.commands.executeCommand(command, ...args)),
change: (listener) => vscode.workspace.onDidChangeNotebookDocument(listener),
Expand Down Expand Up @@ -167,12 +180,23 @@ export class NotebookAdapter {
}

async edit(request: NotebookEditRequest): Promise<NotebookEditResult> {
const edit = request.edit
if (edit.action === "create") {
return this.create(request)
}
const loaded = await this.document(request.directory, request.path)
return this.lock(loaded.target, async () => {
const before = this.remember(loaded.document, loaded.target)
this.revision(before, request.expectedRevision, loaded.path, request.index)
const expectedRevision = request.expectedRevision
if (expectedRevision === undefined) {
throw new NotebookError("invalid_cell", "An expected revision is required for this edit", {
path: loaded.path,
index: request.index,
})
}
this.revision(before, expectedRevision, loaded.path, request.index)
const count = loaded.document.cellCount
const max = request.edit.action === "insert" ? count : count - 1
const max = edit.action === "insert" ? count : count - 1
if (!Number.isInteger(request.index) || request.index < 0 || request.index > max) {
throw new NotebookError("invalid_cell", `Cell index ${request.index} is out of range`, {
path: loaded.path,
Expand All @@ -182,25 +206,25 @@ export class NotebookAdapter {

const expected = [...before.cells]
const edits = (() => {
if (request.edit.action === "delete") {
if (edit.action === "delete") {
expected.splice(request.index, 1)
return [this.deps.delete(request.index)]
}
const language = request.edit.language ?? (request.edit.kind === "code" ? "plaintext" : "markdown")
const language = edit.language ?? (edit.kind === "code" ? "plaintext" : "markdown")
const cell = this.deps.cell({
kind: request.edit.kind,
language: request.edit.language,
source: request.edit.source,
kind: edit.kind,
language: edit.language,
source: edit.source,
})
const value = fingerprint(request.edit.kind, language, request.edit.source)
if (request.edit.action === "insert") {
const value = fingerprint(edit.kind, language, edit.source)
if (edit.action === "insert") {
expected.splice(request.index, 0, value)
return [this.deps.insert(request.index, [cell])]
}
expected.splice(request.index, 1, value)
return [this.deps.replace(request.index, [cell])]
})()
this.revision(this.remember(loaded.document, loaded.target), request.expectedRevision, loaded.path, request.index)
this.revision(this.remember(loaded.document, loaded.target), expectedRevision, loaded.path, request.index)
if (!(await this.deps.apply(this.deps.edit(loaded.document.uri, edits)))) {
throw new NotebookError("unsupported", "VS Code rejected the notebook edit", {
path: loaded.path,
Expand All @@ -217,15 +241,37 @@ export class NotebookAdapter {
requestPath: request.path,
revision: after.revision,
index: request.index,
action: request.edit.action,
action: edit.action,
}
if (request.edit.action !== "delete" && request.index < loaded.document.cellCount) {
if (edit.action !== "delete" && request.index < loaded.document.cellCount) {
result.cell = this.cell(loaded.document.cellAt(request.index), request.index)
}
return result
})
}

private async create(request: NotebookEditRequest): Promise<NotebookEditResult> {
const resolved = await resolveNotebookCreatePath(request.directory, request.path, this.access, this.options.paths)
return this.lock(resolved.target, async () => {
await this.deps.write(this.deps.uri(resolved.target), new TextEncoder().encode(EMPTY_IPYNB))
Comment thread
markijbema marked this conversation as resolved.
const document = await this.deps.open(this.deps.uri(resolved.target))
if (document.isClosed) {
throw new NotebookError("closed", `Notebook ${JSON.stringify(resolved.relative)} is closed`, {
path: resolved.relative,
})
}
const state = this.remember(document, resolved.target)
return {
operation: "edit",
path: resolved.relative,
requestPath: request.path,
revision: state.revision,
index: 0,
action: "create",
}
})
}

async execute(request: NotebookExecuteRequest): Promise<NotebookExecuteResult> {
const loaded = await this.document(request.directory, request.path)
const state = this.remember(loaded.document, loaded.target)
Expand Down
2 changes: 1 addition & 1 deletion packages/kilo-vscode/src/services/notebook/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export class NotebookBridge {
return adapter.edit({
path: request.path,
directory,
expectedRevision: request.expectedRevision,
...(request.expectedRevision !== undefined ? { expectedRevision: request.expectedRevision } : {}),
index: request.index,
edit: request.edit,
})
Expand Down
50 changes: 50 additions & 0 deletions packages/kilo-vscode/src/services/notebook/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,53 @@ export async function resolveNotebookPath(
}
return { target, relative: path.relative(root, target).split(path.sep).join("/") }
}

// Resolve a path for a notebook that does not exist yet. The file itself is not
// realpathed (it must not exist); instead the parent directory is realpathed and
// must be contained in the request root and pass access checks.
export async function resolveNotebookCreatePath(
directory: string,
input: string,
access: NotebookAccess,
deps: NotebookPathDeps = defaults,
): Promise<NotebookPath> {
if (!input || input.length > 4_096 || input.includes("\0")) {
throw invalid(input, "the path is empty, too long, or malformed")
}
if (WINDOWS_ABSOLUTE.test(input) && !path.win32.isAbsolute(directory)) {
throw invalid(input, "the absolute path uses a different platform format")
}
if (!input.toLowerCase().endsWith(".ipynb")) {
throw invalid(input, "only .ipynb notebooks can be created")
}

const base = path.resolve(directory)
const root = await deps.realpath(base)
const candidate = path.resolve(base, input)
if (!path.isAbsolute(input) && !contained(base, candidate)) {
throw invalid(input, "it is outside the request directory")
}

const parent = await deps.realpath(path.dirname(candidate)).catch((error: unknown) => {
const detail = error instanceof Error ? error.message : String(error)
throw new NotebookError("not_found", `Cannot resolve the parent directory of ${JSON.stringify(input)}: ${detail}`, {
path: input,
})
})
if (!contained(root, parent)) {
throw invalid(input, "its parent directory resolves outside the request directory")
}

const target = path.join(parent, path.basename(candidate))
const existing = await deps.realpath(target).then(
Comment thread
markijbema marked this conversation as resolved.
() => true,
() => false,
)
if (existing) {
throw new NotebookError("already_exists", `Notebook ${JSON.stringify(input)} already exists`, { path: input })
}
if (!(await access.validateAccess(target))) {
throw invalid(input, "it is excluded by workspace access or ignore rules")
}
return { target, relative: path.relative(root, target).split(path.sep).join("/") }
}
6 changes: 4 additions & 2 deletions packages/kilo-vscode/src/services/notebook/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export interface NotebookEditResult {
requestPath: string
revision: string
index: number
action: "insert" | "replace" | "delete"
action: "insert" | "replace" | "delete" | "create"
cell?: NotebookCell
}

Expand All @@ -82,6 +82,7 @@ export type NotebookEdit =
| ({ action: "insert" } & NotebookCellInput)
| ({ action: "replace" } & NotebookCellInput)
| { action: "delete" }
| { action: "create" }

export interface NotebookReadRequest {
path: string
Expand All @@ -92,7 +93,7 @@ export interface NotebookReadRequest {
export interface NotebookEditRequest {
path: string
directory: string
expectedRevision: string
expectedRevision?: string
index: number
edit: NotebookEdit
}
Expand All @@ -113,6 +114,7 @@ export interface NotebookAccess {
export interface NotebookAdapterDeps {
documents(): readonly vscode.NotebookDocument[]
open(uri: vscode.Uri): Promise<vscode.NotebookDocument>
write(uri: vscode.Uri, content: Uint8Array): Promise<void>
apply(edit: vscode.WorkspaceEdit): Promise<boolean>
execute(command: string, ...args: unknown[]): Promise<unknown>
change(listener: (event: vscode.NotebookDocumentChangeEvent) => void): vscode.Disposable
Expand Down
77 changes: 74 additions & 3 deletions packages/kilo-vscode/tests/unit/notebook-core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,23 @@ function notebook(cells: vscode.NotebookCell[], file = "/repo/book.ipynb"): vsco
function harness(document: vscode.NotebookDocument, cells: vscode.NotebookCell[]) {
const changes = new Set<(event: vscode.NotebookDocumentChangeEvent) => void>()
const closes = new Set<(document: vscode.NotebookDocument) => void>()
const calls = { open: 0, apply: 0, command: 0, commandArgs: [] as unknown[], edit: undefined as unknown }
const calls = {
open: 0,
apply: 0,
command: 0,
commandArgs: [] as unknown[],
edit: undefined as unknown,
write: [] as Array<{ uri: vscode.Uri; content: Uint8Array }>,
}
const deps: NotebookAdapterDeps = {
documents: () => [document],
open: async () => {
calls.open++
return document
},
write: async (uri, content) => {
calls.write.push({ uri, content })
},
apply: async (edit) => {
calls.apply++
const item = (
Expand Down Expand Up @@ -119,12 +129,12 @@ function harness(document: vscode.NotebookDocument, cells: vscode.NotebookCell[]
const paths = { realpath: async (value: string) => value }
const access = { validateAccess: mock(() => true) }

function adapter(items: ReturnType<typeof cell>[], file = "/repo/book.ipynb") {
function adapter(items: ReturnType<typeof cell>[], file = "/repo/book.ipynb", resolver = paths) {
const cells = items.map((item) => item.value)
const document = notebook(cells, file)
const ctx = harness(document, cells)
return {
adapter: new NotebookAdapter(access, { deps: ctx.deps, paths, timeout: 50 }),
adapter: new NotebookAdapter(access, { deps: ctx.deps, paths: resolver, timeout: 50 }),
document,
cells,
...ctx,
Expand Down Expand Up @@ -467,3 +477,64 @@ describe("notebook adapter", () => {
])
})
})

describe("notebook create", () => {
// The new file must not resolve (it does not exist), but its parent directory must.
const creating = {
realpath: async (value: string) => (value.endsWith("fresh.ipynb") ? Promise.reject(new Error("ENOENT")) : value),
}

it("writes a minimal empty .ipynb, opens it, and returns the initial revision", async () => {
const ctx = adapter([], "/repo/fresh.ipynb", creating)
const result = await ctx.adapter.edit({
directory: "/repo",
path: "fresh.ipynb",
index: 0,
edit: { action: "create" },
})
expect(result).toMatchObject({ operation: "edit", action: "create", path: "fresh.ipynb", index: 0 })
expect(result.revision).toContain("content:")
expect(ctx.calls.write).toHaveLength(1)
expect(ctx.calls.open).toBe(1)
const written = JSON.parse(new TextDecoder().decode(ctx.calls.write[0]!.content))
expect(written).toMatchObject({ cells: [], nbformat: 4 })
})

it("rejects creating a notebook that already exists", async () => {
const ctx = adapter([cell()], "/repo/book.ipynb")
await expect(
ctx.adapter.edit({ directory: "/repo", path: "book.ipynb", index: 0, edit: { action: "create" } }),
).rejects.toMatchObject({ code: "already_exists", path: "book.ipynb" })
expect(ctx.calls.write).toHaveLength(0)
})

it("rejects a missing parent directory with not_found", async () => {
const missing = {
realpath: async (value: string) => (value === "/repo" ? value : Promise.reject(new Error("ENOENT"))),
}
const ctx = adapter([], "/repo/missing/fresh.ipynb", missing)
await expect(
ctx.adapter.edit({ directory: "/repo", path: "missing/fresh.ipynb", index: 0, edit: { action: "create" } }),
).rejects.toMatchObject({ code: "not_found" })
expect(ctx.calls.write).toHaveLength(0)
})

it("rejects non-.ipynb create targets", async () => {
const ctx = adapter([], "/repo/notes.txt", creating)
await expect(
ctx.adapter.edit({ directory: "/repo", path: "notes.txt", index: 0, edit: { action: "create" } }),
).rejects.toMatchObject({ code: "invalid_path" })
expect(ctx.calls.write).toHaveLength(0)
})

it("rejects create targets excluded by access rules", async () => {
const guard = { validateAccess: mock(() => false) }
const document = notebook([], "/repo/fresh.ipynb")
const ctx = harness(document, [])
const core = new NotebookAdapter(guard, { deps: ctx.deps, paths: creating, timeout: 50 })
await expect(
core.edit({ directory: "/repo", path: "fresh.ipynb", index: 0, edit: { action: "create" } }),
).rejects.toMatchObject({ code: "invalid_path" })
expect(ctx.calls.write).toHaveLength(0)
})
})
8 changes: 6 additions & 2 deletions packages/opencode/src/kilocode/notebook/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,15 @@ const CellEdit = {
export const EditRequest = Schema.Struct({
...Base,
operation: Schema.Literal("edit"),
expectedRevision: Revision,
expectedRevision: Schema.optional(Revision).annotate({
description: "Required for insert, replace, and delete; omitted for create, which has no prior revision",
}),
index: Index,
edit: Schema.Union([
Schema.Struct({ action: Schema.Literal("insert"), ...CellEdit }),
Schema.Struct({ action: Schema.Literal("replace"), ...CellEdit }),
Schema.Struct({ action: Schema.Literal("delete") }),
Schema.Struct({ action: Schema.Literal("create") }),
]),
}).annotate({ identifier: "NotebookEditRequest" })

Expand Down Expand Up @@ -114,7 +117,7 @@ export const EditResult = Schema.Struct({
requestPath: Path,
revision: Revision,
index: Index,
action: Schema.Literals(["insert", "replace", "delete"]),
action: Schema.Literals(["insert", "replace", "delete", "create"]),
cell: Schema.optional(Cell),
}).annotate({ identifier: "NotebookEditResult" })

Expand All @@ -141,6 +144,7 @@ export const Result = Schema.Union([ReadResult, EditResult, ExecuteResult]).anno
export type Result = Schema.Schema.Type<typeof Result>

export const ErrorCode = Schema.Literals([
"already_exists",
Comment thread
markijbema marked this conversation as resolved.
"cancelled",
"closed",
"disconnected",
Expand Down
Loading
Loading