Skip to content

Commit b836e5b

Browse files
Merge pull request #415: preserve raw config declarations
Fixes #364 what changed: Preserve raw config declarations during unrelated saves and refuse an unreadable config without a usable snapshot. why: Resolved fallbacks erased role/posture diagnostics and could silently replace broken config. what it touched: machine saving, install-hooks and panel refusals, five regressions, architecture, changelog and inventory. The source task remains in a6d6328; current-main sync only adds the two reviewed workflow assets. what you verified: Verified five failing regressions before the fix, 147 focused checks passing with two disclosed local unsupported symlink cases, normal full commit-gate acceptance on a6d6328 and docs-only acceptance on c1ab427. All seven current-base CI jobs passed in run38565023535 on this unchanged head/base. what you got wrong and fixed: A docs conflict helper's assumption about a neighboring panel row was caught by its guard before writing. Explicitly retained both rows; clean scope and focused/full gates verified the resolution.
2 parents 3d34bfb + c1ab427 commit b836e5b

8 files changed

Lines changed: 117 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in
55

66
## Unreleased
77

8+
- Machine config saves preserve unpatched raw role and posture values, including typos
9+
that diagnostics must still report. An unreadable config without a usable last-good
10+
snapshot refuses an unrelated save rather than persisting the restrictive fallback;
11+
an explicit valid `--role` can repair it. `install-hooks` names the refusal with exit 2
12+
and the panel key endpoint returns HTTP 409 without echoing the key (#364).
13+
814
- `path-authority` and `researcher-release` name a value flag with no value and print usage with exit 2 before resolving paths, including when another flag follows (#288).
915

1016
- `property-replay --all` refuses a missing regression directory with exit 2 and

‎docs/ARCHITECTURE.md‎

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

‎research-kit/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,7 @@ node research-kit/bin/selftest.mjs # all of it
568568
node research-kit/bin/selftest.mjs gate hook # just these files
569569
```
570570

571-
1838 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
571+
1843 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
572572
the assertions settled (ADR-0021), and each test is raced against a watchdog
573573
(`RESEARCH_KIT_TEST_TIMEOUT`, default 60s).
574574

‎research-kit/bin/install-hooks.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ evidencePolicy ${config.evidencePolicy}
109109
`);
110110
process.exit(failed ? 1 : 0);
111111
} catch (err) {
112-
const why = writeFailure(err);
112+
const why = err.code === 'CONFIG-UNREADABLE' ? err.message : writeFailure(err);
113113
if (!why) throw err;
114114
process.stderr.write(`${why}\n`);
115115
process.exit(2);

‎research-kit/lib/machine.mjs‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,12 +236,40 @@ export function loadConfig(env = process.env) {
236236
/** Writing drops retired keys, so the migration happens through normal use. */
237237
export function saveConfig(patch, env = process.env) {
238238
const file = configPath(env);
239-
const current = readMachineConfig(env).settings;
239+
const object = (value) => value !== null && typeof value === 'object' && !Array.isArray(value);
240+
const rawFrom = (source) => {
241+
const raw = parseJson(readText(source) ?? '');
242+
if (!object(raw)) throw new Error('config must be a JSON object');
243+
return raw;
244+
};
245+
let current = {};
246+
if (exists(file)) {
247+
try { current = rawFrom(file); }
248+
catch {
249+
try { current = rawFrom(lastGoodPath(file)); }
250+
catch {
251+
if (!Object.hasOwn(patch, 'role') || !ROLES.includes(patch.role)) {
252+
throw Object.assign(new Error(`machine config ${file} does not parse as a JSON object; repair it or declare the role with --role collector|builder before saving`), {
253+
code: 'CONFIG-UNREADABLE',
254+
});
255+
}
256+
}
257+
}
258+
}
259+
// Save declarations, not the resolved role/posture. A typo remains available for
260+
// doctor to name until the operator patches that field. Only the existing retired
261+
// spelling is migrated here; it must keep its mode when its old key is dropped.
262+
if (current.editGate?.mode === undefined && EDIT_GATE_MODES.includes(current.claudeGate)) {
263+
current = { ...current, editGate: { ...(object(current.editGate) ? current.editGate : {}), mode: current.claudeGate } };
264+
}
240265
const next = {
241266
...current,
242267
...patch,
243-
editGate: { ...current.editGate, ...(patch.editGate ?? {}) },
244268
};
269+
if (patch.editGate !== undefined) {
270+
next.editGate = { ...(object(current.editGate) ? current.editGate : {}), ...(patch.editGate ?? {}) };
271+
}
272+
for (const retired of Object.keys(RETIRED_CONFIG_KEYS)) delete next[retired];
245273
ensureDir(path.dirname(file));
246274
writeText(file, `${JSON.stringify(next, null, 2)}\n`);
247275
writeText(lastGoodPath(file), `${JSON.stringify(next, null, 2)}\n`);

‎research-kit/lib/panel.mjs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,11 @@ export function createPanel({
347347
if (problem) return refuse(res, 400, problem);
348348
const inside = configInsideRepository(env);
349349
if (inside) return refuse(res, 409, `the machine config is inside the repository at ${inside}; a key never goes into a repository (Rule 6). Point RESEARCH_KIT_CONFIG outside it.`);
350-
saveConfig({ [SEARCH_KEY.configKey]: body.key.trim() }, env);
350+
try { saveConfig({ [SEARCH_KEY.configKey]: body.key.trim() }, env); }
351+
catch (err) {
352+
if (err.code === 'CONFIG-UNREADABLE') return refuse(res, 409, err.message);
353+
throw err;
354+
}
351355
return send(res, 200, { saved: true, keys: keyState() });
352356
}
353357

‎research-kit/test/machine.test.mjs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,62 @@ test('saveConfig drops the retired key, so the migration happens through normal
216216
assert.equal(written.role, 'builder');
217217
});
218218

219+
test('an unrelated config save preserves the role the operator actually typed', () => {
220+
const { env, file } = envWith('{"role":"Builder","evidencePolicy":"STRICT"}');
221+
saveConfig({ failOpen: false }, env);
222+
const written = JSON.parse(readText(file));
223+
assert.equal(written.role, 'Builder');
224+
assert.equal(written.evidencePolicy, 'STRICT');
225+
assert.equal(written.failOpen, false);
226+
assert.equal(machineRole(env), 'unknown');
227+
saveConfig({ role: 'collector' }, env);
228+
assert.equal(JSON.parse(readText(file)).role, 'collector');
229+
});
230+
231+
test('a broken config without a usable snapshot refuses an unrelated save and permits a role repair', () => {
232+
for (const snapshot of [null, '[]', 'null']) {
233+
const original = '{"role":"bui';
234+
const { env, file } = envWith(original, { snapshot });
235+
assert.throws(() => saveConfig({ failOpen: true }, env), /does not parse.*--role/);
236+
assert.equal(readText(file), original, 'a refusal must keep the operator\'s only copy');
237+
if (snapshot === null) assert.equal(fs.existsSync(`${file}.last-good`), false);
238+
else assert.equal(readText(`${file}.last-good`), snapshot);
239+
saveConfig({ role: 'collector' }, env);
240+
assert.equal(JSON.parse(readText(file)).role, 'collector');
241+
assert.notEqual(JSON.parse(readText(file)).failOpen, false, 'no fallback-closed setting is persisted');
242+
}
243+
});
244+
245+
test('saving from a last-good snapshot retains its raw role and posture values', () => {
246+
const { env, file } = envWith('{"role":"bui', {
247+
snapshot: '{"role":"Builder","failOpen":"false","editGate":{"mode":"HARD-BLOCK"}}',
248+
});
249+
saveConfig({ serpapiKey: 'unit-test-key' }, env);
250+
const written = JSON.parse(readText(file));
251+
assert.equal(written.role, 'Builder');
252+
assert.equal(written.failOpen, 'false');
253+
assert.equal(written.editGate.mode, 'HARD-BLOCK');
254+
assert.equal(written.serpapiKey, 'unit-test-key');
255+
});
256+
257+
test('install-hooks names an unreadable config save refusal without replacing it', () => {
258+
const original = '{"role":"bui';
259+
const { env, file } = envWith(original);
260+
const isolated = {
261+
...env, RESEARCH_KIT_HOME: KIT_ROOT,
262+
RESEARCH_KIT_INSTALL_STATE: path.join(path.dirname(file), 'install.json'),
263+
GIT_CONFIG_GLOBAL: path.join(path.dirname(file), 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1',
264+
};
265+
const run = spawnSync(process.execPath, [path.join(KIT_ROOT, 'bin', 'install-hooks.mjs'), '--fail-closed', '--git-only'], {
266+
env: isolated, encoding: 'utf8', timeout: 60_000, windowsHide: true,
267+
});
268+
assert.equal(run.status, 2, `${run.stdout}${run.stderr}`);
269+
assert.match(run.stderr, /does not parse.*--role/);
270+
assert.doesNotMatch(run.stderr, /\n\s+at /);
271+
assert.equal(readText(file), original);
272+
assert.equal(fs.existsSync(isolated.RESEARCH_KIT_INSTALL_STATE), false);
273+
});
274+
219275
// Found 2026-10-11: the browser transport and its error messages tell the operator to set
220276
// browserPath (ADR-0088), but the config was read without it and rewritten without it, so
221277
// `install-hooks --role builder` erased a browserPath the operator had set by hand.

‎research-kit/test/panel.test.mjs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,21 @@ test('the page never writes server text as HTML', () => {
184184
assert(/button\.type = 'button'/.test(js) && /aria-label/.test(js), 'request output has no keyboard-accessible control');
185185
});
186186

187+
test('saving a key refuses a broken unsnapshotted config without replacing or echoing it', async () => {
188+
const m = machine();
189+
const original = '{"role":"bui';
190+
writeText(m.configFile, original);
191+
const ctx = await started({ machine: m, project: makeProject() });
192+
try {
193+
const res = await api(ctx, '/api/key/search', { key: KEY });
194+
assertEqual(res.status, 409, res.text);
195+
assert(/does not parse.*--role/.test(res.text), res.text);
196+
assert(!res.text.includes(KEY), 'the refusal echoed the key');
197+
assertEqual(readText(m.configFile), original);
198+
assert(!fs.existsSync(`${m.configFile}.last-good`), 'the refusal created a snapshot');
199+
} finally { await ctx.panel.close(); }
200+
});
201+
187202
test('saving the search key writes the machine config and never echoes the key', async () => {
188203
const m = machine();
189204
const ctx = await started({ machine: m, project: makeProject() });

0 commit comments

Comments
 (0)