diff --git a/src/webview/cm/frontmatter/reveal-state.ts b/src/webview/cm/frontmatter/reveal-state.ts index 59609c19..822d1b95 100644 --- a/src/webview/cm/frontmatter/reveal-state.ts +++ b/src/webview/cm/frontmatter/reveal-state.ts @@ -31,7 +31,13 @@ // be used: it needs `pos < from` strictly, impossible for a from=0 leading // span.) -import { type ChangeDesc, EditorState, StateEffect, type Transaction } from "@codemirror/state"; +import { + type ChangeDesc, + EditorState, + type Facet, + StateEffect, + type Transaction, +} from "@codemirror/state"; import { EditorView } from "@codemirror/view"; import { intersectsAnySelection } from "../decorations/shared.js"; @@ -67,21 +73,39 @@ function changeCoversRange(changes: ChangeDesc, range: { from: number; to: numbe return covered; } -/** The facets `isWritable()` reads — the single source of truth for - * writability's reactive dependencies. Providers whose output is gated on write +/** The SINGLE source of truth for "which facets gate writability" — each facet + * paired with the value that means writable. `writabilityFacets` (the reactive + * dependency list) and `isWritable()` (the predicate) both derive from this, so + * the two can no longer drift: a new writability input is one row here and both + * update. CodeMirror's canonical "can edit" authority is EditorState.readOnly + * (writable when false); EditorView.editable controls the DOM contenteditable + * (writable when true). The reveal logic checks BOTH so a (readOnly=true, + * editable=true) combination cannot leak a reveal (Codex re-review #4). */ +const writabilityInputs: readonly Readonly<{ + facet: Facet; + writableWhen: boolean; +}>[] = [ + { facet: EditorState.readOnly, writableWhen: false }, + { facet: EditorView.editable, writableWhen: true }, +]; + +/** The facets `isWritable()` reads. Providers whose output is gated on write * access (e.g. the collapsed widget's aria-description in frontmatter-field.ts) * spread this into their `compute()` dependency list so a facet-only reconfigure - * still recomputes. KEEP IN SYNC: any facet added to `isWritable()` below MUST be - * added here too, or writability-gated output goes stale on that facet's change. */ -export const writabilityFacets = [EditorState.readOnly, EditorView.editable] as const; - -/** CodeMirror's canonical "can edit" authority is EditorState.readOnly; the - * EditorView.editable facet controls the DOM contenteditable. The reveal logic - * checks BOTH so a (readOnly=true, editable=true) combination cannot leak a - * reveal (Codex re-review #4). Its facet reads are mirrored by - * `writabilityFacets` above. */ + * still recomputes. Derived from `writabilityInputs` — no hand-maintained list to + * drift out of sync with `isWritable()`. Typed `readonly` so the derived list is + * as immutable as the old `as const` tuple was (`.map()` otherwise widens to a + * mutable array, which would undercut the drift guarantee). */ +export const writabilityFacets: readonly Facet[] = writabilityInputs.map( + (input) => input.facet +); + +/** True iff every writability input holds its writable value (readOnly=false AND + * editable=true). Derived from the same `writabilityInputs` source as + * `writabilityFacets`, so the predicate and its reactive dependency list stay in + * lockstep by construction. */ export function isWritable(state: EditorState): boolean { - return !state.readOnly && state.facet(EditorView.editable); + return writabilityInputs.every((input) => state.facet(input.facet) === input.writableWhen); } export function nextRevealState(prev: RevealState, tr: Transaction): RevealState { diff --git a/test/webview/frontmatter/cm-frontmatter-writability.test.ts b/test/webview/frontmatter/cm-frontmatter-writability.test.ts new file mode 100644 index 00000000..025f422b --- /dev/null +++ b/test/webview/frontmatter/cm-frontmatter-writability.test.ts @@ -0,0 +1,53 @@ +// @vitest-environment happy-dom +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { describe, expect, it } from "vitest"; +import { isWritable, writabilityFacets } from "../../../src/webview/cm/frontmatter/reveal-state.js"; + +// Pins the writability contract PR #279 fixed and this change mechanized: +// `isWritable()` (the predicate) and `writabilityFacets` (the +// reactive dependency list spread into compute() in frontmatter-field.ts) derive +// from ONE source, so they cannot drift out of sync. These tests go red if the +// polarity of either facet is inverted OR if the reactive dependency list stops +// covering a facet the predicate reads (the stale-aria-description regression). + +function stateWith(opts: { readOnly: boolean; editable: boolean }): EditorState { + return EditorState.create({ + extensions: [EditorState.readOnly.of(opts.readOnly), EditorView.editable.of(opts.editable)], + }); +} + +describe("frontmatter writability contract", () => { + it("isWritable is true only when readOnly=false AND editable=true", () => { + expect(isWritable(stateWith({ readOnly: false, editable: true }))).toBe(true); + expect(isWritable(stateWith({ readOnly: true, editable: true }))).toBe(false); + expect(isWritable(stateWith({ readOnly: false, editable: false }))).toBe(false); + expect(isWritable(stateWith({ readOnly: true, editable: false }))).toBe(false); + }); + + it("writabilityFacets covers exactly the facets isWritable gates on", () => { + // Reactive-dependency completeness: the collapsed widget's aria-description + // only recomputes when a facet in this list changes, so it MUST list every + // facet isWritable reads — no more, no fewer. + expect(writabilityFacets).toContain(EditorState.readOnly); + expect(writabilityFacets).toContain(EditorView.editable); + expect(writabilityFacets).toHaveLength(2); + }); + + it("every facet in writabilityFacets actually gates isWritable (no dead entries)", () => { + // Drive the real writabilityFacets list: from a fully-writable baseline, + // flipping ANY one listed facet away from its writable value must turn + // isWritable false. A facet listed in writabilityFacets that isWritable + // ignored (a dead entry) would leave isWritable true on that facet's flip. + const base = stateWith({ readOnly: false, editable: true }); + expect(isWritable(base)).toBe(true); + for (const target of writabilityFacets) { + const flipped = EditorState.create({ + extensions: writabilityFacets.map((facet) => + facet.of(facet === target ? !base.facet(facet) : base.facet(facet)) + ), + }); + expect(isWritable(flipped)).toBe(false); + } + }); +});