diff --git a/server/config.atomicWrite.test.js b/server/config.atomicWrite.test.js new file mode 100644 index 0000000..0e56cc1 --- /dev/null +++ b/server/config.atomicWrite.test.js @@ -0,0 +1,84 @@ +// #971: writeConfig must be atomic — a crash mid-write must never truncate +// config.json (the old in-place writeFileSync could leave it zero bytes = total +// config loss). It writes a tmp file then renameSync's it onto the target, so a +// failure at the commit point leaves the PRIOR config fully intact. updateConfig +// is the serialization point: it re-reads the freshest config before mutating, +// so a concurrent caller can't clobber it with a stale whole-config snapshot. +// +// Plain node:assert script — run with `node server/config.atomicWrite.test.js`. + +const assert = require("node:assert/strict"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const TMP = path.join(os.tmpdir(), `config-atomic-${process.pid}-${Date.now()}`); +const CONFIG_DIR = path.join(TMP, ".quadwork"); +const CONFIG_PATH = path.join(CONFIG_DIR, "config.json"); +fs.mkdirSync(CONFIG_DIR, { recursive: true }); + +const origHome = os.homedir; +os.homedir = () => TMP; +process.on("exit", () => { os.homedir = origHome; try { fs.rmSync(TMP, { recursive: true, force: true }); } catch {} }); + +const { writeConfig, readConfig, updateConfig } = require("./config"); + +let passed = 0; +const ok = (c, m) => { assert.ok(c, m); passed++; console.log(` PASS: ${m}`); }; +const tmpSiblings = () => fs.readdirSync(CONFIG_DIR).filter((f) => f.startsWith("config.json.") && f.endsWith(".tmp")); + +// Baseline write. +writeConfig({ port: 8400, projects: [{ id: "p", idle: false }] }); +ok(readConfig().port === 8400, "writeConfig persists config"); +ok((fs.statSync(CONFIG_PATH).mode & 0o777) === 0o600, "config.json is 0600 after atomic write"); +ok(tmpSiblings().length === 0, "no leftover .tmp file after a successful write"); + +// Atomicity: writeConfig must NOT open config.json in place — it writes a tmp +// and renames. Confirm by spying on the fs calls used. +{ + const origWrite = fs.writeFileSync; + const origRename = fs.renameSync; + const writeTargets = []; + const renameCalls = []; + fs.writeFileSync = (p, ...rest) => { writeTargets.push(String(p)); return origWrite(p, ...rest); }; + fs.renameSync = (a, b) => { renameCalls.push([String(a), String(b)]); return origRename(a, b); }; + try { + writeConfig({ port: 9001, projects: [] }); + } finally { + fs.writeFileSync = origWrite; + fs.renameSync = origRename; + } + ok(writeTargets.every((p) => p !== CONFIG_PATH), "config.json is never written in place (only a .tmp)"); + ok(renameCalls.some(([a, b]) => a.endsWith(".tmp") && b === CONFIG_PATH), "commit is a rename(tmp → config.json)"); + ok(readConfig().port === 9001, "atomic write landed the new content"); +} + +// Simulated crash at the commit point: rename throws. The PRIOR config must +// remain intact and no tmp file may be left behind. +writeConfig({ port: 1234, projects: [{ id: "keep" }] }); +{ + const origRename = fs.renameSync; + fs.renameSync = () => { const e = new Error("simulated crash (ENOSPC)"); e.code = "ENOSPC"; throw e; }; + let threw = false; + try { writeConfig({ port: 5678, projects: [] }); } catch { threw = true; } + fs.renameSync = origRename; + ok(threw, "writeConfig surfaces a commit-time failure"); + ok(readConfig().port === 1234, "prior config survives a crash mid-write (not truncated)"); + ok(readConfig().projects[0].id === "keep", "prior config content fully intact"); + ok(tmpSiblings().length === 0, "the orphaned .tmp is cleaned up on failure"); +} + +// updateConfig re-reads the freshest config before mutating (no stale clobber). +writeConfig({ port: 8400, projects: [], flags: {} }); +updateConfig((c) => { c.flags.a = 1; }); +updateConfig((c) => { c.flags.b = 2; }); // must NOT lose flags.a +const after = readConfig(); +ok(after.flags.a === 1 && after.flags.b === 2, "sequential updateConfig calls on different fields both persist"); + +// A mutator that reads a value written by a prior updateConfig sees it (fresh read). +updateConfig((c) => { c.flags.c = (c.flags.a || 0) + 10; }); +ok(readConfig().flags.c === 11, "updateConfig sees the freshest on-disk state"); + +console.log(`\n${passed} passed`); +console.log("server/config.atomicWrite.test.js: all assertions passed"); +process.exit(0); diff --git a/server/config.js b/server/config.js index 5604c43..f6d8a38 100644 --- a/server/config.js +++ b/server/config.js @@ -169,9 +169,35 @@ function writeSecureFile(filePath, data, extraOpts = {}) { try { fs.chmodSync(filePath, 0o600); } catch {} } -/** Write config.json atomically with 0o600 permissions. */ +// #971: write config.json ATOMICALLY (write a tmp file, then renameSync onto +// the target — the same crash-safe pattern migrate-ac.js uses). The old +// in-place writeFileSync could truncate config.json to zero bytes if the +// process died mid-write, losing the entire config; rename() is atomic on the +// same filesystem, so a reader always sees either the old or the new file, never +// a partial one. The tmp name carries the pid so two processes can't collide. function writeConfig(cfg) { - writeSecureFile(CONFIG_PATH, JSON.stringify(cfg, null, 2)); + const data = JSON.stringify(cfg, null, 2); + const tmpPath = `${CONFIG_PATH}.${process.pid}.tmp`; + writeSecureFile(tmpPath, data); // 0o600 + try { + fs.renameSync(tmpPath, CONFIG_PATH); // atomic commit; keeps the tmp's 0o600 + } catch (err) { + try { fs.unlinkSync(tmpPath); } catch { /* leave nothing behind */ } + throw err; + } +} + +// #971: single serialization point for read-modify-write config mutations. +// Runs synchronously (Node can't interleave a sync block), so it reads the +// freshest on-disk config, applies `mutator`, and atomically writes — a +// concurrent caller can't clobber it with a stale whole-config snapshot. Server +// mutators and the field-scoped endpoints go through here instead of doing their +// own GET→mutate→writeConfig. Returns the mutated config. +function updateConfig(mutator) { + const cfg = readConfig(); + mutator(cfg); + writeConfig(cfg); + return cfg; } -module.exports = { readConfig, resolveAgentCwd, resolveAgentCommand, resolveProjectChattr, sanitizeOperatorName, CONFIG_PATH, ensureSecureDir, writeSecureFile, writeConfig }; +module.exports = { readConfig, resolveAgentCwd, resolveAgentCommand, resolveProjectChattr, sanitizeOperatorName, CONFIG_PATH, ensureSecureDir, writeSecureFile, writeConfig, updateConfig }; diff --git a/server/configFlags.test.js b/server/configFlags.test.js new file mode 100644 index 0000000..e543a6e --- /dev/null +++ b/server/configFlags.test.js @@ -0,0 +1,138 @@ +// #971: field-scoped per-project flag endpoint + whole-config PUT preservation. +// - PATCH /api/projects/:id/flags merges ONLY whitelisted flags atomically, so +// two concurrent toggles on different fields both persist (no lost update). +// - a whole-config PUT (SettingsPage.save) can't clobber a flag or overwrite +// the raw operator_name it only ever GET's in sanitized form. +// +// Spins up express + the real router against a temp HOME. Plain node:assert. + +const assert = require("node:assert/strict"); +const http = require("http"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const TMP = path.join(os.tmpdir(), `config-flags-${process.pid}-${Date.now()}`); +const CONFIG_DIR = path.join(TMP, ".quadwork"); +const CONFIG_PATH = path.join(CONFIG_DIR, "config.json"); +fs.mkdirSync(CONFIG_DIR, { recursive: true }); + +const origHome = os.homedir; +os.homedir = () => TMP; +process.on("exit", () => { os.homedir = origHome; try { fs.rmSync(TMP, { recursive: true, force: true }); } catch {} }); + +const writeCfg = (o) => fs.writeFileSync(CONFIG_PATH, JSON.stringify(o, null, 2)); +const readCfg = () => JSON.parse(fs.readFileSync(CONFIG_PATH, "utf-8")); + +writeCfg({ port: 8400, operator_name: "Alice!", projects: [{ id: "lv", name: "lv" }] }); + +const router = require("./routes"); +const express = require("express"); + +function req(server, { method = "GET", urlPath, body, headers = {} } = {}) { + return new Promise((resolve, reject) => { + const { port } = server.address(); + const payload = body === undefined ? null : Buffer.from(JSON.stringify(body)); + const r = http.request({ host: "127.0.0.1", port, method, path: urlPath, + headers: { ...(payload ? { "content-type": "application/json", "content-length": payload.length } : {}), ...headers } }, + (res) => { const c = []; res.on("data", (d) => c.push(d)); res.on("end", () => resolve({ status: res.statusCode, body: Buffer.concat(c).toString() })); }); + r.on("error", reject); + if (payload) r.write(payload); + r.end(); + }); +} + +let passed = 0; +const ok = (c, m) => { assert.ok(c, m); passed++; console.log(` PASS: ${m}`); }; +const PATCH = (server, id, flags) => req(server, { method: "PATCH", urlPath: `/api/projects/${id}/flags`, body: flags }); + +(async () => { + const app = express(); + app.use(express.json()); + app.use(router); + const server = app.listen(0, "127.0.0.1"); + await new Promise((r) => server.once("listening", r)); + + try { + // ── Concurrent toggles on DIFFERENT fields both persist ────────────────── + const [a, b] = await Promise.all([ + PATCH(server, "lv", { idle: true }), + PATCH(server, "lv", { awake_auto: true }), + ]); + ok(a.status === 200 && b.status === 200, "two concurrent flag PATCHes both return 200"); + let entry = readCfg().projects.find((p) => p.id === "lv"); + ok(entry.idle === true && entry.awake_auto === true, "both concurrent toggles persist (no lost update)"); + + // ── More flags, multi-field, and the loop-guard delay ──────────────────── + await Promise.all([ + PATCH(server, "lv", { telegram_auto: true }), + PATCH(server, "lv", { discord_auto: true }), + PATCH(server, "lv", { trigger_auto: true }), + PATCH(server, "lv", { bridge_filter_agents_only: true }), + PATCH(server, "lv", { auto_continue_loop_guard: true, auto_continue_delay_sec: 45 }), + ]); + entry = readCfg().projects.find((p) => p.id === "lv"); + ok(entry.telegram_auto && entry.discord_auto && entry.trigger_auto && entry.bridge_filter_agents_only && + entry.auto_continue_loop_guard === true && entry.auto_continue_delay_sec === 45 && entry.idle === true, + "five concurrent multi-field toggles all persist, none clobbered"); + + // ── Validation ─────────────────────────────────────────────────────────── + ok((await PATCH(server, "lv", { bogus: true })).status === 400, "unknown flag → 400"); + ok((await PATCH(server, "lv", { idle: "yes" })).status === 400, "wrong type (string for boolean) → 400"); + ok((await PATCH(server, "lv", { auto_continue_delay_sec: -1 })).status === 400, "negative delay → 400"); + ok((await PATCH(server, "ghost", { idle: true })).status === 404, "unknown project → 404"); + ok((await req(server, { method: "PATCH", urlPath: "/api/projects/lv/flags", body: {} })).status === 400, "empty body → 400"); + + // ── Whole-config PUT can't clobber a flag or the raw operator_name ──────── + // Simulate SettingsPage.save with a STALE snapshot: idle=false (stale) and + // operator_name = the sanitized value it GET'd. + const getCfg = JSON.parse((await req(server, { urlPath: "/api/config" })).body); + ok(getCfg.operator_name === "Alice", "GET /api/config returns the SANITIZED operator_name"); + getCfg.projects[0].idle = false; // stale — a concurrent PATCH set it true + const put = await req(server, { method: "PUT", urlPath: "/api/config", body: getCfg }); + ok(put.status === 200, "whole-config PUT succeeds"); + const disk = readCfg(); + ok(disk.projects[0].idle === true, "whole-config PUT did NOT clobber the field-scoped idle flag (kept disk value)"); + ok(disk.operator_name === "Alice!", "raw operator_name survives a Settings save (not overwritten with sanitized)"); + + // ── PATCH /api/config: the section-merge Settings save (no whole-config PUT) ─ + // Send only owned sections (Settings strips the flags): operator_name echoed + // sanitized, a new top-level (butler), and a project with edited agents. + const patch = await req(server, { method: "PATCH", urlPath: "/api/config", body: { + operator_name: "Alice", // sanitized echo — must keep raw "Alice!" + butler: { command: "claude", model: "opus" }, + projects: [{ id: "lv", name: "Renamed", agents: { head: { command: "codex" } } }], + }}); + ok(patch.status === 200, "PATCH /api/config (section merge) → 200"); + const d2 = readCfg(); + ok(d2.operator_name === "Alice!", "PATCH keeps the raw operator_name on a sanitized echo"); + ok(d2.butler && d2.butler.command === "claude" && d2.butler.model === "opus", "PATCH merges an owned top-level section (butler)"); + ok(d2.projects[0].name === "Renamed" && d2.projects[0].agents.head.command === "codex", "PATCH merges owned per-project fields (name, agents)"); + ok(d2.projects[0].idle === true && d2.projects[0].telegram_auto === true, "PATCH preserves the field-scoped flags from disk (no clobber)"); + + // PATCH must never touch the field-scoped-owned top-level keys. + writeCfg({ ...readCfg(), pinned_projects: ["lv"] }); + await req(server, { method: "PATCH", urlPath: "/api/config", body: { pinned_projects: [], operator_name: "Bob" } }); + ok(readCfg().pinned_projects.length === 1, "PATCH ignores field-scoped-owned top-level keys (pinned_projects untouched)"); + + // A project id not on disk is appended (Settings can add a project). + await req(server, { method: "PATCH", urlPath: "/api/config", body: { projects: [{ id: "new", name: "New" }] } }); + ok(readCfg().projects.some((p) => p.id === "new") && readCfg().projects.some((p) => p.id === "lv"), + "PATCH appends a new project without dropping existing ones"); + + // ── DELETE /api/projects/:id — Settings "Remove" persists a deletion ───── + // (the section-merge PATCH intentionally never drops projects). + ok(readCfg().projects.some((p) => p.id === "new"), "precondition: 'new' project exists"); + const del = await req(server, { method: "DELETE", urlPath: "/api/projects/new" }); + ok(del.status === 200, "DELETE /api/projects/new → 200"); + ok(!readCfg().projects.some((p) => p.id === "new"), "deleted project is removed from disk"); + ok(readCfg().projects.some((p) => p.id === "lv"), "DELETE leaves other projects intact"); + ok((await req(server, { method: "DELETE", urlPath: "/api/projects/nope" })).status === 404, "DELETE unknown project → 404"); + + console.log(`\n${passed} passed`); + console.log("server/configFlags.test.js: all assertions passed"); + } finally { + server.close(); + } + process.exit(0); +})().catch((err) => { console.error(err.message || err); process.exit(1); }); diff --git a/server/routes.js b/server/routes.js index 077c244..28006bc 100644 --- a/server/routes.js +++ b/server/routes.js @@ -368,6 +368,28 @@ router.put("/api/config", (req, res) => { // never carries it — re-read and preserve it (same reason as the keys above). if ("session_token" in disk) body.session_token = disk.session_token; else delete body.session_token; + // #971: GET returns the SANITIZED operator_name; if the client PUTs back that + // exact sanitized value it did not actually edit it, so keep the raw on-disk + // value instead of overwriting it with the sanitized form (routes.js:327). + if (typeof body.operator_name === "string" && "operator_name" in disk && + body.operator_name === sanitizeOperatorName(disk.operator_name)) { + body.operator_name = disk.operator_name; + } + // #971: the per-project flag fields are owned by PUT /api/projects/:id/flags. + // A whole-config PUT (e.g. SettingsPage.save) carries a possibly-stale + // snapshot of them, so re-read each project's flags off disk and keep those — + // a Settings save can never revert a concurrent toggle (extends #944). + if (Array.isArray(body.projects)) { + const diskById = new Map((disk.projects || []).map((p) => [p.id, p])); + for (const proj of body.projects) { + const dp = diskById.get(proj.id); + if (!dp) continue; + for (const k of PROJECT_FLAG_KEYS) { + if (k in dp) proj[k] = dp[k]; + else delete proj[k]; + } + } + } writeConfig(body); // Trigger sync is handled internally since we're in the same process now if (typeof req.app.get("syncTriggers") === "function") { @@ -379,6 +401,132 @@ router.put("/api/config", (req, res) => { } }); +// #971: section-merge config write for the Settings form — the last frontend +// caller that used the whole-config PUT. It MERGES the provided sections into the +// freshest on-disk config under updateConfig (atomic, serialized), so it never +// does a whole-config read-modify-write and can't clobber a concurrent write: +// - top-level keys the client owns are assigned (never the field-scoped-owned +// keys pinned_projects/sidebar_groups/reviewer_github_user/session_token, +// which have their own endpoints); +// - operator_name keeps the raw on-disk value when the client echoes the +// sanitized one it GET'd; +// - projects are merged by id, PRESERVING each project's field-scoped flag +// fields from disk; a project id not on disk is appended (Settings can add +// one); on-disk projects absent from the body are left untouched. +const CONFIG_MERGE_EXCLUDED = new Set([ + "projects", "pinned_projects", "sidebar_groups", "reviewer_github_user", "session_token", +]); +router.patch("/api/config", (req, res) => { + const body = req.body && typeof req.body === "object" ? req.body : {}; + try { + updateConfig((cfg) => { + for (const [k, v] of Object.entries(body)) { + if (CONFIG_MERGE_EXCLUDED.has(k)) continue; + if (k === "operator_name" && typeof v === "string" && + "operator_name" in cfg && v === sanitizeOperatorName(cfg.operator_name)) { + continue; // unchanged sanitized echo — keep the raw on-disk value + } + cfg[k] = v; + } + if (Array.isArray(body.projects)) { + if (!Array.isArray(cfg.projects)) cfg.projects = []; + const byId = new Map(cfg.projects.map((p) => [p.id, p])); + for (const incoming of body.projects) { + if (!incoming || typeof incoming !== "object" || !incoming.id) continue; + const existing = byId.get(incoming.id); + if (existing) { + const preserved = {}; + for (const fk of PROJECT_FLAG_KEYS) if (fk in existing) preserved[fk] = existing[fk]; + Object.assign(existing, incoming, preserved); + } else { + cfg.projects.push(incoming); + } + } + } + }); + } catch (err) { + return res.status(500).json({ error: "Failed to write config", detail: err.message }); + } + if (typeof req.app.get("syncTriggers") === "function") req.app.get("syncTriggers")(); + res.json({ ok: true }); +}); + +// ─── Per-project flag toggles (field-scoped, race-free) — #971 ────────────── +// The toggle widgets (idle, awake_auto, trigger_auto, telegram/discord auto, +// bridge filter, loop-guard auto-continue) used to GET the whole config, flip +// one field, and PUT the entire object back — so any two overlapping toggles +// (or a Settings save) clobbered each other with stale snapshots. This endpoint +// merges ONLY the whitelisted flag(s) into the target project, under the +// atomic updateConfig() serialization point, so a toggle can never lose an +// unrelated concurrent write. Booleans, except the loop-guard delay (seconds). +const PROJECT_FLAG_KEYS = new Set([ + "idle", + "awake_auto", + "trigger_auto", + "telegram_auto", + "discord_auto", + "bridge_filter_agents_only", + "auto_continue_loop_guard", + "auto_continue_delay_sec", +]); + +router.patch("/api/projects/:id/flags", (req, res) => { + const { id } = req.params; + const body = req.body && typeof req.body === "object" ? req.body : {}; + const keys = Object.keys(body); + if (keys.length === 0) return res.status(400).json({ error: "No flags provided" }); + + const updates = {}; + for (const k of keys) { + if (!PROJECT_FLAG_KEYS.has(k)) return res.status(400).json({ error: `Unknown flag: ${k}` }); + const v = body[k]; + if (k === "auto_continue_delay_sec") { + if (typeof v !== "number" || !Number.isFinite(v) || v < 0) { + return res.status(400).json({ error: "auto_continue_delay_sec must be a non-negative number" }); + } + } else if (typeof v !== "boolean") { + return res.status(400).json({ error: `${k} must be a boolean` }); + } + updates[k] = v; + } + + try { + updateConfig((cfg) => { + const entry = (cfg.projects || []).find((p) => p.id === id); + if (!entry) { const e = new Error("not found"); e.code = "QW_NOT_FOUND"; throw e; } + Object.assign(entry, updates); + }); + } catch (err) { + if (err.code === "QW_NOT_FOUND") return res.status(404).json({ error: `Unknown project: ${id}` }); + return res.status(500).json({ error: "Failed to write config", detail: err.message }); + } + + // idle / trigger_auto gate the scheduler — resync it (same as the whole PUT). + if (typeof req.app.get("syncTriggers") === "function") req.app.get("syncTriggers")(); + res.json({ ok: true }); +}); + +// #971: field-scoped project removal. The section-merge PATCH above never drops +// projects (it can't tell "not edited" from "deleted"), so the Settings "Remove" +// action deletes here — atomically, via updateConfig — instead of relying on a +// whole-config replace. Mirrors the prior behavior (config entry removed; source +// clones/worktrees are left for `quadwork cleanup`). +router.delete("/api/projects/:id", (req, res) => { + const { id } = req.params; + try { + updateConfig((cfg) => { + const before = (cfg.projects || []).length; + cfg.projects = (cfg.projects || []).filter((p) => p.id !== id); + if (cfg.projects.length === before) { const e = new Error("not found"); e.code = "QW_NOT_FOUND"; throw e; } + }); + } catch (err) { + if (err.code === "QW_NOT_FOUND") return res.status(404).json({ error: `Unknown project: ${id}` }); + return res.status(500).json({ error: "Failed to write config", detail: err.message }); + } + if (typeof req.app.get("syncTriggers") === "function") req.app.get("syncTriggers")(); + res.json({ ok: true }); +}); + // ─── Pinned projects & sidebar groups (field-scoped, race-free) ───────────── // #944: pins/groups used to persist via the whole-config GET→mutate→PUT path, // which dropped them whenever any concurrent writer PUT a stale snapshot — and @@ -479,7 +627,7 @@ router.put("/api/reviewer-github-user", (req, res) => { // ─── Chat (file-based) ──────────────────────────────────────────────────── -const { sanitizeOperatorName, ensureSecureDir, writeSecureFile, writeConfig } = require("./config"); +const { sanitizeOperatorName, ensureSecureDir, writeSecureFile, writeConfig, updateConfig } = require("./config"); const { findAgentChattr } = require("./install-agentchattr"); /** diff --git a/server/routes.reviewerAccountSettings.test.js b/server/routes.reviewerAccountSettings.test.js index 2df7e5f..6d9868a 100644 --- a/server/routes.reviewerAccountSettings.test.js +++ b/server/routes.reviewerAccountSettings.test.js @@ -27,6 +27,7 @@ const real = { chmodSync: fs.chmodSync, statSync: fs.statSync, rmSync: fs.rmSync, + renameSync: fs.renameSync, }; const files = new Map([[CONFIG_PATH, JSON.stringify({ port: 8400, reviewer_github_user: "old-reviewer", projects: [] }, null, 2)]]); const modes = new Map(); @@ -73,6 +74,19 @@ fs.statSync = function stubStatSync(p, ...rest) { if (key.startsWith(TEST_DIR)) return { mode: modes.get(key) || 0o600 }; return real.statSync.call(this, p, ...rest); }; +// #971: writeConfig is now atomic (tmp write + renameSync) — model rename in the +// in-memory store so the config PUT/field-scoped writes still work here. +fs.renameSync = function stubRenameSync(src, dst) { + const s = String(src), d = String(dst); + if (s.startsWith(TEST_DIR) || d.startsWith(TEST_DIR)) { + if (!files.has(s)) { const err = new Error("ENOENT"); err.code = "ENOENT"; throw err; } + files.set(d, files.get(s)); + if (modes.has(s)) modes.set(d, modes.get(s)); + files.delete(s); modes.delete(s); + return; + } + return real.renameSync.apply(this, arguments); +}; const express = require("express"); const router = require("./routes"); @@ -86,6 +100,7 @@ function cleanup() { fs.chmodSync = real.chmodSync; fs.statSync = real.statSync; fs.rmSync = real.rmSync; + fs.renameSync = real.renameSync; } process.on("exit", cleanup); diff --git a/src/components/ControlBar.tsx b/src/components/ControlBar.tsx index c461f44..22af4f6 100644 --- a/src/components/ControlBar.tsx +++ b/src/components/ControlBar.tsx @@ -508,18 +508,12 @@ function SystemSection({ projectId, idle = false }: { projectId: string; idle?: manualStopRef.current = false; } try { - const r = await fetch("/api/config"); - if (!r.ok) return; - const cfg = await r.json(); - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (entry) { - entry.awake_auto = next; - await fetch("/api/config", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), - }); - } + // #971: field-scoped write — merges only awake_auto (no whole-config clobber) + await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ awake_auto: next }), + }); } catch { /* non-fatal */ } }, [awakeAuto, projectId]); diff --git a/src/components/DiscordBridgeWidget.tsx b/src/components/DiscordBridgeWidget.tsx index 37df488..bbaae5f 100644 --- a/src/components/DiscordBridgeWidget.tsx +++ b/src/components/DiscordBridgeWidget.tsx @@ -211,18 +211,12 @@ export default function DiscordBridgeWidget({ projectId, idle = false }: Discord prevBatchRef.current = null; } try { - const r = await fetch("/api/config"); - if (!r.ok) return; - const cfg = await r.json(); - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (entry) { - entry.discord_auto = next; - await fetch("/api/config", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), - }); - } + // #971: field-scoped write — merges only discord_auto (no whole-config clobber) + await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ discord_auto: next }), + }); } catch { /* non-fatal */ } }, [autoDiscord, projectId]); diff --git a/src/components/LoopGuardWidget.tsx b/src/components/LoopGuardWidget.tsx index 7248120..2ea44fd 100644 --- a/src/components/LoopGuardWidget.tsx +++ b/src/components/LoopGuardWidget.tsx @@ -111,23 +111,17 @@ export default function LoopGuardWidget({ projectId }: LoopGuardWidgetProps) { const saveAutoContinue = async (nextEnabled: boolean, nextDelay: number) => { setAutoContinueSaving(true); try { - const cfgRes = await fetch(`/api/config`); - if (!cfgRes.ok) throw new Error(`GET /api/config ${cfgRes.status}`); - const cfg = await cfgRes.json(); - if (!cfg || !Array.isArray(cfg.projects)) throw new Error("config shape"); - const idx = cfg.projects.findIndex((p: { id: string }) => p.id === projectId); - if (idx < 0) throw new Error(`project ${projectId} not found`); - cfg.projects[idx] = { - ...cfg.projects[idx], - auto_continue_loop_guard: nextEnabled, - auto_continue_delay_sec: nextDelay, - }; - const putRes = await fetch(`/api/config`, { - method: "PUT", + // #971: field-scoped write — merges only the two loop-guard fields under an + // atomic read-modify-write, so it can't clobber a concurrent toggle. + const putRes = await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), + body: JSON.stringify({ + auto_continue_loop_guard: nextEnabled, + auto_continue_delay_sec: nextDelay, + }), }); - if (!putRes.ok) throw new Error(`PUT /api/config ${putRes.status}`); + if (!putRes.ok) throw new Error(`PATCH flags ${putRes.status}`); } finally { setAutoContinueSaving(false); } diff --git a/src/components/ProjectDashboard.tsx b/src/components/ProjectDashboard.tsx index 04c9ae7..dc919cf 100644 --- a/src/components/ProjectDashboard.tsx +++ b/src/components/ProjectDashboard.tsx @@ -93,22 +93,13 @@ export default function ProjectDashboard({ projectId }: ProjectDashboardProps) { const toggleFilter = useCallback(() => { setFilterSystem((prev) => { const next = !prev; - // #525: persist to project config so bridges respect the filter - fetch("/api/config") - .then((r) => r.ok ? r.json() : null) - .then((cfg) => { - if (!cfg) return; - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (entry) { - entry.bridge_filter_agents_only = next; - return fetch("/api/config", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), - }); - } - }) - .catch(() => {}); + // #525/#971: persist to project config (field-scoped, no whole-config clobber) + // so bridges respect the filter. + fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ bridge_filter_agents_only: next }), + }).catch(() => {}); return next; }); }, [projectId]); diff --git a/src/components/ScheduledTriggerWidget.tsx b/src/components/ScheduledTriggerWidget.tsx index 2a0ae81..2cccf83 100644 --- a/src/components/ScheduledTriggerWidget.tsx +++ b/src/components/ScheduledTriggerWidget.tsx @@ -355,18 +355,12 @@ export default function ScheduledTriggerWidget({ projectId, idle = false }: Sche prevBatchRef.current = null; } try { - const r = await fetch("/api/config"); - if (!r.ok) return; - const cfg = await r.json(); - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (entry) { - entry.trigger_auto = next; - await fetch("/api/config", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), - }); - } + // #971: field-scoped write — merges only trigger_auto (no whole-config clobber) + await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ trigger_auto: next }), + }); } catch { /* non-fatal */ } }, [autoTrigger, projectId]); diff --git a/src/components/SettingsPage.tsx b/src/components/SettingsPage.tsx index e724d47..7367826 100644 --- a/src/components/SettingsPage.tsx +++ b/src/components/SettingsPage.tsx @@ -611,10 +611,27 @@ export default function SettingsPage() { } : config.butler, }; + // #971: save via the section-merge PATCH (no whole-config PUT). Send only + // the sections Settings owns — strip the field-scoped-owned keys so the + // payload can never carry a stale flag/pin the server owns via its own + // endpoints. The server merges into the freshest config under updateConfig. + const FLAG_KEYS = [ + "idle", "awake_auto", "trigger_auto", "telegram_auto", "discord_auto", + "bridge_filter_agents_only", "auto_continue_loop_guard", "auto_continue_delay_sec", + ]; + const patchBody: Record = { ...normalizedConfig }; + for (const k of ["pinned_projects", "sidebar_groups", "reviewer_github_user", "session_token"]) { + delete patchBody[k]; + } + patchBody.projects = normalizedConfig.projects.map((p) => { + const clean: Record = { ...p }; + for (const k of FLAG_KEYS) delete clean[k]; + return clean; + }); const res = await fetch("/api/config", { - method: "PUT", + method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(normalizedConfig), + body: JSON.stringify(patchBody), }); if (!res.ok) throw new Error(`${res.status}`); setConfig(normalizedConfig); @@ -752,11 +769,31 @@ export default function SettingsPage() { updateProject(idx, { archived: false }); }; - const removeProject = (idx: number) => { + // #971: removal persists immediately via the field-scoped DELETE endpoint + // (the section-merge save never drops projects). Local state + the saved + // snapshot both drop it, so any OTHER pending edits stay correctly marked dirty. + const removeProject = async (idx: number) => { if (!config) return; - const projects = config.projects.filter((_, i) => i !== idx); - setConfig({ ...config, projects }); + const target = config.projects[idx]; setConfirmDelete(null); + if (!target) return; + try { + const res = await fetch(`/api/projects/${encodeURIComponent(target.id)}`, { method: "DELETE" }); + if (!res.ok && res.status !== 404) throw new Error(`${res.status}`); + } catch (err) { + console.error(err); + return; // leave the UI unchanged on failure + } + setConfig({ ...config, projects: config.projects.filter((_, i) => i !== idx) }); + if (savedConfigRef.current) { + try { + const saved = JSON.parse(savedConfigRef.current); + if (Array.isArray(saved.projects)) { + saved.projects = saved.projects.filter((p: { id: string }) => p.id !== target.id); + savedConfigRef.current = JSON.stringify(saved); + } + } catch { /* dirty-tracking best-effort */ } + } }; if (!config) return
{t.loading}
; diff --git a/src/components/TelegramBridgeWidget.tsx b/src/components/TelegramBridgeWidget.tsx index f423215..e860779 100644 --- a/src/components/TelegramBridgeWidget.tsx +++ b/src/components/TelegramBridgeWidget.tsx @@ -232,18 +232,12 @@ export default function TelegramBridgeWidget({ projectId, idle = false }: Telegr prevBatchRef.current = null; } try { - const r = await fetch("/api/config"); - if (!r.ok) return; - const cfg = await r.json(); - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (entry) { - entry.telegram_auto = next; - await fetch("/api/config", { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), - }); - } + // #971: field-scoped write — merges only telegram_auto (no whole-config clobber) + await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ telegram_auto: next }), + }); } catch { /* non-fatal */ } }, [autoTelegram, projectId]); diff --git a/src/lib/idle.ts b/src/lib/idle.ts index 0ccff11..9918af7 100644 --- a/src/lib/idle.ts +++ b/src/lib/idle.ts @@ -1,6 +1,6 @@ // #814: shared per-project Active/Inactive (idle) state. The source of truth is -// `project.idle` in config (#812). Toggling persists via PUT /api/config — the -// server's syncTriggers hook stops a now-idle project's trigger — and broadcasts +// `project.idle` in config (#812). Toggling persists via PATCH /api/projects/:id/flags +// (#971) — the server's syncTriggers hook stops a now-idle project's trigger — and broadcasts // a lightweight in-tab signal so every open view (the dashboard's pollers, the // sidebar switch, the settings switch) re-syncs without a manual reload. @@ -24,20 +24,15 @@ export function onIdleChange(handler: (detail: IdleChangeDetail) => void): () => return () => window.removeEventListener(IDLE_EVENT, listener); } -// Persist a project's idle flag: re-read the latest config, flip ONLY this -// project's flag (preserving everything else), write it back. Throws on failure -// so callers can revert their optimistic UI. Broadcasts on success. +// #971: persist a project's idle flag via the field-scoped endpoint — the +// server merges ONLY this field under an atomic read-modify-write, so a toggle +// can't clobber a concurrent write with a stale whole-config snapshot. Throws on +// failure so callers can revert their optimistic UI. Broadcasts on success. export async function persistProjectIdle(projectId: string, idle: boolean): Promise { - const r = await fetch("/api/config"); - if (!r.ok) throw new Error("config read failed"); - const cfg = await r.json(); - const entry = (cfg.projects || []).find((p: { id: string }) => p.id === projectId); - if (!entry) throw new Error("project not found in config"); - entry.idle = idle; - const put = await fetch("/api/config", { - method: "PUT", + const put = await fetch(`/api/projects/${encodeURIComponent(projectId)}/flags`, { + method: "PATCH", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(cfg), + body: JSON.stringify({ idle }), }); if (!put.ok) throw new Error("config write failed"); emitIdleChange(projectId, idle);