From 87652bf5abc847ea278a11e14d0ea930cfedca78 Mon Sep 17 00:00:00 2001 From: StepenkoAnatoli Date: Tue, 13 Oct 2026 20:18:22 +0300 Subject: [PATCH] fix(artifact): flush piped reports before exit what changed: Await the existing exitAfterFlush helper for all artifact CLI help, refusal, success and validation endings. Add large-output regressions for JSON and text with 700 missing files. why: Direct process.exit can discard queued stdout on POSIX pipes, truncating reports after 64 KiB. Drain output before preserving the existing exit code. what it touched: artifact CLI/tests, architecture, changelog and README test inventory. No report schema or exit-code change. what you verified: verified - corrected baseline and fixed CLI each passed 23 artifact-cli tests; large JSON parses and text retains 700 errors, both over 64 KiB and exit 1. Handoff OK, preflight PASS, doctor READY. Full gate is running. what you got wrong and fixed: mistake - initial missing-files fixture was rebuilt by the seal helper. where - artifact-cli test. impact - it did not reproduce the truncation defect; no delivery affected. cause - default inventory recomputation; fix - preserve the mutated inventory using recomputeFiles:false; verified - corrected baseline and fixed CLI each passed 23 tests. --- CHANGELOG.md | 2 ++ docs/ARCHITECTURE.md | 2 +- research-kit/README.md | 2 +- research-kit/bin/artifact.mjs | 28 ++++++++++++------------- research-kit/test/artifact-cli.test.mjs | 21 +++++++++++++++++++ 5 files changed, 39 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7affd4..de0dde8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in `research-kit/lib/core.mjs`; a release is tagged `v` (ADR-0116). ## Unreleased + +- The artifact CLI drains stdout and stderr before ending its reports, preserving large piped JSON and text results on POSIX (#359). - Fixed `AGENTS.md` corpus description to use evergreen relative references rather than stale hardcoded entry counts (#299). - The suite's "every CLI refuses a flag it does not know" table now covers `measure`, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ca0813f..858b67c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -179,7 +179,7 @@ is an open decision (2026-09-30): | `archive.mjs` | **One container format**: `buildZip(entries)` → bytes, `writeZip(file, entries)` → `{ file, bytes, entries }` (written whole or not at all, through `writeBytes`, since 2026-09-28), `crc32`, `entryName`. ZIP with deflate (store when deflating would only grow the payload), written because the kit has no dependencies and cannot shell out to a `zip` binary that may not exist. Knows the container and nothing else — what goes inside a bundle is `lib/audit.mjs`'s. Refuses entry names that are not plain relative paths (an archive must not unpack outside its folder) and an archive with no entries. See ADR-0019. **Changes no name it is handed** (2026-09-28): `entryName` refuses a backslash - it translated one into a separator, which stored `a\b.md` as `a/b.md` while the package manifest still declared the original. | ADR-0019 | | `artifact-zip.mjs` | **Reading a ZIP somebody else wrote** — the opposite job from `archive.mjs`, and a separate file on purpose: a writer that also parses hostile input grows a trust boundary in the middle of itself. `openZip(bytes)` → `{ entries, names, problems, has, read }`, plus `checkEntryName`, `readCentralDirectory`, `inspectEntries`, `readEntry` and the exported `ZIP_LIMITS` (entry count, total and per-entry uncompressed bytes, ratio + floor, name length) so tests drive the refusal paths with small fixtures instead of real bombs. **The order of operations is the security property**: every structural refusal — traversal, absolute, drive letter, backslash, NUL, duplicate, case collision, symlink/special entry, encryption, unknown method, counts, sizes, ratio — is decided from the **central directory before one byte is inflated**. The directory is attacker-controlled too, so it is never the only defence: `inflateRawSync` runs with `maxOutputLength`, making the ceiling the runtime's to enforce rather than a number compared against a field the attacker wrote. Names are judged as raw text, never `path.normalize`'d first — normalising would let `a/../../b` become `../b` and then be judged, when the honest answer is that `..` has no business in a research artifact. Touches no filesystem: entries come back as buffers. ZIP64 is refused rather than half-read. | ADR-0032 | | `artifact-validator.mjs` | **Is this package valid, and does it authorize a build** — two questions, kept apart. `validateArtifact({ file, expectedClientRef })` → `{ status, buildAuthorized, packageId, clientRef, workflowRunId, state, errors, warnings }`, where `status: 'PASS'` means internally consistent and says *nothing* about permission. Offline and read-only by construction: no transport imported, `process.env` never read, nothing from the package executed, no shell, and the only write is a `fs.mkdtemp` copy of `project/` removed in a `finally` — needed because the chain and handoff checks are the kit's existing code (`readCorpus`, `verifyHandoff`) and reuse beats a second implementation of provenance verification nobody tested. Stable error codes (`ZIP-*`, `MANIFEST-*`, `FILE-*`, `LEDGER-*`, `CAPTURE-*`, `AUTHORIZATION-INCONSISTENT`, `SECRET-DETECTED`, `FORMAT-UNSUPPORTED`, `CLIENT-REF-MISMATCH`) and **never a secret value in an error** — the report names the pattern and the line. `authorizationProblems()` is the one implementation of the build rule; the schema encodes it a second time and when the schema catches it first the rule is still reported in its own words. An unsupported format **major** is `INCOMPLETE`, not invalid. A crash is `BLOCKED`, never `PASS`. Its report carries `reviewedBy` (`review.by`, or `undeclared` for a 1.0.0 package) and judges nothing by it (ADR-0074). Reads format majors **1 and 2** (2026-09-30, ADR-0107): a 1.x `HUMAN_REVIEW_REQUIRED` is reported as `REVIEW_REQUIRED`, and `eraProblems` refuses a value from the other major by name - a 2.x package with the old state or `review.by: "human"`, or a 1.x one with the new state. A binary body (NUL in its first 8000 bytes) is not matched against the credential patterns, as a body over 512 KiB is not (ADR-0128, 2026-10-02): `docs/` travels whole, and one vendored DLL carrying the PEM marker it parses would have failed a package with no secret in it. | ADR-0032 | -| `artifact.mjs` | **Packaging a project without changing it, and deriving every authorization field.** `createArtifact` → `{ bytes, name, manifest, entries, derived }`; `writeArtifact` writes then **re-validates with the consumer's own validator** before reporting success; its bytes go through `writeBytes`, so re-creating a package at a name that already holds one cannot trade a good package for a truncated one (2026-09-28). There is no `--build-authorized` and no option that reaches an authorization field: `deriveState(root)` runs the real gate with `env` defaulting to `{}` so a packaging shell's override cannot become a property of a package that outlives it. `mapClassified` and `briefReviewed` have honest machine answers; `findingsReviewState` answers the third **negatively** — a `Finding` byte-identical to what `firstFinding` re-extracts from the capture was demonstrably never rewritten, which detects the unreviewed case without grading the reviewed one (ADR-0013's line, uncrossed). It repeats the collector's own call exactly - the page body after the front-matter, falling back to the page's `title` or `url` from that front-matter - since 2026-09-27: run over the whole file, the extractor read the front-matter as the page, and a URL-only corpus nobody had touched (one JSON capture, run 36287211468) was packaged `findingsReviewed: true`. `packageName` never carries the topic (an artifact listing is world-readable on a public repository) and falls back to `run-`; `checkClientRef` refuses rather than sanitises. Inventory sorted by ZIP path, manifest and digest appended **last**, digest over the exact manifest bytes. README-FIRST in a package that is not yet approved prints the preflight and brief commands and **one `create` line carrying the package's own identity** (`repackageArgs`, 2026-09-27): the manifest is written once, and the package gave its reviewer no way to get one that says `APPROVED_BRIEF`. Commands are spelled by `homeCommand` (ADR-0050). `createArtifact` checks the identity against the manifest schema's own `source` rules before deriving or writing anything (`sourceProblem`, 2026-09-27): `create --commit abc` wrote a package, then reported it did not validate, and left it on disk. **`review.by`** (format 1.1.0, ADR-0074) is the reviewer's declaration read from `BRIEF.md` by `reviewedBy`: `agent` or `undeclared` (`human` was retired in format 2.0.0, ADR-0107). Not an approval condition. README-FIRST no longer says "a human reviewed it"; it names the declared reviewer, or says none was declared. `collectProjectFiles` and the explicit ledger add refuse (`OUTSIDE_PROJECT`) any path whose real location is outside the project: it followed links and packed outside files, a private key included (ADR-0076). The same walk asks the consumer's own `checkEntryName` about every name it is about to store and refuses the run before a byte is written (`UNPACKAGEABLE_NAME`, 2026-09-28): `entryName` translates a backslash into a path separator, so a file named `a\b.md` travelled as `a/b.md` - a directory invented out of a name - while the manifest kept the original, and only the read-back caught the contradiction, after the package had been written. The same refusal covers two files differing only in case (2026-09-28), with the reader's lowercase comparison: the reader's ZIP-CASE-COLLISION fired only after the write. Format **2.0.0** (2026-09-30, ADR-0107): a collected package waiting for review says `REVIEW_REQUIRED`, and README-FIRST gives the review to the agent alone. collectProjectFiles refuses (`UNPACKAGEABLE_FILE`) what packaging cannot read to an end - a FIFO, socket or device, and a folder link back into its own ancestry (judged by real path, so a link to a sibling folder inside the project still packages) - before a byte is read (2026-10-01). **A reviewed brief is complete and current** (2026-10-03, output-reliability audit G1): `deriveState` approves only when every judged section is present and answered (`judgedSection`) and a stamped brief's inputs hash matches the corpus (`draftStamp`, `briefInputsHash`); `briefState` alone read a missing section as answered, so a drafted brief with no review sections derived APPROVED_BRIEF on the repository's own corpus, and a brief whose evidence changed after the draft stayed authorized beside hygiene's brief-stale warning. A brief with no stamp predates ADR-0055 - the root's own and ten decision briefs - and keeps its approval: its currency cannot be checked, `hygiene/brief-unstamped` says so (ADR-0138), a redraft with --force stamps it. **README-FIRST in every state and the collection summary preserve the producer evaluation warnings** through `renderGateWarnings` (ADR-0143). Authorization fields and machine package formats are unchanged. `bin/artifact.mjs` refuses a value flag given no value by name, exit 3, before anything is read or written (2026-10-11, #280): `create` had asked only whether a required flag was present, so a bare `--ref` went into the manifest as "true", a bare `--run-id` as 1 and a bare `--output` wrote a package named `true` that re-validated PASS; a bare `validate --file` tried to open a file called `true`. | ADR-0032, ADR-0031, ADR-0052, ADR-0143 | +| `artifact.mjs` | **Packaging a project without changing it, and deriving every authorization field.** `createArtifact` → `{ bytes, name, manifest, entries, derived }`; `writeArtifact` writes then **re-validates with the consumer's own validator** before reporting success; its bytes go through `writeBytes`, so re-creating a package at a name that already holds one cannot trade a good package for a truncated one (2026-09-28). There is no `--build-authorized` and no option that reaches an authorization field: `deriveState(root)` runs the real gate with `env` defaulting to `{}` so a packaging shell's override cannot become a property of a package that outlives it. `mapClassified` and `briefReviewed` have honest machine answers; `findingsReviewState` answers the third **negatively** — a `Finding` byte-identical to what `firstFinding` re-extracts from the capture was demonstrably never rewritten, which detects the unreviewed case without grading the reviewed one (ADR-0013's line, uncrossed). It repeats the collector's own call exactly - the page body after the front-matter, falling back to the page's `title` or `url` from that front-matter - since 2026-09-27: run over the whole file, the extractor read the front-matter as the page, and a URL-only corpus nobody had touched (one JSON capture, run 36287211468) was packaged `findingsReviewed: true`. `packageName` never carries the topic (an artifact listing is world-readable on a public repository) and falls back to `run-`; `checkClientRef` refuses rather than sanitises. Inventory sorted by ZIP path, manifest and digest appended **last**, digest over the exact manifest bytes. README-FIRST in a package that is not yet approved prints the preflight and brief commands and **one `create` line carrying the package's own identity** (`repackageArgs`, 2026-09-27): the manifest is written once, and the package gave its reviewer no way to get one that says `APPROVED_BRIEF`. Commands are spelled by `homeCommand` (ADR-0050). `createArtifact` checks the identity against the manifest schema's own `source` rules before deriving or writing anything (`sourceProblem`, 2026-09-27): `create --commit abc` wrote a package, then reported it did not validate, and left it on disk. **`review.by`** (format 1.1.0, ADR-0074) is the reviewer's declaration read from `BRIEF.md` by `reviewedBy`: `agent` or `undeclared` (`human` was retired in format 2.0.0, ADR-0107). Not an approval condition. README-FIRST no longer says "a human reviewed it"; it names the declared reviewer, or says none was declared. `collectProjectFiles` and the explicit ledger add refuse (`OUTSIDE_PROJECT`) any path whose real location is outside the project: it followed links and packed outside files, a private key included (ADR-0076). The same walk asks the consumer's own `checkEntryName` about every name it is about to store and refuses the run before a byte is written (`UNPACKAGEABLE_NAME`, 2026-09-28): `entryName` translates a backslash into a path separator, so a file named `a\b.md` travelled as `a/b.md` - a directory invented out of a name - while the manifest kept the original, and only the read-back caught the contradiction, after the package had been written. The same refusal covers two files differing only in case (2026-09-28), with the reader's lowercase comparison: the reader's ZIP-CASE-COLLISION fired only after the write. Format **2.0.0** (2026-09-30, ADR-0107): a collected package waiting for review says `REVIEW_REQUIRED`, and README-FIRST gives the review to the agent alone. collectProjectFiles refuses (`UNPACKAGEABLE_FILE`) what packaging cannot read to an end - a FIFO, socket or device, and a folder link back into its own ancestry (judged by real path, so a link to a sibling folder inside the project still packages) - before a byte is read (2026-10-01). **A reviewed brief is complete and current** (2026-10-03, output-reliability audit G1): `deriveState` approves only when every judged section is present and answered (`judgedSection`) and a stamped brief's inputs hash matches the corpus (`draftStamp`, `briefInputsHash`); `briefState` alone read a missing section as answered, so a drafted brief with no review sections derived APPROVED_BRIEF on the repository's own corpus, and a brief whose evidence changed after the draft stayed authorized beside hygiene's brief-stale warning. A brief with no stamp predates ADR-0055 - the root's own and ten decision briefs - and keeps its approval: its currency cannot be checked, `hygiene/brief-unstamped` says so (ADR-0138), a redraft with --force stamps it. **README-FIRST in every state and the collection summary preserve the producer evaluation warnings** through `renderGateWarnings` (ADR-0143). Authorization fields and machine package formats are unchanged. `bin/artifact.mjs` refuses a value flag given no value by name, exit 3, before anything is read or written (2026-10-11, #280): `create` had asked only whether a required flag was present, so a bare `--ref` went into the manifest as "true", a bare `--run-id` as 1 and a bare `--output` wrote a package named `true` that re-validated PASS; a bare `validate --file` tried to open a file called `true`. The artifact CLI awaits exitAfterFlush for its report, help and refusal endings, preserving queued stdout/stderr and the existing exit status on asynchronous POSIX pipes (#359). | ADR-0032, ADR-0031, ADR-0052, ADR-0143 | | `mcp.mjs` | **The collector as a Model Context Protocol server**, and the first module in this repository built from an APPROVED BRIEF rather than from a plan - every design choice cites an E-row in `docs/decisions/2026-09-21-agent-interface/`. `handle(message, deps)` is a pure function of a JSON-RPC message plus injected transport, so the whole surface is tested offline. **stdio, not Streamable HTTP**: semantics are identical on both, so the difference is operational, and remote costs the entire OAuth 2.1 chain. **Two tools, not one** - `collect` returns a run id immediately, `fetch_corpus` returns a link - because a server cannot initiate a request, so the alternative is one call held open for minutes and an agent that loses a paid run when its process dies. **A `resource_link`, never embedded bytes**: the spec defines embedding but states no maximum size, so a link is the defensible option. **No tool takes a token** and a test asserts no parameter is credential-shaped. `PROTOCOL_VERSION` is pinned and a mismatch answers `-32022` with `data.supported`, the same discipline as pinning `X-GitHub-Api-Version`. `createStdioLoop` serialises chunk processing through one promise chain - overlapping `data` events were mutating the buffer concurrently, which a test caught. **DUAL-ERA**: `SUPPORTED_VERSIONS` is `['2026-07-28', '2025-11-25', '2025-06-18']` (2025-06-18 since 2026-09-27, ADR-0049: a client on the official SDK up to 1.24.0 asks for it and refuses 2025-11-25, and nothing 2025-11-25 changed touches this server; 2025-03-26 is not served, because the results carry `structuredContent` and `resource_link`) and `initialize` is implemented beside `server/discover`, because the specification is ahead of every shipped client - the official SDK at v1.30.0 declares `2025-11-25` as its latest and answered the modern-only first version with `-32601: unknown method initialize`. `initialize` **echoes** a serveable requested version and falls back to **legacy** rather than to this server's preference, because a client throws on a version it does not support. **Arguments that fail a tool's schema are a tool result** (`isError: true`, the reason as text) since 2026-09-27 - SEP-1303 in 2025-11-25 says input validation errors should be Tool Execution Errors, so the model can correct itself; they were JSON-RPC -32602, which a client raises as an exception. An unknown tool stays -32602. `collect`'s `topic` carries `minLength: 1`, and `validateArgs` enforces `minLength` after trimming (2026-09-27): an empty topic had passed and reached GitHub, which answered HTTP 422. `fetch_corpus` checks `out_dir` with `usableOutDir` before it reads the run (2026-09-27), so an unusable folder is an `OUT_DIR` tool error, not `UNKNOWN`. `fetch_corpus` returns `reviewedBy`, and its text says review is for an agent or a person (ADR-0074). `createStdioLoop` bounds a line at `MAX_LINE` (4 MiB, 2026-09-28): a line past it is refused with -32600 as soon as it passes the cap and dropped through its newline, and the next message is served - the buffer had grown without limit until a newline came, so a client that sent none could grow the server until it died. **No request is lost at end of input** (break-test PR #145, 2026-09-29): `exitWhenSettled` waits for every call already read before exiting (bounded by `END_GRACE_MS`), and a final request without a trailing newline is served on `end` - both had been dropped with exit 0. `SERVER_INFO.version` is `KIT_VERSION` (ADR-0116); it had announced a fixed 1.0.0. **The approved fetch response directs the builder to README-FIRST and the brief for warnings and known unknowns** (ADR-0143). It forwards no new machine warning field and derives no new authorization. `validateArgs` checks allowed keys by own membership and enforces the declared arrays of strings: malformed query/URL values and inherited Object property names become schema tool errors before credential lookup or dispatch. Omitted or empty arrays, and valid string arrays, keep their existing normalization. | ADR-0034, ADR-0049, ADR-0143 | | `disclosure.mjs` | **What a public run actually exposes, measured rather than asserted.** Written because the opposite claim was documented first: the collector's header said dispatch inputs are visible to anyone with read access, and checking it found the topic in none of the readable responses. `probeRun` sends **unauthenticated** requests - there is no token parameter, and a probe that quietly authenticated would answer a different question while looking like this one. `summarise` separates `exposedNames` (true on any public repo, and the reason the topic is kept out of run and artifact names) from `exposedContent` and `exposedSubject`. `needleChecked` exists so a report can never let "false" read as "no": without a topic to search for, the rendering says **not checked**. An unreachable endpoint is a refusal to conclude, never a clean result - **enforced since 2026-09-27**: until then that sentence was true of the findings and false of the verdict, and with no network every probe was unreachable and the command exited 0, "only the shape is public". `summarise` now reports `unmeasured` (probes with status 0) and `conclusive`; `exitCode(report)` keeps a measured exposure (2, 1) whatever else failed, and otherwise needs every probe answered for 0, else 3; the rendering says UNREACHABLE and "not tried" instead of "refused", shows `not measured` for a false it could not establish, and closes NOT MEASURED instead of the shape paragraph. An unreachable probe names the cause of the failed fetch through `fetchFailure` (2026-09-30), not the bare `fetch failed`. `probeRun` takes the artifact and job ids from the listings its probes already read (2026-09-30); it had fetched both again inside one try/catch, so a transient failure on the second request left the artifact download and the log untried. | ADR-0035 | | `dispatch.mjs` | **Asking the collector to run, from outside GitHub** - the seam an agent or a desktop app uses. `dispatchCollection` (pins `X-GitHub-Api-Version: 2026-03-10` on the REQUEST, not merely in a comment), `waitForRun`, `listArtifacts`, `downloadArtifact`, `unwrapArtifact`, plus `tokenFromEnv` and `redact`. **No token may be passed as an argument** - `tokenFromEnv` is the only way in, because a credential on a command line reaches the shell history, `ps`, and any log that echoes its own command; a test asserts the source contains no token flag. `redact` is PATTERN-based rather than "remove the token we hold", so it also catches a credential the process was never given - one echoed back by a server, a proxy, a redirect. A **204** is a named `NO_RUN_ID` failure, never an empty success: "the run started and I cannot tell which one" is the failure a caller is least able to diagnose from a status code. **Both routes to a run id are requested**, not just the pinned version: the `X-GitHub-Api-Version: 2026-03-10` header AND `return_run_details: true`, the optional parameter GitHub added 2026-02-19 that returns the run id on `2022-11-28` as well. Measured before it was trusted - four calls on 2026-09-21 established that the old version honours the parameter and the new version accepts rather than rejects it, so sending both is safe and a server that strips or downgrades the header still answers with a run id. That retired ADR-0031's original fallback, which was to poll `/actions/runs` and correlate by `created_at` - a race the design never actually needed. `NO_RUN_ID` now means BOTH routes were ignored, which is a far narrower diagnosis. `waiting` is reported as a status, not a stall - it means a protection rule is holding the job. `unwrapArtifact` opens GitHub's outer ZIP through the same distrustful reader, and passes a hostile or ambiguous container through untouched for the validator to refuse rather than guessing at packaging depth. Every request goes through an injectable `fetch`, so the whole module is tested offline. **Pages by URL** (2026-09-27): `queriesInput` builds the one `queries` input from search queries and pages to fetch, one per line, and refuses a URL that is not http(s) before anything is dispatched. `collect.yml` fetches a line that is an http(s) URL instead of searching it, and with only URLs it searches nothing. `collect-remote --url` and the MCP `collect` tool's `urls` share this function, so all three agree. Until then the collector could only search, and a page the caller already knew had to be collected locally. **`usableOutDir` checks the folder before anything is spent** (2026-09-27): it creates the folder and tests write access, and an unusable one is a named `OUT_DIR` failure with a remedy. `fetchCorpus` calls it before it lists artifacts, the MCP `fetch_corpus` tool before it reads the run, and `collect-remote` before it dispatches. The folder had been checked only after the download, as a raw ENOTDIR the MCP server could report only as `UNKNOWN`, and for `collect-remote` that came after a paid run and up to half an hour of waiting. `fetchCorpus` writes the package it downloaded through `writeBytes` (2026-09-28, ADR-0079): `fs.writeFileSync` emptied a package already at that name when the write failed. `bin/collect-remote.mjs` checks `--max-pages` (1..25), `--depth`, `--runner`, `--search-transport` and `--timeout` against the ranges and choices `collect.yml` enforces, before it asks for a token (2026-09-29, Arena break test): `--max-pages 8o` was dispatched as typed, and the paid run failed on the runner. A dispatch that never left names the cause of the failed fetch through `fetchFailure` (2026-09-30), still passed through `redact`; so does `.github/scripts/check-action-pins.mjs` for a pin it could not check. Both had printed the bare `fetch failed`. `getRun` and `listArtifacts` read their body through `readJsonBody` (2026-09-30): a 200 whose body is not JSON (a maintenance page, a proxy) is `DispatchError` `BAD_BODY` with the status and a remedy, where it had thrown a raw `SyntaxError` that `collect-remote` could only call `UNKNOWN`. `getRun`, `listArtifacts` and `downloadArtifact` reach GitHub through `callApi` (2026-09-30), so a request that never left is `DispatchError` `NETWORK` with its cause (`fetchFailure`, redacted) and a remedy, as a dispatch already was; they had thrown the raw `fetch failed`, which `collect-remote` could only call `UNKNOWN`. fetchCorpus treats names from the API as data: a wrapper entry is flattened to its file name, an artifact name holding a separator, `:`, `..` or a control character is refused (`ARTIFACT_NAME`) and nothing is written (2026-10-01). unwrapArtifact hands on a wrapper whose inner package cannot be decompressed unchanged, for the validator to judge, instead of throwing ZIP-READ (2026-10-01). `waitForRun` retries a failed poll that may pass on its own - no connection, a 429 or 5xx, a body that is not JSON - up to `MAX_POLL_RETRIES` (5) in a row, reset by a good poll and never past the deadline; 401, 403 and 404 stay fatal (2026-10-01). Every failed poll had ended a watch of up to 30 minutes with RUN_FAILED while the run went on. `getRun` gives a 401, 403 or 404 its own remedy (a missing run, a refused token), and the timeout remedy names `--run-id` (2026-10-01). | ADR-0033, ADR-0032 | diff --git a/research-kit/README.md b/research-kit/README.md index 4bc1236..2d306a1 100644 --- a/research-kit/README.md +++ b/research-kit/README.md @@ -563,7 +563,7 @@ node research-kit/bin/selftest.mjs # all of it node research-kit/bin/selftest.mjs gate hook # just these files ``` -1789 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means +1790 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/artifact.mjs b/research-kit/bin/artifact.mjs index a61693f..71dba3d 100644 --- a/research-kit/bin/artifact.mjs +++ b/research-kit/bin/artifact.mjs @@ -16,7 +16,7 @@ import fs from 'node:fs'; import path from 'node:path'; -import { parseFlags, canonicalJson, checkFlagValues } from '../lib/core.mjs'; +import { parseFlags, canonicalJson, checkFlagValues, exitAfterFlush } from '../lib/core.mjs'; import { requireRuntime } from '../lib/runtime.mjs'; import { validateArtifact } from '../lib/artifact-validator.mjs'; import { writeArtifact, packageName, checkClientRef, GITHUB_API_VERSION } from '../lib/artifact.mjs'; @@ -98,10 +98,10 @@ if (flags.help || !command) { for (const flag of Object.keys(flags).filter((f) => !known.has(f))) process.stderr.write(`unknown option --${flag}\n`); } process.stdout.write(HELP); - process.exit(flags.help ? 0 : 2); + await exitAfterFlush(flags.help ? 0 : 2); } -function refuseUnknownFlags(name) { +async function refuseUnknownFlags(name) { const known = new Set(KNOWN_FLAGS[name] ?? []); const unknown = Object.keys(flags).filter((flag) => !known.has(flag)); if (!unknown.length) return; @@ -113,7 +113,7 @@ function refuseUnknownFlags(name) { } } process.stdout.write(HELP); - process.exit(EXIT.BLOCKED); + await exitAfterFlush(EXIT.BLOCKED); } function printResult(result, { json = false, quiet = false } = {}) { @@ -158,29 +158,29 @@ const VALUE_FLAGS = Object.freeze({ }); if (command === 'validate') { - refuseUnknownFlags('validate'); + await refuseUnknownFlags('validate'); checkFlagValues(flags, VALUE_FLAGS.validate, { exit: EXIT.BLOCKED }); - if (!flags.file) { process.stderr.write('validate needs --file \n\n'); process.stdout.write(HELP); process.exit(EXIT.BLOCKED); } + if (!flags.file) { process.stderr.write('validate needs --file \n\n'); process.stdout.write(HELP); await exitAfterFlush(EXIT.BLOCKED); } const result = validateArtifact({ file: String(flags.file), expectedClientRef: flags['expect-client-ref'] === undefined ? null : String(flags['expect-client-ref']), }); printResult(result, { json: Boolean(flags.json), quiet: Boolean(flags.quiet) }); - process.exit(EXIT[result.status] ?? EXIT.BLOCKED); + await exitAfterFlush(EXIT[result.status] ?? EXIT.BLOCKED); } if (command === 'create') { - refuseUnknownFlags('create'); + await refuseUnknownFlags('create'); checkFlagValues(flags, VALUE_FLAGS.create, { exit: EXIT.BLOCKED }); const required = ['repository', 'ref', 'commit', 'workflow', 'run-id']; const missing = required.filter((name) => flags[name] === undefined); if (missing.length) { process.stderr.write(`create needs ${missing.map((m) => `--${m}`).join(', ')}\n\n`); process.stdout.write(HELP); - process.exit(EXIT.BLOCKED); + await exitAfterFlush(EXIT.BLOCKED); } const refCheck = checkClientRef(flags['client-ref'] === undefined ? null : String(flags['client-ref'])); - if (!refCheck.ok) { process.stderr.write(`${refCheck.detail}\n`); process.exit(EXIT.BLOCKED); } + if (!refCheck.ok) { process.stderr.write(`${refCheck.detail}\n`); await exitAfterFlush(EXIT.BLOCKED); } const runId = Number(flags['run-id']); const root = path.resolve(String(flags.root ?? '.')); @@ -203,7 +203,7 @@ if (command === 'create') { }); } catch (err) { process.stderr.write(`could not package ${root}: ${err.message}\n`); - process.exit(EXIT.BLOCKED); + await exitAfterFlush(EXIT.BLOCKED); } const size = fs.statSync(written.file).size; @@ -214,12 +214,12 @@ if (command === 'create') { if (written.validation.status !== 'PASS') { process.stderr.write('\nthe package this run just wrote does NOT validate:\n'); for (const e of written.validation.errors) process.stderr.write(` ${e.code} ${e.message}\n`); - process.exit(EXIT[written.validation.status] ?? EXIT.BLOCKED); + await exitAfterFlush(EXIT[written.validation.status] ?? EXIT.BLOCKED); } process.stdout.write('re-validated after writing: PASS\n'); - process.exit(EXIT.PASS); + await exitAfterFlush(EXIT.PASS); } process.stderr.write(`unknown command ${JSON.stringify(command)}\n\n`); process.stdout.write(HELP); -process.exit(EXIT.BLOCKED); +await exitAfterFlush(EXIT.BLOCKED); diff --git a/research-kit/test/artifact-cli.test.mjs b/research-kit/test/artifact-cli.test.mjs index 88e3e89..1ce5adf 100644 --- a/research-kit/test/artifact-cli.test.mjs +++ b/research-kit/test/artifact-cli.test.mjs @@ -208,6 +208,27 @@ test('--quiet prints the verdict line and nothing else', () => { assert.ok(/^PASS\s+build NOT authorized/.test(lines[0]), lines[0]); }); +test('large validation reports drain piped stdout in JSON and text modes', () => { + const { payload, manifest } = basePackage(collectedProject()); + const template = manifest.files.find((entry) => entry.path.startsWith('project/research/raw/')); + assert.ok(template); + for (let index = 0; index < 700; index += 1) { + manifest.files.push({ ...template, path: `project/research/raw/missing-${String(index).padStart(4, '0')}-${'x'.repeat(70)}.md` }); + } + const file = path.join(scratch, 'large-missing.zip'); + fs.writeFileSync(file, seal(payload, manifest, { recomputeFiles: false })); + const json = run(['validate', '--file', file, '--json'], { maxBuffer: 4 * 1024 * 1024 }); + assert.equal(json.code, 1, json.err); + const parsed = JSON.parse(json.out); + assert.equal(parsed.errors.filter((error) => error.code === 'FILE-MISSING').length, 700); + assert.ok(Buffer.byteLength(json.out) > 65536); + const text = run(['validate', '--file', file], { maxBuffer: 4 * 1024 * 1024 }); + assert.equal(text.code, 1, text.err); + assert.equal((text.out.match(/FILE-MISSING/g) ?? []).length, 700); + assert.ok(Buffer.byteLength(text.out) > 65536); + assert.match(text.out, /FAIL\s+build NOT authorized/); +}); + // ---------------------------------------------------------------- create test('create writes a package, re-validates it, and says so', () => {