From e9be544503c14a74a5e25f5e3b8d5b3fa850944e Mon Sep 17 00:00:00 2001 From: "Juan Carlos Moreno [ackstorm]" Date: Sat, 10 Oct 2026 06:57:53 +0200 Subject: [PATCH 1/2] feat(review): add artifact defaults and link mode overrides --- README.md | 2 + apps/web/src/api/client.ts | 5 + apps/web/src/lib/presentation.ts | 2 + apps/web/src/review/review-app.tsx | 60 +++- .../cloudflare/src/d1-artifact-repository.ts | 76 +++++- deploy/cloudflare/src/d1-migrations.ts | 58 +++- .../tests/agent-dispatch-runtime.test.ts | 2 +- .../cloudflare/tests/comment-runtime.test.ts | 2 +- .../cloudflare/tests/worker-runtime.test.ts | 95 ++++++- docs/cli.md | 3 + docs/mcp.md | 4 +- docs/review-mode.md | 51 ++++ .../artifact-review-mode-browser.json | 1 + project/evidence/artifact-review-mode.json | 1 + project/spec/artifact-review-mode-spec.md | 42 +++ project/spec/conformance.yml | 38 +++ .../references/artifact-operations.md | 3 +- src/application/artifact-management.ts | 51 ++++ src/application/publish-artifact.ts | 13 +- src/application/staged-upload.ts | 3 + src/cli/publish-command.ts | 14 +- src/client/file-publication-client.ts | 19 +- src/core/errors.ts | 1 + src/core/model.ts | 9 + src/core/ports.ts | 16 ++ src/http/create-http-app.ts | 29 ++ src/local/create-local-application-layer.ts | 20 ++ src/mcp/artifact-mcp-server.ts | 37 +++ src/storage/postgres-artifact-repository.ts | 75 +++-- src/storage/postgres-migrations.ts | 26 +- src/storage/public-link-inventory-row.ts | 2 + src/storage/sqlite-artifact-repository.ts | 158 +++++++++-- src/storage/sqlite-schema.ts | 2 +- tests/browser/artifact-review-mode.spec.ts | 256 ++++++++++++++++++ tests/cli/auth-profile-remote-publish.test.ts | 13 + tests/cli/mcp-onboarding.test.ts | 8 +- .../artifact-review-mode-publication.test.ts | 41 +++ .../artifact-review-mode-persistence.test.ts | 139 ++++++++++ .../conformance/artifact-review-mode.test.ts | 122 +++++++++ .../conformance/authorization-policy.test.ts | 1 + tests/conformance/mcp-modern-http.test.ts | 22 ++ .../external-storage-runtime.test.ts | 97 +++++++ tests/support/publishing.ts | 16 +- 43 files changed, 1549 insertions(+), 86 deletions(-) create mode 100644 docs/review-mode.md create mode 100644 project/evidence/artifact-review-mode-browser.json create mode 100644 project/evidence/artifact-review-mode.json create mode 100644 project/spec/artifact-review-mode-spec.md create mode 100644 tests/browser/artifact-review-mode.spec.ts create mode 100644 tests/client/artifact-review-mode-publication.test.ts create mode 100644 tests/conformance/artifact-review-mode-persistence.test.ts create mode 100644 tests/conformance/artifact-review-mode.test.ts diff --git a/README.md b/README.md index a1210379..f78bd3cd 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,8 @@ Open the printed URL. You can also download the portable Node.js package from th Most artifacts are published by an agent through the [Artifact Server Skill](./skills/artifact-server/SKILL.md) or [MCP](./docs/mcp.md). For direct access, use the `artifactserver` CLI to publish files and directories, connect to remote servers, and create new versions. [Read the CLI guide](./docs/cli.md). +HTML prototypes can open in Interact mode so people can use them immediately. Set `--review-mode interact` when creating an artifact, or share a Review link with `mode=interact`. Annotate remains the default, and reviewers can switch modes at any time. [Read about Review mode](./docs/review-mode.md). + ## Security model Artifact Server serves untrusted artifact content from isolated version hosts, separate from the trusted application origin used for authentication, review, comments, API, and MCP. Private content uses version-scoped browser sessions, and untrusted paths cannot select storage locations. diff --git a/apps/web/src/api/client.ts b/apps/web/src/api/client.ts index 55c43141..c48d09b0 100644 --- a/apps/web/src/api/client.ts +++ b/apps/web/src/api/client.ts @@ -1,6 +1,7 @@ import { z } from "zod"; const accessSettingSchema = z.enum(["account_required", "public_link"]); +const reviewModeSchema = z.enum(["annotate", "interact"]); const membershipRoleSchema = z.enum(["administrator", "member"]); const principalKindSchema = z.enum(["human", "service"]); const capabilitySchema = z.enum([ @@ -135,6 +136,7 @@ const artifactSchema = z.object({ id: z.string(), name: z.string(), projectId: z.string(), + reviewMode: reviewModeSchema.default("annotate"), tags: z.array(z.string()), }); @@ -189,6 +191,7 @@ const apiErrorBodySchema = z.object({ const noContentStatuses = new Set([204, 205]); export type AccessSetting = z.infer; +export type ReviewMode = z.infer; export type DeploymentCapabilities = z.infer; export type SourceFreshness = z.infer; export type SourceBinding = z.infer; @@ -295,6 +298,7 @@ export interface ArtifactAction { readonly action: | "capture" | "change_access" + | "change_review_mode" | "change_tags" | "comment_create" | "comment_delete" @@ -394,6 +398,7 @@ const actionSchema: z.ZodType = z.object({ action: z.enum([ "capture", "change_access", + "change_review_mode", "change_tags", "comment_create", "comment_delete", diff --git a/apps/web/src/lib/presentation.ts b/apps/web/src/lib/presentation.ts index a60b931c..b734ab0b 100644 --- a/apps/web/src/lib/presentation.ts +++ b/apps/web/src/lib/presentation.ts @@ -81,6 +81,8 @@ export function actionLabel(value: ArtifactAction["action"]): string { return "Relinked source file"; case "change_access": return "Changed access"; + case "change_review_mode": + return "Changed review mode"; case "change_tags": return "Replaced tags"; case "comment_create": diff --git a/apps/web/src/review/review-app.tsx b/apps/web/src/review/review-app.tsx index 18e2f104..d1a0607d 100644 --- a/apps/web/src/review/review-app.tsx +++ b/apps/web/src/review/review-app.tsx @@ -51,6 +51,7 @@ import { type ArtifactPage, type ArtifactVersion, type Project, + type ReviewMode, type Session, type Version, } from "@/api/client"; @@ -142,6 +143,7 @@ interface ReviewDownload { interface ReviewLocation { readonly artifactId: string | null; + readonly mode: ReviewMode | null; readonly path: string | null; readonly projectId: string | null; readonly versionId: string | null; @@ -533,6 +535,8 @@ function ArtifactReview({ const [selectedPath, setSelectedPath] = useState( initialLocation.path, ); + const [modeOverride, setModeOverride] = useState(initialLocation.mode); + const [reviewModeRestoration, setReviewModeRestoration] = useState(0); const [query, setQuery] = useState(""); const searchQuery = useDeferredValue(query.trim()); const [catalogCommentFilter, setCatalogCommentFilter] = useState("all"); @@ -565,7 +569,37 @@ function ArtifactReview({ const [focusControlsCollapsed, setFocusControlsCollapsed] = useState(false); const [focusControlsHoverArmed, setFocusControlsHoverArmed] = useState(true); const [focusControlsInstant, setFocusControlsInstant] = useState(false); - const [htmlAnnotateModeActive, setHtmlAnnotateModeActive] = useState(true); + const [htmlAnnotateModeActive, setHtmlAnnotateModeActive] = useState( + initialLocation.mode !== "interact", + ); + // History restores the URL's mode; refreshing details preserves manual choices. + const reviewModeContext = `${projectId}:${selectedArtifactId}:${modeOverride}:${reviewModeRestoration}`; + const reviewModeInitializationRef = useRef({ + context: reviewModeContext, + initialized: initialLocation.mode !== null, + }); + const changeHtmlAnnotateMode = useCallback(( + active: boolean | ((current: boolean) => boolean), + ): void => { + reviewModeInitializationRef.current.initialized = true; + setHtmlAnnotateModeActive(active); + }, []); + useEffect(() => { + const initialization = reviewModeInitializationRef.current; + if (initialization.context !== reviewModeContext) { + initialization.context = reviewModeContext; + initialization.initialized = modeOverride !== null; + setHtmlAnnotateModeActive(modeOverride !== "interact"); + } + if ( + !initialization.initialized + && details?.artifact.id === selectedArtifactId + && details.artifact.projectId === projectId + ) { + initialization.initialized = true; + setHtmlAnnotateModeActive(details.artifact.reviewMode !== "interact"); + } + }, [details, modeOverride, projectId, reviewModeContext, selectedArtifactId]); const [homeOpen, setHomeOpen] = useState(false); const focusCommentsButtonRef = useRef(null); const focusControlsRestoreRef = useRef(null); @@ -888,6 +922,7 @@ function ArtifactReview({ useEffect(() => { const href = reviewHref({ artifactId: selectedArtifactId, + mode: modeOverride, path: selectedPath, projectId, versionId: selectedVersionId, @@ -895,7 +930,7 @@ function ArtifactReview({ }); window.history.replaceState(null, "", href); window.sessionStorage.setItem("artifact-review-return-url", href); - }, [focusMode, projectId, selectedArtifactId, selectedPath, selectedVersionId]); + }, [focusMode, modeOverride, projectId, selectedArtifactId, selectedPath, selectedVersionId]); useEffect(() => { const restoreLocation = (): void => { @@ -906,6 +941,8 @@ function ArtifactReview({ setProjectId(restored.projectId); } setSelectedArtifactId(restored.artifactId); + setModeOverride(restored.mode); + setReviewModeRestoration((restoration) => restoration + 1); setSelectedVersionId(restored.versionId); setSelectedPath(restored.path); setFocusMode(restored.view === "focus"); @@ -990,6 +1027,7 @@ function ArtifactReview({ const selectManifestPath = (path: string): void => { window.history.pushState(null, "", reviewHref({ artifactId: selectedArtifactId, + mode: modeOverride, path, projectId, versionId: selectedVersionId, @@ -1148,6 +1186,7 @@ function ArtifactReview({ const enterFocusMode = useCallback((): void => { window.history.pushState(null, "", reviewHref({ artifactId: selectedArtifactId, + mode: modeOverride, path: selectedPath, projectId, versionId: selectedVersionId, @@ -1156,10 +1195,11 @@ function ArtifactReview({ setFocusCommentsOpen(false); setFocusControlsCollapsed(false); setFocusMode(true); - }, [projectId, selectedArtifactId, selectedPath, selectedVersionId]); + }, [modeOverride, projectId, selectedArtifactId, selectedPath, selectedVersionId]); const exitFocusMode = useCallback((): void => { window.history.pushState(null, "", reviewHref({ artifactId: selectedArtifactId, + mode: modeOverride, path: selectedPath, projectId, versionId: selectedVersionId, @@ -1168,7 +1208,7 @@ function ArtifactReview({ setFocusCommentsOpen(false); setFocusControlsCollapsed(false); setFocusMode(false); - }, [projectId, selectedArtifactId, selectedPath, selectedVersionId]); + }, [modeOverride, projectId, selectedArtifactId, selectedPath, selectedVersionId]); useEffect(() => { const handleReviewShortcut = (event: KeyboardEvent): void => { if (homeOpen || reviewShortcutBlocked(event)) return; @@ -1180,7 +1220,7 @@ function ArtifactReview({ setFocusCommentsOpen(false); } else if (htmlAnnotateModeActive) { event.preventDefault(); - setHtmlAnnotateModeActive(false); + changeHtmlAnnotateMode(false); } else if (focusMode) { event.preventDefault(); exitFocusMode(); @@ -1226,6 +1266,7 @@ function ArtifactReview({ return () => window.removeEventListener("keydown", handleReviewShortcut); }, [ catalogItems, + changeHtmlAnnotateMode, enterFocusMode, exitFocusMode, focusCommentsOpen, @@ -1635,7 +1676,7 @@ function ArtifactReview({ aria-pressed={htmlAnnotateModeActive} className="as-button as-focus-controls__button" data-active={htmlAnnotateModeActive} - onClick={() => setHtmlAnnotateModeActive((active) => !active)} + onClick={() => changeHtmlAnnotateMode((active) => !active)} title={htmlAnnotateModeActive ? "Annotate mode: click an element or select text to comment. Press Escape to interact." : "Interact mode: links and controls work normally. Select text or turn annotation mode back on to comment."} @@ -1763,7 +1804,7 @@ function ArtifactReview({ label={htmlAnnotateModeActive ? "Annotate mode: click an element or select text to comment. Press Escape to interact." : "Interact mode: links and controls work normally. Select text or turn annotation mode back on to comment."} - onClick={() => setHtmlAnnotateModeActive((active) => !active)} + onClick={() => changeHtmlAnnotateMode((active) => !active)} > @@ -1838,7 +1879,7 @@ function ArtifactReview({ artifactName={details?.artifact.name ?? selectedItem?.artifact.name ?? "Artifact"} isLight={theme === "dawn"} onOpenRawArtifact={() => void openRawArtifact()} - onAnnotateModeChange={setHtmlAnnotateModeActive} + onAnnotateModeChange={changeHtmlAnnotateMode} onSelectAnnotation={(threadId) => { comments.selectThread(threadId); if (threadId !== null) { @@ -2753,8 +2794,10 @@ function readInitialInspectorOpen(): boolean { function readReviewLocation(): ReviewLocation { const search = new URLSearchParams(window.location.search); + const mode = search.get("mode"); return { artifactId: search.get("artifact"), + mode: mode === "annotate" || mode === "interact" ? mode : null, path: search.get("path"), projectId: search.get("project"), versionId: search.get("version"), @@ -2768,6 +2811,7 @@ function reviewHref(location: ReviewLocation): string { search.set("project", location.projectId); } if (location.artifactId !== null) search.set("artifact", location.artifactId); + if (location.mode !== null) search.set("mode", location.mode); if (location.versionId !== null) search.set("version", location.versionId); if (location.path !== null) search.set("path", location.path); if (location.view !== null) search.set("view", location.view); diff --git a/deploy/cloudflare/src/d1-artifact-repository.ts b/deploy/cloudflare/src/d1-artifact-repository.ts index ac696d0f..9b02d312 100644 --- a/deploy/cloudflare/src/d1-artifact-repository.ts +++ b/deploy/cloudflare/src/d1-artifact-repository.ts @@ -34,6 +34,7 @@ import {normalizeArtifactSearchText} from "../../../src/application/artifact-tags.js"; import { accessSettings, + reviewModes, agentBeaconStates, agentDispatchStates, artifactActionKinds, @@ -80,6 +81,7 @@ import type { ArtifactRepository, CancelAgentDispatch, ChangeArtifactAccessSetting, + ChangeArtifactReviewMode, ChangeArtifactTags, ClearCommentThreads, CommentRepository, @@ -141,6 +143,7 @@ const accessSettingSchema = z.enum([ accessSettings.accountRequired, accessSettings.publicLink, ]); +const reviewModeSchema = z.enum([reviewModes.annotate, reviewModes.interact]); const dispositionSchema = z.enum([ fileDispositions.attachment, fileDispositions.inline, @@ -153,6 +156,7 @@ const uploadStatusSchema = z.enum([ ]); const actionSchema = z.enum([ artifactActionKinds.changeAccess, + artifactActionKinds.changeReviewMode, artifactActionKinds.changeTags, artifactActionKinds.commentCreate, artifactActionKinds.commentDelete, @@ -272,6 +276,7 @@ const gitHistoryMappingRowSchema = z.object({ versionId: z.string(), }); const artifactRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema, createdAt: z.string(), currentVersionId: z.string(), @@ -305,6 +310,7 @@ const entryRowSchema = z.object({ size: z.number().int().nonnegative(), }); const idempotencyRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema.nullable(), artifactId: z.string(), inputDigest: z.string(), @@ -510,7 +516,7 @@ const versionContentRowSchema = z.object({ const artifactSelect = ` SELECT id, project_id AS projectId, name, - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, current_version_id AS currentVersionId, created_at AS createdAt, deleted_at AS deletedAt FROM artifacts @@ -581,7 +587,7 @@ const commentReplyScope = `FROM comment_threads t JOIN comment_replies r ON r.thread_id = t.id AND r.id = ? WHERE t.id = ? AND t.project_id = ? AND t.artifact_id = ?`; const idempotencySelect = ` - SELECT access_setting AS accessSetting, artifact_id AS artifactId, + SELECT review_mode AS reviewMode, access_setting AS accessSetting, artifact_id AS artifactId, input_digest AS inputDigest, operation, tags_json AS tagsJson, version_id AS versionId FROM idempotency_records @@ -764,7 +770,8 @@ export function createD1ArtifactRepository( message: "The idempotency key was already used with different input.", }); } - return readPublishedVersion(projectId, record.versionId, true); + const published = await readPublishedVersion(projectId, record.versionId, true); + return {...published, artifact: {...published.artifact, reviewMode: record.reviewMode}}; }; const readStagedUploadOrNull = async ( projectId: string, @@ -855,6 +862,7 @@ export function createD1ArtifactRepository( replayed: boolean, accessSetting?: ArtifactRecord["accessSetting"], tags?: readonly string[], + reviewMode?: ArtifactRecord["reviewMode"], ): Promise => { const artifact = await readArtifact(projectId, artifactId); const version = await readVersionOrNull(projectId, versionId, artifactId); @@ -865,6 +873,7 @@ export function createD1ArtifactRepository( accessSetting: accessSetting ?? artifact.accessSetting, currentVersionId: versionId, tags: tags ?? artifact.tags, + reviewMode: reviewMode ?? artifact.reviewMode, }, replayed, version, @@ -935,8 +944,8 @@ export function createD1ArtifactRepository( ) => database.prepare(` INSERT INTO idempotency_records ( project_id, idempotency_key, input_digest, artifact_id, version_id, - operation, access_setting, tags_json, created_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + operation, access_setting, tags_json, created_at, review_mode + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, (SELECT review_mode FROM artifacts WHERE id = ?)) `).bind( command.projectId, command.idempotencyKey, @@ -947,6 +956,7 @@ export function createD1ArtifactRepository( accessSetting, tagsJson, command.createdAt, + command.artifactId, ); const mutationGuardStatements = ( id: string, @@ -1085,7 +1095,7 @@ export function createD1ArtifactRepository( ); const managementReplay = async ( command: {readonly idempotencyKey: string; readonly inputDigest: string; readonly projectId: string}, - operation: typeof artifactActionKinds.changeAccess | typeof artifactActionKinds.changeTags | + operation: typeof artifactActionKinds.changeReviewMode | typeof artifactActionKinds.changeAccess | typeof artifactActionKinds.changeTags | typeof artifactActionKinds.restore, ): Promise => { const record = await readIdempotency(command.projectId, command.idempotencyKey); @@ -1103,6 +1113,7 @@ export function createD1ArtifactRepository( true, record.accessSetting ?? undefined, tags, + record.reviewMode, ); }; const applyManagementMutation = async ( @@ -1111,7 +1122,7 @@ export function createD1ArtifactRepository( readonly inputDigest: string; readonly projectId: string; }, - operation: typeof artifactActionKinds.changeAccess | + operation: typeof artifactActionKinds.changeReviewMode | typeof artifactActionKinds.changeAccess | typeof artifactActionKinds.changeTags | typeof artifactActionKinds.restore, statements: D1PreparedStatement[], @@ -2206,15 +2217,16 @@ export function createD1ArtifactRepository( await database.batch([ database.prepare(` INSERT INTO artifacts ( - id, project_id, name, search_name, access_setting, + id, project_id, name, search_name, access_setting, review_mode, current_version_id, created_at, deleted_at - ) VALUES (?, ?, ?, ?, ?, NULL, ?, NULL) + ) VALUES (?, ?, ?, ?, ?, ?, NULL, ?, NULL) `).bind( command.artifactId, command.projectId, command.name, normalizeArtifactSearchText(command.name), command.accessSetting, + command.reviewMode ?? reviewModes.annotate, command.createdAt, ), ...versionStatements(command, 1), @@ -2446,7 +2458,7 @@ export function createD1ArtifactRepository( const result = await database.prepare(` SELECT * FROM ( SELECT id, project_id AS projectId, name, - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, current_version_id AS currentVersionId, created_at AS createdAt, deleted_at AS deletedAt, ( @@ -2506,7 +2518,7 @@ export function createD1ArtifactRepository( artifact.id AS artifactId, artifact.project_id AS projectId, artifact.name AS artifactName, - artifact.access_setting AS accessSetting, + artifact.review_mode AS reviewMode, artifact.access_setting AS accessSetting, artifact.current_version_id AS currentVersionId, artifact.created_at AS artifactCreatedAt, artifact.deleted_at AS artifactDeletedAt, @@ -2546,6 +2558,7 @@ export function createD1ArtifactRepository( ); const artifacts = await Promise.all(parsedRows.map(async (row) => ({ accessSetting: row.accessSetting, + reviewMode: row.reviewMode, createdAt: row.artifactCreatedAt, currentVersionId: row.currentVersionId, deletedAt: row.artifactDeletedAt, @@ -2579,7 +2592,7 @@ export function createD1ArtifactRepository( }, findVersionContent: async (contentToken, requestedPath, fallback) => { const row = await database.prepare(` - SELECT a.access_setting AS accessSetting, a.id AS artifactId, + SELECT a.review_mode AS reviewMode, a.access_setting AS accessSetting, a.id AS artifactId, a.project_id AS projectId, v.content_token AS contentToken, v.id AS versionId, e.path, e.size, e.media_type AS mediaType, e.sha256, e.disposition, @@ -2664,6 +2677,45 @@ export function createD1ArtifactRepository( command.accessSetting, )); }, + changeReviewMode: async (command: ChangeArtifactReviewMode) => { + const replay = await managementReplay(command, artifactActionKinds.changeReviewMode); + if (replay !== null) return replay; + const artifact = await readArtifact(command.projectId, command.artifactId); + assertCurrent(artifact, command.expectedCurrentVersionId); + return applyManagementMutation(command, artifactActionKinds.changeReviewMode, [ + database.prepare(` + UPDATE artifacts SET review_mode = ? + WHERE project_id = ? AND id = ? AND current_version_id = ? AND deleted_at IS NULL + `).bind( + command.reviewMode, + command.projectId, + command.artifactId, + command.expectedCurrentVersionId, + ), + actionStatement(command, command.expectedCurrentVersionId, artifactActionKinds.changeReviewMode), + idempotencyStatement( + command, + command.expectedCurrentVersionId, + artifactActionKinds.changeReviewMode, + artifact.accessSetting, + ), + ...managementGuardStatements( + command, + artifactActionKinds.changeReviewMode, + command.expectedCurrentVersionId, + "a.deleted_at IS NULL AND a.review_mode = ?", + [command.reviewMode], + ), + ], () => artifactState( + command.projectId, + command.artifactId, + command.expectedCurrentVersionId, + false, + artifact.accessSetting, + undefined, + command.reviewMode, + )); + }, changeTags: async (command: ChangeArtifactTags) => { const replay = await managementReplay(command, artifactActionKinds.changeTags); if (replay !== null) return replay; diff --git a/deploy/cloudflare/src/d1-migrations.ts b/deploy/cloudflare/src/d1-migrations.ts index a1c2cce1..e0cd9b04 100644 --- a/deploy/cloudflare/src/d1-migrations.ts +++ b/deploy/cloudflare/src/d1-migrations.ts @@ -8,13 +8,14 @@ import {defaultGitHistoryMaximumCopiedFiles} from "../../../src/git-history/git-history-capability.js"; /** D1 schema revision required by the Cloudflare runtime. */ -export const requiredD1SchemaVersion = 9; +export const requiredD1SchemaVersion = 10; /** SQL literal list of every action kind the ledger accepts. */ const actionKindList = [ "'publish'", "'restore'", "'change_access'", + "'change_review_mode'", "'change_tags'", "'delete'", "'comment_create'", @@ -45,6 +46,7 @@ const schemaSql = ` name TEXT NOT NULL, search_name TEXT NOT NULL, access_setting TEXT NOT NULL CHECK (access_setting IN ('account_required', 'public_link')), + review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact')), current_version_id TEXT, created_at TEXT NOT NULL, deleted_at TEXT @@ -86,9 +88,10 @@ const schemaSql = ` input_digest TEXT NOT NULL, artifact_id TEXT NOT NULL REFERENCES artifacts(id), version_id TEXT NOT NULL REFERENCES versions(id), - operation TEXT NOT NULL CHECK (operation IN ('publish', 'restore', 'change_access', 'change_tags', 'delete')), + operation TEXT NOT NULL CHECK (operation IN ('publish', 'restore', 'change_access', 'change_review_mode', 'change_tags', 'delete')), access_setting TEXT CHECK (access_setting IS NULL OR access_setting IN ('account_required', 'public_link')), tags_json TEXT, + review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact')), created_at TEXT NOT NULL, PRIMARY KEY (project_id, idempotency_key) ); @@ -617,6 +620,7 @@ export async function migrateD1( await addGitHistoryMirrorColumnsIfMissing(database); await widenRegisteredAgentsIfNeeded(database); await addArtifactSearchNameIfMissing(database); + await addArtifactReviewModeIfMissing(database); } await database.batch([ database.prepare(` @@ -779,3 +783,53 @@ async function widenRegisteredAgentsIfNeeded( database.prepare(statement)), ); } + +/** Upgrade saved artifact defaults and retry snapshots in one atomic batch. */ +async function addArtifactReviewModeIfMissing(database: D1Database): Promise { + const artifactColumns = await database.prepare("PRAGMA table_info(artifacts)").all<{name: string}>(); + const idempotencyColumns = await database.prepare("PRAGMA table_info(idempotency_records)").all<{name: string}>(); + const hasArtifactMode = artifactColumns.results.some((column) => column.name === "review_mode"); + const hasSavedMode = idempotencyColumns.results.some((column) => column.name === "review_mode"); + if (hasArtifactMode && hasSavedMode) return; + const savedMode = hasSavedMode ? "review_mode" : "'annotate'"; + const statements = [ + ...(hasArtifactMode ? [] : ["ALTER TABLE artifacts ADD COLUMN review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact'))"]), + `CREATE TABLE idempotency_records_review_mode ( + project_id TEXT NOT NULL REFERENCES projects(id), + idempotency_key TEXT NOT NULL, + input_digest TEXT NOT NULL, + artifact_id TEXT NOT NULL REFERENCES artifacts(id), + version_id TEXT NOT NULL REFERENCES versions(id), + operation TEXT NOT NULL CHECK (operation IN ('publish', 'restore', 'change_access', 'change_review_mode', 'change_tags', 'delete')), + access_setting TEXT CHECK (access_setting IS NULL OR access_setting IN ('account_required', 'public_link')), + tags_json TEXT, + review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact')), + created_at TEXT NOT NULL, + PRIMARY KEY (project_id, idempotency_key) + )`, + `INSERT INTO idempotency_records_review_mode + SELECT project_id, idempotency_key, input_digest, artifact_id, version_id, + operation, access_setting, tags_json, ${savedMode}, created_at + FROM idempotency_records`, + "DROP TABLE idempotency_records", + "ALTER TABLE idempotency_records_review_mode RENAME TO idempotency_records", + `CREATE TABLE actions_review_mode ( + id TEXT PRIMARY KEY, + project_id TEXT NOT NULL REFERENCES projects(id), + artifact_id TEXT NOT NULL REFERENCES artifacts(id), + version_id TEXT NOT NULL REFERENCES versions(id), + action TEXT NOT NULL CHECK (action IN (${actionKindList})), + principal_id TEXT NOT NULL, + authorized_by_principal_id TEXT, + idempotency_key TEXT NOT NULL, + created_at TEXT NOT NULL, + UNIQUE (project_id, idempotency_key) + )`, + `INSERT INTO actions_review_mode SELECT id, project_id, artifact_id, version_id, + action, principal_id, authorized_by_principal_id, idempotency_key, created_at FROM actions`, + "DROP TABLE actions", + "ALTER TABLE actions_review_mode RENAME TO actions", + "CREATE INDEX actions_artifact_created ON actions(project_id, artifact_id, created_at DESC, id DESC)", + ]; + await database.batch(statements.map((statement) => database.prepare(statement))); +} diff --git a/deploy/cloudflare/tests/agent-dispatch-runtime.test.ts b/deploy/cloudflare/tests/agent-dispatch-runtime.test.ts index b73fe80f..fe5d3d59 100644 --- a/deploy/cloudflare/tests/agent-dispatch-runtime.test.ts +++ b/deploy/cloudflare/tests/agent-dispatch-runtime.test.ts @@ -502,7 +502,7 @@ describe("Cloudflare D1 agent dispatch", () => { try { expect(schemaVersionRowSchema.parse(upgraded.prepare( "SELECT version FROM artifact_server_schema WHERE component = 'runtime'", - ).get()).version).toBe(9); + ).get()).version).toBe(10); expect(z.array(tableColumnRowSchema) .parse(upgraded.prepare("PRAGMA table_info(comment_threads)").all()) .map(({name}) => name)).toContain("dispatch_id"); diff --git a/deploy/cloudflare/tests/comment-runtime.test.ts b/deploy/cloudflare/tests/comment-runtime.test.ts index 84c7926c..17f3f601 100644 --- a/deploy/cloudflare/tests/comment-runtime.test.ts +++ b/deploy/cloudflare/tests/comment-runtime.test.ts @@ -497,7 +497,7 @@ describe("Cloudflare D1 comments", () => { try { expect(schemaVersionRowSchema.parse(upgraded.prepare( "SELECT version FROM artifact_server_schema WHERE component = 'runtime'", - ).get()).version).toBe(9); + ).get()).version).toBe(10); const columns = z.array(tableColumnRowSchema) .parse(upgraded.prepare("PRAGMA table_info(login_attempts)").all()) .map(({name}) => name); diff --git a/deploy/cloudflare/tests/worker-runtime.test.ts b/deploy/cloudflare/tests/worker-runtime.test.ts index 095a01fd..9d263036 100644 --- a/deploy/cloudflare/tests/worker-runtime.test.ts +++ b/deploy/cloudflare/tests/worker-runtime.test.ts @@ -1,7 +1,8 @@ import {createHash} from "node:crypto"; -import {mkdtemp, rm} from "node:fs/promises"; +import {mkdtemp, readdir, rm} from "node:fs/promises"; import {tmpdir} from "node:os"; import {join} from "node:path"; +import {DatabaseSync} from "node:sqlite"; import {z} from "zod"; import {unstable_dev, type Unstable_DevWorker} from "wrangler"; @@ -18,7 +19,7 @@ const uploadPlanSchema = z.object({ })).length(1), }); const publicationSchema = z.object({ - artifact: z.object({id: z.string()}), + artifact: z.object({id: z.string(), reviewMode: z.enum(["annotate", "interact"])}), links: z.object({artifact: z.url(), version: z.url()}), version: z.object({id: z.string(), number: z.number().int().positive()}), }); @@ -62,6 +63,96 @@ afterAll(async () => { }); describe("Cloudflare Worker runtime", () => { + it("ART-012: a populated older D1 schema upgrades saved artifacts and retries to annotate", async () => { + const source = "

