Skills: full detail page with diff-confirmed saves (#175)

* Add tests for the skill detail page and diff flow

Covers the line-diff utility (edits, insertions, removals, line
numbering, empty sides), the skill detail page's diff-confirmed save
(Save… opens the review with the diff and writes nothing, Keep editing
keeps the edit, Confirm & save publishes), the version list with compare
and restore, and the new read of a skill at one commit. The roster and
route suites move to the roster-only contract: /skills lists, and a
single skill lives at its own route.

Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti

* Skills: full detail page with diff-confirmed saves

/skills/<name> is now a real page instead of a placeholder: the skill's
content editor, its version list, and a diff view — no new storage, since
a skill's git history already is its version store.

A save is never silent. "Save…" in the top bar opens a review step
showing the diff between the published version and the editor buffer,
with "Confirm & save" and "Keep editing"; the commit happens only on
confirm. The same renderer draws the comparison between any earlier
version and the current one, read through a new
GET /:name/versions/:commitSha that reads a skill at one commit without
writing anything.

The line diff itself is @corbits/text-diff, a dependency-free
longest-common-subsequence script so an edit in the middle of a document
reads as that one edit.

The roster keeps only what a roster does: the inline skill panel and the
create dialog's edit mode are gone, so there is one skill editor rather
than two.

Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti

* Add tests for bounded diffs and the save head precondition

Pins the diff's refusal to allocate for a wholly rewritten large document
and its linear cost when the identical head and tail are trimmed, the
collapse of long unchanged runs, and newline normalization. On the page:
a save that lost the race is refused, re-diffed against the version that
won, and never buries it; side-action failures stay local and never
discard a dirty draft; a missing skill says so; and CRLF in the buffer
neither reads as a change nor changes the bytes written.

Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti

* Skills: bound the diff and refuse a save that lost the race

Two defects in the review flow, both fixed at the root.

The diff allocated a dense longest-common-subsequence table over the whole
document, so a 12k-line skill cost gigabytes. It now normalizes newlines,
trims the identical head and tail before building anything, refuses the
quadratic walk past a hard cap (1500 changed lines per side, 400k
characters) with an honest "too large to show line by line" summary, and
collapses long unchanged runs to a single row. The table itself is
Int32Array-backed, so the capped worst case is about 9MB. The script is
computed once and both the rows and the change summary are read off that
one result.

The confirmed diff was computed against the version loaded when the page
opened, so a save could silently bury whoever published in between. A save
now carries the version it was reviewed against; PUT /:name refuses a
stale one with a 409, and the page re-reads, keeps the edit, and re-opens
the review against what is actually published, saying why.

Also: the buffer is newline-normalized so the bytes reviewed are the bytes
written, and the description is saved exactly as reviewed rather than
trimmed on the way out; side actions (visibility, restore, compare) report
failures next to themselves instead of replacing the page, and never reset
an unsaved edit; failures read through describeApiError rather than
rendering a server message, with a distinct "no skill named …" state for a
404; the new version read validates its id at the route and 404s a commit
that isn't in the skill's history rather than dating it to 1970; and the
versions list stacks at 1100px, as DESIGN.md specifies.

Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti

Author: TheGreatAxios

apps/web/package.json

@@ -37,6 +37,7 @@
     "@corbits/shell-layout": "workspace:*",
     "@corbits/slug": "workspace:*",
     "@corbits/tasks-ui": "workspace:*",
+    "@corbits/text-diff": "workspace:*",
     "@corbits/url-path": "workspace:*",
     "@corbits/workflow-catalog": "workspace:*",
     "@corbits/icons": "workspace:*",

apps/web/src/pages/create-skill-dialog.tsx

@@ -12,11 +12,10 @@
 // frontmatter must carry. Rejecting it here beats a server error after
 // the person has typed a whole skill body.
 //
-// CL-6355: the same form doubles as the edit surface — `mode="edit"` seeds
-// it from `initialValues` and locks the name field (a skill's name is its
-// identity; renaming means creating a new one). No second editor
-// component: `SkillDetailView`'s "Edit" affordance opens this dialog with
-// `mode="edit"` rather than duplicating the form.
+// Creation only. Editing an existing skill happens on its own page
+// (`skill-detail-page.tsx`, CL-6416), where a save is reviewed as a diff
+// before it publishes a new version — this dialog has no edit mode to
+// duplicate that flow.
 
 import {
   Button,
@@ -63,7 +62,8 @@ const NAME_FIELD: IntakeField = {
   help: "Lowercase letters, digits, and hyphens — this becomes the skill's name in the registry.",
 };
 
-const DESCRIPTION_AND_BODY_FIELDS: readonly IntakeField[] = [
+const FIELDS: readonly IntakeField[] = [
+  NAME_FIELD,
   {
     name: "description",
     label: "Description",
@@ -82,33 +82,12 @@ const DESCRIPTION_AND_BODY_FIELDS: readonly IntakeField[] = [
   },
 ];
 
-const CREATE_FIELDS: readonly IntakeField[] = [
-  NAME_FIELD,
-  ...DESCRIPTION_AND_BODY_FIELDS,
-];
-
-/** Edit mode drops the name field entirely rather than disabling it — a
- * skill's name is its identity, not an editable property; renaming means
- * creating a differently-named skill. The dialog shows it as static text
- * instead (see `DialogDescription` below). */
-const EDIT_FIELDS: readonly IntakeField[] = DESCRIPTION_AND_BODY_FIELDS;
-
 /** Every reason a submission is not yet valid, in plain language — never
  * a generic "invalid form". Exported so the create flow can be proven
  * without SSR-rendering the portal-based dialog (Radix portals yield no
- * static markup). `mode="edit"` skips name validation — the field isn't
- * shown, and the value carried through unchanged is already a valid name. */
-export function validationIssues(
-  values: FormValues,
-  mode: "create" | "edit" = "create",
-): readonly string[] {
+ * static markup). */
+export function validationIssues(values: FormValues): readonly string[] {
   const issues: string[] = [];
-  if (mode === "edit") {
-    if (values.description.trim() === "")
-      issues.push("Description is required.");
-    if (values.body.trim() === "") issues.push("Skill body is required.");
-    return issues;
-  }
   const name = values.name.trim();
   if (name === "") {
     issues.push("Name is required.");
@@ -128,34 +107,26 @@ export function CreateSkillDialog({
   open,
   onOpenChange,
   onSubmit,
-  mode = "create",
-  initialValues,
 }: {
   readonly open: boolean;
   readonly onOpenChange: (open: boolean) => void;
-  /** Writes the skill to the registry — `createSkill` in create mode,
-   * `updateSkill` (a new version) in edit mode. A rejection's message is
-   * shown inline and the form is left as typed. */
+  /** Writes the skill to the registry. A rejection's message is shown
+   * inline and the form is left as typed. */
   readonly onSubmit: (input: SkillCreateInput) => Promise<void>;
-  readonly mode?: "create" | "edit";
-  /** Required in edit mode: seeds the form with the skill being edited. */
-  readonly initialValues?: SkillCreateInput;
 }) {
-  const startingValues = initialValues ?? EMPTY_VALUES;
-  const [values, setValues] = useState<FormValues>(startingValues);
+  const [values, setValues] = useState<FormValues>(EMPTY_VALUES);
   const [showIssues, setShowIssues] = useState(false);
   const [serverError, setServerError] = useState<string | null>(null);
   const [submitting, setSubmitting] = useState(false);
 
   function reset() {
-    setValues(startingValues);
+    setValues(EMPTY_VALUES);
     setShowIssues(false);
     setServerError(null);
   }
 
   function handleOpenChange(next: boolean) {
-    if (next) setValues(startingValues);
-    else reset();
+    reset();
     onOpenChange(next);
   }
 
@@ -167,8 +138,7 @@ export function CreateSkillDialog({
     });
   }
 
-  const fields = mode === "edit" ? EDIT_FIELDS : CREATE_FIELDS;
-  const issues = validationIssues(values, mode);
+  const issues = validationIssues(values);
 
   async function handleSubmit() {
     if (issues.length > 0) {
@@ -195,13 +165,10 @@ export function CreateSkillDialog({
     <Dialog open={open} onOpenChange={handleOpenChange}>
       <DialogContent>
         <DialogHeader>
-          <DialogTitle>
-            {mode === "edit" ? `Edit ${values.name}` : "Create skill"}
-          </DialogTitle>
+          <DialogTitle>Create skill</DialogTitle>
           <DialogDescription>
-            {mode === "edit"
-              ? "Saving publishes a new version — the version it replaces stays in history and can be restored."
-              : "Define a reusable capability an agent can declare and this workbench can share."}
+            Define a reusable capability an agent can declare and this workbench
+            can share.
           </DialogDescription>
         </DialogHeader>
         <DialogBody>
@@ -221,10 +188,10 @@ export function CreateSkillDialog({
             </p>
           )}
           <IntakeForm
-            fields={fields}
+            fields={FIELDS}
             values={values}
             onChange={handleFormChange}
-            idPrefix={mode === "edit" ? "edit-skill" : "create-skill"}
+            idPrefix="create-skill"
           />
         </DialogBody>
         <DialogFooter>
@@ -238,9 +205,9 @@ export function CreateSkillDialog({
           <Button
             type="button"
             onClick={() => void handleSubmit()}
-            disabled={submitting || !intakeFieldsComplete(fields, values)}
+            disabled={submitting || !intakeFieldsComplete(FIELDS, values)}
           >
-            {mode === "edit" ? "Save" : "Create skill"}
+            Create skill
           </Button>
         </DialogFooter>
       </DialogContent>

apps/web/src/pages/detail-placeholders.tsx

@@ -4,15 +4,12 @@
 // testable before the page behind it exists.
 
 import { Button, EmptyState, PageShell } from "@corbits/react-ui";
-import { Lightning, SquaresFour } from "@corbits/icons";
+import { SquaresFour } from "@corbits/icons";
 import type { Slug } from "@corbits/slug";
 import type { ReactNode } from "react";
 
 import { Link } from "../navigation";
-import {
-  PLUGINS_PATH_PREFIX,
-  SKILLS_PATH_PREFIX,
-} from "../path-ids";
+import { PLUGINS_PATH_PREFIX } from "../path-ids";
 import { StageTopBar } from "../shell/stage-top-bar";
 
 function DetailPlaceholder({
@@ -49,18 +46,6 @@ function DetailPlaceholder({
   );
 }
 
-export function SkillDetailPlaceholder({ slug }: { readonly slug: Slug }) {
-  return (
-    <DetailPlaceholder
-      slug={slug}
-      entity="Skill"
-      rosterLabel="Skills"
-      rosterPath={SKILLS_PATH_PREFIX}
-      icon={<Lightning />}
-    />
-  );
-}
-
 export function PluginDetailPlaceholder({ slug }: { readonly slug: Slug }) {
   return (
     <DetailPlaceholder

apps/web/src/pages/diff-view.tsx

@@ -0,0 +1,120 @@
+// One diff renderer for every surface that shows "what changed": the
+// save-confirmation step and the version comparison on a detail page both
+// mount this, so a diff always reads the same way. The line script comes
+// from `@corbits/text-diff`; this file is only its presentation, and it
+// computes the script exactly once per render — the change summary is read
+// off the same result the rows come from.
+
+import { Badge } from "@corbits/react-ui";
+import { diffText } from "@corbits/text-diff";
+import type { DiffLine } from "@corbits/text-diff";
+import { useMemo } from "react";
+
+const MARKER: Record<DiffLine["kind"], string> = {
+  context: " ",
+  added: "+",
+  removed: "-",
+  skipped: "⋯",
+};
+
+const ROW_CLASS: Record<DiffLine["kind"], string> = {
+  context: "text-muted-foreground",
+  added: "bg-success/10 text-foreground",
+  removed: "bg-destructive/10 text-foreground",
+  skipped: "text-muted-foreground italic",
+};
+
+function lineNumber(value: number | null): string {
+  return value === null ? "" : String(value);
+}
+
+export function DiffView({
+  before,
+  after,
+  unchangedNotice = "No changes yet.",
+}: {
+  readonly before: string;
+  readonly after: string;
+  readonly unchangedNotice?: string;
+}) {
+  const diff = useMemo(() => diffText(before, after), [before, after]);
+
+  if (diff.status === "identical") {
+    return (
+      <p className="text-sm text-muted-foreground" data-testid="diff-unchanged">
+        {unchangedNotice}
+      </p>
+    );
+  }
+
+  if (diff.status === "too-large") {
+    return (
+      <div className="flex flex-col gap-1" data-testid="diff-too-large">
+        <p className="text-sm text-foreground">
+          This change is too large to show line by line — showing a summary
+          only.
+        </p>
+        <p className="font-mono text-xs tabular-nums text-muted-foreground">
+          {`${String(diff.beforeLines)} lines before, ${String(
+            diff.afterLines,
+          )} after — ${String(diff.changedBeforeLines)} rewritten to ${String(
+            diff.changedAfterLines,
+          )}`}
+        </p>
+      </div>
+    );
+  }
+
+  return (
+    <div className="flex flex-col gap-2" data-testid="diff-view">
+      <p className="font-mono text-xs tabular-nums text-muted-foreground">
+        {`+${String(diff.totals.added)} added, −${String(
+          diff.totals.removed,
+        )} removed`}
+      </p>
+      <div className="max-h-96 overflow-auto rounded-md border border-border bg-muted/30">
+        <table className="w-full border-collapse font-mono text-xs leading-relaxed">
+          <tbody>
+            {diff.lines.map((line, index) => (
+              <tr
+                key={`${String(index)}:${line.kind}`}
+                className={ROW_CLASS[line.kind]}
+              >
+                <td className="w-10 select-none px-2 text-right tabular-nums text-muted-foreground">
+                  {lineNumber(line.beforeLineNumber)}
+                </td>
+                <td className="w-10 select-none px-2 text-right tabular-nums text-muted-foreground">
+                  {lineNumber(line.afterLineNumber)}
+                </td>
+                <td className="w-6 select-none px-1 text-center">
+                  {MARKER[line.kind]}
+                </td>
+                <td className="whitespace-pre-wrap break-words px-2 py-0.5">
+                  {line.text === "" ? " " : line.text}
+                </td>
+              </tr>
+            ))}
+          </tbody>
+        </table>
+      </div>
+    </div>
+  );
+}
+
+export function DiffHeading({
+  beforeLabel,
+  afterLabel,
+}: {
+  readonly beforeLabel: string;
+  readonly afterLabel: string;
+}) {
+  return (
+    <div className="flex flex-wrap items-center gap-2 text-xs">
+      <Badge tone="neutral">{beforeLabel}</Badge>
+      <span aria-hidden="true" className="text-muted-foreground">
+        →
+      </span>
+      <Badge tone="info">{afterLabel}</Badge>
+    </div>
+  );
+}