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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/guide-viewer-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import type { GuideViewerAssets } from "./guide-format";

export const GUIDE_VIEWER_MANIFEST: Omit<GuideViewerAssets, "baseUrl"> = {
js: "viewer.C8JgNbZu.js",
js: "viewer.-z4hZ-DF.js",
css: "viewer.KIp-iPxY.css",
jsIntegrity: "sha384-dqRkZ2N5WNwLIuwa+1kdrvswkOVHpkBxrrJ5mTU4YrUWzO/8fRS5MRTlDGV9t7eB",
jsIntegrity: "sha384-0m4lvEvRi8w3vafYuzG3N918AAH78sDPytVzerfzpOBxU63u3VrdyhHpBOVpDT4u",
cssIntegrity: "sha384-Ty9hGpag8KIAGMDPg7ImsB4L5RabqvonNVjrj/CnYqSJDqIgPRBwjEMoK9/EvgXm",
langs: {
"astro": "chunks/astro.BykyiR6i.js",
Expand Down
41 changes: 36 additions & 5 deletions packages/review-editor/utils/reviewSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { readFileSync } from 'node:fs';
import { join } from 'node:path';
import { resetStorageBackend, setStorageBackend } from '@plannotator/ui/utils/storage';
import { ConfigStoreForTest } from '../../ui/config/configStore';
import { setReviewPanelView } from '../../ui/config/reviewView';
import {
initializeReviewSetup,
needsReviewSetup,
Expand Down Expand Up @@ -32,52 +33,79 @@ afterEach(() => {
});

describe('initializeReviewSetup', () => {
test('a genuinely new reviewer starts with Tree while keeping the since-base diff default', () => {
test('a fresh reviewer reaches setup once after normal settings startup', () => {
installMemoryBackend();
const store = makeStore();

// Rendering reads settings before /api/diff initializes server config.
// Neither step is evidence that the reviewer chose the registry default.
store.get('displayName');
store.init();

expect(initializeReviewSetup(store)).toBe(true);
expect(store.get('reviewPanelView')).toBe('tree');
expect(store.get('reviewPanelViewLastUsed')).toBe('tree');
expect(store.get('defaultDiffType')).toBe('since-base');
expect(needsReviewSetup()).toBe(false);

expect(initializeReviewSetup(store)).toBe(false);
const nextSession = makeStore();
nextSession.init();
expect(initializeReviewSetup(nextSession)).toBe(false);
expect(nextSession.get('reviewPanelView')).toBe('tree');
expect(nextSession.get('reviewPanelViewLastUsed')).toBe('tree');
});

test('an unseen reviewer inherits an existing classic diff default', () => {
installMemoryBackend({
'plannotator-default-diff-type': 'uncommitted',
});
const store = makeStore();
store.get('reviewPanelView');
store.init();

expect(initializeReviewSetup(store)).toBe(true);
expect(store.get('reviewPanelView')).toBe('tree');
expect(store.get('reviewPanelViewLastUsed')).toBe('tree');
expect(store.get('defaultDiffType')).toBe('uncommitted');
});

test('an unseen reviewer inherits a local-vs-remote default', () => {
test('first-run setup preserves the server diff default without writing it back', async () => {
installMemoryBackend({
'plannotator-default-diff-type': 'local-vs-remote',
'plannotator-default-diff-type': 'uncommitted',
});
const store = makeStore();
const synced: Record<string, unknown>[] = [];
store.setServerSync(payload => { synced.push(payload); });
store.get('reviewPanelView');
store.init({ diffOptions: { defaultDiffType: 'local-vs-remote' } });

expect(initializeReviewSetup(store)).toBe(true);
expect(store.get('reviewPanelView')).toBe('tree');
expect(store.get('defaultDiffType')).toBe('local-vs-remote');

await new Promise<void>(resolve => setTimeout(resolve, 350));
expect(synced).toEqual([]);
});

test('an explicit persisted view survives a session that never tripped the seen gate', () => {
// Non-git / workspace / PR / no-since-base sessions never reach the
// initializer, so a reviewer can persist a view from Settings while
// "seen" stays unset. The next plain git session must not seed over it.
installMemoryBackend({
'plannotator-review-panel-view': 'sections',
'plannotator-default-diff-type': 'since-base',
'plannotator-review-panel-view-last-used': 'tree',
});
const settingsStore = makeStore();
settingsStore.get('displayName');
// Even choosing the built-in default is an explicit choice.
setReviewPanelView('sections', undefined, settingsStore);

const store = makeStore();
store.init();

expect(initializeReviewSetup(store)).toBe(false);
expect(store.get('reviewPanelView')).toBe('sections');
expect(store.get('reviewPanelViewLastUsed')).toBe('sections');
expect(store.get('defaultDiffType')).toBe('since-base');
// The one-time setup is consumed, so this cannot be re-evaluated later.
expect(needsReviewSetup()).toBe(false);
Expand All @@ -89,6 +117,7 @@ describe('initializeReviewSetup', () => {
'plannotator-review-panel-view-last-used': 'sections',
});
const store = makeStore();
store.init();

expect(initializeReviewSetup(store)).toBe(false);
expect(store.get('reviewPanelView')).toBe('tree');
Expand All @@ -106,6 +135,8 @@ describe('initializeReviewSetup', () => {
'plannotator-default-diff-type': 'since-base',
});
const store = makeStore();
store.get('reviewPanelView');
store.init();

expect(initializeReviewSetup(store)).toBe(false);
expect(store.get('reviewPanelView')).toBe('sections');
Expand Down
2 changes: 2 additions & 0 deletions packages/review-editor/utils/reviewSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export function initializeReviewSetup(store: typeof configStore = configStore):
// let Settings persist a panel view, so a reviewer can hold an explicit
// choice while "seen" stays unset. Seeding Tree there would overwrite it.
// A persisted view IS the decision: consume the one-time setup and leave it.
// The registry keeps this default in memory only, so a settings read cannot
// manufacture that evidence before this initializer runs.
if (getPersistedReviewPanelView() !== undefined) {
markReviewSetupSeen();
return false;
Expand Down
18 changes: 18 additions & 0 deletions packages/ui/config/configStore.lazyInit.seam.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,22 @@ describe('configStore lazy resolution', () => {
expect(second).toBe(first);
expect(reads.length).toBe(readsAfterLoad);
});

test('default seeding leaves review setup undecided on load and backend hydration', () => {
setStorageBackend(hostBackend);
const store = new ConfigStoreForTest();
const identity = store.get('displayName');

expect(store.get('reviewPanelView')).toBe('sections');
expect(stored.has('plannotator-review-panel-view')).toBe(false);

// A host may install an empty backend after settings were already read.
stored.clear();
store.loadFromBackend();

// Generated defaults still persist, but a panel default is not a choice.
expect(stored.get('plannotator-identity')).toBe(identity);
expect(stored.has('plannotator-review-panel-view')).toBe(false);
expect(new ConfigStoreForTest().get('displayName')).toBe(identity);
});
});
12 changes: 6 additions & 6 deletions packages/ui/config/configStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class ConfigStore {
* first use — deliberately not in the constructor. The singleton is created
* at module import, which for a host app is before configurePlannotatorUI()
* can install its StorageBackend; resolving eagerly there would write every
* missing default (including a generated identity) as cookies onto the host's
* eligible missing default (including a generated identity) onto the host's
* origin. Deferring to first use means a host that configures at startup gets
* its own backend for the initial resolution too — no cookies are ever
* written on a configured host. Plannotator is unchanged: same resolution,
Expand All @@ -105,8 +105,8 @@ class ConfigStore {
: def.defaultValue;
const resolved = fromCookie ?? defaultVal;
this.values.set(name, resolved);
// Persist generated defaults to cookie so the value is stable across calls
if (fromCookie === undefined) {
// Persist defaults for stability unless absence must remain distinguishable.
if (fromCookie === undefined && !('persistDefault' in def && def.persistDefault === false)) {
def.toCookie(resolved as never);
}
}
Expand All @@ -127,9 +127,9 @@ class ConfigStore {
const fromBackend = def.fromCookie();
if (fromBackend !== undefined) {
this.values.set(name, fromBackend);
} else {
// Seed the host backend with the resolved default. This matters when
// the store was already resolved BEFORE the host installed its
} else if (!('persistDefault' in def && def.persistDefault === false)) {
// Seed the host backend with the resolved default unless it opts out.
// This matters when the store was resolved BEFORE the host installed its
// StorageBackend (e.g. something read a setting pre-configure): those
// default-seeding writes went to the earlier backend, not this one.
// Without this, a fresh host store is never populated, so generated
Expand Down
4 changes: 4 additions & 0 deletions packages/ui/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ function isDiffLineBgIntensity(v: unknown): v is DiffLineBgIntensity {

export interface SettingDef<T> {
defaultValue: T | (() => T);
/** Persist missing defaults automatically unless false; explicit writes are unaffected. */
persistDefault?: boolean;
fromCookie: () => T | undefined;
toCookie: (value: T) => void;
/** If set, this setting syncs to server via POST /api/config */
Expand Down Expand Up @@ -223,6 +225,8 @@ export const SETTINGS = {
// previously-persisted 'commits' cookie is treated as unset.
reviewPanelView: {
defaultValue: 'sections' as 'sections' | 'tree',
// An absent cookie means no user choice yet, so first-run setup can choose its default.
persistDefault: false,
fromCookie: () => {
const v = storage.getItem('plannotator-review-panel-view');
return v === 'tree' || v === 'sections' ? v : undefined;
Expand Down