Skip to content

Commit e3bca63

Browse files
committed
The browserPath machine-config key is read, and kept on save
What changed: lib/machine.mjs carries browserPath in DEFAULTS ('') and in shape(), so loadConfig returns it and saveConfig writes it back; a value that is not a string falls back to ''. lib/browser-transport.mjs: scrape and status take config = null and, when the caller passes none, read the machine's with loadConfig(env). An explicit config, or an explicit browserPath option, still wins. Why: ADR-0088 orders the browser lookup as browserPath in the machine config, then RESEARCH_KIT_BROWSER, then the usual paths, and the transport's own errors tell the operator to "set browserPath in the machine config". But shape() never copied the key, so loadConfig dropped it: research.mjs's auto-detect never saw it, and the collector's runScrape(url), called with no options, searched with an empty config. saveConfig rewrites the file from the shaped settings, so any save (install-hooks --role among them) erased a browserPath set by hand. The key was already promised, so this is a bug fix, not a new key under the feature freeze (ADR-0117). Alternative set aside: threading the config through every runScrape caller (collect.mjs, research-run.mjs, decompose.mjs). That is three call sites to keep in step, against one default where the adapter is, the pattern transport.mjs already uses (config ?? loadConfig(env)). No ADR: one obvious remedy. What it touched (the two map rows were re-applied over main's rewrite of them in the rebase): research-kit/lib/machine.mjs, research-kit/lib/browser-transport.mjs (imports loadConfig from machine.mjs; machine.mjs imports only core.mjs, so there is no cycle); research-kit/test/machine.test.mjs and research-kit/test/browser-transport.test.mjs (one test each); docs/ARCHITECTURE.md (the machine.mjs and browser-transport.mjs rows); README.md (the machine-config table gains its browserPath row); CHANGELOG.md (Unreleased); research-kit/README.md (test count 1782 -> 1784). What you verified: - verified: both new tests failed before the fix (selftest.mjs machine browser-transport, cwd /home/user/rk-fix: 55 passed, 2 failed; loadConfig gave undefined, and the scrape ran /opt/pw-browsers/chromium instead of the configured binary) and pass after it (57 passed). - verified: full suite after rebasing both commits onto d6035da, cwd /home/user/rk-fix, Node v22.22.0 on Linux: 1782 passed, 2 failed, 1784 in all, the count now in the README. The two failures are the runtime G7 windows-1252 tests that fail identically on unmodified main: this host's TextDecoder('windows-1252') decodes 0x80 as U+0080. - expected: existing browser tests pass browserPath or config explicitly, so the new loadConfig default is not reached in them; the full run above agrees. - untested: Windows and the Node 24/26 CI legs. What you got wrong and fixed: nothing to report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011D7RsVpgAUVb6VqZpjoV71
1 parent ea4f9c8 commit e3bca63

8 files changed

Lines changed: 50 additions & 7 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ 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+
813
- `researcher-release validate --genesis` and `--checkpoints` are verified. Both were
914
advertised and parsed, but `validateRelease` had no parameters for them, so they were
1015
dropped before the ledger check and an operator who supplied a signed genesis believed it

‎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: 2 additions & 2 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-
1782 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/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);

0 commit comments

Comments
 (0)