feat(lanes): reorder and rename lane cards, and show each file's line delta
The lane strip's tiles are now drag-reorderable (Alt+←/→ is the keyboard
path), and a lane's name can be set in place from the detail header. The
"Lane 23 ·" prefix is gone from both — the bare `#23` stays on the tile,
because that number is how the CLI addresses a lane.
Card order is a per-viewer preference, not a lane fact: it lives in
localStorage (`agenthub.workspace.laneOrder`) beside the split-view layout
and writes nothing server-side, so `GET /api/lanes` and `agenthub lanes`
keep answering in id order. Renaming is the opposite — it is PATCH
/api/lanes/:id with {title}, the one lane column a user now types into
directly.
That made a patch value reachable by hand for the first time, so
`bindable` now rejects any non-scalar for every PATCHABLE column with
EBADFIELD, and the route answers 400 for that and for the
SQLITE_CONSTRAINT family, rather than handing better-sqlite3 a datatype
throw and reporting 500 on plain bad input.
The uncommitted-files tooltip became a real panel. `GET /api/lanes/:id/git`
now returns `files` as {path, status, added, deleted} records — deltas from
one `git diff --numstat HEAD`, null where git has none to report (untracked,
binary, renamed) — and the card renders them portalled to the body, so the
Workspace's own scroll box cannot clip the list.
This commit is contained in:
@@ -143,6 +143,30 @@ describe("/api/lanes", () => {
|
||||
assert.equal((await request("GET", `/api/lanes/${laneId}`)).status, 404);
|
||||
});
|
||||
|
||||
// The Workspace's inline rename is a text field PATCHing this route, so a
|
||||
// title is the one lane column a user types directly. Bad input on any
|
||||
// patchable column is a 400: reaching better-sqlite3 with a non-scalar
|
||||
// raises a datatype error Express can only report as a 500.
|
||||
it("renames a lane, and answers 400 rather than 500 for unbindable input", async () => {
|
||||
const created = await request("POST", "/api/lanes", { cwd: "/tmp/lane-rename-route" });
|
||||
const id = created.body.lane.id;
|
||||
const named = await request("PATCH", `/api/lanes/${id}`, { title: " checkout flow " });
|
||||
assert.equal(named.status, 200);
|
||||
assert.equal(named.body.lane.title, "checkout flow");
|
||||
const cleared = await request("PATCH", `/api/lanes/${id}`, { title: "" });
|
||||
assert.equal(cleared.body.lane.title, "");
|
||||
|
||||
for (const patch of [{ title: { evil: true } }, { notes: { a: 1 } }, { ci_status: true }]) {
|
||||
const bad = await request("PATCH", `/api/lanes/${id}`, patch);
|
||||
assert.equal(bad.status, 400, `expected 400 for ${JSON.stringify(patch)}`);
|
||||
assert.equal(bad.body.error.code, "EBADFIELD");
|
||||
}
|
||||
// A NOT NULL column patched to null is the caller's mistake too.
|
||||
const notNull = await request("PATCH", `/api/lanes/${id}`, { status: null });
|
||||
assert.equal(notNull.status, 400);
|
||||
assert.equal((await request("DELETE", `/api/lanes/${id}`)).status, 200);
|
||||
});
|
||||
|
||||
it("rejects cross-origin lane deletion", async () => {
|
||||
const created = await request("POST", "/api/lanes", { cwd: "/tmp/lane-cross-origin-delete" });
|
||||
const id = created.body.lane.id;
|
||||
|
||||
@@ -358,6 +358,48 @@ describe("lanes lib", () => {
|
||||
assert.equal(after.tool, "codex");
|
||||
assert.equal(after.title, "x");
|
||||
});
|
||||
|
||||
// The Workspace's inline rename PATCHes `title` straight from a text field,
|
||||
// so the column is now reachable by anything same-origin. A non-string would
|
||||
// reach better-sqlite3 as a datatype throw and surface as a 500 on what is
|
||||
// plain bad input.
|
||||
it("normalizes a title on create and on update", () => {
|
||||
const lane = lanes.createLane({ cwd: "/tmp/title-norm", title: " spaced " });
|
||||
assert.equal(lane.title, "spaced");
|
||||
assert.equal(lanes.updateLane(lane.id, { title: 42 }).title, "42");
|
||||
assert.equal(lanes.updateLane(lane.id, { title: "x".repeat(500) }).title.length, 200);
|
||||
// null clears rather than violating the NOT NULL, and the empty string is
|
||||
// a real value: it puts the lane's cwd back in the UI's place for it.
|
||||
assert.equal(lanes.updateLane(lane.id, { title: null }).title, "");
|
||||
assert.equal(lanes.updateLane(lane.id, { title: "" }).title, "");
|
||||
});
|
||||
|
||||
// Every free-text column, not just the one the rename field exposes: a
|
||||
// non-scalar reaches better-sqlite3 as a datatype throw, which Express can
|
||||
// only report as a 500 on what is plainly the caller's mistake.
|
||||
it("rejects a non-scalar patch value on any patchable column", () => {
|
||||
const lane = lanes.createLane({ cwd: "/tmp/patch-scalar" });
|
||||
for (const patch of [
|
||||
{ title: { evil: true } },
|
||||
{ notes: { a: 1 } },
|
||||
{ branch: ["main"] },
|
||||
{ ci_status: true },
|
||||
{ session_id: () => "x" },
|
||||
]) {
|
||||
assert.throws(
|
||||
() => lanes.updateLane(lane.id, patch),
|
||||
(err) => err.code === "EBADFIELD",
|
||||
`expected EBADFIELD for ${JSON.stringify(Object.keys(patch))}`
|
||||
);
|
||||
}
|
||||
assert.throws(
|
||||
() => lanes.createLane({ cwd: "/tmp/patch-scalar-create", title: { evil: true } }),
|
||||
(err) => err.code === "EBADFIELD"
|
||||
);
|
||||
// `links` is the one patchable column that IS an object — it is stored as
|
||||
// JSON, so it must keep working.
|
||||
assert.deepEqual(lanes.updateLane(lane.id, { links: { pr: "u" } }).links, { pr: "u" });
|
||||
});
|
||||
});
|
||||
|
||||
const { withLaneLock } = require("../lib/lane-lock");
|
||||
|
||||
@@ -100,8 +100,16 @@ describe("worktree", () => {
|
||||
assert.equal(s.dirty, 1);
|
||||
assert.equal(s.untracked, 1); // node_modules is ignored, so it does not count
|
||||
assert.match(s.head, /^[0-9a-f]{7,40}$/);
|
||||
// The same lines the counts came from, so a caller can name the files.
|
||||
assert.deepEqual(s.files.sort(), [" M README.md", "?? scratch.txt"]);
|
||||
// The files the counts came from, each with its porcelain status code and
|
||||
// its line delta. An UNTRACKED file has no delta to report — git has never
|
||||
// seen it — so its counts are null rather than invented.
|
||||
assert.deepEqual(
|
||||
s.files.sort((a, b) => a.path.localeCompare(b.path)),
|
||||
[
|
||||
{ path: "README.md", status: " M", added: 1, deleted: 0 },
|
||||
{ path: "scratch.txt", status: "??", added: null, deleted: null },
|
||||
]
|
||||
);
|
||||
// The fixture has no remotes. Given the lane's base branch, unpushedCount
|
||||
// measures base..HEAD — this worktree has committed nothing of its own, so 0.
|
||||
assert.equal(await wt.unpushedCount(dir, "main"), 0);
|
||||
|
||||
+50
-2
@@ -138,6 +138,28 @@ function hydrate(row) {
|
||||
return { ...row, stages, links, ports };
|
||||
}
|
||||
|
||||
/**
|
||||
* A lane's display name, as a column value. The title column is NOT NULL TEXT
|
||||
* and the dashboard's rename field writes it straight through, so it is
|
||||
* trimmed and capped — a pasted file is not a lane name. `null` clears the
|
||||
* title rather than violating the NOT NULL, because clearing is legitimate:
|
||||
* every render is `lane.title || lane.cwd`. A non-scalar is bad input and is
|
||||
* rejected the same way every other patch value is (see `bindable`).
|
||||
*
|
||||
* @param {unknown} value - Whatever the caller offered as a title.
|
||||
* @returns {string} A safe column value.
|
||||
* @throws {Error} code `EBADFIELD` when the value is not a scalar.
|
||||
*/
|
||||
const TITLE_MAX = 200;
|
||||
function normalizeTitle(value) {
|
||||
if (value === null || value === undefined) return "";
|
||||
const t = typeof value;
|
||||
if (t !== "string" && t !== "number" && t !== "bigint") {
|
||||
throw Object.assign(new Error("title must be a string"), { code: "EBADFIELD" });
|
||||
}
|
||||
return String(value).trim().slice(0, TITLE_MAX);
|
||||
}
|
||||
|
||||
function createLane({
|
||||
title = "",
|
||||
cwd,
|
||||
@@ -161,7 +183,7 @@ function createLane({
|
||||
"INSERT INTO lanes (title, cwd, branch, pipeline, kind, source_repo, base_branch, slug, tool, router_profile, stage_since) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
|
||||
)
|
||||
.run(
|
||||
title,
|
||||
normalizeTitle(title),
|
||||
cwd.replace(/\/+$/, ""),
|
||||
branch,
|
||||
pipeline,
|
||||
@@ -184,6 +206,31 @@ function getLane(id) {
|
||||
return hydrate(db.prepare("SELECT * FROM lanes WHERE id = ?").get(id));
|
||||
}
|
||||
|
||||
/**
|
||||
* A patch value, as something better-sqlite3 will actually bind.
|
||||
*
|
||||
* It binds numbers, strings, bigints, buffers and null — and THROWS on
|
||||
* anything else, including the `true` and the `{}` a hand-rolled PATCH can
|
||||
* send for any free-text lane column. That throw is indistinguishable from a
|
||||
* server fault by the time it reaches Express, so the route answered 500 to
|
||||
* what is plainly bad input. Rejecting here gives every PATCHABLE field the
|
||||
* one rule, rather than a special case per field the UI happens to expose.
|
||||
*
|
||||
* @param {string} key - The column being patched, for the error message.
|
||||
* @param {unknown} value - Whatever the caller sent for it.
|
||||
* @returns {string|number|bigint|null} A bindable value.
|
||||
* @throws {Error} code `EBADFIELD` when the value is not a scalar.
|
||||
*/
|
||||
function bindable(key, value) {
|
||||
if (key === "title") return normalizeTitle(value);
|
||||
if (value === null || value === undefined) return null;
|
||||
const t = typeof value;
|
||||
if (t === "string" || t === "number" || t === "bigint") return value;
|
||||
throw Object.assign(new Error(`${key} must be a string, a number or null`), {
|
||||
code: "EBADFIELD",
|
||||
});
|
||||
}
|
||||
|
||||
function updateLane(id, patch = {}) {
|
||||
// Validate kind before building the UPDATE if it's being set
|
||||
if ("kind" in patch && patch.kind !== null && patch.kind !== undefined) {
|
||||
@@ -197,7 +244,8 @@ function updateLane(id, patch = {}) {
|
||||
for (const [k, v] of Object.entries(patch)) {
|
||||
if (!PATCHABLE.has(k)) continue;
|
||||
cols.push(`${k} = ?`);
|
||||
vals.push(k === "links" && typeof v === "object" ? JSON.stringify(v) : v);
|
||||
if (k === "links" && v !== null && typeof v === "object") vals.push(JSON.stringify(v));
|
||||
else vals.push(bindable(k, v));
|
||||
}
|
||||
if (cols.length) {
|
||||
cols.push("updated_at = ?");
|
||||
|
||||
+53
-5
@@ -548,16 +548,51 @@ async function removeWorktree(lane) {
|
||||
await deleteBranchSafely(sourceRepo, branch, baseBranch);
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-file line deltas against HEAD, keyed by path.
|
||||
*
|
||||
* One `git diff --numstat HEAD` covers staged AND unstaged work in a single
|
||||
* subprocess, which is the whole reason it is done this way: the alternative
|
||||
* is one process per file. It says nothing about UNTRACKED files — git has
|
||||
* never seen them, so they have no delta to report and their counts stay null
|
||||
* rather than being invented by reading the file off disk (a lane mid-build
|
||||
* can have an untracked 100 MB artifact).
|
||||
*
|
||||
* A repo with no commits yet has no HEAD to diff against; that throw is not a
|
||||
* fault, it just means every count is unknown.
|
||||
*/
|
||||
async function numstatByPath(dir) {
|
||||
const stats = new Map();
|
||||
try {
|
||||
const result = await git(dir, ["diff", "--numstat", "HEAD"]);
|
||||
for (const line of result.stdout.split("\n")) {
|
||||
const m = line.match(/^(\d+|-)\t(\d+|-)\t(.+)$/);
|
||||
// A binary file reports "-" for both, not a number.
|
||||
if (m) {
|
||||
stats.set(m[3], {
|
||||
added: m[1] === "-" ? null : Number(m[1]),
|
||||
deleted: m[2] === "-" ? null : Number(m[2]),
|
||||
});
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* no HEAD to diff against -> no per-file deltas */
|
||||
}
|
||||
return stats;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse git status --porcelain to count dirty, untracked, and get HEAD commit.
|
||||
* Lines starting with ?? are untracked; others are dirty.
|
||||
*
|
||||
* `files` carries the same lines back verbatim so a caller can show WHICH
|
||||
* files the counts mean, capped because this rides a polled endpoint and a
|
||||
* lane mid-`npm install` can report thousands.
|
||||
* `files` names WHICH files the counts mean, each with its porcelain status
|
||||
* code and its line delta, capped because this rides a polled endpoint and a
|
||||
* lane mid-`npm install` can report thousands. The COUNTS stay exact — only
|
||||
* the listing is capped.
|
||||
*/
|
||||
async function statusCounts(dir) {
|
||||
const result = await git(dir, ["status", "--porcelain=v1", "--untracked-files=normal"]);
|
||||
const stats = await numstatByPath(dir);
|
||||
let dirty = 0;
|
||||
let untracked = 0;
|
||||
const files = [];
|
||||
@@ -569,7 +604,20 @@ async function statusCounts(dir) {
|
||||
} else {
|
||||
dirty++;
|
||||
}
|
||||
if (files.length < STATUS_FILE_CAP) files.push(line.trimEnd());
|
||||
if (files.length < STATUS_FILE_CAP) {
|
||||
// "XY path", and for a rename "XY old -> new" — the new path is the one
|
||||
// that exists now and the one numstat is keyed by. git quotes a path
|
||||
// containing anything unusual; the quotes are presentation, not name.
|
||||
const raw = line.slice(3).trimEnd();
|
||||
const path = raw.split(" -> ").pop().replace(/^"|"$/g, "");
|
||||
files.push({
|
||||
path,
|
||||
status: line.slice(0, 2),
|
||||
added: null,
|
||||
deleted: null,
|
||||
...stats.get(path),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Get short commit hash
|
||||
@@ -583,7 +631,7 @@ async function statusCounts(dir) {
|
||||
* What a lane's working copy looks like right now: which branch it is on, the
|
||||
* short HEAD, that commit's subject, and how much is uncommitted.
|
||||
*
|
||||
* Read-only and cheap, but it is three subprocesses, which is why it lives
|
||||
* Read-only and cheap, but it is five subprocesses, which is why it lives
|
||||
* behind its own endpoint rather than inside the polled `GET /api/lanes`
|
||||
* payload. A detached HEAD reports the literal `HEAD` that git returns — the
|
||||
* caller shows what git says rather than inventing a nicer word for it.
|
||||
|
||||
@@ -268,13 +268,13 @@ const paths = {
|
||||
tags: ["Lanes"],
|
||||
summary: "A lane's working-copy facts",
|
||||
description:
|
||||
"Branch, short HEAD, that commit's subject, and the uncommitted counts for the lane's cwd. Read-only, so no same-origin guard. Kept out of GET /api/lanes because it shells out to git three times and that payload is polled and re-broadcast on every lane_update. A cwd that is missing, is not a git repository, or makes git fail returns available:false with HTTP 200 — a lane pointing at a plain directory is a normal state, not a fault.",
|
||||
"Branch, short HEAD, that commit's subject, and the uncommitted counts for the lane's cwd. Read-only, so no same-origin guard. Kept out of GET /api/lanes because it shells out to git five times and that payload is polled and re-broadcast on every lane_update. A cwd that is missing, is not a git repository, or makes git fail returns available:false with HTTP 200 — a lane pointing at a plain directory is a normal state, not a fault.",
|
||||
operationId: "getLaneGitFacts",
|
||||
parameters: [{ name: "id", in: "path", required: true, schema: { type: "integer" } }],
|
||||
responses: {
|
||||
200: {
|
||||
description:
|
||||
"available:true with {branch, head, subject, dirty, untracked, files}, or available:false alone. files holds the raw `git status --porcelain` lines behind those counts, capped at 20; the counts themselves are never capped.",
|
||||
"available:true with {branch, head, subject, dirty, untracked, files}, or available:false alone. files names the files behind those counts as {path, status, added, deleted} records, capped at 20; the counts themselves are never capped. status is the two-character `git status --porcelain` code and path is the new path for a rename; added/deleted come from one `git diff --numstat HEAD` and are null where git has no delta to report (untracked, binary, renamed, or no commit to diff against).",
|
||||
},
|
||||
404: { description: "Lane not found." },
|
||||
},
|
||||
|
||||
+17
-6
@@ -412,7 +412,12 @@ router.post("/", sameOriginGuard, (req, res) => {
|
||||
broadcastLane(lane.id);
|
||||
res.status(201).json({ lane: payload(lane) });
|
||||
} catch (err) {
|
||||
if (err.code === "EBADCWD" || err.code === "EBADPIPELINE" || err.code === "EBADKIND") {
|
||||
if (
|
||||
err.code === "EBADCWD" ||
|
||||
err.code === "EBADPIPELINE" ||
|
||||
err.code === "EBADKIND" ||
|
||||
err.code === "EBADFIELD"
|
||||
) {
|
||||
return res.status(400).json({ error: { code: err.code, message: err.message } });
|
||||
}
|
||||
if (err.code === "SQLITE_CONSTRAINT_UNIQUE" || String(err.message).includes("UNIQUE")) {
|
||||
@@ -432,12 +437,18 @@ router.patch("/:id", sameOriginGuard, (req, res) => {
|
||||
try {
|
||||
lane = lanesLib.updateLane(req.params.id, req.body || {});
|
||||
} catch (err) {
|
||||
// A bad `kind` or `pipeline` is invalid input, not a server fault — every
|
||||
// sibling route answers 400 here, so this one must too instead of throwing
|
||||
// into Express.
|
||||
if (err.code === "EBADKIND" || err.code === "EBADPIPELINE") {
|
||||
// A bad `kind`, `pipeline` or field value is invalid input, not a server
|
||||
// fault — every sibling route answers 400 here, so this one must too
|
||||
// instead of throwing into Express. The SQLITE_CONSTRAINT family lands
|
||||
// here for the same reason: a NOT NULL column patched to null is the
|
||||
// caller's mistake, and enumerating which columns those are in JS would
|
||||
// just be the schema written twice.
|
||||
if (err.code === "EBADKIND" || err.code === "EBADPIPELINE" || err.code === "EBADFIELD") {
|
||||
return res.status(400).json({ error: { code: err.code, message: err.message } });
|
||||
}
|
||||
if (String(err.code || "").startsWith("SQLITE_CONSTRAINT")) {
|
||||
return res.status(400).json({ error: { code: "EBADFIELD", message: err.message } });
|
||||
}
|
||||
return res.status(500).json({ error: { code: err.code, message: err.message } });
|
||||
}
|
||||
broadcastLane(lane.id);
|
||||
@@ -690,7 +701,7 @@ router.get("/:id/integrations/:name", (req, res) => {
|
||||
* the uncommitted counts. Read-only, so no same-origin guard — that guard
|
||||
* exists for the destructive actions.
|
||||
*
|
||||
* Deliberately NOT part of `GET /api/lanes`: this shells out to git three
|
||||
* Deliberately NOT part of `GET /api/lanes`: this shells out to git five
|
||||
* times, and that payload is polled and re-broadcast on every hook-driven
|
||||
* lane_update. Any failure — no such directory, not a repo, git itself
|
||||
* erroring — is reported as `available: false` rather than a 500, because a
|
||||
|
||||
Reference in New Issue
Block a user