Skip to content

Commit dee68f8

Browse files
Merge pull request #272 from StepenkoAnatoli/claude/loving-shannon-kwdnrq
Fix two silently ignored inputs: researcher-release --genesis/--checkpoints, and browserPath
2 parents d6035da + e3bca63 commit dee68f8

10 files changed

Lines changed: 176 additions & 9 deletions

‎CHANGELOG.md‎

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

66
## Unreleased
77

8+
- The `browserPath` machine-config key is read. The browser transport's errors and ADR-0088
9+
name it, but the config was loaded and saved without it: a fetch through the collector
10+
never consulted it, and any save, `install-hooks --role` among them, erased a value set
11+
by hand. `scrape` and `status` called without a config now read the machine's.
12+
13+
- `researcher-release validate --genesis` and `--checkpoints` are verified. Both were
14+
advertised and parsed, but `validateRelease` had no parameters for them, so they were
15+
dropped before the ledger check and an operator who supplied a signed genesis believed it
16+
was checked. Each must now exist below the record root, as `--ledger` and `--head` must,
17+
and either one without `--ledger` is refused as INCOMPLETE instead of ignored.
18+
819
- Under an explicit refusal of this machine's network (`allowInternalRedirects: false`,
920
or `RESEARCH_KIT_ALLOW_INTERNAL_REDIRECTS=0`) a name asked for is now judged by what it
1021
resolves to - at the browser guard even when it is the page's own host, and before any

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,7 @@ The full table, including the release-evidence validators and the artifact forma
606606
| `evidencePolicy` | `pluralist`, `strict` | `pluralist` | whether a keyless or partial capture passes the gate with a warning or fails it |
607607
| `serpapiKey` | a key | none | SerpAPI search; the environment variable `SERPAPI_API_KEY` works too |
608608
| `searxngUrl` | a URL | none | a SearXNG instance for search |
609+
| `browserPath` | a path | the usual install paths | the Chromium or Chrome the `browser` transport runs; read before `RESEARCH_KIT_BROWSER` (ADR-0088) |
609610
| `failOpen` | `true`, `false` | `true` | what the gate does when it cannot run at all |
610611
| `editGate.mode` | `ask`, `hard-block`, `off` | `ask` | the Claude Code edit-time gate; `install-hooks.mjs --mode` sets it |
611612
| `editGate.settingsPath` | a path | `~/.claude/settings.json` | where the edit-time gate is registered |