Pre-feature D1 artifact

"; + const upload = await stageFile(source); + const publicationInput = {target: {accessSetting: "public_link", kind: "new_artifact", name: "Older D1 artifact"}}; + const publish = () => worker.fetch(upload.commitUrl, { + body: JSON.stringify(publicationInput), + headers: {...authenticatedJsonHeaders(), "Idempotency-Key": "cloudflare-review-mode-upgrade"}, + method: "POST", + }); + const committed = await publish(); + expect(committed.status).toBe(201); + const publication = publicationSchema.parse(await committed.json()); + await worker.stop(); + let downgraded = false; + const files = await readdir(persistPath, {recursive: true}); + for (const filename of files.filter((file) => file.endsWith(".sqlite"))) { + const database = new DatabaseSync(join(persistPath, filename)); + try { + const table = database.prepare("SELECT name FROM sqlite_schema WHERE type = 'table' AND name = 'artifacts'").get(); + if (table === undefined) continue; + database.exec("ALTER TABLE artifacts DROP COLUMN review_mode; ALTER TABLE idempotency_records DROP COLUMN review_mode; UPDATE artifact_server_schema SET version = 9 WHERE component = 'runtime';"); + for (const tableName of ["idempotency_records", "actions"]) { + const schema = z.object({sql: z.string()}).parse(database.prepare( + "SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = ?", + ).get(tableName)); + const indexes = z.array(z.object({sql: z.string()})).parse(database.prepare( + "SELECT sql FROM sqlite_schema WHERE type = 'index' AND tbl_name = ? AND sql IS NOT NULL", + ).all(tableName)); + const legacySchema = schema.sql + .replace(`CREATE TABLE ${tableName}`, `CREATE TABLE ${tableName}_legacy`) + .replace("'change_review_mode', ", ""); + expect(legacySchema).not.toContain("change_review_mode"); + database.exec(`${legacySchema}; INSERT INTO ${tableName}_legacy SELECT * FROM ${tableName}; DROP TABLE ${tableName}; ALTER TABLE ${tableName}_legacy RENAME TO ${tableName};`); + for (const index of indexes) database.exec(index.sql); + } + downgraded = true; + } finally {database.close();} + } + expect(downgraded).toBe(true); + worker = await startWorker(persistPath); + const replay = await publish(); + expect(replay.status).toBe(200); + expect(await replay.json()).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true, version: publication.version}); + const content = await worker.fetch(publication.links.version); + expect(await content.text()).toBe(source); + }, 30_000); + + it("ART-012: review mode and historical mutation replay survive D1 restart without changing content", async () => { + const source = "

Immutable review content

