fix(canvas): publish button back in header, real Settings menu

apps/web/src/app/api/pages/[pageId]/form-target/route.ts

@@ -1,6 +1,6 @@
 /**
- * Authenticated Forms settings API for a Canvas page — lets the Forms tab
- * (apps/web/.../canvas/CanvasFormsSettingsTab.tsx) wire an existing <form>
+ * Authenticated Forms settings API for a Canvas page — lets the Forms category
+ * (apps/web/.../canvas/settings/CanvasFormsSettingsSection.tsx) wire an existing <form>
  * tag on the page to a Sheet, and manage the resulting grant, without going
  * through the AI tool (apps/web/src/lib/ai/tools/form-tools.ts). `pageId`
  * here is the CANVAS page's id, not the target Sheet's — targets are looked

apps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.ts

@@ -21,9 +21,11 @@ vi.mock('@/lib/auth', () => ({
   isAuthError: (result: unknown) => typeof result === 'object' && result !== null && 'error' in result,
   checkMCPPageScope: vi.fn().mockResolvedValue(null),
   canPrincipalEditPage: (auth: { userId: string }, pageId: string) => canUserEditPage(auth.userId, pageId),
+  canPrincipalViewPage: (auth: { userId: string }, pageId: string) => canUserViewPage(auth.userId, pageId),
 }));
 
 const canUserEditPage = vi.fn();
+const canUserViewPage = vi.fn();
 vi.mock('@pagespace/lib/permissions/permissions', () => ({
   canUserEditPage: (...args: unknown[]) => canUserEditPage(...args),
 }));
@@ -130,6 +132,7 @@ beforeEach(() => {
   vi.clearAllMocks();
   authenticateRequestWithOptions.mockResolvedValue({ userId: 'user-1' });
   canUserEditPage.mockResolvedValue(true);
+  canUserViewPage.mockResolvedValue(true);
   allocatePublishSubdomain.mockResolvedValue('acme');
   isPublishConfigured.mockReturnValue(true);
   getPublishAssetBaseUrl.mockReturnValue('https://test-publish.t3.tigrisfiles.io');
@@ -239,6 +242,7 @@ describe('POST /api/pages/[pageId]/publish', () => {
       description: null,
       ogImageUrl: null,
       noindex: false,
+      themeBridgeEnabled: true,
     });
   });
 
@@ -262,6 +266,7 @@ describe('POST /api/pages/[pageId]/publish', () => {
       description: null,
       ogImageUrl: null,
       noindex: false,
+      themeBridgeEnabled: true,
     });
   });
 
@@ -291,6 +296,7 @@ describe('POST /api/pages/[pageId]/publish', () => {
       description: null,
       ogImageUrl: null,
       noindex: false,
+      themeBridgeEnabled: true,
     });
   });
 
@@ -365,6 +371,18 @@ describe('POST /api/pages/[pageId]/publish', () => {
     }));
   });
 
+  it('accepts themeBridgeEnabled: false and threads it into the rendered page as injectThemeBridge: false', async () => {
+    findFirstPage.mockResolvedValue({ id: 'page-1', type: 'CANVAS', title: 'Welcome', content: '<p>hi</p>', driveId: 'drive-1' });
+    findFirstDrive.mockResolvedValue({ id: 'drive-1', slug: 'acme', publishSubdomain: 'acme', kind: 'STANDARD' });
+    findFirstPublished.mockResolvedValue(null);
+
+    const res = await POST(makeReq({ themeBridgeEnabled: false }), { params });
+    expect(res.status).toBe(200);
+    expect(renderPublishedPage).toHaveBeenCalledWith(expect.objectContaining({ injectThemeBridge: false }));
+    const json = await res.json();
+    expect(json.themeBridgeEnabled).toBe(false);
+  });
+
   it('returns 400 when ogImageUrl is not a valid URL', async () => {
     findFirstPage.mockResolvedValue({ id: 'page-1', type: 'CANVAS', title: 'Welcome', content: '<p>hi</p>', driveId: 'drive-1' });
     findFirstDrive.mockResolvedValue({ id: 'drive-1', slug: 'acme', publishSubdomain: 'acme', kind: 'STANDARD' });
@@ -457,18 +475,52 @@ describe('POST /api/pages/[pageId]/publish', () => {
 });
 
 describe('GET /api/pages/[pageId]/publish', () => {
-  it('returns 403 when the user cannot edit the page', async () => {
+  it('returns 403 when the user can neither edit nor view the page', async () => {
     canUserEditPage.mockResolvedValue(false);
+    canUserViewPage.mockResolvedValue(false);
     const res = await GET(makeReq(), { params });
     expect(res.status).toBe(403);
   });
 
-  it('returns { published: false, available: true } when no row exists and publishing is configured', async () => {
+  it('given view-but-not-edit access to a published page, returns 200 with only the rendering-relevant field', async () => {
+    canUserEditPage.mockResolvedValue(false);
+    canUserViewPage.mockResolvedValue(true);
+    findFirstPublished.mockResolvedValue({
+      driveId: 'drive-1', path: 'welcome', publishedAt: new Date(), updatedAt: new Date(), themeBridgeEnabled: false,
+    });
+
+    const res = await GET(makeReq(), { params });
+    expect(res.status).toBe(200);
+    const json = await res.json();
+    // Exact-equal so a future change to the route can't silently start
+    // leaking edit-oriented fields (SEO overrides, url, isStale) to a
+    // view-only collaborator without this test catching it.
+    expect(json).toEqual({ published: true, canEdit: false, themeBridgeEnabled: false });
+    // The drive/domain lookups are skipped entirely for a view-only caller.
+    expect(findFirstDrive).not.toHaveBeenCalled();
+  });
+
+  it('returns { published: false, available: true, canEdit: true } when no row exists and publishing is configured', async () => {
     findFirstPublished.mockResolvedValue(undefined);
     const res = await GET(makeReq(), { params });
     expect(res.status).toBe(200);
     const json = await res.json();
-    expect(json).toEqual({ published: false, available: true });
+    expect(json).toEqual({ published: false, available: true, canEdit: true });
+  });
+
+  it('given view-but-not-edit access to an unpublished page, returns canEdit: false (not just available)', async () => {
+    // Regression test: PublishControls decides whether to show an enabled
+    // Publish button from `available` alone on this branch — without
+    // `canEdit` here too, a view-only viewer would see an enabled button
+    // that only reveals the permission failure once its POST 403s.
+    canUserEditPage.mockResolvedValue(false);
+    canUserViewPage.mockResolvedValue(true);
+    findFirstPublished.mockResolvedValue(undefined);
+
+    const res = await GET(makeReq(), { params });
+    expect(res.status).toBe(200);
+    const json = await res.json();
+    expect(json).toEqual({ published: false, available: true, canEdit: false });
   });
 
   it('returns available: false when the page is in a Home drive (no published row)', async () => {
@@ -489,7 +541,7 @@ describe('GET /api/pages/[pageId]/publish', () => {
     const res = await GET(makeReq(), { params });
     expect(res.status).toBe(200);
     const json = await res.json();
-    expect(json).toEqual({ published: false, available: false });
+    expect(json).toEqual({ published: false, available: false, canEdit: true });
   });
 
   it('reports available: false when the kill-switch is engaged even if the bucket is configured', async () => {
@@ -502,7 +554,7 @@ describe('GET /api/pages/[pageId]/publish', () => {
       expect(res.status).toBe(200);
       const json = await res.json();
       // POST would 503 ("temporarily disabled") here, so the UI must hide the control.
-      expect(json).toEqual({ published: false, available: false });
+      expect(json).toEqual({ published: false, available: false, canEdit: true });
     } finally {
       if (prev === undefined) delete process.env.CANVAS_PUBLISHING_DISABLED;
       else process.env.CANVAS_PUBLISHING_DISABLED = prev;
@@ -521,6 +573,7 @@ describe('GET /api/pages/[pageId]/publish', () => {
     const json = await res.json();
     expect(json).toEqual({
       published: true,
+      canEdit: true,
       available: true,
       isStale: false,
       url: 'https://acme.pagespace.site/welcome',
@@ -531,6 +584,7 @@ describe('GET /api/pages/[pageId]/publish', () => {
       description: null,
       ogImageUrl: null,
       noindex: false,
+      themeBridgeEnabled: true,
     });
   });
 

apps/web/src/app/api/pages/[pageId]/publish/route.ts

@@ -2,7 +2,7 @@ import { NextResponse } from 'next/server';
 import { z } from 'zod/v4';
 import { loggers } from '@pagespace/lib/logging/logger-config';
 import { auditRequest } from '@pagespace/lib/audit/audit-log';
-import { authenticateRequestWithOptions, isAuthError, checkMCPPageScope, canPrincipalEditPage } from '@/lib/auth';
+import { authenticateRequestWithOptions, isAuthError, checkMCPPageScope, canPrincipalEditPage, canPrincipalViewPage } from '@/lib/auth';
 import { isPublishConfigured } from '@/lib/canvas/published-storage';
 import { publishCanvasPage, clearPublishedHomeRoot, regeneratePublishedSiteFiles, PublishError, PUBLISH_HOST } from '@/lib/canvas/publish-page';
 import { deletePageFromCustomHosts, getActiveDomainRecords } from '@/lib/canvas/custom-domain-mirror';
@@ -29,6 +29,9 @@ const publishSchema = z.object({
   // publishCanvasPage — never trusted as a URL directly.
   ogImageFileId: z.string().min(1).optional(),
   noindex: z.boolean().optional(),
+  // Canvas pages only: whether the published artifact follows PageSpace's
+  // light/dark theme. Ignored (has no effect) for other publishable page types.
+  themeBridgeEnabled: z.boolean().optional(),
 }).nullable();
 
 export async function GET(req: Request, { params }: { params: Promise<{ pageId: string }> }) {
@@ -41,8 +44,14 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId:
   const scopeError = await checkMCPPageScope(auth, pageId);
   if (scopeError) return scopeError;
 
+  // A viewer who can't edit still needs this page's `themeBridgeEnabled` —
+  // it drives the canvas View tab's live preview for EVERY collaborator, not
+  // just editors, and it's not sensitive (it's baked into the publicly
+  // published HTML either way). Everything else this route returns (SEO
+  // overrides, publish URL, staleness) stays edit-gated below.
   const canEdit = await canPrincipalEditPage(auth, pageId);
-  if (!canEdit) {
+  const canView = canEdit || await canPrincipalViewPage(auth, pageId);
+  if (!canView) {
     return NextResponse.json({ error: 'You do not have permission to view this page' }, { status: 403 });
   }
 
@@ -61,6 +70,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId:
         publishDescription: true,
         publishOgImageUrl: true,
         noindex: true,
+        themeBridgeEnabled: true,
       },
     });
 
@@ -78,7 +88,21 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId:
           if (drv?.kind === 'HOME') available = false;
         }
       }
-      return NextResponse.json({ published: false, available });
+      // `canEdit` matters even for an unpublished page: a view-only caller
+      // reaching here (canView but !canEdit) must not have PublishControls
+      // read `available` alone and offer an enabled Publish button that
+      // only reveals the permission failure once its POST 403s.
+      return NextResponse.json({ published: false, available, canEdit });
+    }
+
+    // View-only: stop here with just the rendering-relevant field. Skip the
+    // drive/domain/staleness lookups below — nothing else in this response
+    // is meant for a non-editor (SEO overrides, the publish URL, staleness).
+    // `canEdit: false` tells consumers like the Settings tab's Publish/
+    // Appearance categories not to treat `published: true` as "safe to show
+    // editable, savable fields" — those two are otherwise-independent facts.
+    if (!canEdit) {
+      return NextResponse.json({ published: true, canEdit: false, themeBridgeEnabled: row.themeBridgeEnabled ?? true });
     }
 
     const [drive, livePage] = await Promise.all([
@@ -119,6 +143,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId:
 
     return NextResponse.json({
       published: true,
+      canEdit: true,
       available,
       isStale,
       url,
@@ -130,6 +155,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId:
       description: row.publishDescription ?? null,
       ogImageUrl: row.publishOgImageUrl ?? null,
       noindex: row.noindex ?? false,
+      themeBridgeEnabled: row.themeBridgeEnabled ?? true,
     });
   } catch (error) {
     loggers.api.error('Error reading publish status:', error as Error);
@@ -208,6 +234,7 @@ export async function POST(req: Request, { params }: { params: Promise<{ pageId:
       description: parsedBody?.description,
       ogImageUrl,
       noindex: parsedBody?.noindex,
+      themeBridgeEnabled: parsedBody?.themeBridgeEnabled,
     });
 
     auditRequest(req, {

apps/web/src/components/canvas/CanvasFrame.tsx

@@ -15,6 +15,10 @@ import { useNonce } from '@/contexts/NonceContext';
 interface CanvasFrameProps {
   html: string;
   title?: string;
+  /** Mirrors the persisted `published_pages.themeBridgeEnabled` setting (see
+   *  the canvas Settings tab's Appearance category) so the editor's live
+   *  preview matches what publishing will produce. Defaults to `true`. */
+  themeBridgeEnabled?: boolean;
 }
 
 /**
@@ -69,7 +73,7 @@ function hasRecentUserActivation(): boolean {
  * The document string is the same one produced for published pages, so the
  * in-app view and the published artifact render identically.
  */