‎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
@@ -561,7 +561,7 @@ node research-kit/bin/selftest.mjs # all of it
561561
node research-kit/bin/selftest.mjs gate hook # just these files
562562
```
563563

564-
1780 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
564+
1784 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
565565
the assertions settled (ADR-0021), and each test is raced against a watchdog
566566
(`RESEARCH_KIT_TEST_TIMEOUT`, default 60s).
567567

‎research-kit/lib/browser-transport.mjs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { mainContent, htmlToMarkdown, titleOf, gradeCompleteness, isInternal, se
2020
import httpKeyless from './http-transport.mjs';
2121
import { REFUSAL_MARKER } from './browser-guard.mjs';
2222
import { CHILD_OUTPUT_LIMIT, MAX_PAGE_BYTES, outputOverflow } from './runtime.mjs';
23+
import { loadConfig } from './machine.mjs';
2324

2425
const GUARD_CHILD = fileURLToPath(new URL('./browser-guard.mjs', import.meta.url));
2526

@@ -252,10 +253,12 @@ export function command(argv) {
252253
return ['browser', ...argv.slice(-1)].map((p) => (/\s/.test(p) ? JSON.stringify(p) : p)).join(' ');
253254
}
254255

255-
export function scrape(url, { render = renderGuarded, browserPath = null, env = process.env, config = {}, exists, timeout = TIMEOUT_MS, uid,
256+
export function scrape(url, { render = renderGuarded, browserPath = null, env = process.env, config = null, exists, timeout = TIMEOUT_MS, uid,
256257
allowInternalRedirects } = {}) {
257258
const target = String(url);
258-
const binary = browserPath ?? findBrowser({ config, env, exists });
259+
// No config from the caller means the machine's: the collector calls runScrape(url) with no
260+
// options, and an empty default here meant browserPath was never consulted (2026-10-11).
261+
const binary = browserPath ?? findBrowser({ config: config ?? loadConfig(env), env, exists });
259262
const cmd = command([target]);
260263
if (!binary) {
261264
return { ok: false, url: target, transport: name, cmd,
@@ -402,8 +405,8 @@ export function map(url, opts = {}) {
402405
return httpKeyless.map(url, opts);
403406
}
404407

405-
export function status({ config = {}, env = process.env, exists } = {}) {
406-
const path = findBrowser({ config, env, exists });
408+
export function status({ config = null, env = process.env, exists } = {}) {
409+
const path = findBrowser({ config: config ?? loadConfig(env), env, exists });
407410
return { ok: Boolean(path), transport: name, path, error: path ? '' : 'no Chromium or Chrome found' };
408411
}
409412

‎research-kit/lib/machine.mjs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export const DEFAULTS = Object.freeze({
8383
skillRoots: Object.freeze([]),
8484
projectSkillDir: '',
8585
maxAgeDays: 180,
86+
browserPath: '',
8687
});
8788

8889
export const ROLES = Object.freeze(['collector', 'builder']);
@@ -168,6 +169,10 @@ function shape(raw) {
168169
skillRoots: Array.isArray(raw.skillRoots) ? raw.skillRoots.filter((root) => typeof root === 'string' && root.trim() !== '') : [],
169170
projectSkillDir: typeof raw.projectSkillDir === 'string' ? raw.projectSkillDir : '',
170171
maxAgeDays: Number.isFinite(raw.maxAgeDays) ? raw.maxAgeDays : DEFAULTS.maxAgeDays,
172+
// The browser transport's executable (ADR-0088), which its own errors tell the operator
173+
// to set. It was missing here until 2026-10-11, so it was never read, and every save -
174+
// `install-hooks --role` among them - rewrote the file without it.
175+
browserPath: typeof raw.browserPath === 'string' ? raw.browserPath : DEFAULTS.browserPath,
171176
};
172177
// A retired key is READ before it is dropped: a rename that silently relaxes a gate is
173178
// the same defect as a rename that silently stops firing one.

‎research-kit/lib/release-validator.mjs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,7 @@ function loadSchema(schemaDir, name) {
971971
return loaded.error ? { value: null, error: loaded.error } : loaded;
972972
}
973973

974-
export function validateRelease({ root, package: packageName, registryPath, rolesPath, pointersDir, schemaDir = DEFAULT_SCHEMA_DIR, ledgerPath = null, headPath = null, genesisHash = '0'.repeat(64), publicKeys = {}, now = Date.now(), snapshotMaxAgeMs = SNAPSHOT_MAX_AGE_MS } = {}) {
974+
export function validateRelease({ root, package: packageName, registryPath, rolesPath, pointersDir, schemaDir = DEFAULT_SCHEMA_DIR, ledgerPath = null, headPath = null, genesisPath = null, checkpointsDir = null, genesisHash = '0'.repeat(64), publicKeys = {}, now = Date.now(), snapshotMaxAgeMs = SNAPSHOT_MAX_AGE_MS } = {}) {
975975
const errors = [];
976976
const output = {
977977
validatorVersion: VALIDATOR_VERSION,
@@ -1012,6 +1012,11 @@ export function validateRelease({ root, package: packageName, registryPath, role
10121012
if (pointersDir && fs.existsSync(pointersDir) && !inside(root, pointersDir)) add(errors, 'PATH-CONTAINMENT', 'pointer directory escapes record root', pointersDir);
10131013
if (ledgerPath && (!fs.existsSync(ledgerPath) || !inside(root, ledgerPath))) add(errors, 'LEDGER-PATH', 'ledger path must exist below record root', ledgerPath, 'INCOMPLETE');
10141014
if (headPath && (!fs.existsSync(headPath) || !inside(root, headPath))) add(errors, 'LEDGER-PATH', 'head path must exist below record root', headPath, 'INCOMPLETE');
1015+
if (genesisPath && (!fs.existsSync(genesisPath) || !inside(root, genesisPath))) add(errors, 'LEDGER-PATH', 'genesis path must exist below record root', genesisPath, 'INCOMPLETE');
1016+
if (checkpointsDir && (!fs.existsSync(checkpointsDir) || !inside(root, checkpointsDir))) add(errors, 'LEDGER-PATH', 'checkpoints path must exist below record root', checkpointsDir, 'INCOMPLETE');
1017+
// Only the ledger branch reads them: given without --ledger they would be dropped in silence,
1018+
// which is how both flags went unread before 2026-10-11.
1019+
if ((genesisPath || checkpointsDir) && !ledgerPath) add(errors, 'LEDGER-PATH', 'genesis and checkpoints are verified only with --ledger', genesisPath ?? checkpointsDir, 'INCOMPLETE');
10151020
if (!artifacts.length) add(errors, 'INPUT-ARTIFACTS', `registry has no artifacts for ${packageName}`, '.artifacts', 'INCOMPLETE');
10161021
if (!HASH.test(String(roster.rosterSha256 ?? ''))) add(errors, 'ROLE-ROSTER', 'role roster hash is invalid', '.rosterSha256');
10171022
const records = new Map();
@@ -1070,6 +1075,8 @@ export function validateRelease({ root, package: packageName, registryPath, role
10701075
pointer: pointer.value,
10711076
publicKeys,
10721077
genesisHash,
1078+
genesisPath,
1079+
checkpointsDir,
10731080
expectedBenchmarkSpecSha256: registry.benchmarkSpecSha256,
10741081
expectedRoleRosterSha256: registry.roleRosterSha256,
10751082
genesisSchemaPath: path.join(schemaDir, 'qualification-ledger-genesis.schema.json'),

‎research-kit/test/browser-transport.test.mjs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,24 @@ test('the recorded command names the browser, not the keyless transport', () =>
225225
assert.doesNotMatch(none.cmd, /http-keyless/, 'the refusal carries the same command');
226226
});
227227

228+
// Found 2026-10-11: the collector calls runScrape(url) with no options, so the adapter looked
229+
// for the browser with an empty config, and browserPath - the setting its own error message
230+
// names - was never consulted. A caller that passes no config gets the machine's.
231+
test('a scrape with no config of its own runs the browserPath the machine config names', () => {
232+
const dir = tempDir('rk-browser-config-');
233+
const binary = path.join(dir, 'chrome');
234+
fs.writeFileSync(binary, '', 'utf8');
235+
const configFile = path.join(dir, 'research-kit.config.json');
236+
fs.writeFileSync(configFile, JSON.stringify({ browserPath: binary }), 'utf8');
237+
const env = { RESEARCH_KIT_CONFIG: configFile };
238+
const { render, calls } = renderWith({ stdout: PAGE });
239+
const r = browser.runScrape('https://nodejs.org/api/fs.html', { render, env });
240+
assert.equal(r.ok, true, r.error);
241+
assert.equal(calls[0]?.file, binary, 'the configured browser ran');
242+
assert.equal(browser.status({ env }).path, binary, 'status reports the same browser');
243+
assert.equal(browser.status({ env, config: {} }).path === binary, false, 'an explicit config still wins');
244+
});
245+
228246
test('Chromium\'s own error page is a failure that names its code, though it exits 0', () => {
229247
assert.equal(chromeErrorOf(PRIVACY), 'ERR_CERT_AUTHORITY_INVALID');
230248
assert.equal(chromeErrorOf(PAGE), '');

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,17 @@ test('saveConfig drops the retired key, so the migration happens through normal
147147
assert.equal(written.role, 'builder');
148148
});
149149

150+
// Found 2026-10-11: the browser transport and its error messages tell the operator to set
151+
// browserPath (ADR-0088), but the config was read without it and rewritten without it, so
152+
// `install-hooks --role builder` erased a browserPath the operator had set by hand.
153+
test('browserPath is read from the machine config and survives a save', () => {
154+
const { env, file } = envWith(JSON.stringify({ browserPath: '/opt/google/chrome/chrome' }));
155+
assert.equal(loadConfig(env).browserPath, '/opt/google/chrome/chrome');
156+
saveConfig({ role: 'builder' }, env);
157+
assert.equal(JSON.parse(readText(file)).browserPath, '/opt/google/chrome/chrome', 'a save kept the key');
158+
assert.equal(loadConfig(envWith(JSON.stringify({ browserPath: 42 })).env).browserPath, '', 'a path that is not a string is not one');
159+
});
160+
150161
test('a retired ENVIRONMENT variable is reported and never honoured', () => {
151162
const { env } = envWith(null, { extra: { CLAUDE_SETTINGS_PATH: '/somewhere/settings.json' } });
152163
const notes = retiredEnvNotes(env);

‎research-kit/test/release-validator.test.mjs‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,117 @@ test('nested promotion pointers require a committed promotion and verify pointer
336336
}
337337
});
338338

339+
// A one-artifact R28 release with a promotion pointer, so validateRelease reaches its
340+
// ledger branch. Its own findings (an uncommitted promotion, no snapshot) are not what the
341+
// tests below look at.
342+
async function writePointerRelease(root) {
343+
const { payloadHash, sha256, canonicalJson, recordHash } = await import('../lib/release-validator.mjs');
344+
const target = envelope({ artifactId: 'R28-04', payload: { manifest: true }, ownerRole: 'evidence-custodian' });
345+
target.payloadSha256 = payloadHash(target.payload);
346+
const pointerMeta = envelope({ artifactId: 'R28-04', payload: undefined, ownerRole: 'evidence-custodian' });
347+
delete pointerMeta.payload;
348+
const pointer = {
349+
envelope: pointerMeta,
350+
payload: {
351+
pointerVersion: '1.0.0',
352+
promotion: {
353+
state: 'ready', package: 'R28', generation: 1, targetArtifactId: 'R28-04',
354+
targetHash: target.payloadSha256,
355+
predecessorClosureHash: sha256(canonicalJson([])), requiredGate: 'Q0',
356+
publishedAt: '2026-01-01T00:00:00Z', previousPointerHash: null, revokedDescendantHashes: [],
357+
},
358+
},
359+
};
360+
pointer.envelope.payloadSha256 = payloadHash(pointer.payload);
361+
pointer.envelope.recordHash = recordHash(pointer);
362+
writeJson(path.join(root, 'records', 'R28-04.json'), target);
363+
writeJson(path.join(root, 'pointers', 'R28-04.json'), pointer);
364+
const registry = registryFor('R28-04');
365+
registry.artifacts[0].ownerRoles = ['evidence-custodian'];
366+
registry.artifacts[0].pointer = true;
367+
const roles = roster();
368+
roles.assignments[0].roleId = 'evidence-custodian';
369+
writeJson(path.join(root, 'registry.json'), registry);
370+
writeJson(path.join(root, 'roles.json'), roles);
371+
// A single .qlog FILE, not a ledger directory: a directory would supply its own
372+
// genesis.json and checkpoints/, and hide whether the explicit paths were read at all.
373+
fs.mkdirSync(path.join(root, 'ledger'), { recursive: true });
374+
fs.writeFileSync(path.join(root, 'ledger', 'ledger.qlog'), '', 'utf8');
375+
return {
376+
root, package: 'R28',
377+
registryPath: path.join(root, 'registry.json'),
378+
rolesPath: path.join(root, 'roles.json'),
379+
pointersDir: path.join(root, 'pointers'),
380+
ledgerPath: path.join(root, 'ledger', 'ledger.qlog'),
381+
};
382+
}
383+
384+
// Found 2026-10-11: `researcher-release validate` advertised --genesis and --checkpoints and
385+
// parsed them, but validateRelease had no such parameters, so both were dropped before the
386+
// ledger was verified. An operator who supplied a signed genesis believed it was checked.
387+
test('validate verifies the genesis and checkpoints it is given, from the library and the CLI', async () => {
388+
const { validateRelease } = await import('../lib/release-validator.mjs');
389+
const root = tempRoot();
390+
try {
391+
const inputs = await writePointerRelease(root);
392+
const genesisPath = path.join(root, 'anchors', 'genesis.json');
393+
const checkpointsDir = path.join(root, 'anchors', 'checkpoints');
394+
// Unsigned anchors: well-formed JSON, so they are read and then fail verification.
395+
writeJson(genesisPath, { ledgerVersion: '1.0.0', releaseId: 'release-1' });
396+
writeJson(path.join(checkpointsDir, 'checkpoint-1.json'), { ledgerVersion: '1.0.0', releaseId: 'release-1' });
397+
const result = validateRelease({ ...inputs, genesisPath, checkpointsDir });
398+
assert(result.errors.some((error) => error.code === 'LEDGER-GENESIS-SIGNATURE'), `the genesis was not verified: ${JSON.stringify(result.errors)}`);
399+
assert(result.errors.some((error) => error.code === 'LEDGER-CHECKPOINT-SIGNATURE' && error.path.startsWith('.checkpoints[0]')), `the checkpoint was not verified: ${JSON.stringify(result.errors)}`);
400+
assert(result.status !== 'PASS', JSON.stringify(result));
401+
402+
const run = spawnSync(process.execPath, [
403+
'research-kit/bin/researcher-release.mjs', 'validate',
404+
'--root', root, '--package', 'R28', '--registry', inputs.registryPath,
405+
'--roles', inputs.rolesPath, '--pointers', inputs.pointersDir,
406+
'--ledger', inputs.ledgerPath, '--genesis', genesisPath, '--checkpoints', checkpointsDir, '--json',
407+
], { encoding: 'utf8' });
408+
assert(run.status !== 0, `an unsigned genesis passed: ${run.stdout}`);
409+
const codes = JSON.parse(run.stdout).errors.map((error) => error.code);
410+
assert(codes.includes('LEDGER-GENESIS-SIGNATURE'), `the CLI dropped --genesis: ${codes.join(', ')}`);
411+
assert(codes.includes('LEDGER-CHECKPOINT-SIGNATURE'), `the CLI dropped --checkpoints: ${codes.join(', ')}`);
412+
} finally {
413+
cleanup(root);
414+
}
415+
});
416+
417+
test('validate refuses a genesis or checkpoints path it would not read', async () => {
418+
const { validateRelease } = await import('../lib/release-validator.mjs');
419+
const root = tempRoot();
420+
try {
421+
const inputs = await writePointerRelease(root);
422+
const refusal = (result, word) => result.errors.find((error) => error.code === 'LEDGER-PATH' && error.message.includes(word));
423+
424+
const missingGenesis = validateRelease({ ...inputs, genesisPath: path.join(root, 'no-such-genesis.json') });
425+
assert(refusal(missingGenesis, 'genesis'), `a missing genesis was ignored: ${JSON.stringify(missingGenesis.errors)}`);
426+
427+
const missingCheckpoints = validateRelease({ ...inputs, checkpointsDir: path.join(root, 'no-such-checkpoints') });
428+
assert(refusal(missingCheckpoints, 'checkpoints'), `a missing checkpoints directory was ignored: ${JSON.stringify(missingCheckpoints.errors)}`);
429+
430+
const outside = tempRoot();
431+
try {
432+
const outsideGenesis = path.join(outside, 'genesis.json');
433+
writeJson(outsideGenesis, { ledgerVersion: '1.0.0' });
434+
const escaped = validateRelease({ ...inputs, genesisPath: outsideGenesis });
435+
assert(refusal(escaped, 'genesis'), `a genesis outside the record root was accepted: ${JSON.stringify(escaped.errors)}`);
436+
} finally {
437+
cleanup(outside);
438+
}
439+
440+
// Without --ledger nothing verifies a ledger, so a genesis given anyway would be dropped.
441+
writeJson(path.join(root, 'anchors', 'genesis.json'), { ledgerVersion: '1.0.0' });
442+
const noLedger = validateRelease({ ...inputs, ledgerPath: null, genesisPath: path.join(root, 'anchors', 'genesis.json') });
443+
assert(refusal(noLedger, '--ledger'), `a genesis without a ledger was ignored: ${JSON.stringify(noLedger.errors)}`);
444+
assert(noLedger.status !== 'PASS', JSON.stringify(noLedger));
445+
} finally {
446+
cleanup(root);
447+
}
448+
});
449+
339450
test('descendant invalidation attributes multiple roots to each affected pointer', async () => {
340451
const { computeDescendantInvalidation } = await import('../lib/release-validator.mjs');
341452
const rootA = 'a'.repeat(64);

0 commit comments

Comments
 (0)