diff --git a/src/cli.ts b/src/cli.ts index b8c5c3b..bc6e3e7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,12 +5,12 @@ import { dirname, resolve } from "node:path"; import { exportConfiguration } from "./exporter.js"; import type { ConfigExport } from "./model.js"; -import { createPatchPlan, type PatchInput, type PatchPlan } from "./planner.js"; +import { createPatchPlan, planUpdate, type PatchInput, type PatchPlan } from "./planner.js"; import { applyPatchPlan } from "./applier.js"; import { assertPlanNotConsumed } from "./journal.js"; interface Arguments { - command: "list" | "export" | "plan" | "apply" | "help"; + command: "list" | "export" | "plan" | "apply" | "update" | "help"; device?: number; out?: string; timeoutMs: number; @@ -18,6 +18,7 @@ interface Arguments { sets: string[]; plan?: string; yes: boolean; + backup?: string; } function usage(): string { @@ -25,14 +26,19 @@ function usage(): string { mft-config list [--timeout ] mft-config export [--device ] [--out ] [--timeout ] mft-config plan --snapshot --set [--set ] [--out ] + mft-config update --set [--set ] [--yes] [--backup ] mft-config apply --plan --yes [--device ] (disabled) This tool only sends Universal Identity, global pull (0x02), encoder bulk-pull (0x04/0x01), and device-ID pull (0x05) messages. The plan command is offline. -apply is the only command that writes settings, and it is disabled in this -release while the write-path defects in issue #14 are open. list, export, and -plan are unaffected.`; +update reads the controller, works out what actually differs, and writes only +that. Without --yes it prints the difference and writes nothing. + +apply is a separate flow for a reviewed plan file, and is disabled in this +release while the write-path defects in issue #14 are open. update is not +affected: it keeps no journal and no hidden state, and writes a backup file you +name.`; } function parseArguments(argv: string[]): Arguments { @@ -40,7 +46,9 @@ function parseArguments(argv: string[]): Arguments { if (command === "help" || command === "--help" || command === "-h") { return { command: "help", timeoutMs: 500, sets: [], yes: false }; } - if (command !== "list" && command !== "export" && command !== "plan" && command !== "apply") throw new Error(`Unknown command: ${command}`); + if (command !== "list" && command !== "export" && command !== "plan" && command !== "apply" && command !== "update") { + throw new Error(`Unknown command: ${command}`); + } const result: Arguments = { command, timeoutMs: 500, sets: [], yes: false }; for (let index = 1; index < argv.length; index += 1) { @@ -61,6 +69,9 @@ function parseArguments(argv: string[]): Arguments { } else if (option === "--set" && value !== undefined) { result.sets.push(value); index += 1; + } else if (option === "--backup" && value !== undefined) { + result.backup = value; + index += 1; } else if (option === "--plan" && value !== undefined) { result.plan = value; index += 1; @@ -76,6 +87,7 @@ function parseArguments(argv: string[]): Arguments { if (result.device !== undefined && (!Number.isInteger(result.device) || result.device < 0)) { throw new Error("--device must be a non-negative integer"); } + if (command === "update" && result.sets.length === 0) throw new Error("update requires at least one --set "); if (command === "plan" && !result.snapshot) throw new Error("plan requires --snapshot "); if (command === "plan" && result.sets.length === 0) throw new Error("plan requires at least one --set "); // Before the --plan and --yes checks: a disabled command must not coach the @@ -177,6 +189,54 @@ async function main(): Promise { const device = devices[selectedIndex]; if (!device) throw new Error(`Device index ${selectedIndex} does not exist; use the list command first`); + if (args.command === "update") { + // Read, work out the difference, write only that. No plan file changes + // hands, so there is nothing to forge and nothing to replay — which is why + // this needs neither the journal nor the single-use machinery that `apply` + // carries, and is not gated behind them. + const readConnection = backend.connect(device); + let config: ConfigExport; + try { + config = await exportConfiguration(readConnection, device, { timeoutMs: args.timeoutMs }); + } finally { + readConnection.close(); + } + + const { plan, skipped } = planUpdate(config, args.sets.map(parseSet)); + for (const skip of skipped) process.stdout.write(`unchanged ${skip.path} is already ${String(skip.value)}\n`); + if (!plan) { + process.stdout.write("Nothing to do; every requested value is already set.\n"); + return; + } + for (const change of plan.changes) { + process.stdout.write(`change ${change.path}: ${String(change.expected)} -> ${String(change.desired)}\n`); + } + if (!plan.applyEligibility.eligible) { + throw new Error(`This controller cannot accept live writes: ${plan.applyEligibility.reasons.join("; ")}`); + } + if (!args.yes) { + process.stdout.write(`\n${plan.changes.length} change(s) not written. Re-run with --yes to apply.\n`); + return; + } + + const backupPath = resolve(args.backup ?? `mft-backup-${new Date().toISOString().replaceAll(":", "-")}.json`); + const connection = backend.connectForApply(device); + try { + const result = await applyPatchPlan(connection, device, plan, { + journalPath: resolve(dirname(backupPath), "mft-updates.ndjson"), + timeoutMs: args.timeoutMs, + saveBackup: async (snapshot) => { + await writeAtomically(backupPath, `${JSON.stringify(snapshot, null, 2)}\n`); + return backupPath; + }, + }); + process.stdout.write(`\nApplied and verified ${plan.changes.length} change(s).\nBackup: ${result.backupPath}\n`); + } finally { + connection.close(); + } + return; + } + if (args.command === "apply") { const plan = JSON.parse(await readFile(resolve(args.plan!), "utf8")) as PatchPlan; const stateDirectory = resolve(".mft-state"); diff --git a/src/planner.ts b/src/planner.ts index ee282c0..ac85864 100644 --- a/src/planner.ts +++ b/src/planner.ts @@ -258,6 +258,54 @@ export function evaluateApplyEligibility(config: ConfigExport): { eligible: bool return { eligible: reasons.length === 0, reasons }; } +export interface UpdatePlan { + /** Null when every requested value is already set. */ + plan: PatchPlan | null; + skipped: Array<{ path: string; value: number | boolean }>; +} + +/** + * Plans an update: the same content-addressed plan, minus the values already + * at their target. + * + * `createPatchPlan` throws on a no-op, which is right for `plan`, where the + * contract is "the state I described is the state I found". It is wrong for an + * update, where "make the top row green" must not fail because one of those + * knobs is already green. Here a no-op is skipped and reported. + * + * Duplicate paths are rejected rather than resolved. Two `--set`s for one field + * have no obvious winner, and silently picking one is worse than refusing. + */ +export function planUpdate(config: ConfigExport, inputs: PatchInput[], now = new Date()): UpdatePlan { + if (config.schemaVersion !== "djtt.mft.config-export.v1") throw new Error("Unsupported snapshot schema"); + if (inputs.length === 0) throw new Error("At least one --set operation is required"); + const policy = firmwarePolicy(config.device.firmware.date); + const palette = config.globals.colorMap.name === "mf64" ? "mf64" : "classic"; + + const seen = new Set(); + const pending: PatchInput[] = []; + const skipped: UpdatePlan["skipped"] = []; + + for (const input of inputs) { + const target = parseTarget(config, input.path); + if (seen.has(target.normalizedPath)) throw new Error(`Duplicate --set for ${target.normalizedPath}`); + seen.add(target.normalizedPath); + + const rawExpected = target.rawTags[String(target.rule.tag)]; + if (rawExpected === undefined) throw new Error(`${target.normalizedPath} was not reported by this firmware`); + const expected = semanticRaw(rawExpected, target.rule, policy.shiftedChannelIsOneBased); + const desired = resolveValue(input.value, target.rule, palette); + if (desired === expected) { + skipped.push({ path: target.normalizedPath, value: desired }); + continue; + } + pending.push(input); + } + + if (pending.length === 0) return { plan: null, skipped }; + return { plan: createPatchPlan(config, pending, now), skipped }; +} + export function createPatchPlan(config: ConfigExport, inputs: PatchInput[], now = new Date()): PatchPlan { if (config.schemaVersion !== "djtt.mft.config-export.v1") throw new Error("Unsupported snapshot schema"); if (inputs.length === 0) throw new Error("At least one --set operation is required"); diff --git a/test/cli.test.ts b/test/cli.test.ts index bfe79a4..7907106 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -41,7 +41,17 @@ test("help still documents apply as disabled", () => { const result = runCli(["--help"]); assert.equal(result.status, 0); - assert.match(result.stdout, /apply is the only command that writes settings, and it is disabled/); + assert.match(result.stdout, /apply is a separate flow for a reviewed plan file, and is disabled/); + assert.match(result.stdout, /mft-config update --set/); +}); + +test("update rejects an empty change list and is not caught by the apply gate", () => { + // No device is touched: argument validation rejects before discovery. + const result = runCli(["update"]); + + assert.equal(result.status, 1); + assert.match(result.stderr, /update requires at least one --set/); + assert.doesNotMatch(result.stderr, /apply is disabled/); }); test("plan still works while apply is gated", () => { diff --git a/test/planner.test.ts b/test/planner.test.ts index 637bf3d..c5a5178 100644 --- a/test/planner.test.ts +++ b/test/planner.test.ts @@ -1,10 +1,11 @@ import assert from "node:assert/strict"; import { readFile } from "node:fs/promises"; +import { readFileSync } from "node:fs"; import test from "node:test"; import { encoderTransferTag, firmwarePolicy } from "../src/compatibility.js"; import type { ConfigExport, EncoderExport } from "../src/model.js"; -import { createPatchPlan } from "../src/planner.js"; +import { createPatchPlan, planUpdate } from "../src/planner.js"; import { assembleBulkParts, parseBulkPart, parseTagValues } from "../src/protocol.js"; import { snapshotHash } from "../src/snapshot.js"; @@ -112,3 +113,47 @@ test("compatibility fixtures select the expected last encoder tag", async () => assert.equal(policy.liveWriteAllowed, fixture.writeAllowed); } }); + +test("planUpdate skips values already set instead of aborting the whole update", () => { + const config = JSON.parse(readFileSync(new URL("fixtures/synthetic-four-bank.json", import.meta.url), "utf8")) as ConfigExport; + // Encoder 1 is already green in the fixture; encoder 2 is not being changed + // to its current value. `createPatchPlan` refuses the pair outright. + assert.throws( + () => createPatchPlan(config, [ + { path: "bank.1.encoder.1.colors.active", value: "green" }, + { path: "bank.1.encoder.2.colors.active", value: "blue" }, + ]), + /is already/, + ); + + const { plan, skipped } = planUpdate(config, [ + { path: "bank.1.encoder.1.colors.active", value: "green" }, + { path: "bank.1.encoder.2.colors.active", value: "blue" }, + ]); + + assert.equal(skipped.length, 1); + assert.equal(skipped[0]?.path, "bank.1.encoder.1.colors.active"); + assert.equal(plan?.changes.length, 1); + assert.equal(plan?.changes[0]?.path, "bank.1.encoder.2.colors.active"); +}); + +test("planUpdate reports nothing to do when every value is already set", () => { + const config = JSON.parse(readFileSync(new URL("fixtures/synthetic-four-bank.json", import.meta.url), "utf8")) as ConfigExport; + + const { plan, skipped } = planUpdate(config, [{ path: "bank.1.encoder.1.colors.active", value: "green" }]); + + assert.equal(plan, null, "an all-skipped update must succeed having done nothing"); + assert.equal(skipped.length, 1); +}); + +test("planUpdate refuses two --set operations for the same field", () => { + const config = JSON.parse(readFileSync(new URL("fixtures/synthetic-four-bank.json", import.meta.url), "utf8")) as ConfigExport; + + assert.throws( + () => planUpdate(config, [ + { path: "bank.1.encoder.1.colors.active", value: "blue" }, + { path: "bank.1.encoder.1.colors.active", value: "red" }, + ]), + /Duplicate --set/, + ); +});