-export function CanvasFrame({ html, title }: CanvasFrameProps) {
+export function CanvasFrame({ html, title, themeBridgeEnabled = true }: CanvasFrameProps) {
   const nonce = useNonce();
   const router = useRouter();
   const [previewHtml, setPreviewHtml] = useState(html);
@@ -119,14 +123,16 @@ export function CanvasFrame({ html, title }: CanvasFrameProps) {
   // baseTarget '_blank': inside the sandboxed frame an ordinary <a href> (no
   // target) would navigate the frame itself — and many sites refuse framing —
   // so default links to a new tab (works with the iframe's allow-popups).
-  // injectThemeBridge: true injects a script that listens for theme messages
-  // so the canvas iframe's dark/light state matches the app's current theme.
+  // injectThemeBridge injects a script that listens for theme messages so the
+  // canvas iframe's dark/light state matches the app's current theme — unless
+  // the author disabled it (canvas Settings > Appearance) for a design that
+  // shouldn't be fought by an injected `dark` class.
   // navigationBridge: true injects a script that intercepts clicks on internal
   // dashboard page links and hands them to the message handler below, so they
   // route in-app instead of falling through to baseTarget's new-tab behavior.
   const srcDoc = useMemo(
-    () => renderCanvasDocument({ html: previewHtml, title, baseTarget: '_blank', injectThemeBridge: true, navigationBridge: true, nonce }),
-    [previewHtml, title, nonce],
+    () => renderCanvasDocument({ html: previewHtml, title, baseTarget: '_blank', injectThemeBridge: themeBridgeEnabled, navigationBridge: true, nonce }),
+    [previewHtml, title, nonce, themeBridgeEnabled],
   );
 
   // Send the current theme to the canvas iframe whenever it changes or the