"; + const upload = await stageFile(source); + const committed = await worker.fetch(upload.commitUrl, { + body: JSON.stringify({target: { + accessSetting: "public_link", kind: "new_artifact", name: "Interact review", reviewMode: "interact", + }}), + headers: {...authenticatedJsonHeaders(), "Idempotency-Key": "cloudflare-review-mode-publish"}, + method: "POST", + }); + expect(committed.status).toBe(201); + const publication = publicationSchema.parse(await committed.json()); + expect(publication.artifact.reviewMode).toBe("interact"); + const changeMode = (reviewMode: "annotate" | "interact", key: string) => worker.fetch( + `${origin}/api/v1/artifacts/${publication.artifact.id}/review-mode`, + { + body: JSON.stringify({expectedCurrentVersionId: publication.version.id, reviewMode}), + headers: {...authenticatedJsonHeaders(), "Idempotency-Key": key}, + method: "PATCH", + }, + ); + expect((await changeMode("annotate", "cloudflare-review-mode-first")).status).toBe(200); + expect((await changeMode("interact", "cloudflare-review-mode-second")).status).toBe(200); + await worker.stop(); + worker = await startWorker(persistPath); + const replay = await changeMode("annotate", "cloudflare-review-mode-first"); + expect(replay.status).toBe(200); + expect(await replay.json()).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true, version: publication.version}); + const details = await worker.fetch(`${origin}/api/v1/artifacts/${publication.artifact.id}`, { + headers: {Authorization: `Bearer ${apiToken}`}, + }); + expect(details.status).toBe(200); + expect(await details.json()).toMatchObject({artifact: {reviewMode: "interact"}, current: {version: publication.version}}); + const content = await worker.fetch(publication.links.version); + expect(content.status).toBe(200); + expect(await content.text()).toBe(source); + const actions = await worker.fetch(`${origin}/api/v1/artifacts/${publication.artifact.id}/actions`, { + headers: {Authorization: `Bearer ${apiToken}`}, + }); + expect(actionListSchema.parse(await actions.json()).actions.map(({action}) => action).toSorted()) + .toEqual(["change_review_mode", "change_review_mode", "publish"]); + }, 30_000); + it("serves the management application only from its configured origin", async () => { const shell = await worker.fetch(`${origin}/review?project=prj_default`); const shellHtml = await shell.text(); diff --git a/docs/cli.md b/docs/cli.md index ca3683c1..76886f92 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -11,8 +11,11 @@ Publish one finished file or a complete directory: ```sh artifactserver publish ./report.pdf artifactserver publish ./dist --public --name "Product prototype" --tag prototype +artifactserver publish ./dist --name "Interactive prototype" --review-mode interact ``` +New artifacts open in Annotate mode by default. Use `--review-mode interact` to let people click and use an HTML prototype immediately, or `--review-mode annotate` to select the default explicitly. The option applies only when creating an artifact; combining it with `--artifact` is rejected. Later versions preserve the artifact's default. See [Review mode](./review-mode.md) for changing an existing default and sharing a link with a mode override. + Publication returns JSON with three browser links: | Link | Purpose | diff --git a/docs/mcp.md b/docs/mcp.md index 85a3faf2..0f0a0c7e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -55,6 +55,8 @@ An agent uses this sequence: When the agent publishes a new version, it first reads the current version ID. It sends that ID as `expectedCurrentVersionId`. +For a new artifact, `artifact_commit_upload` accepts `target.reviewMode` as `annotate` (the default) or `interact`. Use `interact` for an HTML prototype that should accept clicks immediately. A new version keeps the existing artifact default. To change that default later, use `artifact_set_review_mode` with `artifactId`, `reviewMode`, `expectedCurrentVersionId`, an `idempotencyKey`, and an optional `projectId`. See [Review mode](./review-mode.md) for link overrides and the HTTP equivalent. + ## Use the publication result Every successful publication returns structured data and a short text summary. @@ -88,7 +90,7 @@ opens its raw immutable content. Agents should hand people the Review URL. | Projects | `project_list`, `project_create`, `project_rename`, `project_archive`, `project_unarchive` | | Artifacts | `artifact_list`, `artifact_get`, `artifact_open`, `artifact_version_list`, `artifact_diff` | | Publication | `artifact_create_upload`, `artifact_commit_upload` | -| Management | `artifact_set_visibility`, `artifact_set_tags`, `artifact_restore_version`, `artifact_delete` | +| Management | `artifact_set_visibility`, `artifact_set_tags`, `artifact_set_review_mode`, `artifact_restore_version`, `artifact_delete` | | Comments | `comment_list`, `comment_get`, `comment_create`, `comment_reply`, `comment_update`, `comment_resolve`, `comment_delete` | | Git history planning | `project_git_history_status`, `project_git_history_estimate` | diff --git a/docs/review-mode.md b/docs/review-mode.md new file mode 100644 index 00000000..8a28b0d9 --- /dev/null +++ b/docs/review-mode.md @@ -0,0 +1,51 @@ +# Review mode + +HTML artifacts have two Review modes. **Annotate** captures clicks for comments. **Interact** lets clicks reach the artifact so people can use a prototype. The existing mode toggle and Escape shortcut remain available. + +Every artifact has a default `reviewMode`: `annotate` or `interact`. Existing artifacts and new artifacts without an explicit choice default to `annotate`. The default belongs to the artifact and remains through publication and restore of immutable versions. + +## Choose a default when publishing + +With the CLI: + +```sh +artifactserver publish ./dist --name "Product prototype" --review-mode interact +``` + +`--review-mode` applies only to a new artifact. It cannot be combined with `--artifact` when publishing a new version. + +With MCP, include `reviewMode` in the `new_artifact` target of `artifact_commit_upload`: + +```json +{ + "kind": "new_artifact", + "name": "Product prototype", + "reviewMode": "interact" +} +``` + +The HTTP upload-commit request accepts the same field in `target`. A `new_version` target does not accept it. + +## Change an existing default + +Read the artifact to obtain its current version ID. Call `artifact_set_review_mode` with `artifactId`, `reviewMode`, `expectedCurrentVersionId`, a new `idempotencyKey`, and, when needed, `projectId`. + +The HTTP equivalent is: + +```http +PATCH /api/v1/artifacts/art_example/review-mode?projectId=prj_example +Idempotency-Key: example-review-mode-change +Content-Type: application/json + +{"reviewMode":"interact","expectedCurrentVersionId":"ver_example"} +``` + +The project query is optional. The operation requires artifact-management permission and checks that the current version still matches `expectedCurrentVersionId`. Reuse the idempotency key only for an exact retry. Changing the default records a `change_review_mode` action; it creates no version and changes no stored file bytes or version IDs. + +## Override a mode in a link + +Add `mode=interact` or `mode=annotate` to a Review URL. For example, add `&mode=interact` to the returned `links.review` URL, which already has query parameters. + +The initial mode is chosen in this order: a valid link override, the artifact's stored default, then `annotate`. An invalid mode value is ignored. Review navigation preserves a valid override, and browser back and forward restore it. Changing the mode with the toggle or Escape does not change the stored default. Refreshing Review data or changing the selected version does not reset a manual choice. + +Mode selection changes HTML Review interaction only. It does not change artifact access, sandboxing, raw-content links, or the presentation of image and video previews. diff --git a/project/evidence/artifact-review-mode-browser.json b/project/evidence/artifact-review-mode-browser.json new file mode 100644 index 00000000..e429d5f2 --- /dev/null +++ b/project/evidence/artifact-review-mode-browser.json @@ -0,0 +1 @@ +{"numFailedTestSuites":0,"numFailedTests":0,"numPassedTestSuites":1,"numPassedTests":12,"numPendingTests":0,"numPendingTestSuites":0,"numTodoTests":0,"numTotalTestSuites":1,"numTotalTests":12,"startTime":1791608120061,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["Artifact review mode"],"duration":1896,"failureMessages":[],"fullName":"Artifact review mode PRV-006-B PRV-006-F: artifact review mode: annotate default with interact override starts interact","status":"passed","title":"PRV-006-B PRV-006-F: artifact review mode: annotate default with interact override starts interact"},{"ancestorTitles":["Artifact review mode"],"duration":1164,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: interact default with annotate override starts annotate","status":"passed","title":"artifact review mode: interact default with annotate override starts annotate"},{"ancestorTitles":["Artifact review mode"],"duration":1307,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: interact default with no override starts interact","status":"passed","title":"artifact review mode: interact default with no override starts interact"},{"ancestorTitles":["Artifact review mode"],"duration":1312,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: interact default with ANNOTATE override starts interact","status":"passed","title":"artifact review mode: interact default with ANNOTATE override starts interact"},{"ancestorTitles":["Artifact review mode"],"duration":1135,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: legacy default with invalid override starts annotate","status":"passed","title":"artifact review mode: legacy default with invalid override starts annotate"},{"ancestorTitles":["Artifact review mode"],"duration":1082,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: legacy default with no override starts annotate","status":"passed","title":"artifact review mode: legacy default with no override starts annotate"},{"ancestorTitles":["Artifact review mode"],"duration":1192,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: a link override survives focus navigation and browser history","status":"passed","title":"artifact review mode: a link override survives focus navigation and browser history"},{"ancestorTitles":["Artifact review mode"],"duration":1425,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: Escape and toggles survive version selection and refreshed artifact details","status":"passed","title":"artifact review mode: Escape and toggles survive version selection and refreshed artifact details"},{"ancestorTitles":["Artifact review mode"],"duration":1235,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: browser history restores a link override after a manual toggle","status":"passed","title":"artifact review mode: browser history restores a link override after a manual toggle"},{"ancestorTitles":["Artifact review mode"],"duration":1009,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: mode-only history entries and removal use the restored URL","status":"passed","title":"artifact review mode: mode-only history entries and removal use the restored URL"},{"ancestorTitles":["Artifact review mode"],"duration":1412,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: selecting another artifact applies its default","status":"passed","title":"artifact review mode: selecting another artifact applies its default"},{"ancestorTitles":["Artifact review mode"],"duration":1138,"failureMessages":[],"fullName":"Artifact review mode artifact review mode: a manual Escape before initial details arrives remains selected","status":"passed","title":"artifact review mode: a manual Escape before initial details arrives remains selected"}],"endTime":1791608137251.823,"message":"","name":"/home/jcm/Projects/artifacts-server/.superpowers/review-mode/tests/browser/artifact-review-mode.spec.ts","startTime":1791608120061,"status":"passed"}]} diff --git a/project/evidence/artifact-review-mode.json b/project/evidence/artifact-review-mode.json new file mode 100644 index 00000000..c6dd0fb6 --- /dev/null +++ b/project/evidence/artifact-review-mode.json @@ -0,0 +1 @@ +{"numTotalTestSuites":5,"numPassedTestSuites":5,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":10,"numPassedTests":10,"numFailedTests":0,"numPendingTests":0,"numTodoTests":0,"snapshot":{"added":0,"failure":false,"filesAdded":0,"filesRemoved":0,"filesRemovedList":[],"filesUnmatched":0,"filesUpdated":0,"matched":0,"total":0,"unchecked":0,"uncheckedKeysByFile":[],"unmatched":0,"updated":0,"didUpdate":false},"startTime":1791608190471,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":[],"fullName":"file publication forwards the review default and preserves default-mode retries","status":"passed","title":"file publication forwards the review default and preserves default-mode retries","duration":205.34601799999996,"failureMessages":[],"meta":{},"tags":[]}],"startTime":1791608192449,"endTime":1791608192654.346,"status":"passed","message":"","name":"/home/jcm/Projects/artifacts-server/.superpowers/review-mode/tests/client/artifact-review-mode-publication.test.ts"},{"assertionResults":[{"ancestorTitles":["artifact review mode persistence"],"fullName":"artifact review mode persistence publication defaults to annotate and survives a restart","status":"passed","title":"publication defaults to annotate and survives a restart","duration":44.76984799999997,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode persistence"],"fullName":"artifact review mode persistence interact publication, mutation replay, and immutable version identity are durable","status":"passed","title":"interact publication, mutation replay, and immutable version identity are durable","duration":21.23296399999998,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode persistence"],"fullName":"artifact review mode persistence versions, restores, tags, access changes, and list projections preserve the saved mode","status":"passed","title":"versions, restores, tags, access changes, and list projections preserve the saved mode","duration":16.039143000000024,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode persistence"],"fullName":"artifact review mode persistence rejects stale versions, changed idempotent input, and invalid database values","status":"passed","title":"rejects stale versions, changed idempotent input, and invalid database values","duration":13.697771999999986,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode persistence"],"fullName":"artifact review mode persistence pre-feature rows and saved retries migrate to annotate without changing versions","status":"passed","title":"pre-feature rows and saved retries migrate to annotate without changing versions","duration":21.619479000000013,"failureMessages":[],"meta":{},"tags":[]}],"startTime":1791608192997,"endTime":1791608193114.6194,"status":"passed","message":"","name":"/home/jcm/Projects/artifacts-server/.superpowers/review-mode/tests/conformance/artifact-review-mode-persistence.test.ts"},{"assertionResults":[{"ancestorTitles":["artifact review mode"],"fullName":"artifact review mode ART-012-B: publishing and updating the review default preserves saved version identity","status":"passed","title":"ART-012-B: publishing and updating the review default preserves saved version identity","duration":168.76290700000004,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode"],"fullName":"artifact review mode ART-012-F: invalid, stale, and unauthenticated mode mutations leave the artifact unchanged","status":"passed","title":"ART-012-F: invalid, stale, and unauthenticated mode mutations leave the artifact unchanged","duration":40.74549899999988,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode"],"fullName":"artifact review mode a conflicting publication retry cannot change the original review default","status":"passed","title":"a conflicting publication retry cannot change the original review default","duration":42.822357999999895,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["artifact review mode"],"fullName":"artifact review mode a publish-only credential cannot change the artifact's review default","status":"passed","title":"a publish-only credential cannot change the artifact's review default","duration":42.41767200000004,"failureMessages":[],"meta":{},"tags":[]}],"startTime":1791608191562,"endTime":1791608191856.4177,"status":"passed","message":"","name":"/home/jcm/Projects/artifacts-server/.superpowers/review-mode/tests/conformance/artifact-review-mode.test.ts"}]} \ No newline at end of file diff --git a/project/spec/artifact-review-mode-spec.md b/project/spec/artifact-review-mode-spec.md new file mode 100644 index 00000000..32ad0520 --- /dev/null +++ b/project/spec/artifact-review-mode-spec.md @@ -0,0 +1,42 @@ +# Artifact Review mode + +**Status:** Implemented with local behavior evidence; equivalent live deployment evidence pending. +**Owner:** Artifact core and web application +**Companion documents:** [Product specification](./artifact-server-product-spec.md), [Media previews](./media-preview-spec.md), [Conformance ledger](./conformance.yml) + +## Outcome + +An author can make an HTML prototype open ready for interaction while keeping Annotate as the default for existing artifacts. A reviewer can override the initial mode in a shared Review link and continue using the existing toggle and Escape shortcut. + +## Artifact metadata + +`ArtifactRecord.reviewMode` is required and is either `annotate` or `interact`. Creation inputs may omit it; omission means `annotate`. The default belongs to the artifact, not a version or manifest, and survives new versions, restoration, and reads from SQLite, PostgreSQL, or D1. Each adapter follows its existing migration scheme to initialize legacy artifacts to `annotate`. + +Staged HTTP upload commit and MCP `artifact_commit_upload` accept `target.reviewMode` only for `target.kind: new_artifact`. CLI `publish --review-mode annotate|interact` forwards that creation choice. Combining an explicit CLI option with `--artifact` is rejected rather than silently changing existing metadata. + +## Change contract + +`PATCH /api/v1/artifacts/:artifactId/review-mode` accepts `{reviewMode, expectedCurrentVersionId}`, the standard idempotency-key header, and an optional `projectId` query. MCP `artifact_set_review_mode` accepts the equivalent metadata plus `artifactId`, `idempotencyKey`, and optional `projectId`. + +Both adapters use the normal artifact-management authorization and shared application operation. A stale expected current-version ID or an idempotency key reused with different input fails without changing the default or action history. An exact retry returns the original result. A successful change records `change_review_mode` with normal action attribution and does not create a version, move the current-version pointer, alter a manifest, or rewrite bytes. Archived-project and deleted-artifact rules remain the existing management rules. + +## Browser contract + +The initial HTML Review mode uses this precedence: + +1. An exact `mode=annotate` or `mode=interact` query value. +2. The selected artifact's stored default. +3. `annotate` when metadata is unavailable. + +Invalid query values are ignored. Review URL construction retains a valid mode override when changing artifact, version, file, or focus view. Browser history restores the mode associated with its URL. Once the reviewer manually changes mode with the existing toggle or Escape, data refresh and selected-version loading cannot overwrite that choice. Moving to another artifact without a link override uses that artifact's default. + +Interact allows actual clicks to reach the isolated HTML frame; Annotate retains its current comment-selection behavior. Selecting a mode does not alter authentication, access settings, the frame sandbox, stored content, or native media presentation. + +## Conformance + +| ID | Contract | +| --- | --- | +| `ART-012` | Persist the artifact default, migrate existing artifacts to Annotate, expose creation and management through supported adapters, preserve versions, and enforce normal authorization, concurrency, and idempotency. | +| `PRV-006` | Resolve initial HTML mode from URL and artifact metadata, allow real interaction, retain URL overrides through navigation and history, and preserve manual choices during refresh and version loading. | + +Requirements are `behavior_verified` with passing local evidence. A local run cannot establish live behavior on every supported deployment. diff --git a/project/spec/conformance.yml b/project/spec/conformance.yml index 7e009b1f..8423a73d 100644 --- a/project/spec/conformance.yml +++ b/project/spec/conformance.yml @@ -408,6 +408,25 @@ requirements: depends_on: [ART-002, ART-009, CMT-001] evidence: [] + - id: ART-012 + kind: behavior + behavior: Each artifact has an Annotate or Interact default Review mode, initialized to Annotate for legacy artifacts and mutable without changing any immutable version or file bytes. + owner: artifact-core + source: {file: artifact-review-mode-spec.md, anchor: conformance} + acceptance: + behavior: {id: ART-012-B, description: "Create artifacts with default and explicit Review modes, preserve the stored choice through reads and versions, change it through management with normal action attribution, and replay an exact retry without duplicating the action."} + failure: {id: ART-012-F, description: "Reject invalid modes, unauthorized changes, stale expected current-version IDs, and conflicting idempotency retries without changing metadata, version identity, current pointer, bytes, or action history."} + deployments: *all + status: behavior_verified + proof_gap: Local publication, management, persistence, and upgrade tests pass; equivalent live deployment evidence remains unrecorded. + depends_on: [ART-001, ART-004, AUTH-008, PUB-006, PUB-008, AUD-001] + evidence: + - deployment: local + tests: [ART-012-B, ART-012-F] + result: pass + run: project/evidence/artifact-review-mode.json + recorded_at: "2026-10-10T04:56:30.471Z" + - id: MAN-001 kind: behavior behavior: A canonical manifest contains a sorted list of normalized paths, fingerprints, sizes, media types, disposition, entry file, routing mode, and serving metadata. @@ -4871,3 +4890,22 @@ requirements: result: pass run: project/evidence/browser.json recorded_at: "2026-08-24T03:47:04.897Z" + + - id: PRV-006 + kind: behavior + behavior: HTML Review starts from a valid URL mode override or the artifact default, permits real clicks in Interact, preserves link overrides in navigation and browser history, and retains manual mode changes during data refresh and version loading. + owner: web-application + source: {file: artifact-review-mode-spec.md, anchor: conformance} + acceptance: + behavior: {id: PRV-006-B, description: "Open an HTML artifact with its stored default and both valid link overrides, interact with the actual frame, and preserve overrides through artifact, version, file, focus, and back/forward navigation."} + failure: {id: PRV-006-F, description: "Ignore an invalid link mode, preserve a manual toggle or Escape choice during data refresh and version loading, and keep Annotate as the legacy fallback without changing sandboxing or access."} + deployments: *all + status: behavior_verified + proof_gap: Local browser precedence, frame interaction, navigation, history, and manual-choice tests pass; equivalent live deployment evidence remains unrecorded. + depends_on: [ART-012, PRV-004, CMT-013] + evidence: + - deployment: local + tests: [PRV-006-B, PRV-006-F] + result: pass + run: project/evidence/artifact-review-mode-browser.json + recorded_at: "2026-10-10T04:55:20.061Z" diff --git a/skills/artifact-server/references/artifact-operations.md b/skills/artifact-server/references/artifact-operations.md index 7c481a5a..6b14b240 100644 --- a/skills/artifact-server/references/artifact-operations.md +++ b/skills/artifact-server/references/artifact-operations.md @@ -20,7 +20,7 @@ The publish command always returns structured JSON. Do not add a `--json` flag. artifactserver publish "" --profile "" --server "" --project "" --name "" ``` -Optional creation flags are repeatable `--tag`, `--public`, and `--entry`. Account-required access is the default. +Optional creation flags are repeatable `--tag`, `--public`, `--entry`, and `--review-mode annotate|interact`. Account-required access and Annotate mode are the defaults. Select Interact when the user wants an HTML prototype to accept clicks immediately. `--review-mode` cannot be combined with `--artifact`; a new version preserves the artifact default. ### Publish a new version @@ -50,6 +50,7 @@ Call `artifact_get` immediately before a write to obtain the current version ID. - `artifact_set_visibility`: set `account_required` or `public_link` without changing file bytes. - `artifact_set_tags`: replace the complete tag set. Preserve existing tags the user did not ask to remove. +- `artifact_set_review_mode`: set the artifact's default to `annotate` or `interact` without creating a version or changing file bytes. A valid `mode=annotate` or `mode=interact` parameter on the Review URL overrides this default for that link. - `artifact_restore_version`: make an existing saved version current. This restores an artifact version; it does not restore a server backup. When the user identifies the target relatively, such as "the older of the latest two," run `artifact_version_list` again immediately before the restore and confirm that the chosen version still has that relationship to the current version. An optimistic current-version guard alone does not preserve a relative description. - `artifact_delete`: use only for an explicit request to delete one artifact. Repeat the artifact name, server, and project before the destructive call. This does not delete the installation. diff --git a/src/application/artifact-management.ts b/src/application/artifact-management.ts index 69e024ea..d0af78b7 100644 --- a/src/application/artifact-management.ts +++ b/src/application/artifact-management.ts @@ -37,10 +37,12 @@ import type { ArtifactState, ArtifactVersion, PageCursor, + ReviewMode, VersionRecord, } from "../core/model.js"; import type { ChangeArtifactAccessSetting, + ChangeArtifactReviewMode, ChangeArtifactTags, DeleteArtifact, ListArtifactActions, @@ -87,6 +89,13 @@ export interface ChangeArtifactAccessCommand extends ReadArtifactCommand { readonly idempotencyKey: string; } +/** Input for changing the artifact's initial HTML review mode. */ +export interface ChangeArtifactReviewModeCommand extends ReadArtifactCommand { + readonly expectedCurrentVersionId: string; + readonly idempotencyKey: string; + readonly reviewMode: ReviewMode; +} + /** Input for replacing one artifact's complete tag set. */ export interface ChangeArtifactTagsCommand extends ReadArtifactCommand { readonly expectedCurrentVersionId: string; @@ -120,6 +129,15 @@ export interface DeleteArtifactCommand extends ReadArtifactCommand { /** Repository capabilities required by artifact management. */ export interface ArtifactManagementRepository { + readonly changeReviewMode: ( + command: ChangeArtifactReviewMode, + ) => Effect.Effect< + ArtifactState, + | ArtifactMutationConflict + | ArtifactNotFound + | IdempotencyConflict + | ArtifactRepositoryFailure + >; readonly changeAccessSetting: ( command: ChangeArtifactAccessSetting, ) => Effect.Effect< @@ -202,6 +220,9 @@ export type ArtifactManagementFailure = | ProjectManagementFailure; interface ArtifactManagementOperations { + readonly changeReviewMode: ( + command: ChangeArtifactReviewModeCommand, + ) => Effect.Effect; readonly changeAccess: ( command: ChangeArtifactAccessCommand, ) => Effect.Effect; @@ -526,6 +547,34 @@ function makeArtifactManagementService( }, ); + const changeReviewMode = Effect.fn("ArtifactManagementService.changeReviewMode")( + function*(command: ChangeArtifactReviewModeCommand) { + const project = yield* resolveProjectForRead(command.principal, command.projectId); + const artifact = yield* requireArtifact(project.id, command.artifactId); + yield* authorization.requireArtifactManagement(command.principal); + const idempotencyKey = yield* parseIdempotencyKey(command.idempotencyKey); + const createdAt = DateTime.formatIso(yield* dependencies.clock.now); + return yield* dependencies.repository.changeReviewMode({ + artifactId: artifact.id, + authorizedByPrincipalId: command.principal.authorizedByPrincipalId, + createdAt, + expectedCurrentVersionId: command.expectedCurrentVersionId, + idempotencyKey, + inputDigest: managementInputDigest({ + artifactId: artifact.id, + expectedCurrentVersionId: command.expectedCurrentVersionId, + operation: "change_review_mode", + principalId: command.principal.id, + projectId: project.id, + value: command.reviewMode, + }), + principalId: command.principal.id, + projectId: project.id, + reviewMode: command.reviewMode, + }); + }, + ); + const changeTags = Effect.fn("ArtifactManagementService.changeTags")( function*(command: ChangeArtifactTagsCommand) { const project = yield* resolveProjectForRead( @@ -593,6 +642,7 @@ function makeArtifactManagementService( return ArtifactManagementService.of({ changeAccess, + changeReviewMode, changeTags, deleteArtifact, getArtifact, @@ -610,6 +660,7 @@ interface ManagementInputDigest { readonly expectedCurrentVersionId: string; readonly operation: | "change_access" + | "change_review_mode" | "change_tags" | "delete" | "restore"; diff --git a/src/application/publish-artifact.ts b/src/application/publish-artifact.ts index ffed78f9..78d2c41d 100644 --- a/src/application/publish-artifact.ts +++ b/src/application/publish-artifact.ts @@ -24,6 +24,7 @@ import type { AccessSetting, CanonicalManifest, PublishedVersion, + ReviewMode, } from "../core/model.js"; import type { BlobWrite, @@ -60,6 +61,7 @@ export interface PublishPreparedNewArtifactCommand { readonly name: string; readonly principal: Principal; readonly projectId: string; + readonly reviewMode?: ReviewMode; readonly source: PublicationSource; readonly tags?: readonly string[]; } @@ -229,12 +231,14 @@ function makePublishArtifactService( ); const idempotencyKey = yield* parseIdempotencyKey(command.idempotencyKey); const tags = yield* parseArtifactTags(command.tags ?? []); + const reviewMode = command.reviewMode ?? "annotate"; const inputDigest = newArtifactInputDigest({ accessSetting: command.accessSetting, manifest: command.manifest, name, principalId: command.principal.id, projectId: command.projectId, + reviewMode, tags, }); const replayed = yield* dependencies.repository.findIdempotentPublication( @@ -267,6 +271,7 @@ function makePublishArtifactService( projectId: command.projectId, authorizedByPrincipalId: command.principal.authorizedByPrincipalId, source: command.source, + reviewMode, tags, versionId: dependencies.ids.versionId(), }); @@ -342,6 +347,7 @@ interface NewArtifactDigestInput { readonly name: string; readonly principalId: string; readonly projectId: string; + readonly reviewMode: ReviewMode; readonly tags: readonly string[]; } @@ -354,14 +360,17 @@ interface ArtifactVersionDigestInput { } function newArtifactInputDigest(input: NewArtifactDigestInput): string { - const canonicalInput = JSON.stringify({ + const value = { accessSetting: input.accessSetting, manifestDigest: input.manifest.digest, name: input.name, principalId: input.principalId, projectId: input.projectId, tags: input.tags, - }); + }; + const canonicalInput = JSON.stringify(input.reviewMode === "annotate" + ? value + : {...value, reviewMode: input.reviewMode}); return createHash("sha256").update(canonicalInput).digest("hex"); } diff --git a/src/application/staged-upload.ts b/src/application/staged-upload.ts index dc8db558..0cbe664a 100644 --- a/src/application/staged-upload.ts +++ b/src/application/staged-upload.ts @@ -17,6 +17,7 @@ import { uploadStatuses, type AccessSetting, type PublishedVersion, + type ReviewMode, type RoutingMode, type StagedUpload, } from "../core/model.js"; @@ -71,6 +72,7 @@ export type CommitStagedUploadTarget = readonly accessSetting: AccessSetting; readonly kind: "new_artifact"; readonly name: string; + readonly reviewMode?: ReviewMode; readonly tags?: readonly string[]; } | { @@ -397,6 +399,7 @@ function makeStagedUploadService( projectId: project.id, source, tags: command.target.tags ?? [], + reviewMode: command.target.reviewMode ?? "annotate", })); case "new_version": return yield* expirePublicationAtDeadline(signal, diff --git a/src/cli/publish-command.ts b/src/cli/publish-command.ts index 3dac7ffd..43f8133b 100644 --- a/src/cli/publish-command.ts +++ b/src/cli/publish-command.ts @@ -30,6 +30,7 @@ interface PublishOptions { readonly profile?: string; readonly profileData: string; readonly routing: "spa" | "static"; + readonly reviewMode?: "annotate" | "interact"; readonly server?: string; readonly tag: readonly string[]; readonly tokenFile?: string; @@ -73,6 +74,10 @@ export function configurePublishCommand( .default("static"), ) .addOption(new Option("--name ", "name for a new artifact")) + .addOption( + new Option("--review-mode ", "initial HTML review mode for a new artifact") + .choices(["annotate", "interact"]), + ) .addOption(new Option("--public", "allow the link to open without sign-in").default(false)) .addOption( new Option("--tag ", "tag for a new artifact; repeat for more tags") @@ -142,9 +147,9 @@ function publicationCommand( routingMode: options.routing, }; if (options.artifact !== undefined && options.expectedVersion !== undefined) { - if (options.name !== undefined || options.public || options.tag.length > 0) { + if (options.name !== undefined || options.public || options.tag.length > 0 || options.reviewMode !== undefined) { throw new Error( - "--name, --public, and --tag apply only when creating a new artifact.", + "--name, --public, --tag, and --review-mode apply only when creating a new artifact.", ); } return withEntryPath({ @@ -157,7 +162,7 @@ function publicationCommand( }, options.entry); } - const target = options.name === undefined + const initialTarget = options.name === undefined ? { accessSetting: options.public ? "public_link" as const : "account_required" as const, kind: "new_artifact" as const, @@ -169,6 +174,9 @@ function publicationCommand( name: options.name, tags: options.tag, }; + const target = options.reviewMode === undefined + ? initialTarget + : {...initialTarget, reviewMode: options.reviewMode}; return withEntryPath({ ...common, target, diff --git a/src/client/file-publication-client.ts b/src/client/file-publication-client.ts index 81d749a0..df52c5b9 100644 --- a/src/client/file-publication-client.ts +++ b/src/client/file-publication-client.ts @@ -80,6 +80,7 @@ const artifactSchema = Schema.Struct({ id: Schema.String, name: Schema.String, projectId: Schema.String, + reviewMode: Schema.Literals(["annotate", "interact"]), tags: Schema.Array(Schema.String), }); const versionSchema = Schema.Struct({ @@ -189,6 +190,7 @@ export type FilePublicationTarget = readonly accessSetting: "account_required" | "public_link"; readonly kind: "new_artifact"; readonly name?: string; + readonly reviewMode?: "annotate" | "interact"; readonly tags: readonly string[]; } | { @@ -263,6 +265,7 @@ type CommitPublicationTarget = readonly accessSetting: "account_required" | "public_link"; readonly kind: "new_artifact"; readonly name: string; + readonly reviewMode?: "annotate" | "interact"; readonly tags: readonly string[]; } | { @@ -385,14 +388,14 @@ function effectiveCommitTarget( target: FilePublicationTarget, defaultName: string, ): CommitPublicationTarget { - return target.kind === "new_artifact" - ? { - accessSetting: target.accessSetting, - kind: target.kind, - name: target.name ?? defaultName, - tags: target.tags, - } - : target; + if (target.kind !== "new_artifact") return target; + const resolved = { + accessSetting: target.accessSetting, + kind: target.kind, + name: target.name ?? defaultName, + tags: target.tags, + }; + return target.reviewMode === "interact" ? {...resolved, reviewMode: target.reviewMode} : resolved; } /** Infer a deterministic browser media type from one file name. */ diff --git a/src/core/errors.ts b/src/core/errors.ts index 389f4c7f..f3cac3e6 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -358,6 +358,7 @@ export class ArtifactRepositoryFailure extends Schema.TaggedError; commitVersion(command: CommitArtifactVersion): Promise; + changeReviewMode(command: ChangeArtifactReviewMode): Promise; changeAccessSetting(command: ChangeArtifactAccessSetting): Promise; changeTags(command: ChangeArtifactTags): Promise; deleteArtifact(command: DeleteArtifact): Promise; diff --git a/src/http/create-http-app.ts b/src/http/create-http-app.ts index 45aa95cc..21729675 100644 --- a/src/http/create-http-app.ts +++ b/src/http/create-http-app.ts @@ -192,6 +192,7 @@ const commitUploadSchema = z.object({ accessSetting: accessSettingSchema.default(accessSettings.accountRequired), kind: z.literal("new_artifact"), name: z.string().min(1).max(200), + reviewMode: z.enum(["annotate", "interact"]).default("annotate"), tags: artifactTagsSchema, }).strict(), z.object({ @@ -242,6 +243,10 @@ const changeTagsSchema = z.object({ expectedCurrentVersionId: z.string().min(1).max(200), tags: artifactTagsSchema, }); +const changeReviewModeSchema = z.object({ + expectedCurrentVersionId: z.string().min(1).max(200), + reviewMode: z.enum(["annotate", "interact"]), +}).strict(); const comparisonQuerySchema = z.object({ fromVersionId: z.string().min(1).max(200), toVersionId: z.string().min(1).max(200), @@ -2117,6 +2122,30 @@ export function createHttpApp( }, ); + app.patch( + "/api/v1/artifacts/:artifactId/review-mode", + boundedJsonBody, + async (context) => { + const body = changeReviewModeSchema.parse(await context.req.json()); + const state = await runHttpApplicationEffect( + context, + dependencies, + ArtifactManagementService.use((management) => + management.changeReviewMode({ + ...body, + artifactId: context.req.param("artifactId"), + idempotencyKey: requiredIdempotencyKey(context.req.header("idempotency-key")), + principal: context.get("principal"), + projectId: requestedProjectId(context), + }) + ), + ); + return context.json(artifactStateResponse( + responseApplicationUrl(context, dependencies), dependencies.contentDomain, state, + )); + }, + ); + app.delete( "/api/v1/artifacts/:artifactId", boundedJsonBody, diff --git a/src/local/create-local-application-layer.ts b/src/local/create-local-application-layer.ts index 3e390758..8bd56db2 100644 --- a/src/local/create-local-application-layer.ts +++ b/src/local/create-local-application-layer.ts @@ -464,6 +464,11 @@ export function createApplicationLayer( try: () => adapters.repository.changeAccessSetting(command), catch: classifyChangeAccessFailure, }), + changeReviewMode: (command) => + Effect.tryPromise({ + try: () => adapters.repository.changeReviewMode(command), + catch: classifyChangeReviewModeFailure, + }), changeTags: (command) => Effect.tryPromise({ try: () => adapters.repository.changeTags(command), @@ -1328,6 +1333,21 @@ function classifyChangeAccessFailure(cause: unknown): return repositoryFailure("changeAccessSetting", cause); } +function classifyChangeReviewModeFailure(cause: unknown): + | ArtifactNotFound + | ArtifactMutationConflict + | IdempotencyConflict + | ArtifactRepositoryFailure { + if ( + cause instanceof ArtifactNotFound || + cause instanceof ArtifactMutationConflict || + cause instanceof IdempotencyConflict + ) { + return cause; + } + return repositoryFailure("changeReviewMode", cause); +} + function classifyChangeTagsFailure(cause: unknown): | ArtifactNotFound | ArtifactMutationConflict diff --git a/src/mcp/artifact-mcp-server.ts b/src/mcp/artifact-mcp-server.ts index d76a0d4d..804073a3 100644 --- a/src/mcp/artifact-mcp-server.ts +++ b/src/mcp/artifact-mcp-server.ts @@ -134,6 +134,7 @@ const artifactRecordSchema = z.object({ id: z.string(), name: z.string(), projectId: z.string(), + reviewMode: z.enum(["annotate", "interact"]), tags: z.array(z.string()), }).strict(); const versionRecordSchema = z.object({ @@ -1252,6 +1253,7 @@ export function createArtifactMcpServer( ), kind: z.literal("new_artifact"), name: z.string().min(1).max(200), + reviewMode: z.enum(["annotate", "interact"]).default("annotate"), tags: z.array(tagSchema).max(20).default([]), }).strict(), z.object({ @@ -1331,6 +1333,41 @@ export function createArtifactMcpServer( }), ); + registerNudgedTool( + "artifact_set_review_mode", + { + title: "Change the initial review mode", + description: + "Set the artifact's default HTML review mode to annotate or interact without changing saved files or versions. A valid mode parameter on a review link overrides this default for that visit.", + inputSchema: z.object({ + artifactId: artifactIdSchema, + expectedCurrentVersionId: expectedVersionSchema, + idempotencyKey: idempotencyKeySchema, + projectId: optionalProjectIdSchema, + reviewMode: z.enum(["annotate", "interact"]), + }).strict(), + outputSchema: artifactStateSchema.extend({ + links: z.object({artifact: z.url(), version: z.url()}).strict(), + }).strict(), + annotations: idempotentWriteAnnotations, + }, + async (input) => toolResult(async () => { + const state = await runMcpApplicationEffect( + dependencies, + ArtifactManagementService.use((management) => + management.changeReviewMode({...input, principal: identity.principal}) + ), + ); + return { + ...state, + links: { + artifact: artifactBrowserUrl(applicationUrl, state.artifact.id), + version: versionBrowserUrl(applicationUrl, dependencies.contentDomain, state.version.contentToken), + }, + }; + }), + ); + registerNudgedTool( "artifact_set_tags", { diff --git a/src/storage/postgres-artifact-repository.ts b/src/storage/postgres-artifact-repository.ts index 852a8a2b..bfc12e61 100644 --- a/src/storage/postgres-artifact-repository.ts +++ b/src/storage/postgres-artifact-repository.ts @@ -24,6 +24,7 @@ import { } from "../core/errors.js"; import { accessSettings, + reviewModes, agentBeaconStates, agentDispatchStates, artifactActionKinds, @@ -73,6 +74,7 @@ import type { ArtifactRepository, CancelAgentDispatch, ChangeArtifactAccessSetting, + ChangeArtifactReviewMode, ChangeArtifactTags, ClearCommentThreads, CommentRepository, @@ -147,6 +149,7 @@ const accessSettingSchema = z.enum([ accessSettings.accountRequired, accessSettings.publicLink, ]); +const reviewModeSchema = z.enum([reviewModes.annotate, reviewModes.interact]); const dispositionSchema = z.enum([ fileDispositions.attachment, fileDispositions.inline, @@ -158,6 +161,7 @@ const uploadStatusSchema = z.enum([ ]); const artifactActionKindSchema = z.enum([ artifactActionKinds.changeAccess, + artifactActionKinds.changeReviewMode, artifactActionKinds.changeTags, artifactActionKinds.commentCreate, artifactActionKinds.commentDelete, @@ -223,6 +227,7 @@ const gitHistoryMappingRowSchema = z.object({ versionId: z.string(), }); const artifactRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema, createdAt: z.string(), currentVersionId: z.string(), @@ -249,6 +254,7 @@ const versionRowSchema = z.object({ routingMode: routingModeSchema, }); const publishedRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema, artifactCreatedAt: z.string(), artifactDeletedAt: z.string().nullable(), @@ -286,11 +292,13 @@ const versionContentRowSchema = z.object({ versionId: z.string(), }); const idempotencyRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema.nullable(), artifactId: z.string(), inputDigest: z.string(), operation: z.enum([ "change_access", + "change_review_mode", "change_tags", "delete", "publish", @@ -1258,11 +1266,11 @@ export class PostgresArtifactRepository implements command.createdAt, ); yield* sql`INSERT INTO artifacts ( - installation_id, project_id, id, name, search_name, access_setting, + installation_id, project_id, id, name, search_name, access_setting, review_mode, current_version_id, created_at, deleted_at ) VALUES ( ${installationId}, ${command.projectId}, ${command.artifactId}, ${command.name}, - ${normalizeArtifactSearchText(command.name)}, ${command.accessSetting}, NULL, + ${normalizeArtifactSearchText(command.name)}, ${command.accessSetting}, ${command.reviewMode ?? reviewModes.annotate}, NULL, ${command.createdAt}, NULL )`; yield* this.#replaceTags(command.artifactId, command.tags); @@ -1427,6 +1435,26 @@ export class PostgresArtifactRepository implements ); } + async changeReviewMode( + command: ChangeArtifactReviewMode, + ): Promise { + return this.#managementTransaction( + command, + "change_review_mode", + null, + null, + (sql) => sql`UPDATE artifacts + SET review_mode = ${command.reviewMode} + WHERE installation_id = ${this.#installationId} + AND project_id = ${command.projectId} + AND id = ${command.artifactId} + AND current_version_id = ${command.expectedCurrentVersionId} + AND deleted_at IS NULL + RETURNING id`, + command.reviewMode, + ); + } + async changeTags(command: ChangeArtifactTags): Promise { const installationId = this.#installationId; return this.#database.run(Effect.gen({self: this}, function*() { @@ -1685,7 +1713,7 @@ export class PostgresArtifactRepository implements const rows = yield* sql.unsafe( `SELECT * FROM ( SELECT id, project_id AS "projectId", name, - access_setting AS "accessSetting", + review_mode AS "reviewMode", access_setting AS "accessSetting", current_version_id AS "currentVersionId", created_at AS "createdAt", deleted_at AS "deletedAt", ( @@ -1742,7 +1770,7 @@ export class PostgresArtifactRepository implements artifact.id AS "artifactId", artifact.project_id AS "projectId", artifact.name AS "artifactName", - artifact.access_setting AS "accessSetting", + artifact.review_mode AS "reviewMode", artifact.access_setting AS "accessSetting", artifact.current_version_id AS "currentVersionId", artifact.created_at AS "artifactCreatedAt", artifact.deleted_at AS "artifactDeletedAt", @@ -1784,6 +1812,7 @@ export class PostgresArtifactRepository implements const parsedRows = z.array(publicLinkInventoryRowSchema).parse(rows); const artifacts = yield* this.#withTagsForArtifacts(parsedRows.map((row) => ({ accessSetting: row.accessSetting, + reviewMode: row.reviewMode, createdAt: row.artifactCreatedAt, currentVersionId: row.currentVersionId, deletedAt: row.artifactDeletedAt, @@ -1914,7 +1943,7 @@ export class PostgresArtifactRepository implements return this.#database.run(Effect.gen({self: this}, function*() { const sql = yield* SqlClient; const rows = yield* sql.unsafe( - `SELECT a.access_setting AS "accessSetting", a.id AS "artifactId", + `SELECT a.review_mode AS "reviewMode", a.access_setting AS "accessSetting", a.id AS "artifactId", a.project_id AS "projectId", v.content_token AS "contentToken", v.id AS "versionId", e.path, e.size, e.media_type AS "mediaType", e.sha256, @@ -3286,11 +3315,12 @@ export class PostgresArtifactRepository implements } #managementTransaction( - command: ChangeArtifactAccessSetting, - operation: "change_access", - accessSetting: ArtifactRecord["accessSetting"], + command: ChangeArtifactAccessSetting | ChangeArtifactReviewMode, + operation: "change_access" | "change_review_mode", + accessSetting: ArtifactRecord["accessSetting"] | null, tagsJson: string | null, mutate: (sql: SqlClient) => Effect.Effect, + reviewMode: ArtifactRecord["reviewMode"] | null = null, ): Promise { const installationId = this.#installationId; return this.#database.run(Effect.gen({self: this}, function*() { @@ -3323,7 +3353,7 @@ export class PostgresArtifactRepository implements command.expectedCurrentVersionId, ); yield* this.#insertIdempotency({ - accessSetting, + accessSetting: accessSetting ?? artifact.accessSetting, artifactId: command.artifactId, createdAt: command.createdAt, idempotencyKey: command.idempotencyKey, @@ -3337,8 +3367,10 @@ export class PostgresArtifactRepository implements command.projectId, command.artifactId, command.expectedCurrentVersionId, - accessSetting, + accessSetting ?? artifact.accessSetting, false, + null, + reviewMode, ); })); })); @@ -3415,7 +3447,7 @@ export class PostgresArtifactRepository implements const sql = yield* SqlClient; const rows = yield* sql.unsafe( `SELECT id, project_id AS "projectId", name, - access_setting AS "accessSetting", + review_mode AS "reviewMode", access_setting AS "accessSetting", current_version_id AS "currentVersionId", created_at AS "createdAt", deleted_at AS "deletedAt" FROM artifacts @@ -3570,7 +3602,7 @@ export class PostgresArtifactRepository implements const rows = yield* sql.unsafe( `SELECT a.id AS "artifactId", a.name AS "artifactName", a.project_id AS "projectId", - a.access_setting AS "accessSetting", + a.review_mode AS "reviewMode", a.access_setting AS "accessSetting", a.current_version_id AS "currentVersionId", a.created_at AS "artifactCreatedAt", a.deleted_at AS "artifactDeletedAt", v.id AS "versionId", v.number AS "versionNumber", @@ -3590,6 +3622,7 @@ export class PostgresArtifactRepository implements return { artifact: { accessSetting: parsed.accessSetting, + reviewMode: parsed.reviewMode, createdAt: parsed.artifactCreatedAt, currentVersionId: parsed.currentVersionId, deletedAt: parsed.artifactDeletedAt, @@ -3623,7 +3656,7 @@ export class PostgresArtifactRepository implements return Effect.gen({self: this}, function*() { const sql = yield* SqlClient; const rows = yield* sql.unsafe( - `SELECT access_setting AS "accessSetting", artifact_id AS "artifactId", + `SELECT review_mode AS "reviewMode", access_setting AS "accessSetting", artifact_id AS "artifactId", input_digest AS "inputDigest", operation, tags_json AS "tagsJson", version_id AS "versionId" FROM idempotency_records @@ -3648,13 +3681,14 @@ export class PostgresArtifactRepository implements message: "The idempotency key was already used with different input.", }); } - return yield* this.#readPublishedVersion(projectId, parsed.versionId, true); + const published = yield* this.#readPublishedVersion(projectId, parsed.versionId, true); + return {...published, artifact: {...published.artifact, reviewMode: parsed.reviewMode}}; }); } #findIdempotentManagementResult( projectId: string, - operation: "change_access" | "restore", + operation: "change_access" | "change_review_mode" | "restore", idempotencyKey: string, inputDigest: string, ): Effect.Effect { @@ -3676,6 +3710,8 @@ export class PostgresArtifactRepository implements parsed.versionId, parsed.accessSetting, true, + null, + parsed.reviewMode, ); }); } @@ -3707,6 +3743,7 @@ export class PostgresArtifactRepository implements parsed.accessSetting, true, tags, + parsed.reviewMode, ); }); } @@ -3735,6 +3772,7 @@ export class PostgresArtifactRepository implements accessSetting: ArtifactRecord["accessSetting"], replayed: boolean, tags: readonly string[] | null = null, + reviewMode: ArtifactRecord["reviewMode"] | null = null, ): Effect.Effect { return Effect.gen({self: this}, function*() { const artifact = yield* this.#readArtifact(projectId, artifactId, false); @@ -3752,6 +3790,7 @@ export class PostgresArtifactRepository implements accessSetting, currentVersionId: versionId, tags: tags ?? artifact.tags, + reviewMode: reviewMode ?? artifact.reviewMode, }, replayed, version, @@ -3870,12 +3909,14 @@ export class PostgresArtifactRepository implements const sql = yield* SqlClient; yield* sql`INSERT INTO idempotency_records ( installation_id, project_id, idempotency_key, input_digest, artifact_id, version_id, - operation, access_setting, tags_json, created_at + operation, access_setting, tags_json, created_at, review_mode ) VALUES ( ${installationId}, ${record.projectId}, ${record.idempotencyKey}, ${record.inputDigest}, ${record.artifactId}, ${record.versionId}, ${record.operation}, - ${record.accessSetting}, ${record.tagsJson}, ${record.createdAt} + ${record.accessSetting}, ${record.tagsJson}, ${record.createdAt}, + (SELECT review_mode FROM artifacts WHERE installation_id = ${installationId} + AND project_id = ${record.projectId} AND id = ${record.artifactId}) )`; }); } diff --git a/src/storage/postgres-migrations.ts b/src/storage/postgres-migrations.ts index 7d64dcef..7d2c0b86 100644 --- a/src/storage/postgres-migrations.ts +++ b/src/storage/postgres-migrations.ts @@ -655,6 +655,26 @@ const addArtifactSearchName = Effect.gen(function*() { ); }); +const addArtifactReviewMode = Effect.gen(function*() { + const sql = yield* SqlClient; + yield* sql.unsafe(`ALTER TABLE artifacts ADD COLUMN review_mode TEXT NOT NULL + DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact'))`); + yield* sql.unsafe(`ALTER TABLE idempotency_records ADD COLUMN review_mode TEXT NOT NULL + DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact'))`); + yield* sql.unsafe(`ALTER TABLE idempotency_records + DROP CONSTRAINT idempotency_records_operation_check, + ADD CONSTRAINT idempotency_records_operation_check CHECK (operation IN ( + 'publish', 'restore', 'change_access', 'change_review_mode', 'change_tags', 'delete' + ))`); + yield* sql.unsafe(`ALTER TABLE actions + DROP CONSTRAINT actions_action_check, + ADD CONSTRAINT actions_action_check CHECK (action IN ( + 'publish', 'restore', 'change_access', 'change_review_mode', 'change_tags', 'delete', + 'comment_create', 'comment_reply', 'comment_update', 'comment_resolve', + 'comment_reopen', 'comment_delete' + ))`); +}); + const migrationLoader = Migrator.fromRecord({ "0001_initial_shared_schema": initialSchema, "0002_project_scoped_artifacts": addProjectScope, @@ -667,10 +687,11 @@ const migrationLoader = Migrator.fromRecord({ "0009_git_history_mirror": addGitHistoryMirror, "0010_agent_capabilities": widenRegisteredAgentKind, "0011_artifact_search_name": addArtifactSearchName, + "0012_artifact_review_mode": addArtifactReviewMode, }); /** Schema revision required by this Artifact Server build. */ -export const requiredPostgresSchemaVersion = 11; +export const requiredPostgresSchemaVersion = 12; /** Migration compatibility observed without changing Postgres. */ export interface PostgresMigrationStatus { @@ -758,6 +779,9 @@ export const readPostgresMigrationStatus = Effect.gen(function*() { }, { migration_id: 11, name: "artifact_search_name", + }, { + migration_id: 12, + name: "artifact_review_mode", }] as const; const observedRequiredHistory = rows.filter( (row) => row.migration_id <= requiredPostgresSchemaVersion, diff --git a/src/storage/public-link-inventory-row.ts b/src/storage/public-link-inventory-row.ts index 4df11e3f..e4a5be46 100644 --- a/src/storage/public-link-inventory-row.ts +++ b/src/storage/public-link-inventory-row.ts @@ -3,12 +3,14 @@ import {z} from "zod"; import type {PublicLinkInventoryPage} from "../application/public-link-administration.js"; import { accessSettings, + reviewModes, routingModes, type ArtifactRecord, } from "../core/model.js"; /** Validates the joined row shape used by public-link inventory queries. */ export const publicLinkInventoryRowSchema = z.object({ + reviewMode: z.enum([reviewModes.annotate, reviewModes.interact]), accessSetting: z.enum([ accessSettings.accountRequired, accessSettings.publicLink, diff --git a/src/storage/sqlite-artifact-repository.ts b/src/storage/sqlite-artifact-repository.ts index b527b483..f11f6b5c 100644 --- a/src/storage/sqlite-artifact-repository.ts +++ b/src/storage/sqlite-artifact-repository.ts @@ -31,6 +31,7 @@ import type { import {normalizeArtifactSearchText} from "../application/artifact-tags.js"; import { accessSettings, + reviewModes, agentBeaconStates, agentDispatchStates, artifactActionKinds, @@ -82,6 +83,7 @@ import type { ArtifactRepository, CancelAgentDispatch, ChangeArtifactAccessSetting, + ChangeArtifactReviewMode, ChangeArtifactTags, ClearCommentThreads, CommentRepository, @@ -156,6 +158,7 @@ const accessSettingSchema = z.enum([ accessSettings.accountRequired, accessSettings.publicLink, ]); +const reviewModeSchema = z.enum([reviewModes.annotate, reviewModes.interact]); const dispositionSchema = z.enum([ fileDispositions.attachment, fileDispositions.inline, @@ -168,6 +171,7 @@ const routingModeSchema = z.enum([routingModes.static, routingModes.spa]); const artifactActionKindSchema = z.enum([ artifactActionKinds.capture, artifactActionKinds.changeAccess, + artifactActionKinds.changeReviewMode, artifactActionKinds.changeTags, artifactActionKinds.commentCreate, artifactActionKinds.commentDelete, @@ -196,6 +200,7 @@ const sourceBindingRowSchema = z.object({ projectId: z.string(), }); const publishedRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema, artifactCreatedAt: z.string(), artifactDeletedAt: z.string().nullable(), @@ -224,11 +229,13 @@ const contentBootstrapRowSchema = z.object({ }); const contentSessionRowSchema = contentBootstrapRowSchema; const idempotencyRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema.nullable(), artifactId: z.string(), inputDigest: z.string(), operation: z.enum([ "change_access", + "change_review_mode", "change_tags", "delete", "publish", @@ -249,6 +256,7 @@ const artifactActionRowSchema = z.object({ versionId: z.string(), }); const artifactRowSchema = z.object({ + reviewMode: reviewModeSchema, accessSetting: accessSettingSchema, createdAt: z.string(), currentVersionId: z.string(), @@ -1319,10 +1327,10 @@ export class SqliteArtifactRepository implements this.#database .prepare( `INSERT INTO artifacts ( - id, project_id, name, search_name, access_setting, + id, project_id, name, search_name, access_setting, review_mode, current_version_id, created_at, deleted_at, source_path, source_fingerprint, source_status, source_verified_at - ) VALUES (?, ?, ?, ?, ?, NULL, ?, NULL, ?, ?, ?, ?)`, + ) VALUES (?, ?, ?, ?, ?, ?, NULL, ?, NULL, ?, ?, ?, ?)`, ) .run( command.artifactId, @@ -1330,6 +1338,7 @@ export class SqliteArtifactRepository implements command.name, normalizeArtifactSearchText(command.name), command.accessSetting, + command.reviewMode ?? reviewModes.annotate, command.createdAt, binding === null ? null : binding.path, binding === null ? null : binding.fingerprint, @@ -1635,6 +1644,71 @@ export class SqliteArtifactRepository implements ); } + changeReviewMode( + command: ChangeArtifactReviewMode, + ): Promise { + return Promise.resolve().then(() => + this.#transaction(() => { + const replayed = this.#findIdempotentManagementResult( + command.projectId, + "change_review_mode", + command.idempotencyKey, + command.inputDigest, + ); + if (replayed !== null) return replayed; + const artifact = this.#readArtifact(command.projectId, command.artifactId); + this.#assertExpectedCurrentVersion( + artifact, + command.expectedCurrentVersionId, + ); + const update = this.#database + .prepare( + `UPDATE artifacts + SET review_mode = ? + WHERE project_id = ? AND id = ? AND current_version_id = ? AND deleted_at IS NULL`, + ) + .run( + command.reviewMode, + command.projectId, + command.artifactId, + command.expectedCurrentVersionId, + ); + if (update.changes !== 1) { + throw changedDuringManagement(); + } + this.#insertAction( + command.projectId, + command.artifactId, + command.expectedCurrentVersionId, + command.idempotencyKey, + command.createdAt, + "change_review_mode", + command.principalId, + command.authorizedByPrincipalId, + ); + this.#insertIdempotency( + command.projectId, + command.idempotencyKey, + command.inputDigest, + command.artifactId, + command.expectedCurrentVersionId, + command.createdAt, + "change_review_mode", + artifact.accessSetting, + ); + return this.#readArtifactState( + command.projectId, + command.artifactId, + command.expectedCurrentVersionId, + artifact.accessSetting, + false, + null, + command.reviewMode, + ); + }), + ); + } + changeTags(command: ChangeArtifactTags): Promise { return Promise.resolve().then(() => this.#transaction(() => { @@ -1862,7 +1936,7 @@ export class SqliteArtifactRepository implements id, project_id AS projectId, name, - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, current_version_id AS currentVersionId, created_at AS createdAt, deleted_at AS deletedAt, @@ -1930,7 +2004,7 @@ export class SqliteArtifactRepository implements artifact.id AS artifactId, artifact.project_id AS projectId, artifact.name AS artifactName, - artifact.access_setting AS accessSetting, + artifact.review_mode AS reviewMode, artifact.access_setting AS accessSetting, artifact.current_version_id AS currentVersionId, artifact.created_at AS artifactCreatedAt, artifact.deleted_at AS artifactDeletedAt, @@ -1972,6 +2046,7 @@ export class SqliteArtifactRepository implements const parsedRows = z.array(publicLinkInventoryRowSchema).parse(rows); const artifacts = this.#withTagsForArtifacts(parsedRows.map((row) => ({ accessSetting: row.accessSetting, + reviewMode: row.reviewMode, createdAt: row.artifactCreatedAt, currentVersionId: row.currentVersionId, deletedAt: row.artifactDeletedAt, @@ -2114,7 +2189,7 @@ export class SqliteArtifactRepository implements const row = this.#database .prepare( `SELECT - a.access_setting AS accessSetting, + a.review_mode AS reviewMode, a.access_setting AS accessSetting, a.id AS artifactId, a.project_id AS projectId, v.content_token AS contentToken, @@ -2536,14 +2611,14 @@ export class SqliteArtifactRepository implements #findIdempotentManagementResult( projectId: string, - operation: "change_access" | "restore", + operation: "change_access" | "change_review_mode" | "restore", idempotencyKey: string, inputDigest: string, ): ArtifactState | null { const row = this.#database .prepare( `SELECT - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, artifact_id AS artifactId, input_digest AS inputDigest, operation, @@ -2570,6 +2645,8 @@ export class SqliteArtifactRepository implements parsed.versionId, parsed.accessSetting, true, + null, + parsed.reviewMode, ); } @@ -2581,7 +2658,7 @@ export class SqliteArtifactRepository implements const row = this.#database .prepare( `SELECT - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, artifact_id AS artifactId, input_digest AS inputDigest, operation, @@ -2612,6 +2689,7 @@ export class SqliteArtifactRepository implements parsed.accessSetting, true, tags, + parsed.reviewMode, ); } @@ -2623,7 +2701,7 @@ export class SqliteArtifactRepository implements const row = this.#database .prepare( `SELECT - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, artifact_id AS artifactId, input_digest AS inputDigest, operation, @@ -2658,7 +2736,7 @@ export class SqliteArtifactRepository implements id, project_id AS projectId, name, - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, current_version_id AS currentVersionId, created_at AS createdAt, deleted_at AS deletedAt @@ -2680,7 +2758,7 @@ export class SqliteArtifactRepository implements id, project_id AS projectId, name, - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, current_version_id AS currentVersionId, created_at AS createdAt, deleted_at AS deletedAt @@ -2774,6 +2852,7 @@ export class SqliteArtifactRepository implements accessSetting: ArtifactRecord["accessSetting"], replayed: boolean, tags: readonly string[] | null = null, + reviewMode: ArtifactRecord["reviewMode"] | null = null, ): ArtifactState { const artifact = this.#readArtifact(projectId, artifactId); const version = this.#readVersionOrNull(projectId, versionId, artifactId); @@ -2786,6 +2865,7 @@ export class SqliteArtifactRepository implements accessSetting, currentVersionId: versionId, tags: tags ?? artifact.tags, + reviewMode: reviewMode ?? artifact.reviewMode, }, replayed, version, @@ -2865,7 +2945,7 @@ export class SqliteArtifactRepository implements const row = this.#database .prepare( `SELECT - access_setting AS accessSetting, + review_mode AS reviewMode, access_setting AS accessSetting, artifact_id AS artifactId, input_digest AS inputDigest, operation, @@ -2882,7 +2962,8 @@ export class SqliteArtifactRepository implements message: "The idempotency key was already used with different input.", }); } - return this.#readPublishedVersion(projectId, parsed.versionId, true); + const published = this.#readPublishedVersion(projectId, parsed.versionId, true); + return {...published, artifact: {...published.artifact, reviewMode: parsed.reviewMode}}; } #insertAction( @@ -2984,6 +3065,7 @@ export class SqliteArtifactRepository implements createdAt: string, operation: | "change_access" + | "change_review_mode" | "change_tags" | "delete" | "publish" @@ -2995,8 +3077,8 @@ export class SqliteArtifactRepository implements .prepare( `INSERT INTO idempotency_records ( project_id, idempotency_key, input_digest, artifact_id, version_id, - operation, access_setting, tags_json, created_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + operation, access_setting, tags_json, created_at, review_mode + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, (SELECT review_mode FROM artifacts WHERE id = ?))`, ) .run( projectId, @@ -3008,6 +3090,7 @@ export class SqliteArtifactRepository implements accessSetting, tagsJson, createdAt, + artifactId, ); } @@ -4892,6 +4975,7 @@ export class SqliteArtifactRepository implements this.#addIdempotencyOperationColumnsIfMissing(); this.#addTagIdempotencyOperationIfMissing(); this.#addProjectScopeIfMissing(installationId); + this.#addArtifactReviewModeIfMissing(); if (previousSchemaVersion < 12) { this.#addArtifactSearchNameIfMissing(); } @@ -4908,6 +4992,47 @@ export class SqliteArtifactRepository implements this.#database.exec(`PRAGMA user_version = ${requiredSqliteSchemaVersion};`); } + #addArtifactReviewModeIfMissing(): void { + const table = z.object({sql: z.string()}).parse(this.#database.prepare( + "SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 'idempotency_records'", + ).get()); + if (this.#tableColumns("artifacts").includes("review_mode") && + this.#tableColumns("idempotency_records").includes("review_mode") && + table.sql.includes("'change_review_mode'")) return; + this.#transaction(() => { + if (!this.#tableColumns("artifacts").includes("review_mode")) { + this.#database.exec("ALTER TABLE artifacts ADD COLUMN review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact'))"); + } + const savedMode = this.#tableColumns("idempotency_records").includes("review_mode") + ? "review_mode" : "'annotate'"; + const triggers = z.array(z.object({sql: z.string()})).parse( + this.#database.prepare("SELECT sql FROM sqlite_schema WHERE type = 'trigger' AND tbl_name = 'idempotency_records'").all(), + ); + this.#database.exec(` + CREATE TABLE idempotency_records_review_mode ( + project_id TEXT NOT NULL REFERENCES projects(id), + idempotency_key TEXT NOT NULL, + input_digest TEXT NOT NULL, + artifact_id TEXT NOT NULL REFERENCES artifacts(id), + version_id TEXT NOT NULL REFERENCES versions(id), + operation TEXT NOT NULL DEFAULT 'publish' CHECK (operation IN ('publish', 'restore', 'change_access', 'change_review_mode', 'change_tags', 'delete')), + access_setting TEXT CHECK (access_setting IS NULL OR access_setting IN ('account_required', 'public_link')), + tags_json TEXT, + created_at TEXT NOT NULL, + review_mode TEXT NOT NULL DEFAULT 'annotate' CHECK (review_mode IN ('annotate', 'interact')), + PRIMARY KEY (project_id, idempotency_key) + ) STRICT; + INSERT INTO idempotency_records_review_mode + SELECT project_id, idempotency_key, input_digest, artifact_id, version_id, + operation, access_setting, tags_json, created_at, ${savedMode} + FROM idempotency_records; + DROP TABLE idempotency_records; + ALTER TABLE idempotency_records_review_mode RENAME TO idempotency_records; + `); + for (const trigger of triggers) this.#database.exec(trigger.sql); + }); + } + #addArtifactSearchNameIfMissing(): void { if (!this.#tableColumns("artifacts").includes("search_name")) { this.#database.exec("ALTER TABLE artifacts ADD COLUMN search_name TEXT"); @@ -5585,7 +5710,7 @@ export class SqliteArtifactRepository implements a.id AS artifactId, a.project_id AS projectId, a.name AS artifactName, - a.access_setting AS accessSetting, + a.review_mode AS reviewMode, a.access_setting AS accessSetting, a.current_version_id AS currentVersionId, a.created_at AS artifactCreatedAt, a.deleted_at AS artifactDeletedAt, @@ -5605,6 +5730,7 @@ export class SqliteArtifactRepository implements const parsed = publishedRowSchema.parse(row); const artifact: ArtifactRecord = { accessSetting: parsed.accessSetting, + reviewMode: parsed.reviewMode, createdAt: parsed.artifactCreatedAt, currentVersionId: parsed.currentVersionId, deletedAt: parsed.artifactDeletedAt, diff --git a/src/storage/sqlite-schema.ts b/src/storage/sqlite-schema.ts index eca9c632..2854d995 100644 --- a/src/storage/sqlite-schema.ts +++ b/src/storage/sqlite-schema.ts @@ -6,7 +6,7 @@ import {z} from "zod"; const userVersionRowSchema = z.object({user_version: z.number().int().nonnegative()}); /** SQLite schema revision required by this Artifact Server build. */ -export const requiredSqliteSchemaVersion = 12; +export const requiredSqliteSchemaVersion = 13; /** Compact database compatibility observed without changing the file. */ export interface SqliteMigrationStatus { diff --git a/tests/browser/artifact-review-mode.spec.ts b/tests/browser/artifact-review-mode.spec.ts new file mode 100644 index 00000000..0f4979c8 --- /dev/null +++ b/tests/browser/artifact-review-mode.spec.ts @@ -0,0 +1,256 @@ +import {expect, test} from "@playwright/test"; + +import {publishNew, publishVersion, type PublishNewInput} from "../support/publishing.js"; +import { + localLogin, + startBrowserFixture, + stopBrowserFixture, +} from "./browser-fixture.js"; + +const interactiveArtifact = ` + +

