epic 2.1: a type change stops leaving the old body on disk (T2.1.9)

.github/workflows/dashboard-check.mjs

@@ -21,7 +21,6 @@
 // Usage: node dashboard-check.mjs <base-url>
 
 import vm from 'node:vm';
-import { readFile } from 'node:fs/promises';
 
 const base = process.argv[2];
 if (!base) {
@@ -47,15 +46,15 @@ for (const marker of ['<div id="lock">', '<div id="app">', '<title>artifacts</ti
 }
 // Every inline type the server publishes needs an option inside the compose type select, or
 // that type can only be published through the API, the CLI or MCP. redirect shipped without
-// one (T2.1.7). The list comes from server.js rather than a copy here, so a sixth type fails
-// this check on the day it is added instead of shipping with no way to compose it.
+// one (T2.1.7). The list is imported rather than copied here, so a sixth type fails this check
+// on the day it is added instead of shipping with no way to compose it. It reads SOURCE_EXT,
+// which is what server.js builds TYPES from and what the type-change cleanup keys off, so all
+// three cannot drift apart (T2.1.9).
 const typeSelect = html.match(/<select id="type">([\s\S]*?)<\/select>/);
 if (!typeSelect) fail('the compose form has no <select id="type">');
-const serverSrc = await readFile(new URL('../../server.js', import.meta.url), 'utf8');
-const typesLine = serverSrc.match(/^const TYPES = \[([^\]]*)\]/m);
-if (!typesLine) fail('could not read the TYPES list out of server.js');
-const types = [...typesLine[1].matchAll(/'([^']+)'/g)].map((m) => m[1]);
-if (types.length < 2) fail(`parsed only ${types.length} type(s) from server.js TYPES`);
+const { SOURCE_EXT } = await import(new URL('../../lib/artifact-files.js', import.meta.url));
+const types = Object.keys(SOURCE_EXT);
+if (types.length < 2) fail(`parsed only ${types.length} type(s) from SOURCE_EXT`);
 for (const type of types) {
   if (!typeSelect[1].includes(`value="${type}"`)) {
     fail(`the compose type select has no option for ${type}`);

.github/workflows/smoke.sh

@@ -478,6 +478,42 @@ curl -sf -X PUT "$BASE/api/artifacts/ci-redir-conv" -H "$AUTH" -H "$JSON" \
 curl -sf -X DELETE "$BASE/api/artifacts/ci-redir-conv" -H "$AUTH" > /dev/null
 echo "ok: html converted to a redirect"
 
+# T2.1.9: a type change drops the objects the old type owned. The bytes themselves are not
+# reachable over HTTP (every serve path keys off meta.type, which is why they sat there
+# unnoticed since md shipped), so what this proves is the other half: the cleanup never takes a
+# file the new type still needs. Walk one artifact through every type and read it back after
+# each hop. test/artifact-files.test.js covers which keys each direction drops, and
+# test/storage-local.test.js plus test/storage-sql.test.js cover the delete itself.
+convert_to() { # convert_to <type> <content>
+  curl -sf -X PUT "$BASE/api/artifacts/ci-conv" -H "$AUTH" -H "$JSON" \
+    -d "$(node -e 'process.stdout.write(JSON.stringify({content: process.argv[1], type: process.argv[2]}))' "$2" "$1")" > /dev/null \
+    || fail "converting to $1 was refused"
+  [ "$(list_field ci-conv type)" = "$1" ] || fail "converted artifact is not listed as $1"
+}
+curl -sf -X POST "$BASE/api/artifacts" -H "$AUTH" -H "$JSON" \
+  -d '{"content":"<h1>step html</h1>","type":"html","slug":"ci-conv","visibility":"public"}' > /dev/null
+convert_to md '# step md'
+# md renders inside the frame, so the rendered body is behind ?raw=1 like every other type
+curl -s "$BASE/a/ci-conv?raw=1" | grep -q '<h1>step md</h1>' || fail "md conversion lost the rendered body"
+[ "$(curl -s "$BASE/a/ci-conv/source")" = '# step md' ] || fail "md conversion lost source.md"
+convert_to jsx 'export default () => <p>step jsx</p>'
+curl -s "$BASE/a/ci-conv?raw=1" | grep -q 'step jsx' || fail "jsx conversion lost index.html"
+curl -s "$BASE/a/ci-conv/source" | grep -q 'step jsx' || fail "jsx conversion lost source.jsx"
+# jsx to tsx is the direction where both types bake an index.html and only the extension moves
+convert_to tsx 'export default () => <p>step tsx</p>'
+curl -s "$BASE/a/ci-conv?raw=1" | grep -q 'step tsx' || fail "tsx conversion lost index.html"
+curl -s "$BASE/a/ci-conv/source" | grep -q 'step tsx' || fail "tsx conversion lost source.tsx"
+convert_to redirect 'https://example.com/step-redirect'
+code=$(curl -s -o /dev/null -w '%{http_code}' "$BASE/a/ci-conv")
+expect_code 301 "$code" "the conversion walk's redirect hop"
+[ "$(curl -s "$BASE/a/ci-conv/source")" = 'https://example.com/step-redirect' ] \
+  || fail "redirect conversion lost source.url"
+convert_to html '<h1>step html again</h1>'
+curl -s "$BASE/a/ci-conv?raw=1" | grep -q 'step html again' || fail "html conversion lost index.html"
+curl -s "$BASE/a/ci-conv/source" | grep -q 'step html again' || fail "html conversion lost source.html"
+curl -sf -X DELETE "$BASE/api/artifacts/ci-conv" -H "$AUTH" > /dev/null
+echo "ok: every type conversion serves the type it was given"
+
 # a copy keeps the target
 dupslug=$(curl -s -X POST "$BASE/api/artifacts/ci-redir/duplicate" -H "$AUTH" -H "$JSON" \
   -d '{"slug":"ci-redir-copy","visibility":"public"}' | sed -n 's/.*"slug":"\([^"]*\)".*/\1/p')

docs/mcp.md

@@ -42,12 +42,12 @@ author's own HTML. Details in [Link previews](formats.md#link-previews).
 `FRAME_ENABLED=false` nothing is framed and `frame: true` changes nothing, so a client that gets no
 frame should check the server config before the artifact.
 
-`update_artifact` rewrites the artifact rather than patching it, and two of its optional arguments
-fall back to a default instead of keeping what is there. Pass `type` on every update of a `jsx`,
-`tsx`, `md` or `redirect` artifact, or the artifact comes back as HTML. Pass `title` to keep the
-title, or it resets to the slug. `frame`, `tags`, `project` and `visibility` do keep their current
-value when omitted, and take the values documented on the matching `set_artifact_*` row. To change
-one field and touch nothing else, use that row's tool instead of `update_artifact`.
+`update_artifact` rewrites the artifact rather than patching it, and `type` falls back to `html`
+instead of keeping what is there. Pass it on every update of a `jsx`, `tsx`, `md` or `redirect`
+artifact, or the artifact comes back as HTML and the files the old type owned are deleted. `title`,
+`frame`, `tags`, `project`, `visibility`, `description` and `ogImage` all keep their current value
+when omitted, and take the values documented on the matching `set_artifact_*` row. To change one
+field and touch nothing else, use that row's tool instead of `update_artifact`.
 
 No MCP tool for zip sites, because the payload is binary. Use the [CLI](cli.md) or the [zip endpoint](api.md#zip-sites-multi-file-static-projects).
 

lib/artifact-files.js

@@ -0,0 +1,69 @@
+// Which storage objects an artifact of each type owns, and what a type change leaves behind.
+//
+// This lives outside server.js so a unit test can walk every ordered pair of types without a
+// server or a storage backend. saveArtifact only ever wrote: a PUT converting html to a redirect
+// left index.html and source.html on disk, and the reverse left source.url. Nothing served them,
+// because /a/:slug and /a/:slug/source both key off meta.type, so it was at-rest bloat that grew
+// with every conversion.
+//
+// What this does NOT do: on the git backend the deletion is a commit like any other, so the old
+// bytes stay in history and `git log -p` on the remote still hands them back. A private or
+// password artifact's old body is readable to anyone with read access to GIT_REMOTE_URL, exactly
+// as it was before. Reclaiming that needs a history rewrite, which is not something a publish
+// request gets to do.
+
+// The `source.<ext>` extension each type uses. /a/:slug/source reads it back.
+export const SOURCE_EXT = { html: 'html', jsx: 'jsx', tsx: 'tsx', md: 'md', redirect: 'url' };
+
+// html, jsx and tsx bake an index.html at publish time. md renders per request from source.md
+// and a redirect answers with a header, so neither owns one. A zip site is in neither list: its
+// files live under site/ and no API path converts a zip to anything (storeArtifact refuses to
+// replace one with inline content, storeZipArtifact 409s on a slug that exists), so a zip
+// namespace only ever goes away whole, through deleteSlug.
+const BAKES_INDEX = new Set(['html', 'jsx', 'tsx']);
+
+// The content objects a stored artifact of this type owns. Never meta.json: the record belongs
+// to the artifact, not to the type.
+export function ownedKeys(slug, type) {
+  const keys = [];
+  if (BAKES_INDEX.has(type)) keys.push(`${slug}/index.html`);
+  // hasOwn, not a plain lookup: a hand-edited meta.type of "constructor" or "toString" would
+  // otherwise find a function on the prototype and build `source.function Object() {...}` as a
+  // key. Nothing would match it, but a delete should not be built from a value off the chain.
+  if (Object.hasOwn(SOURCE_EXT, type)) keys.push(`${slug}/source.${SOURCE_EXT[type]}`);
+  return keys;
+}
+
+// The keys the old type owned that the new type does not. Empty when the type did not change
+// and empty on a first publish, where there is no old type.
+export function staleKeys(slug, oldType, newType) {
+  if (!oldType || oldType === newType) return [];
+  const kept = new Set(ownedKeys(slug, newType));
+  return ownedKeys(slug, oldType).filter((key) => !kept.has(key));
+}
+
+// Drop what the conversion left behind. The caller runs this AFTER meta.json names the new type
+// and BEFORE flush, so a crash in between leaves the old record whole rather than a listed
+// artifact with no body, and git carries the deletions in the same commit as the write.
+//
+// It lives here rather than inline in server.js so a test can hand it a fake storage and prove
+// the deletes are actually issued. No test boots server.js, so an inline loop was provably dead
+// weight: neutering it to `for (const key of [])` left both suites green on all five backends.
+//
+// A delete that fails is logged, not thrown. The write has already landed and meta already names
+// the new type, so throwing would turn a successful replace into a 500 the caller would retry.
+export async function dropStaleObjects(storage, slug, oldType, newType) {
+  const dropped = [];
+  for (const key of staleKeys(slug, oldType, newType)) {
+    try {
+      await storage.delete(key);
+      dropped.push(key);
+    } catch (err) {
+      console.warn(
+        `storage: could not drop ${key} after a type change: ${err.message}. ` +
+          'The artifact is fine and nothing serves that file. Nothing retries, so remove it by hand.',
+      );
+    }
+  }
+  return dropped;
+}

server.js

@@ -31,6 +31,7 @@ import {
   validateCredentials,
   parseKeyInput,
 } from './lib/auth.js';
+import { SOURCE_EXT, dropStaleObjects } from './lib/artifact-files.js';
 import { createConfigStore } from './lib/config.js';
 import { ApiError } from './lib/errors.js';
 import { artifactExpired } from './lib/expiry.js';
@@ -210,15 +211,16 @@ function artifactUnlocked(req, meta) {
   return unlockValid(req, meta);
 }
 
-const TYPES = ['html', 'jsx', 'tsx', 'md', 'redirect'];
+// Read off the same table lib/artifact-files.js cleans up from, so a sixth type cannot become
+// publishable here and stay invisible to the type-change cleanup there.
+const TYPES = Object.keys(SOURCE_EXT);
 const SLUG_RE = /^[a-z0-9][a-z0-9-]{2,63}$/;
 const TAG_RE = /^[a-z0-9][a-z0-9-]{0,31}$/;
 const MAX_TAGS = 10;
 // A project is a single grouping label (one per artifact). Friendlier than a
 // slug — Unicode letters/digits, spaces, and - _ . — but bounded, and must
 // start with a letter or digit. Internal whitespace is collapsed on input.
 const PROJECT_RE = /^[\p{L}\p{N}][\p{L}\p{N}\p{M} ._-]{0,63}$/u;
-const SOURCE_EXT = { html: 'html', jsx: 'jsx', tsx: 'tsx', md: 'md', redirect: 'url' };
 
 // Pinned versions shared with the jsx shell. `external=react` keeps packages on
 // the shell's React instance — separate copies cause "Invalid hook call".
@@ -736,6 +738,10 @@ async function storeArtifact(finalSlug, { content, type = 'html', title, descrip
   await storage.put(`${finalSlug}/meta.json`, JSON.stringify(meta, null, 2), {
     contentType: 'application/json',
   });
+  // The old type's objects are unreachable now that meta names the new one, so drop them. After
+  // the meta write and before flush; the ordering and the swallowed failure are explained in
+  // lib/artifact-files.js.
+  await dropStaleObjects(storage, finalSlug, existing?.type, type);
   await storage.flush?.(); // durably commit the completed write (git); no-op elsewhere
   dropMdRender(finalSlug);
   // A non-public artifact needs the session secret resident to mint its capability token;
@@ -1178,9 +1184,15 @@ function parseRange(header, size) {
 // status/headers are flushed and immutable, so an upstream error must ABORT the socket
 // (res.destroy) — never res.end(), which would pass a truncated artifact off as complete.
 function pipeStream(res, stream) {
-  stream.on('error', () => {
-    if (!res.headersSent) res.status(500).type('text/plain').send('internal error');
-    else res.destroy();
+  stream.on('error', (err) => {
+    if (res.headersSent) return res.destroy();
+    // A read that passed the stat can still miss when the open happens: local checks the file
+    // and then opens it, so a write that landed in between took the object away. That is the
+    // same "not there" the stat catches, and the rename path has always answered it as a 404,
+    // so it answers 404 here too rather than the 500 a real read error gets.
+    const gone = err?.code === 'ENOENT' || err?.code === 'ENOTDIR';
+    if (gone) return res.status(404).type('text/plain').send('not found');
+    res.status(500).type('text/plain').send('internal error');
   });
   stream.pipe(res);
 }
@@ -1387,6 +1399,9 @@ app.get('/a/:slug/source', async (req, res, next) => {
     if (target === null) return notFound(res);
     return res.type('text/plain; charset=utf-8').send(target);
   }
+  // meta.type comes off disk unvalidated, and a bare SOURCE_EXT lookup walks the prototype, so a
+  // hand-edited "constructor" built a key out of a function body. Same guard ownedKeys uses.
+  if (!Object.hasOwn(SOURCE_EXT, meta.type)) return notFound(res);
   // forceType keeps source inert: an HTML/JSX source is served as text/plain, never executed.
   serveObject(req, res, `${slug}/source.${SOURCE_EXT[meta.type]}`, {
     forceType: 'text/plain; charset=utf-8',
@@ -1844,7 +1859,7 @@ function createMcpServer(scopes = SCOPES) {
     {
       title: 'Update artifact',
       description:
-        'Rewrite an existing artifact by slug. Only type and title reset when omitted: type becomes html and title becomes the slug, so pass both on every update. Every other field the artifact has keeps its current value. Returns the share URL, tokened for private and password artifacts.',
+        'Rewrite an existing artifact by slug. Only type resets when omitted: it becomes html, and changing the type deletes the files the old type owned, so pass it on every update of a jsx, tsx, md or redirect artifact. Every other field, title included, keeps its current value. Returns the share URL, tokened for private and password artifacts.',
       inputSchema: {
         slug: z.string(),
         content: z.string(),

storage/git.js

@@ -213,6 +213,7 @@ export async function create() {
     put: (key, data, opts) => files.put(key, data, opts),
     move: (oldSlug, newSlug) => files.move(oldSlug, newSlug),
     copySlug: (src, dst) => files.copySlug(src, dst),
+    delete: (key) => files.delete(key),
     deleteSlug: (slug) => files.deleteSlug(slug),
 
     // Commit the completed write and push it, serialized so two operations never race the

storage/index.js

@@ -16,6 +16,7 @@
 //     listMetas()               -> [{ slug, buffer }]            // every artifact's meta.json
 //     move(oldSlug, newSlug)                                     // rename a whole namespace
 //     copySlug(srcSlug, dstSlug)                                 // copy a namespace's content objects (NOT meta.json)
+//     delete(key)                                                // remove ONE object (never a prefix); a key that is gone is not an error
 //     deleteSlug(slug)                                           // remove a whole namespace
 //     flush?()                  // optional: durably commit a completed write (git)
 //   }
@@ -39,6 +40,10 @@
 // content objects first and `<slug>/meta.json` LAST as a commit marker, because readMeta
 // and listMetas key off meta.json — a namespace with no meta is invisible (404), never
 // half-served. deleteSlug removes meta first. See server.js for where this is applied.
+//
+// The same ordering decides where delete(key) goes: a replace that changes the type drops the
+// old type's objects AFTER meta.json names the new one, so a crash mid-conversion leaves the
+// old record whole rather than a listed artifact whose body is gone.
 
 // A key/segment that fails validation. Callers map this to 404 (it only reaches a backend
 // via user-controlled zip sub-paths); it must never surface as a 500.
@@ -73,6 +78,30 @@ const BACKENDS = {
   sqlite: () => import('./sqlite.js'),
 };
 
+// Every method the app calls on a store. A backend that is missing one fails the boot rather
+// than the request that first needs it: the type-change cleanup swallows a failed delete on
+// purpose (a write that already landed must not 500), so a backend with no delete would warn
+// once per conversion forever and never fail a test. flush is left out because it is optional.
+const REQUIRED = [
+  'getBuffer',
+  'get',
+  'head',
+  'put',
+  'listMetas',
+  'move',
+  'copySlug',
+  'delete',
+  'deleteSlug',
+];
+
+export function assertComplete(name, storage) {
+  const missing = REQUIRED.filter((method) => typeof storage?.[method] !== 'function');
+  if (missing.length) {
+    throw new Error(`storage backend "${name}" is missing: ${missing.join(', ')}`);
+  }
+  return storage;
+}
+
 // Instantiate the configured backend and run its boot check (fail-fast, like the
 // ARTIFACTS_API_KEY check) so a misconfigured store crashes at startup, not first request.
 export async function createStorage() {
@@ -90,7 +119,7 @@ export async function createStorage() {
       `storage backend "${name}" could not be loaded — is its dependency installed? (${err.message})`,
     );
   }
-  const storage = await mod.create();
+  const storage = assertComplete(name, await mod.create());
   await storage.init?.();
   return storage;
 }

storage/local.js

@@ -222,6 +222,13 @@ export async function createAt(root) {
       });
     },
 
+    // Remove one object. `force` so a key that is already gone is not an error: a conversion
+    // asks for the old type's files without checking, and an artifact published before that
+    // type owned one of them has nothing there to drop.
+    async delete(key) {
+      await fs.rm(resolveKey(key), { force: true });
+    },
+
     async deleteSlug(slug) {
       // meta.json first so a crash mid-delete leaves an invisible (404) namespace, never a
       // live artifact with missing files.

storage/postgres.js

@@ -78,6 +78,12 @@ export async function create() {
       );
     },
 
+    async delete(key) {
+      // Equality, not a range, so no collation question: this drops one object, and a row that
+      // is not there is a no-op.
+      await q('DELETE FROM artifacts WHERE key = $1', [key]);
+    },
+
     async deleteSlug(slug) {
       await q('DELETE FROM artifacts WHERE key COLLATE "C" >= $1 AND key COLLATE "C" < $2', [`${slug}/`, `${slug}0`]);
     },

storage/s3.js

@@ -258,6 +258,12 @@ export async function create() {
       }
     },
 
+    // Remove one object. deleteKeys already treats a 404 as done, which is what a conversion
+    // needs: it asks for the old type's files without checking whether each one is there.
+    async delete(key) {
+      await deleteKeys([key]);
+    },
+
     async deleteSlug(slug) {
       const keys = await listKeys(slug);
       const meta = `${slug}/meta.json`;