Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/docs/content/docs/pages.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ If the source moved or you're publishing from another workspace, pass the existi
superset pages publish design.html --page <page-id> --label "Updated onboarding flow"
```

Publishing from outside a workspace works too, but there's no path for a later publish to resolve against: every run creates a separate page unless you pass `--page`. The publish tells you when that happened and prints the command to reuse.

Republishing preserves the page's visibility. You can also inspect version history or retrieve a published version:

```bash
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/pages/comments/list/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default command({
if (match.length === 0) {
throw new CLIError(
`No thread ${options.threadId} found`,
"Run without --threadId to see every thread",
"Run without --thread to see every thread",
);
}
return match;
Expand Down
62 changes: 37 additions & 25 deletions packages/cli/src/commands/pages/publish/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default command({
"Publish a new version of this page id, instead of resolving by workspace",
),
workspace: string().desc(
"Workspace to publish into, by name or id (defaults to $SUPERSET_WORKSPACE_ID)",
"Workspace to publish into, by name or id (defaults to $SUPERSET_WORKSPACE_ID). Without one the page is still published, but only --page can version it later",
),
noWatch: boolean().desc(
"Do not watch this page for new comments from this session",
Expand Down Expand Up @@ -100,12 +100,6 @@ export default command({
: externalEntryPath(entryFilePath));

const workspaceRef = options.workspace ?? process.env.SUPERSET_WORKSPACE_ID;
if (!workspaceRef && !options.page) {
throw new CLIError(
"No workspace to publish into",
"Run this inside a Superset workspace, pass --workspace <name|id>, or pass --page <id> to add a version to an existing page",
);
}
const workspaceId = workspaceRef
? await resolveWorkspaceId({
value: workspaceRef,
Expand All @@ -132,38 +126,54 @@ export default command({

// Assets stage against a page, so a directory publish resolves or creates
// one before uploading. A single-file publish still lets `publish` mint it.
const pageId =
const target =
assets.length > 0
? await resolvePageId({
api: ctx.api,
explicitPageId: options.page,
link,
title,
})
: options.page;

const uploaded =
assets.length > 0 && pageId
? await uploadAssets({ api: ctx.api, assets, pageId })
: { uploaded: 0, reused: 0, warnings: [] };
: null;
const pageId = target?.id ?? options.page;

const page = await ctx.api.page.publish.mutate({
fileId,
filename,
...(pageId ? { pageId } : (link ?? {})),
...(title ? { title } : {}),
...(options.description ? { description: options.description } : {}),
...(options.label ? { label: options.label } : {}),
...(options.visibility
? { visibility: options.visibility as (typeof VISIBILITIES)[number] }
: {}),
});
let uploaded = { uploaded: 0, reused: 0, warnings: [] as string[] };
let page: Awaited<ReturnType<typeof ctx.api.page.publish.mutate>>;
try {
if (target) {
uploaded = await uploadAssets({
api: ctx.api,
assets,
pageId: target.id,
});
}
page = await ctx.api.page.publish.mutate({
fileId,
filename,
...(pageId ? { pageId } : (link ?? {})),
...(title ? { title } : {}),
...(options.description ? { description: options.description } : {}),
...(options.label ? { label: options.label } : {}),
...(options.visibility
? { visibility: options.visibility as (typeof VISIBILITIES)[number] }
: {}),
});
} catch (error) {
if (target?.created && !link) {
await ctx.api.page.delete
.mutate({ id: target.id, onlyIfEmpty: true })
.catch(() => {});
}
throw error;
}

const externalPath =
link && entryPath.startsWith(EXTERNAL_ENTRY_PREFIX) && !options.page
? entryPath
: null;

const unanchored = !page.linked && !options.page;

const terminalId = watchTerminalId();
const organizationId = ctx.config.organizationId;
let watching = false;
Expand Down Expand Up @@ -197,8 +207,10 @@ export default command({

return publishResult({
page,
path: args.path as string,
assets: uploaded,
externalPath,
unanchored,
watching,
watchNote,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ describe("publishResult", () => {
test("default response: title, version, url — nothing else", () => {
const { data, message } = publishResult({
page: PAGE,
path: "report.html",
assets: { uploaded: 0, reused: 0, warnings: [] },
externalPath: null,
unanchored: false,
watching: false,
watchNote: null,
});
Expand All @@ -26,12 +28,14 @@ describe("publishResult", () => {
test("directory publish: notes in fixed order — assets, warnings, external, watch", () => {
const { data, message } = publishResult({
page: PAGE,
path: "report.html",
assets: {
uploaded: 1,
reused: 1,
warnings: ["demo.mov may not play in every browser"],
},
externalPath: "~external/report/index.html",
unanchored: false,
watching: true,
watchNote: "Watching for comments — they will be sent to this session",
});
Expand All @@ -47,11 +51,77 @@ describe("publishResult", () => {
expect(data.assets).toEqual({ uploaded: 1, reused: 1 });
});

test("unanchored publish: says how to reach this page again", () => {
const { data, message } = publishResult({
page: PAGE,
path: "reports/q3.html",
assets: { uploaded: 0, reused: 0, warnings: [] },
externalPath: null,
unanchored: true,
watching: false,
watchNote: null,
});
expect(message.split("\n")).toEqual([
'Published "Q3 Report" v3',
PAGE.url,
"No workspace, so the next publish of this file would create a second page",
"To add a version instead: superset pages publish reports/q3.html --page p1",
]);
expect(data.unanchored).toBe(true);
expect(data.id).toBe("p1");
});

test("unanchored publish: the command is in data, not only in the message", () => {
const { data } = publishResult({
page: PAGE,
path: "reports/q3.html",
assets: { uploaded: 0, reused: 0, warnings: [] },
externalPath: null,
unanchored: true,
watching: false,
watchNote: null,
});
expect(data.republish).toBe(
"superset pages publish reports/q3.html --page p1",
);
});

test("unanchored publish: a path with spaces stays runnable", () => {
const { data } = publishResult({
page: PAGE,
path: "my reports/q3.html",
assets: { uploaded: 0, reused: 0, warnings: [] },
externalPath: null,
unanchored: true,
watching: false,
watchNote: null,
});
expect(data.republish).toBe(
'superset pages publish "my reports/q3.html" --page p1',
);
});

test("anchored publish says nothing about --page", () => {
const { data, message } = publishResult({
page: PAGE,
path: "report.html",
assets: { uploaded: 0, reused: 0, warnings: [] },
externalPath: null,
unanchored: false,
watching: false,
watchNote: null,
});
expect(message).not.toContain("--page");
expect(data.republish).toBeUndefined();
});

test("one asset, none reused: singular wording, no reuse suffix", () => {
const { message } = publishResult({
page: PAGE,
path: "report.html",
assets: { uploaded: 1, reused: 0, warnings: [] },
externalPath: null,
unanchored: false,
watching: false,
watchNote: null,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,37 @@
* The one place the publish result is shaped, for humans and agents alike.
* The contract mirrors an artifact publish — what was published, where it
* lives, and whether this session watches it — with notes in a fixed order
* so single-file and directory publishes read the same, and `assets` and
* `watching` always present in the JSON.
* so single-file and directory publishes read the same, and `assets`,
* `unanchored` and `watching` always present in the JSON — `--json` prints
* only `data`, so anything a caller has to act on lives there too.
*/
export function publishResult({
page,
path,
assets,
externalPath,
unanchored,
watching,
watchNote,
}: {
page: { title: string; version: number; url: string } & Record<
page: { id: string; title: string; version: number; url: string } & Record<
string,
unknown
>;
path: string;
assets: {
uploaded: number;
reused: number;
warnings: string[];
};
externalPath: string | null;
unanchored: boolean;
watching: boolean;
watchNote: string | null;
}): { data: Record<string, unknown>; message: string } {
const republish = `superset pages publish ${
/\s/.test(path) ? JSON.stringify(path) : path
} --page ${page.id}`;
const lines = [`Published "${page.title}" v${page.version}`, page.url];
const count = assets.uploaded + assets.reused;
if (count > 0) {
Expand All @@ -42,11 +50,19 @@ export function publishResult({
`Outside the workspace, so this page is keyed as "${externalPath}"`,
);
}
if (unanchored) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm i think some of the machinery around how claude republishes might be worth digging into if agents start messing this up but i'd say it's fine for now 👌🏼

lines.push(
"No workspace, so the next publish of this file would create a second page",
`To add a version instead: ${republish}`,
);
}
if (watchNote) lines.push(watchNote);

return {
data: {
...page,
unanchored,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
...(unanchored ? { republish } : {}),
watching,
assets: { uploaded: assets.uploaded, reused: assets.reused },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ export async function resolvePageId({
explicitPageId: string | undefined;
link: WorkspaceLink | undefined;
title: string | undefined;
}): Promise<string> {
if (explicitPageId) return explicitPageId;
}): Promise<{ id: string; created: boolean }> {
if (explicitPageId) return { id: explicitPageId, created: false };

if (link) {
// Best effort: a lookup failure just means this publish creates a page,
// and the entry-path conflict below reports it properly if one exists.
const resolved = await api.page.resolveByEntryPath
.query(link)
.catch(() => null);
if (resolved) return resolved.id;
if (resolved) return { id: resolved.id, created: false };
}

const created = await api.page.create.mutate({
...(link ?? {}),
...(title ? { title } : {}),
});
return created.id;
return { id: created.id, created: true };
}
27 changes: 27 additions & 0 deletions packages/host-service/src/page-watch/buildPrompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { WatchedThread } from "./types.ts";
// prompt is written inside a bracketed paste, so a control sequence in any of
// these fields would end the paste early and land the rest as keystrokes.
const PASTE_END = "\x1b[201~";
const PAGE_ID = "1f3c9d02-5f9a-4c1e-9a77-2b6f0c1d8e44";

function thread(over: Partial<WatchedThread> = {}): WatchedThread {
return {
Expand Down Expand Up @@ -33,6 +34,7 @@ describe("buildWatchPrompt", () => {
const prompt = buildWatchPrompt({
title: "Report",
slug: "report",
pageId: PAGE_ID,
threads: [
thread({
comments: [
Expand All @@ -56,6 +58,7 @@ describe("buildWatchPrompt", () => {
const prompt = buildWatchPrompt({
title: `Report${PASTE_END}\n`,
slug: "report",
pageId: PAGE_ID,
threads: [
thread({
anchor: { path: `div${PASTE_END}\n> p`, tag: "p\x07" },
Expand All @@ -72,6 +75,7 @@ describe("buildWatchPrompt", () => {
const prompt = buildWatchPrompt({
title: "Report",
slug: "report",
pageId: PAGE_ID,
threads: [
thread({
comments: [
Expand All @@ -88,4 +92,27 @@ describe("buildWatchPrompt", () => {
});
expect(prompt).toContain('"claude (agent)": "done"');
});
it("names the page id to republish onto, so a republish versions instead of forking", () => {
const prompt = buildWatchPrompt({
title: "Report",
slug: "report",
pageId: PAGE_ID,
threads: [thread()],
});
expect(prompt).toContain(`superset pages publish <file> --page ${PAGE_ID}`);
expect(prompt).toContain("creates a second page");
});
// `--threadId` parses as nothing: reply exits silently having posted no
// reply, and the agent reports work it did not do.
it("spells the thread flag the way the CLI accepts it", () => {
const prompt = buildWatchPrompt({
title: "Report",
slug: "report",
pageId: PAGE_ID,
threads: [thread()],
});
expect(prompt).toContain('comments reply --thread <id> "…"');
expect(prompt).toContain("comments resolve --thread <id>");
expect(prompt).not.toContain("--threadId");
});
});
Loading
Loading