waiting

+
Show details

Expanded detail

+ Follow link +

Destination

+`; + +test.describe("Artifact review mode", () => { + for (const scenario of [ + {defaultMode: "annotate", linkMode: "interact", expected: "interact"}, + {defaultMode: "interact", linkMode: "annotate", expected: "annotate"}, + {defaultMode: "interact", linkMode: null, expected: "interact"}, + {defaultMode: "interact", linkMode: "ANNOTATE", expected: "interact"}, + {defaultMode: undefined, linkMode: "invalid", expected: "annotate"}, + {defaultMode: undefined, linkMode: null, expected: "annotate"}, + ] as const) { + const primary = scenario.defaultMode === "annotate" && scenario.linkMode === "interact"; + test(`${primary ? "PRV-006-B PRV-006-F: " : ""}artifact review mode: ${scenario.defaultMode ?? "legacy"} default with ${scenario.linkMode ?? "no"} override starts ${scenario.expected}`, async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const input: PublishNewInput = { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "initial-review-mode", + name: "Interactive fixture", + }; + const published = await publishNew( + fixture.server, + fixture.installation, + scenario.defaultMode === undefined ? input : {...input, reviewMode: scenario.defaultMode}, + ); + await localLogin(fixture); + const url = normalReviewUrl(published.body.links.review); + if (scenario.linkMode !== null) url.searchParams.set("mode", scenario.linkMode); + await fixture.page.goto(url.href); + const review = fixture.page.frameLocator(".as-artifact-frame"); + const preview = review.frameLocator("iframe"); + await expect(fixture.page.getByRole("button", { + name: scenario.expected === "interact" ? /^Interact mode/u : /^Annotate mode/u, + })).toBeVisible(); + await preview.getByRole("button", {name: "Run action"}).click(); + if (scenario.expected === "interact") { + await expect(preview.locator("#count")).toHaveText("clicked"); + await preview.getByText("Show details", {exact: true}).click(); + await expect(preview.getByText("Expanded detail")).toBeVisible(); + await preview.getByRole("link", {name: "Follow link"}).click(); + await expect(preview.locator("#link-result")).toHaveText("followed"); + await expect(review.locator('[data-comment-popover="true"]')).toBeHidden(); + if (primary) { + url.searchParams.set("mode", "invalid"); + await fixture.page.goto(url.href); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + } + } else { + await expect(preview.locator("#count")).toHaveText("waiting"); + await expect(review.locator('[data-comment-popover="true"]')).toBeVisible(); + } + } finally { + await stopBrowserFixture(fixture); + } + }); + } + + test("artifact review mode: a link override survives focus navigation and browser history", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const published = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-navigation", + }); + await localLogin(fixture); + const url = normalReviewUrl(published.body.links.review); + url.searchParams.set("mode", "interact"); + await fixture.page.goto(url.href); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.getByRole("button", {name: "Full screen", exact: true}).click(); + await expect(fixture.page).toHaveURL(/mode=interact/u); + await expect.poll(() => fixture.page.evaluate(() => sessionStorage.getItem("artifact-review-return-url"))) + .toContain("mode=interact"); + await fixture.page.getByRole("button", {name: "Exit full screen", exact: true}).click(); + await fixture.page.goBack(); + await expect(fixture.page.getByRole("button", {name: "Interact mode", exact: true})).toBeVisible(); + await fixture.page.goForward(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await expect(fixture.page).toHaveURL(/mode=interact/u); + } finally { + await stopBrowserFixture(fixture); + } + }); + + test("artifact review mode: Escape and toggles survive version selection and refreshed artifact details", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const first = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-refresh-first", + }); + await publishVersion(fixture.server, fixture.installation, { + artifactId: first.body.artifact.id, + content: interactiveArtifact, + expectedCurrentVersionId: first.body.version.id, + idempotencyKey: "review-mode-refresh-second", + }); + await localLogin(fixture); + await fixture.page.goto(normalReviewUrl(first.body.links.review).href); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + await fixture.page.keyboard.press("Escape"); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.getByRole("tab", {name: /Versions/u}).click(); + const firstRow = fixture.page.locator(".as-version-list li").filter({hasText: "Version 1"}); + await firstRow.getByRole("button", {name: "Make current"}).click(); + await fixture.page.getByRole("dialog", {name: "Make Version 1 current?"}) + .getByRole("button", {name: "Make current"}).click(); + await expect(firstRow.getByText("current", {exact: true})).toBeVisible(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.getByRole("button", {name: /^Interact mode/u}).click(); + await fixture.page.locator(".as-version-list li").filter({hasText: "Version 2"}) + .getByRole("button", {name: /Version 2/u}).first().click(); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + } finally { + await stopBrowserFixture(fixture); + } + }); + + test("artifact review mode: browser history restores a link override after a manual toggle", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const published = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-history-toggle", + }); + await localLogin(fixture); + const url = normalReviewUrl(published.body.links.review); + url.searchParams.set("mode", "interact"); + await fixture.page.goto(url.href); + await fixture.page.getByRole("button", {name: "Full screen", exact: true}).click(); + await fixture.page.getByRole("button", {name: "Interact mode", exact: true}).click(); + await expect(fixture.page.getByRole("button", {name: "Annotate mode", exact: true})).toBeVisible(); + await fixture.page.goBack(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.goForward(); + await expect(fixture.page.getByRole("button", {name: "Interact mode", exact: true})).toBeVisible(); + } finally { + await stopBrowserFixture(fixture); + } + }); + + test("artifact review mode: mode-only history entries and removal use the restored URL", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const published = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-history-override-removal", + reviewMode: "interact", + }); + await localLogin(fixture); + const url = normalReviewUrl(published.body.links.review); + await fixture.page.goto(url.href); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + const annotate = new URL(url); + annotate.searchParams.set("mode", "annotate"); + const interact = new URL(url); + interact.searchParams.set("mode", "interact"); + await fixture.page.evaluate((locations) => { + for (const location of locations) window.history.pushState(null, "", location); + }, [annotate.href, interact.href]); + await fixture.page.goBack(); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + await fixture.page.goBack(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await expect.poll(() => new URL(fixture.page.url()).searchParams.has("mode")).toBe(false); + } finally { + await stopBrowserFixture(fixture); + } + }); + + test("artifact review mode: selecting another artifact applies its default", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + try { + const first = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-artifact-first", + name: "Annotate fixture", + }); + await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-artifact-second", + name: "Interact fixture", + reviewMode: "interact", + }); + await localLogin(fixture); + await fixture.page.goto(normalReviewUrl(first.body.links.review).href); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + await fixture.page.getByRole("button", {name: /Interact fixture/u}).click(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.getByRole("button", {name: /^Interact mode/u}).click(); + await fixture.page.getByRole("button", {name: /Annotate fixture/u}).click(); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode/u})).toBeVisible(); + } finally { + await stopBrowserFixture(fixture); + } + }); + + test("artifact review mode: a manual Escape before initial details arrives remains selected", async ({browser}) => { + const fixture = await startBrowserFixture(browser); + const requested = Promise.withResolvers(); + const release = Promise.withResolvers(); + try { + const published = await publishNew(fixture.server, fixture.installation, { + accessSetting: "account_required", + content: interactiveArtifact, + idempotencyKey: "review-mode-delayed-details", + }); + await localLogin(fixture); + await fixture.page.route(`**/api/v1/artifacts/${published.body.artifact.id}?*`, async (route) => { + requested.resolve(); + await release.promise; + await route.continue(); + }); + await fixture.page.goto(normalReviewUrl(published.body.links.review).href); + await requested.promise; + await fixture.page.keyboard.press("Escape"); + release.resolve(); + await expect(fixture.page.getByRole("button", {name: /^Interact mode/u})).toBeVisible(); + await fixture.page.frameLocator(".as-artifact-frame").frameLocator("iframe") + .getByRole("button", {name: "Run action"}).click(); + await expect(fixture.page.frameLocator(".as-artifact-frame").frameLocator("iframe").locator("#count")) + .toHaveText("clicked"); + } finally { + release.resolve(); + await stopBrowserFixture(fixture); + } + }); +}); + +function normalReviewUrl(href: string): URL { + const url = new URL(href); + url.searchParams.delete("view"); + return url; +} diff --git a/tests/cli/auth-profile-remote-publish.test.ts b/tests/cli/auth-profile-remote-publish.test.ts index ce0d47e3..5b420aab 100644 --- a/tests/cli/auth-profile-remote-publish.test.ts +++ b/tests/cli/auth-profile-remote-publish.test.ts @@ -234,11 +234,24 @@ describe("authenticated CLI profiles and remote publication", () => { "--profile-data", profileData, "--public", + "--review-mode", + "interact", ], environment, ); expect(firstPublication.exitCode).toBe(0); const firstResult = publicationSchema.parse(JSON.parse(firstPublication.stdout)); + expect(JSON.parse(firstPublication.stdout)).toMatchObject({artifact: {reviewMode: "interact"}}); + const invalidMode = await runCli(["publish", fixture, "--review-mode", "INTERACT"], environment); + expect(invalidMode.exitCode).not.toBe(0); + expect(invalidMode.stderr).toContain("Allowed choices are annotate, interact"); + const versionMode = await runCli([ + "publish", fixture, "--profile", "team", "--profile-data", profileData, + "--artifact", firstResult.artifact.id, "--expected-version", firstResult.version.id, + "--review-mode", "interact", + ], environment); + expect(versionMode.exitCode).not.toBe(0); + expect(versionMode.stderr).toContain("--review-mode apply only when creating a new artifact"); expect(await fetchLoopbackContent(firstResult.links.version).then((response) => response.text() )) diff --git a/tests/cli/mcp-onboarding.test.ts b/tests/cli/mcp-onboarding.test.ts index 1242e77d..741c7441 100644 --- a/tests/cli/mcp-onboarding.test.ts +++ b/tests/cli/mcp-onboarding.test.ts @@ -82,7 +82,7 @@ describe("local MCP onboarding", {timeout: testTimeoutMilliseconds}, () => { const first = await connectStdio(dataDirectory); const firstTools = await first.client.listTools(); expect(first.client.getNegotiatedProtocolVersion()).toBe(modernProtocolRevision); - expect(firstTools.tools).toHaveLength(33); + expect(firstTools.tools).toHaveLength(34); const capabilities = await first.client.callTool({ arguments: {}, name: "artifact_capabilities", @@ -104,11 +104,11 @@ describe("local MCP onboarding", {timeout: testTimeoutMilliseconds}, () => { )).toEqual({status: "ok"}); const second = await connectStdio(dataDirectory); - expect((await second.client.listTools()).tools).toHaveLength(33); + expect((await second.client.listTools()).tools).toHaveLength(34); await second.client.close(); const legacy = await connectStdio(dataDirectory, "legacy"); - expect((await legacy.client.listTools()).tools).toHaveLength(33); + expect((await legacy.client.listTools()).tools).toHaveLength(34); const legacyCapabilities = await legacy.client.callTool({ arguments: {}, name: "artifact_capabilities", @@ -124,7 +124,7 @@ describe("local MCP onboarding", {timeout: testTimeoutMilliseconds}, () => { const doctor = await runCli(["doctor", "--data", dataDirectory]); expect(doctor.exitCode).toBe(0); expect(doctorSchema.parse(JSON.parse(doctor.stdout))).toMatchObject({ - discovery: {tools: 33}, + discovery: {tools: 34}, status: "healthy", }); const apiCredential = (await readFile( diff --git a/tests/client/artifact-review-mode-publication.test.ts b/tests/client/artifact-review-mode-publication.test.ts new file mode 100644 index 00000000..dee6d012 --- /dev/null +++ b/tests/client/artifact-review-mode-publication.test.ts @@ -0,0 +1,41 @@ +import {mkdtemp, rm, writeFile} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import path from "node:path"; + +import * as NodeFileSystem from "@effect/platform-node-shared/NodeFileSystem"; +import {Effect, Redacted} from "effect"; +import * as FetchHttpClient from "effect/unstable/http/FetchHttpClient"; +import {expect, test} from "vitest"; + +import {publishPath} from "../../src/client/file-publication-client.js"; +import {createTestInstallation, removeTestInstallation, startTestServer} from "../support/runtime-harness.js"; + +test("file publication forwards the review default and preserves default-mode retries", async () => { + const installation = await createTestInstallation(); + const server = await startTestServer(installation); + const directory = await mkdtemp(path.join(tmpdir(), "artifact-review-mode-client-")); + try { + const inputPath = path.join(directory, "report.html"); + await writeFile(inputPath, ""); + const publish = (key: string, reviewMode?: "annotate" | "interact") => { + const target = {accessSetting: "account_required" as const, kind: "new_artifact" as const, tags: []}; + return Effect.runPromise(publishPath({ + apiToken: Redacted.make(installation.apiToken), serverOrigin: server.baseUrl, + }, { + idempotencyKey: key, inputPath, + target: reviewMode === undefined ? target : {...target, reviewMode}, + }).pipe(Effect.provide(FetchHttpClient.layer), Effect.provide(NodeFileSystem.layer))); + }; + const interact = await publish("client_review_interact_001", "interact"); + expect(interact.artifact.reviewMode).toBe("interact"); + const omitted = await publish("client_review_default_001"); + const explicit = await publish("client_review_default_001", "annotate"); + expect(omitted.artifact.reviewMode).toBe("annotate"); + expect(explicit.replayed).toBe(true); + expect(explicit.artifact.id).toBe(omitted.artifact.id); + } finally { + await server.stop(); + await removeTestInstallation(installation); + await rm(directory, {recursive: true, force: true}); + } +}); diff --git a/tests/conformance/artifact-review-mode-persistence.test.ts b/tests/conformance/artifact-review-mode-persistence.test.ts new file mode 100644 index 00000000..db690856 --- /dev/null +++ b/tests/conformance/artifact-review-mode-persistence.test.ts @@ -0,0 +1,139 @@ +import {mkdtemp, rm} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import path from "node:path"; +import {DatabaseSync} from "node:sqlite"; + +import {afterEach, beforeEach, describe, expect, test} from "vitest"; + +import {defaultProjectId, type ReviewMode} from "../../src/core/model.js"; +import type {CommitNewArtifact} from "../../src/core/ports.js"; +import {createManifest} from "../../src/manifest/create-manifest.js"; +import {SqliteArtifactRepository} from "../../src/storage/sqlite-artifact-repository.js"; + +describe("artifact review mode persistence", () => { + let directory: string; + let databasePath: string; + let repository: SqliteArtifactRepository; + const createdAt = "2026-10-11T00:00:00.000Z"; + const projectId = defaultProjectId; + const manifest = createManifest({ + entryPath: "index.html", + files: [{mediaType: "text/html", path: "index.html", sha256: "a".repeat(64), size: 1}], + routingMode: "static", + }); + + beforeEach(async () => { + directory = await mkdtemp(path.join(tmpdir(), "artifact-review-mode-")); + databasePath = path.join(directory, "artifact-server.db"); + repository = new SqliteArtifactRepository(databasePath); + }); + + afterEach(async () => { + repository.close(); + await rm(directory, {force: true, recursive: true}); + }); + + async function publish(reviewMode?: ReviewMode) { + await repository.createStagedUpload({ + createdAt, + expiresAt: "2026-10-12T00:00:00.000Z", + files: manifest.entries.map((entry) => ({entry, storageToken: "slot"})), + id: "upl_review", + manifest, + principalId: "publisher", + projectId, + }); + await repository.markStagedFileUploaded(projectId, "upl_review", "publisher", "slot", createdAt); + const defaultCommand: CommitNewArtifact = { + accessSetting: "public_link", artifactId: "art_review", authorizedByPrincipalId: null, + contentToken: "content_review", createdAt, idempotencyKey: "publish_review", + inputDigest: "publish_digest", manifest, name: "Review", principalId: "publisher", + projectId, + source: {kind: "staged_upload", principalId: "publisher", projectId, uploadId: "upl_review"}, + tags: [], versionId: "ver_review", + }; + const command = reviewMode === undefined ? defaultCommand : {...defaultCommand, reviewMode}; + return {command, result: await repository.commitNewArtifact(command)}; + } + + const change = (reviewMode: ReviewMode, idempotencyKey: string) => ({ + artifactId: "art_review", authorizedByPrincipalId: "administrator", createdAt, + expectedCurrentVersionId: "ver_review", idempotencyKey, inputDigest: idempotencyKey, + principalId: "publisher", projectId, reviewMode, + }); + + test("publication defaults to annotate and survives a restart", async () => { + const {result} = await publish(); + expect(result.artifact.reviewMode).toBe("annotate"); + repository.close(); + repository = new SqliteArtifactRepository(databasePath); + expect(await repository.findArtifact(projectId, "art_review")).toMatchObject({reviewMode: "annotate"}); + }); + + test("interact publication, mutation replay, and immutable version identity are durable", async () => { + const {command, result} = await publish("interact"); + const first = await repository.changeReviewMode(change("annotate", "mode_first")); + await repository.changeReviewMode(change("interact", "mode_second")); + repository.close(); + repository = new SqliteArtifactRepository(databasePath); + const replay = await repository.changeReviewMode(change("annotate", "mode_first")); + expect(replay).toEqual({...first, replayed: true}); + expect(replay.artifact.reviewMode).toBe("annotate"); + expect(replay.version).toEqual(result.version); + expect(await repository.findArtifact(projectId, "art_review")).toMatchObject({reviewMode: "interact"}); + expect((await repository.commitNewArtifact(command)).artifact.reviewMode).toBe("interact"); + const actions = await repository.listArtifactActions({artifactId: "art_review", cursor: null, limit: 20, projectId}); + expect(actions.items.filter((action) => action.action === "change_review_mode")).toHaveLength(2); + expect(actions.items.find((action) => action.idempotencyKey === "mode_first")).toMatchObject({authorizedByPrincipalId: "administrator"}); + await repository.changeReviewMode(change("annotate", "mode_third")); + expect((await repository.commitNewArtifact(command)).artifact.reviewMode).toBe("interact"); + expect(await repository.findArtifact(projectId, "art_review")).toMatchObject({reviewMode: "annotate"}); + }); + + test("versions, restores, tags, access changes, and list projections preserve the saved mode", async () => { + const {command, result} = await publish("interact"); + await repository.createStagedUpload({ + createdAt, expiresAt: "2026-10-12T00:00:00.000Z", files: manifest.entries.map((entry) => ({entry, storageToken: "second_slot"})), + id: "upl_second", manifest, principalId: "publisher", projectId, + }); + await repository.markStagedFileUploaded(projectId, "upl_second", "publisher", "second_slot", createdAt); + const second = await repository.commitVersion({ + ...command, contentToken: "content_second", expectedCurrentVersionId: result.version.id, + idempotencyKey: "publish_second", inputDigest: "publish_second", + source: {...command.source, uploadId: "upl_second"}, versionId: "ver_second", + }); + expect(second.artifact.reviewMode).toBe("interact"); + const management = {...change("interact", "management"), expectedCurrentVersionId: second.version.id}; + expect((await repository.changeTags({...management, tags: ["saved"]})).artifact.reviewMode).toBe("interact"); + expect((await repository.changeAccessSetting({...management, accessSetting: "account_required", idempotencyKey: "access", inputDigest: "access"})).artifact.reviewMode).toBe("interact"); + const restored = await repository.restoreVersion({...management, idempotencyKey: "restore", inputDigest: "restore", versionId: result.version.id}); + expect(restored.artifact.reviewMode).toBe("interact"); + const listed = await repository.listArtifacts({comments: "all", cursor: null, limit: 10, projectId, sort: "newest", tags: []}); + expect(listed.items).toEqual([expect.objectContaining({reviewMode: "interact", currentVersionId: result.version.id})]); + }); + + test("rejects stale versions, changed idempotent input, and invalid database values", async () => { + await publish(); + await expect(repository.changeReviewMode({...change("interact", "stale"), expectedCurrentVersionId: "ver_missing"})).rejects.toMatchObject({_tag: "ArtifactMutationConflict"}); + await repository.changeReviewMode(change("interact", "same_key")); + await expect(repository.changeReviewMode({...change("annotate", "same_key"), inputDigest: "other"})).rejects.toMatchObject({_tag: "IdempotencyConflict"}); + const database = new DatabaseSync(databasePath); + try { + expect(() => database.exec("UPDATE artifacts SET review_mode = 'invalid'")).toThrow(/CHECK constraint failed/u); + expect(() => database.exec("UPDATE idempotency_records SET review_mode = 'invalid'")).toThrow(/CHECK constraint failed/u); + } finally {database.close();} + }); + + test("pre-feature rows and saved retries migrate to annotate without changing versions", async () => { + const {command, result} = await publish(); + repository.close(); + const database = new DatabaseSync(databasePath); + try { + database.exec("ALTER TABLE artifacts DROP COLUMN review_mode; ALTER TABLE idempotency_records DROP COLUMN review_mode; PRAGMA user_version = 12;"); + } finally {database.close();} + repository = new SqliteArtifactRepository(databasePath); + expect(await repository.findArtifact(projectId, "art_review")).toMatchObject({reviewMode: "annotate", currentVersionId: result.version.id}); + expect(await repository.commitNewArtifact(command)).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true, version: result.version}); + await expect(repository.changeReviewMode(change("interact", "after_migration"))).resolves.toMatchObject({artifact: {reviewMode: "interact"}}); + }); +}); diff --git a/tests/conformance/artifact-review-mode.test.ts b/tests/conformance/artifact-review-mode.test.ts new file mode 100644 index 00000000..cdbea62a --- /dev/null +++ b/tests/conformance/artifact-review-mode.test.ts @@ -0,0 +1,122 @@ +import {afterEach, beforeEach, describe, expect, test} from "vitest"; +import {z} from "zod"; + +import { + apiHeaders, + createTestInstallation, + removeTestInstallation, + type RunningTestServer, + startTestServer, + type TestInstallation, +} from "../support/runtime-harness.js"; +import {publishNew, publishVersion} from "../support/publishing.js"; +import {issueApiKey, signInAdministrator} from "../support/agent-dispatch.js"; + +const stateSchema = z.object({ + artifact: z.object({id: z.string(), reviewMode: z.enum(["annotate", "interact"])}), + replayed: z.boolean(), + version: z.object({id: z.string()}), +}); + +describe("artifact review mode", () => { + let installation: TestInstallation; + let server: RunningTestServer; + beforeEach(async () => { + installation = await createTestInstallation(); + server = await startTestServer(installation); + }); + afterEach(async () => { + await server.stop(); + await removeTestInstallation(installation); + }); + + test("ART-012-B: publishing and updating the review default preserves saved version identity", async () => { + const legacy = await publishNew(server, installation, { + accessSetting: "account_required", content: "legacy", idempotencyKey: "artifact_review_mode_default", + }); + expect(legacy.body.artifact.reviewMode).toBe("annotate"); + const published = await publishNew(server, installation, { + accessSetting: "account_required", content: "", + idempotencyKey: "artifact_review_mode_interact", reviewMode: "interact", + }); + expect(published.body.artifact.reviewMode).toBe("interact"); + const url = `${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}/review-mode`; + const input = {expectedCurrentVersionId: published.body.version.id, reviewMode: "annotate"}; + const change = () => fetch(url, { + body: JSON.stringify(input), headers: apiHeaders(installation, "artifact_review_mode_update"), method: "PATCH", + }); + const response = await change(); + expect(response.status).toBe(200); + expect(stateSchema.parse(await response.json())).toMatchObject({ + artifact: {reviewMode: "annotate"}, replayed: false, version: {id: published.body.version.id}, + }); + expect(stateSchema.parse(await (await change()).json()).replayed).toBe(true); + const version = await publishVersion(server, installation, { + artifactId: published.body.artifact.id, content: "", + expectedCurrentVersionId: published.body.version.id, idempotencyKey: "artifact_review_mode_version", + }); + expect(version.body.artifact.reviewMode).toBe("annotate"); + await server.stop(); + server = await startTestServer(installation); + const details = await fetch(`${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}`, { + headers: apiHeaders(installation, "artifact_review_read_001"), + }); + expect(await details.json()).toMatchObject({artifact: {reviewMode: "annotate"}}); + }); + + test("ART-012-F: invalid, stale, and unauthenticated mode mutations leave the artifact unchanged", async () => { + const published = await publishNew(server, installation, { + accessSetting: "account_required", content: "mode rejection", idempotencyKey: "artifact_review_mode_reject", + }); + const url = `${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}/review-mode`; + await Promise.all(([ + ["artifact_review_invalid_mode", "INTERACT", published.body.version.id, 422], + ["artifact_review_stale_mode", "interact", "ver_stale", 409], + ] as const).map(async ([key, mode, versionId, expected]) => { + const response = await fetch(url, { + body: JSON.stringify({expectedCurrentVersionId: versionId, reviewMode: mode}), + headers: apiHeaders(installation, key), method: "PATCH", + }); + expect(response.status).toBe(expected); + })); + const denied = await fetch(url, { + body: JSON.stringify({expectedCurrentVersionId: published.body.version.id, reviewMode: "interact"}), + headers: {"Content-Type": "application/json"}, method: "PATCH", + }); + expect(denied.status).toBe(401); + const details = await fetch(`${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}`, { + headers: apiHeaders(installation, "artifact_review_read_002"), + }); + expect(await details.json()).toMatchObject({artifact: {reviewMode: "annotate"}}); + }); + + test("a conflicting publication retry cannot change the original review default", async () => { + const input = {accessSetting: "account_required" as const, content: "same bytes", idempotencyKey: "artifact_review_mode_retry"}; + const first = await publishNew(server, installation, input); + const explicit = await publishNew(server, installation, {...input, reviewMode: "annotate"}); + expect(explicit.body.replayed).toBe(true); + expect(explicit.body.artifact.id).toBe(first.body.artifact.id); + await expect(publishNew(server, installation, {...input, reviewMode: "interact"})) + .rejects.toThrow("HTTP 409"); + }); + + test("a publish-only credential cannot change the artifact's review default", async () => { + const published = await publishNew(server, installation, { + accessSetting: "account_required", content: "scoped credential", idempotencyKey: "review_mode_scoped_001", + }); + const administrator = await signInAdministrator(server, installation); + const token = await issueApiKey(server, administrator, ["artifact:publish:any"], "Publisher only"); + const denied = await fetch(`${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}/review-mode`, { + body: JSON.stringify({expectedCurrentVersionId: published.body.version.id, reviewMode: "interact"}), + headers: { + Authorization: `Bearer ${token}`, "Content-Type": "application/json", "Idempotency-Key": "review_mode_scoped_denied", + }, + method: "PATCH", + }); + expect(denied.status).toBe(403); + const details = await fetch(`${server.baseUrl}/api/v1/artifacts/${published.body.artifact.id}`, { + headers: apiHeaders(installation, "review_mode_scoped_read"), + }); + expect(await details.json()).toMatchObject({artifact: {reviewMode: "annotate"}}); + }); +}); diff --git a/tests/conformance/authorization-policy.test.ts b/tests/conformance/authorization-policy.test.ts index dc787835..f94be7f4 100644 --- a/tests/conformance/authorization-policy.test.ts +++ b/tests/conformance/authorization-policy.test.ts @@ -188,6 +188,7 @@ function artifactFixture(): ArtifactRecord { id: "artifact-1", name: "Authorization fixture", projectId: "prj_default", + reviewMode: "annotate", tags: [], }; } diff --git a/tests/conformance/mcp-modern-http.test.ts b/tests/conformance/mcp-modern-http.test.ts index 14407021..bb446cd9 100644 --- a/tests/conformance/mcp-modern-http.test.ts +++ b/tests/conformance/mcp-modern-http.test.ts @@ -202,6 +202,7 @@ describe("modern MCP HTTP", () => { "artifact_create_upload", "artifact_commit_upload", "artifact_set_visibility", + "artifact_set_review_mode", "artifact_set_tags", "artifact_restore_version", "artifact_delete", @@ -416,6 +417,7 @@ describe("modern MCP HTTP", () => { accessSetting: "account_required", kind: "new_artifact", name: "MCP publication proof", + reviewMode: "interact", tags: ["MCP", "Proof"], }, uploadId: upload.uploadId, @@ -447,6 +449,26 @@ describe("modern MCP HTTP", () => { expect(replay.replayed).toBe(true); expect(replay.version.id).toBe(committed.version.id); + const modeArguments = { + artifactId: committed.artifact.id, + expectedCurrentVersionId: committed.version.id, + idempotencyKey: "mcp-review-mode-update-001", + reviewMode: "annotate", + }; + expect(commitResult.structuredContent).toMatchObject({artifact: {reviewMode: "interact"}}); + const changedMode = await callTool(server, installation.apiToken, { + arguments: modeArguments, + name: "artifact_set_review_mode", + }); + expect(changedMode.isError).not.toBe(true); + expect(changedMode.structuredContent).toMatchObject({ + artifact: {reviewMode: "annotate"}, replayed: false, version: {id: committed.version.id}, + }); + expect((await callTool(server, installation.apiToken, { + arguments: modeArguments, + name: "artifact_set_review_mode", + })).structuredContent).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true}); + const detailsResult = await callTool(server, installation.apiToken, { arguments: {artifactId: committed.artifact.id}, name: "artifact_get", diff --git a/tests/integration/external-storage-runtime.test.ts b/tests/integration/external-storage-runtime.test.ts index e6f3fd25..ae0b8e63 100644 --- a/tests/integration/external-storage-runtime.test.ts +++ b/tests/integration/external-storage-runtime.test.ts @@ -330,6 +330,102 @@ describe.sequential("external-storage Postgres and S3 runtime", () => { } }); + test("ART-012: populated PostgreSQL schema upgrades artifacts and historical retries to annotate", async () => { + const identity = {apiToken: managedTestKey("postgres-review-upgrade"), installationId: "postgres-review-upgrade"}; + let server = await startInProcessExternalStorageServer(environment, identity); + const content = "

Pre-feature PostgreSQL artifact

"; + const upload = await stageExternalStorageFile(server.baseUrl, identity.apiToken, content); + const publicationInput = {target: {accessSetting: "public_link", kind: "new_artifact", name: "Older PostgreSQL artifact"}}; + const publish = () => fetch(new URL(new URL(upload.commitUrl).pathname, server.baseUrl), { + body: JSON.stringify(publicationInput), + headers: mutationHeaders(identity.apiToken, "postgres-review-mode-upgrade"), + method: "POST", + }); + const committed = await publish(); + expect(committed.status).toBe(201); + const publication = publishResponseSchema.parse(await committed.json()); + await server.stop(); + const database = await PostgresDatabase.inspect({ + applicationName: "artifact-server-review-mode-upgrade-test", + maxConnections: 1, + url: Redacted.make(environment.databaseUrl, {label: "test-database-url"}), + }); + try { + await database.run(Effect.gen(function*() { + const sql = yield* SqlClient; + yield* sql.withTransaction(Effect.gen(function*() { + yield* sql.unsafe("ALTER TABLE artifacts DROP COLUMN review_mode"); + yield* sql.unsafe("ALTER TABLE idempotency_records DROP COLUMN review_mode"); + yield* sql.unsafe(`ALTER TABLE idempotency_records + DROP CONSTRAINT idempotency_records_operation_check, + ADD CONSTRAINT idempotency_records_operation_check CHECK (operation IN ( + 'publish', 'restore', 'change_access', 'change_tags', 'delete' + ))`); + yield* sql.unsafe(`ALTER TABLE actions + DROP CONSTRAINT actions_action_check, + ADD CONSTRAINT actions_action_check CHECK (action IN ( + 'publish', 'restore', 'change_access', 'change_tags', 'delete', + 'comment_create', 'comment_reply', 'comment_update', 'comment_resolve', + 'comment_reopen', 'comment_delete' + ))`); + yield* sql.unsafe("DELETE FROM artifact_server_postgres_migrations WHERE migration_id = 12"); + })); + })); + } finally {await database.close();} + const applied = await runExternalCli(["migrate", "apply"], { + ARTIFACT_SERVER_DATABASE_URL: environment.databaseUrl, + ARTIFACT_SERVER_INSTALLATION_ID: identity.installationId, + }); + expect(applied.exitCode).toBe(0); + server = await startInProcessExternalStorageServer(environment, identity); + const replay = await publish(); + expect(replay.status).toBe(200); + expect(await replay.json()).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true, version: publication.version}); + expect(await (await fetchContentThroughServer(server.baseUrl, publication.links.version)).text()).toBe(content); + }); + + test("ART-012: PostgreSQL review mode persists across replicas and replays its original state", async () => { + const identity = {apiToken: managedTestKey("postgres-review-mode"), installationId: "postgres-review-mode"}; + let firstServer = await startInProcessExternalStorageServer(environment, identity); + const secondServer = await startInProcessExternalStorageServer(environment, identity); + const content = ""; + const upload = await stageExternalStorageFile(firstServer.baseUrl, identity.apiToken, content); + const committed = await fetch(upload.commitUrl, { + body: JSON.stringify({target: { + accessSetting: "public_link", kind: "new_artifact", name: "Review mode", reviewMode: "interact", + }}), + headers: mutationHeaders(identity.apiToken, "postgres-review-mode-publish"), + method: "POST", + }); + expect(committed.status).toBe(201); + const publication = publishResponseSchema.extend({artifact: z.object({id: z.string(), reviewMode: z.enum(["annotate", "interact"])})}) + .parse(await committed.json()); + expect(publication.artifact.reviewMode).toBe("interact"); + const changeMode = (baseUrl: string, reviewMode: "annotate" | "interact", key: string) => mutateArtifact( + baseUrl, identity.apiToken, `/api/v1/artifacts/${publication.artifact.id}/review-mode`, "PATCH", key, + {expectedCurrentVersionId: publication.version.id, reviewMode}, + ); + const firstChanges = await Promise.all([ + changeMode(firstServer.baseUrl, "annotate", "postgres-review-mode-first"), + changeMode(secondServer.baseUrl, "annotate", "postgres-review-mode-first"), + ]); + expect(firstChanges.map(({status}) => status)).toEqual([200, 200]); + expect((await changeMode(secondServer.baseUrl, "interact", "postgres-review-mode-second")).status).toBe(200); + await firstServer.stop(); + firstServer = await startInProcessExternalStorageServer(environment, identity); + const replay = await changeMode(firstServer.baseUrl, "annotate", "postgres-review-mode-first"); + expect(replay.status).toBe(200); + expect(await replay.json()).toMatchObject({artifact: {reviewMode: "annotate"}, replayed: true, version: publication.version}); + const details = await authenticatedFetch(firstServer.baseUrl, identity.apiToken, `/api/v1/artifacts/${publication.artifact.id}`); + expect(await details.json()).toMatchObject({artifact: {reviewMode: "interact"}, current: {version: publication.version}}); + const savedContent = await fetchContentThroughServer(firstServer.baseUrl, publication.links.version); + expect(savedContent.status).toBe(200); + expect(await savedContent.text()).toBe(content); + const actions = await authenticatedFetch(firstServer.baseUrl, identity.apiToken, `/api/v1/artifacts/${publication.artifact.id}/actions`); + expect(z.object({actions: z.array(z.object({action: z.string()}))}).parse(await actions.json()) + .actions.map(({action}) => action).toSorted()).toEqual(["change_review_mode", "change_review_mode", "publish"]); + }); + test("GIT-010 GIT-013 foundation: Postgres enables Git history and preserves provider identity", async () => { const identity = { apiToken: managedTestKey("postgres-git-history"), @@ -3033,6 +3129,7 @@ function patchCommentThread( } type ArtifactMutationBody = + | {readonly expectedCurrentVersionId: string; readonly reviewMode: "annotate" | "interact"} | {readonly expectedCurrentVersionId: string} | { readonly accessSetting: "account_required" | "public_link"; diff --git a/tests/support/publishing.ts b/tests/support/publishing.ts index a443f465..63650008 100644 --- a/tests/support/publishing.ts +++ b/tests/support/publishing.ts @@ -16,6 +16,7 @@ const publishResponseSchema = z.object({ id: z.string(), name: z.string(), projectId: z.string(), + reviewMode: z.enum(["annotate", "interact"]), tags: z.array(z.string()), }), links: z.object({artifact: z.url(), review: z.url(), version: z.url()}), @@ -70,6 +71,7 @@ export interface PublishNewInput { readonly name?: string; readonly path?: string; readonly projectId?: string; + readonly reviewMode?: "annotate" | "interact"; readonly tags?: readonly string[]; } @@ -99,16 +101,17 @@ export async function publishNew( await requireSuccessfulUploads( uploadEveryStagedFile(installation, upload.body, [file]), ); + const target = { + accessSetting: input.accessSetting, + kind: "new_artifact" as const, + name: input.name ?? "Test artifact", + tags: [...(input.tags ?? [])], + }; return commitStagedUpload( installation, upload.body, input.idempotencyKey, - { - accessSetting: input.accessSetting, - kind: "new_artifact", - name: input.name ?? "Test artifact", - tags: [...(input.tags ?? [])], - }, + input.reviewMode === undefined ? target : {...target, reviewMode: input.reviewMode}, ); } @@ -228,6 +231,7 @@ const commitTargetSchema = z.discriminatedUnion("kind", [ accessSetting: z.enum(["account_required", "public_link"]), kind: z.literal("new_artifact"), name: z.string(), + reviewMode: z.enum(["annotate", "interact"]).optional(), tags: z.array(z.string()).optional(), }), z.object({ From d7113547608ec1becc5221b0010f326550ec3171 Mon Sep 17 00:00:00 2001 From: "Juan Carlos Moreno [ackstorm]" Date: Sat, 10 Oct 2026 07:14:34 +0200 Subject: [PATCH 2/2] test(review): align existing fixtures with artifact mode defaults --- tests/browser/frontend-mvp.spec.ts | 3 ++- tests/integration/external-storage-runtime.test.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/browser/frontend-mvp.spec.ts b/tests/browser/frontend-mvp.spec.ts index 9d50bb86..ac76c8bd 100644 --- a/tests/browser/frontend-mvp.spec.ts +++ b/tests/browser/frontend-mvp.spec.ts @@ -605,7 +605,8 @@ test.describe("Artifact Server frontend MVP", () => { .analyze(); expect(accessibility.violations).toEqual([]); - await fixture.page.getByRole("button", {name: /^Interact mode:/u}).click(); + await expect(fixture.page.getByRole("button", {name: /^Annotate mode:/u})) + .toHaveAttribute("aria-pressed", "true"); await preview.locator("#review-theme-target").click(); const themedCommentPopover = reviewFrame.locator('[data-comment-popover="true"]'); await expect(themedCommentPopover).toBeVisible(); diff --git a/tests/integration/external-storage-runtime.test.ts b/tests/integration/external-storage-runtime.test.ts index ae0b8e63..7402d4e8 100644 --- a/tests/integration/external-storage-runtime.test.ts +++ b/tests/integration/external-storage-runtime.test.ts @@ -612,8 +612,10 @@ describe.sequential("external-storage Postgres and S3 runtime", () => { "ALTER TABLE staged_uploads DROP COLUMN project_id CASCADE", "ALTER TABLE actions DROP COLUMN project_id CASCADE", "ALTER TABLE idempotency_records DROP COLUMN project_id CASCADE", + "ALTER TABLE idempotency_records DROP COLUMN review_mode", "ALTER TABLE versions DROP COLUMN project_id CASCADE", "ALTER TABLE artifacts DROP COLUMN search_name", + "ALTER TABLE artifacts DROP COLUMN review_mode", "ALTER TABLE artifacts DROP COLUMN project_id CASCADE", "ALTER TABLE login_attempts DROP COLUMN nonce", "DROP TABLE comment_replies",