diff --git a/public/app.js b/public/app.js index 697efb8..f3f8c54 100644 --- a/public/app.js +++ b/public/app.js @@ -1741,6 +1741,18 @@ function persistDiffPreferences() { .catch((error) => toast(error.message)); } +function persistDiffRenderer(diffRenderer) { + state.diffPreferenceSave = (state.diffPreferenceSave ?? Promise.resolve()) + .catch(() => undefined) + .then(async () => { + const session = await api("/api/preferences", { + method: "POST", body: JSON.stringify({ diffRenderer }), + }); + if (state.session) state.session.diffRenderer = session.diffRenderer; + }) + .catch((error) => toast(`Could not save renderer preference: ${error.message}`)); +} + function diffViewRow(path, side, line) { return elements.diff.querySelector(`.diff-row[data-path="${CSS.escape(path)}"][data-side="${side}"][data-line="${line}"]`) ?? elements.diff.querySelector(`.diff-row[data-path="${CSS.escape(path)}"][data-${side}-line="${line}"]`); @@ -5600,10 +5612,10 @@ function isInPlaceSessionUpdate(previous, latest) { // Explanations and comments can be updated without replacing the diff. // Only skip the full render when everything else is unchanged, including // the route, selected stop, diff, and review lifecycle. - // Layout/wrap are applied immediately by changeDiffView. A later polling + // Renderer/layout/wrap are applied immediately by their controls. A later polling // acknowledgement of their saved preferences must not rebuild the diff, // replace comment DOM or steal the reader's position. - const structuralSnapshot = ({ revision, agentActivity, activities, comments, messages, outline, pullRequest, diffPreferences, ...session }) => ({ + const structuralSnapshot = ({ revision, agentActivity, activities, comments, messages, outline, pullRequest, diffPreferences, diffRenderer, ...session }) => ({ ...session, pullRequest: pullRequest ? (({ threads, threadsTruncated, updatedAt, ...metadata }) => metadata)(pullRequest) : pullRequest, outline: outline ? { @@ -5626,7 +5638,7 @@ function isSamePhaseNavigationUpdate(previous, latest, { acrossPhases = false } const previousPhase = previous.outline.phases.find((phase) => phase.steps.some((step) => step.id === previousStep?.id)); const latestPhase = latest.outline.phases.find((phase) => phase.steps.some((step) => step.id === latestStep?.id)); if (!previousPhase || !latestPhase || (!acrossPhases && previousPhase.id !== latestPhase.id)) return false; - const navigationSnapshot = ({ revision, agentActivity, currentStepIndex, outline, diffPreferences, ...session }) => ({ + const navigationSnapshot = ({ revision, agentActivity, currentStepIndex, outline, diffPreferences, diffRenderer, ...session }) => ({ ...session, outline: outline ? { ...outline, @@ -5695,6 +5707,7 @@ function renderSessionUpdate(previous, options) { } function renderSession() { + state.monacoRenderer?.restorePreference(state.session.diffRenderer); getBriefingReader().render(state.session); const { comparison, comments } = state.session; syncDryRunSourceCache(); @@ -8388,6 +8401,7 @@ state.monacoRenderer = createMonacoRenderer({ actions: { notify: toast, tourShortcut: handleTourShortcut, viewportChanged: scheduleCurrentStepReturnUpdate, reserveScroll: reserveDiffScrollPosition, + saveRendererPreference: persistDiffRenderer, review: setReviewed, fileSummaryControl: file => globalThis.DiffCompassSemantic?.control(file, false) ?? "", async suggestionCreated(result, path) { diff --git a/public/monaco-controller.mjs b/public/monaco-controller.mjs index 27d6c30..e1c3bad 100644 --- a/public/monaco-controller.mjs +++ b/public/monaco-controller.mjs @@ -6,12 +6,15 @@ import { panReviewScroll } from "./monaco/pen-scroll.mjs"; export function createMonacoRenderer({ container, controls, token, snapshot, render, actions }) { const preferenceKey = "diffcompass-diff-renderer"; - let preferred = "current"; - try { if (localStorage.getItem(preferenceKey) === "monaco") preferred = "monaco"; } catch {} + let browserPreference; + try { browserPreference = localStorage.getItem(preferenceKey); } catch {} + let preferred = browserPreference === "monaco" ? "monaco" : "current"; + let preferenceRestored = false; const label = document.createElement("label"); label.textContent = "Renderer "; const select = document.createElement("select"); select.id = "diff-renderer"; select.setAttribute("aria-label", "Diff renderer"); // Keep the persisted value "current" compatible with existing installations. select.add(new Option("Shiki", "current")); select.add(new Option("Monaco", "monaco")); select.value = preferred; + select.disabled = true; // Restore the saved reviewer preference before accepting changes. label.append(select); controls.append(label); const region = document.createElement("section"); region.className = "monaco-review-region"; region.hidden = true; const notice = document.createElement("p"); notice.className = "monaco-review-notice"; notice.setAttribute("role", "status"); @@ -355,6 +358,7 @@ export function createMonacoRenderer({ container, controls, token, snapshot, ren cancelPhase(); preferred = select.value; failed = false; pendingFailure = null; try { localStorage.setItem(preferenceKey, preferred); } catch {} + actions.saveRendererPreference(preferred); if (preferred === "current") closeAll(); render(); }; @@ -449,6 +453,19 @@ export function createMonacoRenderer({ container, controls, token, snapshot, ren } const controller = { owns, sync, hasDraft, setCommentsVisible, preservePhasePosition, + restorePreference(saved) { + if (preferenceRestored) return; + preferenceRestored = true; + if (["current", "monaco"].includes(saved)) { + preferred = saved; + try { localStorage.setItem(preferenceKey, preferred); } catch {} + } else if (["current", "monaco"].includes(browserPreference)) { + // Migrate the old port-scoped choice only when no durable choice exists. + actions.saveRendererPreference(preferred); + } + select.value = preferred; + select.disabled = false; + }, updateMobileActions: mobileActions.update, cancelPhase, async preparePhase(session) { diff --git a/scripts/check-monaco.ts b/scripts/check-monaco.ts index 44a3c7c..4fb0f30 100644 --- a/scripts/check-monaco.ts +++ b/scripts/check-monaco.ts @@ -23,7 +23,7 @@ try { } const groups = { briefing: { samplePr: false, scripts: ["briefing-entry", "briefing-source", "briefing", "briefing-code", "briefing-diagrams"] }, - local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "monaco-gutter", "monaco-wrapping", "markdown-preview", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] }, + local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "renderer-preference", "monaco-gutter", "monaco-wrapping", "markdown-preview", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] }, navigation: { samplePr: true, scripts: ["monaco-phase", "monaco-phase-scroll", "monaco-upward-tour", "monaco-reviewed-scroll", "monaco-sticky", "monaco-file-explorer"] }, discussions: { samplePr: true, scripts: ["renderer-comments", "monaco-context", "monaco-edits", "monaco-edits-syntax", "monaco-discussions"] }, before: { samplePr: true, beforeTour: true, scripts: ["monaco-before-tour"] }, diff --git a/scripts/renderer-preference-browser.mjs b/scripts/renderer-preference-browser.mjs new file mode 100644 index 0000000..2da4bac --- /dev/null +++ b/scripts/renderer-preference-browser.mjs @@ -0,0 +1,82 @@ +// Renderer choice must survive a new server/port, where localStorage is empty. +import assert from 'node:assert/strict'; +import { copyFile } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { ReviewSession } from '../src/session.ts'; +import { startServer } from '../src/server.ts'; + +const [playwrightPath, url, repository] = process.argv.slice(2); +const { chromium } = await import(pathToFileURL(playwrightPath).href); +const browser = await chromium.launch({ channel: process.env.CI ? undefined : 'chrome', headless: true }); +const context = await browser.newContext({ viewport: { width: 1600, height: 1000 }, reducedMotion: 'reduce' }); +const page = await context.newPage(); +page.setDefaultTimeout(20000); +const errors = []; page.on('pageerror', error => errors.push(error.message)); +let session, server; +const key = 'diffcompass-diff-renderer'; +const saved = value => page.waitForResponse(response => response.url().endsWith('/api/preferences') + && response.request().postDataJSON()?.diffRenderer === value && response.ok()); +const monacoReady = () => page.waitForFunction(() => [...document.querySelectorAll('.monaco-review-frame')].some(frame => + frame.dataset.monacoPath === 'src/cart.ts' && frame.contentWindow?.diffLab?.views.get('src/cart.ts')?.diff.getLineChanges())); +try { + // Upgrade an existing browser-only preference without another manual switch. + await context.addInitScript(({ key, origin }) => { + if (location.origin === origin) localStorage.setItem(key, 'monaco'); + window.initialRendererPreference = localStorage.getItem(key); + }, { key, origin: new URL(url).origin }); + const migrated = saved('monaco'); + await page.goto(url); + await migrated; await monacoReady(); + + // Resume a separate review using the shared settings but a different port. + const directory = dirname(repository); + const stateFile = join(directory, 'renderer-session.json'); + await copyFile(join(directory, 'session.json'), stateFile); + session = await ReviewSession.create(repository, undefined, { + stateFile, resume: true, agentAdapters: [], + diffPreferenceDirectory: join(dirname(directory), 'preferences'), + }); + server = startServer(session, 0, directory); + const nextUrl = `http://127.0.0.1:${server.port}/#token=${session.browserToken}`; + assert.notEqual(new URL(nextUrl).origin, new URL(url).origin); + await page.goto(nextUrl); await monacoReady(); + assert.equal(await page.evaluate(() => window.initialRendererPreference), null, 'New port starts without a browser preference'); + assert.equal(await page.locator('#diff-renderer').inputValue(), 'monaco'); + await page.locator('#briefing-tab').click(); + const source = page.locator('#briefing-view [data-evidence-id="subtotal-body"]'); + await source.scrollIntoViewIfNeeded(); + await source.locator('.briefing-code-frame:not([hidden])').waitFor(); + + // Server settings win over stale storage, and also work with storage blocked. + await page.evaluate(key => localStorage.setItem(key, 'current'), key); + await page.reload(); await monacoReady(); + assert.equal(await page.locator('#diff-renderer').inputValue(), 'monaco'); + await page.close(); + // Use a fresh context so an explicit Shiki selection cannot rely on any cache. + const clean = await browser.newPage({ viewport: { width: 1600, height: 1000 }, reducedMotion: 'reduce' }); + clean.setDefaultTimeout(20000); + clean.on('pageerror', error => errors.push(error.message)); + await clean.addInitScript(key => { + const get = Storage.prototype.getItem, set = Storage.prototype.setItem; + Storage.prototype.getItem = function(name) { if (name === key) throw new Error('Storage unavailable'); return get.call(this, name); }; + Storage.prototype.setItem = function(name, value) { if (name === key) throw new Error('Storage unavailable'); return set.call(this, name, value); }; + }, key); + await clean.route('**/api/session', route => route.fulfill({ status: 503, json: { error: 'Temporary startup failure' } }), { times: 1 }); + await clean.goto(nextUrl); + await clean.waitForFunction(() => !document.getElementById('diff-renderer')?.disabled); + assert.equal(await clean.locator('#diff-renderer').inputValue(), 'monaco'); + const shikiSaved = clean.waitForResponse(response => response.url().endsWith('/api/preferences') + && response.request().postDataJSON()?.diffRenderer === 'current' && response.ok()); + await clean.locator('#diff-renderer').selectOption('current'); await shikiSaved; + await clean.reload(); + await clean.waitForFunction(() => !document.getElementById('diff-renderer')?.disabled); + assert.equal(await clean.locator('#diff-renderer').inputValue(), 'current'); + assert.equal(await clean.locator('.monaco-review-frame').count(), 0); + assert.deepEqual(errors, []); + console.log('PASS renderer migration, new review/port, Briefing inheritance, stale storage precedence, startup retry, and Shiki persistence without browser storage'); +} finally { + await browser.close(); + await server?.stop(true); + await session?.dispose(); +} diff --git a/src/diff-preferences.ts b/src/diff-preferences.ts index c83a0fa..2f06db0 100644 --- a/src/diff-preferences.ts +++ b/src/diff-preferences.ts @@ -1,12 +1,13 @@ import { chmod, mkdir, readFile, rename, writeFile } from "node:fs/promises"; import { dirname, join, resolve } from "node:path"; import { acquireSessionLock } from "./session-library.ts"; -import { isBriefingSectionOrder, type BriefingPreferences, type DiffViewPreferences, type SessionDiffPreferences } from "./types.ts"; +import { isBriefingSectionOrder, isDiffRenderer, type BriefingPreferences, type DiffRenderer, type DiffViewPreferences, type SessionDiffPreferences } from "./types.ts"; interface DiffPreferenceStore { schemaVersion: 1; repositories: Record; briefing?: BriefingPreferences; + diffRenderer?: DiffRenderer; } const STORE_NAME = ".diff-preferences.json"; @@ -57,6 +58,7 @@ async function loadStore(directory: string): Promise { schemaVersion: 1, repositories, briefing: validBriefingPreferences(value.briefing) ? structuredClone(value.briefing) : undefined, + diffRenderer: isDiffRenderer(value.diffRenderer) ? value.diffRenderer : undefined, }; } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return { schemaVersion: 1, repositories: {} }; @@ -107,6 +109,20 @@ export async function loadGlobalBriefingPreferences(directory: string | undefine return store.briefing ? structuredClone(store.briefing) : undefined; } +export async function loadGlobalDiffRenderer(directory: string | undefined): Promise { + return directory ? (await loadStore(directory)).diffRenderer : undefined; +} + +export async function updateGlobalDiffRenderer(directory: string | undefined, renderer: DiffRenderer | undefined): Promise { + if (!directory) return undefined; + return updateStore(directory, (store) => { + const previous = store.diffRenderer; + if (renderer === undefined) delete store.diffRenderer; + else store.diffRenderer = renderer; + return previous; + }); +} + export async function updateGlobalBriefingPreferences(directory: string | undefined, preferences: BriefingPreferences): Promise { if (!directory) return undefined; return updateStore(directory, (store) => { diff --git a/src/server.ts b/src/server.ts index da5fee0..3952552 100644 --- a/src/server.ts +++ b/src/server.ts @@ -50,7 +50,7 @@ import { resolveSymbolPeek } from "./symbol-peek.ts"; import { ReviewSession } from "./session.ts"; import { deleteReviewSession, listReviewSessions, resolveReviewSession, reviewSessionDirectory, updateReviewSessionMetadata } from "./session-library.ts"; import { createUpdateService, type UpdateService } from "./update.ts"; -import { isBriefingSectionOrder, type BriefingPreferences } from "./types.ts"; +import { isBriefingSectionOrder, isDiffRenderer, type BriefingPreferences } from "./types.ts"; import { enumValue, exactKeys, @@ -283,7 +283,13 @@ export function createRequestHandler(session: ReviewSession, sessionDirectory = } if (request.method === "POST" && url.pathname === "/api/preferences") { const input = await readJsonObject(request); - exactKeys(input, ["prefetchSteps", "diffLayout", "diffWrap", "mobile", "showBriefing", "briefingArchitecture", "briefingSequenceDiagrams", "briefingCallFlows", "briefingNeedsAttention", "briefingReviewEffort", "briefingOrder"]); + exactKeys(input, ["prefetchSteps", "diffLayout", "diffWrap", "diffRenderer", "mobile", "showBriefing", "briefingArchitecture", "briefingSequenceDiagrams", "briefingCallFlows", "briefingNeedsAttention", "briefingReviewEffort", "briefingOrder"]); + if ("diffRenderer" in input) { + if (Object.keys(input).length !== 1 || !isDiffRenderer(input.diffRenderer)) { + throw new HttpError(422, "Diff renderer must be current or monaco."); + } + return json(await session.setDiffRenderer(input.diffRenderer)); + } if (input.prefetchSteps !== undefined) { if (Object.keys(input).length !== 1 || !Number.isInteger(input.prefetchSteps) || Number(input.prefetchSteps) < 0 || Number(input.prefetchSteps) > 5) { throw new HttpError(422, "Prepare ahead must be between 0 and 5 steps."); diff --git a/src/session.ts b/src/session.ts index cfdcff0..189e705 100644 --- a/src/session.ts +++ b/src/session.ts @@ -19,12 +19,12 @@ import { extractAcceptanceCriteria, publishPullRequestReview as publishGitHubRev import { runProcess } from "./process.ts"; import { loadPersistedSession, savePersistedSession } from "./store.ts"; import { tourSteps } from "./tour.ts"; -import type { FindingDisposition } from "./types.ts"; +import type { DiffRenderer, FindingDisposition } from "./types.ts"; import type { AgentAdapterOption, AgentChangeActivity, AgentChangeFile, AgentRunActivity, AgentSelection, BriefingPreferences, ChatMessage, ComparisonView, DiffFile, DiffRow, DiffViewPreferences, DryRunDetail, DryRunFlowKind, DryRunPlan, DryRunScenario, DryRunTrace, GitComparison, PersistedReviewSession, PullRequestContext, ReasoningEffort, ReviewerTaskContext, ReviewComment, ReviewDecisionIntent, ReviewFlow, ReviewOutcome, ReviewPolicy, ReviewPreferences, ReviewScreenshot, ReviewStatus, RevisionSummary, SessionDiffPreferences, SourceCheckpoint, TourOutline } from "./types.ts"; -import { isBriefingSectionOrder } from "./types.ts"; +import { isBriefingSectionOrder, isDiffRenderer } from "./types.ts"; import { importReviewScreenshots, screenshotAssetPath } from "./screenshots.ts"; import { revisionSummary } from "./revision.ts"; -import { diffPreferenceStoreDirectory, loadGlobalBriefingPreferences, loadGlobalDiffPreferences, restoreGlobalBriefingPreferences, restoreGlobalDiffPreferences, updateGlobalBriefingPreferences, updateGlobalDiffPreferences } from "./diff-preferences.ts"; +import { diffPreferenceStoreDirectory, loadGlobalBriefingPreferences, loadGlobalDiffPreferences, loadGlobalDiffRenderer, restoreGlobalBriefingPreferences, restoreGlobalDiffPreferences, updateGlobalBriefingPreferences, updateGlobalDiffPreferences, updateGlobalDiffRenderer } from "./diff-preferences.ts"; import { buildNeedsAttention } from "./attention.ts"; import { comparisonFingerprint, dryRunPlanFor, dryRunScenarioFingerprint, traceIsStale, validateDryRunPlans, validateDryRunPlanSourceAnchors, validateDryRunScenario, validateTraceSourceAnchors } from "./dry-runs.ts"; import { ArchifyRenderer, archifyCandidates } from "./archify.ts"; @@ -125,6 +125,7 @@ export class ReviewSession { readonly activities: AgentChangeActivity[]; readonly reviewedFiles: Set; private diffPreferences?: SessionDiffPreferences; + private diffRenderer?: DiffRenderer; readonly browserToken: string; readonly preferences: ReviewPreferences; readonly reviewPolicy: ReviewPolicy; @@ -885,6 +886,8 @@ export class ReviewSession { }); } const session = new ReviewSession(comparison, adapters, { ...defaults, attached, preferences, includeFindings }, bridge, persisted, globalDiffPreferences); + // Renderer choice belongs to the reviewer, including when resuming an older review. + session.diffRenderer = await loadGlobalDiffRenderer(preferenceDirectory); await session.selectComparisonView(); await session.updateRevisionSummary(session.fullComparison).catch(() => undefined); if (session.outline && !session.outline.briefing) session.outline.briefing = await buildBriefing(session.comparison, session.outline, session.briefingIntent()); @@ -1064,6 +1067,7 @@ export class ReviewSession { selectedCheckpointId: this.selectedCheckpointId ?? this.latestCheckpoint()?.id, revisionSummary: structuredClone(this.currentRevisionSummary), diffPreferences: structuredClone(this.diffPreferences), + diffRenderer: this.diffRenderer, preferences: structuredClone(this.preferences), reviewPolicy: structuredClone(this.reviewPolicy), findingGenerationEnabled: this.defaults.includeFindings !== false, @@ -1543,6 +1547,32 @@ export class ReviewSession { }); } + async setDiffRenderer(renderer: DiffRenderer) { + return this.serializeMutation(async () => { + this.ensureOpen(); + if (!isDiffRenderer(renderer)) throw new ReviewError("Diff renderer is invalid."); + if (this.diffRenderer === renderer) return this.snapshot(); + const previous = this.diffRenderer; + const previousRevision = this.revision; + const directory = this.defaults.diffPreferenceDirectory ?? diffPreferenceStoreDirectory(this.defaults.stateFile); + let globalUpdated = false; + let previousGlobal: DiffRenderer | undefined; + try { + previousGlobal = await updateGlobalDiffRenderer(directory, renderer); + globalUpdated = directory !== undefined; + this.diffRenderer = renderer; + this.revision += 1; + await this.persist(); + } catch (error) { + this.diffRenderer = previous; + this.revision = previousRevision; + if (globalUpdated) await updateGlobalDiffRenderer(directory, previousGlobal).catch(() => undefined); + throw error; + } + return this.snapshot(); + }); + } + async setDiffPreferences(scope: "desktop" | "mobile", preferences: DiffViewPreferences) { return this.serializeMutation(async () => { this.ensureOpen(); diff --git a/src/types.ts b/src/types.ts index 55a8eef..cdccbbe 100644 --- a/src/types.ts +++ b/src/types.ts @@ -620,6 +620,12 @@ export type ReviewOutcome = "accepted" | "feedback" | "cancelled"; export type ReviewDecisionIntent = "approve" | "approve-with-comments" | "request-changes" | "cancel"; export type ReviewStatus = "open" | ReviewOutcome; +export type DiffRenderer = "current" | "monaco"; + +export function isDiffRenderer(value: unknown): value is DiffRenderer { + return value === "current" || value === "monaco"; +} + export interface DiffViewPreferences { layout: "unified" | "split"; wrap: "scroll" | "wrap"; diff --git a/tests/lifecycle.test.ts b/tests/lifecycle.test.ts index c9da502..158834c 100644 --- a/tests/lifecycle.test.ts +++ b/tests/lifecycle.test.ts @@ -329,6 +329,57 @@ test("new sessions inherit the last repository-scoped diff preferences", async ( } finally { await first.dispose(); } }); +test("renderer choice follows the reviewer across repositories and resumed reviews", async () => { + const first = await repositoryFixture(); + const second = await repositoryFixture(); + const defaults = { ...noAdapters, diffPreferenceDirectory: join(first.directory, "preferences") }; + const sessions: ReviewSession[] = []; + const open = async (repository: string, stateFile: string, resume = false) => { + const session = await ReviewSession.create(repository, undefined, { ...defaults, stateFile, resume }); + sessions.push(session); + return session; + }; + try { + const original = await open(first.repository, first.stateFile); + expect(original.snapshot().diffRenderer).toBeUndefined(); + await original.setDiffPreferences("desktop", { layout: "unified", wrap: "wrap" }); + await original.setDiffRenderer("monaco"); + + const next = await open(second.repository, second.stateFile); + expect(next.snapshot().diffRenderer).toBe("monaco"); + expect(next.snapshot().diffPreferences).toBeUndefined(); + // Updating other saved preferences must retain the global renderer. + await next.setDiffPreferences("mobile", { layout: "split", wrap: "scroll" }); + const resumed = await open(first.repository, first.stateFile, true); + expect(resumed.snapshot().diffRenderer).toBe("monaco"); + expect(resumed.snapshot().diffPreferences?.desktop).toEqual({ layout: "unified", wrap: "wrap" }); + + await next.setDiffRenderer("current"); + const resumedAgain = await open(first.repository, first.stateFile, true); + expect(resumedAgain.snapshot().diffRenderer).toBe("current"); + } finally { + for (const session of sessions) await session.dispose(); + } +}); + +test("renderer preference persistence failures retain the previous choice", async () => { + const { repository, stateFile, directory } = await repositoryFixture(); + const session = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile }); + try { + await session.setDiffRenderer("monaco"); + const revision = session.revision; + // Fail the session write after the global settings file has been updated. + await rm(stateFile); + await mkdir(stateFile); + await expect(session.setDiffRenderer("current")).rejects.toThrow(); + expect(session.snapshot().diffRenderer).toBe("monaco"); + expect(session.revision).toBe(revision); + const next = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile: join(directory, "next.json") }); + try { expect(next.snapshot().diffRenderer).toBe("monaco"); } + finally { await next.dispose(); } + } finally { await session.dispose(); } +}); + test("new sessions inherit the global briefing preferences", async () => { const { directory, repository, stateFile } = await repositoryFixture(); const first = await ReviewSession.create(repository, undefined, { ...noAdapters, stateFile }); diff --git a/tests/server-handler.test.ts b/tests/server-handler.test.ts index 03e8512..1b4803a 100644 --- a/tests/server-handler.test.ts +++ b/tests/server-handler.test.ts @@ -247,6 +247,24 @@ test("diff preferences persist independently for desktop and mobile sessions", a expect(invalid.status).toBe(422); }); +test("renderer preferences accept both renderers and reject invalid or mixed settings", async () => { + const { session, fetch } = fixture(); + for (const diffRenderer of ["monaco", "current"]) { + const response = await fetch(authorizedRequest(session, "/api/preferences", { diffRenderer })); + expect(response.status).toBe(200); + expect((await response.json()).diffRenderer).toBe(diffRenderer); + } + for (const input of [ + { diffRenderer: "shiki" }, { diffRenderer: null }, { diffRenderer: ["monaco"] }, + { diffRenderer: "monaco", diffLayout: "split", diffWrap: "wrap", mobile: false }, + { diffRenderer: "monaco", prefetchSteps: 1 }, + ]) { + const response = await fetch(authorizedRequest(session, "/api/preferences", input)); + expect(response.status).toBe(422); + expect(session.snapshot().diffRenderer).toBe("current"); + } +}); + test("discussion endpoints validate paths, ranges, stop ids, and preserve quotations", async () => { const { session, fetch } = fixture(); (session as unknown as { outline: unknown; currentStepIndex: number }).outline = { diff --git a/tests/ui-preparation-update.test.ts b/tests/ui-preparation-update.test.ts index 4862362..ed44763 100644 --- a/tests/ui-preparation-update.test.ts +++ b/tests/ui-preparation-update.test.ts @@ -102,10 +102,10 @@ test("pending suggestion updates preserve the diff, tour and reading position", expect(h.classify(next, applied)).toBe(false); }); -test("saved layout preferences arriving by polling do not replace comments or move the diff", async () => { +test("saved layout and renderer preferences arriving by polling do not replace comments or move the diff", async () => { const h = harness(); const next = { ...structuredClone(h.state.session), revision: 5, - diffPreferences: { desktop: { layout: "unified", wrap: "wrap" } } }; + diffPreferences: { desktop: { layout: "unified", wrap: "wrap" } }, diffRenderer: "monaco" }; await h.poll(next); expect(h.renders).toBe(0); expect(h.commentRenders).toBe(0);