Recommend a no-cache rule for GitHub embeds on BYO domains

apps/api/src/storage-verify.test.ts

@@ -385,6 +385,60 @@ describe("verifyStorageConfig — recommended public-URL probe", () => {
     expect(fetchImpl).toHaveBeenCalledOnce();
   });
 
+  it("warns (embed-cache) when the domain serves cacheable headers, without gating ok", async () => {
+    const client = new FakeStorageClient();
+    const fetchImpl = vi.fn(async () => {
+      const [key] = [...client.store.keys()];
+      return new Response(client.store.get(key)!, {
+        status: 200,
+        headers: { "cache-control": "public, max-age=14400" },
+      });
+    });
+    const result = await run(
+      { ...VALID, publicBaseUrl: "https://media.example.com" },
+      client,
+      fetchImpl as unknown as typeof fetch,
+    );
+    expect(result.ok).toBe(true);
+    const embedCache = result.checks.find((c) => c.id === "embed-cache")!;
+    expect(embedCache.ok).toBe(false);
+    expect(embedCache.required).toBe(false);
+    expect(embedCache.hint).toMatch(/optional but recommended/);
+    expect(embedCache.hint).toMatch(/Transform Rule/);
+  });
+
+  it("passes embed-cache when Cache-Control carries no-store/no-cache", async () => {
+    const client = new FakeStorageClient();
+    const fetchImpl = vi.fn(async () => {
+      const [key] = [...client.store.keys()];
+      return new Response(client.store.get(key)!, {
+        status: 200,
+        headers: { "cache-control": "max-age=0, no-cache, no-store, must-revalidate" },
+      });
+    });
+    const result = await run(
+      { ...VALID, publicBaseUrl: "https://media.example.com" },
+      client,
+      fetchImpl as unknown as typeof fetch,
+    );
+    const embedCache = result.checks.find((c) => c.id === "embed-cache")!;
+    expect(embedCache.ok).toBe(true);
+    expect(embedCache.hint).toBeUndefined();
+  });
+
+  it("omits embed-cache entirely when the domain couldn't be reached", async () => {
+    const client = new FakeStorageClient();
+    const fetchImpl = vi.fn(async () => {
+      throw new Error("network error: ENOTFOUND");
+    });
+    const result = await run(
+      { ...VALID, publicBaseUrl: "https://media.example.com" },
+      client,
+      fetchImpl as unknown as typeof fetch,
+    );
+    expect(result.checks.find((c) => c.id === "embed-cache")).toBeUndefined();
+  });
+
   it("reports a thrown fetch (DNS/timeout/subrequest failure) as 'couldn't verify from here', not 'domain is broken'", async () => {
     const client = new FakeStorageClient();
     const fetchImpl = vi.fn(async () => {

apps/api/src/storage-verify.ts

@@ -43,7 +43,7 @@ export interface StorageVerifyCandidate {
 }
 
 export interface StorageVerifyCheck {
-  /** Stable identifier — `"shape" | "auth" | "round-trip" | "not-empty" | "public-url"`. */
+  /** Stable identifier — `"shape" | "auth" | "round-trip" | "not-empty" | "public-url" | "embed-cache"`. */
   id: string;
   ok: boolean;
   /** Required checks gate `StorageVerifyResult.ok`; recommended ones only ever warn. */
@@ -228,31 +228,41 @@ async function checkPublicUrl(
   probeKey: string,
   probeBytes: Uint8Array,
   fetchImpl: typeof fetch,
-): Promise<StorageVerifyCheck> {
+): Promise<{ check: StorageVerifyCheck; cacheControl: string | null | undefined }> {
   const url = `${publicBaseUrl.replace(/\/$/, "")}/${probeKey}`;
   try {
     const res = await fetchImpl(url, {
       redirect: "error",
       signal: AbortSignal.timeout(PUBLIC_URL_PROBE_TIMEOUT_MS),
     });
+    // Read the header before consuming the body: the embed-cache check
+    // (below) only makes sense when the domain actually answered, so the
+    // reading rides along with whichever public-url outcome this is.
+    const cacheControl = res.headers.get("cache-control");
     if (!res.ok) {
       return {
-        id: "public-url",
-        ok: false,
-        required: false,
-        hint: `fetching the probe object via publicBaseUrl returned HTTP ${res.status} — the domain may be connected to a different bucket, or public access isn't enabled yet`,
+        check: {
+          id: "public-url",
+          ok: false,
+          required: false,
+          hint: `fetching the probe object via publicBaseUrl returned HTTP ${res.status} — the domain may be connected to a different bucket, or public access isn't enabled yet`,
+        },
+        cacheControl: undefined,
       };
     }
     const body = new Uint8Array(await res.arrayBuffer());
     if (!bytesEqual(body, probeBytes)) {
       return {
-        id: "public-url",
-        ok: false,
-        required: false,
-        hint: "the bytes served from publicBaseUrl didn't match what was just written — this can mean a cached/stale response from an edge in front of the domain rather than a wiring problem; try again in a minute",
+        check: {
+          id: "public-url",
+          ok: false,
+          required: false,
+          hint: "the bytes served from publicBaseUrl didn't match what was just written — this can mean a cached/stale response from an edge in front of the domain rather than a wiring problem; try again in a minute",
+        },
+        cacheControl,
       };
     }
-    return { id: "public-url", ok: true, required: false };
+    return { check: { id: "public-url", ok: true, required: false }, cacheControl };
   } catch {
     // A thrown fetch means this probe — run from inside the API worker —
     // couldn't reach the domain; it does NOT mean the domain is broken. A
@@ -261,14 +271,39 @@ async function checkPublicUrl(
     // #783). Say "we couldn't verify it from here", not "your domain is
     // broken", and point at the one check that actually settles it.
     return {
-      id: "public-url",
-      ok: false,
-      required: false,
-      hint: "we couldn't verify publicBaseUrl from here — this can happen even when the domain is working fine (a same-account custom domain isn't always reachable as a server-side request). Open a known object's URL in a browser to check for yourself; if that loads, the domain is fine.",
+      check: {
+        id: "public-url",
+        ok: false,
+        required: false,
+        hint: "we couldn't verify publicBaseUrl from here — this can happen even when the domain is working fine (a same-account custom domain isn't always reachable as a server-side request). Open a known object's URL in a browser to check for yourself; if that loads, the domain is fine.",
+      },
+      cacheControl: undefined,
     };
   }
 }
 
+/**
+ * Recommended, non-blocking check (issue #592): whether the custom domain
+ * serves the badge-style no-cache headers GitHub's Camo proxy needs to
+ * revalidate an image after an in-place overwrite — the same Transform Rule
+ * `embed.uploads.sh` carries on hosted storage. Only emitted when the
+ * public-URL probe actually got a response; `null` (header absent) counts as
+ * missing. `no-store` or `no-cache` in Cache-Control is what makes Camo
+ * refetch reliably; a plain short max-age is not enough (issue #152).
+ */
+function checkEmbedCache(cacheControl: string | null): StorageVerifyCheck {
+  const value = (cacheControl ?? "").toLowerCase();
+  const ok = value.includes("no-store") || value.includes("no-cache");
+  return {
+    id: "embed-cache",
+    ok,
+    required: false,
+    hint: ok
+      ? undefined
+      : 'optional but recommended: add a Cloudflare Transform Rule on this domain setting Cache-Control to "max-age=0, no-cache, no-store, must-revalidate" so GitHub embeds refresh when a file is overwritten in place — see the setup guide (/docs/byo-bucket)',
+  };
+}
+
 /** Warning shown when no `publicBaseUrl` is configured — signed-only mode is allowed but degraded (issue #783 follow-up comment). */
 const NO_PUBLIC_URL_HINT =
   "no public base URL set — files will only be reachable through signed links that expire after an hour, and embeds/galleries won't work. Add one any time; it applies retroactively to files already uploaded (URLs are derived on request, nothing is stored per file).";
@@ -339,6 +374,7 @@ export async function verifyStorageConfig(
   let roundTripOk = false;
   let roundTripHint: string | undefined;
   let publicUrlCheck: StorageVerifyCheck | undefined;
+  let publicUrlCacheControl: string | null | undefined;
   try {
     await client.upload(probeKey, probeBytes, { contentType: "application/octet-stream" });
     const downloaded = await client.download(probeKey);
@@ -348,12 +384,9 @@ export async function verifyStorageConfig(
       roundTripHint =
         "wrote and read back a probe object but the bytes didn't match — check for another writer racing this bucket";
     } else if (candidate.publicBaseUrl) {
-      publicUrlCheck = await checkPublicUrl(
-        candidate.publicBaseUrl,
-        probeKey,
-        probeBytes,
-        fetchImpl,
-      );
+      const probed = await checkPublicUrl(candidate.publicBaseUrl, probeKey, probeBytes, fetchImpl);
+      publicUrlCheck = probed.check;
+      publicUrlCacheControl = probed.cacheControl;
     }
   } catch (err) {
     roundTripHint =
@@ -396,6 +429,12 @@ export async function verifyStorageConfig(
         hint: "skipped — the write/read round-trip failed before the public URL could be verified",
       },
     );
+    // Cache-rule reading for GitHub embeds (issue #592) — only when the
+    // domain answered, so the check never piles "couldn't check" noise on
+    // top of an unreachable-domain public-url failure.
+    if (publicUrlCacheControl !== undefined) {
+      checks.push(checkEmbedCache(publicUrlCacheControl));
+    }
   } else {
     // Signed-only mode is allowed, but it's a degraded state, not a neutral
     // default — flag it the same warning-level way a failed recommended

apps/web/src/pages/account/workspaces/[name]/settings/storage.astro

@@ -282,6 +282,12 @@ if (!workspace) return Astro.redirect("/account/workspaces");
                 class="text-foreground">Custom Domains</strong
               >. That domain is the public base URL below.
             </li>
+            <li>
+              Optional but recommended: a Transform Rule on that domain setting <strong
+                class="text-foreground">Cache-Control</strong
+              > to <code>max-age=0, no-cache, no-store, must-revalidate</code> keeps GitHub embeds fresh
+              after overwrites.
+            </li>
           </ol>
           <p class="mt-1 mb-1 text-muted-foreground">
             Full walkthrough in the <a href="/docs/byo-bucket">setup guide</a>.
@@ -540,6 +546,7 @@ if (!workspace) return Astro.redirect("/account/workspaces");
         "round-trip": "We couldn't write a test file to this bucket",
         "not-empty": "This bucket already has files in it",
         "public-url": "The public base URL didn't serve our test file",
+        "embed-cache": "GitHub embeds: add a cache rule to this domain",
       };
 
       let formMode: "connect" | "rotate" = "connect";
@@ -875,7 +882,11 @@ if (!workspace) return Astro.redirect("/account/workspaces");
             const hint = check.hint
               ? `<span class="storage-check-hint text-xs text-muted-foreground">${escapeHtml(check.hint)}</span>`
               : "";
-            return `<li class="flex flex-col gap-0.5 rounded-md border border-border px-3 py-2"><strong class="text-destructive text-[13px]">${escapeHtml(label)}</strong>${hint}</li>`;
+            // `embed-cache` is an optional-but-recommended tip (#592), not a
+            // blocker — render it in body color so it doesn't read as an
+            // error standing between the user and saving.
+            const tone = check.id === "embed-cache" ? "text-foreground" : "text-destructive";
+            return `<li class="flex flex-col gap-0.5 rounded-md border border-border px-3 py-2"><strong class="${tone} text-[13px]">${escapeHtml(label)}</strong>${hint}</li>`;
           })
           .join("");
       }
@@ -921,8 +932,13 @@ if (!workspace) return Astro.redirect("/account/workspaces");
           }
           // Any failing check keeps you on the form with the fix in reach —
           // including `public-url`, which the API treats as recommended but
-          // this form requires.
-          if (verify.result.checks.some((check) => !check.ok)) {
+          // this form requires. The one exception is `embed-cache` (#592):
+          // optional but recommended, so it never blocks the save — the tip
+          // is surfaced after the lane card appears instead.
+          const embedCacheWarn = verify.result.checks.find(
+            (check) => check.id === "embed-cache" && !check.ok,
+          );
+          if (verify.result.checks.some((check) => !check.ok && check.id !== "embed-cache")) {
             storageSaveBtn.disabled = false;
             renderFailures(verify.result);
             revealAdoptRowIfNotEmpty(verify.result);
@@ -940,6 +956,16 @@ if (!workspace) return Astro.redirect("/account/workspaces");
             // lane card appearing IS the confirmation — no "Saved." status
             // line lingering as page furniture (#788).
             applyStatus(saved.status);
+            // Saved fine, but the domain serves cacheable headers: leave the
+            // optional-but-recommended cache-rule tip on whichever lane card
+            // is now showing (#592). Plain text, no error state — the save
+            // succeeded.
+            if (embedCacheWarn) {
+              const tip =
+                "Optional but recommended: add a cache rule to this domain so GitHub embeds refresh after overwrites — see the setup guide.";
+              storageSavedActionStatus.textContent = tip;
+              storageActionStatus.textContent = tip;
+            }
             return;
           }
           if (saved.kind === "invalid") {

apps/web/src/pages/docs/byo-bucket.astro

@@ -33,7 +33,7 @@ const TOC = [
     </h2>
     <p>
       A workspace admin connects from the workspace settings page — one form, filled from three
-      things you set up on the Cloudflare dashboard first.
+      things you set up on the Cloudflare dashboard first (plus one optional cache rule).
     </p>
     <h3>1. Create an R2 bucket</h3>
     <p>
@@ -62,6 +62,21 @@ const TOC = [
       <code>https://media.example.com</code>) into the form as the public base URL. The settings
       page requires one — it's what makes links stable and embeddable. See the serving matrix below.
     </p>
+    <h3>4. Optional but recommended: add a cache rule for GitHub embeds</h3>
+    <p>
+      On hosted storage, images embedded in GitHub comments refresh when a file is overwritten in
+      place, because a dedicated embed host serves badge-style no-cache headers that GitHub's Camo
+      proxy revalidates. You can give your own domain the same behavior with one rule on your zone:
+      <strong>Rules</strong> → <strong>Transform Rules</strong> →
+      <strong>Modify Response Header</strong>, matching your domain's hostname, setting
+      <code>Cache-Control</code> to <code>max-age=0, no-cache, no-store, must-revalidate</code>.
+      Verification checks for it and reminds you if it's missing — without it everything still
+      works, but a GitHub embed can keep showing the old bytes after an in-place overwrite. The
+      trade-off: these headers turn off edge caching for the whole domain, which is negligible for
+      screenshot workflows (R2 egress is free) but worth weighing if the same bucket serves
+      high-traffic assets — in that case, connect a second custom domain to the bucket and scope the
+      rule to just that host.
+    </p>
     <p>
       Then <strong>Verify &amp; save</strong>: one click checks the settings, signs in to the
       bucket, round-trips a test object, and fetches it through your public URL. Anything that fails
@@ -135,10 +150,11 @@ const TOC = [
     </h2>
     <ul>
       <li>
-        <strong>GitHub embeds.</strong> Shared-bucket images get an embed host that GitHub's Camo proxy
-        revalidates after overwrite. That twin doesn't extend to a custom domain, so a BYO image without
-        a public URL (signed-only) won't render inline in a GitHub comment. The file still uploads and
-        still has a share page.
+        <strong>GitHub embeds.</strong> Hosted-storage images get a dedicated embed host that GitHub's
+        Camo proxy revalidates after an in-place overwrite. On your own domain you get the same behavior
+        by adding the <a href="#setup">cache rule above</a> — without it, embeds still render but may
+        show stale bytes after an overwrite. A BYO image with no public URL at all (signed-only) won't
+        render inline in a GitHub comment; the file still uploads and still has a share page.
       </li>
       <li>
         <strong><code>retentionDays</code> auto-cleanup.</strong> Age-based retention walks a prefix on