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
31 changes: 28 additions & 3 deletions canvas/server/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,16 @@ export function createAgentServer(options: {
examplesDir: string;
/** Every project by the name its address carries, `/p/<name>/`. */
projects: () => Map<string, string>;
/** Moves a project the agent has just named into a folder of that name (projects.ts). */
named: (dir: string) => string | undefined;
/** A project by name, or by the name it had before `named` moved it. */
project: (name: string) => string | undefined;
/** This plugin's checkout, whose skills the sessions are given. */
repoRoot: string;
/** `<projects dir>/.workspaces`: a folder per session, a record beside each, and the skills. */
workspaces: string;
}) {
const { examplesDir, projects, repoRoot, workspaces } = options;
const { examplesDir, named, project: projectDir, projects, repoRoot, workspaces } = options;
// One process per message — Claude Code or Codex, by the panel's choice, looked up in
// agents.ts — its output kept here and streamed to the page. The runs are held in memory, the
// newest twenty, for the panel to follow and for the history to say which session is running,
Expand All @@ -68,6 +72,8 @@ export function createAgentServer(options: {
child: ChildProcess;
/** The Stop button was pressed. Windows ends a run by exit code 1, which says nothing. */
stopped: boolean;
/** The project it works on, if any. */
dir: string | undefined;
}
>();

Expand Down Expand Up @@ -368,7 +374,7 @@ export function createAgentServer(options: {
// The project it was sent from, by the name its address carries. None from the home page
// or an example, and then the agent has no project to write to.
const dir =
project === undefined ? undefined : projects().get(project);
project === undefined ? undefined : projectDir(project);
if (project !== undefined && dir === undefined)
return send(404, "no such project");
// The session the panel is in, or a new one. Its id names a folder and a file, so it has
Expand Down Expand Up @@ -562,11 +568,12 @@ export function createAgentServer(options: {
// server's port.
env: {
...process.env,
SP_PROJECT: project,
SP_PROJECT: dir && path.basename(dir),
SP_CANVAS_PORT: String(req.socket.localPort),
},
}),
stopped: false,
dir,
});
runs.set(run.id, run);
record.runs.push(run.id);
Expand Down Expand Up @@ -676,6 +683,24 @@ export function createAgentServer(options: {
finish(error.code === "ENOENT" ? def.missing : String(error));
});
run.child.on("close", (code, signal) => {
// The turn that named an unnamed project is over, so its folder can take the name.
// Not when another turn has already started on it: that one moves it when it ends.
if (
dir !== undefined &&
code === 0 &&
![...runs.values()].some((r) => r !== run && r.dir === dir && !ended(r))
) {
// Never in the way of the run ending: a throw here would leave it running for good.
try {
const moved = named(dir);
if (moved)
record.projects = record.projects.map((had) =>
had === dir ? moved : had,
);
} catch (error) {
console.error(`[agent] ${dir} could not be named: ${error}`);
}
}
settle();
if (ended(run)) return;
const tail = stderr.trim().split("\n").slice(-5).join("\n");
Expand Down
51 changes: 50 additions & 1 deletion canvas/server/projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { AddressInfo } from "node:net";
import os from "node:os";
import path from "node:path";
import { expect, it } from "vitest";
import { createProjectsServer, withoutFiles } from "./projects.ts";
import { createProjectsServer, namedFolder, withoutFiles } from "./projects.ts";

// One server for every project: `/` goes to the one opened, else home, each is at `/p/<name>/`, the
// root has the examples and no project, and a page of the server's own can make one. The folder
Expand Down Expand Up @@ -202,6 +202,18 @@ it("serves every project at its own address and makes new ones", async () => {
expect(
JSON.parse((await ask("/__sp/projects", { name: " " })).text).url,
).toBe(url);
expect(
JSON.parse(
fs.readFileSync(path.join(tmp, "projects/Untitled/project.json"), "utf8"),
).unnamed,
).toBe(true);
// Typed, "Untitled" is the person's name for it, so its folder is never renamed.
await ask("/__sp/projects", { name: "Untitled 9" });
expect(
JSON.parse(
fs.readFileSync(path.join(tmp, "projects/Untitled 9/project.json"), "utf8"),
).unnamed,
).toBeUndefined();
write("projects/Untitled/project.json", JSON.stringify({ name: "Gamma" }));
const titled = JSON.parse((await ask("/__sp/projects.json")).text);
expect(titled.find((p: any) => p.name === "Untitled").title).toBe("Gamma");
Expand Down Expand Up @@ -248,3 +260,40 @@ it("keeps a community board it draws off file: addresses, after its doctype", ()
);
expect(withoutFiles("<p>")).toMatch(/^<meta [^>]+><p>$/);
});

// A project made without a name takes the one its agent writes, and only then.
it("names an Untitled project's folder from its project.json", () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "sp-named-"));
const project = (folder: string, json: object) => {
fs.mkdirSync(path.join(tmp, folder));
fs.writeFileSync(
path.join(tmp, folder, "project.json"),
JSON.stringify({ unnamed: true, ...json }),
);
return path.join(tmp, folder);
};
expect(namedFolder(project("Untitled 10", { name: "Acme Inc. " }))).toBe(
path.join(tmp, "Acme Inc"),
);
expect(namedFolder(project("Untitled 3", { name: " Kasra " }))).toBe(
path.join(tmp, "Kasra"),
);
expect(namedFolder(project("Untitled", { format: 1 }))).toBeUndefined();
expect(namedFolder(project("Mine", { name: "Other" }))).toBeUndefined();
// An "Untitled" the person typed has no `unnamed`.
expect(
namedFolder(project("Untitled 11", { name: "Other", unnamed: undefined })),
).toBeUndefined();
expect(namedFolder(project("Untitled 2", { name: "Mine" }))).toBeUndefined();
expect(namedFolder(project("Untitled 4", { name: "a/b" }))).toBeUndefined();
expect(
namedFolder(project("Untitled 5", { name: ".hidden" })),
).toBeUndefined();
for (const [folder, name] of [
["Untitled 6", "a\\b"],
["Untitled 7", "a: b"],
["Untitled 8", "CON"],
["Untitled 9", 42],
] as const)
expect(namedFolder(project(folder, { name }))).toBeUndefined();
});
99 changes: 94 additions & 5 deletions canvas/server/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { createAgentServer } from "./agent.ts";
import { CANVASES, readJson } from "./boards.ts";
import {
createSpServer,
readProjectJson,
reveal,
SANDBOX,
sameOrigin,
Expand Down Expand Up @@ -109,6 +110,33 @@ export function loopbackHost(host: string | undefined) {
);
}

/**
* Where an "Untitled" project's folder goes once its agent has named it in project.json: a folder
* of that name beside it. Undefined for a folder the person named, "Untitled" included, a name no
* folder can have, and a name another project already has.
*/
export function namedFolder(dir: string) {
const { name: given, unnamed } = readProjectJson(dir);
// Held to what Windows allows on every platform, since a project is also opened there once
// shared: it drops a trailing dot or space, so they go here too, and these characters and device
// names cannot be a folder's.
const name =
typeof given === "string" ? given.trim().replace(/[. ]+$/, "") : "";
if (
unnamed !== true ||
!/^Untitled( \d+)?$/.test(path.basename(dir)) ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish user-named Untitled folders before renaming

When a person explicitly creates a project named Untitled or Untitled N, the server writes the same project.json as it does for a blank name, so this basename check later misclassifies that user-named folder as auto-generated. If its project.json already has a display name or gains one during any successful agent turn, the app unexpectedly moves the folder and changes its /p/<name>/ address despite the stated behavior that person-named folders remain fixed; persist whether the project was created unnamed and require that marker here.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 22ce6d1: a project made without a name gets "unnamed": true in project.json, and namedFolder requires it. A folder the person typed as Untitled N keeps its name.

!name ||
name.startsWith(".") ||
path.basename(name) !== name ||
// oxlint-disable-next-line no-control-regex
/[<>:"|?*\\\x00-\x1f]/.test(name) ||
/^(con|prn|aux|nul|com\d|lpt\d)(\..*)?$/i.test(name)
)
return undefined;
const to = path.join(path.dirname(dir), name);
return fs.existsSync(to) ? undefined : to;
}

export function createProjectsServer(options: {
/** Where every project is listed from, and where `POST /__sp/projects` makes one. */
projectsDir: string;
Expand Down Expand Up @@ -157,10 +185,53 @@ export function createProjectsServer(options: {
repoRoot,
});

// A project made without a name is an "Untitled" folder until its agent writes one into
// project.json (AppShell.tsx). Once that turn is over, with no process holding the old path,
// the folder takes the name, and the pages open on it follow (canvasIndex.ts). A name no folder
// can have, or one another project has, leaves it where it is. A folder the person named keeps
// its name whatever project.json says. Answers the new folder.
// A renamed project's old name, answered with its new folder: a page on the old address still
// saves its last edits there as it leaves, and a message sent as the turn ended names it.
const renamed = new Map<string, string>();
const project = (name: string) => {
const dir = projects().get(name) ?? renamed.get(name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep renamed aliases from being shadowed by reused names

After Untitled is renamed, the next unnamed project reuses that now-free folder name, and this lookup then selects the new folder before consulting the alias. Any stale /p/Untitled/ tab, bookmark, unload save, or chat request that the alias is intended to preserve will consequently open or write to the wrong project instead of the renamed one; reserve aliased names when creating projects or otherwise disambiguate stale requests.

AGENTS.md reference: AGENTS.md:L15-L18

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ef34d1b: a new Untitled project skips any name still held as an alias in renamed.

return dir !== undefined && fs.existsSync(dir) ? dir : undefined;
};
const named = (dir: string) => {
const to = namedFolder(dir);
if (to === undefined) return undefined;
const name = path.basename(to);
// Its server goes first: Windows will not move a folder that is being watched. A page asking
// again, after a move that failed, gets a new one.
const sp = sps.get(dir);
sps.delete(dir);
sp?.unwatch();
try {
fs.renameSync(dir, to);
Comment on lines +206 to +210

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drain active writes before renaming the project directory

On POSIX systems, if a /__sp/canvas-file upload has already entered this project's existing sp handler when the naming turn closes, this rename moves the directory and its open .part file while the handler still retains paths under the old directory. When the upload finishes, sp.ts tries to rename the vanished old .part path, returns 500, and leaves an orphaned partial file under the new directory; buffered canvas saves can similarly return 404. Track or drain in-flight writes, or make existing handlers resolve the moved path before performing the rename.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this one. It needs an upload to be in flight at the moment the naming turn exits, and even then the upload fails loudly rather than corrupting anything. I'll revisit it if it's ever seen.

} catch (error) {
console.error(
`[projects] ${dir} could not be renamed to “${name}”: ${error}`,
);
sp?.close();
return undefined;
}
renamed.set(path.basename(dir), to);
// Every open page, not only the project's: the window's tab for it moves whichever is in front.
const moved = {
from: `/p/${encodeURIComponent(path.basename(dir))}/`,
to: `/p/${encodeURIComponent(name)}/`,
};
for (const other of [root, sp, ...sps.values()]) other?.moved(moved);
sp?.close();
return to;
};

// The agent, once for the whole server: its sessions and the skills they read are kept in a dot
// folder of the projects directory, which the list above skips.
const agent = createAgentServer({
examplesDir,
named,
project,
projects,
repoRoot,
workspaces: path.join(projectsDir, ".workspaces"),
Expand Down Expand Up @@ -312,9 +383,15 @@ export function createProjectsServer(options: {
// "Untitled", and its agent names it (sp.ts, PROJECT_JSON). It goes under the projects
// folder, so there is no place to pick. The checks are the ones the dialog cannot make.
let name = typeof parsed.name === "string" ? parsed.name.trim() : "";
if (name === "") {
const unnamed = name === "";
if (unnamed) {
name = "Untitled";
for (let n = 2; fs.existsSync(path.join(projectsDir, name)); n++)
// Nor a name a renamed project had, which still answers for it (`renamed`).
for (
let n = 2;
fs.existsSync(path.join(projectsDir, name)) || renamed.has(name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject explicit names retained as aliases

Although the new reservation skips aliases while auto-generating an empty name, it only runs inside the name === "" branch, so an explicit request for Untitled after that folder was renamed can still recreate the aliased name. Because project() checks the real folder map before renamed, stale /p/Untitled/ saves or messages will then target the newly created project and can overwrite the wrong data; reject explicit names present in renamed as well. This remaining explicit-name path is fresh evidence after the earlier alias finding's auto-generated case was fixed.

AGENTS.md reference: AGENTS.md:L15-L18

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 22ce6d1: a typed name still held in renamed gets the same 409 as an existing folder.

n++
)
name = `Untitled ${n}`;
}
if (name.startsWith(".") || path.basename(name) !== name)
Expand All @@ -323,7 +400,8 @@ export function createProjectsServer(options: {
"A name cannot start with a dot or have a slash in it.",
);
const dir = path.join(projectsDir, name);
if (fs.existsSync(dir))
// A renamed project's old name is still its own (`renamed`).
if (fs.existsSync(dir) || renamed.has(name))
return send(
409,
`You already have a project called “${name}”. Try another name.`,
Expand All @@ -334,9 +412,10 @@ export function createProjectsServer(options: {
// nothing to copy in.
fs.mkdirSync(path.join(dir, CANVASES), { recursive: true });
// Its id is what a package of it is known by, whatever the folder is renamed to.
// `unnamed` marks the folder as one the app named, so its agent's name can replace it.
fs.writeFileSync(
path.join(dir, "project.json"),
`${JSON.stringify({ format: PROJECT_FORMAT, id: crypto.randomUUID() }, null, 2)}\n`,
`${JSON.stringify({ format: PROJECT_FORMAT, id: crypto.randomUUID(), ...(unnamed && { unnamed }) }, null, 2)}\n`,
);
} catch (e) {
// A new name does not fix an unwritable Documents, so say what failed.
Expand Down Expand Up @@ -441,12 +520,22 @@ export function createProjectsServer(options: {
if (rest === undefined) return root.handle(req, res, next);
let dir: string | undefined;
try {
dir = projects().get(decodeURIComponent(name));
dir = project(decodeURIComponent(name));
} catch {} // a broken escape is no project's name
if (dir === undefined) {
res.statusCode = 404;
return res.end("no such project");
}
// A page asked for by the name the project had, from a card or a link made before it moved:
// sent to its address now, so it opens once, under one tab.
if (!projects().has(decodeURIComponent(name)) && req.method === "GET") {
res.statusCode = 302;
res.setHeader(
"location",
`/p/${encodeURIComponent(path.basename(dir))}${rest}`,
);
return res.end();
}
// Made by a newer app, which may keep it in a way this one would misread, and then write back.
const format = (
readJson(path.join(dir, "project.json")) as { format?: unknown }
Expand Down
21 changes: 14 additions & 7 deletions canvas/server/sp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ export function canvasFile(canvasesDir: string, examplesDir: string, name: strin
/**
* The project's own settings, beside its canvases: which cover it chose, and the name it is shown
* by when that is not its folder's. A project made without a name is an "Untitled" folder whose
* agent writes `name` here (AppShell.tsx), since renaming the folder would move every address the
* open tab and the chat hold.
* agent writes `name` here (AppShell.tsx). The folder takes that name once the turn that wrote it
* is over (projects.ts, `named`), since during it the agent holds the folder's path.
*/
const PROJECT_JSON = "project.json";

Expand Down Expand Up @@ -277,10 +277,11 @@ export function shoot(board: string, size: number[], res: ServerResponse) {
}

/** A project's project.json, or nothing in it when it has none or it does not parse. */
const readProjectJson = (dir: string) =>
export const readProjectJson = (dir: string) =>
(readJson(path.join(dir, PROJECT_JSON)) ?? {}) as {
cover?: ChosenCover;
name?: string;
unnamed?: boolean;
};

export function createSpServer(options: {
Expand Down Expand Up @@ -344,7 +345,7 @@ export function createSpServer(options: {
// edited. canvasIndex.ts listens.
const pages = new Set<ServerResponse>();
const broadcast = (
event: "reload" | "index" | "layout" | "docs" | "content",
event: "reload" | "index" | "layout" | "docs" | "content" | "moved",
data: unknown,
) => {
const frame = `event: ${event}\ndata: ${JSON.stringify(data)}\n\n`;
Expand Down Expand Up @@ -1312,12 +1313,18 @@ export function createSpServer(options: {
docWatcher.unref();
}

const unwatch = () => {
watcher?.close();
docWatcher?.close();
clearTimeout(docBatch);
};
return {
handle,
unwatch,
/** Tells the pages a project's address changed with its folder's name (projects.ts). */
moved: (moved: { from: string; to: string }) => broadcast("moved", moved),
close() {
watcher?.close();
docWatcher?.close();
clearTimeout(docBatch);
unwatch();
for (const page of pages) page.end();
pages.clear();
},
Expand Down
Loading
Loading