From a6d6328bb0531b02d6ab6fd5f1660fc6349b74df Mon Sep 17 00:00:00 2001 From: StepenkoAnatoli Date: Wed, 14 Oct 2026 14:57:39 +0300 Subject: [PATCH] fix(config): preserve raw declarations when saving a patch Fixes #364 what changed: saveConfig patches a raw JSON object or its usable last-good snapshot, retaining unpatched role and posture values for diagnostics. A broken config with no usable snapshot refuses an unrelated save with CONFIG-UNREADABLE; an explicit valid role declaration can repair it. install-hooks names this refusal with exit 2, and the panel key endpoint returns 409 without echoing the key. Existing retired-key migration remains. why: Saving an unrelated setting persisted resolved role unknown and restrictive posture values, destroying the operator's original typo and misrepresenting fallback state as a declaration (#364). A broken unsnapshotted file could be replaced while the CLI or panel reported success. Preserve declarations and refuse a save that has no usable raw state; no alternative design was set aside. what it touched: machine config saving, install-hooks error handling, panel key handling, four machine regressions and one panel regression, the machine/panel/CLI architecture descriptions, changelog and README test inventory (1843 on main 2d807159). No new configuration option or format. what you verified: verified - baseline machine/panel had 46 passing, five expected regression failures and two unsupported symlink cases. After the fix, machine/panel/doctor/architecture-map ran 138 passing checks, zero failures and two unsupported symlink checks in85.8s on Windows Node24.20.0. The direct runner exited1 because unsupported capabilities were unwaived, not because a test failed; its structured result and log are retained. The normal commit gate's existing local unsupported-capability waiver applies to its full suite, while CI blocks unsupported results. Syntax checks and diff checks clean. Handoff OK, preflight PASS (28 passing, one existing warning), doctor READY collector/firecrawl-cli with existing deployment/edit-registration warnings. After refresh onto merged main 82a02cb, the focused machine/panel/doctor/architecture-map run passed 147 checks with zero failures and two unsupported symlink cases in 93.9 seconds. Its documented local allowance produced exit 0 while retaining those unsupported results; CI blocks unsupported cases. The patch was advanced to main 2d807159 after PR412 landed; staged code/test blobs match the focused-tested source and the shared documentation carries the 1,843-test inventory. The normal full gate and GitHub CI follow before merge. what you got wrong and fixed: mistake - a documentation conflict helper assumed the panel addition survived selecting the main conflict block; where - refreshing docs/ARCHITECTURE.md onto merged main; impact - its preservation guard refused before writing a resolved map, with no published result affected; cause - the conflict block included the neighboring panel row too; fix - explicitly retained both new config-saving and panel-refusal descriptions alongside the current map; verified - no unmerged paths, clean staged whitespace and syntax checks. Current-base focused verification passed 147 checks with zero failures and two unsupported symlink cases; the full normal gate follows. --- CHANGELOG.md | 6 ++++ docs/ARCHITECTURE.md | 6 ++-- research-kit/README.md | 2 +- research-kit/bin/install-hooks.mjs | 2 +- research-kit/lib/machine.mjs | 32 +++++++++++++++-- research-kit/lib/panel.mjs | 6 +++- research-kit/test/machine.test.mjs | 56 ++++++++++++++++++++++++++++++ research-kit/test/panel.test.mjs | 15 ++++++++ 8 files changed, 117 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb022486..0674799f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in ## Unreleased +- Machine config saves preserve unpatched raw role and posture values, including typos + that diagnostics must still report. An unreadable config without a usable last-good + snapshot refuses an unrelated save rather than persisting the restrictive fallback; + an explicit valid `--role` can repair it. `install-hooks` names the refusal with exit 2 + and the panel key endpoint returns HTTP 409 without echoing the key (#364). + - `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). - `property-replay --all` refuses a missing regression directory with exit 2 and diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8b59c3af..8cd149db 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -226,10 +226,10 @@ is an open decision (2026-09-30): | `preflight.mjs` | **The verdict** and the **verdict context**. Assembles findings from CHECKS into pass/fail; one function, three callers (commit gate, edit gate, CLI) so they cannot disagree. Owns the verdict's input unit: `verdictContext(root)` reads the corpus, the gate state (`readGateState`), and the operator's `evidencePolicy` together — same snapshot, same verdict, by construction. | ADR-0004 | | `gate.mjs` | Gate policy: `isGated` (the four markers), `evaluate` (commit/edit), the GATE_OFF override recording, **what phase 1 may commit** (ADR-0048: `research/`, the root `SCAFFOLDING` - `.gitattributes`, `.gitignore`, `AGENTS.md`, `START_HERE.md` - and `docs/ARCHITECTURE.md` while `isEmptyArchitectureMap` says its staged bytes, read by `readStagedText`, are still the template's; until 2026-09-27 only `research/`, so a new project's first `git add -A` was refused and its `.gitattributes` could not travel), and the architecture-map rule (`architectureMapBreach` over `loadGateConfig` — guarded paths are project configuration, `research/kit.json`, defaults `src/lib/bin/scripts/app`). Contains no repo-specific path. **Staged paths remain data, with checked acquisition** (ADR-0154): the default commit CLI lazily calls `readStagedPaths` after the ungated and `GATE_OFF` returns. One actual `git diff --cached --name-only --no-renames -z` supplies paths only on successful complete output; the existing 64 MiB capture rejects nonzero exit and buffer failure, and the reader rejects malformed NUL data. Successful zero bytes means `[]`; unreadable scope means null and raises the existing architecture-map breach even after green preflight, ordinary exit 1 under either posture. Explicit arrays/null and the existing stdin/repeated `--staged` options retain their meaning; edits never acquire a list. **One deliberate exception to "no CLI parsing"** remains for explicitly supplied stdin data (`stagedPathsFromStdin` / `splitPathList`), because argv was O(n²) to build and bounded besides. It returns **null for "stdin could not be read" and `[]` for "read, and empty"**, preserving unknown scope rather than assuming an empty set. No list read claims an atomic snapshot with later index materialization. `stdinIsReadable()` asks `isatty(0)` rather than inferring from the file type, which gets two cases wrong in opposite directions (a socketpair is not a FIFO; `/dev/null` is a character device but reads as immediate EOF). The architecture-map block's fix says to **update** the map, then `git add` it (2026-09-27): a bare `git add docs/ARCHITECTURE.md` stages nothing while the map is unchanged, so the printed fix left the commit blocked. `bin/gate.mjs` no longer closes that block with "Phase 1 is not done until preflight prints PASS" - a map block happens only when the gate passes. `isPhaseOneEdit(root, file)` answers the same question for the edit hook, about one file. The empty-index verdict — a gated project whose `research/` is untracked, every fresh project's first commit — removes the scratch dir `materializeIndex` made before it learned the index was empty (2026-09-28): it returned before `scratch` was assigned, so the `finally` that removes it never ran, and every such verdict leaked one temp directory. **Integrity before scope** (ADR-0093): a failing verdict whose findings include an `INTEGRITY_RULES` provenance rule blocks the commit even when it is confined to `research/` - the ADR-0048 allowance covers completeness findings only. `stdinIsReadable(stdin = process.stdin)` takes the stream to ask about, so the hook's terminal case is testable (2026-10-01). **The suite rule** (ADR-0120, 2026-10-01): `suiteBreach(root, stagedPaths, { run })` runs after the map rule, in the commit gate only, and only where `isKitCheckout` finds the kit's four markers and a staged path is under `research-kit/` (or the list is unknown); `runSuiteHere` spawns the checkout's `bin/selftest.mjs` with `RESEARCH_KIT_RESULT_FILE` pointed at a scratch file and `RESEARCH_KIT_ALLOW_UNSUP=1` (unsupported is not red, ADR-0108), git's hook environment stripped (`GIT_INDEX_FILE`, `GIT_DIR`, `GIT_PREFIX` and kin - the same `REPOSITORY_LOCATION` list as the edit hook, else every scratch repository's git in the suite would act on the index being committed), and reads the file, never stdout: failures block naming the counts, no result blocks as a crashed runner, a run past its budget is stopped and blocks as unfinished, the fix naming `RESEARCH_KIT_GATE_TIMEOUT` before the override, and a green run whose runner exited non-zero (the README count check) blocks too. `suiteOwed(root, stagedPaths)` is the predicate alone; `suiteBreach`'s `announce` (`evaluate`'s `announceSuite`) is called right before the run, so `bin/gate.mjs` says the suite is running only when it is - not when the map rule or preflight refused the commit first. `evaluate` takes `runSuite` so the verdict is testable without a suite. **The suite's budget follows the gate's watchdog** (ADR-0142, 2026-10-05): `suiteBudgetMs(env)` is `SUITE_TIMEOUT_MS` (65 min, twice the slowest honest run measured, 1825 s on the operator's Windows PC) unless `RESEARCH_KIT_GATE_TIMEOUT` is set to a positive duration `gateTimeoutSeconds` reads (seconds, an optional fraction, an optional `s`/`m`/`h`/`d`, at most nine digits before the point - the grammar the hook's `gate_timeout_readable` checks too; `0` and anything outside it leave the default), and then that watchdog less `GATE_REST_S` (120 s, the hook's default watchdog for a gate that runs no suite), never more than half a short one, at least 1 ms and at most 2^31 - 1 - so the gate stops the suite and blocks before the hook's watchdog fires, whenever its own work fits what is left. `runSuiteHere` stops the runner with SIGKILL: the harness's SIGTERM listener kept a synchronously blocked runner alive past its budget on POSIX; a stopped run's scratch is left in the temp folder. The budget had been 20 minutes, which that PC's green suite met on every kit commit, and the variable had reached only the watchdog: set below the budget, the watchdog killed the gate first and a fail-open machine allowed a commit whose suite never reported. **The fix lines no longer say `--no-verify` is recorded** (2026-10-03, outside audit): git skips this hook, so nothing can record it; `checks.mjs` had said so since ADR-0035 while the gate's own fix text, the README, AGENTS.md and CONTEXT.md said the opposite. **Red suite diagnostics** (2026-10-08): `suiteBreach` forwards up to five `failed` identifiers, each limited to 160 characters with controls normalized, in its existing detail string; omitted or clipped data is named, and legacy missing labels are explicit. Counts and verdict keys are unchanged. `runSuiteHere` still removes its scratch result; no result retention system is added. | ADR-0002, ADR-0007, ADR-0008, ADR-0020, ADR-0093, ADR-0120, ADR-0142, ADR-0154 | | `doctor.mjs` | Read-only diagnostics: machine + project + gate + chain health in one report. Consumes the verdict context (`verdictContext` / `readGateState`) for the chain-verified corpus and the gate-state triple; adds one machine read of its own (the git-resolved `hooksPathEffective`) for report. **Role-aware** (ADR-0010): the Firecrawl findings are informational on a `builder`, and the handoff findings (ADR-0011) are blockers there — the collection entrypoints' refusal is what makes that split honest. The handoff findings come from the owner, not from a second composition: doctor passes the corpus it already read to `verifyHandoff(root, { corpus })`, so the arrival question has one implementation and one set of pins. One deliberate write: the repository-local hooksPath override log (the third override is silent by nature — the bypassed hook cannot report it). `gateHealth` and `runDoctor` accept explicit Git/config/settings paths and a child environment so diagnostics can run against disposable machine fixtures without consulting host state. **`commitGateState`** asks WHOSE pre-commit is at `core.hooksPath` - `current` / `foreign` / `unusable` - because executable is not the same as ours. It used to ask only whether some executable hook was there, the same existence-for-correspondence mistake the `deploy` check made; `settingsState` has asked the question of the EDIT gate since a registration here named our hook inside a different checkout, and `core.hooksPath` is a single machine-wide setting that husky, lefthook and pre-commit all rewrite. Pure and separate from `gateHealth`, which reads real git config - a rule testable only by fishing a finding out of an array on a host-dependent path is a rule nobody tests (ADR-0004), which is why the older test for it asserts the severity is one of three. **Ownership is decided before executability**, because the first version asked about the mode first and so reported a non-executable FOREIGN hook as `unusable` with `chmod +x` beside it - advice to repair somebody else's gate while leaving this one just as absent. That ordering bug was invisible on Windows, where `hookExecutability` returns ok for any file that exists, and surfaced on Linux in CI. On a **collector**, handoff findings are `info` in the collector's words ("does not exist yet - nothing has been collected here; a builder handed this corpus would be refused"), since 2026-09-26: they were graded `pass` with the builder's wording, so a fresh project printed "pass ... the corpus arrived without its ledger". On a builder they remain blockers, worded as before. It reports the **Node line** and, when a proxy variable is set, whether this Node sends the kit's requests through it A collector without the Firecrawl CLI is told the exact install command, `npm install -g firecrawl-cli@` from `cliInstallSpec()` (2026-09-27; it said "install the Firecrawl CLI" and nothing anywhere said how). (`proxy`: keyless pages, search requests and remote collection; a warning naming the version to move to - or, for a value Node cannot parse, the spelling it needs - since the Firecrawl CLI has its own proxy handling) - measured on 22.20.0, where the fetch went around the proxy (ADR-0046, ADR-0047). A collector that explicitly chose a keyless or browser transport passes the Firecrawl checks; nothing chosen still fails (ADR-0095). The Firecrawl fixes (`firecrawl-cli`, `firecrawl-auth`) name the no-account route beside the install or login - `"transport": "http-keyless"` in the machine config, the setting that makes them pass (ADR-0095) - where the CLI fix had offered only a per-run flag and the login fix nothing (2026-09-29). `editGateState` reads an entry whose `hooks` is not a list, or a hook that is `null`, as naming nothing (2026-09-30); `[null]` had thrown a `TypeError`. `scanForSecrets` catches a folder it cannot read where it reads it, and skips a folder it has already walked by real path (2026-09-30): one try/catch around the whole walk had ended the scan at the first unreadable folder, leaving every file after it unscanned and reporting nothing found. `scanForSecrets` records a cap stop separately from the scanned count (2026-10-08, bounded-scan audit repair): it keeps the 2000-text-file limit and reports incomplete coverage only when that cap actually prevents further traversal; ending at the final file's boundary is not itself incomplete. `runDoctor` reports a capped scan as a `secret-scan` warning, beside any unchanged critical `secret` hits, instead of reporting a capped no-hit scan as passing. It names no unobserved remaining-file count and makes no whole-repository clearance claim. `bin/doctor.mjs` reports `KIT_VERSION`, as `kitVersion` in `--json` and on its last line (ADR-0116). Reports each ill-typed posture key as a `config-ill-typed` warning naming the key, the values the kit reads, and the state it resolved to (ADR-0121). The `firecrawl-cli` finding judges the installed version with the run's own rule (`cliCompatibility`): an unsupported major is a **fail** with the remedy on a collector that needs the CLI and information elsewhere, a newer minor passes and says what it was tested against - doctor had printed `pass 2.0.0` for a CLI the run then refused (2026-10-02, break-test pass 3). `scanForSecrets` skips a binary file and counts it (`skippedBinary`, named in the coverage line) instead of matching credential patterns against an executable's string table (ADR-0128, 2026-10-02): run on a Windows home folder it reported fourteen committed keys, every one the PEM marker OpenSSH and libssh2 parse, or random bytes spelling `sk-`. **`handOnState`** (2026-10-03, ADR-0134; since 2026-10-04 defined in `installer.mjs` and re-exported here, ADR-0141, so the install judges what it records by the same rule): what the hooks hand on to - none, missing, kit, other - and `gateHealth` warns on kit and missing; the maintainer's machine handed every commit on to the previous implementation's gate, which crashed and failed open while doctor said READY. `handOnState` resolves a relative folder from the repository's top level (`base`), where git runs its hooks from; doctor had resolved `.custom-hooks` against its own working directory and from a subdirectory called a working hand-on missing (fourth audit round, 2026-10-03). `scanForSecrets` skips `research/raw` at ANY depth (2026-10-04): the skip matched the relative path `research/raw`, so only the root's captures were outside the scan and a nested decision project's were read as the project's own files - CI tripped `google-api-key` on a Google page's source sibling (ADR-0140), Google's own public keys; a capture is page content, and the folder exists wherever a nested project does (ADR-0030). The shared unusable-proxy check also rejects proxy URLs carrying a non-root path, query or fragment, so those configurations warn instead of promising proxy use (#380, 2026-10-12). `machineHealth` uses the shared `envProxyChoice` with injectable `execArgv` (#381, 2026-10-12). An explicit disabled choice warns that requests go direct and names how to enable the proxy, rather than promising proxy use from Node version support alone; the raw variable value is never echoed. | ADR-0010, ADR-0011, ADR-0095, ADR-0134 | -| `panel.mjs` | **The desktop panel** (ADR-0147, 2026-10-07): `createPanel({ project, env, kitRoot, kitHome })` serves `research-kit/panel/` (one page, a script, a stylesheet) and a small JSON API on **127.0.0.1 only**, a thin shell over commands that already exist. It owns no verdict: doctor, handoff, preflight and install run as children with the fixed argument lists in `PANEL_COMMANDS`, in the chosen project folder, and their output is shown with the search key redacted. **No command from the page collects** - `research.mjs` and `decompose.mjs` are absent from `PANEL_COMMANDS`; the one way pages are fetched through the panel is auto-collect (`lib/auto-collect.mjs`, ADR-0148), for a builder's validated request, on a collector, within the operator's caps. Its routes are `GET /api/requests` (the queue and the meter), `POST /api/autocollect` (the settings), `/api/requests/check` and `/api/requests/resume`. It writes two things: the search provider's key into the machine config through `transport.mjs`'s `SEARCH_KEY` (so the vendor's name stays in the adapter and the registry, NFR-3), refused when the config resolves inside a git repository (Rule 6); and which project it shows, a full path to an existing folder, never searched for. The topic and `BRIEF.md` are read, never written (ADR-0056). `builderInstructions` is the hand-off text: the collector's push, ledger included, then the builder's own steps in `$HOME` spelling (ADR-0050) - declare the role, handoff, preflight, inspect an authored complete brief and its current inputs, never collect. Its readiness checklist requires classified map rows and rewritten Findings; a file-presence observation grants no build authorization. Commit-report placeholders require actual observed results. A collected Git-request result waits for the collector to record review/closure before dependent building. Every request passes four checks: the bound address, a Host header naming it (DNS rebinding), an Origin, when sent, that is its own, and for `/api/` the per-launch token, initialized through the launch URL fragment and required in x-panel-token. The launcher prints that capability URL; terminal-log and browser-history privacy are not guaranteed by this module. POSTs must be JSON, so a plain cross-site form is refused. Update is refused when the panel runs from the installed copy, which has nothing newer to install. Command runs, the project switch and auto-collect cycles share one lock (`running`, handed to `createAutoCollect` as `lock`): none runs beside another, so no preflight or update reads the corpus or the kit tree while a cycle writes it. | ADR-0147, ADR-0148 | +| `panel.mjs` | **The desktop panel** (ADR-0147, 2026-10-07): `createPanel({ project, env, kitRoot, kitHome })` serves `research-kit/panel/` (one page, a script, a stylesheet) and a small JSON API on **127.0.0.1 only**, a thin shell over commands that already exist. It owns no verdict: doctor, handoff, preflight and install run as children with the fixed argument lists in `PANEL_COMMANDS`, in the chosen project folder, and their output is shown with the search key redacted. **No command from the page collects** - `research.mjs` and `decompose.mjs` are absent from `PANEL_COMMANDS`; the one way pages are fetched through the panel is auto-collect (`lib/auto-collect.mjs`, ADR-0148), for a builder's validated request, on a collector, within the operator's caps. Its routes are `GET /api/requests` (the queue and the meter), `POST /api/autocollect` (the settings), `/api/requests/check` and `/api/requests/resume`. It writes two things: the search provider's key into the machine config through `transport.mjs`'s `SEARCH_KEY` (so the vendor's name stays in the adapter and the registry, NFR-3), refused when the config resolves inside a git repository (Rule 6), or with HTTP 409 and a role-repair remedy when an unreadable config has no usable snapshot (#364); and which project it shows, a full path to an existing folder, never searched for. The topic and `BRIEF.md` are read, never written (ADR-0056). `builderInstructions` is the hand-off text: the collector's push, ledger included, then the builder's own steps in `$HOME` spelling (ADR-0050) - declare the role, handoff, preflight, inspect an authored complete brief and its current inputs, never collect. Its readiness checklist requires classified map rows and rewritten Findings; a file-presence observation grants no build authorization. Commit-report placeholders require actual observed results. A collected Git-request result waits for the collector to record review/closure before dependent building. Every request passes four checks: the bound address, a Host header naming it (DNS rebinding), an Origin, when sent, that is its own, and for `/api/` the per-launch token, initialized through the launch URL fragment and required in x-panel-token. The launcher prints that capability URL; terminal-log and browser-history privacy are not guaranteed by this module. POSTs must be JSON, so a plain cross-site form is refused. Update is refused when the panel runs from the installed copy, which has nothing newer to install. Command runs, the project switch and auto-collect cycles share one lock (`running`, handed to `createAutoCollect` as `lock`): none runs beside another, so no preflight or update reads the corpus or the kit tree while a cycle writes it. | ADR-0147, ADR-0148 | | `requests.mjs` | **A builder's fact request as a file** (ADR-0148, 2026-10-07): `research/requests/.json`, the `fact-request` skill made machine-readable. `requestProblems(request, { perRequestPages })` validates it as untrusted input: the fixed fields (`fact`, `blocks`, `urls`, `queries`, `prefer`, `unknown`, `maxPages`, `topic`), bounded sizes, and `urlProblem` refuses a non-http(s) URL, one with a user name or password, and one naming this machine or an internal address (`isInternal` from `http-transport.mjs`, ADR-0110). A request over the per-request cap is refused, not trimmed. `listRequests` reads the folder, skipping the collector's own `.result.json` and `.plan.json` and ignoring a name that is not an id, and a request whose file, `.plan.json` or `.result.json` is a symbolic link (a builder's link would make the collector read or overwrite a file of its own machine; `writeText` follows links). `applyRequest` and `writeResult` refuse to write through a link as well. `applyRequest` writes the contract row (a new OPEN `U-n`, or an existing non-CLOSED row; it refuses requests naming a closed unknown) and the `plan.json` entries, and the plan for that run alone, with `maxScrapes` set to the request's pages. `contractUnknownStatus` reads the status of a contract row for continuation checks. `writeResult` records the outcome beside the request. Terminal and partial results are trusted only when `recordTrustedResult` has recorded hashes of both request and result in the machine-local `research-kit.autocollect.json`; `isTrustedResult` checks those hashes so a builder cannot submit a result as if the collector wrote it. The auto-collect settings (`off`/`auto`, pages a request, pages a day, interval, the folder for new topics) and the daily meter live in `research-kit.autocollect.json` beside the machine config (`readAutoCollect`, `saveAutoCollect`, `recordSpend`, `setPaused`); no key is in them. | ADR-0148 | | `auto-collect.mjs` | **The collector listening** (ADR-0148, 2026-10-07): `createAutoCollect({ project, env, kitRoot, exec, lock })` returns `cycle`, `status`, `schedule`, `resume`, `stop`. A cycle resolves the project folder to its final path (`fs.realpathSync.native`) before comparing it with git's top level, so a Windows 8.3 short name or a symlinked folder cannot turn a path git stages into one outside the repository. `status()` uses that same canonical project path so its trust lookup matches collector records across alternate Windows path spellings. A cycle refuses unless `collectionPolicy` says the machine may collect, and skips while another run holds the shared `lock` (the panel's; its own when none is given). Otherwise it runs `git pull --ff-only`; a failed pull is surfaced and blocks the cycle before requests are read or processed. On success, it goes through each request. Results received over git are ignored unless their request and result hashes match collector-local trust records, including in `status()`; trusted terminal results retry pending commit/push delivery without collecting again, and trusted `partial` results carry continuation state and resume only while their contract unknown is not CLOSED. An invalid one gets a refused result, committed and pushed. A valid one, within the per-request and daily caps and with `research/` clean, is applied (`requests.mjs`), then its target-specific `research/requests/.plan.json` runs, followed by `preflight.mjs`, whose verdict is recorded, not acted on. It then stages `research/` and the ledger by name (`git add -f`), commits, and pushes when there is an upstream. Accounting travels on the fixed Node research child's private IPC descriptor (ADR-0149), never its human output: exactly one bounded, typed report with measured pages within the admitted allowance and the exhaustion provider. Missing, invalid, duplicate or over-allowance reporting reserves the remaining admitted allowance against the daily meter, labels actual spend unknown, records a terminal failed result and pauses. Earlier measured continuation attempts stay counted once. A valid received report survives failed exit or later interruption; interruption also pauses. Diagnostic output keeps a bounded redacted tail with a truncation notice, and failure detail is bounded separately. The legacy `spentPages`/`creditsStopped` exports remain diagnostic compatibility helpers. It never passes `--fallback`: received exhaustion accounting makes the loop commit the partial corpus and continuation state, pause until the operator resumes, then reuse the saved target, unknown and remaining page allowance (ADR-0129). A `topic` request gets `new-project.mjs` and `decompose.mjs --max-scrapes 0` in a dated folder under the folder the operator approved, never over an existing project (ADR-0056). It is refused when no folder is approved. Diagnostic details, refusal reasons and preflight verdicts are scrubbed with the existing redactor before result files or commit reports are written (2026-10-08). Status also scrubs inherited trusted result details, delivery diagnostics and pause text without rewriting those files or changing their trust hashes; the configured redactor defines which secret is recognized. Every child gets a fixed argument list; `exec` is injectable for tests. The panel labels its unchanged numeric meter as pages used or reserved. **Allowance regression coverage** (2026-10-09): the four-cycle credit exhaustion and resume fixture runs real synthetic Node scripts against persisted request, plan, result and meter files, with a finite injected Git executor. Native round-trip, partial-delivery and topic-resume fixtures retain real Git and corpus-transfer assertions. The test watchdog remains 60 seconds. **Complete Git answers** (ADR-0151): the bounded child byte buffer reports a private `truncated` flag. Repository and Git metadata paths, upstream, ahead count, commit subject and changed paths, dirty state, and pending delivery status/diff all refuse truncated output before it can authorize work; a malformed ahead count or missing quiet-diff exit also blocks delivery. A newly written partial result is trusted before fingerprinting, so refused inspection stages nothing and cannot make a restart collect it again. Exit-only operations retain bounded diagnostic logs. **Admission survives collector termination** (ADR-0152): `reserveCollection` saves the newly admitted allowance and an owned in-flight pause atomically before the fixed research child starts; a failed meter write prevents spawn. `settleCollection` refunds unused allowance once on a valid receipt, preserves earlier spend and distinct pauses, and retains the full charge for unknown/spawn-failed attempts. Whole collector termination leaves its charge/pause across restart or project switch. Continuation reserves only its remaining newly admitted pages. Day-aware settlement never refunds yesterday against today; cross-day completion charging is conservative, and an over-cap completion remains paused. The existing file writer is atomic, while the mutex remains per collector instance; racing independent instances and power-loss durability are not certified by this correction. Successful result.next says captures were delivered: the builder reads/judges, the collector records Git Findings, closures, MAP and the current authored BRIEF; another fetch is only for a missing external fact. ADR-0052 local received-package review remains available, and derived approval stays role-agnostic (2026-10-08, ADR-0150). | ADR-0148, ADR-0149, ADR-0150, ADR-0151, ADR-0152 | -| `machine.mjs` | The config carries `searxngUrl` (2026-09-30, ADR-0104): a URL, not a secret. `shape()` keeps only the non-empty strings of `skillRoots`: every other key it reads is type-checked, and this one was checked as an array and passed its contents to `path.join`, so `{"skillRoots":[42]}` killed `doctor` with a raw ERR_INVALID_ARG_TYPE stack (break-test 2026-10-01). A dropped entry is silent; naming it would be a new finding, which the freeze does not allow. `editGate.settingsPath` is taken only as a non-empty string, else the anchor (2026-10-02, arena #198): it was the one key `shape()` passed through raw, and a numeric value reached `path.dirname` in `installEditGate` and killed `install-hooks` with a raw stack - the sibling of the `skillRoots` check, found a day later because the first fix was never swept to it. **An ill-typed posture key resolves to its restrictive state** (ADR-0121, 2026-10-02): `shape()` resolves a `failOpen`, `evidencePolicy` or `editGate.mode` that is present and not of its type to `false`, `strict` and `hard-block` through one `postureKey` helper, lists it in `illTyped`, and `readMachineConfig`/`posture` carry `illTypedKeys` beside `retiredKeys` on every path, the snapshot one included. Absent keys keep their defaults. Until then a typo fell to the lax default, the only place a typo relaxed a gate in silence. `browserPath` is read and kept (2026-10-11): it was missing from `shape()`, so the key the browser transport's errors name was never read, and every `saveConfig` - `install-hooks --role` among them - erased it. Only non-array JSON objects may create or restore a last-good snapshot (#368, 2026-10-13). A parsed scalar/array leaves an existing good snapshot intact; an unusable backup falls back to fail-closed with role unknown. | **Everything outside the project**: machine config (fail-open posture, `editGate` mode and settings path, `evidencePolicy`, `role`), read in **three states** — `absent` (nothing configured; the fail-open default is the answer), `readable`, and **`unreadable`** (the file exists and does not parse), which holds the last config that parsed from a snapshot beside it and, with nothing to hold, **fails closed**. `readMachineConfig()` returns the settings *and* `state`/`source`/`error`, because the settings alone cannot say whether fail-closed was configured or is the fallback this machine adopted (ADR-0020). Install state, git `core.hooksPath` (scope-aware readers that expand Git include and includeIf directives, including scoped reads, so installation preserves and restores the included hooks folder; #369), and the **runtime anchors** — `RUNTIME_ANCHORS`, the one place a runtime's paths are named (settings file, personal skill roots, project-relative skill dir), each a default that `runtimePaths()` / `skillLocations()` let the machine config override (ADR-0012); Git readers/writers accept an explicit child environment, preserving production defaults while allowing R1's disposable machine/Git fixture to isolate global/system origins; reads a retired config key's value into the current shape rather than Its six home-based paths (settings, skills, `agentsHome`, `kitHome`, the config, the install state) use `homeDir()`, so `HOME=""` no longer makes them relative (2026-09-28). dropping a setting the operator had tightened, and reports a retired ENVIRONMENT variable (`CLAUDE_SETTINGS_PATH`) without ever honouring it. Owns the machine role and the collection policy that follows from it (`machineRole`, `collectionPolicy`, `collectionRefusal`) — the single answer to "may this machine collect?". The only module that reaches past the project root. **`localHooksPathOverride` asks git from `cwd`** (2026-10-03, outside audit): git finds the repository by walking up, so a subdirectory - a nested decision project, the folder install-hooks is run from - is governed by the same local `core.hooksPath` as the root; the probe looked for `.git` beside `cwd` and said `null` below it, and `doctor`'s `gateHealth` goes through the same probe now. `previousHooksPath(opts)` reads the folder the install replaced (2026-10-03), the reader beside `setPreviousHooksPath`, with `--type=path` as `hand-on.sh` reads it - the raw value made doctor call a working `~/custom-hooks` missing (second audit, 2026-10-03). `repoTopLevel(opts)` is `git rev-parse --show-toplevel` from a cwd, or null outside a repository (2026-10-03). `skillRoots(env)` is the personal skill roots - the config's `skillRoots`, else the anchor's - which `skillLocations()` and the skill set's deploy both read (2026-10-06, ADR-0146). Install-state reads require an object and shape existing path, path-array and skill-hash fields before consumers use them; doctor names an existing unreadable or pathless record in its deploy warning (#365). Snapshot resolution keeps source naming the broken config and carries snapshotSource separately; doctor and collection refusal distinguish held settings/role from fallback-closed instead of claiming the snapshot failed to parse (#366). | ADR-0002, ADR-0010, ADR-0012 | +| `machine.mjs` | **Config saves preserve declarations** (#364): `saveConfig` patches a raw JSON object or its usable last-good snapshot, retaining unpatched role and posture typos for diagnostics rather than serializing resolved defaults. A broken config without a usable snapshot refuses an unrelated save with `CONFIG-UNREADABLE`; an explicit valid role declaration can repair it. The existing `claudeGate` migration remains, and retired keys are dropped. The config carries `searxngUrl` (2026-09-30, ADR-0104): a URL, not a secret. `shape()` keeps only the non-empty strings of `skillRoots`: every other key it reads is type-checked, and this one was checked as an array and passed its contents to `path.join`, so `{"skillRoots":[42]}` killed `doctor` with a raw ERR_INVALID_ARG_TYPE stack (break-test 2026-10-01). A dropped entry is silent; naming it would be a new finding, which the freeze does not allow. `editGate.settingsPath` is taken only as a non-empty string, else the anchor (2026-10-02, arena #198): it was the one key `shape()` passed through raw, and a numeric value reached `path.dirname` in `installEditGate` and killed `install-hooks` with a raw stack - the sibling of the `skillRoots` check, found a day later because the first fix was never swept to it. **An ill-typed posture key resolves to its restrictive state** (ADR-0121, 2026-10-02): `shape()` resolves a `failOpen`, `evidencePolicy` or `editGate.mode` that is present and not of its type to `false`, `strict` and `hard-block` through one `postureKey` helper, lists it in `illTyped`, and `readMachineConfig`/`posture` carry `illTypedKeys` beside `retiredKeys` on every path, the snapshot one included. Absent keys keep their defaults. Until then a typo fell to the lax default, the only place a typo relaxed a gate in silence. `browserPath` is read and kept (2026-10-11): it was missing from `shape()`, so the key the browser transport's errors name was never read, and every `saveConfig` - `install-hooks --role` among them - erased it. Only non-array JSON objects may create or restore a last-good snapshot (#368, 2026-10-13). A parsed scalar/array leaves an existing good snapshot intact; an unusable backup falls back to fail-closed with role unknown. | **Everything outside the project**: machine config (fail-open posture, `editGate` mode and settings path, `evidencePolicy`, `role`), read in **three states** — `absent` (nothing configured; the fail-open default is the answer), `readable`, and **`unreadable`** (the file exists and does not parse), which holds the last config that parsed from a snapshot beside it and, with nothing to hold, **fails closed**. `readMachineConfig()` returns the settings *and* `state`/`source`/`error`, because the settings alone cannot say whether fail-closed was configured or is the fallback this machine adopted (ADR-0020). Install state, git `core.hooksPath` (scope-aware readers that expand Git include and includeIf directives, including scoped reads, so installation preserves and restores the included hooks folder; #369), and the **runtime anchors** — `RUNTIME_ANCHORS`, the one place a runtime's paths are named (settings file, personal skill roots, project-relative skill dir), each a default that `runtimePaths()` / `skillLocations()` let the machine config override (ADR-0012); Git readers/writers accept an explicit child environment, preserving production defaults while allowing R1's disposable machine/Git fixture to isolate global/system origins; reads a retired config key's value into the current shape rather than Its six home-based paths (settings, skills, `agentsHome`, `kitHome`, the config, the install state) use `homeDir()`, so `HOME=""` no longer makes them relative (2026-09-28). dropping a setting the operator had tightened, and reports a retired ENVIRONMENT variable (`CLAUDE_SETTINGS_PATH`) without ever honouring it. Owns the machine role and the collection policy that follows from it (`machineRole`, `collectionPolicy`, `collectionRefusal`) — the single answer to "may this machine collect?". The only module that reaches past the project root. **`localHooksPathOverride` asks git from `cwd`** (2026-10-03, outside audit): git finds the repository by walking up, so a subdirectory - a nested decision project, the folder install-hooks is run from - is governed by the same local `core.hooksPath` as the root; the probe looked for `.git` beside `cwd` and said `null` below it, and `doctor`'s `gateHealth` goes through the same probe now. `previousHooksPath(opts)` reads the folder the install replaced (2026-10-03), the reader beside `setPreviousHooksPath`, with `--type=path` as `hand-on.sh` reads it - the raw value made doctor call a working `~/custom-hooks` missing (second audit, 2026-10-03). `repoTopLevel(opts)` is `git rev-parse --show-toplevel` from a cwd, or null outside a repository (2026-10-03). `skillRoots(env)` is the personal skill roots - the config's `skillRoots`, else the anchor's - which `skillLocations()` and the skill set's deploy both read (2026-10-06, ADR-0146). Install-state reads require an object and shape existing path, path-array and skill-hash fields before consumers use them; doctor names an existing unreadable or pathless record in its deploy warning (#365). Snapshot resolution keeps source naming the broken config and carries snapshotSource separately; doctor and collection refusal distinguish held settings/role from fallback-closed instead of claiming the snapshot failed to parse (#366). | ADR-0002, ADR-0010, ADR-0012 | | `handoff.mjs` | The **arrival question**: `verifyHandoff(root)` reports a ledger that is absent or empty, an `EVIDENCE.md` that is absent or holds no evidence table (`handoff-evidence-missing`, `handoff-evidence-unparsed`; the report carries `rows`, and the CLI and doctor print the count - 2026-10-02: with no rows there were no citations, so "every cited capture on disk" held vacuously and handoff said OK over a corpus whose table a sync tool had dropped), every capture an evidence row names that is not on disk, and a chain that does not verify — each named. Read-only by design: the machine that asks cannot collect the missing bytes. The report also carries `lineEndings`, the captures whose body hash fails *only* because git rewrote CRLF into them on checkout, and **`handoffRemedy(report)` picks the remedy from the cause** rather than printing one text for every way a corpus can look broken (ADR-0020): `HANDOFF_REMEDY` when something did not travel (the collector must push `research/raw/`, dotfiles included), `lineEndingRemedy` when everything travelled and this machine rewrote it (pin with `.gitattributes`, re-check out, touch no collector and spend no credits), both when a report holds both. Consumed by `bin/handoff.mjs` and doctor (blocker on `role=builder`, silent on a collector; doctor passes the snapshot it already read, so the question is composed once for both). The report carries `nothingCollected` (no ledger entry, no evidence row, no capture) since 2026-09-27, so a caller can tell a project never collected from a corpus that failed to travel. The finding names are unchanged, so the doctor and the artifact validator read them as before. The report also carries `brief: { state, todo }` (`briefReview`, 2026-09-27, ADR-0061): the CLI's "handoff OK" is followed by a note when the brief is a draft (naming the judged sections still TODO) or has not been drafted. It is never a finding, and the exit stays 0. **The line-ending remedy is judged by running it** (2026-09-27, ADR-0062): per affected capture it prints `git rm --cached` (the index entry only) then `git checkout HEAD --`, which rewrites the file as LF through the pin, and it prints the pin (`PIN_LINES`, to add in an editor) only when `.gitattributes` lacks it. Every printed command is a plain git command with no comment, so cmd and PowerShell run it as written (ADR-0070). `--renormalize` had fixed only the index, so the capture handoff reads stayed CRLF. A **third cause** (2026-09-28): a ledger present and empty while captures are on disk lost its entries where the captures are - a one-line ledger whose torn tail `doctor --fix-arity` dropped - so `ledgerLost` gets `ledgerLostRemedy()`: restore it from git, or re-collect with `--force`. It was given the push remedy, which on the collector pushes the same empty ledger. A ledger that is merely **short** is the same cause: a cited capture on disk that no entry records is `handoff-capture-unledgered` (2026-09-28) and gets the same remedy - handoff had said OK and exited 0 while preflight failed it as `fetch-entry-exists`. **`CORPUS_ADD`** (2026-09-28) is how the kit prints putting a corpus into git - `git add research/`, then `git add -f research/raw/.fetches.jsonl` - shared by the push remedy and the commit gate's fix: both said `git add -f` on the whole folder, which also committed the machine-local `.usage.jsonl`, `.failures.jsonl`, `.diagnostics.jsonl` and `.fetches.lock`. A **fourth cause** (2026-10-04, break-test pass 4, F5b): a capture whose body hash fails and NOT by line endings (`body-unmodified/modified`) was changed after its fetch, and the report carries it as `altered`; it no longer counts as "did not travel", and **`alteredRemedy(files, { isRepo })`** prints, per file, `git status --porcelain --` to tell a local change from a committed one, then `git checkout HEAD --` followed by handoff again (no folder-wide checkout, which would discard uncommitted work beside it), and for the committed case `git log --oneline --` to find the commit that held the fetched bytes and restore from it, here or on the collector; without git metadata it prints no command. Judged by running it (2026-10-04, external review): a dirty status says only that this checkout changed the file after the commit - a capture committed altered and edited again is dirty too - so the remedy no longer claims the committed copy is the fetched one; and `--force` is no longer offered, because a new fetch writes a new capture beside this one, or refuses to overwrite a differing source sibling, while `verifyLedger` still checks the entry that names this file. The push remedy it used to get sends the same bytes again when the change was committed, and sends an operator to another machine when the committed copy here was one checkout away. **`safePathspec(file)`** (2026-10-04, two external reviews) decides whether a capture path is printed inside a git command at all: letters, digits, `.`, `_`, `-` and `/`, the alphabet the collector's own names come from. Anything else is named but never printed as a command, because no one spelling is read literally by cmd, PowerShell and sh alike and by git: a space unquoted is two pathspecs, `$copy` inside double quotes is expanded to nothing by PowerShell, and `[12]` is a git glob whatever the shell did, so `git checkout HEAD --` restored a neighbour and discarded its uncommitted work. For those the remedy prints commands that type no name at all: the operator lists them, one per line, in `handoff-names.txt` (`NAMES_FILE`) and runs `git --literal-pathspecs checkout HEAD --pathspec-from-file=handoff-names.txt` (the line-ending remedy adds the `git rm --cached` step the same way) - "quote it for your shell and prefix `:(literal)`" was not enough, because cmd expands `%NAME%` inside double quotes and PowerShell `$name` (third external review, 2026-10-04, verified by running in cmd); `--pathspec-from-file` hands git the bytes of the file with no shell in between, and `--literal-pathspecs` switches pattern syntax off (both git 2.25+). Such a name reaches the ledger rarely but not only by hand: `captureName` embeds the URL's host as it is, so an IPv6 literal or an adapter-supplied host outside DNS's alphabet produces one (third review). The classification runs over EVERY affected file (`classify`), not the five the text names: an unsafe sixth file got no note and "handle the remaining files the same way, each by name" - an instruction to type the glob that restores a neighbour (third external review, 2026-10-04). **`insideRepository(root)`** (2026-10-04, two external reviews) decides whether the runnable remedies are printed, and it asks git: `git -C rev-parse --is-inside-work-tree`. It was `root/.git` alone, so a nested decision project (ADR-0030), which has none of its own, was told it had "no git metadata"; then a walk up for a `.git` entry, which walked past a bare repository the project sat inside and printed a checkout git refused with "this operation must be run in a work tree". The commands are git's, so git decides whether they run; without git on PATH the walk is the fallback: it walks the PHYSICAL path (realpath first - a project reached through a junction into a repository had read as outside it, and one linked out of a repository as inside; third review) and reads `GIT_CEILING_DIRECTORIES` as git does: absolute entries only, each through realpath until an empty entry, after which the rest are taken as written. Git's pathspecs are relative to the cwd, so the printed commands run unchanged from the project folder. The tests that want "no git metadata" from a scratch fixture set `GIT_CEILING_DIRECTORIES` above it and go through `handoffRemedy`, so a detection that always says "repository" fails them. **`repositoryState(root)`** is the full answer, `{ inside, refused }`: when git answered with something other than yes, "not a git repository" or "must be run in a work tree" - `fatal: detected dubious ownership` - `refused` carries its first line and both remedies say git refused to read the repository and to fix that first, where they said "no git metadata, re-copy the corpus" (third review, 2026-10-04); no command is printed either way. | ADR-0011, ADR-0020 | | `bundle.mjs` | The **archive this project arrived as**, and how far the tree has moved from it. `BUNDLE_INDEX.md` lists 80 SHA-256 digests taken at the 2026-09-17 handover; until ADR-0028 **nothing had ever read them**, which is how ADR-0022 could treat them as a constraint for three days without noticing the constraint was already violated. `verifyBundle(root)` sorts every entry into unchanged / drifted / unexpected / missing, where "drifted" means a file in `EXPECTED_TO_DRIFT` — the corpus, the map, the ADR index, the manifest itself, (from 2026-09-20) `CONTEXT.md` and the root `README.md`, which are documentation ABOUT the code and go stale the moment it moves, and (from 2026-09-21) `AGENTS.md`, which is the file the agent protocol is *written in* — caught drifting by this check when ADR-0030 gave decision research a committed home and the rule that sends an agent there had to be where agents read their rules — each of which moves because one of this project's own rules *requires* it to. The list is deliberately narrow: exempting a planning document would leave the report unable to say anything, and a test asserts none is exempted. Hashes **bytes**, not decoded text. Reports; never fails a build — a changed file is a fact about history, and the only thing that makes it a problem is nobody knowing. `bin/bundle.mjs --json` prints `verifyBundle`'s result as JSON (2026-09-28): the flag was accepted and never read, so a caller parsing it got the markdown report. `START_HERE.md` joined `EXPECTED_TO_DRIFT` on 2026-09-29: the archived copy held the author's machine paths, and the portable one is the template rendered with `$HOME`. | ADR-0028 | | `git-config.mjs`, `config.mjs` | Compatibility shims → `machine.mjs`. | ADR-0002 | @@ -267,7 +267,7 @@ build approval. The shared verdict library and all gated formatting stay unchang `gate.mjs` (→ evaluate; `--staged-stdin` reads the piped path list; `--posture` answers 0 allow / 1 fail-closed / 2 unreadable-and-closed, so no shell has to parse JSON), `doctor.mjs` (`--fix-arity`), `install.mjs` (deploy + `--into` per-project -binding, role-aware next steps; collector keyless instructions name the selected machine config rather than a per-run research flag, builder instructions require complete/current review, and an unresolved role prints diagnostics before role-specific advice (2026-10-08, ADR-0150); `tolerateClosedStdout` before its first write, so a reader that leaves early does not turn a finished deploy into exit 1, 2026-09-30), `install-hooks.mjs` (`--fail-closed`, `--role`), +binding, role-aware next steps; collector keyless instructions name the selected machine config rather than a per-run research flag, builder instructions require complete/current review, and an unresolved role prints diagnostics before role-specific advice (2026-10-08, ADR-0150); `tolerateClosedStdout` before its first write, so a reader that leaves early does not turn a finished deploy into exit 1, 2026-09-30), `install-hooks.mjs` (`--fail-closed`, `--role`; a `CONFIG-UNREADABLE` save refusal is named with exit 2 before the requested config patch is written, #364), `new-project.mjs` (its first next step omits `--topic` once the project has one, ADR-0056), `decompose.mjs` (phase 0 CLI, refused on a builder by role), `timeline.mjs`, `audit.mjs` (lists and resolves versions through the manifest's reader), `brief.mjs` (drafts the handoff with all evaluated warning reasons, remedies and diff --git a/research-kit/README.md b/research-kit/README.md index 269f36f4..b85ab518 100644 --- a/research-kit/README.md +++ b/research-kit/README.md @@ -568,7 +568,7 @@ node research-kit/bin/selftest.mjs # all of it node research-kit/bin/selftest.mjs gate hook # just these files ``` -1838 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1843 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means the assertions settled (ADR-0021), and each test is raced against a watchdog (`RESEARCH_KIT_TEST_TIMEOUT`, default 60s). diff --git a/research-kit/bin/install-hooks.mjs b/research-kit/bin/install-hooks.mjs index 59b0a87c..744329f9 100644 --- a/research-kit/bin/install-hooks.mjs +++ b/research-kit/bin/install-hooks.mjs @@ -109,7 +109,7 @@ evidencePolicy ${config.evidencePolicy} `); process.exit(failed ? 1 : 0); } catch (err) { - const why = writeFailure(err); + const why = err.code === 'CONFIG-UNREADABLE' ? err.message : writeFailure(err); if (!why) throw err; process.stderr.write(`${why}\n`); process.exit(2); diff --git a/research-kit/lib/machine.mjs b/research-kit/lib/machine.mjs index 888d747a..c12ae05a 100644 --- a/research-kit/lib/machine.mjs +++ b/research-kit/lib/machine.mjs @@ -236,12 +236,40 @@ export function loadConfig(env = process.env) { /** Writing drops retired keys, so the migration happens through normal use. */ export function saveConfig(patch, env = process.env) { const file = configPath(env); - const current = readMachineConfig(env).settings; + const object = (value) => value !== null && typeof value === 'object' && !Array.isArray(value); + const rawFrom = (source) => { + const raw = parseJson(readText(source) ?? ''); + if (!object(raw)) throw new Error('config must be a JSON object'); + return raw; + }; + let current = {}; + if (exists(file)) { + try { current = rawFrom(file); } + catch { + try { current = rawFrom(lastGoodPath(file)); } + catch { + if (!Object.hasOwn(patch, 'role') || !ROLES.includes(patch.role)) { + 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`), { + code: 'CONFIG-UNREADABLE', + }); + } + } + } + } + // Save declarations, not the resolved role/posture. A typo remains available for + // doctor to name until the operator patches that field. Only the existing retired + // spelling is migrated here; it must keep its mode when its old key is dropped. + if (current.editGate?.mode === undefined && EDIT_GATE_MODES.includes(current.claudeGate)) { + current = { ...current, editGate: { ...(object(current.editGate) ? current.editGate : {}), mode: current.claudeGate } }; + } const next = { ...current, ...patch, - editGate: { ...current.editGate, ...(patch.editGate ?? {}) }, }; + if (patch.editGate !== undefined) { + next.editGate = { ...(object(current.editGate) ? current.editGate : {}), ...(patch.editGate ?? {}) }; + } + for (const retired of Object.keys(RETIRED_CONFIG_KEYS)) delete next[retired]; ensureDir(path.dirname(file)); writeText(file, `${JSON.stringify(next, null, 2)}\n`); writeText(lastGoodPath(file), `${JSON.stringify(next, null, 2)}\n`); diff --git a/research-kit/lib/panel.mjs b/research-kit/lib/panel.mjs index c7de1a85..3bf71156 100644 --- a/research-kit/lib/panel.mjs +++ b/research-kit/lib/panel.mjs @@ -347,7 +347,11 @@ export function createPanel({ if (problem) return refuse(res, 400, problem); const inside = configInsideRepository(env); 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.`); - saveConfig({ [SEARCH_KEY.configKey]: body.key.trim() }, env); + try { saveConfig({ [SEARCH_KEY.configKey]: body.key.trim() }, env); } + catch (err) { + if (err.code === 'CONFIG-UNREADABLE') return refuse(res, 409, err.message); + throw err; + } return send(res, 200, { saved: true, keys: keyState() }); } diff --git a/research-kit/test/machine.test.mjs b/research-kit/test/machine.test.mjs index 691f88b1..fd3df399 100644 --- a/research-kit/test/machine.test.mjs +++ b/research-kit/test/machine.test.mjs @@ -216,6 +216,62 @@ test('saveConfig drops the retired key, so the migration happens through normal assert.equal(written.role, 'builder'); }); +test('an unrelated config save preserves the role the operator actually typed', () => { + const { env, file } = envWith('{"role":"Builder","evidencePolicy":"STRICT"}'); + saveConfig({ failOpen: false }, env); + const written = JSON.parse(readText(file)); + assert.equal(written.role, 'Builder'); + assert.equal(written.evidencePolicy, 'STRICT'); + assert.equal(written.failOpen, false); + assert.equal(machineRole(env), 'unknown'); + saveConfig({ role: 'collector' }, env); + assert.equal(JSON.parse(readText(file)).role, 'collector'); +}); + +test('a broken config without a usable snapshot refuses an unrelated save and permits a role repair', () => { + for (const snapshot of [null, '[]', 'null']) { + const original = '{"role":"bui'; + const { env, file } = envWith(original, { snapshot }); + assert.throws(() => saveConfig({ failOpen: true }, env), /does not parse.*--role/); + assert.equal(readText(file), original, 'a refusal must keep the operator\'s only copy'); + if (snapshot === null) assert.equal(fs.existsSync(`${file}.last-good`), false); + else assert.equal(readText(`${file}.last-good`), snapshot); + saveConfig({ role: 'collector' }, env); + assert.equal(JSON.parse(readText(file)).role, 'collector'); + assert.notEqual(JSON.parse(readText(file)).failOpen, false, 'no fallback-closed setting is persisted'); + } +}); + +test('saving from a last-good snapshot retains its raw role and posture values', () => { + const { env, file } = envWith('{"role":"bui', { + snapshot: '{"role":"Builder","failOpen":"false","editGate":{"mode":"HARD-BLOCK"}}', + }); + saveConfig({ serpapiKey: 'unit-test-key' }, env); + const written = JSON.parse(readText(file)); + assert.equal(written.role, 'Builder'); + assert.equal(written.failOpen, 'false'); + assert.equal(written.editGate.mode, 'HARD-BLOCK'); + assert.equal(written.serpapiKey, 'unit-test-key'); +}); + +test('install-hooks names an unreadable config save refusal without replacing it', () => { + const original = '{"role":"bui'; + const { env, file } = envWith(original); + const isolated = { + ...env, RESEARCH_KIT_HOME: KIT_ROOT, + RESEARCH_KIT_INSTALL_STATE: path.join(path.dirname(file), 'install.json'), + GIT_CONFIG_GLOBAL: path.join(path.dirname(file), 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1', + }; + const run = spawnSync(process.execPath, [path.join(KIT_ROOT, 'bin', 'install-hooks.mjs'), '--fail-closed', '--git-only'], { + env: isolated, encoding: 'utf8', timeout: 60_000, windowsHide: true, + }); + assert.equal(run.status, 2, `${run.stdout}${run.stderr}`); + assert.match(run.stderr, /does not parse.*--role/); + assert.doesNotMatch(run.stderr, /\n\s+at /); + assert.equal(readText(file), original); + assert.equal(fs.existsSync(isolated.RESEARCH_KIT_INSTALL_STATE), false); +}); + // Found 2026-10-11: the browser transport and its error messages tell the operator to set // browserPath (ADR-0088), but the config was read without it and rewritten without it, so // `install-hooks --role builder` erased a browserPath the operator had set by hand. diff --git a/research-kit/test/panel.test.mjs b/research-kit/test/panel.test.mjs index d5bcc6e3..b2fa0439 100644 --- a/research-kit/test/panel.test.mjs +++ b/research-kit/test/panel.test.mjs @@ -184,6 +184,21 @@ test('the page never writes server text as HTML', () => { assert(/button\.type = 'button'/.test(js) && /aria-label/.test(js), 'request output has no keyboard-accessible control'); }); +test('saving a key refuses a broken unsnapshotted config without replacing or echoing it', async () => { + const m = machine(); + const original = '{"role":"bui'; + writeText(m.configFile, original); + const ctx = await started({ machine: m, project: makeProject() }); + try { + const res = await api(ctx, '/api/key/search', { key: KEY }); + assertEqual(res.status, 409, res.text); + assert(/does not parse.*--role/.test(res.text), res.text); + assert(!res.text.includes(KEY), 'the refusal echoed the key'); + assertEqual(readText(m.configFile), original); + assert(!fs.existsSync(`${m.configFile}.last-good`), 'the refusal created a snapshot'); + } finally { await ctx.panel.close(); } +}); + test('saving the search key writes the machine config and never echoes the key', async () => { const m = machine(); const ctx = await started({ machine: m, project: makeProject() });