Skip to content
Open
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
67 changes: 65 additions & 2 deletions src/client/PageReviewCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ import remarkGfm from 'remark-gfm';
import { pageReviewSchema } from '../shared/page-review';
import {
decidePageReview,
fetchReviewTarget,
isDeletedReview,
matchesReviewedDraft,
restorePageReview,
type DeletedReview,
} from './page-review-decision';
import type { ReviewTarget } from './page-review-decision';
import { openPageLink } from './page-navigation';
import type { ReviewedPage } from '../server/pages';
export function approveLabel(isUpdate: boolean) {
return isUpdate ? 'Approve & update page' : 'Approve & create page';
}

export function PageReviewCard({
args,
status,
Expand All @@ -35,6 +41,11 @@ export function PageReviewCard({
const [busy, setBusy] = useState(false);
const [receiptReady, setReceiptReady] = useState(false);
const [restoreAttempt, setRestoreAttempt] = useState(0);
const [target, setTarget] = useState<ReviewTarget | null>(null);
const [targetStatus, setTargetStatus] = useState<
'idle' | 'pending' | 'identified' | 'failed'
>('idle');
const [targetLookupAttempt, setTargetLookupAttempt] = useState(0);
const pending = useRef(false);
const finished = status === 'complete';
const reviewed = savedPage ?? deletedReview;
Expand All @@ -43,6 +54,36 @@ export function PageReviewCard({
const saved = (!!savedPage || removed) && !conflict;
const pageId = savedPage?.id ?? '';
const spaceId = savedPage?.spaceId ?? '';
const targetPageId = draft.success ? (draft.data.pageId ?? null) : null;
const targetSpaceId = draft.success ? draft.data.spaceId : null;
// An update approval is only allowed once the page it would overwrite has
// been identified; a pending or failed lookup must not leave an enabled
// approve button behind an unidentified "Updates an existing page." card.
const targetGated = !!targetPageId && targetStatus !== 'identified';
useEffect(() => {
if (!targetPageId || !targetSpaceId) {
setTarget(null);
setTargetStatus('idle');
return;
}
let active = true;
setTarget(null);
setTargetStatus('pending');
void fetchReviewTarget(targetSpaceId, targetPageId)
.then((found) => {
if (!active) return;
setTarget(found);
setTargetStatus(found ? 'identified' : 'failed');
})
.catch(() => {
if (!active) return;
setTarget(null);
setTargetStatus('failed');
});
return () => {
active = false;
};
}, [targetPageId, targetSpaceId, targetLookupAttempt]);
useEffect(() => {
let active = true;
setReceiptReady(false);
Expand Down Expand Up @@ -70,6 +111,9 @@ export function PageReviewCard({
}, [threadId, toolCallId, restoreAttempt]);
const decide = async (approved: boolean) => {
if (!respond || !receiptReady || conflict || pending.current) return;
// Continuing an already-saved review stays independent of the target
// lookup; only a new update approval is gated on identification.
if (approved && !saved && targetGated) return;
pending.current = true;
setBusy(true);
setError('');
Expand Down Expand Up @@ -145,6 +189,17 @@ export function PageReviewCard({
</span>
</header>
<div className="page-review-body">
{targetPageId && (
<p className="page-review-target">
{targetStatus === 'identified' && target
? `Updates existing page "${target.title}"${
target.spaceName ? ` in ${target.spaceName}` : ''
}.`
: targetStatus === 'failed'
? 'Could not identify the page this would update.'
: 'Identifying the page this would update…'}
</p>
)}
<h3>{draft.success ? draft.data.title : 'Preparing your draft…'}</h3>
{draft.success && (
<ReactMarkdown
Expand All @@ -169,6 +224,14 @@ export function PageReviewCard({
</p>
)}
{error && <p role="alert">{error}</p>}
{targetPageId && targetStatus === 'failed' && !saved && (
<button
type="button"
onClick={() => setTargetLookupAttempt((attempt) => attempt + 1)}
>
Retry target lookup
</button>
)}
{!receiptReady && error && (
<button
type="button"
Expand Down Expand Up @@ -196,7 +259,7 @@ export function PageReviewCard({
<>
<button
type="button"
disabled={busy || (!saved && !draft.success)}
disabled={busy || (!saved && (!draft.success || targetGated))}
className="review-primary"
onClick={() => void decide(true)}
>
Expand All @@ -205,7 +268,7 @@ export function PageReviewCard({
? 'Saving…'
: saved
? 'Continue conversation'
: 'Approve & save'}
: approveLabel(!!targetPageId)}
</button>
{!saved && (
<button
Expand Down
38 changes: 36 additions & 2 deletions src/client/page-review-decision.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { pageReviewSchema, type PageReviewDraft } from '../shared/page-review';
import type { ReviewedPage } from '../server/pages';
import type { Page, ReviewedPage } from '../server/pages';
import { api } from './api';

const reviewPath = (threadId: string) =>
Expand Down Expand Up @@ -33,7 +33,11 @@ export function matchesReviewedDraft(
draft.success &&
draft.data.title === page.reviewDraft.title &&
draft.data.content === page.reviewDraft.content &&
draft.data.spaceId === page.reviewDraft.spaceId
draft.data.spaceId === page.reviewDraft.spaceId &&
(draft.data.pageId ?? undefined) ===
(page.reviewDraft.pageId ?? undefined) &&
(draft.data.expectedRevision ?? undefined) ===
(page.reviewDraft.expectedRevision ?? undefined)
);
}

Expand All @@ -59,3 +63,33 @@ export async function decidePageReview(
toolCallId,
});
}

export interface ReviewTarget {
pageId: string;
title: string;
spaceId: string;
spaceName: string | null;
}

// The approval card must identify which existing page an update would
// overwrite before the owner approves: identical drafts targeting
// different pages otherwise render identical cards.
export async function fetchReviewTarget(
spaceId: string,
pageId: string | null | undefined,
): Promise<ReviewTarget | null> {
if (!pageId) return null;
const page = await api<Page>(
`/spaces/${encodeURIComponent(spaceId)}/pages/${encodeURIComponent(pageId)}`,
);
const workspace = await api<{ spaces: { id: string; name: string }[] }>(
'/workspace',
).catch(() => null);
return {
pageId: page.id,
title: page.title,
spaceId,
spaceName:
workspace?.spaces.find((space) => space.id === spaceId)?.name ?? null,
};
}
9 changes: 9 additions & 0 deletions src/client/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -3753,6 +3753,15 @@ h3 {
font-size: 13px;
line-height: 1.65;
}
.page-review-target {
margin: 0 0 12px;
padding: 8px 12px;
border-radius: 8px;
background: rgba(255, 176, 32, 0.12);
border: 1px solid rgba(255, 176, 32, 0.35);
font-size: 12.5px;
font-weight: 600;
}
.page-review-body h3 {
font-size: 20px;
margin: 0 0 12px;
Expand Down
96 changes: 65 additions & 31 deletions src/server/pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,21 @@ export class Pages {
}
createReviewed(
spaceId: string,
input: Pick<PageReviewDraft, 'title' | 'content'>,
input: Pick<
PageReviewDraft,
'title' | 'content' | 'pageId' | 'expectedRevision'
>,
threadId: string,
toolCallId: string,
): ReviewedPage {
const draft = pageReviewSchema.parse({ ...input, spaceId });
// Strict tool schemas send omitted optional fields as null.
const parsed = pageReviewSchema.parse({ ...input, spaceId });
const { pageId, expectedRevision, ...rest } = parsed;
const draft: PageReviewDraft = {
...rest,
...(pageId ? { pageId } : {}),
...(expectedRevision ? { expectedRevision } : {}),
};
this.db.exec('BEGIN IMMEDIATE');
try {
const previous = this.reviewReceipt(threadId, toolCallId);
Expand All @@ -172,7 +182,9 @@ export class Pages {
if (
previous.draft &&
(previous.draft.title !== draft.title ||
previous.draft.content !== draft.content)
previous.draft.content !== draft.content ||
previous.draft.pageId !== draft.pageId ||
previous.draft.expectedRevision !== draft.expectedRevision)
)
throw new PageError(
'This review was already saved with a different draft. Start a new review for the changed draft.',
Expand All @@ -182,11 +194,27 @@ export class Pages {
this.db.exec('COMMIT');
return { ...page, reviewDraft: previous.draft };
}
const page = this.create(
spaceId,
{ title: draft.title, content: draft.content },
threadId,
);
if (draft.pageId && draft.expectedRevision === undefined)
throw new PageError(
'Revising an existing page needs the revision the draft was based on.',
400,
);
if (!draft.pageId && draft.expectedRevision !== undefined)
throw new PageError(
'A revision can only be given together with the page to revise.',
400,
);
const page = draft.pageId
? this.applyUpdate(spaceId, draft.pageId, {

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.

[P2] Show which existing page will be updated before approval, and distinguish that action from creating a page. This branch replaces an existing page’s title and content, but the review card only shows the proposed draft and “Approve & save.” Otherwise identical drafts targeting two different pages render identical cards, so the owner cannot verify the overwrite target. Include the existing target title/location and an explicit update action in the card, with coverage for distinct targets.

title: draft.title,
content: draft.content,
expectedRevision: draft.expectedRevision!,
})
: this.create(
spaceId,
{ title: draft.title, content: draft.content },
threadId,
);
this.db
.prepare(
'INSERT INTO page_reviews (threadId,toolCallId,pageId,spaceId,draft) VALUES (?,?,?,?,?)',
Expand All @@ -205,37 +233,43 @@ export class Pages {
throw new PageError(
'A valid page patch and expectedRevision are required.',
);
const data = parsed.data;
this.db.exec('BEGIN IMMEDIATE');
try {
const page = this.get(spaceId, id);
if (page.revision !== data.expectedRevision)
throw new PageError(
'This page changed. Reload the latest revision before saving your draft.',
409,
);
const parent =
data.parentId === undefined ? page.parentId : data.parentId;
this.parent(spaceId, parent, id);
this.db
.prepare(
'UPDATE pages SET title=?,content=?,parentId=?,revision=revision+1,updatedAt=? WHERE id=? AND revision=?',
)
.run(
data.title ?? page.title,
data.content ?? page.content,
parent,
Date.now(),
id,
data.expectedRevision,
);
const page = this.applyUpdate(spaceId, id, parsed.data);
this.db.exec('COMMIT');
return this.get(spaceId, id);
return page;
} catch (error) {
this.db.exec('ROLLBACK');
throw error;
}
}
private applyUpdate(
spaceId: string,
id: string,
data: z.output<typeof pagePatch>,
): Page {
const page = this.get(spaceId, id);
if (page.revision !== data.expectedRevision)
throw new PageError(
'This page changed. Reload the latest revision before saving your draft.',
409,
);
const parent = data.parentId === undefined ? page.parentId : data.parentId;
this.parent(spaceId, parent, id);
this.db
.prepare(
'UPDATE pages SET title=?,content=?,parentId=?,revision=revision+1,updatedAt=? WHERE id=? AND revision=?',
)
.run(
data.title ?? page.title,
data.content ?? page.content,
parent,
Date.now(),
id,
data.expectedRevision,
);
return this.get(spaceId, id);
}
thread(pageId: string, dotId: string) {
const row = this.db
.prepare(
Expand Down
4 changes: 3 additions & 1 deletion src/shared/page-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ export const pageReviewSchema = z
title: z.string().trim().min(1).max(160),
content: z.string().min(1).max(20000),
spaceId: z.string().min(1),
pageId: z.string().min(1).nullish(),
expectedRevision: z.number().int().positive().nullish(),
})
.strict();
export type PageReviewDraft = z.infer<typeof pageReviewSchema>;
export const pageReviewTool = {
name: 'review_space_page',
description:
'Present a Markdown draft for human review before saving it into an authorized Space. The user can approve and save, or decline. Do not create the page yourself after this tool: its approved result includes the saved page URL. Call once, then wait for the review result.',
'Present a Markdown draft for human review before saving it into an authorized Space. To revise an existing page instead of creating a new one, pass its pageId and the expectedRevision from read_space_page; approving then updates that page in place. The user can approve and save, or decline. Do not create the page yourself after this tool: its approved result includes the saved page URL. Call once, then wait for the review result.',
parameters: z.toJSONSchema(pageReviewSchema),
};
Loading
Loading