From 8d962a4062f645de9e9d7f32b051a18e00fe5900 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Jul 2026 15:27:56 +1000 Subject: [PATCH] fix(desktop): preserve unselected targets in partial ACP lock bumps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A partial run like `just bump-acp-tools --target aarch64-apple-darwin` rewrote the entire acp-tools.lock.json with only the selected targets, silently dropping the other three targets' entries. Because ensure/ prepare treat a target with zero lock entries as success (stage nothing, exit 0), a later desktop-release-build for a dropped target would ship an empty resources/acp and silently fall back to unpinned user installs — exactly the drift the bundling series exists to prevent. Make --target runs merge instead of replace: before resolving, read the existing lock and carry over every entry whose target is not selected, then write the union. The guard rails match the script's fail-loudly ethos: - A missing lock preserves nothing (there is nothing to drop) and the run proceeds — first-time generation with --target still works. - An unreadable, unparseable, or tools-array-less lock aborts the partial run before any registry work, refusing to overwrite pins it cannot see. - Full runs (no --target, or all four targets) never read the lock, so `just bump-acp-tools` can still regenerate a corrupt lock from scratch. Preserved entries are logged ("Preserved 6 existing lock entries for unselected targets") so a partial bump's diff is explainable from its output, and the usage text documents the merge behaviour. Addresses the partial-run finding from the bundling-series code review (review 2ed3d00d on b52a665a). Verification (against the Block registry, temp lock files): - --target aarch64-apple-darwin on a copy of the committed lock: 6 unselected entries preserved byte-identically; merged lock byte-identical to the committed lock (registry pins unchanged); 8 entries across all 4 targets. - --target with no existing lock: writes only the selected target's 2 entries, exit 0. - --target with corrupt JSON / {"tools":"nope"}: exits 1 in <1s with a refusing-to-overwrite message; lock file left untouched. - pnpm exec biome check + node scripts/check-file-sizes.mjs: clean. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- desktop/scripts/update-acp-tools-lock.mjs | 54 ++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/desktop/scripts/update-acp-tools-lock.mjs b/desktop/scripts/update-acp-tools-lock.mjs index bffcf0c320..dcadaf9b75 100755 --- a/desktop/scripts/update-acp-tools-lock.mjs +++ b/desktop/scripts/update-acp-tools-lock.mjs @@ -1,6 +1,6 @@ #!/usr/bin/env node import { execFile } from "node:child_process"; -import { mkdir, writeFile } from "node:fs/promises"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; import path from "node:path"; import process from "node:process"; import { promisify } from "node:util"; @@ -102,6 +102,10 @@ writes acp-tools.lock.json. Fails loudly when a package or one of its per-target native dependencies cannot be resolved — never silently pins an older version. +A --target run regenerates only the selected targets; the existing lock's +entries for every other target are preserved verbatim, so a partial bump +never drops another target's pins. + Supported targets: ${SUPPORTED_TARGETS.join("\n ")} @@ -341,14 +345,62 @@ async function lockToolForTarget(tool, target) { }; } +// Entries preserved from the existing lock when --target selects a subset. +// A missing lock preserves nothing (there is nothing to drop); an unreadable +// or malformed one aborts the run rather than clobbering pins we cannot see. +async function preservedLockEntries(lockFile, selectedTargets) { + let raw; + try { + raw = await readFile(lockFile, "utf8"); + } catch (error) { + if (error?.code === "ENOENT") return []; + throw new Error( + `Cannot read existing lock ${lockFile} to preserve unselected targets: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + let parsed; + try { + parsed = JSON.parse(raw); + } catch (error) { + throw new Error( + `Existing lock ${lockFile} is not valid JSON — refusing to overwrite it with a partial --target run: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + if (!Array.isArray(parsed?.tools)) { + throw new Error( + `Existing lock ${lockFile} has no tools array — refusing to overwrite it with a partial --target run`, + ); + } + return parsed.tools.filter((entry) => !selectedTargets.has(entry?.target)); +} + async function main() { const { targets, lockFile } = parseArgs(process.argv.slice(2)); + const selectedTargets = new Set(targets); + const fullRun = SUPPORTED_TARGETS.every((target) => + selectedTargets.has(target), + ); + const preserved = fullRun + ? [] + : await preservedLockEntries(lockFile, selectedTargets); const tools = []; for (const tool of TOOL_SPECS) { for (const target of targets) { tools.push(await lockToolForTarget(tool, target)); } } + if (preserved.length) { + console.log( + `Preserved ${preserved.length} existing lock entr${ + preserved.length === 1 ? "y" : "ies" + } for unselected targets`, + ); + } + tools.push(...preserved); tools.sort((left, right) => `${left.id}:${left.target}`.localeCompare(`${right.id}:${right.target}`), );