Carry editor participation through policy writes instead of dropping it

This commit is contained in:
Anthony Stirling
2026-08-28 13:40:00 +01:00
committed by James Brunton
parent b7935f9bd9
commit 595b9cbc85
5 changed files with 107 additions and 7 deletions
@@ -76,6 +76,8 @@ export interface PolicyState {
configured: boolean;
status: PolicyStatus;
sources: string[];
/** Whether the editor runs this policy per file; stored, not derived from `sources`. */
runsOnEditor?: boolean;
scopeTypes: string[];
reviewerEmail: string;
fieldValues: Record<string, boolean | string | string[]>;
@@ -433,6 +435,7 @@ function decoratePolicy(
configured: true,
status,
sources: decoded.sources,
runsOnEditor: decoded.runsOnEditor,
scopeTypes: decoded.scopeTypes,
reviewerEmail: decoded.reviewerEmail,
fieldValues: decoded.fieldValues,
@@ -595,6 +598,9 @@ export function buildWireFromSetup(
enabled,
categoryId: entry.category.id,
sources: result.sources,
// A fresh wizard save: the editor is on when the user picked it as a source,
// the same rule the editor's own buildBackendPolicy applies.
runsOnEditor: result.sources.includes("editor"),
scopeTypes: result.scopeTypes,
reviewerEmail: result.reviewerEmail,
fieldValues: result.fieldValues,
@@ -625,6 +631,8 @@ export function buildWireFromState(
enabled,
categoryId: entry.category.id,
sources: s.sources,
// Carry the stored value through: pause/resume must not re-derive it.
runsOnEditor: s.runsOnEditor === true,
scopeTypes: s.scopeTypes,
reviewerEmail: s.reviewerEmail,
fieldValues: s.fieldValues,
@@ -37,7 +37,10 @@ import {
} from "@app/services/policyBackend";
import { reorderPolicies as reorderBackendPolicies } from "@app/services/policyApi";
import { orderRewritesFirst } from "@app/data/classificationPolicy";
import type { PolicyToStore } from "@app/services/policyPipeline";
import {
EDITOR_SOURCE_ID,
type PolicyToStore,
} from "@app/services/policyPipeline";
import type {
PoliciesByCategory,
PolicyConfigResult,
@@ -172,6 +175,9 @@ export function usePolicies() {
backendId,
fieldValues: result.fieldValues,
sources: result.sources,
// Patch the flag alongside the list it is derived from on save, or the cache
// keeps running a policy on upload that the user just took off the editor.
runsOnEditor: result.sources.includes(EDITOR_SOURCE_ID),
scopeTypes: result.scopeTypes,
reviewerEmail: result.reviewerEmail,
outputMode: result.folder.outputMode,
@@ -209,6 +215,9 @@ export function usePolicies() {
backendId,
fieldValues: result.fieldValues,
sources: result.sources,
// Patch the flag alongside the list it is derived from on save, or the cache
// keeps running a policy on upload that the user just took off the editor.
runsOnEditor: result.sources.includes(EDITOR_SOURCE_ID),
scopeTypes: result.scopeTypes,
reviewerEmail: result.reviewerEmail,
outputMode: result.folder.outputMode,
@@ -274,6 +283,9 @@ export function usePolicies() {
backendId,
fieldValues: result.fieldValues,
sources: result.sources,
// Patch the flag alongside the list it is derived from on save, or the cache
// keeps running a policy on upload that the user just took off the editor.
runsOnEditor: result.sources.includes(EDITOR_SOURCE_ID),
scopeTypes: result.scopeTypes,
reviewerEmail: result.reviewerEmail,
outputMode: result.folder.outputMode,
@@ -8,6 +8,7 @@ const FULL_STATE: PolicyDecodedState = {
enabled: true,
categoryId: "security",
sources: ["editor", "gdrive"],
runsOnEditor: true,
scopeTypes: ["Contracts", "Invoices"],
reviewerEmail: "admin@example.com",
fieldValues: { auditTrail: true, frameworks: ["HIPAA"] },
@@ -44,6 +45,26 @@ describe("toWirePolicy", () => {
expect(opts.position).toBe("prefix");
});
it("sends the editor block so a save never drops editor participation", () => {
expect(toWirePolicy(FULL_STATE).editor).toEqual({
allowed: true,
runOn: "upload",
});
expect(toWirePolicy({ ...FULL_STATE, runsOnEditor: false }).editor).toEqual(
{ allowed: false, runOn: "upload" },
);
});
it("keeps editor participation that empty sources would have re-derived away", () => {
// The seeded Classification policy: editor-run, no sources.
const wire = toWirePolicy({
...FULL_STATE,
sources: [],
runsOnEditor: true,
});
expect(wire.editor?.allowed).toBe(true);
});
it("preserves steps at the top level", () => {
const wire = toWirePolicy(FULL_STATE);
expect(wire.steps).toEqual(FULL_STATE.steps);
@@ -69,15 +90,24 @@ describe("fromWirePolicy → round-trip", () => {
expect(decoded.steps).toEqual(FULL_STATE.steps);
});
it("defaults a missing runOn to the category default (security → export)", () => {
const wire = toWirePolicy(FULL_STATE);
// The moment has two possible homes now (the `editor` block, and the legacy
// options bag), so "nothing stored" means clearing both.
const withNoStoredRunOn = (state: PolicyDecodedState) => {
const wire = toWirePolicy(state);
delete (wire.output.options as Record<string, unknown>).runOn;
expect(fromWirePolicy(wire).runOn).toBe("export");
delete wire.editor;
return wire;
};
it("defaults a missing runOn to the category default (security → export)", () => {
expect(fromWirePolicy(withNoStoredRunOn(FULL_STATE)).runOn).toBe("export");
});
it("defaults a missing runOn to upload for other categories", () => {
const wire = toWirePolicy({ ...FULL_STATE, categoryId: "classification" });
delete (wire.output.options as Record<string, unknown>).runOn;
const wire = withNoStoredRunOn({
...FULL_STATE,
categoryId: "classification",
});
expect(fromWirePolicy(wire).runOn).toBe("upload");
});
@@ -109,6 +139,28 @@ describe("fromWirePolicy → round-trip", () => {
}
});
it("reads editor participation off the editor block, not sources", () => {
const wire = toWirePolicy(FULL_STATE);
expect(fromWirePolicy(wire).runsOnEditor).toBe(true);
expect(
fromWirePolicy({ ...wire, editor: { allowed: false, runOn: "upload" } })
.runsOnEditor,
).toBe(false);
});
it("prefers the editor block's moment over the legacy options bag", () => {
const wire = toWirePolicy(FULL_STATE);
wire.output.options.runOn = "upload";
wire.editor = { allowed: true, runOn: "export" };
expect(fromWirePolicy(wire).runOn).toBe("export");
});
it("falls back to the stored moment when the editor does not run it", () => {
const wire = toWirePolicy({ ...FULL_STATE, runOn: "export" });
wire.editor = { allowed: false, runOn: "upload" };
expect(fromWirePolicy(wire).runOn).toBe("export");
});
it("handles empty options gracefully", () => {
const decoded = fromWirePolicy({
id: "x",
@@ -120,6 +172,7 @@ describe("fromWirePolicy → round-trip", () => {
});
expect(decoded.categoryId).toBe("");
expect(decoded.sources).toEqual([]);
expect(decoded.runsOnEditor).toBe(false);
expect(decoded.runOn).toBe("upload");
expect(decoded.outputMode).toBe("new_version");
});
@@ -41,6 +41,9 @@ export function toWirePolicy(state: PolicyDecodedState): WirePolicy {
trigger: null,
steps: state.steps,
output: { type: "inline", options },
// Omitting this makes the backend stamp EditorConfig.disabled(), so a pause or a
// wizard save would quietly take the policy off the editor.
editor: { allowed: state.runsOnEditor, runOn: state.runOn },
};
}
@@ -63,10 +66,17 @@ export function fromWirePolicy(policy: WirePolicy): PolicyDecodedState {
enabled: policy.enabled,
categoryId,
sources: Array.isArray(raw.sources) ? raw.sources : [],
runsOnEditor: policy.editor?.allowed === true,
scopeTypes: Array.isArray(raw.scopeTypes) ? raw.scopeTypes : [],
reviewerEmail: str(raw.reviewerEmail),
fieldValues: raw.fieldValues ?? {},
runOn: resolveRunOn(raw.runOn, categoryId),
// The moment lives on `editor` now, but only carries meaning while the editor
// runs it (EditorConfig coerces a disabled policy's runOn to "upload"); fall back
// to the legacy options bag otherwise so the wizard still shows what was chosen.
runOn: resolveRunOn(
policy.editor?.allowed ? policy.editor.runOn : raw.runOn,
categoryId,
),
outputMode: raw.mode === "new_file" ? "new_file" : "new_version",
outputName: str(raw.name),
outputNamePosition: position,
@@ -36,6 +36,16 @@ export interface WireOutputSpec {
options: Partial<WireOutputOptions>;
}
/**
* Mirrors `EditorConfig.java`. Absent only on records that never went through the
* backend (hand-built fixtures); a stored policy always carries it, derived from
* the legacy `output.options` bag when it predates the field.
*/
export interface WireEditorConfig {
allowed: boolean;
runOn: "upload" | "export";
}
export interface WirePolicy {
id: string;
name: string;
@@ -44,6 +54,7 @@ export interface WirePolicy {
trigger: null;
steps: WirePipelineStep[];
output: WireOutputSpec;
editor?: WireEditorConfig;
teamId?: string;
}
@@ -80,6 +91,12 @@ export interface PolicyDecodedState {
enabled: boolean;
categoryId: string;
sources: string[];
/**
* Whether the editor runs this policy per file. Its own field, not derived from
* `sources`: the seeded Classification policy is editor-run with empty sources,
* so re-deriving on write would silently take it off the editor.
*/
runsOnEditor: boolean;
scopeTypes: string[];
reviewerEmail: string;
fieldValues: Record<string, boolean | string | string[]>;