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: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ _Ask Scout to open a website, summarize it, save notes, and verify the file. Eve

### Review before saving

Ask a Dot to show a draft before saving it. A CopilotKit human-in-the-loop card pauses the conversation for **Approve & save** or **Decline**. Approval creates the page in an authorized Space and returns a link; retries recover the same saved page. The agent continues after your decision.
Ask a Dot to show a draft before saving it. A CopilotKit human-in-the-loop card pauses the conversation for **Approve & save** or **Decline**. Approval creates the page in an authorized Space and returns a link; retries with the same draft recover that saved page. A changed draft needs a new review. The agent continues after your decision.

### Text and calls

Expand Down
77 changes: 43 additions & 34 deletions src/client/PageReviewCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import { useEffect, useRef, useState } from 'react';
import { Check, FileText, ArrowUpRight } from 'lucide-react';
import ReactMarkdown from 'react-markdown';
import { pageReviewSchema } from '../shared/page-review';
import { decidePageReview, restorePageReview } from './page-review-decision';
import { computerToolResult } from './ComputerToolCard';
import {
decidePageReview,
matchesReviewedDraft,
restorePageReview,
} from './page-review-decision';
import { openPageLink } from './page-navigation';
import type { Page } from '../server/pages';
import type { ReviewedPage } from '../server/pages';
export function PageReviewCard({
args,
status,
result,
respond,
threadId,
toolCallId,
Expand All @@ -24,25 +26,21 @@ export function PageReviewCard({
onSaved: () => void;
}) {
const draft = pageReviewSchema.safeParse(args);
const outcome = computerToolResult(result);
const [savedPage, setSavedPage] = useState<Page>();
const [savedPage, setSavedPage] = useState<ReviewedPage>();
const [error, setError] = useState('');
const [busy, setBusy] = useState(false);
const [receiptReady, setReceiptReady] = useState(false);
const [restoreAttempt, setRestoreAttempt] = useState(0);
const pending = useRef(false);
const finished = status === 'complete';
const recordedApproval = outcome.approved === true;
const saved = !!savedPage || recordedApproval;
const pageId =
savedPage?.id ?? (typeof outcome.pageId === 'string' ? outcome.pageId : '');
const spaceId =
savedPage?.spaceId ??
(typeof outcome.spaceId === 'string' ? outcome.spaceId : '');
const conflict = !!savedPage && !matchesReviewedDraft(savedPage, args);
const saved = !!savedPage && !conflict;
const pageId = savedPage?.id ?? '';
const spaceId = savedPage?.spaceId ?? '';
useEffect(() => {
if (recordedApproval) return;
let active = true;
setReceiptReady(false);
setSavedPage(undefined);
setError('');
void restorePageReview(threadId, toolCallId)
.then((page) => {
Expand All @@ -61,9 +59,9 @@ export function PageReviewCard({
return () => {
active = false;
};
}, [threadId, toolCallId, recordedApproval, restoreAttempt]);
}, [threadId, toolCallId, restoreAttempt]);
const decide = async (approved: boolean) => {
if (!respond || !receiptReady || pending.current) return;
if (!respond || !receiptReady || conflict || pending.current) return;
pending.current = true;
setBusy(true);
setError('');
Expand Down Expand Up @@ -101,22 +99,26 @@ export function PageReviewCard({
<header>
<FileText size={17} />
<strong>
{saved
? 'Saved to your Space'
: !receiptReady
? 'Checking saved review…'
: finished
? 'Review ended'
: 'Ready for your review'}
{conflict
? 'Review changed'
: saved
? 'Saved to your Space'
: !receiptReady
? 'Checking saved review…'
: finished
? 'Review ended'
: 'Ready for your review'}
</strong>
<span>
{saved
? 'Approved'
: !receiptReady
? 'Checking'
: finished
? 'Not saved'
: 'You decide'}
{conflict
? 'Needs new review'
: saved
? 'Approved'
: !receiptReady
? 'Checking'
: finished
? 'Not saved'
: 'You decide'}
</span>
</header>
<div className="page-review-body">
Expand All @@ -136,6 +138,12 @@ export function PageReviewCard({
</ReactMarkdown>
)}
</div>
{conflict && (
<p role="alert">
This review was saved with a different draft. Start a new review for
the changed draft.
</p>
)}
{error && <p role="alert">{error}</p>}
{!receiptReady && error && (
<button
Expand All @@ -146,7 +154,7 @@ export function PageReviewCard({
</button>
)}
<footer>
{saved && pageId && spaceId && (
{(saved || conflict) && pageId && spaceId && (
<button
type="button"
className="review-primary"
Expand All @@ -156,10 +164,11 @@ export function PageReviewCard({
)
}
>
Open page <ArrowUpRight size={15} />
{conflict ? 'Open saved page' : 'Open page'}{' '}
<ArrowUpRight size={15} />
</button>
)}
{!finished && respond && receiptReady && (
{!finished && respond && receiptReady && !conflict && (
<>
<button
type="button"
Expand All @@ -185,7 +194,7 @@ export function PageReviewCard({
)}
</>
)}
{!saved && (
{!saved && !conflict && (
<small>
{!receiptReady
? 'Checking whether this draft was already saved.'
Expand Down
31 changes: 26 additions & 5 deletions src/client/page-review-decision.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,47 @@
import { pageReviewSchema } from '../shared/page-review';
import type { Page } from '../server/pages';
import type { ReviewedPage } from '../server/pages';
import { api } from './api';

const reviewPath = (threadId: string) =>
`/conversations/${encodeURIComponent(threadId)}/reviewed-page`;

export function restorePageReview(threadId: string, toolCallId: string) {
return api<Page | null>(
return api<ReviewedPage | null>(
`${reviewPath(threadId)}/${encodeURIComponent(toolCallId)}`,
);
}

export function matchesReviewedDraft(page: ReviewedPage, args: unknown) {
// Receipts created before draft binding have no original snapshot.
if (!page.reviewDraft) return true;
const draft = pageReviewSchema.safeParse(args);
return (
draft.success &&
draft.data.title === page.reviewDraft.title &&
draft.data.content === page.reviewDraft.content &&
draft.data.spaceId === page.reviewDraft.spaceId
);
}

export async function decidePageReview(
threadId: string,
toolCallId: string,
args: unknown,
approved: boolean,
): Promise<Page | null> {
): Promise<ReviewedPage | null> {
// A previous save may have committed even if its response never arrived.
const previous = await restorePageReview(threadId, toolCallId);
if (previous) return previous;
if (previous) {
if (!matchesReviewedDraft(previous, args))
throw new Error(
'This review was saved with a different draft. Start a new review for the changed draft.',
);
return previous;
}
if (!approved) return null;
const draft = pageReviewSchema.parse(args);
return api<Page>(reviewPath(threadId), 'POST', { ...draft, toolCallId });
return api<ReviewedPage>(reviewPath(threadId), 'POST', {
...draft,
toolCallId,
});
}
7 changes: 4 additions & 3 deletions src/server/page-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ export function pageRoutes(platform: Platform) {
{ error: 'This Dot no longer has access to the selected Space.' },
403,
);
return c.json(
platform.workspace.pages.get(receipt.spaceId, receipt.pageId),
);
return c.json({
...platform.workspace.pages.get(receipt.spaceId, receipt.pageId),
reviewDraft: receipt.draft,
});
});
app.post('/conversations/:id/reviewed-page', async (c) => {
const data = pageReviewSchema
Expand Down
66 changes: 49 additions & 17 deletions src/server/pages.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { randomUUID } from 'node:crypto';
import type { DatabaseSync } from 'node:sqlite';
import { z } from 'zod';
import {
pageReviewSchema,
type PageReviewDraft,
} from '../shared/page-review.js';
export const pageInput = z
.object({
title: z.string().trim().min(1).max(160),
Expand All @@ -27,6 +31,9 @@ export interface Page {
updatedAt: number;
sourceThreadId: string | null;
}
export type ReviewedPage = Page & {
reviewDraft: PageReviewDraft | null;
};
export class PageError extends Error {
constructor(
message: string,
Expand All @@ -41,8 +48,15 @@ export class Pages {
private spaceExists: (id: string) => boolean,
) {
db.exec(
'CREATE TABLE IF NOT EXISTS page_reviews(threadId TEXT NOT NULL, toolCallId TEXT NOT NULL, pageId TEXT NOT NULL, spaceId TEXT NOT NULL, PRIMARY KEY(threadId,toolCallId))',
'CREATE TABLE IF NOT EXISTS page_reviews(threadId TEXT NOT NULL, toolCallId TEXT NOT NULL, pageId TEXT NOT NULL, spaceId TEXT NOT NULL, draft TEXT, PRIMARY KEY(threadId,toolCallId))',
);
if (
!db
.prepare('PRAGMA table_info(page_reviews)')
.all()
.some((row) => row.name === 'draft')
)
db.exec('ALTER TABLE page_reviews ADD COLUMN draft TEXT');
db.exec(`CREATE TABLE IF NOT EXISTS pages(id TEXT PRIMARY KEY, spaceId TEXT NOT NULL, parentId TEXT, title TEXT NOT NULL, content TEXT NOT NULL, revision INTEGER NOT NULL, createdAt INTEGER NOT NULL, updatedAt INTEGER NOT NULL, sourceThreadId TEXT);
CREATE TABLE IF NOT EXISTS page_threads(pageId TEXT NOT NULL,dotId TEXT NOT NULL,threadId TEXT NOT NULL UNIQUE,ready INTEGER NOT NULL DEFAULT 0, leaseUntil INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(pageId,dotId));`);
if (
Expand Down Expand Up @@ -117,45 +131,63 @@ export class Pages {
reviewReceipt(
threadId: string,
toolCallId: string,
): { pageId: string; spaceId: string } | null {
): { pageId: string; spaceId: string; draft: PageReviewDraft | null } | null {
const row = this.db
.prepare(
'SELECT pageId,spaceId FROM page_reviews WHERE threadId=? AND toolCallId=?',
'SELECT pageId,spaceId,draft FROM page_reviews WHERE threadId=? AND toolCallId=?',
)
.get(threadId, toolCallId);
return row
? { pageId: String(row.pageId), spaceId: String(row.spaceId) }
? {
pageId: String(row.pageId),
spaceId: String(row.spaceId),
draft: row.draft
? pageReviewSchema.parse(JSON.parse(String(row.draft)))
: null,
}
: null;
}
createReviewed(
spaceId: string,
input: z.input<typeof pageInput>,
input: Pick<PageReviewDraft, 'title' | 'content'>,
threadId: string,
toolCallId: string,
): Page {
): ReviewedPage {
const draft = pageReviewSchema.parse({ ...input, spaceId });
this.db.exec('BEGIN IMMEDIATE');
try {
const previous = this.db
.prepare(
'SELECT pageId,spaceId FROM page_reviews WHERE threadId=? AND toolCallId=?',
)
.get(threadId, toolCallId);
const previous = this.reviewReceipt(threadId, toolCallId);
if (previous) {
if (previous.spaceId !== spaceId)
throw new PageError(
'This review was already saved to another Space.',
409,
);
const page = this.get(spaceId, String(previous.pageId));
if (
previous.draft &&
(previous.draft.title !== draft.title ||
previous.draft.content !== draft.content)
)
throw new PageError(
'This review was already saved with a different draft. Start a new review for the changed draft.',
409,
);
const page = this.get(spaceId, previous.pageId);
this.db.exec('COMMIT');
return page;
return { ...page, reviewDraft: previous.draft };
}
const page = this.create(spaceId, input, threadId);
const page = this.create(
spaceId,
{ title: draft.title, content: draft.content },
threadId,
);
this.db
.prepare('INSERT INTO page_reviews VALUES (?,?,?,?)')
.run(threadId, toolCallId, page.id, spaceId);
.prepare(
'INSERT INTO page_reviews (threadId,toolCallId,pageId,spaceId,draft) VALUES (?,?,?,?,?)',
)
.run(threadId, toolCallId, page.id, spaceId, JSON.stringify(draft));
this.db.exec('COMMIT');
return page;
return { ...page, reviewDraft: draft };
} catch (error) {
this.db.exec('ROLLBACK');
throw error;
Expand Down
1 change: 1 addition & 0 deletions src/shared/page-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const pageReviewSchema = z
spaceId: z.string().min(1),
})
.strict();
export type PageReviewDraft = z.infer<typeof pageReviewSchema>;
export const pageReviewTool = {
name: 'review_space_page',
description:
Expand Down
32 changes: 31 additions & 1 deletion tests/page-review.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ it('does not decide or save when receipt recovery fails', async () => {

it('checks for a receipt before retrying a save whose response was lost', async () => {
const draft = { title: 'Brief', content: 'Evidence', spaceId: 'space' };
const page = { id: 'saved', ...draft };
const page = { id: 'saved', ...draft, reviewDraft: draft };
vi.mocked(api)
.mockResolvedValueOnce(null)
.mockRejectedValueOnce(new Error('Connection lost'));
Expand All @@ -49,6 +49,36 @@ it('checks for a receipt before retrying a save whose response was lost', async
).toHaveLength(1);
});

it('rejects a changed restored draft before sending a decision or another save', async () => {
const original = {
title: 'Brief',
content: 'Approved text',
spaceId: 'space',
};
const page = {
id: 'saved',
...original,
content: 'The page was edited later.',
reviewDraft: original,
};
vi.mocked(api).mockResolvedValue(page);
expect(
await decidePageReview('thread', 'call', original, true),
).toMatchObject({ id: 'saved' });
for (const changed of [
{ ...original, title: 'Changed' },
{ ...original, content: 'Changed' },
{ ...original, spaceId: 'other' },
]) {
await expect(
decidePageReview('thread', 'call', changed, true),
).rejects.toThrow('different draft');
}
expect(
vi.mocked(api).mock.calls.filter((call) => call[1] === 'POST'),
).toHaveLength(0);
});

it('hides decisions and unsaved claims until the persisted receipt is checked', () => {
const html = renderToStaticMarkup(
<PageReviewCard
Expand Down
Loading
Loading